From 0c00624b297bf60bc92e102ef012d5013169246b Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 06:14:32 -0400 Subject: [PATCH 1/6] Build org-answer entries in one place The org name/position/website/type/address answers were shaped into the same hash in four places, so every new org field (like country) had to be added to each one and they could drift. FormSubmission.org_entry_from now builds it, and ProfileDiff.for_entry reads it. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../event_registrations_controller.rb | 42 ++----------------- .../form_submissions_controller.rb | 8 +--- app/models/form_submission.rb | 15 ++++++- app/services/event_registration_importer.rb | 12 +----- .../capture_from_submission.rb | 24 +++++------ .../link_submitted_organization.rb | 7 +--- .../organization_services/profile_diff.rb | 5 +++ spec/models/form_submission_spec.rb | 22 ++++++++++ .../profile_diff_spec.rb | 9 ++++ 9 files changed, 68 insertions(+), 76 deletions(-) diff --git a/app/controllers/event_registrations_controller.rb b/app/controllers/event_registrations_controller.rb index 4a750e9ebd..0106bf832d 100644 --- a/app/controllers/event_registrations_controller.rb +++ b/app/controllers/event_registrations_controller.rb @@ -685,8 +685,7 @@ def csv_export(registrations) end end - # Each registration-form submission with the org name and position the registrant - # entered on it: [{ submission:, org_name:, position: }], oldest first. Memoized: + # Each registration-form submission's FormSubmission#org_entry, oldest first. Memoized: # the linking actions ask for these entries half a dozen times per request # (profile, address, position, then again to build the notice). def registration_submission_entries(registration) @@ -698,39 +697,11 @@ def build_registration_submission_entries(registration) form = registration.event.registration_form return [] unless form - organization_identifiers = %w[ - organization_name organization_position organization_website organization_type - organization_street organization_city organization_state organization_zip organization_country - ] - field_ids = form.form_fields - .where(field_identifier: organization_identifiers) - .pluck(:field_identifier, :id).to_h - entries = registration.registrant.form_submissions .where(form: form) .order(:created_at) - .includes(:form_answers) - .map do |submission| - answers = submission.form_answers.index_by(&:form_field_id) - answer = ->(identifier) do - id = field_ids[identifier] - id && answers[id]&.submitted_answer - end - { - submission: submission, - org_name: answer.call("organization_name"), - position: answer.call("organization_position"), - website: answer.call("organization_website"), - organization_type: answer.call("organization_type"), - address: { - street_address: answer.call("organization_street"), - city: answer.call("organization_city"), - state: answer.call("organization_state"), - zip_code: answer.call("organization_zip"), - country: answer.call("organization_country") - } - } - end + .includes(form_answers: :form_field) + .map(&:org_entry) # Attach the matching DB org (if any) for each submitted name, so the view can # show its city/state next to the answer. Batched to avoid a query per entry. @@ -813,12 +784,7 @@ def profile_diff_for(organization, linked_count: nil) entry = submission_entry_for(@event_registration, organization, linked_count: linked_count) return [] unless entry - OrganizationServices::ProfileDiff.call( - organization: organization, - website: entry[:website], - organization_type: entry[:organization_type], - address: entry[:address] || {} - ) + OrganizationServices::ProfileDiff.for_entry(organization: organization, entry: entry) end # Link the org to the registration, fill what it's missing from the submission diff --git a/app/controllers/form_submissions_controller.rb b/app/controllers/form_submissions_controller.rb index 159c8422a2..61a1530669 100644 --- a/app/controllers/form_submissions_controller.rb +++ b/app/controllers/form_submissions_controller.rb @@ -154,13 +154,7 @@ def registration_linked_organization end def profile_diff_for(organization) - entry = submission_org_entry - OrganizationServices::ProfileDiff.call( - organization: organization, - website: entry[:website], - organization_type: entry[:organization_type], - address: entry[:address] - ) + OrganizationServices::ProfileDiff.for_entry(organization: organization, entry: submission_org_entry) end # Fill the org's blank profile/address fields from the submission, create the diff --git a/app/models/form_submission.rb b/app/models/form_submission.rb index 78cc037b3b..e28b90007b 100644 --- a/app/models/form_submission.rb +++ b/app/models/form_submission.rb @@ -131,10 +131,21 @@ def self.org_name_answers # OrganizationServices::LinkSubmittedOrganization reads them. Shared by the # admin linking editor and the auto-processing a public submission runs. def org_entry - answers = answers_by_identifier + self.class.org_entry_from(answers_by_identifier).merge(submission: self) + end + + # The same org entry from a bare field_identifier => answer hash, for callers + # holding answers that aren't saved on a submission (the spreadsheet importer, + # a public form mid-submit). + ORG_ENTRY_FIELD_IDENTIFIERS = [ + FormField::ORGANIZATION_NAME_FIELD_IDENTIFIER, FormField::ORGANIZATION_POSITION_FIELD_IDENTIFIER, + "organization_website", "organization_type", "organization_street", "organization_city", + "organization_state", "organization_zip", "organization_country" + ].freeze + + def self.org_entry_from(answers) read = ->(identifier) { answers[identifier].presence } { - submission: self, org_name: read.call(FormField::ORGANIZATION_NAME_FIELD_IDENTIFIER), position: read.call(FormField::ORGANIZATION_POSITION_FIELD_IDENTIFIER), website: read.call("organization_website"), diff --git a/app/services/event_registration_importer.rb b/app/services/event_registration_importer.rb index e6e20aeacf..b61fd9f537 100644 --- a/app/services/event_registration_importer.rb +++ b/app/services/event_registration_importer.rb @@ -849,16 +849,8 @@ def capture_organization(person, registration, submission, organization, answers # The org answers fill-blanks won't apply, from the comparison the link-organization # flow already uses. def organization_declines(organization, answers) - OrganizationServices::ProfileDiff.call( - organization: organization, - website: answers["organization_website"], - organization_type: answers["organization_type"], - address: { - street_address: answers["organization_street"], city: answers["organization_city"], - state: answers["organization_state"], zip_code: answers["organization_zip"], - country: answers["organization_country"] - } - ).map { |discrepancy| [ discrepancy.label, discrepancy.submitted, discrepancy.saved ] } + OrganizationServices::ProfileDiff.for_entry(organization: organization, entry: FormSubmission.org_entry_from(answers)) + .map { |discrepancy| [ discrepancy.label, discrepancy.submitted, discrepancy.saved ] } end def enroll_ce(registration, values) diff --git a/app/services/organization_services/capture_from_submission.rb b/app/services/organization_services/capture_from_submission.rb index abe0303e52..eeb5969666 100644 --- a/app/services/organization_services/capture_from_submission.rb +++ b/app/services/organization_services/capture_from_submission.rb @@ -51,26 +51,18 @@ def call profile_changes = SyncProfile.call( organization: organization, - website: field_value("organization_website"), - organization_type: field_value("organization_type"), + website: entry[:website], + organization_type: entry[:organization_type], overwrite: @overwrite, submission: @submission ).changes - address_result = UpsertAddress.call( - organization: organization, - street_address: field_value("organization_street"), - city: field_value("organization_city"), - state: field_value("organization_state"), - zip_code: field_value("organization_zip"), - country: field_value("organization_country"), - overwrite: @overwrite - ) + address_result = UpsertAddress.call(organization: organization, overwrite: @overwrite, **entry[:address]) AffiliationServices::CreateFromRegistration.call( person: @person, organization: organization, - job_title: field_value(FormField::ORGANIZATION_POSITION_FIELD_IDENTIFIER), + job_title: entry[:position], training_date: @training_date, organization_address: address_result.address, facilitator_training: @facilitator_training, @@ -85,12 +77,18 @@ def call def find_organization return @organization if @organization - name = field_value(FormField::ORGANIZATION_NAME_FIELD_IDENTIFIER)&.strip + name = entry[:org_name]&.strip return nil if name.blank? Organization.find_by(name: name) end + def entry + @entry ||= FormSubmission.org_entry_from( + FormSubmission::ORG_ENTRY_FIELD_IDENTIFIERS.index_with { |identifier| field_value(identifier) } + ) + end + def field_value(key) return @answers[key] if @answers diff --git a/app/services/organization_services/link_submitted_organization.rb b/app/services/organization_services/link_submitted_organization.rb index b082901f9e..502b111f99 100644 --- a/app/services/organization_services/link_submitted_organization.rb +++ b/app/services/organization_services/link_submitted_organization.rb @@ -118,12 +118,7 @@ def upsert_address def conflicts return [] unless @entry - OrganizationServices::ProfileDiff.call( - organization: @organization, - website: @entry[:website], - organization_type: @entry[:organization_type], - address: submitted_address - ) + ProfileDiff.for_entry(organization: @organization, entry: @entry) end def submitted_address diff --git a/app/services/organization_services/profile_diff.rb b/app/services/organization_services/profile_diff.rb index be9a36c262..7a07bee2c1 100644 --- a/app/services/organization_services/profile_diff.rb +++ b/app/services/organization_services/profile_diff.rb @@ -20,6 +20,11 @@ def self.call(organization:, website: nil, organization_type: nil, address: {}) new(organization:, website:, organization_type:, address:).call end + # Compares the website, type, and address of a FormSubmission#org_entry-shaped hash. + def self.for_entry(organization:, entry:) + call(organization:, website: entry[:website], organization_type: entry[:organization_type], address: entry[:address]) + end + def initialize(organization:, website: nil, organization_type: nil, address: {}) @organization = organization @website = website diff --git a/spec/models/form_submission_spec.rb b/spec/models/form_submission_spec.rb index 9e1266f6d7..487d9ca2e1 100644 --- a/spec/models/form_submission_spec.rb +++ b/spec/models/form_submission_spec.rb @@ -111,6 +111,28 @@ end end + describe ".org_entry_from" do + it "shapes org answers by identifier, blanks as nil" do + entry = described_class.org_entry_from( + "organization_name" => "Acme", "organization_position" => "", "organization_city" => "Denver", "organization_state" => "CO" + ) + + expect(entry).to include(org_name: "Acme", position: nil, website: nil) + expect(entry[:address]).to include(city: "Denver", state: "CO", street_address: nil) + end + end + + describe "#org_entry" do + it "builds the entry from the submission's answers and carries the submission" do + form = create(:form) + field = create(:form_field, form: form, field_identifier: "organization_name", name: "Organization") + submission = create(:form_submission, form: form) + submission.form_answers.create!(form_field: field, submitted_answer: "Acme") + + expect(submission.org_entry).to include(submission: submission, org_name: "Acme") + end + end + describe "index filter scopes" do def submission_with_org_answer(org_name, person: create(:person, user: nil)) submission = create(:form_submission, person: person) diff --git a/spec/services/organization_services/profile_diff_spec.rb b/spec/services/organization_services/profile_diff_spec.rb index f8943a6286..52df0b36f6 100644 --- a/spec/services/organization_services/profile_diff_spec.rb +++ b/spec/services/organization_services/profile_diff_spec.rb @@ -61,6 +61,15 @@ expect(diff.map(&:field)).to contain_exactly(:website_url, :organization_type) end + it "reads website, type, and address from an org entry" do + organization.update!(organization_type: "For-profit", website_url: "https://acme.org") + entry = FormSubmission.org_entry_from("organization_website" => "https://other.org", "organization_type" => "Government agency") + + diff = described_class.for_entry(organization: organization, entry: entry) + + expect(diff.map(&:field)).to contain_exactly(:website_url, :organization_type) + end + describe "address" do it "flags street, ZIP and country that differ from the org's same-city address" do create(:address, addressable: organization, street_address: "5 Oak Ave", city: "Austin", state: "TX", zip_code: "78701", country: "USA") From 5c3c5f314dbc4ccdc092186b8804dc683a56ca31 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 07:19:23 -0400 Subject: [PATCH 2/6] Count org-entry identifiers as read by the submission pipelines CaptureFromSubmission now reads org answers through FormSubmission::ORG_ENTRY_FIELD_IDENTIFIERS rather than literal field_value calls, so the coverage spec's scrape missed them. Co-Authored-By: Claude Opus 5.5 (1M context) --- spec/services/smart_form_fields_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/services/smart_form_fields_spec.rb b/spec/services/smart_form_fields_spec.rb index 58d8229125..6413dbe518 100644 --- a/spec/services/smart_form_fields_spec.rb +++ b/spec/services/smart_form_fields_spec.rb @@ -48,7 +48,7 @@ app/services/organization_services/capture_from_submission.rb app/services/public_form_submission.rb ].map { |path| Rails.root.join(path).read }.join("\n") - read_identifiers = sources.scan(/field_value\("([a-z0-9_]+)"\)/).flatten.uniq + read_identifiers = (sources.scan(/field_value\("([a-z0-9_]+)"\)/).flatten + FormSubmission::ORG_ENTRY_FIELD_IDENTIFIERS).uniq documented = described_class.identifiers + described_class::ANSWER_ONLY_IDENTIFIERS # Guards the guard: a regex that stopped matching would pass vacuously. From 38772cd26a4fe4ce319070487a18fac6d4377731 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 07:27:08 -0400 Subject: [PATCH 3/6] Chart registrant locations by where they work Admins want the background charts to show where registrants work, not where they live. Cities, states, and countries now read the org linked on the registration, at the address picked on that registration's affiliation, else the org's oldest address. The drill-ins follow so a clicked row lists the same people; the roster's State/County filters keep matching home addresses. Also builds org_entry_from off the field-map constants so the identifier list and the hash can't drift. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 5 +- app/controllers/events_controller.rb | 43 ++------ app/helpers/events_helper.rb | 2 +- app/models/form_submission.rb | 35 +++--- app/presenters/registrant_city_breakdown.rb | 24 ++-- app/services/attendees_active_filters.rb | 10 +- app/services/attendees_breakdowns.rb | 41 ++----- app/services/event_dashboard.rb | 104 +++++------------- app/services/registration_work_addresses.rb | 71 ++++++++++++ .../events/_registrant_breakdowns.html.erb | 4 +- app/views/events/_registrant_filters.html.erb | 4 +- ...5-registrant-charts-use-work-locations.yml | 13 +++ .../registrant_city_breakdown_spec.rb | 21 ++-- spec/requests/events/attendees_spec.rb | 16 ++- spec/requests/events_spec.rb | 8 +- .../services/attendees_active_filters_spec.rb | 3 +- spec/services/attendees_breakdowns_spec.rb | 11 +- spec/services/event_dashboard_spec.rb | 45 ++++---- .../registration_work_addresses_spec.rb | 45 ++++++++ 19 files changed, 276 insertions(+), 229 deletions(-) create mode 100644 app/services/registration_work_addresses.rb create mode 100644 db/seeds/features/2026-10-25-registrant-charts-use-work-locations.yml create mode 100644 spec/services/registration_work_addresses_spec.rb diff --git a/AGENTS.md b/AGENTS.md index 1c3a76085b..1b215dbc0b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -236,7 +236,8 @@ comparison — add a rule if none fits (e.g. `PersonPolicy#own_profile?`). Conve - `EventProgramStatusReport` — Cross-event program-status report grouped by calendar year: how many organizations were New / Ongoing / Reinstate at each facilitator training, counted both as organization-trainings (summed rows) and as distinct organizations classified at their earliest training. Sibling of `EventRevenueReport`/`EventParticipationReport`/`EventScholarshipReport` (includes `ReportPeriods`); powers the `events#program_statuses` page and the reports-hub program-status card - `EventParticipationReport` — Cross-event participation report grouped by calendar year (unique people trained vs attended seats vs per-status outcome counts, chart series) for the events participation page; sibling of `EventRevenueReport` - `AttendeesRoster` — Cross-event counterpart to `EventDashboard`: builds the per-registrant lookup maps the shared `events/_registrant_roster` partial reads (sector/age/org/status/location/scholarship/CE plus the events-attended column) for a paginated page of people; backs the `events#attendees` index. Takes `events:` + `registrations:` — the index's current filter scopes, already narrowed by `EventPolicy`'s `:reportable` scope — so a person's columns show what's in scope rather than their whole history, and never an event the viewer can't see -- `AttendeesBreakdowns` — Aggregate counterpart to `EventDashboard`'s breakdown methods: computes the chart datasets (sectors, age groups, locations, program status, life experiences, settings, organizations, scholarship/CE) over an arbitrary people set, profile-sourced, for the shared `events/_registrant_breakdowns` partial. Backs the `events#attendees` index charts (cross-event; `events:` / `registrations:` = the index's current filter scopes, already narrowed by `EventPolicy`'s `:reportable` scope) and the `events#recipients` charts frame (one event's scholarship recipients, `registrations:` = `EventRegistration.active` so it counts them regardless of attendance, `as_of:` = that event's start date so program status reads the same verdict its dashboard does). Also exposes `*_registrant_ids_by_*` maps mirroring `EventDashboard`'s, so a breakdown row can drill in by person id — they regroup rows already loaded for the counts, adding no queries +- `RegistrationWorkAddresses` — Where registrants work, per registration: each org linked on the registration (`EventRegistrationOrganization`), at the address picked on the registration's affiliation with it (`Affiliation#organization_address`), else the org's oldest active address. Groups registrant ids by US state, country, and "City, State". The single source for the registrant charts' cities, states, and countries (`EventDashboard`, `AttendeesBreakdowns`) and the attendees index's `country`/`org_state`/`org_city` drill-ins; the roster's State filter still reads registrants' own addresses (`EventDashboard#registrant_address_states`) +- `AttendeesBreakdowns` — Aggregate counterpart to `EventDashboard`'s breakdown methods: computes the chart datasets (sectors, age groups, locations, program status, life experiences, settings, organizations, scholarship/CE) over an arbitrary people set — tags from profiles, cities/states/countries from `RegistrationWorkAddresses` — for the shared `events/_registrant_breakdowns` partial. Backs the `events#attendees` index charts (cross-event; `events:` / `registrations:` = the index's current filter scopes, already narrowed by `EventPolicy`'s `:reportable` scope) and the `events#recipients` charts frame (one event's scholarship recipients, `registrations:` = `EventRegistration.active` so it counts them regardless of attendance, `as_of:` = that event's start date so program status reads the same verdict its dashboard does). Also exposes `*_registrant_ids_by_*` maps mirroring `EventDashboard`'s, so a breakdown row can drill in by person id — they regroup rows already loaded for the counts, adding no queries - `AttendeesActiveFilters` — Human-readable, removable-chip descriptors for a page's "drill-in" filters (registrant_ids, organization, org city, age group / life experience / setting categories, country, school district, scholarship, CE, payment status, scholarship funding source, sector, state) — params that narrow a list without a field in a visible filter form. `CHIP_PARAMS` is the attendees index's set (omitting anything with its own control); pass `chip_params:` for a page with no filter form at all — `ROSTER_CHIP_PARAMS` for the per-event roster, `%w[ registrant_ids ]` for scholarship recipients - `ReportPeriods` — Shared module (included by `EventRevenueReport` and `EventParticipationReport`) resolving the reporting-hub period toggle (this year / last year / all time) to a metric scope + label for the summary cards - `EventScholarshipReport` — Cross-event scholarship report grouped by calendar year: scholarship dollars and award counts (funded vs unfunded, via `EventScholarshipFigures`) per facilitator training, plus an attended-trainee count split into "Live" (scheduled instructor-led) vs "On-demand" (`event.on_demand?`). Sibling of `EventRevenueReport`/`EventParticipationReport` (includes `ReportPeriods`); powers the `events#scholarships` report page and the reports-hub scholarship summary card @@ -287,7 +288,7 @@ comparison — add a rule if none fits (e.g. `PersonPolicy#own_profile?`). Conve - `FeatureCatalog` — Syncs the checked-in feature seeds (`db/seeds/features/*.yml`, one file per feature) into the `Feature` table behind the `/features` page (the "Sync latest updates" button). `#import!` (matched by `name`) loads existing features in one query, creates missing ones (case-insensitive name match), overwrites `SYNCED_FIELDS` (area, display_status, released_on, action_path, pr_number, summary, pro_tips, external_url, rhino_description — rich text compared as rendered HTML) on same-name records with whatever the seed carries, so seed corrections and trims propagate; skips any feature edited in-app since creation (`Feature#edited?`) and writes without bumping `updated_at`; returns a `Result` (`created`/`updated`/`skipped`) - `DisplayImagePresenter` — Image display logic - `ScholarshipsGrouping` (presenter) — Groups scholarships into the index's funder → grant → recipient hierarchy; grant-free awards collect under a trailing "Unfunded" group -- `RegistrantCityBreakdown` (presenter) — Groups an event's registrants by the city of the org linked on their registration, counting registrants + scholarship recipients per city; drives the shared "Registrants by city" card inside `events/_registrant_breakdowns` on all three people-pages — per-event roster, cross-event attendees index, and scholarship recipients (fed plucked data by `EventDashboard` or `AttendeesBreakdowns`) +- `RegistrantCityBreakdown` (presenter) — Groups an event's registrants by the city they work in (`RegistrationWorkAddresses`), counting registrants + scholarship recipients per city; drives the shared "Registrants by city" card inside `events/_registrant_breakdowns` on all three people-pages — per-event roster, cross-event attendees index, and scholarship recipients (fed plucked data by `EventDashboard` or `AttendeesBreakdowns`) - `AllocationLedgerLabel` (presenter) — Shared payment-method/label + check-number labelling for an allocation, used by the invoice and receipt ledgers so they can't drift - `InvoiceIssuer` (presenter) — Single source of truth for the issuing org's invoice/receipt header (name, address, email, tax id, payable-to note), shared by `EventInvoice`, `EventReceipt`, and `InvoicePresenter`. Name and tax id come from `Setting.app_organization`; the addresses and email are app-wide settings. `#address_lines` / `#remittance_address_lines` delegate to `Setting`, which reads the `Address` the row points at (`return_address` / `remittance_address`) and otherwise falls back to the app org's oldest address flagged for invoices (`Address.display_lines_for`); there are no hard-coded address literals. The email is `Setting.info_email`. The header carries the return address; `#remittance_address_lines` is the separate address cheques are mailed to, printed under the payable-to line on both invoices and on the pay-by-check callout diff --git a/app/controllers/events_controller.rb b/app/controllers/events_controller.rb index bc70710f9c..772175bf2b 100644 --- a/app/controllers/events_controller.rb +++ b/app/controllers/events_controller.rb @@ -714,18 +714,16 @@ def filtered_roster_registrants end # Person ids behind the clicked breakdown row. Most dimensions arrive as an - # explicit id list (EventDashboard already has the maps); sector and state have - # no per-value id map, so they arrive as named filters and resolve here — bounded - # to `registrant_ids` (this event's roster), since the result is only ever + # explicit id list (EventDashboard already has the maps); sector has no per-value + # id map, so it arrives as a named filter and resolves here — bounded to + # `registrant_ids` (this event's roster), since the result is only ever # intersected with it and the unbounded lookup would load every person in the app - # tagged with that sector or living in that state. + # tagged with that sector. def roster_drill_in_ids(registrant_ids) if params[:registrant_ids].present? params[:registrant_ids].to_s.split("-").map(&:to_i).to_set elsif params[:sector].present? person_sector_ids(params[:sector]).where(sectorable_id: registrant_ids).pluck(:sectorable_id).to_set - elsif params[:state].present? - person_address_ids(state: params[:state]).where(addressable_id: registrant_ids).pluck(:addressable_id).to_set end end @@ -855,11 +853,12 @@ def filtered_attendees # Breakdown drill-in filters (set when a chart row is clicked). scope = scope.where(id: person_category_ids(params[:age_group])) if params[:age_group].present? scope = scope.where(id: person_category_ids(params[:life_experience])) if params[:life_experience].present? - scope = scope.where(id: person_country_ids(params[:country])) if params[:country].present? + scope = scope.where(id: attendee_work_addresses.registrant_ids_by_country.fetch(params[:country], [])) if params[:country].present? + scope = scope.where(id: attendee_work_addresses.registrant_ids_by_state.fetch(params[:org_state], [])) if params[:org_state].present? scope = scope.where(id: person_school_district_ids(params[:school_district])) if params[:school_district].present? scope = scope.where(id: person_program_status_ids(params[:program_status])) if params[:program_status].present? scope = scope.where(id: person_linked_organization_ids(params[:organization_id])) if params[:organization_id].present? - scope = scope.where(id: person_linked_org_city_ids(params[:org_city])) if params[:org_city].present? + scope = scope.where(id: attendee_work_addresses.registrant_ids_by_city_label.fetch(params[:org_city], [])) if params[:org_city].present? if params[:scholarship].present? # Scoped to one event, every status (including "no") is unambiguous at the # registration level, so reuse the shared scope. Cross-event, "No scholarship" @@ -950,10 +949,6 @@ def person_category_ids(category_id) CategorizableItem.where(categorizable_type: "Person", category_id: category_id).select(:categorizable_id) end - def person_country_ids(country) - Address.active.where(addressable_type: "Person", country: country).select(:addressable_id) - end - # People whose event-registration-linked affiliation points at an organization # address in the given school district (Affiliation#organization_address → # Address#district), scoped to the in-scope registrations — matching the @@ -974,26 +969,10 @@ def person_linked_organization_ids(organization_id) .select(Arel.sql("event_registrations.registrant_id")) end - # People whose linked training org sits in the given "City, State" (matched on - # each org's first active address, mirroring the cities breakdown's labeling). - def person_linked_org_city_ids(city_label) - org_ids = org_ids_by_city_label[city_label] || [] - return Person.none if org_ids.empty? - person_linked_organization_ids(org_ids) - end - - def org_ids_by_city_label - @org_ids_by_city_label ||= Address.active - .where(addressable_type: "Organization", - addressable_id: EventRegistrationOrganization.where(event_registration_id: attendee_registrations.select(:id)).select(:organization_id)) - .order(:id) - .pluck(:addressable_id, :city, :state) - .each_with_object({}) do |(org_id, city, state), first_label| - next if first_label.key?(org_id) - first_label[org_id] = [ city, state ].compact_blank.join(", ").presence - end - .group_by { |_org_id, label| label } - .transform_values { |pairs| pairs.map(&:first) } + # Where each in-scope registration's registrant works, behind the country, org + # state, and org city drill-ins — the same source as the breakdowns they come from. + def attendee_work_addresses + @attendee_work_addresses ||= RegistrationWorkAddresses.new(attendee_registrations.pluck(:id)) end # Anchored the same way the index's own column is: this list spans events, so diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb index 479880bfd7..332ef35fb8 100644 --- a/app/helpers/events_helper.rb +++ b/app/helpers/events_helper.rb @@ -239,7 +239,7 @@ def time_period_to_event_year(time_period) REPORT_SUBNAV_PARAMS = %i[ event_id event_type search funder_sgid time_period view registrant_ids attendance_status payment_status funder scholarship - sector contact_info affiliation_status organization_id org_city state county + sector contact_info affiliation_status organization_id org_city org_state state county age_group life_experience country school_district program_status ].freeze diff --git a/app/models/form_submission.rb b/app/models/form_submission.rb index e28b90007b..e305c32a59 100644 --- a/app/models/form_submission.rb +++ b/app/models/form_submission.rb @@ -134,30 +134,27 @@ def org_entry self.class.org_entry_from(answers_by_identifier).merge(submission: self) end + ORG_ENTRY_FIELDS = { + org_name: FormField::ORGANIZATION_NAME_FIELD_IDENTIFIER, + position: FormField::ORGANIZATION_POSITION_FIELD_IDENTIFIER, + website: "organization_website", + organization_type: "organization_type" + }.freeze + ORG_ADDRESS_FIELDS = { + street_address: "organization_street", + city: "organization_city", + state: "organization_state", + zip_code: "organization_zip", + country: "organization_country" + }.freeze + ORG_ENTRY_FIELD_IDENTIFIERS = (ORG_ENTRY_FIELDS.values + ORG_ADDRESS_FIELDS.values).freeze + # The same org entry from a bare field_identifier => answer hash, for callers # holding answers that aren't saved on a submission (the spreadsheet importer, # a public form mid-submit). - ORG_ENTRY_FIELD_IDENTIFIERS = [ - FormField::ORGANIZATION_NAME_FIELD_IDENTIFIER, FormField::ORGANIZATION_POSITION_FIELD_IDENTIFIER, - "organization_website", "organization_type", "organization_street", "organization_city", - "organization_state", "organization_zip", "organization_country" - ].freeze - def self.org_entry_from(answers) read = ->(identifier) { answers[identifier].presence } - { - org_name: read.call(FormField::ORGANIZATION_NAME_FIELD_IDENTIFIER), - position: read.call(FormField::ORGANIZATION_POSITION_FIELD_IDENTIFIER), - website: read.call("organization_website"), - organization_type: read.call("organization_type"), - address: { - street_address: read.call("organization_street"), - city: read.call("organization_city"), - state: read.call("organization_state"), - zip_code: read.call("organization_zip"), - country: read.call("organization_country") - } - } + ORG_ENTRY_FIELDS.transform_values(&read).merge(address: ORG_ADDRESS_FIELDS.transform_values(&read)) end # The registration-org row a submission is pinned to — the link public diff --git a/app/presenters/registrant_city_breakdown.rb b/app/presenters/registrant_city_breakdown.rb index 14514f9394..6c68782b66 100644 --- a/app/presenters/registrant_city_breakdown.rb +++ b/app/presenters/registrant_city_breakdown.rb @@ -1,24 +1,21 @@ -# Groups an event's registrants by the city of the organization linked on their -# registration, counting distinct registrants and scholarship recipients per -# city. Powers the shared "Registrants by city" breakdown on the event +# Groups an event's registrants by the city they work in (RegistrationWorkAddresses), +# counting distinct registrants and scholarship recipients per city. Powers the shared "Registrants by city" breakdown on the event # background dashboard and the scholarship-recipients page. Pure/in-memory so # EventDashboard feeds it already-plucked data (see [[event_dashboard]]). # # A registrant linked to orgs in two cities counts under each; the "Unknown" -# bucket (a linked org with no city on file) always sorts last. +# bucket (a linked org with no address on file) always sorts last. class RegistrantCityBreakdown UNKNOWN_LABEL = "Unknown".freeze Row = Struct.new(:city, :registrant_count, :scholarship_count, :registrant_ids, :scholarship_recipient_ids, keyword_init: true) - # org_registrant_pairs: [ [ organization_id, person_id ], ... ], one per - # organization linked on a registration. - # city_by_org: { organization_id => "City, State" }; a missing - # entry means that org has no city ⇒ Unknown. + # city_registrant_pairs: [ [ "City, State", person_id ], ... ], one per + # organization linked on a registration; a nil + # city ⇒ Unknown. # scholarship_recipient_ids: person ids awarded a scholarship this event. - def initialize(org_registrant_pairs:, city_by_org:, scholarship_recipient_ids:) - @org_registrant_pairs = org_registrant_pairs - @city_by_org = city_by_org + def initialize(city_registrant_pairs:, scholarship_recipient_ids:) + @city_registrant_pairs = city_registrant_pairs @scholarship_recipient_ids = scholarship_recipient_ids.to_set end @@ -49,9 +46,8 @@ def any? private def registrant_ids_by_city - @registrant_ids_by_city ||= @org_registrant_pairs.each_with_object(Hash.new { |h, k| h[k] = Set.new }) do |(org_id, person_id), map| - city = @city_by_org[org_id].presence || UNKNOWN_LABEL - map[city] << person_id + @registrant_ids_by_city ||= @city_registrant_pairs.each_with_object(Hash.new { |h, k| h[k] = Set.new }) do |(city, person_id), map| + map[city.presence || UNKNOWN_LABEL] << person_id end end end diff --git a/app/services/attendees_active_filters.rb b/app/services/attendees_active_filters.rb index ad6d86d185..0db76e1d71 100644 --- a/app/services/attendees_active_filters.rb +++ b/app/services/attendees_active_filters.rb @@ -15,12 +15,12 @@ class AttendeesActiveFilters # honours but this list omits narrows the page silently AND is dropped by the # filter form's hidden fields on the next change, since both read this list. CHIP_PARAMS = %w[ - registrant_ids organization_id org_city age_group life_experience + registrant_ids organization_id org_city org_state age_group life_experience country school_district payment_status funder ].freeze # The roster has no filter form at all, so every drill-in it accepts needs a - # chip — including the two the index leaves out because it has controls for them. - ROSTER_CHIP_PARAMS = %w[ registrant_ids sector state ].freeze + # chip — including sector, which the index leaves out because it has a control for it. + ROSTER_CHIP_PARAMS = %w[ registrant_ids sector ].freeze def initialize(params, chip_params: CHIP_PARAMS) @params = params @@ -45,12 +45,12 @@ def label_for(param, value) case param when "registrant_ids" then "#{value.to_s.split("-").size} selected people" when "sector" then record_label("Sector", Sector, value) - when "state" then "State: #{value}" when "organization_id" then record_label("Organization", Organization, value) when "org_city" then "Org city: #{value}" + when "org_state" then "Org state: #{value}" when "age_group" then record_label("Age group", Category, value) when "life_experience" then record_label("Life experience", Category, value) - when "country" then "Country: #{value}" + when "country" then "Org country: #{value}" when "school_district" then "School district: #{value}" when "payment_status" then option_label("Payment", EventRegistration::PAYMENT_STATUS_FILTER_OPTIONS, value) when "funder" then option_label("Funding", EventRegistration::FUNDER_FILTER_OPTIONS, value) diff --git a/app/services/attendees_breakdowns.rb b/app/services/attendees_breakdowns.rb index 58d5222dbe..9931451365 100644 --- a/app/services/attendees_breakdowns.rb +++ b/app/services/attendees_breakdowns.rb @@ -3,8 +3,8 @@ # whole filtered attendee population (not one page). Method names/return shapes # match EventDashboard so the shared _registrant_breakdowns partial reads either. # -# Sourced from each person's profile (sectors, age groups, categories, addresses) -# and from the orgs/scholarships/CE tied to their attended-training registrations. +# Sourced from each person's profile (sectors, age groups, categories) and from the +# orgs, work addresses, scholarships, and CE tied to their attended-training registrations. class AttendeesBreakdowns # events: the events whose registrations may be counted (for the attendees index, # the viewer's reportable events narrowed by the active filters), so the @@ -140,13 +140,12 @@ def ce_registrant_ids # --- Cities ---------------------------------------------------------------- - # Attendees grouped by the city of the organization linked on their attended- - # training registrations, with scholarship recipients per city — the shared + # Attendees grouped by the city they work in on their attended-training + # registrations, with scholarship recipients per city — the shared # "Registrants by city" breakdown, aggregated across all trainings. def registrant_city_breakdown @registrant_city_breakdown ||= RegistrantCityBreakdown.new( - org_registrant_pairs: org_registrant_pairs, - city_by_org: city_by_organization, + city_registrant_pairs: work_addresses.rows.map { |row| [ row.city_label, row.registrant_id ] }, scholarship_recipient_ids: scholarship_recipient_ids ) end @@ -181,11 +180,11 @@ def life_experience_registrant_ids_by_category end def state_registrant_ids_by_state - @state_registrant_ids_by_state ||= person_ids_by(address_rows.select { |_, state, _| Address::US_STATE_ABBREVIATIONS.include?(state.to_s.upcase) }) { |row| row[1] } + work_addresses.registrant_ids_by_state end def country_registrant_ids_by_country - @country_registrant_ids_by_country ||= person_ids_by(address_rows.reject { |_, _, country| country.blank? }) { |row| row[2] } + work_addresses.registrant_ids_by_country end def school_district_registrant_ids_by_district @@ -290,19 +289,13 @@ def category_rows(category_type_name) .pluck(:categorizable_id, :category_id) end - # [ [ person_id, state, country ], ... ] — one load behind the state and country - # counts and their id maps. - def address_rows - @address_rows ||= active_addresses.pluck(:addressable_id, :state, :country) - end - # { key => distinct person ids }, grouping [ person_id, ... ] rows by the block. def person_ids_by(rows) rows.group_by { |row| yield(row) }.transform_values { |grouped| grouped.map(&:first).uniq } end - def active_addresses - Address.active.where(addressable_type: "Person", addressable_id: person_ids) + def work_addresses + @work_addresses ||= RegistrationWorkAddresses.new(registration_ids) end def org_registrant_pairs @@ -347,22 +340,6 @@ def program_status_by_organization end end - # "City, State" per linked organization, from its first active address (mirrors - # EventDashboard#city_by_organization). Orgs with no active address are absent, - # so their registrants fall into the breakdown's Unknown bucket. - def city_by_organization - org_ids = org_registrant_pairs.map(&:first).uniq - Address.active - .where(addressable_type: "Organization", addressable_id: org_ids) - .order(:id) - .pluck(:addressable_id, :city, :state) - .each_with_object({}) do |(org_id, city, state), map| - next if map.key?(org_id) - label = [ city, state ].compact_blank.join(", ").presence - map[org_id] = label if label - end - end - def scholarship_recipient_ids @scholarship_recipient_ids ||= Scholarship .not_declined diff --git a/app/services/event_dashboard.rb b/app/services/event_dashboard.rb index 7f78748c07..1dad5402cc 100644 --- a/app/services/event_dashboard.rb +++ b/app/services/event_dashboard.rb @@ -147,14 +147,12 @@ def scholarship_recipient_count recognized_scholarships.map(&:recipient_id).uniq.size end - # This event's registrants grouped by the city of the organization linked on - # their registration, with the scholarship-recipient count per city — the - # shared "Registrants by city" breakdown on the background and recipients - # pages. Fed already-plucked data so the rollup runs in memory. + # This event's registrants grouped by the city they work in, with the + # scholarship-recipient count per city — the shared "Registrants by city" + # breakdown on the background and recipients pages. def registrant_city_breakdown @registrant_city_breakdown ||= RegistrantCityBreakdown.new( - org_registrant_pairs: registration_org_registrant_pairs, - city_by_org: city_by_organization, + city_registrant_pairs: work_addresses.rows.map { |row| [ row.city_label, row.registrant_id ] }, scholarship_recipient_ids: scholarships.distinct.pluck(:recipient_id) ) end @@ -852,52 +850,46 @@ def referral_source_counts .sort_by { |label, count| [ -count, label ] } end - # US states only — international registrants' regions (e.g. provinces) are - # excluded so the States breakdown reflects domestic residents. + # The US states registrants work in (RegistrationWorkAddresses). def states - @states ||= us_state_addresses.distinct.pluck(:state).sort + @states ||= state_registrant_ids_by_state.keys.sort end - # Distinct US-resident registrant count per state. def state_counts - @state_counts ||= us_state_addresses.group(:state).distinct.count(:addressable_id) + @state_counts ||= state_registrant_ids_by_state.transform_values(&:size) end - # Registrant ids that have at least one active US address with a state on file — - # i.e. the people behind the states count. def state_registrant_ids - @state_registrant_ids ||= us_state_addresses.distinct.pluck(:addressable_id) + @state_registrant_ids ||= state_registrant_ids_by_state.values.flatten.uniq end - def countries - @countries ||= Address + def state_registrant_ids_by_state + work_addresses.registrant_ids_by_state + end + + # The US states on registrants' own addresses — the options of the roster's + # State filter, which matches on those addresses (EventRegistration.registrant_state). + def registrant_address_states + @registrant_address_states ||= Address .active .where(addressable_type: "Person", addressable_id: registrant_ids) - .where.not(country: [ nil, "" ]) + .where("UPPER(addresses.state) IN (?)", Address::US_STATE_ABBREVIATIONS) .distinct - .pluck(:country) + .pluck(:state) .sort end - # Distinct registrant count per country. + # The countries registrants work in (RegistrationWorkAddresses). + def countries + @countries ||= country_registrant_ids_by_country.keys.sort + end + def country_counts - @country_counts ||= Address - .active - .where(addressable_type: "Person", addressable_id: registrant_ids) - .where.not(country: [ nil, "" ]) - .group(:country) - .distinct - .count(:addressable_id) + @country_counts ||= country_registrant_ids_by_country.transform_values(&:size) end - # Registrant ids that have at least one active address with a country on file. def country_registrant_ids - @country_registrant_ids ||= Address - .active - .where(addressable_type: "Person", addressable_id: registrant_ids) - .where.not(country: [ nil, "" ]) - .distinct - .pluck(:addressable_id) + @country_registrant_ids ||= country_registrant_ids_by_country.values.flatten.uniq end # School districts across active registrants' affiliation organization addresses @@ -916,17 +908,8 @@ def school_district_registrant_ids @school_district_registrant_ids ||= affiliation_district_pairs.map(&:last).uniq end - # Registrant ids per country, keyed by country name — the people behind each - # country row, for drilling into the matching registrant list. def country_registrant_ids_by_country - @country_registrant_ids_by_country ||= Address - .active - .where(addressable_type: "Person", addressable_id: registrant_ids) - .where.not(country: [ nil, "" ]) - .distinct - .pluck(:country, :addressable_id) - .group_by(&:first) - .transform_values { |rows| rows.map(&:last).uniq } + work_addresses.registrant_ids_by_country end # Registrant ids per school district, for the breakdown's per-row drill-in. A @@ -985,13 +968,8 @@ def life_experience_registrant_ids_by_category private - # Active registrant addresses whose state is a recognized US state/territory — - # the source for every States figure (count card, choropleth, and drill-in). - def us_state_addresses - Address - .active - .where(addressable_type: "Person", addressable_id: registrant_ids) - .where("UPPER(addresses.state) IN (?)", Address::US_STATE_ABBREVIATIONS) + def work_addresses + @work_addresses ||= RegistrationWorkAddresses.new(active_registration_ids) end # [ [ district, registrant_id ], ... ] from the affiliations linked to this event's @@ -1339,32 +1317,6 @@ def self_funded_grant_ids @self_funded_grant_ids ||= Grant.self_funded_ids end - # [ [ organization_id, registrant_id ], ... ] from the organizations linked on - # each active registration (the registration-time snapshot only, not later - # affiliations) — the basis for grouping registrants by their org's city. - def registration_org_registrant_pairs - @registration_org_registrant_pairs ||= EventRegistrationOrganization - .joins(:event_registration) - .where(event_registration_id: active_registration_ids) - .pluck(:organization_id, "event_registrations.registrant_id") - end - - # "City, State" per linked organization, from its first active address (mirrors - # Organization#program_location). Orgs with no active address are absent, so - # their registrants fall into the breakdown's Unknown bucket. - def city_by_organization - org_ids = registration_org_registrant_pairs.map(&:first).uniq - Address.active - .where(addressable_type: "Organization", addressable_id: org_ids) - .order(:id) - .pluck(:addressable_id, :city, :state) - .each_with_object({}) do |(org_id, city, state), map| - next if map.key?(org_id) - label = [ city, state ].compact_blank.join(", ").presence - map[org_id] = label if label - end - end - def paid_registration_ids @paid_registration_ids ||= billable_registration_ids.select do |id| allocated_by_registration.fetch(id, 0) >= event.cost_cents.to_i diff --git a/app/services/registration_work_addresses.rb b/app/services/registration_work_addresses.rb new file mode 100644 index 0000000000..06b0f4828e --- /dev/null +++ b/app/services/registration_work_addresses.rb @@ -0,0 +1,71 @@ +# Where registrants work, per registration: each organization linked on the +# registration, at the address picked on the registration's affiliation with it +# (Affiliation#organization_address), else that org's oldest active address. The +# source for the registrant charts' cities, states, and countries. A linked org +# with no active address still yields a row, with no location, so the cities +# breakdown can count it as Unknown. +class RegistrationWorkAddresses + Row = Struct.new(:organization_id, :registrant_id, :city, :state, :country, keyword_init: true) do + def city_label + [ city, state ].compact_blank.join(", ").presence + end + end + + def initialize(registration_ids) + @registration_ids = registration_ids + end + + def rows + @rows ||= links.map do |registration_id, organization_id, registrant_id| + address = chosen_addresses[chosen_address_ids[[ registration_id, organization_id ]]] || + oldest_address_by_organization[organization_id] + Row.new(organization_id:, registrant_id:, city: address&.city, state: address&.state, country: address&.country) + end + end + + # US states only, so regions abroad stay off the States map. + def registrant_ids_by_state + @registrant_ids_by_state ||= registrant_ids_by(rows.select { |row| Address::US_STATE_ABBREVIATIONS.include?(row.state.to_s.upcase) }, &:state) + end + + def registrant_ids_by_country + @registrant_ids_by_country ||= registrant_ids_by(rows.select { |row| row.country.present? }, &:country) + end + + def registrant_ids_by_city_label + @registrant_ids_by_city_label ||= registrant_ids_by(rows.select(&:city_label), &:city_label) + end + + private + + def registrant_ids_by(rows, &key) + rows.group_by(&key).transform_values { |grouped| grouped.map(&:registrant_id).uniq } + end + + def links + @links ||= EventRegistrationOrganization + .joins(:event_registration) + .where(event_registration_id: @registration_ids) + .pluck(:event_registration_id, :organization_id, Arel.sql("event_registrations.registrant_id")) + end + + def chosen_address_ids + @chosen_address_ids ||= Affiliation + .where(event_registration_id: @registration_ids) + .where.not(organization_address_id: nil) + .pluck(:event_registration_id, :organization_id, :organization_address_id) + .to_h { |registration_id, organization_id, address_id| [ [ registration_id, organization_id ], address_id ] } + end + + def chosen_addresses + @chosen_addresses ||= Address.active.where(id: chosen_address_ids.values.uniq).select(:id, :city, :state, :country).index_by(&:id) + end + + def oldest_address_by_organization + @oldest_address_by_organization ||= Address.active + .where(addressable_type: "Organization", addressable_id: links.map(&:second).uniq) + .order(:id) + .select(:id, :addressable_id, :city, :state, :country) + .each_with_object({}) { |address, map| map[address.addressable_id] ||= address } + end +end diff --git a/app/views/events/_registrant_breakdowns.html.erb b/app/views/events/_registrant_breakdowns.html.erb index 934d8d1a21..88914e527a 100644 --- a/app/views/events/_registrant_breakdowns.html.erb +++ b/app/views/events/_registrant_breakdowns.html.erb @@ -61,7 +61,7 @@ <% sector_paths = data.sectors.to_h { |s| [ s.name, index_path.(sector: s.id) ] } %> <% age_group_paths = data.age_groups.to_h { |c| [ c.name, index_path.(age_group: c.id) ] } %> <% all_age_group_paths = data.all_age_groups.to_h { |c| [ c.name, index_path.(age_group: c.id) ] } %> - <% state_paths = data.state_counts.keys.index_with { |state| index_path.(state: state) } %> + <% state_paths = data.state_counts.keys.index_with { |state| index_path.(org_state: state) } %> <% country_paths = data.country_counts.keys.index_with { |country| index_path.(country: country) } %> <% school_district_paths = data.school_district_counts.keys.index_with { |district| index_path.(school_district: district) } %> <% life_experience_paths = data.life_experiences.to_h { |c| [ c.name, index_path.(life_experience: c.id) ] } %> @@ -77,7 +77,7 @@ <% sector_paths["Other"] = by_ids.(data.other_sector_response_registrant_ids) if data.other_sector_response_count.positive? %> <% age_group_paths = data.age_groups.to_h { |c| [ c.name, by_ids.(data.age_group_registrant_ids_by_category.fetch(c.id, [])) ] } %> <% all_age_group_paths = data.all_age_groups.to_h { |c| [ c.name, by_ids.(data.all_age_group_registrant_ids_by_category.fetch(c.id, [])) ] } %> - <% state_paths = data.states.index_with { |state| roster_event_path(event, state: state, charts: 1) } %> + <% state_paths = data.state_registrant_ids_by_state.transform_values { |ids| by_ids.(ids) } %> <% country_paths = data.country_registrant_ids_by_country.transform_values { |ids| by_ids.(ids) } %> <% school_district_paths = data.school_district_registrant_ids_by_district.transform_values { |ids| by_ids.(ids) } %> <% life_experience_paths = data.life_experiences.to_h { |c| [ c.name, by_ids.(data.life_experience_registrant_ids_by_category.fetch(c.id, [])) ] } %> diff --git a/app/views/events/_registrant_filters.html.erb b/app/views/events/_registrant_filters.html.erb index 7efb8e770d..ff074a2006 100644 --- a/app/views/events/_registrant_filters.html.erb +++ b/app/views/events/_registrant_filters.html.erb @@ -124,9 +124,9 @@ <%= render "shared/filter_text", param: :city, label: "City", placeholder: "City name", field_class: field_class %> - <% if @dashboard&.states&.any? %> + <% if @dashboard&.registrant_address_states&.any? %> <%= render "shared/filter_select", param: :state, label: "State", - options: @dashboard.states, selected: params[:state], blank: "All states", field_class: field_class, wrapper_class: "w-full md:w-32" %> + options: @dashboard.registrant_address_states, selected: params[:state], blank: "All states", field_class: field_class, wrapper_class: "w-full md:w-32" %> <% end %> <% if @dashboard&.counties&.any? %> diff --git a/db/seeds/features/2026-10-25-registrant-charts-use-work-locations.yml b/db/seeds/features/2026-10-25-registrant-charts-use-work-locations.yml new file mode 100644 index 0000000000..a950408a66 --- /dev/null +++ b/db/seeds/features/2026-10-25-registrant-charts-use-work-locations.yml @@ -0,0 +1,13 @@ +name: "Registrant charts show where people work" +area: reporting +display_status: admin_facing +released_on: 2026-10-25 +pr_number: 2842 +summary: >- + The cities, states, and countries charts on an event's roster, the + attendees page, and the scholarship recipients page now count each + registrant where they work: the address of the organization they + registered under. +pro_tips: + - "If the registrant picked one of the organization's addresses, that one counts. Otherwise the organization's first address on file counts." + - "The roster's State and County filters still match the registrant's own address." diff --git a/spec/presenters/registrant_city_breakdown_spec.rb b/spec/presenters/registrant_city_breakdown_spec.rb index 235b926373..4df4cc523e 100644 --- a/spec/presenters/registrant_city_breakdown_spec.rb +++ b/spec/presenters/registrant_city_breakdown_spec.rb @@ -4,8 +4,7 @@ describe "#rows" do it "counts distinct registrants and scholarship recipients per city, most first" do breakdown = described_class.new( - org_registrant_pairs: [ [ 1, 10 ], [ 1, 11 ], [ 2, 12 ] ], - city_by_org: { 1 => "Los Angeles, CA", 2 => "Richmond, CA" }, + city_registrant_pairs: [ [ "Los Angeles, CA", 10 ], [ "Los Angeles, CA", 11 ], [ "Richmond, CA", 12 ] ], scholarship_recipient_ids: [ 10 ] ) @@ -16,8 +15,7 @@ it "exposes per-city registrant and scholarship-recipient ids for drill-in links" do breakdown = described_class.new( - org_registrant_pairs: [ [ 1, 10 ], [ 1, 11 ], [ 2, 12 ] ], - city_by_org: { 1 => "Los Angeles, CA", 2 => "Richmond, CA" }, + city_registrant_pairs: [ [ "Los Angeles, CA", 10 ], [ "Los Angeles, CA", 11 ], [ "Richmond, CA", 12 ] ], scholarship_recipient_ids: [ 10, 12 ] ) @@ -26,20 +24,18 @@ expect(la.scholarship_recipient_ids).to contain_exactly(10) end - it "counts a registrant once per city even when linked to that org twice" do + it "counts a registrant once per city even when listed there twice" do breakdown = described_class.new( - org_registrant_pairs: [ [ 1, 10 ], [ 1, 10 ] ], - city_by_org: { 1 => "Barstow, CA" }, + city_registrant_pairs: [ [ "Barstow, CA", 10 ], [ "Barstow, CA", 10 ] ], scholarship_recipient_ids: [] ) expect(breakdown.rows.map(&:registrant_count)).to eq([ 1 ]) end - it "buckets linked orgs with no city under Unknown, sorted last" do + it "buckets a nil city under Unknown, sorted last" do breakdown = described_class.new( - org_registrant_pairs: [ [ 1, 10 ], [ 1, 11 ], [ 2, 12 ] ], - city_by_org: { 1 => "Perris, CA" }, + city_registrant_pairs: [ [ "Perris, CA", 10 ], [ "Perris, CA", 11 ], [ nil, 12 ] ], scholarship_recipient_ids: [] ) @@ -51,13 +47,12 @@ it "reports city_count and any?" do breakdown = described_class.new( - org_registrant_pairs: [ [ 1, 10 ], [ 2, 11 ] ], - city_by_org: { 1 => "Pomona, CA", 2 => "Tustin, CA" }, + city_registrant_pairs: [ [ "Pomona, CA", 10 ], [ "Tustin, CA", 11 ] ], scholarship_recipient_ids: [] ) expect(breakdown.city_count).to eq(2) expect(breakdown).to be_any - expect(described_class.new(org_registrant_pairs: [], city_by_org: {}, scholarship_recipient_ids: [])).not_to be_any + expect(described_class.new(city_registrant_pairs: [], scholarship_recipient_ids: [])).not_to be_any end end diff --git a/spec/requests/events/attendees_spec.rb b/spec/requests/events/attendees_spec.rb index 828aa4f56c..c2be1864b8 100644 --- a/spec/requests/events/attendees_spec.rb +++ b/spec/requests/events/attendees_spec.rb @@ -130,7 +130,7 @@ get attendees_events_url(country: "Canada", state: "OR") # The chip labels the applied filter and its ✕ links back to the index # with only that param dropped (the other filters are preserved). - expect(response.body).to include("Country: Canada") + expect(response.body).to include("Org country: Canada") expect(Capybara.string(response.body)) .to have_link(href: attendees_events_path(state: "OR")) end @@ -341,14 +341,22 @@ expect(response.body).to include("Online Search") end - it "filters by a breakdown drill-in (country)" do - create(:address, addressable: attendee, country: "Canada", inactive: false) + it "filters by a work country or org state drill-in (via the org they registered under)" do + org = create(:organization, name: "Toronto Org") + create(:address, addressable: org, state: "ON", country: "Canada", inactive: false) + attendee_registration.event_registration_organizations.create!(organization: org) other = create(:person, first_name: "Zed", last_name: "Zulu") + create(:address, addressable: other, state: "OR", country: "Canada", inactive: false) create(:event_registration, event: recent_training, registrant: other, status: "attended") get attendees_events_url(country: "Canada"), headers: frame_headers expect(response.body).to include("Ada Lovelace") expect(response.body).not_to include("Zed Zulu") + + org.addresses.first.update!(state: "WA", country: "United States") + get attendees_events_url(org_state: "WA"), headers: frame_headers + expect(response.body).to include("Ada Lovelace") + expect(response.body).not_to include("Zed Zulu") end it "filters by a school-district drill-in (via the org they registered under)" do @@ -656,7 +664,7 @@ .at_css("turbo-stream[action='replace'][target='attendees_chips'] template") expect(template).to be_present - remove_link = "a[aria-label='Remove Country: Canada filter']" + remove_link = "a[aria-label='Remove Org country: Canada filter']" expect(Capybara.string(template.inner_html)).to have_selector( "#{remove_link}[href='#{attendees_events_path(ce_status: "issued")}']", visible: :all ) diff --git a/spec/requests/events_spec.rb b/spec/requests/events_spec.rb index a769b15bde..6c35660d68 100644 --- a/spec/requests/events_spec.rb +++ b/spec/requests/events_spec.rb @@ -3371,8 +3371,9 @@ def ce_chip_text expect(response.body).to include("Hospice care") end - it "shows a countries breakdown from registrant addresses" do - create(:address, addressable: person, state: "CA", country: "Canada", inactive: false) + it "shows a countries breakdown from the org a registrant registered under" do + create(:address, addressable: organization, state: "ON", country: "Canada", inactive: false) + registration.event_registration_organizations.create!(organization: organization) get roster_event_path(owned_event), headers: charts_headers @@ -3391,7 +3392,8 @@ def ce_chip_text end it "renders the states breakdown as a US choropleth map fed by per-state counts" do - create(:address, addressable: person, state: "CA", inactive: false) + create(:address, addressable: organization, state: "CA", inactive: false) + registration.event_registration_organizations.create!(organization: organization) get roster_event_path(owned_event), headers: charts_headers diff --git a/spec/services/attendees_active_filters_spec.rb b/spec/services/attendees_active_filters_spec.rb index 1b267d5e2e..9ea7f18936 100644 --- a/spec/services/attendees_active_filters_spec.rb +++ b/spec/services/attendees_active_filters_spec.rb @@ -29,7 +29,8 @@ def chips_for(params) end it "labels the literal-value drill-ins" do - expect(chips_for(country: "Canada").first[:label]).to eq("Country: Canada") + expect(chips_for(country: "Canada").first[:label]).to eq("Org country: Canada") + expect(chips_for(org_state: "CA").first[:label]).to eq("Org state: CA") expect(chips_for(school_district: "PPS").first[:label]).to eq("School district: PPS") expect(chips_for(org_city: "Austin, TX").first[:label]).to eq("Org city: Austin, TX") end diff --git a/spec/services/attendees_breakdowns_spec.rb b/spec/services/attendees_breakdowns_spec.rb index 7888c260f9..e5f1719657 100644 --- a/spec/services/attendees_breakdowns_spec.rb +++ b/spec/services/attendees_breakdowns_spec.rb @@ -19,9 +19,14 @@ expect(breakdowns.sector_counts).to eq(sector.id => 1) end - it "counts US states from active addresses" do - create(:address, addressable: person, state: "CA", inactive: false) + it "counts US states and countries from the work address of the org they registered under" do + create(:address, addressable: person, state: "NY", country: "Canada") + organization = create(:organization) + create(:address, addressable: organization, state: "CA", country: "United States") + registration.event_registration_organizations.create!(organization: organization) + expect(breakdowns.state_counts).to eq("CA" => 1) + expect(breakdowns.country_counts).to eq("United States" => 1) end it "aggregates organizations linked on the person's training registrations" do @@ -137,8 +142,8 @@ def answer(who, event, value) it "returns the people behind each dimension" do sector = create(:sector, name: "Healthcare") create(:sectorable_item, sectorable: person, sector: sector, is_primary: true) - create(:address, addressable: person, state: "CA", inactive: false) organization = create(:organization, name: "Wellness Org") + create(:address, addressable: organization, state: "CA", inactive: false) registration.event_registration_organizations.create!(organization: organization) scholarship = create(:scholarship, recipient: person, amount_cents: 1_000) create(:allocation, source: scholarship, allocatable: registration, amount: 1_000) diff --git a/spec/services/event_dashboard_spec.rb b/spec/services/event_dashboard_spec.rb index 9d98f445f2..bb299e3ead 100644 --- a/spec/services/event_dashboard_spec.rb +++ b/spec/services/event_dashboard_spec.rb @@ -438,37 +438,42 @@ end end - describe "states" do - it "returns unique states from active registrants' active addresses" do - expect(dashboard.states).to eq(%w[CA NY]) + describe "work locations" do + before do + reg1.event_registration_organizations.create!(organization: org_a) + reg2.event_registration_organizations.create!(organization: org_c) + cancelled_reg = event.event_registrations.find_by(registrant: cancelled_person) + cancelled_reg.event_registration_organizations.create!(organization: org_excluded) + create(:address, addressable: org_a, city: "Oakland", state: "OR", country: "United States") + chosen = create(:address, addressable: org_a, city: "Fresno", state: "CA", country: "United States") + person1.affiliations.find_by(organization: org_a).update!(organization_address: chosen, event_registration: reg1) + create(:address, addressable: org_c, city: "Albany", state: "NY", country: "Mexico") + create(:address, addressable: org_excluded, city: "Miami", state: "FL", country: "Canada") end - it "counts distinct registrants per state" do + it "reads states from the affiliation's chosen org address, else the org's oldest" do + expect(dashboard.states).to eq(%w[CA NY]) expect(dashboard.state_counts).to eq("CA" => 1, "NY" => 1) + expect(dashboard.state_registrant_ids_by_state).to eq("CA" => [ person1.id ], "NY" => [ person2.id ]) end - it "returns the registrant ids that have a state on file" do - expect(dashboard.state_registrant_ids).to contain_exactly(person1.id, person2.id) - end - end - - describe "counties" do - it "returns unique [ state, county ] pairs from active registrants' active addresses" do - expect(dashboard.counties).to eq([ [ "CA", "Los Angeles" ], [ "NY", "Kings" ] ]) + it "reads countries from the same work addresses, excluding cancelled registrations" do + expect(dashboard.country_counts).to eq("United States" => 1, "Mexico" => 1) + expect(dashboard.country_registrant_ids).to contain_exactly(person1.id, person2.id) end - end - describe "countries" do - it "returns the registrant ids that have a country on file, excluding inactive and cancelled" do - expect(dashboard.country_registrant_ids).to contain_exactly(person1.id, person2.id) + it "groups registrants by work city" do + expect(dashboard.registrant_city_breakdown.rows.map(&:city)).to contain_exactly("Fresno, CA", "Albany, NY") end - it "counts only active registrants' active addresses" do - expect(dashboard.country_counts.values.sum).to eq(2) + it "offers the State filter the registrants' own address states" do + expect(dashboard.registrant_address_states).to eq(%w[CA NY]) end + end - it "maps each country to its registrant ids" do - expect(dashboard.country_registrant_ids_by_country.values.flatten).to contain_exactly(person1.id, person2.id) + describe "counties" do + it "returns unique [ state, county ] pairs from active registrants' active addresses" do + expect(dashboard.counties).to eq([ [ "CA", "Los Angeles" ], [ "NY", "Kings" ] ]) end end diff --git a/spec/services/registration_work_addresses_spec.rb b/spec/services/registration_work_addresses_spec.rb new file mode 100644 index 0000000000..ae170ac343 --- /dev/null +++ b/spec/services/registration_work_addresses_spec.rb @@ -0,0 +1,45 @@ +require "rails_helper" + +RSpec.describe RegistrationWorkAddresses do + let(:event) { create(:event) } + let(:person) { create(:person) } + let(:registration) { create(:event_registration, event: event, registrant: person) } + let(:organization) { create(:organization) } + + subject(:work_addresses) { described_class.new([ registration.id ]) } + + before { registration.event_registration_organizations.create!(organization: organization) } + + it "uses the address picked on the registration's affiliation" do + create(:address, addressable: organization, city: "Portland", state: "OR", country: "United States") + chosen = create(:address, addressable: organization, city: "Fresno", state: "CA", country: "United States") + create(:affiliation, person: person, organization: organization, organization_address: chosen, event_registration: registration) + + expect(work_addresses.rows.map(&:city_label)).to eq([ "Fresno, CA" ]) + end + + it "falls back to the org's oldest active address" do + create(:address, addressable: organization, city: "Old", state: "NV", inactive: true) + create(:address, addressable: organization, city: "Reno", state: "NV") + create(:address, addressable: organization, city: "Elko", state: "NV") + + expect(work_addresses.rows.map(&:city_label)).to eq([ "Reno, NV" ]) + end + + it "skips the registrant's own address" do + create(:address, addressable: person, city: "Boise", state: "ID") + + expect(work_addresses.rows.map(&:city_label)).to eq([ nil ]) + end + + it "groups registrant ids by US state, country, and city" do + create(:address, addressable: organization, city: "Austin", state: "TX", country: "United States") + abroad = create(:organization) + create(:address, addressable: abroad, city: "Toronto", state: "ON", country: "Canada") + registration.event_registration_organizations.create!(organization: abroad) + + expect(work_addresses.registrant_ids_by_state).to eq("TX" => [ person.id ]) + expect(work_addresses.registrant_ids_by_country).to eq("United States" => [ person.id ], "Canada" => [ person.id ]) + expect(work_addresses.registrant_ids_by_city_label.keys).to contain_exactly("Austin, TX", "Toronto, ON") + end +end From 6eff3e89c0650dd478be8a389515b1f106486eb4 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 07:36:07 -0400 Subject: [PATCH 4/6] Filter registrants by where they work The location filters should mean the same thing as the location charts. The roster, bulk-email picker, and attendees State/County/City filters now match the registration's work address (RegistrationWorkAddresses), so the attendees States drill-in can reuse the state param. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 +- app/controllers/events_controller.rb | 44 ++++++------------- app/helpers/events_helper.rb | 2 +- app/models/event_registration.rb | 23 ++++------ app/services/attendees_active_filters.rb | 3 +- app/services/event_dashboard.rb | 27 ++---------- app/services/registration_work_addresses.rb | 35 ++++++++++++--- app/services/reminder_recipient_filter.rb | 16 ++++--- .../events/_registrant_breakdowns.html.erb | 2 +- app/views/events/_registrant_filters.html.erb | 4 +- ...5-registrant-charts-use-work-locations.yml | 10 ++--- spec/models/event_registration_spec.rb | 37 +++++++++++----- spec/requests/events/attendees_spec.rb | 26 ++++++----- spec/requests/events/bulk_reminders_spec.rb | 7 ++- spec/requests/events_spec.rb | 17 ++++--- .../services/attendees_active_filters_spec.rb | 1 - spec/services/event_dashboard_spec.rb | 14 ++---- .../reminder_recipient_filter_spec.rb | 14 +++--- 18 files changed, 139 insertions(+), 145 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 1b215dbc0b..e6ba3b04e6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -236,7 +236,7 @@ comparison — add a rule if none fits (e.g. `PersonPolicy#own_profile?`). Conve - `EventProgramStatusReport` — Cross-event program-status report grouped by calendar year: how many organizations were New / Ongoing / Reinstate at each facilitator training, counted both as organization-trainings (summed rows) and as distinct organizations classified at their earliest training. Sibling of `EventRevenueReport`/`EventParticipationReport`/`EventScholarshipReport` (includes `ReportPeriods`); powers the `events#program_statuses` page and the reports-hub program-status card - `EventParticipationReport` — Cross-event participation report grouped by calendar year (unique people trained vs attended seats vs per-status outcome counts, chart series) for the events participation page; sibling of `EventRevenueReport` - `AttendeesRoster` — Cross-event counterpart to `EventDashboard`: builds the per-registrant lookup maps the shared `events/_registrant_roster` partial reads (sector/age/org/status/location/scholarship/CE plus the events-attended column) for a paginated page of people; backs the `events#attendees` index. Takes `events:` + `registrations:` — the index's current filter scopes, already narrowed by `EventPolicy`'s `:reportable` scope — so a person's columns show what's in scope rather than their whole history, and never an event the viewer can't see -- `RegistrationWorkAddresses` — Where registrants work, per registration: each org linked on the registration (`EventRegistrationOrganization`), at the address picked on the registration's affiliation with it (`Affiliation#organization_address`), else the org's oldest active address. Groups registrant ids by US state, country, and "City, State". The single source for the registrant charts' cities, states, and countries (`EventDashboard`, `AttendeesBreakdowns`) and the attendees index's `country`/`org_state`/`org_city` drill-ins; the roster's State filter still reads registrants' own addresses (`EventDashboard#registrant_address_states`) +- `RegistrationWorkAddresses` — Where registrants work, per registration: each org linked on the registration (`EventRegistrationOrganization`), at the address picked on the registration's affiliation with it (`Affiliation#organization_address`), else the org's oldest active address. Groups registrant ids by US state, country, and "City, State"; `#matching(state:, county:, city:)` tests one address per row. The single source for every registrant location figure and filter: the charts' cities, states, and countries (`EventDashboard`, `AttendeesBreakdowns`), the roster/reminder State, County, and City filters (`EventRegistration.work_state`/`work_county`/`work_city`, `ReminderRecipientFilter`), and the attendees index's State/County/City filters and `country`/`org_city` drill-ins - `AttendeesBreakdowns` — Aggregate counterpart to `EventDashboard`'s breakdown methods: computes the chart datasets (sectors, age groups, locations, program status, life experiences, settings, organizations, scholarship/CE) over an arbitrary people set — tags from profiles, cities/states/countries from `RegistrationWorkAddresses` — for the shared `events/_registrant_breakdowns` partial. Backs the `events#attendees` index charts (cross-event; `events:` / `registrations:` = the index's current filter scopes, already narrowed by `EventPolicy`'s `:reportable` scope) and the `events#recipients` charts frame (one event's scholarship recipients, `registrations:` = `EventRegistration.active` so it counts them regardless of attendance, `as_of:` = that event's start date so program status reads the same verdict its dashboard does). Also exposes `*_registrant_ids_by_*` maps mirroring `EventDashboard`'s, so a breakdown row can drill in by person id — they regroup rows already loaded for the counts, adding no queries - `AttendeesActiveFilters` — Human-readable, removable-chip descriptors for a page's "drill-in" filters (registrant_ids, organization, org city, age group / life experience / setting categories, country, school district, scholarship, CE, payment status, scholarship funding source, sector, state) — params that narrow a list without a field in a visible filter form. `CHIP_PARAMS` is the attendees index's set (omitting anything with its own control); pass `chip_params:` for a page with no filter form at all — `ROSTER_CHIP_PARAMS` for the per-event roster, `%w[ registrant_ids ]` for scholarship recipients - `ReportPeriods` — Shared module (included by `EventRevenueReport` and `EventParticipationReport`) resolving the reporting-hub period toggle (this year / last year / all time) to a metric scope + label for the summary cards diff --git a/app/controllers/events_controller.rb b/app/controllers/events_controller.rb index 772175bf2b..8e0caa471e 100644 --- a/app/controllers/events_controller.rb +++ b/app/controllers/events_controller.rb @@ -244,12 +244,12 @@ def registrants scope = scope.comment_text(params[:comment]) if params[:comment].present? scope = scope.funder_name(params[:funder_name]) if params[:funder_name].present? scope = scope.submission_status(params[:submission_status], @event) if params[:submission_status].present? - scope = scope.registrant_city(params[:city]) if params[:city].present? + scope = scope.work_city(params[:city]) if params[:city].present? scope = scope.organization_linking_status(params[:org_status], @event) if params[:org_status].present? scope = scope.account_status(params[:account_status]) if params[:account_status].present? scope = scope.registrant_ids(params[:registrant_ids]) if params[:registrant_ids].present? - scope = scope.registrant_state(params[:state]) if params[:state].present? - scope = scope.registrant_county(params[:county]) if params[:county].present? + scope = scope.work_state(params[:state]) if params[:state].present? + scope = scope.work_county(params[:county]) if params[:county].present? scope = scope.registrant_sector(params[:sector]) if params[:sector].present? scope = scope.registered_between(params[:registered_from], params[:registered_to]) @@ -854,7 +854,6 @@ def filtered_attendees scope = scope.where(id: person_category_ids(params[:age_group])) if params[:age_group].present? scope = scope.where(id: person_category_ids(params[:life_experience])) if params[:life_experience].present? scope = scope.where(id: attendee_work_addresses.registrant_ids_by_country.fetch(params[:country], [])) if params[:country].present? - scope = scope.where(id: attendee_work_addresses.registrant_ids_by_state.fetch(params[:org_state], [])) if params[:org_state].present? scope = scope.where(id: person_school_district_ids(params[:school_district])) if params[:school_district].present? scope = scope.where(id: person_program_status_ids(params[:program_status])) if params[:program_status].present? scope = scope.where(id: person_linked_organization_ids(params[:organization_id])) if params[:organization_id].present? @@ -870,19 +869,17 @@ def filtered_attendees scope.where(id: scholarship_status_person_ids(params[:scholarship])) end end - # Address filters read the person's own address data directly (no registration - # required) — city is a free-text match, state/county exact. City and state ask - # one subquery so they describe the same address, matching the roster's shared - # join; two subqueries would let someone with homes in two states through on a - # city from one and a state from the other. + # Address filters match where people work on their in-scope registrations. City + # and state test one address, so someone working in two states can't match on a + # city from one and the state from the other. if params[:state].present? || params[:city].present? - scope = scope.where(id: person_address_ids(state: params[:state], city: params[:city])) + scope = scope.where(id: attendee_work_addresses.matching(state: params[:state], city: params[:city]).map(&:registrant_id)) end if params[:county].present? # County options carry their state ("STATE::County") so same-named counties # across states don't collide. county_state, county_name = params[:county].split("::", 2) - scope = scope.where(id: person_address_ids(state: county_state, county: county_name)) + scope = scope.where(id: attendee_work_addresses.matching(state: county_state, county: county_name).map(&:registrant_id)) end # Shared registration-level filters, applied cross-event with "any registration @@ -969,8 +966,8 @@ def person_linked_organization_ids(organization_id) .select(Arel.sql("event_registrations.registrant_id")) end - # Where each in-scope registration's registrant works, behind the country, org - # state, and org city drill-ins — the same source as the breakdowns they come from. + # Where each in-scope registration's registrant works, behind the location filters + # and drill-ins — the same source as the breakdowns they come from. def attendee_work_addresses @attendee_work_addresses ||= RegistrationWorkAddresses.new(attendee_registrations.pluck(:id)) end @@ -1010,21 +1007,6 @@ def person_affiliation_status_ids(status) Affiliation.with_status(status).select(:person_id) end - # Person ids behind active person addresses, narrowed by any of state, county or - # city. City is a free-text LIKE (matching the roster's registrant_city filter), - # state/county are exact. Street isn't a filter anywhere yet, but the column - # exists — add a `street:` clause here the same way if one is ever needed. - def person_address_ids(state: nil, county: nil, city: nil) - scope = Address.active.where(addressable_type: "Person") - scope = scope.where(state: state) if state.present? - scope = scope.where(county: county) if county.present? - if city.present? - like = "%#{Address.sanitize_sql_like(city.downcase.strip)}%" - scope = scope.where("LOWER(addresses.city) LIKE ?", like) - end - scope.select(:addressable_id) - end - # Option lists for the attendees index filter selects, built once per full page # load from the viewer's WHOLE reportable population — deliberately un-narrowed by # the active filters, so filtering never removes the option that would undo it. @@ -1033,9 +1015,9 @@ def set_attendee_filter_options @attendee_years = @attendee_event_options.filter_map { |event| event.start_date&.year }.uniq.sort.reverse person_ids = Person.where(id: EventRegistration.where(event_id: reportable_events.select(:id)).select(:registrant_id)) @attendee_sectors = Sector.where(id: SectorableItem.where(sectorable_type: "Person", sectorable_id: person_ids).select(:sector_id)).order(:name) - addresses = Address.active.where(addressable_type: "Person", addressable_id: person_ids) - @attendee_states = addresses.where.not(state: [ nil, "" ]).distinct.pluck(:state).sort - @attendee_counties = addresses.where.not(county: [ nil, "" ]).where.not(state: [ nil, "" ]).distinct.pluck(:state, :county).sort + work_addresses = RegistrationWorkAddresses.new(EventRegistration.where(event_id: reportable_events.select(:id)).pluck(:id)) + @attendee_states = work_addresses.rows.filter_map(&:state).compact_blank.uniq.sort + @attendee_counties = work_addresses.counties @attendee_topic_types = TopicSubscriptionType.active.ordered end diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb index 332ef35fb8..479880bfd7 100644 --- a/app/helpers/events_helper.rb +++ b/app/helpers/events_helper.rb @@ -239,7 +239,7 @@ def time_period_to_event_year(time_period) REPORT_SUBNAV_PARAMS = %i[ event_id event_type search funder_sgid time_period view registrant_ids attendance_status payment_status funder scholarship - sector contact_info affiliation_status organization_id org_city org_state state county + sector contact_info affiliation_status organization_id org_city state county age_group life_experience country school_district program_status ].freeze diff --git a/app/models/event_registration.rb b/app/models/event_registration.rb index e12b2e9cf1..0240a3a7d0 100644 --- a/app/models/event_registration.rb +++ b/app/models/event_registration.rb @@ -220,27 +220,20 @@ class EventRegistration < ApplicationRecord end scope } - scope :registrant_state, ->(state) { - joins(registrant: :addresses) - .where(addresses: { inactive: false, state: state }) - .distinct + # Where the registrant works on the registration (RegistrationWorkAddresses). + scope :work_state, ->(state) { + where(id: RegistrationWorkAddresses.new(pluck(:id)).matching(state:).map(&:registration_id)) } # Accepts "STATE|County" (from the manage filter) to scope to a county within # a specific state, or a bare county name for backward compatibility. - scope :registrant_county, ->(value) { + scope :work_county, ->(value) { state, county = value.to_s.include?("|") ? value.split("|", 2) : [ nil, value.to_s ] next none if county.blank? - conditions = { inactive: false, county: county } - conditions[:state] = state if state.present? - joins(registrant: :addresses).where(addresses: conditions).distinct + where(id: RegistrationWorkAddresses.new(pluck(:id)).matching(state:, county:).map(&:registration_id)) } - scope :registrant_city, ->(term) { - return all if term.blank? - like = "%#{sanitize_sql_like(term.downcase.strip)}%" - joins(registrant: :addresses) - .where("LOWER(addresses.city) LIKE ?", like) - .where(addresses: { inactive: false }) - .distinct + scope :work_city, ->(term) { + next all if term.blank? + where(id: RegistrationWorkAddresses.new(pluck(:id)).matching(city: term).map(&:registration_id)) } scope :registrant_sector, ->(sector_id) { joins(registrant: :sectorable_items) diff --git a/app/services/attendees_active_filters.rb b/app/services/attendees_active_filters.rb index 0db76e1d71..a2a836629e 100644 --- a/app/services/attendees_active_filters.rb +++ b/app/services/attendees_active_filters.rb @@ -15,7 +15,7 @@ class AttendeesActiveFilters # honours but this list omits narrows the page silently AND is dropped by the # filter form's hidden fields on the next change, since both read this list. CHIP_PARAMS = %w[ - registrant_ids organization_id org_city org_state age_group life_experience + registrant_ids organization_id org_city age_group life_experience country school_district payment_status funder ].freeze # The roster has no filter form at all, so every drill-in it accepts needs a @@ -47,7 +47,6 @@ def label_for(param, value) when "sector" then record_label("Sector", Sector, value) when "organization_id" then record_label("Organization", Organization, value) when "org_city" then "Org city: #{value}" - when "org_state" then "Org state: #{value}" when "age_group" then record_label("Age group", Category, value) when "life_experience" then record_label("Life experience", Category, value) when "country" then "Org country: #{value}" diff --git a/app/services/event_dashboard.rb b/app/services/event_dashboard.rb index 1dad5402cc..3434bb0760 100644 --- a/app/services/event_dashboard.rb +++ b/app/services/event_dashboard.rb @@ -867,18 +867,6 @@ def state_registrant_ids_by_state work_addresses.registrant_ids_by_state end - # The US states on registrants' own addresses — the options of the roster's - # State filter, which matches on those addresses (EventRegistration.registrant_state). - def registrant_address_states - @registrant_address_states ||= Address - .active - .where(addressable_type: "Person", addressable_id: registrant_ids) - .where("UPPER(addresses.state) IN (?)", Address::US_STATE_ABBREVIATIONS) - .distinct - .pluck(:state) - .sort - end - # The countries registrants work in (RegistrationWorkAddresses). def countries @countries ||= country_registrant_ids_by_country.keys.sort @@ -922,19 +910,10 @@ def school_district_registrant_ids_by_district .transform_values(&:to_a) end - # Distinct [ state, county ] pairs across active registrants' active addresses, - # sorted by state then county. Pairing in the state lets the manage filter - # disambiguate same-named counties across states (e.g. "CA - Warren" vs - # "NY - Warren"). + # [ state, county ] pairs registrants work in. Pairing in the state lets the + # manage filter disambiguate same-named counties ("CA - Warren" vs "NY - Warren"). def counties - @counties ||= Address - .active - .where(addressable_type: "Person", addressable_id: registrant_ids) - .where.not(county: [ nil, "" ]) - .where.not(state: [ nil, "" ]) - .distinct - .pluck(:state, :county) - .sort + work_addresses.counties end # Life experiences captured at registration: StoryPopulation categories tagged diff --git a/app/services/registration_work_addresses.rb b/app/services/registration_work_addresses.rb index 06b0f4828e..bc65ec1f9a 100644 --- a/app/services/registration_work_addresses.rb +++ b/app/services/registration_work_addresses.rb @@ -1,11 +1,11 @@ # Where registrants work, per registration: each organization linked on the # registration, at the address picked on the registration's affiliation with it # (Affiliation#organization_address), else that org's oldest active address. The -# source for the registrant charts' cities, states, and countries. A linked org -# with no active address still yields a row, with no location, so the cities -# breakdown can count it as Unknown. +# source for the registrant charts' and filters' cities, counties, states, and +# countries. A linked org with no active address still yields a row, with no +# location, so the cities breakdown can count it as Unknown. class RegistrationWorkAddresses - Row = Struct.new(:organization_id, :registrant_id, :city, :state, :country, keyword_init: true) do + Row = Struct.new(:registration_id, :organization_id, :registrant_id, :city, :county, :state, :country, keyword_init: true) do def city_label [ city, state ].compact_blank.join(", ").presence end @@ -19,10 +19,31 @@ def rows @rows ||= links.map do |registration_id, organization_id, registrant_id| address = chosen_addresses[chosen_address_ids[[ registration_id, organization_id ]]] || oldest_address_by_organization[organization_id] - Row.new(organization_id:, registrant_id:, city: address&.city, state: address&.state, country: address&.country) + Row.new(registration_id:, organization_id:, registrant_id:, + city: address&.city, county: address&.county, state: address&.state, country: address&.country) end end + # Rows whose one address matches every given part: state and county exactly, + # city as a case-insensitive substring. + def matching(state: nil, county: nil, city: nil) + term = city.to_s.downcase.strip.presence + rows.select do |row| + (state.blank? || row.state == state) && + (county.blank? || row.county == county) && + (term.nil? || row.city.to_s.downcase.include?(term)) + end + end + + def rows_by_registration + @rows_by_registration ||= rows.group_by(&:registration_id) + end + + # Distinct [ state, county ] pairs, sorted, for the County filters. + def counties + @counties ||= rows.filter_map { |row| [ row.state, row.county ] if row.state.present? && row.county.present? }.uniq.sort + end + # US states only, so regions abroad stay off the States map. def registrant_ids_by_state @registrant_ids_by_state ||= registrant_ids_by(rows.select { |row| Address::US_STATE_ABBREVIATIONS.include?(row.state.to_s.upcase) }, &:state) @@ -58,14 +79,14 @@ def chosen_address_ids end def chosen_addresses - @chosen_addresses ||= Address.active.where(id: chosen_address_ids.values.uniq).select(:id, :city, :state, :country).index_by(&:id) + @chosen_addresses ||= Address.active.where(id: chosen_address_ids.values.uniq).select(:id, :city, :county, :state, :country).index_by(&:id) end def oldest_address_by_organization @oldest_address_by_organization ||= Address.active .where(addressable_type: "Organization", addressable_id: links.map(&:second).uniq) .order(:id) - .select(:id, :addressable_id, :city, :state, :country) + .select(:id, :addressable_id, :city, :county, :state, :country) .each_with_object({}) { |address, map| map[address.addressable_id] ||= address } end end diff --git a/app/services/reminder_recipient_filter.rb b/app/services/reminder_recipient_filter.rb index 495964d81a..b4282b1b4e 100644 --- a/app/services/reminder_recipient_filter.rb +++ b/app/services/reminder_recipient_filter.rb @@ -79,8 +79,8 @@ def dropdown_matched_ids scope = scope.organization_linking_status(@params[:org_status], @event) if @params[:org_status].present? scope = scope.account_status(@params[:account_status]) if @params[:account_status].present? scope = scope.submission_status(@params[:submission_status], @event) if @params[:submission_status].present? - scope = scope.registrant_state(@params[:state]) if @params[:state].present? - scope = scope.registrant_county(@params[:county]) if @params[:county].present? + scope = scope.work_state(@params[:state]) if @params[:state].present? + scope = scope.work_county(@params[:county]) if @params[:county].present? scope.pluck(:id).to_set end @@ -127,16 +127,18 @@ def matches_comment?(reg) end end - # Skips inactive addresses, matching EventRegistration.registrant_city so the - # same City value narrows the roster and the picker identically. + # Matches the work city, like EventRegistration.work_city, so the same City + # value narrows the roster and the picker identically. def matches_city?(reg) any_term?(:city) do |term| - reg.registrant.addresses.any? do |address| - !address.inactive? && address.city.to_s.downcase.include?(term) - end + work_addresses.rows_by_registration.fetch(reg.id, []).any? { |row| row.city.to_s.downcase.include?(term) } end end + def work_addresses + @work_addresses ||= RegistrationWorkAddresses.new(@event_registrations.map(&:id)) + end + # Matches against every email we hold for the registrant (their account email, # contact email and secondary email), so a search hits whichever address the # admin remembers. diff --git a/app/views/events/_registrant_breakdowns.html.erb b/app/views/events/_registrant_breakdowns.html.erb index 88914e527a..186206752a 100644 --- a/app/views/events/_registrant_breakdowns.html.erb +++ b/app/views/events/_registrant_breakdowns.html.erb @@ -61,7 +61,7 @@ <% sector_paths = data.sectors.to_h { |s| [ s.name, index_path.(sector: s.id) ] } %> <% age_group_paths = data.age_groups.to_h { |c| [ c.name, index_path.(age_group: c.id) ] } %> <% all_age_group_paths = data.all_age_groups.to_h { |c| [ c.name, index_path.(age_group: c.id) ] } %> - <% state_paths = data.state_counts.keys.index_with { |state| index_path.(org_state: state) } %> + <% state_paths = data.state_counts.keys.index_with { |state| index_path.(state: state) } %> <% country_paths = data.country_counts.keys.index_with { |country| index_path.(country: country) } %> <% school_district_paths = data.school_district_counts.keys.index_with { |district| index_path.(school_district: district) } %> <% life_experience_paths = data.life_experiences.to_h { |c| [ c.name, index_path.(life_experience: c.id) ] } %> diff --git a/app/views/events/_registrant_filters.html.erb b/app/views/events/_registrant_filters.html.erb index ff074a2006..7efb8e770d 100644 --- a/app/views/events/_registrant_filters.html.erb +++ b/app/views/events/_registrant_filters.html.erb @@ -124,9 +124,9 @@ <%= render "shared/filter_text", param: :city, label: "City", placeholder: "City name", field_class: field_class %> - <% if @dashboard&.registrant_address_states&.any? %> + <% if @dashboard&.states&.any? %> <%= render "shared/filter_select", param: :state, label: "State", - options: @dashboard.registrant_address_states, selected: params[:state], blank: "All states", field_class: field_class, wrapper_class: "w-full md:w-32" %> + options: @dashboard.states, selected: params[:state], blank: "All states", field_class: field_class, wrapper_class: "w-full md:w-32" %> <% end %> <% if @dashboard&.counties&.any? %> diff --git a/db/seeds/features/2026-10-25-registrant-charts-use-work-locations.yml b/db/seeds/features/2026-10-25-registrant-charts-use-work-locations.yml index a950408a66..a6864fb1b0 100644 --- a/db/seeds/features/2026-10-25-registrant-charts-use-work-locations.yml +++ b/db/seeds/features/2026-10-25-registrant-charts-use-work-locations.yml @@ -1,13 +1,13 @@ -name: "Registrant charts show where people work" +name: "Registrant charts and location filters use where people work" area: reporting display_status: admin_facing released_on: 2026-10-25 pr_number: 2842 summary: >- - The cities, states, and countries charts on an event's roster, the - attendees page, and the scholarship recipients page now count each - registrant where they work: the address of the organization they + The cities, states, and countries charts, and the City, State, and County + filters on the roster, the attendees page, and the bulk email picker, now + use where each registrant works: the address of the organization they registered under. pro_tips: - "If the registrant picked one of the organization's addresses, that one counts. Otherwise the organization's first address on file counts." - - "The roster's State and County filters still match the registrant's own address." + - "A registrant with no organization linked on their registration has no work location, so the location filters leave them out." diff --git a/spec/models/event_registration_spec.rb b/spec/models/event_registration_spec.rb index c1133cfdc0..811bcdf6cb 100644 --- a/spec/models/event_registration_spec.rb +++ b/spec/models/event_registration_spec.rb @@ -600,18 +600,31 @@ def registration_with_scholarship end end - describe ".registrant_city" do - it "matches registrants with an address in the city (case-insensitive, partial)" do - in_city = create(:person) - create(:address, addressable: in_city, city: "Santa Monica") - out_city = create(:person) - create(:address, addressable: out_city, city: "Portland") - reg_in = create(:event_registration, registrant: in_city) - reg_out = create(:event_registration, registrant: out_city) - - results = EventRegistration.registrant_city("santa") - expect(results).to include(reg_in) - expect(results).not_to include(reg_out) + describe "work location scopes" do + def registration_working_in(city:, state:, county: nil) + registration = create(:event_registration) + organization = create(:organization) + create(:address, addressable: organization, city:, state:, county:) + registration.event_registration_organizations.create!(organization:) + registration + end + + let!(:santa_monica) { registration_working_in(city: "Santa Monica", state: "CA", county: "Los Angeles") } + let!(:portland) { registration_working_in(city: "Portland", state: "OR", county: "Multnomah") } + + it "matches the work city (case-insensitive, partial), not the registrant's own address" do + create(:address, addressable: portland.registrant, city: "Santa Monica") + + expect(EventRegistration.work_city("santa")).to contain_exactly(santa_monica) + end + + it "matches the work state" do + expect(EventRegistration.work_state("OR")).to contain_exactly(portland) + end + + it "matches the work county within a state" do + expect(EventRegistration.work_county("CA|Los Angeles")).to contain_exactly(santa_monica) + expect(EventRegistration.work_county("OR|Los Angeles")).to be_empty end end diff --git a/spec/requests/events/attendees_spec.rb b/spec/requests/events/attendees_spec.rb index c2be1864b8..0c572602f7 100644 --- a/spec/requests/events/attendees_spec.rb +++ b/spec/requests/events/attendees_spec.rb @@ -341,7 +341,7 @@ expect(response.body).to include("Online Search") end - it "filters by a work country or org state drill-in (via the org they registered under)" do + it "filters by work country and state (via the org they registered under)" do org = create(:organization, name: "Toronto Org") create(:address, addressable: org, state: "ON", country: "Canada", inactive: false) attendee_registration.event_registration_organizations.create!(organization: org) @@ -354,7 +354,7 @@ expect(response.body).not_to include("Zed Zulu") org.addresses.first.update!(state: "WA", country: "United States") - get attendees_events_url(org_state: "WA"), headers: frame_headers + get attendees_events_url(state: "WA"), headers: frame_headers expect(response.body).to include("Ada Lovelace") expect(response.body).not_to include("Zed Zulu") end @@ -548,23 +548,27 @@ expect(response.body).to include("Ada Lovelace") end - it "filters by address data (city) without requiring a registration to carry it" do - create(:address, addressable: attendee, city: "Portland", state: "OR", inactive: false) + it "filters by work city, not the person's own address" do + org = create(:organization) + create(:address, addressable: org, city: "Portland", state: "OR", inactive: false) + attendee_registration.event_registration_organizations.create!(organization: org) other = create(:person, first_name: "Cara", last_name: "Coast") create(:event_registration, event: recent_training, registrant: other, status: "attended") - create(:address, addressable: other, city: "Seattle", state: "WA", inactive: false) + create(:address, addressable: other, city: "Portland", state: "OR", inactive: false) get attendees_events_url(city: "portland"), headers: frame_headers expect(response.body).to include("Ada Lovelace") expect(response.body).not_to include("Cara Coast") end - # City and state have to describe one address, the way the roster's shared - # join does — otherwise someone with homes in two states matches a city from - # one and a state from the other. - it "matches city and state against the same address" do - create(:address, addressable: attendee, city: "Portland", state: "OR", inactive: false) - create(:address, addressable: attendee, city: "Seattle", state: "WA", inactive: false) + # City and state have to describe one address — otherwise someone working in + # two states matches a city from one and a state from the other. + it "matches city and state against the same work address" do + [ %w[Portland OR], %w[Seattle WA] ].each do |city, state| + org = create(:organization) + create(:address, addressable: org, city:, state:, inactive: false) + attendee_registration.event_registration_organizations.create!(organization: org) + end get attendees_events_url(city: "Seattle", state: "WA"), headers: frame_headers expect(response.body).to include("Ada Lovelace") diff --git a/spec/requests/events/bulk_reminders_spec.rb b/spec/requests/events/bulk_reminders_spec.rb index 91219c8e92..2d3074abaf 100644 --- a/spec/requests/events/bulk_reminders_spec.rb +++ b/spec/requests/events/bulk_reminders_spec.rb @@ -47,8 +47,11 @@ def checked?(body, registration) expect(checked?(response.body, sam)).to be(false) end - it "filters recipients by the shared city search" do - create(:address, addressable: jane.registrant, city: "Santa Monica") + it "filters recipients by the shared city search, on where they work" do + organization = create(:organization) + create(:address, addressable: organization, city: "Santa Monica") + jane.event_registration_organizations.create!(organization:) + create(:address, addressable: sam.registrant, city: "Santa Monica") get preview_reminder_event_path(event, city: "santa"), headers: { "Turbo-Frame" => "reminder_recipients" } diff --git a/spec/requests/events_spec.rb b/spec/requests/events_spec.rb index 6c35660d68..4a33d16042 100644 --- a/spec/requests/events_spec.rb +++ b/spec/requests/events_spec.rb @@ -1779,12 +1779,13 @@ def pay_for_others_classes(template) expect(response.body).to include("Any status") end - it "narrows the roster by city" do + it "narrows the roster by work city" do here = create(:person, first_name: "Cityful", last_name: "Local") - create(:address, addressable: here, city: "Santa Monica") there = create(:person, first_name: "Faraway", last_name: "Person") - create(:address, addressable: there, city: "Portland") - create(:event_registration, event: event, registrant: here) + create(:address, addressable: there, city: "Santa Monica") + here_org = create(:organization) + create(:address, addressable: here_org, city: "Santa Monica") + create(:event_registration, event: event, registrant: here).event_registration_organizations.create!(organization: here_org) create(:event_registration, event: event, registrant: there) get registrants_event_path(event, city: "santa") @@ -2577,9 +2578,11 @@ def ce_chip_text let!(:ca_kings_reg) { create(:event_registration, event: event, registrant: ca_kings_person) } before do - create(:address, addressable: ca_person, state: "CA", county: "Los Angeles") - create(:address, addressable: ny_person, state: "NY", county: "Kings") - create(:address, addressable: ca_kings_person, state: "CA", county: "Kings") + { ca_reg => [ "CA", "Los Angeles" ], ny_reg => [ "NY", "Kings" ], ca_kings_reg => [ "CA", "Kings" ] }.each do |registration, (state, county)| + organization = create(:organization) + create(:address, addressable: organization, state:, county:) + registration.event_registration_organizations.create!(organization:) + end sign_in admin end diff --git a/spec/services/attendees_active_filters_spec.rb b/spec/services/attendees_active_filters_spec.rb index 9ea7f18936..babeb98101 100644 --- a/spec/services/attendees_active_filters_spec.rb +++ b/spec/services/attendees_active_filters_spec.rb @@ -30,7 +30,6 @@ def chips_for(params) it "labels the literal-value drill-ins" do expect(chips_for(country: "Canada").first[:label]).to eq("Org country: Canada") - expect(chips_for(org_state: "CA").first[:label]).to eq("Org state: CA") expect(chips_for(school_district: "PPS").first[:label]).to eq("School district: PPS") expect(chips_for(org_city: "Austin, TX").first[:label]).to eq("Org city: Austin, TX") end diff --git a/spec/services/event_dashboard_spec.rb b/spec/services/event_dashboard_spec.rb index bb299e3ead..0e3564a8e0 100644 --- a/spec/services/event_dashboard_spec.rb +++ b/spec/services/event_dashboard_spec.rb @@ -445,9 +445,9 @@ cancelled_reg = event.event_registrations.find_by(registrant: cancelled_person) cancelled_reg.event_registration_organizations.create!(organization: org_excluded) create(:address, addressable: org_a, city: "Oakland", state: "OR", country: "United States") - chosen = create(:address, addressable: org_a, city: "Fresno", state: "CA", country: "United States") + chosen = create(:address, addressable: org_a, city: "Fresno", state: "CA", county: "Fresno", country: "United States") person1.affiliations.find_by(organization: org_a).update!(organization_address: chosen, event_registration: reg1) - create(:address, addressable: org_c, city: "Albany", state: "NY", country: "Mexico") + create(:address, addressable: org_c, city: "Albany", state: "NY", county: "Albany", country: "Mexico") create(:address, addressable: org_excluded, city: "Miami", state: "FL", country: "Canada") end @@ -466,14 +466,8 @@ expect(dashboard.registrant_city_breakdown.rows.map(&:city)).to contain_exactly("Fresno, CA", "Albany, NY") end - it "offers the State filter the registrants' own address states" do - expect(dashboard.registrant_address_states).to eq(%w[CA NY]) - end - end - - describe "counties" do - it "returns unique [ state, county ] pairs from active registrants' active addresses" do - expect(dashboard.counties).to eq([ [ "CA", "Los Angeles" ], [ "NY", "Kings" ] ]) + it "lists the [ state, county ] pairs registrants work in" do + expect(dashboard.counties).to eq([ [ "CA", "Fresno" ], [ "NY", "Albany" ] ]) end end diff --git a/spec/services/reminder_recipient_filter_spec.rb b/spec/services/reminder_recipient_filter_spec.rb index 612b03fecc..0e5d8204d4 100644 --- a/spec/services/reminder_recipient_filter_spec.rb +++ b/spec/services/reminder_recipient_filter_spec.rb @@ -95,13 +95,15 @@ def matched(params, registrations) expect(matched({ email: "amy@--aisha@" }, [ amy, aisha, sam ])).to eq([ amy.id, aisha.id ].to_set) end - it "filters by registrant city, skipping inactive addresses like the roster does" do + it "filters by work city, like the roster does" do here = registration - create(:address, addressable: here.registrant, city: "Santa Monica") - moved_away = registration(first_name: "Sam") - create(:address, addressable: moved_away.registrant, city: "Santa Monica", inactive: true) - expect(matched({ city: "santa" }, [ here, moved_away ])).to eq([ here.id ].to_set) - expect(EventRegistration.registrant_city("santa")).not_to include(moved_away) + organization = create(:organization) + create(:address, addressable: organization, city: "Santa Monica") + here.event_registration_organizations.create!(organization:) + lives_there = registration(first_name: "Sam") + create(:address, addressable: lives_there.registrant, city: "Santa Monica") + expect(matched({ city: "santa" }, [ here, lives_there ])).to eq([ here.id ].to_set) + expect(EventRegistration.work_city("santa")).to contain_exactly(here) end it "filters by registration comment text" do From 60bf20a7281da39d79e12d0e7daa787b95732e7d Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 07:41:47 -0400 Subject: [PATCH 5/6] Move the work-location charts and filters to #2844 Keeps this PR to the org-answer refactor so each reviews on its own. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 5 +- app/controllers/events_controller.rb | 81 +++++++++---- app/models/event_registration.rb | 23 ++-- app/presenters/registrant_city_breakdown.rb | 24 ++-- app/services/attendees_active_filters.rb | 7 +- app/services/attendees_breakdowns.rb | 41 +++++-- app/services/event_dashboard.rb | 113 ++++++++++++++---- app/services/registration_work_addresses.rb | 92 -------------- app/services/reminder_recipient_filter.rb | 16 ++- .../events/_registrant_breakdowns.html.erb | 2 +- ...5-registrant-charts-use-work-locations.yml | 13 -- spec/models/event_registration_spec.rb | 37 ++---- .../registrant_city_breakdown_spec.rb | 21 ++-- spec/requests/events/attendees_spec.rb | 38 ++---- spec/requests/events/bulk_reminders_spec.rb | 7 +- spec/requests/events_spec.rb | 25 ++-- .../services/attendees_active_filters_spec.rb | 2 +- spec/services/attendees_breakdowns_spec.rb | 11 +- spec/services/event_dashboard_spec.rb | 41 +++---- .../registration_work_addresses_spec.rb | 45 ------- .../reminder_recipient_filter_spec.rb | 14 +-- 21 files changed, 307 insertions(+), 351 deletions(-) delete mode 100644 app/services/registration_work_addresses.rb delete mode 100644 db/seeds/features/2026-10-25-registrant-charts-use-work-locations.yml delete mode 100644 spec/services/registration_work_addresses_spec.rb diff --git a/AGENTS.md b/AGENTS.md index e6ba3b04e6..1c3a76085b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -236,8 +236,7 @@ comparison — add a rule if none fits (e.g. `PersonPolicy#own_profile?`). Conve - `EventProgramStatusReport` — Cross-event program-status report grouped by calendar year: how many organizations were New / Ongoing / Reinstate at each facilitator training, counted both as organization-trainings (summed rows) and as distinct organizations classified at their earliest training. Sibling of `EventRevenueReport`/`EventParticipationReport`/`EventScholarshipReport` (includes `ReportPeriods`); powers the `events#program_statuses` page and the reports-hub program-status card - `EventParticipationReport` — Cross-event participation report grouped by calendar year (unique people trained vs attended seats vs per-status outcome counts, chart series) for the events participation page; sibling of `EventRevenueReport` - `AttendeesRoster` — Cross-event counterpart to `EventDashboard`: builds the per-registrant lookup maps the shared `events/_registrant_roster` partial reads (sector/age/org/status/location/scholarship/CE plus the events-attended column) for a paginated page of people; backs the `events#attendees` index. Takes `events:` + `registrations:` — the index's current filter scopes, already narrowed by `EventPolicy`'s `:reportable` scope — so a person's columns show what's in scope rather than their whole history, and never an event the viewer can't see -- `RegistrationWorkAddresses` — Where registrants work, per registration: each org linked on the registration (`EventRegistrationOrganization`), at the address picked on the registration's affiliation with it (`Affiliation#organization_address`), else the org's oldest active address. Groups registrant ids by US state, country, and "City, State"; `#matching(state:, county:, city:)` tests one address per row. The single source for every registrant location figure and filter: the charts' cities, states, and countries (`EventDashboard`, `AttendeesBreakdowns`), the roster/reminder State, County, and City filters (`EventRegistration.work_state`/`work_county`/`work_city`, `ReminderRecipientFilter`), and the attendees index's State/County/City filters and `country`/`org_city` drill-ins -- `AttendeesBreakdowns` — Aggregate counterpart to `EventDashboard`'s breakdown methods: computes the chart datasets (sectors, age groups, locations, program status, life experiences, settings, organizations, scholarship/CE) over an arbitrary people set — tags from profiles, cities/states/countries from `RegistrationWorkAddresses` — for the shared `events/_registrant_breakdowns` partial. Backs the `events#attendees` index charts (cross-event; `events:` / `registrations:` = the index's current filter scopes, already narrowed by `EventPolicy`'s `:reportable` scope) and the `events#recipients` charts frame (one event's scholarship recipients, `registrations:` = `EventRegistration.active` so it counts them regardless of attendance, `as_of:` = that event's start date so program status reads the same verdict its dashboard does). Also exposes `*_registrant_ids_by_*` maps mirroring `EventDashboard`'s, so a breakdown row can drill in by person id — they regroup rows already loaded for the counts, adding no queries +- `AttendeesBreakdowns` — Aggregate counterpart to `EventDashboard`'s breakdown methods: computes the chart datasets (sectors, age groups, locations, program status, life experiences, settings, organizations, scholarship/CE) over an arbitrary people set, profile-sourced, for the shared `events/_registrant_breakdowns` partial. Backs the `events#attendees` index charts (cross-event; `events:` / `registrations:` = the index's current filter scopes, already narrowed by `EventPolicy`'s `:reportable` scope) and the `events#recipients` charts frame (one event's scholarship recipients, `registrations:` = `EventRegistration.active` so it counts them regardless of attendance, `as_of:` = that event's start date so program status reads the same verdict its dashboard does). Also exposes `*_registrant_ids_by_*` maps mirroring `EventDashboard`'s, so a breakdown row can drill in by person id — they regroup rows already loaded for the counts, adding no queries - `AttendeesActiveFilters` — Human-readable, removable-chip descriptors for a page's "drill-in" filters (registrant_ids, organization, org city, age group / life experience / setting categories, country, school district, scholarship, CE, payment status, scholarship funding source, sector, state) — params that narrow a list without a field in a visible filter form. `CHIP_PARAMS` is the attendees index's set (omitting anything with its own control); pass `chip_params:` for a page with no filter form at all — `ROSTER_CHIP_PARAMS` for the per-event roster, `%w[ registrant_ids ]` for scholarship recipients - `ReportPeriods` — Shared module (included by `EventRevenueReport` and `EventParticipationReport`) resolving the reporting-hub period toggle (this year / last year / all time) to a metric scope + label for the summary cards - `EventScholarshipReport` — Cross-event scholarship report grouped by calendar year: scholarship dollars and award counts (funded vs unfunded, via `EventScholarshipFigures`) per facilitator training, plus an attended-trainee count split into "Live" (scheduled instructor-led) vs "On-demand" (`event.on_demand?`). Sibling of `EventRevenueReport`/`EventParticipationReport` (includes `ReportPeriods`); powers the `events#scholarships` report page and the reports-hub scholarship summary card @@ -288,7 +287,7 @@ comparison — add a rule if none fits (e.g. `PersonPolicy#own_profile?`). Conve - `FeatureCatalog` — Syncs the checked-in feature seeds (`db/seeds/features/*.yml`, one file per feature) into the `Feature` table behind the `/features` page (the "Sync latest updates" button). `#import!` (matched by `name`) loads existing features in one query, creates missing ones (case-insensitive name match), overwrites `SYNCED_FIELDS` (area, display_status, released_on, action_path, pr_number, summary, pro_tips, external_url, rhino_description — rich text compared as rendered HTML) on same-name records with whatever the seed carries, so seed corrections and trims propagate; skips any feature edited in-app since creation (`Feature#edited?`) and writes without bumping `updated_at`; returns a `Result` (`created`/`updated`/`skipped`) - `DisplayImagePresenter` — Image display logic - `ScholarshipsGrouping` (presenter) — Groups scholarships into the index's funder → grant → recipient hierarchy; grant-free awards collect under a trailing "Unfunded" group -- `RegistrantCityBreakdown` (presenter) — Groups an event's registrants by the city they work in (`RegistrationWorkAddresses`), counting registrants + scholarship recipients per city; drives the shared "Registrants by city" card inside `events/_registrant_breakdowns` on all three people-pages — per-event roster, cross-event attendees index, and scholarship recipients (fed plucked data by `EventDashboard` or `AttendeesBreakdowns`) +- `RegistrantCityBreakdown` (presenter) — Groups an event's registrants by the city of the org linked on their registration, counting registrants + scholarship recipients per city; drives the shared "Registrants by city" card inside `events/_registrant_breakdowns` on all three people-pages — per-event roster, cross-event attendees index, and scholarship recipients (fed plucked data by `EventDashboard` or `AttendeesBreakdowns`) - `AllocationLedgerLabel` (presenter) — Shared payment-method/label + check-number labelling for an allocation, used by the invoice and receipt ledgers so they can't drift - `InvoiceIssuer` (presenter) — Single source of truth for the issuing org's invoice/receipt header (name, address, email, tax id, payable-to note), shared by `EventInvoice`, `EventReceipt`, and `InvoicePresenter`. Name and tax id come from `Setting.app_organization`; the addresses and email are app-wide settings. `#address_lines` / `#remittance_address_lines` delegate to `Setting`, which reads the `Address` the row points at (`return_address` / `remittance_address`) and otherwise falls back to the app org's oldest address flagged for invoices (`Address.display_lines_for`); there are no hard-coded address literals. The email is `Setting.info_email`. The header carries the return address; `#remittance_address_lines` is the separate address cheques are mailed to, printed under the payable-to line on both invoices and on the pay-by-check callout diff --git a/app/controllers/events_controller.rb b/app/controllers/events_controller.rb index 8e0caa471e..bc70710f9c 100644 --- a/app/controllers/events_controller.rb +++ b/app/controllers/events_controller.rb @@ -244,12 +244,12 @@ def registrants scope = scope.comment_text(params[:comment]) if params[:comment].present? scope = scope.funder_name(params[:funder_name]) if params[:funder_name].present? scope = scope.submission_status(params[:submission_status], @event) if params[:submission_status].present? - scope = scope.work_city(params[:city]) if params[:city].present? + scope = scope.registrant_city(params[:city]) if params[:city].present? scope = scope.organization_linking_status(params[:org_status], @event) if params[:org_status].present? scope = scope.account_status(params[:account_status]) if params[:account_status].present? scope = scope.registrant_ids(params[:registrant_ids]) if params[:registrant_ids].present? - scope = scope.work_state(params[:state]) if params[:state].present? - scope = scope.work_county(params[:county]) if params[:county].present? + scope = scope.registrant_state(params[:state]) if params[:state].present? + scope = scope.registrant_county(params[:county]) if params[:county].present? scope = scope.registrant_sector(params[:sector]) if params[:sector].present? scope = scope.registered_between(params[:registered_from], params[:registered_to]) @@ -714,16 +714,18 @@ def filtered_roster_registrants end # Person ids behind the clicked breakdown row. Most dimensions arrive as an - # explicit id list (EventDashboard already has the maps); sector has no per-value - # id map, so it arrives as a named filter and resolves here — bounded to - # `registrant_ids` (this event's roster), since the result is only ever + # explicit id list (EventDashboard already has the maps); sector and state have + # no per-value id map, so they arrive as named filters and resolve here — bounded + # to `registrant_ids` (this event's roster), since the result is only ever # intersected with it and the unbounded lookup would load every person in the app - # tagged with that sector. + # tagged with that sector or living in that state. def roster_drill_in_ids(registrant_ids) if params[:registrant_ids].present? params[:registrant_ids].to_s.split("-").map(&:to_i).to_set elsif params[:sector].present? person_sector_ids(params[:sector]).where(sectorable_id: registrant_ids).pluck(:sectorable_id).to_set + elsif params[:state].present? + person_address_ids(state: params[:state]).where(addressable_id: registrant_ids).pluck(:addressable_id).to_set end end @@ -853,11 +855,11 @@ def filtered_attendees # Breakdown drill-in filters (set when a chart row is clicked). scope = scope.where(id: person_category_ids(params[:age_group])) if params[:age_group].present? scope = scope.where(id: person_category_ids(params[:life_experience])) if params[:life_experience].present? - scope = scope.where(id: attendee_work_addresses.registrant_ids_by_country.fetch(params[:country], [])) if params[:country].present? + scope = scope.where(id: person_country_ids(params[:country])) if params[:country].present? scope = scope.where(id: person_school_district_ids(params[:school_district])) if params[:school_district].present? scope = scope.where(id: person_program_status_ids(params[:program_status])) if params[:program_status].present? scope = scope.where(id: person_linked_organization_ids(params[:organization_id])) if params[:organization_id].present? - scope = scope.where(id: attendee_work_addresses.registrant_ids_by_city_label.fetch(params[:org_city], [])) if params[:org_city].present? + scope = scope.where(id: person_linked_org_city_ids(params[:org_city])) if params[:org_city].present? if params[:scholarship].present? # Scoped to one event, every status (including "no") is unambiguous at the # registration level, so reuse the shared scope. Cross-event, "No scholarship" @@ -869,17 +871,19 @@ def filtered_attendees scope.where(id: scholarship_status_person_ids(params[:scholarship])) end end - # Address filters match where people work on their in-scope registrations. City - # and state test one address, so someone working in two states can't match on a - # city from one and the state from the other. + # Address filters read the person's own address data directly (no registration + # required) — city is a free-text match, state/county exact. City and state ask + # one subquery so they describe the same address, matching the roster's shared + # join; two subqueries would let someone with homes in two states through on a + # city from one and a state from the other. if params[:state].present? || params[:city].present? - scope = scope.where(id: attendee_work_addresses.matching(state: params[:state], city: params[:city]).map(&:registrant_id)) + scope = scope.where(id: person_address_ids(state: params[:state], city: params[:city])) end if params[:county].present? # County options carry their state ("STATE::County") so same-named counties # across states don't collide. county_state, county_name = params[:county].split("::", 2) - scope = scope.where(id: attendee_work_addresses.matching(state: county_state, county: county_name).map(&:registrant_id)) + scope = scope.where(id: person_address_ids(state: county_state, county: county_name)) end # Shared registration-level filters, applied cross-event with "any registration @@ -946,6 +950,10 @@ def person_category_ids(category_id) CategorizableItem.where(categorizable_type: "Person", category_id: category_id).select(:categorizable_id) end + def person_country_ids(country) + Address.active.where(addressable_type: "Person", country: country).select(:addressable_id) + end + # People whose event-registration-linked affiliation points at an organization # address in the given school district (Affiliation#organization_address → # Address#district), scoped to the in-scope registrations — matching the @@ -966,10 +974,26 @@ def person_linked_organization_ids(organization_id) .select(Arel.sql("event_registrations.registrant_id")) end - # Where each in-scope registration's registrant works, behind the location filters - # and drill-ins — the same source as the breakdowns they come from. - def attendee_work_addresses - @attendee_work_addresses ||= RegistrationWorkAddresses.new(attendee_registrations.pluck(:id)) + # People whose linked training org sits in the given "City, State" (matched on + # each org's first active address, mirroring the cities breakdown's labeling). + def person_linked_org_city_ids(city_label) + org_ids = org_ids_by_city_label[city_label] || [] + return Person.none if org_ids.empty? + person_linked_organization_ids(org_ids) + end + + def org_ids_by_city_label + @org_ids_by_city_label ||= Address.active + .where(addressable_type: "Organization", + addressable_id: EventRegistrationOrganization.where(event_registration_id: attendee_registrations.select(:id)).select(:organization_id)) + .order(:id) + .pluck(:addressable_id, :city, :state) + .each_with_object({}) do |(org_id, city, state), first_label| + next if first_label.key?(org_id) + first_label[org_id] = [ city, state ].compact_blank.join(", ").presence + end + .group_by { |_org_id, label| label } + .transform_values { |pairs| pairs.map(&:first) } end # Anchored the same way the index's own column is: this list spans events, so @@ -1007,6 +1031,21 @@ def person_affiliation_status_ids(status) Affiliation.with_status(status).select(:person_id) end + # Person ids behind active person addresses, narrowed by any of state, county or + # city. City is a free-text LIKE (matching the roster's registrant_city filter), + # state/county are exact. Street isn't a filter anywhere yet, but the column + # exists — add a `street:` clause here the same way if one is ever needed. + def person_address_ids(state: nil, county: nil, city: nil) + scope = Address.active.where(addressable_type: "Person") + scope = scope.where(state: state) if state.present? + scope = scope.where(county: county) if county.present? + if city.present? + like = "%#{Address.sanitize_sql_like(city.downcase.strip)}%" + scope = scope.where("LOWER(addresses.city) LIKE ?", like) + end + scope.select(:addressable_id) + end + # Option lists for the attendees index filter selects, built once per full page # load from the viewer's WHOLE reportable population — deliberately un-narrowed by # the active filters, so filtering never removes the option that would undo it. @@ -1015,9 +1054,9 @@ def set_attendee_filter_options @attendee_years = @attendee_event_options.filter_map { |event| event.start_date&.year }.uniq.sort.reverse person_ids = Person.where(id: EventRegistration.where(event_id: reportable_events.select(:id)).select(:registrant_id)) @attendee_sectors = Sector.where(id: SectorableItem.where(sectorable_type: "Person", sectorable_id: person_ids).select(:sector_id)).order(:name) - work_addresses = RegistrationWorkAddresses.new(EventRegistration.where(event_id: reportable_events.select(:id)).pluck(:id)) - @attendee_states = work_addresses.rows.filter_map(&:state).compact_blank.uniq.sort - @attendee_counties = work_addresses.counties + addresses = Address.active.where(addressable_type: "Person", addressable_id: person_ids) + @attendee_states = addresses.where.not(state: [ nil, "" ]).distinct.pluck(:state).sort + @attendee_counties = addresses.where.not(county: [ nil, "" ]).where.not(state: [ nil, "" ]).distinct.pluck(:state, :county).sort @attendee_topic_types = TopicSubscriptionType.active.ordered end diff --git a/app/models/event_registration.rb b/app/models/event_registration.rb index 0240a3a7d0..e12b2e9cf1 100644 --- a/app/models/event_registration.rb +++ b/app/models/event_registration.rb @@ -220,20 +220,27 @@ class EventRegistration < ApplicationRecord end scope } - # Where the registrant works on the registration (RegistrationWorkAddresses). - scope :work_state, ->(state) { - where(id: RegistrationWorkAddresses.new(pluck(:id)).matching(state:).map(&:registration_id)) + scope :registrant_state, ->(state) { + joins(registrant: :addresses) + .where(addresses: { inactive: false, state: state }) + .distinct } # Accepts "STATE|County" (from the manage filter) to scope to a county within # a specific state, or a bare county name for backward compatibility. - scope :work_county, ->(value) { + scope :registrant_county, ->(value) { state, county = value.to_s.include?("|") ? value.split("|", 2) : [ nil, value.to_s ] next none if county.blank? - where(id: RegistrationWorkAddresses.new(pluck(:id)).matching(state:, county:).map(&:registration_id)) + conditions = { inactive: false, county: county } + conditions[:state] = state if state.present? + joins(registrant: :addresses).where(addresses: conditions).distinct } - scope :work_city, ->(term) { - next all if term.blank? - where(id: RegistrationWorkAddresses.new(pluck(:id)).matching(city: term).map(&:registration_id)) + scope :registrant_city, ->(term) { + return all if term.blank? + like = "%#{sanitize_sql_like(term.downcase.strip)}%" + joins(registrant: :addresses) + .where("LOWER(addresses.city) LIKE ?", like) + .where(addresses: { inactive: false }) + .distinct } scope :registrant_sector, ->(sector_id) { joins(registrant: :sectorable_items) diff --git a/app/presenters/registrant_city_breakdown.rb b/app/presenters/registrant_city_breakdown.rb index 6c68782b66..14514f9394 100644 --- a/app/presenters/registrant_city_breakdown.rb +++ b/app/presenters/registrant_city_breakdown.rb @@ -1,21 +1,24 @@ -# Groups an event's registrants by the city they work in (RegistrationWorkAddresses), -# counting distinct registrants and scholarship recipients per city. Powers the shared "Registrants by city" breakdown on the event +# Groups an event's registrants by the city of the organization linked on their +# registration, counting distinct registrants and scholarship recipients per +# city. Powers the shared "Registrants by city" breakdown on the event # background dashboard and the scholarship-recipients page. Pure/in-memory so # EventDashboard feeds it already-plucked data (see [[event_dashboard]]). # # A registrant linked to orgs in two cities counts under each; the "Unknown" -# bucket (a linked org with no address on file) always sorts last. +# bucket (a linked org with no city on file) always sorts last. class RegistrantCityBreakdown UNKNOWN_LABEL = "Unknown".freeze Row = Struct.new(:city, :registrant_count, :scholarship_count, :registrant_ids, :scholarship_recipient_ids, keyword_init: true) - # city_registrant_pairs: [ [ "City, State", person_id ], ... ], one per - # organization linked on a registration; a nil - # city ⇒ Unknown. + # org_registrant_pairs: [ [ organization_id, person_id ], ... ], one per + # organization linked on a registration. + # city_by_org: { organization_id => "City, State" }; a missing + # entry means that org has no city ⇒ Unknown. # scholarship_recipient_ids: person ids awarded a scholarship this event. - def initialize(city_registrant_pairs:, scholarship_recipient_ids:) - @city_registrant_pairs = city_registrant_pairs + def initialize(org_registrant_pairs:, city_by_org:, scholarship_recipient_ids:) + @org_registrant_pairs = org_registrant_pairs + @city_by_org = city_by_org @scholarship_recipient_ids = scholarship_recipient_ids.to_set end @@ -46,8 +49,9 @@ def any? private def registrant_ids_by_city - @registrant_ids_by_city ||= @city_registrant_pairs.each_with_object(Hash.new { |h, k| h[k] = Set.new }) do |(city, person_id), map| - map[city.presence || UNKNOWN_LABEL] << person_id + @registrant_ids_by_city ||= @org_registrant_pairs.each_with_object(Hash.new { |h, k| h[k] = Set.new }) do |(org_id, person_id), map| + city = @city_by_org[org_id].presence || UNKNOWN_LABEL + map[city] << person_id end end end diff --git a/app/services/attendees_active_filters.rb b/app/services/attendees_active_filters.rb index a2a836629e..ad6d86d185 100644 --- a/app/services/attendees_active_filters.rb +++ b/app/services/attendees_active_filters.rb @@ -19,8 +19,8 @@ class AttendeesActiveFilters country school_district payment_status funder ].freeze # The roster has no filter form at all, so every drill-in it accepts needs a - # chip — including sector, which the index leaves out because it has a control for it. - ROSTER_CHIP_PARAMS = %w[ registrant_ids sector ].freeze + # chip — including the two the index leaves out because it has controls for them. + ROSTER_CHIP_PARAMS = %w[ registrant_ids sector state ].freeze def initialize(params, chip_params: CHIP_PARAMS) @params = params @@ -45,11 +45,12 @@ def label_for(param, value) case param when "registrant_ids" then "#{value.to_s.split("-").size} selected people" when "sector" then record_label("Sector", Sector, value) + when "state" then "State: #{value}" when "organization_id" then record_label("Organization", Organization, value) when "org_city" then "Org city: #{value}" when "age_group" then record_label("Age group", Category, value) when "life_experience" then record_label("Life experience", Category, value) - when "country" then "Org country: #{value}" + when "country" then "Country: #{value}" when "school_district" then "School district: #{value}" when "payment_status" then option_label("Payment", EventRegistration::PAYMENT_STATUS_FILTER_OPTIONS, value) when "funder" then option_label("Funding", EventRegistration::FUNDER_FILTER_OPTIONS, value) diff --git a/app/services/attendees_breakdowns.rb b/app/services/attendees_breakdowns.rb index 9931451365..58d5222dbe 100644 --- a/app/services/attendees_breakdowns.rb +++ b/app/services/attendees_breakdowns.rb @@ -3,8 +3,8 @@ # whole filtered attendee population (not one page). Method names/return shapes # match EventDashboard so the shared _registrant_breakdowns partial reads either. # -# Sourced from each person's profile (sectors, age groups, categories) and from the -# orgs, work addresses, scholarships, and CE tied to their attended-training registrations. +# Sourced from each person's profile (sectors, age groups, categories, addresses) +# and from the orgs/scholarships/CE tied to their attended-training registrations. class AttendeesBreakdowns # events: the events whose registrations may be counted (for the attendees index, # the viewer's reportable events narrowed by the active filters), so the @@ -140,12 +140,13 @@ def ce_registrant_ids # --- Cities ---------------------------------------------------------------- - # Attendees grouped by the city they work in on their attended-training - # registrations, with scholarship recipients per city — the shared + # Attendees grouped by the city of the organization linked on their attended- + # training registrations, with scholarship recipients per city — the shared # "Registrants by city" breakdown, aggregated across all trainings. def registrant_city_breakdown @registrant_city_breakdown ||= RegistrantCityBreakdown.new( - city_registrant_pairs: work_addresses.rows.map { |row| [ row.city_label, row.registrant_id ] }, + org_registrant_pairs: org_registrant_pairs, + city_by_org: city_by_organization, scholarship_recipient_ids: scholarship_recipient_ids ) end @@ -180,11 +181,11 @@ def life_experience_registrant_ids_by_category end def state_registrant_ids_by_state - work_addresses.registrant_ids_by_state + @state_registrant_ids_by_state ||= person_ids_by(address_rows.select { |_, state, _| Address::US_STATE_ABBREVIATIONS.include?(state.to_s.upcase) }) { |row| row[1] } end def country_registrant_ids_by_country - work_addresses.registrant_ids_by_country + @country_registrant_ids_by_country ||= person_ids_by(address_rows.reject { |_, _, country| country.blank? }) { |row| row[2] } end def school_district_registrant_ids_by_district @@ -289,13 +290,19 @@ def category_rows(category_type_name) .pluck(:categorizable_id, :category_id) end + # [ [ person_id, state, country ], ... ] — one load behind the state and country + # counts and their id maps. + def address_rows + @address_rows ||= active_addresses.pluck(:addressable_id, :state, :country) + end + # { key => distinct person ids }, grouping [ person_id, ... ] rows by the block. def person_ids_by(rows) rows.group_by { |row| yield(row) }.transform_values { |grouped| grouped.map(&:first).uniq } end - def work_addresses - @work_addresses ||= RegistrationWorkAddresses.new(registration_ids) + def active_addresses + Address.active.where(addressable_type: "Person", addressable_id: person_ids) end def org_registrant_pairs @@ -340,6 +347,22 @@ def program_status_by_organization end end + # "City, State" per linked organization, from its first active address (mirrors + # EventDashboard#city_by_organization). Orgs with no active address are absent, + # so their registrants fall into the breakdown's Unknown bucket. + def city_by_organization + org_ids = org_registrant_pairs.map(&:first).uniq + Address.active + .where(addressable_type: "Organization", addressable_id: org_ids) + .order(:id) + .pluck(:addressable_id, :city, :state) + .each_with_object({}) do |(org_id, city, state), map| + next if map.key?(org_id) + label = [ city, state ].compact_blank.join(", ").presence + map[org_id] = label if label + end + end + def scholarship_recipient_ids @scholarship_recipient_ids ||= Scholarship .not_declined diff --git a/app/services/event_dashboard.rb b/app/services/event_dashboard.rb index 3434bb0760..7f78748c07 100644 --- a/app/services/event_dashboard.rb +++ b/app/services/event_dashboard.rb @@ -147,12 +147,14 @@ def scholarship_recipient_count recognized_scholarships.map(&:recipient_id).uniq.size end - # This event's registrants grouped by the city they work in, with the - # scholarship-recipient count per city — the shared "Registrants by city" - # breakdown on the background and recipients pages. + # This event's registrants grouped by the city of the organization linked on + # their registration, with the scholarship-recipient count per city — the + # shared "Registrants by city" breakdown on the background and recipients + # pages. Fed already-plucked data so the rollup runs in memory. def registrant_city_breakdown @registrant_city_breakdown ||= RegistrantCityBreakdown.new( - city_registrant_pairs: work_addresses.rows.map { |row| [ row.city_label, row.registrant_id ] }, + org_registrant_pairs: registration_org_registrant_pairs, + city_by_org: city_by_organization, scholarship_recipient_ids: scholarships.distinct.pluck(:recipient_id) ) end @@ -850,34 +852,52 @@ def referral_source_counts .sort_by { |label, count| [ -count, label ] } end - # The US states registrants work in (RegistrationWorkAddresses). + # US states only — international registrants' regions (e.g. provinces) are + # excluded so the States breakdown reflects domestic residents. def states - @states ||= state_registrant_ids_by_state.keys.sort + @states ||= us_state_addresses.distinct.pluck(:state).sort end + # Distinct US-resident registrant count per state. def state_counts - @state_counts ||= state_registrant_ids_by_state.transform_values(&:size) + @state_counts ||= us_state_addresses.group(:state).distinct.count(:addressable_id) end + # Registrant ids that have at least one active US address with a state on file — + # i.e. the people behind the states count. def state_registrant_ids - @state_registrant_ids ||= state_registrant_ids_by_state.values.flatten.uniq + @state_registrant_ids ||= us_state_addresses.distinct.pluck(:addressable_id) end - def state_registrant_ids_by_state - work_addresses.registrant_ids_by_state - end - - # The countries registrants work in (RegistrationWorkAddresses). def countries - @countries ||= country_registrant_ids_by_country.keys.sort + @countries ||= Address + .active + .where(addressable_type: "Person", addressable_id: registrant_ids) + .where.not(country: [ nil, "" ]) + .distinct + .pluck(:country) + .sort end + # Distinct registrant count per country. def country_counts - @country_counts ||= country_registrant_ids_by_country.transform_values(&:size) + @country_counts ||= Address + .active + .where(addressable_type: "Person", addressable_id: registrant_ids) + .where.not(country: [ nil, "" ]) + .group(:country) + .distinct + .count(:addressable_id) end + # Registrant ids that have at least one active address with a country on file. def country_registrant_ids - @country_registrant_ids ||= country_registrant_ids_by_country.values.flatten.uniq + @country_registrant_ids ||= Address + .active + .where(addressable_type: "Person", addressable_id: registrant_ids) + .where.not(country: [ nil, "" ]) + .distinct + .pluck(:addressable_id) end # School districts across active registrants' affiliation organization addresses @@ -896,8 +916,17 @@ def school_district_registrant_ids @school_district_registrant_ids ||= affiliation_district_pairs.map(&:last).uniq end + # Registrant ids per country, keyed by country name — the people behind each + # country row, for drilling into the matching registrant list. def country_registrant_ids_by_country - work_addresses.registrant_ids_by_country + @country_registrant_ids_by_country ||= Address + .active + .where(addressable_type: "Person", addressable_id: registrant_ids) + .where.not(country: [ nil, "" ]) + .distinct + .pluck(:country, :addressable_id) + .group_by(&:first) + .transform_values { |rows| rows.map(&:last).uniq } end # Registrant ids per school district, for the breakdown's per-row drill-in. A @@ -910,10 +939,19 @@ def school_district_registrant_ids_by_district .transform_values(&:to_a) end - # [ state, county ] pairs registrants work in. Pairing in the state lets the - # manage filter disambiguate same-named counties ("CA - Warren" vs "NY - Warren"). + # Distinct [ state, county ] pairs across active registrants' active addresses, + # sorted by state then county. Pairing in the state lets the manage filter + # disambiguate same-named counties across states (e.g. "CA - Warren" vs + # "NY - Warren"). def counties - work_addresses.counties + @counties ||= Address + .active + .where(addressable_type: "Person", addressable_id: registrant_ids) + .where.not(county: [ nil, "" ]) + .where.not(state: [ nil, "" ]) + .distinct + .pluck(:state, :county) + .sort end # Life experiences captured at registration: StoryPopulation categories tagged @@ -947,8 +985,13 @@ def life_experience_registrant_ids_by_category private - def work_addresses - @work_addresses ||= RegistrationWorkAddresses.new(active_registration_ids) + # Active registrant addresses whose state is a recognized US state/territory — + # the source for every States figure (count card, choropleth, and drill-in). + def us_state_addresses + Address + .active + .where(addressable_type: "Person", addressable_id: registrant_ids) + .where("UPPER(addresses.state) IN (?)", Address::US_STATE_ABBREVIATIONS) end # [ [ district, registrant_id ], ... ] from the affiliations linked to this event's @@ -1296,6 +1339,32 @@ def self_funded_grant_ids @self_funded_grant_ids ||= Grant.self_funded_ids end + # [ [ organization_id, registrant_id ], ... ] from the organizations linked on + # each active registration (the registration-time snapshot only, not later + # affiliations) — the basis for grouping registrants by their org's city. + def registration_org_registrant_pairs + @registration_org_registrant_pairs ||= EventRegistrationOrganization + .joins(:event_registration) + .where(event_registration_id: active_registration_ids) + .pluck(:organization_id, "event_registrations.registrant_id") + end + + # "City, State" per linked organization, from its first active address (mirrors + # Organization#program_location). Orgs with no active address are absent, so + # their registrants fall into the breakdown's Unknown bucket. + def city_by_organization + org_ids = registration_org_registrant_pairs.map(&:first).uniq + Address.active + .where(addressable_type: "Organization", addressable_id: org_ids) + .order(:id) + .pluck(:addressable_id, :city, :state) + .each_with_object({}) do |(org_id, city, state), map| + next if map.key?(org_id) + label = [ city, state ].compact_blank.join(", ").presence + map[org_id] = label if label + end + end + def paid_registration_ids @paid_registration_ids ||= billable_registration_ids.select do |id| allocated_by_registration.fetch(id, 0) >= event.cost_cents.to_i diff --git a/app/services/registration_work_addresses.rb b/app/services/registration_work_addresses.rb deleted file mode 100644 index bc65ec1f9a..0000000000 --- a/app/services/registration_work_addresses.rb +++ /dev/null @@ -1,92 +0,0 @@ -# Where registrants work, per registration: each organization linked on the -# registration, at the address picked on the registration's affiliation with it -# (Affiliation#organization_address), else that org's oldest active address. The -# source for the registrant charts' and filters' cities, counties, states, and -# countries. A linked org with no active address still yields a row, with no -# location, so the cities breakdown can count it as Unknown. -class RegistrationWorkAddresses - Row = Struct.new(:registration_id, :organization_id, :registrant_id, :city, :county, :state, :country, keyword_init: true) do - def city_label - [ city, state ].compact_blank.join(", ").presence - end - end - - def initialize(registration_ids) - @registration_ids = registration_ids - end - - def rows - @rows ||= links.map do |registration_id, organization_id, registrant_id| - address = chosen_addresses[chosen_address_ids[[ registration_id, organization_id ]]] || - oldest_address_by_organization[organization_id] - Row.new(registration_id:, organization_id:, registrant_id:, - city: address&.city, county: address&.county, state: address&.state, country: address&.country) - end - end - - # Rows whose one address matches every given part: state and county exactly, - # city as a case-insensitive substring. - def matching(state: nil, county: nil, city: nil) - term = city.to_s.downcase.strip.presence - rows.select do |row| - (state.blank? || row.state == state) && - (county.blank? || row.county == county) && - (term.nil? || row.city.to_s.downcase.include?(term)) - end - end - - def rows_by_registration - @rows_by_registration ||= rows.group_by(&:registration_id) - end - - # Distinct [ state, county ] pairs, sorted, for the County filters. - def counties - @counties ||= rows.filter_map { |row| [ row.state, row.county ] if row.state.present? && row.county.present? }.uniq.sort - end - - # US states only, so regions abroad stay off the States map. - def registrant_ids_by_state - @registrant_ids_by_state ||= registrant_ids_by(rows.select { |row| Address::US_STATE_ABBREVIATIONS.include?(row.state.to_s.upcase) }, &:state) - end - - def registrant_ids_by_country - @registrant_ids_by_country ||= registrant_ids_by(rows.select { |row| row.country.present? }, &:country) - end - - def registrant_ids_by_city_label - @registrant_ids_by_city_label ||= registrant_ids_by(rows.select(&:city_label), &:city_label) - end - - private - - def registrant_ids_by(rows, &key) - rows.group_by(&key).transform_values { |grouped| grouped.map(&:registrant_id).uniq } - end - - def links - @links ||= EventRegistrationOrganization - .joins(:event_registration) - .where(event_registration_id: @registration_ids) - .pluck(:event_registration_id, :organization_id, Arel.sql("event_registrations.registrant_id")) - end - - def chosen_address_ids - @chosen_address_ids ||= Affiliation - .where(event_registration_id: @registration_ids) - .where.not(organization_address_id: nil) - .pluck(:event_registration_id, :organization_id, :organization_address_id) - .to_h { |registration_id, organization_id, address_id| [ [ registration_id, organization_id ], address_id ] } - end - - def chosen_addresses - @chosen_addresses ||= Address.active.where(id: chosen_address_ids.values.uniq).select(:id, :city, :county, :state, :country).index_by(&:id) - end - - def oldest_address_by_organization - @oldest_address_by_organization ||= Address.active - .where(addressable_type: "Organization", addressable_id: links.map(&:second).uniq) - .order(:id) - .select(:id, :addressable_id, :city, :county, :state, :country) - .each_with_object({}) { |address, map| map[address.addressable_id] ||= address } - end -end diff --git a/app/services/reminder_recipient_filter.rb b/app/services/reminder_recipient_filter.rb index b4282b1b4e..495964d81a 100644 --- a/app/services/reminder_recipient_filter.rb +++ b/app/services/reminder_recipient_filter.rb @@ -79,8 +79,8 @@ def dropdown_matched_ids scope = scope.organization_linking_status(@params[:org_status], @event) if @params[:org_status].present? scope = scope.account_status(@params[:account_status]) if @params[:account_status].present? scope = scope.submission_status(@params[:submission_status], @event) if @params[:submission_status].present? - scope = scope.work_state(@params[:state]) if @params[:state].present? - scope = scope.work_county(@params[:county]) if @params[:county].present? + scope = scope.registrant_state(@params[:state]) if @params[:state].present? + scope = scope.registrant_county(@params[:county]) if @params[:county].present? scope.pluck(:id).to_set end @@ -127,18 +127,16 @@ def matches_comment?(reg) end end - # Matches the work city, like EventRegistration.work_city, so the same City - # value narrows the roster and the picker identically. + # Skips inactive addresses, matching EventRegistration.registrant_city so the + # same City value narrows the roster and the picker identically. def matches_city?(reg) any_term?(:city) do |term| - work_addresses.rows_by_registration.fetch(reg.id, []).any? { |row| row.city.to_s.downcase.include?(term) } + reg.registrant.addresses.any? do |address| + !address.inactive? && address.city.to_s.downcase.include?(term) + end end end - def work_addresses - @work_addresses ||= RegistrationWorkAddresses.new(@event_registrations.map(&:id)) - end - # Matches against every email we hold for the registrant (their account email, # contact email and secondary email), so a search hits whichever address the # admin remembers. diff --git a/app/views/events/_registrant_breakdowns.html.erb b/app/views/events/_registrant_breakdowns.html.erb index 186206752a..934d8d1a21 100644 --- a/app/views/events/_registrant_breakdowns.html.erb +++ b/app/views/events/_registrant_breakdowns.html.erb @@ -77,7 +77,7 @@ <% sector_paths["Other"] = by_ids.(data.other_sector_response_registrant_ids) if data.other_sector_response_count.positive? %> <% age_group_paths = data.age_groups.to_h { |c| [ c.name, by_ids.(data.age_group_registrant_ids_by_category.fetch(c.id, [])) ] } %> <% all_age_group_paths = data.all_age_groups.to_h { |c| [ c.name, by_ids.(data.all_age_group_registrant_ids_by_category.fetch(c.id, [])) ] } %> - <% state_paths = data.state_registrant_ids_by_state.transform_values { |ids| by_ids.(ids) } %> + <% state_paths = data.states.index_with { |state| roster_event_path(event, state: state, charts: 1) } %> <% country_paths = data.country_registrant_ids_by_country.transform_values { |ids| by_ids.(ids) } %> <% school_district_paths = data.school_district_registrant_ids_by_district.transform_values { |ids| by_ids.(ids) } %> <% life_experience_paths = data.life_experiences.to_h { |c| [ c.name, by_ids.(data.life_experience_registrant_ids_by_category.fetch(c.id, [])) ] } %> diff --git a/db/seeds/features/2026-10-25-registrant-charts-use-work-locations.yml b/db/seeds/features/2026-10-25-registrant-charts-use-work-locations.yml deleted file mode 100644 index a6864fb1b0..0000000000 --- a/db/seeds/features/2026-10-25-registrant-charts-use-work-locations.yml +++ /dev/null @@ -1,13 +0,0 @@ -name: "Registrant charts and location filters use where people work" -area: reporting -display_status: admin_facing -released_on: 2026-10-25 -pr_number: 2842 -summary: >- - The cities, states, and countries charts, and the City, State, and County - filters on the roster, the attendees page, and the bulk email picker, now - use where each registrant works: the address of the organization they - registered under. -pro_tips: - - "If the registrant picked one of the organization's addresses, that one counts. Otherwise the organization's first address on file counts." - - "A registrant with no organization linked on their registration has no work location, so the location filters leave them out." diff --git a/spec/models/event_registration_spec.rb b/spec/models/event_registration_spec.rb index 811bcdf6cb..c1133cfdc0 100644 --- a/spec/models/event_registration_spec.rb +++ b/spec/models/event_registration_spec.rb @@ -600,31 +600,18 @@ def registration_with_scholarship end end - describe "work location scopes" do - def registration_working_in(city:, state:, county: nil) - registration = create(:event_registration) - organization = create(:organization) - create(:address, addressable: organization, city:, state:, county:) - registration.event_registration_organizations.create!(organization:) - registration - end - - let!(:santa_monica) { registration_working_in(city: "Santa Monica", state: "CA", county: "Los Angeles") } - let!(:portland) { registration_working_in(city: "Portland", state: "OR", county: "Multnomah") } - - it "matches the work city (case-insensitive, partial), not the registrant's own address" do - create(:address, addressable: portland.registrant, city: "Santa Monica") - - expect(EventRegistration.work_city("santa")).to contain_exactly(santa_monica) - end - - it "matches the work state" do - expect(EventRegistration.work_state("OR")).to contain_exactly(portland) - end - - it "matches the work county within a state" do - expect(EventRegistration.work_county("CA|Los Angeles")).to contain_exactly(santa_monica) - expect(EventRegistration.work_county("OR|Los Angeles")).to be_empty + describe ".registrant_city" do + it "matches registrants with an address in the city (case-insensitive, partial)" do + in_city = create(:person) + create(:address, addressable: in_city, city: "Santa Monica") + out_city = create(:person) + create(:address, addressable: out_city, city: "Portland") + reg_in = create(:event_registration, registrant: in_city) + reg_out = create(:event_registration, registrant: out_city) + + results = EventRegistration.registrant_city("santa") + expect(results).to include(reg_in) + expect(results).not_to include(reg_out) end end diff --git a/spec/presenters/registrant_city_breakdown_spec.rb b/spec/presenters/registrant_city_breakdown_spec.rb index 4df4cc523e..235b926373 100644 --- a/spec/presenters/registrant_city_breakdown_spec.rb +++ b/spec/presenters/registrant_city_breakdown_spec.rb @@ -4,7 +4,8 @@ describe "#rows" do it "counts distinct registrants and scholarship recipients per city, most first" do breakdown = described_class.new( - city_registrant_pairs: [ [ "Los Angeles, CA", 10 ], [ "Los Angeles, CA", 11 ], [ "Richmond, CA", 12 ] ], + org_registrant_pairs: [ [ 1, 10 ], [ 1, 11 ], [ 2, 12 ] ], + city_by_org: { 1 => "Los Angeles, CA", 2 => "Richmond, CA" }, scholarship_recipient_ids: [ 10 ] ) @@ -15,7 +16,8 @@ it "exposes per-city registrant and scholarship-recipient ids for drill-in links" do breakdown = described_class.new( - city_registrant_pairs: [ [ "Los Angeles, CA", 10 ], [ "Los Angeles, CA", 11 ], [ "Richmond, CA", 12 ] ], + org_registrant_pairs: [ [ 1, 10 ], [ 1, 11 ], [ 2, 12 ] ], + city_by_org: { 1 => "Los Angeles, CA", 2 => "Richmond, CA" }, scholarship_recipient_ids: [ 10, 12 ] ) @@ -24,18 +26,20 @@ expect(la.scholarship_recipient_ids).to contain_exactly(10) end - it "counts a registrant once per city even when listed there twice" do + it "counts a registrant once per city even when linked to that org twice" do breakdown = described_class.new( - city_registrant_pairs: [ [ "Barstow, CA", 10 ], [ "Barstow, CA", 10 ] ], + org_registrant_pairs: [ [ 1, 10 ], [ 1, 10 ] ], + city_by_org: { 1 => "Barstow, CA" }, scholarship_recipient_ids: [] ) expect(breakdown.rows.map(&:registrant_count)).to eq([ 1 ]) end - it "buckets a nil city under Unknown, sorted last" do + it "buckets linked orgs with no city under Unknown, sorted last" do breakdown = described_class.new( - city_registrant_pairs: [ [ "Perris, CA", 10 ], [ "Perris, CA", 11 ], [ nil, 12 ] ], + org_registrant_pairs: [ [ 1, 10 ], [ 1, 11 ], [ 2, 12 ] ], + city_by_org: { 1 => "Perris, CA" }, scholarship_recipient_ids: [] ) @@ -47,12 +51,13 @@ it "reports city_count and any?" do breakdown = described_class.new( - city_registrant_pairs: [ [ "Pomona, CA", 10 ], [ "Tustin, CA", 11 ] ], + org_registrant_pairs: [ [ 1, 10 ], [ 2, 11 ] ], + city_by_org: { 1 => "Pomona, CA", 2 => "Tustin, CA" }, scholarship_recipient_ids: [] ) expect(breakdown.city_count).to eq(2) expect(breakdown).to be_any - expect(described_class.new(city_registrant_pairs: [], scholarship_recipient_ids: [])).not_to be_any + expect(described_class.new(org_registrant_pairs: [], city_by_org: {}, scholarship_recipient_ids: [])).not_to be_any end end diff --git a/spec/requests/events/attendees_spec.rb b/spec/requests/events/attendees_spec.rb index 0c572602f7..828aa4f56c 100644 --- a/spec/requests/events/attendees_spec.rb +++ b/spec/requests/events/attendees_spec.rb @@ -130,7 +130,7 @@ get attendees_events_url(country: "Canada", state: "OR") # The chip labels the applied filter and its ✕ links back to the index # with only that param dropped (the other filters are preserved). - expect(response.body).to include("Org country: Canada") + expect(response.body).to include("Country: Canada") expect(Capybara.string(response.body)) .to have_link(href: attendees_events_path(state: "OR")) end @@ -341,22 +341,14 @@ expect(response.body).to include("Online Search") end - it "filters by work country and state (via the org they registered under)" do - org = create(:organization, name: "Toronto Org") - create(:address, addressable: org, state: "ON", country: "Canada", inactive: false) - attendee_registration.event_registration_organizations.create!(organization: org) + it "filters by a breakdown drill-in (country)" do + create(:address, addressable: attendee, country: "Canada", inactive: false) other = create(:person, first_name: "Zed", last_name: "Zulu") - create(:address, addressable: other, state: "OR", country: "Canada", inactive: false) create(:event_registration, event: recent_training, registrant: other, status: "attended") get attendees_events_url(country: "Canada"), headers: frame_headers expect(response.body).to include("Ada Lovelace") expect(response.body).not_to include("Zed Zulu") - - org.addresses.first.update!(state: "WA", country: "United States") - get attendees_events_url(state: "WA"), headers: frame_headers - expect(response.body).to include("Ada Lovelace") - expect(response.body).not_to include("Zed Zulu") end it "filters by a school-district drill-in (via the org they registered under)" do @@ -548,27 +540,23 @@ expect(response.body).to include("Ada Lovelace") end - it "filters by work city, not the person's own address" do - org = create(:organization) - create(:address, addressable: org, city: "Portland", state: "OR", inactive: false) - attendee_registration.event_registration_organizations.create!(organization: org) + it "filters by address data (city) without requiring a registration to carry it" do + create(:address, addressable: attendee, city: "Portland", state: "OR", inactive: false) other = create(:person, first_name: "Cara", last_name: "Coast") create(:event_registration, event: recent_training, registrant: other, status: "attended") - create(:address, addressable: other, city: "Portland", state: "OR", inactive: false) + create(:address, addressable: other, city: "Seattle", state: "WA", inactive: false) get attendees_events_url(city: "portland"), headers: frame_headers expect(response.body).to include("Ada Lovelace") expect(response.body).not_to include("Cara Coast") end - # City and state have to describe one address — otherwise someone working in - # two states matches a city from one and a state from the other. - it "matches city and state against the same work address" do - [ %w[Portland OR], %w[Seattle WA] ].each do |city, state| - org = create(:organization) - create(:address, addressable: org, city:, state:, inactive: false) - attendee_registration.event_registration_organizations.create!(organization: org) - end + # City and state have to describe one address, the way the roster's shared + # join does — otherwise someone with homes in two states matches a city from + # one and a state from the other. + it "matches city and state against the same address" do + create(:address, addressable: attendee, city: "Portland", state: "OR", inactive: false) + create(:address, addressable: attendee, city: "Seattle", state: "WA", inactive: false) get attendees_events_url(city: "Seattle", state: "WA"), headers: frame_headers expect(response.body).to include("Ada Lovelace") @@ -668,7 +656,7 @@ .at_css("turbo-stream[action='replace'][target='attendees_chips'] template") expect(template).to be_present - remove_link = "a[aria-label='Remove Org country: Canada filter']" + remove_link = "a[aria-label='Remove Country: Canada filter']" expect(Capybara.string(template.inner_html)).to have_selector( "#{remove_link}[href='#{attendees_events_path(ce_status: "issued")}']", visible: :all ) diff --git a/spec/requests/events/bulk_reminders_spec.rb b/spec/requests/events/bulk_reminders_spec.rb index 2d3074abaf..91219c8e92 100644 --- a/spec/requests/events/bulk_reminders_spec.rb +++ b/spec/requests/events/bulk_reminders_spec.rb @@ -47,11 +47,8 @@ def checked?(body, registration) expect(checked?(response.body, sam)).to be(false) end - it "filters recipients by the shared city search, on where they work" do - organization = create(:organization) - create(:address, addressable: organization, city: "Santa Monica") - jane.event_registration_organizations.create!(organization:) - create(:address, addressable: sam.registrant, city: "Santa Monica") + it "filters recipients by the shared city search" do + create(:address, addressable: jane.registrant, city: "Santa Monica") get preview_reminder_event_path(event, city: "santa"), headers: { "Turbo-Frame" => "reminder_recipients" } diff --git a/spec/requests/events_spec.rb b/spec/requests/events_spec.rb index 4a33d16042..a769b15bde 100644 --- a/spec/requests/events_spec.rb +++ b/spec/requests/events_spec.rb @@ -1779,13 +1779,12 @@ def pay_for_others_classes(template) expect(response.body).to include("Any status") end - it "narrows the roster by work city" do + it "narrows the roster by city" do here = create(:person, first_name: "Cityful", last_name: "Local") + create(:address, addressable: here, city: "Santa Monica") there = create(:person, first_name: "Faraway", last_name: "Person") - create(:address, addressable: there, city: "Santa Monica") - here_org = create(:organization) - create(:address, addressable: here_org, city: "Santa Monica") - create(:event_registration, event: event, registrant: here).event_registration_organizations.create!(organization: here_org) + create(:address, addressable: there, city: "Portland") + create(:event_registration, event: event, registrant: here) create(:event_registration, event: event, registrant: there) get registrants_event_path(event, city: "santa") @@ -2578,11 +2577,9 @@ def ce_chip_text let!(:ca_kings_reg) { create(:event_registration, event: event, registrant: ca_kings_person) } before do - { ca_reg => [ "CA", "Los Angeles" ], ny_reg => [ "NY", "Kings" ], ca_kings_reg => [ "CA", "Kings" ] }.each do |registration, (state, county)| - organization = create(:organization) - create(:address, addressable: organization, state:, county:) - registration.event_registration_organizations.create!(organization:) - end + create(:address, addressable: ca_person, state: "CA", county: "Los Angeles") + create(:address, addressable: ny_person, state: "NY", county: "Kings") + create(:address, addressable: ca_kings_person, state: "CA", county: "Kings") sign_in admin end @@ -3374,9 +3371,8 @@ def ce_chip_text expect(response.body).to include("Hospice care") end - it "shows a countries breakdown from the org a registrant registered under" do - create(:address, addressable: organization, state: "ON", country: "Canada", inactive: false) - registration.event_registration_organizations.create!(organization: organization) + it "shows a countries breakdown from registrant addresses" do + create(:address, addressable: person, state: "CA", country: "Canada", inactive: false) get roster_event_path(owned_event), headers: charts_headers @@ -3395,8 +3391,7 @@ def ce_chip_text end it "renders the states breakdown as a US choropleth map fed by per-state counts" do - create(:address, addressable: organization, state: "CA", inactive: false) - registration.event_registration_organizations.create!(organization: organization) + create(:address, addressable: person, state: "CA", inactive: false) get roster_event_path(owned_event), headers: charts_headers diff --git a/spec/services/attendees_active_filters_spec.rb b/spec/services/attendees_active_filters_spec.rb index babeb98101..1b267d5e2e 100644 --- a/spec/services/attendees_active_filters_spec.rb +++ b/spec/services/attendees_active_filters_spec.rb @@ -29,7 +29,7 @@ def chips_for(params) end it "labels the literal-value drill-ins" do - expect(chips_for(country: "Canada").first[:label]).to eq("Org country: Canada") + expect(chips_for(country: "Canada").first[:label]).to eq("Country: Canada") expect(chips_for(school_district: "PPS").first[:label]).to eq("School district: PPS") expect(chips_for(org_city: "Austin, TX").first[:label]).to eq("Org city: Austin, TX") end diff --git a/spec/services/attendees_breakdowns_spec.rb b/spec/services/attendees_breakdowns_spec.rb index e5f1719657..7888c260f9 100644 --- a/spec/services/attendees_breakdowns_spec.rb +++ b/spec/services/attendees_breakdowns_spec.rb @@ -19,14 +19,9 @@ expect(breakdowns.sector_counts).to eq(sector.id => 1) end - it "counts US states and countries from the work address of the org they registered under" do - create(:address, addressable: person, state: "NY", country: "Canada") - organization = create(:organization) - create(:address, addressable: organization, state: "CA", country: "United States") - registration.event_registration_organizations.create!(organization: organization) - + it "counts US states from active addresses" do + create(:address, addressable: person, state: "CA", inactive: false) expect(breakdowns.state_counts).to eq("CA" => 1) - expect(breakdowns.country_counts).to eq("United States" => 1) end it "aggregates organizations linked on the person's training registrations" do @@ -142,8 +137,8 @@ def answer(who, event, value) it "returns the people behind each dimension" do sector = create(:sector, name: "Healthcare") create(:sectorable_item, sectorable: person, sector: sector, is_primary: true) + create(:address, addressable: person, state: "CA", inactive: false) organization = create(:organization, name: "Wellness Org") - create(:address, addressable: organization, state: "CA", inactive: false) registration.event_registration_organizations.create!(organization: organization) scholarship = create(:scholarship, recipient: person, amount_cents: 1_000) create(:allocation, source: scholarship, allocatable: registration, amount: 1_000) diff --git a/spec/services/event_dashboard_spec.rb b/spec/services/event_dashboard_spec.rb index 0e3564a8e0..9d98f445f2 100644 --- a/spec/services/event_dashboard_spec.rb +++ b/spec/services/event_dashboard_spec.rb @@ -438,36 +438,37 @@ end end - describe "work locations" do - before do - reg1.event_registration_organizations.create!(organization: org_a) - reg2.event_registration_organizations.create!(organization: org_c) - cancelled_reg = event.event_registrations.find_by(registrant: cancelled_person) - cancelled_reg.event_registration_organizations.create!(organization: org_excluded) - create(:address, addressable: org_a, city: "Oakland", state: "OR", country: "United States") - chosen = create(:address, addressable: org_a, city: "Fresno", state: "CA", county: "Fresno", country: "United States") - person1.affiliations.find_by(organization: org_a).update!(organization_address: chosen, event_registration: reg1) - create(:address, addressable: org_c, city: "Albany", state: "NY", county: "Albany", country: "Mexico") - create(:address, addressable: org_excluded, city: "Miami", state: "FL", country: "Canada") + describe "states" do + it "returns unique states from active registrants' active addresses" do + expect(dashboard.states).to eq(%w[CA NY]) end - it "reads states from the affiliation's chosen org address, else the org's oldest" do - expect(dashboard.states).to eq(%w[CA NY]) + it "counts distinct registrants per state" do expect(dashboard.state_counts).to eq("CA" => 1, "NY" => 1) - expect(dashboard.state_registrant_ids_by_state).to eq("CA" => [ person1.id ], "NY" => [ person2.id ]) end - it "reads countries from the same work addresses, excluding cancelled registrations" do - expect(dashboard.country_counts).to eq("United States" => 1, "Mexico" => 1) + it "returns the registrant ids that have a state on file" do + expect(dashboard.state_registrant_ids).to contain_exactly(person1.id, person2.id) + end + end + + describe "counties" do + it "returns unique [ state, county ] pairs from active registrants' active addresses" do + expect(dashboard.counties).to eq([ [ "CA", "Los Angeles" ], [ "NY", "Kings" ] ]) + end + end + + describe "countries" do + it "returns the registrant ids that have a country on file, excluding inactive and cancelled" do expect(dashboard.country_registrant_ids).to contain_exactly(person1.id, person2.id) end - it "groups registrants by work city" do - expect(dashboard.registrant_city_breakdown.rows.map(&:city)).to contain_exactly("Fresno, CA", "Albany, NY") + it "counts only active registrants' active addresses" do + expect(dashboard.country_counts.values.sum).to eq(2) end - it "lists the [ state, county ] pairs registrants work in" do - expect(dashboard.counties).to eq([ [ "CA", "Fresno" ], [ "NY", "Albany" ] ]) + it "maps each country to its registrant ids" do + expect(dashboard.country_registrant_ids_by_country.values.flatten).to contain_exactly(person1.id, person2.id) end end diff --git a/spec/services/registration_work_addresses_spec.rb b/spec/services/registration_work_addresses_spec.rb deleted file mode 100644 index ae170ac343..0000000000 --- a/spec/services/registration_work_addresses_spec.rb +++ /dev/null @@ -1,45 +0,0 @@ -require "rails_helper" - -RSpec.describe RegistrationWorkAddresses do - let(:event) { create(:event) } - let(:person) { create(:person) } - let(:registration) { create(:event_registration, event: event, registrant: person) } - let(:organization) { create(:organization) } - - subject(:work_addresses) { described_class.new([ registration.id ]) } - - before { registration.event_registration_organizations.create!(organization: organization) } - - it "uses the address picked on the registration's affiliation" do - create(:address, addressable: organization, city: "Portland", state: "OR", country: "United States") - chosen = create(:address, addressable: organization, city: "Fresno", state: "CA", country: "United States") - create(:affiliation, person: person, organization: organization, organization_address: chosen, event_registration: registration) - - expect(work_addresses.rows.map(&:city_label)).to eq([ "Fresno, CA" ]) - end - - it "falls back to the org's oldest active address" do - create(:address, addressable: organization, city: "Old", state: "NV", inactive: true) - create(:address, addressable: organization, city: "Reno", state: "NV") - create(:address, addressable: organization, city: "Elko", state: "NV") - - expect(work_addresses.rows.map(&:city_label)).to eq([ "Reno, NV" ]) - end - - it "skips the registrant's own address" do - create(:address, addressable: person, city: "Boise", state: "ID") - - expect(work_addresses.rows.map(&:city_label)).to eq([ nil ]) - end - - it "groups registrant ids by US state, country, and city" do - create(:address, addressable: organization, city: "Austin", state: "TX", country: "United States") - abroad = create(:organization) - create(:address, addressable: abroad, city: "Toronto", state: "ON", country: "Canada") - registration.event_registration_organizations.create!(organization: abroad) - - expect(work_addresses.registrant_ids_by_state).to eq("TX" => [ person.id ]) - expect(work_addresses.registrant_ids_by_country).to eq("United States" => [ person.id ], "Canada" => [ person.id ]) - expect(work_addresses.registrant_ids_by_city_label.keys).to contain_exactly("Austin, TX", "Toronto, ON") - end -end diff --git a/spec/services/reminder_recipient_filter_spec.rb b/spec/services/reminder_recipient_filter_spec.rb index 0e5d8204d4..612b03fecc 100644 --- a/spec/services/reminder_recipient_filter_spec.rb +++ b/spec/services/reminder_recipient_filter_spec.rb @@ -95,15 +95,13 @@ def matched(params, registrations) expect(matched({ email: "amy@--aisha@" }, [ amy, aisha, sam ])).to eq([ amy.id, aisha.id ].to_set) end - it "filters by work city, like the roster does" do + it "filters by registrant city, skipping inactive addresses like the roster does" do here = registration - organization = create(:organization) - create(:address, addressable: organization, city: "Santa Monica") - here.event_registration_organizations.create!(organization:) - lives_there = registration(first_name: "Sam") - create(:address, addressable: lives_there.registrant, city: "Santa Monica") - expect(matched({ city: "santa" }, [ here, lives_there ])).to eq([ here.id ].to_set) - expect(EventRegistration.work_city("santa")).to contain_exactly(here) + create(:address, addressable: here.registrant, city: "Santa Monica") + moved_away = registration(first_name: "Sam") + create(:address, addressable: moved_away.registrant, city: "Santa Monica", inactive: true) + expect(matched({ city: "santa" }, [ here, moved_away ])).to eq([ here.id ].to_set) + expect(EventRegistration.registrant_city("santa")).not_to include(moved_away) end it "filters by registration comment text" do From cd111febf63bbd5bec08f44f49cb1f2973fad09d Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 08:06:48 -0400 Subject: [PATCH 6/6] Read the org fields in one query, and keep the coverage guard honest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Building the entry up front meant the form-params path looked up each org field on its own, so a form with no org section paid nine queries for answers it never had. One batched lookup covers them all. The identifier-coverage guard counted the appended constants too, which left it passing on a scan that had stopped matching most of the pipeline — it now counts only what it scanned. Co-Authored-By: Claude Opus 5 (1M context) --- .../capture_from_submission.rb | 15 +++++++-------- spec/services/smart_form_fields_spec.rb | 5 +++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/services/organization_services/capture_from_submission.rb b/app/services/organization_services/capture_from_submission.rb index eeb5969666..e5cc5f9bc0 100644 --- a/app/services/organization_services/capture_from_submission.rb +++ b/app/services/organization_services/capture_from_submission.rb @@ -84,17 +84,16 @@ def find_organization end def entry - @entry ||= FormSubmission.org_entry_from( - FormSubmission::ORG_ENTRY_FIELD_IDENTIFIERS.index_with { |identifier| field_value(identifier) } - ) + @entry ||= FormSubmission.org_entry_from(org_answers) end - def field_value(key) - return @answers[key] if @answers + def org_answers + identifiers = FormSubmission::ORG_ENTRY_FIELD_IDENTIFIERS + return @answers.slice(*identifiers) if @answers - field = @form.form_fields.find_by(field_identifier: key) - return nil unless field - @form_params[field.id.to_s] + @form.form_fields.where(field_identifier: identifiers) + .pluck(:field_identifier, :id) + .to_h { |identifier, id| [ identifier, @form_params[id.to_s] ] } end end end diff --git a/spec/services/smart_form_fields_spec.rb b/spec/services/smart_form_fields_spec.rb index 6413dbe518..427132d420 100644 --- a/spec/services/smart_form_fields_spec.rb +++ b/spec/services/smart_form_fields_spec.rb @@ -48,11 +48,12 @@ app/services/organization_services/capture_from_submission.rb app/services/public_form_submission.rb ].map { |path| Rails.root.join(path).read }.join("\n") - read_identifiers = (sources.scan(/field_value\("([a-z0-9_]+)"\)/).flatten + FormSubmission::ORG_ENTRY_FIELD_IDENTIFIERS).uniq + scanned = sources.scan(/field_value\("([a-z0-9_]+)"\)/).flatten.uniq + read_identifiers = (scanned + FormSubmission::ORG_ENTRY_FIELD_IDENTIFIERS).uniq documented = described_class.identifiers + described_class::ANSWER_ONLY_IDENTIFIERS # Guards the guard: a regex that stopped matching would pass vacuously. - expect(read_identifiers.size).to be > 20 + expect(scanned.size).to be > 15 expect(read_identifiers - documented).to be_empty end end