Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
-- #329: an order links to the stop it's collected at as well as the one it's delivered to, so a
-- shipment can have several pickups (a milk run) and each pickup's orders go in transit when the
-- vehicle leaves that pickup.
--
-- Table role: Order is authoritative. Index budget: no new index; Order is already well over budget.
-- Orders are only ever looked up by pickup stop within one shipment, and that query is served by the
-- OrderShipment(shipmentId) index. No backfill: a null pickupStopId means the shipment's first
-- pickup stop, which is what every existing order was collected at.

ALTER TABLE "Order" ADD COLUMN "pickupStopId" TEXT;

ALTER TABLE "Order" ADD CONSTRAINT "Order_pickupStopId_fkey" FOREIGN KEY ("pickupStopId") REFERENCES "ShipmentStop"("id") ON DELETE SET NULL ON UPDATE CASCADE;
10 changes: 8 additions & 2 deletions backend/prisma/schema/tms.prisma
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,9 @@ model ShipmentStop {
status String @default("pending") // pending, arrived, in_progress, completed, skipped

// Orders to be delivered at this stop
orders Order[]
orders Order[] @relation("OrderDeliveryStop")
// Orders collected at this stop (#329)
pickupOrders Order[] @relation("OrderPickupStop")

// Geofencing (for automatic status updates)
geofenceRadius Float? // Radius in meters for geofence trigger
Expand Down Expand Up @@ -589,8 +591,12 @@ model Order {
exceptionResolvedAt DateTime? // When the exception was resolved

// Link to the specific stop where this order should be delivered (for multi-stop shipments)
deliveryStop ShipmentStop? @relation(fields: [deliveryStopId], references: [id])
deliveryStop ShipmentStop? @relation("OrderDeliveryStop", fields: [deliveryStopId], references: [id])
deliveryStopId String?
// Where the order is collected (#329). Null on orders linked before multi-pickup: they're picked
// up at their shipment's first pickup stop.
pickupStop ShipmentStop? @relation("OrderPickupStop", fields: [pickupStopId], references: [id])
pickupStopId String?

// Trackable units (pallets, totes, boxes, etc.)
trackableUnits TrackableUnit[]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ function makeShipment(overrides: any = {}) {
};
}

// The shipment already has its pickup at the orders' origin; any other stop lookup finds nothing.
const findStop = (args: any) => Promise.resolve(
args?.where?.stopType?.in?.includes('pickup') && args.where.locationId === 'loc-origin' ? { id: 'stop-pickup', sequenceNumber: 1 } : null,
);

function makeTx(shipment: any = makeShipment()) {
return {
shipment: {
Expand All @@ -43,7 +48,7 @@ function makeTx(shipment: any = makeShipment()) {
order: { findMany: jest.fn(), update: jest.fn().mockResolvedValue({}) },
orderShipment: { create: jest.fn().mockResolvedValue({}), count: jest.fn().mockResolvedValue(1) },
shipmentStop: {
findFirst: jest.fn().mockResolvedValue(null),
findFirst: jest.fn(findStop),
create: jest.fn().mockResolvedValue({ id: 'stop-1' }),
aggregate: jest.fn().mockResolvedValue({ _max: { sequenceNumber: null } }),
findMany: jest.fn().mockResolvedValue([]),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ function makeOrder(overrides: any = {}) {
};
}

// The shipment already has its pickup at the orders' origin; any other stop lookup finds nothing.
const findStop = (args: any) => Promise.resolve(
args?.where?.stopType?.in?.includes('pickup') && args.where.locationId === 'loc-origin' ? { id: 'stop-pickup', sequenceNumber: 1 } : null,
);

function makeTx() {
return {
order: { findMany: jest.fn(), update: jest.fn().mockResolvedValue({}) },
Expand All @@ -31,7 +36,7 @@ function makeTx() {
},
orderShipment: { create: jest.fn().mockResolvedValue({}) },
shipmentStop: {
findFirst: jest.fn().mockResolvedValue(null),
findFirst: jest.fn(findStop),
create: jest.fn().mockResolvedValue({ id: 'stop-1' }),
aggregate: jest.fn().mockResolvedValue({ _max: { sequenceNumber: null } }),
findMany: jest.fn().mockResolvedValue([]),
Expand Down
28 changes: 28 additions & 0 deletions backend/src/__tests__/commands/OrderDeliveryCommands.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,34 @@ describe('RecordStopOrdersDeliveryCommandHandler (#325)', () => {
expect(result.events.map((e) => e.type)).toEqual([EVENT_TYPES.ORDER_DELIVERY_STATUS_CHANGED]);
});

it("only moves the orders collected at a later pickup when it completes (#329)", async () => {
const { tx, prisma } = mockTx();
tx.shipmentStop.findFirst.mockResolvedValue(stopRow('stop-mid', 'pickup'));
const handler = new RecordStopOrdersDeliveryCommandHandler(prisma, mockEventBus().bus);

await handler.execute(createTestCommand(RECORD_STOP_ORDERS_DELIVERY, {
stopId: 'stop-mid', status: 'completed', method: 'geofence', occurredAt: OCCURRED,
}));

expect(tx.order.findMany).toHaveBeenCalledWith(expect.objectContaining({
where: expect.objectContaining({ AND: [{ OR: [{ pickupStopId: 'stop-mid' }] }] }),
}));
});

it('counts orders with no pickup stop as collected at the first pickup (#329)', async () => {
const { tx, prisma } = mockTx();
tx.shipmentStop.findFirst.mockResolvedValue(stopRow('stop-origin', 'pickup'));
const handler = new RecordStopOrdersDeliveryCommandHandler(prisma, mockEventBus().bus);

await handler.execute(createTestCommand(RECORD_STOP_ORDERS_DELIVERY, {
stopId: 'stop-origin', status: 'completed', method: 'geofence', occurredAt: OCCURRED,
}));

expect(tx.order.findMany).toHaveBeenCalledWith(expect.objectContaining({
where: expect.objectContaining({ AND: [{ OR: [{ pickupStopId: 'stop-origin' }, { pickupStopId: null }] }] }),
}));
});

it('changes no orders when the pickup only arrives', async () => {
const { tx, prisma } = mockTx();
tx.shipmentStop.findFirst.mockResolvedValue(stopRow('stop-origin', 'pickup'));
Expand Down
59 changes: 59 additions & 0 deletions backend/src/__tests__/commands/linkOrdersToShipment.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { linkOrdersToShipment } from '../../commands/shipments/linkOrdersToShipment';

/** An in-memory stop table for one shipment. */
function mockTx(stops: Array<{ id: string; locationId: string; stopType: string }>) {
let rows = stops.map((s, i) => ({ ...s, sequenceNumber: i + 1 }));
let nextId = 1;
const matches = (r: any, where: any) =>
(!where.locationId || r.locationId === where.locationId)
&& (!where.stopType || where.stopType.in.includes(r.stopType))
&& (!where.sequenceNumber || r.sequenceNumber >= where.sequenceNumber.gte);
const sorted = (orderBy: any) => [...rows].sort((a, b) => (orderBy?.sequenceNumber === 'desc' ? b.sequenceNumber - a.sequenceNumber : a.sequenceNumber - b.sequenceNumber));
const tx = {
shipment: { update: jest.fn().mockResolvedValue({}) },
shipmentStop: {
findFirst: jest.fn(async ({ where, orderBy }: any) => sorted(orderBy).find((r) => matches(r, where)) ?? null),
findMany: jest.fn(async ({ where, orderBy }: any) => sorted(orderBy).filter((r) => matches(r, where))),
update: jest.fn(async ({ where, data }: any) => { Object.assign(rows.find((r) => r.id === where.id)!, data); return {}; }),
create: jest.fn(async ({ data }: any) => { const row = { id: `new-${nextId++}`, ...data }; rows.push(row); return row; }),
aggregate: jest.fn(async () => ({ _max: { sequenceNumber: Math.max(0, ...rows.map((r) => r.sequenceNumber)) } })),
},
orderShipment: { create: jest.fn().mockResolvedValue({}) },
order: { update: jest.fn().mockResolvedValue({}) },
auditLog: { create: jest.fn().mockResolvedValue({}) },
} as any;
const route = () => sorted(null).map((r) => `${r.sequenceNumber}:${r.locationId}:${r.stopType}`);
return { tx, route };
}

const order = (id: string, originId: string, destinationId: string) => ({ id, orderNumber: id, status: 'verified', originId, destinationId, trackableUnits: [], lineItems: [] });

describe('linkOrdersToShipment pickups (#329)', () => {
it("adds a pickup for an order from a new origin after the existing pickups and before the drops", async () => {
const { tx, route } = mockTx([
{ id: 'p1', locationId: 'A', stopType: 'pickup' },
{ id: 'd1', locationId: 'X', stopType: 'delivery' },
]);

const { stopsCreated } = await linkOrdersToShipment(tx, { id: 'ship-1', reference: 'S', items: [] }, [order('o2', 'B', 'Y')], { orgId: 'org-1', actorId: null }, () => 'linked', jest.fn());

expect(stopsCreated).toBe(2);
expect(route()).toEqual(['1:A:pickup', '2:B:pickup', '3:X:delivery', '4:Y:delivery']);
expect(tx.order.update).toHaveBeenCalledWith(expect.objectContaining({
data: expect.objectContaining({ pickupStopId: 'new-1', deliveryStopId: 'new-2' }),
}));
});

it('reuses the pickup and drop already at those places', async () => {
const { tx, route } = mockTx([
{ id: 'p1', locationId: 'A', stopType: 'pickup' },
{ id: 'd1', locationId: 'X', stopType: 'delivery' },
]);

const { stopsCreated } = await linkOrdersToShipment(tx, { id: 'ship-1', reference: 'S', items: [] }, [order('o1', 'A', 'X')], { orgId: 'org-1', actorId: null }, () => 'linked', jest.fn());

expect(stopsCreated).toBe(0);
expect(route()).toEqual(['1:A:pickup', '2:X:delivery']);
expect(tx.order.update).toHaveBeenCalledWith(expect.objectContaining({ data: expect.objectContaining({ pickupStopId: 'p1', deliveryStopId: 'd1' }) }));
});
});
2 changes: 1 addition & 1 deletion backend/src/__tests__/commands/syncShipmentStops.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ function mockTx(stored: StoredStop[] = []) {
let nextId = 1;
const tx = {
shipmentStop: {
findMany: jest.fn(async () => rows.map((r) => ({ id: r.id, locationId: r.locationId, _count: { orders: r.orders } }))),
findMany: jest.fn(async () => rows.map((r) => ({ id: r.id, locationId: r.locationId, stopType: r.stopType, _count: { orders: r.orders, pickupOrders: 0 } }))),
deleteMany: jest.fn(async ({ where }: any) => { rows = rows.filter((r) => !where.id.in.includes(r.id)); return { count: 0 }; }),
updateMany: jest.fn(async () => { rows.forEach((r) => { r.sequenceNumber += 100000; }); return { count: rows.length }; }),
update: jest.fn(async ({ where, data }: any) => { Object.assign(rows.find((r) => r.id === where.id)!, data); return {}; }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ interface StopSpec {
locationId: string;
sequenceNumber: number;
status?: string;
stopType?: string;
actualArrival?: Date | null;
actualDeparture?: Date | null;
}
Expand Down Expand Up @@ -163,5 +164,37 @@ describe('ArrivalCriteriaEvaluationService', () => {

expect(dispatched()).toEqual([[RECORD_JOURNEY_CHECKPOINT, expect.objectContaining({ shipmentId: 'ship-1' })]]);
});

describe('a second pickup (#329)', () => {
const milkRun = (mid: Partial<StopSpec>): StopSpec[] => [
{ id: 'stop-origin', locationId: 'loc-origin', sequenceNumber: 1, stopType: 'pickup', status: 'completed' },
{ id: 'stop-mid', locationId: 'loc-mid', sequenceNumber: 2, stopType: 'pickup', ...mid },
{ id: 'stop-dest', locationId: 'loc-dest', sequenceNumber: 3, stopType: 'delivery' },
];

it('arrives at a later pickup without completing it', async () => {
const { ping, dispatched, deliveryService } = setup(milkRun({}));
await ping('loc-mid');
expect(dispatched()).toEqual([[RECORD_GEOFENCE_ARRIVAL, expect.objectContaining({ stopId: 'stop-mid', completesStop: false })]]);
expect(deliveryService.updateOrdersForStop).toHaveBeenCalledWith('org-1', 'stop-mid', 'arrived', 'geofence', expect.any(Date));
});

it('completes a later pickup when the vehicle leaves it, moving its orders', async () => {
const { ping, dispatched, deliveryService } = setup(milkRun({ status: 'arrived' }));
await ping('loc-mid', { lat: 40.75 });
expect(dispatched()).toEqual([[RECORD_GEOFENCE_DEPARTURE, expect.objectContaining({ stopId: 'stop-mid' })]]);
expect(deliveryService.updateOrdersForStop).toHaveBeenCalledWith('org-1', 'stop-mid', 'completed', 'geofence', expect.any(Date));
});

it('infers the departure from a pickup it never saw the vehicle leave when a drop is reached', async () => {
const { ping, dispatched, deliveryService } = setup(milkRun({}));
await ping('loc-dest');
expect(dispatched()).toEqual([
[RECORD_GEOFENCE_DEPARTURE, expect.objectContaining({ stopId: 'stop-mid', inferred: true })],
[RECORD_GEOFENCE_ARRIVAL, expect.objectContaining({ stopId: 'stop-dest', completesStop: true })],
]);
expect(deliveryService.updateOrdersForStop).toHaveBeenCalledWith('org-1', 'stop-mid', 'completed', 'geofence', expect.any(Date));
});
});
});

28 changes: 11 additions & 17 deletions backend/src/__tests__/services/OrderConversionService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,20 +148,14 @@ describe('OrderConversionService', () => {
);
});

it('rejects combining orders with different origins, without dispatching', async () => {
it('allows combining orders from different origins: each becomes a pickup (#329)', async () => {
const orderA = makeOrder({ id: 'order-a', originId: 'loc-origin-1' });
const orderB = makeOrder({ id: 'order-b', originId: 'loc-origin-2' });
const prisma = {
order: { findMany: jest.fn().mockResolvedValue([orderA, orderB]) },
} as any;
const commandBus = makeCommandBus();
const service = new OrderConversionService(prisma, commandBus);
const service = new OrderConversionService({ order: { findMany: jest.fn().mockResolvedValue([orderA, orderB]) } } as any, makeCommandBus());

const result = await service.batchConvert('test-org', ['order-a', 'order-b'], { mode: 'combine' });
const check = await service.checkCompatibility('test-org', ['order-a', 'order-b']);

expect(result.success).toBe(false);
expect(result.errors[0]).toMatch(/different origins/);
expect(commandBus.dispatch).not.toHaveBeenCalled();
expect(check.errors.some((e) => /origin/.test(e))).toBe(false);
});
});

Expand Down Expand Up @@ -265,21 +259,21 @@ describe('OrderConversionService', () => {
expect(result.errors).toEqual(['Shipment not found']);
});

it('rejects orders with a different origin than the shipment, without dispatching', async () => {
it('accepts an order from another origin, which adds a pickup stop (#329)', async () => {
const order = makeOrder({ originId: 'some-other-origin' });
const shipment = makeShipment();
const prisma = {
shipment: { findFirst: jest.fn().mockResolvedValue(shipment) },
order: { findMany: jest.fn().mockResolvedValue([order]) },
orderShipment: { count: jest.fn().mockResolvedValue(0) },
} as any;
const commandBus = makeCommandBus();
const commandBus = makeCommandBus({ success: true, data: { shipmentId: 'ship-1', addedOrderIds: ['order-1'] }, events: [] });
const service = new OrderConversionService(prisma, commandBus);

const result = await service.addOrdersToShipment('test-org', 'ship-1', ['order-1']);

expect(result.success).toBe(false);
expect(result.errors[0]).toMatch(/different origin/);
expect(commandBus.dispatch).not.toHaveBeenCalled();
expect(result.errors).toEqual([]);
expect(commandBus.dispatch).toHaveBeenCalled();
});

it('rejects orders with a different customer than the shipment, without dispatching', async () => {
Expand Down Expand Up @@ -316,7 +310,7 @@ describe('OrderConversionService', () => {
});

describe('checkCompatibility', () => {
it('flags different origins and different customers as errors (#325)', async () => {
it('flags different customers as an error, but not different origins (#325, #329)', async () => {
const orderA = makeOrder({ id: 'order-a', customerId: 'cust-a', originId: 'loc-origin-1' });
const orderB = makeOrder({ id: 'order-b', customerId: 'cust-b', originId: 'loc-origin-2' });
const prisma = {
Expand All @@ -335,7 +329,7 @@ describe('OrderConversionService', () => {
expect.objectContaining({ where: expect.objectContaining({ orgId: 'test-org' }) }),
);
expect(check.compatible).toBe(false);
expect(check.errors.some((e) => /different origins/.test(e))).toBe(true);
expect(check.errors.some((e) => /origin/.test(e))).toBe(false);
expect(check.errors.some((e) => /different customers/.test(e))).toBe(true);
});
});
Expand Down
22 changes: 14 additions & 8 deletions backend/src/commands/orders/RecordStopOrdersDeliveryCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ const ACTIVE_DELIVERY = [{ deliveryStatus: null }, { deliveryStatus: 'in_transit
* Moves a shipment stop to a new status and the orders it affects with it (#325).
*
* BUSINESS RULES:
* - Completing the pickup (leaving the origin) puts every not-yet-moving order on the shipment in
* transit. It never delivers anything: before #325 this marked orders delivered if their
* delivery stop happened to be the origin (#307).
* - Completing a pickup (leaving it) puts the not-yet-moving orders collected there in transit
* (#329; orders with no pickup stop count as collected at the first pickup). It never delivers
* anything: before #325 this marked orders delivered if their delivery stop was the origin (#307).
* - Arriving at a delivery stop puts its not-yet-moving orders in transit.
* - Completing a delivery stop delivers its active orders. A refusal afterwards is a delivery
* exception raised against the order, not a reversal here.
Expand Down Expand Up @@ -74,15 +74,21 @@ export class RecordStopOrdersDeliveryCommandHandler extends BaseCommandHandler<R
},
});

const isPickup = stop.stopType === 'pickup' || stop.id === findOriginStop(stop.shipment.stops, stop.shipment.originId)?.id;
const firstPickup = findOriginStop(stop.shipment.stops, stop.shipment.originId);
const isPickup = stop.stopType === 'pickup' || stop.id === firstPickup?.id;
const change = ordersToChange(isPickup, status);
if (!change) return { ordersUpdated: 0, shipmentId: stop.shipmentId };

const orders = await tx.order.findMany({
where: {
orgId,
...(change.scope === 'shipment'
? { orderShipments: { some: { shipmentId: stop.shipmentId } } }
...(change.scope === 'pickup'
? {
orderShipments: { some: { shipmentId: stop.shipmentId } },
// The orders collected here; orders linked before multi-pickup have no pickup stop and
// were collected at the first one (#329).
AND: [{ OR: [{ pickupStopId: stopId }, ...(stop.id === firstPickup?.id ? [{ pickupStopId: null }] : [])] }],
}
: { deliveryStopId: stopId }),
OR: change.from === 'unmoved' ? [{ deliveryStatus: null }] : ACTIVE_DELIVERY,
},
Expand Down Expand Up @@ -129,13 +135,13 @@ export class RecordStopOrdersDeliveryCommandHandler extends BaseCommandHandler<R
}

interface OrderChange {
scope: 'shipment' | 'stop';
scope: 'pickup' | 'stop';
from: 'unmoved' | 'active';
to: DeliveryStatus;
}

function ordersToChange(isPickup: boolean, status: string): OrderChange | null {
if (isPickup) return status === 'completed' ? { scope: 'shipment', from: 'unmoved', to: 'in_transit' } : null;
if (isPickup) return status === 'completed' ? { scope: 'pickup', from: 'unmoved', to: 'in_transit' } : null;
if (status === 'completed') return { scope: 'stop', from: 'active', to: 'delivered' };
if (status === 'arrived' || status === 'in_progress') return { scope: 'stop', from: 'unmoved', to: 'in_transit' };
return null;
Expand Down
3 changes: 3 additions & 0 deletions backend/src/commands/shipments/CreateShipmentCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ export interface CreateShipmentPayload {
properShippingName?: string | null;
requiredEquipmentType?: string | null;
waypoints?: string[];
/** Further pickups after the origin, in order (#329). */
pickupWaypoints?: string[];
}

export interface CreateShipmentResult {
Expand Down Expand Up @@ -317,6 +319,7 @@ export class CreateShipmentCommandHandler extends BaseCommandHandler<CreateShipm
orgId: command.orgId,
shipmentId: shipment.id,
originId: finalOriginId,
pickupWaypoints: body.pickupWaypoints,
waypoints: body.waypoints,
destinationId: finalDestinationId,
});
Expand Down
Loading
Loading