Skip to content

Feature/extend logger for comment and file#50

Merged
eeebbaandersson merged 9 commits into
mainfrom
feature/extend-logger-for-comment-and-file
Apr 24, 2026
Merged

Feature/extend logger for comment and file#50
eeebbaandersson merged 9 commits into
mainfrom
feature/extend-logger-for-comment-and-file

Conversation

@eeebbaandersson

@eeebbaandersson eeebbaandersson commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features

    • Added audit logging for comments (added/updated/deleted) and files (uploaded/deleted/downloaded), recording actor and timestamps.
    • Comment and file actions now persist audit entries and are exposed via services.
  • Tests

    • Added unit tests for audit mappers to ensure correct conversion between entities and DTOs.
  • Documentation

    • Removed outdated inline comments.

@coderabbitai

coderabbitai Bot commented Apr 24, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@eeebbaandersson has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 45 minutes and 35 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 45 minutes and 35 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8abda120-5253-44d2-9266-97e1d3011b13

📥 Commits

Reviewing files that changed from the base of the PR and between 3f76301 and 68d8072.

📒 Files selected for processing (3)
  • src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java
  • src/test/java/org/example/visacasemanagementsystem/audit/mapper/CommentLogMapperTest.java
  • src/test/java/org/example/visacasemanagementsystem/audit/mapper/FileLogMapperTest.java
📝 Walkthrough

Walkthrough

Adds audit logging for comments and files: enums, DTOs, JPA entities, mappers, repositories, and services; integrates CommentLogService into CommentService and FileLogService into VisaService; minor cleanup in VisaLog comments and FileService formatting.

Changes

Cohort / File(s) Summary
Event Enums
src/main/java/org/example/visacasemanagementsystem/audit/CommentEventType.java, src/main/java/org/example/visacasemanagementsystem/audit/FileEventType.java
Adds CommentEventType (ADDED, UPDATED, DELETED) and FileEventType (UPLOADED, DELETED, DOWNLOADED).
Comment Audit DTO / Entity / Mapper / Repo
src/main/java/org/example/visacasemanagementsystem/audit/dto/CommentLogDTO.java, src/main/java/org/example/visacasemanagementsystem/audit/entity/CommentLog.java, src/main/java/org/example/visacasemanagementsystem/audit/mapper/CommentLogMapper.java, src/main/java/org/example/visacasemanagementsystem/audit/repository/CommentLogRepository.java
Adds CommentLog DTO (record), JPA entity with created timestamp, mapper for entity↔DTO, and JpaRepository for persistence.
File Audit DTO / Entity / Mapper / Repo
src/main/java/org/example/visacasemanagementsystem/audit/dto/FileLogDTO.java, src/main/java/org/example/visacasemanagementsystem/audit/entity/FileLog.java, src/main/java/org/example/visacasemanagementsystem/audit/mapper/FileLogMapper.java, src/main/java/org/example/visacasemanagementsystem/audit/repository/FileLogRepository.java
Adds FileLog DTO (record), JPA entity with created timestamp, mapper for entity↔DTO, and JpaRepository for persistence.
Audit Services
src/main/java/org/example/visacasemanagementsystem/audit/service/CommentLogService.java, src/main/java/org/example/visacasemanagementsystem/audit/service/FileLogService.java
Adds services to create log entries and list all logs, wiring mappers and repositories.
Service Integrations
src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java, src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java
Replaces VisaLog usage in CommentService with CommentLogService; injects FileLogService into VisaService and emits file audit events on upload/delete flows.
Tests
src/test/java/org/example/visacasemanagementsystem/audit/mapper/CommentLogMapperTest.java, src/test/java/org/example/visacasemanagementsystem/audit/mapper/FileLogMapperTest.java
Adds unit tests for CommentLogMapper and FileLogMapper verifying toDTO/toEntity behaviors and null handling.
Cleanup / Formatting
src/main/java/org/example/visacasemanagementsystem/audit/entity/VisaLog.java, src/main/java/org/example/visacasemanagementsystem/file/FileService.java
Removes inline Swedish comments from VisaLog and fixes minor brace/blank-line formatting in FileService.deleteFile().

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant CommentService
    participant CommentLogService
    participant CommentLogRepository
    participant CommentLogMapper

    Client->>CommentService: create comment
    CommentService->>CommentLogService: createCommentLog(actorId, visaId, commentId, ADDED, desc)
    CommentLogService->>CommentLogMapper: toEntity(actorId, visaId, commentId, ADDED, desc)
    CommentLogMapper-->>CommentLogService: CommentLog entity
    CommentLogService->>CommentLogRepository: save(CommentLog)
    CommentLogRepository-->>CommentLogService: saved entity
    CommentLogService-->>CommentService: (void)
    CommentService-->>Client: return created comment
Loading
sequenceDiagram
    participant Client
    participant VisaService
    participant FileService
    participant FileLogService
    participant FileLogRepository
    participant FileLogMapper

    Client->>VisaService: applyForVisa/create or add document (s3Key)
    VisaService->>FileService: upload/attach s3Key
    FileService-->>VisaService: s3Key stored
    VisaService->>FileLogService: createFileLog(actorId, visaId, fileName, UPLOADED, desc)
    FileLogService->>FileLogMapper: toEntity(actorId, visaId, fileName, UPLOADED, desc)
    FileLogMapper-->>FileLogService: FileLog entity
    FileLogService->>FileLogRepository: save(FileLog)
    FileLogRepository-->>FileLogService: saved entity
    FileLogService-->>VisaService: (void)
    VisaService-->>Client: return visa
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • EdvinSandgren
  • Martin-E-Karlsson
  • dinbah18

Poem

"🐰 Hop, I logged a little note today,
ADDED, UPLOADED — I watched them play.
Files and comments in tidy rows,
Audit carrots where the data grows.
Hooray for trails that never stray!"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 17.65% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'Feature/extend logger for comment and file' accurately describes the main changes: adding comprehensive audit logging for comments and files with new entities, DTOs, mappers, repositories, and services.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/extend-logger-for-comment-and-file

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (6)
src/main/java/org/example/visacasemanagementsystem/audit/dto/FileLogDTO.java (1)

7-15: Minor: component naming.

timeStamp is usually written as a single word timestamp in Java (matches LocalDateTime, SLF4J, @CreationTimestamp, etc.). Consider renaming for consistency with CommentLogDTO and the entity field — but only if the rename is applied consistently across FileLog, CommentLog, FileLogMapper, and CommentLogMapper to avoid JSON/contract drift.

🤖 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/dto/FileLogDTO.java`
around lines 7 - 15, Rename the FileLogDTO field timeStamp to timestamp to match
Java conventions and the rest of the model, and apply the same rename
consistently across the FileLog entity/class, FileLogMapper, and any related
serialization/JSON contracts to avoid drift; also verify
CommentLogDTO/CommentLogMapper already use timestamp and update
constructors/records, mapping methods, DTO consumers and tests to use the new
field name so compilation and JSON payloads remain correct.
src/main/java/org/example/visacasemanagementsystem/audit/service/FileLogService.java (1)

29-34: Consider pagination for findAll().

Audit logs tend to grow unbounded; returning the full table as a List<FileLogDTO> will get progressively slower and eventually OOM. If this is exposed via a controller, prefer Page<FileLogDTO> findAll(Pageable pageable) (or at minimum a bounded query) before usage grows. Not blocking for this PR.

🤖 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/FileLogService.java`
around lines 29 - 34, The findAll method currently returns an unbounded List and
can OOM; change FileLogService.findAll to a paginated signature (e.g.,
Page<FileLogDTO> findAll(Pageable pageable)), call
fileLogRepository.findAll(pageable) and map results with fileLogMapper::toDTO
via .map(...), and propagate Pageable from the controller/consumer; update
imports and callers to accept Pageable and return Page<FileLogDTO> so queries
are bounded.
src/main/java/org/example/visacasemanagementsystem/audit/service/CommentLogService.java (2)

24-28: Fix parameter name typo: visaCasIdvisaCaseId.

Minor naming slip that propagates into IDE hints and any future caller. The mapper helper on Line 25 has the same typo. Rename both for clarity.

Proposed change
-    public void createCommentLog(Long actorUserId, Long visaCasId, Long commentId, CommentEventType commentEventType, String description) {
-        CommentLog commentLog = commentLogMapper.toEntity(actorUserId, visaCasId, commentId,commentEventType, description);
+    public void createCommentLog(Long actorUserId, Long visaCaseId, Long commentId, CommentEventType commentEventType, String description) {
+        CommentLog commentLog = commentLogMapper.toEntity(actorUserId, visaCaseId, commentId, commentEventType, description);
         commentLogRepository.save(commentLog);
-
     }
🤖 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/CommentLogService.java`
around lines 24 - 28, Rename the misspelled parameter visaCasId to visaCaseId in
the CommentLogService.createCommentLog method signature and update all usages
inside the method (including the call to commentLogMapper.toEntity) to use
visaCaseId; ensure the mapper method invocation and local variable names
(commentLog) match the renamed parameter and update any corresponding parameter
name in the commentLogMapper.toEntity call site if it also uses the typo.

30-35: findAll() on an audit table is an unbounded scan.

Audit tables typically grow without bound, so exposing an unpaginated findAll will become a memory/latency liability once the table is non-trivial. Consider switching to Page<CommentLogDTO> findAll(Pageable pageable) before this is wired into a controller, and adding filter methods by visaCaseId/commentId for the likely UI access patterns.

🤖 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/CommentLogService.java`
around lines 30 - 35, The current CommentLogService.findAll implementation
performs an unbounded repository scan via commentLogRepository.findAll() and
maps to DTOs with commentLogMapper::toDTO; change the service method to support
pagination (e.g., Page<CommentLogDTO> findAll(Pageable pageable)) by calling
commentLogRepository.findAll(pageable) and mapping the Page content to DTOs, and
add repository/service methods to fetch paged/filterable results by visaCaseId
and by commentId (e.g., findByVisaCaseId(Pageable, UUID) /
findByCommentId(Pageable, UUID)) so UI access patterns avoid unbounded
memory/latency issues.
src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java (1)

212-216: Minor: relying on dirty-checking to persist the s3Key add.

Previously the s3Keys collection was populated before save; now you mutate savedVisa.getS3Keys() after the save call and depend on Hibernate's dirty-checking flush at commit to persist the addition. It works under the current @Transactional setup, but it's subtly fragile — any future change to propagation, flush mode, or making this method non-transactional would silently drop the attachment. Consider populating s3Keys on visa before visaRepository.save(visa) (as it was before), which removes the implicit dependency on the enclosing transaction.

🤖 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 212 - 216, The code mutates savedVisa.getS3Keys() after calling
visaRepository.save(visa), relying on Hibernate dirty-checking; move the
addition to the s3Keys collection onto the incoming visa before the save to make
persistence explicit. Concretely, in VisaService update the logic so that if
s3Key is non-null/blank you call visa.getS3Keys().add(s3Key) prior to invoking
visaRepository.save(visa) (then assign the result to savedVisa as before)
instead of mutating savedVisa after save.
src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java (1)

68-75: Consider a more informative audit description.

The fixed string "User added a new comment" drops useful context (e.g., visaId, author id, or a text preview). Since the mapper already captures actorUserId, visaCaseId, and commentId as structured columns, this is minor — but if the description is intended to be human-readable in an audit UI, a slightly richer message is cheap to add.

🤖 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/service/CommentService.java`
around lines 68 - 75, The audit description passed to
commentLogService.createCommentLog is a generic fixed string; update the
description to include contextual details (e.g., author.getId(), visa.getId(),
and a short preview of the comment text) so the human-readable message is more
informative in the audit UI; modify the call that uses CommentEventType.ADDED
(the createCommentLog invocation) to build a concise message combining these
values (truncate the comment text to a safe length) and pass that string as the
description.
🤖 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/audit/service/FileLogService.java`:
- Around line 23-24: The method and parameter name typo `visaCasId` should be
renamed to `visaCaseId` across the public signatures and mapper parameters:
update FileLogService.createFileLog(Long actorUserId, Long visaCasId, ...) to
use visaCaseId, change the corresponding parameter name in
FileLogMapper.toEntity(...) to visaCaseId, and do the same rename in
CommentLogService.createCommentLog(...) and CommentLogMapper.toEntity(...);
ensure you update the parameter names inside each method/body and mapper
implementation to reference visaCaseId (keeping the same types and argument
order) so call sites using positional arguments remain unaffected.

In
`@src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java`:
- Around line 214-224: In applyForVisa, the call to fileLogService.createFileLog
is executed unconditionally which allows a null/blank s3Key to produce a FileLog
with null fileName; move the fileLogService.createFileLog(...) call inside the
existing if (s3Key != null && !s3Key.isBlank()) block (the same block that calls
savedVisa.getS3Keys().add(s3Key)) so the audit entry is only created when a real
s3Key is present, ensuring FileLog.fileName is non-null and the UPLOADED event
only records actual uploads.

---

Nitpick comments:
In
`@src/main/java/org/example/visacasemanagementsystem/audit/dto/FileLogDTO.java`:
- Around line 7-15: Rename the FileLogDTO field timeStamp to timestamp to match
Java conventions and the rest of the model, and apply the same rename
consistently across the FileLog entity/class, FileLogMapper, and any related
serialization/JSON contracts to avoid drift; also verify
CommentLogDTO/CommentLogMapper already use timestamp and update
constructors/records, mapping methods, DTO consumers and tests to use the new
field name so compilation and JSON payloads remain correct.

In
`@src/main/java/org/example/visacasemanagementsystem/audit/service/CommentLogService.java`:
- Around line 24-28: Rename the misspelled parameter visaCasId to visaCaseId in
the CommentLogService.createCommentLog method signature and update all usages
inside the method (including the call to commentLogMapper.toEntity) to use
visaCaseId; ensure the mapper method invocation and local variable names
(commentLog) match the renamed parameter and update any corresponding parameter
name in the commentLogMapper.toEntity call site if it also uses the typo.
- Around line 30-35: The current CommentLogService.findAll implementation
performs an unbounded repository scan via commentLogRepository.findAll() and
maps to DTOs with commentLogMapper::toDTO; change the service method to support
pagination (e.g., Page<CommentLogDTO> findAll(Pageable pageable)) by calling
commentLogRepository.findAll(pageable) and mapping the Page content to DTOs, and
add repository/service methods to fetch paged/filterable results by visaCaseId
and by commentId (e.g., findByVisaCaseId(Pageable, UUID) /
findByCommentId(Pageable, UUID)) so UI access patterns avoid unbounded
memory/latency issues.

In
`@src/main/java/org/example/visacasemanagementsystem/audit/service/FileLogService.java`:
- Around line 29-34: The findAll method currently returns an unbounded List and
can OOM; change FileLogService.findAll to a paginated signature (e.g.,
Page<FileLogDTO> findAll(Pageable pageable)), call
fileLogRepository.findAll(pageable) and map results with fileLogMapper::toDTO
via .map(...), and propagate Pageable from the controller/consumer; update
imports and callers to accept Pageable and return Page<FileLogDTO> so queries
are bounded.

In
`@src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java`:
- Around line 68-75: The audit description passed to
commentLogService.createCommentLog is a generic fixed string; update the
description to include contextual details (e.g., author.getId(), visa.getId(),
and a short preview of the comment text) so the human-readable message is more
informative in the audit UI; modify the call that uses CommentEventType.ADDED
(the createCommentLog invocation) to build a concise message combining these
values (truncate the comment text to a safe length) and pass that string as the
description.

In
`@src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java`:
- Around line 212-216: The code mutates savedVisa.getS3Keys() after calling
visaRepository.save(visa), relying on Hibernate dirty-checking; move the
addition to the s3Keys collection onto the incoming visa before the save to make
persistence explicit. Concretely, in VisaService update the logic so that if
s3Key is non-null/blank you call visa.getS3Keys().add(s3Key) prior to invoking
visaRepository.save(visa) (then assign the result to savedVisa as before)
instead of mutating savedVisa after save.
🪄 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: eef926f0-f95b-4f24-bb40-35b9356ce0bc

📥 Commits

Reviewing files that changed from the base of the PR and between a66d5ef and 403f440.

📒 Files selected for processing (16)
  • src/main/java/org/example/visacasemanagementsystem/audit/CommentEventType.java
  • src/main/java/org/example/visacasemanagementsystem/audit/FileEventType.java
  • src/main/java/org/example/visacasemanagementsystem/audit/dto/CommentLogDTO.java
  • src/main/java/org/example/visacasemanagementsystem/audit/dto/FileLogDTO.java
  • src/main/java/org/example/visacasemanagementsystem/audit/entity/CommentLog.java
  • src/main/java/org/example/visacasemanagementsystem/audit/entity/FileLog.java
  • src/main/java/org/example/visacasemanagementsystem/audit/entity/VisaLog.java
  • src/main/java/org/example/visacasemanagementsystem/audit/mapper/CommentLogMapper.java
  • src/main/java/org/example/visacasemanagementsystem/audit/mapper/FileLogMapper.java
  • src/main/java/org/example/visacasemanagementsystem/audit/repository/CommentLogRepository.java
  • src/main/java/org/example/visacasemanagementsystem/audit/repository/FileLogRepository.java
  • src/main/java/org/example/visacasemanagementsystem/audit/service/CommentLogService.java
  • src/main/java/org/example/visacasemanagementsystem/audit/service/FileLogService.java
  • src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java
  • src/main/java/org/example/visacasemanagementsystem/file/FileService.java
  • src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java

Comment on lines +23 to +24
public void createFileLog(Long actorUserId, Long visaCasId, String fileName, FileEventType fileEventType, String description) {
FileLog fileLog = fileLogMapper.toEntity(actorUserId, visaCasId, fileName, fileEventType, description);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find all call sites and the mapper signature that need the same rename.
rg -nP '\bvisaCasId\b' --type=java -C2
rg -nP '\bcreateFileLog\s*\(' --type=java -C2

Repository: ithsjava25/visa-case-management-system

Length of output: 4281


Fix typo in parameter names: visaCasIdvisaCaseId.

This typo appears in public method signatures and should be corrected before release:

  • FileLogService.createFileLog() (line 23)
  • FileLogMapper.toEntity() (parameter)
  • CommentLogService.createCommentLog() (line 24)
  • CommentLogMapper.toEntity() (parameter)

Call sites in VisaService use positional arguments and will not be affected by the rename.

🤖 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/FileLogService.java`
around lines 23 - 24, The method and parameter name typo `visaCasId` should be
renamed to `visaCaseId` across the public signatures and mapper parameters:
update FileLogService.createFileLog(Long actorUserId, Long visaCasId, ...) to
use visaCaseId, change the corresponding parameter name in
FileLogMapper.toEntity(...) to visaCaseId, and do the same rename in
CommentLogService.createCommentLog(...) and CommentLogMapper.toEntity(...);
ensure you update the parameter names inside each method/body and mapper
implementation to reference visaCaseId (keeping the same types and argument
order) so call sites using positional arguments remain unaffected.

Comment thread src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java Outdated
@eeebbaandersson eeebbaandersson linked an issue Apr 24, 2026 that may be closed by this pull request

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

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/visa/service/VisaService.java (1)

230-230: ⚠️ Potential issue | 🟡 Minor

Inconsistent null/blank check for s3Key in audit message.

Line 214 guards with s3Key != null && !s3Key.isBlank(), but this log message only checks s3Key != null. If a blank (but non-null) key is ever passed in, the visa log will incorrectly state "Document attached." while no file log/attachment was recorded. Tighten the ternary to match.

Proposed fix
-                "Visa application submitted." + (s3Key != null ? " Document attached." : "")
+                "Visa application submitted." + (s3Key != null && !s3Key.isBlank() ? " Document attached." : "")
🤖 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`
at line 230, The audit message uses a looser check (s3Key != null) than the
earlier guard (s3Key != null && !s3Key.isBlank()), causing "Document attached."
to be logged for blank keys; update the ternary in the VisaService audit/log
string to use the same strict check (s3Key != null && !s3Key.isBlank()) so the
message only reports an attachment when a non-blank s3Key exists (referencing
s3Key and the audit/log string in VisaService).
🧹 Nitpick comments (3)
src/test/java/org/example/visacasemanagementsystem/audit/mapper/CommentLogMapperTest.java (2)

17-22: Nit: fix variable naming and indentation.

  • Line 17/21: commentLogmapper should be commentLogMapper for consistent camelCase.
  • Line 20: void setUp() is indented with 2 spaces while the rest of the class uses 4.
✏️ Proposed tweak
-    private CommentLogMapper commentLogmapper;
+    private CommentLogMapper commentLogMapper;
 
     `@BeforeEach`
-  void setUp() {
-        commentLogmapper = new CommentLogMapper();
+    void setUp() {
+        commentLogMapper = new CommentLogMapper();
     }

(and update the two call sites on lines 39 and 57 accordingly)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/org/example/visacasemanagementsystem/audit/mapper/CommentLogMapperTest.java`
around lines 17 - 22, Rename the field commentLogmapper to commentLogMapper and
update all usages to the new camelCase name (including its references in the
test methods that call it), and fix indentation of the setUp method
signature/body to match the class style (use 4-space indentation) where the
CommentLogMapper instance is initialized; ensure the constructor call remains
commentLogMapper = new CommentLogMapper() and all call sites reference
commentLogMapper.

64-82: Consider adding a null/edge-case test for toEntity.

Coverage for toEntity is limited to the happy path. Since the mapper has no null guards, a passing test documenting the current behavior when some identifiers or the description are null (and/or a test where commentEventType is null) would lock in expectations and catch accidental regressions—especially given the @NotNull constraints on the entity fields. Not blocking.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/org/example/visacasemanagementsystem/audit/mapper/CommentLogMapperTest.java`
around lines 64 - 82, Add tests in CommentLogMapperTest that call
commentLogmapper.toEntity with null arguments to lock in current behavior: one
test should pass null for actorUserId, visaCaseId, commentId and description
(but a valid CommentEventType) and assert the resulting CommentLog has those
corresponding fields null while id and timeStamp remain null; another test
should pass a null CommentEventType and valid identifiers/description and assert
getCommentEventType() is null (and other fields match inputs). Use the existing
toEntity invocation pattern and assertions to document and protect the mapper's
current null-handling behavior.
src/main/java/org/example/visacasemanagementsystem/audit/service/FileLogService.java (1)

29-34: findAll() loads all audit logs into memory — will degrade as the audit table grows.

Audit tables typically grow unbounded. Consider returning Page<FileLogDTO> (accept a Pageable) or providing filtered query methods (by visaCaseId, actorUserId, time range) on FileLogRepository. This also improves controller/UX by supporting sorting and pagination out of the box.

🤖 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/FileLogService.java`
around lines 29 - 34, The current FileLogService.findAll() loads the entire
audit table into memory; change its signature to accept a Pageable and return
Page<FileLogDTO> (e.g., FileLogService.findAll(Pageable pageable)) and use
fileLogRepository.findAll(pageable).map(fileLogMapper::toDTO) to stream mapping
without materializing all rows; alternatively or additionally add filtered repo
methods (e.g., fileLogRepository.findByVisaCaseId(Long visaCaseId, Pageable),
findByActorUserId(...), findByCreatedAtBetween(...)) and expose corresponding
service methods to support pagination/sorting in the controller/UX.
🤖 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`:
- Line 313: The audit message is hard-coded as "Applicant removed file." in
fileLogService.createFileLog even though removeVisaDocument can be invoked by
admins; change the description to reflect the actual actor by checking the actor
against the visa owner and/or their role inside removeVisaDocument: if userId
equals visa.getApplicantId() log "Applicant removed file.", otherwise if the
actor has admin/sysadmin role log "Admin removed file." (include the userId or
role in the message for clarity). Update the call to
fileLogService.createFileLog(...) to build the description dynamically using
removeVisaDocument, fileLogService.createFileLog and FileEventType.DELETED so
the audit accurately records who performed the deletion.

In
`@src/test/java/org/example/visacasemanagementsystem/audit/mapper/FileLogMapperTest.java`:
- Line 15: The `@DisplayName` value in the test class FileLogMapperTest contains a
typo ("FilLogMapper"); update the DisplayName string to "FileLogMapper unit
tests" so the test display reads correctly—locate the `@DisplayName` annotation in
the FileLogMapperTest class and correct the spelling.
- Around line 44-50: The test FileLogMapperTest currently omits asserting the
visaCaseId field; add an assertion verifying dto.visaCaseId() equals the
entity's value (200L) to ensure FileLogMapper.toDTO copies that field. Locate
the test where the entity is set with visaCaseId(200L) and append an assertion
similar to assertThat(dto.visaCaseId()).isEqualTo(200L) so the test truly
validates "every field" is mapped.

---

Outside diff comments:
In
`@src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java`:
- Line 230: The audit message uses a looser check (s3Key != null) than the
earlier guard (s3Key != null && !s3Key.isBlank()), causing "Document attached."
to be logged for blank keys; update the ternary in the VisaService audit/log
string to use the same strict check (s3Key != null && !s3Key.isBlank()) so the
message only reports an attachment when a non-blank s3Key exists (referencing
s3Key and the audit/log string in VisaService).

---

Nitpick comments:
In
`@src/main/java/org/example/visacasemanagementsystem/audit/service/FileLogService.java`:
- Around line 29-34: The current FileLogService.findAll() loads the entire audit
table into memory; change its signature to accept a Pageable and return
Page<FileLogDTO> (e.g., FileLogService.findAll(Pageable pageable)) and use
fileLogRepository.findAll(pageable).map(fileLogMapper::toDTO) to stream mapping
without materializing all rows; alternatively or additionally add filtered repo
methods (e.g., fileLogRepository.findByVisaCaseId(Long visaCaseId, Pageable),
findByActorUserId(...), findByCreatedAtBetween(...)) and expose corresponding
service methods to support pagination/sorting in the controller/UX.

In
`@src/test/java/org/example/visacasemanagementsystem/audit/mapper/CommentLogMapperTest.java`:
- Around line 17-22: Rename the field commentLogmapper to commentLogMapper and
update all usages to the new camelCase name (including its references in the
test methods that call it), and fix indentation of the setUp method
signature/body to match the class style (use 4-space indentation) where the
CommentLogMapper instance is initialized; ensure the constructor call remains
commentLogMapper = new CommentLogMapper() and all call sites reference
commentLogMapper.
- Around line 64-82: Add tests in CommentLogMapperTest that call
commentLogmapper.toEntity with null arguments to lock in current behavior: one
test should pass null for actorUserId, visaCaseId, commentId and description
(but a valid CommentEventType) and assert the resulting CommentLog has those
corresponding fields null while id and timeStamp remain null; another test
should pass a null CommentEventType and valid identifiers/description and assert
getCommentEventType() is null (and other fields match inputs). Use the existing
toEntity invocation pattern and assertions to document and protect the mapper's
current null-handling behavior.
🪄 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: 48de7cd0-aacd-40ba-88c8-b186e3c4939c

📥 Commits

Reviewing files that changed from the base of the PR and between 403f440 and 3f76301.

📒 Files selected for processing (5)
  • src/main/java/org/example/visacasemanagementsystem/audit/service/CommentLogService.java
  • src/main/java/org/example/visacasemanagementsystem/audit/service/FileLogService.java
  • src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java
  • src/test/java/org/example/visacasemanagementsystem/audit/mapper/CommentLogMapperTest.java
  • src/test/java/org/example/visacasemanagementsystem/audit/mapper/FileLogMapperTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/java/org/example/visacasemanagementsystem/audit/service/CommentLogService.java

Comment thread src/main/java/org/example/visacasemanagementsystem/visa/service/VisaService.java Outdated
@eeebbaandersson
eeebbaandersson merged commit 325552a into main Apr 24, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extend Logger

1 participant