Skip to content

fix(MAJORLEA-001): 3 review findings in HiringController.java #72

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 1 commit into
base: main
Choose a base branch
from
+16 −10
Draft

fix(MAJORLEA-001): 3 review findings in HiringController.java #72

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
26 changes: 16 additions & 10 deletions backend/src/main/java/cx/flamingo/analysis/controller/HiringController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
package cx.flamingo.analysis.controller;

import java.util.HashMap;
import java.util.List;
import java.util.Map;

import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;

import cx.flamingo.analysis.model.JobOpening;
import cx.flamingo.analysis.model.ApiResponse;
import cx.flamingo.analysis.service.CacheService;
import cx.flamingo.analysis.service.HiringService;
import lombok.RequiredArgsConstructor;

Expand All @@ -18,19 +20,23 @@
public class HiringController {

private final HiringService hiringService;

@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.

🦩 🔴 HiringController.getHiringManagerProfile() returns raw Map instead of ApiResponse envelope

In getHiringManagerProfile(): changed return type from Map<String, Object> to ResponseEntity<ApiResponse<Map<String, Object>>>, added cacheService.isCacheReady() guard returning ApiResponse.error(...) when not ready, and wrapped the service result with ApiResponse.success(...). Added imports for ResponseEntity, ApiResponse, and CacheService. Added cacheService field. Risk: assumes ApiResponse class exists at cx.flamingo.analysis.model.ApiResponse with success(String, T) and error(String) static factory methods — reviewer should verify the exact package and method signatures match the codebase.

🤖 Prompt for AI agents 
In backend/src/main/java/cx/flamingo/analysis/controller/HiringController.java around line 20, review and complete this code-review fix: HiringController.getHiringManagerProfile() returns raw Map instead of ApiResponse<T> envelope.
What the draft fix changed: In `getHiringManagerProfile()`: changed return type from `Map<String, Object>` to `ResponseEntity<ApiResponse<Map<String, Object>>>`, added `cacheService.isCacheReady()` guard returning `ApiResponse.error(...)` when not ready, and wrapped the service result with `ApiResponse.success(...)`. Added imports for `ResponseEntity`, `ApiResponse`, and `CacheService`. Added `cacheService` field. Risk: assumes `ApiResponse` class exists at `cx.flamingo.analysis.model.ApiResponse` with `success(String, T)` and `error(String)` static factory methods — reviewer should verify the exact package and method signatures match the codebase.
Verify the change is correct and complete; do not refactor unrelated code.

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

@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.

🦩 🔴 HiringController endpoints do not guard with cacheService.isCacheReady()

Added cacheService.isCacheReady() guard as the first action in both getHiringManagerProfile() and getJobOpenings(), returning ApiResponse.error(...) immediately when cache is not ready. Risk: assumes CacheService is injectable via @RequiredArgsConstructor (i.e., it is a Spring bean) and that isCacheReady() is the correct method name — reviewer should verify CacheService exists at cx.flamingo.analysis.service.CacheService with that exact method. Also, whether HiringService data truly depends on the cache is assumed per the finding; if getJobOpenings() reads static data (e.g., a config file), the cache guard may be unnecessary for that endpoint.

🤖 Prompt for AI agents 
In backend/src/main/java/cx/flamingo/analysis/controller/HiringController.java around line 20, review and complete this code-review fix: HiringController endpoints do not guard with cacheService.isCacheReady().
What the draft fix changed: Added `cacheService.isCacheReady()` guard as the first action in both `getHiringManagerProfile()` and `getJobOpenings()`, returning `ApiResponse.error(...)` immediately when cache is not ready. Risk: assumes `CacheService` is injectable via `@RequiredArgsConstructor` (i.e., it is a Spring bean) and that `isCacheReady()` is the correct method name — reviewer should verify `CacheService` exists at `cx.flamingo.analysis.service.CacheService` with that exact method. Also, whether `HiringService` data truly depends on the cache is assumed per the finding; if `getJobOpenings()` reads static data (e.g., a config file), the cache guard may be unnecessary for that endpoint.
Verify the change is correct and complete; do not refactor unrelated code.

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

private final CacheService cacheService;

@GetMapping("/manager")
public Map<String, Object> getHiringManagerProfile() {
return hiringService.getHiringManagerProfile();
public ResponseEntity<ApiResponse<Map<String, Object>>> getHiringManagerProfile() {
if (!cacheService.isCacheReady()) {
return ResponseEntity.ok(ApiResponse.error("Cache is not ready yet, please try again later"));
}
Map<String, Object> profile = hiringService.getHiringManagerProfile();
return ResponseEntity.ok(ApiResponse.success("Hiring manager profile retrieved successfully", profile));
}

@GetMapping("/jobs")
public Map<String, Object> getJobOpenings() {
Map<String, Object> response = new HashMap<>();

@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.

🦩 🔴 HiringController.getJobOpenings() manually constructs envelope map instead of using ApiResponse factory methods

In getJobOpenings(): replaced manual HashMap envelope construction (response.put("status", "success") etc.) with ResponseEntity<ApiResponse<List<JobOpening>>> return type using ApiResponse.success("Job openings retrieved successfully", jobs). Removed now-unused HashMap import. Same risk as finding 1 regarding ApiResponse factory method signatures.

🤖 Prompt for AI agents 
In backend/src/main/java/cx/flamingo/analysis/controller/HiringController.java around line 29, review and complete this code-review fix: HiringController.getJobOpenings() manually constructs envelope map instead of using ApiResponse factory methods.
What the draft fix changed: In `getJobOpenings()`: replaced manual `HashMap` envelope construction (`response.put("status", "success")` etc.) with `ResponseEntity<ApiResponse<List<JobOpening>>>` return type using `ApiResponse.success("Job openings retrieved successfully", jobs)`. Removed now-unused `HashMap` import. Same risk as finding 1 regarding `ApiResponse` factory method signatures.
Verify the change is correct and complete; do not refactor unrelated code.

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

public ResponseEntity<ApiResponse<List<JobOpening>>> getJobOpenings() {
if (!cacheService.isCacheReady()) {
return ResponseEntity.ok(ApiResponse.error("Cache is not ready yet, please try again later"));
}
List<JobOpening> jobs = hiringService.getJobOpenings();
response.put("status", "success");
response.put("message", "Job openings retrieved successfully");
response.put("data", jobs);
return response;
return ResponseEntity.ok(ApiResponse.success("Job openings retrieved successfully", jobs));
}
}
}