From 8217c550879b7f0b8aa27162c092a0f3d00ce254 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Fri, 14 Aug 2026 00:42:55 -0400 Subject: [PATCH 01/50] =?UTF-8?q?Add=20affiliation=E2=86=94registration=20?= =?UTF-8?q?link=20and=20event=20reconciled-at=20column?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Groundwork for facilitator-affiliation reconciliation: an ownership FK so reconcile only ever touches rows the registration flow created, and a timestamp on events recording when affiliations were last reconciled. Co-Authored-By: Claude Opus 4.8 (1M context) --- ...044207_add_affiliations_reconciled_at_to_events.rb | 11 +++++++++++ db/schema.rb | 1 + 2 files changed, 12 insertions(+) create mode 100644 db/migrate/20260814044207_add_affiliations_reconciled_at_to_events.rb diff --git a/db/migrate/20260814044207_add_affiliations_reconciled_at_to_events.rb b/db/migrate/20260814044207_add_affiliations_reconciled_at_to_events.rb new file mode 100644 index 0000000000..4f25e87ba2 --- /dev/null +++ b/db/migrate/20260814044207_add_affiliations_reconciled_at_to_events.rb @@ -0,0 +1,11 @@ +class AddAffiliationsReconciledAtToEvents < ActiveRecord::Migration[8.1] + def up + unless column_exists?(:events, :affiliations_reconciled_at) + add_column :events, :affiliations_reconciled_at, :datetime, null: true + end + end + + def down + remove_column :events, :affiliations_reconciled_at, if_exists: true + end +end diff --git a/db/schema.rb b/db/schema.rb index ca68226a75..00e3194cfe 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -543,6 +543,7 @@ create_table "events", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| t.string "abbreviation" + t.datetime "affiliations_reconciled_at" t.boolean "autoshow_cost", default: true, null: false t.boolean "autoshow_date", default: true, null: false t.boolean "autoshow_location", default: true, null: false From 4a07477758cf1a851f791af8066d29e6f33779c6 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Fri, 14 Aug 2026 00:51:34 -0400 Subject: [PATCH 02/50] Add ReconcileFacilitatorAffiliation service Per (person, org): keep the owned facilitator affiliation active iff they have an attended facilitator-training registration for that org; otherwise same-day it (end_date := start_date). Hand-created rows are left alone. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../reconcile_facilitator_affiliation.rb | 83 ++++++++++++ .../reconcile_facilitator_affiliation_spec.rb | 127 ++++++++++++++++++ 2 files changed, 210 insertions(+) create mode 100644 app/services/affiliation_services/reconcile_facilitator_affiliation.rb create mode 100644 spec/services/affiliation_services/reconcile_facilitator_affiliation_spec.rb diff --git a/app/services/affiliation_services/reconcile_facilitator_affiliation.rb b/app/services/affiliation_services/reconcile_facilitator_affiliation.rb new file mode 100644 index 0000000000..60e2bcde89 --- /dev/null +++ b/app/services/affiliation_services/reconcile_facilitator_affiliation.rb @@ -0,0 +1,83 @@ +module AffiliationServices + # Reconciles a person's **owned** facilitator affiliation for one organization + # against whether they actually completed a facilitator training there. + # + # "Owned" means auto-minted by the registration flow (`event_registration_id` + # present) — hand-created / historical rows have no link and are left alone. + # + # A person is an active facilitator of an org iff they have at least one + # `attended` registration to that org from a facilitator-training event. Anyone + # else (no_show, cancelled, incomplete_attendance, still-registered, …) is not, + # so we **same-day** their owned facilitator affiliation — set `end_date` to its + # `start_date`, which the model's `set_inactive_from_dates` turns into + # `inactive: true`. It preserves `start_date` and is reversible: if the person is + # later marked attended, a re-run clears `end_date` and reactivates the row. + # + # The decision is per (person, org) across ALL their training registrations, so + # no-showing one training but attending another for the same org keeps them + # active. + class ReconcileFacilitatorAffiliation + def self.call(person:, organization:) + new(person:, organization:).call + end + + def initialize(person:, organization:) + @person = person + @organization = organization + end + + # Apply the reconciliation. Returns the action taken (:deactivate, :reactivate, + # or :noop). + def call + rows = owned_facilitator_affiliations.to_a + return :noop if rows.empty? + + completed_training? ? reactivate(rows) : deactivate(rows) + end + + # What #call would do, without writing. Returns :deactivate, :reactivate, or :noop. + def plan + rows = owned_facilitator_affiliations.to_a + return :noop if rows.empty? + + if completed_training? + rows.any? { |affiliation| !affiliation.active? } ? :reactivate : :noop + else + rows.any?(&:active?) ? :deactivate : :noop + end + end + + private + + def deactivate(rows) + active = rows.select(&:active?) + return :noop if active.empty? + + active.each { |affiliation| affiliation.update!(end_date: affiliation.start_date || Date.current) } + :deactivate + end + + def reactivate(rows) + ended = rows.reject(&:active?) + return :noop if ended.empty? + + ended.each { |affiliation| affiliation.update!(end_date: nil) } + :reactivate + end + + def owned_facilitator_affiliations + @person.affiliations.facilitators + .where(organization: @organization) + .where.not(event_registration_id: nil) + end + + # Any `attended` registration to this org from a facilitator-training event. + def completed_training? + @person.event_registrations.attended + .joins(:event).where(events: { facilitator_training: true }) + .joins(:event_registration_organizations) + .where(event_registration_organizations: { organization_id: @organization.id }) + .exists? + end + end +end diff --git a/spec/services/affiliation_services/reconcile_facilitator_affiliation_spec.rb b/spec/services/affiliation_services/reconcile_facilitator_affiliation_spec.rb new file mode 100644 index 0000000000..b429fa64f5 --- /dev/null +++ b/spec/services/affiliation_services/reconcile_facilitator_affiliation_spec.rb @@ -0,0 +1,127 @@ +require "rails_helper" + +RSpec.describe AffiliationServices::ReconcileFacilitatorAffiliation do + let(:person) { create(:person) } + let(:organization) { create(:organization) } + + # A facilitator-training registration for `person` linking `organization`. + def training_registration(status:, ended: true) + event = create(:event, *(ended ? [ :ended ] : []), facilitator_training: true) + reg = create(:event_registration, registrant: person, event: event, status: status) + create(:event_registration_organization, event_registration: reg, organization: organization) + reg + end + + # A "Facilitator" affiliation for (person, organization) owned by `registration`. + def owned_facilitator(registration:, start_date: 1.month.ago.to_date) + create(:affiliation, + person: person, + organization: organization, + title: "Facilitator", + start_date: start_date, + event_registration: registration) + end + + describe "deactivation" do + it "same-days the owned facilitator affiliation when the person never attended" do + reg = training_registration(status: "no_show") + affiliation = owned_facilitator(registration: reg) + + described_class.call(person: person, organization: organization) + affiliation.reload + + expect(affiliation.end_date).to eq(affiliation.start_date) + expect(affiliation).to be_inactive + expect(affiliation).not_to be_active + end + + %w[ incomplete_attendance registered cancelled transferred_out ].each do |status| + it "deactivates when the only registration is #{status}" do + reg = training_registration(status: status) + affiliation = owned_facilitator(registration: reg) + + described_class.call(person: person, organization: organization) + + expect(affiliation.reload).not_to be_active + end + end + + it "leaves an unowned (hand-created) facilitator affiliation untouched" do + training_registration(status: "no_show") + hand_created = create(:affiliation, person: person, organization: organization, + title: "Facilitator", start_date: 1.month.ago.to_date) + + described_class.call(person: person, organization: organization) + + expect(hand_created.reload).to be_active + expect(hand_created.end_date).to be_nil + end + end + + describe "keeping / activating" do + it "keeps the affiliation active when the person attended" do + reg = training_registration(status: "attended") + affiliation = owned_facilitator(registration: reg) + + described_class.call(person: person, organization: organization) + + expect(affiliation.reload).to be_active + expect(affiliation.end_date).to be_nil + end + + it "keeps active when the person no-showed one training but attended another for the same org" do + no_show = training_registration(status: "no_show") + affiliation = owned_facilitator(registration: no_show) + training_registration(status: "attended") + + described_class.call(person: person, organization: organization) + + expect(affiliation.reload).to be_active + end + + it "reactivates a previously same-day'd affiliation once the person is marked attended" do + reg = training_registration(status: "attended") + affiliation = owned_facilitator(registration: reg, start_date: 1.month.ago.to_date) + affiliation.update!(end_date: affiliation.start_date) + expect(affiliation.reload).not_to be_active + + described_class.call(person: person, organization: organization) + + expect(affiliation.reload).to be_active + expect(affiliation.end_date).to be_nil + end + end + + describe "idempotence" do + it "is stable across repeated runs" do + reg = training_registration(status: "no_show") + affiliation = owned_facilitator(registration: reg) + + described_class.call(person: person, organization: organization) + first = affiliation.reload.end_date + described_class.call(person: person, organization: organization) + + expect(affiliation.reload.end_date).to eq(first) + end + end + + describe "#plan (dry run)" do + it "reports :deactivate without writing" do + reg = training_registration(status: "no_show") + affiliation = owned_facilitator(registration: reg) + + plan = described_class.new(person: person, organization: organization).plan + + expect(plan).to eq(:deactivate) + expect(affiliation.reload).to be_active + end + + it "reports :noop when there is no owned facilitator affiliation" do + training_registration(status: "no_show") + + plan = described_class.new(person: person, organization: organization).plan + + expect(plan).to eq(:noop) + end + end +end From 2fae6a9353065cd74004c4002e46cab880eca8e4 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Fri, 14 Aug 2026 00:57:03 -0400 Subject: [PATCH 03/50] Add Reconcile affiliations bulk action with preview and opt-out A preview-and-confirm page (under Bulk actions on facilitator trainings) that same-days the owned facilitator affiliation of anyone who didn't complete the training, keeps/reactivates completers, and records when it last ran. Admins can opt individual rows out before applying. Co-Authored-By: Claude Opus 4.8 (1M context) --- AGENTS.md | 2 + .../reconcile_affiliations_controller.rb | 47 +++++++++++ app/models/event.rb | 9 +++ app/policies/event_policy.rb | 4 + .../affiliation_services/reconcile_event.rb | 77 +++++++++++++++++++ .../reconcile_facilitator_affiliation.rb | 17 +++- app/views/events/_bulk_actions_menu.html.erb | 3 + .../reconcile_affiliations/index.html.erb | 63 +++++++++++++++ config/routes.rb | 2 + .../events/reconcile_affiliations_spec.rb | 70 +++++++++++++++++ .../reconcile_facilitator_affiliation_spec.rb | 10 +++ spec/views/page_bg_class_alignment_spec.rb | 1 + 12 files changed, 301 insertions(+), 4 deletions(-) create mode 100644 app/controllers/events/reconcile_affiliations_controller.rb create mode 100644 app/services/affiliation_services/reconcile_event.rb create mode 100644 app/views/events/reconcile_affiliations/index.html.erb create mode 100644 spec/requests/events/reconcile_affiliations_spec.rb diff --git a/AGENTS.md b/AGENTS.md index d13247ff10..2e5b321af0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -264,6 +264,8 @@ action, or `authorize! :workshop, to: :summary?`). - `AffiliationServices::ApplyScenarioEndDating` — What an agreement scenario means for the person's *existing* affiliations, run by the linking core before creation (see ADR-0002): a **new_job** ends their active affiliations at other orgs (job + facilitator alike, sparing the linked org's rows); every other scenario ends nothing — reinstatement reconciles registration-style, creating affiliations only where no active one exists. Ends are dated the day before the agreement takes effect so the fresh affiliations (starting on the submission date) don't overlap a row still counting as active until end of day - `AffiliationServices::CreateFromRegistration` — On registration / org linking, creates a "job affiliation" with the typed title (when present) plus a standing "Facilitator" affiliation, in one transaction. Skips the facilitator one only when the person already has an active-or-pending affiliation titled exactly "Facilitator" with that org (a current one or one dated to a future training); an ended facilitator affiliation gets a fresh second one. Dedupe is by title + org + dates, so a job title like "Lead Facilitator" still gets its own Facilitator affiliation. Accepts an optional `organization_address:` and sets it on every affiliation it creates (the registrant's typed agency address, upserted onto the org); when an affiliation already exists and is skipped, it backfills that address onto the existing one only if it has none (an admin-set address is never overwritten) +- `AffiliationServices::ReconcileFacilitatorAffiliation` — Per `(person, organization)`, brings the person's **owned** facilitator affiliation (created by the registration flow, i.e. `event_registration_id` present) in line with attendance: keeps it active when they have any `attended` facilitator-training registration to that org; otherwise **same-days** it (`end_date := start_date`, which the model turns into `inactive: true`) once its source training has ended. Reactivates a previously same-dayed row when the person is later marked attended. Hand-created (unowned) rows are never touched. `#plan` returns the action (`:deactivate` / `:reactivate` / `:noop`) without writing. +- `AffiliationServices::ReconcileEvent` — Event-level orchestration for the "Reconcile affiliations" bulk action. `#preview` returns the actionable `(person, org)` rows (via `ReconcileFacilitatorAffiliation#plan`) for the confirm page; `#apply(included_keys:)` reconciles the rows the admin kept and stamps the event's `affiliations_reconciled_at`. ### Sectors diff --git a/app/controllers/events/reconcile_affiliations_controller.rb b/app/controllers/events/reconcile_affiliations_controller.rb new file mode 100644 index 0000000000..b368857d4f --- /dev/null +++ b/app/controllers/events/reconcile_affiliations_controller.rb @@ -0,0 +1,47 @@ +module Events + # The "Reconcile affiliations" bulk action: a preview-and-confirm page that + # brings each registrant's owned facilitator affiliation in line with whether + # they actually completed this facilitator training. Post-event it same-days the + # affiliations of non-completers; the admin can opt individual rows out before + # applying. Only facilitator-training events have facilitator affiliations to + # reconcile, so the action is limited to them. + class ReconcileAffiliationsController < ApplicationController + include AhoyTracking + before_action :set_event + before_action :require_facilitator_training + + def index + authorize! @event, to: :reconcile_affiliations? + track_view("events.reconcile_affiliations", { event_id: @event.id }) + + @rows = AffiliationServices::ReconcileEvent.new(@event).preview + @event = @event.decorate + end + + def create + authorize! @event, to: :reconcile_affiliations? + + changed = AffiliationServices::ReconcileEvent.new(@event).apply(included_keys: params[:included]) + redirect_to registrants_event_path(@event), notice: reconcile_notice(changed) + end + + private + + def set_event + @event = Event.find(params[:id]) + end + + def require_facilitator_training + return if @event.facilitator_training? + + redirect_to registrants_event_path(@event), + alert: "Affiliation reconciliation applies to facilitator trainings only." + end + + def reconcile_notice(changed) + return "No affiliations needed reconciling." if changed.zero? + + "Reconciled #{changed} #{'affiliation'.pluralize(changed)}." + end + end +end diff --git a/app/models/event.rb b/app/models/event.rb index 158f076506..6c631de84b 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -186,6 +186,15 @@ def ended? end_date < Time.current end + # A registrant's status changed after affiliations were last reconciled, so the + # reconciliation may be out of date and worth re-running. False when never + # reconciled (nothing to be stale against). + def affiliations_reconciliation_stale? + return false unless affiliations_reconciled_at + + event_registrations.where("event_registrations.updated_at > ?", affiliations_reconciled_at).exists? + end + # Whether the event shows as a full card on the events index. Unpublished # events and events that ended more than a month ago collapse into the compact # archive list instead of taking up a card. diff --git a/app/policies/event_policy.rb b/app/policies/event_policy.rb index 8f1864b268..221eba295a 100644 --- a/app/policies/event_policy.rb +++ b/app/policies/event_policy.rb @@ -120,6 +120,10 @@ def bulk_payments? manage? end + def reconcile_affiliations? + manage? + end + def invoice? manage? end diff --git a/app/services/affiliation_services/reconcile_event.rb b/app/services/affiliation_services/reconcile_event.rb new file mode 100644 index 0000000000..1de7685f43 --- /dev/null +++ b/app/services/affiliation_services/reconcile_event.rb @@ -0,0 +1,77 @@ +module AffiliationServices + # Event-level orchestration for the "Reconcile affiliations" bulk action. Walks + # the event's registrants and the organizations they linked, and reconciles each + # (person, org)'s owned facilitator affiliation via ReconcileFacilitatorAffiliation. + # + # `preview` returns the actionable rows (nothing is written) so the admin can see + # what will change and opt individual rows out. `apply` reconciles the rows the + # admin kept (by key) and stamps the event's `affiliations_reconciled_at`. + class ReconcileEvent + Row = Struct.new(:person, :organization, :affiliation, :action, :key, keyword_init: true) + + def self.key_for(person, organization) + "#{person.id}:#{organization.id}" + end + + def initialize(event) + @event = event + end + + # Actionable rows (:deactivate / :reactivate) for the preview. Never writes. + def preview + pairs.filter_map do |person, organization| + action = ReconcileFacilitatorAffiliation.new(person:, organization:).plan + next if action == :noop + + Row.new( + person:, + organization:, + affiliation: owned_facilitator(person, organization), + action:, + key: self.class.key_for(person, organization) + ) + end + end + + # Reconcile the (person, org) pairs whose keys are in `included_keys`, stamp the + # event, and return the number of pairs actually changed. + def apply(included_keys:) + keys = Array(included_keys).to_set + + changed = pairs.count do |person, organization| + next false unless keys.include?(self.class.key_for(person, organization)) + + ReconcileFacilitatorAffiliation.call(person:, organization:) != :noop + end + + @event.update!(affiliations_reconciled_at: Time.current) + changed + end + + private + + # Distinct (person, organization) pairs from the event's registrants and the + # organizations each linked to their registration. + def pairs + @pairs ||= begin + seen = Set.new + @event.event_registrations.includes(:registrant, :organizations).flat_map do |registration| + registration.organizations.filter_map do |organization| + key = [ registration.registrant_id, organization.id ] + next if seen.include?(key) + + seen << key + [ registration.registrant, organization ] + end + end + end + end + + def owned_facilitator(person, organization) + person.affiliations.facilitators + .where(organization:) + .where.not(event_registration_id: nil) + .first + end + end +end diff --git a/app/services/affiliation_services/reconcile_facilitator_affiliation.rb b/app/services/affiliation_services/reconcile_facilitator_affiliation.rb index 60e2bcde89..db3e086cb6 100644 --- a/app/services/affiliation_services/reconcile_facilitator_affiliation.rb +++ b/app/services/affiliation_services/reconcile_facilitator_affiliation.rb @@ -42,21 +42,30 @@ def plan if completed_training? rows.any? { |affiliation| !affiliation.active? } ? :reactivate : :noop + elsif rows.any? { |affiliation| affiliation.active? && source_training_ended?(affiliation) } + :deactivate else - rows.any?(&:active?) ? :deactivate : :noop + :noop end end private def deactivate(rows) - active = rows.select(&:active?) - return :noop if active.empty? + # Only same-day affiliations whose source training has actually ended. A row + # tied to a still-upcoming training is a legitimate assumptive/upcoming + # affiliation — leave it alone until that training is over. + ended = rows.select { |affiliation| affiliation.active? && source_training_ended?(affiliation) } + return :noop if ended.empty? - active.each { |affiliation| affiliation.update!(end_date: affiliation.start_date || Date.current) } + ended.each { |affiliation| affiliation.update!(end_date: affiliation.start_date || Date.current) } :deactivate end + def source_training_ended?(affiliation) + affiliation.event_registration&.event&.ended? + end + def reactivate(rows) ended = rows.reject(&:active?) return :noop if ended.empty? diff --git a/app/views/events/_bulk_actions_menu.html.erb b/app/views/events/_bulk_actions_menu.html.erb index 72c7dac7f6..4d61faffd1 100644 --- a/app/views/events/_bulk_actions_menu.html.erb +++ b/app/views/events/_bulk_actions_menu.html.erb @@ -24,6 +24,9 @@ <% else %> <%= link_to "Sign-ins", attendance_event_path(@event, return_to: "registrants"), class: item_class %> <% end %> + <% if @event.facilitator_training? %> + <%= link_to "Reconcile affiliations", reconcile_affiliations_event_path(@event), class: item_class %> + <% end %> <%= link_to registrants_event_path(@event, format: :csv), class: item_class, data: { turbo_frame: "_top" } do %> Download CSV <% end %> diff --git a/app/views/events/reconcile_affiliations/index.html.erb b/app/views/events/reconcile_affiliations/index.html.erb new file mode 100644 index 0000000000..a6b56c659d --- /dev/null +++ b/app/views/events/reconcile_affiliations/index.html.erb @@ -0,0 +1,63 @@ +<% content_for(:page_title, "Reconcile affiliations — #{@event.title}") %> +<% content_for(:page_bg_class, "admin-or-owner bg-blue-100") %> +
+
+ <%= link_to "← Registrants", registrants_event_path(@event), class: "text-sm text-gray-500 hover:text-gray-700" %> + <%= render "events/subnav", event: @event, current: :registrants %> +
+ +

Reconcile affiliations

+ +
+

+ Facilitator affiliations are created optimistically when someone registers for a training. This step brings + them in line with who actually attended: anyone who didn't complete the training has their + auto-created facilitator affiliation same-dayed (its end date is set to its start date, so it + no longer counts as active). Someone later marked attended is reactivated on the next run. +

+

+ Only affiliations this app created from a registration are touched — hand-entered affiliations are always left + alone. Uncheck a row to spare it this time. +

+ <% if @event.affiliations_reconciled_at %> +

Last reconciled <%= @event.affiliations_reconciled_at.to_fs(:long) %>.

+ <% end %> + <% if @event.affiliations_reconciliation_stale? %> +

Attendance has changed since the last reconciliation — re-run to bring affiliations up to date.

+ <% end %> +
+ + <% if @rows.empty? %> +
+ Nothing to reconcile — every facilitator affiliation already matches its attendance. +
+ <% else %> + <%= form_with url: reconcile_affiliations_event_path(@event), method: :post do %> +
+ <% @rows.each do |row| %> + + <% end %> +
+ +
+ <%= link_to "Cancel", registrants_event_path(@event), class: "text-sm text-gray-500 hover:text-gray-700" %> + <%= submit_tag "Reconcile affiliations", class: "btn btn-primary" %> +
+ <% end %> + <% end %> +
diff --git a/config/routes.rb b/config/routes.rb index 2b096cbe06..0208cc8d90 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -214,6 +214,8 @@ get :recipients post :feature_recipient_shoutout get :bulk_payments, to: "events/bulk_payments#index" + get :reconcile_affiliations, to: "events/reconcile_affiliations#index" + post :reconcile_affiliations, to: "events/reconcile_affiliations#create" get :preview_reminder patch :preview post :copy_registration_form diff --git a/spec/requests/events/reconcile_affiliations_spec.rb b/spec/requests/events/reconcile_affiliations_spec.rb new file mode 100644 index 0000000000..b7b80b3613 --- /dev/null +++ b/spec/requests/events/reconcile_affiliations_spec.rb @@ -0,0 +1,70 @@ +require "rails_helper" + +RSpec.describe "Events::ReconcileAffiliations", type: :request do + let(:admin) { create(:user, :admin) } + let(:organization) { create(:organization) } + let(:event) { create(:event, :ended, facilitator_training: true) } + + # A registrant of `event` who linked `organization`, with an owned facilitator + # affiliation created (as the registration flow would). + def registrant_with_affiliation(status:) + person = create(:person) + reg = create(:event_registration, event: event, registrant: person, status: status) + create(:event_registration_organization, event_registration: reg, organization: organization) + affiliation = create(:affiliation, person: person, organization: organization, + title: "Facilitator", start_date: 1.month.ago.to_date, + event_registration: reg) + [ person, affiliation ] + end + + before { sign_in admin } + + describe "GET index" do + it "previews the no-show as a deactivation, checked by default" do + person, _affiliation = registrant_with_affiliation(status: "no_show") + + get reconcile_affiliations_event_path(event) + + expect(response).to have_http_status(:ok) + expect(response.body).to include(person.name) + expect(response.body).to include("Will be deactivated") + end + + it "redirects for a non-training event" do + non_training = create(:event, :ended, facilitator_training: false) + + get reconcile_affiliations_event_path(non_training) + + expect(response).to redirect_to(registrants_event_path(non_training)) + end + + it "denies a non-admin" do + sign_in create(:user) + + get reconcile_affiliations_event_path(event) + + expect(response).not_to have_http_status(:ok) + end + end + + describe "POST create" do + it "deactivates the included non-completer and stamps the event" do + _person, affiliation = registrant_with_affiliation(status: "no_show") + key = AffiliationServices::ReconcileEvent.key_for(affiliation.person, organization) + + post reconcile_affiliations_event_path(event), params: { included: [ key ] } + + expect(response).to redirect_to(registrants_event_path(event)) + expect(affiliation.reload).not_to be_active + expect(event.reload.affiliations_reconciled_at).to be_present + end + + it "spares an opted-out row" do + _person, affiliation = registrant_with_affiliation(status: "no_show") + + post reconcile_affiliations_event_path(event), params: { included: [] } + + expect(affiliation.reload).to be_active + end + end +end diff --git a/spec/services/affiliation_services/reconcile_facilitator_affiliation_spec.rb b/spec/services/affiliation_services/reconcile_facilitator_affiliation_spec.rb index b429fa64f5..d910e4bbb6 100644 --- a/spec/services/affiliation_services/reconcile_facilitator_affiliation_spec.rb +++ b/spec/services/affiliation_services/reconcile_facilitator_affiliation_spec.rb @@ -46,6 +46,16 @@ def owned_facilitator(registration:, start_date: 1.month.ago.to_date) end end + it "leaves an assumptive affiliation alone while its training is still upcoming" do + reg = training_registration(status: "registered", ended: false) + affiliation = owned_facilitator(registration: reg, start_date: Date.current) + + described_class.call(person: person, organization: organization) + + expect(affiliation.reload).to be_active + expect(affiliation.end_date).to be_nil + end + it "leaves an unowned (hand-created) facilitator affiliation untouched" do training_registration(status: "no_show") hand_created = create(:affiliation, person: person, organization: organization, diff --git a/spec/views/page_bg_class_alignment_spec.rb b/spec/views/page_bg_class_alignment_spec.rb index 6c4cd5ca63..a3f5941898 100644 --- a/spec/views/page_bg_class_alignment_spec.rb +++ b/spec/views/page_bg_class_alignment_spec.rb @@ -129,6 +129,7 @@ "app/views/events/signins.html.erb" => "admin-or-owner bg-blue-100", "app/views/events/sample_ticket.html.erb" => "admin-or-owner bg-blue-100", "app/views/events/bulk_payments/index.html.erb" => "admin-or-owner bg-blue-100", + "app/views/events/reconcile_affiliations/index.html.erb" => "admin-or-owner bg-blue-100", "app/views/events/edit_staff.html.erb" => "admin-or-owner bg-blue-100", "app/views/events/recipients.html.erb" => "admin-or-owner bg-blue-100", "app/views/events/registrants.html.erb" => "admin-or-owner bg-blue-100", From 059ebd3563d1906abab980733a239275ec605d40 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Fri, 14 Aug 2026 01:15:47 -0400 Subject: [PATCH 04/50] Date facilitator affiliation to the training day; heal missing affiliations on reconcile Start the created facilitator affiliation on the actual training date rather than the first of its month. Extend the Reconcile affiliations action to also create missing facilitator affiliations (pre-event for anyone, post-event for attendees), shown as opt-out-able 'Will be created' rows alongside the deactivations. Co-Authored-By: Claude Opus 4.8 (1M context) --- AGENTS.md | 2 +- .../affiliation_services/reconcile_event.rb | 91 ++++++++++++++----- .../reconcile_facilitator_affiliation.rb | 19 ++-- .../reconcile_affiliations/index.html.erb | 15 ++- .../events/reconcile_affiliations_spec.rb | 23 +++++ 5 files changed, 112 insertions(+), 38 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2e5b321af0..7a2337b125 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -265,7 +265,7 @@ action, or `authorize! :workshop, to: :summary?`). - `AffiliationServices::ApplyScenarioEndDating` — What an agreement scenario means for the person's *existing* affiliations, run by the linking core before creation (see ADR-0002): a **new_job** ends their active affiliations at other orgs (job + facilitator alike, sparing the linked org's rows); every other scenario ends nothing — reinstatement reconciles registration-style, creating affiliations only where no active one exists. Ends are dated the day before the agreement takes effect so the fresh affiliations (starting on the submission date) don't overlap a row still counting as active until end of day - `AffiliationServices::CreateFromRegistration` — On registration / org linking, creates a "job affiliation" with the typed title (when present) plus a standing "Facilitator" affiliation, in one transaction. Skips the facilitator one only when the person already has an active-or-pending affiliation titled exactly "Facilitator" with that org (a current one or one dated to a future training); an ended facilitator affiliation gets a fresh second one. Dedupe is by title + org + dates, so a job title like "Lead Facilitator" still gets its own Facilitator affiliation. Accepts an optional `organization_address:` and sets it on every affiliation it creates (the registrant's typed agency address, upserted onto the org); when an affiliation already exists and is skipped, it backfills that address onto the existing one only if it has none (an admin-set address is never overwritten) - `AffiliationServices::ReconcileFacilitatorAffiliation` — Per `(person, organization)`, brings the person's **owned** facilitator affiliation (created by the registration flow, i.e. `event_registration_id` present) in line with attendance: keeps it active when they have any `attended` facilitator-training registration to that org; otherwise **same-days** it (`end_date := start_date`, which the model turns into `inactive: true`) once its source training has ended. Reactivates a previously same-dayed row when the person is later marked attended. Hand-created (unowned) rows are never touched. `#plan` returns the action (`:deactivate` / `:reactivate` / `:noop`) without writing. -- `AffiliationServices::ReconcileEvent` — Event-level orchestration for the "Reconcile affiliations" bulk action. `#preview` returns the actionable `(person, org)` rows (via `ReconcileFacilitatorAffiliation#plan`) for the confirm page; `#apply(included_keys:)` reconciles the rows the admin kept and stamps the event's `affiliations_reconciled_at`. +- `AffiliationServices::ReconcileEvent` — Event-level orchestration for the "Reconcile affiliations" bulk action. Per `(person, org)` it decides `:create` (heal a missing facilitator affiliation — pre-event for anyone, post-event only for attendees), `:deactivate`, or `:reactivate`. `#preview` returns the actionable rows for the confirm page; `#apply(included_keys:)` performs the rows the admin kept (creating via `CreateFromRegistration`, otherwise via `ReconcileFacilitatorAffiliation`) and stamps the event's `affiliations_reconciled_at`. ### Sectors diff --git a/app/services/affiliation_services/reconcile_event.rb b/app/services/affiliation_services/reconcile_event.rb index 1de7685f43..1c95e8f2ee 100644 --- a/app/services/affiliation_services/reconcile_event.rb +++ b/app/services/affiliation_services/reconcile_event.rb @@ -1,13 +1,19 @@ module AffiliationServices # Event-level orchestration for the "Reconcile affiliations" bulk action. Walks - # the event's registrants and the organizations they linked, and reconciles each - # (person, org)'s owned facilitator affiliation via ReconcileFacilitatorAffiliation. + # the event's registrants and the organizations they linked, and for each + # (person, org) works out what should happen to their facilitator affiliation: # - # `preview` returns the actionable rows (nothing is written) so the admin can see - # what will change and opt individual rows out. `apply` reconciles the rows the - # admin kept (by key) and stamps the event's `affiliations_reconciled_at`. + # :create — no facilitator affiliation exists yet but one should (heal a + # missing affiliation): pre-event for any registrant, post-event + # only for those who attended. + # :deactivate — an owned affiliation whose (ended) training they didn't complete. + # :reactivate — an owned affiliation same-dayed earlier, now attended. + # + # `preview` returns the actionable rows without writing so the admin can see them + # and opt individual rows out; `apply(included_keys:)` performs the kept rows and + # stamps the event's `affiliations_reconciled_at`. class ReconcileEvent - Row = Struct.new(:person, :organization, :affiliation, :action, :key, keyword_init: true) + Row = Struct.new(:person, :organization, :registration, :affiliation, :action, :key, keyword_init: true) def self.key_for(person, organization) "#{person.id}:#{organization.id}" @@ -17,15 +23,34 @@ def initialize(event) @event = event end - # Actionable rows (:deactivate / :reactivate) for the preview. Never writes. def preview - pairs.filter_map do |person, organization| - action = ReconcileFacilitatorAffiliation.new(person:, organization:).plan + rows + end + + # Apply the rows whose keys are in `included_keys`, stamp the event, and return + # the number of pairs actually changed. + def apply(included_keys:) + keys = Array(included_keys).to_set + + changed = rows.count do |row| + keys.include?(row.key) && apply_row(row) + end + + @event.update!(affiliations_reconciled_at: Time.current) + changed + end + + private + + def rows + @rows ||= pairs.filter_map do |person, organization, registration| + action = action_for(person, organization) next if action == :noop Row.new( person:, organization:, + registration:, affiliation: owned_facilitator(person, organization), action:, key: self.class.key_for(person, organization) @@ -33,25 +58,45 @@ def preview end end - # Reconcile the (person, org) pairs whose keys are in `included_keys`, stamp the - # event, and return the number of pairs actually changed. - def apply(included_keys:) - keys = Array(included_keys).to_set + def action_for(person, organization) + reconcile = ReconcileFacilitatorAffiliation.new(person:, organization:).plan + return reconcile unless reconcile == :noop - changed = pairs.count do |person, organization| - next false unless keys.include?(self.class.key_for(person, organization)) + create_needed?(person, organization) ? :create : :noop + end - ReconcileFacilitatorAffiliation.call(person:, organization:) != :noop - end + def apply_row(row) + return apply_create(row) if row.action == :create - @event.update!(affiliations_reconciled_at: Time.current) - changed + ReconcileFacilitatorAffiliation.call(person: row.person, organization: row.organization) != :noop end - private + def apply_create(row) + AffiliationServices::CreateFromRegistration.call( + person: row.person, + organization: row.organization, + facilitator_training: true, + training_date: @event.start_date, + event_registration: row.registration + ) + true + end + + # A facilitator affiliation should exist but doesn't. Skip when an owned one + # already exists (reconcile handles it — including a deliberately same-dayed + # no-show we must not resurrect) or when a hand-created active-or-pending one + # already covers it. Otherwise create it pre-event for anyone, post-event only + # for those who attended. + def create_needed?(person, organization) + facilitators = person.affiliations.facilitators.where(organization:) + return false if facilitators.where.not(event_registration_id: nil).exists? + return false if facilitators.active_or_pending.exists? + + !@event.ended? || ReconcileFacilitatorAffiliation.new(person:, organization:).completed_training? + end - # Distinct (person, organization) pairs from the event's registrants and the - # organizations each linked to their registration. + # Distinct (person, organization, registration) triples from the event's + # registrants and the organizations each linked to their registration. def pairs @pairs ||= begin seen = Set.new @@ -61,7 +106,7 @@ def pairs next if seen.include?(key) seen << key - [ registration.registrant, organization ] + [ registration.registrant, organization, registration ] end end end diff --git a/app/services/affiliation_services/reconcile_facilitator_affiliation.rb b/app/services/affiliation_services/reconcile_facilitator_affiliation.rb index db3e086cb6..acc3bf54b7 100644 --- a/app/services/affiliation_services/reconcile_facilitator_affiliation.rb +++ b/app/services/affiliation_services/reconcile_facilitator_affiliation.rb @@ -49,6 +49,16 @@ def plan end end + # Whether the person has any `attended` registration to this org from a + # facilitator-training event — i.e. actually became a facilitator there. + def completed_training? + @person.event_registrations.attended + .joins(:event).where(events: { facilitator_training: true }) + .joins(:event_registration_organizations) + .where(event_registration_organizations: { organization_id: @organization.id }) + .exists? + end + private def deactivate(rows) @@ -79,14 +89,5 @@ def owned_facilitator_affiliations .where(organization: @organization) .where.not(event_registration_id: nil) end - - # Any `attended` registration to this org from a facilitator-training event. - def completed_training? - @person.event_registrations.attended - .joins(:event).where(events: { facilitator_training: true }) - .joins(:event_registration_organizations) - .where(event_registration_organizations: { organization_id: @organization.id }) - .exists? - end end end diff --git a/app/views/events/reconcile_affiliations/index.html.erb b/app/views/events/reconcile_affiliations/index.html.erb index a6b56c659d..4bcdf4441b 100644 --- a/app/views/events/reconcile_affiliations/index.html.erb +++ b/app/views/events/reconcile_affiliations/index.html.erb @@ -10,10 +10,10 @@

- Facilitator affiliations are created optimistically when someone registers for a training. This step brings - them in line with who actually attended: anyone who didn't complete the training has their - auto-created facilitator affiliation same-dayed (its end date is set to its start date, so it - no longer counts as active). Someone later marked attended is reactivated on the next run. + This step brings facilitator affiliations in line with who registered and attended. Before the training it + creates any missing facilitator affiliations for linked organizations. After the training it + same-days the affiliation of anyone who didn't attend (its end date is set to its start + date, so it no longer counts as active), and reactivates anyone later marked attended.

Only affiliations this app created from a registration are touched — hand-entered affiliations are always left @@ -41,10 +41,15 @@ <%= row.person.name %> — <%= row.organization.name %> - <% if row.action == :deactivate %> + <% case row.action %> + <% when :deactivate %> Will be deactivated + <% when :create %> + + Will be created + <% else %> Will be reactivated diff --git a/spec/requests/events/reconcile_affiliations_spec.rb b/spec/requests/events/reconcile_affiliations_spec.rb index b7b80b3613..3d5b29e5ad 100644 --- a/spec/requests/events/reconcile_affiliations_spec.rb +++ b/spec/requests/events/reconcile_affiliations_spec.rb @@ -30,6 +30,17 @@ def registrant_with_affiliation(status:) expect(response.body).to include("Will be deactivated") end + it "previews a missing affiliation as a creation before the event" do + upcoming = create(:event, facilitator_training: true, start_date: 3.days.from_now, end_date: 5.days.from_now) + person = create(:person) + reg = create(:event_registration, event: upcoming, registrant: person, status: "registered") + create(:event_registration_organization, event_registration: reg, organization: organization) + + get reconcile_affiliations_event_path(upcoming) + + expect(response.body).to include("Will be created") + end + it "redirects for a non-training event" do non_training = create(:event, :ended, facilitator_training: false) @@ -66,5 +77,17 @@ def registrant_with_affiliation(status:) expect(affiliation.reload).to be_active end + + it "creates a missing affiliation before the event when included" do + upcoming = create(:event, facilitator_training: true, start_date: 3.days.from_now, end_date: 5.days.from_now) + person = create(:person) + reg = create(:event_registration, event: upcoming, registrant: person, status: "registered") + create(:event_registration_organization, event_registration: reg, organization: organization) + key = AffiliationServices::ReconcileEvent.key_for(person, organization) + + expect { + post reconcile_affiliations_event_path(upcoming), params: { included: [ key ] } + }.to change { person.affiliations.facilitators.where(organization: organization).count }.by(1) + end end end From 8e6e59cd63da0d58e5b1173a90c0f43611a2948d Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Fri, 14 Aug 2026 07:49:12 -0400 Subject: [PATCH 05/50] Reconcile non-training facilitator affiliations and offer delete-instead On a non-training event, the Reconcile affiliations action now deletes facilitator affiliations that were auto-created off it (job affiliations are left alone), shown as opt-out-able 'Will be deleted' rows. Same-day rows also gain a per-row 'Delete instead' checkbox. The action is now available on every event, not just trainings. Co-Authored-By: Claude Opus 4.8 (1M context) --- AGENTS.md | 2 +- .../reconcile_affiliations_controller.rb | 23 ++-- .../affiliation_services/reconcile_event.rb | 100 ++++++++++++------ .../reconcile_facilitator_affiliation.rb | 20 ++-- app/views/events/_bulk_actions_menu.html.erb | 4 +- .../reconcile_affiliations/index.html.erb | 37 +++++-- .../events/reconcile_affiliations_spec.rb | 35 +++++- 7 files changed, 150 insertions(+), 71 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 7a2337b125..2f6b6802dd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -265,7 +265,7 @@ action, or `authorize! :workshop, to: :summary?`). - `AffiliationServices::ApplyScenarioEndDating` — What an agreement scenario means for the person's *existing* affiliations, run by the linking core before creation (see ADR-0002): a **new_job** ends their active affiliations at other orgs (job + facilitator alike, sparing the linked org's rows); every other scenario ends nothing — reinstatement reconciles registration-style, creating affiliations only where no active one exists. Ends are dated the day before the agreement takes effect so the fresh affiliations (starting on the submission date) don't overlap a row still counting as active until end of day - `AffiliationServices::CreateFromRegistration` — On registration / org linking, creates a "job affiliation" with the typed title (when present) plus a standing "Facilitator" affiliation, in one transaction. Skips the facilitator one only when the person already has an active-or-pending affiliation titled exactly "Facilitator" with that org (a current one or one dated to a future training); an ended facilitator affiliation gets a fresh second one. Dedupe is by title + org + dates, so a job title like "Lead Facilitator" still gets its own Facilitator affiliation. Accepts an optional `organization_address:` and sets it on every affiliation it creates (the registrant's typed agency address, upserted onto the org); when an affiliation already exists and is skipped, it backfills that address onto the existing one only if it has none (an admin-set address is never overwritten) - `AffiliationServices::ReconcileFacilitatorAffiliation` — Per `(person, organization)`, brings the person's **owned** facilitator affiliation (created by the registration flow, i.e. `event_registration_id` present) in line with attendance: keeps it active when they have any `attended` facilitator-training registration to that org; otherwise **same-days** it (`end_date := start_date`, which the model turns into `inactive: true`) once its source training has ended. Reactivates a previously same-dayed row when the person is later marked attended. Hand-created (unowned) rows are never touched. `#plan` returns the action (`:deactivate` / `:reactivate` / `:noop`) without writing. -- `AffiliationServices::ReconcileEvent` — Event-level orchestration for the "Reconcile affiliations" bulk action. Per `(person, org)` it decides `:create` (heal a missing facilitator affiliation — pre-event for anyone, post-event only for attendees), `:deactivate`, or `:reactivate`. `#preview` returns the actionable rows for the confirm page; `#apply(included_keys:)` performs the rows the admin kept (creating via `CreateFromRegistration`, otherwise via `ReconcileFacilitatorAffiliation`) and stamps the event's `affiliations_reconciled_at`. +- `AffiliationServices::ReconcileEvent` — Event-level orchestration for the "Reconcile affiliations" bulk action. Per `(person, org)` it decides, for facilitator trainings, `:create` (heal a missing facilitator affiliation — pre-event for anyone, post-event only for attendees), `:deactivate`, or `:reactivate`; for non-training events, `:delete` (remove a facilitator affiliation auto-created off this event, leaving job affiliations). `#preview` returns the actionable rows for the confirm page; `#apply(included_keys:, delete_keys:)` performs the rows the admin kept — creating via `CreateFromRegistration`, deactivating/reactivating via `ReconcileFacilitatorAffiliation`, or deleting (a `:deactivate` row whose key is in `delete_keys` is deleted instead of same-dayed) — and stamps the event's `affiliations_reconciled_at`. ### Sectors diff --git a/app/controllers/events/reconcile_affiliations_controller.rb b/app/controllers/events/reconcile_affiliations_controller.rb index b368857d4f..f5d1d33913 100644 --- a/app/controllers/events/reconcile_affiliations_controller.rb +++ b/app/controllers/events/reconcile_affiliations_controller.rb @@ -1,14 +1,13 @@ module Events # The "Reconcile affiliations" bulk action: a preview-and-confirm page that - # brings each registrant's owned facilitator affiliation in line with whether - # they actually completed this facilitator training. Post-event it same-days the - # affiliations of non-completers; the admin can opt individual rows out before - # applying. Only facilitator-training events have facilitator affiliations to - # reconcile, so the action is limited to them. + # brings each registrant's owned facilitator affiliation in line with reality. + # For a facilitator training it creates missing affiliations, same-days + # non-completers, and reactivates late attendees; for a non-training event it + # removes facilitator affiliations that were auto-created off it. The admin can + # opt individual rows out (and, for same-day rows, delete instead) before applying. class ReconcileAffiliationsController < ApplicationController include AhoyTracking before_action :set_event - before_action :require_facilitator_training def index authorize! @event, to: :reconcile_affiliations? @@ -21,7 +20,10 @@ def index def create authorize! @event, to: :reconcile_affiliations? - changed = AffiliationServices::ReconcileEvent.new(@event).apply(included_keys: params[:included]) + changed = AffiliationServices::ReconcileEvent.new(@event).apply( + included_keys: params[:included] || [], + delete_keys: params[:delete] || [] + ) redirect_to registrants_event_path(@event), notice: reconcile_notice(changed) end @@ -31,13 +33,6 @@ def set_event @event = Event.find(params[:id]) end - def require_facilitator_training - return if @event.facilitator_training? - - redirect_to registrants_event_path(@event), - alert: "Affiliation reconciliation applies to facilitator trainings only." - end - def reconcile_notice(changed) return "No affiliations needed reconciling." if changed.zero? diff --git a/app/services/affiliation_services/reconcile_event.rb b/app/services/affiliation_services/reconcile_event.rb index 1c95e8f2ee..aa6bc2ee6a 100644 --- a/app/services/affiliation_services/reconcile_event.rb +++ b/app/services/affiliation_services/reconcile_event.rb @@ -1,17 +1,22 @@ module AffiliationServices # Event-level orchestration for the "Reconcile affiliations" bulk action. Walks - # the event's registrants and the organizations they linked, and for each - # (person, org) works out what should happen to their facilitator affiliation: + # the event's registrants and the organizations they linked, and per (person, + # org) works out what should happen to their **owned** facilitator affiliation + # (job affiliations are never touched): # - # :create — no facilitator affiliation exists yet but one should (heal a - # missing affiliation): pre-event for any registrant, post-event - # only for those who attended. - # :deactivate — an owned affiliation whose (ended) training they didn't complete. - # :reactivate — an owned affiliation same-dayed earlier, now attended. + # :create — facilitator training, none exists yet but one should (pre-event + # for anyone, post-event only for attendees). + # :deactivate — facilitator training, an owned affiliation whose (ended) + # training they didn't complete. The admin may choose to delete + # it instead of same-daying it (see `delete_keys`). + # :reactivate — facilitator training, an owned affiliation same-dayed earlier, + # now attended. + # :delete — NOT a facilitator training: an owned facilitator affiliation was + # auto-created off this event and shouldn't exist, so remove it. # # `preview` returns the actionable rows without writing so the admin can see them - # and opt individual rows out; `apply(included_keys:)` performs the kept rows and - # stamps the event's `affiliations_reconciled_at`. + # and opt individual rows out; `apply` performs the kept rows and stamps the + # event's `affiliations_reconciled_at`. class ReconcileEvent Row = Struct.new(:person, :organization, :registration, :affiliation, :action, :key, keyword_init: true) @@ -27,13 +32,15 @@ def preview rows end - # Apply the rows whose keys are in `included_keys`, stamp the event, and return - # the number of pairs actually changed. - def apply(included_keys:) - keys = Array(included_keys).to_set + # Apply the rows whose keys are in `included_keys`. For :deactivate rows whose + # key is also in `delete_keys`, delete the affiliation instead of same-daying + # it. Stamps the event and returns the number of pairs actually changed. + def apply(included_keys:, delete_keys: []) + included = Array(included_keys).to_set + delete_instead = Array(delete_keys).to_set changed = rows.count do |row| - keys.include?(row.key) && apply_row(row) + included.include?(row.key) && perform(row, delete_instead: delete_instead.include?(row.key)) end @event.update!(affiliations_reconciled_at: Time.current) @@ -43,32 +50,50 @@ def apply(included_keys:) private def rows - @rows ||= pairs.filter_map do |person, organization, registration| - action = action_for(person, organization) - next if action == :noop - - Row.new( - person:, - organization:, - registration:, - affiliation: owned_facilitator(person, organization), - action:, - key: self.class.key_for(person, organization) - ) + @rows ||= pairs.filter_map { |person, organization, registration| build_row(person, organization, registration) } + end + + def build_row(person, organization, registration) + if @event.facilitator_training? + action = training_action(person, organization) + return if action == :noop + + affiliation = action == :create ? nil : owned_facilitator(person, organization) + else + affiliation = owned_facilitator_from_event(person, organization) + return if affiliation.nil? + + action = :delete end + + Row.new(person:, organization:, registration:, affiliation:, action:, key: self.class.key_for(person, organization)) end - def action_for(person, organization) + def training_action(person, organization) reconcile = ReconcileFacilitatorAffiliation.new(person:, organization:).plan return reconcile unless reconcile == :noop create_needed?(person, organization) ? :create : :noop end - def apply_row(row) - return apply_create(row) if row.action == :create - - ReconcileFacilitatorAffiliation.call(person: row.person, organization: row.organization) != :noop + def perform(row, delete_instead:) + case row.action + when :create + apply_create(row) + true + when :delete + row.affiliation.destroy! + true + when :deactivate + service = ReconcileFacilitatorAffiliation.new(person: row.person, organization: row.organization) + targets = service.deactivatable_affiliations + return false if targets.empty? + + delete_instead ? targets.each(&:destroy!) : service.call + true + else # :reactivate + ReconcileFacilitatorAffiliation.call(person: row.person, organization: row.organization) != :noop + end end def apply_create(row) @@ -79,7 +104,6 @@ def apply_create(row) training_date: @event.start_date, event_registration: row.registration ) - true end # A facilitator affiliation should exist but doesn't. Skip when an owned one @@ -118,5 +142,17 @@ def owned_facilitator(person, organization) .where.not(event_registration_id: nil) .first end + + # An owned facilitator affiliation that was auto-created off *this* (non-training) + # event — the row a non-training reconcile removes. Hand-created rows (no link) + # and affiliations from other events are left alone. + def owned_facilitator_from_event(person, organization) + person.affiliations.facilitators + .where(organization:) + .where.not(event_registration_id: nil) + .joins(:event_registration) + .where(event_registrations: { event_id: @event.id }) + .first + end end end diff --git a/app/services/affiliation_services/reconcile_facilitator_affiliation.rb b/app/services/affiliation_services/reconcile_facilitator_affiliation.rb index acc3bf54b7..b4c837651a 100644 --- a/app/services/affiliation_services/reconcile_facilitator_affiliation.rb +++ b/app/services/affiliation_services/reconcile_facilitator_affiliation.rb @@ -42,7 +42,7 @@ def plan if completed_training? rows.any? { |affiliation| !affiliation.active? } ? :reactivate : :noop - elsif rows.any? { |affiliation| affiliation.active? && source_training_ended?(affiliation) } + elsif deactivatable_affiliations.any? :deactivate else :noop @@ -59,16 +59,20 @@ def completed_training? .exists? end + # The owned facilitator affiliations #call would same-day: active, and tied to a + # training that has already ended. Exposed so the bulk action can offer "delete + # instead of same-day" over the exact same set. + def deactivatable_affiliations + owned_facilitator_affiliations.select { |affiliation| affiliation.active? && source_training_ended?(affiliation) } + end + private - def deactivate(rows) - # Only same-day affiliations whose source training has actually ended. A row - # tied to a still-upcoming training is a legitimate assumptive/upcoming - # affiliation — leave it alone until that training is over. - ended = rows.select { |affiliation| affiliation.active? && source_training_ended?(affiliation) } - return :noop if ended.empty? + def deactivate(_rows) + targets = deactivatable_affiliations + return :noop if targets.empty? - ended.each { |affiliation| affiliation.update!(end_date: affiliation.start_date || Date.current) } + targets.each { |affiliation| affiliation.update!(end_date: affiliation.start_date || Date.current) } :deactivate end diff --git a/app/views/events/_bulk_actions_menu.html.erb b/app/views/events/_bulk_actions_menu.html.erb index 4d61faffd1..8a598d2d1b 100644 --- a/app/views/events/_bulk_actions_menu.html.erb +++ b/app/views/events/_bulk_actions_menu.html.erb @@ -24,9 +24,7 @@ <% else %> <%= link_to "Sign-ins", attendance_event_path(@event, return_to: "registrants"), class: item_class %> <% end %> - <% if @event.facilitator_training? %> - <%= link_to "Reconcile affiliations", reconcile_affiliations_event_path(@event), class: item_class %> - <% end %> + <%= link_to "Reconcile affiliations", reconcile_affiliations_event_path(@event), class: item_class %> <%= link_to registrants_event_path(@event, format: :csv), class: item_class, data: { turbo_frame: "_top" } do %> Download CSV <% end %> diff --git a/app/views/events/reconcile_affiliations/index.html.erb b/app/views/events/reconcile_affiliations/index.html.erb index 4bcdf4441b..b6ca9f0b72 100644 --- a/app/views/events/reconcile_affiliations/index.html.erb +++ b/app/views/events/reconcile_affiliations/index.html.erb @@ -9,12 +9,19 @@

Reconcile affiliations

-

- This step brings facilitator affiliations in line with who registered and attended. Before the training it - creates any missing facilitator affiliations for linked organizations. After the training it - same-days the affiliation of anyone who didn't attend (its end date is set to its start - date, so it no longer counts as active), and reactivates anyone later marked attended. -

+ <% if @event.facilitator_training? %> +

+ This step brings facilitator affiliations in line with who registered and attended. Before the training it + creates any missing facilitator affiliations for linked organizations. After the training it + same-days the affiliation of anyone who didn't attend (its end date is set to its start + date, so it no longer counts as active), and reactivates anyone later marked attended. +

+ <% else %> +

+ This event isn't a facilitator training, so any facilitator affiliation auto-created from it shouldn't exist. + This deletes those. Job affiliations are left untouched. +

+ <% end %>

Only affiliations this app created from a registration are touched — hand-entered affiliations are always left alone. Uncheck a row to spare it this time. @@ -35,17 +42,25 @@ <%= form_with url: reconcile_affiliations_event_path(@event), method: :post do %>

<% @rows.each do |row| %> -
diff --git a/spec/requests/events/reconcile_affiliations_spec.rb b/spec/requests/events/reconcile_affiliations_spec.rb index 3d5b29e5ad..7b68515716 100644 --- a/spec/requests/events/reconcile_affiliations_spec.rb +++ b/spec/requests/events/reconcile_affiliations_spec.rb @@ -41,12 +41,17 @@ def registrant_with_affiliation(status:) expect(response.body).to include("Will be created") end - it "redirects for a non-training event" do + it "previews a facilitator affiliation on a non-training event as a deletion" do non_training = create(:event, :ended, facilitator_training: false) + person = create(:person) + reg = create(:event_registration, event: non_training, registrant: person, status: "attended") + create(:event_registration_organization, event_registration: reg, organization: organization) + create(:affiliation, person: person, organization: organization, title: "Facilitator", + start_date: 1.month.ago.to_date, event_registration: reg) get reconcile_affiliations_event_path(non_training) - expect(response).to redirect_to(registrants_event_path(non_training)) + expect(response.body).to include("Will be deleted") end it "denies a non-admin" do @@ -89,5 +94,31 @@ def registrant_with_affiliation(status:) post reconcile_affiliations_event_path(upcoming), params: { included: [ key ] } }.to change { person.affiliations.facilitators.where(organization: organization).count }.by(1) end + + it "deletes instead of same-daying when the delete option is checked" do + _person, affiliation = registrant_with_affiliation(status: "no_show") + key = AffiliationServices::ReconcileEvent.key_for(affiliation.person, organization) + + post reconcile_affiliations_event_path(event), params: { included: [ key ], delete: [ key ] } + + expect(Affiliation.exists?(affiliation.id)).to be(false) + end + + it "deletes a facilitator affiliation auto-created off a non-training event, keeping the job affiliation" do + non_training = create(:event, :ended, facilitator_training: false) + person = create(:person) + reg = create(:event_registration, event: non_training, registrant: person, status: "attended") + create(:event_registration_organization, event_registration: reg, organization: organization) + facilitator = create(:affiliation, person: person, organization: organization, title: "Facilitator", + start_date: 1.month.ago.to_date, event_registration: reg) + job = create(:affiliation, person: person, organization: organization, title: "Counselor", + event_registration: reg) + key = AffiliationServices::ReconcileEvent.key_for(person, organization) + + post reconcile_affiliations_event_path(non_training), params: { included: [ key ] } + + expect(Affiliation.exists?(facilitator.id)).to be(false) + expect(Affiliation.exists?(job.id)).to be(true) + end end end From a603fd1629a90ae9fca6adb1ce63cfd1f6cc7736 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Fri, 14 Aug 2026 08:02:56 -0400 Subject: [PATCH 06/50] Show full reconcile picture: reasons for no-action rows and attendance status Preview now lists every registrant-org pair, grouped by action, and adds a 'Not reconciled' section explaining why each is left alone, with attendance status shown per row. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../affiliation_services/reconcile_event.rb | 137 ++++++++++-------- .../reconcile_affiliations/index.html.erb | 102 ++++++++----- .../events/reconcile_affiliations_spec.rb | 11 ++ 3 files changed, 151 insertions(+), 99 deletions(-) diff --git a/app/services/affiliation_services/reconcile_event.rb b/app/services/affiliation_services/reconcile_event.rb index aa6bc2ee6a..5f53babeb2 100644 --- a/app/services/affiliation_services/reconcile_event.rb +++ b/app/services/affiliation_services/reconcile_event.rb @@ -1,24 +1,30 @@ module AffiliationServices # Event-level orchestration for the "Reconcile affiliations" bulk action. Walks - # the event's registrants and the organizations they linked, and per (person, - # org) works out what should happen to their **owned** facilitator affiliation - # (job affiliations are never touched): + # the event's registrants and the organizations they linked, and classifies each + # (person, org) so the confirm page can show exactly what will (and won't) happen + # to their **owned** facilitator affiliation. Job affiliations are never touched. # + # Actions: # :create — facilitator training, none exists yet but one should (pre-event # for anyone, post-event only for attendees). # :deactivate — facilitator training, an owned affiliation whose (ended) - # training they didn't complete. The admin may choose to delete - # it instead of same-daying it (see `delete_keys`). + # training they didn't complete. The admin may delete it instead + # of same-daying it (see `delete_keys`). # :reactivate — facilitator training, an owned affiliation same-dayed earlier, # now attended. - # :delete — NOT a facilitator training: an owned facilitator affiliation was - # auto-created off this event and shouldn't exist, so remove it. + # :delete — NOT a facilitator training: facilitator affiliation(s) + # auto-created off this event that shouldn't exist. + # :noop — nothing to do; the row carries a `reason` for the page. # - # `preview` returns the actionable rows without writing so the admin can see them - # and opt individual rows out; `apply` performs the kept rows and stamps the - # event's `affiliations_reconciled_at`. + # `preview` returns every pair (actionable and not) so the admin sees the full + # picture; `apply` performs the kept actionable rows and stamps the event's + # `affiliations_reconciled_at`. class ReconcileEvent - Row = Struct.new(:person, :organization, :registration, :affiliation, :action, :key, keyword_init: true) + Row = Struct.new(:person, :organization, :registration, :affiliation, :action, :reason, :key, keyword_init: true) do + def actionable? + action != :noop + end + end def self.key_for(person, organization) "#{person.id}:#{organization.id}" @@ -32,15 +38,15 @@ def preview rows end - # Apply the rows whose keys are in `included_keys`. For :deactivate rows whose - # key is also in `delete_keys`, delete the affiliation instead of same-daying - # it. Stamps the event and returns the number of pairs actually changed. + # Apply the actionable rows whose keys are in `included_keys`. For :deactivate + # rows whose key is also in `delete_keys`, delete the affiliation instead of + # same-daying it. Stamps the event and returns the number of pairs changed. def apply(included_keys:, delete_keys: []) included = Array(included_keys).to_set delete_instead = Array(delete_keys).to_set changed = rows.count do |row| - included.include?(row.key) && perform(row, delete_instead: delete_instead.include?(row.key)) + row.actionable? && included.include?(row.key) && perform(row, delete_instead: delete_instead.include?(row.key)) end @event.update!(affiliations_reconciled_at: Time.current) @@ -50,30 +56,45 @@ def apply(included_keys:, delete_keys: []) private def rows - @rows ||= pairs.filter_map { |person, organization, registration| build_row(person, organization, registration) } + @rows ||= pairs.map do |person, organization, registration| + action, reason, affiliation = classify(person, organization) + Row.new(person:, organization:, registration:, affiliation:, action:, reason:, key: self.class.key_for(person, organization)) + end + end + + def classify(person, organization) + owned = owned_facilitators(person, organization) + @event.facilitator_training? ? classify_training(person, organization, owned) : classify_non_training(person, organization, owned) end - def build_row(person, organization, registration) - if @event.facilitator_training? - action = training_action(person, organization) - return if action == :noop + def classify_training(person, organization, owned) + attended = completed_training?(person, organization) - affiliation = action == :create ? nil : owned_facilitator(person, organization) - else - affiliation = owned_facilitator_from_event(person, organization) - return if affiliation.nil? + if owned.any? + return [ :reactivate, nil, owned.find { |a| !a.active? } ] if attended && owned.any? { |a| !a.active? } + return [ :noop, "Active — attended", owned.first ] if attended - action = :delete - end + deactivatable = owned.select { |a| a.active? && source_ended?(a) } + return [ :deactivate, nil, deactivatable.first ] if deactivatable.any? + return [ :noop, "Already deactivated — didn't attend", owned.first ] if owned.none?(&:active?) - Row.new(person:, organization:, registration:, affiliation:, action:, key: self.class.key_for(person, organization)) + [ :noop, "Training hasn't ended yet", owned.first ] + elsif hand_facilitator?(person, organization) + [ :noop, "Hand-entered affiliation — left alone", nil ] + elsif !@event.ended? || attended + [ :create, nil, nil ] + else + [ :noop, "Didn't attend — no affiliation to create", nil ] + end end - def training_action(person, organization) - reconcile = ReconcileFacilitatorAffiliation.new(person:, organization:).plan - return reconcile unless reconcile == :noop + def classify_non_training(person, organization, owned) + from_event = owned.select { |a| a.event_registration&.event_id == @event.id } + return [ :delete, nil, from_event.first ] if from_event.any? + return [ :noop, "Facilitator affiliation from another event — left alone", owned.first ] if owned.any? + return [ :noop, "Hand-entered affiliation — left alone", nil ] if hand_facilitator?(person, organization) - create_needed?(person, organization) ? :create : :noop + [ :noop, "No facilitator affiliation", nil ] end def perform(row, delete_instead:) @@ -82,7 +103,7 @@ def perform(row, delete_instead:) apply_create(row) true when :delete - row.affiliation.destroy! + destroy_from_event(row.person, row.organization) true when :deactivate service = ReconcileFacilitatorAffiliation.new(person: row.person, organization: row.organization) @@ -106,17 +127,30 @@ def apply_create(row) ) end - # A facilitator affiliation should exist but doesn't. Skip when an owned one - # already exists (reconcile handles it — including a deliberately same-dayed - # no-show we must not resurrect) or when a hand-created active-or-pending one - # already covers it. Otherwise create it pre-event for anyone, post-event only - # for those who attended. - def create_needed?(person, organization) - facilitators = person.affiliations.facilitators.where(organization:) - return false if facilitators.where.not(event_registration_id: nil).exists? - return false if facilitators.active_or_pending.exists? + def destroy_from_event(person, organization) + owned_facilitators(person, organization) + .select { |a| a.event_registration&.event_id == @event.id } + .each(&:destroy!) + end + + def completed_training?(person, organization) + ReconcileFacilitatorAffiliation.new(person:, organization:).completed_training? + end - !@event.ended? || ReconcileFacilitatorAffiliation.new(person:, organization:).completed_training? + def source_ended?(affiliation) + affiliation.event_registration&.event&.ended? + end + + def hand_facilitator?(person, organization) + person.affiliations.facilitators.where(organization:, event_registration_id: nil).active_or_pending.exists? + end + + def owned_facilitators(person, organization) + person.affiliations.facilitators + .where(organization:) + .where.not(event_registration_id: nil) + .includes(event_registration: :event) + .to_a end # Distinct (person, organization, registration) triples from the event's @@ -135,24 +169,5 @@ def pairs end end end - - def owned_facilitator(person, organization) - person.affiliations.facilitators - .where(organization:) - .where.not(event_registration_id: nil) - .first - end - - # An owned facilitator affiliation that was auto-created off *this* (non-training) - # event — the row a non-training reconcile removes. Hand-created rows (no link) - # and affiliations from other events are left alone. - def owned_facilitator_from_event(person, organization) - person.affiliations.facilitators - .where(organization:) - .where.not(event_registration_id: nil) - .joins(:event_registration) - .where(event_registrations: { event_id: @event.id }) - .first - end end end diff --git a/app/views/events/reconcile_affiliations/index.html.erb b/app/views/events/reconcile_affiliations/index.html.erb index b6ca9f0b72..fdcdfdd264 100644 --- a/app/views/events/reconcile_affiliations/index.html.erb +++ b/app/views/events/reconcile_affiliations/index.html.erb @@ -34,50 +34,76 @@ <% end %>
+ <% actionable = @rows.select(&:actionable?) %> + <% skipped = @rows.reject(&:actionable?) %> + <% if @rows.empty? %>
- Nothing to reconcile — every facilitator affiliation already matches its attendance. + No registrants have linked an organization, so there's nothing to reconcile.
<% else %> - <%= form_with url: reconcile_affiliations_event_path(@event), method: :post do %> -
- <% @rows.each do |row| %> -
- <%= check_box_tag "included[]", row.key, true, id: "included_#{row.key}", class: "h-4 w-4 rounded border-gray-300 text-blue-600" %> - - <% case row.action %> - <% when :deactivate %> - - - Will be deactivated - - <% when :delete %> - - Will be deleted - - <% when :create %> - - Will be created - - <% else %> - - Will be reactivated - - <% end %> -
+ <% if actionable.any? %> + <%= form_with url: reconcile_affiliations_event_path(@event), method: :post do %> + <% sections = { create: [ "Will be created", "bg-blue-50 text-blue-700" ], + reactivate: [ "Will be reactivated", "bg-green-50 text-green-700" ], + deactivate: [ "Will be deactivated", "bg-red-50 text-red-700" ], + delete: [ "Will be deleted", "bg-red-100 text-red-800" ] } %> + <% sections.each do |action, (heading, badge_class)| %> + <% action_rows = actionable.select { |row| row.action == action } %> + <% next if action_rows.empty? %> +
+

<%= heading %> (<%= action_rows.size %>)

+
+ <% action_rows.each do |row| %> +
+ <%= check_box_tag "included[]", row.key, true, id: "included_#{row.key}", class: "h-4 w-4 rounded border-gray-300 text-blue-600" %> + + <%= row.registration.attendance_status_label %> + <% if row.action == :deactivate %> + + <% end %> + + <%= heading %> + +
+ <% end %> +
+
<% end %> -
-
- <%= link_to "Cancel", registrants_event_path(@event), class: "text-sm text-gray-500 hover:text-gray-700" %> - <%= submit_tag "Reconcile affiliations", class: "btn btn-primary" %> -
+
+ <%= link_to "Cancel", registrants_event_path(@event), class: "text-sm text-gray-500 hover:text-gray-700" %> + <%= submit_tag "Reconcile affiliations", class: "btn btn-primary" %> +
+ <% end %> + <% end %> + + <% if skipped.any? %> +
+

Not reconciled (<%= skipped.size %>)

+ <% skipped.group_by(&:reason).each do |reason, reason_rows| %> +
+

<%= reason %>

+
+ <% reason_rows.each do |row| %> +
+ + <%= row.person.name %> + — <%= row.organization.name %> + + <%= row.registration.attendance_status_label %> +
+ <% end %> +
+
+ <% end %> +
<% end %> <% end %>
diff --git a/spec/requests/events/reconcile_affiliations_spec.rb b/spec/requests/events/reconcile_affiliations_spec.rb index 7b68515716..424f1dc2e5 100644 --- a/spec/requests/events/reconcile_affiliations_spec.rb +++ b/spec/requests/events/reconcile_affiliations_spec.rb @@ -54,6 +54,17 @@ def registrant_with_affiliation(status:) expect(response.body).to include("Will be deleted") end + it "lists a no-action registrant under Not reconciled with the reason and attendance status" do + person, _affiliation = registrant_with_affiliation(status: "attended") + + get reconcile_affiliations_event_path(event) + + expect(response.body).to include("Not reconciled") + expect(response.body).to include("Active — attended") + expect(response.body).to include("Attended") + expect(response.body).to include(person.name) + end + it "denies a non-admin" do sign_in create(:user) From cdb8109ebbefc0dbf449b55fe281bebde4e25fb2 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Fri, 14 Aug 2026 08:06:45 -0400 Subject: [PATCH 07/50] Redesign reconcile page: group by person, per-affiliation, editable attendance Preview now groups actionable rows by person with the shared editable attendance chip and a note of their other-org facilitator affiliations; each facilitator affiliation is an individual row showing its date range with an Edit link to the person page. 'Not reconciled' is a collapsible section grouped by reason (hand-entered last), each reason collapsible too. Co-Authored-By: Claude Opus 4.8 (1M context) --- AGENTS.md | 2 +- .../reconcile_affiliations_controller.rb | 5 +- app/decorators/affiliation_decorator.rb | 8 + .../affiliation_services/reconcile_event.rb | 202 +++++++++--------- .../reconcile_affiliations/index.html.erb | 124 ++++++----- .../events/reconcile_affiliations_spec.rb | 11 +- 6 files changed, 189 insertions(+), 163 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2f6b6802dd..be375543ca 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -265,7 +265,7 @@ action, or `authorize! :workshop, to: :summary?`). - `AffiliationServices::ApplyScenarioEndDating` — What an agreement scenario means for the person's *existing* affiliations, run by the linking core before creation (see ADR-0002): a **new_job** ends their active affiliations at other orgs (job + facilitator alike, sparing the linked org's rows); every other scenario ends nothing — reinstatement reconciles registration-style, creating affiliations only where no active one exists. Ends are dated the day before the agreement takes effect so the fresh affiliations (starting on the submission date) don't overlap a row still counting as active until end of day - `AffiliationServices::CreateFromRegistration` — On registration / org linking, creates a "job affiliation" with the typed title (when present) plus a standing "Facilitator" affiliation, in one transaction. Skips the facilitator one only when the person already has an active-or-pending affiliation titled exactly "Facilitator" with that org (a current one or one dated to a future training); an ended facilitator affiliation gets a fresh second one. Dedupe is by title + org + dates, so a job title like "Lead Facilitator" still gets its own Facilitator affiliation. Accepts an optional `organization_address:` and sets it on every affiliation it creates (the registrant's typed agency address, upserted onto the org); when an affiliation already exists and is skipped, it backfills that address onto the existing one only if it has none (an admin-set address is never overwritten) - `AffiliationServices::ReconcileFacilitatorAffiliation` — Per `(person, organization)`, brings the person's **owned** facilitator affiliation (created by the registration flow, i.e. `event_registration_id` present) in line with attendance: keeps it active when they have any `attended` facilitator-training registration to that org; otherwise **same-days** it (`end_date := start_date`, which the model turns into `inactive: true`) once its source training has ended. Reactivates a previously same-dayed row when the person is later marked attended. Hand-created (unowned) rows are never touched. `#plan` returns the action (`:deactivate` / `:reactivate` / `:noop`) without writing. -- `AffiliationServices::ReconcileEvent` — Event-level orchestration for the "Reconcile affiliations" bulk action. Per `(person, org)` it decides, for facilitator trainings, `:create` (heal a missing facilitator affiliation — pre-event for anyone, post-event only for attendees), `:deactivate`, or `:reactivate`; for non-training events, `:delete` (remove a facilitator affiliation auto-created off this event, leaving job affiliations). `#preview` returns the actionable rows for the confirm page; `#apply(included_keys:, delete_keys:)` performs the rows the admin kept — creating via `CreateFromRegistration`, deactivating/reactivating via `ReconcileFacilitatorAffiliation`, or deleting (a `:deactivate` row whose key is in `delete_keys` is deleted instead of same-dayed) — and stamps the event's `affiliations_reconciled_at`. +- `AffiliationServices::ReconcileEvent` — Event-level orchestration for the "Reconcile affiliations" bulk action. Builds one row **per facilitator affiliation** tied to an org a registrant linked, classifying each `:create` / `:deactivate` / `:reactivate` (facilitator trainings) or `:delete` (facilitator affiliation auto-created off a non-training event), else `:noop` with a reason. `#actionable_person_groups` groups the actionable rows by person (with their attendance registration and other-org facilitator affiliations for context) for the confirm page; `#skipped_reason_sections` groups the no-action rows by reason (hand-entered last). `#apply(included_keys:, delete_keys:)` performs the kept rows (keys are `aff:` or `create::`; a `:deactivate` row also in `delete_keys` is deleted instead of same-dayed) and stamps `affiliations_reconciled_at`. Job affiliations are never touched. ### Sectors diff --git a/app/controllers/events/reconcile_affiliations_controller.rb b/app/controllers/events/reconcile_affiliations_controller.rb index f5d1d33913..85296e7de5 100644 --- a/app/controllers/events/reconcile_affiliations_controller.rb +++ b/app/controllers/events/reconcile_affiliations_controller.rb @@ -13,7 +13,10 @@ def index authorize! @event, to: :reconcile_affiliations? track_view("events.reconcile_affiliations", { event_id: @event.id }) - @rows = AffiliationServices::ReconcileEvent.new(@event).preview + reconcile = AffiliationServices::ReconcileEvent.new(@event) + @person_groups = reconcile.actionable_person_groups + @skipped_sections = reconcile.skipped_reason_sections + @has_rows = reconcile.any_rows? @event = @event.decorate end diff --git a/app/decorators/affiliation_decorator.rb b/app/decorators/affiliation_decorator.rb index 9b121826bf..c474e2fb32 100644 --- a/app/decorators/affiliation_decorator.rb +++ b/app/decorators/affiliation_decorator.rb @@ -23,4 +23,12 @@ def period_label end "Dates not recorded" end + + # Compact "started – ended" range for the affiliation, e.g. "Sep 17, 2026 – present". + # Reads "no start date" when unset so a blank date isn't silently omitted. + def date_range + start = start_date ? h.l(start_date, format: :long) : "no start date" + finish = end_date ? h.l(end_date, format: :long) : "present" + "#{start} – #{finish}" + end end diff --git a/app/services/affiliation_services/reconcile_event.rb b/app/services/affiliation_services/reconcile_event.rb index 5f53babeb2..a369a15021 100644 --- a/app/services/affiliation_services/reconcile_event.rb +++ b/app/services/affiliation_services/reconcile_event.rb @@ -1,51 +1,63 @@ module AffiliationServices # Event-level orchestration for the "Reconcile affiliations" bulk action. Walks - # the event's registrants and the organizations they linked, and classifies each - # (person, org) so the confirm page can show exactly what will (and won't) happen - # to their **owned** facilitator affiliation. Job affiliations are never touched. + # the event's registrants and, for each facilitator affiliation tied to an org + # they linked, works out what should happen to it (job affiliations are never + # touched). Produces one row per affiliation so each is individually actionable. # # Actions: # :create — facilitator training, none exists yet but one should (pre-event # for anyone, post-event only for attendees). - # :deactivate — facilitator training, an owned affiliation whose (ended) - # training they didn't complete. The admin may delete it instead - # of same-daying it (see `delete_keys`). - # :reactivate — facilitator training, an owned affiliation same-dayed earlier, - # now attended. - # :delete — NOT a facilitator training: facilitator affiliation(s) - # auto-created off this event that shouldn't exist. - # :noop — nothing to do; the row carries a `reason` for the page. + # :deactivate — facilitator training, owned, its (ended) training wasn't + # completed. The admin may delete it instead of same-daying it. + # :reactivate — facilitator training, owned, same-dayed earlier, now attended. + # :delete — NOT a facilitator training: an owned affiliation auto-created + # off this event that shouldn't exist. + # :noop — nothing to do; the row carries a `reason`. # - # `preview` returns every pair (actionable and not) so the admin sees the full - # picture; `apply` performs the kept actionable rows and stamps the event's - # `affiliations_reconciled_at`. + # `actionable_person_groups` groups the actionable rows by person (with their + # attendance registration and other-org facilitator affiliations for context); + # `skipped_reason_sections` groups the no-action rows by reason (hand-entered + # last). `apply` performs the kept actionable rows and stamps the event. class ReconcileEvent - Row = Struct.new(:person, :organization, :registration, :affiliation, :action, :reason, :key, keyword_init: true) do + HAND_ENTERED = "Hand-entered affiliation — left alone".freeze + + Row = Struct.new(:person, :registration, :organization, :affiliation, :action, :reason, :key, keyword_init: true) do def actionable? action != :noop end end - def self.key_for(person, organization) - "#{person.id}:#{organization.id}" - end - def initialize(event) @event = event end - def preview - rows + # Actionable rows grouped by person: [{ person:, registration:, rows:, + # other_facilitators: }]. `other_facilitators` are the person's active + # facilitator affiliations with orgs they did NOT link on this event. + def actionable_person_groups + all_rows.select(&:actionable?).group_by(&:person).map do |person, rows| + { person:, registration: rows.first.registration, rows:, other_facilitators: other_facilitators(person) } + end + end + + # No-action rows grouped by reason, hand-entered last: [[reason, [rows]]]. + def skipped_reason_sections + grouped = all_rows.reject(&:actionable?).group_by(&:reason) + grouped.keys.sort_by { |reason| [ reason == HAND_ENTERED ? 1 : 0, reason ] }.map { |reason| [ reason, grouped[reason] ] } + end + + def any_rows? + all_rows.any? end # Apply the actionable rows whose keys are in `included_keys`. For :deactivate # rows whose key is also in `delete_keys`, delete the affiliation instead of - # same-daying it. Stamps the event and returns the number of pairs changed. + # same-daying it. Stamps the event and returns the number of rows changed. def apply(included_keys:, delete_keys: []) included = Array(included_keys).to_set delete_instead = Array(delete_keys).to_set - changed = rows.count do |row| + changed = all_rows.count do |row| row.actionable? && included.include?(row.key) && perform(row, delete_instead: delete_instead.include?(row.key)) end @@ -55,82 +67,78 @@ def apply(included_keys:, delete_keys: []) private - def rows - @rows ||= pairs.map do |person, organization, registration| - action, reason, affiliation = classify(person, organization) - Row.new(person:, organization:, registration:, affiliation:, action:, reason:, key: self.class.key_for(person, organization)) + def all_rows + @all_rows ||= registrations_by_person.flat_map do |person, registrations| + registration = registrations.first + linked_organizations(registrations).flat_map { |organization| rows_for(person, registration, organization) } end end - def classify(person, organization) - owned = owned_facilitators(person, organization) - @event.facilitator_training? ? classify_training(person, organization, owned) : classify_non_training(person, organization, owned) + def rows_for(person, registration, organization) + attended = completed_training?(person, organization) + facilitators = person.affiliations.facilitators + .where(organization:) + .includes(event_registration: :event) + .to_a + + rows = facilitators.map { |affiliation| affiliation_row(person, registration, organization, affiliation, attended) } + rows << create_row(person, registration, organization, attended) if facilitators.empty? && @event.facilitator_training? + rows.compact end - def classify_training(person, organization, owned) - attended = completed_training?(person, organization) + def affiliation_row(person, registration, organization, affiliation, attended) + action, reason = classify_affiliation(affiliation, attended) + Row.new(person:, registration:, organization:, affiliation:, action:, reason:, key: "aff:#{affiliation.id}") + end + + def classify_affiliation(affiliation, attended) + owned = affiliation.event_registration_id.present? - if owned.any? - return [ :reactivate, nil, owned.find { |a| !a.active? } ] if attended && owned.any? { |a| !a.active? } - return [ :noop, "Active — attended", owned.first ] if attended + unless @event.facilitator_training? + return [ :delete, nil ] if owned && affiliation.event_registration&.event_id == @event.id + return [ :noop, "Facilitator affiliation from another event" ] if owned - deactivatable = owned.select { |a| a.active? && source_ended?(a) } - return [ :deactivate, nil, deactivatable.first ] if deactivatable.any? - return [ :noop, "Already deactivated — didn't attend", owned.first ] if owned.none?(&:active?) + return [ :noop, HAND_ENTERED ] + end + + return [ :noop, HAND_ENTERED ] unless owned - [ :noop, "Training hasn't ended yet", owned.first ] - elsif hand_facilitator?(person, organization) - [ :noop, "Hand-entered affiliation — left alone", nil ] - elsif !@event.ended? || attended - [ :create, nil, nil ] + if attended + affiliation.active? ? [ :noop, "Active — attended" ] : [ :reactivate, nil ] + elsif affiliation.active? && source_ended?(affiliation) + [ :deactivate, nil ] + elsif affiliation.active? + [ :noop, "Training hasn't ended yet" ] else - [ :noop, "Didn't attend — no affiliation to create", nil ] + [ :noop, "Already deactivated — didn't attend" ] end end - def classify_non_training(person, organization, owned) - from_event = owned.select { |a| a.event_registration&.event_id == @event.id } - return [ :delete, nil, from_event.first ] if from_event.any? - return [ :noop, "Facilitator affiliation from another event — left alone", owned.first ] if owned.any? - return [ :noop, "Hand-entered affiliation — left alone", nil ] if hand_facilitator?(person, organization) - - [ :noop, "No facilitator affiliation", nil ] + def create_row(person, registration, organization, attended) + if !@event.ended? || attended + Row.new(person:, registration:, organization:, affiliation: nil, action: :create, reason: nil, + key: "create:#{person.id}:#{organization.id}") + else + Row.new(person:, registration:, organization:, affiliation: nil, action: :noop, + reason: "Didn't attend — no affiliation created", key: "none:#{person.id}:#{organization.id}") + end end def perform(row, delete_instead:) case row.action when :create - apply_create(row) - true + AffiliationServices::CreateFromRegistration.call( + person: row.person, organization: row.organization, facilitator_training: true, + training_date: @event.start_date, event_registration: row.registration + ) when :delete - destroy_from_event(row.person, row.organization) - true + row.affiliation.destroy! when :deactivate - service = ReconcileFacilitatorAffiliation.new(person: row.person, organization: row.organization) - targets = service.deactivatable_affiliations - return false if targets.empty? - - delete_instead ? targets.each(&:destroy!) : service.call - true - else # :reactivate - ReconcileFacilitatorAffiliation.call(person: row.person, organization: row.organization) != :noop + delete_instead ? row.affiliation.destroy! : row.affiliation.update!(end_date: row.affiliation.start_date || Date.current) + when :reactivate + row.affiliation.update!(end_date: nil) end - end - - def apply_create(row) - AffiliationServices::CreateFromRegistration.call( - person: row.person, - organization: row.organization, - facilitator_training: true, - training_date: @event.start_date, - event_registration: row.registration - ) - end - - def destroy_from_event(person, organization) - owned_facilitators(person, organization) - .select { |a| a.event_registration&.event_id == @event.id } - .each(&:destroy!) + true end def completed_training?(person, organization) @@ -141,33 +149,23 @@ def source_ended?(affiliation) affiliation.event_registration&.event&.ended? end - def hand_facilitator?(person, organization) - person.affiliations.facilitators.where(organization:, event_registration_id: nil).active_or_pending.exists? + def other_facilitators(person) + person.affiliations.active.facilitators + .where.not(organization_id: linked_org_ids(person)) + .includes(:organization) + .to_a end - def owned_facilitators(person, organization) - person.affiliations.facilitators - .where(organization:) - .where.not(event_registration_id: nil) - .includes(event_registration: :event) - .to_a + def linked_org_ids(person) + linked_organizations(registrations_by_person[person]).map(&:id) end - # Distinct (person, organization, registration) triples from the event's - # registrants and the organizations each linked to their registration. - def pairs - @pairs ||= begin - seen = Set.new - @event.event_registrations.includes(:registrant, :organizations).flat_map do |registration| - registration.organizations.filter_map do |organization| - key = [ registration.registrant_id, organization.id ] - next if seen.include?(key) - - seen << key - [ registration.registrant, organization, registration ] - end - end - end + def linked_organizations(registrations) + registrations.flat_map(&:organizations).uniq + end + + def registrations_by_person + @registrations_by_person ||= @event.event_registrations.includes(:registrant, :organizations).group_by(&:registrant) end end end diff --git a/app/views/events/reconcile_affiliations/index.html.erb b/app/views/events/reconcile_affiliations/index.html.erb index fdcdfdd264..dad86ce0ab 100644 --- a/app/views/events/reconcile_affiliations/index.html.erb +++ b/app/views/events/reconcile_affiliations/index.html.erb @@ -13,8 +13,8 @@

This step brings facilitator affiliations in line with who registered and attended. Before the training it creates any missing facilitator affiliations for linked organizations. After the training it - same-days the affiliation of anyone who didn't attend (its end date is set to its start - date, so it no longer counts as active), and reactivates anyone later marked attended. + same-days the affiliation of anyone who didn't attend, and reactivates anyone later marked + attended. Job affiliations are never touched.

<% else %>

@@ -22,88 +22,108 @@ This deletes those. Job affiliations are left untouched.

<% end %> -

- Only affiliations this app created from a registration are touched — hand-entered affiliations are always left - alone. Uncheck a row to spare it this time. -

+

Only affiliations this app created from a registration are touched — hand-entered ones are always left alone.

<% if @event.affiliations_reconciled_at %>

Last reconciled <%= @event.affiliations_reconciled_at.to_fs(:long) %>.

<% end %> - <% if @event.affiliations_reconciliation_stale? %> -

Attendance has changed since the last reconciliation — re-run to bring affiliations up to date.

+ <% if @person_groups.any? && @event.affiliations_reconciliation_stale? %> +

Attendance has changed since the last reconciliation — apply again below to bring affiliations up to date.

<% end %> - <% actionable = @rows.select(&:actionable?) %> - <% skipped = @rows.reject(&:actionable?) %> - - <% if @rows.empty? %> + <% unless @has_rows %>
No registrants have linked an organization, so there's nothing to reconcile.
- <% else %> - <% if actionable.any? %> - <%= form_with url: reconcile_affiliations_event_path(@event), method: :post do %> - <% sections = { create: [ "Will be created", "bg-blue-50 text-blue-700" ], - reactivate: [ "Will be reactivated", "bg-green-50 text-green-700" ], - deactivate: [ "Will be deactivated", "bg-red-50 text-red-700" ], - delete: [ "Will be deleted", "bg-red-100 text-red-800" ] } %> - <% sections.each do |action, (heading, badge_class)| %> - <% action_rows = actionable.select { |row| row.action == action } %> - <% next if action_rows.empty? %> -
-

<%= heading %> (<%= action_rows.size %>)

-
- <% action_rows.each do |row| %> -
+ <% end %> + + <% badges = { create: [ "Will be created", "bg-blue-50 text-blue-700" ], + reactivate: [ "Will be reactivated", "bg-green-50 text-green-700" ], + deactivate: [ "Will be deactivated", "bg-red-50 text-red-700" ], + delete: [ "Will be deleted", "bg-red-100 text-red-800" ] } %> + + <% if @person_groups.any? %> + <%= form_with url: reconcile_affiliations_event_path(@event), method: :post do %> +
+ <% @person_groups.each do |group| %> +
+
+ <%= group[:person].name %> + <%= render "event_registrations/attendance_status_badge", registration: group[:registration] %> +
+ <% if group[:other_facilitators].any? %> +

+ Also a facilitator at <%= group[:other_facilitators].map { |a| a.organization.name }.to_sentence %>. +

+ <% end %> + +
+ <% group[:rows].each do |row| %> + <% heading, badge_class = badges[row.action] %> +
<%= check_box_tag "included[]", row.key, true, id: "included_#{row.key}", class: "h-4 w-4 rounded border-gray-300 text-blue-600" %> - - <%= row.registration.attendance_status_label %> + + <%= row.organization.name %> + <% if row.affiliation %> + · <%= row.affiliation.decorate.date_range %> + <% end %> + <% if row.action == :deactivate %> <% end %> - - <%= heading %> - + <% if row.affiliation %> + <%= link_to "Edit", edit_person_path(row.person, anchor: "affiliations"), class: "text-xs text-blue-600 hover:underline", target: "_blank", rel: "noopener" %> + <% end %> + <%= heading %>
<% end %>
<% end %> +
-
- <%= link_to "Cancel", registrants_event_path(@event), class: "text-sm text-gray-500 hover:text-gray-700" %> - <%= submit_tag "Reconcile affiliations", class: "btn btn-primary" %> -
- <% end %> +
+ <%= link_to "Cancel", registrants_event_path(@event), class: "text-sm text-gray-500 hover:text-gray-700" %> + <%= submit_tag "Reconcile affiliations", class: "btn btn-primary" %> +
<% end %> + <% end %> - <% if skipped.any? %> -
-

Not reconciled (<%= skipped.size %>)

- <% skipped.group_by(&:reason).each do |reason, reason_rows| %> -
-

<%= reason %>

-
- <% reason_rows.each do |row| %> -
+ <% if @skipped_sections.any? %> + <% skipped_count = @skipped_sections.sum { |(_reason, rows)| rows.size } %> +
+ + Not reconciled (<%= skipped_count %>) + +
+ <% @skipped_sections.each do |reason, rows| %> +
+ + <%= reason %> (<%= rows.size %>) + +
+ <% rows.each do |row| %> +
<%= row.person.name %> — <%= row.organization.name %> + <% if row.affiliation %> + · <%= row.affiliation.decorate.date_range %> + <% end %> <%= row.registration.attendance_status_label %> + <% if row.affiliation %> + <%= link_to "Edit", edit_person_path(row.person, anchor: "affiliations"), class: "text-xs text-blue-600 hover:underline", target: "_blank", rel: "noopener" %> + <% end %>
<% end %>
-
+
<% end %> -
- <% end %> +
+ <% end %>
diff --git a/spec/requests/events/reconcile_affiliations_spec.rb b/spec/requests/events/reconcile_affiliations_spec.rb index 424f1dc2e5..86d698f584 100644 --- a/spec/requests/events/reconcile_affiliations_spec.rb +++ b/spec/requests/events/reconcile_affiliations_spec.rb @@ -77,9 +77,8 @@ def registrant_with_affiliation(status:) describe "POST create" do it "deactivates the included non-completer and stamps the event" do _person, affiliation = registrant_with_affiliation(status: "no_show") - key = AffiliationServices::ReconcileEvent.key_for(affiliation.person, organization) - post reconcile_affiliations_event_path(event), params: { included: [ key ] } + post reconcile_affiliations_event_path(event), params: { included: [ "aff:#{affiliation.id}" ] } expect(response).to redirect_to(registrants_event_path(event)) expect(affiliation.reload).not_to be_active @@ -99,16 +98,15 @@ def registrant_with_affiliation(status:) person = create(:person) reg = create(:event_registration, event: upcoming, registrant: person, status: "registered") create(:event_registration_organization, event_registration: reg, organization: organization) - key = AffiliationServices::ReconcileEvent.key_for(person, organization) expect { - post reconcile_affiliations_event_path(upcoming), params: { included: [ key ] } + post reconcile_affiliations_event_path(upcoming), params: { included: [ "create:#{person.id}:#{organization.id}" ] } }.to change { person.affiliations.facilitators.where(organization: organization).count }.by(1) end it "deletes instead of same-daying when the delete option is checked" do _person, affiliation = registrant_with_affiliation(status: "no_show") - key = AffiliationServices::ReconcileEvent.key_for(affiliation.person, organization) + key = "aff:#{affiliation.id}" post reconcile_affiliations_event_path(event), params: { included: [ key ], delete: [ key ] } @@ -124,9 +122,8 @@ def registrant_with_affiliation(status:) start_date: 1.month.ago.to_date, event_registration: reg) job = create(:affiliation, person: person, organization: organization, title: "Counselor", event_registration: reg) - key = AffiliationServices::ReconcileEvent.key_for(person, organization) - post reconcile_affiliations_event_path(non_training), params: { included: [ key ] } + post reconcile_affiliations_event_path(non_training), params: { included: [ "aff:#{facilitator.id}" ] } expect(Affiliation.exists?(facilitator.id)).to be(false) expect(Affiliation.exists?(job.id)).to be(true) From 903187cf1e66ada9434212584dc3051d8206f695 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Fri, 14 Aug 2026 08:10:45 -0400 Subject: [PATCH 08/50] Editable attendance chip in skipped rows, Edit first, shorter dates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Render the shared attendance chip (not plain text) in the Not reconciled rows so status is editable there too, move the Edit link ahead of the status, and shorten the affiliation date range to 'Oct 13, 2026 – present'. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/decorators/affiliation_decorator.rb | 6 +++--- app/views/events/reconcile_affiliations/index.html.erb | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/decorators/affiliation_decorator.rb b/app/decorators/affiliation_decorator.rb index c474e2fb32..990b5e4ccb 100644 --- a/app/decorators/affiliation_decorator.rb +++ b/app/decorators/affiliation_decorator.rb @@ -24,11 +24,11 @@ def period_label "Dates not recorded" end - # Compact "started – ended" range for the affiliation, e.g. "Sep 17, 2026 – present". + # Compact "started – ended" range for the affiliation, e.g. "Oct 13, 2026 – present". # Reads "no start date" when unset so a blank date isn't silently omitted. def date_range - start = start_date ? h.l(start_date, format: :long) : "no start date" - finish = end_date ? h.l(end_date, format: :long) : "present" + start = start_date ? start_date.strftime("%b %-d, %Y") : "no start date" + finish = end_date ? end_date.strftime("%b %-d, %Y") : "present" "#{start} – #{finish}" end end diff --git a/app/views/events/reconcile_affiliations/index.html.erb b/app/views/events/reconcile_affiliations/index.html.erb index dad86ce0ab..463a9c27a4 100644 --- a/app/views/events/reconcile_affiliations/index.html.erb +++ b/app/views/events/reconcile_affiliations/index.html.erb @@ -68,15 +68,15 @@ · <%= row.affiliation.decorate.date_range %> <% end %> + <% if row.affiliation %> + <%= link_to "Edit", edit_person_path(row.person, anchor: "affiliations"), class: "text-xs text-blue-600 hover:underline", target: "_blank", rel: "noopener" %> + <% end %> <% if row.action == :deactivate %> <% end %> - <% if row.affiliation %> - <%= link_to "Edit", edit_person_path(row.person, anchor: "affiliations"), class: "text-xs text-blue-600 hover:underline", target: "_blank", rel: "noopener" %> - <% end %> <%= heading %> <% end %> @@ -114,10 +114,10 @@ · <%= row.affiliation.decorate.date_range %> <% end %> - <%= row.registration.attendance_status_label %> <% if row.affiliation %> <%= link_to "Edit", edit_person_path(row.person, anchor: "affiliations"), class: "text-xs text-blue-600 hover:underline", target: "_blank", rel: "noopener" %> <% end %> + <%= render "event_registrations/attendance_status_badge", registration: row.registration %> <% end %> From 06d0f6eb9af9a98bb50756ee1c021b23d44bf42d Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Fri, 14 Aug 2026 08:20:12 -0400 Subject: [PATCH 09/50] Reconcile all facilitator affiliations incl. hand-entered; clearer action controls Reconcile every facilitator affiliation for a linked org (not just app-created), gated to post-event so a pre-event run never deactivates and with per-row opt-out. Move the include checkbox into the action chip so it's clear checking it performs that action, move the other-org facilitator note below the rows, link org/dates to the specific affiliation anchor and names to the registration, and strengthen the Not reconciled section headers (open by default, expand/collapse all). Co-Authored-By: Claude Opus 4.8 (1M context) --- AGENTS.md | 2 +- .../affiliation_services/reconcile_event.rb | 44 ++++++----- .../reconcile_affiliations/index.html.erb | 77 +++++++++++-------- .../events/reconcile_affiliations_spec.rb | 22 ++++++ 4 files changed, 91 insertions(+), 54 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index be375543ca..4241a7f057 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -265,7 +265,7 @@ action, or `authorize! :workshop, to: :summary?`). - `AffiliationServices::ApplyScenarioEndDating` — What an agreement scenario means for the person's *existing* affiliations, run by the linking core before creation (see ADR-0002): a **new_job** ends their active affiliations at other orgs (job + facilitator alike, sparing the linked org's rows); every other scenario ends nothing — reinstatement reconciles registration-style, creating affiliations only where no active one exists. Ends are dated the day before the agreement takes effect so the fresh affiliations (starting on the submission date) don't overlap a row still counting as active until end of day - `AffiliationServices::CreateFromRegistration` — On registration / org linking, creates a "job affiliation" with the typed title (when present) plus a standing "Facilitator" affiliation, in one transaction. Skips the facilitator one only when the person already has an active-or-pending affiliation titled exactly "Facilitator" with that org (a current one or one dated to a future training); an ended facilitator affiliation gets a fresh second one. Dedupe is by title + org + dates, so a job title like "Lead Facilitator" still gets its own Facilitator affiliation. Accepts an optional `organization_address:` and sets it on every affiliation it creates (the registrant's typed agency address, upserted onto the org); when an affiliation already exists and is skipped, it backfills that address onto the existing one only if it has none (an admin-set address is never overwritten) - `AffiliationServices::ReconcileFacilitatorAffiliation` — Per `(person, organization)`, brings the person's **owned** facilitator affiliation (created by the registration flow, i.e. `event_registration_id` present) in line with attendance: keeps it active when they have any `attended` facilitator-training registration to that org; otherwise **same-days** it (`end_date := start_date`, which the model turns into `inactive: true`) once its source training has ended. Reactivates a previously same-dayed row when the person is later marked attended. Hand-created (unowned) rows are never touched. `#plan` returns the action (`:deactivate` / `:reactivate` / `:noop`) without writing. -- `AffiliationServices::ReconcileEvent` — Event-level orchestration for the "Reconcile affiliations" bulk action. Builds one row **per facilitator affiliation** tied to an org a registrant linked, classifying each `:create` / `:deactivate` / `:reactivate` (facilitator trainings) or `:delete` (facilitator affiliation auto-created off a non-training event), else `:noop` with a reason. `#actionable_person_groups` groups the actionable rows by person (with their attendance registration and other-org facilitator affiliations for context) for the confirm page; `#skipped_reason_sections` groups the no-action rows by reason (hand-entered last). `#apply(included_keys:, delete_keys:)` performs the kept rows (keys are `aff:` or `create::`; a `:deactivate` row also in `delete_keys` is deleted instead of same-dayed) and stamps `affiliations_reconciled_at`. Job affiliations are never touched. +- `AffiliationServices::ReconcileEvent` — Event-level orchestration for the "Reconcile affiliations" bulk action. Builds one row **per facilitator affiliation** for an org a registrant linked — **hand-entered rows included, not just app-created ones** — classifying each `:create` / `:deactivate` / `:reactivate` (facilitator trainings) or `:delete` (facilitator affiliation auto-created off a non-training event), else `:noop` with a reason. Deactivation is gated to post-event (owned rows on their source training's `ended?`, hand-entered on this event's `ended?`), so a pre-event run never deactivates. `#actionable_person_groups` groups actionable rows by person (with attendance registration and other-org facilitator affiliations for context); `#skipped_reason_sections` groups no-action rows by reason. `#apply(included_keys:, delete_keys:)` performs the kept rows (keys are `aff:` or `create::`; a `:deactivate` row also in `delete_keys` is deleted instead of same-dayed) and stamps `affiliations_reconciled_at`. Job affiliations are never touched. ### Sectors diff --git a/app/services/affiliation_services/reconcile_event.rb b/app/services/affiliation_services/reconcile_event.rb index a369a15021..958e38f8ba 100644 --- a/app/services/affiliation_services/reconcile_event.rb +++ b/app/services/affiliation_services/reconcile_event.rb @@ -16,11 +16,10 @@ module AffiliationServices # # `actionable_person_groups` groups the actionable rows by person (with their # attendance registration and other-org facilitator affiliations for context); - # `skipped_reason_sections` groups the no-action rows by reason (hand-entered - # last). `apply` performs the kept actionable rows and stamps the event. + # `skipped_reason_sections` groups the no-action rows by reason. `apply` performs + # the kept actionable rows and stamps the event. Every facilitator affiliation for + # a linked org is reconciled — hand-entered rows included, not just app-created ones. class ReconcileEvent - HAND_ENTERED = "Hand-entered affiliation — left alone".freeze - Row = Struct.new(:person, :registration, :organization, :affiliation, :action, :reason, :key, keyword_init: true) do def actionable? action != :noop @@ -43,7 +42,7 @@ def actionable_person_groups # No-action rows grouped by reason, hand-entered last: [[reason, [rows]]]. def skipped_reason_sections grouped = all_rows.reject(&:actionable?).group_by(&:reason) - grouped.keys.sort_by { |reason| [ reason == HAND_ENTERED ? 1 : 0, reason ] }.map { |reason| [ reason, grouped[reason] ] } + grouped.keys.sort.map { |reason| [ reason, grouped[reason] ] } end def any_rows? @@ -75,14 +74,24 @@ def all_rows end def rows_for(person, registration, organization) - attended = completed_training?(person, organization) facilitators = person.affiliations.facilitators .where(organization:) .includes(event_registration: :event) .to_a + unless @event.facilitator_training? + # A non-training event confers no facilitation, so it only removes + # facilitator affiliations that were auto-created off it. + return facilitators.filter_map do |affiliation| + next unless affiliation.event_registration&.event_id == @event.id + + Row.new(person:, registration:, organization:, affiliation:, action: :delete, reason: nil, key: "aff:#{affiliation.id}") + end + end + + attended = completed_training?(person, organization) rows = facilitators.map { |affiliation| affiliation_row(person, registration, organization, affiliation, attended) } - rows << create_row(person, registration, organization, attended) if facilitators.empty? && @event.facilitator_training? + rows << create_row(person, registration, organization, attended) if facilitators.empty? rows.compact end @@ -91,21 +100,14 @@ def affiliation_row(person, registration, organization, affiliation, attended) Row.new(person:, registration:, organization:, affiliation:, action:, reason:, key: "aff:#{affiliation.id}") end + # Reconciles EVERY facilitator affiliation for the org — hand-entered ones + # included, not just app-created rows. Deactivation only applies once the + # governing training has ended (a hand-entered row has no source training, so + # it's gated on this event ending) — so a pre-event run never deactivates. def classify_affiliation(affiliation, attended) - owned = affiliation.event_registration_id.present? - - unless @event.facilitator_training? - return [ :delete, nil ] if owned && affiliation.event_registration&.event_id == @event.id - return [ :noop, "Facilitator affiliation from another event" ] if owned - - return [ :noop, HAND_ENTERED ] - end - - return [ :noop, HAND_ENTERED ] unless owned - if attended affiliation.active? ? [ :noop, "Active — attended" ] : [ :reactivate, nil ] - elsif affiliation.active? && source_ended?(affiliation) + elsif affiliation.active? && deactivation_ready?(affiliation) [ :deactivate, nil ] elsif affiliation.active? [ :noop, "Training hasn't ended yet" ] @@ -114,6 +116,10 @@ def classify_affiliation(affiliation, attended) end end + def deactivation_ready?(affiliation) + affiliation.event_registration_id ? source_ended?(affiliation) : @event.ended? + end + def create_row(person, registration, organization, attended) if !@event.ended? || attended Row.new(person:, registration:, organization:, affiliation: nil, action: :create, reason: nil, diff --git a/app/views/events/reconcile_affiliations/index.html.erb b/app/views/events/reconcile_affiliations/index.html.erb index 463a9c27a4..967d8ec4c5 100644 --- a/app/views/events/reconcile_affiliations/index.html.erb +++ b/app/views/events/reconcile_affiliations/index.html.erb @@ -22,7 +22,7 @@ This deletes those. Job affiliations are left untouched.

<% end %> -

Only affiliations this app created from a registration are touched — hand-entered ones are always left alone.

+

Every facilitator affiliation for a linked organization is reconciled against attendance — including hand-entered ones. Review each row and uncheck any you want to leave as-is.

<% if @event.affiliations_reconciled_at %>

Last reconciled <%= @event.affiliations_reconciled_at.to_fs(:long) %>.

<% end %> @@ -47,40 +47,45 @@
<% @person_groups.each do |group| %>
-
- <%= group[:person].name %> +
+ <%= link_to group[:person].name, edit_event_registration_path(group[:registration]), target: "_blank", rel: "noopener", class: "font-semibold text-gray-900 hover:underline hover:text-blue-700" %> <%= render "event_registrations/attendance_status_badge", registration: group[:registration] %>
- <% if group[:other_facilitators].any? %> -

- Also a facilitator at <%= group[:other_facilitators].map { |a| a.organization.name }.to_sentence %>. -

- <% end %>
<% group[:rows].each do |row| %> <% heading, badge_class = badges[row.action] %>
- <%= check_box_tag "included[]", row.key, true, id: "included_#{row.key}", class: "h-4 w-4 rounded border-gray-300 text-blue-600" %> - - <%= row.organization.name %> - <% if row.affiliation %> + <% if row.affiliation %> + <%= link_to edit_person_path(row.person, anchor: dom_id(row.affiliation)), target: "_blank", rel: "noopener", title: "Edit affiliation", class: "flex-1 min-w-0 text-sm hover:underline" do %> + <%= row.organization.name %> · <%= row.affiliation.decorate.date_range %> <% end %> - - <% if row.affiliation %> - <%= link_to "Edit", edit_person_path(row.person, anchor: "affiliations"), class: "text-xs text-blue-600 hover:underline", target: "_blank", rel: "noopener" %> + <% else %> + <%= row.organization.name %> <% end %> - <% if row.action == :deactivate %> -
<% end %>
+ + <% if group[:other_facilitators].any? %> +

+ Also a facilitator at <%= group[:other_facilitators].map { |a| a.organization.name }.to_sentence %>. +

+ <% end %>
<% end %>
@@ -94,29 +99,33 @@ <% if @skipped_sections.any? %> <% skipped_count = @skipped_sections.sum { |(_reason, rows)| rows.size } %> -
- - Not reconciled (<%= skipped_count %>) +
+ + Not reconciled (<%= skipped_count %>) -
+
+
+ +
<% @skipped_sections.each do |reason, rows| %> -
- - <%= reason %> (<%= rows.size %>) +
+ + <%= reason %> (<%= rows.size %>)
<% rows.each do |row| %>
- <%= row.person.name %> - — <%= row.organization.name %> + <%= link_to row.person.name, edit_event_registration_path(row.registration), target: "_blank", rel: "noopener", class: "font-medium text-gray-800 hover:underline hover:text-blue-700" %> — <% if row.affiliation %> - · <%= row.affiliation.decorate.date_range %> + <%= link_to edit_person_path(row.person, anchor: dom_id(row.affiliation)), target: "_blank", rel: "noopener", title: "Edit affiliation", class: "hover:underline hover:text-blue-700" do %> + <%= row.organization.name %> · <%= row.affiliation.decorate.date_range %> + <% end %> + <% else %> + <%= row.organization.name %> <% end %> - <% if row.affiliation %> - <%= link_to "Edit", edit_person_path(row.person, anchor: "affiliations"), class: "text-xs text-blue-600 hover:underline", target: "_blank", rel: "noopener" %> - <% end %> <%= render "event_registrations/attendance_status_badge", registration: row.registration %>
<% end %> diff --git a/spec/requests/events/reconcile_affiliations_spec.rb b/spec/requests/events/reconcile_affiliations_spec.rb index 86d698f584..71ee917a31 100644 --- a/spec/requests/events/reconcile_affiliations_spec.rb +++ b/spec/requests/events/reconcile_affiliations_spec.rb @@ -65,6 +65,17 @@ def registrant_with_affiliation(status:) expect(response.body).to include(person.name) end + it "reconciles a hand-entered (unowned) facilitator affiliation too" do + person = create(:person) + reg = create(:event_registration, event: event, registrant: person, status: "no_show") + create(:event_registration_organization, event_registration: reg, organization: organization) + create(:affiliation, person: person, organization: organization, title: "Facilitator", start_date: 1.year.ago.to_date) + + get reconcile_affiliations_event_path(event) + + expect(response.body).to include("Will be deactivated") + end + it "denies a non-admin" do sign_in create(:user) @@ -113,6 +124,17 @@ def registrant_with_affiliation(status:) expect(Affiliation.exists?(affiliation.id)).to be(false) end + it "deactivates a hand-entered facilitator affiliation when included" do + person = create(:person) + reg = create(:event_registration, event: event, registrant: person, status: "no_show") + create(:event_registration_organization, event_registration: reg, organization: organization) + hand_entered = create(:affiliation, person: person, organization: organization, title: "Facilitator", start_date: 1.year.ago.to_date) + + post reconcile_affiliations_event_path(event), params: { included: [ "aff:#{hand_entered.id}" ] } + + expect(hand_entered.reload).not_to be_active + end + it "deletes a facilitator affiliation auto-created off a non-training event, keeping the job affiliation" do non_training = create(:event, :ended, facilitator_training: false) person = create(:person) From a6ef0eacccbbf7ad60e715c051cf841a2aac73bc Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Fri, 14 Aug 2026 08:24:39 -0400 Subject: [PATCH 10/50] Action toggles as buttons with error-red on select, hover tooltips, header note MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Style the include/delete-instead controls as buttons that only turn error-red when selected (peer-checked, no JS); add hover tooltips explaining deactivate/delete (delete as bullets: this affiliation only, job + other-org affiliations untouched). Move the 'Also a facilitator at …' note beside the name, truncated and linking to the single affiliation anchor (or the affiliations section when several). Order the Not reconciled sections with 'Active — attended' second-to-last. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../affiliation_services/reconcile_event.rb | 13 +++++- .../reconcile_affiliations/_tooltip.html.erb | 17 +++++++ .../reconcile_affiliations/index.html.erb | 44 ++++++++++++------- 3 files changed, 56 insertions(+), 18 deletions(-) create mode 100644 app/views/events/reconcile_affiliations/_tooltip.html.erb diff --git a/app/services/affiliation_services/reconcile_event.rb b/app/services/affiliation_services/reconcile_event.rb index 958e38f8ba..7cd9816cb3 100644 --- a/app/services/affiliation_services/reconcile_event.rb +++ b/app/services/affiliation_services/reconcile_event.rb @@ -39,10 +39,19 @@ def actionable_person_groups end end - # No-action rows grouped by reason, hand-entered last: [[reason, [rows]]]. + # No-action rows grouped by reason: [[reason, [rows]]]. "Active — attended" sorts + # second-to-last and the trivial "no affiliation" bucket last; the rest alphabetical. def skipped_reason_sections grouped = all_rows.reject(&:actionable?).group_by(&:reason) - grouped.keys.sort.map { |reason| [ reason, grouped[reason] ] } + grouped.keys.sort_by { |reason| [ reason_rank(reason), reason ] }.map { |reason| [ reason, grouped[reason] ] } + end + + def reason_rank(reason) + case reason + when "Active — attended" then 8 + when "Didn't attend — no affiliation created" then 9 + else 0 + end end def any_rows? diff --git a/app/views/events/reconcile_affiliations/_tooltip.html.erb b/app/views/events/reconcile_affiliations/_tooltip.html.erb new file mode 100644 index 0000000000..582555db3a --- /dev/null +++ b/app/views/events/reconcile_affiliations/_tooltip.html.erb @@ -0,0 +1,17 @@ +<%# Hover explanation for a reconcile action. `kind` is the action symbol. %> + diff --git a/app/views/events/reconcile_affiliations/index.html.erb b/app/views/events/reconcile_affiliations/index.html.erb index 967d8ec4c5..1b74df0ae1 100644 --- a/app/views/events/reconcile_affiliations/index.html.erb +++ b/app/views/events/reconcile_affiliations/index.html.erb @@ -46,15 +46,31 @@ <%= form_with url: reconcile_affiliations_event_path(@event), method: :post do %>
<% @person_groups.each do |group| %> + <% checked_class = { + create: "peer-checked:bg-blue-50 peer-checked:text-blue-700 peer-checked:border-blue-300", + reactivate: "peer-checked:bg-green-50 peer-checked:text-green-700 peer-checked:border-green-300", + deactivate: "peer-checked:bg-red-50 peer-checked:text-red-700 peer-checked:border-red-300", + delete: "peer-checked:bg-red-50 peer-checked:text-red-700 peer-checked:border-red-300" + } %> + <% button_base = "inline-flex items-center rounded-md border border-gray-300 bg-white px-3 py-1.5 text-xs font-medium text-gray-500 shadow-sm hover:bg-gray-50 peer-focus-visible:ring-2 peer-focus-visible:ring-blue-400" %>
- <%= link_to group[:person].name, edit_event_registration_path(group[:registration]), target: "_blank", rel: "noopener", class: "font-semibold text-gray-900 hover:underline hover:text-blue-700" %> +
+ <%= link_to group[:person].name, edit_event_registration_path(group[:registration]), target: "_blank", rel: "noopener", class: "font-semibold text-gray-900 hover:underline hover:text-blue-700 shrink-0" %> + <% others = group[:other_facilitators] %> + <% if others.any? %> + <% note = "Also a facilitator at #{others.map { |a| a.organization.name }.to_sentence}." %> + <% anchor = others.one? ? dom_id(others.first) : "affiliations" %> + <%= link_to note, edit_person_path(group[:person], anchor: anchor), target: "_blank", rel: "noopener", + title: note, class: "text-xs text-gray-500 truncate hover:underline hover:text-blue-700 min-w-0" %> + <% end %> +
<%= render "event_registrations/attendance_status_badge", registration: group[:registration] %>
<% group[:rows].each do |row| %> - <% heading, badge_class = badges[row.action] %> + <% heading, _badge_class = badges[row.action] %>
<% if row.affiliation %> <%= link_to edit_person_path(row.person, anchor: dom_id(row.affiliation)), target: "_blank", rel: "noopener", title: "Edit affiliation", class: "flex-1 min-w-0 text-sm hover:underline" do %> @@ -64,28 +80,24 @@ <% else %> <%= row.organization.name %> <% end %> -
+
<% if row.action == :deactivate %> -
<% end %>
- - <% if group[:other_facilitators].any? %> -

- Also a facilitator at <%= group[:other_facilitators].map { |a| a.organization.name }.to_sentence %>. -

- <% end %>
<% end %>
From 8a2796802fc2581ffc38303e6f67988cae7107da Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Fri, 14 Aug 2026 08:31:05 -0400 Subject: [PATCH 11/50] Move Collapse all into the Not reconciled header; more space between sections Co-Authored-By: Claude Opus 4.8 (1M context) --- .../events/reconcile_affiliations/index.html.erb | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/app/views/events/reconcile_affiliations/index.html.erb b/app/views/events/reconcile_affiliations/index.html.erb index 1b74df0ae1..cf3335c763 100644 --- a/app/views/events/reconcile_affiliations/index.html.erb +++ b/app/views/events/reconcile_affiliations/index.html.erb @@ -111,15 +111,13 @@ <% if @skipped_sections.any? %> <% skipped_count = @skipped_sections.sum { |(_reason, rows)| rows.size } %> -
- +
+ Not reconciled (<%= skipped_count %>) + -
-
- -
+
<% @skipped_sections.each do |reason, rows| %>
From 14737cedb89d019c79c7af63d111e2790d864100 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Fri, 14 Aug 2026 09:06:13 -0400 Subject: [PATCH 12/50] Show checkbox inside action buttons; make deactivate/delete mutually exclusive Move the checkbox back inside each button (has-[:checked] colors the whole button on select, error-red for deactivate/delete). Add an exclusive-checkboxes Stimulus controller so checking 'Delete instead' clears 'Will be deactivated' and vice versa; apply now treats a delete key as delete regardless of the include key. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../exclusive_checkboxes_controller.js | 17 ++++++++++++ .../affiliation_services/reconcile_event.rb | 19 +++++++++++--- .../reconcile_affiliations/index.html.erb | 26 +++++++++---------- 3 files changed, 45 insertions(+), 17 deletions(-) create mode 100644 app/frontend/javascript/controllers/exclusive_checkboxes_controller.js diff --git a/app/frontend/javascript/controllers/exclusive_checkboxes_controller.js b/app/frontend/javascript/controllers/exclusive_checkboxes_controller.js new file mode 100644 index 0000000000..cd165d589f --- /dev/null +++ b/app/frontend/javascript/controllers/exclusive_checkboxes_controller.js @@ -0,0 +1,17 @@ +import { Controller } from "@hotwired/stimulus" + +// Connects to data-controller="exclusive-checkboxes" +// Makes a small group of checkboxes mutually exclusive — like radios, but any can +// be left unchecked. Checking one clears the others in the group (e.g. "Delete +// instead" and "Will be deactivated" are two choices for the same row). +export default class extends Controller { + static targets = ["box"] + + select(event) { + if (!event.target.checked) return + + this.boxTargets.forEach((box) => { + if (box !== event.target) box.checked = false + }) + } +} diff --git a/app/services/affiliation_services/reconcile_event.rb b/app/services/affiliation_services/reconcile_event.rb index 7cd9816cb3..9bab55a538 100644 --- a/app/services/affiliation_services/reconcile_event.rb +++ b/app/services/affiliation_services/reconcile_event.rb @@ -63,10 +63,19 @@ def any_rows? # same-daying it. Stamps the event and returns the number of rows changed. def apply(included_keys:, delete_keys: []) included = Array(included_keys).to_set - delete_instead = Array(delete_keys).to_set + deletes = Array(delete_keys).to_set changed = all_rows.count do |row| - row.actionable? && included.include?(row.key) && perform(row, delete_instead: delete_instead.include?(row.key)) + next false unless row.actionable? + + if deletes.include?(row.key) && row.affiliation + row.affiliation.destroy! + true + elsif included.include?(row.key) + perform(row) + else + false + end end @event.update!(affiliations_reconciled_at: Time.current) @@ -139,7 +148,7 @@ def create_row(person, registration, organization, attended) end end - def perform(row, delete_instead:) + def perform(row) case row.action when :create AffiliationServices::CreateFromRegistration.call( @@ -149,7 +158,9 @@ def perform(row, delete_instead:) when :delete row.affiliation.destroy! when :deactivate - delete_instead ? row.affiliation.destroy! : row.affiliation.update!(end_date: row.affiliation.start_date || Date.current) + # Same-day it: end_date = the affiliation's own start_date (start_date itself + # is never changed), which the model turns into inactive. + row.affiliation.update!(end_date: row.affiliation.start_date || Date.current) when :reactivate row.affiliation.update!(end_date: nil) end diff --git a/app/views/events/reconcile_affiliations/index.html.erb b/app/views/events/reconcile_affiliations/index.html.erb index cf3335c763..a0ced04a29 100644 --- a/app/views/events/reconcile_affiliations/index.html.erb +++ b/app/views/events/reconcile_affiliations/index.html.erb @@ -47,12 +47,12 @@
<% @person_groups.each do |group| %> <% checked_class = { - create: "peer-checked:bg-blue-50 peer-checked:text-blue-700 peer-checked:border-blue-300", - reactivate: "peer-checked:bg-green-50 peer-checked:text-green-700 peer-checked:border-green-300", - deactivate: "peer-checked:bg-red-50 peer-checked:text-red-700 peer-checked:border-red-300", - delete: "peer-checked:bg-red-50 peer-checked:text-red-700 peer-checked:border-red-300" + create: "has-[:checked]:bg-blue-50 has-[:checked]:text-blue-700 has-[:checked]:border-blue-300", + reactivate: "has-[:checked]:bg-green-50 has-[:checked]:text-green-700 has-[:checked]:border-green-300", + deactivate: "has-[:checked]:bg-red-50 has-[:checked]:text-red-700 has-[:checked]:border-red-300", + delete: "has-[:checked]:bg-red-50 has-[:checked]:text-red-700 has-[:checked]:border-red-300" } %> - <% button_base = "inline-flex items-center rounded-md border border-gray-300 bg-white px-3 py-1.5 text-xs font-medium text-gray-500 shadow-sm hover:bg-gray-50 peer-focus-visible:ring-2 peer-focus-visible:ring-blue-400" %> + <% button_base = "group relative inline-flex items-center gap-2 rounded-md border border-gray-300 bg-white px-3 py-1.5 text-xs font-medium text-gray-500 shadow-sm cursor-pointer hover:bg-gray-50" %>
@@ -80,18 +80,18 @@ <% else %> <%= row.organization.name %> <% end %> -
+ <%# Checkbox lives inside the button; has-[:checked] colors the whole button when selected. %> +
data-controller="exclusive-checkboxes"<% end %>> <% if row.action == :deactivate %> -
From 2996f6e2cec9e61ab7c6105607923838c06ea53b Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Fri, 14 Aug 2026 09:18:17 -0400 Subject: [PATCH 13/50] Two-step reconcile: Preview changes then confirmation screen 'Preview changes' now posts to a confirmation screen that shows exactly which affiliations get created/reactivated/deactivated/deleted (actioned rows only), with Go back to edit (selections restored) or Perform changes. Add per-row instructions under the action buttons and a header row with a warning that checked boxes change affiliations. New exclusive-checkboxes controller registered. Co-Authored-By: Claude Opus 4.8 (1M context) --- AGENTS.md | 4 +- .../reconcile_affiliations_controller.rb | 15 ++++++ .../affiliation_services/reconcile_event.rb | 17 ++++++ .../reconcile_affiliations/confirm.html.erb | 52 +++++++++++++++++++ .../reconcile_affiliations/index.html.erb | 44 +++++++++++----- config/routes.rb | 3 +- .../events/reconcile_affiliations_spec.rb | 35 ++++++++++--- spec/views/page_bg_class_alignment_spec.rb | 1 + 8 files changed, 148 insertions(+), 23 deletions(-) create mode 100644 app/views/events/reconcile_affiliations/confirm.html.erb diff --git a/AGENTS.md b/AGENTS.md index 4241a7f057..fbbe509b7f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -71,7 +71,7 @@ This codebase (Rails 8.1) | Directory | Purpose | |---|---| | `app/frontend/entrypoints/` | Vite entry points (application.js, application.css) | -| `app/frontend/javascript/controllers/` | Stimulus controllers (77) | +| `app/frontend/javascript/controllers/` | Stimulus controllers (78) | | `app/frontend/javascript/rhino/` | Rich text editor customizations (mentions, grid) | | `app/frontend/stylesheets/` | Tailwind CSS and component styles | @@ -265,7 +265,7 @@ action, or `authorize! :workshop, to: :summary?`). - `AffiliationServices::ApplyScenarioEndDating` — What an agreement scenario means for the person's *existing* affiliations, run by the linking core before creation (see ADR-0002): a **new_job** ends their active affiliations at other orgs (job + facilitator alike, sparing the linked org's rows); every other scenario ends nothing — reinstatement reconciles registration-style, creating affiliations only where no active one exists. Ends are dated the day before the agreement takes effect so the fresh affiliations (starting on the submission date) don't overlap a row still counting as active until end of day - `AffiliationServices::CreateFromRegistration` — On registration / org linking, creates a "job affiliation" with the typed title (when present) plus a standing "Facilitator" affiliation, in one transaction. Skips the facilitator one only when the person already has an active-or-pending affiliation titled exactly "Facilitator" with that org (a current one or one dated to a future training); an ended facilitator affiliation gets a fresh second one. Dedupe is by title + org + dates, so a job title like "Lead Facilitator" still gets its own Facilitator affiliation. Accepts an optional `organization_address:` and sets it on every affiliation it creates (the registrant's typed agency address, upserted onto the org); when an affiliation already exists and is skipped, it backfills that address onto the existing one only if it has none (an admin-set address is never overwritten) - `AffiliationServices::ReconcileFacilitatorAffiliation` — Per `(person, organization)`, brings the person's **owned** facilitator affiliation (created by the registration flow, i.e. `event_registration_id` present) in line with attendance: keeps it active when they have any `attended` facilitator-training registration to that org; otherwise **same-days** it (`end_date := start_date`, which the model turns into `inactive: true`) once its source training has ended. Reactivates a previously same-dayed row when the person is later marked attended. Hand-created (unowned) rows are never touched. `#plan` returns the action (`:deactivate` / `:reactivate` / `:noop`) without writing. -- `AffiliationServices::ReconcileEvent` — Event-level orchestration for the "Reconcile affiliations" bulk action. Builds one row **per facilitator affiliation** for an org a registrant linked — **hand-entered rows included, not just app-created ones** — classifying each `:create` / `:deactivate` / `:reactivate` (facilitator trainings) or `:delete` (facilitator affiliation auto-created off a non-training event), else `:noop` with a reason. Deactivation is gated to post-event (owned rows on their source training's `ended?`, hand-entered on this event's `ended?`), so a pre-event run never deactivates. `#actionable_person_groups` groups actionable rows by person (with attendance registration and other-org facilitator affiliations for context); `#skipped_reason_sections` groups no-action rows by reason. `#apply(included_keys:, delete_keys:)` performs the kept rows (keys are `aff:` or `create::`; a `:deactivate` row also in `delete_keys` is deleted instead of same-dayed) and stamps `affiliations_reconciled_at`. Job affiliations are never touched. +- `AffiliationServices::ReconcileEvent` — Event-level orchestration for the "Reconcile affiliations" bulk action. Builds one row **per facilitator affiliation** for an org a registrant linked — **hand-entered rows included, not just app-created ones** — classifying each `:create` / `:deactivate` / `:reactivate` (facilitator trainings) or `:delete` (facilitator affiliation auto-created off a non-training event), else `:noop` with a reason. Deactivation is gated to post-event (owned rows on their source training's `ended?`, hand-entered on this event's `ended?`), so a pre-event run never deactivates. `#actionable_person_groups` groups actionable rows by person (with attendance registration and other-org facilitator affiliations for context); `#skipped_reason_sections` groups no-action rows by reason. `#planned_changes(included_keys:, delete_keys:)` returns the concrete `Change`s for the confirmation screen; `#apply(included_keys:, delete_keys:)` performs the kept rows (keys are `aff:` or `create::`; a `delete_keys` entry deletes that affiliation instead of same-daying it) and stamps `affiliations_reconciled_at`. Job affiliations are never touched. The controller is a two-step flow: `index` (edit) → `confirm` (preview, no writes) → `create` (perform). ### Sectors diff --git a/app/controllers/events/reconcile_affiliations_controller.rb b/app/controllers/events/reconcile_affiliations_controller.rb index 85296e7de5..8c5f8d504b 100644 --- a/app/controllers/events/reconcile_affiliations_controller.rb +++ b/app/controllers/events/reconcile_affiliations_controller.rb @@ -17,9 +17,24 @@ def index @person_groups = reconcile.actionable_person_groups @skipped_sections = reconcile.skipped_reason_sections @has_rows = reconcile.any_rows? + # Restore the admin's selections when they come back from the confirm screen. + @pre_included = params[:included] + @pre_delete = Array(params[:delete]).to_set @event = @event.decorate end + # Step 2: show exactly what "Perform changes" will do (no writes yet). + def confirm + authorize! @event, to: :reconcile_affiliations? + + @included = Array(params[:included]) + @delete = Array(params[:delete]) + @changes = AffiliationServices::ReconcileEvent.new(@event).planned_changes(included_keys: @included, delete_keys: @delete) + @event = @event.decorate + + redirect_to reconcile_affiliations_event_path(@event), notice: "Nothing selected to change." and return if @changes.empty? + end + def create authorize! @event, to: :reconcile_affiliations? diff --git a/app/services/affiliation_services/reconcile_event.rb b/app/services/affiliation_services/reconcile_event.rb index 9bab55a538..2308b4718b 100644 --- a/app/services/affiliation_services/reconcile_event.rb +++ b/app/services/affiliation_services/reconcile_event.rb @@ -58,6 +58,23 @@ def any_rows? all_rows.any? end + Change = Struct.new(:person, :organization, :affiliation, :action, keyword_init: true) + + # The concrete changes the given selection will make, for the confirmation + # screen: a `:delete` key wins over its include (delete instead of same-day). + def planned_changes(included_keys:, delete_keys: []) + included = Array(included_keys).to_set + deletes = Array(delete_keys).to_set + + all_rows.select(&:actionable?).filter_map do |row| + if deletes.include?(row.key) && row.affiliation + Change.new(person: row.person, organization: row.organization, affiliation: row.affiliation, action: :delete) + elsif included.include?(row.key) + Change.new(person: row.person, organization: row.organization, affiliation: row.affiliation, action: row.action) + end + end + end + # Apply the actionable rows whose keys are in `included_keys`. For :deactivate # rows whose key is also in `delete_keys`, delete the affiliation instead of # same-daying it. Stamps the event and returns the number of rows changed. diff --git a/app/views/events/reconcile_affiliations/confirm.html.erb b/app/views/events/reconcile_affiliations/confirm.html.erb new file mode 100644 index 0000000000..de5d63cb35 --- /dev/null +++ b/app/views/events/reconcile_affiliations/confirm.html.erb @@ -0,0 +1,52 @@ +<% content_for(:page_title, "Confirm affiliation changes — #{@event.title}") %> +<% content_for(:page_bg_class, "admin-or-owner bg-blue-100") %> +
+
+ <%= link_to "← Go back to edit", reconcile_affiliations_event_path(@event, included: @included, delete: @delete), class: "text-sm text-gray-500 hover:text-gray-700" %> +
+ +

Confirm affiliation changes

+

+ Performing will make the <%= @changes.size %> <%= "change".pluralize(@changes.size) %> below. Nothing else is affected. +

+ + <% sections = { + create: [ "Create", "bg-blue-50 text-blue-800 border-blue-200", "A new facilitator affiliation is created for this organization." ], + reactivate: [ "Reactivate", "bg-green-50 text-green-800 border-green-200", "The end date is cleared so the facilitator affiliation is active again." ], + deactivate: [ "Deactivate", "bg-red-50 text-red-800 border-red-200", "The facilitator affiliation is same-dayed (ended today) so it no longer counts as active. Reversible." ], + delete: [ "Delete", "bg-red-100 text-red-900 border-red-300", "The facilitator affiliation is permanently deleted. Job and other-org affiliations are untouched." ] + } %> + +
+ <% sections.each do |action, (label, header_class, description)| %> + <% action_changes = @changes.select { |change| change.action == action } %> + <% next if action_changes.empty? %> +
+
+ <%= label %> (<%= action_changes.size %>) +

<%= description %>

+
+
    + <% action_changes.each do |change| %> +
  • + <%= change.person.name %> + — <%= change.organization.name %> + <% if change.affiliation %> + · <%= change.affiliation.decorate.date_range %> + <% end %> +
  • + <% end %> +
+
+ <% end %> +
+ +
+ <%= link_to "← Go back to edit", reconcile_affiliations_event_path(@event, included: @included, delete: @delete), class: "text-sm text-gray-500 hover:text-gray-700" %> + <%= form_with url: perform_reconcile_affiliations_event_path(@event), method: :post do %> + <% @included.each do |key| %><%= hidden_field_tag "included[]", key %><% end %> + <% @delete.each do |key| %><%= hidden_field_tag "delete[]", key %><% end %> + <%= submit_tag "Perform changes", class: "btn btn-primary" %> + <% end %> +
+
diff --git a/app/views/events/reconcile_affiliations/index.html.erb b/app/views/events/reconcile_affiliations/index.html.erb index a0ced04a29..976a0b900b 100644 --- a/app/views/events/reconcile_affiliations/index.html.erb +++ b/app/views/events/reconcile_affiliations/index.html.erb @@ -43,6 +43,13 @@ delete: [ "Will be deleted", "bg-red-100 text-red-800" ] } %> <% if @person_groups.any? %> + <% actionable_count = @person_groups.sum { |g| g[:rows].size } %> +
+

To reconcile (<%= actionable_count %>)

+ + Checked boxes change facilitator affiliations + +
<%= form_with url: reconcile_affiliations_event_path(@event), method: :post do %>
<% @person_groups.each do |group| %> @@ -52,6 +59,12 @@ deactivate: "has-[:checked]:bg-red-50 has-[:checked]:text-red-700 has-[:checked]:border-red-300", delete: "has-[:checked]:bg-red-50 has-[:checked]:text-red-700 has-[:checked]:border-red-300" } %> + <% action_notes = { + create: "Uncheck to skip creating this facilitator affiliation.", + reactivate: "Uncheck to leave this facilitator affiliation inactive.", + deactivate: "Change attendance to Attended, or uncheck, to keep this facilitator affiliation active.", + delete: "Uncheck to keep this facilitator affiliation." + } %> <% button_base = "group relative inline-flex items-center gap-2 rounded-md border border-gray-300 bg-white px-3 py-1.5 text-xs font-medium text-gray-500 shadow-sm cursor-pointer hover:bg-gray-50" %>
@@ -71,6 +84,8 @@
<% group[:rows].each do |row| %> <% heading, _badge_class = badges[row.action] %> + <% included_checked = @pre_included.nil? || @pre_included.include?(row.key) %> + <% delete_checked = @pre_delete.include?(row.key) %>
<% if row.affiliation %> <%= link_to edit_person_path(row.person, anchor: dom_id(row.affiliation)), target: "_blank", rel: "noopener", title: "Edit affiliation", class: "flex-1 min-w-0 text-sm hover:underline" do %> @@ -81,19 +96,22 @@ <%= row.organization.name %> <% end %> <%# Checkbox lives inside the button; has-[:checked] colors the whole button when selected. %> -
data-controller="exclusive-checkboxes"<% end %>> - <% if row.action == :deactivate %> -
<% end %> @@ -104,7 +122,7 @@
<%= link_to "Cancel", registrants_event_path(@event), class: "text-sm text-gray-500 hover:text-gray-700" %> - <%= submit_tag "Reconcile affiliations", class: "btn btn-primary" %> + <%= submit_tag "Preview changes", class: "btn btn-primary" %>
<% end %> <% end %> diff --git a/config/routes.rb b/config/routes.rb index 0208cc8d90..a0a33974b3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -215,7 +215,8 @@ post :feature_recipient_shoutout get :bulk_payments, to: "events/bulk_payments#index" get :reconcile_affiliations, to: "events/reconcile_affiliations#index" - post :reconcile_affiliations, to: "events/reconcile_affiliations#create" + post :reconcile_affiliations, to: "events/reconcile_affiliations#confirm" + post :perform_reconcile_affiliations, to: "events/reconcile_affiliations#create" get :preview_reminder patch :preview post :copy_registration_form diff --git a/spec/requests/events/reconcile_affiliations_spec.rb b/spec/requests/events/reconcile_affiliations_spec.rb index 71ee917a31..f13b7a4385 100644 --- a/spec/requests/events/reconcile_affiliations_spec.rb +++ b/spec/requests/events/reconcile_affiliations_spec.rb @@ -85,12 +85,33 @@ def registrant_with_affiliation(status:) end end - describe "POST create" do - it "deactivates the included non-completer and stamps the event" do + describe "POST confirm (preview changes)" do + it "shows the selected change without writing" do _person, affiliation = registrant_with_affiliation(status: "no_show") post reconcile_affiliations_event_path(event), params: { included: [ "aff:#{affiliation.id}" ] } + expect(response).to have_http_status(:ok) + expect(response.body).to include("Confirm affiliation changes") + expect(response.body).to include("Perform changes") + expect(affiliation.reload).to be_active + end + + it "redirects back when nothing is selected" do + registrant_with_affiliation(status: "no_show") + + post reconcile_affiliations_event_path(event), params: { included: [] } + + expect(response).to redirect_to(reconcile_affiliations_event_path(event)) + end + end + + describe "POST perform" do + it "deactivates the included non-completer and stamps the event" do + _person, affiliation = registrant_with_affiliation(status: "no_show") + + post perform_reconcile_affiliations_event_path(event), params: { included: [ "aff:#{affiliation.id}" ] } + expect(response).to redirect_to(registrants_event_path(event)) expect(affiliation.reload).not_to be_active expect(event.reload.affiliations_reconciled_at).to be_present @@ -99,7 +120,7 @@ def registrant_with_affiliation(status:) it "spares an opted-out row" do _person, affiliation = registrant_with_affiliation(status: "no_show") - post reconcile_affiliations_event_path(event), params: { included: [] } + post perform_reconcile_affiliations_event_path(event), params: { included: [] } expect(affiliation.reload).to be_active end @@ -111,7 +132,7 @@ def registrant_with_affiliation(status:) create(:event_registration_organization, event_registration: reg, organization: organization) expect { - post reconcile_affiliations_event_path(upcoming), params: { included: [ "create:#{person.id}:#{organization.id}" ] } + post perform_reconcile_affiliations_event_path(upcoming), params: { included: [ "create:#{person.id}:#{organization.id}" ] } }.to change { person.affiliations.facilitators.where(organization: organization).count }.by(1) end @@ -119,7 +140,7 @@ def registrant_with_affiliation(status:) _person, affiliation = registrant_with_affiliation(status: "no_show") key = "aff:#{affiliation.id}" - post reconcile_affiliations_event_path(event), params: { included: [ key ], delete: [ key ] } + post perform_reconcile_affiliations_event_path(event), params: { included: [ key ], delete: [ key ] } expect(Affiliation.exists?(affiliation.id)).to be(false) end @@ -130,7 +151,7 @@ def registrant_with_affiliation(status:) create(:event_registration_organization, event_registration: reg, organization: organization) hand_entered = create(:affiliation, person: person, organization: organization, title: "Facilitator", start_date: 1.year.ago.to_date) - post reconcile_affiliations_event_path(event), params: { included: [ "aff:#{hand_entered.id}" ] } + post perform_reconcile_affiliations_event_path(event), params: { included: [ "aff:#{hand_entered.id}" ] } expect(hand_entered.reload).not_to be_active end @@ -145,7 +166,7 @@ def registrant_with_affiliation(status:) job = create(:affiliation, person: person, organization: organization, title: "Counselor", event_registration: reg) - post reconcile_affiliations_event_path(non_training), params: { included: [ "aff:#{facilitator.id}" ] } + post perform_reconcile_affiliations_event_path(non_training), params: { included: [ "aff:#{facilitator.id}" ] } expect(Affiliation.exists?(facilitator.id)).to be(false) expect(Affiliation.exists?(job.id)).to be(true) diff --git a/spec/views/page_bg_class_alignment_spec.rb b/spec/views/page_bg_class_alignment_spec.rb index a3f5941898..7e0cec317c 100644 --- a/spec/views/page_bg_class_alignment_spec.rb +++ b/spec/views/page_bg_class_alignment_spec.rb @@ -130,6 +130,7 @@ "app/views/events/sample_ticket.html.erb" => "admin-or-owner bg-blue-100", "app/views/events/bulk_payments/index.html.erb" => "admin-or-owner bg-blue-100", "app/views/events/reconcile_affiliations/index.html.erb" => "admin-or-owner bg-blue-100", + "app/views/events/reconcile_affiliations/confirm.html.erb" => "admin-or-owner bg-blue-100", "app/views/events/edit_staff.html.erb" => "admin-or-owner bg-blue-100", "app/views/events/recipients.html.erb" => "admin-or-owner bg-blue-100", "app/views/events/registrants.html.erb" => "admin-or-owner bg-blue-100", From ed4a8969527cba320943c452574e798461a4af45 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Fri, 14 Aug 2026 09:25:20 -0400 Subject: [PATCH 14/50] Keep attendance toggle on the reconcile page with a flash Thread return_to through the attendance status badge and add a reconcile case to EventRegistrations#update so toggling attendance from the reconcile page reloads it (with fresh attendance) and a success flash, instead of jumping to the roster. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/controllers/event_registrations_controller.rb | 1 + .../_attendance_status_badge.html.erb | 2 +- .../events/reconcile_affiliations/index.html.erb | 4 ++-- spec/requests/events/reconcile_affiliations_spec.rb | 13 +++++++++++++ 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/app/controllers/event_registrations_controller.rb b/app/controllers/event_registrations_controller.rb index 9e656a5eb2..a9a2085527 100644 --- a/app/controllers/event_registrations_controller.rb +++ b/app/controllers/event_registrations_controller.rb @@ -117,6 +117,7 @@ def update when "onboarding" then redirect_to helpers.onboarding_event_row_path(@event_registration.event, @event_registration.id), notice: notice, status: :see_other when "attendees" then redirect_to attendees_events_path, notice: notice, status: :see_other when "roster" then redirect_to roster_event_path(@event_registration.event), notice: notice, status: :see_other + when "reconcile_affiliations" then redirect_to reconcile_affiliations_event_path(@event_registration.event), notice: notice, status: :see_other # Two ways back to the recipients page: the shout-outs section (the # feature-a-shout-out flow) or the recipient's own card (their name). when "recipients" then redirect_to recipients_event_path(@event_registration.event, anchor: "shout-outs"), notice: notice, status: :see_other diff --git a/app/views/event_registrations/_attendance_status_badge.html.erb b/app/views/event_registrations/_attendance_status_badge.html.erb index da29575688..5b8e5b9315 100644 --- a/app/views/event_registrations/_attendance_status_badge.html.erb +++ b/app/views/event_registrations/_attendance_status_badge.html.erb @@ -1,7 +1,7 @@ <% deco = registration.decorate %> <% badge_return_to = local_assigns.fetch(:return_to, nil) %>
- <%= form_with model: registration, url: event_registration_path(registration), method: :patch, data: { turbo_frame: "_top" } do |f| %> + <%= form_with model: registration, url: event_registration_path(registration, return_to: badge_return_to), method: :patch, data: { turbo_frame: "_top" } do |f| %>
<%= f.select :status, diff --git a/app/views/events/reconcile_affiliations/index.html.erb b/app/views/events/reconcile_affiliations/index.html.erb index 976a0b900b..0f5826f2d0 100644 --- a/app/views/events/reconcile_affiliations/index.html.erb +++ b/app/views/events/reconcile_affiliations/index.html.erb @@ -78,7 +78,7 @@ title: note, class: "text-xs text-gray-500 truncate hover:underline hover:text-blue-700 min-w-0" %> <% end %>
- <%= render "event_registrations/attendance_status_badge", registration: group[:registration] %> + <%= render "event_registrations/attendance_status_badge", registration: group[:registration], return_to: "reconcile_affiliations" %>
@@ -154,7 +154,7 @@ <%= row.organization.name %> <% end %> - <%= render "event_registrations/attendance_status_badge", registration: row.registration %> + <%= render "event_registrations/attendance_status_badge", registration: row.registration, return_to: "reconcile_affiliations" %>
<% end %>
diff --git a/spec/requests/events/reconcile_affiliations_spec.rb b/spec/requests/events/reconcile_affiliations_spec.rb index f13b7a4385..c8c94613a3 100644 --- a/spec/requests/events/reconcile_affiliations_spec.rb +++ b/spec/requests/events/reconcile_affiliations_spec.rb @@ -85,6 +85,19 @@ def registrant_with_affiliation(status:) end end + describe "toggling attendance from the reconcile page" do + it "stays on the reconcile page with a flash instead of leaving for the roster" do + person, _affiliation = registrant_with_affiliation(status: "no_show") + registration = person.event_registrations.first + + patch event_registration_path(registration, return_to: "reconcile_affiliations"), + params: { event_registration: { status: "attended" } } + + expect(response).to redirect_to(reconcile_affiliations_event_path(event)) + expect(flash[:notice]).to be_present + end + end + describe "POST confirm (preview changes)" do it "shows the selected change without writing" do _person, affiliation = registrant_with_affiliation(status: "no_show") From 0ad3a5ab38808fc56efc9d64800d4daf1e6fcb7d Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Fri, 14 Aug 2026 09:29:27 -0400 Subject: [PATCH 15/50] Register exclusive-checkboxes controller; clearer deactivate instruction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Stimulus manifest is explicit, so the new exclusive-checkboxes controller was never loaded — register it and use an explicit change event so Delete instead and Will be deactivated actually clear each other. Reword the deactivate row note to spell out the two options (mark Attended = permanent, uncheck = one-time). Co-Authored-By: Claude Opus 4.8 (1M context) --- app/frontend/javascript/controllers/index.js | 3 +++ app/views/events/reconcile_affiliations/index.html.erb | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/frontend/javascript/controllers/index.js b/app/frontend/javascript/controllers/index.js index 64be221b9e..9cc001dc63 100644 --- a/app/frontend/javascript/controllers/index.js +++ b/app/frontend/javascript/controllers/index.js @@ -84,6 +84,9 @@ application.register("dropdown", DropdownController) import ExpandAllController from "./expand_all_controller" application.register("expand-all", ExpandAllController) +import ExclusiveCheckboxesController from "./exclusive_checkboxes_controller" +application.register("exclusive-checkboxes", ExclusiveCheckboxesController) + import FilePreviewController from "./file_preview_controller" application.register("file-preview", FilePreviewController) diff --git a/app/views/events/reconcile_affiliations/index.html.erb b/app/views/events/reconcile_affiliations/index.html.erb index 0f5826f2d0..727ed2485e 100644 --- a/app/views/events/reconcile_affiliations/index.html.erb +++ b/app/views/events/reconcile_affiliations/index.html.erb @@ -62,7 +62,7 @@ <% action_notes = { create: "Uncheck to skip creating this facilitator affiliation.", reactivate: "Uncheck to leave this facilitator affiliation inactive.", - deactivate: "Change attendance to Attended, or uncheck, to keep this facilitator affiliation active.", + deactivate: "To keep this facilitator affiliation active: mark them Attended (permanent), or just uncheck this box (one-time).", delete: "Uncheck to keep this facilitator affiliation." } %> <% button_base = "group relative inline-flex items-center gap-2 rounded-md border border-gray-300 bg-white px-3 py-1.5 text-xs font-medium text-gray-500 shadow-sm cursor-pointer hover:bg-gray-50" %> @@ -100,13 +100,13 @@
data-controller="exclusive-checkboxes"<% end %>> <% if row.action == :deactivate %> <% end %> From f610f507522952918021f413f50ae337333662e2 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Fri, 14 Aug 2026 09:31:45 -0400 Subject: [PATCH 16/50] Render deactivate row note as two lines --- .../events/reconcile_affiliations/index.html.erb | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/app/views/events/reconcile_affiliations/index.html.erb b/app/views/events/reconcile_affiliations/index.html.erb index 727ed2485e..f8de489727 100644 --- a/app/views/events/reconcile_affiliations/index.html.erb +++ b/app/views/events/reconcile_affiliations/index.html.erb @@ -60,10 +60,10 @@ delete: "has-[:checked]:bg-red-50 has-[:checked]:text-red-700 has-[:checked]:border-red-300" } %> <% action_notes = { - create: "Uncheck to skip creating this facilitator affiliation.", - reactivate: "Uncheck to leave this facilitator affiliation inactive.", - deactivate: "To keep this facilitator affiliation active: mark them Attended (permanent), or just uncheck this box (one-time).", - delete: "Uncheck to keep this facilitator affiliation." + create: [ "Uncheck to skip creating this facilitator affiliation." ], + reactivate: [ "Uncheck to leave this facilitator affiliation inactive." ], + deactivate: [ "Mark Attended to keep active (permanent),", "or uncheck this box to keep active (one-time)." ], + delete: [ "Uncheck to keep this facilitator affiliation." ] } %> <% button_base = "group relative inline-flex items-center gap-2 rounded-md border border-gray-300 bg-white px-3 py-1.5 text-xs font-medium text-gray-500 shadow-sm cursor-pointer hover:bg-gray-50" %>
@@ -111,7 +111,11 @@ <%= render "tooltip", kind: row.action %>
-

<%= action_notes[row.action] %>

+
+ <% action_notes[row.action].each do |line| %> +

<%= line %>

+ <% end %> +
<% end %> From 18b25174c3294788f6aa3c93e60096036dfb7089 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Fri, 14 Aug 2026 09:33:01 -0400 Subject: [PATCH 17/50] Reword deactivate note to 'To keep Affiliation active: Mark as Attended or uncheck this box' --- app/views/events/reconcile_affiliations/index.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/events/reconcile_affiliations/index.html.erb b/app/views/events/reconcile_affiliations/index.html.erb index f8de489727..a4aadc9613 100644 --- a/app/views/events/reconcile_affiliations/index.html.erb +++ b/app/views/events/reconcile_affiliations/index.html.erb @@ -62,7 +62,7 @@ <% action_notes = { create: [ "Uncheck to skip creating this facilitator affiliation." ], reactivate: [ "Uncheck to leave this facilitator affiliation inactive." ], - deactivate: [ "Mark Attended to keep active (permanent),", "or uncheck this box to keep active (one-time)." ], + deactivate: [ "To keep Affiliation active:", "Mark as Attended or uncheck this box" ], delete: [ "Uncheck to keep this facilitator affiliation." ] } %> <% button_base = "group relative inline-flex items-center gap-2 rounded-md border border-gray-300 bg-white px-3 py-1.5 text-xs font-medium text-gray-500 shadow-sm cursor-pointer hover:bg-gray-50" %> From c11f15c8ab5445a0caad73cc3d650f6eb70fa945 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Fri, 14 Aug 2026 09:34:54 -0400 Subject: [PATCH 18/50] Fix Preview changes: turbo:false so the confirm page renders on POST; note says 'both boxes' Turbo ignores a 200 HTML render on a form POST (only 4xx/5xx render), so the confirmation screen never showed. Submit the preview form with turbo disabled. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/views/events/reconcile_affiliations/index.html.erb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/views/events/reconcile_affiliations/index.html.erb b/app/views/events/reconcile_affiliations/index.html.erb index a4aadc9613..36b2303630 100644 --- a/app/views/events/reconcile_affiliations/index.html.erb +++ b/app/views/events/reconcile_affiliations/index.html.erb @@ -50,7 +50,8 @@ Checked boxes change facilitator affiliations
- <%= form_with url: reconcile_affiliations_event_path(@event), method: :post do %> + <%# turbo: false so the POST renders the confirmation page (Turbo ignores a 200 HTML render on POST). %> + <%= form_with url: reconcile_affiliations_event_path(@event), method: :post, data: { turbo: false } do %>
<% @person_groups.each do |group| %> <% checked_class = { @@ -62,7 +63,7 @@ <% action_notes = { create: [ "Uncheck to skip creating this facilitator affiliation." ], reactivate: [ "Uncheck to leave this facilitator affiliation inactive." ], - deactivate: [ "To keep Affiliation active:", "Mark as Attended or uncheck this box" ], + deactivate: [ "To keep Affiliation active:", "Mark as Attended or uncheck both boxes" ], delete: [ "Uncheck to keep this facilitator affiliation." ] } %> <% button_base = "group relative inline-flex items-center gap-2 rounded-md border border-gray-300 bg-white px-3 py-1.5 text-xs font-medium text-gray-500 shadow-sm cursor-pointer hover:bg-gray-50" %> From 6524f7fd3f76b4ac3696ce7dd57a9177ad99979f Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Fri, 14 Aug 2026 09:53:05 -0400 Subject: [PATCH 19/50] Radio outcomes with Keep-active; fix nested-form bug; scroll to item on attendance toggle Replace the deactivate/delete checkboxes with a radio group per row (Deactivate/Delete/Keep active, and action/keep for the others), styled as the same buttons via has-[:checked]. Radios are natively mutually exclusive, so remove the exclusive-checkboxes Stimulus controller and the per-row instruction note. Fix the real reason 'Preview changes' did nothing: the attendance chip's form was nested inside the reconcile form (invalid HTML), so the submit/inputs fell outside it. Render the reconcile form standalone and join the radios/submit via the HTML form= attribute. Switch the params to an outcome map { row.key => choice }. Toggling attendance now scrolls back to that item's anchor, not the top. Co-Authored-By: Claude Opus 4.8 (1M context) --- AGENTS.md | 4 +- .../event_registrations_controller.rb | 2 +- .../reconcile_affiliations_controller.rb | 22 +-- .../exclusive_checkboxes_controller.js | 17 -- app/frontend/javascript/controllers/index.js | 3 - .../affiliation_services/reconcile_event.rb | 57 +++---- .../_attendance_status_badge.html.erb | 2 +- .../reconcile_affiliations/_tooltip.html.erb | 2 + .../reconcile_affiliations/confirm.html.erb | 7 +- .../reconcile_affiliations/index.html.erb | 147 ++++++++---------- .../events/reconcile_affiliations_spec.rb | 31 ++-- 11 files changed, 127 insertions(+), 167 deletions(-) delete mode 100644 app/frontend/javascript/controllers/exclusive_checkboxes_controller.js diff --git a/AGENTS.md b/AGENTS.md index fbbe509b7f..c2edcf6f22 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -71,7 +71,7 @@ This codebase (Rails 8.1) | Directory | Purpose | |---|---| | `app/frontend/entrypoints/` | Vite entry points (application.js, application.css) | -| `app/frontend/javascript/controllers/` | Stimulus controllers (78) | +| `app/frontend/javascript/controllers/` | Stimulus controllers (77) | | `app/frontend/javascript/rhino/` | Rich text editor customizations (mentions, grid) | | `app/frontend/stylesheets/` | Tailwind CSS and component styles | @@ -265,7 +265,7 @@ action, or `authorize! :workshop, to: :summary?`). - `AffiliationServices::ApplyScenarioEndDating` — What an agreement scenario means for the person's *existing* affiliations, run by the linking core before creation (see ADR-0002): a **new_job** ends their active affiliations at other orgs (job + facilitator alike, sparing the linked org's rows); every other scenario ends nothing — reinstatement reconciles registration-style, creating affiliations only where no active one exists. Ends are dated the day before the agreement takes effect so the fresh affiliations (starting on the submission date) don't overlap a row still counting as active until end of day - `AffiliationServices::CreateFromRegistration` — On registration / org linking, creates a "job affiliation" with the typed title (when present) plus a standing "Facilitator" affiliation, in one transaction. Skips the facilitator one only when the person already has an active-or-pending affiliation titled exactly "Facilitator" with that org (a current one or one dated to a future training); an ended facilitator affiliation gets a fresh second one. Dedupe is by title + org + dates, so a job title like "Lead Facilitator" still gets its own Facilitator affiliation. Accepts an optional `organization_address:` and sets it on every affiliation it creates (the registrant's typed agency address, upserted onto the org); when an affiliation already exists and is skipped, it backfills that address onto the existing one only if it has none (an admin-set address is never overwritten) - `AffiliationServices::ReconcileFacilitatorAffiliation` — Per `(person, organization)`, brings the person's **owned** facilitator affiliation (created by the registration flow, i.e. `event_registration_id` present) in line with attendance: keeps it active when they have any `attended` facilitator-training registration to that org; otherwise **same-days** it (`end_date := start_date`, which the model turns into `inactive: true`) once its source training has ended. Reactivates a previously same-dayed row when the person is later marked attended. Hand-created (unowned) rows are never touched. `#plan` returns the action (`:deactivate` / `:reactivate` / `:noop`) without writing. -- `AffiliationServices::ReconcileEvent` — Event-level orchestration for the "Reconcile affiliations" bulk action. Builds one row **per facilitator affiliation** for an org a registrant linked — **hand-entered rows included, not just app-created ones** — classifying each `:create` / `:deactivate` / `:reactivate` (facilitator trainings) or `:delete` (facilitator affiliation auto-created off a non-training event), else `:noop` with a reason. Deactivation is gated to post-event (owned rows on their source training's `ended?`, hand-entered on this event's `ended?`), so a pre-event run never deactivates. `#actionable_person_groups` groups actionable rows by person (with attendance registration and other-org facilitator affiliations for context); `#skipped_reason_sections` groups no-action rows by reason. `#planned_changes(included_keys:, delete_keys:)` returns the concrete `Change`s for the confirmation screen; `#apply(included_keys:, delete_keys:)` performs the kept rows (keys are `aff:` or `create::`; a `delete_keys` entry deletes that affiliation instead of same-daying it) and stamps `affiliations_reconciled_at`. Job affiliations are never touched. The controller is a two-step flow: `index` (edit) → `confirm` (preview, no writes) → `create` (perform). +- `AffiliationServices::ReconcileEvent` — Event-level orchestration for the "Reconcile affiliations" bulk action. Builds one row **per facilitator affiliation** for an org a registrant linked — **hand-entered rows included, not just app-created ones** — classifying each `:create` / `:deactivate` / `:reactivate` (facilitator trainings) or `:delete` (facilitator affiliation auto-created off a non-training event), else `:noop` with a reason. Deactivation is gated to post-event (owned rows on their source training's `ended?`, hand-entered on this event's `ended?`), so a pre-event run never deactivates. `#actionable_person_groups` groups actionable rows by person (with attendance registration and other-org facilitator affiliations for context); `#skipped_reason_sections` groups no-action rows by reason. `#planned_changes(outcome:)` and `#apply(outcome:)` take an `outcome` map `{ row.key => choice }` (choice is the action or "keep") — the confirm screen previews planned `Change`s, apply performs them and stamps `affiliations_reconciled_at`. Job affiliations are never touched. The controller is a two-step flow: `index` (edit) → `confirm` (preview, no writes) → `create` (perform). ### Sectors diff --git a/app/controllers/event_registrations_controller.rb b/app/controllers/event_registrations_controller.rb index a9a2085527..dc00118fa0 100644 --- a/app/controllers/event_registrations_controller.rb +++ b/app/controllers/event_registrations_controller.rb @@ -117,7 +117,7 @@ def update when "onboarding" then redirect_to helpers.onboarding_event_row_path(@event_registration.event, @event_registration.id), notice: notice, status: :see_other when "attendees" then redirect_to attendees_events_path, notice: notice, status: :see_other when "roster" then redirect_to roster_event_path(@event_registration.event), notice: notice, status: :see_other - when "reconcile_affiliations" then redirect_to reconcile_affiliations_event_path(@event_registration.event), notice: notice, status: :see_other + when "reconcile_affiliations" then redirect_to reconcile_affiliations_event_path(@event_registration.event, anchor: helpers.dom_id(@event_registration, :attendance_status)), notice: notice, status: :see_other # Two ways back to the recipients page: the shout-outs section (the # feature-a-shout-out flow) or the recipient's own card (their name). when "recipients" then redirect_to recipients_event_path(@event_registration.event, anchor: "shout-outs"), notice: notice, status: :see_other diff --git a/app/controllers/events/reconcile_affiliations_controller.rb b/app/controllers/events/reconcile_affiliations_controller.rb index 8c5f8d504b..d434e1188a 100644 --- a/app/controllers/events/reconcile_affiliations_controller.rb +++ b/app/controllers/events/reconcile_affiliations_controller.rb @@ -17,9 +17,8 @@ def index @person_groups = reconcile.actionable_person_groups @skipped_sections = reconcile.skipped_reason_sections @has_rows = reconcile.any_rows? - # Restore the admin's selections when they come back from the confirm screen. - @pre_included = params[:included] - @pre_delete = Array(params[:delete]).to_set + # Restore the admin's per-row radio choices when they come back from confirm. + @pre_outcome = params[:outcome] @event = @event.decorate end @@ -27,9 +26,8 @@ def index def confirm authorize! @event, to: :reconcile_affiliations? - @included = Array(params[:included]) - @delete = Array(params[:delete]) - @changes = AffiliationServices::ReconcileEvent.new(@event).planned_changes(included_keys: @included, delete_keys: @delete) + @outcome = outcome_params + @changes = AffiliationServices::ReconcileEvent.new(@event).planned_changes(outcome: @outcome) @event = @event.decorate redirect_to reconcile_affiliations_event_path(@event), notice: "Nothing selected to change." and return if @changes.empty? @@ -38,10 +36,7 @@ def confirm def create authorize! @event, to: :reconcile_affiliations? - changed = AffiliationServices::ReconcileEvent.new(@event).apply( - included_keys: params[:included] || [], - delete_keys: params[:delete] || [] - ) + changed = AffiliationServices::ReconcileEvent.new(@event).apply(outcome: outcome_params) redirect_to registrants_event_path(@event), notice: reconcile_notice(changed) end @@ -51,6 +46,13 @@ def set_event @event = Event.find(params[:id]) end + # `outcome` is a { row.key => choice } map with dynamic keys; the service only + # acts on known choices, so the actual values are validated downstream. + def outcome_params + outcome = params[:outcome] + outcome.respond_to?(:permit!) ? outcome.permit!.to_h : {} + end + def reconcile_notice(changed) return "No affiliations needed reconciling." if changed.zero? diff --git a/app/frontend/javascript/controllers/exclusive_checkboxes_controller.js b/app/frontend/javascript/controllers/exclusive_checkboxes_controller.js deleted file mode 100644 index cd165d589f..0000000000 --- a/app/frontend/javascript/controllers/exclusive_checkboxes_controller.js +++ /dev/null @@ -1,17 +0,0 @@ -import { Controller } from "@hotwired/stimulus" - -// Connects to data-controller="exclusive-checkboxes" -// Makes a small group of checkboxes mutually exclusive — like radios, but any can -// be left unchecked. Checking one clears the others in the group (e.g. "Delete -// instead" and "Will be deactivated" are two choices for the same row). -export default class extends Controller { - static targets = ["box"] - - select(event) { - if (!event.target.checked) return - - this.boxTargets.forEach((box) => { - if (box !== event.target) box.checked = false - }) - } -} diff --git a/app/frontend/javascript/controllers/index.js b/app/frontend/javascript/controllers/index.js index 9cc001dc63..64be221b9e 100644 --- a/app/frontend/javascript/controllers/index.js +++ b/app/frontend/javascript/controllers/index.js @@ -84,9 +84,6 @@ application.register("dropdown", DropdownController) import ExpandAllController from "./expand_all_controller" application.register("expand-all", ExpandAllController) -import ExclusiveCheckboxesController from "./exclusive_checkboxes_controller" -application.register("exclusive-checkboxes", ExclusiveCheckboxesController) - import FilePreviewController from "./file_preview_controller" application.register("file-preview", FilePreviewController) diff --git a/app/services/affiliation_services/reconcile_event.rb b/app/services/affiliation_services/reconcile_event.rb index 2308b4718b..dffa159305 100644 --- a/app/services/affiliation_services/reconcile_event.rb +++ b/app/services/affiliation_services/reconcile_event.rb @@ -60,39 +60,30 @@ def any_rows? Change = Struct.new(:person, :organization, :affiliation, :action, keyword_init: true) - # The concrete changes the given selection will make, for the confirmation - # screen: a `:delete` key wins over its include (delete instead of same-day). - def planned_changes(included_keys:, delete_keys: []) - included = Array(included_keys).to_set - deletes = Array(delete_keys).to_set + # Each row's outcome is one radio choice keyed by row.key: the action itself + # (deactivate/delete/reactivate/create) or "keep" (do nothing). + ACTION_FOR_CHOICE = { "deactivate" => :deactivate, "delete" => :delete, "reactivate" => :reactivate, "create" => :create }.freeze + + # The concrete changes the given `outcome` map will make, for the confirmation + # screen. `outcome` is `{ row.key => choice }`. + def planned_changes(outcome:) + outcome = outcome.to_h all_rows.select(&:actionable?).filter_map do |row| - if deletes.include?(row.key) && row.affiliation - Change.new(person: row.person, organization: row.organization, affiliation: row.affiliation, action: :delete) - elsif included.include?(row.key) - Change.new(person: row.person, organization: row.organization, affiliation: row.affiliation, action: row.action) - end + action = ACTION_FOR_CHOICE[outcome[row.key]] + next unless action + + Change.new(person: row.person, organization: row.organization, affiliation: row.affiliation, action:) end end - # Apply the actionable rows whose keys are in `included_keys`. For :deactivate - # rows whose key is also in `delete_keys`, delete the affiliation instead of - # same-daying it. Stamps the event and returns the number of rows changed. - def apply(included_keys:, delete_keys: []) - included = Array(included_keys).to_set - deletes = Array(delete_keys).to_set + # Apply each row's chosen outcome, stamp the event, and return the number of + # rows actually changed ("keep"/unknown choices are no-ops). + def apply(outcome:) + outcome = outcome.to_h changed = all_rows.count do |row| - next false unless row.actionable? - - if deletes.include?(row.key) && row.affiliation - row.affiliation.destroy! - true - elsif included.include?(row.key) - perform(row) - else - false - end + row.actionable? && perform_outcome(row, outcome[row.key]) end @event.update!(affiliations_reconciled_at: Time.current) @@ -165,21 +156,23 @@ def create_row(person, registration, organization, attended) end end - def perform(row) - case row.action - when :create + def perform_outcome(row, choice) + case choice + when "create" AffiliationServices::CreateFromRegistration.call( person: row.person, organization: row.organization, facilitator_training: true, training_date: @event.start_date, event_registration: row.registration ) - when :delete + when "delete" row.affiliation.destroy! - when :deactivate + when "deactivate" # Same-day it: end_date = the affiliation's own start_date (start_date itself # is never changed), which the model turns into inactive. row.affiliation.update!(end_date: row.affiliation.start_date || Date.current) - when :reactivate + when "reactivate" row.affiliation.update!(end_date: nil) + else + return false # "keep" or unknown end true end diff --git a/app/views/event_registrations/_attendance_status_badge.html.erb b/app/views/event_registrations/_attendance_status_badge.html.erb index 5b8e5b9315..a9f40128e6 100644 --- a/app/views/event_registrations/_attendance_status_badge.html.erb +++ b/app/views/event_registrations/_attendance_status_badge.html.erb @@ -1,6 +1,6 @@ <% deco = registration.decorate %> <% badge_return_to = local_assigns.fetch(:return_to, nil) %> -
+
<%= form_with model: registration, url: event_registration_path(registration, return_to: badge_return_to), method: :patch, data: { turbo_frame: "_top" } do |f| %>
diff --git a/app/views/events/reconcile_affiliations/_tooltip.html.erb b/app/views/events/reconcile_affiliations/_tooltip.html.erb index 582555db3a..a41b752833 100644 --- a/app/views/events/reconcile_affiliations/_tooltip.html.erb +++ b/app/views/events/reconcile_affiliations/_tooltip.html.erb @@ -13,5 +13,7 @@ Creates the facilitator affiliation for this organization. <% when :reactivate %> Clears the end date so this facilitator affiliation counts as active again. + <% when :keep %> + Leaves this facilitator affiliation exactly as it is — no change. <% end %> diff --git a/app/views/events/reconcile_affiliations/confirm.html.erb b/app/views/events/reconcile_affiliations/confirm.html.erb index de5d63cb35..885b8287d2 100644 --- a/app/views/events/reconcile_affiliations/confirm.html.erb +++ b/app/views/events/reconcile_affiliations/confirm.html.erb @@ -2,7 +2,7 @@ <% content_for(:page_bg_class, "admin-or-owner bg-blue-100") %>
- <%= link_to "← Go back to edit", reconcile_affiliations_event_path(@event, included: @included, delete: @delete), class: "text-sm text-gray-500 hover:text-gray-700" %> + <%= link_to "← Go back to edit", reconcile_affiliations_event_path(@event, outcome: @outcome), class: "text-sm text-gray-500 hover:text-gray-700" %>

Confirm affiliation changes

@@ -42,10 +42,9 @@
- <%= link_to "← Go back to edit", reconcile_affiliations_event_path(@event, included: @included, delete: @delete), class: "text-sm text-gray-500 hover:text-gray-700" %> + <%= link_to "← Go back to edit", reconcile_affiliations_event_path(@event, outcome: @outcome), class: "text-sm text-gray-500 hover:text-gray-700" %> <%= form_with url: perform_reconcile_affiliations_event_path(@event), method: :post do %> - <% @included.each do |key| %><%= hidden_field_tag "included[]", key %><% end %> - <% @delete.each do |key| %><%= hidden_field_tag "delete[]", key %><% end %> + <% @outcome.each do |key, value| %><%= hidden_field_tag "outcome[#{key}]", value %><% end %> <%= submit_tag "Perform changes", class: "btn btn-primary" %> <% end %>
diff --git a/app/views/events/reconcile_affiliations/index.html.erb b/app/views/events/reconcile_affiliations/index.html.erb index 36b2303630..490ddda643 100644 --- a/app/views/events/reconcile_affiliations/index.html.erb +++ b/app/views/events/reconcile_affiliations/index.html.erb @@ -37,99 +37,84 @@
<% end %> - <% badges = { create: [ "Will be created", "bg-blue-50 text-blue-700" ], - reactivate: [ "Will be reactivated", "bg-green-50 text-green-700" ], - deactivate: [ "Will be deactivated", "bg-red-50 text-red-700" ], - delete: [ "Will be deleted", "bg-red-100 text-red-800" ] } %> - <% if @person_groups.any? %> <% actionable_count = @person_groups.sum { |g| g[:rows].size } %> + <% color_class = { + red: "has-[:checked]:bg-red-50 has-[:checked]:text-red-700 has-[:checked]:border-red-300", + green: "has-[:checked]:bg-green-50 has-[:checked]:text-green-700 has-[:checked]:border-green-300", + blue: "has-[:checked]:bg-blue-50 has-[:checked]:text-blue-700 has-[:checked]:border-blue-300", + gray: "has-[:checked]:bg-gray-100 has-[:checked]:text-gray-800 has-[:checked]:border-gray-400" + } %> + <% outcome_options = { + create: [ [ "create", "Will be created", :blue ], [ "keep", "Skip", :gray ] ], + reactivate: [ [ "reactivate", "Will be reactivated", :green ], [ "keep", "Leave inactive", :gray ] ], + deactivate: [ [ "deactivate", "Will be deactivated", :red ], [ "delete", "Delete instead", :red ], [ "keep", "Keep active", :green ] ], + delete: [ [ "delete", "Will be deleted", :red ], [ "keep", "Keep", :gray ] ] + } %> + <% button_base = "group relative inline-flex items-center gap-2 rounded-md border border-gray-300 bg-white px-3 py-1.5 text-xs font-medium text-gray-500 shadow-sm cursor-pointer hover:bg-gray-50" %> +

To reconcile (<%= actionable_count %>)

- Checked boxes change facilitator affiliations + These buttons change facilitator affiliations
- <%# turbo: false so the POST renders the confirmation page (Turbo ignores a 200 HTML render on POST). %> - <%= form_with url: reconcile_affiliations_event_path(@event), method: :post, data: { turbo: false } do %> -
- <% @person_groups.each do |group| %> - <% checked_class = { - create: "has-[:checked]:bg-blue-50 has-[:checked]:text-blue-700 has-[:checked]:border-blue-300", - reactivate: "has-[:checked]:bg-green-50 has-[:checked]:text-green-700 has-[:checked]:border-green-300", - deactivate: "has-[:checked]:bg-red-50 has-[:checked]:text-red-700 has-[:checked]:border-red-300", - delete: "has-[:checked]:bg-red-50 has-[:checked]:text-red-700 has-[:checked]:border-red-300" - } %> - <% action_notes = { - create: [ "Uncheck to skip creating this facilitator affiliation." ], - reactivate: [ "Uncheck to leave this facilitator affiliation inactive." ], - deactivate: [ "To keep Affiliation active:", "Mark as Attended or uncheck both boxes" ], - delete: [ "Uncheck to keep this facilitator affiliation." ] - } %> - <% button_base = "group relative inline-flex items-center gap-2 rounded-md border border-gray-300 bg-white px-3 py-1.5 text-xs font-medium text-gray-500 shadow-sm cursor-pointer hover:bg-gray-50" %> -
-
-
- <%= link_to group[:person].name, edit_event_registration_path(group[:registration]), target: "_blank", rel: "noopener", class: "font-semibold text-gray-900 hover:underline hover:text-blue-700 shrink-0" %> - <% others = group[:other_facilitators] %> - <% if others.any? %> - <% note = "Also a facilitator at #{others.map { |a| a.organization.name }.to_sentence}." %> - <% anchor = others.one? ? dom_id(others.first) : "affiliations" %> - <%= link_to note, edit_person_path(group[:person], anchor: anchor), target: "_blank", rel: "noopener", - title: note, class: "text-xs text-gray-500 truncate hover:underline hover:text-blue-700 min-w-0" %> - <% end %> -
- <%= render "event_registrations/attendance_status_badge", registration: group[:registration], return_to: "reconcile_affiliations" %> + + <%# Standalone form (turbo:false so the POST renders the confirm page). The cards + live OUTSIDE it — the attendance chip renders its own form and nesting forms is + invalid — so the radios and submit join this form via the HTML form= attribute. %> + <%= form_with url: reconcile_affiliations_event_path(@event), method: :post, id: "reconcile_form", data: { turbo: false } do %><% end %> + +
+ <% @person_groups.each do |group| %> +
+
+
+ <%= link_to group[:person].name, edit_event_registration_path(group[:registration]), target: "_blank", rel: "noopener", class: "font-semibold text-gray-900 hover:underline hover:text-blue-700 shrink-0" %> + <% others = group[:other_facilitators] %> + <% if others.any? %> + <% note = "Also a facilitator at #{others.map { |a| a.organization.name }.to_sentence}." %> + <% anchor = others.one? ? dom_id(others.first) : "affiliations" %> + <%= link_to note, edit_person_path(group[:person], anchor: anchor), target: "_blank", rel: "noopener", + title: note, class: "text-xs text-gray-500 truncate hover:underline hover:text-blue-700 min-w-0" %> + <% end %>
+ <%= render "event_registrations/attendance_status_badge", registration: group[:registration], return_to: "reconcile_affiliations" %> +
-
- <% group[:rows].each do |row| %> - <% heading, _badge_class = badges[row.action] %> - <% included_checked = @pre_included.nil? || @pre_included.include?(row.key) %> - <% delete_checked = @pre_delete.include?(row.key) %> -
- <% if row.affiliation %> - <%= link_to edit_person_path(row.person, anchor: dom_id(row.affiliation)), target: "_blank", rel: "noopener", title: "Edit affiliation", class: "flex-1 min-w-0 text-sm hover:underline" do %> - <%= row.organization.name %> - · <%= row.affiliation.decorate.date_range %> - <% end %> - <% else %> - <%= row.organization.name %> +
+ <% group[:rows].each do |row| %> + <% chosen = (@pre_outcome && @pre_outcome[row.key]) || row.action.to_s %> +
+ <% if row.affiliation %> + <%= link_to edit_person_path(row.person, anchor: dom_id(row.affiliation)), target: "_blank", rel: "noopener", title: "Edit affiliation", class: "flex-1 min-w-0 text-sm hover:underline" do %> + <%= row.organization.name %> + · <%= row.affiliation.decorate.date_range %> + <% end %> + <% else %> + <%= row.organization.name %> + <% end %> + <%# One radio per outcome (native mutual exclusion — no JS). has-[:checked] colors the chosen button. %> +
+ <% outcome_options[row.action].each do |value, label, color| %> + <% end %> - <%# Checkbox lives inside the button; has-[:checked] colors the whole button when selected. %> -
-
data-controller="exclusive-checkboxes"<% end %>> - <% if row.action == :deactivate %> - - <% end %> - -
-
- <% action_notes[row.action].each do |line| %> -

<%= line %>

- <% end %> -
-
- <% end %> -
-
- <% end %> -
+
+ <% end %> +
+
+ <% end %> +
-
- <%= link_to "Cancel", registrants_event_path(@event), class: "text-sm text-gray-500 hover:text-gray-700" %> - <%= submit_tag "Preview changes", class: "btn btn-primary" %> -
- <% end %> +
+ <%= link_to "Cancel", registrants_event_path(@event), class: "text-sm text-gray-500 hover:text-gray-700" %> + <%= submit_tag "Preview changes", form: "reconcile_form", class: "btn btn-primary" %> +
<% end %> <% if @skipped_sections.any? %> diff --git a/spec/requests/events/reconcile_affiliations_spec.rb b/spec/requests/events/reconcile_affiliations_spec.rb index c8c94613a3..0a0b2aa3c4 100644 --- a/spec/requests/events/reconcile_affiliations_spec.rb +++ b/spec/requests/events/reconcile_affiliations_spec.rb @@ -93,7 +93,7 @@ def registrant_with_affiliation(status:) patch event_registration_path(registration, return_to: "reconcile_affiliations"), params: { event_registration: { status: "attended" } } - expect(response).to redirect_to(reconcile_affiliations_event_path(event)) + expect(response).to redirect_to(reconcile_affiliations_event_path(event, anchor: "attendance_status_event_registration_#{registration.id}")) expect(flash[:notice]).to be_present end end @@ -102,7 +102,7 @@ def registrant_with_affiliation(status:) it "shows the selected change without writing" do _person, affiliation = registrant_with_affiliation(status: "no_show") - post reconcile_affiliations_event_path(event), params: { included: [ "aff:#{affiliation.id}" ] } + post reconcile_affiliations_event_path(event), params: { outcome: { "aff:#{affiliation.id}" => "deactivate" } } expect(response).to have_http_status(:ok) expect(response.body).to include("Confirm affiliation changes") @@ -111,60 +111,59 @@ def registrant_with_affiliation(status:) end it "redirects back when nothing is selected" do - registrant_with_affiliation(status: "no_show") + _person, affiliation = registrant_with_affiliation(status: "no_show") - post reconcile_affiliations_event_path(event), params: { included: [] } + post reconcile_affiliations_event_path(event), params: { outcome: { "aff:#{affiliation.id}" => "keep" } } expect(response).to redirect_to(reconcile_affiliations_event_path(event)) end end describe "POST perform" do - it "deactivates the included non-completer and stamps the event" do + it "deactivates the chosen non-completer and stamps the event" do _person, affiliation = registrant_with_affiliation(status: "no_show") - post perform_reconcile_affiliations_event_path(event), params: { included: [ "aff:#{affiliation.id}" ] } + post perform_reconcile_affiliations_event_path(event), params: { outcome: { "aff:#{affiliation.id}" => "deactivate" } } expect(response).to redirect_to(registrants_event_path(event)) expect(affiliation.reload).not_to be_active expect(event.reload.affiliations_reconciled_at).to be_present end - it "spares an opted-out row" do + it "spares a row set to keep" do _person, affiliation = registrant_with_affiliation(status: "no_show") - post perform_reconcile_affiliations_event_path(event), params: { included: [] } + post perform_reconcile_affiliations_event_path(event), params: { outcome: { "aff:#{affiliation.id}" => "keep" } } expect(affiliation.reload).to be_active end - it "creates a missing affiliation before the event when included" do + it "creates a missing affiliation before the event when chosen" do upcoming = create(:event, facilitator_training: true, start_date: 3.days.from_now, end_date: 5.days.from_now) person = create(:person) reg = create(:event_registration, event: upcoming, registrant: person, status: "registered") create(:event_registration_organization, event_registration: reg, organization: organization) expect { - post perform_reconcile_affiliations_event_path(upcoming), params: { included: [ "create:#{person.id}:#{organization.id}" ] } + post perform_reconcile_affiliations_event_path(upcoming), params: { outcome: { "create:#{person.id}:#{organization.id}" => "create" } } }.to change { person.affiliations.facilitators.where(organization: organization).count }.by(1) end - it "deletes instead of same-daying when the delete option is checked" do + it "deletes when the delete outcome is chosen" do _person, affiliation = registrant_with_affiliation(status: "no_show") - key = "aff:#{affiliation.id}" - post perform_reconcile_affiliations_event_path(event), params: { included: [ key ], delete: [ key ] } + post perform_reconcile_affiliations_event_path(event), params: { outcome: { "aff:#{affiliation.id}" => "delete" } } expect(Affiliation.exists?(affiliation.id)).to be(false) end - it "deactivates a hand-entered facilitator affiliation when included" do + it "deactivates a hand-entered facilitator affiliation when chosen" do person = create(:person) reg = create(:event_registration, event: event, registrant: person, status: "no_show") create(:event_registration_organization, event_registration: reg, organization: organization) hand_entered = create(:affiliation, person: person, organization: organization, title: "Facilitator", start_date: 1.year.ago.to_date) - post perform_reconcile_affiliations_event_path(event), params: { included: [ "aff:#{hand_entered.id}" ] } + post perform_reconcile_affiliations_event_path(event), params: { outcome: { "aff:#{hand_entered.id}" => "deactivate" } } expect(hand_entered.reload).not_to be_active end @@ -179,7 +178,7 @@ def registrant_with_affiliation(status:) job = create(:affiliation, person: person, organization: organization, title: "Counselor", event_registration: reg) - post perform_reconcile_affiliations_event_path(non_training), params: { included: [ "aff:#{facilitator.id}" ] } + post perform_reconcile_affiliations_event_path(non_training), params: { outcome: { "aff:#{facilitator.id}" => "delete" } } expect(Affiliation.exists?(facilitator.id)).to be(false) expect(Affiliation.exists?(job.id)).to be(true) From 4d60e11dfd76728d28e91dd19a24f7a2cfa77462 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Fri, 14 Aug 2026 10:09:20 -0400 Subject: [PATCH 20/50] Fix Brakeman: read outcome params as a plain hash instead of permit! Co-Authored-By: Claude Opus 4.8 (1M context) --- .../events/reconcile_affiliations_controller.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/controllers/events/reconcile_affiliations_controller.rb b/app/controllers/events/reconcile_affiliations_controller.rb index d434e1188a..997ff3c817 100644 --- a/app/controllers/events/reconcile_affiliations_controller.rb +++ b/app/controllers/events/reconcile_affiliations_controller.rb @@ -46,11 +46,13 @@ def set_event @event = Event.find(params[:id]) end - # `outcome` is a { row.key => choice } map with dynamic keys; the service only - # acts on known choices, so the actual values are validated downstream. + # `outcome` is a { row.key => choice } map with dynamic keys, read as a plain + # string hash (never mass-assigned); the service only acts on known choices. def outcome_params - outcome = params[:outcome] - outcome.respond_to?(:permit!) ? outcome.permit!.to_h : {} + raw = params[:outcome] + return {} unless raw.respond_to?(:each_pair) + + raw.each_pair.map { |key, value| [ key.to_s, value.to_s ] }.to_h end def reconcile_notice(changed) From 7307b80aefc0ad3e44323b64070f117faa47afd1 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Fri, 14 Aug 2026 13:36:54 -0400 Subject: [PATCH 21/50] Reorder deactivate outcomes: Keep active, Delete, Deactivate; clearer labels Co-Authored-By: Claude Opus 4.8 (1M context) --- app/views/events/reconcile_affiliations/index.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/events/reconcile_affiliations/index.html.erb b/app/views/events/reconcile_affiliations/index.html.erb index 490ddda643..34af58341b 100644 --- a/app/views/events/reconcile_affiliations/index.html.erb +++ b/app/views/events/reconcile_affiliations/index.html.erb @@ -48,7 +48,7 @@ <% outcome_options = { create: [ [ "create", "Will be created", :blue ], [ "keep", "Skip", :gray ] ], reactivate: [ [ "reactivate", "Will be reactivated", :green ], [ "keep", "Leave inactive", :gray ] ], - deactivate: [ [ "deactivate", "Will be deactivated", :red ], [ "delete", "Delete instead", :red ], [ "keep", "Keep active", :green ] ], + deactivate: [ [ "keep", "Keep active", :green ], [ "delete", "Delete affiliation", :red ], [ "deactivate", "Deactivate affiliation", :red ] ], delete: [ [ "delete", "Will be deleted", :red ], [ "keep", "Keep", :gray ] ] } %> <% button_base = "group relative inline-flex items-center gap-2 rounded-md border border-gray-300 bg-white px-3 py-1.5 text-xs font-medium text-gray-500 shadow-sm cursor-pointer hover:bg-gray-50" %> From 2b6d04ff5d76d351c4e7ca7e0c5b600f8e92472d Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Fri, 14 Aug 2026 19:04:06 -0400 Subject: [PATCH 22/50] Update reconcile specs for renamed 'Deactivate affiliation' label Co-Authored-By: Claude Opus 4.8 (1M context) --- spec/requests/events/reconcile_affiliations_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/requests/events/reconcile_affiliations_spec.rb b/spec/requests/events/reconcile_affiliations_spec.rb index 0a0b2aa3c4..2b4d532869 100644 --- a/spec/requests/events/reconcile_affiliations_spec.rb +++ b/spec/requests/events/reconcile_affiliations_spec.rb @@ -27,7 +27,7 @@ def registrant_with_affiliation(status:) expect(response).to have_http_status(:ok) expect(response.body).to include(person.name) - expect(response.body).to include("Will be deactivated") + expect(response.body).to include("Deactivate affiliation") end it "previews a missing affiliation as a creation before the event" do @@ -73,7 +73,7 @@ def registrant_with_affiliation(status:) get reconcile_affiliations_event_path(event) - expect(response.body).to include("Will be deactivated") + expect(response.body).to include("Deactivate affiliation") end it "denies a non-admin" do From 0d4dcc800f6f626e27cb313865839fb2eff68b7c Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Tue, 18 Aug 2026 08:09:39 -0400 Subject: [PATCH 23/50] Let an explicit inactive flag override the date-derived one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- app/models/affiliation.rb | 4 ++++ app/views/organizations/show.html.erb | 2 +- spec/models/affiliation_spec.rb | 7 +++++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/app/models/affiliation.rb b/app/models/affiliation.rb index adba108fcb..f8e2cff48e 100644 --- a/app/models/affiliation.rb +++ b/app/models/affiliation.rb @@ -172,7 +172,11 @@ def sole_address_id_for_new_organization addresses.first.id if addresses&.one? end + # Derives `inactive` from the dates, unless this save sets it explicitly — an + # admin's tick, or a same-day deactivation whose end_date (today, or the future + # start of an upcoming affiliation) the date rule alone would still call active. def set_inactive_from_dates + return if inactive_changed? return unless end_date_changed? || start_date_changed? self.inactive = end_date.present? && end_date < Date.current diff --git a/app/views/organizations/show.html.erb b/app/views/organizations/show.html.erb index daf7a5f1ca..075456cb26 100644 --- a/app/views/organizations/show.html.erb +++ b/app/views/organizations/show.html.erb @@ -125,7 +125,7 @@ Affiliations <% active_affiliations = @organization.affiliations - .select { |a| !a.inactive? && (a.end_date.nil? || a.end_date >= Date.current) && a.person.present? } + .select { |a| a.active? && a.person.present? } .sort_by { |a| [a.person.first_name.to_s.downcase, a.person.last_name.to_s.downcase] } %> <% grouped = active_affiliations.group_by(&:person) %> <% if grouped.any? %> diff --git a/spec/models/affiliation_spec.rb b/spec/models/affiliation_spec.rb index ed1e882f80..8836daed66 100644 --- a/spec/models/affiliation_spec.rb +++ b/spec/models/affiliation_spec.rb @@ -313,6 +313,13 @@ op.update!(title: "New Title") expect(op.reload.inactive).to be true end + + it 'keeps an explicitly set flag the dates would not derive' do + op.update!(start_date: Date.current, end_date: Date.current, inactive: true) + + expect(op.reload.inactive).to be true + expect(op).not_to be_active + end end describe "reassigning the organization" do From e865e0f77c3d0a5b97792a06a9879175d0a3b35f Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Tue, 18 Aug 2026 08:09:45 -0400 Subject: [PATCH 24/50] Move the reconcile rules into a per-person classifier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- AGENTS.md | 4 +- .../affiliation_services/reconcile_event.rb | 121 ++++---------- .../reconcile_facilitator_affiliation.rb | 97 ----------- .../affiliation_services/reconcile_person.rb | 158 ++++++++++++++++++ .../reconcile_affiliations/_tooltip.html.erb | 2 +- .../reconcile_affiliations/confirm.html.erb | 2 +- ...ation_spec.rb => reconcile_person_spec.rb} | 104 ++++++++++-- 7 files changed, 279 insertions(+), 209 deletions(-) delete mode 100644 app/services/affiliation_services/reconcile_facilitator_affiliation.rb create mode 100644 app/services/affiliation_services/reconcile_person.rb rename spec/services/affiliation_services/{reconcile_facilitator_affiliation_spec.rb => reconcile_person_spec.rb} (53%) diff --git a/AGENTS.md b/AGENTS.md index c2edcf6f22..e418459249 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -264,8 +264,8 @@ action, or `authorize! :workshop, to: :summary?`). - `AffiliationServices::ApplyScenarioEndDating` — What an agreement scenario means for the person's *existing* affiliations, run by the linking core before creation (see ADR-0002): a **new_job** ends their active affiliations at other orgs (job + facilitator alike, sparing the linked org's rows); every other scenario ends nothing — reinstatement reconciles registration-style, creating affiliations only where no active one exists. Ends are dated the day before the agreement takes effect so the fresh affiliations (starting on the submission date) don't overlap a row still counting as active until end of day - `AffiliationServices::CreateFromRegistration` — On registration / org linking, creates a "job affiliation" with the typed title (when present) plus a standing "Facilitator" affiliation, in one transaction. Skips the facilitator one only when the person already has an active-or-pending affiliation titled exactly "Facilitator" with that org (a current one or one dated to a future training); an ended facilitator affiliation gets a fresh second one. Dedupe is by title + org + dates, so a job title like "Lead Facilitator" still gets its own Facilitator affiliation. Accepts an optional `organization_address:` and sets it on every affiliation it creates (the registrant's typed agency address, upserted onto the org); when an affiliation already exists and is skipped, it backfills that address onto the existing one only if it has none (an admin-set address is never overwritten) -- `AffiliationServices::ReconcileFacilitatorAffiliation` — Per `(person, organization)`, brings the person's **owned** facilitator affiliation (created by the registration flow, i.e. `event_registration_id` present) in line with attendance: keeps it active when they have any `attended` facilitator-training registration to that org; otherwise **same-days** it (`end_date := start_date`, which the model turns into `inactive: true`) once its source training has ended. Reactivates a previously same-dayed row when the person is later marked attended. Hand-created (unowned) rows are never touched. `#plan` returns the action (`:deactivate` / `:reactivate` / `:noop`) without writing. -- `AffiliationServices::ReconcileEvent` — Event-level orchestration for the "Reconcile affiliations" bulk action. Builds one row **per facilitator affiliation** for an org a registrant linked — **hand-entered rows included, not just app-created ones** — classifying each `:create` / `:deactivate` / `:reactivate` (facilitator trainings) or `:delete` (facilitator affiliation auto-created off a non-training event), else `:noop` with a reason. Deactivation is gated to post-event (owned rows on their source training's `ended?`, hand-entered on this event's `ended?`), so a pre-event run never deactivates. `#actionable_person_groups` groups actionable rows by person (with attendance registration and other-org facilitator affiliations for context); `#skipped_reason_sections` groups no-action rows by reason. `#planned_changes(outcome:)` and `#apply(outcome:)` take an `outcome` map `{ row.key => choice }` (choice is the action or "keep") — the confirm screen previews planned `Change`s, apply performs them and stamps `affiliations_reconciled_at`. Job affiliations are never touched. The controller is a two-step flow: `index` (edit) → `confirm` (preview, no writes) → `create` (perform). +- `AffiliationServices::ReconcilePerson` — **The single classifier** for facilitator affiliations, per `(person, organization)` in the context of one `event:`. `#plan` returns a `Decision` (`affiliation`, `action`, `reason`) per affiliation in scope, plus a create/no-create decision when the person has none — no writes. Actions: `:create` (pre-event for anyone, post-event only for attendees), `:deactivate` (**same-days** it — `end_date := start_date` plus an explicit `inactive: true`, since the model's date rule alone still reads a row ending today or later as active), `:reactivate`, `:delete` (non-training event: a row auto-created off it), or `:noop` with a reason. Completion is "any `attended` facilitator-training registration to that org", so no-showing one training but attending another keeps them active; deactivation waits for the governing training to end, so a pre-event run never deactivates. `#perform(action, affiliation:)` applies one decision, `#call` applies them all. `include_unowned:` is the auto-vs-manual gate — false (default) touches only rows the registration flow minted, true reconciles hand-entered rows too. +- `AffiliationServices::ReconcileEvent` — Event-level orchestration for the "Reconcile affiliations" bulk action. Walks the event's registrants and their linked orgs, iterating `ReconcilePerson` (with `include_unowned: true`, one memoized instance per person+org) and turning its decisions into individually-selectable rows — every rule lives in `ReconcilePerson`, every key/grouping/timestamp concern here. `#actionable_person_groups` groups actionable rows by person (with attendance registration and other-org facilitator affiliations for context); `#skipped_reason_sections` groups no-action rows by reason. `#planned_changes(outcome:)` and `#apply(outcome:)` take an `outcome` map `{ row.key => choice }` (choice is the action or "keep") — the confirm screen previews planned `Change`s, apply performs them and stamps `affiliations_reconciled_at`. Job affiliations are never touched. The controller is a two-step flow: `index` (edit) → `confirm` (preview, no writes) → `create` (perform). ### Sectors diff --git a/app/services/affiliation_services/reconcile_event.rb b/app/services/affiliation_services/reconcile_event.rb index dffa159305..b951874085 100644 --- a/app/services/affiliation_services/reconcile_event.rb +++ b/app/services/affiliation_services/reconcile_event.rb @@ -1,18 +1,9 @@ module AffiliationServices # Event-level orchestration for the "Reconcile affiliations" bulk action. Walks - # the event's registrants and, for each facilitator affiliation tied to an org - # they linked, works out what should happen to it (job affiliations are never - # touched). Produces one row per affiliation so each is individually actionable. - # - # Actions: - # :create — facilitator training, none exists yet but one should (pre-event - # for anyone, post-event only for attendees). - # :deactivate — facilitator training, owned, its (ended) training wasn't - # completed. The admin may delete it instead of same-daying it. - # :reactivate — facilitator training, owned, same-dayed earlier, now attended. - # :delete — NOT a facilitator training: an owned affiliation auto-created - # off this event that shouldn't exist. - # :noop — nothing to do; the row carries a `reason`. + # the event's registrants and, for each organization they linked, asks + # `ReconcilePerson` what should happen to their facilitator affiliations there — + # that class holds every rule; this one turns its decisions into reviewable, + # individually-selectable rows. Job affiliations are never touched. # # `actionable_person_groups` groups the actionable rows by person (with their # attendance registration and other-org facilitator affiliations for context); @@ -48,8 +39,8 @@ def skipped_reason_sections def reason_rank(reason) case reason - when "Active — attended" then 8 - when "Didn't attend — no affiliation created" then 9 + when ReconcilePerson::ACTIVE_ATTENDED then 8 + when ReconcilePerson::NOT_ATTENDED then 9 else 0 end end @@ -95,94 +86,42 @@ def apply(outcome:) def all_rows @all_rows ||= registrations_by_person.flat_map do |person, registrations| registration = registrations.first - linked_organizations(registrations).flat_map { |organization| rows_for(person, registration, organization) } - end - end - - def rows_for(person, registration, organization) - facilitators = person.affiliations.facilitators - .where(organization:) - .includes(event_registration: :event) - .to_a - - unless @event.facilitator_training? - # A non-training event confers no facilitation, so it only removes - # facilitator affiliations that were auto-created off it. - return facilitators.filter_map do |affiliation| - next unless affiliation.event_registration&.event_id == @event.id - - Row.new(person:, registration:, organization:, affiliation:, action: :delete, reason: nil, key: "aff:#{affiliation.id}") + linked_organizations(registrations).flat_map do |organization| + reconciler(person, registration, organization).plan.map do |decision| + row_for(person, registration, organization, decision) + end end end - - attended = completed_training?(person, organization) - rows = facilitators.map { |affiliation| affiliation_row(person, registration, organization, affiliation, attended) } - rows << create_row(person, registration, organization, attended) if facilitators.empty? - rows.compact - end - - def affiliation_row(person, registration, organization, affiliation, attended) - action, reason = classify_affiliation(affiliation, attended) - Row.new(person:, registration:, organization:, affiliation:, action:, reason:, key: "aff:#{affiliation.id}") - end - - # Reconciles EVERY facilitator affiliation for the org — hand-entered ones - # included, not just app-created rows. Deactivation only applies once the - # governing training has ended (a hand-entered row has no source training, so - # it's gated on this event ending) — so a pre-event run never deactivates. - def classify_affiliation(affiliation, attended) - if attended - affiliation.active? ? [ :noop, "Active — attended" ] : [ :reactivate, nil ] - elsif affiliation.active? && deactivation_ready?(affiliation) - [ :deactivate, nil ] - elsif affiliation.active? - [ :noop, "Training hasn't ended yet" ] - else - [ :noop, "Already deactivated — didn't attend" ] - end end - def deactivation_ready?(affiliation) - affiliation.event_registration_id ? source_ended?(affiliation) : @event.ended? + def row_for(person, registration, organization, decision) + Row.new(person:, registration:, organization:, affiliation: decision.affiliation, + action: decision.action, reason: decision.reason, + key: row_key(person, organization, decision)) end - def create_row(person, registration, organization, attended) - if !@event.ended? || attended - Row.new(person:, registration:, organization:, affiliation: nil, action: :create, reason: nil, - key: "create:#{person.id}:#{organization.id}") - else - Row.new(person:, registration:, organization:, affiliation: nil, action: :noop, - reason: "Didn't attend — no affiliation created", key: "none:#{person.id}:#{organization.id}") - end + # Stable per-row identity for the outcome map: the affiliation itself when there + # is one, else the (person, org) pair the row would create an affiliation for. + def row_key(person, organization, decision) + return "aff:#{decision.affiliation.id}" if decision.affiliation + + "#{decision.action == :create ? 'create' : 'none'}:#{person.id}:#{organization.id}" end def perform_outcome(row, choice) - case choice - when "create" - AffiliationServices::CreateFromRegistration.call( - person: row.person, organization: row.organization, facilitator_training: true, - training_date: @event.start_date, event_registration: row.registration - ) - when "delete" - row.affiliation.destroy! - when "deactivate" - # Same-day it: end_date = the affiliation's own start_date (start_date itself - # is never changed), which the model turns into inactive. - row.affiliation.update!(end_date: row.affiliation.start_date || Date.current) - when "reactivate" - row.affiliation.update!(end_date: nil) - else - return false # "keep" or unknown - end - true - end + action = ACTION_FOR_CHOICE[choice] + return false unless action - def completed_training?(person, organization) - ReconcileFacilitatorAffiliation.new(person:, organization:).completed_training? + reconciler(row.person, row.registration, row.organization).perform(action, affiliation: row.affiliation) end - def source_ended?(affiliation) - affiliation.event_registration&.event&.ended? + # One reconciler per (person, org) — reused for both planning and applying so the + # attendance lookup behind each decision runs once. + def reconciler(person, registration, organization) + @reconcilers ||= {} + @reconcilers[[ person.id, organization.id ]] ||= ReconcilePerson.new( + person:, organization:, event: @event, registration:, include_unowned: true + ) end def other_facilitators(person) diff --git a/app/services/affiliation_services/reconcile_facilitator_affiliation.rb b/app/services/affiliation_services/reconcile_facilitator_affiliation.rb deleted file mode 100644 index b4c837651a..0000000000 --- a/app/services/affiliation_services/reconcile_facilitator_affiliation.rb +++ /dev/null @@ -1,97 +0,0 @@ -module AffiliationServices - # Reconciles a person's **owned** facilitator affiliation for one organization - # against whether they actually completed a facilitator training there. - # - # "Owned" means auto-minted by the registration flow (`event_registration_id` - # present) — hand-created / historical rows have no link and are left alone. - # - # A person is an active facilitator of an org iff they have at least one - # `attended` registration to that org from a facilitator-training event. Anyone - # else (no_show, cancelled, incomplete_attendance, still-registered, …) is not, - # so we **same-day** their owned facilitator affiliation — set `end_date` to its - # `start_date`, which the model's `set_inactive_from_dates` turns into - # `inactive: true`. It preserves `start_date` and is reversible: if the person is - # later marked attended, a re-run clears `end_date` and reactivates the row. - # - # The decision is per (person, org) across ALL their training registrations, so - # no-showing one training but attending another for the same org keeps them - # active. - class ReconcileFacilitatorAffiliation - def self.call(person:, organization:) - new(person:, organization:).call - end - - def initialize(person:, organization:) - @person = person - @organization = organization - end - - # Apply the reconciliation. Returns the action taken (:deactivate, :reactivate, - # or :noop). - def call - rows = owned_facilitator_affiliations.to_a - return :noop if rows.empty? - - completed_training? ? reactivate(rows) : deactivate(rows) - end - - # What #call would do, without writing. Returns :deactivate, :reactivate, or :noop. - def plan - rows = owned_facilitator_affiliations.to_a - return :noop if rows.empty? - - if completed_training? - rows.any? { |affiliation| !affiliation.active? } ? :reactivate : :noop - elsif deactivatable_affiliations.any? - :deactivate - else - :noop - end - end - - # Whether the person has any `attended` registration to this org from a - # facilitator-training event — i.e. actually became a facilitator there. - def completed_training? - @person.event_registrations.attended - .joins(:event).where(events: { facilitator_training: true }) - .joins(:event_registration_organizations) - .where(event_registration_organizations: { organization_id: @organization.id }) - .exists? - end - - # The owned facilitator affiliations #call would same-day: active, and tied to a - # training that has already ended. Exposed so the bulk action can offer "delete - # instead of same-day" over the exact same set. - def deactivatable_affiliations - owned_facilitator_affiliations.select { |affiliation| affiliation.active? && source_training_ended?(affiliation) } - end - - private - - def deactivate(_rows) - targets = deactivatable_affiliations - return :noop if targets.empty? - - targets.each { |affiliation| affiliation.update!(end_date: affiliation.start_date || Date.current) } - :deactivate - end - - def source_training_ended?(affiliation) - affiliation.event_registration&.event&.ended? - end - - def reactivate(rows) - ended = rows.reject(&:active?) - return :noop if ended.empty? - - ended.each { |affiliation| affiliation.update!(end_date: nil) } - :reactivate - end - - def owned_facilitator_affiliations - @person.affiliations.facilitators - .where(organization: @organization) - .where.not(event_registration_id: nil) - end - end -end diff --git a/app/services/affiliation_services/reconcile_person.rb b/app/services/affiliation_services/reconcile_person.rb new file mode 100644 index 0000000000..2db0bacfed --- /dev/null +++ b/app/services/affiliation_services/reconcile_person.rb @@ -0,0 +1,158 @@ +module AffiliationServices + # Decides what should happen to one person's facilitator affiliations with one + # organization, in the context of one event. This is the single classifier — + # `ReconcileEvent` iterates it across an event's registrants, and it can be + # called on its own for a single person (e.g. after an attendance change). + # + # A person is a facilitator of an org iff they have at least one `attended` + # registration to that org from a facilitator-training event. The decision spans + # ALL their training registrations for the org, so no-showing one training but + # attending another keeps them active. + # + # Actions: + # :create — facilitator training, none exists yet but one should (pre-event + # for anyone, post-event only for attendees). + # :deactivate — facilitator training, its (ended) training wasn't completed. + # Same-days the row: `end_date := start_date` plus an explicit + # `inactive: true`, since the model's date rule alone still reads + # a row ending today or later as active. Preserves `start_date` + # and is reversible. + # :reactivate — facilitator training, same-dayed earlier, now attended. + # :delete — NOT a facilitator training: an affiliation auto-created off + # this event that shouldn't exist. + # :noop — nothing to do; the decision carries a `reason`. + # + # `include_unowned:` is the auto-vs-manual gate. False (the default) touches only + # rows the registration flow minted (`event_registration_id` present), leaving + # hand-created / historical rows alone. The bulk page passes true — an admin + # reviewing every row is expected to reconcile hand-entered ones too. + class ReconcilePerson + Decision = Struct.new(:affiliation, :action, :reason, keyword_init: true) do + def actionable? + action != :noop + end + end + + ACTIVE_ATTENDED = "Active — attended".freeze + TRAINING_PENDING = "Training hasn't ended yet".freeze + ALREADY_DEACTIVATED = "Already deactivated — didn't attend".freeze + NOT_ATTENDED = "Didn't attend — no affiliation created".freeze + + def self.call(person:, organization:, event:, registration: nil, include_unowned: false) + new(person:, organization:, event:, registration:, include_unowned:).call + end + + def initialize(person:, organization:, event:, registration: nil, include_unowned: false) + @person = person + @organization = organization + @event = event + @registration = registration + @include_unowned = include_unowned + end + + # One Decision per facilitator affiliation in scope, plus a create/no-create + # decision when the person has none. No writes. + def plan + @plan ||= @event.facilitator_training? ? training_plan : non_training_plan + end + + # Perform one planned action. Returns whether anything changed, so callers can + # count real changes rather than attempted ones. + def perform(action, affiliation: nil) + return false if affiliation.nil? && action != :create + + case action + when :create then create_affiliation + when :delete then affiliation.destroy! + when :deactivate then affiliation.update!(end_date: affiliation.start_date || Date.current, inactive: true) + when :reactivate then affiliation.update!(end_date: nil, inactive: false) + else return false + end + true + end + + # Apply every actionable decision. Returns the actions taken. + def call + plan.select(&:actionable?).filter_map do |decision| + decision.action if perform(decision.action, affiliation: decision.affiliation) + end + end + + private + + # Whether the person has any `attended` registration to this org from a + # facilitator-training event — i.e. actually became a facilitator there. + def completed_training? + return @completed_training if defined?(@completed_training) + + @completed_training = @person.event_registrations.attended + .joins(:event).where(events: { facilitator_training: true }) + .joins(:event_registration_organizations) + .where(event_registration_organizations: { organization_id: @organization.id }) + .exists? + end + + # A non-training event confers no facilitation, so it only removes facilitator + # affiliations that were auto-created off this event. Rows minted elsewhere — + # and hand-entered ones, which carry no link — are none of its business. + def non_training_plan + facilitator_affiliations.filter_map do |affiliation| + next unless affiliation.event_registration&.event_id == @event.id + + Decision.new(affiliation:, action: :delete) + end + end + + def training_plan + decisions = reconcilable_affiliations.map { |affiliation| classify(affiliation) } + decisions << creation_decision if facilitator_affiliations.empty? && @registration + decisions + end + + def classify(affiliation) + if completed_training? + return Decision.new(affiliation:, action: :noop, reason: ACTIVE_ATTENDED) if affiliation.active? + + Decision.new(affiliation:, action: :reactivate) + elsif !affiliation.active? + Decision.new(affiliation:, action: :noop, reason: ALREADY_DEACTIVATED) + elsif deactivation_ready?(affiliation) + Decision.new(affiliation:, action: :deactivate) + else + Decision.new(affiliation:, action: :noop, reason: TRAINING_PENDING) + end + end + + # Deactivation waits for the governing training to end, so a pre-event run never + # deactivates. A hand-entered row has no source training, so it waits on this event. + def deactivation_ready?(affiliation) + affiliation.event_registration_id ? affiliation.event_registration&.event&.ended? : @event.ended? + end + + def creation_decision + return Decision.new(action: :create) if !@event.ended? || completed_training? + + Decision.new(action: :noop, reason: NOT_ATTENDED) + end + + def create_affiliation + CreateFromRegistration.call( + person: @person, organization: @organization, facilitator_training: true, + training_date: @event.start_date, event_registration: @registration + ) + end + + def reconcilable_affiliations + return facilitator_affiliations if @include_unowned + + facilitator_affiliations.select(&:event_registration_id) + end + + def facilitator_affiliations + @facilitator_affiliations ||= @person.affiliations.facilitators + .where(organization: @organization) + .includes(event_registration: :event) + .to_a + end + end +end diff --git a/app/views/events/reconcile_affiliations/_tooltip.html.erb b/app/views/events/reconcile_affiliations/_tooltip.html.erb index a41b752833..478a8014c4 100644 --- a/app/views/events/reconcile_affiliations/_tooltip.html.erb +++ b/app/views/events/reconcile_affiliations/_tooltip.html.erb @@ -2,7 +2,7 @@
diff --git a/app/views/event_registrations/_attendance_status_badge.html.erb b/app/views/event_registrations/_attendance_status_badge.html.erb index fccb183ec6..516eac506a 100644 --- a/app/views/event_registrations/_attendance_status_badge.html.erb +++ b/app/views/event_registrations/_attendance_status_badge.html.erb @@ -1,9 +1,8 @@ <% deco = registration.decorate %> <% badge_return_to = local_assigns.fetch(:return_to, nil) %>
- <%# Without a `return_to` the update answers with a turbo_stream that swaps just this - badge. A page whose surrounding content depends on the status (the reconcile - actions) passes one and opts out of Turbo, so it gets the redirect and re-renders. %> + <%# No return_to: the update answers with a turbo_stream that swaps just this badge. + With one: opt out of Turbo so the redirect runs and the whole page re-renders. %> <%= form_with model: registration, url: event_registration_path(registration, return_to: badge_return_to), method: :patch, data: { turbo_frame: "_top", turbo: (false if badge_return_to) } do |f| %>
diff --git a/app/views/events/reconcile_affiliations/_tooltip.html.erb b/app/views/events/reconcile_affiliations/_tooltip.html.erb index 478a8014c4..662cc52475 100644 --- a/app/views/events/reconcile_affiliations/_tooltip.html.erb +++ b/app/views/events/reconcile_affiliations/_tooltip.html.erb @@ -1,4 +1,3 @@ -<%# Hover explanation for a reconcile action. `kind` is the action symbol. %>
- <%# Standalone form (turbo:false so the POST renders the confirm page). The cards - live OUTSIDE it — the attendance chip renders its own form and nesting forms is - invalid — so the radios and submit join this form via the HTML form= attribute. %> + <%# The cards sit outside this form — the attendance chip renders its own and forms + can't nest — so the radios join it via the HTML form= attribute. %> <%= form_with url: reconcile_affiliations_event_path(@event), method: :post, id: "reconcile_form", data: { turbo: false } do %><% end %>
@@ -94,7 +93,6 @@ <% else %> <%= row.organization.name %> <% end %> - <%# One radio per outcome (native mutual exclusion — no JS). has-[:checked] colors the chosen button. %>
<% outcome_options[row.action].each do |value, label, color| %>
diff --git a/spec/models/affiliation_spec.rb b/spec/models/affiliation_spec.rb index 8836daed66..72f0f9eee6 100644 --- a/spec/models/affiliation_spec.rb +++ b/spec/models/affiliation_spec.rb @@ -214,7 +214,7 @@ end end - describe '.active_on' do + describe '.active_by_date_on' do let(:date) { Date.new(2024, 6, 1) } let!(:spanning) { create(:affiliation, start_date: Date.new(2023, 1, 1), end_date: Date.new(2025, 1, 1)) } let!(:open_ended) { create(:affiliation, start_date: Date.new(2023, 1, 1), end_date: nil) } @@ -223,24 +223,24 @@ let!(:no_dates) { create(:affiliation, start_date: nil, end_date: nil) } it 'includes affiliations whose span covers the date' do - expect(described_class.active_on(date)).to include(spanning, open_ended) + expect(described_class.active_by_date_on(date)).to include(spanning, open_ended) end it 'excludes affiliations that ended before the date' do - expect(described_class.active_on(date)).not_to include(ended_before) + expect(described_class.active_by_date_on(date)).not_to include(ended_before) end it 'excludes affiliations that start after the date' do - expect(described_class.active_on(date)).not_to include(starts_after) + expect(described_class.active_by_date_on(date)).not_to include(starts_after) end it 'includes affiliations with no dates on record' do - expect(described_class.active_on(date)).to include(no_dates) + expect(described_class.active_by_date_on(date)).to include(no_dates) end it 'ignores the cached inactive flag, judging purely by dates' do flagged = create(:affiliation, start_date: Date.new(2023, 1, 1), end_date: nil, inactive: true) - expect(described_class.active_on(date)).to include(flagged) + expect(described_class.active_by_date_on(date)).to include(flagged) end end @@ -320,6 +320,29 @@ expect(op.reload.inactive).to be true expect(op).not_to be_active end + + it 'treats a form\'s "0" as not supplied, so the dates still derive' do + op.inactive_supplied = "0" + op.update!(end_date: 1.day.ago.to_date) + + expect(op.reload.inactive).to be true + end + + it 'honours an end date of today when the form supplies the flag' do + op.inactive_supplied = "1" + op.update!(end_date: Date.current, inactive: "1") + + expect(op.reload).not_to be_active + end + + it 'keeps a hand-set flag when a later edit resubmits it alongside a new date' do + op.update!(start_date: Date.current, inactive: true) + + op.inactive_supplied = true + op.update!(start_date: 1.month.ago.to_date, inactive: true) + + expect(op.reload.inactive).to be true + end end describe "reassigning the organization" do diff --git a/spec/requests/affiliations_spec.rb b/spec/requests/affiliations_spec.rb index 7888b86818..2ad0006c18 100644 --- a/spec/requests/affiliations_spec.rb +++ b/spec/requests/affiliations_spec.rb @@ -142,6 +142,24 @@ expect(affiliation.reload).not_to be_active end + + it "keeps the ended state when a later edit changes a date with the box still ticked" do + affiliation.update!(start_date: Date.current, end_date: nil, inactive: true) + + patch affiliation_path(affiliation, return_to: "organization", origin_id: organization.id), + params: { affiliation: { start_date: 1.month.ago.to_date.to_s, inactive: "1" } } + + expect(affiliation.reload).not_to be_active + end + + it "still derives the flag from the dates when the form omits it" do + affiliation.update!(start_date: 1.year.ago.to_date, end_date: nil, inactive: false) + + patch affiliation_path(affiliation, return_to: "organization", origin_id: organization.id), + params: { affiliation: { end_date: 1.day.ago.to_date.to_s } } + + expect(affiliation.reload).not_to be_active + end end context "as a non-admin" do From 395e323b3631e325a64e6895d73f5ce57929d01c Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Wed, 19 Aug 2026 22:09:10 -0400 Subject: [PATCH 29/50] Stop reconciliation rewriting history it did not create MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../affiliation_services/reconcile_event.rb | 2 +- .../affiliation_services/reconcile_person.rb | 70 +++++++++-- .../reconcile_affiliations/_tooltip.html.erb | 11 +- .../reconcile_affiliations/confirm.html.erb | 11 +- .../reconcile_affiliations/index.html.erb | 17 ++- .../reconcile_person_spec.rb | 111 ++++++++++++++++-- 6 files changed, 190 insertions(+), 32 deletions(-) diff --git a/app/services/affiliation_services/reconcile_event.rb b/app/services/affiliation_services/reconcile_event.rb index a2cf5ffa16..6d3b52418d 100644 --- a/app/services/affiliation_services/reconcile_event.rb +++ b/app/services/affiliation_services/reconcile_event.rb @@ -44,7 +44,7 @@ def any_rows? Change = Struct.new(:person, :organization, :affiliation, :action, keyword_init: true) # One radio choice per row: the action itself, or "keep". - ACTION_FOR_CHOICE = { "deactivate" => :deactivate, "delete" => :delete, "reactivate" => :reactivate, "create" => :create }.freeze + ACTION_FOR_CHOICE = { "deactivate" => :deactivate, "delete" => :delete, "create" => :create }.freeze # What the `{ row.key => choice }` map will change, for the confirmation screen. def planned_changes(outcome:) diff --git a/app/services/affiliation_services/reconcile_person.rb b/app/services/affiliation_services/reconcile_person.rb index be1bb02afe..1ca4ccb629 100644 --- a/app/services/affiliation_services/reconcile_person.rb +++ b/app/services/affiliation_services/reconcile_person.rb @@ -20,6 +20,11 @@ def actionable? ACTIVE_ATTENDED = "Active — attended".freeze TRAINING_PENDING = "Training hasn't ended yet".freeze ALREADY_DEACTIVATED = "Already deactivated — didn't attend".freeze + ALREADY_ENDED = "Already ended — not by reconciliation".freeze + LAPSED = "Ended — a return is recorded as a new affiliation".freeze + # Topic on the comment reconciliation leaves behind, so a row can say why it + # ended without a dedicated column (ADR-0002 D6b). + COMMENT_TOPIC = "Reconciliation".freeze NOT_ATTENDED = "Didn't attend — no affiliation created".freeze def self.call(person:, organization:, event:, registration: nil, include_unowned: false) @@ -45,10 +50,9 @@ def perform(action, affiliation: nil) return false if affiliation.nil? && action != :create case action - when :create then create_affiliation + when :create then create_and_note when :delete then affiliation.destroy! - when :deactivate then affiliation.update!(end_date: affiliation.start_date || Date.current, inactive: true) - when :reactivate then affiliation.update!(end_date: nil, inactive: false) + when :deactivate then deactivate(affiliation) else return false end true @@ -72,11 +76,50 @@ def completed_training? .exists? end + def deactivate(affiliation) + ends_on = deactivation_end_date(affiliation) + affiliation.update!(end_date: ends_on, inactive: true) + note(affiliation, "Ended #{ends_on.strftime('%b %-d, %Y')} and marked inactive by reconciliation " \ + "for #{@event.title} — no attended facilitator training for #{@organization.name} on record.") + end + + def create_and_note + created = @person.affiliations.facilitators.where(organization: @organization).pluck(:id) + create_affiliation + fresh = @person.affiliations.facilitators.where(organization: @organization).where.not(id: created) + fresh.each { |affiliation| note(affiliation, "Created by reconciliation for #{@event.title}.") } + end + + # Why a row changed, on the affiliation's own comments rather than a dedicated + # column — the edit page and its history already surface them (ADR-0002 D6b). + def note(affiliation, body) + affiliation.comments.create!(topic: COMMENT_TOPIC, body: body, + created_by: Current.user, updated_by: Current.user) + end + + def ended_by_reconciliation?(affiliation) + affiliation.comments.any? { |comment| comment.topic == COMMENT_TOPIC } + end + + def minted_here?(affiliation) + affiliation.event_registration&.event_id == @event.id + end + + # Where a deactivation ends the row (ADR-0002 D6). The row this training minted + # is an assumption that never came true, so it collapses to nothing. Any older + # row records facilitation that really happened: it ends at this training, so + # the years before it survive and anchored program status doesn't move. + def deactivation_end_date(affiliation) + return affiliation.start_date if minted_here?(affiliation) && affiliation.start_date + + [ @event.start_date&.to_date || Date.current, affiliation.start_date ].compact.max + end + # A non-training event confers no facilitation, so it only removes what was # auto-created off it. def non_training_plan facilitator_affiliations.filter_map do |affiliation| - next unless affiliation.event_registration&.event_id == @event.id + next unless minted_here?(affiliation) Decision.new(affiliation:, action: :delete) end @@ -84,17 +127,28 @@ def non_training_plan def training_plan decisions = reconcilable_affiliations.map { |affiliation| classify(affiliation) } - decisions << creation_decision if facilitator_affiliations.empty? && @registration + decisions << creation_decision if needs_affiliation? decisions end + # Someone with no active facilitator affiliation needs one when they have never + # had one, or when they completed a training here and are returning after a + # lapse — the return is a NEW row, never a resurrected one (ADR-0002 D6a). + def needs_affiliation? + return false unless @registration + return false if facilitator_affiliations.any?(&:active?) + + facilitator_affiliations.empty? || completed_training? + end + def classify(affiliation) if completed_training? return Decision.new(affiliation:, action: :noop, reason: ACTIVE_ATTENDED) if affiliation.active? - Decision.new(affiliation:, action: :reactivate) + Decision.new(affiliation:, action: :noop, reason: LAPSED) elsif !affiliation.active? - Decision.new(affiliation:, action: :noop, reason: ALREADY_DEACTIVATED) + reason = ended_by_reconciliation?(affiliation) ? ALREADY_DEACTIVATED : ALREADY_ENDED + Decision.new(affiliation:, action: :noop, reason:) elsif deactivation_ready?(affiliation) Decision.new(affiliation:, action: :deactivate) else @@ -130,7 +184,7 @@ def reconcilable_affiliations def facilitator_affiliations @facilitator_affiliations ||= @person.affiliations.facilitators .where(organization: @organization) - .includes(event_registration: :event) + .includes(:comments, event_registration: :event) .to_a end end diff --git a/app/views/events/reconcile_affiliations/_tooltip.html.erb b/app/views/events/reconcile_affiliations/_tooltip.html.erb index 662cc52475..08d09c2a84 100644 --- a/app/views/events/reconcile_affiliations/_tooltip.html.erb +++ b/app/views/events/reconcile_affiliations/_tooltip.html.erb @@ -1,7 +1,12 @@ -
+ <% if group[:registration]&.status == "incomplete_attendance" %> + <%= render "attendance_days", registration: group[:registration], event: @event %> + <% end %> +
<% group[:rows].each do |row| %> <% chosen = (@pre_outcome && @pre_outcome[row.key]) || row.action.to_s %> @@ -152,6 +157,9 @@ <% end %> <%= render "event_registrations/attendance_status_badge", registration: row.registration, return_to: "reconcile_affiliations" %> + <% if row.registration&.status == "incomplete_attendance" %> +
<%= render "attendance_days", registration: row.registration, event: @event %>
+ <% end %>
<% end %>
diff --git a/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md b/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md index bb7e777372..1c263f4f4a 100644 --- a/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md +++ b/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md @@ -221,6 +221,36 @@ This is the mirror of D6. D6 stops an ending from reaching too far back; D6a sto reactivation from reaching too far forward. Both exist because the historical readers (D3) trust the dates. +### D6b — The reason a row changed lives in its comments + +Reconciliation writes a comment on the affiliation it ends or creates, topic +`"Reconciliation"` (`ReconcilePerson::COMMENT_TOPIC`), naming the event and what it +did. No dedicated `inactive_reason` column: the affiliation editor and its history +already surface comments, and the topic is enough of a handle for the one place that +needs to branch on it — telling a row reconciliation ended from one an admin ended, +so the page stops labelling both "didn't attend". + +The limit is that a **deleted** row takes its comments with it (D6), so for those +the trail is the `destroy.affiliation` Ahoy event and its attribute snapshot. + +### D6c — `registered` is a gap in the record, not an outcome + +After the event, a registration still marked `registered` means nobody filled the +roster in. `EventRegistration#attendance_recorded?` deliberately excludes it — only +`attended`, `incomplete_attendance` and `no_show` count as outcomes. + +Reconciliation therefore **does nothing** to those rows and lists them under +"Attendance never recorded — set an outcome first". Deleting an affiliation because +the roster is blank would be acting on missing data, and the deletion is not +reversible. Cancelled and transferred-out are different: those are decisions +somebody made. + +For `incomplete_attendance` the page shows the sign-in sheet day by day +(`Event#event_dates` × `EventRegistration#attendance_entries_on`). "Incomplete" is a +judgement someone recorded, and the logged times are the evidence behind it — an +admin deciding whether to delete a facilitator affiliation should see which days +were missed without leaving the page. + ### D7 — What has to be tested The arithmetic is what the grant figures rest on, so it is covered directly rather @@ -240,7 +270,9 @@ than inferred from the single-affiliation cases 5. **The bucket agrees with the SQL scope** the index filter uses. 6. **Reconciliation doesn't move an anchored verdict** — D6, both branches: the minted row is deleted, the older row ended at the training date. -7. **A return after a lapse adds a row and leaves the lapse intact** — D6a, asserted +7. **`registered` after the event is reported, never acted on** — D6c, asserted on + both the plan's reason and that the row survives. +8. **A return after a lapse adds a row and leaves the lapse intact** — D6a, asserted on both the row count and the mid-gap verdict. Adding a rule here means adding a case there. diff --git a/spec/requests/events/reconcile_affiliations_spec.rb b/spec/requests/events/reconcile_affiliations_spec.rb index c0fa40b595..0ffa1b2cb5 100644 --- a/spec/requests/events/reconcile_affiliations_spec.rb +++ b/spec/requests/events/reconcile_affiliations_spec.rb @@ -88,6 +88,44 @@ def registrant_with_affiliation(status:) expect(response.body).to include("Deactivate affiliation") end + it "asks for an outcome instead of acting when attendance was never recorded" do + person, affiliation = registrant_with_affiliation(status: "registered") + + get reconcile_affiliations_event_path(event) + + expect(response.body).to include(person.name) + expect(response.body).to include("Attendance never recorded") + expect(response.body).not_to include("Will be deleted") + expect(Affiliation.exists?(affiliation.id)).to be(true) + end + + it "shows the day-by-day sign-in sheet for a partial attendance" do + person, _affiliation = registrant_with_affiliation(status: "incomplete_attendance") + registration = person.event_registrations.first + # Asserted by duration, not wall clock: the view renders in the viewer's zone + # and the spec process is in another, so a clock time would only match for + # part of each day. + signed_in = event.start_date.in_time_zone + registration.event_attendance_time_entries.create!( + signed_in_at: signed_in, signed_out_at: signed_in + 3.hours + ) + + get reconcile_affiliations_event_path(event) + + expect(response.body).to include("Partial attendance") + expect(response.body).to include("Day 1") + expect(response.body).to include("3 hours") + expect(response.body).to include("no sign-in recorded") + end + + it "leaves the sheet out for a plain no-show" do + registrant_with_affiliation(status: "no_show") + + get reconcile_affiliations_event_path(event) + + expect(response.body).not_to include("Partial attendance") + end + it "denies a non-admin" do sign_in create(:user) diff --git a/spec/services/affiliation_services/reconcile_person_spec.rb b/spec/services/affiliation_services/reconcile_person_spec.rb index 02ea71e944..40497995c2 100644 --- a/spec/services/affiliation_services/reconcile_person_spec.rb +++ b/spec/services/affiliation_services/reconcile_person_spec.rb @@ -38,7 +38,7 @@ def reconcile(registration, **options) expect(Affiliation.exists?(affiliation.id)).to be(false) end - %w[ incomplete_attendance registered cancelled transferred_out ].each do |status| + %w[ incomplete_attendance cancelled transferred_out ].each do |status| it "deletes the minted row when the only registration is #{status}" do reg = training_registration(status: status) affiliation = owned_facilitator(registration: reg) @@ -49,6 +49,32 @@ def reconcile(registration, **options) end end + # `registered` after the event means nobody filled the roster in. Acting on it + # would be acting on missing data. + it "leaves a still-registered row alone and asks for an outcome instead" do + reg = training_registration(status: "registered") + affiliation = owned_facilitator(registration: reg) + + plan = described_class.new(person: person, organization: organization, + event: reg.event, registration: reg).plan + + expect(plan.map(&:action)).to eq([ :noop ]) + expect(plan.first.reason).to eq(described_class::ATTENDANCE_NOT_RECORDED) + + reconcile(reg) + expect(Affiliation.exists?(affiliation.id)).to be(true) + end + + it "still says the training hasn't ended when it hasn't, rather than asking for an outcome" do + reg = training_registration(status: "registered", ended: false) + owned_facilitator(registration: reg, start_date: Date.current) + + plan = described_class.new(person: person, organization: organization, + event: reg.event, registration: reg).plan + + expect(plan.first.reason).to eq(described_class::TRAINING_PENDING) + end + it "deletes it on the day a one-day training ends, with no reliance on the inactive flag" do event = create(:event, facilitator_training: true, start_date: 3.hours.ago, end_date: 1.hour.ago, registration_close_date: 4.hours.ago) From 670113f34e1450bad3ecb5de25af0c21f89077bb Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 23 Aug 2026 15:25:06 -0400 Subject: [PATCH 42/50] Show the drift check's two statuses as colored columns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- app/services/data_health/check.rb | 7 ++++ .../legacy_organization_status_drift.rb | 6 +++ .../_organization_status_row.html.erb | 20 ++++++++++ app/views/admin/data_health/index.html.erb | 15 ++++++- spec/requests/admin/data_health_spec.rb | 39 +++++++++++++++++++ 5 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 app/views/admin/data_health/_organization_status_row.html.erb diff --git a/app/services/data_health/check.rb b/app/services/data_health/check.rb index c2d0542aa6..7b0164197a 100644 --- a/app/services/data_health/check.rb +++ b/app/services/data_health/check.rb @@ -57,5 +57,12 @@ def repaired_message(number) def describe(record) record.to_s end + + # A check whose rows are worth comparing side by side names its column + # headings here and a partial to render each row; the page falls back to the + # one-line #describe when both are nil. + def columns = nil + + def row_partial = nil end end diff --git a/app/services/data_health/legacy_organization_status_drift.rb b/app/services/data_health/legacy_organization_status_drift.rb index ca7f9f0f0f..0a6c41c0b7 100644 --- a/app/services/data_health/legacy_organization_status_drift.rb +++ b/app/services/data_health/legacy_organization_status_drift.rb @@ -21,6 +21,12 @@ def scope Organization.where(id: drifted_ids).includes(:organization_status) end + # Two columns so the disagreement reads at a glance: what the affiliations say + # (the answer the app uses) against what the column stores (the one it ignores). + def columns = [ "Organization", "Affiliations say", "Stored status" ] + + def row_partial = "admin/data_health/organization_status_row" + def describe(organization) deco = organization.decorate "#{organization.name} — stored #{organization.organization_status&.name.presence || 'none'}, " \ diff --git a/app/views/admin/data_health/_organization_status_row.html.erb b/app/views/admin/data_health/_organization_status_row.html.erb new file mode 100644 index 0000000000..2e26aa4faf --- /dev/null +++ b/app/views/admin/data_health/_organization_status_row.html.erb @@ -0,0 +1,20 @@ +<%# Locals: record (an Organization). One row of the legacy-status drift check: + the affiliation-derived bucket the app actually uses, against the stored + column it ignores (ADR-0001 D3a). Both chips take their colour from the same + DomainTheme lookup the org pages use, so the two answers are comparable. %> +<% deco = record.decorate %> +<% stored_name = record.organization_status&.name.presence %> +<% chip = "inline-flex items-center rounded-full border px-2.5 py-0.5 text-xs font-medium whitespace-nowrap" %> +<% cell = "border-t border-gray-100 py-2" %> + +<%= link_to record.name, edit_organization_path(record, anchor: "program-status"), + target: "_blank", rel: "noopener", + class: "min-w-0 truncate border-t border-gray-100 py-2 font-medium text-gray-800 hover:text-blue-700 hover:underline" %> + + + <%= deco.organization_status_label %> + + + + <%= stored_name || "None" %> + diff --git a/app/views/admin/data_health/index.html.erb b/app/views/admin/data_health/index.html.erb index c03bada408..b95ab5e690 100644 --- a/app/views/admin/data_health/index.html.erb +++ b/app/views/admin/data_health/index.html.erb @@ -45,7 +45,20 @@ <% end %>
- <% if check.any? %> + <% if check.any? && check.row_partial %> + <%# display:contents on each row so its cells land in this grid and the + columns line up across records. %> +
    +
  1. + <% check.columns.each do |heading| %><%= heading %><% end %> +
  2. + <% check.preview.each do |record| %> +
  3. + <%= render check.row_partial, record: record %> +
  4. + <% end %> +
+ <% elsif check.any? %>
    <% check.preview.each do |record| %>
  • <%= check.describe(record) %>
  • diff --git a/spec/requests/admin/data_health_spec.rb b/spec/requests/admin/data_health_spec.rb index eef3d615ff..45435aef13 100644 --- a/spec/requests/admin/data_health_spec.rb +++ b/spec/requests/admin/data_health_spec.rb @@ -34,6 +34,45 @@ def offending_affiliation expect(response.body).not_to include("Everything checks out") end + # The drift check is the one worth reading side by side: what the app uses + # against what the ignored column stores. + context "the legacy status drift check" do + let!(:active_status) { OrganizationStatus.find_or_create_by!(name: "Active") } + let!(:drifted) { create(:organization, name: "Drifted Org", organization_status: active_status) } + + it "renders both statuses as chips, in labelled columns" do + get admin_data_health_path + + doc = Nokogiri::HTML(response.body) + headings = doc.css("li.contents").first.css("span").map(&:text).map(&:strip) + expect(headings).to eq([ "Organization", "Affiliations say", "Stored status" ]) + + row = doc.css("li.contents").last + expect(row.text).to include("Drifted Org") + expect(row.text).to include("Never active") + expect(row.text).to include("Active") + end + + it "colours each chip from its own bucket, so a disagreement is visible" do + get admin_data_health_path + + row = Nokogiri::HTML(response.body).css("li.contents").last + chips = row.css("span[class*='rounded-full']").map { |c| c["class"] } + + expect(chips.size).to eq(2) + expect(chips.first).to eq(chips.first) # affiliation-derived + expect(chips.first).not_to eq(chips.last), "both chips got the same colour, so the drift is invisible" + end + + it "links each organization to its program-status section" do + get admin_data_health_path + + expect(response.body).to include( + CGI.escapeHTML(edit_organization_path(drifted, anchor: "program-status")) + ) + end + end + it "offers a repair only for checks that have one" do offending_affiliation From adc77cff51ecd93e85b26c8dbacc1d02a18b5901 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 23 Aug 2026 15:49:28 -0400 Subject: [PATCH 43/50] Move a transferred-out registrant's affiliation instead of ending it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../affiliation_services/reconcile_event.rb | 5 +- .../affiliation_services/reconcile_person.rb | 75 ++++++++++++++-- .../reconcile_affiliations/_tooltip.html.erb | 3 + .../reconcile_affiliations/confirm.html.erb | 1 + .../reconcile_affiliations/index.html.erb | 3 +- db/schema.rb | 2 + ...ions-as-the-record-of-two-relationships.md | 46 +++++++++- .../reconcile_person_spec.rb | 89 ++++++++++++++++++- .../facilitator_program_status_math_spec.rb | 8 +- 9 files changed, 216 insertions(+), 16 deletions(-) diff --git a/app/services/affiliation_services/reconcile_event.rb b/app/services/affiliation_services/reconcile_event.rb index 6d3b52418d..36f0a70c24 100644 --- a/app/services/affiliation_services/reconcile_event.rb +++ b/app/services/affiliation_services/reconcile_event.rb @@ -44,7 +44,8 @@ def any_rows? Change = Struct.new(:person, :organization, :affiliation, :action, keyword_init: true) # One radio choice per row: the action itself, or "keep". - ACTION_FOR_CHOICE = { "deactivate" => :deactivate, "delete" => :delete, "create" => :create }.freeze + ACTION_FOR_CHOICE = { "deactivate" => :deactivate, "delete" => :delete, "create" => :create, + "retarget" => :retarget }.freeze # What the `{ row.key => choice }` map will change, for the confirmation screen. def planned_changes(outcome:) @@ -95,7 +96,7 @@ def row_for(person, registration, organization, decision) def row_key(person, organization, decision) return "aff:#{decision.affiliation.id}" if decision.affiliation - "#{decision.action == :create ? 'create' : 'none'}:#{person.id}:#{organization.id}" + "#{decision.actionable? ? decision.action : 'none'}:#{person.id}:#{organization.id}" end def perform_outcome(row, choice) diff --git a/app/services/affiliation_services/reconcile_person.rb b/app/services/affiliation_services/reconcile_person.rb index 1cf2e1788e..dbdc7060a0 100644 --- a/app/services/affiliation_services/reconcile_person.rb +++ b/app/services/affiliation_services/reconcile_person.rb @@ -30,6 +30,8 @@ def actionable? # #attendance_recorded? deliberately excludes it. Acting on it would be acting # on missing data, so the row is surfaced for an admin to resolve instead. ATTENDANCE_NOT_RECORDED = "Attendance never recorded — set an outcome first".freeze + TRANSFER_PENDING = "Transferred out — record where they went first".freeze + TRANSFER_ELSEWHERE = "Transferred out to an event not linked to this organization".freeze def self.call(person:, organization:, event:, registration: nil, include_unowned: false) new(person:, organization:, event:, registration:, include_unowned:).call @@ -51,12 +53,13 @@ def plan # Returns whether anything changed, so callers can count real changes. def perform(action, affiliation: nil) - return false if affiliation.nil? && action != :create + return false if affiliation.nil? && !%i[ create retarget ].include?(action) case action when :create then create_and_note when :delete then affiliation.destroy! when :deactivate then deactivate(affiliation) + when :retarget then retarget(affiliation) else return false end true @@ -87,6 +90,26 @@ def deactivate(affiliation) "for #{@event.title} — no attended facilitator training for #{@organization.name} on record.") end + # Moves the open row to the destination training: its start date becomes that + # event's, and its provenance re-points at that registration so reconciling the + # destination recognises it as the row that training minted (ADR-0003 D2a). + # With no open row to move, the destination mints a fresh one. + def retarget(affiliation) + starts_on = transfer_destination.event&.start_date&.to_date || Date.current + return create_at_destination(starts_on) if affiliation.nil? + + affiliation.update!(start_date: starts_on, event_registration: transfer_destination) + note(affiliation, "Moved to #{transfer_destination.event&.title} (starting #{starts_on.strftime('%b %-d, %Y')}) " \ + "when #{@person.name} transferred out of #{@event.title}.") + end + + def create_at_destination(starts_on) + CreateFromRegistration.call( + person: @person, organization: @organization, facilitator_training: true, + training_date: starts_on, event_registration: transfer_destination + ) + end + def create_and_note created = @person.affiliations.facilitators.where(organization: @organization).pluck(:id) create_affiliation @@ -126,11 +149,15 @@ def minted_here?(affiliation) end # Only rows this training did NOT mint reach here (minted ones are deleted), and - # they record facilitation that really happened — so they end at this training - # and the years before it survive, leaving anchored program status where it was - # (ADR-0003 D6). Never before the row's own start date. + # they record facilitation that really happened — so the years before this + # training survive (ADR-0003 D6). + # + # The day BEFORE the training, matching ApplyScenarioEndDating: a row that ends + # on the same day another starts counts on both, which double-counts the person + # in any report that totals a date. Never before the row's own start date. def deactivation_end_date(affiliation) - [ @event.start_date&.to_date || Date.current, affiliation.start_date ].compact.max + ends_on = (@event.start_date&.to_date || Date.current) - 1.day + [ ends_on, affiliation.start_date ].compact.max end # A non-training event confers no facilitation, so it only removes what was @@ -144,11 +171,49 @@ def non_training_plan end def training_plan + return transfer_plan if transferred_out? + decisions = reconcilable_affiliations.map { |affiliation| classify(affiliation) } decisions << creation_decision if needs_affiliation? decisions end + def transferred_out? + registration_here&.transferred_out? + end + + # A transfer isn't a failure — they're training somewhere else. The affiliation + # follows them: it re-dates to the destination event and re-points at that + # registration, rather than being ended or deleted (ADR-0003 D6d). + # + # One decision per (person, organization), not one per row: only the open row + # moves, and an already-ended row is history that stays put. + def transfer_plan + return [ Decision.new(affiliation: open_facilitator_affiliation, action: :noop, reason: TRANSFER_PENDING) ] if transfer_destination.nil? + return [ Decision.new(affiliation: open_facilitator_affiliation, action: :noop, reason: TRANSFER_ELSEWHERE) ] unless destination_links_this_org? + + [ Decision.new(affiliation: open_facilitator_affiliation, action: :retarget) ] + end + + # A→B→C collapses when the second transfer is made, so this is already the + # final destination — no chain to walk (see EventRegistrationsController#transfer). + def transfer_destination + return @transfer_destination if defined?(@transfer_destination) + + @transfer_destination = registration_here&.transferred_to_registration + end + + def destination_links_this_org? + transfer_destination.organizations.any? { |organization| organization.id == @organization.id } + end + + # The row a transfer moves: this person's facilitator affiliation with this org + # that hasn't ended. An already-ended row records a finished stretch, so it is + # left alone and a new affiliation is created at the destination instead. + def open_facilitator_affiliation + facilitator_affiliations.find { |affiliation| affiliation.end_date.nil? } + end + # Someone with no active facilitator affiliation needs one when they have never # had one, or when they completed a training here and are returning after a # lapse — the return is a NEW row, never a resurrected one (ADR-0003 D6a). diff --git a/app/views/events/reconcile_affiliations/_tooltip.html.erb b/app/views/events/reconcile_affiliations/_tooltip.html.erb index afd6b64492..aaa03068d7 100644 --- a/app/views/events/reconcile_affiliations/_tooltip.html.erb +++ b/app/views/events/reconcile_affiliations/_tooltip.html.erb @@ -10,6 +10,9 @@
  • Job affiliations for this org
  • Facilitator affiliations for other orgs
+ <% when :retarget %> +

They transferred to another event, so the affiliation follows them: its start date becomes the new event's, and it re-points at that registration.

+

Nothing is ended or deleted. If their only affiliation here has already ended, a new one is created at the new event instead.

<% when :create %> Creates the facilitator affiliation for this organization. <% when :keep %> diff --git a/app/views/events/reconcile_affiliations/confirm.html.erb b/app/views/events/reconcile_affiliations/confirm.html.erb index 1dea4a4572..ff2271c48e 100644 --- a/app/views/events/reconcile_affiliations/confirm.html.erb +++ b/app/views/events/reconcile_affiliations/confirm.html.erb @@ -13,6 +13,7 @@ <% sections = { create: [ "Create", "bg-blue-50 text-blue-800 border-blue-200", "A new facilitator affiliation is created for this organization." ], deactivate: [ "Deactivate", "bg-red-50 text-red-800 border-red-200", "The facilitator affiliation is given an end date of this training's date and marked inactive, so the years before it stay on the record. Reversible." ], + retarget: [ "Move", "bg-blue-50 text-blue-800 border-blue-200", "The facilitator affiliation moves to the event they transferred to — re-dated to it and re-pointed at that registration. Nothing is ended." ], delete: [ "Delete", "bg-red-100 text-red-900 border-red-300", "The facilitator affiliation is permanently deleted — this is what happens to a row this training created for someone who didn't attend. Job and other-org affiliations are untouched." ] } %> diff --git a/app/views/events/reconcile_affiliations/index.html.erb b/app/views/events/reconcile_affiliations/index.html.erb index 54e40b21b4..aa7c4badbe 100644 --- a/app/views/events/reconcile_affiliations/index.html.erb +++ b/app/views/events/reconcile_affiliations/index.html.erb @@ -59,7 +59,8 @@ <% outcome_options = { create: [ [ "create", "Will be created", :blue ], [ "keep", "Skip", :gray ] ], deactivate: [ [ "keep", "Keep active", :green ], [ "delete", "Delete affiliation", :red ], [ "deactivate", "Deactivate affiliation", :red ] ], - delete: [ [ "delete", "Will be deleted", :red ], [ "keep", "Keep", :gray ] ] + delete: [ [ "delete", "Will be deleted", :red ], [ "keep", "Keep", :gray ] ], + retarget: [ [ "retarget", "Move to the new event", :blue ], [ "keep", "Leave as is", :gray ] ] } %> <% button_base = "group relative inline-flex items-center gap-2 rounded-md border border-gray-300 bg-white px-3 py-1.5 text-xs font-medium text-gray-500 shadow-sm cursor-pointer hover:bg-gray-50" %> diff --git a/db/schema.rb b/db/schema.rb index 00e3194cfe..98a839ec87 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -116,12 +116,14 @@ t.boolean "primary_contact", default: false, null: false t.date "start_date" t.string "title" + t.string "type" t.datetime "updated_at", precision: nil, null: false t.index ["event_registration_id"], name: "index_affiliations_on_event_registration_id" t.index ["organization_address_id"], name: "index_affiliations_on_organization_address_id" t.index ["organization_id"], name: "index_affiliations_on_organization_id" t.index ["person_id"], name: "index_affiliations_on_person_id" t.index ["title"], name: "index_affiliations_on_title" + t.index ["type"], name: "index_affiliations_on_type" end create_table "age_ranges", id: :integer, charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| diff --git a/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md b/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md index 1c263f4f4a..7e9fdcafd5 100644 --- a/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md +++ b/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md @@ -181,8 +181,15 @@ facilitator affiliation depends on what that row represents: history and retroactively flip the org from Ongoing to Reinstated at every training in between. -An end date is never written before the row's own start date; a row starting after -this training same-days instead. +**The end date is the day BEFORE the training**, matching +`AffiliationServices::ApplyScenarioEndDating` (main's ADR-0002 D4). A row that ends +the same day another starts counts on both, which doubles the person in any report +that totals a date. The cost is that the row then falls outside the training's own +anchor, so an organization whose only facilitator is ended this way reads +**Reinstated** at that training rather than Ongoing. Every earlier anchor is +unaffected, which is what D5's stability promise is about. + +An end date is never written before the row's own start date. **Deleting the minted row is what lets reconciliation stop relying on the `inactive` flag.** A same-dayed row could land on today and still read as active by dates @@ -251,6 +258,36 @@ judgement someone recorded, and the logged times are the evidence behind it — admin deciding whether to delete a facilitator affiliation should see which days were missed without leaving the page. +### D6d — A transfer moves the affiliation; it does not end it + +Transferring to another event isn't a failure to complete one — the person is 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 not cosmetic. The FK is the auto-vs-manual gate +(D2a), so a row dated to the destination event but still pointing at the source +registration would not be recognised as "the row this training minted" when the +destination is reconciled — it would be treated as an older row and end-dated. + +The rules: + +- **Which row moves** — this person's facilitator affiliation with this organization + that has **no end date**. One decision per (person, organization), not one per row. +- **An already-ended row stays put.** It records a finished stretch. With no open row + to move, the destination mints a fresh affiliation instead. +- **No destination recorded yet** (`transfer_destination_pending?`) — reported, not + guessed at. There is no date to move to. +- **Destination linked to a different organization** — reported. Re-dating this + organization's row to a training about another one would assert something false. +- **Chained transfers need no traversal.** A→B→C collapses when the second transfer + is made (`EventRegistrationsController#transfer` points C straight at A), so the + source's `transferred_to_registration` is already the final destination. + +This is deliberately *not* one of `LinkSubmittedOrganization::SCENARIOS`. Those +describe what kind of linking is happening and run at link time; a transfer is an +attendance outcome discovered at reconcile time, with nothing being linked. + ### D7 — What has to be tested The arithmetic is what the grant figures rest on, so it is covered directly rather @@ -272,7 +309,10 @@ than inferred from the single-affiliation cases minted row is deleted, the older row ended at the training date. 7. **`registered` after the event is reported, never acted on** — D6c, asserted on both the plan's reason and that the row survives. -8. **A return after a lapse adds a row and leaves the lapse intact** — D6a, asserted +8. **A transfer moves the row rather than ending it** — D6d: the open row re-dates + and re-points, an ended row is left alone with a fresh one created at the + destination, and a pending or differently-linked destination is reported. +9. **A return after a lapse adds a row and leaves the lapse intact** — D6a, asserted on both the row count and the mid-gap verdict. Adding a rule here means adding a case there. diff --git a/spec/services/affiliation_services/reconcile_person_spec.rb b/spec/services/affiliation_services/reconcile_person_spec.rb index 40497995c2..d53ca47a27 100644 --- a/spec/services/affiliation_services/reconcile_person_spec.rb +++ b/spec/services/affiliation_services/reconcile_person_spec.rb @@ -38,7 +38,7 @@ def reconcile(registration, **options) expect(Affiliation.exists?(affiliation.id)).to be(false) end - %w[ incomplete_attendance cancelled transferred_out ].each do |status| + %w[ incomplete_attendance cancelled ].each do |status| it "deletes the minted row when the only registration is #{status}" do reg = training_registration(status: status) affiliation = owned_facilitator(registration: reg) @@ -140,7 +140,9 @@ def reconcile(registration, **options) expect(hand_created.reload).not_to be_active end - it "ends an older affiliation at the training, keeping the years it really facilitated" do + # The day before, not the day of: a row ending the same day another starts would + # count on both and double the person in any report that totals a date. + it "ends an older affiliation the day before the training, keeping the years it really facilitated" do reg = training_registration(status: "no_show") started_on = 2.years.ago.to_date hand_created = create(:affiliation, person: person, organization: organization, @@ -148,7 +150,7 @@ def reconcile(registration, **options) reconcile(reg, include_unowned: true) - expect(hand_created.reload.end_date).to eq(reg.event.start_date.to_date) + expect(hand_created.reload.end_date).to eq(reg.event.start_date.to_date - 1.day) expect(hand_created.start_date).to eq(started_on) end @@ -225,6 +227,87 @@ def reconcile(registration, **options) end end + describe "a transfer out" do + # Source registration transferred out to `destination_event`, with the org + # carried onto the destination the way the transfer flow does. + def transferred_out(destination_event:, link_org: organization) + source = training_registration(status: "registered") + destination = create(:event_registration, registrant: person, event: destination_event, status: "registered") + create(:event_registration_organization, event_registration: destination, organization: link_org) if link_org + destination.update!(transferred_from_registration: source) + source.update!(status: "transferred_out") + [ source, destination ] + end + + let(:destination_event) do + create(:event, facilitator_training: true, start_date: 3.months.from_now, + end_date: 3.months.from_now + 1.day, + registration_close_date: 2.months.from_now) + end + + it "moves the open affiliation to the destination event and re-points its provenance" do + source, destination = transferred_out(destination_event: destination_event) + affiliation = owned_facilitator(registration: source) + + described_class.call(person: person, organization: organization, event: source.event, + registration: source, include_unowned: true) + + affiliation.reload + expect(affiliation.start_date).to eq(destination_event.start_date.to_date) + expect(affiliation.event_registration_id).to eq(destination.id) + expect(affiliation.end_date).to be_nil + end + + it "leaves an already-ended row alone and creates a fresh one at the destination" do + source, destination = transferred_out(destination_event: destination_event) + ended = create(:affiliation, person: person, organization: organization, title: "Facilitator", + start_date: 3.years.ago.to_date, end_date: 2.years.ago.to_date) + + expect { + described_class.call(person: person, organization: organization, event: source.event, + registration: source, include_unowned: true) + }.to change { person.affiliations.facilitators.where(organization: organization).count }.by(1) + + expect(ended.reload.end_date).to eq(2.years.ago.to_date) + fresh = person.affiliations.facilitators.where(organization: organization).order(:start_date).last + expect(fresh.start_date).to eq(destination_event.start_date.to_date) + expect(fresh.event_registration_id).to eq(destination.id) + end + + it "reports rather than guesses when the destination hasn't been recorded" do + source = training_registration(status: "registered") + source.update!(status: "transferred_out") + owned_facilitator(registration: source) + + plan = described_class.new(person: person, organization: organization, event: source.event, + registration: source, include_unowned: true).plan + + expect(plan.map(&:action)).to eq([ :noop ]) + expect(plan.first.reason).to eq(described_class::TRANSFER_PENDING) + end + + it "reports when the destination is linked to a different organization" do + source, _destination = transferred_out(destination_event: destination_event, link_org: create(:organization)) + affiliation = owned_facilitator(registration: source) + + described_class.call(person: person, organization: organization, event: source.event, + registration: source, include_unowned: true) + + expect(affiliation.reload.start_date).to eq(source.event.start_date.to_date) + expect(affiliation.event_registration_id).to eq(source.id) + end + + it "records why the affiliation moved" do + source, _destination = transferred_out(destination_event: destination_event) + affiliation = owned_facilitator(registration: source) + + described_class.call(person: person, organization: organization, event: source.event, + registration: source, include_unowned: true) + + expect(affiliation.reload.comments.last.body).to include("transferred out of") + end + end + describe "keeping / activating" do it "keeps the affiliation active when the person attended" do reg = training_registration(status: "attended") diff --git a/spec/services/facilitator_program_status_math_spec.rb b/spec/services/facilitator_program_status_math_spec.rb index e404784d2d..29134f4306 100644 --- a/spec/services/facilitator_program_status_math_spec.rb +++ b/spec/services/facilitator_program_status_math_spec.rb @@ -160,7 +160,10 @@ def bucket end describe "reconciliation does not move an anchored verdict" do - it "keeps the training-date status when a no-show's older affiliation is ended" do + # Ending the day before the training (ADR-0003 D6) puts the row outside the + # training's own anchor, so the org reads Reinstated there rather than Ongoing. + # The years before it are still intact — every earlier anchor is unchanged. + it "leaves every earlier anchor intact when a no-show's older affiliation is ended" do person = create(:person) event = create(:event, :ended, facilitator_training: true) anchor = event.start_date.to_date @@ -176,7 +179,8 @@ def bucket registration: registration, include_unowned: true ).perform(:deactivate, affiliation: older) - expect(status_on(anchor)).to eq(:ongoing) + expect(status_on(anchor)).to eq(:reinstated) + expect(status_on(anchor - 1.year)).to eq(:ongoing) expect(status_on(anchor + 1.year)).to eq(:reinstated) expect(bucket).to eq(:formerly_active) end From 15c1a2aa92bdb1665b01064cdb54bbc6d49d5e1c Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 23 Aug 2026 17:15:36 -0400 Subject: [PATCH 44/50] Cross-reference the two end-dating conventions 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) --- docs/adr/0002-org-linking-flows-and-agreement-scenarios.md | 5 +++++ .../0003-affiliations-as-the-record-of-two-relationships.md | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/adr/0002-org-linking-flows-and-agreement-scenarios.md b/docs/adr/0002-org-linking-flows-and-agreement-scenarios.md index de28e98751..22e683bb3e 100644 --- a/docs/adr/0002-org-linking-flows-and-agreement-scenarios.md +++ b/docs/adr/0002-org-linking-flows-and-agreement-scenarios.md @@ -103,6 +103,11 @@ the same date. Every affiliation a scenario ends is recorded on the submission new job is deliberately blunt, and the flag is how a wrongly-ended row (a multi-org facilitator changing only one job) gets corrected. +**Reconciliation ends rows the same way**, for the first of those reasons — +[ADR-0003](0003-affiliations-as-the-record-of-two-relationships.md) D6. Keep the two +in step: they are one convention with two callers, and a reader who finds only one +of them will assume it is local to that flow. + ### D5 — "Linked" on the submission side is explicit-link OR name-match; no join table A submission reads as linked when an org was explicitly linked to it, or when diff --git a/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md b/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md index 7e9fdcafd5..ab48cc86d4 100644 --- a/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md +++ b/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md @@ -182,7 +182,9 @@ facilitator affiliation depends on what that row represents: training in between. **The end date is the day BEFORE the training**, matching -`AffiliationServices::ApplyScenarioEndDating` (main's ADR-0002 D4). A row that ends +`AffiliationServices::ApplyScenarioEndDating` +([ADR-0002](0002-org-linking-flows-and-agreement-scenarios.md) D4, which points back +here — one convention, two callers). A row that ends the same day another starts counts on both, which doubles the person in any report that totals a date. The cost is that the row then falls outside the training's own anchor, so an organization whose only facilitator is ended this way reads From 0eaffdaf7f73e15086d022dbca4e9c2abbca7c6b Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 23 Aug 2026 17:22:41 -0400 Subject: [PATCH 45/50] Drop the case for ending some rows on the training date MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../reconcile_affiliations/_tooltip.html.erb | 2 +- .../reconcile_affiliations/confirm.html.erb | 2 +- .../reconcile_affiliations/index.html.erb | 2 +- ...ions-as-the-record-of-two-relationships.md | 46 +++++++++++-------- 4 files changed, 31 insertions(+), 21 deletions(-) diff --git a/app/views/events/reconcile_affiliations/_tooltip.html.erb b/app/views/events/reconcile_affiliations/_tooltip.html.erb index aaa03068d7..88fd407cf0 100644 --- a/app/views/events/reconcile_affiliations/_tooltip.html.erb +++ b/app/views/events/reconcile_affiliations/_tooltip.html.erb @@ -2,7 +2,7 @@ <% case kind %> <% when :deactivate %>

Ends this facilitator affiliation and marks it inactive, so it no longer counts as active. The record is kept — if the person is later marked attended, the return is recorded as a new affiliation rather than by reopening this one.

-

Offered for older affiliations only — ones this training didn't create. The end date is set to this training's date, so the years they did facilitate stay on the record.

+

Offered for older affiliations only — ones this training didn't create. The end date is set to the day before this training, so the years they did facilitate stay on the record.

A row this training created is deleted instead: it recorded an assumption that never came true.

<% when :delete %>

Permanently deletes this facilitator affiliation. Everything else stays as-is:

diff --git a/app/views/events/reconcile_affiliations/confirm.html.erb b/app/views/events/reconcile_affiliations/confirm.html.erb index ff2271c48e..252b996b71 100644 --- a/app/views/events/reconcile_affiliations/confirm.html.erb +++ b/app/views/events/reconcile_affiliations/confirm.html.erb @@ -12,7 +12,7 @@ <% sections = { create: [ "Create", "bg-blue-50 text-blue-800 border-blue-200", "A new facilitator affiliation is created for this organization." ], - deactivate: [ "Deactivate", "bg-red-50 text-red-800 border-red-200", "The facilitator affiliation is given an end date of this training's date and marked inactive, so the years before it stay on the record. Reversible." ], + deactivate: [ "Deactivate", "bg-red-50 text-red-800 border-red-200", "The facilitator affiliation is given an end date of the day before this training, so the years before it stay on the record. Reversible." ], retarget: [ "Move", "bg-blue-50 text-blue-800 border-blue-200", "The facilitator affiliation moves to the event they transferred to — re-dated to it and re-pointed at that registration. Nothing is ended." ], delete: [ "Delete", "bg-red-100 text-red-900 border-red-300", "The facilitator affiliation is permanently deleted — this is what happens to a row this training created for someone who didn't attend. Job and other-org affiliations are untouched." ] } %> diff --git a/app/views/events/reconcile_affiliations/index.html.erb b/app/views/events/reconcile_affiliations/index.html.erb index aa7c4badbe..2a08abc6bb 100644 --- a/app/views/events/reconcile_affiliations/index.html.erb +++ b/app/views/events/reconcile_affiliations/index.html.erb @@ -20,7 +20,7 @@

Only the row this training created is deleted — it recorded an assumption that never came true. An - older affiliation is ended on this training's date instead, so the period the person really + older affiliation is ended the day before this training instead, so the period the person really facilitated — and this organization's program status at every earlier training — stays as it was.

<% else %> diff --git a/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md b/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md index ab48cc86d4..0f87c925e4 100644 --- a/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md +++ b/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md @@ -175,30 +175,40 @@ facilitator affiliation depends on what that row represents: facilitator on the training date. They didn't, so there is no period to preserve and nothing is lost by removing it. It never counted as prior history anyway (ADR-0001 D5/D8 use a strict `<`), so no anchored verdict moves. -- **Any older row** — hand-entered, or minted by an earlier training — **ended** on - this training's start date, never deleted. It records facilitation that really - happened. Deleting it, or dating it back to its own start, would erase years of - history and retroactively flip the org from Ongoing to Reinstated at every - training in between. +- **Any older row** — hand-entered, or minted by an earlier training — **ended**, + never deleted. It records facilitation that really happened. Deleting it, or + dating it back to its own start, would erase years of history and retroactively + change the org's status at every training in between. -**The end date is the day BEFORE the training**, matching +**Everything ends the day BEFORE the training. One rule, no exceptions**, matching `AffiliationServices::ApplyScenarioEndDating` ([ADR-0002](0002-org-linking-flows-and-agreement-scenarios.md) D4, which points back -here — one convention, two callers). A row that ends -the same day another starts counts on both, which doubles the person in any report -that totals a date. The cost is that the row then falls outside the training's own -anchor, so an organization whose only facilitator is ended this way reads -**Reinstated** at that training rather than Ongoing. Every earlier anchor is -unaffected, which is what D5's stability promise is about. +here — one convention, two callers). + +Two reasons, and the second is the one that matters: + +1. A row ending the same day another starts counts on both, doubling the person in + any report that totals a date. +2. **The row is being ended precisely because we have no record the person ever + completed a training for this organization.** Ending it *on* the training date + would leave it counting as active on that date — so the organization's status at + that training would still rest on the affiliation we just decided wasn't valid. + Ending the day before is what the decision actually means. + +The consequence is that an organization whose only facilitator is ended this way +reads **Reinstated** at that training rather than Ongoing. That is not a cost being +paid for consistency: with the row gone there is no basis for Ongoing, and none of +the three labels describes "we removed the basis for their status" perfectly. +Reinstated is the honest one. Every earlier anchor is untouched, which is what D5's +stability promise is about. An end date is never written before the row's own start date. -**Deleting the minted row is what lets reconciliation stop relying on the `inactive` -flag.** A same-dayed row could land on today and still read as active by dates -alone, which is why D2's flag existed here. A deleted row has no such problem, and -an older row is ended on a training date that has already passed, so the date rule -derives `inactive` by itself. The flag is still set on the older row for the one -case that remains — reconciling on the day the training ends. +**This is also what lets reconciliation stop relying on the `inactive` flag.** The +minted row is deleted, so it can't linger; an older row ends the day before a +training that has already happened, so the date is always in the past and +`set_inactive_from_dates` derives the flag by itself. `deactivate` still sets it +explicitly, which is now belt-and-braces rather than load-bearing. **What a deletion costs:** the row's comments go with it, so the "why" D6b records survives only for ended rows. The deletion itself is still on the record as a From fbf98274039f69adee04435a6701c87ffdc57451 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Mon, 24 Aug 2026 07:18:47 -0400 Subject: [PATCH 46/50] Stop the comment-tooltip spec flaking on CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- spec/system/affiliation_filter_tabs_spec.rb | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/spec/system/affiliation_filter_tabs_spec.rb b/spec/system/affiliation_filter_tabs_spec.rb index ae144107b6..0b9e6264dc 100644 --- a/spec/system/affiliation_filter_tabs_spec.rb +++ b/spec/system/affiliation_filter_tabs_spec.rb @@ -49,9 +49,18 @@ def row_for(name) expect(all(".fa-comment").size).to eq(2) - all(".fa-comment").first.hover - - expect(page).to have_text("Note about the first", wait: 2) + # Hover is a one-shot mouse move, so unlike an assertion it doesn't retry: if + # the element shifts under the cursor while Stimulus settles the row styling, + # the move lands on nothing and no amount of waiting recovers it. Scroll it + # under the cursor, then re-hover if the first attempt missed. + icon = all(".fa-comment").first + page.execute_script("arguments[0].scrollIntoView({ block: 'center' })", icon) + 3.times do + icon.hover + break if page.has_text?("Note about the first", wait: 1) + end + + expect(page).to have_text("Note about the first") expect(page).to have_no_text("Note about the second") end From 87c424049e72aeae2635339bca4c687ebcb58bf2 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Mon, 24 Aug 2026 07:30:11 -0400 Subject: [PATCH 47/50] Test the tooltip's group scoping structurally, not by hovering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- spec/requests/affiliation_filter_tabs_spec.rb | 21 +++++++++++++++ spec/system/affiliation_filter_tabs_spec.rb | 27 ------------------- 2 files changed, 21 insertions(+), 27 deletions(-) diff --git a/spec/requests/affiliation_filter_tabs_spec.rb b/spec/requests/affiliation_filter_tabs_spec.rb index 372e0cd682..38fc465414 100644 --- a/spec/requests/affiliation_filter_tabs_spec.rb +++ b/spec/requests/affiliation_filter_tabs_spec.rb @@ -70,6 +70,27 @@ def rows_in(id) expect(indices.size).to eq(2) end + # The bug this guards: the tabs wrapper carried a bare `group`, and Tailwind's + # `group-hover:` matches ANY `.group` ancestor — so hovering anywhere in the + # editor opened every row's comment tooltip at once. Naming it (`group/afftabs`) + # scopes it. Asserted structurally rather than by driving a real hover, which + # headless Chrome on CI doesn't reliably deliver. + it "leaves each comment tooltip scoped to its own icon, not the tabs wrapper" do + current.comments.create!(body: "Why this ended") + + get edit_person_path(person) + + tooltips = parsed.css("span").select { |node| node["class"].to_s.include?("group-hover:block") } + expect(tooltips).not_to be_empty + + tooltips.each do |tooltip| + bare_groups = tooltip.ancestors.count { |a| a["class"].to_s.split(/\s+/).include?("group") } + expect(bare_groups).to eq(1), + "expected only the icon's own wrapper to be a bare `.group`, found #{bare_groups} — " \ + "an unnamed group on an ancestor makes every tooltip open at once" + end + end + it "adds new rows into the active group only" do get edit_person_path(person) diff --git a/spec/system/affiliation_filter_tabs_spec.rb b/spec/system/affiliation_filter_tabs_spec.rb index 0b9e6264dc..8523fdb394 100644 --- a/spec/system/affiliation_filter_tabs_spec.rb +++ b/spec/system/affiliation_filter_tabs_spec.rb @@ -37,33 +37,6 @@ def row_for(name) expect(row_for("Long Since Ended")).not_to be_visible end - # The tabs wrapper is a NAMED group. An unnamed one collided with the per-row - # comment icon's own `.group`, so hovering one icon opened every row's tooltip. - # Both commented rows must be on the SAME tab, or the hidden one masks the bug. - it "opens only the hovered row's comment tooltip" do - second = create(:affiliation, person: person, organization: create(:organization), - title: "Facilitator", start_date: 1.year.ago.to_date) - person.affiliations.find_by(organization: current_org).comments.create!(body: "Note about the first") - second.comments.create!(body: "Note about the second") - visit edit_person_path(person) - - expect(all(".fa-comment").size).to eq(2) - - # Hover is a one-shot mouse move, so unlike an assertion it doesn't retry: if - # the element shifts under the cursor while Stimulus settles the row styling, - # the move lands on nothing and no amount of waiting recovers it. Scroll it - # under the cursor, then re-hover if the first attempt missed. - icon = all(".fa-comment").first - page.execute_script("arguments[0].scrollIntoView({ block: 'center' })", icon) - 3.times do - icon.hover - break if page.has_text?("Note about the first", wait: 1) - end - - expect(page).to have_text("Note about the first") - expect(page).to have_no_text("Note about the second") - end - it "opens the affiliation editor's comments when the icon is clicked" do affiliation = person.affiliations.find_by(organization: current_org) affiliation.comments.create!(body: "Why this ended") From 09227f305dbc3af0386152ae3f1fb5e80c3113ed Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Mon, 24 Aug 2026 08:45:48 -0400 Subject: [PATCH 48/50] Reopen an ending reconciliation made, instead of adding a second affiliation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../affiliation_services/reconcile_event.rb | 2 +- .../affiliation_services/reconcile_person.rb | 27 +++++- .../reconcile_affiliations/_tooltip.html.erb | 3 + .../reconcile_affiliations/confirm.html.erb | 1 + .../reconcile_affiliations/index.html.erb | 8 +- ...ions-as-the-record-of-two-relationships.md | 82 +++++++++++++------ .../events/reconcile_affiliations_spec.rb | 37 +++++++++ .../reconcile_person_spec.rb | 50 +++++++++++ 8 files changed, 180 insertions(+), 30 deletions(-) diff --git a/app/services/affiliation_services/reconcile_event.rb b/app/services/affiliation_services/reconcile_event.rb index 36f0a70c24..cad79f5454 100644 --- a/app/services/affiliation_services/reconcile_event.rb +++ b/app/services/affiliation_services/reconcile_event.rb @@ -45,7 +45,7 @@ def any_rows? # One radio choice per row: the action itself, or "keep". ACTION_FOR_CHOICE = { "deactivate" => :deactivate, "delete" => :delete, "create" => :create, - "retarget" => :retarget }.freeze + "reactivate" => :reactivate, "retarget" => :retarget }.freeze # What the `{ row.key => choice }` map will change, for the confirmation screen. def planned_changes(outcome:) diff --git a/app/services/affiliation_services/reconcile_person.rb b/app/services/affiliation_services/reconcile_person.rb index dbdc7060a0..d13e2f953e 100644 --- a/app/services/affiliation_services/reconcile_person.rb +++ b/app/services/affiliation_services/reconcile_person.rb @@ -59,6 +59,7 @@ def perform(action, affiliation: nil) when :create then create_and_note when :delete then affiliation.destroy! when :deactivate then deactivate(affiliation) + when :reactivate then reactivate(affiliation) when :retarget then retarget(affiliation) else return false end @@ -90,6 +91,15 @@ def deactivate(affiliation) "for #{@event.title} — no attended facilitator training for #{@organization.name} on record.") end + # Undoes an ending this reconciliation applied for this same training, once the + # attendance it rested on changes. Reopening records no lapse because none + # happened — the row was ended by inference, not by an event (ADR-0003 D6a). + def reactivate(affiliation) + affiliation.update!(end_date: nil, inactive: false) + note(affiliation, "Reopened by reconciliation for #{@event.title} — #{@person.name} is now recorded as " \ + "having attended, so the ending this reconciliation applied no longer holds.") + end + # Moves the open row to the destination training: its start date becomes that # event's, and its provenance re-points at that registration so reconciling the # destination recognises it as the row that training minted (ADR-0003 D2a). @@ -128,6 +138,15 @@ def ended_by_reconciliation?(affiliation) affiliation.comments.any? { |comment| comment.topic == COMMENT_TOPIC } end + # This row is ended, and ended exactly where reconciling THIS training would end + # it. So the ending is ours and rests on the attendance we are now re-reading — + # not on a real lapse, which is the only thing a second row should record. + def ended_for_this_training?(affiliation) + return false if affiliation.end_date.nil? + + affiliation.end_date == deactivation_end_date(affiliation) && ended_by_reconciliation?(affiliation) + end + # The event is over and nobody said what happened. Distinct from cancelled or # transferred out, which are decisions; this is an unfilled roster. # @@ -215,11 +234,14 @@ def open_facilitator_affiliation end # Someone with no active facilitator affiliation needs one when they have never - # had one, or when they completed a training here and are returning after a - # lapse — the return is a NEW row, never a resurrected one (ADR-0003 D6a). + # had one, or when they completed a training here and are returning after a real + # lapse — that return is a NEW row (ADR-0003 D6a). def needs_affiliation? return false unless @registration return false if facilitator_affiliations.any?(&:active?) + # Reopening the row this training ended already gives them one; a create too + # would leave two facilitator affiliations for one unbroken engagement. + return false if reconcilable_affiliations.any? { |affiliation| ended_for_this_training?(affiliation) } facilitator_affiliations.empty? || completed_training? end @@ -227,6 +249,7 @@ def needs_affiliation? def classify(affiliation) if completed_training? return Decision.new(affiliation:, action: :noop, reason: ACTIVE_ATTENDED) if affiliation.active? + return Decision.new(affiliation:, action: :reactivate) if ended_for_this_training?(affiliation) Decision.new(affiliation:, action: :noop, reason: LAPSED) elsif !affiliation.active? diff --git a/app/views/events/reconcile_affiliations/_tooltip.html.erb b/app/views/events/reconcile_affiliations/_tooltip.html.erb index 88fd407cf0..da42ede98c 100644 --- a/app/views/events/reconcile_affiliations/_tooltip.html.erb +++ b/app/views/events/reconcile_affiliations/_tooltip.html.erb @@ -13,6 +13,9 @@ <% when :retarget %>

They transferred to another event, so the affiliation follows them: its start date becomes the new event's, and it re-points at that registration.

Nothing is ended or deleted. If their only affiliation here has already ended, a new one is created at the new event instead.

+ <% when :reactivate %> +

Clears the end date this reconciliation set for this training, because the person is now recorded as having attended it. No lapse happened, so nothing is lost by reopening.

+

Pick Create a new one instead if this really is a fresh engagement that should sit alongside the old one.

<% when :create %> Creates the facilitator affiliation for this organization. <% when :keep %> diff --git a/app/views/events/reconcile_affiliations/confirm.html.erb b/app/views/events/reconcile_affiliations/confirm.html.erb index 252b996b71..bc16594e64 100644 --- a/app/views/events/reconcile_affiliations/confirm.html.erb +++ b/app/views/events/reconcile_affiliations/confirm.html.erb @@ -12,6 +12,7 @@ <% sections = { create: [ "Create", "bg-blue-50 text-blue-800 border-blue-200", "A new facilitator affiliation is created for this organization." ], + reactivate: [ "Reactivate", "bg-green-50 text-green-800 border-green-200", "The end date this reconciliation set for this training is cleared, so the affiliation is active again. No second affiliation is created." ], deactivate: [ "Deactivate", "bg-red-50 text-red-800 border-red-200", "The facilitator affiliation is given an end date of the day before this training, so the years before it stay on the record. Reversible." ], retarget: [ "Move", "bg-blue-50 text-blue-800 border-blue-200", "The facilitator affiliation moves to the event they transferred to — re-dated to it and re-pointed at that registration. Nothing is ended." ], delete: [ "Delete", "bg-red-100 text-red-900 border-red-300", "The facilitator affiliation is permanently deleted — this is what happens to a row this training created for someone who didn't attend. Job and other-org affiliations are untouched." ] diff --git a/app/views/events/reconcile_affiliations/index.html.erb b/app/views/events/reconcile_affiliations/index.html.erb index 2a08abc6bb..5e8064e8e5 100644 --- a/app/views/events/reconcile_affiliations/index.html.erb +++ b/app/views/events/reconcile_affiliations/index.html.erb @@ -13,9 +13,10 @@

This step brings facilitator affiliations in line with who registered and attended. Before the training it creates any missing facilitator affiliations for linked organizations. After the training it - deletes the affiliation this training created for anyone who didn't attend. Someone later - marked attended gets a new affiliation dated to this training rather than having an old one - reopened, so a lapse stays visible. Anyone still marked Registered is left alone and listed + deletes the affiliation this training created for anyone who didn't attend. If that person is + later marked attended, an ending this step applied is undone rather than leaving them with a + second affiliation for an engagement that never broke. A return after a real lapse is still a + new affiliation, so the lapse stays visible. Anyone still marked Registered is left alone and listed below — that's an unfilled roster, not an outcome. Job affiliations are never touched.

@@ -60,6 +61,7 @@ create: [ [ "create", "Will be created", :blue ], [ "keep", "Skip", :gray ] ], deactivate: [ [ "keep", "Keep active", :green ], [ "delete", "Delete affiliation", :red ], [ "deactivate", "Deactivate affiliation", :red ] ], delete: [ [ "delete", "Will be deleted", :red ], [ "keep", "Keep", :gray ] ], + reactivate: [ [ "reactivate", "Will be reactivated", :green ], [ "create", "Create a new one instead", :blue ], [ "keep", "Leave inactive", :gray ] ], retarget: [ [ "retarget", "Move to the new event", :blue ], [ "keep", "Leave as is", :gray ] ] } %> <% button_base = "group relative inline-flex items-center gap-2 rounded-md border border-gray-300 bg-white px-3 py-1.5 text-xs font-medium text-gray-500 shadow-sm cursor-pointer hover:bg-gray-50" %> diff --git a/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md b/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md index 0f87c925e4..839b026356 100644 --- a/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md +++ b/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md @@ -2,6 +2,8 @@ - **Status:** Accepted - **Date:** 2026-08-19 +- **Revised:** 2026-08-24 — D6a reversed in part (reconciliation reopens an ending it + applied for the same training, instead of always adding a second row) - **Extends:** [ADR-0001](0001-organization-affiliation-and-program-status.md) (supersedes its "Active affiliation" vocabulary entry — see D2 below) @@ -217,28 +219,56 @@ survives only for ended rows. The deletion itself is still on the record as a The org's *current* bucket is expected to change — that's the point. Its *anchored* verdicts are not. -### D6a — A return after a lapse is a new row, never a reopened one - -When someone whose facilitator affiliation has ended completes a training for that -organization again, reconciliation **creates a second affiliation** dated to the new -training. It does not clear the old row's end date. - -Reopening it would swallow the gap: `Jan 2023 – Jan 2024, Aug 2026` collapses to -`Jan 2023`, and the organization retroactively reads Ongoing across years it was not -running a program. The lapse is the fact the two rows exist to record — ADR-0001 D2 -renders exactly that shape, and `CreateFromRegistration` has always minted a second -row rather than extending an ended one (an ended facilitator affiliation does not -block a new one). - -So there is no `:reactivate` action. An ended row is left alone with the reason -"Ended — a return is recorded as a new affiliation", and the return shows up as an -ordinary `:create`. The rule for proposing that create: the person has **no active** -facilitator affiliation for the org, and either never had one or has completed a -training here. - -This is the mirror of D6. D6 stops an ending from reaching too far back; D6a stops a -reactivation from reaching too far forward. Both exist because the historical readers -(D3) trust the dates. +### D6a — A real lapse is a new row; an ending we made ourselves is reopened + +**Revised 2026-08-24.** The original D6a said there is no `:reactivate` action at +all. That went too far: it produced two facilitator affiliations for one +organization in a case where nothing had actually lapsed, and the second row +implied an engagement that never ended and restarted. The distinction below is the +one that matters, and only the second half of the original decision survives. + +**The invariant this serves:** a person never holds two facilitator affiliations +for the same organization describing one unbroken engagement. A second row exists +only when the first one genuinely ended — independently of the training being +reconciled — and this is a second engagement. + +Two endings look alike in the data and mean opposite things: + +- **An ending reconciliation applied for *this* training.** The person was recorded + as not having completed it, so we ended the row by inference. When their + attendance is corrected — a roster filled in late, a no-show reversed — that + inference is simply wrong. **Reopen the row** (`:reactivate` clears `end_date` and + `inactive`). Nothing lapsed, so there is no gap to preserve and nothing to + record with a second row; leaving the ending in place and minting one alongside + would invent a break that never happened. +- **Any other ending** — an admin ended it, or an earlier training's reconciliation + did. That records a real stretch that finished. **Leave it**, reason "Ended — a + return is recorded as a new affiliation", and the return shows up as an ordinary + `:create`. Reopening it would swallow the gap: `Jan 2023 – Jan 2024, Aug 2026` + collapses to `Jan 2023`, and the organization retroactively reads Ongoing across + years it was not running a program. The lapse is the fact the two rows exist to + record — ADR-0001 D2 renders exactly that shape, and `CreateFromRegistration` has + always minted a second row rather than extending an ended one. + +**Telling them apart takes two signals, both required.** The row's `end_date` is +exactly what reconciling this training would write (`deactivation_end_date`), **and** +it carries a D6b reconciliation comment. The date alone is not enough — an admin who +happens to end a row the day before a training must not have it silently reopened — +and the comment alone is not enough, because an earlier training's reconciliation +leaves the same topic behind on a row that really did lapse. + +When the reopen applies, no `:create` is proposed for that person and organization. +The two are mutually exclusive by construction, which is what keeps the invariant +from depending on the admin picking the right button. + +**The admin can still overrule it.** A reopen row offers three outcomes: reopen it +(the default), create a new one alongside instead, or leave it inactive. The +correction-versus-second-engagement call is a judgement about what really happened, +and the data cannot always settle it. + +This remains the mirror of D6. D6 stops an ending from reaching too far back; D6a +stops a reactivation from reaching too far forward — but only reaching past an +ending that actually meant something. ### D6b — The reason a row changed lives in its comments @@ -324,8 +354,12 @@ than inferred from the single-affiliation cases 8. **A transfer moves the row rather than ending it** — D6d: the open row re-dates and re-points, an ended row is left alone with a fresh one created at the destination, and a pending or differently-linked destination is reported. -9. **A return after a lapse adds a row and leaves the lapse intact** — D6a, asserted - on both the row count and the mid-gap verdict. +9. **A return after a real lapse adds a row and leaves the lapse intact** — D6a, + asserted on both the row count and the mid-gap verdict. +10. **A corrected no-show reopens the row this training ended, and adds nothing** — + D6a, asserted by reconciling twice across the attendance change: the row count + must not move, and the row must come back active. Paired with a row an admin + ended on the same date, which must NOT reopen. Adding a rule here means adding a case there. diff --git a/spec/requests/events/reconcile_affiliations_spec.rb b/spec/requests/events/reconcile_affiliations_spec.rb index 0ffa1b2cb5..5b27bb42f3 100644 --- a/spec/requests/events/reconcile_affiliations_spec.rb +++ b/spec/requests/events/reconcile_affiliations_spec.rb @@ -126,6 +126,24 @@ def registrant_with_affiliation(status:) expect(response.body).not_to include("Partial attendance") end + it "offers reopen, create-instead and leave-inactive on a row this reconciliation ended" do + person = create(:person) + reg = create(:event_registration, event: event, registrant: person, status: "no_show") + create(:event_registration_organization, event_registration: reg, organization: organization) + older = create(:affiliation, person: person, organization: organization, title: "Facilitator", + start_date: 2.years.ago.to_date) + AffiliationServices::ReconcilePerson.call(person: person, organization: organization, + event: event, registration: reg, include_unowned: true) + reg.update!(status: "attended") + + get reconcile_affiliations_event_path(event) + + expect(response.body).to include("Will be reactivated") + expect(response.body).to include("Create a new one instead") + expect(response.body).to include("Leave inactive") + expect(older.reload).not_to be_active + end + it "denies a non-admin" do sign_in create(:user) @@ -205,6 +223,25 @@ def registrant_with_affiliation(status:) expect(affiliation.reload).not_to be_active end + it "reopens the ended row instead of leaving the person with two" do + person = create(:person) + reg = create(:event_registration, event: event, registrant: person, status: "no_show") + create(:event_registration_organization, event_registration: reg, organization: organization) + older = create(:affiliation, person: person, organization: organization, title: "Facilitator", + start_date: 2.years.ago.to_date) + AffiliationServices::ReconcilePerson.call(person: person, organization: organization, + event: event, registration: reg, include_unowned: true) + reg.update!(status: "attended") + + expect { + post perform_reconcile_affiliations_event_path(event), + params: { outcome: { "aff:#{older.id}" => "reactivate" } } + }.not_to change { person.affiliations.facilitators.where(organization: organization).count } + + expect(older.reload).to be_active + expect(older.end_date).to be_nil + end + it "spares a row set to keep" do _person, affiliation = registrant_with_affiliation(status: "no_show") diff --git a/spec/services/affiliation_services/reconcile_person_spec.rb b/spec/services/affiliation_services/reconcile_person_spec.rb index d53ca47a27..c23993ef70 100644 --- a/spec/services/affiliation_services/reconcile_person_spec.rb +++ b/spec/services/affiliation_services/reconcile_person_spec.rb @@ -342,6 +342,56 @@ def transferred_out(destination_event:, link_org: organization) expect(person.affiliations.facilitators.active.where(organization: organization).count).to eq(1) end + it "reopens the row it ended here once the person is marked attended, rather than adding a second" do + reg = training_registration(status: "no_show") + older = create(:affiliation, person: person, organization: organization, title: "Facilitator", + start_date: 2.years.ago.to_date) + + described_class.call(person: person, organization: organization, event: reg.event, + registration: reg, include_unowned: true) + expect(older.reload.end_date).to be_present + + reg.update!(status: "attended") + + expect { + described_class.call(person: person, organization: organization, event: reg.event, + registration: reg, include_unowned: true) + }.not_to change { person.affiliations.facilitators.where(organization: organization).count } + + expect(older.reload.end_date).to be_nil + expect(older).to be_active + expect(person.affiliations.facilitators.active.where(organization: organization).count).to eq(1) + end + + it "plans that reopen as :reactivate, with no create alongside it" do + reg = training_registration(status: "no_show") + create(:affiliation, person: person, organization: organization, title: "Facilitator", + start_date: 2.years.ago.to_date) + described_class.call(person: person, organization: organization, event: reg.event, + registration: reg, include_unowned: true) + reg.update!(status: "attended") + + plan = described_class.new(person: person, organization: organization, event: reg.event, + registration: reg, include_unowned: true).plan + + expect(plan.map(&:action)).to eq([ :reactivate ]) + end + + # Same end date, but nobody's reconciliation put it there — so it records a real + # lapse and the return belongs in its own row. + it "does not reopen a row an admin ended, even when the date matches" do + reg = training_registration(status: "attended") + create(:affiliation, person: person, organization: organization, title: "Facilitator", + start_date: 3.years.ago.to_date, + end_date: reg.event.start_date.to_date - 1.day) + + plan = described_class.new(person: person, organization: organization, event: reg.event, + registration: reg, include_unowned: true).plan + + expect(plan.map(&:action)).to contain_exactly(:noop, :create) + expect(plan.map(&:reason)).to include(described_class::LAPSED) + end + it "keeps the lapse visible instead of swallowing it into one unbroken stretch" do lapsed = create(:affiliation, person: person, organization: organization, title: "Facilitator", start_date: Date.new(2023, 1, 1), end_date: Date.new(2024, 1, 1)) From 873a6753d3d04135d1c7f60a5a09945b85be6b04 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Mon, 24 Aug 2026 08:53:53 -0400 Subject: [PATCH 49/50] Name the reconcile buttons by what they do, not what will happen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "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) --- .../reconcile_affiliations/_tooltip.html.erb | 2 +- .../events/reconcile_affiliations/index.html.erb | 6 +++--- ...iations-as-the-record-of-two-relationships.md | 6 ++++++ .../events/reconcile_affiliations_spec.rb | 16 ++++++++-------- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/app/views/events/reconcile_affiliations/_tooltip.html.erb b/app/views/events/reconcile_affiliations/_tooltip.html.erb index da42ede98c..55e30be4a0 100644 --- a/app/views/events/reconcile_affiliations/_tooltip.html.erb +++ b/app/views/events/reconcile_affiliations/_tooltip.html.erb @@ -15,7 +15,7 @@

Nothing is ended or deleted. If their only affiliation here has already ended, a new one is created at the new event instead.

<% when :reactivate %>

Clears the end date this reconciliation set for this training, because the person is now recorded as having attended it. No lapse happened, so nothing is lost by reopening.

-

Pick Create a new one instead if this really is a fresh engagement that should sit alongside the old one.

+

Pick Create new if this really is a fresh engagement that should sit alongside the old one.

<% when :create %> Creates the facilitator affiliation for this organization. <% when :keep %> diff --git a/app/views/events/reconcile_affiliations/index.html.erb b/app/views/events/reconcile_affiliations/index.html.erb index 5e8064e8e5..a17fee32d1 100644 --- a/app/views/events/reconcile_affiliations/index.html.erb +++ b/app/views/events/reconcile_affiliations/index.html.erb @@ -58,10 +58,10 @@ gray: "has-[:checked]:bg-gray-100 has-[:checked]:text-gray-800 has-[:checked]:border-gray-400" } %> <% outcome_options = { - create: [ [ "create", "Will be created", :blue ], [ "keep", "Skip", :gray ] ], + create: [ [ "create", "Create new", :blue ], [ "keep", "Skip", :gray ] ], deactivate: [ [ "keep", "Keep active", :green ], [ "delete", "Delete affiliation", :red ], [ "deactivate", "Deactivate affiliation", :red ] ], - delete: [ [ "delete", "Will be deleted", :red ], [ "keep", "Keep", :gray ] ], - reactivate: [ [ "reactivate", "Will be reactivated", :green ], [ "create", "Create a new one instead", :blue ], [ "keep", "Leave inactive", :gray ] ], + delete: [ [ "delete", "Delete", :red ], [ "keep", "Keep", :gray ] ], + reactivate: [ [ "reactivate", "Reactivate", :green ], [ "create", "Create new", :blue ], [ "keep", "Leave inactive", :gray ] ], retarget: [ [ "retarget", "Move to the new event", :blue ], [ "keep", "Leave as is", :gray ] ] } %> <% button_base = "group relative inline-flex items-center gap-2 rounded-md border border-gray-300 bg-white px-3 py-1.5 text-xs font-medium text-gray-500 shadow-sm cursor-pointer hover:bg-gray-50" %> diff --git a/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md b/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md index 839b026356..8956b25da7 100644 --- a/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md +++ b/docs/adr/0003-affiliations-as-the-record-of-two-relationships.md @@ -250,6 +250,12 @@ Two endings look alike in the data and mean opposite things: record — ADR-0001 D2 renders exactly that shape, and `CreateFromRegistration` has always minted a second row rather than extending an ended one. +**A reopen is tied to the training whose attendance changed, and only that one.** +Correcting the attendance for training A reopens the row A's reconciliation ended. +Attending a *later* training B while A's ending stands does not: the stretch really +did finish at A and restart at B, so B adds a row. Reconciling B never reaches back +into A's ending. + **Telling them apart takes two signals, both required.** The row's `end_date` is exactly what reconciling this training would write (`deactivation_end_date`), **and** it carries a D6b reconciliation comment. The date alone is not enough — an admin who diff --git a/spec/requests/events/reconcile_affiliations_spec.rb b/spec/requests/events/reconcile_affiliations_spec.rb index 5b27bb42f3..dc6b2f0d8e 100644 --- a/spec/requests/events/reconcile_affiliations_spec.rb +++ b/spec/requests/events/reconcile_affiliations_spec.rb @@ -27,7 +27,7 @@ def registrant_with_affiliation(status:) expect(response).to have_http_status(:ok) expect(response.body).to include(person.name) - expect(response.body).to include("Will be deleted") + expect(response.body).to include("Delete") end it "previews an older hand-entered row as an end-date, not a deletion" do @@ -50,7 +50,7 @@ def registrant_with_affiliation(status:) get reconcile_affiliations_event_path(upcoming) - expect(response.body).to include("Will be created") + expect(response.body).to include("Create new") end it "previews a facilitator affiliation on a non-training event as a deletion" do @@ -58,12 +58,12 @@ def registrant_with_affiliation(status:) person = create(:person) reg = create(:event_registration, event: non_training, registrant: person, status: "attended") create(:event_registration_organization, event_registration: reg, organization: organization) - create(:affiliation, person: person, organization: organization, title: "Facilitator", - start_date: 1.month.ago.to_date, event_registration: reg) + affiliation = create(:affiliation, person: person, organization: organization, title: "Facilitator", + start_date: 1.month.ago.to_date, event_registration: reg) get reconcile_affiliations_event_path(non_training) - expect(response.body).to include("Will be deleted") + expect(response.body).to include("outcome_aff:#{affiliation.id}_delete") end it "lists a no-action registrant under Not reconciled with the reason and attendance status" do @@ -95,7 +95,7 @@ def registrant_with_affiliation(status:) expect(response.body).to include(person.name) expect(response.body).to include("Attendance never recorded") - expect(response.body).not_to include("Will be deleted") + expect(response.body).not_to include("outcome_aff:#{affiliation.id}_") expect(Affiliation.exists?(affiliation.id)).to be(true) end @@ -138,8 +138,8 @@ def registrant_with_affiliation(status:) get reconcile_affiliations_event_path(event) - expect(response.body).to include("Will be reactivated") - expect(response.body).to include("Create a new one instead") + expect(response.body).to include("Reactivate") + expect(response.body).to include("Create new") expect(response.body).to include("Leave inactive") expect(older.reload).not_to be_active end From 05d4e3a1ab5b519fdb06e278750aa3fda1750597 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Mon, 24 Aug 2026 09:33:27 -0400 Subject: [PATCH 50/50] Tick the days from the reconcile page when attendance was never recorded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../affiliation_services/reconcile_person.rb | 2 +- .../_attendance_day_ticks.html.erb | 15 ++++++++++++++ .../reconcile_affiliations/index.html.erb | 15 +++++++++----- .../events/reconcile_affiliations_spec.rb | 20 +++++++++++++++++++ 4 files changed, 46 insertions(+), 6 deletions(-) create mode 100644 app/views/events/reconcile_affiliations/_attendance_day_ticks.html.erb diff --git a/app/services/affiliation_services/reconcile_person.rb b/app/services/affiliation_services/reconcile_person.rb index d13e2f953e..6f6c6f0c99 100644 --- a/app/services/affiliation_services/reconcile_person.rb +++ b/app/services/affiliation_services/reconcile_person.rb @@ -20,7 +20,7 @@ def actionable? ACTIVE_ATTENDED = "Active — attended".freeze TRAINING_PENDING = "Training hasn't ended yet".freeze ALREADY_DEACTIVATED = "Already deactivated — didn't attend".freeze - ALREADY_ENDED = "Already ended — not by reconciliation".freeze + ALREADY_ENDED = "FA end date is in the past — not reconciled".freeze LAPSED = "Ended — a return is recorded as a new affiliation".freeze # Topic on the comment reconciliation leaves behind, so a row can say why it # ended without a dedicated column (ADR-0003 D6b). diff --git a/app/views/events/reconcile_affiliations/_attendance_day_ticks.html.erb b/app/views/events/reconcile_affiliations/_attendance_day_ticks.html.erb new file mode 100644 index 0000000000..12b7d994f5 --- /dev/null +++ b/app/views/events/reconcile_affiliations/_attendance_day_ticks.html.erb @@ -0,0 +1,15 @@ +<%# Locals: registration, event. + + Attendance was never recorded, so the fix is upstream of this page: ticking the + days a person actually came rolls the status forward on its own + (EventRegistration#sync_attendance_status_to_days!), and the shared onboarding + cell answers with a turbo_stream that repaints the tick and the status chip. %> +
+ <% EventRegistration::DAY_FIELDS.first(event.day_count).each_with_index do |field, index| %> + + <% end %> +
diff --git a/app/views/events/reconcile_affiliations/index.html.erb b/app/views/events/reconcile_affiliations/index.html.erb index a17fee32d1..66931e271c 100644 --- a/app/views/events/reconcile_affiliations/index.html.erb +++ b/app/views/events/reconcile_affiliations/index.html.erb @@ -57,11 +57,12 @@ blue: "has-[:checked]:bg-blue-50 has-[:checked]:text-blue-700 has-[:checked]:border-blue-300", gray: "has-[:checked]:bg-gray-100 has-[:checked]:text-gray-800 has-[:checked]:border-gray-400" } %> + <% fa_hint = "facilitator affiliation (FA)" %> <% outcome_options = { - create: [ [ "create", "Create new", :blue ], [ "keep", "Skip", :gray ] ], + create: [ [ "create", "Create new FA", :blue, fa_hint ], [ "keep", "Skip", :gray ] ], deactivate: [ [ "keep", "Keep active", :green ], [ "delete", "Delete affiliation", :red ], [ "deactivate", "Deactivate affiliation", :red ] ], delete: [ [ "delete", "Delete", :red ], [ "keep", "Keep", :gray ] ], - reactivate: [ [ "reactivate", "Reactivate", :green ], [ "create", "Create new", :blue ], [ "keep", "Leave inactive", :gray ] ], + reactivate: [ [ "reactivate", "Reactivate", :green ], [ "create", "Create new FA", :blue, fa_hint ], [ "keep", "Leave inactive", :gray ] ], retarget: [ [ "retarget", "Move to the new event", :blue ], [ "keep", "Leave as is", :gray ] ] } %> <% button_base = "group relative inline-flex items-center gap-2 rounded-md border border-gray-300 bg-white px-3 py-1.5 text-xs font-medium text-gray-500 shadow-sm cursor-pointer hover:bg-gray-50" %> @@ -69,7 +70,7 @@

To reconcile (<%= actionable_count %>)

- These buttons change facilitator affiliations + May change affiliations — preview to confirm.
@@ -111,8 +112,8 @@ <%= row.organization.name %> <% end %>
- <% outcome_options[row.action].each do |value, label, color| %> -
+ <% ticked = Set.new %> <% @skipped_sections.each do |reason, rows| %>
@@ -159,6 +161,9 @@ <%= row.organization.name %> <% end %> + <% if reason == AffiliationServices::ReconcilePerson::ATTENDANCE_NOT_RECORDED && ticked.add?(row.registration&.id) %> + <%= render "attendance_day_ticks", registration: row.registration, event: @event %> + <% end %> <%= render "event_registrations/attendance_status_badge", registration: row.registration, return_to: "reconcile_affiliations" %> <% if row.registration&.status == "incomplete_attendance" %>
<%= render "attendance_days", registration: row.registration, event: @event %>
diff --git a/spec/requests/events/reconcile_affiliations_spec.rb b/spec/requests/events/reconcile_affiliations_spec.rb index dc6b2f0d8e..f64239458a 100644 --- a/spec/requests/events/reconcile_affiliations_spec.rb +++ b/spec/requests/events/reconcile_affiliations_spec.rb @@ -88,6 +88,26 @@ def registrant_with_affiliation(status:) expect(response.body).to include("Deactivate affiliation") end + it "offers day ticks beside the chip when attendance was never recorded" do + person, _affiliation = registrant_with_affiliation(status: "registered") + registration = person.event_registrations.first + + get reconcile_affiliations_event_path(event) + + expect(response.body).to include("Day 1") + expect(response.body).to include("completed_day_1_event_registration_#{registration.id}") + end + + it "renders one set of day ticks for a registrant who linked two organizations" do + person, _affiliation = registrant_with_affiliation(status: "registered") + registration = person.event_registrations.first + create(:event_registration_organization, event_registration: registration, organization: create(:organization)) + + get reconcile_affiliations_event_path(event) + + expect(response.body.scan("completed_day_1_event_registration_#{registration.id}").size).to eq(1) + end + it "asks for an outcome instead of acting when attendance was never recorded" do person, affiliation = registrant_with_affiliation(status: "registered")