diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb index 0a750512a..4c3cc888a 100644 --- a/app/services/staff_action_logger.rb +++ b/app/services/staff_action_logger.rb @@ -18,14 +18,15 @@ class StaffActionLogger ) end - def log_trust_level_change(user, new_trust_level, opts={}) + def log_trust_level_change(user, old_trust_level, new_trust_level, opts={}) raise Discourse::InvalidParameters.new('user is nil') unless user and user.is_a?(User) + raise Discourse::InvalidParameters.new('old trust level is invalid') unless TrustLevel.levels.values.include? old_trust_level raise Discourse::InvalidParameters.new('new trust level is invalid') unless TrustLevel.levels.values.include? new_trust_level StaffActionLog.create!( action: StaffActionLog.actions[:change_trust_level], staff_user_id: @admin.id, target_user_id: user.id, - details: [:id, :username, :name, :created_at, :trust_level, :last_seen_at, :last_emailed_at].map { |x| "#{x}: #{user.send(x)}" }.join(', ') + ", new trust level: #{new_trust_level}" + details: "old trust level: #{old_trust_level}, new trust level: #{new_trust_level}" ) end end diff --git a/lib/boost_trust_level.rb b/lib/boost_trust_level.rb index 50785270e..bb06aa0ea 100644 --- a/lib/boost_trust_level.rb +++ b/lib/boost_trust_level.rb @@ -11,12 +11,13 @@ class BoostTrustLevel end def save! + previous_level = @user.trust_level success = if @level < @user.trust_level demote! else @user.update_attributes!(trust_level: @level) end - @logger.log_trust_level_change(@user, @level) + @logger.log_trust_level_change(@user, previous_level, @level) success end diff --git a/spec/components/boost_trust_level_spec.rb b/spec/components/boost_trust_level_spec.rb index 674a540e9..951775cc3 100644 --- a/spec/components/boost_trust_level_spec.rb +++ b/spec/components/boost_trust_level_spec.rb @@ -3,7 +3,7 @@ require 'boost_trust_level' describe BoostTrustLevel do - let(:user) { Fabricate(:user) } + let(:user) { Fabricate(:user, trust_level: TrustLevel.levels[:newuser]) } let(:logger) { StaffActionLogger.new(Fabricate(:admin)) } @@ -14,13 +14,12 @@ describe BoostTrustLevel do end it "should log the action" do - StaffActionLogger.any_instance.expects(:log_trust_level_change).with(user, TrustLevel.levels[:basic]).once + StaffActionLogger.any_instance.expects(:log_trust_level_change).with(user, TrustLevel.levels[:newuser], TrustLevel.levels[:basic]).once boostr = BoostTrustLevel.new(user: user, level: TrustLevel.levels[:basic], logger: logger) boostr.save! end describe "demotions" do - before { user.update_attributes(trust_level: TrustLevel.levels[:newuser]) } context "for a user that has not done the requisite things to attain their trust level" do @@ -30,7 +29,7 @@ describe BoostTrustLevel do end it "should demote the user and log the action" do - StaffActionLogger.any_instance.expects(:log_trust_level_change).with(user, TrustLevel.levels[:newuser]).once + StaffActionLogger.any_instance.expects(:log_trust_level_change).with(user, TrustLevel.levels[:basic], TrustLevel.levels[:newuser]).once boostr = BoostTrustLevel.new(user: user, level: TrustLevel.levels[:newuser], logger: logger) boostr.save!.should be_true user.trust_level.should == TrustLevel.levels[:newuser] @@ -48,7 +47,7 @@ describe BoostTrustLevel do end it "should not demote the user and not log the action" do - StaffActionLogger.any_instance.expects(:log_trust_level_change).with(user, TrustLevel.levels[:newuser]).never + StaffActionLogger.any_instance.expects(:log_trust_level_change).never boostr = BoostTrustLevel.new(user: user, level: TrustLevel.levels[:newuser], logger: logger) expect { boostr.save! }.to raise_error(Discourse::InvalidAccess, "You attempted to demote #{user.name} to 'newuser'. However their trust level is already 'basic'. #{user.name} will remain at 'basic'") user.trust_level.should == TrustLevel.levels[:basic] diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 20bf3d592..9661ac055 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -137,14 +137,14 @@ describe Admin::UsersController do end it "upgrades the user's trust level" do - StaffActionLogger.any_instance.expects(:log_trust_level_change).with(@another_user, 2).once + StaffActionLogger.any_instance.expects(:log_trust_level_change).with(@another_user, @another_user.trust_level, 2).once xhr :put, :trust_level, user_id: @another_user.id, level: 2 @another_user.reload @another_user.trust_level.should == 2 end it "raises an error when demoting a user below their current trust level" do - StaffActionLogger.any_instance.expects(:log_trust_level_change).with(@another_user, TrustLevel.levels[:newuser]).never + StaffActionLogger.any_instance.expects(:log_trust_level_change).never @another_user.topics_entered = SiteSetting.basic_requires_topics_entered + 1 @another_user.posts_read_count = SiteSetting.basic_requires_read_posts + 1 @another_user.time_read = SiteSetting.basic_requires_time_spent_mins * 60 diff --git a/spec/services/staff_action_logger_spec.rb b/spec/services/staff_action_logger_spec.rb index b7d12cb72..62d24058b 100644 --- a/spec/services/staff_action_logger_spec.rb +++ b/spec/services/staff_action_logger_spec.rb @@ -35,22 +35,24 @@ describe StaffActionLogger do describe 'log_trust_level_change' do let(:admin) { Fabricate(:admin) } let(:user) { Fabricate(:user) } + let(:old_trust_level) { TrustLevel.levels[:newuser] } let(:new_trust_level) { TrustLevel.levels[:basic] } - subject(:log_trust_level_change) { described_class.new(admin).log_trust_level_change(user, new_trust_level) } + subject(:log_trust_level_change) { described_class.new(admin).log_trust_level_change(user, old_trust_level, new_trust_level) } it 'raises an error when user or trust level is nil' do - expect { described_class.new(admin).log_trust_level_change(nil, new_trust_level) }.to raise_error(Discourse::InvalidParameters) - expect { described_class.new(admin).log_trust_level_change(user, nil) }.to raise_error(Discourse::InvalidParameters) + expect { described_class.new(admin).log_trust_level_change(nil, old_trust_level, new_trust_level) }.to raise_error(Discourse::InvalidParameters) + expect { described_class.new(admin).log_trust_level_change(user, nil, new_trust_level) }.to raise_error(Discourse::InvalidParameters) + expect { described_class.new(admin).log_trust_level_change(user, old_trust_level, nil) }.to raise_error(Discourse::InvalidParameters) end it 'raises an error when user is not a User' do - expect { described_class.new(admin).log_trust_level_change(1, new_trust_level) }.to raise_error(Discourse::InvalidParameters) + expect { described_class.new(admin).log_trust_level_change(1, old_trust_level, new_trust_level) }.to raise_error(Discourse::InvalidParameters) end it 'raises an error when new trust level is not a Trust Level' do max_level = TrustLevel.levels.values.max - expect { described_class.new(admin).log_trust_level_change(user, max_level + 1) }.to raise_error(Discourse::InvalidParameters) + expect { described_class.new(admin).log_trust_level_change(user, old_trust_level, max_level + 1) }.to raise_error(Discourse::InvalidParameters) end it 'creates a new StaffActionLog record' do