diff --git a/src/features/orders/components/OrderEditForm/OrderEditForm.tsx b/src/features/orders/components/OrderEditForm/OrderEditForm.tsx index 379c1d8..a6d0f71 100644 --- a/src/features/orders/components/OrderEditForm/OrderEditForm.tsx +++ b/src/features/orders/components/OrderEditForm/OrderEditForm.tsx @@ -1,7 +1,7 @@ import { zodResolver } from '@hookform/resolvers/zod' import LocalShippingOutlinedIcon from '@mui/icons-material/LocalShippingOutlined' import { Alert, Box, Button, Stack } from '@mui/material' -import { useMemo, useState } from 'react' +import { useEffect, useMemo, useState } from 'react' import { useForm, useWatch } from 'react-hook-form' import { useNavigate } from 'react-router-dom' import { useDebouncedValue } from 'shared/hooks/useDebouncedValue' @@ -135,17 +135,29 @@ export function OrderEditForm({ errors: destinationErrors, } = destination.formState + // En cuanto se toca el domicilio pasa a validarse entero: sin esto, el modo + // `onChange` sólo marca el campo que se editó, y una ciudad o provincia que + // faltaban de antes apagaban el guardado sin decir por qué. + const { trigger: validateDestination } = destination + useEffect(() => { + if (destinationDirty) void validateDestination() + }, [destinationDirty, validateDestination]) + const locked = lockReason(order, shipment) const readOnly = locked !== null const removed = original.filter((line) => !lines.some((current) => current.key === line.key)) const shortfalls = stockShortfalls(lines, removed, stocks.stocks) const changed = linesChanged(original, lines) const dirty = changed || contextDirty || destinationDirty + // El domicilio se valida sólo si el operador lo tocó: las órdenes de webhook + // no traen ciudad ni provincia, y exigirlas dejaba el guardado apagado —sin + // ningún campo en rojo— aunque sólo se quisiera cambiar el estado (hallazgo + // de auditoría, TESIS-89). Sin tocarlo, tampoco viaja (`toUpdatePayload`). const canSave = !readOnly && dirty && contextValid && - destinationValid && + (destinationDirty ? destinationValid : true) && lines.length > 0 && lines.every((line) => isQuantityValid(line.quantity)) && shortfalls.length === 0 @@ -181,6 +193,7 @@ export function OrderEditForm({ { ...context.getValues(), ...destination.getValues() }, lines, changed, + destinationDirty, ) update.mutate(payload, { onSuccess: () => { diff --git a/src/features/orders/pages/OrderEditPage.test.tsx b/src/features/orders/pages/OrderEditPage.test.tsx index b9511dd..052d038 100644 --- a/src/features/orders/pages/OrderEditPage.test.tsx +++ b/src/features/orders/pages/OrderEditPage.test.tsx @@ -199,6 +199,48 @@ describe('OrderEditPage', { timeout: 15_000 }, () => { expect(lastPayload().order.customer_city).toBe('Buenos Aires') }) + // Hallazgo de auditoría (TESIS-89): las órdenes de webhook no traen ciudad ni + // provincia, y exigirlas dejaba el guardado apagado —sin ningún campo en + // rojo— aunque sólo se quisiera cambiar el estado. + describe('an order that came by webhook without city nor province', () => { + const fromWebhook = { ...ORDER, customerCity: null, customerProvince: null } + + async function markPaid() { + fireEvent.mouseDown(screen.getByRole('combobox', { name: 'Estado' })) + fireEvent.click(await screen.findByRole('option', { name: 'Pagada' })) + } + + it('can still be marked as paid', async () => { + mockAll({ order: fromWebhook }) + renderPage() + await markPaid() + + await waitFor(() => expect(saveButton()).toBeEnabled()) + }) + + // Vacía, la provincia no pasa la validación de la API. + it('does not send the address it did not touch', async () => { + mockAll({ order: fromWebhook }) + renderPage() + await markPaid() + await waitFor(() => expect(saveButton()).toBeEnabled()) + fireEvent.click(saveButton()) + + expect(lastPayload().order).not.toHaveProperty('customer_province') + }) + + it('asks for every missing field once the address is touched', async () => { + mockAll({ order: fromWebhook }) + renderPage() + fireEvent.change(screen.getByRole('textbox', { name: 'Calle y número' }), { + target: { value: 'Av. Siempreviva 742' }, + }) + + expect(await screen.findByText('Ingresá la ciudad.')).toBeInTheDocument() + expect(saveButton()).toBeDisabled() + }) + }) + // Criterio de la card: las validaciones bloquean el guardado. it('blocks saving with a missing customer', async () => { renderPage() diff --git a/src/features/orders/types.ts b/src/features/orders/types.ts index 9bab6f8..2e3be6e 100644 --- a/src/features/orders/types.ts +++ b/src/features/orders/types.ts @@ -231,10 +231,15 @@ export interface UpdateOrderPayload { order: { customer_name: string customer_document: string - customer_address: string - customer_city: string - customer_province: string - customer_zip_code: string + /** + * El domicilio viaja sólo si el operador lo tocó (ver `toUpdatePayload`): + * las órdenes de webhook no traen ciudad ni provincia, y mandarlas vacías + * haría que la API las rechace. + */ + customer_address?: string + customer_city?: string + customer_province?: string + customer_zip_code?: string status: EditableOrderStatus items?: ( | { id: number; quantity: number } diff --git a/src/features/orders/utils/edit.test.ts b/src/features/orders/utils/edit.test.ts index 68244ed..72bd41c 100644 --- a/src/features/orders/utils/edit.test.ts +++ b/src/features/orders/utils/edit.test.ts @@ -189,6 +189,12 @@ describe('toUpdatePayload', () => { ]) }) + it('leaves the address out when it was not touched', () => { + const payload = toUpdatePayload(HEADER, [line()], false, false) + + expect(Object.keys(payload.order)).toEqual(['customer_name', 'customer_document', 'status']) + }) + it('refuses to send a new line without a warehouse instead of inventing one', () => { expect(() => toUpdatePayload(HEADER, [added({ warehouseId: null })], true)).toThrow( /has no warehouse/, diff --git a/src/features/orders/utils/edit.ts b/src/features/orders/utils/edit.ts index 506d024..4fdea5d 100644 --- a/src/features/orders/utils/edit.ts +++ b/src/features/orders/utils/edit.ts @@ -211,19 +211,28 @@ export function editSubtotal(lines: EditLine[]): number { * el backend borra las que no vienen. Una existente manda su `id` y la cantidad * nueva —el precio facturado no se toca—; una nueva manda los cuatro datos del * alta. + * + * El domicilio, también sólo si cambió. Una orden que entró por webhook no trae + * ciudad ni provincia (el canal no las manda), y mandar esos campos vacíos haría + * que la API rechace la provincia: no se podría ni pasarla a «Pagada». */ export function toUpdatePayload( header: EditHeader, lines: EditLine[], includeLines: boolean, + includeDestination = true, ): UpdateOrderPayload { const order: UpdateOrderPayload['order'] = { customer_name: header.customerName.trim(), customer_document: header.customerDocument.trim(), - customer_address: header.address.trim(), - customer_city: header.city.trim(), - customer_province: header.province, - customer_zip_code: header.zipCode.trim(), + ...(includeDestination + ? { + customer_address: header.address.trim(), + customer_city: header.city.trim(), + customer_province: header.province, + customer_zip_code: header.zipCode.trim(), + } + : {}), status: header.status, }