Skip to content

Commit edf46ec

Browse files
authored
Merge pull request #1741 from nafiuishaaq/fix/1665-1666-eta-state-codes-and-metric-denominators
fix(eta,carriers): disambiguate colliding US state codes; correct two metric denominators
2 parents 0dd39ac + 2c539cc commit edf46ec

2 files changed

Lines changed: 131 additions & 11 deletions

File tree

‎backend/src/carriers/carriers.service.ts‎

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export class CarriersService {
1515
const [
1616
totalAcceptedResult,
1717
totalCompletedResult,
18-
totalCancelledResult,
18+
carrierCancelledResult,
1919
deliveredCountResult,
2020
onTimeResult,
2121
totalEarningsResult,
@@ -36,21 +36,44 @@ export class CarriersService {
3636
status: ShipmentStatus.COMPLETED,
3737
})
3838
.getRawOne<{ count: string }>(),
39+
// Carrier-initiated cancellations only (#1666). A cancellation is the
40+
// carrier's fault only when the carrier is the one recorded as changing the
41+
// status; `changedById` in the status history is the shipped record of who
42+
// cancelled. Counting every cancelled shipment penalised the carrier for
43+
// cancellations the shipper made, which it cannot prevent or influence.
3944
this.shipmentRepo
4045
.createQueryBuilder('shipment')
4146
.select('COUNT(*)', 'count')
4247
.where('shipment.carrierId = :carrierId', { carrierId })
4348
.andWhere('shipment.status = :status', {
4449
status: ShipmentStatus.CANCELLED,
4550
})
51+
.andWhere(
52+
`EXISTS (
53+
SELECT 1 FROM shipment_status_history h
54+
WHERE h.shipment_id = shipment.id
55+
AND h.to_status = :cancelled
56+
AND h.changed_by_id = :carrierId
57+
)`,
58+
{ cancelled: ShipmentStatus.CANCELLED, carrierId },
59+
)
4660
.getRawOne<{ count: string }>(),
61+
// Denominator for the on-time rate: only deliveries that actually had an
62+
// estimate to be measured against (#1666).
63+
//
64+
// `estimatedDeliveryDate` is optional when a shipment is created, so this
65+
// used to count *every* delivered/completed shipment while the numerator
66+
// required both dates. A shipment with no estimate was therefore always
67+
// counted as not on time, dragging a carrier's rate down for shipments
68+
// that were never given a target in the first place.
4769
this.shipmentRepo
4870
.createQueryBuilder('shipment')
4971
.select('COUNT(*)', 'count')
5072
.where('shipment.carrierId = :carrierId', { carrierId })
5173
.andWhere('shipment.status IN (:...statuses)', {
5274
statuses: [ShipmentStatus.DELIVERED, ShipmentStatus.COMPLETED],
5375
})
76+
.andWhere('shipment.estimatedDeliveryDate IS NOT NULL')
5477
.getRawOne<{ count: string }>(),
5578
this.shipmentRepo
5679
.createQueryBuilder('shipment')
@@ -75,21 +98,28 @@ export class CarriersService {
7598

7699
const totalAccepted = Number(totalAcceptedResult?.count ?? '0');
77100
const totalCompleted = Number(totalCompletedResult?.count ?? '0');
78-
const totalCancelled = Number(totalCancelledResult?.count ?? '0');
101+
const carrierCancelled = Number(carrierCancelledResult?.count ?? '0');
79102
const delivered = Number(deliveredCountResult?.count ?? '0');
80103
const onTimeDeliveries = Number(onTimeResult?.count ?? '0');
81104
const onTimeRate = delivered > 0 ? onTimeDeliveries / delivered : 0;
82105
const totalEarnings = parseFloat(totalEarningsResult?.total ?? '0');
83106
const cancellationRate =
84-
totalAccepted > 0 ? totalCancelled / totalAccepted : 0;
107+
totalAccepted > 0 ? carrierCancelled / totalAccepted : 0;
85108

86109
return {
87110
totalAccepted,
88111
totalCompleted,
89-
totalCancelled,
112+
/** Cancellations the carrier itself performed (see the query above). */
113+
totalCancelled: carrierCancelled,
90114
onTimeRate: Math.round(onTimeRate * 100) / 100,
91115
cancellationRate: Math.round(cancellationRate * 100) / 100,
92116
totalEarnings,
117+
// Counts are returned alongside the rates so a UI or an admin decision can
118+
// see how few deliveries a rate is based on — a 0.0 or 1.0 rate over three
119+
// shipments is not comparable to the same rate over three hundred.
120+
onTimeDeliveries,
121+
deliveriesWithEstimate: delivered,
122+
carrierCancelledCancellations: carrierCancelled,
93123
};
94124
}
95125
}

‎backend/src/shipments/eta.service.ts‎

Lines changed: 97 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,39 @@ const CITY_TOKENS: Readonly<Record<string, ShipmentZone>> = Object.freeze({
212212
MUMBAI: ShipmentZone.AS,
213213
});
214214

215+
/**
216+
* US state and district codes → `ShipmentZone.US`.
217+
*
218+
* These collide head-on with three ISO 3166-1 alpha-2 codes that are already in
219+
* `LOCATION_TOKENS`: `CA` (Canada), `DE` (Germany) and `IN` (India). A US
220+
* address like "Los Angeles, CA" therefore matched the city table (US) *and* the
221+
* country table (CA) and `resolveZone` rejected it as a conflicting location,
222+
* while "Indianapolis, IN" — a city with no entry — silently resolved to the
223+
* Asia zone and quoted a 12-day transit.
224+
*
225+
* The whole-token guarantee that the country table's doc comment relies on does
226+
* not save this: US state codes *are* legitimate whole tokens. Only the trailing
227+
* `City, ST` shape disambiguates them, so `collectZoneMatches` applies this
228+
* table under narrow conditions — see the note there.
229+
*/
230+
const US_STATE_TOKENS: Readonly<Record<string, ShipmentZone>> = Object.freeze({
231+
AL: ShipmentZone.US, AK: ShipmentZone.US, AZ: ShipmentZone.US, AR: ShipmentZone.US,
232+
CA: ShipmentZone.US, CO: ShipmentZone.US, CT: ShipmentZone.US, DE: ShipmentZone.US,
233+
FL: ShipmentZone.US, GA: ShipmentZone.US, HI: ShipmentZone.US, ID: ShipmentZone.US,
234+
IL: ShipmentZone.US, IN: ShipmentZone.US, IA: ShipmentZone.US, KS: ShipmentZone.US,
235+
KY: ShipmentZone.US, LA: ShipmentZone.US, ME: ShipmentZone.US, MD: ShipmentZone.US,
236+
MA: ShipmentZone.US, MI: ShipmentZone.US, MN: ShipmentZone.US, MS: ShipmentZone.US,
237+
MO: ShipmentZone.US, MT: ShipmentZone.US, NE: ShipmentZone.US, NV: ShipmentZone.US,
238+
NH: ShipmentZone.US, NJ: ShipmentZone.US, NM: ShipmentZone.US, NY: ShipmentZone.US,
239+
NC: ShipmentZone.US, ND: ShipmentZone.US, OH: ShipmentZone.US, OK: ShipmentZone.US,
240+
OR: ShipmentZone.US, PA: ShipmentZone.US, RI: ShipmentZone.US, SC: ShipmentZone.US,
241+
SD: ShipmentZone.US, TN: ShipmentZone.US, TX: ShipmentZone.US, UT: ShipmentZone.US,
242+
VT: ShipmentZone.US, VA: ShipmentZone.US, WA: ShipmentZone.US, WV: ShipmentZone.US,
243+
WI: ShipmentZone.US, WY: ShipmentZone.US,
244+
// District of Columbia — not a state, but shipped from and to like one.
245+
DC: ShipmentZone.US,
246+
});
247+
215248
/** Splits free-form location text into comparable upper-case tokens. */
216249
function tokenize(location: string): string[] {
217250
return (
@@ -232,7 +265,7 @@ interface ZoneMatch {
232265
zone: ShipmentZone;
233266
/** The exact text that matched, for error messages. */
234267
phrase: string;
235-
kind: 'city' | 'country';
268+
kind: 'city' | 'country' | 'state';
236269
}
237270

238271
/**
@@ -248,29 +281,86 @@ function collectZoneMatches(location: string): ZoneMatch[] {
248281
const matches: ZoneMatch[] = [];
249282
const seenPhrases = new Set<string>();
250283

251-
for (const [table, kind] of [
252-
[CITY_TOKENS, 'city'],
253-
[LOCATION_TOKENS, 'country'],
254-
] as const) {
284+
const scan = (
285+
table: Readonly<Record<string, ShipmentZone>>,
286+
kind: ZoneMatch['kind'],
287+
firstEnd?: { value: number },
288+
): void => {
255289
for (
256290
let length = MAX_PLACE_PHRASE_TOKENS;
257291
length >= 1;
258292
length -= 1
259293
) {
260294
for (let start = 0; start + length <= tokens.length; start += 1) {
261295
const phrase = tokens.slice(start, start + length).join(' ');
262-
// A phrase already matched by the city table must not be
263-
// re-reported by the country table.
296+
// A phrase already matched by an earlier table must not be
297+
// re-reported by a later one.
264298
if (seenPhrases.has(phrase)) continue;
265299
const zone = table[phrase];
266300
if (zone) {
267301
seenPhrases.add(phrase);
268302
matches.push({ zone, phrase, kind });
303+
if (firstEnd !== undefined && firstEnd.value < 0) {
304+
firstEnd.value = start + length;
305+
}
269306
}
270307
}
271308
}
309+
};
310+
311+
// Index just past the first city match, used below to tell the `City, ST`
312+
// shape from a longer string that merely ends in a state-like token.
313+
const cityEnd = { value: -1 };
314+
scan(CITY_TOKENS, 'city', cityEnd);
315+
316+
// Disambiguate a trailing two-letter token that is both a US state code and an
317+
// ISO country code (`CA`, `DE`, `IN`). Two deliberate restrictions keep this
318+
// from breaking the cases that already worked:
319+
//
320+
// 1. Only when the city table matched nothing. A known city has already
321+
// decided the zone, and a state code must not overrule it — that is what
322+
// keeps "Toronto, CA" in Canada and "Los Angeles, CA" in the US rather
323+
// than turning the former into a conflict.
324+
// 2. Only when a token precedes it, i.e. the `City, ST` shape. A bare "DE"
325+
// must keep resolving to Germany, which is what a caller sending to
326+
// Germany with no city typed actually means.
327+
//
328+
// Trade-off: for a city that is *not* in `CITY_TOKENS`, "Frankfurt, DE" now
329+
// reads as Delaware. That ambiguity is inherent — nothing in the string
330+
// distinguishes the two — and the fix is to type the country name, which the
331+
// country table matches on the earlier, explicit-name pass and which therefore
332+
// takes priority over the bare code.
333+
const trailing = tokens.length > 0 ? tokens[tokens.length - 1] : undefined;
334+
335+
if (matches.length === 0 && tokens.length > 1 && trailing !== undefined) {
336+
const zone = US_STATE_TOKENS[trailing];
337+
if (zone) {
338+
seenPhrases.add(trailing);
339+
matches.push({ zone, phrase: trailing, kind: 'state' });
340+
}
341+
} else if (
342+
matches.length > 0 &&
343+
trailing !== undefined &&
344+
// Only the immediate `City, ST` shape. "Lagos, NG vs CA" also ends in a bare
345+
// `CA`, but it is not a US address, and suppressing that `CA` would hide a
346+
// real conflict the caller needs told about.
347+
cityEnd.value === tokens.length - 1 &&
348+
US_STATE_TOKENS[trailing] !== undefined
349+
) {
350+
// A city match already decided the zone, and the trailing token is a state
351+
// code that is *also* a country code. The country table must not report it
352+
// again: that pair is what made "Los Angeles, CA" resolve to both US and CA
353+
// and be rejected as a conflict.
354+
//
355+
// Only US state codes are suppressed, so genuine country/city disagreements
356+
// are still reported — "Lagos, US" still conflicts, because `US` is not a
357+
// state code, and "Lagos, Canada" still conflicts, because `CANADA` is not a
358+
// bare two-letter token.
359+
seenPhrases.add(trailing);
272360
}
273361

362+
scan(LOCATION_TOKENS, 'country');
363+
274364
return matches;
275365
}
276366

0 commit comments

Comments
 (0)