fix issue

This commit is contained in:
Marshal
2026-07-27 05:59:49 +00:00
parent 773456c256
commit 15d78e1d67
10 changed files with 410 additions and 36 deletions

View File

@@ -14,7 +14,8 @@ export interface IRatesRepository {
findLiveRatesDetailed(): Promise<Rate[]>;
findByPattern(pattern: {
rateType: string;
rateUnit: string;
/** Omitted for singly-resolved rates — see the repository implementation. */
rateUnit?: string;
containerTypeId?: string | null;
cargoTypeId?: string | null;
tradeDirection?: string | null;

View File

@@ -18,10 +18,17 @@ export class RatesRepository implements IRatesRepository {
return this.repo.findOne({ where: { id } });
}
/**
* Every LIVE rate, newest first. The ordering is load-bearing: pricing picks
* the first match for a pattern, so without it Postgres heap order decided
* which of two overlapping rates a booking was billed at. Newest-first also
* means the most recent configuration wins where legacy overlaps still exist.
*/
findLiveRates(): Promise<Rate[]> {
return this.repo
.createQueryBuilder('rate')
.where('rate.status = :status', { status: 'LIVE' })
.orderBy('rate.created_at', 'DESC')
.getMany();
}
@@ -47,9 +54,21 @@ export class RatesRepository implements IRatesRepository {
* insert so the admin gets a friendly error instead of a raw constraint fault.
* NULL scope columns are matched with IS NULL, mirroring the COALESCE index.
*/
/**
* The live/draft rate already covering a pricing pattern, if any.
*
* `rateUnit` is optional on purpose. Where pricing resolves ONE rate for a
* lane (base freight, customs, lashing, empty return) the unit is not part of
* the identity — a per-container and a per-wagon row for the same lane are
* two answers to one question and the engine picks whichever came back first,
* so the caller omits it and the second row is rejected. Additive surcharges
* (hazard, reefer, demurrage…) are the opposite: the engine bills every
* matching rate by its own unit, so one per freight shape is the design and
* the caller passes the unit to keep them apart.
*/
findByPattern(pattern: {
rateType: string;
rateUnit: string;
rateUnit?: string;
containerTypeId?: string | null;
cargoTypeId?: string | null;
tradeDirection?: string | null;
@@ -59,9 +78,12 @@ export class RatesRepository implements IRatesRepository {
const qb = this.repo
.createQueryBuilder('rate')
.where('rate.rate_type = :rateType', { rateType: pattern.rateType })
.andWhere('rate.rate_unit = :rateUnit', { rateUnit: pattern.rateUnit })
.andWhere('rate.status <> :superseded', { superseded: 'SUPERSEDED' });
if (pattern.rateUnit) {
qb.andWhere('rate.rate_unit = :rateUnit', { rateUnit: pattern.rateUnit });
}
if (pattern.containerTypeId) {
qb.andWhere('rate.container_type_id = :containerTypeId', { containerTypeId: pattern.containerTypeId });
} else {

View File

@@ -0,0 +1,138 @@
import { ConflictException } from '@nestjs/common';
import { RatesService } from './rates.service';
import type { Rate } from '../entities/rate.entity';
/**
* One rate per lane + scope, whatever the unit.
*
* Pricing resolves a single rate for a (type, container type, leg) and then
* applies whatever unit it carries — it has no way to choose between a
* per-container and a per-wagon row for the same 20ft lane, and used to bill
* whichever the database happened to return first. So the unit is NOT part of a
* rate's identity: changing how a lane is billed means editing its rate.
*/
describe('RatesService — one rate per pattern', () => {
const DJ = '11111111-1111-4000-8000-000000000001';
const ET = '11111111-1111-4000-8000-000000000002';
const CT20 = '11111111-1111-4000-8000-000000000003';
const existing = (over: Partial<Rate> = {}): Rate =>
({
id: 'rate-existing',
rateType: 'CONTAINER_IMPORT',
rateUnit: 'PER_WAGON',
rateValue: 1690,
containerTypeId: CT20,
cargoTypeId: null,
tradeDirection: 'IMPORT',
originYardId: DJ,
destinationYardId: ET,
status: 'LIVE',
...over,
}) as Rate;
const dto = {
appliesTo: 'CONTAINER',
trigger: 'ALWAYS',
tradeDirection: 'IMPORT',
containerTypeId: CT20,
originYardId: DJ,
destinationYardId: ET,
rateValue: 845,
rateUnit: 'PER_CONTAINER',
};
let repository: { findByPattern: jest.Mock; create: jest.Mock };
let service: RatesService;
beforeEach(() => {
repository = {
findByPattern: jest.fn().mockResolvedValue(null),
create: jest.fn(async (r) => ({ id: 'rate-new', ...r })),
};
service = new RatesService(
repository as never,
{
findById: jest.fn(async (id: string) => ({
id,
country: id === DJ ? 'Djibouti' : 'Ethiopia',
label: id === DJ ? 'Doraleh' : 'Gelan',
})),
} as never,
{ findById: jest.fn().mockResolvedValue(null) } as never,
);
});
it('refuses a second rate on the same lane that only differs by unit', async () => {
repository.findByPattern.mockResolvedValue(existing());
await expect(service.create(dto as never, 'staff-1')).rejects.toBeInstanceOf(
ConflictException,
);
expect(repository.create).not.toHaveBeenCalled();
});
it('looks the pattern up without the unit, so either order collides', async () => {
await service.create(dto as never, 'staff-1');
const pattern = repository.findByPattern.mock.calls[0][0];
expect(pattern).not.toHaveProperty('rateUnit');
expect(pattern).toMatchObject({
rateType: 'CONTAINER_IMPORT',
containerTypeId: CT20,
originYardId: DJ,
destinationYardId: ET,
});
});
it('still allows the same unit on a different lane', async () => {
await service.create(dto as never, 'staff-1');
expect(repository.create).toHaveBeenCalledWith(
expect.objectContaining({
rateUnit: 'PER_CONTAINER',
rateValue: 845,
status: 'DRAFT',
}),
);
});
/**
* Additive surcharges are billed per matching rate, each by its own unit, so
* hazard is legitimately per-container for boxes AND per-ton for bulk. The
* unit stays part of their identity or the second one could never be created.
*/
it('keeps the unit in the key for an additive surcharge', async () => {
await service.create(
{
appliesTo: 'OTHER',
trigger: 'HAZARDOUS',
rateValue: 300,
rateUnit: 'PER_CONTAINER',
} as never,
'staff-1',
);
expect(repository.findByPattern.mock.calls[0][0]).toMatchObject({
rateType: 'HAZARD_SURCHARGE',
rateUnit: 'PER_CONTAINER',
});
});
it('treats lashing as singly resolved — one unit per direction', async () => {
await service.create(
{
appliesTo: 'OTHER',
trigger: 'LASHING',
tradeDirection: 'IMPORT',
rateValue: 40,
rateUnit: 'PER_TON',
} as never,
'staff-1',
);
expect(repository.findByPattern.mock.calls[0][0]).not.toHaveProperty(
'rateUnit',
);
});
});

View File

@@ -132,6 +132,24 @@ export class RatesService {
);
}
/**
* True when pricing resolves exactly ONE rate for this shape (base freight,
* customs clearance, lashing, empty-container return — all `find()`-based
* lookups). For those the unit is not part of the rate's identity: two rows
* for the same lane differing only by unit are a duplicate the engine cannot
* choose between.
*
* The additive surcharges are the opposite — the engine bills EVERY matching
* rate by its own unit, which is how hazard can be per-container for boxes
* and per-ton for bulk at the same time — so their unit stays part of the key.
*/
private resolvesSingleRate(
appliesTo: Rate['appliesTo'],
trigger: Rate['trigger'],
): boolean {
return this.isRouteScoped(appliesTo, trigger) || trigger === 'LASHING';
}
/**
* Which country each end of the leg must sit in, given what the rate is for.
* The railway only sells three shapes: import lands at the Djibouti ports and
@@ -326,10 +344,17 @@ export class RatesService {
* Reject a second rate with the same identity pattern (rateType + scope). With
* effective-date windows gone, two LIVE/DRAFT rates for the same pattern would
* make pricing ambiguous — so we allow exactly one per pattern.
*
* The UNIT is not part of that identity. Pricing resolves one rate per lane +
* scope and then applies whatever unit it carries; a per-container and a
* per-wagon row for the same 20ft lane are two answers to one question, and
* the engine silently picked one of them. Changing how a lane is billed means
* editing its rate, not adding a second.
*/
private async assertNoDuplicatePattern(pattern: {
rateType: string;
rateUnit: string;
/** Passed only for additive surcharges — see {@link resolvesSingleRate}. */
rateUnit?: string;
containerTypeId: string | null;
cargoTypeId: string | null;
tradeDirection: string | null;
@@ -415,7 +440,7 @@ export class RatesService {
await this.assertNoDuplicatePattern({
rateType,
rateUnit,
...(this.resolvesSingleRate(appliesTo, trigger) ? {} : { rateUnit }),
containerTypeId,
cargoTypeId,
tradeDirection,
@@ -600,7 +625,7 @@ export class RatesService {
// Guard the pattern uniqueness for the new identity, ignoring this row.
await this.assertNoDuplicatePattern({
rateType,
rateUnit,
...(this.resolvesSingleRate(appliesTo, trigger) ? {} : { rateUnit }),
containerTypeId: updates.containerTypeId,
cargoTypeId: updates.cargoTypeId,
tradeDirection: updates.tradeDirection,