Repository navigation
Conversation
Contributor
📝 WalkthroughRecorridoSe actualiza la lógica de construcción de filtros en el servicio Cambios
Esfuerzo estimado de revisión de código🎯 4 (Complejo) | ⏱️ ~50 minutos PRs posiblemente relacionados
Revisores sugeridos
Poema 🐰
🚥 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 docstrings
🧪 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 (1)
tests/e2e/stores.test.js (1)
170-207: Sumá cobertura paracounty paraisOffer=true/false.Este caso solo cubre el camino donde no llega filtro de oferta y además inspecciona
findMany, pero nocount. Si se rompe la rama nueva que deja un solo lado delOR(?isOffer=true/falseo?is_offer=) ocountqueda armando otrowhere, el endpoint puede volver a404y este test seguir verde.Podés cerrar el gap con casos como:
?isOffer=true&price_min=10&price_max=15→ solo ramaoffer_price?is_offer=false&price_min=10&price_max=15→ solo ramaprice- misma verificación del
whereenprisma.products.count🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/stores.test.js` around lines 170 - 207, Add tests to cover the missing branches and ensure both prisma.products.findMany and prisma.products.count receive the correct where filter: add a request to "/api/commerces/products/filter/1?isOffer=true&price_min=10&price_max=15" and assert prisma.products.findMany and prisma.products.count are called with a where that only contains the offer_price clause (is_offer: true AND offer_price gte/lte), add a request to "/api/commerces/products/filter/1?is_offer=false&price_min=10&price_max=15" and assert both findMany and count are called with a where that only contains the price clause (is_offer: false AND price gte/lte), and mirror the same expect.objectContaining checks you used in the existing test (referencing prisma.products.findMany and prisma.products.count) so the new tests fail if either side of the OR or the count query is constructed incorrectly.
🤖 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/commerces/store.service.js`:
- Around line 841-875: Valida resolvedMinPrice y resolvedMaxPrice antes de
construir effectivePriceRange: rechaza valores vacíos o no numéricos (evitar que
Number("") => 0 o Number("abc") => NaN) y responde con 400; convierte sólo
cuando Number.isFinite(Number(value)) es true; además, si ambos existen valida
que Number(resolvedMinPrice) <= Number(resolvedMaxPrice) y rechaza con 400 si
no; después de estas comprobaciones procede a poblar effectivePriceRange y
construir las ramas que usan normalizedIsOfferFilter y whereConditions.OR como
ahora.
---
Nitpick comments:
In `@tests/e2e/stores.test.js`:
- Around line 170-207: Add tests to cover the missing branches and ensure both
prisma.products.findMany and prisma.products.count receive the correct where
filter: add a request to
"/api/commerces/products/filter/1?isOffer=true&price_min=10&price_max=15" and
assert prisma.products.findMany and prisma.products.count are called with a
where that only contains the offer_price clause (is_offer: true AND offer_price
gte/lte), add a request to
"/api/commerces/products/filter/1?is_offer=false&price_min=10&price_max=15" and
assert both findMany and count are called with a where that only contains the
price clause (is_offer: false AND price gte/lte), and mirror the same
expect.objectContaining checks you used in the existing test (referencing
prisma.products.findMany and prisma.products.count) so the new tests fail if
either side of the OR or the count query is constructed incorrectly.
🪄 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: c43f76c8-7d3d-484a-a5f6-149004a2c065
📒 Files selected for processing (2)
src/modules/commerce/commerces/store.service.jstests/e2e/stores.test.js
Comment on lines
+841
to
875
| const effectivePriceRange = {}; | ||
| if (resolvedMinPrice !== undefined && resolvedMinPrice !== null) { | ||
| whereConditions.price = { gte: Number(resolvedMinPrice) }; | ||
| effectivePriceRange.gte = Number(resolvedMinPrice); | ||
| } | ||
|
|
||
| if (resolvedMaxPrice !== undefined && resolvedMaxPrice !== null) { | ||
| whereConditions.price = { | ||
| ...whereConditions.price, | ||
| lte: Number(resolvedMaxPrice) | ||
| }; | ||
| effectivePriceRange.lte = Number(resolvedMaxPrice); | ||
| } | ||
|
|
||
| if (Object.keys(effectivePriceRange).length > 0) { | ||
| const effectivePriceBranches = []; | ||
|
|
||
| if (normalizedIsOfferFilter !== true) { | ||
| effectivePriceBranches.push({ | ||
| AND: [ | ||
| { is_offer: false }, | ||
| { price: effectivePriceRange } | ||
| ] | ||
| }); | ||
| } | ||
|
|
||
| if (normalizedIsOfferFilter !== false) { | ||
| effectivePriceBranches.push({ | ||
| AND: [ | ||
| { is_offer: true }, | ||
| { offer_price: effectivePriceRange } | ||
| ] | ||
| }); | ||
| } | ||
|
|
||
| whereConditions.OR = [ | ||
| ...(Array.isArray(whereConditions.OR) ? whereConditions.OR : []), | ||
| ...effectivePriceBranches | ||
| ]; | ||
| } |
Contributor
There was a problem hiding this comment.
Validá price_min y price_max antes de armar el rango.
Acá Number("") termina en 0 y Number("abc") en NaN. Con el fallback a req.query, eso puede abrir el filtro de más o mandar un rango inválido a Prisma en vez de rechazar la request con 400. También conviene rechazar price_min > price_max antes de construir el OR.
🛠️ Propuesta de ajuste
const resolvedMinPrice = minPrice ?? price_min;
const resolvedMaxPrice = maxPrice ?? price_max;
+ const parseOptionalPrice = (value, fieldName) => {
+ if (value === undefined || value === null) {
+ return undefined;
+ }
+
+ if (typeof value === "string" && value.trim() === "") {
+ return undefined;
+ }
+
+ const parsedValue = Number(value);
+ if (!Number.isFinite(parsedValue)) {
+ throw { status: 400, message: `${fieldName} invalido` };
+ }
+
+ return parsedValue;
+ };
+
- const effectivePriceRange = {};
- if (resolvedMinPrice !== undefined && resolvedMinPrice !== null) {
- effectivePriceRange.gte = Number(resolvedMinPrice);
- }
+ const parsedMinPrice = parseOptionalPrice(resolvedMinPrice, "price_min");
+ const parsedMaxPrice = parseOptionalPrice(resolvedMaxPrice, "price_max");
+
+ if (
+ parsedMinPrice !== undefined &&
+ parsedMaxPrice !== undefined &&
+ parsedMinPrice > parsedMaxPrice
+ ) {
+ throw { status: 400, message: "price_min no puede ser mayor que price_max" };
+ }
+
+ const effectivePriceRange = {};
+ if (parsedMinPrice !== undefined) {
+ effectivePriceRange.gte = parsedMinPrice;
+ }
- if (resolvedMaxPrice !== undefined && resolvedMaxPrice !== null) {
- effectivePriceRange.lte = Number(resolvedMaxPrice);
+ if (parsedMaxPrice !== undefined) {
+ effectivePriceRange.lte = parsedMaxPrice;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/commerce/commerces/store.service.js` around lines 841 - 875,
Valida resolvedMinPrice y resolvedMaxPrice antes de construir
effectivePriceRange: rechaza valores vacíos o no numéricos (evitar que
Number("") => 0 o Number("abc") => NaN) y responde con 400; convierte sólo
cuando Number.isFinite(Number(value)) es true; además, si ambos existen valida
que Number(resolvedMinPrice) <= Number(resolvedMaxPrice) y rechaza con 400 si
no; después de estas comprobaciones procede a poblar effectivePriceRange y
construir las ramas que usan normalizedIsOfferFilter y whereConditions.OR como
ahora.
CrisNAC
approved these changes
Mar 27, 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.
Los cambios de coderabbit
Summary by CodeRabbit
Notas de Lanzamiento
Correcciones de Errores
Pruebas