diff --git a/app/controllers/api/v1/activity_controller.rb b/app/controllers/api/v1/activity_controller.rb new file mode 100644 index 00000000..9c594e0d --- /dev/null +++ b/app/controllers/api/v1/activity_controller.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Api + module V1 + # Lo último que pasó en la empresa, para el panel de la campanita + # (TESIS-162). Colección: viaja en `data` sin `meta`, porque no pagina —es + # un desplegable con un techo, no un listado (ADR-015). + class ActivityController < ApplicationController + def index + authorize :activity, :index? + # El feed no sale de un scope: son tres consultas a tres modelos, y los + # tres ya filtran por tenant —`Order` y `FailedEvent` con CompanyScoped, + # los eventos de envío por el join con `Shipment`—. No hay un + # `policy_scope` que aplicar sin inventar un modelo que no existe. + skip_policy_scope + + render json: { data: Activity::BuildFeed.new(limit: scalar_param(:limit)).call } + end + end + end +end diff --git a/app/controllers/api/v1/orders_controller.rb b/app/controllers/api/v1/orders_controller.rb index d6161fb5..6e6d5fff 100644 --- a/app/controllers/api/v1/orders_controller.rb +++ b/app/controllers/api/v1/orders_controller.rb @@ -18,7 +18,8 @@ class OrdersController < ApplicationController # Los datos del cliente que acepta el body. `items` queda afuera: se arma # aparte, línea por línea, en `items_params`. ORDER_FIELDS = %i[customer_name customer_document customer_address - customer_zip_code customer_city customer_province].freeze + customer_zip_code customer_city customer_province + requires_shipping].freeze # Campos sobre los que corre el buscador del listado (TESIS-52). Son las # formas en que un operador nombra una venta: el id con el que la conoce el diff --git a/app/controllers/api/v1/products_controller.rb b/app/controllers/api/v1/products_controller.rb index c8faae63..e5cf9b0c 100644 --- a/app/controllers/api/v1/products_controller.rb +++ b/app/controllers/api/v1/products_controller.rb @@ -8,7 +8,14 @@ class ProductsController < ApplicationController # Qué acepta el body del producto. `stocks` no entra: se arma aparte en # `stock_params`, línea por línea. - PRODUCT_FIELDS = %i[sku name description category weight dimensions].freeze + PRODUCT_FIELDS = %i[sku name description category packaging technical_standard + weight dimensions].freeze + + # Las pestañas del catálogo: cómo se llama cada una en la respuesta y con + # qué estado se filtra. «Todos» no filtra, por eso su estado es nil. El + # orden es el que muestra la pantalla. + CATALOG_TABS = { all: nil, available: 'available', low: 'low', + out_of_stock: 'out_of_stock' }.freeze before_action :set_product, only: %i[show update destroy] rescue_from ActiveRecord::RecordNotUnique, with: :render_conflict @@ -49,6 +56,17 @@ def categories render json: { data: Product::CATEGORIES } end + # Cuántos productos tiene cada pestaña del catálogo, respetando el mismo + # buscador y el mismo filtro de categoría que el listado: si no, el número + # de la pestaña y las filas que se ven dirían cosas distintas. + # + # Una consulta por estado y no cuatro requests: es lo que la pantalla + # hacía desde el cliente (TESIS-162), y cada una era un round trip. + def counts + skip_authorization + render json: { data: CATALOG_TABS.transform_values { |status| tab_count(status) } } + end + def create authorize Product @@ -83,12 +101,26 @@ def destroy # Las tres pestañas del catálogo más el buscador y la categoría. El orden # importa: `with_total_stock` arma el GROUP BY y `by_stock_status` cuelga # su HAVING de esa agregación. - def filtered_products + # + # Una sola definición de la cadena, y el estado por parámetro: el listado + # pasa el del request y cada pestaña pasa el suyo. Con dos copias, el + # invariante que la card pide —que el número de la pestaña y las filas + # visibles digan lo mismo— dependía de mantenerlas iguales a mano. + # + # `in_transit:` sale del scope cuando sólo se va a contar: la subconsulta + # es por fila y el conteo no la lee. + def filtered_products(status = scalar_param(:status), in_transit: true) policy_scope(Product) .search_catalog(scalar_param(:search)) .by_category(scalar_param(:category)) - .with_total_stock - .by_stock_status(scalar_param(:status)) + .with_total_stock(in_transit: in_transit) + .by_stock_status(status) + end + + # Cuántas filas matchean cada pestaña. `nil` es «Todos»: el scope sin + # filtro de estado. + def tab_count(status) + count_of(filtered_products(status, in_transit: false)) end # Cuántas filas matchean, sobre el scope YA filtrado. diff --git a/app/controllers/api/v1/shipment_quotes_controller.rb b/app/controllers/api/v1/shipment_quotes_controller.rb index 169546ad..3247f0f8 100644 --- a/app/controllers/api/v1/shipment_quotes_controller.rb +++ b/app/controllers/api/v1/shipment_quotes_controller.rb @@ -4,9 +4,16 @@ module Api module V1 # Cotización logística de una orden (TESIS-46). class ShipmentQuotesController < ApplicationController + rescue_from Shipments::PickupOrderError, with: :render_unprocessable + def create order = Order.find(params.expect(:order_id)) authorize order, :quote? + # Una venta de retiro no tiene a quién pedirle precio: no hay envío. Sin + # esto se cotizaba igual —y se podía despachar— una orden que después + # `CreateShipment` rechaza, o peor, una que ya tenía su envío abierto y + # pasó a retiro por PUT: una etiqueta pagada por una venta que no sale. + raise Shipments::PickupOrderError.new(order: order) unless order.requires_shipping? quotes = Shipments::QuoteShipment.for_order( order: order, origin_warehouse: origin_warehouse diff --git a/app/controllers/api/v1/shipments_controller.rb b/app/controllers/api/v1/shipments_controller.rb index b1645619..1a3d8129 100644 --- a/app/controllers/api/v1/shipments_controller.rb +++ b/app/controllers/api/v1/shipments_controller.rb @@ -11,6 +11,7 @@ class ShipmentsController < ApplicationController before_action :set_shipment, only: %i[show] rescue_from Shipments::UnshippableOrderError, with: :render_unprocessable + rescue_from Shipments::PickupOrderError, with: :render_unprocessable rescue_from Shipments::DuplicateShipmentError, with: :render_conflict rescue_from Shipments::AlreadyDispatchedError, with: :render_conflict rescue_from Shipments::InvalidCourierIntegrationError, with: :render_unprocessable diff --git a/app/controllers/api/v1/warehouses_controller.rb b/app/controllers/api/v1/warehouses_controller.rb index 2287ecc0..3de5a4f6 100644 --- a/app/controllers/api/v1/warehouses_controller.rb +++ b/app/controllers/api/v1/warehouses_controller.rb @@ -66,7 +66,7 @@ def set_warehouse # silencio las claves que no permite. (El comentario anterior decía que # respondía 400; en Rails 8.1 no es así, y el spec de company_id lo fija.) def warehouse_params - params.expect(warehouse: %i[name zip_code address]) + params.expect(warehouse: %i[name zip_code address capacity]) end # Tres cosas bloquean el borrado y el mensaje tiene que decir cuál. Se diff --git a/app/models/order.rb b/app/models/order.rb index 8e9d15e1..b979d541 100644 --- a/app/models/order.rb +++ b/app/models/order.rb @@ -5,6 +5,12 @@ class Order < ApplicationRecord STATUSES = %w[pending paid cancelled].freeze + # El estado final de una venta que no va a salir. Con nombre porque son varios + # los lugares que lo preguntan —el alta del envío, el despacho, el cálculo de + # lo comprometido, la edición y los reportes— y un literal repetido en cada + # uno es un typo esperando. + CANCELLED = 'cancelled' + # Las 24 jurisdicciones de la Argentina, con el nombre con el que se muestran # en el select de provincia del alta manual (TESIS-58). No es una lista # citable como nombres oficiales: Tierra del Fuego, por ejemplo, se llama @@ -44,6 +50,15 @@ class Order < ApplicationRecord validates :total_amount, numericality: { greater_than_or_equal_to: 0 }, allow_nil: true # allow_nil: las órdenes anteriores a TESIS-128 y las de webhook no la tienen. validates :customer_province, inclusion: { in: PROVINCES }, allow_nil: true + # `inclusion` y no `presence`: la columna es NOT NULL y el campo es editable + # por la API (TESIS-162), así que un `null` llegaba a la base y reventaba con + # NotNullViolation -> 500. `presence` tampoco serviría: `false` es un valor + # válido y para `presence` es ausencia. + # + # Un texto cualquiera sigue entrando como `true`: el casteo a booleano lo + # hace ActiveRecord **antes** de validar, así que desde acá no se distingue + # de un `true` legítimo. Es el comportamiento de Rails en toda la API. + validates :requires_shipping, inclusion: { in: [true, false] } validate :company_integration_belongs_to_company def display_name diff --git a/app/models/product.rb b/app/models/product.rb index 2d97386e..b8b236a7 100644 --- a/app/models/product.rb +++ b/app/models/product.rb @@ -24,6 +24,12 @@ class Product < ApplicationRecord # Los tres estados de disponibilidad, en el vocabulario de la pantalla. STOCK_STATUSES = %w[out_of_stock low available].freeze + # Un envío en este estado todavía no salió, así que sus unidades siguen + # físicamente en el depósito aunque ya estén vendidas. Una orden sin envío + # —todavía no se abrió, o es un retiro en el local— está en la misma + # situación, y por eso el filtro acepta también el NULL del LEFT JOIN. + UNDISPATCHED_SHIPMENT_STATUSES = [nil, 'pending'].freeze + # Unidades en vuelo hacia/desde depósitos, como subconsulta escalar. # # Subconsulta y no un segundo left_joins: `with_total_stock` ya hace join con @@ -53,11 +59,14 @@ class Product < ApplicationRecord # tienen ninguna y no hay con qué inferirla. validates :category, inclusion: { in: CATEGORIES }, allow_nil: true - scope :with_total_stock, lambda { - left_joins(:stocks) - .group(:id) - .select('products.*', 'COALESCE(SUM(stocks.quantity), 0) AS total_stock', - "(#{IN_TRANSIT_SUBQUERY}) AS in_transit_quantity") + # `in_transit:` lo pide quien va a leerlo. Es una subconsulta correlacionada + # —una por fila— y los contadores de las pestañas no la miran: pedirla ahí + # era pagarla cuatro veces para descartarla. + scope :with_total_stock, lambda { |in_transit: true| + columnas = ['products.*', 'COALESCE(SUM(stocks.quantity), 0) AS total_stock'] + columnas << "(#{IN_TRANSIT_SUBQUERY}) AS in_transit_quantity" if in_transit + + left_joins(:stocks).group(:id).select(*columnas) } # Filtro por disponibilidad, para las pestañas del catálogo. @@ -139,6 +148,26 @@ def in_transit_quantity stock_transfers.in_flight.sum(:quantity) end + # Unidades vendidas que todavía no salieron del depósito, por depósito. + # + # El stock se descuenta al **crear** la orden (`Catalog::DeductStock`) y el + # despacho no vuelve a tocar `stocks`, así que estas unidades ya no figuran en + # ninguna fila de stock pero siguen estando en el estante hasta que el courier + # se las lleva. Son las que el detalle muestra como «Comprometido». + # + # Las líneas sin depósito (anteriores a TESIS-126) quedan afuera: no se pueden + # atribuir a ninguno, y contarlas en el total pero en ningún depósito dejaría + # una pantalla cuyas filas no suman el encabezado. + def committed_by_warehouse + @committed_by_warehouse ||= committed_scope + .group(:warehouse_id, 'warehouses.name') + .order(:warehouse_id) + .sum(:quantity) + .map do |(warehouse_id, name), quantity| + { warehouse_id: warehouse_id, name: name, quantity: quantity.to_i } + end + end + # Unidades en vuelo hacia cada depósito: lo que todavía no figura en ningún # número del destino. El saliente no va: ya está descontado del on hand del # origen al despachar, y mostrarlo en esa fila se leería como si siguiera ahí. @@ -159,6 +188,20 @@ def in_transit_by_warehouse end end + # Lo comprometido de todo el producto. Suma el desglose en vez de volver a la + # base: es el mismo número por definición, y así no puede discrepar con las + # filas que muestra la pantalla. + def committed_quantity + committed_by_warehouse.sum { |row| row[:quantity] } + end + + # Lo que hay físicamente: lo que queda libre más lo vendido sin despachar. + def on_hand_quantity = total_stock + committed_quantity + + # Lo que se puede prometer es lo que queda libre: lo vendido ya se descontó de + # `stocks` al crear la orden, así que no hay que volver a restarlo. + def available_to_promise = total_stock + # Depósito donde está el grueso de las unidades. Lo consume la columna # "Location Node" del listado, que muestra un nodo y no el desglose. # @@ -174,4 +217,34 @@ def primary_stock stocks.reject { |stock| stock.quantity.zero? } .min_by { |stock| [-stock.quantity, stock.warehouse_id] } end + + private + + # Las líneas que cuentan como comprometidas. El LEFT JOIN con `shipments` es + # lo que deja entrar a las órdenes que todavía no tienen envío; un INNER las + # dejaría afuera, que es justo el caso más común apenas entra una venta. + # + # **Los retiros en el local no cuentan.** Lo comprometido es lo vendido que + # todavía está en el estante, y lo que lo saca de ahí es el despacho. Una + # venta de retiro no tiene envío —`CreateShipment` lo rechaza— y `Order` + # no tiene un estado «retirada», así que nada la cerraría nunca: quedaría + # comprometida para siempre y el «En depósito» crecería con cada retiro. + # + # La decisión es tratar el mostrador como lo que es: registrar la venta y + # entregarla son el mismo momento, el cliente está ahí. Las unidades salen + # del estante al crear la orden, que es exactamente cuando `DeductStock` las + # saca de `stocks`. Así los dos números dicen lo mismo y no hace falta un + # estado nuevo. + # + # Lo que sí queda abierto es la cancelación: sus unidades vuelven al estante + # pero nada las devuelve a `stocks`, así que no las cuenta ni este scope ni + # `total_stock`. Eso lo cierra TESIS-168, que es la card que repone el + # stock al cancelar. + def committed_scope + order_items.joins(:order, :warehouse) + .left_outer_joins(order: :shipment) + .where.not(orders: { status: Order::CANCELLED }) + .where(orders: { requires_shipping: true }) + .where(shipments: { status: UNDISPATCHED_SHIPMENT_STATUSES }) + end end diff --git a/app/models/warehouse.rb b/app/models/warehouse.rb index 95fe087b..59dc0a0a 100644 --- a/app/models/warehouse.rb +++ b/app/models/warehouse.rb @@ -38,7 +38,24 @@ class Warehouse < ApplicationRecord foreign_key: :destination_warehouse_id, dependent: :restrict_with_error + # Lo que entra en la columna `integer` de la base. Declarar más unidades de + # las que un entero de 4 bytes puede guardar no es una capacidad: es un error + # de carga, y se responde como tal. + MAX_CAPACITY = 2_147_483_647 + validates :name, presence: true + # Capacidad declarada del depósito, en unidades. Opcional: un depósito sin + # capacidad cargada no es un error, es uno del que nadie declaró el techo + # todavía, y la pantalla no dibuja su barra de ocupación. Mayor que cero + # porque un depósito de capacidad cero no podría guardar nada. + # + # El techo no es cosmético: la columna es `integer` de 4 bytes, así que un + # número más grande pasaba la validación y reventaba al guardar con + # ActiveModel::RangeError -> 500. Mismo criterio que `Shipment::MAX_SHIPPING_COST`. + validates :capacity, + numericality: { only_integer: true, greater_than: 0, + less_than_or_equal_to: MAX_CAPACITY }, + allow_nil: true validates :zip_code, presence: true validates :address, presence: true diff --git a/app/policies/activity_policy.rb b/app/policies/activity_policy.rb new file mode 100644 index 00000000..0f0ab9d6 --- /dev/null +++ b/app/policies/activity_policy.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +# El feed no tiene modelo propio: se arma con órdenes, eventos de envío y +# eventos fallidos, y los tres ya están scopeados por tenant. La policy existe +# igual para que el controller no tenga que saltear la verificación de Pundit +# —`verify_authorized` está activo en toda la API— y para que quede un lugar +# donde apretar el permiso si algún día el feed deja de ser para cualquiera. +class ActivityPolicy < ApplicationPolicy + def index? = user.present? +end diff --git a/app/poros/activity/build_feed.rb b/app/poros/activity/build_feed.rb new file mode 100644 index 00000000..7d2eaeec --- /dev/null +++ b/app/poros/activity/build_feed.rb @@ -0,0 +1,129 @@ +# frozen_string_literal: true + +module Activity + # Lo último que pasó en la empresa, en una sola lista ordenada por fecha: es lo + # que muestra el panel de la campanita (TESIS-162) y el «feed de actividad + # reciente» que RF-26 compromete para el panel de control. + # + # No hay tabla de notificaciones ni estado de leído por usuario a propósito: + # eso es otro dominio —leídas por persona, vencimiento, preferencias— y nadie + # lo pidió. Acá la actividad se **deriva** de lo que el sistema ya registra, + # así que no puede desincronizarse de los hechos ni hay nada que mantener. + # + # Un endpoint y no tres: cada fuente es una consulta, pero la pantalla hace un + # solo request y recibe la lista ya ordenada. Pedir tres y mezclarlas en el + # cliente obligaría a que el front supiera cómo se ordena una actividad. + class BuildFeed < ApplicationPoro + DEFAULT_LIMIT = 20 + # Techo del `limit`: es un panel desplegable, no un listado paginado. Sin + # tope, un `?limit=` grande haría tres consultas sin límite. + MAX_LIMIT = 50 + + # Las ventas que entraron, los envíos que salieron y lo que se cayó a la + # cola de reintentos. Son las tres cosas sobre las que el operador actúa. + TYPES = %w[order_created shipment_dispatched event_failed].freeze + + DISPATCH_STATUS = Shipments::ConfirmDispatch::DISPATCHED_STATUS + + # Los estados de la cola sobre los que todavía hay algo que hacer. Uno que + # se reprocesó bien (`succeeded`) o que alguien descartó a mano + # (`discarded`) ya no es una novedad: mostrarlo volvería a avisar de un + # problema resuelto y, peor, ocuparía el cupo del feed desplazando a los + # que sí están abiertos. + OPEN_FAILURE_STATUSES = %w[pending processing dead].freeze + + def initialize(limit: nil) + super() + @limit = normalize_limit(limit) + end + + # Se traen las últimas `limit` de cada fuente y recién después se mezclan: + # cualquiera de las tres podría ser toda la lista, así que ninguna puede + # traer menos. + def call + (recent_orders + recent_dispatches + recent_failures) + .sort_by { |event| [-event[:occurred_at].to_f, event[:id]] } + .first(@limit) + end + + private + + # Un `limit` que no es un número vuelve al default y no a uno: `'muchas'.to_i` + # es 0, y acotarlo devolvería una sola fila, que es una respuesta rara para + # un parámetro que nadie escribió a propósito. Lo que sí es un número se + # acota, con el mismo criterio que la paginación (TESIS-155). + def normalize_limit(limit) + value = limit.to_s.strip + return DEFAULT_LIMIT unless value.match?(/\A\d+\z/) + + value.to_i.clamp(1, MAX_LIMIT) + end + + # `Order` es CompanyScoped: el feed es del tenant del request sin filtrar a + # mano, igual que el resto de la API. + def recent_orders + Order.order(created_at: :desc, id: :desc).limit(@limit).map do |order| + { + id: "order-#{order.id}", type: 'order_created', occurred_at: order.created_at, + order_id: order.id, customer_name: order.customer_name, + external_order_id: order.external_order_id, + total_amount: order.total_amount&.to_f + } + end + end + + # El despacho no deja una marca de tiempo propia en `shipments`: lo dice el + # primer evento `ready_to_ship` de la bitácora, que es el que escribe + # `ConfirmDispatch`. Mismo criterio que usan los reportes. + # + # **Uno por envío, el más temprano.** `ready_to_ship` no aparece una sola + # vez: `RegisterTrackingEvent` guarda con el estado actual del envío + # cualquier push que la plantilla del courier no sepa traducir, y después + # del despacho ese estado es `ready_to_ship`. Sin el `DISTINCT ON`, un + # «paquete recibido en sucursal» entra al feed como un segundo despacho del + # mismo envío, con la hora del push, y las repeticiones ocupan el cupo + # desplazando a los despachos de otros envíos. + # + # `ShipmentEvent` no es CompanyScoped —no tiene company_id— así que el + # aislamiento entra por el join con `Shipment`, que sí lo es. + def recent_dispatches + ShipmentEvent.from(first_dispatch_per_shipment, :shipment_events) + .includes(shipment: { company_integration: :service }) + .order(occurred_at: :desc, id: :desc).limit(@limit) + .map { |event| dispatch_entry(event, event.shipment) } + end + + # El `ORDER BY` arranca por `shipment_id` porque Postgres lo exige para el + # `DISTINCT ON`; `occurred_at` decide cuál de las filas de cada envío queda, + # y el `id` desempata dos eventos de la misma hora. + def first_dispatch_per_shipment + ShipmentEvent.where(internal_status: DISPATCH_STATUS) + .joins(:shipment).merge(Shipment.all) + .select('DISTINCT ON (shipment_events.shipment_id) shipment_events.*') + .order(:shipment_id, :occurred_at, :id) + end + + def dispatch_entry(event, shipment) + { + id: "dispatch-#{event.id}", type: 'shipment_dispatched', occurred_at: event.occurred_at, + shipment_id: shipment.id, order_id: shipment.order_id, + tracking_number: shipment.tracking_number, + courier: shipment.company_integration&.service_name + } + end + + # Igual que las órdenes: `FailedEvent` es CompanyScoped. Se filtran los + # estados abiertos: ver `OPEN_FAILURE_STATUSES`. + def recent_failures + FailedEvent.where(status: OPEN_FAILURE_STATUSES) + .includes(company_integration: :service) + .order(created_at: :desc, id: :desc).limit(@limit).map do |failure| + { + id: "failure-#{failure.id}", type: 'event_failed', occurred_at: failure.created_at, + failed_event_id: failure.id, event_type: failure.event_type, + status: failure.status, integration: failure.company_integration&.service_name + } + end + end + end +end diff --git a/app/poros/catalog/product_version.rb b/app/poros/catalog/product_version.rb index cc6a83e1..5adbdd49 100644 --- a/app/poros/catalog/product_version.rb +++ b/app/poros/catalog/product_version.rb @@ -33,8 +33,13 @@ def fingerprint (edited_fields + stock_pairs).join(SEPARATOR) end + # Todo lo que `ProductsController::PRODUCT_FIELDS` deja editar menos el + # `sku`, que es inmutable. Un campo editable que no entre acá no mueve la + # versión: dos operadores que lo cambian a la vez no reciben el 412 y el + # segundo revierte al primero sin enterarse. def edited_fields - [@product.name.to_s, @product.description.to_s, + [@product.name.to_s, @product.description.to_s, @product.category.to_s, + @product.packaging.to_s, @product.technical_standard.to_s, @product.weight.to_s, @product.dimensions.to_s] end diff --git a/app/poros/orders/order_not_editable_error.rb b/app/poros/orders/order_not_editable_error.rb index 61533142..aba93c8e 100644 --- a/app/poros/orders/order_not_editable_error.rb +++ b/app/poros/orders/order_not_editable_error.rb @@ -18,7 +18,7 @@ def initialize(order:) # lo asigna el courier al confirmar (TESIS-47). Mismo criterio que # Shipments::AlreadyDispatchedError, y el mensaje nombra lo que bloquea. def reason(order) - return 'a cancelled order cannot be modified' if order.status == 'cancelled' + return 'a cancelled order cannot be modified' if order.status == Order::CANCELLED shipment = order.shipment if shipment.status == 'pending' diff --git a/app/poros/orders/order_version.rb b/app/poros/orders/order_version.rb index 15c24138..8abacbcc 100644 --- a/app/poros/orders/order_version.rb +++ b/app/poros/orders/order_version.rb @@ -13,8 +13,13 @@ module Orders # la misma orden no tocan `orders` hasta que se recalcula el total. class OrderVersion < ApplicationPoro SEPARATOR = '|' + # Todo lo que `OrdersController::ORDER_FIELDS` deja editar, más el estado. + # Un campo editable fuera de esta lista no mueve la versión, así que dos + # operadores que lo cambian a la vez no reciben el 412 y el segundo revierte + # al primero en silencio. HEADER_FIELDS = %i[customer_name customer_document customer_address - customer_zip_code customer_city customer_province status].freeze + customer_zip_code customer_city customer_province + requires_shipping status].freeze def initialize(order:) super() diff --git a/app/poros/orders/process_webhook_order.rb b/app/poros/orders/process_webhook_order.rb index 08013552..fd0fbee1 100644 --- a/app/poros/orders/process_webhook_order.rb +++ b/app/poros/orders/process_webhook_order.rb @@ -21,7 +21,6 @@ class UnmappedProductError < StandardError; end MISSING_ITEMS = 'the payload does not carry any order item' UNREADABLE_ITEMS = 'the template could not read %d of the order items in the payload' ORDERS_UNIQUE_INDEX = 'index_orders_on_integration_and_external_order_id' - CANCELLED = 'cancelled' def initialize(webhook_log:) super() @@ -107,7 +106,7 @@ def register_item(order, item, mapping) # que las anteriores a TESIS-126, y como la orden ya está cancelada nada va a # intentar devolverle unidades. def take_units(order, product, quantity) - return if order.status == CANCELLED + return if order.status == Order::CANCELLED Catalog::DeductStock.new(product: product, quantity: quantity).call.warehouse_id end @@ -152,7 +151,24 @@ def order_attributes .slice(:external_order_id, :customer_name, :customer_document, :customer_address, :customer_zip_code, :customer_city) .merge(company_id: @log.company_id, company_integration: @log.company_integration, - status: status, customer_province: province) + status: status, customer_province: province, + requires_shipping: requires_shipping?) + end + + # Si la venta se despacha o la retira el cliente (TESIS-162). Lo informa la + # plantilla del canal cuando el payload lo trae —Shopify lo manda, Mercado + # Libre no siempre—; cuando no dice nada, se asume que hay envío. + # + # El default es `true` y no `false` a propósito: asumir envío y que sobre + # deja una orden lista para despachar que nadie despacha, y eso se ve en la + # pantalla. Asumir retiro y que falte deja una venta que había que mandar + # sin ninguna señal de que falta hacerlo, y eso no se ve hasta que reclama + # el comprador. + def requires_shipping? + value = translated[:order][:requires_shipping] + return true if value.nil? + + ActiveModel::Type::Boolean.new.cast(value) != false end # La provincia se valida contra Order::PROVINCES, y cada canal la escribe a diff --git a/app/poros/orders/translate_webhook_payload.rb b/app/poros/orders/translate_webhook_payload.rb index e780c88c..f7edd409 100644 --- a/app/poros/orders/translate_webhook_payload.rb +++ b/app/poros/orders/translate_webhook_payload.rb @@ -14,7 +14,7 @@ class TranslateWebhookPayload < ApplicationPoro # flujos (ej. tracking_number en la respuesta de un courier). ORDER_KEYS = %w[external_order_id customer_name customer_document customer_address customer_zip_code customer_city - customer_province status].freeze + customer_province status requires_shipping].freeze ITEM_KEYS = %w[external_product_id quantity unit_price].freeze def initialize(service:, payload:) diff --git a/app/poros/orders/update_order.rb b/app/poros/orders/update_order.rb index 6fd11625..3c3bd752 100644 --- a/app/poros/orders/update_order.rb +++ b/app/poros/orders/update_order.rb @@ -75,7 +75,7 @@ def validate_status! # El envío `pending` sin número de seguimiento todavía no salió, y ése sí # admite cambios. def ensure_editable! - return unless @order.status == 'cancelled' || dispatched? + return unless @order.status == Order::CANCELLED || dispatched? raise OrderNotEditableError.new(order: @order) end diff --git a/app/poros/reports/build_overview.rb b/app/poros/reports/build_overview.rb index 34848089..4b02f0b7 100644 --- a/app/poros/reports/build_overview.rb +++ b/app/poros/reports/build_overview.rb @@ -55,7 +55,7 @@ def trended # Las ventas que cuentan: las canceladas no facturaron. def sales(range) - Order.where(created_at: range).where.not(status: 'cancelled') + Order.where(created_at: range).where.not(status: Order::CANCELLED) end # `shipments` no guarda cuándo se despachó: lo dice el primer evento de la diff --git a/app/poros/shipments/confirm_dispatch.rb b/app/poros/shipments/confirm_dispatch.rb index c7d02153..9bf9f766 100644 --- a/app/poros/shipments/confirm_dispatch.rb +++ b/app/poros/shipments/confirm_dispatch.rb @@ -75,10 +75,15 @@ def call # 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. + # El estado va primero: una orden cancelada **de retiro** tiene que decir + # que está cancelada, que es lo accionable, y no «esta venta se retira en el + # local». Separar los dos errores fue justamente para no confundirlos. def validate_order! - return unless CreateShipment::NON_SHIPPABLE_STATUSES.include?(order.status) + raise UnshippableOrderError.new(order: order) if + CreateShipment::NON_SHIPPABLE_STATUSES.include?(order.status) + return if order.requires_shipping? - raise UnshippableOrderError.new(order: order) + raise PickupOrderError.new(order: order) end # Se valida antes de llamar al courier para no gastar una etiqueta —que el diff --git a/app/poros/shipments/create_shipment.rb b/app/poros/shipments/create_shipment.rb index 003d708b..2de39a31 100644 --- a/app/poros/shipments/create_shipment.rb +++ b/app/poros/shipments/create_shipment.rb @@ -19,7 +19,7 @@ class CreateShipment < ApplicationPoro # inalcanzable justo para el alta manual que necesita el wizard de TESIS-59. # Cuando exista la transición de estados, esta constante es el único lugar a # tocar. - NON_SHIPPABLE_STATUSES = %w[cancelled].freeze + NON_SHIPPABLE_STATUSES = [Order::CANCELLED].freeze # Redundante con el default de la columna (TESIS-45) y a propósito: el estado # inicial es parte del contrato de este caso de uso, no un detalle del schema. @@ -38,10 +38,15 @@ def call private + # El estado va primero: una orden cancelada de retiro tiene que responder + # que está cancelada y no «esta venta se retira en el local», que es la + # confusión que motivó separar los dos errores. def validate_status! - return unless NON_SHIPPABLE_STATUSES.include?(@order.status) + raise UnshippableOrderError.new(order: @order) if + NON_SHIPPABLE_STATUSES.include?(@order.status) + return if @order.requires_shipping? - raise UnshippableOrderError.new(order: @order) + raise PickupOrderError.new(order: @order) end # Una sola escritura: sin transacción explícita, porque un choque contra la diff --git a/app/poros/shipments/pickup_order_error.rb b/app/poros/shipments/pickup_order_error.rb new file mode 100644 index 00000000..f9950826 --- /dev/null +++ b/app/poros/shipments/pickup_order_error.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Shipments + # La orden la retira el cliente en el local, así que no entra al circuito + # logístico: no hay envío que abrir ni etiqueta que emitir (TESIS-162). + # + # Es un error de datos y no transitorio, igual que `UnshippableOrderError`: + # reintentar no cambia nada mientras la orden siga siendo un retiro. El + # controller lo mapea a 422. + # + # Va aparte y no como otro estado de `UnshippableOrderError` porque el motivo + # es distinto y la pantalla hace cosas distintas con cada uno: una orden + # cancelada es un callejón sin salida, un retiro es una venta sana que + # simplemente no se despacha. + class PickupOrderError < StandardError + attr_reader :order_id + + def initialize(order:) + @order_id = order.id + super('an order picked up at the store has no shipment') + end + end +end diff --git a/app/serializers/order_list_serializer.rb b/app/serializers/order_list_serializer.rb index e9594d8f..bc33c49b 100644 --- a/app/serializers/order_list_serializer.rb +++ b/app/serializers/order_list_serializer.rb @@ -10,7 +10,8 @@ class OrderListSerializer < ApplicationSerializer # vienen desde el principio; la ciudad y la provincia desde TESIS-128, así que # las órdenes anteriores y las de webhook las traen en null. fields :customer_name, :customer_document, :customer_address, :customer_zip_code, - :customer_city, :customer_province, :external_order_id, :status, :created_at, :updated_at + :customer_city, :customer_province, :external_order_id, :status, :requires_shipping, + :created_at, :updated_at # La columna Total del listado (TESIS-52). Sale de la columna persistida y no # de sumar las líneas: sumarlas por fila sería un SELECT por orden, y además diff --git a/app/serializers/order_serializer.rb b/app/serializers/order_serializer.rb index a44b895a..2de3167f 100644 --- a/app/serializers/order_serializer.rb +++ b/app/serializers/order_serializer.rb @@ -4,7 +4,8 @@ class OrderSerializer < ApplicationSerializer identifier :id fields :customer_name, :customer_document, :customer_address, :customer_zip_code, - :customer_city, :customer_province, :external_order_id, :status, :created_at, :updated_at + :customer_city, :customer_province, :external_order_id, :status, :requires_shipping, + :created_at, :updated_at # Mismo criterio que unit_price: decimal en la DB, número en el JSON. Puede # ser nil en órdenes anteriores a TESIS-114 que no tienen líneas. diff --git a/app/serializers/product_serializer.rb b/app/serializers/product_serializer.rb index 8294bef7..c2e909bd 100644 --- a/app/serializers/product_serializer.rb +++ b/app/serializers/product_serializer.rb @@ -3,8 +3,18 @@ class ProductSerializer < ApplicationSerializer identifier :id - fields :sku, :name, :description, :category, :dimensions, :total_stock, - :in_transit_quantity, :created_at, :updated_at + fields :sku, :name, :description, :category, :packaging, :technical_standard, + :dimensions, :total_stock, :in_transit_quantity, :created_at, :updated_at + + # Los tres números del stock que muestra el detalle (TESIS-162). Van los tres + # desde el backend y ninguno se deriva en el cliente: `on_hand` no es + # `total_stock` —lo vendido sin despachar sigue en el estante pero ya salió de + # `stocks`— y restarlos mal del lado del front es el error que esta card viene + # a cerrar. + fields :committed_quantity, :on_hand_quantity, :available_to_promise + + # Lo comprometido por depósito, con la misma forma que `in_transit_by_warehouse`. + field :committed_by_warehouse # weight es decimal en la DB y BigDecimal se serializa como string por # defecto; exponerlo como número evita que el front tenga que parsear. diff --git a/app/serializers/warehouse_serializer.rb b/app/serializers/warehouse_serializer.rb index 292a4981..f9360eff 100644 --- a/app/serializers/warehouse_serializer.rb +++ b/app/serializers/warehouse_serializer.rb @@ -2,7 +2,9 @@ class WarehouseSerializer < ApplicationSerializer identifier :id - fields :name, :zip_code, :address + # `capacity` puede ser null y eso significa algo: nadie declaró el techo de + # este depósito todavía. No es cero, que diría que no entra nada. + fields :name, :zip_code, :address, :capacity # Unidades guardadas en el deposito. Alimenta el widget de capacidad del panel # (TESIS-55): la barra compara depositos entre si, no contra una capacidad diff --git a/bin/worker_windows.rb b/bin/worker_windows.rb new file mode 100644 index 00000000..bdf0d4de --- /dev/null +++ b/bin/worker_windows.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +# Worker de Solid Queue para Windows. +# +# `bin/jobs` no arranca acá: el supervisor registra SIGQUIT, que no existe en la +# plataforma (architecture.md §8.3). Esto levanta un worker en el proceso +# actual, sin supervisor ni fork, que es todo lo que hace falta para ver correr +# la ingesta de webhooks y el sync de stock. +# +# bundle exec rails runner bin/worker_windows.rb +# +# Corta con Ctrl-C. + +COLAS = ENV.fetch('QUEUES', '*') + +worker = SolidQueue::Worker.new(queues: COLAS, threads: 3, polling_interval: 0.5) +corriendo = true + +# El handler no hace más que bajar la bandera. `worker.stop` puede terminar +# tomando un lock —al instrumentar con ActiveSupport::Notifications— y Ruby +# prohíbe tomar locks en contexto de trap: saldría un ThreadError en vez de un +# corte limpio. El worker se para abajo, en el hilo principal. +Signal.trap('INT') { corriendo = false } + +puts "[worker] escuchando las colas: #{COLAS}" +puts '[worker] Ctrl-C para cortar' + +# `start` levanta los hilos y vuelve enseguida: sin esta espera el proceso +# termina y el worker se muere sin tomar ningun job. +worker.start +sleep 0.5 while corriendo +worker.stop +puts '[worker] terminado' diff --git a/config/routes.rb b/config/routes.rb index a97bbdfd..c2fc9d63 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -30,6 +30,12 @@ # Vocabulario de categorías: ruta de colección, no depende de un producto. get :categories, on: :collection + # Cuántos productos cae en cada pestaña del catálogo, en una sola + # respuesta (TESIS-162). Ruta propia y no dentro del `meta` del listado: + # los contadores no cambian al pasar de página, así que el cliente los + # pide una vez y los invalida recién cuando algo muta. + get :counts, on: :collection + resources :mappings, only: %i[index create destroy], controller: 'product_mappings' end @@ -69,6 +75,11 @@ post :dispatch, on: :member, action: :confirm end + # Actividad reciente de la empresa (TESIS-162): la lista que abre la + # campanita. Recurso singular en plural por costumbre REST, pero sin id: + # es una vista agregada, no una colección de filas propias. + resources :activity, only: %i[index] + resources :failed_events, path: 'failed-events', only: %i[index] do member do post :retry, action: :requeue diff --git a/db/migrate/20261004194406_add_capacity_to_warehouses.rb b/db/migrate/20261004194406_add_capacity_to_warehouses.rb new file mode 100644 index 00000000..c31b032d --- /dev/null +++ b/db/migrate/20261004194406_add_capacity_to_warehouses.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +# Capacidad del depósito, en unidades. La barra de ocupación del detalle de +# producto compara lo guardado contra un techo, y ese techo no se puede derivar +# de ningún dato: lo sabe quien conoce el depósito. +# +# Nullable a propósito: los depósitos que ya existen no tienen una capacidad que +# alguien haya declarado, y poner un número por defecto sería inventarlo. Sin +# capacidad cargada, la pantalla no dibuja la barra (TESIS-163). +class AddCapacityToWarehouses < ActiveRecord::Migration[8.1] + def change + add_column :warehouses, :capacity, :integer + end +end diff --git a/db/migrate/20261004194411_add_packaging_and_technical_standard_to_products.rb b/db/migrate/20261004194411_add_packaging_and_technical_standard_to_products.rb new file mode 100644 index 00000000..8ab0eed8 --- /dev/null +++ b/db/migrate/20261004194411_add_packaging_and_technical_standard_to_products.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +# Empaque y norma técnica del producto. El detalle los muestra desde el diseño +# y hasta ahora decía que no existían en la API. +# +# Texto libre y no un vocabulario cerrado como `category`: una norma técnica es +# un código de un organismo externo (IRAM, IEC) y el empaque describe el bulto +# con las palabras del rubro. Encerrarlos en una lista nuestra dejaría afuera el +# caso que importa el día que aparezca. +class AddPackagingAndTechnicalStandardToProducts < ActiveRecord::Migration[8.1] + def change + add_column :products, :packaging, :string + add_column :products, :technical_standard, :string + end +end diff --git a/db/migrate/20261004194416_add_requires_shipping_to_orders.rb b/db/migrate/20261004194416_add_requires_shipping_to_orders.rb new file mode 100644 index 00000000..11667070 --- /dev/null +++ b/db/migrate/20261004194416_add_requires_shipping_to_orders.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +# Si la orden se despacha o la retira el cliente. Hasta ahora toda orden se +# asumía despachada, y no había forma de registrar una venta con retiro en el +# local (TESIS-162). +# +# `default: true` y `null: false`: las órdenes que ya existen se despachan —es +# lo único que el sistema sabía hacer— y el default cubre el caso en que el +# canal no informa nada. Asumir envío y que sobre es recuperable; asumir retiro +# y que falte deja una venta sin despachar sin que nadie se entere. +class AddRequiresShippingToOrders < ActiveRecord::Migration[8.1] + def change + add_column :orders, :requires_shipping, :boolean, default: true, null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 1b64168b..a966fa80 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_10_02_045858) do +ActiveRecord::Schema[8.1].define(version: 2026_10_04_194416) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" @@ -108,6 +108,7 @@ t.string "customer_province" t.string "customer_zip_code" t.string "external_order_id" + t.boolean "requires_shipping", default: true, null: false t.string "status", default: "pending", null: false t.decimal "total_amount", precision: 10, scale: 2 t.datetime "updated_at", null: false @@ -139,7 +140,9 @@ t.text "description" t.string "dimensions" t.string "name", null: false + t.string "packaging" t.string "sku", null: false + t.string "technical_standard" t.datetime "updated_at", null: false t.decimal "weight", precision: 10, scale: 2, default: "0.0" t.index ["company_id", "category"], name: "index_products_on_company_id_and_category" @@ -259,6 +262,7 @@ create_table "warehouses", force: :cascade do |t| t.string "address", null: false + t.integer "capacity" t.bigint "company_id", null: false t.datetime "created_at", null: false t.string "name", null: false diff --git a/db/seeds.rb b/db/seeds.rb index ab806e10..eabcb54e 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -35,7 +35,12 @@ { email: 'operador@norte.com', password: 'password123' } ], warehouses: [ - { name: 'Depósito Central', zip_code: '1900', address: 'Av. 7 N° 1234, La Plata' }, + # `capacity` es la capacidad declarada (TESIS-162): la barra de ocupación + # del detalle de producto compara lo guardado contra este techo. El + # satélite va sin declarar a propósito, para que se vea el caso en que la + # pantalla no dibuja la barra porque nadie cargó el número. + { name: 'Depósito Central', zip_code: '1900', address: 'Av. 7 N° 1234, La Plata', + capacity: 6_000 }, { name: 'Depósito Satélite Norte', zip_code: '1602', address: 'Calle 25 N° 456, Florida' } ] }, @@ -119,10 +124,17 @@ end attrs[:warehouses].each do |warehouse_attrs| - Warehouse.find_or_create_by!(name: warehouse_attrs[:name], company: company) do |w| + warehouse = Warehouse.find_or_create_by!(name: warehouse_attrs[:name], company: company) do |w| w.zip_code = warehouse_attrs[:zip_code] w.address = warehouse_attrs[:address] end + + # Fuera del bloque de creación, por el mismo motivo que la categoría de los + # productos: así las bases ya sembradas antes de que existiera la columna + # también quedan con capacidad. El `if` no pisa una cargada a mano. + if warehouse.capacity.nil? && warehouse_attrs[:capacity] + warehouse.update!(capacity: warehouse_attrs[:capacity]) + end end end @@ -353,6 +365,13 @@ 'line_items[].variant_id' => 'external_product_id', 'line_items[].quantity' => 'quantity', 'line_items[].price' => 'unit_price' + # `requires_shipping` NO se mapea, y por eso toda venta que entra por acá + # queda como envío (ProcessWebhookOrder asume `true` cuando la plantilla + # no lo declara). Shopify no manda un booleano único: lo más cercano es + # la ausencia de `shipping_lines`, que el formato de la plantilla + # —camino de origen a campo— no sabe expresar. Un canal que quiera + # registrar retiros tiene que declararlo en su response_mapper; hasta + # entonces el retiro es sólo para el alta manual. }, request_value_mapper: {}, # Estados de pago de Shopify que no se llaman igual en el OMS. `paid` y @@ -573,6 +592,15 @@ { celular => 'Electronics', notebook => 'Electronics', mouse => 'Electronics' } .each { |product, category| product.update!(category: category) if product.category.nil? } + # Empaque y norma técnica (TESIS-162). Mismo criterio que la categoría: fuera + # del bloque de creación, y sólo si no están, para no pisar lo cargado a mano. + { celular => ['Caja individual', 'IRAM 4220'], + notebook => ['Caja con separadores x4', 'IEC 62368-1'], + mouse => ['Blíster x12', 'IRAM 2063'] }.each do |product, (packaging, standard)| + product.update!(packaging: packaging) if product.packaging.nil? + product.update!(technical_standard: standard) if product.technical_standard.nil? + end + # Stock en depósitos de Norte central = Warehouse.find_by(company: norte_company, name: 'Depósito Central') satelite = Warehouse.find_by(company: norte_company, name: 'Depósito Satélite Norte') @@ -794,6 +822,35 @@ end end +# Venta de Norte que el cliente retira en el local (TESIS-162): no entra al +# circuito logístico, así que el detalle no ofrece crearle un envío y la API +# rechaza el alta con 422. Es el caso que distingue «esta orden no lleva envío» +# de «a esta orden le falta el envío», que hasta ahora se veían igual. +if norte_company && celular + retiro = Order.find_or_create_by!( + company: norte_company, external_order_id: nil, customer_name: 'Retiro en Mostrador' + ) do |o| + o.customer_document = '27-35123456-4' + o.status = 'paid' + o.requires_shipping = false + end + + central = Warehouse.find_by(company: norte_company, name: 'Depósito Central') + + OrderItem.find_or_create_by!(order: retiro, product: celular) do |i| + i.quantity = 1 + i.unit_price = 285_000.00 + i.warehouse = central + # El alta real descuenta el stock (`Catalog::DeductStock`); el seed escribe + # la fila a mano, así que descuenta también. Sin esto la unidad quedaba + # contada dos veces —en `stocks` y vendida— y el «En depósito» del celular + # salía uno más alto que el estante. + Stock.find_by(product: celular, warehouse: central)&.then do |stock| + stock.update!(quantity: [stock.quantity - 1, 0].max) + end + end +end + if sur_company # Venta manual de Comercial Sur: cubre el caso borde de una orden sin # dirección de envío (retiro en sucursal) y con status 'cancelled'. diff --git a/docs/adr/ADR-015-convencion-de-respuesta-de-la-api.md b/docs/adr/ADR-015-convencion-de-respuesta-de-la-api.md index 0204cb3e..af06474e 100644 --- a/docs/adr/ADR-015-convencion-de-respuesta-de-la-api.md +++ b/docs/adr/ADR-015-convencion-de-respuesta-de-la-api.md @@ -45,9 +45,11 @@ cualquier error → { "error": "..." } | Respuesta | Por qué no pagina | | --- | --- | | `GET /orders/provinces`, `GET /products/categories` | Vocabularios fijos del dominio, no filas de una tabla: su tamaño lo fija el código, no los datos de la empresa | +| `GET /products/counts` | Los cuatro contadores de las pestañas del catálogo: un agregado de largo fijo, no una colección (TESIS-162) | +| `GET /activity` | La actividad reciente de la empresa, con un techo propio (`BuildFeed::MAX_LIMIT`): es un panel desplegable, no un listado que se pagine (TESIS-162) | | `POST /orders/:id/quotes` | El resultado de una acción —una cotización por courier configurado—, no una consulta | -La distinción importa para el consumidor: leer `meta.total` en cualquiera de esas tres devuelve `undefined`. La regla corta es **si el largo lo decide la empresa, pagina; si lo decide el código, no**. +La distinción importa para el consumidor: leer `meta.total` en cualquiera de ellas devuelve `undefined`. La regla corta es **si el largo lo decide la empresa, pagina; si lo decide el código, no**. Hubo que cambiar dos endpoints. `integrations#index`, que devolvía un array en la raíz, y `auth/register`, que respondía sus dos errores como `{ "errors": [...] }` —plural y array—. El registro no lo detectó la primera pasada porque el spec de contrato no lo cubría; ahora sí. diff --git a/docs/guidelines/architecture.md b/docs/guidelines/architecture.md index 9f516033..37b4986f 100644 --- a/docs/guidelines/architecture.md +++ b/docs/guidelines/architecture.md @@ -145,7 +145,7 @@ POST /api/v1/products → { "id": 1, "sku": "...", ... } cualquier error → { "error": "..." } ``` -`meta` cuenta el scope **ya filtrado**, no la tabla entera, y lo lleva todo **listado de registros**: ninguno devuelve una cantidad ilimitada de filas. Los vocabularios fijos (`/orders/provinces`, `/products/categories`) y el resultado de una acción (`/orders/:id/quotes`) viajan en `data` sin `meta`, porque su largo lo decide el código y no los datos de la empresa (ver ADR-015). +`meta` cuenta el scope **ya filtrado**, no la tabla entera, y lo lleva todo **listado de registros**: ninguno devuelve una cantidad ilimitada de filas. Los vocabularios fijos (`/orders/provinces`, `/products/categories`), los agregados (`/products/counts`, `/activity`) y el resultado de una acción (`/orders/:id/quotes`) viajan en `data` sin `meta`, porque su largo lo decide el código y no los datos de la empresa (ver ADR-015). El cálculo vive en un solo lugar, el concern `Api::V1::Paginatable`, con el techo (`MAX_PER_PAGE = 100`) y los dos defaults: 20 para una pantalla paginada y 100 para los listados que el consumidor lee enteros —depósitos, mapeos, integraciones— y usa para llenar un select. `page` y `per_page` fuera de rango se acotan en vez de romper. @@ -513,14 +513,17 @@ bin/jobs --queues=realtime # Sólo una cola ⚠️ **En Windows `bin/jobs` no arranca**: el supervisor de Solid Queue registra `SIGQUIT`, señal que no existe en la plataforma. Para probar un worker localmente en Windows: -```ruby -# bundle exec rails runner "..." -worker = SolidQueue::Worker.new(queues: 'realtime', threads: 1, polling_interval: 0.2) -Thread.new { worker.start } -sleep 5 -worker.stop +Eso mismo, empaquetado y con corte por `Ctrl-C`, vive en `bin/worker_windows.rb`: + +```bash +bundle exec rails runner bin/worker_windows.rb +QUEUES=realtime bundle exec rails runner bin/worker_windows.rb ``` +Levanta el worker en el proceso actual, sin supervisor ni fork, que es todo lo que hace falta para ver correr la ingesta de webhooks y el sync de stock. + +> El handler de `SIGINT` no para el worker: sólo baja una bandera, y el `stop` corre en el hilo principal. Ruby prohíbe tomar locks en contexto de trap, y `worker.stop` puede terminar tomando uno al instrumentar con `ActiveSupport::Notifications`: hacerlo adentro cambiaría un corte limpio por un `ThreadError`. + Alternativas: WSL, Docker, o dejar la verificación de workers al CI/deploy (Linux). ⚠️ **En macOS los workers se caen al arrancar** (`Abort trap: 6`, con un volcado de memoria que menciona `performForkChildInitialize`). Solid Queue arranca sus procesos con `fork`, y macOS aborta el proceso hijo si toca clases de Objective-C que el padre dejó a medio inicializar. Pasa igual con `bin/jobs` y con los workers dentro de Puma. Se evita con esta variable de entorno: diff --git a/docs/guidelines/feature-structure.md b/docs/guidelines/feature-structure.md index ba8cf79c..bcafc053 100644 --- a/docs/guidelines/feature-structure.md +++ b/docs/guidelines/feature-structure.md @@ -8,6 +8,11 @@ Los archivos se agrupan por **dominio de negocio** (qué hace para el negocio), ## 2. Los seis dominios del proyecto +Los seis son los del alcance, uno por épica del cronograma. No son todos los +namespaces de `app/poros/`: hay cuatro más que **sólo leen** lo que los seis +escriben y que no tienen épica propia porque no modelan nada nuevo. Están +abajo, en «Namespaces de lectura». + | Dominio | Epic Jira | Alcance | | -------------- | ---------- | ------------------------------------------------------------------ | | `auth` | TESIS-19 | Companies, Users, Warehouses, autenticación JWT, multi-tenancy | @@ -97,8 +102,27 @@ shipments → orders ✅ (un envío pertenece a una orden) webhooks → orders ✅ (los webhooks disparan procesamiento de órdenes) webhooks → shipments ✅ (los webhooks de couriers actualizan tracking) reports → orders, shipments ✅ (sólo lee: agrega ventas y despachos, nunca escribe) +activity → orders, shipments, webhooks ✅ (sólo lee: el feed de la campanita) ``` +### Namespaces de lectura + +Cuatro namespaces de `app/poros/` no son dominios: no tienen épica, no modelan +entidades propias y **no escriben** nada. Leen lo que los seis ya registran y +arman una vista. Por eso pueden cruzar dominios sin romper la jerarquía: una +dependencia de lectura no crea un ciclo ni un acoplamiento de escritura. + +| Namespace | Qué arma | De dónde lee | +| ---------- | ------------------------------------------------------------------------ | ----------------------- | +| `activity` | El feed de la campanita y el de «actividad reciente» del panel (RF-26) | orders, shipments, webhooks | +| `reports` | Los agregados de la pantalla de Reportes | orders, shipments | +| `products` | El alta y la edición de producto, que son del dominio `catalog` | catalog | +| `users` | Lo que cuelga de `auth` y no entra en el model | auth | + +La regla sigue siendo la del párrafo de arriba: si **escribe** algo que cruza +dominios, va en `app/poros/shared/` o en el model. Si sólo lee para mostrar, +puede vivir en su namespace y nombrar a los seis. + Los dominios nunca importan hacia arriba en la jerarquía de dependencia. Si dos dominios necesitan lógica compartida que no pertenece a ninguno, esa lógica va en `app/poros/shared/` o en el model directamente. ## 5. Cómo agregar una nueva entidad a un dominio existente diff --git a/spec/models/product_spec.rb b/spec/models/product_spec.rb index a3c342bf..ac41d5c4 100644 --- a/spec/models/product_spec.rb +++ b/spec/models/product_spec.rb @@ -104,6 +104,124 @@ end end + # TESIS-162: el stock se descuenta al CREAR la orden y el despacho no vuelve a + # tocar `stocks`, así que lo vendido y no despachado sigue en el estante + # aunque ya no figure en ninguna fila de stock. + describe 'the units already sold that did not leave yet' do + let(:central) do + Warehouse.create!(company: company, name: 'Central', zip_code: '1900', address: 'Calle 1') + end + let(:north) do + Warehouse.create!(company: company, name: 'North', zip_code: '1901', address: 'Calle 2') + end + + before do + product.save! + Stock.create!(product: product, warehouse: central, quantity: 10) + end + + def sell(quantity, warehouse: central, status: 'paid', shipment_status: nil, ships: true) + order = Order.create!(company: company, customer_name: 'Cliente', status: status, + requires_shipping: ships) + OrderItem.create!(order: order, product: product, warehouse: warehouse, + quantity: quantity, unit_price: 100) + ship(order, shipment_status) unless shipment_status.nil? + order + end + + def ship(order, status) + Shipment.create!(company: company, order: order, status: status, + tracking_number: status == 'pending' ? nil : 'AND-1') + end + + it 'counts a sale that has no shipment yet' do + sell(3) + + expect(product.committed_quantity).to eq(3) + end + + it 'counts a sale whose shipment was opened but not dispatched' do + sell(2, shipment_status: 'pending') + + expect(product.committed_quantity).to eq(2) + end + + # Ya salió del depósito: deja de estar comprometida porque deja de estar. + it 'stops counting it once the shipment was dispatched' do + sell(4, shipment_status: 'ready_to_ship') + + expect(product.committed_quantity).to be_zero + end + + it 'ignores a cancelled order' do + sell(5, status: 'cancelled') + + expect(product.committed_quantity).to be_zero + end + + # Un retiro no tiene envío —`CreateShipment` lo rechaza— y `Order` no tiene + # un estado «retirada»: contarlo lo dejaría comprometido para siempre y el + # «En depósito» crecería con cada venta de mostrador. En el mostrador + # registrar la venta y entregarla son el mismo momento. + it 'ignores an order the customer picks up at the store' do + sell(6, ships: false) + + expect(product.committed_quantity).to be_zero + end + + it 'leaves a pickup out of the breakdown by warehouse too' do + sell(6, ships: false) + + expect(product.committed_by_warehouse).to be_empty + end + + it 'still counts the sales that do ship, alongside a pickup' do + sell(6, ships: false) && sell(2) + + expect(product.committed_quantity).to eq(2) + end + + # Las anteriores a TESIS-126 no saben de qué depósito salieron: contarlas en + # el total pero en ningún depósito dejaría filas que no suman el encabezado. + it 'leaves out a line that does not record its warehouse' do + order = Order.create!(company: company, customer_name: 'Cliente', status: 'paid') + OrderItem.create!(order: order, product: product, quantity: 7, unit_price: 100) + + expect(product.committed_quantity).to be_zero + end + + it 'breaks it down by warehouse, with the name of each one' do + sell(3) && sell(1, warehouse: north) + + expect(product.committed_by_warehouse).to eq( + [{ warehouse_id: central.id, name: 'Central', quantity: 3 }, + { warehouse_id: north.id, name: 'North', quantity: 1 }] + ) + end + + # El encabezado del detalle muestra estos tres, y tienen que cerrar entre sí. + describe 'the three figures of the detail' do + before { sell(3) } + + it 'promises what is free' do + expect(product.available_to_promise).to eq(10) + end + + it 'commits what was sold' do + expect(product.committed_quantity).to eq(3) + end + + it 'holds both on the shelf' do + expect(product.on_hand_quantity).to eq(13) + end + end + + it 'promises zero and not nil for a product nobody reserved', :aggregate_failures do + expect(product.committed_quantity).to eq(0) + expect(product.committed_by_warehouse).to eq([]) + end + end + %i[sku name].each do |attribute| it "is invalid without #{attribute}" do product.public_send("#{attribute}=", nil) diff --git a/spec/poros/catalog/product_version_spec.rb b/spec/poros/catalog/product_version_spec.rb index 33977ed6..d27d97d6 100644 --- a/spec/poros/catalog/product_version_spec.rb +++ b/spec/poros/catalog/product_version_spec.rb @@ -31,6 +31,20 @@ def version # El caso que motiva la card: el modal guarda cantidades absolutas, asi que un # movimiento de stock ajeno tiene que invalidar lo que el usuario vio. + # Los tres entran en PRODUCT_FIELDS, así que el modal los edita y tienen que + # mover la versión. Categoría viene de TESIS-150; empaque y norma, de TESIS-162. + it 'changes when the category changes' do + expect { product.update!(category: 'Machinery') }.to(change { version }) + end + + it 'changes when the packaging changes' do + expect { product.update!(packaging: 'Caja x 12') }.to(change { version }) + end + + it 'changes when the technical standard changes' do + expect { product.update!(technical_standard: 'IRAM 2178') }.to(change { version }) + end + it 'changes when the stock of a warehouse moves' do stock = Stock.create!(product: product, warehouse: central, quantity: 10) before_change = version diff --git a/spec/poros/orders/order_version_spec.rb b/spec/poros/orders/order_version_spec.rb index 3cd92ffa..c80595d5 100644 --- a/spec/poros/orders/order_version_spec.rb +++ b/spec/poros/orders/order_version_spec.rb @@ -38,6 +38,13 @@ def version = described_class.new(order: Order.includes(:order_items).find(order expect { order.update!(customer_province: 'Santa Fe') }.to(change { version }) end + # TESIS-162: `requires_shipping` se edita por el mismo PUT. Sin esto, cambiar + # una venta de envío a retiro no movía la versión, y el que guardó con el + # If-Match viejo la devolvía a envío sin recibir el 412. + it 'changes when the order stops needing a shipment' do + expect { order.update!(requires_shipping: false) }.to(change { version }) + end + it 'changes when the status changes' do expect { order.update!(status: 'paid') }.to(change { version }) end diff --git a/spec/poros/orders/process_webhook_order_spec.rb b/spec/poros/orders/process_webhook_order_spec.rb index 02fcc0b3..db3bd411 100644 --- a/spec/poros/orders/process_webhook_order_spec.rb +++ b/spec/poros/orders/process_webhook_order_spec.rb @@ -568,4 +568,69 @@ def payload_in(state) end end end + + # TESIS-162: el envío pasa a ser opcional, también para las ventas de canal. + # + # Se pisa `integration` con un `let` —el mismo patrón que usa el bloque de la + # provincia— para que `publish` mapee el producto contra la integración cuyo + # servicio trae la clave nueva en la plantilla. + describe 'whether the sale is shipped or picked up at the store' do + subject(:ingest) { described_class.new(webhook_log: create_log(body)).call } + + let(:integration) { CompanyIntegration.create!(company: company, service: pickup_service) } + let(:body) { order_payload(items: [line('MLA-1', 1, 100)]) } + + def pickup_service + Service.create!(service_name: "Canal #{SecureRandom.hex(4)}", type: 'ecommerce', + http_method: 'GET', uri: 'https://api.canal.test/orders', + response_mapper: order_mapper.merge( + 'delivery.requires_shipping' => 'requires_shipping' + ), + response_value_mapper: { 'pagado' => 'paid' }) + end + + before { publish('SKU-1', 'MLA-1', stock: 20) } + + # El default: asumir envío y que sobre deja una orden lista para despachar + # que se ve en la pantalla. Asumir retiro y que falte deja una venta que + # había que mandar, sin ninguna señal de que falta hacerlo. + context 'when the payload does not carry the flag' do + it 'assumes there is a shipment' do + expect(ingest.requires_shipping).to be(true) + end + end + + context 'when the channel reports a pickup' do + let(:body) do + order_payload(items: [line('MLA-1', 1, 100)]) + .merge('delivery' => { 'requires_shipping' => false }) + end + + it 'records it as a pickup' do + expect(ingest.requires_shipping).to be(false) + end + end + + context 'when the channel sends the flag as text' do + let(:body) do + order_payload(items: [line('MLA-1', 1, 100)]) + .merge('delivery' => { 'requires_shipping' => 'false' }) + end + + it 'reads it all the same' do + expect(ingest.requires_shipping).to be(false) + end + end + + # La plantilla mapea la clave pero el canal no la manda en esta venta. + context 'when the mapped key is missing from this payload' do + let(:body) do + order_payload(items: [line('MLA-1', 1, 100)]).merge('delivery' => {}) + end + + it 'falls back to a shipment' do + expect(ingest.requires_shipping).to be(true) + end + end + end end diff --git a/spec/poros/shipments/confirm_dispatch_spec.rb b/spec/poros/shipments/confirm_dispatch_spec.rb index 9bc20317..261e6c7a 100644 --- a/spec/poros/shipments/confirm_dispatch_spec.rb +++ b/spec/poros/shipments/confirm_dispatch_spec.rb @@ -44,7 +44,7 @@ def attempt_dispatch(target = shipment, using: integration) dispatch(target, using: using) rescue Shipments::AlreadyDispatchedError, Shipments::DispatchResponseError, Shipments::InvalidCourierIntegrationError, Shipments::UnshippableOrderError, - Integrations::AdapterExecutionError + Shipments::PickupOrderError, Integrations::AdapterExecutionError nil end @@ -185,6 +185,32 @@ def dispatch_costing(cost) end end + # El alta del envío ya rechazaba los retiros; el despacho no lo miraba, así + # que una orden con envío `pending` a la que después se le pone + # `requires_shipping: false` por PUT se podía despachar igual: pagar una + # etiqueta por una venta que el cliente se lleva del mostrador. + describe 'when the order became a pickup after the shipment was opened' do + before { order.update!(requires_shipping: false) } + + it 'refuses to dispatch it' do + expect { dispatch }.to raise_error(Shipments::PickupOrderError, /picked up/) + end + + it 'does not call the courier' do + stub = stub_courier + attempt_dispatch + + expect(stub).not_to have_been_requested + end + + # El estado manda sobre el motivo: lo accionable es que está cancelada. + it 'says a cancelled pickup is cancelled, and not that it is a pickup' do + order.update!(status: 'cancelled') + + expect { dispatch }.to raise_error(Shipments::UnshippableOrderError) + 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 diff --git a/spec/requests/api/v1/activity_spec.rb b/spec/requests/api/v1/activity_spec.rb new file mode 100644 index 00000000..fc7c600f --- /dev/null +++ b/spec/requests/api/v1/activity_spec.rb @@ -0,0 +1,261 @@ +# frozen_string_literal: true + +require 'rails_helper' + +# TESIS-162: lo que abre la campanita. No hay tabla de notificaciones: la +# actividad se deriva de lo que el sistema ya registra, así que no puede +# desincronizarse de los hechos. +RSpec.describe 'Activity 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) } + + def auth_headers(for_user) + post '/api/v1/auth/login', params: { email: for_user.email, password: 'password123' }, + headers: { 'X-Tenant-Slug' => for_user.company.slug } + { 'Authorization' => "Bearer #{response.parsed_body['token']}" } + end + + def create_order(owner: company, customer: 'Ana', at: Time.current, external: nil) + Current.set(company_id: owner.id) do + Order.create!(company: owner, customer_name: customer, status: 'paid', + external_order_id: external, created_at: at) + end + end + + def courier_integration(owner: company) + service = Service.create!(service_name: "Andreani #{SecureRandom.hex(4)}", type: 'courier', + http_method: 'POST', uri: 'https://api.andreani.test') + Current.set(company_id: owner.id) { CompanyIntegration.create!(company: owner, service: service) } + end + + def dispatch_order(order, at: Time.current, tracking: 'AND-1', owner: company) + Current.set(company_id: owner.id) do + shipment = Shipment.create!(company: owner, order: order, status: 'ready_to_ship', + tracking_number: tracking, + company_integration: courier_integration(owner: owner)) + ShipmentEvent.create!(shipment: shipment, internal_status: 'ready_to_ship', + external_status: 'Despachado', occurred_at: at) + shipment + end + end + + def fail_event(at: Time.current, owner: company, status: 'pending') + Current.set(company_id: owner.id) do + FailedEvent.create!(company: owner, event_type: 'order_ingestion', direction: 'inbound', + payload: {}, created_at: at, status: status) + end + end + + # Un aviso informativo posterior al despacho: la plantilla no lo sabe + # traducir, así que RegisterTrackingEvent lo guarda con el estado actual del + # envío, que después del despacho es `ready_to_ship`. + def log_another_push(shipment, at: 1.hour.ago) + Current.set(company_id: company.id) do + ShipmentEvent.create!(shipment: shipment, internal_status: 'ready_to_ship', + external_status: 'Recibido en sucursal', occurred_at: at) + end + end + + def feed + get '/api/v1/activity', headers: headers + response.parsed_body['data'] + end + + it 'returns 401 without a token' do + get '/api/v1/activity' + expect(response).to have_http_status(:unauthorized) + end + + it 'answers with data and no meta: it is a panel, not a paginated listing' do + get '/api/v1/activity', headers: headers + + expect(response.parsed_body.keys).to eq(['data']) + end + + describe 'what it reports' do + it 'reports a sale that came in', :aggregate_failures do + order = create_order(customer: 'Distribuidora Sur', external: 'SHOP-99') + + entry = feed.first + expect(entry).to include('type' => 'order_created', 'order_id' => order.id, + 'customer_name' => 'Distribuidora Sur', + 'external_order_id' => 'SHOP-99') + end + + # Una venta cargada a mano no tiene id externo, y eso es lo que la distingue + # de una de canal en la pantalla. + it 'tells a manual sale apart by its missing external id' do + create_order + + expect(feed.first['external_order_id']).to be_nil + end + + # `shipments` no guarda cuándo se despachó: lo dice el primer evento + # `ready_to_ship` de la bitácora, igual que en los reportes. + it 'reports a dispatch with its courier and its tracking', :aggregate_failures do + shipment = dispatch_order(create_order, tracking: 'AND-500') + + entry = feed.find { |event| event['type'] == 'shipment_dispatched' } + expect(entry).to include('shipment_id' => shipment.id, 'order_id' => shipment.order_id, + 'tracking_number' => 'AND-500') + expect(entry['courier']).to start_with('Andreani') + end + + it 'reports an event that fell to the retry queue', :aggregate_failures do + failure = fail_event + + entry = feed.find { |event| event['type'] == 'event_failed' } + expect(entry).to include('failed_event_id' => failure.id, + 'event_type' => 'order_ingestion', 'status' => 'pending') + end + + # `RegisterTrackingEvent` guarda con el estado actual del envío cualquier + # push que la plantilla del courier no sepa traducir, y después del despacho + # ese estado es `ready_to_ship`. Sin tomar uno por envío, cada aviso + # informativo entra como otro despacho del mismo paquete. + it 'reports one dispatch per shipment, however many events it logged' do + log_another_push(dispatch_order(create_order, at: 2.hours.ago)) + + expect(feed.count { |event| event['type'] == 'shipment_dispatched' }).to eq(1) + end + + # El que queda es el primero: es el que escribió ConfirmDispatch, y su hora + # es la del despacho, no la del último aviso del courier. + it 'keeps the earliest of them, which is the one that dispatched it' do + log_another_push(dispatch_order(create_order, at: 2.hours.ago)) + + entry = feed.find { |event| event['type'] == 'shipment_dispatched' } + expect(Time.zone.parse(entry['occurred_at'])).to be_within(2.seconds).of(2.hours.ago) + end + + it 'still reports a dispatch for each shipment that had one' do + dispatch_order(create_order, tracking: 'AND-1') + dispatch_order(create_order, tracking: 'AND-2') + + expect(feed.count { |event| event['type'] == 'shipment_dispatched' }).to eq(2) + end + + # Un evento que se reprocesó bien o que alguien descartó a mano ya no es una + # novedad: volver a avisarlo ocuparía el cupo del feed y desplazaría a los + # que siguen abiertos. + it 'leaves out a failure that was already resolved', :aggregate_failures do + fail_event(status: 'succeeded') + fail_event(status: 'discarded') + + expect(feed.select { |event| event['type'] == 'event_failed' }).to be_empty + end + + it 'keeps the ones that are still open', :aggregate_failures do + fail_event(status: 'pending') + fail_event(status: 'dead') + fail_event(status: 'succeeded') + + expect(feed.count { |event| event['type'] == 'event_failed' }).to eq(2) + end + + # Los tres datos que el feed deja en null cuando no hay: la orden sin total + # (las anteriores a TESIS-114 no tienen líneas con qué calcularlo), el envío + # sin courier y el evento fallido sin integración (los de la ingesta por + # webhook pueden no tener una todavía). Null y no un texto de relleno: la + # pantalla decide cómo se muestra lo que falta. + # Una orden sin líneas no tiene con qué calcular su total (TESIS-114). + it 'leaves the total of an order that has none in null' do + create_order + + expect(feed.first['total_amount']).to be_nil + end + + def dispatch_without_courier + Current.set(company_id: company.id) do + shipment = Shipment.create!(company: company, order: create_order, + status: 'ready_to_ship', tracking_number: 'SIN-COURIER') + ShipmentEvent.create!(shipment: shipment, internal_status: 'ready_to_ship', + external_status: 'Despachado', occurred_at: Time.current) + end + end + + it 'leaves the courier in null for a shipment that has none' do + dispatch_without_courier + + expect(feed.find { |event| event['type'] == 'shipment_dispatched' }['courier']).to be_nil + end + + it 'leaves the integration in null for a failure that has none' do + fail_event + + expect(feed.find { |event| event['type'] == 'event_failed' }['integration']).to be_nil + end + + it 'has nothing to report for a company where nothing happened' do + expect(feed).to eq([]) + end + end + + describe 'how it is ordered and bounded' do + it 'puts the most recent first, mixing the three sources', :aggregate_failures do + create_order(at: 3.days.ago) + dispatch_order(create_order(at: 5.days.ago), at: 1.day.ago) + fail_event(at: 2.days.ago) + + expect(feed.pluck('type')) + .to eq(%w[shipment_dispatched event_failed order_created order_created]) + end + + it 'defaults to the last twenty' do + 25.times { |i| create_order(at: i.minutes.ago) } + + expect(feed.size).to eq(Activity::BuildFeed::DEFAULT_LIMIT) + end + + it 'honours a smaller limit' do + 5.times { |i| create_order(at: i.minutes.ago) } + + get '/api/v1/activity', params: { limit: 2 }, headers: headers + + expect(response.parsed_body['data'].size).to eq(2) + end + + # Es un desplegable con techo, no un listado: sin tope, un `?limit=` grande + # haría tres consultas sin límite. + it 'caps a limit that asks for too much' do + 60.times { |i| create_order(at: i.minutes.ago) } + + get '/api/v1/activity', params: { limit: 1_000 }, headers: headers + + expect(response.parsed_body['data'].size).to eq(Activity::BuildFeed::MAX_LIMIT) + end + + it 'falls back to the default for a limit that is not a number' do + 3.times { |i| create_order(at: i.minutes.ago) } + + get '/api/v1/activity', params: { limit: 'muchas' }, headers: headers + + expect(response.parsed_body['data'].size).to eq(3) + end + end + + describe 'tenant isolation' do + let(:other_company) { Company.create!(name: 'Tenant B', tax_id: '30-22222222-2') } + + it 'leaves out the orders of another company' do + create_order(owner: other_company, customer: 'Ajena') + + expect(feed).to eq([]) + end + + # `ShipmentEvent` no tiene company_id: su aislamiento entra por el join con + # `Shipment`, así que vale la pena probarlo aparte. + it 'leaves out the dispatches of another company' do + dispatch_order(create_order(owner: other_company), owner: other_company) + + expect(feed).to eq([]) + end + + it 'leaves out the failed events of another company' do + fail_event(owner: other_company) + + expect(feed).to eq([]) + end + end +end diff --git a/spec/requests/api/v1/api_contract_spec.rb b/spec/requests/api/v1/api_contract_spec.rb index 9c6e19bf..27abcb55 100644 --- a/spec/requests/api/v1/api_contract_spec.rb +++ b/spec/requests/api/v1/api_contract_spec.rb @@ -35,18 +35,20 @@ module ContratoDeLaApi empresa: %w[id name], producto_fila: %w[id sku name description category weight dimensions total_stock stock_status in_transit_quantity primary_warehouse warehouse_count created_at updated_at], - producto: %w[id sku name description category weight dimensions total_stock stock_status - in_transit_quantity in_transit_by_warehouse stocks created_at updated_at], + producto: %w[id sku name description category packaging technical_standard weight + dimensions total_stock stock_status in_transit_quantity + in_transit_by_warehouse committed_quantity on_hand_quantity + available_to_promise committed_by_warehouse stocks created_at updated_at], stock: %w[id quantity warehouse_id warehouse stock_status created_at updated_at], - deposito_referencia: %w[id name address zip_code], + deposito_referencia: %w[id name address zip_code capacity], transito: %w[warehouse_id name quantity], - deposito: %w[id name address zip_code stored_units], + deposito: %w[id name address zip_code capacity stored_units], orden_fila: %w[id external_order_id customer_name customer_document customer_address - customer_zip_code customer_city customer_province status courier total_amount - item_count created_at updated_at], + customer_zip_code customer_city customer_province status requires_shipping + courier total_amount item_count created_at updated_at], orden: %w[id external_order_id customer_name customer_document customer_address - customer_zip_code customer_city customer_province status total_amount order_items - created_at updated_at], + customer_zip_code customer_city customer_province status requires_shipping + total_amount order_items created_at updated_at], linea: %w[id product_id warehouse_id quantity unit_price product created_at updated_at], courier: %w[id service_id name], envio: %w[id order_id status tracking_number shipping_label_url shipping_cost courier events diff --git a/spec/requests/api/v1/every_listing_envelope_spec.rb b/spec/requests/api/v1/every_listing_envelope_spec.rb index 5854910c..3fc220be 100644 --- a/spec/requests/api/v1/every_listing_envelope_spec.rb +++ b/spec/requests/api/v1/every_listing_envelope_spec.rb @@ -11,7 +11,7 @@ # Toma de `Rails.application.routes` cada GET de `/api/v1` y lo clasifica: # # - un `index` es una colección: viaja en `data` + `meta` (`page`, `per_page`, -# `total`), sin excepción (TESIS-108); +# `total`), salvo los feeds acotados, que no paginan y van pelados; # - los vocabularios fijos van en `data` sin `meta` (ADR-015, «Excepciones»); # - los recursos sueltos (`show`, `me`, `tenant-config`) se prueban en sus # specs: acá sólo se verifica que estén clasificados. @@ -24,11 +24,18 @@ # `ContratoDeLaApi`). module FormaDeCadaRuta VOCABULARIOS = %w[products#categories orders#provinces].freeze + # Feeds acotados: son `index` pero no paginan. `/activity` (TESIS-162) corta + # en un límite duro y se lee entero de una vez, así que un `meta` con `page` + # y `total` describiría una paginación que no existe. Van en `data` pelado, + # como los vocabularios, y por eso se excluyen del barrido de colecciones. + FEEDS = %w[activity#index].freeze # `reports#overview` (TESIS-999007) es un recurso calculado: va pelado. Está # anotado acá aunque la ruta llegue en otra rama, para que el orden de merge # no rompa este spec. + # `products#counts` (TESIS-162) es un agregado calculado, como `reports#overview`: + # devuelve los cuatro contadores de las pestañas del catálogo, no una colección. RECURSOS = %w[me#show tenant_config#show warehouses#show products#show orders#show - shipments#show reports#overview].freeze + shipments#show reports#overview products#counts].freeze end RSpec.describe 'Every listing keeps the response envelope (ADR-015)', type: :request do @@ -58,7 +65,11 @@ def api_get_routes end end - def listings = api_get_routes.select { |action, _| action.end_with?('#index') } + def listings + api_get_routes.select do |action, _| + action.end_with?('#index') && FormaDeCadaRuta::FEEDS.exclude?(action) + end + end def sample(path) = path.gsub(':product_id', product.id.to_s) @@ -80,8 +91,9 @@ def shapes_of_listings expect(shapes_of_listings).to all(satisfy { |_action, shape| shape == expected }) end - it 'answers every vocabulary with data and no meta', :aggregate_failures do - api_get_routes.slice(*FormaDeCadaRuta::VOCABULARIOS).each do |action, path| + it 'answers every vocabulary and bounded feed with data and no meta', :aggregate_failures do + bare = FormaDeCadaRuta::VOCABULARIOS + FormaDeCadaRuta::FEEDS + api_get_routes.slice(*bare).each do |action, path| get path, headers: headers expect(response.parsed_body.keys).to eq(['data']), "#{action} should be a bare vocabulary" @@ -92,7 +104,8 @@ def shapes_of_listings # (que sea un `index`), un vocabulario o un recurso, y sumala a su lista. it 'knows the shape of every other GET route' do unclassified = api_get_routes.keys.reject do |action| - action.end_with?('#index') || FormaDeCadaRuta::VOCABULARIOS.include?(action) || FormaDeCadaRuta::RECURSOS.include?(action) + action.end_with?('#index') || FormaDeCadaRuta::VOCABULARIOS.include?(action) || + FormaDeCadaRuta::RECURSOS.include?(action) end expect(unclassified).to be_empty diff --git a/spec/requests/api/v1/orders_spec.rb b/spec/requests/api/v1/orders_spec.rb index 3cafee14..b348b937 100644 --- a/spec/requests/api/v1/orders_spec.rb +++ b/spec/requests/api/v1/orders_spec.rb @@ -396,6 +396,33 @@ def order_of_another_company expect(response).to have_http_status(:unprocessable_content) end + # TESIS-162: hasta ahora toda orden se asumía despachada y no había forma + # de registrar una venta que el cliente retira en el local. + it 'is shipped when the body says nothing about it', :aggregate_failures do + post_order + + expect(response).to have_http_status(:created) + expect(response.parsed_body['requires_shipping']).to be(true) + end + + # La columna es NOT NULL y el campo entra por strong params desde + # TESIS-162: un `null` llegaba a la base y levantaba NotNullViolation, + # que ningún rescue_from mapea. + it 'refuses a null requires_shipping with 422 and not 500', :aggregate_failures do + post_order(build_payload.deep_merge(order: { requires_shipping: nil })) + + expect(response).to have_http_status(:unprocessable_content) + expect(response.parsed_body['error']).to include('shipping') + end + + it 'records a pickup when the body asks for one', :aggregate_failures do + post_order(build_payload.deep_merge(order: { requires_shipping: false })) + + expect(response).to have_http_status(:created) + expect(response.parsed_body['requires_shipping']).to be(false) + expect(Order.last.requires_shipping).to be(false) + end + # Pasaba la validación (0,5 > 0), la columna integer la guardaba en 0 y # DeductStock reventaba con ArgumentError: 500. it 'rejects a quantity below one unit with 422, not 500', :aggregate_failures do diff --git a/spec/requests/api/v1/products_spec.rb b/spec/requests/api/v1/products_spec.rb index 09eebe8d..50481296 100644 --- a/spec/requests/api/v1/products_spec.rb +++ b/spec/requests/api/v1/products_spec.rb @@ -460,6 +460,77 @@ def row end end + # TESIS-162: la pantalla pedía un request por pestaña, sólo para leer el total + # de cada una. + describe 'GET /api/v1/products/counts' do + let(:warehouse) do + Warehouse.create!(company: company, name: 'Central', zip_code: '1900', address: 'Calle 1') + end + + def stocked(sku, quantity, category: nil) + product = Product.create!(company: company, sku: sku, name: sku, category: category) + Stock.create!(product: product, warehouse: warehouse, quantity: quantity) + product + end + + # Uno de cada pestaña, para que los cuatro contadores digan algo distinto. + def one_product_per_tab + stocked('OUT-1', 0) + stocked('LOW-1', Product::LOW_STOCK_THRESHOLD) + stocked('OK-1', Product::LOW_STOCK_THRESHOLD + 1) + end + + it 'returns 401 without a token' do + get '/api/v1/products/counts' + expect(response).to have_http_status(:unauthorized) + end + + it 'counts every tab of the catalog in one answer', :aggregate_failures do + one_product_per_tab + + get '/api/v1/products/counts', headers: headers + + expect(response).to have_http_status(:ok) + expect(response.parsed_body['data']) + .to eq('all' => 3, 'available' => 1, 'low' => 1, 'out_of_stock' => 1) + end + + # Si los contadores ignoraran el buscador, el número de la pestaña y las + # filas que se ven dirían cosas distintas. + it 'honours the same search as the listing' do + stocked('ALPHA-1', 5) + stocked('BETA-1', 5) + + get '/api/v1/products/counts', params: { search: 'ALPHA' }, headers: headers + + expect(response.parsed_body['data']).to include('all' => 1, 'low' => 1) + end + + it 'honours the category filter too' do + stocked('CAT-1', 5, category: 'Cabling') + stocked('CAT-2', 5, category: 'Power') + + get '/api/v1/products/counts', params: { category: 'Cabling' }, headers: headers + + expect(response.parsed_body['data']).to include('all' => 1) + end + + it 'does not count the products of another company' do + other_product + + get '/api/v1/products/counts', headers: headers + + expect(response.parsed_body['data']['all']).to be_zero + end + + # Vocabulario, no colección paginada: `data` sola (ADR-015). + it 'answers without a meta envelope' do + get '/api/v1/products/counts', headers: headers + + expect(response.parsed_body.keys).to eq(['data']) + end + end + describe 'GET /api/v1/products/:id' do let!(:product) do wh = Warehouse.create!(company: company, name: 'Central', zip_code: '1900', address: 'Calle 1') @@ -479,6 +550,65 @@ def row expect(response.parsed_body['total_stock']).to eq(7) end + # TESIS-162: los tres números del detalle dejan de ser «—» y pasan a ser + # dato. Se exponen desde acá y no se derivan en el cliente: `on_hand` no es + # `total_stock`, y restarlos mal del lado del front era el riesgo. + describe 'the three stock figures' do + def sell(quantity) + warehouse = Warehouse.find_by(name: 'Central') + order = Order.create!(company: company, customer_name: 'Cliente', status: 'paid') + OrderItem.create!(order: order, product: product, warehouse: warehouse, + quantity: quantity, unit_price: 100) + end + + it 'answers zero and not null for a product nobody reserved', :aggregate_failures do + get "/api/v1/products/#{product.id}", headers: headers + + expect(response.parsed_body) + .to include('committed_quantity' => 0, 'committed_by_warehouse' => [], + 'available_to_promise' => 7, 'on_hand_quantity' => 7) + end + + it 'separates what is free from what is on the shelf' do + sell(2) + + get "/api/v1/products/#{product.id}", headers: headers + + expect(response.parsed_body) + .to include('committed_quantity' => 2, 'available_to_promise' => 7, + 'on_hand_quantity' => 9) + end + + it 'breaks the commitment down by warehouse' do + sell(3) + + get "/api/v1/products/#{product.id}", headers: headers + + expect(response.parsed_body['committed_by_warehouse'].first) + .to include('name' => 'Central', 'quantity' => 3) + end + + # Una consulta agregada y no una por depósito. + it 'does not add a query per warehouse' do + add_extra_stocks(product) && sell(1) + + queries = count_queries(matching: /FROM "order_items"/) do + get "/api/v1/products/#{product.id}", headers: headers + end + + expect(queries).to eq(1) + end + end + + it 'returns the packaging and the technical standard', :aggregate_failures do + product.update!(packaging: 'Caja x12', technical_standard: 'IRAM 2063') + + get "/api/v1/products/#{product.id}", headers: headers + + expect(response.parsed_body['packaging']).to eq('Caja x12') + expect(response.parsed_body['technical_standard']).to eq('IRAM 2063') + end + it 'returns 404 for a product from another company', :aggregate_failures do expect(other_product.company_id).to eq(other_company.id) get "/api/v1/products/#{other_product.id}", headers: headers diff --git a/spec/requests/api/v1/shipment_quotes_spec.rb b/spec/requests/api/v1/shipment_quotes_spec.rb index 2bbce6f7..0d25f619 100644 --- a/spec/requests/api/v1/shipment_quotes_spec.rb +++ b/spec/requests/api/v1/shipment_quotes_spec.rb @@ -125,4 +125,19 @@ def foreign expect(response).to have_http_status(:bad_request) end + + # No hay a quién pedirle precio: la venta se retira en el local. Sin esto se + # cotizaba igual una orden que después `CreateShipment` rechaza, y una que ya + # tenía su envío abierto y pasó a retiro por PUT se podía despachar: una + # etiqueta pagada por una venta que no sale. + describe 'an order the customer picks up at the store' do + before { order.update!(requires_shipping: false) } + + it 'is refused instead of quoted', :aggregate_failures do + quote + + expect(response).to have_http_status(:unprocessable_content) + expect(response.parsed_body['error']).to include('picked up') + end + end end diff --git a/spec/requests/api/v1/shipments_spec.rb b/spec/requests/api/v1/shipments_spec.rb index aba64690..b651463a 100644 --- a/spec/requests/api/v1/shipments_spec.rb +++ b/spec/requests/api/v1/shipments_spec.rb @@ -404,6 +404,35 @@ def create_shipment(order_id, auth: headers) post "/api/v1/orders/#{order_id}/shipment", headers: auth, as: :json end + # TESIS-162: una venta con retiro en el local no entra al circuito + # logístico. Es un 422 propio y no el de la orden cancelada: una cancelada + # es un callejón sin salida, un retiro es una venta sana que no se despacha. + context 'when the customer picks the order up at the store' do + let(:pickup) do + order_with('paid').tap { |o| o.update!(requires_shipping: false) } + end + + it 'refuses to open a shipment', :aggregate_failures do + expect { create_shipment(pickup.id) }.not_to change(Shipment, :count) + + expect(response).to have_http_status(:unprocessable_content) + expect(response.parsed_body['error']).to include('picked up') + end + + it 'still opens one for an order that is shipped' do + expect { create_shipment(order_with('paid').id) }.to change(Shipment, :count).by(1) + end + + # El estado va primero: lo accionable es que está cancelada, no que se + # retira en el local. Separar los dos errores fue para no confundirlos. + it 'says a cancelled pickup is cancelled, and not that it is a pickup' do + pickup.update!(status: 'cancelled') + create_shipment(pickup.id) + + expect(response.parsed_body['error']).not_to include('picked up') + end + end + # Mismo motivo que foreign_shipment: assign_current_company pisa el company: # manual si Current.company_id quedó seteado por un request previo. def foreign_order diff --git a/spec/requests/api/v1/warehouses_spec.rb b/spec/requests/api/v1/warehouses_spec.rb index a9479770..f2bc3ebd 100644 --- a/spec/requests/api/v1/warehouses_spec.rb +++ b/spec/requests/api/v1/warehouses_spec.rb @@ -184,6 +184,62 @@ def stock_queries_while(&) expect(response).to have_http_status(:created) end + # TESIS-162: la barra de ocupación del detalle de producto compara lo + # guardado contra un techo que nadie puede derivar de otro dato. + describe 'the declared capacity' do + it 'is saved when it comes in the body', :aggregate_failures do + post '/api/v1/warehouses', params: { warehouse: warehouse_attrs.merge(capacity: 6000) }, + headers: headers, as: :json + + expect(response).to have_http_status(:created) + expect(response.parsed_body['capacity']).to eq(6000) + end + + # Null y no cero: nadie declaró el techo todavía, que no es lo mismo que + # decir que no entra nada. + it 'stays null when nobody declared it' do + post '/api/v1/warehouses', params: { warehouse: warehouse_attrs }, + headers: headers, as: :json + + expect(response.parsed_body['capacity']).to be_nil + end + + it 'refuses a capacity that could not hold anything', :aggregate_failures do + post '/api/v1/warehouses', params: { warehouse: warehouse_attrs.merge(capacity: 0) }, + headers: headers, as: :json + + expect(response).to have_http_status(:unprocessable_content) + expect(response.parsed_body['error']).to include('Capacity') + end + + # La columna es `integer` de 4 bytes: sin tope, el número pasaba la + # validación y reventaba al guardar con ActiveModel::RangeError -> 500. + it 'refuses a capacity that does not fit in the column, with 422 and not 500', + :aggregate_failures do + post '/api/v1/warehouses', + params: { warehouse: warehouse_attrs.merge(capacity: 99_999_999_999) }, + headers: headers, as: :json + + expect(response).to have_http_status(:unprocessable_content) + expect(response.parsed_body['error']).to include('Capacity') + end + + it 'accepts the largest capacity the column can hold' do + post '/api/v1/warehouses', + params: { warehouse: warehouse_attrs.merge(capacity: Warehouse::MAX_CAPACITY) }, + headers: headers, as: :json + + expect(response).to have_http_status(:created) + end + + it 'refuses a capacity that is not a whole number of units' do + post '/api/v1/warehouses', params: { warehouse: warehouse_attrs.merge(capacity: 1.5) }, + headers: headers, as: :json + + expect(response).to have_http_status(:unprocessable_content) + end + end + it 'assigns the company from the JWT, ignoring any company_id in the body' do post '/api/v1/warehouses', params: { warehouse: warehouse_attrs.merge(company_id: other_company.id) },