Repository navigation
Conversation
Contributor
📝 WalkthroughWalkthroughSe eliminó el endpoint de eliminación de productos y su servicio asociado, reemplazándolos por un nuevo endpoint de comparación que reutiliza el servicio de búsqueda con límites expandidos. El servicio de búsqueda se mejoró con sanitización de términos y ordenamiento condicional de relevancia. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutos Possibly related PRs
Suggested reviewers
Poem
🚥 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)
📝 Coding Plan
Comment Tip You can customize the high-level summary generated by CodeRabbit.Configure the |
![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)
src/modules/commerce/products/product.controller.js (1)
93-101: El endpoint de comparación debería validar que se proporcione un término de búsqueda o categoría.Actualmente, si se llama a
/compare/searchsinsearchnicategoryId, el endpoint retorna los primeros 50 productos ordenados por ID, lo cual no tiene sentido semántico para una funcionalidad de "comparar productos similares".✨ Sugerencia para validar parámetros requeridos
export const compareProducts = async (request, response) => { try { const { search, categoryId } = request.query; + if (!search?.trim() && !categoryId) { + return response.status(400).json({ + message: "Se requiere al menos un término de búsqueda o categoría para comparar productos" + }); + } + // Reutiliza la búsqueda actual, pero con un límite mayor const filters = { search, categoryId, page: 1, limit: 50 };🤖 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 93 - 101, El endpoint de comparación construye los filtros (variable filters) a partir de request.query pero no valida que exista al menos search o categoryId; agrega una comprobación al inicio del handler del endpoint /compare/search (antes de crear filters) que si ni request.query.search ni request.query.categoryId están presentes responde con un error 400 y un mensaje claro (por ejemplo "search or categoryId required") y no procede a consultar productos; deja intacta la creación de filters y la lógica posterior cuando la validación pasa.src/modules/commerce/products/product.service.js (1)
570-588: La sanitización de búsqueda puede generarsafeSearchvacío que aún activa el ordenamiento por relevancia.Cuando el término de búsqueda contiene solo caracteres reservados (por ejemplo
":::"o"&&&"), la sanitización los reemplaza por espacios, y después deltrim()ysplit(/\s+/),safeSearchresulta en""(string vacío). Sin embargo, el string vacío es falsy, por lo que el ternario en la línea 580 caería correctamente alorderBypor defecto.No obstante, existe un edge case: si
searches" & "(espacios con un&), después de.replace(/[':()&|!]/g, " ")queda" ", luego.trim()da"", y.split(/\s+/)sobre string vacío da[""], y.join(" & ")da"". Esto es correcto.Sin embargo, recomendaría validar explícitamente que
safeSearchno esté vacío antes de usarlo para mayor claridad y evitar posibles errores en futuras versiones de Prisma con_relevance.♻️ Sugerencia para mayor robustez
const safeSearch = search // reemplazar caracteres reservados de tsquery por espacios .replace(/[':()&|!]/g, " ") .trim() // convertir espacios múltiples en operador AND .split(/\s+/) + .filter(Boolean) .join(" & "); // cambia el orden, solo si se busca o filtra por algun parametro - orderBy = safeSearch + orderBy = safeSearch.length > 0 ? { _relevance: { fields: ["name"], search: safeSearch, sort: "desc" } } : orderBy;🤖 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 570 - 588, The current sanitization chain (the safeSearch variable created via .replace(/[':()&|!]/g, " ").trim().split(/\s+/).join(" & ")) can produce an empty string that still risks being used for Prisma's _relevance ordering; update the logic so you explicitly validate safeSearch is non-empty before assigning orderBy: compute safeSearch as you do, then check e.g. that safeSearch.trim().length > 0 (or Boolean(safeSearch)) and only then set orderBy = { _relevance: { fields: ["name"], search: safeSearch, sort: "desc" } }, otherwise leave orderBy unchanged; reference safeSearch and orderBy/_relevance to locate and change the code.
🤖 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/products/product.controller.js`:
- Around line 91-141: The compareProducts handler is returning the baseProduct
twice because offers is built from all products including the first
(baseProduct); update the offers construction in compareProducts so it excludes
the base product (e.g., filter products where p.id_product !==
baseProduct.id_product or use products.slice(1) before mapping) and then map the
remaining items to the existing offer shape; keep baseProduct as the separate
product field and ensure pagination/response unchanged.
---
Nitpick comments:
In `@src/modules/commerce/products/product.controller.js`:
- Around line 93-101: El endpoint de comparación construye los filtros (variable
filters) a partir de request.query pero no valida que exista al menos search o
categoryId; agrega una comprobación al inicio del handler del endpoint
/compare/search (antes de crear filters) que si ni request.query.search ni
request.query.categoryId están presentes responde con un error 400 y un mensaje
claro (por ejemplo "search or categoryId required") y no procede a consultar
productos; deja intacta la creación de filters y la lógica posterior cuando la
validación pasa.
In `@src/modules/commerce/products/product.service.js`:
- Around line 570-588: The current sanitization chain (the safeSearch variable
created via .replace(/[':()&|!]/g, " ").trim().split(/\s+/).join(" & ")) can
produce an empty string that still risks being used for Prisma's _relevance
ordering; update the logic so you explicitly validate safeSearch is non-empty
before assigning orderBy: compute safeSearch as you do, then check e.g. that
safeSearch.trim().length > 0 (or Boolean(safeSearch)) and only then set orderBy
= { _relevance: { fields: ["name"], search: safeSearch, sort: "desc" } },
otherwise leave orderBy unchanged; reference safeSearch and orderBy/_relevance
to locate and change the code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fb5d0780-9096-45b6-9bcc-00df4dae9f42
📒 Files selected for processing (3)
src/modules/commerce/products/product.controller.jssrc/modules/commerce/products/product.routes.jssrc/modules/commerce/products/product.service.js
Comment on lines
+91
to
+141
| export const compareProducts = async (request, response) => { | ||
| try { | ||
| const { search, categoryId } = request.query; | ||
|
|
||
| // Reutiliza la búsqueda actual, pero con un límite mayor | ||
| const filters = { | ||
| search, | ||
| categoryId, | ||
| page: 1, | ||
| limit: 50 | ||
| }; | ||
|
|
||
| const result = await getProductsSearchService(filters); | ||
| const products = Array.isArray(result?.products) ? result.products : []; | ||
|
|
||
| if (!products.length) { | ||
| return response.status(200).json({ | ||
| product: null, | ||
| offers: [], | ||
| pagination: result?.pagination || null | ||
| }); | ||
| } | ||
|
|
||
| // Producto base: el primero de la lista (más relevante según tu servicio) | ||
| const baseProduct = products[0]; | ||
|
|
||
| const offers = products.map((p) => ({ | ||
| productId: p.id_product, | ||
| name: p.name, | ||
| description: p.description, | ||
| price: Number(p.price), | ||
| store: p.store | ||
| ? { | ||
| id: p.store.id_store, | ||
| name: p.store.name | ||
| } | ||
| : null | ||
| })); | ||
|
|
||
| return response.status(200).json({ | ||
| product: baseProduct, | ||
| offers, | ||
| pagination: result.pagination | ||
| }); | ||
| } catch (error) { | ||
| console.error("Error al comparar productos:", error); | ||
| return response | ||
| .status(error.status || 500) | ||
| .json({ message: error.message || "Error interno del servidor." }); | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
El endpoint compareProducts incluye el producto base duplicado en el array de ofertas.
El producto base (baseProduct) es el primer elemento de la lista, y luego el array offers incluye todos los productos, incluyendo ese mismo producto base. Esto resulta en datos duplicados en la respuesta.
Si la intención es mostrar el producto de referencia separado de las demás ofertas, deberías excluirlo del array offers:
🐛 Propuesta para evitar duplicación
// Producto base: el primero de la lista (más relevante según tu servicio)
const baseProduct = products[0];
- const offers = products.map((p) => ({
+ const offers = products.slice(1).map((p) => ({
productId: p.id_product,
name: p.name,
description: p.description,
price: Number(p.price),
store: p.store
? {
id: p.store.id_store,
name: p.store.name
}
: null
}));🤖 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 91 - 141,
The compareProducts handler is returning the baseProduct twice because offers is
built from all products including the first (baseProduct); update the offers
construction in compareProducts so it excludes the base product (e.g., filter
products where p.id_product !== baseProduct.id_product or use products.slice(1)
before mapping) and then map the remaining items to the existing offer shape;
keep baseProduct as the separate product field and ensure pagination/response
unchanged.
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 la Versión
Nuevas Funcionalidades
Mejoras
Cambios Removidos