Add a users (logins) deduper

AGENTS.md

@@ -49,7 +49,7 @@ This codebase (Rails 8.1)
 | Directory | Purpose | Count |
 |---|---|---|
 | `app/models/` | ActiveRecord models | ~90 files |
-| `app/services/` | Service objects and POROs (e.g. `MoneyFormatter` for currency display, `StoryImporter` for the stories spreadsheet import) | ~72 files |
+| `app/services/` | Service objects and POROs (e.g. `MoneyFormatter` for currency display, `StoryImporter` for the stories spreadsheet import) | ~73 files |
 | `app/jobs/` | SolidQueue background jobs | 6 files |
 | `app/models/concerns/` | Shared model modules | 23 concerns |
 
@@ -248,7 +248,7 @@ action, or `authorize! :workshop, to: :summary?`).
 - `PersonInviter` — Invites a single person to the portal: creates a user account if they have none, then sends the welcome/invite email attributed to the sender. Backs the bulk "Send Portal invite emails" flow on the reminders page (`?mode=invite`)
 - `FormBuilderService` — Builds configurable forms from composable sections with per-field visibility
 - `FormResponseAggregator` — Rolls up every submission to one form into a per-question report for the `/forms/:id/results` page (`FormsController#results`): selectable questions become chartable `[ label, count ]` tallies (sector/age-group ids resolved to names, `Other: <text>` write-ins split into their own list), geographic fields (`mailing_state`/`organization_state`/`ce_license_issuing_state` → US map, `mailing_country`/`organization_country` → world map) become choropleths, free-text questions become newest-first answer lists, file questions a count. Returns `FieldReport` structs in form order; rendered via the shared `events/_breakdown_card` partial (pie/bar/map/world_map) plus `forms/_text_responses_card`
-- `ModelDeduper` — Deduplication logic. On a natural-key collision it now merges the losing row into the survivor (recursively, via itself) so the loser's own children move first — instead of destroying the loser and cascading them away or aborting on a protective `before_destroy` (e.g. a professional license with CE registrations). It finds reassignable children two ways: the model's own `has_many` declarations, and a scan of the DB's foreign keys. **A polymorphic child carries no foreign key**, so it is reachable only through an explicit `has_many … as:` — without one, `#unhandled_references` sees it and the merge is refused outright (this is why `Person`/`Organization` declare `has_many :invoices, as: :invoicee`)
+- `ModelDeduper` — Deduplication logic. On a natural-key collision it now merges the losing row into the survivor (recursively, via itself) so the loser's own children move first — instead of destroying the loser and cascading them away or aborting on a protective `before_destroy` (e.g. a professional license with CE registrations). It finds reassignable children two ways: the model's own `has_many` declarations, and a scan of the DB's foreign keys. **A polymorphic child carries no foreign key**, so it is reachable only through an explicit `has_many … as:` — without one, `#unhandled_references` sees it and the merge is refused outright (this is why `Person`/`Organization` declare `has_many :invoices, as: :invoicee`). A config can also pass `reference_columns:` — column names that point at the model in any table without a DB foreign key (the users deduper names `user_id`/`created_by_id`/`updated_by_id`/`welcome_instructions_sent_by_id`). Self-references on the model's own table follow the keeper, except the keeper's own reference to the duplicate, which clears
 - `ContinuingEducationDeduper` — Collapses duplicate CE registrations left sharing one event registration + license after a merge, moving the losers' allocations (payments) and comments onto the survivor. Keeps the real cost (larger of the two) rather than inventing one to cover the combined payments — if they now exceed it, `ContinuingEducationRegistration#over_allocated?` flags it (`ContinuingEducationRegistrationDecorator#overpayment_badge` → "Check payments") for an admin. CE has no DB unique index, so `ModelDeduper` never compares CE rows and just moves them all onto the keeper; a unique index is deliberately avoided because it would force the engine to move CE via validated `update!`, which trips `ContinuingEducationRegistration#license_belongs_to_registrant` mid-merge. Run from the people deduper's `after_merge`
 - `AddressDeduper` — After a person or organization merge, folds duplicate addresses the generic merge moves onto the survivor wholesale (addresses have no unique index, so nothing collapses them). Groups the survivor's addresses by a normalized identity signature (`street_address`, `city`, `state`, `zip_code`, `country`, `county`, `district`, `address_type`, `locality`) and folds each duplicate into its match via `ModelDeduper#merge`, which repoints the folded address's inbound FKs — `contact_methods.address_id`, `invoices.bill_to_address_id`, `affiliations.organization_address_id` — onto the survivor. Within a group the survivor is the copy still in use — active before inactive, then the primary, then the oldest — since `Address.active` drives bill-to addresses, an org's program location and the geographic breakdowns; columns outside the identity (`CARRIED_COLUMNS` — `phone` and the three `la_*` districts) are copied up from the folded duplicate when the survivor has none. Then settles a single primary matching the signature captured in the deduper's `merge_keeper` (the survivor's own pre-merge primary address, else the deleted record's), demoting the rest. Runs in the people and organizations dedupers' `after_merge`
 - `PersonServices::ReconcilePrimaryDesignations` — After a person merge, keeps the survivor's own primary sector + primary age range and demotes the duplicate's, which would otherwise leave the kept person with two primaries and trip Person's single-primary validations. The survivor's pre-merge primary ids are captured in the people deduper's `merge_keeper` (they're indistinguishable once merged); reconciliation runs in `after_merge`
@@ -338,6 +338,7 @@ Both flows record on `EventRegistrationOrganization` what the submission meant f
 
 - `UserServices::ProcessEmailChange` — Email change processing
 - `UserServices::ProcessEmailManualConfirm` — Manual email confirmation
+- `UserServices::DuplicateFinder` — Candidate groups for the users (logins) deduper: logins sharing one person (e.g. left by a person merge); a login that can sign in leads, most recently updated first, so the deduper suggests the login the person uses. The deduper's pickers query `SearchController`'s virtual `user_any_status` model (every login via `User.activity_search`, labelled with `UserDecorator#access_statuses`)
 
 ## Decorators (Draper)
 

app/controllers/concerns/dedupable.rb

@@ -38,7 +38,8 @@ def dedupe_preview
       return redirect_to url_for(action: :dedupe_index),
         alert: "#{mc.model_name.human} not found (ID: #{missing.join(', ')})."
     end
-    deduper = ModelDeduper.new(model_class: mc, movable_attachments: config[:movable_attachments])
+    deduper = ModelDeduper.new(model_class: mc, movable_attachments: config[:movable_attachments],
+                               reference_columns: config[:reference_columns])
     @reassignment_delete = deduper.reassignment_preview(@record_to_delete)
     @reassignment_keep = deduper.reassignment_preview(@record_to_keep)
     @lost_references = deduper.lost_references(@record_to_delete)
@@ -60,10 +61,7 @@ def dedupe_update_keep
     record = mc.find(params[:id])
     keep_param_key = "#{mn}_to_keep"
 
-    if params[keep_param_key].present?
-      editable = mc.column_names - %w[id created_at updated_at legacy_id]
-      record.update!(params.require(keep_param_key).permit(editable))
-    end
+    record.update!(params.require(keep_param_key).permit(dedupe_editable_columns(config))) if params[keep_param_key].present?
 
     head :ok
   rescue ActionPolicy::Unauthorized
@@ -81,7 +79,8 @@ def dedupe_perform
     record_to_delete = mc.find(params["#{mn}_to_delete_id"])
     record_to_keep = mc.find(params["#{mn}_to_keep_id"])
 
-    unhandled = ModelDeduper.new(model_class: mc).unhandled_references(record_to_delete)
+    unhandled = ModelDeduper.new(model_class: mc, reference_columns: config[:reference_columns])
+                            .unhandled_references(record_to_delete)
     if unhandled.any?
       tables = unhandled.map { |ref| ref[:table] }.uniq.join(", ")
       return redirect_to url_for(action: :dedupe_index),
@@ -95,23 +94,23 @@ def dedupe_perform
 
     keep_param_key = "#{mn}_to_keep"
     if params[keep_param_key].present?
-      editable = mc.column_names - %w[id created_at updated_at legacy_id]
-      record_to_keep.assign_attributes(params.require(keep_param_key).permit(editable))
+      record_to_keep.assign_attributes(params.require(keep_param_key).permit(dedupe_editable_columns(config)))
     end
 
     # Combine values that must survive the merge rather than be replaced wholesale
     # (e.g. an org's FileMaker codes), so the kept record keeps both records' links.
     config[:merge_keeper]&.call(record_to_keep, record_to_delete)
 
     deduper = ModelDeduper.new(model_class: mc, logger: Rails.logger, dry_run: false, min_usage: 0,
-                               movable_attachments: config[:movable_attachments])
+                               movable_attachments: config[:movable_attachments],
+                               reference_columns: config[:reference_columns])
 
     if respond_to?(:track_event, true)
       track_event("dedupe.#{mn}", {
         resource_type: mc.name,
         resource_id: record_to_keep.id,
-        deleted_record: record_to_delete.attributes,
-        kept_record: { id: record_to_keep.id, name: record_to_keep.name },
+        deleted_record: dedupe_parameter_filter.filter(record_to_delete.attributes),
+        kept_record: { id: record_to_keep.id, name: dedupe_record_label(config, record_to_keep) },
         associations_moved: deduper.reassignment_counts(record_to_delete).values.sum
       })
     end
@@ -130,7 +129,7 @@ def dedupe_perform
 
     label = mc.model_name.human.pluralize
     redirect_to url_for(action: :index),
-      notice: "#{label} merged successfully. '#{record_to_delete.name}' was merged into '#{record_to_keep.name}'."
+      notice: "#{label} merged successfully. '#{dedupe_record_label(config, record_to_delete)}' was merged into '#{dedupe_record_label(config, record_to_keep)}'."
   rescue ActionPolicy::Unauthorized
     raise
   rescue StandardError => e
@@ -140,6 +139,20 @@ def dedupe_perform
 
   private
 
+  # The preview only renders editable_columns when a config lists them, so only those are accepted.
+  def dedupe_editable_columns(config)
+    config[:editable_columns].presence || (config[:model_class].column_names - %w[id created_at updated_at legacy_id])
+  end
+
+  def dedupe_record_label(config, record)
+    (config[:record_label] || :name.to_proc).call(record)
+  end
+
+  # Keeps credentials (a login's password hash and tokens) out of the tracked deleted record.
+  def dedupe_parameter_filter
+    ActiveSupport::ParameterFilter.new(%i[passw crypt salt token secret otp])
+  end
+
   # Subclasses must implement this, returning a hash with:
   #   model_class:        The ActiveRecord model (e.g. Category)
   #   domain:             Symbol for DomainTheme (e.g. :categories) (optional, derived from model)
@@ -150,6 +163,16 @@ def dedupe_perform
   #   field_notes:        Hash of { column_name => hint } rendered under the keeper's field, to guide
   #                       an admin on what to enter (e.g. full_name is a legacy free-text author) (optional)
   #   deprecated_columns: Array of column names to render muted with a "Deprecated" badge (optional)
+  #   editable_columns:   Array of column names the preview shows and lets an admin edit on the keeper;
+  #                       only these are accepted on save (optional, defaults to every column)
+  #   read_only_columns:  Array of column names shown side by side for comparison but not editable —
+  #                       e.g. a login's email and sign-in history (optional)
+  #   record_label:       Lambda(record) naming a record across the deduper when #name doesn't tell two
+  #                       apart, e.g. two logins of one person (optional, defaults to #name)
+  #   search_model:       Search model the index pickers query (optional, defaults to the model's own
+  #                       remote search), e.g. "user_any_status" to reach logins that can't sign in
+  #   reference_columns:  Array of column names that reference this model in any table but carry no DB
+  #                       foreign key, so the merge moves them too (e.g. created_by_id stamps) (optional)
   #   movable_attachments: Array of has_one_attached names to move to the keeper when it has none
   #                       (otherwise dropped with the deleted record), e.g. %w[thumbnail header] (optional)
   #   preview_images:     Lambda(record) returning that record's attached images (ActiveStorage
@@ -198,8 +221,10 @@ def build_dedupe_vars(config)
       delete_id_param: "#{mn}_to_delete_id",
       keep_id_param: "#{mn}_to_keep_id",
       keep_param_key: "#{mn}_to_keep".to_sym,
-      search_model: mn,
+      search_model: config[:search_model] || mn,
       editable_columns: config[:editable_columns],
+      read_only_columns: Array(config[:read_only_columns]).map(&:to_s),
+      record_label: config[:record_label] || :name.to_proc,
       union_columns: Array(config[:union_columns]).map(&:to_s),
       belongs_to_options: opts.is_a?(Proc) ? opts.call : (opts || {}),
       remote_select_options: config[:remote_select_options] || {},

app/controllers/search_controller.rb

@@ -4,9 +4,12 @@ class SearchController < ApplicationController
   # Virtual "model" that searches people and organizations together for the
   # compound payer / funder pickers, listing organizations first.
   COMPOUND_MODEL = "person_or_organization".freeze
+  # Every login, whatever its access, labelled with why it can't sign in — for the users deduper.
+  USER_ANY_STATUS_MODEL = "user_any_status".freeze
 
   def index
     return render json: compound_results if params[:model] == COMPOUND_MODEL
+    return render json: user_any_status_results if params[:model] == USER_ANY_STATUS_MODEL
 
     model_class = allowed_model(params[:model])
     unless model_class
@@ -54,6 +57,17 @@ def compound_results
     records.map(&:compound_search_label)
   end
 
+  def user_any_status_results
+    authorize! User, to: :search?
+
+    query = params[:q].to_s.strip
+    return [] if query.blank?
+
+    records = authorized_scope(User.activity_search(query))
+    records = records.where.not(id: params[:exclude].split(",").map(&:to_i)) if params[:exclude].present?
+    records.includes(:person).limit(25).map { |user| user.decorate.search_label_with_status }
+  end
+
   def allowed_model(model_param)
     {
       "person"   => Person,

app/controllers/users_controller.rb

@@ -1,4 +1,6 @@
 class UsersController < ApplicationController
+  include AhoyTracking, Dedupable
+
   before_action :set_user, only: [ :show, :edit, :update, :destroy,
                                    :toggle_lock_status, :confirm_email,
                                    :send_welcome_instructions, :send_reset_password_instructions,
@@ -321,6 +323,46 @@ def flow_diagram
 
   private
 
+  def dedupe_config
+    {
+      model_class: User,
+      domain: :users,
+      candidate_finder: -> { UserServices::DuplicateFinder.new.groups },
+      record_label: ->(user) { user.full_name_with_email },
+      search_model: SearchController::USER_ANY_STATUS_MODEL,
+      read_only_columns: %w[email super_user confirmed_at last_sign_in_at sign_in_count inactive locked_at],
+      editable_columns: %w[time_zone notes],
+      # Audit stamps and analytics carry no DB foreign key to users, so name them for the merge to move.
+      reference_columns: %w[user_id created_by_id updated_by_id welcome_instructions_sent_by_id],
+      record_extras: ->(user) {
+        last_sign_in = user.last_sign_in_at ? "last signed in #{user.last_sign_in_at.to_date}" : "never signed in"
+        [ user.super_user? && "Admin", *user.decorate.access_statuses, last_sign_in ].compact_blank.join(" · ")
+      },
+      merge_notes: ->(keep, delete) {
+        notes = [ "Signing in as #{delete.email} stops working after the merge — #{keep.email} stays the login. Swap to keep the other address." ]
+        notes << "#{delete.email} is an admin login and #{keep.email} isn't, so admin access ends with the merge." if delete.super_user? && !keep.super_user?
+        notes << "The kept login has no person, so it takes over #{delete.person.full_name}'s profile." if delete.person && !keep.person
+        notes
+      },
+      merge_blocks: ->(keep, delete) {
+        blocks = []
+        blocks << "You're signed in as #{delete.email}. Swap to keep it — you can't merge away your own login." if delete == current_user
+        if keep.person && delete.person && keep.person_id != delete.person_id
+          blocks << "These logins belong to different people (#{keep.person.full_name} and #{delete.person.full_name}). Merge the people first, then their logins."
+        end
+        blocks
+      },
+      merge_keeper: ->(keep, delete) {
+        keep.person_id ||= delete.person_id
+        @merged_login_id = delete.id
+      },
+      # paper_trail records the acting login as a string id, outside the foreign-key scan.
+      after_merge: ->(keep) {
+        PaperTrail::Version.where(whodunnit: @merged_login_id.to_s).update_all(whodunnit: keep.id.to_s)
+      }
+    }
+  end
+
   def set_user
     @user = User.find(params[:id])
   end

app/decorators/user_decorator.rb

@@ -29,6 +29,15 @@ def full_name
     end
   end
 
+  # Why this login can't sign in — empty when it can.
+  def access_statuses
+    [ ("Inactive" if inactive?), ("Locked" if locked_at.present?), ("Unconfirmed" if confirmed_at.nil?) ].compact
+  end
+
+  def search_label_with_status
+    { id: id, label: [ full_name_with_email, access_statuses.join(", ").presence ].compact.join(" · ") }
+  end
+
   def last_logged_in
     return "never" unless last_sign_in_at
     "#{h.time_ago_in_words(last_sign_in_at)} ago"

app/helpers/admin_cards_helper.rb

@@ -71,7 +71,8 @@ def deduper_cards
       custom_card("Dedupe organizations", dedupe_index_organizations_path, icon: "🧹", color: DomainTheme.color_for(:organizations), intensity: 100),
       custom_card("Dedupe categories", dedupe_index_categories_path, icon: "🧹", color: DomainTheme.color_for(:categories), intensity: 100),
       custom_card("Dedupe event registrations", dedupe_index_event_registrations_path, icon: "🧹", color: DomainTheme.color_for(:event_registrations), intensity: 100),
-      custom_card("Dedupe sectors", dedupe_index_sectors_path, icon: "🧹", color: DomainTheme.color_for(:sectors), intensity: 100)
+      custom_card("Dedupe sectors", dedupe_index_sectors_path, icon: "🧹", color: DomainTheme.color_for(:sectors), intensity: 100),
+      custom_card("Dedupe users", dedupe_index_users_path, icon: "🧹", color: DomainTheme.color_for(:users), intensity: 100)
     ]
   end