From 8d177ca9992bbc268eebbd7b830afad31a8aec5d Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Thu, 8 Oct 2026 21:19:58 -0400 Subject: [PATCH 01/10] Add fm:import_people to turn Rolodex archive rows into people The FileMaker Rolodex is archived in fm_rolodexes but none of those contacts exist as people yet. Links on exact name + email so existing people aren't duplicated, and reports looser collisions for hand review. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 + .../person_services/import_from_rolodex.rb | 154 ++++++++++++++++++ lib/tasks/fm_import_people.rake | 34 ++++ .../import_from_rolodex_spec.rb | 99 +++++++++++ 4 files changed, 289 insertions(+) create mode 100644 app/services/person_services/import_from_rolodex.rb create mode 100644 lib/tasks/fm_import_people.rake create mode 100644 spec/services/person_services/import_from_rolodex_spec.rb diff --git a/AGENTS.md b/AGENTS.md index 7209978b43..6ad1ed218c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -280,6 +280,7 @@ action, or `authorize! :workshop, to: :summary?`). - `NewsSubscriptionCapture` — Successor to the retired person-level mailing-list consent flag: given a `person` + `source`, idempotently creates an active **News** `TopicSubscription` (skips when one is already active, no-op when the News topic isn't seeded). Called from `PublicRegistration` (source = event) and `PublicFormSubmission` (source = form) when the `communication_consent` question is answered - `OrganizationServices::CaptureFromSubmission` — The shared org-capture for **both** the event registration (`PublicRegistration`) and standalone (`PublicFormSubmission`) flows: resolves the submitted organization by name (`FormField::ORGANIZATION_NAME_FIELD_IDENTIFIER`, case-insensitive per the column collation; never creates one — an unmatched name is left for an admin), syncs its website/type (`SyncProfile`, latest-wins) and work address (`UpsertAddress`, which adds another work address rather than rewriting a different one). `AffiliationServices::CreateFromRegistration` then mints the person's job affiliation from the typed position, plus the standing Facilitator one when `facilitator_training:` — which only an event supplies (`@event.facilitator_training`), so a standalone form gets the job affiliation alone, the same as registering for a non-training event. **Agreement-role submissions never come through here** — they run `LinkSubmittedOrganization`, which owns scenario end-dating and the new-job start date. Returns the org + the autofill changes, which the event flow records on its registration↔org link - `PersonServices::CaptureFromSubmission` — The shared person-capture for **both** the event registration (`PublicRegistration`) and standalone (`PublicFormSubmission`) flows, run for a newly-created **and** an existing person, so the person smart fields on the Smart form settings page behave identically across both forms. Fills identity extras (pronouns, name `pronunciation`, `nickname` → first name / legal first name, primary email type, secondary email + type), the profile (`racial_ethnic_identity` + `SyncSharingPreferences`), the mailing address, the phone contact method, and sector/age tags (mirrored onto any organizations passed in). A non-blank answer overwrites what's on file (newest submission wins); a blank never clobbers. Sector and age tags both run through `SubmissionTagging` and follow one rule on the **person**: submitting both a primary and additional set **overwrites** the person's tags with the latest (`replace: true`), submitting only a primary **reassigns** the primary and leaves the rest (additive); the org mirror always stays additive (orgs aggregate their members' tags). Self-gates on each field identifier, so a form that doesn't ask a question no-ops for it +- `PersonServices::ImportFromRolodex` — Creates people from FileMaker Rolodex archive rows (`FmRolodex`), carrying the row's ID as `filemaker_code` plus profile fields and the primary address/phone. Links an existing person on an exact name + email match; flags email-only, same-name-no-email, and already-linked collisions as ambiguous for hand review; skips agencies, soft-deleted, and nameless rows. Idempotent on `filemaker_code`. Driven by `fm:import_people` - `PersonServices::SyncSharingPreferences` — Writes the two content-sharing answers (`anonymous_contributions`, `display_name_preference`) onto the `Person`, mapping each submitted option label back to its stored value; an unanswered or unrecognized answer leaves the profile alone, so a form that doesn't ask the questions is a no-op and every flow can call it unconditionally. The person-side counterpart to `OrganizationServices::SyncProfile`. Shared by `PersonServices::CaptureFromSubmission` (so a submission's answers land in the "what this submission changed" audit via the Ahoy trail) and `CalloutFormSubmission` (whose `#profile_changes` re-reports the same edits for its own Ahoy tracking) - `EventRegistrationServices::TransferContinuingEducation` — Splits/relocates a registrant's CE when they transfer events (issue #1944): a simple forward transfer leaves a paid, zero-hours **stub** on the source (its payments count at the original event) and creates a **live** record on the destination carrying the hours and the outstanding balance; when the reg being transferred out is itself a transfer-in (a collapsing double transfer, or a transfer back to the origin) its live record is relocated forward — merging back into the origin's stub — instead of split again, so no third record appears. Runs inside the transfer transaction, after the destination is saved and before a collapsing middle reg is destroyed - `EventRegistrationServices::ReconcileMergedContinuingEducation` — Run from the event-registration deduper's `after_merge`. A CE registration moved from the deleted registration still points at that (deleted) registrant's license — invalid, since the registration now belongs to the kept registrant — so this repoints each to the kept registrant's equivalent license (same kind + number, found or created), then folds the resulting duplicates into one with `ContinuingEducationDeduper` so payments are preserved @@ -576,6 +577,7 @@ Located in `lib/tasks/` (16 files): - `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`) - `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) - `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) - `seed_tag_aliases.rake` — Record the legacy sector and age-group terms from ADR-0007 as tag aliases (`data:seed_tag_aliases`, via `TagAliasSeeder`, idempotent; also part of `db:seed`) - `seed_survey_forms.rake` — Create the Day 1 / Day 2 / recipients survey template forms without the full db:seed (`data:seed_survey_forms`, via `SurveyFormSeeder`, idempotent on form name; run before configuring the survey callouts) diff --git a/app/services/person_services/import_from_rolodex.rb b/app/services/person_services/import_from_rolodex.rb new file mode 100644 index 0000000000..ecaf6168b3 --- /dev/null +++ b/app/services/person_services/import_from_rolodex.rb @@ -0,0 +1,154 @@ +module PersonServices + # Turns FileMaker Rolodex archive rows (FmRolodex) into people. Each row lands in + # one outcome bucket of #results: + # * created — new Person, with the row's primary address and phone + # * linked — an existing person with the same name + email gets the + # row's ID as their filemaker_code + # * already_imported — a person already carries the row's ID + # * ambiguous — a name or email collides with someone we can't safely + # claim is the same person; reconcile by hand + # * skipped — agency, soft-deleted, or nameless rows + # * failed — the Person didn't validate + class ImportFromRolodex + OUTCOMES = %i[created linked already_imported ambiguous skipped failed].freeze + FALSE_FLAGS = %w[0 n no false].freeze + DATE_FORMATS = %w[%m/%d/%Y %Y-%m-%d].freeze + CONTACT_TYPES = { "home" => "personal", "personal" => "personal", "work" => "work", "mailing" => "mailing" }.freeze + EMAIL_PATTERN = /[^\s,;<>"']+@[^\s,;<>"']+\.[^\s,;<>"']+/ + UNKNOWN_LOCALITY = "Unknown" + + attr_reader :results + + def initialize(scope: FmRolodex.all) + @scope = scope + @results = OUTCOMES.index_with { [] } + @imported_codes = Person.where.not(filemaker_code: [ nil, "" ]).pluck(:filemaker_code).map(&:strip).to_set + end + + def call + @scope.find_each { |rolodex| import(rolodex) } + self + end + + private + + def import(rolodex) + data = rolodex.data + first_name = data["FirstName"].to_s.strip + last_name = data["LastName"].to_s.strip + label = "#{rolodex.fm_id} #{first_name} #{last_name}".squish + + return record(:already_imported, label) if @imported_codes.include?(rolodex.fm_id) + return record(:skipped, "#{label}: agency") if flag?(data["isAgency__c"]) + return record(:skipped, "#{label}: marked deleted") if flag?(data["Delete"]) + return record(:skipped, "#{label}: missing first or last name") if first_name.blank? || last_name.blank? + + email, email_2 = data["EMail"].to_s.scan(EMAIL_PATTERN) + same_name = Person.where("LOWER(first_name) = ? AND LOWER(last_name) = ?", first_name.downcase, last_name.downcase) + + if email + match = same_name.where("LOWER(email) = :email OR LOWER(email_2) = :email", email: email.downcase).to_a + return link(match.first, rolodex, label) if match.one? + return record(:ambiguous, "#{label}: #{match.size} people share this name and email (#{ids(match)})") if match.many? + + others = Person.where("LOWER(email) = :email OR LOWER(email_2) = :email", email: email.downcase).to_a + return record(:ambiguous, "#{label}: #{email} belongs to #{others.map { |p| "##{p.id} #{p.full_name}" }.join(', ')}") if others.any? + else + no_email = same_name.where(email: [ nil, "" ]).to_a + return record(:ambiguous, "#{label}: no email, same name as #{ids(no_email)}") if no_email.any? + end + + create(rolodex, label, first_name:, last_name:, email:, email_2:) + end + + def link(person, rolodex, label) + if person.filemaker_code.present? + return record(:ambiguous, "#{label}: matches ##{person.id}, already linked to FileMaker #{person.filemaker_code}") + end + + return record(:failed, "#{label}: ##{person.id} #{person.errors.full_messages.to_sentence}") unless person.update(filemaker_code: rolodex.fm_id) + + @imported_codes << rolodex.fm_id + record(:linked, "#{label} → ##{person.id}") + end + + def create(rolodex, label, **names) + data = rolodex.data + person = Person.new( + **names, + filemaker_code: rolodex.fm_id, + pronouns: data["Pronouns"].presence, + best_time_to_call: data["BestTimeToCall"].presence, + racial_ethnic_identity: data["Ethnicity"].presence, + date_of_birth: parse_date(data["DateOfBirth"]), + notes: data["Comments"].presence + ) + address = address_attributes(data["PrimaryAddrsID"]) + person.addresses.build(address) if address + phone = phone_attributes(data["PrimaryPhoneID"]) + person.contact_methods.build(phone) if phone + + return record(:failed, "#{label}: #{person.errors.full_messages.to_sentence}") unless person.save + + @imported_codes << rolodex.fm_id + record(:created, "#{label} → ##{person.id}") + end + + def address_attributes(fm_id) + fields = fm_id.presence && FmAddress.find_by(fm_id:)&.data + return unless fields + + street = [ fields["AddrsLine1"], fields["AddrsLine2"] ].filter_map { |line| line.to_s.strip.presence }.join(", ") + city = fields["AddrsCity"].to_s.strip + state = fields["AddrsState"].to_s.strip + return if street.blank? || city.blank? || state.blank? + + { + street_address: street, + city:, + state:, + zip_code: fields["AddrsPostalCode"].to_s.strip, + country: fields["AddrsCountry"].presence, + address_type: contact_type(fields["AddrsType"]), + locality: UNKNOWN_LOCALITY, + primary: true + } + end + + def phone_attributes(fm_id) + fields = fm_id.presence && FmPhoneNumber.find_by(fm_id:)&.data + number = fields && fields["PhoneNumber"].to_s.strip + return if number.blank? + + { kind: "phone", value: number, contact_type: contact_type(fields["PhoneType"]), primary: true } + end + + def contact_type(value) + CONTACT_TYPES[value.to_s.strip.downcase] + end + + def parse_date(value) + value = value.to_s.strip + return if value.blank? + + DATE_FORMATS.each do |format| + return Date.strptime(value, format) + rescue Date::Error + next + end + nil + end + + def flag?(value) + value.to_s.strip.presence && !FALSE_FLAGS.include?(value.to_s.strip.downcase) + end + + def ids(people) + people.map { |person| "##{person.id}" }.join(", ") + end + + def record(outcome, message) + @results[outcome] << message + end + end +end diff --git a/lib/tasks/fm_import_people.rake b/lib/tasks/fm_import_people.rake new file mode 100644 index 0000000000..a5ce188e88 --- /dev/null +++ b/lib/tasks/fm_import_people.rake @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +# Create people from the FileMaker Rolodex archive (fm_rolodexes, loaded by fm:import). +# See PersonServices::ImportFromRolodex for the match/link/create rules. Idempotent: +# a row whose ID is already some person's filemaker_code is left alone. +# +# Dry-run by default. Set DRY_RUN=false to write changes; VERBOSE=true lists every row. +# bin/rails fm:import_people # preview, writes nothing +# DRY_RUN=false bin/rails fm:import_people # create + link people +namespace :fm do + desc "Create people from FileMaker Rolodex archive rows (link existing by name + email). Idempotent. DRY_RUN=false to execute." + task import_people: :environment do + dry_run = ENV["DRY_RUN"] != "false" + verbose = ENV["VERBOSE"] == "true" + detailed = verbose ? PersonServices::ImportFromRolodex::OUTCOMES : %i[ambiguous failed] + import = nil + + ActiveRecord::Base.transaction do + import = PersonServices::ImportFromRolodex.new.call + raise ActiveRecord::Rollback if dry_run + end + + import.results.each do |outcome, messages| + next unless detailed.include?(outcome) && messages.any? + + puts "\n#{outcome.to_s.humanize} (#{messages.size}):" + messages.each { |message| puts " #{message}" } + end + + puts + import.results.each { |outcome, messages| puts "#{outcome.to_s.humanize.ljust(17)} #{messages.size}" } + puts(dry_run ? "\nDRY RUN — rolled back, nothing written. Set DRY_RUN=false to execute." : "\nDone.") + end +end diff --git a/spec/services/person_services/import_from_rolodex_spec.rb b/spec/services/person_services/import_from_rolodex_spec.rb new file mode 100644 index 0000000000..ccab0afe15 --- /dev/null +++ b/spec/services/person_services/import_from_rolodex_spec.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe PersonServices::ImportFromRolodex do + def rolodex(fm_id, **data) + FmRolodex.create!(fm_id:, data: { "FirstName" => "Ada", "LastName" => "Lovelace" }.merge(data.stringify_keys)) + end + + def results + described_class.new.call.results + end + + it "creates a person with profile fields, primary address, and primary phone" do + FmAddress.create!(fm_id: "A1", data: { "AddrsLine1" => "1 Main St", "AddrsLine2" => "Apt 2", "AddrsCity" => "Los Angeles", + "AddrsState" => "CA", "AddrsPostalCode" => "90001", "AddrsType" => "Home" }) + FmPhoneNumber.create!(fm_id: "P1", data: { "PhoneNumber" => "555-0100", "PhoneType" => "Work" }) + rolodex("00006", EMail: "ada@example.com", Pronouns: "she/her", DateOfBirth: "12/10/1815", + Comments: "Met at training", PrimaryAddrsID: "A1", PrimaryPhoneID: "P1") + + expect(results[:created].size).to eq(1) + + person = Person.find_by!(filemaker_code: "00006") + expect(person).to have_attributes(first_name: "Ada", last_name: "Lovelace", email: "ada@example.com", + pronouns: "she/her", date_of_birth: Date.new(1815, 12, 10), notes: "Met at training") + expect(person.addresses.sole).to have_attributes(street_address: "1 Main St, Apt 2", city: "Los Angeles", state: "CA", + zip_code: "90001", address_type: "personal", locality: "Unknown", primary: true) + expect(person.contact_methods.sole).to have_attributes(kind: "phone", value: "555-0100", contact_type: "work", primary: true) + end + + it "splits two emails in one field into email and email_2" do + rolodex("00007", EMail: "ada@example.com; ada@work.example.com") + + results + + expect(Person.find_by!(filemaker_code: "00007")).to have_attributes(email: "ada@example.com", email_2: "ada@work.example.com") + end + + it "links an existing person with the same name and email instead of creating one" do + person = create(:person, first_name: "ada", last_name: "LOVELACE", email: "Ada@Example.com") + rolodex("00008", EMail: "ada@example.com") + + expect { results }.not_to change(Person, :count) + expect(person.reload.filemaker_code).to eq("00008") + end + + it "leaves rows whose ID a person already carries alone" do + create(:person, filemaker_code: "00009") + rolodex("00009") + + expect(results[:already_imported].size).to eq(1) + expect(Person.where(filemaker_code: "00009").count).to eq(1) + end + + it "flags an email that belongs to a differently named person" do + create(:person, first_name: "Charles", last_name: "Babbage", email: "ada@example.com") + rolodex("00010", EMail: "ada@example.com") + + expect { expect(results[:ambiguous].size).to eq(1) }.not_to change(Person, :count) + end + + it "flags a same-name match with no email to confirm it" do + create(:person, first_name: "Ada", last_name: "Lovelace", email: nil) + rolodex("00011") + + expect { expect(results[:ambiguous].size).to eq(1) }.not_to change(Person, :count) + end + + it "flags a name + email match already linked to another FileMaker record" do + person = create(:person, first_name: "Ada", last_name: "Lovelace", email: "ada@example.com", filemaker_code: "99999") + rolodex("00012", EMail: "ada@example.com") + + expect(results[:ambiguous].size).to eq(1) + expect(person.reload.filemaker_code).to eq("99999") + end + + it "skips agencies, deleted rows, and nameless rows" do + rolodex("00013", isAgency__c: "1") + rolodex("00014", Delete: "Yes") + rolodex("00015", FirstName: "", LastName: "") + + expect(results[:skipped].size).to eq(3) + expect(Person.where(filemaker_code: %w[00013 00014 00015])).to be_empty + end + + it "treats a false-y flag as unset" do + rolodex("00016", isAgency__c: "0", Delete: "No") + + expect(results[:created].size).to eq(1) + end + + it "doesn't create the same person twice when the import reruns" do + rolodex("00017", EMail: "ada@example.com") + + described_class.new.call + + expect { results }.not_to change(Person, :count) + end +end From 44f3e4971c877523e8de59ef56c28b70ff39bad8 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Mon, 12 Oct 2026 12:12:53 -0400 Subject: [PATCH 02/10] Keep the Rolodex import from losing rows to its own edge cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A dry run over 39,000 rows is only useful if the rows it reports are the rows it would write: - Two-digit years landed in the first century (10/5/83 became 83 AD), and because strptime stops where the format ends, 10-5-1983 read as 1910-05-19 — a date the archive never recorded. Formats now have to match the whole value, and a bare year pair gets a century. - An over-long address line or note failed the entire person on a 255-character column, dropping a usable record over an incidental field. Copied free text is truncated instead. - The house number has its own field, so addresses imported without it unless the first line happened to repeat it. - Phone types were resolved against the address set, letting a phone claim a type the contact-method form can't show. Each model now declares the slice it accepts, and a type we can't place reads as unknown rather than leaving the record untyped. Also drops the credentials.yml key that came back in a rebase — #2579 removed it deliberately — and corrects the rake-task count in AGENTS.md. Co-Authored-By: Claude Opus 5 (1M context) --- app/models/contact_method.rb | 2 +- .../person_services/import_from_rolodex.rb | 78 +++++++++++++++---- .../import_from_rolodex_spec.rb | 68 ++++++++++++++++ 3 files changed, 132 insertions(+), 16 deletions(-) diff --git a/app/models/contact_method.rb b/app/models/contact_method.rb index c361eb2220..6bce340c21 100644 --- a/app/models/contact_method.rb +++ b/app/models/contact_method.rb @@ -1,5 +1,5 @@ class ContactMethod < ApplicationRecord - CONTACT_TYPES = [ nil, "work", "personal" ].freeze + CONTACT_TYPES = [ nil, "work", "personal", "unknown" ].freeze belongs_to :contactable, polymorphic: true belongs_to :created_by, class_name: "User", optional: true diff --git a/app/services/person_services/import_from_rolodex.rb b/app/services/person_services/import_from_rolodex.rb index ecaf6168b3..d65d088512 100644 --- a/app/services/person_services/import_from_rolodex.rb +++ b/app/services/person_services/import_from_rolodex.rb @@ -9,13 +9,22 @@ module PersonServices # claim is the same person; reconcile by hand # * skipped — agency, soft-deleted, or nameless rows # * failed — the Person didn't validate + # + # A row whose email matches nobody is created even when someone already shares the + # name: folding the occasional duplicate with the person deduper beats hand-resolving + # every name collision in a 39,000-row archive. class ImportFromRolodex OUTCOMES = %i[created linked already_imported ambiguous skipped failed].freeze FALSE_FLAGS = %w[0 n no false].freeze - DATE_FORMATS = %w[%m/%d/%Y %Y-%m-%d].freeze - CONTACT_TYPES = { "home" => "personal", "personal" => "personal", "work" => "work", "mailing" => "mailing" }.freeze + DATE_FORMATS = %w[%m/%d/%Y %Y-%m-%d %m-%d-%Y %Y/%m/%d].freeze + # A two-digit year below this is read as 20xx, at or above it as 19xx. + TWO_DIGIT_YEAR_PIVOT = 10 + CONTACT_TYPES = { "home" => "personal", "personal" => "personal", "cell" => "personal", + "mobile" => "personal", "work" => "work", "mailing" => "mailing" }.freeze EMAIL_PATTERN = /[^\s,;<>"']+@[^\s,;<>"']+\.[^\s,;<>"']+/ UNKNOWN_LOCALITY = "Unknown" + UNKNOWN_CONTACT_TYPE = "unknown" + STRING_LIMIT = 255 attr_reader :results @@ -77,9 +86,9 @@ def create(rolodex, label, **names) person = Person.new( **names, filemaker_code: rolodex.fm_id, - pronouns: data["Pronouns"].presence, - best_time_to_call: data["BestTimeToCall"].presence, - racial_ethnic_identity: data["Ethnicity"].presence, + pronouns: clip(data["Pronouns"]), + best_time_to_call: clip(data["BestTimeToCall"]), + racial_ethnic_identity: clip(data["Ethnicity"]), date_of_birth: parse_date(data["DateOfBirth"]), notes: data["Comments"].presence ) @@ -98,18 +107,18 @@ def address_attributes(fm_id) fields = fm_id.presence && FmAddress.find_by(fm_id:)&.data return unless fields - street = [ fields["AddrsLine1"], fields["AddrsLine2"] ].filter_map { |line| line.to_s.strip.presence }.join(", ") + street = [ street_line(fields), fields["AddrsLine2"] ].filter_map { |line| line.to_s.strip.presence }.join(", ") city = fields["AddrsCity"].to_s.strip state = fields["AddrsState"].to_s.strip return if street.blank? || city.blank? || state.blank? { - street_address: street, + street_address: clip(street), city:, state:, zip_code: fields["AddrsPostalCode"].to_s.strip, - country: fields["AddrsCountry"].presence, - address_type: contact_type(fields["AddrsType"]), + country: clip(fields["AddrsCountry"]), + address_type: contact_type(fields["AddrsType"], allowed: Address::CONTACT_TYPES), locality: UNKNOWN_LOCALITY, primary: true } @@ -120,11 +129,31 @@ def phone_attributes(fm_id) number = fields && fields["PhoneNumber"].to_s.strip return if number.blank? - { kind: "phone", value: number, contact_type: contact_type(fields["PhoneType"]), primary: true } + { kind: "phone", value: number, contact_type: contact_type(fields["PhoneType"], allowed: ContactMethod::CONTACT_TYPES), primary: true } end - def contact_type(value) - CONTACT_TYPES[value.to_s.strip.downcase] + # The archive's types map onto one set of ours, but each model declares the slice it + # accepts — an address can be the mailing one, a phone number can't. A type we can't + # place is recorded as unknown, so an imported record never reads as simply untyped. + def contact_type(value, allowed:) + mapped = CONTACT_TYPES[value.to_s.strip.downcase] + mapped.presence_in(allowed) || UNKNOWN_CONTACT_TYPE.presence_in(allowed) + end + + # The house number has its own field, sometimes already repeated at the front of + # the first line. + def street_line(fields) + number = fields["StreetNo"].to_s.strip + line = fields["AddrsLine1"].to_s.strip + return line if number.blank? || line[/\A\S+/] == number + + "#{number} #{line}".strip + end + + # Every string copied here lands in a varchar(255) the archive's free text isn't + # bounded by, and an over-long incidental field would otherwise cost the person. + def clip(value) + value.presence && value.to_s.truncate(STRING_LIMIT) end def parse_date(value) @@ -132,13 +161,32 @@ def parse_date(value) return if value.blank? DATE_FORMATS.each do |format| - return Date.strptime(value, format) - rescue Date::Error - next + parsed = parse_with(value, format) + return parsed if parsed end nil end + # Parsing stops where the format ends, so "10-5-1983" read as %Y-%m-%d takes 19 + # as the day and drops the 83 — digits left over mean the format split the value + # in the wrong place, so the next one gets its turn. + def parse_with(value, format) + parts = Date._strptime(value, format) + return if parts.nil? || parts[:leftover].to_s.match?(/\A\d/) + + expand_century(Date.new(parts[:year], parts[:mon], parts[:mday])) + rescue Date::Error, TypeError + nil + end + + # %Y reads a two-digit year as the year itself, so "10/5/83" is 83 AD. The archive + # mixes two- and four-digit years, so a bare pair gets a century: 83 is 1983, 05 is 2005. + def expand_century(date) + return date if date.year >= 100 + + date.change(year: date.year + (date.year < TWO_DIGIT_YEAR_PIVOT ? 2000 : 1900)) + end + def flag?(value) value.to_s.strip.presence && !FALSE_FLAGS.include?(value.to_s.strip.downcase) end diff --git a/spec/services/person_services/import_from_rolodex_spec.rb b/spec/services/person_services/import_from_rolodex_spec.rb index ccab0afe15..ebf5d89b6b 100644 --- a/spec/services/person_services/import_from_rolodex_spec.rb +++ b/spec/services/person_services/import_from_rolodex_spec.rb @@ -36,6 +36,74 @@ def results expect(Person.find_by!(filemaker_code: "00007")).to have_attributes(email: "ada@example.com", email_2: "ada@work.example.com") end + it "prefixes the street number when the first line doesn't already carry it" do + FmAddress.create!(fm_id: "A2", data: { "StreetNo" => "1234", "AddrsLine1" => "Main St", + "AddrsCity" => "Los Angeles", "AddrsState" => "CA" }) + FmAddress.create!(fm_id: "A3", data: { "StreetNo" => "1234", "AddrsLine1" => "1234 Main St", + "AddrsCity" => "Los Angeles", "AddrsState" => "CA" }) + rolodex("00030", FirstName: "Grace", EMail: "grace@example.com", PrimaryAddrsID: "A2") + rolodex("00031", FirstName: "Hedy", EMail: "hedy@example.com", PrimaryAddrsID: "A3") + + results + + expect(Person.find_by!(filemaker_code: "00030").addresses.sole.street_address).to eq("1234 Main St") + expect(Person.find_by!(filemaker_code: "00031").addresses.sole.street_address).to eq("1234 Main St") + end + + it "truncates over-long free text instead of losing the person to it" do + FmAddress.create!(fm_id: "A4", data: { "AddrsLine1" => "x" * 200, "AddrsLine2" => "y" * 200, + "AddrsCity" => "Los Angeles", "AddrsState" => "CA" }) + rolodex("00032", EMail: "ada@example.com", BestTimeToCall: "z" * 300, PrimaryAddrsID: "A4") + + expect(results[:failed]).to be_empty + + person = Person.find_by!(filemaker_code: "00032") + expect(person.best_time_to_call.length).to eq(255) + expect(person.addresses.sole.street_address.length).to eq(255) + end + + it "types a phone the contact method can't take as unknown, keeping it on the address" do + FmAddress.create!(fm_id: "A5", data: { "AddrsLine1" => "1 Main St", "AddrsCity" => "Los Angeles", + "AddrsState" => "CA", "AddrsType" => "Mailing" }) + FmPhoneNumber.create!(fm_id: "P2", data: { "PhoneNumber" => "555-0100", "PhoneType" => "Mailing" }) + FmPhoneNumber.create!(fm_id: "P3", data: { "PhoneNumber" => "555-0200", "PhoneType" => "Cell" }) + rolodex("00033", EMail: "ada@example.com", PrimaryAddrsID: "A5", PrimaryPhoneID: "P2") + rolodex("00034", FirstName: "Grace", EMail: "grace@example.com", PrimaryPhoneID: "P3") + + results + + mailing = Person.find_by!(filemaker_code: "00033") + expect(mailing.addresses.sole.address_type).to eq("mailing") + expect(mailing.contact_methods.sole.contact_type).to eq("unknown") + expect(Person.find_by!(filemaker_code: "00034").contact_methods.sole.contact_type).to eq("personal") + end + + it "types an address and phone the archive left blank as unknown" do + FmAddress.create!(fm_id: "A6", data: { "AddrsLine1" => "1 Main St", "AddrsCity" => "Los Angeles", "AddrsState" => "CA" }) + FmPhoneNumber.create!(fm_id: "P4", data: { "PhoneNumber" => "555-0300" }) + rolodex("00035", EMail: "ada@example.com", PrimaryAddrsID: "A6", PrimaryPhoneID: "P4") + + results + + person = Person.find_by!(filemaker_code: "00035") + expect(person.addresses.sole.address_type).to eq("unknown") + expect(person.contact_methods.sole.contact_type).to eq("unknown") + end + + it "reads the archive's date formats, giving a two-digit year its century" do + rolodex("00020", FirstName: "Grace", EMail: "grace@example.com", DateOfBirth: "10/5/83") + rolodex("00021", FirstName: "Katherine", EMail: "katherine@example.com", DateOfBirth: "3/4/05") + rolodex("00022", FirstName: "Hedy", EMail: "hedy@example.com", DateOfBirth: "10-5-1983") + rolodex("00023", FirstName: "Mary", EMail: "mary@example.com", DateOfBirth: "1983/10/05") + + results + + expect(Person.find_by!(filemaker_code: "00020").date_of_birth).to eq(Date.new(1983, 10, 5)) + expect(Person.find_by!(filemaker_code: "00021").date_of_birth).to eq(Date.new(2005, 3, 4)) + expect(Person.find_by!(filemaker_code: "00022").date_of_birth).to eq(Date.new(1983, 10, 5)) + expect(Person.find_by!(filemaker_code: "00023").date_of_birth).to eq(Date.new(1983, 10, 5)) + end + it "links an existing person with the same name and email instead of creating one" do person = create(:person, first_name: "ada", last_name: "LOVELACE", email: "Ada@Example.com") rolodex("00008", EMail: "ada@example.com") From 367d50bee7d8e84a6c151c8cac824881a1447bdb Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Mon, 12 Oct 2026 12:24:58 -0400 Subject: [PATCH 03/10] Record an untyped address or phone as unknown instead of blank MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every address and contact method now answers "what kind?": a blank type is written as unknown on save, from any path, so the gap closes as records are touched rather than needing a backfill. Rows nothing saves keep their blank. Knowing that a type is unknown isn't the same as knowing the type, so the public registration form still asks a signed-in registrant for their address type — Address#known_type? keeps the normalized value from reading as an answer already on file and hiding the question. With the models owning the fallback, the Rolodex import drops its own. Features & tips gains the import and the new Unknown type. Co-Authored-By: Claude Opus 5 (1M context) --- .../events/public_registrations_controller.rb | 2 +- app/models/address.rb | 19 ++++++++++++- app/models/contact_method.rb | 13 ++++++++- .../person_services/import_from_rolodex.rb | 6 ++--- config/features.yml | 27 +++++++++++++++++++ spec/models/address_spec.rb | 24 +++++++++++++++++ 6 files changed, 84 insertions(+), 7 deletions(-) diff --git a/app/controllers/events/public_registrations_controller.rb b/app/controllers/events/public_registrations_controller.rb index 10be7b9a4b..0cdacee4a0 100644 --- a/app/controllers/events/public_registrations_controller.rb +++ b/app/controllers/events/public_registrations_controller.rb @@ -404,7 +404,7 @@ def person_known_identifiers(person) if person.addresses.exists? address = person.addresses.find_by(primary: true) || person.addresses.first keys << "mailing_street" if address.street_address.present? - keys << "mailing_address_type" if address.address_type.present? + keys << "mailing_address_type" if address.known_type? keys << "mailing_city" if address.city.present? keys << "mailing_state" if address.state.present? keys << "mailing_zip" if address.zip_code.present? diff --git a/app/models/address.rb b/app/models/address.rb index 2b91986bbc..8fa490e80e 100644 --- a/app/models/address.rb +++ b/app/models/address.rb @@ -4,7 +4,8 @@ class Address < ApplicationRecord LOCALITIES = [ "LA City", "LA County", "Southern CA", "Northern CA", "Central CA", "Orange County", "Outside CA", "Outside USA", "Unknown" ] - CONTACT_TYPES = [ nil, "work", "personal", "mailing", "unknown" ].freeze + UNKNOWN_CONTACT_TYPE = "unknown" + CONTACT_TYPES = [ nil, "work", "personal", "mailing", UNKNOWN_CONTACT_TYPE ].freeze # USPS abbreviations for the 50 states, DC, and the US territories the atlas # draws — the whitelist behind every "States" breakdown, so international # regions (e.g. "ON", "England") are excluded (they belong to the Countries map). @@ -19,6 +20,8 @@ class Address < ApplicationRecord # the link rather than block deletion when an org address is removed. has_many :affiliations, foreign_key: :organization_address_id, dependent: :nullify, inverse_of: :organization_address + before_validation :type_as_unknown + validates :locality, presence: true validates :city, presence: true, length: { maximum: 255 } validates :state, presence: true @@ -35,4 +38,18 @@ class Address < ApplicationRecord def name "#{street_address}, #{city}, #{state} #{zip_code}" end + + # Unknown is a recorded answer, not a known one, so a flow that asks what kind of + # address this is still has to ask. + def known_type? + address_type.present? && address_type != UNKNOWN_CONTACT_TYPE + end + + private + + # Every address carries a type, so one saved without an answer says so. Rows written + # before this keep their blank until something saves them. + def type_as_unknown + self.address_type = UNKNOWN_CONTACT_TYPE if address_type.blank? + end end diff --git a/app/models/contact_method.rb b/app/models/contact_method.rb index 6bce340c21..f3ed4e0dc4 100644 --- a/app/models/contact_method.rb +++ b/app/models/contact_method.rb @@ -1,5 +1,6 @@ class ContactMethod < ApplicationRecord - CONTACT_TYPES = [ nil, "work", "personal", "unknown" ].freeze + UNKNOWN_CONTACT_TYPE = "unknown" + CONTACT_TYPES = [ nil, "work", "personal", UNKNOWN_CONTACT_TYPE ].freeze belongs_to :contactable, polymorphic: true belongs_to :created_by, class_name: "User", optional: true @@ -12,6 +13,16 @@ class ContactMethod < ApplicationRecord whatsapp: "whatsapp" } + before_validation :type_as_unknown + validates :value, presence: true, length: { maximum: 255 } validates :kind, presence: true + + private + + # Every contact method carries a type, so one saved without an answer says so. Rows + # written before this keep their blank until something saves them. + def type_as_unknown + self.contact_type = UNKNOWN_CONTACT_TYPE if contact_type.blank? + end end diff --git a/app/services/person_services/import_from_rolodex.rb b/app/services/person_services/import_from_rolodex.rb index d65d088512..658ad5baf2 100644 --- a/app/services/person_services/import_from_rolodex.rb +++ b/app/services/person_services/import_from_rolodex.rb @@ -23,7 +23,6 @@ class ImportFromRolodex "mobile" => "personal", "work" => "work", "mailing" => "mailing" }.freeze EMAIL_PATTERN = /[^\s,;<>"']+@[^\s,;<>"']+\.[^\s,;<>"']+/ UNKNOWN_LOCALITY = "Unknown" - UNKNOWN_CONTACT_TYPE = "unknown" STRING_LIMIT = 255 attr_reader :results @@ -134,10 +133,9 @@ def phone_attributes(fm_id) # The archive's types map onto one set of ours, but each model declares the slice it # accepts — an address can be the mailing one, a phone number can't. A type we can't - # place is recorded as unknown, so an imported record never reads as simply untyped. + # place is left blank, which both models then record as unknown. def contact_type(value, allowed:) - mapped = CONTACT_TYPES[value.to_s.strip.downcase] - mapped.presence_in(allowed) || UNKNOWN_CONTACT_TYPE.presence_in(allowed) + CONTACT_TYPES[value.to_s.strip.downcase].presence_in(allowed) end # The house number has its own field, sometimes already repeated at the front of diff --git a/config/features.yml b/config/features.yml index e95bfe0954..ec69aa54af 100644 --- a/config/features.yml +++ b/config/features.yml @@ -4630,3 +4630,30 @@ AWBW staff to add or change it, the same way the primary email works. pro_tips: - "Admins approve it from the change-requests queue, and it's saved right away (no confirmation email)." + +- name: "FileMaker Rolodex contacts as people" + area: people + display_status: admin_facing + released_on: 2026-10-12 + pr_number: 2630 + summary: >- + Contacts from the old FileMaker Rolodex can now be brought into the portal as + people, carrying their FileMaker code, profile details, and primary address and + phone number. + pro_tips: + - "A contact who already has a portal record is linked to it rather than duplicated — matching takes the same name and the same email." + - "Anything that can't be matched safely is listed for someone to sort out by hand instead of guessed at." + - "A person's FileMaker code on their page links straight to their row in the archive." + action_path: "/people" + +- name: "Unknown as an address and phone type" + area: people + display_status: user_facing + released_on: 2026-10-12 + pr_number: 2630 + summary: >- + An address or phone number saved without a type now reads as Unknown instead of + showing nothing, and Unknown is one of the choices in the type menu. + pro_tips: + - "Pick Work or Personal to replace Unknown whenever you know which it is." + - "Records saved before this keep their blank type until the next time they're saved." diff --git a/spec/models/address_spec.rb b/spec/models/address_spec.rb index f257b4fb70..1387858d5d 100644 --- a/spec/models/address_spec.rb +++ b/spec/models/address_spec.rb @@ -37,6 +37,30 @@ end end + describe "address type" do + it "records an untyped address as unknown on save" do + address = create(:address, address_type: nil) + + expect(address.address_type).to eq("unknown") + expect(address).not_to be_known_type + end + + it "leaves a type it was given alone" do + expect(create(:address, address_type: "mailing")).to have_attributes(address_type: "mailing", known_type?: true) + end + + it "types a legacy blank row on its next save, not before" do + address = create(:address) + address.update_columns(address_type: nil) + + expect(address.reload.address_type).to be_nil + + address.update!(city: "Missoula") + + expect(address.address_type).to eq("unknown") + end + end + describe "optional fields" do let(:address) { build(:address) } From 0d4e9cfa9f732ccf6c162655462fea6025c5de2f Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Mon, 12 Oct 2026 13:41:30 -0400 Subject: [PATCH 04/10] Match the Rolodex rows through the shared PersonMatcher rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Main's #2632/#2633 made PersonMatcher the one rule for "which person does this name + email belong to" — it reaches the secondary and login addresses a person counts as theirs, and compares names with look-alike dashes folded. The import was a new matching path with its own narrower query, so it would have created a fresh person for anyone the archive knows by their login address, reopening the gap that just closed. PersonMatcher gains #matches so a caller that has to tell one match from several can have the relation. Archive names go through Person.normalize_name before they're compared, since that's the form they'll be stored in. The merge reconcilers read unknown and blank as the same type, so a row written before an untyped address or number said so still folds into its twin. Co-Authored-By: Claude Opus 5 (1M context) --- app/services/address_deduper.rb | 7 ++++++- app/services/person_matcher.rb | 12 +++++++++--- .../person_services/import_from_rolodex.rb | 15 ++++++++------- .../person_services/reconcile_contact_methods.rb | 7 ++++++- spec/services/address_deduper_spec.rb | 10 ++++++++++ .../person_services/import_from_rolodex_spec.rb | 15 ++++++++++++++- .../reconcile_contact_methods_spec.rb | 10 ++++++++++ 7 files changed, 63 insertions(+), 13 deletions(-) diff --git a/app/services/address_deduper.rb b/app/services/address_deduper.rb index f1f3698bc5..97f900ed33 100644 --- a/app/services/address_deduper.rb +++ b/app/services/address_deduper.rb @@ -20,7 +20,12 @@ class AddressDeduper def self.signature(address) return unless address - IDENTITY_COLUMNS.map { |column| address.public_send(column).to_s.strip.downcase } + IDENTITY_COLUMNS.map do |column| + value = address.public_send(column).to_s.strip.downcase + # A row written before an untyped address said so holds a blank, so unknown and + # blank have to read alike or an otherwise identical pair won't fold. + value == Address::UNKNOWN_CONTACT_TYPE && column == "address_type" ? "" : value + end end # The signature of the primary address to preserve, read before the merge while diff --git a/app/services/person_matcher.rb b/app/services/person_matcher.rb index 102ffae028..51675f8fb7 100644 --- a/app/services/person_matcher.rb +++ b/app/services/person_matcher.rb @@ -14,8 +14,14 @@ def initialize(email:, last_name:, first_names: nil) end def call - return if @email.blank? || @last_name.blank? - return if @first_names&.empty? + matches.first + end + + # Everyone the name + email reaches, for a caller that has to tell one match from + # several rather than take the first. + def matches + return Person.none if @email.blank? || @last_name.blank? + return Person.none if @first_names&.empty? scope = Person.left_joins(:user) .where("LOWER(people.last_name) = ?", @last_name) @@ -24,6 +30,6 @@ def call "LOWER(people.first_name) IN (:names) OR LOWER(COALESCE(people.legal_first_name, '')) IN (:names)", names: @first_names ) if @first_names - scope.first + scope end end diff --git a/app/services/person_services/import_from_rolodex.rb b/app/services/person_services/import_from_rolodex.rb index 658ad5baf2..96eed10695 100644 --- a/app/services/person_services/import_from_rolodex.rb +++ b/app/services/person_services/import_from_rolodex.rb @@ -2,8 +2,9 @@ module PersonServices # Turns FileMaker Rolodex archive rows (FmRolodex) into people. Each row lands in # one outcome bucket of #results: # * created — new Person, with the row's primary address and phone - # * linked — an existing person with the same name + email gets the - # row's ID as their filemaker_code + # * linked — the person PersonMatcher finds for the row's name + email — + # which reaches their secondary and login addresses too — gets + # the row's ID as their filemaker_code # * already_imported — a person already carries the row's ID # * ambiguous — a name or email collides with someone we can't safely # claim is the same person; reconcile by hand @@ -42,8 +43,8 @@ def call def import(rolodex) data = rolodex.data - first_name = data["FirstName"].to_s.strip - last_name = data["LastName"].to_s.strip + first_name = Person.normalize_name(data["FirstName"]).to_s + last_name = Person.normalize_name(data["LastName"]).to_s label = "#{rolodex.fm_id} #{first_name} #{last_name}".squish return record(:already_imported, label) if @imported_codes.include?(rolodex.fm_id) @@ -52,17 +53,17 @@ def import(rolodex) return record(:skipped, "#{label}: missing first or last name") if first_name.blank? || last_name.blank? email, email_2 = data["EMail"].to_s.scan(EMAIL_PATTERN) - same_name = Person.where("LOWER(first_name) = ? AND LOWER(last_name) = ?", first_name.downcase, last_name.downcase) if email - match = same_name.where("LOWER(email) = :email OR LOWER(email_2) = :email", email: email.downcase).to_a + match = PersonMatcher.new(email:, last_name:, first_names: [ first_name ]).matches.to_a return link(match.first, rolodex, label) if match.one? return record(:ambiguous, "#{label}: #{match.size} people share this name and email (#{ids(match)})") if match.many? others = Person.where("LOWER(email) = :email OR LOWER(email_2) = :email", email: email.downcase).to_a return record(:ambiguous, "#{label}: #{email} belongs to #{others.map { |p| "##{p.id} #{p.full_name}" }.join(', ')}") if others.any? else - no_email = same_name.where(email: [ nil, "" ]).to_a + no_email = Person.where("LOWER(first_name) = ? AND LOWER(last_name) = ?", first_name.downcase, last_name.downcase) + .where(email: [ nil, "" ]).to_a return record(:ambiguous, "#{label}: no email, same name as #{ids(no_email)}") if no_email.any? end diff --git a/app/services/person_services/reconcile_contact_methods.rb b/app/services/person_services/reconcile_contact_methods.rb index 4a1889060c..37fdc2e2d0 100644 --- a/app/services/person_services/reconcile_contact_methods.rb +++ b/app/services/person_services/reconcile_contact_methods.rb @@ -14,7 +14,12 @@ class ReconcileContactMethods def self.signature(contact_method) return unless contact_method - IDENTITY_COLUMNS.map { |column| contact_method.public_send(column).to_s.strip.downcase } + IDENTITY_COLUMNS.map do |column| + value = contact_method.public_send(column).to_s.strip.downcase + # A row written before an untyped number said so holds a blank, so unknown and + # blank have to read alike or an otherwise identical pair won't fold. + value == ContactMethod::UNKNOWN_CONTACT_TYPE && column == "contact_type" ? "" : value + end end # One preserved-primary signature per kind, read before the merge while the two diff --git a/spec/services/address_deduper_spec.rb b/spec/services/address_deduper_spec.rb index b730db9981..d14791c962 100644 --- a/spec/services/address_deduper_spec.rb +++ b/spec/services/address_deduper_spec.rb @@ -24,6 +24,16 @@ def address(primary: false, **overrides) expect(Address.exists?(dup.id)).to be false end + it "folds a legacy untyped address into one that now says unknown" do + keep = address(address_type: Address::UNKNOWN_CONTACT_TYPE) + legacy = address(address_type: Address::UNKNOWN_CONTACT_TYPE) + legacy.update_columns(address_type: nil) + + described_class.new(person).call + + expect(person.addresses.reload.pluck(:id)).to eq([ keep.id ]) + end + it "keeps addresses that differ in any detail" do address address(street_address: "2 Other St") diff --git a/spec/services/person_services/import_from_rolodex_spec.rb b/spec/services/person_services/import_from_rolodex_spec.rb index ebf5d89b6b..9eaefe1e27 100644 --- a/spec/services/person_services/import_from_rolodex_spec.rb +++ b/spec/services/person_services/import_from_rolodex_spec.rb @@ -112,6 +112,17 @@ def results expect(person.reload.filemaker_code).to eq("00008") end + it "links a person the archive knows by their login email" do + person = create(:person, first_name: "Ada", last_name: "Lovelace", email: "ada.l@example.com") + # Bypassing the sync leaves the login address the person's own columns never got, + # which is the legacy shape PersonMatcher reaches. + person.user.update_columns(email: "ada@example.com") + rolodex("00040", EMail: "ada@example.com") + + expect { results }.not_to change(Person, :count) + expect(person.reload.filemaker_code).to eq("00040") + end + it "leaves rows whose ID a person already carries alone" do create(:person, filemaker_code: "00009") rolodex("00009") @@ -128,7 +139,9 @@ def results end it "flags a same-name match with no email to confirm it" do - create(:person, first_name: "Ada", last_name: "Lovelace", email: nil) + # A linked login would become their primary email (PersonLoginEmailSync), so the + # person this guards against is one with no account. + create(:person, first_name: "Ada", last_name: "Lovelace", email: nil, user: nil) rolodex("00011") expect { expect(results[:ambiguous].size).to eq(1) }.not_to change(Person, :count) diff --git a/spec/services/person_services/reconcile_contact_methods_spec.rb b/spec/services/person_services/reconcile_contact_methods_spec.rb index 51a09af7f1..185c4a3fe9 100644 --- a/spec/services/person_services/reconcile_contact_methods_spec.rb +++ b/spec/services/person_services/reconcile_contact_methods_spec.rb @@ -24,6 +24,16 @@ def address expect(ContactMethod.exists?(dup.id)).to be false end + it "folds a legacy untyped number into one that now says unknown" do + keep = contact_method + legacy = contact_method + legacy.update_columns(contact_type: nil) + + described_class.new(person).call + + expect(person.contact_methods.reload.pluck(:id)).to eq([ keep.id ]) + end + it "keeps contact methods that differ in value or kind" do contact_method(value: "555-1000") contact_method(value: "555-2000") From f52c00aa9ce699e52cddc68fb95fc1651cfb7593 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Mon, 12 Oct 2026 15:01:57 -0400 Subject: [PATCH 05/10] Read stored FileMaker codes as the list they can now be #2647 lets a person carry several codes, so an index built from whole column values would miss the row whose ID sits inside one and import it again. Co-Authored-By: Claude Opus 5 (1M context) --- app/services/person_services/import_from_rolodex.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/services/person_services/import_from_rolodex.rb b/app/services/person_services/import_from_rolodex.rb index 96eed10695..b336eb542f 100644 --- a/app/services/person_services/import_from_rolodex.rb +++ b/app/services/person_services/import_from_rolodex.rb @@ -31,7 +31,8 @@ class ImportFromRolodex def initialize(scope: FmRolodex.all) @scope = scope @results = OUTCOMES.index_with { [] } - @imported_codes = Person.where.not(filemaker_code: [ nil, "" ]).pluck(:filemaker_code).map(&:strip).to_set + @imported_codes = Person.where.not(filemaker_code: [ nil, "" ]).pluck(:filemaker_code) + .flat_map { |value| Person.split_filemaker_codes(value) }.to_set end def call From 9e27cb7751bffa491333f0c6b283a00a79b33aa2 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Wed, 14 Oct 2026 20:33:55 -0400 Subject: [PATCH 06/10] Carry the rest of what the archive knows about a person MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FmRolodex::PORTAL_FIELDS documents which archive field answers to which portal column; the import only wrote half of them. It now also carries the email type, the facilitator start month/year as member_since, the service areas and age group (through SubmissionTagging, the same seam a submission uses, additively), and the archive's own entry and modification dates — so an imported person dates from when FileMaker first knew them rather than from the import run. Addresses and phones come from FmRolodex#addresses / #phone_numbers rather than the single PrimaryAddrsID/PrimaryPhoneID, so a contact's other addresses arrive too, the row's primary first. A service area no sector answers to is counted in #unmatched_tags and printed by the task, since "Child Abuse" and "Child Abuse/Neglect" are the same sector but "Homelessness" with no seeded sector is a gap someone has to close. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 2 +- .../person_services/import_from_rolodex.rb | 103 +++++++++++++++--- config/features.yml | 5 +- lib/tasks/fm_import_people.rake | 5 + .../import_from_rolodex_spec.rb | 67 ++++++++++++ 5 files changed, 163 insertions(+), 19 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 6ad1ed218c..584621a53a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -280,7 +280,7 @@ action, or `authorize! :workshop, to: :summary?`). - `NewsSubscriptionCapture` — Successor to the retired person-level mailing-list consent flag: given a `person` + `source`, idempotently creates an active **News** `TopicSubscription` (skips when one is already active, no-op when the News topic isn't seeded). Called from `PublicRegistration` (source = event) and `PublicFormSubmission` (source = form) when the `communication_consent` question is answered - `OrganizationServices::CaptureFromSubmission` — The shared org-capture for **both** the event registration (`PublicRegistration`) and standalone (`PublicFormSubmission`) flows: resolves the submitted organization by name (`FormField::ORGANIZATION_NAME_FIELD_IDENTIFIER`, case-insensitive per the column collation; never creates one — an unmatched name is left for an admin), syncs its website/type (`SyncProfile`, latest-wins) and work address (`UpsertAddress`, which adds another work address rather than rewriting a different one). `AffiliationServices::CreateFromRegistration` then mints the person's job affiliation from the typed position, plus the standing Facilitator one when `facilitator_training:` — which only an event supplies (`@event.facilitator_training`), so a standalone form gets the job affiliation alone, the same as registering for a non-training event. **Agreement-role submissions never come through here** — they run `LinkSubmittedOrganization`, which owns scenario end-dating and the new-job start date. Returns the org + the autofill changes, which the event flow records on its registration↔org link - `PersonServices::CaptureFromSubmission` — The shared person-capture for **both** the event registration (`PublicRegistration`) and standalone (`PublicFormSubmission`) flows, run for a newly-created **and** an existing person, so the person smart fields on the Smart form settings page behave identically across both forms. Fills identity extras (pronouns, name `pronunciation`, `nickname` → first name / legal first name, primary email type, secondary email + type), the profile (`racial_ethnic_identity` + `SyncSharingPreferences`), the mailing address, the phone contact method, and sector/age tags (mirrored onto any organizations passed in). A non-blank answer overwrites what's on file (newest submission wins); a blank never clobbers. Sector and age tags both run through `SubmissionTagging` and follow one rule on the **person**: submitting both a primary and additional set **overwrites** the person's tags with the latest (`replace: true`), submitting only a primary **reassigns** the primary and leaves the rest (additive); the org mirror always stays additive (orgs aggregate their members' tags). Self-gates on each field identifier, so a form that doesn't ask a question no-ops for it -- `PersonServices::ImportFromRolodex` — Creates people from FileMaker Rolodex archive rows (`FmRolodex`), carrying the row's ID as `filemaker_code` plus profile fields and the primary address/phone. Links an existing person on an exact name + email match; flags email-only, same-name-no-email, and already-linked collisions as ambiguous for hand review; skips agencies, soft-deleted, and nameless rows. Idempotent on `filemaker_code`. Driven by `fm:import_people` +- `PersonServices::ImportFromRolodex` — Creates people from FileMaker Rolodex archive rows (`FmRolodex`), carrying the row's ID as `filemaker_code` plus every field `FmRolodex::PORTAL_FIELDS` maps onto a person (profile fields, email type, facilitator start → `member_since`, service areas → sectors and `PrimaryAgeGroup` → age group via `SubmissionTagging`, and the archive's own entry/modification dates as `created_at`/`updated_at`), plus every address and phone the row answers to (`FmRolodex#addresses` / `#phone_numbers`, primary first). Links an existing person through `PersonMatcher`; flags email-only, same-name-no-email, and already-linked collisions as ambiguous for hand review; skips agencies, soft-deleted, and nameless rows. Counts service areas no sector answers to in `#unmatched_tags`. Idempotent on `filemaker_code`. Driven by `fm:import_people` - `PersonServices::SyncSharingPreferences` — Writes the two content-sharing answers (`anonymous_contributions`, `display_name_preference`) onto the `Person`, mapping each submitted option label back to its stored value; an unanswered or unrecognized answer leaves the profile alone, so a form that doesn't ask the questions is a no-op and every flow can call it unconditionally. The person-side counterpart to `OrganizationServices::SyncProfile`. Shared by `PersonServices::CaptureFromSubmission` (so a submission's answers land in the "what this submission changed" audit via the Ahoy trail) and `CalloutFormSubmission` (whose `#profile_changes` re-reports the same edits for its own Ahoy tracking) - `EventRegistrationServices::TransferContinuingEducation` — Splits/relocates a registrant's CE when they transfer events (issue #1944): a simple forward transfer leaves a paid, zero-hours **stub** on the source (its payments count at the original event) and creates a **live** record on the destination carrying the hours and the outstanding balance; when the reg being transferred out is itself a transfer-in (a collapsing double transfer, or a transfer back to the origin) its live record is relocated forward — merging back into the origin's stub — instead of split again, so no third record appears. Runs inside the transfer transaction, after the destination is saved and before a collapsing middle reg is destroyed - `EventRegistrationServices::ReconcileMergedContinuingEducation` — Run from the event-registration deduper's `after_merge`. A CE registration moved from the deleted registration still points at that (deleted) registrant's license — invalid, since the registration now belongs to the kept registrant — so this repoints each to the kept registrant's equivalent license (same kind + number, found or created), then folds the resulting duplicates into one with `ContinuingEducationDeduper` so payments are preserved diff --git a/app/services/person_services/import_from_rolodex.rb b/app/services/person_services/import_from_rolodex.rb index b336eb542f..400500fb3c 100644 --- a/app/services/person_services/import_from_rolodex.rb +++ b/app/services/person_services/import_from_rolodex.rb @@ -1,7 +1,9 @@ module PersonServices # Turns FileMaker Rolodex archive rows (FmRolodex) into people. Each row lands in # one outcome bucket of #results: - # * created — new Person, with the row's primary address and phone + # * created — new Person: the fields FmRolodex::PORTAL_FIELDS maps to a + # person (including its sector/age tags and the archive's own + # entry date), plus the row's primary address and phone # * linked — the person PersonMatcher finds for the row's name + email — # which reaches their secondary and login addresses too — gets # the row's ID as their filemaker_code @@ -26,13 +28,16 @@ class ImportFromRolodex UNKNOWN_LOCALITY = "Unknown" STRING_LIMIT = 255 - attr_reader :results + attr_reader :results, :unmatched_tags def initialize(scope: FmRolodex.all) @scope = scope @results = OUTCOMES.index_with { [] } @imported_codes = Person.where.not(filemaker_code: [ nil, "" ]).pluck(:filemaker_code) .flat_map { |value| Person.split_filemaker_codes(value) }.to_set + @sectors_by_name = name_index(Sector.all) + @age_groups_by_name = name_index(Category.age_ranges) + @unmatched_tags = Hash.new(0) end def call @@ -91,23 +96,91 @@ def create(rolodex, label, **names) best_time_to_call: clip(data["BestTimeToCall"]), racial_ethnic_identity: clip(data["Ethnicity"]), date_of_birth: parse_date(data["DateOfBirth"]), - notes: data["Comments"].presence + email_type: email_type(data["EmailType"]), + member_since: member_since(data), + notes: data["Comments"].presence, + **archive_timestamps(data) ) - address = address_attributes(data["PrimaryAddrsID"]) - person.addresses.build(address) if address - phone = phone_attributes(data["PrimaryPhoneID"]) - person.contact_methods.build(phone) if phone + build_addresses(person, rolodex) + build_phones(person, rolodex) return record(:failed, "#{label}: #{person.errors.full_messages.to_sentence}") unless person.save + tag_service_areas(person, data) + # Tagging touches the person, so the archive's dates are written once more here — + # update_columns, so the touch doesn't win. + timestamps = archive_timestamps(data) + person.update_columns(timestamps) if timestamps.any? @imported_codes << rolodex.fm_id record(:created, "#{label} → ##{person.id}") end - def address_attributes(fm_id) - fields = fm_id.presence && FmAddress.find_by(fm_id:)&.data - return unless fields + # The archive's own entry and modification dates, so an imported person dates from + # when FileMaker first knew them rather than from the import run. + def archive_timestamps(data) + entered = parse_date(data["DateEntered"]) + { created_at: entered, updated_at: parse_date(data["DateTimeModified"]) || entered }.compact + end + + def email_type(value) + CONTACT_TYPES[value.to_s.strip.downcase].presence_in(Person::CONTACT_TYPES) + end + + # FileMaker keeps the facilitator's start as a separate month and year; the portal + # keeps one date, and a year with no month reads as January. + def member_since(data) + year = data["FacilitatorSinceYear"].to_s[/\A\d{4}\z/] + return unless year + + Date.new(year.to_i, month_number(data["FacilitatorSinceMonth"]) || 1, 1) + end + + def month_number(value) + value = value.to_s.strip + return value.to_i if value.match?(/\A(0?[1-9]|1[0-2])\z/) + + [ Date::MONTHNAMES, Date::ABBR_MONTHNAMES ].filter_map { |names| names.index { |name| name&.casecmp?(value) } }.first + end + + # The archive's service areas and age group tag the person through the same seam a + # submission uses — additive, since the import is never the whole picture. + def tag_service_areas(person, data) + SubmissionTagging.apply(person:, organizations: [], tag_with: :tag_sectors, + primary_ids: tag_ids(data["PrimaryServiceArea"], @sectors_by_name).first(1), + additional_ids: tag_ids(data["OtherServiceArea"], @sectors_by_name)) + SubmissionTagging.apply(person:, organizations: [], tag_with: :tag_age_groups, + primary_ids: tag_ids(data["PrimaryAgeGroup"], @age_groups_by_name)) + end + + # One field can list several areas. Each resolves by name, then by the portal's + # qualified form of it ("Child Abuse" → "Child Abuse/Neglect"); a name nothing + # answers to is counted in #unmatched_tags rather than guessed at. + def tag_ids(value, index) + value.to_s.split(/[,;]/).map { |name| name.strip.downcase }.reject(&:empty?).uniq.filter_map do |name| + id = index[name] || index.find { |stored, _| stored.start_with?("#{name}/") }&.last + @unmatched_tags[name] += 1 unless id + id + end + end + + def name_index(scope) + scope.pluck(:name, :id).to_h { |name, id| [ name.to_s.strip.downcase, id ] } + end + + # Every address the row answers to, not just the one it names as primary — + # FmRolodex#addresses lists them with the primary and worksite ones first, so the + # first that can be built is the person's primary and the rest ride along. + def build_addresses(person, rolodex) + attributes = rolodex.addresses.filter_map { |address| address_attributes(address.data) } + attributes.each_with_index { |attrs, index| person.addresses.build(attrs.merge(primary: index.zero?)) } + end + + def build_phones(person, rolodex) + attributes = rolodex.phone_numbers.filter_map { |phone| phone_attributes(phone.data) } + attributes.each_with_index { |attrs, index| person.contact_methods.build(attrs.merge(primary: index.zero?)) } + end + def address_attributes(fields) street = [ street_line(fields), fields["AddrsLine2"] ].filter_map { |line| line.to_s.strip.presence }.join(", ") city = fields["AddrsCity"].to_s.strip state = fields["AddrsState"].to_s.strip @@ -120,17 +193,15 @@ def address_attributes(fm_id) zip_code: fields["AddrsPostalCode"].to_s.strip, country: clip(fields["AddrsCountry"]), address_type: contact_type(fields["AddrsType"], allowed: Address::CONTACT_TYPES), - locality: UNKNOWN_LOCALITY, - primary: true + locality: UNKNOWN_LOCALITY } end - def phone_attributes(fm_id) - fields = fm_id.presence && FmPhoneNumber.find_by(fm_id:)&.data - number = fields && fields["PhoneNumber"].to_s.strip + def phone_attributes(fields) + number = clip(fields["PhoneNumber"]) return if number.blank? - { kind: "phone", value: number, contact_type: contact_type(fields["PhoneType"], allowed: ContactMethod::CONTACT_TYPES), primary: true } + { kind: "phone", value: number, contact_type: contact_type(fields["PhoneType"], allowed: ContactMethod::CONTACT_TYPES) } end # The archive's types map onto one set of ours, but each model declares the slice it diff --git a/config/features.yml b/config/features.yml index ec69aa54af..12514891db 100644 --- a/config/features.yml +++ b/config/features.yml @@ -4638,12 +4638,13 @@ pr_number: 2630 summary: >- Contacts from the old FileMaker Rolodex can now be brought into the portal as - people, carrying their FileMaker code, profile details, and primary address and - phone number. + people, carrying their FileMaker code, profile details, service areas and age + group, and every address and phone number the old record held. pro_tips: - "A contact who already has a portal record is linked to it rather than duplicated — matching takes the same name and the same email." - "Anything that can't be matched safely is listed for someone to sort out by hand instead of guessed at." - "A person's FileMaker code on their page links straight to their row in the archive." + - "An imported person carries their service areas, age group, facilitator start date, and every address and phone the old record held — and their profile dates from when FileMaker first knew them, not from the day they were imported." action_path: "/people" - name: "Unknown as an address and phone type" diff --git a/lib/tasks/fm_import_people.rake b/lib/tasks/fm_import_people.rake index a5ce188e88..69b1be3f27 100644 --- a/lib/tasks/fm_import_people.rake +++ b/lib/tasks/fm_import_people.rake @@ -27,6 +27,11 @@ namespace :fm do messages.each { |message| puts " #{message}" } end + if import.unmatched_tags.any? + puts "\nService areas and age groups no sector or category answers to:" + import.unmatched_tags.sort_by { |name, count| [ -count, name ] }.each { |name, count| puts " #{count.to_s.rjust(5)} #{name}" } + end + puts import.results.each { |outcome, messages| puts "#{outcome.to_s.humanize.ljust(17)} #{messages.size}" } puts(dry_run ? "\nDRY RUN — rolled back, nothing written. Set DRY_RUN=false to execute." : "\nDone.") diff --git a/spec/services/person_services/import_from_rolodex_spec.rb b/spec/services/person_services/import_from_rolodex_spec.rb index 9eaefe1e27..5b7eece9b2 100644 --- a/spec/services/person_services/import_from_rolodex_spec.rb +++ b/spec/services/person_services/import_from_rolodex_spec.rb @@ -36,6 +36,55 @@ def results expect(Person.find_by!(filemaker_code: "00007")).to have_attributes(email: "ada@example.com", email_2: "ada@work.example.com") end + describe "the fields FmRolodex::PORTAL_FIELDS maps onto a person" do + let!(:age_range_type) { create(:category_type, name: "AgeRange", published: true) } + let!(:adults) { create(:category, category_type: age_range_type, name: "Adults") } + let!(:domestic_violence) { create(:sector, name: "Domestic Violence") } + let!(:child_abuse) { create(:sector, name: "Child Abuse/Neglect") } + + it "carries the email type, facilitator start, service areas, age group, and entry date" do + rolodex("00050", EMail: "ada@example.com", EmailType: "Work", FacilitatorSinceMonth: "3", + FacilitatorSinceYear: "2011", PrimaryServiceArea: "Domestic Violence", + OtherServiceArea: "Child Abuse; Homelessness", PrimaryAgeGroup: "Adults", + DateEntered: "03/14/2011", DateTimeModified: "09/02/2014 10:22:31") + + import = described_class.new.call + person = Person.find_by!(filemaker_code: "00050") + + expect(person).to have_attributes(email_type: "work", member_since: Date.new(2011, 3, 1)) + expect(person.created_at.to_date).to eq(Date.new(2011, 3, 14)) + expect(person.updated_at.to_date).to eq(Date.new(2014, 9, 2)) + expect(person.primary_sector).to eq(domestic_violence) + expect(person.sectorable_items.map(&:sector)).to contain_exactly(domestic_violence, child_abuse) + expect(person.primary_age_groups).to eq([ adults ]) + expect(import.unmatched_tags).to eq({ "homelessness" => 1 }) + end + + it "reads a facilitator year with no month as January, and a month name" do + rolodex("00051", FirstName: "Grace", EMail: "grace@example.com", FacilitatorSinceYear: "2018") + rolodex("00052", FirstName: "Hedy", EMail: "hedy@example.com", FacilitatorSinceMonth: "June", + FacilitatorSinceYear: "2014") + + results + + expect(Person.find_by!(filemaker_code: "00051").member_since).to eq(Date.new(2018, 1, 1)) + expect(Person.find_by!(filemaker_code: "00052").member_since).to eq(Date.new(2014, 6, 1)) + end + + it "leaves the fields alone when the archive row doesn't carry them" do + rolodex("00053", EMail: "ada@example.com") + + import = described_class.new.call + person = Person.find_by!(filemaker_code: "00053") + + expect(person).to have_attributes(email_type: nil, member_since: nil) + expect(person.sectorable_items).to be_empty + expect(person.categorizable_items).to be_empty + expect(person.created_at).to be_within(1.minute).of(Time.current) + expect(import.unmatched_tags).to be_empty + end + end + it "prefixes the street number when the first line doesn't already carry it" do FmAddress.create!(fm_id: "A2", data: { "StreetNo" => "1234", "AddrsLine1" => "Main St", "AddrsCity" => "Los Angeles", "AddrsState" => "CA" }) @@ -50,6 +99,24 @@ def results expect(Person.find_by!(filemaker_code: "00031").addresses.sole.street_address).to eq("1234 Main St") end + it "brings in every address and phone the row answers to, primary first" do + FmAddress.create!(fm_id: "A20", data: { "RolodexID" => "00060", "AddrsLine1" => "2 Second St", + "AddrsCity" => "Oakland", "AddrsState" => "CA" }) + FmAddress.create!(fm_id: "A21", data: { "AddrsLine1" => "1 Main St", "AddrsCity" => "Los Angeles", + "AddrsState" => "CA" }) + FmPhoneNumber.create!(fm_id: "P20", data: { "RolodexID" => "00060", "PhoneNumber" => "555-0200" }) + FmPhoneNumber.create!(fm_id: "P21", data: { "PhoneNumber" => "555-0100" }) + rolodex("00060", EMail: "ada@example.com", PrimaryAddrsID: "A21", PrimaryPhoneID: "P21") + + results + + person = Person.find_by!(filemaker_code: "00060") + expect(person.addresses.order(:id).pluck(:street_address, :primary)) + .to eq([ [ "1 Main St", true ], [ "2 Second St", false ] ]) + expect(person.contact_methods.order(:id).pluck(:value, :primary)) + .to eq([ [ "555-0100", true ], [ "555-0200", false ] ]) + end + it "truncates over-long free text instead of losing the person to it" do FmAddress.create!(fm_id: "A4", data: { "AddrsLine1" => "x" * 200, "AddrsLine2" => "y" * 200, "AddrsCity" => "Los Angeles", "AddrsState" => "CA" }) From 384c40db8a12e555fd2933cc7278655fa25aafca Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Thu, 15 Oct 2026 08:56:23 -0400 Subject: [PATCH 07/10] Link Rolodex collisions instead of parking them for review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A person can hold several FileMaker codes now, so a row that matches several people, someone already linked to another code, or an email-less same-name person is the same person — adding the code to their list turns three hand- review buckets into imports. Only an email owned by a differently named person stays ambiguous, since that one may really be someone else. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 +- .../person_services/import_from_rolodex.rb | 30 ++++++++++--------- config/features.yml | 4 ++- .../import_from_rolodex_spec.rb | 24 ++++++++++----- 4 files changed, 37 insertions(+), 23 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 584621a53a..d54ce1c940 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -280,7 +280,7 @@ action, or `authorize! :workshop, to: :summary?`). - `NewsSubscriptionCapture` — Successor to the retired person-level mailing-list consent flag: given a `person` + `source`, idempotently creates an active **News** `TopicSubscription` (skips when one is already active, no-op when the News topic isn't seeded). Called from `PublicRegistration` (source = event) and `PublicFormSubmission` (source = form) when the `communication_consent` question is answered - `OrganizationServices::CaptureFromSubmission` — The shared org-capture for **both** the event registration (`PublicRegistration`) and standalone (`PublicFormSubmission`) flows: resolves the submitted organization by name (`FormField::ORGANIZATION_NAME_FIELD_IDENTIFIER`, case-insensitive per the column collation; never creates one — an unmatched name is left for an admin), syncs its website/type (`SyncProfile`, latest-wins) and work address (`UpsertAddress`, which adds another work address rather than rewriting a different one). `AffiliationServices::CreateFromRegistration` then mints the person's job affiliation from the typed position, plus the standing Facilitator one when `facilitator_training:` — which only an event supplies (`@event.facilitator_training`), so a standalone form gets the job affiliation alone, the same as registering for a non-training event. **Agreement-role submissions never come through here** — they run `LinkSubmittedOrganization`, which owns scenario end-dating and the new-job start date. Returns the org + the autofill changes, which the event flow records on its registration↔org link - `PersonServices::CaptureFromSubmission` — The shared person-capture for **both** the event registration (`PublicRegistration`) and standalone (`PublicFormSubmission`) flows, run for a newly-created **and** an existing person, so the person smart fields on the Smart form settings page behave identically across both forms. Fills identity extras (pronouns, name `pronunciation`, `nickname` → first name / legal first name, primary email type, secondary email + type), the profile (`racial_ethnic_identity` + `SyncSharingPreferences`), the mailing address, the phone contact method, and sector/age tags (mirrored onto any organizations passed in). A non-blank answer overwrites what's on file (newest submission wins); a blank never clobbers. Sector and age tags both run through `SubmissionTagging` and follow one rule on the **person**: submitting both a primary and additional set **overwrites** the person's tags with the latest (`replace: true`), submitting only a primary **reassigns** the primary and leaves the rest (additive); the org mirror always stays additive (orgs aggregate their members' tags). Self-gates on each field identifier, so a form that doesn't ask a question no-ops for it -- `PersonServices::ImportFromRolodex` — Creates people from FileMaker Rolodex archive rows (`FmRolodex`), carrying the row's ID as `filemaker_code` plus every field `FmRolodex::PORTAL_FIELDS` maps onto a person (profile fields, email type, facilitator start → `member_since`, service areas → sectors and `PrimaryAgeGroup` → age group via `SubmissionTagging`, and the archive's own entry/modification dates as `created_at`/`updated_at`), plus every address and phone the row answers to (`FmRolodex#addresses` / `#phone_numbers`, primary first). Links an existing person through `PersonMatcher`; flags email-only, same-name-no-email, and already-linked collisions as ambiguous for hand review; skips agencies, soft-deleted, and nameless rows. Counts service areas no sector answers to in `#unmatched_tags`. Idempotent on `filemaker_code`. Driven by `fm:import_people` +- `PersonServices::ImportFromRolodex` — Creates people from FileMaker Rolodex archive rows (`FmRolodex`), carrying the row's ID as `filemaker_code` plus every field `FmRolodex::PORTAL_FIELDS` maps onto a person (profile fields, email type, facilitator start → `member_since`, service areas → sectors and `PrimaryAgeGroup` → age group via `SubmissionTagging`, and the archive's own entry/modification dates as `created_at`/`updated_at`), plus every address and phone the row answers to (`FmRolodex#addresses` / `#phone_numbers`, primary first). Links an existing person through `PersonMatcher` (or, for an email-less row, a same-name person with no email) by adding the row's ID to their `filemaker_code` list — every match when several answer, and alongside any code they already carry; flags only an email belonging to a differently named person as ambiguous for hand review; skips agencies, soft-deleted, and nameless rows. Counts service areas no sector answers to in `#unmatched_tags`. Idempotent on `filemaker_code`. Driven by `fm:import_people` - `PersonServices::SyncSharingPreferences` — Writes the two content-sharing answers (`anonymous_contributions`, `display_name_preference`) onto the `Person`, mapping each submitted option label back to its stored value; an unanswered or unrecognized answer leaves the profile alone, so a form that doesn't ask the questions is a no-op and every flow can call it unconditionally. The person-side counterpart to `OrganizationServices::SyncProfile`. Shared by `PersonServices::CaptureFromSubmission` (so a submission's answers land in the "what this submission changed" audit via the Ahoy trail) and `CalloutFormSubmission` (whose `#profile_changes` re-reports the same edits for its own Ahoy tracking) - `EventRegistrationServices::TransferContinuingEducation` — Splits/relocates a registrant's CE when they transfer events (issue #1944): a simple forward transfer leaves a paid, zero-hours **stub** on the source (its payments count at the original event) and creates a **live** record on the destination carrying the hours and the outstanding balance; when the reg being transferred out is itself a transfer-in (a collapsing double transfer, or a transfer back to the origin) its live record is relocated forward — merging back into the origin's stub — instead of split again, so no third record appears. Runs inside the transfer transaction, after the destination is saved and before a collapsing middle reg is destroyed - `EventRegistrationServices::ReconcileMergedContinuingEducation` — Run from the event-registration deduper's `after_merge`. A CE registration moved from the deleted registration still points at that (deleted) registrant's license — invalid, since the registration now belongs to the kept registrant — so this repoints each to the kept registrant's equivalent license (same kind + number, found or created), then folds the resulting duplicates into one with `ContinuingEducationDeduper` so payments are preserved diff --git a/app/services/person_services/import_from_rolodex.rb b/app/services/person_services/import_from_rolodex.rb index 400500fb3c..32d74e93e0 100644 --- a/app/services/person_services/import_from_rolodex.rb +++ b/app/services/person_services/import_from_rolodex.rb @@ -4,12 +4,13 @@ module PersonServices # * created — new Person: the fields FmRolodex::PORTAL_FIELDS maps to a # person (including its sector/age tags and the archive's own # entry date), plus the row's primary address and phone - # * linked — the person PersonMatcher finds for the row's name + email — - # which reaches their secondary and login addresses too — gets - # the row's ID as their filemaker_code + # * linked — every person PersonMatcher finds for the row's name + email + # (which reaches their secondary and login addresses too), or + # every same-name person with no email when the row has none, + # adds the row's ID to their filemaker_code list # * already_imported — a person already carries the row's ID - # * ambiguous — a name or email collides with someone we can't safely - # claim is the same person; reconcile by hand + # * ambiguous — the email belongs to a differently named person; reconcile + # by hand # * skipped — agency, soft-deleted, or nameless rows # * failed — the Person didn't validate # @@ -62,29 +63,30 @@ def import(rolodex) if email match = PersonMatcher.new(email:, last_name:, first_names: [ first_name ]).matches.to_a - return link(match.first, rolodex, label) if match.one? - return record(:ambiguous, "#{label}: #{match.size} people share this name and email (#{ids(match)})") if match.many? + return link(match, rolodex, label) if match.any? others = Person.where("LOWER(email) = :email OR LOWER(email_2) = :email", email: email.downcase).to_a return record(:ambiguous, "#{label}: #{email} belongs to #{others.map { |p| "##{p.id} #{p.full_name}" }.join(', ')}") if others.any? else no_email = Person.where("LOWER(first_name) = ? AND LOWER(last_name) = ?", first_name.downcase, last_name.downcase) .where(email: [ nil, "" ]).to_a - return record(:ambiguous, "#{label}: no email, same name as #{ids(no_email)}") if no_email.any? + return link(no_email, rolodex, label) if no_email.any? end create(rolodex, label, first_name:, last_name:, email:, email_2:) end - def link(person, rolodex, label) - if person.filemaker_code.present? - return record(:ambiguous, "#{label}: matches ##{person.id}, already linked to FileMaker #{person.filemaker_code}") - end + # Several matches are one person entered more than once, so each carries the code + # and the deduper unions them when they're merged. + def link(people, rolodex, label) + people.each do |person| + next if person.update(filemaker_code: Person.join_filemaker_codes(person.filemaker_code, rolodex.fm_id)) - return record(:failed, "#{label}: ##{person.id} #{person.errors.full_messages.to_sentence}") unless person.update(filemaker_code: rolodex.fm_id) + return record(:failed, "#{label}: ##{person.id} #{person.errors.full_messages.to_sentence}") + end @imported_codes << rolodex.fm_id - record(:linked, "#{label} → ##{person.id}") + record(:linked, "#{label} → #{ids(people)}") end def create(rolodex, label, **names) diff --git a/config/features.yml b/config/features.yml index 12514891db..78e549a55f 100644 --- a/config/features.yml +++ b/config/features.yml @@ -4642,7 +4642,9 @@ group, and every address and phone number the old record held. pro_tips: - "A contact who already has a portal record is linked to it rather than duplicated — matching takes the same name and the same email." - - "Anything that can't be matched safely is listed for someone to sort out by hand instead of guessed at." + - "A contact with no email is linked to a portal person of the same name who also has no email." + - "A person can carry several FileMaker codes, so a contact who matches someone already linked to another FileMaker record is added to their list." + - "Only an email that belongs to a differently named person is listed for someone to sort out by hand instead of guessed at." - "A person's FileMaker code on their page links straight to their row in the archive." - "An imported person carries their service areas, age group, facilitator start date, and every address and phone the old record held — and their profile dates from when FileMaker first knew them, not from the day they were imported." action_path: "/people" diff --git a/spec/services/person_services/import_from_rolodex_spec.rb b/spec/services/person_services/import_from_rolodex_spec.rb index 5b7eece9b2..08c918bb43 100644 --- a/spec/services/person_services/import_from_rolodex_spec.rb +++ b/spec/services/person_services/import_from_rolodex_spec.rb @@ -205,21 +205,31 @@ def results expect { expect(results[:ambiguous].size).to eq(1) }.not_to change(Person, :count) end - it "flags a same-name match with no email to confirm it" do + it "links a same-name person with no email" do # A linked login would become their primary email (PersonLoginEmailSync), so the - # person this guards against is one with no account. - create(:person, first_name: "Ada", last_name: "Lovelace", email: nil, user: nil) + # person this reaches is one with no account. + person = create(:person, first_name: "Ada", last_name: "Lovelace", email: nil, user: nil) rolodex("00011") - expect { expect(results[:ambiguous].size).to eq(1) }.not_to change(Person, :count) + expect { expect(results[:linked].size).to eq(1) }.not_to change(Person, :count) + expect(person.reload.filemaker_code).to eq("00011") end - it "flags a name + email match already linked to another FileMaker record" do + it "adds the row's ID to a match already linked to another FileMaker record" do person = create(:person, first_name: "Ada", last_name: "Lovelace", email: "ada@example.com", filemaker_code: "99999") rolodex("00012", EMail: "ada@example.com") - expect(results[:ambiguous].size).to eq(1) - expect(person.reload.filemaker_code).to eq("99999") + expect(results[:linked].size).to eq(1) + expect(person.reload.filemaker_codes).to eq(%w[00012 99999]) + end + + it "adds the row's ID to every person the name and email answer to" do + first = create(:person, first_name: "Ada", last_name: "Lovelace", email: "ada@example.com") + second = create(:person, first_name: "Ada", last_name: "Lovelace", email: "ada.l@example.com", email_2: "ada@example.com") + rolodex("00018", EMail: "ada@example.com") + + expect { expect(results[:linked].size).to eq(1) }.not_to change(Person, :count) + expect([ first, second ].map { |person| person.reload.filemaker_code }).to eq(%w[00018 00018]) end it "skips agencies, deleted rows, and nameless rows" do From 6f53c426d79590a03bc8af6166a5da3b04152627 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Thu, 15 Oct 2026 09:10:50 -0400 Subject: [PATCH 08/10] Create a person when a Rolodex email belongs to someone else An email shared by two differently named people is usually a household or office address, not a sign they're the same person. Creating the contact and letting the deduper fold the rare true duplicate empties the last hand-review bucket, so the import no longer has an ambiguous outcome at all. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 +- app/services/person_services/import_from_rolodex.rb | 13 ++++--------- config/features.yml | 2 +- lib/tasks/fm_import_people.rake | 2 +- .../person_services/import_from_rolodex_spec.rb | 8 +++++--- 5 files changed, 12 insertions(+), 15 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index d54ce1c940..1c0e6ea010 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -280,7 +280,7 @@ action, or `authorize! :workshop, to: :summary?`). - `NewsSubscriptionCapture` — Successor to the retired person-level mailing-list consent flag: given a `person` + `source`, idempotently creates an active **News** `TopicSubscription` (skips when one is already active, no-op when the News topic isn't seeded). Called from `PublicRegistration` (source = event) and `PublicFormSubmission` (source = form) when the `communication_consent` question is answered - `OrganizationServices::CaptureFromSubmission` — The shared org-capture for **both** the event registration (`PublicRegistration`) and standalone (`PublicFormSubmission`) flows: resolves the submitted organization by name (`FormField::ORGANIZATION_NAME_FIELD_IDENTIFIER`, case-insensitive per the column collation; never creates one — an unmatched name is left for an admin), syncs its website/type (`SyncProfile`, latest-wins) and work address (`UpsertAddress`, which adds another work address rather than rewriting a different one). `AffiliationServices::CreateFromRegistration` then mints the person's job affiliation from the typed position, plus the standing Facilitator one when `facilitator_training:` — which only an event supplies (`@event.facilitator_training`), so a standalone form gets the job affiliation alone, the same as registering for a non-training event. **Agreement-role submissions never come through here** — they run `LinkSubmittedOrganization`, which owns scenario end-dating and the new-job start date. Returns the org + the autofill changes, which the event flow records on its registration↔org link - `PersonServices::CaptureFromSubmission` — The shared person-capture for **both** the event registration (`PublicRegistration`) and standalone (`PublicFormSubmission`) flows, run for a newly-created **and** an existing person, so the person smart fields on the Smart form settings page behave identically across both forms. Fills identity extras (pronouns, name `pronunciation`, `nickname` → first name / legal first name, primary email type, secondary email + type), the profile (`racial_ethnic_identity` + `SyncSharingPreferences`), the mailing address, the phone contact method, and sector/age tags (mirrored onto any organizations passed in). A non-blank answer overwrites what's on file (newest submission wins); a blank never clobbers. Sector and age tags both run through `SubmissionTagging` and follow one rule on the **person**: submitting both a primary and additional set **overwrites** the person's tags with the latest (`replace: true`), submitting only a primary **reassigns** the primary and leaves the rest (additive); the org mirror always stays additive (orgs aggregate their members' tags). Self-gates on each field identifier, so a form that doesn't ask a question no-ops for it -- `PersonServices::ImportFromRolodex` — Creates people from FileMaker Rolodex archive rows (`FmRolodex`), carrying the row's ID as `filemaker_code` plus every field `FmRolodex::PORTAL_FIELDS` maps onto a person (profile fields, email type, facilitator start → `member_since`, service areas → sectors and `PrimaryAgeGroup` → age group via `SubmissionTagging`, and the archive's own entry/modification dates as `created_at`/`updated_at`), plus every address and phone the row answers to (`FmRolodex#addresses` / `#phone_numbers`, primary first). Links an existing person through `PersonMatcher` (or, for an email-less row, a same-name person with no email) by adding the row's ID to their `filemaker_code` list — every match when several answer, and alongside any code they already carry; flags only an email belonging to a differently named person as ambiguous for hand review; skips agencies, soft-deleted, and nameless rows. Counts service areas no sector answers to in `#unmatched_tags`. Idempotent on `filemaker_code`. Driven by `fm:import_people` +- `PersonServices::ImportFromRolodex` — Creates people from FileMaker Rolodex archive rows (`FmRolodex`), carrying the row's ID as `filemaker_code` plus every field `FmRolodex::PORTAL_FIELDS` maps onto a person (profile fields, email type, facilitator start → `member_since`, service areas → sectors and `PrimaryAgeGroup` → age group via `SubmissionTagging`, and the archive's own entry/modification dates as `created_at`/`updated_at`), plus every address and phone the row answers to (`FmRolodex#addresses` / `#phone_numbers`, primary first). Links an existing person through `PersonMatcher` (or, for an email-less row, a same-name person with no email) by adding the row's ID to their `filemaker_code` list — every match when several answer, and alongside any code they already carry; creates a new person when nobody matches, even when the email belongs to a differently named person; skips agencies, soft-deleted, and nameless rows. Counts service areas no sector answers to in `#unmatched_tags`. Idempotent on `filemaker_code`. Driven by `fm:import_people` - `PersonServices::SyncSharingPreferences` — Writes the two content-sharing answers (`anonymous_contributions`, `display_name_preference`) onto the `Person`, mapping each submitted option label back to its stored value; an unanswered or unrecognized answer leaves the profile alone, so a form that doesn't ask the questions is a no-op and every flow can call it unconditionally. The person-side counterpart to `OrganizationServices::SyncProfile`. Shared by `PersonServices::CaptureFromSubmission` (so a submission's answers land in the "what this submission changed" audit via the Ahoy trail) and `CalloutFormSubmission` (whose `#profile_changes` re-reports the same edits for its own Ahoy tracking) - `EventRegistrationServices::TransferContinuingEducation` — Splits/relocates a registrant's CE when they transfer events (issue #1944): a simple forward transfer leaves a paid, zero-hours **stub** on the source (its payments count at the original event) and creates a **live** record on the destination carrying the hours and the outstanding balance; when the reg being transferred out is itself a transfer-in (a collapsing double transfer, or a transfer back to the origin) its live record is relocated forward — merging back into the origin's stub — instead of split again, so no third record appears. Runs inside the transfer transaction, after the destination is saved and before a collapsing middle reg is destroyed - `EventRegistrationServices::ReconcileMergedContinuingEducation` — Run from the event-registration deduper's `after_merge`. A CE registration moved from the deleted registration still points at that (deleted) registrant's license — invalid, since the registration now belongs to the kept registrant — so this repoints each to the kept registrant's equivalent license (same kind + number, found or created), then folds the resulting duplicates into one with `ContinuingEducationDeduper` so payments are preserved diff --git a/app/services/person_services/import_from_rolodex.rb b/app/services/person_services/import_from_rolodex.rb index 32d74e93e0..ecfa680b51 100644 --- a/app/services/person_services/import_from_rolodex.rb +++ b/app/services/person_services/import_from_rolodex.rb @@ -9,16 +9,14 @@ module PersonServices # every same-name person with no email when the row has none, # adds the row's ID to their filemaker_code list # * already_imported — a person already carries the row's ID - # * ambiguous — the email belongs to a differently named person; reconcile - # by hand # * skipped — agency, soft-deleted, or nameless rows # * failed — the Person didn't validate # - # A row whose email matches nobody is created even when someone already shares the - # name: folding the occasional duplicate with the person deduper beats hand-resolving - # every name collision in a 39,000-row archive. + # A row whose name + email matches nobody is created, even when someone already shares + # the name or the email: folding the occasional duplicate with the person deduper beats + # hand-resolving every collision in a 39,000-row archive. class ImportFromRolodex - OUTCOMES = %i[created linked already_imported ambiguous skipped failed].freeze + OUTCOMES = %i[created linked already_imported skipped failed].freeze FALSE_FLAGS = %w[0 n no false].freeze DATE_FORMATS = %w[%m/%d/%Y %Y-%m-%d %m-%d-%Y %Y/%m/%d].freeze # A two-digit year below this is read as 20xx, at or above it as 19xx. @@ -64,9 +62,6 @@ def import(rolodex) if email match = PersonMatcher.new(email:, last_name:, first_names: [ first_name ]).matches.to_a return link(match, rolodex, label) if match.any? - - others = Person.where("LOWER(email) = :email OR LOWER(email_2) = :email", email: email.downcase).to_a - return record(:ambiguous, "#{label}: #{email} belongs to #{others.map { |p| "##{p.id} #{p.full_name}" }.join(', ')}") if others.any? else no_email = Person.where("LOWER(first_name) = ? AND LOWER(last_name) = ?", first_name.downcase, last_name.downcase) .where(email: [ nil, "" ]).to_a diff --git a/config/features.yml b/config/features.yml index 78e549a55f..83bf25a0c4 100644 --- a/config/features.yml +++ b/config/features.yml @@ -4644,7 +4644,7 @@ - "A contact who already has a portal record is linked to it rather than duplicated — matching takes the same name and the same email." - "A contact with no email is linked to a portal person of the same name who also has no email." - "A person can carry several FileMaker codes, so a contact who matches someone already linked to another FileMaker record is added to their list." - - "Only an email that belongs to a differently named person is listed for someone to sort out by hand instead of guessed at." + - "A contact whose email belongs to someone with a different name becomes a new person — merge them with the person deduper if they turn out to be the same." - "A person's FileMaker code on their page links straight to their row in the archive." - "An imported person carries their service areas, age group, facilitator start date, and every address and phone the old record held — and their profile dates from when FileMaker first knew them, not from the day they were imported." action_path: "/people" diff --git a/lib/tasks/fm_import_people.rake b/lib/tasks/fm_import_people.rake index 69b1be3f27..4f043abd8d 100644 --- a/lib/tasks/fm_import_people.rake +++ b/lib/tasks/fm_import_people.rake @@ -12,7 +12,7 @@ namespace :fm do task import_people: :environment do dry_run = ENV["DRY_RUN"] != "false" verbose = ENV["VERBOSE"] == "true" - detailed = verbose ? PersonServices::ImportFromRolodex::OUTCOMES : %i[ambiguous failed] + detailed = verbose ? PersonServices::ImportFromRolodex::OUTCOMES : %i[failed] import = nil ActiveRecord::Base.transaction do diff --git a/spec/services/person_services/import_from_rolodex_spec.rb b/spec/services/person_services/import_from_rolodex_spec.rb index 08c918bb43..00829001b5 100644 --- a/spec/services/person_services/import_from_rolodex_spec.rb +++ b/spec/services/person_services/import_from_rolodex_spec.rb @@ -198,11 +198,13 @@ def results expect(Person.where(filemaker_code: "00009").count).to eq(1) end - it "flags an email that belongs to a differently named person" do - create(:person, first_name: "Charles", last_name: "Babbage", email: "ada@example.com") + it "creates a new person when the email belongs to a differently named one" do + babbage = create(:person, first_name: "Charles", last_name: "Babbage", email: "ada@example.com") rolodex("00010", EMail: "ada@example.com") - expect { expect(results[:ambiguous].size).to eq(1) }.not_to change(Person, :count) + expect { expect(results[:created].size).to eq(1) }.to change(Person, :count).by(1) + expect(Person.find_by!(filemaker_code: "00010")).to have_attributes(first_name: "Ada", email: "ada@example.com") + expect(babbage.reload.filemaker_code).to be_nil end it "links a same-name person with no email" do From f4595fbd06cfd70ba7bf97598e6b563fee32f066 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Thu, 15 Oct 2026 09:30:53 -0400 Subject: [PATCH 09/10] Translate the Rolodex's who-they-serve fields through TagTranslation The import matched tags by name with its own prefix guess and never read Settings or LifeExperiences, which hold most of the archive's mentions (~28k of ~33k). Going through the ADR-0007 translations resolves the legacy words, honors the decided non-translations, and lets a sector field carry an age group. A primary answer naming several tags lands them all as additional so a person never ends up with two primaries and fails validation on next save. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 +- app/models/fm_rolodex.rb | 2 + .../person_services/import_from_rolodex.rb | 52 ++++++++------- config/features.yml | 3 +- lib/tasks/fm_import_people.rake | 2 +- .../import_from_rolodex_spec.rb | 64 ++++++++++++++++++- 6 files changed, 96 insertions(+), 29 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 1c0e6ea010..49f9c745b0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -280,7 +280,7 @@ action, or `authorize! :workshop, to: :summary?`). - `NewsSubscriptionCapture` — Successor to the retired person-level mailing-list consent flag: given a `person` + `source`, idempotently creates an active **News** `TopicSubscription` (skips when one is already active, no-op when the News topic isn't seeded). Called from `PublicRegistration` (source = event) and `PublicFormSubmission` (source = form) when the `communication_consent` question is answered - `OrganizationServices::CaptureFromSubmission` — The shared org-capture for **both** the event registration (`PublicRegistration`) and standalone (`PublicFormSubmission`) flows: resolves the submitted organization by name (`FormField::ORGANIZATION_NAME_FIELD_IDENTIFIER`, case-insensitive per the column collation; never creates one — an unmatched name is left for an admin), syncs its website/type (`SyncProfile`, latest-wins) and work address (`UpsertAddress`, which adds another work address rather than rewriting a different one). `AffiliationServices::CreateFromRegistration` then mints the person's job affiliation from the typed position, plus the standing Facilitator one when `facilitator_training:` — which only an event supplies (`@event.facilitator_training`), so a standalone form gets the job affiliation alone, the same as registering for a non-training event. **Agreement-role submissions never come through here** — they run `LinkSubmittedOrganization`, which owns scenario end-dating and the new-job start date. Returns the org + the autofill changes, which the event flow records on its registration↔org link - `PersonServices::CaptureFromSubmission` — The shared person-capture for **both** the event registration (`PublicRegistration`) and standalone (`PublicFormSubmission`) flows, run for a newly-created **and** an existing person, so the person smart fields on the Smart form settings page behave identically across both forms. Fills identity extras (pronouns, name `pronunciation`, `nickname` → first name / legal first name, primary email type, secondary email + type), the profile (`racial_ethnic_identity` + `SyncSharingPreferences`), the mailing address, the phone contact method, and sector/age tags (mirrored onto any organizations passed in). A non-blank answer overwrites what's on file (newest submission wins); a blank never clobbers. Sector and age tags both run through `SubmissionTagging` and follow one rule on the **person**: submitting both a primary and additional set **overwrites** the person's tags with the latest (`replace: true`), submitting only a primary **reassigns** the primary and leaves the rest (additive); the org mirror always stays additive (orgs aggregate their members' tags). Self-gates on each field identifier, so a form that doesn't ask a question no-ops for it -- `PersonServices::ImportFromRolodex` — Creates people from FileMaker Rolodex archive rows (`FmRolodex`), carrying the row's ID as `filemaker_code` plus every field `FmRolodex::PORTAL_FIELDS` maps onto a person (profile fields, email type, facilitator start → `member_since`, service areas → sectors and `PrimaryAgeGroup` → age group via `SubmissionTagging`, and the archive's own entry/modification dates as `created_at`/`updated_at`), plus every address and phone the row answers to (`FmRolodex#addresses` / `#phone_numbers`, primary first). Links an existing person through `PersonMatcher` (or, for an email-less row, a same-name person with no email) by adding the row's ID to their `filemaker_code` list — every match when several answer, and alongside any code they already carry; creates a new person when nobody matches, even when the email belongs to a differently named person; skips agencies, soft-deleted, and nameless rows. Counts service areas no sector answers to in `#unmatched_tags`. Idempotent on `filemaker_code`. Driven by `fm:import_people` +- `PersonServices::ImportFromRolodex` — Creates people from FileMaker Rolodex archive rows (`FmRolodex`), carrying the row's ID as `filemaker_code` plus every field `FmRolodex::PORTAL_FIELDS` maps onto a person (profile fields, email type, facilitator start → `member_since`, service areas, `Settings`, `LifeExperiences` and `PrimaryAgeGroup` → sectors and age groups translated through `TagTranslation` (ADR-0007; primary fields → primary, the rest additional; a primary answer naming several tags lands them all additional) via `SubmissionTagging`, and the archive's own entry/modification dates as `created_at`/`updated_at`), plus every address and phone the row answers to (`FmRolodex#addresses` / `#phone_numbers`, primary first). Links an existing person through `PersonMatcher` (or, for an email-less row, a same-name person with no email) by adding the row's ID to their `filemaker_code` list — every match when several answer, and alongside any code they already carry; creates a new person when nobody matches, even when the email belongs to a differently named person; skips agencies, soft-deleted, and nameless rows. Exposes `TagTranslation#unmatched` as `#unmatched_tags`. Idempotent on `filemaker_code`. Driven by `fm:import_people` - `PersonServices::SyncSharingPreferences` — Writes the two content-sharing answers (`anonymous_contributions`, `display_name_preference`) onto the `Person`, mapping each submitted option label back to its stored value; an unanswered or unrecognized answer leaves the profile alone, so a form that doesn't ask the questions is a no-op and every flow can call it unconditionally. The person-side counterpart to `OrganizationServices::SyncProfile`. Shared by `PersonServices::CaptureFromSubmission` (so a submission's answers land in the "what this submission changed" audit via the Ahoy trail) and `CalloutFormSubmission` (whose `#profile_changes` re-reports the same edits for its own Ahoy tracking) - `EventRegistrationServices::TransferContinuingEducation` — Splits/relocates a registrant's CE when they transfer events (issue #1944): a simple forward transfer leaves a paid, zero-hours **stub** on the source (its payments count at the original event) and creates a **live** record on the destination carrying the hours and the outstanding balance; when the reg being transferred out is itself a transfer-in (a collapsing double transfer, or a transfer back to the origin) its live record is relocated forward — merging back into the origin's stub — instead of split again, so no third record appears. Runs inside the transfer transaction, after the destination is saved and before a collapsing middle reg is destroyed - `EventRegistrationServices::ReconcileMergedContinuingEducation` — Run from the event-registration deduper's `after_merge`. A CE registration moved from the deleted registration still points at that (deleted) registrant's license — invalid, since the registration now belongs to the kept registrant — so this repoints each to the kept registrant's equivalent license (same kind + number, found or created), then folds the resulting duplicates into one with `ContinuingEducationDeduper` so payments are preserved diff --git a/app/models/fm_rolodex.rb b/app/models/fm_rolodex.rb index 07edb40b08..91f513581f 100644 --- a/app/models/fm_rolodex.rb +++ b/app/models/fm_rolodex.rb @@ -123,6 +123,8 @@ class FmRolodex < ApplicationRecord "FacilitatorSinceYear" => "people.member_since (with FacilitatorSinceMonth)", "PrimaryServiceArea" => "the person's primary sector", "OtherServiceArea" => "the person's additional sectors", + "Settings" => "the person's additional sectors", + "LifeExperiences" => "the person's additional sectors", "PrimaryAgeGroup" => "the person's primary age group", "DateEntered" => "people.created_at", "DateTimeModified" => "people.updated_at", diff --git a/app/services/person_services/import_from_rolodex.rb b/app/services/person_services/import_from_rolodex.rb index ecfa680b51..6affa63d27 100644 --- a/app/services/person_services/import_from_rolodex.rb +++ b/app/services/person_services/import_from_rolodex.rb @@ -2,8 +2,10 @@ module PersonServices # Turns FileMaker Rolodex archive rows (FmRolodex) into people. Each row lands in # one outcome bucket of #results: # * created — new Person: the fields FmRolodex::PORTAL_FIELDS maps to a - # person (including its sector/age tags and the archive's own - # entry date), plus the row's primary address and phone + # person (including the sector/age tags its service areas, + # settings, life experiences and age group translate to, and + # the archive's own entry date), plus the row's addresses and + # phones # * linked — every person PersonMatcher finds for the row's name + email # (which reaches their secondary and login addresses too), or # every same-name person with no email when the row has none, @@ -27,16 +29,18 @@ class ImportFromRolodex UNKNOWN_LOCALITY = "Unknown" STRING_LIMIT = 255 - attr_reader :results, :unmatched_tags + # Whether a translated tag lands as primary or additional follows the field (ADR-0007). + PRIMARY_TAG_FIELDS = %w[PrimaryServiceArea PrimaryAgeGroup].freeze + ADDITIONAL_TAG_FIELDS = %w[OtherServiceArea Settings LifeExperiences].freeze + + attr_reader :results def initialize(scope: FmRolodex.all) @scope = scope @results = OUTCOMES.index_with { [] } @imported_codes = Person.where.not(filemaker_code: [ nil, "" ]).pluck(:filemaker_code) .flat_map { |value| Person.split_filemaker_codes(value) }.to_set - @sectors_by_name = name_index(Sector.all) - @age_groups_by_name = name_index(Category.age_ranges) - @unmatched_tags = Hash.new(0) + @translation = TagTranslation.new end def call @@ -44,6 +48,10 @@ def call self end + def unmatched_tags + @translation.unmatched + end + private def import(rolodex) @@ -139,29 +147,27 @@ def month_number(value) [ Date::MONTHNAMES, Date::ABBR_MONTHNAMES ].filter_map { |names| names.index { |name| name&.casecmp?(value) } }.first end - # The archive's service areas and age group tag the person through the same seam a + # The archive's who-they-serve fields tag the person through the same seam a # submission uses — additive, since the import is never the whole picture. def tag_service_areas(person, data) - SubmissionTagging.apply(person:, organizations: [], tag_with: :tag_sectors, - primary_ids: tag_ids(data["PrimaryServiceArea"], @sectors_by_name).first(1), - additional_ids: tag_ids(data["OtherServiceArea"], @sectors_by_name)) - SubmissionTagging.apply(person:, organizations: [], tag_with: :tag_age_groups, - primary_ids: tag_ids(data["PrimaryAgeGroup"], @age_groups_by_name)) - end + primary = translate(data, PRIMARY_TAG_FIELDS) + additional = translate(data, ADDITIONAL_TAG_FIELDS) - # One field can list several areas. Each resolves by name, then by the portal's - # qualified form of it ("Child Abuse" → "Child Abuse/Neglect"); a name nothing - # answers to is counted in #unmatched_tags rather than guessed at. - def tag_ids(value, index) - value.to_s.split(/[,;]/).map { |name| name.strip.downcase }.reject(&:empty?).uniq.filter_map do |name| - id = index[name] || index.find { |stored, _| stored.start_with?("#{name}/") }&.last - @unmatched_tags[name] += 1 unless id - id + { tag_sectors: :sector_ids, tag_age_groups: :age_group_ids }.each do |tag_with, kind| + primary_ids, extra_ids = split_primary(primary.flat_map(&kind).uniq) + SubmissionTagging.apply(person:, organizations: [], tag_with:, primary_ids:, + additional_ids: extra_ids + additional.flat_map(&kind)) end end - def name_index(scope) - scope.pluck(:name, :id).to_h { |name, id| [ name.to_s.strip.downcase, id ] } + def translate(data, fields) + fields.map { |field| @translation.resolve(data[field]) } + end + + # A person carries one primary of each kind, so a primary answer naming several — + # "Families" is every age range — lands them all as additional rather than picking one. + def split_primary(ids) + ids.one? ? [ ids, [] ] : [ [], ids ] end # Every address the row answers to, not just the one it names as primary — diff --git a/config/features.yml b/config/features.yml index 83bf25a0c4..87e4b7e18d 100644 --- a/config/features.yml +++ b/config/features.yml @@ -4644,9 +4644,10 @@ - "A contact who already has a portal record is linked to it rather than duplicated — matching takes the same name and the same email." - "A contact with no email is linked to a portal person of the same name who also has no email." - "A person can carry several FileMaker codes, so a contact who matches someone already linked to another FileMaker record is added to their list." + - "FileMaker's old words for who someone serves are translated through the recorded tag aliases, so \"Student Services\" becomes Education and \"Families\" tags every age range." - "A contact whose email belongs to someone with a different name becomes a new person — merge them with the person deduper if they turn out to be the same." - "A person's FileMaker code on their page links straight to their row in the archive." - - "An imported person carries their service areas, age group, facilitator start date, and every address and phone the old record held — and their profile dates from when FileMaker first knew them, not from the day they were imported." + - "An imported person carries their service areas, settings, life experiences, age group, facilitator start date, and every address and phone the old record held — and their profile dates from when FileMaker first knew them, not from the day they were imported." action_path: "/people" - name: "Unknown as an address and phone type" diff --git a/lib/tasks/fm_import_people.rake b/lib/tasks/fm_import_people.rake index 4f043abd8d..39c73a1804 100644 --- a/lib/tasks/fm_import_people.rake +++ b/lib/tasks/fm_import_people.rake @@ -28,7 +28,7 @@ namespace :fm do end if import.unmatched_tags.any? - puts "\nService areas and age groups no sector or category answers to:" + puts "\nTerms no sector, age group or recorded translation answers to:" import.unmatched_tags.sort_by { |name, count| [ -count, name ] }.each { |name, count| puts " #{count.to_s.rjust(5)} #{name}" } end diff --git a/spec/services/person_services/import_from_rolodex_spec.rb b/spec/services/person_services/import_from_rolodex_spec.rb index 00829001b5..48149d0601 100644 --- a/spec/services/person_services/import_from_rolodex_spec.rb +++ b/spec/services/person_services/import_from_rolodex_spec.rb @@ -38,9 +38,25 @@ def results describe "the fields FmRolodex::PORTAL_FIELDS maps onto a person" do let!(:age_range_type) { create(:category_type, name: "AgeRange", published: true) } - let!(:adults) { create(:category, category_type: age_range_type, name: "Adults") } - let!(:domestic_violence) { create(:sector, name: "Domestic Violence") } - let!(:child_abuse) { create(:sector, name: "Child Abuse/Neglect") } + let!(:children) { create(:category, :published, category_type: age_range_type, name: "Children") } + let!(:teens) { create(:category, :published, category_type: age_range_type, name: "Teens") } + let!(:adults) { create(:category, :published, category_type: age_range_type, name: "Adults") } + let!(:elders) { create(:category, :published, category_type: age_range_type, name: "Elders") } + let!(:domestic_violence) { create(:sector, :published, name: "Domestic Violence") } + let!(:child_abuse) { create(:sector, :published, name: "Child Abuse/Neglect") } + let!(:education) { create(:sector, :published, name: "Education") } + let!(:mental_health) { create(:sector, :published, name: "Mental Health") } + let!(:grief) { create(:sector, :published, name: "Grief/Loss") } + + before do + create(:tag_alias, name: "Child Abuse", aliasable: child_abuse) + create(:tag_alias, name: "Student Services", aliasable: education) + create(:tag_alias, name: "Clinical Setting", aliasable: mental_health) + create(:tag_alias, name: "Virtually", aliasable: nil, ignored: true) + create(:tag_alias, name: "Children's Services", aliasable: children) + create(:tag_alias, name: "Elder Abuse", aliasable: elders) + [ children, teens, adults, elders ].each { |age| create(:tag_alias, name: "Families", aliasable: age) } + end it "carries the email type, facilitator start, service areas, age group, and entry date" do rolodex("00050", EMail: "ada@example.com", EmailType: "Work", FacilitatorSinceMonth: "3", @@ -60,6 +76,48 @@ def results expect(import.unmatched_tags).to eq({ "homelessness" => 1 }) end + it "resolves legacy words through the recorded translations" do + rolodex("00054", EMail: "ada@example.com", PrimaryServiceArea: "Student Services") + + described_class.new.call + + expect(Person.find_by!(filemaker_code: "00054").primary_sector).to eq(education) + end + + it "adds the settings and life experiences as additional sectors" do + rolodex("00055", EMail: "ada@example.com", PrimaryServiceArea: "Domestic Violence", + Settings: "Clinical Setting\rVirtually", LifeExperiences: "Grief/Loss\rDomestic Violence") + + import = described_class.new.call + person = Person.find_by!(filemaker_code: "00055") + + expect(person.primary_sector).to eq(domestic_violence) + expect(person.sectorable_items.where(is_primary: false).map(&:sector)).to contain_exactly(mental_health, grief) + expect(import.unmatched_tags).to be_empty + end + + it "takes an age group a sector field names as primary or additional by the field it sat in" do + rolodex("00056", EMail: "ada@example.com", PrimaryServiceArea: "Children's Services", LifeExperiences: "Elder Abuse") + + described_class.new.call + person = Person.find_by!(filemaker_code: "00056") + + expect(person.primary_age_groups).to eq([ children ]) + expect(person.additional_age_groups).to eq([ elders ]) + expect(person.sectorable_items).to be_empty + end + + it "tags every age range for families without marking one primary" do + rolodex("00057", EMail: "ada@example.com", PrimaryAgeGroup: "Families") + + described_class.new.call + person = Person.find_by!(filemaker_code: "00057") + + expect(person.primary_age_groups).to be_empty + expect(person.additional_age_groups).to contain_exactly(children, teens, adults, elders) + expect(person).to be_valid + end + it "reads a facilitator year with no month as January, and a month name" do rolodex("00051", FirstName: "Grace", EMail: "grace@example.com", FacilitatorSinceYear: "2018") rolodex("00052", FirstName: "Hedy", EMail: "hedy@example.com", FacilitatorSinceMonth: "June", From 7cda6db362ceadb877b6b31f5387ce902420b6c8 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Thu, 15 Oct 2026 11:23:15 -0400 Subject: [PATCH 10/10] Fill linked Rolodex people from the archive, keeping portal values on conflict A matched person only got the code, so they ended up thinner than an identical person the import created. Staff edited both systems, so neither side wins by rule: blanks take the archive's value, and a disagreement keeps the portal's and is left as a comment on the person (and listed by the task) for someone to check. A primary sector answer naming several sectors now makes the first primary, unless the person already holds one. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 +- .../person_services/import_from_rolodex.rb | 136 ++++++++++++++---- config/features.yml | 1 + lib/tasks/fm_import_people.rake | 7 +- .../import_from_rolodex_spec.rb | 75 ++++++++++ 5 files changed, 192 insertions(+), 29 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 49f9c745b0..a19a80aa95 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -280,7 +280,7 @@ action, or `authorize! :workshop, to: :summary?`). - `NewsSubscriptionCapture` — Successor to the retired person-level mailing-list consent flag: given a `person` + `source`, idempotently creates an active **News** `TopicSubscription` (skips when one is already active, no-op when the News topic isn't seeded). Called from `PublicRegistration` (source = event) and `PublicFormSubmission` (source = form) when the `communication_consent` question is answered - `OrganizationServices::CaptureFromSubmission` — The shared org-capture for **both** the event registration (`PublicRegistration`) and standalone (`PublicFormSubmission`) flows: resolves the submitted organization by name (`FormField::ORGANIZATION_NAME_FIELD_IDENTIFIER`, case-insensitive per the column collation; never creates one — an unmatched name is left for an admin), syncs its website/type (`SyncProfile`, latest-wins) and work address (`UpsertAddress`, which adds another work address rather than rewriting a different one). `AffiliationServices::CreateFromRegistration` then mints the person's job affiliation from the typed position, plus the standing Facilitator one when `facilitator_training:` — which only an event supplies (`@event.facilitator_training`), so a standalone form gets the job affiliation alone, the same as registering for a non-training event. **Agreement-role submissions never come through here** — they run `LinkSubmittedOrganization`, which owns scenario end-dating and the new-job start date. Returns the org + the autofill changes, which the event flow records on its registration↔org link - `PersonServices::CaptureFromSubmission` — The shared person-capture for **both** the event registration (`PublicRegistration`) and standalone (`PublicFormSubmission`) flows, run for a newly-created **and** an existing person, so the person smart fields on the Smart form settings page behave identically across both forms. Fills identity extras (pronouns, name `pronunciation`, `nickname` → first name / legal first name, primary email type, secondary email + type), the profile (`racial_ethnic_identity` + `SyncSharingPreferences`), the mailing address, the phone contact method, and sector/age tags (mirrored onto any organizations passed in). A non-blank answer overwrites what's on file (newest submission wins); a blank never clobbers. Sector and age tags both run through `SubmissionTagging` and follow one rule on the **person**: submitting both a primary and additional set **overwrites** the person's tags with the latest (`replace: true`), submitting only a primary **reassigns** the primary and leaves the rest (additive); the org mirror always stays additive (orgs aggregate their members' tags). Self-gates on each field identifier, so a form that doesn't ask a question no-ops for it -- `PersonServices::ImportFromRolodex` — Creates people from FileMaker Rolodex archive rows (`FmRolodex`), carrying the row's ID as `filemaker_code` plus every field `FmRolodex::PORTAL_FIELDS` maps onto a person (profile fields, email type, facilitator start → `member_since`, service areas, `Settings`, `LifeExperiences` and `PrimaryAgeGroup` → sectors and age groups translated through `TagTranslation` (ADR-0007; primary fields → primary, the rest additional; a primary answer naming several tags lands them all additional) via `SubmissionTagging`, and the archive's own entry/modification dates as `created_at`/`updated_at`), plus every address and phone the row answers to (`FmRolodex#addresses` / `#phone_numbers`, primary first). Links an existing person through `PersonMatcher` (or, for an email-less row, a same-name person with no email) by adding the row's ID to their `filemaker_code` list — every match when several answer, and alongside any code they already carry; creates a new person when nobody matches, even when the email belongs to a differently named person; skips agencies, soft-deleted, and nameless rows. Exposes `TagTranslation#unmatched` as `#unmatched_tags`. Idempotent on `filemaker_code`. Driven by `fm:import_people` +- `PersonServices::ImportFromRolodex` — Creates people from FileMaker Rolodex archive rows (`FmRolodex`), carrying the row's ID as `filemaker_code` plus every field `FmRolodex::PORTAL_FIELDS` maps onto a person (profile fields, email type, facilitator start → `member_since`, service areas, `Settings`, `LifeExperiences` and `PrimaryAgeGroup` → sectors and age groups translated through `TagTranslation` (ADR-0007; primary fields → primary, the rest additional; a primary answer naming several sectors makes the first primary, several age groups all land additional) via `SubmissionTagging`, and the archive's own entry/modification dates as `created_at`/`updated_at`), plus every address and phone the row answers to (`FmRolodex#addresses` / `#phone_numbers`, primary first). Links an existing person through `PersonMatcher` (or, for an email-less row, a same-name person with no email) by adding the row's ID to their `filemaker_code` list — every match when several answer, and alongside any code they already carry — and fills them from the row: blank fields take the archive's value, missing addresses/phones/tags are added (an existing primary stays primary), and a field the archive disagrees on keeps the portal value and is noted as a `Comment` on the person and in `#differences`; creates a new person when nobody matches, even when the email belongs to a differently named person; skips agencies, soft-deleted, and nameless rows. Exposes `TagTranslation#unmatched` as `#unmatched_tags`. Idempotent on `filemaker_code`. Driven by `fm:import_people` - `PersonServices::SyncSharingPreferences` — Writes the two content-sharing answers (`anonymous_contributions`, `display_name_preference`) onto the `Person`, mapping each submitted option label back to its stored value; an unanswered or unrecognized answer leaves the profile alone, so a form that doesn't ask the questions is a no-op and every flow can call it unconditionally. The person-side counterpart to `OrganizationServices::SyncProfile`. Shared by `PersonServices::CaptureFromSubmission` (so a submission's answers land in the "what this submission changed" audit via the Ahoy trail) and `CalloutFormSubmission` (whose `#profile_changes` re-reports the same edits for its own Ahoy tracking) - `EventRegistrationServices::TransferContinuingEducation` — Splits/relocates a registrant's CE when they transfer events (issue #1944): a simple forward transfer leaves a paid, zero-hours **stub** on the source (its payments count at the original event) and creates a **live** record on the destination carrying the hours and the outstanding balance; when the reg being transferred out is itself a transfer-in (a collapsing double transfer, or a transfer back to the origin) its live record is relocated forward — merging back into the origin's stub — instead of split again, so no third record appears. Runs inside the transfer transaction, after the destination is saved and before a collapsing middle reg is destroyed - `EventRegistrationServices::ReconcileMergedContinuingEducation` — Run from the event-registration deduper's `after_merge`. A CE registration moved from the deleted registration still points at that (deleted) registrant's license — invalid, since the registration now belongs to the kept registrant — so this repoints each to the kept registrant's equivalent license (same kind + number, found or created), then folds the resulting duplicates into one with `ContinuingEducationDeduper` so payments are preserved diff --git a/app/services/person_services/import_from_rolodex.rb b/app/services/person_services/import_from_rolodex.rb index 6affa63d27..7bb8896689 100644 --- a/app/services/person_services/import_from_rolodex.rb +++ b/app/services/person_services/import_from_rolodex.rb @@ -9,7 +9,11 @@ module PersonServices # * linked — every person PersonMatcher finds for the row's name + email # (which reaches their secondary and login addresses too), or # every same-name person with no email when the row has none, - # adds the row's ID to their filemaker_code list + # adds the row's ID to their filemaker_code list and is filled + # from it: blank fields take the archive's value, missing + # addresses, phones and tags are added, and a field the archive + # disagrees on keeps the person's value — noted in #differences + # and as a comment on the person # * already_imported — a person already carries the row's ID # * skipped — agency, soft-deleted, or nameless rows # * failed — the Person didn't validate @@ -32,12 +36,14 @@ class ImportFromRolodex # Whether a translated tag lands as primary or additional follows the field (ADR-0007). PRIMARY_TAG_FIELDS = %w[PrimaryServiceArea PrimaryAgeGroup].freeze ADDITIONAL_TAG_FIELDS = %w[OtherServiceArea Settings LifeExperiences].freeze + PRIMARY_TAG_LABELS = { tag_sectors: "Primary sector", tag_age_groups: "Primary age group" }.freeze - attr_reader :results + attr_reader :results, :differences def initialize(scope: FmRolodex.all) @scope = scope @results = OUTCOMES.index_with { [] } + @differences = [] @imported_codes = Person.where.not(filemaker_code: [ nil, "" ]).pluck(:filemaker_code) .flat_map { |value| Person.split_filemaker_codes(value) }.to_set @translation = TagTranslation.new @@ -83,9 +89,12 @@ def import(rolodex) # and the deduper unions them when they're merged. def link(people, rolodex, label) people.each do |person| - next if person.update(filemaker_code: Person.join_filemaker_codes(person.filemaker_code, rolodex.fm_id)) + person.filemaker_code = Person.join_filemaker_codes(person.filemaker_code, rolodex.fm_id) + kept = fill(person, rolodex) + return record(:failed, "#{label}: ##{person.id} #{person.errors.full_messages.to_sentence}") unless person.save - return record(:failed, "#{label}: ##{person.id} #{person.errors.full_messages.to_sentence}") + kept.concat(tag_service_areas(person, rolodex.data)) + note_differences(person, rolodex, label, kept) if kept.any? end @imported_codes << rolodex.fm_id @@ -94,18 +103,7 @@ def link(people, rolodex, label) def create(rolodex, label, **names) data = rolodex.data - person = Person.new( - **names, - filemaker_code: rolodex.fm_id, - pronouns: clip(data["Pronouns"]), - best_time_to_call: clip(data["BestTimeToCall"]), - racial_ethnic_identity: clip(data["Ethnicity"]), - date_of_birth: parse_date(data["DateOfBirth"]), - email_type: email_type(data["EmailType"]), - member_since: member_since(data), - notes: data["Comments"].presence, - **archive_timestamps(data) - ) + person = Person.new(**names, filemaker_code: rolodex.fm_id, **profile_attributes(data), **archive_timestamps(data)) build_addresses(person, rolodex) build_phones(person, rolodex) @@ -120,6 +118,53 @@ def create(rolodex, label, **names) record(:created, "#{label} → ##{person.id}") end + def profile_attributes(data) + { + pronouns: clip(data["Pronouns"]), + best_time_to_call: clip(data["BestTimeToCall"]), + racial_ethnic_identity: clip(data["Ethnicity"]), + date_of_birth: parse_date(data["DateOfBirth"]), + email_type: email_type(data["EmailType"]), + member_since: member_since(data), + notes: data["Comments"].presence + } + end + + # Staff edited both systems, so neither side is newer by rule: a blank takes the + # archive's value, and a disagreement keeps the person's and is returned as + # [ label, portal value, archive value ] to note. + def fill(person, rolodex) + kept = profile_attributes(rolodex.data).filter_map do |attribute, value| + next if value.blank? + next person[attribute] = value if person[attribute].blank? + next if same?(person[attribute], value) + + [ Person.human_attribute_name(attribute), person[attribute], value ] + end + fill_email_2(person, rolodex.data) + build_addresses(person, rolodex) + build_phones(person, rolodex) + kept + end + + def fill_email_2(person, data) + return if person.email_2.present? + + held = [ person.email, person.user&.email ].compact_blank.map(&:downcase) + person.email_2 = data["EMail"].to_s.scan(EMAIL_PATTERN).find { |email| held.exclude?(email.downcase) } + end + + def same?(portal, archive) + portal.to_s.squish.casecmp?(archive.to_s.squish) + end + + def note_differences(person, rolodex, label, kept) + lines = kept.map { |field, portal, archive| "#{field}: kept “#{portal}”, FileMaker has “#{archive}”" } + Comment.create!(commentable: person, + body: "FileMaker Rolodex #{rolodex.fm_id} differs from this profile:\n#{lines.join("\n")}") + @differences << "#{label} → ##{person.id}: #{lines.join('; ')}" + end + # The archive's own entry and modification dates, so an imported person dates from # when FileMaker first knew them rather than from the import run. def archive_timestamps(data) @@ -148,39 +193,76 @@ def month_number(value) end # The archive's who-they-serve fields tag the person through the same seam a - # submission uses — additive, since the import is never the whole picture. + # submission uses — additive, since the import is never the whole picture. A primary + # the person already holds stays; the archive's then lands as additional and is + # returned as a difference to note. def tag_service_areas(person, data) primary = translate(data, PRIMARY_TAG_FIELDS) additional = translate(data, ADDITIONAL_TAG_FIELDS) + sector_ids = primary.flat_map(&:sector_ids).uniq + age_group_ids = primary.flat_map(&:age_group_ids).uniq + age_primary_ids, age_additional_ids = split_age_primary(age_group_ids) + + [ + apply_tags(person, :tag_sectors, Sector, person.sectorable_items.where(is_primary: true).pluck(:sector_id), + primary_ids: sector_ids.first(1), additional_ids: sector_ids.drop(1) + additional.flat_map(&:sector_ids)), + apply_tags(person, :tag_age_groups, Category, person.primary_age_category_ids, + primary_ids: age_primary_ids, additional_ids: age_additional_ids + additional.flat_map(&:age_group_ids)) + ].compact + end - { tag_sectors: :sector_ids, tag_age_groups: :age_group_ids }.each do |tag_with, kind| - primary_ids, extra_ids = split_primary(primary.flat_map(&kind).uniq) - SubmissionTagging.apply(person:, organizations: [], tag_with:, primary_ids:, - additional_ids: extra_ids + additional.flat_map(&kind)) + def apply_tags(person, tag_with, model, held_ids, primary_ids:, additional_ids:) + if held_ids.any? + difference = [ PRIMARY_TAG_LABELS[tag_with], names(model, held_ids), names(model, primary_ids) ] unless (primary_ids - held_ids).empty? + primary_ids, additional_ids = [], primary_ids + additional_ids end + SubmissionTagging.apply(person:, organizations: [], tag_with:, primary_ids:, additional_ids:) + difference end def translate(data, fields) fields.map { |field| @translation.resolve(data[field]) } end - # A person carries one primary of each kind, so a primary answer naming several — - # "Families" is every age range — lands them all as additional rather than picking one. - def split_primary(ids) + # A person carries one primary age group, and an answer naming several — "Families" + # is every age range — has no first to pick, so they all land as additional. + def split_age_primary(ids) ids.one? ? [ ids, [] ] : [ [], ids ] end + def names(model, ids) + model.where(id: ids).pluck(:name).to_sentence + end + # Every address the row answers to, not just the one it names as primary — # FmRolodex#addresses lists them with the primary and worksite ones first, so the - # first that can be built is the person's primary and the rest ride along. + # first that can be built is the person's primary and the rest ride along. One the + # person already has is left out, and a primary they already hold stays primary. def build_addresses(person, rolodex) + held = person.addresses.map { |address| address_key(address.street_address, address.city) } attributes = rolodex.addresses.filter_map { |address| address_attributes(address.data) } - attributes.each_with_index { |attrs, index| person.addresses.build(attrs.merge(primary: index.zero?)) } + .reject { |attrs| held.include?(address_key(attrs[:street_address], attrs[:city])) } + build_primary_first(person.addresses, attributes) end def build_phones(person, rolodex) + held = person.contact_methods.map { |phone| phone_key(phone.value) } attributes = rolodex.phone_numbers.filter_map { |phone| phone_attributes(phone.data) } - attributes.each_with_index { |attrs, index| person.contact_methods.build(attrs.merge(primary: index.zero?)) } + .reject { |attrs| held.include?(phone_key(attrs[:value])) } + build_primary_first(person.contact_methods, attributes) + end + + def build_primary_first(association, attributes) + has_primary = association.any?(&:primary?) + attributes.each_with_index { |attrs, index| association.build(attrs.merge(primary: index.zero? && !has_primary)) } + end + + def address_key(street, city) + [ street, city ].map { |part| part.to_s.squish.downcase } + end + + def phone_key(value) + value.to_s.gsub(/\D/, "") end def address_attributes(fields) diff --git a/config/features.yml b/config/features.yml index 87e4b7e18d..7b817c6e83 100644 --- a/config/features.yml +++ b/config/features.yml @@ -4642,6 +4642,7 @@ group, and every address and phone number the old record held. pro_tips: - "A contact who already has a portal record is linked to it rather than duplicated — matching takes the same name and the same email." + - "A linked person's blank fields are filled in from FileMaker. Where FileMaker says something different, the portal's value is kept and a comment on the person lists what FileMaker had." - "A contact with no email is linked to a portal person of the same name who also has no email." - "A person can carry several FileMaker codes, so a contact who matches someone already linked to another FileMaker record is added to their list." - "FileMaker's old words for who someone serves are translated through the recorded tag aliases, so \"Student Services\" becomes Education and \"Families\" tags every age range." diff --git a/lib/tasks/fm_import_people.rake b/lib/tasks/fm_import_people.rake index 39c73a1804..262b93aead 100644 --- a/lib/tasks/fm_import_people.rake +++ b/lib/tasks/fm_import_people.rake @@ -1,7 +1,7 @@ # frozen_string_literal: true # Create people from the FileMaker Rolodex archive (fm_rolodexes, loaded by fm:import). -# See PersonServices::ImportFromRolodex for the match/link/create rules. Idempotent: +# See PersonServices::ImportFromRolodex for the match/link/fill/create rules. Idempotent: # a row whose ID is already some person's filemaker_code is left alone. # # Dry-run by default. Set DRY_RUN=false to write changes; VERBOSE=true lists every row. @@ -27,6 +27,11 @@ namespace :fm do messages.each { |message| puts " #{message}" } end + if import.differences.any? + puts "\nKept the portal's value where FileMaker differs (#{import.differences.size}, also left as a comment on each person):" + import.differences.each { |difference| puts " #{difference}" } + end + if import.unmatched_tags.any? puts "\nTerms no sector, age group or recorded translation answers to:" import.unmatched_tags.sort_by { |name, count| [ -count, name ] }.each { |name, count| puts " #{count.to_s.rjust(5)} #{name}" } diff --git a/spec/services/person_services/import_from_rolodex_spec.rb b/spec/services/person_services/import_from_rolodex_spec.rb index 48149d0601..de54a681c8 100644 --- a/spec/services/person_services/import_from_rolodex_spec.rb +++ b/spec/services/person_services/import_from_rolodex_spec.rb @@ -107,6 +107,33 @@ def results expect(person.sectorable_items).to be_empty end + it "makes the first sector a primary answer names the primary, the rest additional" do + homelessness = create(:sector, :published, name: "Homelessness") + create(:tag_alias, name: "Domestic Violence & Homelessness", aliasable: domestic_violence) + create(:tag_alias, name: "Domestic Violence & Homelessness", aliasable: homelessness) + rolodex("00058", EMail: "ada@example.com", PrimaryServiceArea: "Domestic Violence & Homelessness") + + described_class.new.call + person = Person.find_by!(filemaker_code: "00058") + + expect(person.primary_sector).to eq(domestic_violence) + expect(person.sectorable_items.where(is_primary: false).map(&:sector)).to eq([ homelessness ]) + end + + it "adds a linked person's tags without replacing the primary they already have" do + person = create(:person, first_name: "Ada", last_name: "Lovelace", email: "ada@example.com") + person.tag_sectors(primary_ids: [ education.id ], additional_ids: []) + rolodex("00059", EMail: "ada@example.com", PrimaryServiceArea: "Domestic Violence", PrimaryAgeGroup: "Adults") + + import = described_class.new.call + person.reload + + expect(person.primary_sector).to eq(education) + expect(person.sectorable_items.where(is_primary: false).map(&:sector)).to eq([ domestic_violence ]) + expect(person.primary_age_groups).to eq([ adults ]) + expect(import.differences.sole).to include("Primary sector", "Domestic Violence", "Education") + end + it "tags every age range for families without marking one primary" do rolodex("00057", EMail: "ada@example.com", PrimaryAgeGroup: "Families") @@ -229,6 +256,54 @@ def results expect(Person.find_by!(filemaker_code: "00023").date_of_birth).to eq(Date.new(1983, 10, 5)) end + describe "filling a linked person from the archive" do + let!(:person) { create(:person, first_name: "Ada", last_name: "Lovelace", email: "ada@example.com", pronouns: "she/her") } + + it "fills the fields the person has blank" do + rolodex("00070", EMail: "ada@example.com; ada@work.example.com", DateOfBirth: "12/10/1815", + BestTimeToCall: "Mornings", Comments: "Met at training") + + described_class.new.call + + expect(person.reload).to have_attributes(date_of_birth: Date.new(1815, 12, 10), best_time_to_call: "Mornings", + notes: "Met at training", email_2: "ada@work.example.com") + end + + it "keeps the person's value where the archive differs and notes the difference" do + rolodex("00071", EMail: "ada@example.com", Pronouns: "they/them") + + import = described_class.new.call + + expect(person.reload.pronouns).to eq("she/her") + expect(person.comments.sole.body).to include("00071", "Pronouns", "they/them", "she/her") + expect(import.differences.sole).to include("00071", "##{person.id}", "Pronouns", "they/them", "she/her") + end + + it "doesn't note a value that only differs in case or spacing" do + rolodex("00072", EMail: "ada@example.com", Pronouns: " She/Her ") + + import = described_class.new.call + + expect(person.comments).to be_empty + expect(import.differences).to be_empty + end + + it "adds the archive's addresses and phones the person doesn't have, keeping their primary" do + create(:address, addressable: person, street_address: "1 Main St", city: "Los Angeles", state: "CA", primary: true) + FmAddress.create!(fm_id: "A70", data: { "AddrsLine1" => "1 main st", "AddrsCity" => "Los Angeles", "AddrsState" => "CA" }) + FmAddress.create!(fm_id: "A71", data: { "RolodexID" => "00073", "AddrsLine1" => "2 Second St", + "AddrsCity" => "Oakland", "AddrsState" => "CA" }) + FmPhoneNumber.create!(fm_id: "P70", data: { "PhoneNumber" => "555-0100" }) + rolodex("00073", EMail: "ada@example.com", PrimaryAddrsID: "A70", PrimaryPhoneID: "P70") + + described_class.new.call + + expect(person.addresses.order(:id).pluck(:street_address, :primary)) + .to eq([ [ "1 Main St", true ], [ "2 Second St", false ] ]) + expect(person.contact_methods.pluck(:value, :primary)).to eq([ [ "555-0100", true ] ]) + end + end + it "links an existing person with the same name and email instead of creating one" do person = create(:person, first_name: "ada", last_name: "LOVELACE", email: "Ada@Example.com") rolodex("00008", EMail: "ada@example.com")