FIX: Don't raise an exception if we can't update the user on demotion

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.

diff --git a/lib/promotion.rb b/lib/promotion.rb
index d96b304..c39d13f 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 a26da77..01e6e81 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

GitHub sha: ac9577bc

This commit has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/requesting-help-with-failure-finding-discourseredis-connector-running-tests/155599/2