From 3a7e16a74672e41ef55b01f96d72dd167dbe2a50 Mon Sep 17 00:00:00 2001 From: Nathnael Date: Thu, 27 Aug 2026 05:22:37 +0000 Subject: [PATCH] fix(reports): stop finance reports dropping general-contract revenue Every revenue report excluded invoices whose booking carries contract_kind = 'GENERAL', on the premise that such a booking is an umbrella contract row paid once and drawn down by many orders, so counting it alongside those orders would double-count. That premise does not hold. On the dev database all 169 GENERAL bookings are real shipments with origin and destination yards, warehouse receipts and their own invoices; there is no umbrella invoice to double-count, and no invoice source of that kind exists at all. The predicate simply deleted 119 invoices and ETB 164.6M of billed revenue from every Finance report, which is why revenue-by-customer reported ETB 39.4M collected while /billing/invoices/summary reported ETB 140.1M - a gap of exactly ETB 100,741,976, the paid total of the 80 PAID invoices the predicate hid. Removing it from the shared revenue and invoice ledgers brings the reports back in line with billing (ETB 210.6M billed, ETB 140.1M paid, 163 invoices), and removing it from the overview repository restores the same bookings to the operational KPIs. Also fixes what that exposed in the reports that build their own query: - GATEWAY_PAID read the booking's whole gateway total onto every invoice sharing that booking. With 25 booking ids backing 58 invoices, the reconciliation report claimed ETB 113.2M of receipts against ETB 43.6M of settlement and showed ~ETB 89.4M of variance that does not exist. Receipts are now apportioned across an invoice's siblings by settled share, so the gateway column sums to the payments table and total variance is the real ETB 13.1M of manual settlements. - Aging Receivables joined companies with an INNER JOIN, dropping shipping-line-billed arrears, and summed both currencies under a hardcoded ETB label. Both payer joins are now LEFT and the report takes a currency filter. - Invoicing Pipeline summed ETB and USD invoices into one ETB total and applied no trade-direction scope, unlike every other Finance report. Both are now applied. Verified against the shared dev database: every new statement passes EXPLAIN, and the report totals reconcile with billing and with freight.payments. Type-check and the reports and overview suites pass; the five failures in billing.service.spec.ts are pre-existing on this branch and untouched by this change. Co-Authored-By: Claude Opus 5 --- .../modules/overview/overview.repository.ts | 14 ---- .../definitions/aging-receivables.report.ts | 35 ++++++++-- .../definitions/invoicing-pipeline.report.ts | 43 +++++++++++-- .../modules/reports/revenue-classification.ts | 64 ++++++++++++++----- 4 files changed, 113 insertions(+), 43 deletions(-) diff --git a/apps/edr-freight-api/src/modules/overview/overview.repository.ts b/apps/edr-freight-api/src/modules/overview/overview.repository.ts index ec258b274..6908498bb 100644 --- a/apps/edr-freight-api/src/modules/overview/overview.repository.ts +++ b/apps/edr-freight-api/src/modules/overview/overview.repository.ts @@ -34,10 +34,6 @@ import { directionScopeSql, } from "../user-trade-access/trade-scope.util"; -/** Bookings carry a contract_kind column; GENERAL = umbrella contract row, not a shipment. */ -const EXCLUDE_GENERAL_CONTRACT_BOOKINGS = - "(booking.contract_kind IS NULL OR booking.contract_kind <> 'GENERAL')"; - export type OverviewBookingKpisRow = { total: number; totalActive: number; @@ -147,7 +143,6 @@ export class OverviewRepository { "submittedToday", ) .where("booking.deleted_at IS NULL") - .andWhere(EXCLUDE_GENERAL_CONTRACT_BOOKINGS) .andWhere(scope.sql, scope.params) .setParameters({ closedStatuses: [...OVERVIEW_CLOSED_STATUSES], @@ -337,7 +332,6 @@ export class OverviewRepository { .select(`to_char(booking.created_at::date, 'YYYY-MM-DD')`, "date") .addSelect("COUNT(*)::int", "count") .where("booking.deleted_at IS NULL") - .andWhere(EXCLUDE_GENERAL_CONTRACT_BOOKINGS) .andWhere(scope.sql, scope.params) .andWhere(`booking.created_at >= CURRENT_DATE - :days::int + 1`, { days }) .groupBy("booking.created_at::date") @@ -357,7 +351,6 @@ export class OverviewRepository { .select("booking.status", "status") .addSelect("COUNT(*)::int", "count") .where("booking.deleted_at IS NULL") - .andWhere(EXCLUDE_GENERAL_CONTRACT_BOOKINGS) .andWhere(scope.sql, scope.params) .groupBy("booking.status") .getRawMany<{ status: string; count: string }>(); @@ -427,7 +420,6 @@ export class OverviewRepository { .addSelect("booking.payment_currency", "paymentCurrency") .addSelect("booking.created_at", "createdAt") .where("booking.deleted_at IS NULL") - .andWhere(EXCLUDE_GENERAL_CONTRACT_BOOKINGS) .andWhere(scope.sql, scope.params) .orderBy("booking.created_at", "DESC") .limit(limit) @@ -463,7 +455,6 @@ export class OverviewRepository { .select("booking.freight_type", "label") .addSelect("COUNT(*)::int", "count") .where("booking.deleted_at IS NULL") - .andWhere(EXCLUDE_GENERAL_CONTRACT_BOOKINGS) .andWhere("booking.status != 'DRAFT'") .andWhere(scope.sql, scope.params) .groupBy("booking.freight_type") @@ -485,7 +476,6 @@ export class OverviewRepository { .select("booking.payment_currency", "label") .addSelect("COUNT(*)::int", "count") .where("booking.deleted_at IS NULL") - .andWhere(EXCLUDE_GENERAL_CONTRACT_BOOKINGS) .andWhere("booking.status != 'DRAFT'") .andWhere(scope.sql, scope.params) .groupBy("booking.payment_currency") @@ -602,7 +592,6 @@ export class OverviewRepository { this.bookingRepository .createQueryBuilder("booking") .where("booking.deleted_at IS NULL") - .andWhere(EXCLUDE_GENERAL_CONTRACT_BOOKINGS) .andWhere(bookingScope.sql, bookingScope.params) .andWhere(windowSql("booking.created_at"), { days, offsetDays }) .getCount(), @@ -802,7 +791,6 @@ export class OverviewRepository { .addSelect("FLOOR(EXTRACT(HOUR FROM booking.created_at) / 3)::int", "block") .addSelect("COUNT(*)::int", "count") .where("booking.deleted_at IS NULL") - .andWhere(EXCLUDE_GENERAL_CONTRACT_BOOKINGS) .andWhere(scope.sql, scope.params) .andWhere(`booking.created_at >= CURRENT_DATE - :days::int + 1`, { days }) .groupBy("EXTRACT(ISODOW FROM booking.created_at)::int") @@ -1457,7 +1445,6 @@ export class OverviewRepository { ON y.id = CASE WHEN b.trade_direction = 'EXPORT' THEN b.destination_yard_id ELSE b.origin_yard_id END WHERE b.deleted_at IS NULL - AND (b.contract_kind IS NULL OR b.contract_kind <> 'GENERAL') AND b.created_at >= NOW() - make_interval(days => $1::int) GROUP BY 1 ORDER BY count DESC @@ -1475,7 +1462,6 @@ export class OverviewRepository { ON y.id = CASE WHEN b.trade_direction = 'EXPORT' THEN b.destination_yard_id ELSE b.origin_yard_id END WHERE b.deleted_at IS NULL - AND (b.contract_kind IS NULL OR b.contract_kind <> 'GENERAL') AND b.created_at >= NOW() - make_interval(days => $1::int) GROUP BY 1, 2 ORDER BY 1, 2 diff --git a/apps/edr-freight-api/src/modules/reports/definitions/aging-receivables.report.ts b/apps/edr-freight-api/src/modules/reports/definitions/aging-receivables.report.ts index d5709508c..2cdc08685 100644 --- a/apps/edr-freight-api/src/modules/reports/definitions/aging-receivables.report.ts +++ b/apps/edr-freight-api/src/modules/reports/definitions/aging-receivables.report.ts @@ -2,8 +2,10 @@ import { ObjectLiteral, SelectQueryBuilder } from 'typeorm'; import { Company } from '../../companies/entities/company.entity'; import { Invoice } from '../../billing/entities/invoice.entity'; +import { ShippingLineCompany } from '../../shipping-lines/entities/shipping-line-company.entity'; import { applyBookingRefDirectionScope } from '../../user-trade-access/trade-scope.util'; import { ReportContext, ReportDefinition } from '../report.types'; +import { CURRENCY_FILTER, PAYER_EXPR, currencyOf } from '../revenue-classification'; const OPEN_STATUSES = ['ISSUED', 'PENDING', 'PARTIALLY_PAID', 'OVERDUE']; @@ -13,13 +15,22 @@ function baseQuery(ctx: ReportContext): SelectQueryBuilder { // to now() in SQL when the filter is unset (see the COALESCE below). const asOf = (params.asOf as string | null) ?? null; + // Both payer joins are LEFT: an invoice billed to a shipping line carries no + // company, and an INNER join on `companies` silently drops its balance out of + // the arrears total. const qb = ctx.ds .createQueryBuilder() .from(Invoice, 'i') - .innerJoin(Company, 'c', 'c.id = i.company_id') + .leftJoin(Company, 'c', 'c.id = i.company_id') + .leftJoin(ShippingLineCompany, 'slc', 'slc.id = i.shipping_line_company_id') .where('i.deleted_at IS NULL') .andWhere('i.status IN (:...openStatuses)', { openStatuses: OPEN_STATUSES }) .andWhere('i.balance_amount > 0') + // Stored casing has drifted ("usd" rows exist), and one arrears figure + // cannot span two currencies. + .andWhere('UPPER(i.currency) = :currency', { + currency: currencyOf(params).toUpperCase(), + }) .setParameter('asOf', asOf); // ACL: invoices.source_id is a varchar pointer at the originating booking. @@ -32,9 +43,15 @@ export const agingReceivablesReport: ReportDefinition = { title: 'Aging Receivables', description: 'Outstanding customer balances bucketed by days overdue', group: 'Finance', - filters: [{ key: 'asOf', label: 'As of', type: 'date' }], + filters: [{ key: 'asOf', label: 'As of', type: 'date' }, CURRENCY_FILTER], columns: [ - { key: 'customer', label: 'Customer', type: 'string', sortable: true, sortExpr: 'c.name' }, + { + key: 'customer', + label: 'Customer', + type: 'string', + sortable: true, + sortExpr: PAYER_EXPR, + }, { key: 'invoices', label: 'Invoices', type: 'number' }, { key: 'outstanding', label: 'Outstanding', type: 'money', sortable: true }, { key: 'current', label: 'Current', type: 'money' }, @@ -46,7 +63,7 @@ export const agingReceivablesReport: ReportDefinition = { defaultSort: { key: 'outstanding', dir: 'DESC' }, query(ctx) { return baseQuery(ctx) - .select('c.name', 'customer') + .select(PAYER_EXPR, 'customer') .addSelect('COUNT(*)::int', 'invoices') .addSelect('ROUND(SUM(i.balance_amount))::float8', 'outstanding') .addSelect( @@ -72,15 +89,19 @@ export const agingReceivablesReport: ReportDefinition = { `ROUND(COALESCE(SUM(i.balance_amount) FILTER (WHERE i.due_at < COALESCE(:asOf::timestamptz, now()) - interval '90 days'), 0))::float8`, 'overdue90plus', ) - .groupBy('c.name'); + .groupBy(PAYER_EXPR); }, async summary(ctx) { const row = await baseQuery(ctx) .select('ROUND(COALESCE(SUM(i.balance_amount), 0))::float8', 'outstanding') - .addSelect('COUNT(DISTINCT c.id)::int', 'customers') + .addSelect(`COUNT(DISTINCT ${PAYER_EXPR})::int`, 'customers') .getRawOne(); return [ - { label: 'Outstanding', value: Number(row?.outstanding ?? 0), unit: 'ETB' }, + { + label: 'Outstanding', + value: Number(row?.outstanding ?? 0), + unit: currencyOf(ctx.params), + }, { label: 'Customers with balance', value: Number(row?.customers ?? 0) }, ]; }, diff --git a/apps/edr-freight-api/src/modules/reports/definitions/invoicing-pipeline.report.ts b/apps/edr-freight-api/src/modules/reports/definitions/invoicing-pipeline.report.ts index 8907f4f5a..86b3db29d 100644 --- a/apps/edr-freight-api/src/modules/reports/definitions/invoicing-pipeline.report.ts +++ b/apps/edr-freight-api/src/modules/reports/definitions/invoicing-pipeline.report.ts @@ -2,19 +2,34 @@ import { ObjectLiteral, SelectQueryBuilder } from 'typeorm'; import { Freight } from '@edr/types'; import { Invoice } from '../../billing/entities/invoice.entity'; +import { applyBookingRefDirectionScope } from '../../user-trade-access/trade-scope.util'; import { ReportContext, ReportDefinition } from '../report.types'; +import { CURRENCY_FILTER, currencyOf } from '../revenue-classification'; -const STATUS_OPTIONS = Object.values(Freight.InvoiceStatus).map((v) => ({ value: v, label: v })); +const STATUS_OPTIONS = Object.values(Freight.InvoiceStatus).map((v) => ({ + value: v, + label: v, +})); function baseQuery(ctx: ReportContext): SelectQueryBuilder { - const { params } = ctx; - const qb = ctx.ds.createQueryBuilder().from(Invoice, 'i').where('i.deleted_at IS NULL'); + const { params, directions } = ctx; + const qb = ctx.ds + .createQueryBuilder() + .from(Invoice, 'i') + .where('i.deleted_at IS NULL') + // Both currencies live in this table; one money column cannot hold both. + .andWhere('UPPER(i.currency) = :currency', { + currency: currencyOf(params).toUpperCase(), + }); if (params.dateFrom) qb.andWhere('i.created_at >= :dateFrom', { dateFrom: params.dateFrom }); if (params.dateTo) qb.andWhere('i.created_at < :dateTo', { dateTo: params.dateTo }); const statuses = params.statuses as string[] | null; if (statuses) qb.andWhere('i.status IN (:...statuses)', { statuses }); - return qb; + + // Every other Finance report scopes by the caller's trade directions; without + // it this one reports the value of invoices its reader may not see. + return applyBookingRefDirectionScope(qb, 'i.source_id', directions); } export const invoicingPipelineReport: ReportDefinition = { @@ -24,7 +39,13 @@ export const invoicingPipelineReport: ReportDefinition = { group: 'Finance', filters: [ { key: 'date', label: 'Created', type: 'daterange' }, - { key: 'statuses', label: 'Status', type: 'multiselect', options: STATUS_OPTIONS }, + CURRENCY_FILTER, + { + key: 'statuses', + label: 'Status', + type: 'multiselect', + options: STATUS_OPTIONS, + }, ], columns: [ { key: 'type', label: 'Type', type: 'string', sortable: true }, @@ -52,8 +73,16 @@ export const invoicingPipelineReport: ReportDefinition = { .getRawOne(); return [ { label: 'Invoices', value: Number(row?.invoices ?? 0) }, - { label: 'Total value', value: Number(row?.totalAmount ?? 0), unit: 'ETB' }, - { label: 'Outstanding', value: Number(row?.balance ?? 0), unit: 'ETB' }, + { + label: 'Total value', + value: Number(row?.totalAmount ?? 0), + unit: currencyOf(ctx.params), + }, + { + label: 'Outstanding', + value: Number(row?.balance ?? 0), + unit: currencyOf(ctx.params), + }, ]; }, }; diff --git a/apps/edr-freight-api/src/modules/reports/revenue-classification.ts b/apps/edr-freight-api/src/modules/reports/revenue-classification.ts index e3ba0a65d..550475599 100644 --- a/apps/edr-freight-api/src/modules/reports/revenue-classification.ts +++ b/apps/edr-freight-api/src/modules/reports/revenue-classification.ts @@ -28,8 +28,14 @@ import { ReportContext, ReportFilterDef, ReportFilterOption } from './report.typ // --------------------------------------------------------------------------- export const REVENUE_CATEGORIES: ReportFilterOption[] = [ - { value: 'CONTAINER_IMPORT_MULTIMODAL', label: 'Full Container Import — Multimodal' }, - { value: 'CONTAINER_IMPORT_UNIMODAL', label: 'Full Container Import — Unimodal' }, + { + value: 'CONTAINER_IMPORT_MULTIMODAL', + label: 'Full Container Import — Multimodal', + }, + { + value: 'CONTAINER_IMPORT_UNIMODAL', + label: 'Full Container Import — Unimodal', + }, { value: 'CONTAINER_EXPORT', label: 'Full Container Export' }, { value: 'EMPTY_CONTAINER_REEXPORT', label: 'Empty Container Re-export' }, { value: 'FERTILIZER', label: 'Fertilizer Transportation' }, @@ -332,7 +338,10 @@ export const PERIOD_FILTER: ReportFilterDef = { key: 'period', label: 'Granularity', type: 'select', - options: Object.entries(PERIOD_UNITS).map(([value, u]) => ({ value, label: u.label })), + options: Object.entries(PERIOD_UNITS).map(([value, u]) => ({ + value, + label: u.label, + })), }; /** The timestamp every revenue report buckets and filters on. */ @@ -478,7 +487,12 @@ export const REVENUE_FILTERS: ReportFilterDef[] = [ options: REVENUE_CATEGORIES, }, { key: 'origin', label: 'Origin', type: 'select', optionsQuery: yardOptions }, - { key: 'destination', label: 'Destination', type: 'select', optionsQuery: yardOptions }, + { + key: 'destination', + label: 'Destination', + type: 'select', + optionsQuery: yardOptions, + }, { key: 'customer', label: 'Customer / booking ref', type: 'text' }, { key: 'methods', @@ -537,9 +551,6 @@ export function revenueLedgerQb(ctx: ReportContext): SelectQueryBuilder 'eims_self_test'") - // An umbrella general contract is paid once and drawn down by many orders; - // counting both double-counts its value. - .andWhere("(b.id IS NULL OR b.contract_kind IS NULL OR b.contract_kind <> 'GENERAL')") // Mixing ETB and USD into one SUM produces a meaningless number. .andWhere('il.currency = :currency', { currency: currencyOf(params) }); @@ -611,13 +622,13 @@ export function invoiceLedgerQb(ctx: ReportContext): SelectQueryBuilder 'eims_self_test'") - .andWhere("(b.id IS NULL OR b.contract_kind IS NULL OR b.contract_kind <> 'GENERAL')") .andWhere('i.currency = :currency', { currency: currencyOf(params) }); if (params.dateFrom) qb.andWhere(`${REVENUE_DATE} >= :dateFrom`, { dateFrom: params.dateFrom }); if (params.dateTo) qb.andWhere(`${REVENUE_DATE} < :dateTo`, { dateTo: params.dateTo }); if (params.origin) qb.andWhere('oy.code = :origin', { origin: params.origin }); - if (params.destination) qb.andWhere('dy.code = :destination', { destination: params.destination }); + if (params.destination) + qb.andWhere('dy.code = :destination', { destination: params.destination }); if (params.customer) { qb.andWhere( '(co.name ILIKE :customer OR slc.name ILIKE :customer OR b.reference ILIKE :customer)', @@ -630,14 +641,37 @@ export function invoiceLedgerQb(ctx: ReportContext): SelectQueryBuilder