Repository navigation
Conversation
Contributor
📝 WalkthroughResumen GeneralEl servicio Cambios
Esfuerzo estimado de revisión de código🎯 3 (Moderado) | ⏱️ ~20 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)
⚔️ Resolve merge conflicts
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/users/users/services/users.services.js (1)
298-301:⚠️ Potential issue | 🟠 MajorInconsistencia en la autorización entre servicios de actualización.
updateUserServiceusagetAuthorizedUserForUpdateService(permite cualquier rol autenticado), peroupdateUserPasswordServiceusagetAuthorizedCustomerService(solo permite CUSTOMER). Esto significa que usuarios con rol SELLER o ADMIN pueden actualizar su nombre/email/teléfono, pero no pueden cambiar su contraseña.Aclarar si este comportamiento es intencional o si se debe unificar la autorización en ambos servicios.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/users/users/services/users.services.js` around lines 298 - 301, Hay una inconsistencia de autorización: updateUserService usa getAuthorizedUserForUpdateService (permite cualquier rol autenticado) mientras que updateUserPasswordService usa getAuthorizedCustomerService (solo CUSTOMER), lo que impide que SELLER/ADMIN cambien su contraseña; unifica la política reemplazando la llamada a getAuthorizedCustomerService en updateUserPasswordService por getAuthorizedUserForUpdateService (o ajusta ambas funciones para compartir una nueva función común de autorización) y asegúrate de mantener/propagar los mismos parámetros y comprobaciones (authenticatedUserId, requestedUserId) y de actualizar los tests/documentación según corresponda.
🧹 Nitpick comments (2)
src/modules/users/users/services/users.services.js (2)
387-412: Duplicación de código congetAuthorizedCustomerService.Esta nueva función es casi idéntica a
getAuthorizedCustomerService(líneas 51-99), diferenciándose solo en la ausencia de la verificación de rol CUSTOMER. Considerar refactorizar para evitar duplicación.♻️ Refactor sugerido - extraer lógica común
+const getAuthorizedUserBase = async (authenticatedUserId, requestedUserId, errorMessage) => { + if (!authenticatedUserId) { + throw { status: 401, message: "Usuario autenticado requerido" }; + } + + const authenticatedId = parsePositiveInteger(authenticatedUserId, "ID de usuario autenticado"); + const targetUserId = parsePositiveInteger(requestedUserId, "ID de usuario"); + + if (authenticatedId !== targetUserId) { + throw { status: 403, message: errorMessage }; + } + + const user = await prisma.users.findUnique({ + where: { id_user: targetUserId }, + select: { id_user: true, role: true, status: true }, + }); + + if (!user || !user.status) { + throw { status: 404, message: "Usuario no encontrado o inactivo" }; + } + + return user; +}; + +export const getAuthorizedCustomerService = async (authenticatedUserId, requestedUserId) => { + const user = await getAuthorizedUserBase(authenticatedUserId, requestedUserId, "No tiene permisos para editar este perfil"); + if (user.role !== "CUSTOMER") { + throw { status: 403, message: "El usuario no es un cliente" }; + } + return user; +}; + +const getAuthorizedUserForUpdateService = async (authenticatedUserId, requestedUserId) => { + return getAuthorizedUserBase(authenticatedUserId, requestedUserId, "No tiene permisos para editar este perfil"); +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/users/users/services/users.services.js` around lines 387 - 412, The getAuthorizedUserForUpdateService duplicates most logic from getAuthorizedCustomerService; extract the shared validation and lookup into a helper (e.g., authorizeAndFetchUser or getAuthorizedUserById) that accepts parameters for whether to enforce a specific role, reusing parsePositiveInteger and prisma.users.findUnique to validate IDs and fetch {id_user, role, status}; then implement getAuthorizedCustomerService by calling the helper with role="CUSTOMER" and getAuthorizedUserForUpdateService by calling it with no role enforcement (or role=null) so both functions delegate to the common logic.
191-191: Nombre de variable engañoso.La variable
customerahora puede contener cualquier tipo de usuario (no solo CUSTOMER). Considerar renombrarla auseroauthorizedUserpara mayor claridad.♻️ Refactor sugerido
- const customer = await getAuthorizedUserForUpdateService(authenticatedUserId, requestedUserId); + const user = await getAuthorizedUserForUpdateService(authenticatedUserId, requestedUserId);Y actualizar las referencias en las líneas 265 y 284:
- if (existingUser && existingUser.id_user !== customer.id_user) { + if (existingUser && existingUser.id_user !== user.id_user) {- id_user: customer.id_user, + id_user: user.id_user,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/users/users/services/users.services.js` at line 191, La variable llamada `customer` que recibe el resultado de getAuthorizedUserForUpdateService debe renombrarse a algo genérico como `user` o `authorizedUser` porque puede representar cualquier tipo de usuario; update todas las referencias posteriores que usan `customer` (por ejemplo las lecturas/propagaciones que actualmente aparecen tras la llamada a getAuthorizedUserForUpdateService) para usar el nuevo nombre (`user` o `authorizedUser`) y mantener consistencia en las líneas donde se lee/usa ese valor (referencias actuales a `customer` en el bloque de código posterior deben actualizarse).
🤖 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/users/users/services/users.services.js`:
- Around line 298-301: Hay una inconsistencia de autorización: updateUserService
usa getAuthorizedUserForUpdateService (permite cualquier rol autenticado)
mientras que updateUserPasswordService usa getAuthorizedCustomerService (solo
CUSTOMER), lo que impide que SELLER/ADMIN cambien su contraseña; unifica la
política reemplazando la llamada a getAuthorizedCustomerService en
updateUserPasswordService por getAuthorizedUserForUpdateService (o ajusta ambas
funciones para compartir una nueva función común de autorización) y asegúrate de
mantener/propagar los mismos parámetros y comprobaciones (authenticatedUserId,
requestedUserId) y de actualizar los tests/documentación según corresponda.
---
Nitpick comments:
In `@src/modules/users/users/services/users.services.js`:
- Around line 387-412: The getAuthorizedUserForUpdateService duplicates most
logic from getAuthorizedCustomerService; extract the shared validation and
lookup into a helper (e.g., authorizeAndFetchUser or getAuthorizedUserById) that
accepts parameters for whether to enforce a specific role, reusing
parsePositiveInteger and prisma.users.findUnique to validate IDs and fetch
{id_user, role, status}; then implement getAuthorizedCustomerService by calling
the helper with role="CUSTOMER" and getAuthorizedUserForUpdateService by calling
it with no role enforcement (or role=null) so both functions delegate to the
common logic.
- Line 191: La variable llamada `customer` que recibe el resultado de
getAuthorizedUserForUpdateService debe renombrarse a algo genérico como `user` o
`authorizedUser` porque puede representar cualquier tipo de usuario; update
todas las referencias posteriores que usan `customer` (por ejemplo las
lecturas/propagaciones que actualmente aparecen tras la llamada a
getAuthorizedUserForUpdateService) para usar el nuevo nombre (`user` o
`authorizedUser`) y mantener consistencia en las líneas donde se lee/usa ese
valor (referencias actuales a `customer` en el bloque de código posterior deben
actualizarse).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9aaa6695-b841-467d-86d8-8df6d1cac2db
📒 Files selected for processing (1)
src/modules/users/users/services/users.services.js
CrisNAC
approved these changes
Mar 24, 2026
Merged
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 una funcion en Users para autorizar cualquier tipo de usuario
Summary by CodeRabbit
Notas de versión
Bug Fixes
Refactor