Repository navigation
OM-467(reportes): implementar reporte de producto para clientes y resolución para admin - #121
Merged
Merged
Conversation
added 3 commits
April 21, 2026 14:36
…stración Contexto: - Primera funcionalidad de moderación de contenido del panel admin - Los productos tienen approval_status (PENDING/ACTIVE/REJECTED) y visible - Al aprobar: producto visible en catálogo público - Al rechazar: producto oculto + notificación automática al vendedor - Sigue el mismo patrón de aprobación/rechazo que ya existía en admin/stores Endpoints: - GET /api/admin/products — lista productos con filtros y paginación - PATCH /api/admin/products/:id/status — aprueba o rechaza un producto Filtros del GET: - search: busca por nombre de producto o nombre de tienda (case insensitive) - approvalStatus: PENDING | ACTIVE | REJECTED - page + limit: paginación vía parsePagination middleware Lógica del PATCH: - ACTIVE: approval_status=ACTIVE, visible=true, rejection_reason=null - REJECTED: approval_status=REJECTED, visible=false, rejection_reason=reason requerido - Validación de estado idempotente (evita re-aprobar o re-rechazar) - Notificación al vendedor creada en la misma transacción al rechazar
…olución para admin
Contexto:
El módulo de reportes de productos ya existía con funcionalidades para Admin y Seller
(listar, filtrar, actualizar estado). Este ticket extiende ese módulo con las
capacidades del lado del cliente y del administrador para cerrar el ciclo completo.
Cambios en product-report.service.js:
- Se agrega PRODUCT_REPORT_REASONS: catálogo exportado con { value, label } en español.
Es la fuente única de verdad para los motivos válidos: usado en la validación de
createProductReportService y expuesto vía el endpoint /reasons para el frontend.
- getProductReportReasonsService: devuelve el catálogo sin lógica adicional ni DB.
- createProductReportService: cliente crea un reporte de producto.
Valida productId (entero positivo), reason contra el catálogo, verifica que el
producto exista y esté activo (status=true), y previene duplicados lanzando
ConflictError si ya existe un reporte PENDING del mismo usuario para el mismo producto.
- checkProductReportService: cliente consulta si ya tiene un reporte PENDING para
un producto. Retorna { hasReport: boolean, reportId: number | null }.
- resolveProductReportAdminService: admin cierra un reporte como RESOLVED o REJECTED.
Requiere notes obligatorias y no permite modificar reportes ya cerrados.
Cambios en product-report.controller.js:
- getProductReportReasons: controlador síncrono (sin try/catch ni DB) para el catálogo.
- createProductReport: 201 Created al crear el reporte.
- checkProductReport: lee productId desde query params.
- resolveProductReport: lee reportId desde params y { status, notes } desde body.
Cambios en product-report.routes.js:
- GET /api/reports/products/reasons → sin auth, catálogo para el frontend
- POST /api/reports/products → requireRole(CUSTOMER)
- GET /api/reports/products/check?productId= → requireRole(CUSTOMER)
- PATCH /api/reports/products/:reportId/resolve → requireRole(ADMIN)
Tests (product-report.test.js):
- Se actualiza el mock de Prisma agregando products.findFirst y productReports.create.
- getProductReportReasonsService: 4 tests.
- createProductReportService: 9 tests.
- checkProductReportService: 3 tests.
- resolveProductReportAdminService: 12 tests.
Total: 67 tests pasando.
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 16 minutes and 12 seconds. ⌛ 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 (3)
📝 WalkthroughDescripción GeneralSe agrega un módulo completo de gestión de productos para administradores con aprobación de estado, junto con expansión de funcionalidades de reportes de productos incluyendo creación, validación y resolución de reportes con notificaciones condicionales. Cambios
Diagrama de SecuenciasequenceDiagram
actor Admin as Administrador
participant Route as Ruta admin/products
participant Controller as Controlador
participant Service as Servicio
participant DB as Base de Datos
participant Notif as Servicio Notificaciones
Admin->>Route: PATCH /api/admin/products/:id/status<br/>{status, reason}
Route->>Controller: updateProductStatus(req, res, next)
rect rgba(100, 150, 255, 0.5)
Note over Controller: Validación de autenticación
Controller->>Service: updateProductApprovalStatusService(adminId, productId, {status, reason})
end
rect rgba(100, 200, 100, 0.5)
Note over Service: Validación de entrada
Service->>Service: Validar status (ACTIVE/REJECTED)
Service->>Service: Validar reason si REJECTED
end
rect rgba(200, 200, 100, 0.5)
Note over Service: Operaciones en BD
Service->>DB: findFirst(producto activo)
DB-->>Service: producto
Service->>Service: Verificar estado actual != nuevo estado
end
alt Status = REJECTED
rect rgba(255, 150, 100, 0.5)
Note over Service: Rechazo con notificación
Service->>DB: update(rejection_reason, visible=false)
Service->>Notif: create(notificación al vendedor)
DB-->>Service: ok
Notif-->>Service: ok
end
else Status = ACTIVE
rect rgba(150, 255, 100, 0.5)
Note over Service: Aprobación sin notificación
Service->>DB: update(clear rejection_reason, visible=true)
DB-->>Service: ok
end
end
Service-->>Controller: producto actualizado
Controller-->>Admin: 200 JSON{producto}
Esfuerzo estimado de revisión del código🎯 4 (Complejo) | ⏱️ ~45 minutos PRs posiblemente relacionados
Revisores sugeridos
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. 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
🧹 Nitpick comments (8)
src/modules/admin/products/admin-products.service.js (1)
139-162: Reusarreasonnormalizado.
reason.toString().trim()se calcula en Line 142 y Line 159. Computarlo una sola vez mejora legibilidad y evita divergencias futuras.♻️ Refactor propuesto
+ const trimmedReason = isApproving ? null : reason.toString().trim(); + const updateData = { approval_status: normalizedStatus, visible: isApproving, - rejection_reason: isApproving ? null : reason.toString().trim(), + rejection_reason: trimmedReason, }; @@ - prisma.notifications.create({ - data: { - fk_user: product.store.fk_user, - title: "Producto rechazado", - message: `Tu producto "${product.name}" ha sido rechazado. Motivo: ${reason.toString().trim()}`, - }, - }), + prisma.notifications.create({ + data: { + fk_user: product.store.fk_user, + title: "Producto rechazado", + message: `Tu producto "${product.name}" ha sido rechazado. Motivo: ${trimmedReason}`, + }, + }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/admin/products/admin-products.service.js` around lines 139 - 162, Compute a single normalizedReason once (e.g., const normalizedReason = reason.toString().trim()) and use that variable in updateData.rejection_reason and in the notifications.create message instead of calling reason.toString().trim() twice; keep the existing logic that sets rejection_reason to null when isApproving is true and use normalizedReason only when creating the rejection notification and the message string.tests/unit/admin/admin-products.test.js (1)
260-284: Cobertura sugerida: validar formato deproductIdno numérico.Hay test para
productIdnegativo, pero no para strings no numéricos (p. ej.,"abc") ni0. Dado queproductIdviene dereq.params.id(siempre string), vale la pena una aserción explícita para esos casos, especialmenteNumber("abc") → NaN.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/admin/admin-products.test.js` around lines 260 - 284, Add tests covering non-numeric and zero productId inputs for updateProductApprovalStatusService: add one test that calls updateProductApprovalStatusService(1, "abc", { status: "ACTIVE" }) (or with params coming from req like "abc") and expects a ValidationError, and another that calls updateProductApprovalStatusService(1, 0, { status: "ACTIVE" }) (and/or "0") and expects a ValidationError; ensure tests use the same mocking pattern (e.g., prisma.products.findFirst mocked where needed) and mirror existing negative/REJECTED test structure so these invalid productId cases are asserted consistently.src/modules/admin/products/admin-products.controller.js (1)
16-30: Chequeo dereq.userredundante e inconsistente con el resto del módulo.La ruta ya aplica
authenticate+requireRole(ROLES.ADMIN)(veradmin-products.routes.jsLine 14), por lo quereq.user.id_usersiempre existirá al llegar aquí; si no, cualquier falla de auth se gestiona víanext(ForbiddenError/UnauthorizedError)y elerrorHandlerglobal. Además, la respuesta ad-hoc{ success: false, message: ... }rompe la convención de respuestas del resto del controlador (que devuelve el payload del servicio directamente). Sugerido eliminar el bloque.♻️ Cambio propuesto
export const updateProductStatus = async (req, res, next) => { try { - if (!req.user?.id_user) { - return res.status(401).json({ success: false, message: "Usuario autenticado requerido" }); - } const result = await updateProductApprovalStatusService( req.user.id_user, req.params.id, req.body ); return res.status(200).json(result); } catch (error) { next(error); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/admin/products/admin-products.controller.js` around lines 16 - 30, The updateProductStatus controller contains a redundant req.user check and an ad-hoc 401 JSON response that breaks the module's response convention; remove the entire if (!req.user?.id_user) branch and its custom res.status(401)... return so the function always calls updateProductApprovalStatusService(req.user.id_user, req.params.id, req.body) and returns res.status(200).json(result) (keeping the existing try/catch and next(error)), thereby relying on the authenticate/requireRole middleware to handle auth errors and preserving consistent service-returned payloads.src/modules/admin/products/admin-products.routes.js (1)
11-14: Rutas correctamente protegidas conauthenticate+requireRole(ROLES.ADMIN).Cadena de middlewares consistente con el resto de módulos admin;
parsePaginationsolo se aplica alGET /como corresponde. Considerar, como mejora opcional, validar el body dePATCH /:id/statuscon un validador (zod/joi) a nivel de middleware en lugar de delegar toda la validación al servicio; hoy el servicio ya cubre los casos, por lo que es un nice-to-have.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/admin/products/admin-products.routes.js` around lines 11 - 14, Routes are correctly protected, but add request-body validation for PATCH /:id/status instead of relying solely on the service; create a middleware (e.g., validateProductStatusBody) using your chosen schema library (zod or joi) that enforces { status: "ACTIVE" | "REJECTED", reason?: string } and returns 400 on validation failure, then insert that middleware into the route chain before updateProductStatus in admin-products.routes.js (router.patch("/:id/status", authenticate, requireRole(ROLES.ADMIN), validateProductStatusBody, updateProductStatus)) so updateProductStatus receives already-validated input.src/modules/global/reports/product/product-report.service.js (2)
237-270: Condición de carrera en la verificación de duplicados.El patrón
findFirst+createes TOCTOU: dos requests concurrentes del mismo cliente sobre el mismo producto pueden pasar el chequeo antes de que cualquiera inserte, terminando con dos reportesPENDINGduplicados y evadiendo elConflictError. Para reportes desde UI es poco probable pero no imposible (doble clic, reintentos de red).Como respaldo a nivel de datos, te recomiendo agregar un índice parcial único en Postgres sobre
(fk_product, fk_reporter)cuandoreport_status = 'PENDING'ystatus = true, y capturarP2002en elcreatepara relanzarlo comoConflictError. Esto garantiza la unicidad incluso ante concurrencia.Ejemplo de migración (Prisma no soporta índices parciales de forma declarativa, usar
Unsupported/migrationSQL):CREATE UNIQUE INDEX IF NOT EXISTS uq_product_reports_pending_reporter ON "ProductReports" (fk_product, fk_reporter) WHERE report_status = 'PENDING' AND status = true;Y en el servicio:
try { - const report = await prisma.productReports.create({ ... }); + const report = await prisma.productReports.create({ ... }); + return report; + } catch (e) { + if (e?.code === "P2002") throw new ConflictError("Ya tenés un reporte pendiente para este producto"); + throw e; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/global/reports/product/product-report.service.js` around lines 237 - 270, The current TOCTOU check using prisma.productReports.findFirst followed by prisma.productReports.create can produce duplicate PENDING reports under concurrent requests; add a Postgres partial unique index on (fk_product, fk_reporter) WHERE report_status = 'PENDING' AND status = true (e.g. CREATE UNIQUE INDEX ...), then wrap the prisma.productReports.create call in a try/catch that detects Prisma's P2002 unique-constraint error and rethrows a ConflictError("Ya tenés un reporte pendiente para este producto"); keep the existing findFirst check for fast-fail UX but rely on the DB constraint plus P2002 handling around productReports.create to guarantee correctness.
10-20: Catálogo mutable compartido por referencia.
getProductReportReasonsServicedevuelve la misma referencia quePRODUCT_REPORT_REASONS, que también es la fuente única para validarreasonencreateProductReportService. Si algún consumidor (handler, middleware, etc.) llega a mutar el array o alguno de sus objetos, queda corrompida la validación y todas las respuestas futuras deGET /reasons. Como protección barata, considerá congelar el catálogo.♻️ Sugerencia
-export const PRODUCT_REPORT_REASONS = [ - { value: "DEFECTIVE", label: "Producto en mal estado" }, - { value: "WRONG_ITEM", label: "Producto incorrecto o no es lo que pedí" }, - { value: "MISSING_ITEM", label: "Producto incompleto o faltante" }, - { value: "LATE_DELIVERY", label: "Entrega tardía o no recibida" }, - { value: "CUSTOMER_SERVICE", label: "Mala atención del vendedor" }, - { value: "OTHER", label: "Otro" }, -]; +export const PRODUCT_REPORT_REASONS = Object.freeze([ + Object.freeze({ value: "DEFECTIVE", label: "Producto en mal estado" }), + Object.freeze({ value: "WRONG_ITEM", label: "Producto incorrecto o no es lo que pedí" }), + Object.freeze({ value: "MISSING_ITEM", label: "Producto incompleto o faltante" }), + Object.freeze({ value: "LATE_DELIVERY", label: "Entrega tardía o no recibida" }), + Object.freeze({ value: "CUSTOMER_SERVICE", label: "Mala atención del vendedor" }), + Object.freeze({ value: "OTHER", label: "Otro" }), +]);Nota: el test
retorna exactamente el catálogo exportado PRODUCT_REPORT_REASONS(línea 551) seguirá pasando porque sigue siendo la misma referencia.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/global/reports/product/product-report.service.js` around lines 10 - 20, PRODUCT_REPORT_REASONS is mutable and returned by getProductReportReasonsService, so freeze the shared catalog to prevent accidental mutation: deep-freeze each reason object then freeze the PRODUCT_REPORT_REASONS array where it’s defined; keep getProductReportReasonsService returning the same reference (it will now be immutable) and leave createProductReportService validation using PRODUCT_REPORT_REASONS unchanged. Ensure you only modify the declaration of PRODUCT_REPORT_REASONS (freeze objects then the array) and do not change the API of getProductReportReasonsService or createProductReportService.src/modules/global/reports/product/product-report.controller.js (1)
81-122: LGTM — handlers consistentes con el resto del archivo.El patrón (401 defensivo + try/catch +
next(error)) es coherente con los handlers existentes. Un detalle menor: enresolveProductReportaccedés areq.params.reportIdinline, mientras queupdateProductReport(línea 46) desestructuraconst { reportId } = req.params;. Unificar ayuda a la lectura, pero no es bloqueante.♻️ Nit opcional
export const resolveProductReport = async (req, res, next) => { try { if (!req.user?.id_user) { return res.status(401).json({ success: false, message: "Usuario autenticado requerido" }); } + const { reportId } = req.params; const { status, notes } = req.body; - const result = await resolveProductReportAdminService(req.user.id_user, req.params.reportId, { status, notes }); + const result = await resolveProductReportAdminService(req.user.id_user, reportId, { status, notes }); return res.status(200).json(result); } catch (error) { next(error); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/global/reports/product/product-report.controller.js` around lines 81 - 122, Unify the parameter access style by destructuring reportId from req.params in resolveProductReport (currently using req.params.reportId inline) to match updateProductReport; update the resolveProductReport function to extract const { reportId } = req.params before calling resolveProductReportAdminService so the handler style is consistent with other handlers like updateProductReport.src/modules/global/reports/product/product-report.routes.js (1)
197-207: Faltan bloques Swagger para los nuevos endpoints.Los endpoints existentes (
GET /products,GET /products/filtered,PUT /products/:reportId) tienen bloques@swaggercompletos. Los cuatro nuevos (GET /products/reasons,POST /products,GET /products/check,PATCH /products/:reportId/resolve) no los tienen, lo cual rompe la consistencia de la documentación OpenAPI del módulo y dificulta el consumo desde el frontend.Si usás
swagger-jsdoc, agregá los bloques arriba de cadarouter.<method>replicando el estilo del archivo. No bloquea el merge, pero sería bueno cerrarlo antes de liberar a consumidores.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/global/reports/product/product-report.routes.js` around lines 197 - 207, Add Swagger JSDoc blocks above the four routes for getProductReportReasons, createProductReport, checkProductReport, and resolveProductReport to match the existing style in this file: create an `@swagger` block for GET /products/reasons (public, tag "Reports", summary, 200 response schema), for POST /products (authenticated CUSTOMER, include requestBody schema for report payload, 201/400/401 responses and security bearer token), for GET /products/check (authenticated CUSTOMER with productId query param and 200/401 responses), and for PATCH /products/:reportId/resolve (authenticated ADMIN with path param reportId and requestBody for resolution payload plus 200/400/401 responses and security). Use the same field names and response examples as the other blocks and the swagger-jsdoc conventions already used in this file so the OpenAPI docs remain consistent.
🤖 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/admin/products/admin-products.service.js`:
- Around line 102-166: In updateProductApprovalStatusService: when transitioning
REJECTED→ACTIVE do not force visible: true and send a notification to the
seller; specifically, change the updateData construction to preserve the current
product.visible on approval (e.g., visible: isApproving ? product.visible :
product.visible) or simply omit changing visible when isApproving, and add a
prisma.notifications.create entry in the transaction for the approval path
(similar to the rejection notification) with a title like "Producto aprobado"
and a message informing the seller their product was reapproved; update the
transaction spread that currently only adds notifications on rejection to
include the approval notification when isApproving is true and remove the
unconditional forcing of visible to true in updateData.
In `@src/modules/global/reports/product/product-report.service.js`:
- Around line 383-415: The update is vulnerable to TOCTOU: after fetching the
report with prisma.productReports.findFirst you must include the current
report_status in the prisma.productReports.update where clause to ensure you
only update if the status is still the same (use the fetched
report.report_status as the expected current status), and convert the P2025
catch branch to throw a ConflictError indicating the report was modified by
another process rather than NotFound; keep the same update data
(normalizedStatus, trimmedNotes, resolvedAdminId, resolved_at) and selected
fields but change the where to include id_product_report: resolvedReportId,
status: true, report_status: report.report_status.
---
Nitpick comments:
In `@src/modules/admin/products/admin-products.controller.js`:
- Around line 16-30: The updateProductStatus controller contains a redundant
req.user check and an ad-hoc 401 JSON response that breaks the module's response
convention; remove the entire if (!req.user?.id_user) branch and its custom
res.status(401)... return so the function always calls
updateProductApprovalStatusService(req.user.id_user, req.params.id, req.body)
and returns res.status(200).json(result) (keeping the existing try/catch and
next(error)), thereby relying on the authenticate/requireRole middleware to
handle auth errors and preserving consistent service-returned payloads.
In `@src/modules/admin/products/admin-products.routes.js`:
- Around line 11-14: Routes are correctly protected, but add request-body
validation for PATCH /:id/status instead of relying solely on the service;
create a middleware (e.g., validateProductStatusBody) using your chosen schema
library (zod or joi) that enforces { status: "ACTIVE" | "REJECTED", reason?:
string } and returns 400 on validation failure, then insert that middleware into
the route chain before updateProductStatus in admin-products.routes.js
(router.patch("/:id/status", authenticate, requireRole(ROLES.ADMIN),
validateProductStatusBody, updateProductStatus)) so updateProductStatus receives
already-validated input.
In `@src/modules/admin/products/admin-products.service.js`:
- Around line 139-162: Compute a single normalizedReason once (e.g., const
normalizedReason = reason.toString().trim()) and use that variable in
updateData.rejection_reason and in the notifications.create message instead of
calling reason.toString().trim() twice; keep the existing logic that sets
rejection_reason to null when isApproving is true and use normalizedReason only
when creating the rejection notification and the message string.
In `@src/modules/global/reports/product/product-report.controller.js`:
- Around line 81-122: Unify the parameter access style by destructuring reportId
from req.params in resolveProductReport (currently using req.params.reportId
inline) to match updateProductReport; update the resolveProductReport function
to extract const { reportId } = req.params before calling
resolveProductReportAdminService so the handler style is consistent with other
handlers like updateProductReport.
In `@src/modules/global/reports/product/product-report.routes.js`:
- Around line 197-207: Add Swagger JSDoc blocks above the four routes for
getProductReportReasons, createProductReport, checkProductReport, and
resolveProductReport to match the existing style in this file: create an
`@swagger` block for GET /products/reasons (public, tag "Reports", summary, 200
response schema), for POST /products (authenticated CUSTOMER, include
requestBody schema for report payload, 201/400/401 responses and security bearer
token), for GET /products/check (authenticated CUSTOMER with productId query
param and 200/401 responses), and for PATCH /products/:reportId/resolve
(authenticated ADMIN with path param reportId and requestBody for resolution
payload plus 200/400/401 responses and security). Use the same field names and
response examples as the other blocks and the swagger-jsdoc conventions already
used in this file so the OpenAPI docs remain consistent.
In `@src/modules/global/reports/product/product-report.service.js`:
- Around line 237-270: The current TOCTOU check using
prisma.productReports.findFirst followed by prisma.productReports.create can
produce duplicate PENDING reports under concurrent requests; add a Postgres
partial unique index on (fk_product, fk_reporter) WHERE report_status =
'PENDING' AND status = true (e.g. CREATE UNIQUE INDEX ...), then wrap the
prisma.productReports.create call in a try/catch that detects Prisma's P2002
unique-constraint error and rethrows a ConflictError("Ya tenés un reporte
pendiente para este producto"); keep the existing findFirst check for fast-fail
UX but rely on the DB constraint plus P2002 handling around
productReports.create to guarantee correctness.
- Around line 10-20: PRODUCT_REPORT_REASONS is mutable and returned by
getProductReportReasonsService, so freeze the shared catalog to prevent
accidental mutation: deep-freeze each reason object then freeze the
PRODUCT_REPORT_REASONS array where it’s defined; keep
getProductReportReasonsService returning the same reference (it will now be
immutable) and leave createProductReportService validation using
PRODUCT_REPORT_REASONS unchanged. Ensure you only modify the declaration of
PRODUCT_REPORT_REASONS (freeze objects then the array) and do not change the API
of getProductReportReasonsService or createProductReportService.
In `@tests/unit/admin/admin-products.test.js`:
- Around line 260-284: Add tests covering non-numeric and zero productId inputs
for updateProductApprovalStatusService: add one test that calls
updateProductApprovalStatusService(1, "abc", { status: "ACTIVE" }) (or with
params coming from req like "abc") and expects a ValidationError, and another
that calls updateProductApprovalStatusService(1, 0, { status: "ACTIVE" })
(and/or "0") and expects a ValidationError; ensure tests use the same mocking
pattern (e.g., prisma.products.findFirst mocked where needed) and mirror
existing negative/REJECTED test structure so these invalid productId cases are
asserted consistently.
🪄 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: 8f5a45a3-fc8f-4fff-b58d-8f21f2fc7a11
📒 Files selected for processing (10)
src/app.jssrc/modules/admin/index.jssrc/modules/admin/products/admin-products.controller.jssrc/modules/admin/products/admin-products.routes.jssrc/modules/admin/products/admin-products.service.jssrc/modules/global/reports/product/product-report.controller.jssrc/modules/global/reports/product/product-report.routes.jssrc/modules/global/reports/product/product-report.service.jstests/unit/admin/admin-products.test.jstests/unit/report/product-report.test.js
Comment on lines
+102
to
+166
| export const updateProductApprovalStatusService = async (adminUserId, productId, { status, reason }) => { | ||
| const resolvedProductId = Number(productId); | ||
| if (!Number.isInteger(resolvedProductId) || resolvedProductId <= 0) { | ||
| throw new ValidationError("productId debe ser un entero positivo"); | ||
| } | ||
|
|
||
| const normalizedStatus = status?.toString().trim().toUpperCase(); | ||
| if (!["ACTIVE", "REJECTED"].includes(normalizedStatus)) { | ||
| throw new ValidationError("status debe ser ACTIVE o REJECTED"); | ||
| } | ||
|
|
||
| if (normalizedStatus === "REJECTED" && !reason?.toString().trim()) { | ||
| throw new ValidationError("reason es requerido al rechazar un producto"); | ||
| } | ||
|
|
||
| const product = await prisma.products.findFirst({ | ||
| where: { id_product: resolvedProductId, status: true }, | ||
| select: { | ||
| id_product: true, | ||
| name: true, | ||
| approval_status: true, | ||
| store: { select: { fk_user: true, name: true } }, | ||
| }, | ||
| }); | ||
|
|
||
| if (!product) { | ||
| throw new NotFoundError("Producto no encontrado"); | ||
| } | ||
|
|
||
| if (product.approval_status === normalizedStatus) { | ||
| throw new ValidationError( | ||
| `El producto ya tiene el estado ${normalizedStatus}` | ||
| ); | ||
| } | ||
|
|
||
| const isApproving = normalizedStatus === "ACTIVE"; | ||
|
|
||
| const updateData = { | ||
| approval_status: normalizedStatus, | ||
| visible: isApproving, | ||
| rejection_reason: isApproving ? null : reason.toString().trim(), | ||
| }; | ||
|
|
||
| const [updated] = await prisma.$transaction([ | ||
| prisma.products.update({ | ||
| where: { id_product: resolvedProductId }, | ||
| data: updateData, | ||
| select: productSelect, | ||
| }), | ||
| // Notificar al vendedor solo al rechazar | ||
| ...(isApproving | ||
| ? [] | ||
| : [ | ||
| prisma.notifications.create({ | ||
| data: { | ||
| fk_user: product.store.fk_user, | ||
| title: "Producto rechazado", | ||
| message: `Tu producto "${product.name}" ha sido rechazado. Motivo: ${reason.toString().trim()}`, | ||
| }, | ||
| }), | ||
| ]), | ||
| ]); | ||
|
|
||
| return mapProductResponse(updated); | ||
| }; No newline at end of file |
Contributor
There was a problem hiding this comment.
Transición REJECTED → ACTIVE: sin notificación y visible forzado.
Al reaprobar un producto previamente rechazado, se limpia rejection_reason, se fuerza visible: true y no se notifica al vendedor (solo hay notificación en el rechazo). Si la reaprobación es un flujo válido, conviene (a) notificar también al vendedor en el camino de aprobación y (b) evaluar si visible debería quedar bajo control del vendedor en lugar de forzarse a true, para no sobrescribir una decisión manual de ocultar el producto.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/admin/products/admin-products.service.js` around lines 102 - 166,
In updateProductApprovalStatusService: when transitioning REJECTED→ACTIVE do not
force visible: true and send a notification to the seller; specifically, change
the updateData construction to preserve the current product.visible on approval
(e.g., visible: isApproving ? product.visible : product.visible) or simply omit
changing visible when isApproving, and add a prisma.notifications.create entry
in the transaction for the approval path (similar to the rejection notification)
with a title like "Producto aprobado" and a message informing the seller their
product was reapproved; update the transaction spread that currently only adds
notifications on rejection to include the approval notification when isApproving
is true and remove the unconditional forcing of visible to true in updateData.
Comment on lines
+383
to
+415
| const report = await prisma.productReports.findFirst({ | ||
| where: { id_product_report: resolvedReportId, status: true }, | ||
| select: { id_product_report: true, report_status: true }, | ||
| }); | ||
|
|
||
| if (!report) throw new NotFoundError("Reporte no encontrado"); | ||
|
|
||
| if (report.report_status === "RESOLVED" || report.report_status === "REJECTED") { | ||
| throw new ValidationError("Este reporte ya fue cerrado y no puede modificarse"); | ||
| } | ||
|
|
||
| let updated; | ||
| try { | ||
| updated = await prisma.productReports.update({ | ||
| where: { id_product_report: resolvedReportId, status: true }, | ||
| data: { | ||
| report_status: normalizedStatus, | ||
| notes: trimmedNotes, | ||
| resolved_by: resolvedAdminId, | ||
| resolved_at: new Date(), | ||
| }, | ||
| select: { | ||
| id_product_report: true, | ||
| report_status: true, | ||
| notes: true, | ||
| resolved_at: true, | ||
| resolver: { select: { id_user: true, name: true } }, | ||
| }, | ||
| }); | ||
| } catch (e) { | ||
| if (e?.code === "P2025") throw new NotFoundError("El reporte ya no está disponible o fue modificado por otro proceso"); | ||
| throw e; | ||
| } |
Contributor
There was a problem hiding this comment.
TOCTOU: el update no protege contra cierres concurrentes.
Entre el findFirst (línea 383) que valida que el reporte todavía esté abierto y el update (línea 396), otro admin puede cerrar el mismo reporte. El where del update sólo filtra por id_product_report y status: true, sin incluir report_status, así que la segunda request sobrescribe silenciosamente report_status, notes, resolved_by y resolved_at del primer cierre — violando el invariante "no permite cerrar reportes ya cerrados".
updateProductReportService (línea 185) ya usa el patrón correcto incluyendo report_status: currentStatus en el where. Aplicá lo mismo acá y traducí el P2025 como conflicto:
🔒 Fix propuesto
const report = await prisma.productReports.findFirst({
where: { id_product_report: resolvedReportId, status: true },
select: { id_product_report: true, report_status: true },
});
if (!report) throw new NotFoundError("Reporte no encontrado");
if (report.report_status === "RESOLVED" || report.report_status === "REJECTED") {
throw new ValidationError("Este reporte ya fue cerrado y no puede modificarse");
}
+ const currentStatus = report.report_status;
+
let updated;
try {
updated = await prisma.productReports.update({
- where: { id_product_report: resolvedReportId, status: true },
+ where: {
+ id_product_report: resolvedReportId,
+ status: true,
+ report_status: currentStatus,
+ },
data: {
report_status: normalizedStatus,
notes: trimmedNotes,
resolved_by: resolvedAdminId,
resolved_at: new Date(),
},
select: { ... },
});
} catch (e) {
- if (e?.code === "P2025") throw new NotFoundError("El reporte ya no está disponible o fue modificado por otro proceso");
+ if (e?.code === "P2025") {
+ // Puede ser que otro admin lo cerró en paralelo (lost update evitado) o que ya no existe.
+ throw new ValidationError("El reporte fue modificado por otro proceso. Recargá y volvé a intentar.");
+ }
throw e;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/global/reports/product/product-report.service.js` around lines
383 - 415, The update is vulnerable to TOCTOU: after fetching the report with
prisma.productReports.findFirst you must include the current report_status in
the prisma.productReports.update where clause to ensure you only update if the
status is still the same (use the fetched report.report_status as the expected
current status), and convert the P2025 catch branch to throw a ConflictError
indicating the report was modified by another process rather than NotFound; keep
the same update data (normalizedStatus, trimmedNotes, resolvedAdminId,
resolved_at) and selected fields but change the where to include
id_product_report: resolvedReportId, status: true, report_status:
report.report_status.
DavidVillarM
approved these changes
Apr 22, 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.
Contexto:
El módulo de reportes de productos ya existía con funcionalidades para Admin y Seller
(listar, filtrar, actualizar estado). Este ticket extiende ese módulo con las
capacidades del lado del cliente y del administrador para cerrar el ciclo completo.
Cambios en product-report.service.js:
Es la fuente única de verdad para los motivos válidos: usado en la validación de
createProductReportService y expuesto vía el endpoint /reasons para el frontend.
Valida productId (entero positivo), reason contra el catálogo, verifica que el
producto exista y esté activo (status=true), y previene duplicados lanzando
ConflictError si ya existe un reporte PENDING del mismo usuario para el mismo producto.
un producto. Retorna { hasReport: boolean, reportId: number | null }.
Requiere notes obligatorias y no permite modificar reportes ya cerrados.
Cambios en product-report.controller.js:
Cambios en product-report.routes.js:
Tests (product-report.test.js):
Total: 67 tests pasando.
Summary by CodeRabbit
Notas de Lanzamiento