From b970b072f6a5ab877f8174ec81f1513527596ce6 Mon Sep 17 00:00:00 2001 From: Gerhard Schlager Date: Fri, 1 Jun 2018 16:23:21 +0200 Subject: [PATCH] FIX: User merge should not fail when primary email address is missing The merge process might move all email addresses of the source user to the target user. Destroying the source user failed in that case. --- app/services/user_merger.rb | 6 +++++- spec/services/user_merger_spec.rb | 9 +++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/app/services/user_merger.rb b/app/services/user_merger.rb index d745bc0e7cc..c8e8cc530ec 100644 --- a/app/services/user_merger.rb +++ b/app/services/user_merger.rb @@ -347,7 +347,11 @@ class UserMerger def delete_source_user @source_user.reload - @source_user.update_attribute(:admin, false) + @source_user.update_attributes( + admin: false, + email: "#{@source_user.username}_#{SecureRandom.hex}@no-email.invalid" + ) + UserDestroyer.new(Discourse.system_user).destroy(@source_user) end diff --git a/spec/services/user_merger_spec.rb b/spec/services/user_merger_spec.rb index 2d81ad8e3be..6306291138b 100644 --- a/spec/services/user_merger_spec.rb +++ b/spec/services/user_merger_spec.rb @@ -953,6 +953,15 @@ describe UserMerger do expect(User.find_by_username(source_user.username)).to be_nil end + it "deletes the source user even when it is a member of a group that grants a trust level" do + group = Fabricate(:group, grant_trust_level: 3) + group.bulk_add([source_user.id, target_user.id]) + + merge_users! + + expect(User.find_by_username(source_user.username)).to be_nil + end + it "deletes external auth infos of source user" do FacebookUserInfo.create(user_id: source_user.id, facebook_user_id: "example") GithubUserInfo.create(user_id: source_user.id, screen_name: "example", github_user_id: "examplel123123")