Repository navigation
OM-127: Agregando correcta autenticación - #43
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughSe añade el middleware de autenticación en la ruta POST /product-review y el controlador pasa a leer Changes
Sequence Diagram(s)sequenceDiagram
participant Cliente as Cliente
participant Router as Router (routes)
participant Auth as Middleware authenticate
participant Controller as ProductReviewController
participant Service as createProductReviewService
Cliente->>Router: POST /product-review (body, token)
Router->>Auth: validar token (authenticate)
Auth-->>Router: usuario validado / 401 rechazado
Router->>Controller: createProductReview(req con user)
Controller->>Service: createProductReviewService(payload, customerId)
Service-->>Controller: resultado creado / error
Controller-->>Cliente: 201 Created / error (status y safeMessage)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
![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: 2
🧹 Nitpick comments (1)
src/modules/users/product-review/product-review.controller.js (1)
9-9: Evitá loguear el objetoerrorcompleto.En Line 9 conviene registrar sólo campos acotados (message/status) para reducir riesgo de exponer datos sensibles en logs.
🧹 Propuesta de logging más seguro
- console.error("Error creando reseña:", error); + console.error("Error creando reseña", { + message: error?.message, + status: error?.status + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/users/product-review/product-review.controller.js` at line 9, Replace the current full-object logging at the console.error call so you don't dump the whole error object; instead extract and log only safe fields (e.g. error.message and error.status or code) and any minimal contextual text (for example in the product-review controller's error handler where you call console.error("Error creando reseña:", error)). Update that site to log a concise message and the selected fields only (or use your app logger like logger.error with a message plus { status: error.status }) rather than the entire error object.
🤖 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/users/product-review/product-review.controller.js`:
- Around line 10-12: Normalize error.status to the valid HTTP error range by
replacing the current Number.isInteger(error?.status) check with a defensive
check that ensures error?.status is an integer between 400 and 599 (inclusive),
otherwise fallback to 500; update the assignment to the status variable
(currently using error?.status) and keep the subsequent
res.status(status).json(...) behavior so the controller always returns a valid
4xx/5xx status.
- Line 5: El controller usa directamente const customerId = req.user.id_user;
sin validación defensiva; cambia a comprobar que req.user?.id_user existe antes
de usarlo (mirror a product.controller.js), y si falta devolver un 401 con un
mensaje claro (por ejemplo "Token inválido o usuario no autenticado"), dejando
el resto de la función sin cambios; busca la variable customerId en
product-review.controller.js y aplica la validación previa al uso para evitar
TypeError y mantener consistencia con product.controller.js.
---
Nitpick comments:
In `@src/modules/users/product-review/product-review.controller.js`:
- Line 9: Replace the current full-object logging at the console.error call so
you don't dump the whole error object; instead extract and log only safe fields
(e.g. error.message and error.status or code) and any minimal contextual text
(for example in the product-review controller's error handler where you call
console.error("Error creando reseña:", error)). Update that site to log a
concise message and the selected fields only (or use your app logger like
logger.error with a message plus { status: error.status }) rather than the
entire error object.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7a8e4ff9-28ac-4328-b712-6aa2f2a81c65
📒 Files selected for processing (2)
src/modules/users/product-review/product-review.controller.jssrc/modules/users/product-review/product-review.routes.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/modules/users/product-review/product-review.controller.js (1)
5-5: Evitá dejar comentarios de revisión en código productivo.Los comentarios
Revisión 1/2parecen temporales del proceso de PR; te sugiero reemplazarlos por comentarios funcionales o removerlos.Also applies to: 16-16
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/users/product-review/product-review.controller.js` at line 5, Remove the temporary PR review comments "Revisión 1" and "Revisión 2" from product-review.controller.js and either delete them or replace them with concise, functional comments explaining intent (e.g., why extra defensive validation is present) so production code contains only meaningful documentation; search for the exact strings "Revisión 1" and "Revisión 2" in the file and update the surrounding comment to reference the relevant function or validation logic instead of keeping PR artifacts.
🤖 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/users/product-review/product-review.controller.js`:
- Around line 23-25: The response builder in the product-review controller
currently uses error.message directly for 4xx responses, which can be undefined;
update the return in the res.status(status).json(...) path to provide a fallback
string when error.message is falsy (e.g., use a default like "Solicitud
inválida" or "Error del cliente" or derive one from status) so that clients
always receive a useful message; locate the code using the variables/status and
res.status(...) in product-review.controller.js and change the message
expression to include this fallback.
---
Nitpick comments:
In `@src/modules/users/product-review/product-review.controller.js`:
- Line 5: Remove the temporary PR review comments "Revisión 1" and "Revisión 2"
from product-review.controller.js and either delete them or replace them with
concise, functional comments explaining intent (e.g., why extra defensive
validation is present) so production code contains only meaningful
documentation; search for the exact strings "Revisión 1" and "Revisión 2" in the
file and update the surrounding comment to reference the relevant function or
validation logic instead of keeping PR artifacts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ce90e788-efc6-44a7-804e-d9717234aabd
📒 Files selected for processing (1)
src/modules/users/product-review/product-review.controller.js
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/modules/users/product-review/product-review.controller.js (1)
16-24:⚠️ Potential issue | 🟠 MajorNormalizá
error.statusantes de usarlo enres.status.En Line 17-23 se usa
error.statusdirecto. Si llegaundefined,nullo un valor fuera de 4xx/5xx, el handler puede fallar o responder con status inválido. Esto además reintroduce un punto ya reportado antes.🔧 Ajuste propuesto
- const safeMessage = - error.status < 500 && typeof error?.message === "string" && error.message.trim() - ? error.message - : error.status < 500 - ? "Solicitud inválida" - : "Error interno del servidor"; + const status = + Number.isInteger(error?.status) && error.status >= 400 && error.status < 600 + ? error.status + : 500; + + const safeMessage = + status < 500 && typeof error?.message === "string" && error.message.trim() + ? error.message + : status < 500 + ? "Solicitud inválida" + : "Error interno del servidor"; - return res.status(error.status).json({ + return res.status(status).json({ message: safeMessage });En Express 5.x, ¿qué ocurre cuando `res.status(...)` recibe `undefined`, `null` o un código fuera del rango HTTP válido?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/users/product-review/product-review.controller.js` around lines 16 - 24, The handler uses error.status directly (seen in safeMessage and res.status) which can be undefined, null or out-of-range; normalize it first (e.g., compute a validatedStatus variable by coercing to a number, defaulting to 500 for missing/invalid values, and clamping to the 4xx/5xx range) and use that validatedStatus in the safeMessage logic and the res.status call (referencing safeMessage, error.status and res.status in the controller). Ensure the logic still treats <500 as client errors when appropriate but uses the validatedStatus value for both message selection and response status to avoid sending invalid HTTP codes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/modules/users/product-review/product-review.controller.js`:
- Around line 16-24: The handler uses error.status directly (seen in safeMessage
and res.status) which can be undefined, null or out-of-range; normalize it first
(e.g., compute a validatedStatus variable by coercing to a number, defaulting to
500 for missing/invalid values, and clamping to the 4xx/5xx range) and use that
validatedStatus in the safeMessage logic and the res.status call (referencing
safeMessage, error.status and res.status in the controller). Ensure the logic
still treats <500 as client errors when appropriate but uses the validatedStatus
value for both message selection and response status to avoid sending invalid
HTTP codes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2de41be4-5c07-4b5a-9c30-8d9c1fdb7be6
📒 Files selected for processing (1)
src/modules/users/product-review/product-review.controller.js
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
Nueva Funcionalidad
Correcciones de Errores