fix: [TESIS-136] refuse to dispatch the shipment of a cancelled order

[TomasMartin2004]

Ticket de Jira

https://proyectofinalfrlp.atlassian.net/browse/TESIS-136

---

Descripción

Sale de la review de TESIS-134 (proyecto-web#56), donde lo marcó Santiago.

Shipments::CreateShipment se niega a abrir el envío de una orden cancelada. Shipments::ConfirmDispatch no tenía el chequeo equivalente: valida el estado del envío y nada más, así que un envío abierto antes de cancelar la orden se podía despachar igual por la API, pagándole al courier la etiqueta de una venta que no sale. El front de TESIS-134 no ofrece la acción, pero el endpoint quedaba abierto para cualquier cliente.

Dos decisiones que conviene mirar:

422 y no el 409 que decía la card. Yo escribí esa card, y al implementarla me quedó claro que 409 estaba mal. El 409 del despacho es el conflicto de estado del envío —eso es AlreadyDispatchedError— y el frontend lo lee exactamente así: el diálogo de TESIS-134 hace dispatch.error?.status === 409 y muestra «el envío ya se despachó mientras tanto», que acá sería falso. Con 422 cae en el camino genérico y muestra el mensaje del servidor, que es el correcto. De paso, abrir el envío de una orden cancelada ya responde 422 con el mismo UnshippableOrderError y el mismo «cannot be shipped»: el mismo hecho de negocio ahora contesta lo mismo en los dos endpoints. Actualicé la card.

El chequeo corre sólo antes de llamar al courier, a diferencia del estado del envío, que además se revalida bajo el lock!. Si la orden se cancela mientras el courier contesta, la etiqueta ya se emitió: descartar el número de seguimiento perdería el rastro de un paquete que el courier ya conoce. Se guarda, y la cancelación se resuelve por donde corresponda. Está anotado en el código.

La lista de estados es CreateShipment::NON_SHIPPABLE_STATUSES, no una copia: es la misma regla, y cuando exista la máquina de estados hay un solo lugar que tocar.

• ConfirmDispatch#validate_order!, primero de las validaciones.
• Specs del PORO: rechazo, que no se llama al courier, que el envío queda intacto, que no se escribe en la bitácora, y el control negativo con una orden paid.
• Specs de request: 422, el mensaje, que no se llama al courier y que el envío sigue pending y sin tracking.

---

Evidencia visual

N/A

---

Cómo probar

1. Crear una orden, abrir su envío (POST /orders/:id/shipment) y cancelar la orden (desde Avo o un webhook: la API no expone la transición).
2. POST /api/v1/shipments/:id/dispatch con una integración de despacho válida → 422 {"error": "an order in status 'cancelled' cannot be shipped"}. En master responde 201 y emite la etiqueta.
3. El envío sigue pending, sin tracking_number y sin evento nuevo en la bitácora.
4. La misma orden en pending o paid despacha igual que siempre.

Verificación: bundle exec rspec (1555 ejemplos, 0 fallas, cobertura 99.89% línea / 99.81% rama), rubocop y brakeman limpios.

Dientes. Saqué validate_order! de call y se pusieron en rojo los 8 tests nuevos —los 4 del PORO y los 4 de request—, ninguno de los viejos. La regla no estaba cubierta por accidente en otro lado.

---

Impacto y consideraciones

¿Introduce breaking changes?
Sí, acotado: despachar el envío de una orden cancelada pasa de 201 a 422. Es el bug. Ningún cliente del front hace ese request (TESIS-134 no ofrece la acción en una orden cancelada).

¿Requiere nuevas variables de entorno?
No

¿Afecta la arquitectura o genera un nuevo patrón?
No. Una validación más en un PORO que ya tiene cuatro, reusando el error y la constante que ya existían.

🤖 Generated with Claude Code

https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp

[Sanntinat]

Revisión — TESIS-136 (PR #94) · Rechazar el despacho del envío de una orden cancelada

Revisión de TESIS-136-refuse-dispatch-of-cancelled-order (1e4330d), contra origin/master de proyecto-api.

Check	Resultado
CI (GitHub Actions)	lint, scan_ruby, test y validate-pr-title en verde
Rama vs master	Al día. Mergea limpio
Suite local (sobre #93 + #94 mergeados)	1567 ejemplos · 0 fallas · cobertura 99,89 % línea / 99,81 % rama
RuboCop / Brakeman	281 archivos sin ofensas · 0 advertencias
Conflictos con otros PRs	Con #93 (TESIS-133), ninguno: tocan archivos distintos, y la suite de arriba es la de los dos juntos

✅ La regla vive donde la puede garantizar

ConfirmDispatch#validate_order! corre primero, antes de la llamada al courier, y usa CreateShipment::NON_SHIPPABLE_STATUSES en lugar de una copia. Es la misma regla de negocio en los dos casos de uso, así que cuando exista la transición de estados hay un solo lugar que tocar.

✅ 422 y no 409: la corrección de la card es la correcta

Lo confirmé contra el front que ya está en master. DispatchShipmentDialog trata el 409 como «el envío ya se despachó mientras tanto» y, en ese caso, no ofrece reintentar. Con un 409, una orden cancelada habría mostrado un motivo falso. Con 422 cae en el camino genérico y muestra el mensaje del servidor. Además, el hecho de negocio («una orden cancelada no se envía») contesta igual en los dos endpoints: mismo error, mismo status, mismo texto. ShipmentsController ya rescataba UnshippableOrderError como 422, así que el PR no necesita tocar el controller.

✅ Chequear sólo antes de la llamada

No revalidar bajo el lock! es una decisión y está bien razonada: si la orden se cancela mientras el courier contesta, la etiqueta ya existe, y descartar el número de seguimiento perdería el rastro de un paquete que el courier conoce. El comentario lo deja escrito para quien venga a «completar» el chequeo.

✅ Los dientes que declara el PR, probados de nuevo

Saqué validate_order! de call y corrí los dos archivos de specs del despacho: 64 ejemplos, 8 fallas. Son exactamente los 4 del PORO y los 4 de request que agrega el PR, y ningún spec anterior. El control negativo con una orden paid sigue pasando, que es lo que tiene que pasar.

⚪ Para después, del lado del front (no es de este PR)

Si la orden se cancela con el diálogo de despacho abierto, el 422 llega como error genérico y el botón sigue diciendo «Reintentar el despacho», un reintento que no puede salir bien. El texto también llega en inglés («an order in status 'cancelled' cannot be shipped»), igual que los errores del courier. Es un detalle de TESIS-134 y no bloquea nada: por la UI, a una orden cancelada no se le ofrece «Despachar».

Los criterios de la card

• POST /api/v1/shipments/:id/dispatch sobre el envío de una orden cancelada responde 422 y no toca la base (el envío sigue pending, sin tracking y sin evento nuevo).
• No se llama al courier, verificado sobre el stub del adaptador y no sólo sobre la respuesta.
• Una orden pending (los specs de request existentes) o paid (control del PORO) despacha igual.
• Spec de request para el 422 y spec del PORO para la regla.
• RuboCop y RSpec en verde.

Veredicto

APPROVE.

Es el cambio justo para lo que pide la card. La desviación del 409 al 422 está bien fundada y ya quedó corregida en la card, y cada regla tiene un test que falla si se la saca.