View-only seats, pure shrinks, book-for override refactor - #78
Merged
Conversation
Book-as was really a book-for mechanism: a zone admin books on behalf of another user. Reframing it that way lets bookable seats in an admin's zone work correctly during book-for even when the target is a viewer, and lets the frontend show real occupancy/assignment icons in view-only zones instead of a blanket "unavailable" for every seat. - getSeats/apply/autoBook: bookable under book-for now requires the actor to administer the zone and the target to merely be a member (any role) of it, overriding seat-level assignment restrictions there. Release confinement to the actor's own zones is unchanged. - seat.js: removed the early !bookable short-circuit and the VIEW_ONLY_TAKEN state; every seat now computes its normal occupancy/assignment state, with bookable only demoting action states (book/rebook) to informational ones. - Renamed book-as -> book-for across code, UI, CSS, i18n, and docs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A user's own booking is always releasable server-side (apply()'s remove bypasses the per-seat zone-admin check for own bookings via the Book.login != g.login filter, but the bookings table denied it: viewers got rw=false (role 30 > USER), and bookings in zones the user had lost access to vanished entirely (the INNER JOIN on user_to_zone_roles dropped them). Switch the list query to a LEFT JOIN with the login in the ON clause and admit own bookings in no-access zones via Book.login == g.login (other users' bookings there stay hidden — the disjunct only admits own rows); make rw true for any own booking unconditionally. Foreign bookings stay gated by the actor's zone-admin standing. Updates the bookings-page e2e (flip the wrong-behaviour PUBLIC_VIEW own-booking test to assert the delete icon; clarify the inaccessible-zone test as another user's booking; add an own-booking-in-inaccessible-zone end-to-end release test) and FEATURES.md §11.1 + the §26 footnote. Co-Authored-By: GLM 5.2 <glm-5.2@z.ai> EOF )
The view-only-zone display fix (taken/assigned/yours icons survive,
book demoted to unavailable) had no e2e regression guard — only the
backend-free OfficeMap component suite asserted sprite hrefs; the
booking/permission specs only assert action-button visibility. Add the
7 sprite tests from PLAN_VIEW_ONLY_SEATS.md Phase 2 test plan:
1. viewer-zone seat booked by another -> cell-taken; click opens no modal
2. assigned to another, free -> cell-assigned
3. everyone-only, free -> cell-unavailable
4. free unassigned -> cell-unavailable; click opens no modal
5. own exact booking in a view-only zone -> cell-yours; release via modal
6. mixed ENABLED + PUBLIC_VIEW in one zone_group: viewer-zone taken stays
taken, enabled-zone free shows rebook (risk #1 conflict-map guard)
7. assigned-to-me in a bookable ENABLED zone -> cell-availableAssigned
(risk #5 guard)
Asserts the active <use> href on #sprite-<sid> (OfficeMap keeps one <use>
per distinct cell name and toggles display, so the helper reads the
visible one and retries via expect.poll).
Co-Authored-By: GLM 5.2 <glm-5.2@z.ai>
hasUnmanageableConflict() (frontend) gates the doomed "update" when a book-for rebook would release the target's booking in a zone the actor doesn't administer. It reads the zoneAdmin map from getSeats, but that map was built from effective_roles — which only covers zones ON the current plan. A conflict booking can sit in a same-group zone on ANOTHER plan, so an administered cross-plan conflict zone was marked not-administered and the update was wrongly blocked. Query the actor's roles for every zone in the response (plan + conflict) so the cross-plan case is judged correctly; site admins administer every zone. zoneAdmin is only read by hasUnmanageableConflict (conflict seats) and the per-seat seat-edit gate (plan zones, already correct), so the change is scoped to the cross-plan release-confinement path. Update the stale bookfor-zone-group e2e that expected the old doomed "update" for an inaccessible same-group conflict: the seat is still a rebook (the conflict is recognised from the server-supplied login — book is not offered) but the update is now correctly blocked by release confinement, with the explanatory "zone you don't administer" message. The server-side 102 pin is the companion test in the same file. Co-Authored-By: GLM 5.2 <glm-5.2@z.ai>
apply() already lets a zone admin booking FOR a target override a seat-level assignment (it skips 106/110 under is_book_for — pinned by zone-permissions-bookfor B5/B6 via API), but the plan UI showed the seat as grey ASSIGNED with no book action, so the only path was the seat-edit workaround. Under book-for (factory.login != the real login) and where bookable (which under book-for already means the actor administers the zone), let a seat assigned to a third person — or beyond the target's days-in-advance window — fall through to CAN_BOOK instead of ASSIGNED, flagged so a new green "assignedOverride" sprite distinguishes "assigned, but you can book here via override" from both the grey ASSIGNED (not bookable here) and the blue availableAssigned (assigned to the target, within window). Self-booking never overrides (106/110 apply), so the new icon is book-for-only. The book action already attaches book.login under book-for, so apply() gets is_book_for and overrides — only the state/sprite was hiding it. Adds a new cell-assignedOverride to seat_icons.svg (the assigned-to head+tie glyph in the available colour family — reuses existing --warp-seat-available-* vars, no new theme colour), the legend row + help-map entry + the 5 i18n legend strings, a FEATURES.md §9 note, and a sprite e2e (self-view = grey assigned; book-for = green override; book succeeds for the target). Co-Authored-By: GLM 5.2 <glm-5.2@z.ai>
A pure viewer (isZoneViewer: view-only access to every zone on the plan) could see their own booking on the map (blue "yours" head) but couldn't release it there — the action modal was removed from the DOM by data-requires="notZoneViewer" and initActionMenu bailed out on isZoneViewer, so the only path was the bookings table. That conflicted with the "always can unbook your own booking" invariant the bookings table now upholds. Keep the action modal in the DOM for viewers (drop the data-requires="notZoneViewer") and let initActionMenu register the click handler. The existing state machine already restricts what a viewer sees: CAN_BOOK is demoted to VIEW_ONLY (early return, no modal), TAKEN/ASSIGNED push no action and isMyZoneAdmin() is false so no seat-edit (actions empty → return before open), and only CAN_DELETE / CAN_DELETE_EXACT (own booking) push 'delete' → the Release modal. So non-actionable seats open no panel, and the auto-book FAB + book-for input stay hidden for viewers via their own gates. Update the stale zone-permissions assertion that expected the action modal absent for viewers (it now exists; the auto-book FAB still doesn't). Add a sprite e2e: pure viewer with an own booking → yours + Release works end-to-end, while a free seat and a taken-by-other seat open no modal. FEATURES.md §27 notes the viewer release path. Co-Authored-By: GLM 5.2 <glm-5.2@z.ai>
In a view-only zone (!bookable), an own booking that overlaps the selected dates but isn't an exact time match used to fall from CAN_CHANGE to CAN_DELETE via the !bookable demotion, and CAN_DELETE renders the grey "taken" icon — so the moment the slider didn't match the booking exactly, the user's own booking turned grey (looking like someone else's / not actionable), even though it was still theirs and releasable. Demote the action (CAN_CHANGE -> CAN_DELETE: only Release is offered, not Update — apply() would 403 a change in a view-only zone) but flag it so spriteFor renders the blue "yours" head for that case. The exact-match case (CAN_DELETE_EXACT) is unchanged. Normal (bookable) zones are untouched — CAN_CHANGE still shows the blue "yoursChange" (with change arrows). isExactMatch (auto-book FAB "already exactly booked" detection) only checks state === CAN_DELETE_EXACT, so the new flag doesn't affect it. Update the legend copy for the "yours" icon (all 5 locales) to note it also shows for a non-exact own booking in a view-only zone (release only). Add a sprite e2e: pure viewer, own booking seeded at 10:00-16:00 with the default 09:00-17:00 slider -> cell-yours (not grey taken, not yoursChange), and release still works. Co-Authored-By: GLM 5.2 <glm-5.2@z.ai>
A zone admin could release someone else's booking from the bookings table (apply()'s remove requires per-seat zone-admin for foreign bookings, which an admin of the seat's zone satisfies) but not from the plan map: clicking a taken (foreign-booked) seat only offered seat-edit, never Release. The plan-view click handler had no action for the TAKEN state. Add a TAKEN case gated on isMyZoneAdmin(): it pushes 'delete' (Release) and flags a "foreign release". The delete action then targets the foreign booking's bid(s) on that seat (new WarpSeat.getForeignBookings, which walks the bookings overlapping the current selection and skips the acting user's own), not the acting user's own same-group conflicts (getMyConflictingBookings) that an own-booking delete/update removes. The "to be released" panel row names the foreign owner. Non-admins get no action on a taken seat (TAKEN stays informational) — isMyZoneAdmin() is false for them. apply() is unchanged: a foreign remove already passes when the actor administers the seat's zone (seatsReqZoneAdmin). The bookings-table path is unaffected (it never used the plan-view click handler). Update FEATURES.md §27 (zone admin: + Release on a taken seat) and add a sprite e2e: admin clicks a foreign-booked seat -> Release modal (delete offered, book not, the to-be-released row names the owner) -> booking gone; a non-admin clicking a foreign-booked seat gets no modal. Co-Authored-By: GLM 5.2 <glm-5.2@z.ai>
Book-for onto a seat assigned to a third party renders plain green 'available'; a seat assigned to the target beyond its days-in-advance window renders blue 'availableAssigned' (falls out of assignedToMe once the flag is gone). The state-machine fall-through (book-for override -> CAN_BOOK) stays; only the flag/sprite plumbing goes. - seat.js: drop the assignedOverride flag init, the two assignment sites, the spriteFor parameter + CAN_BOOK branch, and the call-site argument. Keep the bookForOverride const and its fall-through logic. - seat_icons.svg: remove the cell-assignedOverride group + palette row, restore the 552 viewBox width. - plan.js: drop the bookAssignedOverride help-map entry. - plan.html: remove the assignedOverride legend row; trim the viewOnly legend row's edge-case clause (edge-case-free legend, Phase 3 guiding principle). - i18n (en/de/es/fr/pl): drop the assignedOverride key; shorten the viewOnly key+value in all five locales. - FEATURES.md §9: update the override note to plain green/availableAssigned wording. - e2e zone-view-sprites: test 8 now expects cell-available (third-party assignment); add test 8b pinning cell-availableAssigned for a seat assigned to the target beyond its window. Co-Authored-By: GLM 5.2 <glm-5.2@z.ai>
…rink bypass (Phase 3B+3C) 3B — reverts the display half of the ownReleaseOnly approach (commit dd38364): in view-only / DISABLED zones a non-exact own booking stays CAN_CHANGE -> 'yoursChange', same as bookable zones. The !bookable demotion map shrinks to a single line (CAN_BOOK -> VIEW_ONLY). - seat.js: delete the CAN_CHANGE -> CAN_DELETE demotion branch and the ownReleaseOnly flag init; revert spriteFor's CAN_DELETE special case to plain 'taken' and drop the parameter + call-site argument. Add WarpSeat.isSelectionShrinkOfMine(): true iff every selected range is fully contained within one of the acting user's OWN bookings on this seat. Uses window.warpGlobals.login (the real actor), not factory.login, so a book-for target's booking in a non-administered (!bookable) zone does NOT count -- changing/releasing it would 403 (release confinement / seatsReqZoneAdmin), so Update must not be offered there. This is a correctness refinement of the plan's literal 'book.login == factory.login' wording, which would have offered a doomed Update in that book-for edge case. - plan.js click handler, case CAN_CHANGE: push 'delete' always; push 'update' iff this.bookable OR isSelectionShrinkOfMine(). - plan.html + i18n (en/de/es/fr/pl): revert dd38364's added view-only note on the 'yours' legend row (edge-case-free legend). - e2e zone-view-sprites: flip test 10 to cell-yoursChange (and assert Update is not offered when the slider extends beyond the booking); add test 12 -- pure viewer shrinks an own booking through the modal (Update offered + succeeds, booking narrows; widening the slider back offers Release only). 3C -- the pure-shrink bypass in apply(). A self update (no book.login) whose every booked range is fully covered by one of the actor's own bookings being removed on that seat only narrows an existing own booking; a shrink never increases exposure, so it skips every booking check (role 104, DISABLED zone 104, seat-disabled 105, assignment 106/110, horizon 103). Book-for is never a shrink (is_book_for excluded). Remove-bid ownership is verified server-side by the login filter. - warp/xhr/plan.py: compute is_pure_shrink once at the top of the book block; guard each check with 'and not is_pure_shrink'. - e2e zone-shrink-bypass.spec.ts: new API-level spec -- viewer-zone shrink -> 200 (replaced) / extend -> 403 (104) / no-remove -> 403 (104); DISABLED-zone shrink -> 200; assigned-to-another shrink -> 200 / no-remove -> 403 (106). Co-Authored-By: GLM 5.2 <glm-5.2@z.ai>
Seat-disabled (enabled=false, code 105) is a seat-level restriction like assignments — a zone admin controls it and may override it under book-for (they could re-enable the seat anyway). The zone-type DISABLED block (code 104, zone-level, "enable the zone first") stays, per the rule that book-for never targets a DISABLED zone. - warp/xhr/plan.py: skip the seat-enabled check (105) when is_book_for (the zone-admin gate already ran via seatsReqZoneAdmin — an explicit book.login puts the sid in that set). - js/views/modules/seat.js _updateState(): hoist the bookForOverride const above the !this.enabled early return so a disabled seat under book-for skips it and falls through to the normal pipeline -> CAN_BOOK -> green available / availableAssigned. Self-booking never overrides (the early return still fires). - e2e zone-permissions-bookfor: D1 book-for onto a disabled seat -> 200, self-book onto it -> 403 / 105; D2 auto-book-for on a plan whose only free seat is disabled books nothing there (auto-book keeps excluding disabled seats). - e2e zone-view-sprites: 8c pins the display -- self-view of a disabled seat -> grey unavailable; under book-for -> green available, book works. Also drops now-stale plan-phase citations from comments in plan.py and the zone-view-sprites / zone-shrink-bypass / zone-permissions-bookfor specs (the implementation plan is ephemeral; permanent files stand on their own). Co-Authored-By: GLM 5.2 <glm-5.2@z.ai>
…hase 3E/3F) 3E -- book-for with self selected is normal mode (already the shipped behaviour, zero code change). Documented in PERMISSIONS.md §8: a zone admin who selects their own login in the book-for picker gets regular-user treatment for that selection -- no book.login on the wire, no green override, assignments/windows apply. The picker keeps the admin's own login as the exit from book-for mode. 3F -- state the four design invariants verbatim in PERMISSIONS.md as the new §11 "Seat-icon & action invariants": icons state facts / colours carry meaning; the click reveals permitted actions (the icon never enumerates them); any strict shrink of the actor's own bookings is always allowed everywhere; a zone admin is unrestricted for seat-level matters inside their zone and powerless across its boundary. Any behaviour not derivable from these is a bug -- or requires amending them first. FEATURES.md updates for the user-facing Phase 3 behaviour changes: - §9 "Book For": book-for also overrides a seat-level disable (the zone-type DISABLED block still rejects it outright). - §27 viewer release note: a viewer can also shorten (Update) a non-exact own booking via a pure shrink, even in a view-only / disabled zone. e2e pin (3E): zone-view-sprites 8d -- admin selects themselves in book-for -> a seat assigned to a third person stays grey `assigned` (not green) and the book action is not offered (self-book there would be 106), confirming no book.login is sent. Co-Authored-By: GLM 5.2 <glm-5.2@z.ai>
The own+foreign overlap seat state (CAN_DELETE) renders the same red "taken" icon as a pure foreign booking, not a blue icon — the in-app legend has never had a separate blue conflict glyph (helpSpriteMap maps userConflict to 'taken', and no plan.html legend row references it). FEATURES.md §7.2 and §27 still carried a stale "Blue (conflict)" label from an older design. - §7.2 icon table: drop the "Blue (conflict)" row; fold its meaning into "Red (taken)" (booked by someone else, or your own booking overlaps theirs — same icon either way; action: Release your own overlap, or a zone-admin release of the foreign booking). Also fix the "Gray (view-only)" row to match the in-app legend wording (no "free vs. taken" — occupied/assigned seats in view-only zones show their real icon). - §27 interaction summary: rename the "Blue (Conflict)" column to "Red (Taken, own overlap)" and "Red (Taken)" to "Red (Taken, foreign)" — same icon, different action (Remove own overlap vs zone-admin Release of the foreign booking). Actions unchanged. Co-Authored-By: GLM 5.2 <glm-5.2@z.ai>
…sabled-icon cue
A rigid post-implementation review of Phase 3 found two user-facing
inconsistencies and one missing visual cue. All resolved here; the
agreements are recorded in PLAN_VIEW_ONLY_SEATS.md ("Phase 3 review
agreements") for the reviewer, and the permanent docs reflect them.
1. yoursChange legend contradicted invariant #2 (PERMISSIONS.md §11: "the
icon never enumerates actions"). The userRebook legend said "You can
update; …" — but 3B made yoursChange appear in view-only zones for a
non-exact own booking where the selection extends beyond the booking, and
there the modal offers Release only. Reworded plan.html + all 5 i18n to
describe the click, not a promised action: "… click to release it or
update it to the selected time."
2. Doomed Release under book-for in a non-administered zone. The target's
"own" booking (CAN_CHANGE/CAN_DELETE/CAN_DELETE_EXACT) there offered
Release, but releasing the target's booking is a foreign release apply()
rejects with 403/102 (seatsReqZoneAdmin). hasUnmanageableConflict guarded
the doomed *update* (CAN_REBOOK) but not the doomed *release*. plan.js
click handler now computes bookForForeignRelease = factory.login !==
window.warpGlobals.login && !this.bookable and skips the Release push for
those three states (no modal opens). e2e zone-view-sprites 8e pins it.
3. Disabled seat under book-for keeps the disabled icon, but still acts.
The original 3D rendered it plain green `available` (no cue the seat is
off). Revised: under book-for (administered zone) the disabled seat runs
the normal pipeline (so its state drives the click actions: Book, or
Update via CAN_REBOOK when the target has a conflicting booking in the
zone group — book-alone would fail the DB book_overlap trigger — or
Release when the target already holds it), but _updateView forces the
sprite to 'unavailable' (grey X) as the visual cue. Reuses the existing
CAN_REBOOK conflict machinery; the earlier DISABLED click case is removed
(no longer needed). e2e 8c (book) and 8f (update-on-conflict) pin it.
4. Documented the intentional self-vs-book-for asymmetry for disabled seats:
an admin cannot book *themselves* onto a seat they disabled (re-enable
first; self-booking is the regular flow), but book-for onto it for
another member is offered. FEATURES.md §9 and PERMISSIONS.md §8 ("Acting
for yourself is normal mode"). Backend apply() 105 stays for self (the
frontend simply doesn't expose it).
Co-Authored-By: GLM 5.2 <glm-5.2@z.ai>
- e2e: admin/zones.spec.ts viewer-role test now expects #action_modal present in the DOM (the click handler gates viewer actions), matching the equivalent flip in zone-permissions.spec.ts that 58d7981 missed. Suite was 20/21 since. - PERMISSIONS.md §11 invariant #1: document the exception that the disabled-seat cue under book-for stays grey while actionable. - GLOSSARY.md: book-for's seat-level overrides now list disabled seats alongside assignments. - e2e: replace literal `10 /* DISABLED */` with the exported ZONE_TYPE_DISABLED in zone-shrink-bypass.spec.ts, and the same literal in zone-permissions-bookfor.spec.ts (consistency). - .gitignore: ignore a stray root-level test-results/ (playwright once ran from the repo root; e2e/.gitignore only covers its own). - seat.js: reset this.bookForOverride=false before the NOT_AVAILABLE early return so a stale book-for cue can't survive a no-dates re-render. Co-Authored-By: GLM 5.2 <glm-5.2@z.ai>
…E + 5 cleanups
The one real bug from the view-only-seats code review: the "don't offer a
doomed action" guard lived only on CAN_REBOOK. With a multi-day book-for
selection where the target also holds a same-zone-group booking in a zone the
admin doesn't administer, the modal offered an Update/Release that apply()
guaranteed to 403 (code 102) and roll the whole request back — a UX bug, not a
safety one (the server-side invariant held). The fix mirrors the guard already
written two cases down.
- plan.js: extend seatFactory.hasUnmanageableConflict() from CAN_REBOOK to
CAN_CHANGE/CAN_DELETE/CAN_DELETE_EXACT so book-for update/release that would
403 on a cross-zone same-group conflict is explained ("zone you don't
administer") instead of offered. Update stays gated on CAN_CHANGE; the guard
returns false outside book-for.
- seat.js: replace the stored bookForOverride flag with an on-demand
_isBookForOverride() so it can't go stale across _updateState early returns.
- plan.py getSeats: reuse effective_roles for admin_zids (the materialized
view has a unique (login,zid) row with MIN(zone_role), so the dict lookup is
equivalent to the old role-gated query) and query only the cross-plan
same-group zones absent from it — no query in the common case.
- plan.py apply(): collapse the book-for role check (nested if/elif) and the
assignment check (assignedQ two-step) to one line each.
- e2e: hoist activateBookFor/clearBookFor to helpers/booking.ts; drop two
local copies and three inline duplications; add a CAN_CHANGE blocked-action
test mirroring the existing CAN_REBOOK one.
Co-Authored-By: Claude Fable 5 <claude@anthropic.com>
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
1 participant
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.
Summary
Implementation of the view-only-seats refactor plan: viewers keep their own booking blue in a view-only zone, the pure-shrink bypass lets owners release/shrink their own booking from the plan map or bookings table in any zone, disabled seats gains a book-for override for zone admins, and the seat icon/action invariants are tightened across view-only / disabled / assigned seats. Also renames
book-as→book-forand fixes the book-for release-confinement check for cross-plan conflict zones.Roughly the structure of the work (oldest → newest commit on this branch):
book-as→book-forand fix seat display in view-only zonesassignedOverrideseat icon!bookablezone staysyoursChange+ pure-shrink bypass; book-for onto a disabled seat (zone-admin override)CAN_REBOOKtoCAN_CHANGE/CAN_DELETE/CAN_DELETE_EXACT(the one real bug — book-for update/release that would 403 on a cross-zone same-group conflict is now explained instead of offered), plus five cleanups: replace the storedbookForOverrideflag with an on-demand_isBookForOverride(), reuseeffective_rolesforadmin_zidsingetSeats(querying only cross-plan same-group zones — no query in the common case), and collapse twoapply()checks.Test plan
book-for CAN_CHANGE: update/release blocked when a same-group conflict lies in a non-administered zonecd js && npm run buildclean;python3 res/check_i18n.pyconsistent (280 keys × 5 locales)--warp-*CSS vars introducedThe server-side invariant (no partial apply on a doomed book-for update/release) held throughout; this was a UX fix plus cleanups.
Co-Authored-By: GLM 5.2 glm-5.2@z.ai
Co-Authored-By: Claude Fable 5 claude@anthropic.com