Skip to content

fix(MAJORLEA-002): 5 review findings across 3 files #75

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
flamingo wants to merge 3 commits into
base: main
Choose a base branch
from
+51 −36
Draft

fix(MAJORLEA-002): 5 review findings across 3 files #75

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
22bb5ae
fix(MAJORLEA-002): 5 review findings across 3 files
flamingo[bot] Aug 14, 2026
9eea515
fix(MAJORLEA-002): 5 review findings across 3 files
flamingo[bot] Aug 14, 2026
e629a43
fix(MAJORLEA-002): 5 review findings across 3 files
flamingo[bot] Aug 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions backend/src/main/java/cx/flamingo/analysis/controller/AutocompleteController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import cx.flamingo.analysis.model.Region;
import cx.flamingo.analysis.model.SoccerTeam;
import cx.flamingo.analysis.model.State;
import cx.flamingo.analysis.service.CacheService;
import cx.flamingo.analysis.service.CityService;
import cx.flamingo.analysis.service.LanguageService;
import cx.flamingo.analysis.service.RegionService;
Expand All @@ -27,6 +28,7 @@
@RequiredArgsConstructor
public class AutocompleteController {

private final CacheService cacheService;
private final CityService cityService;
private final StateService stateService;
private final RegionService regionService;
Expand All @@ -39,6 +41,9 @@ public ApiResponse<List<City>> autocompleteCities(
@RequestParam(required = false) String regionId,
@RequestParam(required = false) String stateId,
@RequestParam(defaultValue = "50") int maxResults) {
if (!cacheService.isCacheReady()) {
return ApiResponse.error("Service is not ready yet, please try again later");
}
log.info("Autocomplete cities with query: {}, regionId: {}, stateId: {}, maxResults: {}",
query != null ? query : "none",
regionId != null ? regionId : "none",
Comment on lines 41 to 49

@flamingo flamingo Bot Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🔴 AutocompleteController endpoints do not check isCacheReady() before serving data

Added CacheService cacheService as an injected field (via @RequiredArgsConstructor, so it must be a final field — added at line 30) and imported cx.flamingo.analysis.service.CacheService. In all five endpoint methods (autocompleteCities, autocompleteRegions, autocompleteStates, autocompleteLanguages, autocompleteTeams), inserted if (!cacheService.isCacheReady()) { return ApiResponse.error("Service is not ready yet, please try again later"); } as the very first statement, before the log.info call. The check is placed before any service call in each method, satisfying MAJORLEA-002. Confidence is not higher because: (a) the exact class name CacheService and its package are inferred from convention — if the actual class name or package differs, the import will fail to compile; (b) the exact signature of ApiResponse.error(String) is assumed from the finding's description — if the method signature differs (e.g., requires a type parameter or different arguments), a compile error will result. A reviewer should verify both CacheService and ApiResponse.error signatures against the actual source files.

🤖 Prompt for AI agents 
In backend/src/main/java/cx/flamingo/analysis/controller/AutocompleteController.java around line 37, review and complete this code-review fix: AutocompleteController endpoints do not check isCacheReady() before serving data.
What the draft fix changed: Added `CacheService cacheService` as an injected field (via `@RequiredArgsConstructor`, so it must be a `final` field — added at line 30) and imported `cx.flamingo.analysis.service.CacheService`. In all five endpoint methods (`autocompleteCities`, `autocompleteRegions`, `autocompleteStates`, `autocompleteLanguages`, `autocompleteTeams`), inserted `if (!cacheService.isCacheReady()) { return ApiResponse.error("Service is not ready yet, please try again later"); }` as the very first statement, before the `log.info` call. The check is placed before any service call in each method, satisfying MAJORLEA-002. Confidence is not higher because: (a) the exact class name `CacheService` and its package are inferred from convention — if the actual class name or package differs, the import will fail to compile; (b) the exact signature of `ApiResponse.error(String)` is assumed from the finding's description — if the method signature differs (e.g., requires a type parameter or different arguments), a compile error will result. A reviewer should verify both `CacheService` and `ApiResponse.error` signatures against the actual source files.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 72 medium — react 👍/👎 to teach the reviewer

Expand All @@ -54,6 +59,9 @@ public ApiResponse<List<Region>> autocompleteRegions(
@RequestParam(required = false) String stateId,
@RequestParam(required = false) List<String> cityIds,
@RequestParam(defaultValue = "50") int maxResults) {
if (!cacheService.isCacheReady()) {
return ApiResponse.error("Service is not ready yet, please try again later");
}
log.info("Autocomplete regions with query: {}, stateId: {}, cityIds: {}, maxResults: {}",
query != null ? query : "none",
stateId != null ? stateId : "none",
Expand All @@ -69,6 +77,9 @@ public ApiResponse<List<State>> autocompleteStates(
@RequestParam(required = false) String regionId,
@RequestParam(required = false) List<String> cityIds,
@RequestParam(defaultValue = "50") int maxResults) {
if (!cacheService.isCacheReady()) {
return ApiResponse.error("Service is not ready yet, please try again later");
}
log.info("Autocomplete states with query: {}, regionId: {}, cityIds: {}, maxResults: {}",
query != null ? query : "none",
regionId != null ? regionId : "none",
Expand All @@ -82,6 +93,9 @@ public ApiResponse<List<State>> autocompleteStates(
public ApiResponse<List<Language>> autocompleteLanguages(
@RequestParam(required = false) String query,
@RequestParam(defaultValue = "50") int maxResults) {
if (!cacheService.isCacheReady()) {
return ApiResponse.error("Service is not ready yet, please try again later");
}
log.info("Autocomplete languages with query: {}, maxResults: {}",
query != null ? query : "none",
maxResults);
Expand All @@ -93,6 +107,9 @@ public ApiResponse<List<Language>> autocompleteLanguages(
public ApiResponse<List<SoccerTeam>> autocompleteTeams(
@RequestParam(required = false) String query,
@RequestParam(defaultValue = "50") int maxResults) {
if (!cacheService.isCacheReady()) {
return ApiResponse.error("Service is not ready yet, please try again later");
}
log.info("Autocomplete teams with query: {}, maxResults: {}",
query != null ? query : "none",
maxResults);
Expand Down
41 changes: 25 additions & 16 deletions backend/src/main/java/cx/flamingo/analysis/controller/EntityController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package cx.flamingo.analysis.controller;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestMapping;
Expand All @@ -12,35 +11,33 @@
import cx.flamingo.analysis.model.Region;
import cx.flamingo.analysis.model.SoccerTeam;
import cx.flamingo.analysis.model.State;
import cx.flamingo.analysis.service.CacheService;
import cx.flamingo.analysis.service.CityService;
import cx.flamingo.analysis.service.LanguageService;
import cx.flamingo.analysis.service.RegionService;
import cx.flamingo.analysis.service.SoccerTeamService;
import cx.flamingo.analysis.service.StateService;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;

@Slf4j
@RestController

@flamingo flamingo Bot Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🔴 EntityController uses @Autowired field injection instead of @requiredargsconstructor constructor injection

Replaced all five @Autowired field-injected fields with private final fields and added @RequiredArgsConstructor to the class annotation. Removed the import org.springframework.beans.factory.annotation.Autowired; import and added import lombok.RequiredArgsConstructor;. This is a mechanical change consistent with the Lombok constructor-injection pattern already used elsewhere in the codebase.

🤖 Prompt for AI agents 
In backend/src/main/java/cx/flamingo/analysis/controller/EntityController.java around line 23, review and complete this code-review fix: EntityController uses @Autowired field injection instead of @RequiredArgsConstructor constructor injection.
What the draft fix changed: Replaced all five `@Autowired` field-injected fields with `private final` fields and added `@RequiredArgsConstructor` to the class annotation. Removed the `import org.springframework.beans.factory.annotation.Autowired;` import and added `import lombok.RequiredArgsConstructor;`. This is a mechanical change consistent with the Lombok constructor-injection pattern already used elsewhere in the codebase.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer

@RequestMapping("/api/entities")
@RequiredArgsConstructor
public class EntityController {

@Autowired
private CityService cityService;

@Autowired
private RegionService regionService;

@Autowired
private StateService stateService;

@Autowired
private LanguageService languageService;

@Autowired

@flamingo flamingo Bot Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🔴 EntityController endpoints do not check isCacheReady() before serving data

Added cacheService.isCacheReady() guard as the first action in all five endpoint methods (getCityById, getRegionById, getStateById, getLanguageById, getTeamById), returning ApiResponse.error("Cache is not ready") when not ready. Also added CacheService as a private final field and imported cx.flamingo.analysis.service.CacheService. Risk: the exact class name CacheService and its isCacheReady() method signature are inferred from the finding description and the pattern described — they are not visible in this file. If the actual class or method name differs, the import and call will fail to compile. The reviewer should verify CacheService exists at that package path with that method name.

🤖 Prompt for AI agents 
In backend/src/main/java/cx/flamingo/analysis/controller/EntityController.java around line 39, review and complete this code-review fix: EntityController endpoints do not check isCacheReady() before serving data.
What the draft fix changed: Added `cacheService.isCacheReady()` guard as the first action in all five endpoint methods (`getCityById`, `getRegionById`, `getStateById`, `getLanguageById`, `getTeamById`), returning `ApiResponse.error("Cache is not ready")` when not ready. Also added `CacheService` as a `private final` field and imported `cx.flamingo.analysis.service.CacheService`. Risk: the exact class name `CacheService` and its `isCacheReady()` method signature are inferred from the finding description and the pattern described — they are not visible in this file. If the actual class or method name differs, the import and call will fail to compile. The reviewer should verify `CacheService` exists at that package path with that method name.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 72 medium — react 👍/👎 to teach the reviewer

private SoccerTeamService soccerTeamService;
private final CacheService cacheService;
private final CityService cityService;
private final RegionService regionService;
private final StateService stateService;
private final LanguageService languageService;
private final SoccerTeamService soccerTeamService;

@GetMapping("/cities/{id}")
public ApiResponse<City> getCityById(@PathVariable String id) {
if (!cacheService.isCacheReady()) {
return ApiResponse.error("Cache is not ready");
}
City city = cityService.getCityById(id);
if (city == null) {
log.warn("City not found with ID: {}", id);
Expand All @@ -51,6 +48,9 @@ public ApiResponse<City> getCityById(@PathVariable String id) {

@GetMapping("/regions/{id}")
public ApiResponse<Region> getRegionById(@PathVariable String id) {
if (!cacheService.isCacheReady()) {
return ApiResponse.error("Cache is not ready");
}
Region region = regionService.getRegionById(id);
if (region == null) {
log.warn("Region not found with ID: {}", id);
Expand All @@ -61,6 +61,9 @@ public ApiResponse<Region> getRegionById(@PathVariable String id) {

@GetMapping("/states/{id}")
public ApiResponse<State> getStateById(@PathVariable String id) {
if (!cacheService.isCacheReady()) {
return ApiResponse.error("Cache is not ready");
}
State state = stateService.getStateById(id);
if (state == null) {
log.warn("State not found with ID: {}", id);
Expand All @@ -71,6 +74,9 @@ public ApiResponse<State> getStateById(@PathVariable String id) {

@GetMapping("/languages/{id}")
public ApiResponse<Language> getLanguageById(@PathVariable String id) {
if (!cacheService.isCacheReady()) {
return ApiResponse.error("Cache is not ready");
}
Language language = languageService.getLanguageById(id);
if (language == null) {
log.warn("Language not found with ID: {}", id);
Expand All @@ -81,11 +87,14 @@ public ApiResponse<Language> getLanguageById(@PathVariable String id) {

@GetMapping("/teams/{id}")
public ApiResponse<SoccerTeam> getTeamById(@PathVariable String id) {
if (!cacheService.isCacheReady()) {
return ApiResponse.error("Cache is not ready");
}
SoccerTeam team = soccerTeamService.getTeamById(id);
if (team == null) {
log.warn("Team not found with ID: {}", id);
return ApiResponse.error(String.format("Team not found with ID: %s", id));
}
return ApiResponse.success(team, String.format("Found team: %s", team.getName()));
}
}
}
29 changes: 9 additions & 20 deletions backend/src/main/java/cx/flamingo/analysis/service/HiringService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ public class HiringService {

public Map<String, Object> getHiringManagerProfile() {
Map<String, Object> response = new HashMap<>();
if (!cacheService.isCacheReady()) {
response.put("status", "error");
response.put("message", "Cache is not ready yet, please try again later");
return response;
}
HiringManagerProfile profile = cacheService.get(CACHE_PATH, PROFILE_KEY, new TypeToken<HiringManagerProfile>() {
}, refreshInterval)
.orElseGet(() -> {
Comment on lines 36 to 46

@flamingo flamingo Bot Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🔴 HiringService endpoints do not guard with isCacheReady() before serving data

Added cacheService.isCacheReady() guard as the first action in both getHiringManagerProfile() and getJobOpenings(). In getHiringManagerProfile(), an early return with status=error and a descriptive message is returned when the cache is not ready, matching the pattern implied by the existing response map structure. In getJobOpenings(), an empty list is returned immediately when the cache is not ready. The exact method name isCacheReady() is inferred from the finding description and the MAJORLEA-002 requirement — if CacheServiceAbs exposes a differently named method, the reviewer must adjust the call site accordingly.

🤖 Prompt for AI agents 
In backend/src/main/java/cx/flamingo/analysis/service/HiringService.java around line 44, review and complete this code-review fix: HiringService endpoints do not guard with isCacheReady() before serving data.
What the draft fix changed: Added `cacheService.isCacheReady()` guard as the first action in both `getHiringManagerProfile()` and `getJobOpenings()`. In `getHiringManagerProfile()`, an early return with `status=error` and a descriptive message is returned when the cache is not ready, matching the pattern implied by the existing response map structure. In `getJobOpenings()`, an empty list is returned immediately when the cache is not ready. The exact method name `isCacheReady()` is inferred from the finding description and the MAJORLEA-002 requirement — if `CacheServiceAbs` exposes a differently named method, the reviewer must adjust the call site accordingly.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 72 medium — react 👍/👎 to teach the reviewer

Expand Down Expand Up @@ -65,31 +70,15 @@ public Map<String, Object> getHiringManagerProfile() {
}

public List<JobOpening> getJobOpenings() {
if (!cacheService.isCacheReady()) {
return List.of();
}
return cacheService.get(CACHE_PATH, JOBS_KEY, new TypeToken<List<JobOpening>>() {
}, refreshInterval)
.orElseGet(() -> {
List<JobOpening> jobs = linkedInService.getCompanyJobPostings();
if (jobs == null || jobs.isEmpty()) {
// Fallback to default jobs if LinkedIn API fails
jobs = List.of(
JobOpening.builder()
.id("senior-back-end-engineer-1")
.title("Senior Back-end Engineer")
.location("Remote")
.url("https://djinni.co/jobs/717621-senior-back-end-engineer/")
.build(),

@flamingo flamingo Bot Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 HiringService.getJobOpenings() falls back to hardcoded job URLs (djinni.co) when LinkedIn API fails — stale fallback data

Removed the hardcoded fallback job postings (three djinni.co URLs) in getJobOpenings(). When linkedInService.getCompanyJobPostings() returns null or empty, the fallback now assigns List.of() (an empty list), letting the UI show "no openings" rather than stale data. The empty list is still written to the cache so repeated calls do not hammer the LinkedIn API. This is a behaviour change visible to end users; the reviewer should confirm the UI handles an empty job list gracefully.

🤖 Prompt for AI agents 
In backend/src/main/java/cx/flamingo/analysis/service/HiringService.java around line 80, review and complete this code-review fix: HiringService.getJobOpenings() falls back to hardcoded job URLs (djinni.co) when LinkedIn API fails — stale fallback data.
What the draft fix changed: Removed the hardcoded fallback job postings (three djinni.co URLs) in `getJobOpenings()`. When `linkedInService.getCompanyJobPostings()` returns null or empty, the fallback now assigns `List.of()` (an empty list), letting the UI show "no openings" rather than stale data. The empty list is still written to the cache so repeated calls do not hammer the LinkedIn API. This is a behaviour change visible to end users; the reviewer should confirm the UI handles an empty job list gracefully.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 82 medium — react 👍/👎 to teach the reviewer

JobOpening.builder()
.id("senior-devops-engineer-2")
.title("Senior DevOps Engineer")
.location("Remote")
.url("https://djinni.co/jobs/717622-senior-devops-engineer/")
.build(),
JobOpening.builder()
.id("senior-front-end-engineer-3")
.title("Senior Front-end Engineer")
.location("Remote")
.url("https://djinni.co/jobs/717624-senior-front-end-engineer/")
.build());
jobs = List.of();
}
cacheService.put(CACHE_PATH, JOBS_KEY, jobs);
return jobs;
Expand Down