Reconcile facilitator affiliations against attendance (admin bulk action) - #2195
Open
maebeale wants to merge 50 commits into
Open
Reconcile facilitator affiliations against attendance (admin bulk action)#2195maebeale wants to merge 50 commits into
maebeale wants to merge 50 commits into
Conversation
maebeale
force-pushed
the
maebeale/facilitator-affiliation-reconciliation
branch
from
August 14, 2026 04:58
68913c9 to
e765a96
Compare
maebeale
changed the title
maebeale
marked this pull request as ready for review
August 14, 2026 05:11
maebeale
commented
Aug 14, 2026
|
|
||
| def facilitator_start_date | ||
| (@training_date || Date.current).to_date.beginning_of_month | ||
| (@training_date || Date.current).to_date |
Collaborator
Author
There was a problem hiding this comment.
🤖 From Claude: Intentional shift toward accurate dates: this starts the facilitator affiliation on the actual training date rather than the first of its month (changing code that just landed in #2194). Reconciliation works either way; flagging since it edits a freshly-merged file.
maebeale
changed the title
maebeale
changed the title
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Same-daying an affiliation on or after its start date left it active: the callback always recomputed `inactive` from the dates, and a row ending today still reads as active. Admins also had no way to set it — the column was permitted everywhere but had no field, and ticking it alongside a date edit was silently overwritten. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The classification lived in ReconcileEvent while ReconcileFacilitatorAffiliation held a second, owned-only copy that nothing but its own spec reached — two implementations of the same rules, already disagreeing on hand-entered rows. ReconcilePerson is now the only place a decision is made; ReconcileEvent iterates it and keeps the keys, grouping and timestamp. Owned-vs-all becomes an argument, so the per-person reconciler is callable on its own (e.g. from an attendance change) without re-deriving anything. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The badge's Turbo submit answered with a stream that swapped only the status chip, so the row kept offering its pre-toggle action and the new return_to redirect never ran. Pages that pass a return_to now opt out of Turbo and get the full re-render; the registrants and onboarding pages keep the inline swap. Also lists the bulk action on the Features & tips seed and drops the "uncheck" wording left over from before the row controls became radios. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Main moved per-affiliation editing to the gear editor, so that — not the dense inline row — is where the flag belongs. Trims the comments added across this branch down to the ones carrying a why. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The person History card and activity timeline had nothing multi-year to render, so affiliation edits, trainings, memberships and comments couldn't be seen interleaved. Two gaps kept the seeded rows invisible: affiliation comments were missing from PersonCommentAggregator (Affiliation became commentable in #2235 without being added), and payment lifecycle events record the STI subclass ("CashPayment"), which the person's Payment filter never matched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Inactive checkbox only held until the next time anyone touched a date. The guard tested `inactive_changed?`, which is false when a form re-submits the value the record already holds, so the date rule ran and derived the flag away — an unrelated start-date edit silently reactivated a row an admin had ended. `inactive_supplied` records that a caller set the value on purpose. The standalone editor always posts the checkbox, so the controller sets it from the params; the nested rows set it when their end date changes. It is a cast writer because forms send "0", which is truthy in Ruby and would otherwise suppress the date rule on every nested row. An end date of today or earlier now ticks the box for you. The date rule compares strictly, so today alone still reads as active — the flag is what carries it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Deactivating same-dayed every row, including ones minted years earlier by a different training. That erased the whole period a person actually facilitated, and the anchored program status moved with it: an organization that read Ongoing at its 2026 training read Reinstated afterwards, changing figures that back grant applications. Only the row this training minted collapses to its start date — it recorded an assumption that never came true, and a strict `<` already excludes it from its own anchor. Anything older ends on this training's date instead, so the years before it survive. Reactivation had the mirror problem: clearing an end date swallowed the gap, so "Art program since" collapsed `Jan 2023 – Jan 2024, Aug 2026` into `Jan 2023`. A return is now a new row, which is what CreateFromRegistration has always done, so `:reactivate` is gone entirely. Why a row changed is recorded as a comment on the affiliation rather than a new column — the edit page already surfaces them, and the comment topic is enough to stop labelling an admin-ended row "didn't attend". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…atus column `Organization.active` counted a stored "Active" status as enough on its own, and `#published?` short-circuited on it before ever looking at affiliations — so an org whose column had drifted read as active with nobody facilitating there. ADR-0001 D3 says the column plays no part; these two were the exceptions. Also replaces nine open-coded copies of `!inactive? && (end_date.nil? || end_date >= today)` with `active?`. The rule now lives in one place, which matters more now that the flag can disagree with the dates. Expect orgs with a stale "Active" column and no active affiliation to start rendering as unpublished. That is the drift ADR-0001 D3a warns about, surfaced rather than introduced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`active?` and `active_on` sat two characters apart while answering different questions with different inputs — one reads the dates and the inactive flag to say what is true now, the other reads dates alone to say what was true on a date. The new name says which input it uses. ADR-0002 writes down what ADR-0001 left implicit: the two relationships the one table carries, that `inactive` is now an override rather than a cache, what `event_registration_id` does and does not mean, and the two rules above about not erasing history. The arithmetic behind the grant figures is covered directly rather than inferred from single-affiliation cases — several people at one anchor, Jan 1 vs Dec 31 in both directions, a full new → ongoing → reinstated → ongoing walk, and that reconciling a no-show leaves an anchored verdict where it was. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Long affiliation lists mix people who facilitate now with rows that ended years ago. The server already knows which is which, so it renders both groups and two radios drive the visibility through :has() — no JavaScript, and the person and organization forms share one partial instead of two copies. A row you end while editing stays exactly where it is and just restyles; its bucket only moves once you save. That is why this is not the registrants page's server-round-trip filter: switching tabs must not discard unsaved edits. Two Tailwind traps shape the markup. Radio ids cannot contain underscores — Tailwind reads `_` as a space inside an arbitrary value, so the selector matches nothing. And the group is named, because `group-hover:` matches any `.group` ancestor and an unnamed one made hovering pop every row's comment tooltip at once. The standalone editor now uses the same live styling, its comment icon opens the comments it is previewing, and a back link to an ended row lands on the section rather than a row hidden on the other tab. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An end date of "yesterday" computed in UTC is still today in the viewer's zone for part of each day, and ApplicationController sets the zone per user — so the row read as active and the redirect landed on it instead of the affiliations section. A month back removes the ambiguity. Same failure mode #2264 just fixed on the recipients program-status spec. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
inactive-toggle decides "ended" from the browser's local date, while Ruby's Date.current follows the Rails zone — for part of each day they are different dates, and the end-date-of-today examples failed on the difference rather than on the behaviour. Asking the browser for its own today tests what the controller actually compares against. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#2292 centralized the muted gray for back-nav links while this branch was in flight, so the two reconcile screens were the only ones still hardcoding it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
FacilitatorProgramStatus#active_on_anchor is private to a file upstream edits often — #2295 rewrote comments in it during this branch's life — so renaming it here bought a conflict on every rebase and no clarity at any call site. That file is byte-identical to main again. The scope keeps the name: Affiliation.active_by_date_on is what a new caller sees, and it is the one that has to be distinguishable from #active?. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Some inconsistencies only show up across the whole table. The reconcile page finds
facilitator affiliations minted by a non-training registration one event at a time,
and the org edit form warns about legacy-status drift one organization at a time —
neither answers "how many are there, everywhere".
Three checks to start:
- facilitator affiliations from non-training events, which count toward program
status without anyone having trained (deletes them, via destroy so the
organization's status and dates stay in step)
- affiliations whose minting registration belongs to a different organization,
which breaks ADR-0002 D2a's invariant and makes reconciliation treat the row as
auto-created for an org it never belonged to (unlinks them, the conservative
direction: the row becomes hand-entered, which reconciliation spares)
- legacy organization-status drift, which reports only. The stored vocabulary has
no value meaning "never active", and the affiliation callbacks only ever write
Active/Inactive, so any automatic rewrite would drift straight back.
Report-only is the base class default rather than an exception, because a wrong row
is not always one we know how to put right. The repair route resolves its param
against the registered checks and refuses anything else, so a report-only check
can't be coaxed into running one.
Also qualifies `affiliations.title` in the `.facilitators` scope. It broke as soon
as the scope was joined to `events`, which has a title of its own — the same
ambiguity `.active` already guards against for `end_date`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A row minted by the registration for this training records an assumption: that the person would become a facilitator on the training date. When they don't attend, there is no period to preserve — a zero-length row that reads as "ended the day it started" is just a worse way of saying it never happened. Older rows are unchanged: hand-entered ones, and ones from an earlier training, still end on this training's date. Those record facilitation that really happened, and deleting them would erase years of history and move the anchored program status at every training in between. This is what lets reconciliation stop leaning on the inactive flag. A same-dayed row could land on today and still read as active by dates alone, which is the case the flag existed for here. A deleted row has no such problem, and an older row ends on a training date that has already passed, so the date rule derives the flag by itself. The cost is that a deleted row takes its comments with it, so the reason D6b records survives only for ended rows. The deletion is still on the record as a destroy.affiliation event with the full attribute snapshot, and there is a spec for that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Main landed its own ADR-0002 (org linking flows and agreement scenarios) while this branch was in flight, so both files claimed the number — and both define a D4, which made every "ADR-0002 D4" in a comment ambiguous. This branch's ADR moves to 0003 and only the references that point at its decisions move with it; the ones naming main's scenario work stay put. Main also grew an AffiliationDecorator#period_label that renders an affiliation's span, which is what this branch's #date_range was doing. Dropped the duplicate and switched the reconcile screens to period_label — month precision rather than day, matching ADR-0001 D2 and every other affiliation period in the app. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Main grew its own live tinting on this editor while the branch was in flight, and taking its version of the field block dropped two things from this branch: the checkbox's inactiveCheckbox target, and the end-date field's endDateChanged action. Without them the box neither drove the styling nor ticked itself for a past end date. The system specs caught both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two statuses were being treated as "didn't attend" when they aren't. `registered` after the event is a gap in the record, not an outcome — #attendance_recorded? excludes it for exactly that reason. Deleting a facilitator affiliation because nobody filled the roster in is acting on missing data, and the deletion doesn't come back. Those rows are now left alone and listed under "Attendance never recorded — set an outcome first", which also nudges someone to go fix the roster. Cancelled and transferred-out stay as they were: those are decisions somebody made. `incomplete_attendance` still ends the affiliation, but the page now shows the sign-in sheet day by day underneath the person. "Incomplete" is a judgement someone recorded and the logged times are the evidence — an admin about to delete a facilitator affiliation should be able to see which days were missed without leaving the page. The status is looked up rather than read off the passed registration, because callers that only want a plan don't have to pass one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The row read as a sentence — "stored Active, affiliations say Never active" — which buries the only thing that matters: the two answers disagree. Side by side as chips, in labelled columns, the disagreement is the first thing you see. Both chips come from OrganizationDecorator.status_classes_for_bucket, the same DomainTheme lookup the org profile and index use, so a bucket means the same colour here as everywhere else and the two columns are actually comparable. Checks whose rows are worth comparing declare `columns` and a `row_partial`; the page still falls back to the one-line #describe for the rest, so nothing else had to change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Transferring to another event isn't failing to complete one — they're still going to train, somewhere else. So the affiliation follows them: its start date becomes the destination event's and its event_registration_id re-points at the destination registration. Re-pointing the provenance is load-bearing, not tidying. The FK is the auto-vs-manual gate (ADR-0003 D2a), so a row dated to the destination event but still pointing at the source would not be recognised as the row that training minted when the destination is reconciled — it would be treated as an older row and end-dated. Only the row with no end date moves; an already-ended row records a finished stretch and stays put, with a fresh affiliation created at the destination instead. A destination that hasn't been recorded yet, or one linked to a different organization, is reported rather than guessed at. Chained transfers need no traversal: A→B→C collapses when the second transfer is made, so the source already points at the final destination. Built as a fourth action on ReconcilePerson rather than a LinkSubmittedOrganization scenario — those describe what kind of linking is happening and run at link time, while a transfer is an attendance outcome found at reconcile time with nothing being linked. Also moves the end date for an older row to the day BEFORE the training, matching ApplyScenarioEndDating: a row ending the same day another starts counts on both and doubles the person in any report totalling a date. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ApplyScenarioEndDating and reconciliation both end an affiliation the day before the date that supersedes it, for the same reason: a row ending as another starts counts on both. They were documented independently, so a reader who found one would reasonably assume it was local to that flow and pick a different rule for the next caller. Each ADR now names the other. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The argument for it was that an org whose only facilitator is ended keeps reading Ongoing at that training. But the row is ended precisely because there is no record the person ever completed a training for that organization — so ending it ON the training date leaves the org's status at that training resting on the affiliation we just decided wasn't valid. Ending the day before is what the decision means. Reinstated at that anchor isn't a cost paid for consistency; with the row gone there is no basis for Ongoing, and none of the three labels fits "we removed the basis for their status" cleanly. No behaviour change — the code already ended everything the day before. This is the ADR catching up, plus three UI strings that still said "this training's date". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two things made it fragile. It hardcoded `wait: 2`, undercutting the suite's own 5s default — which exists precisely because "2s is too tight for selenium + Turbo on loaded CI runners". And hover is a one-shot mouse move: unlike an assertion it never retries, so if the row shifts under the cursor while Stimulus settles the styling, the move lands on nothing and waiting longer can't recover it. Now scrolls the icon under the cursor, re-hovers if the first attempt missed, and uses the default wait. The assertion itself is unchanged — it still fails if hovering one row's icon reveals another row's tooltip, which is the collision it was written for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Headless Chrome on CI never applies the CSS :hover from Selenium's synthetic mouse move, so the assertion could not pass there no matter how many times it retried. The bug being guarded is structural anyway — an unnamed `.group` on an ancestor makes `group-hover:` fire for every row at once — so assert that invariant against the rendered DOM instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…liation Dropping :reactivate entirely went too far. A no-show corrected to attended left the person with two facilitator affiliations for one organization, implying an engagement that ended and restarted when nothing had lapsed — the row was ended by our own inference from attendance that has since changed. A reopen now applies only where the ending is provably ours: the end date is exactly what reconciling this training writes AND the row carries a reconciliation comment. An admin's ending on the same date, or an earlier training's, still reads as a real lapse and the return is still a new row. Where a reopen applies, no create is proposed, so the two can't both fire. The admin can overrule with "Create a new one instead". ADR-0003 D6a rewritten accordingly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"Will be reactivated" / "Will be created" / "Will be deleted" described an outcome
the admin hasn't chosen yet; the buttons are the choice, so they read as verbs.
Two body-text assertions keyed off those labels and would now match the wrong row
("Delete" is also a label on a deactivate row) — they assert on the row's own radio
ids instead, which don't move when copy does.
Also records in ADR-0003 D6a that a reopen is tied to the training whose attendance
changed: attending a later training while an earlier ending stands is a new
engagement, not a correction.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A registrant still marked Registered after the event is a gap in the record, and the page could only say so — the fix lived on another screen. The day ticks now sit beside the status chip and reuse the onboarding cell, so ticking a day rolls the status forward and repaints both the tick and the chip in place. They render once per registration: someone who linked two organizations gets two rows in that bucket, and the turbo target ids have to stay unique. Also: buttons say "Create new FA" with the abbreviation on hover, the header note no longer overstates what the buttons do before a preview, and an end date nobody reconciled reads as what it is rather than as a reconciliation outcome. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
maebeale
force-pushed
the
maebeale/facilitator-affiliation-reconciliation
branch
from
August 25, 2026 15:34
de2051a to
05d4e3a
Compare
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.
🤖 suggested review level: 5 Inspect 🔬 changes what reconciliation writes to affiliation history, which the grant figures read
Brings facilitator affiliations in line with who actually attended a training, as an admin bulk action with a preview-and-confirm step.
What reconciliation does, by attendance status
attendedno_show,cancelled,incomplete_attendancetransferred_outregisteredtransferred_inisn't a status (it's an FK), so those reconcile on whichever status they hold.Why deleting, not deactivating
A row minted by this training's registration records an assumption: that the person would become a facilitator on the training date. If they didn't attend there's no period to preserve, and a zero-length row is a worse way of saying it never happened.
Anything older — hand-entered, or from an earlier training — is only ever end-dated. Deleting those would erase years of real history and move the organization's program status at every training in between. That distinction is the core of this PR.
Why the dates matter
Program status (New / Ongoing / Reinstated) is anchored to each training's date and feeds the annual grant report. A reconciliation that rewrites an affiliation's dates rewrites those figures retroactively — an organization that read Ongoing at its 2026 training could start reading Reinstated afterwards. The end date is the day before the training, matching
ApplyScenarioEndDating, so a row ending as another starts can't double-count the person.Also here
:has()on two detached radios)inactiveas a deliberate override — an explicitly supplied flag survives a later date edit, and an end date of today or earlier ticks it for youNotes for review
docs/adr/0003-affiliations-as-the-record-of-two-relationships.mdis the fastest way in; D6/D6a–D6d are the reconciliation rules