diff --git a/app/avo/resources/service.rb b/app/avo/resources/service.rb index 397243b..a5379b2 100644 --- a/app/avo/resources/service.rb +++ b/app/avo/resources/service.rb @@ -16,6 +16,10 @@ def fields # consulta periódica pregunta por sus envíos (TESIS-49). field :tracking_service, as: :belongs_to, use_resource: Avo::Resources::Service, name: 'Tracking template', only_on: %i[show forms] + # En la plantilla que despacha: la que le pide tarifas al mismo + # proveedor. Sin ella, el courier no se ofrece al cotizar (TESIS-131). + field :quote_service, as: :belongs_to, use_resource: Avo::Resources::Service, + name: 'Quote template', only_on: %i[show forms] mapper_fields end diff --git a/app/controllers/api/v1/draft_quotes_controller.rb b/app/controllers/api/v1/draft_quotes_controller.rb new file mode 100644 index 0000000..2c31313 --- /dev/null +++ b/app/controllers/api/v1/draft_quotes_controller.rb @@ -0,0 +1,116 @@ +# frozen_string_literal: true + +module Api + module V1 + # Cotización de un alta que todavía no se confirmó (TESIS-131). + # + # La cotización de TESIS-46 cuelga de una orden, y crear la orden descuenta + # el stock. El paso 3 del alta manual (TESIS-59) necesita mostrar las tarifas + # ANTES de que el operador confirme, así que acá se cotiza el paquete con lo + # que el asistente ya juntó: depósito de origen, destino y líneas. La orden se + # crea una sola vez, cuando el operador elige y confirma. + class DraftQuotesController < ApplicationController + # El parámetro que falta es un 400 de contrato, con el mismo cuerpo + # `{ error }` que el resto de la API. + rescue_from ActionController::ParameterMissing, with: :render_bad_request + + # Es el primer endpoint autenticado que sale a los couriers sin dejar nada + # en la base: antes cotizar exigía crear la orden, que era un freno natural. + # Sin tope, un cliente en loop —un `useEffect` mal puesto que cotiza en + # cada tecla— genera tantas llamadas a los proveedores como quiera, con las + # credenciales de la empresa. Se cuenta por usuario y no por IP: todos los + # requests llegan autenticados, y un depósito detrás de un mismo NAT no + # debería compartir el cupo. El asistente cotiza al entrar al paso 3 y en + # cada reintento, así que 20 por minuto sobra para el uso normal. + QUOTES_PER_WINDOW = 20 + QUOTE_WINDOW = 1.minute + + rate_limit to: QUOTES_PER_WINDOW, within: QUOTE_WINDOW, only: :create, + by: -> { current_user.id }, with: :render_too_many_quotes + + def create + # Cotizar un borrador es el paso previo a darlo de alta: se autoriza como + # crear una orden. Cada request va a los couriers con las credenciales de + # la empresa, pero no lee ni toca nada que no sea del propio tenant. + authorize Order, :create? + + quotes = Shipments::QuoteShipment.new( + origin_warehouse: origin_warehouse, destination: destination, lines: lines + ).call + + # 200 y una lista vacía cuando nadie contestó, igual que la cotización de + # una orden: el front distingue «sin opciones» de «falló la cotización». + render json: { data: quotes }, status: :ok + end + + private + + def quote_params + params.expect(quote: [:origin_warehouse_id, :destination_zip_code, :destination_address, + { items: [%i[product_id quantity]] }]) + end + + # find y no find_by: Warehouse es CompanyScoped, así que un id de otra + # empresa levanta RecordNotFound -> 404 en vez de revelar que existe. + def origin_warehouse + Warehouse.find(required(:origin_warehouse_id)) + end + + # El código postal es lo que cotizan todas las plantillas: sin él no hay + # tarifa posible. La dirección viaja si está, porque algunas la piden. + def destination + { zip_code: required(:destination_zip_code).to_s.strip, + address: quote_params[:destination_address].to_s.strip.presence } + end + + # Pares [producto, cantidad]. Los productos se buscan dentro del tenant: uno + # ajeno no aparece y responde 404, como el resto de la API. + def lines + products = Product.where(id: items.map(&:first)).index_by(&:id) + items.map do |product_id, quantity| + [products.fetch(product_id) { raise ActiveRecord::RecordNotFound }, quantity] + end + end + + # El mismo tope que el alta (OrdersController::MAX_ITEMS): cotizar algo que + # después no se podría crear no le sirve a nadie. + def items + @items ||= begin + raw = quote_params[:items] + raise ActionController::ParameterMissing, :items if raw.blank? + + limit = OrdersController::MAX_ITEMS + raise MalformedParameterError, "items exceeds maximum of #{limit}" if raw.size > limit + + raw.map do |item| + [positive_integer(item, :product_id), positive_integer(item, :quantity)] + end + end + end + + # «Bien formado» acá es un entero positivo. Un 0, un negativo o un texto + # no son un producto ni una cantidad que se pueda enviar. + def positive_integer(item, key) + value = Integer(item[key].to_s, exception: false) + return value if value&.positive? + + raise MalformedParameterError, "each item needs a positive integer #{key}" + end + + def render_too_many_quotes + response.set_header('Retry-After', QUOTE_WINDOW.to_i.to_s) + render json: { error: 'Too many quotes, try again later' }, status: :too_many_requests + end + + # `expect` cubre la clave ausente, no el valor vacío: sin esto un id en + # blanco llegaba a `find('')` y salía como 404, diciéndole al cliente que el + # recurso no existe cuando lo que falta es el parámetro. + def required(name) + value = quote_params[name] + raise ActionController::ParameterMissing, name if value.blank? + + value + end + end + end +end diff --git a/app/controllers/api/v1/shipment_quotes_controller.rb b/app/controllers/api/v1/shipment_quotes_controller.rb index 7ffcad3..169546a 100644 --- a/app/controllers/api/v1/shipment_quotes_controller.rb +++ b/app/controllers/api/v1/shipment_quotes_controller.rb @@ -8,7 +8,7 @@ def create order = Order.find(params.expect(:order_id)) authorize order, :quote? - quotes = Shipments::QuoteShipment.new( + quotes = Shipments::QuoteShipment.for_order( order: order, origin_warehouse: origin_warehouse ).call diff --git a/app/controllers/api/v1/shipments_controller.rb b/app/controllers/api/v1/shipments_controller.rb index e6a7f5e..8a4c841 100644 --- a/app/controllers/api/v1/shipments_controller.rb +++ b/app/controllers/api/v1/shipments_controller.rb @@ -15,6 +15,7 @@ class ShipmentsController < ApplicationController rescue_from Shipments::AlreadyDispatchedError, with: :render_conflict rescue_from Shipments::InvalidCourierIntegrationError, with: :render_unprocessable rescue_from Shipments::DispatchResponseError, with: :render_bad_gateway + rescue_from Shipments::InvalidShippingCostError, with: :render_bad_request rescue_from Integrations::AdapterExecutionError, with: :render_courier_failure # El parámetro que falta es un 400 de contrato, no un 422 de negocio. rescue_from ActionController::ParameterMissing, with: :render_bad_request @@ -68,7 +69,7 @@ def confirm dispatched = Shipments::ConfirmDispatch.new( shipment: shipment, company_integration: courier_integration, - origin_warehouse: origin_warehouse + origin_warehouse: origin_warehouse, shipping_cost: shipping_cost ).call render json: ShipmentSerializer.render(dispatched), status: :ok @@ -118,7 +119,19 @@ def origin_warehouse end def dispatch_params - params.expect(dispatch: %i[company_integration_id origin_warehouse_id]) + params.expect(dispatch: %i[company_integration_id origin_warehouse_id shipping_cost]) + end + + # El costo de la opción que el operador confirmó al cotizar (TESIS-131). + # Opcional: sin él, el despacho funciona como antes. Acá sólo se lee como + # número; el rango lo valida ConfirmDispatch contra el modelo, antes de + # llamar al courier, para que la regla viva en un solo lugar. + def shipping_cost + raw = dispatch_params[:shipping_cost] + return nil if raw.blank? + + BigDecimal(raw.to_s, exception: false) || + raise(MalformedParameterError, 'shipping_cost must be a number') end # `expect` cubre la clave ausente, no el valor vacío: sin esto un id en diff --git a/app/models/service.rb b/app/models/service.rb index db5589c..53572e8 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -24,12 +24,22 @@ class Service < ApplicationRecord has_many :tracked_services, class_name: 'Service', foreign_key: :tracking_service_id, inverse_of: :tracking_service, dependent: :nullify + # Plantilla con la que se le piden tarifas a este courier (TESIS-131). Cuelga + # de la plantilla que despacha, igual que la de seguimiento: es lo que permite + # pasar de una opción cotizada a su despacho, porque la cotización la contesta + # una plantilla y la etiqueta la emite otra. + belongs_to :quote_service, class_name: 'Service', optional: true, + inverse_of: :quoted_services + has_many :quoted_services, class_name: 'Service', foreign_key: :quote_service_id, + inverse_of: :quote_service, dependent: :nullify + validates :service_name, presence: true, uniqueness: true validates :uri, presence: true validates :http_method, presence: true validates :type, presence: true, inclusion: { in: TYPES } validate :mappers_are_valid_json validate :tracking_service_answers_tracking + validate :quote_service_quotes_shipping # Sólo los canales de e-commerce generan ventas: el gateway lo usa para decidir # si un webhook entrante va al procesador de órdenes (TESIS-43) o queda a la @@ -146,4 +156,29 @@ def tracking_service_problem 'no es una plantilla de consulta de tracking' unless tracking_service.answers_tracking? end + + def quote_service_quotes_shipping + reason = quote_service_problem + errors.add(:quote_service, reason) if reason + end + + # Las dos últimas reglas sostienen lo que la cotización asume: cada opción + # cotizada se despacha con UNA integración (QuoteShipment#dispatchers indexa + # por plantilla de cotización). Si dos plantillas de despacho compartieran el + # cotizador, una de las dos desaparecía de las opciones sin aviso; y una + # plantilla que no despacha con cotizador cargado es una configuración que no + # hace nada. El índice único de `quote_service_id` lo respalda en la base. + def quote_service_problem + return if quote_service.nil? + return 'solo aplica a couriers' unless courier? + return 'no puede ser la misma plantilla' if quote_service == self + return 'solo aplica a la plantilla con la que el courier despacha' unless dispatches_shipment? + return 'no es una plantilla de cotización' unless quote_service.quotes_shipping? + + 'ya es la plantilla de cotización de otro courier' if quote_service_taken? + end + + def quote_service_taken? + self.class.where(quote_service_id: quote_service_id).where.not(id: id).exists? + end end diff --git a/app/models/shipment.rb b/app/models/shipment.rb index 937456e..6cb352d 100644 --- a/app/models/shipment.rb +++ b/app/models/shipment.rb @@ -12,6 +12,12 @@ class Shipment < ApplicationRecord # cuota del proveedor. IN_FLIGHT_STATUSES = %w[ready_to_ship in_transit].freeze + # Tope de `shipping_cost`: la columna es decimal(10,2), así que lo más grande + # que entra es 99.999.999,99. Validarlo en el modelo hace que un costo fuera de + # rango sea un error de validación y no un RangeError de la base; el despacho + # lo prueba contra esta regla antes de pedir la etiqueta (TESIS-131). + MAX_SHIPPING_COST = 100_000_000 + belongs_to :company # La integración se asigna al inicializar el envío y puede no existir todavía # (se completa al confirmar el despacho con un courier). @@ -26,7 +32,8 @@ class Shipment < ApplicationRecord scope :in_flight, -> { where(status: IN_FLIGHT_STATUSES).where.not(tracking_number: nil) } validates :status, presence: true, inclusion: { in: STATUSES } - validates :shipping_cost, numericality: { greater_than_or_equal_to: 0 }, allow_nil: true + validates :shipping_cost, numericality: { greater_than_or_equal_to: 0, + less_than: MAX_SHIPPING_COST }, allow_nil: true # Restricción 1 a 1 de la card: una orden no puede tener dos envíos. El índice # único sobre order_id (migración) es la garantía a nivel motor; la validación # del modelo da un mensaje de error limpio antes de llegar a la DB. diff --git a/app/poros/shipments/confirm_dispatch.rb b/app/poros/shipments/confirm_dispatch.rb index 764d2fc..f2dcf16 100644 --- a/app/poros/shipments/confirm_dispatch.rb +++ b/app/poros/shipments/confirm_dispatch.rb @@ -31,16 +31,21 @@ class ConfirmDispatch < ApplicationPoro # es NOT NULL y es lo que la pantalla muestra como lo que pasó (TESIS-60). INITIAL_EXTERNAL_STATUS = 'Etiqueta generada' - def initialize(shipment:, company_integration:, origin_warehouse:) + # `shipping_cost` es el de la opción que el operador confirmó al cotizar + # (TESIS-131). Es opcional: un despacho que no lo trae deja el costo como + # estaba. + def initialize(shipment:, company_integration:, origin_warehouse:, shipping_cost: nil) super() @shipment = shipment @integration = company_integration @origin = origin_warehouse + @shipping_cost = shipping_cost end def call validate_integration! validate_status!(@shipment) + validate_cost! parsed = request_label persist(parsed) @@ -62,6 +67,23 @@ def validate_status!(shipment) raise AlreadyDispatchedError.new(shipment: shipment) end + # El costo se prueba contra la regla del modelo —la misma que aplica el + # `update!` de `persist`— y no contra una copia: si la columna cambia, la + # validación la sigue. Sin esto, un costo que el modelo rechaza (fuera de + # rango, NaN, infinito) pasaba, se pedía la etiqueta y el `update!` fallaba + # después: el envío seguía en `pending` y un reintento emitía otra etiqueta. + # + # Se valida un envío nuevo con sólo el costo para no tocar `@shipment` antes + # de la llamada externa; del resultado se lee únicamente `shipping_cost`. + def validate_cost! + return if @shipping_cost.nil? + + probe = Shipment.new(shipping_cost: @shipping_cost) + probe.validate + reasons = probe.errors.messages_for(:shipping_cost) + raise InvalidShippingCostError, reasons if reasons.any? + end + def validate_integration! reason = integration_problem return if reason.nil? @@ -140,11 +162,18 @@ def persist(parsed) @shipment.update!(company_integration: @integration, tracking_number: tracking_number!(parsed), shipping_label_url: parsed[LABEL_KEY], - status: DISPATCHED_STATUS) + status: DISPATCHED_STATUS, + **confirmed_cost) register_event end end + # El costo se escribe sólo si vino: sin él, el despacho no tiene por qué + # borrar uno que ya estuviera cargado. + def confirmed_cost + @shipping_cost.nil? ? {} : { shipping_cost: @shipping_cost } + end + # Sin número de seguimiento el despacho no sirve para nada: no se puede # seguir el paquete ni emparejar los eventos que el courier empuje después. # La etiqueta, en cambio, puede faltar — no todos los proveedores devuelven diff --git a/app/poros/shipments/invalid_shipping_cost_error.rb b/app/poros/shipments/invalid_shipping_cost_error.rb new file mode 100644 index 0000000..bf68585 --- /dev/null +++ b/app/poros/shipments/invalid_shipping_cost_error.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Shipments + # El costo confirmado es un número, pero no uno que el envío pueda guardar: + # negativo, fuera del rango de la columna, NaN o infinito. Se detecta antes de + # pedir la etiqueta, así que el courier no se llegó a llamar. Es un dato del + # request, y el controller lo mapea a 400. + class InvalidShippingCostError < StandardError + def initialize(reasons) + super("shipping_cost #{reasons.to_sentence}") + end + end +end diff --git a/app/poros/shipments/quote_shipment.rb b/app/poros/shipments/quote_shipment.rb index eb956f3..6dcd9a3 100644 --- a/app/poros/shipments/quote_shipment.rb +++ b/app/poros/shipments/quote_shipment.rb @@ -17,10 +17,26 @@ class QuoteShipment < ApplicationPoro # request y el usuario está esperando. La card pide 3-5s. TIMEOUTS = { open: 4, read: 4 }.freeze - def initialize(order:, origin_warehouse:) + # La cotización de una orden que ya existe (TESIS-46): el destino y las + # líneas salen de la orden. + def self.for_order(order:, origin_warehouse:) + new(origin_warehouse: origin_warehouse, + destination: { zip_code: order.customer_zip_code, address: order.customer_address }, + lines: order.order_items.includes(:product).map { |item| [item.product, item.quantity] }) + end + + # Lo que se cotiza es el paquete, no la orden: de dónde sale, a dónde va y + # qué lleva. Así se puede cotizar también un alta que todavía no se confirmó + # (TESIS-131), sin crear la orden —y descontar el stock— para averiguar + # cuánto cuesta enviarla. + # + # `lines` son pares `[producto, cantidad]`, y `destination` lleva + # `:zip_code` y `:address`. + def initialize(origin_warehouse:, destination:, lines:) super() - @order = order @origin = origin_warehouse + @destination = destination + @lines = lines end def call @@ -40,15 +56,37 @@ def call private - # Sólo las integraciones activas cuyo template sabe cotizar. Un courier puede - # tener también una plantilla de despacho: ésa no contesta tarifas y pedírsela - # sería llamar al endpoint equivocado (ver Service#quotes_shipping?). + # Las integraciones a las que se les piden tarifas: activas, con un template + # que sabe cotizar (una plantilla de despacho no contesta tarifas, ver + # Service#quotes_shipping?) y con una integración que despache por el mismo + # courier (TESIS-131). + # + # Lo último se filtra acá y no después de cotizar: una opción que no se puede + # despachar no es una opción, y pedirle la tarifa sería gastar una llamada al + # proveedor —y hacer esperar al operador— por algo que no se va a mostrar. def integrations - @integrations ||= CompanyIntegration.where(is_active: true) - .joins(:service) - .where(services: { type: Service::COURIER }) - .includes(:service) - .select { |ci| ci.service.quotes_shipping? } + @integrations ||= active_couriers.select do |ci| + ci.service.quotes_shipping? && dispatchers.key?(ci.service_id) + end + end + + # La integración que despacha por cada plantilla de cotización, indexada por + # el id de esa plantilla. La cotización la contesta una plantilla y la + # etiqueta la emite otra; el vínculo lo declara `Service#quote_service`. + # `index_by` no pierde a nadie porque una plantilla de cotización es de un + # solo despachador: lo valida Service y lo respalda un índice único. + def dispatchers + @dispatchers ||= active_couriers.select { |ci| ci.service.dispatches_shipment? } + .select { |ci| ci.service.quote_service_id.present? } + .index_by { |ci| ci.service.quote_service_id } + end + + def active_couriers + @active_couriers ||= CompanyIntegration.where(is_active: true) + .joins(:service) + .where(services: { type: Service::COURIER }) + .includes(:service) + .to_a end # El rescate va DENTRO del hilo: `Thread#value` re-levanta la excepción del @@ -82,12 +120,19 @@ def quote_with(integration, body) # Una respuesta sin costo no es una opción que el usuario pueda elegir: se # descarta como si el operador no hubiera contestado, en vez de ofrecer una # tarifa vacía. `estimated_days` sí puede faltar — es informativo. + # + # El nombre es el del courier (la plantilla que despacha) y no el de su + # plantilla de cotización: el operador elige «Andreani», no + # «Andreani - Cotización». `dispatch_integration_id` es lo que se le manda al + # despacho para confirmar esta opción. def normalize(integration, parsed) cost = parsed[COST_KEY] return nil if cost.blank? + dispatcher = dispatchers.fetch(integration.service_id) { company_integration_id: integration.id, - provider_name: integration.service.service_name, + dispatch_integration_id: dispatcher.id, + provider_name: dispatcher.service.service_name, shipping_cost: BigDecimal(cost.to_s), estimated_days: parsed[DAYS_KEY]&.to_i } rescue ArgumentError @@ -100,18 +145,18 @@ def payload { 'origin_zip_code' => @origin.zip_code, 'origin_address' => @origin.address, - 'destination_zip_code' => @order.customer_zip_code, - 'destination_address' => @order.customer_address, + 'destination_zip_code' => @destination[:zip_code], + 'destination_address' => @destination[:address], 'total_weight' => total_weight, - 'total_items' => @order.order_items.sum(:quantity) + 'total_items' => @lines.sum { |_product, quantity| quantity } } end - # Peso del paquete: la suma de peso × cantidad de cada ítem. `products.weight` + # Peso del paquete: la suma de peso × cantidad de cada línea. `products.weight` # es decimal y arranca en 0, así que un producto sin peso cargado no rompe la # cotización — suma cero. def total_weight - @order.order_items.includes(:product).sum { |item| item.product.weight * item.quantity } + @lines.sum { |product, quantity| product.weight * quantity } end end end diff --git a/config/routes.rb b/config/routes.rb index 10de6e3..c5e5dab 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -35,6 +35,10 @@ end end + # Cotización de un alta que todavía no existe (TESIS-131): cuelga de la + # raíz y no de una orden, porque lo que se cotiza es el borrador. + resources :quotes, only: %i[create], controller: 'draft_quotes' + resources :orders, only: %i[index show create update] do resources :quotes, only: %i[create], controller: 'shipment_quotes' diff --git a/db/migrate/20260924130000_add_quote_service_to_services.rb b/db/migrate/20260924130000_add_quote_service_to_services.rb new file mode 100644 index 0000000..c00556d --- /dev/null +++ b/db/migrate/20260924130000_add_quote_service_to_services.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class AddQuoteServiceToServices < ActiveRecord::Migration[8.1] + # Plantilla con la que se le piden tarifas al courier que despacha con esta + # plantilla (TESIS-131). Cotizar y despachar son dos endpoints del proveedor y, + # por convención, dos `Service`; esto es lo que dice que son del mismo + # proveedor, para que una opción cotizada se pueda despachar. + # + # Mismo criterio que `tracking_service_id` (TESIS-49): es una relación entre + # plantillas, no entre integraciones, y cuelga de la plantilla que despacha. + # nullable: un courier sin plantilla de cotización simplemente no se ofrece. + # + # Único: una plantilla de cotización es de un solo despachador. La cotización + # devuelve una opción por cotizador y la despacha con UNA integración; si dos + # plantillas de despacho compartieran la de cotización, una se perdería. + def change + add_reference :services, :quote_service, + index: { unique: true }, + foreign_key: { to_table: :services, on_delete: :nullify } + end +end diff --git a/db/schema.rb b/db/schema.rb index 2789dcc..a88bf77 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -148,6 +148,7 @@ create_table "services", force: :cascade do |t| t.datetime "created_at", null: false t.string "http_method", null: false + t.bigint "quote_service_id" t.jsonb "request_mapper", default: {}, null: false t.jsonb "request_value_mapper", default: {}, null: false t.jsonb "response_mapper", default: {}, null: false @@ -157,6 +158,7 @@ t.string "type", null: false t.datetime "updated_at", null: false t.string "uri", null: false + t.index ["quote_service_id"], name: "index_services_on_quote_service_id", unique: true t.index ["service_name"], name: "index_services_on_service_name", unique: true t.index ["tracking_service_id"], name: "index_services_on_tracking_service_id" t.check_constraint "type::text = ANY (ARRAY['ecommerce'::character varying, 'courier'::character varying]::text[])", name: "services_type_check" @@ -276,6 +278,7 @@ add_foreign_key "product_mappings", "company_integrations", on_delete: :cascade add_foreign_key "product_mappings", "products", on_delete: :cascade add_foreign_key "products", "companies", on_delete: :cascade + add_foreign_key "services", "services", column: "quote_service_id", on_delete: :nullify add_foreign_key "services", "services", column: "tracking_service_id", on_delete: :nullify add_foreign_key "shipment_events", "shipments", on_delete: :cascade add_foreign_key "shipments", "companies", on_delete: :cascade diff --git a/db/seeds.rb b/db/seeds.rb index 35fa217..29820e2 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -307,6 +307,12 @@ correo_tracking = Service.find_by(service_name: 'Correo Argentino - Seguimiento') correo_service&.update!(tracking_service: correo_tracking) if correo_tracking +# Andreani cotiza con una plantilla y despacha con otra (TESIS-131): el vínculo +# es lo que deja despachar la opción que el operador eligió al cotizar. +andreani_dispatch = Service.find_by(service_name: 'Andreani') +andreani_quote = Service.find_by(service_name: 'Andreani - Cotización') +andreani_dispatch&.update!(quote_service: andreani_quote) if andreani_quote + # Vincula la primera empresa activa con Mercado Libre (integración de ejemplo). # La variable ml_integration la consume la orden de webhook de la sección TESIS-40 # más abajo (sin ella, `db:seed` cortaba con NameError: undefined ml_integration). diff --git a/docs/adr/ADR-016-cotizacion-del-alta-manual.md b/docs/adr/ADR-016-cotizacion-del-alta-manual.md new file mode 100644 index 0000000..f6c8dde --- /dev/null +++ b/docs/adr/ADR-016-cotizacion-del-alta-manual.md @@ -0,0 +1,93 @@ +# ADR-016: Cotización del alta manual y despacho de la opción elegida + +**Fecha:** 2026-09-24 +**Estado:** Aceptado + +--- + +## Contexto + +El paso 3 del alta manual de órdenes (TESIS-59, diseño S07) sigue este recorrido: al entrar se cotiza contra todos los operadores, el operador elige una opción y **«Confirmar orden»** crea la orden y emite el despacho. Las piezas de la API existían por separado —cotizar (TESIS-46), crear el envío (TESIS-105, [ADR-012](ADR-012-creacion-de-envios.md)) y despacharlo (TESIS-47)—, pero juntas no alcanzaban para ese recorrido. Había tres huecos: + +1. **La cotización necesitaba una orden** (`POST /orders/:id/quotes`). Para mostrar las tarifas antes de confirmar había que crear la orden, y eso descuenta el stock. Si el operador abandonaba el paso, quedaba una orden `pending` que nadie podía cancelar: `Orders::UpdateOrder` deja la cancelación afuera a propósito ([ADR-013](ADR-013-modificacion-de-ordenes.md)). +2. **Una cotización no se podía despachar.** Cotizar y despachar son dos endpoints del proveedor y, por convención, dos `Service` («Andreani - Cotización» y «Andreani»). Cada opción informaba la integración de la plantilla que **cotiza**, y `Shipments::ConfirmDispatch` exige una que **despache**. Nada en el modelo decía que eran del mismo courier. +3. **El costo elegido se perdía.** El despacho no escribía `shipments.shipping_cost`, así que el detalle de la orden mostraba el envío «a cotizar» para siempre. + +## Decisión + +``` +Paso 3 del alta + │ + ├─ POST /api/v1/quotes cotiza el borrador, sin crear nada + │ { origin_warehouse_id, destination_zip_code, destination_address, + │ items: [{ product_id, quantity }] } + │ -> [{ company_integration_id, dispatch_integration_id, + │ provider_name, shipping_cost, estimated_days }] + │ + └─ «Confirmar orden» + ├─ POST /api/v1/orders crea la orden y descuenta el stock + ├─ POST /api/v1/orders/:id/shipment abre el envío en `pending` + └─ POST /api/v1/shipments/:id/dispatch + { company_integration_id: , + origin_warehouse_id, shipping_cost } +``` + +| Pieza | Rol | +| --------------------------------------- | ----------------------------------------------------------------------------------------- | +| `Api::V1::DraftQuotesController` | `POST /api/v1/quotes`: arma el paquete del borrador dentro del tenant y delega | +| `Shipments::QuoteShipment` | Cotiza un paquete (origen, destino y líneas); `.for_order` lo arma desde una orden | +| `Service#quote_service` | En la plantilla que despacha: la plantilla con la que se cotiza al mismo courier | +| `Shipments::ConfirmDispatch` | Guarda además el `shipping_cost` confirmado, si viene | + +### Se cotiza el paquete, no la orden + +`QuoteShipment` recibe el origen, el destino y pares `[producto, cantidad]`. El peso sigue saliendo de `products.weight` del lado del backend: el cliente manda qué lleva el paquete, no cuánto pesa. La cotización de una orden (`POST /orders/:id/quotes`) arma ese mismo contexto con `QuoteShipment.for_order` y responde igual que antes. + +**Alternativas descartadas:** + +- **Un estado `draft` de la orden** que no descuente stock hasta confirmar. Resolvía el hueco 1, pero tocaba los estados, el listado, los KPIs del panel y el momento del descuento: un cambio de dominio para un problema de secuencia. +- **Que el front mande el peso total.** Duplica en el cliente un dato que ya es del backend, y deja que el cliente decida sobre qué se cotiza. + +### El vínculo entre plantillas cuelga de la que despacha + +`services.quote_service_id` sigue el patrón de `tracking_service_id` ([ADR-014](ADR-014-pull-tracking-de-couriers.md)). El `Service` del proveedor es el que despacha, y sus plantillas auxiliares (la de seguimiento, la de cotización) cuelgan de él. La validación también es la misma: sólo couriers, nunca la propia plantilla, y sólo una plantilla que cotice (`Service#quotes_shipping?`). + +Además, **una plantilla de cotización es de un solo despachador**, y sólo la plantilla que despacha puede tener una. La cotización devuelve una opción por plantilla de cotización y la despacha con una integración; si dos plantillas de despacho («Andreani» y «Andreani Express») compartieran el cotizador, una desaparecería de las opciones sin aviso. `Service` lo valida con un mensaje para el panel, y un índice único sobre `quote_service_id` lo respalda en la base. Si algún día dos servicios del mismo proveedor necesitan el mismo endpoint de tarifas, se carga una plantilla de cotización por cada uno: son filas, no código. + +Con el vínculo, cada opción informa `dispatch_integration_id` y se nombra por el courier («Andreani», no «Andreani - Cotización»). + +**Una plantilla de cotización sin integración de despacho activa no se consulta.** Se filtra antes de abrir los hilos, no después de cotizar: una opción que el operador no puede confirmar no es una opción, y pedirle la tarifa sería hacerlo esperar por algo que no se va a mostrar. + +### El costo que se guarda es el que se confirmó + +El despacho acepta `shipping_cost` opcional y lo escribe en el envío. Se validó la alternativa de volver a cotizar al despachar y guardar lo que conteste el courier, y se descartó por dos motivos: es una segunda llamada externa dentro del request, y el precio podría no coincidir con el que el operador aceptó segundos antes. + +El valor se valida antes de pedir la etiqueta (que el courier cobra), y contra la regla del modelo, no contra una copia: `ConfirmDispatch` prueba el costo con las validaciones de `Shipment` —no negativo y menor a 100.000.000, lo que entra en `decimal(10,2)`— antes de llamar al courier. Un costo que no es un número, negativo, fuera de rango, `NaN` o infinito responde 400 sin haber llamado a nadie. Validarlo sólo en el controller dejaba pasar los tres últimos: se emitía la etiqueta, el `update!` fallaba después y el envío seguía `pending`, así que un reintento pagaba una segunda etiqueta. Un despacho sin costo deja el que hubiera. + +El costo lo manda el cliente y no se compara con lo que devolvió la cotización: el operador está autenticado dentro de su propio tenant, y el costo que se guarda es informativo —no se le cobra a nadie a partir de él—. Si algún día se factura con este dato, la comparación pasa a ser necesaria. + +### Cotizar tiene un tope por usuario + +`POST /quotes` es el primer endpoint autenticado que sale a los proveedores sin dejar rastro en la base: antes, cotizar exigía crear la orden, que funcionaba como freno. Sin tope, un cliente en loop —un `useEffect` mal puesto que cotiza en cada tecla— generaría tantas llamadas a los couriers como quisiera, con las credenciales de la empresa. Se aplica el `rate_limit` de Rails (el mismo mecanismo que el registro, TESIS-82): **20 cotizaciones por minuto por usuario**, con 429 y `Retry-After`. Se cuenta por usuario y no por IP porque todos los requests llegan autenticados, y los operarios de un depósito detrás de un mismo NAT no deberían compartir el cupo. + +## Consecuencias + +**A favor** + +- El stock se descuenta una sola vez, cuando el operador confirma. Cotizar no crea nada. +- Una opción cotizada siempre se puede despachar: la que no tendría con qué, no aparece. +- El detalle de la orden muestra el costo que se eligió. + +**En contra** + +- **Confirmar son tres requests encadenados, y no es atómico.** El despacho llama a un courier externo y no puede ir en la misma transacción que el alta. Si falla, la orden queda creada con su envío `pending`, que es un estado válido y se puede volver a despachar. Lo resuelve el front, reintentando el despacho sobre la orden ya creada. +- **El precio puede cambiar entre la cotización y el despacho.** Se guarda el que se confirmó, no el que el courier cobre al emitir la etiqueta. Si un proveedor empieza a devolver el costo en la respuesta del despacho, esa es la fuente mejor, y este ADR se revisa. +- Un courier cargado sin su `quote_service` no se ofrece al cotizar. En el panel de administración, el vínculo se edita en la plantilla que despacha, junto a la de seguimiento. +- El contador del tope vive en la cache de la app (Solid Cache en producción). En test es `:null_store` y el tope nunca se alcanza, salvo en el spec que lo prueba. + +## Referencias + +- TESIS-131 — la card de este ADR +- TESIS-59 — el paso 3 del alta manual, que consume este flujo +- TESIS-46 / TESIS-47 / TESIS-105 — cotización, despacho y alta del envío +- [ADR-012](ADR-012-creacion-de-envios.md), [ADR-014](ADR-014-pull-tracking-de-couriers.md) diff --git a/docs/guidelines/architecture.md b/docs/guidelines/architecture.md index 5195994..5240f0e 100644 --- a/docs/guidelines/architecture.md +++ b/docs/guidelines/architecture.md @@ -424,7 +424,10 @@ respuesta un dato que no sirve más tarde. del request**, no desde un job. El motivo es el producto: el usuario está esperando la lista de tarifas para elegir una. Devolverla por un job obligaría a sondear o a abrir un canal de tiempo real para un dato que se consume en el acto -y que caduca enseguida. +y que caduca enseguida. `POST /api/v1/quotes` (TESIS-131) es la misma cotización +sobre un alta que todavía no se confirmó, y corre por el mismo caso de uso: la +excepción es una sola, no dos (ver +[ADR-016](../adr/ADR-016-cotizacion-del-alta-manual.md)). `POST /api/v1/shipments/:id/dispatch` (TESIS-47) hace lo propio con el operador que el usuario eligió: le pide la etiqueta y devuelve el número de seguimiento y diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 0942e95..10f427c 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -205,6 +205,97 @@ def template(name, uri) end end + # La plantilla que cotiza por el courier que despacha con ésta (TESIS-131). + describe 'quote_service' do + subject(:courier) do + described_class.new(service_name: 'Andreani', type: 'courier', http_method: 'POST', + uri: 'https://api.andreani.test/ordenes', + response_mapper: { 'numero' => 'tracking_number' }) + end + + let(:quote_template) do + described_class.create!(service_name: 'Andreani - Cotización', type: 'courier', + http_method: 'POST', uri: 'https://api.andreani.test/tarifas', + response_mapper: { 'total' => 'shipping_cost' }) + end + + it 'accepts a template that quotes shipping' do + courier.quote_service = quote_template + expect(courier).to be_valid + end + + it 'rejects a template that does not quote shipping' do + courier.quote_service = described_class.create!( + service_name: 'Andreani - Seguimiento', type: 'courier', http_method: 'GET', + uri: 'https://api.andreani.test/envios/:tracking_number' + ) + expect(courier).not_to be_valid + end + + it 'rejects pointing a template at itself' do + quote_template.quote_service = quote_template + expect(quote_template).not_to be_valid + end + + # Otra plantilla de despacho del mismo proveedor, como «Andreani Express». + def other_dispatcher(**attrs) + described_class.create!(service_name: 'Andreani Express', type: 'courier', http_method: 'POST', + uri: 'https://api.andreani.test/express', + response_mapper: { 'numero' => 'tracking_number' }, **attrs) + end + + # Sin esto QuoteShipment#dispatchers se quedaba con uno de los dos y el otro + # desaparecía de las opciones sin aviso: lo encontró la review de TESIS-131. + it 'rejects a quote template that another courier already dispatches with', :aggregate_failures do + other_dispatcher(quote_service: quote_template) + courier.quote_service = quote_template + + expect(courier).not_to be_valid + expect(courier.errors[:quote_service]).to include('ya es la plantilla de cotización de otro courier') + end + + it 'lets the courier that has the quote template keep it' do + courier.update!(quote_service: quote_template) + courier.uri = 'https://api.andreani.test/v2/ordenes' + + expect(courier).to be_valid + end + + it 'backs the rule with a unique index' do + courier.update!(quote_service: quote_template) + + expect { other_dispatcher.update_column(:quote_service_id, quote_template.id) } # rubocop:disable Rails/SkipsModelValidations + .to raise_error(ActiveRecord::RecordNotUnique) + end + + # Una plantilla de cotización o de seguimiento con cotizador cargado es una + # configuración que no hace nada: la cotización sólo mira a los que despachan. + it 'rejects a quote template on a template that does not dispatch', :aggregate_failures do + tracking = described_class.new(service_name: 'Andreani - Seguimiento', type: 'courier', + http_method: 'GET', quote_service: quote_template, + uri: 'https://api.andreani.test/envios/:tracking_number') + + expect(tracking).not_to be_valid + expect(tracking.errors[:quote_service]).to include('solo aplica a la plantilla con la que el courier despacha') + end + + it 'rejects a quote template on a sales channel' do + service.quote_service = quote_template + expect(service).not_to be_valid + end + + it 'lets the quote template reach the courier it quotes for' do + courier.update!(quote_service: quote_template) + expect(quote_template.quoted_services).to contain_exactly(courier) + end + + it 'is released when the quote template is destroyed' do + courier.update!(quote_service: quote_template) + quote_template.destroy! + expect(courier.reload.quote_service).to be_nil + end + end + it 'persists nested JSONB mappers', :aggregate_failures do service.update!(request_mapper: { 'order' => { 'id' => 'external_id' } }) expect(service.reload.request_mapper).to eq('order' => { 'id' => 'external_id' }) diff --git a/spec/models/shipment_spec.rb b/spec/models/shipment_spec.rb index 73feef6..415ee6b 100644 --- a/spec/models/shipment_spec.rb +++ b/spec/models/shipment_spec.rb @@ -40,6 +40,19 @@ expect(shipment.errors[:shipping_cost]).to include('must be greater than or equal to 0') end + # decimal(10,2): lo que no entra en la columna es un error de validación, no + # un RangeError de la base (TESIS-131). + it 'accepts the largest shipping_cost the column holds' do + shipment.shipping_cost = BigDecimal('99999999.99') + expect(shipment).to be_valid + end + + it 'rejects a shipping_cost that does not fit the column', :aggregate_failures do + shipment.shipping_cost = Shipment::MAX_SHIPPING_COST + expect(shipment).not_to be_valid + expect(shipment.errors[:shipping_cost]).to include('must be less than 100000000') + end + it 'is invalid without a company' do shipment.company = nil expect(shipment).not_to be_valid diff --git a/spec/poros/shipments/confirm_dispatch_spec.rb b/spec/poros/shipments/confirm_dispatch_spec.rb index f3472b4..79b719d 100644 --- a/spec/poros/shipments/confirm_dispatch_spec.rb +++ b/spec/poros/shipments/confirm_dispatch_spec.rb @@ -128,6 +128,29 @@ def attempt_dispatch(target = shipment, using: integration) expect(dispatch.reload.shipping_label_url).to be_nil end + # La regla del costo es la del modelo: lo que el `update!` rechazaría se + # rechaza antes de pedir la etiqueta, que el courier cobra (TESIS-131). + describe 'when the confirmed cost does not fit the shipment' do + def dispatch_costing(cost) + described_class.new(shipment: shipment, company_integration: integration, + origin_warehouse: warehouse, shipping_cost: cost).call + end + + it 'refuses a cost out of the range of the column, saying why' do + expect { dispatch_costing(BigDecimal('1e8')) } + .to raise_error(Shipments::InvalidShippingCostError, 'shipping_cost must be less than 100000000') + end + + it 'does not call the courier', :aggregate_failures do + stub = stub_courier + %w[1e8 NaN Infinity -1].each do |cost| + expect { dispatch_costing(BigDecimal(cost)) }.to raise_error(Shipments::InvalidShippingCostError) + end + + expect(stub).not_to have_been_requested + 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/poros/shipments/quote_shipment_spec.rb b/spec/poros/shipments/quote_shipment_spec.rb index 383289a..c03329b 100644 --- a/spec/poros/shipments/quote_shipment_spec.rb +++ b/spec/poros/shipments/quote_shipment_spec.rb @@ -3,7 +3,7 @@ require 'rails_helper' RSpec.describe Shipments::QuoteShipment, type: :poro do - subject(:quotes) { described_class.new(order: order, origin_warehouse: origin).call } + subject(:quotes) { described_class.for_order(order: order, origin_warehouse: origin).call } let(:company) { Company.create!(name: 'Acme', tax_id: '20-12345678-9') } let(:origin) do @@ -14,11 +14,25 @@ customer_address: 'Av. Siempreviva 742') end - def quoting_service(name, uri) - Service.create!(service_name: name, type: 'courier', http_method: 'POST', uri: uri, - request_mapper: { 'cp' => 'destination_zip_code' }, - response_mapper: { 'precio' => 'shipping_cost', 'dias' => 'estimated_days' }, - request_value_mapper: {}, response_value_mapper: {}) + # La plantilla que cotiza por un courier. Salvo que se pida lo contrario, el + # courier queda completo: su plantilla de despacho vinculada a ésta + # (TESIS-131) y ya integrada, porque sin ella la opción no se ofrecería. + def quoting_service(name, uri, request_mapper: { 'cp' => 'destination_zip_code' }, + dispatchable: true) + quote = Service.create!(service_name: "#{name} - Cotización", type: 'courier', + http_method: 'POST', uri: uri, request_mapper: request_mapper, + response_mapper: { 'precio' => 'shipping_cost', + 'dias' => 'estimated_days' }, + request_value_mapper: {}, response_value_mapper: {}) + integrate(dispatch_service(name, quote_service: quote)) if dispatchable + quote + end + + def dispatch_service(name, quote_service: nil) + Service.create!(service_name: name, type: 'courier', http_method: 'POST', + uri: "https://#{name.downcase}.test/ordenes", quote_service: quote_service, + response_mapper: { 'numero' => 'tracking_number' }, + request_mapper: {}, request_value_mapper: {}, response_value_mapper: {}) end def integrate(service) @@ -26,14 +40,19 @@ def integrate(service) credentials: { 'access_token' => 'T' }, is_active: true) end + # Un courier listo para cotizar. Devuelve la integración que cotiza. + def courier(name, uri, **quote_options) + integrate(quoting_service(name, uri, **quote_options)) + end + before { Current.company_id = company.id } after { Current.company_id = nil } context 'with two couriers that answer' do before do - integrate(quoting_service('Fast', 'https://fast.test/rates')) - integrate(quoting_service('Cheap', 'https://cheap.test/rates')) + courier('Fast', 'https://fast.test/rates') + courier('Cheap', 'https://cheap.test/rates') stub_request(:post, 'https://fast.test/rates') .to_return(status: 200, body: { precio: 2500.0, dias: 1 }.to_json) stub_request(:post, 'https://cheap.test/rates') @@ -56,6 +75,53 @@ def integrate(service) it 'reports which integration produced each option' do expect(quotes.pluck(:company_integration_id)).to all(be_present) end + + # Lo que el despacho necesita para confirmar la opción elegida (TESIS-131): + # la integración que emite la etiqueta, no la que contestó la tarifa. + it 'reports the integration that dispatches each option' do + dispatcher = CompanyIntegration.joins(:service).find_by!(services: { service_name: 'Cheap' }) + + expect(quotes.first[:dispatch_integration_id]).to eq(dispatcher.id) + end + + # El operador elige «Cheap», no «Cheap - Cotización». + it 'names each option after its courier, not after its quote template' do + expect(quotes.pluck(:provider_name)).not_to include(a_string_ending_with('Cotización')) + end + end + + # Una opción que no se puede despachar no se ofrece, y ni siquiera se cotiza: + # sería hacer esperar al operador por algo que no va a poder confirmar. + context 'with a quote template that no dispatch template points at' do + before do + integrate(quoting_service('Orphan', 'https://orphan.test/rates', dispatchable: false)) + stub_request(:post, 'https://orphan.test/rates') + .to_return(status: 200, body: { precio: 900.0 }.to_json) + end + + it 'does not offer it' do + expect(quotes).to eq([]) + end + + it 'is not even asked for a price' do + quotes + + expect(WebMock).not_to have_requested(:post, 'https://orphan.test/rates') + end + end + + context 'when the integration that dispatches is inactive' do + before do + courier('Fast', 'https://fast.test/rates') + CompanyIntegration.joins(:service).find_by!(services: { service_name: 'Fast' }) + .update!(is_active: false) + stub_request(:post, 'https://fast.test/rates') + .to_return(status: 200, body: { precio: 900.0 }.to_json) + end + + it 'does not offer the option' do + expect(quotes).to eq([]) + end end # El paralelismo NO se verifica acá, y no por olvido: WebMock no es @@ -64,8 +130,8 @@ def integrate(service) # está hecha contra un servidor HTTP real y documentada en el PR. context 'when one courier is down' do before do - integrate(quoting_service('Fast', 'https://fast.test/rates')) - integrate(quoting_service('Broken', 'https://broken.test/rates')) + courier('Fast', 'https://fast.test/rates') + courier('Broken', 'https://broken.test/rates') stub_request(:post, 'https://fast.test/rates') .to_return(status: 200, body: { precio: 2500.0, dias: 1 }.to_json) stub_request(:post, 'https://broken.test/rates').to_return(status: 500) @@ -82,7 +148,7 @@ def integrate(service) context 'when every courier fails' do before do - integrate(quoting_service('Broken', 'https://broken.test/rates')) + courier('Broken', 'https://broken.test/rates') stub_request(:post, 'https://broken.test/rates').to_timeout end @@ -93,7 +159,7 @@ def integrate(service) context 'when a courier answers without a price' do before do - integrate(quoting_service('Empty', 'https://empty.test/rates')) + courier('Empty', 'https://empty.test/rates') stub_request(:post, 'https://empty.test/rates') .to_return(status: 200, body: { dias: 3 }.to_json) end @@ -170,9 +236,36 @@ def integrate(service) end end + # Lo que viaja es el paquete de la orden: peso × cantidad de cada línea y el + # total de bultos, además de origen y destino. + context 'when quoting an order' do + let(:rates) { 'https://fast.test/rates' } + + before do + sensor = Product.create!(company: company, sku: 'S-1', name: 'Sensor', weight: 0.5) + cable = Product.create!(company: company, sku: 'C-1', name: 'Cable', weight: 2) + order.order_items.create!(product: sensor, quantity: 3, unit_price: 100) + order.order_items.create!(product: cable, quantity: 2, unit_price: 100) + + courier('Fast', rates, request_mapper: { 'desde' => 'origin_zip_code', + 'hasta' => 'destination_zip_code', + 'kilos' => 'total_weight', + 'bultos' => 'total_items' }) + stub_request(:post, rates).to_return(status: 200, body: { precio: 900 }.to_json) + end + + it 'sends its weight, its item count, its origin and its destination' do + quotes + + expect(WebMock).to have_requested(:post, rates) + .with(body: hash_including('desde' => '1900', 'hasta' => '5000', + 'kilos' => '5.5', 'bultos' => 5)) + end + end + context 'with an inactive courier' do before do - integration = integrate(quoting_service('Fast', 'https://fast.test/rates')) + integration = courier('Fast', 'https://fast.test/rates') integration.update!(is_active: false) end diff --git a/spec/requests/api/v1/draft_quotes_spec.rb b/spec/requests/api/v1/draft_quotes_spec.rb new file mode 100644 index 0000000..3b949eb --- /dev/null +++ b/spec/requests/api/v1/draft_quotes_spec.rb @@ -0,0 +1,188 @@ +# frozen_string_literal: true + +require 'rails_helper' + +# Cotizar un alta antes de crear la orden (TESIS-131). +RSpec.describe 'Draft quotes API', type: :request do + let(:company) { Company.create!(name: 'Tenant A', tax_id: '30-11111111-1') } + let(:user) { User.create!(email: 'a@example.com', password: 'password123', company: company) } + let(:headers) { auth_headers(user) } + let(:rates) { 'https://fast.test/rates' } + + def warehouse + @warehouse ||= Warehouse.create!(company: company, name: 'Central', zip_code: '1900', + address: 'Calle 1') + end + + def product + @product ||= Product.create!(company: company, sku: 'S-1', name: 'Sensor', weight: 1.5) + end + + def auth_headers(user) + post '/api/v1/auth/login', params: { email: user.email, password: 'password123' }, + headers: { 'X-Tenant-Slug' => user.company.slug } + { 'Authorization' => "Bearer #{response.parsed_body['token']}" } + end + + # Un courier con sus dos plantillas vinculadas: la que cotiza y la que despacha. + def courier + quote = Service.create!(service_name: 'Fast - Cotización', type: 'courier', + http_method: 'POST', uri: rates, + request_mapper: { 'cp' => 'destination_zip_code', + 'kilos' => 'total_weight' }, + response_mapper: { 'precio' => 'shipping_cost', + 'dias' => 'estimated_days' }) + dispatch = Service.create!(service_name: 'Fast', type: 'courier', http_method: 'POST', + uri: 'https://fast.test/ordenes', quote_service: quote, + response_mapper: { 'numero' => 'tracking_number' }) + [quote, dispatch].map do |service| + CompanyIntegration.create!(company: company, service: service, + credentials: { 'access_token' => 'T' }, is_active: true) + end + end + + def draft(**overrides) + { origin_warehouse_id: warehouse.id, destination_zip_code: '5000', + destination_address: 'Av. Siempreviva 742', + items: [{ product_id: product.id, quantity: 2 }] }.merge(overrides) + end + + def quote_draft(body = draft, auth: headers) + post '/api/v1/quotes', params: { quote: body }, headers: auth, as: :json + end + + it 'returns 401 without a token' do + post '/api/v1/quotes', params: { quote: draft }, as: :json + + expect(response).to have_http_status(:unauthorized) + end + + context 'with a courier that answers' do + before do + courier + stub_request(:post, rates).to_return(status: 200, body: { precio: 2500.0, dias: 3 }.to_json) + end + + it 'returns the options in the same shape as the quote of an order', :aggregate_failures do + quote_draft + + expect(response).to have_http_status(:ok) + expect(response.parsed_body['data'].first.keys) + .to match_array(%w[company_integration_id dispatch_integration_id provider_name + shipping_cost estimated_days]) + end + + it 'quotes the parcel of the draft: its weight and its destination' do + quote_draft + + expect(WebMock).to have_requested(:post, rates) + .with(body: hash_including('cp' => '5000', 'kilos' => '3.0')) + end + + # El punto de la card: cotizar no crea la orden ni toca el stock. + it 'does not create an order' do + expect { quote_draft }.not_to change(Order, :count) + end + end + + it 'returns 200 with an empty list when no courier answers' do + quote_draft + + expect(response.parsed_body).to eq('data' => []) + end + + describe 'isolation between companies' do + let(:other) { Company.create!(name: 'Tenant B', tax_id: '30-22222222-2') } + + it 'answers 404 for a warehouse of another company' do + foreign = Warehouse.create!(company: other, name: 'Ajeno', zip_code: '1', address: 'x') + quote_draft(draft(origin_warehouse_id: foreign.id)) + + expect(response).to have_http_status(:not_found) + end + + it 'answers 404 for a product of another company' do + foreign = Product.create!(company: other, sku: 'X-1', name: 'Ajeno') + quote_draft(draft(items: [{ product_id: foreign.id, quantity: 1 }])) + + expect(response).to have_http_status(:not_found) + end + end + + describe 'a malformed draft' do + { + 'without the origin warehouse' => { origin_warehouse_id: nil }, + 'without the destination zip code' => { destination_zip_code: '' }, + 'without items' => { items: [] }, + 'with a quantity of zero' => { items: [{ product_id: 1, quantity: 0 }] }, + 'with a quantity that is not a number' => { items: [{ product_id: 1, quantity: 'dos' }] }, + 'with an item that is not an object' => { items: ['S-1'] } + }.each do |label, overrides| + it "returns 400 #{label}" do + quote_draft(draft(**overrides)) + + expect(response).to have_http_status(:bad_request) + end + end + + it 'says which parameter is missing, like the rest of the API' do + quote_draft(draft(destination_zip_code: '')) + + expect(response.parsed_body['error']).to include('destination_zip_code') + end + + it 'returns 400 above the item limit of an order' do + items = Array.new(Api::V1::OrdersController::MAX_ITEMS + 1) do + { product_id: product.id, quantity: 1 } + end + quote_draft(draft(items: items)) + + expect(response.parsed_body['error']).to include('maximum') + end + end + + # Cada cotización sale a los couriers sin crear nada (TESIS-131). En test la + # cache es :null_store y el límite nunca se alcanza; acá el contador usa una + # cache de verdad, sólo durante cada ejemplo. + describe 'rate limit' do + let(:counter) { ActiveSupport::Cache::MemoryStore.new } + + # Agota el cupo del usuario y olvida los requests que hizo para eso: lo que + # importa es qué pasa con el siguiente. + def use_up_quotes + Api::V1::DraftQuotesController::QUOTES_PER_WINDOW.times { quote_draft } + WebMock.reset_executed_requests! + end + + before do + courier + stub_request(:post, rates).to_return(status: 200, body: { precio: 2500.0, dias: 3 }.to_json) + store = Api::V1::DraftQuotesController.cache_store + allow(store).to receive(:increment) { |*args, **options| counter.increment(*args, **options) } + end + + it 'lets the quotes of the window through' do + use_up_quotes + + expect(response).to have_http_status(:ok) + end + + it 'answers 429 past the limit, without asking the couriers', :aggregate_failures do + use_up_quotes + quote_draft + + expect(response).to have_http_status(:too_many_requests) + expect(response.headers['Retry-After']).to eq('60') + expect(WebMock).not_to have_requested(:post, rates) + end + + it 'counts each user apart' do + other = User.create!(email: 'b@example.com', password: 'password123', company: company) + other_headers = auth_headers(other) + use_up_quotes + quote_draft(auth: other_headers) + + expect(response).to have_http_status(:ok) + end + end +end diff --git a/spec/requests/api/v1/shipment_dispatches_spec.rb b/spec/requests/api/v1/shipment_dispatches_spec.rb index 28b2e44..cf53e91 100644 --- a/spec/requests/api/v1/shipment_dispatches_spec.rb +++ b/spec/requests/api/v1/shipment_dispatches_spec.rb @@ -43,10 +43,10 @@ def stub_courier(status: 200, body: { numero: 'AND-999', etiqueta: 'https://l.te end def dispatch_shipment(id: shipment.id, integration_id: integration.id, - warehouse_id: warehouse.id, auth: headers) + warehouse_id: warehouse.id, auth: headers, **extra) post "/api/v1/shipments/#{id}/dispatch", params: { dispatch: { company_integration_id: integration_id, - origin_warehouse_id: warehouse_id } }, + origin_warehouse_id: warehouse_id, **extra } }, headers: auth, as: :json end @@ -85,6 +85,51 @@ def dispatch_shipment(id: shipment.id, integration_id: integration.id, end end + # El costo de la opción que el operador confirmó al cotizar (TESIS-131). Sin + # esto el detalle de la orden mostraba el envío «a cotizar» para siempre. + describe 'the confirmed shipping cost' do + before { stub_courier } + + it 'is kept on the shipment and read back from it', :aggregate_failures do + dispatch_shipment(shipping_cost: 2500.5) + expect(response.parsed_body['shipping_cost']).to eq(2500.5) + + get "/api/v1/shipments/#{shipment.id}", headers: headers + expect(response.parsed_body['shipping_cost']).to eq(2500.5) + end + + it 'leaves the cost as it was when the dispatch does not bring one' do + shipment.update!(shipping_cost: 900) + dispatch_shipment + + expect(shipment.reload.shipping_cost).to eq(900) + end + + # Se valida antes de pedir la etiqueta: el courier la cobra, y gastarla en + # un despacho que después no se puede guardar es plata tirada. + # + # Los tres últimos son números que BigDecimal acepta y la columna no + # (decimal(10,2)): antes pasaban el chequeo del controller, se pedía la + # etiqueta y el `update!` fallaba después, con el envío todavía en `pending` + # y el número de seguimiento perdido en el rollback. + { 'negative' => -1, 'not a number' => 'mucho', 'too big for the column' => 100_000_000, + 'NaN' => 'NaN', 'infinite' => 'Infinity' }.each do |label, cost| + it "answers 400 for a cost that is #{label}, without asking the courier", :aggregate_failures do + dispatch_shipment(shipping_cost: cost) + + expect(response).to have_http_status(:bad_request) + expect(WebMock).not_to have_requested(:post, 'https://andreani.test/ordenes') + expect(shipment.reload).to have_attributes(status: 'pending', tracking_number: nil) + end + end + + it 'takes the largest cost the column holds' do + dispatch_shipment(shipping_cost: '99999999.99') + + expect(shipment.reload.shipping_cost).to eq(BigDecimal('99999999.99')) + end + end + # Criterio de la card: no se puede despachar dos veces el mismo paquete. it 'returns 409 when the shipment was already dispatched' do stub_courier diff --git a/spec/requests/api/v1/shipment_quotes_spec.rb b/spec/requests/api/v1/shipment_quotes_spec.rb index da5e2fc..2bbce6f 100644 --- a/spec/requests/api/v1/shipment_quotes_spec.rb +++ b/spec/requests/api/v1/shipment_quotes_spec.rb @@ -19,12 +19,22 @@ def auth_headers(user) { 'Authorization' => "Bearer #{response.parsed_body['token']}" } end + # Un courier con sus dos plantillas: la que cotiza y la que despacha, vinculada + # a la primera (TESIS-131). Sin la de despacho la opción no se ofrecería. def courier(name, uri) - service = Service.create!(service_name: name, type: 'courier', http_method: 'POST', uri: uri, - request_mapper: { 'cp' => 'destination_zip_code' }, - response_mapper: { 'precio' => 'shipping_cost', - 'dias' => 'estimated_days' }, - request_value_mapper: {}, response_value_mapper: {}) + quote = Service.create!(service_name: "#{name} - Cotización", type: 'courier', + http_method: 'POST', uri: uri, + request_mapper: { 'cp' => 'destination_zip_code' }, + response_mapper: { 'precio' => 'shipping_cost', + 'dias' => 'estimated_days' }, + request_value_mapper: {}, response_value_mapper: {}) + integrate(Service.create!(service_name: name, type: 'courier', http_method: 'POST', + uri: "#{uri}/ordenes", quote_service: quote, + response_mapper: { 'numero' => 'tracking_number' })) + integrate(quote) + end + + def integrate(service) CompanyIntegration.create!(company: company, service: service, credentials: { 'access_token' => 'T' }, is_active: true) end @@ -58,6 +68,12 @@ def quote(warehouse_id: warehouse.id, order_id: order.id, auth: headers) expect(option['shipping_cost'].to_f).to eq(2500.0) expect(option['estimated_days']).to eq(3) end + + it 'says which integration dispatches the option' do + dispatcher = CompanyIntegration.joins(:service).find_by!(services: { service_name: 'Fast' }) + + expect(response.parsed_body['data'].first['dispatch_integration_id']).to eq(dispatcher.id) + end end # Sin opciones no es un error: el front distingue "ningún operador contestó"