diff --git a/AGENTS.md b/AGENTS.md index 7099d2f963..ee92d49bd2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -607,7 +607,7 @@ Located in `lib/tasks/` (20 files): - `migrate_workshop_logs.rake` — Workshop log migration - `backfill_user_stamps.rake` — Backfill `created_by_id`/`updated_by_id` on legacy rows from the Ahoy trail (`data:backfill_user_stamps`) - `backfill_notification_stamps.rake` — Backfill notifications' `created_by_id`/`updated_by_id` from `sender_id`, nil-only (`data:backfill_notification_stamps`) -- `translate_addresses.rake` — Rewrite stored address countries/states and their form answers as the dropdowns store them (`data:translate_addresses`, via `AddressServices::TranslateExisting`, idempotent, DRY_RUN, lists values nothing translates; every run, dry or not, saves its report as an ActiveStorage blob and prints how to read it back; 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 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) - `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 index 56e25e1e41..bf89e7e673 100644 --- a/app/services/address_services/translate_existing.rb +++ b/app/services/address_services/translate_existing.rb @@ -4,7 +4,10 @@ module AddressServices # Unknown. Each changed record is one autochange Ahoy event, all in one visit. Idempotent, # so it runs before and after an import. class TranslateExisting - Result = Data.define(:addresses, :answers, :changes, :unmatched) + 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) + ROW_HEADERS = Row.members.freeze REASON = "address_translation" # Each address answer, and the answer beside it that the translation reads alongside. @@ -24,6 +27,7 @@ def call @translation = AddressTranslation.new @tracker = Ahoy::Tracker.new(user: @user) @changes = Hash.new(0) + @rows = [] addresses = answers = 0 ActiveRecord::Base.transaction do @@ -31,7 +35,7 @@ def call address_answers.find_each { |answer| answers += 1 if translate_answer(answer) } end - Result.new(addresses:, answers:, changes: @changes, unmatched: @translation.unmatched) + Result.new(addresses:, answers:, changes: @changes, unmatched: @translation.unmatched, rows: @rows) end private @@ -46,7 +50,9 @@ def translate_address(address) locality = @translation.locality(city: address.city, state:, country:, county: address.county) updates[:locality] = locality if locality != address.locality end - apply(address, updates, title: address.name, addressable_type: address.addressable_type, addressable_id: address.addressable_id) + 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) end def translate_answer(answer) @@ -60,16 +66,19 @@ def translate_answer(answer) end return false if translated == raw.to_s || (translated.blank? && identifier.end_with?("_state")) - apply(answer, { submitted_answer: translated }, title: answer.form_field.name, + 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, title:, **extra) + def apply(record, updates, owner:, title:, **extra) return false if updates.empty? label = record.class.model_name.human updates.each do |field, after| @changes["#{label} #{field.to_s.humanize(capitalize: false)}: #{record[field].inspect} → #{after.inspect}"] += 1 + @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:) end @log&.call(" #{label} ##{record.id}: #{updates.map { |field, after| "#{field} #{record[field].inspect} → #{after.inspect}" }.join(', ')}") return true if @dry_run @@ -83,7 +92,7 @@ def apply(record, updates, title:, **extra) end def address_answers - FormAnswer.joins(:form_field).where(form_fields: { field_identifier: ANSWER_PAIRS.keys }).includes(:form_field) + 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 diff --git a/app/services/address_translation.rb b/app/services/address_translation.rb index 39b95336ad..2ed8920d00 100644 --- a/app/services/address_translation.rb +++ b/app/services/address_translation.rb @@ -30,12 +30,17 @@ class AddressTranslation "Australia" => [ "nsw", "new south wales", "vic", "victoria", "qld", "queensland", "tas", "tasmania", "south australia", "western australia" ], "United Kingdom" => [ "england", "scotland", "wales", "northern ireland", "london", "bucks", "buckinghamshire", - "somerset", "argyle and bute", "argyll and bute" ], + "somerset", "argyle and bute", "argyll and bute", "yorkshire", "e sussex", "east sussex" ], "Vietnam" => [ "ha noi", "hanoi" ], "Mexico" => [ "mexico city", "cdmx" ], "Japan" => [ "tokyo" ], "Philippines" => [ "metro manila" ], - "Ghana" => [ "greater accra" ], + "Ghana" => [ "greater accra", "accra" ], + "South Africa" => [ "gauteng" ], + "India" => [ "tamil nadu", "rajasthan" ], + "Pakistan" => [ "lahore" ], + "Liberia" => [ "montserrado", "monnserrado" ], + "Zimbabwe" => [ "matabeleland north", "matebeleland north" ], "Trinidad and Tobago" => [ "trinidad", "tobago" ] }.flat_map { |country, regions| regions.map { |region| [ region, country ] } }.to_h.freeze diff --git a/lib/tasks/translate_addresses.rake b/lib/tasks/translate_addresses.rake index a9a42ebc0d..8c880d45f8 100644 --- a/lib/tasks/translate_addresses.rake +++ b/lib/tasks/translate_addresses.rake @@ -8,17 +8,21 @@ # 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 and their form answers to dropdown values. 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 @@ -33,6 +37,9 @@ namespace :data do 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.("answers changed: #{result.answers}") say.(dry_run ? "\nDRY RUN — nothing written. Set DRY_RUN=false to execute." : "\nDone.") diff --git a/spec/services/address_services/translate_existing_spec.rb b/spec/services/address_services/translate_existing_spec.rb index adc5c320ad..12ec99ec5e 100644 --- a/spec/services/address_services/translate_existing_spec.rb +++ b/spec/services/address_services/translate_existing_spec.rb @@ -123,6 +123,26 @@ def answer(submission, identifier, value) expect(Ahoy::Event.where(name: %w[autochange.address autochange.form_answer]).distinct.count(:visit_id)).to eq(1) end + it "lists every 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") + ) + end + it "writes and logs nothing on a dry run, but reports what it would change" do usa = address_with(country: "USA", state: "TX") diff --git a/spec/services/address_translation_spec.rb b/spec/services/address_translation_spec.rb index 66e6075560..defc06d20c 100644 --- a/spec/services/address_translation_spec.rb +++ b/spec/services/address_translation_spec.rb @@ -48,6 +48,21 @@ expect(translation.country("West Africa", state: "Togo")).to eq("Togo") end + it "reads the regions staging's addresses name with no country" do + { "Gauteng" => "South Africa", "Accra" => "Ghana", "Tamil Nadu" => "India", "Rajasthan" => "India", + "Lahore" => "Pakistan", "MONNSERRADO" => "Liberia", "Montserrado" => "Liberia", + "Matebeleland North" => "Zimbabwe", "Matabeleland North" => "Zimbabwe", + "Yorkshire" => "United Kingdom", "E Sussex" => "United Kingdom", "East Sussex" => "United Kingdom" }.each do |state, country| + expect(translation.country(nil, state:)).to eq(country) + end + end + + it "leaves a place that names more than one country for a person to decide" do + [ "Christchurch", "Essex", "Salamanca", "Amsterdam", "West Indies" ].each do |state| + expect(translation.country(nil, state:)).to be_nil + end + end + it "clears a country it can't place, so every stored country is a dropdown one, and reports it" do expect(translation.country(" Narnia ", state: "Central Region")).to be_nil expect(translation.country("West Africa", state: "-")).to be_nil diff --git a/spec/tasks/translate_addresses_spec.rb b/spec/tasks/translate_addresses_spec.rb index 5a81f62698..b6aa4f8bcc 100644 --- a/spec/tasks/translate_addresses_spec.rb +++ b/spec/tasks/translate_addresses_spec.rb @@ -40,6 +40,22 @@ def report_text = saved_report.download.force_encoding(Encoding::UTF_8) 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 = "Address,#{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(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:)