diff --git a/AGENTS.md b/AGENTS.md index 401843082a..fa22fbe5ab 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -62,7 +62,7 @@ This codebase (Rails 8.1) | `app/decorators/` | Draper decorators for view logic | ~50 files | | `app/policies/` | ActionPolicy authorization rules | ~63 files | | `app/presenters/` | Presentation objects | 6 files | -| `app/helpers/` | View helpers | ~47 files | +| `app/helpers/` | View helpers | ~48 files | | `app/mailers/` | ActionMailer classes | 6 files | | `app/inputs/` | Custom SimpleForm inputs | 1 file | @@ -146,6 +146,7 @@ This codebase (Rails 8.1) | `Communicable` | Gives a record the `notifications` association, nested attributes, a default `communications_scope`, and a `before_validation` addressing a hand-logged communication to the record's `communications_email`. Included by every model whose form renders `shared/_comments_and_communications` | | `AuthorCreditable` | Author attribution. Credits are formatted by the credited **person's profile** (`Person#display_name_preference`), not by the record. The record's `author_credit_preference` is the consent snapshot taken at create time and is human-editable only on the author credit divergences page. Anonymity is a one-way latch: the profile or the record can set it, neither can strip it from the other. `credits_to_org` flips the unattributed label to "AWBW Staff"; `credits_creator` (idea models, workshop logs) credits the submitting account's person when no author is named | | `Featureable` | `featured`, `publicly_featured` scopes | +| `FilemakerCoded` | Reads `filemaker_code` as the trimmed, comma-separated **list** it is (Person, Organization): `#filemaker_codes` for the set a record answers to, `.join_filemaker_codes` to combine any mix of strings/lists into one normalized column value (both dedupers union the codes on merge). Nothing should compare the whole column against a single code — the `filemaker_code_links` helper renders one archive link per code | | `Mentioner` | ActionText @mention extraction and grouping | | `NameFilterable` | Name-based filtering | | `PayChargeExtensions` | Included into `Pay::Charge` (via `config/initializers/pay.rb`): turns a Stripe charge into an AWBW `Payment` + `Allocation`, resolving the membership invoice it pays | diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index 6f3bd41b24..2dcee25c9a 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -377,6 +377,7 @@ def dedupe_config first_name legal_first_name last_name email email_type email_2 email_2_type date_of_birth pronouns filemaker_code member_since notes ], + union_columns: %w[filemaker_code], # Merging two people who each have a login leaves the kept person with both # (users.person_id has no unique constraint); both logins still sign in to the # one record. Surface it so the admin isn't surprised — it doesn't block. @@ -397,6 +398,7 @@ def dedupe_config # they become indistinguishable), capture which primary sector + age range to # keep: the survivor's own, or — when it has none — the deleted person's. merge_keeper: ->(keep, delete) { + keep.filemaker_code = Person.join_filemaker_codes(keep.filemaker_code, delete.filemaker_code) @keeper_primary_sector_id = keep.sectorable_items.find_by(is_primary: true)&.sector_id || delete.sectorable_items.find_by(is_primary: true)&.sector_id @keeper_primary_age_category_id = keep.age_range_categorizable_items.find_by(is_primary: true)&.category_id || diff --git a/app/helpers/filemaker_helper.rb b/app/helpers/filemaker_helper.rb new file mode 100644 index 0000000000..e5fff83072 --- /dev/null +++ b/app/helpers/filemaker_helper.rb @@ -0,0 +1,18 @@ +module FilemakerHelper + # One archive link per FileMaker code a record carries, since a record can answer to + # several (see FilemakerCoded). A code the archive has no row for renders as plain + # text. `label` prefixes each link, for a button or hint that has to read as an action. + def filemaker_code_links(record, archive:, table:, link_class: nil, label: nil) + codes = record.filemaker_codes + return if codes.empty? + + stored = archive.where(fm_id: codes).pluck(:fm_id).to_set + + safe_join(codes.map do |code| + text = [ label, code ].compact.join(" ") + next tag.span(text) unless stored.include?(code) + + link_to text, fm_archive_path(code, table: table), class: link_class, target: "_blank", rel: "noopener" + end, " ") + end +end diff --git a/app/models/concerns/filemaker_coded.rb b/app/models/concerns/filemaker_coded.rb new file mode 100644 index 0000000000..ce5bb971ad --- /dev/null +++ b/app/models/concerns/filemaker_coded.rb @@ -0,0 +1,23 @@ +# FileMaker is the source of truth, and one record can answer to more than one +# FileMaker code — two records that each mapped to a FileMaker row get merged, or the +# archive itself holds several rows for one person. The `filemaker_code` column holds +# them as a trimmed, comma-separated list; this is the seam for reading and combining +# them, so nothing compares the whole column against a single code. +module FilemakerCoded + extend ActiveSupport::Concern + + def filemaker_codes + self.class.split_filemaker_codes(filemaker_code) + end + + class_methods do + def split_filemaker_codes(value) + value.to_s.split(",").map(&:strip).reject(&:blank?).uniq + end + + # A single normalized column value from any mix of code strings/lists. + def join_filemaker_codes(*values) + values.flatten.flat_map { |value| split_filemaker_codes(value) }.uniq.sort.join(", ").presence + end + end +end diff --git a/app/models/organization.rb b/app/models/organization.rb index c7b15c36a6..91af0f0cd4 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -1,6 +1,7 @@ class Organization < ApplicationRecord include RemoteSearchable, TagFilterable, Trendable, WindowsTypeFilterable, SectorsTaggable, AgeGroupTaggable # Publishable include Communicable + include FilemakerCoded belongs_to :organization_status belongs_to :created_by, class_name: "User", optional: true belongs_to :updated_by, class_name: "User", optional: true @@ -354,23 +355,6 @@ def rollup_cache_version remote_searchable_by :name - # FileMaker is the source of truth, and a record can carry more than one - # FileMaker code (e.g. when two orgs that each mapped to a FileMaker record are - # merged). The `filemaker_code` column holds them as a trimmed, comma-separated - # list; these are the seam for reading and combining them. - def filemaker_codes - Organization.split_filemaker_codes(filemaker_code) - end - - def self.split_filemaker_codes(value) - value.to_s.split(",").map(&:strip).reject(&:blank?).uniq - end - - # A single normalized column value from any mix of code strings/lists. - def self.join_filemaker_codes(*values) - values.flatten.flat_map { |value| split_filemaker_codes(value) }.uniq.sort.join(", ").presence - end - # Returns the website as a clickable, scheme-qualified URL — prepending # https:// to a bare domain like "awbw.org" — or nil when the value is blank or # not a usable web address. Drives the external links on the org profile and diff --git a/app/models/person.rb b/app/models/person.rb index dc95d257f3..cef678c39f 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -1,6 +1,7 @@ class Person < ApplicationRecord include RemoteSearchable, TagFilterable, Trendable, WindowsTypeFilterable, SectorsTaggable, AgeGroupTaggable, StaffTaggable include Communicable + include FilemakerCoded pay_customer default_payment_processor: :stripe diff --git a/app/services/person_services/duplicate_finder.rb b/app/services/person_services/duplicate_finder.rb index 21423c962c..792435c08a 100644 --- a/app/services/person_services/duplicate_finder.rb +++ b/app/services/person_services/duplicate_finder.rb @@ -60,8 +60,12 @@ def reasons_for(records) end def filemaker_reasons(records) - codes = records.filter_map { |person| person.filemaker_code.presence }.uniq.sort + code_sets = records.map(&:filemaker_codes) + codes = code_sets.flatten.uniq.sort return [] if codes.empty? + + shared = codes.select { |code| code_sets.count { |set| set.include?(code) } > 1 } + return [ "Same FileMaker code (#{shared.join(", ")})" ] if shared.any? return [ "⚠ Different FileMaker codes (#{codes.join(", ")}) — verify these are the same person" ] if codes.size > 1 [ "Same FileMaker code (#{codes.first})" ] @@ -91,8 +95,7 @@ def email_keys(person) end def filemaker_key(person) - code = person.filemaker_code.to_s.strip.downcase - "fm|#{code}" if code.present? + person.filemaker_codes.map { |code| "fm|#{code.downcase}" } end def dob_key(person) diff --git a/app/views/organizations/_form.html.erb b/app/views/organizations/_form.html.erb index 217f56f7ae..7d9c15319b 100644 --- a/app/views/organizations/_form.html.erb +++ b/app/views/organizations/_form.html.erb @@ -183,11 +183,10 @@ input_html: { class: "rounded-md border-gray-300 bg-blue-100 shadow-sm focus:ring-blue-500 focus:border-blue-500" } %> - <%- if params[:admin] == "true" && f.object.filemaker_code.present? && FmProject.exists?(fm_id: f.object.filemaker_code) %> + <%- if params[:admin] == "true" %>

- <%= link_to "View filemaker archive", - fm_archive_path(f.object.filemaker_code, table: "projects"), - class: "text-blue-600 hover:text-blue-800 underline" %> + <%= filemaker_code_links(f.object, archive: FmProject, table: "projects", label: "View archive", + link_class: "text-blue-600 hover:text-blue-800 underline") %>

<% end %> <% high_profile_gem = tag.i(class: "fa-solid fa-gem text-purple-600", aria: { hidden: true }) %> diff --git a/app/views/organizations/edit.html.erb b/app/views/organizations/edit.html.erb index ec7d1d7e06..31a36d4c4b 100644 --- a/app/views/organizations/edit.html.erb +++ b/app/views/organizations/edit.html.erb @@ -9,8 +9,9 @@ <% end %> <% end %> <% actions = capture do %> - <% if params[:admin] == "true" && @organization.filemaker_code.present? && FmProject.exists?(fm_id: @organization.filemaker_code) %> - <%= link_to "FM Project", fm_archive_path(@organization.filemaker_code, table: "projects"), class: "admin-only bg-blue-100 text-sm #{eyebrow_link_class} px-2 py-1" %> + <% if params[:admin] == "true" %> + <%= filemaker_code_links(@organization, archive: FmProject, table: "projects", label: "FM project", + link_class: "admin-only bg-blue-100 text-sm #{eyebrow_link_class} px-2 py-1") %> <% end %> <%= link_to "Home", root_path, class: "text-sm #{eyebrow_link_class} px-2 py-1" %> <%= link_to "Organizations", organizations_path, class: "text-sm #{eyebrow_link_class} px-2 py-1" %> diff --git a/app/views/people/_admin_details.html.erb b/app/views/people/_admin_details.html.erb index c6a8c969db..959aebb3c8 100644 --- a/app/views/people/_admin_details.html.erb +++ b/app/views/people/_admin_details.html.erb @@ -98,13 +98,8 @@
FileMaker code
- <% if person.filemaker_code.present? %> - <% if FmRolodex.exists?(fm_id: person.filemaker_code) %> - <%= link_to person.filemaker_code, fm_archive_path(person.filemaker_code, table: "rolodexes"), class: "text-blue-600 underline hover:text-blue-800", target: "_blank", rel: "noopener" %> - <% else %> - <%= person.filemaker_code %> - <% end %> - <% else %>—<% end %> + <%= filemaker_code_links(person, archive: FmRolodex, table: "rolodexes", + link_class: "text-blue-600 underline hover:text-blue-800") || "—" %>
Racial/ethnic identity
<%= person.racial_ethnic_identity.presence || "—" %>
diff --git a/app/views/people/_form.html.erb b/app/views/people/_form.html.erb index 1c86fe17f0..a2f373c923 100644 --- a/app/views/people/_form.html.erb +++ b/app/views/people/_form.html.erb @@ -469,11 +469,10 @@ label: "FileMaker code", input_html: { class: "w-full" }, wrapper_html: { class: "w-full" } %> - <%- if params[:admin] == "true" && f.object.filemaker_code.present? && FmRolodex.exists?(fm_id: f.object.filemaker_code) %> + <%- if params[:admin] == "true" %>

- <%= link_to "View filemaker archive", - fm_archive_path(f.object.filemaker_code, table: "rolodexes"), - class: "text-blue-600 hover:text-blue-800 underline" %> + <%= filemaker_code_links(f.object, archive: FmRolodex, table: "rolodexes", label: "View archive", + link_class: "text-blue-600 hover:text-blue-800 underline") %>

<% end %> diff --git a/app/views/people/edit.html.erb b/app/views/people/edit.html.erb index 2ce6d585fc..0120c27367 100644 --- a/app/views/people/edit.html.erb +++ b/app/views/people/edit.html.erb @@ -35,8 +35,9 @@ <% if allowed_to?(:index?, with: Admin::AhoyActivityPolicy) %> <%= link_to "Activity", admin_activities_events_path(person_id: @person.id, time_period: "all_time", audience: %w[visitors users staff], hide_account: "1", hide_interactions: "1"), class: "admin-only bg-blue-100 text-sm #{eyebrow_link_class} px-2 py-1" %> <% end %> - <% if params[:admin] == "true" && @person.filemaker_code.present? && FmRolodex.exists?(fm_id: @person.filemaker_code) %> - <%= link_to "Rolodex", fm_archive_path(@person.filemaker_code, table: "rolodexes"), class: "admin-only bg-blue-100 text-sm #{eyebrow_link_class} px-2 py-1" %> + <% if params[:admin] == "true" %> + <%= filemaker_code_links(@person, archive: FmRolodex, table: "rolodexes", label: "Rolodex", + link_class: "admin-only bg-blue-100 text-sm #{eyebrow_link_class} px-2 py-1") %> <% end %> <%= link_to "Change password", change_password_path, class: "text-sm #{eyebrow_link_class} px-2 py-1" %> <%= link_to "Home", root_path, class: "text-sm #{eyebrow_link_class} px-2 py-1" %> diff --git a/config/features.yml b/config/features.yml index 2575bb4e5a..6b99f3d9c2 100644 --- a/config/features.yml +++ b/config/features.yml @@ -4163,3 +4163,16 @@ details on ones nobody has touched. pro_tips: - "After syncing, the message tells you how many edited features were kept as-is." + +- name: "Merging people keeps both FileMaker codes" + area: people + display_status: admin_facing + released_on: 2026-10-12 + pr_number: 2647 + action_path: "/people/dedupe_index" + summary: >- + Merging two people who each came from FileMaker now keeps both of their + FileMaker codes on the kept person instead of dropping one. + pro_tips: + - "A record can answer to several codes — they're shown as a comma-separated list, and each one links to its own row in the archive." + - "The duplicate list now reads two records as a match when their codes overlap, rather than flagging them as conflicting." diff --git a/spec/models/person_spec.rb b/spec/models/person_spec.rb index 4bf10d69c3..fe2d83f899 100644 --- a/spec/models/person_spec.rb +++ b/spec/models/person_spec.rb @@ -979,6 +979,26 @@ def answer(identifier, value) end end + describe "FileMaker codes" do + it "parses a trimmed, de-duplicated list from the column" do + person = build(:person, filemaker_code: " 00006 , 00012,00006 ") + + expect(person.filemaker_codes).to eq(%w[00006 00012]) + end + + it "is empty when the column is blank" do + expect(build(:person, filemaker_code: nil).filemaker_codes).to eq([]) + end + + it "combines mixed strings and lists into one sorted normalized value" do + expect(Person.join_filemaker_codes("00012, 00006", " 00031 ", nil, "00006")).to eq("00006, 00012, 00031") + end + + it "returns nil when there is nothing to combine" do + expect(Person.join_filemaker_codes(nil, "")).to be_nil + end + end + describe ".find_by_any_email" do it "matches on email, email_2, or the login email, case-insensitively" do by_primary = create(:person, user: nil, email: "primary@example.com", email_2: nil) diff --git a/spec/requests/dedupable_spec.rb b/spec/requests/dedupable_spec.rb index ef6ab0e682..e6f48cdb5a 100644 --- a/spec/requests/dedupable_spec.rb +++ b/spec/requests/dedupable_spec.rb @@ -489,6 +489,18 @@ expect(keep.reload.notes).to eq("Canonical record") end + it "combines both people's FileMaker codes onto the keeper" do + keep.update!(filemaker_code: "00006") + delete_rec.update!(filemaker_code: "00012") + + post dedupe_perform_people_path, params: { + person_to_delete_id: delete_rec.id, + person_to_keep_id: keep.id + } + + expect(keep.reload.filemaker_code).to eq("00006, 00012") + end + # invoicee is polymorphic, so it carries no DB foreign key for the schema scan to # find — without Person's `has_many :invoices, as: :invoicee` the safeguard sees an # unhandled reference and refuses the merge outright. diff --git a/spec/services/person_services/duplicate_finder_spec.rb b/spec/services/person_services/duplicate_finder_spec.rb index 76bf4ee31d..7da051e61a 100644 --- a/spec/services/person_services/duplicate_finder_spec.rb +++ b/spec/services/person_services/duplicate_finder_spec.rb @@ -64,6 +64,16 @@ def person(attrs = {}) expect(group.reasons).to include("Same FileMaker code (FM777)") end + it "groups people whose FileMaker code lists overlap" do + a = person(first_name: "Erin", last_name: "Three", email: "e3@example.com", filemaker_code: "FM777") + b = person(first_name: "Erin", last_name: "Four", email: "e4@example.com", filemaker_code: "FM888, FM777") + + group = described_class.new.groups.first + + expect(group.records).to contain_exactly(a, b) + expect(group.reasons).to include("Same FileMaker code (FM777)") + end + it "flags different FileMaker codes on records grouped by another signal as a caution" do person(first_name: "Fay", last_name: "Nguyen", email: "fay@example.com", filemaker_code: "FM1") person(first_name: "Fay", last_name: "Nguyen", email: "fay2@example.com", filemaker_code: "FM2")