Repository navigation
Implement delivery assignment functionality and refactor order updates - #132
Merged
Merged
Conversation
…ion and service integration
…add delivery assignment logic
…order status updates
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 (5)
📝 WalkthroughWalkthroughSe implementa una nueva funcionalidad de respuesta a asignaciones de entregas que permite a usuarios con rol DELIVERY aceptar o rechazar pedidos asignados. Se agregó un endpoint POST, su correspondiente validación, servicio y tests. Se modificó Changes
Sequence Diagram(s)sequenceDiagram
actor DeliveryUser as Delivery User
participant Controller
participant Service as respondToAssignmentService
participant Database as Prisma
DeliveryUser->>Controller: POST /orders/:orderId/delivery-response<br/>{action: "ACCEPT" | "REJECT"}
activate Controller
Controller->>Controller: Validar payload con schema
alt Validación fallida
Controller-->>DeliveryUser: 400 Bad Request
else Validación exitosa
Controller->>Service: respondToAssignmentService(orderId, userId, action)
activate Service
Service->>Database: Buscar asignación PENDING<br/>para el orden y usuario
alt No existe asignación
Service-->>Controller: 404 Not Found
else Asignación existe pero usuario no coincide
Service-->>Controller: 403 Forbidden
else Usuario autorizado
alt action === "ACCEPT"
Service->>Database: Actualizar asignación a ACCEPTED
Service->>Database: Actualizar orden a SHIPPED
Service-->>Controller: 200 OK
else action === "REJECT"
Service->>Database: Actualizar asignación a REJECTED
Service->>Database: Buscar siguiente delivery activo<br/>sin asignación PENDING
alt Existe siguiente delivery
Service->>Database: Crear nueva asignación PENDING<br/>con siguiente delivery
Service-->>Controller: 200 OK
else No hay deliveries disponibles
Service->>Database: Revertir orden a PENDING
Service-->>Controller: 404 Not Found
end
end
end
deactivate Service
Controller-->>DeliveryUser: Response
end
deactivate Controller
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutos Possibly related PRs
Suggested reviewers
🚥 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)
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 38 minutes and 39 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: 3
🧹 Nitpick comments (2)
src/modules/delivery/delivery-assignments/delivery-assignments.routes.js (1)
41-42: 💤 Low valueConsiderar agregar comentario descriptivo.
Las otras rutas tienen comentarios descriptivos en español (ej. "// Marcar asignación de delivery como completado"). Para mantener la consistencia, sería bueno agregar uno similar para esta nueva ruta.
📝 Sugerencia
+// Responder a asignación de delivery (aceptar o rechazar) router.post("/orders/:orderId/delivery-response", authenticate, requireRole(ROLES.DELIVERY), respondToAssignment);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/delivery/delivery-assignments/delivery-assignments.routes.js` around lines 41 - 42, Agregar un comentario descriptivo en español encima de la ruta POST definida por router.post("/orders/:orderId/delivery-response", authenticate, requireRole(ROLES.DELIVERY), respondToAssignment); que siga el estilo de las demás rutas (por ejemplo "// Responder a la asignación de delivery" o similar) para mantener consistencia; ubica el comentario justo antes de la línea que contiene router.post y referencia la ruta y el handler respondToAssignment al escribir el texto.src/modules/users/orders/order.service.js (1)
909-940: ⚖️ Poor tradeoffDuplicación de lógica con
createAssignmentService.La lógica de selección automática de delivery (líneas 910-939) duplica código de
createAssignmentServiceendelivery-assignments.service.js. Ambos buscan un delivery ACTIVE sin asignaciones PENDING y crean una asignación con sequence incrementado.Considerar extraer esta lógica a una función compartida para evitar drift y facilitar mantenimiento.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/users/orders/order.service.js` around lines 909 - 940, El bloque en order.service.js que busca un delivery ACTIVE sin asignaciones PENDING y crea una deliveryAssignment duplica la lógica de createAssignmentService en delivery-assignments.service.js; refactoriza extrayendo esa lógica a una función compartida (por ejemplo exportar assignDeliveryToOrder/ createAssignmentForOrder) que acepte el transaction client (tx), fk_store, resolvedOrderId (o fk_order) y lance ValidationError si no hay delivery; la función debe realizar la misma consulta a tx.deliveries (delivery_status: "ACTIVE", status: true, delivery_assignments: { none: { assignment_status: "PENDING" } }), calcular assignment_sequence usando tx.deliveryAssignments.findFirst y crear la entrada con assignment_status "PENDING" y status true, y devolver la assignment creada; luego reemplaza el bloque en order.service.js para llamar a esa función y reutiliza createAssignmentService donde corresponda.
🤖 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.service.js`:
- Around line 14-40: The validation schema createAssignmentSchema currently
requires fk_delivery, preventing the service's auto-selection branch from ever
running; change the fk_delivery field in the schema from
z.number().int().positive("Delivery ID requerido") to an optional variant (e.g.,
z.number().int().positive("Delivery ID requerido").optional()) so the controller
can omit fk_delivery and allow delivery-assignments.service.js logic (the
fk_delivery_input handling and auto-selection using prisma.deliveries) to
execute; ensure any downstream code that expects fk_delivery handles undefined
accordingly.
In `@src/modules/users/orders/order.service.js`:
- Around line 878-943: The state-transition validation is done before the
transaction, allowing two concurrent requests to both pass and create duplicate
assignments; fix by performing the status-check and update atomically inside the
transaction (use tx.orders.update with a conditional where that includes
order_status: "PENDING" so the update fails if another request already changed
it) and, before creating a new assignment in the tx block, query
tx.deliveryAssignments.findFirst for { fk_order: resolvedOrderId,
assignment_status: "PENDING", status: true } and skip/throw if one exists
(mirror the logic in createAssignmentService). Ensure you replace the current
tx.orders.update and tx.deliveryAssignments.create flow to first do the
conditional update and then check existing PENDING assignment within the same tx
to avoid race conditions.
In `@tests/unit/order/order.test.js`:
- Around line 50-57: El mock actual cierra el objeto prisma demasiado pronto y
define deliveries y deliveryAssignments como propiedades externas; reubica las
propiedades deliveries (findFirst) y deliveryAssignments (findFirst, create)
dentro del objeto prisma en el mock para que queden como prisma.deliveries y
prisma.deliveryAssignments, ajustando la llave de cierre que actualmente termina
el objeto prisma para envolver esas entradas; busca el mock en order.test.js
donde se crea el objeto prisma y mueve/mergea las claves deliveries y
deliveryAssignments dentro de ese mismo objeto mock.
---
Nitpick comments:
In `@src/modules/delivery/delivery-assignments/delivery-assignments.routes.js`:
- Around line 41-42: Agregar un comentario descriptivo en español encima de la
ruta POST definida por router.post("/orders/:orderId/delivery-response",
authenticate, requireRole(ROLES.DELIVERY), respondToAssignment); que siga el
estilo de las demás rutas (por ejemplo "// Responder a la asignación de
delivery" o similar) para mantener consistencia; ubica el comentario justo antes
de la línea que contiene router.post y referencia la ruta y el handler
respondToAssignment al escribir el texto.
In `@src/modules/users/orders/order.service.js`:
- Around line 909-940: El bloque en order.service.js que busca un delivery
ACTIVE sin asignaciones PENDING y crea una deliveryAssignment duplica la lógica
de createAssignmentService en delivery-assignments.service.js; refactoriza
extrayendo esa lógica a una función compartida (por ejemplo exportar
assignDeliveryToOrder/ createAssignmentForOrder) que acepte el transaction
client (tx), fk_store, resolvedOrderId (o fk_order) y lance ValidationError si
no hay delivery; la función debe realizar la misma consulta a tx.deliveries
(delivery_status: "ACTIVE", status: true, delivery_assignments: { none: {
assignment_status: "PENDING" } }), calcular assignment_sequence usando
tx.deliveryAssignments.findFirst y crear la entrada con assignment_status
"PENDING" y status true, y devolver la assignment creada; luego reemplaza el
bloque en order.service.js para llamar a esa función y reutiliza
createAssignmentService donde corresponda.
🪄 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: 464204cf-8496-44ad-9d98-27ba6bd02e89
📒 Files selected for processing (8)
src/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.jssrc/modules/users/orders/.gitkeepsrc/modules/users/orders/order.service.jstests/unit/delivery/delivery-assignments.test.jstests/unit/order/order.test.js
…oint and update delivery schemas
…l and enhance order status update logic with atomic condition and conflict handling Co-authored-by: Copilot <copilot@github.com>
|
This was referenced May 4, 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
Notas de Lanzamiento