FIX: Merging users shouldn't add more than 1 secondary email

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
 

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)