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
52 changes: 26 additions & 26 deletions docs/guidelines/architecture.md

Large diffs are not rendered by default.

22 changes: 17 additions & 5 deletions src/features/orders/content.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,15 @@ export const ordersCopy = {
error: 'No pudimos cargar el envío de la orden.',
retry: 'Reintentar',
},
/** Abrir el envío de una orden que todavía no lo tiene (TESIS-141). */
openShipment: {
action: 'Crear envío',
creating: 'Creando…',
created: 'Envío creado. Ya se puede despachar.',
/** 409: lo abrió el asistente del alta, u otra pestaña. */
duplicated: 'La orden ya tenía un envío. Lo acabamos de traer.',
error: 'No pudimos crear el envío de la orden.',
},
/** Despachar desde el detalle un envío que quedó sin despachar (TESIS-134). */
dispatch: {
action: 'Despachar',
Expand Down Expand Up @@ -359,12 +368,15 @@ export const ordersCopy = {
/** La orden ya existe: lo que falló es el envío o el despacho. */
dispatch: (orderLabel: string) =>
`La orden ${orderLabel} se creó, pero no pudimos emitir el despacho.`,
/** El envío quedó `pending`: el detalle de la orden lo puede despachar (TESIS-134). */
/**
* Un solo mensaje para las dos fallas que dejan la orden creada: que no
* se haya abierto el envío y que no se haya podido despachar. Desde
* TESIS-141 las dos tienen la misma salida, porque el detalle de la orden
* abre el envío que falta (TESIS-141) y despacha el que quedó `pending`
* (TESIS-134). Antes eran dos textos porque sin envío no había a dónde ir.
*/
dispatchLater:
'Podés reintentarlo acá o, si salís de esta pantalla, despachar el envío desde el detalle de la orden.',
/** Ni siquiera se abrió el envío: ninguna otra pantalla lo abre, ésta es la única salida. */
shipmentMissing:
'Reintentalo antes de salir: la orden ya descontó el stock y todavía no tiene envío, y ninguna otra pantalla lo puede abrir.',
'Podés reintentarlo acá o, si salís de esta pantalla, terminar el envío desde el detalle de la orden.',
retryDispatch: 'Reintentar el despacho',
viewOrder: 'Ver la orden',
},
Expand Down
14 changes: 7 additions & 7 deletions src/features/orders/hooks/useConfirmDraftOrder.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,10 @@ describe('useConfirmDraftOrder', () => {
await waitFor(() => expect(result.current.createdOrderId).toBe(8829))
})

// Con el envío abierto, el detalle de la orden lo puede despachar (TESIS-134);
// sin él, no.
it('tells the page whether the shipment was opened before the dispatch failed', async () => {
vi.spyOn(api, 'createOrder').mockResolvedValue(ORDER)
// El envío que no llegó a abrirse se reintenta; la orden, que sí se creó, no
// se vuelve a crear: sería una segunda venta.
it('retries the shipment it could not open without creating the order twice', async () => {
const createOrder = vi.spyOn(api, 'createOrder').mockResolvedValue(ORDER)
const createShipment = vi
.spyOn(api, 'createOrderShipment')
.mockRejectedValueOnce(new Error('boom'))
Expand All @@ -111,11 +111,11 @@ describe('useConfirmDraftOrder', () => {
const { result } = renderHook(() => useConfirmDraftOrder(), { wrapper })

await act(() => result.current.mutateAsync(INPUT).catch(() => undefined))
expect(result.current.createdShipmentId).toBeNull()

await act(() => result.current.mutateAsync(INPUT).catch(() => undefined))
await waitFor(() => expect(result.current.createdShipmentId).toBe(31))

expect(createShipment).toHaveBeenCalledTimes(2)
expect(createOrder).toHaveBeenCalledTimes(1)
await waitFor(() => expect(result.current.createdOrderId).toBe(8829))
})

it('leaves no created order behind when the order itself is rejected', async () => {
Expand Down
14 changes: 6 additions & 8 deletions src/features/orders/hooks/useConfirmDraftOrder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ interface Options {
* volver a crearla sería una segunda venta. `createdOrderId` le dice a la
* pantalla si eso pasó.
*
* `createdShipmentId` le dice si el envío también llegó a abrirse: con envío
* `pending`, lo que falte se puede despachar desde el detalle de la orden
* (TESIS-134); sin envío, este paso sigue siendo el único lugar que lo abre.
* No hace falta distinguir hasta dónde llegó: con la orden creada, el detalle
* termina el envío de las dos maneras —lo abre si no llegó a abrirse (TESIS-141)
* y lo despacha si quedó `pending` (TESIS-134)—. `progress` sí lleva la cuenta,
* pero puertas adentro, para que el reintento no repita lo que ya salió bien.
*/
export function useConfirmDraftOrder({ onOrderCreated }: Options = {}) {
const queryClient = useQueryClient()
const progress = useRef<Progress>({ orderId: null, shipmentId: null })
const [createdOrderId, setCreatedOrderId] = useState<number | null>(null)
const [createdShipmentId, setCreatedShipmentId] = useState<number | null>(null)

const mutation = useMutation<number, ApiRequestError, ConfirmDraftOrderInput>({
mutationFn: async ({ order, dispatch }) => {
Expand All @@ -52,9 +52,7 @@ export function useConfirmDraftOrder({ onOrderCreated }: Options = {}) {
const orderId = progress.current.orderId

if (progress.current.shipmentId === null) {
const shipment = await createOrderShipment(orderId)
progress.current.shipmentId = shipment.id
setCreatedShipmentId(shipment.id)
progress.current.shipmentId = (await createOrderShipment(orderId)).id
}

await dispatchShipment(progress.current.shipmentId, dispatch)
Expand All @@ -74,5 +72,5 @@ export function useConfirmDraftOrder({ onOrderCreated }: Options = {}) {
},
})

return { ...mutation, createdOrderId, createdShipmentId }
return { ...mutation, createdOrderId }
}
73 changes: 73 additions & 0 deletions src/features/orders/hooks/useCreateOrderShipment.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
import { act, renderHook } from '@testing-library/react'
import type { ReactNode } from 'react'
import { beforeEach, describe, expect, it, vi } from 'vitest'

import * as api from '../api'
import { orderKeys } from '../queryKeys'
import type { Shipment } from '../types'

import { useCreateOrderShipment } from './useCreateOrderShipment'

const SHIPMENT = { id: 31, status: 'pending' } as Shipment

// El detalle de la orden y su envío, ya en caché. El envío resuelto en «no
// hay», que es el estado desde el que se ofrece abrirlo.
function seededClient() {
const queryClient = new QueryClient({ defaultOptions: { mutations: { retry: false } } })
queryClient.setQueryData(orderKeys.detail(8829), {})
queryClient.setQueryData(orderKeys.shipment(8829), { kind: 'none' })
return queryClient
}

function renderFor(queryClient: QueryClient) {
return renderHook(() => useCreateOrderShipment(), {
wrapper: ({ children }: { children: ReactNode }) => (
<QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
),
})
}

beforeEach(() => {
vi.restoreAllMocks()
})

describe('useCreateOrderShipment', () => {
it('opens the shipment of the order', async () => {
const create = vi.spyOn(api, 'createOrderShipment').mockResolvedValue(SHIPMENT)
const { result } = renderFor(seededClient())

await act(() => result.current.mutateAsync(8829))

expect(create).toHaveBeenCalledWith(8829)
})

// Criterio de la card: al terminar, la tarjeta muestra el envío `pending` y
// con él aparece «Despachar».
it('refreshes the order and its shipment', async () => {
vi.spyOn(api, 'createOrderShipment').mockResolvedValue(SHIPMENT)
const queryClient = seededClient()
const { result } = renderFor(queryClient)

await act(() => result.current.mutateAsync(8829))

expect(queryClient.getQueryState(orderKeys.detail(8829))?.isInvalidated).toBe(true)
expect(queryClient.getQueryState(orderKeys.shipment(8829))?.isInvalidated).toBe(true)
})

// Un 409 es que el envío ya existe —lo abrió el asistente del alta, u otra
// pestaña—: lo que corresponde es traer el que está, no insistir.
it('refreshes the shipment also when the order already had one', async () => {
vi.spyOn(api, 'createOrderShipment').mockRejectedValue(
Object.assign(new Error('Order already has a shipment'), { status: 409 }),
)
const queryClient = seededClient()
const { result } = renderFor(queryClient)

await act(async () => {
await result.current.mutateAsync(8829).catch(() => undefined)
})

expect(queryClient.getQueryState(orderKeys.shipment(8829))?.isInvalidated).toBe(true)
})
})
24 changes: 24 additions & 0 deletions src/features/orders/hooks/useCreateOrderShipment.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { useMutation, useQueryClient } from '@tanstack/react-query'
import type { ApiRequestError } from 'shared/api/types'

import { createOrderShipment } from '../api'
import { orderKeys } from '../queryKeys'
import type { Shipment } from '../types'

/**
* Abre el envío de una orden desde su detalle (TESIS-141): `POST
* /orders/:id/shipment` (TESIS-105). Nace `pending` y sin courier; elegirlo es
* la otra acción, la de despachar (TESIS-134).
*
* `onSettled` y no `onSuccess`: un 409 quiere decir que el envío ya existe
* —otra pestaña, o el asistente del alta— y lo que corresponde ahí es traer el
* que ya está, no insistir. El detalle se refresca en los dos casos.
*/
export function useCreateOrderShipment() {
const queryClient = useQueryClient()

return useMutation<Shipment, ApiRequestError, number>({
mutationFn: (orderId) => createOrderShipment(orderId),
onSettled: () => queryClient.invalidateQueries({ queryKey: orderKeys.all }),
})
}
51 changes: 13 additions & 38 deletions src/features/orders/pages/CarrierStepPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,6 @@ function renderPage() {
const option = (name: RegExp) => screen.getByRole('radio', { name })
const confirmButton = () => screen.getByRole('button', { name: /Confirmar orden/ })

// Lo que hace el navegador al recargar: si algún listener cancela el evento,
// pregunta antes de salir.
function leaveIsBlocked() {
const event = new Event('beforeunload', { cancelable: true })
window.dispatchEvent(event)
return event.defaultPrevented
}
const total = () => screen.getByLabelText('Total final')

function stubConfirmation() {
Expand Down Expand Up @@ -327,21 +320,10 @@ describe('CarrierStepPage', () => {
fireEvent.click(confirmButton())

expect(
await screen.findByText(/despachar el envío desde el detalle de la orden/),
await screen.findByText(/terminar el envío desde el detalle de la orden/),
).toBeInTheDocument()
})

it('does not ask before reloading or closing the tab', async () => {
failDispatchOnce()
renderPage()

fireEvent.click(option(/Andreani/))
fireEvent.click(confirmButton())
await screen.findByText(/se creó, pero/)

expect(leaveIsBlocked()).toBe(false)
})

it('no longer offers going back to a draft that is already an order', async () => {
failDispatchOnce()
renderPage()
Expand All @@ -366,8 +348,10 @@ describe('CarrierStepPage', () => {
})
})

// Si falla la apertura del envío, la orden existe y no tiene envío: el
// detalle no lo puede despachar, y ninguna otra pantalla lo abre.
// Si falla la apertura del envío, la orden existe y todavía no tiene envío.
// Desde TESIS-141 el detalle de la orden lo abre, así que la salida es la
// misma que cuando el envío existe y lo que falló fue el despacho: por eso no
// hay un mensaje aparte ni un aviso al salir de la pantalla.
describe('when the shipment could not even be opened', () => {
function failShipmentOnce() {
const calls = stubConfirmation()
Expand All @@ -377,31 +361,21 @@ describe('CarrierStepPage', () => {
return calls
}

it('says no other screen can open it', async () => {
it('points at the order detail, the same way a failed dispatch does', async () => {
failShipmentOnce()
renderPage()

fireEvent.click(option(/Andreani/))
fireEvent.click(confirmButton())

expect(
await screen.findByText(/todavía no tiene envío, y ninguna otra pantalla lo puede abrir/),
await screen.findByText(/terminar el envío desde el detalle de la orden/),
).toBeInTheDocument()
})

it('asks before reloading or closing the tab', async () => {
failShipmentOnce()
renderPage()
expect(leaveIsBlocked()).toBe(false)

fireEvent.click(option(/Andreani/))
fireEvent.click(confirmButton())
await screen.findByText(/se creó, pero/)

expect(leaveIsBlocked()).toBe(true)
})

it('stops asking once the retry opens the shipment, even if the dispatch fails', async () => {
// El envío que no llegó a abrirse se reintenta; la orden, que sí se creó, no
// se vuelve a crear.
it('opens the shipment on the retry, without creating the order again', async () => {
const calls = failShipmentOnce()
calls.dispatch.mockReset()
calls.dispatch.mockRejectedValue(new Error('El courier no contestó.'))
Expand All @@ -411,8 +385,9 @@ describe('CarrierStepPage', () => {
fireEvent.click(confirmButton())
fireEvent.click(await screen.findByRole('button', { name: 'Reintentar el despacho' }))

await screen.findByText(/despachar el envío desde el detalle de la orden/)
expect(leaveIsBlocked()).toBe(false)
await screen.findByText(/terminar el envío desde el detalle de la orden/)
expect(calls.createShipment).toHaveBeenCalledTimes(2)
expect(calls.createOrder).toHaveBeenCalledTimes(1)
})
})

Expand Down
32 changes: 5 additions & 27 deletions src/features/orders/pages/CarrierStepPage.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import LocalShippingOutlinedIcon from '@mui/icons-material/LocalShippingOutlined'
import { Alert, Box, Button, Stack } from '@mui/material'
import { useEffect, useMemo, useState } from 'react'
import { useMemo, useState } from 'react'
import { Navigate, useNavigate } from 'react-router-dom'
import { PageWrapper } from 'shared/components'
import { notify, useOrderDraftStore } from 'shared/store'
Expand Down Expand Up @@ -89,9 +89,6 @@ export function CarrierStepPage() {
)
const quotes = useDraftQuotes(payload)
const confirm = useConfirmDraftOrder({ onOrderCreated: clearDraft })
const shipmentMissing =
confirm.createdOrderId !== null && confirm.createdShipmentId === null && !confirm.isSuccess
useLeaveWarning(shipmentMissing)

// Sin cliente o sin líneas no hay orden que cotizar; sin origen o destino hay
// que volver al paso 2. Se llegó por URL, recargando, o se canceló el
Expand Down Expand Up @@ -134,13 +131,10 @@ export function CarrierStepPage() {
{createdOrderId === null ? carrier.errors.order : carrier.errors.dispatch(orderLabel)}{' '}
{confirm.error.message}
</span>
{createdOrderId === null ? null : (
<span>
{confirm.createdShipmentId === null
? carrier.errors.shipmentMissing
: carrier.errors.dispatchLater}
</span>
)}
{/* Un solo mensaje: con la orden ya creada, el detalle termina el
envío tanto si no llegó a abrirse (TESIS-141) como si quedó
`pending` sin despachar (TESIS-134). */}
{createdOrderId === null ? null : <span>{carrier.errors.dispatchLater}</span>}
{createdOrderId === null ? null : (
<Button
color="inherit"
Expand Down Expand Up @@ -205,19 +199,3 @@ export function CarrierStepPage() {
</PageWrapper>
)
}

/**
* Pide confirmación al recargar o cerrar la pestaña mientras `active`. Es lo que
* el navegador permite: el texto del diálogo es el suyo, no uno propio.
*/
function useLeaveWarning(active: boolean) {
useEffect(() => {
if (!active) return undefined

const warn = (event: BeforeUnloadEvent) => {
event.preventDefault()
}
window.addEventListener('beforeunload', warn)
return () => window.removeEventListener('beforeunload', warn)
}, [active])
}
Loading
Loading