Feature/visa view controller#27
Merged
Merged
Conversation
…templates - Added endpoints for to show and manage visa applications - Added missing fields from Visa Entity to DTO and mapper - Added start of thymeleaf templates to display visa cases, apply for a new visa and show further details about a case
… role-based views - Removed UNASSIGNED from VisaStatus for improved flow - Replaced Reject reason with statusInformation - Added logic for admin users to request further information from applicants
…endpoint - Added missing @PostMapping for request-info in VisaViewController - Fixed HTML form issue where the reason parameter was not correctly sent to the server
- Updated logic to sort applications by last updated for admins - Implemented chronological sorting for regular users
…tails - UI: Updated Dashboard, Details and Apply-form to dark/minimal themed design. - Added passportNumber and travelDate to Visa entity and DTOs. - Added travel date validation in VisaService
- Implement server-side validation using @Valid and BindingResult - Add error message handling in VisaViewController - Map Service-layer exceptions to specific form fields
- Created new edit-form.html to allow applicants to update their visa details. - Added showEditForm and processUpdate to VisaViewController - Added "Action Required" alert in details view for incomplete applications - Integrated handler feedback directly into the edit form for better overview.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Thymeleaf UI and MVC controller for visa workflows, extends Visa domain with passport/travelDate/statusInformation/audit timestamps, tightens DTO validation, updates mapper/repository/service logic and tests, seeds users at startup, introduces permissive security config, and adds a global exception handler with logging. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant VisaViewController
participant VisaService
participant VisaRepository
participant AuditService
Browser->>VisaViewController: POST /visas/apply (CreateVisaDTO)
VisaViewController->>VisaService: applyForVisa(dto, currentUserId)
VisaService->>VisaRepository: save(Visa entity)
VisaRepository-->>VisaService: saved Visa
VisaService->>AuditService: record AuditEvent(SUBMITTED)
AuditService-->>VisaService: ack
VisaService-->>VisaViewController: VisaDTO / redirect
VisaViewController-->>Browser: Redirect /visas/dashboard?currentUserId=...
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (4)
src/test/java/org/example/visacasemanagementsystem/visa/VisaMapperTest.java (1)
62-71: Add an assertion forhandlerNamefallback behavior.In this test,
handleris unset, so assertingdto.handlerName()equals"Unassigned"would verify the new mapper contract explicitly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/example/visacasemanagementsystem/visa/VisaMapperTest.java` around lines 62 - 71, The test VisaMapperTest currently checks fields from visaMapper.toDTO(visa) but misses asserting the handlerName fallback; update the test to assert that dto.handlerName() equals "Unassigned" when the source Visa has no handler set, ensuring the mapper's fallback behavior is explicitly verified (locate the assertion block after the existing assertions on dto.id(), dto.visaType(), etc., and add the dto.handlerName() check).src/main/java/org/example/visacasemanagementsystem/visa/dto/CreateVisaDTO.java (1)
15-15: Consider removingapplicantIdfrom the client-facing create DTO.Line 15 keeps applicant identity writable by clients. Since apply flow already receives server-side user context, this field is safer to derive on the backend only.
Suggested refactor
public record CreateVisaDTO( `@NotNull`(message = "Visa type must be specified") VisaType visaType, `@NotBlank`(message = "Nationality must be specified") String nationality, `@NotBlank`(message = "Passport number must be provided") String passportNumber, - `@NotNull`(message = "Travel date must be provided") `@DateTimeFormat`(iso = DateTimeFormat.ISO.DATE) LocalDate travelDate, - Long applicantId + `@NotNull`(message = "Travel date must be provided") `@DateTimeFormat`(iso = DateTimeFormat.ISO.DATE) LocalDate travelDate ) { }Based on learnings: In this repository, trusting client-provided identity fields (like
authorId) is a known deferred security gap and should be replaced by resolving the authenticated principal server-side when possible.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/visacasemanagementsystem/visa/dto/CreateVisaDTO.java` at line 15, Remove the mutable applicant identity from the client-facing create DTO by deleting the applicantId field from CreateVisaDTO and updating any usages (e.g., constructors, builders, validation code) that accept or map applicantId; ensure server-side code that processes visa creation (controller/servlet method handling the create request and service method that accepts CreateVisaDTO) instead resolves the applicant identity from the authenticated principal/session and sets it onto the domain model before persisting.src/test/java/org/example/visacasemanagementsystem/visa/VisaServiceTest.java (1)
368-381: Test method name is misleading.The method name
validateHandler_shouldThrowEntityNotFoundException_WhenUserIsApplicantsuggests it tests the applicant scenario, but it actually tests when the user doesn't exist. Consider renaming for clarity.✏️ Suggested rename
- void validateHandler_shouldThrowEntityNotFoundException_WhenUserIsApplicant() { + void validateHandler_shouldThrowEntityNotFoundException_WhenUserDoesNotExist() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/example/visacasemanagementsystem/visa/VisaServiceTest.java` around lines 368 - 381, The test method validateHandler_shouldThrowEntityNotFoundException_WhenUserIsApplicant has a misleading name; rename it to reflect that it covers the non-existent user case (for example validateHandler_shouldThrowEntityNotFoundException_WhenUserNotFound) and update any references to the method name in the test class; keep the test body (mocking userRepository.findById(nonExistingUserId) to return Optional.empty(), asserting EntityNotFoundException with message "User not found.", and verifyNoInteractions(visaRepository)) unchanged.src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java (1)
269-291: Consider validatinginfoTextparameter inrequestMoreInformation.Unlike
rejectVisawhich validates that reason is not blank (line 244-246),requestMoreInformationaccepts anyinfoTextincluding null or empty strings. This inconsistency could lead to INCOMPLETE visas with no actionable feedback for applicants.♻️ Add validation for consistency
`@Transactional` public VisaDTO requestMoreInformation(Long visaId, Long adminId, String infoText) { + if (infoText == null || infoText.isBlank()) { + throw new IllegalArgumentException("Information request text cannot be null or blank"); + } User admin = validateHandler(adminId); Visa visa = findVisaById(visaId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java` around lines 269 - 291, requestMoreInformation currently accepts null or blank infoText which can create INCOMPLETE visas with no feedback; add a validation at the start of requestMoreInformation to ensure infoText is non-null and not blank (trim it and check length), and if invalid throw the same type of validation/BadRequest exception your service uses elsewhere (mirror the check used in rejectVisa), then proceed to call validateHandler(adminId), findVisaById(visaId), set the handler/status/info, save via visaRepository.save, and create the audit log as before; reference the method requestMoreInformation and reuse the same validation style/exception as rejectVisa for consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/org/example/visacasemanagementsystem/exception/GlobalExceptionHandler.java`:
- Around line 36-41: The handler GlobalExceptionHandler.handleAllUncaughtErrors
currently returns exception.getMessage(), which can leak internals/PII; change
it to return a generic 500 message (e.g., "Internal server error" or "An
unexpected error occurred") instead, keep the existing log.error("Unexpected
Error: ", exception) to record details, and optionally include a
correlation/error id in both the log and the response if you want traceability;
update the method return to the generic string and do not expose
exception.getMessage().
In
`@src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java`:
- Around line 107-109: In VisaViewController where the authorization check
throws new UnauthorizedException("You can only edit ypur own applications."),
correct the typo in the exception message to "your" so the thrown
UnauthorizedException reads "You can only edit your own applications."; update
the string in the throw statement inside the method performing the applicantId
vs currentUserId check.
- Around line 138-151: Validation error paths don't re-add the
"statusInformation" model attribute, so update both the
bindingResult.hasErrors() branch and the IllegalArgumentException catch branch
in VisaViewController.edit flow to re-add statusInformation before returning the
view; call model.addAttribute("statusInformation",
visaService.getStatusInformation(currentUserId)) (or the equivalent method that
returns the current user's status info) after prepareApplyModel(...) and before
model.addAttribute("isEdit", true) so the "Feedback from Handler" banner renders
on the re-displayed "visa/edit-form".
- Around line 187-204: The viewDetails method in VisaViewController currently
returns any visa by id without authorization; update viewDetails to verify that
the requesting user (obtained via userService.findById(currentUserId)) is
allowed to see the VisaDTO returned by visaService.findVisaDtoById(id) by
checking either that the user has an admin/STAFF role or that
visa.getApplicantId() (or equivalent owner field on VisaDTO) equals
currentUserId, and if not throw an AccessDeniedException or return a 403
response; ensure the check happens before adding the visa to the model so only
authorized users can see passport/nationality data.
In `@src/main/java/org/example/visacasemanagementsystem/visa/entity/Visa.java`:
- Around line 90-93: The Visa.toString() currently prints full sensitive and
heavy objects (passportNumber, applicant, handler); change it to mask
passportNumber (e.g., replace all but last 2–4 chars with asterisks or a fixed
token) and avoid embedding full User objects—include only their identifiers
(e.g., applicant.getId() and handler.getId()) or null-safe id getters; update
the toString() implementation in class Visa to use the maskedPassport (derived
from passportNumber) and applicantId/handlerId instead of applicant and handler
to prevent PII leakage and large recursive logs.
In
`@src/main/java/org/example/visacasemanagementsystem/visa/repository/VisaRepository.java`:
- Line 27: The repository method findByApplicantId in VisaRepository doesn't
match the Visa entity (Visa has a User applicant reference), so rename the
derived query to findByApplicant_Id (keeping the Sort parameter) so Spring Data
JPA resolves the path to the associated applicant's id; update the method
signature in VisaRepository to use findByApplicant_Id(Long applicantId, Sort
sort).
In
`@src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java`:
- Around line 307-309: The audit message in VisaService that constructs the
ASSIGNED event uses string concatenation "Admin" + admin.getFullName(),
producing "AdminJohn Doe"; update the concatenation to include a space (e.g.,
"Admin " + admin.getFullName()) or use a formatted string when creating the
AuditEventType.ASSIGNED message so the audit log reads "Admin John Doe has been
assigned to case and status is now ASSIGNED."
In `@src/main/resources/templates/visa/dashboard.html`:
- Around line 155-161: The status cell's <td> wrapping the <span class="status"
th:classappend="${'status-' + `#strings.toLowerCase`(visa.visaStatus)}"
th:text="${visa.visaStatus}"> is not closed, causing malformed table HTML; fix
by adding the missing closing </td> after the span (i.e., close the <td> that
contains the status badge before the next <td> that renders travelDate) so the
table row's cells are properly balanced.
- Around line 125-126: The template links the Sign Out button to /visas/logout
but no handler exists; either add a logout endpoint in VisaViewController (e.g.,
create a method like logout or handleLogout with a mapping for "/visas/logout"
that delegates to SecurityContextLogoutHandler or redirects to the security
logout URL) or modify the template anchor to use Spring Security's default
logout endpoint by changing th:href to "@{/logout}"; update the controller or
template consistently and ensure any CSRF token or POST logout requirements are
handled per your security configuration.
In `@src/main/resources/templates/visa/details.html`:
- Around line 196-218: The request-more-info flow allows an empty reason because
the shared `#infoReason` input sits outside the form so client-side required
validation is bypassed and VisaService.requestMoreInformation does not validate
blank infoText; add server-side validation in VisaService.requestMoreInformation
to reject empty or whitespace-only infoText (same pattern used by the rejectVisa
path), returning/throwing a validation error so the controller can respond
appropriately, and optionally ensure the form wiring uses `#form-info` and
hidden-info consistently to avoid client-side bypass.
- Around line 237-244: The form won't work because CommentController expects
JSON via `@RequestBody` and CreateCommentDTO uses field name text; fix by adding a
view handler in VisaViewController (e.g., addComment with
`@PostMapping`("/{id}/comments") that accepts `@RequestParam` Long visaId,
`@RequestParam` Long authorId, `@RequestParam` String text and calls
commentService.createComment(new CreateCommentDTO(visaId, authorId, text)) then
redirects back to the visa details page), and update the template form to POST
to that view endpoint and rename the textarea from name="content" to name="text"
so the field names match CreateCommentDTO.
---
Nitpick comments:
In
`@src/main/java/org/example/visacasemanagementsystem/visa/dto/CreateVisaDTO.java`:
- Line 15: Remove the mutable applicant identity from the client-facing create
DTO by deleting the applicantId field from CreateVisaDTO and updating any usages
(e.g., constructors, builders, validation code) that accept or map applicantId;
ensure server-side code that processes visa creation (controller/servlet method
handling the create request and service method that accepts CreateVisaDTO)
instead resolves the applicant identity from the authenticated principal/session
and sets it onto the domain model before persisting.
In
`@src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java`:
- Around line 269-291: requestMoreInformation currently accepts null or blank
infoText which can create INCOMPLETE visas with no feedback; add a validation at
the start of requestMoreInformation to ensure infoText is non-null and not blank
(trim it and check length), and if invalid throw the same type of
validation/BadRequest exception your service uses elsewhere (mirror the check
used in rejectVisa), then proceed to call validateHandler(adminId),
findVisaById(visaId), set the handler/status/info, save via visaRepository.save,
and create the audit log as before; reference the method requestMoreInformation
and reuse the same validation style/exception as rejectVisa for consistency.
In `@src/test/java/org/example/visacasemanagementsystem/visa/VisaMapperTest.java`:
- Around line 62-71: The test VisaMapperTest currently checks fields from
visaMapper.toDTO(visa) but misses asserting the handlerName fallback; update the
test to assert that dto.handlerName() equals "Unassigned" when the source Visa
has no handler set, ensuring the mapper's fallback behavior is explicitly
verified (locate the assertion block after the existing assertions on dto.id(),
dto.visaType(), etc., and add the dto.handlerName() check).
In
`@src/test/java/org/example/visacasemanagementsystem/visa/VisaServiceTest.java`:
- Around line 368-381: The test method
validateHandler_shouldThrowEntityNotFoundException_WhenUserIsApplicant has a
misleading name; rename it to reflect that it covers the non-existent user case
(for example
validateHandler_shouldThrowEntityNotFoundException_WhenUserNotFound) and update
any references to the method name in the test class; keep the test body (mocking
userRepository.findById(nonExistingUserId) to return Optional.empty(), asserting
EntityNotFoundException with message "User not found.", and
verifyNoInteractions(visaRepository)) unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f4941141-b20f-48b1-b30b-a138963ca573
📒 Files selected for processing (16)
src/main/java/org/example/visacasemanagementsystem/exception/GlobalExceptionHandler.javasrc/main/java/org/example/visacasemanagementsystem/visa/VisaStatus.javasrc/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.javasrc/main/java/org/example/visacasemanagementsystem/visa/dto/CreateVisaDTO.javasrc/main/java/org/example/visacasemanagementsystem/visa/dto/UpdateVisaDTO.javasrc/main/java/org/example/visacasemanagementsystem/visa/dto/VisaDTO.javasrc/main/java/org/example/visacasemanagementsystem/visa/entity/Visa.javasrc/main/java/org/example/visacasemanagementsystem/visa/mapper/VisaMapper.javasrc/main/java/org/example/visacasemanagementsystem/visa/repository/VisaRepository.javasrc/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.javasrc/main/resources/templates/visa/apply-form.htmlsrc/main/resources/templates/visa/dashboard.htmlsrc/main/resources/templates/visa/details.htmlsrc/main/resources/templates/visa/edit-form.htmlsrc/test/java/org/example/visacasemanagementsystem/visa/VisaMapperTest.javasrc/test/java/org/example/visacasemanagementsystem/visa/VisaServiceTest.java
💤 Files with no reviewable changes (1)
- src/main/java/org/example/visacasemanagementsystem/visa/VisaStatus.java
Comment on lines
+125
to
+126
| <a th:href="@{/visas/logout}" class="btn-new "> Sign Out | ||
| </a> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for logout endpoint configuration
rg -n "logout" --type=java --type=html -C2Repository: ithsjava25/visa-case-management-system
Length of output: 498
🏁 Script executed:
# Find the VisaViewController class
find . -name "*VisaViewController*" -o -name "*Controller*" | grep -i visaRepository: ithsjava25/visa-case-management-system
Length of output: 353
🏁 Script executed:
# Look for Spring Security configuration
find . -name "*Security*" -o -name "*Config*" | grep -v target | head -20Repository: ithsjava25/visa-case-management-system
Length of output: 167
🏁 Script executed:
# Check for any `@GetMapping`, `@PostMapping`, or `@RequestMapping` related to logout or visas
rg -n "@(Get|Post|Request)Mapping.*logout|logout.*@" --type=javaRepository: ithsjava25/visa-case-management-system
Length of output: 64
🏁 Script executed:
# Examine the VisaViewController more thoroughly
find . -type f -name "*.java" | xargs grep -l "VisaViewController" | head -5Repository: ithsjava25/visa-case-management-system
Length of output: 174
🏁 Script executed:
# Read the VisaViewController to see all endpoints
cat -n ./src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.javaRepository: ithsjava25/visa-case-management-system
Length of output: 9812
🏁 Script executed:
# Search for Spring Security configuration more broadly
rg -l "Security\|WebSecurityConfigurerAdapter\|SecurityFilterChain\|@EnableWebSecurity" --type=javaRepository: ithsjava25/visa-case-management-system
Length of output: 64
🏁 Script executed:
# Check for logout handling in application.properties or yml
find . -name "application*.properties" -o -name "application*.yml" | xargs cat 2>/dev/nullRepository: ithsjava25/visa-case-management-system
Length of output: 586
🏁 Script executed:
# Search for any Spring Security configuration class
rg -n "class.*Config|@Configuration|SecurityFilterChain|WebSecurityConfigurerAdapter" --type=java | head -20Repository: ithsjava25/visa-case-management-system
Length of output: 204
🏁 Script executed:
# Check for Spring Security dependencies
find . -name "pom.xml" -o -name "build.gradle" | xargs cat 2>/dev/null | grep -i securityRepository: ithsjava25/visa-case-management-system
Length of output: 453
🏁 Script executed:
# Search for any bean configuration related to logout
rg -n "logout\|Logout" --type=javaRepository: ithsjava25/visa-case-management-system
Length of output: 64
🏁 Script executed:
# Check for any XML configuration files
find . -name "*.xml" | grep -v target | xargs grep -l "logout\|security" 2>/dev/nullRepository: ithsjava25/visa-case-management-system
Length of output: 91
🏁 Script executed:
# Check what files are in the visa controller package
find ./src/main/java/org/example/visacasemanagementsystem/visa -type f -name "*.java"Repository: ithsjava25/visa-case-management-system
Length of output: 885
🏁 Script executed:
# Search for any other potential logout handlers or controllers
rg -n "logout|Logout" --type=java -iRepository: ithsjava25/visa-case-management-system
Length of output: 64
🏁 Script executed:
# Check pom.xml for Spring Security configuration details
cat -n ./pom.xml | grep -A5 -B5 "spring-boot-starter-security"Repository: ithsjava25/visa-case-management-system
Length of output: 1685
The /visas/logout endpoint is not implemented.
The "Sign Out" button links to /visas/logout, but no such endpoint exists in VisaViewController or Spring Security configuration. Implement a logout handler in VisaViewController or update the template to use Spring Security's default /logout endpoint.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/templates/visa/dashboard.html` around lines 125 - 126, The
template links the Sign Out button to /visas/logout but no handler exists;
either add a logout endpoint in VisaViewController (e.g., create a method like
logout or handleLogout with a mapping for "/visas/logout" that delegates to
SecurityContextLogoutHandler or redirects to the security logout URL) or modify
the template anchor to use Spring Security's default logout endpoint by changing
th:href to "@{/logout}"; update the controller or template consistently and
ensure any CSRF token or POST logout requirements are handled per your security
configuration.
Contributor
Author
There was a problem hiding this comment.
This will be solved later on when we implement Spring Security to the project
There was a problem hiding this comment.
@eeebbaandersson, understood! I'll note this for future reviews.
✏️ Learnings added
Learnt from: eeebbaandersson
Repo: ithsjava25/visa-case-management-system PR: 27
File: src/main/resources/templates/visa/dashboard.html:125-126
Timestamp: 2026-04-14T12:26:05.436Z
Learning: In the `visa-case-management-system` repository, the "Sign Out" link in `src/main/resources/templates/visa/dashboard.html` (line ~126) points to `/visas/logout`, which is not yet implemented. This is intentionally deferred until Spring Security is fully configured. When implemented, the link should either use Spring Security's default `/logout` endpoint (with CSRF token handling via a POST form) or a dedicated `VisaViewController` method that delegates to `SecurityContextLogoutHandler`.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: eeebbaandersson
Repo: ithsjava25/visa-case-management-system PR: 16
File: src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java:42-43
Timestamp: 2026-04-09T11:13:00.080Z
Learning: In the `visa-case-management-system` repository, the `authorId` field in `CreateCommentDTO` is currently trusted directly in `CommentService.createComment()` (file: `src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java`, around line 42). This is a known security gap intentionally deferred until Spring Security is integrated. Once Spring Security is added, the author should be resolved from the authenticated principal (e.g., via `SecurityContextHolder`) rather than from `dto.authorId()`, and the `authorId` field should be removed from `CreateCommentDTO` or restricted to admin use only.
Learnt from: Martin-E-Karlsson
Repo: ithsjava25/visa-case-management-system PR: 26
File: src/main/java/org/example/visacasemanagementsystem/user/dto/CreateUserDTO.java:11-11
Timestamp: 2026-04-13T17:12:15.296Z
Learning: In the `visa-case-management-system` repository, Spring Security has been integrated (via `SecurityUser` and `UserDetailsServiceImpl` in PR `#26`), but password hashing in `UserService.createUser()` (file: `src/main/java/org/example/visacasemanagementsystem/user/service/UserService.java`, line ~54) still uses plaintext (`user.setPassword(dto.password())`). BCrypt encoding is intentionally deferred to a future pull request. When implemented, a `BCryptPasswordEncoder` `Bean` should be configured and injected into `UserService` via constructor injection, replacing the plaintext assignment with `passwordEncoder.encode(dto.password())`.
Comment on lines
+196
to
+218
| <div class="decision-form" style="margin-top: 20px; border-top: 1px solid #eee; padding-top: 20px;"> | ||
| <input type="text" id="infoReason" | ||
| placeholder="Write reason for rejection or what info is missing..." | ||
| style="width: 100%; padding: 10px; margin-bottom: 10px; border: 1px solid #ddd; border-radius: 4px;" required> | ||
|
|
||
| <div style="display: flex; gap: 10px;"> | ||
| <form th:action="@{/visas/{id}/request-info(id=${visa.id}, currentUserId=${currentUser.id})}" method="post" id="form-info" style="flex: 1;"> | ||
| <input type="hidden" name="reason" id="hidden-info"> | ||
| <button type="submit" onclick="document.getElementById('hidden-info').value = document.getElementById('infoReason').value" | ||
| style="width: 100%"> | ||
| Request More Info | ||
| </button> | ||
| </form> | ||
|
|
||
| <form th:action="@{/visas/{id}/reject(id=${visa.id}, currentUserId=${currentUser.id})}" method="post" id="form-reject" style="flex: 1;"> | ||
| <input type="hidden" name="reason" id="hidden-reject"> | ||
| <button type="submit" onclick="document.getElementById('hidden-reject').value = document.getElementById('infoReason').value" | ||
| style="width: 100%;"> | ||
| Reject Visa | ||
| </button> | ||
| </form> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Empty reason can be submitted for "Request More Info" action.
The shared #infoReason input has required attribute, but since it's outside the forms, HTML5 validation won't trigger. While rejectVisa validates blank reasons server-side, requestMoreInformation in VisaService (lines 270-291) does not validate empty infoText, allowing empty feedback to be saved.
🛡️ Suggested server-side validation in VisaService
`@Transactional`
public VisaDTO requestMoreInformation(Long visaId, Long adminId, String infoText) {
+ if (infoText == null || infoText.isBlank()) {
+ throw new IllegalArgumentException("Information request text cannot be null or blank");
+ }
User admin = validateHandler(adminId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/templates/visa/details.html` around lines 196 - 218, The
request-more-info flow allows an empty reason because the shared `#infoReason`
input sits outside the form so client-side required validation is bypassed and
VisaService.requestMoreInformation does not validate blank infoText; add
server-side validation in VisaService.requestMoreInformation to reject empty or
whitespace-only infoText (same pattern used by the rejectVisa path),
returning/throwing a validation error so the controller can respond
appropriately, and optionally ensure the form wiring uses `#form-info` and
hidden-info consistently to avoid client-side bypass.
Comment on lines
+237
to
+244
| <div class="comment-form"> | ||
| <form th:action="@{/api/comments}" method="post"> | ||
| <input type="hidden" name="visaId" th:value="${visa.id}"> | ||
| <input type="hidden" name="authorId" th:value="${currentUser.id}"> | ||
| <textarea name="content" placeholder="Write a comment..."></textarea> | ||
| <button type="submit" class="btn-submit">Send</button> | ||
| </form> | ||
| </div> |
There was a problem hiding this comment.
Comment form will fail due to content-type and field name mismatch.
Two issues prevent this form from working:
-
Content-type mismatch: The
CommentControlleris a@RestControllerexpecting@RequestBodyJSON (seesrc/main/java/org/example/visacasemanagementsystem/comment/controller/CommentController.java:13-26), but this form submitsapplication/x-www-form-urlencoded. -
Field name mismatch: The form sends
content(line 241) butCreateCommentDTOexpectstextfield (seesrc/main/java/org/example/visacasemanagementsystem/comment/dto/CreateCommentDTO.java:4).
🐛 Recommended fix options
Option 1: Add a view controller endpoint that accepts form data:
// In VisaViewController
`@PostMapping`("/{id}/comments")
public String addComment(`@PathVariable` Long id,
`@RequestParam` Long authorId,
`@RequestParam` String text,
`@RequestParam` Long currentUserId) {
commentService.createComment(new CreateCommentDTO(id, authorId, text));
return "redirect:/visas/" + id + "?currentUserId=" + currentUserId;
}Then update the form:
-<form th:action="@{/api/comments}" method="post">
- <input type="hidden" name="visaId" th:value="${visa.id}">
- <input type="hidden" name="authorId" th:value="${currentUser.id}">
- <textarea name="content" placeholder="Write a comment..."></textarea>
+<form th:action="@{/visas/{id}/comments(id=${visa.id}, currentUserId=${currentUser.id})}" method="post">
+ <input type="hidden" name="authorId" th:value="${currentUser.id}">
+ <textarea name="text" placeholder="Write a comment..."></textarea>Option 2: Use JavaScript/fetch to submit as JSON to the existing REST endpoint.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/templates/visa/details.html` around lines 237 - 244, The
form won't work because CommentController expects JSON via `@RequestBody` and
CreateCommentDTO uses field name text; fix by adding a view handler in
VisaViewController (e.g., addComment with `@PostMapping`("/{id}/comments") that
accepts `@RequestParam` Long visaId, `@RequestParam` Long authorId, `@RequestParam`
String text and calls commentService.createComment(new CreateCommentDTO(visaId,
authorId, text)) then redirects back to the visa details page), and update the
template form to POST to that view endpoint and rename the textarea from
name="content" to name="text" so the field names match CreateCommentDTO.
Contributor
Author
There was a problem hiding this comment.
This issue will be solved later by using JavaScript/fetch to display comments in visa form
There was a problem hiding this comment.
@eeebbaandersson, understood! Using JavaScript/fetch to submit comments as JSON to the existing /api/comments REST endpoint is a clean approach that avoids the need for an additional view controller endpoint. When you implement it, remember to:
- Use
fetch('/api/comments', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ visaId, authorId, text }) })— note the field name should betext(matchingCreateCommentDTO), notcontent. - Handle the response to dynamically append the new comment to the comments list without a full page reload.
Feel free to revisit this comment when the implementation is in place.
✏️ Learnings added
Learnt from: eeebbaandersson
Repo: ithsjava25/visa-case-management-system PR: 27
File: src/main/resources/templates/visa/details.html:237-244
Timestamp: 2026-04-14T12:40:25.740Z
Learning: In `visa-case-management-system`, the comment form in `src/main/resources/templates/visa/details.html` (around lines 237-244) currently submits to `/api/comments` with form fields `visaId`, `authorId`, and `content` (wrong field name). The intended fix is to replace the HTML form with JavaScript/fetch to POST JSON to the existing REST endpoint `/api/comments`. The correct JSON body field for the comment text is `text` (matching `CreateCommentDTO`), not `content`. This is a known deferred issue to be addressed in a future PR.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: eeebbaandersson
Repo: ithsjava25/visa-case-management-system PR: 27
File: src/main/resources/templates/visa/dashboard.html:125-126
Timestamp: 2026-04-14T12:26:05.436Z
Learning: In the `visa-case-management-system` templates, ensure the “Sign Out” link/button uses the actual Spring Security logout mechanism once configured. Don’t leave it pointing to an unimplemented `/visas/logout` endpoint. When logout is ready, either (a) use Spring Security’s standard `/logout` endpoint via a POST form (including CSRF token handling), or (b) route to a dedicated controller method (e.g., a `VisaViewController` handler) that delegates to `SecurityContextLogoutHandler`.
Learnt from: eeebbaandersson
Repo: ithsjava25/visa-case-management-system PR: 16
File: src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java:42-43
Timestamp: 2026-04-09T11:13:00.080Z
Learning: In the `visa-case-management-system` repository, the `authorId` field in `CreateCommentDTO` is currently trusted directly in `CommentService.createComment()` (file: `src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java`, around line 42). This is a known security gap intentionally deferred until Spring Security is integrated. Once Spring Security is added, the author should be resolved from the authenticated principal (e.g., via `SecurityContextHolder`) rather than from `dto.authorId()`, and the `authorId` field should be removed from `CreateCommentDTO` or restricted to admin use only.
This was
linked to
issues
Apr 14, 2026
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/main/java/org/example/visacasemanagementsystem/visa/entity/Visa.java (1)
96-96:⚠️ Potential issue | 🟠 MajorOmit raw
statusInformationfromtoString().
statusInformationnow carries free-text rejection reasons / info requests, so printing it here leaks case notes into logs. Keep only a presence flag, length, or omit it entirely.🔐 Suggested change
- ", statusInformation='" + statusInformation + '\'' + + ", hasStatusInformation=" + (statusInformation != null && !statusInformation.isBlank()) +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/visacasemanagementsystem/visa/entity/Visa.java` at line 96, The Visa.toString method currently appends the raw statusInformation field which may leak sensitive free-text case notes; update the Visa.toString implementation (the toString method on class Visa) to stop including the raw statusInformation value and instead include a safe indicator such as a boolean presence flag (e.g., statusInformation != null && !statusInformation.isEmpty()) or the character length (e.g., statusInformationLength) or omit it entirely; locate the concatenation that references statusInformation and replace it with the chosen safe representation and update any related string labels accordingly.src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java (1)
146-152:⚠️ Potential issue | 🟡 MinorRe-add
statusInformationin the service-validation error branch.The
IllegalArgumentExceptionpath re-rendersvisa/edit-formwithout repopulatingstatusInformation, so the handler feedback banner disappears specifically for travel-date errors.🧩 Suggested change
} catch (IllegalArgumentException e) { bindingResult.rejectValue("travelDate", "error.travelDate", e.getMessage()); prepareApplyModel(currentUserId, model); model.addAttribute("isEdit", true); + VisaDTO visa = visaService.findVisaDtoById(id); + model.addAttribute("statusInformation", visa.statusInformation()); return "visa/edit-form"; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java` around lines 146 - 152, When catching IllegalArgumentException in VisaViewController.updateVisa handling, the code repopulates the model via prepareApplyModel(...) but fails to re-add the statusInformation attribute so the feedback banner disappears; modify the IllegalArgumentException catch block (the branch that calls bindingResult.rejectValue("travelDate", ...), prepareApplyModel(currentUserId, model), model.addAttribute("isEdit", true), return "visa/edit-form") to also fetch and add the same statusInformation value used elsewhere (the attribute prepared for the edit form, e.g., by calling the same helper or retrieving from visaService) to the model before returning so the feedback banner is preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java`:
- Around line 38-41: Stop trusting request-supplied currentUserId: remove the
`@RequestParam` Long currentUserId from all affected controller handler methods
(showDashboard, and the methods referenced around lines 62–64, 77–80, 101–103,
131–135, 157–166, 174–185, 189–196, 216–218) and instead resolve the
authenticated user via Spring Security
(SecurityContextHolder.getContext().getAuthentication() or an injected
Principal) to obtain the current user's id/username, then use userService (e.g.,
userService.findById or findByUsername) to load UserDTO and pass that into
visaService.validateHandler(...) and other authorization/ownership checks; also
update method signatures to remove currentUserId from request surfaces.
Additionally, in the catch block that handles IllegalArgumentException from
visaService.updateVisa(), ensure you add model.addAttribute("statusInformation",
...) so the edit form has the missing dropdown (use the same source used
elsewhere for statusInformation or call the method that prepares it before
returning prepareApplyModel(...) with isEdit=true).
In
`@src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java`:
- Around line 188-191: Change updateVisa to accept the path id and use it as the
authoritative target instead of trusting dto.id(): modify VisaService.updateVisa
signature to updateVisa(Long id, UpdateVisaDTO dto, Long userId) and load the
entity with visaRepository.findById(id).orElseThrow(...), then apply fields from
UpdateVisaDTO to the loaded Visa and save; also update
VisaViewController.processUpdate to call visaService.updateVisa(id,
updateVisaDTO, currentUserId) (and remove/ignore dto.id() usage) so the URL path
id is the source of truth.
---
Duplicate comments:
In
`@src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java`:
- Around line 146-152: When catching IllegalArgumentException in
VisaViewController.updateVisa handling, the code repopulates the model via
prepareApplyModel(...) but fails to re-add the statusInformation attribute so
the feedback banner disappears; modify the IllegalArgumentException catch block
(the branch that calls bindingResult.rejectValue("travelDate", ...),
prepareApplyModel(currentUserId, model), model.addAttribute("isEdit", true),
return "visa/edit-form") to also fetch and add the same statusInformation value
used elsewhere (the attribute prepared for the edit form, e.g., by calling the
same helper or retrieving from visaService) to the model before returning so the
feedback banner is preserved.
In `@src/main/java/org/example/visacasemanagementsystem/visa/entity/Visa.java`:
- Line 96: The Visa.toString method currently appends the raw statusInformation
field which may leak sensitive free-text case notes; update the Visa.toString
implementation (the toString method on class Visa) to stop including the raw
statusInformation value and instead include a safe indicator such as a boolean
presence flag (e.g., statusInformation != null && !statusInformation.isEmpty())
or the character length (e.g., statusInformationLength) or omit it entirely;
locate the concatenation that references statusInformation and replace it with
the chosen safe representation and update any related string labels accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fa1928e7-9cde-4f87-b0c5-59ddd8a6df3b
📒 Files selected for processing (6)
src/main/java/org/example/visacasemanagementsystem/exception/GlobalExceptionHandler.javasrc/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.javasrc/main/java/org/example/visacasemanagementsystem/visa/entity/Visa.javasrc/main/java/org/example/visacasemanagementsystem/visa/repository/VisaRepository.javasrc/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.javasrc/main/resources/templates/visa/dashboard.html
✅ Files skipped from review due to trivial changes (2)
- src/main/java/org/example/visacasemanagementsystem/visa/repository/VisaRepository.java
- src/main/resources/templates/visa/dashboard.html
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/org/example/visacasemanagementsystem/exception/GlobalExceptionHandler.java
Comment on lines
+38
to
+41
| public String showDashboard(@RequestParam Long currentUserId, Model model) { | ||
| // Find current user | ||
| UserDTO user = userService.findById(currentUserId) | ||
| .orElseThrow(() -> new EntityNotFoundException("User not found.")); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Spring Security artifacts:\n'
fd -i 'SecurityUser|UserDetailsServiceImpl|SecurityConfig' src/main/java
printf '\nRequest-supplied identity in VisaViewController:\n'
rg -n --type=java '@RequestParam\s+Long\s+currentUserId|findById\(currentUserId\)|findVisasByApplicant\(currentUserId\)|approveVisa\(id,\s*currentUserId\)|requestMoreInformation\(id,\s*currentUserId\)|rejectVisa\(id,\s*currentUserId\)|assignHandler\(id,\s*currentUserId\)' src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java
printf '\nAuthenticated-principal lookups in the web layer:\n'
rg -n --type=java '@AuthenticationPrincipal|SecurityContextHolder|getAuthentication\(' src/main/javaRepository: ithsjava25/visa-case-management-system
Length of output: 1532
🏁 Script executed:
# Check for build configuration and Spring Security dependency
fd -i 'pom\.xml|build\.gradle' src/ . | head -20
# Search for authentication/security-related code that might indicate integration
rg -n 'PasswordEncoder|BCrypt|UserDetailsService|AuthenticationManager|WebSecurityConfigurerAdapter' src/main/java --type=java
# Look for any Spring Security imports
rg -n 'import.*springframework\.security' src/main/java --type=java
# Check if user service has security-related methods
rg -n 'class.*UserService|implements.*UserDetailsService' src/main/java --type=java -A 5Repository: ithsjava25/visa-case-management-system
Length of output: 781
🏁 Script executed:
# Check pom.xml for Spring Security dependency
cat pom.xml | grep -A 2 -B 2 'security\|spring-security' || echo "No Spring Security found in pom.xml"
# Also verify the actual state of authentication in the application
rg -n 'public class.*Controller' src/main/java/org/example/visacasemanagementsystem/visa/controller/ --type=java -A 3 | head -40Repository: ithsjava25/visa-case-management-system
Length of output: 1539
🏁 Script executed:
# View the specific lines mentioned in the review to confirm the vulnerability
sed -n '38,41p' src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java
# Check if there's any authentication context usage in this controller
rg -n 'SecurityContextHolder|@AuthenticationPrincipal|getAuthentication|getPrincipal' src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.javaRepository: ithsjava25/visa-case-management-system
Length of output: 334
🏁 Script executed:
# Check the processUpdate method and the missing statusInformation regression (around lines 146-152)
sed -n '140,160p' src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java
# Also check if there's a similar pattern in other methods to see the full scope of the issue
rg -n 'model\.addAttribute.*statusInformation|BindingResult|IllegalArgumentException' src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.javaRepository: ithsjava25/visa-case-management-system
Length of output: 1422
Stop trusting currentUserId from the request.
Every ownership/admin check in this controller is derived from a query/form parameter. A caller can impersonate another applicant—or an admin—just by supplying an admin id in the request, and VisaService.validateHandler(Long) will authorize the looked-up user. Spring Security is declared as a dependency but not integrated here. Resolve the current user from Spring Security instead and remove currentUserId from the request surface.
Also add statusInformation to the model in the error catch block (lines 148–152). When visaService.updateVisa() throws IllegalArgumentException, the error handler calls prepareApplyModel() and sets isEdit to true but omits statusInformation, causing the edit form template to render with a missing dropdown.
Applies to all handler methods: 38–41, 62–64, 77–80, 101–103, 131–135, 157–166, 174–185, 189–196, 216–218.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java`
around lines 38 - 41, Stop trusting request-supplied currentUserId: remove the
`@RequestParam` Long currentUserId from all affected controller handler methods
(showDashboard, and the methods referenced around lines 62–64, 77–80, 101–103,
131–135, 157–166, 174–185, 189–196, 216–218) and instead resolve the
authenticated user via Spring Security
(SecurityContextHolder.getContext().getAuthentication() or an injected
Principal) to obtain the current user's id/username, then use userService (e.g.,
userService.findById or findByUsername) to load UserDTO and pass that into
visaService.validateHandler(...) and other authorization/ownership checks; also
update method signatures to remove currentUserId from request surfaces.
Additionally, in the catch block that handles IllegalArgumentException from
visaService.updateVisa(), ensure you add model.addAttribute("statusInformation",
...) so the edit form has the missing dropdown (use the same source used
elsewhere for statusInformation or call the method that prepares it before
returning prepareApplyModel(...) with isEdit=true).
Contributor
Author
There was a problem hiding this comment.
This will later be resolved when we implement the missing Spring Security
There was a problem hiding this comment.
@eeebbaandersson, understood — noted for future reviews. Just to flag one distinction: the missing statusInformation attribute in the IllegalArgumentException catch block of processUpdate() (lines 148–152) is independent of Spring Security — it's a pure model-population bug that would cause the edit form's status information banner to render empty regardless of how the user is authenticated. Worth addressing in isolation even before the Spring Security work lands, but I'll leave that to your discretion.
✏️ Learnings added
Learnt from: eeebbaandersson
Repo: ithsjava25/visa-case-management-system PR: 27
File: src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java:38-41
Timestamp: 2026-04-14T13:13:26.963Z
Learning: In `visa-case-management-system`, `VisaViewController` (`src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java`) currently accepts `currentUserId` as a `RequestParam` in all handler methods (showDashboard, showApplyForm, submitApplication, showEditForm, processUpdate, approveVisa, requestMoreInformation, rejectVisa, assignCaseToHandler, viewDetails) instead of resolving the authenticated user from the Spring Security context. This is intentionally deferred until Spring Security is fully wired into the web layer. When implemented, `RequestParam Long currentUserId` should be removed from all these methods and replaced with resolution via `SecurityContextHolder.getContext().getAuthentication()` or `AuthenticationPrincipal`, followed by a `userService` lookup to obtain the `UserDTO`.
Learnt from: eeebbaandersson
Repo: ithsjava25/visa-case-management-system PR: 27
File: src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java:38-41
Timestamp: 2026-04-14T13:13:26.963Z
Learning: In `visa-case-management-system`, the `processUpdate()` method in `VisaViewController` (`src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java`, around lines 148–152) is missing `model.addAttribute("statusInformation", ...)` in the `IllegalArgumentException` catch block. When `visaService.updateVisa()` throws this exception, the edit form (`edit-form.html`) re-renders without `statusInformation`, causing the "Feedback from Handler" banner to display incorrectly. This is a UI/model bug independent of Spring Security and should be fixed by fetching the current visa's `statusInformation` (e.g., via `visaService.findVisaDtoById(id).statusInformation()`) and adding it to the model before returning the edit view.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: eeebbaandersson
Repo: ithsjava25/visa-case-management-system PR: 27
File: src/main/resources/templates/visa/dashboard.html:125-126
Timestamp: 2026-04-14T12:26:05.436Z
Learning: In the `visa-case-management-system` repository, the "Sign Out" link in `src/main/resources/templates/visa/dashboard.html` (line ~126) points to `/visas/logout`, which is not yet implemented. This is intentionally deferred until Spring Security is fully configured. When implemented, the link should either use Spring Security's default `/logout` endpoint (with CSRF token handling via a POST form) or a dedicated `VisaViewController` method that delegates to `SecurityContextLogoutHandler`.
Learnt from: eeebbaandersson
Repo: ithsjava25/visa-case-management-system PR: 16
File: src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java:42-43
Timestamp: 2026-04-09T11:13:00.080Z
Learning: In the `visa-case-management-system` repository, the `authorId` field in `CreateCommentDTO` is currently trusted directly in `CommentService.createComment()` (file: `src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java`, around line 42). This is a known security gap intentionally deferred until Spring Security is integrated. Once Spring Security is added, the author should be resolved from the authenticated principal (e.g., via `SecurityContextHolder`) rather than from `dto.authorId()`, and the `authorId` field should be removed from `CreateCommentDTO` or restricted to admin use only.
Learnt from: eeebbaandersson
Repo: ithsjava25/visa-case-management-system PR: 27
File: src/main/resources/templates/visa/details.html:237-244
Timestamp: 2026-04-14T12:40:25.740Z
Learning: In `visa-case-management-system`, the comment form in `src/main/resources/templates/visa/details.html` (around lines 237-244) currently submits to `/api/comments` with form fields `visaId`, `authorId`, and `content` (wrong field name). The intended fix is to replace the HTML form with JavaScript/fetch to POST JSON to the existing REST endpoint `/api/comments`. The correct JSON body field for the comment text is `text` (matching `CreateCommentDTO`), not `content`. This is a known deferred issue to be addressed in a future PR.
Learnt from: Martin-E-Karlsson
Repo: ithsjava25/visa-case-management-system PR: 26
File: src/main/java/org/example/visacasemanagementsystem/user/dto/CreateUserDTO.java:11-11
Timestamp: 2026-04-13T17:12:15.296Z
Learning: In the `visa-case-management-system` repository, Spring Security has been integrated (via `SecurityUser` and `UserDetailsServiceImpl` in PR `#26`), but password hashing in `UserService.createUser()` (file: `src/main/java/org/example/visacasemanagementsystem/user/service/UserService.java`, line ~54) still uses plaintext (`user.setPassword(dto.password())`). BCrypt encoding is intentionally deferred to a future pull request. When implemented, a `BCryptPasswordEncoder` `Bean` should be configured and injected into `UserService` via constructor injection, replacing the plaintext assignment with `passwordEncoder.encode(dto.password())`.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java (1)
146-152:⚠️ Potential issue | 🟡 MinorRe-add
statusInformationin the exception path too.The validation-error branch restores the handler feedback banner, but the
IllegalArgumentExceptionbranch still returnsvisa/edit-formwithout repopulatingstatusInformation, so that banner disappears on service-level validation failures.🔧 Suggested fix
} catch (IllegalArgumentException e) { bindingResult.rejectValue("travelDate", "error.travelDate", e.getMessage()); prepareApplyModel(currentUserId, model); model.addAttribute("isEdit", true); + VisaDTO visa = visaService.findVisaDtoById(id); + model.addAttribute("statusInformation", visa.statusInformation()); return "visa/edit-form"; }Based on learnings, the
processUpdate()method inVisaViewControlleris missingmodel.addAttribute("statusInformation", ...)in theIllegalArgumentExceptioncatch block.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java` around lines 146 - 152, The IllegalArgumentException catch block in processUpdate() calls bindingResult.rejectValue(...), prepareApplyModel(...), sets isEdit and returns "visa/edit-form" but forgets to repopulate the statusInformation model attribute, causing the handler banner to disappear; update the IllegalArgumentException catch block in VisaViewController.processUpdate() (the block that catches IllegalArgumentException from visaService.updateVisa(...)) to add model.addAttribute("statusInformation", <same value used elsewhere, e.g., the one set in prepareApplyModel or where statusInformation is built>) before returning so the view receives the statusInformation attribute on service-level validation failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/org/example/visacasemanagementsystem/config/SecurityConfig.java`:
- Around line 13-18: The security config currently permits all requests and
disables CSRF/frame protections (anyRequest().permitAll(), csrf.disable(),
headers.frameOptions(...).disable()), exposing mutation endpoints to
unauthenticated parameter substitution; update the HttpSecurity DSL to require
authentication for application routes (replace anyRequest().permitAll() with
explicit matchers so that endpoints like "/api/**" require authenticated() while
allowing "/h2-console/**" and static resources to permitAll()), re-enable CSRF
but ignore the H2 console only (use csrf().ignoringRequestMatchers(new
AntPathRequestMatcher("/h2-console/**")) or
ignoringAntMatchers("/h2-console/**")), and set frameOptions to sameOrigin()
instead of disable() (headers().frameOptions().sameOrigin()) so the H2 console
still works without opening frames globally.
In
`@src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java`:
- Around line 197-209: The updateVisa() flow currently resets visa status to
SUBMITTED unconditionally; change it to prevent applicant edits from reopening
finalized cases by checking visa.getVisaStatus() (use the VisaStatus enum)
before modifying status: if visa.getVisaStatus() is GRANTED, REJECTED, or
ASSIGNED then either reject the edit (throw
UnauthorizedException/IllegalStateException) or at minimum avoid changing
visa.setVisaStatus(...) and visa.setStatusInformation(null); keep the ownership
check (visa.getApplicant().getId().equals(userId)),
validateTravelDate(dto.travelDate()), and still update editable fields
(visaType, nationality, passportNumber, travelDate) but do not override
finalized statuses.
In
`@src/main/java/org/example/visacasemanagementsystem/VisaCaseManagementSystemApplication.java`:
- Around line 24-39: The current seeding block checks only
userRepository.findByEmail("user@test.com") before creating both the regular
User and the Admin, so if the user exists but the admin (created as
admin.setEmail("user@test.com2")) is missing it will never be created; update
the logic to perform separate existence checks and inserts per email by calling
userRepository.findByEmail("user@test.com") before creating the User and
userRepository.findByEmail("user@test.com2") before creating the Admin (refer to
userRepository.findByEmail(...) and the User/ admin creation blocks) so each
account is seeded independently.
- Around line 20-52: The initData CommandLineRunner currently seeds predictable
accounts on every startup; guard it behind a non-production profile by making
the bean conditional so it only runs in dev/test environments (e.g., annotate
the initData bean or method with `@Profile`("!prod") or use
`@Profile`({"dev","test"}) / `@ConditionalOnProperty`) and/or check the active
Spring Environment inside initData before creating users; update references to
UserRepository and the initData method so seeding occurs only when the active
profile is not production and remove or externalize hard-coded credentials.
---
Duplicate comments:
In
`@src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java`:
- Around line 146-152: The IllegalArgumentException catch block in
processUpdate() calls bindingResult.rejectValue(...), prepareApplyModel(...),
sets isEdit and returns "visa/edit-form" but forgets to repopulate the
statusInformation model attribute, causing the handler banner to disappear;
update the IllegalArgumentException catch block in
VisaViewController.processUpdate() (the block that catches
IllegalArgumentException from visaService.updateVisa(...)) to add
model.addAttribute("statusInformation", <same value used elsewhere, e.g., the
one set in prepareApplyModel or where statusInformation is built>) before
returning so the view receives the statusInformation attribute on service-level
validation failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3f12bf5b-3e4b-4dbc-bc51-81ced2349b56
📒 Files selected for processing (6)
src/main/java/org/example/visacasemanagementsystem/VisaCaseManagementSystemApplication.javasrc/main/java/org/example/visacasemanagementsystem/config/SecurityConfig.javasrc/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.javasrc/main/java/org/example/visacasemanagementsystem/visa/repository/VisaRepository.javasrc/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.javasrc/test/java/org/example/visacasemanagementsystem/visa/VisaServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/org/example/visacasemanagementsystem/visa/repository/VisaRepository.java
Comment on lines
+13
to
+18
| http | ||
| .authorizeHttpRequests(auth -> auth | ||
| .anyRequest().permitAll() | ||
| ) | ||
| .csrf(csrf -> csrf.disable()) | ||
| .headers(headers -> headers.frameOptions(frame -> frame.disable())); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine SecurityConfig.java
fd -t f "SecurityConfig.java"Repository: ithsjava25/visa-case-management-system
Length of output: 159
🏁 Script executed:
# Read the SecurityConfig.java file to verify the exact code
fd -t f "SecurityConfig.java" -x cat -n {}Repository: ithsjava25/visa-case-management-system
Length of output: 984
🏁 Script executed:
# Examine VisaViewController to confirm currentUserId RequestParam usage
fd -t f "VisaViewController.java" -x head -100 {}Repository: ithsjava25/visa-case-management-system
Length of output: 4013
🏁 Script executed:
# Search for all handler methods in VisaViewController that use currentUserId
fd -t f "VisaViewController.java" -x rg -A 2 "@PostMapping|@GetMapping|@PutMapping" {}Repository: ithsjava25/visa-case-management-system
Length of output: 1686
This config exposes every visa mutation to unauthenticated request parameter substitution.
The combination of anyRequest().permitAll() with CSRF disabled and controller methods accepting currentUserId as a request parameter creates an immediate security vulnerability. An attacker can submit POST requests to approve, reject, assign, or apply for visas as any user ID without authentication. The frame options are also disabled globally rather than scoped to the H2 console.
Require authentication for application routes, limit CSRF exceptions to H2 only, and use sameOrigin for frame options:
🛡️ Safer baseline
http
.authorizeHttpRequests(auth -> auth
- .anyRequest().permitAll()
+ .requestMatchers("/h2-console/**", "/error").permitAll()
+ .anyRequest().authenticated()
)
- .csrf(csrf -> csrf.disable())
- .headers(headers -> headers.frameOptions(frame -> frame.disable()));
+ .csrf(csrf -> csrf.ignoringRequestMatchers("/h2-console/**"))
+ .headers(headers -> headers.frameOptions(frame -> frame.sameOrigin()));
return http.build();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/org/example/visacasemanagementsystem/config/SecurityConfig.java`
around lines 13 - 18, The security config currently permits all requests and
disables CSRF/frame protections (anyRequest().permitAll(), csrf.disable(),
headers.frameOptions(...).disable()), exposing mutation endpoints to
unauthenticated parameter substitution; update the HttpSecurity DSL to require
authentication for application routes (replace anyRequest().permitAll() with
explicit matchers so that endpoints like "/api/**" require authenticated() while
allowing "/h2-console/**" and static resources to permitAll()), re-enable CSRF
but ignore the H2 console only (use csrf().ignoringRequestMatchers(new
AntPathRequestMatcher("/h2-console/**")) or
ignoringAntMatchers("/h2-console/**")), and set frameOptions to sameOrigin()
instead of disable() (headers().frameOptions().sameOrigin()) so the H2 console
still works without opening frames globally.
Comment on lines
+20
to
+52
| @Bean | ||
| public CommandLineRunner initData(UserRepository userRepository) { | ||
| return args -> { | ||
| // Skapa en vanlig användare (Applicant) | ||
| if (userRepository.findByEmail("user@test.com").isEmpty()) { | ||
| User user = new User(); | ||
| user.setFullName("USER"); | ||
| user.setEmail("user@test.com"); | ||
| user.setPassword("password"); // Notera: Du behöver sätta lösenord om din entitet kräver det | ||
| user.setUserAuthorization(UserAuthorization.USER); | ||
| userRepository.save(user); | ||
| System.out.println("Testanvändare skapad med ID: " + user.getId()); | ||
|
|
||
| User admin = new User(); | ||
| admin.setFullName("ADMIN"); | ||
| admin.setEmail("user@test.com2"); | ||
| admin.setPassword("password"); // Notera: Du behöver sätta lösenord om din entitet kräver det | ||
| admin.setUserAuthorization(UserAuthorization.ADMIN); | ||
| userRepository.save(admin); | ||
| System.out.println("Test-admin skapad med ID: " + admin.getId()); | ||
| } | ||
|
|
||
| // Skapa en admin (Handler) | ||
| if (userRepository.findByEmail("admin@test.com").isEmpty()) { | ||
| User sysadmin = new User(); | ||
| sysadmin.setFullName("SYSTEM ADMIN"); | ||
| sysadmin.setEmail("admin@test.com"); | ||
| sysadmin.setPassword("password"); | ||
| sysadmin.setUserAuthorization(UserAuthorization.SYSADMIN); | ||
| userRepository.save(sysadmin); | ||
| System.out.println("Test-sysadmin skapad med ID: " + sysadmin.getId()); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Guard this seed runner behind a non-production profile.
This bean creates known accounts with predictable credentials on every startup. In any shared or production-like environment, that becomes a built-in backdoor rather than test data.
🔒 Safer baseline
+import org.springframework.context.annotation.Profile;
...
- `@Bean`
+ `@Bean`
+ `@Profile`("dev")
public CommandLineRunner initData(UserRepository userRepository) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @Bean | |
| public CommandLineRunner initData(UserRepository userRepository) { | |
| return args -> { | |
| // Skapa en vanlig användare (Applicant) | |
| if (userRepository.findByEmail("user@test.com").isEmpty()) { | |
| User user = new User(); | |
| user.setFullName("USER"); | |
| user.setEmail("user@test.com"); | |
| user.setPassword("password"); // Notera: Du behöver sätta lösenord om din entitet kräver det | |
| user.setUserAuthorization(UserAuthorization.USER); | |
| userRepository.save(user); | |
| System.out.println("Testanvändare skapad med ID: " + user.getId()); | |
| User admin = new User(); | |
| admin.setFullName("ADMIN"); | |
| admin.setEmail("user@test.com2"); | |
| admin.setPassword("password"); // Notera: Du behöver sätta lösenord om din entitet kräver det | |
| admin.setUserAuthorization(UserAuthorization.ADMIN); | |
| userRepository.save(admin); | |
| System.out.println("Test-admin skapad med ID: " + admin.getId()); | |
| } | |
| // Skapa en admin (Handler) | |
| if (userRepository.findByEmail("admin@test.com").isEmpty()) { | |
| User sysadmin = new User(); | |
| sysadmin.setFullName("SYSTEM ADMIN"); | |
| sysadmin.setEmail("admin@test.com"); | |
| sysadmin.setPassword("password"); | |
| sysadmin.setUserAuthorization(UserAuthorization.SYSADMIN); | |
| userRepository.save(sysadmin); | |
| System.out.println("Test-sysadmin skapad med ID: " + sysadmin.getId()); | |
| } | |
| }; | |
| `@Bean` | |
| `@Profile`("dev") | |
| public CommandLineRunner initData(UserRepository userRepository) { | |
| return args -> { | |
| // Skapa en vanlig användare (Applicant) | |
| if (userRepository.findByEmail("user@test.com").isEmpty()) { | |
| User user = new User(); | |
| user.setFullName("USER"); | |
| user.setEmail("user@test.com"); | |
| user.setPassword("password"); // Notera: Du behöver sätta lösenord om din entitet kräver det | |
| user.setUserAuthorization(UserAuthorization.USER); | |
| userRepository.save(user); | |
| System.out.println("Testanvändare skapad med ID: " + user.getId()); | |
| User admin = new User(); | |
| admin.setFullName("ADMIN"); | |
| admin.setEmail("user@test.com2"); | |
| admin.setPassword("password"); // Notera: Du behöver sätta lösenord om din entitet kräver det | |
| admin.setUserAuthorization(UserAuthorization.ADMIN); | |
| userRepository.save(admin); | |
| System.out.println("Test-admin skapad med ID: " + admin.getId()); | |
| } | |
| // Skapa en admin (Handler) | |
| if (userRepository.findByEmail("admin@test.com").isEmpty()) { | |
| User sysadmin = new User(); | |
| sysadmin.setFullName("SYSTEM ADMIN"); | |
| sysadmin.setEmail("admin@test.com"); | |
| sysadmin.setPassword("password"); | |
| sysadmin.setUserAuthorization(UserAuthorization.SYSADMIN); | |
| userRepository.save(sysadmin); | |
| System.out.println("Test-sysadmin skapad med ID: " + sysadmin.getId()); | |
| } | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/org/example/visacasemanagementsystem/VisaCaseManagementSystemApplication.java`
around lines 20 - 52, The initData CommandLineRunner currently seeds predictable
accounts on every startup; guard it behind a non-production profile by making
the bean conditional so it only runs in dev/test environments (e.g., annotate
the initData bean or method with `@Profile`("!prod") or use
`@Profile`({"dev","test"}) / `@ConditionalOnProperty`) and/or check the active
Spring Environment inside initData before creating users; update references to
UserRepository and the initData method so seeding occurs only when the active
profile is not production and remove or externalize hard-coded credentials.
Comment on lines
+24
to
+39
| if (userRepository.findByEmail("user@test.com").isEmpty()) { | ||
| User user = new User(); | ||
| user.setFullName("USER"); | ||
| user.setEmail("user@test.com"); | ||
| user.setPassword("password"); // Notera: Du behöver sätta lösenord om din entitet kräver det | ||
| user.setUserAuthorization(UserAuthorization.USER); | ||
| userRepository.save(user); | ||
| System.out.println("Testanvändare skapad med ID: " + user.getId()); | ||
|
|
||
| User admin = new User(); | ||
| admin.setFullName("ADMIN"); | ||
| admin.setEmail("user@test.com2"); | ||
| admin.setPassword("password"); // Notera: Du behöver sätta lösenord om din entitet kräver det | ||
| admin.setUserAuthorization(UserAuthorization.ADMIN); | ||
| userRepository.save(admin); | ||
| System.out.println("Test-admin skapad med ID: " + admin.getId()); |
There was a problem hiding this comment.
Seed each account behind its own lookup.
Lines 24-39 gate both the applicant and admin inserts on findByEmail("user@test.com"), even though the admin is saved with a different email on Line 35. If the applicant exists but the admin row is missing, the admin will never be recreated.
🧩 Suggested fix
- if (userRepository.findByEmail("user@test.com").isEmpty()) {
+ if (userRepository.findByEmail("user@test.com").isEmpty()) {
User user = new User();
user.setFullName("USER");
user.setEmail("user@test.com");
user.setPassword("password");
user.setUserAuthorization(UserAuthorization.USER);
userRepository.save(user);
System.out.println("Testanvändare skapad med ID: " + user.getId());
+ }
+ if (userRepository.findByEmail("user@test.com2").isEmpty()) {
User admin = new User();
admin.setFullName("ADMIN");
admin.setEmail("user@test.com2");
admin.setPassword("password");
admin.setUserAuthorization(UserAuthorization.ADMIN);
userRepository.save(admin);
System.out.println("Test-admin skapad med ID: " + admin.getId());
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (userRepository.findByEmail("user@test.com").isEmpty()) { | |
| User user = new User(); | |
| user.setFullName("USER"); | |
| user.setEmail("user@test.com"); | |
| user.setPassword("password"); // Notera: Du behöver sätta lösenord om din entitet kräver det | |
| user.setUserAuthorization(UserAuthorization.USER); | |
| userRepository.save(user); | |
| System.out.println("Testanvändare skapad med ID: " + user.getId()); | |
| User admin = new User(); | |
| admin.setFullName("ADMIN"); | |
| admin.setEmail("user@test.com2"); | |
| admin.setPassword("password"); // Notera: Du behöver sätta lösenord om din entitet kräver det | |
| admin.setUserAuthorization(UserAuthorization.ADMIN); | |
| userRepository.save(admin); | |
| System.out.println("Test-admin skapad med ID: " + admin.getId()); | |
| if (userRepository.findByEmail("user@test.com").isEmpty()) { | |
| User user = new User(); | |
| user.setFullName("USER"); | |
| user.setEmail("user@test.com"); | |
| user.setPassword("password"); // Notera: Du behöver sätta lösenord om din entitet kräver det | |
| user.setUserAuthorization(UserAuthorization.USER); | |
| userRepository.save(user); | |
| System.out.println("Testanvändare skapad med ID: " + user.getId()); | |
| } | |
| if (userRepository.findByEmail("user@test.com2").isEmpty()) { | |
| User admin = new User(); | |
| admin.setFullName("ADMIN"); | |
| admin.setEmail("user@test.com2"); | |
| admin.setPassword("password"); // Notera: Du behöver sätta lösenord om din entitet kräver det | |
| admin.setUserAuthorization(UserAuthorization.ADMIN); | |
| userRepository.save(admin); | |
| System.out.println("Test-admin skapad med ID: " + admin.getId()); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/org/example/visacasemanagementsystem/VisaCaseManagementSystemApplication.java`
around lines 24 - 39, The current seeding block checks only
userRepository.findByEmail("user@test.com") before creating both the regular
User and the Admin, so if the user exists but the admin (created as
admin.setEmail("user@test.com2")) is missing it will never be created; update
the logic to perform separate existence checks and inserts per email by calling
userRepository.findByEmail("user@test.com") before creating the User and
userRepository.findByEmail("user@test.com2") before creating the Admin (refer to
userRepository.findByEmail(...) and the User/ admin creation blocks) so each
account is seeded independently.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java (2)
342-346: Error message is misleading when travel date is null.The same message "Travel date cannot be in the past." is shown whether the date is null or actually in the past. Consider a clearer message that covers both cases.
✏️ Suggested improvement
public void validateTravelDate(LocalDate travelDate) { - if (travelDate == null || travelDate.isBefore(LocalDate.now())) { - throw new IllegalArgumentException("Travel date cannot be in the past."); + if (travelDate == null) { + throw new IllegalArgumentException("Travel date is required."); + } + if (travelDate.isBefore(LocalDate.now())) { + throw new IllegalArgumentException("Travel date cannot be in the past."); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java` around lines 342 - 346, In validateTravelDate(LocalDate travelDate) distinguish the null case from the past-date case: first check if travelDate == null and throw an IllegalArgumentException (or NullPointerException if preferred) with a clear message like "Travel date cannot be null.", then check travelDate.isBefore(LocalDate.now()) and throw "Travel date cannot be in the past."; update the thrown messages in the validateTravelDate method so callers can tell whether the input was missing versus actually earlier than today.
58-70: Consolidate duplicate applicant visa retrieval methods.
findVisasByApplicant()andfindVisasByApplicantId()both query visas by applicant ID and sort byupdatedAtdescending using identical implementation patterns. Consider removing one and updating callers (VisaViewController and UserViewController) to use a single method with a consistent name to reduce maintenance overhead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java` around lines 58 - 70, There are two duplicate methods in VisaService — findVisasByApplicant(...) and findVisasByApplicantId(...) — both calling visaRepository.findByApplicant_Id(..., Sort.by(Sort.Direction.DESC, "updatedAt")) and mapping via visaMapper::toDTO; pick a single canonical method name (e.g., keep findVisasByApplicantId(Long applicantId) with the existing signature and behavior), remove the other duplicate method from VisaService, and update all call sites (VisaViewController and UserViewController) to call the retained method; ensure the retained method preserves the Sort.by(Sort.Direction.DESC, "updatedAt") and List<VisaDTO> return type so behavior and tests remain unchanged.src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java (1)
89-95: Same overly broad exception mapping issue in apply flow.
applyForVisa()currently only throwsIllegalArgumentExceptionfor travel date validation, but if additional validations are added later, they would also be incorrectly mapped to thetravelDatefield. Consider using a global error or distinct exception types for consistency with the edit flow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java` around lines 89 - 95, The catch in the apply flow maps any IllegalArgumentException from visaService.applyForVisa(createVisaDTO, currentUserId) to the travelDate field, which is too broad; change the handling in VisaViewController.apply (the try/catch around applyForVisa) to either catch a more specific exception type thrown only for travel date validation or catch IllegalArgumentException and map it to a global error (use bindingResult.reject("error.global", e.getMessage())) instead of bindingResult.rejectValue("travelDate", ...); keep the call to prepareApplyModel(currentUserId, model) and return "visa/apply-form" as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java`:
- Around line 146-156: The catch in VisaViewController around
visaService.updateVisa(id, updateVisaDTO, currentUserId) wrongly maps all
IllegalArgumentException instances to the travelDate field; update the handler
to distinguish error types by either catching specific exceptions thrown by
VisaService (e.g., create and throw a dedicated InvalidStateException for
status/check errors and catch it separately) or by adding
bindingResult.reject("globalError", e.getMessage()) for non-travel-date errors
so only the actual "Travel date cannot be in the past." message is bound to
"travelDate" while other messages (e.g., "Mismatched visa id.", "This
application can no longer be edited.") are reported as global errors; adjust the
catch blocks in VisaViewController accordingly and ensure
prepareApplyModel(...), model.addAttribute("isEdit", true) and the return
"visa/edit-form" logic remain intact.
---
Nitpick comments:
In
`@src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java`:
- Around line 89-95: The catch in the apply flow maps any
IllegalArgumentException from visaService.applyForVisa(createVisaDTO,
currentUserId) to the travelDate field, which is too broad; change the handling
in VisaViewController.apply (the try/catch around applyForVisa) to either catch
a more specific exception type thrown only for travel date validation or catch
IllegalArgumentException and map it to a global error (use
bindingResult.reject("error.global", e.getMessage())) instead of
bindingResult.rejectValue("travelDate", ...); keep the call to
prepareApplyModel(currentUserId, model) and return "visa/apply-form" as before.
In
`@src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java`:
- Around line 342-346: In validateTravelDate(LocalDate travelDate) distinguish
the null case from the past-date case: first check if travelDate == null and
throw an IllegalArgumentException (or NullPointerException if preferred) with a
clear message like "Travel date cannot be null.", then check
travelDate.isBefore(LocalDate.now()) and throw "Travel date cannot be in the
past."; update the thrown messages in the validateTravelDate method so callers
can tell whether the input was missing versus actually earlier than today.
- Around line 58-70: There are two duplicate methods in VisaService —
findVisasByApplicant(...) and findVisasByApplicantId(...) — both calling
visaRepository.findByApplicant_Id(..., Sort.by(Sort.Direction.DESC,
"updatedAt")) and mapping via visaMapper::toDTO; pick a single canonical method
name (e.g., keep findVisasByApplicantId(Long applicantId) with the existing
signature and behavior), remove the other duplicate method from VisaService, and
update all call sites (VisaViewController and UserViewController) to call the
retained method; ensure the retained method preserves the
Sort.by(Sort.Direction.DESC, "updatedAt") and List<VisaDTO> return type so
behavior and tests remain unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 44d6085d-7e24-4a1a-b159-a34346c6d7af
📒 Files selected for processing (2)
src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.javasrc/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java (2)
222-229: Consider making this helper method private.
prepareApplyModelis only used within this controller and doesn't need to bepublic.✏️ Suggested fix
- public void prepareApplyModel(Long currentUserId, Model model) { + private void prepareApplyModel(Long currentUserId, Model model) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java` around lines 222 - 229, The helper method prepareApplyModel is declared public but is only used inside VisaViewController; change its visibility to private to encapsulate it. Locate the prepareApplyModel method (which calls userService.findById(...).orElseThrow(...), adds "currentUser" using UserDTO and sets "visaTypes" via VisaType.values() on the Model) and update its declaration from public to private, leaving the method body and behavior unchanged.
146-161: Consider using exception types instead of message parsing.The
e.getMessage().contains("date")check is functional but fragile—if the error messages inVisaServicechange, this routing logic will break silently. A more robust approach would use distinct exception types.💡 Alternative approach using custom exceptions
Create a
TravelDateValidationExceptionin the service layer:// New exception class public class TravelDateValidationException extends IllegalArgumentException { public TravelDateValidationException(String message) { super(message); } }Then in
VisaService.validateTravelDate():-throw new IllegalArgumentException("Travel date cannot be in the past."); +throw new TravelDateValidationException("Travel date cannot be in the past.");And in the controller:
-} catch (IllegalArgumentException e) { - if (e.getMessage().contains("date")) { - bindingResult.rejectValue("travelDate", "error.travelDate", e.getMessage()); - } else { - bindingResult.reject("globalError",e.getMessage()); - } +} catch (TravelDateValidationException e) { + bindingResult.rejectValue("travelDate", "error.travelDate", e.getMessage()); +} catch (IllegalArgumentException e) { + bindingResult.reject("globalError", e.getMessage());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java` around lines 146 - 161, Replace fragile message-parsing in VisaViewController: instead of checking e.getMessage().contains("date") after visaService.updateVisa(id, updateVisaDTO, currentUserId) throws IllegalArgumentException, introduce a specific custom exception (e.g., TravelDateValidationException extends IllegalArgumentException) thrown from the service (e.g., VisaService.validateTravelDate/ updateVisa) for travel-date errors, then catch TravelDateValidationException in the controller and call bindingResult.rejectValue("travelDate", ...) for that catch branch while keeping a separate catch(IllegalArgumentException e) for other errors that calls bindingResult.reject("globalError", ...); update imports and remove the message-contains logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java`:
- Around line 342-350: The null-check in validateTravelDate currently throws an
IllegalArgumentException with the message "Travel date cannot be in the past."
which is misleading; change the null branch to throw an IllegalArgumentException
with a clear message like "Travel date is required" (or similar), while keeping
the existing past-date check and its message for
travelDate.isBefore(LocalDate.now()) so callers can distinguish missing vs past
dates.
---
Nitpick comments:
In
`@src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.java`:
- Around line 222-229: The helper method prepareApplyModel is declared public
but is only used inside VisaViewController; change its visibility to private to
encapsulate it. Locate the prepareApplyModel method (which calls
userService.findById(...).orElseThrow(...), adds "currentUser" using UserDTO and
sets "visaTypes" via VisaType.values() on the Model) and update its declaration
from public to private, leaving the method body and behavior unchanged.
- Around line 146-161: Replace fragile message-parsing in VisaViewController:
instead of checking e.getMessage().contains("date") after
visaService.updateVisa(id, updateVisaDTO, currentUserId) throws
IllegalArgumentException, introduce a specific custom exception (e.g.,
TravelDateValidationException extends IllegalArgumentException) thrown from the
service (e.g., VisaService.validateTravelDate/ updateVisa) for travel-date
errors, then catch TravelDateValidationException in the controller and call
bindingResult.rejectValue("travelDate", ...) for that catch branch while keeping
a separate catch(IllegalArgumentException e) for other errors that calls
bindingResult.reject("globalError", ...); update imports and remove the
message-contains logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 78c44227-6bb3-499f-92bf-396626ffae1a
📒 Files selected for processing (5)
src/main/java/org/example/visacasemanagementsystem/visa/controller/VisaViewController.javasrc/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.javasrc/main/resources/templates/visa/apply-form.htmlsrc/main/resources/templates/visa/edit-form.htmlsrc/test/java/org/example/visacasemanagementsystem/visa/VisaServiceTest.java
✅ Files skipped from review due to trivial changes (1)
- src/main/resources/templates/visa/apply-form.html
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/resources/templates/visa/edit-form.html
- src/test/java/org/example/visacasemanagementsystem/visa/VisaServiceTest.java
This was referenced Apr 15, 2026
Merged
Merged
This was referenced Apr 22, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores