Repository navigation
Om 432 - #78
Merged
Merged
Om 432#78
Conversation
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 0 minutes and 32 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 (2)
📝 WalkthroughWalkthroughSe añadieron/ajustaron variables de entorno en el workflow de tests, se cargó Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 (2)
tests/e2e/tags.test.js (1)
60-69: Este caso no prueba realmente el filtrosearch.Como
findManyya está mockeado con el resultado filtrado, el test sigue pasando aunque el service ignorereq.query.search. Conviene afirmar también la query enviada a Prisma.💡 Ajuste sugerido
const res = await request(app).get("/products/tags?search=oferta"); expect(res.status).toBe(200); expect(res.body).toHaveLength(1); expect(res.body[0].name).toBe("Oferta"); + expect(prisma.productTags.findMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ + name: expect.objectContaining({ + contains: "oferta", + mode: "insensitive", + }), + }), + }) + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/tags.test.js` around lines 60 - 69, The test currently mocks prisma.productTags.findMany to return a filtered array so it doesn't verify that the controller/service actually applied the search filter; update the test to also assert the call to prisma.productTags.findMany contains the expected query object (e.g. check prisma.productTags.findMany was called with an objectContaining where.name contains the search term "oferta" with appropriate case/insensitive options) after making the GET request to "/products/tags?search=oferta" so the test fails if the service ignores req.query.search.tests/e2e/product/products.test.js (1)
153-160: Hacé este 400 más específico.Con el assert actual, el caso pasa con cualquier respuesta que tenga
errors, aunque no marque ambos parámetros inválidos. Ya que este camino debería cortar antes de pegarle a Prisma, también conviene afirmarlo.💡 Ajuste sugerido
const res = await request(app).get("/products?page=-1&limit=0"); expect(res.status).toBe(400); - expect(res.body).toHaveProperty("errors"); + expect(res.body.errors).toEqual( + expect.arrayContaining([ + expect.objectContaining({ field: "page" }), + expect.objectContaining({ field: "limit" }), + ]) + ); + expect(prisma.products.count).not.toHaveBeenCalled(); + expect(prisma.products.findMany).not.toHaveBeenCalled();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/product/products.test.js` around lines 153 - 160, Tighten the test "Error 400 cuando los parametros son invalidos": assert that the response errors specifically mention both invalid "page" and "limit" parameters (instead of just checking existence of errors) and also assert that prisma.products.count and prisma.products.findMany were not called to ensure validation short-circuits before DB access; locate the test's request(app).get("/products?page=-1&limit=0") call and replace the generic expect(res.body).toHaveProperty("errors") with checks for the expected error messages and expect(prisma.products.count).not.toHaveBeenCalled() and expect(prisma.products.findMany).not.toHaveBeenCalled().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/e2e/stores.test.js`:
- Around line 225-231: El stub global de fetch se crea en beforeEach usando
vi.stubGlobal("fetch", ...) y no se restaura, lo que puede filtrar a otros
tests; añade un afterEach que invoque vi.unstubAllGlobals() para limpiar los
stubs globales (restaurando fetch) al finalizar cada test, manteniendo la
configuración actual de beforeEach con vi.resetAllMocks().
---
Nitpick comments:
In `@tests/e2e/product/products.test.js`:
- Around line 153-160: Tighten the test "Error 400 cuando los parametros son
invalidos": assert that the response errors specifically mention both invalid
"page" and "limit" parameters (instead of just checking existence of errors) and
also assert that prisma.products.count and prisma.products.findMany were not
called to ensure validation short-circuits before DB access; locate the test's
request(app).get("/products?page=-1&limit=0") call and replace the generic
expect(res.body).toHaveProperty("errors") with checks for the expected error
messages and expect(prisma.products.count).not.toHaveBeenCalled() and
expect(prisma.products.findMany).not.toHaveBeenCalled().
In `@tests/e2e/tags.test.js`:
- Around line 60-69: The test currently mocks prisma.productTags.findMany to
return a filtered array so it doesn't verify that the controller/service
actually applied the search filter; update the test to also assert the call to
prisma.productTags.findMany contains the expected query object (e.g. check
prisma.productTags.findMany was called with an objectContaining where.name
contains the search term "oferta" with appropriate case/insensitive options)
after making the GET request to "/products/tags?search=oferta" so the test fails
if the service ignores req.query.search.
🪄 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: 143d946b-7e44-4b9d-b3fc-22eee3d19d16
📒 Files selected for processing (6)
.github/workflows/vitest-supertest.ymlsrc/modules/users/users/services/users.services.jstests/e2e/product/products.test.jstests/e2e/stores.test.jstests/e2e/tags.test.jstests/e2e/users.test.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
package.json (1)
12-12: Evitá depender de exclusión por path fijo en CI.En Line 12, excluir un archivo puntual puede ocultar regresiones si ese script se activa en pipeline. Conviene mover esta excepción a una config de CI dedicada y dejar trazabilidad (issue/link + fecha objetivo de remoción).
Propuesta concreta
- "test:ci": "vitest run --exclude tests/e2e/product/filter-product.test.js", + "test:ci": "vitest run -c vitest.config.ci.js",// vitest.config.ci.js (nuevo) import base from "./vitest.config.js"; import { defineConfig, mergeConfig } from "vitest/config"; export default mergeConfig( base, defineConfig({ test: { exclude: ["tests/e2e/product/filter-product.test.js"] // TODO: remover cuando se resuelva ISSUE-XXX } }) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 12, La línea "test:ci" en package.json no debe depender de una exclusión por path fijo; quita la opción --exclude de la entrada "test:ci" y en su lugar crea un nuevo archivo de configuración vitest.config.ci.js que importe/mergue vitest.config.js y ponga la exclusión ("tests/e2e/product/filter-product.test.js") en test.exclude con un comentario TODO con el número de issue/link y fecha objetivo de eliminación; luego actualiza "test:ci" para usar esa config CI (ej. --config vitest.config.ci.js).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/vitest-supertest.yml:
- Around line 28-29: Remueve las credenciales embebidas en las variables
DATABASE_URL y DIRECT_URL y sustitúyelas por referencias a secretos/variables de
entorno administradas (por ejemplo usar secrets como TEST_DATABASE_URL y
TEST_DIRECT_URL o variables de entorno del runner) en la configuración del
workflow; actualiza la configuración del repositorio/CI para añadir esos
secretos y asegúrate de que cualquier servicio de base de datos de prueba (p.
ej. el servicio Docker/postgres usado en la job) acepte la URL desde esas
variables en lugar de valores en texto plano.
---
Nitpick comments:
In `@package.json`:
- Line 12: La línea "test:ci" en package.json no debe depender de una exclusión
por path fijo; quita la opción --exclude de la entrada "test:ci" y en su lugar
crea un nuevo archivo de configuración vitest.config.ci.js que importe/mergue
vitest.config.js y ponga la exclusión
("tests/e2e/product/filter-product.test.js") en test.exclude con un comentario
TODO con el número de issue/link y fecha objetivo de eliminación; luego
actualiza "test:ci" para usar esa config CI (ej. --config vitest.config.ci.js).
🪄 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: 33b1037e-7074-4b02-8256-54fdeaa9cb85
📒 Files selected for processing (3)
.github/workflows/vitest-supertest.ymlpackage.jsonsrc/lib/prisma.js
✅ Files skipped from review due to trivial changes (1)
- src/lib/prisma.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/vitest-supertest.yml:
- Around line 55-59: The workflow currently runs the unstable e2e suite via the
"npm run test:run" step despite the note about a failing file; change the CI
step to run the deterministic suite by replacing or uncommenting the "npm run
test:ci" invocation and remove or update the comment that says one file fails so
it no longer executes the unstable "test:run"; target the lines that reference
the run commands "npm run test:run" and "npm run test:ci" in the
vitest-supertest.yml and ensure only "npm run test:ci" runs for PRs.
- Around line 31-32: The CI workflow is exporting DATABASE_URL from secrets so
the test job (which runs "npx prisma db push") can hit the production DB; change
the workflow to set DATABASE_URL to the test secret (use DATABASE_URL_TEST) for
the vitest-supertest job or ensure the job exports DATABASE_URL_TEST and the
test step sets DATABASE_URL=$DATABASE_URL_TEST before running migrations;
additionally consider updating src/lib/prisma.js to select
process.env.DATABASE_URL_TEST when NODE_ENV==='test' (instead of always reading
process.env.DATABASE_URL) so "npx prisma db push" and Prisma client
instantiation use the test DB during tests.
🪄 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: b91bc6e8-eabf-4865-ab38-71b31f3eee25
📒 Files selected for processing (1)
.github/workflows/vitest-supertest.yml
J-Kanami-PS
approved these changes
Apr 6, 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 Lanzamiento
Bug Fixes
Tests
Chores