FIX: down downgrade trust level if all requirements are met. (#25953)

Currently, the trust level method  is calculating trust level based on maximum value from:
- locked trust level
- group automatic trust level
- previously granted trust level by admin

https://github.com/discourse/discourse/blob/main/lib/trust_level.rb#L33

Let's say the user belongs to groups with automatic trust level 1 and in the meantime meets all criteria to get trust level 2.

Each time, a user is removed from a group with automatic trust_level 1, they will be downgraded to trust_level 1 and promoted to trust_level 2

120a2f70a9/lib/promotion.rb (L142)

This will cause duplicated promotion messages.

Therefore, we have to check if the user meets the criteria, before downgrading.
This commit is contained in:
Krzysztof Kotlarek 2024-03-04 09:30:30 +11:00 committed by GitHub
parent b5199eac80
commit 41f78b31a9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 33 additions and 0 deletions

View File

@ -137,6 +137,9 @@ class Promotion
TrustLevel.calculate(user, use_previous_trust_level: use_previous_trust_level) || TrustLevel.calculate(user, use_previous_trust_level: use_previous_trust_level) ||
TrustLevel[0] TrustLevel[0]
granted_trust_level = user.trust_level if granted_trust_level < user.trust_level &&
!can_downgrade_trust_level?(user)
# TrustLevel.calculate always returns a value, however we added extra protection just # TrustLevel.calculate always returns a value, however we added extra protection just
# in case this changes # in case this changes
user.update_column(:trust_level, TrustLevel[granted_trust_level]) user.update_column(:trust_level, TrustLevel[granted_trust_level])
@ -153,4 +156,12 @@ class Promotion
user.change_trust_level!(TrustLevel[2], log_action_for: performed_by || Discourse.system_user) user.change_trust_level!(TrustLevel[2], log_action_for: performed_by || Discourse.system_user)
end end
end end
def self.can_downgrade_trust_level?(user)
return false if user.trust_level == TrustLevel[1] && tl1_met?(user)
return false if user.trust_level == TrustLevel[2] && tl2_met?(user)
return false if user.trust_level == TrustLevel[3] && tl3_met?(user)
true
end
end end

View File

@ -303,5 +303,27 @@ RSpec.describe GroupUser do
# keep in mind that we do not restore tl3, cause reqs can be lost # keep in mind that we do not restore tl3, cause reqs can be lost
expect(user.reload.trust_level).to eq(2) expect(user.reload.trust_level).to eq(2)
end end
it "protects user trust level if all requirements are met" do
Promotion.stubs(:tl2_met?).returns(true)
user = Fabricate(:user)
expect(user.trust_level).to eq(1)
group.update!(grant_trust_level: 1)
Promotion.recalculate(user)
expect(user.reload.trust_level).to eq(2)
group_user = Fabricate(:group_user, group: group, user: user)
expect_not_enqueued_with(
job: :send_system_message,
args: {
user_id: user.id,
message_type: "tl2_promotion_message",
},
) { group_user.destroy! }
expect(user.reload.trust_level).to eq(2)
end
end end
end end