From 79590e4becc9eb3cba2adebd772efe5eccfe71fd Mon Sep 17 00:00:00 2001 From: Gerhard Schlager Date: Thu, 1 Mar 2018 17:50:13 +0100 Subject: [PATCH] FIX: Merging users shouldn't add more than 1 secondary email --- app/services/user_merger.rb | 3 ++- spec/services/user_merger_spec.rb | 17 +++++++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/app/services/user_merger.rb b/app/services/user_merger.rb index e08a5c021a2..7f6b7dd5251 100644 --- a/app/services/user_merger.rb +++ b/app/services/user_merger.rb @@ -371,7 +371,7 @@ class UserMerger update_user_id(:user_custom_fields, conditions: "x.name = y.name") - update_user_id(:user_emails, conditions: "x.email = y.email", updates: '"primary" = false') + update_user_id(:user_emails, conditions: "x.email = y.email OR y.primary = false", updates: '"primary" = false') UserExport.where(user_id: @source_user.id).update_all(user_id: @target_user.id) @@ -405,6 +405,7 @@ class UserMerger def update_user_id(table_name, opts = {}) builder = update_user_id_sql_builder(table_name, opts) + puts builder.to_sql builder.exec(source_user_id: @source_user.id, target_user_id: @target_user.id) end diff --git a/spec/services/user_merger_spec.rb b/spec/services/user_merger_spec.rb index 8a4b3fcdf94..0890645a9e6 100644 --- a/spec/services/user_merger_spec.rb +++ b/spec/services/user_merger_spec.rb @@ -5,8 +5,10 @@ describe UserMerger do let!(:source_user) { Fabricate(:user_single_email, username: 'alice1', email: 'alice@work.com') } let(:walter) { Fabricate(:walter_white) } - def merge_users! - UserMerger.new(source_user, target_user).merge! + def merge_users!(source = nil, target = nil) + source ||= source_user + target ||= target_user + UserMerger.new(source, target).merge! end it "changes owner of topics and posts" do @@ -836,6 +838,17 @@ describe UserMerger do expect(UserEmail.where(user_id: source_user.id).count).to eq(0) end + it "skips merging email adresses when a secondary email address exists" do + merge_users!(source_user, target_user) + + alice2 = Fabricate(:user_single_email, username: 'alice2', email: 'alice@foo.com') + merge_users!(alice2, target_user) + + emails = UserEmail.where(user_id: target_user.id).pluck(:email, :primary) + expect(emails).to contain_exactly(['alice@example.com', true], ['alice@work.com', false]) + expect(UserEmail.where(user_id: source_user.id).count).to eq(0) + end + it "updates exports" do UserExport.create(file_name: "user-archive-alice1-190218-003249", user_id: source_user.id)