From ac9577bcc7b69593bcdd204368d35492fc0cee15 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Mon, 22 Jun 2020 15:43:59 -0400 Subject: [PATCH] FIX: Don't raise an exception if we can't update the user on demotion This is causing issues when purging old users, if they are set up in the exact condition where they will be demoted into another group, but also do not have a primary email. --- lib/promotion.rb | 4 ++-- spec/services/user_destroyer_spec.rb | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/promotion.rb b/lib/promotion.rb index d96b304d5ab..c39d13f44f7 100644 --- a/lib/promotion.rb +++ b/lib/promotion.rb @@ -126,8 +126,8 @@ class Promotion # Then consider the group locked level user_group_granted_trust_level = user.group_granted_trust_level - unless user_group_granted_trust_level.blank? - return user.update!( + if user_group_granted_trust_level.present? + return user.update( trust_level: user_group_granted_trust_level ) end diff --git a/spec/services/user_destroyer_spec.rb b/spec/services/user_destroyer_spec.rb index a26da77d4ea..01e6e81027f 100644 --- a/spec/services/user_destroyer_spec.rb +++ b/spec/services/user_destroyer_spec.rb @@ -390,6 +390,17 @@ describe UserDestroyer do d.destroy(user) }.to change { User.count }.by(-1) end + + it 'can delete the user if they were to fall into another trust level and have no email' do + g2 = Fabricate(:group, grant_trust_level: 1) + g2.add(user) + + UserEmail.where(user: user).delete_all + user.reload + expect { + UserDestroyer.new(admin).destroy(user) + }.to change { User.count }.by(-1) + end end context 'user has staff action logs' do