diff --git a/client/src/components/Planner/TravelerChips.test.tsx b/client/src/components/Planner/TravelerChips.test.tsx new file mode 100644 index 000000000..d983ad005 --- /dev/null +++ b/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(); + // Each assigned traveler carries an avatar_url, so one 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(); + // 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(); + 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(); + 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( + + ); + expect(container).toBeEmptyDOMElement(); + }); + + it('FE-COMP-TRAVELERS-006: read-only with travelers renders chips but hides the picker button', () => { + render(); + expect(document.querySelectorAll('img')).toHaveLength(2); + expect(screen.queryByRole('button', { name: 'Assign travelers' })).not.toBeInTheDocument(); + }); +}); diff --git a/client/src/store/slices/reservationsSlice.test.ts b/client/src/store/slices/reservationsSlice.test.ts new file mode 100644 index 000000000..4a9dfb588 --- /dev/null +++ b/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([]); + }); +}); diff --git a/server/tests/unit/nest/reservations.controller.test.ts b/server/tests/unit/nest/reservations.controller.test.ts index 9713dae59..e24ea881d 100644 --- a/server/tests/unit/nest/reservations.controller.test.ts +++ b/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); + 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); + 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); diff --git a/server/tests/unit/nest/reservations.service.test.ts b/server/tests/unit/nest/reservations.service.test.ts index 9e2f12616..d0ff8277c 100644 --- a/server/tests/unit/nest/reservations.service.test.ts +++ b/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'); diff --git a/server/tests/unit/services/reservationTravelers.test.ts b/server/tests/unit/services/reservationTravelers.test.ts new file mode 100644 index 000000000..abd1867b3 --- /dev/null +++ b/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([]); + }); +});