fix(operations-targets): derive cargo category from its dimension

A station target's plan is per station AND per cargo type; the other two
dimensions already carry the category inside dimension_key. update() kept
whatever was stored whenever the payload omitted the key, and the admin
form builds its payload from visible fields only — so switching "Plan by"
away from Station left the old category behind on a row that no longer has
any use for one.

That row is not merely untidy. It survives the COALESCE(cargo_category,'')
unique index alongside the legitimate null-category row for the same key,
plannedRowsSql groups by the column, and the two plan rows then join the
same operated row through the FULL OUTER JOIN: the category lists twice,
each line carrying the full operated tonnage, while the summary tiles are
computed separately and stay correct — so the table disagrees with its own
totals and nothing says why.

create() and update() now resolve the slot through one place, which is the
point: the two paths cannot drift again. cargoCategory is derived from the
effective dimension rather than carried over, and a station target without
one is rejected instead of stored as a plan the report can never match.

dimensionKey is checked against the vocabulary the reports actually emit —
CARGO_CATEGORIES / CONTAINER_CLASSES, or live yards.code for stations. An
unknown key used to store fine, list fine and fall back to showing the raw
key, while being a plan no report would ever find.

Also drops @Global from the module. Nothing outside it injects either
service; the reports read both tables in raw SQL, so the docstring's stated
reason for being global was not true.

apps/edr-freight-api/src/modules/operations-reporting/dto/create-operations-target.dto.ts

@@ -58,13 +58,18 @@ export class CreateOperationsTargetDto {
 
   @ApiPropertyOptional({
     description:
-      'Station targets only: which cargo category this station plan covers. Leave blank for the other dimensions.',
+      'Station targets only: which cargo category this station plan covers. Ignored for the ' +
+      'other dimensions, whose key already carries the category.',
     example: 'CONTAINER_IMPORT_MULTIMODAL',
   })
   @IsOptional()
+  // `'' ?? null` is `''`, and an empty string matches neither the unique
+  // index's `COALESCE(cargo_category, '')` nor the report's join — it reads as
+  // a category that does not exist. Blank means absent.
+  @Transform(({ value }) => (value === '' ? null : value))
   @IsString()
   @MaxLength(60)
-  cargoCategory?: string;
+  cargoCategory?: string | null;
 
   @ApiPropertyOptional()
   @IsOptional()

apps/edr-freight-api/src/modules/operations-reporting/operations-reporting.module.ts

@@ -1,4 +1,4 @@
-import { Global, Module } from '@nestjs/common';
+import { Module } from '@nestjs/common';
 import { TypeOrmModule } from '@nestjs/typeorm';
 
 import { OperationsStandard } from './entities/operations-standard.entity';
@@ -13,10 +13,11 @@ import { OperationsTargetsService } from './operations-targets.service';
  * standards (one settings row) and the planned targets the reports compare
  * actuals against.
  *
- * Global because the reports module reads the standards row on every run and
+ * Not global, and deliberately so: nothing outside this module injects either
- * has no other reason to import this.
+ * service. The reports read both tables in raw SQL — `STANDARDS_JOIN` and
+ * `plannedRowsSql` in `reports/operations-classification.ts` — so the exports
+ * below are for future callers, not current ones.
  */
-@Global()
 @Module({
   imports: [TypeOrmModule.forFeature([OperationsStandard, OperationsTarget])],
   controllers: [OperationsStandardsController, OperationsTargetsController],

apps/edr-freight-api/src/modules/operations-reporting/operations-targets.service.ts

@@ -1,5 +1,10 @@
 import { PaginatedResponse } from '@edr/types';
-import { ConflictException, Injectable, NotFoundException } from '@nestjs/common';
+import {
+  BadRequestException,
+  ConflictException,
+  Injectable,
+  NotFoundException,
+} from '@nestjs/common';
 import { InjectRepository } from '@nestjs/typeorm';
 import { Brackets, IsNull, Repository } from 'typeorm';
 
@@ -12,6 +17,8 @@ import {
   TARGET_DIMENSION_LABELS,
   TARGET_METRIC_LABELS,
   TARGET_PERIOD_LABELS,
+  TargetDimension,
+  TargetMetric,
   TargetPeriodType,
 } from './entities/operations-target.entity';
 import {
@@ -76,6 +83,27 @@ const LABELS_BY_DIMENSION: Record<string, Map<string, string>> = {
 
 const CARGO_CATEGORY_LABELS = LABELS_BY_DIMENSION.cargo_category;
 
+/**
+ * The keys a target may be stored against, per dimension. A report matches a
+ * target by this exact string, so a key outside the set here is a plan no
+ * report can ever find — and nothing downstream would ever say so. `station` is
+ * absent on purpose: yard codes are admin-managed rows, resolved live.
+ *
+ * `UNCLASSIFIED` is accepted for `cargo_category` even though the admin form
+ * does not offer it, because `CARGO_CATEGORY_EXPR` does emit it — rejecting a
+ * key the reports can match would be stricter than the reports themselves.
+ */
+const KEYS_BY_DIMENSION: Record<Exclude<TargetDimension, 'station'>, Set<string>> = {
+  cargo_category: new Set(CARGO_CATEGORIES.map((o) => o.value)),
+  container_class: new Set(CONTAINER_CLASSES.map((o) => o.value)),
+};
+
+/** The columns that decide which report row a target lines up with. */
+type TargetSlot = Pick<
+  OperationsTarget,
+  'periodType' | 'periodStart' | 'metric' | 'dimension' | 'dimensionKey' | 'cargoCategory'
+>;
+
 @Injectable()
 export class OperationsTargetsService {
   constructor(
@@ -152,35 +180,113 @@ export class OperationsTargetsService {
   }
 
   async create(dto: CreateOperationsTargetDto): Promise<OperationsTarget> {
-    const periodStart = normalisePeriodStart(dto.periodType, dto.periodStart);
+    const slot = await this.resolveSlot(dto);
-    const cargoCategory = dto.cargoCategory ?? null;
+    await this.assertSlotFree(slot);
-    await this.assertSlotFree({ ...dto, periodStart, cargoCategory });
+    return this.repository.save(this.repository.create({ ...dto, ...slot }));
-    return this.repository.save(this.repository.create({ ...dto, periodStart, cargoCategory }));
   }
 
   async update(id: string, dto: UpdateOperationsTargetDto): Promise<OperationsTarget> {
     const current = await this.findById(id);
-    const periodType = dto.periodType ?? current.periodType;
+    const slot = await this.resolveSlot({
-    const periodStart = normalisePeriodStart(periodType, dto.periodStart ?? current.periodStart);
+      periodType: dto.periodType ?? current.periodType,
-    const next = {
+      periodStart: dto.periodStart ?? current.periodStart,
-      periodType,
-      periodStart,
       metric: dto.metric ?? current.metric,
       dimension: dto.dimension ?? current.dimension,
       dimensionKey: dto.dimensionKey ?? current.dimensionKey,
+      // An absent key means "unchanged" only while the dimension still wants a
+      // category at all — `resolveSlot` drops it when the dimension no longer
+      // does, which is the whole point of routing both paths through it.
       cargoCategory:
-        dto.cargoCategory !== undefined ? (dto.cargoCategory ?? null) : current.cargoCategory ?? null,
+        dto.cargoCategory !== undefined ? dto.cargoCategory : current.cargoCategory,
-    };
+    });
-    await this.assertSlotFree(next, id);
+    await this.assertSlotFree(slot, id);
 
     await this.repository.update(id, {
-      ...next,
+      ...slot,
       ...(dto.plannedValue != null ? { plannedValue: dto.plannedValue } : {}),
       ...(dto.note !== undefined ? { note: dto.note } : {}),
     });
     return this.findById(id);
   }
 
+  /**
+   * Everything that decides which report row a target lines up with, resolved
+   * in one place so `create` and `update` cannot drift apart.
+   *
+   * `cargoCategory` is **derived from the dimension, never carried over**. A
+   * station's plan is per station AND per cargo type; the other two dimensions
+   * already carry the category in `dimensionKey`. A stale category left on a
+   * row whose dimension has moved on is not cosmetic — it survives the
+   * `COALESCE(cargo_category, '')` unique index alongside the legitimate
+   * null-category row, `plannedRowsSql` groups by it, and the two plan rows
+   * then both join the same operated row: the category lists twice, each time
+   * carrying the full operated tonnage, while the summary tiles stay correct.
+   */
+  private async resolveSlot(input: {
+    periodType: TargetPeriodType;
+    periodStart: string;
+    metric: TargetMetric;
+    dimension: TargetDimension;
+    dimensionKey: string;
+    cargoCategory?: string | null;
+  }): Promise<TargetSlot> {
+    const periodStart = normalisePeriodStart(input.periodType, input.periodStart);
+    await this.assertDimensionKey(input.dimension, input.dimensionKey);
+
+    const base = {
+      periodType: input.periodType,
+      periodStart,
+      metric: input.metric,
+      dimension: input.dimension,
+      dimensionKey: input.dimensionKey,
+    };
+
+    if (input.dimension !== 'station') {
+      return { ...base, cargoCategory: null };
+    }
+
+    const cargoCategory = input.cargoCategory || null;
+    if (!cargoCategory) {
+      throw new BadRequestException(
+        'A station target needs a cargo category — the plan is per station and per cargo type. ' +
+          'Without one the report has nothing to match it against.',
+      );
+    }
+    if (!KEYS_BY_DIMENSION.cargo_category.has(cargoCategory)) {
+      throw new BadRequestException(
+        `"${cargoCategory}" is not a cargo category the reports produce. ` +
+          `Expected one of: ${[...KEYS_BY_DIMENSION.cargo_category].join(', ')}`,
+      );
+    }
+    return { ...base, cargoCategory };
+  }
+
+  /**
+   * A `dimensionKey` the reports never emit is a plan that silently never
+   * joins — the row lists fine and its label falls back to the raw key, so
+   * nothing downstream ever reports the mistake. Cheaper to reject on write.
+   */
+  private async assertDimensionKey(dimension: TargetDimension, key: string): Promise<void> {
+    if (dimension === 'station') {
+      const yards = await this.yardLabels();
+      if (!yards.has(key)) {
+        throw new BadRequestException(
+          `"${key}" is not a known station code. A station target is keyed on ` +
+            '`yards.code`, which is what the reports match against.',
+        );
+      }
+      return;
+    }
+
+    const allowed = KEYS_BY_DIMENSION[dimension];
+    if (!allowed.has(key)) {
+      throw new BadRequestException(
+        `"${key}" is not a ${TARGET_DIMENSION_LABELS[dimension].toLowerCase()} the reports ` +
+          `produce. Expected one of: ${[...allowed].join(', ')}`,
+      );
+    }
+  }
+
   async remove(id: string): Promise<void> {
     await this.findById(id);
     await this.repository.softDelete(id);

apps/edr-freight-web/backoffice/src/pages/ruleEngine/config/resources.ts

@@ -149,6 +149,12 @@ const TRADE_DIRECTIONS = [
  * a report matches a target by this exact key, so a value here that the API
  * does not emit is a plan the report will never find. The API spec
  * `operations-classification.spec.ts` guards the API side of the pair.
+ *
+ * Drift is no longer silent: `OperationsTargetsService.assertDimensionKey`
+ * rejects any key outside the API's own vocabulary, so a stale entry here
+ * surfaces as a 400 on save rather than a plan that quietly never joins.
+ * `UNCLASSIFIED` is left out deliberately — the API accepts it, but there is no
+ * sense in planning against cargo nobody has classified.
  */
 export const OPERATIONS_CARGO_CATEGORIES = [
   { label: "Multimodal container import", value: "CONTAINER_IMPORT_MULTIMODAL" },