Skip to content

feat: let a weekly occurrence override its purpose and visibility (#55) - #60

Merged
pataniaeli merged 2 commits into
devfrom
feat/issue-55-occurrence-overrides
Aug 28, 2026
Merged

feat: let a weekly occurrence override its purpose and visibility (#55)#60
pataniaeli merged 2 commits into
devfrom
feat/issue-55-occurrence-overrides

Conversation

@pataniaeli

Copy link
Copy Markdown
Collaborator

Starts #55. Two of the three overrides, end to end — purpose and hidden. Draft, because the migration isn't applied and is_event is deliberately left out (reasoning below).

Approach

Occurrences already diverge from their series on room, time, status and reservation code — each a nullable column on weekly_room_occurrences where NULL means inherit. These follow that convention exactly; the only difference is they inherit from the bookings row two levels up rather than from weekly_room_bookings.

Why hidden is a nullable boolean

It needs three states, not two: inherit, forced visible, forced hidden.

not null default false would collapse inherit into visible, making it impossible to publish a single week of an otherwise hidden series — which is the more interesting half of what the issue asks for.

That third state changes how visibility is decided, so the filter in lib/my-rooms-data.ts is reworked. Previously a booking was dropped when hidden && !canManage. Now hidden occurrences are stripped first, and a weekly series survives if any occurrence remains:

series occurrence result
visible inherit shown
visible hidden = true that week hidden, rest shown
hidden inherit nothing shown
hidden hidden = false only that week shown

One-time and tabling bookings have no per-occurrence override and keep the old booking-level rule.

The stripping happens server-side, deliberately. Filtering in the client's flatten step would mean sending a hidden occurrence to a browser that isn't allowed to see it and trusting the UI not to draw it — the shape of the leak #29 already had to be fixed once.

Purpose precedence

Resolves with ??, not the || the neighbouring room/time fields use, since only null should mean inherit. Empty input is normalised to null on write, so a cleared field reads as inherit rather than as a booking whose purpose is blank.

What's not here

is_event. Storing it is trivial — but the Events tab finds events by querying bookings where is_event, and its rows are bookings with sessions nested underneath. Teaching it about occurrence-level events changes both that query and the question of how such an event should be presented in a list whose unit is the booking. That's a design decision, not a mechanical addition, so I left it out entirely rather than ship a column and a toggle that nothing reads.

Worth deciding before I build it: should an occurrence-level event appear as its own row in the Events tab, or as the parent booking with only the flagged occurrences listed under it?

Before merging

⚠️ The migration is not applied. It adds two nullable columns and is additive — no backfill, no rewrite, existing rows read as "inherit" — but it's yours to run.

Typecheck, build and lint all pass (lint 3 warnings better than dev). The compiler was genuinely useful here: it caught two separate duplicate WeeklyOccurrence declarations in the admin components that also needed the new fields.

Not manually tested — this needs the migration applied and a signed-in admin to exercise the editor.

A weekly occurrence can already diverge from its series on room, time, status
and reservation code. This adds two more of the three overrides the issue asks
for: purpose, and hidden.

Both follow the convention already in place -- a nullable column on
weekly_room_occurrences where NULL means inherit -- the difference being that
these inherit from the `bookings` row two levels up rather than from
weekly_room_bookings.

hidden is a nullable boolean rather than `not null default false` because it
needs three states, not two: inherit, forced visible, forced hidden. A NOT NULL
default would collapse inherit into visible and make it impossible to publish a
single week of an otherwise hidden series, which is the more interesting half of
what the issue is asking for.

That third state changes how visibility is decided, and the filter in
lib/my-rooms-data.ts is reworked for it. Previously a booking was dropped when
`hidden && !canManage`. Now hidden occurrences are stripped first, and a weekly
series survives if any occurrence remains -- so a hidden series with one
occurrence forced visible shows that week and nothing else. One-time and tabling
bookings have no per-occurrence override and keep the old booking-level rule.

The stripping happens server-side, on purpose. Filtering in the client's flatten
step would mean sending a hidden occurrence to a browser not allowed to see it
and trusting the UI not to draw it, which is the shape of the leak issue #29
already had to be fixed once.

purpose resolves with `??` rather than the `||` the neighbouring room and time
fields use, since only null should mean inherit. Empty input is normalised to
null on write, so a cleared field reads as inherit rather than as a booking
whose purpose is blank.

NOT included: the per-occurrence `is_event` the issue also asks for. Storing it
is trivial, but the Events tab finds events by querying `bookings` where
is_event, and its rows are bookings with their sessions nested underneath.
Teaching it about occurrence-level events changes both that query and the
question of how such an event should be presented in that list -- a design
decision rather than a mechanical addition. Left out entirely rather than
shipping a column and a toggle that nothing reads.

The migration is not applied.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

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

Project Deployment Actions Updated (UTC)
chambers Ready Ready Preview Aug 28, 2026 11:44pm

Completes the issue. The occurrence is the event marker, so a weekly event is
one week rather than a whole series, and it appears in the Events tab in its own
right.

is_event is NOT NULL DEFAULT false on the occurrence, not a nullable override
like the purpose and hidden columns in the previous commit. A series is not an
event that individual weeks opt out of -- one week is the event -- so the
occurrence is authoritative with nothing to inherit. The editor gets a checkbox
rather than a three-way select for the same reason, and "Clear all overrides for
this date" deliberately leaves it alone, because being an event is not an
override of anything.

Nothing needs backfilling. Every booking currently flagged is_event is a
One-Time Room, and the Administrator UI has never offered Mark Event on weekly
bookings at all -- only a badge -- so booking-level is_event stays exactly as it
is for one-time and tabling.

The Events route cannot reach these through its existing query: filtering an
embedded resource narrows the child array without selecting the parent, so a
second query fetches flagged occurrences with their ancestry and folds them in.
Each becomes its own row, with `<bookingId>:<date>` as its id -- the checklist
and the pending-actions highlighting are both keyed by row id, and every flagged
week of one series would otherwise collide on the booking's id. The row carries
occurrence_date so the detail block prints one date instead of a "Sep 1 - Sep 1"
range, and its purpose resolves through the occurrence override.

event_tracking had to change shape. booking_id was the primary key, so a booking
had exactly one checklist and two flagged weeks of the same series would have
shared it -- ticking a form on one would tick it on the other. The target is now
(booking_id, occurrence_date), null meaning the booking itself, which is what
one-time and tabling events keep using and what every existing row already is.

Keyed on the date rather than an occurrence id deliberately: the weekly PATCH
handler regenerates occurrences on every save, deleting them all and reinserting
with fresh ids, and values survive only by being carried across on the date. A
foreign key to weekly_room_occurrences(id) would have dropped every checklist
the next time anyone edited the booking. The date is the identifier that write
model actually preserves.

UNIQUE NULLS NOT DISTINCT is what makes the booking-level row work; a plain
unique index treats NULLs as distinct, so the upsert would insert a new row on
every toggle instead of updating. Both migrations were applied and rolled back
against production to confirm the primary-key swap preserves the existing rows.

Migration 20260829001000 is not applied.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@pataniaeli
pataniaeli marked this pull request as ready for review August 28, 2026 23:44
@pataniaeli

pataniaeli commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator Author

is_event is now in — the occurrence is the marker (82f3d74)

Issue #55 is complete. A weekly event is one week, not a series, and appears in the Events tab in its own right.

is_event is NOT NULL DEFAULT false on the occurrence — not a nullable override like purpose and hidden. A series isn't an event that individual weeks opt out of; one week is the event, so the occurrence is authoritative with nothing to inherit. Hence a checkbox rather than a three-way select, and "Clear all overrides for this date" deliberately leaves it alone — being an event isn't an override of anything.

No backfill needed: every currently-flagged booking is a One-Time Room, and the Administrator UI has never offered Mark Event on weekly bookings (only a badge), so booking-level is_event is untouched for one-time and tabling.

Two things this forced

The Events route couldn't reach them. Filtering an embedded resource narrows the child array without selecting the parent, so flagged occurrences are fetched by a second query with their ancestry and folded in. Each becomes its own row with id <bookingId>:<date> — the checklist and pending-actions highlighting are both keyed by row id, and every flagged week of one series would otherwise collide on the booking's id. The row carries occurrence_date so the detail block prints one date instead of a "Sep 1 – Sep 1" range, and its purpose resolves through the override from the first commit.

event_tracking had to change shape. booking_id was the primary key — one checklist per booking — so two flagged weeks of the same series would have shared it: ticking a form on one would tick it on the other. The target is now (booking_id, occurrence_date), with null meaning the booking itself, which is what one-time and tabling events keep using and what every existing row already is.

⚠️ Keyed on the date, not an occurrence id — this matters. The weekly PATCH handler regenerates occurrences on every save: it deletes them all and reinserts with fresh UUIDs, and values survive only by being carried across on the date. A foreign key to weekly_room_occurrences(id) would have silently dropped every checklist the next time anyone edited the booking. The date is the identifier that write model actually preserves.

UNIQUE NULLS NOT DISTINCT is what makes the booking-level row work — a plain unique index treats NULLs as distinct, so the upsert would insert a new row on every toggle instead of updating.

Verified against production

Both migrations applied and rolled back in a transaction: all 3 existing event_tracking rows preserved through the primary-key swap, columns added, constraint built. Also proved separately that a booking-level row and two occurrence-level rows coexist and that the upsert updates rather than duplicating.

Typecheck, build and lint pass (lint 3 better than dev).

Two migrations now

20260829000000 (purpose + hidden) — the one you said you'd apply.
20260829001000 (is_event + event_tracking) — also needs applying, and it's the one that alters an existing key, so worth reading first.

Still not manually tested — needs both migrations plus an admin to flag a week and tick its checklist.

@pataniaeli
pataniaeli merged commit 2145ab1 into dev Aug 28, 2026
4 checks passed
@pataniaeli
pataniaeli deleted the feat/issue-55-occurrence-overrides branch August 28, 2026 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant