From e96279df858d16de8b1c028a8fe6e6cdad6bea6b Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sat, 24 Oct 2026 18:18:52 -0400 Subject: [PATCH 01/10] Start: shared address translation for imports and cleanup Co-Authored-By: Claude Opus 5.5 (1M context) From 0e7e66dba9d6f19a9afd4d0eee125bd48bde8e17 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sat, 24 Oct 2026 18:22:10 -0400 Subject: [PATCH 02/10] Translate imported address values into what the dropdowns store FileMaker and LMS imports carried free-text states and countries ("California", "USA", "CA - California", provinces in the state field) that split one place into several and don't preselect in the dropdowns. Co-Authored-By: Claude Opus 5.5 (1M context) --- app/services/address_translation.rb | 146 ++++++++++++++++++ app/services/event_registration_importer.rb | 15 +- .../person_services/import_from_rolodex.rb | 26 ++-- lib/tasks/fm_import_people.rake | 7 + spec/services/address_translation_spec.rb | 130 ++++++++++++++++ .../event_registration_importer_spec.rb | 14 ++ .../import_from_rolodex_spec.rb | 27 ++++ 7 files changed, 351 insertions(+), 14 deletions(-) create mode 100644 app/services/address_translation.rb create mode 100644 spec/services/address_translation_spec.rb diff --git a/app/services/address_translation.rb b/app/services/address_translation.rb new file mode 100644 index 0000000000..83e5e381d1 --- /dev/null +++ b/app/services/address_translation.rb @@ -0,0 +1,146 @@ +# Turns the address values other systems and older free-text forms wrote — FileMaker, the +# LMS registration sheet, the portal's own answers from before the dropdowns — into what the +# portal's address dropdowns store (ADR-0009). Translation is by lookup table, and a value +# nothing answers to is kept as typed and counted in #unmatched, never guessed at. +# +# Built once per run and reused, so an import's report can list everything it couldn't place. +class AddressTranslation + UNITED_STATES = "United States" + + # The words the archive and the forms use for a kind of address, phone, or email. + CONTACT_TYPES = { "home" => "personal", "personal" => "personal", "cell" => "personal", + "mobile" => "personal", "work" => "work", "business" => "work", + "professional" => "work", "mailing" => "mailing" }.freeze + + # Spellings found in our data that the countries gem doesn't read, by ISO code. A US city + # or county typed as the country is the US; the outlying islands aren't on the dropdown. + ALIASES = { + "unites states" => "US", "untied states" => "US", "los angeles" => "US", "la county" => "US", + "us minor outlying islands" => "US", "united states minor outlying islands" => "US", + "korea" => "KR", "republic of ireland" => "IE", "mexcio" => "MX", "scotland" => "GB", "republic of armenia" => "AM" + }.merge(GeographyHelper::PLAIN_COUNTRY_NAMES.to_h { |code, name| [ name.downcase, code ] }).freeze + + # Provinces and regions written in the state field, keyed as AddressNormalizer.words joins them. + REGIONS = { + "Canada" => [ "on", "ontario", "bc", "british columbia", "ab", "alberta", "ns", "nova scotia", "qc", "quebec", + "mb", "manitoba", "sk", "saskatchewan", "nb", "new brunswick", "nl", "newfoundland", + "newfoundland and labrador", "pe", "pei", "prince edward island", "yt", "yukon", "nu", "nunavut", + "northwest territories" ], + "Australia" => [ "nsw", "new south wales", "vic", "victoria", "qld", "queensland", "tas", "tasmania", + "south australia", "western australia" ], + "United Kingdom" => [ "england", "scotland", "wales", "northern ireland", "london", "bucks", "buckinghamshire", + "somerset", "argyle and bute", "argyll and bute" ], + "Vietnam" => [ "ha noi", "hanoi" ], + "Mexico" => [ "mexico city", "cdmx" ], + "Japan" => [ "tokyo" ], + "Philippines" => [ "metro manila" ], + "Ghana" => [ "greater accra" ], + "Trinidad and Tobago" => [ "trinidad", "tobago" ] + }.flat_map { |country, regions| regions.map { |region| [ region, country ] } }.to_h.freeze + + # Typed in place of a country to say there isn't one, keyed as AddressNormalizer.words joins them. + PLACEHOLDERS = [ "n a", "none", "unknown", "not applicable" ].to_set.freeze + + # A US state's full name anywhere in the value, so "CA - California" and "Maryland (MD)" read as a state. + US_STATE_NAME = /\b(#{AddressNormalizer::STATE_NAMES.keys.map { |name| Regexp.escape(name) }.join("|")})\b/ + + attr_reader :unmatched + + def initialize + @unmatched = Hash.new { |hash, field| hash[field] = Hash.new(0) } + end + + # A dropdown country: the one the value names, else the one its state implies, else the + # value as typed (reported). nil when it's blank or a placeholder and the state says nothing. + def country(value, state:) + text = value.to_s.strip + text = "" if placeholder?(text) + return UNITED_STATES if us_state_code(text) && us_state_code(state) + + found = country_named(text) || country_from_state(state) + return found if found + + @unmatched[:country][text] += 1 if text.present? + text.presence + end + + # A US state's USPS code; any other region as typed. A value that reads as neither, on an + # address in the US or with no country, is reported. + def state(value, country:) + text = value.to_s.strip + return if text.blank? + + abroad = country_named(country.to_s.strip).then { |named| named.present? && named != UNITED_STATES } + return text if abroad + + code = us_state_code(text) + return code.upcase if code + + @unmatched[:state][text] += 1 unless country_from_state(text) + text + end + + def address_type(value) + CONTACT_TYPES[value.to_s.strip.downcase].presence_in(Address::CONTACT_TYPES) + end + + def locality(city:, state:, country:, county: nil) + AddressLocality.for(city:, state:, country:, county:) + end + + # Address attributes with their state, country, and type translated and a locality worked out. + def address(attributes) + state = state(attributes[:state], country: attributes[:country]) + country = country(attributes[:country], state: attributes[:state]) + attributes.merge(state:, country:, address_type: address_type(attributes[:address_type]), + locality: locality(city: attributes[:city], state:, country:, county: attributes[:county])) + end + + private + + def country_named(text) + return if text.blank? + + code = ALIASES[text.downcase] + code ||= "US" if AddressNormalizer::US_COUNTRY_NAMES.include?(text.downcase.delete("^a-z")) + found = code ? ISO3166::Country.new(code) : gem_country(text) + name_for(found) if found + end + + def gem_country(text) + ISO3166::Country.find_country_by_alpha2(text) || + ISO3166::Country.find_country_by_alpha3(text) || + ISO3166::Country.find_country_by_any_name(text) + end + + # A US state, a known province or region, or a country named as the state ("England", "Togo"). + def country_from_state(state) + return UNITED_STATES if us_state_code(state) + + key = AddressNormalizer.words(state).join(" ") + return if key.blank? + return REGIONS[key] if REGIONS.key?(key) + + found = ISO3166::Country.find_country_by_any_name(state.to_s.strip) + name_for(found) if found + end + + def name_for(country) + return UNITED_STATES if ALIASES[country.common_name.downcase] == "US" + + GeographyHelper::PLAIN_COUNTRY_NAMES[country.alpha2] || country.common_name + end + + # The lowercase USPS code a value names, or nil: "CA", "Ca.", "California", "CA - California". + def us_state_code(value) + code = AddressNormalizer.state(value) + return code if AddressNormalizer::US_STATES.include?(code) + + name = AddressNormalizer.words(value).join(" ")[US_STATE_NAME] + AddressNormalizer::STATE_NAMES[name] if name + end + + def placeholder?(text) + !text.match?(/[[:alpha:]]/) || PLACEHOLDERS.include?(AddressNormalizer.words(text).join(" ")) + end +end diff --git a/app/services/event_registration_importer.rb b/app/services/event_registration_importer.rb index 688ebdd4dc..3d2a2ae58b 100644 --- a/app/services/event_registration_importer.rb +++ b/app/services/event_registration_importer.rb @@ -192,6 +192,7 @@ def initialize(file_path:, event:, import_user: nil, source: nil, dry_run: false skipped: [], rows: [] ) @organization_matcher = OrganizationServices::NameMatcher.new + @address_translation = AddressTranslation.new @organization_cache = {} @form_field_cache = {} @seen_row_keys = {} @@ -526,15 +527,27 @@ def build_answers(values, preview) answers["additional_sectors"] = sector_ids(values[:additional_sectors], preview, scope: Sector.published).map(&:to_s).presence answers["primary_age_group"] = age_group_ids(values[:primary_age_group], preview).first&.to_s answers["additional_age_groups"] = age_group_ids(values[:additional_age_groups], preview).map(&:to_s).presence + translate_address_answers(answers, "mailing") + translate_address_answers(answers, "organization") answers.compact end + # The sheet's free-text state and country, as the registration form's dropdowns store them. + def translate_address_answers(answers, prefix) + state = answers["#{prefix}_state"] + country = answers["#{prefix}_country"] + return if state.blank? && country.blank? + + answers["#{prefix}_state"] = @address_translation.state(state, country:) + answers["#{prefix}_country"] = @address_translation.country(country, state:) + end + def organization_type(raw) Organization::ORGANIZATION_TYPES.find { |type| type.casecmp?(raw) } || "Other: #{raw}" end def contact_type(raw) - CONTACT_TYPES.find { |type| type.casecmp?(raw.to_s) } + CONTACT_TYPES.find { |type| type.casecmp?(@address_translation.address_type(raw).to_s) } end def sector_ids(raw, preview, scope:) diff --git a/app/services/person_services/import_from_rolodex.rb b/app/services/person_services/import_from_rolodex.rb index bbd7eb85f1..5c88f3cc4d 100644 --- a/app/services/person_services/import_from_rolodex.rb +++ b/app/services/person_services/import_from_rolodex.rb @@ -40,9 +40,7 @@ class ImportFromRolodex DATE_FORMATS = %w[%m/%d/%Y %Y-%m-%d %m-%d-%Y %Y/%m/%d].freeze # A two-digit year below this is read as 20xx, at or above it as 19xx. TWO_DIGIT_YEAR_PIVOT = 10 - CONTACT_TYPES = { "home" => "personal", "personal" => "personal", "cell" => "personal", - "mobile" => "personal", "work" => "work", "business" => "work", - "professional" => "work", "mailing" => "mailing" }.freeze + CONTACT_TYPES = AddressTranslation::CONTACT_TYPES # Answers to "which kind of email is this?" that aren't a kind at all. PRECEDENCE_TYPES = %w[preferred primary main].freeze EMAIL_PATTERN = /[^\s,;<>"']+@[^\s,;<>"']+\.[^\s,;<>"']+/ @@ -76,6 +74,7 @@ def initialize(scope: FmRolodex.all, relink: false, progress: nil, run: nil) @imported_codes = Person.where.not(filemaker_code: [ nil, "" ]).pluck(:filemaker_code) .flat_map { |value| Person.split_filemaker_codes(value) }.to_set @translation = TagTranslation.new + @address_translation = AddressTranslation.new @staff = StaffTranslation.new @import_user_id = Setting.import_user&.id end @@ -104,6 +103,10 @@ def unmatched_staff @staff.unmatched end + def unmatched_addresses + @address_translation.unmatched + end + private # Each batch commits on its own (a savepoint inside a dry run's rollback), so a run that @@ -433,20 +436,17 @@ def address_attributes(fields) state = clip(fields["AddrsState"].to_s.strip) return if street.blank? || city.blank? || state.blank? - country = clip(fields["AddrsCountry"]) - county = county(fields) - { + # FileMaker records a locality only on a project, so a person's own address has + # none to carry over and the translated parts have to say. + @address_translation.address( street_address: clip(street), city:, state:, zip_code: zip_code(fields), - country:, - county:, - address_type: contact_type(fields["AddrsType"], allowed: Address::CONTACT_TYPES), - # FileMaker records a locality only on a project, so a person's own address has - # none to carry over and the parts have to say. - locality: AddressLocality.for(city:, state:, country:, county:) - } + country: clip(fields["AddrsCountry"]), + county: county(fields), + address_type: fields["AddrsType"] + ) end # The address's own postal code, or the one it points at in the archive's ZIP table diff --git a/lib/tasks/fm_import_people.rake b/lib/tasks/fm_import_people.rake index be697f02d4..7eb27ac7ca 100644 --- a/lib/tasks/fm_import_people.rake +++ b/lib/tasks/fm_import_people.rake @@ -72,6 +72,13 @@ namespace :fm do import.unmatched_tags.sort_by { |name, count| [ -count, name ] }.each { |name, count| say.(" #{count.to_s.rjust(5)} #{name}") } end + import.unmatched_addresses.each do |field, values| + next if values.empty? + + say.("\nAddress #{field} values nothing translates (kept as typed):") + values.sort_by { |value, count| [ -count, value ] }.each { |value, count| say.(" #{count.to_s.rjust(5)} #{value}") } + end + if import.unmatched_staff.any? say.("\nStaff names no portal user answers to (record them with data:load_staff_aliases):") import.unmatched_staff.sort_by { |name, count| [ -count, name ] }.each { |name, count| say.(" #{count.to_s.rjust(5)} #{name}") } diff --git a/spec/services/address_translation_spec.rb b/spec/services/address_translation_spec.rb new file mode 100644 index 0000000000..b65b79140d --- /dev/null +++ b/spec/services/address_translation_spec.rb @@ -0,0 +1,130 @@ +require "rails_helper" + +RSpec.describe AddressTranslation do + subject(:translation) { described_class.new } + + let(:us) { "United States" } + + describe "#country" do + it "reads every spelling of the US found in our data as one country" do + [ "US", "Us", "USA", "U.S.A.", "United States of America", "america", "Unites States", "Untied States", + "Los Angeles", "LA County", "US Minor Outlying Islands" ].each do |typed| + expect(translation.country(typed, state: "XX")).to eq(us) + end + end + + it "uses the dropdown's plain names" do + expect(translation.country("Russian Federation", state: "XX")).to eq("Russia") + expect(translation.country("RU", state: "XX")).to eq("Russia") + expect(translation.country("CAN", state: "XX")).to eq("Canada") + expect(translation.country("Congo, The Democratic Republic of the", state: "XX")).to eq("Democratic Republic of the Congo") + expect(translation.country("AUSTRALIA", state: "XX")).to eq("Australia") + end + + it "reads the FileMaker spellings the countries gem doesn't" do + expect(translation.country("Korea", state: "XX")).to eq("South Korea") + expect(translation.country("Republic of Ireland", state: "XX")).to eq("Ireland") + expect(translation.country("Mexcio", state: "XX")).to eq("Mexico") + expect(translation.country("Scotland", state: "XX")).to eq("United Kingdom") + expect(translation.country("Republic of Armenia", state: "XX")).to eq("Armenia") + end + + it "reads a US state code typed as the country next to that state as the US" do + expect(translation.country("CA", state: "CA")).to eq(us) + expect(translation.country("CA", state: "ON")).to eq("Canada") + end + + it "falls back to the country the state implies" do + expect(translation.country(nil, state: "CA - California")).to eq(us) + expect(translation.country("", state: "Maryland (MD)")).to eq(us) + expect(translation.country("-", state: "VIRGINIA")).to eq(us) + expect(translation.country("unknown", state: "Ca.")).to eq(us) + expect(translation.country(nil, state: "BC")).to eq("Canada") + expect(translation.country(nil, state: "VIC")).to eq("Australia") + expect(translation.country(nil, state: "Bucks")).to eq("United Kingdom") + expect(translation.country(nil, state: "Ha Noi")).to eq("Vietnam") + expect(translation.country(nil, state: "GREATER-ACCRA")).to eq("Ghana") + expect(translation.country(nil, state: "England")).to eq("United Kingdom") + expect(translation.country("West Africa", state: "Togo")).to eq("Togo") + end + + it "keeps a country it can't place as typed and reports it" do + expect(translation.country(" Narnia ", state: "Central Region")).to eq("Narnia") + expect(translation.unmatched[:country]).to eq("Narnia" => 1) + end + + it "is nil when neither the country nor the state names one" do + [ nil, "", " ", "-", "n/a", "None", "UNKNOWN", "not applicable" ].each do |blank| + expect(translation.country(blank, state: "-")).to be_nil + end + expect(translation.unmatched[:country]).to be_empty + end + + it "only ever answers with a dropdown country" do + ISO3166::Country.all.each do |country| + [ country.alpha2, country.alpha3, country.common_name, country.iso_short_name ].each do |typed| + expect(GeographyHelper::COUNTRIES).to include(translation.country(typed, state: "XX")) + end + end + end + + it "reads every country value seen in production and FileMaker into the dropdown" do + [ "Canada", "Australia", "United Kingdom", "South Africa", "China", "Japan", "Philippines", "Cameroon", + "Dominican Republic", "India", "Mexico", "Ireland", "New Zealand", "Russia", "Pakistan", "England", "Belgium", + "Germany", "Hong Kong", "Rwanda", "Greece", "France", "Cayman Islands", "Norway", "South Korea", "Singapore", + "Guam", "Korea, Republic of", "Uganda", "Zimbabwe", "Kenya", "Lebanon", "Azerbaijan", "Jamaica", "Netherlands", + "Spain", "Hungary", "Ghana", "Switzerland", "Italy", "Iran", "Malaysia", "Nepal", "Taiwan", "Armenia" ].each do |typed| + expect(GeographyHelper::COUNTRIES).to include(translation.country(typed, state: "XX")) + end + end + end + + describe "#state" do + it "stores a US state as its USPS code" do + { "CA" => "CA", "ca" => "CA", "Ca." => "CA", "California" => "CA", "VIRGINIA" => "VA", "Kansas" => "KS", + "CA - California" => "CA", "Maryland (MD)" => "MD", "Washington DC" => "DC", "PR" => "PR" }.each do |typed, code| + expect(translation.state(typed, country: nil)).to eq(code) + end + end + + it "keeps a region outside the US as typed" do + expect(translation.state(" ON ", country: "Canada")).to eq("ON") + expect(translation.state("Georgia", country: "Georgia")).to eq("Georgia") + expect(translation.unmatched[:state]).to be_empty + end + + it "reports a value that reads as neither a US state nor a place abroad" do + expect(translation.state("90401", country: "USA")).to eq("90401") + expect(translation.unmatched[:state]).to eq("90401" => 1) + end + + it "is nil when blank" do + expect(translation.state(" ", country: nil)).to be_nil + end + end + + describe "#address_type" do + it "reads the archive's and forms' words as the address types we store" do + { "Home" => "personal", "cell" => "personal", "Work" => "work", "Business" => "work", "Mailing" => "mailing" }.each do |typed, type| + expect(translation.address_type(typed)).to eq(type) + end + expect(translation.address_type("Vacation")).to be_nil + end + end + + describe "#address" do + it "translates an address's state, country, and type, and works out its locality" do + translated = translation.address(street_address: "1 Main St", city: "Los Angeles", state: "California", + country: "USA", county: nil, address_type: "Home") + + expect(translated).to include(street_address: "1 Main St", state: "CA", country: us, + address_type: "personal", locality: Address::LA_CITY) + end + + it "reads a country from a province and marks it outside the US" do + translated = translation.address(city: "Toronto", state: "ON", country: nil) + + expect(translated).to include(state: "ON", country: "Canada", locality: Address::OUTSIDE_USA) + end + end +end diff --git a/spec/services/event_registration_importer_spec.rb b/spec/services/event_registration_importer_spec.rb index 0cecebb40d..c06d529852 100644 --- a/spec/services/event_registration_importer_spec.rb +++ b/spec/services/event_registration_importer_spec.rb @@ -538,6 +538,20 @@ def registration_of(person) expect(Person.find_by(email: "pat@example.org").addresses.sole).to have_attributes(street_address: "", zip_code: "62701") end + it "stores the sheet's state, country, and address type as the dropdowns do" do + import_rows(%w[FirstName LastName EMail Street City State Zip Country AddressType], + [ "Pat", "Example", "pat@example.org", "2 Oak Ave", "Fresno", "California", "93701", "USA", "Home" ]) + + expect(Person.find_by(email: "pat@example.org").addresses.sole) + .to have_attributes(state: "CA", country: "United States", address_type: "personal") + end + + it "reads a missing country from the sheet's state" do + import_rows(%w[FirstName LastName EMail Street City State Zip], [ "Pat", "Example", "pat@example.org", "1 King St", "Toronto", "ON", "M5H" ]) + + expect(Person.find_by(email: "pat@example.org").addresses.sole).to have_attributes(state: "ON", country: "Canada") + end + it "names a new person by the sheet's nickname and keeps the first name as their legal first name" do import_rows(%w[FirstName Nickname LastName EMail], [ "Robert", "Bob", "Example", "bob@example.org" ]) person = Person.find_by(email: "bob@example.org") diff --git a/spec/services/person_services/import_from_rolodex_spec.rb b/spec/services/person_services/import_from_rolodex_spec.rb index 0110f1cb65..c90cf7d3c6 100644 --- a/spec/services/person_services/import_from_rolodex_spec.rb +++ b/spec/services/person_services/import_from_rolodex_spec.rb @@ -383,6 +383,33 @@ def results .to eq([ "LA County", "Outside CA", "Outside USA" ]) end + it "translates each address's state and country, reading the country from the state when the archive has none" do + FmAddress.create!(fm_id: "A15", data: { "AddrsLine1" => "1 Main St", "AddrsCity" => "Austin", "AddrsState" => "Texas", "AddrsCountry" => "USA" }) + FmAddress.create!(fm_id: "A16", data: { "AddrsLine1" => "2 Main St", "AddrsCity" => "Fresno", "AddrsState" => "CA - California" }) + FmAddress.create!(fm_id: "A17", data: { "AddrsLine1" => "3 Queen St", "AddrsCity" => "Victoria", "AddrsState" => "BC" }) + FmAddress.create!(fm_id: "A18", data: { "AddrsLine1" => "4 Reforma", "AddrsCity" => "Mexico City", "AddrsState" => "CDMX", "AddrsCountry" => "Mexcio" }) + rolodex("00050", EMail: "ada@example.com", PrimaryAddrsID: "A15") + rolodex("00051", FirstName: "Grace", EMail: "grace@example.com", PrimaryAddrsID: "A16") + rolodex("00052", FirstName: "Hedy", EMail: "hedy@example.com", PrimaryAddrsID: "A17") + rolodex("00053", FirstName: "Mary", EMail: "mary@example.com", PrimaryAddrsID: "A18") + + results + + addresses = %w[00050 00051 00052 00053].map { |code| Person.find_by!(filemaker_code: code).addresses.sole } + expect(addresses.map { [ _1.state, _1.country ] }) + .to eq([ [ "TX", "United States" ], [ "CA", "United States" ], [ "BC", "Canada" ], [ "CDMX", "Mexico" ] ]) + expect(FmAddress.find_by!(fm_id: "A15").data).to include("AddrsState" => "Texas", "AddrsCountry" => "USA") + end + + it "reports address values it can't translate" do + FmAddress.create!(fm_id: "A19", data: { "AddrsLine1" => "1 Main St", "AddrsCity" => "Nowhere", "AddrsState" => "ZZ", "AddrsCountry" => "Narnia" }) + rolodex("00054", EMail: "ada@example.com", PrimaryAddrsID: "A19") + + import = described_class.new.call + + expect(import.unmatched_addresses[:country]).to eq("Narnia" => 1) + end + it "prefixes the street number when the first line doesn't already carry it" do FmAddress.create!(fm_id: "A2", data: { "StreetNo" => "1234", "AddrsLine1" => "Main St", "AddrsCity" => "Los Angeles", "AddrsState" => "CA" }) From 5d39c849c37eac44600b11c9571e3645d651e5c9 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sat, 24 Oct 2026 18:26:28 -0400 Subject: [PATCH 03/10] Translate existing address countries/states and their form answers, logging each change Portal data from before the dropdowns holds the same free-text spellings the imports did; one idempotent pass (before and after the FileMaker imports) brings it in line. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 7 +- app/models/form_answer.rb | 4 +- .../address_services/translate_existing.rb | 96 ++++++++++++++ ...translate-address-countries-and-states.yml | 12 ++ ...maker-organization-and-personnel-import.md | 2 + .../0009-legacy-address-value-translation.md | 68 ++++++++++ lib/tasks/translate_addresses.rake | 27 ++++ .../translate_existing_spec.rb | 119 ++++++++++++++++++ 8 files changed, 332 insertions(+), 3 deletions(-) create mode 100644 app/services/address_services/translate_existing.rb create mode 100644 db/seeds/features/2026-10-24-translate-address-countries-and-states.yml create mode 100644 docs/adr/0009-legacy-address-value-translation.md create mode 100644 lib/tasks/translate_addresses.rake create mode 100644 spec/services/address_services/translate_existing_spec.rb diff --git a/AGENTS.md b/AGENTS.md index 8b60da2b2a..e01d2013ab 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -49,7 +49,7 @@ This codebase (Rails 8.1) | Directory | Purpose | Count | |---|---|---| | `app/models/` | ActiveRecord models | ~146 files | -| `app/services/` | Service objects and POROs (e.g. `MoneyFormatter` for currency display, `StoryImporter` for the stories spreadsheet import) | ~150 files | +| `app/services/` | Service objects and POROs (e.g. `MoneyFormatter` for currency display, `StoryImporter` for the stories spreadsheet import) | ~155 files | | `app/jobs/` | SolidQueue background jobs | 6 files | | `app/models/concerns/` | Shared model modules | 23 concerns | @@ -266,6 +266,8 @@ or symbol. So write `authorize! @setting` in `show` (not `authorize! @setting, t - `ModelDeduper` — Deduplication logic. On a natural-key collision it now merges the losing row into the survivor (recursively, via itself) so the loser's own children move first — instead of destroying the loser and cascading them away or aborting on a protective `before_destroy` (e.g. a professional license with CE registrations). It finds reassignable children two ways: the model's own `has_many` declarations, and a scan of the DB's foreign keys. **A polymorphic child carries no foreign key**, so it is reachable only through an explicit `has_many … as:` — without one, `#unhandled_references` sees it and the merge is refused outright (this is why `Person`/`Organization` declare `has_many :invoices, as: :invoicee`). A config can also pass `reference_columns:` — column names that point at the model in any table without a DB foreign key (the users deduper names `user_id`/`created_by_id`/`updated_by_id`/`welcome_instructions_sent_by_id`). Self-references on the model's own table follow the keeper, except the keeper's own reference to the duplicate, which clears. The preview lists each moving record by its decorator's optional `#dedupe_label` (Affiliation, ContactMethod, Comment, Notification define one; text snippets cap at `SNIPPET_LENGTH`) - `ContinuingEducationDeduper` — Collapses duplicate CE registrations left sharing one event registration + license after a merge, moving the losers' allocations (payments) and comments onto the survivor. Keeps the real cost (larger of the two) rather than inventing one to cover the combined payments — if they now exceed it, `ContinuingEducationRegistration#over_allocated?` flags it (`ContinuingEducationRegistrationDecorator#overpayment_badge` → "Check payments") for an admin. CE has no DB unique index, so `ModelDeduper` never compares CE rows and just moves them all onto the keeper; a unique index is deliberately avoided because it would force the engine to move CE via validated `update!`, which trips `ContinuingEducationRegistration#license_belongs_to_registrant` mid-merge. Run from the people deduper's `after_merge` - `AddressNormalizer` — Comparison keys for address parts so spellings of one place match: street suffixes/directionals/ordinals (`Street`→`st`, `North`→`n`, `First`→`1st`), unit designators (`Suite`/`Apt`/`Unit`/`#` → `#`), `Saint`/`Mount`/`Fort` in cities, state names → USPS abbreviations, ZIP+4 → five digits, US country spellings (and a blank country in a US state) → `us`, a trailing "County", and an "unknown" type/locality → blank. `.signature(address)` is the whole-address key. Matching only — stored addresses keep their spelling. Used by `AddressDeduper`, `OrganizationServices::AddressMatcher`, `OrganizationServices::DuplicateFinder`'s shared-address signal, and `PersonServices::CaptureFromSubmission`'s mailing-address match +- `AddressTranslation` — Turns imported and legacy address values into what the address dropdowns store (ADR-0009): `#country(value, state:)` → a `GeographyHelper::COUNTRIES` name (aliases for our data's spellings, ISO codes, plain names, a country read from the state — a US state incl. "CA - California", a `REGIONS` province, or a country named as the state; placeholders like "-"/"n/a"/"unknown" → nil; "CA" next to state CA → US), `#state(value, country:)` → USPS code for a US state (other regions kept), `#address_type` (the archive's/forms' words via `CONTACT_TYPES`), `#locality` (`AddressLocality` on the translated parts), and `#address(attrs)` for a whole hash. Lookup tables only — a value nothing answers to is kept as typed and counted in `#unmatched` (reported, never guessed, like `TagTranslation`). Used by `PersonServices::ImportFromRolodex` (unmatched listed in `fm:import_people`'s report), `EventRegistrationImporter` (country/state answers + address/phone types), and `AddressServices::TranslateExisting`. No save hook — the dropdowns own new data +- `AddressServices::TranslateExisting` — Rewrites stored `Address` countries/states and the `mailing_*`/`organization_*` country/state `FormAnswer`s through `AddressTranslation` (each answer read beside its paired state/country answer), fills a locality only where it's Unknown, and logs each changed record as one `autochange.address`/`autochange.form_answer` Ahoy event (all in one visit, `reason: "address_translation"`). Idempotent; `dry_run:` reports without writing. Drives the `data:translate_addresses` rake task - `AddressLocality` — Works out which `Address::LOCALITIES` bucket an address falls in from its own city/state/country/county, for a writer that has an address but never asked the question (`PersonServices::ImportFromRolodex`, `PersonServices::CaptureFromSubmission`, `OrganizationServices::UpsertAddress`, `ProfileChangeRequests::Apply` — all on create only, so a locality a person picked is never overwritten). Reads a named non-US country as **Outside USA**, a non-CA US state as **Outside CA**, a Los Angeles mailing city as **LA City**, the rest of Los Angeles County as **LA County**, and Orange County as **Orange County**. Everything else stays **Unknown**: the Northern/Central/Southern line through California is AWBW's own and nothing records where it runs, and a state no US state answers to could as easily be a typo as a foreign region. Every comparison goes through `AddressNormalizer`. FileMaker records a `Locality` only on a **project**, so the organizations pass (ADR-0008) carries that across rather than deriving it; a person's own address has none - `AddressDeduper` — Merges a person's or organization's addresses that are one place into a single address. `.matching_sets` groups by `AddressNormalizer.signature`, then merges a less complete copy (blank ZIP, street, country, …) into the single fuller address whose every part it shares — skipped when it fits more than one; works fullest-first so a chain of partial copies lands on the fullest. Each merge goes through `ModelDeduper#merge`, which repoints the merged address's inbound FKs — `contact_methods.address_id`, `invoices.bill_to_address_id`, `affiliations.organization_address_id` — onto the survivor. Within a set the survivor is the copy still in use — active before inactive, then the primary, then the oldest (`.keeper_order`) — since `Address.active` drives bill-to addresses, an org's program location and the geographic breakdowns; blank columns on the survivor (`CARRIED_COLUMNS` — the street/ZIP/country/county/district/type/locality, `phone` and the three `la_*` districts; "unknown" type/locality counts as blank) are filled from the merged copies. Then settles a single primary on the set holding the signature captured in the deduper's `merge_keeper` (the survivor's own pre-merge primary address, else the deleted record's), demoting the rest. Runs in the people and organizations dedupers' `after_merge`; `AddressDeduper.merge_duplicates(addressable)` runs it standalone (keeping the owner's current primary) for the **Dedupe addresses** admin page (`AddressesController#dedupe_index`/`#dedupe_perform`) - `AddressServices::DuplicateFinder` — Lists the people and organizations holding addresses that `AddressDeduper.matching_sets` would merge, for the **Dedupe addresses** page. Compares only within one owner — two orgs at one street are separate records, not duplicates @@ -591,7 +593,7 @@ RuboCop linting on PRs and pushes to main. ## Rake Tasks -Located in `lib/tasks/` (18 files): +Located in `lib/tasks/` (20 files): - `dev.rake` — Development database seeding from XML/CSV - `rhino_migrator.rake` — Rich text editor migration - `attachment_report.rake` — Attachment reporting @@ -604,6 +606,7 @@ Located in `lib/tasks/` (18 files): - `migrate_workshop_logs.rake` — Workshop log migration - `backfill_user_stamps.rake` — Backfill `created_by_id`/`updated_by_id` on legacy rows from the Ahoy trail (`data:backfill_user_stamps`) - `backfill_notification_stamps.rake` — Backfill notifications' `created_by_id`/`updated_by_id` from `sender_id`, nil-only (`data:backfill_notification_stamps`) +- `translate_addresses.rake` — Rewrite stored address countries/states and their form answers as the dropdowns store them (`data:translate_addresses`, via `AddressServices::TranslateExisting`, idempotent, DRY_RUN, lists values nothing translates) - `fm_import.rake` — Load the FileMaker CSV exports into the `fm_*` archive tables (`fm:import`, reads each CSV from the `FM Archive` resources, batch `insert_all`, skips rows whose key is already stored) - `fm_import_people.rake` — Create people from the FileMaker Rolodex archive (`fm:import_people`, via `PersonServices::ImportFromRolodex`, links existing by name + email, idempotent, DRY_RUN; VERBOSE=true lists every row, FM_IDS=a,b re-runs just those rows past the idempotence guard; commits every 1,000 rows, prints progress, saves the full report as an ActiveStorage blob even on a crash, and aborts when no `TagAlias` rows exist or no `Setting.import_user` is chosen; logs every record it creates or fills as activity under a run name (`Current.source = "fm_import_people"`), which `fm:undo_import_people RUN=…` reverses via `PersonServices::UndoRolodexImport`) - `resolve_resource_organizations.rake` — Link resources to organizations from the legacy `agency` free-text name (`data:resolve_resource_organizations`, match-or-create, idempotent, DRY_RUN) diff --git a/app/models/form_answer.rb b/app/models/form_answer.rb index ea8fd0950f..3d24ad0e18 100644 --- a/app/models/form_answer.rb +++ b/app/models/form_answer.rb @@ -1,6 +1,8 @@ # `submitted_answer` is stored verbatim — exactly what the applicant typed — so # their words are never silently altered (e.g. "revenue < $5k" or "C++ " -# survive intact). It is NOT HTML-sanitized on write. +# survive intact). It is NOT HTML-sanitized on write. The one exception is the +# address country/state answers, which AddressServices::TranslateExisting rewrites +# as the dropdowns store them (ADR-0009), logging every change as an Ahoy event. # # Safety is enforced at render time instead: every place that displays an answer # uses plain `<%= %>`, which auto-escapes, so any markup shows as inert text. diff --git a/app/services/address_services/translate_existing.rb b/app/services/address_services/translate_existing.rb new file mode 100644 index 0000000000..ea6d1f147e --- /dev/null +++ b/app/services/address_services/translate_existing.rb @@ -0,0 +1,96 @@ +module AddressServices + # Rewrites stored address countries and states, and the country/state answers on form + # submissions, as the dropdowns store them (ADR-0009), filling a locality only where it's + # Unknown. Each changed record is one autochange Ahoy event, all in one visit. Idempotent, + # so it runs before and after an import. + class TranslateExisting + Result = Data.define(:addresses, :answers, :changes, :unmatched) + + REASON = "address_translation" + # Each address answer, and the answer beside it that the translation reads alongside. + ANSWER_PAIRS = { + "mailing_country" => "mailing_state", "mailing_state" => "mailing_country", + "organization_country" => "organization_state", "organization_state" => "organization_country" + }.freeze + + # `log` receives one line per changed record, for the rake task's preview. + def initialize(dry_run: false, log: nil, user: nil) + @dry_run = dry_run + @log = log + @user = user + end + + def call + @translation = AddressTranslation.new + @tracker = Ahoy::Tracker.new(user: @user) + @changes = Hash.new(0) + addresses = answers = 0 + + ActiveRecord::Base.transaction do + Address.find_each { |address| addresses += 1 if translate_address(address) } + address_answers.find_each { |answer| answers += 1 if translate_answer(answer) } + end + + Result.new(addresses:, answers:, changes: @changes, unmatched: @translation.unmatched) + end + + private + + def translate_address(address) + state = @translation.state(address.state, country: address.country) + country = @translation.country(address.country, state: address.state) + updates = {} + updates[:state] = state if state.present? && state != address.state + updates[:country] = country if country != address.country + if address.locality.blank? || address.locality == Address::UNKNOWN_LOCALITY + locality = @translation.locality(city: address.city, state:, country:, county: address.county) + updates[:locality] = locality if locality != address.locality + end + apply(address, updates, title: address.name, addressable_type: address.addressable_type, addressable_id: address.addressable_id) + end + + def translate_answer(answer) + identifier = answer.form_field.field_identifier + paired = paired_answers.dig(answer.form_submission_id, ANSWER_PAIRS[identifier]) + raw = answer.submitted_answer + translated = if identifier.end_with?("_country") + @translation.country(raw, state: paired) + else + @translation.state(raw, country: paired) + end + return false if translated.blank? || translated == raw + + apply(answer, { submitted_answer: translated }, title: answer.form_field.name, + form_submission_id: answer.form_submission_id, field_identifier: identifier) + end + + def apply(record, updates, title:, **extra) + return false if updates.empty? + + label = record.class.model_name.human + updates.each do |field, after| + @changes["#{label} #{field.to_s.humanize(capitalize: false)}: #{record[field].inspect} → #{after.inspect}"] += 1 + end + @log&.call(" #{label} ##{record.id}: #{updates.map { |field, after| "#{field} #{record[field].inspect} → #{after.inspect}" }.join(', ')}") + return true if @dry_run + + changes = updates.to_h { |field, after| [ field, { before: record[field], after: } ] } + record.update_columns(updates) + @tracker.track("autochange.#{record.class.model_name.singular}", { + resource_type: record.class.name, resource_id: record.id, resource_title: title, changes:, reason: REASON, **extra + }) + true + end + + def address_answers + FormAnswer.joins(:form_field).where(form_fields: { field_identifier: ANSWER_PAIRS.keys }).includes(:form_field) + end + + # { submission_id => { identifier => answer } }, read before any answer is rewritten so + # each translation sees what the person typed beside it. + def paired_answers + @paired_answers ||= address_answers.pluck(:form_submission_id, "form_fields.field_identifier", :submitted_answer) + .each_with_object(Hash.new { |hash, id| hash[id] = {} }) { |(id, identifier, value), map| map[id][identifier] = value } + end + end +end diff --git a/db/seeds/features/2026-10-24-translate-address-countries-and-states.yml b/db/seeds/features/2026-10-24-translate-address-countries-and-states.yml new file mode 100644 index 0000000000..85b0146835 --- /dev/null +++ b/db/seeds/features/2026-10-24-translate-address-countries-and-states.yml @@ -0,0 +1,12 @@ +name: "Address countries and states stored the way the dropdowns do" +area: people +display_status: admin_facing +released_on: 2026-10-24 +pr_number: 2829 +summary: >- + Imports from FileMaker and the registration spreadsheet now store countries and states the + way the dropdowns do, and a one-time cleanup does the same for the ones already in the + portal — "USA" becomes United States, "California" becomes CA, a province fills in its country. +pro_tips: + - "Every change is logged in Activities, so you can see what was rewritten and what it was before." + - "Anything it can't place is listed in the cleanup's report for a hand fix." diff --git a/docs/adr/0008-filemaker-organization-and-personnel-import.md b/docs/adr/0008-filemaker-organization-and-personnel-import.md index 3b159c61dc..a6750a0adf 100644 --- a/docs/adr/0008-filemaker-organization-and-personnel-import.md +++ b/docs/adr/0008-filemaker-organization-and-personnel-import.md @@ -59,6 +59,8 @@ Constraints already decided elsewhere: no locality to carry, which is why the Rolodex pass works it out with `AddressLocality`. Those project rows also carry real County → Locality pairs, so they are where to read AWBW's Northern/Central/Southern line from instead of inventing it. +- **State, country and address type go through `AddressTranslation#address`** (ADR-0009), + as the Rolodex pass does, so organization addresses store what the dropdowns offer. - **Conflicts:** same rule as people — fill blanks, keep the portal value where the archive disagrees, and report each difference as a comment plus in the task output. - **Out of scope:** event registrations (`fm_participants`, 68,497 rows), payments, funding, diff --git a/docs/adr/0009-legacy-address-value-translation.md b/docs/adr/0009-legacy-address-value-translation.md new file mode 100644 index 0000000000..8f7365cf9a --- /dev/null +++ b/docs/adr/0009-legacy-address-value-translation.md @@ -0,0 +1,68 @@ +# ADR-0009 — Translating legacy address values into what the dropdowns store + +- **Status:** Accepted +- **Date:** 2026-10-24 + +## Context + +The portal's address country is now a dropdown (`GeographyHelper::COUNTRIES` — the +`countries` gem's names, with plain names for the formal ones), and its state field stores a +USPS code. Everything written before, and everything other systems send, is free text: + +| Source | Fields | +|---|---| +| FileMaker (`fm_addresses`) | `AddrsCountry`, `AddrsState`, `AddrsType` | +| LMS registration spreadsheet (ongoing) | country, state, address type, phone type columns | +| Portal addresses and form answers from before the dropdowns | `addresses.country`/`state`, `mailing_*`/`organization_*` answers | + +The same place arrives many ways — "US", "USA", "United States of America", "Untied States", +"California", "Ca.", "CA - California", a province typed in the state field with no country — +so one country shows up as several on dashboards, and a stored value the dropdown doesn't +list can't be preselected. + +## Decision + +One translator, `AddressTranslation`, used by every import and by the cleanup of existing +data. Like ADR-0007, translation is **by lookup table, and a value nothing answers to is +reported, never guessed** — it's kept as typed and counted in `#unmatched`. + +- **Country** → a dropdown name. In order: a placeholder ("-", blank, "n/a", "none", + "unknown") counts as no country; a US state code typed as the country next to that state + is the US ("CA" beside state CA is not Canada); a spelling our data uses that the gem + doesn't read (`ALIASES` — e.g. "Untied States", "Mexcio", "Korea" → South Korea, a US city + or county typed as the country → US); the gem's own names and ISO codes, shown with the + dropdown's plain names. When the country says nothing, **the state can**: a US state means + the US, a known province or region (`REGIONS` — ON, VIC, Bucks, Ha Noi…) means its country, + and a country typed as the state ("England", "Togo") means that country. +- **State** → the USPS code when the value reads as a US state, including a full name + anywhere in it ("Maryland (MD)"); any other region is kept as typed. A value that reads as + neither, on a US or country-less address, is reported. +- **Address type** → the archive's and forms' words ("Home", "cell", "Business") as the + types we store, via one `CONTACT_TYPES` table (the Rolodex import reads phone and email + types from it too). +- **Locality** → `AddressLocality` on the translated state and country. + +### Where it runs + +- **Imports, inline:** the FileMaker Rolodex import and the LMS registration importer, and + the FileMaker organizations import when it's built (ADR-0008). Each reports what it + couldn't place. +- **Existing data, once and again:** `AddressServices::TranslateExisting` + (`data:translate_addresses`) rewrites stored address countries and states, and the + country/state form answers, filling a locality only where it's Unknown. It's idempotent, so it runs before and after an import. Every changed + record is an `autochange.*` Ahoy event with its before and after. +- **Not on save.** The dropdowns own new data; nothing rewrites an address when someone + saves a form. + +### Form answers + +Form answers are otherwise stored verbatim. Country and state answers are the one exception: +they're rewritten so form results and maps count one place once, and each rewrite is logged. + +## Consequences + +- A new spelling turns up in an import's or the cleanup's unmatched report; adding it is a + line in `ALIASES` or `REGIONS`. +- The `fm_*` archive tables keep FileMaker's raw values. +- What the cleanup can't place is listed in its report for a hand fix, so a form's + exact-match preselect can't silently blank one on save. diff --git a/lib/tasks/translate_addresses.rake b/lib/tasks/translate_addresses.rake new file mode 100644 index 0000000000..68b19375d9 --- /dev/null +++ b/lib/tasks/translate_addresses.rake @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +# Rewrite stored address countries/states and country/state form answers as the dropdowns +# store them. See AddressServices::TranslateExisting. +# +# Dry-run by default. Set DRY_RUN=false to write changes (each one logged as an Ahoy event). +# bin/rails data:translate_addresses # preview, writes nothing +# DRY_RUN=false bin/rails data:translate_addresses # translate + log +namespace :data do + desc "Translate stored address countries/states and their form answers to dropdown values. Idempotent. DRY_RUN=false to execute." + task translate_addresses: :environment do + dry_run = ENV["DRY_RUN"] != "false" + result = AddressServices::TranslateExisting.new(dry_run:).call + + puts "Changes (#{result.changes.values.sum}):" + result.changes.sort_by { |label, count| [ -count, label ] }.each { |label, count| puts " #{count.to_s.rjust(5)} #{label}" } + result.unmatched.each do |field, values| + next if values.empty? + + puts "\n#{field.to_s.humanize} values nothing translates (kept as typed — fix by hand):" + values.sort_by { |value, count| [ -count, value ] }.each { |value, count| puts " #{count.to_s.rjust(5)} #{value}" } + end + puts "\naddresses changed: #{result.addresses}" + puts "answers changed: #{result.answers}" + puts(dry_run ? "\nDRY RUN — nothing written. Set DRY_RUN=false to execute." : "\nDone.") + end +end diff --git a/spec/services/address_services/translate_existing_spec.rb b/spec/services/address_services/translate_existing_spec.rb new file mode 100644 index 0000000000..dc96da38fc --- /dev/null +++ b/spec/services/address_services/translate_existing_spec.rb @@ -0,0 +1,119 @@ +require "rails_helper" + +RSpec.describe AddressServices::TranslateExisting do + let(:us) { "United States" } + + # Stored as old rows hold them, past the model. + def address_with(country:, state:, locality: Address::UNKNOWN_LOCALITY, city: "Springfield") + create(:address, addressable: create(:person), city:).tap do |address| + address.update_columns(country:, state:, locality:) + end + end + + def stored(address) = Address.where(id: address.id).pick(:country, :state, :locality) + + def answer(submission, identifier, value) + field = create(:form_field, form: submission.form, field_identifier: identifier, name: identifier) + create(:form_answer, form_submission: submission, form_field: field, submitted_answer: value) + end + + it "translates stored countries and states, reading a missing country from the state" do + usa = address_with(country: "USA", state: "Texas") + blank = address_with(country: nil, state: "CA - California") + province = address_with(country: "-", state: "BC") + russia = address_with(country: "Russian Federation", state: "Moscow") + + described_class.new.call + + expect(stored(usa).first(2)).to eq([ us, "TX" ]) + expect(stored(blank).first(2)).to eq([ us, "CA" ]) + expect(stored(province).first(2)).to eq([ "Canada", "BC" ]) + expect(stored(russia).first).to eq("Russia") + end + + it "fills a locality only where it's Unknown" do + unknown = address_with(country: nil, state: "MA", city: "Cambridge") + chosen = address_with(country: nil, state: "CA", city: "Fresno", locality: "Central CA") + + described_class.new.call + + expect(stored(unknown).last).to eq(Address::OUTSIDE_CA) + expect(stored(chosen).last).to eq("Central CA") + end + + it "clears a placeholder country when the state names none" do + dash = address_with(country: "-", state: "Central Region") + unknown = address_with(country: "unknown", state: "-") + + described_class.new.call + + expect([ stored(dash).first, stored(unknown).first ]).to eq([ nil, nil ]) + end + + it "keeps a country it can't place and reports it" do + narnia = address_with(country: "Narnia", state: "Central Region") + + result = described_class.new.call + + expect(stored(narnia).first).to eq("Narnia") + expect(result.unmatched[:country]).to include("Narnia" => 1) + end + + it "translates country and state answers using the submission's other answer" do + submission = create(:form_submission) + answer(submission, "mailing_state", "California") + country = answer(submission, "mailing_country", "") + org_country = answer(submission, "organization_country", "Untied States") + org_state = answer(submission, "organization_state", "Texas") + + described_class.new.call + + expect(country.reload.submitted_answer).to eq(us) + expect(org_country.reload.submitted_answer).to eq(us) + expect(org_state.reload.submitted_answer).to eq("TX") + expect(FormAnswer.joins(:form_field).find_by(form_fields: { field_identifier: "mailing_state" }).submitted_answer).to eq("CA") + end + + it "leaves a blank answer blank when its section was skipped" do + submission = create(:form_submission) + org_country = answer(submission, "organization_country", "") + + described_class.new.call + + expect(org_country.reload.submitted_answer).to eq("") + end + + it "logs each changed record as one autochange event, all in one visit" do + usa = address_with(country: "USA", state: "TX", locality: Address::OUTSIDE_CA) + submission = create(:form_submission) + answer(submission, "mailing_state", "TX") + country = answer(submission, "mailing_country", "USA") + + expect { described_class.new.call }.to change(Ahoy::Event, :count).by(2) + + address_event = Ahoy::Event.find_by!(name: "autochange.address", resource_id: usa.id) + expect(address_event.properties).to include("changes" => { "country" => { "before" => "USA", "after" => us } }, + "reason" => "address_translation") + answer_event = Ahoy::Event.find_by!(name: "autochange.form_answer", resource_id: country.id) + expect(answer_event.properties["changes"]).to eq("submitted_answer" => { "before" => "USA", "after" => us }) + expect(Ahoy::Event.where(name: %w[autochange.address autochange.form_answer]).distinct.count(:visit_id)).to eq(1) + end + + it "writes and logs nothing on a dry run, but reports what it would change" do + usa = address_with(country: "USA", state: "TX") + + result = nil + expect { result = described_class.new(dry_run: true).call }.not_to change(Ahoy::Event, :count) + + expect(stored(usa).first).to eq("USA") + expect(result.changes).to include("Address country: \"USA\" → \"United States\"" => 1) + end + + it "changes nothing the second time" do + address_with(country: "USA", state: "Texas") + + described_class.new.call + + expect { described_class.new.call }.not_to change(Ahoy::Event, :count) + end +end From ae28781ba0a5d0738046e6016eae9bd711762a9a Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sat, 24 Oct 2026 18:43:33 -0400 Subject: [PATCH 04/10] Clear a country nothing translates, so every stored country is a dropdown one Portal addresses and country answers should only hold dropdown values; the old text stays in the Ahoy log, the fm_* archive, or the import report. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 4 +-- .../address_services/translate_existing.rb | 6 ++-- app/services/address_translation.rb | 15 +++++----- ...translate-address-countries-and-states.yml | 2 +- .../0009-legacy-address-value-translation.md | 14 ++++++---- lib/tasks/fm_import_people.rake | 2 +- lib/tasks/translate_addresses.rake | 3 +- .../translate_existing_spec.rb | 28 +++++++++++++++++-- spec/services/address_translation_spec.rb | 7 +++-- 9 files changed, 55 insertions(+), 26 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e01d2013ab..3970108f1f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -266,8 +266,8 @@ or symbol. So write `authorize! @setting` in `show` (not `authorize! @setting, t - `ModelDeduper` — Deduplication logic. On a natural-key collision it now merges the losing row into the survivor (recursively, via itself) so the loser's own children move first — instead of destroying the loser and cascading them away or aborting on a protective `before_destroy` (e.g. a professional license with CE registrations). It finds reassignable children two ways: the model's own `has_many` declarations, and a scan of the DB's foreign keys. **A polymorphic child carries no foreign key**, so it is reachable only through an explicit `has_many … as:` — without one, `#unhandled_references` sees it and the merge is refused outright (this is why `Person`/`Organization` declare `has_many :invoices, as: :invoicee`). A config can also pass `reference_columns:` — column names that point at the model in any table without a DB foreign key (the users deduper names `user_id`/`created_by_id`/`updated_by_id`/`welcome_instructions_sent_by_id`). Self-references on the model's own table follow the keeper, except the keeper's own reference to the duplicate, which clears. The preview lists each moving record by its decorator's optional `#dedupe_label` (Affiliation, ContactMethod, Comment, Notification define one; text snippets cap at `SNIPPET_LENGTH`) - `ContinuingEducationDeduper` — Collapses duplicate CE registrations left sharing one event registration + license after a merge, moving the losers' allocations (payments) and comments onto the survivor. Keeps the real cost (larger of the two) rather than inventing one to cover the combined payments — if they now exceed it, `ContinuingEducationRegistration#over_allocated?` flags it (`ContinuingEducationRegistrationDecorator#overpayment_badge` → "Check payments") for an admin. CE has no DB unique index, so `ModelDeduper` never compares CE rows and just moves them all onto the keeper; a unique index is deliberately avoided because it would force the engine to move CE via validated `update!`, which trips `ContinuingEducationRegistration#license_belongs_to_registrant` mid-merge. Run from the people deduper's `after_merge` - `AddressNormalizer` — Comparison keys for address parts so spellings of one place match: street suffixes/directionals/ordinals (`Street`→`st`, `North`→`n`, `First`→`1st`), unit designators (`Suite`/`Apt`/`Unit`/`#` → `#`), `Saint`/`Mount`/`Fort` in cities, state names → USPS abbreviations, ZIP+4 → five digits, US country spellings (and a blank country in a US state) → `us`, a trailing "County", and an "unknown" type/locality → blank. `.signature(address)` is the whole-address key. Matching only — stored addresses keep their spelling. Used by `AddressDeduper`, `OrganizationServices::AddressMatcher`, `OrganizationServices::DuplicateFinder`'s shared-address signal, and `PersonServices::CaptureFromSubmission`'s mailing-address match -- `AddressTranslation` — Turns imported and legacy address values into what the address dropdowns store (ADR-0009): `#country(value, state:)` → a `GeographyHelper::COUNTRIES` name (aliases for our data's spellings, ISO codes, plain names, a country read from the state — a US state incl. "CA - California", a `REGIONS` province, or a country named as the state; placeholders like "-"/"n/a"/"unknown" → nil; "CA" next to state CA → US), `#state(value, country:)` → USPS code for a US state (other regions kept), `#address_type` (the archive's/forms' words via `CONTACT_TYPES`), `#locality` (`AddressLocality` on the translated parts), and `#address(attrs)` for a whole hash. Lookup tables only — a value nothing answers to is kept as typed and counted in `#unmatched` (reported, never guessed, like `TagTranslation`). Used by `PersonServices::ImportFromRolodex` (unmatched listed in `fm:import_people`'s report), `EventRegistrationImporter` (country/state answers + address/phone types), and `AddressServices::TranslateExisting`. No save hook — the dropdowns own new data -- `AddressServices::TranslateExisting` — Rewrites stored `Address` countries/states and the `mailing_*`/`organization_*` country/state `FormAnswer`s through `AddressTranslation` (each answer read beside its paired state/country answer), fills a locality only where it's Unknown, and logs each changed record as one `autochange.address`/`autochange.form_answer` Ahoy event (all in one visit, `reason: "address_translation"`). Idempotent; `dry_run:` reports without writing. Drives the `data:translate_addresses` rake task +- `AddressTranslation` — Turns imported and legacy address values into what the address dropdowns store (ADR-0009): `#country(value, state:)` → a `GeographyHelper::COUNTRIES` name (aliases for our data's spellings, ISO codes, plain names, a country read from the state — a US state incl. "CA - California", a `REGIONS` province, or a country named as the state; placeholders like "-"/"n/a"/"unknown" → nil; "CA" next to state CA → US), `#state(value, country:)` → USPS code for a US state (other regions kept), `#address_type` (the archive's/forms' words via `CONTACT_TYPES`), `#locality` (`AddressLocality` on the translated parts), and `#address(attrs)` for a whole hash. Lookup tables only, never guessed (like `TagTranslation`): a country nothing answers to is cleared, so every stored country is a dropdown name or blank, and counted in `#unmatched`; an unreadable state is kept and counted. Used by `PersonServices::ImportFromRolodex` (unmatched listed in `fm:import_people`'s report), `EventRegistrationImporter` (country/state answers + address/phone types), and `AddressServices::TranslateExisting`. No save hook — the dropdowns own new data +- `AddressServices::TranslateExisting` — Rewrites stored `Address` countries/states and the `mailing_*`/`organization_*` country/state `FormAnswer`s through `AddressTranslation` (each answer read beside its paired state/country answer), fills a locality only where it's Unknown, clears (address) or blanks (answer) a country nothing translates, and logs each changed record as one `autochange.address`/`autochange.form_answer` Ahoy event (all in one visit, `reason: "address_translation"`). Idempotent; `dry_run:` reports without writing. Drives the `data:translate_addresses` rake task - `AddressLocality` — Works out which `Address::LOCALITIES` bucket an address falls in from its own city/state/country/county, for a writer that has an address but never asked the question (`PersonServices::ImportFromRolodex`, `PersonServices::CaptureFromSubmission`, `OrganizationServices::UpsertAddress`, `ProfileChangeRequests::Apply` — all on create only, so a locality a person picked is never overwritten). Reads a named non-US country as **Outside USA**, a non-CA US state as **Outside CA**, a Los Angeles mailing city as **LA City**, the rest of Los Angeles County as **LA County**, and Orange County as **Orange County**. Everything else stays **Unknown**: the Northern/Central/Southern line through California is AWBW's own and nothing records where it runs, and a state no US state answers to could as easily be a typo as a foreign region. Every comparison goes through `AddressNormalizer`. FileMaker records a `Locality` only on a **project**, so the organizations pass (ADR-0008) carries that across rather than deriving it; a person's own address has none - `AddressDeduper` — Merges a person's or organization's addresses that are one place into a single address. `.matching_sets` groups by `AddressNormalizer.signature`, then merges a less complete copy (blank ZIP, street, country, …) into the single fuller address whose every part it shares — skipped when it fits more than one; works fullest-first so a chain of partial copies lands on the fullest. Each merge goes through `ModelDeduper#merge`, which repoints the merged address's inbound FKs — `contact_methods.address_id`, `invoices.bill_to_address_id`, `affiliations.organization_address_id` — onto the survivor. Within a set the survivor is the copy still in use — active before inactive, then the primary, then the oldest (`.keeper_order`) — since `Address.active` drives bill-to addresses, an org's program location and the geographic breakdowns; blank columns on the survivor (`CARRIED_COLUMNS` — the street/ZIP/country/county/district/type/locality, `phone` and the three `la_*` districts; "unknown" type/locality counts as blank) are filled from the merged copies. Then settles a single primary on the set holding the signature captured in the deduper's `merge_keeper` (the survivor's own pre-merge primary address, else the deleted record's), demoting the rest. Runs in the people and organizations dedupers' `after_merge`; `AddressDeduper.merge_duplicates(addressable)` runs it standalone (keeping the owner's current primary) for the **Dedupe addresses** admin page (`AddressesController#dedupe_index`/`#dedupe_perform`) - `AddressServices::DuplicateFinder` — Lists the people and organizations holding addresses that `AddressDeduper.matching_sets` would merge, for the **Dedupe addresses** page. Compares only within one owner — two orgs at one street are separate records, not duplicates diff --git a/app/services/address_services/translate_existing.rb b/app/services/address_services/translate_existing.rb index ea6d1f147e..56e25e1e41 100644 --- a/app/services/address_services/translate_existing.rb +++ b/app/services/address_services/translate_existing.rb @@ -54,11 +54,11 @@ def translate_answer(answer) paired = paired_answers.dig(answer.form_submission_id, ANSWER_PAIRS[identifier]) raw = answer.submitted_answer translated = if identifier.end_with?("_country") - @translation.country(raw, state: paired) + @translation.country(raw, state: paired).to_s else - @translation.state(raw, country: paired) + @translation.state(raw, country: paired).to_s end - return false if translated.blank? || translated == raw + return false if translated == raw.to_s || (translated.blank? && identifier.end_with?("_state")) apply(answer, { submitted_answer: translated }, title: answer.form_field.name, form_submission_id: answer.form_submission_id, field_identifier: identifier) diff --git a/app/services/address_translation.rb b/app/services/address_translation.rb index 83e5e381d1..39b95336ad 100644 --- a/app/services/address_translation.rb +++ b/app/services/address_translation.rb @@ -1,7 +1,8 @@ # Turns the address values other systems and older free-text forms wrote — FileMaker, the # LMS registration sheet, the portal's own answers from before the dropdowns — into what the -# portal's address dropdowns store (ADR-0009). Translation is by lookup table, and a value -# nothing answers to is kept as typed and counted in #unmatched, never guessed at. +# portal's address dropdowns store (ADR-0009). Translation is by lookup table, never guessed: +# a country nothing answers to is cleared so every stored country is a dropdown one, and +# counted in #unmatched with the state values it can't read. # # Built once per run and reused, so an import's report can list everything it couldn't place. class AddressTranslation @@ -50,18 +51,16 @@ def initialize @unmatched = Hash.new { |hash, field| hash[field] = Hash.new(0) } end - # A dropdown country: the one the value names, else the one its state implies, else the - # value as typed (reported). nil when it's blank or a placeholder and the state says nothing. + # A dropdown country: the one the value names, else the one its state implies, else nil + # (reported when there was a value to read). def country(value, state:) text = value.to_s.strip text = "" if placeholder?(text) return UNITED_STATES if us_state_code(text) && us_state_code(state) found = country_named(text) || country_from_state(state) - return found if found - - @unmatched[:country][text] += 1 if text.present? - text.presence + @unmatched[:country][text] += 1 if found.nil? && text.present? + found end # A US state's USPS code; any other region as typed. A value that reads as neither, on an diff --git a/db/seeds/features/2026-10-24-translate-address-countries-and-states.yml b/db/seeds/features/2026-10-24-translate-address-countries-and-states.yml index 85b0146835..4310426b0f 100644 --- a/db/seeds/features/2026-10-24-translate-address-countries-and-states.yml +++ b/db/seeds/features/2026-10-24-translate-address-countries-and-states.yml @@ -9,4 +9,4 @@ summary: >- portal — "USA" becomes United States, "California" becomes CA, a province fills in its country. pro_tips: - "Every change is logged in Activities, so you can see what was rewritten and what it was before." - - "Anything it can't place is listed in the cleanup's report for a hand fix." + - "A country it can't place is left blank and listed in the cleanup's report; the old value is in Activities." diff --git a/docs/adr/0009-legacy-address-value-translation.md b/docs/adr/0009-legacy-address-value-translation.md index 8f7365cf9a..2ad0fd0621 100644 --- a/docs/adr/0009-legacy-address-value-translation.md +++ b/docs/adr/0009-legacy-address-value-translation.md @@ -24,7 +24,10 @@ list can't be preselected. One translator, `AddressTranslation`, used by every import and by the cleanup of existing data. Like ADR-0007, translation is **by lookup table, and a value nothing answers to is -reported, never guessed** — it's kept as typed and counted in `#unmatched`. +reported, never guessed** — it's counted in `#unmatched`. Every stored country ends up a +dropdown name or blank: a country nothing answers to is cleared (its old value stays in the +cleanup's Ahoy log, the `fm_*` archive, or the import's report). A state nothing reads is kept, +since a state is required and regions outside the US are legitimate. - **Country** → a dropdown name. In order: a placeholder ("-", blank, "n/a", "none", "unknown") counts as no country; a US state code typed as the country next to that state @@ -57,12 +60,13 @@ reported, never guessed** — it's kept as typed and counted in `#unmatched`. ### Form answers Form answers are otherwise stored verbatim. Country and state answers are the one exception: -they're rewritten so form results and maps count one place once, and each rewrite is logged. +they're rewritten so form results and maps count one place once (a country answer nothing +answers to becomes blank), and each rewrite is logged. ## Consequences - A new spelling turns up in an import's or the cleanup's unmatched report; adding it is a - line in `ALIASES` or `REGIONS`. + line in `ALIASES` or `REGIONS`, and a re-run fills the country it had cleared (from the + state, or by hand from the logged value). - The `fm_*` archive tables keep FileMaker's raw values. -- What the cleanup can't place is listed in its report for a hand fix, so a form's - exact-match preselect can't silently blank one on save. +- Every stored country preselects in the dropdowns, so saving a form can't silently blank one. diff --git a/lib/tasks/fm_import_people.rake b/lib/tasks/fm_import_people.rake index 7eb27ac7ca..4db19c862c 100644 --- a/lib/tasks/fm_import_people.rake +++ b/lib/tasks/fm_import_people.rake @@ -75,7 +75,7 @@ namespace :fm do import.unmatched_addresses.each do |field, values| next if values.empty? - say.("\nAddress #{field} values nothing translates (kept as typed):") + say.("\nAddress #{field} values nothing translates (#{field == :country ? 'left blank' : 'kept as typed'}):") values.sort_by { |value, count| [ -count, value ] }.each { |value, count| say.(" #{count.to_s.rjust(5)} #{value}") } end diff --git a/lib/tasks/translate_addresses.rake b/lib/tasks/translate_addresses.rake index 68b19375d9..6c716de3f4 100644 --- a/lib/tasks/translate_addresses.rake +++ b/lib/tasks/translate_addresses.rake @@ -17,7 +17,8 @@ namespace :data do result.unmatched.each do |field, values| next if values.empty? - puts "\n#{field.to_s.humanize} values nothing translates (kept as typed — fix by hand):" + outcome = field == :country ? "left blank, was logged" : "kept as typed" + puts "\n#{field.to_s.humanize} values nothing translates (#{outcome} — fix by hand if you know it):" values.sort_by { |value, count| [ -count, value ] }.each { |value, count| puts " #{count.to_s.rjust(5)} #{value}" } end puts "\naddresses changed: #{result.addresses}" diff --git a/spec/services/address_services/translate_existing_spec.rb b/spec/services/address_services/translate_existing_spec.rb index dc96da38fc..4a70c483e9 100644 --- a/spec/services/address_services/translate_existing_spec.rb +++ b/spec/services/address_services/translate_existing_spec.rb @@ -50,13 +50,37 @@ def answer(submission, identifier, value) expect([ stored(dash).first, stored(unknown).first ]).to eq([ nil, nil ]) end - it "keeps a country it can't place and reports it" do + it "clears a country it can't place, logging what it was and reporting it" do narnia = address_with(country: "Narnia", state: "Central Region") result = described_class.new.call - expect(stored(narnia).first).to eq("Narnia") + expect(stored(narnia).first).to be_nil expect(result.unmatched[:country]).to include("Narnia" => 1) + expect(Ahoy::Event.find_by!(name: "autochange.address", resource_id: narnia.id).properties["changes"]) + .to include("country" => { "before" => "Narnia", "after" => nil }) + end + + it "blanks a country answer it can't place" do + submission = create(:form_submission) + answer(submission, "mailing_state", "-") + country = answer(submission, "mailing_country", "West Africa") + + described_class.new.call + + expect(country.reload.submitted_answer).to eq("") + end + + it "leaves every stored country and country answer on the dropdown" do + address_with(country: "Narnia", state: "Central Region") + address_with(country: "USA", state: "TX") + submission = create(:form_submission) + answer(submission, "organization_country", "Atlantis") + + described_class.new.call + + expect(Address.where.not(country: [ nil, "" ]).pluck(:country) - GeographyHelper::COUNTRIES).to be_empty + expect(FormAnswer.where.not(submitted_answer: [ nil, "" ]).pluck(:submitted_answer) - GeographyHelper::COUNTRIES).to be_empty end it "translates country and state answers using the submission's other answer" do diff --git a/spec/services/address_translation_spec.rb b/spec/services/address_translation_spec.rb index b65b79140d..66e6075560 100644 --- a/spec/services/address_translation_spec.rb +++ b/spec/services/address_translation_spec.rb @@ -48,9 +48,10 @@ expect(translation.country("West Africa", state: "Togo")).to eq("Togo") end - it "keeps a country it can't place as typed and reports it" do - expect(translation.country(" Narnia ", state: "Central Region")).to eq("Narnia") - expect(translation.unmatched[:country]).to eq("Narnia" => 1) + it "clears a country it can't place, so every stored country is a dropdown one, and reports it" do + expect(translation.country(" Narnia ", state: "Central Region")).to be_nil + expect(translation.country("West Africa", state: "-")).to be_nil + expect(translation.unmatched[:country]).to eq("Narnia" => 1, "West Africa" => 1) end it "is nil when neither the country nor the state names one" do From 8ff00daeefed211861377d4a75a0a03588f9ceac Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sat, 24 Oct 2026 18:47:24 -0400 Subject: [PATCH 05/10] Show the address values an LMS import can't match to the dropdowns A blanked country or unreadable state shouldn't land silently; the preview lists them before importing and the notice counts them after. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../event_registration_imports_controller.rb | 11 +++++++- app/services/event_registration_importer.rb | 6 +++-- .../create.html.erb | 9 +++++++ .../event_registration_imports_spec.rb | 26 +++++++++++++++++++ .../event_registration_importer_spec.rb | 8 ++++++ 5 files changed, 57 insertions(+), 3 deletions(-) diff --git a/app/controllers/event_registration_imports_controller.rb b/app/controllers/event_registration_imports_controller.rb index 854f524133..6fe61be3f3 100644 --- a/app/controllers/event_registration_imports_controller.rb +++ b/app/controllers/event_registration_imports_controller.rb @@ -116,6 +116,15 @@ def import_notice(result) "#{", #{result.payments_recorded} payments (#{helpers.dollars_from_cents(result.payments_amount_cents)})" if result.payments_recorded.positive?}" \ "#{", #{result.discounts_recorded} discounts (#{helpers.dollars_from_cents(result.discounts_amount_cents)})" if result.discounts_recorded.positive?}" \ "#{", #{result.ce_enrollments} CE enrollments" if result.ce_enrollments.positive?}" \ - "#{", #{result.skipped.size} rows skipped" if result.skipped.any?}." + "#{", #{result.skipped.size} rows skipped" if result.skipped.any?}" \ + "#{untranslated_notice(result.untranslated_addresses)}." + end + + def untranslated_notice(untranslated) + return if untranslated.empty? + + outcomes = untranslated.keys.map { |field| field == :country ? "country left blank" : "state kept" } + ", #{helpers.pluralize(untranslated.values.sum { |values| values.values.sum }, 'address value')} " \ + "the dropdowns don't offer (#{outcomes.join(', ')})" end end diff --git a/app/services/event_registration_importer.rb b/app/services/event_registration_importer.rb index 3d2a2ae58b..e6e20aeacf 100644 --- a/app/services/event_registration_importer.rb +++ b/app/services/event_registration_importer.rb @@ -132,7 +132,7 @@ def self.importable?(event) :payments_recorded, :payments_amount_cents, :scholarships_recorded, :scholarships_amount_cents, :discounts_recorded, :discounts_amount_cents, - :ce_enrollments, :unrecognized_headers, + :ce_enrollments, :unrecognized_headers, :untranslated_addresses, :skipped, :rows, keyword_init: true ) do @@ -188,7 +188,7 @@ def initialize(file_path:, event:, import_user: nil, source: nil, dry_run: false payments_recorded: 0, payments_amount_cents: 0, scholarships_recorded: 0, scholarships_amount_cents: 0, discounts_recorded: 0, discounts_amount_cents: 0, - ce_enrollments: 0, unrecognized_headers: [], + ce_enrollments: 0, unrecognized_headers: [], untranslated_addresses: {}, skipped: [], rows: [] ) @organization_matcher = OrganizationServices::NameMatcher.new @@ -208,6 +208,8 @@ def call @payment_batches.each_value { |members| apply_payment_batch(members) } raise ActiveRecord::Rollback if @dry_run end + unmatched = @address_translation.unmatched + @result.untranslated_addresses = %i[country state].select { |field| unmatched[field].any? }.index_with { |field| unmatched[field].to_h } @result end diff --git a/app/views/event_registration_imports/create.html.erb b/app/views/event_registration_imports/create.html.erb index 87d7687033..694697ac7d 100644 --- a/app/views/event_registration_imports/create.html.erb +++ b/app/views/event_registration_imports/create.html.erb @@ -41,6 +41,15 @@ <% end %> + <% if @result.untranslated_addresses.any? %> +
+

Address values the dropdowns don't offer

+ <% @result.untranslated_addresses.each do |field, values| %> +

<%= field == :country ? "Country (left blank)" : "State (kept as typed)" %>: <%= values.map { |value, count| "“#{value}”#{" (#{pluralize(count, 'row')})" if count > 1}" }.to_sentence %>

+ <% end %> +
+ <% end %> +
  • Matched organizations are linked to each registration<%= " and add a facilitator affiliation" if @event.facilitator_training %>. diff --git a/spec/requests/event_registration_imports_spec.rb b/spec/requests/event_registration_imports_spec.rb index 342e589692..e8956f317e 100644 --- a/spec/requests/event_registration_imports_spec.rb +++ b/spec/requests/event_registration_imports_spec.rb @@ -128,6 +128,32 @@ def signed_blob end end + describe "address values the dropdowns don't offer" do + before { sign_in admin } + + let(:sheet) do + "FirstName,LastName,EMail,Organization,Street,City,State,Zip,Country\n" \ + "Pat,Example,pat@example.org,Sample Org,1 Main St,Kenmore,Kenmore,98028,Narnia\n" + end + + it "lists them on the preview" do + file = Rack::Test::UploadedFile.new(StringIO.new(sheet), "text/csv", original_filename: "registrants.csv") + + post event_registration_import_path, params: { event_id: event.id, file: } + + expect(response.body).to include("Address values the dropdowns don't offer") + expect(response.body).to include("Country (left blank): “Narnia”", "State (kept as typed): “Kenmore”") + end + + it "counts them in the notice after importing" do + blob = ActiveStorage::Blob.create_and_upload!(io: StringIO.new(sheet), filename: "registrants.csv", content_type: "text/csv") + + post confirm_event_registration_import_path, params: { signed_id: blob.signed_id, event_id: event.id } + + expect(flash[:notice]).to include("2 address values the dropdowns don't offer (country left blank, state kept)") + end + end + describe "POST /event_registrations/import/confirm" do before { sign_in admin } diff --git a/spec/services/event_registration_importer_spec.rb b/spec/services/event_registration_importer_spec.rb index c06d529852..bc9d802d5b 100644 --- a/spec/services/event_registration_importer_spec.rb +++ b/spec/services/event_registration_importer_spec.rb @@ -546,6 +546,14 @@ def registration_of(person) .to have_attributes(state: "CA", country: "United States", address_type: "personal") end + it "leaves a country it can't place blank and reports it with any unreadable state" do + result = import_rows(%w[FirstName LastName EMail Street City State Zip Country], + [ "Pat", "Example", "pat@example.org", "1 Main St", "Kenmore", "Kenmore", "98028", "Narnia" ]) + + expect(Person.find_by(email: "pat@example.org").addresses.sole.country).to be_nil + expect(result.untranslated_addresses).to eq(country: { "Narnia" => 1 }, state: { "Kenmore" => 1 }) + end + it "reads a missing country from the sheet's state" do import_rows(%w[FirstName LastName EMail Street City State Zip], [ "Pat", "Example", "pat@example.org", "1 King St", "Toronto", "ON", "M5H" ]) From 9390b0c03e71466c1197674bcc39f9fd7401a272 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sat, 24 Oct 2026 18:55:43 -0400 Subject: [PATCH 06/10] Build FileMaker addresses in one shared FmAddressAttributes, translated Every FileMaker import (people now, organizations and affiliations next) needs the same row-to-address reading; keeping it beside AddressTranslation keeps all address handling in one place. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 3 +- app/services/fm_address_attributes.rb | 68 +++++++++++++++++++ .../person_services/import_from_rolodex.rb | 56 +-------------- ...maker-organization-and-personnel-import.md | 5 +- .../0009-legacy-address-value-translation.md | 6 +- spec/services/fm_address_attributes_spec.rb | 45 ++++++++++++ 6 files changed, 124 insertions(+), 59 deletions(-) create mode 100644 app/services/fm_address_attributes.rb create mode 100644 spec/services/fm_address_attributes_spec.rb diff --git a/AGENTS.md b/AGENTS.md index 3970108f1f..3e6a4739d9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -266,7 +266,8 @@ or symbol. So write `authorize! @setting` in `show` (not `authorize! @setting, t - `ModelDeduper` — Deduplication logic. On a natural-key collision it now merges the losing row into the survivor (recursively, via itself) so the loser's own children move first — instead of destroying the loser and cascading them away or aborting on a protective `before_destroy` (e.g. a professional license with CE registrations). It finds reassignable children two ways: the model's own `has_many` declarations, and a scan of the DB's foreign keys. **A polymorphic child carries no foreign key**, so it is reachable only through an explicit `has_many … as:` — without one, `#unhandled_references` sees it and the merge is refused outright (this is why `Person`/`Organization` declare `has_many :invoices, as: :invoicee`). A config can also pass `reference_columns:` — column names that point at the model in any table without a DB foreign key (the users deduper names `user_id`/`created_by_id`/`updated_by_id`/`welcome_instructions_sent_by_id`). Self-references on the model's own table follow the keeper, except the keeper's own reference to the duplicate, which clears. The preview lists each moving record by its decorator's optional `#dedupe_label` (Affiliation, ContactMethod, Comment, Notification define one; text snippets cap at `SNIPPET_LENGTH`) - `ContinuingEducationDeduper` — Collapses duplicate CE registrations left sharing one event registration + license after a merge, moving the losers' allocations (payments) and comments onto the survivor. Keeps the real cost (larger of the two) rather than inventing one to cover the combined payments — if they now exceed it, `ContinuingEducationRegistration#over_allocated?` flags it (`ContinuingEducationRegistrationDecorator#overpayment_badge` → "Check payments") for an admin. CE has no DB unique index, so `ModelDeduper` never compares CE rows and just moves them all onto the keeper; a unique index is deliberately avoided because it would force the engine to move CE via validated `update!`, which trips `ContinuingEducationRegistration#license_belongs_to_registrant` mid-merge. Run from the people deduper's `after_merge` - `AddressNormalizer` — Comparison keys for address parts so spellings of one place match: street suffixes/directionals/ordinals (`Street`→`st`, `North`→`n`, `First`→`1st`), unit designators (`Suite`/`Apt`/`Unit`/`#` → `#`), `Saint`/`Mount`/`Fort` in cities, state names → USPS abbreviations, ZIP+4 → five digits, US country spellings (and a blank country in a US state) → `us`, a trailing "County", and an "unknown" type/locality → blank. `.signature(address)` is the whole-address key. Matching only — stored addresses keep their spelling. Used by `AddressDeduper`, `OrganizationServices::AddressMatcher`, `OrganizationServices::DuplicateFinder`'s shared-address signal, and `PersonServices::CaptureFromSubmission`'s mailing-address match -- `AddressTranslation` — Turns imported and legacy address values into what the address dropdowns store (ADR-0009): `#country(value, state:)` → a `GeographyHelper::COUNTRIES` name (aliases for our data's spellings, ISO codes, plain names, a country read from the state — a US state incl. "CA - California", a `REGIONS` province, or a country named as the state; placeholders like "-"/"n/a"/"unknown" → nil; "CA" next to state CA → US), `#state(value, country:)` → USPS code for a US state (other regions kept), `#address_type` (the archive's/forms' words via `CONTACT_TYPES`), `#locality` (`AddressLocality` on the translated parts), and `#address(attrs)` for a whole hash. Lookup tables only, never guessed (like `TagTranslation`): a country nothing answers to is cleared, so every stored country is a dropdown name or blank, and counted in `#unmatched`; an unreadable state is kept and counted. Used by `PersonServices::ImportFromRolodex` (unmatched listed in `fm:import_people`'s report), `EventRegistrationImporter` (country/state answers + address/phone types), and `AddressServices::TranslateExisting`. No save hook — the dropdowns own new data +- `AddressTranslation` — Turns imported and legacy address values into what the address dropdowns store (ADR-0009): `#country(value, state:)` → a `GeographyHelper::COUNTRIES` name (aliases for our data's spellings, ISO codes, plain names, a country read from the state — a US state incl. "CA - California", a `REGIONS` province, or a country named as the state; placeholders like "-"/"n/a"/"unknown" → nil; "CA" next to state CA → US), `#state(value, country:)` → USPS code for a US state (other regions kept), `#address_type` (the archive's/forms' words via `CONTACT_TYPES`), `#locality` (`AddressLocality` on the translated parts), and `#address(attrs)` for a whole hash. Lookup tables only, never guessed (like `TagTranslation`): a country nothing answers to is cleared, so every stored country is a dropdown name or blank, and counted in `#unmatched`; an unreadable state is kept and counted. Used by `FmAddressAttributes` (every FileMaker import), `EventRegistrationImporter` (country/state answers + address/phone types; unmatched values listed on the import preview and counted in its notice), and `AddressServices::TranslateExisting`. No save hook — the dropdowns own new data +- `FmAddressAttributes` — Turns an `FmAddress` row into `Address` attributes for any FileMaker import (street from `StreetNo` + lines, ZIP or its `PostalCodeLookup`, county from the archive's ZIP table read once, everything clipped to 255), with state/country/type/locality through `AddressTranslation`; nil when the row lacks a street, city or state. An `address_type:` overrides the row's (an organization's addresses are work). `#unmatched` gathers what it couldn't translate. Used by `PersonServices::ImportFromRolodex` (listed in `fm:import_people`'s report) and the FileMaker organizations/affiliations imports (ADR-0008) - `AddressServices::TranslateExisting` — Rewrites stored `Address` countries/states and the `mailing_*`/`organization_*` country/state `FormAnswer`s through `AddressTranslation` (each answer read beside its paired state/country answer), fills a locality only where it's Unknown, clears (address) or blanks (answer) a country nothing translates, and logs each changed record as one `autochange.address`/`autochange.form_answer` Ahoy event (all in one visit, `reason: "address_translation"`). Idempotent; `dry_run:` reports without writing. Drives the `data:translate_addresses` rake task - `AddressLocality` — Works out which `Address::LOCALITIES` bucket an address falls in from its own city/state/country/county, for a writer that has an address but never asked the question (`PersonServices::ImportFromRolodex`, `PersonServices::CaptureFromSubmission`, `OrganizationServices::UpsertAddress`, `ProfileChangeRequests::Apply` — all on create only, so a locality a person picked is never overwritten). Reads a named non-US country as **Outside USA**, a non-CA US state as **Outside CA**, a Los Angeles mailing city as **LA City**, the rest of Los Angeles County as **LA County**, and Orange County as **Orange County**. Everything else stays **Unknown**: the Northern/Central/Southern line through California is AWBW's own and nothing records where it runs, and a state no US state answers to could as easily be a typo as a foreign region. Every comparison goes through `AddressNormalizer`. FileMaker records a `Locality` only on a **project**, so the organizations pass (ADR-0008) carries that across rather than deriving it; a person's own address has none - `AddressDeduper` — Merges a person's or organization's addresses that are one place into a single address. `.matching_sets` groups by `AddressNormalizer.signature`, then merges a less complete copy (blank ZIP, street, country, …) into the single fuller address whose every part it shares — skipped when it fits more than one; works fullest-first so a chain of partial copies lands on the fullest. Each merge goes through `ModelDeduper#merge`, which repoints the merged address's inbound FKs — `contact_methods.address_id`, `invoices.bill_to_address_id`, `affiliations.organization_address_id` — onto the survivor. Within a set the survivor is the copy still in use — active before inactive, then the primary, then the oldest (`.keeper_order`) — since `Address.active` drives bill-to addresses, an org's program location and the geographic breakdowns; blank columns on the survivor (`CARRIED_COLUMNS` — the street/ZIP/country/county/district/type/locality, `phone` and the three `la_*` districts; "unknown" type/locality counts as blank) are filled from the merged copies. Then settles a single primary on the set holding the signature captured in the deduper's `merge_keeper` (the survivor's own pre-merge primary address, else the deleted record's), demoting the rest. Runs in the people and organizations dedupers' `after_merge`; `AddressDeduper.merge_duplicates(addressable)` runs it standalone (keeping the owner's current primary) for the **Dedupe addresses** admin page (`AddressesController#dedupe_index`/`#dedupe_perform`) diff --git a/app/services/fm_address_attributes.rb b/app/services/fm_address_attributes.rb new file mode 100644 index 0000000000..3a5090a073 --- /dev/null +++ b/app/services/fm_address_attributes.rb @@ -0,0 +1,68 @@ +# Turns an FmAddress row's fields into Address attributes — state, country, type and +# locality translated as the dropdowns store them (ADR-0009) — or nil when the row lacks the +# street, city or state an address needs. One instance serves a whole import: the archive's +# ZIP table is read once, and #unmatched gathers every value it couldn't translate. +class FmAddressAttributes + STRING_LIMIT = 255 + + def initialize + @translation = AddressTranslation.new + end + + def unmatched = @translation.unmatched + + def call(fields, address_type: nil) + street = [ street_line(fields), fields["AddrsLine2"] ].filter_map { |line| line.to_s.strip.presence }.join(", ") + city = clip(fields["AddrsCity"].to_s.strip) + state = clip(fields["AddrsState"].to_s.strip) + return if street.blank? || city.blank? || state.blank? + + @translation.address( + street_address: clip(street), + city:, + state:, + zip_code: zip_code(fields), + country: clip(fields["AddrsCountry"]), + county: county(fields), + address_type: fields["AddrsType"] + ).then { |attributes| address_type ? attributes.merge(address_type:) : attributes } + end + + private + + # The house number has its own field, sometimes already repeated at the front of + # the first line. + def street_line(fields) + number = fields["StreetNo"].to_s.strip + line = fields["AddrsLine1"].to_s.strip + return line if number.blank? || line[/\A\S+/] == number + + "#{number} #{line}".strip + end + + # The address's own postal code, or the one it points at in the archive's ZIP table + # when it carries none itself — the field it mirrors, and where the county is read. + def zip_code(fields) + [ fields["AddrsPostalCode"], fields["PostalCodeLookup"] ] + .filter_map { |value| value.to_s.strip.presence }.first.to_s.truncate(STRING_LIMIT) + end + + def county(fields) + zip_counties[zip_lookup(fields)[/\A\d{5}/]] + end + + def zip_lookup(fields) + (fields["PostalCodeLookup"].presence || fields["AddrsPostalCode"]).to_s.strip + end + + # One query for the archive's ~80,000 zip codes beats one per address across a full import. + def zip_counties + @zip_counties ||= FmPostalCode.pluck(:fm_id, Arel.sql("JSON_UNQUOTE(JSON_EXTRACT(data, '$.County'))")) + .to_h { |zip, county| [ zip, clip(county) ] } + end + + # The archive's free text isn't bounded by the varchar(255) it lands in. + def clip(value) + value.presence && value.to_s.truncate(STRING_LIMIT) + end +end diff --git a/app/services/person_services/import_from_rolodex.rb b/app/services/person_services/import_from_rolodex.rb index 5c88f3cc4d..a37c60a122 100644 --- a/app/services/person_services/import_from_rolodex.rb +++ b/app/services/person_services/import_from_rolodex.rb @@ -74,7 +74,7 @@ def initialize(scope: FmRolodex.all, relink: false, progress: nil, run: nil) @imported_codes = Person.where.not(filemaker_code: [ nil, "" ]).pluck(:filemaker_code) .flat_map { |value| Person.split_filemaker_codes(value) }.to_set @translation = TagTranslation.new - @address_translation = AddressTranslation.new + @address_attributes = FmAddressAttributes.new @staff = StaffTranslation.new @import_user_id = Setting.import_user&.id end @@ -104,7 +104,7 @@ def unmatched_staff end def unmatched_addresses - @address_translation.unmatched + @address_attributes.unmatched end private @@ -399,7 +399,7 @@ def names(model, ids) def build_addresses(person, rolodex) held = person.addresses.map { |address| address_key(address.street_address, address.city) } attributes = rolodex.addresses.reject { |address| work_address?(rolodex, address) } - .filter_map { |address| address_attributes(address.data) } + .filter_map { |address| @address_attributes.call(address.data) } .reject { |attrs| held.include?(address_key(attrs[:street_address], attrs[:city])) } build_primary_first(person.addresses, attributes) end @@ -430,46 +430,6 @@ def phone_key(value) value.to_s.gsub(/\D/, "") end - def address_attributes(fields) - street = [ street_line(fields), fields["AddrsLine2"] ].filter_map { |line| line.to_s.strip.presence }.join(", ") - city = clip(fields["AddrsCity"].to_s.strip) - state = clip(fields["AddrsState"].to_s.strip) - return if street.blank? || city.blank? || state.blank? - - # FileMaker records a locality only on a project, so a person's own address has - # none to carry over and the translated parts have to say. - @address_translation.address( - street_address: clip(street), - city:, - state:, - zip_code: zip_code(fields), - country: clip(fields["AddrsCountry"]), - county: county(fields), - address_type: fields["AddrsType"] - ) - end - - # The address's own postal code, or the one it points at in the archive's ZIP table - # when it carries none itself — the field it mirrors, and where the county is read. - def zip_code(fields) - [ fields["AddrsPostalCode"], fields["PostalCodeLookup"] ] - .filter_map { |value| value.to_s.strip.presence }.first.to_s.truncate(STRING_LIMIT) - end - - def county(fields) - zip_counties[zip_lookup(fields)[/\A\d{5}/]] - end - - def zip_lookup(fields) - (fields["PostalCodeLookup"].presence || fields["AddrsPostalCode"]).to_s.strip - end - - # One query for the archive's ~80,000 zip codes beats one per address across a full import. - def zip_counties - @zip_counties ||= FmPostalCode.pluck(:fm_id, Arel.sql("JSON_UNQUOTE(JSON_EXTRACT(data, '$.County'))")) - .to_h { |zip, county| [ zip, clip(county) ] } - end - def phone_attributes(fields) number = clip(fields["PhoneNumber"]) return if number.blank? @@ -484,16 +444,6 @@ def contact_type(value, allowed:) CONTACT_TYPES[value.to_s.strip.downcase].presence_in(allowed) end - # The house number has its own field, sometimes already repeated at the front of - # the first line. - def street_line(fields) - number = fields["StreetNo"].to_s.strip - line = fields["AddrsLine1"].to_s.strip - return line if number.blank? || line[/\A\S+/] == number - - "#{number} #{line}".strip - end - # Every string copied here lands in a varchar(255) the archive's free text isn't # bounded by, and an over-long incidental field would otherwise cost the person — # or, for a column with no length validation behind it, raise mid-import. diff --git a/docs/adr/0008-filemaker-organization-and-personnel-import.md b/docs/adr/0008-filemaker-organization-and-personnel-import.md index a6750a0adf..0a55b214ab 100644 --- a/docs/adr/0008-filemaker-organization-and-personnel-import.md +++ b/docs/adr/0008-filemaker-organization-and-personnel-import.md @@ -59,8 +59,9 @@ Constraints already decided elsewhere: no locality to carry, which is why the Rolodex pass works it out with `AddressLocality`. Those project rows also carry real County → Locality pairs, so they are where to read AWBW's Northern/Central/Southern line from instead of inventing it. -- **State, country and address type go through `AddressTranslation#address`** (ADR-0009), - as the Rolodex pass does, so organization addresses store what the dropdowns offer. +- **Addresses are built by `FmAddressAttributes`** (ADR-0009), as the Rolodex pass does, so + state, country, type and locality are stored as the dropdowns offer; a project's city/state + goes through `AddressTranslation` the same way. - **Conflicts:** same rule as people — fill blanks, keep the portal value where the archive disagrees, and report each difference as a comment plus in the task output. - **Out of scope:** event registrations (`fm_participants`, 68,497 rows), payments, funding, diff --git a/docs/adr/0009-legacy-address-value-translation.md b/docs/adr/0009-legacy-address-value-translation.md index 2ad0fd0621..338ec3df86 100644 --- a/docs/adr/0009-legacy-address-value-translation.md +++ b/docs/adr/0009-legacy-address-value-translation.md @@ -47,9 +47,9 @@ since a state is required and regions outside the US are legitimate. ### Where it runs -- **Imports, inline:** the FileMaker Rolodex import and the LMS registration importer, and - the FileMaker organizations import when it's built (ADR-0008). Each reports what it - couldn't place. +- **Imports, inline:** every FileMaker import builds addresses through `FmAddressAttributes` + (the Rolodex import now, the organizations and affiliations imports of ADR-0008), and the + LMS registration importer translates its answers. Each reports what it couldn't place. - **Existing data, once and again:** `AddressServices::TranslateExisting` (`data:translate_addresses`) rewrites stored address countries and states, and the country/state form answers, filling a locality only where it's Unknown. It's idempotent, so it runs before and after an import. Every changed diff --git a/spec/services/fm_address_attributes_spec.rb b/spec/services/fm_address_attributes_spec.rb new file mode 100644 index 0000000000..551e22a8ce --- /dev/null +++ b/spec/services/fm_address_attributes_spec.rb @@ -0,0 +1,45 @@ +require "rails_helper" + +RSpec.describe FmAddressAttributes do + subject(:builder) { described_class.new } + + let(:row) do + { "StreetNo" => "1234", "AddrsLine1" => "Main St", "AddrsLine2" => "Apt 2", "AddrsCity" => "Los Angeles", + "AddrsState" => "California", "AddrsPostalCode" => "90001", "AddrsCountry" => "USA", "AddrsType" => "Home" } + end + + it "builds translated address attributes from an archive row" do + expect(builder.call(row)).to eq( + street_address: "1234 Main St, Apt 2", city: "Los Angeles", state: "CA", zip_code: "90001", + country: "United States", county: nil, address_type: "personal", locality: Address::LA_CITY + ) + end + + it "takes the address type the caller gives over the row's" do + expect(builder.call(row, address_type: "work")).to include(address_type: "work") + end + + it "reads the county and a missing ZIP from the archive's ZIP table" do + FmPostalCode.create!(fm_id: "91101", data: { "County" => "Los Angeles" }) + + attributes = builder.call(row.merge("AddrsCity" => "Pasadena", "AddrsPostalCode" => nil, "PostalCodeLookup" => "91101")) + + expect(attributes).to include(zip_code: "91101", county: "Los Angeles", locality: Address::LA_COUNTY) + end + + it "reads a missing country from the state" do + expect(builder.call(row.merge("AddrsState" => "BC", "AddrsCountry" => nil))).to include(state: "BC", country: "Canada") + end + + it "skips a row with no street, city, or state" do + expect(builder.call(row.merge("AddrsLine1" => "", "StreetNo" => "", "AddrsLine2" => ""))).to be_nil + expect(builder.call(row.merge("AddrsCity" => " "))).to be_nil + expect(builder.call(row.merge("AddrsState" => nil))).to be_nil + end + + it "reports the values it can't translate" do + builder.call(row.merge("AddrsCountry" => "Narnia", "AddrsState" => "ZZ")) + + expect(builder.unmatched[:country]).to eq("Narnia" => 1) + end +end From 467fd73fc9faedd791ee08a2176856c89af2d7bb Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sat, 24 Oct 2026 18:56:34 -0400 Subject: [PATCH 07/10] Translate street-less FileMaker places (a project's city) the same way Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 +- app/services/fm_address_attributes.rb | 7 +++++++ spec/services/fm_address_attributes_spec.rb | 10 ++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 3e6a4739d9..90fd55121f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -267,7 +267,7 @@ or symbol. So write `authorize! @setting` in `show` (not `authorize! @setting, t - `ContinuingEducationDeduper` — Collapses duplicate CE registrations left sharing one event registration + license after a merge, moving the losers' allocations (payments) and comments onto the survivor. Keeps the real cost (larger of the two) rather than inventing one to cover the combined payments — if they now exceed it, `ContinuingEducationRegistration#over_allocated?` flags it (`ContinuingEducationRegistrationDecorator#overpayment_badge` → "Check payments") for an admin. CE has no DB unique index, so `ModelDeduper` never compares CE rows and just moves them all onto the keeper; a unique index is deliberately avoided because it would force the engine to move CE via validated `update!`, which trips `ContinuingEducationRegistration#license_belongs_to_registrant` mid-merge. Run from the people deduper's `after_merge` - `AddressNormalizer` — Comparison keys for address parts so spellings of one place match: street suffixes/directionals/ordinals (`Street`→`st`, `North`→`n`, `First`→`1st`), unit designators (`Suite`/`Apt`/`Unit`/`#` → `#`), `Saint`/`Mount`/`Fort` in cities, state names → USPS abbreviations, ZIP+4 → five digits, US country spellings (and a blank country in a US state) → `us`, a trailing "County", and an "unknown" type/locality → blank. `.signature(address)` is the whole-address key. Matching only — stored addresses keep their spelling. Used by `AddressDeduper`, `OrganizationServices::AddressMatcher`, `OrganizationServices::DuplicateFinder`'s shared-address signal, and `PersonServices::CaptureFromSubmission`'s mailing-address match - `AddressTranslation` — Turns imported and legacy address values into what the address dropdowns store (ADR-0009): `#country(value, state:)` → a `GeographyHelper::COUNTRIES` name (aliases for our data's spellings, ISO codes, plain names, a country read from the state — a US state incl. "CA - California", a `REGIONS` province, or a country named as the state; placeholders like "-"/"n/a"/"unknown" → nil; "CA" next to state CA → US), `#state(value, country:)` → USPS code for a US state (other regions kept), `#address_type` (the archive's/forms' words via `CONTACT_TYPES`), `#locality` (`AddressLocality` on the translated parts), and `#address(attrs)` for a whole hash. Lookup tables only, never guessed (like `TagTranslation`): a country nothing answers to is cleared, so every stored country is a dropdown name or blank, and counted in `#unmatched`; an unreadable state is kept and counted. Used by `FmAddressAttributes` (every FileMaker import), `EventRegistrationImporter` (country/state answers + address/phone types; unmatched values listed on the import preview and counted in its notice), and `AddressServices::TranslateExisting`. No save hook — the dropdowns own new data -- `FmAddressAttributes` — Turns an `FmAddress` row into `Address` attributes for any FileMaker import (street from `StreetNo` + lines, ZIP or its `PostalCodeLookup`, county from the archive's ZIP table read once, everything clipped to 255), with state/country/type/locality through `AddressTranslation`; nil when the row lacks a street, city or state. An `address_type:` overrides the row's (an organization's addresses are work). `#unmatched` gathers what it couldn't translate. Used by `PersonServices::ImportFromRolodex` (listed in `fm:import_people`'s report) and the FileMaker organizations/affiliations imports (ADR-0008) +- `FmAddressAttributes` — Turns an `FmAddress` row into `Address` attributes for any FileMaker import (street from `StreetNo` + lines, ZIP or its `PostalCodeLookup`, county from the archive's ZIP table read once, everything clipped to 255), with state/country/type/locality through `AddressTranslation`; nil when the row lacks a street, city or state. An `address_type:` overrides the row's (an organization's addresses are work); `#place(city:, state:, county:)` builds a street-less address for a place the archive records by city alone (a project). `#unmatched` gathers what it couldn't translate. Used by `PersonServices::ImportFromRolodex` (listed in `fm:import_people`'s report) and the FileMaker organizations/affiliations imports (ADR-0008) - `AddressServices::TranslateExisting` — Rewrites stored `Address` countries/states and the `mailing_*`/`organization_*` country/state `FormAnswer`s through `AddressTranslation` (each answer read beside its paired state/country answer), fills a locality only where it's Unknown, clears (address) or blanks (answer) a country nothing translates, and logs each changed record as one `autochange.address`/`autochange.form_answer` Ahoy event (all in one visit, `reason: "address_translation"`). Idempotent; `dry_run:` reports without writing. Drives the `data:translate_addresses` rake task - `AddressLocality` — Works out which `Address::LOCALITIES` bucket an address falls in from its own city/state/country/county, for a writer that has an address but never asked the question (`PersonServices::ImportFromRolodex`, `PersonServices::CaptureFromSubmission`, `OrganizationServices::UpsertAddress`, `ProfileChangeRequests::Apply` — all on create only, so a locality a person picked is never overwritten). Reads a named non-US country as **Outside USA**, a non-CA US state as **Outside CA**, a Los Angeles mailing city as **LA City**, the rest of Los Angeles County as **LA County**, and Orange County as **Orange County**. Everything else stays **Unknown**: the Northern/Central/Southern line through California is AWBW's own and nothing records where it runs, and a state no US state answers to could as easily be a typo as a foreign region. Every comparison goes through `AddressNormalizer`. FileMaker records a `Locality` only on a **project**, so the organizations pass (ADR-0008) carries that across rather than deriving it; a person's own address has none - `AddressDeduper` — Merges a person's or organization's addresses that are one place into a single address. `.matching_sets` groups by `AddressNormalizer.signature`, then merges a less complete copy (blank ZIP, street, country, …) into the single fuller address whose every part it shares — skipped when it fits more than one; works fullest-first so a chain of partial copies lands on the fullest. Each merge goes through `ModelDeduper#merge`, which repoints the merged address's inbound FKs — `contact_methods.address_id`, `invoices.bill_to_address_id`, `affiliations.organization_address_id` — onto the survivor. Within a set the survivor is the copy still in use — active before inactive, then the primary, then the oldest (`.keeper_order`) — since `Address.active` drives bill-to addresses, an org's program location and the geographic breakdowns; blank columns on the survivor (`CARRIED_COLUMNS` — the street/ZIP/country/county/district/type/locality, `phone` and the three `la_*` districts; "unknown" type/locality counts as blank) are filled from the merged copies. Then settles a single primary on the set holding the signature captured in the deduper's `merge_keeper` (the survivor's own pre-merge primary address, else the deleted record's), demoting the rest. Runs in the people and organizations dedupers' `after_merge`; `AddressDeduper.merge_duplicates(addressable)` runs it standalone (keeping the owner's current primary) for the **Dedupe addresses** admin page (`AddressesController#dedupe_index`/`#dedupe_perform`) diff --git a/app/services/fm_address_attributes.rb b/app/services/fm_address_attributes.rb index 3a5090a073..596c4d3de7 100644 --- a/app/services/fm_address_attributes.rb +++ b/app/services/fm_address_attributes.rb @@ -28,6 +28,13 @@ def call(fields, address_type: nil) ).then { |attributes| address_type ? attributes.merge(address_type:) : attributes } end + # An address holding just a city and state — where the archive records a place (a + # project's city) with no street — translated the same way. + def place(city:, state:, county: nil) + @translation.address(street_address: "", zip_code: "", city: clip(city), state: clip(state), country: nil, + county: clip(county), address_type: nil) + end + private # The house number has its own field, sometimes already repeated at the front of diff --git a/spec/services/fm_address_attributes_spec.rb b/spec/services/fm_address_attributes_spec.rb index 551e22a8ce..549f7b20d0 100644 --- a/spec/services/fm_address_attributes_spec.rb +++ b/spec/services/fm_address_attributes_spec.rb @@ -37,6 +37,16 @@ expect(builder.call(row.merge("AddrsState" => nil))).to be_nil end + describe "#place" do + it "builds a street-less address for a city and state, translated" do + expect(builder.place(city: "Toronto", state: "Ontario")).to eq( + street_address: "", zip_code: "", city: "Toronto", state: "Ontario", country: "Canada", + county: nil, address_type: nil, locality: Address::OUTSIDE_USA + ) + expect(builder.place(city: "Fresno", state: "California", county: "Fresno")).to include(state: "CA", country: "United States") + end + end + it "reports the values it can't translate" do builder.call(row.merge("AddrsCountry" => "Narnia", "AddrsState" => "ZZ")) From c5b236a14470a66da4ce21dee73035c02a9f6db2 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sat, 24 Oct 2026 19:51:55 -0400 Subject: [PATCH 08/10] Save data:translate_addresses' report like fm:import_people does A console-only report scrolls away; the list of values nothing translates is what someone fixes by hand afterwards, so every run keeps a copy in storage. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 +- lib/tasks/translate_addresses.rake | 43 +++++++++++++++------ spec/tasks/translate_addresses_spec.rb | 52 ++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 12 deletions(-) create mode 100644 spec/tasks/translate_addresses_spec.rb diff --git a/AGENTS.md b/AGENTS.md index 90fd55121f..f6fa7bbb5e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -607,7 +607,7 @@ Located in `lib/tasks/` (20 files): - `migrate_workshop_logs.rake` — Workshop log migration - `backfill_user_stamps.rake` — Backfill `created_by_id`/`updated_by_id` on legacy rows from the Ahoy trail (`data:backfill_user_stamps`) - `backfill_notification_stamps.rake` — Backfill notifications' `created_by_id`/`updated_by_id` from `sender_id`, nil-only (`data:backfill_notification_stamps`) -- `translate_addresses.rake` — Rewrite stored address countries/states and their form answers as the dropdowns store them (`data:translate_addresses`, via `AddressServices::TranslateExisting`, idempotent, DRY_RUN, lists values nothing translates) +- `translate_addresses.rake` — Rewrite stored address countries/states and their form answers as the dropdowns store them (`data:translate_addresses`, via `AddressServices::TranslateExisting`, idempotent, DRY_RUN, lists values nothing translates; every run, dry or not, saves its report as an ActiveStorage blob and prints how to read it back) - `fm_import.rake` — Load the FileMaker CSV exports into the `fm_*` archive tables (`fm:import`, reads each CSV from the `FM Archive` resources, batch `insert_all`, skips rows whose key is already stored) - `fm_import_people.rake` — Create people from the FileMaker Rolodex archive (`fm:import_people`, via `PersonServices::ImportFromRolodex`, links existing by name + email, idempotent, DRY_RUN; VERBOSE=true lists every row, FM_IDS=a,b re-runs just those rows past the idempotence guard; commits every 1,000 rows, prints progress, saves the full report as an ActiveStorage blob even on a crash, and aborts when no `TagAlias` rows exist or no `Setting.import_user` is chosen; logs every record it creates or fills as activity under a run name (`Current.source = "fm_import_people"`), which `fm:undo_import_people RUN=…` reverses via `PersonServices::UndoRolodexImport`) - `resolve_resource_organizations.rake` — Link resources to organizations from the legacy `agency` free-text name (`data:resolve_resource_organizations`, match-or-create, idempotent, DRY_RUN) diff --git a/lib/tasks/translate_addresses.rake b/lib/tasks/translate_addresses.rake index 6c716de3f4..cc0cf76062 100644 --- a/lib/tasks/translate_addresses.rake +++ b/lib/tasks/translate_addresses.rake @@ -4,25 +4,46 @@ # store them. See AddressServices::TranslateExisting. # # Dry-run by default. Set DRY_RUN=false to write changes (each one logged as an Ahoy event). +# Every run, dry or not, also saves its report as an ActiveStorage blob — the command to +# read it back is printed at the end. # bin/rails data:translate_addresses # preview, writes nothing # DRY_RUN=false bin/rails data:translate_addresses # translate + log namespace :data do desc "Translate stored address countries/states and their form answers to dropdown values. Idempotent. DRY_RUN=false to execute." task translate_addresses: :environment do dry_run = ENV["DRY_RUN"] != "false" - result = AddressServices::TranslateExisting.new(dry_run:).call + report = [] + say = ->(line = "") { puts line; report << line } + $stdout.sync = true - puts "Changes (#{result.changes.values.sum}):" - result.changes.sort_by { |label, count| [ -count, label ] }.each { |label, count| puts " #{count.to_s.rjust(5)} #{label}" } - result.unmatched.each do |field, values| - next if values.empty? + begin + result = AddressServices::TranslateExisting.new(dry_run:).call - outcome = field == :country ? "left blank, was logged" : "kept as typed" - puts "\n#{field.to_s.humanize} values nothing translates (#{outcome} — fix by hand if you know it):" - values.sort_by { |value, count| [ -count, value ] }.each { |value, count| puts " #{count.to_s.rjust(5)} #{value}" } + say.("Changes (#{result.changes.values.sum}):") + result.changes.sort_by { |label, count| [ -count, label ] }.each { |label, count| say.(" #{count.to_s.rjust(5)} #{label}") } + result.unmatched.each do |field, values| + next if values.empty? + + outcome = field == :country ? "left blank, was logged" : "kept as typed" + say.("\n#{field.to_s.humanize} values nothing translates (#{outcome} — fix by hand if you know it):") + values.sort_by { |value, count| [ -count, value ] }.each { |value, count| say.(" #{count.to_s.rjust(5)} #{value}") } + end + say.("\naddresses changed: #{result.addresses}") + say.("answers changed: #{result.answers}") + say.(dry_run ? "\nDRY RUN — nothing written. Set DRY_RUN=false to execute." : "\nDone.") + rescue StandardError => e + say.("\nStopped by #{e.class}: #{e.message}#{" — nothing was written; rerun to continue." unless dry_run}") + raise + ensure + begin + blob = ActiveStorage::Blob.create_and_upload!( + io: StringIO.new(report.join("\n")), content_type: "text/plain", + filename: "translate-addresses-#{Time.current.strftime("%Y%m%d-%H%M%S")}#{"-dry-run" if dry_run}.txt" + ) + puts "\nReport saved. Read it back with: bin/rails runner 'puts ActiveStorage::Blob.find(#{blob.id}).download'" + rescue StandardError => e + puts "\nCould not save the report (#{e.class}: #{e.message}) — copy the output above." + end end - puts "\naddresses changed: #{result.addresses}" - puts "answers changed: #{result.answers}" - puts(dry_run ? "\nDRY RUN — nothing written. Set DRY_RUN=false to execute." : "\nDone.") end end diff --git a/spec/tasks/translate_addresses_spec.rb b/spec/tasks/translate_addresses_spec.rb new file mode 100644 index 0000000000..dd94ec029d --- /dev/null +++ b/spec/tasks/translate_addresses_spec.rb @@ -0,0 +1,52 @@ +require "rails_helper" +require "rake" + +RSpec.describe "data:translate_addresses" do + before(:all) do + Rails.application.load_tasks unless Rake::Task.task_defined?("data:translate_addresses") + end + + before { Rake::Task["data:translate_addresses"].reenable } + + after { ENV.delete("DRY_RUN") } + + def run_task + original = $stdout + $stdout = StringIO.new + Rake::Task["data:translate_addresses"].invoke + $stdout.string + ensure + $stdout = original + end + + def address_with(country:, state:) + create(:address, addressable: create(:person)).tap { |address| address.update_columns(country:, state:) } + end + + def saved_report = ActiveStorage::Blob.order(:id).last + + def report_text = saved_report.download.force_encoding(Encoding::UTF_8) + + it "previews by default, writing no addresses, and saves the report" do + usa = address_with(country: "USA", state: "TX") + address_with(country: "Narnia", state: "Central Region") + + output = run_task + + expect(usa.reload.country).to eq("USA") + expect(output).to include("DRY RUN", "Narnia", "Report saved") + expect(saved_report.filename.to_s).to match(/\Atranslate-addresses-\d{8}-\d{6}-dry-run\.txt\z/) + expect(report_text).to include("Address country: \"USA\" → \"United States\"", "Narnia") + end + + it "writes with DRY_RUN=false and saves that run's report too" do + usa = address_with(country: "USA", state: "TX") + ENV["DRY_RUN"] = "false" + + run_task + + expect(usa.reload.country).to eq("United States") + expect(saved_report.filename.to_s).not_to include("dry-run") + expect(report_text).to include("Done.") + end +end From 2dadbc964ea0d16436768e67d6d3775d66a64f78 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sat, 24 Oct 2026 19:54:46 -0400 Subject: [PATCH 09/10] Pin that a translate_addresses dry run creates no Ahoy events or visits Co-Authored-By: Claude Opus 5.5 (1M context) --- spec/services/address_services/translate_existing_spec.rb | 2 +- spec/tasks/translate_addresses_spec.rb | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/spec/services/address_services/translate_existing_spec.rb b/spec/services/address_services/translate_existing_spec.rb index 4a70c483e9..adc5c320ad 100644 --- a/spec/services/address_services/translate_existing_spec.rb +++ b/spec/services/address_services/translate_existing_spec.rb @@ -127,7 +127,7 @@ def answer(submission, identifier, value) usa = address_with(country: "USA", state: "TX") result = nil - expect { result = described_class.new(dry_run: true).call }.not_to change(Ahoy::Event, :count) + expect { result = described_class.new(dry_run: true).call }.not_to change { [ Ahoy::Event.count, Ahoy::Visit.count ] } expect(stored(usa).first).to eq("USA") expect(result.changes).to include("Address country: \"USA\" → \"United States\"" => 1) diff --git a/spec/tasks/translate_addresses_spec.rb b/spec/tasks/translate_addresses_spec.rb index dd94ec029d..6dd2553b67 100644 --- a/spec/tasks/translate_addresses_spec.rb +++ b/spec/tasks/translate_addresses_spec.rb @@ -31,7 +31,8 @@ def report_text = saved_report.download.force_encoding(Encoding::UTF_8) usa = address_with(country: "USA", state: "TX") address_with(country: "Narnia", state: "Central Region") - output = run_task + output = nil + expect { output = run_task }.not_to change { [ Ahoy::Event.count, Ahoy::Visit.count ] } expect(usa.reload.country).to eq("USA") expect(output).to include("DRY RUN", "Narnia", "Report saved") From d36bec4d04a2cc7beffb9e392f5dc114abc59179 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sat, 24 Oct 2026 19:59:31 -0400 Subject: [PATCH 10/10] Credit translate_addresses' logged changes to the Settings import user Every rewrite should name who made it; the same import user fm:import_people credits is the one to use, and a real run won't proceed without one. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 +- lib/tasks/translate_addresses.rake | 9 +++++++-- spec/tasks/translate_addresses_spec.rb | 13 ++++++++++++- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index f6fa7bbb5e..7099d2f963 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -607,7 +607,7 @@ Located in `lib/tasks/` (20 files): - `migrate_workshop_logs.rake` — Workshop log migration - `backfill_user_stamps.rake` — Backfill `created_by_id`/`updated_by_id` on legacy rows from the Ahoy trail (`data:backfill_user_stamps`) - `backfill_notification_stamps.rake` — Backfill notifications' `created_by_id`/`updated_by_id` from `sender_id`, nil-only (`data:backfill_notification_stamps`) -- `translate_addresses.rake` — Rewrite stored address countries/states and their form answers as the dropdowns store them (`data:translate_addresses`, via `AddressServices::TranslateExisting`, idempotent, DRY_RUN, lists values nothing translates; every run, dry or not, saves its report as an ActiveStorage blob and prints how to read it back) +- `translate_addresses.rake` — Rewrite stored address countries/states and their form answers as the dropdowns store them (`data:translate_addresses`, via `AddressServices::TranslateExisting`, idempotent, DRY_RUN, lists values nothing translates; every run, dry or not, saves its report as an ActiveStorage blob and prints how to read it back; a real run credits its Ahoy events to `Setting.import_user` and aborts when none is chosen) - `fm_import.rake` — Load the FileMaker CSV exports into the `fm_*` archive tables (`fm:import`, reads each CSV from the `FM Archive` resources, batch `insert_all`, skips rows whose key is already stored) - `fm_import_people.rake` — Create people from the FileMaker Rolodex archive (`fm:import_people`, via `PersonServices::ImportFromRolodex`, links existing by name + email, idempotent, DRY_RUN; VERBOSE=true lists every row, FM_IDS=a,b re-runs just those rows past the idempotence guard; commits every 1,000 rows, prints progress, saves the full report as an ActiveStorage blob even on a crash, and aborts when no `TagAlias` rows exist or no `Setting.import_user` is chosen; logs every record it creates or fills as activity under a run name (`Current.source = "fm_import_people"`), which `fm:undo_import_people RUN=…` reverses via `PersonServices::UndoRolodexImport`) - `resolve_resource_organizations.rake` — Link resources to organizations from the legacy `agency` free-text name (`data:resolve_resource_organizations`, match-or-create, idempotent, DRY_RUN) diff --git a/lib/tasks/translate_addresses.rake b/lib/tasks/translate_addresses.rake index cc0cf76062..a9a42ebc0d 100644 --- a/lib/tasks/translate_addresses.rake +++ b/lib/tasks/translate_addresses.rake @@ -3,7 +3,8 @@ # Rewrite stored address countries/states and country/state form answers as the dropdowns # store them. See AddressServices::TranslateExisting. # -# Dry-run by default. Set DRY_RUN=false to write changes (each one logged as an Ahoy event). +# Dry-run by default. Set DRY_RUN=false to write changes (each one logged as an Ahoy event +# credited to Setting.import_user, which a real run requires). # Every run, dry or not, also saves its report as an ActiveStorage blob — the command to # read it back is printed at the end. # bin/rails data:translate_addresses # preview, writes nothing @@ -12,12 +13,16 @@ namespace :data do desc "Translate stored address countries/states and their form answers to dropdown values. Idempotent. DRY_RUN=false to execute." task translate_addresses: :environment do dry_run = ENV["DRY_RUN"] != "false" + import_user = Setting.import_user + if !dry_run && import_user.nil? + abort "No import user chosen — pick one in admin Settings (Imports); every change this run logs is credited to them." + end report = [] say = ->(line = "") { puts line; report << line } $stdout.sync = true begin - result = AddressServices::TranslateExisting.new(dry_run:).call + result = AddressServices::TranslateExisting.new(dry_run:, user: import_user).call say.("Changes (#{result.changes.values.sum}):") result.changes.sort_by { |label, count| [ -count, label ] }.each { |label, count| say.(" #{count.to_s.rjust(5)} #{label}") } diff --git a/spec/tasks/translate_addresses_spec.rb b/spec/tasks/translate_addresses_spec.rb index 6dd2553b67..5a81f62698 100644 --- a/spec/tasks/translate_addresses_spec.rb +++ b/spec/tasks/translate_addresses_spec.rb @@ -40,14 +40,25 @@ def report_text = saved_report.download.force_encoding(Encoding::UTF_8) expect(report_text).to include("Address country: \"USA\" → \"United States\"", "Narnia") end - it "writes with DRY_RUN=false and saves that run's report too" do + it "writes with DRY_RUN=false, credits each change to the import user, and saves that run's report" do + import_user = create(:user) + Setting.create!(import_user:) usa = address_with(country: "USA", state: "TX") ENV["DRY_RUN"] = "false" run_task expect(usa.reload.country).to eq("United States") + expect(Ahoy::Event.where(name: "autochange.address", resource_id: usa.id).pluck(:user_id)).to eq([ import_user.id ]) expect(saved_report.filename.to_s).not_to include("dry-run") expect(report_text).to include("Done.") end + + it "won't write without an import user to credit" do + usa = address_with(country: "USA", state: "TX") + ENV["DRY_RUN"] = "false" + + expect { run_task }.to raise_error(SystemExit) + expect(usa.reload.country).to eq("USA") + end end