feat: implement core visa logic and improve project structure#6
Open
eeebbaandersson wants to merge 9 commits intomainfrom
Open
feat: implement core visa logic and improve project structure#6eeebbaandersson wants to merge 9 commits intomainfrom
eeebbaandersson wants to merge 9 commits intomainfrom
Conversation
Co-authored-by: Martin Karlsson <m.karlsson91@gmail.com> Co-authored-by: Edvin Sandgren <sandgrenedvin@gmail.com>
…eparate package for Comment
- Added validateHandler to verify that a user exists and has ADMIN/SYSADMIN roles. - Implemented exception handling for unauthorized access.
- Added assignHandler with authority validation. - Refactored approve, reject, and update methods to use internal helpers.
📝 WalkthroughWalkthroughRefactors logging into a new Audit module, migrates comment classes to a comment package, removes username from User types, adds VisaType enum and rejectionReason on Visa, implements repositories/services for Visa and User, introduces centralized exception handling, and comments out an S3 Maven dependency. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant VisaController
participant VisaService
participant UserRepository
participant VisaRepository
participant AuditService
participant Database
Client->>VisaController: applyForVisa(CreateVisaDTO, userId)
VisaController->>VisaService: applyForVisa(dto, userId)
VisaService->>UserRepository: findById(userId)
UserRepository->>Database: SELECT user
Database-->>UserRepository: User
UserRepository-->>VisaService: User
VisaService->>VisaRepository: save(visa status=SUBMITTED)
VisaRepository->>Database: INSERT visa
Database-->>VisaRepository: saved visa
VisaService->>AuditService: createAuditLog(userId, visaId, CREATED, "submitted")
AuditService->>Database: INSERT audit_log
Database-->>AuditService: saved
VisaService-->>VisaController: VisaDTO
VisaController-->>Client: 200 OK with VisaDTO
sequenceDiagram
actor Admin
participant VisaController
participant VisaService
participant UserRepository
participant VisaRepository
participant AuditService
participant GlobalExceptionHandler
participant Database
Admin->>VisaController: approveVisa(visaId, handlerId)
VisaController->>VisaService: approveVisa(visaId, handlerId)
VisaService->>UserRepository: findById(handlerId)
UserRepository->>Database: SELECT handler
Database-->>UserRepository: User (role)
UserRepository-->>VisaService: User
VisaService->>VisaRepository: save(visa status=GRANTED)
VisaRepository->>Database: UPDATE visa
Database-->>VisaRepository: updated visa
VisaService->>AuditService: createAuditLog(handlerId, visaId, GRANTED, "approved")
AuditService->>Database: INSERT audit_log
Database-->>AuditService: saved
VisaService-->>VisaController: VisaDTO
VisaController-->>Admin: 200 OK with VisaDTO
alt Handler missing or not authorized
VisaService->>GlobalExceptionHandler: throw UnauthorizedException
GlobalExceptionHandler-->>Admin: 403 Forbidden
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 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: 12
🧹 Nitpick comments (7)
src/main/java/org/example/visacasemanagementsystem/user/service/UserService.java (1)
31-33: Remove or implement the commented method to keep service code clean.At Lines 31-33, keeping commented-out API methods adds noise and uncertainty.
🤖 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 31 - 33, The commented-out service method findByEmail in UserService is dead code; either delete the commented block to keep the class clean or restore it by implementing a public Optional<User> findByEmail(String email) that delegates to userRepository.findByEmail(email) (and add any necessary null checks or logging). Update method signature and javadoc as appropriate and remove the commented lines after implementing to avoid duplicate/noisy code.pom.xml (1)
146-151: Delete the commented-out S3 dependency block.The codebase contains no active references to S3 or AWS Spring Cloud classes/configuration, confirming this dependency removal is safe. The commented block at lines 146-151 should be deleted entirely rather than left as dead code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pom.xml` around lines 146 - 151, Remove the dead commented-out Maven dependency block for the S3 starter (the commented <dependency> section containing <groupId>io.awspring.cloud</groupId>, <artifactId>spring-cloud-aws-starter-s3</artifactId>, <version>4.0.0</version>) from pom.xml so the file no longer contains commented AWS S3 dependency artifacts; simply delete those commented lines entirely.src/main/java/org/example/visacasemanagementsystem/comment/mapper/CommentMapper.java (1)
24-30: TODO: Service layer must complete entity initialization.The
toEntitymethod only setstext, leavingvisaandauthorasnull. Since both columns arenullable = false, the service layer must set these before persisting to avoid constraint violations.Would you like me to open an issue to track the implementation of the service layer logic for setting
visaandauthor?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/visacasemanagementsystem/comment/mapper/CommentMapper.java` around lines 24 - 30, The toEntity method in CommentMapper (method: toEntity(CreateCommentDTO)) only sets Comment.setText(...) leaving Comment.visa and Comment.author null; update the service layer (e.g., wherever comments are created) to fully initialize the entity before persisting by resolving and setting the associated Visa and Author entities on the Comment (call Comment.setVisa(resolvedVisa) and Comment.setAuthor(resolvedAuthor)) using IDs or objects from CreateCommentDTO or related inputs, and ensure null checks and appropriate exceptions if those related entities cannot be found to prevent DB constraint violations.src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java (1)
179-196: Minor inconsistency: ReturnvisaMapper.toDTO(savedVisa)for consistency.Other methods return
visaMapper.toDTO(savedVisa)after saving, butassignHandlerreturnsvisaMapper.toDTO(visa). While functionally equivalent (the reference is the same aftersave()), usingsavedVisawould be consistent with the pattern inupdateVisaStatus,approveVisa, andrejectVisa.Proposed fix
visa.setHandler(admin); // Connects handler - visaRepository.save(visa); + Visa savedVisa = visaRepository.save(visa); // Create log auditService.createAuditLog( adminId, visaId, AuditEventType.ASSIGNED, "Admin has been assigned to case." ); - return visaMapper.toDTO(visa); + return visaMapper.toDTO(savedVisa); }🤖 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 179 - 196, In VisaService.assignHandler, change the save/return pattern to mirror other handlers: capture the result of visaRepository.save(visa) into a variable (e.g., savedVisa) and return visaMapper.toDTO(savedVisa) instead of visaMapper.toDTO(visa); this keeps consistency with updateVisaStatus/approveVisa/rejectVisa and ensures the saved entity is the one converted to DTO.src/main/java/org/example/visacasemanagementsystem/visa/dto/CreateVisaDTO.java (1)
5-5: Unused import:VisaStatus.The
VisaStatusimport on line 5 is not used in this file and should be removed.🤖 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 5, Remove the unused import of VisaStatus from CreateVisaDTO: delete the line importing org.example.visacasemanagementsystem.visa.VisaStatus in the CreateVisaDTO source so the class no longer contains an unused import (then rebuild/compile to ensure no other references to VisaStatus remain in CreateVisaDTO).src/main/java/org/example/visacasemanagementsystem/visa/mapper/VisaMapper.java (1)
31-33: Acknowledged: TODO about mapper needing UserRepository.The TODO is accurate—
toEntitydoesn't set the applicant because the mapper intentionally delegates that responsibility toVisaService.applyForVisa(), which fetches and sets the applicant. This design decision keeps the mapper stateless.Would you like me to help document this architectural decision or open an issue to track whether this pattern should be formalized?
🤖 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/mapper/VisaMapper.java` around lines 31 - 33, The VisaMapper currently leaves applicant unset which causes constraint violations; modify VisaMapper to take and use a UserRepository dependency (inject via constructor or `@Autowired`) and in the toEntity(...) method fetch the User (e.g., userRepository.findById(dto.getApplicantId()) or similar) and set it on the Visa entity (visa.setApplicant(user)) before returning; keep behavior consistent with or remove the delegation in VisaService.applyForVisa() so only one place sets the applicant.src/main/java/org/example/visacasemanagementsystem/audit/mapper/AuditMapper.java (1)
29-38: Redundant manual timestamp setting conflicts with@CreatedDate.The
AuditLogentity has@CreatedDateon thetimeStampfield (seeAuditLog.javalines 27-29), which automatically populates the timestamp when the entity is persisted. Manually setting it here is redundant and creates ambiguity about the source of truth.Remove the manual timestamp assignment and let JPA auditing handle it consistently.
Proposed fix
public AuditLog toEntity(Long userId, Long visaCaseId, AuditEventType auditEventType, String description){ AuditLog auditLog = new AuditLog(); auditLog.setUserId(userId); auditLog.setVisaCaseId(visaCaseId); auditLog.setAuditEventType(auditEventType); auditLog.setDescription(description); - auditLog.setTimeStamp(LocalDateTime.now()); return auditLog; - }🤖 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/mapper/AuditMapper.java` around lines 29 - 38, In AuditMapper.toEntity remove the manual timestamp assignment auditLog.setTimeStamp(LocalDateTime.now()) so the AuditLog.timeStamp field (annotated with `@CreatedDate` in AuditLog) is left unset and can be populated by JPA auditing when the entity is persisted; ensure you only construct and return the AuditLog in the toEntity method and do not add any other timestamp logic there.
🤖 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/entity/User.java`:
- Line 56: The User.toString() currently includes the email field which can leak
PII; update the User class's toString() method to stop exposing raw email
(either remove the email concatenation entirely or replace it with a
masked/placeholder value such as "[REDACTED]" or only include a non-identifying
part like the domain). Locate the toString() implementation in class User and
modify the string building that references the email variable so logs no longer
contain the full email address.
In
`@src/main/java/org/example/visacasemanagementsystem/user/service/UserService.java`:
- Around line 23-29: The findAll and findById methods in UserService currently
return entity objects (User) from userRepository which can leak sensitive fields
like password; update UserService.findAll and UserService.findById to map
returned User entities to a DTO (e.g., UserDto) using the existing UserMapper
(or add one if missing) before returning—for findAll map the List<User> to
List<UserDto>, and for findById map Optional<User> to Optional<UserDto>—ensuring
the mapper excludes sensitive fields such as password.
In
`@src/main/java/org/example/visacasemanagementsystem/visa/dto/CreateVisaDTO.java`:
- Line 9: In CreateVisaDTO change the validation annotation on the visaType
field from `@NotBlank` to `@NotNull`: locate the CreateVisaDTO class and the
VisaType visaType declaration and replace the `@NotBlank`(message = "Visa type
must be specified") with `@NotNull`(message = "Visa type must be specified") so
the enum is validated correctly (matching the Visa entity) and avoids nulls
slipping through to VisaService.applyForVisa().
In
`@src/main/java/org/example/visacasemanagementsystem/visa/dto/UpdateVisaDTO.java`:
- Line 10: Replace the invalid `@NotBlank` on the VisaType field in the
UpdateVisaDTO class with `@NotNull`: locate the visaType field declaration
(VisaType visaType) in UpdateVisaDTO and change its annotation to `@NotNull`, then
remove the now-unused `@NotBlank` import if it isn't used elsewhere in the file;
ensure the import for javax/ jakarta.validation.constraints.NotNull (or your
project's validation package) is present.
In
`@src/main/java/org/example/visacasemanagementsystem/visa/repository/VisaRepository.java`:
- Line 25: The repository method signature findByHandlerIdAndVisaStatus is
invalid because Visa.handler is a `@ManyToOne` association; rename it to
findByHandler_IdAndVisaStatus in the VisaRepository interface so Spring Data JPA
will traverse the handler entity's id property (update the method declaration in
VisaRepository to use findByHandler_IdAndVisaStatus with the same parameters and
return type).
In
`@src/main/java/org/example/visacasemanagementsystem/visa/service/UnauthorizedException.java`:
- Around line 3-6: The UnauthorizedException constructor currently discards the
message; update the constructor in class UnauthorizedException to pass the
incoming String s to the superclass by calling super(s) so that getMessage()
returns the provided text (this will fix the message lost when
VisaService.validateHandler() throws new UnauthorizedException("User is not
authorized to perform this action.")).
In
`@src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java`:
- Around line 59-70: The method VisaService.findVisaByStatus uses
VisaStatus.valueOf(visaStatus.toUpperCase()) which will throw
IllegalArgumentException for invalid input; update this method to validate the
input or catch IllegalArgumentException and throw a descriptive domain exception
(e.g., InvalidVisaStatusException) with a clear message including the invalid
value, or return an appropriate empty result/optional depending on your API
contract; keep the rest of the flow (calling visaRepository.findByVisaStatus and
mapping with visaMapper::toDTO) unchanged and ensure unit tests cover both valid
and invalid visaStatus inputs.
- Around line 115-136: The applyForVisa method has an unused parameter reason;
update the implementation so the audit message includes that reason instead of
the hardcoded string or remove the parameter from the method signature and all
callers. Specifically, modify applyForVisa(CreateVisaDTO dto, Long userId,
String reason) to pass a composed message (e.g., include reason) to
auditService.createAuditLog(...) or remove the reason parameter and adjust
usages; check the method applyForVisa, the auditService.createAuditLog
invocation, and any callers to ensure consistency.
In `@src/main/resources/application.properties`:
- Line 8: Remove or disable the Hidden HTTP Method filter by setting the
spring.mvc.hiddenmethod.filter.enabled property to false (or deleting the
property) so the HiddenHttpMethodFilter is not active; update the
application.properties entry for spring.mvc.hiddenmethod.filter.enabled
accordingly to false and ensure no code relies on _method form tunneling or
HiddenHttpMethodFilter before committing.
- Around line 11-12: Disable SQL logging by removing or setting
spring.jpa.show-sql and spring.jpa.properties.hibernate.format_sql to false in
the default application.properties and move enabling them into a dev/local
profile (e.g., application-local.properties or an active spring.profiles
configuration) so that SQL statement logging is only turned on for local
debugging; specifically update the entries for spring.jpa.show-sql and
spring.jpa.properties.hibernate.format_sql accordingly and document that they
should be enabled only under the dev/local profile.
- Around line 5-6: The properties file exposes hardcoded defaults for database
credentials (spring.datasource.username and spring.datasource.password and the
localhost URL on the preceding line); remove the insecure defaults and implement
Spring profiles by: keep development-friendly defaults in the base
application.properties, create an application-prod.properties that replaces
defaults with required placeholders (e.g. use ${DB_USER} ${DB_PASSWORD}
${DB_URL} without fallback defaults) so the app fails fast if env vars are
missing, ensure production uses the prod profile (spring.profiles.active=prod or
external config) and add brief documentation listing the required environment
variables (DB_USER, DB_PASSWORD, DB_URL) for production deployment.
- Line 10: Replace the global runtime schema mutation setting by removing or
changing spring.jpa.hibernate.ddl-auto=update from the common configuration and
instead set environment-specific values: put
spring.jpa.hibernate.ddl-auto=update into the local profile properties (e.g.,
application-local.properties) and set spring.jpa.hibernate.ddl-auto=validate or
none in the default/production profile; also ensure Flyway is wired up (enable
Flyway migrations, add migration files under db/migration, and configure
spring.flyway.* as needed) so schema changes are managed via migrations rather
than runtime auto-update.
---
Nitpick comments:
In `@pom.xml`:
- Around line 146-151: Remove the dead commented-out Maven dependency block for
the S3 starter (the commented <dependency> section containing
<groupId>io.awspring.cloud</groupId>,
<artifactId>spring-cloud-aws-starter-s3</artifactId>, <version>4.0.0</version>)
from pom.xml so the file no longer contains commented AWS S3 dependency
artifacts; simply delete those commented lines entirely.
In
`@src/main/java/org/example/visacasemanagementsystem/audit/mapper/AuditMapper.java`:
- Around line 29-38: In AuditMapper.toEntity remove the manual timestamp
assignment auditLog.setTimeStamp(LocalDateTime.now()) so the AuditLog.timeStamp
field (annotated with `@CreatedDate` in AuditLog) is left unset and can be
populated by JPA auditing when the entity is persisted; ensure you only
construct and return the AuditLog in the toEntity method and do not add any
other timestamp logic there.
In
`@src/main/java/org/example/visacasemanagementsystem/comment/mapper/CommentMapper.java`:
- Around line 24-30: The toEntity method in CommentMapper (method:
toEntity(CreateCommentDTO)) only sets Comment.setText(...) leaving Comment.visa
and Comment.author null; update the service layer (e.g., wherever comments are
created) to fully initialize the entity before persisting by resolving and
setting the associated Visa and Author entities on the Comment (call
Comment.setVisa(resolvedVisa) and Comment.setAuthor(resolvedAuthor)) using IDs
or objects from CreateCommentDTO or related inputs, and ensure null checks and
appropriate exceptions if those related entities cannot be found to prevent DB
constraint violations.
In
`@src/main/java/org/example/visacasemanagementsystem/user/service/UserService.java`:
- Around line 31-33: The commented-out service method findByEmail in UserService
is dead code; either delete the commented block to keep the class clean or
restore it by implementing a public Optional<User> findByEmail(String email)
that delegates to userRepository.findByEmail(email) (and add any necessary null
checks or logging). Update method signature and javadoc as appropriate and
remove the commented lines after implementing to avoid duplicate/noisy code.
In
`@src/main/java/org/example/visacasemanagementsystem/visa/dto/CreateVisaDTO.java`:
- Line 5: Remove the unused import of VisaStatus from CreateVisaDTO: delete the
line importing org.example.visacasemanagementsystem.visa.VisaStatus in the
CreateVisaDTO source so the class no longer contains an unused import (then
rebuild/compile to ensure no other references to VisaStatus remain in
CreateVisaDTO).
In
`@src/main/java/org/example/visacasemanagementsystem/visa/mapper/VisaMapper.java`:
- Around line 31-33: The VisaMapper currently leaves applicant unset which
causes constraint violations; modify VisaMapper to take and use a UserRepository
dependency (inject via constructor or `@Autowired`) and in the toEntity(...)
method fetch the User (e.g., userRepository.findById(dto.getApplicantId()) or
similar) and set it on the Visa entity (visa.setApplicant(user)) before
returning; keep behavior consistent with or remove the delegation in
VisaService.applyForVisa() so only one place sets the applicant.
In
`@src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java`:
- Around line 179-196: In VisaService.assignHandler, change the save/return
pattern to mirror other handlers: capture the result of
visaRepository.save(visa) into a variable (e.g., savedVisa) and return
visaMapper.toDTO(savedVisa) instead of visaMapper.toDTO(visa); this keeps
consistency with updateVisaStatus/approveVisa/rejectVisa and ensures the saved
entity is the one converted to DTO.
🪄 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: 5c44228d-3963-433b-9e51-aeee291df42d
📒 Files selected for processing (31)
pom.xmlsrc/main/java/org/example/visacasemanagementsystem/audit/AuditEventType.javasrc/main/java/org/example/visacasemanagementsystem/audit/dto/AuditDTO.javasrc/main/java/org/example/visacasemanagementsystem/audit/entity/AuditLog.javasrc/main/java/org/example/visacasemanagementsystem/audit/mapper/AuditMapper.javasrc/main/java/org/example/visacasemanagementsystem/audit/repository/AuditRepository.javasrc/main/java/org/example/visacasemanagementsystem/audit/service/AuditService.javasrc/main/java/org/example/visacasemanagementsystem/comment/dto/CommentDTO.javasrc/main/java/org/example/visacasemanagementsystem/comment/dto/CreateCommentDTO.javasrc/main/java/org/example/visacasemanagementsystem/comment/entity/Comment.javasrc/main/java/org/example/visacasemanagementsystem/comment/mapper/CommentMapper.javasrc/main/java/org/example/visacasemanagementsystem/log/dto/LogDTO.javasrc/main/java/org/example/visacasemanagementsystem/log/mapper/LogMapper.javasrc/main/java/org/example/visacasemanagementsystem/user/dto/CreateUserDTO.javasrc/main/java/org/example/visacasemanagementsystem/user/dto/UpdateUserDTO.javasrc/main/java/org/example/visacasemanagementsystem/user/dto/UserDTO.javasrc/main/java/org/example/visacasemanagementsystem/user/entity/User.javasrc/main/java/org/example/visacasemanagementsystem/user/mapper/UserMapper.javasrc/main/java/org/example/visacasemanagementsystem/user/repository/UserRepository.javasrc/main/java/org/example/visacasemanagementsystem/user/service/UserService.javasrc/main/java/org/example/visacasemanagementsystem/visa/VisaStatus.javasrc/main/java/org/example/visacasemanagementsystem/visa/VisaType.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/UnauthorizedException.javasrc/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.javasrc/main/resources/application.properties
💤 Files with no reviewable changes (6)
- src/main/java/org/example/visacasemanagementsystem/user/dto/UserDTO.java
- src/main/java/org/example/visacasemanagementsystem/user/dto/UpdateUserDTO.java
- src/main/java/org/example/visacasemanagementsystem/user/dto/CreateUserDTO.java
- src/main/java/org/example/visacasemanagementsystem/log/dto/LogDTO.java
- src/main/java/org/example/visacasemanagementsystem/log/mapper/LogMapper.java
- src/main/java/org/example/visacasemanagementsystem/user/mapper/UserMapper.java
src/main/java/org/example/visacasemanagementsystem/user/entity/User.java
Outdated
Show resolved
Hide resolved
src/main/java/org/example/visacasemanagementsystem/user/service/UserService.java
Outdated
Show resolved
Hide resolved
src/main/java/org/example/visacasemanagementsystem/visa/dto/CreateVisaDTO.java
Outdated
Show resolved
Hide resolved
| public record UpdateVisaDTO( | ||
| @NotNull Long id, | ||
| @NotBlank(message = "Visa type must be specified") String visaType, | ||
| @NotBlank(message = "Visa type must be specified") VisaType visaType, |
There was a problem hiding this comment.
@NotBlank is incompatible with enum types.
@NotBlank only applies to CharSequence types (e.g., String). Since visaType is now a VisaType enum, this constraint will either fail at runtime or be silently ignored by Hibernate Validator. Use @NotNull instead.
🐛 Fix: Replace `@NotBlank` with `@NotNull`
- `@NotBlank`(message = "Visa type must be specified") VisaType visaType,
+ `@NotNull`(message = "Visa type must be specified") VisaType visaType,After applying, you can also remove the unused @NotBlank import if it's no longer needed elsewhere in this file.
📝 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
| @NotBlank(message = "Visa type must be specified") VisaType visaType, | |
| `@NotNull`(message = "Visa type must be specified") VisaType visaType, |
🤖 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/UpdateVisaDTO.java`
at line 10, Replace the invalid `@NotBlank` on the VisaType field in the
UpdateVisaDTO class with `@NotNull`: locate the visaType field declaration
(VisaType visaType) in UpdateVisaDTO and change its annotation to `@NotNull`, then
remove the now-unused `@NotBlank` import if it isn't used elsewhere in the file;
ensure the import for javax/ jakarta.validation.constraints.NotNull (or your
project's validation package) is present.
src/main/java/org/example/visacasemanagementsystem/visa/repository/VisaRepository.java
Outdated
Show resolved
Hide resolved
src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java
Show resolved
Hide resolved
Comment on lines
+5
to
+6
| spring.datasource.username=${DB_USER:user} | ||
| spring.datasource.password=${DB_PASSWORD:secret} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the file exists and read it
if [ -f "src/main/resources/application.properties" ]; then
echo "=== File found ==="
wc -l src/main/resources/application.properties
echo ""
echo "=== Full contents ==="
cat -n src/main/resources/application.properties
else
echo "File not found at src/main/resources/application.properties"
fiRepository: ithsjava25/visa-case-management-system
Length of output: 761
🏁 Script executed:
#!/bin/bash
# Check for Spring profile-specific configuration files
echo "=== Checking for Spring profile configs ==="
find src/main/resources -name "application*.properties" -type f | sortRepository: ithsjava25/visa-case-management-system
Length of output: 167
🏁 Script executed:
#!/bin/bash
# Check for any documentation about environment configuration
echo "=== Searching for config-related documentation ==="
find . -maxdepth 2 -type f \( -name "README*" -o -name "*.md" \) | head -20Repository: ithsjava25/visa-case-management-system
Length of output: 132
🏁 Script executed:
#!/bin/bash
# Look for Docker and deployment configs
echo "=== Docker Compose file ==="
find . -maxdepth 2 -name "docker-compose*" -o -name "Dockerfile*" | xargs cat 2>/dev/null || echo "Not found in expected locations"
echo ""
echo "=== Check pom.xml for Spring profile management ==="
if [ -f "pom.xml" ]; then
grep -A 10 -B 2 "profile\|resource" pom.xml | head -40
else
echo "pom.xml not found"
fi
echo ""
echo "=== Look for .env or environment files ==="
find . -maxdepth 2 -name ".env*" -o -name "*.env" | head -10Repository: ithsjava25/visa-case-management-system
Length of output: 206
🏁 Script executed:
#!/bin/bash
# Check for any production or deployment documentation
echo "=== Searching for deployment/config docs ==="
find . -maxdepth 3 -type f \( -name "*.md" -o -name "*.yml" -o -name "*.yaml" \) | grep -i -E "(deploy|config|prod|env)" | head -20Repository: ithsjava25/visa-case-management-system
Length of output: 126
Remove default credentials and use Spring profiles for environment-specific configuration.
Using user/secret defaults on lines 5-6 is a security risk if environment variables are missing in deployed environments. Additionally, line 4 has the same issue with a hardcoded localhost database URL.
Removing these defaults without a configuration strategy would break local development. Instead, implement environment-specific profiles:
- Keep development-friendly defaults in the base
application.properties - Create
application-prod.propertieswith required (no-default) placeholder variables that force environment variable configuration - Document which environment variables are required for production deployment
Alternatively, if this is intentionally a development-only file, mark it accordingly and ensure production uses a separate configuration mechanism.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/application.properties` around lines 5 - 6, The properties
file exposes hardcoded defaults for database credentials
(spring.datasource.username and spring.datasource.password and the localhost URL
on the preceding line); remove the insecure defaults and implement Spring
profiles by: keep development-friendly defaults in the base
application.properties, create an application-prod.properties that replaces
defaults with required placeholders (e.g. use ${DB_USER} ${DB_PASSWORD}
${DB_URL} without fallback defaults) so the app fails fast if env vars are
missing, ensure production uses the prod profile (spring.profiles.active=prod or
external config) and add brief documentation listing the required environment
variables (DB_USER, DB_PASSWORD, DB_URL) for production deployment.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java (1)
52-57: Redundant sort: sorting byvisaTypewhen already filtering by it.All results from
findByVisaTypewill have the samevisaTypevalue, makingSort.by("visaType")ineffective. Consider sorting by a more meaningful field likecreatedAtoridfor consistent ordering.Proposed fix
public List<VisaDTO> findVisaByType(VisaType visaType) { - return visaRepository.findByVisaType(visaType, Sort.by("visaType").descending()) + return visaRepository.findByVisaType(visaType, Sort.by("createdAt").descending()) .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 52 - 57, The current findVisaByType method in VisaService uses a redundant Sort.by("visaType").descending() when calling visaRepository.findByVisaType; replace that sort with a meaningful ordering such as Sort.by("createdAt").descending() (or Sort.by("id").descending()) so results are consistently ordered; update the call in findVisaByType and keep mapping via visaMapper::toDTO unchanged.
🤖 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 42-45: The findAll() method currently returns List<Visa>
(entities) which is inconsistent with other query methods that return
List<VisaDTO>; change the method signature to return List<VisaDTO> and map the
results of visaRepository.findAll() to DTOs using the same mapping logic used by
other methods (e.g., the mapper or conversion helper used in
findVisaByType/findVisaByStatus), ensuring lazy associations are not exposed and
callers receive DTOs.
- Around line 206-219: The UnauthorizedException thrown by validateHandler
currently maps to HTTP 500 because it extends RuntimeException without mapping;
update the UnauthorizedException class to be annotated with
`@ResponseStatus`(HttpStatus.FORBIDDEN) so Spring returns 403, or alternatively
add a global `@ControllerAdvice` that catches UnauthorizedException and returns
ResponseEntity.status(HttpStatus.FORBIDDEN). Modify the UnauthorizedException
class (or create a new exception handler) and ensure validateHandler(Long
handlerId) continues throwing UnauthorizedException so the new mapping is
applied.
---
Nitpick comments:
In
`@src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java`:
- Around line 52-57: The current findVisaByType method in VisaService uses a
redundant Sort.by("visaType").descending() when calling
visaRepository.findByVisaType; replace that sort with a meaningful ordering such
as Sort.by("createdAt").descending() (or Sort.by("id").descending()) so results
are consistently ordered; update the call in findVisaByType and keep mapping via
visaMapper::toDTO 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: b1238107-4263-49e7-a52a-6c656bae2d47
📒 Files selected for processing (9)
src/main/java/org/example/visacasemanagementsystem/audit/mapper/AuditMapper.javasrc/main/java/org/example/visacasemanagementsystem/user/entity/User.javasrc/main/java/org/example/visacasemanagementsystem/user/service/UserService.javasrc/main/java/org/example/visacasemanagementsystem/visa/dto/CreateVisaDTO.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/UnauthorizedException.javasrc/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.javasrc/main/resources/application.properties
✅ Files skipped from review due to trivial changes (1)
- src/main/java/org/example/visacasemanagementsystem/visa/service/UnauthorizedException.java
🚧 Files skipped from review as they are similar to previous changes (7)
- src/main/resources/application.properties
- src/main/java/org/example/visacasemanagementsystem/visa/mapper/VisaMapper.java
- src/main/java/org/example/visacasemanagementsystem/user/entity/User.java
- src/main/java/org/example/visacasemanagementsystem/audit/mapper/AuditMapper.java
- src/main/java/org/example/visacasemanagementsystem/user/service/UserService.java
- src/main/java/org/example/visacasemanagementsystem/visa/dto/CreateVisaDTO.java
- src/main/java/org/example/visacasemanagementsystem/visa/repository/VisaRepository.java
src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java
Show resolved
Hide resolved
Comment on lines
+206
to
+219
| public User validateHandler(Long handlerId) throws UnauthorizedException { | ||
| // Get user by id | ||
| User user = userRepository.findById(handlerId) | ||
| .orElseThrow(() -> new EntityNotFoundException("User not found.")); | ||
|
|
||
| // Checks Authorization | ||
| boolean isAdmin = user.getUserAuthorization() == UserAuthorization.ADMIN; | ||
| boolean isSysAdmin = user.getUserAuthorization() == UserAuthorization.SYSADMIN; | ||
|
|
||
| if (!isAdmin && !isSysAdmin) { | ||
| throw new UnauthorizedException("User is not authorized to perform this action."); | ||
| } | ||
| return user; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there's a `@ControllerAdvice` that handles UnauthorizedException
rg -n "@ControllerAdvice|@ExceptionHandler.*UnauthorizedException|@ResponseStatus" --type=javaRepository: ithsjava25/visa-case-management-system
Length of output: 64
🏁 Script executed:
# Find the UnauthorizedException class
fd -t f "UnauthorizedException.java"Repository: ithsjava25/visa-case-management-system
Length of output: 172
🏁 Script executed:
# Search for any exception handler or controller advice files
fd -t f -i "exception" --type f | grep -E "\.(java|xml|properties)$"Repository: ithsjava25/visa-case-management-system
Length of output: 172
🏁 Script executed:
# Check for GlobalExceptionHandler or error handling configuration
rg -l "GlobalExceptionHandler|ErrorHandler|ExceptionResolver" --type=javaRepository: ithsjava25/visa-case-management-system
Length of output: 64
🏁 Script executed:
cat -n src/main/java/org/example/visacasemanagementsystem/visa/service/UnauthorizedException.javaRepository: ithsjava25/visa-case-management-system
Length of output: 323
UnauthorizedException` will return HTTP 500 instead of 403 Forbidden.
The UnauthorizedException class extends RuntimeException without @ResponseStatus annotation, and no global exception handler exists in the codebase. Spring will return HTTP 500 Internal Server Error instead of the expected 403 Forbidden when authorization fails.
Add @ResponseStatus(HttpStatus.FORBIDDEN) to the UnauthorizedException class, or implement a global @ControllerAdvice exception handler to map this exception appropriately.
🤖 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 206 - 219, The UnauthorizedException thrown by validateHandler
currently maps to HTTP 500 because it extends RuntimeException without mapping;
update the UnauthorizedException class to be annotated with
`@ResponseStatus`(HttpStatus.FORBIDDEN) so Spring returns 403, or alternatively
add a global `@ControllerAdvice` that catches UnauthorizedException and returns
ResponseEntity.status(HttpStatus.FORBIDDEN). Modify the UnauthorizedException
class (or create a new exception handler) and ensure validateHandler(Long
handlerId) continues throwing UnauthorizedException so the new mapping is
applied.
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)
169-187: Consider validating state and rejection reason.Similar to
approveVisa, this doesn't validate the current visa state. Additionally, thereasonparameter should be validated to ensure meaningful audit trails.♻️ Proposed validation
public VisaDTO rejectVisa(Long visaId, Long adminId, String reason) { + if (reason == null || reason.isBlank()) { + throw new IllegalArgumentException("Rejection reason is required"); + } validateHandler(adminId); Visa visa = findVisaById(visaId); + if (visa.getVisaStatus() != VisaStatus.SUBMITTED) { + throw new IllegalStateException("Visa cannot be rejected from status: " + visa.getVisaStatus()); + } + visa.setVisaStatus(VisaStatus.REJECTED);🤖 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 169 - 187, The rejectVisa flow currently skips state checks and accepts any rejection reason; update rejectVisa to first validate the current visa state (use findVisaById() result and allow rejection only from the same prior states approveVisa allows, e.g., PENDING) and throw an IllegalStateException (or existing domain exception) if not allowed, then validate the reason parameter (non-null, not blank, enforce a sensible max length) before calling visa.setVisaStatus(VisaStatus.REJECTED) and visa.setRejectionReason(...); preserve the audit via auditService.createAuditLog(adminId, visaId, AuditEventType.REJECTED, "Visa rejected. Reason: " + reason) after validation and save with visaRepository.save(...) and return visaMapper.toDTO(...).
149-166: Consider validating visa state before approval.The method doesn't check if the visa is in a valid state (e.g.,
SUBMITTED) before granting. This allows approving an alreadyREJECTEDorGRANTEDvisa, which may lead to inconsistent state transitions.♻️ Proposed state validation
public VisaDTO approveVisa(Long visaId, Long adminId) { validateHandler(adminId); Visa visa = findVisaById(visaId); + if (visa.getVisaStatus() != VisaStatus.SUBMITTED) { + throw new IllegalStateException("Visa cannot be approved from status: " + visa.getVisaStatus()); + } + visa.setVisaStatus(VisaStatus.GRANTED);🤖 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 149 - 166, The approveVisa method currently sets VisaStatus.GRANTED unconditionally; update it to validate the current visa state (use findVisaById to get the Visa and inspect visa.getVisaStatus()) and only allow transition from an expected state (e.g., VisaStatus.SUBMITTED); if the current state is not permitted (e.g., already GRANTED or REJECTED) throw a suitable exception (IllegalStateException or a domain-specific exception) before calling visaRepository.save, auditService.createAuditLog, and visaMapper.toDTO so that saving and audit logging only occur for valid transitions.src/main/java/org/example/visacasemanagementsystem/exception/GlobalExceptionHandler.java (1)
10-25: Consider adding a handler forIllegalArgumentExceptionand using structured error responses.The
findVisaByStatusmethod inVisaServicethrowsIllegalArgumentExceptionfor invalid status values, but there's no handler here for it—resulting in HTTP 500. Additionally, returning plain strings is inconsistent with REST API conventions; a structured JSON response (with fields likeerror,message,timestamp) would improve client-side error handling.♻️ Proposed enhancement
`@ControllerAdvice` public class GlobalExceptionHandler { + `@ResponseStatus`(value = HttpStatus.BAD_REQUEST) + `@ExceptionHandler`(IllegalArgumentException.class) + `@ResponseBody` + public String handleIllegalArgumentException(IllegalArgumentException exception) { + return "Bad Request: " + exception.getMessage(); + } + `@ResponseStatus`(value = HttpStatus.FORBIDDEN) `@ExceptionHandler`(UnauthorizedException.class) `@ResponseBody`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/visacasemanagementsystem/exception/GlobalExceptionHandler.java` around lines 10 - 25, Update GlobalExceptionHandler to return a structured error payload instead of plain strings and add a handler for IllegalArgumentException: create a simple ErrorResponse DTO (fields: error, message, timestamp) and adjust the existing handlers (handleUnauthorizedException, handleNotFoundException) to build and return that ErrorResponse (preferably via ResponseEntity) with the appropriate HttpStatus; add a new handler method annotated with `@ExceptionHandler`(IllegalArgumentException.class) (e.g., handleBadRequestException) that returns an ErrorResponse with HttpStatus.BAD_REQUEST and uses exception.getMessage() for the message field.
🤖 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 63-70: The method findVisaByStatus currently calls
visaStatus.toUpperCase() which throws a NullPointerException if visaStatus is
null; before calling toUpperCase or VisaStatus.valueOf, validate that the
parameter is not null (e.g., check visaStatus == null and throw new
IllegalArgumentException("Invalid visa status: null") or similar) so the method
consistently throws an IllegalArgumentException for invalid input; update the
null check in findVisaByStatus and keep the existing try-catch around
VisaStatus.valueOf to handle unknown enum names.
---
Nitpick comments:
In
`@src/main/java/org/example/visacasemanagementsystem/exception/GlobalExceptionHandler.java`:
- Around line 10-25: Update GlobalExceptionHandler to return a structured error
payload instead of plain strings and add a handler for IllegalArgumentException:
create a simple ErrorResponse DTO (fields: error, message, timestamp) and adjust
the existing handlers (handleUnauthorizedException, handleNotFoundException) to
build and return that ErrorResponse (preferably via ResponseEntity) with the
appropriate HttpStatus; add a new handler method annotated with
`@ExceptionHandler`(IllegalArgumentException.class) (e.g.,
handleBadRequestException) that returns an ErrorResponse with
HttpStatus.BAD_REQUEST and uses exception.getMessage() for the message field.
In
`@src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java`:
- Around line 169-187: The rejectVisa flow currently skips state checks and
accepts any rejection reason; update rejectVisa to first validate the current
visa state (use findVisaById() result and allow rejection only from the same
prior states approveVisa allows, e.g., PENDING) and throw an
IllegalStateException (or existing domain exception) if not allowed, then
validate the reason parameter (non-null, not blank, enforce a sensible max
length) before calling visa.setVisaStatus(VisaStatus.REJECTED) and
visa.setRejectionReason(...); preserve the audit via
auditService.createAuditLog(adminId, visaId, AuditEventType.REJECTED, "Visa
rejected. Reason: " + reason) after validation and save with
visaRepository.save(...) and return visaMapper.toDTO(...).
- Around line 149-166: The approveVisa method currently sets VisaStatus.GRANTED
unconditionally; update it to validate the current visa state (use findVisaById
to get the Visa and inspect visa.getVisaStatus()) and only allow transition from
an expected state (e.g., VisaStatus.SUBMITTED); if the current state is not
permitted (e.g., already GRANTED or REJECTED) throw a suitable exception
(IllegalStateException or a domain-specific exception) before calling
visaRepository.save, auditService.createAuditLog, and visaMapper.toDTO so that
saving and audit logging only occur for valid transitions.
🪄 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: 7742c353-f6eb-4d3e-9afa-93776226f741
📒 Files selected for processing (3)
src/main/java/org/example/visacasemanagementsystem/exception/GlobalExceptionHandler.javasrc/main/java/org/example/visacasemanagementsystem/exception/UnauthorizedException.javasrc/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java
✅ Files skipped from review due to trivial changes (1)
- src/main/java/org/example/visacasemanagementsystem/exception/UnauthorizedException.java
src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java (1)
174-188: Require a non-blank rejection reason for rejected cases.Allowing null/blank
reasonweakens both business traceability and audit log value.Suggested patch
public VisaDTO rejectVisa(Long visaId, Long adminId, String reason) { + if (reason == null || reason.isBlank()) { + throw new IllegalArgumentException("Rejection reason cannot be null or blank"); + } validateHandler(adminId); Visa visa = findVisaById(visaId); visa.setVisaStatus(VisaStatus.REJECTED);🤖 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 174 - 188, In rejectVisa, require a non-blank rejection reason before proceeding: validate that the String reason is not null and not empty/whitespace (e.g., using trim().isEmpty()) and throw an appropriate exception (IllegalArgumentException or a domain-specific ValidationException) if it is invalid; if valid, continue with the existing flow (call validateHandler(adminId), findVisaById(visaId), setVisaStatus to REJECTED, setRejectionReason, save via visaRepository.save, and call auditService.createAuditLog with the reason). Ensure the validation check is placed at the start of the method (before persisting or auditing) and reference the rejectVisa method and the calls to validateHandler, findVisaById, visaRepository.save, and auditService.createAuditLog when making the change.
🤖 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 86-88: Add null checks for date-like filter parameters to avoid
NPEs and return a clear validation error: in findVisaByDateCreated(LocalDate
date) validate that date != null and if null throw an IllegalArgumentException
(or return an appropriate empty response) with a descriptive message before
calling date.atStartOfDay()/atTime(); do the same for the other method that
dereferences "since" (the method handling the since parameter around lines
99-100) so both findVisaByDateCreated and the method using "since" validate
inputs up-front and fail fast with clear messages.
- Around line 109-115: In VisaService.updateVisaStatus, fail fast if the
incoming newStatus is null: before calling findVisaById or setting the entity,
validate the newStatus parameter and throw an IllegalArgumentException (or a
domain-specific exception) with a clear message when newStatus == null; this
prevents calling Visa.setVisaStatus(null) and avoids persisting an invalid state
via visaRepository.save. Ensure the check is placed at the start of
updateVisaStatus (near validateHandler and before findVisaById) so callers get
immediate, clear feedback.
---
Nitpick comments:
In
`@src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java`:
- Around line 174-188: In rejectVisa, require a non-blank rejection reason
before proceeding: validate that the String reason is not null and not
empty/whitespace (e.g., using trim().isEmpty()) and throw an appropriate
exception (IllegalArgumentException or a domain-specific ValidationException) if
it is invalid; if valid, continue with the existing flow (call
validateHandler(adminId), findVisaById(visaId), setVisaStatus to REJECTED,
setRejectionReason, save via visaRepository.save, and call
auditService.createAuditLog with the reason). Ensure the validation check is
placed at the start of the method (before persisting or auditing) and reference
the rejectVisa method and the calls to validateHandler, findVisaById,
visaRepository.save, and auditService.createAuditLog when making the change.
🪄 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: 58569ac3-9036-4d7e-a5cc-9deb75467513
📒 Files selected for processing (1)
src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java
Comment on lines
+86
to
+88
| public List<VisaDTO> findVisaByDateCreated(LocalDate date){ | ||
| LocalDateTime startOfDay = date.atStartOfDay(); | ||
| LocalDateTime endOfDay = date.atTime(LocalTime.MAX); |
There was a problem hiding this comment.
Add null guards for date filters to prevent NPEs.
date/since are dereferenced directly; null input will throw before a meaningful validation error is returned.
Suggested patch
public List<VisaDTO> findVisaByDateCreated(LocalDate date){
+ if (date == null) {
+ throw new IllegalArgumentException("Date cannot be null");
+ }
LocalDateTime startOfDay = date.atStartOfDay();
LocalDateTime endOfDay = date.atTime(LocalTime.MAX);
@@
public List<VisaDTO> findVisaByDateUpdated(LocalDateTime since){
+ if (since == null) {
+ throw new IllegalArgumentException("Updated-since timestamp cannot be null");
+ }
return visaRepository.findByUpdatedAtAfter(since, Sort.by("updatedAt").descending())
.stream()
.map(visaMapper::toDTO)
.toList();
}Also applies to: 99-100
🤖 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 86 - 88, Add null checks for date-like filter parameters to avoid
NPEs and return a clear validation error: in findVisaByDateCreated(LocalDate
date) validate that date != null and if null throw an IllegalArgumentException
(or return an appropriate empty response) with a descriptive message before
calling date.atStartOfDay()/atTime(); do the same for the other method that
dereferences "since" (the method handling the since parameter around lines
99-100) so both findVisaByDateCreated and the method using "since" validate
inputs up-front and fail fast with clear messages.
Comment on lines
+109
to
+115
| public VisaDTO updateVisaStatus(Long visaId, VisaStatus newStatus, Long adminId) { | ||
| validateHandler(adminId); | ||
| Visa visa = findVisaById(visaId); | ||
|
|
||
| // Update visa status | ||
| visa.setVisaStatus(newStatus); | ||
| Visa savedVisa = visaRepository.save(visa); |
There was a problem hiding this comment.
Fail fast when newStatus is null.
updateVisaStatus currently accepts null and writes it into the entity, which can create invalid state or deferred persistence errors.
Suggested patch
public VisaDTO updateVisaStatus(Long visaId, VisaStatus newStatus, Long adminId) {
+ if (newStatus == null) {
+ throw new IllegalArgumentException("New visa status cannot be null");
+ }
validateHandler(adminId);
Visa visa = findVisaById(visaId);
// Update visa status
visa.setVisaStatus(newStatus);📝 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
| public VisaDTO updateVisaStatus(Long visaId, VisaStatus newStatus, Long adminId) { | |
| validateHandler(adminId); | |
| Visa visa = findVisaById(visaId); | |
| // Update visa status | |
| visa.setVisaStatus(newStatus); | |
| Visa savedVisa = visaRepository.save(visa); | |
| public VisaDTO updateVisaStatus(Long visaId, VisaStatus newStatus, Long adminId) { | |
| if (newStatus == null) { | |
| throw new IllegalArgumentException("New visa status cannot be null"); | |
| } | |
| validateHandler(adminId); | |
| Visa visa = findVisaById(visaId); | |
| // Update visa status | |
| visa.setVisaStatus(newStatus); | |
| Visa savedVisa = visaRepository.save(visa); |
🤖 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 109 - 115, In VisaService.updateVisaStatus, fail fast if the
incoming newStatus is null: before calling findVisaById or setting the entity,
validate the newStatus parameter and throw an IllegalArgumentException (or a
domain-specific exception) with a clear message when newStatus == null; this
prevents calling Visa.setVisaStatus(null) and avoids persisting an invalid state
via visaRepository.save. Ensure the check is placed at the start of
updateVisaStatus (near validateHandler and before findVisaById) so callers get
immediate, clear feedback.
eeebbaandersson
requested review from
EdvinSandgren,
Martin-E-Karlsson and
dinbah18
EdvinSandgren
left a comment
EdvinSandgren
left a comment
There was a problem hiding this comment.
Ser bra ut! Riktigt bra jobbat.
Finns vissa saker som CodeRabbit anmärkt på (exempelvis i UpdateVisaDTO bör det vara NotNull istället för NotBlank på VisaType), och det behövs kanske lite mer validering av indata och liknande, men det kan vi lösa senare. Blir tydligare vad som behövs när mer är på plats.
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
2 participants
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
Refactor
Chores