Skip to content
Merged
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
17 changes: 15 additions & 2 deletions src/features/orders/components/OrderEditForm/OrderEditForm.tsx
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -181,6 +193,7 @@ export function OrderEditForm({
{ ...context.getValues(), ...destination.getValues() },
lines,
changed,
destinationDirty,
)
update.mutate(payload, {
onSuccess: () => {
Expand Down
42 changes: 42 additions & 0 deletions src/features/orders/pages/OrderEditPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
13 changes: 9 additions & 4 deletions src/features/orders/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
6 changes: 6 additions & 0 deletions src/features/orders/utils/edit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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/,
Expand Down
17 changes: 13 additions & 4 deletions src/features/orders/utils/edit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down
Loading