Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export function EditProductModal({
onClose,
submitting = false,
conflict,
submitError,
}: EditProductModalProps) {
const [menuAnchor, setMenuAnchor] = useState<HTMLElement | null>(null)

Expand Down Expand Up @@ -132,6 +133,8 @@ export function EditProductModal({
<ModalBody>
{/* El conflicto va arriba del formulario y no reemplaza nada: lo que
el usuario cargó sigue intacto abajo. */}
{submitError === undefined ? null : <Alert severity="error">{submitError}</Alert>}

{conflict === undefined ? null : (
<Alert severity="warning" variant="outlined">
<AlertTitle>{modal.conflict.title}</AlertTitle>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
12 changes: 12 additions & 0 deletions src/features/inventory/content.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.',
Expand Down
42 changes: 41 additions & 1 deletion src/features/inventory/pages/ProductDetailPage.test.tsx
Original file line number Diff line number Diff line change
@@ -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'

Expand Down Expand Up @@ -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()
})
})
})
30 changes: 19 additions & 11 deletions src/features/inventory/pages/ProductDetailPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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)
Expand Down Expand Up @@ -276,17 +288,6 @@ export function ProductDetailPage() {
footnote={distributionCopy.pending}
/>
</Box>

{/* 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 ? (
<Typography variant="bodyMd" role="alert" sx={{ color: 'error.main' }}>
{updateMutation.error.message}
</Typography>
) : null}
</Stack>

<EditProductModal
Expand All @@ -295,6 +296,13 @@ export function ProductDetailPage() {
warehouses={warehouses.data}
submitting={updateMutation.isPending}
conflict={conflict}
// El 412 no va acá: lo explica `conflict`. 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 como error
// genérico durante ese render.
submitError={
updateMutation.isError && !isConflict ? saveError(updateMutation.error.status) : undefined
}
onClose={() => {
// 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.
Expand Down
Loading