feat: optional "Maybe" answer and attendance limit with waitlist - #207
Open
luflow wants to merge 2 commits into
Open
feat: optional "Maybe" answer and attendance limit with waitlist#207luflow wants to merge 2 commits into
luflow wants to merge 2 commits into
Conversation
"Maybe" is not a useful answer everywhere — issue #12 asks for control over which options people get, and the attendance limit in #27 needs yes/no only, because a "maybe" silently holds a spot away from someone who would commit. This is the narrow half of #12: the option set stays yes/no/maybe in storage, an appointment only decides whether the third one is offered. allow_maybe on the appointment is deliberately tri-state. NULL means the appointment has no opinion and follows the instance-wide setting, so flipping that setting still reaches every appointment nobody has decided about. The parameter has no such state: null there means the caller left the field alone, and an appointment that has an answer keeps it. The guard lives in one place because there are two writers. AppointmentService::applyResponse() serves web, mobile and on-behalf; ResponseService::submitResponse() serves the quick-response links in notifications, and the two have already drifted (the link path has no visibility check and rejects withdrawal). A rule added to only one of them would be a hole in the other, so both now call ResponsePolicyService and both have a test that fails if the call goes away. Collapsing that duplication is worth doing on its own; it changes quick-link behaviour in two user-visible ways and does not belong in a feature commit. The notification drops its "Maybe" button where the appointment does not offer one — the link would only ever answer 400 — and falls back to showing all three for an appointment it can no longer find. The clients get the resolved boolean, never the tri-state: serializeAppointment() reads the column against the instance default, and every appointment payload now goes through it. Old mobile clients keep working; they see the new allowMaybe field, ignore it, and get a 400 if they post an answer the appointment does not take. New clients gate on the responseOptions capability, with allowMaybeDefault carrying the instance setting so the appointment editor knows where to start its switch. The matching Flutter gate follows separately. psalm-baseline.xml grows by 17 lines, all of the structural kind every sibling already carries: two controller params (route-dispatched, like every other param in that file), the entity property and its type (magic accessors, like the other 24 in Appointment), the migration's UnusedClass (like every Version*), and ClassMustBeFinal plus the DI constructor for the new service (final would break PHPUnit mocking — see CLAUDE.md). Everything psalm could actually be fixed on was fixed in the code.
Closes #27. An appointment can cap how many people get in; once it is full the next person queues instead of being turned away, and a spot that frees up promotes whoever is next. Who holds a spot is derived, never stored. The yes-responses ordered by spot_claimed_at, the first max_attendees of them in, the rest waiting. That is what makes promotion free: somebody withdraws, and everyone behind them moves up as a consequence of the ordering — no write, no job, nothing that can drift out of step with the answers it is built from. A fourth response value would have cost far more: 'yes'|'no'|'maybe' is switched on across thirty PHP files and every old mobile client, none of which would know what to do with it. spot_claimed_at rather than responded_at, because responded_at is rewritten when somebody only edits their comment, which would quietly send them to the back of the queue. It is stamped when a response becomes a yes and cleared when it stops being one, so answering yes again joins the back — the behaviour the issue describes for somebody who declines and changes their mind. Deliberately orthogonal to the booking dimension. Both answer "who has a place", but booking is organizer-authored and switched on instance-wide, capacity is self-service and per-appointment. Overloading booking_status with both would have left the close-time wave unable to tell "the organizer did not plan you in" from "you were fourth in line". A limit takes "Maybe" off the table, on the server and not only in the form: a maybe holds a spot away from somebody who would commit to it. The intent is explicit rather than inferred. respond takes acceptWaitlist, and without it a full appointment answers 400 — so a client that has not been taught about queues (an old mobile build, a signed quick-response link, a page loaded before the last spot went) gets an honest refusal instead of a silent place in line its UI would render as "attending". Organizers answering on somebody's behalf may exceed the limit; real rosters have exceptions, and making them edit the limit and edit it back would produce a worse audit trail. Lowering a limit below what is already taken demotes nobody. The appointment sits over capacity until people drop out, which is the one thing a waitlist must never do: take back a spot somebody was told they had. Where a place in line differs from a place at the appointment, it says so: the calendar feed leaves the slot free and marks the title (immediately, not gated on close — this is where the person stands, not a verdict the organizer has yet to make), the export sheet an organizer takes to the door says Waitlist rather than Yes, and the check-in list flags them. Statistics stay on the raw response: they measure responsiveness over time, and queue position is transient. Two notifications, both fire-once through a marker in the same shape as the booking wave: a spot came free, and — at close — no spot came free, which is what releases somebody from holding a date for nothing. Joining the queue is recorded but never announced; they just clicked the button. The promotion is also the one state change here with no human behind it, so it gets an audit entry: it answers "why am I suddenly in?". psalm-baseline.xml shrinks by 89 entries. Typing four legacy parameters that the new code reaches through — generateVEvent's $l and $response, getTableXml's rows, the summary's serializeResponse — fixed 105 findings and let those entries leave the baseline. The ~16 added back are the structural kind every sibling already carries: route-dispatched controller params, entity properties behind the magic accessors, the migration's UnusedClass, and ClassMustBeFinal plus the DI constructor for the two new services (final would break PHPUnit mocking). The e2e spec could not be run here — no Docker in this environment — so it is covered by CI. Everything else in ./scripts/check.sh passes.
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.
Closes #27, and lands the half of #12 that #27 needed.
Two commits, meant to be read in order — the second depends on the first.
9a3d120— an appointment can drop "Maybe" from its answersThe narrow slice of #12: the option set stays
yes/no/maybein storage, an appointment only decides whether the third one is offered. A full custom-options refactor would have to touch those literals across thirty PHP files and every old mobile client; this doesn't.allow_maybeis tri-state on purpose. NULL means the appointment has no opinion and follows the instance-wide setting, so flipping that setting still reaches every appointment nobody has decided about. The parameter has no such state — null there means the caller left the field alone.3f9b57a— attendance limit with an optional waitlistCap how many people get in; once full the next person queues, and a spot that frees up promotes whoever is next.
Who holds a spot is derived, never stored. Yes-responses ordered by
spot_claimed_at, the firstmax_attendeesin, the rest waiting. That is what makes promotion free: somebody withdraws and everyone behind them moves up as a consequence of the ordering — no write, no job, nothing that can drift out of step with the answers it is built from.spot_claimed_atrather thanresponded_at, because the latter is rewritten when somebody only edits their comment, which would quietly send them to the back of the queue.Design decisions worth reviewing
AppointmentService::applyResponse()(web, mobile, on-behalf) andResponseService::submitResponse()(quick-response links) are near-identical twins that have already drifted. Both now callResponsePolicyService, and both have a test that fails if the call goes away. Collapsing the duplication is worth doing on its own and deliberately isn't in here — it changes quick-link behaviour in two user-visible ways.booking_statuswould leave the close-time wave unable to tell "the organizer did not plan you in" from "you were fourth in line".respondtakesacceptWaitlist; without it a full appointment answers 400. An old mobile build, a signed quick-response link, or a page loaded before the last spot went gets an honest refusal instead of a silent place in line its UI would render as "attending".Where a place in line differs from a place at the appointment
The calendar feed leaves the slot free and marks the title (immediately, not gated on close — this is where the person stands, not a verdict the organizer has yet to make); the export sheet says Waitlist rather than Yes; the check-in list flags them. Statistics stay on the raw response: they measure responsiveness over time, and queue position is transient.
Two fire-once notifications in the same shape as the booking wave — a spot came free, and at close no spot came free, which is what releases somebody from holding a date for nothing. Joining the queue is recorded but never announced. The promotion is the one state change here with no human behind it, so it gets an audit entry: it answers "why am I suddenly in?".
psalm-baseline.xml shrinks by 89 entries
The new code reached through four untyped legacy parameters —
generateVEvent's$land$response,getTableXml's rows, the summary'sserializeResponse. Typing them fixed 105 findings and let those entries leave the baseline. The ~16 added back are the structural kind every sibling already carries: route-dispatched controller params, entity properties behind the magic accessors, the migration'sUnusedClass, andClassMustBeFinalplus the DI constructor for the two new services (final would break PHPUnit mocking).Verification
./scripts/check.shpasses all ten gates — 410 unit tests, psalm, php-cs-fixer, eslint, stylelint, the vite build, both l10n checks, and the OpenAPI diff.tests/e2e/33-attendance-limit.spec.jshas not been run. No Docker in the environment it was written in, so it is unverified until CI runs it. It covers full → refused → join → drop → promote, the maybe interaction, the no-waitlist path, and that an unlimited appointment behaves exactly as before.Follow-ups, not in here
attendanceLimitandresponseOptions, plusallowMaybeDefault. Appointments carrymaxAttendees,waitlistEnabled,occupancy,isFull; responses carrywaitlisted/waitlistPosition. Server-first is safe by construction — old clients get the explicit 400 rather than a silent wrong state.🤖 Generated with Claude Code
https://claude.ai/code/session_01HTQweHTfkj8qSZQYccEk8d
Generated by Claude Code