Repository navigation
Tests e2e 100% cubiertos - #167
Merged
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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 (1)
📝 WalkthroughWalkthroughSe agregan 5 suites e2e (Vitest + Supertest) que cubren endpoints para gestión de banners, horarios comerciales, carrito, aprobación de comercios y recuperación de contraseña. Cada suite mockea Prisma, simula autenticación mediante headers de prueba, y valida códigos de respuesta, autorización basada en roles y validaciones de entrada. ChangesE2E endpoint test coverage
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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: 1
🧹 Nitpick comments (7)
tests/e2e/business-hours.test.js (1)
1-1: ⚡ Quick winRestaurá
process.env.JWT_SECRETal finalizar la suite.En Line 29 se modifica estado global del proceso y no se revierte; eso puede contaminar otras suites según el orden de ejecución.
💡 Propuesta
-import { vi, describe, it, expect, beforeEach, beforeAll } from "vitest"; +import { vi, describe, it, expect, beforeEach, beforeAll, afterAll } from "vitest"; @@ let sellerToken; +let previousJwtSecret; beforeAll(() => { + previousJwtSecret = process.env.JWT_SECRET; process.env.JWT_SECRET = TEST_JWT_SECRET; sellerToken = jwt.sign( { id_user: SELLER_ID, email: "seller@test.com", role: "SELLER" }, TEST_JWT_SECRET ); }); + +afterAll(() => { + process.env.JWT_SECRET = previousJwtSecret; +});Also applies to: 28-34
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/business-hours.test.js` at line 1, La suite de tests modifica la variable global process.env.JWT_SECRET (en el bloque que comienza alrededor de la modificación en la prueba) y no la restaura; guarda el valor original de process.env.JWT_SECRET antes de cambiarlo y restaura ese valor en un afterAll (o afterEach) para evitar contaminación entre suites; busca las referencias a process.env.JWT_SECRET en este archivo y envuélvelas con lógica de backup/restore utilizando beforeAll/afterAll (o beforeEach/afterEach) para garantizar que el valor original se restaure al finalizar la suite.tests/e2e/stock.test.js (1)
388-394: ⚡ Quick winEvitá
TypeErrorsilencioso al usarfind(...)sin validar resultado.Si no se encuentra el item, el test rompe al acceder a
.producty deja un error menos útil. Conviene assertar que el item exista antes de validarstock.💡 Cambio propuesto
- const itemConStock = items.find((i) => i.product.id === 1); - expect(itemConStock.product).toHaveProperty("stock"); + const itemConStock = items.find((i) => i.product.id === 1); + expect(itemConStock).toBeDefined(); + expect(itemConStock.product).toHaveProperty("stock"); expect(itemConStock.product.stock).toBe(5); - const itemSinStock = items.find((i) => i.product.id === 2); - expect(itemSinStock.product).toHaveProperty("stock"); + const itemSinStock = items.find((i) => i.product.id === 2); + expect(itemSinStock).toBeDefined(); + expect(itemSinStock.product).toHaveProperty("stock"); expect(itemSinStock.product.stock).toBe(0);- const itemConStock = res.body.find((i) => i.product.id === 3); - expect(itemConStock.product).toHaveProperty("stock"); + const itemConStock = res.body.find((i) => i.product.id === 3); + expect(itemConStock).toBeDefined(); + expect(itemConStock.product).toHaveProperty("stock"); expect(itemConStock.product.stock).toBe(8); - const itemSinStock = res.body.find((i) => i.product.id === 4); - expect(itemSinStock.product).toHaveProperty("stock"); + const itemSinStock = res.body.find((i) => i.product.id === 4); + expect(itemSinStock).toBeDefined(); + expect(itemSinStock.product).toHaveProperty("stock"); expect(itemSinStock.product.stock).toBe(0);Also applies to: 445-451
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/stock.test.js` around lines 388 - 394, El test usa items.find(...) para obtener itemConStock e itemSinStock pero no comprueba que el resultado exista, lo que provoca TypeError silencioso al acceder a .product; antes de validar stock en las variables itemConStock y itemSinStock asegúrate de afirmar su existencia (por ejemplo con expect(itemConStock).toBeDefined()/toBeTruthy() o similar) y solo entonces acceder a .product.stock; aplica el mismo patrón a las comprobaciones en las líneas 445-451 para evitar errores si find devuelve undefined.tests/e2e/banners.test.js (3)
159-197: ⚡ Quick winValidá filtros también en
findManypara evitar falsos positivos.Acá sólo se comprueba el
whereencount; sifindManyno aplicara esos filtros, estos tests igual podrían pasar.Diff sugerido
it("aplica el filtro active=true a la consulta", async () => { @@ expect(prisma.banners.count).toHaveBeenCalledWith( expect.objectContaining({ where: expect.objectContaining({ is_active: true }) }) ); + expect(prisma.banners.findMany).toHaveBeenCalledWith( + expect.objectContaining({ where: expect.objectContaining({ is_active: true }) }) + ); }); it("aplica el filtro status=false a la consulta", async () => { @@ expect(prisma.banners.count).toHaveBeenCalledWith( expect.objectContaining({ where: expect.objectContaining({ status: false }) }) ); + expect(prisma.banners.findMany).toHaveBeenCalledWith( + expect.objectContaining({ where: expect.objectContaining({ status: false }) }) + ); }); it("aplica búsqueda por título y descripción con el parámetro search", async () => { @@ expect(prisma.banners.count).toHaveBeenCalledWith( expect.objectContaining({ where: expect.objectContaining({ OR: [ { title: { contains: "eco", mode: "insensitive" } }, { description: { contains: "eco", mode: "insensitive" } }, ], }), }) ); + expect(prisma.banners.findMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ + OR: [ + { title: { contains: "eco", mode: "insensitive" } }, + { description: { contains: "eco", mode: "insensitive" } }, + ], + }), + }) + ); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/banners.test.js` around lines 159 - 197, The tests only assert the `where` passed to `prisma.banners.count`, allowing false positives if `prisma.banners.findMany` doesn't receive the same filters; update each case to also assert that `prisma.banners.findMany` was called with an objectContaining the same `where` conditions (e.g., include checks for `is_active: true`, `status: false`, and the `OR` search clause with `title`/`description` contains in the respective tests) so both `prisma.banners.count` and `prisma.banners.findMany` are validated for the expected filters.
320-348: ⚡ Quick winChequeá el payload de activación/desactivación en la operación de update.
En ambos casos
200, faltan aserciones de queupdateManyhaya escritois_activecon el valor pedido (false/true).Diff sugerido
it("devuelve 200 al desactivar un banner activo", async () => { @@ expect(res.status).toBe(200); expect(res.body).toHaveProperty("isActive", false); + expect(prisma.banners.updateMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ id_banner: 1 }), + data: expect.objectContaining({ is_active: false }), + }) + ); }); it("devuelve 200 al activar un banner inactivo", async () => { @@ expect(res.status).toBe(200); expect(res.body).toHaveProperty("isActive", true); + expect(prisma.banners.updateMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ id_banner: 1 }), + data: expect.objectContaining({ is_active: true }), + }) + ); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/banners.test.js` around lines 320 - 348, Add assertions to both tests to verify the update payload sent to prisma.banners.updateMany includes the requested is_active value; specifically, after invoking the PATCH via asAdmin(request(app).patch("/api/admin/banners/1/active").send(...)), assert that prisma.banners.updateMany was called and its call includes data: { is_active: false } in the deactivate test and data: { is_active: true } in the activate test (use expect(prisma.banners.updateMany).toHaveBeenCalled() and expect(...).toHaveBeenCalledWith(expect.objectContaining({ data: expect.objectContaining({ is_active: <bool> }) })) to locate the call regardless of extra fields).
253-267: ⚡ Quick winAserción faltante de persistencia en el update exitoso.
El test puede pasar con el segundo
findUniquemockeado aunque elupdateManyno reciba el cambio esperado. Conviene validar la escritura.Diff sugerido
it("devuelve 200 con el banner actualizado", async () => { @@ expect(res.status).toBe(200); expect(res.body).toHaveProperty("id", 1); expect(res.body).toHaveProperty("title", "Nuevo Titulo"); + expect(prisma.banners.updateMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ id_banner: 1 }), + data: expect.objectContaining({ title: "Nuevo Titulo" }), + }) + ); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/banners.test.js` around lines 253 - 267, The test currently only checks the response body but not that the DB write occurred; add an assertion to verify prisma.banners.updateMany was invoked with the expected where/data payload (e.g., id 1 and title "Nuevo Titulo") or at least that updateMany was called once and returned the expected count; locate the test in banners.test.js and assert against prisma.banners.updateMany (and/or its mock.calls) after the asAdmin request to ensure the update was persisted.tests/e2e/store-approval-flow.test.js (2)
370-393: ⚡ Quick winAsegurá cantidad/orden de llamadas en
updateManypara detectar efectos colaterales.
toHaveBeenCalledWithpasa aunque existan llamadas extra inesperadas. Acá conviene fijar que sea exactamente una llamada por etapa.Diff propuesto
expect(updateRes.status).toBe(200); + expect(prisma.products.updateMany).toHaveBeenCalledTimes(1); expect(prisma.products.updateMany).toHaveBeenCalledWith( expect.objectContaining({ where: expect.objectContaining({ fk_store: STORE_ID, status: true }), data: { visible: false }, }) @@ const approveRes = await request(app) .patch(`/api/admin/stores/${STORE_ID}/approve`) .set("Cookie", `userToken=${adminToken}`); expect(approveRes.status).toBe(200); + expect(prisma.products.updateMany).toHaveBeenCalledTimes(1); expect(prisma.products.updateMany).toHaveBeenCalledWith( expect.objectContaining({ where: expect.objectContaining({ fk_store: STORE_ID, status: true }), data: { visible: true }, })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/store-approval-flow.test.js` around lines 370 - 393, Verificar que solo haya una llamada a prisma.products.updateMany en cada etapa comprobando la cantidad y el orden además de los argumentos; después del primer PATCH (variable updateRes) usar expect(prisma.products.updateMany).toHaveBeenCalledTimes(1) y expect(prisma.products.updateMany).toHaveBeenNthCalledWith(1, ...) para afirmar el llamado que oculta productos, luego limpiar mocks o usar mockClear(), y tras el PATCH approve (variable approveRes) volver a usar toHaveBeenCalledTimes(1) en el contexto limpio o usar toHaveBeenNthCalledWith(1, ...) para afirmar el llamado que muestra productos con data: { visible: true } asegurando así que no haya llamadas extra inesperadas.
332-343: ⚡ Quick winValidá el resultado del rechazo antes de inspeccionar notificaciones.
Este test asume que el
PATCH /rejectfue exitoso, pero no lo verifica. Si ese paso falla, la aserción demessagepuede romper por un motivo secundario y esconder la causa real.Diff propuesto
- await request(app) + const rejectRes = await request(app) .patch(`/api/admin/stores/${STORE_ID}/reject`) .set("Cookie", `userToken=${adminToken}`) .send({ reason }); + + expect(rejectRes.status).toBe(200); const res = await request(app) .get("/api/notifications") .set("Cookie", `userToken=${sellerToken}`);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/store-approval-flow.test.js` around lines 332 - 343, The test currently patches /api/admin/stores/${STORE_ID}/reject but never asserts that call succeeded; capture the response from the PATCH (the request that uses adminToken and reason), assert the expected success status (e.g., 200/204) and any expected response body fields (e.g., status or message) before proceeding to call GET /api/notifications with sellerToken and asserting the notification message; reference the existing variables STORE_ID, adminToken, reason, sellerToken and the two endpoints to locate where to add the check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/e2e/users.test.js`:
- Around line 353-380: Add an explicit assertion for res.body.message in both
"devuelve 200 con mensaje genérico..." tests to ensure the response message is
identical for existing and non-existing emails; update the two it blocks that
post to "/api/users/forgot-password" (the tests that call
prisma.users.findUnique.mockResolvedValue(null) and
prisma.users.findUnique.mockResolvedValue({ id_user: 1, status: true })) to
expect the exact generic message returned by the endpoint—either by
importing/using the same constant (e.g., GENERIC_FORGOT_PASSWORD_MESSAGE) used
by the route handler or by hardcoding the exact string the API returns—so both
tests assert equal res.body.message equality in addition to status and success
checks.
---
Nitpick comments:
In `@tests/e2e/banners.test.js`:
- Around line 159-197: The tests only assert the `where` passed to
`prisma.banners.count`, allowing false positives if `prisma.banners.findMany`
doesn't receive the same filters; update each case to also assert that
`prisma.banners.findMany` was called with an objectContaining the same `where`
conditions (e.g., include checks for `is_active: true`, `status: false`, and the
`OR` search clause with `title`/`description` contains in the respective tests)
so both `prisma.banners.count` and `prisma.banners.findMany` are validated for
the expected filters.
- Around line 320-348: Add assertions to both tests to verify the update payload
sent to prisma.banners.updateMany includes the requested is_active value;
specifically, after invoking the PATCH via
asAdmin(request(app).patch("/api/admin/banners/1/active").send(...)), assert
that prisma.banners.updateMany was called and its call includes data: {
is_active: false } in the deactivate test and data: { is_active: true } in the
activate test (use expect(prisma.banners.updateMany).toHaveBeenCalled() and
expect(...).toHaveBeenCalledWith(expect.objectContaining({ data:
expect.objectContaining({ is_active: <bool> }) })) to locate the call regardless
of extra fields).
- Around line 253-267: The test currently only checks the response body but not
that the DB write occurred; add an assertion to verify prisma.banners.updateMany
was invoked with the expected where/data payload (e.g., id 1 and title "Nuevo
Titulo") or at least that updateMany was called once and returned the expected
count; locate the test in banners.test.js and assert against
prisma.banners.updateMany (and/or its mock.calls) after the asAdmin request to
ensure the update was persisted.
In `@tests/e2e/business-hours.test.js`:
- Line 1: La suite de tests modifica la variable global process.env.JWT_SECRET
(en el bloque que comienza alrededor de la modificación en la prueba) y no la
restaura; guarda el valor original de process.env.JWT_SECRET antes de cambiarlo
y restaura ese valor en un afterAll (o afterEach) para evitar contaminación
entre suites; busca las referencias a process.env.JWT_SECRET en este archivo y
envuélvelas con lógica de backup/restore utilizando beforeAll/afterAll (o
beforeEach/afterEach) para garantizar que el valor original se restaure al
finalizar la suite.
In `@tests/e2e/stock.test.js`:
- Around line 388-394: El test usa items.find(...) para obtener itemConStock e
itemSinStock pero no comprueba que el resultado exista, lo que provoca TypeError
silencioso al acceder a .product; antes de validar stock en las variables
itemConStock y itemSinStock asegúrate de afirmar su existencia (por ejemplo con
expect(itemConStock).toBeDefined()/toBeTruthy() o similar) y solo entonces
acceder a .product.stock; aplica el mismo patrón a las comprobaciones en las
líneas 445-451 para evitar errores si find devuelve undefined.
In `@tests/e2e/store-approval-flow.test.js`:
- Around line 370-393: Verificar que solo haya una llamada a
prisma.products.updateMany en cada etapa comprobando la cantidad y el orden
además de los argumentos; después del primer PATCH (variable updateRes) usar
expect(prisma.products.updateMany).toHaveBeenCalledTimes(1) y
expect(prisma.products.updateMany).toHaveBeenNthCalledWith(1, ...) para afirmar
el llamado que oculta productos, luego limpiar mocks o usar mockClear(), y tras
el PATCH approve (variable approveRes) volver a usar toHaveBeenCalledTimes(1) en
el contexto limpio o usar toHaveBeenNthCalledWith(1, ...) para afirmar el
llamado que muestra productos con data: { visible: true } asegurando así que no
haya llamadas extra inesperadas.
- Around line 332-343: The test currently patches
/api/admin/stores/${STORE_ID}/reject but never asserts that call succeeded;
capture the response from the PATCH (the request that uses adminToken and
reason), assert the expected success status (e.g., 200/204) and any expected
response body fields (e.g., status or message) before proceeding to call GET
/api/notifications with sellerToken and asserting the notification message;
reference the existing variables STORE_ID, adminToken, reason, sellerToken and
the two endpoints to locate where to add the check.
🪄 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: 1d751c70-7549-4df0-a506-dddf412959d9
📒 Files selected for processing (5)
tests/e2e/banners.test.jstests/e2e/business-hours.test.jstests/e2e/stock.test.jstests/e2e/store-approval-flow.test.jstests/e2e/users.test.js
|
Collaborator
Author
|
alguien me acepta? ya corregi lo de rabbit, solo se debe aceptar. Voy a dormir |
CrisNAC
approved these changes
May 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.
Summary by CodeRabbit
Notas de Versión