diff --git a/AGENTS.md b/AGENTS.md index 375ebb393d..e03d3a5af4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -510,7 +510,7 @@ RuboCop linting on PRs and pushes to main. ## Rake Tasks -Located in `lib/tasks/` (9 files): +Located in `lib/tasks/` (10 files): - `dev.rake` — Development database seeding from XML/CSV - `rhino_migrator.rake` — Rich text editor migration - `attachment_report.rake` — Attachment reporting @@ -520,4 +520,4 @@ Located in `lib/tasks/` (9 files): - `migrate_sectors.rake` — Sector data migration - `import_stories.rake` — Imports stories from a WordPress Posts Export CSV (`StoryImporter`) - `migrate_workshop_logs.rake` — Workshop log migration -- `migrate_sectors.rake` — Sector data migration +- `backfill_affiliation_facilitator.rake` — One-off post-deploy backfill of `affiliations.facilitator` from the title diff --git a/app/frontend/javascript/controllers/affiliation_dates_controller.js b/app/frontend/javascript/controllers/affiliation_dates_controller.js index 88013eed64..7c4731f5f6 100644 --- a/app/frontend/javascript/controllers/affiliation_dates_controller.js +++ b/app/frontend/javascript/controllers/affiliation_dates_controller.js @@ -1,4 +1,5 @@ import { Controller } from "@hotwired/stimulus" +import { isFacilitatorTitle } from "../lib/affiliation" export default class extends Controller { static targets = ["facilitatorSince", "affiliatedNote", "affiliatedNoteText", "memberSinceFlag", "affiliationsContainer", "programStatus"] @@ -51,11 +52,7 @@ export default class extends Controller { const now = new Date() const today = new Date(Date.UTC(now.getFullYear(), now.getMonth(), now.getDate())) - // Mirrors Affiliation#facilitator?: exact, case-sensitive, trimmed — so the - // live figure matches the server render. - const facilitatorAffiliations = affiliations.filter(a => - a.title.trim() === "Facilitator" - ) + const facilitatorAffiliations = affiliations.filter(a => isFacilitatorTitle(a.title)) const facStartDates = facilitatorAffiliations.map(a => a.startDate).filter(Boolean) const facilitatorSince = facStartDates.length ? new Date(Math.min(...facStartDates.map(d => new Date(d)))) diff --git a/app/frontend/javascript/controllers/affiliation_facilitator_warning_controller.js b/app/frontend/javascript/controllers/affiliation_facilitator_warning_controller.js index 702edbf6a8..4260502dd9 100644 --- a/app/frontend/javascript/controllers/affiliation_facilitator_warning_controller.js +++ b/app/frontend/javascript/controllers/affiliation_facilitator_warning_controller.js @@ -1,4 +1,5 @@ import { Controller } from "@hotwired/stimulus"; +import { isFacilitatorTitle } from "../lib/affiliation"; // Connects to data-controller="affiliation-facilitator-warning" // @@ -65,8 +66,7 @@ export default class extends Controller { startDate, endDate, destroyed, - // Mirror Affiliation#facilitator?: exact, case-sensitive "Facilitator" (trimmed). - facilitator: title.trim() === "Facilitator", + facilitator: isFacilitatorTitle(title), }; } diff --git a/app/frontend/javascript/controllers/inactive_toggle_controller.js b/app/frontend/javascript/controllers/inactive_toggle_controller.js index 5dad1705e6..3083aeff31 100644 --- a/app/frontend/javascript/controllers/inactive_toggle_controller.js +++ b/app/frontend/javascript/controllers/inactive_toggle_controller.js @@ -1,4 +1,5 @@ import { Controller } from "@hotwired/stimulus"; +import { isFacilitatorTitle } from "../lib/affiliation"; // Live styling for the affiliation editor row as you edit, before saving. Four // states by colour: role is the hue (facilitator = purple, else blue) and status @@ -92,9 +93,7 @@ export default class extends Controller { return this.expiredValue; } - // Mirror Affiliation#facilitator? — an exact, case-sensitive match on - // "Facilitator" (trimmed), so the live styling matches what the server renders. isFacilitator() { - return this.hasTitleTarget && this.titleTarget.value.trim() === "Facilitator"; + return this.hasTitleTarget && isFacilitatorTitle(this.titleTarget.value); } } diff --git a/app/frontend/javascript/lib/affiliation.js b/app/frontend/javascript/lib/affiliation.js new file mode 100644 index 0000000000..32fb8aa5af --- /dev/null +++ b/app/frontend/javascript/lib/affiliation.js @@ -0,0 +1,9 @@ +// The single JS source of truth for "is this the standing Facilitator +// affiliation?", mirroring Ruby's Affiliation#facilitator? / .facilitators: the +// title must be *exactly* "Facilitator" (trimmed, case-sensitive). Variants like +// "Lead Facilitator" or "facilitator" are deliberately excluded. The affiliation +// editors drive their live preview off the typed title, so they compare the input +// value through this helper rather than the persisted boolean. +export const facilitatorTitle = "Facilitator" + +export const isFacilitatorTitle = (title) => (title ?? "").trim() === facilitatorTitle diff --git a/app/models/affiliation.rb b/app/models/affiliation.rb index 94f9fd650c..3a4e5c16d2 100644 --- a/app/models/affiliation.rb +++ b/app/models/affiliation.rb @@ -49,11 +49,10 @@ class Affiliation < ApplicationRecord .where("affiliations.end_date IS NULL OR affiliations.end_date >= ?", date) } - # Only the exact, case-sensitive title "Facilitator" counts — variants like - # "Lead Facilitator" or "facilitator" are deliberately excluded. BINARY forces - # a case-sensitive comparison under MySQL's default case-insensitive collation; - # TRIM mirrors the in-memory #facilitator? strip so stray whitespace still matches. - scope :facilitators, -> { where("BINARY TRIM(title) = ?", "Facilitator") } + # Reads the denormalized `facilitator` flag, which #sync_facilitator_from_title + # keeps in lock-step with the title rule (exactly "Facilitator", trimmed, + # case-sensitive). An executable agreement spec locks the column to that rule. + scope :facilitators, -> { where(facilitator: true) } # Affiliations whose #status_on(date) equals the given status, expressed in SQL # so it composes as a subquery (e.g. person-id narrowing). Kept in lock-step with @@ -78,6 +77,7 @@ class Affiliation < ApplicationRecord end } + before_validation :sync_facilitator_from_title before_validation :skip_if_duplicate # Runs before validation so a reassigned org drops its stale organization_address_id # before organization_address_belongs_to_organization would reject it. @@ -89,13 +89,11 @@ class Affiliation < ApplicationRecord after_destroy :sync_organization_affiliation_dates # Methods - # A facilitator affiliation is one whose title is *exactly* "Facilitator" - # (trimmed, case-sensitive). Variants like "Lead Facilitator" or "facilitator" - # are deliberately excluded. Mirrors the .facilitators scope so in-memory and - # SQL checks agree. - def facilitator? - title.to_s.strip == "Facilitator" - end + # `facilitator?` is the boolean column's auto-generated reader. A facilitator + # affiliation is one whose title is *exactly* "Facilitator" (trimmed, + # case-sensitive); #sync_facilitator_from_title keeps the column in step with + # that rule on every save, so #facilitator? and the .facilitators scope agree. + # Variants like "Lead Facilitator" or "facilitator" are deliberately excluded. # Current: not flagged inactive and not past its end date. Mirrors the `active` # scope so already-loaded affiliations can be filtered in Ruby without another @@ -169,6 +167,14 @@ def set_inactive_from_dates self.inactive = end_date.present? && end_date < Date.current end + # Keep the denormalized flag in step with the title rule (exactly "Facilitator", + # trimmed, case-sensitive) on every save, so the .facilitators scope and + # #facilitator? agree. Invariant: never write `title` via update_columns / + # update_all — that skips this callback and lets the flag drift. + def sync_facilitator_from_title + self.facilitator = title.to_s.strip == FACILITATOR_TITLE + end + def sync_organization_affiliation_dates org = organization affiliations = org.affiliations.where.not(id: destroyed_by_association ? id : nil) diff --git a/db/migrate/20260819125941_add_facilitator_to_affiliations.rb b/db/migrate/20260819125941_add_facilitator_to_affiliations.rb new file mode 100644 index 0000000000..3dc80927e7 --- /dev/null +++ b/db/migrate/20260819125941_add_facilitator_to_affiliations.rb @@ -0,0 +1,15 @@ +class AddFacilitatorToAffiliations < ActiveRecord::Migration[8.1] + # Denormalized cache of "is this the standing Facilitator affiliation?", kept in + # sync from the title by Affiliation. Replaces the raw BINARY TRIM(title) scope. + # Schema only — existing rows are backfilled by the affiliations:backfill_facilitator + # rake task after deploy (see lib/tasks). + def up + return if column_exists?(:affiliations, :facilitator) + + add_column :affiliations, :facilitator, :boolean, null: false, default: false + end + + def down + remove_column :affiliations, :facilitator, if_exists: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 1581f5f647..2e7223bb04 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -108,6 +108,7 @@ t.datetime "created_at", precision: nil, null: false t.date "end_date" t.bigint "event_registration_id" + t.boolean "facilitator", default: false, null: false t.string "filemaker_code" t.boolean "inactive", default: false, null: false t.bigint "organization_address_id" diff --git a/lib/tasks/backfill_affiliation_facilitator.rake b/lib/tasks/backfill_affiliation_facilitator.rake new file mode 100644 index 0000000000..490ca9b1b1 --- /dev/null +++ b/lib/tasks/backfill_affiliation_facilitator.rake @@ -0,0 +1,11 @@ +namespace :affiliations do + desc "Backfill the affiliations.facilitator flag from the title (one-off, post-deploy)" + task backfill_facilitator: :environment do + # Same rule as the retired .facilitators SQL scope: exactly "Facilitator", + # trimmed, case-sensitive. update_all is deliberate — the value is computed + # inline, so no per-row callback is needed and this stays a single bulk write. + scope = Affiliation.where("BINARY TRIM(title) = ?", "Facilitator") + count = scope.update_all(facilitator: true) + puts "Backfilled facilitator: true on #{count} affiliation(s)." + end +end diff --git a/spec/models/affiliation_spec.rb b/spec/models/affiliation_spec.rb index fa1ff6a1a2..3915524bfc 100644 --- a/spec/models/affiliation_spec.rb +++ b/spec/models/affiliation_spec.rb @@ -118,26 +118,43 @@ end end - describe '#facilitator?' do + describe '#facilitator? (synced from title on validation)' do + # #facilitator? reads the denormalized column, which sync_facilitator_from_title + # sets in before_validation — so validate before reading it. + def facilitator_flag(title) + build(:affiliation, title: title).tap(&:validate).facilitator? + end + it 'is true for the exact title "Facilitator"' do - expect(build(:affiliation, title: "Facilitator").facilitator?).to be true + expect(facilitator_flag("Facilitator")).to be true end it 'ignores surrounding whitespace' do - expect(build(:affiliation, title: " Facilitator ").facilitator?).to be true + expect(facilitator_flag(" Facilitator ")).to be true end it 'is false for title variants like "Lead Facilitator"' do - expect(build(:affiliation, title: "Lead Facilitator").facilitator?).to be false + expect(facilitator_flag("Lead Facilitator")).to be false end it 'is case-sensitive' do - expect(build(:affiliation, title: "facilitator").facilitator?).to be false - expect(build(:affiliation, title: "FACILITATOR").facilitator?).to be false + expect(facilitator_flag("facilitator")).to be false + expect(facilitator_flag("FACILITATOR")).to be false end it 'is false when the title is blank' do - expect(build(:affiliation, title: nil).facilitator?).to be false + expect(facilitator_flag(nil)).to be false + end + + it 'flips the column when a row is retitled to or from "Facilitator"' do + affiliation = create(:affiliation, title: "Facilitator") + expect(affiliation.facilitator?).to be true + + affiliation.update!(title: "Lead Facilitator") + expect(affiliation.reload.facilitator?).to be false + + affiliation.update!(title: "Facilitator") + expect(affiliation.reload.facilitator?).to be true end end @@ -150,6 +167,17 @@ it 'includes only the exact, case-sensitive title "Facilitator" (whitespace-trimmed)' do expect(described_class.facilitators).to contain_exactly(exact, whitespace) end + + it 'returns exactly the rows whose title matches the rule (column ↔ scope agree)' do + expected = described_class.all.select { |a| a.title.to_s.strip == "Facilitator" }.map(&:id).sort + expect(described_class.facilitators.ids.sort).to eq(expected) + end + + it 'keeps the persisted facilitator column in step with the title rule' do + described_class.find_each do |affiliation| + expect(affiliation.facilitator).to eq(affiliation.title.to_s.strip == "Facilitator") + end + end end describe '#sync_organization_status_with_affiliations' do