download personalization image by url#175
Conversation
📝 WalkthroughWalkthroughSe incorpora ChangesDescargas de personalizaciones
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Cliente
participant PaginaPedido
participant DownloadRoute
participant OrderRepository
participant GenerateReadUrlUseCase
Cliente->>PaginaPedido: Seleccionar descarga
PaginaPedido->>DownloadRoute: Solicitar imagen por pedido y personalización
DownloadRoute->>OrderRepository: Validar pedido y autorización
DownloadRoute->>GenerateReadUrlUseCase: Generar URL con imageUploadId
GenerateReadUrlUseCase-->>DownloadRoute: URL de lectura
DownloadRoute-->>Cliente: Stream de imagen
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/[locale]/orders/[orderId]/page.tsx (1)
185-192: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftNo elimines la descarga de snapshots históricos.
El cambio exige
imageUploadId, pero los registros existentes pueden conservar únicamenteimageUrl; sin backfill o fallback, ambas vistas dejan de ofrecer descarga y el endpoint responde 404.
app/[locale]/orders/[orderId]/page.tsx#L185-L192: conserva una ruta de compatibilidad para pedidos de clientes antiguos.app/[locale]/seller/orders/[orderId]/page.tsx#L209-L216: aplica la misma compatibilidad en la vista del vendedor.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/`[locale]/orders/[orderId]/page.tsx around lines 185 - 192, Preserva la descarga de snapshots históricos en ambas vistas: en app/[locale]/orders/[orderId]/page.tsx, dentro del enlace de descarga, y en app/[locale]/seller/orders/[orderId]/page.tsx, aplica la misma compatibilidad. Permite mostrar la descarga cuando exista imageUploadId o, para registros antiguos, cuando solo exista imageUrl, usando la ruta de fallback establecida para esos snapshots.modules/orders/application/handle-cart-checked-out.ts (1)
186-198: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
imageUploadIdno se normaliza anullcomo en el resto de constructores deCustomizationSnapshot.A diferencia de
buildCustomizationSnapshot(checkout-cart.ts:364) ytoSnapshot(customization-lookup-adapter.ts:49), aquí se asignacustomization.imageUploadIdsin?? null. Como el campo es opcional enCustomizationLookupSnapshot, si el lookup del módulo orders devuelveundefined, el snapshot congelado quedará inconsistente respecto al resto del contrato (aunque la lectura posterior enprisma-order-repository.tslo normaliza víanormalizeNullableString).🔧 Fix propuesto
- imageUploadId: customization.imageUploadId, + imageUploadId: customization.imageUploadId ?? null,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/orders/application/handle-cart-checked-out.ts` around lines 186 - 198, Actualiza toFrozenCustomizationSnapshot para normalizar customization.imageUploadId a null cuando sea undefined, manteniendo el valor existente cuando esté presente y alineando el resultado con buildCustomizationSnapshot y toSnapshot.
🧹 Nitpick comments (1)
app/api/orders/[orderId]/customizations/[customizationId]/download/route.ts (1)
7-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftFalta cobertura de test para esta ruta.
No se incluye ningún test para este endpoint, que concentra lógica de autorización crítica (cliente/vendedor/admin, ownership de la imagen) y el flujo de streaming. ¿Quieres que genere los tests unitarios (mockeando
container, la sesión yfetch) cubriendo los casos 401, 404 por pedido/personalización inexistente, 403 implícito (vendedor de otro pedido) y el caso feliz?As per coding guidelines, tests for
**/*.{test,spec}.{ts,tsx}should cover "empty states, invalid inputs, error conditions, and fakes or mocks for infrastructure dependencies."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/orders/`[orderId]/customizations/[customizationId]/download/route.ts around lines 7 - 69, Add unit tests for the GET handler covering unauthenticated requests, missing orders or customizations, unauthorized sellers accessing another seller’s order, and authorized customer/seller/admin requests. Mock getSessionUserContext, container repositories, GenerateReadUrlUseCase, fetch, and handleApiError; verify status codes, ownership arguments, and successful streaming response headers.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/api/orders/`[orderId]/customizations/[customizationId]/download/route.ts:
- Around line 19-47: Extrae la lógica de autorización, resolución del pedido,
búsqueda de la personalización y generación de la URL desde el route handler
hacia un caso de uso de application como DownloadCustomizationImageUseCase
dentro del módulo orders. Haz que reciba sessionUserId, orderId y
customizationId, use puertos/interfaces para repositorios, sellers y
almacenamiento, y preserve las respuestas de recurso no encontrado y la
propiedad del comprador al firmar la imagen. Reduce el handler a un adaptador
HTTP que invoque el caso de uso y traduzca su resultado a NextResponse.
- Around line 44-55: La llamada fetch en la ruta de descarga puede quedar
esperando indefinidamente. Actualiza el flujo alrededor de
GenerateReadUrlUseCase y fetch para usar un AbortController con un timeout
explícito, abortando la solicitud cuando se exceda y conservando la respuesta
502 para fallos de disponibilidad o cancelación.
In `@modules/customizations/application/create-customer-customization.ts`:
- Line 17: Actualiza assertCapability() para incluir imageUploadId junto con
imageUrl y designPosition al determinar si la personalización contiene una
imagen. Un payload que solo tenga imageUploadId debe activar la misma validación
de capacidad fotográfica y rechazarse cuando el producto no permita fotos,
manteniendo el comportamiento existente para los demás casos.
In
`@tests/unit/modules/customizations/application/create-customer-customization.test.ts`:
- Around line 70-92: Extend the CreateCustomerCustomization tests with an
invalid photo-upload case: configure ProductCustomizationConfig with mode
“text”, provide imageUploadId without imageUrl, and assert that useCase.execute
rejects with ValidationError. Keep the existing successful preservation test
unchanged and target the validation path that enforces allowsPhoto().
---
Outside diff comments:
In `@app/`[locale]/orders/[orderId]/page.tsx:
- Around line 185-192: Preserva la descarga de snapshots históricos en ambas
vistas: en app/[locale]/orders/[orderId]/page.tsx, dentro del enlace de
descarga, y en app/[locale]/seller/orders/[orderId]/page.tsx, aplica la misma
compatibilidad. Permite mostrar la descarga cuando exista imageUploadId o, para
registros antiguos, cuando solo exista imageUrl, usando la ruta de fallback
establecida para esos snapshots.
In `@modules/orders/application/handle-cart-checked-out.ts`:
- Around line 186-198: Actualiza toFrozenCustomizationSnapshot para normalizar
customization.imageUploadId a null cuando sea undefined, manteniendo el valor
existente cuando esté presente y alineando el resultado con
buildCustomizationSnapshot y toSnapshot.
---
Nitpick comments:
In `@app/api/orders/`[orderId]/customizations/[customizationId]/download/route.ts:
- Around line 7-69: Add unit tests for the GET handler covering unauthenticated
requests, missing orders or customizations, unauthorized sellers accessing
another seller’s order, and authorized customer/seller/admin requests. Mock
getSessionUserContext, container repositories, GenerateReadUrlUseCase, fetch,
and handleApiError; verify status codes, ownership arguments, and successful
streaming response headers.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 99db924b-5af9-42d4-970b-0ec64dfebc26
📒 Files selected for processing (19)
app/[locale]/orders/[orderId]/page.tsxapp/[locale]/seller/orders/[orderId]/page.tsxapp/api/orders/[orderId]/customizations/[customizationId]/download/route.tsmodules/cart/application/checkout-cart.tsmodules/cart/domain/customer-customization-create-port.tsmodules/cart/domain/customization-lookup-port.tsmodules/cart/infrastructure/customization-lookup-adapter.tsmodules/cart/presentation/schemas/cart-schemas.tsmodules/customizations/application/create-customer-customization.tsmodules/customizations/domain/entities/customization.tsmodules/customizations/infrastructure/prisma-customization-repository.tsmodules/orders/application/handle-cart-checked-out.tsmodules/orders/domain/customization-lookup-port.tsmodules/orders/infrastructure/prisma-order-repository.tsprisma/migrations/20260719163000_add_customization_image_upload_id/migration.sqlprisma/schema.prismatests/integration/orders/prisma-order-repository.integration.test.tstests/unit/modules/cart/application/checkout-cart.test.tstests/unit/modules/customizations/application/create-customer-customization.test.ts
| const { orderId, customizationId } = await context.params; | ||
| const order = await container.getOrderRepository().findById(orderId); | ||
| if (!order) { | ||
| return NextResponse.json({ error: 'Not found' }, { status: 404 }); | ||
| } | ||
|
|
||
| const isCustomer = order.userId === session.userId; | ||
| const seller = isCustomer | ||
| ? null | ||
| : await container.getSellerLookup().findByUserId(session.userId); | ||
| const isSeller = seller?.sellerId === order.sellerId; | ||
| const isAdmin = session.role === 'ADMIN'; | ||
| if (!isCustomer && !isSeller && !isAdmin) { | ||
| return NextResponse.json({ error: 'Not found' }, { status: 404 }); | ||
| } | ||
|
|
||
| const customization = order.lineItems | ||
| ?.flatMap((item) => item.customizationSnapshot ?? []) | ||
| .find((snapshot) => snapshot.id === customizationId); | ||
| if (!customization?.imageUploadId) { | ||
| return NextResponse.json({ error: 'Not found' }, { status: 404 }); | ||
| } | ||
|
|
||
| // The order authorization above permits the seller; the upload remains | ||
| // owned by the buyer, so preserve that ownership check when signing. | ||
| const readUrl = await new GenerateReadUrlUseCase( | ||
| container.getUploadRepository(), | ||
| container.getStoragePort(), | ||
| ).execute(customization.imageUploadId, undefined, order.userId); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Lógica de negocio y orquestación cross-módulo en la capa de presentación.
El route handler resuelve el pedido, decide autorización cliente/vendedor/admin y localiza la personalización, importando directamente repositorios y lookups de los módulos orders, sellers y uploads (container.getOrderRepository(), container.getSellerLookup(), container.getUploadRepository(), container.getStoragePort(), y la instanciación manual de GenerateReadUrlUseCase en línea 44-47). Esto mezcla lógica de dominio/aplicación en la presentación y acopla 3 módulos directamente en lugar de a través de eventos o puertos.
Se recomienda extraer un caso de uso (por ejemplo DownloadCustomizationImageUseCase) en la capa application del módulo orders que reciba sessionUserId, orderId, customizationId y encapsule la autorización y el lookup; el route handler quedaría solo como adaptador HTTP.
As per coding guidelines, "Every module must have clear domain, application, infrastructure, and presentation layers" and "Modules may communicate only through domain events or interfaces (ports/adapters)."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/api/orders/`[orderId]/customizations/[customizationId]/download/route.ts
around lines 19 - 47, Extrae la lógica de autorización, resolución del pedido,
búsqueda de la personalización y generación de la URL desde el route handler
hacia un caso de uso de application como DownloadCustomizationImageUseCase
dentro del módulo orders. Haz que reciba sessionUserId, orderId y
customizationId, use puertos/interfaces para repositorios, sellers y
almacenamiento, y preserve las respuestas de recurso no encontrado y la
propiedad del comprador al firmar la imagen. Reduce el handler a un adaptador
HTTP que invoque el caso de uso y traduzca su resultado a NextResponse.
Source: Coding guidelines
| const readUrl = await new GenerateReadUrlUseCase( | ||
| container.getUploadRepository(), | ||
| container.getStoragePort(), | ||
| ).execute(customization.imageUploadId, undefined, order.userId); | ||
| const sourceUrl = new URL(readUrl.url, request.nextUrl.origin); | ||
| const source = await fetch(sourceUrl); | ||
| if (!source.ok || !source.body) { | ||
| return NextResponse.json( | ||
| { error: 'Download unavailable' }, | ||
| { status: 502 }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Llamada fetch al storage sin timeout.
fetch(sourceUrl) no tiene límite de tiempo; si el backend de almacenamiento no responde, la petición queda colgada consumiendo el hilo/función de la ruta indefinidamente.
🔧 Fix propuesto
- const source = await fetch(sourceUrl);
+ const source = await fetch(sourceUrl, { signal: AbortSignal.timeout(10_000) });📝 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 readUrl = await new GenerateReadUrlUseCase( | |
| container.getUploadRepository(), | |
| container.getStoragePort(), | |
| ).execute(customization.imageUploadId, undefined, order.userId); | |
| const sourceUrl = new URL(readUrl.url, request.nextUrl.origin); | |
| const source = await fetch(sourceUrl); | |
| if (!source.ok || !source.body) { | |
| return NextResponse.json( | |
| { error: 'Download unavailable' }, | |
| { status: 502 }, | |
| ); | |
| } | |
| const readUrl = await new GenerateReadUrlUseCase( | |
| container.getUploadRepository(), | |
| container.getStoragePort(), | |
| ).execute(customization.imageUploadId, undefined, order.userId); | |
| const sourceUrl = new URL(readUrl.url, request.nextUrl.origin); | |
| const source = await fetch(sourceUrl, { signal: AbortSignal.timeout(10_000) }); | |
| if (!source.ok || !source.body) { | |
| return NextResponse.json( | |
| { error: 'Download unavailable' }, | |
| { status: 502 }, | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/api/orders/`[orderId]/customizations/[customizationId]/download/route.ts
around lines 44 - 55, La llamada fetch en la ruta de descarga puede quedar
esperando indefinidamente. Actualiza el flujo alrededor de
GenerateReadUrlUseCase y fetch para usar un AbortController con un timeout
explícito, abortando la solicitud cuando se exceda y conservando la respuesta
502 para fallos de disponibilidad o cancelación.
| color?: string | null; | ||
| size?: string | null; | ||
| imageUrl?: string | null; | ||
| imageUploadId?: string | null; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Incluye imageUploadId en la validación de capacidad fotográfica.
assertCapability() solo considera imageUrl y designPosition (Lines 40–43). Un payload con únicamente imageUploadId puede persistir una personalización de imagen aunque el producto no permita fotos.
Propuesta
- const hasImage = dto.imageUrl !== undefined && dto.imageUrl !== null;
+ const hasImage = dto.imageUrl !== undefined && dto.imageUrl !== null;
+ const hasImageUpload =
+ dto.imageUploadId !== undefined && dto.imageUploadId !== null;
const hasDesignPosition =
dto.designPosition !== undefined && dto.designPosition !== null;
- const hasImageForCapability = hasImage || hasDesignPosition;
+ const hasImageForCapability =
+ hasImage || hasImageUpload || hasDesignPosition;Also applies to: 87-87
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@modules/customizations/application/create-customer-customization.ts` at line
17, Actualiza assertCapability() para incluir imageUploadId junto con imageUrl y
designPosition al determinar si la personalización contiene una imagen. Un
payload que solo tenga imageUploadId debe activar la misma validación de
capacidad fotográfica y rechazarse cuando el producto no permita fotos,
manteniendo el comportamiento existente para los demás casos.
| it('preserves the design upload id for later authorized downloads', async () => { | ||
| capability = { | ||
| async getConfig() { | ||
| return ProductCustomizationConfig.fromJson({ | ||
| mode: 'photo', | ||
| previewEnabled: true, | ||
| previewTemplateUrl: 'https://cdn.example.com/base.png', | ||
| }); | ||
| }, | ||
| }; | ||
| useCase = new CreateCustomerCustomization(repo, capability); | ||
|
|
||
| const result = await useCase.execute( | ||
| { | ||
| productId: 'p-1', | ||
| imageUrl: 'https://cdn.example.com/design.png', | ||
| imageUploadId: 'upload-1', | ||
| }, | ||
| 'user-1', | ||
| ); | ||
|
|
||
| expect(result.imageUploadId).toBe('upload-1'); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Añade una prueba para rechazar imágenes en productos no compatibles.
Este test solo cubre el caso exitoso. Añade un caso con imageUploadId sin imageUrl para un producto con modo text, esperando ValidationError; así se evita que el nuevo campo evada allowsPhoto().
Según las directrices de codificación, los tests deben cubrir entradas inválidas y condiciones de error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@tests/unit/modules/customizations/application/create-customer-customization.test.ts`
around lines 70 - 92, Extend the CreateCustomerCustomization tests with an
invalid photo-upload case: configure ProductCustomizationConfig with mode
“text”, provide imageUploadId without imageUrl, and assert that useCase.execute
rejects with ValidationError. Keep the existing successful preservation test
unchanged and target the validation path that enforces allowsPhoto().
Source: Coding guidelines
1 participant
Summary by CodeRabbit
Nuevas funcionalidades
Correcciones