Repository navigation
feat: [TESIS-149] show the real numbers of the company in the reports screen - #64
Conversation
…rts screen The reports screen rendered a fixed sample dataset with a "sample data" badge, because there was no aggregates endpoint. It now reads GET /reports/overview: revenue and dispatched units with their trend, the daily or weekly curve, and how many shipments of each carrier were delivered. What the model cannot compute comes as null and is shown without value, saying why: on-time rate needs a committed date that shipments do not keep, and there is no anomaly entity, which is not the same as zero anomalies. The carrier column stops painting rates in semantic tones, since a shipment still on its way is not bad service. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Revisado. Lo veo bien para implementar. Lo mergearía por lo que saca, no por lo que agrega: hoy S14 muestra un dataset inventado con el cartel «Datos de muestra», y RF-26 (Panel de Control Analítico) es alcance comprometido. Defender un panel con números fabricados es el riesgo más caro de toda esta tanda. Lo que más me gustó Que se hayan borrado los umbrales de nivel de servicio inventados ( Lo que verifiqué
Un nit Quedan el texto Antes de mergear Depende de proyecto-api#101, que hay que mergear primero. Ese PR además está con conflicto contra |
TomasMartin2004
left a comment
There was a problem hiding this comment.
Revisión del diff completo. El código es idéntico al que leí cuando los PRs estaban en draft —ningún commit nuevo—, así que lo que sigue es el veredicto formal.
✅ Aprobado
Lo apruebo por lo que saca, no por lo que agrega: hoy S14 muestra un dataset inventado con el cartel «Datos de muestra», y RF-26 es alcance comprometido. Defender un panel con números fabricados es el riesgo más caro de toda la tanda.
Lo que más me gustó
Que se hayan borrado los umbrales de nivel de servicio inventados (98 / 95 / 90) en vez de dejarlos alimentados con datos reales. La tarjeta pasa a ser «Entregas por operador» con la justificación correcta: sin fecha comprometida en el modelo no hay incumplimiento que pintar, y un envío en camino no es mal servicio. Es menos vistoso que el diseño y es lo honesto. Lo mismo con cumplimiento de plazo y anomalías, que llegan en null y se muestran sin dato diciendo por qué.
Lo que verifiqué
sampleData.tsno se borra pero deja de alimentar la pantalla: queda como fixture de los tests, y está dicho en el comentario del archivo.useReportsOverviewhace una sola consulta por período, así tarjetas, curva y tabla no pueden terminar hablando de ventanas distintas.- Los rótulos de la curva se arman en UTC para no correr el día. Es el error clásico y está contemplado.
- El spec cambia de
flags the numbers as sample dataano longer flags...: que el test se invierta explícitamente, en vez de borrarse, deja el cambio de contrato a la vista en el diff.
🟡 Un nit
Quedan el texto sampleData / sampleDataHint en content.ts y la fila de ReportsHeader en architecture.md describiendo el distintivo. No molesta, pero si no va a volver a usarse, borrarlo evita que dentro de dos meses alguien crea que la pantalla todavía muestra datos de muestra.
⚠️ Antes de mergear
Depende de proyecto-api#101, que hay que mergear y rebasar primero.
Sanntinat
left a comment
There was a problem hiding this comment.
Revisión — TESIS-149 (PR #64) · Reportes con los datos reales de la empresa
Revisión de TESIS-999008-reports-real-data (2086116), contra origin/master de proyecto-web.
| Check | Resultado |
|---|---|
| CI (GitHub Actions) | Los cuatro jobs en verde |
Rama vs master |
2 commits atrás. Mergea limpio |
Sobre el merge con master (local) |
lint, prettier y tsc -b limpios · 81 archivos · 737 tests · 0 fallas |
| Dependencia | proyecto-api#101 ya está en master |
| Navegador | Contra la API de master, con los seeds de cuatro semanas de Norte (TESIS-156), en los tres períodos |
| Conflictos con otros PRs | Sólo architecture.md, con #68, #79 y #80. Es mecánico: este PR reescribe la sección de reports y ellos tocan otras |
✅ La pantalla muestra lo que responde la API
Comparé cada período contra la respuesta cruda de GET /reports/overview:
| Período | API | Pantalla |
|---|---|---|
| 30 días | dispatched_units 104 (+845,5 %), revenue 6.474.281,94 (sin tendencia), Andreani 5/9, Correo 5/7 |
«104 · +845,5 %», «$ 6,5 MM» sin chip, «5 de 9 · 55,6 %», «5 de 7 · 71,4 %» |
| 7 días | 14 (−74,1 %), 1.880.796,99 (+145,9 %), Andreani 0/3, Correo 0/1 | «14 · −74,1 %», «$ 1,9 MM · +145,9 %», «0 de 3 · 0 %», «0 de 1 · 0 %» |
| 90 días | 115, 6.474.281,94, 13 puntos semanales | «115», «$ 6,5 MM», 13 rótulos del 06/07 al 28/09 |
- En 7 días los rótulos van de «lun» a «dom», y el primer punto (28/09) es lunes. En 90 días, día/mes del lunes de cada semana. La descripción del gráfico cambia de «órdenes por día» a «órdenes por semana».
- Cumplimiento y anomalías muestran «—» con su nota. La tabla dice que el sistema todavía no registra anomalías, distinto de «ninguna en el período».
- Una tendencia
nullno dibuja el chip, en vez de un «0 %» que se leería como dato.
✅ Las decisiones están bien fundadas
- «Entregas por operador» en un solo tono. El argumento es justo: un envío despachado ayer que todavía viaja no es mal servicio, y el rojo del diseño diría eso.
- El rótulo armado a mano en vez de
Intlcon2-digit, y el día de la semana en UTC.
Los dientes, probados:
| Rotura | Falla |
|---|---|
La API traduce anomalías a [] en vez de null |
leaves without value what the model cannot compute |
La tabla no distingue null de vacío |
shows without value and explains what the model cannot compute |
| El día de la semana en la zona del navegador y no en UTC | names the weekday in a one-week report, turns the dates of the curve into axis labels, empty days included (corriendo en hora de Argentina) |
🔴 En el período por defecto (30 días) el eje X es ilegible
Con 30 puntos diarios, los 30 rótulos se dibujan todos y quedan pegados: en pantalla se lee «05/0906/0907/0908/0909/09…», y el último («04/10») queda cortado. Medido en el navegador a 1280 px: cada rótulo ocupa 29 px, el espacio entre rótulos es 0 y 12 de los 30 se pisan con el anterior. Es lo primero que se ve al entrar a Reportes, porque «Últimos 30 días» es el período por defecto.
Con el dataset de muestra no pasaba, porque traía pocos puntos. El cuerpo del PR deja la evidencia visual «pendiente de captura», que es justo donde se habría visto.
Alcanza con que el gráfico muestre un subconjunto de rótulos cuando no entran: por ejemplo, uno de cada ceil(n / 8), siempre con el primero y el último. La curva y los puntos siguen siendo los 30. Conviene probarlo en chart.ts como función pura (qué índices se rotulan para 7, 13 y 30 puntos).
🟡 El eje Y rotula ticks que no existen (no es de este PR, pero lo deja a la vista)
axisTicks divide el techo en cuartos (con techo 10: 10 · 7,5 · 5 · 2,5 · 0), y formatCompact los redondea a enteros. En la pantalla quedan:
- 30 días: «10 · 8 · 5 · 3 · 0» (son 7,5 y 2,5).
- 7 días: «2 · 2 · 1 · 1 · 0» (son 1,5 y 0,5): el eje repite valores.
Viene de TESIS-64 y no lo introduce este PR. Con los miles del dataset de muestra no se notaba; con las órdenes por día de una empresa real, sí. Lo anoto para una card aparte: o techos múltiplos de 4 cuando la serie es de conteos, o un decimal en esos ticks.
⚪ Quedaron restos del dataset de muestra
ReportsHeaderconserva la propsampleDatay el chip «Datos de muestra», ycontent.tssus dos textos, aunque ya nadie la pasa. SisampleData.tsqueda sólo como fixture, la prop no tiene consumidor.ReportsPage.tsx, líneas 17 y 22, todavía dicen «nivel de servicio».api.ts, línea 11, citaTESIS-999007: ahora la card es TESIS-148.
Los criterios de la card
- La pantalla muestra lo que responde la API para el período elegido (los números, verificados en los tres períodos). El rotulado de 30 días es lo que falla.
- Cumplimiento y anomalías se muestran sin dato y con su explicación.
- Tests del mapeo de la API, de la tarjeta de operadores y de la página.
-
lint,test,builden verde.
Veredicto
REQUEST CHANGES, por el 🔴.
Los números están bien y las decisiones también. Pero en la vista por defecto el eje X no se puede leer, y es lo primero que se va a mostrar en la demo. Con el raleo de rótulos, apruebo.
Ticket de Jira
https://proyectofinalfrlp.atlassian.net/browse/TESIS-149
Descripción
La pantalla de Reportes (S14) mostraba un dataset de muestra fijo con el distintivo «Datos de muestra», porque no había endpoint de agregados. Con proyecto-api#101 (card 007), este PR la conecta a
GET /reports/overview: facturación y unidades despachadas con su tendencia contra el período anterior, la curva por día o por semana, y cuántos envíos de cada operador ya se entregaron.Depende de proyecto-api#101. Mergear la API primero.
Decisiones que conviene mirar:
Sin dato, pero diciendo por qué. Cumplimiento de plazo y anomalías activas llegan en
null: los envíos no guardan una fecha comprometida y no existe una entidad de anomalía. Las dos tarjetas del diseño quedan, con «—» y una nota que lo explica, en vez de desaparecer o mostrar un cero que se leería como dato. La tabla de anomalías distinguenull(«el sistema todavía no las registra») de una lista vacía («no hay en el período»).«Nivel de servicio» pasa a «Entregas por operador». El diseño muestra entregas en plazo; el modelo sólo puede decir cuántos de los envíos despachados en el período ya llegaron («171 de 180», 95 %). Las barras van en un solo tono: con los umbrales semánticos del diseño, un envío despachado ayer que todavía viaja pintaría de rojo a un operador que no hizo nada mal. Por eso se elimina
serviceLevelTone.Rótulos de la curva. Las fechas llegan como días calendario ya cortados en hora de Argentina. En 7 días se rotulan con el día de la semana («lun»), y en 30 y 90 con día/mes («28/09»), armado a mano porque, según la versión de ICU,
es-ARcon2-digitda «28/09» o «28/9», y el rótulo es la identidad del punto. El día de la semana se formatea en UTC para no correr la fecha en navegadores al oeste de UTC.fetchReportsOverviewcontra la API, contoOverviewycurveLabelexportados para probarlos solos.types.ts:onTimeDeliveryRate,activeAnomaliesyanomaliesadmitennull;granularity;carriersen lugar deserviceLevels.ReportMetricsmuestra sin dato las métricas ennull;ServiceLevelCardpasa a entregas por operador con estado vacío;AnomaliesTabledistinguenullde vacío;DispatchCurveCarddescribe la curva por día o por semana.ReportsPagedeja de mostrar el distintivo de muestra.sampleData.tsqueda como fixture de las pruebas de componentes.reportsendocs/guidelines/architecture.md.Evidencia visual
Pendiente de captura con proyecto-api#101 levantada.
Cómo probar
Precondición: API con proyecto-api#101,
bin/rails db:seed, login con un usuario de Norte.Verificación:
npm run test(684 tests, 0 fallas),npm run lint,npm run format:checkynpm run buildlimpios.Impacto y consideraciones
¿Introduce breaking changes?
No para el usuario. Requiere el endpoint de proyecto-api#101: sin él, la pantalla muestra su error con reintento.
¿Requiere nuevas variables de entorno?
No
¿Afecta la arquitectura o genera un nuevo patrón?
No.
api.tsya estaba pensado como el único archivo que cambiaba el día que existiera el endpoint.🤖 Generated with Claude Code