diff --git a/.gitignore b/.gitignore index e30b6712d..3e5585fdd 100644 --- a/.gitignore +++ b/.gitignore @@ -64,3 +64,4 @@ yarn-debug.log* # Worktrees .worktrees/ +/log/merge_duplicate_members/ diff --git a/Makefile b/Makefile index 2b5558b25..453091fa1 100644 --- a/Makefile +++ b/Makefile @@ -90,6 +90,26 @@ test: ## Run the test suite in parallel bundle exec rake parallel:setup bundle exec parallel_rspec spec/ -n 3 +detect_duplicate_members: ## Detect members duplicated by the codebar auth flow + DB_NAME=$(DUMP_DB) bundle exec rake member:duplicates:detect + +fix_duplicate_members: ## Dry-run merge of duplicate members (set APPLY=1 to execute) + DB_NAME=$(DUMP_DB) bundle exec rake member:duplicates:fix + +verify_duplicate_members: ## Verify no codebar-auth duplicate members remain + DB_NAME=$(DUMP_DB) bundle exec rake member:duplicates:verify + +detect_duplicate_members_production: ## Detect duplicates on production DB directly + @read -p "Connect to PRODUCTION database? [y/N] " ans && [ "$$ans" = "y" ] || exit 1 + @DB_URL=$$(heroku config:get DATABASE_URL --app=$(DUMP_APP)) bundle exec rake member:duplicates:detect + +fix_duplicate_members_production: ## Dry-run merge on production DB (APPLY=1 executes) + @read -p "This MODIFIES the PRODUCTION database. Continue? [y/N] " ans && [ "$$ans" = "y" ] || exit 1 + @DB_URL=$$(heroku config:get DATABASE_URL --app=$(DUMP_APP)) bundle exec rake member:duplicates:fix + +verify_duplicate_members_production: ## Verify no duplicates remain on production DB + @DB_URL=$$(heroku config:get DATABASE_URL --app=$(DUMP_APP)) bundle exec rake member:duplicates:verify + check: ## Run setup checks bundle exec rake setup:check diff --git a/docs/merge_duplicate_members.md b/docs/merge_duplicate_members.md new file mode 100644 index 000000000..d4c471e47 --- /dev/null +++ b/docs/merge_duplicate_members.md @@ -0,0 +1,117 @@ +# Merge Duplicate Members + +Temporary tool to detect and merge duplicate members created by the `/auth/codebar` GitHub sign-in flow (codebar/planner#2805). + +## Background + +When the codebar auth app was merged into planner on 2026-08-06, members with a GitHub account whose email differed from their `auth_services` record could not be matched automatically. The codebar auth flow created new accounts instead of linking to existing ones. + +This tool finds those duplicates (by name, email, and auth UID heuristics) and merges their data into the original member. + +## Prerequisites + +A PostgreSQL dump of the production database, accessible as `codebar_production_dump`. + +## Tasks + +### Detect duplicates + +```bash +make detect_duplicate_members +``` + +Dry run — lists all duplicate pairs found, with which detection strategies matched. + +### Fix duplicates (dry run) + +```bash +make fix_duplicate_members +``` + +Shows every step the merger would take, but wraps everything in a transaction that is rolled back. No data is modified. + +### Fix duplicates (execute) + +```bash +make fix_duplicate_members APPLY=1 +``` + +Performs the merge for real inside a transaction. +Only high-confidence matches are merged (see below). To also merge low-confidence matches after review: + +```bash +make fix_duplicate_members APPLY=1 INCLUDE_WEAK=1 +``` +A JSON log file is written to: + +``` +log/merge_duplicate_members/run_YYYYMMDDTHHMMSSZ.json +``` + +The path is printed after the run completes. + +### Verify + +```bash +make verify_duplicate_members +``` + +Re-detects duplicates and exits `0` when no high-confidence duplicates remain, or `1` with the remaining pairs. +Low-confidence matches are reported but do not fail the check. + +## Detection strategies and confidence + +Merges are gated by confidence. A shared name is **not** proof of the same person — two different members can register with the same name (e.g. members 31336 / 25796, who hold two different GitHub accounts). Only identity evidence merges by default. + +| Strategy | Description | Confidence | Merged by default | +|----------|-------------|------------|-------------------| +| `email` | Exact case-insensitive match on email | high | yes | +| `manual` | Hard-coded override reviewed by a human | high | yes | +| `name+surname` | Exact case-insensitive match on both fields | low | no — review first | +| `first-name+uid-surname` | First name matches; duplicate has no surname, but the codebar auth UID contains the original’s surname | low | no — review first | +| `domain+local-part` | Non-generic domain; local parts overlap | low | no — review first | + +`detect` lists all matches with a confidence column. `fix` merges only high-confidence matches; low-confidence matches are listed for manual review. +To merge a reviewed low-confidence pair, add it to `MANUAL_OVERRIDES` (preferred — it records the human decision) or re-run with `INCLUDE_WEAK=1` to merge all matches. + +The tool also applies hard-coded manual overrides for edge cases the heuristics cannot detect. + +## Safety properties + +- **Dry run by default** — must pass `APPLY=1` to change data. +- **Idempotent** — re-running after a successful merge reports no duplicates. +- **Deactivates, not deletes** — duplicates are renamed to `duplicate..merged-into.@codebar.io`. The duplicate's `codebar` auth service is re-pointed to the original member (so the original's sign-in keeps working); any remaining auth services and all roles are removed. Audit history is preserved in a `MemberNote`. +- **Logs every execution** — merge results written to a new per-run JSON file, even on failure. The log contains only opaque member IDs (`dup_id`, `orig_id`), strategies, and status. No names, emails, UIDs, or other PII. Safe to share or attach to PRs. + +## Running in production + +### Option A: local execution (no deploy required) + +You can run the tool locally and connect directly to the Heroku production database using a connection string: + +```bash +make detect_duplicate_members_production # lists duplicates from production +make fix_duplicate_members_production # dry-run against production +make fix_duplicate_members_production APPLY=1 # execute on production +make verify_duplicate_members_production # verify production +``` + +These targets fetch a fresh `DATABASE_URL` from Heroku each time and pass it via `DB_URL`. A confirmation prompt guards the detect and fix targets. A loud `WARNING: Connecting to REMOTE database …` message is printed when `DB_URL` points to a non-localhost host. + +### Option B: run on Heroku dyno + +Deploy the branch first, then run on Heroku: + +```bash +heroku run rake member:duplicates:detect --app codebar-production +heroku run rake member:duplicates:fix APPLY=1 --app codebar-production +heroku run rake member:duplicates:verify --app codebar-production +``` + +## What to do if a false positive appears + +Duplicate pairs can be added to the `MANUAL_OVERRIDES` array or excluded before execution. If in doubt, err on the side of not merging — the merge does not delete records, but it does permanently move associated data and deactivate the account. + +--- + +*This is a temporary tool. Once all existing duplicates are resolved, it can be removed.* diff --git a/lib/tasks/merge_duplicate_members.rake b/lib/tasks/merge_duplicate_members.rake new file mode 100644 index 000000000..2dcfee252 --- /dev/null +++ b/lib/tasks/merge_duplicate_members.rake @@ -0,0 +1,591 @@ +# frozen_string_literal: true +# rubocop:disable all + +# Temporary task to detect and merge members created as duplicates by the +# codebar auth GitHub sign-in flow (codebar/planner#2805). +# +# Usage: +# Local dump: +# make detect_duplicate_members +# make fix_duplicate_members +# make fix_duplicate_members APPLY=1 +# +# Production: +# heroku run rake member:duplicates:detect --app codebar-production +# heroku run rake member:duplicates:fix APPLY=1 --app codebar-production +# +# The task switches to the local dump when DB_NAME is set; otherwise it uses the +# current Rails environment's database (e.g. Heroku DATABASE_URL). + +namespace :member do + namespace :duplicates do + desc 'Detect duplicate members created by the codebar auth flow' + task detect: :environment do + MergeDuplicateMembers.establish_connection! + duplicates = MergeDuplicateMembers::Detector.new.call + MergeDuplicateMembers::Reporter.new(duplicates).print + end + + desc 'Merge duplicate members into their originals (set APPLY=1 to execute)' + task fix: :environment do + MergeDuplicateMembers.establish_connection! + dry_run = ENV['APPLY'] != '1' + duplicates = MergeDuplicateMembers::Detector.new.call + + if duplicates.empty? + puts 'No duplicate members detected.' + next + end + + high_confidence, low_confidence = duplicates.partition(&:high_confidence?) + + targets = + if ENV['INCLUDE_WEAK'] == '1' + puts 'INCLUDE_WEAK=1 — merging ALL matches, including low-confidence ones.' + puts + duplicates + else + low_confidence.each do |m| + puts "Skipping low-confidence match #{m.dup_member_id} → #{m.original_member_id} " \ + "(#{m.merge_strategies}) — a shared name is not proof of the same person. Review manually, " \ + 'then add a MANUAL_OVERRIDES entry or re-run with INCLUDE_WEAK=1.' + end + puts + high_confidence + end + + if targets.empty? + puts 'No high-confidence duplicates detected.' + if low_confidence.any? + puts "#{low_confidence.size} low-confidence match(es) listed above — review before merging." + end + next + end + + puts dry_run ? 'DRY RUN — no changes will be made.' : 'APPLYING merges...' + puts + + # A pair whose original is itself a merge target would strand live data + # on a deactivated tombstone once the earlier pair runs: the codebar + # auth service and subscriptions would land on an account that no + # longer signs in. Refuse and report instead. + chain_pairs = [] + dup_ids = Set.new(targets.map(&:dup_member_id)) + targets, chain_pairs = targets.partition do |pair| + next true unless dup_ids.include?(pair.original_member_id) + + chain_pairs << pair + false + end + chain_pairs.each do |pair| + warn "CHAIN: duplicate #{pair.dup_member_id} targets #{pair.original_member_id}, which is itself a merge target this run. Skipping — resolve the chain manually (or in a second ordered run)." + end + puts if chain_pairs.any? + + logger = MergeDuplicateMembers::RunLogger.new(dry_run: dry_run) + + targets.each do |pair| + begin + MergeDuplicateMembers::Merger.new(pair, dry_run: dry_run).call + logger.record_merge(dup_id: pair.dup_member_id, orig_id: pair.original_member_id, strategies: pair.merge_strategies, status: 'success') + rescue StandardError => e + # The log must be written even on failure: pairs already merged are + # committed production changes that only the log can account for. + logger.record_error(e) + logger.flush + raise + end + puts + end + + log_path = logger.flush + puts dry_run ? "Run with APPLY=1 to execute these #{targets.size} merges." : 'Done.' + puts "Log: #{log_path}" if log_path + end + + desc 'Verify no duplicate members remain after merging' + task verify: :environment do + MergeDuplicateMembers.establish_connection! + duplicates = MergeDuplicateMembers::Detector.new.call + high_confidence, low_confidence = duplicates.partition(&:high_confidence?) + + if high_confidence.empty? + puts 'PASS: no high-confidence duplicate members detected.' + if low_confidence.any? + puts "Note: #{low_confidence.size} low-confidence match(es) remain — review manually before merging:" + MergeDuplicateMembers::Reporter.new(low_confidence).print + end + else + puts "FAIL: #{high_confidence.size} high-confidence duplicate member(s) still detected:" + MergeDuplicateMembers::Reporter.new(high_confidence).print + exit 1 + end + end + end +end + +module MergeDuplicateMembers + # The merge of the /auth/codebar sign-in flow into planner. + # Duplicates cannot have been created before this point. + CUTOFF_TIME = Time.utc(2026, 8, 6, 15, 25, 11) + + # Email domains that are too generic for the domain-similarity heuristic. + COMMON_DOMAINS = %w[ + gmail.com googlemail.com outlook.com hotmail.com live.com yahoo.com + ymail.com icloud.com me.com mac.com aol.com qq.com 163.com 126.com + foxmail.com protonmail.com proton.me + ].freeze + + # Hard-coded merges for cases the heuristics cannot safely detect. + # Format: [duplicate_member_id, original_member_id] + MANUAL_OVERRIDES = [ + [31_257, 27_714], # Lou Alldis's third account + [31_292, 13_771] # Lena Krasnova — signed up with work email, name typed as one word + ].freeze + + class << self + def establish_connection! + if ENV['DB_URL'].present? + url = URI.parse(ENV['DB_URL']) + unless %w[localhost 127.0.0.1].include?(url.host) + puts "WARNING: Connecting to REMOTE database #{url.host}" + end + ActiveRecord::Base.establish_connection(ENV['DB_URL']) + elsif ENV['DB_NAME'].present? + ActiveRecord::Base.establish_connection( + adapter: 'postgresql', + host: ENV.fetch('DB_HOST', 'localhost'), + port: ENV.fetch('DB_PORT', 5432), + database: ENV.fetch('DB_NAME'), + username: ENV.fetch('DB_USER', ''), + password: ENV.fetch('POSTGRES_PASSWORD', '') + ) + end + end + + def truncate(string, max_length) + string.length > max_length ? "#{string[0...max_length - 1]}…" : string + end + end + + class Detector + def call + detected = (name_matches + email_matches + firstname_uid_surname_matches + concatenated_name_matches + domain_local_matches) + .group_by(&:dup_member_id) + .transform_values { |matches| best_match(matches) } + + apply_manual_overrides(detected) + detected.values.reject { |m| already_merged?(m) }.sort_by { |m| m.original_member.id } + end + + private + + def already_merged?(match) + dup = match.dup_member + dup.email.start_with?('duplicate.') || + !AuthService.exists?(member_id: dup.id, provider: 'codebar') + end + + private + + def codebar_members + Member.joins(:auth_services) + .where(auth_services: { provider: 'codebar' }) + .where('members.created_at > ?', CUTOFF_TIME) + end + + def name_matches + codebar_members + .joins(<<~SQL) + JOIN members originals + ON LOWER(TRIM(members.name)) = LOWER(TRIM(originals.name)) + AND LOWER(TRIM(members.surname)) = LOWER(TRIM(originals.surname)) + AND NULLIF(TRIM(members.name), '') IS NOT NULL + AND NULLIF(TRIM(members.surname), '') IS NOT NULL + SQL + .joins("JOIN auth_services original_auth ON original_auth.member_id = originals.id AND original_auth.provider = 'github'") + .where('originals.created_at < members.created_at') + .select("members.id AS dup_member_id, originals.id AS original_member_id, 'name+surname' AS strategy") + .map { |r| Match.new(r.dup_member_id, r.original_member_id, r.strategy) } + end + + def email_matches + codebar_members + .joins('JOIN members originals ON LOWER(TRIM(members.email)) = LOWER(TRIM(originals.email)) AND originals.id != members.id') + .joins("JOIN auth_services original_auth ON original_auth.member_id = originals.id AND original_auth.provider = 'github'") + .where('originals.created_at < members.created_at') + .select("members.id AS dup_member_id, originals.id AS original_member_id, 'email' AS strategy") + .map { |r| Match.new(r.dup_member_id, r.original_member_id, r.strategy) } + end + + # Catches dups who typed their whole name into one field (e.g. name + # "LenaKrasnova", surname blank) so exact name+surname never matches. + # Compares lowercased/trimmed name||surname in both orders (field swaps). + # Requires at least one side to have a surname, otherwise it degenerates + # into a first-name-only match. Low-confidence: a shared name is not proof + # of the same person. + def concatenated_name_matches + codebar_members + .joins(<<~SQL) + JOIN members originals + ON COALESCE(LOWER(TRIM(members.name)), '') || COALESCE(LOWER(TRIM(members.surname)), '') = + COALESCE(LOWER(TRIM(originals.name)), '') || COALESCE(LOWER(TRIM(originals.surname)), '') + OR COALESCE(LOWER(TRIM(members.name)), '') || COALESCE(LOWER(TRIM(members.surname)), '') = + COALESCE(LOWER(TRIM(originals.surname)), '') || COALESCE(LOWER(TRIM(originals.name)), '') + SQL + .joins("JOIN auth_services original_auth ON original_auth.member_id = originals.id AND original_auth.provider = 'github'") + .where('originals.created_at < members.created_at') + .where("NULLIF(TRIM(members.name), '') IS NOT NULL") + .where("NULLIF(TRIM(originals.name), '') IS NOT NULL") + .where("NULLIF(TRIM(COALESCE(members.surname, '')), '') IS NOT NULL OR NULLIF(TRIM(COALESCE(originals.surname, '')), '') IS NOT NULL") + .select("members.id AS dup_member_id, originals.id AS original_member_id, 'concatenated-name' AS strategy") + .map { |r| Match.new(r.dup_member_id, r.original_member_id, r.strategy) } + end + + def firstname_uid_surname_matches + codebar_members + .joins('JOIN members originals ON LOWER(TRIM(members.name)) = LOWER(TRIM(originals.name))') + .joins("JOIN auth_services original_auth ON original_auth.member_id = originals.id AND original_auth.provider = 'github'") + .where('NULLIF(TRIM(members.name), \'\') IS NOT NULL') + .where("members.surname IS NULL OR TRIM(members.surname) = ''") + .where('originals.created_at < members.created_at') + # An empty-string surname must not degenerate the LIKE into a no-op + # first-name-only match ('%' || '' || '%'). + .where("NULLIF(TRIM(originals.surname), '') IS NOT NULL") + .where("LOWER(auth_services.uid) LIKE '%' || LOWER(originals.surname) || '%'") + .select("members.id AS dup_member_id, originals.id AS original_member_id, 'first-name+uid-surname' AS strategy") + .map { |r| Match.new(r.dup_member_id, r.original_member_id, r.strategy) } + end + + def domain_local_matches + non_common_domains = COMMON_DOMAINS.map { |d| "'#{d}'" }.join(',') + + codebar_members + .joins('JOIN members originals ON split_part(members.email, \'@\', 2) ILIKE split_part(originals.email, \'@\', 2)') + .joins("JOIN auth_services original_auth ON original_auth.member_id = originals.id AND original_auth.provider = 'github'") + .where('originals.id != members.id') + .where('originals.created_at < members.created_at') + .where(Arel.sql("split_part(members.email, '@', 2) NOT IN (#{non_common_domains})")) + .where(<<~SQL) + split_part(members.email, '@', 1) ILIKE '%' || split_part(originals.email, '@', 1) || '%' + OR split_part(originals.email, '@', 1) ILIKE '%' || split_part(members.email, '@', 1) || '%' + SQL + .where(<<~SQL) + (SELECT COUNT(*) FROM members_permissions mp WHERE mp.member_id = originals.id) > 0 + OR (SELECT COUNT(*) FROM members_roles mr WHERE mr.member_id = originals.id) > 0 + OR (SELECT COUNT(*) FROM subscriptions s WHERE s.member_id = originals.id) > 0 + OR (SELECT COUNT(*) FROM workshop_invitations wi WHERE wi.member_id = originals.id) > 0 + SQL + .select("members.id AS dup_member_id, originals.id AS original_member_id, 'domain+local-part' AS strategy") + .map { |r| Match.new(r.dup_member_id, r.original_member_id, r.strategy) } + end + + def best_match(matches) + # A high-confidence match always beats an activity-based guess. + matches.max_by do |m| + orig = m.original_member + [m.high_confidence? ? 1 : 0, orig.roles.count, orig.subscriptions.count, orig.workshop_invitations.count] + end + end + + def apply_manual_overrides(detected) + MANUAL_OVERRIDES.each do |dup_id, orig_id| + next unless Member.exists?(dup_id) && Member.exists?(orig_id) + + detected[dup_id] = Match.new(dup_id, orig_id, 'manual') + end + end + end + + class Match + # Only evidence of the same identity justifies merging. A shared name is + # not: different people can share a name (see member 31336 / 25796). + HIGH_CONFIDENCE_STRATEGIES = %w[email manual].freeze + + attr_reader :dup_member_id, :original_member_id, :strategies + + def initialize(dup_member_id, original_member_id, strategy) + @dup_member_id = dup_member_id + @original_member_id = original_member_id + @strategies = Set[strategy] + end + + def high_confidence? + @strategies.any? { |s| HIGH_CONFIDENCE_STRATEGIES.include?(s) } + end + + def dup_member + @dup_member ||= Member.find(@dup_member_id) + end + + def original_member + @original_member ||= Member.find(@original_member_id) + end + + def merge_strategies + @strategies.to_a.sort.join(', ') + end + + def confidence + high_confidence? ? 'high' : 'low' + end + end + + class Reporter + def initialize(matches) + @matches = matches + end + + def print + puts format('%-10s %-25s %-35s %-10s %-25s %-35s %-6s %-25s', 'Dup id', 'Dup name', 'Dup email', 'Orig id', 'Orig name', 'Orig email', 'Conf', 'Strategies') + puts '-' * 180 + @matches.each do |m| + dup = m.dup_member + orig = m.original_member + dup_name = [dup.name, dup.surname].compact.join(' ') + orig_name = [orig.name, orig.surname].compact.join(' ') + puts format('%-10s %-25s %-35s %-10s %-25s %-35s %-6s %-25s', + dup.id, MergeDuplicateMembers.truncate(dup_name, 25), MergeDuplicateMembers.truncate(dup.email, 35), + orig.id, MergeDuplicateMembers.truncate(orig_name, 25), MergeDuplicateMembers.truncate(orig.email, 35), m.confidence, m.merge_strategies) + end + puts "\n#{@matches.size} match(es) detected." + end + end + + class RunLogger + LOG_DIR = Rails.root.join('log', 'merge_duplicate_members').freeze + + def initialize(dry_run:) + @dry_run = dry_run + @started_at = Time.now.iso8601 + @merges = [] + @errors = [] + end + + def log_path + @log_path ||= LOG_DIR.join("run_#{Time.now.utc.strftime('%Y%m%dT%H%M%SZ')}.json") + end + + def record_merge(dup_id:, orig_id:, strategies:, status:) + return if @dry_run + + @merges << { dup_id: dup_id, orig_id: orig_id, strategies: strategies, status: status } + end + + def record_error(error) + return if @dry_run + + @errors << { message: error.message, backtrace: error.backtrace.to_a.first(5) } + end + + def flush + return if @dry_run || (@merges.empty? && @errors.empty?) + + entry = { + timestamp: @started_at, + dry_run: @dry_run, + environment: Rails.env, + merges: @merges, + errors: @errors, + total_merges: @merges.size + } + + FileUtils.mkdir_p(LOG_DIR) + File.write(log_path, JSON.pretty_generate(entry)) + log_path + end + end + + class Merger + def initialize(match, dry_run: true) + @match = match + @dry_run = dry_run + end + + def call + dup = @match.dup_member + orig = @match.original_member + + puts "#{dry_run_label}Merging member #{dup.id} (#{dup.email}) into #{orig.id} (#{orig.email})" + + if already_merged?(dup, orig) + puts ' Already merged; skipping.' + return + end + + within_transaction do + move_auth_service(dup, orig) + merge_subscriptions(dup, orig) + merge_workshop_invitations(dup, orig) + merge_invitations(dup, orig) + merge_meeting_invitations(dup, orig) + merge_member_notes(dup, orig) + merge_bans(dup, orig) + merge_eligibility_inquiries(dup, orig) + merge_attendance_warnings(dup, orig) + merge_member_email_deliveries(dup, orig) + merge_testimonials(dup, orig) + merge_feedback_requests(dup, orig) + update_invitation_logs(dup, orig) + deactivate_duplicate(dup, orig) + end + + puts " #{dry_run_label}Done." + end + + private + + def dry_run_label + @dry_run ? '[DRY RUN] ' : '' + end + + def already_merged?(dup, orig) + !Member.exists?(dup.id) || + dup.email.start_with?('duplicate.') || + (AuthService.exists?(member_id: orig.id, provider: 'codebar') && + !AuthService.exists?(member_id: dup.id, provider: 'codebar')) + end + + def within_transaction + if @dry_run + ActiveRecord::Base.transaction do + yield + raise ActiveRecord::Rollback + end + else + ActiveRecord::Base.transaction { yield } + end + end + + def move_auth_service(dup, orig) + auth = AuthService.find_by(member_id: dup.id, provider: 'codebar') + return unless auth + + puts " #{dry_run_label}Moving codebar auth service (#{auth.uid}) to original" + auth.update!(member_id: orig.id) + end + + def merge_subscriptions(dup, orig) + # Discarded subscriptions are tombstones (issue #2920): the original may + # have unsubscribed long ago, which must not count as "same subscription + # exists" — an active duplicate subscription moves, it is not destroyed. + dup.subscriptions.kept.find_each do |sub| + if orig.subscriptions.kept.exists?(group_id: sub.group_id) + puts " #{dry_run_label}Deleting duplicate subscription for group #{sub.group_id}" + sub.destroy! + else + puts " #{dry_run_label}Moving subscription for group #{sub.group_id}" + sub.update!(member_id: orig.id) + end + end + end + + def merge_workshop_invitations(dup, orig) + dup.workshop_invitations.find_each do |wi| + if orig.workshop_invitations.exists?(workshop_id: wi.workshop_id, role: wi.role) + puts " #{dry_run_label}Deleting duplicate workshop invitation #{wi.workshop_id}/#{wi.role}" + wi.destroy! + else + puts " #{dry_run_label}Moving workshop invitation #{wi.workshop_id}/#{wi.role}" + wi.update!(member_id: orig.id) + end + end + end + + def merge_invitations(dup, orig) + dup.invitations.find_each do |inv| + if orig.invitations.exists?(event_id: inv.event_id, role: inv.role) + puts " #{dry_run_label}Deleting duplicate invitation for event #{inv.event_id}" + inv.destroy! + else + puts " #{dry_run_label}Moving invitation for event #{inv.event_id}" + inv.update!(member_id: orig.id) + end + end + end + + def merge_meeting_invitations(dup, orig) + dup.meeting_invitations.find_each do |mi| + if orig.meeting_invitations.exists?(meeting_id: mi.meeting_id) + puts " #{dry_run_label}Deleting duplicate meeting invitation #{mi.meeting_id}" + mi.destroy! + else + puts " #{dry_run_label}Moving meeting invitation #{mi.meeting_id}" + mi.update!(member_id: orig.id) + end + end + end + + + + def merge_member_notes(dup, orig) + dup.member_notes.update_all(member_id: orig.id) + end + + def merge_bans(dup, orig) + dup.bans.update_all(member_id: orig.id) + end + + def merge_eligibility_inquiries(dup, orig) + dup.eligibility_inquiries.update_all(member_id: orig.id) + end + + def merge_attendance_warnings(dup, orig) + dup.attendance_warnings.update_all(member_id: orig.id) + end + + def merge_member_email_deliveries(dup, orig) + # Deliveries are an immutable log with a UNIQUE (member_id, email_type) + # index (PR #2832). A colliding type stays on the renamed duplicate — + # destroying it would erase history, and moving it would violate the index. + dup.member_email_deliveries.find_each do |delivery| + if orig.member_email_deliveries.exists?(email_type: delivery.email_type) + puts " #{dry_run_label}Keeping delivery #{delivery.email_type} on the renamed member (type already on original)" + else + puts " #{dry_run_label}Moving email delivery #{delivery.email_type}" + delivery.update!(member_id: orig.id) + end + end + end + + def merge_testimonials(dup, orig) + return unless defined?(Testimonial) + + count = Testimonial.where(member_id: dup.id).update_all(member_id: orig.id) + puts " #{dry_run_label}Moved #{count} testimonial(s)" if count.positive? + end + + def merge_feedback_requests(dup, orig) + # Like member_email_deliveries, feedback requests carry a UNIQUE + # (member_id, workshop_id) index. A colliding row stays on the renamed + # duplicate instead of violating the constraint mid-merge. + FeedbackRequest.where(member_id: dup.id).find_each do |request| + if FeedbackRequest.exists?(member_id: orig.id, workshop_id: request.workshop_id) + puts " #{dry_run_label}Keeping feedback request #{request.id} on the renamed member (original already has one for workshop #{request.workshop_id})" + else + puts " #{dry_run_label}Moving feedback request #{request.id}" + request.update!(member_id: orig.id) + end + end + end + + def update_invitation_logs(dup, orig) + count = InvitationLog.where(initiator_id: dup.id).update_all(initiator_id: orig.id) + puts " #{dry_run_label}Updated #{count} invitation log initiator(s)" if count.positive? + end + + def deactivate_duplicate(dup, orig) + dup.auth_services.destroy_all + dup.roles.clear + + new_email = "duplicate.#{dup.id}.merged-into.#{orig.id}@codebar.io" + puts " #{dry_run_label}Renaming duplicate email to #{new_email}" + dup.update_columns(email: new_email) + + note = "Duplicate of member #{orig.id} (#{orig.email}). Merged #{Time.zone.now.iso8601}. Login disabled." + dup.member_notes.create!(note: note, author_id: orig.id) + end + end +end + +# rubocop:enable all diff --git a/spec/lib/tasks/merge_duplicate_members_rake_spec.rb b/spec/lib/tasks/merge_duplicate_members_rake_spec.rb new file mode 100644 index 000000000..eff544674 --- /dev/null +++ b/spec/lib/tasks/merge_duplicate_members_rake_spec.rb @@ -0,0 +1,228 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'rake member:duplicates', type: :task do + let(:cutoff) { MergeDuplicateMembers::CUTOFF_TIME } + + before do + allow($stdout).to receive(:puts) + end + + def create_member(email:, name:, surname: nil, created_at: nil) + member = Member.new(email:, name:, surname:, about_you: 'n/a', accepted_toc_at: Time.zone.now) + member.save(validate: false) + member.update_columns(created_at:) if created_at + member + end + + def create_duplicate(email: 'dup@example.com', name: 'Sam', surname: 'Dup', uid: nil, created_at: cutoff + 1.day) + member = create_member(email:, name:, surname:, created_at:) + member.auth_services.create!(provider: 'codebar', uid: uid || email) + member.reload + end + + def create_original(email: 'orig@example.com', name: 'Sam', surname: 'Dup', created_at: cutoff - 1.day) + member = create_member(email:, name:, surname:, created_at:) + member.auth_services.create!(provider: 'github', uid: '1234567') + member.reload + end + + def merge!(dup, orig) + MergeDuplicateMembers::Merger.new( + MergeDuplicateMembers::Match.new(dup.id, orig.id, 'email'), dry_run: false + ).call + end + + describe 'member:duplicates:fix' do + let(:task) { Rake::Task['member:duplicates:fix'] } + + after { task.reenable } + + it 'preloads the Rails environment' do + expect(task.prerequisites).to include 'environment' + end + + it 'changes nothing in dry-run mode' do + dup = create_duplicate + create_original + old_email = dup.email + + task.execute + + expect(dup.reload.email).to eq(old_email) + expect(dup.auth_services).not_to be_empty + end + + it 'writes the run log when a pair fails mid-run' do + create_duplicate + create_original + failing_pair = MergeDuplicateMembers::Match.new(-1, -2, 'email') + + # The task constructs its own Detector and Merger internally, so + # instance-level stubbing is the only seam into that flow. + # rubocop:disable RSpec/AnyInstance + allow_any_instance_of(MergeDuplicateMembers::Detector) + .to receive(:call).and_return([failing_pair]) + allow_any_instance_of(MergeDuplicateMembers::Merger).to receive(:call) + .and_raise(ActiveRecord::RecordNotFound) + # rubocop:enable RSpec/AnyInstance + + # Safe recovery: the task re-raises after logging, so the spec rescues + # and asserts on the side effect. + ENV['APPLY'] = '1' + begin + task.execute + rescue ActiveRecord::RecordNotFound + nil + ensure + ENV.delete('APPLY') + end + + logs = Dir.glob(Rails.root.join('log/merge_duplicate_members/run_*.json').to_s) + expect(logs).not_to be_empty + content = JSON.parse(File.read(logs.max_by { |f| File.mtime(f) })) + expect(content['errors']).not_to be_empty + + File.delete(logs.max_by { |f| File.mtime(f) }) + end + end + + describe 'Merger member_email_deliveries' do + it 'moves the duplicate delivery when the original has no delivery of that type' do + dup = create_duplicate + orig = create_original + delivery = Fabricate(:member_email_delivery, member: dup, email_type: 'chaser') + Fabricate(:member_email_delivery, member: orig, email_type: 'welcome') + + merge!(dup, orig) + + expect(delivery.reload.member_id).to eq(orig.id) + end + + it 'keeps the duplicate delivery on the renamed member when the original already has that type' do + dup = create_duplicate + orig = create_original + delivery = Fabricate(:member_email_delivery, member: dup, email_type: 'welcome') + Fabricate(:member_email_delivery, member: orig, email_type: 'welcome') + + merge!(dup, orig) + + expect(delivery.reload.member_id).to eq(dup.id) + end + end + + describe 'Merger subscriptions' do + it 'deletes the duplicate subscription when the original has an active one for the group' do + dup = create_duplicate + orig = create_original + group = Fabricate(:group) + Fabricate(:subscription, member: orig, group:) + duplicate_subscription = Fabricate(:subscription, member: dup, group:) + + merge!(dup, orig) + + expect { duplicate_subscription.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'moves the duplicate active subscription when the original only has a discarded one' do + dup = create_duplicate + orig = create_original + group = Fabricate(:group) + Fabricate(:discarded_subscription, member: orig, group:) + duplicate_subscription = Fabricate(:subscription, member: dup, group:) + + merge!(dup, orig) + + expect(duplicate_subscription.reload.member_id).to eq(orig.id) + expect(duplicate_subscription.discarded_at).to be_nil + end + end + + describe 'Merger invitations' do + it 'moves the duplicate invitation when the roles differ on the same event' do + dup = create_duplicate + orig = create_original + event = Fabricate(:event) + Fabricate(:coach_invitation, member: orig, event:) + duplicate_invitation = Fabricate(:invitation, member: dup, event:) + + merge!(dup, orig) + + expect(duplicate_invitation.reload.member_id).to eq(orig.id) + end + + it 'deletes the duplicate invitation when the original has the same event and role' do + dup = create_duplicate + orig = create_original + event = Fabricate(:event) + Fabricate(:invitation, member: orig, event:) + duplicate_invitation = Fabricate(:invitation, member: dup, event:) + + merge!(dup, orig) + + expect { duplicate_invitation.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + describe 'Merger deactivation' do + it 're-points the codebar auth service, renames the duplicate, and strips its services' do + dup = create_duplicate + orig = create_original + old_uid = dup.auth_services.find_by(provider: 'codebar').uid + + merge!(dup, orig) + + expect(dup.reload.email).to eq("duplicate.#{dup.id}.merged-into.#{orig.id}@codebar.io") + expect(dup.auth_services).to be_empty + expect(orig.auth_services.where(provider: 'codebar').pluck(:uid)).to include(old_uid) + end + end + + describe 'Merger feedback_requests' do + it 'moves the duplicate request when the original has none for that workshop' do + dup = create_duplicate + orig = create_original + request = Fabricate(:feedback_request, member: dup) + + merge!(dup, orig) + + expect(request.reload.member_id).to eq(orig.id) + end + + it 'keeps the duplicate request on the renamed member when the original already has one for that workshop' do + dup = create_duplicate + orig = create_original + request = Fabricate(:feedback_request, member: dup) + Fabricate(:feedback_request, member: orig, workshop: request.workshop, token: 'orig_token') + + merge!(dup, orig) + + expect(request.reload.member_id).to eq(dup.id) + end + end + + describe 'Detector firstname_uid_surname_matches' do + subject(:matches) do + MergeDuplicateMembers::Detector.new.call.map do |match| + [match.dup_member_id, match.original_member_id, match.merge_strategies] + end + end + + it 'does not match on first name alone when the original surname is blank' do + create_duplicate(name: 'joana', surname: nil, uid: 'j.pocopkaite@gmail.com') + create_original(name: 'Joana', surname: '', email: 'joana.afonso1@outlook.com') + + expect(matches).to be_empty + end + + it 'matches when the original surname appears in the duplicate uid' do + dup = create_duplicate(name: 'joana', surname: nil, uid: 'j.pocopkaite@gmail.com') + orig = create_original(name: 'Joana', surname: 'Pocopkaite', email: 'joana.afonso1@outlook.com') + + expect(matches).to contain_exactly( + [dup.id, orig.id, 'first-name+uid-surname'] + ) + end + end +end