diff --git a/E2E_USER_STORIES.md b/E2E_USER_STORIES.md index e6a98ff3619..dc2c5d07451 100644 --- a/E2E_USER_STORIES.md +++ b/E2E_USER_STORIES.md @@ -299,7 +299,7 @@ task that found it, so the row can be traced. | 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.** **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. | +| 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)** | **partial** — `desk-initialisation.spec.ts` (DESK-21) covers the desk form. It holds the 300 ms settle timer of the desk flow, so its edits to the title and "Require locker" come before the form reset on any hardware. It fails when the replay of the "Require locker" edit is removed. No spec covers the other forms, and the flows below still converge on the form state. History: 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 @@ -327,7 +327,7 @@ Config gaps caused several production incidents, and they are invisible to UI sp or through `use_bookings`?** Nobody has answered it. If the answer is "the calendar", the green rows in §1b guard a path those customers never take. This needs a product answer, not more specs. -- **Nine rows are blocked on product fixes, not on test effort** (REG-08, REG-09, REG-10, +- **Eight rows are blocked on product fixes, not on test effort** (REG-08, REG-09, VIS-15, ROOM-11, ROOM-13, ROOM-14, ROOM-21, ROOM-22). All were found by this suite. Leaving them visible here is the point — a blocked row is coverage information, a deleted row is not. - **One row is blocked by the environment rather than by a bug** (AUTH-E2E-08), and it says @@ -355,8 +355,9 @@ Config gaps caused several production incidents, and they are invisible to UI sp `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 +- **REG-10 is invisible to the booking flows of this suite by design.** `bookDeskViaUI` converges + on the form state, so no flow will catch it regressing. Only `desk-initialisation.spec.ts` + (DESK-21) detects it, and only for the desk form: it holds the settle timer of the desk flow. 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 on hardware this fast. Deleting the converging block passes serially and fails about one full parallel run in three, on the symptom rather than on the mechanism. diff --git a/apps/outlook-addin/src/app/desks/desk-form.component.ts b/apps/outlook-addin/src/app/desks/desk-form.component.ts index 97c9a6ce507..4d8ef21e47b 100644 --- a/apps/outlook-addin/src/app/desks/desk-form.component.ts +++ b/apps/outlook-addin/src/app/desks/desk-form.component.ts @@ -1,16 +1,15 @@ -import { - Component, - inject, - Injector, - OnInit, -} from '@angular/core'; +import { Component, computed, inject, Injector, OnInit } from '@angular/core'; import { FormsModule } from '@angular/forms'; import { FormField } from '@angular/forms/signals'; import { MatCheckboxModule } from '@angular/material/checkbox'; import { MatFormFieldModule } from '@angular/material/form-field'; import { MatInputModule } from '@angular/material/input'; import { AssetListFieldComponent } from '@placeos/assets'; -import { BookingFormService, DeskListFieldComponent } from '@placeos/bookings'; +import { + BookingFormService, + DeskListFieldComponent, + requiresLocker, +} from '@placeos/bookings'; import { AsyncHandler, Desk, @@ -67,7 +66,10 @@ import { name="start-time" [ngModel]="model().date" (ngModelChange)=" - model.update((m) => ({ ...m, date: $event })) + model.update((m) => ({ + ...m, + date: $event, + })) " [ngModelOptions]="{ standalone: true }" > @@ -96,9 +98,7 @@ import {
@@ -395,6 +392,11 @@ export class NewDeskFormDetailsComponent public readonly setFeature = (f, e) => this._state.setFeature(f, e); + /** Apply a user change from a control that has no `[formField]` binding. */ + public readonly setUserValues: BookingFormService['setUserValues'] = ( + value, + ) => this._state.setUserValues(value); + public get max_duration() { return ( this._settings.get('app.desks.max_duration') || @@ -414,6 +416,10 @@ export class NewDeskFormDetailsComponent return this._settings.get('app.desks.can_book_lockers'); } + public readonly requires_locker = computed(() => + requiresLocker(this.model().secondary_resource), + ); + public get can_recurr() { return this._settings.get('app.desks.allow_recurrence'); } @@ -517,10 +523,10 @@ export class NewDeskFormDetailsComponent } public onRecurrenceChange(recurrence: BookingRecurrence) { - this.model.update((m) => ({ ...m, ...recurrence })); + this.setUserValues(recurrence); } public onFirstInstanceChange(date: number) { - this.model.update((m) => ({ ...m, date })); + this.setUserValues({ date }); } } diff --git a/apps/workplace/src/tests/book/desk-flow/desk-flow-confirm.component.spec.ts b/apps/workplace/src/tests/book/desk-flow/desk-flow-confirm.component.spec.ts index 6ccaaefdcc3..2bede755281 100644 --- a/apps/workplace/src/tests/book/desk-flow/desk-flow-confirm.component.spec.ts +++ b/apps/workplace/src/tests/book/desk-flow/desk-flow-confirm.component.spec.ts @@ -232,6 +232,17 @@ describe('NewDeskFlowConfirmComponent', () => { expect(spectator.component.needs_locker).toBe(true); }); + it('should not require a locker for a saved `{}` value', () => { + // Some saved desk bookings hold `{}`, which an earlier default of + // the form wrote. + const settings = spectator.inject(SettingsService); + vi.mocked(settings.get).mockImplementation( + (key: string) => key === 'app.desks.can_book_lockers', + ); + model.update((m) => ({ ...m, secondary_resource: {} })); + expect(spectator.component.needs_locker).toBe(false); + }); + it('should not require a locker when locker booking is disabled', () => { model.update((m) => ({ ...m, secondary_resource: 'locker' })); expect(spectator.component.needs_locker).toBeFalsy(); diff --git a/apps/workplace/src/tests/book/desk-flow/desk-form-details.component.spec.ts b/apps/workplace/src/tests/book/desk-flow/desk-form-details.component.spec.ts index e8d29231325..08d92e33301 100644 --- a/apps/workplace/src/tests/book/desk-flow/desk-form-details.component.spec.ts +++ b/apps/workplace/src/tests/book/desk-flow/desk-form-details.component.spec.ts @@ -1,7 +1,9 @@ -import { signal, SimpleChange } from '@angular/core'; +import { NO_ERRORS_SCHEMA, signal, SimpleChange } from '@angular/core'; +import { By } from '@angular/platform-browser'; import { createComponentFactory, Spectator } from '@ngneat/spectator/vitest'; import { BookingFormService } from '@placeos/bookings'; import { OrganisationService, SettingsService } from '@placeos/common'; +import { TranslatePipe } from '@placeos/components'; import { MockProvider } from 'ng-mocks'; import { NewDeskFormDetailsComponent } from '../../../app/book/desk-flow/desk-form-details.component'; @@ -10,10 +12,24 @@ describe('NewDeskFormDetailsComponent', () => { let model: ReturnType>>; let settings: Record; let set_options: any; + let set_user_values: ReturnType; const createComponent = createComponentFactory({ component: NewDeskFormDetailsComponent, detectChanges: false, + // The specs that render the template test the event bindings only, + // so the form field components are not necessary. + overrideComponents: [ + [ + NewDeskFormDetailsComponent, + { + set: { + imports: [TranslatePipe], + schemas: [NO_ERRORS_SCHEMA], + }, + }, + ], + ], providers: [ MockProvider(OrganisationService, { building_list: signal([]), @@ -26,6 +42,7 @@ describe('NewDeskFormDetailsComponent', () => { beforeEach(() => { settings = {}; set_options = vi.fn(); + set_user_values = vi.fn(); model = signal>({ id: '', date: Date.now(), @@ -42,6 +59,7 @@ describe('NewDeskFormDetailsComponent', () => { options: signal({ type: 'desk', group: false }), features: signal([]), setOptions: set_options, + setUserValues: set_user_values, setFeature: vi.fn(), model, get auto_allocation() { @@ -62,17 +80,46 @@ describe('NewDeskFormDetailsComponent', () => { expect(set_options).toHaveBeenCalledWith({ group: true }); }); - it('should merge recurrence changes into the model', () => { + it('should send recurrence changes to the service as user edits', () => { spectator.component.onRecurrenceChange({ recurrence_type: 'weekly', }); - expect(model().recurrence_type).toBe('weekly'); + expect(set_user_values).toHaveBeenCalledWith({ + recurrence_type: 'weekly', + }); }); - it('should update the model date on first-instance change', () => { + it('should send the first-instance date to the service as a user edit', () => { const date = new Date('2026-05-01T00:00:00.000Z').valueOf(); spectator.component.onFirstInstanceChange(date); - expect(model().date).toBe(date); + expect(set_user_values).toHaveBeenCalledWith({ date }); + }); + + it('should send a "Require locker" change to the service as a user edit', () => { + settings['app.desks.can_book_lockers'] = true; + spectator.setInput('form', (() => ({})) as any); + spectator.detectChanges(); + + const checkbox = spectator.debugElement + .queryAll(By.css('mat-checkbox')) + .find((_) => _.nativeElement.textContent.includes('LOCKER')); + checkbox.triggerEventHandler('ngModelChange', true); + + expect(set_user_values).toHaveBeenCalledWith({ + secondary_resource: 'locker', + }); + }); + + it('should send a start time change to the service as a user edit', () => { + spectator.setInput('form', (() => ({})) as any); + spectator.detectChanges(); + + const date = new Date('2026-05-01T02:00:00.000Z').valueOf(); + spectator.debugElement + .query(By.css('a-time-field')) + .triggerEventHandler('ngModelChange', date); + + expect(set_user_values).toHaveBeenCalledWith({ date }); }); it('should apply the selected desk to resources when the form changes', () => { diff --git a/e2e/support/desk/GROUP_DESK.md b/e2e/support/desk/GROUP_DESK.md index a98a2c68e2e..20c8bf33dad 100644 --- a/e2e/support/desk/GROUP_DESK.md +++ b/e2e/support/desk/GROUP_DESK.md @@ -33,9 +33,9 @@ stack port. The config rejects non-local backends. The selection runs six group tests and one backend preflight. Results are in `reports/e2e/workplace/index.html`. -The group page reselects Group after date initialisation, which can reset the -form (REG-10). This suite does not claim to cover that separate initialisation -regression. It waits for actual directory results rather than selecting the +The group page selects Group again after date initialisation as a safeguard. +Form initialisation keeps a Group selection since the PPT-2643 fix. This suite +does not cover form initialisation. It waits for actual directory results rather than selecting the similarly labelled external-user option. Group selection uses the existing English button label. It does not require the diff --git a/e2e/support/desk/desk-form.page.ts b/e2e/support/desk/desk-form.page.ts index 490e7be5b01..0734d09d90d 100644 --- a/e2e/support/desk/desk-form.page.ts +++ b/e2e/support/desk/desk-form.page.ts @@ -20,11 +20,13 @@ * ## The form is rebuilt while it initialises (REG-10 / PPT-2643) * * When async init completes the form is rebuilt and defaults are restored — - * title back to "Booking", All Day back off, Require locker back ON. It is a + * title back to "Booking", All Day back off. It is a * RACE, not a step: on a warm run it lands before anything is typed. Every * setter here is therefore used inside a converging block by its callers, the * same workaround `bookDeskViaUI` uses, and for the same reason it means these - * specs cannot DETECT that bug (REG-10 has its own row saying so). + * specs cannot DETECT that bug (REG-10 has its own row saying so). The one + * exception is `desk-initialisation.spec.ts`, which holds the settle timer of + * the desk flow so that its edits come before the rebuild. */ import { Locator, Page, expect } from '@playwright/test'; @@ -82,9 +84,8 @@ export class DeskForm { /** * The "Require locker" checkbox. * - * It defaults to CHECKED and no lockers are seeded, which makes the form - * unsatisfiable with no error shown — the single most confusing thing about - * this form. Every spec has to clear it. + * It defaults to unchecked. Specs clear it as a safeguard, because no + * lockers are seeded. * * Matched on label text, which breaks the no-visible-text rule and is * inherited from the dev's `setCheckbox` helper: these two checkboxes carry diff --git a/e2e/support/desk/desk-group.page.ts b/e2e/support/desk/desk-group.page.ts index 2d41b10b911..3b6f92e16b9 100644 --- a/e2e/support/desk/desk-group.page.ts +++ b/e2e/support/desk/desk-group.page.ts @@ -20,7 +20,7 @@ export class DeskGroupPage { await this.form.open(); await this.form.groupModeButton.click(); await this.form.pickDate(slot.date_ms); - // Date/init can rebuild the form and restore single mode (REG-10). + // Safeguard: this sets Group mode again. It does not toggle the mode. await this.form.groupModeButton.click(); await expect(this.form.root.locator('a-user-list-field')).toBeVisible(); await this.form.setChecked(this.form.requireLocker, false); diff --git a/e2e/support/flows.ts b/e2e/support/flows.ts index 0458db09faf..8f2615e9395 100644 --- a/e2e/support/flows.ts +++ b/e2e/support/flows.ts @@ -63,8 +63,8 @@ export async function bookDeskViaUI( // re-affirmed after desk selection, because the form can be rebuilt underneath // us — see the converging block below. // - // - "Require locker" defaults to CHECKED and no lockers are seeded, so leaving - // it on makes the form unsatisfiable, with no error shown. + // - "Require locker" defaults to unchecked. It is cleared here as a + // safeguard, because no lockers are seeded. // - "All Day" replaces the default slot, which is the next 5-minute boundary. // That default makes the booking implicitly time-sensitive: a slow run // crosses the boundary, the start time falls into the past, and the form @@ -97,7 +97,7 @@ export async function bookDeskViaUI( // // The form is rebuilt when its async initialisation (org data, settings, // resource lists) completes, and that rebuild restores defaults — title back - // to "Booking", All Day back off, Require locker back on. Crucially it is a + // to "Booking", All Day back off. Crucially it is a // RACE, not a step: on a warm run it lands before we touch anything and // nothing is lost, on a cold one it lands mid-flow and silently discards our // input. Locally that showed up as a booking created under the wrong title; in diff --git a/libs/bookings/src/lib/booking-form.model.ts b/libs/bookings/src/lib/booking-form.model.ts index e3970ba90d7..6f3787e1d67 100644 --- a/libs/bookings/src/lib/booking-form.model.ts +++ b/libs/bookings/src/lib/booking-form.model.ts @@ -60,7 +60,8 @@ export interface BookingFormValue { booked_by: User; booked_by_id: string; booked_by_email: string; - secondary_resource: any; + /** Read this value with `requiresLocker`. */ + secondary_resource: unknown; location: string; attendance_type: string; phone?: string; @@ -91,6 +92,16 @@ export interface BookingFormValue { is_assgined: boolean; } +/** + * Whether a `secondary_resource` form value is a request for a locker. + * + * Only a text value is a request. Some saved desk bookings hold `{}`, which + * an earlier default of the form wrote. That value is not a request. + */ +export function requiresLocker(secondary_resource: unknown): boolean { + return typeof secondary_resource === 'string' && !!secondary_resource; +} + export function bookingAttachments(booking: Booking = new Booking()): string[] { booking = booking || new Booking(); const extension_data = booking.extension_data || {}; @@ -156,10 +167,11 @@ export function bookingFormValue( booked_by: currentUser(), booked_by_id: booking.booked_by_id || '', booked_by_email: booking.booked_by_email || '', + // Empty when no secondary resource is requested. secondary_resource: extension_data.other_asset_type || extension_data.secondary_resource || - {}, + '', location: extension_data.location || '', attendance_type: extension_data.attendance_type || 'ANY', phone: extension_data.phone || '', diff --git a/libs/bookings/src/lib/booking-form.service.ts b/libs/bookings/src/lib/booking-form.service.ts index 3b3fd23622c..5fd163c312f 100644 --- a/libs/bookings/src/lib/booking-form.service.ts +++ b/libs/bookings/src/lib/booking-form.service.ts @@ -160,6 +160,26 @@ type GroupContainerForm = Partial & { extension_data?: Record; }; +/** Edits the user made on a booking form before a form reset. */ +interface UserEdits { + /** Values of the fields that the user changed. */ + values: Partial; + /** Group selection. Options have no dirty flags to read. */ + options: Pick; +} + +/** Fields that only the desk form lets the user change. */ +const DESK_ONLY_FIELDS: (keyof BookingFormValue)[] = [ + 'secondary_resource', + 'recurrence_type', + 'recurrence_days', + 'recurrence_nth_of_month', + 'recurrence_interval', + 'recurrence_end', + 'recurrence_instances', + 'recurrence_custom', +]; + /** A booking type, or `undefined` for the blank `' '` placeholder that * `new Booking()` sets when no type is given. */ function knownBookingType( @@ -222,10 +242,10 @@ export class BookingFormService extends AsyncHandler { public readonly view = signal('form'); /** - * Edits the user made while `newForm` was waiting on the current user, - * carried across the deferred re-entry so the reset does not discard them. + * Edits the user made before a form reset, carried across the reset so + * that it does not discard them. */ - private _pending_user_edits: Partial | null = null; + private _pending_user_edits: UserEdits | null = null; /** Apply a partial patch to the booking form model. */ private _patch(value: Partial, _opts?: unknown) { @@ -237,8 +257,10 @@ export class BookingFormService extends AsyncHandler { * * Reads the signal-forms dirty flags rather than diffing against a * default. Programmatic writes (`_patch`, `model.set`) do not mark a field - * dirty, so this returns genuine user input and nothing else — which is - * what makes it safe to replay over a freshly loaded booking. + * dirty, so this returns user input — which is what makes it safe to + * replay over a freshly loaded booking. One exception: the recurrence + * field adjusts a recurrence when the date changes, and that write goes + * through `setUserValues`. */ private _userEditedValues(): Partial { const form = this.form; @@ -268,11 +290,65 @@ export class BookingFormService extends AsyncHandler { * capture with an empty one on the second call. */ private _captureUserEdits() { - const edits = { - ...(this._pending_user_edits || {}), - ...this._userEditedValues(), - }; - this._pending_user_edits = Object.keys(edits).length ? edits : null; + const pending = this._pending_user_edits; + const values = { ...pending?.values, ...this._userEditedValues() }; + // The first capture gives the options. A reset that ran after it has + // cleared them. + const { group, members } = this._options(); + const options = + pending?.options ?? + (group || members?.length ? { group, members } : {}); + const has_edits = + Object.keys(values).length || Object.keys(options).length; + this._pending_user_edits = has_edits ? { values, options } : null; + } + + /** + * The stashed edits that apply to a reset for the given booking type. + * + * A change from one booking flow to a different one does not reset the + * form, so the stash can hold edits from the form of a different booking + * type. A locker request, a recurrence, and a group selection apply only + * to a desk form, because no other form shows them. All other edits + * apply to each form. + */ + private _userEditsFor(type?: BookingType): UserEdits | null { + const edits = this._pending_user_edits; + if (!edits || type === 'desk') return edits; + const values = { ...edits.values }; + for (const key of DESK_ONLY_FIELDS) delete values[key]; + return { values, options: {} }; + } + + /** + * Re-apply the user's edits over the booking that a reset loaded. + * + * A replayed start time or duration moves the end time with it. If the + * end time stays at the value of the loaded booking, the time sync + * calculates the duration from it and discards the user's value. + */ + private _replayUserEdits(edits: UserEdits | null) { + if (!edits) return; + this._patch(edits.values); + const { date, duration, all_day } = this.model(); + const window_changed = + 'date' in edits.values || 'duration' in edits.values; + if (all_day || !window_changed) return; + this._patch({ date_end: addMinutes(date, duration).valueOf() }); + } + + /** + * Apply a change the user made in a control that has no `[formField]` + * binding. + * + * Marks each changed field dirty, as `[formField]` does, so + * `_userEditedValues` keeps the change across a form reset. + */ + public setUserValues(value: Partial) { + this._patch(value); + for (const key of Object.keys(value) as (keyof BookingFormValue)[]) { + this.form[key]?.().markAsDirty(); + } } private _syncAssetOptions() { @@ -752,7 +828,7 @@ export class BookingFormService extends AsyncHandler { }); return; } - const user_edits = this._pending_user_edits; + const user_edits = this._userEditsFor(type); this._pending_user_edits = null; // Never apply an existing booking's edit state to a different type // (e.g. editing parking then opening the desk form). @@ -795,9 +871,7 @@ export class BookingFormService extends AsyncHandler { // the time-sync window; and before `_syncWindowIfUnchanged`, which // compares against `initial_date`/`initial_duration` and so leaves a // user-changed window alone of its own accord. - if (user_edits && Object.keys(user_edits).length) { - this._patch(user_edits, { emitEvent: false }); - } + this._replayUserEdits(user_edits); // Give an untyped form the flow type, so validators that depend on // the type (e.g. parking plate number) run for callers that do not // set it themselves. @@ -823,7 +897,12 @@ export class BookingFormService extends AsyncHandler { booking.duration, ); this._booking.set(new Booking(booking)); - this._options.set({ type: this._options().type }); + // A new form clears the options, except a group selection that the + // user made before this reset. + this._options.set({ + type: this._options().type, + ...user_edits?.options, + }); // Persist immediately to avoid edit-flow races where the consumer // reloads from session before async form change events occur. this.storeForm(); @@ -1009,12 +1088,12 @@ export class BookingFormService extends AsyncHandler { // already entered. Capture before `form().reset()` clears the dirty // flags `_userEditedValues` reads. this._captureUserEdits(); - const user_edits = this._pending_user_edits; + const user_edits = this._userEditsFor(expected_type); // Flows call `loadForm(type)` and then `newForm(type)` in the same tick - // (desk-flow.component.ts:62 and :65, and the locker/parking - // equivalents). Leave the capture in place so that second reset replays - // it too, and release it at the end of the tick, where it can no longer - // reach an unrelated form. + // (desk-flow.component.ts, and the locker/parking equivalents). Leave + // the capture in place so that second reset replays it too, and + // release it at the end of the tick, where it can no longer reach an + // unrelated form. queueMicrotask(() => (this._pending_user_edits = null)); this._startNetwork(); this._calendar.loadCalendars(); @@ -1057,9 +1136,7 @@ export class BookingFormService extends AsyncHandler { // Re-apply the user's own edits over the loaded booking, before // `applyDurationSettings` so a restored `all_day` still drives the // time-sync window — same ordering as `newForm`. - if (user_edits && Object.keys(user_edits).length) { - this._patch(user_edits, { emitEvent: false }); - } + this._replayUserEdits(user_edits); // Give an untyped booking the flow type. Otherwise the flow sees a // type mismatch and replaces the booking with a new form. if (expected_type) { diff --git a/libs/bookings/src/lib/bookings.fn.ts b/libs/bookings/src/lib/bookings.fn.ts index e2ace568bb6..ee8ec4a0188 100644 --- a/libs/bookings/src/lib/bookings.fn.ts +++ b/libs/bookings/src/lib/bookings.fn.ts @@ -361,8 +361,13 @@ export const saveBooking = async ( /** * Remove booking from the database * @param id ID of the booking to remove + * @param q Set `instance` and `start_time` to remove only one instance of a + * recurring booking */ -export function removeBooking(id: string, q: any = {}): Promise { +export function removeBooking( + id: string, + q: { instance?: boolean; start_time?: number } = {}, +): Promise { if (q.instance) { return removeBookingInstance(id, q.start_time); } diff --git a/libs/bookings/src/test/booking-form.service.spec.ts b/libs/bookings/src/test/booking-form.service.spec.ts index d2d01b2ab87..520772f0972 100644 --- a/libs/bookings/src/test/booking-form.service.spec.ts +++ b/libs/bookings/src/test/booking-form.service.spec.ts @@ -3533,6 +3533,121 @@ describe('BookingFormService', () => { expect(spectator.service.model().all_day).toBe(true); }); + it('does not request a locker on a new desk form', () => { + spectator.service.loadForm('desk'); + spectator.service.newForm('desk'); + + expect(spectator.service.model().secondary_resource).toBe(''); + }); + + it('keeps a "Require locker" choice made before the flow initialises the form', () => { + spectator.service.setUserValues({ secondary_resource: 'locker' }); + + spectator.service.loadForm('desk'); + spectator.service.newForm('desk'); + + expect(spectator.service.model().secondary_resource).toBe('locker'); + }); + + it('keeps a cleared "Require locker" over a stored locker request', () => { + sessionStorage.setItem( + 'PLACEOS.booking_form', + JSON.stringify({ secondary_resource: 'locker' }), + ); + spectator.service.setUserValues({ secondary_resource: '' }); + + spectator.service.loadForm('desk'); + + expect(spectator.service.model().secondary_resource).toBe(''); + }); + + it('keeps a start time chosen before the flow initialises the form', () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date(2026, 9, 12, 9, 0)); + try { + const date = new Date(2026, 9, 12, 11, 0).valueOf(); + spectator.service.setUserValues({ date }); + // Let the time sync set the end time for the user's start. + TestBed.tick(); + + spectator.service.loadForm('desk'); + spectator.service.newForm('desk'); + // The delayed window sync must not replace the user's time. + vi.runAllTimers(); + TestBed.tick(); + + const model = spectator.service.model(); + expect(model.date).toBe(date); + // The end time moves with the start time. The end time of + // the new booking is before the user's start, and the time + // sync would reduce the duration to the minimum. + expect(model.duration).toBe(60); + expect(model.date_end).toBe(date + 60 * 60 * 1000); + } finally { + vi.useRealTimers(); + } + }); + + it('keeps a duration chosen before the flow initialises the form', () => { + userEdits('duration', 120); + // Let the time sync set the end time for the user's duration. + TestBed.tick(); + + spectator.service.loadForm('desk'); + spectator.service.newForm('desk'); + TestBed.tick(); + + expect(spectator.service.model().duration).toBe(120); + }); + + it('keeps a recurrence chosen before the flow initialises the form', () => { + spectator.service.setUserValues({ + recurrence_type: 'weekly', + recurrence_interval: 2, + }); + + spectator.service.loadForm('desk'); + spectator.service.newForm('desk'); + + const { recurrence_type, recurrence_interval } = + spectator.service.model(); + expect(recurrence_type).toBe('weekly'); + expect(recurrence_interval).toBe(2); + }); + + it('keeps a group selection made before the flow initialises the form', () => { + const members = [new StaffUser({ email: 'jo@example.com' })]; + spectator.service.setOptions({ group: true, members }); + + spectator.service.loadForm('desk'); + spectator.service.newForm('desk'); + + expect(spectator.service.options().group).toBe(true); + expect(spectator.service.options().members).toEqual(members); + }); + + it('keeps a choice made on a desk form that a reset already initialised', () => { + // The usual path in the app: the landing page clears the form, + // which gives the model the type `desk`. + spectator.service.clearForm(); + spectator.service.setUserValues({ secondary_resource: 'locker' }); + + spectator.service.loadForm('desk'); + spectator.service.newForm('desk'); + + expect(spectator.service.model().secondary_resource).toBe('locker'); + }); + + it('keeps input on a visitor form after the landing page cleared the form', () => { + spectator.service.clearForm(); + userEdits('title', 'Visitor title'); + + spectator.service.loadForm('visitor'); + spectator.service.newForm('visitor'); + + expect(spectator.service.model().title).toBe('Visitor title'); + }); + it('carries a typed title between booking forms, deliberately', () => { // Switching desk -> parking without leaving the booking area does not // reset the form, so the user's own typing follows them. Pinned @@ -3545,6 +3660,26 @@ describe('BookingFormService', () => { expect(spectator.service.model().title).toBe('Desk title'); }); + it('does not move desk choices to the form of a different booking type', () => { + // No other form shows a locker request, a recurrence, or a group + // selection. + spectator.service.newForm('desk'); + spectator.service.setUserValues({ + secondary_resource: 'locker', + recurrence_type: 'weekly', + }); + spectator.service.setOptions({ group: true }); + + spectator.service.loadForm('parking'); + spectator.service.newForm('parking'); + + const { secondary_resource, recurrence_type } = + spectator.service.model(); + expect(secondary_resource).toBe(''); + expect(recurrence_type).toBe('none'); + expect(spectator.service.options().group).toBeFalsy(); + }); + it('does not carry those edits into a later unrelated form', () => { userEdits('title', 'Quiet corner desk'); spectator.service.loadForm('desk');