Change trust level logs the previous trust level

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

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
 

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]

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

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(nil, old_trust_level, 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(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