Skip to content

fix(bookings): keep user input entered while the booking form initialises #478

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MrYuion merged 1 commit into from
Aug 4, 2026
Merged

fix(bookings): keep user input entered while the booking form initialises #478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 51 additions & 1 deletion libs/bookings/src/lib/booking-form.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,11 +314,44 @@ export class BookingFormService extends AsyncHandler {
private _asset_window = '';
public readonly view = signal<BookingFlowView>('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.
*/
private _pending_user_edits: Partial<BookingFormValue> | null = null;

/** Apply a partial patch to the booking form model. */
private _patch(value: Partial<BookingFormValue>, _opts?: unknown) {
this.model.update((m) => ({ ...m, ...value }));
}

/**
* The fields the user has actually edited, with their current values.
*
* 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.
*/
private _userEditedValues(): Partial<BookingFormValue> {
const form = this.form as any;
if (!form) return {};
const model = untracked(this.model) as Record<string, any>;
const edits: Record<string, any> = {};
for (const key of Object.keys(model || {})) {
const field = form[key];
if (typeof field !== 'function') continue;
// A field can throw if the tree has no matching sub-field, which
// is not worth losing the rest of the user's input over.
try {
if (field()?.dirty?.()) edits[key] = model[key];
} catch {
continue;
}
}
return edits;
}

private _syncAssetOptions() {
const { date, duration } = untracked(this.model);
const next_asset_window = assetWindowKey(date, duration);
Expand Down Expand Up @@ -783,9 +816,18 @@ export class BookingFormService extends AsyncHandler {

public newForm(type: BookingType, booking: Booking = new Booking({})) {
if (!currentUserIsLoaded()) {
currentUserLoaded().then(() => this.newForm(type, booking));
// The form is already rendered and interactive at this point, so
// anything typed or toggled before the user resolves would be
// 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.newForm(type, booking);
});
return;
}
const user_edits = this._pending_user_edits;
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).
if (isCrossTypeEdit(booking, type)) booking = new Booking({});
Expand Down Expand Up @@ -822,6 +864,14 @@ export class BookingFormService extends AsyncHandler {
),
{ emitEvent: false },
);
// Re-apply the user's own edits over the incoming booking. Done here,
// before `applyDurationSettings`, so a restored `all_day` still drives
// 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.applyDurationSettings();
this._syncAssetOptions();
const form_change = effect(
Expand Down
250 changes: 180 additions & 70 deletions libs/bookings/src/test/booking-form.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1717,13 +1717,13 @@ describe('BookingFormService', () => {
).mockResolvedValue(true);
const saved_desks: string[] = [];
vi.spyOn(spectator.service, 'postForm').mockImplementation(async () => {
const value = spectator.service.model();
saved_desks.push(value.asset_id);
return new Booking({
id: `booking-${saved_desks.length}`,
user_email: value.user_email,
asset_id: value.asset_id,
});
const value = spectator.service.model();
saved_desks.push(value.asset_id);
return new Booking({
id: `booking-${saved_desks.length}`,
user_email: value.user_email,
asset_id: value.asset_id,
});
});
spectator.service.newForm(
'desk',
Expand Down Expand Up @@ -1803,17 +1803,17 @@ describe('BookingFormService', () => {
extension_name: string;
}[] = [];
vi.spyOn(spectator.service, 'postForm').mockImplementation(async () => {
const value = spectator.service.model();
saved_forms.push({
asset_id: value.asset_id,
resource_id: value.resources?.[0]?.id,
extension_name: value.name,
});
return new Booking({
id: `booking-${saved_forms.length}`,
user_email: value.user_email,
asset_id: value.asset_id,
});
const value = spectator.service.model();
saved_forms.push({
asset_id: value.asset_id,
resource_id: value.resources?.[0]?.id,
extension_name: value.name,
});
return new Booking({
id: `booking-${saved_forms.length}`,
user_email: value.user_email,
asset_id: value.asset_id,
});
});
spectator.service.newForm(
'desk',
Expand Down Expand Up @@ -1897,14 +1897,14 @@ describe('BookingFormService', () => {
).mockResolvedValue(true);
const child_parent_ids: string[] = [];
vi.spyOn(spectator.service, 'postForm').mockImplementation(async () => {
const value = spectator.service.model();
child_parent_ids.push(value.parent_id);
return new Booking({
id: `booking-child-${child_parent_ids.length}`,
parent_id: value.parent_id,
user_email: value.user_email,
asset_id: value.asset_id,
});
const value = spectator.service.model();
child_parent_ids.push(value.parent_id);
return new Booking({
id: `booking-child-${child_parent_ids.length}`,
parent_id: value.parent_id,
user_email: value.user_email,
asset_id: value.asset_id,
});
});
spectator.service.newForm(
'desk',
Expand Down Expand Up @@ -1998,17 +1998,17 @@ describe('BookingFormService', () => {
).mockResolvedValue(true);
const saved_users: string[] = [];
vi.spyOn(spectator.service, 'postForm').mockImplementation(async () => {
const value = spectator.service.model();
saved_users.push(value.user_email);
if (value.user_email === 'member.one@example.com') {
throw new Error('Save failed');
}
return new Booking({
id: `booking-child-${saved_users.length}`,
parent_id: value.parent_id,
user_email: value.user_email,
asset_id: value.asset_id,
});
const value = spectator.service.model();
saved_users.push(value.user_email);
if (value.user_email === 'member.one@example.com') {
throw new Error('Save failed');
}
return new Booking({
id: `booking-child-${saved_users.length}`,
parent_id: value.parent_id,
user_email: value.user_email,
asset_id: value.asset_id,
});
});
spectator.service.newForm(
'desk',
Expand Down Expand Up @@ -2114,13 +2114,13 @@ describe('BookingFormService', () => {
).mockResolvedValue(true);
const saved_names: string[] = [];
vi.spyOn(spectator.service, 'postForm').mockImplementation(async () => {
const value = spectator.service.model();
saved_names.push(value.asset_name);
return new Booking({
id: `booking-${saved_names.length}`,
user_email: value.user_email,
asset_id: value.asset_id,
});
const value = spectator.service.model();
saved_names.push(value.asset_name);
return new Booking({
id: `booking-${saved_names.length}`,
user_email: value.user_email,
asset_id: value.asset_id,
});
});
spectator.service.newForm(
'desk',
Expand Down Expand Up @@ -2199,16 +2199,16 @@ describe('BookingFormService', () => {
).mockResolvedValue([all_desks[2]]);
const saved_forms: { user_email: string; asset_id: string }[] = [];
vi.spyOn(spectator.service, 'postForm').mockImplementation(async () => {
const value = spectator.service.model();
saved_forms.push({
user_email: value.user_email,
asset_id: value.asset_id,
});
return new Booking({
id: `booking-${saved_forms.length}`,
user_email: value.user_email,
asset_id: value.asset_id,
});
const value = spectator.service.model();
saved_forms.push({
user_email: value.user_email,
asset_id: value.asset_id,
});
return new Booking({
id: `booking-${saved_forms.length}`,
user_email: value.user_email,
asset_id: value.asset_id,
});
});
spectator.service.newForm(
'desk',
Expand Down Expand Up @@ -2306,14 +2306,14 @@ describe('BookingFormService', () => {
).mockResolvedValue([all_desks[1]]);
const saved_forms: { id: string; parent_id: string }[] = [];
vi.spyOn(spectator.service, 'postForm').mockImplementation(async () => {
const value = spectator.service.model();
saved_forms.push({ id: value.id, parent_id: value.parent_id });
return new Booking({
id: value.id || `booking-child-${saved_forms.length}`,
parent_id: value.parent_id,
user_email: value.user_email,
asset_id: value.asset_id,
});
const value = spectator.service.model();
saved_forms.push({ id: value.id, parent_id: value.parent_id });
return new Booking({
id: value.id || `booking-child-${saved_forms.length}`,
parent_id: value.parent_id,
user_email: value.user_email,
asset_id: value.asset_id,
});
});
spectator.service.newForm(
'desk',
Expand Down Expand Up @@ -2452,14 +2452,14 @@ describe('BookingFormService', () => {
];
let booking_count = 0;
vi.spyOn(spectator.service, 'postForm').mockImplementation(async () => {
const value = spectator.service.model();
booking_count++;
return new Booking({
id: `booking-${booking_count}`,
user_email: value.user_email,
asset_id: value.asset_id,
extension_data: { group_members: group_members_payload },
});
const value = spectator.service.model();
booking_count++;
return new Booking({
id: `booking-${booking_count}`,
user_email: value.user_email,
asset_id: value.asset_id,
extension_data: { group_members: group_members_payload },
});
});
spectator.service.newForm(
'desk',
Expand Down Expand Up @@ -2883,4 +2883,114 @@ describe('BookingFormService', () => {
expect(savedBookings().length).toBe(1);
expect((savedBookings()[0] as Booking).asset_ids).toEqual(['desk-2']);
});

describe('initialisation while the user is still loading', () => {
/**
* Put the service into the state `newForm` sees on a slow load: no
* current user yet, so it defers and returns while the form is already
* rendered and interactive.
*
* Nothing is mocked. `currentUserIsLoaded()` is real, and it reports
* "loaded" whenever it detects a test runtime, so the runtime probe
* (`typeof vi`) is what has to be neutralised to reach the branch.
* That keeps the real promise plumbing in `currentUserLoaded()` under
* test rather than a stub of it.
*/
function deferCurrentUser() {
const runtime_vi = (globalThis as any).vi;
setCurrentUser(new StaffUser({}));
(globalThis as any).vi = undefined;
const restore = () => ((globalThis as any).vi = runtime_vi);
return {
restore,
release: async () => {
restore();
setCurrentUser(
new StaffUser({
id: 'current-user',
email: 'current.user@example.com',
name: 'Current User',
}),
);
// let the `.then` re-entry and its patches settle
await Promise.resolve();
await Promise.resolve();
await Promise.resolve();
},
};
}

/** Type into a field the way the Field directive does. */
function userEdits(field: string, value: any) {
const node = (spectator.service.form as any)[field]();
node.value.set(value);
node.markAsDirty();
}

it('keeps input the user entered before initialisation finished', async () => {
const deferred = deferCurrentUser();
try {
spectator.service.newForm('desk');

userEdits('title', 'Quiet corner desk');
userEdits('all_day', true);

await deferred.release();

expect(spectator.service.model().title).toBe(
'Quiet corner desk',
);
expect(spectator.service.model().all_day).toBe(true);
} finally {
deferred.restore();
}
});

it('does not resurrect that input on the next new form', async () => {
// The preserved edits are one-shot. If they leaked, opening a
// second form would silently inherit the previous booking's title.
const deferred = deferCurrentUser();
try {
spectator.service.newForm('desk');
userEdits('title', 'Quiet corner desk');
await deferred.release();
} finally {
deferred.restore();
}

spectator.service.newForm('desk');

expect(spectator.service.model().title).not.toBe(
'Quiet corner desk',
);
});

it('leaves untouched fields to the incoming booking', async () => {
// Only dirty fields are carried across. A field the user never
// touched must still take its value from the booking being opened.
const deferred = deferCurrentUser();
try {
spectator.service.newForm(
'desk',
new Booking({
id: 'bkn-1',
booking_type: 'desk',
title: 'From the booking',
asset_id: 'desk-1',
}),
);

userEdits('all_day', true);

await deferred.release();

expect(spectator.service.model().all_day).toBe(true);
expect(spectator.service.model().title).toBe(
'From the booking',
);
} finally {
deferred.restore();
}
});
});
});
Loading