mirror of
https://github.com/Tria-plc/edr-platform.git
synced 2026-08-27 22:30:55 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
Nathnael
@@ -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
|
||||
|
||||
@@ -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<ObjectLiteral> {
|
||||
// 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) },
|
||||
];
|
||||
},
|
||||
|
||||
@@ -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<ObjectLiteral> {
|
||||
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),
|
||||
},
|
||||
];
|
||||
},
|
||||
};
|
||||
|
||||
@@ -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<ObjectLi
|
||||
deadInvoiceStatuses: DEAD_INVOICE_STATUSES,
|
||||
})
|
||||
.andWhere("i.source <> '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<ObjectLi
|
||||
deadInvoiceStatuses: DEAD_INVOICE_STATUSES,
|
||||
})
|
||||
.andWhere("i.source <> '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<ObjectLi
|
||||
}
|
||||
|
||||
/**
|
||||
* What the payment gateway actually recorded against this invoice, summed.
|
||||
* `invoices.payment_id` points at a payment-api intent id rather than a
|
||||
* `freight.payments` row, so the reliable link is the booking id both sides
|
||||
* carry.
|
||||
* What the payment gateway actually recorded against this invoice.
|
||||
*
|
||||
* `freight.payments` is keyed by the booking, not the invoice — `ref_id` holds
|
||||
* the booking id and there is no invoice column — while one booking routinely
|
||||
* carries several invoices (25 booking ids here back 58 of them). Reading the
|
||||
* booking's gateway total straight off each invoice therefore hands the same
|
||||
* money to every sibling: 113M of gateway receipts claimed against 44M of
|
||||
* recorded settlement, which surfaced as ~89M of variance that does not exist.
|
||||
*
|
||||
* So the booking's receipts are apportioned across its invoices by their share
|
||||
* of what was recorded as settled — the same device as {@link PAID_SHARE}, and
|
||||
* the only split that makes the report's gateway column sum to the payments
|
||||
* table. A booking whose invoices record no settlement at all cannot be split
|
||||
* that way; it falls back to the billed share, so gateway money nobody booked
|
||||
* still shows up as variance instead of vanishing.
|
||||
*
|
||||
* `invoices.payment_id` does resolve to a `freight.payments` row, but only 72
|
||||
* of 85 successful payments are pointed at by one, so keying on it drops real
|
||||
* receipts.
|
||||
*/
|
||||
export const GATEWAY_PAID = `(
|
||||
SELECT COALESCE(SUM(p.amount), 0) FROM freight.payments p
|
||||
WHERE p.ref_id = i.source_id AND p.status = 'success'
|
||||
(SELECT COALESCE(SUM(p.amount), 0) FROM freight.payments p
|
||||
WHERE p.ref_id = i.source_id AND p.status = 'success')
|
||||
* COALESCE(
|
||||
i.paid_amount / NULLIF((SELECT SUM(i2.paid_amount) FROM freight.invoices i2
|
||||
WHERE i2.source_id = i.source_id AND i2.deleted_at IS NULL
|
||||
AND i2.status NOT IN ('DRAFT', 'CANCELLED')), 0),
|
||||
i.total_amount / NULLIF((SELECT SUM(i2.total_amount) FROM freight.invoices i2
|
||||
WHERE i2.source_id = i.source_id AND i2.deleted_at IS NULL
|
||||
AND i2.status NOT IN ('DRAFT', 'CANCELLED')), 0),
|
||||
0)
|
||||
)`;
|
||||
|
||||
/** The payer, whichever of the two mutually exclusive payer columns is set. */
|
||||
|
||||
Reference in New Issue
Block a user