diff --git a/apps/edr-freight-api/src/modules/reports/operations-classification.spec.ts b/apps/edr-freight-api/src/modules/reports/operations-classification.spec.ts index 63a7db20b..a37bc3f0c 100644 --- a/apps/edr-freight-api/src/modules/reports/operations-classification.spec.ts +++ b/apps/edr-freight-api/src/modules/reports/operations-classification.spec.ts @@ -7,6 +7,7 @@ import { TARGET_DIMENSION_KEYS, cycleRateExpr, implementRateExpr, + plannedRowsSql, } from './operations-classification'; import { TARGET_DIMENSIONS, TARGET_METRICS } from '../operations-reporting/entities/operations-target.entity'; @@ -91,4 +92,74 @@ describe('operations classification', () => { expect(rate(65, 78)).toBeLessThan(100); expect(cycleRateExpr('ad', 'sc')).toContain('NULLIF(sc, 0)'); }); + + /** + * The plan side is FULL OUTER JOINed to the operated side, so a plan row for + * a category the user filtered out comes back as a row of zeros — the bug + * where `?categories=FERTILIZER` still returned all ten planned categories. + */ + describe('plannedRowsSql cargo filter', () => { + const sqlFor = (dimension: string, params: Record): string => + plannedRowsSql('VOLUME_TONS', dimension, params, 'SELECT 1'); + + it('restricts targets to the selected categories', () => { + expect(sqlFor('cargo_category', { categories: ['FERTILIZER'] })).toContain( + "AND ot.dimension_key IN ('FERTILIZER')", + ); + }); + + it('restricts a station target on its cargo type, not its key', () => { + const sql = sqlFor('station', { categories: ['SAND'] }); + expect(sql).toContain("AND ot.cargo_category IN ('SAND')"); + expect(sql).not.toContain('ot.dimension_key IN'); + }); + + it('filters a container class report on its own vocabulary', () => { + expect(sqlFor('container_class', { classes: ['CONTAINER_EXPORT'] })).toContain( + "AND ot.dimension_key IN ('CONTAINER_EXPORT')", + ); + }); + + it('leaves every target when nothing is selected', () => { + expect(sqlFor('cargo_category', {})).not.toContain('ot.dimension_key IN'); + }); + + it('matches nothing on a value no category expression can emit', () => { + expect(sqlFor('cargo_category', { categories: ["x'; DROP TABLE"] })).toContain('AND FALSE'); + }); + + it('narrows a station plan to the chosen country rather than suppressing it', () => { + const sql = sqlFor('station', { country: 'Djibouti' }); + expect(sql).toContain("y.country = 'Djibouti'"); + expect(sql).not.toContain('AND FALSE'); + }); + + it('ignores a country that is not one of the two sides', () => { + expect(sqlFor('station', { country: "' OR true --" })).not.toContain('y.country'); + }); + + it('leaves the country alone on a plan not keyed by station', () => { + expect(sqlFor('cargo_category', { country: 'Djibouti' })).not.toContain('y.country'); + }); + + /** + * No target carries a route, a train or a direction, so beside a + * route-filtered actual the plan would be the whole corridor's target. + */ + it.each(['origin', 'destination', 'trainNumber', 'direction'])( + 'reports no plan at all when %s narrows below the target grain', + (key) => { + expect(sqlFor('cargo_category', { [key]: 'X' })).toContain('AND FALSE'); + }, + ); + + it('keeps the plan when only period, date and category are set', () => { + const sql = sqlFor('cargo_category', { + period: 'month', + dateFrom: '2026-01-01', + categories: ['SAND'], + }); + expect(sql).not.toContain('AND FALSE'); + }); + }); }); diff --git a/apps/edr-freight-api/src/modules/reports/operations-classification.ts b/apps/edr-freight-api/src/modules/reports/operations-classification.ts index 1c8f8504c..cffb787ee 100644 --- a/apps/edr-freight-api/src/modules/reports/operations-classification.ts +++ b/apps/edr-freight-api/src/modules/reports/operations-classification.ts @@ -513,7 +513,9 @@ export const PLAN_GRANULARITY_NOTE = 'Plan is the committed figure and never moves. Required is the same target treated as a ' + 'quota: whatever is still outstanding, spread across the time still left, so a period ' + 'that fell behind raises what the periods after it must carry. A target already met in ' + - 'full requires nothing further.'; + 'full requires nothing further. No target carries a route, a train or a direction, so ' + + 'filtering by one leaves the plan columns empty rather than comparing a corridor’s whole ' + + 'target against one slice of its work.'; /** * The user's date filter as open-ended bounds, so the clipping arithmetic below @@ -575,6 +577,60 @@ END`; * Period bounds ride on `:planFrom` / `:planTo`, which the caller must bind * with {@link plannedRowsParams} — they come from the user's date filter. */ +/** + * The plan side of a plan-versus-actual report has to obey the same cargo + * filter the operated side does. Without it the FULL OUTER JOIN re-introduces + * every planned key the user filtered out, as a row of zeros. + * + * Values are whitelisted against the vocabulary and inlined rather than bound: + * this fragment is assembled into raw CTE text, and the filter params are not + * validated upstream. An unknown value matches nothing — same as it does on the + * operated side, where the CASE can never emit it. + */ +const planKeyFilter = (dimension: string, params: Record): string => { + const isClass = dimension === 'container_class'; + const selected = (isClass ? params.classes : params.categories) as string[] | null; + if (!selected?.length) return ''; + const vocab = isClass ? CONTAINER_CLASSES : CARGO_CATEGORIES; + const valid = selected.filter((v) => vocab.some((o) => o.value === v)); + // A station target is keyed on the yard and carries its cargo type alongside. + const column = dimension === 'station' ? 'ot.cargo_category' : 'ot.dimension_key'; + return valid.length ? `AND ${column} IN (${quote(valid)})` : 'AND FALSE'; +}; + +/** + * Filters that narrow the operated population below the grain any target is + * kept at. No target carries a route, a train or a direction, so a plan read + * beside a route-filtered actual is the whole corridor's plan sitting next to + * one slice of its work — the implement rate then reads as a miss that never + * happened. + * + * There is no honest number to show, so the plan side reports nothing at all + * and Implement Rate goes NULL, the same way it does for a period with no + * target. `country` is absent on purpose: on a station plan it is a property of + * the planned key itself, and {@link planCountryFilter} narrows rather than + * suppresses. + */ +const PLAN_GRAIN_BREAKERS = ['origin', 'destination', 'trainNumber', 'direction']; + +const planGrainFilter = (params: Record): string => + PLAN_GRAIN_BREAKERS.some((key) => params[key]) ? 'AND FALSE' : ''; + +/** + * A station target is keyed on a yard code, so the country filter — which + * decides which end of the corridor the report calls "the station" — is a real + * predicate on the plan, not a grain break. Without it the Djibouti view lists + * every Ethiopian station's target as a row that moved nothing. + */ +const planCountryFilter = (dimension: string, params: Record): string => { + if (dimension !== 'station') return ''; + const country = COUNTRY_FILTER.options?.find((o) => o.value === params.country)?.value; + if (!country) return ''; + return `AND EXISTS (SELECT 1 FROM freight.yards y + WHERE y.code = ot.dimension_key AND y.deleted_at IS NULL + AND y.country = '${country}')`; +}; + export const plannedRowsSql = ( metric: string, dimension: string, @@ -597,6 +653,9 @@ export const plannedRowsSql = ( AND ot.metric = '${metric}' AND ot.dimension = '${dimension}' AND ot.planned_value > 0 + ${planKeyFilter(dimension, params)} + ${planCountryFilter(dimension, params)} + ${planGrainFilter(params)} ), -- One row per target per bucket. Generated a day at a time rather than a -- bucket at a time: the ragged units restart their blocks each January, so