From 041ad2ff3358267e36433b7b1d5b101f692a57f4 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Wed, 5 Aug 2026 16:20:35 +1000 Subject: [PATCH 1/2] fix(bookings): keep form input across the reset flows actually run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PPT-2643 again. #478 fixed `newForm`'s deferred re-entry, but that branch is not the one the booking flows take. The current user is restored from the localStorage cache within about 50ms of bootstrap, whereas every flow calls its form lifecycle only after org data lands — `NewDeskFlowComponent.ngOnInit` awaits `waitUntilInitialised()` plus a 300ms settle, then calls `loadForm` and, for a fresh booking, `newForm`. So `currentUserIsLoaded()` is already true, the deferral never fires, and the captured-edits replay never runs. `loadForm` had no capture at all, and it is the first of the two resets. Its `model.set(...)` restores defaults — `all_day` false, a truthy `secondary_resource` that re-checks "Require locker" — over whatever the user typed into a form that has been interactive since first paint. Capture in `loadForm` too, and replay over the loaded booking before `applyDurationSettings` so a restored `all_day` still drives the time-sync window. The capture merges rather than replaces, because `form().reset()` clears the dirty flags the capture reads, so the `newForm` that follows in the same tick would otherwise overwrite a real capture with an empty one. The stash is released on a microtask, which is late enough for that chained reset and early enough that it cannot reach an unrelated form. Two specs, both seen red first, driving the ordinary path with no mocking and no runtime probe neutralised: input entered before initialisation survives `loadForm` + `newForm`, and it is not resurrected in a later form. Fixes PPT-2643 Co-Authored-By: Claude Fable 5 --- libs/bookings/src/lib/booking-form.service.ts | 37 +++++++++++++++- .../src/test/booking-form.service.spec.ts | 44 +++++++++++++++++++ 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/libs/bookings/src/lib/booking-form.service.ts b/libs/bookings/src/lib/booking-form.service.ts index 7861525673..18b27aa541 100644 --- a/libs/bookings/src/lib/booking-form.service.ts +++ b/libs/bookings/src/lib/booking-form.service.ts @@ -352,6 +352,22 @@ export class BookingFormService extends AsyncHandler { return edits; } + /** + * Stash the user's in-progress edits for the reset that is about to run. + * + * Merges rather than replaces. A flow resets twice in a row — `loadForm` + * then `newForm` — and the first `form().reset()` clears the dirty flags + * `_userEditedValues` reads, so a plain assignment would overwrite a real + * capture with an empty one on the second call. + */ + private _captureUserEdits() { + const edits = { + ...(this._pending_user_edits || {}), + ...this._userEditedValues(), + }; + this._pending_user_edits = Object.keys(edits).length ? edits : null; + } + private _syncAssetOptions() { const { date, duration } = untracked(this.model); const next_asset_window = assetWindowKey(date, duration); @@ -821,7 +837,7 @@ export class BookingFormService extends AsyncHandler { // destroyed by the reset below. Capture it on the way back in — // as late as possible, so we take the user's final state. currentUserLoaded().then(() => { - this._pending_user_edits = this._userEditedValues(); + this._captureUserEdits(); this.newForm(type, booking); }); return; @@ -1073,6 +1089,19 @@ export class BookingFormService extends AsyncHandler { currentUserLoaded().then(() => this.loadForm(expected_type)); return; } + // Same hazard as `newForm`, and the one the flows actually hit: the form + // is rendered from first paint, but every flow calls this only after org + // data lands, so the reset below arrives on top of whatever the user has + // already entered. Capture before `form().reset()` clears the dirty + // flags `_userEditedValues` reads. + this._captureUserEdits(); + const user_edits = this._pending_user_edits; + // Flows call `loadForm(type)` and then `newForm(type)` in the same tick + // (desk-flow.component.ts:62 and :65, and the locker/parking + // equivalents). Leave the capture in place so that second reset replays + // it too, and release it at the end of the tick, where it can no longer + // reach an unrelated form. + queueMicrotask(() => (this._pending_user_edits = null)); this._startNetwork(); this._calendar.loadCalendars(); const data = JSON.parse( @@ -1111,6 +1140,12 @@ export class BookingFormService extends AsyncHandler { [null, undefined, ''], ); this._patch(booking_data, { emitEvent: false }); + // Re-apply the user's own edits over the loaded booking, before + // `applyDurationSettings` so a restored `all_day` still drives the + // time-sync window — same ordering as `newForm`. + if (user_edits && Object.keys(user_edits).length) { + this._patch(user_edits, { emitEvent: false }); + } this.applyDurationSettings(); this._form_value.set(this.model()); this._syncAssetOptions(); diff --git a/libs/bookings/src/test/booking-form.service.spec.ts b/libs/bookings/src/test/booking-form.service.spec.ts index d24a803df8..cf35a71ed5 100644 --- a/libs/bookings/src/test/booking-form.service.spec.ts +++ b/libs/bookings/src/test/booking-form.service.spec.ts @@ -2884,6 +2884,50 @@ describe('BookingFormService', () => { expect((savedBookings()[0] as Booking).asset_ids).toEqual(['desk-2']); }); + describe('initialisation after the user has already loaded', () => { + /** + * The case the flows actually hit. Every booking flow renders its form + * on first paint but initialises it late: `NewDeskFlowComponent.ngOnInit` + * awaits org initialisation plus a 300ms settle, then calls `loadForm` + * and — for a fresh booking — `newForm`, back to back. + * + * The current user is restored from the localStorage cache within about + * 50ms of bootstrap, long before org data arrives, so `newForm` never + * takes its deferred branch here. Nothing is mocked and no runtime probe + * is neutralised: this is the ordinary path. + */ + function userEdits(field: string, value: any) { + const node = (spectator.service.form as any)[field](); + node.value.set(value); + node.markAsDirty(); + } + + it('keeps input entered before the flow initialises the form', () => { + userEdits('title', 'Quiet corner desk'); + userEdits('all_day', true); + + // exactly what desk-flow.component.ts does once org data lands + spectator.service.loadForm('desk'); + spectator.service.newForm('desk'); + + expect(spectator.service.model().title).toBe('Quiet corner desk'); + expect(spectator.service.model().all_day).toBe(true); + }); + + it('does not carry those edits into a later unrelated form', () => { + userEdits('title', 'Quiet corner desk'); + spectator.service.loadForm('desk'); + spectator.service.newForm('desk'); + expect(spectator.service.model().title).toBe('Quiet corner desk'); + + // A form opened later must start clean, not inherit the last one. + spectator.service.newForm('desk'); + expect(spectator.service.model().title).not.toBe( + 'Quiet corner desk', + ); + }); + }); + describe('initialisation while the user is still loading', () => { /** * Put the service into the state `newForm` sees on a slow load: no From cc6bb36350a76301e05c7447877a57dbb7a1ba9f Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Wed, 5 Aug 2026 20:41:46 +1000 Subject: [PATCH 2/2] test(bookings): pin the cross-form carry-over of typed input Switching between booking forms without leaving the booking area does not reset the form, so edits captured for the initialisation replay follow the user across. That is a consequence of the fix worth stating rather than discovering later: only fields the user actually edited move, isCrossTypeEdit still discards the previous booking's identity, and leaving the section calls clearForm(). Co-Authored-By: Claude Fable 5 --- libs/bookings/src/test/booking-form.service.spec.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/libs/bookings/src/test/booking-form.service.spec.ts b/libs/bookings/src/test/booking-form.service.spec.ts index cf35a71ed5..02221fc67b 100644 --- a/libs/bookings/src/test/booking-form.service.spec.ts +++ b/libs/bookings/src/test/booking-form.service.spec.ts @@ -2914,6 +2914,18 @@ describe('BookingFormService', () => { expect(spectator.service.model().all_day).toBe(true); }); + it('carries a typed title between booking forms, deliberately', () => { + // Switching desk -> parking without leaving the booking area does not + // reset the form, so the user's own typing follows them. Pinned + // rather than left to chance: only fields they actually edited move, + // `isCrossTypeEdit` still discards the previous booking's identity, + // and leaving the booking section entirely calls `clearForm()`. + userEdits('title', 'Desk title'); + spectator.service.loadForm('parking'); + spectator.service.newForm('parking'); + expect(spectator.service.model().title).toBe('Desk title'); + }); + it('does not carry those edits into a later unrelated form', () => { userEdits('title', 'Quiet corner desk'); spectator.service.loadForm('desk');