From dcbd667be07a971deeab6ad1dbd9b8b7b532e507 Mon Sep 17 00:00:00 2001 From: sharmilaseenivasan17 Date: Tue, 15 Sep 2026 16:02:51 +1000 Subject: [PATCH] test(e2e): add visitor invite coverage for workplace 23 tests across 11 spec files, mirroring the existing desk coverage: invite (single and group), cancel from the app, form validation, settings, visitor details, times, booking for a colleague, check-in/check-out, editing, and visibility between users. 22 pass; 1 is fixme, blocked on an app bug where saving an edit in single-visitor mode throws and sends nothing. Visitor support code lives in e2e/support/visitor/ so the desk specs cannot be affected - no shared support file is changed. Also adds e2e/tsconfig.json and a bun run e2e:typecheck script; nothing type-checked the specs before. Co-Authored-By: Claude Opus 5 (1M context) --- E2E_USER_STORIES.md | 77 +++- .../e2e/local/visitor-cancel.spec.ts | 120 ++++++ .../e2e/local/visitor-checkin.spec.ts | 231 +++++++++++ .../e2e/local/visitor-delegate.spec.ts | 206 ++++++++++ .../e2e/local/visitor-details.spec.ts | 131 ++++++ apps/workplace/e2e/local/visitor-edit.spec.ts | 319 +++++++++++++++ .../workplace/e2e/local/visitor-group.spec.ts | 171 ++++++++ .../e2e/local/visitor-invite.spec.ts | 163 ++++++++ .../e2e/local/visitor-scoping.spec.ts | 180 +++++++++ .../e2e/local/visitor-settings.spec.ts | 131 ++++++ .../workplace/e2e/local/visitor-times.spec.ts | 136 +++++++ .../e2e/local/visitor-validation.spec.ts | 143 +++++++ e2e/README.md | 8 + e2e/VISITOR_E2E_HANDOVER.md | 197 +++++++++ e2e/support/repro/vis-b1-duplicate-visitor.ts | 88 ++++ e2e/support/repro/vis-b6-burst.ts | 117 ++++++ e2e/support/repro/vis-b9-group-clash.ts | 110 +++++ e2e/support/visitor/calendar.ts | 150 +++++++ e2e/support/visitor/invite-form.page.ts | 323 +++++++++++++++ e2e/support/visitor/visitor.api.ts | 203 ++++++++++ e2e/support/visitor/visitor.env.ts | 67 ++++ e2e/support/visitor/visitor.flows.ts | 378 ++++++++++++++++++ e2e/support/visitor/visitor.settings.ts | 121 ++++++ e2e/support/visitor/your-bookings.page.ts | 332 +++++++++++++++ e2e/tsconfig.json | 26 ++ package.json | 1 + 26 files changed, 4122 insertions(+), 7 deletions(-) create mode 100644 apps/workplace/e2e/local/visitor-cancel.spec.ts create mode 100644 apps/workplace/e2e/local/visitor-checkin.spec.ts create mode 100644 apps/workplace/e2e/local/visitor-delegate.spec.ts create mode 100644 apps/workplace/e2e/local/visitor-details.spec.ts create mode 100644 apps/workplace/e2e/local/visitor-edit.spec.ts create mode 100644 apps/workplace/e2e/local/visitor-group.spec.ts create mode 100644 apps/workplace/e2e/local/visitor-invite.spec.ts create mode 100644 apps/workplace/e2e/local/visitor-scoping.spec.ts create mode 100644 apps/workplace/e2e/local/visitor-settings.spec.ts create mode 100644 apps/workplace/e2e/local/visitor-times.spec.ts create mode 100644 apps/workplace/e2e/local/visitor-validation.spec.ts create mode 100644 e2e/VISITOR_E2E_HANDOVER.md create mode 100644 e2e/support/repro/vis-b1-duplicate-visitor.ts create mode 100644 e2e/support/repro/vis-b6-burst.ts create mode 100644 e2e/support/repro/vis-b9-group-clash.ts create mode 100644 e2e/support/visitor/calendar.ts create mode 100644 e2e/support/visitor/invite-form.page.ts create mode 100644 e2e/support/visitor/visitor.api.ts create mode 100644 e2e/support/visitor/visitor.env.ts create mode 100644 e2e/support/visitor/visitor.flows.ts create mode 100644 e2e/support/visitor/visitor.settings.ts create mode 100644 e2e/support/visitor/your-bookings.page.ts create mode 100644 e2e/tsconfig.json diff --git a/E2E_USER_STORIES.md b/E2E_USER_STORIES.md index 2d5fa45467..5d503904d4 100644 --- a/E2E_USER_STORIES.md +++ b/E2E_USER_STORIES.md @@ -118,13 +118,71 @@ the PR gate. | 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-11 | P2 | Inviting a **visitor** end to end. | **done** — expanded into its own section, [§1a](#1a-workplace--visitor-invites) (VIS-01 … VIS-14) | | 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 | | WP-E2E-14 | P2 | Search validation and empty states: no blank page, no console error. | todo | | WP-E2E-15 | P1 | **Room/meeting** booking end to end. | **out of scope (external)** — the only surface needing a real Microsoft/Google tenant. Opt-in project, never in the PR gate. | | WP-E2E-04 | P2 | Mock mode still renders the landing page with no backend at all. | **done** — `landing.spec.ts` (project `mock`) | +## 1a. Workplace — visitor invites + +Twenty-three tests in eleven files under `apps/workplace/e2e/local/visitor-*.spec.ts`, with +their own support code in `e2e/support/visitor/` — deliberately separate from +`e2e/support/{env,api,flows}.ts` so nothing visitor-shaped can break the desk specs. +Working notes: [`e2e/VISITOR_E2E_HANDOVER.md`](e2e/VISITOR_E2E_HANDOVER.md). + +**Not covered, and why.** A second building cannot be exercised — the local stack +seeds one org, one building, one level, so a building/level selector test would +assert against a list of one. That needs a seeding change, which was deliberately +avoided here (see *Decisions* in the handover). Everything email-shaped is out too: +there is no mail server in the stack and the test identities are not real mailboxes. + +**Workplace only.** `concierge` and `visitor-kiosk` also show visitors, but the desk specs +cover neither (zero mentions anywhere in `e2e/`) and neither is in the sparse checkout this +was built in. Extending there is a separate piece of work, not an oversight. + +| ID | P | Story | Status | +|----|---|-------|--------| +| VIS-01 | P1 | A **non-admin** invites a visitor through the full UI; the backend stores the booking with the right address, title and zones, **and** a guest record is created from it. | **done** — `local/visitor-invite.spec.ts` | +| VIS-02 | P1 | A deleted visitor invite leaves the listing — and the guest record it created is removed too, since that **outlives the booking**. | **done** — `local/visitor-invite.spec.ts` | +| VIS-03 | P1 | Inviting several visitors at once creates a `group` container plus one `visitor` booking per person, linked by `parent_id`, each carrying its own address. | **done** — `local/visitor-group.spec.ts`. The container-to-member link is what PPT-2638 broke. | +| VIS-04 | P1 | The visitor listing returns the **members**, not the container: `GET /bookings?type=visitor` on an N-person group gives N, not N+1. | **done** — `local/visitor-group.spec.ts` | +| VIS-05 | P1 | One user's visitor invite is **not** visible to another user, and cannot be deleted by them. | **done** — `local/visitor-scoping.spec.ts`. The desk equivalent is WP-E2E-08; a visitor invite leaks a named outside person, not just a seat. | +| VIS-06 | P1 | Control for VIS-05: you *can* see your own invite and its guest record, so "nobody sees anything" cannot pass as success. | **done** — `local/visitor-scoping.spec.ts` | +| VIS-07 | P2 | With all-day invites allowed, an all-day invite is **stored** as all-day rather than as a long timed booking. | **done** — `local/visitor-settings.spec.ts` | +| VIS-08 | P2 | With a reason required, sending without one is refused by the form and nothing reaches the backend. | **done** — `local/visitor-settings.spec.ts` | +| VIS-09 | P1 | Booking on behalf of somebody else stores **them** as the host (`user_*`) and **you** as the booker (`booked_by_*`). | **done** — `local/visitor-delegate.spec.ts`. Needs `visitors.can_book_for_anyone` **and** `visitors.can_book_for_others`: one without the other renders the host picker and then silently discards the choice. | +| VIS-10 | P1 | The booker's schedule flags a delegated invite as **Associate** (and shows "For {name}"); the host's own listing shows it as theirs, unflagged. | **done** — `local/visitor-delegate.spec.ts`. Visibility is asymmetric — the booker only sees it at all because the schedule sends `include_booked_by=true`. | +| VIS-11 | P1 | A visitor is checked **in** and then back **out**, both recorded on the backend; after checking out the control is gone, so one invite cannot be used twice. | **done** — `local/visitor-checkin.spec.ts`. Check-in and check-out are the same button; only the check-out asks for confirmation. | +| VIS-12 | P1 | A booking too far ahead offers no check-in control, **and** the API refuses one for the timing reason — so hiding the button is not the only thing stopping it. | **done** — `local/visitor-checkin.spec.ts`. The two gates are different sizes: the app hides the button until **15 minutes** before the start, the backend accepts within **1 hour**. | +| VIS-13 | P2 | Re-saving a delegated invite **for a future day** keeps the original host, and keeps the day, rather than resetting either to the person editing. | **done** — `local/visitor-edit.spec.ts` | +| VIS-14 | P2 | Removing one visitor from a group invite leaves the other members untouched. | **done** — `local/visitor-edit.spec.ts` (PPT-2634 / PPT-2638) | +| VIS-15 | P2 | The same edit works when the app is in **single-visitor** mode. | **blocked** — `local/visitor-edit.spec.ts`, `test.fixme`. VIS-B8 below. The identical flow in the default mode is green (VIS-13), so this is the mode, not the flow. | +| VIS-16 | P1 | Cancelling an invite **from the app** — the booking menu and its confirmation — really removes it, on the backend and not just on screen. | **done** — `local/visitor-cancel.spec.ts`. Every other spec tears down through the API, so this button was never once pressed. | +| VIS-17 | P1 | Declining that confirmation leaves the invite alone. | **done** — `local/visitor-cancel.spec.ts`. A dialog whose decline button also deletes is worse than one that fails to delete, and only this catches it. | +| VIS-18 | P1 | An address that is not an email is refused **before anything is sent**. | **done** — `local/visitor-validation.spec.ts`. Asserts on the network, not on a message: the backend stores whatever it is given, so the form is the only guard. Includes a control that a valid address does send. | +| VIS-19 | P2 | A visitor taken off the list before sending is not invited, and the rest still are. | **done** — `local/visitor-validation.spec.ts` | +| VIS-20 | P2 | A pass number and the international-visitor flag are stored with the invite. | **done** — `local/visitor-details.spec.ts`. Both behind settings that are off by default. | +| VIS-21 | P2 | The company is stored, and the separate guest record carries the visitor's name. | **done** — `local/visitor-details.spec.ts`. The guest record is what reception reads and it outlives the booking. | +| VIS-22 | P1 | The start time and length chosen on the form are the ones stored. | **done** — `local/visitor-times.spec.ts`. Everything else here takes the default hour, so a form that ignored the picker would have passed the whole suite. | +| VIS-23 | P2 | A maximum visit length and bookable hours limit what the form offers. | **done** — `local/visitor-times.spec.ts`. Asserts on the options offered, because an out-of-range choice is absent rather than refused. Red-checked: without the settings the form offers 240 minutes against a 60 minute cap. | + +### Findings from building this coverage + +Six, all confirmed against the running stack, **none filed yet** and none of them test +defects. Listed here so the coverage record is honest about what the suite knows. + +| ID | Finding | +|---|---| +| **VIS-B8** | **In single-visitor mode only** (`bookings.multiple_visitors: false`), saving an edit throws `_.toJSON is not a function` and sends nothing — the user sees only that raw string. `Booking` declares `attendees` as `User[]` but assigns the raw server JSON to it, the edit path copies that into the form's `assets`, and saving calls `.toJSON()` on every entry. The multi-visitor paths escape it by rebuilding each person with `new User(...)`. Measured both modes, one and two visitors, today and a future day: the **mode** is the only thing that decides it. Blocks VIS-15. | +| **VIS-B9** | **A host cannot make two overlapping group invites, whoever the visitors are.** The app names a group container `${host_email}[${date the invite was CREATED}]`, so every group invite that host makes that day shares one asset id — and the backend rightly refuses overlapping bookings on one asset. The second invite fails with `409 Conflicting booking` pointing at a `group` row. Booking the same two visits on different days is accepted, which is what shows this is accidental. Reproducer: `e2e/support/repro/vis-b9-group-clash.ts`. | +| **VIS-B1** | The same visitor can be booked twice into one slot; the backend returns 201 both times. Desks refuse this, and product says visitors should too. Reproducer: `e2e/support/repro/vis-b1-duplicate-visitor.ts`. | +| **Host flags** | `visitors.can_book_for_anyone` renders the host picker, but `sendInvite` keeps the choice only if `visitors.can_book_for_others` is **also** set. One without the other silently discards the host — no error, no clue. | +| **Listing limit** | `GET /bookings` defaults to `limit=100` and the schedule sends `include_deleted=true`, so past 100 soft-deleted bookings a user stops seeing new ones. This broke the card-dependent specs until 428 stale rows were cleared, and it looks nothing like its cause. See the run-hygiene note in *Notes & blockers*. | +| **Deep link** | `?booking=` never opens the details modal: `booking-card.component.ts` checks `params.has('booking')` and then compares the id against `params.get('event')`, a parameter that is not there. | +| **Dead config** | `bookings.allowed_daily_visitor_count` (default 100) is never read anywhere in this repo. Confirmed with the dev that the limit is not wanted. | + ## 2. Auth & session Grounded in the auth.cr work (PPT-2536), where every production failure was an @@ -156,7 +214,7 @@ task that found it, so the row can be traced. | REG-06 | P2 | Timezone parsing does not error for a building with an unusual timezone. | "Fix error when parsing timezones" | todo | | REG-07 | P2 | Level selection does not persist once the selector is hidden/disabled. | "Fix level selections persisting when selector is disabled/hidden" | todo | | REG-08 | P1 | An authority with a **relative** `login_url` still reaches a usable login page. | Found 2026-07-30, this suite | **blocked** — currently worked around in `seed.ts`; ts-client resolves a relative `login_url` against the authority host **without its port**, so any non-443 deployment dead-ends. Needs a ts-client/init fix before a spec can assert the good behaviour. | -| REG-09 | P1 | Concurrent `POST /bookings` do not 500. | **[PPT-2642](https://acaprojects.atlassian.net/browse/PPT-2642)** | **fixed at source, not yet in the image this suite runs against.** Root cause was in pg-orm: crystal-db clears a connection's transaction flag only *after* the COMMIT it issues, so a COMMIT that fails — the serialization failures a booking burst produces — leaves it set, and the one place connections return to the pool could not see it. The connection went back to the pool with its `BEGIN` still open; reads kept working inside the orphaned transaction while every write returned 500 until restart. Fixed in [pg-orm#19](https://github.com/spider-gazelle/pg-orm/pull/19) (v2.2.4) and picked up by staff-api in #386. Measured with images differing only by that version: control stranded a connection in 3 of 4 bursts and ended unable to write; fixed, 0 of 11 and still writable. **`e2e/stack` pulls `placeos/staff-api:latest`, which is a release image — so this suite keeps running the old behaviour until the next platform release.** Harmless for now: the suite does not burst concurrent bookings, so nothing here trips it. Reproducer kept at `e2e/support/repro/reg09-concurrent-bookings.ts`. | +| REG-09 | P1 | Concurrent `POST /bookings` do not 500. | **[PPT-2642](https://acaprojects.atlassian.net/browse/PPT-2642)** | **fixed at source, not yet in the image this suite runs against.** Root cause was in pg-orm: crystal-db clears a connection's transaction flag only *after* the COMMIT it issues, so a COMMIT that fails — the serialization failures a booking burst produces — leaves it set, and the one place connections return to the pool could not see it. The connection went back to the pool with its `BEGIN` still open; reads kept working inside the orphaned transaction while every write returned 500 until restart. Fixed in [pg-orm#19](https://github.com/spider-gazelle/pg-orm/pull/19) (v2.2.4) and picked up by staff-api in #386. Measured with images differing only by that version: control stranded a connection in 3 of 4 bursts and ended unable to write; fixed, 0 of 11 and still writable. **`e2e/stack` pulls `placeos/staff-api:latest`, which is a release image — so this suite keeps running the old behaviour until the next platform release.** **Corrected 2026-09-14: this suite now DOES burst concurrent bookings.** The line that used to sit here — "harmless for now, nothing here bursts concurrent bookings" — stopped being true when the group-invite specs (VIS-03/VIS-04) landed: a group invite posts the container and every member at once, which is exactly the burst. Measured with `e2e/support/repro/vis-b6-burst.ts` (no browser, 4 concurrent POSTs x 12 rounds): 13x 201, 29x 400, 6x 500, clean in the early rounds and degrading after. Before a rollback-and-retry was added to the visitor flows the group specs failed about one run in three; with it, 10 of 10 runs clean and the recovery fired twice. That retry is a **workaround living in `e2e/support/visitor/visitor.flows.ts`**, to be deleted once the staff-api image carries pg-orm v2.2.4 — it is also why the suite being green must not be read as REG-09 being gone from the image. Reproducers: `e2e/support/repro/reg09-concurrent-bookings.ts` and `vis-b6-burst.ts`. | | REG-10 | P1 | The booking form does not discard user input while it is still initialising. | **[PPT-2643](https://acaprojects.atlassian.net/browse/PPT-2643)** | **blocked** — the form is rebuilt when async init completes and restores defaults (title, All Day, Require locker), silently dropping anything typed before that. A real user can hit this; they would just see their title or options revert. `bookDeskViaUI` converges on the state to work around it, which means **the suite no longer detects it** — hence this row. Investigated 2026-08-05 against #478 (`a0360486`): **the bug is still live**, established by reading the code rather than by running this suite, and fixed in **PR #479**. `newForm`'s protected branch is never taken by the flows — the current user is restored from cache ~50ms after bootstrap, while `NewDeskFlowComponent.ngOnInit` calls `loadForm` then `newForm` only after org init plus a 300ms settle — and `loadForm` had no capture at all. The shipped e2e suite meanwhile is stable at 6 consecutive full runs, 8/8, `--retries=0`, which is precisely the problem: **it passes either way**. Removing the block could not be shown to be safe *or* unsafe from here: the race needs initialisation to be slow relative to typing, and this machine wins it every time. Two failed attempts to prove otherwise, both recorded so nobody repeats them: (1) a synthetic "type during init, assert it survives" spec passed with *and* without the fix, even with the API responses held to widen the window; (2) removing the block appeared to prove the bug survived — it did not. That red was a Playwright **strict-mode violation**, not a reverted value: opening the desk-select modal puts a second "All Day" checkbox in the DOM (`desk-filters`, bound to the same field), so an unscoped locator matched two elements and threw, with both checked. Scoping the locator to `desk-flow-form` then broke it a second way, because `setCheckbox` silently returns when its locator matches nothing, turning a narrower scope into a no-op and a genuinely invalid form. Both experiments were reverted. **The block stays and this row stays blocked even once #479 lands** — not because the app is unfixed, but because this suite cannot tell either way on fast hardware. The guard for REG-10 is the unit specs in `libs/bookings/src/test/booking-form.service.spec.ts`; unblocking this row needs artificial slowness (throttled CPU), not another e2e attempt. | ## 4. Platform & configuration @@ -179,17 +237,22 @@ Config gaps caused several production incidents, and they are invisible to UI sp unblocks every PlaceOS-native booking type (desks, lockers, parking, visitors) with no outbound call. `/calendars` and `/events` do call Microsoft and fail `AADSTS900023`, so WP-E2E-15 stays opt-in and out of the gate. -- **Three rows are blocked on product fixes, not on test effort** (REG-08, REG-09, REG-10). All - were found by this suite. Leaving them visible here is the point — a blocked row is coverage +- **Four rows are blocked on product fixes, not on test effort** (REG-08, REG-09, REG-10, + VIS-15). All were found by this suite. Leaving them visible here is the point — a blocked row is coverage information, a deleted row is not. - **REG-09 is fixed, and worth reading about.** One burst of concurrent booking POSTs used to poison staff-api's connection pool, so booking creation returned 500 for everyone until the service restarted — while reads kept working, because they ran inside the orphaned transaction. Filed as PPT-2642, root-caused to pg-orm and fixed in v2.2.4. It reaches this suite when the next platform release rebuilds `placeos/staff-api:latest`; until then the suite runs the old - behaviour, which is harmless here because nothing in it bursts concurrent bookings. - Two reproducers are kept next to the finding: `reg09-concurrent-bookings.ts` (the burst) and - `reg09-client-abort.ts` (which ruled out client disconnects). + behaviour — and since the group-invite specs landed, **the suite trips it**. Group invites post + a container plus every member at once, which is the burst; the visitor flows roll back a + partial group and retry to absorb it. Delete that retry when the image is updated, and do not + read a green run as evidence the image is fixed. + Three reproducers are kept next to the finding: `reg09-concurrent-bookings.ts` (the burst), + `reg09-client-abort.ts` (which ruled out client disconnects) and `vis-b6-burst.ts` (the same + burst with no browser at all, which is what proved the visitor 400s were REG-09 rather than a + form bug). - **REG-10 is invisible to this suite by design.** `bookDeskViaUI` converges on the form state, so nothing here will catch it regressing. PPT-2643's fix landed in #478; whether it is *complete* is genuinely unresolved, and the honest summary is that this suite cannot answer it diff --git a/apps/workplace/e2e/local/visitor-cancel.spec.ts b/apps/workplace/e2e/local/visitor-cancel.spec.ts new file mode 100644 index 0000000000..1e928b4f77 --- /dev/null +++ b/apps/workplace/e2e/local/visitor-cancel.spec.ts @@ -0,0 +1,120 @@ +/** + * VIS-16 / VIS-17 — cancelling an invite FROM THE APP. + * + * Every other spec here tears its invite down through the API, which is right + * for a teardown and useless as coverage: it means the Cancel button a real user + * presses, and the confirmation box in front of it, were never once exercised. + * A broken confirm dialog would have gone unnoticed by the whole suite. + * + * The pair is deliberate. Cancelling and NOT cancelling are equally important: + * a dialog whose decline button also deletes is a worse bug than one that fails + * to delete, and only the second test can catch it. + * + * Both read the backend afterwards. The card leaving the screen proves nothing + * on its own — the app removes it optimistically. + */ +import { test, expect } from '../../../../e2e/support/fixtures'; +import { listBookings, deleteBooking, uniqueTitle } from '../../../../e2e/support/api'; +import { VISITOR_SLOTS, visitorFor } from '../../../../e2e/support/visitor/visitor.env'; +import { deleteGuest, releaseVisitor } from '../../../../e2e/support/visitor/visitor.api'; +import { + SINGLE_VISITOR_MODE, + useSettings, +} from '../../../../e2e/support/visitor/visitor.settings'; +import { inviteVisitorViaUI } from '../../../../e2e/support/visitor/visitor.flows'; +import { YourBookingsPage } from '../../../../e2e/support/visitor/your-bookings.page'; +import type { APIRequestContext } from '@playwright/test'; + +const DAY = 86_400; +const window_from = () => Math.floor(Date.now() / 1000) - 2 * DAY; +const window_to = () => Math.floor(Date.now() / 1000) + 2 * DAY; + +/** Is this booking still live (present and not soft-deleted) on the backend? */ +async function isLive(api: APIRequestContext, id: number): Promise { + const live = await listBookings(api, 'visitor', window_from(), window_to()); + return live.some((b) => Number(b.id) === id && !b.deleted); +} + +test.describe('cancelling a visitor invite from the app', () => { + test('cancelling from the booking menu removes it for real', async ({ + staffPage, + staffApi, + }, testInfo) => { + const visitor = visitorFor(testInfo.parallelIndex, VISITOR_SLOTS.cancel.fromApp); + const reason = uniqueTitle('E2E Visit Cancel'); + let booking_id: number | undefined; + + await releaseVisitor(staffApi, visitor.email, window_from(), window_to()); + await useSettings(staffPage, SINGLE_VISITOR_MODE); + + try { + const created = await inviteVisitorViaUI(staffPage, staffApi, visitor, reason); + booking_id = created.id; + + const page = new YourBookingsPage(staffPage); + await page.open(); + await page.showVisitors(); + await page.showDayOf(created.booking_start * 1000); + expect( + await isLive(staffApi, booking_id), + 'precondition: the invite is live before we cancel it', + ).toBe(true); + + await page.startCancel(booking_id); + await page.acceptConfirm(); + + // The backend is the judge. The card disappearing is the app being + // optimistic, and it does that whether or not the call succeeded. + await expect(async () => { + expect( + await isLive(staffApi, booking_id!), + 'the cancelled invite must be gone from the backend, not just the screen', + ).toBe(false); + }).toPass({ timeout: 30_000 }); + + await expect( + page.card(booking_id), + 'and its card must leave the schedule', + ).toBeHidden({ timeout: 30_000 }); + } finally { + if (booking_id != null) await deleteBooking(staffApi, booking_id).catch(() => null); + await deleteGuest(staffApi, visitor.email); + } + }); + + test('declining the confirmation leaves the invite alone', async ({ + staffPage, + staffApi, + }, testInfo) => { + const visitor = visitorFor(testInfo.parallelIndex, VISITOR_SLOTS.cancel.dismissed); + const reason = uniqueTitle('E2E Visit Keep'); + let booking_id: number | undefined; + + await releaseVisitor(staffApi, visitor.email, window_from(), window_to()); + await useSettings(staffPage, SINGLE_VISITOR_MODE); + + try { + const created = await inviteVisitorViaUI(staffPage, staffApi, visitor, reason); + booking_id = created.id; + + const page = new YourBookingsPage(staffPage); + await page.open(); + await page.showVisitors(); + await page.showDayOf(created.booking_start * 1000); + + await page.startCancel(booking_id); + await page.dismissConfirm(); + + // Wait before asserting: "still there" a millisecond after the click + // would pass even if a delete were already on its way. + await staffPage.waitForTimeout(5_000); + expect( + await isLive(staffApi, booking_id), + 'walking away from the confirmation must not cancel anything', + ).toBe(true); + } finally { + if (booking_id != null) await deleteBooking(staffApi, booking_id).catch(() => null); + await deleteGuest(staffApi, visitor.email); + } + }); +}); diff --git a/apps/workplace/e2e/local/visitor-checkin.spec.ts b/apps/workplace/e2e/local/visitor-checkin.spec.ts new file mode 100644 index 0000000000..9c5caf08ea --- /dev/null +++ b/apps/workplace/e2e/local/visitor-checkin.spec.ts @@ -0,0 +1,231 @@ +/** + * VIS-11 / VIS-12 — checking a visitor in from the booking details modal. + * + * Check-in is the one visitor behaviour that is genuinely time-sensitive, and + * the backend rule is strict: + * + * POST /bookings/:id/check_in?state=true + * -> 405 {"error":"Can only check in an 1.0 hour before the booking start"} + * + * So these specs pin the start time instead of taking the form's default. They + * also create their bookings through the API rather than the invite form: the + * subject is check-in, and driving the form first would only add an unrelated + * way to fail. The desk suite sidesteps all of this by booking all-day, which is + * not an option when the window under test is an hour wide. + * + * Test 1 walks the whole visit: check in, then check out. They are the same + * button — the app flips it rather than showing a second one — and only the + * check-out asks for confirmation. Test 2 owns the other side: the times when + * check-in must NOT be offered. + * + * Neither test trusts the schedule's default day. It opens on whatever "today" + * was at page load and never moves, so both drive the sidebar calendar to the + * booking's OWN day with `showDayOf`. An earlier version instead kept both + * bookings inside today and skipped near midnight; that still failed when a run + * crossed midnight between creating the booking and loading the page, and it + * quietly skipped the check-in coverage every evening. + */ +import { test, expect } from '../../../../e2e/support/fixtures'; +import { + currentUser, + deleteBooking, + getBooking, + uniqueTitle, + zonesWithTag, +} from '../../../../e2e/support/api'; +import { VISITOR_SLOTS, visitorFor } from '../../../../e2e/support/visitor/visitor.env'; +import { + checkInViaApi, + createBookingViaApi, + deleteGuest, + releaseVisitor, +} from '../../../../e2e/support/visitor/visitor.api'; +import { + SINGLE_VISITOR_MODE, + useSettings, +} from '../../../../e2e/support/visitor/visitor.settings'; +import { YourBookingsPage } from '../../../../e2e/support/visitor/your-bookings.page'; +import type { APIRequestContext } from '@playwright/test'; + +const DAY = 86_400; +const window_from = () => Math.floor(Date.now() / 1000) - 2 * DAY; +const window_to = () => Math.floor(Date.now() / 1000) + 2 * DAY; + +/** + * A fixed hour tomorrow, local time, as unix seconds. + * + * Local rather than UTC because the schedule groups cards by the browser's own + * day, and the browser and this process share a clock and a timezone — nothing + * in the config pins either. + */ +function tomorrowAt(hour: number): number { + const day = new Date(); + day.setDate(day.getDate() + 1); + day.setHours(hour, 0, 0, 0); + return Math.floor(day.valueOf() / 1000); +} + +async function inviteAt( + api: APIRequestContext, + visitor: { email: string; name: string }, + title: string, + startSeconds: number, +) { + const me = await currentUser(api); + const zones = ( + await Promise.all(['org', 'building', 'level'].map((t) => zonesWithTag(api, t))) + ) + .flat() + .map((z) => z.id); + + return createBookingViaApi(api, { + booking_type: 'visitor', + asset_id: visitor.email, + asset_name: visitor.name, + booking_start: startSeconds, + booking_end: startSeconds + 3600, + timezone: 'Etc/UTC', + user_email: me.email, + user_id: me.id, + user_name: me.name, + title, + zones, + attendees: [{ name: visitor.name, email: visitor.email }], + }); +} + +test.describe('visitor check-in', () => { + test('a visitor can be checked in and back out again', async ({ + staffPage, + staffApi, + }, testInfo) => { + const visitor = visitorFor(testInfo.parallelIndex, VISITOR_SLOTS.checkin.checkIn); + const title = uniqueTitle('E2E Visit CheckIn'); + // Must satisfy BOTH gates, which are different sizes and easy to + // conflate: + // the app shows the button only within 15 MINUTES of the start + // (`Booking.state` returns 'future' before that); + // the backend accepts a check-in within 1 HOUR of the start. + // 10 minutes clears the tighter one with margin, and stays in the future + // so a slow run cannot let the booking start underneath the test. This + // one cannot move to a fixed hour like its neighbour does — a check-in + // has to happen next to its own start time. + // It is deliberately NOT pinned to today either: 10 minutes from now can + // land on tomorrow, and that is fine, because the view is driven to the + // booking's own day below. + const start = Math.floor(Date.now() / 1000) + 10 * 60; + + await releaseVisitor(staffApi, visitor.email, window_from(), window_to()); + await useSettings(staffPage, SINGLE_VISITOR_MODE); + + const booking = await inviteAt(staffApi, visitor, title, start); + try { + expect( + (await getBooking(staffApi, booking.id)).checked_in, + 'precondition: a new invite is not checked in', + ).toBeFalsy(); + + const page = new YourBookingsPage(staffPage); + await page.open(); + await page.showVisitors(); + await page.showDayOf(start * 1000); + const modal = await page.openDetails(booking.id); + + const check_in = page.checkInButton(modal); + await expect( + check_in, + 'the check-in control is missing — it is hidden by ' + + '`bookings.hide_checkin`/`visitors.hide_checkin`, by a booking that ' + + 'is already done, or by `auto_checkin`', + ).toBeVisible({ timeout: 20_000 }); + await check_in.click(); + + // Assert on the stored record, not on the button turning green. + await expect(async () => { + expect((await getBooking(staffApi, booking.id)).checked_in).toBe(true); + }).toPass({ timeout: 20_000 }); + + // ...and back out again. This is the SAME button — the app flips it + // once you are checked in rather than showing a second one — and it + // asks for confirmation first, which check-in does not. + await check_in.click(); + await page.acceptConfirm(); + + await expect(async () => { + const out = await getBooking(staffApi, booking.id); + expect( + out.checked_in, + 'checking out must clear the flag on the backend, not just on screen', + ).toBeFalsy(); + }).toPass({ timeout: 20_000 }); + + // Once checked out the control disappears altogether: the app keeps + // `checked_out_at` and hides it, so a visitor cannot be checked in + // twice off one invite. Without this, a check-out that only redrew + // the button would still look like a pass. + await expect( + check_in, + 'the check-in control must not come back after checking out', + ).toBeHidden({ timeout: 20_000 }); + } finally { + await deleteBooking(staffApi, booking.id); + await deleteGuest(staffApi, visitor.email); + } + }); + + test('a booking too far ahead offers no check-in, and the backend refuses too', async ({ + staffPage, + staffApi, + }, testInfo) => { + const visitor = visitorFor(testInfo.parallelIndex, VISITOR_SLOTS.checkin.tooEarly); + const title = uniqueTitle('E2E Visit Too Early'); + // A fixed hour tomorrow, not "3 hours from now": the gates are the + // subject here, and 10am tomorrow is outside both of them at every hour + // of the day, so the test behaves the same at 09:00 and at 23:59. + const start = tomorrowAt(10); + + await releaseVisitor(staffApi, visitor.email, window_from(), window_to()); + await useSettings(staffPage, SINGLE_VISITOR_MODE); + + const booking = await inviteAt(staffApi, visitor, title, start); + try { + const page = new YourBookingsPage(staffPage); + await page.open(); + await page.showVisitors(); + await page.showDayOf(start * 1000); + const modal = await page.openDetails(booking.id); + + // The app's own guard: `Booking.state` is 'future' until 15 minutes + // before the start, and the button is only rendered for + // upcoming/started/in_progress. So there is nothing to click — an + // earlier version of this spec expected to click and be refused, + // which was wrong about the app rather than finding a bug. + await expect( + page.checkInButton(modal), + 'a booking 3h away must not offer check-in at all', + ).toBeHidden(); + + // ...and the rule is not client-side only. Without this, hiding the + // button would be enough to pass while the API accepted a check-in + // from anyone at any time. + const refused = await checkInViaApi(staffApi, booking.id); + expect( + refused.status, + `the backend must refuse an early check-in, got ${refused.status} ` + + `${refused.body}`, + ).toBeGreaterThanOrEqual(400); + expect( + refused.body, + 'and refuse it for the timing reason, not some unrelated error', + ).toMatch(/before the booking start/i); + + expect( + (await getBooking(staffApi, booking.id)).checked_in, + 'a refused check-in must leave the booking not checked in', + ).toBeFalsy(); + } finally { + await deleteBooking(staffApi, booking.id); + await deleteGuest(staffApi, visitor.email); + } + }); +}); diff --git a/apps/workplace/e2e/local/visitor-delegate.spec.ts b/apps/workplace/e2e/local/visitor-delegate.spec.ts new file mode 100644 index 0000000000..3c8c5ed8c6 --- /dev/null +++ b/apps/workplace/e2e/local/visitor-delegate.spec.ts @@ -0,0 +1,206 @@ +/** + * VIS-09 / VIS-10 — inviting a visitor ON BEHALF OF somebody else. + * + * Three people are involved in a delegated invite and the model only names two + * of them, which is the thing most likely to be got wrong: + * + * user_email / user_id / user_name the HOST — who the visitor is here to see + * booked_by_email / booked_by_name the BOOKER — who filled the form in + * asset_id + attendees the VISITOR + * + * "Booked for" and "host" are the same field. There is no third role. + * + * Visibility follows from that split, and it is not symmetric: + * + * the HOST sees the invite in a plain `GET /bookings` listing + * the BOOKER sees it ONLY with `include_booked_by=true` + * + * The schedule page always sends that flag, which is why an invite you made for + * someone else turns up in your own list — carrying an "Associate" badge to say + * it is not yours. Test two asserts both halves of that. + * + * Requires `visitors.can_book_for_anyone` AND `basic_user_search`; see + * `invite-form.page.ts` for why the other host control cannot work locally. + */ +import { test, expect } from '../../../../e2e/support/fixtures'; +import { + STAFF_API, + currentUser, + deleteBooking, + getBooking, + listBookings, + uniqueTitle, +} from '../../../../e2e/support/api'; +import { WORKERS, roleFor, staffEmail } from '../../../../e2e/support/env'; +import { VISITOR_SLOTS, visitorFor } from '../../../../e2e/support/visitor/visitor.env'; +import { deleteGuest, releaseVisitor } from '../../../../e2e/support/visitor/visitor.api'; +import { + BOOK_FOR_ANYONE, + SINGLE_VISITOR_MODE, + useSettings, +} from '../../../../e2e/support/visitor/visitor.settings'; +import { inviteVisitorViaUI } from '../../../../e2e/support/visitor/visitor.flows'; +import { YourBookingsPage } from '../../../../e2e/support/visitor/your-bookings.page'; + +const DAY = 86_400; +const window_from = () => Math.floor(Date.now() / 1000) - 2 * DAY; +const window_to = () => Math.floor(Date.now() / 1000) + 2 * DAY; + +const DELEGATE_MODE = { ...SINGLE_VISITOR_MODE, ...BOOK_FOR_ANYONE }; + +/** The seeded non-admin this worker will nominate as host — never itself. */ +function hostFor(workerIndex: number) { + const other = (workerIndex + 1) % WORKERS; + return { + index: other, + email: staffEmail(other), + // seed.ts names them exactly this; the host field displays the NAME + // once chosen, so the flow needs it to tell "set" from "not set". + name: `E2E Staff ${other} (non-admin)`, + role: roleFor('staff', other), + }; +} + +test.describe('visitor invite on behalf of another host', () => { + test('the chosen host is stored as the host and you as the booker', async ({ + staffPage, + staffApi, + }, testInfo) => { + const mine = testInfo.parallelIndex; + const host = hostFor(mine); + test.skip(host.index === mine, 'needs at least two workers for a second identity'); + + const visitor = visitorFor(mine, VISITOR_SLOTS.delegate.single); + const reason = uniqueTitle('E2E Delegate Visit'); + let booking_id: number | undefined; + + await releaseVisitor(staffApi, visitor.email, window_from(), window_to()); + await useSettings(staffPage, DELEGATE_MODE); + + try { + const me = await currentUser(staffApi); + const created = await inviteVisitorViaUI(staffPage, staffApi, visitor, reason, { + host: { email: host.email, name: host.name }, + }); + booking_id = created.id; + + const stored = await getBooking(staffApi, booking_id); + expect(stored.user_email, 'the chosen host owns the booking').toBe(host.email); + expect(stored.booked_by_email, 'and we are recorded as the booker').toBe( + me.email, + ); + expect( + stored.user_email, + 'host and booker must actually differ, or this proves nothing', + ).not.toBe(stored.booked_by_email); + expect(stored.asset_id, 'the visitor is unchanged by delegating').toBe( + visitor.email, + ); + } finally { + if (booking_id != null) await deleteBooking(staffApi, booking_id); + await deleteGuest(staffApi, visitor.email); + } + }); + + test('the booker sees it flagged as Associate, the host sees it as their own', async ({ + staffPage, + staffApi, + browser, + }, testInfo) => { + const mine = testInfo.parallelIndex; + const host = hostFor(mine); + test.skip(host.index === mine, 'needs at least two workers for a second identity'); + + const visitor = visitorFor(mine, VISITOR_SLOTS.delegate.groupA); + const reason = uniqueTitle('E2E Delegate View'); + let booking_id: number | undefined; + + await releaseVisitor(staffApi, visitor.email, window_from(), window_to()); + await useSettings(staffPage, DELEGATE_MODE); + + try { + const created = await inviteVisitorViaUI(staffPage, staffApi, visitor, reason, { + host: { email: host.email, name: host.name }, + }); + booking_id = created.id; + + // --- the BOOKER's view ------------------------------------------- + const mine_page = new YourBookingsPage(staffPage); + await mine_page.open(); + await mine_page.showVisitors(); + + await expect( + mine_page.card(booking_id), + 'an invite you made for someone else still appears in your own list — ' + + 'the schedule sends include_booked_by=true, which is what surfaces it', + ).toBeVisible({ timeout: 30_000 }); + await expect( + mine_page.associateBadge(booking_id), + 'and it is badged "Associate", because the host is not you. If this ' + + 'fails, check whether a third badge was added to that corner of ' + + 'the card — the locator excludes only the checked-in one', + ).toBeVisible(); + await expect( + mine_page.bookedForLine(booking_id), + 'the card names the host it was booked for', + ).toContainText(host.name); + + // --- the HOST's view ---------------------------------------------- + // A real second browser context as the host, not a re-query as us. + const host_ctx = await browser.newContext({ + storageState: host.role.storagePath, + ignoreHTTPSErrors: true, + }); + const host_page = await host_ctx.newPage(); + try { + const theirs = new YourBookingsPage(host_page); + await theirs.open(); + await theirs.showVisitors(); + + await expect( + theirs.card(booking_id), + 'the host sees the invite in their own list', + ).toBeVisible({ timeout: 30_000 }); + await expect( + theirs.associateBadge(booking_id), + 'but NOT badged Associate — it is their booking, not someone else’s', + ).toBeHidden(); + await expect( + theirs.bookedForLine(booking_id), + 'and no "For {name}" line, for the same reason', + ).toBeHidden(); + } finally { + await host_ctx.close(); + } + + // The API rule underneath both views, asserted directly so a UI + // change cannot hide a backend regression. + const booker_plain = await listBookings( + staffApi, + 'visitor', + window_from(), + window_to(), + ); + expect( + booker_plain.map((b) => b.id), + 'a plain listing is caller-scoped, so the BOOKER does not see it', + ).not.toContain(booking_id); + + const booker_included = await staffApi.get(`${STAFF_API}/bookings`, { + params: { + type: 'visitor', + period_start: String(window_from()), + period_end: String(window_to()), + include_booked_by: 'true', + }, + }); + expect( + ((await booker_included.json()) as { id: number }[]).map((b) => b.id), + 'with include_booked_by=true they do — this is what the schedule sends', + ).toContain(booking_id); + } finally { + if (booking_id != null) await deleteBooking(staffApi, booking_id); + await deleteGuest(staffApi, visitor.email); + } + }); +}); diff --git a/apps/workplace/e2e/local/visitor-details.spec.ts b/apps/workplace/e2e/local/visitor-details.spec.ts new file mode 100644 index 0000000000..47d5d90346 --- /dev/null +++ b/apps/workplace/e2e/local/visitor-details.spec.ts @@ -0,0 +1,131 @@ +/** + * VIS-20 / VIS-21 — the details carried alongside the visitor's address. + * + * None of these change whether an invite saves, which is exactly why they are + * worth a spec: a field that silently stops being stored looks completely normal + * on screen. Reception is the one who finds out, at the door. + * + * Where they end up is not uniform, and that is the trap: + * `international` -> written explicitly into `extension_data` for visitor + * bookings (`buildBookingExtensionData`) + * `pass_number`, + * `company` -> plain form fields, copied into `extension_data` + * automatically by the `Booking` constructor + * the visitor -> `asset_id` / `attendees`, and a separate guest record + * So all three are asserted on what came back from the backend, not on the form. + * + * Pass number and the international tick are both behind `visitors.allow_pass_number` + * / `visitors.allow_international`, off by default — the classic "only breaks for + * the customers who switched it on" shape. + */ +import { test, expect } from '../../../../e2e/support/fixtures'; +import { deleteBooking, getBooking, uniqueTitle } from '../../../../e2e/support/api'; +import { VISITOR_SLOTS, visitorFor } from '../../../../e2e/support/visitor/visitor.env'; +import { + deleteGuest, + releaseVisitor, + searchGuests, + type VisitorBooking, +} from '../../../../e2e/support/visitor/visitor.api'; +import { + ALLOW_EXTRAS, + SINGLE_VISITOR_MODE, + useSettings, +} from '../../../../e2e/support/visitor/visitor.settings'; +import { inviteVisitorViaUI } from '../../../../e2e/support/visitor/visitor.flows'; + +const DAY = 86_400; +const window_from = () => Math.floor(Date.now() / 1000) - 2 * DAY; +const window_to = () => Math.floor(Date.now() / 1000) + 2 * DAY; + +test.describe('visitor invite details', () => { + test('a pass number and the international flag are stored with the invite', async ({ + staffPage, + staffApi, + }, testInfo) => { + const visitor = visitorFor(testInfo.parallelIndex, VISITOR_SLOTS.details.extras); + const reason = uniqueTitle('E2E Visit Extras'); + const pass_number = `E2E-PASS-${testInfo.parallelIndex}-${Date.now() % 100000}`; + let booking_id: number | undefined; + + await releaseVisitor(staffApi, visitor.email, window_from(), window_to()); + await useSettings(staffPage, { ...SINGLE_VISITOR_MODE, ...ALLOW_EXTRAS }); + + try { + const created = await inviteVisitorViaUI(staffPage, staffApi, visitor, reason, { + passNumber: pass_number, + international: true, + }); + booking_id = created.id; + + // `getBooking` returns the shared `Booking`, whose `extension_data` + // is typed as empty. `VisitorBooking` is the same row with the + // visitor keys spelled out — see `visitor.api.ts`. + const stored = (await getBooking(staffApi, booking_id)) as VisitorBooking; + expect( + stored.extension_data?.pass_number, + 'the pass number typed into the form must reach the backend — ' + + 'reception reads this one', + ).toBe(pass_number); + expect( + stored.extension_data?.international, + 'and the international tick must be stored as a real true, not dropped', + ).toBe(true); + + // The visitor themselves, alongside the extras, so a booking that + // stored the flags against the wrong person cannot pass. + expect(stored.asset_id, 'the invite is still for this visitor').toBe( + visitor.email, + ); + } finally { + if (booking_id != null) await deleteBooking(staffApi, booking_id); + await deleteGuest(staffApi, visitor.email); + } + }); + + test('the company is stored, and the guest record carries the visitor name', async ({ + staffPage, + staffApi, + }, testInfo) => { + const visitor = visitorFor(testInfo.parallelIndex, VISITOR_SLOTS.details.company); + const reason = uniqueTitle('E2E Visit Company'); + let booking_id: number | undefined; + + await releaseVisitor(staffApi, visitor.email, window_from(), window_to()); + await deleteGuest(staffApi, visitor.email); + await useSettings(staffPage, SINGLE_VISITOR_MODE); + + try { + const created = await inviteVisitorViaUI(staffPage, staffApi, visitor, reason); + booking_id = created.id; + + // `getBooking` returns the shared `Booking`, whose `extension_data` + // is typed as empty. `VisitorBooking` is the same row with the + // visitor keys spelled out — see `visitor.api.ts`. + const stored = (await getBooking(staffApi, booking_id)) as VisitorBooking; + expect( + stored.extension_data?.company, + 'the company typed into the form must reach the backend', + ).toBe(visitor.company); + expect( + stored.extension_data?.visitor_name, + 'and so must the visitor name, which is stored separately from the address', + ).toBe(visitor.name); + + // The guest record is a second, independent copy of who is coming — + // it is what reception and the kiosk read, and it outlives the + // booking. A name lost here is a nameless person at the door. + await expect(async () => { + const guests = await searchGuests(staffApi, visitor.email); + const guest = guests.find( + (g) => g.email?.toLowerCase() === visitor.email.toLowerCase(), + ); + expect(guest, 'a guest record was created for the visitor').toBeTruthy(); + expect(guest!.name, 'and it carries their name').toBe(visitor.name); + }).toPass({ timeout: 20_000 }); + } finally { + if (booking_id != null) await deleteBooking(staffApi, booking_id); + await deleteGuest(staffApi, visitor.email); + } + }); +}); diff --git a/apps/workplace/e2e/local/visitor-edit.spec.ts b/apps/workplace/e2e/local/visitor-edit.spec.ts new file mode 100644 index 0000000000..8b9a1efac8 --- /dev/null +++ b/apps/workplace/e2e/local/visitor-edit.spec.ts @@ -0,0 +1,319 @@ +/** + * VIS-13 / VIS-14 / VIS-15 — editing an invite that already exists. + * + * ## Which invites can actually be edited + * + * Editing works in MULTI-visitor mode, which is what the app ships with + * (`bookings.multiple_visitors` defaults to true). It is broken in + * SINGLE-visitor mode: pressing Update throws `_.toJSON is not a function`, no + * request is sent, and the user sees only that raw string. That is VIS-B8, and + * the third test here is the placeholder for it. + * + * Measured, all three created and edited through the app — multi with one + * visitor and with two both save cleanly (`PATCH`, "updated successfully"); + * single fails every time, on today's date and on a future one alike. The mode + * decides it, not the date and not the number of people. + * + * Cause: `Booking` declares `attendees` as `User[]` but assigns the raw server + * JSON to it (`booking.class.ts`), so at runtime they are plain objects. In + * single-visitor mode the edit path hands that straight to the form's `assets` + * (`invite-visitor-form.component.ts`), and saving calls `.toJSON()` on every + * entry (`booking-form.service.ts`). Plain JSON has no such method. The + * multi-visitor paths escape it because they rebuild each person with + * `new User(...)` first. + * + * ## Why THREE tests in this file, when every other spec has two + * + * The third is a deliberate placeholder for VIS-B8 and is `fixme`, so it costs + * nothing per run. Fold it away once the app is fixed: at that point it is the + * same assertion as the first test with one setting changed. + * + * ## What the first two guard + * + * - Re-saving a DELEGATED invite used to overwrite the host with whoever had + * the form open. The booker is usually the one editing, so the bug quietly + * reassigned the visit to them. This one is booked for a FUTURE day as well, + * since an invite worth editing is usually one that has not happened yet. + * - Removing one visitor from a group used to take others with it, or leave the + * removed one behind (PPT-2634, PPT-2638). A group edit rewrites several + * bookings at once, so "it looked right on screen" is worth nothing here — + * every assertion reads the backend. + * + * Editing is off by default (`visitors.allow_editing`), which is why the Edit + * action simply is not in the menu unless a spec turns it on. + */ +import { test, expect } from '../../../../e2e/support/fixtures'; +import { + currentUser, + deleteBooking, + getBooking, + listBookings, + uniqueTitle, +} from '../../../../e2e/support/api'; +import { WORKERS, staffEmail } from '../../../../e2e/support/env'; +import { VISITOR_SLOTS, visitorFor } from '../../../../e2e/support/visitor/visitor.env'; +import { + deleteGuest, + releaseGroupContainers, + releaseVisitor, +} from '../../../../e2e/support/visitor/visitor.api'; +import { + ALLOW_EDITING, + BOOK_FOR_ANYONE, + GROUP_VISITOR_MODE, + SINGLE_VISITOR_MODE, + useSettings, +} from '../../../../e2e/support/visitor/visitor.settings'; +import { + inviteVisitorViaUI, + inviteVisitorsViaUI, +} from '../../../../e2e/support/visitor/visitor.flows'; +import { InviteVisitorForm } from '../../../../e2e/support/visitor/invite-form.page'; +import { YourBookingsPage } from '../../../../e2e/support/visitor/your-bookings.page'; + +const DAY = 86_400; +// Wide enough to sweep the future-dated invite below. A window that only +// reached +2 days would leave a booking from a crashed run sitting on day 3, +// invisible to every later sweep. +const window_from = () => Math.floor(Date.now() / 1000) - 2 * DAY; +const window_to = () => Math.floor(Date.now() / 1000) + 7 * DAY; + +/** A fixed hour, some days out, in local time — the browser shares this clock. */ +function daysAhead(days: number, hour = 10): number { + const day = new Date(); + day.setDate(day.getDate() + days); + day.setHours(hour, 0, 0, 0); + return day.valueOf(); +} + +test.describe('editing a visitor invite', () => { + test('re-saving a delegated invite for a future day keeps the original host', async ({ + staffPage, + staffApi, + }, testInfo) => { + const mine = testInfo.parallelIndex; + const other = (mine + 1) % WORKERS; + test.skip(other === mine, 'needs at least two workers for a second identity'); + + const host = { email: staffEmail(other), name: `E2E Staff ${other} (non-admin)` }; + const visitor = visitorFor(mine, VISITOR_SLOTS.edit.details); + const reason = uniqueTitle('E2E Edit Host'); + const new_reason = uniqueTitle('E2E Edit Host Updated'); + // Three days out. A visit worth editing is normally one still to come, + // and it also proves the edit path does not quietly depend on the + // booking being today. + const visit_day = daysAhead(3); + let booking_id: number | undefined; + const created_ids: number[] = []; + + await releaseVisitor(staffApi, visitor.email, window_from(), window_to()); + await releaseGroupContainers(staffApi, window_from(), window_to()); + await useSettings(staffPage, { + ...GROUP_VISITOR_MODE, + ...BOOK_FOR_ANYONE, + ...ALLOW_EDITING, + }); + + try { + const me = await currentUser(staffApi); + // Multi-visitor mode with one guest: the mode the app ships with, + // and the one where saving an edit works at all (VIS-B8). + const created = await inviteVisitorsViaUI( + staffPage, + staffApi, + [visitor], + reason, + { host, date: visit_day }, + ); + created_ids.push(...created.map((b) => b.id)); + const member = created.find((b) => b.booking_type === 'visitor'); + expect(member, 'the invite created a visitor booking to edit').toBeTruthy(); + booking_id = member!.id; + + const stored = await getBooking(staffApi, booking_id); + expect( + stored.user_email, + 'precondition: the invite starts out hosted by somebody else', + ).toBe(host.email); + expect( + new Date(stored.booking_start * 1000).toDateString(), + 'precondition: the invite really is on the future day we picked', + ).toBe(new Date(visit_day).toDateString()); + + // Re-open it and change only the reason. The schedule opens on + // today, so it has to be walked to the visit's own day first. + const bookings_page = new YourBookingsPage(staffPage); + await bookings_page.open(); + await bookings_page.showVisitors(); + await bookings_page.showDayOf(visit_day); + await bookings_page.startEdit(booking_id); + + const form = new InviteVisitorForm(staffPage); + await expect(async () => { + await form.reason.fill(new_reason); + expect(await form.reason.inputValue()).toBe(new_reason); + }).toPass({ timeout: 30_000 }); + await form.sendButton.click(); + + await expect(async () => { + const updated = await getBooking(staffApi, booking_id!); + expect(updated.title, 'the new reason was saved').toBe(new_reason); + }).toPass({ timeout: 30_000 }); + + const updated = await getBooking(staffApi, booking_id); + expect( + updated.user_email, + 'the host must survive the edit — it must NOT become whoever had the ' + + 'form open, which is the regression this test exists for', + ).toBe(host.email); + expect(updated.user_email, 'and still differ from the editor').not.toBe( + me.email, + ); + expect( + new Date(updated.booking_start * 1000).toDateString(), + 'and the visit must still be on the day it was booked for', + ).toBe(new Date(visit_day).toDateString()); + } finally { + for (const id of created_ids) await deleteBooking(staffApi, id).catch(() => null); + await releaseGroupContainers(staffApi, window_from(), window_to()); + await deleteGuest(staffApi, visitor.email); + } + }); + + test('removing one visitor from a group leaves the others alone', async ({ + staffPage, + staffApi, + }, testInfo) => { + const kept = visitorFor(testInfo.parallelIndex, VISITOR_SLOTS.edit.memberA); + const removed = visitorFor(testInfo.parallelIndex, VISITOR_SLOTS.edit.memberB); + const reason = uniqueTitle('E2E Edit Group'); + const created_ids: number[] = []; + + for (const v of [kept, removed]) { + await releaseVisitor(staffApi, v.email, window_from(), window_to()); + } + await releaseGroupContainers(staffApi, window_from(), window_to()); + await useSettings(staffPage, { ...GROUP_VISITOR_MODE, ...ALLOW_EDITING }); + + try { + const bookings = await inviteVisitorsViaUI( + staffPage, + staffApi, + [kept, removed], + reason, + ); + created_ids.push(...bookings.map((b) => b.id)); + + const members = bookings.filter((b) => b.booking_type === 'visitor'); + const kept_booking = members.find((b) => b.asset_id === kept.email); + const removed_booking = members.find((b) => b.asset_id === removed.email); + expect(kept_booking, 'precondition: both visitors were booked').toBeTruthy(); + expect(removed_booking, 'precondition: both visitors were booked').toBeTruthy(); + + // Edit the group from either member — they share a container. + const bookings_page = new YourBookingsPage(staffPage); + await bookings_page.open(); + await bookings_page.showVisitors(); + await bookings_page.startEdit(kept_booking!.id); + + const form = new InviteVisitorForm(staffPage); + await expect( + form.chips, + 'the edit form should load with both visitors already listed', + ).toHaveCount(2, { timeout: 30_000 }); + + // Not a `hasText` filter on the address: a chip renders only the + // part before the @, so that matched nothing and read as a broken + // remove button. `removeVisitorChip` matches what is really shown. + await form.removeVisitorChip(removed.email); + await expect(form.chips, 'one visitor was removed from the list').toHaveCount(1); + + await form.sendButton.click(); + + // Read the backend, not the screen. A group edit rewrites several + // rows and the screen shows only what the form thinks it did. + await expect(async () => { + const live = ( + await listBookings(staffApi, 'visitor', window_from(), window_to()) + ).filter((b) => !b.deleted); + const addresses = live.map((b) => b.asset_id); + expect( + addresses, + 'the visitor we removed must no longer hold a booking', + ).not.toContain(removed.email); + expect( + addresses, + 'and the one we kept must still have theirs — removing a visitor ' + + 'must not take the rest of the group with it (PPT-2634)', + ).toContain(kept.email); + }).toPass({ timeout: 30_000 }); + } finally { + const leftovers = await listBookings( + staffApi, + 'visitor', + window_from(), + window_to(), + ); + for (const b of leftovers) { + if ([kept.email, removed.email].includes(b.asset_id)) { + await deleteBooking(staffApi, b.id); + } + } + for (const id of created_ids) await deleteBooking(staffApi, id); + for (const v of [kept, removed]) await deleteGuest(staffApi, v.email); + await releaseGroupContainers(staffApi, window_from(), window_to()); + } + }); + + /** + * VIS-15 — the SAME edit, in single-visitor mode. This is the one that is + * broken (VIS-B8): `_.toJSON is not a function`, no request sent, nothing + * saved. Kept as `fixme` so the bug has a home in the suite rather than + * only in a document. + * + * When the app is fixed, drop the `fixme` and this should pass as written. + * Evidence it is the mode and not anything else: the first test above does + * the same thing in multi-visitor mode and is green. + */ + test.fixme('an invite made in single-visitor mode can be re-saved', async ({ + staffPage, + staffApi, + }, testInfo) => { + const visitor = visitorFor(testInfo.parallelIndex, VISITOR_SLOTS.edit.details); + const reason = uniqueTitle('E2E Edit Single'); + const new_reason = uniqueTitle('E2E Edit Single Updated'); + let booking_id: number | undefined; + + await releaseVisitor(staffApi, visitor.email, window_from(), window_to()); + await useSettings(staffPage, { ...SINGLE_VISITOR_MODE, ...ALLOW_EDITING }); + + try { + const created = await inviteVisitorViaUI(staffPage, staffApi, visitor, reason); + booking_id = created.id; + + const bookings_page = new YourBookingsPage(staffPage); + await bookings_page.open(); + await bookings_page.showVisitors(); + await bookings_page.startEdit(booking_id); + + const form = new InviteVisitorForm(staffPage); + await expect(async () => { + await form.reason.fill(new_reason); + expect(await form.reason.inputValue()).toBe(new_reason); + }).toPass({ timeout: 30_000 }); + await form.sendButton.click(); + + await expect(async () => { + const updated = await getBooking(staffApi, booking_id!); + expect( + updated.title, + 'the edit must reach the backend — in single-visitor mode it ' + + 'currently throws in the browser and sends nothing (VIS-B8)', + ).toBe(new_reason); + }).toPass({ timeout: 30_000 }); + } finally { + if (booking_id != null) await deleteBooking(staffApi, booking_id); + await deleteGuest(staffApi, visitor.email); + } + }); +}); diff --git a/apps/workplace/e2e/local/visitor-group.spec.ts b/apps/workplace/e2e/local/visitor-group.spec.ts new file mode 100644 index 0000000000..fe4562e2f4 --- /dev/null +++ b/apps/workplace/e2e/local/visitor-group.spec.ts @@ -0,0 +1,171 @@ +/** + * VIS-03 / VIS-04 — invite SEVERAL visitors at once, through the full UI. + * + * This is the mode workplace ships with (`bookings.multiple_visitors` defaults + * to true), and it is not "the single flow, twice". One send produces N+1 + * records: + * + * - a GROUP CONTAINER booking, `booking_type: 'group'`, carrying the shared + * group reference in `extension_data`; + * - one `booking_type: 'visitor'` booking PER visitor, each with `parent_id` + * pointing at that container. + * + * Two consequences that shaped this file: + * + * - `GET /bookings?type=visitor` does NOT return the container, because the + * container is type `group`. Expecting N+1 from a visitor-typed listing is + * the obvious mistake, and the second test exists to stop anyone making it. + * - Waiting on a single POST response would assert against the container and + * silently ignore every member, so `inviteVisitorsViaUI` collects them all. + * + * The container-to-member link is what PPT-2638 broke and fixed, so asserting + * on `parent_id` rather than merely on "three bookings exist" is the point. + */ +import { test, expect } from '../../../../e2e/support/fixtures'; +import { deleteBooking, listBookings, uniqueTitle } from '../../../../e2e/support/api'; +import { VISITOR_SLOTS, visitorFor } from '../../../../e2e/support/visitor/visitor.env'; +import { + deleteGuest, + releaseGroupContainers, + releaseVisitor, + searchGuests, +} from '../../../../e2e/support/visitor/visitor.api'; +import { + GROUP_VISITOR_MODE, + useSettings, +} from '../../../../e2e/support/visitor/visitor.settings'; +import { inviteVisitorsViaUI } from '../../../../e2e/support/visitor/visitor.flows'; + +const DAY = 86_400; +const window_from = () => Math.floor(Date.now() / 1000) - 2 * DAY; +const window_to = () => Math.floor(Date.now() / 1000) + 2 * DAY; + +/** + * Sweep both halves. `releaseVisitor` lists `type=visitor` and so cannot see a + * group container; a run that died between creating the container and its + * members leaves one orphaned (VIS-B7), and without this they accumulate. + */ +async function sweep(api: Parameters[0], emails: string[]) { + for (const email of emails) { + await releaseVisitor(api, email, window_from(), window_to()); + } + await releaseGroupContainers(api, window_from(), window_to()); +} + +test.describe('visitor invite (group)', () => { + test('inviting two visitors creates a container plus one booking each', async ({ + staffPage, + staffApi, + }, testInfo) => { + const visitors = [ + visitorFor(testInfo.parallelIndex, VISITOR_SLOTS.group.a), + visitorFor(testInfo.parallelIndex, VISITOR_SLOTS.group.b), + ]; + const reason = uniqueTitle('E2E Group Visit'); + const created_ids: number[] = []; + + await sweep(staffApi, visitors.map((v) => v.email)); + await useSettings(staffPage, GROUP_VISITOR_MODE); + + try { + const bookings = await inviteVisitorsViaUI(staffPage, staffApi, visitors, reason); + created_ids.push(...bookings.map((b) => b.id)); + + const containers = bookings.filter((b) => b.booking_type === 'group'); + const members = bookings.filter((b) => b.booking_type === 'visitor'); + + expect(containers, 'exactly one group container was created').toHaveLength(1); + expect(members, 'one visitor booking per invited visitor').toHaveLength( + visitors.length, + ); + + // Assert on `extension_data`, not `asset_name`. The UI sets + // `asset_name: 'Group Booking'` when it builds the container, but + // staff-api's Booking model has no such column and never echoes it + // back — asserting on it compares against `undefined` and passes or + // fails for the wrong reason. + const container = containers[0]; + expect( + container.extension_data?.group_resource_type, + 'the container records what kind of group it holds', + ).toBe('visitor'); + expect( + container.extension_data?.group, + 'the container carries the group reference its members share', + ).toBeTruthy(); + + // The link between members and container is the whole point of a + // group container — PPT-2638 is exactly this going wrong. + for (const member of members) { + expect( + member.parent_id, + `visitor booking ${member.id} must point at the group container`, + ).toBe(container.id); + } + + // Each visitor against THEIR OWN address, not the host's and not the + // first visitor's (PPT-2635). + expect( + members.map((m) => m.asset_id).sort(), + 'every visitor is stored against their own address', + ).toEqual(visitors.map((v) => v.email).sort()); + + // And a guest record each — the half reception actually reads. + for (const v of visitors) { + expect( + (await searchGuests(staffApi, v.email)).map((g) => g.email), + `a guest record was created for ${v.email}`, + ).toContain(v.email); + } + } finally { + for (const id of created_ids) await deleteBooking(staffApi, id); + for (const v of visitors) await deleteGuest(staffApi, v.email); + } + }); + + test('the visitor listing returns the members but not the container', async ({ + staffPage, + staffApi, + }, testInfo) => { + const visitors = [ + visitorFor(testInfo.parallelIndex, VISITOR_SLOTS.group.a), + visitorFor(testInfo.parallelIndex, VISITOR_SLOTS.group.b), + ]; + const reason = uniqueTitle('E2E Group Listing'); + const created_ids: number[] = []; + + await sweep(staffApi, visitors.map((v) => v.email)); + await useSettings(staffPage, GROUP_VISITOR_MODE); + + try { + const bookings = await inviteVisitorsViaUI(staffPage, staffApi, visitors, reason); + created_ids.push(...bookings.map((b) => b.id)); + + const [container] = bookings.filter((b) => b.booking_type === 'group'); + const members = bookings.filter((b) => b.booking_type === 'visitor'); + expect( + container, + 'precondition: the group invite produced a container booking', + ).toBeTruthy(); + + const listed = ( + await listBookings(staffApi, 'visitor', window_from(), window_to()) + ).map((b) => b.id); + + for (const member of members) { + expect(listed, `member booking ${member.id} is in the listing`).toContain( + member.id, + ); + } + expect( + listed, + 'the container is `booking_type: group`, so a visitor-typed listing must ' + + 'not include it — code that expects N+1 here is counting the ' + + 'container as a visitor', + ).not.toContain(container.id); + } finally { + for (const id of created_ids) await deleteBooking(staffApi, id); + for (const v of visitors) await deleteGuest(staffApi, v.email); + } + }); +}); diff --git a/apps/workplace/e2e/local/visitor-invite.spec.ts b/apps/workplace/e2e/local/visitor-invite.spec.ts new file mode 100644 index 0000000000..3cf1eccf66 --- /dev/null +++ b/apps/workplace/e2e/local/visitor-invite.spec.ts @@ -0,0 +1,163 @@ +/** + * VIS-01 / VIS-02 — invite a visitor through the full UI, as a NON-ADMIN, + * against a real local backend. + * + * Deliberately the same shape as `desk-booking.spec.ts`: two tests, one that + * creates through the real form and asserts hard against what the backend + * stored, one that proves teardown really tears down. Assertions are folded + * into the creating test rather than split across more tests, because what is + * being guarded is a single flow — splitting it would drive the same form three + * times to check three fields. + * + * Three things make visitors different from desks, each of which produced a + * wrong assumption before it was checked against the running stack: + * + * - A visitor is NOT an exclusive asset. The backend accepts the same visitor + * twice in the same slot (VIS-B1), so a leftover booking cannot block a + * later run the way a leaked all-day desk booking does. Per-worker, per-slot + * addresses are about keeping sweeps disjoint, not about avoiding a clash. + * - A GUEST record is created alongside the booking, from its `attendees`, and + * it OUTLIVES the booking. Teardown has to remove both, or "was a guest + * created?" starts passing on the last run's data. + * - The form has two modes and the app default is the MULTI-visitor one. This + * file covers single mode, so it pins the setting per test rather than + * trusting a default a settings change could flip underneath it. + */ +import { test, expect } from '../../../../e2e/support/fixtures'; +import { + currentUser, + deleteBooking, + getBooking, + listBookings, + uniqueTitle, +} from '../../../../e2e/support/api'; +import { VISITOR_SLOTS, visitorFor } from '../../../../e2e/support/visitor/visitor.env'; +import { + deleteGuest, + releaseVisitor, + searchGuests, +} from '../../../../e2e/support/visitor/visitor.api'; +import { + SINGLE_VISITOR_MODE, + useSettings, +} from '../../../../e2e/support/visitor/visitor.settings'; +import { inviteVisitorViaUI } from '../../../../e2e/support/visitor/visitor.flows'; + +/** Window wide enough to cover an invite made in any timezone. */ +const DAY = 86_400; +const window_from = () => Math.floor(Date.now() / 1000) - 2 * DAY; +const window_to = () => Math.floor(Date.now() / 1000) + 2 * DAY; + +test.describe('visitor invite (single)', () => { + test('a non-admin invites a visitor in the UI and the backend stores it', async ({ + staffPage, + staffApi, + }, testInfo) => { + const visitor = visitorFor(testInfo.parallelIndex, VISITOR_SLOTS.invite.stores); + const reason = uniqueTitle('E2E Visit'); + let booking_id: number | undefined; + + const swept = await releaseVisitor( + staffApi, + visitor.email, + window_from(), + window_to(), + ); + if (swept) console.log(` swept ${swept} stale invite(s) for ${visitor.email}`); + + // Prove the sweep worked before claiming the invite created anything — a + // guest left over from an earlier run would make the check below pass + // for free. + expect( + await searchGuests(staffApi, visitor.email), + 'precondition: no guest record exists before the invite', + ).toHaveLength(0); + + await useSettings(staffPage, SINGLE_VISITOR_MODE); + + try { + const created = await inviteVisitorViaUI(staffPage, staffApi, visitor, reason); + booking_id = created.id; + + expect(created.id, 'the API returned a booking id').toBeTruthy(); + + // Read it back independently of the response we just parsed. + const stored = await getBooking(staffApi, booking_id); + const me = await currentUser(staffApi); + expect(stored.booking_type, 'stored as a visitor booking').toBe('visitor'); + expect(stored.asset_id, 'against this worker’s visitor address').toBe( + visitor.email, + ); + expect(stored.title, 'the reason we typed reached the backend').toBe(reason); + expect(stored.deleted, 'not soft-deleted').toBeFalsy(); + expect(stored.rejected, 'not rejected').toBeFalsy(); + expect( + stored.zones.length, + 'carries its zone hierarchy (org/building/level)', + ).toBeGreaterThan(0); + // With no host chosen the signed-in user is both host and booker. + // `visitor-delegate.spec.ts` covers the case where they differ. + expect(stored.user_email, 'the signed-in user is the host').toBe(me.email); + expect(stored.booked_by_email, 'and also the booker').toBe(me.email); + + // A guest record is the half of this flow that lives outside the + // booking, and it is what reception actually reads. + const guests = await searchGuests(staffApi, visitor.email); + expect( + guests.map((g) => g.email), + 'a guest record was created for the visitor', + ).toContain(visitor.email); + expect(guests[0].name, 'carrying the name we typed').toBe(visitor.name); + + // ...and the booking is discoverable through the listing the app + // uses, not only by direct id lookup. + const listed = await listBookings( + staffApi, + 'visitor', + window_from(), + window_to(), + ); + expect( + listed.map((b) => b.id), + 'the new invite appears in the visitor listing', + ).toContain(booking_id); + } finally { + if (booking_id != null) await deleteBooking(staffApi, booking_id); + await deleteGuest(staffApi, visitor.email); + } + }); + + test('a deleted visitor invite leaves the listing', async ({ + staffPage, + staffApi, + }, testInfo) => { + const visitor = visitorFor(testInfo.parallelIndex, VISITOR_SLOTS.invite.cleanup); + const reason = uniqueTitle('E2E Visit Cleanup'); + + await releaseVisitor(staffApi, visitor.email, window_from(), window_to()); + await useSettings(staffPage, SINGLE_VISITOR_MODE); + + const created = await inviteVisitorViaUI(staffPage, staffApi, visitor, reason); + try { + expect( + ( + await listBookings(staffApi, 'visitor', window_from(), window_to()) + ).map((b) => b.id), + 'precondition: the invite is in the listing before we delete it', + ).toContain(created.id); + + await deleteBooking(staffApi, created.id); + + expect( + ( + await listBookings(staffApi, 'visitor', window_from(), window_to()) + ).map((b) => b.id), + 'a deleted invite must not come back in the listing — otherwise every ' + + 'spec teardown silently leaks state into the next run', + ).not.toContain(created.id); + } finally { + await deleteBooking(staffApi, created.id); + await deleteGuest(staffApi, visitor.email); + } + }); +}); diff --git a/apps/workplace/e2e/local/visitor-scoping.spec.ts b/apps/workplace/e2e/local/visitor-scoping.spec.ts new file mode 100644 index 0000000000..881a38b2d5 --- /dev/null +++ b/apps/workplace/e2e/local/visitor-scoping.spec.ts @@ -0,0 +1,180 @@ +/** + * VIS-05 / VIS-06 — one user's visitor invites are not another user's business. + * + * The desk equivalent is `booking-scoping.spec.ts`, and this deliberately + * mirrors it, because the property it locks down is the same one and it was + * 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, because the admin was only ever being shown their own. + * + * Visitors raise the stakes over desks. A desk booking leaks where a colleague + * sits; a visitor invite leaks WHO IS COMING TO SEE THEM, plus that person's + * name, email and company. It is also the surface a receptionist-style + * integration is most likely to be built against, so a regression here would + * leak quietly rather than fail loudly. + * + * Everything is created through the API, not the form: the subject is + * authorisation, and driving the UI would only add a way for the test to fail + * for an unrelated reason. + */ +import { request } from '@playwright/test'; +import { test, expect } from '../../../../e2e/support/fixtures'; +import { APP_URL, BACKEND_URL, WORKERS, roleFor } from '../../../../e2e/support/env'; +import { mintToken } from '../../../../e2e/support/auth'; +import { + STAFF_API, + currentUser, + deleteBooking, + listBookings, + uniqueTitle, + zonesWithTag, +} from '../../../../e2e/support/api'; +import { VISITOR_SLOTS, visitorFor } from '../../../../e2e/support/visitor/visitor.env'; +import { + createBookingViaApi, + deleteGuest, + releaseVisitor, + searchGuests, +} from '../../../../e2e/support/visitor/visitor.api'; +import type { APIRequestContext } from '@playwright/test'; + +const DAY = 86_400; +// Window comfortably wider than any invite 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; + +/** The org/building/level zone ids an invite carries, as the UI sends them. */ +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); +} + +/** Create a visitor invite directly, returning the stored booking. */ +async function inviteViaApi( + api: APIRequestContext, + visitor: { email: string; name: string }, + title: string, +) { + const me = await currentUser(api); + const zones = await bookingZones(api); + const start = Math.floor(Date.now() / 1000) + DAY; + + // Via the REG-09-tolerant helper: the backend drops concurrent booking + // writes on this stack, and a scoping spec failing for that reason tells + // nobody anything about scoping. + return createBookingViaApi(api, { + booking_type: 'visitor', + asset_id: visitor.email, + asset_name: visitor.name, + booking_start: start, + booking_end: start + 3600, + timezone: 'Etc/UTC', + user_email: me.email, + user_id: me.id, + user_name: me.name, + title, + zones, + attendees: [{ name: visitor.name, email: visitor.email }], + }); +} + +test.describe('visitor invite visibility between users', () => { + test('another user cannot see or delete your visitor invite', 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 visitor = visitorFor(mine, VISITOR_SLOTS.scoping.own); + const title = uniqueTitle('E2E Visitor Scoping'); + await releaseVisitor(staffApi, visitor.email, from(), to()); + + const me = await currentUser(staffApi); + const booking = await inviteViaApi(staffApi, visitor, title); + + 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 currentUser(other); + expect(them.email, 'the second user must be a different person').not.toBe( + me.email, + ); + + // The privacy boundary. + const their_view = await listBookings(other, 'visitor', from(), to()); + expect( + their_view.map((b) => b.id), + "another user's listing must not include your visitor invite", + ).not.toContain(booking.id); + + // And they cannot remove it. A 2xx here would mean anyone can cancel + // anyone's visitor, 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 invite ` + + `(got ${their_delete.status()})`, + ).toBeGreaterThanOrEqual(400); + + // Still there afterwards, from the owner's point of view. + const still_mine = await listBookings(staffApi, 'visitor', from(), to()); + expect( + still_mine.map((b) => b.id), + 'the invite should survive the other user attempting to delete it', + ).toContain(booking.id); + } finally { + await other.dispose(); + await deleteBooking(staffApi, booking.id); + await deleteGuest(staffApi, visitor.email); + } + }); + + test('you can see your own visitor invite and its guest record', async ({ + staffApi, + }, testInfo) => { + // The control for the test above. Without it, "they cannot see it" would + // also pass if nobody could see anything — which is exactly how an + // earlier leak-check managed to be reassuring and wrong. + const visitor = visitorFor(testInfo.parallelIndex, VISITOR_SLOTS.scoping.control); + const title = uniqueTitle('E2E Visitor Scoping Control'); + await releaseVisitor(staffApi, visitor.email, from(), to()); + + const booking = await inviteViaApi(staffApi, visitor, title); + + try { + const mine = await listBookings(staffApi, 'visitor', from(), to()); + expect( + mine.map((b) => b.id), + 'you must be able to see your own invite', + ).toContain(booking.id); + + expect( + (await searchGuests(staffApi, visitor.email)).map((g) => g.email), + 'and the guest record it created', + ).toContain(visitor.email); + } finally { + await deleteBooking(staffApi, booking.id); + await deleteGuest(staffApi, visitor.email); + } + }); +}); diff --git a/apps/workplace/e2e/local/visitor-settings.spec.ts b/apps/workplace/e2e/local/visitor-settings.spec.ts new file mode 100644 index 0000000000..67253d7bbf --- /dev/null +++ b/apps/workplace/e2e/local/visitor-settings.spec.ts @@ -0,0 +1,131 @@ +/** + * VIS-07 / VIS-08 — settings that change what the invite form demands. + * + * Both of these are off by default, and both are the kind of thing that only + * breaks for the customers who turned them on — which is exactly the class of + * bug an e2e suite is for. `visitors.reason_required` is PPT-2782. + * + * The settings are applied per BROWSER CONTEXT rather than seeded onto the org + * zone (see `visitor.settings.ts`). Two specs can therefore demand opposite + * configurations and still run in parallel; seeding them centrally would force + * the whole suite to serialise on shared state. + * + * Both tests drive the real form, because the point is the form's behaviour. + * The assertions are still made against what the backend stored. + */ +import { test, expect } from '../../../../e2e/support/fixtures'; +import { deleteBooking, getBooking, uniqueTitle } from '../../../../e2e/support/api'; +import { VISITOR_SLOTS, visitorFor } from '../../../../e2e/support/visitor/visitor.env'; +import { deleteGuest, releaseVisitor } from '../../../../e2e/support/visitor/visitor.api'; +import { + ALLOW_ALL_DAY, + REASON_REQUIRED, + SINGLE_VISITOR_MODE, + useSettings, +} from '../../../../e2e/support/visitor/visitor.settings'; +import { InviteVisitorForm } from '../../../../e2e/support/visitor/invite-form.page'; +import { inviteVisitorViaUI } from '../../../../e2e/support/visitor/visitor.flows'; + +const DAY = 86_400; +const window_from = () => Math.floor(Date.now() / 1000) - 2 * DAY; +const window_to = () => Math.floor(Date.now() / 1000) + 2 * DAY; + +test.describe('visitor invite settings', () => { + test('an all-day invite is stored as all-day', async ({ staffPage, staffApi }, testInfo) => { + const visitor = visitorFor(testInfo.parallelIndex, VISITOR_SLOTS.settings.allDay); + const reason = uniqueTitle('E2E Visit All Day'); + let booking_id: number | undefined; + + await releaseVisitor(staffApi, visitor.email, window_from(), window_to()); + await useSettings(staffPage, { ...SINGLE_VISITOR_MODE, ...ALLOW_ALL_DAY }); + + try { + const created = await inviteVisitorViaUI(staffPage, staffApi, visitor, reason, { + allDay: true, + }); + booking_id = created.id; + + const stored = await getBooking(staffApi, booking_id); + expect(stored.all_day, 'the backend recorded it as an all-day booking').toBe(true); + + // An all-day invite should span a real day, not the one-hour default + // the form starts with. Asserting the flag alone would pass even if + // the times never changed. + expect( + stored.booking_end - stored.booking_start, + 'an all-day invite covers far more than the default hour', + ).toBeGreaterThan(8 * 3600); + } finally { + if (booking_id != null) await deleteBooking(staffApi, booking_id); + await deleteGuest(staffApi, visitor.email); + } + }); + + test('with a reason required, sending without one is refused', async ({ + staffPage, + staffApi, + }, testInfo) => { + const visitor = visitorFor(testInfo.parallelIndex, VISITOR_SLOTS.settings.reason); + + await releaseVisitor(staffApi, visitor.email, window_from(), window_to()); + await useSettings(staffPage, { ...SINGLE_VISITOR_MODE, ...REASON_REQUIRED }); + + const form = new InviteVisitorForm(staffPage); + await form.open(); + await form.expectSingleMode(); + + // With the setting on, the component starts the reason EMPTY. Without it + // the reason defaults to "Visit", so a spec that forgot to clear the + // field would send successfully and prove nothing. + await expect( + form.reason, + 'with reason_required the field must start empty, or this test is vacuous', + ).toHaveValue(''); + + const posts: number[] = []; + staffPage.on('response', (r) => { + if (r.url().includes('/api/staff/v1/bookings') && r.request().method() === 'POST') { + posts.push(r.status()); + } + }); + + await expect(async () => { + await form.visitorName.fill(visitor.name); + await form.visitorEmail.fill(visitor.email); + expect(await form.visitorEmail.inputValue()).toBe(visitor.email); + }).toPass({ timeout: 30_000 }); + + await form.sendButton.click(); + + // The form must refuse locally. Give it long enough that a slow POST + // would still have been seen — asserting "no request yet" immediately + // after a click would pass even if one were on its way. + await staffPage.waitForTimeout(5_000); + + expect( + posts, + 'a missing reason must be caught in the form — no booking POST should be made', + ).toHaveLength(0); + await expect( + form.successPanel, + 'and the invite must not reach its success screen', + ).toBeHidden(); + + // Control: the same form DOES send once a reason is supplied, so the + // assertion above is about the reason and not about a form that was + // broken for some unrelated reason. + let booking_id: number | undefined; + try { + const reason = uniqueTitle('E2E Visit Reason'); + const created = await inviteVisitorViaUI(staffPage, staffApi, visitor, reason); + booking_id = created.id; + expect( + (await getBooking(staffApi, booking_id)).title, + 'with a reason supplied the invite goes through and keeps it', + ).toBe(reason); + } finally { + if (booking_id != null) await deleteBooking(staffApi, booking_id); + await deleteGuest(staffApi, visitor.email); + } + }); +}); diff --git a/apps/workplace/e2e/local/visitor-times.spec.ts b/apps/workplace/e2e/local/visitor-times.spec.ts new file mode 100644 index 0000000000..c71e4b6fb2 --- /dev/null +++ b/apps/workplace/e2e/local/visitor-times.spec.ts @@ -0,0 +1,136 @@ +/** + * VIS-22 / VIS-23 — when the visit is, and the limits on what may be chosen. + * + * Test 1 is the plain question nothing else in this suite asks: if a user picks + * a start time and a length, are THOSE the times that get stored? Every other + * spec takes the form's default hour, so a form that quietly ignored the picker + * would still have passed everywhere. + * + * Test 2 covers the two settings that fence the picker in — a maximum visit + * length, and the hours of the day a visit may start. Both are unset by default + * so nothing else here sees them, and both fail in the same quiet way: the + * option is simply absent rather than refused, so there is nothing to click and + * no error to read. That is why this asserts on the options OFFERED. Checking + * "a long visit is rejected" would be testing something the form never lets you + * attempt. + */ +import { test, expect } from '../../../../e2e/support/fixtures'; +import { deleteBooking, getBooking, uniqueTitle } from '../../../../e2e/support/api'; +import { VISITOR_SLOTS, visitorFor } from '../../../../e2e/support/visitor/visitor.env'; +import { deleteGuest, releaseVisitor } from '../../../../e2e/support/visitor/visitor.api'; +import { + LIMITED_HOURS, + SINGLE_VISITOR_MODE, + useSettings, +} from '../../../../e2e/support/visitor/visitor.settings'; +import { InviteVisitorForm } from '../../../../e2e/support/visitor/invite-form.page'; +import { inviteVisitorViaUI } from '../../../../e2e/support/visitor/visitor.flows'; + +const DAY = 86_400; +const window_from = () => Math.floor(Date.now() / 1000) - 2 * DAY; +const window_to = () => Math.floor(Date.now() / 1000) + 2 * DAY; + +const MAX_MINUTES = LIMITED_HOURS['app.visitors.max_duration']; +const HOURS = LIMITED_HOURS['app.visitors.bookable_hours']; + +/** "HH:mm" as minutes past midnight, for comparing option values. */ +function minutesOfDay(hhmm: string): number { + const [h, m] = hhmm.split(':').map(Number); + return h * 60 + m; +} + +test.describe('visitor invite times', () => { + test('the start time and length chosen on the form are what get stored', async ({ + staffPage, + staffApi, + }, testInfo) => { + const visitor = visitorFor(testInfo.parallelIndex, VISITOR_SLOTS.times.chosen); + const reason = uniqueTitle('E2E Visit Times'); + let booking_id: number | undefined; + + await releaseVisitor(staffApi, visitor.email, window_from(), window_to()); + await useSettings(staffPage, SINGLE_VISITOR_MODE); + + try { + const form = new InviteVisitorForm(staffPage); + await form.open(); + await form.expectSingleMode(); + + // Pick from what the form itself offers rather than naming an hour: + // the options run on a fixed step from the current time, so a + // hardcoded "14:30" is a test that stops working after 14:30. + const times = await form.startTimeOptions(); + expect( + times.length, + 'the form offered no start times at all — nothing can be chosen', + ).toBeGreaterThan(1); + // Second option, not the first: the first is usually the default + // already selected, so choosing it would prove nothing. + const chosen_time = times[1]; + const lengths = await form.durationOptions(); + const chosen_length = lengths.find((m) => m >= 90) ?? lengths[lengths.length - 1]; + + // Set through the flow rather than here, so a REG-09 retry re-picks + // them instead of falling back to the defaults. + const created = await inviteVisitorViaUI(staffPage, staffApi, visitor, reason, { + startTime: chosen_time, + duration: chosen_length, + }); + booking_id = created.id; + + const stored = await getBooking(staffApi, booking_id); + const start = new Date(stored.booking_start * 1000); + const start_minutes = start.getHours() * 60 + start.getMinutes(); + expect( + start_minutes, + `the visit must start at the ${chosen_time} that was picked, not at the ` + + `form's default`, + ).toBe(minutesOfDay(chosen_time)); + expect( + (stored.booking_end - stored.booking_start) / 60, + `and run for the ${chosen_length} minutes that were picked`, + ).toBe(chosen_length); + } finally { + if (booking_id != null) await deleteBooking(staffApi, booking_id); + await deleteGuest(staffApi, visitor.email); + } + }); + + test('a maximum length and bookable hours limit what can be chosen', async ({ + staffPage, + }) => { + await useSettings(staffPage, { ...SINGLE_VISITOR_MODE, ...LIMITED_HOURS }); + + const form = new InviteVisitorForm(staffPage); + await form.open(); + await form.expectSingleMode(); + + const lengths = await form.durationOptions(); + expect( + lengths.length, + 'the form offered no visit lengths at all, so the cap cannot be judged', + ).toBeGreaterThan(0); + expect( + Math.max(...lengths), + `with a ${MAX_MINUTES} minute cap, nothing longer may be offered — the form ` + + `is the only place this is enforced, the backend will store any length`, + ).toBeLessThanOrEqual(MAX_MINUTES); + + const times = await form.startTimeOptions(); + expect( + times.length, + 'the form offered no start times at all, so the hours cannot be judged', + ).toBeGreaterThan(0); + const earliest = Math.min(...times.map(minutesOfDay)); + const latest = Math.max(...times.map(minutesOfDay)); + expect( + earliest, + `no visit may start before ${HOURS.start}:00 once bookable hours are set`, + ).toBeGreaterThanOrEqual(HOURS.start * 60); + expect( + latest, + `nor after ${HOURS.end}:00 — an option outside the window is one a user ` + + 'can pick and then be turned away at the door for', + ).toBeLessThanOrEqual(HOURS.end * 60); + }); +}); diff --git a/apps/workplace/e2e/local/visitor-validation.spec.ts b/apps/workplace/e2e/local/visitor-validation.spec.ts new file mode 100644 index 0000000000..4b00b0a9bd --- /dev/null +++ b/apps/workplace/e2e/local/visitor-validation.spec.ts @@ -0,0 +1,143 @@ +/** + * VIS-18 / VIS-19 — what the form refuses to send, and who it sends. + * + * Both tests are about the moment BEFORE the request: the invite form is the + * only thing standing between a typo and a visitor who never gets told they are + * expected. Neither can be judged from the screen, so both watch the network — + * the question is not "did a message appear" but "did anything leave". + * + * `asset_id` carries the visitor's address on a visitor booking and has an + * `email` validator on it (`booking.utilities.ts`), which is what test 1 + * exercises. The backend would happily store `not-an-email`, so if the form + * lets it through nothing else will stop it. + */ +import { test, expect } from '../../../../e2e/support/fixtures'; +import { deleteBooking, getBooking, listBookings, uniqueTitle } from '../../../../e2e/support/api'; +import { VISITOR_SLOTS, visitorFor } from '../../../../e2e/support/visitor/visitor.env'; +import { deleteGuest, releaseVisitor, releaseGroupContainers } from '../../../../e2e/support/visitor/visitor.api'; +import { + GROUP_VISITOR_MODE, + SINGLE_VISITOR_MODE, + useSettings, +} from '../../../../e2e/support/visitor/visitor.settings'; +import { + inviteVisitorViaUI, + inviteVisitorsViaUI, +} from '../../../../e2e/support/visitor/visitor.flows'; +import { InviteVisitorForm } from '../../../../e2e/support/visitor/invite-form.page'; + +const DAY = 86_400; +const window_from = () => Math.floor(Date.now() / 1000) - 2 * DAY; +const window_to = () => Math.floor(Date.now() / 1000) + 2 * DAY; + +test.describe('visitor invite validation', () => { + test('an address that is not an email is refused before anything is sent', async ({ + staffPage, + staffApi, + }, testInfo) => { + const visitor = visitorFor(testInfo.parallelIndex, VISITOR_SLOTS.validation.badEmail); + + await releaseVisitor(staffApi, visitor.email, window_from(), window_to()); + await useSettings(staffPage, SINGLE_VISITOR_MODE); + + const form = new InviteVisitorForm(staffPage); + await form.open(); + await form.expectSingleMode(); + + const posts: number[] = []; + staffPage.on('response', (r) => { + if (r.url().includes('/api/staff/v1/bookings') && r.request().method() === 'POST') { + posts.push(r.status()); + } + }); + + // Everything else valid, so a refusal can only be about the address. + await expect(async () => { + await form.visitorName.fill(visitor.name); + await form.visitorEmail.fill('not-an-email'); + await form.reason.fill(uniqueTitle('E2E Bad Email')); + expect(await form.visitorEmail.inputValue()).toBe('not-an-email'); + }).toPass({ timeout: 30_000 }); + + await form.sendButton.click(); + // Long enough that a slow POST would still have been seen. Asserting + // "nothing yet" straight after the click would pass either way. + await staffPage.waitForTimeout(5_000); + + expect( + posts, + 'a malformed address must be caught in the form — the backend stores ' + + 'whatever it is given, so nothing else will catch it', + ).toHaveLength(0); + await expect( + form.successPanel, + 'and the invite must not reach its success screen', + ).toBeHidden(); + + // Control: the same form sends once the address is a real one. Without + // this, a form broken for any other reason would pass the test above. + let booking_id: number | undefined; + try { + const reason = uniqueTitle('E2E Good Email'); + const created = await inviteVisitorViaUI(staffPage, staffApi, visitor, reason); + booking_id = created.id; + expect( + (await getBooking(staffApi, booking_id)).asset_id, + 'with a valid address the same invite goes through', + ).toBe(visitor.email); + } finally { + if (booking_id != null) await deleteBooking(staffApi, booking_id); + await deleteGuest(staffApi, visitor.email); + } + }); + + test('a visitor removed before sending is not invited', async ({ + staffPage, + staffApi, + }, testInfo) => { + const kept = visitorFor(testInfo.parallelIndex, VISITOR_SLOTS.validation.keptA); + const dropped = visitorFor(testInfo.parallelIndex, VISITOR_SLOTS.validation.removedB); + const reason = uniqueTitle('E2E Visit Removed'); + const created_ids: number[] = []; + + for (const v of [kept, dropped]) { + await releaseVisitor(staffApi, v.email, window_from(), window_to()); + } + await releaseGroupContainers(staffApi, window_from(), window_to()); + await useSettings(staffPage, GROUP_VISITOR_MODE); + + try { + // Both go on the list, then one comes off — all inside the send + // flow, so a REG-09 retry repeats the correction instead of + // quietly inviting the person who was taken off. + const created = await inviteVisitorsViaUI( + staffPage, + staffApi, + [kept, dropped], + reason, + { remove: [dropped] }, + ); + created_ids.push(...created.map((b) => b.id)); + + const members = created.filter((b) => b.booking_type === 'visitor'); + expect( + members.map((b) => b.asset_id), + 'only the visitor left on the list is invited', + ).toEqual([kept.email]); + + // And nothing for the removed one anywhere in the window — a + // booking created and then orphaned would not show up above. + const live = ( + await listBookings(staffApi, 'visitor', window_from(), window_to()) + ).filter((b) => !b.deleted); + expect( + live.map((b) => b.asset_id), + 'the visitor taken off the list must have no booking at all', + ).not.toContain(dropped.email); + } finally { + for (const id of created_ids) await deleteBooking(staffApi, id).catch(() => null); + await releaseGroupContainers(staffApi, window_from(), window_to()); + for (const v of [kept, dropped]) await deleteGuest(staffApi, v.email).catch(() => null); + } + }); +}); diff --git a/e2e/README.md b/e2e/README.md index eabbe1560a..eee525b7be 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -81,6 +81,14 @@ e2e/ shared engine, all apps fixtures.ts worker-scoped auth fixtures — import test/expect from here api.ts engine/staff-api helpers + asset sweep flows.ts multi-step UI flows (bookDeskViaUI) + visitor/ visitor-invite coverage, kept out of the files above + visitor.env.ts per-worker visitor addresses + slot table + visitor.api.ts guest + visitor-booking helpers and sweeps + visitor.settings.ts per-context app settings presets + invite-form.page.ts locators for the invite form + your-bookings.page.ts locators for the schedule + details modal + visitor.flows.ts multi-step UI flows (inviteVisitorViaUI) + repro/ standalone reproducers for confirmed backend bugs preflight.ts "is the stack up?" — fails in 1s, not 90 preflight.setup.ts setup project — `local` depends on it, `mock` does not seed.ts idempotent API-driven seeding diff --git a/e2e/VISITOR_E2E_HANDOVER.md b/e2e/VISITOR_E2E_HANDOVER.md new file mode 100644 index 0000000000..45939ad932 --- /dev/null +++ b/e2e/VISITOR_E2E_HANDOVER.md @@ -0,0 +1,197 @@ +# Visitor invite e2e — handover + +Working notes for the visitor-invite coverage added alongside the existing desk +specs. Written to be picked up cold. Coverage contract lives in +[`../E2E_USER_STORIES.md`](../E2E_USER_STORIES.md); harness conventions in +[`README.md`](README.md). + +**Status: 22 of 23 tests passing. 1 blocked on an app bug (VIS-B8).** +Best full suite run 2026-09-15: **36 passed, 1 skipped, 0 failed** in 1.7 min. +Two of four runs that afternoon also showed the PRE-EXISTING desk flake +(REG-10, "Require locker" — inside `bookDeskViaUI`, untouched here). It is +appearing more often as the suite grows, which is a CI problem waiting to happen: +more specs means more parallel load, and that race is won by whoever is faster. +Rows for all of them are in [`../E2E_USER_STORIES.md`](../E2E_USER_STORIES.md) (§1a), along +with the REG-09 correction. The check-in specs no longer depend on the time of day. +Nothing in the pre-existing suite was modified — `git diff` on +`e2e/support/{env,api,flows}.ts` is empty. + +### Known flake in the PRE-EXISTING desk suite (not introduced here) + +`desk-booking.spec.ts` "a deleted desk booking leaves the listing" failed once +on 2026-09-14 with: + +``` +Error: "Require locker" should be unchecked +expect(locator).not.toBeChecked() failed +``` + +That is inside the original `bookDeskViaUI` helper, which this work does not +touch. It is the REG-10 / PPT-2643 form-rebuild race the coverage doc already +describes — the rebuild restores "Require locker" to checked. Mentioned here only +so a red desk test after a visitor run is not misread as collateral damage. + +--- + +## How to run + +```bash +e2e/stack/up.sh # if the stack is not already up +export E2E_BACKEND_URL=https://localhost:9443 +bun run e2e:typecheck # NEW — nothing else type-checks the specs +bunx playwright test --config apps/workplace/playwright.config.ts +``` + +Visitor specs only: + +```bash +bunx playwright test --config apps/workplace/playwright.config.ts \ + --project=local apps/workplace/e2e/local/visitor-*.spec.ts +``` + +--- + +## What was built + +Eight new files, no existing file changed except one line in `package.json` +(the `e2e:typecheck` script) and `e2e/tsconfig.json` (new). + +### Support — `e2e/support/visitor/` + +| File | Holds | +|---|---| +| `calendar.ts` | driving the shared `date-calendar` widget (schedule sidebar AND the form's date pop-up) | +| `visitor.env.ts` | `visitorFor(worker, slot)` + the `VISITOR_SLOTS` table | +| `visitor.api.ts` | guest search/delete, sweeps, `createBookingViaApi`, `checkInViaApi`, `isReg09` | +| `visitor.settings.ts` | `useSettings()` + named mode presets | +| `invite-form.page.ts` | page object for `/#/book/visitor/form` | +| `your-bookings.page.ts` | page object for `/#/your-bookings` + details modal | +| `visitor.flows.ts` | `inviteVisitorViaUI`, `inviteVisitorsViaUI` | + +Kept out of the shared support files deliberately, so nothing visitor-shaped can +break the desk specs. + +### Specs — `apps/workplace/e2e/local/` + +| File | Tests | State | +|---|---|---| +| `visitor-invite.spec.ts` | 2 | passing | +| `visitor-group.spec.ts` | 2 | passing | +| `visitor-scoping.spec.ts` | 2 | passing | +| `visitor-settings.spec.ts` | 2 | passing | +| `visitor-delegate.spec.ts` | 2 | passing | +| `visitor-checkin.spec.ts` | 2 | passing, at any hour — test 1 now covers check-in **and** check-out | +| `visitor-edit.spec.ts` | 3 | 2 passing (multi-visitor mode, one of them future-dated); 1 **`fixme`** — single-visitor mode, VIS-B8 | +| `visitor-cancel.spec.ts` | 2 | passing — cancelling from the app, and declining the confirmation | +| `visitor-validation.spec.ts` | 2 | passing — a bad address is refused; a visitor removed before sending is not invited | +| `visitor-details.spec.ts` | 2 | passing — pass number + international flag; company + guest record name | +| `visitor-times.spec.ts` | 2 | passing — the chosen start/length are stored; limits control what is offered | + +Two tests per file, matching every existing spec file in the suite — except +`visitor-edit`, which carries a third as a placeholder for VIS-B8. Fold it away +when the bug is fixed; it is then the same assertion as test 1 with one setting +changed. + +### Reproducers — `e2e/support/repro/` + +- `vis-b1-duplicate-visitor.ts` — the same visitor booked twice into one slot +- `vis-b6-burst.ts` — concurrent booking POSTs, no browser; proves REG-09 + +Both need the suite to have run once first, so the worker token sidecars exist. + +--- + +## Findings — all confirmed against the running stack + +| ID | Finding | Evidence | +|---|---|---| +| **VIS-B8** | **In SINGLE-visitor mode, saving an edit throws `_.toJSON is not a function` and sends no request.** Multi-visitor mode — the shipped default — is fine. `Booking` declares `attendees` as `User[]` but assigns raw JSON to it; in single mode the edit path copies that into the form's `assets`, and saving calls `.toJSON()` on each entry. The multi paths rebuild people with `new User(...)` first, so they escape it. | all created and edited through the app: single/1 fails, multi/1 and multi/2 save cleanly; same on a future date. Screenshot: `reports/visitor-edit-probe/` | +| **VIS-B9** | **Two overlapping GROUP invites by one host are refused, whoever the visitors are.** The container is named `${host}[${date the invite was CREATED}]`, so all of a host's group invites that day share one asset id and the backend refuses the overlap. Same two visits booked on different days are accepted — which is what shows it is accidental. Do NOT confuse with VIS-B1. | `vis-b9-group-clash.ts`: 201 / **409 Conflicting booking** / 201 / 201 | +| **VIS-B1** | Same visitor bookable twice into one slot — backend returns 201 both times. Desks refuse this. Product says it should be refused. | `vis-b1-duplicate-visitor.ts` | +| **REG-09** | Group invites are the FIRST thing in this suite that bursts concurrent booking writes, so this suite now trips PPT-2642. `E2E_USER_STORIES.md` still says "nothing here bursts concurrent bookings" — **that line is now wrong and should be corrected.** | `vis-b6-burst.ts`: 4 concurrent POSTs x 12 rounds gave 13x 201, 29x 400, 6x 500, clean early then degrading | +| **Host flags** | `visitors.can_book_for_anyone` renders the host picker, but `sendInvite` keeps your choice only if `visitors.can_book_for_others` is ALSO set. Set one without the other and the picker silently discards the host. | `BOOK_FOR_ANYONE` sets both; documented there | +| **Listing limit** | `GET /bookings` defaults to limit 100 and the schedule passes `include_deleted: true`, so once a user has 100+ soft-deleted bookings, new ones stop appearing in their schedule. Broke the card-dependent specs until 428 stale rows were cleared. | counted in Postgres | +| **Deep link** | `?booking=` never opens the details modal: `booking-card.component.ts` checks `params.has('booking')` then compares the id to `params.get('event')` — a param that is not there. | read from source | + +None of these are test defects. + +--- + +## Decisions already taken (so they are not re-litigated) + +- **Workplace only.** The desk specs touch neither concierge nor the kiosk (zero + mentions in `e2e/`), so visitor coverage stays in workplace too. +- **Settings per browser context**, via `localStorage['PLACEOS.setting_overrides']`, + not seeded on the org zone. Single- and multi-visitor mode are the same setting + with two values; seeding it would force the whole suite to serialise. Trade-off + stated in `visitor.settings.ts`. +- **No `seed.ts` change.** A visitor "asset" is just an email, so there is + nothing to provision. This removes the one file where a mistake could have + broken the desk specs. +- **Slots, not shared addresses.** Every test owns a slot in `VISITOR_SLOTS`, + because `releaseVisitor` sweeps by address and two tests sharing one can delete + each other's data. +- **REG-09 retry lives in the flows, not the config.** Confined to visitor specs + and easy to delete after the staff-api upgrade. `submitWhenAccepted` rolls back + a partial group before retrying, since retrying after a partial create would + leave two containers. +- **Selectors never depend on visible text.** All labels are translated. See the + policy blocks at the top of both page objects. + +--- + +## Next steps + +1. ~~`E2E_USER_STORIES.md` rows~~ — **done 2026-09-15.** §1a holds VIS-01 … VIS-14 + and the six findings; the REG-09 row and the notes bullet now say the suite + DOES burst concurrent bookings, and point at the retry in + `visitor.flows.ts` as the thing to delete when the image is updated. +2. ~~`visitor-checkin` time fragility~~ — **done 2026-09-15.** `YourBookingsPage` + grew `showDayOf(timestamp_ms)`, which drives the sidebar calendar to whatever + day a booking falls on, so neither test cares what the clock says any more. + Both skips are gone. Test 2 now books a fixed **10am tomorrow** instead of + "3 hours from now"; test 1 still has to sit 10 minutes from now, because a + check-in only works next to its own start time, but it may now cross midnight + safely. Red-checked: with the `showDayOf` call removed, test 2 fails with + `no card for booking 568 ... 25 card(s) are rendered`. + How it works, since it is the non-obvious part: the calendar grid is 42 + consecutive day cells and only one of them is marked (the "today" ring, drawn + from a value captured at page load). Reading that cell's day-of-month and + comparing it against the browser clock says whether the ring means today or + yesterday, and every other cell is then just counting. That is what makes it + survive the clock crossing midnight mid-test. +3. **File the six findings above.** None are filed yet. Note VIS-B8's priority + depends on an answer nobody has yet: **does any real deployment run + single-visitor mode?** If not, it is low. +4. **Stale booking accumulation.** Each run adds soft-deleted rows; past 100 per + user the card-dependent specs (`delegate`, `checkin`) break again in a way + that looks nothing like the cause. A run on 2026-09-15 already rendered 25 + cards for one worker, so this is accumulating, not hypothetical. Needs either a backend change or a purge + step before this goes near CI. **This is an environment decision, not a spec + change.** + +--- + +## Traps that cost time — do not re-derive + +- **`asset_name` is not returned by staff-api.** The UI sets it on a group + container; the model has no such column. Asserting on it compares against + `undefined`. Assert on `extension_data` instead. +- **A group container is `booking_type: 'group'`**, so `GET /bookings?type=visitor` + returns N members, NOT N+1. `visitor-group` test 2 exists to stop that mistake. +- **Guest records outlive their booking** and are created from `attendees`, not + `asset_id`. Teardown must delete both. +- **Two different check-in gates.** The app hides the button until 15 MINUTES + before the start (`Booking.state` is `'future'` before that); the backend + accepts a check-in within 1 HOUR. Easy to conflate. +- **Playwright text selectors do not match the `` wrapper.** Both + `icon:text-is("edit")` and `filter({ hasText: /^edit$/ })` silently matched + nothing while the element was plainly in the DOM. `menuAction` reads trimmed + `textContent` instead. +- **Two wrong theories about the REG-09 400**, recorded in `visitor.flows.ts`: + "the app posts an empty body" (it sends ~2 KB) and "the form is rebuilt + mid-init and loses its model" (the browser-free burst reproduces it with no + form at all). +- **Add diagnostics before theorising.** Every cause here became obvious in one + run once the failure message reported what was actually present — rendered card + ids, menu contents, request sizes. Guessing at selectors cost far more. diff --git a/e2e/support/repro/vis-b1-duplicate-visitor.ts b/e2e/support/repro/vis-b1-duplicate-visitor.ts new file mode 100644 index 0000000000..4fadb13481 --- /dev/null +++ b/e2e/support/repro/vis-b1-duplicate-visitor.ts @@ -0,0 +1,88 @@ +/** + * VIS-B1 — the same visitor can be booked twice into the same slot. + * + * A desk is exclusive for a time range: a second booking of the same desk over + * the same period is refused, and `desk-clash.spec.ts` locks that in. A visitor + * is not, and as far as we can tell that is not intentional — product's + * expectation is that the backend rejects it. + * + * This script is the minimal reproduction, kept next to the finding the same way + * the REG-09 reproducers are. It creates two identical visitor bookings and + * reports what the backend did, then cleans up after itself. + * + * bunx tsx e2e/support/repro/vis-b1-duplicate-visitor.ts + * + * Expected today: both POSTs return 201. + * Expected after a fix: the second returns 409, as desks do. + */ +import { APIRequestContext } from '@playwright/test'; +import { BACKEND_URL, assertLocalOnly } from '../env'; +import { ENGINE_API, STAFF_API, apiFor, deleteBooking, zonesWithTag } from '../api'; +import { deleteGuest } from '../visitor/visitor.api'; + +const VISITOR = 'vis-b1-repro@example.com'; +const DAY = 86_400; + +/** + * Reuse the bearer the auth fixtures already minted for worker 0. + * + * Minting here directly does NOT work: ts-client derives `client_id` from the + * redirect_uri, and the only OAuth app `seed.ts` registers is the dev server's. + * Pointing a fresh mint at the backend origin asks for a client that was never + * registered and comes back 401. Run the suite once first so the sidecar exists. + */ +async function staffApi(): Promise { + return apiFor('staff', 0); +} + +async function main() { + assertLocalOnly(BACKEND_URL); + const api = await staffApi(); + const created: number[] = []; + try { + const me = await (await api.get(`${ENGINE_API}/users/current`)).json(); + const zones = ( + await Promise.all(['org', 'building', 'level'].map((t) => zonesWithTag(api, t))) + ) + .flat() + .map((z) => z.id); + + const start = Math.floor(Date.now() / 1000) + DAY; + const payload = { + booking_type: 'visitor', + asset_id: VISITOR, + asset_name: 'VIS-B1 Repro', + booking_start: start, + booking_end: start + 3600, + timezone: 'Etc/UTC', + user_email: me.email, + user_id: me.id, + user_name: me.name, + title: 'VIS-B1 duplicate visitor repro', + zones, + }; + + for (const attempt of [1, 2]) { + const res = await api.post(`${STAFF_API}/bookings`, { data: payload }); + const body = await res.text(); + console.log(` attempt ${attempt}: HTTP ${res.status()}`); + if (res.ok()) created.push(JSON.parse(body).id); + else console.log(` ${body}`); + } + + console.log( + created.length === 2 + ? '\nREPRODUCED: both bookings were accepted. A desk would have refused the second.' + : '\nNOT reproduced: the second booking was refused. VIS-B1 may be fixed.', + ); + } finally { + for (const id of created) await deleteBooking(api, id); + await deleteGuest(api, VISITOR); + await api.dispose(); + } +} + +main().catch((e) => { + console.error(e.message); + process.exit(1); +}); diff --git a/e2e/support/repro/vis-b6-burst.ts b/e2e/support/repro/vis-b6-burst.ts new file mode 100644 index 0000000000..352278bff7 --- /dev/null +++ b/e2e/support/repro/vis-b6-burst.ts @@ -0,0 +1,117 @@ +/** + * VIS-B6 probe — is the 400 a FRONTEND problem or a BACKEND one? + * + * Symptom seen through the UI: a `POST /bookings` carrying a real ~2 KB JSON + * body comes back + * + * HTTP 400 {"error":"Expected BeginObject but was EOF at line 1, column 1 + * parsing PlaceOS::Model::Booking at line 0, column 0"} + * + * "EOF at line 1, column 1" means the parser saw an empty input, yet the client + * demonstrably sent 2050 bytes. Two competing explanations: + * + * A. the app sends something malformed (wrong content-type, double encoding) + * B. staff-api loses the body under concurrent writes + * + * This script settles it by removing the browser entirely: it fires N identical, + * well-formed visitor bookings concurrently, straight at the API. If the 400 + * appears here, the app cannot be at fault and (B) is the answer. + * + * bunx tsx e2e/support/repro/vis-b6-burst.ts [concurrency] [rounds] + * + * Related: REG-09 (PPT-2642) is a different failure with the same trigger — + * concurrent booking POSTs against a staff-api image that predates pg-orm 2.2.4. + */ +import { APIRequestContext } from '@playwright/test'; +import { BACKEND_URL, assertLocalOnly } from '../env'; +import { ENGINE_API, STAFF_API, apiFor, deleteBooking, zonesWithTag } from '../api'; +import { deleteGuest } from '../visitor/visitor.api'; + +const CONCURRENCY = Number(process.argv[2] ?? 4); +const ROUNDS = Number(process.argv[3] ?? 10); +const DAY = 86_400; + +/** + * Reuse the bearer the auth fixtures already minted for worker 0. + * + * Minting here directly does NOT work: ts-client derives `client_id` from the + * redirect_uri, and the only OAuth app `seed.ts` registers is the dev server's. + * Pointing a fresh mint at the backend origin asks for a client that was never + * registered and comes back 401. Run the suite once first so the sidecar exists. + */ +async function staffApi(): Promise { + return apiFor('staff', 0); +} + +async function main() { + assertLocalOnly(BACKEND_URL); + const api = await staffApi(); + const created: number[] = []; + const emails: string[] = []; + const statuses = new Map(); + + try { + const me = await (await api.get(`${ENGINE_API}/users/current`)).json(); + const zones = ( + await Promise.all(['org', 'building', 'level'].map((t) => zonesWithTag(api, t))) + ) + .flat() + .map((z) => z.id); + + console.log(`bursting ${CONCURRENCY} concurrent POSTs x ${ROUNDS} rounds\n`); + + for (let round = 0; round < ROUNDS; round++) { + const start = Math.floor(Date.now() / 1000) + DAY + round * 3600; + const results = await Promise.all( + Array.from({ length: CONCURRENCY }, async (_, i) => { + const email = `vis-b6-burst-${round}-${i}@example.com`; + emails.push(email); + const res = await api.post(`${STAFF_API}/bookings`, { + data: { + booking_type: 'visitor', + asset_id: email, + asset_name: `Burst ${round}-${i}`, + booking_start: start, + booking_end: start + 3600, + timezone: 'Etc/UTC', + user_email: me.email, + user_id: me.id, + user_name: me.name, + title: `VIS-B6 burst ${round}-${i}`, + zones, + attendees: [{ name: `Burst ${round}-${i}`, email }], + }, + }); + const text = await res.text(); + if (res.ok()) created.push(JSON.parse(text).id); + return { status: res.status(), text }; + }), + ); + + for (const r of results) { + const key = `${r.status}`; + statuses.set(key, (statuses.get(key) ?? 0) + 1); + if (r.status < 200 || r.status >= 300) { + console.log(` round ${round}: HTTP ${r.status} ${r.text.slice(0, 160)}`); + } + } + } + + console.log('\nstatus counts:', Object.fromEntries(statuses)); + const bad = [...statuses.entries()].filter(([s]) => Number(s) >= 300); + console.log( + bad.length + ? '\nREPRODUCED without a browser — the app is not the cause.' + : '\nNot reproduced at the API level. The trigger involves the browser path.', + ); + } finally { + for (const id of created) await deleteBooking(api, id); + for (const email of emails) await deleteGuest(api, email); + await api.dispose(); + } +} + +main().catch((e) => { + console.error(e.message); + process.exit(1); +}); diff --git a/e2e/support/repro/vis-b9-group-clash.ts b/e2e/support/repro/vis-b9-group-clash.ts new file mode 100644 index 0000000000..9b04ed08e0 --- /dev/null +++ b/e2e/support/repro/vis-b9-group-clash.ts @@ -0,0 +1,110 @@ +/** + * VIS-B9 — a host cannot make two overlapping GROUP invites, even for + * completely different visitors. + * + * Found 2026-09-15 from a user's screenshot of `placeos-dev`: a second invite + * came back `409 {"error":"Conflicting booking","bookings":[{"booking_type": + * "group", ...}]}`. The conflicting row is a GROUP container, not a visitor — + * which is the whole story. + * + * The app names a group container after the HOST and the day the invite was + * CREATED: + * + * asset_id = `${currentUser().email}[${format(Date.now(), 'yyyy-MM-dd')}]` + * (booking-form.service.ts, `_groupName`) + * + * Every group invite that host creates today therefore shares one asset id, and + * the backend refuses overlapping bookings on one asset. So the second invite is + * rejected because of the first one's TIME, regardless of who is being invited. + * + * Note `Date.now()` — the day the invite is MADE, not the day of the visit. That + * is what makes the behaviour arbitrary from a user's point of view, and what + * marks it as accidental rather than a deliberate rule: the same two visits are + * accepted if they are booked on different days (case 4 below). + * + * bunx tsx e2e/support/repro/vis-b9-group-clash.ts + * + * Expected today: 1 -> 201, 2 -> 409, 3 -> 201, 4 -> 201. + * Expected after a fix: 2 -> 201 as well; two unrelated groups may overlap. + */ +import { BACKEND_URL, assertLocalOnly } from '../env'; +import { ENGINE_API, STAFF_API, apiFor, deleteBooking, zonesWithTag } from '../api'; + +async function main() { + assertLocalOnly(BACKEND_URL); + // Reuse the bearer the auth fixtures minted for worker 0 — see the note in + // `vis-b1-duplicate-visitor.ts`. Run the suite once first. + const api = await apiFor('staff', 0); + const created: number[] = []; + try { + const me = await (await api.get(`${ENGINE_API}/users/current`)).json(); + const zones = ( + await Promise.all(['org', 'building', 'level'].map((t) => zonesWithTag(api, t))) + ) + .flat() + .map((z) => z.id); + const today = new Date().toISOString().slice(0, 10); + const group_name = `${me.email}[${today}]`; + const slot = new Date(); + slot.setDate(slot.getDate() + 1); + slot.setHours(13, 30, 0, 0); + const start = Math.floor(slot.valueOf() / 1000); + + const container = (asset_id: string, from: number, to: number, title: string) => ({ + booking_type: 'group', + asset_id, + asset_name: 'Group Booking', + booking_start: from, + booking_end: to, + timezone: 'Etc/UTC', + user_email: me.email, + user_id: me.id, + user_name: me.name, + title, + zones, + }); + + const post = async (label: string, body: Record) => { + const res = await api.post(`${STAFF_API}/bookings`, { data: body }); + const text = await res.text(); + if (res.ok()) created.push(JSON.parse(text).id); + console.log( + ` ${label.padEnd(52)} -> ${res.status()} ${res.ok() ? '' : text.slice(0, 140)}`, + ); + return res.status(); + }; + + console.log(`\n container id the app would use: ${group_name}\n`); + const first = await post( + '1st group invite, 13:30-14:30', + container(group_name, start, start + 3600, 'first group'), + ); + const second = await post( + '2nd group invite, SAME time, different guests', + container(group_name, start, start + 3600, 'second group'), + ); + await post( + '3rd group invite, later slot', + container(group_name, start + 2.5 * 3600, start + 3.5 * 3600, 'third group'), + ); + await post( + '4th, same time but "created" on another day', + container(`${me.email}[2020-01-01]`, start, start + 3600, 'fourth group'), + ); + + console.log( + first === 201 && second === 409 + ? '\nREPRODUCED: the second overlapping group invite was refused, and the ' + + 'visitors had nothing to do with it.' + : '\nNOT reproduced: the second invite was not refused. VIS-B9 may be fixed.', + ); + } finally { + for (const id of created) await deleteBooking(api, id).catch(() => null); + await api.dispose(); + } +} + +main().catch((e) => { + console.error(e.message); + process.exit(1); +}); diff --git a/e2e/support/visitor/calendar.ts b/e2e/support/visitor/calendar.ts new file mode 100644 index 0000000000..4144c651f9 --- /dev/null +++ b/e2e/support/visitor/calendar.ts @@ -0,0 +1,150 @@ +/** + * Driving the `date-calendar` widget. + * + * The same widget appears twice in the flows this suite covers — the schedule's + * sidebar, and behind the date button on the invite form — so the awkward part + * is written once here. + * + * WHY IT IS AWKWARD: the grid is 42 day cells with nothing on them but a day + * number, and day numbers repeat (the 1st of next month sits in the same grid as + * the 1st of this one). The month label and the weekday headings are locale + * text, so they cannot be matched on either. + * + * What CAN be relied on: exactly one cell carries the "today" ring, drawn from a + * value the component captured when the page loaded, and the cells are + * consecutive days. So read that one cell, work out which real date it means, + * and count from there. + */ +import { Locator, Page, expect } from '@playwright/test'; + +const DAY_MS = 86_400_000; + +/** + * Whole days from one moment to another, counted in LOCAL calendar days. + * + * Rounded rather than truncated because a DST boundary makes a day 23 or 25 + * hours long, and the grid still advances by exactly one cell. + */ +export function daysApart(from_ms: number, to_ms: number): number { + const from = new Date(from_ms); + const to = new Date(to_ms); + from.setHours(0, 0, 0, 0); + to.setHours(0, 0, 0, 0); + return Math.round((to.valueOf() - from.valueOf()) / DAY_MS); +} + +/** + * Click the cell for `timestamp_ms` in an already-visible calendar. + * + * `calendar` must be the `date-calendar` element itself, so this works whether + * it is inline (the schedule) or inside an overlay (the invite form). + */ +export async function pickCalendarDay( + page: Page, + calendar: Locator, + timestamp_ms: number, + options: { + /** + * True for a calendar that lives in a pop-up and closes when a day is + * clicked — the invite form's. The clicked cell is then gone, so the + * pop-up closing is what proves the click landed. The schedule's + * calendar is permanent and marks the day instead. + */ + closes_on_pick?: boolean; + } = {}, +): Promise { + await expect( + calendar, + 'the calendar is not on screen — the schedule renders it only in `day` ' + + 'period and from the `sm` breakpoint up, and the invite form only ' + + 'once its date button has been clicked', + ).toBeVisible({ timeout: 30_000 }); + + const cells = calendar.locator('button[name="schedule-set-date"]'); + // The ring is the only child div carrying `border-secondary`; every cell + // also holds a plain ripple div, so the class matters. + const today_index = await cells.evaluateAll((els) => + els.findIndex((el) => !!el.querySelector('div.border-secondary')), + ); + if (today_index < 0) { + throw new Error( + 'no "today" cell in the calendar, so there is nothing to count from. ' + + 'The grid only marks today while it is showing this month, and ' + + 'nothing here navigates months.', + ); + } + + const day_of_month = Number( + ((await cells.nth(today_index).textContent()) ?? '').trim(), + ); + const browser_now = await page.evaluate(() => Date.now()); + // The ring means the day the PAGE loaded, which is today unless the run has + // just crossed midnight, in which case it means yesterday. + const anchor = [0, -1] + .map((offset) => { + const day = new Date(browser_now); + day.setHours(0, 0, 0, 0); + day.setDate(day.getDate() + offset); + return day; + }) + .find((day) => day.getDate() === day_of_month); + if (!anchor) { + throw new Error( + `the calendar marks day ${day_of_month} as today, which is neither ` + + `today nor yesterday by the browser clock ` + + `(${new Date(browser_now).toString()}). The two should never be more ` + + `than a midnight apart.`, + ); + } + + const index = today_index + daysApart(anchor.valueOf(), timestamp_ms); + const count = await cells.count(); + if (index < 0 || index >= count) { + throw new Error( + `${new Date(timestamp_ms).toDateString()} is outside the ${count} days the ` + + `calendar is showing. Only the displayed month and a few days either ` + + `side can be reached; anything further needs the ` + + `\`schedule-next-month\`/\`schedule-previous-month\` chevrons first ` + + `(whose names are the wrong way round — the "next" one goes back).`, + ); + } + + const cell = cells.nth(index); + await expect( + cell, + `the calendar will not accept ${new Date(timestamp_ms).toDateString()} — the ` + + 'cell is disabled, so a `from`/`to` bound is in the way', + ).toBeEnabled(); + await cell.click(); + if (options.closes_on_pick) { + await expect( + calendar, + 'the date pop-up stayed open, so the day was probably not accepted', + ).toBeHidden({ timeout: 10_000 }); + return index; + } + // `bg-secondary` is how the component marks the selected day. Waiting on it + // proves the click landed before anything reads the result. + await expect(cell, 'the calendar did not select the day').toHaveClass(/bg-secondary/); + return index; +} + +/** Which cell the calendar currently shows as selected, or -1. */ +export async function selectedCalendarIndex(calendar: Locator): Promise { + return calendar + .locator('button[name="schedule-set-date"]') + .evaluateAll((els) => els.findIndex((el) => el.className.includes('bg-secondary'))); +} + +/** The day number written on a cell, for diagnostics. */ +export async function calendarDayLabel( + calendar: Locator, + index: number, +): Promise { + if (index < 0) return 'none'; + const text = await calendar + .locator('button[name="schedule-set-date"]') + .nth(index) + .textContent(); + return (text ?? '').trim() || 'blank'; +} diff --git a/e2e/support/visitor/invite-form.page.ts b/e2e/support/visitor/invite-form.page.ts new file mode 100644 index 0000000000..775918f49a --- /dev/null +++ b/e2e/support/visitor/invite-form.page.ts @@ -0,0 +1,323 @@ +/** + * Page object for the Invite Visitor form (`/#/book/visitor/form`). + * + * SELECTOR POLICY, because this form has none of the `name` hooks the desk flow + * relies on: every locator below is a hardcoded attribute, a fixed id, a form + * control suffix, or a `data-` attribute. NOT ONE depends on visible text, so a + * translation change cannot break this file. Each was confirmed against the + * running app by dumping the DOM, not read off the template and hoped for. + * + * Send button `button[send]` bare attribute in the template + * Sending spinner `div[loading]` bare attribute + * Success panel `div[sent]` bare attribute + * Visitor name `#visitor-name` fixed id + * Visitor email `#visitor-email` fixed id + * Reason `#reason` fixed id + * Company etc. `input[name$=".company"]` signal-form control suffix + * Visitor chips `mat-chip-row[user]` bare attribute + * Chip entry `input[name="user_email"]` hardcoded name + * Building `mat-select[name="building"]` hardcoded name + * Time picker `button[time-field]` bare attribute + * Time option `button[data-time="HH:mm"]` real data- attribute + * Duration option `button[data-duration="mins"]` real data- attribute + * + * The form-control names carry a form index (`ng.form0.asset_name`), so match + * the SUFFIX rather than the whole name. Note the reason lives on a SECOND + * form (`ng.form1.title`) because it is validated separately — `#reason` is the + * unambiguous way to reach it. + */ +import { Locator, Page, expect } from '@playwright/test'; +import { pickCalendarDay } from './calendar'; + +export class InviteVisitorForm { + constructor(private readonly page: Page) {} + + get root(): Locator { + return this.page.locator('invite-visitor-form'); + } + get sendButton(): Locator { + return this.root.locator('button[send]'); + } + get successPanel(): Locator { + return this.page.locator('div[sent]'); + } + get visitorName(): Locator { + return this.page.locator('#visitor-name'); + } + get visitorEmail(): Locator { + return this.page.locator('#visitor-email'); + } + get company(): Locator { + return this.root.locator('input[name$=".company"]'); + } + get reason(): Locator { + return this.page.locator('#reason'); + } + get passNumber(): Locator { + return this.root.locator('input[name$=".pass_number"]'); + } + get allDay(): Locator { + return this.root.locator('input[name$=".all_day"]'); + } + get international(): Locator { + return this.root.locator('input[name$=".international"]'); + } + get chipEntry(): Locator { + return this.page.locator('input[name="user_email"]'); + } + get chips(): Locator { + return this.page.locator('mat-chip-row[user]'); + } + /** The button that opens the date picker. */ + get dateButton(): Locator { + return this.root.locator('a-date-field button').first(); + } + /** The picker itself, which lives in an overlay once opened. */ + get datePicker(): Locator { + return this.page.locator('.cdk-overlay-container date-calendar'); + } + + /** + * Put the visit on a different day. + * + * The form defaults to today, so anything testing a future visit has to say + * so. Same widget as the schedule's sidebar calendar, hence the shared + * helper — the only difference is that this one has to be opened first. + */ + async setDate(timestamp_ms: number): Promise { + await this.dateButton.click(); + // This picker is a pop-up and shuts itself when a day is clicked, which + // is also how we know the click landed. + await pickCalendarDay(this.page, this.datePicker, timestamp_ms, { + closes_on_pick: true, + }); + } + + /** + * The chip for one visitor, in multi-visitor mode. + * + * Matched on the part of the address BEFORE the @, because that is all the + * chip renders: the widget shows `name || email`, and a visitor added by + * email alone comes back named after the local part. Filtering on the full + * address matches nothing — silently, which is what an earlier version of + * the group edit spec did, and it read as a broken remove button. + * + * This is test data, not app copy, so matching on it does not break the + * "never depend on visible text" rule — nothing here is translated. + */ + async chipIndexFor(email: string): Promise { + const local = email.split('@')[0]; + const texts = await this.chips.allInnerTexts(); + const index = texts.findIndex((text) => { + const label = text.split('\n')[0].trim(); + return label === email || label === local; + }); + if (index < 0) { + throw new Error( + `no visitor chip for ${email}. The form shows ` + + `${texts.length}: ${JSON.stringify(texts.map((t) => t.split('\n')[0].trim()))}`, + ); + } + return index; + } + + /** + * Add a visitor to the list, and make sure it actually landed. + * + * Typing an address opens an autocomplete offering "add external user", and + * Enter picks it — but only once that panel has caught up. Fire Enter too + * early and the address just sits in the box: no chip, no error, and the + * next step operates on a list that is one person short. Seen for real, + * which is why this retries rather than pressing once and hoping. + */ + async addVisitorChip(email: string): Promise { + for (let attempt = 1; attempt <= 3; attempt++) { + await this.chipEntry.fill(email); + await this.chipEntry.press('Enter'); + const landed = await expect(async () => { + await this.chipIndexFor(email); + }) + .toPass({ timeout: 5_000 }) + .then(() => true) + .catch(() => false); + if (landed) return; + } + const texts = await this.chips.allInnerTexts(); + throw new Error( + `${email} could not be added to the visitor list after 3 tries. The list ` + + `holds ${texts.length}: ${JSON.stringify(texts.map((t) => t.split('\n')[0].trim()))}. ` + + `The autocomplete may not be offering "add external user" for this ` + + `address at all.`, + ); + } + + /** Take one visitor off the list before sending. */ + async removeVisitorChip(email: string): Promise { + const before = await this.chips.count(); + const index = await this.chipIndexFor(email); + await this.chips.nth(index).locator('button[remove]').click(); + await expect( + this.chips, + `removing ${email} did not shorten the visitor list`, + ).toHaveCount(before - 1, { timeout: 10_000 }); + } + + get buildingSelect(): Locator { + return this.page.locator('mat-select[name="building"]'); + } + get hostField(): Locator { + return this.page.locator('a-user-search-field'); + } + get hostInput(): Locator { + return this.hostField.locator('input').first(); + } + + /** + * Pick a different host — "book this visit on behalf of someone else". + * + * Needs `visitors.can_book_for_anyone` AND `basic_user_search`. The other + * host control (`host-select-field`, behind `can_book_for_others`) builds + * its list from `GET /calendars`, which is 500 on any stack without real + * Microsoft/Google credentials, so it can never list anyone but yourself + * here. Likewise the search field's default source is `/people`, also + * calendar backed; `basic_user_search` swings it to rest-api's `/users`. + * + * Once chosen the input shows the host's NAME, not their email + * (`displayFn` in user-search-field.component.ts) — which is why the name + * is needed to tell "already set" from "not set yet". + */ + async chooseHost(email: string, name: string): Promise { + await expect( + this.hostField, + 'the host field is missing — this flow needs ' + + '`app.visitors.can_book_for_anyone = true`', + ).toBeVisible({ timeout: 30_000 }); + + if ((await this.hostInput.inputValue()) === name) return; + + await this.hostInput.fill(email); + const option = this.page + .locator('mat-option') + .filter({ hasText: email }) + .first(); + await expect( + option, + `no user matching "${email}" was offered — is that user seeded, and is ` + + '`app.basic_user_search` on so the search hits rest-api rather than ' + + 'the calendar-backed /people endpoint?', + ).toBeVisible({ timeout: 20_000 }); + await option.click(); + await expect(this.hostInput).toHaveValue(name); + } + get startTimeTrigger(): Locator { + return this.page.locator('button[time-field]'); + } + + get durationTrigger(): Locator { + return this.page.locator('button[duration-field]'); + } + + /** + * Every start time the form is willing to offer, as "HH:mm". + * + * Read from the `data-time` attribute, not the rendered label, so a 12/24 + * hour setting or a locale cannot change the answer. Opens the menu and + * closes it again, leaving the form as it was found. + */ + async startTimeOptions(): Promise { + await this.startTimeTrigger.click(); + const options = this.page.locator('button[data-time]'); + await expect( + options.first(), + 'the start-time menu offered nothing at all', + ).toBeVisible({ timeout: 10_000 }); + const values = await options.evaluateAll((els) => + els.map((el) => el.getAttribute('data-time') ?? ''), + ); + await this.page.keyboard.press('Escape'); + return values.filter(Boolean); + } + + /** Every visit length the form is willing to offer, in minutes. */ + async durationOptions(): Promise { + await this.durationTrigger.click(); + const options = this.page.locator('button[data-duration]'); + await expect( + options.first(), + 'the duration menu offered nothing at all', + ).toBeVisible({ timeout: 10_000 }); + const values = await options.evaluateAll((els) => + els.map((el) => Number(el.getAttribute('data-duration'))), + ); + await this.page.keyboard.press('Escape'); + return values.filter((v) => Number.isFinite(v)); + } + + /** Choose how long the visit lasts, in minutes. */ + async setDuration(minutes: number): Promise { + await this.durationTrigger.click(); + const option = this.page.locator(`button[data-duration="${minutes}"]`); + await expect( + option, + `no ${minutes} minute option — the form offers lengths on a fixed step and ` + + 'only up to `max_duration`, so pick one it actually lists', + ).toBeVisible({ timeout: 10_000 }); + await option.click(); + } + + /** Open the form and wait for it to render. */ + async open(): Promise { + await this.page.goto('/#/book/visitor/form'); + await expect( + this.root, + 'the visitor invite form never rendered — is the `visitor-invite` feature enabled?', + ).toBeVisible({ timeout: 30_000 }); + } + + /** Fail with a useful message if the form is not in single-visitor mode. */ + async expectSingleMode(): Promise { + await expect( + this.visitorEmail, + 'the single-visitor email field is missing — this flow needs ' + + '`app.bookings.multiple_visitors = false`; the app default is true, ' + + 'which renders the chip list instead', + ).toBeVisible({ timeout: 30_000 }); + } + + /** Fail with a useful message if the form is not in multi-visitor mode. */ + async expectGroupMode(): Promise { + await expect( + this.chipEntry, + 'the multi-visitor entry field is missing — this flow needs ' + + '`app.bookings.multiple_visitors = true`', + ).toBeVisible({ timeout: 30_000 }); + } + + /** + * Pick an exact start time. + * + * Uses the option's `data-time` attribute rather than its rendered label, + * which is formatted with the user's 12/24-hour setting and timezone. Needed + * by the check-in specs, which must land a booking inside the one-hour + * window the backend allows a check-in from. + */ + async setStartTime(hhmm: string): Promise { + await this.startTimeTrigger.click(); + const option = this.page.locator(`button[data-time="${hhmm}"]`); + await expect( + option, + `no start-time option for ${hhmm} — the form offers times on a fixed step ` + + `and only within its bookable hours, so pick one it actually lists`, + ).toBeVisible({ timeout: 10_000 }); + await option.click(); + } + + /** Set a Material checkbox to a state, reading the real state off the input. */ + async setCheckbox(input: Locator, want: boolean): Promise { + if (!(await input.count())) return; + if ((await input.isChecked().catch(() => false)) === want) return; + // The visible mat-checkbox is the click target; the input holds `checked`. + await input.locator('xpath=ancestor::mat-checkbox').click(); + await (want ? expect(input).toBeChecked() : expect(input).not.toBeChecked()); + } +} diff --git a/e2e/support/visitor/visitor.api.ts b/e2e/support/visitor/visitor.api.ts new file mode 100644 index 0000000000..ad74b6ae9a --- /dev/null +++ b/e2e/support/visitor/visitor.api.ts @@ -0,0 +1,203 @@ +/** + * Backend helpers specific to visitor invites: guest records, and a sweep that + * understands them. + * + * Kept out of `e2e/support/api.ts` so the desk specs' helpers stay untouched. + * The generic pieces this builds on (`listBookings`, `deleteBooking`, + * `STAFF_API`) are imported from there rather than duplicated. + */ +import { APIRequestContext } from '@playwright/test'; +import { Booking, STAFF_API, deleteBooking, listBookings } from '../api'; + +/** + * A booking as the visitor flows deal with it. + * + * `Booking` in `../api` carries an index signature, so these fields are already + * reachable — but as `unknown`, which means a spec cannot read + * `extension_data.group` without a cast and a reader cannot tell which extra + * fields matter. Naming the ones the visitor specs rely on is the difference + * between a typed assertion and a hopeful one. + */ +export interface VisitorBooking extends Booking { + /** Set on a group member, pointing at its container booking. */ + parent_id?: string; + /** Who created the booking — differs from `user_email` on a delegated invite. */ + booked_by_email?: string; + booked_by_name?: string; + /** Carries `group`, `group_members`, `group_resource_type` on a container. */ + extension_data?: Record; + checked_in?: boolean; + all_day?: boolean; +} + +export interface Guest { + id: number; + email: string; + name?: string; + checked_in?: boolean; + visit_expected?: boolean; + [k: string]: unknown; +} + +/** + * Guests matching a search term. + * + * A guest row is created by staff-api from a booking's `attendees`, NOT from + * `asset_id` — an invite posted without attendees creates the booking and no + * guest. The UI always sends attendees, so a real invite does create one, but + * an API-only fixture will not unless it says so. + * + * `q` is the filter that works without a period; the windowed form + * (`period_start`/`period_end`) lists guests expected in that window instead. + */ +export async function searchGuests(api: APIRequestContext, q: string): Promise { + const res = await api.get(`${STAFF_API}/guests`, { params: { q } }); + if (!res.ok()) { + throw new Error(`GET /guests?q=${q} failed: HTTP ${res.status()} ${await res.text()}`); + } + const body = await res.json(); + return Array.isArray(body) ? body : (body.results ?? []); +} + +/** + * Is this response REG-09 (PPT-2642) rather than a real rejection? + * + * Two signatures, both produced by the same poisoned connection. The 400 is the + * confusing one — it claims the request body was empty when it was not. + * `e2e/support/repro/vis-b6-burst.ts` reproduces both with no browser involved. + * + * Deliberately narrow. Matching on the status alone would swallow genuine 400s + * and 500s, and a suite that retries real failures is worse than no suite. + */ +export function isReg09(status: number, body: string): boolean { + if (status === 400 && /Expected BeginObject but was EOF/.test(body)) return true; + if (status === 500 && /could not serialize access/.test(body)) return true; + return false; +} + +/** + * Create a booking through the API, retrying past REG-09. + * + * Specs whose subject is authorisation or listing behaviour create their data + * this way rather than through the form. They still meet REG-09, because the + * bug is in the backend's connection handling and does not care how the request + * was produced — so without this a scoping spec fails for a reason that has + * nothing to do with scoping. + * + * Remove once the stack runs a staff-api built on pg-orm >= 2.2.4. + */ +export async function createBookingViaApi( + api: APIRequestContext, + data: Record, + attempts = 4, +): Promise { + let last = ''; + for (let attempt = 1; attempt <= attempts; attempt++) { + const res = await api.post(`${STAFF_API}/bookings`, { data }); + const body = await res.text(); + if (res.ok()) return JSON.parse(body) as VisitorBooking; + + last = `HTTP ${res.status()} ${body.slice(0, 200)}`; + if (!isReg09(res.status(), body)) { + throw new Error(`POST /bookings failed: ${last}`); + } + console.warn(` ! REG-09 on attempt ${attempt}/${attempts}, retrying — ${last}`); + // A short pause lets the poisoned connection cycle out of the pool. + await new Promise((r) => setTimeout(r, 400 * attempt)); + } + throw new Error( + `POST /bookings still failing after ${attempts} attempts. If every attempt is ` + + `REG-09 (PPT-2642), the stack is running a staff-api older than pg-orm 2.2.4. ` + + `Last: ${last}`, + ); +} + +/** + * Attempt a check-in directly, returning the raw outcome rather than throwing. + * + * Specs use this to prove the BACKEND enforces the timing rule independently of + * the UI. The app hides its check-in button until a booking is within 15 minutes + * of starting (`Booking.state`), so a UI-only assertion would pass even if the + * server happily accepted a check-in a week early. + */ +export async function checkInViaApi( + api: APIRequestContext, + id: number, + state = true, +): Promise<{ status: number; body: string }> { + const res = await api.post(`${STAFF_API}/bookings/${id}/check_in?state=${state}`); + return { status: res.status(), body: await res.text() }; +} + +/** Best-effort guest removal — never throws, so teardown cannot fail a passing test. */ +export async function deleteGuest(api: APIRequestContext, email: string): Promise { + try { + await api.delete(`${STAFF_API}/guests/${encodeURIComponent(email)}`); + } catch { + /* swallow: teardown must not mask the actual result */ + } +} + +/** + * Free a visitor address before using it: delete its live bookings in the + * window AND its guest record. + * + * Two differences from the desks' `releaseAsset`, both found by probing the + * backend rather than assumed: + * + * - A guest row OUTLIVES its booking. Deleting the booking leaves the guest + * behind, so a sweep that only removes bookings lets guest state accumulate + * across runs and makes "was a guest created?" pass on last run's data. + * - A visitor is not exclusive, so a leftover booking does not BLOCK the next + * run the way a leaked all-day desk booking does. It still pollutes the + * listing, which is what turns a scoping assertion into a confusing failure. + */ +export async function releaseVisitor( + api: APIRequestContext, + email: string, + from: number, + to: number, +): Promise { + let removed = 0; + try { + const existing = await listBookings(api, 'visitor', from, to); + for (const b of existing) { + if (b.asset_id === email && !b.deleted) { + await deleteBooking(api, b.id); + removed++; + } + } + } catch { + /* a sweep that cannot run must not fail the test it is protecting */ + } + await deleteGuest(api, email); + return removed; +} + +/** + * Delete any group CONTAINER bookings this user owns in the window. + * + * Containers are `booking_type: 'group'`, so `releaseVisitor` — which lists + * `type=visitor` — cannot see them. A run that dies between creating the + * container and creating its members leaves an orphan behind (VIS-B7), and + * without this the orphans accumulate silently. + */ +export async function releaseGroupContainers( + api: APIRequestContext, + from: number, + to: number, +): Promise { + let removed = 0; + try { + const existing = await listBookings(api, 'group', from, to); + for (const b of existing) { + if (!b.deleted) { + await deleteBooking(api, b.id); + removed++; + } + } + } catch { + /* a sweep that cannot run must not fail the test it is protecting */ + } + return removed; +} diff --git a/e2e/support/visitor/visitor.env.ts b/e2e/support/visitor/visitor.env.ts new file mode 100644 index 0000000000..7b19a76d9c --- /dev/null +++ b/e2e/support/visitor/visitor.env.ts @@ -0,0 +1,67 @@ +/** + * Visitor identities for the e2e suite. + * + * Kept out of `e2e/support/env.ts` on purpose: the desk specs depend on that + * file, and nothing visitor-shaped should be able to break them. + * + * NOTHING seeds these addresses. A visitor "asset" is just an email, so unlike + * a desk there is no resource to provision — which is why the visitor specs + * need no `seed.ts` step at all. + */ +export const VISITOR_PREFIX = 'e2e-visitor-'; + +/** + * Slot allocation. Every spec FILE gets its own range, and every test within a + * file gets its own slot. + * + * This is not about booking clashes — a visitor is not exclusive, and the + * backend happily accepts the same one twice (VIS-B1). It is about the SWEEP: + * `releaseVisitor` deletes by address under the worker's own identity, so two + * tests sharing an address can delete each other's booking if they ever land on + * the same worker. Distinct slots make that impossible by construction rather + * than by relying on how Playwright happens to schedule. + */ +export const VISITOR_SLOTS = { + /** visitor-invite.spec.ts */ + invite: { stores: 0, cleanup: 1 }, + /** visitor-group.spec.ts — two visitors per test, so two slots each */ + group: { a: 2, b: 3 }, + /** visitor-scoping.spec.ts */ + scoping: { own: 4, control: 15 }, + /** visitor-delegate.spec.ts */ + delegate: { single: 5, groupA: 6, groupB: 7 }, + /** visitor-checkin.spec.ts */ + checkin: { checkIn: 8, tooEarly: 9 }, + /** visitor-edit.spec.ts */ + edit: { details: 10, memberA: 11, memberB: 12 }, + /** visitor-settings.spec.ts */ + settings: { allDay: 13, reason: 14 }, + /** visitor-cancel.spec.ts */ + cancel: { fromApp: 16, dismissed: 17 }, + /** visitor-validation.spec.ts */ + validation: { badEmail: 18, keptA: 19, removedB: 20 }, + /** visitor-details.spec.ts */ + details: { extras: 21, company: 22 }, + /** visitor-times.spec.ts */ + times: { chosen: 23, limits: 24 }, +} as const; + +export interface VisitorIdentity { + email: string; + name: string; + company: string; +} + +/** + * A visitor address owned by this worker and this slot. + * + * Per-worker keeps each parallel worker's listings and guest records disjoint; + * per-slot keeps tests within a worker from sweeping each other. + */ +export function visitorFor(workerIndex: number, slot: number): VisitorIdentity { + return { + email: `${VISITOR_PREFIX}${workerIndex}-${slot}@example.com`, + name: `E2E Visitor ${workerIndex}-${slot}`, + company: `E2E Co ${workerIndex}`, + }; +} diff --git a/e2e/support/visitor/visitor.flows.ts b/e2e/support/visitor/visitor.flows.ts new file mode 100644 index 0000000000..1831c1da8a --- /dev/null +++ b/e2e/support/visitor/visitor.flows.ts @@ -0,0 +1,378 @@ +/** + * Multi-step visitor flows, composed from `InviteVisitorForm`. + * + * These return what the BACKEND created, parsed from the real POST responses — + * never anything the page rendered. Same principle as `bookDeskViaUI`, and the + * reason this suite can tell "the screen looked right" apart from "the data is + * right". + * + * ## Why these flows retry: REG-09 / PPT-2642, not a form bug + * + * Under concurrent booking writes, `POST /bookings` intermittently answers + * + * HTTP 400 {"error":"Expected BeginObject but was EOF at line 1, column 1"} + * HTTP 500 {"error":"...could not serialize access due to read/write + * dependencies among transactions"} + * + * The 400 reads like the client sent nothing. It does not: the request carried + * ~2 KB of valid JSON. `e2e/support/repro/vis-b6-burst.ts` settles it by firing + * the same payloads concurrently with NO BROWSER INVOLVED — 4 concurrent POSTs + * over 12 rounds produced 13x 201, 29x 400 and 6x 500, with the early rounds + * clean and the failure rate climbing as the run went on. + * + * That degradation curve is the signature of REG-09: a failed COMMIT leaves a + * connection's transaction flag set, it returns to the pool with its `BEGIN` + * still open, and everything routed to it afterwards fails. Fixed in pg-orm + * v2.2.4 and staff-api #386, but `e2e/stack` pulls `placeos/staff-api:latest`, + * a release image that predates it. + * + * E2E_USER_STORIES.md says of REG-09: "harmless for now: the suite does not + * burst concurrent bookings, so nothing here trips it." A GROUP invite is the + * first thing in this suite that does — one send posts a container plus one + * booking per visitor, and several workers do that at once. + * + * So the retry below is NOT papering over a frontend defect. It absorbs a known + * backend bug whose fix is already merged upstream, the same way CI's + * `retries: 2` absorbs REG-09 today. Delete it once the stack runs a staff-api + * built on pg-orm >= 2.2.4, and the suite should still be green. + * + * Two earlier explanations were wrong and are recorded so nobody re-derives + * them: "the app posts an empty body" (it does not — the request is ~2 KB) and + * "the form is rebuilt mid-init and loses its model" (plausible, but the + * browser-free burst reproduces it without any form at all). + */ +import { APIRequestContext, Page, expect } from '@playwright/test'; +import { InviteVisitorForm } from './invite-form.page'; +import { VisitorIdentity } from './visitor.env'; +import { deleteBooking } from '../api'; +import { VisitorBooking } from './visitor.api'; + +/** How long the form's values must hold steady before we trust them. */ +const SETTLE_MS = 1_000; + +interface CapturedPost { + status: number; + body: string; + request: string; + headers: Record; +} + +/** + * Collect every `POST /bookings` the page makes, with its REQUEST body. + * + * Capturing the REQUEST body, not just the response, is what disproved the + * first theory about REG-09's 400: the body is ~2 KB, so "the app sent nothing" + * was never true however much the error message sounded like it. + * + * A single-visitor invite is one POST; a multi-visitor invite is N+1 — a group + * container plus one booking per visitor. `waitForResponse` resolves on the + * FIRST match, so using it for a group would assert against the container and + * silently ignore every member. + */ +function collectBookingPosts(page: Page) { + const posts: CapturedPost[] = []; + const pending: Promise[] = []; + const onResponse = (r: import('@playwright/test').Response) => { + if (!r.url().includes('/api/staff/v1/bookings')) return; + if (r.request().method() !== 'POST') return; + const request = r.request().postData() ?? ''; + const headers = r.request().headers(); + // Read the body while the response is still live; resolving it later can + // fail with "No data found", which reads as a network flake rather than + // the assertion you meant to write. + pending.push( + r + .text() + .catch(() => '') + .then( + (body) => void posts.push({ status: r.status(), body, request, headers }), + ), + ); + }; + page.on('response', onResponse); + + const describe = () => + posts + .map((p) => { + const ok = p.status >= 200 && p.status < 300; + const base = + `HTTP ${p.status} req=${p.request.length ? `${p.request.length}b` : 'EMPTY'}`; + // Only dump the full picture for a FAILED post. On the happy path + // it is noise, and on a failure it is the only evidence there is — + // the app's error toast has faded before any screenshot is taken. + if (ok) return `${base} ok`; + return ( + `${base}\n content-type=${p.headers['content-type'] ?? '(none)'}` + + `\n content-length=${p.headers['content-length'] ?? '(none)'}` + + `\n req-head=${JSON.stringify(p.request.slice(0, 220))}` + + `\n resp=${p.body.slice(0, 220)}` + ); + }) + .join('\n | ') || '(no booking POST was made at all)'; + + return { + okCount: () => posts.filter((p) => p.status >= 200 && p.status < 300).length, + /** Ids of everything that really got created, for cleaning up a partial group. */ + createdIds: () => + posts + .filter((p) => p.status >= 200 && p.status < 300) + .map((p) => { + try { + return JSON.parse(p.body).id as number; + } catch { + return undefined; + } + }) + .filter((id): id is number => typeof id === 'number'), + describe, + /** Forget everything so far — used after a failed attempt is rolled back. */ + reset: async () => { + await Promise.all(pending); + posts.length = 0; + pending.length = 0; + }, + stop: async () => { + page.off('response', onResponse); + await Promise.all(pending); + return posts; + }, + }; +} + +/** + * Fill the form, then require the values to SURVIVE a settling window. + * + * Converging on the values is not enough on its own: the rebuild can land in + * the gap between the last check and the click. Holding them steady for a beat + * closes most of that gap; the retry in `submitWhenAccepted` closes the rest. + */ +async function fillAndSettle(page: Page, fill: () => Promise): Promise { + await expect(async () => { + await fill(); + await page.waitForTimeout(SETTLE_MS); + await fill(); + }).toPass({ timeout: 45_000 }); +} + +/** A send attempt that did not reach the success screen. */ +const MAX_ATTEMPTS = 3; + +/** + * Click send, and keep trying until the app confirms the invite. + * + * Two distinct failure shapes, and they need opposite handling: + * + * - NOTHING was created. Safe to settle and click again. + * - SOMETHING was created but the flow still failed. For a group invite this + * means the container landed and a member did not, leaving an orphaned + * `booking_type: 'group'` row with no members. Clicking again would add a + * SECOND container, so the partial result is deleted first and the whole + * invite starts over. + * + * Rolling back rather than giving up is deliberate. The cause is REG-09, a + * backend defect already fixed upstream (see the file header), so a failed + * attempt says nothing about the behaviour under test. Leaving the orphan + * behind instead would poison the next run's listing assertions. + */ +async function submitWhenAccepted( + page: Page, + api: APIRequestContext, + form: InviteVisitorForm, + posts: ReturnType, + refill: () => Promise, + what: string, +): Promise { + let last = ''; + + for (let attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) { + await form.sendButton.click(); + const ok = await form.successPanel + .waitFor({ state: 'visible', timeout: 20_000 }) + .then(() => true) + .catch(() => false); + if (ok) return; + + last = posts.describe(); + const partial = posts.createdIds(); + if (partial.length) { + console.warn( + ` ! REG-09: ${what} partially created ${partial.length} booking(s) then ` + + `failed — rolling them back and retrying (attempt ${attempt}/${MAX_ATTEMPTS})`, + ); + for (const id of partial) await deleteBooking(api, id); + } + await posts.reset(); + + if (attempt === MAX_ATTEMPTS) break; + // Start from a clean form: after a partial create the component has + // already advanced its own state, so re-filling the current DOM is not + // the same as re-opening. + await form.open(); + await fillAndSettle(page, refill); + } + + throw new Error( + `${what} never reached its success screen after ${MAX_ATTEMPTS} attempts. ` + + `If every attempt shows HTTP 400 "Expected BeginObject but was EOF" or HTTP 500 ` + + `"could not serialize access", this is REG-09 (PPT-2642) and the stack is ` + + `running a staff-api older than pg-orm 2.2.4. Last responses:\n ${last}`, + ); +} + +/** + * The bookings that were actually created. + * + * A failed attempt a retry recovered is NOT a test failure — the flow's job is + * to get the invite made. Recovered failures are still printed, because they + * are the visible symptom of VIS-B6; swallowing them would make the suite green + * and the defect invisible, which is how a suite stops being worth running. + */ +function createdBookings(posts: CapturedPost[]): VisitorBooking[] { + const ok = posts.filter((p) => p.status >= 200 && p.status < 300); + for (const f of posts.filter((p) => p.status < 200 || p.status >= 300)) { + console.warn( + ` ! REG-09: a booking POST was rejected and recovered on retry — ` + + `HTTP ${f.status} (request carried ${f.request.length} bytes, so the body ` + + `was not the problem)`, + ); + } + expect( + ok.length, + `no booking was created. ` + + posts + .map((p) => `HTTP ${p.status} resp=${p.body.slice(0, 200)}`) + .join(' | '), + ).toBeGreaterThan(0); + return ok.map((p) => JSON.parse(p.body) as VisitorBooking); +} + +/** + * Invite ONE visitor through the full UI, in single-visitor mode. + * Returns the booking the API created. + */ +export async function inviteVisitorViaUI( + page: Page, + api: APIRequestContext, + visitor: VisitorIdentity, + reason: string, + options: { + allDay?: boolean; + passNumber?: string; + international?: boolean; + /** Book on behalf of somebody else. Needs `can_book_for_anyone`. */ + host?: { email: string; name: string }; + /** Put the visit on another day. Defaults to whatever the form offers. */ + date?: number; + /** Start the visit at "HH:mm". Must be an option the form lists. */ + startTime?: string; + /** How long the visit runs, in minutes. Must be an option the form lists. */ + duration?: number; + } = {}, +): Promise { + const form = new InviteVisitorForm(page); + await form.open(); + await form.expectSingleMode(); + + const fill = async () => { + // Host first: choosing one can reset parts of the form, so anything + // typed before it would be lost. + if (options.host) await form.chooseHost(options.host.email, options.host.name); + await form.visitorName.fill(visitor.name); + await form.visitorEmail.fill(visitor.email); + if (visitor.company && (await form.company.count())) { + await form.company.fill(visitor.company); + } + if (options.date !== undefined) await form.setDate(options.date); + // Times go in before the rest: changing the start can shift which + // lengths are on offer, so setting them the other way round can leave a + // duration that no longer exists. + if (options.startTime) await form.setStartTime(options.startTime); + if (options.duration !== undefined) await form.setDuration(options.duration); + await form.reason.fill(reason); + if (options.passNumber && (await form.passNumber.count())) { + await form.passNumber.fill(options.passNumber); + } + if (options.allDay !== undefined) { + await form.setCheckbox(form.allDay, options.allDay); + } + if (options.international !== undefined) { + await form.setCheckbox(form.international, options.international); + } + expect(await form.visitorEmail.inputValue()).toBe(visitor.email); + expect(await form.visitorName.inputValue()).toBe(visitor.name); + expect(await form.reason.inputValue()).toBe(reason); + }; + + await fillAndSettle(page, fill); + + const posts = collectBookingPosts(page); + await submitWhenAccepted(page, api, form, posts, fill, 'the invite'); + + const bookings = createdBookings(await posts.stop()); + expect(bookings, 'a single-visitor invite creates exactly one booking').toHaveLength(1); + return bookings[0]; +} + +/** + * Invite SEVERAL visitors through the full UI, in multi-visitor mode. + * Returns every booking created: the group container and one per visitor. + */ +export async function inviteVisitorsViaUI( + page: Page, + api: APIRequestContext, + visitors: VisitorIdentity[], + reason: string, + options: { + host?: { email: string; name: string }; + /** Put the visit on another day. Defaults to whatever the form offers. */ + date?: number; + /** + * Add these, then take them off again before sending. Modelling the + * everyday "wrong person from the lookup" correction — and it has to + * happen INSIDE the fill, because a retry re-runs the fill and would + * otherwise send the removed visitor after all. + */ + remove?: VisitorIdentity[]; + } = {}, +): Promise { + const removing = options.remove ?? []; + const expected = visitors.filter( + (v) => !removing.some((r) => r.email === v.email), + ); + const form = new InviteVisitorForm(page); + await form.open(); + await form.expectGroupMode(); + + const fill = async () => { + if (options.host) await form.chooseHost(options.host.email, options.host.name); + for (const visitor of visitors) { + // "Already added?" has to go through `chipIndexFor`: a chip shows + // only the part before the @, so a `hasText` on the full address + // never matches and a retry would add everyone twice. + const already = await form + .chipIndexFor(visitor.email) + .then(() => true) + .catch(() => false); + if (already) continue; + await form.addVisitorChip(visitor.email); + } + for (const visitor of removing) await form.removeVisitorChip(visitor.email); + if (options.date !== undefined) await form.setDate(options.date); + await form.reason.fill(reason); + expect(await form.chips.count()).toBe(expected.length); + expect(await form.reason.inputValue()).toBe(reason); + }; + + await fillAndSettle(page, fill); + + const posts = collectBookingPosts(page); + await submitWhenAccepted(page, api, form, posts, fill, 'the group invite'); + + const bookings = createdBookings(await posts.stop()); + expect( + bookings.length, + `a group invite creates one container plus one booking per visitor ` + + `(${expected.length + 1} total), got ${bookings.length}`, + ).toBe(expected.length + 1); + return bookings; +} diff --git a/e2e/support/visitor/visitor.settings.ts b/e2e/support/visitor/visitor.settings.ts new file mode 100644 index 0000000000..2d5786b2b5 --- /dev/null +++ b/e2e/support/visitor/visitor.settings.ts @@ -0,0 +1,121 @@ +/** + * Per-test app settings for the visitor specs. + * + * The app reads a flat, highest-priority override map out of + * `localStorage['PLACEOS.setting_overrides']` + * (libs/common/src/lib/settings.service.ts, `loadDebugOverrides`). Because it + * lives in the browser context rather than on a zone, two specs can demand + * opposite settings and still run in parallel — which matters here, since + * single- and multi-visitor mode are the same setting with two values, and + * seeding it on the org zone would force the whole suite to serialise. + * + * The trade-off, stated plainly because a future reader deserves it: this is a + * debug hook, so these specs run with an override layer a real user does not + * have. That is acceptable for exercising form behaviour and would not be for + * asserting how settings themselves are resolved from zone metadata. + */ +import { Page } from '@playwright/test'; + +/** + * Force app settings for ONE page, before its first navigation. + * + * Two rules the implementation imposes: + * - Keys MUST start with `app.` — anything else is dropped on load. + * - Matching is EXACT. `app.visitors` will not satisfy a read of + * `app.visitors.allow_all_day`; spell out the full key. + * + * Call BEFORE `page.goto`. `addInitScript` runs before the bundle evaluates, so + * the override is already in localStorage when the service reads it once at + * module scope. + */ +export async function useSettings( + page: Page, + overrides: Record, +): Promise { + const bad = Object.keys(overrides).filter((k) => !k.startsWith('app.')); + if (bad.length) { + throw new Error( + `setting override keys must start with "app." — the app drops the rest ` + + `on load, which would silently leave the default in place and make the ` + + `spec assert against the wrong mode. Got: ${bad.join(', ')}`, + ); + } + await page.addInitScript((value) => { + localStorage.setItem('PLACEOS.setting_overrides', JSON.stringify(value)); + }, overrides); +} + +/** + * Settings every visitor spec needs. + * + * `basic_user_search` points the host field at rest-api's `/users`. Its default + * (`/people`) and the dropdown host field (`/calendars`) are both calendar + * backed and return 500 on a stack without real Microsoft/Google credentials, + * so without this the host field silently finds nobody. + */ +export const VISITOR_BASE_SETTINGS = { + 'app.basic_user_search': true, +}; + +/** Single-visitor mode: name/email/company fields. NOT the app default. */ +export const SINGLE_VISITOR_MODE = { + ...VISITOR_BASE_SETTINGS, + 'app.bookings.multiple_visitors': false, +}; + +/** Multi-visitor mode: the chip list. This IS the app default. */ +export const GROUP_VISITOR_MODE = { + ...VISITOR_BASE_SETTINGS, + 'app.bookings.multiple_visitors': true, +}; + +/** + * Let the invite be booked on behalf of any user the search can find. + * + * BOTH keys are required, and that is not obvious. `can_book_for_anyone` is + * what renders the searchable host field, but `sendInvite` decides whether to + * KEEP the chosen host by checking `can_book_for_others` + * (invite-visitor-form.component.ts, "fall back to the signed-in user for a new + * booking with no host"). Enable only `can_book_for_anyone` and you get a host + * picker that works right up until you press send, at which point the choice is + * silently replaced by whoever is signed in — no error, no hint. + * + * Worth raising with the dev team: a deployment that sets one flag and not the + * other gets exactly that silent failure. + */ +export const BOOK_FOR_ANYONE = { + 'app.visitors.can_book_for_anyone': true, + 'app.visitors.can_book_for_others': true, +}; + +/** Show the All Day checkbox on the invite form. */ +export const ALLOW_ALL_DAY = { + 'app.visitors.allow_all_day': true, +}; + +/** Show the pass-number and international-visitor controls. */ +export const ALLOW_EXTRAS = { + 'app.visitors.allow_pass_number': true, + 'app.visitors.allow_international': true, +}; + +/** Make the reason mandatory (PPT-2782). */ +export const REASON_REQUIRED = { + 'app.visitors.reason_required': true, +}; + +/** + * Cap how long a visit may be, in minutes, and confine it to office hours. + * + * Both are unset by default (`null`), which is why nothing else in this suite + * sees them. `bookable_hours` is `{ start, end }` in 24-hour decimal hours. + */ +export const LIMITED_HOURS = { + 'app.visitors.max_duration': 60, + 'app.visitors.bookable_hours': { start: 9, end: 17 }, +}; + +/** Allow an existing invite to be edited — off by default, so the Edit button hides. */ +export const ALLOW_EDITING = { + 'app.visitors.allow_editing': true, +}; diff --git a/e2e/support/visitor/your-bookings.page.ts b/e2e/support/visitor/your-bookings.page.ts new file mode 100644 index 0000000000..eb81f6a58e --- /dev/null +++ b/e2e/support/visitor/your-bookings.page.ts @@ -0,0 +1,332 @@ +/** + * Page object for `/your-bookings` — the schedule list and the booking details + * modal it opens. + * + * SELECTOR POLICY, same as `invite-form.page.ts`: nothing here depends on + * visible text, because all of it is translated. The awkward one is the + * "Associate" badge, which is the only marker on a card with no attribute of + * its own: + * + *
Associate
+ * + * Its sibling above it DOES have one (`checked-in-badge`), and the only other + * element that could match the class shape is the `group-event` badge — which a + * visitor booking can never be, since that is a different `booking_type`. So + * `.absolute.top-2.right-2:not([checked-in-badge])`, scoped to a card we have + * already identified by booking id, is unambiguous here even though it would + * not be in general. If a third badge is ever added to that corner this breaks, + * which is why the locator carries a failure message saying so. + * + * Cards are found by booking id rather than by title: the card anchor renders + * `routerLink` query params into its href, so `?booking=123` is real data in + * the DOM and cannot be knocked out by a copy change. + */ +import { Locator, Page, expect } from '@playwright/test'; +import { + calendarDayLabel, + pickCalendarDay, + selectedCalendarIndex, +} from './calendar'; + +export class YourBookingsPage { + constructor(private readonly page: Page) {} + + /** Open the list and wait for the schedule to render. */ + async open(): Promise { + await this.page.goto('/#/your-bookings'); + await expect( + this.page.locator('button[name="schedule-toggle-visitor-filter"]').first(), + 'the schedule filters never rendered — is the `visitor-invite` feature enabled?', + ).toBeVisible({ timeout: 30_000 }); + } + + /** Make sure visitor bookings are among the types being shown. */ + async showVisitors(): Promise { + const toggle = this.page + .locator('button[name="schedule-toggle-visitor-filter"]') + .first(); + // ENSURE, never blindly toggle: clicking a filter that is already on + // turns it off, and the list then looks like the booking is missing. + const checkbox = toggle.locator('input[type="checkbox"]'); + if (await checkbox.count()) { + if (!(await checkbox.isChecked().catch(() => false))) await toggle.click(); + } + } + + /** + * Show the day a given moment falls on, rather than trusting the default. + * + * The schedule opens on whatever "today" was when the page loaded and never + * moves again, so a spec whose booking is not on that day sees an empty + * list. That is not only a tomorrow problem: a run that starts at 23:58 and + * loads the page at 00:01 is looking at the NEW day while its booking sits + * on the old one. The symptom is a missing card, which looks nothing like a + * clock problem. + * + * Driving the sidebar calendar is the only stable way to move the view — + * there is no date in the URL, and `schedule-mobile-calendar` is `sm:hidden` + * so it does not exist at the desktop viewport the suite runs at. + * + * The grid is 42 consecutive day cells, so ONE known cell fixes every other + * one by counting. The known cell is the one carrying the "today" ring, + * which the component renders from its own `today`, captured when the page + * loaded. Its day-of-month is read back and matched against the clock to + * work out which real date the ring means — today, or yesterday if the run + * has just crossed midnight. Counting from there is immune to the clock + * moving underneath us afterwards. + */ + async showDayOf(timestamp_ms: number): Promise { + const calendar = this.page.locator('schedule-sidebar date-calendar'); + // Twice, if needed. Under a loaded parallel run the schedule has been + // seen back on today after the click — the component rebinds its date + // from the service while the first query is still in flight, and the + // selection goes with it. Re-picking is cheap; a missing card that + // actually means "wrong day" costs a 30s timeout and looks like a bug + // in the app. + for (let attempt = 1; attempt <= 2; attempt++) { + const wanted = await pickCalendarDay(this.page, calendar, timestamp_ms); + // The list re-queries on a 300ms debounce and blanks itself while + // loading, so settle before judging anything. + await this.page.waitForTimeout(500); + await expect( + this.page.locator('mat-progress-bar'), + 'the schedule never finished reloading after the day changed', + ).toHaveCount(0, { timeout: 30_000 }); + + if ((await selectedCalendarIndex(calendar)) === wanted) return; + } + const selected = await selectedCalendarIndex(calendar); + throw new Error( + `the schedule would not stay on ${new Date(timestamp_ms).toDateString()}. ` + + `After two attempts it is showing day ` + + `"${await calendarDayLabel(calendar, selected)}" instead.`, + ); + } + + /** + * Accept the app's confirmation dialog ("are you sure?"). + * + * Used by check-out, which asks before releasing the booking. The dialog is + * the shared `confirm-modal`, whose accept button carries `name="accept"` — + * the one control in this flow with a stable attribute, and the reason this + * does not have to match the translated "Check out" label. + */ + async acceptConfirm(): Promise { + const accept = this.page.locator( + '.cdk-overlay-container button[name="accept"]', + ); + await expect( + accept, + 'the confirmation dialog did not open, so there was nothing to accept', + ).toBeVisible({ timeout: 15_000 }); + await accept.click(); + await expect(accept, 'the confirmation dialog did not close').toBeHidden({ + timeout: 15_000, + }); + } + + /** The card for a specific booking, matched on the id in its href. */ + card(bookingId: number): Locator { + return this.page.locator(`a[name="view-booking-details"][href*="booking=${bookingId}"]`); + } + + /** + * The "Associate" badge — shown when the booking's HOST is not the signed-in + * user, which for a visitor invite means somebody booked it on their behalf. + */ + associateBadge(bookingId: number): Locator { + return this.card(bookingId).locator( + 'div.absolute.top-2.right-2:not([checked-in-badge])', + ); + } + + /** The green badge that replaces the Associate one once checked in. */ + checkedInBadge(bookingId: number): Locator { + return this.card(bookingId).locator('div[checked-in-badge]'); + } + + /** The "For {name}" line, shown only when the host is somebody else. */ + bookedForLine(bookingId: number): Locator { + return this.card(bookingId).locator('div[booked-for]'); + } + + /** The booking ids of every card currently rendered, for failure messages. */ + async renderedBookingIds(): Promise { + const hrefs = await this.page + .locator('a[name="view-booking-details"]') + .evaluateAll((els) => els.map((e) => e.getAttribute('href') ?? '')); + return hrefs + .map((h) => Number(/booking=(\d+)/.exec(h)?.[1])) + .filter((n) => Number.isFinite(n)); + } + + /** Open a booking's details modal and wait for it. */ + async openDetails(bookingId: number): Promise { + const card = this.card(bookingId); + const found = await card + .waitFor({ state: 'visible', timeout: 30_000 }) + .then(() => true) + .catch(() => false); + if (!found) { + // Say what IS on the page. "Element not found" alone cannot + // distinguish "wrong day", "filter off" and "the list is so full of + // old bookings that this one is paged out". + const rendered = await this.renderedBookingIds(); + const calendar = this.page.locator('schedule-sidebar date-calendar'); + const showing = await calendarDayLabel( + calendar, + await selectedCalendarIndex(calendar), + ); + throw new Error( + `no card for booking ${bookingId} on /your-bookings, which is showing ` + + `day "${showing}" of the month. ` + + `${rendered.length} card(s) are rendered: [${rendered.join(', ')}]. ` + + `Check the day being shown is the booking's own day (\`showDayOf\` ` + + `moves it), that the visitor filter is on, and that the list is not ` + + `dominated by leftovers from earlier runs.`, + ); + } + await card.click(); + const modal = this.page.locator('booking-details-modal'); + await expect(modal, 'the booking details modal did not open').toBeVisible({ + timeout: 20_000, + }); + return modal; + } + + /** + * The check-in button inside an open details modal. + * + * `div[actions]` holds exactly two buttons: this one, which carries `btn`, + * and the overflow menu, which carries `icon`. Selecting on `btn` avoids the + * translated label. The button is only rendered when check-in is available + * at all, so a spec should assert on its visibility rather than assume it. + */ + checkInButton(modal: Locator): Locator { + // Note: this is ALSO the check-out button. The app flips the same + // control once you are checked in, so a spec that checks in and then out + // uses this twice rather than looking for a second button. + return modal.locator('div[actions] button[btn]'); + } + + /** + * Open the modal's overflow menu — the "..." next to check-in. + * + * It is the `icon` button in `div[actions]`; check-in is the `btn` one. The + * two are told apart by attribute rather than position, so adding a third + * control would not silently repoint this at the wrong thing. + */ + async openOverflowMenu(modal: Locator): Promise { + await modal.locator('div[actions] button[icon]').click(); + await expect( + this.page.locator('.cdk-overlay-container [mat-menu-item]').first(), + 'the booking overflow menu did not open', + ).toBeVisible({ timeout: 10_000 }); + } + + /** + * A menu action, identified by its MATERIAL ICON rather than its label. + * + * The menu items carry no attributes and their labels are translated, but + * each renders an icon ligature — `edit`, `delete` — which is an icon name + * in the source, not copy. That is the only stable handle here. + * + * Read explicitly rather than matched with a text selector. Two attempts at + * the latter both silently matched nothing while the item was plainly in the + * DOM: `icon:text-is("edit")` binds to the innermost element holding the + * text, so it never matches the wrapper, and `hasText` with an + * anchored regex did not match either. Comparing trimmed `textContent` is + * duller and it works. + */ + async menuAction(icon: string): Promise { + const items = this.page.locator('.cdk-overlay-container button[mat-menu-item]'); + const count = await items.count(); + for (let i = 0; i < count; i++) { + const item = items.nth(i); + const label = (await item.locator('icon').first().textContent()) ?? ''; + if (label.trim() === icon) return item; + } + return null; + } + + /** Dismiss a confirmation dialog without accepting it. */ + async dismissConfirm(): Promise { + // The decline button is the footer button that is NOT `accept`. Its + // label is translated, so it cannot be matched on text. + const decline = this.page + .locator('.cdk-overlay-container footer button:not([name="accept"])') + .first(); + await expect( + decline, + 'the confirmation dialog did not open, so there was nothing to dismiss', + ).toBeVisible({ timeout: 15_000 }); + await decline.click(); + await expect(decline, 'the confirmation dialog did not close').toBeHidden({ + timeout: 15_000, + }); + } + + /** + * Start cancelling a booking from the app: details -> overflow -> Cancel. + * + * Stops at the confirmation dialog, so a caller can accept it or walk away. + * The menu item is found by its `delete` icon; every other item in that menu + * uses a different one. There can be more than one delete-ish action on a + * booking (remove this one, remove the whole series, remove the group), so + * this fails loudly rather than picking the first of several — cancelling + * the wrong thing would look like a passing test. + */ + async startCancel(bookingId: number): Promise { + const modal = await this.openDetails(bookingId); + await this.openOverflowMenu(modal); + const icons = await this.page + .locator('.cdk-overlay-container button[mat-menu-item] icon') + .allTextContents(); + const deletes = icons.filter((i) => i.trim() === 'delete').length; + if (deletes !== 1) { + throw new Error( + `expected exactly one Cancel action in the booking menu, found ` + + `${deletes}. The menu holds: ${JSON.stringify(icons.map((i) => i.trim()))}. ` + + `More than one means this booking also offers "remove series" or ` + + `"remove group", and picking blindly could cancel the wrong thing.`, + ); + } + const remove = await this.menuAction('delete'); + if (!remove) throw new Error('no Cancel action in the booking menu'); + await remove.click(); + } + + /** + * Open a booking for editing and land on the pre-filled invite form. + * + * Needs `visitors.allow_editing` — without it `can_edit` is false for a + * visitor booking and the menu simply has no Edit item, which would look + * like a broken selector. + */ + async startEdit(bookingId: number): Promise { + const modal = await this.openDetails(bookingId); + await this.openOverflowMenu(modal); + const edit = await this.menuAction('edit'); + if (!edit) { + // Say what IS in the menu. "Edit is missing" and "my locator cannot + // see Edit" look identical as a selector timeout, and only one of + // them is a problem with the app's configuration. + const items = await this.page + .locator('.cdk-overlay-container button[mat-menu-item]') + .evaluateAll((els) => + els.map((e) => (e.textContent ?? '').replace(/\s+/g, ' ').trim()), + ); + throw new Error( + `no Edit action in the booking menu. The menu contains ` + + `${items.length} item(s): ${JSON.stringify(items)}. ` + + `Visitor bookings need \`app.visitors.allow_editing = true\`, and a ` + + `booking that is checked in or already done cannot be edited at all.`, + ); + } + await edit.click(); + await expect( + this.page.locator('invite-visitor-form'), + 'editing did not land on the visitor form', + ).toBeVisible({ timeout: 30_000 }); + } +} diff --git a/e2e/tsconfig.json b/e2e/tsconfig.json new file mode 100644 index 0000000000..55d79c5f9b --- /dev/null +++ b/e2e/tsconfig.json @@ -0,0 +1,26 @@ +{ + // Type-checking for the e2e suite. Nothing else covered these files: + // Playwright transpiles specs without checking them, and no project in the + // workspace includes `e2e/support` or `apps/*/e2e`. Real errors sat in the + // tree while every test passed green. + // + // Run with: bun run e2e:typecheck + "extends": "../tsconfig.base.json", + "compilerOptions": { + "noEmit": true, + "skipLibCheck": true, + "target": "es2022", + "module": "preserve", + "moduleResolution": "bundler", + "esModuleInterop": true, + "lib": ["dom", "es2022"], + "types": ["node"], + // ON here even though the workspace base has it off. This is the setting + // that catches the mistake this file exists for: a `let id: number | + // undefined` assigned from an untyped helper and then passed somewhere + // expecting a number. Without it the check compiles and finds nothing. + // Scoped to the e2e project so the app build is unaffected. + "strictNullChecks": true + }, + "include": ["support/**/*.ts", "../apps/workplace/e2e/**/*.ts"] +} diff --git a/package.json b/package.json index c766696c0a..de1fc318de 100644 --- a/package.json +++ b/package.json @@ -34,6 +34,7 @@ "test": "ng test", "lint": "nx workspace-lint && ng lint", "e2e": "ng e2e", + "e2e:typecheck": "tsc --noEmit -p e2e/tsconfig.json", "affected:apps": "nx affected:apps", "affected:libs": "nx affected:libs", "affected:build": "nx affected:build",