Feat/ux better details pr1#141
Conversation
|
Warning Review limit reached
Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughLa página del carrito resuelve snapshots de personalización y los muestra con enlaces de edición. También se actualizan estilos responsivos, formato monetario español, textos localizados, límites de uploads, semántica de la página inicial y pruebas asociadas. ChangesCarrito y presentación
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CartPage
participant CustomizationLookup
participant CartView
CartPage->>CustomizationLookup: findByIds(customizationIdList)
CustomizationLookup-->>CartPage: snapshots de personalización
CartPage->>CartView: items enriquecidos y labels
CartView-->>CartPage: enlace de edición con parámetros
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
modules/cart/presentation/components/cart-view.module.css (1)
43-328: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRestablece el cumplimiento de Stylelint.
Los selectores camelCase modificados incumplen
selector-class-pattern, y la media query incumplemedia-feature-range-notation. Renombra los selectores junto con sus consumidores CSS Module, o configura una excepción específica para CSS Modules si camelCase es la convención intencional.🤖 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/cart/presentation/components/cart-view.module.css` around lines 43 - 328, Restablece el cumplimiento de Stylelint en los selectores del módulo CSS: renombra las clases camelCase de cart-view.module.css a kebab-case y actualiza todos sus consumidores CSS Module correspondientes, o añade una excepción específica para CSS Modules si la convención camelCase debe mantenerse. Corrige también la media query responsive para usar la notación de rango exigida por media-feature-range-notation.Source: Linters/SAST tools
🧹 Nitpick comments (3)
tests/unit/app/[locale]/products/[id]/page.test.tsx (1)
210-255: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsidera reemplazar el bloque comentado por un TODO o un issue.
45 líneas de código de prueba comentado son código muerto que no se ejecuta y puede quedar desactualizado fácilmente. Ya que la característica está planificada para PR-3, sería más sostenible registrarla como un issue o dejar un comentario breve
// TODO(PR-3): …en lugar de commitar el bloque completo.🤖 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/app/`[locale]/products/[id]/page.test.tsx around lines 210 - 255, Remove the commented-out test block for cart customization editing and replace it with a concise TODO referencing PR-3 and the intended behavior, or track the work in an issue instead. Keep the existing executable tests unchanged.modules/uploads/application/upload-error-map.ts (1)
8-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHaz que el mapa dependa de
UPLOAD_ERROR_CODES.Los literales duplicados y
Record<string, string>permiten que las claves del mapa y los códigos se desincronicen sin error de tipos. Define primero los códigos y usa claves calculadas consatisfiespara mantener el contrato tipado.🤖 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/uploads/application/upload-error-map.ts` around lines 8 - 16, Define primero UPLOAD_ERROR_CODES, then update UPLOAD_ERROR_MAP to use computed keys from those constants and validate it with satisfies, replacing the broad Record<string, string> annotation so key and value contracts remain synchronized.tests/unit/modules/uploads/application/upload-error-map.test.ts (1)
34-52: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winComprueba la whitelist completa, no solo sus miembros.
toContaindetecta MIME faltantes, pero las pruebas seguirían pasando si se añade accidentalmente un tipo no permitido. Compara el array completo contoEqual([...])para proteger el contrato de validación.🤖 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/uploads/application/upload-error-map.test.ts` around lines 34 - 52, Actualiza las pruebas de ALLOWED_MIME_TYPES para comparar el array completo mediante toEqual([...]) en lugar de comprobaciones individuales con toContain. Incluye exactamente los MIME permitidos actuales: image/jpeg, image/png, image/webp, video/mp4 y video/webm, preservando el contrato completo de la whitelist.
🤖 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 `@tests/unit/app/`[locale]/cart/cart-view-customization.test.tsx:
- Around line 87-89: Update the customization link flow to pass the cart line
identifier item.id into buildCustomizationHref and serialize it as the
customizationCartItemId query parameter, ensuring the generated edit href
identifies the correct cart item.
In `@tests/unit/app/`[locale]/products/[id]/mockup-canvas-control.test.tsx:
- Around line 113-114: Actualiza la prueba alrededor de
`fireEvent.load(replacementImage)` para limpiar el mock `context.clearRect`
después del montaje inicial y antes de disparar el evento `load`. Mantén la
aserción posterior verificando que `clearRect` fue llamado, aislando así el
redibujado provocado por el manejador `onLoad`.
In `@tests/unit/components/cart/add-to-cart-button.test.tsx`:
- Around line 151-195: Actualiza la lógica de AddToCartButton para que, cuando
exista una línea invitada del mismo producto con una personalización distinta,
permita añadir la nueva línea sin mostrar alreadyInCartDifferent. Conserva el
uso de addItem con la nueva customizationText y ajusta la prueba solo si el
comportamiento esperado del producto sigue siendo mostrar el aviso.
---
Outside diff comments:
In `@modules/cart/presentation/components/cart-view.module.css`:
- Around line 43-328: Restablece el cumplimiento de Stylelint en los selectores
del módulo CSS: renombra las clases camelCase de cart-view.module.css a
kebab-case y actualiza todos sus consumidores CSS Module correspondientes, o
añade una excepción específica para CSS Modules si la convención camelCase debe
mantenerse. Corrige también la media query responsive para usar la notación de
rango exigida por media-feature-range-notation.
---
Nitpick comments:
In `@modules/uploads/application/upload-error-map.ts`:
- Around line 8-16: Define primero UPLOAD_ERROR_CODES, then update
UPLOAD_ERROR_MAP to use computed keys from those constants and validate it with
satisfies, replacing the broad Record<string, string> annotation so key and
value contracts remain synchronized.
In `@tests/unit/app/`[locale]/products/[id]/page.test.tsx:
- Around line 210-255: Remove the commented-out test block for cart
customization editing and replace it with a concise TODO referencing PR-3 and
the intended behavior, or track the work in an issue instead. Keep the existing
executable tests unchanged.
In `@tests/unit/modules/uploads/application/upload-error-map.test.ts`:
- Around line 34-52: Actualiza las pruebas de ALLOWED_MIME_TYPES para comparar
el array completo mediante toEqual([...]) en lugar de comprobaciones
individuales con toContain. Incluye exactamente los MIME permitidos actuales:
image/jpeg, image/png, image/webp, video/mp4 y video/webm, preservando el
contrato completo de la whitelist.
🪄 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: 531e726d-7d42-4198-9921-e8fd9cf9e468
⛔ Files ignored due to path filters (1)
public/img/decorations/formas-15.svgis excluded by!**/*.svg
📒 Files selected for processing (23)
app/[locale]/cart/page.tsxapp/[locale]/page.tsxmodules/cart/presentation/components/cart-view.module.cssmodules/uploads/application/create-upload-use-case.tsmodules/uploads/application/upload-error-map.tsshared/kernel/domain/value-objects/money.tsshared/ui/button.tsxshared/ui/index.tsshared/ui/middle-section.module.cssshared/ui/middle-section.tsxtests/unit/app/[locale]/admin/sellers/[sellerId]/seller-detail-form.test.tsxtests/unit/app/[locale]/cart/cart-view-customization.test.tsxtests/unit/app/[locale]/page.test.tsxtests/unit/app/[locale]/products/[id]/mockup-canvas-control.test.tsxtests/unit/app/[locale]/products/[id]/page.test.tsxtests/unit/app/cart/cart-view-guest.test.tsxtests/unit/app/cart/cart-view-snapshots.test.tsxtests/unit/app/cart/cart-view.test.tsxtests/unit/components/cart/add-to-cart-button.test.tsxtests/unit/modules/presentation/components/button.test.tsxtests/unit/modules/presentation/components/login-modal.test.tsxtests/unit/modules/uploads/application/upload-error-map.test.tstests/unit/shared/kernel/money.test.ts
💤 Files with no reviewable changes (3)
- shared/ui/index.ts
- shared/ui/middle-section.tsx
- shared/ui/middle-section.module.css
| expect(link.getAttribute('href')).toContain( | ||
| 'customizationCartItemId=item-1', | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Incluye el identificador de la línea del carrito en el enlace de edición.
La nueva aserción falla porque el href omite customizationCartItemId. Propaga item.id a buildCustomizationHref y serializa ese parámetro; sin él, el PDP no puede identificar qué línea debe actualizar.
🧰 Tools
🪛 GitHub Actions: CI / 2_unit.txt
[error] 87-89: Test failed: link href missing expected query param. Expected href to contain 'customizationCartItemId=item-1' but received '/es/products/prod-1?customizationText=Hello&customizationColor=Blue&customizationSize=M&customizationImageUrl=%2Fcustomization.png'.
🪛 GitHub Actions: CI / unit
[error] 87-87: CartView customization links test failed. Expected link href to contain 'customizationCartItemId=item-1', but received '/es/products/prod-1?customizationText=Hello&customizationColor=Blue&customizationSize=M&customizationImageUrl=%2Fcustomization.png'.
🪛 GitHub Check: unit
[failure] 87-87: [unit-react] tests/unit/app/[locale]/cart/cart-view-customization.test.tsx > CartView customization links > links back to the PDP with customization query parameters
AssertionError: expected '/es/products/prod-1?customizationText…' to contain 'customizationCartItemId=item-1'
Expected: "customizationCartItemId=item-1"
Received: "/es/products/prod-1?customizationText=Hello&customizationColor=Blue&customizationSize=M&customizationImageUrl=%2Fcustomization.png"
❯ tests/unit/app/[locale]/cart/cart-view-customization.test.tsx:87:39
🤖 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/app/`[locale]/cart/cart-view-customization.test.tsx around lines
87 - 89, Update the customization link flow to pass the cart line identifier
item.id into buildCustomizationHref and serialize it as the
customizationCartItemId query parameter, ensuring the generated edit href
identifies the correct cart item.
Source: Linters/SAST tools
| fireEvent.load(replacementImage); | ||
| expect(context.clearRect).toHaveBeenCalled(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
La aserción no aísla el redibujado tras el evento load.
context.clearRect ya se invoca durante el montaje inicial (vía configureCanvas → drawCanvas en el useEffect), por lo que expect(context.clearRect).toHaveBeenCalled() pasaría incluso si el manejador onLoad no funcionara. Para verificar realmente que el redibujado ocurre a causa del evento load, limpia el mock antes de dispararlo:
🧪 Propuesta de corrección
expect(replacementImage.getAttribute('src')).toBe('/replacement.png');
+ context.clearRect.mockClear();
fireEvent.load(replacementImage);
expect(context.clearRect).toHaveBeenCalled();📝 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.
| fireEvent.load(replacementImage); | |
| expect(context.clearRect).toHaveBeenCalled(); | |
| expect(replacementImage.getAttribute('src')).toBe('/replacement.png'); | |
| context.clearRect.mockClear(); | |
| fireEvent.load(replacementImage); | |
| expect(context.clearRect).toHaveBeenCalled(); |
🤖 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/app/`[locale]/products/[id]/mockup-canvas-control.test.tsx around
lines 113 - 114, Actualiza la prueba alrededor de
`fireEvent.load(replacementImage)` para limpiar el mock `context.clearRect`
después del montaje inicial y antes de disparar el evento `load`. Mantén la
aserción posterior verificando que `clearRect` fue llamado, aislando así el
redibujado provocado por el manejador `onLoad`.
| it('adds a distinct personalized line without showing a conflicting-customization warning', async () => { | ||
| mockUseGuestCart.mockReturnValue({ | ||
| items: [ | ||
| { | ||
| id: 'guest-item-1', | ||
| productId: 'prod-1', | ||
| sellerId: 'seller-1', | ||
| quantity: 1, | ||
| unitPriceSnapshot: 29.99, | ||
| customizationText: 'First design', | ||
| }, | ||
| ], | ||
| itemCount: 1, | ||
| addItem: mockAddItem, | ||
| updateQuantity: vi.fn(), | ||
| removeItem: vi.fn(), | ||
| updateItemQuantity: vi.fn(), | ||
| removeItemById: vi.fn(), | ||
| updateCustomization: vi.fn(), | ||
| updateItemCustomization: vi.fn(), | ||
| clearCart: vi.fn(), | ||
| hydrated: true, | ||
| }); | ||
|
|
||
| render( | ||
| <AddToCartButton | ||
| {...defaultProps} | ||
| customization={{ text: 'Second design' }} | ||
| labels={{ | ||
| ...defaultProps.labels, | ||
| alreadyInCartDifferent: 'Already customized differently', | ||
| }} | ||
| />, | ||
| ); | ||
|
|
||
| expect( | ||
| screen.queryByText('Already customized differently'), | ||
| ).not.toBeInTheDocument(); | ||
|
|
||
| fireEvent.click(screen.getByRole('button', { name: /add to cart/i })); | ||
|
|
||
| expect(mockAddItem).toHaveBeenCalledWith( | ||
| expect.objectContaining({ customizationText: 'Second design' }), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Alinea el flujo invitado con la nueva expectativa de personalizaciones distintas.
Esta prueba falla porque una línea existente del mismo producto activa alreadyInCartDifferent y muestra el aviso. Ajusta la lógica para permitir la nueva línea personalizada sin ese aviso, o revisa la expectativa si ese aviso sigue siendo el comportamiento requerido.
🧰 Tools
🪛 GitHub Actions: CI / 2_unit.txt
[error] 188-188: Test failed: expected element NOT to be in the document. Found Already customized differently instead. (expect(...).not.toBeInTheDocument())
🪛 GitHub Actions: CI / unit
[error] 188-188: AddToCartButton test failed (guest user). Expected element not to be in the document, but found Already customized differently.
🪛 GitHub Check: unit
[failure] 188-188: [unit-react] tests/unit/components/cart/add-to-cart-button.test.tsx > AddToCartButton > guest user (unauthenticated) > adds a distinct personalized line without showing a conflicting-customization warning
Error: expect(element).not.toBeInTheDocument()
expected document not to contain element, found <span
class="_alreadyInCartLabel_d81c52"
Already customized differently
instead
❯ tests/unit/components/cart/add-to-cart-button.test.tsx:188:13
🤖 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/components/cart/add-to-cart-button.test.tsx` around lines 151 -
195, Actualiza la lógica de AddToCartButton para que, cuando exista una línea
invitada del mismo producto con una personalización distinta, permita añadir la
nueva línea sin mostrar alreadyInCartDifferent. Conserva el uso de addItem con
la nueva customizationText y ajusta la prueba solo si el comportamiento esperado
del producto sigue siendo mostrar el aviso.
Source: Linters/SAST tools
1 participant
Summary by CodeRabbit
Nuevas funcionalidades
Mejoras visuales
Correcciones