From a09b579537790155d81989658e43e908c4a5cba8 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 07:38:16 -0400 Subject: [PATCH 1/9] Chart and filter registrant locations by where they work Admins want the location charts and the State/County/City filters to show where registrants work, not where they live. Both now read the org linked on the registration, at the address picked on that registration's affiliation, else the org's oldest address (RegistrationWorkAddresses). Drill-ins follow so a clicked row lists the same people. Moved out of #2842 so it reviews separately from the org-answer refactor. 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, 351 insertions(+), 307 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 a2b06ae67b..331cd56b20 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"; `#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 - `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 +289,7 @@ comparison — add a rule if none fits (e.g. `PersonPolicy#own_profile?`). Conve - `DisplayImagePresenter` — Image display logic - `DeployedRevision` — The deployed commit from `GIT_SHA` (the DigitalOcean web service maps it to `${web.COMMIT_HASH}`); App settings shows a muted `build ` link to GitHub's compare view of everything merged to `main` since. In development it falls back to the checked-out commit; nil, and no link, when unset elsewhere - `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..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]) @@ -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,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: 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: 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" @@ -871,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 @@ -950,10 +946,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 +966,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 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 # Anchored the same way the index's own column is: this list spans events, so @@ -1031,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. @@ -1054,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/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/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..a2a836629e 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 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,11 @@ 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 "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..3434bb0760 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,34 @@ 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 state_registrant_ids_by_state + work_addresses.registrant_ids_by_state + end + + # The countries registrants work in (RegistrationWorkAddresses). def countries - @countries ||= Address - .active - .where(addressable_type: "Person", addressable_id: registrant_ids) - .where.not(country: [ nil, "" ]) - .distinct - .pluck(:country) - .sort + @countries ||= country_registrant_ids_by_country.keys.sort end - # Distinct registrant count per country. 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 +896,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 @@ -939,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 @@ -985,13 +947,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 +1296,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..bc65ec1f9a --- /dev/null +++ b/app/services/registration_work_addresses.rb @@ -0,0 +1,92 @@ +# 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 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 934d8d1a21..186206752a 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.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/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..a6864fb1b0 --- /dev/null +++ b/db/seeds/features/2026-10-25-registrant-charts-use-work-locations.yml @@ -0,0 +1,13 @@ +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 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/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..0c572602f7 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 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) 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 @@ -540,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") @@ -656,7 +668,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/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 a769b15bde..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 @@ -3371,8 +3374,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 +3395,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..babeb98101 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("Country: Canada") + expect(chips_for(country: "Canada").first[:label]).to eq("Org 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 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..0e3564a8e0 100644 --- a/spec/services/event_dashboard_spec.rb +++ b/spec/services/event_dashboard_spec.rb @@ -438,37 +438,36 @@ 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", 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") 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" ] ]) - end - end - - describe "countries" do - it "returns the registrant ids that have a country on file, excluding inactive and cancelled" do + 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 - it "counts only active registrants' active addresses" do - expect(dashboard.country_counts.values.sum).to eq(2) + it "groups registrants by work city" do + expect(dashboard.registrant_city_breakdown.rows.map(&:city)).to contain_exactly("Fresno, CA", "Albany, NY") 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) + 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/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 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 9569d450b06792ba02c93a52700194084780ca5c Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 07:40:46 -0400 Subject: [PATCH 2/9] Fall back to the registrant's own address when no work address is known A registrant with no linked org (or an org with no address) dropped out of the location filters entirely; counting them where they live keeps them findable. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 +- app/services/registration_work_addresses.rb | 49 +++++++++++++------ ...5-registrant-charts-use-work-locations.yml | 2 +- spec/models/event_registration_spec.rb | 7 +++ spec/requests/events/attendees_spec.rb | 7 ++- spec/requests/events/bulk_reminders_spec.rb | 3 ++ spec/requests/events_spec.rb | 4 +- .../registration_work_addresses_spec.rb | 26 +++++++++- .../reminder_recipient_filter_spec.rb | 18 ++++--- 9 files changed, 92 insertions(+), 26 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 331cd56b20..0ecc794d9d 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"; `#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 +- `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; a registration with no work address falls back to the registrant's own primary (else 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/services/registration_work_addresses.rb b/app/services/registration_work_addresses.rb index bc65ec1f9a..03d9e5e5b0 100644 --- a/app/services/registration_work_addresses.rb +++ b/app/services/registration_work_addresses.rb @@ -1,9 +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' 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. +# (Affiliation#organization_address), else that org's oldest active address. A +# registration with no work address falls back to the registrant's own primary +# (else oldest) active address. The source for the registrant charts' and +# filters' cities, counties, states, and countries. A linked org with no address +# and no own address to fall back on 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 @@ -16,11 +18,12 @@ def initialize(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) + @rows ||= registrations.flat_map do |registration_id, registrant_id| + work = organization_ids_by_registration.fetch(registration_id, []).map do |organization_id| + [ organization_id, chosen_addresses[chosen_address_ids[[ registration_id, organization_id ]]] || oldest_address_by_organization[organization_id] ] + end + work = [ [ nil, own_address_by_person[registrant_id] ] ] if work.none?(&:last) && own_address_by_person[registrant_id] + work.map { |organization_id, address| row(registration_id, organization_id, registrant_id, address) } end end @@ -63,11 +66,21 @@ 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) + def row(registration_id, organization_id, registrant_id, address) + Row.new(registration_id:, organization_id:, registrant_id:, + city: address&.city, county: address&.county, state: address&.state, country: address&.country) + end + + def registrations + @registrations ||= EventRegistration.where(id: @registration_ids).pluck(:id, :registrant_id) + end + + def organization_ids_by_registration + @organization_ids_by_registration ||= EventRegistrationOrganization .where(event_registration_id: @registration_ids) - .pluck(:event_registration_id, :organization_id, Arel.sql("event_registrations.registrant_id")) + .pluck(:event_registration_id, :organization_id) + .group_by(&:first) + .transform_values { |pairs| pairs.map(&:last) } end def chosen_address_ids @@ -84,9 +97,17 @@ def chosen_addresses def oldest_address_by_organization @oldest_address_by_organization ||= Address.active - .where(addressable_type: "Organization", addressable_id: links.map(&:second).uniq) + .where(addressable_type: "Organization", addressable_id: organization_ids_by_registration.values.flatten.uniq) .order(:id) .select(:id, :addressable_id, :city, :county, :state, :country) .each_with_object({}) { |address, map| map[address.addressable_id] ||= address } end + + def own_address_by_person + @own_address_by_person ||= Address.active + .where(addressable_type: "Person", addressable_id: registrations.map(&:last).uniq) + .order(primary: :desc, id: :asc) + .select(:id, :addressable_id, :city, :county, :state, :country) + .each_with_object({}) { |address, map| map[address.addressable_id] ||= address } + end end 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 a6864fb1b0..68220bb930 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 @@ -10,4 +10,4 @@ summary: >- 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." + - "A registrant with no organization address on their registration counts where they live instead." diff --git a/spec/models/event_registration_spec.rb b/spec/models/event_registration_spec.rb index 811bcdf6cb..2a73635967 100644 --- a/spec/models/event_registration_spec.rb +++ b/spec/models/event_registration_spec.rb @@ -618,6 +618,13 @@ def registration_working_in(city:, state:, county: nil) expect(EventRegistration.work_city("santa")).to contain_exactly(santa_monica) end + it "falls back to the registrant's own address when no org is linked" do + unlinked = create(:event_registration) + create(:address, addressable: unlinked.registrant, city: "Santa Barbara", state: "CA") + + expect(EventRegistration.work_city("santa")).to contain_exactly(santa_monica, unlinked) + end + it "matches the work state" do expect(EventRegistration.work_state("OR")).to contain_exactly(portland) end diff --git a/spec/requests/events/attendees_spec.rb b/spec/requests/events/attendees_spec.rb index 0c572602f7..80f6e9d904 100644 --- a/spec/requests/events/attendees_spec.rb +++ b/spec/requests/events/attendees_spec.rb @@ -346,7 +346,7 @@ 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(:address, addressable: other, state: "OR", country: "Mexico", inactive: false) create(:event_registration, event: recent_training, registrant: other, status: "attended") get attendees_events_url(country: "Canada"), headers: frame_headers @@ -553,8 +553,11 @@ 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") + other_registration = create(:event_registration, event: recent_training, registrant: other, status: "attended") create(:address, addressable: other, city: "Portland", state: "OR", inactive: false) + other_org = create(:organization) + create(:address, addressable: other_org, city: "Seattle", state: "WA", inactive: false) + other_registration.event_registration_organizations.create!(organization: other_org) get attendees_events_url(city: "portland"), 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 2d3074abaf..9bc9347dd0 100644 --- a/spec/requests/events/bulk_reminders_spec.rb +++ b/spec/requests/events/bulk_reminders_spec.rb @@ -51,6 +51,9 @@ def checked?(body, registration) organization = create(:organization) create(:address, addressable: organization, city: "Santa Monica") jane.event_registration_organizations.create!(organization:) + elsewhere = create(:organization) + create(:address, addressable: elsewhere, city: "Portland") + sam.event_registration_organizations.create!(organization: elsewhere) create(:address, addressable: sam.registrant, city: "Santa Monica") get preview_reminder_event_path(event, city: "santa"), diff --git a/spec/requests/events_spec.rb b/spec/requests/events_spec.rb index 4a33d16042..cd7631968e 100644 --- a/spec/requests/events_spec.rb +++ b/spec/requests/events_spec.rb @@ -1786,7 +1786,9 @@ def pay_for_others_classes(template) 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) + there_org = create(:organization) + create(:address, addressable: there_org, city: "Portland") + create(:event_registration, event: event, registrant: there).event_registration_organizations.create!(organization: there_org) get registrants_event_path(event, city: "santa") diff --git a/spec/services/registration_work_addresses_spec.rb b/spec/services/registration_work_addresses_spec.rb index ae170ac343..747a842986 100644 --- a/spec/services/registration_work_addresses_spec.rb +++ b/spec/services/registration_work_addresses_spec.rb @@ -26,12 +26,36 @@ expect(work_addresses.rows.map(&:city_label)).to eq([ "Reno, NV" ]) end - it "skips the registrant's own address" do + it "prefers the work address over the registrant's own" do + create(:address, addressable: organization, city: "Reno", state: "NV") create(:address, addressable: person, city: "Boise", state: "ID") + expect(work_addresses.rows.map(&:city_label)).to eq([ "Reno, NV" ]) + end + + it "falls back to the registrant's primary address when the linked org has none" do + create(:address, addressable: person, city: "Boise", state: "ID") + create(:address, addressable: person, city: "Nampa", state: "ID", primary: true) + + expect(work_addresses.rows.map(&:city_label)).to eq([ "Nampa, ID" ]) + end + + it "falls back to the registrant's own address when no org is linked" do + unlinked = create(:event_registration, event: event) + create(:address, addressable: unlinked.registrant, city: "Boise", state: "ID") + + rows = described_class.new([ unlinked.id ]).rows + expect(rows.map { |row| [ row.city_label, row.organization_id ] }).to eq([ [ "Boise, ID", nil ] ]) + end + + it "leaves a located-nowhere linked registration as an Unknown row" do expect(work_addresses.rows.map(&:city_label)).to eq([ nil ]) end + it "has no row for an unlinked registrant with no address" do + expect(described_class.new([ create(:event_registration, event: event).id ]).rows).to be_empty + 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) diff --git a/spec/services/reminder_recipient_filter_spec.rb b/spec/services/reminder_recipient_filter_spec.rb index 0e5d8204d4..1055e1b5f1 100644 --- a/spec/services/reminder_recipient_filter_spec.rb +++ b/spec/services/reminder_recipient_filter_spec.rb @@ -95,15 +95,21 @@ 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 - here = registration + it "filters by work city, else the registrant's own, like the roster does" do + works_there = registration organization = create(:organization) create(:address, addressable: organization, city: "Santa Monica") - here.event_registration_organizations.create!(organization:) - lives_there = registration(first_name: "Sam") + works_there.event_registration_organizations.create!(organization:) + works_elsewhere = registration(first_name: "Sam") + elsewhere = create(:organization) + create(:address, addressable: elsewhere, city: "Portland") + works_elsewhere.event_registration_organizations.create!(organization: elsewhere) + create(:address, addressable: works_elsewhere.registrant, city: "Santa Monica") + lives_there = registration(first_name: "Lia") 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) + + expect(matched({ city: "santa" }, [ works_there, works_elsewhere, lives_there ])).to eq([ works_there.id, lives_there.id ].to_set) + expect(EventRegistration.work_city("santa")).to contain_exactly(works_there, lives_there) end it "filters by registration comment text" do From 70d47b561846389e288f4d1ed34cd2d8d1525681 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 07:41:16 -0400 Subject: [PATCH 3/9] Point the work-locations feature entry at its own PR Co-Authored-By: Claude Opus 5.5 (1M context) --- .../2026-10-25-registrant-charts-use-work-locations.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 68220bb930..eddd59f558 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 @@ -2,7 +2,7 @@ name: "Registrant charts and location filters use where people work" area: reporting display_status: admin_facing released_on: 2026-10-25 -pr_number: 2842 +pr_number: 2844 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 From decf60badc2fafa214a4bf9cd195a5872afa15d6 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 07:44:03 -0400 Subject: [PATCH 4/9] Name registrant locations for what they are, not just work With the fallback to a registrant's own address, "work" no longer describes the whole rule: RegistrationWorkAddresses -> RegistrationLocations and work_* scopes -> located_in_*. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 6 ++-- app/controllers/events_controller.rb | 30 +++++++++---------- app/models/event_registration.rb | 14 ++++----- app/presenters/registrant_city_breakdown.rb | 2 +- app/services/attendees_breakdowns.rb | 14 ++++----- app/services/event_dashboard.rb | 20 ++++++------- ...addresses.rb => registration_locations.rb} | 18 +++++------ app/services/reminder_recipient_filter.rb | 12 ++++---- spec/models/event_registration_spec.rb | 14 ++++----- ...spec.rb => registration_locations_spec.rb} | 20 ++++++------- .../reminder_recipient_filter_spec.rb | 2 +- 11 files changed, 76 insertions(+), 76 deletions(-) rename app/services/{registration_work_addresses.rb => registration_locations.rb} (84%) rename spec/services/{registration_work_addresses_spec.rb => registration_locations_spec.rb} (76%) diff --git a/AGENTS.md b/AGENTS.md index 0ecc794d9d..3f2165f87e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -236,8 +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 -- `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; a registration with no work address falls back to the registrant's own primary (else 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 +- `RegistrationLocations` — Where each registrant is, per registration — where they work, else where they live: 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; a registration with no work address falls back to the registrant's own primary (else 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.located_in_state`/`located_in_county`/`located_in_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 `RegistrationLocations` — 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 @@ -289,7 +289,7 @@ comparison — add a rule if none fits (e.g. `PersonPolicy#own_profile?`). Conve - `DisplayImagePresenter` — Image display logic - `DeployedRevision` — The deployed commit from `GIT_SHA` (the DigitalOcean web service maps it to `${web.COMMIT_HASH}`); App settings shows a muted `build ` link to GitHub's compare view of everything merged to `main` since. In development it falls back to the checked-out commit; nil, and no link, when unset elsewhere - `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 their location's city (`RegistrationLocations` — work, else home), 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..92a6cc8307 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.located_in_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.located_in_state(params[:state]) if params[:state].present? + scope = scope.located_in_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]) @@ -853,11 +853,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: attendee_locations.registrant_ids_by_country.fetch(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: attendee_locations.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" @@ -869,17 +869,17 @@ 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 + # Address filters match where people are (RegistrationLocations) on their in-scope + # registrations. City and state test one address, so someone 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: attendee_work_addresses.matching(state: params[:state], city: params[:city]).map(&:registrant_id)) + scope = scope.where(id: attendee_locations.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: attendee_work_addresses.matching(state: county_state, county: county_name).map(&:registrant_id)) + scope = scope.where(id: attendee_locations.matching(state: county_state, county: county_name).map(&:registrant_id)) end # Shared registration-level filters, applied cross-event with "any registration @@ -966,10 +966,10 @@ 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 + # Where each in-scope registration's registrant is, 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)) + def attendee_locations + @attendee_locations ||= RegistrationLocations.new(attendee_registrations.pluck(:id)) end # Anchored the same way the index's own column is: this list spans events, so @@ -1015,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) - 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 + locations = RegistrationLocations.new(EventRegistration.where(event_id: reportable_events.select(:id)).pluck(:id)) + @attendee_states = locations.rows.filter_map(&:state).compact_blank.uniq.sort + @attendee_counties = locations.counties @attendee_topic_types = TopicSubscriptionType.active.ordered end diff --git a/app/models/event_registration.rb b/app/models/event_registration.rb index 0240a3a7d0..ee65371718 100644 --- a/app/models/event_registration.rb +++ b/app/models/event_registration.rb @@ -220,20 +220,20 @@ 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)) + # Where the registrant is on the registration — work, else home (RegistrationLocations). + scope :located_in_state, ->(state) { + where(id: RegistrationLocations.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 :work_county, ->(value) { + scope :located_in_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)) + where(id: RegistrationLocations.new(pluck(:id)).matching(state:, county:).map(&:registration_id)) } - scope :work_city, ->(term) { + scope :located_in_city, ->(term) { next all if term.blank? - where(id: RegistrationWorkAddresses.new(pluck(:id)).matching(city: term).map(&:registration_id)) + where(id: RegistrationLocations.new(pluck(:id)).matching(city: term).map(&:registration_id)) } 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..a24630a7f8 100644 --- a/app/presenters/registrant_city_breakdown.rb +++ b/app/presenters/registrant_city_breakdown.rb @@ -1,4 +1,4 @@ -# Groups an event's registrants by the city they work in (RegistrationWorkAddresses), +# Groups an event's registrants by their location's city (RegistrationLocations), # 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]]). diff --git a/app/services/attendees_breakdowns.rb b/app/services/attendees_breakdowns.rb index 9931451365..58d30f75d7 100644 --- a/app/services/attendees_breakdowns.rb +++ b/app/services/attendees_breakdowns.rb @@ -4,7 +4,7 @@ # 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. +# orgs, locations, 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,12 +140,12 @@ def ce_registrant_ids # --- Cities ---------------------------------------------------------------- - # Attendees grouped by the city they work in on their attended-training + # Attendees grouped by their location's city 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 ] }, + city_registrant_pairs: locations.rows.map { |row| [ row.city_label, row.registrant_id ] }, scholarship_recipient_ids: scholarship_recipient_ids ) end @@ -180,11 +180,11 @@ def life_experience_registrant_ids_by_category end def state_registrant_ids_by_state - work_addresses.registrant_ids_by_state + locations.registrant_ids_by_state end def country_registrant_ids_by_country - work_addresses.registrant_ids_by_country + locations.registrant_ids_by_country end def school_district_registrant_ids_by_district @@ -294,8 +294,8 @@ 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 locations + @locations ||= RegistrationLocations.new(registration_ids) end def org_registrant_pairs diff --git a/app/services/event_dashboard.rb b/app/services/event_dashboard.rb index 3434bb0760..cf5fa42961 100644 --- a/app/services/event_dashboard.rb +++ b/app/services/event_dashboard.rb @@ -147,12 +147,12 @@ 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 + # This event's registrants grouped by their location's city, 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( - city_registrant_pairs: work_addresses.rows.map { |row| [ row.city_label, row.registrant_id ] }, + city_registrant_pairs: locations.rows.map { |row| [ row.city_label, row.registrant_id ] }, scholarship_recipient_ids: scholarships.distinct.pluck(:recipient_id) ) end @@ -850,7 +850,7 @@ def referral_source_counts .sort_by { |label, count| [ -count, label ] } end - # The US states registrants work in (RegistrationWorkAddresses). + # The US states registrants are in — work, else home (RegistrationLocations). def states @states ||= state_registrant_ids_by_state.keys.sort end @@ -864,10 +864,10 @@ def state_registrant_ids end def state_registrant_ids_by_state - work_addresses.registrant_ids_by_state + locations.registrant_ids_by_state end - # The countries registrants work in (RegistrationWorkAddresses). + # The countries registrants are in (RegistrationLocations). def countries @countries ||= country_registrant_ids_by_country.keys.sort end @@ -897,7 +897,7 @@ def school_district_registrant_ids end def country_registrant_ids_by_country - work_addresses.registrant_ids_by_country + locations.registrant_ids_by_country end # Registrant ids per school district, for the breakdown's per-row drill-in. A @@ -910,10 +910,10 @@ def school_district_registrant_ids_by_district .transform_values(&:to_a) end - # [ state, county ] pairs registrants work in. Pairing in the state lets the + # [ state, county ] pairs registrants are in. Pairing in the state lets the # manage filter disambiguate same-named counties ("CA - Warren" vs "NY - Warren"). def counties - work_addresses.counties + locations.counties end # Life experiences captured at registration: StoryPopulation categories tagged @@ -947,8 +947,8 @@ def life_experience_registrant_ids_by_category private - def work_addresses - @work_addresses ||= RegistrationWorkAddresses.new(active_registration_ids) + def locations + @locations ||= RegistrationLocations.new(active_registration_ids) end # [ [ district, registrant_id ], ... ] from the affiliations linked to this event's diff --git a/app/services/registration_work_addresses.rb b/app/services/registration_locations.rb similarity index 84% rename from app/services/registration_work_addresses.rb rename to app/services/registration_locations.rb index 03d9e5e5b0..a4432e582a 100644 --- a/app/services/registration_work_addresses.rb +++ b/app/services/registration_locations.rb @@ -1,12 +1,12 @@ -# 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. A -# registration with no work address falls back to the registrant's own primary -# (else oldest) active address. The source for the registrant charts' and +# Where each registrant is, per registration — where they work, else where they +# live: 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; with no such address, the registrant's own +# primary (else oldest) active address. The source for the registrant charts' and # filters' cities, counties, states, and countries. A linked org with no address # and no own address to fall back on yields a row with no location, so the # cities breakdown can count it as Unknown. -class RegistrationWorkAddresses +class RegistrationLocations 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 @@ -19,11 +19,11 @@ def initialize(registration_ids) def rows @rows ||= registrations.flat_map do |registration_id, registrant_id| - work = organization_ids_by_registration.fetch(registration_id, []).map do |organization_id| + located = organization_ids_by_registration.fetch(registration_id, []).map do |organization_id| [ organization_id, chosen_addresses[chosen_address_ids[[ registration_id, organization_id ]]] || oldest_address_by_organization[organization_id] ] end - work = [ [ nil, own_address_by_person[registrant_id] ] ] if work.none?(&:last) && own_address_by_person[registrant_id] - work.map { |organization_id, address| row(registration_id, organization_id, registrant_id, address) } + located = [ [ nil, own_address_by_person[registrant_id] ] ] if located.none?(&:last) && own_address_by_person[registrant_id] + located.map { |organization_id, address| row(registration_id, organization_id, registrant_id, address) } end end diff --git a/app/services/reminder_recipient_filter.rb b/app/services/reminder_recipient_filter.rb index b4282b1b4e..a0dedda264 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.located_in_state(@params[:state]) if @params[:state].present? + scope = scope.located_in_county(@params[:county]) if @params[:county].present? scope.pluck(:id).to_set end @@ -127,16 +127,16 @@ def matches_comment?(reg) end end - # Matches the work city, like EventRegistration.work_city, so the same City + # Matches the registrant's location, like EventRegistration.located_in_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) } + locations.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)) + def locations + @locations ||= RegistrationLocations.new(@event_registrations.map(&:id)) end # Matches against every email we hold for the registrant (their account email, diff --git a/spec/models/event_registration_spec.rb b/spec/models/event_registration_spec.rb index 2a73635967..525d23037a 100644 --- a/spec/models/event_registration_spec.rb +++ b/spec/models/event_registration_spec.rb @@ -600,7 +600,7 @@ def registration_with_scholarship end end - describe "work location scopes" do + describe "location scopes" do def registration_working_in(city:, state:, county: nil) registration = create(:event_registration) organization = create(:organization) @@ -612,26 +612,26 @@ def registration_working_in(city:, state:, county: nil) 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 + it "matches the work city (case-insensitive, partial) over the registrant's own address" do create(:address, addressable: portland.registrant, city: "Santa Monica") - expect(EventRegistration.work_city("santa")).to contain_exactly(santa_monica) + expect(EventRegistration.located_in_city("santa")).to contain_exactly(santa_monica) end it "falls back to the registrant's own address when no org is linked" do unlinked = create(:event_registration) create(:address, addressable: unlinked.registrant, city: "Santa Barbara", state: "CA") - expect(EventRegistration.work_city("santa")).to contain_exactly(santa_monica, unlinked) + expect(EventRegistration.located_in_city("santa")).to contain_exactly(santa_monica, unlinked) end it "matches the work state" do - expect(EventRegistration.work_state("OR")).to contain_exactly(portland) + expect(EventRegistration.located_in_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 + expect(EventRegistration.located_in_county("CA|Los Angeles")).to contain_exactly(santa_monica) + expect(EventRegistration.located_in_county("OR|Los Angeles")).to be_empty end end diff --git a/spec/services/registration_work_addresses_spec.rb b/spec/services/registration_locations_spec.rb similarity index 76% rename from spec/services/registration_work_addresses_spec.rb rename to spec/services/registration_locations_spec.rb index 747a842986..aff75e7aef 100644 --- a/spec/services/registration_work_addresses_spec.rb +++ b/spec/services/registration_locations_spec.rb @@ -1,12 +1,12 @@ require "rails_helper" -RSpec.describe RegistrationWorkAddresses do +RSpec.describe RegistrationLocations 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 ]) } + subject(:locations) { described_class.new([ registration.id ]) } before { registration.event_registration_organizations.create!(organization: organization) } @@ -15,7 +15,7 @@ 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" ]) + expect(locations.rows.map(&:city_label)).to eq([ "Fresno, CA" ]) end it "falls back to the org's oldest active address" do @@ -23,21 +23,21 @@ 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" ]) + expect(locations.rows.map(&:city_label)).to eq([ "Reno, NV" ]) end it "prefers the work address over the registrant's own" do create(:address, addressable: organization, city: "Reno", state: "NV") create(:address, addressable: person, city: "Boise", state: "ID") - expect(work_addresses.rows.map(&:city_label)).to eq([ "Reno, NV" ]) + expect(locations.rows.map(&:city_label)).to eq([ "Reno, NV" ]) end it "falls back to the registrant's primary address when the linked org has none" do create(:address, addressable: person, city: "Boise", state: "ID") create(:address, addressable: person, city: "Nampa", state: "ID", primary: true) - expect(work_addresses.rows.map(&:city_label)).to eq([ "Nampa, ID" ]) + expect(locations.rows.map(&:city_label)).to eq([ "Nampa, ID" ]) end it "falls back to the registrant's own address when no org is linked" do @@ -49,7 +49,7 @@ end it "leaves a located-nowhere linked registration as an Unknown row" do - expect(work_addresses.rows.map(&:city_label)).to eq([ nil ]) + expect(locations.rows.map(&:city_label)).to eq([ nil ]) end it "has no row for an unlinked registrant with no address" do @@ -62,8 +62,8 @@ 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") + expect(locations.registrant_ids_by_state).to eq("TX" => [ person.id ]) + expect(locations.registrant_ids_by_country).to eq("United States" => [ person.id ], "Canada" => [ person.id ]) + expect(locations.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 1055e1b5f1..13ff77afa6 100644 --- a/spec/services/reminder_recipient_filter_spec.rb +++ b/spec/services/reminder_recipient_filter_spec.rb @@ -109,7 +109,7 @@ def matched(params, registrations) create(:address, addressable: lives_there.registrant, city: "Santa Monica") expect(matched({ city: "santa" }, [ works_there, works_elsewhere, lives_there ])).to eq([ works_there.id, lives_there.id ].to_set) - expect(EventRegistration.work_city("santa")).to contain_exactly(works_there, lives_there) + expect(EventRegistration.located_in_city("santa")).to contain_exactly(works_there, lives_there) end it "filters by registration comment text" do From 43585dc51be23588a8f25cb5f7b08b4febc5b31c Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 08:03:19 -0400 Subject: [PATCH 5/9] Keep the location filters on one address, and off every id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three chained location scopes each tested any of a registration's addresses, so a registrant working in two states matched a city from one and the state from the other — the shared addresses join used to rule that out. One `located_in(state:, county:, city:)` call makes the invariant structural, and the reminder picker's City search now runs through it instead of matching rows the State and County dropdowns had already filtered separately. RegistrationLocations also takes a relation now. The attendees index built its filter options by plucking every reportable registration id and feeding the array back as IN lists across five queries; as subqueries that stays one statement no matter how large the population. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 2 +- app/controllers/events_controller.rb | 8 +-- app/models/event_registration.rb | 29 ++++----- app/services/attendees_breakdowns.rb | 10 ++-- app/services/event_dashboard.rb | 2 +- app/services/registration_locations.rb | 37 ++++++++---- app/services/reminder_recipient_filter.rb | 60 ++++++++++++------- ...5-registrant-charts-use-work-locations.yml | 1 + spec/models/event_registration_spec.rb | 33 ++++++++-- spec/requests/events_spec.rb | 19 ++++++ spec/services/registration_locations_spec.rb | 16 +++++ .../reminder_recipient_filter_spec.rb | 17 +++++- 12 files changed, 171 insertions(+), 63 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 3f2165f87e..6fa36ec6b3 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 -- `RegistrationLocations` — Where each registrant is, per registration — where they work, else where they live: 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; a registration with no work address falls back to the registrant's own primary (else 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.located_in_state`/`located_in_county`/`located_in_city`, `ReminderRecipientFilter`), and the attendees index's State/County/City filters and `country`/`org_city` drill-ins +- `RegistrationLocations` — Where each registrant is, per registration — where they work, else where they live: 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; a registration with no work address falls back to the registrant's own primary (else oldest) active address. Groups registrant ids by US state, country, and "City, State"; `#matching(state:, county:, city:)` tests one address per row. Takes a relation (kept as a subquery, so a whole-population lookup never inlines every registration id) or an explicit id array. 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.located_in`, which takes all three at once so they test the same address, and `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 `RegistrationLocations` — 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 92a6cc8307..2630e04468 100644 --- a/app/controllers/events_controller.rb +++ b/app/controllers/events_controller.rb @@ -244,12 +244,10 @@ 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.located_in_city(params[:city]) if params[:city].present? + scope = scope.located_in(state: params[:state], county: params[:county], city: params[:city]) 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.located_in_state(params[:state]) if params[:state].present? - scope = scope.located_in_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]) @@ -969,7 +967,7 @@ def person_linked_organization_ids(organization_id) # Where each in-scope registration's registrant is, behind the location filters # and drill-ins — the same source as the breakdowns they come from. def attendee_locations - @attendee_locations ||= RegistrationLocations.new(attendee_registrations.pluck(:id)) + @attendee_locations ||= RegistrationLocations.new(attendee_registrations) end # Anchored the same way the index's own column is: this list spans events, so @@ -1015,7 +1013,7 @@ 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) - locations = RegistrationLocations.new(EventRegistration.where(event_id: reportable_events.select(:id)).pluck(:id)) + locations = RegistrationLocations.new(EventRegistration.where(event_id: reportable_events.select(:id))) @attendee_states = locations.rows.filter_map(&:state).compact_blank.uniq.sort @attendee_counties = locations.counties @attendee_topic_types = TopicSubscriptionType.active.ordered diff --git a/app/models/event_registration.rb b/app/models/event_registration.rb index ee65371718..a7b2b760d0 100644 --- a/app/models/event_registration.rb +++ b/app/models/event_registration.rb @@ -220,20 +220,21 @@ class EventRegistration < ApplicationRecord end scope } - # Where the registrant is on the registration — work, else home (RegistrationLocations). - scope :located_in_state, ->(state) { - where(id: RegistrationLocations.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 :located_in_county, ->(value) { - state, county = value.to_s.include?("|") ? value.split("|", 2) : [ nil, value.to_s ] - next none if county.blank? - where(id: RegistrationLocations.new(pluck(:id)).matching(state:, county:).map(&:registration_id)) - } - scope :located_in_city, ->(term) { - next all if term.blank? - where(id: RegistrationLocations.new(pluck(:id)).matching(city: term).map(&:registration_id)) + # Where the registrant is on the registration — work, else home + # (RegistrationLocations). City, state and county arrive together in one call so + # they test the same location row: a registrant working in two states would + # otherwise match a city from one and the state from the other. County accepts + # "STATE|County" (from the roster filter) to scope to a county within a specific + # state, or a bare county name for backward compatibility; a county state that + # contradicts the State filter can match nothing. + scope :located_in, ->(state: nil, county: nil, city: nil) { + county_state, county_name = county.to_s.include?("|") ? county.split("|", 2) : [ nil, county.to_s ] + next none if county.present? && county_name.blank? + states = [ state, county_state ].compact_blank.uniq + next none if states.many? + next all if states.empty? && county_name.blank? && city.blank? + matches = RegistrationLocations.new(all).matching(state: states.first, county: county_name.presence, city: city.presence) + where(id: matches.map(&:registration_id)) } scope :registrant_sector, ->(sector_id) { joins(registrant: :sectorable_items) diff --git a/app/services/attendees_breakdowns.rb b/app/services/attendees_breakdowns.rb index 58d30f75d7..ce29faeac6 100644 --- a/app/services/attendees_breakdowns.rb +++ b/app/services/attendees_breakdowns.rb @@ -240,10 +240,12 @@ def person_ids @person_ids ||= @people.except(:includes, :eager_load, :preload, :order).ids end + def scoped_registrations + @scoped_registrations ||= @registrations.where(registrant_id: person_ids, event_id: @events.select(:id)) + end + def registration_ids - @registration_ids ||= @registrations - .where(registrant_id: person_ids, event_id: @events.select(:id)) - .pluck(:id) + @registration_ids ||= scoped_registrations.pluck(:id) end def sector_rows @@ -295,7 +297,7 @@ def person_ids_by(rows) end def locations - @locations ||= RegistrationLocations.new(registration_ids) + @locations ||= RegistrationLocations.new(scoped_registrations) end def org_registrant_pairs diff --git a/app/services/event_dashboard.rb b/app/services/event_dashboard.rb index cf5fa42961..1551494211 100644 --- a/app/services/event_dashboard.rb +++ b/app/services/event_dashboard.rb @@ -948,7 +948,7 @@ def life_experience_registrant_ids_by_category private def locations - @locations ||= RegistrationLocations.new(active_registration_ids) + @locations ||= RegistrationLocations.new(active_registrations) end # [ [ district, registrant_id ], ... ] from the affiliations linked to this event's diff --git a/app/services/registration_locations.rb b/app/services/registration_locations.rb index a4432e582a..0d9688649b 100644 --- a/app/services/registration_locations.rb +++ b/app/services/registration_locations.rb @@ -13,12 +13,18 @@ def city_label end end - def initialize(registration_ids) - @registration_ids = registration_ids + # Takes the registrations to locate, as a relation (kept as a subquery, so a + # whole-population lookup never inlines every id) or as an explicit id array. + def initialize(registrations) + @registrations = if registrations.is_a?(ActiveRecord::Relation) + registrations.except(:includes, :eager_load, :preload, :order, :select, :limit, :offset) + else + EventRegistration.where(id: registrations) + end end def rows - @rows ||= registrations.flat_map do |registration_id, registrant_id| + @rows ||= registration_pairs.flat_map do |registration_id, registrant_id| located = organization_ids_by_registration.fetch(registration_id, []).map do |organization_id| [ organization_id, chosen_addresses[chosen_address_ids[[ registration_id, organization_id ]]] || oldest_address_by_organization[organization_id] ] end @@ -71,13 +77,17 @@ def row(registration_id, organization_id, registrant_id, address) city: address&.city, county: address&.county, state: address&.state, country: address&.country) end - def registrations - @registrations ||= EventRegistration.where(id: @registration_ids).pluck(:id, :registrant_id) + def registration_pairs + @registration_pairs ||= @registrations.pluck(:id, :registrant_id) + end + + def registration_ids + @registrations.select(:id) end def organization_ids_by_registration @organization_ids_by_registration ||= EventRegistrationOrganization - .where(event_registration_id: @registration_ids) + .where(event_registration_id: registration_ids) .pluck(:event_registration_id, :organization_id) .group_by(&:first) .transform_values { |pairs| pairs.map(&:last) } @@ -85,19 +95,26 @@ def organization_ids_by_registration def chosen_address_ids @chosen_address_ids ||= Affiliation - .where(event_registration_id: @registration_ids) + .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) + @chosen_addresses ||= Address.active + .where(id: Affiliation.where(event_registration_id: registration_ids).select(:organization_address_id)) + .select(:id, :city, :county, :state, :country) + .index_by(&:id) + end + + def linked_organization_ids + EventRegistrationOrganization.where(event_registration_id: registration_ids).select(:organization_id) end def oldest_address_by_organization @oldest_address_by_organization ||= Address.active - .where(addressable_type: "Organization", addressable_id: organization_ids_by_registration.values.flatten.uniq) + .where(addressable_type: "Organization", addressable_id: linked_organization_ids) .order(:id) .select(:id, :addressable_id, :city, :county, :state, :country) .each_with_object({}) { |address, map| map[address.addressable_id] ||= address } @@ -105,7 +122,7 @@ def oldest_address_by_organization def own_address_by_person @own_address_by_person ||= Address.active - .where(addressable_type: "Person", addressable_id: registrations.map(&:last).uniq) + .where(addressable_type: "Person", addressable_id: @registrations.select(:registrant_id)) .order(primary: :desc, id: :asc) .select(:id, :addressable_id, :city, :county, :state, :country) .each_with_object({}) { |address, map| map[address.addressable_id] ||= address } diff --git a/app/services/reminder_recipient_filter.rb b/app/services/reminder_recipient_filter.rb index a0dedda264..ca33bafb95 100644 --- a/app/services/reminder_recipient_filter.rb +++ b/app/services/reminder_recipient_filter.rb @@ -10,7 +10,7 @@ class ReminderRecipientFilter # `funder_name` is a free-text search on the scholarship funder's name — kept # distinct from the awbw/external funding-source `funder` scope so the shared # roster/picker filter bar can't collide the two on the roster. - TEXT_KEYS = %i[ name reg_org funder_name comment email city ].freeze + TEXT_KEYS = %i[ name reg_org funder_name comment email ].freeze # Dropdown filters shared with the registrants roster. They reuse the # EventRegistration scopes (run once as a query) so both pages stay in sync — # same param names, options, and semantics. @@ -22,11 +22,14 @@ class ReminderRecipientFilter # keeps registrants whose person holds an active subscription to any of the chosen # topic subscription lists; matched in memory against the registrant's topic_subscriptions. PICKER_KEYS = %i[ topic_subscription ].freeze + # The City search plus the State and County dropdowns. Resolved as one group so + # they describe the same address, which is why city isn't a TEXT_KEY. + LOCATION_KEYS = %i[ city state county ].freeze # Registration date-range filters shared with the roster. Backed by the # EventRegistration.registered_between scope (run once as a query), so the same # range narrows both pages identically. DATE_KEYS = %i[ registered_from registered_to ].freeze - FILTER_KEYS = (TEXT_KEYS + DROPDOWN_KEYS + PICKER_KEYS + DATE_KEYS).freeze + FILTER_KEYS = (TEXT_KEYS + DROPDOWN_KEYS + PICKER_KEYS + LOCATION_KEYS + DATE_KEYS).uniq.freeze # Every key above that is also a registrants-roster param, so the roster's # "Send bulk emails" link can carry the active filters straight into the picker # (see EventHelper#reminder_recipient_filters). The remaining text keys (name, @@ -41,7 +44,7 @@ def initialize(event_registrations, params, event: nil) def matched_ids text_matched = @event_registrations.select { |reg| matches_text?(reg) }.map(&:id).to_set - text_matched & dropdown_matched_ids & topic_matched_ids + text_matched & dropdown_matched_ids & location_matched_ids & topic_matched_ids end # True when at least one filter is narrowing the list. The view uses this to @@ -58,15 +61,14 @@ def matches_text?(reg) matches_reg_org?(reg) && matches_funder?(reg) && matches_comment?(reg) && - matches_email?(reg) && - matches_city?(reg) + matches_email?(reg) end # Apply the registrants-roster scopes for whichever dropdowns / date-range # filters are set, then return the matching ids. With none set this is every # registration, so the text-match set passes through unchanged. def dropdown_matched_ids - return @event_registrations.map(&:id).to_set if @event.nil? + return all_registration_ids if @event.nil? scope = @event.event_registrations scope = scope.registered_between(@params[:registered_from], @params[:registered_to]) @@ -79,17 +81,35 @@ 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.located_in_state(@params[:state]) if @params[:state].present? - scope = scope.located_in_county(@params[:county]) if @params[:county].present? scope.pluck(:id).to_set end + # The City search and the State / County dropdowns, resolved together through + # EventRegistration.located_in so the same values narrow the roster and the picker + # identically — and so a registrant working in two states can't match a city from + # one and the state from the other. City is multi-term, so each term asks for the + # rows that match it *and* the dropdowns, then the terms are unioned. + def location_matched_ids + return all_registration_ids if LOCATION_KEYS.none? { |key| @params[key].present? } + + terms = terms_for(:city) + return located_in_ids(nil) if terms.empty? + terms.reduce(Set.new) { |ids, term| ids | located_in_ids(term) } + end + + def located_in_ids(city) + EventRegistration.where(id: all_registration_ids.to_a) + .located_in(state: @params[:state], county: @params[:county], city: city) + .pluck(:id) + .to_set + end + # Keeps registrants whose person holds an active subscription to any of the chosen # topic subscription lists. With no topic set this is every registration, so it # passes the other matches through unchanged. def topic_matched_ids type_ids = Array(@params[:topic_subscription]).reject(&:blank?).map(&:to_i).to_set - return @event_registrations.map(&:id).to_set if type_ids.empty? + return all_registration_ids if type_ids.empty? @event_registrations.select do |reg| reg.registrant.topic_subscriptions.any? do |sub| @@ -127,18 +147,6 @@ def matches_comment?(reg) end end - # Matches the registrant's location, like EventRegistration.located_in_city, so the same City - # value narrows the roster and the picker identically. - def matches_city?(reg) - any_term?(:city) do |term| - locations.rows_by_registration.fetch(reg.id, []).any? { |row| row.city.to_s.downcase.include?(term) } - end - end - - def locations - @locations ||= RegistrationLocations.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. @@ -158,8 +166,16 @@ def registrant_emails(reg) # when ANY of them hits (e.g. "amy--aisha" keeps both Amy and Aisha). Single # hyphens inside a value are preserved. Returns true when no term was typed. def any_term?(key) - tokens = @params[key].to_s.downcase.split("--").map(&:strip).reject(&:blank?) + tokens = terms_for(key) return true if tokens.empty? tokens.any? { |token| yield token } end + + def terms_for(key) + @params[key].to_s.downcase.split("--").map(&:strip).reject(&:blank?) + end + + def all_registration_ids + @all_registration_ids ||= @event_registrations.map(&:id).to_set + end end 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 eddd59f558..702149a326 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 @@ -11,3 +11,4 @@ summary: >- 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 address on their registration counts where they live instead." + - "City, State and County always describe the same address, so someone registered under organizations in two states won't show up for a city in one and a state in the other." diff --git a/spec/models/event_registration_spec.rb b/spec/models/event_registration_spec.rb index 525d23037a..1d3bcb4424 100644 --- a/spec/models/event_registration_spec.rb +++ b/spec/models/event_registration_spec.rb @@ -615,23 +615,46 @@ def registration_working_in(city:, state:, county: nil) it "matches the work city (case-insensitive, partial) over the registrant's own address" do create(:address, addressable: portland.registrant, city: "Santa Monica") - expect(EventRegistration.located_in_city("santa")).to contain_exactly(santa_monica) + expect(EventRegistration.located_in(city: "santa")).to contain_exactly(santa_monica) end it "falls back to the registrant's own address when no org is linked" do unlinked = create(:event_registration) create(:address, addressable: unlinked.registrant, city: "Santa Barbara", state: "CA") - expect(EventRegistration.located_in_city("santa")).to contain_exactly(santa_monica, unlinked) + expect(EventRegistration.located_in(city: "santa")).to contain_exactly(santa_monica, unlinked) end it "matches the work state" do - expect(EventRegistration.located_in_state("OR")).to contain_exactly(portland) + expect(EventRegistration.located_in(state: "OR")).to contain_exactly(portland) end it "matches the work county within a state" do - expect(EventRegistration.located_in_county("CA|Los Angeles")).to contain_exactly(santa_monica) - expect(EventRegistration.located_in_county("OR|Los Angeles")).to be_empty + expect(EventRegistration.located_in(county: "CA|Los Angeles")).to contain_exactly(santa_monica) + expect(EventRegistration.located_in(county: "OR|Los Angeles")).to be_empty + end + + it "passes everything through when no location filter is given" do + expect(EventRegistration.located_in).to include(santa_monica, portland) + end + + # One call, not three chained scopes: otherwise someone working in two states + # matches a city from one and the state from the other. + it "tests city, state and county against the same work address" do + two_states = create(:event_registration) + [ [ "Portland", "OR", "Multnomah" ], [ "Seattle", "WA", "King" ] ].each do |city, state, county| + organization = create(:organization) + create(:address, addressable: organization, city:, state:, county:) + two_states.event_registration_organizations.create!(organization:) + end + + expect(EventRegistration.located_in(city: "Seattle", state: "WA")).to contain_exactly(two_states) + expect(EventRegistration.located_in(city: "Seattle", state: "OR")).to be_empty + expect(EventRegistration.located_in(city: "Seattle", county: "OR|Multnomah")).to be_empty + end + + it "matches nothing when the State and County filters name different states" do + expect(EventRegistration.located_in(state: "CA", county: "OR|Multnomah")).to be_empty end end diff --git a/spec/requests/events_spec.rb b/spec/requests/events_spec.rb index cd7631968e..d306af0968 100644 --- a/spec/requests/events_spec.rb +++ b/spec/requests/events_spec.rb @@ -1796,6 +1796,25 @@ def pay_for_others_classes(template) expect(response.body).not_to include("Faraway") end + # The City and State controls sit in the same filter form, so they have to + # describe one address — otherwise a registrant working in two states matches + # a city from one and the state from the other. + it "matches the roster's city and state filters against the same work address" do + person = create(:person, first_name: "Bistate", last_name: "Worker") + registration = create(:event_registration, event: event, registrant: person) + [ %w[Portland OR], %w[Seattle WA] ].each do |city, state| + organization = create(:organization) + create(:address, addressable: organization, city: city, state: state) + registration.event_registration_organizations.create!(organization: organization) + end + + get registrants_event_path(event, city: "Seattle", state: "WA") + expect(response.body).to include("Bistate") + + get registrants_event_path(event, city: "Seattle", state: "OR") + expect(response.body).not_to include("Bistate") + end + it "does not crash on the new funder / comment-text / submission filters" do get registrants_event_path(event, funder_name: "someone") expect(response).to have_http_status(:ok) diff --git a/spec/services/registration_locations_spec.rb b/spec/services/registration_locations_spec.rb index aff75e7aef..6c94ad4e38 100644 --- a/spec/services/registration_locations_spec.rb +++ b/spec/services/registration_locations_spec.rb @@ -56,6 +56,22 @@ expect(described_class.new([ create(:event_registration, event: event).id ]).rows).to be_empty end + # A relation stays a subquery, so a whole-population lookup never inlines every + # registration id (the attendees index builds its filter options that way). + it "keeps a relation as a subquery instead of plucking its ids" do + create(:address, addressable: organization, city: "Austin", state: "TX") + + queries = [] + subscriber = ->(*, payload) { queries << payload[:sql] } + rows = ActiveSupport::Notifications.subscribed(subscriber, "sql.active_record") do + described_class.new(EventRegistration.where(event: event)).rows + end + + expect(rows.map(&:city_label)).to eq([ "Austin, TX" ]) + expect(queries.grep(/event_registration_organizations/).first) + .to include("SELECT `event_registrations`.`id` FROM `event_registrations`") + 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) diff --git a/spec/services/reminder_recipient_filter_spec.rb b/spec/services/reminder_recipient_filter_spec.rb index 13ff77afa6..d9e53d4c68 100644 --- a/spec/services/reminder_recipient_filter_spec.rb +++ b/spec/services/reminder_recipient_filter_spec.rb @@ -109,7 +109,22 @@ def matched(params, registrations) create(:address, addressable: lives_there.registrant, city: "Santa Monica") expect(matched({ city: "santa" }, [ works_there, works_elsewhere, lives_there ])).to eq([ works_there.id, lives_there.id ].to_set) - expect(EventRegistration.located_in_city("santa")).to contain_exactly(works_there, lives_there) + expect(EventRegistration.located_in(city: "santa")).to contain_exactly(works_there, lives_there) + end + + # The City search and the State dropdown have to describe the same address, like + # the roster — otherwise someone working in two states matches a city from one + # and the state from the other. + it "tests the city search and the state dropdown against the same address" do + two_states = registration + [ [ "Portland", "OR" ], [ "Seattle", "WA" ] ].each do |city, state| + organization = create(:organization) + create(:address, addressable: organization, city:, state:) + two_states.event_registration_organizations.create!(organization:) + end + + expect(matched({ city: "seattle", state: "WA" }, [ two_states ])).to eq([ two_states.id ].to_set) + expect(matched({ city: "seattle", state: "OR" }, [ two_states ])).to be_empty end it "filters by registration comment text" do From 32d766af29734bc2267ea500eb58dac9fc22d4c7 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 08:31:29 -0400 Subject: [PATCH 6/9] Let every location filter describe one address, and one field name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the work-location branch turned up three things. The attendees index resolved County in a second RegistrationLocations pass, so City and County could land on different addresses — the exact case located_in was built to prevent, and the one the feature entry promises is impossible. Its State and County dropdowns were filled by building a location row for every registration in the viewer's whole reportable population, where two grouped queries over the address tables answer the same question. And rows_by_registration had no callers. Rows now carry every field an address uses to say where it is — everything but the street and phone — so the next chart, count or filter asks for a field name rather than another method: registrant_ids_by(:country) replaces a hand-written country map, and locality, zip, school district and the LA districts come along for free. matching takes the whole filter set in one call, and filters_from folds a County option's own state into it so the roster scope and the index controller stop keeping separate copies of that rule. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 2 +- app/controllers/events_controller.rb | 28 ++-- app/models/event_registration.rb | 23 ++-- app/services/attendees_breakdowns.rb | 2 +- app/services/event_dashboard.rb | 2 +- app/services/registration_locations.rb | 124 +++++++++++++----- ...5-registrant-charts-use-work-locations.yml | 2 +- spec/requests/events/attendees_spec.rb | 31 ++++- spec/services/registration_locations_spec.rb | 74 ++++++++++- 9 files changed, 214 insertions(+), 74 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 6fa36ec6b3..f4083e543b 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 -- `RegistrationLocations` — Where each registrant is, per registration — where they work, else where they live: 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; a registration with no work address falls back to the registrant's own primary (else oldest) active address. Groups registrant ids by US state, country, and "City, State"; `#matching(state:, county:, city:)` tests one address per row. Takes a relation (kept as a subquery, so a whole-population lookup never inlines every registration id) or an explicit id array. 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.located_in`, which takes all three at once so they test the same address, and `ReminderRecipientFilter`), and the attendees index's State/County/City filters and `country`/`org_city` drill-ins +- `RegistrationLocations` — Where each registrant is, per registration — where they work, else where they live: 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; a registration with no work address falls back to the registrant's own primary (else oldest) active address. Each row carries every `LOCATION_FIELDS` value the address holds — city, county, state, country, locality, zip, school district and the three LA districts, i.e. everything bar the street and phone — so a new chart, count or filter needs only a field name: `#registrant_ids_by(field)` groups ids by any of them (`#registrant_ids_by_state` is US-only for the map, `#registrant_ids_by_city_label` keys on "City, State"), and `#matching(**fields)` tests ONE address per row against the whole filter set at once — chaining two calls is what lets someone working in two places answer a city from one and a county from the other. `.filters_from(**params)` folds a County option's own state ("CA|Los Angeles" on the roster, "CA::Los Angeles" on the attendees index) into `state:` and returns nil when a State filter contradicts it. `#candidate_values(*fields)` answers filter option lists straight from the org/registrant address tables (two grouped queries, a superset) so a whole-population dropdown never builds a row per registration. Takes a relation (kept as a subquery, so a whole-population lookup never inlines every registration id) or an explicit id array. The single source for every registrant location figure and filter: the charts' cities, states, and countries (`EventDashboard`, `AttendeesBreakdowns`), the roster/reminder City/State/County filters (`EventRegistration.located_in`, which takes them all at once, and `ReminderRecipientFilter`), and the attendees index's City/State/County 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 `RegistrationLocations` — 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 2630e04468..f714f2aac2 100644 --- a/app/controllers/events_controller.rb +++ b/app/controllers/events_controller.rb @@ -851,7 +851,6 @@ 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_locations.registrant_ids_by_country.fetch(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? @@ -867,17 +866,18 @@ def filtered_attendees scope.where(id: scholarship_status_person_ids(params[:scholarship])) end end - # Address filters match where people are (RegistrationLocations) on their in-scope - # registrations. City and state test one address, so someone 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: attendee_locations.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: attendee_locations.matching(state: county_state, county: county_name).map(&:registrant_id)) + # Where people are (RegistrationLocations) on their in-scope registrations. The + # City box, the State and County dropdowns and the countries drill-in resolve in + # one pass, so they all describe the same address: someone working in two states + # can't match a city from one and the county from the other. A State that + # contradicts the county's own state matches nobody. + location_filters = RegistrationLocations.filters_from( + state: params[:state], county: params[:county], city: params[:city], country: params[:country] + ) + if location_filters.nil? + scope = scope.none + elsif location_filters.any? + scope = scope.where(id: attendee_locations.matching(**location_filters).map(&:registrant_id)) end # Shared registration-level filters, applied cross-event with "any registration @@ -1014,8 +1014,8 @@ def set_attendee_filter_options 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) locations = RegistrationLocations.new(EventRegistration.where(event_id: reportable_events.select(:id))) - @attendee_states = locations.rows.filter_map(&:state).compact_blank.uniq.sort - @attendee_counties = locations.counties + @attendee_states = locations.candidate_values(:state) + @attendee_counties = locations.candidate_values(:state, :county) @attendee_topic_types = TopicSubscriptionType.active.ordered end diff --git a/app/models/event_registration.rb b/app/models/event_registration.rb index a7b2b760d0..5b895ce609 100644 --- a/app/models/event_registration.rb +++ b/app/models/event_registration.rb @@ -221,20 +221,15 @@ class EventRegistration < ApplicationRecord scope } # Where the registrant is on the registration — work, else home - # (RegistrationLocations). City, state and county arrive together in one call so - # they test the same location row: a registrant working in two states would - # otherwise match a city from one and the state from the other. County accepts - # "STATE|County" (from the roster filter) to scope to a county within a specific - # state, or a bare county name for backward compatibility; a county state that - # contradicts the State filter can match nothing. - scope :located_in, ->(state: nil, county: nil, city: nil) { - county_state, county_name = county.to_s.include?("|") ? county.split("|", 2) : [ nil, county.to_s ] - next none if county.present? && county_name.blank? - states = [ state, county_state ].compact_blank.uniq - next none if states.many? - next all if states.empty? && county_name.blank? && city.blank? - matches = RegistrationLocations.new(all).matching(state: states.first, county: county_name.presence, city: city.presence) - where(id: matches.map(&:registration_id)) + # (RegistrationLocations). Takes any of its LOCATION_FIELDS, and they arrive + # together in one call so they test the same location row: a registrant working + # in two states would otherwise match a city from one and the state from the + # other. + scope :located_in, ->(**filters) { + resolved = RegistrationLocations.filters_from(**filters) + next none if resolved.nil? + next all if resolved.empty? + where(id: RegistrationLocations.new(all).matching(**resolved).map(&:registration_id)) } scope :registrant_sector, ->(sector_id) { joins(registrant: :sectorable_items) diff --git a/app/services/attendees_breakdowns.rb b/app/services/attendees_breakdowns.rb index ce29faeac6..2f81b0395b 100644 --- a/app/services/attendees_breakdowns.rb +++ b/app/services/attendees_breakdowns.rb @@ -184,7 +184,7 @@ def state_registrant_ids_by_state end def country_registrant_ids_by_country - locations.registrant_ids_by_country + locations.registrant_ids_by(:country) end def school_district_registrant_ids_by_district diff --git a/app/services/event_dashboard.rb b/app/services/event_dashboard.rb index 1551494211..793fcf38b6 100644 --- a/app/services/event_dashboard.rb +++ b/app/services/event_dashboard.rb @@ -897,7 +897,7 @@ def school_district_registrant_ids end def country_registrant_ids_by_country - locations.registrant_ids_by_country + locations.registrant_ids_by(:country) end # Registrant ids per school district, for the breakdown's per-row drill-in. A diff --git a/app/services/registration_locations.rb b/app/services/registration_locations.rb index 0d9688649b..a6c47f88b4 100644 --- a/app/services/registration_locations.rb +++ b/app/services/registration_locations.rb @@ -2,17 +2,51 @@ # live: 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; with no such address, the registrant's own -# primary (else oldest) active address. The source for the registrant charts' and -# filters' cities, counties, states, and countries. A linked org with no address -# and no own address to fall back on yields a row with no location, so the -# cities breakdown can count it as Unknown. +# primary (else oldest) active address. The source for every registrant location +# chart, count, and filter. A linked org with no address and no own address to +# fall back on yields a row with no location, so the cities breakdown can count +# it as Unknown. class RegistrationLocations - Row = Struct.new(:registration_id, :organization_id, :registrant_id, :city, :county, :state, :country, keyword_init: true) do + # Everything an address says about *where* it is — the whole record bar the + # street and phone, which identify a building rather than place it. A row + # carries all of them so a new chart, count, or filter needs only a field name. + LOCATION_FIELDS = %i[ + city county state country locality zip_code district + la_city_council_district la_supervisorial_district la_service_planning_area + ].freeze + # Fields a filter matches on a case-insensitive substring, because they arrive + # from a free-text box. The rest are picked from a list this class built, so + # they match whole (case-insensitively, as the database's own collation does). + # Add a field here when it gets a text box of its own. + SEARCHED_FIELDS = %i[ city ].freeze + # County filter options carry their state so same-named counties across states + # don't collide — "CA|Los Angeles" on the registrants roster, "CA::Los Angeles" + # on the attendees index. + COUNTY_STATE_SEPARATOR = /::|\|/ + + Row = Struct.new(:registration_id, :organization_id, :registrant_id, *LOCATION_FIELDS, keyword_init: true) do def city_label [ city, state ].compact_blank.join(", ").presence end end + # A page's raw location params as fields #matching understands: the county's + # own state folded into `state`, blanks dropped. Returns nil when nothing can + # satisfy the combination — the State dropdown naming one state and the County + # dropdown's own state another. + def self.filters_from(**filters) + county_state, county = split_county(filters[:county]) + return nil if filters[:county].present? && county.blank? + states = [ filters[:state], county_state ].compact_blank.uniq + return nil if states.many? + filters.merge(state: states.first, county: county).compact_blank + end + + def self.split_county(value) + state, county = value.to_s.split(COUNTY_STATE_SEPARATOR, 2) + county.nil? ? [ nil, state ] : [ state, county ] + end + # Takes the registrations to locate, as a relation (kept as a subquery, so a # whole-population lookup never inlines every id) or as an explicit id array. def initialize(registrations) @@ -33,19 +67,13 @@ def rows 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) + # Rows whose one address answers to every field given — so a registrant working + # in two states can't match a city from one and the county from the other. Pass + # the whole filter set in one call; chaining two calls is what loses that. + def matching(**filters) + filters = filters.compact_blank + filters.assert_valid_keys(*LOCATION_FIELDS) + rows.select { |row| filters.all? { |field, value| matches?(row, field, value) } } end # Distinct [ state, county ] pairs, sorted, for the County filters. @@ -53,28 +81,53 @@ def counties @counties ||= rows.filter_map { |row| [ row.state, row.county ] if row.state.present? && row.county.present? }.uniq.sort end + # { value => distinct registrant ids } for a location field — the shape every + # breakdown chart, count, and drill-in reads. Rows with no value sit it out. + def registrant_ids_by(field) + registrant_id_maps[field] ||= group_registrant_ids(rows.select { |row| row.public_send(field).present? }, &field) + 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) + @registrant_ids_by_state ||= group_registrant_ids(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) + def registrant_ids_by_city_label + @registrant_ids_by_city_label ||= group_registrant_ids(rows.select(&:city_label), &:city_label) end - def registrant_ids_by_city_label - @registrant_ids_by_city_label ||= registrant_ids_by(rows.select(&:city_label), &:city_label) + # Distinct values of one or more location fields across every address these + # registrations could resolve to — the linked organizations' and the + # registrants' own — for a filter's option list. Two grouped queries rather + # than a row per registration, so the attendees index can offer options drawn + # from its whole reportable population. It skips the work-else-home precedence + # #rows applies, so it can offer a value no registration settles on; an option + # list is already deliberately wider than the active filters. + def candidate_values(*fields) + [ organization_addresses, registrant_addresses ].flat_map { |scope| + fields.reduce(scope) { |relation, field| relation.where.not(field => [ nil, "" ]) }.distinct.pluck(*fields) + }.uniq.sort end private - def registrant_ids_by(rows, &key) + def matches?(row, field, value) + actual = row.public_send(field).to_s + return actual.downcase.include?(value.to_s.downcase.strip) if SEARCHED_FIELDS.include?(field) + actual.casecmp?(value.to_s) + end + + def registrant_id_maps + @registrant_id_maps ||= {} + end + + def group_registrant_ids(rows, &key) rows.group_by(&key).transform_values { |grouped| grouped.map(&:registrant_id).uniq } end def row(registration_id, organization_id, registrant_id, address) Row.new(registration_id:, organization_id:, registrant_id:, - city: address&.city, county: address&.county, state: address&.state, country: address&.country) + **LOCATION_FIELDS.index_with { |field| address&.public_send(field) }) end def registration_pairs @@ -104,27 +157,30 @@ def chosen_address_ids def chosen_addresses @chosen_addresses ||= Address.active .where(id: Affiliation.where(event_registration_id: registration_ids).select(:organization_address_id)) - .select(:id, :city, :county, :state, :country) + .select(:id, *LOCATION_FIELDS) .index_by(&:id) end - def linked_organization_ids - EventRegistrationOrganization.where(event_registration_id: registration_ids).select(:organization_id) + def organization_addresses + Address.active.where(addressable_type: "Organization", + addressable_id: EventRegistrationOrganization.where(event_registration_id: registration_ids).select(:organization_id)) + end + + def registrant_addresses + Address.active.where(addressable_type: "Person", addressable_id: @registrations.select(:registrant_id)) end def oldest_address_by_organization - @oldest_address_by_organization ||= Address.active - .where(addressable_type: "Organization", addressable_id: linked_organization_ids) + @oldest_address_by_organization ||= organization_addresses .order(:id) - .select(:id, :addressable_id, :city, :county, :state, :country) + .select(:id, :addressable_id, *LOCATION_FIELDS) .each_with_object({}) { |address, map| map[address.addressable_id] ||= address } end def own_address_by_person - @own_address_by_person ||= Address.active - .where(addressable_type: "Person", addressable_id: @registrations.select(:registrant_id)) + @own_address_by_person ||= registrant_addresses .order(primary: :desc, id: :asc) - .select(:id, :addressable_id, :city, :county, :state, :country) + .select(:id, :addressable_id, *LOCATION_FIELDS) .each_with_object({}) { |address, map| map[address.addressable_id] ||= address } end end 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 702149a326..58b1fdb20d 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 @@ -11,4 +11,4 @@ summary: >- 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 address on their registration counts where they live instead." - - "City, State and County always describe the same address, so someone registered under organizations in two states won't show up for a city in one and a state in the other." + - "City, State, County and Country always describe the same address, so someone registered under organizations in two states won't show up for a city in one and a state in the other." diff --git a/spec/requests/events/attendees_spec.rb b/spec/requests/events/attendees_spec.rb index 80f6e9d904..9982085401 100644 --- a/spec/requests/events/attendees_spec.rb +++ b/spec/requests/events/attendees_spec.rb @@ -564,19 +564,36 @@ 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| + # Every location filter has to describe one address — otherwise someone + # working in two places matches a city from one and the state, county or + # country from the other. + it "matches city, state, county and country against the same work address" do + [ [ "Portland", "OR", "Multnomah", "United States" ], [ "Vancouver", "BC", "Greater Vancouver", "Canada" ] ].each do |city, state, county, country| org = create(:organization) - create(:address, addressable: org, city:, state:, inactive: false) + create(:address, addressable: org, city:, state:, county:, country:, inactive: false) attendee_registration.event_registration_organizations.create!(organization: org) end - get attendees_events_url(city: "Seattle", state: "WA"), headers: frame_headers + get attendees_events_url(city: "Vancouver", state: "BC"), headers: frame_headers expect(response.body).to include("Ada Lovelace") - get attendees_events_url(city: "Seattle", state: "OR"), headers: frame_headers + get attendees_events_url(city: "Vancouver", state: "OR"), headers: frame_headers + expect(response.body).not_to include("Ada Lovelace") + + get attendees_events_url(city: "Vancouver", county: "OR::Multnomah"), headers: frame_headers + expect(response.body).not_to include("Ada Lovelace") + + get attendees_events_url(city: "Vancouver", country: "United States"), headers: frame_headers + expect(response.body).not_to include("Ada Lovelace") + end + + it "matches nobody when the State dropdown contradicts the county's own state" do + org = create(:organization) + create(:address, addressable: org, city: "Portland", state: "OR", county: "Multnomah", inactive: false) + attendee_registration.event_registration_organizations.create!(organization: org) + + get attendees_events_url(state: "OR", county: "WA::King"), headers: frame_headers + expect(response).to have_http_status(:ok) expect(response.body).not_to include("Ada Lovelace") end diff --git a/spec/services/registration_locations_spec.rb b/spec/services/registration_locations_spec.rb index 6c94ad4e38..2807709352 100644 --- a/spec/services/registration_locations_spec.rb +++ b/spec/services/registration_locations_spec.rb @@ -79,7 +79,79 @@ registration.event_registration_organizations.create!(organization: abroad) expect(locations.registrant_ids_by_state).to eq("TX" => [ person.id ]) - expect(locations.registrant_ids_by_country).to eq("United States" => [ person.id ], "Canada" => [ person.id ]) + expect(locations.registrant_ids_by(:country)).to eq("United States" => [ person.id ], "Canada" => [ person.id ]) expect(locations.registrant_ids_by_city_label.keys).to contain_exactly("Austin, TX", "Toronto, ON") end + + it "groups by any other location field the address carries" do + create(:address, addressable: organization, city: "Austin", state: "TX", + zip_code: "78701", county: "Travis", locality: Address::OUTSIDE_CA, district: "Austin ISD") + + expect(locations.registrant_ids_by(:zip_code)).to eq("78701" => [ person.id ]) + expect(locations.registrant_ids_by(:locality)).to eq(Address::OUTSIDE_CA => [ person.id ]) + expect(locations.registrant_ids_by(:district)).to eq("Austin ISD" => [ person.id ]) + end + + describe "#matching" do + before do + create(:address, addressable: organization, city: "Austin", state: "TX", county: "Travis", country: "United States") + abroad = create(:organization) + create(:address, addressable: abroad, city: "Toronto", state: "ON", county: "York", country: "Canada") + registration.event_registration_organizations.create!(organization: abroad) + end + + # One call, not two: chaining them is what lets a registrant working in two + # places answer a city from one and a county from the other. + it "tests every field against the same address" do + expect(locations.matching(city: "toronto", country: "Canada")).to be_present + expect(locations.matching(city: "toronto", country: "United States")).to be_empty + expect(locations.matching(city: "toronto", county: "Travis")).to be_empty + end + + it "matches a searched field on a substring and the rest whole, both case-insensitively" do + expect(locations.matching(city: "AUST")).to be_present + expect(locations.matching(state: "tx")).to be_present + expect(locations.matching(county: "Trav")).to be_empty + end + + it "rejects a field that isn't a location" do + expect { locations.matching(street_address: "908 N 6th St") }.to raise_error(ArgumentError) + end + end + + describe ".filters_from" do + it "folds the county option's own state into the state filter" do + expect(described_class.filters_from(county: "CA|Los Angeles")).to eq(state: "CA", county: "Los Angeles") + expect(described_class.filters_from(county: "CA::Los Angeles", city: "pas")).to eq(state: "CA", county: "Los Angeles", city: "pas") + end + + it "keeps a bare county name, and drops blanks" do + expect(described_class.filters_from(county: "Los Angeles", state: "")).to eq(county: "Los Angeles") + expect(described_class.filters_from(state: nil, county: nil, city: nil)).to eq({}) + end + + it "reports no possible match when the state and the county's state disagree" do + expect(described_class.filters_from(state: "CA", county: "OR|Multnomah")).to be_nil + expect(described_class.filters_from(county: "CA|")).to be_nil + end + end + + describe "#candidate_values" do + # Option lists are built from the whole reportable population, so they read the + # address tables directly rather than resolving a row per registration. + it "lists the values the linked orgs' and registrants' addresses carry" do + create(:address, addressable: organization, state: "TX", county: "Travis") + create(:address, addressable: person, state: "ID", county: "Ada") + + expect(locations.candidate_values(:state)).to eq(%w[ID TX]) + expect(locations.candidate_values(:state, :county)).to eq([ [ "ID", "Ada" ], [ "TX", "Travis" ] ]) + end + + it "skips addresses missing any of the fields asked for" do + create(:address, addressable: organization, state: "TX", county: "") + + expect(locations.candidate_values(:state)).to eq([ "TX" ]) + expect(locations.candidate_values(:state, :county)).to be_empty + end + end end From 62a328c1398a8494417cdd0cf8379f06ee739f6a Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 13:25:53 -0400 Subject: [PATCH 7/9] Settle which address stands for a registrant when only one can MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The own-address fallback picked the primary flag, which is pre-checked on every address added and so rarely points at the current one. Address::STANDING ranks them instead: active work, else the most recently edited active personal, else a retired personal, else a retired work. Work and personal swap places below the active line on purpose — a current work address is the sharpest thing we know about someone and a retired one the stalest, because people change jobs and home outlasts the job. The list is written once in Ruby and compiled into the ORDER BY, because a hand-kept SQL twin drifts the first time the order changes. AddressDecorator sorts its already-loaded association by the same list instead of its own near-copy of half of it. Retired addresses now count, which is why registrant_addresses drops .active: a registrant holding nothing but a stale address belongs on the map at that address rather than nowhere. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 2 +- app/decorators/address_decorator.rb | 5 ++- app/models/address.rb | 34 ++++++++++++++++ app/services/registration_locations.rb | 15 ++++--- ...5-registrant-charts-use-work-locations.yml | 2 +- spec/models/address_spec.rb | 40 +++++++++++++++++++ spec/services/registration_locations_spec.rb | 14 +++++-- 7 files changed, 101 insertions(+), 11 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index f4083e543b..d8e6a37944 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 -- `RegistrationLocations` — Where each registrant is, per registration — where they work, else where they live: 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; a registration with no work address falls back to the registrant's own primary (else oldest) active address. Each row carries every `LOCATION_FIELDS` value the address holds — city, county, state, country, locality, zip, school district and the three LA districts, i.e. everything bar the street and phone — so a new chart, count or filter needs only a field name: `#registrant_ids_by(field)` groups ids by any of them (`#registrant_ids_by_state` is US-only for the map, `#registrant_ids_by_city_label` keys on "City, State"), and `#matching(**fields)` tests ONE address per row against the whole filter set at once — chaining two calls is what lets someone working in two places answer a city from one and a county from the other. `.filters_from(**params)` folds a County option's own state ("CA|Los Angeles" on the roster, "CA::Los Angeles" on the attendees index) into `state:` and returns nil when a State filter contradicts it. `#candidate_values(*fields)` answers filter option lists straight from the org/registrant address tables (two grouped queries, a superset) so a whole-population dropdown never builds a row per registration. Takes a relation (kept as a subquery, so a whole-population lookup never inlines every registration id) or an explicit id array. The single source for every registrant location figure and filter: the charts' cities, states, and countries (`EventDashboard`, `AttendeesBreakdowns`), the roster/reminder City/State/County filters (`EventRegistration.located_in`, which takes them all at once, and `ReminderRecipientFilter`), and the attendees index's City/State/County filters and `country`/`org_city` drill-ins +- `RegistrationLocations` — Where each registrant is, per registration — where they work, else where they live: 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; a registration with no work address falls back to the one address that stands for the registrant themselves (`Address.ranked_for_owner` / `Address::STANDING` — active work, else latest-edited active personal, else inactive personal, else inactive work; work and personal swap below the active line because a current work address is the sharpest thing we know and a retired one the stalest. Written once in Ruby and compiled into the ORDER BY; `AddressDecorator#saved_address` sorts a loaded association by the same list. Retired addresses count, because placing someone by a stale address beats leaving them off the map). Each row carries every `LOCATION_FIELDS` value the address holds — city, county, state, country, locality, zip, school district and the three LA districts, i.e. everything bar the street and phone — so a new chart, count or filter needs only a field name: `#registrant_ids_by(field)` groups ids by any of them (`#registrant_ids_by_state` is US-only for the map, `#registrant_ids_by_city_label` keys on "City, State"), and `#matching(**fields)` tests ONE address per row against the whole filter set at once — chaining two calls is what lets someone working in two places answer a city from one and a county from the other. `.filters_from(**params)` folds a County option's own state ("CA|Los Angeles" on the roster, "CA::Los Angeles" on the attendees index) into `state:` and returns nil when a State filter contradicts it. `#candidate_values(*fields)` answers filter option lists straight from the org/registrant address tables (two grouped queries, a superset) so a whole-population dropdown never builds a row per registration. Takes a relation (kept as a subquery, so a whole-population lookup never inlines every registration id) or an explicit id array. The single source for every registrant location figure and filter: the charts' cities, states, and countries (`EventDashboard`, `AttendeesBreakdowns`), the roster/reminder City/State/County filters (`EventRegistration.located_in`, which takes them all at once, and `ReminderRecipientFilter`), and the attendees index's City/State/County 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 `RegistrationLocations` — 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/decorators/address_decorator.rb b/app/decorators/address_decorator.rb index 98a0190238..7f3b4e2860 100644 --- a/app/decorators/address_decorator.rb +++ b/app/decorators/address_decorator.rb @@ -56,10 +56,13 @@ def default_locality # The saved address a new one copies: an active address before a retired one, and the # most recently updated of those. + # The one address that stands for this owner (Address::STANDING), sorted in + # memory off the loaded association so a form with unsaved blocks pays no query + # — the same order Address.ranked_for_owner gives a relation. def saved_address return @saved_address if defined?(@saved_address) saved = addressable&.addresses&.select(&:persisted?) || [] - @saved_address = saved.min_by { |address| [ address.inactive? ? 1 : 0, -address.updated_at.to_f ] } + @saved_address = saved.min_by { |address| [ address.standing, -address.updated_at.to_f ] } end end diff --git a/app/models/address.rb b/app/models/address.rb index f6bda0fce7..0f26e17564 100644 --- a/app/models/address.rb +++ b/app/models/address.rb @@ -51,6 +51,40 @@ class Address < ApplicationRecord scope :active, -> { where(inactive: false) } scope :for_invoices, -> { where(invoice_address: true) } + # Which of an owner's addresses stands for them when only one can, best first: + # [ inactive?, address_type ] — a nil type matches whatever is left at that + # active/inactive level. Work and personal swap places below the active line on + # purpose: a current work address is the sharpest thing we know about someone, + # and a retired one the stalest (people change jobs; home outlasts the job). A + # retired address still beats placing them nowhere. + # + # Written once, in Ruby, because both the ORDER BY and Address#standing read it — + # a hand-kept SQL twin of this list would drift the first time an order changed. + STANDING = [ + [ false, "work" ], + [ false, "personal" ], + [ false, nil ], + [ true, "personal" ], + [ true, "work" ] + ].freeze + + # Interpolated, not bound: every value comes from STANDING above, never a caller. + STANDING_SQL = STANDING.map.with_index { |(inactive, type), rank| + tests = [ "addresses.inactive = #{inactive ? 1 : 0}" ] + tests << "addresses.address_type = '#{type}'" if type + "WHEN #{tests.join(' AND ')} THEN #{rank}" + }.join(" ").then { |whens| "CASE #{whens} ELSE #{STANDING.size} END" }.freeze + + # Best-standing first, then most recently edited — "latest edited" is how two + # addresses at the same standing are told apart. + scope :ranked_for_owner, -> { order(Arel.sql(STANDING_SQL), updated_at: :desc, id: :asc) } + + # This address's place in STANDING, for sorting an already-loaded set the same + # way the scope sorts a query. + def standing + STANDING.index { |rank_inactive, type| rank_inactive == inactive? && (type.nil? || type == address_type) } || STANDING.size + end + # A nested-attributes block nobody filled in, which the parent drops instead of # building an address that can only fail validation. def self.unfilled_attributes?(attributes) diff --git a/app/services/registration_locations.rb b/app/services/registration_locations.rb index a6c47f88b4..493e48bb46 100644 --- a/app/services/registration_locations.rb +++ b/app/services/registration_locations.rb @@ -1,8 +1,10 @@ # Where each registrant is, per registration — where they work, else where they # live: 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; with no such address, the registrant's own -# primary (else oldest) active address. The source for every registrant location +# that org's oldest active address; with no such address, the one address that +# stands for the registrant themselves (Address#ranked_for_owner — active work, +# else latest-edited active personal, else inactive personal, else inactive work). +# The source for every registrant location # chart, count, and filter. A linked org with no address and no own address to # fall back on yields a row with no location, so the cities breakdown can count # it as Unknown. @@ -166,8 +168,11 @@ def organization_addresses addressable_id: EventRegistrationOrganization.where(event_registration_id: registration_ids).select(:organization_id)) end + # Not narrowed to active: a registrant with nothing but a retired address is + # better placed there than left out of the map entirely (Address#ranked_for_owner + # ranks retired addresses last). def registrant_addresses - Address.active.where(addressable_type: "Person", addressable_id: @registrations.select(:registrant_id)) + Address.where(addressable_type: "Person", addressable_id: @registrations.select(:registrant_id)) end def oldest_address_by_organization @@ -179,8 +184,8 @@ def oldest_address_by_organization def own_address_by_person @own_address_by_person ||= registrant_addresses - .order(primary: :desc, id: :asc) - .select(:id, :addressable_id, *LOCATION_FIELDS) + .ranked_for_owner + .select(:id, :addressable_id, :inactive, :address_type, :updated_at, *LOCATION_FIELDS) .each_with_object({}) { |address, map| map[address.addressable_id] ||= address } end end 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 58b1fdb20d..cb2c267db2 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 @@ -10,5 +10,5 @@ summary: >- 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 address on their registration counts where they live instead." + - "A registrant with no organization address on their registration counts where their own address puts them: their active work address, else their most recently edited active personal one, else a retired personal, else a retired work. A retired address still counts — better to place someone than to leave them off the map." - "City, State, County and Country always describe the same address, so someone registered under organizations in two states won't show up for a city in one and a state in the other." diff --git a/spec/models/address_spec.rb b/spec/models/address_spec.rb index 197c70e00c..54d110d224 100644 --- a/spec/models/address_spec.rb +++ b/spec/models/address_spec.rb @@ -167,6 +167,46 @@ end end + describe ".ranked_for_owner" do + let(:person) { create(:person) } + + def address_typed(type, inactive: false, city: type.to_s) + create(:address, addressable: person, address_type: type, inactive: inactive, city: city) + end + + it "prefers the active work address" do + address_typed("personal") + work = address_typed("work") + + expect(person.addresses.ranked_for_owner.first).to eq(work) + end + + it "falls back to the most recently edited active personal address" do + older = address_typed("personal", city: "Boise") + newer = address_typed("personal", city: "Nampa") + older.update!(zip_code: "83701") + newer.update!(zip_code: "83651") + + expect(person.addresses.ranked_for_owner.first).to eq(newer) + end + + it "takes an untyped active address over any retired one" do + address_typed("work", inactive: true) + unknown = address_typed(Address::UNKNOWN_CONTACT_TYPE) + + expect(person.addresses.ranked_for_owner.first).to eq(unknown) + end + + # A retired work address is the stalest thing we hold — people change jobs, + # and home outlasts the job — so it sorts behind a retired personal one. + it "prefers a retired personal address to a retired work address" do + address_typed("work", inactive: true) + personal = address_typed("personal", inactive: true) + + expect(person.addresses.ranked_for_owner.first).to eq(personal) + end + end + describe ".unfilled_attributes?" do it "is true for a block carrying only the pre-filled country, state, and locality" do attrs = { "country" => "Canada", "state" => "AB", "locality" => "Outside USA", diff --git a/spec/services/registration_locations_spec.rb b/spec/services/registration_locations_spec.rb index 2807709352..383c10cf0a 100644 --- a/spec/services/registration_locations_spec.rb +++ b/spec/services/registration_locations_spec.rb @@ -33,13 +33,21 @@ expect(locations.rows.map(&:city_label)).to eq([ "Reno, NV" ]) end - it "falls back to the registrant's primary address when the linked org has none" do - create(:address, addressable: person, city: "Boise", state: "ID") - create(:address, addressable: person, city: "Nampa", state: "ID", primary: true) + # Address#ranked_for_owner: active work, else latest-edited active personal, + # else inactive personal, else inactive work. + it "falls back to the address that stands for the registrant" do + create(:address, addressable: person, city: "Boise", state: "ID", address_type: "personal") + create(:address, addressable: person, city: "Nampa", state: "ID", address_type: "work") expect(locations.rows.map(&:city_label)).to eq([ "Nampa, ID" ]) end + it "places a registrant by a retired address rather than nowhere" do + create(:address, addressable: person, city: "Boise", state: "ID", address_type: "personal", inactive: true) + + expect(locations.rows.map(&:city_label)).to eq([ "Boise, ID" ]) + end + it "falls back to the registrant's own address when no org is linked" do unlinked = create(:event_registration, event: event) create(:address, addressable: unlinked.registrant, city: "Boise", state: "ID") From 1f62ddc48d37f4707bff2af676b35a6865db9529 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 14:10:51 -0400 Subject: [PATCH 8/9] Locate people by their active affiliations on a date, not by the registration One rule for every location chart, filter, and report: the work addresses of the affiliations active on a date (the event's start date on event pages, today elsewhere), else the person's own standing address. That lets pages that span events, the reports, and later the people index share the rule, while a past event still doesn't move when someone changes jobs. RegistrationLocations becomes PersonLocations with the same interface. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 6 +- app/controllers/events_controller.rb | 14 +-- app/models/event_registration.rb | 15 ++- app/presenters/registrant_city_breakdown.rb | 6 +- app/services/attendees_breakdowns.rb | 2 +- app/services/event_dashboard.rb | 6 +- app/services/event_participation_report.rb | 18 +-- ...ation_locations.rb => person_locations.rb} | 106 ++++++++---------- app/services/reminder_recipient_filter.rb | 2 +- ...5-registrant-charts-use-work-locations.yml | 15 +-- spec/models/event_registration_spec.rb | 6 +- spec/requests/events/attendees_spec.rb | 16 +-- spec/requests/events/bulk_reminders_spec.rb | 4 +- spec/requests/events_spec.rb | 19 ++-- spec/services/attendees_breakdowns_spec.rb | 9 +- .../event_participation_report_spec.rb | 14 ++- ...tions_spec.rb => person_locations_spec.rb} | 91 +++++++++------ .../reminder_recipient_filter_spec.rb | 6 +- 18 files changed, 182 insertions(+), 173 deletions(-) rename app/services/{registration_locations.rb => person_locations.rb} (54%) rename spec/services/{registration_locations_spec.rb => person_locations_spec.rb} (68%) diff --git a/AGENTS.md b/AGENTS.md index d8e6a37944..729b88eb4d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -236,8 +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 -- `RegistrationLocations` — Where each registrant is, per registration — where they work, else where they live: 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; a registration with no work address falls back to the one address that stands for the registrant themselves (`Address.ranked_for_owner` / `Address::STANDING` — active work, else latest-edited active personal, else inactive personal, else inactive work; work and personal swap below the active line because a current work address is the sharpest thing we know and a retired one the stalest. Written once in Ruby and compiled into the ORDER BY; `AddressDecorator#saved_address` sorts a loaded association by the same list. Retired addresses count, because placing someone by a stale address beats leaving them off the map). Each row carries every `LOCATION_FIELDS` value the address holds — city, county, state, country, locality, zip, school district and the three LA districts, i.e. everything bar the street and phone — so a new chart, count or filter needs only a field name: `#registrant_ids_by(field)` groups ids by any of them (`#registrant_ids_by_state` is US-only for the map, `#registrant_ids_by_city_label` keys on "City, State"), and `#matching(**fields)` tests ONE address per row against the whole filter set at once — chaining two calls is what lets someone working in two places answer a city from one and a county from the other. `.filters_from(**params)` folds a County option's own state ("CA|Los Angeles" on the roster, "CA::Los Angeles" on the attendees index) into `state:` and returns nil when a State filter contradicts it. `#candidate_values(*fields)` answers filter option lists straight from the org/registrant address tables (two grouped queries, a superset) so a whole-population dropdown never builds a row per registration. Takes a relation (kept as a subquery, so a whole-population lookup never inlines every registration id) or an explicit id array. The single source for every registrant location figure and filter: the charts' cities, states, and countries (`EventDashboard`, `AttendeesBreakdowns`), the roster/reminder City/State/County filters (`EventRegistration.located_in`, which takes them all at once, and `ReminderRecipientFilter`), and the attendees index's City/State/County 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 `RegistrationLocations` — 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 +- `PersonLocations` — Where each person is on a date (`on:`, default today) — where they work, else where they live: every affiliation active on that date (`Affiliation.active_by_date_on`, or `.active` for today), at its picked address (`Affiliation#organization_address`), else its org's oldest active address — every active affiliation counts; with none, the one address that stands for the person themselves (`Address.ranked_for_owner` / `Address::STANDING` — active work, else latest-edited active personal, else inactive personal, else inactive work; work and personal swap below the active line because a current work address is the sharpest thing we know and a retired one the stalest. Written once in Ruby and compiled into the ORDER BY; `AddressDecorator#saved_address` sorts a loaded association by the same list. Retired addresses count, because placing someone by a stale address beats leaving them off the map). Each row carries every `LOCATION_FIELDS` value the address holds — city, county, state, country, locality, zip, school district and the three LA districts, i.e. everything bar the street and phone — so a new chart, count or filter needs only a field name: `#registrant_ids_by(field)` groups ids by any of them (`#registrant_ids_by_state` is US-only for the map, `#registrant_ids_by_city_label` keys on "City, State"), and `#matching(**fields)` tests ONE address per row against the whole filter set at once — chaining two calls is what lets someone working in two places answer a city from one and a county from the other. `.filters_from(**params)` folds a County option's own state ("CA|Los Angeles" on the roster, "CA::Los Angeles" on the attendees index) into `state:` and returns nil when a State filter contradicts it. `#candidate_values(*fields)` answers filter option lists straight from the org/registrant address tables (two grouped queries, a superset) so a whole-population dropdown never builds a row per person. Takes a person-id relation (kept as a subquery, so a whole-population lookup never inlines every id) or an explicit id array. Event pages pass the event's start date so a past event doesn't move when someone changes jobs; cross-event pages and reports use today. The single source for every person location figure and filter: the charts' cities, states, and countries (`EventDashboard` on the event date, `AttendeesBreakdowns` on `as_of`), the roster/reminder City/State/County filters (`EventRegistration.located_in(on:, ...)`, which takes them all at once, and `ReminderRecipientFilter`), the attendees index's City/State/County filters and `country`/`org_city` drill-ins (the filtered event's date, else today), and `EventParticipationReport#demographics` (today) +- `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 `PersonLocations` — 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 @@ -289,7 +289,7 @@ comparison — add a rule if none fits (e.g. `PersonPolicy#own_profile?`). Conve - `DisplayImagePresenter` — Image display logic - `DeployedRevision` — The deployed commit from `GIT_SHA` (the DigitalOcean web service maps it to `${web.COMMIT_HASH}`); App settings shows a muted `build ` link to GitHub's compare view of everything merged to `main` since. In development it falls back to the checked-out commit; nil, and no link, when unset elsewhere - `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 their location's city (`RegistrationLocations` — work, else home), 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 their location's city (`PersonLocations` — work, else home), 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 f714f2aac2..97cf0591d4 100644 --- a/app/controllers/events_controller.rb +++ b/app/controllers/events_controller.rb @@ -244,7 +244,7 @@ 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.located_in(state: params[:state], county: params[:county], city: params[:city]) + scope = scope.located_in(on: @event.start_date, state: params[:state], county: params[:county], city: params[:city]) 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? @@ -866,12 +866,12 @@ def filtered_attendees scope.where(id: scholarship_status_person_ids(params[:scholarship])) end end - # Where people are (RegistrationLocations) on their in-scope registrations. The + # Where people are (PersonLocations) — on the filtered event's date, else today. The # City box, the State and County dropdowns and the countries drill-in resolve in # one pass, so they all describe the same address: someone working in two states # can't match a city from one and the county from the other. A State that # contradicts the county's own state matches nobody. - location_filters = RegistrationLocations.filters_from( + location_filters = PersonLocations.filters_from( state: params[:state], county: params[:county], city: params[:city], country: params[:country] ) if location_filters.nil? @@ -964,10 +964,10 @@ def person_linked_organization_ids(organization_id) .select(Arel.sql("event_registrations.registrant_id")) end - # Where each in-scope registration's registrant is, behind the location filters - # and drill-ins — the same source as the breakdowns they come from. + # Where each in-scope registrant is, behind the location filters and drill-ins — + # the same source as the breakdowns they come from. def attendee_locations - @attendee_locations ||= RegistrationLocations.new(attendee_registrations) + @attendee_locations ||= PersonLocations.new(attendee_registrations.select(:registrant_id), on: @filter_event&.start_date) end # Anchored the same way the index's own column is: this list spans events, so @@ -1013,7 +1013,7 @@ 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) - locations = RegistrationLocations.new(EventRegistration.where(event_id: reportable_events.select(:id))) + locations = PersonLocations.new(EventRegistration.where(event_id: reportable_events.select(:id)).select(:registrant_id)) @attendee_states = locations.candidate_values(:state) @attendee_counties = locations.candidate_values(:state, :county) @attendee_topic_types = TopicSubscriptionType.active.ordered diff --git a/app/models/event_registration.rb b/app/models/event_registration.rb index 5b895ce609..ef85353fa4 100644 --- a/app/models/event_registration.rb +++ b/app/models/event_registration.rb @@ -220,16 +220,15 @@ class EventRegistration < ApplicationRecord end scope } - # Where the registrant is on the registration — work, else home - # (RegistrationLocations). Takes any of its LOCATION_FIELDS, and they arrive - # together in one call so they test the same location row: a registrant working - # in two states would otherwise match a city from one and the state from the - # other. - scope :located_in, ->(**filters) { - resolved = RegistrationLocations.filters_from(**filters) + # Where the registrant is on `on` (the event's start date) — work, else home + # (PersonLocations). Takes any of its LOCATION_FIELDS, and they arrive together + # in one call so they test the same location row: a registrant working in two + # states would otherwise match a city from one and the state from the other. + scope :located_in, ->(on: nil, **filters) { + resolved = PersonLocations.filters_from(**filters) next none if resolved.nil? next all if resolved.empty? - where(id: RegistrationLocations.new(all).matching(**resolved).map(&:registration_id)) + where(registrant_id: PersonLocations.new(all.select(:registrant_id), on:).matching(**resolved).map(&:registrant_id)) } 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 a24630a7f8..953b58913f 100644 --- a/app/presenters/registrant_city_breakdown.rb +++ b/app/presenters/registrant_city_breakdown.rb @@ -1,10 +1,10 @@ -# Groups an event's registrants by their location's city (RegistrationLocations), +# Groups an event's registrants by their location's city (PersonLocations), # 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. +# A registrant working in two cities counts under each; the "Unknown" +# bucket (an address with no city) always sorts last. class RegistrantCityBreakdown UNKNOWN_LABEL = "Unknown".freeze diff --git a/app/services/attendees_breakdowns.rb b/app/services/attendees_breakdowns.rb index 2f81b0395b..8e2d3c6f3d 100644 --- a/app/services/attendees_breakdowns.rb +++ b/app/services/attendees_breakdowns.rb @@ -297,7 +297,7 @@ def person_ids_by(rows) end def locations - @locations ||= RegistrationLocations.new(scoped_registrations) + @locations ||= PersonLocations.new(scoped_registrations.select(:registrant_id), on: @as_of) end def org_registrant_pairs diff --git a/app/services/event_dashboard.rb b/app/services/event_dashboard.rb index 793fcf38b6..64a5606905 100644 --- a/app/services/event_dashboard.rb +++ b/app/services/event_dashboard.rb @@ -850,7 +850,7 @@ def referral_source_counts .sort_by { |label, count| [ -count, label ] } end - # The US states registrants are in — work, else home (RegistrationLocations). + # The US states registrants are in on the event date — work, else home (PersonLocations). def states @states ||= state_registrant_ids_by_state.keys.sort end @@ -867,7 +867,7 @@ def state_registrant_ids_by_state locations.registrant_ids_by_state end - # The countries registrants are in (RegistrationLocations). + # The countries registrants are in (PersonLocations). def countries @countries ||= country_registrant_ids_by_country.keys.sort end @@ -948,7 +948,7 @@ def life_experience_registrant_ids_by_category private def locations - @locations ||= RegistrationLocations.new(active_registrations) + @locations ||= PersonLocations.new(active_registrations.select(:registrant_id), on: reference_date) end # [ [ district, registrant_id ], ... ] from the affiliations linked to this event's diff --git a/app/services/event_participation_report.rb b/app/services/event_participation_report.rb index 5739ebef5a..53cf853dd3 100644 --- a/app/services/event_participation_report.rb +++ b/app/services/event_participation_report.rb @@ -183,8 +183,8 @@ def prior_year # The reach of the scoped registrants: how many distinct organizations, # sectors, states, countries and cities they span, for a given calendar year # (nil = every year in scope). Read "via registrants", like the event - # dashboard's breakdown cards. States/countries come from registrant addresses; - # cities come from the linked organizations' addresses. + # dashboard's breakdown cards. States, countries, and cities are where each + # registrant is today (PersonLocations — work, else home). Demographics = Struct.new(:organizations, :sectors, :states, :countries, :cities, keyword_init: true) def demographics(year: nil) @@ -194,19 +194,13 @@ def demographics(year: nil) .where(event_registration_id: registrations.select(:id)) .distinct .pluck(:organization_id) - addresses = Address.active - .where(addressable_type: "Person", addressable_id: person_ids) - .pluck(:state, :country) + locations = PersonLocations.new(person_ids) Demographics.new( organizations: organization_ids.size, sectors: SectorableItem.where(sectorable_type: "Person", sectorable_id: person_ids).distinct.count(:sector_id), - states: addresses.map(&:first).reject(&:blank?).uniq.size, - countries: addresses.map(&:last).reject(&:blank?).uniq.size, - cities: Address.active - .where(addressable_type: "Organization", addressable_id: organization_ids) - .where.not(city: [ nil, "" ]) - .distinct - .count(:city) + states: locations.registrant_ids_by(:state).size, + countries: locations.registrant_ids_by(:country).size, + cities: locations.registrant_ids_by(:city).size ) end diff --git a/app/services/registration_locations.rb b/app/services/person_locations.rb similarity index 54% rename from app/services/registration_locations.rb rename to app/services/person_locations.rb index 493e48bb46..57379ccf24 100644 --- a/app/services/registration_locations.rb +++ b/app/services/person_locations.rb @@ -1,14 +1,12 @@ -# Where each registrant is, per registration — where they work, else where they -# live: 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; with no such address, the one address that -# stands for the registrant themselves (Address#ranked_for_owner — active work, -# else latest-edited active personal, else inactive personal, else inactive work). -# The source for every registrant location -# chart, count, and filter. A linked org with no address and no own address to -# fall back on yields a row with no location, so the cities breakdown can count -# it as Unknown. -class RegistrationLocations +# Where each person is on a date — where they work, else where they live: every +# affiliation active on that date (today by default), at its picked address +# (Affiliation#organization_address), else its org's oldest active address; with +# none of those, the one address that stands for the person themselves +# (Address#ranked_for_owner — active work, else latest-edited active personal, +# else inactive personal, else inactive work). Event pages pass the event's start +# date so a past event doesn't move when someone changes jobs. The source for +# every person location chart, count, and filter. +class PersonLocations # Everything an address says about *where* it is — the whole record bar the # street and phone, which identify a building rather than place it. A row # carries all of them so a new chart, count, or filter needs only a field name. @@ -26,7 +24,7 @@ class RegistrationLocations # on the attendees index. COUNTY_STATE_SEPARATOR = /::|\|/ - Row = Struct.new(:registration_id, :organization_id, :registrant_id, *LOCATION_FIELDS, keyword_init: true) do + Row = Struct.new(:organization_id, :registrant_id, *LOCATION_FIELDS, keyword_init: true) do def city_label [ city, state ].compact_blank.join(", ").presence end @@ -49,23 +47,26 @@ def self.split_county(value) county.nil? ? [ nil, state ] : [ state, county ] end - # Takes the registrations to locate, as a relation (kept as a subquery, so a + # Takes the people to locate, as a relation (kept as a subquery, so a # whole-population lookup never inlines every id) or as an explicit id array. - def initialize(registrations) - @registrations = if registrations.is_a?(ActiveRecord::Relation) - registrations.except(:includes, :eager_load, :preload, :order, :select, :limit, :offset) + def initialize(people, on: nil) + @person_ids = if people.is_a?(ActiveRecord::Relation) + people.except(:includes, :eager_load, :preload, :order, :limit, :offset) else - EventRegistration.where(id: registrations) + people end + @on = on end def rows - @rows ||= registration_pairs.flat_map do |registration_id, registrant_id| - located = organization_ids_by_registration.fetch(registration_id, []).map do |organization_id| - [ organization_id, chosen_addresses[chosen_address_ids[[ registration_id, organization_id ]]] || oldest_address_by_organization[organization_id] ] + @rows ||= begin + work = affiliation_triples.filter_map do |person_id, organization_id, address_id| + address = chosen_addresses[address_id] || oldest_address_by_organization[organization_id] + row(organization_id, person_id, address) if address end - located = [ [ nil, own_address_by_person[registrant_id] ] ] if located.none?(&:last) && own_address_by_person[registrant_id] - located.map { |organization_id, address| row(registration_id, organization_id, registrant_id, address) } + located = work.map(&:registrant_id).to_set + home = own_address_by_person.except(*located).map { |person_id, address| row(nil, person_id, address) } + (work + home).uniq end end @@ -99,14 +100,14 @@ def registrant_ids_by_city_label end # Distinct values of one or more location fields across every address these - # registrations could resolve to — the linked organizations' and the - # registrants' own — for a filter's option list. Two grouped queries rather - # than a row per registration, so the attendees index can offer options drawn - # from its whole reportable population. It skips the work-else-home precedence - # #rows applies, so it can offer a value no registration settles on; an option - # list is already deliberately wider than the active filters. + # people could resolve to — their affiliated organizations' and their own — for + # a filter's option list. Two grouped queries rather than a row per person, so + # the attendees index can offer options drawn from its whole reportable + # population. It skips the work-else-home precedence #rows applies, so it can + # offer a value no one settles on; an option list is already deliberately wider + # than the active filters. def candidate_values(*fields) - [ organization_addresses, registrant_addresses ].flat_map { |scope| + [ organization_addresses, person_addresses ].flat_map { |scope| fields.reduce(scope) { |relation, field| relation.where.not(field => [ nil, "" ]) }.distinct.pluck(*fields) }.uniq.sort end @@ -127,52 +128,35 @@ def group_registrant_ids(rows, &key) rows.group_by(&key).transform_values { |grouped| grouped.map(&:registrant_id).uniq } end - def row(registration_id, organization_id, registrant_id, address) - Row.new(registration_id:, organization_id:, registrant_id:, - **LOCATION_FIELDS.index_with { |field| address&.public_send(field) }) + def row(organization_id, registrant_id, address) + Row.new(organization_id:, registrant_id:, **LOCATION_FIELDS.index_with { |field| address&.public_send(field) }) end - def registration_pairs - @registration_pairs ||= @registrations.pluck(:id, :registrant_id) + def affiliations + scope = @on ? Affiliation.active_by_date_on(@on) : Affiliation.active + scope.where(person_id: @person_ids) end - def registration_ids - @registrations.select(:id) - end - - def organization_ids_by_registration - @organization_ids_by_registration ||= EventRegistrationOrganization - .where(event_registration_id: registration_ids) - .pluck(:event_registration_id, :organization_id) - .group_by(&:first) - .transform_values { |pairs| pairs.map(&:last) } - 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 ] } + def affiliation_triples + @affiliation_triples ||= affiliations.distinct.pluck(:person_id, :organization_id, :organization_address_id) end def chosen_addresses @chosen_addresses ||= Address.active - .where(id: Affiliation.where(event_registration_id: registration_ids).select(:organization_address_id)) + .where(id: affiliations.select(:organization_address_id)) .select(:id, *LOCATION_FIELDS) .index_by(&:id) end def organization_addresses - Address.active.where(addressable_type: "Organization", - addressable_id: EventRegistrationOrganization.where(event_registration_id: registration_ids).select(:organization_id)) + Address.active.where(addressable_type: "Organization", addressable_id: affiliations.select(:organization_id)) end - # Not narrowed to active: a registrant with nothing but a retired address is - # better placed there than left out of the map entirely (Address#ranked_for_owner - # ranks retired addresses last). - def registrant_addresses - Address.where(addressable_type: "Person", addressable_id: @registrations.select(:registrant_id)) + # Not narrowed to active: a person with nothing but a retired address is better + # placed there than left out of the map entirely (Address#ranked_for_owner ranks + # retired addresses last). + def person_addresses + Address.where(addressable_type: "Person", addressable_id: @person_ids) end def oldest_address_by_organization @@ -183,7 +167,7 @@ def oldest_address_by_organization end def own_address_by_person - @own_address_by_person ||= registrant_addresses + @own_address_by_person ||= person_addresses .ranked_for_owner .select(:id, :addressable_id, :inactive, :address_type, :updated_at, *LOCATION_FIELDS) .each_with_object({}) { |address, map| map[address.addressable_id] ||= address } diff --git a/app/services/reminder_recipient_filter.rb b/app/services/reminder_recipient_filter.rb index ca33bafb95..68fced6467 100644 --- a/app/services/reminder_recipient_filter.rb +++ b/app/services/reminder_recipient_filter.rb @@ -99,7 +99,7 @@ def location_matched_ids def located_in_ids(city) EventRegistration.where(id: all_registration_ids.to_a) - .located_in(state: @params[:state], county: @params[:county], city: city) + .located_in(on: @event&.start_date, state: @params[:state], county: @params[:county], city: city) .pluck(:id) .to_set end 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 cb2c267db2..db287e6f78 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 @@ -4,11 +4,12 @@ display_status: admin_facing released_on: 2026-10-25 pr_number: 2844 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. + The cities, states, and countries charts, the City, State, and County + filters on the roster, the attendees page, and the bulk email picker, and + the participation report's reach figures now use where each person works: + the addresses of the organizations they're affiliated with. 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 address on their registration counts where their own address puts them: their active work address, else their most recently edited active personal one, else a retired personal, else a retired work. A retired address still counts — better to place someone than to leave them off the map." - - "City, State, County and Country always describe the same address, so someone registered under organizations in two states won't show up for a city in one and a state in the other." + - "On an event's pages, it's the affiliations active on the event's start date, so a past event doesn't change when someone changes jobs. Across events and on the reports, it's today's active affiliations." + - "Every active affiliation counts. If one has a work address picked, that one is used; otherwise the organization's first address on file." + - "Someone with no affiliation address counts where their own address puts them: their active work address, else their most recently edited active personal one, else a retired personal, else a retired work." + - "City, State, County and Country always describe the same address, so someone working in two states won't show up for a city in one and a state in the other." diff --git a/spec/models/event_registration_spec.rb b/spec/models/event_registration_spec.rb index 1d3bcb4424..8d4991bf5d 100644 --- a/spec/models/event_registration_spec.rb +++ b/spec/models/event_registration_spec.rb @@ -605,7 +605,7 @@ 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:) + create(:affiliation, person: registration.registrant, organization:) registration end @@ -618,7 +618,7 @@ def registration_working_in(city:, state:, county: nil) expect(EventRegistration.located_in(city: "santa")).to contain_exactly(santa_monica) end - it "falls back to the registrant's own address when no org is linked" do + it "falls back to the registrant's own address with no active affiliation" do unlinked = create(:event_registration) create(:address, addressable: unlinked.registrant, city: "Santa Barbara", state: "CA") @@ -645,7 +645,7 @@ def registration_working_in(city:, state:, county: nil) [ [ "Portland", "OR", "Multnomah" ], [ "Seattle", "WA", "King" ] ].each do |city, state, county| organization = create(:organization) create(:address, addressable: organization, city:, state:, county:) - two_states.event_registration_organizations.create!(organization:) + create(:affiliation, person: two_states.registrant, organization:) end expect(EventRegistration.located_in(city: "Seattle", state: "WA")).to contain_exactly(two_states) diff --git a/spec/requests/events/attendees_spec.rb b/spec/requests/events/attendees_spec.rb index 9982085401..7669cfe635 100644 --- a/spec/requests/events/attendees_spec.rb +++ b/spec/requests/events/attendees_spec.rb @@ -341,10 +341,10 @@ expect(response.body).to include("Online Search") end - it "filters by work country and state (via the org they registered under)" do + it "filters by work country and state (via their active affiliations)" 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) + create(:affiliation, person: attendee_registration.registrant, organization: org) other = create(:person, first_name: "Zed", last_name: "Zulu") create(:address, addressable: other, state: "OR", country: "Mexico", inactive: false) create(:event_registration, event: recent_training, registrant: other, status: "attended") @@ -381,13 +381,13 @@ it "renders the cities breakdown and filters by an org-city drill-in" do org = create(:organization, name: "Wellness Org") create(:address, addressable: org, city: "Austin", state: "TX", inactive: false) - attendee_registration.event_registration_organizations.create!(organization: org) + create(:affiliation, person: attendee_registration.registrant, organization: org) other = create(:person, first_name: "Zed", last_name: "Zulu") other_registration = create(:event_registration, event: recent_training, registrant: other, status: "attended") other_org = create(:organization, name: "Other Org") create(:address, addressable: other_org, city: "Reno", state: "NV", inactive: false) - other_registration.event_registration_organizations.create!(organization: other_org) + create(:affiliation, person: other_registration.registrant, organization: other_org) # An org with no address falls into the non-clickable "Unknown" bucket — # this used to raise a UrlGenerationError (nil event) on the index. @@ -411,7 +411,7 @@ # that raised a nil-event UrlGenerationError). org = create(:organization, name: "Wellness Org") create(:address, addressable: org, city: "Austin", state: "TX", inactive: false) - attendee_registration.event_registration_organizations.create!(organization: org) + create(:affiliation, person: attendee_registration.registrant, organization: org) award = create(:scholarship, recipient: attendee, amount_cents: 1_000, grant: create(:grant)) create(:allocation, source: award, allocatable: attendee_registration, amount: 1_000) @@ -551,13 +551,13 @@ 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) + create(:affiliation, person: attendee_registration.registrant, organization: org) other = create(:person, first_name: "Cara", last_name: "Coast") other_registration = create(:event_registration, event: recent_training, registrant: other, status: "attended") create(:address, addressable: other, city: "Portland", state: "OR", inactive: false) other_org = create(:organization) create(:address, addressable: other_org, city: "Seattle", state: "WA", inactive: false) - other_registration.event_registration_organizations.create!(organization: other_org) + create(:affiliation, person: other_registration.registrant, organization: other_org) get attendees_events_url(city: "portland"), headers: frame_headers expect(response.body).to include("Ada Lovelace") @@ -571,7 +571,7 @@ [ [ "Portland", "OR", "Multnomah", "United States" ], [ "Vancouver", "BC", "Greater Vancouver", "Canada" ] ].each do |city, state, county, country| org = create(:organization) create(:address, addressable: org, city:, state:, county:, country:, inactive: false) - attendee_registration.event_registration_organizations.create!(organization: org) + create(:affiliation, person: attendee_registration.registrant, organization: org) end get attendees_events_url(city: "Vancouver", state: "BC"), headers: frame_headers diff --git a/spec/requests/events/bulk_reminders_spec.rb b/spec/requests/events/bulk_reminders_spec.rb index 9bc9347dd0..3122ec443a 100644 --- a/spec/requests/events/bulk_reminders_spec.rb +++ b/spec/requests/events/bulk_reminders_spec.rb @@ -50,10 +50,10 @@ def checked?(body, registration) 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(:affiliation, person: jane.registrant, organization:) elsewhere = create(:organization) create(:address, addressable: elsewhere, city: "Portland") - sam.event_registration_organizations.create!(organization: elsewhere) + create(:affiliation, person: sam.registrant, organization: elsewhere) create(:address, addressable: sam.registrant, city: "Santa Monica") get preview_reminder_event_path(event, city: "santa"), diff --git a/spec/requests/events_spec.rb b/spec/requests/events_spec.rb index d306af0968..b23aff9a9b 100644 --- a/spec/requests/events_spec.rb +++ b/spec/requests/events_spec.rb @@ -1785,10 +1785,12 @@ def pay_for_others_classes(template) 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: here) + create(:affiliation, person: here, organization: here_org) there_org = create(:organization) create(:address, addressable: there_org, city: "Portland") - create(:event_registration, event: event, registrant: there).event_registration_organizations.create!(organization: there_org) + create(:event_registration, event: event, registrant: there) + create(:affiliation, person: there, organization: there_org) get registrants_event_path(event, city: "santa") @@ -1805,7 +1807,7 @@ def pay_for_others_classes(template) [ %w[Portland OR], %w[Seattle WA] ].each do |city, state| organization = create(:organization) create(:address, addressable: organization, city: city, state: state) - registration.event_registration_organizations.create!(organization: organization) + create(:affiliation, person: registration.registrant, organization: organization) end get registrants_event_path(event, city: "Seattle", state: "WA") @@ -2602,7 +2604,7 @@ def ce_chip_text { 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:) + create(:affiliation, person: registration.registrant, organization:) end sign_in admin end @@ -3455,10 +3457,10 @@ def ce_chip_text expect(response.body).not_to include("Only scholarship orgs") end - it "groups registrants by the city of the org linked on their registration" do + it "groups registrants by the city they work in" do city_org = create(:organization, name: "City Org") create(:address, addressable: city_org, city: "Los Angeles", state: "CA", inactive: false) - create(:event_registration_organization, event_registration: registration, organization: city_org) + create(:affiliation, person: person, organization: city_org) scholarship = create(:scholarship, recipient: person, amount_cents: 500) create(:allocation, source: scholarship, allocatable: registration, amount: 500) @@ -3676,11 +3678,10 @@ def ce_chip_text expect(response.body).to include("Billed to original event") end - it "shows a recipient city breakdown, grouped by the registration-linked org, in the lazy charts frame" do + it "shows a recipient city breakdown, grouped by where they work, in the lazy charts frame" do org = create(:organization, name: "Reach Org") create(:address, addressable: org, city: "Richmond", state: "CA", inactive: false) - registration = EventRegistration.find_by!(registrant: applicant, event: event) - create(:event_registration_organization, event_registration: registration, organization: org) + create(:affiliation, person: applicant, organization: org) get recipients_event_path(event), headers: { "Turbo-Frame" => "recipients_charts" } diff --git a/spec/services/attendees_breakdowns_spec.rb b/spec/services/attendees_breakdowns_spec.rb index e5f1719657..e62821f7a8 100644 --- a/spec/services/attendees_breakdowns_spec.rb +++ b/spec/services/attendees_breakdowns_spec.rb @@ -19,11 +19,11 @@ 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 + it "counts US states and countries from where they work (active affiliations)" 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) + create(:affiliation, person: registration.registrant, organization: organization) expect(breakdowns.state_counts).to eq("CA" => 1) expect(breakdowns.country_counts).to eq("United States" => 1) @@ -36,10 +36,10 @@ expect(breakdowns.organization_counts).to eq(organization.id => 1) end - it "groups attendees by the city of the org linked on their training registration" do + it "groups attendees by the city they work in" do organization = create(:organization, name: "Wellness Org") create(:address, addressable: organization, city: "Austin", state: "TX", inactive: false) - registration.event_registration_organizations.create!(organization: organization) + create(:affiliation, person: registration.registrant, organization: organization) rows = breakdowns.registrant_city_breakdown.rows expect(rows.map(&:city)).to eq([ "Austin, TX" ]) @@ -145,6 +145,7 @@ def answer(who, event, value) organization = create(:organization, name: "Wellness Org") create(:address, addressable: organization, state: "CA", inactive: false) registration.event_registration_organizations.create!(organization: organization) + create(:affiliation, person: registration.registrant, 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_participation_report_spec.rb b/spec/services/event_participation_report_spec.rb index f4626ba736..8a405d41b7 100644 --- a/spec/services/event_participation_report_spec.rb +++ b/spec/services/event_participation_report_spec.rb @@ -143,7 +143,7 @@ registration = create(:event_registration, event: event, registrant: person, status: "attended") create(:event_registration_organization, event_registration: registration, organization: organization) create(:sectorable_item, sectorable: person, sector: create(:sector)) - create(:address, addressable: person, state: "CA", country: "US") + create(:address, addressable: person, city: "Fresno", state: "CA", country: "US") create(:address, addressable: organization, city: "Los Angeles") end @@ -155,8 +155,16 @@ expect(reach.countries).to eq(1) end - it "counts distinct cities from the linked organizations' addresses" do - expect(report.demographics.cities).to eq(1) + it "counts states, countries, and cities where registrants work, else live" do + worker = create(:person) + create(:event_registration, event: event, registrant: worker, status: "attended") + create(:affiliation, person: worker, organization: organization) + organization.addresses.first.update!(state: "NV", country: "Canada") + + reach = report.demographics + expect(reach.states).to eq(2) + expect(reach.countries).to eq(2) + expect(reach.cities).to eq(2) end end diff --git a/spec/services/registration_locations_spec.rb b/spec/services/person_locations_spec.rb similarity index 68% rename from spec/services/registration_locations_spec.rb rename to spec/services/person_locations_spec.rb index 383c10cf0a..84bdd2edd3 100644 --- a/spec/services/registration_locations_spec.rb +++ b/spec/services/person_locations_spec.rb @@ -1,19 +1,16 @@ require "rails_helper" -RSpec.describe RegistrationLocations do - let(:event) { create(:event) } +RSpec.describe PersonLocations do let(:person) { create(:person) } - let(:registration) { create(:event_registration, event: event, registrant: person) } let(:organization) { create(:organization) } + let!(:affiliation) { create(:affiliation, person: person, organization: organization) } - subject(:locations) { described_class.new([ registration.id ]) } + subject(:locations) { described_class.new([ person.id ]) } - before { registration.event_registration_organizations.create!(organization: organization) } - - it "uses the address picked on the registration's affiliation" do + it "uses the address picked on the 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) + affiliation.update!(organization_address: chosen) expect(locations.rows.map(&:city_label)).to eq([ "Fresno, CA" ]) end @@ -26,65 +23,89 @@ expect(locations.rows.map(&:city_label)).to eq([ "Reno, NV" ]) end - it "prefers the work address over the registrant's own" do + it "counts every active affiliation" do + create(:address, addressable: organization, city: "Reno", state: "NV") + other = create(:organization) + create(:address, addressable: other, city: "Elko", state: "NV") + create(:affiliation, person: person, organization: other) + + expect(locations.rows.map(&:city_label)).to contain_exactly("Reno, NV", "Elko, NV") + end + + it "prefers the work address over the person's own" do create(:address, addressable: organization, city: "Reno", state: "NV") create(:address, addressable: person, city: "Boise", state: "ID") expect(locations.rows.map(&:city_label)).to eq([ "Reno, NV" ]) end + it "skips an ended affiliation today" do + create(:address, addressable: organization, city: "Reno", state: "NV") + affiliation.update!(end_date: 1.month.ago.to_date) + create(:address, addressable: person, city: "Boise", state: "ID") + + expect(locations.rows.map(&:city_label)).to eq([ "Boise, ID" ]) + end + + describe "on a date" do + before do + create(:address, addressable: organization, city: "Reno", state: "NV") + create(:address, addressable: person, city: "Boise", state: "ID") + end + + it "counts an affiliation that was active then, even if it has ended since" do + affiliation.update!(start_date: Date.new(2024, 1, 1), end_date: Date.new(2025, 1, 1)) + + expect(described_class.new([ person.id ], on: Date.new(2024, 6, 1)).rows.map(&:city_label)).to eq([ "Reno, NV" ]) + end + + it "skips an affiliation that started after it" do + affiliation.update!(start_date: Date.new(2025, 1, 1)) + + expect(described_class.new([ person.id ], on: Date.new(2024, 6, 1)).rows.map(&:city_label)).to eq([ "Boise, ID" ]) + end + end + # Address#ranked_for_owner: active work, else latest-edited active personal, # else inactive personal, else inactive work. - it "falls back to the address that stands for the registrant" do + it "falls back to the address that stands for the person" do + affiliation.destroy! create(:address, addressable: person, city: "Boise", state: "ID", address_type: "personal") create(:address, addressable: person, city: "Nampa", state: "ID", address_type: "work") - expect(locations.rows.map(&:city_label)).to eq([ "Nampa, ID" ]) + expect(locations.rows.map { |row| [ row.city_label, row.organization_id ] }).to eq([ [ "Nampa, ID", nil ] ]) end - it "places a registrant by a retired address rather than nowhere" do + it "places a person by a retired address rather than nowhere" do create(:address, addressable: person, city: "Boise", state: "ID", address_type: "personal", inactive: true) expect(locations.rows.map(&:city_label)).to eq([ "Boise, ID" ]) end - it "falls back to the registrant's own address when no org is linked" do - unlinked = create(:event_registration, event: event) - create(:address, addressable: unlinked.registrant, city: "Boise", state: "ID") - - rows = described_class.new([ unlinked.id ]).rows - expect(rows.map { |row| [ row.city_label, row.organization_id ] }).to eq([ [ "Boise, ID", nil ] ]) - end - - it "leaves a located-nowhere linked registration as an Unknown row" do - expect(locations.rows.map(&:city_label)).to eq([ nil ]) - end - - it "has no row for an unlinked registrant with no address" do - expect(described_class.new([ create(:event_registration, event: event).id ]).rows).to be_empty + it "has no row for someone with no work or own address" do + expect(locations.rows).to be_empty end # A relation stays a subquery, so a whole-population lookup never inlines every - # registration id (the attendees index builds its filter options that way). + # id (the attendees index builds its filter options that way). it "keeps a relation as a subquery instead of plucking its ids" do create(:address, addressable: organization, city: "Austin", state: "TX") queries = [] subscriber = ->(*, payload) { queries << payload[:sql] } rows = ActiveSupport::Notifications.subscribed(subscriber, "sql.active_record") do - described_class.new(EventRegistration.where(event: event)).rows + described_class.new(Person.where(id: person.id)).rows end expect(rows.map(&:city_label)).to eq([ "Austin, TX" ]) - expect(queries.grep(/event_registration_organizations/).first) - .to include("SELECT `event_registrations`.`id` FROM `event_registrations`") + expect(queries.grep(/FROM `affiliations`/).first).to include("SELECT `people`.`id` FROM `people`") 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) + create(:affiliation, person: person, organization: abroad) expect(locations.registrant_ids_by_state).to eq("TX" => [ person.id ]) expect(locations.registrant_ids_by(:country)).to eq("United States" => [ person.id ], "Canada" => [ person.id ]) @@ -105,10 +126,10 @@ create(:address, addressable: organization, city: "Austin", state: "TX", county: "Travis", country: "United States") abroad = create(:organization) create(:address, addressable: abroad, city: "Toronto", state: "ON", county: "York", country: "Canada") - registration.event_registration_organizations.create!(organization: abroad) + create(:affiliation, person: person, organization: abroad) end - # One call, not two: chaining them is what lets a registrant working in two + # One call, not two: chaining them is what lets someone working in two # places answer a city from one and a county from the other. it "tests every field against the same address" do expect(locations.matching(city: "toronto", country: "Canada")).to be_present @@ -146,8 +167,8 @@ describe "#candidate_values" do # Option lists are built from the whole reportable population, so they read the - # address tables directly rather than resolving a row per registration. - it "lists the values the linked orgs' and registrants' addresses carry" do + # address tables directly rather than resolving a row per person. + it "lists the values the affiliated orgs' and people's own addresses carry" do create(:address, addressable: organization, state: "TX", county: "Travis") create(:address, addressable: person, state: "ID", county: "Ada") diff --git a/spec/services/reminder_recipient_filter_spec.rb b/spec/services/reminder_recipient_filter_spec.rb index d9e53d4c68..0efd38f3e8 100644 --- a/spec/services/reminder_recipient_filter_spec.rb +++ b/spec/services/reminder_recipient_filter_spec.rb @@ -99,11 +99,11 @@ def matched(params, registrations) works_there = registration organization = create(:organization) create(:address, addressable: organization, city: "Santa Monica") - works_there.event_registration_organizations.create!(organization:) + create(:affiliation, person: works_there.registrant, organization:) works_elsewhere = registration(first_name: "Sam") elsewhere = create(:organization) create(:address, addressable: elsewhere, city: "Portland") - works_elsewhere.event_registration_organizations.create!(organization: elsewhere) + create(:affiliation, person: works_elsewhere.registrant, organization: elsewhere) create(:address, addressable: works_elsewhere.registrant, city: "Santa Monica") lives_there = registration(first_name: "Lia") create(:address, addressable: lives_there.registrant, city: "Santa Monica") @@ -120,7 +120,7 @@ def matched(params, registrations) [ [ "Portland", "OR" ], [ "Seattle", "WA" ] ].each do |city, state| organization = create(:organization) create(:address, addressable: organization, city:, state:) - two_states.event_registration_organizations.create!(organization:) + create(:affiliation, person: two_states.registrant, organization:) end expect(matched({ city: "seattle", state: "WA" }, [ two_states ])).to eq([ two_states.id ].to_set) From f89f492316c2551a687da22447ffa71e1d36977b Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 14:20:45 -0400 Subject: [PATCH 9/9] Label the city and country chips without "Org" Both now read PersonLocations, which falls back to a person's own address, so "Org city" / "Org country" could label a home-address match as an org one. Closes #2845 Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 +- app/services/attendees_active_filters.rb | 4 ++-- spec/requests/events/attendees_spec.rb | 4 ++-- spec/services/attendees_active_filters_spec.rb | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 729b88eb4d..09a0aee7c0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -238,7 +238,7 @@ comparison — add a rule if none fits (e.g. `PersonPolicy#own_profile?`). Conve - `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 - `PersonLocations` — Where each person is on a date (`on:`, default today) — where they work, else where they live: every affiliation active on that date (`Affiliation.active_by_date_on`, or `.active` for today), at its picked address (`Affiliation#organization_address`), else its org's oldest active address — every active affiliation counts; with none, the one address that stands for the person themselves (`Address.ranked_for_owner` / `Address::STANDING` — active work, else latest-edited active personal, else inactive personal, else inactive work; work and personal swap below the active line because a current work address is the sharpest thing we know and a retired one the stalest. Written once in Ruby and compiled into the ORDER BY; `AddressDecorator#saved_address` sorts a loaded association by the same list. Retired addresses count, because placing someone by a stale address beats leaving them off the map). Each row carries every `LOCATION_FIELDS` value the address holds — city, county, state, country, locality, zip, school district and the three LA districts, i.e. everything bar the street and phone — so a new chart, count or filter needs only a field name: `#registrant_ids_by(field)` groups ids by any of them (`#registrant_ids_by_state` is US-only for the map, `#registrant_ids_by_city_label` keys on "City, State"), and `#matching(**fields)` tests ONE address per row against the whole filter set at once — chaining two calls is what lets someone working in two places answer a city from one and a county from the other. `.filters_from(**params)` folds a County option's own state ("CA|Los Angeles" on the roster, "CA::Los Angeles" on the attendees index) into `state:` and returns nil when a State filter contradicts it. `#candidate_values(*fields)` answers filter option lists straight from the org/registrant address tables (two grouped queries, a superset) so a whole-population dropdown never builds a row per person. Takes a person-id relation (kept as a subquery, so a whole-population lookup never inlines every id) or an explicit id array. Event pages pass the event's start date so a past event doesn't move when someone changes jobs; cross-event pages and reports use today. The single source for every person location figure and filter: the charts' cities, states, and countries (`EventDashboard` on the event date, `AttendeesBreakdowns` on `as_of`), the roster/reminder City/State/County filters (`EventRegistration.located_in(on:, ...)`, which takes them all at once, and `ReminderRecipientFilter`), the attendees index's City/State/County filters and `country`/`org_city` drill-ins (the filtered event's date, else today), and `EventParticipationReport#demographics` (today) - `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 `PersonLocations` — 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 +- `AttendeesActiveFilters` — Human-readable, removable-chip descriptors for a page's "drill-in" filters (registrant_ids, organization, 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 - `EventAttendanceReport` — Per-event attendance sign-in/out report from `EventAttendanceTimeEntry`, grouped by day then reported line, with per-day and grand-total minutes; `ce_only:` scopes to CE registrants and surfaces license number + awarded hours. The reported unit is a `Row` (registration + optional `ContinuingEducationRegistration`), so a registrant claiming CE against two licences gets a line each, keyed by `Row#key` — which is what `attendance_cell_id` and the editor's `row` param use. The times behind two lines are shared, so the everyone-totals dedupe by registration while hours awarded sum per line. The in-portal CE hour sign-in sheet, linked from the participation report (`?ce=true`) at `attendance_event_path` diff --git a/app/services/attendees_active_filters.rb b/app/services/attendees_active_filters.rb index a2a836629e..df5982ef52 100644 --- a/app/services/attendees_active_filters.rb +++ b/app/services/attendees_active_filters.rb @@ -46,10 +46,10 @@ def label_for(param, value) when "registrant_ids" then "#{value.to_s.split("-").size} selected people" 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_city" then "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/spec/requests/events/attendees_spec.rb b/spec/requests/events/attendees_spec.rb index 7669cfe635..d28f91c93f 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 @@ -688,7 +688,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/services/attendees_active_filters_spec.rb b/spec/services/attendees_active_filters_spec.rb index babeb98101..150572d1b4 100644 --- a/spec/services/attendees_active_filters_spec.rb +++ b/spec/services/attendees_active_filters_spec.rb @@ -29,9 +29,9 @@ 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") + expect(chips_for(org_city: "Austin, TX").first[:label]).to eq("City: Austin, TX") end # Scholarship and CE status each have their own visible select on the index, so