Repository navigation
feat(admin): implementar moderación de productos para panel de administración - #120
Conversation
…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
|
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 38 minutes and 9 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)
📝 WalkthroughWalkthroughSe agregaron rutas, controlador y servicios para funcionalidad de administración de productos, incluyendo listado con filtros y actualización de estado de aprobación. El enrutador se montó en la aplicación Express y se incluyeron pruebas unitarias para validar la lógica de negocio. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 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)
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}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/unit/admin/admin-products.test.js (1)
272-278: Mock innecesario: la validación dereasonocurre antes delfindFirst.En
updateProductApprovalStatusServicela validación dereason(L113-115) se evalúa antes delprisma.products.findFirst(L117), por lo que el mockprisma.products.findFirst.mockResolvedValue(mockProduct)nunca se consume en este test. No afecta la correctness del test, pero puede confundir al lector haciéndole creer que se testea idempotencia + falta de reason juntos.🧹 Limpieza sugerida
it("lanza ValidationError cuando se rechaza sin reason", async () => { - prisma.products.findFirst.mockResolvedValue(mockProduct); - await expect( updateProductApprovalStatusService(1, 1, { status: "REJECTED", reason: "" }) ).rejects.toThrow(ValidationError); });🤖 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 272 - 278, The test unnecessarily mocks prisma.products.findFirst (prisma.products.findFirst.mockResolvedValue(mockProduct)) even though updateProductApprovalStatusService validates the 'reason' and throws ValidationError before any DB call; remove that mock from the "lanza ValidationError cuando se rechaza sin reason" test so the test only asserts that calling updateProductApprovalStatusService(1, 1, { status: "REJECTED", reason: "" }) rejects with ValidationError, keeping the test focused and less confusing.src/modules/admin/products/admin-products.controller.js (1)
16-30: Check dereq.user?.id_userredundante dado el middlewareauthenticate+requireRole(ADMIN).La ruta ya aplica
authenticateyrequireRole(ROLES.ADMIN), que garantizanreq.userpresente antes de entrar al handler. El 401 aquí nunca debería dispararse en el happy path. Es defensivo y no rompe nada, pero si se quiere mantener consistencia con los otros controllers de admin y dejar el manejo de auth centralizado en middlewares, se podría simplificar:♻️ Simplificación opcional
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); } };Nota: si se elimina el parámetro
adminUserIddel servicio (ver comentario enadmin-products.service.js), este check queda naturalmente fuera.🤖 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 handler updateProductStatus contains a redundant auth check for req.user?.id_user because authenticate + requireRole(ROLES.ADMIN) already guarantee req.user; remove the early 401 block and simply call updateProductApprovalStatusService using req.user.id_user, req.params.id and req.body, returning the result (keep the try/catch and next(error) as-is); also consider removing the adminUserId parameter from updateProductApprovalStatusService (referenced by name) in a follow-up if you fully centralize auth in middleware.
🤖 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-165: The adminUserId parameter is accepted by
updateProductApprovalStatusService but never used; either persist it for
auditability or remove it to avoid a misleading API. If you have an approver
column (e.g., approved_by / reviewed_by) or an audit table, include adminUserId
in the prisma.products.update data (add reviewed_by: adminUserId or create a
prisma.productAudits.create entry inside the same $transaction alongside
prisma.products.update and prisma.notifications.create) and return it via the
existing productSelect/mapProductResponse; otherwise remove adminUserId from the
updateProductApprovalStatusService signature and from the controller call to
align intent across admin modules (also mirror the same change in admin/stores).
- Around line 117-163: El chequeo de estado se hace fuera de la transacción
(prisma.products.findFirst / product.approval_status === normalizedStatus)
permitiendo TOCTOU; muévelo dentro de la transacción y realiza un update
condicional que solo aplique si el approval_status actual difiere de
normalizedStatus (por ejemplo usar prisma.products.update with where: {
id_product: resolvedProductId, approval_status: { not: normalizedStatus } } y
data: updateData), ejecutar la creación de notificación
(prisma.notifications.create) dentro del mismo prisma.$transaction solo cuando
el update realmente afecte un registro, y, si el update no devolvió un registro
actualizado, lanzar ValidationError `El producto ya tiene el estado
${normalizedStatus}`; así usa los símbolos prisma.$transaction,
prisma.products.update, prisma.notifications.create, approval_status,
normalizedStatus y updateData para localizar y aplicar el cambio.
---
Nitpick comments:
In `@src/modules/admin/products/admin-products.controller.js`:
- Around line 16-30: The handler updateProductStatus contains a redundant auth
check for req.user?.id_user because authenticate + requireRole(ROLES.ADMIN)
already guarantee req.user; remove the early 401 block and simply call
updateProductApprovalStatusService using req.user.id_user, req.params.id and
req.body, returning the result (keep the try/catch and next(error) as-is); also
consider removing the adminUserId parameter from
updateProductApprovalStatusService (referenced by name) in a follow-up if you
fully centralize auth in middleware.
In `@tests/unit/admin/admin-products.test.js`:
- Around line 272-278: The test unnecessarily mocks prisma.products.findFirst
(prisma.products.findFirst.mockResolvedValue(mockProduct)) even though
updateProductApprovalStatusService validates the 'reason' and throws
ValidationError before any DB call; remove that mock from the "lanza
ValidationError cuando se rechaza sin reason" test so the test only asserts that
calling updateProductApprovalStatusService(1, 1, { status: "REJECTED", reason:
"" }) rejects with ValidationError, keeping the test focused and less confusing.
🪄 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: 0301c335-8673-4d4a-b87e-abb0b3ccc7d4
📒 Files selected for processing (6)
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.jstests/unit/admin/admin-products.test.js
DavidVillarM
left a comment
DavidVillarM
left a comment
There was a problem hiding this comment.
Funcionalidad correcta
2 participants
Endpoints:
Filtros del GET:
Lógica del PATCH:
Summary by CodeRabbit
Notas de Lanzamiento