Repository navigation
Conversation
📝 WalkthroughWalkthroughSe implementó una funcionalidad de búsqueda de productos con soporte para filtrado por nombre/descripción, paginación y ordenamiento por relevancia. Se agregó un generador Prisma para fullTextSearchPostgres, un nuevo endpoint GET, controlador y servicio con lógica de búsqueda e filtraje. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Router
participant Controller
participant Service
participant Database
Client->>Router: GET /?search=keyword&page=1&limit=10
Router->>Controller: getProductsSearch(request, response)
Controller->>Service: getProductsSearchService(query)
Service->>Database: COUNT products (status=true, visible=true)
Service->>Database: SELECT products (filtered, paginated, ordered)
Database-->>Service: totalProducts, products[]
Service-->>Controller: {products, pagination}
Controller-->>Client: 200 {data, pagination}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/modules/commerce/products/product.controller.js (2)
24-27: Elconsole.infoestá ubicado después de obtener los productos.El mensaje "Obteniendo productos..." se loguea después de que los productos ya fueron obtenidos. Si el propósito es trazar el inicio de la operación, debería estar antes de la llamada al servicio. Si es para confirmar éxito, el mensaje debería reflejar eso.
♻️ Sugerencia para corregir el orden del log
export const getProductsSearch = async (request, response) => { try { + console.info("Obteniendo productos..."); const filterProducts = await getProductsSearchService(request.query); - console.info("Obteniendo productos...") return response.status(200).json(filterProducts); }🤖 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 24 - 27, El log "Obteniendo productos..." está después de la llamada a getProductsSearchService, por lo que no marca el inicio de la operación; mueve la llamada a console.info antes de invocar getProductsSearchService(request.query) (por ejemplo colocar console.info("Obteniendo productos...") justo antes de la llamada) o, si prefieres confirmar éxito, cambia el texto del log posterior a algo como "Productos obtenidos" y deja el inicio antes; edita el bloque que usa getProductsSearchService y filterProducts para reflejar ese cambio.
17-17: Pequeño typo en el comentario.Falta un espacio: "losproductos" → "los productos".
🤖 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` at line 17, Corrige el comentario en product.controller.js donde aparece "/** GET: Para obtener todos losproductos..." cambiando "losproductos" a "los productos" (localiza el comentario text "/** GET: Para obtener todos losproductos o buscar segun nombre o descripcion (por el momento)." y añade el espacio entre "los" y "productos").src/modules/commerce/products/product.service.js (1)
254-260: El ordenamiento por relevancia solo consideraname, pero la búsqueda incluyedescription.La búsqueda con
containsbusca ennameydescription, pero_relevancesolo calcula score sobrename. Esto puede causar que productos que coinciden solo endescriptionaparezcan con relevancia baja o cero.♻️ Considerar incluir description en _relevance
orderBy = { _relevance: { - fields: ['name'], + fields: ['name', 'description'], search: search, sort: 'desc', }, }🤖 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 254 - 260, La ordenación por relevancia (la variable orderBy con _relevance y fields: ['name']) solo calcula score sobre name mientras la búsqueda usa search sobre name y description; actualiza orderBy._relevance.fields para incluir 'description' junto a 'name' (p. ej. fields: ['name','description']) — asegurándote de preservar la variable search y la estructura _relevance para que los resultados que coincidan solo en description obtengan puntuación de relevancia coherente.
🤖 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.routes.js`:
- Around line 6-7: The POST route currently calls createProduct without
authentication; add the existing authenticate middleware (the same used in
store.routes.js) to the router.post call so requests to createProduct run
authentication first and reject missing/invalid x-user-id before hitting the
controller; update the route declaration to include authenticate between
router.post and createProduct to ensure consistent validation.
In `@src/modules/commerce/products/product.service.js`:
- Around line 238-240: The limit variable currently has no upper bound, allowing
very large queries; clamp it to a safe maximum (e.g., 100) before computing
skip. Update the logic where page, limit, and skip are computed in
product.service.js (variables page, limit, skip) to coerce filters.limit to
Number, enforce a maxLimit (100), and use the bounded value when calculating
skip so clients cannot request more than 100 items per page.
---
Nitpick comments:
In `@src/modules/commerce/products/product.controller.js`:
- Around line 24-27: El log "Obteniendo productos..." está después de la llamada
a getProductsSearchService, por lo que no marca el inicio de la operación; mueve
la llamada a console.info antes de invocar
getProductsSearchService(request.query) (por ejemplo colocar
console.info("Obteniendo productos...") justo antes de la llamada) o, si
prefieres confirmar éxito, cambia el texto del log posterior a algo como
"Productos obtenidos" y deja el inicio antes; edita el bloque que usa
getProductsSearchService y filterProducts para reflejar ese cambio.
- Line 17: Corrige el comentario en product.controller.js donde aparece "/**
GET: Para obtener todos losproductos..." cambiando "losproductos" a "los
productos" (localiza el comentario text "/** GET: Para obtener todos
losproductos o buscar segun nombre o descripcion (por el momento)." y añade el
espacio entre "los" y "productos").
In `@src/modules/commerce/products/product.service.js`:
- Around line 254-260: La ordenación por relevancia (la variable orderBy con
_relevance y fields: ['name']) solo calcula score sobre name mientras la
búsqueda usa search sobre name y description; actualiza
orderBy._relevance.fields para incluir 'description' junto a 'name' (p. ej.
fields: ['name','description']) — asegurándote de preservar la variable search y
la estructura _relevance para que los resultados que coincidan solo en
description obtengan puntuación de relevancia coherente.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2dcd627f-f898-4713-900e-db027016013c
📒 Files selected for processing (4)
prisma/schema.prismasrc/modules/commerce/products/product.controller.jssrc/modules/commerce/products/product.routes.jssrc/modules/commerce/products/product.service.js
| router.post("/", createProduct); | ||
| router.get("/", getProductsSearch); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verificar cómo se registran las rutas de productos y si hay middlewares de autenticación aplicados a nivel superior
# Buscar dónde se monta el router de productos
rg -n "product" --type js -g '*.routes.js' -g 'index.js' -g 'app.js' -A 3 -B 1
# Verificar si hay middleware de autenticación
rg -n "auth" --type js -g '*.js' -A 2Repository: CrisNAC/BackendMarketplace
Length of output: 5260
🏁 Script executed:
# Ver el archivo completo de rutas de productos
cat -n src/modules/commerce/products/product.routes.js
# Ver la implementación del controlador getProductsSearch
cat -n src/modules/commerce/products/product.controller.jsRepository: CrisNAC/BackendMarketplace
Length of output: 1979
Agregar middleware de autenticación a la ruta POST.
La ruta GET / sin autenticación es intencional—es un endpoint público para búsqueda de productos (como se indica en el comentario del controlador, línea 22). Sin embargo, la ruta POST / depende de x-user-id extraído del cliente sin validar. El servicio interno valida su presencia (lanzando 401 si falta), pero es más seguro usar el middleware authenticate importado en otras rutas (store.routes.js). Esto proporciona validación consistente y fallará antes de llegar al controlador.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/commerce/products/product.routes.js` around lines 6 - 7, The POST
route currently calls createProduct without authentication; add the existing
authenticate middleware (the same used in store.routes.js) to the router.post
call so requests to createProduct run authentication first and reject
missing/invalid x-user-id before hitting the controller; update the route
declaration to include authenticate between router.post and createProduct to
ensure consistent validation.
| const page = Number(filters.page) > 0 ? Number(filters.page) : 1; | ||
| const limit = Number(filters.limit) > 0 ? Number(filters.limit) : 20; //por defecto trae hasta 20 productos | ||
| const skip = (page - 1) * limit; |
There was a problem hiding this comment.
Agregar límite máximo al parámetro limit para evitar consultas excesivamente grandes.
No hay un límite superior para limit, lo que podría permitir que un cliente solicite miles de registros en una sola petición. Otros servicios similares en el código (como product-category.service.js y product-tag.service.js) limitan a 100.
🛡️ Sugerencia para agregar límite máximo
//Paginacion
const page = Number(filters.page) > 0 ? Number(filters.page) : 1;
- const limit = Number(filters.limit) > 0 ? Number(filters.limit) : 20; //por defecto trae hasta 20 productos
+ const limitRaw = Number(filters.limit);
+ const limit = Number.isInteger(limitRaw) && limitRaw > 0
+ ? Math.min(limitRaw, 100)
+ : 20; //por defecto trae hasta 20 productos, máximo 100
const skip = (page - 1) * limit;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const page = Number(filters.page) > 0 ? Number(filters.page) : 1; | |
| const limit = Number(filters.limit) > 0 ? Number(filters.limit) : 20; //por defecto trae hasta 20 productos | |
| const skip = (page - 1) * limit; | |
| const page = Number(filters.page) > 0 ? Number(filters.page) : 1; | |
| const limitRaw = Number(filters.limit); | |
| const limit = Number.isInteger(limitRaw) && limitRaw > 0 | |
| ? Math.min(limitRaw, 100) | |
| : 20; //por defecto trae hasta 20 productos, máximo 100 | |
| const skip = (page - 1) * limit; |
🤖 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 238 - 240, The
limit variable currently has no upper bound, allowing very large queries; clamp
it to a safe maximum (e.g., 100) before computing skip. Update the logic where
page, limit, and skip are computed in product.service.js (variables page, limit,
skip) to coerce filters.limit to Number, enforce a maxLimit (100), and use the
bounded value when calculating skip so clients cannot request more than 100
items per page.
El ticket OM-77 terminado.
Obs.: Hacer "npx prisma generate" debido a un cambio en schema.prisma.
Summary by CodeRabbit
Notas de Lanzamiento