diff --git a/app/controllers/concerns/dedupable.rb b/app/controllers/concerns/dedupable.rb index a4f5d0440..33c03ab0b 100644 --- a/app/controllers/concerns/dedupable.rb +++ b/app/controllers/concerns/dedupable.rb @@ -115,6 +115,10 @@ def dedupe_perform # being deleted as a conflict. Atomic: a failed save rolls the merge back. ActiveRecord::Base.transaction do deduper.merge(record_to_keep, record_to_delete) + # After the duplicate's records have moved onto the keeper, reconcile any that + # need it (e.g. re-crediting a moved scholarship to the keeper's registrant). + # Runs inside the transaction so a failure rolls the whole merge back. + config[:after_merge]&.call(record_to_keep) record_to_keep.save! if record_to_keep.changed? end @@ -147,6 +151,13 @@ def dedupe_perform # merge_notes: Lambda(keep, delete) returning an array of informational (non-blocking) # strings to surface on the preview (e.g. "both people have a login") (optional) # record_extras: Lambda(record) returning extra detail string for index listing (optional) + # back_path: Override the index eyebrow's return path (e.g. when the deduper was + # opened from an event's registrants page, not the model index) (optional) + # back_label: Label for that eyebrow link, without the "← " prefix (optional) + # subtitle: Extra line under the index header, e.g. naming the scope being deduped (optional) + # after_merge: Lambda(keep) run inside the merge transaction AFTER the duplicate's records + # have moved onto the keeper — to reconcile anything the move leaves inconsistent + # (e.g. re-credit a moved scholarship to the keeper's registrant). Raise to roll back (optional) def dedupe_config raise NotImplementedError, "#{self.class} must implement #dedupe_config" end @@ -186,7 +197,10 @@ def build_dedupe_vars(config) field_notes: config[:field_notes] || {}, deprecated_columns: Array(config[:deprecated_columns]).map(&:to_s), preview_images: config[:preview_images], - record_extras: config[:record_extras] + record_extras: config[:record_extras], + back_path: config[:back_path], + back_label: config[:back_label], + subtitle: config[:subtitle] } end end diff --git a/app/controllers/event_registrations_controller.rb b/app/controllers/event_registrations_controller.rb index 319b1a3ea..ef2add260 100644 --- a/app/controllers/event_registrations_controller.rb +++ b/app/controllers/event_registrations_controller.rb @@ -1,4 +1,6 @@ class EventRegistrationsController < ApplicationController + include Dedupable + require "csv" # show redirects to slug URL; kept for backwards compatibility @@ -507,6 +509,49 @@ def set_form_variables private + def dedupe_config + # Opened from an event's registrants page (bulk actions) → scope the suggested + # duplicates to that event and return the eyebrow there; opened from the global + # registrations index → all registrations, default eyebrow. + dedupe_event = Event.find_by(id: params[:event_id]) + finder_scope = dedupe_event ? EventRegistration.where(event: dedupe_event) : EventRegistration.all + + { + model_class: EventRegistration, + domain: :event_registrations, + candidate_finder: -> { EventRegistrationServices::DuplicateFinder.new(finder_scope).groups }, + back_path: dedupe_event ? registrants_event_path(dedupe_event) : nil, + back_label: dedupe_event ? "Registrants" : nil, + subtitle: dedupe_event ? "Showing possible duplicate registrations for #{dedupe_event.title}." : nil, + editable_columns: %w[ + status expected_payment_method fee_note shoutout intends_to_pay + someone_else_will_pay invoice_requested scholarship_requested w9_requested + payment_unresolved + ], + # The (registrant_id, event_id) index means two registrations only ever + # collide when their registrants are different Person records. Merging keeps + # them separate — say so, so the admin also merges the people if they match. + merge_notes: ->(keep, delete) { + next [] if keep.registrant_id == delete.registrant_id + + [ "These registrations have different registrants (#{keep.registrant&.full_name} vs #{delete.registrant&.full_name}). Merging combines the registrations only — the two people stay separate. Merge them in the people deduper too if they're the same person." ] + }, + # A scholarship (and CE registration) moves onto the kept registration with the + # merge, but a scholarship still credits the deleted registrant. Re-credit any + # scholarship now on the keeper to the keeper's registrant so its recipient + # matches its allocation. CE delegates its registrant to the registration, so + # it follows automatically and needs no fixup here. + after_merge: ->(keep) { + keep.scholarships.where.not(recipient_id: keep.registrant_id).find_each do |scholarship| + scholarship.update!(recipient: keep.registrant) + end + }, + record_extras: ->(registration) { + [ registration.registrant&.preferred_email.presence, registration.status&.humanize ].compact.join(" · ").presence + } + } + end + def redirect_after_failed_create(alert) case params[:return_to] when "registrants" then redirect_to registrants_event_path(@event_registration.event), alert: alert diff --git a/app/services/event_registration_services/duplicate_finder.rb b/app/services/event_registration_services/duplicate_finder.rb new file mode 100644 index 000000000..de4e75ecf --- /dev/null +++ b/app/services/event_registration_services/duplicate_finder.rb @@ -0,0 +1,106 @@ +require "set" + +module EventRegistrationServices + # Surfaces likely-duplicate registrations for the deduper's candidate list. The + # (registrant_id, event_id) unique index already forbids one person registering + # for the same event twice, so a duplicate is one real person registered for the + # same event under two different Person records. Clusters registrations for the + # same event whose registrants share a signal — nickname/legal-name-aware name, + # email (or email_2), or FileMaker code — then annotates each cluster with why it + # was flagged. Registrants grouped by name but carrying different FileMaker codes + # are surfaced as a caution to verify, never silently treated as a clean merge. + class DuplicateFinder + Group = Struct.new(:key, :label, :records, :reasons, keyword_init: true) + + def initialize(scope = EventRegistration.all) + @registrations = scope.includes(:registrant, :event).to_a + end + + def groups + union = UnionFind.new(@registrations.map(&:id)) + cluster(union) { |registration| name_keys(registration) } + cluster(union) { |registration| email_keys(registration) } + cluster(union) { |registration| Array(filemaker_key(registration)) } + + by_id = @registrations.index_by(&:id) + union.components.filter_map do |ids| + next if ids.size < 2 + records = ids.map { |id| by_id[id] }.sort_by(&:id) + Group.new( + key: records.map(&:id).join("-"), + label: label_for(records), + records: records, + reasons: reasons_for(records) + ) + end.sort_by { |group| group.label.to_s.downcase } + end + + private + + def cluster(union) + buckets = Hash.new { |hash, key| hash[key] = [] } + @registrations.each do |registration| + yield(registration).each { |key| buckets[key] << registration.id } + end + buckets.each_value { |ids| union.union_all(ids) if ids.size > 1 } + end + + def label_for(records) + registrant = records.first.registrant + "#{registrant&.full_name} — #{records.first.event&.title}" + end + + def reasons_for(records) + reasons = [] + reasons << "Same registrant name" if shared?(records) { |registration| name_keys(registration) } + reasons << "Shared registrant email" if shared?(records) { |registration| email_keys(registration) } + reasons.concat(filemaker_reasons(records)) + reasons + end + + def filemaker_reasons(records) + codes = records.filter_map { |registration| registration.registrant&.filemaker_code.presence }.uniq.sort + return [] if codes.empty? + return [ "⚠ Different FileMaker codes (#{codes.join(", ")}) — verify these are the same person" ] if codes.size > 1 + + [ "Same registrant FileMaker code (#{codes.first})" ] + end + + def shared?(records) + keys = records.flat_map { |registration| Array(yield(registration)) } + keys.tally.any? { |_key, count| count > 1 } + end + + # Only registrations for the same event can be duplicates, so every signal is + # scoped by event_id — the same person on two different events is legitimate. + # First-name variant (nickname or legal first name) paired with the last name, + # so "Bob Smith" and "Robert Smith" on one event cluster together. + def name_keys(registration) + registrant = registration.registrant + last = normalized(registrant&.last_name) + return [] if last.blank? + + first_forms = [ registrant&.first_name, registrant&.legal_first_name ].filter_map { |name| name.presence } + first_forms + .flat_map { |name| NicknameMap.variants_for(name) } + .uniq + .map { |variant| "#{registration.event_id}|#{variant}|#{last}" } + end + + def email_keys(registration) + registrant = registration.registrant + [ registrant&.email, registrant&.email_2 ] + .filter_map { |email| email.to_s.strip.downcase.presence } + .map { |email| "#{registration.event_id}|#{email}" } + end + + def filemaker_key(registration) + code = registration.registrant&.filemaker_code.to_s.strip.downcase + "#{registration.event_id}|fm|#{code}" if code.present? + end + + def normalized(value) + NicknameMap.normalize(value) + end + end +end diff --git a/app/services/model_deduper.rb b/app/services/model_deduper.rb index 7c376b246..b88a2eacb 100644 --- a/app/services/model_deduper.rb +++ b/app/services/model_deduper.rb @@ -42,7 +42,7 @@ def merge(record_to_keep, record_to_delete) # sees exactly what moves (affiliations, event registrations, reports, …). def reassignment_counts(record) reassignable_joins.each_with_object({}) do |join, counts| - count = join[:join_class].where(join[:foreign_key] => record.id).count + count = join_references(join, record.id).count next if count.zero? counts[join_label(join[:join_class])] = count @@ -57,7 +57,7 @@ def reassignment_counts(record) def reassignment_preview(record) reassignable_joins.filter_map do |join| - scope = join[:join_class].where(join[:foreign_key] => record.id) + scope = join_references(join, record.id) count = scope.count next if count.zero? @@ -228,7 +228,13 @@ def has_many_joins fk = assoc.foreign_key.to_s next unless join_klass.column_names.include?(fk) - join_for(join_klass, fk) + # A polymorphic `has_many … as:` (allocations, comments) is scoped by its + # `*_type` column too, so a merge only moves this model's own rows and can't + # steal another type's rows that happen to share the deleted record's id. + type_column = assoc.type.to_s if assoc.options[:as] + type_column = nil unless type_column && join_klass.column_names.include?(type_column) + + join_for(join_klass, fk, type_column: type_column) end end @@ -269,14 +275,24 @@ def anonymous_model_for(table) klass end - def join_for(join_klass, fk) + def join_for(join_klass, fk, type_column: nil) { join_class: join_klass, foreign_key: fk.to_sym, + type_column: type_column, natural_key: natural_key_columns(join_klass, fk) } end + # Rows of `join` that reference `id` — narrowed to this model's polymorphic type + # when the join is a polymorphic `as:` association, so it never touches another + # type's rows sharing the same id. + def join_references(join, id) + scope = join[:join_class].where(join[:foreign_key] => id) + scope = scope.where(join[:type_column] => model_class.polymorphic_name) if join[:type_column] + scope + end + # Columns (other than the FK) of a unique index that includes the FK. def natural_key_columns(join_klass, fk) join_klass.connection.indexes(join_klass.table_name) @@ -384,14 +400,14 @@ def merge_join(primary, dupe, join) natural_key = join[:natural_key] if natural_key.empty? - moved = jc.where(fk => dupe.id).update_all(fk => primary.id) + moved = join_references(join, dupe.id).update_all(fk => primary.id) logger.info " moved #{moved} #{jc.name} to primary" if moved > 0 return end - existing = jc.where(fk => primary.id).pluck(*natural_key).map { |values| Array(values) }.to_set + existing = join_references(join, primary.id).pluck(*natural_key).map { |values| Array(values) }.to_set - jc.where(fk => dupe.id).find_each do |item| + join_references(join, dupe.id).find_each do |item| key = natural_key.map { |col| item.public_send(col) } if existing.include?(key) item.destroy! @@ -403,7 +419,7 @@ def merge_join(primary, dupe, join) end end - remaining = jc.where(fk => dupe.id).count + remaining = join_references(join, dupe.id).count raise "ABORT: #{remaining} #{jc.name} items still reference #{model_label} #{dupe.id}" if remaining > 0 end end diff --git a/app/views/dedupes/index.html.erb b/app/views/dedupes/index.html.erb index 06b64cd5b..f747b28f5 100644 --- a/app/views/dedupes/index.html.erb +++ b/app/views/dedupes/index.html.erb @@ -4,7 +4,8 @@
- <%= link_to "← #{@dedupe[:model_label_plural]}", url_for(action: :index), + <%= link_to "← #{@dedupe[:back_label] || @dedupe[:model_label_plural]}", + @dedupe[:back_path] || url_for(action: :index), class: "text-sm #{eyebrow_link_class} px-2 py-1" %>

Dedupe <%= @dedupe[:model_label_plural].downcase %> @@ -13,6 +14,9 @@ Pick which <%= @dedupe[:model_label].downcase %> to delete and which to keep — its records move to the one you keep before it's removed.

+ <% if @dedupe[:subtitle].present? %> +

<%= @dedupe[:subtitle] %>

+ <% end %>

diff --git a/app/views/event_registrations/index.html.erb b/app/views/event_registrations/index.html.erb index e3a7f8870..7ece93afd 100644 --- a/app/views/event_registrations/index.html.erb +++ b/app/views/event_registrations/index.html.erb @@ -18,6 +18,10 @@ event_registrations_path(request.query_parameters.merge(format: :csv)), class: "text-sm #{eyebrow_link_class} px-2 py-1" %> <% end %> + <% if allowed_to?(:manage?, EventRegistration) %> + <%= link_to "Dedupe", dedupe_index_event_registrations_path, + class: "text-sm #{eyebrow_link_class} px-2 py-1" %> + <% end %> <% if allowed_to?(:new?, EventRegistration) %> <%= link_to "New #{EventRegistration.model_name.human.downcase}", new_event_registration_path(return_to: "index"), diff --git a/app/views/events/_bulk_actions_menu.html.erb b/app/views/events/_bulk_actions_menu.html.erb index e21e858e2..8496ff4f3 100644 --- a/app/views/events/_bulk_actions_menu.html.erb +++ b/app/views/events/_bulk_actions_menu.html.erb @@ -25,6 +25,9 @@ <%= link_to "Timesheets", attendance_event_path(@event, return_to: "registrants"), class: item_class %> <% end %> <%= link_to "Reconcile affiliations", reconcile_affiliations_event_path(@event), class: item_class %> + <% if allowed_to?(:manage?, EventRegistration) %> + <%= link_to "Dedupe registrations", dedupe_index_event_registrations_path(event_id: @event.id), class: item_class %> + <% end %> <% if allowed_to?(:import?, EventRegistration) %> <%= link_to new_event_registration_import_path(event_id: @event.id), class: item_class do %> Import registrants diff --git a/config/features.yml b/config/features.yml index bf94271fa..d567c0b0d 100644 --- a/config/features.yml +++ b/config/features.yml @@ -2993,3 +2993,18 @@ attendance status, so the history isn't lost. pro_tips: - "Everyone else sees only active registrations (registered, attended, incomplete)." + +- name: "Merge duplicate registrations" + area: registration + display_status: admin_facing + released_on: 2026-08-31 + action_path: "/event_registrations/dedupe_index" + summary: >- + A new "Dedupe" tool on the Event registrations page finds and merges duplicate + registrations — the same person signed up for one event under two different + people records. It suggests likely matches, previews exactly what moves, and + combines the two registrations into one. + pro_tips: + - "Duplicates are suggested when two registrations for the same event have registrants with a matching name, email, or FileMaker code." + - "Merging combines the registrations only — the two people stay separate, so merge them in the People deduper too if they're the same person." + - "The preview shows every attendance, payment, and organization link that moves to the kept registration before you confirm, and blocks the merge if anything can't be moved safely." diff --git a/config/routes.rb b/config/routes.rb index e8ff3cc95..10c0b3b5a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -119,6 +119,12 @@ post :confirm end resources :event_registrations do + collection do + get :dedupe_index + get :dedupe_preview + post :dedupe_perform + patch :dedupe_update_keep + end member do get :confirm post :process_confirm diff --git a/spec/requests/dedupable_spec.rb b/spec/requests/dedupable_spec.rb index 631261aae..8842a26dd 100644 --- a/spec/requests/dedupable_spec.rb +++ b/spec/requests/dedupable_spec.rb @@ -695,4 +695,144 @@ end end end + + # ============================================================ + # EVENT REGISTRATIONS — one person registered for the same event + # under two different Person records. Merging combines the + # registrations only; the people stay separate. + # ============================================================ + + describe "Event registrations" do + before { sign_in admin } + + let(:event) { create(:event) } + + def registration_for(person_attrs) + create(:event_registration, event: event, registrant: create(:person, { user: nil }.merge(person_attrs))) + end + + describe "GET dedupe_index" do + it "surfaces same-event candidate groups from the duplicate finder" do + registration_for(first_name: "Jane", last_name: "Doe", email: "jane@example.com") + registration_for(first_name: "Jane", last_name: "Doe", email: "jane@work.com") + + get dedupe_index_event_registrations_path + + expect(response).to have_http_status(:ok) + expect(response.body).to include("Same registrant name") + end + + it "denies access to a regular user" do + sign_in regular_user + get dedupe_index_event_registrations_path + expect(response).not_to have_http_status(:ok) + end + + it "scopes suggestions to one event and returns the eyebrow there when opened from it" do + registration_for(first_name: "Jane", last_name: "Doe", email: "jane@example.com") + registration_for(first_name: "Jane", last_name: "Doe", email: "jane@work.com") + other_event = create(:event) + create(:event_registration, event: other_event, registrant: create(:person, first_name: "Zed", last_name: "Zed", email: "z1@example.com", user: nil)) + create(:event_registration, event: other_event, registrant: create(:person, first_name: "Zed", last_name: "Zed", email: "z2@example.com", user: nil)) + + get dedupe_index_event_registrations_path(event_id: event.id) + + expect(response).to have_http_status(:ok) + expect(response.body).to include("Jane Doe") + expect(response.body).not_to include("Zed Zed") + expect(response.body).to include(registrants_event_path(event)) + expect(response.body).to include("Showing possible duplicate registrations for") + end + end + + describe "GET dedupe_preview" do + let!(:keep) { registration_for(first_name: "Keep", last_name: "Person", email: "keep@example.com") } + let!(:delete_rec) { registration_for(first_name: "Kepe", last_name: "Persson", email: "keep@example.com") } + before { create(:event_attendance_time_entry, event_registration: delete_rec) } + + it "renders the preview with a reassignment summary" do + get dedupe_preview_event_registrations_path( + event_registration_to_keep_id: keep.id, + event_registration_to_delete_id: delete_rec.id + ) + + expect(response).to have_http_status(:ok) + expect(response.body).to include("Records that will move") + end + + it "notes that a merge across different registrants keeps the people separate" do + get dedupe_preview_event_registrations_path( + event_registration_to_keep_id: keep.id, + event_registration_to_delete_id: delete_rec.id + ) + + expect(response.body).to include("different registrants") + expect(response.body).not_to include("Merge blocked") + end + end + + describe "POST dedupe_perform" do + let!(:keep) { registration_for(first_name: "Keeper", last_name: "Person", email: "dupe@example.com") } + let!(:delete_rec) { registration_for(first_name: "Keepr", last_name: "Persn", email: "dupe@example.com") } + let!(:time_entry) { create(:event_attendance_time_entry, event_registration: delete_rec) } + + it "merges, reassigns child records, and deletes the duplicate" do + expect { + post dedupe_perform_event_registrations_path, params: { + event_registration_to_delete_id: delete_rec.id, + event_registration_to_keep_id: keep.id + } + }.to change(EventRegistration, :count).by(-1) + + expect(response).to redirect_to(event_registrations_path) + expect(EventRegistration.exists?(delete_rec.id)).to be false + expect(time_entry.reload.event_registration_id).to eq(keep.id) + end + + it "applies keep-field edits before merging" do + post dedupe_perform_event_registrations_path, params: { + event_registration_to_delete_id: delete_rec.id, + event_registration_to_keep_id: keep.id, + event_registration_to_keep: { fee_note: "Canonical registration" } + } + + expect(keep.reload.fee_note).to eq("Canonical registration") + end + + it "moves the deleted registration's scholarship onto the keeper and re-credits it to the kept registrant" do + scholarship = create(:scholarship, recipient: delete_rec.registrant, amount_cents: 1_000) + create(:allocation, source: scholarship, allocatable: delete_rec, amount: 1_000) + + post dedupe_perform_event_registrations_path, params: { + event_registration_to_delete_id: delete_rec.id, + event_registration_to_keep_id: keep.id + } + + expect(keep.reload.scholarships).to include(scholarship) + expect(scholarship.reload.recipient).to eq(keep.registrant) + end + + it "moves the deleted registration's CE registration onto the keeper" do + ce = create(:continuing_education_registration, event_registration: delete_rec) + + post dedupe_perform_event_registrations_path, params: { + event_registration_to_delete_id: delete_rec.id, + event_registration_to_keep_id: keep.id + } + + expect(ce.reload.event_registration_id).to eq(keep.id) + end + + it "denies access to a regular user and does not merge" do + sign_in regular_user + + expect { + post dedupe_perform_event_registrations_path, params: { + event_registration_to_delete_id: delete_rec.id, + event_registration_to_keep_id: keep.id + } + }.not_to change(EventRegistration, :count) + end + end + end end diff --git a/spec/services/event_registration_services/duplicate_finder_spec.rb b/spec/services/event_registration_services/duplicate_finder_spec.rb new file mode 100644 index 000000000..ceaac87f3 --- /dev/null +++ b/spec/services/event_registration_services/duplicate_finder_spec.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe EventRegistrationServices::DuplicateFinder do + let(:event) { create(:event) } + + def registration(attrs = {}) + registration_event = attrs.delete(:event) || event + create(:event_registration, + event: registration_event, + registrant: create(:person, { user: nil }.merge(attrs))) + end + + describe "#groups" do + it "groups same-event registrations whose registrants share a name" do + a = registration(first_name: "Jane", last_name: "Doe", email: "jane@example.com") + b = registration(first_name: "Jane", last_name: "Doe", email: "jane.doe@work.com") + registration(first_name: "Unrelated", last_name: "Person", email: "u@example.com") + + groups = described_class.new.groups + + expect(groups.size).to eq(1) + expect(groups.first.records).to contain_exactly(a, b) + expect(groups.first.reasons).to include("Same registrant name") + end + + it "matches a nickname or legal-name variant of the registrant's first name" do + a = registration(first_name: "Bob", last_name: "Smith", email: "bob@example.com") + b = registration(first_name: "Robert", last_name: "Smith", email: "robert@example.com") + + group = described_class.new.groups.first + + expect(group.records).to contain_exactly(a, b) + expect(group.reasons).to include("Same registrant name") + end + + it "groups registrations whose registrants share an email address" do + a = registration(first_name: "Chris", last_name: "Alpha", email: "shared@example.com") + b = registration(first_name: "Kris", last_name: "Beta", email: "SHARED@example.com") + + group = described_class.new.groups.first + + expect(group.records).to contain_exactly(a, b) + expect(group.reasons).to include("Shared registrant email") + end + + it "matches an email stored in the registrant's secondary email_2 field" do + a = registration(first_name: "Dana", last_name: "Gamma", email: "dana@primary.com") + b = registration(first_name: "Dana", last_name: "Delta", email: "dana@other.com", email_2: "dana@primary.com") + + expect(described_class.new.groups.first.records).to contain_exactly(a, b) + end + + it "groups registrations whose registrants share a FileMaker code" do + a = registration(first_name: "Erin", last_name: "One", email: "e1@example.com", filemaker_code: "FM777") + b = registration(first_name: "Erin", last_name: "Two", email: "e2@example.com", filemaker_code: "FM777") + + group = described_class.new.groups.first + + expect(group.records).to contain_exactly(a, b) + expect(group.reasons).to include("Same registrant FileMaker code (FM777)") + end + + it "flags different FileMaker codes on registrants grouped by another signal" do + registration(first_name: "Fay", last_name: "Nguyen", email: "fay@example.com", filemaker_code: "FM1") + registration(first_name: "Fay", last_name: "Nguyen", email: "fay2@example.com", filemaker_code: "FM2") + + expect(described_class.new.groups.first.reasons) + .to include(a_string_matching(/Different FileMaker codes \(FM1, FM2\)/)) + end + + it "does not group the same person registered for different events" do + person = create(:person, first_name: "Same", last_name: "Person", email: "same@example.com", user: nil) + create(:event_registration, event: event, registrant: person) + create(:event_registration, event: create(:event), registrant: person) + + expect(described_class.new.groups).to be_empty + end + + it "returns nothing when there are no duplicates" do + registration(first_name: "Only", last_name: "One", email: "only@example.com") + + expect(described_class.new.groups).to be_empty + end + end +end diff --git a/spec/services/model_deduper_spec.rb b/spec/services/model_deduper_spec.rb index 6baf7c987..f07f28194 100644 --- a/spec/services/model_deduper_spec.rb +++ b/spec/services/model_deduper_spec.rb @@ -167,6 +167,21 @@ expect(bookmark.reload.bookmarkable).to eq(keep) end + it "scopes a polymorphic reassignment by type, never stealing another type's same-id row" do + mine = create(:bookmark, bookmarkable: dupe) + # A bookmark of a different type whose bookmarkable_id collides with dupe.id. + foreign = build(:bookmark, bookmarkable: create(:workshop)) + foreign.bookmarkable_type = "Workshop" + foreign.bookmarkable_id = dupe.id + foreign.save!(validate: false) + + service.merge(keep, dupe) + + expect(mine.reload.bookmarkable_id).to eq(keep.id) + expect(foreign.reload.bookmarkable_id).to eq(dupe.id) + expect(foreign.bookmarkable_type).to eq("Workshop") + end + # Payment/Story reference organizations by FK but Organization declares no # inverse has_many, so they are only reachable via the belongs_to scan. Their # DB foreign keys would otherwise block the destroy.