test(reservations): server + client coverage for traveler assignment

client/src/components/Planner/TravelerChips.test.tsx

@@ -0,0 +1,66 @@
+// FE-COMP-TRAVELERS-001 to FE-COMP-TRAVELERS-006
+import { render, screen } from '../../../tests/helpers/render';
+import userEvent from '@testing-library/user-event';
+import type { ReservationTraveler } from '@trek/shared';
+import type { TripMember } from '../Budget/BudgetPanelMemberChips';
+import TravelerChips from './TravelerChips';
+
+const tripMembers: TripMember[] = [
+  { id: 1, username: 'alice', avatar_url: null },
+  { id: 2, username: 'bob', avatar_url: null },
+];
+
+const travelers: ReservationTraveler[] = [
+  { user_id: 1, username: 'alice', avatar_url: '/uploads/avatars/alice.jpg' },
+  { user_id: 2, username: 'bob', avatar_url: '/uploads/avatars/bob.jpg' },
+];
+
+describe('TravelerChips', () => {
+  it('FE-COMP-TRAVELERS-001: renders an avatar chip per traveler', () => {
+    render(<TravelerChips travelers={travelers} tripMembers={tripMembers} onSetTravelers={vi.fn()} />);
+    // Each assigned traveler carries an avatar_url, so one <img> renders per chip.
+    expect(document.querySelectorAll('img')).toHaveLength(2);
+  });
+
+  it('FE-COMP-TRAVELERS-002: picker button opens the dropdown listing trip members', async () => {
+    const user = userEvent.setup();
+    render(<TravelerChips travelers={[]} tripMembers={tripMembers} onSetTravelers={vi.fn()} />);
+    // Dropdown starts closed — members are not in the DOM yet.
+    expect(screen.queryByText('alice')).not.toBeInTheDocument();
+    // reservations.travelers.assign = "Assign travelers"
+    await user.click(screen.getByRole('button', { name: 'Assign travelers' }));
+    expect(screen.getByText('alice')).toBeInTheDocument();
+    expect(screen.getByText('bob')).toBeInTheDocument();
+  });
+
+  it('FE-COMP-TRAVELERS-003: toggling a member on calls onSetTravelers with the added id', async () => {
+    const user = userEvent.setup();
+    const onSetTravelers = vi.fn();
+    render(<TravelerChips travelers={[]} tripMembers={tripMembers} onSetTravelers={onSetTravelers} />);
+    await user.click(screen.getByRole('button', { name: 'Assign travelers' }));
+    await user.click(screen.getByText('alice'));
+    expect(onSetTravelers).toHaveBeenCalledWith([1]);
+  });
+
+  it('FE-COMP-TRAVELERS-004: toggling an assigned member off calls onSetTravelers without their id', async () => {
+    const user = userEvent.setup();
+    const onSetTravelers = vi.fn();
+    render(<TravelerChips travelers={[travelers[0]]} tripMembers={tripMembers} onSetTravelers={onSetTravelers} />);
+    await user.click(screen.getByRole('button', { name: 'Assign travelers' }));
+    await user.click(screen.getByText('alice'));
+    expect(onSetTravelers).toHaveBeenCalledWith([]);
+  });
+
+  it('FE-COMP-TRAVELERS-005: read-only with no travelers renders nothing', () => {
+    const { container } = render(
+      <TravelerChips travelers={[]} tripMembers={tripMembers} onSetTravelers={vi.fn()} readOnly />
+    );
+    expect(container).toBeEmptyDOMElement();
+  });
+
+  it('FE-COMP-TRAVELERS-006: read-only with travelers renders chips but hides the picker button', () => {
+    render(<TravelerChips travelers={travelers} tripMembers={tripMembers} onSetTravelers={vi.fn()} readOnly />);
+    expect(document.querySelectorAll('img')).toHaveLength(2);
+    expect(screen.queryByRole('button', { name: 'Assign travelers' })).not.toBeInTheDocument();
+  });
+});

client/src/store/slices/reservationsSlice.test.ts

@@ -0,0 +1,56 @@
+// FE-STORE-RESERVATIONS-001 to FE-STORE-RESERVATIONS-003
+import { http, HttpResponse } from 'msw';
+import { server } from '../../../tests/helpers/msw/server';
+import { resetAllStores, seedStore } from '../../../tests/helpers/store';
+import { buildReservation } from '../../../tests/helpers/factories';
+import { useTripStore } from '../tripStore';
+
+beforeEach(() => {
+  resetAllStores();
+  server.resetHandlers();
+});
+
+describe('reservationsSlice', () => {
+  it('FE-STORE-RESERVATIONS-001: setReservationTravelers patches travelers on matching reservation', async () => {
+    const reservation = buildReservation({ id: 5, trip_id: 1, travelers: [] });
+    seedStore(useTripStore, { reservations: [reservation] });
+
+    const travelers = [{ user_id: 1, username: 'alice', avatar_url: null, avatar: null, is_guest: 0 }];
+    server.use(
+      http.put('/api/trips/1/reservations/5/travelers', () =>
+        HttpResponse.json({ travelers })
+      )
+    );
+    await useTripStore.getState().setReservationTravelers(1, 5, [1]);
+    const stored = useTripStore.getState().reservations.find(r => r.id === 5);
+    expect(stored?.travelers).toHaveLength(1);
+    expect(stored?.travelers?.[0].user_id).toBe(1);
+  });
+
+  it('FE-STORE-RESERVATIONS-002: setReservationTravelers throws on API error', async () => {
+    const reservation = buildReservation({ id: 6, trip_id: 1 });
+    seedStore(useTripStore, { reservations: [reservation] });
+
+    server.use(
+      http.put('/api/trips/1/reservations/6/travelers', () =>
+        HttpResponse.json({ error: 'forbidden' }, { status: 403 })
+      )
+    );
+    await expect(useTripStore.getState().setReservationTravelers(1, 6, [1])).rejects.toThrow();
+  });
+
+  it('FE-STORE-RESERVATIONS-003: setReservationTravelers leaves other reservations untouched', async () => {
+    const a = buildReservation({ id: 7, trip_id: 1, travelers: [] });
+    const b = buildReservation({ id: 8, trip_id: 1, travelers: [] });
+    seedStore(useTripStore, { reservations: [a, b] });
+
+    const travelers = [{ user_id: 2, username: 'bob', avatar_url: null }];
+    server.use(
+      http.put('/api/trips/1/reservations/7/travelers', () =>
+        HttpResponse.json({ travelers })
+      )
+    );
+    await useTripStore.getState().setReservationTravelers(1, 7, [2]);
+    expect(useTripStore.getState().reservations.find(r => r.id === 8)?.travelers).toEqual([]);
+  });
+});

server/tests/unit/nest/reservations.controller.test.ts

@@ -95,6 +95,28 @@ describe('ReservationsController (parity with the legacy /api/trips/:tripId/rese
     });
   });
 
+  describe('PUT /:id/travelers', () => {
+    it('400 when user_ids is not an array', () => {
+      expect(thrown(() => new ReservationsController(makeService()).updateTravelers(user, '5', '9', 'nope'))).toEqual({ status: 400, body: { error: 'user_ids must be an array' } });
+    });
+
+    it('404 when the reservation is off-trip / missing', () => {
+      const svc = makeService({ setTravelers: vi.fn().mockReturnValue(null) } as Partial<ReservationsService>);
+      expect(thrown(() => new ReservationsController(svc).updateTravelers(user, '5', '9', [1]))).toEqual({ status: 404, body: { error: 'Reservation not found' } });
+    });
+
+    it('assigns travelers, broadcasts, and returns { travelers, reservation }', () => {
+      const travelers = [{ user_id: 2, username: 'Sam', avatar: null, is_guest: 0 }];
+      const reservation = { id: 9, travelers };
+      const setTravelers = vi.fn().mockReturnValue({ travelers, reservation });
+      const broadcast = vi.fn();
+      const svc = makeService({ setTravelers, broadcast } as Partial<ReservationsService>);
+      expect(new ReservationsController(svc).updateTravelers(user, '5', '9', [2], 'sock')).toEqual({ travelers, reservation });
+      expect(setTravelers).toHaveBeenCalledWith('9', '5', [2]);
+      expect(broadcast).toHaveBeenCalledWith('5', 'reservation:travelers-updated', { reservationId: 9, travelers }, 'sock');
+    });
+  });
+
   describe('DELETE /:id', () => {
     it('404 when nothing deleted', () => {
       const svc = makeService({ remove: vi.fn().mockReturnValue({ deleted: undefined, accommodationDeleted: false, deletedBudgetItemId: null }) } as Partial<ReservationsService>);

server/tests/unit/nest/reservations.service.test.ts

@@ -23,6 +23,7 @@ const { resv } = vi.hoisted(() => ({
     verifyTripAccess: vi.fn(), listReservations: vi.fn(), createReservation: vi.fn(), updatePositions: vi.fn(),
     getReservation: vi.fn(), updateReservation: vi.fn(), deleteReservation: vi.fn(),
     notifyBookingChange: vi.fn(),
+    setReservationTravelers: vi.fn(), loadTravelers: vi.fn(), getReservationWithJoins: vi.fn(),
   },
 }));
 vi.mock('../../../src/services/reservationService', () => resv);
@@ -55,6 +56,24 @@ describe('ReservationsService', () => {
     expect(resv.deleteReservation).toHaveBeenCalledWith('9', '5');
   });
 
+  describe('setTravelers', () => {
+    it('returns null when the reservation is not on the trip (off-trip guard)', () => {
+      resv.getReservation.mockReturnValue(undefined);
+      expect(svc().setTravelers('9', '5', [2])).toBeNull();
+      expect(resv.setReservationTravelers).not.toHaveBeenCalled();
+    });
+
+    it('assigns travelers and returns the refreshed travelers + reservation', () => {
+      resv.getReservation.mockReturnValue({ id: 9 });
+      const travelers = [{ user_id: 2, username: 'Sam', avatar: null, is_guest: 0 }];
+      resv.loadTravelers.mockReturnValue(travelers);
+      resv.getReservationWithJoins.mockReturnValue({ id: 9, travelers });
+      expect(svc().setTravelers('9', '5', [2])).toEqual({ travelers, reservation: { id: 9, travelers } });
+      expect(resv.setReservationTravelers).toHaveBeenCalledWith('9', '5', [2]);
+      expect(resv.getReservationWithJoins).toHaveBeenCalledWith(9);
+    });
+  });
+
   describe('syncBudgetOnCreate', () => {
     it('does nothing without a positive price', () => {
       svc().syncBudgetOnCreate('5', 9, 'Hotel', 'lodging', undefined, 'sock');

server/tests/unit/services/reservationTravelers.test.ts

@@ -0,0 +1,121 @@
+/**
+ * Unit tests for reservation traveler assignment — RES-TRAV-001..005.
+ * The #1517 backend: assign trip members / named guests to a booking, with a
+ * cross-trip guard so a stale client can't leak a user from another trip.
+ */
+import { describe, it, expect, beforeAll, beforeEach, afterAll, vi } from 'vitest';
+
+const { testDb, dbMock } = vi.hoisted(() => {
+  const Database = require('better-sqlite3');
+  const db = new Database(':memory:');
+  db.exec('PRAGMA foreign_keys = ON');
+  return { testDb: db, dbMock: { db } };
+});
+vi.mock('../../../src/db/database', () => dbMock);
+
+import { createTables } from '../../../src/db/schema';
+import { runMigrations } from '../../../src/db/migrations';
+import { resetTestDb } from '../../helpers/test-db';
+import { createUser, createTrip, createReservation, addTripMember } from '../../helpers/factories';
+import { setReservationTravelers, loadTravelers, listReservations } from '../../../src/services/reservationService';
+
+beforeAll(() => { createTables(testDb); runMigrations(testDb); });
+beforeEach(() => resetTestDb(testDb));
+afterAll(() => testDb.close());
+
+/** Create a named guest user (no credentials) and add it to the trip. */
+function addGuest(tripId: number, displayName: string) {
+  const { user } = createUser(testDb);
+  testDb.prepare('UPDATE users SET is_guest = 1, display_name = ? WHERE id = ?').run(displayName, user.id);
+  addTripMember(testDb, tripId, user.id);
+  return user;
+}
+
+describe('setReservationTravelers / loadTravelers (#1517)', () => {
+  it('RES-TRAV-001: assigns trip members and returns them via loadTravelers', () => {
+    const { user: owner } = createUser(testDb);
+    const { user: member } = createUser(testDb);
+    const trip = createTrip(testDb, owner.id);
+    addTripMember(testDb, trip.id, member.id);
+    const res = createReservation(testDb, trip.id);
+
+    setReservationTravelers(res.id, trip.id, [member.id]);
+
+    const travelers = loadTravelers(res.id);
+    expect(travelers).toHaveLength(1);
+    expect(travelers[0]).toMatchObject({ user_id: member.id, username: member.username, is_guest: 0 });
+  });
+
+  it('RES-TRAV-002: silently drops a user id that is not on the trip (cross-trip guard)', () => {
+    const { user: owner } = createUser(testDb);
+    const { user: member } = createUser(testDb);
+    const trip = createTrip(testDb, owner.id);
+    addTripMember(testDb, trip.id, member.id);
+    const res = createReservation(testDb, trip.id);
+
+    // An outsider who belongs to a different trip only.
+    const { user: outsider } = createUser(testDb);
+    const otherTrip = createTrip(testDb, outsider.id);
+    addTripMember(testDb, otherTrip.id, outsider.id);
+
+    setReservationTravelers(res.id, trip.id, [member.id, outsider.id]);
+
+    const ids = loadTravelers(res.id).map(t => t.user_id);
+    expect(ids).toEqual([member.id]);
+    expect(ids).not.toContain(outsider.id);
+  });
+
+  it('RES-TRAV-003: assigns a named guest (users.is_guest = 1) joined via trip_members', () => {
+    const { user: owner } = createUser(testDb);
+    const trip = createTrip(testDb, owner.id);
+    const guest = addGuest(trip.id, 'Grandma');
+    const res = createReservation(testDb, trip.id);
+
+    setReservationTravelers(res.id, trip.id, [guest.id]);
+
+    const travelers = loadTravelers(res.id);
+    expect(travelers).toHaveLength(1);
+    expect(travelers[0]).toMatchObject({ user_id: guest.id, username: 'Grandma', is_guest: 1 });
+  });
+
+  it('RES-TRAV-004: re-setting replaces the previous set', () => {
+    const { user: owner } = createUser(testDb);
+    const { user: a } = createUser(testDb);
+    const { user: b } = createUser(testDb);
+    const trip = createTrip(testDb, owner.id);
+    addTripMember(testDb, trip.id, a.id);
+    addTripMember(testDb, trip.id, b.id);
+    const res = createReservation(testDb, trip.id);
+
+    setReservationTravelers(res.id, trip.id, [a.id]);
+    expect(loadTravelers(res.id).map(t => t.user_id)).toEqual([a.id]);
+
+    setReservationTravelers(res.id, trip.id, [b.id]);
+    const ids = loadTravelers(res.id).map(t => t.user_id);
+    expect(ids).toEqual([b.id]);
+    expect(ids).not.toContain(a.id);
+
+    // Clearing removes everyone.
+    setReservationTravelers(res.id, trip.id, []);
+    expect(loadTravelers(res.id)).toHaveLength(0);
+  });
+
+  it('RES-TRAV-005: listReservations attaches travelers[] per reservation', () => {
+    const { user: owner } = createUser(testDb);
+    const { user: member } = createUser(testDb);
+    const trip = createTrip(testDb, owner.id);
+    addTripMember(testDb, trip.id, member.id);
+    const withTravelers = createReservation(testDb, trip.id, { title: 'Flight' });
+    const withoutTravelers = createReservation(testDb, trip.id, { title: 'Hotel' });
+
+    setReservationTravelers(withTravelers.id, trip.id, [member.id]);
+
+    const list = listReservations(trip.id);
+    const assigned = list.find(r => r.id === withTravelers.id);
+    const empty = list.find(r => r.id === withoutTravelers.id);
+
+    expect(assigned.travelers).toHaveLength(1);
+    expect(assigned.travelers[0]).toMatchObject({ user_id: member.id, username: member.username });
+    expect(empty.travelers).toEqual([]);
+  });
+});