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
27 changes: 27 additions & 0 deletions app/poros/shipments/confirm_dispatch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand All @@ -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.
#
Expand Down
49 changes: 48 additions & 1 deletion spec/poros/shipments/confirm_dispatch_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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',
Expand Down
36 changes: 36 additions & 0 deletions spec/requests/api/v1/shipment_dispatches_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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: '')
Expand Down
Loading