Task/critical tests#40
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
💤 Files with no reviewable changes (2)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds multiple new unit and integration tests for audit logging, controllers, and services, plus minor whitespace cleanups in two DTO/entity files. Tests exercise audit sanitization, URI-derived context capture, and role-based authorization across Case and Employee APIs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ 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: 2
🤖 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/test/java/org/example/projektarendehantering/AuditIntegrationTest.java`:
- Around line 56-60: The test uses auditEventRepository.findAll() and picks the
last element (events.get(events.size() - 1)), which is nondeterministic; change
the test to fetch a deterministically ordered event (for example use a
repository query that orders by timestamp or id such as
findFirstByOrderByTimestampDesc or findTopByOrderByIdDesc, or sort the returned
list by AuditEventEntity.getTimestamp() before selecting the latest) and update
assertions that reference latest.getRequestPath() accordingly; apply the same
change pattern for the other occurrences flagged (lines around the other
assertions in AuditIntegrationTest).
In
`@src/test/java/org/example/projektarendehantering/presentation/rest/AuditControllerTest.java`:
- Around line 76-87: The test list_withFilters_shouldPassParameters currently
only asserts HTTP 200 but doesn't verify that controller forwarded
caseId/page/size to auditService.listEvents; update the test to verify
auditService.listEvents was called (e.g., verify(auditService).listEvents(...))
and assert the Pageable argument reflects page=1 and size=10 by using an
ArgumentCaptor<Pageable> or Mockito.argThat(Pageable ->
pageable.getPageNumber()==1 && pageable.getPageSize()==10), and also assert the
caseId is passed via eq(caseId) and the method is invoked once. Ensure you
reference the existing test method name list_withFilters_shouldPassParameters
and the target method auditService.listEvents when adding the verify/assertion.
🪄 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: e1b4acee-d0a2-4a3d-bebc-48d84576a8ac
📒 Files selected for processing (7)
src/test/java/org/example/projektarendehantering/AuditIntegrationTest.javasrc/test/java/org/example/projektarendehantering/application/service/CaseServiceTest.javasrc/test/java/org/example/projektarendehantering/application/service/EmployeeServiceTest.javasrc/test/java/org/example/projektarendehantering/infrastructure/security/SecurityActorAdapterTest.javasrc/test/java/org/example/projektarendehantering/presentation/rest/AuditControllerTest.javasrc/test/java/org/example/projektarendehantering/presentation/rest/CaseControllerTest.javasrc/test/java/org/example/projektarendehantering/presentation/rest/EmployeeControllerTest.java
…torAdapterTest compilation
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/test/java/org/example/projektarendehantering/application/service/AuditServiceTest.java (3)
59-67: Strengthen success-path assertions to validate DTO mapping output.At Line 66 and Line 81, asserting only size can pass even if
auditEventMapper.toDTO(...)returns wrong/null content. Assert the mapped DTO instance (or critical fields) to make these tests regression-resistant.Suggested test assertion hardening
@@ - when(auditEventMapper.toDTO(any())).thenReturn(new AuditEventDTO()); + AuditEventDTO expectedDto = new AuditEventDTO(); + when(auditEventMapper.toDTO(any())).thenReturn(expectedDto); @@ - assertThat(result.getContent()).hasSize(1); + assertThat(result.getContent()).containsExactly(expectedDto); @@ - when(auditEventMapper.toDTO(any())).thenReturn(new AuditEventDTO()); + AuditEventDTO expectedDto = new AuditEventDTO(); + when(auditEventMapper.toDTO(any())).thenReturn(expectedDto); @@ - assertThat(result.getContent()).hasSize(1); + assertThat(result.getContent()).containsExactly(expectedDto);Also applies to: 75-82
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/example/projektarendehantering/application/service/AuditServiceTest.java` around lines 59 - 67, The test currently only asserts page size, which won't catch incorrect or null mapping results from auditEventMapper.toDTO; update AuditServiceTest to assert the actual DTO content returned by auditService.listEvents by stubbing auditEventMapper.toDTO to return a DTO with known values (e.g., set an identifiable field on AuditEventDTO) and then assert that result.getContent().get(0) (or its critical fields) equals those expected values; target the test block that builds PageImpl<AuditEventEntity>, the when(auditEventMapper.toDTO(...)) stub, and the final assertions so the test validates both presence and correctness of the mapped AuditEventDTO.
85-91: Assert no audit-event repository access on denied requests.For Line 88 and Line 97 denial paths, add interaction assertions to guarantee no audit read is performed before/while denying access.
Suggested security-focused interaction assertions
@@ import static org.mockito.Mockito.when; +import static org.mockito.Mockito.verifyNoInteractions; @@ void listEvents_shouldDenyDoctorAccessToUnownedCase() { when(caseRepository.findAllByOwnerId(doctorActor.userId())).thenReturn(Collections.emptyList()); @@ .isInstanceOf(NotAuthorizedException.class) .hasMessageContaining("Not allowed to view audit events for this case"); + + verifyNoInteractions(auditEventRepository); } @@ void listEvents_shouldDenyPatient() { Actor patientActor = new Actor(UUID.randomUUID(), Role.PATIENT); @@ assertThatThrownBy(() -> auditService.listEvents(patientActor, null, null, null, Pageable.unpaged())) .isInstanceOf(NotAuthorizedException.class); + + verifyNoInteractions(auditEventRepository, caseRepository); }Also applies to: 97-99
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/example/projektarendehantering/application/service/AuditServiceTest.java` around lines 85 - 91, The test currently asserts that auditService.listEvents throws NotAuthorizedException but doesn't verify the audit repository wasn't queried; update AuditServiceTest (e.g., in listEvents_shouldDenyDoctorAccessToUnownedCase and the other denial test around lines 97-99) to assert no audit reads occur by adding interaction assertions such as verifyNoInteractions(auditEventRepository) or verify(auditEventRepository, never()).find*(...) after the exception assertion to guarantee auditEventRepository is not accessed when access is denied.
57-109: Add tests for remaining authorization branches inAuditService.listEvents.Based on
src/main/java/org/example/projektarendehantering/application/service/AuditService.java(Line 214 onward), two key branches are still untested here: manager with explicitcaseId, and nurse access path. Adding these would close meaningful RBAC coverage gaps.Suggested additional test cases
+ `@Test` + void listEvents_shouldAllowManagerToFilterByCaseId() { + when(auditEventRepository.findAllByCaseIdAndOccurredAtBetweenOrderByOccurredAtDesc(eq(caseId), any(), any(), any())) + .thenReturn(new PageImpl<>(List.of(new AuditEventEntity()))); + when(auditEventMapper.toDTO(any())).thenReturn(new AuditEventDTO()); + + Page<AuditEventDTO> result = auditService.listEvents(managerActor, null, null, caseId, Pageable.unpaged()); + + assertThat(result.getContent()).hasSize(1); + } + + `@Test` + void listEvents_shouldAllowNurseToSeeOnlyHandledCases() { + // Arrange a nurse actor + handled case and assert filtered audit access path. + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/example/projektarendehantering/application/service/AuditServiceTest.java` around lines 57 - 109, Add two unit tests exercising the remaining AuditService.listEvents branches: one where managerActor passes an explicit caseId and should be allowed to view events, and one for nurseActor's access path. For the manager case, mock auditEventRepository.findAllByCaseIdInAndOccurredAtBetweenOrderByOccurredAtDesc(Set.of(caseId), ...) to return a Page of AuditEventEntity and mock auditEventMapper.toDTO(...) then call auditService.listEvents(managerActor, ..., caseId, ...) and assert the page contains the expected DTOs. For the nurse case, create a nurseActor, stub caseRepository.findAllByOwnerId(nurseActor.userId()) (or the repository method used for nurse filtering) and auditEventRepository calls similarly, then call auditService.listEvents(nurseActor, ...) and assert allowed/denied behavior consistent with AuditService.listEvents; use the existing managerActor, caseId, auditEventRepository, caseRepository, auditEventMapper, and auditService symbols to locate where to add these tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/test/java/org/example/projektarendehantering/application/service/AuditServiceTest.java`:
- Around line 59-67: The test currently only asserts page size, which won't
catch incorrect or null mapping results from auditEventMapper.toDTO; update
AuditServiceTest to assert the actual DTO content returned by
auditService.listEvents by stubbing auditEventMapper.toDTO to return a DTO with
known values (e.g., set an identifiable field on AuditEventDTO) and then assert
that result.getContent().get(0) (or its critical fields) equals those expected
values; target the test block that builds PageImpl<AuditEventEntity>, the
when(auditEventMapper.toDTO(...)) stub, and the final assertions so the test
validates both presence and correctness of the mapped AuditEventDTO.
- Around line 85-91: The test currently asserts that auditService.listEvents
throws NotAuthorizedException but doesn't verify the audit repository wasn't
queried; update AuditServiceTest (e.g., in
listEvents_shouldDenyDoctorAccessToUnownedCase and the other denial test around
lines 97-99) to assert no audit reads occur by adding interaction assertions
such as verifyNoInteractions(auditEventRepository) or
verify(auditEventRepository, never()).find*(...) after the exception assertion
to guarantee auditEventRepository is not accessed when access is denied.
- Around line 57-109: Add two unit tests exercising the remaining
AuditService.listEvents branches: one where managerActor passes an explicit
caseId and should be allowed to view events, and one for nurseActor's access
path. For the manager case, mock
auditEventRepository.findAllByCaseIdInAndOccurredAtBetweenOrderByOccurredAtDesc(Set.of(caseId),
...) to return a Page of AuditEventEntity and mock auditEventMapper.toDTO(...)
then call auditService.listEvents(managerActor, ..., caseId, ...) and assert the
page contains the expected DTOs. For the nurse case, create a nurseActor, stub
caseRepository.findAllByOwnerId(nurseActor.userId()) (or the repository method
used for nurse filtering) and auditEventRepository calls similarly, then call
auditService.listEvents(nurseActor, ...) and assert allowed/denied behavior
consistent with AuditService.listEvents; use the existing managerActor, caseId,
auditEventRepository, caseRepository, auditEventMapper, and auditService symbols
to locate where to add these tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f605c1a2-06e8-49f0-9ce4-36200bb1857d
📒 Files selected for processing (4)
src/test/java/org/example/projektarendehantering/AuditIntegrationTest.javasrc/test/java/org/example/projektarendehantering/application/service/AuditServiceTest.javasrc/test/java/org/example/projektarendehantering/infrastructure/security/SecurityActorAdapterTest.javasrc/test/java/org/example/projektarendehantering/presentation/rest/AuditControllerTest.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/test/java/org/example/projektarendehantering/AuditIntegrationTest.java
- src/test/java/org/example/projektarendehantering/infrastructure/security/SecurityActorAdapterTest.java
- src/test/java/org/example/projektarendehantering/presentation/rest/AuditControllerTest.java
… resolve potential merge conflicts
This was referenced Apr 14, 2026
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #33
Summary by CodeRabbit