feat: [TESIS-162] model stock reservations, pickup orders, catalog counts and recent activity - #115
TomasMartin2004 wants to merge 6 commits into
Conversation
…unts and recent activity From the system audit of 04/10. Half of the findings are screen-level but could not be fixed without data behind them, so this is the backend half. **Stock reservations.** The product detail showed "Comprometido" and "Disponible para prometer" as em dashes with a note saying the API does not model them. It does now, and nothing new had to be stored: DeductStock removes the units from `stocks` when the order is created, ConfirmDispatch never touches `stocks`, and `order_items.warehouse_id` has recorded the origin since TESIS-126. So what was sold and has not shipped is still on the shelf and can be derived: - committed: lines of non-cancelled orders whose shipment was not dispatched, by warehouse; - available to promise: `stocks.quantity`, because what was sold is already out of it; - on hand: the two together, which is what somebody counts if they walk in. A product nobody reserved answers 0, not null: the figure exists and is zero. Lines with no warehouse stay out of both the breakdown and the total, so the rows of the screen always add up to its header. **Pickup orders.** `orders.requires_shipping` makes the shipment optional, for manual sales and for channel ones. The template reads it from the payload when the channel reports it; when it does not, the order is assumed to be shipped — assuming a shipment that turns out unnecessary is visible on screen, assuming a pickup that was really a shipment is a sale nobody sends. CreateShipment refuses a pickup with its own error, separate from the cancelled one: a cancelled order is a dead end, a pickup is a healthy sale that does not ship. **Catalog counts.** `/inventory` fired four requests, one per tab, each reading `meta.total` of a page of one row. `GET /products/counts` answers the four in one call, honouring the same search and category filter as the listing. It is its own endpoint and not part of the listing's `meta` because the counts do not change when paging. **Recent activity.** `GET /activity` feeds the bell: orders created, shipments dispatched and events that fell to the retry queue, in one list ordered by date. No notifications table and no per-user read state — that is another domain and nobody asked for it. Derived from what the system already records, so it cannot drift from the facts. Also: `warehouses.capacity` for the occupancy bar, which cannot be derived from any other data, and `packaging` / `technical_standard` on products. Seeds carry a pickup order, a warehouse with its capacity declared and one without, so both paths can be seen. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp
El RuboCop del CI es 1.90 y marca `Layout/EmptyLines` donde el local —1.86, el que fija el Gemfile.lock— no decía nada.
LoLoo03
left a comment
LoLoo03
left a comment
There was a problem hiding this comment.
Review de #115 (TESIS-162)
Revisé el diff completo (commit 40824b0) leyendo también el código que rodea a cada cambio. No corrí la suite ni la app: todo lo de abajo sale de leer el código, y donde algo no lo pude confirmar lo digo.
La idea de derivar lo comprometido de lo que el sistema ya registra, en vez de guardar un número nuevo, me parece correcta. Los specs de aislamiento de tenant (incluido el de ShipmentEvent por el join con Shipment) también están bien pensados. Lo que sigue son cosas que encontré para resolver o discutir antes de mergear.
Importantes
- El feed repite envíos: recent_dispatches toma todos los eventos ready_to_ship app/poros/activity/build_feed.rb:74
El comentario dice que el despacho lo marca «el primer evento ready_to_ship», pero la consulta trae todos. Y la bitácora los escribe más de una vez: en RegisterTrackingEvent#internal_status, un evento que la plantilla no sabe traducir se guarda con @shipment.status, y después del despacho ese estado es ready_to_ship. Cualquier push informativo posterior («Paquete recibido en sucursal») queda con internal_status = 'ready_to_ship'. Un mismo envío aparece varias veces como shipment_dispatched en la campanita, con la hora de cada evento del courier, y esas repeticiones ocupan el limit y desplazan los despachos de otros envíos.
Sugerencia: una fila por envío, la más temprana (DISTINCT ON (shipment_id) ... ORDER BY shipment_id, occurred_at, o group(:shipment_id).minimum(:occurred_at)).
- Una orden de retiro (o cualquiera que nunca recibe envío) queda «comprometida» para siempre app/models/product.rb:31 y :193
committed_scope cuenta toda orden no cancelada sin envío (el NULL del LEFT JOIN) o con envío pending. Una orden con requires_shipping: false nunca va a tener envío (CreateShipment lo rechaza) y Order::STATUSES no tiene un estado «retirada». Cuando el cliente retira las unidades, siguen sumando a committed_quantity y por lo tanto a on_hand_quantity = total_stock + committed_quantity. El «En depósito» sobrestima el físico y crece con cada retiro. Lo mismo pasa con cualquier orden histórica o de webhook que nunca llegó a tener envío.
La orden «Retiro en Mostrador» de los seeds ya lo muestra: queda comprometida permanentemente, y además el seed no descuenta la unidad de stocks, así que el invariante «lo comprometido ya no está en stocks» tampoco se cumple ahí.
Esto toca la decisión de fondo de la card, así que lo marco como lo más importante a discutir: hace falta algún estado «retirada/entregada en mostrador», o decidir explícitamente cómo se tratan los retiros en el cálculo.
- requires_shipping: null devuelve 500 app/controllers/api/v1/orders_controller.rb:20
El campo ahora entra en ORDER_FIELDS, la columna es NOT NULL y el modelo no valida nada sobre ella. Un {"order": {"requires_shipping": null, ...}} (o string vacío) pasa permit, llega a la base y levanta ActiveRecord::NotNullViolation, que ningún rescue_from mapea: 500. Pasa igual en PUT /orders/:id por update_params. Además, cualquier texto raro ('no', 'pickup') se castea a true en silencio en vez de rechazarse.
Sugerencia: validates :requires_shipping, inclusion: { in: [true, false] } en Order + un spec con null.
- Los campos nuevos editables no entran en el ETag app/poros/orders/order_version.rb:16 y app/poros/catalog/product_version.rb:36
requires_shipping se puede cambiar por PUT /orders/:id, pero OrderVersion::HEADER_FIELDS no lo incluye. Si un operador lo cambia, la versión no cambia, y otro que guardó con el If-Match viejo lo revierte sin recibir el 412. Es la pérdida de actualización que esa clase existe para evitar. Pasa lo mismo con packaging y technical_standard en ProductVersion#edited_fields.
- capacity sin tope superior → 500 por RangeError app/models/warehouse.rb:32
La validación es entero > 0, pero la columna es integer de 4 bytes. capacity: 99999999999 pasa la validación y falla al guardar con ActiveModel::RangeError, que no se mapea: 500. El repo ya documenta este caso (Shipment::MAX_SHIPPING_COST, y ProductMapping con less_than: 100_000_000).
Sugerencia: less_than_or_equal_to: 2_147_483_647 (o un máximo razonable) + spec.
Medios
6. requires_shipping sólo se hace cumplir al crear el envío app/poros/shipments/create_shipment.rb:42
ConfirmDispatch#validate_order! sólo mira los estados no despachables, y la cotización (POST /orders/:id/quotes) tampoco lo mira. Una orden con envío pending a la que después se le pone requires_shipping: false por PUT (ensure_editable! sólo bloquea las despachadas) se puede cotizar y despachar igual, o sea, pagar una etiqueta por una venta de retiro. Además, el raise PickupOrderError va antes del chequeo de cancelada: una orden cancelada de retiro responde «an order picked up at the store has no shipment», justo la confusión que motivó separar los dos errores.
- El feed muestra como event_failed los eventos ya resueltos app/poros/activity/build_feed.rb:92
Trae todas las filas de FailedEvent, incluidas las succeeded y discarded. Un evento que falló y después se reprocesó bien sigue apareciendo como un fallo nuevo en la campanita, y puede desplazar a los pending/dead del top limit. Convendría filtrar a los estados accionables o dejar claro en el contrato que el cliente debe filtrar por status.
- Las unidades de órdenes canceladas desaparecen del «En depósito» app/models/product.rb:166
Las canceladas se excluyen de lo comprometido, pero hoy nada devuelve su stock a stocks (UpdateOrder dice expresamente que la cancelación no pasa por ahí). Esas unidades están en el estante, ya salieron de stocks y ahora tampoco figuran en committed, así que on_hand_quantity queda por debajo del físico real. Es el caso contrario al de los retiros, y también contradice «lo que cuenta alguien que va y mira».
- El camino de retiro por canal no se activa con los seeds app/poros/orders/process_webhook_order.rb:143
Ninguna plantilla de Service de los seeds mapea requires_shipping (grep sólo lo encuentra en la orden manual «Retiro en Mostrador»). Entonces translated[:order][:requires_shipping] es siempre nil y toda venta de canal queda con envío, aunque el comentario diga que Shopify lo manda. Sólo lo ejercita el spec con una plantilla sintética. Conviene aclararlo en la descripción (las plantillas tienen que declararlo) o mapearlo en los seeds.
Limpieza
10. tab_count duplica la cadena de filtered_products app/controllers/api/v1/products_controller.rb:106
El invariante que la card quiere garantizar («el número de la pestaña y las filas visibles dicen lo mismo») pasa a depender de que dos copias de la cadena search → category → total stock → status se mantengan iguales. Un filtered_products(status = scalar_param(:status)) sirve a index y a counts. De paso, tab_count quedó metido entre el comentario de filtered_products y el método: éste perdió su comentario y hay dos bloques de comentario apilados sobre tab_count.
- counts hace cuatro consultas pesadas app/controllers/api/v1/products_controller.rb:65
Cada contador arma with_total_stock completo (join con stocks, GROUP BY, y la subconsulta correlacionada de in_transit_quantity por producto, que el conteo no lee). La pestaña «Todos» ni siquiera necesita el join. Un solo agregado (SUM(stocks.quantity) por producto en una subconsulta y COUNT(*) FILTER (WHERE ...)) devuelve los cuatro en una pasada.
- Order::CANCELLED lo usa un solo lugar app/models/order.rb:11
El comentario justifica la constante porque la preguntan «el alta del envío, el despacho, el cálculo de lo comprometido», pero sólo el cálculo de lo comprometido la usa. NON_SHIPPABLE_STATUSES = %w[cancelled], UpdateOrder#ensure_editable! y Order::STATUSES siguen con el literal. O se reemplazan, o se saca la constante.
Alcance y docs
13. bin/worker_windows.rb no tiene que ver con esta card bin/worker_windows.rb
No está en la descripción y no se relaciona con TESIS-162; bin/* además está excluido de RuboCop, así que nadie lo revisa por lint. Sugiero sacarlo a su propio PR (o documentarlo en architecture.md §8.3). Una duda que no pude verificar: llama a worker.stop dentro de un Signal.trap, y si SolidQueue::Worker#stop toma algún lock (por ejemplo al instrumentar con ActiveSupport::Notifications), Ruby lo prohíbe en contexto de trap (ThreadError). Más seguro es setear una bandera en el handler y parar el worker afuera.
- Un séptimo «dominio» que la documentación no conoce app/poros/activity/build_feed.rb:3
feature-structure.md define «Los seis dominios del proyecto» y dice que la lógica entre dominios va en app/poros/shared/ o en el modelo. Activity::BuildFeed lee Order, Shipment/ShipmentEvent y FailedEvent, y depende de Shipments::ConfirmDispatch::DISPATCHED_STATUS, sin que ninguna regla de dependencia lo contemple. Además, en ADR-015 (línea 52) quedó «en cualquiera de esas tres devuelve undefined», cuando la tabla ahora lista cinco endpoints.
Resumen
Antes de mergear, lo que más me preocupa es (2) (cambia el significado del número de la card y crece con los retiros) y (1) (el feed repite envíos). (3), (4) y (5) son arreglos chicos con su spec. El resto se puede resolver acá o pasar a cards aparte, a criterio de quien lleve la card.
Four files conflicted. `Product` gained `committed_by_warehouse` here and `in_transit_by_warehouse` on master, and the automatic merge left the block they share duplicated. The schema keeps the newer migration, and the two specs took both sides. The contract sweeps that landed on master (TESIS-160 and TESIS-161) caught three things this branch adds, which is what they are for: - `/activity` is an `index` that does not paginate: it is a bounded feed read whole, so a `meta` with `page` and `total` would describe a pagination that does not exist. It joins the vocabularies as a bare `data` shape, in its own list so the rule keeps its teeth for every other listing. - `products#counts` is a computed aggregate, like `reports#overview`. - The warehouse nested in a stock row now carries `capacity`. The case is still "without its load": what does not travel is `stored_units`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp
…d three 500s Seven of the findings reported on the PR. **The feed repeated shipments.** `ready_to_ship` is not written once: `RegisterTrackingEvent` stores any push the courier template cannot translate with the shipment's current status, and after dispatch that status is `ready_to_ship`. A "parcel received at branch" came back as a second dispatch of the same shipment, with the time of the push, and the repeats ate the feed's budget. It now takes one row per shipment, the earliest, which is the one `ConfirmDispatch` wrote. **The feed also reported failures that were already resolved.** A failed event that was reprocessed or dismissed by hand is not news, and showing it pushed the open ones out of the list. Only the actionable statuses travel. **A pickup stayed committed forever.** It never gets a shipment -- the only thing that closes the committed window -- and `Order` has no "picked up" status, so the figure grew with every counter sale and the on-hand number drifted above the shelf. The decision taken is that at the counter, recording the sale and handing it over are the same moment: the customer is standing there. So a pickup is not committed at all, and its units leave on-hand when `DeductStock` takes them out of `stocks`. The two numbers now say the same thing without inventing a state. **Pickups were only enforced when the shipment was opened.** An order with a pending shipment that later became a pickup through PUT could still be quoted and dispatched: a label paid for a sale that never leaves. Both now refuse it. In both places the status is checked first, so a cancelled pickup says it is cancelled -- which is the actionable part -- instead of explaining that it is picked up at the store. **Three ways to reach a 500**, all of them now 422: - `requires_shipping: null` hit the NOT NULL column. A non-boolean string is still cast to `true`: ActiveRecord does that before validating, and from the model it cannot be told apart from a real `true`. - A `capacity` larger than a 4-byte integer passed the validation and raised `ActiveModel::RangeError` on save. - Both are the same class of hole as `Shipment::MAX_SHIPPING_COST`. **The ETag did not cover every editable field.** `requires_shipping` on orders and `category`, `packaging` and `technical_standard` on products could be changed through PUT without moving the version, so two operators editing them at once never got the 412 and the second silently reverted the first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp
…docs and the seeds The cleanup half of the findings. `tab_count` carried its own copy of the search → category → total stock → status chain, so the invariant the card asks for -- that a tab's number and the rows you see agree -- depended on keeping two copies identical by hand. There is one definition now and the status travels as an argument. While collapsing them, the four counters stopped asking for `in_transit_quantity`: it is a correlated subquery, one per row, and a count never reads it. `Order::CANCELLED` justified itself by saying three places ask for it, and only one did. The other four kept the literal, which is exactly the typo the constant exists to prevent: the shipment guard, the edit guard, its error message, the reports window and the webhook ingestion all use it now. `bin/worker_windows.rb` stays, documented in architecture.md §8.3 next to the snippet it replaces, and its signal handler no longer stops the worker inside the trap: Ruby forbids taking a lock there, and `SolidQueue::Worker#stop` can take one through `ActiveSupport::Notifications`, which would turn a clean Ctrl-C into a `ThreadError`. The handler lowers a flag and the stop runs on the main thread. `feature-structure.md` said six domains while `app/poros/` had ten namespaces. The four extra ones are not domains: they have no epic, model nothing of their own and never write. They read what the six record and build a view, which is why they can name several domains without creating a cycle. That is now the rule, with `activity` in the dependency list. ADR-015 said "any of those three" over a table that lists more. The seeds built the pickup order's line without taking the unit out of `stocks`, so the phone was counted twice -- on the shelf and sold -- and the on-hand figure came out one above the shelf. The seed deducts it, like the real path does. The Shopify template still does not declare `requires_shipping`, and that is now written where someone would look for it: Shopify sends no single boolean for it, and a channel that wants to record pickups has to declare it in its own mapper. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp
|
Los 14 puntos, resueltos. Resumen de lo que cambió y de las dos decisiones que no eran mecánicas. (2) El retiro comprometido para siempre. Tenías razón en que toca la decisión de fondo. La tomé así: en el mostrador, registrar la venta y entregarla son el mismo momento —el cliente está ahí—, así que un retiro no cuenta como comprometido y sus unidades salen del on hand cuando (8) Las unidades de las canceladas. Esa no la cierro acá: lo que falta es reponer el stock al cancelar, que es exactamente TESIS-999009 (api#102). Lo dejé dicho en el mismo comentario de (1) El feed repetía envíos. Confirmado, y por el camino que marcaste: (3) (4) El ETag. (5) (6) El retiro sólo se hacía cumplir al abrir el envío. Ahora la cotización y el despacho también lo rechazan, y en los dos lugares el estado va primero: una orden cancelada de retiro responde que está cancelada, que es la confusión que marcabas. (7) Los eventos ya resueltos en el feed. Filtrado a los estados accionables. (9) Los seeds. Dos cosas: la plantilla de Shopify no declara (10) a (12) Una sola definición de la cadena con el estado por parámetro, los contadores dejaron de pedir la subconsulta de en tránsito que no leen, y (13) (14) El séptimo dominio. Son cuatro, no uno: También mergeé master, que había quedado 11 commits atrás. Los barridos de contrato que entraron con TESIS-160 y TESIS-161 agarraron tres cosas de esta rama, que es para lo que están: 1.815 ejemplos, 0 fallas. Cobertura 99,43 % línea · 95,21 % rama. RuboCop 319 archivos sin ofensas, Brakeman limpio, seeds corriendo. Cada regla nueva tiene su teeth-test: la rompí y verifiqué que falla sólo el caso que la fija. |
Sanntinat
left a comment
Sanntinat
left a comment
There was a problem hiding this comment.
Revisión — TESIS-162 (PR #115) · Reservas de stock, retiro en local, contadores del catálogo y actividad reciente
Revisión de TESIS-162-audit-stock-reservations-and-pickup (7ff8eb4), contra origin/master de proyecto-api. Ya hubo una review de @LoLoo03 sobre 40824b0, con 14 hallazgos, y Tomás respondió con 3e602d1 y 6a1c5e9. Esta revisión verifica esas respuestas y suma lo que encontré levantando la API junto con el front de TESIS-163.
| Check | Resultado |
|---|---|
| CI (GitHub Actions) | lint, scan_ruby, test y validate-pr-title en verde |
Rama vs master |
Al día (incluye #96 y #116). Mergea limpio |
7ff8eb4 (local) |
1824 ejemplos · 0 fallas · RuboCop 319 archivos sin ofensas · Brakeman 0 |
| Seeds en una base nueva | Cargan sin errores; los dos casos de capacidad y la orden «Retiro en Mostrador» quedan como dice el PR |
| Verificado en el navegador | Con proyecto-web#80 (que trae #79) contra esta API |
✅ La review anterior, resuelta
Leí los dos commits contra cada punto, y los cuatro que tocan comportamiento los probé rompiéndolos:
| Rotura | Falla |
|---|---|
Sacar .where(orders: { requires_shipping: true }) de committed_scope |
3, entre ellas ignores an order the customer picks up at the store |
Sacar el DISTINCT ON del feed |
2, entre ellas reports one dispatch per shipment, however many events it logged |
Sacar el rechazo del retiro en ShipmentQuotesController |
is refused instead of quoted |
Volver a meter succeeded y discarded en el feed |
2, entre ellas leaves out a failure that was already resolved |
El resto también está como dice la respuesta: inclusion sobre requires_shipping, los campos nuevos en los dos ETag (y category, que faltaba desde TESIS-150), MAX_CAPACITY, el estado antes que el retiro en los tres lugares, Order::CANCELLED usado de verdad, una sola cadena de filtros con el estado por parámetro, y el Signal.trap de bin/worker_windows.rb que ahora sólo baja una bandera.
En la API levantada:
GET /products/:idde NOR-005 devuelvecommitted_quantity: 4,available_to_promise: 40yon_hand_quantity: 44, con el desglose encommitted_by_warehouse.POST /orders/3/shipmentsobre «Retiro en Mostrador» → 422.GET /activitymezcla ventas y eventos caídos ordenados por fecha, y el buscador de envíos de #116 sigue funcionando sobre esta rama.
🟡 La ocupación del depósito no cuenta lo comprometido
La card define «En depósito (físico)» como stocks.quantity + comprometido, y el detalle de producto ya lo muestra así. Pero stored_units, que alimenta la barra de capacidad nueva y el KPI «Unidades en stock» del panel, sigue siendo sólo SUM(stocks.quantity) (Warehouse.with_stored_units). Con los seeds:
| Depósito Central | Unidades |
|---|---|
Libres (stocks) |
292 |
| Comprometidas (vendidas sin despachar) | 5 |
| Físicamente en el depósito | 297 |
| Lo que usa la barra contra la capacidad de 6.000 | 292 |
En los seeds la diferencia es chica. En operación es todo lo vendido y no despachado, que en un día de muchas ventas es justo lo que ocupa más lugar. El PR de front dice que la barra «mide ocupación real», y con este dato no la mide. Lo mismo pasa en el panel: el KPI dice 377 y la suma de los «En depósito» de los productos da 382.
Sugerencia: sumar lo comprometido por depósito en with_stored_units. Conviene que salga de la misma definición que committed_scope, por ejemplo un scope OrderItem.committed que usen Product y Warehouse, así que lo que cuenta como comprometido se decide en un solo lugar. No lo pongo como condición porque la card no lo pide de forma explícita, pero es la otra mitad de la decisión de fondo.
🟡 «El cliente está ahí» vale para el mostrador, no para un retiro que entra por canal
La decisión de que un retiro nunca cuenta como comprometido está bien razonada para el alta manual: el operador la carga con el cliente adelante. Pero ProcessWebhookOrder también acepta requires_shipping: false cuando la plantilla lo informa, y una venta de Shopify con retiro en el local entra horas o días antes de que el cliente pase a buscarla. Mientras tanto, las unidades siguen en el estante y no figuran ni en stocks ni en lo comprometido.
Hoy no se activa: ninguna plantilla de los seeds mapea el campo, como ya se dijo en la review anterior. Pero el comentario de committed_scope lo presenta como cierto en general. Alcanza con acotarlo («vale para el alta manual; un retiro por canal necesita un estado "retirada" que hoy no existe») para que quien sume requires_shipping a una plantilla sepa lo que deja abierto.
⚪ TESIS-999009 no es un número de card
El comentario de committed_scope y la respuesta a la review remiten a «TESIS-999009» para la reposición del stock al cancelar. Es el ID provisorio del borrador api#102, y no existe en Jira. Cuando ese PR tome su número real, el comentario va a apuntar a algo que no se puede buscar. Conviene nombrar el PR (proyecto-api#102) o levantar la card ahora.
Los criterios de la card
-
GET /products/:iddevuelve comprometido, en tránsito y disponible para prometer, por depósito y en total, sin N+1 (el listado usaProductListSerializer, que no los pide). - Sin reservas devuelve 0 y no
null. -
GET /products/countscon los cuatro contadores, respetandosearchy la categoría. -
POST /ordersaceptarequires_shipping, y una orden de retiro no abre envío (422), no cotiza ni despacha. - La ingesta por webhook setea
requires_shippingcuando el canal lo informa. -
GET /activitycon los tres tipos, ordenados y scopeados por tenant. - Empaque y norma técnica se guardan, se validan y se serializan.
- Las lecturas nuevas están en los barridos de TESIS-160 y TESIS-161.
-
rubocop,rspecybrakemanen verde.
Veredicto
APPROVE.
La review anterior está resuelta, y bien: los cuatro arreglos de comportamiento tienen un spec que los fija. Los dos 🟡 son de la misma decisión de fondo, qué cuenta como físico, y no rompen nada de lo que la card pide. El primero, el de la ocupación, conviene resolverlo acá o en una card propia antes de mostrarle la barra a alguien.
3 participants
Descripción
Sale de la auditoría general del 04/10. La mitad de los hallazgos son de pantalla pero no se pueden arreglar sin dato detrás: esta es esa mitad. La hermana de front es TESIS-163.
1. Reservas de stock: los tres números existen
El detalle de producto mostraba «Comprometido» y «Disponible para prometer» con un «—» y la aclaración de que la API no los modelaba. La auditoría pide dato real, y 0 cuando el valor es cero. Poner 0 sin modelar habría afirmado «no hay nada reservado» cuando la verdad era «el sistema no lo registra», así que había que modelarlo.
Y resultó que no hacía falta guardar nada nuevo. Verifiqué contra el código:
Catalog::DeductStockdescuenta destocksal crear la orden, no al despachar.Shipments::ConfirmDispatchnunca tocastocks.order_items.warehouse_idguarda el origen desde TESIS-126, también en la ingesta por webhook.O sea que lo vendido y no despachado ya no figura en ninguna fila de stock pero sigue en el estante, y se deriva:
pending), por depósitostocks.quantity— lo vendido ya salió de ahíUn producto que nadie reservó devuelve 0, no null. Las líneas sin depósito quedan fuera del desglose y del total, para que las filas de la pantalla siempre sumen su encabezado.
2. Retiro en local
orders.requires_shipping(defaulttrue) hace opcional el envío, en el alta manual y en las ventas de canal.CreateShipmentrechaza un retiro con un error propio,PickupOrderError, separado del de la orden cancelada: una cancelada es un callejón sin salida, un retiro es una venta sana que no se despacha, y la pantalla hace cosas distintas con cada una.3. Contadores del catálogo
/inventoryhacía seis requests; cuatro eran contadores, uno por pestaña, cada uno pidiendo una página de una fila para leermeta.total.GET /products/countsresponde los cuatro en una, respetando el mismosearchy la misma categoría que el listado —si no, el número de la pestaña y las filas visibles dirían cosas distintas—.Endpoint propio y no dentro del
metadel listado: los contadores no cambian al pasar de página, así que el cliente los pide una vez. Vocabulario,datasinmeta(ADR-015, actualizado).4. Actividad reciente
GET /activityalimenta la campanita: órdenes creadas, envíos despachados y eventos caídos a la DLQ, en una sola lista ordenada por fecha. Ya estaba comprometido: RF-26 nombra el «feed de actividad reciente de órdenes».Sin tabla de notificaciones y sin estado de leído por usuario: eso es otro dominio y nadie lo pidió. Al derivarse de lo que el sistema ya registra, no puede desincronizarse de los hechos.
ShipmentEventno tienecompany_id, así que su aislamiento entra por el join conShipment— tiene su propio test de tenant por eso.5. Capacidad, empaque y norma técnica
warehouses.capacity(nullable): la barra de ocupación compara contra un techo que no se deriva de ningún dato. Sin capacidad cargada, la pantalla no dibuja la barra. Es el único camino que no inventa el número.products.packagingyproducts.technical_standard: texto libre y no un vocabulario cerrado comocategory, porque una norma técnica es un código de un organismo externo (IRAM, IEC).Cómo probar
bin/rails db:migrate && bin/rails db:seedLos seeds traen el escenario completo: Depósito Central con capacidad 6.000 y el Satélite sin declarar (los dos casos de la barra), empaque y norma en los tres productos de Norte, y la orden «Retiro en Mostrador», que es
paidyrequires_shipping: false.GET /products/:idde NOR-001 →committed_quantity,on_hand_quantity,available_to_promiseycommitted_by_warehouse.GET /products/counts→ los cuatro contadores.POST /orders/:id/shipmentsobre la orden de retiro → 422 «an order picked up at the store has no shipment».GET /activity→ los tres tipos mezclados y ordenados.Verificación
rubocop✅ — 309 archivos, 0 ofensasrspec✅ — 1718 ejemplos, 0 fallasbrakeman -q✅ — sin warningsapi_contract_spec(TESIS-90) se puso en rojo con los campos nuevos, que es exactamente para lo que existe: actualicé las cuatro formas que fija.Notas para la review
Producty los serializers de producto para exponerstock_statusein_transit_by_warehouse. El conflicto es mecánico pero hay que resolverlo; sugiero mergear feat: [TESIS-143] expose the stock status and the incoming units per warehouse in the product detail #99 primero y rebasar éste encima.db/schema.rbcon sólo los cambios reales: mi Postgres local vuelca los CHECK constraints con otro formato y eso ensuciaba el diff entero.🤖 Generated with Claude Code
https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp