From ccd25f8c240e99add447cfedce308cf571cc1cef Mon Sep 17 00:00:00 2001 From: LauAubert Date: Fri, 2 Oct 2026 02:19:11 -0300 Subject: [PATCH] fix: [TESIS-999025] explain a rejected product save inside the edit form Any failure of the product save other than the 412 was drawn in the body of the detail page, which the backdrop of the open edit dialog covers. A 409 because a sale or transfer held the stock lock, a 422 or a network failure looked like a save that finished and did nothing. The edit modal now takes a submitError and shows it above the form, where the user is looking, with the typed data intact. The raw English message of the API is replaced by one per status: the stock is busy (409), some data was refused (422), or a generic retry. The 412 keeps its own conflict notice. Co-Authored-By: Claude Opus 5.5 --- .../EditProductModal/EditProductModal.tsx | 3 ++ .../EditProductModal.types.ts | 6 +++ src/features/inventory/content.ts | 12 ++++++ .../pages/ProductDetailPage.test.tsx | 42 ++++++++++++++++++- .../inventory/pages/ProductDetailPage.tsx | 30 ++++++++----- 5 files changed, 81 insertions(+), 12 deletions(-) diff --git a/src/features/inventory/components/EditProductModal/EditProductModal.tsx b/src/features/inventory/components/EditProductModal/EditProductModal.tsx index 60752d1..860104d 100644 --- a/src/features/inventory/components/EditProductModal/EditProductModal.tsx +++ b/src/features/inventory/components/EditProductModal/EditProductModal.tsx @@ -70,6 +70,7 @@ export function EditProductModal({ onClose, submitting = false, conflict, + submitError, }: EditProductModalProps) { const [menuAnchor, setMenuAnchor] = useState(null) @@ -132,6 +133,8 @@ export function EditProductModal({ {/* El conflicto va arriba del formulario y no reemplaza nada: lo que el usuario cargó sigue intacto abajo. */} + {submitError === undefined ? null : {submitError}} + {conflict === undefined ? null : ( {modal.conflict.title} diff --git a/src/features/inventory/components/EditProductModal/EditProductModal.types.ts b/src/features/inventory/components/EditProductModal/EditProductModal.types.ts index 46e5a57..1008544 100644 --- a/src/features/inventory/components/EditProductModal/EditProductModal.types.ts +++ b/src/features/inventory/components/EditProductModal/EditProductModal.types.ts @@ -24,6 +24,12 @@ export interface EditProductModalProps { * está— pero ahora pisa el trabajo de otra persona, y eso se nombra. */ conflict?: ConflictChange[] + /** + * Rechazo del guardado que no es el 412, ya traducido. Se muestra dentro del + * modal: afuera quedaba tapado por el fondo del diálogo, y el usuario veía + * que el guardado terminaba sin que pasara nada. + */ + submitError?: string } export interface SectionHeadingProps { diff --git a/src/features/inventory/content.ts b/src/features/inventory/content.ts index 9319f40..798a0fc 100644 --- a/src/features/inventory/content.ts +++ b/src/features/inventory/content.ts @@ -150,6 +150,18 @@ export const inventoryCopy = { title: (productName: string) => `Editar producto: ${productName}`, subtitle: (sku: string) => `Actualizá especificaciones y stock del SKU: ${sku}`, close: 'Cerrar', + /** + * Rechazos del guardado que no son el 412 (ese lo explica `conflict`). El + * mensaje de la API viene en inglés: se muestra uno propio por status. + */ + saveFailed: { + /** 409: el stock del producto está tomado por otra operación (una venta, una transferencia). */ + locked: + 'Otra operación está moviendo el stock de este producto. Tus cambios siguen acá: probá de nuevo en unos segundos.', + /** 422: la API rechazó algún dato del formulario. */ + invalid: 'La API rechazó alguno de los datos. Revisalos y volvé a guardar.', + generic: 'No pudimos guardar los cambios. Tus cambios siguen acá: probá de nuevo.', + }, conflict: { title: 'Alguien editó este producto mientras lo tenías abierto', body: 'Tus cambios siguen acá. Si guardás ahora, pisás lo que modificó la otra persona.', diff --git a/src/features/inventory/pages/ProductDetailPage.test.tsx b/src/features/inventory/pages/ProductDetailPage.test.tsx index 4ece89f..5c0cea9 100644 --- a/src/features/inventory/pages/ProductDetailPage.test.tsx +++ b/src/features/inventory/pages/ProductDetailPage.test.tsx @@ -1,4 +1,4 @@ -import { fireEvent, screen, waitFor } from '@testing-library/react' +import { fireEvent, screen, waitFor, within } from '@testing-library/react' import { MemoryRouter, Route, Routes, useLocation } from 'react-router-dom' import { beforeEach, describe, expect, it, vi } from 'vitest' @@ -104,4 +104,44 @@ describe('ProductDetailPage', () => { await waitFor(() => expect(editForm()).not.toBeInTheDocument()) }) + + // Hallazgo de auditoría (TESIS-89): el mensaje se dibujaba en el cuerpo de la + // página, tapado por el fondo del diálogo. El usuario veía que el guardado + // terminaba y nada más. + describe('a save that the API rejects', () => { + function failSave(status: number) { + vi.mocked(useUpdateProduct).mockReturnValue({ + mutate: vi.fn(), + reset: vi.fn(), + isPending: false, + isError: true, + error: Object.assign(new Error('raw message from the api'), { status }), + } as never) + } + + it('explains a 409 inside the edit form, where the user is looking', async () => { + failSave(409) + renderDetail({ edit: true }) + + const dialog = within(await screen.findByRole('dialog')) + expect(dialog.getByText(/Otra operación está moviendo el stock/)).toBeInTheDocument() + }) + + it('never shows the raw message of the API', async () => { + failSave(500) + renderDetail({ edit: true }) + + await screen.findByRole('dialog') + expect(screen.queryByText('raw message from the api')).not.toBeInTheDocument() + }) + + // El 412 lo explica el aviso de conflicto, no un error genérico. + it('leaves a 412 to the conflict notice', async () => { + failSave(412) + renderDetail({ edit: true }) + + await screen.findByRole('dialog') + expect(screen.queryByText(/No pudimos guardar los cambios/)).not.toBeInTheDocument() + }) + }) }) diff --git a/src/features/inventory/pages/ProductDetailPage.tsx b/src/features/inventory/pages/ProductDetailPage.tsx index fb3b4f2..f8d2907 100644 --- a/src/features/inventory/pages/ProductDetailPage.tsx +++ b/src/features/inventory/pages/ProductDetailPage.tsx @@ -31,6 +31,11 @@ const { specs: specsCopy, master: masterCopy, distribution: distributionCopy, st // del breadcrumb se declara acá. const CATALOG_PATH = '/inventory' +// Rechazos del guardado: el stock está tomado por otra operación (409, el lock +// de ADR-009) o algún dato no pasó la validación (422). +const LOCKED_STATUS = 409 +const INVALID_STATUS = 422 + // La grilla del diseño: columna fija para el stock maestro y el resto para la // distribución. En pantallas angostas se apilan. const CONTENT_GRID = { @@ -40,6 +45,13 @@ const CONTENT_GRID = { alignItems: 'stretch', } +/** El rechazo del guardado en palabras, por status. El 412 no pasa por acá. */ +function saveError(status: number | undefined): string { + if (status === LOCKED_STATUS) return inventoryCopy.modal.saveFailed.locked + if (status === INVALID_STATUS) return inventoryCopy.modal.saveFailed.invalid + return inventoryCopy.modal.saveFailed.generic +} + /** Cuadrícula de especificaciones. Lo que la API no expone se muestra sin dato. */ function buildSpecs(product: Product): ProductSpec[] { const dimensions = parseDimensions(product.dimensions) @@ -276,17 +288,6 @@ export function ProductDetailPage() { footnote={distributionCopy.pending} /> - - {/* El 412 no es un error a mostrar acá: lo explica el propio modal, que - queda abierto con lo que el usuario cargó. La condición mira - `isConflict` y no `conflict`, que es `undefined` también mientras se - resuelve el refetch: con lo otro, el 412 se filtraba a este banner - durante ese render. */} - {updateMutation.isError && !isConflict ? ( - - {updateMutation.error.message} - - ) : null} { // Sin el reset, el error de la mutación sobrevive al modal y queda // colgado en la página — un 412 que ya no aplica a nada visible.