test: add unit tests for CommentPolicy and CommentService - #124
Merged
Conversation
- Introduced comprehensive unit tests for `CommentPolicy` methods: `canCreate`, `canView`, `canUpdate`, and `canDelete`. - Simplified test dependencies in `pom.xml` by consolidating into `spring-boot-starter-test`.
- Added comprehensive test coverage for `CommentService` methods: `create`, `getByRecord`, `update`, `delete`, and `countByRecord`. - Included policy-based validation and activity logging assertions. Closes #97 - Ensured proper exception handling for missing resources in test cases.
annikaholmqvist94
requested review from
TatjanaTrajkovic,
johanbriger and
lindaeskilsson
April 4, 2026 10:55
📝 WalkthroughWalkthroughThis pull request adds comprehensive unit tests for CommentService and CommentPolicy using JUnit 5 and Mockito, verifying authorization rules and business logic. Additionally, Maven test dependencies are consolidated by replacing three separate Spring Boot test starters with a single Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
🧹 Nitpick comments (2)
src/test/java/org/example/vet1177/services/CommentServiceTest.java (1)
67-106: Add policy-denied path tests to lock down “no side effects on authorization failure”.Right now the suite validates not-found failures, but not policy rejections. Add explicit tests where
commentPolicythrows and assertsave/delete/logare not invoked.✅ Suggested test additions
+ `@Test` + void create_whenPolicyDenies_shouldPropagateAndNotSaveOrLog() { + when(medicalRecordRepository.findById(recordId)).thenReturn(Optional.of(record)); + doThrow(new org.example.vet1177.exception.ForbiddenException("Åtkomst nekad")) + .when(commentPolicy).canCreate(currentUser, record); + + assertThatThrownBy(() -> commentService.create(recordId, "En kommentar.", currentUser)) + .isInstanceOf(org.example.vet1177.exception.ForbiddenException.class); + + verify(commentRepository, never()).save(any()); + verify(activityLogService, never()).log(any(), any(), any(), any()); + } + + `@Test` + void delete_whenPolicyDenies_shouldPropagateAndNotDeleteOrLog() { + when(commentRepository.findById(commentId)).thenReturn(Optional.of(comment)); + doThrow(new org.example.vet1177.exception.ForbiddenException("Åtkomst nekad")) + .when(commentPolicy).canDelete(currentUser, comment); + + assertThatThrownBy(() -> commentService.delete(commentId, currentUser)) + .isInstanceOf(org.example.vet1177.exception.ForbiddenException.class); + + verify(commentRepository, never()).delete(any()); + verify(activityLogService, never()).log(any(), any(), any(), any()); + }Also applies to: 146-226, 232-260
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/example/vet1177/services/CommentServiceTest.java` around lines 67 - 106, Add tests that simulate policy denial by making commentPolicy.canCreate(...) and commentPolicy.canDelete(...) throw the same authorization exception your service expects, then call commentService.create(...) and commentService.delete(...). After each call assert that the service throws the authorization exception and verify side-effect collaborators are not invoked (verify(commentRepository, never()).save(...), verify(commentRepository, never()).delete(...), verify(activityLogService, never()).log(...), etc.). Use the existing test setup symbols (commentService.create, commentService.delete, commentPolicy.canCreate, commentPolicy.canDelete, commentRepository, activityLogService) and the same argument matchers as other tests (any(Comment.class), specific record/currentUser) so the new tests mirror the existing style.src/test/java/org/example/vet1177/policy/CommentPolicyTest.java (1)
94-107: Consider extracting repeated user-fixture creation into a helper.
otherOwner/vetNoClinic/othersetup repeats the same pattern. A small factory/helper would reduce duplication and future edit churn.♻️ Example refactor
+ private User newUserWithId(String name, String email, Role role) throws Exception { + User user = new User(name, email, "hash", role); + setPrivateField(user, "id", UUID.randomUUID()); + return user; + } + + private User newVetWithOptionalClinic(String name, String email, Clinic clinic) throws Exception { + User user = (clinic == null) + ? new User(name, email, "hash", Role.VET) + : new User(name, email, "hash", Role.VET, clinic); + setPrivateField(user, "id", UUID.randomUUID()); + return user; + }Also applies to: 151-164, 198-200, 223-224
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/example/vet1177/policy/CommentPolicyTest.java` around lines 94 - 107, Extract the repeated user fixture creation in CommentPolicyTest into a private helper like buildUser(String name, String email, Role role) (and optional overload accepting UUID) that constructs new User(...), sets a random UUID via setPrivateField(..., "id", UUID.randomUUID()) and returns the user; then replace the inline creations of otherOwner, vetNoClinic, other, etc. in methods such as canCreate_ownerOnOthersRecord_shouldThrowForbiddenException and canCreate_vetWithNullUserClinic_shouldThrowForbiddenException with calls to this helper to remove duplication.
🤖 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/vet1177/policy/CommentPolicyTest.java`:
- Around line 94-107: Extract the repeated user fixture creation in
CommentPolicyTest into a private helper like buildUser(String name, String
email, Role role) (and optional overload accepting UUID) that constructs new
User(...), sets a random UUID via setPrivateField(..., "id", UUID.randomUUID())
and returns the user; then replace the inline creations of otherOwner,
vetNoClinic, other, etc. in methods such as
canCreate_ownerOnOthersRecord_shouldThrowForbiddenException and
canCreate_vetWithNullUserClinic_shouldThrowForbiddenException with calls to this
helper to remove duplication.
In `@src/test/java/org/example/vet1177/services/CommentServiceTest.java`:
- Around line 67-106: Add tests that simulate policy denial by making
commentPolicy.canCreate(...) and commentPolicy.canDelete(...) throw the same
authorization exception your service expects, then call
commentService.create(...) and commentService.delete(...). After each call
assert that the service throws the authorization exception and verify
side-effect collaborators are not invoked (verify(commentRepository,
never()).save(...), verify(commentRepository, never()).delete(...),
verify(activityLogService, never()).log(...), etc.). Use the existing test setup
symbols (commentService.create, commentService.delete, commentPolicy.canCreate,
commentPolicy.canDelete, commentRepository, activityLogService) and the same
argument matchers as other tests (any(Comment.class), specific
record/currentUser) so the new tests mirror the existing style.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e388291d-2bbd-42b0-8b6b-3a5feb1b6232
📒 Files selected for processing (3)
pom.xmlsrc/test/java/org/example/vet1177/policy/CommentPolicyTest.javasrc/test/java/org/example/vet1177/services/CommentServiceTest.java
TatjanaTrajkovic
approved these changes
Apr 4, 2026
TatjanaTrajkovic
left a comment
TatjanaTrajkovic
left a comment
Contributor
There was a problem hiding this comment.
Ser bra ut för min del!
This was referenced Apr 5, 2026
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
ResourceNotFoundException kastas vid saknade resurser
Test approach
vid fel
Closes #97
Summary by CodeRabbit
Tests
Chores