Repository navigation
Add delivery order history endpoint and validation schema - #133
Merged
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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 (2)
📝 WalkthroughWalkthroughSe añade un endpoint protegido GET /api/assignments/{id}/orders con validación Zod, paginación y un servicio que consulta el historial de asignaciones de entrega, plus esquema Swagger para la respuesta paginada y tests unitarios que cubren filtros y formato de respuesta. ChangesHistorial de órdenes de entrega
Sequence Diagram(s)sequenceDiagram
participant Client
participant Router
participant Middleware
participant Controller
participant Service
participant Database
Client->>Router: GET /api/assignments/{id}/orders?page=1&period=7d
Router->>Middleware: authenticate, requireRole, parsePagination
Middleware-->>Router: ok
Router->>Controller: getDeliveryOrderHistory(req)
Controller->>Controller: validar con deliveryOrderHistoryQuerySchema
Controller->>Service: getDeliveryOrderHistoryService(id, userId, filters, pagination)
Service->>Database: find delivery por id
Database-->>Service: delivery (o null)
Service->>Database: $transaction [findMany(filters,pagination), count(filters)]
Database-->>Service: assignments[], totalCount
Service->>Service: mapear contenido y convertir montos a Number
Service-->>Controller: respuesta paginada
Controller-->>Client: 200 {content, total_elements, total_pages, page, size}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: Turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value). 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. Review rate limit: 0/1 reviews remaining, refill in 52 minutes and 49 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/modules/delivery/delivery-assignments/delivery-assignments.routes.js`:
- Around line 158-163: El 400 response en este endpoint está apuntando al schema
DeliveryStatusErrorResponse (orientado a delivery_status) y debe usar un schema
de validación de query; update the OpenAPI response $ref in
delivery-assignments.routes.js to point to a proper query-validation schema
(e.g., QueryValidationErrorResponse or InvalidQueryErrorResponse), and if that
schema does not exist add it to components/schemas describing invalid query
params (message, invalidFields, etc.); ensure the 400 entry replaces $ref
'#/components/schemas/DeliveryStatusErrorResponse' with the new schema name and
keep response content/application/json structure unchanged.
In `@src/modules/delivery/delivery-assignments/delivery-assignments.service.js`:
- Line 380: The spread uses a truthy check "(...orderId && { fk_order: orderId
})" which skips valid falsy ids (e.g., 0); update the spread to only omit the
filter when orderId is strictly undefined by replacing that truthy check with an
explicit undefined check or a conditional that returns an empty object when
orderId is undefined, so fk_order is included for values like 0; target the
expression using orderId and fk_order in delivery-assignments.service.js.
🪄 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: 84015711-d0df-4190-b32e-96bc4fd724c7
📒 Files selected for processing (5)
src/docs/schemas/delivery.schema.jssrc/modules/delivery/delivery-assignments/delivery-assignments.controller.jssrc/modules/delivery/delivery-assignments/delivery-assignments.routes.jssrc/modules/delivery/delivery-assignments/delivery-assignments.service.jssrc/modules/delivery/delivery-assignments/delivery-assignments.validation.js
…elivery response reference
… with pagination and filtering
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/unit/delivery/delivery-assignments.test.js (1)
390-503: ⚡ Quick winFalta validar que
countreciba los mismos filtros quefindMany.Las pruebas de filtros hoy sólo chequean el
wheredefindMany. Sicountse desalinea, la paginación (total_elements,total_pages) puede romperse sin que el suite lo detecte.🧪 Cobertura sugerida (ejemplo)
it("filtra correctamente por orderId", async () => { prisma.deliveries.findUnique.mockResolvedValue(mockDeliveryForHistory); prisma.deliveryAssignments.findMany.mockResolvedValue([mockAssignmentHistory]); prisma.deliveryAssignments.count.mockResolvedValue(1); await getDeliveryOrderHistoryService(1, 5, { orderId: 100 }, mockPagination); expect(prisma.deliveryAssignments.findMany).toHaveBeenCalledWith( expect.objectContaining({ where: expect.objectContaining({ fk_order: 100 }) }) ); + expect(prisma.deliveryAssignments.count).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ fk_order: 100 }) + }) + ); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/delivery/delivery-assignments.test.js` around lines 390 - 503, The tests only assert that prisma.deliveryAssignments.findMany is called with the expected where filters but do not verify prisma.deliveryAssignments.count is invoked with the same filters, which can hide pagination bugs; update each filter-related test (e.g., the cases calling getDeliveryOrderHistoryService with period, assignment_status, orderId, userName, and the no-filters case) to also expect prisma.deliveryAssignments.count toHaveBeenCalledWith an objectContaining the same where clause (matching the same created_at, assignment_status, fk_order, or nested order.user.name filters or absence thereof) so count and findMany remain aligned for pagination calculations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/unit/delivery/delivery-assignments.test.js`:
- Line 325: Reemplaza la llamada en beforeEach que usa vi.clearAllMocks() por
vi.resetAllMocks() para asegurar que también se reinicien las implementaciones
(o, como alternativa, reinstala explícitamente cada mock antes de cada test),
especialmente los mocks configurados con mockResolvedValue(...) y la
implementación de $transaction usada en estos tests; busca la beforeEach que
contiene vi.clearAllMocks(), y cambia a vi.resetAllMocks() o añade reinstalación
explícita de mockImplementation/mockResolvedValue para $transaction y otros
mocks reutilizados.
---
Nitpick comments:
In `@tests/unit/delivery/delivery-assignments.test.js`:
- Around line 390-503: The tests only assert that
prisma.deliveryAssignments.findMany is called with the expected where filters
but do not verify prisma.deliveryAssignments.count is invoked with the same
filters, which can hide pagination bugs; update each filter-related test (e.g.,
the cases calling getDeliveryOrderHistoryService with period, assignment_status,
orderId, userName, and the no-filters case) to also expect
prisma.deliveryAssignments.count toHaveBeenCalledWith an objectContaining the
same where clause (matching the same created_at, assignment_status, fk_order, or
nested order.user.name filters or absence thereof) so count and findMany remain
aligned for pagination calculations.
🪄 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: d1ae53cf-c8a9-4665-abc3-a7061b659945
📒 Files selected for processing (4)
src/docs/schemas/delivery.schema.jssrc/modules/delivery/delivery-assignments/delivery-assignments.routes.jssrc/modules/delivery/delivery-assignments/delivery-assignments.service.jstests/unit/delivery/delivery-assignments.test.js
✅ Files skipped from review due to trivial changes (3)
- src/docs/schemas/delivery.schema.js
- src/modules/delivery/delivery-assignments/delivery-assignments.routes.js
- src/modules/delivery/delivery-assignments/delivery-assignments.service.js
…et mocks and implement transaction handling
|
leoAchu16
approved these changes
May 2, 2026
This was referenced May 5, 2026
Merged
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.
Summary by CodeRabbit
Nuevas Funcionalidades
Documentación
Tests