Feature/TicketService test suite

[simonforsberg]

Adds a unit test suite for TicketService using JUnit 5 and Mockito.
All tests are isolated with mocked dependencies. No Spring context is loaded.

This PR also includes some minor refactoring in TicketService for improved readability and testing.

Summary by CodeRabbit

• Refactor

  • Improved ticket status management and access-control validation logic for enhanced reliability.

• Tests

  • Added comprehensive test suite for ticket service covering ticket creation, retrieval, status transitions, and investigator assignments.

[coderabbitai]

📝 Walkthrough

Walkthrough

The pull request refactors access-control and status-transition validation logic in TicketService, introducing a computed isAssignedInvestigator boolean and replacing potentially null allowedTransitions with a default-safe approach. A comprehensive new test suite with 800+ lines covers ticket creation, retrieval, status updates, and investigator assignment workflows.

Changes

Cohort / File(s)	Summary
TicketService Refactoring src/main/java/org/example/alfs/services/TicketService.java	Refactored updateTicketStatus access-control logic: replaced nested role checks with a single isAssignedInvestigator boolean computation. Updated status-transition validation to use allowedTransitions.getOrDefault(currentStatus, Set.of()) and compute isValidTransition to eliminate null-checking. Updated exception message to reference currentStatus.
TicketService Test Suite src/test/java/org/example/alfs/services/TicketServiceTest.java	Added comprehensive JUnit 5 test suite covering ticket CRUD operations (createNewTicket, getTicketById, getMyTickets), status-based retrieval (getTicketsByStatus, getTicketsByStatusAndInvestigator), status transitions with validation, investigator assignment/unassignment, and access-control scenarios for admin, investigator, and reporter roles.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Possibly related PRs

• Feature/ticket lifecycle update #12: Modifies the same updateTicketStatus and allowedTransitions handling logic within TicketService.
• Implement ticket access control and status workflow #14: Adjusts TicketService access-control and transition-validation logic, including investigator checks and allowed-transitions handling.
• Feature/ticket controller #11: Introduced the TicketService class; this PR refactors its core access-control and status-transition mechanisms.

Poem

🐰 Tickets now flow with safer checks,
Boolean flags prevent the wrecks,
Tests abound in eighty lines,
Each transition clearly shines,
Investigators find their place—
Hopping code with style and grace! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 2.27% 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
Title check	✅ Passed	The title 'Feature/TicketService test suite' accurately summarizes the main change: adding comprehensive unit tests for the TicketService class.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ 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 feature/ticket-service-test

---

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/main/java/org/example/alfs/services/TicketService.java (1)

209-211: Prefer fail-fast over silent fallback for transition-map drift.

Using getOrDefault(..., Set.of()) can hide missing ALLOWED_TRANSITIONS keys (e.g., after future enum changes) by returning generic invalid-transition errors. Consider explicit handling for unmapped currentStatus so config drift is caught immediately.

♻️ Proposed refactor

-        Set<TicketStatus> allowedTransitions = ALLOWED_TRANSITIONS.getOrDefault(currentStatus, Set.of());
+        Set<TicketStatus> allowedTransitions = ALLOWED_TRANSITIONS.get(currentStatus);
+        if (allowedTransitions == null) {
+            throw new IllegalStateException("Missing transition mapping for status: " + currentStatus);
+        }

Also applies to: 229-234

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/example/alfs/services/TicketService.java` around lines 209
- 211, The code uses ALLOWED_TRANSITIONS.getOrDefault(currentStatus, Set.of())
which silently masks missing mapping keys; instead, in TicketService check
explicitly whether ALLOWED_TRANSITIONS.containsKey(currentStatus) and throw a
clear runtime/configuration exception (e.g., IllegalStateException with context
including currentStatus) if the key is absent so mapping drift is caught
immediately; then obtain the allowedTransitions via
ALLOWED_TRANSITIONS.get(currentStatus) and compute isValidTransition as before.
Apply the same explicit-key check and exception behavior for the other
occurrence around lines 229-234 where transitions are validated.

🤖 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/main/java/org/example/alfs/services/TicketService.java`:
- Around line 209-211: The code uses
ALLOWED_TRANSITIONS.getOrDefault(currentStatus, Set.of()) which silently masks
missing mapping keys; instead, in TicketService check explicitly whether
ALLOWED_TRANSITIONS.containsKey(currentStatus) and throw a clear
runtime/configuration exception (e.g., IllegalStateException with context
including currentStatus) if the key is absent so mapping drift is caught
immediately; then obtain the allowedTransitions via
ALLOWED_TRANSITIONS.get(currentStatus) and compute isValidTransition as before.
Apply the same explicit-key check and exception behavior for the other
occurrence around lines 229-234 where transitions are validated.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 06b703ec-a427-4ba1-9df8-15209b23f2ee

📥 Commits

Reviewing files that changed from the base of the PR and between da5c0c4 and 9caa606.

📒 Files selected for processing (2)

• src/main/java/org/example/alfs/services/TicketService.java
• src/test/java/org/example/alfs/services/TicketServiceTest.java