Feature/search filter - #63
Conversation
|
Warning Rate limit exceeded
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 19 minutes and 28 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR refactors comment authorship to use authenticated user context instead of client-provided authorId, adds dynamic filtering capability to ticket listing via query parameters, and applies Lombok annotations to TicketAttachment for cleaner code. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CommentController
participant AuthenticationPrincipal as Auth Context
participant CommentService
participant TicketRepository
participant ActivityLogService
rect rgba(100, 200, 150, 0.5)
Note over Client,ActivityLogService: Old Flow (Client-Provided Author)
Client->>CommentController: POST /api/tickets/{id}/comments<br/>(CommentDTO with authorId)
CommentController->>CommentService: addComment(ticketId, dto)
CommentService->>TicketRepository: findById(ticketId)
TicketRepository-->>CommentService: Ticket
CommentService->>CommentService: lookup Staff by dto.authorId
end
rect rgba(150, 150, 200, 0.5)
Note over Client,ActivityLogService: New Flow (Authenticated Author)
Client->>CommentController: POST /api/tickets/{id}/comments<br/>(CommentDTO without authorId)
AuthenticationPrincipal-->>CommentController: Staff (authenticated principal)
CommentController->>CommentService: addComment(ticketId, dto, author)
CommentService->>TicketRepository: findById(ticketId)
TicketRepository-->>CommentService: Ticket
CommentService->>ActivityLogService: log activity
ActivityLogService-->>CommentService: logged
CommentService-->>CommentController: Comment created
CommentController-->>Client: 201 CommentResponseDTO
end
sequenceDiagram
participant Client
participant TicketController
participant TicketService
participant TicketRepository
participant DTOMapper
rect rgba(150, 200, 150, 0.5)
Note over Client,DTOMapper: Ticket Filtering Flow
Client->>TicketController: GET /api/tickets?status=OPEN&priority=HIGH
TicketController->>TicketController: parse query params → TicketFilterParams
alt filters.isEmpty()
TicketService->>TicketService: getAllTickets()
else filters present
TicketService->>TicketRepository: findByFilters(status, priority, ...)
TicketRepository->>TicketRepository: dynamic WHERE clause filtering
TicketRepository-->>TicketService: List\<Ticket\>
end
TicketService->>DTOMapper: map to TicketResponseDTO list
DTOMapper-->>TicketService: List\<TicketResponseDTO\>
TicketService-->>TicketController: filtered results
TicketController-->>Client: 200 List\<TicketResponseDTO\>
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/org/example/cyberwatch/features/comment/service/CommentService.java (1)
38-45: Add a fail-fast guard for missing authenticated author.Consider rejecting
nullauthorearly to avoid persisting invalid data or surfacing a later NPE/constraint error if security wiring is misconfigured.Proposed patch
public CommentResponseDTO addComment(Long ticketId, CommentDTO dto, Staff author) { + if (author == null) { + throw new IllegalStateException("Authenticated author is required"); + } + Ticket ticket = ticketRepository.findById(ticketId) .orElseThrow(() -> new TicketNotFoundException(ticketId));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/cyberwatch/features/comment/service/CommentService.java` around lines 38 - 45, The addComment(Long ticketId, CommentDTO dto, Staff author) method should fail fast when author is null: add an early null-check for the author parameter at the start of the method (before creating Comment), and throw a clear exception (e.g., IllegalArgumentException or a custom UnauthorizedException) with a descriptive message so we never proceed to setTicket/setAuthor or persist an invalid Comment; update tests if any rely on null-author behavior.
🤖 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/cyberwatch/features/ticket/service/TicketService.java`:
- Around line 97-106: The branch that returns all tickets when filters.isEmpty()
is inconsistent with the filtered path's ordering; update getAllTickets() (or
the empty-filters branch) so it returns tickets ordered by createdAt DESC to
match ticketRepository.findByFilters(...) ordering. Locate the getAllTickets()
method in TicketService and change its repository call to use an ordered query
(or delegate to the repository method ensuring createdAt DESC), so both the
no-filter path and the filtered path return tickets "nyast först".
---
Nitpick comments:
In
`@src/main/java/org/example/cyberwatch/features/comment/service/CommentService.java`:
- Around line 38-45: The addComment(Long ticketId, CommentDTO dto, Staff author)
method should fail fast when author is null: add an early null-check for the
author parameter at the start of the method (before creating Comment), and throw
a clear exception (e.g., IllegalArgumentException or a custom
UnauthorizedException) with a descriptive message so we never proceed to
setTicket/setAuthor or persist an invalid Comment; update tests if any rely on
null-author 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: 126b3c70-5ecb-459e-a391-08c7b7a933b4
📒 Files selected for processing (9)
src/main/java/org/example/cyberwatch/config/SecurityConfig.javasrc/main/java/org/example/cyberwatch/features/comment/controller/CommentController.javasrc/main/java/org/example/cyberwatch/features/comment/model/CommentDTO.javasrc/main/java/org/example/cyberwatch/features/comment/service/CommentService.javasrc/main/java/org/example/cyberwatch/features/ticket/controller/TicketController.javasrc/main/java/org/example/cyberwatch/features/ticket/model/TicketAttachment.javasrc/main/java/org/example/cyberwatch/features/ticket/model/TicketFilterParams.javasrc/main/java/org/example/cyberwatch/features/ticket/repository/TicketRepository.javasrc/main/java/org/example/cyberwatch/features/ticket/service/TicketService.java
Resolved:
#35 — Sök och filtrera tickets
#47 — Comment author från SecurityContext istället för request body
#52 — Inconsistent Lombok usage (TicketAttachment)
#59 — JWT HMAC-nyckelderivering fixad + try-catch i OAuth2SuccessHandler
Summary by CodeRabbit
New Features
Refactor
authorIdis no longer required in requests.