diff --git a/app/poros/shipments/confirm_dispatch.rb b/app/poros/shipments/confirm_dispatch.rb index f2dcf16..c7d0215 100644 --- a/app/poros/shipments/confirm_dispatch.rb +++ b/app/poros/shipments/confirm_dispatch.rb @@ -43,6 +43,7 @@ def initialize(shipment:, company_integration:, origin_warehouse:, shipping_cost end def call + validate_order! validate_integration! validate_status!(@shipment) validate_cost! @@ -54,6 +55,32 @@ def call private + # Una orden cancelada no entra al circuito logístico, y que su envío ya esté + # abierto no lo cambia: emitir la etiqueta de una venta que no sale es pagar + # un despacho de más (TESIS-136). Hasta acá la regla vivía sólo en el alta + # del envío, así que un envío abierto antes de cancelar la orden se podía + # despachar igual por la API. + # + # La lista de estados es la de `CreateShipment` y no una copia: es la misma + # regla de negocio, y cuando exista la transición de estados hay un solo + # lugar que tocar. + # + # Mismo error y mismo 422 que el alta, a propósito. 409 sería el conflicto de + # estado del ENVÍO —eso es `AlreadyDispatchedError`— y el frontend lo lee + # así: ante un 409 el diálogo de despacho dice que el envío ya se despachó + # mientras tanto, que acá sería falso. + # + # Sólo antes de la llamada al courier, y no otra vez bajo el lock como el + # estado del envío: si la orden se cancela mientras el courier contesta, la + # etiqueta ya se emitió, y 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 el canal que corresponda. + def validate_order! + return unless CreateShipment::NON_SHIPPABLE_STATUSES.include?(order.status) + + raise UnshippableOrderError.new(order: order) + end + # Se valida antes de llamar al courier para no gastar una etiqueta —que el # proveedor cobra— en un envío que después no vamos a poder guardar. # diff --git a/spec/poros/shipments/confirm_dispatch_spec.rb b/spec/poros/shipments/confirm_dispatch_spec.rb index 79b719d..9bc2031 100644 --- a/spec/poros/shipments/confirm_dispatch_spec.rb +++ b/spec/poros/shipments/confirm_dispatch_spec.rb @@ -43,7 +43,8 @@ def dispatch(target = shipment, using: integration) def attempt_dispatch(target = shipment, using: integration) dispatch(target, using: using) rescue Shipments::AlreadyDispatchedError, Shipments::DispatchResponseError, - Shipments::InvalidCourierIntegrationError, Integrations::AdapterExecutionError + Shipments::InvalidCourierIntegrationError, Shipments::UnshippableOrderError, + Integrations::AdapterExecutionError nil end @@ -151,6 +152,52 @@ def dispatch_costing(cost) end end + # El alta del envío ya excluye las órdenes canceladas; el despacho no lo hacía, + # así que un envío abierto ANTES de cancelar la orden se podía despachar igual + # (TESIS-136, de la review de TESIS-134). + describe 'when the order was cancelled after the shipment was opened' do + before { order.update!(status: 'cancelled') } + + it 'refuses to dispatch it' do + expect { dispatch }.to raise_error(Shipments::UnshippableOrderError, /cannot be shipped/) + end + + # La etiqueta se paga: el chequeo va antes de la llamada, no después. + it 'does not call the courier' do + stub = stub_courier + attempt_dispatch + + expect(stub).not_to have_been_requested + end + + it 'leaves the shipment untouched', :aggregate_failures do + stub_courier + attempt_dispatch + + expect(shipment.reload.status).to eq('pending') + expect(shipment.tracking_number).to be_nil + end + + it 'records nothing in the log' do + stub_courier + + expect { attempt_dispatch }.not_to change(ShipmentEvent, :count) + end + end + + # Control negativo del bloque de arriba: lo que se rechaza es el estado + # cancelado, no cualquier estado distinto del inicial. + describe 'when the order is paid' do + before do + order.update!(status: 'paid') + stub_courier + end + + it 'dispatches it as usual' do + expect(dispatch.reload.status).to eq('ready_to_ship') + end + end + describe 'when the shipment is not pending' do let(:shipment) do Shipment.create!(company: company, order: order, status: 'in_transit', diff --git a/spec/requests/api/v1/shipment_dispatches_spec.rb b/spec/requests/api/v1/shipment_dispatches_spec.rb index cf53e91..fe8e1e0 100644 --- a/spec/requests/api/v1/shipment_dispatches_spec.rb +++ b/spec/requests/api/v1/shipment_dispatches_spec.rb @@ -211,6 +211,42 @@ def dispatch_shipment(id: shipment.id, integration_id: integration.id, expect(response).to have_http_status(:bad_gateway) end + # El alta del envío ya rechaza las órdenes canceladas con un 422; el despacho + # no lo hacía, así que un envío abierto antes de cancelar la orden se podía + # despachar por la API aunque la UI no lo ofreciera (TESIS-136). + describe 'when the order was cancelled after the shipment was opened' do + before { order.update!(status: 'cancelled') } + + it 'returns 422, the same as opening the shipment of a cancelled order' do + stub_courier + dispatch_shipment + + expect(response).to have_http_status(:unprocessable_content) + end + + it 'explains why it was rejected' do + stub_courier + dispatch_shipment + + expect(response.parsed_body['error']).to include('cannot be shipped') + end + + it 'does not call the courier' do + stub = stub_courier + dispatch_shipment + + expect(stub).not_to have_been_requested + end + + it 'leaves the shipment pending and without tracking', :aggregate_failures do + stub_courier + dispatch_shipment + + expect(shipment.reload.status).to eq('pending') + expect(shipment.tracking_number).to be_nil + end + end + describe 'request contract' do it 'returns 400 without the integration' do dispatch_shipment(integration_id: '')