Test: add tests for ActivityLogPolicy to check access for admin, vet …

[TatjanaTrajkovic]

…and owner

Summary by CodeRabbit

• Tests
  • Added comprehensive test coverage for activity log access control policies, verifying role-based access permissions across different user types and edge cases including null input handling.

[coderabbitai]

📝 Walkthrough

Walkthrough

A new JUnit 5 test class ActivityLogPolicyTest is introduced to comprehensively test the ActivityLogPolicy class. The test suite verifies access control behavior across different user roles (admin, vet, owner) with various clinic and ownership scenarios, as well as null/invalid input handling using Mockito mocks.

Changes

Cohort / File(s)	Summary
ActivityLogPolicy Test Suite src/test/java/org/example/vet1177/policy/ActivityLogPolicyTest.java	Added comprehensive unit tests covering canView() access control across roles (admin always allowed, vet allowed only for same clinic, owner allowed only for same owner), null/invalid input handling, and canUpdate()/canDelete() always throwing ForbiddenException.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• Test: Add Unit tests for ActivityLogPolicy #135: Adds unit tests for ActivityLogPolicy class, directly exercising the same canView, canUpdate, and canDelete behavior being tested.

Possibly related PRs

• Add ActivityLogPolicy with role-based access control and immutable rules #63: Implements the ActivityLogPolicy class whose behavior is now being tested in this PR.
• Changes ActivityLogPolicy #75: Modifies ActivityLogPolicy implementation (constants and control flow), making these tests directly applicable to validating the changes.
• Changes ActivityLogService after ActivityLogController implements #70: Integrates ActivityLogPolicy into ActivityLogService, making the policy tests essential for validating the service's authorization behavior.

Suggested reviewers

• johanbriger
• lindaeskilsson

Poem

🐰 Hops through the test suite with glee,
Checking what vet, owner, admin can see,
Mocking the clinics, the records, the logs,
No more blind spots—just policies that's solid as dogs! 🧪✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 45.45% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: adding unit tests for ActivityLogPolicy to verify access permissions for admin, vet, and owner roles.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch test/unit_tests_activity_log_policy

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

src/test/java/org/example/vet1177/policy/ActivityLogPolicyTest.java (1)

111-132: Add tests for the remaining defensive branches in canView.

The suite still misses explicit cases for null role (user.getRole() == null), vet null-clinic branches, and owner null-owner/null-owner-id branches. Covering these would fully lock down the guard logic.

Proposed additional test cases

+    `@Test`
+    void canView_shouldThrow_whenUserRoleIsNull() {
+        User noRoleUser = mock(User.class);
+        when(noRoleUser.getRole()).thenReturn(null);
+
+        assertThrows(ForbiddenException.class, () -> policy.canView(noRoleUser, log));
+    }
+
+    `@Test`
+    void canView_shouldThrow_whenVetClinicIsNull() {
+        User vetNoClinic = mock(User.class);
+        when(vetNoClinic.getRole()).thenReturn(Role.VET);
+        when(vetNoClinic.getClinic()).thenReturn(null);
+
+        assertThrows(ForbiddenException.class, () -> policy.canView(vetNoClinic, log));
+    }
+
+    `@Test`
+    void canView_shouldThrow_whenRecordOwnerIsNull() {
+        when(record.getOwner()).thenReturn(null);
+
+        assertThrows(ForbiddenException.class, () -> policy.canView(owner, log));
+    }

🤖 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/ActivityLogPolicyTest.java` around
lines 111 - 132, Add unit tests in ActivityLogPolicyTest to cover the remaining
defensive branches of policy.canView: write one test where the user's role is
null (mock user.getRole() to return null) and
assertThrows(ForbiddenException.class, () -> policy.canView(user, log)); write
one test where the medical record's vet has a null clinic (mock
log.getMedicalRecord().getVet() to return a vet whose getClinic() returns null)
and assertThrows for policy.canView(vetUser, log); and write two tests for
owner-null scenarios: one where record.getOwner() returns null and one where
record.getOwner().getId() returns null, both asserting that
policy.canView(admin, log) throws ForbiddenException; use the existing
mocks/variables (policy, log, admin, etc.) and Mockito when(...) chains to set
these return values.

🤖 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/ActivityLogPolicyTest.java`:
- Around line 111-132: Add unit tests in ActivityLogPolicyTest to cover the
remaining defensive branches of policy.canView: write one test where the user's
role is null (mock user.getRole() to return null) and
assertThrows(ForbiddenException.class, () -> policy.canView(user, log)); write
one test where the medical record's vet has a null clinic (mock
log.getMedicalRecord().getVet() to return a vet whose getClinic() returns null)
and assertThrows for policy.canView(vetUser, log); and write two tests for
owner-null scenarios: one where record.getOwner() returns null and one where
record.getOwner().getId() returns null, both asserting that
policy.canView(admin, log) throws ForbiddenException; use the existing
mocks/variables (policy, log, admin, etc.) and Mockito when(...) chains to set
these return values.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c55f5329-2e69-4df6-ae80-cd0f66de8a37

📥 Commits

Reviewing files that changed from the base of the PR and between ba13529 and 529848f.

📒 Files selected for processing (1)

• src/test/java/org/example/vet1177/policy/ActivityLogPolicyTest.java

[annikaholmqvist94]

Bra jobb då har vi med alla cases:)