Repository navigation
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 (6)
📝 WalkthroughWalkthroughSe añaden funcionalidades de entregas y asignaciones: nuevos controladores, servicios, rutas y validaciones; se actualiza el montaje de routers en Changes
Sequence Diagram(s)sequenceDiagram
participant Cliente as Cliente
participant API as API (Express)
participant Auth as Auth Middleware
participant Service as Servicio (assignment.service)
participant DB as Prisma / Base de datos
rect rgba(0, 128, 0, 0.5)
Cliente->>API: POST /api/assignments/:id/complete (token)
end
API->>Auth: verificar JWT / inyectar req.user
Auth-->>API: usuario autenticado
API->>Service: completeAssignment(req.params.id, req.user.id_user)
Service->>DB: deliveryAssignments.findUnique(id)
DB-->>Service: assignment record
Service->>DB: $transaction([ deliveryAssignments.update status=DELIVERED, orders.update order_status=DELIVERED ])
DB-->>Service: transaction result
Service-->>API: resultado (updated assignment)
API-->>Cliente: 200 { assignment_status: "DELIVERED" }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poema
🚥 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 59 minutes and 12 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 (1)
tests/unit/commerce/delivery.test.js (1)
48-61: Sumá aserciones sobre los llamados a Prisma.Hoy la suite valida status y body, pero no que
findMany,findUniqueycreatereciban elwhere/datacorrecto. Un cambio accidental en los filtros o en el payload podría pasar igual.Also applies to: 129-149
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/commerce/delivery.test.js` around lines 48 - 61, Add assertions that Prisma methods receive the expected query/payload: in the search test assert prisma.users.findMany was called with an object containing the correct where filter derived from the q param (e.g. a where with name/email contains 'del' or the exact filter your handler builds) and not just that it was called; likewise update the other tests (the ones covering lines ~129-149) to assert prisma.users.findUnique is called with the correct where:{ id: ... } and prisma.users.create is called with the correct data:{ ... } payload that the endpoint should send. Use the existing mocked prisma.users.findMany / findUnique / create symbols to check the actual argument objects passed.
🤖 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/commerce/deliveries/delivery.controller.js`:
- Around line 20-27: Validate that req.body.fk_user is a numeric ID before
calling createDeliveryService: parse it (e.g., const fkUser =
Number(req.body.fk_user)), check Number.isInteger(fkUser) && fkUser > 0 (or
other allowed range), and if the check fails return res.status(400).json({
message: "fk_user debe ser un id numérico válido" }); then call
createDeliveryService(req.user?.id_user, id, fkUser) with the parsed number.
Ensure you keep the existing early-return for missing fk_user but prefer the
numeric check to catch non-numeric strings.
In `@src/modules/commerce/deliveries/delivery.routes.js`:
- Around line 8-12: El endpoint registrado como deliveryRouter.get("/search",
authenticate, searchDeliveries) expone email/phone de repartidores a cualquier
usuario autenticado; restringí acceso añadiendo la autorización adecuada: o
insertar el middleware requireRole(...) (por ejemplo requireRole('store') o el
rol específico de comercios habilitados) entre authenticate y searchDeliveries,
o validar en la función searchDeliveries que el usuario tenga el scope/permiso
para vincular repartidores (y rechazar con 403 si no). Asegurate de usar la
misma política de autorización que el resto del módulo y mantener el mensaje de
error consistente.
In `@src/modules/commerce/deliveries/delivery.service.js`:
- Around line 33-43: El primer uso de prisma.users.findUnique es incorrecto
porque el filtro mezcla id_user con role y status; reemplaza esa llamada por
prisma.users.findFirst({ where: { id_user: deliveryUserId, role: 'DELIVERY',
status: true } }) para que la consulta acepte filtros no únicos (la variable
afectada es user); deja la segunda llamada prisma.deliveries.findUnique({ where:
{ fk_user: deliveryUserId } }) tal como está porque fk_user es único.
---
Nitpick comments:
In `@tests/unit/commerce/delivery.test.js`:
- Around line 48-61: Add assertions that Prisma methods receive the expected
query/payload: in the search test assert prisma.users.findMany was called with
an object containing the correct where filter derived from the q param (e.g. a
where with name/email contains 'del' or the exact filter your handler builds)
and not just that it was called; likewise update the other tests (the ones
covering lines ~129-149) to assert prisma.users.findUnique is called with the
correct where:{ id: ... } and prisma.users.create is called with the correct
data:{ ... } payload that the endpoint should send. Use the existing mocked
prisma.users.findMany / findUnique / create symbols to check the actual argument
objects passed.
🪄 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: 65ab4f15-fae9-4d47-a0f9-d3c96f107070
📒 Files selected for processing (5)
src/app.jssrc/modules/commerce/deliveries/delivery.controller.jssrc/modules/commerce/deliveries/delivery.routes.jssrc/modules/commerce/deliveries/delivery.service.jstests/unit/commerce/delivery.test.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
src/modules/commerce/deliveries/delivery.service.js (1)
5-24: Recomendado: normalizar query y limitar resultados de búsqueda.En Line 6 y en la consulta de Line 8-24, faltan
trim+take/orden. Así evitás búsquedas vacías con espacios y respuestas demasiado grandes en endpoints públicos de búsqueda.💡 Propuesta de ajuste
export const searchDeliveryCandidatesService = async (query) => { - if (!query) return []; + const normalizedQuery = query?.trim(); + if (!normalizedQuery) return []; const candidates = await prisma.users.findMany({ where: { role: 'DELIVERY', status: true, OR: [ - { email: { contains: query, mode: 'insensitive' } }, - { phone: { contains: query, mode: 'insensitive' } } + { email: { contains: normalizedQuery, mode: 'insensitive' } }, + { phone: { contains: normalizedQuery, mode: 'insensitive' } } ], delivery: null }, + take: 20, + orderBy: { id_user: "desc" }, select: { id_user: true, name: true, email: true, phone: true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/commerce/deliveries/delivery.service.js` around lines 5 - 24, La función searchDeliveryCandidatesService no normaliza el término de búsqueda ni limita/ordena resultados; primero trimpea el parámetro query (p. ej. const q = query?.trim()) y cambiar el early return para devolver [] si q es vacío, luego pasar q a prisma.users.findMany en lugar de query, y añadir opciones de paginación y orden determinista (por ejemplo take: <n> y orderBy: { name: 'asc' } o similar) en la llamada a prisma.users.findMany para evitar búsquedas por espacios y respuestas demasiado grandes; referencia los símbolos searchDeliveryCandidatesService y prisma.users.findMany al actualizar.src/modules/delivery/delivery/delivery.validation.js (1)
14-28: Modeládelivery_statuscomo enum requerido, no con dosrefine.Con un body vacío terminás generando dos
issuespara el mismo campo (es requeridoydebe ser ACTIVE o INACTIVE). Unz.enum(["ACTIVE", "INACTIVE"])requerido deja una sola validación, simplifica el schema y evita lógica duplicada.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/delivery/delivery/delivery.validation.js` around lines 14 - 28, The current updateDeliveryStatusSchema uses delivery_status: z.string().optional() plus two .refine calls which produces duplicate errors; replace that field with a required enum by defining delivery_status: z.enum(["ACTIVE","INACTIVE"]) inside updateDeliveryStatusSchema so the schema enforces presence and allowed values in one validation (remove the optional and both .refine calls).
🤖 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/commerce/deliveries/delivery.service.js`:
- Around line 31-35: Validate deliveryUserIdStr before calling prisma: parse an
integer from deliveryUserIdStr, ensure it's a finite positive integer
(deliveryUserId > 0) and if invalid return a 400 response early; only then set
deliveryUserId = Number(...) and call prisma.users.findFirst with where: {
id_user: deliveryUserId, role: 'DELIVERY', status: true }. Add clear error
handling for the invalid id path instead of letting prisma receive NaN.
- Around line 41-58: Reemplazar el patrón "findUnique -> create" por una
operación atómica: intenta directamente crear con prisma.deliveries.create
dentro de un bloque try/catch y, en caso de error de unicidad (Prisma error
P2002), consulta el registro existente (usando prisma.deliveries.findUnique con
where: { fk_user }) para decidir si lanzar ConflictError("El delivery ya está
vinculado a este comercio") o ConflictError("El delivery ya está vinculado a un
comercio"); de este modo evitas la condición de carrera entre
prisma.deliveries.findUnique y prisma.deliveries.create y mantienes la lógica de
mensajes basada en fk_store/fk_user.
In
`@src/modules/delivery/delivery-assignments/delivery-assignments.controller.js`:
- Around line 37-42: The current ID parsing uses Number.parseInt(id, 10) which
accepts partially numeric strings (e.g., "123abc"); update the validation to
first assert the raw id matches a strict digits-only regex (/^\d+$/) before
converting to Number so only purely numeric IDs are accepted, then parse with
Number.parseInt and keep the existing Number.isNaN check; apply this change to
every occurrence where assignmentId (and similar id parsing) is done in this
file (the blocks around the current parse at the assignmentId declaration and
the other occurrences flagged at lines ~58-63, ~80-85, ~109-114, ~130-135,
~151-156) so all endpoints reject non-numeric or partially-numeric id inputs.
In `@src/modules/delivery/delivery-assignments/delivery-assignments.routes.js`:
- Around line 23-35: Routes for delivery assignments (router.get("/:id",
authenticate, getAssignmentById), router.get("/orders/:orderId/assignments",
authenticate, getOrderAssignments),
router.get("/deliveries/:deliveryId/assignments", authenticate,
getDeliveryAssignments), router.get("/deliveries/:deliveryId/pending",
authenticate, getDeliveryPendingAssignments),
router.get("/orders/:orderId/accepted", authenticate, getAcceptedAssignment))
currently only enforce authentication but not authorization or resource
ownership/tenancy; add/attach an authorization middleware (e.g., authorizeRole
or authorizeResource) to these routes to restrict by allowed roles and then
validate ownership/tenancy of the target resource (orderId/deliveryId) before
calling the service handlers, and modify getAcceptedAssignment/get* handlers to
strip sensitive contact info if the requester is not owner/allowed role. Ensure
the middleware checks user.role and user.tenantId (or user.id) against the
resource via existing services (e.g., order/delivery lookup) and return 403 when
unauthorized.
In `@src/modules/delivery/delivery-assignments/delivery-assignments.service.js`:
- Around line 4-65: createAssignmentService must validate that the authenticated
user owns the order before creating an assignment: change the signature of
createAssignmentService to accept the authenticated user (e.g., user or
authUser) in addition to data, after fetching the order with
prisma.orders.findUnique check that order.fk_seller (or whatever owner field
your orders table uses, e.g., fk_commerce/fk_store) matches the authenticated
user's id/merchant id; if it does not match, throw { status: 403, message:
"Acceso denegado" } and do not proceed to the transaction; keep the existing
PENDING/sequence checks (deliveryAssignments logic) unchanged.
- Around line 196-229: The status pre-check allows a race where two requests
both see ACCEPTED; move the status guard inside the prisma.$transaction by
performing the deliveryAssignments.update with a where that includes both
id_delivery_assignment and assignment_status: "ACCEPTED" (so it only matches if
still ACCEPTED), then check the update result (or catch a
NotFound/RecordNotUpdated) and throw 409 if no row was updated; keep the
ownership check (assignment.delivery?.fk_user !== id_user) before the
transaction but ensure the transaction updates deliveryAssignments (function
completeAssignmentService, call to prisma.$transaction and
tx.deliveryAssignments.update) and then updates orders, and if the conditional
update failed, abort the transaction and return the proper 409 error "Solo se
pueden completar asignaciones aceptadas".
In `@src/modules/delivery/delivery/delivery.controller.js`:
- Around line 37-42: The ID parsing is too permissive — replace the current
Number.parseInt-based validation in the delivery controller with a strict
decimal-digit check (e.g. test the incoming id param against /^\d+$/) before
converting; update the logic around the deliveryId creation in the handler that
currently uses Number.parseInt(id, 10) so it returns 400 on non-digit input and
only then does Number.parseInt/Number(...) to get the numeric id. Apply the same
strict-digit validation to the other handlers mentioned (getPendingAssignments
and getDeliveryById) so each uses the /^\d+$/ check on the id/path param,
returns the same 400 JSON error shape when invalid, and only then proceeds to
convert and use the numeric id.
In `@src/modules/delivery/delivery/delivery.service.js`:
- Around line 61-84: The service getPendingAssignmentsService currently fetches
by id_delivery without verifying ownership; update its signature to accept the
authenticated user's id (e.g., id_user) and enforce ownership in the DB query
(either add fk_user: id_user to the where clause or verify delivery.fk_user ===
id_user after fetch) and throw a 403 when it doesn't belong to the caller; apply
the same change to getDeliveryByIdService and update the controller to pass
req.user.id_user into both service calls so a delivery cannot be read by another
user.
- Around line 12-24: La verificación con findUnique no evita la condición de
carrera; envuelve la llamada a prisma.users.create (donde se asigna newUser) en
un try/catch y captura errores de Prisma verificando que sean
PrismaClientKnownRequestError con code === 'P2002' (violación de unique
constraint) para volver a lanzar { status: 409, message: "Email ya registrado"
}; deja el bcrypt.hash(password, SALT_ROUNDS) tal cual y solo mapea
explícitamente P2002 a 409, re-lanzando el error original para cualquier otro
caso, referenciando prisma.users.create y la variable existingUser/newUser para
ubicar el bloque a cambiar.
---
Nitpick comments:
In `@src/modules/commerce/deliveries/delivery.service.js`:
- Around line 5-24: La función searchDeliveryCandidatesService no normaliza el
término de búsqueda ni limita/ordena resultados; primero trimpea el parámetro
query (p. ej. const q = query?.trim()) y cambiar el early return para devolver
[] si q es vacío, luego pasar q a prisma.users.findMany en lugar de query, y
añadir opciones de paginación y orden determinista (por ejemplo take: <n> y
orderBy: { name: 'asc' } o similar) en la llamada a prisma.users.findMany para
evitar búsquedas por espacios y respuestas demasiado grandes; referencia los
símbolos searchDeliveryCandidatesService y prisma.users.findMany al actualizar.
In `@src/modules/delivery/delivery/delivery.validation.js`:
- Around line 14-28: The current updateDeliveryStatusSchema uses
delivery_status: z.string().optional() plus two .refine calls which produces
duplicate errors; replace that field with a required enum by defining
delivery_status: z.enum(["ACTIVE","INACTIVE"]) inside updateDeliveryStatusSchema
so the schema enforces presence and allowed values in one validation (remove the
optional and both .refine calls).
🪄 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: 944d3996-f8c3-44ce-a2c0-2036502f1829
📒 Files selected for processing (17)
prisma/migrations/20260428223931_add_delivered_to_assignment_status/migration.sqlprisma/schema.prismasrc/app.jssrc/modules/commerce/deliveries/delivery.controller.jssrc/modules/commerce/deliveries/delivery.routes.jssrc/modules/commerce/deliveries/delivery.service.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.jssrc/modules/delivery/delivery/delivery.controller.jssrc/modules/delivery/delivery/delivery.routes.jssrc/modules/delivery/delivery/delivery.service.jssrc/modules/delivery/delivery/delivery.validation.jstests/unit/delivery/delivery-complete.test.jstests/unit/delivery/delivery-register.test.jstests/unit/delivery/delivery-status.test.js
✅ Files skipped from review due to trivial changes (4)
- tests/unit/delivery/delivery-status.test.js
- prisma/migrations/20260428223931_add_delivered_to_assignment_status/migration.sql
- prisma/schema.prisma
- src/modules/commerce/deliveries/delivery.routes.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/modules/commerce/deliveries/delivery.controller.js
|
CrisNAC
approved these changes
Apr 29, 2026
This was referenced Apr 30, 2026
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 Características
Tests
Chores