From 12c5b788300fc62295439abc87a6d80fc0e538f1 Mon Sep 17 00:00:00 2001 From: Charles Pizzato <311327716+modernitconsultants@users.noreply.github.com> Date: Thu, 13 Aug 2026 10:49:33 +1000 Subject: [PATCH 1/2] fix(rooms): allow moving a room to another room type, same-property checked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UpdateRoomDto omits roomTypeId entirely, so PATCH /rooms/:id cannot re-categorise a room — there is no API path at all for a legitimate admin operation (splitting a shared type into per-room types, re-tiering inventory). Re-added as an optional update field WITH the FK-ownership check the codebase already applies to inbound-reservation mappings: the target type must belong to the same property, or 404 — an update can never re-point a room at another tenant's type. --- apps/api/src/modules/room/dto/update-room.dto.ts | 16 +++++++++++++++- apps/api/src/modules/room/room.service.ts | 12 ++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/apps/api/src/modules/room/dto/update-room.dto.ts b/apps/api/src/modules/room/dto/update-room.dto.ts index f3c4873d..a8454fa0 100644 --- a/apps/api/src/modules/room/dto/update-room.dto.ts +++ b/apps/api/src/modules/room/dto/update-room.dto.ts @@ -1,6 +1,20 @@ import { PartialType, OmitType } from '@nestjs/swagger'; +import { ApiPropertyOptional } from '@nestjs/swagger'; +import { IsOptional, IsUUID } from 'class-validator'; import { CreateRoomDto } from './create-room.dto'; export class UpdateRoomDto extends PartialType( OmitType(CreateRoomDto, ['propertyId', 'roomTypeId'] as const), -) {} +) { + /** + * Moving a room to another room type is a legitimate admin operation + * (re-categorising inventory; splitting a shared type into per-room types). + * Re-added deliberately after being omitted: the service verifies the target + * type belongs to the same property before writing, so this cannot re-point + * a room at another tenant's type. + */ + @ApiPropertyOptional({ description: 'Move the room to another room type (same property)' }) + @IsOptional() + @IsUUID() + roomTypeId?: string; +} diff --git a/apps/api/src/modules/room/room.service.ts b/apps/api/src/modules/room/room.service.ts index 7a8ebb53..0e95fda3 100644 --- a/apps/api/src/modules/room/room.service.ts +++ b/apps/api/src/modules/room/room.service.ts @@ -83,6 +83,18 @@ export class RoomService { } async updateRoom(id: string, propertyId: string, dto: UpdateRoomDto) { + // FK ownership: a roomTypeId in the update is client-supplied — verify it + // belongs to THIS property before writing, or an update could re-point a + // room at another tenant's type (same rule as inbound-reservation mapping). + if (dto.roomTypeId) { + const [type] = await this.db + .select({ id: roomTypes.id }) + .from(roomTypes) + .where(and(eq(roomTypes.id, dto.roomTypeId), eq(roomTypes.propertyId, propertyId))); + if (!type) { + throw new NotFoundException(`Room type ${dto.roomTypeId} not found for this property`); + } + } const [room] = await this.db .update(rooms) .set({ ...dto, updatedAt: new Date() }) From 8c868beeb6bb5dfa54dad37ca359bfee503e35ce Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 13 Aug 2026 05:06:24 +0000 Subject: [PATCH 2/2] =?UTF-8?q?fix(rooms):=20harden=20room-type=20move=20?= =?UTF-8?q?=E2=80=94=20block=20in-stay=20retype=20+=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up on the roomTypeId PATCH unlock: reject moves while the room is occupied or linked to an assigned/in-house reservation (avoids desyncing reservation.roomTypeId), and cover FK ownership + guards in unit tests. Co-authored-by: Charles Pizzato <311327716+modernitconsultants@users.noreply.github.com> --- README.md | 8 +- .../modules/room/room-fk-ownership.spec.ts | 126 +++++++++++++++++- apps/api/src/modules/room/room.service.ts | 42 +++++- docs/test-stats.json | 4 +- 4 files changed, 169 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 3332e520..dc3f1375 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ NestJS PostgreSQL Apache 2.0 License -1504 Tests Passing 12 AI Agents +1509 Tests Passing 12 AI Agents

@@ -510,7 +510,7 @@ Operator notes for activating existing adapters, metasearch landings on the dire | OTA Channels | Booking.com + Expedia (EQC) + SiteMinder + DerbySoft | Direct + aggregated OTA connectivity (ARI + content) | | XML Processing | fast-xml-parser | Booking.com OTA XML protocol | | Package Manager | pnpm workspaces | Monorepo management | -| Testing | Vitest (1504 tests across 214 test files) | Unit and integration tests || Build | tsup (packages) + Vite (dashboard) + nest build (API) | Fast builds | +| Testing | Vitest (1509 tests across 214 test files) | Unit and integration tests || Build | tsup (packages) + Vite (dashboard) + nest build (API) | Fast builds | | Containers | Docker + docker-compose | Local dev and production deployment | | CI/CD | GitHub Actions | Automated testing, builds, and releases | @@ -642,7 +642,7 @@ Before going live, verify the items in [`docs/deployment.md`](./docs/deployment. ### Run tests ```bash -# All tests (1504 tests across 214 test files) +# All tests (1509 tests across 214 test files) # API tests only pnpm --filter @telivityhaip/api test @@ -1188,7 +1188,7 @@ HAIP is built in public and contributions are welcome. pnpm install # Install dependencies pnpm build # Build all workspace packages pnpm dev # Start API in dev mode (hot reload) -pnpm test # Run all tests (1504 tests, 214 files) +pnpm test # Run all tests (1509 tests, 214 files) pnpm lint # ESLint ``` diff --git a/apps/api/src/modules/room/room-fk-ownership.spec.ts b/apps/api/src/modules/room/room-fk-ownership.spec.ts index 78ae01bd..adff80f2 100644 --- a/apps/api/src/modules/room/room-fk-ownership.spec.ts +++ b/apps/api/src/modules/room/room-fk-ownership.spec.ts @@ -1,5 +1,5 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { BadRequestException } from '@nestjs/common'; +import { describe, it, expect, vi } from 'vitest'; +import { BadRequestException, NotFoundException } from '@nestjs/common'; import { Test } from '@nestjs/testing'; import { RoomService } from './room.service'; import { DRIZZLE } from '../../database/database.module'; @@ -11,6 +11,26 @@ import { DRIZZLE } from '../../database/database.module'; * (cross-tenant link the DB FK alone does not block). */ const A = 'aaaaaaaa-0000-4000-a000-000000000001'; +const ROOM_ID = 'bbbbbbbb-0000-4000-a000-000000000001'; +const RT_OLD = 'cccccccc-0000-4000-a000-000000000001'; +const RT_NEW = 'dddddddd-0000-4000-a000-000000000001'; + +function selectQueue(results: unknown[][]) { + let i = 0; + return vi.fn().mockImplementation(() => ({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockImplementation(() => { + const rows = results[i++] ?? []; + const thenable = Promise.resolve(rows); + return { + limit: vi.fn().mockResolvedValue(rows), + then: thenable.then.bind(thenable), + catch: thenable.catch.bind(thenable), + }; + }), + }), + })); +} describe('RoomService — createRoom cross-tenant FK ownership (audit #5)', () => { it('rejects when dto.roomTypeId belongs to another property', async () => { @@ -55,3 +75,105 @@ describe('RoomService — createRoom cross-tenant FK ownership (audit #5)', () = expect(db.insert).toHaveBeenCalled(); }); }); + +describe('RoomService — updateRoom roomTypeId move', () => { + const vacantRoom = { + id: ROOM_ID, + propertyId: A, + roomTypeId: RT_OLD, + number: '101', + status: 'vacant_clean', + }; + + it('rejects cross-tenant target room type (404)', async () => { + const db: any = { + select: selectQueue([[vacantRoom], []]), + update: vi.fn(), + }; + const mod = await Test.createTestingModule({ + providers: [RoomService, { provide: DRIZZLE, useValue: db }], + }).compile(); + const svc = mod.get(RoomService); + + await expect( + svc.updateRoom(ROOM_ID, A, { roomTypeId: 'foreign-rt' } as any), + ).rejects.toBeInstanceOf(NotFoundException); + expect(db.update).not.toHaveBeenCalled(); + }); + + it('rejects move while room status is occupied', async () => { + const occupied = { ...vacantRoom, status: 'occupied' }; + const db: any = { + select: selectQueue([[occupied], [{ id: RT_NEW }]]), + update: vi.fn(), + }; + const mod = await Test.createTestingModule({ + providers: [RoomService, { provide: DRIZZLE, useValue: db }], + }).compile(); + const svc = mod.get(RoomService); + + await expect( + svc.updateRoom(ROOM_ID, A, { roomTypeId: RT_NEW } as any), + ).rejects.toBeInstanceOf(BadRequestException); + expect(db.update).not.toHaveBeenCalled(); + }); + + it('rejects move while an assigned/in-house reservation links the room', async () => { + const db: any = { + select: selectQueue([[vacantRoom], [{ id: RT_NEW }], [{ id: 'res-1' }]]), + update: vi.fn(), + }; + const mod = await Test.createTestingModule({ + providers: [RoomService, { provide: DRIZZLE, useValue: db }], + }).compile(); + const svc = mod.get(RoomService); + + await expect( + svc.updateRoom(ROOM_ID, A, { roomTypeId: RT_NEW } as any), + ).rejects.toBeInstanceOf(BadRequestException); + expect(db.update).not.toHaveBeenCalled(); + }); + + it('allows same-property move when vacant and unlinked', async () => { + const updated = { ...vacantRoom, roomTypeId: RT_NEW }; + const db: any = { + select: selectQueue([[vacantRoom], [{ id: RT_NEW }], []]), + update: vi.fn().mockReturnValue({ + set: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + returning: vi.fn().mockResolvedValue([updated]), + }), + }), + }), + }; + const mod = await Test.createTestingModule({ + providers: [RoomService, { provide: DRIZZLE, useValue: db }], + }).compile(); + const svc = mod.get(RoomService); + + const out = await svc.updateRoom(ROOM_ID, A, { roomTypeId: RT_NEW } as any); + expect(out.roomTypeId).toBe(RT_NEW); + expect(db.update).toHaveBeenCalled(); + }); + + it('skips move guards when roomTypeId is unchanged', async () => { + const db: any = { + select: selectQueue([[vacantRoom]]), + update: vi.fn().mockReturnValue({ + set: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + returning: vi.fn().mockResolvedValue([{ ...vacantRoom, floor: '2' }]), + }), + }), + }), + }; + const mod = await Test.createTestingModule({ + providers: [RoomService, { provide: DRIZZLE, useValue: db }], + }).compile(); + const svc = mod.get(RoomService); + + const out = await svc.updateRoom(ROOM_ID, A, { roomTypeId: RT_OLD, floor: '2' } as any); + expect(out.floor).toBe('2'); + expect(db.update).toHaveBeenCalled(); + }); +}); diff --git a/apps/api/src/modules/room/room.service.ts b/apps/api/src/modules/room/room.service.ts index 0e95fda3..f15cb411 100644 --- a/apps/api/src/modules/room/room.service.ts +++ b/apps/api/src/modules/room/room.service.ts @@ -1,11 +1,19 @@ import { Injectable, Inject, NotFoundException, BadRequestException } from '@nestjs/common'; -import { eq, and } from 'drizzle-orm'; -import { rooms, roomTypes } from '@telivityhaip/database'; +import { eq, and, inArray } from 'drizzle-orm'; +import { rooms, roomTypes, reservations } from '@telivityhaip/database'; import { DRIZZLE } from '../../database/database.module'; import { CreateRoomTypeDto } from './dto/create-room-type.dto'; import { CreateRoomDto } from './dto/create-room.dto'; import { UpdateRoomDto } from './dto/update-room.dto'; +/** Stay statuses that pin a physical room — retyping would desync reservation.roomTypeId. */ +const ROOM_TYPE_MOVE_BLOCKING_RESERVATION_STATUSES = [ + 'assigned', + 'checked_in', + 'stayover', + 'due_out', +] as const; + @Injectable() export class RoomService { constructor(@Inject(DRIZZLE) private readonly db: any) {} @@ -83,10 +91,12 @@ export class RoomService { } async updateRoom(id: string, propertyId: string, dto: UpdateRoomDto) { + const existing = await this.findRoomById(id, propertyId); + // FK ownership: a roomTypeId in the update is client-supplied — verify it // belongs to THIS property before writing, or an update could re-point a // room at another tenant's type (same rule as inbound-reservation mapping). - if (dto.roomTypeId) { + if (dto.roomTypeId && dto.roomTypeId !== existing.roomTypeId) { const [type] = await this.db .select({ id: roomTypes.id }) .from(roomTypes) @@ -94,7 +104,33 @@ export class RoomService { if (!type) { throw new NotFoundException(`Room type ${dto.roomTypeId} not found for this property`); } + + // Occupied / in-stay rooms must not change type — assign/check-in enforce + // room.roomTypeId === reservation.roomTypeId; a silent retype breaks that. + if (existing.status === 'occupied') { + throw new BadRequestException( + `Cannot move room ${existing.number} to another type while status is occupied`, + ); + } + + const [linked] = await this.db + .select({ id: reservations.id }) + .from(reservations) + .where( + and( + eq(reservations.propertyId, propertyId), + eq(reservations.roomId, id), + inArray(reservations.status, [...ROOM_TYPE_MOVE_BLOCKING_RESERVATION_STATUSES]), + ), + ) + .limit(1); + if (linked) { + throw new BadRequestException( + `Cannot move room ${existing.number} to another type while it is linked to an active stay (${linked.id})`, + ); + } } + const [room] = await this.db .update(rooms) .set({ ...dto, updatedAt: new Date() }) diff --git a/docs/test-stats.json b/docs/test-stats.json index b19a9dc8..05b277d9 100644 --- a/docs/test-stats.json +++ b/docs/test-stats.json @@ -1,5 +1,5 @@ { - "tests": 1504, + "tests": 1509, "files": 214, - "updatedAt": "2026-08-13T04:06:48.382Z" + "updatedAt": "2026-08-13T05:06:18.343Z" }