Repository navigation
Conversation
…uct, store y user. Falta agregar a los demas endponits.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSe agrega soporte de imágenes: dependencias (Supabase, Multer), validación de entorno, constantes, middlewares (upload, roles, error), rutas, controladores y servicios que suben/eliminan archivos en Supabase y actualizan Prisma; además migración para campos de imagen y ajustes de configuración Prisma/app. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Router as "Express Router"
participant Auth as "authenticate / requireRole"
participant Multer as "Multer (upload.single)"
participant Controller as "Controller"
participant Service as "Image Service"
participant Prisma
participant Supabase
Client->>Router: POST /products/:id/image (multipart/form-data)
Router->>Auth: authenticate + requireRole
Auth-->>Router: ok
Router->>Multer: upload.single('image')
Multer-->>Router: añade req.file
Router->>Controller: uploadProductImage(req)
Controller->>Service: upsertProductImage(id, file, user)
Service->>Supabase: uploadImage(buffer, bucket, path, mime)
Supabase-->>Service: publicUrl
Service->>Prisma: UPDATE products.image_url = publicUrl
Prisma-->>Service: actualizado
Service->>Supabase: deleteImage(oldPath) (si aplica)
Service-->>Controller: image_url
Controller-->>Client: 201 { image_url }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app.js (1)
77-84:⚠️ Potential issue | 🔴 CriticalMové estos mounts arriba del 404.
Al quedar registrados después del middleware que siempre hace
next(new NotFoundError(...)), los endpoints de imágenes nunca llegan a sus handlers y responden 404 aunque la ruta exista.🐛 Propuesta de ajuste
// Rutas de distancias app.use("/api/distances", distanceRoutes); +app.use('/products', productImageRoutes) +app.use('/users', userImageRoutes) +app.use('/stores', storeImageRoutes) + // Ruta no encontrada — va ANTES del errorHandler app.use((req, _res, next) => { next(new NotFoundError(`Ruta ${req.method} ${req.path} no encontrada`)); }); - -app.use('/products', productImageRoutes) -app.use('/users', userImageRoutes) -app.use('/stores', storeImageRoutes)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app.js` around lines 77 - 84, The routes for image endpoints are mounted after the catch-all 404 middleware so requests never reach their handlers; move the app.use('/products', productImageRoutes), app.use('/users', userImageRoutes) and app.use('/stores', storeImageRoutes) calls to be registered before the NotFoundError middleware (the app.use((req, _res, next) => next(new NotFoundError(...))) block) so productImageRoutes, userImageRoutes and storeImageRoutes are invoked normally.
🧹 Nitpick comments (5)
src/utils/contants/image.constant.js (1)
1-4: Renombrá esta constante o cambiá la unidad.
5 * 1024 * 1024está expresado en bytes, no en MB. Dejarla comoMAX_SIZE_MBhace fácil que más adelante alguien la reutilice como si el valor real fuese5.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/contants/image.constant.js` around lines 1 - 4, IMAGE.MAX_SIZE_MB is actually in bytes (5 * 1024 * 1024) which misleads callers; either rename the key to MAX_SIZE_BYTES or change the value to the numeric MB (5) and keep the MB unit. Update the constant declaration (IMAGE.MAX_SIZE_MB -> IMAGE.MAX_SIZE_BYTES with value 5 * 1024 * 1024, or keep MAX_SIZE_MB and set value to 5) and then adjust all usages of IMAGE.MAX_SIZE_MB throughout the codebase to match the new name/unit (convert checks that compare sizes to use bytes if you renamed, or multiply by 1024*1024 where necessary if you keep MB). Ensure tests and any validations referencing IMAGE.ALLOWED_TYPES or IMAGE.MAX_SIZE_MB/IMAGE.MAX_SIZE_BYTES are updated accordingly.src/modules/images/services/image.service.js (1)
39-43: Conviene normalizar URL antes de extraer elfilePath.Si la URL llega con query params o hash, el path resultante puede quedar contaminado y fallar el delete.
♻️ Propuesta
export function extractFilePath(publicUrl, bucket) { + const cleanUrl = String(publicUrl).split('?')[0].split('#')[0] const marker = `/object/public/${bucket}/` - const idx = publicUrl.indexOf(marker) + const idx = cleanUrl.indexOf(marker) if (idx === -1) return null - return publicUrl.slice(idx + marker.length) + return cleanUrl.slice(idx + marker.length) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/images/services/image.service.js` around lines 39 - 43, La función extractFilePath está devolviendo paths con query string o hash (ej. "file.png?ver=1") que rompen el delete; cambia la extracción para normalizar la URL usando el constructor URL: parsea publicUrl con new URL(publicUrl) (dentro de try/catch para soportar URLs relativas/invalidas), usa url.pathname en lugar de la cadena completa, busca el marker `/object/public/${bucket}/` dentro de pathname y devuelve la porción posterior; si URL constructor falla, como fallback extrae como ahora pero recortando cualquier parte después de '?' o '#' antes de devolver el filePath.src/modules/images/controllers/product-image.controller.js (2)
22-31: Funciones updateProductImage y deleteProductImage correctas.El alias para update y la implementación de delete siguen el patrón consistente establecido.
Nota menor: El archivo no tiene newline al final (EOF). Agregar una línea vacía al final es una convención común para evitar warnings en algunos linters y herramientas de diff.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/images/controllers/product-image.controller.js` around lines 22 - 31, The file is missing a trailing newline at EOF; open src/modules/images/controllers/product-image.controller.js and add a single newline character at the end of the file (after the existing export/ function block that defines updateProductImage and deleteProductImage) so the file ends with a blank line to satisfy linters and diff tools.
12-20: Misma recomendación: usarValidationErrorpara consistencia.Al igual que en
store-image.controller.js, considerar usarValidationErroren lugar del status 400 manual para mantener consistencia en el formato de respuestas de error del proyecto.♻️ Refactor sugerido
+import { ValidationError } from '../../../lib/errors.js' import * as productImageService from '../services/product-image.service.js' export async function uploadProductImage(req, res, next) { try { - if (!req.file) return res.status(400).json({ message: 'No se recibió ningún archivo' }) + if (!req.file) throw new ValidationError('No se recibió ningún archivo') const image_url = await productImageService.upsertProductImage(req.params.id, req.file) return res.status(201).json({ image_url }) } catch (error) { next(error) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/images/controllers/product-image.controller.js` around lines 12 - 20, En uploadProductImage reemplaza la respuesta directa res.status(400)... con el error de validación usado en el proyecto: en la función uploadProductImage comprueba req.file como ahora pero en vez de return res.status(400).json(...) lanza o pasa al middleware next(new ValidationError('No se recibió ningún archivo')) (importa ValidationError desde donde esté definido) para mantener el formato de error consistente con store-image.controller.js; conserva el resto del flujo y el try/catch que hace next(error).src/modules/images/controllers/store-image.controller.js (1)
12-20: Considerar usarValidationErrorpara consistencia en el manejo de errores.En línea 14, se retorna manualmente un status 400 con JSON. El proyecto ya tiene una clase
ValidationErrorensrc/lib/errors.jsque produce HTTP 400 automáticamente a través del middleware de errores. UsarValidationErrormantendría consistencia con el resto del codebase y centralizaría el formato de respuestas de error.♻️ Refactor sugerido
+import { ValidationError } from '../../../lib/errors.js' import * as storeImageService from '../services/store-image.service.js' export async function uploadStoreImage(req, res, next) { try { - if (!req.file) return res.status(400).json({ message: 'No se recibió ningún archivo' }) + if (!req.file) throw new ValidationError('No se recibió ningún archivo') const logo = await storeImageService.upsertStoreImage(req.params.id, req.file) return res.status(201).json({ logo }) } catch (error) { next(error) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/images/controllers/store-image.controller.js` around lines 12 - 20, Replace the manual res.status(400).json response in uploadStoreImage with the project's ValidationError so the error middleware handles it consistently: in uploadStoreImage, when req.file is missing, throw or pass new ValidationError('No se recibió ningún archivo') (import ValidationError from src/lib/errors.js) instead of returning the 400 response; keep the rest of the try/catch and let next(error) handle other errors and the error middleware format the response.
🤖 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/app.js`:
- Around line 82-84: Los routers productImageRoutes, userImageRoutes y
storeImageRoutes se exponen sin autenticación ni validación de ownership; agrega
middleware de autenticación antes de montar esos routers (por ejemplo usando el
mismo auth middleware que pone req.user) y aplica una middleware/validación de
autorización para los métodos que mutan (POST, PUT, DELETE) que compruebe que
req.user.id coincide con el propietario del recurso; además modifica las
funciones del service upsertStoreImage, upsertUserImage y upsertProductImage
para aceptar el userId autenticado y reforzar la verificación de ownership
dentro del service (y devolver 403 en caso de mismatch), y asegúrate de que los
controladores que usan upload.single('image') y validan req.file también pasen
el req.user.id a los services y respeten la autorización antes de intentar
cualquier modificación.
In `@src/config/supabase.config.js`:
- Around line 1-6: Actualmente el cliente de Supabase se crea al importar el
módulo (export const supabase = createClient(...)), lo que falla cuando dotenv
aún no cargó las variables; reemplazá la exportación directa por un getter lazy
llamado getSupabase() que valida process.env.SUPABASE_URL y
process.env.SUPABASE_SERVICE_ROLE_KEY y crea/almacena la instancia la primera
vez que se llama (referenciar la antigua exportación supabase y la función
createClient para localizar el código). Luego actualizá todos los servicios que
importan la instancia (image.service.js, product-image.service.js,
user-image.service.js, store-image.service.js) para importar y usar
getSupabase() en lugar de acceder a la instancia directamente. Asegurate de
lanzar un error claro si las vars de entorno faltan para facilitar debugging.
In `@src/middlewares/upload.middleware.js`:
- Around line 4-14: En el middleware exportado upload (revisar la función
fileFilter dentro de upload en src/middlewares/upload.middleware.js) no lances
un Error genérico para mime inválido; crea/lanza una instancia de
ValidationError con el mensaje de formato y pásala al callback (cb) para que sea
tratada como 4xx; además, en el handler global (función errorHandler en
src/middlewares/errorHandler.js) detecta errores de tipo MulterError y en
particular el código LIMIT_FILE_SIZE y mapea esos casos a una respuesta 400 con
mensaje claro en lugar de caer al 500; conserva el comportamiento existente para
otros errores inesperados.
In `@src/modules/images/controllers/user-image.controller.js`:
- Line 16: The current handler reuses the POST response (res.status(201).json({
avatar_url })) for PUT; create a separate updateUserImage handler (named
updateUserImage) that performs the update logic but returns 200 (with
avatar_url) or 204 (no body) instead of 201, keep the existing
createUserImage/new handler returning 201, and update the routing so PUT calls
updateUserImage; locate the code that references avatar_url and replace the 201
response in the update flow with res.status(200).json({ avatar_url }) or
res.sendStatus(204) as appropriate.
In `@src/modules/images/routes/product-image.routes.js`:
- Around line 13-15: The product image write endpoints (routes registering
uploadProductImage, updateProductImage, deleteProductImage) lack authentication,
so add the existing auth middleware to protect them: insert the auth middleware
(e.g., authenticate, requireAuth, or ensureAdmin — whichever your app uses) into
each route before the file-upload and handler (e.g., router.post('/:id/image',
authenticate, upload.single('image'), uploadProductImage),
router.put('/:id/image', authenticate, upload.single('image'),
updateProductImage), router.delete('/:id/image', authenticate,
deleteProductImage)). Ensure you choose the correct middleware that enforces
user identity/roles (e.g., admin) consistent with other protected routes.
In `@src/modules/images/routes/store-image.routes.js`:
- Around line 13-15: Las rutas mutables de logo (POST/PUT/DELETE) no tienen
protección; agrega middleware de autenticación y autorización antes de los
handlers para evitar que cualquier cliente modifique logos: en las rutas que
usan uploadStoreImage, updateStoreImage y deleteStoreImage, inserta un
middleware como requireAuth y un comprobador de permisos (por ejemplo
ensureStoreOwnerOrAdmin) entre upload.single('image') y los handlers o antes de
upload.single si tu middleware necesita validar al usuario primero; asegúrate
que el comprobador valide que el usuario tiene permiso sobre el :id de la tienda
y devuelve 401/403 según corresponda.
In `@src/modules/images/routes/user-image.routes.js`:
- Around line 13-15: The routes for user images (router.post, router.put,
router.delete) lack authentication and authorization; import and apply the
project's auth middleware (e.g., authenticate/requireAuth) to the mutable routes
(POST/PUT/DELETE) so only authenticated users can call them, and in the
controller functions uploadUserImage, updateUserImage, and deleteUserImage add
an ownership check following the addresses/wishlist pattern: verify req.user.id
(or token user id) matches the target user id (req.params.id) or the image's
ownerId from the service, and return 403 if not authorized; also ensure the
underlying service methods validate ownership before mutating or deleting
resources.
In `@src/modules/images/services/product-image.service.js`:
- Line 5: Agregar validación fail-fast al cargar el módulo para asegurar que la
variable de entorno SUPABASE_BUCKET_PRODUCTS exista: reemplaza la asignación
directa de const BUCKET = process.env.SUPABASE_BUCKET_PRODUCTS por una
comprobación que lance un Error descriptivo si
process.env.SUPABASE_BUCKET_PRODUCTS es falsy, y luego asigna BUCKET al valor
validado; esto garantiza que cualquier uso en product-image.service.js
(referenciado por la constante BUCKET) falle en arranque en vez de en tiempo de
request.
- Around line 22-34: Se está borrando la imagen antigua (extractFilePath +
deleteImage) antes de garantizar que la nueva se suba y se consolide en la BD,
lo que puede dejar image_url apuntando a un recurso inexistente; cambia la
secuencia: primero sube la nueva imagen con uploadImage (usar filePath), guarda
el publicUrl y luego actualiza prisma.products.update para establecer image_url
al publicUrl; solo después de un update exitoso borra la imagen antigua
(deleteImage sobre oldPath); además, si la subida se realizó pero el update
falla, elimina la nueva subida para evitar orfandad y reporta/propaga el error
para manejo superior.
In `@src/modules/images/services/store-image.service.js`:
- Line 5: El módulo define const BUCKET = process.env.SUPABASE_BUCKET_STORES
pero no valida que exista; agrega una comprobación al inicio de
src/modules/images/services/store-image.service.js (antes de usar BUCKET) que
verifique process.env.SUPABASE_BUCKET_STORES y corte temprano si falta: lanza un
Error descriptivo o registra y procesa exit(1) indicando la variable faltante.
Asegúrate de referenciar la constante BUCKET en el mensaje de error para
facilitar diagnóstico y evita continuar la ejecución del servicio si no está
configurada.
- Around line 22-34: La secuencia actual borra primero la imagen vieja y luego
sube y persiste la nueva URL (extractFilePath, deleteImage, uploadImage,
prisma.stores.update), lo que deja el campo logo inconsistente si algo falla;
cambia la lógica para: 1) obtener oldPath con extractFilePath; 2) subir la nueva
imagen con uploadImage para obtener publicUrl; 3) actualizar la DB con
prisma.stores.update usando la nueva publicUrl; 4) si la actualización fue
exitosa, borrar la oldPath con deleteImage; 5) envolver en try/catch y, si falla
la actualización o cualquier paso tras la subida, eliminar la imagen recién
subida (deleteImage con el filePath) y re-lanzar el error para evitar orphans y
mantener consistencia.
In `@src/modules/images/services/user-image.service.js`:
- Line 5: La constante BUCKET se asigna desde process.env.SUPABASE_BUCKET_USERS
sin validación; agrega una comprobación temprana justo después de la declaración
de const BUCKET en user-image.service.js (o al inicio del módulo) que verifique
que BUCKET existe y, si no, lance un Error claro o registre y termine el proceso
(por ejemplo throw new Error / process.exit(1)) para que la aplicación falle al
iniciar en lugar de fallar en tiempo de petición; referencia la constante BUCKET
para localizar el lugar a modificar.
- Around line 22-34: Current flow deletes the old avatar before ensuring the new
file is uploaded and DB updated, risking broken avatar_url if something fails;
change sequence in the avatar update flow (functions/symbols: uploadImage,
extractFilePath, deleteImage, prisma.users.update, user.avatar_url, BUCKET,
filePath) to: first upload the new file (uploadImage) and obtain publicUrl, then
persist publicUrl to the user via prisma.users.update, and only after the DB
update succeeds delete the old file (use extractFilePath + deleteImage) so the
database always points to an existing file; ensure error handling so that if
upload or DB update fails, the old avatar is left intact.
---
Outside diff comments:
In `@src/app.js`:
- Around line 77-84: The routes for image endpoints are mounted after the
catch-all 404 middleware so requests never reach their handlers; move the
app.use('/products', productImageRoutes), app.use('/users', userImageRoutes) and
app.use('/stores', storeImageRoutes) calls to be registered before the
NotFoundError middleware (the app.use((req, _res, next) => next(new
NotFoundError(...))) block) so productImageRoutes, userImageRoutes and
storeImageRoutes are invoked normally.
---
Nitpick comments:
In `@src/modules/images/controllers/product-image.controller.js`:
- Around line 22-31: The file is missing a trailing newline at EOF; open
src/modules/images/controllers/product-image.controller.js and add a single
newline character at the end of the file (after the existing export/ function
block that defines updateProductImage and deleteProductImage) so the file ends
with a blank line to satisfy linters and diff tools.
- Around line 12-20: En uploadProductImage reemplaza la respuesta directa
res.status(400)... con el error de validación usado en el proyecto: en la
función uploadProductImage comprueba req.file como ahora pero en vez de return
res.status(400).json(...) lanza o pasa al middleware next(new
ValidationError('No se recibió ningún archivo')) (importa ValidationError desde
donde esté definido) para mantener el formato de error consistente con
store-image.controller.js; conserva el resto del flujo y el try/catch que hace
next(error).
In `@src/modules/images/controllers/store-image.controller.js`:
- Around line 12-20: Replace the manual res.status(400).json response in
uploadStoreImage with the project's ValidationError so the error middleware
handles it consistently: in uploadStoreImage, when req.file is missing, throw or
pass new ValidationError('No se recibió ningún archivo') (import ValidationError
from src/lib/errors.js) instead of returning the 400 response; keep the rest of
the try/catch and let next(error) handle other errors and the error middleware
format the response.
In `@src/modules/images/services/image.service.js`:
- Around line 39-43: La función extractFilePath está devolviendo paths con query
string o hash (ej. "file.png?ver=1") que rompen el delete; cambia la extracción
para normalizar la URL usando el constructor URL: parsea publicUrl con new
URL(publicUrl) (dentro de try/catch para soportar URLs relativas/invalidas), usa
url.pathname en lugar de la cadena completa, busca el marker
`/object/public/${bucket}/` dentro de pathname y devuelve la porción posterior;
si URL constructor falla, como fallback extrae como ahora pero recortando
cualquier parte después de '?' o '#' antes de devolver el filePath.
In `@src/utils/contants/image.constant.js`:
- Around line 1-4: IMAGE.MAX_SIZE_MB is actually in bytes (5 * 1024 * 1024)
which misleads callers; either rename the key to MAX_SIZE_BYTES or change the
value to the numeric MB (5) and keep the MB unit. Update the constant
declaration (IMAGE.MAX_SIZE_MB -> IMAGE.MAX_SIZE_BYTES with value 5 * 1024 *
1024, or keep MAX_SIZE_MB and set value to 5) and then adjust all usages of
IMAGE.MAX_SIZE_MB throughout the codebase to match the new name/unit (convert
checks that compare sizes to use bytes if you renamed, or multiply by 1024*1024
where necessary if you keep MB). Ensure tests and any validations referencing
IMAGE.ALLOWED_TYPES or IMAGE.MAX_SIZE_MB/IMAGE.MAX_SIZE_BYTES are updated
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e7d31eca-a0eb-4cdf-9ba8-dd71ff4f99f5
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (21)
package.jsonprisma.config.jsprisma/migrations/20260331151903_add_image_fields/migration.sqlprisma/schema.prismasrc/app.jssrc/config/supabase.config.jssrc/middlewares/pagination.middleware.jssrc/middlewares/upload.middleware.jssrc/modules/images/controllers/product-image.controller.jssrc/modules/images/controllers/store-image.controller.jssrc/modules/images/controllers/user-image.controller.jssrc/modules/images/routes/product-image.routes.jssrc/modules/images/routes/store-image.routes.jssrc/modules/images/routes/user-image.routes.jssrc/modules/images/services/image.service.jssrc/modules/images/services/product-image.service.jssrc/modules/images/services/store-image.service.jssrc/modules/images/services/user-image.service.jssrc/utils/.gitkeepsrc/utils/contants/image.constant.jssrc/utils/contants/pagination.contant.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
src/middlewares/errorHandler.js (1)
54-62: Valor de tamaño hardcodeado crea acoplamiento.El mensaje menciona "5 MB" de forma fija, pero el límite real se define en
upload.middleware.jsconIMAGE.MAX_SIZE_MB. Si ese valor cambia, el mensaje quedará desactualizado.♻️ Propuesta para usar una constante compartida
+import { IMAGE } from '../utils/contants/image.constant.js' + // Mapear errores de multer if (err?.code === 'LIMIT_FILE_SIZE') { return res.status(400).json({ error: { code: 400, - message: 'Archivo muy grande. Tamaño máximo: 5 MB.' + message: `Archivo muy grande. Tamaño máximo: ${IMAGE.MAX_SIZE_MB} MB.` } }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/middlewares/errorHandler.js` around lines 54 - 62, The error message for multer's LIMIT_FILE_SIZE is hardcoded to "5 MB"; instead import or require the shared constant (IMAGE.MAX_SIZE_MB) from upload.middleware.js or the central config and use it to build the response message in the error handler block that checks err?.code === 'LIMIT_FILE_SIZE' so the text dynamically reflects the actual limit; update the JSON message construction in that error branch to reference IMAGE.MAX_SIZE_MB (and format it as "X MB") rather than a literal string.src/modules/images/services/product-image.service.js (1)
44-48: El cleanup podría fallar y enmascarar el error original.Si
deleteImageen la línea 46 lanza una excepción, se perdería el error original del update de la BD. Considerar envolver el cleanup en try-catch.♻️ Cleanup más robusto
} catch (error) { // Si el update de BD falla, borramos la imagen recién subida para evitar archivos huérfanos - await deleteImage(BUCKET, filePath) + try { + await deleteImage(BUCKET, filePath) + } catch (cleanupError) { + console.error('Error limpiando imagen huérfana:', cleanupError) + } throw error }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/images/services/product-image.service.js` around lines 44 - 48, En el catch donde se llama a deleteImage(BUCKET, filePath) envuelve esa llamada en su propio try-catch para que cualquier fallo en el cleanup no sobrescriba la excepción original del update de BD; captura el error original (error) antes del cleanup, intenta await deleteImage(...) dentro de un try, si el cleanup falla registra/loguea el error de cleanup (por ejemplo con processLogger.error o console.error) sin lanzar, y finalmente vuelve a lanzar el error original para preservar la causa primaria del fallo.
🤖 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/middlewares/auth.middleware.js`:
- Around line 8-12: El middleware requireRole debe validar que req.user exista
antes de acceder a req.user.role para evitar un TypeError si no se ejecutó el
middleware de autenticación; modifica la función requireRole para primero
comprobar if (!req.user || !req.user.role) y en ese caso invocar next(new
ForbiddenError('No tenés permisos para realizar esta acción')) (o un error más
específico si prefieres), y luego continuar con la comprobación
roles.includes(req.user.role) y llamar next() si pasa la validación.
In `@src/modules/images/controllers/product-image.controller.js`:
- Around line 22-30: The updateProductImage handler returns 204 with a JSON body
which violates RFC 7231; update the function (updateProductImage) so that if you
want to return the new image URL from productImageService.upsertProductImage use
a 200 OK (res.status(200).json({ image_url })) or, if you prefer 204 No Content,
remove the body and send only res.sendStatus(204) (or res.status(204).end())
after upsertProductImage completes; ensure the chosen branch replaces the
current res.status(204).json({ image_url }) line accordingly.
In `@src/modules/images/controllers/store-image.controller.js`:
- Around line 22-30: The updateStoreImage handler currently returns HTTP 204
with a JSON body (res.status(204).json({ logo })), which violates 204 semantics;
change the response to a 200 OK and return the updated logo (e.g., replace
res.status(204).json({ logo }) with res.status(200).json({ logo })) in the
updateStoreImage function so the client receives the logo payload; mirror the
same fix pattern used in user-image.controller.js if present.
In `@src/modules/images/controllers/user-image.controller.js`:
- Around line 22-30: The updateUserImage handler currently responds with status
204 but includes a JSON body (avatar_url), which violates RFC 7231; change the
response to return a 200 status with the avatar_url JSON (i.e., replace
res.status(204).json({ avatar_url }) with a 200 response), or alternatively keep
204 and remove any body—locate this in the updateUserImage function and the call
to userImageService.upsertUserImage to ensure the updated avatar_url is returned
correctly.
In `@src/modules/images/routes/product-image.routes.js`:
- Line 3: The import path for the authenticate module in product-image.routes.js
is wrong; update the import that currently references
'../../../middlewares/authenticate.js' to import the authenticate symbol from
the jwt config module where it actually lives (the module exporting authenticate
in jwt.config.js) so the route uses the correct authenticate export; locate the
import statement for authenticate in product-image.routes.js and change its path
to point to the jwt config module that exports authenticate.
In `@src/modules/images/services/product-image.service.js`:
- Around line 64-70: La secuencia actual borra el archivo de storage antes de
actualizar la BD (usando extractFilePath/product.image_url y deleteImage) lo que
puede dejar image_url apuntando a un recurso inexistente si
prisma.products.update falla; invierte la secuencia: primero ejecuta
prisma.products.update({ where: { id_product: Number(id) }, data: { image_url:
null } }) y tras confirmar éxito, obtén filePath (extractFilePath) y llama a
deleteImage(BUCKET, filePath); además captura y loggea cualquier error de
deleteImage para evitar pérdida silenciosa, o reintenta/loggea si necesitas
consistencia adicional.
In `@src/modules/images/services/store-image.service.js`:
- Around line 58-66: The current removeStoreImage flow deletes the storage file
before updating the DB (deleteImage -> prisma.stores.update), which can leave
the DB pointing to a missing file if the DB update fails; change the sequence in
the removeStoreImage implementation to first perform prisma.stores.update({
where: { id_store: Number(id) }, data: { logo: null } }) (or wrap the update in
a transaction) and only after the update succeeds call
extractFilePath(store.logo) and deleteImage(BUCKET, filePath); ensure you still
validate store.logo existence (throw ValidationError) and add error handling so
a failed deleteImage does not leave DB in an inconsistent state (e.g., log the
delete error and surface it appropriately).
In `@src/modules/images/services/user-image.service.js`:
- Around line 58-66: The current removeUserImage deletes the storage file before
updating the DB, which can leave the DB pointing to a non-existent file if
prisma.users.update fails; change the sequence in removeUserImage to first
update the user record (prisma.users.update) setting avatar_url to null, then
call deleteImage(BUCKET, filePath) using the path from
extractFilePath(user.avatar_url, BUCKET); additionally, handle errors from
deleteImage by logging them and, if needed, attempt to restore the previous
avatar_url via prisma.users.update to avoid leaving the DB inconsistent.
In `@src/utils/contants/roles.js`:
- Around line 1-6: Renombra el directorio typo "contants" a "constants" y
corrige el archivo "pagination.contant.js" a "pagination.constant.js"; luego
actualiza todas las importaciones que apuntan a the old path — por ejemplo en
src/modules/images/services/user-image.service.js, store-image.service.js,
product-image.service.js (línea 4 en cada),
src/modules/images/routes/store-image.routes.js y product-image.routes.js (línea
11 en cada), y en src/middlewares/upload.middleware.js (línea 3) y
src/middlewares/pagination.middleware.js (línea 1) — para usar the new path
"src/utils/constants/..." y el nuevo nombre "pagination.constant.js" en lugar
del antiguo.
---
Nitpick comments:
In `@src/middlewares/errorHandler.js`:
- Around line 54-62: The error message for multer's LIMIT_FILE_SIZE is hardcoded
to "5 MB"; instead import or require the shared constant (IMAGE.MAX_SIZE_MB)
from upload.middleware.js or the central config and use it to build the response
message in the error handler block that checks err?.code === 'LIMIT_FILE_SIZE'
so the text dynamically reflects the actual limit; update the JSON message
construction in that error branch to reference IMAGE.MAX_SIZE_MB (and format it
as "X MB") rather than a literal string.
In `@src/modules/images/services/product-image.service.js`:
- Around line 44-48: En el catch donde se llama a deleteImage(BUCKET, filePath)
envuelve esa llamada en su propio try-catch para que cualquier fallo en el
cleanup no sobrescriba la excepción original del update de BD; captura el error
original (error) antes del cleanup, intenta await deleteImage(...) dentro de un
try, si el cleanup falla registra/loguea el error de cleanup (por ejemplo con
processLogger.error o console.error) sin lanzar, y finalmente vuelve a lanzar el
error original para preservar la causa primaria del fallo.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f35c5f7f-1648-45a8-ab73-a555e647fc07
📒 Files selected for processing (16)
src/app.jssrc/config/env.config.jssrc/config/supabase.config.jssrc/middlewares/auth.middleware.jssrc/middlewares/errorHandler.jssrc/middlewares/upload.middleware.jssrc/modules/images/controllers/product-image.controller.jssrc/modules/images/controllers/store-image.controller.jssrc/modules/images/controllers/user-image.controller.jssrc/modules/images/routes/product-image.routes.jssrc/modules/images/routes/store-image.routes.jssrc/modules/images/routes/user-image.routes.jssrc/modules/images/services/product-image.service.jssrc/modules/images/services/store-image.service.jssrc/modules/images/services/user-image.service.jssrc/utils/contants/roles.js
🚧 Files skipped from review as they are similar to previous changes (5)
- src/app.js
- src/middlewares/upload.middleware.js
- src/config/supabase.config.js
- src/modules/images/routes/user-image.routes.js
- src/modules/images/routes/store-image.routes.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (4)
src/modules/images/routes/product-image.routes.js (1)
3-3:⚠️ Potential issue | 🔴 CriticalRuta de import incorrecta causa fallo del pipeline.
El módulo
authenticateno existe en../../../middlewares/authenticate.js. Según el resumen y la estructura del proyecto, debería importarse desde../../../config/jwt.config.js.🐛 Corrección del import
-import authenticate from '../../../middlewares/authenticate.js' +import authenticate from '../../../config/jwt.config.js'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/images/routes/product-image.routes.js` at line 3, The import for the authenticate middleware in product-image.routes.js is pointing to the wrong module; replace the incorrect import of authenticate from '../../../middlewares/authenticate.js' with the correct source in jwt.config.js (import authenticate from '../../../config/jwt.config.js') so the file imports the exported authenticate function from jwt.config.js and the pipeline no longer fails.src/modules/images/services/user-image.service.js (1)
6-6:⚠️ Potential issue | 🟠 MajorValidá
SUPABASE_BUCKET_USERSal iniciar el módulo.En Line 6
BUCKETpuede quedarundefinedy el fallo recién aparece en tiempo de request. Conviene fallar al boot con error explícito.💡 Propuesta
const BUCKET = process.env.SUPABASE_BUCKET_USERS +if (!BUCKET) { + throw new Error('Falta configurar SUPABASE_BUCKET_USERS') +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/images/services/user-image.service.js` at line 6, La constante BUCKET (process.env.SUPABASE_BUCKET_USERS) puede quedar undefined y debe validarse al iniciar el módulo: at the top of src/modules/images/services/user-image.service.js check process.env.SUPABASE_BUCKET_USERS and if falsy throw a clear, hard failure (throw new Error with a descriptive message) so the app fails fast on boot instead of erroring at request time; ensure the check is run before any functions that use BUCKET (reference the BUCKET symbol and any exported functions/classes in this module) so callers never see an undefined bucket.src/modules/images/services/store-image.service.js (2)
6-6:⚠️ Potential issue | 🟠 MajorValidá
SUPABASE_BUCKET_STORESal cargar el módulo.Hoy el servicio levanta igual con
BUCKETvacío y recién falla en caliente al intentar subir/borrar imágenes, con errores bastante opacos. Conviene cortar temprano si falta esa config.🛡️ Propuesta
const BUCKET = process.env.SUPABASE_BUCKET_STORES +if (!BUCKET) { + throw new Error('Falta configurar SUPABASE_BUCKET_STORES') +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/images/services/store-image.service.js` at line 6, Verificar al cargar el módulo que la constante BUCKET (actualmente definida como const BUCKET = process.env.SUPABASE_BUCKET_STORES) no esté vacía; si falta o está vacía, lanzar un error inmediato (o registrar y terminar el proceso) para fallar rápido en vez de permitir que funciones como las del servicio de subida/borrado de imágenes intenten operar con una configuración inválida; añadí esta validación en la inicialización del archivo donde se define BUCKET y referencia explícita a BUCKET para proporcionar un mensaje de error claro.
62-67:⚠️ Potential issue | 🟠 Major
removeStoreImagepuede devolver error después de haber quedado aplicado.Después de Line 62 a Line 65 el
logoya está ennull. SideleteImagefalla en Line 67, la API responde error aunque el cambio en DB ya quedó comprometido; un retry después cae en “El comercio no tiene logo” y el archivo queda huérfano. Conviene capturar ese cleanup por separado y tratarlo como best-effort/async.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/images/services/store-image.service.js` around lines 62 - 67, The DB update via prisma.stores.update currently runs before calling deleteImage, but if deleteImage fails the service throws even though the DB change is committed; change removeStoreImage so the call to deleteImage(BUCKET, filePath) is executed best-effort and does not bubble up errors—wrap the deleteImage call in its own try/catch (or fire-and-forget Promise) after prisma.stores.update, log any deletion error instead of rethrowing, and ensure the function returns success based on the completed prisma.stores.update operation (use the existing id/filePath symbols to locate 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/images/services/store-image.service.js`:
- Around line 9-15: Los lookups por id_store en store-image.service.js usan solo
id y permiten comercios con borrado lógico; actualiza las llamadas a
prisma.stores.findUnique (y los otros dos flujos mencionados en las líneas 18-21
y 49-52) para filtrar también por el estado del comercio (p. ej. add to the
where clause { id_store: Number(id), status: { not: 'DELETED' } } or status:
'ACTIVE' según el enum en schema.prisma) para que, como en
src/modules/commerce/commerces/store.service.js, los comercios dados de baja
sean tratados como “Comercio no encontrado” (seguido de lanzar NotFoundError
cuando no se encuentra).
- Around line 17-31: En upsertStoreImage valida que el parámetro file existe y
contiene las propiedades necesarias antes de acceder a file.mimetype y
file.buffer; si falta o es inválido lanza un ValidationError con mensaje claro
(por ejemplo "Archivo inválido o ausente") en vez de permitir un TypeError;
añade la comprobación justo antes de usar file (antes de obtener ext y
file.buffer) y asegúrate también de manejar casos donde file.mimetype no
contiene '/' para evitar errores al partir la cadena.
- Around line 33-44: El problema es que si deleteImage(BUCKET, oldPath) falla
después de await prisma.stores.update(...) el catch borrará filePath aunque la
BD ya apunta a publicUrl; fija esto introduciendo una bandera/estado (por
ejemplo dbUpdated) o comprobando si la variable updated fue asignada por
prisma.stores.update: marcar dbUpdated = true justo después de la llamada a
prisma.stores.update (o usar la existencia de updated) y en el catch solo
ejecutar await deleteImage(BUCKET, filePath) si dbUpdated es false (es decir la
actualización en prisma.stores.update no se completó); mantén las llamadas a
deleteImage(BUCKET, oldPath) y el return updated.logo igual.
In `@src/modules/images/services/user-image.service.js`:
- Around line 42-44: El catch actual realiza await deleteImage(BUCKET, filePath)
y luego lanza el error capturado, pero si deleteImage falla se perderá el error
original (por ejemplo de prisma.users.update); cambia el cleanup por best-effort
envolviendo la llamada a deleteImage(BUCKET, filePath) en su propio try/catch
que sólo registre/ignore cualquier fallo de deleteImage y siempre relance el
error original capturado en el catch externo; localiza la sección donde se
maneja el catch alrededor de prisma.users.update y la llamada a deleteImage para
aplicar este cambio.
- Around line 29-31: Validate and whitelist MIME types before deriving the
extension and calling uploadImage: replace the direct use of file.mimetype in
the ext assignment with a check against an allowed MIME map (e.g., {
'image/png':'png', 'image/jpeg':'jpg', 'image/webp':'webp' }), throw or return a
controlled error if file.mimetype is not in the map, then use the mapped
extension to build filePath (the ext, filePath and uploadImage calls) so you
never trust split('/')[1] and only upload supported image types.
---
Duplicate comments:
In `@src/modules/images/routes/product-image.routes.js`:
- Line 3: The import for the authenticate middleware in product-image.routes.js
is pointing to the wrong module; replace the incorrect import of authenticate
from '../../../middlewares/authenticate.js' with the correct source in
jwt.config.js (import authenticate from '../../../config/jwt.config.js') so the
file imports the exported authenticate function from jwt.config.js and the
pipeline no longer fails.
In `@src/modules/images/services/store-image.service.js`:
- Line 6: Verificar al cargar el módulo que la constante BUCKET (actualmente
definida como const BUCKET = process.env.SUPABASE_BUCKET_STORES) no esté vacía;
si falta o está vacía, lanzar un error inmediato (o registrar y terminar el
proceso) para fallar rápido en vez de permitir que funciones como las del
servicio de subida/borrado de imágenes intenten operar con una configuración
inválida; añadí esta validación en la inicialización del archivo donde se define
BUCKET y referencia explícita a BUCKET para proporcionar un mensaje de error
claro.
- Around line 62-67: The DB update via prisma.stores.update currently runs
before calling deleteImage, but if deleteImage fails the service throws even
though the DB change is committed; change removeStoreImage so the call to
deleteImage(BUCKET, filePath) is executed best-effort and does not bubble up
errors—wrap the deleteImage call in its own try/catch (or fire-and-forget
Promise) after prisma.stores.update, log any deletion error instead of
rethrowing, and ensure the function returns success based on the completed
prisma.stores.update operation (use the existing id/filePath symbols to locate
the code).
In `@src/modules/images/services/user-image.service.js`:
- Line 6: La constante BUCKET (process.env.SUPABASE_BUCKET_USERS) puede quedar
undefined y debe validarse al iniciar el módulo: at the top of
src/modules/images/services/user-image.service.js check
process.env.SUPABASE_BUCKET_USERS and if falsy throw a clear, hard failure
(throw new Error with a descriptive message) so the app fails fast on boot
instead of erroring at request time; ensure the check is run before any
functions that use BUCKET (reference the BUCKET symbol and any exported
functions/classes in this module) so callers never see an undefined bucket.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 005f0db5-c938-468a-a2e7-2c351cad4f9e
📒 Files selected for processing (12)
src/middlewares/auth.middleware.jssrc/middlewares/pagination.middleware.jssrc/modules/images/controllers/product-image.controller.jssrc/modules/images/controllers/store-image.controller.jssrc/modules/images/controllers/user-image.controller.jssrc/modules/images/routes/product-image.routes.jssrc/modules/images/routes/store-image.routes.jssrc/modules/images/services/product-image.service.jssrc/modules/images/services/store-image.service.jssrc/modules/images/services/user-image.service.jssrc/utils/contants/pagination.constant.jssrc/utils/contants/roles.constant.js
✅ Files skipped from review due to trivial changes (5)
- src/utils/contants/roles.constant.js
- src/utils/contants/pagination.constant.js
- src/middlewares/pagination.middleware.js
- src/modules/images/controllers/user-image.controller.js
- src/modules/images/controllers/product-image.controller.js
🚧 Files skipped from review as they are similar to previous changes (4)
- src/middlewares/auth.middleware.js
- src/modules/images/routes/store-image.routes.js
- src/modules/images/controllers/store-image.controller.js
- src/modules/images/services/product-image.service.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
src/modules/images/services/product-image.service.js (2)
6-6:⚠️ Potential issue | 🟠 MajorFalta fail-fast para
SUPABASE_BUCKET_PRODUCTS.Si esta variable no existe, el problema aparece recién al atender requests. Mejor validarla al cargar el módulo y fallar temprano.
💡 Ajuste sugerido
const BUCKET = process.env.SUPABASE_BUCKET_PRODUCTS +if (!BUCKET) { + throw new Error('Falta configurar SUPABASE_BUCKET_PRODUCTS') +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/images/services/product-image.service.js` at line 6, The module reads SUPABASE_BUCKET_PRODUCTS into the BUCKET constant without validating it, causing late failures; update product-image.service.js to fail fast by checking BUCKET (or process.env.SUPABASE_BUCKET_PRODUCTS) at module load and throwing a clear error if missing or empty (e.g., inside the top-level initialization where BUCKET is defined or a new init function used by get/upload methods), so any missing env var surfaces immediately rather than at request time.
39-75:⚠️ Potential issue | 🔴 CriticalQuedó el bloque viejo antes del fix y el código corregido hoy es inalcanzable.
El
try/catchde Line 39-Line 52 siempre hacereturnothrow, así que Line 54-Line 75 nunca se ejecuta. En la práctica sigue activo el comportamiento anterior: si falladeleteImage(BUCKET, oldPath)después delupdate, elcatchborrafilePathaunqueimage_urlya quedó persistida.💡 Ajuste sugerido
- try { - const updated = await prisma.products.update({ - where: { id_product: Number(id) }, - data: { image_url: publicUrl } - }) - - if (oldPath && oldPath !== filePath) await deleteImage(BUCKET, oldPath) - - return updated.image_url - } catch (error) { - // Si el update de BD falla, borramos la imagen recién subida para evitar archivos huérfanos - await deleteImage(BUCKET, filePath) - throw error - } - let updated try { updated = await prisma.products.update({ where: { id_product: Number(id) }, data: { image_url: publicUrl } }) } catch (error) { // Solo borramos la nueva imagen si la BD NO se actualizó await deleteImage(BUCKET, filePath).catch(() => { }) throw error }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/images/services/product-image.service.js` around lines 39 - 75, The file contains a leftover, unreachable try/catch block that returns or throws immediately (using prisma.products.update and returning updated.image_url), making the corrected logic below it dead; remove the first try/catch (the block that calls prisma.products.update, conditionally await deleteImage(BUCKET, oldPath) and returns) and keep the later corrected flow: perform prisma.products.update inside a try, on error deleteImage(BUCKET, filePath) and rethrow, then if update succeeded attempt to deleteImage(BUCKET, oldPath) inside its own try/catch (log warnings on cleanup failure) and finally return updated.image_url so deleteImage(BUCKET, filePath) is only called when the DB update fails.src/modules/images/services/user-image.service.js (1)
6-6:⚠️ Potential issue | 🟠 MajorFalta fail-fast para
SUPABASE_BUCKET_USERS.Si esta env no está definida, el módulo queda “aparentemente” sano y recién falla cuando entra una request. Mejor validarla al inicio.
💡 Ajuste sugerido
const BUCKET = process.env.SUPABASE_BUCKET_USERS +if (!BUCKET) { + throw new Error('Falta configurar SUPABASE_BUCKET_USERS') +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/images/services/user-image.service.js` at line 6, El módulo define const BUCKET = process.env.SUPABASE_BUCKET_USERS but does not validate it; add a fail-fast check at module initialization (e.g., right after the BUCKET assignment in user-image.service.js) that throws a clear Error if SUPABASE_BUCKET_USERS is undefined or empty so the service fails on startup instead of at request time; reference the BUCKET constant and ensure any exported functions that rely on BUCKET (in this module) can assume it's present after the check.src/modules/images/services/store-image.service.js (1)
6-6:⚠️ Potential issue | 🟠 MajorFalta fail-fast para
SUPABASE_BUCKET_STORES.Si esta env queda vacía, el servicio recién va a fallar en request-time cuando intente subir, borrar o parsear imágenes. Conviene cortar al cargar el módulo.
💡 Ajuste sugerido
const BUCKET = process.env.SUPABASE_BUCKET_STORES +if (!BUCKET) { + throw new Error('Falta configurar SUPABASE_BUCKET_STORES') +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/images/services/store-image.service.js` at line 6, El módulo debe fallar rápido si falta SUPABASE_BUCKET_STORES: en la inicialización del archivo que define const BUCKET (identificador BUCKET en store-image.service.js) comprueba que process.env.SUPABASE_BUCKET_STORES esté presente y, si no, lanza inmediatamente un Error (o hace process.exit(1)) para evitar errores en tiempo de petición al usar las funciones de subida/borrado/parseo de imágenes; aplica el check junto al lugar donde se define BUCKET para que el módulo no se cargue en un estado inválido.
🤖 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/lib/prisma.js`:
- Around line 2-16: The current file only exports the Prisma configuration via
export default defineConfig(...) but consumers expect a runtime Prisma client;
add an import of PrismaClient from '@prisma/client', instantiate it with const
prisma = new PrismaClient(), and export it as a named export (export { prisma })
so other modules can do import { prisma } from '.../lib/prisma.js'; keep or
adjust the existing defineConfig export (defineConfig) but ensure you do not
remove or conflict with the new named export prisma.
In `@src/middlewares/errorHandler.js`:
- Around line 55-63: Change the response for multer file size errors in the
error handler so that when err?.code === 'LIMIT_FILE_SIZE' you return HTTP 413
instead of 400; update the status and the error.code field to 413 in the JSON
payload (the block that references IMAGE.MAX_SIZE and the condition err?.code
=== 'LIMIT_FILE_SIZE' in errorHandler.js) so clients receive a proper "Payload
Too Large" signal.
In `@src/modules/images/services/product-image.service.js`:
- Line 2: El import en product-image.service.js está apuntando a
'../../../lib/image.service.js' que no existe; actualizar la declaración de
import para referenciar el archivo local al mismo directorio (usar
'./image.service.js') para que las funciones importadas uploadImage, deleteImage
y extractFilePath sean resueltas correctamente por Vitest; localiza la línea con
import { uploadImage, deleteImage, extractFilePath } en product-image.service.js
y corrige la ruta al módulo relativo correcto.
In `@src/modules/images/services/store-image.service.js`:
- Line 2: The import path in store-image.service.js is incorrect; update the
import statement to reference the local sibling module by changing
"../../../lib/image.service.js" to "./image.service.js" while keeping the
imported symbols uploadImage, deleteImage, and extractFilePath so functions used
in storeImage (or any functions inside this module) resolve to
src/modules/images/services/image.service.js.
---
Duplicate comments:
In `@src/modules/images/services/product-image.service.js`:
- Line 6: The module reads SUPABASE_BUCKET_PRODUCTS into the BUCKET constant
without validating it, causing late failures; update product-image.service.js to
fail fast by checking BUCKET (or process.env.SUPABASE_BUCKET_PRODUCTS) at module
load and throwing a clear error if missing or empty (e.g., inside the top-level
initialization where BUCKET is defined or a new init function used by get/upload
methods), so any missing env var surfaces immediately rather than at request
time.
- Around line 39-75: The file contains a leftover, unreachable try/catch block
that returns or throws immediately (using prisma.products.update and returning
updated.image_url), making the corrected logic below it dead; remove the first
try/catch (the block that calls prisma.products.update, conditionally await
deleteImage(BUCKET, oldPath) and returns) and keep the later corrected flow:
perform prisma.products.update inside a try, on error deleteImage(BUCKET,
filePath) and rethrow, then if update succeeded attempt to deleteImage(BUCKET,
oldPath) inside its own try/catch (log warnings on cleanup failure) and finally
return updated.image_url so deleteImage(BUCKET, filePath) is only called when
the DB update fails.
In `@src/modules/images/services/store-image.service.js`:
- Line 6: El módulo debe fallar rápido si falta SUPABASE_BUCKET_STORES: en la
inicialización del archivo que define const BUCKET (identificador BUCKET en
store-image.service.js) comprueba que process.env.SUPABASE_BUCKET_STORES esté
presente y, si no, lanza inmediatamente un Error (o hace process.exit(1)) para
evitar errores en tiempo de petición al usar las funciones de
subida/borrado/parseo de imágenes; aplica el check junto al lugar donde se
define BUCKET para que el módulo no se cargue en un estado inválido.
In `@src/modules/images/services/user-image.service.js`:
- Line 6: El módulo define const BUCKET = process.env.SUPABASE_BUCKET_USERS but
does not validate it; add a fail-fast check at module initialization (e.g.,
right after the BUCKET assignment in user-image.service.js) that throws a clear
Error if SUPABASE_BUCKET_USERS is undefined or empty so the service fails on
startup instead of at request time; reference the BUCKET constant and ensure any
exported functions that rely on BUCKET (in this module) can assume it's present
after the check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 90322f40-58a7-45da-a96e-272b96e2f29e
📒 Files selected for processing (6)
src/lib/prisma.jssrc/middlewares/errorHandler.jssrc/modules/images/services/product-image.service.jssrc/modules/images/services/store-image.service.jssrc/modules/images/services/user-image.service.jssrc/utils/contants/image.constant.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/utils/contants/image.constant.js
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
src/modules/images/services/user-image.service.js (3)
33-35:⚠️ Potential issue | 🟠 MajorRestringí MIME types con whitelist antes de armar
filePath.En Line 33 se deriva extensión desde
file.mimetypesin lista permitida; eso deja pasar formatos no esperados.💡 Propuesta
- const ext = file.mimetype.split('/')[1] - const filePath = `${id}/avatar-${Date.now()}.${ext}` + const allowedMime = new Map([ + ['image/jpeg', 'jpg'], + ['image/png', 'png'], + ['image/webp', 'webp'] + ]) + const ext = allowedMime.get(file.mimetype) + if (!ext) throw new ValidationError('Formato de imagen no soportado') + const filePath = `${Number(id)}/avatar-${Date.now()}.${ext}`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/images/services/user-image.service.js` around lines 33 - 35, Valida el MIME type contra una whitelist antes de construir filePath: en user-image.service.js, antes de usar file.mimetype para derivar la extensión y llamar a uploadImage, comprueba que file.mimetype exista en una constante ALLOWED_MIME_TYPES o en un mapa MIME_EXTENSIONS (por ejemplo { "image/png":"png", "image/jpeg":"jpg" }) y, si no está permitido, lanza/retorna un error; usa el valor mapeado para formar filePath (en lugar de file.mimetype.split('/')[1]) y así proteger uploadImage/BUCKET de tipos no esperados.
6-6:⚠️ Potential issue | 🟠 MajorValidá
SUPABASE_BUCKET_USERSal cargar el módulo.Si
BUCKETviene vacío, el servicio falla recién en runtime durante requests en vez de fallar al iniciar.💡 Propuesta
const BUCKET = process.env.SUPABASE_BUCKET_USERS +if (!BUCKET) { + throw new Error('Falta configurar SUPABASE_BUCKET_USERS') +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/images/services/user-image.service.js` at line 6, El módulo define BUCKET from process.env.SUPABASE_BUCKET_USERS but no valida su existencia al cargar, por lo que los fallos aparecen en runtime; al inicializar el módulo valida que process.env.SUPABASE_BUCKET_USERS esté presente y no vacío (antes de asignar BUCKET) y, si falta, lanza un Error con un mensaje claro que incluya "SUPABASE_BUCKET_USERS" para detener el arranque; actualiza la declaración/creación de BUCKET en user-image.service.js para realizar esta comprobación y lanzar la excepción durante la carga del módulo.
75-80:⚠️ Potential issue | 🟠 MajorHacé best-effort en el cleanup de storage al remover avatar.
Si
deleteImagefalla en Line 80, la DB ya quedó enavatar_url: nullpero la API responde error; eso genera estado aplicado con respuesta de falla.💡 Propuesta
await prisma.users.update({ where: { id_user: Number(id) }, data: { avatar_url: null } }) - if (filePath) await deleteImage(BUCKET, filePath) + if (filePath) { + try { + await deleteImage(BUCKET, filePath) + } catch (cleanupError) { + console.warn(`[WARN] No se pudo eliminar avatar en storage: ${filePath}`, cleanupError) + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/images/services/user-image.service.js` around lines 75 - 80, La operación hace update de prisma.users (prisma.users.update) y luego llama a deleteImage(BUCKET, filePath), lo que deja la DB en avatar_url:null si la eliminación del storage falla; cambia el flujo para hacer un "best-effort" cleanup: primero guarda el valor previo del avatar (p. ej. prevAvatar), luego intenta eliminar el archivo con deleteImage(BUCKET, filePath) dentro de un try/catch; si la eliminación falla, en el catch intenta revertir la DB restaurando avatar_url a prevAvatar (usando prisma.users.update) y lanza/retorna el error; alternativamente, para evitar inconsistencias puedes eliminar primero el archivo y sólo si tiene éxito hacer prisma.users.update para setear avatar_url:null — referencia los símbolos prisma.users.update, deleteImage, BUCKET y filePath para localizar el cambio.src/modules/images/services/store-image.service.js (1)
9-11:⚠️ Potential issue | 🟠 MajorFalta filtrar comercios con borrado lógico en los tres lookups.
En Line [9], Line [22] y Line [62] se consulta solo por
id_store; eso permite leer/modificar logo de comercios dados de baja.♻️ Propuesta de ajuste
- const store = await prisma.stores.findUnique({ - where: { id_store: Number(id) }, + const store = await prisma.stores.findFirst({ + where: { + id_store: Number(id), + status: { not: 'DELETED' } // o status: 'ACTIVE', según enum real + }, select: { logo: true } })- const store = await prisma.stores.findUnique({ - where: { id_store: Number(id) } + const store = await prisma.stores.findFirst({ + where: { + id_store: Number(id), + status: { not: 'DELETED' } // o ACTIVE + } })Also applies to: 22-24, 62-64
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/images/services/store-image.service.js` around lines 9 - 11, The three prisma.stores.findUnique lookups (the calls to prisma.stores.findUnique near the logo read/update/remove logic) need to exclude logically deleted stores; update each where clause to include the logical-delete predicate used by our schema (e.g., add deleted_at: null or is_deleted: false depending on the model) so the combined where becomes { id_store: Number(id), <logical-delete-field>: <active-value> } and apply the same change consistently for all three occurrences.src/modules/images/services/product-image.service.js (2)
93-98:⚠️ Potential issue | 🟠 MajorNo hagas fallar el endpoint si sólo falla el cleanup de Storage.
Desde Line 93 la BD ya quedó con
image_url: null. Si Line 98 explota, la API responde error aunque el cambio persistió, y un reintento después va a encontrar el producto sin imagen. Tratá ese borrado como cleanup con log o retry.🛠️ Cambio sugerido
- if (filePath) await deleteImage(BUCKET, filePath) + if (filePath) { + try { + await deleteImage(BUCKET, filePath) + } catch (cleanupError) { + console.warn(`[WARN] No se pudo eliminar imagen del storage: ${filePath}`, cleanupError) + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/images/services/product-image.service.js` around lines 93 - 98, Después de actualizar la DB con prisma.products.update (estableciendo image_url: null), no dejes que una excepción en deleteImage(BUCKET, filePath) haga fallar el endpoint; trata ese borrado como cleanup: envolver la llamada a deleteImage en un try/catch (o aplicar un pequeño retry) y en el catch registrar el error (incluyendo BUCKET y filePath) sin volver a lanzar, de modo que la respuesta al cliente refleje el éxito del update aunque el cleanup de Storage falle.
6-6:⚠️ Potential issue | 🟠 MajorValidá
SUPABASE_BUCKET_PRODUCTSal cargar el módulo.Si
BUCKETquedaundefined, el servicio igual levanta y el error aparece recién cuando pega contra Storage. Mejor fallar en startup con un mensaje explícito.🛠️ Cambio sugerido
-const BUCKET = process.env.SUPABASE_BUCKET_PRODUCTS +const BUCKET = process.env.SUPABASE_BUCKET_PRODUCTS +if (!BUCKET) { + throw new Error('Falta configurar SUPABASE_BUCKET_PRODUCTS') +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/images/services/product-image.service.js` at line 6, Al cargar el módulo, valida que process.env.SUPABASE_BUCKET_PRODUCTS exista y asigna a la constante BUCKET; si es undefined lanza un Error con un mensaje claro (p. ej. "SUPABASE_BUCKET_PRODUCTS is not set") para detener el arranque en vez de dejar que falle en tiempo de ejecución; modifica el archivo donde se declara la constante BUCKET y deja la validación junto a esa declaración para que cualquier función que use BUCKET en este módulo (referencia: BUCKET) no se ejecute si falta la variable.
🤖 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/images/services/product-image.service.js`:
- Around line 39-52: The catch block in product-image.service.js currently
deletes the newly uploaded image for any error, which also runs if
deleteImage(BUCKET, oldPath) fails after a successful prisma.products.update;
change the flow so prisma.products.update(...) is awaited first, then—only after
a successful update—attempt to delete oldPath with deleteImage(BUCKET, oldPath)
and handle its errors separately (log or return a non-destructive error) so the
DB never points to a removed file; in the catch for the update step only,
perform the rollback of the new upload by calling deleteImage(BUCKET, filePath)
and rethrow the update error; update or remove the unreachable block (lines
54-75) accordingly and keep references to prisma.products.update, deleteImage,
oldPath, filePath and BUCKET when making the changes.
In `@src/modules/images/services/store-image.service.js`:
- Line 80: No hagas que un fallo en el cleanup post-commit reviente la
operación: envuelve la llamada a deleteImage(BUCKET, filePath) en un bloque
try/catch (cuando filePath exista), atrapa el error y registra la
advertencia/error con el logger existente en lugar de volver a lanzar; así el
campo logo ya persistido (p. ej. en la función que realiza el commit en
store-image.service.js) no provocará reintentos inconsistentes y el borrado se
intenta en modo best-effort.
- Around line 8-10: Valida el parámetro id antes de convertirlo a Number en las
funciones que usan Number(id) (p. ej. getStoreImage y las demás funciones
exportadas en este archivo que convierten req.params.id) para evitar NaN y un
500; comprobar que id es una cadena numérica o un entero válido (por ejemplo con
/^\d+$/.test(id) o parseInt + Number.isInteger) y si no es válido
lanzar/retornar un ValidationError controlado con mensaje claro (por ejemplo
"Invalid store id") en vez de continuar con Number(id).
---
Duplicate comments:
In `@src/modules/images/services/product-image.service.js`:
- Around line 93-98: Después de actualizar la DB con prisma.products.update
(estableciendo image_url: null), no dejes que una excepción en
deleteImage(BUCKET, filePath) haga fallar el endpoint; trata ese borrado como
cleanup: envolver la llamada a deleteImage en un try/catch (o aplicar un pequeño
retry) y en el catch registrar el error (incluyendo BUCKET y filePath) sin
volver a lanzar, de modo que la respuesta al cliente refleje el éxito del update
aunque el cleanup de Storage falle.
- Line 6: Al cargar el módulo, valida que process.env.SUPABASE_BUCKET_PRODUCTS
exista y asigna a la constante BUCKET; si es undefined lanza un Error con un
mensaje claro (p. ej. "SUPABASE_BUCKET_PRODUCTS is not set") para detener el
arranque en vez de dejar que falle en tiempo de ejecución; modifica el archivo
donde se declara la constante BUCKET y deja la validación junto a esa
declaración para que cualquier función que use BUCKET en este módulo
(referencia: BUCKET) no se ejecute si falta la variable.
In `@src/modules/images/services/store-image.service.js`:
- Around line 9-11: The three prisma.stores.findUnique lookups (the calls to
prisma.stores.findUnique near the logo read/update/remove logic) need to exclude
logically deleted stores; update each where clause to include the logical-delete
predicate used by our schema (e.g., add deleted_at: null or is_deleted: false
depending on the model) so the combined where becomes { id_store: Number(id),
<logical-delete-field>: <active-value> } and apply the same change consistently
for all three occurrences.
In `@src/modules/images/services/user-image.service.js`:
- Around line 33-35: Valida el MIME type contra una whitelist antes de construir
filePath: en user-image.service.js, antes de usar file.mimetype para derivar la
extensión y llamar a uploadImage, comprueba que file.mimetype exista en una
constante ALLOWED_MIME_TYPES o en un mapa MIME_EXTENSIONS (por ejemplo {
"image/png":"png", "image/jpeg":"jpg" }) y, si no está permitido, lanza/retorna
un error; usa el valor mapeado para formar filePath (en lugar de
file.mimetype.split('/')[1]) y así proteger uploadImage/BUCKET de tipos no
esperados.
- Line 6: El módulo define BUCKET from process.env.SUPABASE_BUCKET_USERS but no
valida su existencia al cargar, por lo que los fallos aparecen en runtime; al
inicializar el módulo valida que process.env.SUPABASE_BUCKET_USERS esté presente
y no vacío (antes de asignar BUCKET) y, si falta, lanza un Error con un mensaje
claro que incluya "SUPABASE_BUCKET_USERS" para detener el arranque; actualiza la
declaración/creación de BUCKET en user-image.service.js para realizar esta
comprobación y lanzar la excepción durante la carga del módulo.
- Around line 75-80: La operación hace update de prisma.users
(prisma.users.update) y luego llama a deleteImage(BUCKET, filePath), lo que deja
la DB en avatar_url:null si la eliminación del storage falla; cambia el flujo
para hacer un "best-effort" cleanup: primero guarda el valor previo del avatar
(p. ej. prevAvatar), luego intenta eliminar el archivo con deleteImage(BUCKET,
filePath) dentro de un try/catch; si la eliminación falla, en el catch intenta
revertir la DB restaurando avatar_url a prevAvatar (usando prisma.users.update)
y lanza/retorna el error; alternativamente, para evitar inconsistencias puedes
eliminar primero el archivo y sólo si tiene éxito hacer prisma.users.update para
setear avatar_url:null — referencia los símbolos prisma.users.update,
deleteImage, BUCKET y filePath para localizar el cambio.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 211113b6-bdb5-4b1b-bb49-d0114e3c080d
📒 Files selected for processing (7)
src/lib/prisma.jssrc/middlewares/errorHandler.jssrc/modules/images/services/image.service.jssrc/modules/images/services/product-image.service.jssrc/modules/images/services/store-image.service.jssrc/modules/images/services/user-image.service.jssrc/utils/contants/image.constant.js
✅ Files skipped from review due to trivial changes (1)
- src/utils/contants/image.constant.js
🚧 Files skipped from review as they are similar to previous changes (2)
- src/modules/images/services/image.service.js
- src/lib/prisma.js
leoAchu16
approved these changes
Apr 3, 2026
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.
Crear los endpoint para manejo de imagenes en product, store y user. Falta agregar a los demas endponits.
Summary by CodeRabbit
Nuevas Características
Seguridad y Acceso
Validaciones y Errores
Base de Datos
Otros