Feature/migration imgs supabase #178
Conversation
WalkthroughRemoves S3/Backblaze/Multer deps and legacy Backblaze/repository code. Introduces a Supabase-based photo module with domain entity, interfaces, exceptions, DTOs, controller, and routes. Updates validation middleware to plainToInstance. Minor log formatting and an import/typing tweak in organization controller. Adds photo module README. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Router as Express Router (/photo)
participant Controller as PhotoController
participant Service as SupabasePhotoService
participant Storage as Supabase Storage
participant DB as Prisma (photos)
rect rgb(245,248,250)
note over Client,DB: Upload flow
Client->>Router: POST /upload (file, userId)
Router->>Controller: uploadPhoto(req,res)
Controller->>Service: upload(file, userId, metadata?)
Service->>Storage: upload(fileName, buffer, contentType)
Storage-->>Service: public URL or error
Service->>DB: create photo {userId,url,metadata,uploadedAt}
DB-->>Service: created record
Service-->>Controller: PhotoEntity
Controller-->>Client: 201 Created (photo)
end
rect rgb(250,247,245)
note over Client,DB: Get / Delete flow
Client->>Router: GET /:id or DELETE /:id
Router->>Controller: getPhotoById / deletePhoto
Controller->>Service: getById(id)
Service->>DB: find photo by id
DB-->>Service: record or null
alt found
Controller-->>Client: 200 OK (get) / proceed (delete)
Controller->>Service: delete(id) [DELETE only]
Service->>DB: find & delete record
Service->>Storage: remove(fileName)
Storage-->>Service: ack
Service-->>Controller: done
Controller-->>Client: 204 No Content [DELETE]
else not found
Controller-->>Client: 404 Not Found
end
end
rect rgb(245,250,247)
note over Client,DB: Update metadata flow
Client->>Router: PATCH /:id/metadata
Router->>Controller: updatePhotoMetadata
Controller->>Service: updateMetadata(id, metadata)
Service->>DB: update metadata
DB-->>Service: updated metadata
Service-->>Controller: updated metadata
Controller-->>Client: 200 OK (metadata)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/index.ts (1)
168-169: Bug: messages router mounted on the NFT router (wrong path)This nests messages under /nft/messages instead of /messages. Mount on app.
Apply:
-router.use("/messages", messageRoutes); +app.use("/messages", messageRoutes);src/modules/photo/domain/entities/photo.entity.ts (1)
41-48: create() ignores id and uploadedAt; returned entity lacks persistence identity.This breaks downstream flows (e.g., delete by id) and misreports uploadedAt.
Apply:
public static create(props: IPhotoProps): PhotoEntity { const photo = new PhotoEntity(); + if (props.id) photo.id = props.id; photo.url = props.url; photo.userId = props.userId; photo.metadata = props.metadata ?? {}; + if (props.uploadedAt) { + // BaseEntity should expose createdAt; adjust if different. + (photo as any).createdAt = props.uploadedAt; + } photo.validate(); return photo; }If BaseEntity exposes a different timestamp field, map accordingly.
🧹 Nitpick comments (11)
src/index.ts (1)
75-84: Error handler is registered before routesExpress error middleware should come after routes to reliably catch downstream errors.
Move the error-handler block to after all app.use(...) route registrations (right before DB init). Example:
-// Error handler middleware -app.use((err: Error, req: express.Request, res: express.Response, next: express.NextFunction) => { - errorHandler(err, req, res, next); -}); ... -// API Routes... +// API Routes... app.use("/test", testRoutes); + +// Error handler middleware (must be last in the middleware chain) +app.use((err: Error, req: express.Request, res: express.Response, next: express.NextFunction) => { + errorHandler(err, req, res, next); +});src/modules/organization/presentation/controllers/organization.controller.ts (1)
40-41: Preserve param typing for stronger guaranteesReverting to Request keeps compile-time safety and aligns with DTO validation.
Apply:
- async (req: Request, res: Response): Promise<void> => { + async (req: Request<UuidParamsDto>, res: Response): Promise<void> => {src/shared/middleware/validation.middleware.ts (1)
120-121: Assigning typed params back to req.paramsCasting to Record<string, string> hides actual numeric types after conversion. If you rely on numbers (e.g., Photo DTOs), keep runtime types.
Apply one of:
- Assign without lying about types:
- req.params = dto as Record<string, string>; + // Preserve runtime types; Express typing stays opaque here + req.params = dto as any;
- Or attach validated data separately to avoid mutating Express fields:
// declare a Request augmentation (e.g., req.validated = { params: dto })src/modules/photo/README.md (2)
8-13: Clarify and fix grammar in “Key Architectural Differences.”The paragraph is hard to parse and has grammar issues. Suggested rewrite:
-No use cases or repositories are used, only maintained Entity to get easy access on this information, but this Entity is generate for mi adapter, adapter interface required a return of PhotoEntity. +We do not use traditional use-cases or repositories here. We keep a single Entity for simple access to photo data. The storage adapter’s interface returns a PhotoEntity instance, and the concrete adapter constructs it.
26-36: Fix path typos in structure block.
domain/entitites→domain/entities- Consider tightening comments:
- domain/entities/interfaces/ #interfaces of entity - domain/entitites/photo.entity.ts #Entity + domain/entities/interfaces/ # entity interfaces + domain/entities/photo.entity.ts # PhotoEntitysrc/modules/photo/presentation/routes.ts (2)
13-19: Optional: validate before upload?For multipart, body fields arrive with the file. If you want to reject early without buffering the file, you’ll need a light multipart field parser. Otherwise, current order (multer → validate) is acceptable.
8-12: Service/controller contract drift to watch.Ensure
SupabasePhotoService.uploadreturns a Photo with correctid(DB id, not URL) and that controller/clients rely on that. The current adapter snippet setsid = photo.url— that will break DELETE/GET by id.I can open a follow-up PR adjusting the adapter return mapping.
src/modules/photo/infrastructure/adapters/interface/photo-service.adapter.ts (1)
4-16: Decouple from Express/Multer in the adapter interface.Keeping Express.Multer.File here leaks transport concerns into infra contracts. Consider a minimal input type (buffer, mimeType, originalName, size) instead.
Example:
-export interface IPhotoServiceAdapter { - upload( - file: Express.Multer.File, +type UploadInput = { buffer: Buffer; mimeType: string; originalName: string; size: number }; +export interface IPhotoServiceAdapter { + upload( + file: UploadInput, userId: string, metadata?: IPhotoMetadata ): Promise<PhotoEntity>;src/modules/photo/presentation/controllers/photo-controller.ts (2)
20-29: Parse optional metadata and pass it to the service.Currently metadata in body (if any) is ignored.
Apply:
- const { userId } = req.body; + const { userId, metadata } = req.body ?? {}; ... - const photo = await this.supabasePhotoService.upload(req.file, userId); + const photo = await this.photoService.upload(req.file, userId, metadata);
32-47: Authorization check is missing.Ensure the requester owns the photo (or has proper scope) before returning or mutating resources.
Suggested approach:
- Inject current userId from auth middleware.
- Compare with photo.userId (extend getById to return it) or check via DB.
- Return 403 when unauthorized.
src/modules/photo/infrastructure/adapters/supabase-service.adapter.ts (1)
1-6: Consistent imports and path aliases.Mixing "@/..." and relative "../../..." hurts maintainability. Standardize on one (prefer aliases).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (22)
package.json(0 hunks)src/index.ts(1 hunks)src/modules/organization/presentation/controllers/organization.controller.ts(2 hunks)src/modules/photo/README.md(1 hunks)src/modules/photo/domain/entities/interfaces/photo.interface.ts(1 hunks)src/modules/photo/domain/entities/photo.entity.ts(2 hunks)src/modules/photo/domain/exceptions/domain.exception.ts(1 hunks)src/modules/photo/entities/photo.entity.ts(0 hunks)src/modules/photo/infrastructure/adapters/interface/photo-service.adapter.ts(1 hunks)src/modules/photo/infrastructure/adapters/supabase-service.adapter.ts(1 hunks)src/modules/photo/infrastructure/services/BackblazeService.ts(0 hunks)src/modules/photo/interfaces/photo.interface.ts(0 hunks)src/modules/photo/presentation/controllers/PhotoController.ts(0 hunks)src/modules/photo/presentation/controllers/photo-controller.ts(1 hunks)src/modules/photo/presentation/dto/delete-photo.dto.ts(1 hunks)src/modules/photo/presentation/dto/get-photo.dto.ts(1 hunks)src/modules/photo/presentation/dto/index.ts(1 hunks)src/modules/photo/presentation/dto/upload-photo.dto.ts(1 hunks)src/modules/photo/presentation/routes.ts(1 hunks)src/repository/IPhotoRepository.ts(0 hunks)src/repository/PhotoRepository.ts(0 hunks)src/shared/middleware/validation.middleware.ts(5 hunks)
💤 Files with no reviewable changes (7)
- src/repository/PhotoRepository.ts
- src/modules/photo/infrastructure/services/BackblazeService.ts
- package.json
- src/modules/photo/interfaces/photo.interface.ts
- src/repository/IPhotoRepository.ts
- src/modules/photo/entities/photo.entity.ts
- src/modules/photo/presentation/controllers/PhotoController.ts
🧰 Additional context used
🧬 Code graph analysis (8)
src/modules/photo/infrastructure/adapters/interface/photo-service.adapter.ts (2)
src/modules/photo/domain/entities/interfaces/photo.interface.ts (1)
IPhotoMetadata(6-30)src/modules/photo/domain/entities/photo.entity.ts (1)
PhotoEntity(8-60)
src/modules/photo/presentation/dto/get-photo.dto.ts (1)
src/modules/photo/presentation/dto/index.ts (1)
GetPhotoDto(2-2)
src/modules/photo/presentation/dto/delete-photo.dto.ts (1)
src/modules/photo/presentation/dto/index.ts (1)
DeletePhotoDto(1-1)
src/modules/photo/presentation/routes.ts (6)
src/modules/photo/infrastructure/adapters/supabase-service.adapter.ts (2)
upload(9-55)SupabasePhotoService(7-105)src/modules/photo/presentation/controllers/photo-controller.ts (1)
PhotoController(10-106)src/shared/middleware/validation.middleware.ts (1)
validateDto(15-51)src/modules/photo/presentation/dto/upload-photo.dto.ts (1)
UploadPhotoDto(3-6)src/modules/photo/presentation/dto/delete-photo.dto.ts (1)
DeletePhotoDto(3-6)src/modules/photo/presentation/dto/get-photo.dto.ts (1)
GetPhotoDto(3-6)
src/modules/photo/infrastructure/adapters/supabase-service.adapter.ts (4)
src/modules/photo/infrastructure/adapters/interface/photo-service.adapter.ts (1)
IPhotoServiceAdapter(4-16)src/modules/photo/domain/entities/interfaces/photo.interface.ts (1)
IPhotoMetadata(6-30)src/modules/photo/domain/entities/photo.entity.ts (1)
PhotoEntity(8-60)src/config/supabase.ts (1)
supabase(6-6)
src/modules/photo/presentation/dto/upload-photo.dto.ts (1)
src/modules/photo/presentation/dto/index.ts (1)
UploadPhotoDto(3-3)
src/modules/photo/presentation/controllers/photo-controller.ts (1)
src/modules/photo/infrastructure/adapters/supabase-service.adapter.ts (1)
SupabasePhotoService(7-105)
src/modules/photo/domain/entities/photo.entity.ts (2)
src/modules/photo/domain/entities/interfaces/photo.interface.ts (2)
IPhotoMetadata(6-30)IPhotoProps(32-38)src/modules/photo/domain/exceptions/domain.exception.ts (2)
InvalidPhotoUrlException(3-7)MissingUserIdException(9-13)
🪛 LanguageTool
src/modules/photo/README.md
[grammar] ~1-~1: There might be a mistake here.
Context: # 📸 Photos Module This module handles uploading, fetching,...
(QB_NEW_EN)
[grammar] ~6-~6: There might be a mistake here.
Context: ...Ls). # 🔑 Key Architectural Differences No use cases or repositories are used, o...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ...erface required a return of PhotoEntity. The flow is more direct: Controller → c...
(QB_NEW_EN)
[grammar] ~21-~21: There might be a mistake here.
Context: ...pabase Storage. Simplified Controllers: Controllers use asyncHandler, validators...
(QB_NEW_EN)
[grammar] ~24-~24: There might be a mistake here.
Context: ...ter for file processing. # 📂 Structure text modules/ photo/ domain/entities/interfaces/ #interfaces of entity domain/entitites/photo.entity.ts #Entity infrastructure/ presentation/controllers/ presentation/dto routes.ts README.md
(QB_NEW_EN)
🔇 Additional comments (10)
src/index.ts (1)
189-192: Startup log reformat: LGTMSingle-line log with structured metadata keeps parity and improves readability.
src/modules/organization/presentation/controllers/organization.controller.ts (1)
11-11: Import alias change: LGTMPath alias to "@/shared/dto/base.dto" is consistent with the repo refactor.
src/shared/middleware/validation.middleware.ts (1)
80-82: In-place query mutation: OK, but be mindful of type shapesThis preserves req.query reference; good. Just note downstream code may now see non-string values (numbers/booleans).
Please confirm no consumers assume req.query values are always strings.
src/modules/photo/README.md (1)
17-20: Align file/interface names with actual paths.If the contract file lives at
infrastructure/adapters/interface/photo-service.adapter.ts, rename the reference fromphoto-service.adapter.interface.tsto the actual path to avoid confusion. Also state explicitly which interface is the canonical contract (domain vs infrastructure).src/modules/photo/domain/entities/interfaces/photo.interface.ts (1)
32-38: Unify userId and id types across layers.Here
userIdisstringandidisstring. DTOs currently validate them as integers and routes readidfrom params. Align types with persistence and service APIs (which usestring). Update DTOs accordingly (see route comment).Also applies to: 40-49
src/modules/photo/presentation/dto/index.ts (1)
1-3: LGTM.Barrel re-exports are correct and minimal.
src/modules/photo/domain/exceptions/domain.exception.ts (2)
3-13: LGTM: focused, domain-specific exceptions.Clear messages, extend the shared base, no side effects. Good.
1-1: Import path is correct. The DomainException file resides at src/modules/shared/domain/exceptions/domain.exception.ts, so the relative import ../../../shared/domain/exceptions/domain.exception is valid.Likely an incorrect or invalid review comment.
src/modules/photo/domain/entities/photo.entity.ts (2)
15-29: Validation is fine.Covers empty/invalid URL and missing userId.
1-1: No action needed: BaseEntity import resolves correctly
The relative path "../../../shared/domain/entities/base.entity" points to src/modules/shared/domain/entities/base.entity.ts; no update required.
| export interface IStorageAdapter { | ||
| upload(file: Buffer, filename: string): Promise<{ url: string; key: string }>; | ||
| delete(key: string): Promise<void>; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove or relocate IStorageAdapter; it conflicts with the actual adapter contract.
Domain-level IStorageAdapter couples to Node Buffer and returns {url,key}, while SupabasePhotoService.upload(file: Express.Multer.File, userId: string, metadata?) → PhotoEntity uses a different shape. Keep the storage contract in infrastructure (e.g., IPhotoServiceAdapter) and delete this interface from domain to avoid duplication and drift. If a domain contract is required, make it framework-agnostic (no Buffer/Express types) and align signatures.
-export interface IStorageAdapter {
- upload(file: Buffer, filename: string): Promise<{ url: string; key: string }>;
- delete(key: string): Promise<void>;
-}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export interface IStorageAdapter { | |
| upload(file: Buffer, filename: string): Promise<{ url: string; key: string }>; | |
| delete(key: string): Promise<void>; | |
| } |
🤖 Prompt for AI Agents
In src/modules/photo/domain/entities/interfaces/photo.interface.ts lines 1-4:
remove this domain-level IStorageAdapter (delete the file or its export) because
it couples to Node Buffer and a {url,key} shape that conflicts with
SupabasePhotoService; instead move or recreate the adapter contract under the
infrastructure layer (e.g., src/modules/photo/infrastructure/ with a name like
IPhotoServiceAdapter) and update all imports to point to the new infra
interface; if the domain truly needs an interface, replace it with a
framework-agnostic contract (no Buffer/Express types) whose signature accepts an
opaque binary type or stream and returns a domain PhotoEntity (or a
domain-friendly DTO), and ensure SupabasePhotoService implements the infra
adapter signature so signatures no longer drift.
| getById(photoId: string): Promise<{ id: string; url: string } | null>; | ||
| delete(photoUrl: string): Promise<void>; | ||
| updateMetadata( |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align delete() contract to operate by photoId (not URL).
Implementation and controller use an id. The interface should reflect that to avoid confusion at the boundary.
Apply:
- delete(photoUrl: string): Promise<void>;
+ delete(photoId: string): Promise<void>;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| getById(photoId: string): Promise<{ id: string; url: string } | null>; | |
| delete(photoUrl: string): Promise<void>; | |
| updateMetadata( | |
| getById(photoId: string): Promise<{ id: string; url: string } | null>; | |
| delete(photoId: string): Promise<void>; | |
| updateMetadata( |
🤖 Prompt for AI Agents
In src/modules/photo/infrastructure/adapters/interface/photo-service.adapter.ts
around lines 10 to 12, the delete method signature currently accepts a photoUrl
but the implementation and controller operate with photoId; change the interface
to delete(photoId: string): Promise<void> and update any dependent types/usages
to expect an id (not a URL). Ensure method name and parameter name in
implementations, controllers, and unit tests are updated to match the new
signature and run tests/TS compile to catch call sites.
| file: Express.Multer.File, | ||
| userId: string, | ||
| metadata?: IPhotoMetadata | ||
| ): Promise<PhotoEntity> { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Multer memory storage prerequisite.
upload() relies on file.buffer; ensure Multer uses memoryStorage for the photo route.
Run:
🏁 Script executed:
#!/bin/bash
rg -nP 'multer\\s*\\(|memoryStorage\\s*\\(' -C2 srcLength of output: 126
🏁 Script executed:
#!/bin/bash
rg -nP 'import\s+multer' -C2 src
rg -nP 'multer\s*\(\s*{[^}]*storage' -C2 src
rg -nP 'memoryStorage\s*\(' -C2 srcLength of output: 493
Configure Multer with memoryStorage for photo uploads
upload() in supabase-service.adapter.ts reads file.buffer, which is only populated when Multer is using memoryStorage. In src/modules/photo/presentation/routes.ts, Multer is imported but not configured—update it to:
// src/modules/photo/presentation/routes.ts
-import multer from "multer";
-const upload = multer();
+import multer from "multer";
+const upload = multer({ storage: multer.memoryStorage() });and continue using upload.single('file') (or the appropriate variant) for the photo route.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| file: Express.Multer.File, | |
| userId: string, | |
| metadata?: IPhotoMetadata | |
| ): Promise<PhotoEntity> { | |
| // src/modules/photo/presentation/routes.ts | |
| import multer from "multer"; | |
| const upload = multer({ storage: multer.memoryStorage() }); | |
| // then in your route: | |
| router.post( | |
| "/photos", | |
| upload.single("file"), | |
| photoController.upload.bind(photoController) | |
| ); |
🤖 Prompt for AI Agents
In src/modules/photo/infrastructure/adapters/supabase-service.adapter.ts around
lines 10 to 13, the upload() implementation expects file.buffer (Multer
memoryStorage) but the photo route imports Multer without configuring it; update
src/modules/photo/presentation/routes.ts to import multer, create an uploader
using multer({ storage: multer.memoryStorage() }), and use that uploader (e.g.,
upload.single('file')) on the photo upload route so req.file.buffer is populated
for the adapter.
| const fileName = `${Date.now()}-${file.originalname}`; | ||
|
|
||
| // Subir archivo a Supabase Storage | ||
| const { error: uploadError } = await supabase.storage | ||
| .from("photos") | ||
| .upload(fileName, file.buffer, { | ||
| contentType: file.mimetype, | ||
| upsert: false, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use robust, sanitized object keys; avoid timestamp+originalname.
Risk of collisions and path injection via originalname. Use UUID and keep an extension whitelist; optionally namespace by userId.
Apply:
+import { randomUUID } from "crypto";
+import path from "path";
...
- const fileName = `${Date.now()}-${file.originalname}`;
+ const ext = (path.extname(file.originalname) || ".bin").toLowerCase();
+ const safeExt = [".jpg", ".jpeg", ".png", ".webp", ".gif", ".heic", ".bin"].includes(ext) ? ext : ".bin";
+ const fileName = `${userId}/${randomUUID()}${safeExt}`;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/modules/photo/infrastructure/adapters/supabase-service.adapter.ts around
lines 14 to 22, the current filename generation uses Date.now() + originalname
which risks collisions and path injection; replace this by generating a secure
UUID-based filename, derive and validate the file extension against a whitelist
(reject or map disallowed extensions), and construct the stored key as a safe
namespace (e.g., `${userId || 'anon'}/${uuid}.${ext}`) rather than using the raw
originalname; ensure you never include slashes or unsafe characters from
originalname, and keep upsert:false as before.
| const { data: publicUrl } = supabase.storage | ||
| .from("photos") | ||
| .getPublicUrl(fileName); | ||
|
|
||
| if (!publicUrl) throw new Error("Could not get public URL"); | ||
|
|
||
| // Guardar en DB con Prisma |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden public URL retrieval and error handling.
Check the error object and the presence of data.publicUrl explicitly.
Apply:
- const { data: publicUrl } = supabase.storage
+ const { data: publicUrlData, error: publicUrlError } = supabase.storage
.from("photos")
.getPublicUrl(fileName);
-
- if (!publicUrl) throw new Error("Could not get public URL");
+ if (publicUrlError || !publicUrlData?.publicUrl) {
+ throw new Error("Could not get public URL");
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { data: publicUrl } = supabase.storage | |
| .from("photos") | |
| .getPublicUrl(fileName); | |
| if (!publicUrl) throw new Error("Could not get public URL"); | |
| // Guardar en DB con Prisma | |
| const { data: publicUrlData, error: publicUrlError } = supabase.storage | |
| .from("photos") | |
| .getPublicUrl(fileName); | |
| if (publicUrlError || !publicUrlData?.publicUrl) { | |
| throw new Error("Could not get public URL"); | |
| } | |
| // Guardar en DB con Prisma |
🤖 Prompt for AI Agents
In src/modules/photo/infrastructure/adapters/supabase-service.adapter.ts around
lines 27 to 33, the current code assumes the supabase response contains a valid
data.publicUrl and throws a generic error; update it to inspect the response
shape: check the returned error object from supabase and validate that data &&
data.publicUrl exist, and if not throw an Error containing the supabase error
message (or the response object serialized) and the fileName for context; ensure
subsequent code uses data.publicUrl (not the whole data) and avoid accessing
undefined properties.
| router.delete("/:id", validateDto(DeletePhotoDto), photoController.deletePhoto); | ||
|
|
||
| router.get("/:id", validateDto(GetPhotoDto), photoController.getPhotoById); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validation applied to body, but id is in params → routes will 400/skip validation incorrectly.
validateDto reads req.body; :id lives in req.params. Create a params validator and use it here. Also, ids are string in the service/DB.
Apply this change in routes:
-router.delete("/:id", validateDto(DeletePhotoDto), photoController.deletePhoto);
-router.get("/:id", validateDto(GetPhotoDto), photoController.getPhotoById);
+router.delete("/:id", validateParamsDto(DeletePhotoParamsDto), photoController.deletePhoto);
+router.get("/:id", validateParamsDto(GetPhotoParamsDto), photoController.getPhotoById);Add this shared middleware:
export function validateParamsDto<T extends object>(dtoClass: new () => T) {
return async (req: Request, res: Response, next: NextFunction) => {
try {
const dto = plainToInstance(dtoClass, req.params);
const errors = await validate(dto);
if (errors.length) {
res.status(400).json({ success: false, error: "Validation failed", details: errors });
return;
}
req.params = dto as any;
next();
} catch {
res.status(500).json({ success: false, error: "Internal server error during validation" });
}
};
}And DTOs:
// delete-photo-params.dto.ts
import { IsString } from "class-validator";
export class DeletePhotoParamsDto { @IsString() id!: string; }
// get-photo-params.dto.ts
import { IsString } from "class-validator";
export class GetPhotoParamsDto { @IsString() id!: string; }🤖 Prompt for AI Agents
In src/modules/photo/presentation/routes.ts around lines 21-23, the current
validateDto middleware validates req.body but the :id param lives in req.params,
causing incorrect 400s; replace validateDto for these routes with a params
validator that validates req.params and casts it back to req.params as the DTO.
Add a shared middleware validateParamsDto that uses plainToInstance on
req.params, runs class-validator, returns 400 with details on validation errors
or 500 on exceptions, and assigns the validated dto back to req.params; create
DeletePhotoParamsDto and GetPhotoParamsDto with @IsString() id fields (IDs are
strings in service/DB) and use validateParamsDto(DeletePhotoParamsDto) for
DELETE "/:id" and validateParamsDto(GetPhotoParamsDto) for GET "/:id".
| Simplified Controllers: | ||
| Controllers use asyncHandler, validators, and Multer for file processing. | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm Multer is a declared dependency.
Routes import Multer. Ensure multer (and @types/multer if TS) are present in package.json to avoid runtime/TS errors.
Run:
🏁 Script executed:
#!/bin/bash
jq -r '.dependencies, .devDependencies' package.json | rg -n 'multer'Length of output: 101
Add Multer to package.json dependencies
Only @types/multer is declared; add "multer": "⟨latest-version⟩" under dependencies in package.json to prevent runtime/TS errors.
🧰 Tools
🪛 LanguageTool
[grammar] ~21-~21: There might be a mistake here.
Context: ...pabase Storage. Simplified Controllers: Controllers use asyncHandler, validators...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In src/modules/photo/README.md around lines 21 to 23, the note mentions Multer
but package.json only declares @types/multer; update package.json by adding a
runtime dependency for multer (e.g. add "multer": "<latest-version>" under
dependencies) so the library is installed at runtime and TypeScript typings
remain compatible; ensure package.json dependencies include the multer entry and
run npm/yarn install to update lockfiles.
| ): Promise<void> => { | ||
| try { | ||
| const dto = plainToClass(dtoClass, req.body); | ||
| const dto = plainToInstance(dtoClass, req.body); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implicit conversion for body DTOs
Enable class-transformer implicit conversion so numeric/boolean types (e.g., id) are correctly coerced.
Apply:
- const dto = plainToInstance(dtoClass, req.body);
+ const dto = plainToInstance(dtoClass, req.body, { enableImplicitConversion: true });Optionally add whitelist to validation to drop unknown fields:
- const errors = await validate(dto);
+ const errors = await validate(dto, { whitelist: true });🤖 Prompt for AI Agents
In src/shared/middleware/validation.middleware.ts around line 22,
plainToInstance is called without enabling class-transformer implicit conversion
so numeric/boolean body fields aren't coerced; update the plainToInstance call
to pass class-transformer options with enableImplicitConversion: true so types
like id are converted, and when calling validate pass validation options (e.g.,
{ whitelist: true }) to strip unknown fields if desired — ensure you only add
these options and do not change DTO classes.
| const dto = plainToInstance(dtoClass, req.query); | ||
| const errors = await validate(dto); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implicit conversion for query DTOs
Same rationale as body; many query params arrive as strings.
Apply:
- const dto = plainToInstance(dtoClass, req.query);
+ const dto = plainToInstance(dtoClass, req.query, { enableImplicitConversion: true });(Optional) Keep whitelist:
- const errors = await validate(dto);
+ const errors = await validate(dto, { whitelist: true });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const dto = plainToInstance(dtoClass, req.query); | |
| const errors = await validate(dto); | |
| const dto = plainToInstance(dtoClass, req.query, { enableImplicitConversion: true }); | |
| const errors = await validate(dto, { whitelist: true }); |
🤖 Prompt for AI Agents
In src/shared/middleware/validation.middleware.ts around lines 60-61, the query
DTO conversion currently treats all query params as strings; convert and
validate them correctly by using plainToInstance with enableImplicitConversion
set to true so numeric/boolean types are coerced, then call validate with the
same whitelist options you use for body DTOs (e.g., whitelist: true, optionally
forbidNonWhitelisted) to strip/forbid unexpected props; update the
plainToInstance and validate calls accordingly to enable implicit conversion and
keep the whitelist behavior.
| const dto = plainToInstance(dtoClass, req.params); | ||
| const errors = await validate(dto); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implicit conversion for params DTOs
Enable coercion so route params like ":id" can be numbers.
Apply:
- const dto = plainToInstance(dtoClass, req.params);
+ const dto = plainToInstance(dtoClass, req.params, { enableImplicitConversion: true });(Optional) Whitelist:
- const errors = await validate(dto);
+ const errors = await validate(dto, { whitelist: true });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const dto = plainToInstance(dtoClass, req.params); | |
| const errors = await validate(dto); | |
| const dto = plainToInstance(dtoClass, req.params, { enableImplicitConversion: true }); | |
| const errors = await validate(dto, { whitelist: true }); |
🤖 Prompt for AI Agents
In src/shared/middleware/validation.middleware.ts around lines 100 to 101, the
params DTO is created without enabling transformation/coercion so route params
like ":id" remain strings; update plainToInstance/validation to enable
transformation/coercion (e.g., pass class-transformer options to convert types
or use validate with { whitelist: true, transform: true, transformOptions: {
enableImplicitConversion: true } }) and optionally enable whitelist to strip
unknown properties; ensure you pass these options to validate() (or use a
combined transform+validate helper) so numeric params are coerced to numbers and
DTO validation behaves correctly.
2 participants
🚀 Volunchain Pull Request
Mark with an
xall the checkboxes that apply (like[x])📌 Type of Change
📝 Changes description
Add readme with information of arquitecture of module photo, the reason of implement adapter, is the logic of photo is basic and it's not required use cases
Add adapter to photos.
Migrate to supabaseStorage to image.
⏰ Time spent breakdown
3 hours
Summary by CodeRabbit
New Features
Documentation
Refactor
Chores