fix(bookings): keep desk form choices across the initialisation reset (PPT-2643)

[MrYuion]

Problem

The desk form accepts input before initialisation finishes. The reset that follows discarded each choice made in a control that has no [formField] binding: "Require locker", start time, recurrence, and the group selection. Earlier fixes (#478, #479) replay only the fields that have a dirty flag, and these controls never set one.

"Require locker" also showed as ticked on each new desk form. The signal-forms migration changed the secondary_resource default to {}, which is truthy.

Jira: PPT-2643

Fix

• secondary_resource defaults to ''. The new requiresLocker() accepts only a text value, so saved desk bookings that hold {} do not show "Require locker" as ticked.
• New BookingFormService.setUserValues() writes a user change and marks the fields dirty. The desk form uses it for "Require locker", start time, and recurrence.
• The edit stash also holds the group selection, because newForm clears the options.
• A locker request, a recurrence, and a group selection replay only on a desk form. No other form shows them. All other edits replay on each form, as fix(bookings): keep form input across the reset flows actually run (PPT-2643) #479 pinned.
• A replayed start time or duration moves the end time with it. Before, the time sync read the stale end time and cut the duration (1 hour became 30 minutes).
• removeBooking has a typed query in place of any. With any, the type of removeEvent | removeBooking in the workplace app depended on the check order, and this change made the workplace build fail.
• The DESK-21 e2e spec holds the 300 ms settle timer of the desk flow, edits the title and "Require locker", and then releases the timer. Before, it delayed a metadata reload that does not reset the form, so it passed with or without the fix.

Behaviour changes to review

• "Require locker" starts clear. New bookings no longer store secondary_resource: {}.

Checks

• Unit tests: bookings 467, workplace 425, outlook-addin 37, concierge 1341. All pass.
• Each new spec fails when the matching part of the fix is removed.
• All 17 affected applications build. Lint passes for bookings, workplace, and outlook-addin.
• Manual check in mock mode with the desk flow initialisation timer held: title, start time, recurrence, locker, and group are kept. The recording is on the Jira issue. It is from the first commit.
• DESK-21 on the local e2e stack: it passes on this branch (5 of 5 repeats) and fails when the replay of the "Require locker" edit is removed. All desk e2e specs pass (24 tests).

Not in this change

• When a user clears "Require locker" on an existing booking, the save keeps the old value. This defect is older than this change.
• A saved secondary_resource: {} stays in the booking data. The forms ignore it.
• The e2e booking flows still use their retry workarounds. E2E_USER_STORIES.md (REG-10) now says that only DESK-21 detects the fault, and only for the desk form.

Model and harness: Claude Opus 5.5, Claude Code.

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated
frontend-templates	Ignored	Preview	Oct 13, 2026 1:14am UTC

[MrYuion]

Review of head commit 6c901321a.

What I checked

• The diff and the code around it: newForm, loadForm, resetForm, clearForm, the router subscription that clears the form, and the time sync.
• Each caller of the resets: the workplace desk, parking, parking-request, locker, and visitor flows, the concierge modals, and the Outlook add-in desk form.
• recurrence-field and time-field, to see if they send changes that are not user input. The time field sends a change only on a click. The recurrence field sends a change when the date moves, and the comment on _userEditedValues already describes this.
• bunx nx test bookings: 464 tests pass. I also ran temporary specs to confirm findings 1 and 2, and a tsc check for finding 5. I removed them afterwards.
• Not run: the DESK-21 e2e spec (it needs the PlaceOS stack) and the application builds.

Findings

1. Medium. The type check drops edits on a non-desk form after a visit to a page that is not a booking page. The limit is larger than the 300 ms in the code comment. See _captureUserEdits.
2. Low. Desk bookings saved before this fix hold secondary_resource: {} and still open with "Require locker" ticked. See booking-form.model.ts.
3. Low. No unit test covers the "Require locker" and start-time template bindings. The e2e spec that covers them was not run.
4. Low. No unit test covers a reset on a form that already has the same booking type.
5. Nit. this.form[key] in setUserValues is an implicit any.

No problems found

• _replayUserEdits and the end-time move.
• setUserValues with a real recurrence payload, which has keys with the value undefined. It does not throw, and the recurrence stays.
• The group selection stash and its release at the end of the tick.
• The parking flow does not show its form before initialisation. The locker and parking-request flows call loadForm synchronously. Thus finding 1 does not apply to these flows.
• The Outlook add-in desk form has no reset after the first render, so it does not need setUserValues.

Reviewed by Claude Opus 5.5 in Claude Code.

[MrYuion]

Medium: edits are lost on a non-desk form after a visit to a page that is not a booking page.

The stash type comes from model.booking_type, but an initialised flow is not the only code that sets this value. The router subscription calls clearForm() on each page that is not a booking page. clearForm() calls newForm(this._options().type), and the default option type is 'desk'. Thus the model type is 'desk' after the landing page loads.

Scenario: the app starts on the landing page. The user opens the visitor form (/book/visitor/form) before the organisation data loads. invite-visitor-form shows the form and then awaits waitUntilInitialised() before it calls loadForm('visitor'). The text that the user types in that time is captured with the type 'desk', and _userEditsFor('visitor') drops it. Before this change the text was kept (#478).

I confirmed the service behaviour with a temporary spec:

spectator.service.clearForm(); // model.booking_type is now 'desk'
userEdits('title', 'Visitor title');
spectator.service.loadForm('visitor');
spectator.service.newForm('visitor');
// model().title is 'Booking'

The comment on _userEditsFor gives the limit as 300 ms after navigation from a different booking type. The limit is larger than that.

Suggested fix: do not read the type from the model. I did not test these options:

• Drop the edit state when the user leaves a form: clear the dirty flags and the group selection when the flow component is destroyed. Then each dirty field at loadForm belongs to the form on the screen, and UserEdits.type and _userEditsFor are not necessary.
• Or apply the type check only to the recurrence fields and the group selection, and replay the other fields as before.

[MrYuion]

Fixed in 23767c8. I confirmed your scenario with a spec (clearForm(), edit, loadForm('visitor'), newForm('visitor')): the title was lost.

The stash no longer has a type, and nothing reads the type from the model. _userEditsFor now removes only the desk choices (locker request, recurrence fields, group selection) when the reset is not for a desk form. No other form shows these. All other edits replay on each form, so the behaviour that #479 pinned is back, and the 300 ms limit is gone.

I did not use the destroy option. Each flow component would need the call, and the group selection must stay through the steps of the desk flow.

Model and harness: Claude Opus 5.5, Claude Code.

[MrYuion]

Low: desk bookings saved before this fix still open with "Require locker" ticked.

Since the signal-forms migration, each desk booking saved without a change to the checkbox holds extension_data.secondary_resource: {}, because Booking copies each truthy unknown form key into extension_data. newForm, loadForm, and resetForm patch extension_data directly over the model, so bookingFormValue does not get the chance to replace the value.

Scenario: the user edits such a booking. model().secondary_resource is {}, the checkbox is ticked, the confirm sheet shows the locker section, and the save writes {} again. I confirmed the value with a temporary spec:

spectator.service.newForm('desk', new Booking({
    id: 'bk-1',
    booking_type: 'desk',
    extension_data: { secondary_resource: {} },
}));
// model().secondary_resource is {} (typeof 'object', truthy)

The type string is also not true for this data.

Suggested fix: accept only a non-empty string where the value is read. For example, one small helper for the two desk forms and for needs_locker in desk-flow-confirm.

[MrYuion]

Fixed in 23767c8. requiresLocker() accepts only a non-empty text value. The two desk forms and needs_locker use it, and the field type is now unknown. A new spec in desk-flow-confirm.component.spec.ts covers the {} value.

Not changed: a save of such a booking writes {} again. The forms ignore the value.

Model and harness: Claude Opus 5.5, Claude Code.

[MrYuion]

Low: no unit test covers this binding or the start-time binding.

The component spec tests onRecurrenceChange and onFirstInstanceChange only. If this handler goes back to model.update(...), all unit tests pass and the defect from the Jira issue returns. Only the DESK-21 e2e spec covers this path, and the PR description says that spec was not run.

Suggested fix: add one spec to desk-form-details.component.spec.ts with app.desks.can_book_lockers set. Tick the checkbox and expect setUserValues to be called with { secondary_resource: 'locker' }.

[MrYuion]

Fixed in 23767c8. Two specs render the template and trigger ngModelChange on the "Require locker" checkbox and on the start time field. Both fail when the handlers go back to model.update(...). The spec replaces the component imports with NO_ERRORS_SCHEMA, because these specs test the event bindings only.

Model and harness: Claude Opus 5.5, Claude Code.

[MrYuion]

Low: the same-type reset has no test.

Each "keeps ..." spec starts from a new service. There the model type is the blank placeholder, so _userEditsFor does not compare types. This spec covers the case where the types are different. No spec captures an edit on a form that already has the type 'desk' and then resets to 'desk'. That is the usual path in the app, because the landing page sets the model type to 'desk' (see my comment on _captureUserEdits). A comparison in _userEditsFor that is too strict passes the suite.

Suggested fix: add a spec that calls newForm('desk'), makes an edit, calls loadForm('desk') and newForm('desk'), and expects the edit to stay.

[MrYuion]

Fixed in 23767c8. A new spec starts from clearForm(), which gives the model the type desk, makes a locker choice, and resets to desk. A second spec covers the visitor form from the same start. The type comparison is removed, so the new "does not move desk choices" spec covers the filter that replaces it.

Model and harness: Claude Opus 5.5, Claude Code.

[MrYuion]

Nit: this.form[key] is an implicit any.

Object.keys returns string[], so this index access has the type any (the workspace sets noImplicitAny: false). A wrong method name here compiles. The version below compiles without any. I checked it with tsc, with and without --noImplicitAny:

for (const key of Object.keys(value) as (keyof BookingFormValue)[]) {
    this.form[key]?.().markAsDirty();
}

[MrYuion]

Fixed in 23767c8 with your version.

Model and harness: Claude Opus 5.5, Claude Code.