Selectable booking duration per meeting type - #153
Merged
Conversation
A meeting type gets a set of allowed lengths in a new meeting_type_duration table, each carrying optional per-duration buffers; the invitee picks one via a ?duration= link above the slot grid and the grid re-renders for it. The default stays meeting_type.duration_minutes and is an implicit member of the set, so there is no backfill and no way to edit a set that omits it — an amendment to ADR-0003, which had the save reject the removal instead. Also amends ADR-0002: where a host override and a duration override both apply, the max is taken over the overrides actually set. Letting a NULL fall back to the type's buffer inside the max would raise a host's deliberate 5 back to the type's 10. Upstream answered both open questions on #119: fixed lattice (the cadence anchors to the shortest allowed length, so start times do not move when the invitee switches), and no per-host duration limits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MrnPiNwAQhe2RtmYTteNcj
Membership is a union at read time — rows ∪ {duration_minutes} — so no
sequence of edits can produce a set that omits the default. That
removes the reject-at-save path ADR-0003 relied on, and with it the
only way the meeting type's main edit form and the durations form
could disagree about what the set contains.
A row for the default now carries only that length's buffer overrides;
deleting it drops the overrides, not the duration.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MrnPiNwAQhe2RtmYTteNcj
Five entries went stale with selectable duration. Meeting type offers lengths, not a length. Buffer names its two possible sources and says an unset one is the absence of a requirement. Cadence belongs to the type, never to a host — that shared lattice is what lets co-hosts' slots intersect. Allowed durations is a union, so it cannot omit its default. Booking carries its own length, and reschedule moves it rather than resizing it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MrnPiNwAQhe2RtmYTteNcj
Multi-host slots are anchored to 00:00 in each host's own timezone, and the per-host free sets are intersected by exact start instant. Host-local midnight is a different instant per host, so the combs coincide only when the hosts' UTC offsets differ by a whole number of the cadence. London and Berlin on a 45-minute cadence differ by 60, which is not a multiple of 45: the intersection is empty every day forever, and the page renders the ordinary "no times available" state. ADR-0008 gives the type one lattice, anchored at LocalDate.EPOCH.atStartOfDay(creatorZone). The Creator's zone supplies the phase, so start times stay round on the clock of whoever defined the type and an all-one-timezone shared type is unchanged. The constant date supplies request-independence: anchoring to the request's own from-date would let the booking page and the submit-time re-check disagree at a cadence that does not divide 1440. Filed as calit-io9y and fixed inside calit-p5xm, which rewrites the same grid line. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MrnPiNwAQhe2RtmYTteNcj
Twelve tasks, each red-green-commit. Groups the booking template's chrome/captcha params first so the feature diff lands on a readable signature, then the table, slot parameterisation, the shared-lattice fix (calit-io9y), buffers, the booking write path, reschedule, email, both UIs, translations, and verification. Notes the one ordering hazard: task 4 replaces the dayAnchoredGrid boolean on the same signature task 6 adds the duration to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MrnPiNwAQhe2RtmYTteNcj
Task 4 removes the boolean parameter task 3's test was written against, so task 4 owns updating that test. lengthOf must be public: task 8 consumes it from a different package. Both test book(...) calls were missing the honeypot argument. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MrnPiNwAQhe2RtmYTteNcj
Templates.book took 14 positional arguments including a bare null and a bare empty string, from two call sites that must stay in lockstep. No behaviour change.
PublicBookingPageTest does not exist; the proof of no-behaviour-change is the existing renderers of book.html passing untouched.
Berlin 09:00-17:00 against Kathmandu 11:00-19:00 on a 29-minute cadence: the offset is 285 minutes and 285 mod 29 = 24, so under per-host anchoring the two combs shared no instant at all. Asserts a single comb survives, not merely that something does.
New meeting_type_duration table, one row per extra length with optional per-duration buffers. The type's own duration_minutes is an implicit member of the set (ADR-0003), so the migration is DDL only and every existing type is already valid. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MrnPiNwAQhe2RtmYTteNcj
The grid step now falls back to the type's shortest allowed length rather than its duration, so the lattice of candidate starts does not move when an invitee switches length. Only the slot body varies. Single-duration types are unaffected: shortest == duration. Deletes MeetingType.effectiveSlotIntervalMinutes(): its only caller was the SlotService line just replaced, and its only remaining reference was a now-dead unit test of the method itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MrnPiNwAQhe2RtmYTteNcj
The window end is inclusive, so 11:15 belongs: its body ends exactly at 12:00. Also note that the test helper must seed OwnerSettings.
Multi-host slots were anchored to midnight in each host's own timezone and then intersected by exact instant, so two hosts' grids coincided only when their UTC offsets differed by a whole number of the cadence. London and Berlin on a 45-minute cadence never did: the intersection was empty every day and the page rendered the ordinary 'no times available' state. Every host now aligns to one anchor at the creator's local midnight on a fixed reference date (ADR-0008). Single-host stays window-anchored, byte-identical to before. Fixes calit-io9y.
An origin instant plus multiples of the step freezes whatever offset the creator's zone had on the origin's date. Asia/Kathmandu was +05:30 until 1986 and +05:45 since, so an epoch origin moves an all-Kathmandu team from 09:00/09:30 to 09:15/09:45 -- charging a team that never had the bug to fix someone else's. The lattice is now the instants whose local time-of-day in the creator's zone is a whole number of steps past midnight. No origin, so the zone's rules are read at each instant; hosts still agree by construction because there is one definition in one zone. Task 4 rewritten on it; 2a59d0f is superseded and stays in history.
Multi-host slots were anchored to midnight in each host's own timezone and then intersected by exact instant, so two hosts' grids coincided only when their UTC offsets differed by a whole number of the cadence. London and Berlin on a 45-minute cadence never did: the intersection was empty every day and the page rendered the ordinary 'no times available' state. The lattice is now the instants whose local time-of-day in the creator's zone is a whole number of steps past midnight (ADR-0008). One definition, one zone, so hosts agree by construction, and the zone's rules are read at each instant rather than frozen at an origin date -- an all-Kathmandu team keeps the round times it has today, which a fixed epoch origin would have moved by 15 minutes because Kathmandu shifted +05:30 to +05:45 in 1986. Supersedes the origin-based lattice in 2a59d0f. Fixes calit-io9y.
Review of the previous commit found the lattice branch computed its phase once per window and carried it forward with plusMinutes, so a window whose Creator-local start fell late the previous day (or a cadence that doesn't divide 1440) drifted onto a different comb than a window starting fresh at that day's own midnight -- two combs again, reproduced with a LA/Berlin cadence-50 case (intersection 0, ADR predicate says 7). Candidate starts are now enumerated per Creator-local day via ZoneRules.getValidOffsets: zero offsets skips a spring-forward gap, two offsets emits both instants of a fall-back's repeated hour (both are real and satisfy the predicate, keeping single- and multi-host slot counts consistent across the same transition). The per-day scan uses continue rather than break on the window-end test, since the resolved-instant sequence is no longer monotone once a local minute can resolve to zero, one, or two instants; the method's output is sorted by start instant before returning so callers always see chronological order. Also: BookingService.hostFreeSlots reads the Host's timezone through OwnerSettings.coerceZone (a legacy row could otherwise throw before generateRawSlots's own coercion helps), and SlotServiceLatticeTest#theLatticeIsRoundInTheCreatorsZoneNotTheHosts now uses a Kathmandu Creator (whose +05:45 offset is not a multiple of the cadence) so the test actually distinguishes a Creator-anchored lattice from a UTC- or Host-anchored one -- the previous Kolkata Creator (+05:30) could not. Fixes calit-io9y.
A host override and a chosen length's override can both apply; per ADR-0002 the larger of those actually SET governs, and an unset one is the absence of a requirement rather than a floor at the type's buffer. Every existing row has a null duration override, so its effective buffer is unchanged.
availableSlots and book carry the chosen length; the submitted value is checked against the type's allowed set before anything else, since an arbitrary duration would otherwise build a self-consistent lattice that passes every downstream check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MrnPiNwAQhe2RtmYTteNcj
Both reschedule paths recomputed the end from the meeting type's duration, so a booking made at a non-default length would silently shrink when moved. A booking carries its own length.
The single-booking test alone couldn't verify rescheduleGroup, the more intricate of the two paths: it moves several rows and takes the length from whichever row drove the reschedule. Add a multi-host test that books at a non-default length and asserts every row in the group -- not just the initiating one -- keeps that length after the move. Confirmed red against a temporary revert of rescheduleGroup's end-time computation (expected 90, was 60) before restoring the fix.
A 120-minute booking on a 30-minute-default type announced itself as 30 minutes in its own confirmation. All 9 EmailService call sites now read BookingService.lengthOf(l.booking) instead of l.meetingType.durationMinutes. ICS and Google sync already read the booking's own start/end instants and needed no change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MrnPiNwAQhe2RtmYTteNcj
A ?duration= link per allowed length above the slot grid, rendered only when a type offers more than one. Plain anchors, so it works with JavaScript disabled, and a shared link with a preselected length falls out for free. An unknown value falls back to the default rather than 404ing; the submitted length rides along as a hidden form field and survives a validation bounce. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MrnPiNwAQhe2RtmYTteNcj
renderManage passed type.durationMinutes to daySlots, so the reschedule page offered slots at the type's default length. Task 7 already re-checks a reschedule submission at the booking's own length, so a non-default-length booking (e.g. 120 min on a 30-min-default type) saw the page offer slots the submit would then reject with a 409 — the grid and the re-check disagreed. Per the design spec, reschedule freezes the booked length and offers no picker; renderManage now passes BookingService.lengthOf(booking) instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MrnPiNwAQhe2RtmYTteNcj
One row per allowed length plus a blank spare; saving persists every row with a duration and drops the rest, so clearing a duration is how a length is removed. No JavaScript, one POST. The default is an implicit member of the set, so clearing its row drops only its buffers, never the length (ADR-0003). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MrnPiNwAQhe2RtmYTteNcj
Tasks 9-10 added eight AppMessages/AdminMessages keys for the invitee duration picker and the owner's allowed-durations editor with English defaults only, leaving MultiHostMessageParityTest red on both locale checks. Adds German and Hebrew values, reusing existing terminology (Puffer/חיץ for buffer, Dauer/משך for duration, Min./דק' for the minutes abbreviation) so the new strings read as part of the same bundle rather than inventing new vocabulary. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MrnPiNwAQhe2RtmYTteNcj
Both fully ticked with summaries. calit-p5xm ships the feature; calit-io9y ships the cross-timezone lattice fix that the same grid line made cheap to land alongside it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MrnPiNwAQhe2RtmYTteNcj
…duration Fixes four Important findings from the branch's final review: the owner-side reschedule grid was drawn at the type's default length instead of the booking's own (mirrors the invitee-side fix), duplicate duration rows could 500 the whole durations save via an @IdClass collision, a per-duration/ per-host buffer of 0 couldn't survive a save/render/save round trip (Qute's falsy rule treats 0 as blank) in both meetingTypeDetail.html and its sharedAvailability.html twin, and the landing page regained an N+1 on allowedDurations that the existing bookableTypeIds batching pattern already solved once. Also folds in the review's cheap hygiene: deletes now-dead assertSlotAvailable/effectiveBuffer overloads, makes assertDurationAllowed private, adds a missing ORDER BY in SlotService's rules query, and fixes a duplicated i18n key choice on the durations table. Adds the requested coverage: an end-to-end cross-timezone availableSlots test (manually confirmed red when latticeZone is forced null), a cross-owner isolation test for the durations route, and regression tests for the duplicate-duration and zero-buffer fixes. Full suite: 975 tests, 0 failures, 0 errors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MrnPiNwAQhe2RtmYTteNcj
The durations editor grows one row per save. The working-hours grid already clones rows client-side from a <template>, and both editors post the same parallel-array shape, so this needs no server change. The blank spare row stays as the no-JS path.
The allowed-durations editor grew one row per save: adding a second length meant saving and coming back, and the default could only be changed from the Basics section, in a different part of the page. durations.js follows workplan.js — a delegated handler scoped to [data-durations] that clones [data-duration-template]. The trailing blank row stays: it is the no-JS path, so an owner without JavaScript still adds one length per save. Removing the last row blanks it rather than deleting it, so there is always somewhere to type. The Default radio carries the row INDEX rather than a duration, which is what lets a length typed into the blank spare be made default in the same save — at render time that row has no value for the server to match on. durations.js renumbers after every add and remove, or the radio would point at a shifted or deleted row. Server-side that is one edit to meeting_type.duration_minutes, so ADR-0003 is unchanged: the default is still an implicit member of the set, the previous default keeps its row, and moving the default never drops a length. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MrnPiNwAQhe2RtmYTteNcj
Design review of the durations editor found three things, all mine: The panel used two words for one concept -- the section, column and save button said "duration" while the add button and hint said "length". An action keeps one name through a flow, and this project has a glossary to stop exactly this. Everything says duration now. The default row had an empty cell where the x sits on every other row, which reads as a broken column rather than a rule. It now renders a disabled x whose title says why: the default is always offered, move it to another row first. The hint had accreted into three dense lines as the feature grew -- adding, removing, buffer inheritance and default semantics all in one paragraph. It is one line now. Buffer inheritance moved onto the inputs that own it, as a placeholder showing the meeting type's own buffer, so a blank field displays the number it will actually use instead of describing it in words that did not fit the box. That also retires a message key rather than adding one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MrnPiNwAQhe2RtmYTteNcj
S3776 (critical) — generateRawSlots reached cognitive complexity 37 against a limit of 15: five nesting levels, with the window-anchored and lattice-anchored branches inlined side by side. Each branch is now its own method, so the loop body dispatches instead of unfolding. No behaviour change; the ADR-0008 reasoning moves with the code it explains. S7467 ×2 — two catch blocks bound an exception they never read; Java 25 unnamed patterns say so directly. S5778 — the assertThrows lambda in BookingDurationTest had two calls that could throw, so a failure in the slot lookup would have read as the rejection under test. The lookup is resolved outside the lambda. S107 — the 11-arg book(...) overload predates this branch and exists precisely to preserve the pre-duration arity, so it carries the same waiver as the 12-arg form rather than being collapsed into a record. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MrnPiNwAQhe2RtmYTteNcj
No og: or twitter: meta exists anywhere today, so a booking link unfurls as a bare URL — on the product's most-shared artifact. Notes the capability-URL leak as the first task: the manage and decline pages are reachable without logging in, so a rich preview would render the invitee's name and meeting time into whatever chat the token was pasted into.
Instant has no plusMinutes, so the code multiplied by 60 at each use and passed bare long second counts between methods. Instant.plus does take a TemporalAmount, so both spans are Durations now: the slot body and the gap between starts say what they are, and the magic 60 disappears from the call sites along with the seconds-vs-minutes question at every parameter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MrnPiNwAQhe2RtmYTteNcj
…reopen he09 calit-xjrg corrects a call I got wrong in the p5xm final review: I recorded the unvalidated meeting-type duration as a non-blocking minor because it looked unreachable. The Basics field has no min attribute, so it is reachable by typing, and a zero there spins slot generation forever and pins the request thread. calit-ntek is the same fallback with a small number instead of zero. he09 goes back to todo: I marked it completed with an unchecked item.
Saving durationMinutes=0 through the create form returned 200 and persisted. Loading that type's public page then hung: jstack showed the request thread in SlotService.addWindowAnchored with GC at 93-98% -- the cadence falls back to the shortest allowed length, a zero step means neither loop can advance, and each iteration allocates a TimeSlot, so it races toward OOM while pinning the thread. Four layers, since each alone is bypassable: min="1" on both duration inputs (the create form lacked it too); a server-side rejection that renders localized like the slug rules, because an HTML attribute is a hint to a browser and not a guard against a POST; V30 repairing any existing non-positive row to 30 BEFORE adding the CHECK, so the constraint cannot fail validation and take the app down at boot on someone else's data; and Math.max(1, step) in SlotService for rows written before the guard existed. The durations table needed nothing -- parsePositive already refused <= 0 there. Fixes calit-xjrg. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MrnPiNwAQhe2RtmYTteNcj
The exclusion constraint's GiST index already has the right shape -- owner equality, time overlap, filtered to held rows -- but the query expresses overlap as two btree comparisons the planner cannot match to it, so it scans every held booking the owner has ever had. Filed with measurement as the first task: at one-person self-hosted volumes this may never matter, and that is worth finding out before adding an index to maintain.
|
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
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.



A meeting type could only ever be booked at one length, so an owner running 30-, 60- and 120-minute sessions kept three near-duplicate types. It now carries a set of allowed lengths, each with optional per-duration buffers, and the invitee picks one above the slot grid.
Closes #119.
What changed
meeting_type_duration(V29, DDL only) holds the extra lengths. The type's ownduration_minutesis an implicit member of the set — read as a union — so a set omitting the default is not expressible, there is no backfill, and no reject-at-save path exists anywhere (ADR-0003).NULLis the absence of a requirement, not a floor at the type's buffer, so a host's deliberate 5 is never raised to the type's 10 (ADR-0002).?duration=Nabove the grid, no JavaScript. A single-duration type renders an identical page to before.The owner's editor
A table of allowed durations on the meeting-type page, with a per-row before/after buffer.
+ Durationadds a row and×drops one, both client-side, so several lengths can be entered before saving. With JavaScript disabled the button does nothing — which is why the table always renders one spare blank row: that is the no-JS path, and it adds one length per save.Defaultradio picks the length an invitee sees before choosing. It carries the row index rather than a duration, which is what lets a length typed into the blank spare be made default in the same save — at render time that row has no value for the server to match on.×is disabled, not absent: the default is always a member of the set, so an empty cell would read as a broken column rather than a rule. Its title says which rule.Three bugs fixed alongside
Rescheduling resized bookings. Both reschedule paths recomputed the end from
type.durationMinutes. Latent until a type could offer a second length, then it silently shrank a 120-minute booking to 30.Shared types across timezones offered nothing at all. Multi-host slot grids were anchored to midnight in each host's own timezone and then intersected by exact instant, so two hosts whose UTC offsets differed by a non-multiple of the cadence matched on nothing — London and Berlin on a 45-minute type, forever, behind an ordinary "no available times" message. Every host now shares one lattice defined by local time-of-day in the creator's clock (ADR-0008). The zone's rules are read at each instant rather than frozen at an origin date, which is what keeps a team wholly inside one timezone on the round times they already have —
Asia/Kathmandumoved +05:30→+05:45 in 1986, and an epoch-anchored design would have shifted them 15 minutes for a problem they do not have.A zero duration hung slot generation. Saving
durationMinutes=0returned 200 and persisted — the field had nomin, the setter no validation, andmeeting_type.duration_minutesno CHECK. The cadence falls back to the shortest allowed length, so a zero step meant neither slot loop could advance, and each iteration allocates:jstackshowed the request thread parked inSlotServicewith GC at 93–98%, racing toward OOM while pinning the thread. Refused now at the input, at the setter, and inV30— which repairs any existing non-positive row before adding the CHECK, so the constraint cannot fail validation and take an instance down at boot on someone else's data.SlotServicealso floors the step at 1 for rows written before the guard.Upgrade
Nothing to do — no configuration, no manual data migration. Existing meeting types keep exactly the lengths and start times they have.
Two caveats worth knowing:
:15/:45). Those are the only instants everyone can actually share, and previously the type offered none.V30rewrites any meeting type whoseduration_minutesis zero or negative to 30. Such a type is already broken — every render of its public page hangs — so this repairs it rather than failing the migration.Verification
mvn test— 983 tests, 0 failures, 0 errors.spotless:checkclean. The five SonarCloud findings this branch raised are cleared, including a cognitive-complexity refactor ofgenerateRawSlots.Exercised manually against
quarkus:devwithcurl, which runs no script at all — the honest test for the no-JS guarantee: a single-duration type renders no picker;?duration=120and?duration=30share an identical lattice prefix with the longer grid stopping earlier;?duration=45and?duration=abcfall back to the default rather than erroring; a 120-minute booking made through plain HTML forms confirmed and rescheduled at 120. The editor's add/remove/default behaviour was driven in a real browser: two lengths added in one save, and a brand-new 90 made default in that same save with 60 surviving as an ordinary length.Docs (changelog, meeting-type usage, and screenshots of the editor, the picker and a co-host's buffer) are on the
docs-sitebranch, pushed separately.Follow-ups filed
calit-o89d— no page has anyog:/twitter:meta, so a booking link unfurls as a bare URL. Its first task is a leak that exists today: the token-bearing manage pages neednoindexand no preview.calit-xqle—heldOverlappingcannot use the GiST index the exclusion constraint already provides, so it scans every held booking an owner has ever had. Measurement first.calit-ntek— a 1-minute allowed duration silently drops the cadence to 1.calit-he09— verify whether the working-hours grid really cannot add a frame without JavaScript.🤖 Generated with Claude Code
https://claude.ai/code/session_01MrnPiNwAQhe2RtmYTteNcj