Repository navigation
Conversation
Contributor
📝 WalkthroughRecorridoEsta solicitud de fusión extiende el sistema de productos para soportar precios promocionales mediante la introducción de utilidades de cálculo de precios, actualización de esquemas y DTOs, y enriquecimiento de respuestas de productos con campos relacionados con ofertas (precioOriginal, precioOferta, esOferta) en los servicios de producto, tienda y lista de deseos. Cambios
Esfuerzo Estimado de Revisión de Código🎯 4 (Complejo) | ⏱️ ~60 minutos PRs Posiblemente Relacionadas
Revisores Sugeridos
Poema
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/modules/commerce/commerces/store.service.js (1)
836-848:⚠️ Potential issue | 🟠 MajorEl filtro de precio filtra por el precio base, no por el precio efectivo mostrado.
El filtro
price_min/price_maxopera sobre el campopricede la base de datos, pero la respuesta devuelve el precio efectivo calculado porgetEffectiveProductPrice()que usaoffer_pricecuandois_offer=true.Ejemplo: un producto con
price=100,offer_price=50,is_offer=truese mostrará a precio 50 pero no coincidirá con un filtroprice_max=60, generando una inconsistencia entre el precio mostrado y los resultados filtrados.Se recomienda aplicar el filtro también sobre el precio efectivo usando una condición OR:
Solución sugerida
whereConditions.OR = [ { AND: [ { price: { gte: Number(resolvedMinPrice), lte: Number(resolvedMaxPrice) } }, { is_offer: false } ] }, { AND: [ { offer_price: { gte: Number(resolvedMinPrice), lte: Number(resolvedMaxPrice) } }, { is_offer: true } ] } ];🤖 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 836 - 848, The current price filtering uses the DB column price but the UI shows getEffectiveProductPrice() (which uses offer_price when is_offer=true), so update the whereConditions construction (whereConditions, resolvedMinPrice, resolvedMaxPrice) to apply the range filter against the effective price via an OR: add two branches—one AND branch applying the numeric gte/lte constraints to price together with is_offer=false, and another AND branch applying the numeric gte/lte constraints to offer_price together with is_offer=true; ensure you handle cases where only min or max is provided (only include gte or lte as appropriate) and merge this OR into the existing whereConditions without clobbering other filters.
🧹 Nitpick comments (8)
src/modules/commerce/products/product.controller.js (1)
140-146: El fallback paraoriginal_pricees innecesario.Según el context snippet de
product.service.js(líneas 769-772),getProductsSearchServiceya retornaoriginal_pricecalculado víagetOriginalProductPrice(). El fallbackp.original_price === undefined ? Number(p.price)nunca se ejecutará.♻️ Simplificación sugerida
const offers = products.map((p) => ({ productId: p.id_product, name: p.name, description: p.description, price: Number(p.price), - originalPrice: - p.original_price === undefined ? Number(p.price) : Number(p.original_price), - offerPrice: - p.offer_price === undefined || p.offer_price === null - ? null - : Number(p.offer_price), - isOffer: Boolean(p.is_offer), + originalPrice: Number(p.original_price), + offerPrice: p.offer_price, + isOffer: p.is_offer, store: p.store🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/commerce/products/product.controller.js` around lines 140 - 146, El mapeo de originalPrice incluye un fallback innecesario; elimina la ternaria que cae a Number(p.price) y asigna simplemente Number(p.original_price) (o mantener un guard explícito solo si quieres evitar NaN), ya que getProductsSearchService junto con getOriginalProductPrice() garantiza original_price; actualiza la propiedad originalPrice en el mapeo (donde aparece p.original_price) para usar directamente Number(p.original_price).src/lib/product-pricing.js (1)
9-9: PosibleNaNsiproduct.priceesundefinedonull.
Number(undefined)oNumber(null)retornaNaN. Si bien en uso normalpricesiempre debería existir, considerar agregar validación defensiva como entoNullableNumber, o documentar quepricees requerido.🛡️ Validación defensiva opcional
-export const getOriginalProductPrice = (product) => Number(product.price); +export const getOriginalProductPrice = (product) => Number(product?.price ?? 0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/product-pricing.js` at line 9, La función getOriginalProductPrice convierte product.price con Number(...) y puede devolver NaN si price es undefined/null; cambia getOriginalProductPrice para validar de forma defensiva usando la utilidad toNullableNumber(product.price) y, si el resultado es null, lanzar un TypeError o devolver un valor por defecto claro (p. ej. 0) según la política del proyecto; asegúrate de referenciar getOriginalProductPrice y toNullableNumber en el cambio y documentar el comportamiento elegido.src/modules/global/dtos/products/product.request.dto.ts (1)
143-146: Inconsistencia:isOfferen filtros solo acepta"true"/"false", no"1"/"0".El esquema
booleanish(líneas 3-6) acepta"true","false","1","0", peroisOfferenFilterProductDTOsolo acepta"true"y"false". Considera reutilizar el patrón debooleanishpara consistencia con el resto de la API.♻️ Refactor sugerido
isOffer: z - .enum(["true", "false"]) - .transform((v) => v === "true") + .enum(["true", "false", "1", "0"]) + .transform((v) => v === "true" || v === "1") .optional(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/global/dtos/products/product.request.dto.ts` around lines 143 - 146, The isOffer field in FilterProductDTO currently uses a z.enum(["true","false"]) which is inconsistent with the existing booleanish schema; update FilterProductDTO to reuse the booleanish schema (or the same z.union/z.enum pattern used in booleanish) and apply the same .transform((v) => v === "true" || v === "1") logic so "true"/"false"/"1"/"0" are accepted and converted to a boolean; replace the current isOffer enum usage with a reference to booleanish (or mirror its union+transform) in the product.request.dto.ts to keep behavior consistent.src/modules/global/dtos/products/product.response.dto.ts (1)
48-56: Lógica de precios duplicada consrc/lib/product-pricing.js.Esta lógica replica exactamente lo que hace
getProductPricing()ensrc/lib/product-pricing.js(líneas 20-25). Reutilizar esa función centraliza la lógica y evita inconsistencias futuras.♻️ Refactor sugerido
+import { getProductPricing } from "../../../../lib/product-pricing.js"; + // En el constructor: - this.originalPrice = Number(data.price); - this.offerPrice = - data.offer_price === null || data.offer_price === undefined - ? null - : Number(data.offer_price); - this.isOffer = Boolean(data.is_offer); - this.price = this.isOffer && this.offerPrice !== null - ? this.offerPrice - : this.originalPrice; + const pricing = getProductPricing(data); + this.originalPrice = pricing.originalPrice; + this.offerPrice = pricing.offerPrice; + this.isOffer = pricing.isOffer; + this.price = pricing.price;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/global/dtos/products/product.response.dto.ts` around lines 48 - 56, La lógica de cálculo de precios en product.response.dto.ts duplica getProductPricing() de src/lib/product-pricing.js; importa y reutiliza esa función para evitar duplicación. Reemplaza el bloque que asigna this.originalPrice, this.offerPrice, this.isOffer y this.price por una llamada a getProductPricing(data) (o desestructura su resultado) y asigna los valores devueltos a las propiedades correspondientes (manteniendo la conversión Number/Boolean si getProductPricing no lo hace internamente). Asegúrate de importar exactamente getProductPricing desde src/lib/product-pricing.js y conservar el comportamiento previo de valores nulos/undefined para offerPrice.src/modules/global/dtos/commerce/filter-store-products.response.js (1)
9-14: Inconsistencia de convención de nombres entre DTOs de respuesta.El archivo
filter-store-products.response.jsusasnake_case(original_price,offer_price,is_offer) mientras queProductResponseDTOenproduct.response.dto.tsyfilter-store-products.dto.jsusancamelCase(originalPrice,offerPrice,isOffer). Esto genera respuestas de API inconsistentes: los mismos datos se retornan con nombres de propiedades diferentes según el endpoint.Uniformizar la convención de nombres en todos los DTOs de respuesta de este módulo.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/global/dtos/commerce/filter-store-products.response.js` around lines 9 - 14, The DTO in filter-store-products.response.js uses snake_case properties (original_price, offer_price, is_offer) causing inconsistent API responses; update the DTO to use camelCase property names (originalPrice, offerPrice, isOffer) and map values the same way as currently done (e.g., this.originalPrice = Number(data.original_price ?? data.price) or source accordingly), ensuring the constructor/mapping logic in the class (the lines setting original_price/offer_price/is_offer) is replaced with the camelCase names to match ProductResponseDTO and filter-store-products.dto.js so all response DTOs use the same naming convention.src/docs/schemas/commerce/product.schema.js (1)
87-106: Convención de nombres diferente en endpoint de búsqueda.
ProductSearchItemResponseusa snake_case (original_price,offer_price,is_offer) mientras queProductResponseusa camelCase. Esta inconsistencia está documentada pero podría generar confusión en los consumidores de la API. Considerar unificar la convención en el futuro.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/docs/schemas/commerce/product.schema.js` around lines 87 - 106, La definición ProductSearchItemResponse usa snake_case para campos (original_price, offer_price, is_offer, id_store) y debe unificarse con la convención de ProductResponse; rename esos campos a camelCase (originalPrice, offerPrice, isOffer, idStore) manteniendo tipos, nullable y ejemplos, y actualizar cualquier uso/serialización relacionado para evitar ruptura en ProductSearchItemResponse y en el objeto store dentro de esa respuesta.src/modules/commerce/commerces/store.service.js (1)
155-174: Duplicación deparseBooleanFieldentre servicios.Esta función es idéntica a la implementada en
src/modules/commerce/products/product.service.js(líneas 168-194). Considerar extraerla a un módulo utilitario compartido comosrc/lib/parsers.jspara evitar duplicación y facilitar mantenimiento.♻️ Sugerencia de refactor
Crear un módulo compartido:
// src/lib/parsers.js export const parseBooleanField = (value, fieldName) => { if (typeof value === "boolean") { return value; } // ... resto de la implementación };Luego importar en ambos servicios:
+import { parseBooleanField } from "../../../lib/parsers.js";🤖 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 155 - 174, The parseBooleanField helper is duplicated; extract the function into a shared module (e.g., parsers.js) and export it, then replace the local definitions in both services by importing parseBooleanField; update the existing usages in store.service (parseBooleanField) and product.service (parseBooleanField) to import from the shared module so there is a single implementation to maintain.src/modules/commerce/products/product.service.js (1)
765-786: Mapeo de respuesta con campos de precio calculados.El mapeo usa snake_case (
original_price,offer_price,is_offer) para el endpoint de búsqueda, lo cual es consistente con el schemaProductSearchItemResponse. Nota: el endpoint de detalle usa camelCase (originalPrice,offerPrice,isOffer), lo cual es una convención intencional pero podría causar confusión a los consumidores de la API.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/commerce/products/product.service.js` around lines 765 - 786, The search response is using snake_case keys (original_price, offer_price, is_offer) while the product detail endpoint uses camelCase; pick a single convention and update the search mapping to match detail: change the mapped keys inside the products.map block to originalPrice, offerPrice, isOffer (keeping values from getOriginalProductPrice(product), getOfferProductPrice(product), and Boolean(product.is_offer)), and update the ProductSearchItemResponse schema/type to camelCase as well so both endpoints are consistent; alternatively, if you prefer snake_case, make the detail endpoint use original_price/offer_price/is_offer—apply the change around the products: products.map(...) mapping and corresponding response type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/modules/commerce/commerces/store.service.js`:
- Around line 836-848: The current price filtering uses the DB column price but
the UI shows getEffectiveProductPrice() (which uses offer_price when
is_offer=true), so update the whereConditions construction (whereConditions,
resolvedMinPrice, resolvedMaxPrice) to apply the range filter against the
effective price via an OR: add two branches—one AND branch applying the numeric
gte/lte constraints to price together with is_offer=false, and another AND
branch applying the numeric gte/lte constraints to offer_price together with
is_offer=true; ensure you handle cases where only min or max is provided (only
include gte or lte as appropriate) and merge this OR into the existing
whereConditions without clobbering other filters.
---
Nitpick comments:
In `@src/docs/schemas/commerce/product.schema.js`:
- Around line 87-106: La definición ProductSearchItemResponse usa snake_case
para campos (original_price, offer_price, is_offer, id_store) y debe unificarse
con la convención de ProductResponse; rename esos campos a camelCase
(originalPrice, offerPrice, isOffer, idStore) manteniendo tipos, nullable y
ejemplos, y actualizar cualquier uso/serialización relacionado para evitar
ruptura en ProductSearchItemResponse y en el objeto store dentro de esa
respuesta.
In `@src/lib/product-pricing.js`:
- Line 9: La función getOriginalProductPrice convierte product.price con
Number(...) y puede devolver NaN si price es undefined/null; cambia
getOriginalProductPrice para validar de forma defensiva usando la utilidad
toNullableNumber(product.price) y, si el resultado es null, lanzar un TypeError
o devolver un valor por defecto claro (p. ej. 0) según la política del proyecto;
asegúrate de referenciar getOriginalProductPrice y toNullableNumber en el cambio
y documentar el comportamiento elegido.
In `@src/modules/commerce/commerces/store.service.js`:
- Around line 155-174: The parseBooleanField helper is duplicated; extract the
function into a shared module (e.g., parsers.js) and export it, then replace the
local definitions in both services by importing parseBooleanField; update the
existing usages in store.service (parseBooleanField) and product.service
(parseBooleanField) to import from the shared module so there is a single
implementation to maintain.
In `@src/modules/commerce/products/product.controller.js`:
- Around line 140-146: El mapeo de originalPrice incluye un fallback
innecesario; elimina la ternaria que cae a Number(p.price) y asigna simplemente
Number(p.original_price) (o mantener un guard explícito solo si quieres evitar
NaN), ya que getProductsSearchService junto con getOriginalProductPrice()
garantiza original_price; actualiza la propiedad originalPrice en el mapeo
(donde aparece p.original_price) para usar directamente
Number(p.original_price).
In `@src/modules/commerce/products/product.service.js`:
- Around line 765-786: The search response is using snake_case keys
(original_price, offer_price, is_offer) while the product detail endpoint uses
camelCase; pick a single convention and update the search mapping to match
detail: change the mapped keys inside the products.map block to originalPrice,
offerPrice, isOffer (keeping values from getOriginalProductPrice(product),
getOfferProductPrice(product), and Boolean(product.is_offer)), and update the
ProductSearchItemResponse schema/type to camelCase as well so both endpoints are
consistent; alternatively, if you prefer snake_case, make the detail endpoint
use original_price/offer_price/is_offer—apply the change around the products:
products.map(...) mapping and corresponding response type.
In `@src/modules/global/dtos/commerce/filter-store-products.response.js`:
- Around line 9-14: The DTO in filter-store-products.response.js uses snake_case
properties (original_price, offer_price, is_offer) causing inconsistent API
responses; update the DTO to use camelCase property names (originalPrice,
offerPrice, isOffer) and map values the same way as currently done (e.g.,
this.originalPrice = Number(data.original_price ?? data.price) or source
accordingly), ensuring the constructor/mapping logic in the class (the lines
setting original_price/offer_price/is_offer) is replaced with the camelCase
names to match ProductResponseDTO and filter-store-products.dto.js so all
response DTOs use the same naming convention.
In `@src/modules/global/dtos/products/product.request.dto.ts`:
- Around line 143-146: The isOffer field in FilterProductDTO currently uses a
z.enum(["true","false"]) which is inconsistent with the existing booleanish
schema; update FilterProductDTO to reuse the booleanish schema (or the same
z.union/z.enum pattern used in booleanish) and apply the same .transform((v) =>
v === "true" || v === "1") logic so "true"/"false"/"1"/"0" are accepted and
converted to a boolean; replace the current isOffer enum usage with a reference
to booleanish (or mirror its union+transform) in the product.request.dto.ts to
keep behavior consistent.
In `@src/modules/global/dtos/products/product.response.dto.ts`:
- Around line 48-56: La lógica de cálculo de precios en product.response.dto.ts
duplica getProductPricing() de src/lib/product-pricing.js; importa y reutiliza
esa función para evitar duplicación. Reemplaza el bloque que asigna
this.originalPrice, this.offerPrice, this.isOffer y this.price por una llamada a
getProductPricing(data) (o desestructura su resultado) y asigna los valores
devueltos a las propiedades correspondientes (manteniendo la conversión
Number/Boolean si getProductPricing no lo hace internamente). Asegúrate de
importar exactamente getProductPricing desde src/lib/product-pricing.js y
conservar el comportamiento previo de valores nulos/undefined para offerPrice.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: de5cf3f5-8168-4cc4-a70c-de9119826c3e
📒 Files selected for processing (13)
src/docs/schemas/commerce/product.schema.jssrc/lib/product-pricing.jssrc/modules/commerce/commerces/store.service.jssrc/modules/commerce/products/product.controller.jssrc/modules/commerce/products/product.routes.jssrc/modules/commerce/products/product.service.jssrc/modules/global/dtos/commerce/filter-store-products.dto.jssrc/modules/global/dtos/commerce/filter-store-products.response.jssrc/modules/global/dtos/products/product.request.dto.tssrc/modules/global/dtos/products/product.response.dto.tssrc/modules/users/wishlist/wishlist.service.jstests/e2e/products.test.jstests/e2e/stores.test.js
CrisNAC
approved these changes
Mar 26, 2026
This was referenced Mar 26, 2026
Merged
Closed
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.
Agregue filtro de ofertas y agregue los nuevos campos de productos como respuestas en otros metodos
Summary by CodeRabbit
Release Notes