Feature/user endpoints#26
Merged
Merged
Conversation
- Created SecurityUser, which is a wrapper for user meant to safely interact with Spring Security. - Created UserViewerController mean tto contain User specific views. - Added Dashboard logic to ApplicationViewController. - Added helper methods for the dashboard to AuditService and VisaService which are yet to be correctly implemented.
- Moved dashboards from ApplicationViewController to UserViewController - Added basic controller endpoint structure - Added basic user signup logic
- Added controller logic to viewProfile, showProfileEditForm, updateProfile, userListView - Added additional validation methods for checking access requirements to UserService
- Changed from /applicant/dashboard to /dashboard/applicant ApplicationViewController: - Added dashboard redirection logic based on user role Templates: - Added templates following existing style in visa application pages for the following views: signup, view, login, list, header, edit - Updated dashboards to follow the new theme Security: - Added UserDetailsServiceImpl which lets Spring Security access the UserRepository to fetch usernames
📝 WalkthroughWalkthroughAdds role-based authentication and authorization: a SecurityUser and UserDetailsService, authorization helpers in UserService, role-aware dashboard routing, new user/profile controllers, repository/service queries for visa/audit, and multiple Thymeleaf templates and fragments for role-specific UIs. Changes
Sequence DiagramsequenceDiagram
participant Browser as User (Browser)
participant Auth as Spring Security
participant UDS as UserDetailsServiceImpl
participant Repo as UserRepository
participant SU as SecurityUser
participant MVC as UserViewController / ApplicationViewController
participant VisaSvc as VisaService
participant AuditSvc as AuditService
Browser->>Auth: POST /user/login (email,password)
Auth->>UDS: loadUserByUsername(email)
UDS->>Repo: findByEmail(email)
Repo-->>UDS: User entity
UDS->>SU: new SecurityUser(user)
SU-->>UDS: SecurityUser (ROLE_*)
UDS-->>Auth: UserDetails
Auth->>Auth: validate password
Auth-->>Browser: authenticated (principal=SecurityUser)
Browser->>MVC: GET /dashboard
MVC->>MVC: inspect principal.authorities
alt ROLE_SYSADMIN
MVC-->>Browser: redirect /dashboard/sysadmin
MVC->>AuditSvc: findAll()
MVC->>Repo: (user listing via UserService)
else ROLE_ADMIN
MVC-->>Browser: redirect /dashboard/admin
MVC->>VisaSvc: findVisasByHandlerId(...) / findVisasByApplicantId(...)
else Default
MVC-->>Browser: redirect /dashboard/applicant
MVC->>VisaSvc: findVisasByApplicantId(principal.userId)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/org/example/visacasemanagementsystem/user/service/UserService.java (1)
50-61:⚠️ Potential issue | 🟠 MajorPassword encryption should now be implemented.
Spring Security is being integrated in this PR, which was the condition for adding password encryption. The plaintext password storage at line 53 should be replaced with
BCryptPasswordEncoder:🔐 Proposed fix
Inject
PasswordEncoderinto the service:private final PasswordEncoder passwordEncoder; public UserService(UserRepository userRepository, UserMapper userMapper, PasswordEncoder passwordEncoder) { this.userRepository = userRepository; this.userMapper = userMapper; this.passwordEncoder = passwordEncoder; }Then encode the password:
-user.setPassword(dto.password()); +user.setPassword(passwordEncoder.encode(dto.password()));Based on learnings: "password encryption in
UserService.createUser()is intentionally deferred until Spring Security is integrated."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/visacasemanagementsystem/user/service/UserService.java` around lines 50 - 61, Replace storing plaintext password in createUser by injecting and using a PasswordEncoder: add a private final PasswordEncoder field to UserService, update the UserService constructor to accept PasswordEncoder alongside UserRepository and UserMapper, and then replace user.setPassword(dto.password()) in createUser(...) with user.setPassword(passwordEncoder.encode(dto.password())); keep existing exception handling and userAuthorization logic untouched.
🧹 Nitpick comments (7)
src/main/java/org/example/visacasemanagementsystem/visa/repository/VisaRepository.java (1)
27-29: Consider addingSortparameter for consistency.Other query methods in this repository accept a
Sortparameter for ordering results. The new methods return unsorted lists, which may lead to inconsistent display ordering in dashboards. Consider addingSortfor parity:List<Visa> findVisasByApplicantId(Long applicantId, Sort sort); List<Visa> findVisasByHandlerId(Long handlerId, Sort sort);🤖 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/repository/VisaRepository.java` around lines 27 - 29, In VisaRepository update the two finder signatures (findVisasByApplicantId and findVisasByHandlerId) to accept a Spring Data Sort parameter (e.g., change to findVisasByApplicantId(Long applicantId, Sort sort) and findVisasByHandlerId(Long handlerId, Sort sort)), add the necessary import for org.springframework.data.domain.Sort, and then update any callers/tests to pass a Sort instance (or Sort.unsorted()) so results are returned with explicit ordering consistent with other repository methods.src/main/resources/templates/user/signup.html (2)
153-158: Consider adding client-side password requirements.Adding
minlengthattribute provides immediate feedback to users about password requirements:<input type="password" name="password" placeholder="Choose a password" minlength="8" required>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/templates/user/signup.html` around lines 153 - 158, The password input currently lacks client-side length validation; update the password <input> element used in the signup form (the password input with name="password") to include a minlength attribute (e.g. minlength="8") so browsers give immediate feedback, and optionally add a brief title or pattern if you want stricter rules (keep required=true). Ensure you only modify the input tag for the password field to include minlength="8".
112-121: Unused CSS classes.The
.is-invalidand.error-textCSS classes are defined but not used in the template. Either remove them or implement field-level validation styling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/templates/user/signup.html` around lines 112 - 121, The CSS defines .is-invalid and .error-text but they are unused; either remove these rules or apply them to the form inputs and error messages: update the signup form inputs (e.g., the email, password, confirmPassword inputs) to conditionally add the "is-invalid" class when validation errors exist and render a sibling element with class "error-text" to show the field-specific error message (use your template's existing validation/error object or server-side error variables); if you prefer removal, delete the .is-invalid and .error-text rules from the template stylesheet.src/main/java/org/example/visacasemanagementsystem/audit/service/AuditService.java (1)
28-33: Consider pagination for audit log retrieval.Fetching all audit logs without pagination may cause performance issues as the audit table grows. For an initial implementation this is acceptable, but consider adding pagination support (e.g.,
Pageableparameter) before production use.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/visacasemanagementsystem/audit/service/AuditService.java` around lines 28 - 33, The current AuditService.findAll method loads all audit rows which will not scale; change its signature to accept a Pageable (e.g., findAll(Pageable pageable)) and use auditRepository.findAll(pageable) then map results with auditMapper::toDTO, returning a Page<AuditDTO> (or Slice<AuditDTO>) so pagination metadata is preserved; update callers and tests to supply Pageable and handle the Page<AuditDTO> response accordingly.src/main/java/org/example/visacasemanagementsystem/ApplicationViewController.java (1)
12-22: Consider null check for unauthenticated access.If an unauthenticated user somehow reaches this endpoint (e.g., misconfigured security),
principalwill benull, causing an NPE. While Spring Security should protect this route, a defensive check provides clearer error handling:🛡️ Proposed defensive check
`@GetMapping`("/dashboard") public String dashboard(`@AuthenticationPrincipal` SecurityUser principal) { + if (principal == null) { + return "redirect:/user/login"; + } boolean isSysAdmin = principal.getAuthorities().stream()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/visacasemanagementsystem/ApplicationViewController.java` around lines 12 - 22, In ApplicationViewController.dashboard, add a defensive null check for the AuthenticationPrincipal parameter (SecurityUser principal) before calling principal.getAuthorities(); if principal is null, return a safe response such as "redirect:/login" (or throw an AccessDeniedException) so an unauthenticated request won’t cause an NPE; keep the rest of the authority checks (isSysAdmin/isAdmin) unchanged and perform them only after the null check.src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java (1)
248-260: Add null guards for new finder methods.Line 248-Line 260 should validate
applicantIdandhandlerIdfor null to match the defensive style used in the rest of this service.🔧 Proposed refactor
public List<VisaDTO> findVisasByApplicantId(Long applicantId) { + if (applicantId == null) { + throw new IllegalArgumentException("Applicant id cannot be null"); + } return visaRepository.findVisasByApplicantId(applicantId) .stream() .map(visaMapper::toDTO) .toList(); } public List<VisaDTO> findVisasByHandlerId(Long handlerId) { + if (handlerId == null) { + throw new IllegalArgumentException("Handler id cannot be null"); + } return visaRepository.findVisasByHandlerId(handlerId) .stream() .map(visaMapper::toDTO) .toList(); }🤖 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 248 - 260, The new finder methods findVisasByApplicantId and findVisasByHandlerId must validate their input IDs are not null like other methods in this service: add a null check for applicantId/handlerId at the start of each method and throw an IllegalArgumentException (or use Objects.requireNonNull with a clear message) when null, so the methods fail fast with a descriptive message rather than passing null into visaRepository.src/main/java/org/example/visacasemanagementsystem/user/controller/UserViewController.java (1)
157-158: Consider pagination/limits for sysadmin dashboard datasets.Fetching all users and all audit logs for one dashboard request can become expensive and degrade page load as data grows.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/visacasemanagementsystem/user/controller/UserViewController.java` around lines 157 - 158, The dashboard currently calls userService.findAll() and auditService.findAll() in UserViewController (variables allUsers and recentLogs) which will load entire tables; change these calls to use paginated or limited queries (e.g., userService.findPage(Pageable) or userService.findLatest(limit) and auditService.findPage(Pageable) or auditService.findLatest(limit)) and update UserViewController to accept/compute page/limit parameters (or use sensible defaults like latest 50 entries) so the controller only fetches a bounded result set rather than all rows.
🤖 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/user/controller/UserViewController.java`:
- Around line 73-87: In viewProfile (UserViewController.viewProfile) add the
same ownership/role guard used by the edit routes by invoking the existing
validateProfileAccess(...) before loading the UserDTO; if validateProfileAccess
is a private helper on the controller reuse it (e.g.
validateProfileAccess(principal, userId)) or call the same service method used
by edit endpoints, and short-circuit (throw AccessDeniedException or return
appropriate error/redirect) when access is denied so only owners or sysadmins
can view arbitrary profiles.
- Around line 143-163: Add explicit role checks at the start of adminDashboard
and sysAdminDashboard to enforce defense-in-depth: verify the authenticated
SecurityUser (principal) has the required role (e.g., ROLE_ADMIN for
adminDashboard, ROLE_SYSADMIN for sysAdminDashboard) and if not, either throw an
AccessDeniedException or return a 403 view (e.g., "error/403"); mirror the
in-handler check pattern used in userListView to prevent direct URL access to
sensitive data. Ensure the check uses principal.getRoles() or equivalent and
short-circuit before calling visaService, userService, or auditService.
In
`@src/main/java/org/example/visacasemanagementsystem/user/security/SecurityUser.java`:
- Around line 33-47: SecurityUser currently returns plaintext passwords via
getPassword and UserService.createUser assigns dto.password() directly; inject a
PasswordEncoder (use BCryptPasswordEncoder) into UserService and replace
user.setPassword(dto.password()) in createUser with
user.setPassword(passwordEncoder.encode(dto.password())); also register a
BCryptPasswordEncoder bean (e.g., as PasswordEncoder) in your configuration so
Spring can autowire it and ensure SecurityUser.getPassword continues to return
the encoded value stored on the User entity.
In
`@src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java`:
- Around line 72-74: The catch block in VisaService.java uses `_` as the
exception variable which is a reserved keyword in Java 25; change the catch
parameter to a valid identifier (e.g., catch (IllegalArgumentException e)) in
the method where the block appears and, if needed, reference the exception
variable when rethrowing or logging (for example include e.getMessage() if you
want details) to ensure the code compiles.
In `@src/main/resources/templates/dashboard/admin.html`:
- Around line 167-170: The dashboard action links currently use placeholders;
add controller endpoints that expose the existing service methods (e.g., create
POST/PUT routes wired to approveVisa(), rejectVisa(), assignHandler() in the
appropriate controller) and update the admin.html action anchors to point to
those routes (use proper HTTP verbs via forms or fetch calls for
Approve/Reject/Assign and a GET route for Review to reuse the Review handler).
Ensure route names and HTTP methods align with the service method semantics and
CSRF/authorization checks are applied when wiring the links.
In `@src/main/resources/templates/dashboard/applicant.html`:
- Line 139: Replace the placeholder anchors that use href="#" with real
application routes: update the anchor with class "btn-apply" to point to the
new-application route (e.g. the create/apply page) and update the other
placeholder anchor(s) on this template (the application-detail link at the later
anchor) to use the application details route including the application id
placeholder (e.g. /applications/{id} or your framework’s route helper). Ensure
you use your templating/URL helper (not a hardcoded "#") so links render
per-user (e.g., use the framework’s path helper or a variable like
application.id) and preserve the existing classes/attributes such as btn-apply
when making the change.
In `@src/main/resources/templates/profile/edit.html`:
- Around line 138-164: The POST profile edit form
(th:action="@{/profile/edit/{id}(id=${user.id()})}") lacks a CSRF token and will
be rejected by Spring Security; add a hidden input inside the form that uses
Thymeleaf's _csrf attributes (th:name="${_csrf.parameterName}" and
th:value="${_csrf.token}") so the token is submitted with the request.
---
Outside diff comments:
In
`@src/main/java/org/example/visacasemanagementsystem/user/service/UserService.java`:
- Around line 50-61: Replace storing plaintext password in createUser by
injecting and using a PasswordEncoder: add a private final PasswordEncoder field
to UserService, update the UserService constructor to accept PasswordEncoder
alongside UserRepository and UserMapper, and then replace
user.setPassword(dto.password()) in createUser(...) with
user.setPassword(passwordEncoder.encode(dto.password())); keep existing
exception handling and userAuthorization logic untouched.
---
Nitpick comments:
In
`@src/main/java/org/example/visacasemanagementsystem/ApplicationViewController.java`:
- Around line 12-22: In ApplicationViewController.dashboard, add a defensive
null check for the AuthenticationPrincipal parameter (SecurityUser principal)
before calling principal.getAuthorities(); if principal is null, return a safe
response such as "redirect:/login" (or throw an AccessDeniedException) so an
unauthenticated request won’t cause an NPE; keep the rest of the authority
checks (isSysAdmin/isAdmin) unchanged and perform them only after the null
check.
In
`@src/main/java/org/example/visacasemanagementsystem/audit/service/AuditService.java`:
- Around line 28-33: The current AuditService.findAll method loads all audit
rows which will not scale; change its signature to accept a Pageable (e.g.,
findAll(Pageable pageable)) and use auditRepository.findAll(pageable) then map
results with auditMapper::toDTO, returning a Page<AuditDTO> (or Slice<AuditDTO>)
so pagination metadata is preserved; update callers and tests to supply Pageable
and handle the Page<AuditDTO> response accordingly.
In
`@src/main/java/org/example/visacasemanagementsystem/user/controller/UserViewController.java`:
- Around line 157-158: The dashboard currently calls userService.findAll() and
auditService.findAll() in UserViewController (variables allUsers and recentLogs)
which will load entire tables; change these calls to use paginated or limited
queries (e.g., userService.findPage(Pageable) or userService.findLatest(limit)
and auditService.findPage(Pageable) or auditService.findLatest(limit)) and
update UserViewController to accept/compute page/limit parameters (or use
sensible defaults like latest 50 entries) so the controller only fetches a
bounded result set rather than all rows.
In
`@src/main/java/org/example/visacasemanagementsystem/visa/repository/VisaRepository.java`:
- Around line 27-29: In VisaRepository update the two finder signatures
(findVisasByApplicantId and findVisasByHandlerId) to accept a Spring Data Sort
parameter (e.g., change to findVisasByApplicantId(Long applicantId, Sort sort)
and findVisasByHandlerId(Long handlerId, Sort sort)), add the necessary import
for org.springframework.data.domain.Sort, and then update any callers/tests to
pass a Sort instance (or Sort.unsorted()) so results are returned with explicit
ordering consistent with other repository methods.
In
`@src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java`:
- Around line 248-260: The new finder methods findVisasByApplicantId and
findVisasByHandlerId must validate their input IDs are not null like other
methods in this service: add a null check for applicantId/handlerId at the start
of each method and throw an IllegalArgumentException (or use
Objects.requireNonNull with a clear message) when null, so the methods fail fast
with a descriptive message rather than passing null into visaRepository.
In `@src/main/resources/templates/user/signup.html`:
- Around line 153-158: The password input currently lacks client-side length
validation; update the password <input> element used in the signup form (the
password input with name="password") to include a minlength attribute (e.g.
minlength="8") so browsers give immediate feedback, and optionally add a brief
title or pattern if you want stricter rules (keep required=true). Ensure you
only modify the input tag for the password field to include minlength="8".
- Around line 112-121: The CSS defines .is-invalid and .error-text but they are
unused; either remove these rules or apply them to the form inputs and error
messages: update the signup form inputs (e.g., the email, password,
confirmPassword inputs) to conditionally add the "is-invalid" class when
validation errors exist and render a sibling element with class "error-text" to
show the field-specific error message (use your template's existing
validation/error object or server-side error variables); if you prefer removal,
delete the .is-invalid and .error-text rules from the template stylesheet.
🪄 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: 1155dcd8-1268-4e6d-852f-cb7d659caeb2
📒 Files selected for processing (17)
src/main/java/org/example/visacasemanagementsystem/ApplicationViewController.javasrc/main/java/org/example/visacasemanagementsystem/audit/service/AuditService.javasrc/main/java/org/example/visacasemanagementsystem/user/controller/UserViewController.javasrc/main/java/org/example/visacasemanagementsystem/user/security/SecurityUser.javasrc/main/java/org/example/visacasemanagementsystem/user/security/UserDetailsServiceImpl.javasrc/main/java/org/example/visacasemanagementsystem/user/service/UserService.javasrc/main/java/org/example/visacasemanagementsystem/visa/repository/VisaRepository.javasrc/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.javasrc/main/resources/templates/dashboard/admin.htmlsrc/main/resources/templates/dashboard/applicant.htmlsrc/main/resources/templates/dashboard/sysadmin.htmlsrc/main/resources/templates/fragments/header.htmlsrc/main/resources/templates/profile/edit.htmlsrc/main/resources/templates/profile/view.htmlsrc/main/resources/templates/user/list.htmlsrc/main/resources/templates/user/login.htmlsrc/main/resources/templates/user/signup.html
Comment on lines
+167
to
+170
| <a href="#" class="action-link">Approve</a> | ||
| <a href="#" class="action-link">Reject</a> | ||
| <a href="#" class="view-link">Review →</a> | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if backend endpoints exist for admin case actions
rg -n --type=java -C3 'Mapping\\(".*(approve|reject|assign|review)'
# Confirm placeholders still present in this template
rg -n -C2 'href="#"' src/main/resources/templates/dashboard/admin.htmlRepository: ithsjava25/visa-case-management-system
Length of output: 803
🏁 Script executed:
#!/bin/bash
# Search for controller mappings related to case actions
echo "=== Searching for controller endpoints for approve, reject, assign ==="
rg -n --type=java -i '(approve|reject|assign|review)' --glob='*Controller*.java' -B2 -A2
echo -e "\n=== Check for RequestMapping or PostMapping annotations ==="
rg -n --type=java '(RequestMapping|PostMapping|GetMapping).*/(approve|reject|assign|review|admin)' -B1 -A3
echo -e "\n=== List Java files in controllers directory ==="
fd --type f '\.java$' | grep -i controller | head -20Repository: ithsjava25/visa-case-management-system
Length of output: 2655
🏁 Script executed:
#!/bin/bash
# Search for service methods related to case actions
echo "=== Search in service files for approve/reject/assign methods ==="
rg -n --type=java -i 'def\s+(approve|reject|assign)' -B1 -A5
echo -e "\n=== Search for Visa/Case service implementations ==="
fd --type f '\.java$' | grep -i 'service' | head -20
echo -e "\n=== Check VisaService for action methods ==="
fd --type f -i 'visaservice.java' -x cat {} +Repository: ithsjava25/visa-case-management-system
Length of output: 9622
🏁 Script executed:
#!/bin/bash
# Search for controllers that call approveVisa, rejectVisa, or assignHandler
echo "=== Search for usage of service action methods ==="
rg -n --type=java '(approveVisa|rejectVisa|assignHandler)' -B2 -A5
echo -e "\n=== Check for any REST or API controllers ==="
fd --type f '\.java$' | xargs grep -l 'PostMapping\|PutMapping' | head -10
echo -e "\n=== Search for any href patterns in admin.html ==="
rg -n 'href=' src/main/resources/templates/dashboard/admin.html | head -20Repository: ithsjava25/visa-case-management-system
Length of output: 4497
🏁 Script executed:
#!/bin/bash
# Check CommentController to see if there's a pattern for POST mappings
echo "=== Check CommentController structure ==="
rg -n --type=java '@PostMapping|@PutMapping' src/main/java/org/example/visacasemanagementsystem/comment/controller/CommentController.java -B1 -A5
echo -e "\n=== Check if there are any forms in admin.html for form submission ==="
rg -n '<form' src/main/resources/templates/dashboard/admin.html -A10
echo -e "\n=== Search for any script or AJAX in admin.html ==="
rg -n '<script|fetch|XMLHttpRequest' src/main/resources/templates/dashboard/admin.htmlRepository: ithsjava25/visa-case-management-system
Length of output: 557
Create endpoints for visa action flows and wire dashboard links to real routes.
The Approve, Reject, Review, and Assign actions in the admin dashboard still use href="#" placeholders (lines 167-170 and 210), making these workflows non-functional. While the backend service methods exist (approveVisa(), rejectVisa(), assignHandler()), no controller endpoints expose them. These links must be wired to actual routes once the corresponding POST/PUT endpoints are implemented.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/templates/dashboard/admin.html` around lines 167 - 170,
The dashboard action links currently use placeholders; add controller endpoints
that expose the existing service methods (e.g., create POST/PUT routes wired to
approveVisa(), rejectVisa(), assignHandler() in the appropriate controller) and
update the admin.html action anchors to point to those routes (use proper HTTP
verbs via forms or fetch calls for Approve/Reject/Assign and a GET route for
Review to reuse the Review handler). Ensure route names and HTTP methods align
with the service method semantics and CSRF/authorization checks are applied when
wiring the links.
Contributor
Author
There was a problem hiding this comment.
They will be linked in a future issue
There was a problem hiding this comment.
@Martin-E-Karlsson, understood! Would you like me to open a GitHub issue to track wiring the Approve, Reject, Review, and Assign action links in the admin dashboard to their respective controller endpoints (backed by approveVisa(), rejectVisa(), and assignHandler() in VisaService)? That way it won't get lost.
🧠 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.
| <!-- Empty state --> | ||
| <div th:if="${visas.isEmpty()}" class="empty-state"> | ||
| <p>You haven't submitted any visa applications yet.</p> | ||
| <a href="#" class="btn-apply">Apply for a Visa</a> |
There was a problem hiding this comment.
Wire applicant CTA/actions to actual routes.
Line 139 and Line 170 are placeholder links (href="#"), so users can’t apply for a visa or open application details from this page.
Also applies to: 170-170
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/templates/dashboard/applicant.html` at line 139, Replace
the placeholder anchors that use href="#" with real application routes: update
the anchor with class "btn-apply" to point to the new-application route (e.g.
the create/apply page) and update the other placeholder anchor(s) on this
template (the application-detail link at the later anchor) to use the
application details route including the application id placeholder (e.g.
/applications/{id} or your framework’s route helper). Ensure you use your
templating/URL helper (not a hardcoded "#") so links render per-user (e.g., use
the framework’s path helper or a variable like application.id) and preserve the
existing classes/attributes such as btn-apply when making the change.
Contributor
Author
There was a problem hiding this comment.
They will be linked in a future issue
There was a problem hiding this comment.
@Martin-E-Karlsson, understood! Would you like me to open a GitHub issue to track the wiring of the placeholder links (href="#") on the applicant dashboard (the "Apply for a Visa" CTA and the application-detail link) to their real routes? That way it won't be forgotten.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/java/org/example/visacasemanagementsystem/ApplicationViewController.java (1)
17-20: Extract role check helper to reduce duplication.The two stream scans are repetitive and harder to maintain as roles grow. A small helper (
hasRole(principal, role)) will keep this logic cleaner.Refactor sketch
- boolean isSysAdmin = principal.getAuthorities().stream() - .anyMatch(a -> Objects.equals(a.getAuthority(), "ROLE_SYSADMIN")); - boolean isAdmin = principal.getAuthorities().stream() - .anyMatch(a -> Objects.equals(a.getAuthority(), "ROLE_ADMIN")); + boolean isSysAdmin = hasRole(principal, "ROLE_SYSADMIN"); + boolean isAdmin = hasRole(principal, "ROLE_ADMIN");private boolean hasRole(SecurityUser principal, String role) { return principal.getAuthorities().stream() .anyMatch(a -> Objects.equals(a.getAuthority(), role)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/visacasemanagementsystem/ApplicationViewController.java` around lines 17 - 20, Duplicate role-check streams in ApplicationViewController (calculating isSysAdmin and isAdmin) should be replaced by a small helper to centralize logic: add a private boolean hasRole(SecurityUser principal, String role) that performs principal.getAuthorities().stream().anyMatch(a -> Objects.equals(a.getAuthority(), role)), then replace the two duplicated expressions with hasRole(principal, "ROLE_SYSADMIN") and hasRole(principal, "ROLE_ADMIN") respectively to improve readability and maintainability.
🤖 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/ApplicationViewController.java`:
- Line 24: The controller currently defaults any authenticated user to
"redirect:/dashboard/applicant" without verifying ROLE_APPLICANT; update the
routing logic in ApplicationViewController so you explicitly check the user's
authorities (e.g. via
SecurityContextHolder.getContext().getAuthentication().getAuthorities()) for the
ROLE_APPLICANT authority before returning "redirect:/dashboard/applicant",
mirror the pattern used by validateAdmin() and validateSysAdmin() (or add a new
validateApplicant() method) and if the applicant role is missing route to an
access-denied page or return an appropriate error/redirect instead of silently
sending them to the applicant dashboard.
In
`@src/main/java/org/example/visacasemanagementsystem/user/dto/CreateUserDTO.java`:
- Line 11: The service currently persists plaintext passwords in
UserService.createUser() via user.setPassword(dto.password()); fix this by
declaring a BCryptPasswordEncoder bean (e.g., a `@Bean` public PasswordEncoder
passwordEncoder() returning new BCryptPasswordEncoder()), inject PasswordEncoder
into UserService (constructor injection), and replace the plaintext assignment
with user.setPassword(passwordEncoder.encode(dto.password())); ensure imports
reference org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder and
PasswordEncoder and update any tests to expect encoded passwords.
---
Nitpick comments:
In
`@src/main/java/org/example/visacasemanagementsystem/ApplicationViewController.java`:
- Around line 17-20: Duplicate role-check streams in ApplicationViewController
(calculating isSysAdmin and isAdmin) should be replaced by a small helper to
centralize logic: add a private boolean hasRole(SecurityUser principal, String
role) that performs principal.getAuthorities().stream().anyMatch(a ->
Objects.equals(a.getAuthority(), role)), then replace the two duplicated
expressions with hasRole(principal, "ROLE_SYSADMIN") and hasRole(principal,
"ROLE_ADMIN") respectively to improve readability and maintainability.
🪄 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: c4a81d6e-eae6-42a4-9f15-9cc16afb1a6c
📒 Files selected for processing (7)
src/main/java/org/example/visacasemanagementsystem/ApplicationViewController.javasrc/main/java/org/example/visacasemanagementsystem/user/controller/UserViewController.javasrc/main/java/org/example/visacasemanagementsystem/user/dto/CreateUserDTO.javasrc/main/java/org/example/visacasemanagementsystem/user/service/UserService.javasrc/main/java/org/example/visacasemanagementsystem/visa/repository/VisaRepository.javasrc/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.javasrc/main/resources/templates/profile/edit.html
✅ Files skipped from review due to trivial changes (1)
- src/main/resources/templates/profile/edit.html
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/org/example/visacasemanagementsystem/visa/repository/VisaRepository.java
- src/main/java/org/example/visacasemanagementsystem/user/service/UserService.java
- src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java
- src/main/java/org/example/visacasemanagementsystem/user/controller/UserViewController.java
| @NotBlank(message = "Full name must be specified") String fullName, | ||
| @NotBlank(message = "Email must be specified") String email, | ||
| @NotBlank String password, | ||
| @Size(min = 8, message = "Password must be at least 8 characters") @NotBlank String password, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Locate user service classes"
fd -i "UserService*.java"
echo
echo "2) Check password handling in service layer"
rg -n --type=java -C3 '\b(setPassword\s*\(|passwordEncoder|BCryptPasswordEncoder|\.encode\s*\()' src/main/java
echo
echo "3) Check create-user flow for direct dto.password usage"
rg -n --type=java -C3 '\bdto\.password\s*\(' src/main/javaRepository: ithsjava25/visa-case-management-system
Length of output: 2259
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking for Spring Security integration in this PR ==="
echo
echo "1) Look for Spring Security config files:"
fd -i ".*security.*config.*\.java"
echo
echo "2) Check for Spring Security dependencies in pom.xml:"
if [ -f "pom.xml" ]; then
rg -A2 "spring-security" pom.xml
else
echo "No pom.xml found"
fi
echo
echo "3) Look for SecurityContextHolder or Spring Security annotations:"
rg -n --type=java '\b(SecurityContextHolder|@EnableWebSecurity|SecurityConfig|@PreAuthorize|@Secured)' src/main/java | head -20
echo
echo "4) Check if Spring Security was added in this specific service:"
head -40 src/main/java/org/example/visacasemanagementsystem/user/service/UserService.java | grep -E "import|@|class"Repository: ithsjava25/visa-case-management-system
Length of output: 239
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking pom.xml for Spring Security status ==="
if [ -f "pom.xml" ]; then
echo "pom.xml found. Checking Spring dependencies:"
cat pom.xml | grep -A5 -B5 "spring-security\|spring-boot" | head -50
else
echo "pom.xml not found. Listing root files:"
ls -la | head -20
fi
echo
echo "=== Complete UserService.createUser method ==="
sed -n '51,70p' src/main/java/org/example/visacasemanagementsystem/user/service/UserService.javaRepository: ithsjava25/visa-case-management-system
Length of output: 3259
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Searching for PasswordEncoder configuration ==="
echo
echo "1) Search for PasswordEncoder bean definitions:"
rg -n --type=java '\b(PasswordEncoder|BCryptPasswordEncoder|@Bean.*[Pp]assword)' src/main/java
echo
echo "2) Search for config classes that might define security beans:"
fd -i "config.*\.java" src/main/java
echo
echo "3) Check all imports in UserService to see what's available:"
head -30 src/main/java/org/example/visacasemanagementsystem/user/service/UserService.java | grep "import"Repository: ithsjava25/visa-case-management-system
Length of output: 182
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Quick check: any password encoding in entire project ==="
rg "encode\|Encoder\|hash" src/main/java --type=java
echo
echo "=== List all config files in project ==="
find src/main/java -name "*Config*.java" -o -name "*Security*.java"Repository: ithsjava25/visa-case-management-system
Length of output: 142
Passwords are stored in plaintext despite Spring Security integration—implement password encoding immediately.
Spring Security is active in the project, but UserService.createUser() still persists raw passwords without encoding (line 54: user.setPassword(dto.password())). DTO-level validation on line 11 is good, but insufficient. Configure a BCryptPasswordEncoder bean and inject it into UserService, replacing the plaintext assignment with passwordEncoder.encode(dto.password()) before persistence.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/org/example/visacasemanagementsystem/user/dto/CreateUserDTO.java`
at line 11, The service currently persists plaintext passwords in
UserService.createUser() via user.setPassword(dto.password()); fix this by
declaring a BCryptPasswordEncoder bean (e.g., a `@Bean` public PasswordEncoder
passwordEncoder() returning new BCryptPasswordEncoder()), inject PasswordEncoder
into UserService (constructor injection), and replace the plaintext assignment
with user.setPassword(passwordEncoder.encode(dto.password())); ensure imports
reference org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder and
PasswordEncoder and update any tests to expect encoded passwords.
Contributor
Author
There was a problem hiding this comment.
Bcrypt will be implemented in a future pull request
There was a problem hiding this comment.
@Martin-E-Karlsson, understood! Since Spring Security is now integrated in this PR, I'll update the note in long-term memory to reflect that BCrypt password encoding is the remaining piece, explicitly deferred to a future PR.
Would you like me to open a GitHub issue to track the BCrypt implementation as a follow-up task?
✏️ Learnings added
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:11.362Z
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())`.
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 removed
Learnt from: Martin-E-Karlsson
Repo: ithsjava25/visa-case-management-system PR: 14
File: src/main/java/org/example/visacasemanagementsystem/user/service/UserService.java:53-53
Timestamp: 2026-04-08T23:24:51.563Z
Learning: In the `visa-case-management-system` repository, password encryption in `UserService.createUser()` (file: `src/main/java/org/example/visacasemanagementsystem/user/service/UserService.java`, line ~53) is intentionally deferred until Spring Security is integrated. The current `user.setPassword(dto.password())` is a temporary plaintext placeholder. When Spring Security is added, a `BCryptPasswordEncoder` bean should be configured and injected into `UserService`, replacing the direct assignment with `passwordEncoder.encode(dto.password())`.
🧠 Learnings used
Learnt from: Martin-E-Karlsson
Repo: ithsjava25/visa-case-management-system PR: 14
File: src/main/java/org/example/visacasemanagementsystem/user/service/UserService.java:53-53
Timestamp: 2026-04-08T23:24:51.563Z
Learning: In the `visa-case-management-system` repository, password encryption in `UserService.createUser()` (file: `src/main/java/org/example/visacasemanagementsystem/user/service/UserService.java`, line ~53) is intentionally deferred until Spring Security is integrated. The current `user.setPassword(dto.password())` is a temporary plaintext placeholder. When Spring Security is added, a `BCryptPasswordEncoder` bean should be configured and injected into `UserService`, replacing the direct assignment with `passwordEncoder.encode(dto.password())`.
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 referenced Apr 19, 2026
Merged
Add comprehensive unit and integration tests for UserViewController, UserMapper, and UserService
#37
Merged
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