feat: [TESIS-59] add manual order wizard step 3: carrier and confirmation - #52
Conversation
The boundary for step 3 of the manual order wizard: quote the draft before the order exists (POST /quotes, TESIS-131), create the order, open its shipment and dispatch it with the chosen option. useConfirmDraftOrder chains the last three. They cannot be one atomic request because the dispatch calls an external courier, so a retry after a failed dispatch resumes where it stopped instead of creating the order (and deducting the stock) a second time. toQuotePayload goes away: it quoted an order that already existed, which is the flow this card replaces. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Step 3 lists the quoted carriers as the same kind of selectable card as the origin warehouses of step 2: primary border and container when chosen, in both modes. SelectableCard moves out of the picker now that it has a second consumer, not before (feature-structure.md §6). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tion /orders/new/carrier (S07) quotes the draft as soon as it opens, lists the options with their price and delivery time, adds the chosen one to the final total and confirms: create the order, open its shipment and dispatch it with the chosen carrier, then back to the listing. - The quote shows a loading state while the couriers answer, tells an empty answer (no carrier made it in time) apart from a failure, and offers both to quote again and to go back to origin and destination. - The cheapest option, which comes first, is flagged. S07 also draws "Alta confiabilidad" and "Seguimiento incluido", but the API carries neither, so they are left out. - The draft is emptied as soon as the order exists, so a reload after a failed dispatch cannot create the same sale twice. The screen keeps showing a copy of the draft taken when confirming, and the button becomes "Reintentar el despacho", which resumes from the dispatch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The orders row gains the third step, utils/shipping.ts lists the builders of the quote, the order and the dispatch, and the table gets the new components and hooks with the why of each: the quote key does not hang from `orders`, and the confirmation resumes instead of creating the order again. orderDraftStore says when the draft is emptied on confirm. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TomasMartin2004
left a comment
There was a problem hiding this comment.
Revisión — TESIS-59 (PR #52) · Paso 3 del alta manual
Revisado sobre TESIS-59-manual-order-wizard-step-3 (722298c de base) contra origin/master, con S07, la card y el PR de backend (api#90) al lado.
| Check | Resultado |
|---|---|
npm run test (local) |
572 tests, 0 fallas |
npm run lint · prettier --check . · npm run build |
Limpios |
| Dependencia | api#90 (TESIS-131). Sin él, POST /quotes es 404 |
✅ Lo que más me importaba: la confirmación no puede crear dos ventas
Es la parte peligrosa del paso —tres requests que no pueden ser atómicos porque el tercero llama a un courier— y está bien resuelta. El progress en un ref hace que el reintento retome desde donde quedó, y la pantalla cambia de discurso: «Reintentar el despacho», sin «Paso anterior», con «Ver la orden».
Tiene dientes. Forcé que el reintento vuelva a crear la orden y caen dos ejemplos, uno del hook y uno de la pantalla:
× retries from where it failed, without creating the order again
× retries only the dispatch
Y el hallazgo de las cotizaciones también. Colgué quoteKeys de orderKeys otra vez y se pone en rojo el ejemplo que lo cubre:
× refreshes the orders without asking the carriers for a new quote
Que lo hayas encontrado mirando los logs de la API y no adivinando es la diferencia entre invalidar «por las dudas» y saber qué se invalida: confirmar disparaba una tanda nueva de llamadas a todos los couriers justo después de crear la orden.
✅ El resto
- Se cotiza qué lleva el paquete, no cuánto pesa. Mantiene la regla del backend y evita que el front sea la fuente de un dato que no le corresponde.
- Despachar con
dispatchIntegrationIdy no con la integración que contestó la tarifa: es exactamente el hueco que TESIS-131 vino a tapar, y acá se usa como corresponde. - La opción elegida sólo cuenta si sigue entre las cotizadas. Volver a cotizar puede traer otra lista, y sin eso el despacho saldría con un id que ya no está en pantalla.
- Los dos estados vacíos dicen cosas distintas («No pudimos cotizar» contra «Ningún operador pudo cotizar»). Es la distinción que el paso 2 también hace, y la que evita que el operador crea que el problema es suyo.
- Los badges que S07 dibuja y la API no informa no se muestran. Preferible a inventarlos.
🔴 Si el despacho falla y la persona recarga, la orden queda sin salida por la interfaz
El borrador se vacía apenas existe la orden (onOrderCreated: clearDraft), que es correcto para no crear la venta dos veces. Pero deja este camino abierto:
- Confirmar. La orden se crea, el stock se descuenta, el despacho falla (el courier está caído, que es justo cuando pasa).
- La persona recarga, o cierra la pestaña y vuelve — impaciencia razonable en ese momento.
draft === null, así queCarrierStepPageredirige al paso 1.
Y no hay otra pantalla para retomar: dispatchShipment sólo se usa acá.
$ grep -rl "dispatchShipment" src/
src/features/orders/api.ts
src/features/orders/api.test.ts
src/features/orders/hooks/useConfirmDraftOrder.ts
src/features/orders/hooks/useConfirmDraftOrder.test.tsx
src/features/orders/pages/CarrierStepPage.test.tsx
La orden queda creada, con stock descontado, sin envío despachado, y sólo se puede completar por API o backoffice. Para la demo es un final feo: el courier caído es el escenario que uno mismo provoca para mostrar el reintento.
No pido resolverlo acá si preferís que sea otra card —es de la pantalla de detalle, no de ésta—, pero algo hay que hacer con esas órdenes:
- Lo más barato: que el detalle de la orden ofrezca «Despachar» cuando tiene envío sin despachar. Ya tiene el ciclo de vida dibujado (TESIS-60).
- Alternativa dentro de este PR: vaciar el borrador recién cuando el despacho sale bien, y aceptar el riesgo de duplicar si la persona recarga entre medio — que me parece peor.
Como mínimo, que el mensaje de error diga qué hacer si se pierde la pantalla («la orden #N quedó creada; podés despacharla desde su detalle»), aunque el botón llegue en otra card.
🟡 Menor
shipping_cost llega como string y se convierte en la frontera, que está bien. Verificá que el redondeo no muerda cuando el courier devuelva centavos: el resumen suma totalWithShipping y el backend guarda el BigDecimal. Con los valores de la demo no se nota, pero un 41200.5 sumado en punto flotante puede mostrar un total que no cierra con el del detalle.
Veredicto
REQUEST CHANGES, sólo por lo de la orden sin salida. Si preferís sacarlo a una card aparte y dejar acá el texto que le diga a la persona dónde quedó la orden, con eso me alcanza y lo apruebo.
El resto del paso está muy bien: la parte riesgosa es la que mejor está tratada, y los dos hallazgos que trajiste de la prueba en el navegador —el re-cotizar al confirmar, y el 422 al despachar con la integración equivocada— son de los que no aparecen leyendo el código.
🤖 Generated with Claude Code
Resolves the conflict with TESIS-104: the selectable card of the origin picker now lives in SelectableCard, and the styles added by this branch read their colors from theme.vars like the rest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A courier can answer with more than two decimals. The screen rounded the binary approximation (1.005 showed as 1.00) while the shipment keeps the decimal rounded half up (1.01), so the total of step 3 and the one of the order detail could differ by a cent. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Once the order exists the draft is gone, so after a failed dispatch step 3 is the only place its shipment can be dispatched from. The error now says so, and the browser asks before reloading or closing the tab while the dispatch is pending. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Respuesta — TESIS-59 (review de @TomasMartin2004, 25/sep)Commits nuevos en
🔴 La orden sin salida: mitigada acá, y la salida real en TESIS-134Tomé el camino que ofreciste: el botón en el detalle va en una card aparte, y acá la pantalla dice dónde queda la orden. Le sumé algo más, porque el caso que describiste empieza con un reload.
Dos ejemplos nuevos en 🟡 El redondeo: sí mordíaLo verifiqué y tenías razón, aunque no con
Ahora El merge con
|
TomasMartin2004
left a comment
There was a problem hiding this comment.
Segunda vuelta — TESIS-59 (PR #52)
Verificado sobre la rama actualizada. npm run test → 599 tests, 0 fallas; lint, prettier y build limpios.
🔴 La orden sin salida
La mitigación es la correcta y prefiero cómo lo resolviste a lo que yo había sugerido: no prometer lo que todavía no existe. Decir «podés despacharla desde su detalle» cuando ese botón no está sería peor que no decir nada.
El aviso antes de salir es el agregado que faltaba, porque mi caso empezaba justamente con un reload. Tiene dientes: desactivé el efecto y cae el ejemplo que lo cubre:
× asks before reloading or closing the tab
Con la card nueva para el botón en el detalle, el camino queda cerrado de punta a punta.
🟡 El redondeo mordía más de lo que yo pensaba
Tenías razón en la corrección: 41200.5 es exacto en binario y no falla. El caso real es el de más de dos decimales. Lo comparé con las dos implementaciones:
"1.005" → anterior 1 nuevo 1.01 ← difieren
"2.675" → anterior 2.68 nuevo 2.68
"1234.565" → anterior 1234.57 nuevo 1234.57
"0.005" → anterior 0.01 nuevo 0.01
Con "1.005" la pantalla mostraba 1,00 y el envío guardaba 1,01, porque Math.round(1.005 * 100) opera sobre 1.00499… Correr la coma en el texto redondea el decimal que mandó el backend y no su aproximación binaria, así que lo que se muestra, lo que suma el total y lo que viaja al despacho son el mismo número que queda en la base.
✅ El merge con master
Verifiqué lo que decís de los estilos: cero lecturas directas de theme.palette en SelectableCard, QuoteOptionList y OrderConfirmCard. Haber aprovechado el merge para pasarlos a theme.vars evita que estos tres queden como los únicos que no repintan con el toggle, que es lo que TESIS-104 vino a arreglar.
Veredicto
APPROVE.
El paso queda cerrado y el único camino que dejaba una orden a medias ahora avisa antes de que ocurra. Orden de merge: api#90 primero —ya lo aprobé— y después éste.
🤖 Generated with Claude Code
🔗 Link
Este paso usa tres cosas que agrega ese PR: cotizar el borrador sin crear la orden (
POST /quotes), eldispatch_integration_idde cada opción y elshipping_costen el despacho. Orden de merge: api#90 primero, éste después. Contra el master actual de la API, la cotización responde 404.📝 Descripción
Tercer y último paso del asistente de alta manual (S07): cotizar el envío, elegir el operador y confirmar. Es la ruta a la que ya apuntaba «Siguiente» del paso 2 (
/orders/new/carrier).De dónde sale la card de backend. La card avisaba que «la cotización se pide sobre una orden que ya existe». Al cruzarla con la API aparecieron tres huecos, no uno: cotizar exigía crear la orden (y descontar stock) antes de que el operador confirmara; la opción cotizada no se podía despachar, porque la cotización devolvía la integración de la plantilla que cotiza y el despacho exige la que despacha (lo verifiqué contra master: 422); y el costo elegido no se guardaba. Los tres se resolvieron en TESIS-131 (ADR-016 del backend), y este paso sigue S07 tal como está dibujado.
Decisiones:
useConfirmDraftOrder). No pueden ser un solo request atómico, porque el despacho llama a un courier externo. Si el despacho falla, la orden ya existe y ya descontó el stock, así que el reintento retoma desde el despacho y no crea otra orden. El botón pasa a «Reintentar el despacho», «Paso anterior» desaparece (ya no hay borrador al que volver) y la alerta ofrece «Ver la orden».dispatchIntegrationId, no con la integración que contestó la tarifa, y viaja el costo elegido para que quede en el envío. El detalle de la orden ya lo muestra.orderKeys(quoteKeyspropio). Confirmar invalida las órdenes, y con la clave colgada de ahí se volvía a cotizar con todos los couriers justo después de crear la orden. Lo encontré en los logs de la API durante la prueba en el navegador; hay un test que lo fija.🛠️ Cambios realizados
features/orders/pages/CarrierStepPage.tsx: paso 3 en/orders/new/carrier. Sin cliente o sin líneas redirige al paso 1; sin origen o destino, al paso 2.features/orders/hooks/useDraftQuotes.tsyuseConfirmDraftOrder.ts: la cotización y la confirmación que retoma.features/orders/api.ts+types.ts+queryKeys.ts:quoteDraft,createOrder,createOrderShipment,dispatchShipment, el tipoShippingQuoteyquoteKeys.shipping_costllega como string (BigDecimal de Rails) y se convierte en la frontera.features/orders/utils/shipping.ts:toDraftQuotePayload,toDispatchPayloadytotalWithShipping. SaletoQuotePayload, que cotizaba una orden ya creada.features/orders/components/QuoteOptionList/yOrderConfirmCard/: las opciones y el resumen de S07.features/orders/components/SelectableCard/: la tarjeta elegible del paso 2, extraída al aparecer el segundo consumidor (commit de refactor aparte).app/router/routes.tsx: ruta/orders/new/carrier.docs/guidelines/architecture.md: piezas nuevas deorders, y cuándo se vacía el borrador al confirmar.api.test.ts), los armadores (utils/shipping.test.ts), la confirmación (useConfirmDraftOrder.test.tsx) yCarrierStepPage.test.tsxcon los criterios de la card.🧪 Cómo probarlo (Opcional)
Precondiciones: backend con api#90 en
localhost:3000, con los seeds del tenantnorte(vinculan «Andreani» con «Andreani - Cotización»), y sesión iniciada. Para que los couriers contesten en local hace falta apuntar sus plantillas a un courier simulado; yo usé uno en Node que responde lo mínimo que mapean las plantillas.Caso 1: cotizar, elegir y confirmar
Caso 2: el despacho falla
POST /orders.Caso 3: sin cotizaciones
📸 Evidencia (Opcional)
Probado en el navegador contra la API de api#90, con una base propia de revisión (no la de desarrollo) y un courier simulado con tres operadores: Andreani y Correo Argentino cotizan y despachan; OCASA responde 500 al cotizar.
AND-DEMO-1, el evento «Etiqueta generada» y envío $ 58.300 en el resumen de pago.POST /shipments/8/dispatchy la orden feat: [TESIS-10] use CODEOWNERS for reviewer assignment #12 quedó despachada. El listado pasó de 8 a 9 órdenes: una sola./orders/new/carriercon una opción elegida, en oscuro y en claro)Verificación:
npm run test(60 archivos, 572 tests, 0 fallas),npm run lint,prettier --check .ynpm run buildlimpios.Dientes. Rompí cada decisión de a una y en todos los casos se puso en rojo el test que la cubre:
retries from where it failed, without creating the order againcreates the order, opens its shipment and dispatches it…ordersrefreshes the orders without asking the carriers for a new quote¿Afecta la arquitectura o genera un nuevo patrón?
No. Sigue ADR-002/003 y el
orderDraftStorede los pasos 1 y 2.SelectableCardse extrajo por la Regla de Dos.docs/guidelines/architecture.mdquedó actualizado.Relación con otros PRs:
ordersenarchitecture.mdy el final deapi.test.ts, donde los dos PRs agregan sudescribey comparten las llaves de cierre. Resuelto así, la combinación pasa: 585 tests y el build limpio.🤖 Generated with Claude Code