fix(MAJORLEA-002): 5 review findings across 3 files - #75
Conversation
| @Autowired | ||
| private LanguageService languageService; | ||
|
|
||
| @Autowired |
There was a problem hiding this comment.
🦩 🔴 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
| import lombok.extern.slf4j.Slf4j; | ||
|
|
||
| @Slf4j | ||
| @RestController |
There was a problem hiding this comment.
🦩 🔴 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
|
|
||
| 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(() -> { |
There was a problem hiding this comment.
🦩 🔴 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
| .title("Senior Back-end Engineer") | ||
| .location("Remote") | ||
| .url("https://djinni.co/jobs/717621-senior-back-end-engineer/") | ||
| .build(), |
There was a problem hiding this comment.
🦩 🟠 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
| @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", |
There was a problem hiding this comment.
🦩 🔴 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
Closes 5 review findings across 3 files.
Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.
backend/src/main/java/cx/flamingo/analysis/controller/EntityController.java:39backend/src/main/java/cx/flamingo/analysis/controller/EntityController.java:23backend/src/main/java/cx/flamingo/analysis/service/HiringService.java:44backend/src/main/java/cx/flamingo/analysis/service/HiringService.java:80backend/src/main/java/cx/flamingo/analysis/controller/AutocompleteController.java:37What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.
Run: https://product-hub.flamingo.so/admin/code-review
Run id:
8f1c6ef6-6b61-4dcd-bb0e-59bc6a7d37e8Merging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.