From c1c362ab9d93398bd49d279a7f96c6a4482eb226 Mon Sep 17 00:00:00 2001 From: Hagernesh Date: Mon, 20 Jul 2026 08:03:16 +0000 Subject: [PATCH] fix(mile): only send bookings that bought EDR haulage to the mile queues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rule that a customer's own truck and an EDR road leg are alternatives existed on the truck side only (CustomerTruckService.assertSelfHaulPaid). LastMileService had no counterpart: create checked payment and nothing else, so any paid booking could be accepted into the queue. A booking took a customer truck at 06:42 and an EDR last-mile leg with a real EDR truck at 06:47, neither side aware of the other, on a contract that had chosen no road legs at all. The road legs are chosen on the contract and copied onto the booking, and the pickup/delivery address is the only per-booking record of that choice. service_types cannot serve: every type ships with includes_first_mile and includes_last_mile set to true, so reading them would mean no booking could ever self-haul. That same always-true flag had already killed the first-mile guard, whose `address || serviceType.includesFirstMile` admitted every paid export booking. One shared rule now answers it for both sides, so the two halves cannot drift apart again: last-mile create rejects a booking that chose no road legs and one already carrying a customer truck; first-mile no longer honours the service-type flag; the customer-truck guard reads the same helper. Existing legs are untouched — the guards are on creation, so the one booking already carrying both needs a human to reconcile it. Co-Authored-By: Claude Opus 4.8 --- .../src/common/mile-haulage.util.spec.ts | 52 ++++++++++ .../src/common/mile-haulage.util.ts | 49 +++++++++ .../bookings/customer-truck.service.ts | 21 ++-- .../modules/first-mile/first-mile.service.ts | 8 +- .../modules/last-mile/last-mile.guard.spec.ts | 99 +++++++++++++++++++ .../modules/last-mile/last-mile.service.ts | 48 +++++++++ 6 files changed, 262 insertions(+), 15 deletions(-) create mode 100644 apps/edr-freight-api/src/common/mile-haulage.util.spec.ts create mode 100644 apps/edr-freight-api/src/common/mile-haulage.util.ts create mode 100644 apps/edr-freight-api/src/modules/last-mile/last-mile.guard.spec.ts diff --git a/apps/edr-freight-api/src/common/mile-haulage.util.spec.ts b/apps/edr-freight-api/src/common/mile-haulage.util.spec.ts new file mode 100644 index 000000000..8c3310321 --- /dev/null +++ b/apps/edr-freight-api/src/common/mile-haulage.util.spec.ts @@ -0,0 +1,52 @@ +import { usesEdrMileService } from './mile-haulage.util'; + +/** + * The road legs are chosen on the contract and copied onto the booking. EDR + * haulage and a customer's own truck are alternatives, so this one answer gates + * both sides — the customer-truck guard and the mile-queue guard. + */ +describe('usesEdrMileService', () => { + const booking = (over: Partial[0]> = {}) => ({ + tradeDirection: 'IMPORT', + firstMile: null, + lastMile: null, + ...over, + }); + + it('an import that chose delivery uses EDR haulage', () => { + expect(usesEdrMileService(booking({ lastMile: 'Bole, Addis Ababa' }))).toBe(true); + }); + + it('an import that chose nothing does not', () => { + expect(usesEdrMileService(booking())).toBe(false); + }); + + it('ignores the pickup address on an import — collection is the export leg', () => { + expect(usesEdrMileService(booking({ firstMile: 'Modjo' }))).toBe(false); + }); + + it('an export that chose collection uses EDR haulage', () => { + expect( + usesEdrMileService(booking({ tradeDirection: 'EXPORT', firstMile: 'Modjo' })), + ).toBe(true); + }); + + it('ignores the delivery address on an export — delivery is the import leg', () => { + expect( + usesEdrMileService(booking({ tradeDirection: 'EXPORT', lastMile: 'Djibouti' })), + ).toBe(false); + }); + + it('a domestic booking counts either leg', () => { + expect( + usesEdrMileService(booking({ tradeDirection: 'DOMESTIC', firstMile: 'Adama' })), + ).toBe(true); + expect( + usesEdrMileService(booking({ tradeDirection: 'DOMESTIC', lastMile: 'Dire Dawa' })), + ).toBe(true); + }); + + it('treats a whitespace-only address as no choice', () => { + expect(usesEdrMileService(booking({ lastMile: ' ' }))).toBe(false); + }); +}); diff --git a/apps/edr-freight-api/src/common/mile-haulage.util.ts b/apps/edr-freight-api/src/common/mile-haulage.util.ts new file mode 100644 index 000000000..1ca83d06e --- /dev/null +++ b/apps/edr-freight-api/src/common/mile-haulage.util.ts @@ -0,0 +1,49 @@ +/** The booking fields that decide who hauls the road legs. */ +export interface MileHaulageRow { + tradeDirection: string | null; + /** `first_mile_pickup_address` — set when the customer asked EDR to collect. */ + firstMile: string | null; + /** `last_mile_delivery_address` — set when the customer asked EDR to deliver. */ + lastMile: string | null; +} + +/** + * Whether the customer bought the EDR road leg that matters for their direction: + * delivery at the end of an import, collection at the start of an export. A + * DOMESTIC booking can use either, so either one counts. + * + * The address is the signal because it is the only per-booking record of the + * choice. `service_types.includes_first_mile` / `includes_last_mile` cannot be + * used — every service type ships with both set to true, so reading them would + * mean every booking uses EDR haulage and none could ever self-haul. + */ +export function usesEdrMileService(booking: MileHaulageRow): boolean { + const hasFirstMile = Boolean(booking.firstMile?.trim()); + const hasLastMile = Boolean(booking.lastMile?.trim()); + switch (booking.tradeDirection) { + case 'IMPORT': + return hasLastMile; + case 'EXPORT': + return hasFirstMile; + default: + return hasFirstMile || hasLastMile; + } +} + +/** + * EDR haulage and a customer's own truck are alternatives, never both. Whichever + * side is being set up, it has to reject the other — a guard on only one side + * lets the two paths open on the same booking, each unaware of the other. + */ +export const SELF_HAUL_CONFLICT_MESSAGE = + 'This booking is delivered by the customer’s own truck — an EDR mile leg cannot also be assigned.'; + +export const EDR_HAULAGE_CONFLICT_MESSAGE = + 'Customer truck assignment is only allowed when first/last mile delivery is not selected'; + +/** + * The road legs are chosen on the contract. A booking whose contract bought + * neither has no business in the first/last-mile queues at all. + */ +export const NO_MILE_SERVICE_MESSAGE = + 'This booking did not select first/last mile delivery on its contract, so it cannot be assigned an EDR mile leg.'; diff --git a/apps/edr-freight-api/src/modules/bookings/customer-truck.service.ts b/apps/edr-freight-api/src/modules/bookings/customer-truck.service.ts index f366faa96..7b0145db2 100644 --- a/apps/edr-freight-api/src/modules/bookings/customer-truck.service.ts +++ b/apps/edr-freight-api/src/modules/bookings/customer-truck.service.ts @@ -12,6 +12,10 @@ import { AddCustomerTruckDto } from './dto/add-customer-truck.dto'; import { DepartCustomerTruckDto } from './dto/depart-customer-truck.dto'; import { CustomerTruckAssignment } from './entities/customer-truck-assignment.entity'; import { CustomerTruckContainer } from './entities/customer-truck-container.entity'; +import { + EDR_HAULAGE_CONFLICT_MESSAGE, + usesEdrMileService, +} from '../../common/mile-haulage.util'; import { CustomerTruckAssignmentsRepository } from './customer-truck-assignments.repository'; import { NotificationInboxService } from '../notification-inbox/notification-inbox.service'; import { NotificationsService } from '../notifications/notifications.service'; @@ -534,18 +538,11 @@ export class CustomerTruckService { } private assertSelfHaulPaid(booking: BookingGuardRow): void { - const hasFirstMile = Boolean(booking.firstMile?.trim()); - const hasLastMile = Boolean(booking.lastMile?.trim()); - const usesMileService = - booking.tradeDirection === 'IMPORT' - ? hasLastMile - : booking.tradeDirection === 'EXPORT' - ? hasFirstMile - : hasFirstMile || hasLastMile; - if (usesMileService) { - throw new BadRequestException( - 'Customer truck assignment is only allowed when first/last mile delivery is not selected', - ); + // Shared with the EDR side (LastMileService.assertNoCustomerTruck) so the two + // halves of this rule cannot drift apart — they did, and a booking ended up + // with a customer truck and an EDR leg at once. + if (usesEdrMileService(booking)) { + throw new BadRequestException(EDR_HAULAGE_CONFLICT_MESSAGE); } if (booking.paymentStatus !== 'PAID') { throw new BadRequestException( diff --git a/apps/edr-freight-api/src/modules/first-mile/first-mile.service.ts b/apps/edr-freight-api/src/modules/first-mile/first-mile.service.ts index 00dbb80cc..0143f0796 100644 --- a/apps/edr-freight-api/src/modules/first-mile/first-mile.service.ts +++ b/apps/edr-freight-api/src/modules/first-mile/first-mile.service.ts @@ -333,10 +333,12 @@ export class FirstMileService { firstMilePickupAddress?: string | null; serviceType?: { includesFirstMile?: boolean | null } | null; }): boolean { + // The pickup address is the only record of what the contract chose. + // `serviceType.includesFirstMile` used to satisfy this too, but every + // service type ships with it set to true, so the OR made the address check + // dead and admitted every paid export booking into the queue. return Boolean( - booking.tradeDirection === 'EXPORT' && - (booking.firstMilePickupAddress?.trim() || - booking.serviceType?.includesFirstMile), + booking.tradeDirection === 'EXPORT' && booking.firstMilePickupAddress?.trim(), ); } diff --git a/apps/edr-freight-api/src/modules/last-mile/last-mile.guard.spec.ts b/apps/edr-freight-api/src/modules/last-mile/last-mile.guard.spec.ts new file mode 100644 index 000000000..232f59d50 --- /dev/null +++ b/apps/edr-freight-api/src/modules/last-mile/last-mile.guard.spec.ts @@ -0,0 +1,99 @@ +import { BadRequestException } from '@nestjs/common'; +import type { DataSource } from 'typeorm'; + +import { LastMileService } from './last-mile.service'; + +/** + * A booking reaches the last-mile queue only if its contract bought EDR + * delivery, and never if the customer is hauling it themselves. Creation used + * to check payment alone, so any paid booking could be accepted — which put a + * self-haul booking and an EDR leg on the same shipment at once. + */ +type BookingRow = { tradeDirection: string; firstMile: string | null; lastMile: string | null }; + +function makeService(opts: { booking?: BookingRow; hasCustomerTruck?: boolean }) { + const booking = opts.booking ?? { + tradeDirection: 'IMPORT', + firstMile: null, + lastMile: 'Bole, Addis Ababa', + }; + + const query = jest.fn((sql: string) => { + if (sql.includes('customer_truck_assignments')) { + return Promise.resolve(opts.hasCustomerTruck ? [{ '?column?': 1 }] : []); + } + if (sql.includes('FROM freight.bookings')) { + return Promise.resolve([booking]); + } + return Promise.resolve([]); + }); + + const lastMileRepository = { + findAll: jest.fn().mockResolvedValue([]), + create: jest.fn((row: unknown) => Promise.resolve({ id: 'lm-1', ...(row as object) })), + }; + + const service = new LastMileService( + lastMileRepository as never, + {} as never, // bookingsRepository + { setAvailability: jest.fn() } as never, // vehiclesService + {} as never, // driversService + {} as never, // smsClient + { query } as unknown as DataSource, + { record: jest.fn() } as never, // history + {} as never, // billing + {} as never, // filesService + ); + + return { service, lastMileRepository, query }; +} + +describe('LastMileService.create — haulage guard', () => { + it('accepts a booking whose contract chose EDR delivery', async () => { + const { service, lastMileRepository } = makeService({}); + + await service.create({ bookingId: 'b-1', advancedPayment: 0 } as never); + + expect(lastMileRepository.create).toHaveBeenCalled(); + }); + + it('rejects a booking that chose no road legs on its contract', async () => { + const { service, lastMileRepository } = makeService({ + booking: { tradeDirection: 'IMPORT', firstMile: null, lastMile: null }, + }); + + await expect( + service.create({ bookingId: 'b-1', advancedPayment: 0 } as never), + ).rejects.toBeInstanceOf(BadRequestException); + expect(lastMileRepository.create).not.toHaveBeenCalled(); + }); + + it('rejects a booking already hauled by the customer’s own truck', async () => { + const { service, lastMileRepository } = makeService({ hasCustomerTruck: true }); + + await expect( + service.create({ bookingId: 'b-1', advancedPayment: 0 } as never), + ).rejects.toBeInstanceOf(BadRequestException); + expect(lastMileRepository.create).not.toHaveBeenCalled(); + }); + + it('rejects an import that only chose collection — that is the export leg', async () => { + const { service } = makeService({ + booking: { tradeDirection: 'IMPORT', firstMile: 'Modjo', lastMile: null }, + }); + + await expect( + service.create({ bookingId: 'b-1', advancedPayment: 0 } as never), + ).rejects.toBeInstanceOf(BadRequestException); + }); + + it('returns the existing leg without re-checking, so the queue stays idempotent', async () => { + const { service, lastMileRepository } = makeService({ hasCustomerTruck: true }); + lastMileRepository.findAll.mockResolvedValue([{ id: 'lm-existing' }]); + + const result = await service.create({ bookingId: 'b-1', advancedPayment: 0 } as never); + + expect(result).toEqual({ id: 'lm-existing' }); + expect(lastMileRepository.create).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/edr-freight-api/src/modules/last-mile/last-mile.service.ts b/apps/edr-freight-api/src/modules/last-mile/last-mile.service.ts index 1ee9d7191..aa293759c 100644 --- a/apps/edr-freight-api/src/modules/last-mile/last-mile.service.ts +++ b/apps/edr-freight-api/src/modules/last-mile/last-mile.service.ts @@ -7,6 +7,11 @@ import { } from '@nestjs/common'; import { DataSource, FindOptionsWhere, In, IsNull, Not } from 'typeorm'; +import { + NO_MILE_SERVICE_MESSAGE, + SELF_HAUL_CONFLICT_MESSAGE, + usesEdrMileService, +} from '../../common/mile-haulage.util'; import { BookingsRepository } from '../bookings/bookings.repository'; import { DriversService } from '../drivers/drivers.service'; import { SmsClientService } from '../notifications/sms-client.service'; @@ -126,6 +131,47 @@ export class LastMileService { } } + /** + * Only a booking that actually bought EDR delivery belongs in the last-mile + * queue, and a booking hauled by the customer's own truck must never also get + * an EDR leg. + * + * Both halves were missing: creation checked payment alone, so any paid + * booking could be accepted into the queue — including one whose contract + * chose no road legs at all, and one already carrying a customer truck. The + * mirror rule existed on the truck side only + * (CustomerTruckService.assertSelfHaulPaid), so whichever side acted second + * silently opened a competing delivery on the same booking. + */ + private async assertEdrHaulsThisBooking(bookingId?: string | null): Promise { + if (!bookingId) return; + + const [booking] = await this.dataSource.query( + `SELECT trade_direction AS "tradeDirection", + first_mile_pickup_address AS "firstMile", + last_mile_delivery_address AS "lastMile" + FROM freight.bookings + WHERE id = $1 AND deleted_at IS NULL`, + [bookingId], + ); + // The road legs are chosen on the contract and copied onto the booking, so + // the booking's own addresses answer this without a join. + if (booking && !usesEdrMileService(booking)) { + throw new BadRequestException(NO_MILE_SERVICE_MESSAGE); + } + + const [truck] = await this.dataSource.query( + `SELECT 1 + FROM freight.customer_truck_assignments + WHERE booking_id = $1 AND deleted_at IS NULL + LIMIT 1`, + [bookingId], + ); + if (truck) { + throw new BadRequestException(SELF_HAUL_CONFLICT_MESSAGE); + } + } + async acceptBooking(bookingReference: string): Promise { const booking = await this.bookingsRepository.findByReference(bookingReference); @@ -352,6 +398,8 @@ export class LastMileService { return existing; } + await this.assertEdrHaulsThisBooking(dto.bookingId); + const record = await this.lastMileRepository.create({ bookingId: dto.bookingId, status: dto.status ?? 'READY_TO_TRANSIT',