From 25e9978b3a460049ad4755f593a49b39093903e4 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 05:47:56 -0400 Subject: [PATCH 01/10] Start: keep form answers verbatim; translate where answers become addresses Co-Authored-By: Claude Opus 5.5 (1M context) From 7e1e31efe34e6d044b6e8a8a93616afd335100d0 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 05:53:28 -0400 Subject: [PATCH 02/10] Keep form answers as typed; translate where answers become addresses An answer is a record of what someone submitted, and the dashboards count addresses, not answers. The one real risk of untranslated answers is link-organization and capture re-reading an old "USA" into an address, so those two write points translate instead. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 6 +- app/models/form_answer.rb | 4 +- .../address_services/translate_existing.rb | 82 ++++++------------- .../organization_services/upsert_address.rb | 5 +- .../capture_from_submission.rb | 5 +- .../2026-10-25-form-answers-stay-as-typed.yml | 9 ++ .../0009-legacy-address-value-translation.md | 20 +++-- lib/tasks/translate_addresses.rake | 7 +- spec/requests/event_registrations_spec.rb | 2 +- .../translate_existing_spec.rb | 69 ++++------------ .../public_registration_spec.rb | 6 +- .../upsert_address_spec.rb | 21 ++++- .../capture_from_submission_spec.rb | 14 ++++ spec/tasks/translate_addresses_spec.rb | 4 +- 14 files changed, 114 insertions(+), 140 deletions(-) create mode 100644 db/seeds/features/2026-10-25-form-answers-stay-as-typed.yml diff --git a/AGENTS.md b/AGENTS.md index 4e25420a52..facc8f833b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -269,9 +269,9 @@ comparison — add a rule if none fits (e.g. `PersonPolicy#own_profile?`). Conve - `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 `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 +- `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), `OrganizationServices::UpsertAddress` and `PersonServices::CaptureFromSubmission` (where submitted answers become addresses — a country is translated only when one was submitted), `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); `#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 +- `AddressServices::TranslateExisting` — Rewrites stored `Address` countries/states through `AddressTranslation`, fills a locality only where it's Unknown, clears a country nothing translates, and logs each changed address as one `autochange.address` Ahoy event (all in one visit, `reason: "address_translation"`). Form answers are never rewritten — they stay as typed and are translated where they become addresses (`OrganizationServices::UpsertAddress`, `PersonServices::CaptureFromSubmission`). Idempotent; `dry_run:` reports without writing; `#rows` lists every change (address, person/organization ids, field, before, after). 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 @@ -608,7 +608,7 @@ Located in `lib/tasks/` (19 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 — including every change as a CSV row (record, person/organization/submission ids, field, before, after; `VERBOSE=true` also prints them) — 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) +- `translate_addresses.rake` — Rewrite stored address countries/states as the dropdowns store them (form answers stay as typed) (`data:translate_addresses`, via `AddressServices::TranslateExisting`, idempotent, DRY_RUN, lists values nothing translates; every run, dry or not, saves its report — including every change as a CSV row (address, person/organization ids, field, before, after; `VERBOSE=true` also prints them) — 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/app/models/form_answer.rb b/app/models/form_answer.rb index 3d24ad0e18..ea8fd0950f 100644 --- a/app/models/form_answer.rb +++ b/app/models/form_answer.rb @@ -1,8 +1,6 @@ # `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. 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. +# survive intact). It is NOT HTML-sanitized on write. # # 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 index bf89e7e673..17bbae254c 100644 --- a/app/services/address_services/translate_existing.rb +++ b/app/services/address_services/translate_existing.rb @@ -1,22 +1,17 @@ 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. + # Rewrites stored address countries and states as the dropdowns store them (ADR-0009), + # filling a locality only where it's Unknown. Form answers stay as people typed them; they're + # translated where they become addresses instead. Each changed address 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, :rows) - # One field one record changed (or would change, on a dry run), and whose record it is. - Row = Data.define(:record_type, :record_id, :person_id, :organization_id, :form_submission_id, :field, :before, :after) + Result = Data.define(:addresses, :changes, :unmatched, :rows) + # One field one address changed (or would change, on a dry run), and whose address it is. + Row = Data.define(:address_id, :person_id, :organization_id, :field, :before, :after) ROW_HEADERS = Row.members.freeze 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. + # `log` receives one line per changed address, for the rake task's preview. def initialize(dry_run: false, log: nil, user: nil) @dry_run = dry_run @log = log @@ -28,19 +23,18 @@ def call @tracker = Ahoy::Tracker.new(user: @user) @changes = Hash.new(0) @rows = [] - addresses = answers = 0 + addresses = 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) } + Address.find_each { |address| addresses += 1 if translate(address) } end - Result.new(addresses:, answers:, changes: @changes, unmatched: @translation.unmatched, rows: @rows) + Result.new(addresses:, changes: @changes, unmatched: @translation.unmatched, rows: @rows) end private - def translate_address(address) + def translate(address) state = @translation.state(address.state, country: address.country) country = @translation.country(address.country, state: address.state) updates = {} @@ -50,56 +44,28 @@ def translate_address(address) locality = @translation.locality(city: address.city, state:, country:, county: address.county) updates[:locality] = locality if locality != address.locality end - owner = { person_id: (address.addressable_id if address.addressable_type == "Person"), - organization_id: (address.addressable_id if address.addressable_type == "Organization") } - apply(address, updates, owner:, title: address.name, addressable_type: address.addressable_type, addressable_id: address.addressable_id) + apply(address, updates) 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).to_s - else - @translation.state(raw, country: paired).to_s - end - return false if translated == raw.to_s || (translated.blank? && identifier.end_with?("_state")) - - owner = { person_id: answer.form_submission&.person_id, form_submission_id: answer.form_submission_id } - apply(answer, { submitted_answer: translated }, owner:, title: answer.form_field.name, - form_submission_id: answer.form_submission_id, field_identifier: identifier) - end - - def apply(record, updates, owner:, title:, **extra) + def apply(address, updates) return false if updates.empty? - label = record.class.model_name.human + person_id = address.addressable_id if address.addressable_type == "Person" + organization_id = address.addressable_id if address.addressable_type == "Organization" updates.each do |field, after| - @changes["#{label} #{field.to_s.humanize(capitalize: false)}: #{record[field].inspect} → #{after.inspect}"] += 1 - @rows << Row.new(record_type: label, record_id: record.id, person_id: nil, organization_id: nil, - form_submission_id: nil, **owner, field:, before: record[field], after:) + @changes["Address #{field}: #{address[field].inspect} → #{after.inspect}"] += 1 + @rows << Row.new(address_id: address.id, person_id:, organization_id:, field:, before: address[field], after:) end - @log&.call(" #{label} ##{record.id}: #{updates.map { |field, after| "#{field} #{record[field].inspect} → #{after.inspect}" }.join(', ')}") + @log&.call(" Address ##{address.id}: #{updates.map { |field, after| "#{field} #{address[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 + changes = updates.to_h { |field, after| [ field, { before: address[field], after: } ] } + address.update_columns(updates) + @tracker.track("autochange.address", { + resource_type: "Address", resource_id: address.id, resource_title: address.name, changes:, reason: REASON, + addressable_type: address.addressable_type, addressable_id: address.addressable_id }) true end - - def address_answers - FormAnswer.joins(:form_field).where(form_fields: { field_identifier: ANSWER_PAIRS.keys }).includes(:form_field, :form_submission) - 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/app/services/organization_services/upsert_address.rb b/app/services/organization_services/upsert_address.rb index ec30295b59..c4d9cd8456 100644 --- a/app/services/organization_services/upsert_address.rb +++ b/app/services/organization_services/upsert_address.rb @@ -39,10 +39,11 @@ def self.call(organization:, city: nil, state: nil, street_address: nil, zip_cod def initialize(organization:, city: nil, state: nil, street_address: nil, zip_code: nil, country: nil, overwrite: true) @organization = organization @city = city&.strip - @state = state&.strip.presence + translation = AddressTranslation.new + @state = translation.state(state, country:) @street_address = street_address&.strip @zip_code = zip_code&.strip - @country = country&.strip.presence + @country = translation.country(country, state:) if country.present? @overwrite = overwrite end diff --git a/app/services/person_services/capture_from_submission.rb b/app/services/person_services/capture_from_submission.rb index afb787a0c5..e5448f4bf0 100644 --- a/app/services/person_services/capture_from_submission.rb +++ b/app/services/person_services/capture_from_submission.rb @@ -91,8 +91,9 @@ def capture_mailing_address new_city = field_value("mailing_city")&.strip return if new_city.blank? - new_state = field_value("mailing_state")&.strip - new_country = field_value("mailing_country")&.strip + translation = AddressTranslation.new + new_state = translation.state(field_value("mailing_state"), country: field_value("mailing_country")) + new_country = translation.country(field_value("mailing_country"), state: field_value("mailing_state")) if field_value("mailing_country").present? existing = @person.addresses.order(:id).find do |address| AddressNormalizer.city(address.city) == AddressNormalizer.city(new_city) && AddressNormalizer.state(address.state) == AddressNormalizer.state(new_state) diff --git a/db/seeds/features/2026-10-25-form-answers-stay-as-typed.yml b/db/seeds/features/2026-10-25-form-answers-stay-as-typed.yml new file mode 100644 index 0000000000..a119088b44 --- /dev/null +++ b/db/seeds/features/2026-10-25-form-answers-stay-as-typed.yml @@ -0,0 +1,9 @@ +name: "Form answers keep the country people typed" +area: registration +display_status: admin_facing +released_on: 2026-10-25 +pr_number: 2841 +summary: >- + The address cleanup now fixes addresses only — a person's form answers stay exactly as they + submitted them. When an answer is turned into an address (linking an organization, or saving + a registrant's mailing address), its country and state are stored the way the dropdowns do. diff --git a/docs/adr/0009-legacy-address-value-translation.md b/docs/adr/0009-legacy-address-value-translation.md index 338ec3df86..83f5baa7ae 100644 --- a/docs/adr/0009-legacy-address-value-translation.md +++ b/docs/adr/0009-legacy-address-value-translation.md @@ -13,7 +13,7 @@ USPS code. Everything written before, and everything other systems send, is free |---|---| | 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 | +| Portal addresses and form answers from before the dropdowns | `addresses.country`/`state`; `mailing_*`/`organization_*` answers (read when an old submission is linked or captured) | 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 — @@ -51,17 +51,23 @@ since a state is required and regions outside the US are legitimate. (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 - record is an `autochange.*` Ahoy event with its before and after. + (`data:translate_addresses`) rewrites stored address countries and states, filling a + locality only where it's Unknown. It's idempotent, so it runs before and after an import. + Every changed address is an `autochange.address` Ahoy event with its before and after. +- **Where answers become addresses:** `OrganizationServices::UpsertAddress` (organization + capture and link-organization) and `PersonServices::CaptureFromSubmission` (the mailing + address) translate the country and state they're handed, so re-reading an old answer can't + add a spelling the dropdowns don't offer. A submission with no country leaves the stored + one alone. - **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 (a country answer nothing -answers to becomes blank), and each rewrite is logged. +Form answers stay verbatim, as a record of what the person submitted — the cleanup never +rewrites them. They're translated where they become addresses instead (above), and the +dashboards and maps that count countries read addresses. The form-results page for an older +form still shows each answer as typed. ## Consequences diff --git a/lib/tasks/translate_addresses.rake b/lib/tasks/translate_addresses.rake index 8c880d45f8..a19381a356 100644 --- a/lib/tasks/translate_addresses.rake +++ b/lib/tasks/translate_addresses.rake @@ -1,7 +1,7 @@ # frozen_string_literal: true -# Rewrite stored address countries/states and country/state form answers as the dropdowns -# store them. See AddressServices::TranslateExisting. +# Rewrite stored address countries/states as the dropdowns store them (form answers stay as +# typed). See AddressServices::TranslateExisting. # # 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). @@ -11,7 +11,7 @@ # VERBOSE=true bin/rails data:translate_addresses # also print every change as CSV rows # 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." + desc "Translate stored address countries/states to dropdown values (form answers stay as typed). Idempotent. DRY_RUN=false to execute." task translate_addresses: :environment do dry_run = ENV["DRY_RUN"] != "false" verbose = ENV["VERBOSE"] == "true" @@ -41,7 +41,6 @@ namespace :data do note.(CSV.generate_line(AddressServices::TranslateExisting::ROW_HEADERS, row_sep: "")) result.rows.each { |row| note.(CSV.generate_line(row.to_h.values, row_sep: "")) } 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}") diff --git a/spec/requests/event_registrations_spec.rb b/spec/requests/event_registrations_spec.rb index 80cdb1511a..c2858a0098 100644 --- a/spec/requests/event_registrations_spec.rb +++ b/spec/requests/event_registrations_spec.rb @@ -1863,7 +1863,7 @@ def details_open?(body, heading) address = organization.addresses.find_by(city: "Austin") expect(address).to be_present - expect(address.country).to eq("USA") + expect(address.country).to eq("United States") expect(regular_user.person.affiliations.where(organization: organization).map(&:organization_address)) .to all(eq(address)) end diff --git a/spec/services/address_services/translate_existing_spec.rb b/spec/services/address_services/translate_existing_spec.rb index 12ec99ec5e..9ccb8a5f04 100644 --- a/spec/services/address_services/translate_existing_spec.rb +++ b/spec/services/address_services/translate_existing_spec.rb @@ -61,85 +61,50 @@ def answer(submission, identifier, value) .to include("country" => { "before" => "Narnia", "after" => nil }) end - it "blanks a country answer it can't place" do + it "leaves form answers exactly as people typed them" do submission = create(:form_submission) - answer(submission, "mailing_state", "-") - country = answer(submission, "mailing_country", "West Africa") + typed = { "mailing_state" => "California", "mailing_country" => "USA", + "organization_country" => "Untied States", "organization_state" => "Texas" } + answers = typed.to_h { |identifier, value| [ identifier, answer(submission, identifier, value) ] } - described_class.new.call + expect { described_class.new.call }.not_to change(Ahoy::Event.where(name: "autochange.form_answer"), :count) - expect(country.reload.submitted_answer).to eq("") + expect(answers.transform_values { _1.reload.submitted_answer }).to eq(typed) end - it "leaves every stored country and country answer on the dropdown" do + it "leaves every stored address country 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 - 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 + it "logs each changed address 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") + texas = address_with(country: nil, state: "Texas", locality: Address::OUTSIDE_CA) 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) + expect(Ahoy::Event.where(name: "autochange.address", resource_id: [ usa.id, texas.id ]).distinct.count(:visit_id)).to eq(1) end - it "lists every change with its owner and before/after, on a dry run too" do + it "lists every address change with its owner and before/after, on a dry run too" do usa = address_with(country: "USA", state: "Texas", locality: Address::OUTSIDE_CA) org_address = create(:address, addressable: create(:organization)).tap { |address| address.update_columns(country: "Usa", state: "TX", locality: Address::OUTSIDE_CA) } - submission = create(:form_submission, person: create(:person)) - answer(submission, "mailing_state", "TX") - country = answer(submission, "mailing_country", "US") rows = described_class.new(dry_run: true).call.rows - expect(rows).to include( - have_attributes(record_type: "Address", record_id: usa.id, person_id: usa.addressable_id, organization_id: nil, - form_submission_id: nil, field: :country, before: "USA", after: "United States"), - have_attributes(record_type: "Address", record_id: usa.id, field: :state, before: "Texas", after: "TX"), - have_attributes(record_type: "Address", record_id: org_address.id, person_id: nil, organization_id: org_address.addressable_id, - field: :country, before: "Usa", after: "United States"), - have_attributes(record_type: "Form answer", record_id: country.id, person_id: submission.person_id, - form_submission_id: submission.id, field: :submitted_answer, before: "US", after: "United States") + expect(rows).to contain_exactly( + have_attributes(address_id: usa.id, person_id: usa.addressable_id, organization_id: nil, + field: :country, before: "USA", after: "United States"), + have_attributes(address_id: usa.id, field: :state, before: "Texas", after: "TX"), + have_attributes(address_id: org_address.id, person_id: nil, organization_id: org_address.addressable_id, + field: :country, before: "Usa", after: "United States") ) end diff --git a/spec/services/event_registration_services/public_registration_spec.rb b/spec/services/event_registration_services/public_registration_spec.rb index 36342a6502..49c8c79dc6 100644 --- a/spec/services/event_registration_services/public_registration_spec.rb +++ b/spec/services/event_registration_services/public_registration_spec.rb @@ -115,7 +115,7 @@ def register_with(position:) address = organization.addresses.find_by(city: "Austin") expect(address).to be_present - expect(address.country).to eq("USA") + expect(address.country).to eq("United States") linked = person.affiliations.where(organization: organization) expect(linked.pluck(:title)).to contain_exactly("Counselor", "Facilitator") expect(linked.map(&:organization_address)).to all(eq(address)) @@ -396,7 +396,7 @@ def register(answers) described_class.call(event: event, registration_form: form, form_params: params) person = Person.find_by!(email: "ada@example.com") - expect(person.addresses.find_by(primary: true).country).to eq("USA") + expect(person.addresses.find_by(primary: true).country).to eq("United States") end it "updates a returning registrant's address when the city and state are spelled differently" do @@ -439,7 +439,7 @@ def register_with_org(extra) expect(organization.organization_type).to eq("501c3/nonprofit") expect(organization.website_url).to include("helpinghands.org") - expect(organization.addresses.find_by(primary: true).country).to eq("USA") + expect(organization.addresses.find_by(primary: true).country).to eq("United States") end # find_organization only ever finds, so a registrant always changes an org diff --git a/spec/services/organization_services/upsert_address_spec.rb b/spec/services/organization_services/upsert_address_spec.rb index d2f4205250..65a1b505d5 100644 --- a/spec/services/organization_services/upsert_address_spec.rb +++ b/spec/services/organization_services/upsert_address_spec.rb @@ -20,12 +20,27 @@ city: "Austin", state: "TX", zip_code: "78701", - country: "USA", + country: "United States", address_type: "work", primary: true ) end + it "stores the submitted state and country as the dropdowns do, so an old answer can't add a stray spelling" do + result = described_class.call(organization:, street_address: "1 Main St", city: "Austin", state: "Texas", zip_code: "78701", country: "USA") + + expect(result.address).to have_attributes(state: "TX", country: "United States") + end + + it "leaves the country alone when the submission gives none" do + existing = create(:address, addressable: organization, street_address: "1 King St", city: "Toronto", state: "ON", zip_code: "M5H", country: "Canada") + + result = described_class.call(organization:, street_address: "1 King St", city: "Toronto", state: "ON", zip_code: "M5H") + + expect(existing.reload.country).to eq("Canada") + expect(result.changes).to be_empty + end + # Browsers don't trim text inputs, and a padded value stored verbatim would read # as a difference to ProfileDiff and get re-reported as filled on every relink. it "strips the submitted street and ZIP before storing them" do @@ -213,7 +228,7 @@ expect(result).to have_attributes(address: existing, created: false) expect(organization.addresses.count).to eq(1) # State already on file is kept (never flipped); the blank country is filled. - expect(existing.reload).to have_attributes(state: "Texas", country: "USA", street_address: "1 Main St") + expect(existing.reload).to have_attributes(state: "Texas", country: "United States", street_address: "1 Main St") end it "updates the same street/ZIP address instead of duplicating when the city is respelled, keeping the saved city" do @@ -231,7 +246,7 @@ expect(result).to have_attributes(address: existing, created: false) expect(organization.addresses.count).to eq(1) - expect(existing.reload).to have_attributes(city: "Saint Louis", country: "USA") + expect(existing.reload).to have_attributes(city: "Saint Louis", country: "United States") end it "fills a blank city on the matched address rather than leaving it empty" do diff --git a/spec/services/person_services/capture_from_submission_spec.rb b/spec/services/person_services/capture_from_submission_spec.rb index 7b7cb0e2d3..7e51dbd657 100644 --- a/spec/services/person_services/capture_from_submission_spec.rb +++ b/spec/services/person_services/capture_from_submission_spec.rb @@ -10,6 +10,20 @@ expect(person.addresses.sole).to have_attributes(city: "Springfield", street_address: "", zip_code: "") end + it "stores the answered state and country as the dropdowns do" do + described_class.call(person:, answers: { "mailing_city" => "Fresno", "mailing_state" => "California", "mailing_country" => "USA" }) + + expect(person.addresses.sole).to have_attributes(state: "CA", country: "United States") + end + + it "translates a country it fills on an address already on file" do + address = create(:address, addressable: person, city: "Toronto", state: "ON", country: nil) + + described_class.call(person:, answers: { "mailing_city" => "Toronto", "mailing_state" => "ON", "mailing_country" => "can" }, overwrite: false) + + expect(address.reload.country).to eq("Canada") + end + it "keeps a saved ZIP and street when an overwriting submission skips them" do address = create(:address, addressable: person, city: "Springfield", state: "IL", street_address: "1 Old Rd", zip_code: "62701") diff --git a/spec/tasks/translate_addresses_spec.rb b/spec/tasks/translate_addresses_spec.rb index b6aa4f8bcc..efe4b83566 100644 --- a/spec/tasks/translate_addresses_spec.rb +++ b/spec/tasks/translate_addresses_spec.rb @@ -42,11 +42,11 @@ def report_text = saved_report.download.force_encoding(Encoding::UTF_8) it "saves every change as CSV rows, printing them only with VERBOSE=true" do usa = address_with(country: "USA", state: "TX") - row = "Address,#{usa.id},#{usa.addressable_id},,,country,USA,United States" + row = "#{usa.id},#{usa.addressable_id},,country,USA,United States" output = run_task - expect(report_text).to include("record_type,record_id,person_id,organization_id,form_submission_id,field,before,after", row) + expect(report_text).to include("address_id,person_id,organization_id,field,before,after", row) expect(output).not_to include(row) Rake::Task["data:translate_addresses"].reenable From 8b57325b918af8fe837479149c2cccb67c4c4f34 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 06:31:54 -0400 Subject: [PATCH 03/10] Record the one-time hand translation of country answers (2026-10-25) Country answers that translate to a dropdown country were rewritten once by hand and logged, so the verbatim rule names its one exception and where the trail lives. Co-Authored-By: Claude Opus 5.5 (1M context) --- app/models/form_answer.rb | 4 +++- .../0009-legacy-address-value-translation.md | 19 +++++++++++++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/app/models/form_answer.rb b/app/models/form_answer.rb index ea8fd0950f..1f9d5bc40c 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. One exception, made once by hand +# on 2026-10-25 (ADR-0009): country answers that translate to a dropdown country were +# rewritten to it, each logged as an "autochange.form_answer" Ahoy event with its before. # # 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/docs/adr/0009-legacy-address-value-translation.md b/docs/adr/0009-legacy-address-value-translation.md index 83f5baa7ae..5d9c1f399b 100644 --- a/docs/adr/0009-legacy-address-value-translation.md +++ b/docs/adr/0009-legacy-address-value-translation.md @@ -64,10 +64,21 @@ since a state is required and regions outside the US are legitimate. ### Form answers -Form answers stay verbatim, as a record of what the person submitted — the cleanup never -rewrites them. They're translated where they become addresses instead (above), and the -dashboards and maps that count countries read addresses. The form-results page for an older -form still shows each answer as typed. +Form answers are otherwise stored verbatim, as a record of what the person submitted, and +`data:translate_addresses` never rewrites them. They're translated where they become +addresses instead (above). + +**One exception, made once by hand on 2026-10-25:** in a console pass alongside the +address cleanup, every `mailing_country` / `organization_country` answer that translates to a +dropdown country was rewritten to it ("USA", "LA County " → United States). Blank and +untranslatable answers, and every state answer, were left as typed. Each rewrite is an +`autochange.form_answer` Ahoy event (reason `answer_translation`) carrying the answer's +before value, credited to the Settings import user, with `updated_at`/`updated_by` stamped. + +In the same pass, fills on addresses — a blank country read from its state, `""` → nil, a +locality filled from Unknown — were stamped but logged as one `autochange.address_fills` event +pointing at the saved report (every change as CSV), while each correction of a typed value got +its own event. ## Consequences From 4fb4d1c215269acc44439e6c152eab0a5fdc35e1 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 07:47:00 -0400 Subject: [PATCH 04/10] Stop flagging an org address against the answer it was built from UpsertAddress stores "Texas"/"USA" as TX/United States, but the answers stay as typed, so link-organization's ProfileDiff reported the address it had just saved as a difference. Comparing the answer through AddressTranslation lets them match. Also clarifies in ADR-0009 how the 2026-10-25 hand pass logged address fills differently from a data:translate_addresses run. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 4 ++-- .../organization_services/profile_diff.rb | 21 +++++++++++++++++-- .../capture_from_submission.rb | 6 ++++-- .../2026-10-25-form-answers-stay-as-typed.yml | 2 ++ .../0009-legacy-address-value-translation.md | 10 +++++---- .../profile_diff_spec.rb | 19 +++++++++++++++++ 6 files changed, 52 insertions(+), 10 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index facc8f833b..a036617b6b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -269,7 +269,7 @@ comparison — add a rule if none fits (e.g. `PersonPolicy#own_profile?`). Conve - `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 `FmAddressAttributes` (every FileMaker import), `OrganizationServices::UpsertAddress` and `PersonServices::CaptureFromSubmission` (where submitted answers become addresses — a country is translated only when one was submitted), `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 +- `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), `OrganizationServices::UpsertAddress` and `PersonServices::CaptureFromSubmission` (where submitted answers become addresses — a country is translated only when one was submitted), `OrganizationServices::ProfileDiff` (so the answer an address was built from isn't flagged against it), `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); `#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 through `AddressTranslation`, fills a locality only where it's Unknown, clears a country nothing translates, and logs each changed address as one `autochange.address` Ahoy event (all in one visit, `reason: "address_translation"`). Form answers are never rewritten — they stay as typed and are translated where they become addresses (`OrganizationServices::UpsertAddress`, `PersonServices::CaptureFromSubmission`). Idempotent; `dry_run:` reports without writing; `#rows` lists every change (address, person/organization ids, field, before, after). 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 @@ -352,7 +352,7 @@ comparison — add a rule if none fits (e.g. `PersonPolicy#own_profile?`). Conve - `OrganizationServices::UpsertAddress` — Find-or-create an organization's "work" address from a registrant's submitted organization fields (street/city/state/zip/country), matching via `AddressMatcher`; updates the match, else adds a new address; never demotes the org's existing primary (a registrant's address becomes primary only when the org has none yet). `overwrite: true` (default, public flow) replaces the matching address's details; `overwrite: false` (admin org-linking) fills only its blank fields so a conflicting saved address is kept and flagged instead. City and state are always fill-only (never flipped, in either mode), so a cross-city ZIP match keeps the saved city and lets `ProfileDiff` flag the difference; that fill still earns its keep because a legacy row can hold `""` in either column, and the street + ZIP match is the one path that reaches such a row to repair it. Returns a `Result` (`address`, `created`, and `changes` — an `AutofillChange` per field actually written, carrying the value) so callers can report what was really saved. Every field is trimmed before it's stored, so a padded answer isn't later read back as a difference from itself. Tolerates optional answers: a skipped street/ZIP is stored blank (both are `NOT NULL` columns), and `address` is nil — nothing saved — when there's no city, or no state and no existing address to update, rather than failing the whole registration. Shared by `PublicRegistration` and the admin org-linking actions - `OrganizationServices::SyncProfile` — Populate an organization's structured profile columns (`website_url`, `organization_type`/`organization_type_other`) from a registrant's submitted answers, folding an "Other: " type answer into the label + free text (and recording the free text as an `OtherResponse` for the curation queue). `overwrite: true` (default, public flow) replaces non-blank values latest-wins; `overwrite: false` (admin link/create-and-link) only fills columns that are currently blank. Returns a `Result` whose `changes` name only the columns whose value actually changed (read off `saved_changes`) and the value each ended up with, so re-submitting what's already on file isn't reported as a change. Shared by `PublicRegistration` and `EventRegistrationsController#{select,create}_organization` - `OrganizationServices::LinkSubmittedOrganization` — The shared core of both org-linking editors (event registration + form submission): given a submitted-answers "entry" (org_name/position/website/organization_type/address, nil for an org no submission describes), fill-blank-syncs the org profile (`SyncProfile`) and work address (`UpsertAddress`), creates the affiliations (`AffiliationServices::CreateFromRegistration`, anchored to the upserted or sole address), and returns a `Result` (`saved`, `conflicts` via `ProfileDiff`, `address`) whose `#notice`/`#warning` build the escaped flash strings. Callers keep their flow-specific wrappers (registration link row/pin/autofill notes; submission metadata link) -- `OrganizationServices::ProfileDiff` — Read-only comparison of a registrant's submitted type/website/address answers against an org's saved profile, returning `Discrepancy` structs where the submitted value differs from a value already on the org (so the fill-blanks sync leaves it unapplied). URL comparison ignores scheme/`www.`/trailing slash; type comparison parses "Other: "; address is compared against the org's corresponding address (via `AddressMatcher`) on street/city/state/ZIP/country. Powers the org-linking flash warning and the linking page's persistent per-org discrepancy note +- `OrganizationServices::ProfileDiff` — Read-only comparison of a registrant's submitted type/website/address answers against an org's saved profile, returning `Discrepancy` structs where the submitted value differs from a value already on the org (so the fill-blanks sync leaves it unapplied). URL comparison ignores scheme/`www.`/trailing slash; type comparison parses "Other: "; address is compared against the org's corresponding address (via `AddressMatcher`) on street/city/state/ZIP/country, with the submitted state/country also read through `AddressTranslation` so the dropdown value `UpsertAddress` built from an answer ("Texas" → `TX`, "USA" → United States) isn't reported as differing from it. Powers the org-linking flash warning and the linking page's persistent per-org discrepancy note Which submission's answers apply to which org is decided by `EventRegistrationsController#submission_entry_for`: the submission pinned on the link, else the one whose typed org name matches the org, else — only when there's exactly one submission and one linked org — that sole entry (which covers a registrant who named no org, and an admin resolving a typo'd "Acme Inc" to the saved "Acme Corporation"). An extra org an admin links by hand matches nothing, so another org's answers — profile, address, and job title alike — are never written onto it or reported against it; its affiliation is created untitled (just "Facilitator"). diff --git a/app/services/organization_services/profile_diff.rb b/app/services/organization_services/profile_diff.rb index 7a07bee2c1..88f2a89a88 100644 --- a/app/services/organization_services/profile_diff.rb +++ b/app/services/organization_services/profile_diff.rb @@ -71,12 +71,29 @@ def address_discrepancies [ field_discrepancy(:address_street, "Address – street", @address[:street_address], existing.street_address), field_discrepancy(:address_city, "Address – city", @address[:city], existing.city), - field_discrepancy(:address_state, "Address – state", @address[:state], existing.state), + state_discrepancy(existing), field_discrepancy(:address_zip, "Address – ZIP", @address[:zip_code], existing.zip_code), - field_discrepancy(:address_country, "Address – country", @address[:country], existing.country) + country_discrepancy(existing) ].compact end + # Compared as UpsertAddress stores it (ADR-0009), so "Texas"/"USA" matches the "TX"/"United States" it became. + def state_discrepancy(existing) + return if translation.state(@address[:state], country: @address[:country]).to_s.casecmp?(existing.state.to_s) + + field_discrepancy(:address_state, "Address – state", @address[:state], existing.state) + end + + def country_discrepancy(existing) + return if translation.country(@address[:country], state: @address[:state]).to_s.casecmp?(existing.country.to_s) + + field_discrepancy(:address_country, "Address – country", @address[:country], existing.country) + end + + def translation + @translation ||= AddressTranslation.new + end + def matching_address AddressMatcher.call( @organization, diff --git a/app/services/person_services/capture_from_submission.rb b/app/services/person_services/capture_from_submission.rb index e5448f4bf0..3a0bb77ebe 100644 --- a/app/services/person_services/capture_from_submission.rb +++ b/app/services/person_services/capture_from_submission.rb @@ -91,9 +91,11 @@ def capture_mailing_address new_city = field_value("mailing_city")&.strip return if new_city.blank? + state_answer = field_value("mailing_state") + country_answer = field_value("mailing_country") translation = AddressTranslation.new - new_state = translation.state(field_value("mailing_state"), country: field_value("mailing_country")) - new_country = translation.country(field_value("mailing_country"), state: field_value("mailing_state")) if field_value("mailing_country").present? + new_state = translation.state(state_answer, country: country_answer) + new_country = translation.country(country_answer, state: state_answer) if country_answer.present? existing = @person.addresses.order(:id).find do |address| AddressNormalizer.city(address.city) == AddressNormalizer.city(new_city) && AddressNormalizer.state(address.state) == AddressNormalizer.state(new_state) diff --git a/db/seeds/features/2026-10-25-form-answers-stay-as-typed.yml b/db/seeds/features/2026-10-25-form-answers-stay-as-typed.yml index a119088b44..f5443197e3 100644 --- a/db/seeds/features/2026-10-25-form-answers-stay-as-typed.yml +++ b/db/seeds/features/2026-10-25-form-answers-stay-as-typed.yml @@ -7,3 +7,5 @@ summary: >- The address cleanup now fixes addresses only — a person's form answers stay exactly as they submitted them. When an answer is turned into an address (linking an organization, or saving a registrant's mailing address), its country and state are stored the way the dropdowns do. +pro_tips: + - "The link-organization page treats an answer like USA or Texas as matching the United States or TX it was saved as, so it is not flagged as a difference." diff --git a/docs/adr/0009-legacy-address-value-translation.md b/docs/adr/0009-legacy-address-value-translation.md index 5d9c1f399b..c710ecef27 100644 --- a/docs/adr/0009-legacy-address-value-translation.md +++ b/docs/adr/0009-legacy-address-value-translation.md @@ -75,10 +75,12 @@ untranslatable answers, and every state answer, were left as typed. Each rewrite `autochange.form_answer` Ahoy event (reason `answer_translation`) carrying the answer's before value, credited to the Settings import user, with `updated_at`/`updated_by` stamped. -In the same pass, fills on addresses — a blank country read from its state, `""` → nil, a -locality filled from Unknown — were stamped but logged as one `autochange.address_fills` event -pointing at the saved report (every change as CSV), while each correction of a typed value got -its own event. +**Addresses in that same hand pass** carry a different trail from a `data:translate_addresses` +run: its fills — a blank country read from its state, `""` → nil, a locality filled from +Unknown — were stamped but collapsed into one `autochange.address_fills` event pointing at the +saved report (every change as CSV), while each correction of a typed value got its own event. +The rake task emits one `autochange.address` per changed address (reason +`address_translation`), fills included. ## Consequences diff --git a/spec/services/organization_services/profile_diff_spec.rb b/spec/services/organization_services/profile_diff_spec.rb index 52df0b36f6..f74c76aae5 100644 --- a/spec/services/organization_services/profile_diff_spec.rb +++ b/spec/services/organization_services/profile_diff_spec.rb @@ -105,6 +105,17 @@ expect(diff).to be_empty end + # UpsertAddress stores the dropdown spelling while the answer keeps the typed one. + it "does not flag a state or country the submission itself stored as the dropdowns do" do + create(:address, addressable: organization, street_address: "1 Main St", city: "Austin", state: "TX", zip_code: "78701", country: "United States") + + diff = described_class.call(organization: organization, address: { + street_address: "1 Main St", city: "Austin", state: "Texas", zip_code: "78701", country: "USA" + }) + + expect(diff).to be_empty + end + it "flags a city that differs on the address matched across cities by street and ZIP" do create(:address, addressable: organization, street_address: "1 Main St", city: "Saint Louis", state: "MO", zip_code: "63101") @@ -135,5 +146,13 @@ expect(diff).to be_empty end + + # The linking flow diffs right after the upsert, against the address those answers just built. + it "reports nothing on the address UpsertAddress just built from the same answers" do + submitted = { street_address: "1 Main St", city: "Austin", state: "Texas", zip_code: "78701", country: "USA" } + OrganizationServices::UpsertAddress.call(organization: organization, **submitted) + + expect(described_class.call(organization: organization, address: submitted)).to be_empty + end end end From 9b86b8a7866a747a101e408287bb243e35ce4980 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 08:59:03 -0400 Subject: [PATCH 05/10] Correct ADR-0009: the country-answer hand pass never ran The prod cleanup was done by hand on addresses in the portal, superseding the console script that would have rewritten country answers, so the docs no longer claim it did. Co-Authored-By: Claude Opus 5.5 (1M context) --- app/models/form_answer.rb | 4 +--- .../adr/0009-legacy-address-value-translation.md | 16 +++------------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/app/models/form_answer.rb b/app/models/form_answer.rb index 1f9d5bc40c..ea8fd0950f 100644 --- a/app/models/form_answer.rb +++ b/app/models/form_answer.rb @@ -1,8 +1,6 @@ # `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. One exception, made once by hand -# on 2026-10-25 (ADR-0009): country answers that translate to a dropdown country were -# rewritten to it, each logged as an "autochange.form_answer" Ahoy event with its before. +# survive intact). It is NOT HTML-sanitized on write. # # 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/docs/adr/0009-legacy-address-value-translation.md b/docs/adr/0009-legacy-address-value-translation.md index c710ecef27..fb0a1e8f4b 100644 --- a/docs/adr/0009-legacy-address-value-translation.md +++ b/docs/adr/0009-legacy-address-value-translation.md @@ -68,19 +68,9 @@ Form answers are otherwise stored verbatim, as a record of what the person submi `data:translate_addresses` never rewrites them. They're translated where they become addresses instead (above). -**One exception, made once by hand on 2026-10-25:** in a console pass alongside the -address cleanup, every `mailing_country` / `organization_country` answer that translates to a -dropdown country was rewritten to it ("USA", "LA County " → United States). Blank and -untranslatable answers, and every state answer, were left as typed. Each rewrite is an -`autochange.form_answer` Ahoy event (reason `answer_translation`) carrying the answer's -before value, credited to the Settings import user, with `updated_at`/`updated_by` stamped. - -**Addresses in that same hand pass** carry a different trail from a `data:translate_addresses` -run: its fills — a blank country read from its state, `""` → nil, a locality filled from -Unknown — were stamped but collapsed into one `autochange.address_fills` event pointing at the -saved report (every change as CSV), while each correction of a typed value got its own event. -The rake task emits one `autochange.address` per changed address (reason -`address_translation`), fills included. +The prod cleanup on 2026-10-25 was done by hand: an admin corrected address cities, states, +and countries in the portal, and answers were left as typed, since the addresses are the +record. ## Consequences From 563f5205b299507fa169eeb32fec2fda1327e03a Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 09:18:40 -0400 Subject: [PATCH 06/10] ADR-0009: note which kinds of prod data were cleaned on 2026-10-24/25 Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/adr/0009-legacy-address-value-translation.md | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/docs/adr/0009-legacy-address-value-translation.md b/docs/adr/0009-legacy-address-value-translation.md index fb0a1e8f4b..1ace87690f 100644 --- a/docs/adr/0009-legacy-address-value-translation.md +++ b/docs/adr/0009-legacy-address-value-translation.md @@ -68,9 +68,17 @@ Form answers are otherwise stored verbatim, as a record of what the person submi `data:translate_addresses` never rewrites them. They're translated where they become addresses instead (above). -The prod cleanup on 2026-10-25 was done by hand: an admin corrected address cities, states, -and countries in the portal, and answers were left as typed, since the addresses are the -record. +### Prod data cleanup, 2026-10-24 to 2026-10-25 + +Prod data was cleaned by hand over these two days, with addresses as the record: + +- **Addresses:** cities, states, and countries corrected in the portal. +- **Organization address answers on submissions not yet linked to an organization** — the ones + that still become an address when linked: country spellings, placeholder values ("N/A", + "-"), and values typed into the wrong field. Each change is an `autochange.form_answer` Ahoy + event with its before value. + +All other answers stay as typed. ## Consequences From e9185f6740019ff3697361ad67b49d95c79e62b9 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 09:20:09 -0400 Subject: [PATCH 07/10] ADR-0009: say why some form answers were edited in the cleanup They hadn't become addresses yet, so fixing them was the only way to keep their values off the dropdown-backed address they'll turn into when an organization is linked. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/adr/0009-legacy-address-value-translation.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/adr/0009-legacy-address-value-translation.md b/docs/adr/0009-legacy-address-value-translation.md index 1ace87690f..4516b213d6 100644 --- a/docs/adr/0009-legacy-address-value-translation.md +++ b/docs/adr/0009-legacy-address-value-translation.md @@ -66,17 +66,18 @@ since a state is required and regions outside the US are legitimate. Form answers are otherwise stored verbatim, as a record of what the person submitted, and `data:translate_addresses` never rewrites them. They're translated where they become -addresses instead (above). +addresses instead (above). The one exception is the hand cleanup below. ### Prod data cleanup, 2026-10-24 to 2026-10-25 Prod data was cleaned by hand over these two days, with addresses as the record: - **Addresses:** cities, states, and countries corrected in the portal. -- **Organization address answers on submissions not yet linked to an organization** — the ones - that still become an address when linked: country spellings, placeholder values ("N/A", - "-"), and values typed into the wrong field. Each change is an `autochange.form_answer` Ahoy - event with its before value. +- **Some form answers**, because they hadn't become addresses yet: organization address + answers on submissions not yet linked to an organization, which turn into the + organization's address when someone links one. Fixed: country spellings, placeholder values + ("N/A", "-"), and values typed into the wrong field. Each change is an + `autochange.form_answer` Ahoy event with its before value. All other answers stay as typed. From 7e7ad3959cc296b71a41b7efeb253c08b24e0a52 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 09:37:53 -0400 Subject: [PATCH 08/10] Remove the address translation task now that prod was cleaned by hand The 2026-10-24/25 cleanup corrected addresses in the portal and fixed the few answers that could still become addresses, so the one-time rewrite task and the translation at the answer-to-address write points have nothing left to do. AddressTranslation stays for the FileMaker and registration imports. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 6 +- .../address_services/translate_existing.rb | 71 ---------- .../organization_services/profile_diff.rb | 21 +-- .../organization_services/upsert_address.rb | 5 +- .../capture_from_submission.rb | 7 +- .../2026-10-25-form-answers-stay-as-typed.yml | 11 -- .../0009-legacy-address-value-translation.md | 28 ++-- lib/tasks/translate_addresses.rake | 60 -------- spec/requests/event_registrations_spec.rb | 2 +- .../translate_existing_spec.rb | 128 ------------------ .../public_registration_spec.rb | 6 +- .../profile_diff_spec.rb | 19 --- .../upsert_address_spec.rb | 21 +-- .../capture_from_submission_spec.rb | 14 -- spec/tasks/translate_addresses_spec.rb | 80 ----------- 15 files changed, 25 insertions(+), 454 deletions(-) delete mode 100644 app/services/address_services/translate_existing.rb delete mode 100644 db/seeds/features/2026-10-25-form-answers-stay-as-typed.yml delete mode 100644 lib/tasks/translate_addresses.rake delete mode 100644 spec/services/address_services/translate_existing_spec.rb delete mode 100644 spec/tasks/translate_addresses_spec.rb diff --git a/AGENTS.md b/AGENTS.md index a036617b6b..78576108c3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -269,9 +269,8 @@ comparison — add a rule if none fits (e.g. `PersonPolicy#own_profile?`). Conve - `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 `FmAddressAttributes` (every FileMaker import), `OrganizationServices::UpsertAddress` and `PersonServices::CaptureFromSubmission` (where submitted answers become addresses — a country is translated only when one was submitted), `OrganizationServices::ProfileDiff` (so the answer an address was built from isn't flagged against it), `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 +- `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) and `EventRegistrationImporter` (country/state answers + address/phone types; unmatched values listed on the import preview and counted in its notice). 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); `#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 through `AddressTranslation`, fills a locality only where it's Unknown, clears a country nothing translates, and logs each changed address as one `autochange.address` Ahoy event (all in one visit, `reason: "address_translation"`). Form answers are never rewritten — they stay as typed and are translated where they become addresses (`OrganizationServices::UpsertAddress`, `PersonServices::CaptureFromSubmission`). Idempotent; `dry_run:` reports without writing; `#rows` lists every change (address, person/organization ids, field, before, after). 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 @@ -352,7 +351,7 @@ comparison — add a rule if none fits (e.g. `PersonPolicy#own_profile?`). Conve - `OrganizationServices::UpsertAddress` — Find-or-create an organization's "work" address from a registrant's submitted organization fields (street/city/state/zip/country), matching via `AddressMatcher`; updates the match, else adds a new address; never demotes the org's existing primary (a registrant's address becomes primary only when the org has none yet). `overwrite: true` (default, public flow) replaces the matching address's details; `overwrite: false` (admin org-linking) fills only its blank fields so a conflicting saved address is kept and flagged instead. City and state are always fill-only (never flipped, in either mode), so a cross-city ZIP match keeps the saved city and lets `ProfileDiff` flag the difference; that fill still earns its keep because a legacy row can hold `""` in either column, and the street + ZIP match is the one path that reaches such a row to repair it. Returns a `Result` (`address`, `created`, and `changes` — an `AutofillChange` per field actually written, carrying the value) so callers can report what was really saved. Every field is trimmed before it's stored, so a padded answer isn't later read back as a difference from itself. Tolerates optional answers: a skipped street/ZIP is stored blank (both are `NOT NULL` columns), and `address` is nil — nothing saved — when there's no city, or no state and no existing address to update, rather than failing the whole registration. Shared by `PublicRegistration` and the admin org-linking actions - `OrganizationServices::SyncProfile` — Populate an organization's structured profile columns (`website_url`, `organization_type`/`organization_type_other`) from a registrant's submitted answers, folding an "Other: " type answer into the label + free text (and recording the free text as an `OtherResponse` for the curation queue). `overwrite: true` (default, public flow) replaces non-blank values latest-wins; `overwrite: false` (admin link/create-and-link) only fills columns that are currently blank. Returns a `Result` whose `changes` name only the columns whose value actually changed (read off `saved_changes`) and the value each ended up with, so re-submitting what's already on file isn't reported as a change. Shared by `PublicRegistration` and `EventRegistrationsController#{select,create}_organization` - `OrganizationServices::LinkSubmittedOrganization` — The shared core of both org-linking editors (event registration + form submission): given a submitted-answers "entry" (org_name/position/website/organization_type/address, nil for an org no submission describes), fill-blank-syncs the org profile (`SyncProfile`) and work address (`UpsertAddress`), creates the affiliations (`AffiliationServices::CreateFromRegistration`, anchored to the upserted or sole address), and returns a `Result` (`saved`, `conflicts` via `ProfileDiff`, `address`) whose `#notice`/`#warning` build the escaped flash strings. Callers keep their flow-specific wrappers (registration link row/pin/autofill notes; submission metadata link) -- `OrganizationServices::ProfileDiff` — Read-only comparison of a registrant's submitted type/website/address answers against an org's saved profile, returning `Discrepancy` structs where the submitted value differs from a value already on the org (so the fill-blanks sync leaves it unapplied). URL comparison ignores scheme/`www.`/trailing slash; type comparison parses "Other: "; address is compared against the org's corresponding address (via `AddressMatcher`) on street/city/state/ZIP/country, with the submitted state/country also read through `AddressTranslation` so the dropdown value `UpsertAddress` built from an answer ("Texas" → `TX`, "USA" → United States) isn't reported as differing from it. Powers the org-linking flash warning and the linking page's persistent per-org discrepancy note +- `OrganizationServices::ProfileDiff` — Read-only comparison of a registrant's submitted type/website/address answers against an org's saved profile, returning `Discrepancy` structs where the submitted value differs from a value already on the org (so the fill-blanks sync leaves it unapplied). URL comparison ignores scheme/`www.`/trailing slash; type comparison parses "Other: "; address is compared against the org's corresponding address (via `AddressMatcher`) on street/city/state/ZIP/country. Powers the org-linking flash warning and the linking page's persistent per-org discrepancy note Which submission's answers apply to which org is decided by `EventRegistrationsController#submission_entry_for`: the submission pinned on the link, else the one whose typed org name matches the org, else — only when there's exactly one submission and one linked org — that sole entry (which covers a registrant who named no org, and an admin resolving a typo'd "Acme Inc" to the saved "Acme Corporation"). An extra org an admin links by hand matches nothing, so another org's answers — profile, address, and job title alike — are never written onto it or reported against it; its affiliation is created untitled (just "Facilitator"). @@ -608,7 +607,6 @@ Located in `lib/tasks/` (19 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 as the dropdowns store them (form answers stay as typed) (`data:translate_addresses`, via `AddressServices::TranslateExisting`, idempotent, DRY_RUN, lists values nothing translates; every run, dry or not, saves its report — including every change as a CSV row (address, person/organization ids, field, before, after; `VERBOSE=true` also prints them) — 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/app/services/address_services/translate_existing.rb b/app/services/address_services/translate_existing.rb deleted file mode 100644 index 17bbae254c..0000000000 --- a/app/services/address_services/translate_existing.rb +++ /dev/null @@ -1,71 +0,0 @@ -module AddressServices - # Rewrites stored address countries and states as the dropdowns store them (ADR-0009), - # filling a locality only where it's Unknown. Form answers stay as people typed them; they're - # translated where they become addresses instead. Each changed address is one autochange - # Ahoy event, all in one visit. Idempotent, so it runs before and after an import. - class TranslateExisting - Result = Data.define(:addresses, :changes, :unmatched, :rows) - # One field one address changed (or would change, on a dry run), and whose address it is. - Row = Data.define(:address_id, :person_id, :organization_id, :field, :before, :after) - ROW_HEADERS = Row.members.freeze - - REASON = "address_translation" - - # `log` receives one line per changed address, 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) - @rows = [] - addresses = 0 - - ActiveRecord::Base.transaction do - Address.find_each { |address| addresses += 1 if translate(address) } - end - - Result.new(addresses:, changes: @changes, unmatched: @translation.unmatched, rows: @rows) - end - - private - - def translate(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) - end - - def apply(address, updates) - return false if updates.empty? - - person_id = address.addressable_id if address.addressable_type == "Person" - organization_id = address.addressable_id if address.addressable_type == "Organization" - updates.each do |field, after| - @changes["Address #{field}: #{address[field].inspect} → #{after.inspect}"] += 1 - @rows << Row.new(address_id: address.id, person_id:, organization_id:, field:, before: address[field], after:) - end - @log&.call(" Address ##{address.id}: #{updates.map { |field, after| "#{field} #{address[field].inspect} → #{after.inspect}" }.join(', ')}") - return true if @dry_run - - changes = updates.to_h { |field, after| [ field, { before: address[field], after: } ] } - address.update_columns(updates) - @tracker.track("autochange.address", { - resource_type: "Address", resource_id: address.id, resource_title: address.name, changes:, reason: REASON, - addressable_type: address.addressable_type, addressable_id: address.addressable_id - }) - true - end - end -end diff --git a/app/services/organization_services/profile_diff.rb b/app/services/organization_services/profile_diff.rb index 88f2a89a88..7a07bee2c1 100644 --- a/app/services/organization_services/profile_diff.rb +++ b/app/services/organization_services/profile_diff.rb @@ -71,29 +71,12 @@ def address_discrepancies [ field_discrepancy(:address_street, "Address – street", @address[:street_address], existing.street_address), field_discrepancy(:address_city, "Address – city", @address[:city], existing.city), - state_discrepancy(existing), + field_discrepancy(:address_state, "Address – state", @address[:state], existing.state), field_discrepancy(:address_zip, "Address – ZIP", @address[:zip_code], existing.zip_code), - country_discrepancy(existing) + field_discrepancy(:address_country, "Address – country", @address[:country], existing.country) ].compact end - # Compared as UpsertAddress stores it (ADR-0009), so "Texas"/"USA" matches the "TX"/"United States" it became. - def state_discrepancy(existing) - return if translation.state(@address[:state], country: @address[:country]).to_s.casecmp?(existing.state.to_s) - - field_discrepancy(:address_state, "Address – state", @address[:state], existing.state) - end - - def country_discrepancy(existing) - return if translation.country(@address[:country], state: @address[:state]).to_s.casecmp?(existing.country.to_s) - - field_discrepancy(:address_country, "Address – country", @address[:country], existing.country) - end - - def translation - @translation ||= AddressTranslation.new - end - def matching_address AddressMatcher.call( @organization, diff --git a/app/services/organization_services/upsert_address.rb b/app/services/organization_services/upsert_address.rb index c4d9cd8456..ec30295b59 100644 --- a/app/services/organization_services/upsert_address.rb +++ b/app/services/organization_services/upsert_address.rb @@ -39,11 +39,10 @@ def self.call(organization:, city: nil, state: nil, street_address: nil, zip_cod def initialize(organization:, city: nil, state: nil, street_address: nil, zip_code: nil, country: nil, overwrite: true) @organization = organization @city = city&.strip - translation = AddressTranslation.new - @state = translation.state(state, country:) + @state = state&.strip.presence @street_address = street_address&.strip @zip_code = zip_code&.strip - @country = translation.country(country, state:) if country.present? + @country = country&.strip.presence @overwrite = overwrite end diff --git a/app/services/person_services/capture_from_submission.rb b/app/services/person_services/capture_from_submission.rb index 3a0bb77ebe..afb787a0c5 100644 --- a/app/services/person_services/capture_from_submission.rb +++ b/app/services/person_services/capture_from_submission.rb @@ -91,11 +91,8 @@ def capture_mailing_address new_city = field_value("mailing_city")&.strip return if new_city.blank? - state_answer = field_value("mailing_state") - country_answer = field_value("mailing_country") - translation = AddressTranslation.new - new_state = translation.state(state_answer, country: country_answer) - new_country = translation.country(country_answer, state: state_answer) if country_answer.present? + new_state = field_value("mailing_state")&.strip + new_country = field_value("mailing_country")&.strip existing = @person.addresses.order(:id).find do |address| AddressNormalizer.city(address.city) == AddressNormalizer.city(new_city) && AddressNormalizer.state(address.state) == AddressNormalizer.state(new_state) diff --git a/db/seeds/features/2026-10-25-form-answers-stay-as-typed.yml b/db/seeds/features/2026-10-25-form-answers-stay-as-typed.yml deleted file mode 100644 index f5443197e3..0000000000 --- a/db/seeds/features/2026-10-25-form-answers-stay-as-typed.yml +++ /dev/null @@ -1,11 +0,0 @@ -name: "Form answers keep the country people typed" -area: registration -display_status: admin_facing -released_on: 2026-10-25 -pr_number: 2841 -summary: >- - The address cleanup now fixes addresses only — a person's form answers stay exactly as they - submitted them. When an answer is turned into an address (linking an organization, or saving - a registrant's mailing address), its country and state are stored the way the dropdowns do. -pro_tips: - - "The link-organization page treats an answer like USA or Texas as matching the United States or TX it was saved as, so it is not flagged as a difference." diff --git a/docs/adr/0009-legacy-address-value-translation.md b/docs/adr/0009-legacy-address-value-translation.md index 4516b213d6..ac2b26c657 100644 --- a/docs/adr/0009-legacy-address-value-translation.md +++ b/docs/adr/0009-legacy-address-value-translation.md @@ -22,11 +22,10 @@ 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 +One translator, `AddressTranslation`, used by every import. Like ADR-0007, translation is **by lookup table, and a value nothing answers to is 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, +`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", @@ -50,23 +49,17 @@ since a state is required and regions outside the US are legitimate. - **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, filling a - locality only where it's Unknown. It's idempotent, so it runs before and after an import. - Every changed address is an `autochange.address` Ahoy event with its before and after. -- **Where answers become addresses:** `OrganizationServices::UpsertAddress` (organization - capture and link-organization) and `PersonServices::CaptureFromSubmission` (the mailing - address) translate the country and state they're handed, so re-reading an old answer can't - add a spelling the dropdowns don't offer. A submission with no country leaves the stored - one alone. +- **Existing data:** cleaned once by hand (below). No task rewrites stored addresses. +- **Where answers become addresses:** answers are stored on the address as submitted. Every + form's state and country fields are dropdowns, and the older answers that could still + become an address were fixed in the cleanup. - **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, as a record of what the person submitted, and -`data:translate_addresses` never rewrites them. They're translated where they become -addresses instead (above). The one exception is the hand cleanup below. +Form answers are stored verbatim, as a record of what the person submitted; nothing rewrites +them. The one exception is the hand cleanup below. ### Prod data cleanup, 2026-10-24 to 2026-10-25 @@ -83,8 +76,7 @@ All other answers stay as typed. ## 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`, and a re-run fills the country it had cleared (from the - state, or by hand from the logged value). +- A new spelling turns up in an import's unmatched report; adding it is a line in `ALIASES` + or `REGIONS`. - The `fm_*` archive tables keep FileMaker's raw values. - Every stored country preselects in the dropdowns, so saving a form can't silently blank one. diff --git a/lib/tasks/translate_addresses.rake b/lib/tasks/translate_addresses.rake deleted file mode 100644 index a19381a356..0000000000 --- a/lib/tasks/translate_addresses.rake +++ /dev/null @@ -1,60 +0,0 @@ -# frozen_string_literal: true - -# Rewrite stored address countries/states as the dropdowns store them (form answers stay as -# typed). See AddressServices::TranslateExisting. -# -# 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 -# VERBOSE=true bin/rails data:translate_addresses # also print every change as CSV rows -# DRY_RUN=false bin/rails data:translate_addresses # translate + log -namespace :data do - desc "Translate stored address countries/states to dropdown values (form answers stay as typed). Idempotent. DRY_RUN=false to execute." - task translate_addresses: :environment do - dry_run = ENV["DRY_RUN"] != "false" - verbose = ENV["VERBOSE"] == "true" - 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 } - # Every change goes in the saved report; the console shows them only when VERBOSE=true. - note = ->(line) { puts line if verbose; report << line } - $stdout.sync = true - - begin - 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}") } - 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.("\nEvery change (CSV#{", in the saved report — VERBOSE=true prints them here" unless verbose}):") - note.(CSV.generate_line(AddressServices::TranslateExisting::ROW_HEADERS, row_sep: "")) - result.rows.each { |row| note.(CSV.generate_line(row.to_h.values, row_sep: "")) } - say.("\naddresses changed: #{result.addresses}") - 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 - end -end diff --git a/spec/requests/event_registrations_spec.rb b/spec/requests/event_registrations_spec.rb index c2858a0098..80cdb1511a 100644 --- a/spec/requests/event_registrations_spec.rb +++ b/spec/requests/event_registrations_spec.rb @@ -1863,7 +1863,7 @@ def details_open?(body, heading) address = organization.addresses.find_by(city: "Austin") expect(address).to be_present - expect(address.country).to eq("United States") + expect(address.country).to eq("USA") expect(regular_user.person.affiliations.where(organization: organization).map(&:organization_address)) .to all(eq(address)) end diff --git a/spec/services/address_services/translate_existing_spec.rb b/spec/services/address_services/translate_existing_spec.rb deleted file mode 100644 index 9ccb8a5f04..0000000000 --- a/spec/services/address_services/translate_existing_spec.rb +++ /dev/null @@ -1,128 +0,0 @@ -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 "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 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 "leaves form answers exactly as people typed them" do - submission = create(:form_submission) - typed = { "mailing_state" => "California", "mailing_country" => "USA", - "organization_country" => "Untied States", "organization_state" => "Texas" } - answers = typed.to_h { |identifier, value| [ identifier, answer(submission, identifier, value) ] } - - expect { described_class.new.call }.not_to change(Ahoy::Event.where(name: "autochange.form_answer"), :count) - - expect(answers.transform_values { _1.reload.submitted_answer }).to eq(typed) - end - - it "leaves every stored address country on the dropdown" do - address_with(country: "Narnia", state: "Central Region") - address_with(country: "USA", state: "TX") - - described_class.new.call - - expect(Address.where.not(country: [ nil, "" ]).pluck(:country) - GeographyHelper::COUNTRIES).to be_empty - end - - it "logs each changed address as one autochange event, all in one visit" do - usa = address_with(country: "USA", state: "TX", locality: Address::OUTSIDE_CA) - texas = address_with(country: nil, state: "Texas", locality: Address::OUTSIDE_CA) - - 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") - expect(Ahoy::Event.where(name: "autochange.address", resource_id: [ usa.id, texas.id ]).distinct.count(:visit_id)).to eq(1) - end - - it "lists every address change with its owner and before/after, on a dry run too" do - usa = address_with(country: "USA", state: "Texas", locality: Address::OUTSIDE_CA) - org_address = create(:address, addressable: create(:organization)).tap { |address| address.update_columns(country: "Usa", state: "TX", locality: Address::OUTSIDE_CA) } - - rows = described_class.new(dry_run: true).call.rows - - expect(rows).to contain_exactly( - have_attributes(address_id: usa.id, person_id: usa.addressable_id, organization_id: nil, - field: :country, before: "USA", after: "United States"), - have_attributes(address_id: usa.id, field: :state, before: "Texas", after: "TX"), - have_attributes(address_id: org_address.id, person_id: nil, organization_id: org_address.addressable_id, - field: :country, before: "Usa", after: "United States") - ) - 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, Ahoy::Visit.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 diff --git a/spec/services/event_registration_services/public_registration_spec.rb b/spec/services/event_registration_services/public_registration_spec.rb index 49c8c79dc6..36342a6502 100644 --- a/spec/services/event_registration_services/public_registration_spec.rb +++ b/spec/services/event_registration_services/public_registration_spec.rb @@ -115,7 +115,7 @@ def register_with(position:) address = organization.addresses.find_by(city: "Austin") expect(address).to be_present - expect(address.country).to eq("United States") + expect(address.country).to eq("USA") linked = person.affiliations.where(organization: organization) expect(linked.pluck(:title)).to contain_exactly("Counselor", "Facilitator") expect(linked.map(&:organization_address)).to all(eq(address)) @@ -396,7 +396,7 @@ def register(answers) described_class.call(event: event, registration_form: form, form_params: params) person = Person.find_by!(email: "ada@example.com") - expect(person.addresses.find_by(primary: true).country).to eq("United States") + expect(person.addresses.find_by(primary: true).country).to eq("USA") end it "updates a returning registrant's address when the city and state are spelled differently" do @@ -439,7 +439,7 @@ def register_with_org(extra) expect(organization.organization_type).to eq("501c3/nonprofit") expect(organization.website_url).to include("helpinghands.org") - expect(organization.addresses.find_by(primary: true).country).to eq("United States") + expect(organization.addresses.find_by(primary: true).country).to eq("USA") end # find_organization only ever finds, so a registrant always changes an org diff --git a/spec/services/organization_services/profile_diff_spec.rb b/spec/services/organization_services/profile_diff_spec.rb index f74c76aae5..52df0b36f6 100644 --- a/spec/services/organization_services/profile_diff_spec.rb +++ b/spec/services/organization_services/profile_diff_spec.rb @@ -105,17 +105,6 @@ expect(diff).to be_empty end - # UpsertAddress stores the dropdown spelling while the answer keeps the typed one. - it "does not flag a state or country the submission itself stored as the dropdowns do" do - create(:address, addressable: organization, street_address: "1 Main St", city: "Austin", state: "TX", zip_code: "78701", country: "United States") - - diff = described_class.call(organization: organization, address: { - street_address: "1 Main St", city: "Austin", state: "Texas", zip_code: "78701", country: "USA" - }) - - expect(diff).to be_empty - end - it "flags a city that differs on the address matched across cities by street and ZIP" do create(:address, addressable: organization, street_address: "1 Main St", city: "Saint Louis", state: "MO", zip_code: "63101") @@ -146,13 +135,5 @@ expect(diff).to be_empty end - - # The linking flow diffs right after the upsert, against the address those answers just built. - it "reports nothing on the address UpsertAddress just built from the same answers" do - submitted = { street_address: "1 Main St", city: "Austin", state: "Texas", zip_code: "78701", country: "USA" } - OrganizationServices::UpsertAddress.call(organization: organization, **submitted) - - expect(described_class.call(organization: organization, address: submitted)).to be_empty - end end end diff --git a/spec/services/organization_services/upsert_address_spec.rb b/spec/services/organization_services/upsert_address_spec.rb index 65a1b505d5..d2f4205250 100644 --- a/spec/services/organization_services/upsert_address_spec.rb +++ b/spec/services/organization_services/upsert_address_spec.rb @@ -20,27 +20,12 @@ city: "Austin", state: "TX", zip_code: "78701", - country: "United States", + country: "USA", address_type: "work", primary: true ) end - it "stores the submitted state and country as the dropdowns do, so an old answer can't add a stray spelling" do - result = described_class.call(organization:, street_address: "1 Main St", city: "Austin", state: "Texas", zip_code: "78701", country: "USA") - - expect(result.address).to have_attributes(state: "TX", country: "United States") - end - - it "leaves the country alone when the submission gives none" do - existing = create(:address, addressable: organization, street_address: "1 King St", city: "Toronto", state: "ON", zip_code: "M5H", country: "Canada") - - result = described_class.call(organization:, street_address: "1 King St", city: "Toronto", state: "ON", zip_code: "M5H") - - expect(existing.reload.country).to eq("Canada") - expect(result.changes).to be_empty - end - # Browsers don't trim text inputs, and a padded value stored verbatim would read # as a difference to ProfileDiff and get re-reported as filled on every relink. it "strips the submitted street and ZIP before storing them" do @@ -228,7 +213,7 @@ expect(result).to have_attributes(address: existing, created: false) expect(organization.addresses.count).to eq(1) # State already on file is kept (never flipped); the blank country is filled. - expect(existing.reload).to have_attributes(state: "Texas", country: "United States", street_address: "1 Main St") + expect(existing.reload).to have_attributes(state: "Texas", country: "USA", street_address: "1 Main St") end it "updates the same street/ZIP address instead of duplicating when the city is respelled, keeping the saved city" do @@ -246,7 +231,7 @@ expect(result).to have_attributes(address: existing, created: false) expect(organization.addresses.count).to eq(1) - expect(existing.reload).to have_attributes(city: "Saint Louis", country: "United States") + expect(existing.reload).to have_attributes(city: "Saint Louis", country: "USA") end it "fills a blank city on the matched address rather than leaving it empty" do diff --git a/spec/services/person_services/capture_from_submission_spec.rb b/spec/services/person_services/capture_from_submission_spec.rb index 7e51dbd657..7b7cb0e2d3 100644 --- a/spec/services/person_services/capture_from_submission_spec.rb +++ b/spec/services/person_services/capture_from_submission_spec.rb @@ -10,20 +10,6 @@ expect(person.addresses.sole).to have_attributes(city: "Springfield", street_address: "", zip_code: "") end - it "stores the answered state and country as the dropdowns do" do - described_class.call(person:, answers: { "mailing_city" => "Fresno", "mailing_state" => "California", "mailing_country" => "USA" }) - - expect(person.addresses.sole).to have_attributes(state: "CA", country: "United States") - end - - it "translates a country it fills on an address already on file" do - address = create(:address, addressable: person, city: "Toronto", state: "ON", country: nil) - - described_class.call(person:, answers: { "mailing_city" => "Toronto", "mailing_state" => "ON", "mailing_country" => "can" }, overwrite: false) - - expect(address.reload.country).to eq("Canada") - end - it "keeps a saved ZIP and street when an overwriting submission skips them" do address = create(:address, addressable: person, city: "Springfield", state: "IL", street_address: "1 Old Rd", zip_code: "62701") diff --git a/spec/tasks/translate_addresses_spec.rb b/spec/tasks/translate_addresses_spec.rb deleted file mode 100644 index efe4b83566..0000000000 --- a/spec/tasks/translate_addresses_spec.rb +++ /dev/null @@ -1,80 +0,0 @@ -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 = 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") - 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 "saves every change as CSV rows, printing them only with VERBOSE=true" do - usa = address_with(country: "USA", state: "TX") - row = "#{usa.id},#{usa.addressable_id},,country,USA,United States" - - output = run_task - - expect(report_text).to include("address_id,person_id,organization_id,field,before,after", row) - expect(output).not_to include(row) - - Rake::Task["data:translate_addresses"].reenable - ENV["VERBOSE"] = "true" - expect(run_task).to include(row) - ensure - ENV.delete("VERBOSE") - end - - 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 From 31f0d1b966d6ef192d0cb90b62a9648b7080ce80 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 09:48:50 -0400 Subject: [PATCH 09/10] ADR-0009: note the blank-field fill-in from the 2026-10-25 cleanup Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/adr/0009-legacy-address-value-translation.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/adr/0009-legacy-address-value-translation.md b/docs/adr/0009-legacy-address-value-translation.md index ac2b26c657..2e084ad3ff 100644 --- a/docs/adr/0009-legacy-address-value-translation.md +++ b/docs/adr/0009-legacy-address-value-translation.md @@ -66,6 +66,10 @@ them. The one exception is the hand cleanup below. Prod data was cleaned by hand over these two days, with addresses as the record: - **Addresses:** cities, states, and countries corrected in the portal. +- **Blank address fields filled in** from the rest of each address: country from the state, + county from the ZIP (FileMaker's ZIP table), and an Unknown locality from the city, county, + and the regions FileMaker projects give each California county. Values someone set were + kept. One `autochange.address_fills` Ahoy event points at the saved report of every fill. - **Some form answers**, because they hadn't become addresses yet: organization address answers on submissions not yet linked to an organization, which turn into the organization's address when someone links one. Fixed: country spellings, placeholder values From 5d0539ad10916e34252b0b770a4862f85d0e3824 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 11:17:51 -0400 Subject: [PATCH 10/10] AGENTS.md: 18 rake tasks after removing data:translate_addresses Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 78576108c3..4d8f83175d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -594,7 +594,7 @@ RuboCop linting on PRs and pushes to main. ## Rake Tasks -Located in `lib/tasks/` (19 files): +Located in `lib/tasks/` (18 files): - `dev.rake` — Development database seeding from XML/CSV - `rhino_migrator.rake` — Rich text editor migration - `attachment_report.rake` — Attachment reporting