From 277a28083a42e38736d09d7c0df9d845ad387bf5 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Mon, 3 Aug 2026 18:05:22 +1000 Subject: [PATCH 1/9] test(e2e): assert the PKCE handshake on the wire (AUTH-E2E-01) Closes the last outstanding P0. login.spec.ts proves a login works and that the token is usable; this proves the handshake that produced it was safe, which is a different question. A client that leaked a secret or quietly dropped PKCE would still end up with a valid token and a working app, so nothing in the response tells you the exchange was sound. You have to look at what the browser sent. loginViaUI now records every /auth/* request it observes, and the new spec asserts S256 (never "plain"), a real base64url challenge, a code_verifier, the expected client_id, and no client_secret anywhere. Plus: the password never appears in a URL, and only ever reaches /auth/signin. Red-checked. These assertions previously lived in tasks/PPT-2536/, where nothing ran them. Co-Authored-By: Claude Opus 5 (1M context) --- E2E_USER_STORIES.md | 2 +- apps/workplace/e2e/local/pkce.spec.ts | 94 +++++++++++++++++++++++++++ e2e/support/login.ts | 31 ++++++++- 3 files changed, 124 insertions(+), 3 deletions(-) create mode 100644 apps/workplace/e2e/local/pkce.spec.ts diff --git a/E2E_USER_STORIES.md b/E2E_USER_STORIES.md index f372908bb4..e5a5761abe 100644 --- a/E2E_USER_STORIES.md +++ b/E2E_USER_STORIES.md @@ -133,7 +133,7 @@ environment, data or real-client gap that unit specs could not see. | ID | P | Story | Status | |----|---|-------|--------| -| AUTH-E2E-01 | P0 | Authorization-code + PKCE exchange in a real browser: no `client_secret` anywhere, `S256` challenge, token is a JWT. | **partial** — `login.spec.ts` asserts the exchange and token shape; the explicit no-secret / challenge-recomputation assertions still live in `tasks/PPT-2536/e2e/backoffice-login.spec.js` and should move here. | +| AUTH-E2E-01 | P0 | Authorization-code + PKCE exchange in a real browser: no `client_secret` anywhere, `S256` challenge, token is a JWT. | **done** — `local/pkce.spec.ts`. Asserts on the wire, not the response: a client that leaked a secret or dropped PKCE would still return a valid-looking token, so the response cannot tell you the handshake was sound. Also checks the password never appears in a URL and only ever reaches `/auth/signin`. Ported from `tasks/PPT-2536/e2e/backoffice-login.spec.js`. | | AUTH-E2E-02 | P0 | A refreshed token keeps its scope, is rotated, preserves `sub`, and is still accepted by rest-api. | **done** — `login.spec.ts`. This is the exact 2026-07-23 revert (403 on `/oauth_apps` after refresh). | | AUTH-E2E-03 | P1 | A refresh chain survives N sequential refreshes without degrading scope or access. | todo — covered API-only by `tasks/PPT-2536/integration/` (RF-03); wanted in-browser. | | AUTH-E2E-04 | P1 | A stale/incompatible session cookie from a previous auth implementation does not break sign-in. | todo — verified manually (SC-01); needs automating. | diff --git a/apps/workplace/e2e/local/pkce.spec.ts b/apps/workplace/e2e/local/pkce.spec.ts new file mode 100644 index 0000000000..467a2808f0 --- /dev/null +++ b/apps/workplace/e2e/local/pkce.spec.ts @@ -0,0 +1,94 @@ +/** + * AUTH-E2E-01 — the PKCE handshake, asserted on the wire. + * + * `login.spec.ts` proves a login works and that the resulting token is usable. + * This proves the handshake that produced it was actually safe, which is a + * different question: a client that leaked a secret, or quietly dropped PKCE, + * would still end up with a perfectly valid token and a perfectly working app. + * Nothing about the response tells you the exchange was sound. You have to look + * at what the browser sent. + * + * These assertions previously lived in tasks/PPT-2536/e2e/backoffice-login.spec.js, + * written during the auth.cr migration. They belong with the suite rather than in + * a task folder, where nothing runs them. + */ +import { test, expect } from '../../../../e2e/support/fixtures'; +import { loginViaUI } from '../../../../e2e/support/login'; +import { APP_URL, roleFor } from '../../../../e2e/support/env'; +import { clientId, redirectUriFor } from '../../../../e2e/support/auth'; + +// The subject is the handshake itself, so start with no credentials. +test.use({ storageState: undefined }); + +test.describe('PKCE handshake', () => { + test('the browser performs a public-client PKCE exchange with no secret', async ({ page }) => { + const { requests, token } = await loginViaUI(page, roleFor('admin')); + + const authorize = requests.find((r) => r.url.includes('/oauth/authorize')); + const exchange = requests.find( + (r) => r.url.includes('/oauth/token') && !r.url.includes('refresh_token'), + ); + + expect(authorize, 'an /oauth/authorize request should have been made').toBeTruthy(); + expect(exchange, 'an /oauth/token request should have been made').toBeTruthy(); + + // --- the authorize leg ------------------------------------------------- + // 302 is the redirect back to the app carrying ?code=. A 200 here would + // mean we were served a page instead, i.e. the handshake never completed. + expect(authorize!.status, 'authorize redirects back with a code').toBe(302); + + expect(authorize!.url, 'PKCE must use S256, never "plain"').toContain( + 'code_challenge_method=S256', + ); + + const challenge = new URL(authorize!.url).searchParams.get('code_challenge') ?? ''; + expect(challenge.length, 'a real code_challenge was sent').toBeGreaterThan(20); + // base64url, so it must not contain the standard-base64-only characters. + expect(challenge, 'the challenge is base64url encoded').not.toMatch(/[+/=]/); + + // --- the token leg ----------------------------------------------------- + expect(exchange!.status, 'token exchange succeeds').toBe(200); + + // The whole point of a public client: the browser holds no secret, so it + // must never send one. If this ever fails, a credential is sitting in a + // shipped bundle where anyone can read it. + const wire = `${exchange!.url} ${exchange!.postData ?? ''}`; + expect(wire, 'no client_secret anywhere in the token request').not.toContain( + 'client_secret', + ); + + // The verifier is what proves this client started the flow. Without it, + // an intercepted code could be redeemed by anyone. + expect(wire, 'the code_verifier was sent').toContain('code_verifier'); + + // And it is the client we registered, not something else. + expect(wire, 'the expected client_id was used').toContain( + clientId(redirectUriFor(APP_URL)), + ); + + expect(token.access_token, 'the exchange produced a token').toBeTruthy(); + }); + + test('no credentials are exposed in any /auth request', async ({ page }) => { + const { requests } = await loginViaUI(page, roleFor('admin')); + const password = roleFor('admin').password; + + // The password legitimately appears once, in the sign-in POST body. It must + // not turn up anywhere else, and above all never in a URL, where it would be + // captured by browser history, proxies and server logs. + for (const r of requests) { + expect(r.url, `password must never appear in a URL (${r.method} ${r.url})`).not.toContain( + password, + ); + expect(r.url, `no client_secret in a URL (${r.method} ${r.url})`).not.toContain( + 'client_secret', + ); + } + + const bodies_with_password = requests.filter((r) => (r.postData ?? '').includes(password)); + expect( + bodies_with_password.every((r) => r.url.includes('/auth/signin')), + 'the password should only ever be sent to /auth/signin', + ).toBeTruthy(); + }); +}); diff --git a/e2e/support/login.ts b/e2e/support/login.ts index e6f3a35bf9..82449ee9d5 100644 --- a/e2e/support/login.ts +++ b/e2e/support/login.ts @@ -17,9 +17,26 @@ import { Page, expect } from '@playwright/test'; import { APP_URL, Role } from './env'; +/** One request the browser made to /auth/*, as observed on the wire. */ +export interface AuthRequest { + url: string; + method: string; + status: number; + postData: string | null; +} + export interface LoginResult { /** The token-endpoint response the browser actually received. */ token: { access_token?: string; refresh_token?: string; scope?: string; expires_in?: number }; + /** + * Every `/auth/*` request the browser made during the login, in order. + * + * Captured so specs can assert on what was actually sent rather than on what + * the SDK claims it sent. The distinction matters: a client that leaks a + * secret, or silently drops PKCE, still returns a perfectly valid-looking + * token, so the response alone cannot tell you the handshake was safe. + */ + requests: AuthRequest[]; } /** @@ -36,8 +53,18 @@ export interface LoginResult { */ export async function loginViaUI(page: Page, role: Role): Promise { let token: LoginResult['token'] = {}; + const requests: AuthRequest[] = []; page.on('response', async (res) => { - if (res.url().includes('/oauth/token') && res.status() === 200) { + const url = res.url(); + if (!url.includes('/auth/')) return; + const req = res.request(); + requests.push({ + url, + method: req.method(), + status: res.status(), + postData: req.postData(), + }); + if (url.includes('/oauth/token') && res.status() === 200) { try { token = await res.json(); } catch { @@ -64,7 +91,7 @@ export async function loginViaUI(page: Page, role: Role): Promise { await page.waitForURL(new RegExp(escapeRe(new URL(APP_URL).host)), { timeout: 30_000 }); await expect(page.locator('topbar')).toBeVisible({ timeout: 30_000 }); - return { token }; + return { token, requests }; } function escapeRe(s: string): string { From 46eb63047d86c331364efb2863f98c696efc296a Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Mon, 3 Aug 2026 18:11:14 +1000 Subject: [PATCH 2/9] test(e2e): one user's bookings are not another user's business Covers WP-E2E-08 and AUTH-E2E-05. A second seeded user cannot see the booking in their listing, and cannot delete it; the owner still can. Includes a control asserting you can see your own booking, otherwise "nobody sees anything" would pass as success. This locks down something we learned the hard way: GET /bookings is scoped to the caller. An early leak check written as an admin reported zero bookings while the database plainly held one. It is both a privacy boundary and a trap for anyone writing tooling, and a regression would leak quietly rather than fail loudly. Red-checked: inverting the assertion shows the other user's listing really is empty while the booking exists. Also corrects two rows in the contract. Lockers and parking do NOT follow the desk metadata pattern as claimed - lockers come from banks then lockers within them, parking needs level zones tagged `parking` plus a separate spaces API. Both are more setup than desks, and the contract now says so rather than implying they are quick wins. Co-Authored-By: Claude Opus 5 (1M context) --- E2E_USER_STORIES.md | 8 +- .../e2e/local/booking-scoping.spec.ts | 173 ++++++++++++++++++ 2 files changed, 177 insertions(+), 4 deletions(-) create mode 100644 apps/workplace/e2e/local/booking-scoping.spec.ts diff --git a/E2E_USER_STORIES.md b/E2E_USER_STORIES.md index e5a5761abe..d2f0b25af7 100644 --- a/E2E_USER_STORIES.md +++ b/E2E_USER_STORIES.md @@ -116,9 +116,9 @@ the PR gate. | WP-E2E-06 | P1 | A deleted booking disappears from the listing (teardown really tears down). | **done** — `local/desk-booking.spec.ts` | | WP-E2E-03 | P1 | Building/level selectors are populated from seeded zones, and changing them re-scopes what is bookable. | todo | | WP-E2E-07 | P1 | "Your bookings" lists the user's own booking; cancelling it moves it out of the upcoming list. | todo | -| WP-E2E-08 | P1 | A booking made by one user is **not** visible in another user's "your bookings" (per-user scoping). | todo — see AUTH-E2E-05 | -| WP-E2E-09 | P1 | Booking a **locker** end to end. Same metadata + per-worker-asset + sweep pattern as desks. | todo | -| WP-E2E-10 | P1 | Booking a **parking** space end to end. | todo | +| WP-E2E-08 | P1 | A booking made by one user is **not** visible in another user's listing, and cannot be deleted by them. | **done** — `local/booking-scoping.spec.ts`. Red-checked: the other user's listing really is empty while the booking exists. | +| WP-E2E-09 | P1 | Booking a **locker** end to end. | todo — **more setup than desks**, not the same pattern. Lockers come from locker *banks* then lockers within them (`loadLockerResources`), so seeding is two-level. Budget accordingly. | +| WP-E2E-10 | P1 | Booking a **parking** space end to end. | todo — **more setup than desks**. Needs a level zone tagged `parking` plus spaces created through the parking API (`queryParkingSpacesForZones`), not Zone metadata. | | WP-E2E-11 | P2 | Inviting a **visitor** end to end. | todo | | WP-E2E-12 | P2 | Directory / colleagues search returns seeded users. | todo | | WP-E2E-13 | P2 | The explore/map view renders for a seeded level and reflects availability. | todo — needs map metadata seeded | @@ -137,7 +137,7 @@ environment, data or real-client gap that unit specs could not see. | AUTH-E2E-02 | P0 | A refreshed token keeps its scope, is rotated, preserves `sub`, and is still accepted by rest-api. | **done** — `login.spec.ts`. This is the exact 2026-07-23 revert (403 on `/oauth_apps` after refresh). | | AUTH-E2E-03 | P1 | A refresh chain survives N sequential refreshes without degrading scope or access. | todo — covered API-only by `tasks/PPT-2536/integration/` (RF-03); wanted in-browser. | | AUTH-E2E-04 | P1 | A stale/incompatible session cookie from a previous auth implementation does not break sign-in. | todo — verified manually (SC-01); needs automating. | -| AUTH-E2E-05 | P1 | A non-admin cannot read or mutate another user's bookings; an admin's own listing does not leak others'. | todo — **and it matters**: `GET /bookings` is caller-scoped, which we only learned by getting a leak check wrong. | +| AUTH-E2E-05 | P1 | A non-admin cannot read or mutate another user's bookings. | **done** — `local/booking-scoping.spec.ts`, same spec as WP-E2E-08. Covers both halves: the listing excludes it, and a delete attempt is rejected. Includes a control asserting you *can* see your own, so "nobody sees anything" can't pass as success. | | AUTH-E2E-06 | P2 | Token expiry mid-session recovers without stranding the SPA. | todo | | AUTH-E2E-07 | P2 | Malformed and hostile `/auth/*` requests return 4xx, never 5xx and never a backtrace. | todo — covered by auth.cr unit specs (SEC-01); browser-level coverage optional. | | AUTH-E2E-08 | P1 | `SameSite` behaviour in a genuine third-party/iframe context. | **blocked** — Playwright Chromium cannot create a true third-party context. Known untested incident class (B.7). | diff --git a/apps/workplace/e2e/local/booking-scoping.spec.ts b/apps/workplace/e2e/local/booking-scoping.spec.ts new file mode 100644 index 0000000000..6d3256138e --- /dev/null +++ b/apps/workplace/e2e/local/booking-scoping.spec.ts @@ -0,0 +1,173 @@ +/** + * WP-E2E-08 / AUTH-E2E-05 — one user's bookings are not another user's business. + * + * This locks down a property we learned about the hard way. `GET /bookings` is + * scoped to the caller, which is easy to assume but was not obvious: an early + * leak-check written as an admin reported zero bookings while the database + * plainly held one, because the admin was only ever being shown their own. + * + * That behaviour is load-bearing in two directions. It is a privacy boundary + * (your colleagues cannot enumerate where you sit), and it is a trap for anyone + * writing tooling against the API. Worth a test either way, because a regression + * here would leak quietly rather than fail loudly. + */ +import { request } from '@playwright/test'; +import { test, expect } from '../../../../e2e/support/fixtures'; +import { BACKEND_URL, WORKERS, roleFor } from '../../../../e2e/support/env'; +import { mintToken } from '../../../../e2e/support/auth'; +import { APP_URL } from '../../../../e2e/support/env'; +import { + STAFF_API, + deleteBooking, + listBookings, + releaseAsset, + uniqueTitle, + zonesWithTag, +} from '../../../../e2e/support/api'; +import type { APIRequestContext } from '@playwright/test'; +import { deskFor } from '../../../../e2e/support/env'; + + +/** + * The org/building/level zone ids a booking carries, matching what the UI sends. + * Always queried by tag: `GET /zones` with no `tags` parameter comes back empty. + */ +async function bookingZones(api: APIRequestContext): Promise { + const groups = await Promise.all( + ['org', 'building', 'level'].map((tag) => zonesWithTag(api, tag)), + ); + return groups.flat().map((z) => z.id); +} + +const DAY = 86_400; +// Window comfortably wider than any booking these specs create. Keep it that +// way: a booking placed exactly on the boundary is not returned by the listing, +// which reads as "scoping is broken" rather than "the window was too tight". +const from = () => Math.floor(Date.now() / 1000) - 3 * DAY; +const to = () => Math.floor(Date.now() / 1000) + 3 * DAY; + +test.describe('booking visibility between users', () => { + test('another user cannot see or delete your booking', async ({ staffApi }, testInfo) => { + const mine = testInfo.parallelIndex; + // A genuinely different seeded user. With one worker there is nobody else + // to compare against, so the spec would be meaningless. + const theirs = (mine + 1) % WORKERS; + test.skip(theirs === mine, 'needs at least two workers to have two distinct users'); + + const desk = deskFor(mine); + const title = uniqueTitle('E2E Scoping'); + await releaseAsset(staffApi, 'desk', desk.id, from(), to()); + + // Create as *this* worker's user, through the API rather than the UI — + // the subject here is authorisation, not the booking form. + const me = await (await staffApi.get('/api/engine/v2/users/current')).json(); + const zones = await bookingZones(staffApi); + const start = Math.floor(Date.now() / 1000) + DAY; + + const created = await staffApi.post(`${STAFF_API}/bookings`, { + data: { + booking_type: 'desk', + asset_id: desk.id, + booking_start: start, + booking_end: start + 3600, + timezone: 'Etc/UTC', + user_email: me.email, + user_id: me.id, + user_name: me.name, + title, + zones, + }, + }); + expect(created.ok(), `creating the booking should succeed: ${created.status()}`).toBeTruthy(); + const booking = await created.json(); + + // A second, genuinely different user. + const other_role = roleFor('staff', theirs); + const other_mint = await mintToken( + BACKEND_URL, + APP_URL, + other_role.email, + other_role.password, + ); + const other = await request.newContext({ + baseURL: BACKEND_URL, + ignoreHTTPSErrors: true, + extraHTTPHeaders: { Authorization: `Bearer ${other_mint.accessToken}` }, + }); + + try { + // Sanity: the two identities really are different, or everything below + // would pass for the wrong reason. + const them = await (await other.get('/api/engine/v2/users/current')).json(); + expect(them.email, 'the second user must be a different person').not.toBe(me.email); + + // The privacy boundary. + const their_view = await listBookings(other, 'desk', from(), to()); + expect( + their_view.map((b) => b.id), + "another user's default listing must not include your booking", + ).not.toContain(booking.id); + + // And they cannot remove it. A 2xx here would mean anyone can cancel + // anyone's desk, which is worse than merely being able to see it. + const their_delete = await other.delete(`${STAFF_API}/bookings/${booking.id}`); + expect( + their_delete.status(), + `another user must not be able to delete your booking ` + + `(got ${their_delete.status()})`, + ).toBeGreaterThanOrEqual(400); + + // Still there afterwards, from the owner's point of view. + const still_mine = await listBookings(staffApi, 'desk', from(), to()); + expect( + still_mine.map((b) => b.id), + 'the booking should survive the other user attempting to delete it', + ).toContain(booking.id); + } finally { + await other.dispose(); + await deleteBooking(staffApi, booking.id); + } + }); + + test('you can see your own booking in the listing', async ({ staffApi }, testInfo) => { + // The control for the test above. Without it, "they cannot see it" would + // also pass if nobody could see anything. + const desk = deskFor(testInfo.parallelIndex); + const title = uniqueTitle('E2E Scoping Control'); + await releaseAsset(staffApi, 'desk', desk.id, from(), to()); + + const me = await (await staffApi.get('/api/engine/v2/users/current')).json(); + const zones = await bookingZones(staffApi); + const start = Math.floor(Date.now() / 1000) + DAY; + + const created = await staffApi.post(`${STAFF_API}/bookings`, { + data: { + booking_type: 'desk', + asset_id: desk.id, + booking_start: start, + booking_end: start + 3600, + timezone: 'Etc/UTC', + user_email: me.email, + user_id: me.id, + user_name: me.name, + title, + zones, + }, + }); + expect( + created.ok(), + `creating the booking should succeed: ${created.status()} ${await created.text()}`, + ).toBeTruthy(); + const booking = await created.json(); + + try { + const mine = await listBookings(staffApi, 'desk', from(), to()); + expect( + mine.map((b) => b.id), + 'you must be able to see your own booking', + ).toContain(booking.id); + } finally { + await deleteBooking(staffApi, booking.id); + } + }); +}); From b4a1f5acf46d2c90ddd0952b2b01ab305240a5b4 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Mon, 3 Aug 2026 18:17:11 +1000 Subject: [PATCH 3/9] test(e2e): two people cannot hold the same desk (REG-02) Maps to a shipped fix, "Fix rejecting overlapping bookings on desk assignment" (2607.1). Double-booking is the kind of regression that doesn't announce itself: nothing errors, nobody notices, and two people turn up to the same desk on Tuesday. Attempted as a SECOND user deliberately, because that's the real scenario and because a clash check that only consulted your own bookings would still pass a single-user version of this. Covers the identical slot and a partial overlap, which is the case a naive check misses. Two controls, so the test can't pass for the wrong reason: a genuinely non-overlapping slot must still be accepted (otherwise a backend that rejected everything would look correct), and the desk must free up once the booking is deleted (a cancelled booking that still blocks the desk is harder to diagnose than a plain double-booking). Red-checked: the API really does return 409. Co-Authored-By: Claude Opus 5 (1M context) --- E2E_USER_STORIES.md | 2 +- apps/workplace/e2e/local/desk-clash.spec.ts | 156 ++++++++++++++++++++ 2 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 apps/workplace/e2e/local/desk-clash.spec.ts diff --git a/E2E_USER_STORIES.md b/E2E_USER_STORIES.md index d2f0b25af7..3d86e12c1f 100644 --- a/E2E_USER_STORIES.md +++ b/E2E_USER_STORIES.md @@ -150,7 +150,7 @@ task that found it, so the row can be traced. | ID | P | Story | Source | Status | |----|---|-------|--------|--------| | REG-01 | P0 | Scope is not lost on token refresh; downstream authorisation still passes. | PPT-2536, 2026-07-23 revert | **done** — AUTH-E2E-02 | -| REG-02 | P1 | An overlapping desk booking is rejected rather than silently accepted. | `2607.1` "Fix rejecting overlapping bookings on desk assignment" | todo | +| REG-02 | P1 | An overlapping desk booking is rejected rather than silently accepted. | `2607.1` "Fix rejecting overlapping bookings on desk assignment" | **done** — `local/desk-clash.spec.ts`. Identical and partially-overlapping slots both refused (409), attempted as a *second* user so a per-user-only check would fail. Includes a control that a non-overlapping slot is accepted, and that the desk frees up after deletion. Red-checked. | | REG-03 | P1 | A clash check uses the **current** `booking_end`, not a stale one. | `2607.1` "Fix stale booking_end being used for clash check" | todo | | REG-04 | P1 | Desk booking status displays correctly in the booking list. | `2606.1` "Fix status display for desk bookings" | todo | | REG-05 | P2 | The authorised-user check has no race on boot (no flash of unauthorised). | `2607.1` "Fix race condition for authorised check" | todo | diff --git a/apps/workplace/e2e/local/desk-clash.spec.ts b/apps/workplace/e2e/local/desk-clash.spec.ts new file mode 100644 index 0000000000..9ae90ef48b --- /dev/null +++ b/apps/workplace/e2e/local/desk-clash.spec.ts @@ -0,0 +1,156 @@ +/** + * REG-02 — two people cannot hold the same desk at the same time. + * + * Maps to a real shipped fix: "Fix rejecting overlapping bookings on desk + * assignment" (release 2607.1). Double-booking is the kind of regression that + * doesn't announce itself. Nothing errors, nobody notices, and two people turn up + * to the same desk on Tuesday. + * + * Deliberately attempted as a SECOND user, because that's the real scenario and + * because a clash check that only looked at your own bookings would still pass a + * single-user version of this test. + */ +import { request } from '@playwright/test'; +import { test, expect } from '../../../../e2e/support/fixtures'; +import { APP_URL, BACKEND_URL, WORKERS, deskFor, roleFor } from '../../../../e2e/support/env'; +import { mintToken } from '../../../../e2e/support/auth'; +import { + STAFF_API, + deleteBooking, + releaseAsset, + uniqueTitle, + zonesWithTag, +} from '../../../../e2e/support/api'; +import type { APIRequestContext } from '@playwright/test'; + +const DAY = 86_400; +const from = () => Math.floor(Date.now() / 1000) - 3 * DAY; +const to = () => Math.floor(Date.now() / 1000) + 3 * DAY; + +async function bookingZones(api: APIRequestContext): Promise { + const groups = await Promise.all( + ['org', 'building', 'level'].map((tag) => zonesWithTag(api, tag)), + ); + return groups.flat().map((z) => z.id); +} + +async function book( + api: APIRequestContext, + asset_id: string, + start: number, + end: number, + zones: string[], +) { + const me = await (await api.get('/api/engine/v2/users/current')).json(); + return api.post(`${STAFF_API}/bookings`, { + data: { + booking_type: 'desk', + asset_id, + booking_start: start, + booking_end: end, + timezone: 'Etc/UTC', + user_email: me.email, + user_id: me.id, + user_name: me.name, + title: uniqueTitle('E2E Clash'), + zones, + }, + }); +} + +test.describe('desk double-booking', () => { + test('a second person cannot book a desk that is already taken', async ({ + staffApi, + }, testInfo) => { + const mine = testInfo.parallelIndex; + const theirs = (mine + 1) % WORKERS; + test.skip(theirs === mine, 'needs at least two workers to have two distinct users'); + + const desk = deskFor(mine); + const zones = await bookingZones(staffApi); + await releaseAsset(staffApi, 'desk', desk.id, from(), to()); + + // A fixed, future window. Not "now", so the test can't be tripped by the + // clock crossing a boundary mid-run. + const start = Math.floor(Date.now() / 1000) + DAY; + const end = start + 3600; + + const first = await book(staffApi, desk.id, start, end, zones); + expect( + first.ok(), + `the first booking should succeed: ${first.status()} ${await first.text()}`, + ).toBeTruthy(); + const booking = await first.json(); + + const other_role = roleFor('staff', theirs); + const other_mint = await mintToken( + BACKEND_URL, + APP_URL, + other_role.email, + other_role.password, + ); + const other = await request.newContext({ + baseURL: BACKEND_URL, + ignoreHTTPSErrors: true, + extraHTTPHeaders: { Authorization: `Bearer ${other_mint.accessToken}` }, + }); + + try { + // Exactly the same slot. + const exact = await book(other, desk.id, start, end, zones); + expect( + exact.status(), + `an identical slot must be refused, got ${exact.status()}`, + ).toBeGreaterThanOrEqual(400); + + // And a partial overlap, which is the case a naive check misses: it + // starts before the existing booking ends. + const partial = await book(other, desk.id, start + 1800, end + 1800, zones); + expect( + partial.status(), + `an overlapping slot must be refused, got ${partial.status()}`, + ).toBeGreaterThanOrEqual(400); + + // Control: a slot that genuinely doesn't overlap is fine. Without this, + // a backend that rejected everything would pass the two checks above. + const clear = await book(other, desk.id, end + 3600, end + 7200, zones); + expect( + clear.ok(), + `a non-overlapping slot should be accepted: ${clear.status()} ${await clear.text()}`, + ).toBeTruthy(); + const clear_booking = await clear.json(); + await deleteBooking(other, clear_booking.id); + } finally { + await other.dispose(); + await deleteBooking(staffApi, booking.id); + } + }); + + test('the desk frees up once the booking is deleted', async ({ staffApi }, testInfo) => { + // Guards a nastier version of the same bug: a cancelled booking that still + // blocks the desk. Users would see it as free and be unable to book it, + // which is harder to diagnose than a straightforward double-booking. + const desk = deskFor(testInfo.parallelIndex); + const zones = await bookingZones(staffApi); + await releaseAsset(staffApi, 'desk', desk.id, from(), to()); + + const start = Math.floor(Date.now() / 1000) + 2 * DAY; + const end = start + 3600; + + const first = await book(staffApi, desk.id, start, end, zones); + expect(first.ok(), `first booking: ${first.status()}`).toBeTruthy(); + const booking = await first.json(); + + const blocked = await book(staffApi, desk.id, start, end, zones); + expect(blocked.status(), 'the slot is taken while the booking exists').toBeGreaterThanOrEqual(400); + + await deleteBooking(staffApi, booking.id); + + const after = await book(staffApi, desk.id, start, end, zones); + expect( + after.ok(), + `the same slot should be bookable again once freed: ${after.status()} ${await after.text()}`, + ).toBeTruthy(); + await deleteBooking(staffApi, (await after.json()).id); + }); +}); From 041ad2ff3358267e36433b7b1d5b101f692a57f4 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Wed, 5 Aug 2026 16:20:35 +1000 Subject: [PATCH 4/9] fix(bookings): keep form input across the reset flows actually run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PPT-2643 again. #478 fixed `newForm`'s deferred re-entry, but that branch is not the one the booking flows take. The current user is restored from the localStorage cache within about 50ms of bootstrap, whereas every flow calls its form lifecycle only after org data lands — `NewDeskFlowComponent.ngOnInit` awaits `waitUntilInitialised()` plus a 300ms settle, then calls `loadForm` and, for a fresh booking, `newForm`. So `currentUserIsLoaded()` is already true, the deferral never fires, and the captured-edits replay never runs. `loadForm` had no capture at all, and it is the first of the two resets. Its `model.set(...)` restores defaults — `all_day` false, a truthy `secondary_resource` that re-checks "Require locker" — over whatever the user typed into a form that has been interactive since first paint. Capture in `loadForm` too, and replay over the loaded booking before `applyDurationSettings` so a restored `all_day` still drives the time-sync window. The capture merges rather than replaces, because `form().reset()` clears the dirty flags the capture reads, so the `newForm` that follows in the same tick would otherwise overwrite a real capture with an empty one. The stash is released on a microtask, which is late enough for that chained reset and early enough that it cannot reach an unrelated form. Two specs, both seen red first, driving the ordinary path with no mocking and no runtime probe neutralised: input entered before initialisation survives `loadForm` + `newForm`, and it is not resurrected in a later form. Fixes PPT-2643 Co-Authored-By: Claude Fable 5 --- libs/bookings/src/lib/booking-form.service.ts | 37 +++++++++++++++- .../src/test/booking-form.service.spec.ts | 44 +++++++++++++++++++ 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/libs/bookings/src/lib/booking-form.service.ts b/libs/bookings/src/lib/booking-form.service.ts index 7861525673..18b27aa541 100644 --- a/libs/bookings/src/lib/booking-form.service.ts +++ b/libs/bookings/src/lib/booking-form.service.ts @@ -352,6 +352,22 @@ export class BookingFormService extends AsyncHandler { return edits; } + /** + * Stash the user's in-progress edits for the reset that is about to run. + * + * Merges rather than replaces. A flow resets twice in a row — `loadForm` + * then `newForm` — and the first `form().reset()` clears the dirty flags + * `_userEditedValues` reads, so a plain assignment would overwrite a real + * capture with an empty one on the second call. + */ + private _captureUserEdits() { + const edits = { + ...(this._pending_user_edits || {}), + ...this._userEditedValues(), + }; + this._pending_user_edits = Object.keys(edits).length ? edits : null; + } + private _syncAssetOptions() { const { date, duration } = untracked(this.model); const next_asset_window = assetWindowKey(date, duration); @@ -821,7 +837,7 @@ export class BookingFormService extends AsyncHandler { // destroyed by the reset below. Capture it on the way back in — // as late as possible, so we take the user's final state. currentUserLoaded().then(() => { - this._pending_user_edits = this._userEditedValues(); + this._captureUserEdits(); this.newForm(type, booking); }); return; @@ -1073,6 +1089,19 @@ export class BookingFormService extends AsyncHandler { currentUserLoaded().then(() => this.loadForm(expected_type)); return; } + // Same hazard as `newForm`, and the one the flows actually hit: the form + // is rendered from first paint, but every flow calls this only after org + // data lands, so the reset below arrives on top of whatever the user has + // already entered. Capture before `form().reset()` clears the dirty + // flags `_userEditedValues` reads. + this._captureUserEdits(); + const user_edits = this._pending_user_edits; + // Flows call `loadForm(type)` and then `newForm(type)` in the same tick + // (desk-flow.component.ts:62 and :65, and the locker/parking + // equivalents). Leave the capture in place so that second reset replays + // it too, and release it at the end of the tick, where it can no longer + // reach an unrelated form. + queueMicrotask(() => (this._pending_user_edits = null)); this._startNetwork(); this._calendar.loadCalendars(); const data = JSON.parse( @@ -1111,6 +1140,12 @@ export class BookingFormService extends AsyncHandler { [null, undefined, ''], ); this._patch(booking_data, { emitEvent: false }); + // Re-apply the user's own edits over the loaded booking, before + // `applyDurationSettings` so a restored `all_day` still drives the + // time-sync window — same ordering as `newForm`. + if (user_edits && Object.keys(user_edits).length) { + this._patch(user_edits, { emitEvent: false }); + } this.applyDurationSettings(); this._form_value.set(this.model()); this._syncAssetOptions(); diff --git a/libs/bookings/src/test/booking-form.service.spec.ts b/libs/bookings/src/test/booking-form.service.spec.ts index d24a803df8..cf35a71ed5 100644 --- a/libs/bookings/src/test/booking-form.service.spec.ts +++ b/libs/bookings/src/test/booking-form.service.spec.ts @@ -2884,6 +2884,50 @@ describe('BookingFormService', () => { expect((savedBookings()[0] as Booking).asset_ids).toEqual(['desk-2']); }); + describe('initialisation after the user has already loaded', () => { + /** + * The case the flows actually hit. Every booking flow renders its form + * on first paint but initialises it late: `NewDeskFlowComponent.ngOnInit` + * awaits org initialisation plus a 300ms settle, then calls `loadForm` + * and — for a fresh booking — `newForm`, back to back. + * + * The current user is restored from the localStorage cache within about + * 50ms of bootstrap, long before org data arrives, so `newForm` never + * takes its deferred branch here. Nothing is mocked and no runtime probe + * is neutralised: this is the ordinary path. + */ + function userEdits(field: string, value: any) { + const node = (spectator.service.form as any)[field](); + node.value.set(value); + node.markAsDirty(); + } + + it('keeps input entered before the flow initialises the form', () => { + userEdits('title', 'Quiet corner desk'); + userEdits('all_day', true); + + // exactly what desk-flow.component.ts does once org data lands + spectator.service.loadForm('desk'); + spectator.service.newForm('desk'); + + expect(spectator.service.model().title).toBe('Quiet corner desk'); + expect(spectator.service.model().all_day).toBe(true); + }); + + it('does not carry those edits into a later unrelated form', () => { + userEdits('title', 'Quiet corner desk'); + spectator.service.loadForm('desk'); + spectator.service.newForm('desk'); + expect(spectator.service.model().title).toBe('Quiet corner desk'); + + // A form opened later must start clean, not inherit the last one. + spectator.service.newForm('desk'); + expect(spectator.service.model().title).not.toBe( + 'Quiet corner desk', + ); + }); + }); + describe('initialisation while the user is still loading', () => { /** * Put the service into the state `newForm` sees on a slow load: no From 0d18d46419298d8dfabe24025a65dc14a6a2af82 Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Wed, 5 Aug 2026 19:15:46 +1000 Subject: [PATCH 5/9] fix(events): restore attendee-only notification option (PPT-2514) --- libs/common/src/lib/general.ts | 15 +++ libs/common/src/tests/general.spec.ts | 22 +++++ libs/events/src/lib/event-form.service.ts | 60 ++++++++++-- .../src/tests/event-form.service.spec.ts | 97 +++++++++++++++++++ 4 files changed, 185 insertions(+), 9 deletions(-) diff --git a/libs/common/src/lib/general.ts b/libs/common/src/lib/general.ts index 76b6f93cbb..58f6d0cd04 100644 --- a/libs/common/src/lib/general.ts +++ b/libs/common/src/lib/general.ts @@ -1168,6 +1168,9 @@ export function setupFormTimeSync( date_end: finiteNumber(snap().date_end), all_day: snap().all_day, }; + let timed_window: + | { date: number; duration: number; date_end: number } + | undefined; const refreshPrev = () => { const s = snap(); prev.date = finiteNumber(s.date); @@ -1585,6 +1588,11 @@ export function setupFormTimeSync( () => { const all_day = snap().all_day; if (all_day) { + timed_window = { + date: normaliseTimeValue(snap().date), + duration: normaliseTimeValue(snap().duration), + date_end: normaliseTimeValue(snap().date_end), + }; applyPatch( getAllDayTimeRange( normaliseTimeValue(snap().date), @@ -1593,7 +1601,14 @@ export function setupFormTimeSync( all_day_end, ) as Partial, ); + } else if ( + timed_window && + !isMultiday(timed_window.date, normaliseTimeValue(snap().date)) + ) { + applyPatch(timed_window as Partial); + timed_window = undefined; } else { + timed_window = undefined; const date = normaliseTimeValue(snap().date); const duration = normaliseTimeValue(snap().duration); const date_end = normaliseTimeValue(snap().date_end); diff --git a/libs/common/src/tests/general.spec.ts b/libs/common/src/tests/general.spec.ts index 8eefc24b4d..ee4bed2bb8 100644 --- a/libs/common/src/tests/general.spec.ts +++ b/libs/common/src/tests/general.spec.ts @@ -659,6 +659,28 @@ describe('General Methods', () => { // --- all_day --- + it('should restore the timed window after toggling all_day on and off', () => { + const date_end = addMinutes(BASE, 90).valueOf(); + const form = createForm({ + date: BASE, + duration: 90, + date_end, + }); + setupFormTimeSync(form, {}, injector); + + setField(form, { all_day: true }); + setField(form, { all_day: false }); + + expect(form()).toEqual( + expect.objectContaining({ + date: BASE, + duration: 90, + date_end, + all_day: false, + }), + ); + }); + it('should reset duration to default_duration when all_day is toggled off', () => { const form = createForm({ date: BASE, diff --git a/libs/events/src/lib/event-form.service.ts b/libs/events/src/lib/event-form.service.ts index b6479ee284..c7d601b759 100644 --- a/libs/events/src/lib/event-form.service.ts +++ b/libs/events/src/lib/event-form.service.ts @@ -75,22 +75,46 @@ const BOOKING_URLS = [ 'upcoming', ]; -/** - * Form fields that change without the user editing them, so they can't be used - * to work out whether a booking has details other than its attendees changed. - * `attendees` is compared on its own, and the remaining fields are either - * derived (`date_end`), re-normalised on load (`system`) or re-hydrated with - * extra detail from the API (`organiser`, `resources`) — the identity of those - * last two is compared separately. - */ +/** Form fields that are derived or need semantic comparison below. */ const IGNORED_DETAIL_FIELDS = [ 'attendees', + 'body', 'system', 'date_end', 'organiser', + 'recurrence', 'resources', ]; +function normaliseEventBody(body: string) { + const template = document.createElement('template'); + template.innerHTML = body || ''; + const serialise = (node: Node): string => { + if (node.nodeType === Node.TEXT_NODE) { + return (node.textContent || '').replace(/\u200b/g, ''); + } + if (node.nodeType !== Node.ELEMENT_NODE) return ''; + const element = node as Element; + if (element.tagName === 'BR') return '\n'; + const content = [...element.childNodes].map(serialise).join(''); + if (element.tagName === 'DIV' || element.tagName === 'P') { + return `\n${content}\n`; + } + const tag = element.tagName.toLowerCase(); + const attributes = [...element.attributes] + .sort((a, b) => a.name.localeCompare(b.name)) + .map(({ name, value }) => ` ${name}="${value}"`) + .join(''); + return `<${tag}${attributes}>${content}`; + }; + return [...template.content.childNodes] + .map(serialise) + .join('') + .replace(/[ \t]+\n|\n[ \t]+/g, '\n') + .replace(/\n+/g, '\n') + .trim(); +} + enum Tags { Availability = 'AVAILABILITY', BookingRules = 'BOOKING_RULES', @@ -1318,10 +1342,28 @@ export class EventFormService extends AsyncHandler { const details = Object.entries(value).filter( ([key]) => !IGNORED_DETAIL_FIELDS.includes(key), ); + const recurrence = value.recurrence; + details.push(['body', normaliseEventBody(value.body)]); details.push(['host_email', (value.organiser as any)?.email || '']); + details.push([ + 'recurrence', + recurrence?.pattern && recurrence?._pattern !== 'none' + ? [ + recurrence.pattern, + recurrence.interval || 1, + [...(recurrence.days_of_week || [])].sort(), + recurrence.nth_of_month || null, + recurrence.start || null, + recurrence.end || null, + recurrence.occurrences || null, + ] + : null, + ]); details.push([ 'space_ids', - (value.resources || []).map((_: any) => _.id || _.email || ''), + (value.resources || []) + .map((_: any) => (_.email || _.id || '').toLowerCase()) + .sort(), ]); details.sort(([a], [b]) => (a > b ? 1 : -1)); return JSON.stringify(details); diff --git a/libs/events/src/tests/event-form.service.spec.ts b/libs/events/src/tests/event-form.service.spec.ts index 5b771f8c1e..250572f0a2 100644 --- a/libs/events/src/tests/event-form.service.spec.ts +++ b/libs/events/src/tests/event-form.service.spec.ts @@ -330,6 +330,103 @@ describe('EventFormService', () => { expect(service.can_notify_new_attendees_only()).toBe(false); }); + it('should restore attendee-only notification eligibility for equivalent form values', () => { + const date = new Date(2028, 5, 15, 10).valueOf(); + const event = new CalendarEvent({ + id: 'event-1', + host: 'host@test.com', + title: 'Team meeting', + body: 'Original notes', + date, + duration: 90, + attendees: [{ email: 'existing@test.com' } as any], + resources: [ + { + id: 'calendar-resource-1', + email: 'space-1@test.com', + zones: [], + } as any, + ], + }); + service.newForm(event); + TestBed.tick(); + service.model.update((model) => ({ + ...model, + attendees: [...model.attendees, { email: 'new@test.com' }], + })); + + service.model.update((model) => ({ ...model, all_day: true })); + TestBed.tick(); + expect(service.can_notify_new_attendees_only()).toBe(false); + service.model.update((model) => ({ ...model, all_day: false })); + TestBed.tick(); + expect(service.model()).toEqual( + expect.objectContaining({ + date, + duration: 90, + date_end: date + 90 * 60 * 1000, + all_day: false, + }), + ); + expect(service.can_notify_new_attendees_only()).toBe(true); + + service.model.update((model) => ({ + ...model, + recurrence: { + pattern: 'daily', + _pattern: 'daily', + interval: 1, + start: date, + end: date + 24 * 60 * 60 * 1000, + }, + })); + expect(service.can_notify_new_attendees_only()).toBe(false); + service.model.update((model) => ({ + ...model, + recurrence: { + pattern: 'daily', + _pattern: 'none', + interval: 1, + days_of_week: [], + start: date, + end: date, + }, + })); + expect(service.can_notify_new_attendees_only()).toBe(true); + + service.model.update((model) => ({ + ...model, + resources: [ + { + id: 'placeos-system-2', + email: 'space-2@test.com', + } as any, + ], + })); + expect(service.can_notify_new_attendees_only()).toBe(false); + service.model.update((model) => ({ + ...model, + resources: [ + { + id: 'placeos-system-1', + email: 'space-1@test.com', + } as any, + ], + })); + expect(service.can_notify_new_attendees_only()).toBe(true); + + service.model.update((model) => ({ + ...model, + body: '
Changed notes
', + })); + expect(service.can_notify_new_attendees_only()).toBe(false); + service.model.update((model) => ({ + ...model, + body: '
Original notes
', + })); + expect(service.can_notify_new_attendees_only()).toBe(true); + }); + it('should suppress existing attendee notifications for attendee-only edits', async () => { const event = new CalendarEvent({ id: 'event-1', From cc6bb36350a76301e05c7447877a57dbb7a1ba9f Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Wed, 5 Aug 2026 20:41:46 +1000 Subject: [PATCH 6/9] test(bookings): pin the cross-form carry-over of typed input Switching between booking forms without leaving the booking area does not reset the form, so edits captured for the initialisation replay follow the user across. That is a consequence of the fix worth stating rather than discovering later: only fields the user actually edited move, isCrossTypeEdit still discards the previous booking's identity, and leaving the section calls clearForm(). Co-Authored-By: Claude Fable 5 --- libs/bookings/src/test/booking-form.service.spec.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/libs/bookings/src/test/booking-form.service.spec.ts b/libs/bookings/src/test/booking-form.service.spec.ts index cf35a71ed5..02221fc67b 100644 --- a/libs/bookings/src/test/booking-form.service.spec.ts +++ b/libs/bookings/src/test/booking-form.service.spec.ts @@ -2914,6 +2914,18 @@ describe('BookingFormService', () => { expect(spectator.service.model().all_day).toBe(true); }); + it('carries a typed title between booking forms, deliberately', () => { + // Switching desk -> parking without leaving the booking area does not + // reset the form, so the user's own typing follows them. Pinned + // rather than left to chance: only fields they actually edited move, + // `isCrossTypeEdit` still discards the previous booking's identity, + // and leaving the booking section entirely calls `clearForm()`. + userEdits('title', 'Desk title'); + spectator.service.loadForm('parking'); + spectator.service.newForm('parking'); + expect(spectator.service.model().title).toBe('Desk title'); + }); + it('does not carry those edits into a later unrelated form', () => { userEdits('title', 'Quiet corner desk'); spectator.service.loadForm('desk'); From 64b40c07e62eeb910a6f34b3aca740a1efb3918c Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Wed, 5 Aug 2026 22:06:05 +1000 Subject: [PATCH 7/9] test(e2e): keep the PPT-2642 client-abort diagnostic Rules out client disconnects as the trigger for the connection poisoning: aborting requests mid-flight strands nothing, while a burst with no aborts at all does. Kept next to the burst reproducer because a diagnostic that lives only in a ticket comment rots. Co-Authored-By: Claude Fable 5 --- e2e/support/repro/reg09-client-abort.ts | 89 +++++++++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 e2e/support/repro/reg09-client-abort.ts diff --git a/e2e/support/repro/reg09-client-abort.ts b/e2e/support/repro/reg09-client-abort.ts new file mode 100644 index 0000000000..f44243eb7b --- /dev/null +++ b/e2e/support/repro/reg09-client-abort.ts @@ -0,0 +1,89 @@ +/** + * PPT-2642 diagnostic — does a client that disconnects mid-request leave a + * transaction open on the server? + * + * The burst reproducer leaves a backend `idle in transaction` for as long as the + * process lives, with pg-orm 2.2.3's discard handling compiled in. That means a + * `BEGIN` was issued and neither committed nor rolled back — which is what you + * would expect if the request fiber died between the two, rather than unwinding + * through the `ensure` that returns the connection. + * + * The burst always aborted a couple of requests client-side, so this isolates + * that one variable: no concurrency, no serialization failure, just a POST that + * is cut off after the transaction has certainly started. + * + * e2e/stack/up.sh + * docker compose -p placeos-e2e restart staff-api # clean pool + * ABORT_MS=60 N=5 bunx tsx e2e/support/repro/reg09-client-abort.ts + * + * Then, with no other traffic: + * docker exec placeos-e2e-postgres-1 psql -U placeos -d placeos \ + * -c "SELECT pid, state, now()-state_change AS idle_for FROM pg_stat_activity + * WHERE state = 'idle in transaction';" + * + * A backend still `idle in transaction` seconds later means the abort is the + * trigger, and the fix belongs where the request fiber is torn down. + */ +import { mintToken } from '../auth'; + +const B = 'https://localhost:9443'; +const ABORT_MS = Number(process.env.ABORT_MS ?? 60); +const N = Number(process.env.N ?? 5); + +process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0'; + +(async () => { + const m = await mintToken( + B, + `${B}/backoffice`, + 'support@place.tech', + 'development', + ); + const auth = { Authorization: `Bearer ${m.accessToken}` }; + + const zones = ( + await (await fetch(`${B}/api/engine/v2/zones?limit=100`, { headers: auth })).json() + ).map((z: any) => z.id); + const me = await ( + await fetch(`${B}/api/engine/v2/users/current`, { headers: auth }) + ).json(); + + const base = Math.floor(Date.now() / 1000) + 86400 * 30; + let aborted = 0; + let completed = 0; + + for (let i = 0; i < N; i++) { + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), ABORT_MS); + try { + await fetch(`${B}/api/staff/v1/bookings`, { + method: 'POST', + signal: controller.signal, + headers: { ...auth, 'Content-Type': 'application/json' }, + body: JSON.stringify({ + booking_type: 'desk', + asset_id: `e2e-desk-${i % 5}`, + booking_start: base + i * 7200, + booking_end: base + i * 7200 + 3600, + timezone: 'Etc/UTC', + user_email: me.email, + user_id: me.id, + user_name: me.name, + title: `REG09-abort ${i}`, + zones, + }), + }); + completed += 1; + } catch { + aborted += 1; + } finally { + clearTimeout(timer); + } + // let the server finish whatever it is doing before the next one + await new Promise((r) => setTimeout(r, 300)); + } + + console.log(`\n ${N} POSTs, aborted after ${ABORT_MS}ms`); + console.log(` aborted client-side: ${aborted}, completed: ${completed}`); + console.log(' now check pg_stat_activity for `idle in transaction`'); +})(); From 3b36ac654575b08cf05fb39c69dfb4f88695b37b Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Wed, 5 Aug 2026 23:11:51 +1000 Subject: [PATCH 8/9] fix(concierge): prevent broadcast recipient flicker (PPT-2400) --- .../broadcast-email-modal.component.ts | 11 ++++++++-- .../broadcast-email-modal.component.spec.ts | 22 ++++++++++++++++++- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/apps/concierge/src/app/email-templates/broadcast-email-modal.component.ts b/apps/concierge/src/app/email-templates/broadcast-email-modal.component.ts index 6525056592..cfa51d28af 100644 --- a/apps/concierge/src/app/email-templates/broadcast-email-modal.component.ts +++ b/apps/concierge/src/app/email-templates/broadcast-email-modal.component.ts @@ -1,4 +1,11 @@ -import { Component, computed, effect, inject, signal } from '@angular/core'; +import { + Component, + computed, + effect, + inject, + signal, + untracked, +} from '@angular/core'; import { form, FormField, required, validate } from '@angular/forms/signals'; import { MatDialogModule, MatDialogRef } from '@angular/material/dialog'; import { MatFormFieldModule } from '@angular/material/form-field'; @@ -231,7 +238,7 @@ export class BroadcastEmailModalComponent { effect(() => { this.form.recipient_group().value(); this.form.recipients().value(); - this.updateRecipients(); + untracked(() => this.updateRecipients()); }); } diff --git a/apps/concierge/src/tests/email-templates/broadcast-email-modal.component.spec.ts b/apps/concierge/src/tests/email-templates/broadcast-email-modal.component.spec.ts index 9571c12e9c..d84b832023 100644 --- a/apps/concierge/src/tests/email-templates/broadcast-email-modal.component.spec.ts +++ b/apps/concierge/src/tests/email-templates/broadcast-email-modal.component.spec.ts @@ -1,5 +1,8 @@ -import { ComponentFixtureAutoDetect } from '@angular/core/testing'; +import { ComponentFixtureAutoDetect, TestBed } from '@angular/core/testing'; import { MatDialogRef } from '@angular/material/dialog'; +import { MatFormFieldModule } from '@angular/material/form-field'; +import { MatInputModule } from '@angular/material/input'; +import { MatSelectModule } from '@angular/material/select'; import { createComponentFactory, Spectator } from '@ngneat/spectator/vitest'; import { OrganisationService, @@ -31,6 +34,7 @@ describe('BroadcastEmailModalComponent', () => { MockComponent(FullscreenModalShellComponent), MockComponent(UserListFieldComponent), ], + imports: [MatFormFieldModule, MatInputModule, MatSelectModule], providers: [ { provide: ComponentFixtureAutoDetect, useValue: false }, MockProvider(MatDialogRef, { close: dialog_close } as any), @@ -103,6 +107,22 @@ describe('BroadcastEmailModalComponent', () => { ]); }); + it('should not re-resolve recipients when the subject changes', () => { + TestBed.flushEffects(); + const update_recipients = vi.spyOn( + spectator.component, + 'updateRecipients', + ); + + spectator.component.model.update((m) => ({ + ...m, + subject: 'Emergency notice', + })); + TestBed.flushEffects(); + + expect(update_recipients).not.toHaveBeenCalled(); + }); + it('should notify an error and skip sending when the mailer is missing', async () => { smtp_module = null; spectator.component.model.update((m) => ({ From 7f3be9bb8bcd2220ed24ae15484bef17904220cb Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Wed, 5 Aug 2026 23:14:41 +1000 Subject: [PATCH 9/9] fix(concierge): clarify overnight parking bookings --- .../parking-bookings-list.component.ts | 24 ++++++++- .../parking-bookings-list.component.spec.ts | 52 ++++++++++++++++++- 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/apps/concierge/src/app/parking/parking-bookings-list.component.ts b/apps/concierge/src/app/parking/parking-bookings-list.component.ts index cdc9fd8982..f13ac2d1d5 100644 --- a/apps/concierge/src/app/parking/parking-bookings-list.component.ts +++ b/apps/concierge/src/app/parking/parking-bookings-list.component.ts @@ -24,6 +24,8 @@ import { TableColumn, TranslatePipe, } from '@placeos/components'; +import { isSameDay } from 'date-fns'; +import { toZonedTime } from 'date-fns-tz'; import { ParkingBookingsWeekViewComponent } from './parking-bookings-week-view.component'; import { ParkingStateService } from './parking-state.service'; import { @@ -147,6 +149,9 @@ interface ParkingBookingColumnTemplates { } @else { {{ row.date | date: time_format : timezone }} - {{ row.date_end | date: time_format : timezone }} + @if (isNextDay(row)) { + +1 + } } @@ -593,7 +598,12 @@ export class ParkingBookingsListComponent private _state = inject(ParkingStateService); private _settings = inject(SettingsService); - public readonly bookings = this._state.bookings; + public readonly bookings = computed(() => { + const selected_date = this._state.options().date; + return this._state + .bookings() + .filter((booking) => this._isSameDay(booking.date, selected_date)); + }); public readonly options = this._state.options; public readonly loading = this._state.loading; public readonly period = this._state.period; @@ -774,6 +784,18 @@ export class ParkingBookingsListComponent ); } + public isNextDay(booking: Booking) { + return !this._isSameDay(booking.date, booking.date_end); + } + + private _isSameDay(first: number, second: number) { + const timezone = this.timezone; + return isSameDay( + timezone ? toZonedTime(first, timezone) : first, + timezone ? toZonedTime(second, timezone) : second, + ); + } + public statusLabel(booking: Booking) { return this.isAssignedBooking(booking) ? 'APP.CONCIERGE.BOOKING_STATUS_ASSIGNED' diff --git a/apps/concierge/src/tests/parking/parking-bookings-list.component.spec.ts b/apps/concierge/src/tests/parking/parking-bookings-list.component.spec.ts index 11f0ea2025..c09a520e06 100644 --- a/apps/concierge/src/tests/parking/parking-bookings-list.component.spec.ts +++ b/apps/concierge/src/tests/parking/parking-bookings-list.component.spec.ts @@ -18,6 +18,7 @@ describe('ParkingBookingsListComponent', () => { let custom_booking_columns: any[] = []; let bookable_hours: { start: number; end: number } | undefined; let timezone = 'Australia/Perth'; + let selected_date = Date.now(); let request_filter: 'all' | 'bookings' | 'requests' | 'waitlist' = 'all'; const createComponent = createComponentFactory({ @@ -26,7 +27,7 @@ describe('ParkingBookingsListComponent', () => { MockProvider(ParkingStateService, { bookings: (() => bookings) as any, options: (() => ({ - date: Date.now(), + date: selected_date, search: '', zones: [], period: 'day', @@ -93,6 +94,7 @@ describe('ParkingBookingsListComponent', () => { custom_booking_columns = []; bookable_hours = undefined; timezone = 'Australia/Perth'; + selected_date = Date.now(); request_filter = 'all'; settingSignal('parking.allow_editing', true).set(true); settingSignal('parking.allow_deleting', false).set(false); @@ -154,6 +156,7 @@ describe('ParkingBookingsListComponent', () => { }); it('should show start and end times for all-day bookings', () => { + selected_date = new Date(2026, 6, 21, 8).valueOf(); bookings = [ { id: 'booking-1', @@ -174,6 +177,7 @@ describe('ParkingBookingsListComponent', () => { it('should show all day when the booking matches the bookable period', () => { bookable_hours = { start: 8, end: 17 }; + selected_date = new Date(2026, 6, 21, 8).valueOf(); bookings = [ { id: 'booking-1', @@ -193,6 +197,50 @@ describe('ParkingBookingsListComponent', () => { ).not.toHaveText(':'); }); + it('should only show bookings that start on the selected day', () => { + selected_date = new Date('2026-08-03T12:00:00+08:00').valueOf(); + bookings = [ + { + id: 'previous-day', + asset_id: 'bay-1', + status: 'approved', + date: new Date('2026-08-02T17:30:00+08:00').valueOf(), + date_end: new Date('2026-08-03T06:30:00+08:00').valueOf(), + duration: 13 * 60, + }, + { + id: 'selected-day', + asset_id: 'bay-2', + status: 'approved', + date: new Date('2026-08-03T17:30:00+08:00').valueOf(), + date_end: new Date('2026-08-04T06:30:00+08:00').valueOf(), + duration: 13 * 60, + }, + ] as Booking[]; + spectator = createComponent(); + + expect( + spectator.component.filtered_events().map(({ id }) => id), + ).toEqual(['selected-day']); + }); + + it('should mark overnight booking end times as the next day', () => { + selected_date = new Date('2026-08-03T12:00:00+08:00').valueOf(); + bookings = [ + { + id: 'overnight', + asset_id: 'bay-1', + status: 'approved', + date: new Date('2026-08-03T17:30:00+08:00').valueOf(), + date_end: new Date('2026-08-04T06:30:00+08:00').valueOf(), + duration: 13 * 60, + } as Booking, + ]; + spectator = createComponent(); + + expect(spectator.query('sup')).toHaveText('+1'); + }); + it('should add custom extension data columns', () => { custom_booking_columns = [ { field: 'cost_code', name: 'Cost Code' }, @@ -456,6 +504,8 @@ describe('ParkingBookingsListComponent', () => { bookings = [ { asset_id: 'bay-1', + date: selected_date, + date_end: selected_date + 60 * 60 * 1000, extension_data: { vehicle_type: 'truck' }, } as unknown as Booking, ];