From 8585246bab16e001c1caf5f229dd43aa2dd49dcb Mon Sep 17 00:00:00 2001 From: LauAubert Date: Fri, 2 Oct 2026 01:14:23 -0300 Subject: [PATCH 1/2] feat: [TESIS-999007] aggregate the real sales and dispatches for the reports screen The reports screen (S14) shows a fixed sample dataset because there was no aggregates endpoint. Part of it can be computed with today's model, and real numbers of the company are worth more in the demo than an invented dataset. GET /api/v1/reports/overview?period=7d|30d|90d answers, for the last N calendar days in Argentina time and against the previous window of the same length: orders and revenue without cancelled ones, units dispatched (by the dispatch event of the shipment log, since shipments keep no dispatch date), a daily or weekly curve with its empty buckets, and the shipments of each carrier with how many were delivered. On-time rate and anomalies come as an explicit null: the model has nothing to compute them from. Co-Authored-By: Claude Opus 5.5 --- app/controllers/api/v1/reports_controller.rb | 19 ++++ app/policies/report_policy.rb | 8 ++ app/poros/reports/build_overview.rb | 101 ++++++++++++++++ app/poros/reports/window.rb | 59 ++++++++++ config/routes.rb | 3 + docs/guidelines/feature-structure.md | 10 +- spec/poros/reports/build_overview_spec.rb | 114 +++++++++++++++++++ spec/poros/reports/window_spec.rb | 51 +++++++++ spec/requests/api/v1/reports_spec.rb | 77 +++++++++++++ 9 files changed, 439 insertions(+), 3 deletions(-) create mode 100644 app/controllers/api/v1/reports_controller.rb create mode 100644 app/policies/report_policy.rb create mode 100644 app/poros/reports/build_overview.rb create mode 100644 app/poros/reports/window.rb create mode 100644 spec/poros/reports/build_overview_spec.rb create mode 100644 spec/poros/reports/window_spec.rb create mode 100644 spec/requests/api/v1/reports_spec.rb diff --git a/app/controllers/api/v1/reports_controller.rb b/app/controllers/api/v1/reports_controller.rb new file mode 100644 index 00000000..a66d4deb --- /dev/null +++ b/app/controllers/api/v1/reports_controller.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Api + module V1 + class ReportsController < ApplicationController + rescue_from Reports::Window::UnknownPeriodError, with: :render_bad_request + + # GET /api/v1/reports/overview?period=7d|30d|90d + # + # Un recurso y no una colección: viaja pelado, sin `data` (ADR-015). + def overview + authorize :report, :overview? + + window = Reports::Window.new(period: scalar_param(:period)) + render json: Reports::BuildOverview.new(window: window).call + end + end + end +end diff --git a/app/policies/report_policy.rb b/app/policies/report_policy.rb new file mode 100644 index 00000000..68504038 --- /dev/null +++ b/app/policies/report_policy.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +# Política sin registro (headless): un reporte no es un recurso con dueño, es una +# vista sobre los datos del tenant. El aislamiento lo da `CompanyScoped` en cada +# modelo que el PORO consulta; acá sólo se exige una sesión. +class ReportPolicy < ApplicationPolicy + def overview? = user.present? +end diff --git a/app/poros/reports/build_overview.rb b/app/poros/reports/build_overview.rb new file mode 100644 index 00000000..34848089 --- /dev/null +++ b/app/poros/reports/build_overview.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +module Reports + # Agregados de la pantalla de Reportes (S14) sobre una ventana de tiempo. + # + # Calcula sólo lo que el modelo permite calcular. El diseño tiene además el + # cumplimiento de plazo y las anomalías regionales, que no tienen dominio + # detrás (no hay fecha comprometida en `shipments`, ni una entidad de + # anomalía): esos van como `nil` explícito y no como un número inventado. + # + # Todo pasa por los modelos con `CompanyScoped`, así que el reporte es del + # tenant del request sin filtrar a mano. + class BuildOverview < ApplicationPoro + DISPATCH_STATUS = Shipments::ConfirmDispatch::DISPATCHED_STATUS + + def initialize(window:) + super() + @window = window + end + + def call + { + period: @window.period, + from: @window.from.iso8601, + to: @window.to.iso8601, + granularity: @window.granularity, + kpis: kpis, + curve: curve, + carriers: carriers + } + end + + private + + def kpis + { + orders: trended { |range| sales(range).count }, + revenue: trended { |range| sales(range).sum(:total_amount).to_f }, + dispatched_units: trended { |range| dispatched_units(range) }, + on_time_delivery_rate: nil, + active_anomalies: nil + } + end + + # El valor de la ventana y su variación porcentual contra la anterior. Sin + # nada en la anterior, la variación es `nil`: un "+100 %" o un "0 %" ahí se + # leería como un dato. + def trended + current = yield(@window.range) + previous = yield(@window.previous_range) + trend = previous.zero? ? nil : ((current - previous) * 100.0 / previous).round(1) + + { value: current, trend: trend } + end + + # Las ventas que cuentan: las canceladas no facturaron. + def sales(range) + Order.where(created_at: range).where.not(status: 'cancelled') + end + + # `shipments` no guarda cuándo se despachó: lo dice el primer evento de la + # bitácora, el `ready_to_ship` que escribe `ConfirmDispatch`. + def dispatched_shipments(range) + Shipment.where(id: ShipmentEvent.where(internal_status: DISPATCH_STATUS, occurred_at: range) + .select(:shipment_id)) + end + + def dispatched_units(range) + OrderItem.where(order_id: dispatched_shipments(range).select(:order_id)).sum(:quantity) + end + + def curve + totals = sales(@window.range).pluck(:created_at, :total_amount) + .group_by { |created_at, _| @window.bucket_for(created_at) } + + @window.buckets.map do |date| + rows = totals.fetch(date, []) + { date: date.iso8601, orders: rows.size, revenue: rows.sum { |_, amount| amount.to_f } } + end + end + + # Volumen por operador: cuántos envíos se despacharon con cada integración de + # courier en la ventana, y cuántos de esos ya se entregaron. No es el + # cumplimiento de plazo del diseño: es lo que el modelo puede decir. + def carriers + shipments = dispatched_shipments(@window.range).where.not(company_integration_id: nil) + dispatched = shipments.group(:company_integration_id).count + delivered = shipments.where(status: 'delivered').group(:company_integration_id).count + + CompanyIntegration.includes(:service).where(id: dispatched.keys) + .map { |integration| carrier_row(integration, dispatched, delivered) } + .sort_by { |row| [-row[:dispatched], row[:name]] } + end + + def carrier_row(integration, dispatched, delivered) + { company_integration_id: integration.id, name: integration.service_name, + dispatched: dispatched.fetch(integration.id, 0), + delivered: delivered.fetch(integration.id, 0) } + end + end +end diff --git a/app/poros/reports/window.rb b/app/poros/reports/window.rb new file mode 100644 index 00000000..588756ca --- /dev/null +++ b/app/poros/reports/window.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +module Reports + # La ventana de tiempo de un reporte: los últimos N días calendario, en hora de + # Argentina, desde el inicio del primer día hasta ahora. + # + # Hora de Argentina y no la del servidor (UTC): el MVP opera sólo en Argentina + # (E4b, exclusión 9), y un corte de día en UTC partiría las ventas de las + # 21 a las 24 en el día siguiente. + # + # El período anterior es la ventana del mismo largo inmediatamente antes, y es + # contra lo que se calcula la tendencia. + class Window + class UnknownPeriodError < StandardError; end + + PERIODS = { '7d' => 7, '30d' => 30, '90d' => 90 }.freeze + DEFAULT_PERIOD = '7d' + # A partir de este largo la curva va por semana: 90 puntos diarios no se + # leen en una tarjeta. + WEEKLY_FROM = 90 + TIME_ZONE = 'America/Argentina/Buenos_Aires' + + attr_reader :period, :days, :from, :to + + def initialize(period:, now: Time.current) + @period = period.presence || DEFAULT_PERIOD + @days = PERIODS.fetch(@period) do + raise UnknownPeriodError, "period must be one of #{PERIODS.keys.join(', ')}" + end + @to = now.in_time_zone(zone) + @from = (@to.to_date - (@days - 1)).in_time_zone(zone).beginning_of_day + end + + def range = from..to + + # Mismo largo, inmediatamente antes. Excluye el borde para no contar dos veces + # lo que pasó justo en el inicio de la ventana actual. + def previous_range = (from - days.days)...from + + def granularity = days >= WEEKLY_FROM ? 'week' : 'day' + + # El bucket al que pertenece un instante: su día, o el lunes de su semana. + def bucket_for(time) + date = time.in_time_zone(zone).to_date + granularity == 'week' ? date.beginning_of_week : date + end + + # Todos los buckets de la ventana, en orden, incluidos los que quedan vacíos: + # una curva sin los días en cero dibujaría una línea entre dos ventas como si + # hubiera habido ventas en el medio. + def buckets + (bucket_for(from)..to.to_date).select { |date| bucket_for(date) == date } + end + + private + + def zone = ActiveSupport::TimeZone[TIME_ZONE] + end +end diff --git a/config/routes.rb b/config/routes.rb index c5e5dabd..27f1d9e9 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -19,6 +19,9 @@ # del token. get 'me', to: 'me#show' + # Agregados de la pantalla de Reportes (S14) sobre una ventana de tiempo. + get 'reports/overview', to: 'reports#overview' + resources :integrations, only: %i[index update], param: :service_id resources :warehouses, only: %i[index show create update destroy] resources :products, only: %i[index show create update destroy] do diff --git a/docs/guidelines/feature-structure.md b/docs/guidelines/feature-structure.md index f1876b88..ba8cf79c 100644 --- a/docs/guidelines/feature-structure.md +++ b/docs/guidelines/feature-structure.md @@ -41,9 +41,12 @@ app/poros/ │ ├── create_order.rb │ ├── confirm_order.rb # Confirma orden + descuenta stock │ └── process_webhook_order.rb # Ingesta orden de un canal externo -└── shipments/ - ├── quote_shipment.rb # Cotización concurrente con múltiples couriers - └── confirm_dispatch.rb # Confirma despacho + obtiene tracking +├── shipments/ +│ ├── quote_shipment.rb # Cotización concurrente con múltiples couriers +│ └── confirm_dispatch.rb # Confirma despacho + obtiene tracking +└── reports/ + ├── window.rb # Ventana de N días en hora de Argentina + período anterior + └── build_overview.rb # Agregados de S14: ventas, unidades despachadas, couriers ``` Un PORO = un caso de uso. El nombre del archivo es el verbo + sustantivo del caso de uso. @@ -93,6 +96,7 @@ orders → catalog ✅ (confirmar orden descuenta stock) 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) ``` 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. diff --git a/spec/poros/reports/build_overview_spec.rb b/spec/poros/reports/build_overview_spec.rb new file mode 100644 index 00000000..3483ff7a --- /dev/null +++ b/spec/poros/reports/build_overview_spec.rb @@ -0,0 +1,114 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Reports::BuildOverview do + let(:company) { Company.create!(name: 'Norte', tax_id: '30-11111111-1') } + let(:now) { Time.zone.parse('2026-10-02 15:00:00 UTC') } + let(:window) { Reports::Window.new(period: '7d', now: now) } + let(:product) { Product.create!(company: company, sku: 'R-1', name: 'Reportado') } + + around { |example| Current.set(company_id: company.id) { example.run } } + + def overview = described_class.new(window: window).call + + def order(at:, amount:, status: 'paid', units: 1) + Order.create!(company: company, customer_name: 'Cliente', total_amount: amount, + status: status, created_at: at).tap do |created| + OrderItem.create!(order: created, product: product, quantity: units, unit_price: amount) + end + end + + def dispatch(order, at:, with:, delivered: false) + shipment = Shipment.create!(company: company, order: order, company_integration: with, + status: delivered ? 'delivered' : 'ready_to_ship') + ShipmentEvent.create!(shipment: shipment, internal_status: 'ready_to_ship', + external_status: 'Etiqueta generada', occurred_at: at) + end + + describe 'the sales of the window' do + before do + order(at: now - 1.day, amount: 1_000) + order(at: now - 2.days, amount: 500) + order(at: now - 1.day, amount: 9_999, status: 'cancelled') + order(at: now - 10.days, amount: 750) + end + + it 'adds up the revenue of the orders that were not cancelled' do + expect(overview[:kpis][:revenue][:value]).to eq(1_500.0) + end + + it 'counts those orders' do + expect(overview[:kpis][:orders][:value]).to eq(2) + end + + # 750 en la semana anterior contra 1.500 en esta. + it 'compares them with the previous window of the same length' do + expect(overview[:kpis][:revenue][:trend]).to eq(100.0) + end + end + + it 'answers no trend when the previous window had nothing' do + order(at: now - 1.day, amount: 1_000) + + expect(overview[:kpis][:revenue]).to eq(value: 1_000.0, trend: nil) + end + + it 'draws one point per day, the empty ones in zero', :aggregate_failures do + order(at: now - 1.day, amount: 1_000) + + expect(overview[:curve].size).to eq(7) + expect(overview[:curve].sum { |point| point[:orders] }).to eq(1) + expect(overview[:curve].count { |point| point[:orders].zero? }).to eq(6) + end + + describe 'the dispatched units and the carriers' do + # Métodos y no `let`: el grupo ya hereda cuatro helpers y + # RSpec/MultipleMemoizedHelpers corta en cinco. + def andreani = CompanyIntegration.find_by(service: Service.find_by(service_name: 'Andreani')) + def oca = CompanyIntegration.find_by(service: Service.find_by(service_name: 'OCA')) + + before do + courier_integration(company: company, name: 'Andreani') + courier_integration(company: company, name: 'OCA') + dispatch(order(at: now - 3.days, amount: 10, units: 4), at: now - 1.day, with: andreani) + dispatch(order(at: now - 3.days, amount: 10, units: 2), at: now - 2.days, with: andreani, + delivered: true) + dispatch(order(at: now - 3.days, amount: 10, units: 7), at: now - 1.day, with: oca) + # Despachada la semana anterior: no cuenta en esta ventana. + dispatch(order(at: now - 9.days, amount: 10, units: 50), at: now - 8.days, with: oca) + end + + # Por la fecha del despacho, no por la de la orden. + it 'counts the units of the shipments dispatched in the window' do + expect(overview[:kpis][:dispatched_units][:value]).to eq(13) + end + + it 'counts the shipments of each carrier and how many were delivered' do + expect(overview[:carriers]).to eq( + [{ company_integration_id: andreani.id, name: 'Andreani', dispatched: 2, delivered: 1 }, + { company_integration_id: oca.id, name: 'OCA', dispatched: 1, delivered: 0 }] + ) + end + end + + # El modelo no tiene con qué calcularlos: `nil` y no un número inventado. + it 'leaves out what the model cannot answer', :aggregate_failures do + expect(overview[:kpis][:on_time_delivery_rate]).to be_nil + expect(overview[:kpis][:active_anomalies]).to be_nil + end + + it 'only sees the orders of the current tenant' do + order_of_another_company + + expect(overview[:kpis][:revenue][:value]).to eq(0.0) + end + + def order_of_another_company + other = Company.create!(name: 'Sur', tax_id: '30-22222222-2') + Current.set(company_id: other.id) do + Order.create!(company: other, customer_name: 'Otro', total_amount: 5_000, + status: 'paid', created_at: now - 1.day) + end + end +end diff --git a/spec/poros/reports/window_spec.rb b/spec/poros/reports/window_spec.rb new file mode 100644 index 00000000..6240090c --- /dev/null +++ b/spec/poros/reports/window_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Reports::Window do + # Jueves 02/10/2026 a las 01:30 en Argentina, que en UTC ya es 04:30. + let(:now) { Time.zone.parse('2026-10-02 04:30:00 UTC') } + + def window(period = '7d') = described_class.new(period: period, now: now) + + it 'starts at the beginning of the first of the last N days, in Argentina' do + expect(window.from.iso8601).to eq('2026-09-26T00:00:00-03:00') + end + + it 'ends now' do + expect(window.to).to eq(now) + end + + it 'defaults to the last seven days' do + expect(described_class.new(period: nil, now: now).days).to eq(7) + end + + it 'refuses a period it does not know' do + expect { window('1y') }.to raise_error(described_class::UnknownPeriodError, /7d, 30d, 90d/) + end + + it 'puts the previous window right before, with the same length' do + expect(window.previous_range).to eq((window.from - 7.days)...window.from) + end + + it 'lists every day of the window, empty ones included' do + expect(window.buckets.map(&:iso8601)).to eq( + %w[2026-09-26 2026-09-27 2026-09-28 2026-09-29 2026-09-30 2026-10-01 2026-10-02] + ) + end + + # 21:30 en Argentina del 01/10 ya es 02/10 en UTC: el corte va por la hora local. + it 'buckets an instant by its day in Argentina, not in UTC' do + expect(window.bucket_for(Time.zone.parse('2026-10-02 00:30:00 UTC')).iso8601).to eq('2026-10-01') + end + + context 'with ninety days' do + it 'groups the curve by week, starting on Monday', :aggregate_failures do + buckets = window('90d').buckets + + expect(window('90d').granularity).to eq('week') + expect(buckets).to all(satisfy(&:monday?)) + expect(buckets.last.iso8601).to eq('2026-09-28') + end + end +end diff --git a/spec/requests/api/v1/reports_spec.rb b/spec/requests/api/v1/reports_spec.rb new file mode 100644 index 00000000..266c2010 --- /dev/null +++ b/spec/requests/api/v1/reports_spec.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Reports 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(user) + post '/api/v1/auth/login', params: { email: user.email, password: 'password123' }, + headers: { 'X-Tenant-Slug' => user.company.slug } + { 'Authorization' => "Bearer #{response.parsed_body['token']}" } + end + + def order_of_another_company + other = Company.create!(name: 'Tenant B', tax_id: '30-22222222-2') + Current.set(company_id: other.id) do + Order.create!(company: other, customer_name: 'Otro', total_amount: 9_000, status: 'paid') + end + end + + describe 'GET /api/v1/reports/overview' do + it 'returns 401 without a token' do + get '/api/v1/reports/overview' + + expect(response).to have_http_status(:unauthorized) + end + + it 'answers the overview of the last seven days by default', :aggregate_failures do + get '/api/v1/reports/overview', headers: headers + + expect(response).to have_http_status(:ok) + expect(response.parsed_body['period']).to eq('7d') + expect(response.parsed_body['curve'].size).to eq(7) + end + + # Un recurso viaja pelado (ADR-015): sin `data` ni `meta`. + it 'answers a bare resource with the agreed keys', :aggregate_failures do + get '/api/v1/reports/overview', params: { period: '30d' }, headers: headers + + expect(response.parsed_body.keys) + .to match_array(%w[period from to granularity kpis curve carriers]) + expect(response.parsed_body['kpis'].keys) + .to match_array(%w[orders revenue dispatched_units on_time_delivery_rate active_anomalies]) + end + + it 'counts the orders of the tenant of the token' do + Order.create!(company: company, customer_name: 'Cliente', total_amount: 1_200, status: 'paid') + + get '/api/v1/reports/overview', headers: headers + + expect(response.parsed_body.dig('kpis', 'revenue', 'value')).to eq(1_200.0) + end + + it 'never adds up the orders of another company' do + order_of_another_company + + get '/api/v1/reports/overview', headers: headers + + expect(response.parsed_body.dig('kpis', 'revenue', 'value')).to eq(0.0) + end + + it 'returns 400 for a period it does not know', :aggregate_failures do + get '/api/v1/reports/overview', params: { period: '1y' }, headers: headers + + expect(response).to have_http_status(:bad_request) + expect(response.parsed_body['error']).to eq('period must be one of 7d, 30d, 90d') + end + + it 'returns 400 when the period comes as a list' do + get '/api/v1/reports/overview', params: { period: ['7d'] }, headers: headers + + expect(response).to have_http_status(:bad_request) + end + end +end From 9b298fa6ae74235dac954e91cb319830e97842a0 Mon Sep 17 00:00:00 2001 From: LauAubert Date: Fri, 2 Oct 2026 02:13:49 -0300 Subject: [PATCH 2/2] test: [TESIS-999007] pin the reports overview in the API contract ADR-015 asks every new endpoint to join the contract spec: the overview is a bare resource with fixed keys, and so are its KPIs, curve points and carriers. Co-Authored-By: Claude Opus 5.5 --- spec/requests/api/v1/api_contract_spec.rb | 31 ++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/spec/requests/api/v1/api_contract_spec.rb b/spec/requests/api/v1/api_contract_spec.rb index af16773f..6c49bd68 100644 --- a/spec/requests/api/v1/api_contract_spec.rb +++ b/spec/requests/api/v1/api_contract_spec.rb @@ -50,7 +50,11 @@ module ContratoDeLaApi envio: %w[id order_id status tracking_number shipping_label_url shipping_cost courier events created_at updated_at], evento: %w[id internal_status external_status description occurred_at created_at], - meta: %w[page per_page total] + meta: %w[page per_page total], + reporte: %w[period from to granularity kpis curve carriers], + reporte_kpis: %w[orders revenue dispatched_units on_time_delivery_rate active_anomalies], + punto_curva: %w[date orders revenue], + operador: %w[company_integration_id name dispatched delivered] }.freeze end @@ -83,6 +87,14 @@ def product end end + # Un envío despachado hoy con un courier: lo que el reporte cuenta por operador. + def dispatch_with_a_courier + sent = Shipment.create!(company: company, order: order, status: 'ready_to_ship', + company_integration: courier_integration(company: company)) + ShipmentEvent.create!(shipment: sent, internal_status: 'ready_to_ship', + external_status: 'Etiqueta generada', occurred_at: Time.current) + end + def order @order ||= begin o = Order.create!(company: company, customer_name: 'Cliente', customer_address: 'Av. 2', @@ -141,6 +153,23 @@ def shipment expect(response.parsed_body.keys).to include('sku') end + # Un reporte es un recurso calculado, no una colección: viaja pelado. + it 'answers the reports overview as a bare resource with the agreed keys', :aggregate_failures do + get '/api/v1/reports/overview', headers: headers + + expect(response.parsed_body.keys).to match_array(claves[:reporte]) + expect(response.parsed_body['kpis'].keys).to match_array(claves[:reporte_kpis]) + expect(response.parsed_body['curve'].first.keys).to match_array(claves[:punto_curva]) + end + + it 'answers each carrier of the overview with the agreed keys' do + dispatch_with_a_courier + + get '/api/v1/reports/overview', headers: headers + + expect(response.parsed_body['carriers'].first.keys).to match_array(claves[:operador]) + end + # Era el único que devolvía un array pelado. Un array en la raíz no admite # `meta` sin romper a quien lo consume, así que ninguna colección puede # quedar así.