FIX: recalculating trust levels was not working (#20492)

The recalculate code was never firing cause TrustLevel.calculate unconditionally
returned a trust level. (albeit a wrong one)

New code ensures we only bypass promotion checks for cases where trust level
is locked.

see: https://meta.discourse.org/t/user-trust-level-resets-to-zero-when-unlocked/255444

lib/promotion.rb

@@ -133,17 +133,23 @@ class Promotion
   # Figure out what a user's trust level should be from scratch
   def self.recalculate(user, performed_by = nil, use_previous_trust_level: false)
     granted_trust_level =
-      TrustLevel.calculate(user, use_previous_trust_level: use_previous_trust_level)
-    return user.update(trust_level: granted_trust_level) if granted_trust_level.present?
+      TrustLevel.calculate(user, use_previous_trust_level: use_previous_trust_level) ||
+        TrustLevel[0]
 
-    user.update_column(:trust_level, TrustLevel[0])
+    # TrustLevel.calculate always returns a value, however we added extra protection just
+    # in case this changes
+    user.update_column(:trust_level, TrustLevel[granted_trust_level])
 
-    p = Promotion.new(user)
-    p.review_tl0
-    p.review_tl1
-    p.review_tl2
-    if user.trust_level == 3 && Promotion.tl3_lost?(user)
-      user.change_trust_level!(2, log_action_for: performed_by || Discourse.system_user)
+    return if user.manual_locked_trust_level.present?
+
+    promotion = Promotion.new(user)
+
+    promotion.review_tl0 if granted_trust_level < TrustLevel[1]
+    promotion.review_tl1 if granted_trust_level < TrustLevel[2]
+    promotion.review_tl2 if granted_trust_level < TrustLevel[3]
+
+    if user.trust_level == TrustLevel[3] && Promotion.tl3_lost?(user)
+      user.change_trust_level!(TrustLevel[2], log_action_for: performed_by || Discourse.system_user)
     end
   end
 end

lib/trust_level.rb

@@ -38,7 +38,7 @@ class TrustLevel
     granted_trust_level = user.group_granted_trust_level || 0
     previous_trust_level = use_previous_trust_level ? find_previous_trust_level(user) : 0
 
-    [granted_trust_level, previous_trust_level].max
+    [granted_trust_level, previous_trust_level, SiteSetting.default_trust_level].max
   end
 
   private

spec/lib/promotion_spec.rb

@@ -99,9 +99,18 @@ RSpec.describe Promotion do
         stat.posts_read_count = SiteSetting.tl1_requires_read_posts
         stat.time_read = SiteSetting.tl1_requires_time_spent_mins * 60
         Promotion.recalculate(user)
+
+        expect(user.trust_level).to eq(1)
+
         expect(Jobs::SendSystemMessage.jobs.length).to eq(0)
       end
 
+      it "respects default trust level" do
+        SiteSetting.default_trust_level = 2
+        Promotion.recalculate(user)
+        expect(user.trust_level).to eq(2)
+      end
+
       it "can be turned off" do
         SiteSetting.send_tl1_welcome_message = false
         @result = promotion.review

spec/models/group_spec.rb

@@ -253,7 +253,7 @@ RSpec.describe Group do
         tl3_users = Group.find(Group::AUTO_GROUPS[:trust_level_3])
         tl3_users.add(user)
 
-        events = DiscourseEvent.track_events { Group.refresh_automatic_group!(:trust_level_3) }
+        _events = DiscourseEvent.track_events { Group.refresh_automatic_group!(:trust_level_3) }
 
         expect(GroupUser.exists?(group: tl3_users, user: user)).to eq(false)
         publish_event_job_args = Jobs::PublishGroupMembershipUpdates.jobs.last["args"].first
@@ -267,7 +267,7 @@ RSpec.describe Group do
 
         expect(GroupUser.exists?(group: tl0_users, user: user)).to eq(false)
 
-        events = DiscourseEvent.track_events { Group.refresh_automatic_group!(:trust_level_0) }
+        _events = DiscourseEvent.track_events { Group.refresh_automatic_group!(:trust_level_0) }
 
         expect(GroupUser.exists?(group: tl0_users, user: user)).to eq(true)
         publish_event_job_args = Jobs::PublishGroupMembershipUpdates.jobs.last["args"].first
@@ -641,8 +641,8 @@ RSpec.describe Group do
     describe "when a user has qualified for trust level 1" do
       fab!(:user) { Fabricate(:user, trust_level: 1, created_at: Time.zone.now - 10.years) }
 
-      fab!(:group) { Fabricate(:group, grant_trust_level: 1) }
-      fab!(:group2) { Fabricate(:group, grant_trust_level: 0) }
+      fab!(:group) { Fabricate(:group, grant_trust_level: 3) }
+      fab!(:group2) { Fabricate(:group, grant_trust_level: 2) }
 
       before { user.user_stat.update!(topics_entered: 999, posts_read_count: 999, time_read: 999) }
 
@@ -650,11 +650,15 @@ RSpec.describe Group do
         group.add(user)
         group2.add(user)
 
-        expect(user.reload.trust_level).to eq(1)
+        expect(user.reload.trust_level).to eq(3)
 
         group.remove(user)
 
-        expect(user.reload.trust_level).to eq(0)
+        expect(user.reload.trust_level).to eq(2)
+
+        group2.remove(user)
+
+        expect(user.reload.trust_level).to eq(1)
       end
     end
 

spec/models/group_user_spec.rb

@@ -293,15 +293,16 @@ RSpec.describe GroupUser do
       user = Fabricate(:user)
       expect(user.trust_level).to eq(1)
 
+      user.change_trust_level!(1, log_action_for: Discourse.system_user)
       user.change_trust_level!(2, log_action_for: Discourse.system_user)
-      user.change_trust_level!(3, log_action_for: Discourse.system_user)
       group.update!(grant_trust_level: 4)
 
       group_user = Fabricate(:group_user, group: group, user: user)
       expect(user.reload.trust_level).to eq(4)
 
       group_user.destroy!
-      expect(user.reload.trust_level).to eq(3)
+      # keep in mind that we do not restore tl3, cause reqs can be lost
+      expect(user.reload.trust_level).to eq(2)
     end
   end
 end