feat(warehouses): keep failed cargo off the train until it is re-passed

An inspection report mirrored its outcome onto the inventory item but
never touched the item's status, and the loading paths gated on status
alone. Cargo that passed, reached READY_FOR_LOADING and was then
re-inspected as FAILED kept that status and loaded anyway.

Gate loading on the inspection outcome. load() is the single choke point
every loading path runs through, including loadItemsOntoTrain, so the
check sits there: nothing but PASSED travels, and the message names the
outcome so the operator knows what to fix.

A failed or under-review re-inspection also pulls the cargo back out of
the ready queue. load() refuses it either way, but leaving it READY_FOR_*
would keep it on the loading and pickup lists as though nothing had
happened.

Reversing a held inspection now needs a reason. Passing cargo whose
current inspection is FAILED or NEEDS_REVIEW is rejected without remarks,
so the record says why cargo that was deliberately held may now travel.
A first-time pass is unaffected.

Mark Selected as Inspected skips failed and under-review items instead of
clearing them. Overturning a failure is a deliberate, reasoned act, never
a side effect of ticking a row in a list; those items are reported back
with a reason telling the operator to re-inspect them individually.

Verified: freight-api type-check passes; 15 warehouse suites pass
(90 tests, 12 of them new).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
hager
2026-09-06 20:06:56 +00:00
parent cb150e46b7
commit 22250ebdb9
3 changed files with 189 additions and 1 deletions

View File

@@ -0,0 +1,146 @@
import { BadRequestException } from '@nestjs/common';
import { WarehouseInspectionService } from './warehouse-inspection.service';
import { WarehouseInventoryService } from './warehouse-inventory.service';
/**
* Cargo whose inspection failed or is under review must not travel. It becomes
* loadable only by being re-inspected and passed, and that reversal has to say
* why — the cargo was deliberately held, so its release is deliberate too.
*
* Only the collaborators each rule touches are stubbed; the instances are built
* off the prototype rather than wiring all 20-odd dependencies.
*/
describe('load() — inspection gate', () => {
const loadWithInspection = (inspectionStatus: string | null) => {
const service = Object.create(WarehouseInventoryService.prototype) as Record<string, unknown>;
service.findById = jest.fn().mockResolvedValue({
id: 'inv-1',
status: 'READY_FOR_LOADING',
inspectionStatus,
warehouseId: 'w-1',
yardId: 'y-1',
zoneId: 'z-1',
});
service.assertTransition = jest.fn();
// Reached only if the gate lets the item through — failing loudly here
// proves the gate did NOT stop it.
service.scheduling = {
findWagon: jest.fn().mockRejectedValue(new Error('gate did not block')),
};
return (
service as unknown as { load: (id: string, dto: unknown) => Promise<unknown> }
).load.bind(service);
};
it.each(['FAILED', 'NEEDS_REVIEW'])('refuses to load %s cargo', async (status) => {
await expect(loadWithInspection(status)('inv-1', { wagonId: 'w' })).rejects.toBeInstanceOf(
BadRequestException,
);
});
it('refuses to load cargo that was never inspected', async () => {
await expect(loadWithInspection(null)('inv-1', { wagonId: 'w' })).rejects.toBeInstanceOf(
BadRequestException,
);
});
it('names the outcome so the operator knows what to fix', async () => {
await expect(loadWithInspection('FAILED')('inv-1', { wagonId: 'w' })).rejects.toThrow(
/FAILED/,
);
});
it('lets passed cargo through the gate', async () => {
// It fails later, at the wagon lookup — which is the proof it got past the
// inspection gate rather than being stopped by it.
await expect(loadWithInspection('PASSED')('inv-1', { wagonId: 'w' })).rejects.toThrow(
'gate did not block',
);
});
});
describe('inspection report — reversing a held inspection', () => {
const createReport = (previousStatus: string, itemStatus = 'RECEIVED') => {
const update = jest.fn().mockResolvedValue(undefined);
const service = Object.create(WarehouseInspectionService.prototype) as Record<string, unknown>;
service.dataSource = {
getRepository: () => ({
findOne: jest.fn().mockResolvedValue({
id: 'inv-1',
bookingId: 'b-1',
inspectionStatus: previousStatus,
status: itemStatus,
}),
update,
}),
};
service.inspectionRepository = {
findAll: jest.fn().mockResolvedValue([]),
create: jest.fn().mockResolvedValue({ id: 'rep-1' }),
};
service.markImportPickupReadyAndAcceptLastMile = jest.fn().mockResolvedValue(undefined);
const create = (
service as unknown as {
create: (id: string, dto: unknown) => Promise<unknown>;
}
).create.bind(service);
return { create, update };
};
it.each(['FAILED', 'NEEDS_REVIEW'])(
'rejects passing %s cargo with no reason given',
async (previous) => {
const { create } = createReport(previous);
await expect(
create('inv-1', { reportType: 'INSPECTION', inspectionStatus: 'PASSED' }),
).rejects.toBeInstanceOf(BadRequestException);
},
);
it('rejects whitespace as a reason', async () => {
const { create } = createReport('FAILED');
await expect(
create('inv-1', { reportType: 'INSPECTION', inspectionStatus: 'PASSED', remarks: ' ' }),
).rejects.toBeInstanceOf(BadRequestException);
});
it('accepts the reversal once a reason is recorded', async () => {
const { create } = createReport('FAILED');
await expect(
create('inv-1', {
reportType: 'INSPECTION',
inspectionStatus: 'PASSED',
remarks: 'Reworked packaging, re-weighed and verified.',
}),
).resolves.toBeDefined();
});
it('needs no reason for a first-time pass', async () => {
const { create } = createReport(null as unknown as string);
await expect(
create('inv-1', { reportType: 'INSPECTION', inspectionStatus: 'PASSED' }),
).resolves.toBeDefined();
});
it('pulls failed cargo back out of the ready-to-load queue', async () => {
const { create, update } = createReport('PASSED', 'READY_FOR_LOADING');
await create('inv-1', { reportType: 'INSPECTION', inspectionStatus: 'FAILED' });
expect(update).toHaveBeenCalledWith('inv-1', { status: 'RECEIVED' });
});
it('leaves cargo that never reached the ready queue where it is', async () => {
const { create, update } = createReport('PASSED', 'STORED');
await create('inv-1', { reportType: 'INSPECTION', inspectionStatus: 'FAILED' });
expect(update).not.toHaveBeenCalledWith('inv-1', { status: 'RECEIVED' });
});
});

View File

@@ -1,4 +1,4 @@
import { Injectable, Logger, NotFoundException } from '@nestjs/common';
import { BadRequestException, Injectable, Logger, NotFoundException } from '@nestjs/common';
import { DataSource } from 'typeorm';
import { NotificationAudience, NotificationType } from '@edr/types';
@@ -37,6 +37,20 @@ export class WarehouseInspectionService {
throw new NotFoundException(`Inventory item ${inventoryId} not found`);
}
// Overturning a held inspection is a deliberate act: the cargo was kept off
// the train, and the record has to say why it may now travel. A bare PASS
// with no remarks leaves the release unexplained.
const previousStatus = inventory.inspectionStatus;
if (
dto.inspectionStatus === 'PASSED' &&
(previousStatus === 'FAILED' || previousStatus === 'NEEDS_REVIEW') &&
!dto.remarks?.trim()
) {
throw new BadRequestException(
`Give a reason in Remarks for passing cargo whose inspection is ${previousStatus}`,
);
}
const expected = dto.expectedWeight ?? null;
const actual = dto.actualWeight ?? null;
const weightLoss = expected !== null && actual !== null ? Math.max(0, expected - actual) : null;
@@ -83,6 +97,15 @@ export class WarehouseInspectionService {
if (dto.inspectionStatus === 'PASSED') {
await this.markImportPickupReadyAndAcceptLastMile(inventoryId);
} else if (
inventory.status === 'READY_FOR_LOADING' ||
inventory.status === 'READY_FOR_PICKUP'
) {
// A failed or under-review re-inspection pulls the cargo back out of the
// ready queue. load() refuses it either way, but leaving it READY_FOR_*
// would keep it sitting on the loading and pickup lists as if nothing
// had happened.
await inventoryRepo.update(inventoryId, { status: 'RECEIVED' });
}
return report;

View File

@@ -3096,6 +3096,13 @@ export class WarehouseInventoryService {
const item = await this.inventoryRepository.findById(inventoryId);
if (!item) { skip('Inventory not found'); continue; }
if (item.inspectionStatus === 'PASSED') { skip('Already inspected'); continue; }
// Overturning a failure is a deliberate, reasoned act — never a side
// effect of ticking a row in a list. Those items are held back for an
// individual re-inspection that records why the cargo may now travel.
if (item.inspectionStatus === 'FAILED' || item.inspectionStatus === 'NEEDS_REVIEW') {
skip(`Inspection ${item.inspectionStatus} — re-inspect this item individually and give a reason`);
continue;
}
if (!eligible.includes(item.status)) { skip(`Status ${item.status} not eligible for inspection`); continue; }
// Reuse the existing inspection service: creates a minimal PASSED report + sets inspectionStatus/inspectedAt.
@@ -5816,6 +5823,18 @@ export class WarehouseInventoryService {
// 1. inventory status must be READY_FOR_LOADING (and not already LOADED).
this.assertTransition(item.status, 'LOADED');
// 1b. Failed or under-review cargo does not travel. Status alone is not
// enough: an item that passed, reached READY_FOR_LOADING and was then
// re-inspected as FAILED keeps that status, so the inspection outcome is
// checked here — the one choke point every loading path runs through.
if (item.inspectionStatus !== 'PASSED') {
throw new BadRequestException(
item.inspectionStatus
? `Inspection is ${item.inspectionStatus} — the cargo must be re-inspected and passed, with a reason, before it can be loaded`
: 'Inventory must pass inspection before it can be loaded',
);
}
// 2. inventory is at a valid warehouse/yard/zone location.
if (!item.warehouseId || !item.yardId || !item.zoneId) {
throw new BadRequestException('Inventory must be at a warehouse/yard/zone before loading');