Skip to content

feat: [TESIS-163] audit the screens: header, panel, pickups, stock figures and the shipments listing - #79

Open
TomasMartin2004 wants to merge 9 commits into
masterfrom
TESIS-163-audit-frontend
Open

TomasMartin2004 wants to merge 9 commits into
masterfrom
TESIS-163-audit-frontend

Conversation

@TomasMartin2004

@TomasMartin2004 TomasMartin2004 commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Descripción

Sale de la auditoría general del 04/10 y cubre los diez hallazgos de pantalla. Después se le sumaron dos rondas más: lo que pediste al levantarlo en el navegador, y los hallazgos de la review de @LoLoo03.

Depende de proyecto-api#115 (TESIS-162). Sin esa API, los tres números del stock, la capacidad, el empaque, la norma, el retiro en el local y la campanita no tienen de dónde leer.

Cambió de alcance respecto de la descripción original. El listado de envíos iba a ir en un PR aparte y terminó entrando acá (commit c7be77d), y el KPI de «Eventos fallidos» que se proponía como reemplazo se descartó. El detalle está en «Cómo llegó a esto».

1. El header

  • Se va el buscador. NavSearch nunca recibió un onSearch: era un input decorativo que no buscaba nada. Se van el componente, su copy y sus estilos.
  • Se va «Mi perfil». El ítem existía pero Header nunca pasó onProfileClick, no hay pantalla de perfil y ningún RF la pide. El menú queda con quién tiene la sesión y «Cerrar sesión».

2. La campanita abre la actividad

No hacía nada: Header no pasaba onNotificationsClick y el badge sólo lo encendía la demo del design system. Ahora abre un Popover sobre GET /activity con las últimas órdenes, despachos y eventos caídos a la DLQ.

Vive en shared/components porque quien lo monta es el header, que está en app/ y no puede importar una feature; por eso las rutas de destino llegan por props.

3. El panel

Se van el KPI «Salud del sistema» y la card «Integraciones» —y con ellos useInfraHealth, IntegrationNodeList, NodeStatusIcon y useNow, que quedaban sin consumidor—.

  • KPI «Unidades en stock» — el inventario del que vive la operación. Sale del mismo dato que ya alimenta la carga por depósito: ni un request más.
  • Card «Últimos envíos» — los últimos despachados con su operador y su estado, cada fila enlazando a su orden.

Sobre RF-26. El requisito enumera cuatro elementos del panel y uno es textualmente «salud de los nodos de integración». Este PR lo saca del panel y no lo reemplaza por un equivalente. La decisión es del dueño del producto, no mía, y la dejo dicha acá para que se vea: si se quiere conservar la trazabilidad contra E4a, hace falta una card que decida dónde vive esa salud. La cola de eventos fallidos ya tiene su pantalla propia (/failed-events, TESIS-147, mergeada), que es donde ese dato es accionable.

4. Retiro en el local

  • Paso 2: una sección nueva, «Cómo la recibe el cliente», con envío a domicilio o retiro en el local.
  • Paso 3: con retiro no cotiza ni despacha y se puede confirmar sin elegir operador. useConfirmDraftOrder acepta dispatch: null y termina en el alta.
  • Detalle y listado: una orden de retiro lo dice, en vez de verse igual que una a la que le falta el envío.

5. Los tres números del stock

Comprometido, en tránsito y disponible para prometer muestran cifras, y 0 cuando el valor es cero.

Ninguno se deriva en el cliente: onHand no es la suma de stocks[].quantity, porque lo vendido sin despachar ya salió de esas filas y sigue en el estante.

Categoría, empaque y norma técnica también salen de la API, así que no queda en pantalla ningún aviso de «esto todavía no existe en la API».

6. La barra de ocupación

En los depósitos que declararon capacidad la barra mide ocupación real y la fila lo dice; en los que no, sigue comparando contra el más cargado. Cada fila dice contra qué se mide, porque las dos barras se ven igual y significan cosas distintas.

7. «Editar stock» edita stock

El botón montaba EditProductModal entero. Ahora el modal tiene un scope, y con "stock" muestra sólo las cantidades por depósito. Es un alcance del mismo modal y no un segundo componente a propósito: el guardado, el If-Match y el 412 (TESIS-101) son los mismos.

8. El catálogo resuelve en tres requests

useProductCounts disparaba cuatro consultas, una por pestaña. Con GET /products/counts (TESIS-162) la pantalla pasa de 6 requests a 3.

9. El listado de envíos (/shipments)

Pestañas por estado del ciclo y paginado sobre GET /api/v1/shipments, que existía desde TESIS-113 y sólo alimentaba el KPI del panel.

No hay pantalla de detalle del envío y no es un olvido: el ciclo de vida, la bitácora y la etiqueta viven en el detalle de su orden (S08), así que cada fila lleva ahí. La acción va como el ojito directo en cada fila y no detrás de un kebab: abrir un menú para elegir lo único que tiene es un clic que no compra nada (f286040).

Cómo llegó a esto

Tres rondas, y conviene leerlas por separado al revisar.

Ronda 1 — la auditoría (cbd1149). Los puntos 1 a 8 de arriba. Acá el reemplazo del KPI de «Salud del sistema» era «Eventos fallidos».

Ronda 2 — lo que apareció al levantarlo (c7be77d, f286040). Cuatro cosas, y dos cambian lo de arriba:

  • El KPI de «Eventos fallidos» se descartó y lo reemplaza «Unidades en stock».
  • Entró el listado de envíos, que la descripción original mandaba a otro PR. Lo que decía ese párrafo sigue siendo cierto —ningún RF pide un listado; RF-21 a RF-25 son modelo, cotización, despacho, webhook y cron— pero la pantalla hacía falta igual y mandarla a un PR aparte era trámite.
  • El panel de notificaciones es más alto: cada fila ocupa más.
  • Las cifras de stock mostraban NaN; formatUnits ahora devuelve «—» para lo que no es finito.

Ronda 3 — la review de @LoLoo03 (774191e, ec3a40e, e300092). Merge con master más seis hallazgos:

Hallazgo Qué pasaba
Caché Nada invalidaba ['shipments'] ni ['activity']. Crear una orden abre un envío y escribe en la campanita, despachar cambia su estado, y editar puede dejarla sin envío: tres pantallas quedaban viejas los cinco minutos del staleTime.
Retiro perdido requiresShipping estaba en el store pero no en su partialize: recargar en el paso 2 o 3 lo devolvía a true y convertía un retiro en envío sin avisar.
Cotizaciones de más El paso 3 leía la bandera del store mientras el resto venía de la copia congelada. onOrderCreated vacía el borrador apenas existe la orden, así que a mitad de confirmar un retiro salía a pedirles precio a los couriers y el botón quedaba deshabilitado. Ahora viaja dentro del snapshot.
Stock maestro sin cerrar El titular mostraba totalStock mientras una cubeta contaba lo comprometido: nunca sumaban. Pasa a onHand, que es comprometido + disponible y que la API ya mandaba sin que nadie lo leyera.
KPI con cero falso Sumaba sobre la lista vacía cuando /warehouses fallaba y anunciaba cero unidades. Cero es una respuesta real y ésta no lo era: ahora no hay número y la card dice que no se pudo cargar.
Campos ausentes capacity ausente pasaba el !== null y daba «NaN %»; una capacidad de cero hacía lo mismo por 0/0. Y una orden sin requires_shipping se leía como retiro, porque undefined es falsy: marcaba todas así.

Un hallazgo más se resolvió solo al mergear master: un depósito cuya última unidad se vendió —sin fila de stock, con las unidades todavía en el estante— desaparecía de la tabla de distribución. distributionPositions (TESIS-144) ya sabía ver los depósitos sin fila; sólo le faltaba conocer lo comprometido (TESIS-162). Cada rama tenía la mitad de esa tabla.

El merge con master

Trece archivos en conflicto. La mayoría aditivos; tres pedían decidir:

  • Las filas del detalle de producto salen ahora de distributionPositions y cada una lleva su comprometido.
  • El selector de categoría del modal se queda con el de master (TESIS-150) y se descarta el campo deshabilitado de esta rama, adentro del onlyStock que ésta agrega.
  • Los textos de «esto todavía no existe en la API» (empaque, norma, comprometido, disponible) se van: TESIS-162 expone los cuatro.

Además, ApiProduct quedó con in_transit_quantity declarado dos veces después del merge automático, y los fixtures de cinco archivos de test necesitaban los campos que cada lado agregó.

Cómo probar

Con la API de proyecto-api#115 (bin/rails db:migrate && bin/rails db:seed), npm run dev y admin@norte.com:

  1. Header: sin buscador; el menú de cuenta no ofrece «Mi perfil».
  2. Campanita: abre el historial; cada fila de orden lleva a su detalle.
  3. Panel: KPI «Unidades en stock» y card «Últimos envíos»; la barra del Depósito Central dice «% de su capacidad» y la del Satélite, unidades —los seeds dejan uno de cada caso—.
  4. Envíos: /shipments lista con pestañas por estado; el ojito de cada fila abre la orden sin pasar por un menú.
  5. Inventario → NOR-001: el titular del stock maestro es lo que hay en el estante y las tres cubetas lo descomponen; «Editar stock» abre sólo las cantidades.
  6. Órdenes → «Retiro en Mostrador»: el detalle dice que la retira el cliente y el listado la marca.
  7. Nueva orden: en el paso 2 elegí «Retiro en el local», recargá la página —la elección sobrevive— y confirmá en el paso 3 sin elegir operador.
  8. Despachá un envío desde el detalle de una orden y volvé a /shipments: la fila y los contadores de las pestañas ya están al día.

Verificación

  • npm run lint ✅
  • npm run test ✅ — 83 archivos, 783 tests (eran 698 en la primera ronda)
  • npm run build ✅ (tsc -b incluido)
  • npm run format:check ✅

Cada regla nueva tiene su teeth-test: la rompí y verifiqué que falla sólo el caso que la fija. Dos no mordieron al primer intento y los rehice — el del KPI probaba la página con el hook mockeado, así que la regla se fija en el test del hook; y el del paso 3 necesitaba reproducir la ventana real en la que el borrador ya se vació.

Lo que queda afuera

Los nueve hallazgos menores de la review no entran acá: duplicados de helpers entre features contra la Regla de Dos, accesibilidad de las filas de la campanita y de «Últimos envíos», el parpadeo del modal al cerrar, los cinco requests de los contadores de envíos, el destino que el paso 2 sigue exigiendo con retiro, y comentarios obsoletos. Son limpieza sin riesgo funcional y este PR ya es grande; van como card aparte.

Choques con PRs abiertos

Resueltos: #60 (TESIS-144) y #66 (TESIS-150) se mergearon y el conflicto con los dos está resuelto en 774191e.

proyecto-web#80 (TESIS-164) sale de esta rama y le agrega el buscador al listado de envíos. Conviene mergear ésta primero.

🤖 Generated with Claude Code

https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp

…how real stock figures

From the system audit of 04/10. Nine of its ten screen findings; the tenth, the
shipments listing, goes in its own PR (see below).

**The header.** The search box is gone: it never received an `onSearch`, it was
a decorative input that searched nothing. "Mi perfil" is gone too — the header
never passed its handler, there is no profile screen and no RF asks for one, so
the menu keeps who is signed in and "Cerrar sesión" instead of an action that
leads nowhere.

**The bell opens the activity feed.** It did nothing: `Header` never passed
`onNotificationsClick` and only the design system demo lit the badge. It now
opens a panel over `GET /activity` with the last orders, dispatches and events
that fell to the retry queue. It lives in `shared/components` because the
header is in `app/` and cannot import a feature, so its destinations arrive as
props.

**The panel.** The "Salud del sistema" KPI and the Integraciones card are gone.
RF-26 names "salud de los nodos de integración" among the four elements of the
panel, so the replacements are not filler: the "Eventos fallidos" KPI is the
same health said as a number somebody can act on, and the "Últimos envíos" card
reinforces the other dimension RF-26 names. Both come from endpoints that
already existed, so no mock anywhere.

**Pickup orders.** Step 2 now asks how the customer gets the order. With a
pickup, step 3 neither quotes nor dispatches, and the order detail and the
listing say so: an order that carries no shipment stopped looking like one that
is missing it.

**Real stock figures.** Committed, in transit and available to promise show
numbers, and **zero when the value is zero** — the figure exists and is zero.
Category, packaging and technical standard come from the API too, so every
"this does not exist in the API yet" notice is off the screen.

**The occupancy bar is real** where the warehouse declared its capacity, and
keeps comparing warehouses against each other where it did not.

**«Editar stock» edits stock.** It used to mount the whole product form. It is
a scope of the same modal and not a second one: the save, the `If-Match` and
the 412 are the same and duplicating them would mean two versions of the
delicate part.

**The catalogue resolves in three requests**, not six: the four tab counters
were one request each, reading `meta.total` of a page of one row.

Depends on proyecto-api#115 (TESIS-162).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp
@TomasMartin2004
TomasMartin2004 requested a review from a team as a code owner October 4, 2026 20:42
@TomasMartin2004
TomasMartin2004 requested review from LoLoo03 and removed request for a team October 4, 2026 20:42
TomasMartin2004 and others added 2 commits October 4, 2026 19:56
…d of feedback

Four things from trying it in the browser.

**NaN.** The three stock figures printed `NaN` against an API that does not
send them yet — a front deployed before its backend. `formatUnits` now answers
with the em dash for anything that is not a finite number: TypeScript says it
cannot happen, but the value comes from the API, and printing `NaN` at the
operator is the one outcome nobody wants.

**The failed events KPI is out.** "Unidades en stock" takes its place: the
inventory the operation lives off, from the same data that already feeds the
load per warehouse, so it costs no extra request. It does mean the panel no
longer carries anything about the health of the integration nodes, which RF-26
names — that was the whole argument for the previous KPI, and the call was made
twice, so it goes.

**The activity rows got taller.** The panel is a history somebody reads, not a
menu scanned with the eyes: each row now carries an icon tile for its kind, sits
on 14px of vertical padding, separates with a divider, and the panel widened
from 360 to 420.

**The shipments listing exists.** `/shipments`, with tabs over the lifecycle and
pagination on `GET /api/v1/shipments`, which has been there since TESIS-113 and
only fed the panel KPI. There is no screen for a single shipment and that is
not an oversight: its lifecycle, its log and its label live in the detail of its
order, which is where the operator decides something, so every row leads there.

Three cells say what is missing rather than drawing a blank, because the courier,
the tracking and the cost are all written by the dispatch: a shipment with no
quote does not cost zero, it has no price yet.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp
The listing had a single action behind a kebab, and its column reused the
"Estado" header, so the table showed that word twice: once over the badge
and once over a menu that only held "Ver la orden". Opening a menu to pick
the only thing in it is a click that buys nothing.

The action is now its own column with the eye in every row, under an
"Acciones" header. Each button names the shipment it belongs to, because the
icon repeats down the table and "Ver la orden" alone would not say which one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp

@LoLoo03 LoLoo03 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reporté 15 hallazgos sobre el PR #79 con ReportFindings. Revisé el head de la rama TESIS-163-audit-frontend (los dos commits, 96 archivos), con los archivos completos vía git show. No corrí lint, los tests ni el build.

Este PR no es lo que dice su descripción. El segundo commit, c7be77d, saca el KPI «Eventos fallidos» y lo reemplaza por «Unidades en stock». Además agrega el listado /shipments, que la descripción dice que va en otro PR. Así, RF-26 («salud de los nodos de integración») ya no figura en ningún lado del panel.

Los más graves, por orden:

Cache de envíos nunca se invalida. Nada invalida ['shipments'] ni ['activity'], y staleTime es de 5 minutos. «Últimos envíos», los contadores de la pestaña Envíos y la campanita quedan viejos después de despachar o crear una orden.
Stock del detalle inconsistente. El titular «Stock maestro» y el badge siguen sumando stocks[], mientras las tres cubetas usan los números de la API, así que pueden contradecirse. Product.onHand se mapea y nadie lo lee. Además, el comprometido de un depósito sin fila de stock no aparece en la tabla.
requiresShipping no se persiste. No está en partialize, así que recargar en el paso 2 o 3 convierte un retiro en envío sin avisar.
Retiro: el paso 3 consulta cotizaciones de más. clearDraft devuelve requiresShipping a true a mitad de la confirmación, pero el snapshot ConfirmedDraft no lo incluye. Eso dispara un POST /quotes contra los couriers, hace parpadear la pantalla y deja el botón de confirmar deshabilitado.
«Unidades en stock» muestra 0 si falla /warehouses. Debería mostrar «—», como dice el comentario de formatCount.
Campos ausentes de la API se renderizan mal. capacity ausente da una barra con «NaN %» y un requires_shipping ausente marca todas las órdenes como retiro. Es el mismo riesgo de desplegar el front antes que el back que c7be77d ya cubrió solo para formatUnits.

Después hay hallazgos menores:

Retiro en el paso 2: el destino se sigue exigiendo, aunque un comentario dice que deja de pedirse.
Barras de carga: las barras sin capacidad ya no dicen contra qué se miden.
Pestaña Envíos: el encabezado de la columna de acciones dice «Estado».
Modal «Editar stock»: parpadea al cerrar.
Contadores de envíos: cinco requests por pestaña, el patrón que este PR quita del catálogo.
Duplicados: helpers copiados entre features, contra la Regla de Dos.
Tests: faltan tests del feed, de fetchRecentShipments y del mapeo de comprometidos.
Accesibilidad: filas de la campanita y de «Últimos envíos».
Comentarios: hay comentarios obsoletos.

Todo salió de leer el código; no los reproduje en el navegador, salvo el formateo de horas con node. En Node 24, hour12: false imprime 00:14 y no 24:14, así que el riesgo de medianoche depende del navegador.

TomasMartin2004 and others added 3 commits October 5, 2026 08:22
Thirteen files conflicted, all of them between this branch and what landed in
master meanwhile. Most were additive and resolved by keeping both sides; three
needed a decision:

- The product detail rows now come from `distributionPositions` (TESIS-144,
  which sees warehouses with no stock row) and each row carries its committed
  units (TESIS-162). Before the merge each side had half of the table.
- `distributionPositions` learned about `committedByWarehouse`, so a warehouse
  whose last unit was sold -- no stock row left, units still on the shelf --
  stops disappearing from the table. That was one of the review findings on
  this PR, and it falls out of merging the two features rather than being a
  separate fix.
- The category field of the edit modal keeps master's picker (TESIS-150) and
  drops this branch's disabled placeholder, inside the `onlyStock` guard this
  branch added.

The copy that said committed, available-to-promise, packaging and technical
standard "do not exist in the API yet" is gone: TESIS-162 exposes all four.

`ApiProduct` had `in_transit_quantity` declared twice after the automatic
merge, and the product fixtures of five test files needed the fields each side
added.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp
…and absent fields

Five of the findings reported on this PR.

The shipments listing, its tab counters and the bell were never invalidated.
Creating an order opens a shipment and writes an activity entry, dispatching
one changes its state, and editing an order can drop it, but the three
mutations only refreshed orders and inventory, so three screens kept showing
the previous state for the five minutes of staleTime. `['shipments']` goes in
as a literal because it belongs to another feature; the activity feed lives in
`shared`, so that one uses its own key factory.

`requiresShipping` was in the draft store but not in its `partialize`, so
reloading on step 2 or 3 dropped it back to its `true` default and silently
turned a pickup into a shipment.

Three places trusted a field to be there:

- A warehouse with no `capacity` passed the `!== null` check and made the bar
  `NaN %`. A capacity of zero did the same through `0 / 0`. Absent now reads as
  "not declared", and the guard is `> 0`.
- The stored-units KPI summed an empty list when `/warehouses` failed and
  announced zero units. Zero is a real answer -- the company stores nothing --
  and this is not it, so the figure is now absent and the card says the load
  could not be loaded instead of claiming there are no warehouses.
- An order whose API does not send `requires_shipping` was read as a pickup,
  because `undefined` is falsy. Both mappings default to shipping, which is
  what every order was before pickups existed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp
…ckets add up to

Two more findings from the review.

Step 3 kept reading `requiresShipping` from the store while everything else
came from the copy frozen when the button was pressed. `onOrderCreated` empties
the draft as soon as the order exists, and that puts the flag back to its
`true` default, so mid-confirmation a pickup started asking the carriers for
prices, the panel flickered into the carrier list and the confirm button went
disabled for want of a chosen option. The flag now travels inside the snapshot,
and the whole render reads it from there.

The master stock card headlined `totalStock` -- the units nobody sold yet --
while one of its buckets counted the committed ones. The two never added up:
committed + available-to-promise is `onHand`, which the API already sends and
nothing read. The headline is now `onHand` and the card closes, with in-transit
still outside it because those units are in no warehouse yet. Its caption
counts distribution positions instead of `stocks`, so a warehouse whose last
unit was sold still counts.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp
@TomasMartin2004 TomasMartin2004 changed the title feat: [TESIS-163] clean the header and the panel, model pickups and show real stock figures feat: [TESIS-163] audit the screens: header, panel, pickups, stock figures and the shipments listing Oct 5, 2026
@TomasMartin2004

Copy link
Copy Markdown
Contributor Author

Actualicé la descripción: el alcance cambió respecto de la original y eso era parte de lo que marcaste.

  • El listado de envíos iba a ir en un PR aparte y terminó entrando acá (c7be77d).
  • El KPI de «Eventos fallidos» se descartó y lo reemplaza «Unidades en stock».
  • RF-26 queda sin representación en el panel. No lo tapo: lo dejé dicho explícitamente en la descripción, con el razonamiento, para que se decida como card aparte y no se pierda contra E4a.

La descripción ahora separa las tres rondas (auditoría / lo que salió al levantarlo / tu review) para que se pueda revisar commit por commit.

De tus seis hallazgos graves, los seis están resueltos. Uno —el comprometido de un depósito sin fila de stock— salió de mergear master: distributionPositions (TESIS-144) ya sabía ver esos depósitos y sólo le faltaba conocer lo comprometido (TESIS-162). Cada rama tenía la mitad de esa tabla.

Dos cosas que vale aclarar sobre tu review:

Los nueve hallazgos menores los dejé afuera a propósito y están listados en «Lo que queda afuera»: son limpieza sin riesgo funcional y este PR ya es grande.

Suite en 783 tests, lint, build y format en verde. Cada regla nueva tiene su teeth-test.

… the modal and the rows

The rest of the review, minus two that became their own cards.

Step 2 demanded a delivery address even for a pickup. A comment right above
the section said it stopped being asked for, and it did not: the form gated
the button on the destination schema whatever the choice. Now it only gates
when the order ships, the section says the address is optional, and whatever
was typed still travels -- the customer's address can matter for the invoice,
and step 3 needs a destination to not send you back.

The load bars without a declared capacity said how many units a warehouse
holds but not what the bar compares against, while the ones with a capacity
read as an occupancy. The two look identical and mean different things, so the
card now says it once below the list. Their accessible label was also keyed on
`capacity === null`, which disagreed with the bar itself for a capacity of
zero: it would have announced "of 0 units of capacity" while the bar compared
against the fullest warehouse.

The stock editor flashed the full product form on its way out: the scope was
read as `editing ?? 'product'`, and closing set `editing` to null while the
modal was still animating. Open state and scope are two pieces of state now,
and the scope only changes when something opens.

Rows in the bell panel that lead nowhere were still announced as buttons --
`ListItemButton` does that even mounted on a `div` -- and closed the panel when
touched. A row that only informs no longer promises an action. In "Últimos
envíos" the icon of every row carried the card's own title as its label, which
is noise to a screen reader, and the row's name now says what activating it
does.

Three footnotes explained that the API did not expose the committed units, the
per-warehouse in-transit, the packaging or the technical standard. It exposes
all four since TESIS-162 and TESIS-144, so the text, the copy and the three
props nothing passes any more are gone.

New tests for the feed, for `fetchRecentShipments` and for the warehouse load
mapping, which the review asked for.

Two findings left this PR as cards of their own, because both reach well past
it: the five requests behind the shipment tab counters (TESIS-165, it needs an
endpoint) and the ten copies of three number formatters spread over seven
features (TESIS-166).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp
@TomasMartin2004

Copy link
Copy Markdown
Contributor Author

Los nueve hallazgos menores, cerrados. Siete acá y dos como cards propias, porque los dos se van bien lejos de este PR.

Paso 2 con retiro. Tenías razón y era peor de lo que decía el punto: el comentario arriba de la sección decía que el destino dejaba de pedirse y el formulario lo seguía exigiendo igual, porque el botón se apoyaba en el schema de destino pasara lo que pasara. Ahora sólo lo exige cuando hay envío, la sección dice que el domicilio es opcional, y lo que se haya escrito viaja igual —puede hacer falta para la factura, y el paso 3 necesita un destino para no mandar de vuelta al 2—.

Las barras sin capacidad. Lo dice una línea abajo de la lista, una sola vez. Y encontré algo más mientras lo miraba: el rótulo accesible se apoyaba en capacity === null, que no coincide con lo que hace la barra para una capacidad de cero: habría anunciado «de 0 unidades de capacidad» mientras la barra comparaba contra el más cargado.

El modal. Era scope={editing ?? 'product'}: al cerrar, el alcance volvía a product mientras el modal todavía se estaba yendo. Ahora abierto y alcance son dos estados, y el alcance sólo cambia al abrir.

Accesibilidad. Las filas de la campanita que no llevan a ningún lado se anunciaban como botones —ListItemButton lo hace aunque se lo monte sobre un div— y además cerraban el panel al tocarlas. Ya no prometen una acción. En «Últimos envíos» el ícono de cada fila llevaba el título de la tarjeta como rótulo, que para un lector de pantalla es ruido, y el nombre de la fila ahora dice a dónde lleva.

Comentarios obsoletos. Los tres footnote del detalle decían que la API no exponía el comprometido, el en tránsito por depósito, el empaque o la norma. Los expone los cuatro desde TESIS-162 y TESIS-144, así que se fueron el texto, el copy y los tres props, que ya no los pasaba nadie.

Tests. Los tres que pedías: el feed (ActivityPanel.test.tsx, nuevo), fetchRecentShipments (dashboard/api.test.ts, nuevo) y el mapeo de comprometidos, que quedó cubierto en stock.test.ts al mergear master.

Los dos que no entran:

  • TESIS-165 — los cinco requests de los contadores de envíos. No se arregla acá: hace falta un GET /shipments/counts, que es backend.
  • TESIS-166 — los formateadores duplicados. Son diez definiciones de tres funciones repartidas en siete features, y la mayoría no las trajo este PR. Tocarlas acá haría este PR todavía más grande para arreglar deuda ajena. Vale la pena mirar la card: ya hay un caso de lo que preocupa —TESIS-163 endureció formatUnits para que lo no finito salga «—», y ese endurecimiento vive en una sola de las cuatro copias.

Actualicé también la descripción del PR, que era la otra mitad de lo que marcaste: el alcance cambió respecto de la original y ahora lo dice, incluida la parte incómoda de que RF-26 queda sin representación en el panel.

803 tests (eran 698), lint, build, tsc y format en verde. Teeth-test en cada regla nueva.

TESIS-140, 141 and 142 landed. Three of the four conflicts are this branch
meeting what it already assumed:

- The router still carried the integrations route next to the shipments one it
  adds. TESIS-140 removed that screen, so only shipments stays.
- The order detail merges both sides: master's `shipmentAction`, which already
  resolves dispatching and opening the shipment, plus the `pickup` flag this
  branch adds.
- Step 3 only differed in the import order master left behind.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp

@Sanntinat Sanntinat left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revisión — TESIS-163 (PR #79) · Auditoría de pantallas: header, panel, campanita, retiro en local, stock con dato real y listado de envíos

Revisión de TESIS-163-audit-frontend (424fb36), contra origin/master de proyecto-web. Ya hubo una review de @LoLoo03 con 15 hallazgos, y Tomás respondió en ec3a40e, e300092 y 18d41c7. Después mergeó master otra vez (424fb36), con TESIS-140, 141 y 142 adentro. Esta revisión se apoya sobre todo en el navegador, con la API de proyecto-api#115 y sus seeds.

Check Resultado
CI (GitHub Actions) Los cuatro jobs en verde
Rama vs master Al día. Mergea limpio
424fb36 (local) ESLint, Prettier y tsc -b limpios · 86 archivos · 823 tests · 0 fallas
Verificado en el navegador Panel, campanita, /shipments, detalle de producto, detalle de una orden de retiro

🔴 Una orden de retiro ofrece «Crear envío», y falla

Abrí la orden «Retiro en Mostrador» de los seeds (/orders/3). La tarjeta del ciclo de vida dice bien «El cliente retira esta orden en el local. No lleva envío.», pero al lado está el botón «Crear envío». Al apretarlo, la API responde 422 (PickupOrderError, de #115) y el operador ve el toast genérico «No pudimos crear el envío de la orden».

Es el primer criterio de la card que toca el retiro: «Una orden con retiro en local no ofrece crear envío». La card lo había anticipado: «desde TESIS-141 la segunda ofrecería "Crear envío", que es justo lo que no corresponde».

De dónde sale. No estaba en ninguna de las rondas: lo trajo el último merge con master. TESIS-141 (#77) agregó el botón con canOpenShipment(order.data.status, shipmentView), que mira el estado de la orden y si existe el envío, pero no requiresShipping. OrderDetailPage le pasa a la tarjeta pickup y action por separado, y ShipmentLifecycleCard dibuja la acción aunque pickup sea true. Git no lo marcó como conflicto porque cada rama tocó líneas distintas, y ningún test lo ve: la suite da 823 de 823 con el botón a la vista.

El arreglo es que canOpenShipment reciba requiresShipping (o que la página no arme la acción cuando la orden es de retiro), con un test del detalle que fije que una orden de retiro no muestra el botón.

🟡 El panel «Datos del envío» de esa misma orden habla de un envío que no va a existir

En la misma pantalla, la columna derecha dice «Número de seguimiento: Pendiente de despacho», «Etiqueta de envío: Se emite al despachar» y «Envío: Sin cotizar» en el resumen de pago. Para una orden que no lleva envío las tres afirmaciones son falsas. Lo que pide la card es justamente distinguir «falta crear el envío» de «esta orden no lleva envío». Alcanza con que el panel diga lo mismo que la tarjeta, o que no se muestre para un retiro.

🟡 La columna «En depósito» de la distribución no es lo que está en el depósito

En el detalle de NOR-005, el titular dice 44 unidades (onHand, físico) y las cubetas lo descomponen bien en 4 comprometidas y 40 disponibles. Pero la tabla de distribución, en su columna «En depósito», muestra la cantidad libre de cada fila: Depósito Central 15, cuando ahí hay 15 libres y 4 comprometidas, o sea 19 en el estante. Las filas suman 40 y el titular dice 44.

Es la misma palabra con dos significados en la misma pantalla. La card define «En depósito (físico)» como libre + comprometido. Como cada fila ya tiene su comprometido, la columna puede sumarlo. Si no, conviene renombrarla a «Disponible». (ProductDetailPage.tsx:166, onHand: formatUnits(position.quantity)).

Relacionado, y del lado de la API (lo dejé en #115): la barra de capacidad del panel usa stored_units, que tampoco cuenta lo comprometido. La descripción dice que la barra «mide ocupación real», y con ese dato no la mide.

🟡 RF-26 queda sin dueño

Está bien que la descripción lo diga de frente: el panel deja de mostrar «salud de los nodos de integración», que es uno de los cuatro elementos que RF-26 enumera. La card había elegido los reemplazos para que eso no pasara, y el KPI de «Eventos fallidos» que lo cubría se descartó en la ronda 2. La descripción dice que hace falta una card que decida dónde vive esa salud, pero esa card no existe: las que se levantaron fueron TESIS-165 y TESIS-166. Pido levantarla antes del merge, para que la trazabilidad contra E4a no dependa de que alguien relea esta descripción.

✅ Lo que verifiqué y está bien

  • Header: sin buscador y sin «Mi perfil».
  • Campanita: abre «Actividad reciente» con ventas y eventos caídos, ordenados por hora.
  • Panel: «Unidades en stock» (377) y «Últimos envíos» con operador y estado. El Depósito Central dice «% de su capacidad».
  • /shipments: pestañas con contadores (Todos 20, Pendientes 2, Listos 1, En tránsito 6, Entregados 11), paginado y el ojito directo en cada fila.
  • Detalle de producto: los tres números con dato real, 0 donde corresponde, y ningún aviso de «esto todavía no existe en la API».
  • La review anterior: los seis hallazgos graves y los menores están resueltos como dice la respuesta, y lo que quedó afuera tiene card (TESIS-165 y TESIS-166).

Los criterios de la card

  • El header no tiene buscador y el menú no ofrece acciones que no llevan a ningún lado.
  • La campanita abre el historial.
  • El panel no muestra «Salud del sistema» ni «Integraciones», y las piezas nuevas salen de datos reales (ver 🟡 de RF-26).
  • /shipments lista, filtra y pagina, y cada fila abre la orden.
  • Una orden con retiro en local no ofrece crear envío — ver 🔴.
  • Los tres números del stock muestran cifras, con 0 donde corresponde.
  • El catálogo resuelve con tres requests.
  • «Editar stock» abre un modal que sólo edita cantidades.
  • Ningún aviso de «esto todavía no existe en la API».
  • Lint, test y build en verde.

Veredicto

REQUEST CHANGES, por el 🔴.

Es un arreglo chico, pero cae justo sobre un criterio de la card y deja al operador ante un botón que siempre falla. No lo trajo el trabajo de este PR sino el último merge con master, y por eso ninguna review anterior pudo verlo. Con el botón condicionado y su test, y la card de RF-26 levantada, apruebo. Los dos 🟡 de pantalla conviene resolverlos acá, porque son el mismo tipo de confusión que la card viene a sacar.

…80)

The listing shipped with tabs and pagination but no way in. When a buyer
calls, the operator holds the tracking number the courier issued or the id
the sales channel gave the order, and neither could be typed anywhere.

A search box goes in the header, debounced so a sixteen-character tracking
number is one request and not sixteen. The term travels to the API as the
`search` param TESIS-164 added on the backend: the filtering is the database's
job, because filtering here would only see the page already fetched.

Three things the term has to respect:

- It goes into the tab counters too. Without that the tabs would keep
  reporting how many shipments the company has while the table shows three.
- It resets the page. Searching from page 3 returned an empty page, since the
  result of a search fits in one.
- It narrows the active tab instead of replacing it.

The empty table now says which emptiness it is: a tab with no shipments and a
search with no matches read the same to the code and not to the person, so the
page picks the message and the table takes it as a prop.


Claude-Session: https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants