feat: add Queue Management APIs#8
Conversation
- GET /api/queues/:departmentId/current (current serving ticket) - POST /api/queues/:departmentId/next (admin only, move next to serving) - GET /api/queues/:departmentId/waiting (waiting tickets list) - Admin-only middleware for role-based access control - 24 new tests with 100% coverage Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds admin-only middleware, three queue controller endpoints for department-scoped ticket management, a router mounted at /api/queues, and Jest/Supertest tests for middleware, controllers, and routes. ChangesQueue Management System
Sequence DiagramsequenceDiagram
participant Client
participant AuthMiddleware
participant AdminMiddleware
participant QueueController
participant TicketModel
participant Response
Client->>AuthMiddleware: Request to /api/queues/:departmentId/next (JWT)
AuthMiddleware->>AdminMiddleware: Forward authenticated request
AdminMiddleware->>AdminMiddleware: Check req.user.role === 'admin'
alt Non-admin or missing user
AdminMiddleware->>Response: 403 { success: false, message: 'Access denied. Admin only.' }
Response->>Client: 403
else Admin user
AdminMiddleware->>QueueController: callNext(req)
QueueController->>TicketModel: Find oldest waiting (LOCK.UPDATE, skipLocked)
TicketModel-->>QueueController: waiting ticket (or null)
alt No waiting
QueueController->>Response: 404 { success: false, message: 'No waiting tickets' }
Response->>Client: 404
else Promote ticket
QueueController->>TicketModel: Find current serving (FOR UPDATE)
TicketModel-->>QueueController: current serving (or null)
QueueController->>TicketModel: Mark current completed & save (if exists)
QueueController->>TicketModel: Update waiting -> serving & save
QueueController->>Response: 200 { success: true, ticket: {...} }
Response->>Client: 200
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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)
Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Wrap findOne/save in sequelize.transaction with LOCK.UPDATE - Complete currently serving ticket before promoting next - Use skipLocked to prevent duplicate selection under concurrency - Add test for completing current ticket before next promotion Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
| if (currentServing) { | ||
| currentServing.status = 'completed'; | ||
| await currentServing.save({ transaction: t }); | ||
| } | ||
|
|
||
| // Promote next waiting ticket with row lock | ||
| const ticket = await Ticket.findOne({ | ||
| where: { departmentId, status: 'waiting' }, | ||
| order: [['createdAt', 'ASC']], | ||
| transaction: t, | ||
| lock: t.LOCK.UPDATE, | ||
| skipLocked: true, | ||
| }); | ||
| if (!ticket) return null; |
There was a problem hiding this comment.
🔴 callNext silently completes the currently serving ticket even when no waiting tickets exist
In callNext, the transaction first marks the currently serving ticket as 'completed' (lines 58-61), then looks for the next waiting ticket (lines 64-70). If no waiting ticket is found, the transaction callback returns null without throwing, so the transaction commits. The currently serving ticket is now permanently marked 'completed' in the database, but the API responds with a 404 "No waiting tickets" error. The admin has no indication that the current customer's ticket was just completed as a side effect of a failed operation. This leaves the department in a state where no ticket is being served, which is likely unintended.
Scenario walkthrough
- Department has one
'serving'ticket and zero'waiting'tickets. - Admin calls
POST /api/queues/:departmentId/next. - Transaction:
currentServingfound → status set to'completed'→ saved. - Transaction:
ticket(next waiting) →null→ returnnull. - Transaction commits (no error thrown).
- Response:
404 { message: 'No waiting tickets' }. - The serving ticket is now completed. No ticket is serving.
Prompt for agents
In callNext (backend/src/controllers/queue.controller.js), the transaction unconditionally marks the current serving ticket as completed before checking if a next waiting ticket exists. If no waiting ticket is found, the transaction still commits, leaving the department with no serving ticket.
Two possible fixes:
1. Only complete the current ticket if a next ticket exists: move the currentServing completion after the next-waiting lookup, so both updates happen atomically only when there is a next ticket.
2. Throw an error inside the transaction when no waiting ticket is found (after checking) to trigger a rollback, preserving the current serving ticket's state. For example, throw a custom error that the outer catch can detect and return the 404 response.
Option 1 is cleaner:
- First, find the next waiting ticket with the lock.
- If no waiting ticket, return null immediately (without touching currentServing).
- If found, then complete the current serving ticket and promote the waiting ticket.
This ensures the current ticket is only completed when there is someone to replace it.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed — reordered the transaction logic to find the next waiting ticket first. The current serving ticket is now only completed when a replacement exists, preventing the department from ending up with no active ticket on an empty queue.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 participant
Summary
Adds three queue management endpoints under
/api/queues/:departmentId/:/current/nextserving/waitingKey additions:
queue.controller.js— three handlers with department existence validationadminOnlymiddleware — checksreq.user.role === 'admin', returns 403 otherwisequeue.routes.js— wiresauthenticateglobally +adminOnlyon thenextendpointLink to Devin session: https://app.devin.ai/sessions/b08d56b5cecf4545a99acb077c5d872d
Requested by: @0xpritam
Summary by CodeRabbit
New Features
Access Control
Tests