Merge pull request #1269 from Tria-plc/alpha

fix: ( notifications ) stop group-booking SMS greeting the wrong pass…

apps/edr-passenger-api/src/common/utils/booking-sms.utils.spec.ts

@@ -0,0 +1,153 @@
+import { buildSeatSummary } from './booking-sms.utils';
+
+const seat = (passengerName: string, seatNumber: string, leg = 1, coachType = 'VIP Bed') => ({
+  passengerName,
+  leg,
+  seat: { seatNumber, coach: { number: 'VIP-0001 (DJ)', coachType: { name: coachType } } },
+});
+
+describe('buildSeatSummary', () => {
+  it('greets a solo traveller by name and omits the name from the seat line', () => {
+    const { passengerName, trainSeatLines } = buildSeatSummary([seat('Yanet', '9')], 'ONE_WAY');
+
+    expect(passengerName).toBe('Yanet');
+    expect(trainSeatLines).toBe('VIP-0001 (DJ) VIP Bed, seat no. 9');
+    expect(trainSeatLines).not.toContain('Train/Seat');
+    expect(trainSeatLines).not.toContain('Yanet');
+  });
+
+  it('greets a group collectively and names each seat', () => {
+    const { passengerName, trainSeatLines } = buildSeatSummary(
+      [seat('Yanet', '4'), seat('Abebe', '6'), seat('Sara', '9'), seat('Helen', '10')],
+      'ONE_WAY',
+    );
+
+    expect(passengerName).toBe('Passengers');
+    expect(trainSeatLines).toBe(
+      [
+        'Yanet, VIP-0001 (DJ) VIP Bed, seat no. 4',
+        'Abebe, VIP-0001 (DJ) VIP Bed, seat no. 6',
+        'Sara, VIP-0001 (DJ) VIP Bed, seat no. 9',
+        'Helen, VIP-0001 (DJ) VIP Bed, seat no. 10',
+      ].join('\n'),
+    );
+  });
+
+  // The reported bug: the seats query had no orderBy, so Postgres heap order put the LAST
+  // passenger first and the SMS greeted them while texting the first passenger's phone.
+  it('is immune to seat rows arriving in an arbitrary order', () => {
+    const rows = [seat('Yanet', '9'), seat('Helen', '10'), seat('Abebe', '6'), seat('Sara', '4')];
+
+    const { passengerName, trainSeatLines } = buildSeatSummary(rows, 'ONE_WAY');
+
+    expect(passengerName).toBe('Passengers');
+    // Every line pairs the right person with their own seat, regardless of input order.
+    expect(trainSeatLines).toBe(
+      [
+        'Sara, VIP-0001 (DJ) VIP Bed, seat no. 4',
+        'Abebe, VIP-0001 (DJ) VIP Bed, seat no. 6',
+        'Yanet, VIP-0001 (DJ) VIP Bed, seat no. 9',
+        'Helen, VIP-0001 (DJ) VIP Bed, seat no. 10',
+      ].join('\n'),
+    );
+  });
+
+  it('sorts seat numbers numerically, not lexicographically', () => {
+    const { trainSeatLines } = buildSeatSummary(
+      [seat('A', '9'), seat('B', '10'), seat('C', '6'), seat('D', '4')],
+      'ONE_WAY',
+    );
+
+    expect(trainSeatLines.match(/seat no\. \d+/g)).toEqual([
+      'seat no. 4',
+      'seat no. 6',
+      'seat no. 9',
+      'seat no. 10',
+    ]);
+  });
+
+  it('labels round-trip legs as Outbound/Return, listing each passenger once per leg', () => {
+    const { passengerName, trainSeatLines } = buildSeatSummary(
+      [seat('Yanet', '9', 1), seat('Abebe', '10', 1), seat('Yanet', '3', 2), seat('Abebe', '4', 2)],
+      'ROUND_TRIP',
+    );
+
+    expect(passengerName).toBe('Passengers');
+    expect(trainSeatLines).toBe(
+      [
+        'Outbound:',
+        'Yanet, VIP-0001 (DJ) VIP Bed, seat no. 9',
+        'Abebe, VIP-0001 (DJ) VIP Bed, seat no. 10',
+        'Return:',
+        'Yanet, VIP-0001 (DJ) VIP Bed, seat no. 3',
+        'Abebe, VIP-0001 (DJ) VIP Bed, seat no. 4',
+      ].join('\n'),
+    );
+  });
+
+  // TRANSIT leg 2 is a connecting segment of the same outbound journey — never a return.
+  it('labels transit legs as Leg 1/Leg 2, never Return', () => {
+    const { trainSeatLines } = buildSeatSummary(
+      [seat('Yanet', '9', 1), seat('Abebe', '10', 1), seat('Yanet', '3', 2), seat('Abebe', '4', 2)],
+      'TRANSIT',
+    );
+
+    expect(trainSeatLines).toContain('Leg 1:');
+    expect(trainSeatLines).toContain('Leg 2:');
+    expect(trainSeatLines).not.toContain('Return');
+    expect(trainSeatLines).not.toContain('Outbound');
+  });
+
+  it('labels all four round-trip-transit legs', () => {
+    const { trainSeatLines } = buildSeatSummary(
+      [1, 2, 3, 4].map((leg) => seat('Yanet', String(leg), leg)),
+      'ROUND_TRIP_TRANSIT',
+    );
+
+    expect(trainSeatLines).toBe(
+      [
+        'Outbound leg 1:',
+        'VIP-0001 (DJ) VIP Bed, seat no. 1',
+        'Outbound leg 2:',
+        'VIP-0001 (DJ) VIP Bed, seat no. 2',
+        'Return leg 1:',
+        'VIP-0001 (DJ) VIP Bed, seat no. 3',
+        'Return leg 2:',
+        'VIP-0001 (DJ) VIP Bed, seat no. 4',
+      ].join('\n'),
+    );
+  });
+
+  it('greets a solo round-trip traveller by name (same person on both legs)', () => {
+    const { passengerName } = buildSeatSummary(
+      [seat('Yanet', '9', 1), seat('Yanet', '3', 2)],
+      'ROUND_TRIP',
+    );
+
+    expect(passengerName).toBe('Yanet');
+  });
+
+  it('trims a trailing space on the coach type instead of emitting "Bed , seat"', () => {
+    const { trainSeatLines } = buildSeatSummary([seat('Yanet', '9', 1, 'VIP Bed ')], 'ONE_WAY');
+
+    expect(trainSeatLines).toBe('VIP-0001 (DJ) VIP Bed, seat no. 9');
+  });
+
+  it('falls back safely on empty or malformed input', () => {
+    expect(buildSeatSummary([], 'ONE_WAY')).toEqual({ passengerName: 'Passenger', trainSeatLines: '' });
+
+    const { passengerName, trainSeatLines } = buildSeatSummary([{ leg: 1 }], 'ONE_WAY');
+    expect(passengerName).toBe('Passenger');
+    expect(trainSeatLines).toBe('-, seat no. -');
+  });
+
+  it('falls back to a generic leg heading for an unknown booking type', () => {
+    const { trainSeatLines } = buildSeatSummary(
+      [seat('Yanet', '9', 1), seat('Yanet', '3', 2)],
+      'SOMETHING_NEW',
+    );
+
+    expect(trainSeatLines).toContain('Leg 1:');
+    expect(trainSeatLines).toContain('Leg 2:');
+  });
+});

apps/edr-passenger-api/src/common/utils/booking-sms.utils.ts

@@ -0,0 +1,115 @@
+/**
+ * Builds the two passenger-facing values the `booking.created` SMS/email template needs:
+ * the `{{passengerName}}` salutation and the `{{trainSeatLines}}` block.
+ *
+ * Why this is a shared pure helper rather than inline logic: the salutation used to be
+ * `seats[0]?.passengerName`, and the query loading those seats had no `orderBy`. Postgres
+ * returns heap order for an unordered SELECT, and an UPDATE relocates a row to the end of
+ * the heap — so a group booking regularly greeted the LAST passenger while texting the
+ * first one's phone. Deriving both values from the whole seat set, sorted deterministically,
+ * removes the dependency on row order entirely, and keeps the formatting unit-testable
+ * without a Nest testing module.
+ *
+ * Group bookings send ONE SMS to Booking.contactPhone by design — BookingSeat has no
+ * phone/email column, so there is no per-passenger recipient. Hence 2+ passengers are
+ * greeted collectively and each seat line names its own occupant.
+ */
+
+export interface SeatSummary {
+  /** Salutation: the traveller's name when solo, otherwise 'Passengers'. */
+  passengerName: string;
+  /** One line per booked seat, newline-joined, with a heading per leg on multi-leg bookings. */
+  trainSeatLines: string;
+}
+
+/**
+ * Seat numbers are stored as strings of digits (Seat.seatNumber), so they must be compared
+ * numerically — a plain string compare orders '10' before '9'. Non-numeric labels sort last,
+ * then alphabetically among themselves.
+ */
+function compareSeatNumber(a: string, b: string): number {
+  const na = Number.parseInt(a, 10);
+  const nb = Number.parseInt(b, 10);
+  const aNum = Number.isNaN(na);
+  const bNum = Number.isNaN(nb);
+  if (aNum && bNum) return a.localeCompare(b);
+  if (aNum) return 1;
+  if (bNum) return -1;
+  return na - nb || a.localeCompare(b);
+}
+
+const str = (v: unknown): string => (typeof v === 'string' ? v.trim() : v == null ? '' : String(v).trim());
+
+const ROUND_TRIP_TRANSIT_LEGS: Record<number, string> = {
+  1: 'Outbound leg 1',
+  2: 'Outbound leg 2',
+  3: 'Return leg 1',
+  4: 'Return leg 2',
+};
+
+/**
+ * Leg numbering means different things per booking type — see the enum documented on
+ * TicketsController.validate. TRANSIT's leg 2 is a connecting segment of the SAME outbound
+ * journey, so it must never be labelled 'Return'.
+ */
+function legLabel(bookingType: string | undefined, leg: number): string {
+  switch (bookingType) {
+    case 'ROUND_TRIP':
+      return leg === 1 ? 'Outbound' : leg === 2 ? 'Return' : `Leg ${leg}`;
+    case 'TRANSIT':
+      return `Leg ${leg}`;
+    case 'ROUND_TRIP_TRANSIT':
+      return ROUND_TRIP_TRANSIT_LEGS[leg] ?? `Leg ${leg}`;
+    default:
+      // Unknown or newly added booking type — degrade to a generic heading rather than guessing.
+      return `Leg ${leg}`;
+  }
+}
+
+export function buildSeatSummary(seats: any[], bookingType?: string): SeatSummary {
+  const rows = [...(seats ?? [])].sort(
+    (a, b) =>
+      (a?.leg ?? 1) - (b?.leg ?? 1) ||
+      str(a?.seat?.coach?.number).localeCompare(str(b?.seat?.coach?.number)) ||
+      compareSeatNumber(str(a?.seat?.seatNumber), str(b?.seat?.seatNumber)),
+  );
+
+  // Distinct travellers. A round-trip/transit booking has one row per passenger PER LEG, so
+  // the same name legitimately repeats — count people, not rows.
+  const names: string[] = [];
+  for (const row of rows) {
+    const name = str(row?.passengerName);
+    if (name && !names.includes(name)) names.push(name);
+  }
+  const isGroup = names.length > 1;
+
+  const line = (row: any): string => {
+    const coach = str(row?.seat?.coach?.number) || '-';
+    const coachType = str(row?.seat?.coach?.coachType?.name);
+    const seatNo = str(row?.seat?.seatNumber) || '-';
+    // Trim each part before joining: the coach-type name carries a trailing space in some
+    // records, which a `.replace(/ +/g, ' ')` collapse cannot remove (it shrinks runs of
+    // spaces but leaves a single one), and it surfaced as 'VIP Bed , seat no. 9'.
+    const where = [coach, coachType].filter(Boolean).join(' ');
+    const who = isGroup ? `${str(row?.passengerName) || 'Passenger'}, ` : '';
+    return `${who}${where}, seat no. ${seatNo}`;
+  };
+
+  const legs = [...new Set(rows.map((row) => row?.leg ?? 1))];
+  const trainSeatLines =
+    legs.length > 1
+      ? legs
+          .map((leg) =>
+            [
+              `${legLabel(bookingType, leg)}:`,
+              ...rows.filter((row) => (row?.leg ?? 1) === leg).map(line),
+            ].join('\n'),
+          )
+          .join('\n')
+      : rows.map(line).join('\n');
+
+  return {
+    passengerName: isGroup ? 'Passengers' : (names[0] || 'Passenger'),
+    trainSeatLines,
+  };
+}

apps/edr-passenger-api/src/modules/notifications/notifications.service.ts

@@ -8,6 +8,7 @@ import { EmailClientService } from './email-client.service';
 import { SmsClientService } from './sms-client.service';
 import { CreateTemplateDto, UpdateTemplateDto } from './notifications.dto';
 import { resolveBookingSegment } from '../../common/utils/segment-resolver.utils';
+import { buildSeatSummary } from '../../common/utils/booking-sms.utils';
 
 export type NotificationChannelType = 'EMAIL' | 'SMS' | 'PUSH' | 'IN_APP';
 
@@ -305,7 +306,7 @@ export class NotificationsService {
       where: { id: bookingId },
       include: {
         schedule: { include: { originStation: true, destinationStation: true, train: true, stopTimes: { include: { station: true } } } },
-        seats: { include: { seat: { include: { coach: { include: { coachType: true } } } } } },
+        seats: { include: { seat: { include: { coach: { include: { coachType: true } } } } }, orderBy: { leg: 'asc' } },
       },
     });
 
@@ -367,30 +368,19 @@ export class NotificationsService {
   }
 
   /**
-   * Builds the interpolation context for the `booking.created` template. `trainSeatLines` is a
-   * pre-joined block of one "Train/Seat: …" line per booked seat (multi-passenger bookings get
-   * several lines).
+   * Builds the interpolation context for the `booking.created` template. `passengerName` and
+   * `trainSeatLines` both come from buildSeatSummary — a solo booking is greeted by name with
+   * bare "coach, seat no." lines, while a group is greeted as "Passengers" and each line names
+   * its own occupant (one SMS goes to Booking.contactPhone for the whole party).
    */
   private buildBookingCreatedContext(booking: any, ref: string): Record<string, unknown> {
     const s = booking?.schedule ?? {};
-    const trainName = s.train?.name ?? s.train?.number ?? '';
     const fmtDate = (d: any) =>
       d ? new Date(d).toLocaleDateString('en-US', { month: 'short', day: '2-digit', year: 'numeric' }) : 'TBD';
     const fmtTime = (d: any) =>
       d ? new Date(d).toLocaleTimeString('en-US', { hour: '2-digit', minute: '2-digit', hour12: true }) : 'TBD';
 
-    const seats = booking?.seats ?? [];
-    const trainSeatLines = seats
-      .map((bs: any) => {
-        const coach = bs.seat?.coach?.number ?? '-';
-        const cls = bs.seat?.coach?.coachType?.name ?? '';
-        const seatNo = bs.seat?.seatNumber ?? '-';
-        return `Train/Seat: Train ${trainName}, ${coach} ${cls}, seat no. ${seatNo}`.replace(/ +/g, ' ').trim();
-      })
-      .join('\n');
-
-    // Lead passenger (leg-1 seat). Booking has no contactName; the traveller name lives on the seat.
-    const passengerName = seats[0]?.passengerName ?? 'Passenger';
+    const { passengerName, trainSeatLines } = buildSeatSummary(booking?.seats ?? [], booking?.bookingType);
     const payLink = `${process.env.PORTAL_URL ?? 'http://localhost:5174'}/booking/detail?ref=${ref}`;
     const segment = resolveBookingSegment(s, booking?.originStationId, booking?.destinationStationId);