From 874003b7b11bf4a4558a7f6f034a4a8cd97eb05b Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Tue, 10 Apr 2018 14:19:08 +0800 Subject: [PATCH] FIX: Group can't be deleted if certain users are demoted. --- app/models/group_user.rb | 11 +--- lib/promotion.rb | 12 ++--- spec/models/group_spec.rb | 103 +++++++++++++++++++++++++------------- 3 files changed, 76 insertions(+), 50 deletions(-) diff --git a/app/models/group_user.rb b/app/models/group_user.rb index 53569330128..5840b536649 100644 --- a/app/models/group_user.rb +++ b/app/models/group_user.rb @@ -82,15 +82,8 @@ class GroupUser < ActiveRecord::Base .includes(:group) .maximum("groups.grant_trust_level") - if highest_level.nil? - # If the user no longer has a group with a trust level, - # unlock them, start at 0 and consider promotions. - user.update!(group_locked_trust_level: nil) - Promotion.recalculate(user) - else - user.update!(group_locked_trust_level: highest_level) - user.change_trust_level!(highest_level) - end + user.update!(group_locked_trust_level: highest_level) + Promotion.recalculate(user) end end diff --git a/lib/promotion.rb b/lib/promotion.rb index e40c39e10a6..d1d16b5e914 100644 --- a/lib/promotion.rb +++ b/lib/promotion.rb @@ -111,16 +111,16 @@ class Promotion def self.recalculate(user, performed_by = nil) # First, use the manual locked level unless user.manual_locked_trust_level.nil? - user.trust_level = user.manual_locked_trust_level - user.save - return + return user.update!( + trust_level: user.manual_locked_trust_level + ) end # Then consider the group locked level if user.group_locked_trust_level - user.trust_level = user.group_locked_trust_level - user.save - return + return user.update!( + trust_level: user.group_locked_trust_level + ) end user.update_column(:trust_level, TrustLevel[0]) diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index e39266ed46e..d4d6e5a954b 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -541,50 +541,83 @@ describe Group do end end - it "correctly grants a trust level to members" do - group = Fabricate(:group, grant_trust_level: 2) - u0 = Fabricate(:user, trust_level: 0) - u3 = Fabricate(:user, trust_level: 3) + describe 'trust level management' do + it "correctly grants a trust level to members" do + group = Fabricate(:group, grant_trust_level: 2) + u0 = Fabricate(:user, trust_level: 0) + u3 = Fabricate(:user, trust_level: 3) - group.add(u0) - expect(u0.reload.trust_level).to eq(2) + group.add(u0) + expect(u0.reload.trust_level).to eq(2) - group.add(u3) - expect(u3.reload.trust_level).to eq(3) - end + group.add(u3) + expect(u3.reload.trust_level).to eq(3) + end - it "adjusts the user trust level" do - g0 = Fabricate(:group, grant_trust_level: 2) - g1 = Fabricate(:group, grant_trust_level: 3) - g2 = Fabricate(:group) + describe 'when a user has qualified for trust level 1' do + let(:user) do + Fabricate(:user, + trust_level: 1, + created_at: Time.zone.now - 10.years + ) + end - user = Fabricate(:user, trust_level: 0) + let(:group) { Fabricate(:group, grant_trust_level: 1) } + let(:group2) { Fabricate(:group, grant_trust_level: 0) } - # Add a group without one to consider `NULL` check - g2.add(user) - expect(user.group_locked_trust_level).to be_nil - expect(user.manual_locked_trust_level).to be_nil + before do + user.user_stat.update!( + topics_entered: 999, + posts_read_count: 999, + time_read: 999 + ) + end - g0.add(user) - expect(user.reload.trust_level).to eq(2) - expect(user.group_locked_trust_level).to eq(2) - expect(user.manual_locked_trust_level).to be_nil + it "should not demote the user" do + group.add(user) + group2.add(user) - g1.add(user) - expect(user.reload.trust_level).to eq(3) - expect(user.group_locked_trust_level).to eq(3) - expect(user.manual_locked_trust_level).to be_nil + expect(user.reload.trust_level).to eq(1) - g1.remove(user) - expect(user.reload.trust_level).to eq(2) - expect(user.group_locked_trust_level).to eq(2) - expect(user.manual_locked_trust_level).to be_nil + group.remove(user) - g0.remove(user) - user.reload - expect(user.manual_locked_trust_level).to be_nil - expect(user.group_locked_trust_level).to be_nil - expect(user.trust_level).to eq(0) + expect(user.reload.trust_level).to eq(0) + end + end + + it "adjusts the user trust level" do + g0 = Fabricate(:group, grant_trust_level: 2) + g1 = Fabricate(:group, grant_trust_level: 3) + g2 = Fabricate(:group) + + user = Fabricate(:user, trust_level: 0) + + # Add a group without one to consider `NULL` check + g2.add(user) + expect(user.group_locked_trust_level).to be_nil + expect(user.manual_locked_trust_level).to be_nil + + g0.add(user) + expect(user.reload.trust_level).to eq(2) + expect(user.group_locked_trust_level).to eq(2) + expect(user.manual_locked_trust_level).to be_nil + + g1.add(user) + expect(user.reload.trust_level).to eq(3) + expect(user.group_locked_trust_level).to eq(3) + expect(user.manual_locked_trust_level).to be_nil + + g1.remove(user) + expect(user.reload.trust_level).to eq(2) + expect(user.group_locked_trust_level).to eq(2) + expect(user.manual_locked_trust_level).to be_nil + + g0.remove(user) + user.reload + expect(user.manual_locked_trust_level).to be_nil + expect(user.group_locked_trust_level).to be_nil + expect(user.trust_level).to eq(0) + end end it 'should cook the bio' do