Repository navigation
fix(bookings): leave approval to the backend for users who cannot approve - #548
Merged
Merged
Conversation
|
Deployment failed for project frontend-templates with the following error: Learn More: https://vercel.com/placeos?upgradeToPro=build-rate-limit |
camreeves
added a commit
that referenced
this pull request
Oct 7, 2026
…e check Thirteen `test.fixme` guards come off: ROOM-28 and CON-SURV-01 are fixed on develop, DESK-21 passes as written, VIS-28 and VIS-28b hold on the 2.2609.6 images, and HOME-09, LOCK-01/03/04, ROOM-22, ROOM-24, CON-B2 and ROOM-21 pass once their app fixes are on develop (#544 to #548). ROOM-21 now asserts what the app can promise: a standard user's booking under `app.bookings.no_approval` is stored and pending rather than refused, since staff-api only accepts `approved` from approvers. `room-booking` asserted the booking's own title is "Room Booking"; develop keeps the meeting name on a native booking, so it asserts that. The workplace locker seeder used the same asset names as the concierge seeder on the same building zone, so each found the other's lockers. Its assets are now `E2E Workplace Locker*`. ROOM-22 accepts an order linked by `parent_id` as well as by `extension_data.event_id`.
Collaborator
|
I checked this against staff-api's Edits can 403. export function bookingSaveData(booking: Booking): Partial<Booking> {
const { approved, rejected, ...data } = booking.toJSON();
return data.id ? data : { ...data, approved, rejected };
}Zone managers lose auto-approval. staff-api also accepts Settings text is out of date. Smaller things.
I have a patch for everything except zone managers. It adds a test that an edit keeps the approval state, and the test fails without the fix. Tests for bookings, events and common pass, and workplace and concierge build. Most of the diff in Patch (git apply)diff --git a/apps/concierge/src/app/ui/app-settings/workplace-settings-form-modal.component.ts b/apps/concierge/src/app/ui/app-settings/workplace-settings-form-modal.component.ts
index 879b4d098..d36996b05 100644
--- a/apps/concierge/src/app/ui/app-settings/workplace-settings-form-modal.component.ts
+++ b/apps/concierge/src/app/ui/app-settings/workplace-settings-form-modal.component.ts
@@ -1193,7 +1193,7 @@ import { errorText } from '../modal-actions';
[formField]="form.bookings.multiple_visitors"
></settings-toggle>
<settings-toggle
- label="Auto-approve bookings"
+ label="Auto-approve admin and support bookings"
[formField]="form.bookings.no_approval"
></settings-toggle>
<settings-toggle
diff --git a/apps/workplace/src/environments/settings.schema.json b/apps/workplace/src/environments/settings.schema.json
index d45caf5a7..a0320227d 100644
--- a/apps/workplace/src/environments/settings.schema.json
+++ b/apps/workplace/src/environments/settings.schema.json
@@ -205,7 +205,7 @@
},
"no_approval": {
"type": "boolean",
- "description": "Whether new bookings should skip the approval process and be created in the approved state"
+ "description": "Whether new bookings by admin and support users are created in the approved state. Bookings by other users are left for an approver or the auto-approval driver"
},
"all_day_default": {
"type": "boolean",
diff --git a/docs/settings/workplace.md b/docs/settings/workplace.md
index f32ebfaeb..7957e88e4 100644
--- a/docs/settings/workplace.md
+++ b/docs/settings/workplace.md
@@ -177,7 +177,7 @@ These apply to all resource booking flows (desks, parking, lockers). Most can be
| Setting | Type | Default | Description |
|---------|------|---------|-------------|
-| `bookings.no_approval` | boolean | `false` | Create new bookings already approved. Only an admin or support user's bookings are sent approved; staff-api refuses `approved` from other users, whose bookings are left for an approver or the auto-approval driver. |
+| `bookings.no_approval` | boolean | `false` | Create new bookings by admin and support users already approved. staff-api refuses `approved` from other users, so their bookings are left for an approver or the auto-approval driver. Edits keep the stored approval state. |
| `bookings.all_day_default` | boolean | `false` | Turn the "all day" option on by default for new bookings. |
| `bookings.allow_all_day` | boolean | – | Make the "all day" option available in resource booking flows. |
| `bookings.allowed_daily_visitor_count` | number | `100` | Maximum number of visitor invites allowed for a single day. |
diff --git a/libs/bookings/src/lib/booking-form.service.ts b/libs/bookings/src/lib/booking-form.service.ts
index 2f219679e..1f99d3a22 100644
--- a/libs/bookings/src/lib/booking-form.service.ts
+++ b/libs/bookings/src/lib/booking-form.service.ts
@@ -25,7 +25,6 @@ import {
BookingRuleset,
BookingType,
currentUser,
- currentUserCanApprove,
currentUserIsLoaded,
currentUserLoaded,
Desk,
@@ -65,10 +64,12 @@ import {
bookingAttachments,
bookingFormValue,
type BookingFormValue,
+ bookingSaveData,
bookingHostUser,
findNearbyFeature,
generateBookingForm,
loadLockerResources,
+ newBookingApproved,
} from './booking.utilities';
import {
bookedResourceList,
@@ -1468,24 +1469,29 @@ export class BookingFormService extends AsyncHandler {
)
: [];
const result = await saveBooking(
- new Booking({
- type: this._options().type,
- ...formBookingData(value),
- description:
- value.booking_type === 'visitor'
- ? value.description || value.title || value.asset_name
- : value.asset_name || value.description,
- user_id: value.user?.id ?? value.user_id,
- user_name: value.user?.name || value.user_name,
- user_email: value.user?.email || value.user_email,
- extension_data: buildBookingExtensionData(value, group_members),
- approved:
- this._settings.get('app.bookings.no_approval') === true &&
- currentUserCanApprove(),
- zones: unique([...zones, ...(value.zones || [])]).filter(
- (_) => _,
- ),
- }).toJSON(),
+ bookingSaveData(
+ new Booking({
+ type: this._options().type,
+ ...formBookingData(value),
+ description:
+ value.booking_type === 'visitor'
+ ? value.description ||
+ value.title ||
+ value.asset_name
+ : value.asset_name || value.description,
+ user_id: value.user?.id ?? value.user_id,
+ user_name: value.user?.name || value.user_name,
+ user_email: value.user?.email || value.user_email,
+ extension_data: buildBookingExtensionData(
+ value,
+ group_members,
+ ),
+ approved: newBookingApproved(this._settings),
+ zones: unique([...zones, ...(value.zones || [])]).filter(
+ (_) => _,
+ ),
+ }),
+ ),
q,
).catch(async (e) => {
this._loading.set('');
@@ -2129,32 +2135,34 @@ export class BookingFormService extends AsyncHandler {
].filter((_) => _),
);
return saveBooking(
- new Booking({
- ...formBookingData(form_data),
- id,
- parent_id: '',
- asset_id: group_name,
- asset_name: 'Group Booking',
- // The opened visitor booking can carry the old attendee list.
- ...(resource_type === 'visitor' ? { attendees: members } : {}),
- booking_type: 'group',
- type: 'group',
- description: form.title || 'Group Booking',
- title: form.title || 'Group Booking',
- user_name: form.user?.name || form.user_name,
- user_email: form.user?.email || form.user_email,
- user_id: form.user?.id || form.user_id,
- approved:
- this._settings.get('app.bookings.no_approval') === true &&
- currentUserCanApprove(),
- zones,
- extension_data: {
- ...formExtensionData(form.extension_data),
- group: group_name,
- group_members,
- group_resource_type: resource_type,
- },
- }).toJSON(),
+ bookingSaveData(
+ new Booking({
+ ...formBookingData(form_data),
+ id,
+ parent_id: '',
+ asset_id: group_name,
+ asset_name: 'Group Booking',
+ // The opened visitor booking can carry the old attendee list.
+ ...(resource_type === 'visitor'
+ ? { attendees: members }
+ : {}),
+ booking_type: 'group',
+ type: 'group',
+ description: form.title || 'Group Booking',
+ title: form.title || 'Group Booking',
+ user_name: form.user?.name || form.user_name,
+ user_email: form.user?.email || form.user_email,
+ user_id: form.user?.id || form.user_id,
+ approved: newBookingApproved(this._settings),
+ zones,
+ extension_data: {
+ ...formExtensionData(form.extension_data),
+ group: group_name,
+ group_members,
+ group_resource_type: resource_type,
+ },
+ }),
+ ),
).catch((error) => {
this._loading.set('');
throw error;
diff --git a/libs/bookings/src/lib/booking.utilities.ts b/libs/bookings/src/lib/booking.utilities.ts
index 5c24caa8b..c200ebfcb 100644
--- a/libs/bookings/src/lib/booking.utilities.ts
+++ b/libs/bookings/src/lib/booking.utilities.ts
@@ -21,12 +21,14 @@ import {
CalendarEvent,
current_user,
currentUser,
+ currentUserIsAdminOrSupport,
fromEventRecurrence,
guardModelUndefinedWrites,
onFieldChange,
OrganisationService,
Point,
settingSignal,
+ SettingsService,
setupFormTimeSync,
toBookingRecurrence,
unique,
@@ -508,6 +510,29 @@ export async function findNearbyFeature(
return closest;
}
+/**
+ * Whether to create a new booking already approved. `app.bookings.no_approval`
+ * skips approval, but staff-api accepts `approved` only from admin, support and
+ * zone manager users. Zone manager access is not known client side, so other
+ * users' bookings are left for an approver or the auto-approval driver.
+ */
+export function newBookingApproved(settings: SettingsService) {
+ return (
+ settings.get('app.bookings.no_approval') === true &&
+ currentUserIsAdminOrSupport()
+ );
+}
+
+/**
+ * Request data to save a booking from a form. staff-api treats a changed
+ * approval state on update as an approval, which only approvers may make, so
+ * an update leaves it out and keeps the stored state.
+ */
+export function bookingSaveData(booking: Booking): Partial<Booking> {
+ const { approved, rejected, ...data } = booking.toJSON();
+ return data.id ? data : { ...data, approved, rejected };
+}
+
/** Convert an event form value into a native room booking that holds every selected room. */
export function newBookingFromCalendarEvent(event: CalendarEvent) {
const date = event.date || event.event_start * 1000;
diff --git a/libs/bookings/src/test/booking-form.service.spec.ts b/libs/bookings/src/test/booking-form.service.spec.ts
index 2802dab76..27e885218 100644
--- a/libs/bookings/src/test/booking-form.service.spec.ts
+++ b/libs/bookings/src/test/booking-form.service.spec.ts
@@ -900,6 +900,29 @@ describe('BookingFormService', () => {
expect((savedBookings()[0] as Booking).approved).toBe(true);
});
+ it('should keep the stored approval state when editing a booking', async () => {
+ (spectator.inject(PaymentsService) as any).enabled = false;
+ spectator.service.newForm(
+ 'desk',
+ new Booking({
+ id: 'bkn-1',
+ booking_type: 'desk',
+ date: Date.now() + 60 * 60 * 1000,
+ duration: 60,
+ asset_id: 'desk-1',
+ asset_name: 'Desk 1',
+ approved: true,
+ }),
+ );
+
+ spectator.service.model.update((m) => ({ ...m, title: 'Updated' }));
+ await spectator.service.postForm(true);
+
+ expect(savedBookings().length).toBe(1);
+ expect(savedBookings()[0]).not.toHaveProperty('approved');
+ expect(savedBookings()[0]).not.toHaveProperty('rejected');
+ });
+
it('should keep the host when editing a delegated visitor booking', async () => {
(spectator.inject(PaymentsService) as any).enabled = false;
spectator.service.newForm(
diff --git a/libs/common/src/lib/user-state.ts b/libs/common/src/lib/user-state.ts
index 2c9a14d6c..4a7c58fef 100644
--- a/libs/common/src/lib/user-state.ts
+++ b/libs/common/src/lib/user-state.ts
@@ -324,13 +324,9 @@ export function currentUser() {
return _current_user.getValue() || EMPTY_USER;
}
-/**
- * Whether the current user may set a booking's approval state. staff-api
- * accepts `approved` on create only from admin, support and zone manager
- * users; zone management is not known client side
- */
-export function currentUserCanApprove() {
- const groups = currentUser()?.groups || [];
+/** Whether the current user is a PlaceOS admin or support user */
+export function currentUserIsAdminOrSupport() {
+ const groups = currentUser().groups || [];
return (
groups.includes('placeos_admin') || groups.includes('placeos_support')
);
diff --git a/libs/events/src/lib/event-form.service.ts b/libs/events/src/lib/event-form.service.ts
index fa69df177..3ce2f08f4 100644
--- a/libs/events/src/lib/event-form.service.ts
+++ b/libs/events/src/lib/event-form.service.ts
@@ -17,7 +17,6 @@ import {
BookingRuleset,
CalendarEvent,
currentUser,
- currentUserCanApprove,
currentUserIsLoaded,
currentUserLoaded,
DEFAULT_SETTINGS,
@@ -47,7 +46,11 @@ import { AssetRequest, OrganisationService } from '@placeos/common';
import { UserPipe } from '@placeos/users';
import { AssetStateService } from 'libs/assets/src/lib/asset-state.service';
import { validateAssetRequestsForResource } from 'libs/assets/src/lib/assets.fn';
-import { newBookingFromCalendarEvent } from 'libs/bookings/src/lib/booking.utilities';
+import {
+ bookingSaveData,
+ newBookingApproved,
+ newBookingFromCalendarEvent,
+} from 'libs/bookings/src/lib/booking.utilities';
import {
createBookingsForEvent,
queryResourceAvailability,
@@ -1485,16 +1488,16 @@ export class EventFormService extends AsyncHandler {
}
return this.book_internal
? saveBooking(
- newBookingFromCalendarEvent({
- ...event.toJSON(),
- // Native recurrence needs weekday indices and millisecond dates.
- recurrence: event.recurrence,
- status:
- this._settings.get('app.bookings.no_approval') ===
- true && currentUserCanApprove()
+ bookingSaveData(
+ newBookingFromCalendarEvent({
+ ...event.toJSON(),
+ // Native recurrence needs weekday indices and millisecond dates.
+ recurrence: event.recurrence,
+ status: newBookingApproved(this._settings)
? 'approved'
: 'tentative',
- } as any),
+ } as any),
+ ),
).then((_) => newCalendarEventFromBooking(_))
: saveEvent(event, query);
}
diff --git a/libs/events/src/tests/event-form.service.spec.ts b/libs/events/src/tests/event-form.service.spec.ts
index 01e119d24..b2d2f2b74 100644
--- a/libs/events/src/tests/event-form.service.spec.ts
+++ b/libs/events/src/tests/event-form.service.spec.ts
@@ -13,6 +13,7 @@ import {
setCurrentUser,
SettingsService,
Space,
+ StaffUser,
User,
} from '@placeos/common';
import { Subject } from 'rxjs';
@@ -165,11 +166,13 @@ describe('EventFormService', () => {
});
it('should send approved for a support user', async () => {
- setCurrentUser({
- email: 'support@test.com',
- name: 'Support',
- groups: ['placeos_support'],
- } as any);
+ setCurrentUser(
+ new StaffUser({
+ email: 'support@test.com',
+ name: 'Support',
+ groups: ['placeos_support'],
+ }),
+ );
await performBooking(service);
Review by Claude Opus 5.5 in Claude Code. |
…rove With `app.bookings.no_approval` on, every booking form sent `approved: true`. staff-api only accepts that from admin, support and zone manager users (PPT-2767), so a standard user's booking was refused with 403 and the setting stopped them booking at all. Send `approved` only for admin and support users. Everyone else's booking is created pending, for an approver or the auto-approval driver.
MrYuion
force-pushed
the
fix/no-approval-standard-users
branch
from
October 8, 2026 03:32
8b6a73d to
f5763be
Compare
|
Deployment failed for project frontend-templates with the following error: Learn More: https://vercel.com/placeos?upgradeToPro=build-rate-limit |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was broken
With
app.bookings.no_approvalon, every booking form sentapproved: true: desks, parking, lockers and group bookings throughBookingFormService.postForm, rooms throughEventFormServiceasstatus: 'approved'. Since staff-api3891234(PPT-2767, in placeos-2.2609.6) only an admin, support or zone manager user may create a booking that is already approved; anyone else gets:So on a deployment with the setting on, a standard user could not book anything. Measured on the e2e stack with the #497 spec ROOM-21: the room booking POST answered 403.
Change
Send
approvedonly when the current user is an admin or support user (currentUserCanApprove, which reads theplaceos_admin/placeos_supportgroups the user model already derives). Everyone else's booking is created pending, for an approver or the auto-approval driver, which is what PPT-2767 intends. Zone managers are not recognised client side, so their bookings are also left pending; they are approved by the driver or themselves afterwards.The setting's documentation says so. Unit tests cover the desk and room paths for both kinds of user. ROOM-21 passes against this change.