feat: improve ticket detail retrieval and feedback handling#92
Merged
Conversation
Contributor
Reviewer's GuideSplits ticket retrieval into ID- and number-based endpoints with consistent authorization, aligns backend feedback creation with form-data submission, and improves frontend error handling and feedback API calls to work with the new backend behavior. Sequence diagram for ticket detail retrieval by ID and numbersequenceDiagram
actor User
participant FrontendApp
participant TicketApi
participant BackendTicketsRouter
participant DjangoORM
participant Database
User->>FrontendApp: Open ticket detail view (by id or ticket_number)
FrontendApp->>TicketApi: getTicket(identifier)
alt numeric identifier
TicketApi->>BackendTicketsRouter: GET /tickets/{id}
BackendTicketsRouter->>DjangoORM: Ticket.objects.annotate(comments_count=Count(comments)).get(id=id)
else non_numeric identifier
TicketApi->>BackendTicketsRouter: GET /tickets/number/{ticket_number}
BackendTicketsRouter->>DjangoORM: Ticket.objects.annotate(comments_count=Count(comments)).get(ticket_number=ticket_number)
end
DjangoORM->>Database: Query Ticket with comments_count
Database-->>DjangoORM: Ticket or not found
alt ticket found
DjangoORM-->>BackendTicketsRouter: Ticket instance
BackendTicketsRouter->>BackendTicketsRouter: Check ticket.student == request.user or request.user.is_staff
alt authorized
BackendTicketsRouter-->>TicketApi: 200 TicketSchema.from_orm(ticket, request)
TicketApi-->>FrontendApp: Ticket data
FrontendApp-->>User: Render ticket detail
else unauthorized
BackendTicketsRouter-->>TicketApi: 404 {detail: Not found.}
TicketApi-->>FrontendApp: Error(detail)
FrontendApp-->>User: Show not found error
end
else ticket not found
BackendTicketsRouter-->>TicketApi: 404 {detail: Not found.}
TicketApi-->>FrontendApp: Error(detail)
FrontendApp-->>User: Show not found error
end
Sequence diagram for creating ticket feedback with form data and detailed errorssequenceDiagram
actor User
participant FrontendApp
participant FeedbackApi
participant BackendTicketsRouter
participant DjangoORM
participant Database
User->>FrontendApp: Submit feedback form (rating, comments)
FrontendApp->>FeedbackApi: createFeedback(ticketId, payload)
FeedbackApi->>FeedbackApi: Build FormData
FeedbackApi->>FeedbackApi: formData.append(rating, payload.rating)
FeedbackApi->>FeedbackApi: formData.append(comments, payload.comments)
FeedbackApi->>BackendTicketsRouter: POST /tickets/{id}/feedback/
BackendTicketsRouter->>DjangoORM: get_object_or_404(Ticket, id=id)
DjangoORM->>Database: Query Ticket by id
Database-->>DjangoORM: Ticket or not found
alt ticket found
DjangoORM-->>BackendTicketsRouter: Ticket instance
BackendTicketsRouter->>BackendTicketsRouter: Check ticket.student == request.user
BackendTicketsRouter->>BackendTicketsRouter: Check ticket.status == resolved
alt owner and resolved
BackendTicketsRouter->>BackendTicketsRouter: Validate TicketFeedbackCreateSchema from Form data
alt validation ok
BackendTicketsRouter->>Database: Create TicketFeedback
Database-->>BackendTicketsRouter: Saved feedback
BackendTicketsRouter-->>FeedbackApi: 201 TicketFeedbackSchema
FeedbackApi-->>FrontendApp: Feedback data
FrontendApp-->>User: Show success state
else validation error
BackendTicketsRouter-->>FeedbackApi: 400 {detail: [ {msg: error1}, {msg: error2} ]}
FeedbackApi->>FeedbackApi: Parse body.detail array to detail string
FeedbackApi-->>FrontendApp: Error(message or detail)
FrontendApp-->>User: Show validation error messages
end
else not owner or not resolved
BackendTicketsRouter-->>FeedbackApi: 403 {detail: Not allowed to submit feedback}
FeedbackApi-->>FrontendApp: Error(message or detail)
FrontendApp-->>User: Show permission error
end
else ticket not found
BackendTicketsRouter-->>FeedbackApi: 404 {detail: Not found.}
FeedbackApi-->>FrontendApp: Error(message or detail)
FrontendApp-->>User: Show not found error
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
📝 WalkthroughWalkthroughBackend ticket endpoints now accept numeric IDs with authorization checks returning 404 when unauthorized, include a new endpoint to fetch by ticket number, and return properly serialized TicketSchema responses. Frontend feedback creation converts to FormData format, and error handling across frontend API utilities improves by falling back to body.detail fields. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The error handling logic that derives a message from
body.message/body.detail(including the array-of-objects case) is now duplicated and slightly inconsistent betweenfeedback.tsandticket.ts(and withinfeedback.tsfor delete vs. other calls); consider extracting a shared helper to keep behavior uniform and easier to maintain. - The split between
/tickets/{id}(numeric id) and/tickets/number/{ticket_number}changes the original behavior where a single endpoint accepted either ids or ticket numbers; if any callers still rely on the old path shape or on passing ticket numbers to/tickets/{...}, you may want to add a compatibility layer or explicitly deprecate the old usage.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The error handling logic that derives a message from `body.message`/`body.detail` (including the array-of-objects case) is now duplicated and slightly inconsistent between `feedback.ts` and `ticket.ts` (and within `feedback.ts` for delete vs. other calls); consider extracting a shared helper to keep behavior uniform and easier to maintain.
- The split between `/tickets/{id}` (numeric id) and `/tickets/number/{ticket_number}` changes the original behavior where a single endpoint accepted either ids or ticket numbers; if any callers still rely on the old path shape or on passing ticket numbers to `/tickets/{...}`, you may want to add a compatibility layer or explicitly deprecate the old usage.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Polqt
reviewed
Feb 28, 2026
|
|
||
| @router.post("/{id}/feedback/", response={201: TicketFeedbackSchema, 400: dict, 403: dict}) | ||
| def create_feedback(request, id: int, payload: TicketFeedbackCreateSchema, attachment: UploadedFile = File(None)): | ||
| def create_feedback(request, id: int, payload: TicketFeedbackCreateSchema = Form(...), attachment: UploadedFile = File(None)): |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/apps/tickets/views.py (1)
246-258: Add query optimization to avoid N+1 queries.Both
ticket_detailandticket_detail_by_numberlackselect_relatedandprefetch_relatedthat other endpoints use (e.g.,ticket_listat lines 75-77). SinceTicketSchema.from_ormaccessesstudent,category,priority, andattachments_tickets, this will cause multiple additional queries per request.♻️ Proposed fix
`@router.get`("/{id}", response={200: TicketSchema, 404: dict}) def ticket_detail(request, id: int): - ticket = get_object_or_404(Ticket.objects.annotate(comments_count=Count('comments')), id=id) + ticket = get_object_or_404( + Ticket.objects.select_related('category', 'priority', 'student') + .prefetch_related('attachments_tickets') + .annotate(comments_count=Count('comments')), + id=id + ) if ticket.student != request.user and not request.user.is_staff: return {"detail": "Not found."}, 404 return 200, TicketSchema.from_orm(ticket, request) `@router.get`("/number/{ticket_number}", response={200: TicketSchema, 404: dict}) def ticket_detail_by_number(request, ticket_number: str): - ticket = get_object_or_404(Ticket.objects.annotate(comments_count=Count('comments')), ticket_number=ticket_number) + ticket = get_object_or_404( + Ticket.objects.select_related('category', 'priority', 'student') + .prefetch_related('attachments_tickets') + .annotate(comments_count=Count('comments')), + ticket_number=ticket_number + ) if ticket.student != request.user and not request.user.is_staff: return {"detail": "Not found."}, 404 return 200, TicketSchema.from_orm(ticket, request)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/tickets/views.py` around lines 246 - 258, Both ticket_detail and ticket_detail_by_number fetch Ticket without the same select_related/prefetch_related used in ticket_list, causing N+1 queries when TicketSchema.from_orm accesses relations; update the queryset in both functions (the get_object_or_404 calls inside ticket_detail and ticket_detail_by_number) to include the same optimizations as ticket_list by adding .select_related('student', 'category', 'priority') and .prefetch_related('attachments_tickets') while keeping the annotate(Count('comments')) so related objects are loaded in a single query.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/apps/tickets/views.py`:
- Around line 246-258: Both ticket_detail and ticket_detail_by_number fetch
Ticket without the same select_related/prefetch_related used in ticket_list,
causing N+1 queries when TicketSchema.from_orm accesses relations; update the
queryset in both functions (the get_object_or_404 calls inside ticket_detail and
ticket_detail_by_number) to include the same optimizations as ticket_list by
adding .select_related('student', 'category', 'priority') and
.prefetch_related('attachments_tickets') while keeping the
annotate(Count('comments')) so related objects are loaded in a single query.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/apps/tickets/views.pyfrontend/src/lib/api/feedback.tsfrontend/src/lib/api/ticket.ts
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.
manki ko
Summary by Sourcery
Improve ticket detail endpoints and align feedback submission and error handling between backend and frontend.
New Features:
Enhancements:
detailmessages and structured validation errors.Summary by CodeRabbit
New Features
Bug Fixes