Feat/ux better details pr3 - #145
Conversation
📝 WalkthroughWalkthroughSe incorporan etiquetas localizadas para fotos personalizables, con validación, persistencia y edición por traducción. Las experiencias de personalización y los formularios de vendedor ahora conservan ids y muestran esas etiquetas. También se añade un guard de navegación para cambios no guardados con textos traducidos. ChangesEtiquetas localizadas y cambios no guardados
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Seller as ProductForm
participant Gallery as ProductPhotoBucketGallery
participant API as UpdateProductUseCase
participant DB as PrismaProductRepository
Seller->>Gallery: muestra localizedPhotoLabels
Gallery->>Seller: actualiza photoLabel por photo.id
Seller->>API: envía traducciones e imágenes
API->>DB: persiste etiquetas limpiadas
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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
modules/products/application/create-product-use-case.ts (1)
57-59: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFalta
photoLabelsen el path de traducción única.El objeto construido en el path de
dto.locale + dto.name(líneas 53-60) incluyetags,sizesydesignChangeDescriptiondesdedto.translation?.*, pero omitephotoLabels. Como resultado,translation.photoLabelssiempre esundefineden este path, y el fallback?? {}en la línea 85 descarta cualquier etiqueta enviada víadto.translation.photoLabels.🐛 Propuesta de fix
{ locale: dto.locale, name: dto.name, description: dto.description, tags: dto.translation?.tags, sizes: dto.translation?.sizes, designChangeDescription: dto.translation?.designChangeDescription, + photoLabels: dto.translation?.photoLabels, },🤖 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/products/application/create-product-use-case.ts` around lines 57 - 59, Incluye `photoLabels: dto.translation?.photoLabels` en el objeto de traducción del path `dto.locale + dto.name`, junto a `tags`, `sizes` y `designChangeDescription`, para conservar las etiquetas enviadas en `dto.translation.photoLabels` y evitar que el fallback `?? {}` las descarte.
🧹 Nitpick comments (3)
tests/unit/shared/hooks/use-unsaved-changes-guard.test.tsx (1)
42-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFalta cobertura para el caso de enlaces de solo-hash.
Ya que el hook debería ignorar navegación a
#hashen la misma página (ver comentario enshared/hooks/use-unsaved-changes-guard.tsxlíneas 38-46), sería valioso añadir un caso de test similar a este que verifique queevent.defaultPreventedsigue siendofalsepara un enlacehref="#section"incluso conisDirty = true.🤖 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/shared/hooks/use-unsaved-changes-guard.test.tsx` around lines 42 - 52, Añade un caso de prueba junto a “does not intercept a navigation link when clean” que renderice useUnsavedChangesGuard(true, labels), cree un enlace con href="`#section`" y dispare un click cancelable. Verifica que event.defaultPrevented permanezca en false, cubriendo la navegación hash en la misma página.shared/hooks/use-unsaved-changes-guard.tsx (1)
47-52: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy liftEl guard no cubre la navegación con el botón "atrás" del navegador (
popstate).Solo se escuchan
beforeunloadyclick; una navegación vía historial (back/forward) no dispara ninguno de los dos, por lo que el usuario puede perder cambios sin ver el aviso. Implementarlo correctamente requiere interceptarpopstatey "cancelarlo" reinsertando una entrada en el historial, lo cual es delicado en Next.js App Router.🤖 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 `@shared/hooks/use-unsaved-changes-guard.tsx` around lines 47 - 52, Amplía el guard alrededor de handleBeforeUnload y handleClick para interceptar también popstate durante navegaciones atrás/adelante. Cuando existan cambios sin guardar, muestra el aviso y reintroduce la entrada necesaria en history para cancelar la navegación, manteniendo el comportamiento compatible con Next.js App Router. Registra el listener y elimínalo junto con los listeners existentes en la limpieza.tests/unit/modules/products/infrastructure/prisma-product-repository.test.ts (1)
43-82: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCobertura de prueba insuficiente para
photoLabelscon datos reales.El fixture
makeProductno incluyephotoLabelsen las traducciones, por lo que todos los tests solo ejercitan la rutaPrisma.JsonNull. No hay pruebas que verifiquen la persistencia de datos reales (e.g.,{ 'img-1': 'Frente' }) ni la lógica defensiva del mapper (filtrado de valores no-string, rechazo de arrays).🧪 Propuesta de test con photoLabels reales
function makeProduct(overrides: Partial<ProductEntity> = {}): ProductEntity { return { // ... translations: [ { locale: 'es', name: 'Camiseta', description: 'Camiseta base', tags: ['ropa'], sizes: ['S', 'M'], designChangeDescription: null, + photoLabels: { 'img-1': 'Frente', 'img-2': 'Detrás' }, }, { locale: 'cat', name: 'Samarreta', description: 'Samarreta base', tags: ['roba'], sizes: ['M', 'L'], designChangeDescription: 'Canvi', + photoLabels: { 'img-1': 'Davant' }, }, ], // ... }; }Y actualizar las aserciones correspondientes para verificar que
photoLabelsse persiste con los valores reales en lugar dePrisma.JsonNull.🤖 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/products/infrastructure/prisma-product-repository.test.ts` around lines 43 - 82, Actualiza el fixture makeProduct para incluir photoLabels reales en las traducciones, por ejemplo { 'img-1': 'Frente' }, y ajusta las aserciones de persistencia para esperar esos valores en lugar de Prisma.JsonNull. Añade cobertura del mapper para filtrar valores que no sean strings y rechazar entradas donde photoLabels sea un array, preservando el comportamiento defensivo esperado.
🤖 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/`[locale]/products/[id]/customization-experience.tsx:
- Around line 145-164: Exclude the transient error field from the
unsaved-changes comparison around initialDraftSnapshot and
useUnsavedChangesGuard. Normalize or remove draft.error before comparing it with
the initial snapshot so validation failures do not trigger the guard when no
editable fields changed.
In `@app/`[locale]/seller/products/product-form.tsx:
- Around line 611-627: Alinea initialFormSnapshot con el estado inicial
normalizado: en la construcción de initialFormSnapshot, reutiliza la misma
limpieza de translations mediante cleanPhotoLabels y validPhotoIds aplicada al
estado form dentro del inicializador de useState. Mantén idénticos los valores
normalizados para evitar que photoLabels huérfanos activen el guard al cargar.
- Around line 351-369: Actualiza buildPayload para que una traducción cuyo único
contenido sea photoLabels con valores vacíos no se incluya en translations[].
Considera photoLabels como contenido solo cuando al menos una etiqueta tenga un
valor no vacío, manteniendo la lógica existente para name y otros campos. Aplica
la misma regla en findTranslationWithMissingName para que esas traducciones se
consideren vacías al validar nombres.
- Around line 648-670: Actualiza initialFormSnapshot y la comparación pasada a
useUnsavedChangesGuard para incluir el estado de images, cubriendo cover,
showcase y customizableBase. Usa la misma representación normalizada del orden y
contenido de imágenes en ambos snapshots para detectar adiciones, eliminaciones
y reordenamientos, manteniendo sin cambios la comparación existente de price,
translations y customizationConfig.
In `@modules/sellers/presentation/components/seller-detail-form.tsx`:
- Around line 47-52: Update the unsavedChangesLabels configuration passed to
useUnsavedChangesGuard so each label property—title, message, leave, and
stay—uses its default value when undefined, rather than only defaulting the
entire object. Preserve caller-provided values for fields that are defined.
- Around line 39-46: Actualiza el flujo de SellerDetailForm para que use como
referencia los últimos valores guardados y no el estado saved al activar
useUnsavedChangesGuard. Tras un guardado exitoso en handleSubmit, actualiza esa
referencia con name y description; así el guard permanece desactivado sin
cambios posteriores y vuelve a activarse cuando el usuario edite cualquiera de
esos campos.
In `@shared/hooks/use-unsaved-changes-guard.tsx`:
- Around line 38-46: Actualiza el manejador de navegación para que los enlaces
cuyo destino solo cambia el hash, sin modificar pathname ni search respecto a la
ubicación actual, se ignoren antes de llamar a event.preventDefault() o
setPendingHref. Mantén la protección existente para navegaciones internas que
cambien de página o consulta.
---
Outside diff comments:
In `@modules/products/application/create-product-use-case.ts`:
- Around line 57-59: Incluye `photoLabels: dto.translation?.photoLabels` en el
objeto de traducción del path `dto.locale + dto.name`, junto a `tags`, `sizes` y
`designChangeDescription`, para conservar las etiquetas enviadas en
`dto.translation.photoLabels` y evitar que el fallback `?? {}` las descarte.
---
Nitpick comments:
In `@shared/hooks/use-unsaved-changes-guard.tsx`:
- Around line 47-52: Amplía el guard alrededor de handleBeforeUnload y
handleClick para interceptar también popstate durante navegaciones
atrás/adelante. Cuando existan cambios sin guardar, muestra el aviso y
reintroduce la entrada necesaria en history para cancelar la navegación,
manteniendo el comportamiento compatible con Next.js App Router. Registra el
listener y elimínalo junto con los listeners existentes en la limpieza.
In
`@tests/unit/modules/products/infrastructure/prisma-product-repository.test.ts`:
- Around line 43-82: Actualiza el fixture makeProduct para incluir photoLabels
reales en las traducciones, por ejemplo { 'img-1': 'Frente' }, y ajusta las
aserciones de persistencia para esperar esos valores en lugar de
Prisma.JsonNull. Añade cobertura del mapper para filtrar valores que no sean
strings y rechazar entradas donde photoLabels sea un array, preservando el
comportamiento defensivo esperado.
In `@tests/unit/shared/hooks/use-unsaved-changes-guard.test.tsx`:
- Around line 42-52: Añade un caso de prueba junto a “does not intercept a
navigation link when clean” que renderice useUnsavedChangesGuard(true, labels),
cree un enlace con href="`#section`" y dispare un click cancelable. Verifica que
event.defaultPrevented permanezca en false, cubriendo la navegación hash en la
misma página.
🪄 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: 81c22e95-1602-406e-b943-f80a939941fc
📒 Files selected for processing (35)
app/[locale]/admin/sellers/[sellerId]/page.tsxapp/[locale]/products/[id]/customization-experience.tsxapp/[locale]/products/[id]/customization-form.tsxapp/[locale]/products/[id]/page.tsxapp/[locale]/seller/products/[id]/edit/page.tsxapp/[locale]/seller/products/product-form.tsxapp/[locale]/seller/products/product-photo-gallery.tsxmodules/products/application/create-product-use-case.tsmodules/products/application/product-image-builder.tsmodules/products/application/update-product-use-case.tsmodules/products/domain/entities/product-translation.tsmodules/products/infrastructure/mapper.tsmodules/products/infrastructure/prisma-product-repository.tsmodules/products/presentation/components/product-translation-section.tsxmodules/products/presentation/product-form-labels.tsmodules/products/presentation/schemas/product-form-schema.tsmodules/sellers/presentation/components/seller-detail-form.tsxprisma/migrations/20260713110000_add_product_translation_photo_labels/migration.sqlprisma/schema.prismashared/hooks/use-unsaved-changes-guard.tsxshared/i18n/locales/cat.jsonshared/i18n/locales/es.jsonshared/ui/button.tsxtests/unit/app/[locale]/admin/sellers/[sellerId]/page.test.tsxtests/unit/app/[locale]/admin/sellers/[sellerId]/seller-detail-form.test.tsxtests/unit/app/[locale]/products/[id]/customization-experience.test.tsxtests/unit/app/[locale]/products/[id]/customization-form.test.tsxtests/unit/app/[locale]/seller/products/product-photo-gallery.test.tsxtests/unit/modules/products/application/create-product-use-case.test.tstests/unit/modules/products/application/update-product-use-case.test.tstests/unit/modules/products/domain/entities/product-translation.test.tstests/unit/modules/products/infrastructure/prisma-product-repository.test.tstests/unit/modules/products/presentation/components/product-translation-section.test.tsxtests/unit/modules/products/presentation/schemas/product-form-schema-photo-labels.test.tstests/unit/shared/hooks/use-unsaved-changes-guard.test.tsx
| const initialDraftSnapshot = { | ||
| text: initialDraft?.text ?? null, | ||
| color: initialDraft?.color ?? null, | ||
| size: initialDraft?.size ?? null, | ||
| imageUploadId: initialDraft?.imageUploadId ?? null, | ||
| imageUrl: initialDraft?.imageUrl ?? null, | ||
| designPosition: initialDraft?.designPosition ?? null, | ||
| error: null, | ||
| }; | ||
| const unsavedGuard = useUnsavedChangesGuard( | ||
| JSON.stringify(draft) !== JSON.stringify(initialDraftSnapshot), | ||
| { | ||
| title: labels.unsavedChangesTitle ?? 'Unsaved changes', | ||
| message: | ||
| labels.unsavedChangesMessage ?? | ||
| 'You have unsaved changes. Leave this page?', | ||
| leave: labels.unsavedChangesLeave ?? 'Leave', | ||
| stay: labels.unsavedChangesStay ?? 'Stay', | ||
| }, | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
El campo transitorio error en la comparación de cambios puede generar falsos positivos.
initialDraftSnapshot incluye error: null y la comparación JSON.stringify(draft) !== JSON.stringify(initialDraftSnapshot) abarca ese campo. Si validateDraft() se ejecuta (al enviar el formulario) y la validación falla, draft.error pasa a un valor no nulo aunque el usuario no haya modificado ningún campo, activando el guard de navegación sin cambios reales.
🔧 Propuesta de fix: normalizar `error` antes de comparar
const unsavedGuard = useUnsavedChangesGuard(
- JSON.stringify(draft) !== JSON.stringify(initialDraftSnapshot),
+ JSON.stringify({ ...draft, error: null }) !==
+ JSON.stringify(initialDraftSnapshot),
{📝 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 initialDraftSnapshot = { | |
| text: initialDraft?.text ?? null, | |
| color: initialDraft?.color ?? null, | |
| size: initialDraft?.size ?? null, | |
| imageUploadId: initialDraft?.imageUploadId ?? null, | |
| imageUrl: initialDraft?.imageUrl ?? null, | |
| designPosition: initialDraft?.designPosition ?? null, | |
| error: null, | |
| }; | |
| const unsavedGuard = useUnsavedChangesGuard( | |
| JSON.stringify(draft) !== JSON.stringify(initialDraftSnapshot), | |
| { | |
| title: labels.unsavedChangesTitle ?? 'Unsaved changes', | |
| message: | |
| labels.unsavedChangesMessage ?? | |
| 'You have unsaved changes. Leave this page?', | |
| leave: labels.unsavedChangesLeave ?? 'Leave', | |
| stay: labels.unsavedChangesStay ?? 'Stay', | |
| }, | |
| ); | |
| const initialDraftSnapshot = { | |
| text: initialDraft?.text ?? null, | |
| color: initialDraft?.color ?? null, | |
| size: initialDraft?.size ?? null, | |
| imageUploadId: initialDraft?.imageUploadId ?? null, | |
| imageUrl: initialDraft?.imageUrl ?? null, | |
| designPosition: initialDraft?.designPosition ?? null, | |
| error: null, | |
| }; | |
| const unsavedGuard = useUnsavedChangesGuard( | |
| JSON.stringify({ ...draft, error: null }) !== | |
| JSON.stringify(initialDraftSnapshot), | |
| { | |
| title: labels.unsavedChangesTitle ?? 'Unsaved changes', | |
| message: | |
| labels.unsavedChangesMessage ?? | |
| 'You have unsaved changes. Leave this page?', | |
| leave: labels.unsavedChangesLeave ?? 'Leave', | |
| stay: labels.unsavedChangesStay ?? 'Stay', | |
| }, | |
| ); |
🤖 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]/products/[id]/customization-experience.tsx around lines 145 -
164, Exclude the transient error field from the unsaved-changes comparison
around initialDraftSnapshot and useUnsavedChangesGuard. Normalize or remove
draft.error before comparing it with the initial snapshot so validation failures
do not trigger the guard when no editable fields changed.
| function addPhotoLabels( | ||
| translations: TranslationMap, | ||
| photoIds: readonly string[], | ||
| ): TranslationMap { | ||
| if (photoIds.length === 0) return translations; | ||
|
|
||
| return Object.fromEntries( | ||
| Object.entries(translations).map(([locale, translation]) => [ | ||
| locale, | ||
| { | ||
| ...translation, | ||
| photoLabels: { | ||
| ...translation.photoLabels, | ||
| ...Object.fromEntries(photoIds.map((photoId) => [photoId, ''])), | ||
| }, | ||
| }, | ||
| ]), | ||
| ) as TranslationMap; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether findTranslationWithMissingName considers photoLabels, and inspect the full schema used for the `translations` array field.
rg -n "findTranslationWithMissingName" -A 20 "app/[locale]/seller/products/product-form.tsx"
rg -n "productTranslationInputSchema|translations:" -A 5 "modules/products/presentation/schemas/product-form-schema.ts"Repository: henri318/728-store
Length of output: 2223
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the helpers and payload construction around the mentioned lines.
sed -n '300,520p' "app/[locale]/seller/products/product-form.tsx"
echo '--- mapErrors / schema area ---'
sed -n '1,220p' "modules/products/presentation/schemas/product-form-schema.ts"Repository: henri318/728-store
Length of output: 11049
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the error mapper to see how translation schema errors are surfaced.
rg -n "function mapErrors|const mapErrors|mapErrors\\(" "app/[locale]/seller/products/product-form.tsx" -A 120 -B 20Repository: henri318/728-store
Length of output: 6354
No cuentes photoLabels vacíos como contenido en buildPayload
Una traducción con solo photoLabels: { [id]: '' } entra en translations[] aunque name siga vacío; eso hace que productTranslationInputSchema rechace el payload y el error quede mal expuesto porque mapErrors solo mira issue.path[0]. Aplica la misma regla en findTranslationWithMissingName si ahí también debe considerarse vacía.
🤖 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]/seller/products/product-form.tsx around lines 351 - 369,
Actualiza buildPayload para que una traducción cuyo único contenido sea
photoLabels con valores vacíos no se incluya en translations[]. Considera
photoLabels como contenido solo cuando al menos una etiqueta tenga un valor no
vacío, manteniendo la lógica existente para name y otros campos. Aplica la misma
regla en findTranslationWithMissingName para que esas traducciones se consideren
vacías al validar nombres.
| const [form, setForm] = useState<FormState>(() => { | ||
| const images = normalizeInitialImages(labels, initialValues.images); | ||
| const validPhotoIds = photoIdsFor(images); | ||
| const translations = Object.fromEntries( | ||
| Object.entries(buildTranslationMap(initialLocale, initialValues)).map( | ||
| ([translationLocale, translation]) => [ | ||
| translationLocale, | ||
| { | ||
| ...translation, | ||
| photoLabels: cleanPhotoLabels( | ||
| translation.photoLabels, | ||
| validPhotoIds, | ||
| ), | ||
| }, | ||
| ], | ||
| ), | ||
| ) as TranslationMap; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Inconsistencia entre el estado inicial del formulario y el snapshot usado por el guard.
Al inicializar form, translations se limpia con cleanPhotoLabels(..., validPhotoIds) (Líneas 620-623), pero initialFormSnapshot usa buildTranslationMap(...) sin limpiar (Línea 652). Si existen photoLabels huérfanos en datos del servidor (referenciando fotos ya no presentes), el snapshot inicial y el estado inicial diferirán, provocando que el guard marque "cambios sin guardar" nada más cargar la página, sin que el usuario haya tocado nada.
Also applies to: 648-656
🤖 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]/seller/products/product-form.tsx around lines 611 - 627, Alinea
initialFormSnapshot con el estado inicial normalizado: en la construcción de
initialFormSnapshot, reutiliza la misma limpieza de translations mediante
cleanPhotoLabels y validPhotoIds aplicada al estado form dentro del
inicializador de useState. Mantén idénticos los valores normalizados para evitar
que photoLabels huérfanos activen el guard al cargar.
| const initialFormSnapshot = useMemo( | ||
| () => | ||
| JSON.stringify({ | ||
| price: String(initialValues.price), | ||
| translations: buildTranslationMap(initialLocale, initialValues), | ||
| customizationConfig: initialValues.customizationConfig, | ||
| }), | ||
| [initialLocale, initialValues], | ||
| ); | ||
| const unsavedGuard = useUnsavedChangesGuard( | ||
| mode === 'edit' && | ||
| JSON.stringify({ | ||
| price: form.price, | ||
| translations: form.translations, | ||
| customizationConfig: form.customizationConfig, | ||
| }) !== initialFormSnapshot, | ||
| labels.unsavedChanges ?? { | ||
| title: 'Unsaved changes', | ||
| message: 'You have unsaved changes. Leave this page?', | ||
| leave: 'Leave', | ||
| stay: 'Stay', | ||
| }, | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
El guard de cambios no guardados no detecta cambios en images.
initialFormSnapshot y la condición pasada a useUnsavedChangesGuard solo comparan {price, translations, customizationConfig}. Añadir/eliminar/reordenar fotos en cover o showcase, o reordenar customizableBase, no modifica translations, por lo que el usuario puede navegar fuera sin advertencia pese a tener cambios de fotos sin guardar.
💡 Propuesta de fix
const initialFormSnapshot = useMemo(
() =>
JSON.stringify({
price: String(initialValues.price),
translations: buildTranslationMap(initialLocale, initialValues),
customizationConfig: initialValues.customizationConfig,
+ images: normalizeInitialImages(labels, initialValues.images),
}),
[initialLocale, initialValues],
);
const unsavedGuard = useUnsavedChangesGuard(
mode === 'edit' &&
JSON.stringify({
price: form.price,
translations: form.translations,
customizationConfig: form.customizationConfig,
+ images: form.images,
}) !== initialFormSnapshot,📝 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 initialFormSnapshot = useMemo( | |
| () => | |
| JSON.stringify({ | |
| price: String(initialValues.price), | |
| translations: buildTranslationMap(initialLocale, initialValues), | |
| customizationConfig: initialValues.customizationConfig, | |
| }), | |
| [initialLocale, initialValues], | |
| ); | |
| const unsavedGuard = useUnsavedChangesGuard( | |
| mode === 'edit' && | |
| JSON.stringify({ | |
| price: form.price, | |
| translations: form.translations, | |
| customizationConfig: form.customizationConfig, | |
| }) !== initialFormSnapshot, | |
| labels.unsavedChanges ?? { | |
| title: 'Unsaved changes', | |
| message: 'You have unsaved changes. Leave this page?', | |
| leave: 'Leave', | |
| stay: 'Stay', | |
| }, | |
| ); | |
| const initialFormSnapshot = useMemo( | |
| () => | |
| JSON.stringify({ | |
| price: String(initialValues.price), | |
| translations: buildTranslationMap(initialLocale, initialValues), | |
| customizationConfig: initialValues.customizationConfig, | |
| images: normalizeInitialImages(labels, initialValues.images), | |
| }), | |
| [initialLocale, initialValues], | |
| ); | |
| const unsavedGuard = useUnsavedChangesGuard( | |
| mode === 'edit' && | |
| JSON.stringify({ | |
| price: form.price, | |
| translations: form.translations, | |
| customizationConfig: form.customizationConfig, | |
| images: form.images, | |
| }) !== initialFormSnapshot, | |
| labels.unsavedChanges ?? { | |
| title: 'Unsaved changes', | |
| message: 'You have unsaved changes. Leave this page?', | |
| leave: 'Leave', | |
| stay: 'Stay', | |
| }, | |
| ); |
🤖 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]/seller/products/product-form.tsx around lines 648 - 670,
Actualiza initialFormSnapshot y la comparación pasada a useUnsavedChangesGuard
para incluir el estado de images, cubriendo cover, showcase y customizableBase.
Usa la misma representación normalizada del orden y contenido de imágenes en
ambos snapshots para detectar adiciones, eliminaciones y reordenamientos,
manteniendo sin cambios la comparación existente de price, translations y
customizationConfig.
| const [name, setName] = useState(initialName); | ||
| const [description, setDescription] = useState(initialDescription); | ||
| const [loading, setLoading] = useState(false); | ||
| const [error, setError] = useState<string | null>(null); | ||
| const [saved, setSaved] = useState<string | null>(null); | ||
| const guard = useUnsavedChangesGuard( | ||
| saved === null && | ||
| (name !== initialName || description !== initialDescription), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
El guard deja de proteger tras el primer guardado exitoso, aunque haya nuevos cambios sin guardar.
La condición saved === null && (name !== initialName || description !== initialDescription) usa initialName/initialDescription (valores del montaje) y saved (que solo vuelve a null al iniciar un nuevo handleSubmit). Tras guardar una vez, saved queda con el mensaje de éxito; si el usuario sigue editando después de ese guardado y trata de navegar sin volver a enviar el formulario, saved !== null hace que la condición completa sea false y el guard no se activa, pese a existir cambios reales sin persistir.
🐛 Propuesta de fix
+ const [lastSaved, setLastSaved] = useState({
+ name: initialName,
+ description: initialDescription,
+ });
const guard = useUnsavedChangesGuard(
- saved === null &&
- (name !== initialName || description !== initialDescription),
+ name !== lastSaved.name || description !== lastSaved.description,
unsavedChangesLabels ?? {
title: 'Unsaved changes',
message: 'You have unsaved changes. Leave this page?',
leave: 'Leave',
stay: 'Stay',
},
);Y en handleSubmit, tras setSaved(savedLabel):
setSaved(savedLabel);
+ setLastSaved({ name: name.trim(), description: description.trim() });📝 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 [name, setName] = useState(initialName); | |
| const [description, setDescription] = useState(initialDescription); | |
| const [loading, setLoading] = useState(false); | |
| const [error, setError] = useState<string | null>(null); | |
| const [saved, setSaved] = useState<string | null>(null); | |
| const guard = useUnsavedChangesGuard( | |
| saved === null && | |
| (name !== initialName || description !== initialDescription), | |
| const [name, setName] = useState(initialName); | |
| const [description, setDescription] = useState(initialDescription); | |
| const [loading, setLoading] = useState(false); | |
| const [error, setError] = useState<string | null>(null); | |
| const [saved, setSaved] = useState<string | null>(null); | |
| const [lastSaved, setLastSaved] = useState({ | |
| name: initialName, | |
| description: initialDescription, | |
| }); | |
| const guard = useUnsavedChangesGuard( | |
| name !== lastSaved.name || description !== lastSaved.description, | |
| unsavedChangesLabels ?? { | |
| title: 'Unsaved changes', | |
| message: 'You have unsaved changes. Leave this page?', | |
| leave: 'Leave', | |
| stay: 'Stay', | |
| }, | |
| ); |
🤖 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/sellers/presentation/components/seller-detail-form.tsx` around lines
39 - 46, Actualiza el flujo de SellerDetailForm para que use como referencia los
últimos valores guardados y no el estado saved al activar
useUnsavedChangesGuard. Tras un guardado exitoso en handleSubmit, actualiza esa
referencia con name y description; así el guard permanece desactivado sin
cambios posteriores y vuelve a activarse cuando el usuario edite cualquiera de
esos campos.
| unsavedChangesLabels ?? { | ||
| title: 'Unsaved changes', | ||
| message: 'You have unsaved changes. Leave this page?', | ||
| leave: 'Leave', | ||
| stay: 'Stay', | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect the target component around the cited lines.
FILE="modules/sellers/presentation/components/seller-detail-form.tsx"
wc -l "$FILE"
sed -n '1,140p' "$FILE"
# 2) Inspect the caller mentioned in the comment.
PAGE="app/[locale]/admin/sellers/[sellerId]/page.tsx"
wc -l "$PAGE"
sed -n '1,140p' "$PAGE"
# 3) Find all references to unsavedChangesLabels to understand its shape and defaults.
rg -n "unsavedChangesLabels" .
# 4) If there is a translation/localization helper nearby, inspect related label composition.
rg -n "Unsaved changes|Leave this page|Leave|Stay" modules app .Repository: henri318/728-store
Length of output: 11400
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="shared/hooks/use-unsaved-changes-guard.tsx"
wc -l "$FILE"
sed -n '1,220p' "$FILE"
# Inspect the product customization pattern that already handles per-field fallbacks.
FILE2="app/[locale]/products/[id]/customization-experience.tsx"
wc -l "$FILE2"
sed -n '140,175p' "$FILE2"Repository: henri318/728-store
Length of output: 4081
Aplica ?? por campo en las etiquetas del guard
unsavedChangesLabels puede llegar con campos undefined, y useUnsavedChangesGuard los renderiza tal cual; el modal quedará con textos vacíos. El fallback actual solo cubre el objeto completo, así que conviene resolver cada propiedad con ?? antes de llamar al hook.
🤖 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/sellers/presentation/components/seller-detail-form.tsx` around lines
47 - 52, Update the unsavedChangesLabels configuration passed to
useUnsavedChangesGuard so each label property—title, message, leave, and
stay—uses its default value when undefined, rather than only defaulting the
entire object. Preserve caller-provided values for fields that are defined.
| const anchor = (event.target as Element | null)?.closest('a'); | ||
| if (!anchor || anchor.target === '_blank' || !anchor.href) return; | ||
| const destination = new URL(anchor.href, globalThis.location.href); | ||
| if (destination.origin !== globalThis.location.origin) return; | ||
| event.preventDefault(); | ||
| setPendingHref( | ||
| `${destination.pathname}${destination.search}${destination.hash}`, | ||
| ); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
El guard bloquea también la navegación por anclas internas (#hash).
destination.pathname/search/hash se comparan solo por origin, así que un enlace href="#section" (mismo pathname, sin recarga) también dispara preventDefault() y el modal de "cambios sin guardar", impidiendo el scroll normal a la sección incluso si el usuario decide "quedarse" (el preventDefault original nunca se revierte).
🩹 Propuesta de fix
const destination = new URL(anchor.href, globalThis.location.href);
if (destination.origin !== globalThis.location.origin) return;
+ const isSamePageHashLink =
+ destination.pathname === globalThis.location.pathname &&
+ destination.search === globalThis.location.search &&
+ destination.hash !== '';
+ if (isSamePageHashLink) return;
event.preventDefault();📝 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 anchor = (event.target as Element | null)?.closest('a'); | |
| if (!anchor || anchor.target === '_blank' || !anchor.href) return; | |
| const destination = new URL(anchor.href, globalThis.location.href); | |
| if (destination.origin !== globalThis.location.origin) return; | |
| event.preventDefault(); | |
| setPendingHref( | |
| `${destination.pathname}${destination.search}${destination.hash}`, | |
| ); | |
| }; | |
| const anchor = (event.target as Element | null)?.closest('a'); | |
| if (!anchor || anchor.target === '_blank' || !anchor.href) return; | |
| const destination = new URL(anchor.href, globalThis.location.href); | |
| if (destination.origin !== globalThis.location.origin) return; | |
| const isSamePageHashLink = | |
| destination.pathname === globalThis.location.pathname && | |
| destination.search === globalThis.location.search && | |
| destination.hash !== ''; | |
| if (isSamePageHashLink) return; | |
| event.preventDefault(); | |
| setPendingHref( | |
| `${destination.pathname}${destination.search}${destination.hash}`, | |
| ); | |
| }; |
🤖 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 `@shared/hooks/use-unsaved-changes-guard.tsx` around lines 38 - 46, Actualiza
el manejador de navegación para que los enlaces cuyo destino solo cambia el
hash, sin modificar pathname ni search respecto a la ubicación actual, se
ignoren antes de llamar a event.preventDefault() o setPendingHref. Mantén la
protección existente para navegaciones internas que cambien de página o
consulta.
1 participant
Summary by CodeRabbit
Nuevas funcionalidades
Correcciones
Pruebas