From a8df9778b54e014c4f635f9927ddb49492d8a243 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Tue, 23 Jul 2013 17:58:26 -0400 Subject: [PATCH] Rename AdminLog to StaffActionLog --- app/controllers/admin/users_controller.rb | 2 +- .../{admin_log.rb => staff_action_log.rb} | 16 ++--- .../services/staff_action_logger.rb | 14 ++--- db/migrate/20130723212758_rename_admin_log.rb | 11 ++++ lib/user_destroyer.rb | 4 +- spec/components/admin_logger_spec.rb | 62 ------------------- spec/components/boost_trust_level_spec.rb | 9 ++- spec/components/user_destroyer_spec.rb | 6 +- .../admin/users_controller_spec.rb | 4 +- spec/models/admin_log_spec.rb | 5 -- spec/models/staff_action_log_spec.rb | 5 ++ spec/services/staff_action_logger_spec.rb | 61 ++++++++++++++++++ 12 files changed, 103 insertions(+), 96 deletions(-) rename app/models/{admin_log.rb => staff_action_log.rb} (51%) rename lib/admin_logger.rb => app/services/staff_action_logger.rb (80%) create mode 100644 db/migrate/20130723212758_rename_admin_log.rb delete mode 100644 spec/components/admin_logger_spec.rb delete mode 100644 spec/models/admin_log_spec.rb create mode 100644 spec/models/staff_action_log_spec.rb create mode 100644 spec/services/staff_action_logger_spec.rb diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index b097ed82573..1fad23fe242 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -72,7 +72,7 @@ class Admin::UsersController < Admin::AdminController def trust_level guardian.ensure_can_change_trust_level!(@user) - logger = AdminLogger.new(current_user) + logger = StaffActionLogger.new(current_user) BoostTrustLevel.new(user: @user, level: params[:level], logger: logger).save! render_serialized(@user, AdminUserSerializer) end diff --git a/app/models/admin_log.rb b/app/models/staff_action_log.rb similarity index 51% rename from app/models/admin_log.rb rename to app/models/staff_action_log.rb index e98323ddbae..b43c1a04e2d 100644 --- a/app/models/admin_log.rb +++ b/app/models/staff_action_log.rb @@ -1,11 +1,11 @@ -# AdminLog stores information about actions that admins and moderators have taken, +# StaffActionLog stores information about actions that staff members have taken, # like deleting users, changing site settings, etc. -# Use the AdminLogger class to log records to this table. -class AdminLog < ActiveRecord::Base - belongs_to :admin, class_name: 'User' - belongs_to :target_user, class_name: 'User' # can be nil, or return nil if user record was nuked +# Use the StaffActionLogger class to log records to this table. +class StaffActionLog < ActiveRecord::Base + belongs_to :staff_user, class_name: 'User' + belongs_to :target_user, class_name: 'User' - validates_presence_of :admin_id + validates_presence_of :staff_user_id validates_presence_of :action def self.actions @@ -15,11 +15,11 @@ end # == Schema Information # -# Table name: admin_logs +# Table name: staff_action_logs # # id :integer not null, primary key # action :integer not null -# admin_id :integer not null +# staff_user_id :integer not null # target_user_id :integer # details :text # created_at :datetime not null diff --git a/lib/admin_logger.rb b/app/services/staff_action_logger.rb similarity index 80% rename from lib/admin_logger.rb rename to app/services/staff_action_logger.rb index c74dfee3896..c46c0bdffc9 100644 --- a/lib/admin_logger.rb +++ b/app/services/staff_action_logger.rb @@ -1,5 +1,5 @@ # Responsible for logging the actions of admins and moderators. -class AdminLogger +class StaffActionLogger def initialize(admin) @admin = admin raise Discourse::InvalidParameters.new('admin is nil') unless @admin and @admin.is_a?(User) @@ -7,9 +7,9 @@ class AdminLogger def log_user_deletion(deleted_user) raise Discourse::InvalidParameters.new('user is nil') unless deleted_user and deleted_user.is_a?(User) - AdminLog.create( - action: AdminLog.actions[:delete_user], - admin_id: @admin.id, + StaffActionLog.create( + action: StaffActionLog.actions[:delete_user], + staff_user_id: @admin.id, target_user_id: deleted_user.id, details: [:id, :username, :name, :created_at, :trust_level, :last_seen_at, :last_emailed_at].map { |x| "#{x}: #{deleted_user.send(x)}" }.join(', ') ) @@ -18,9 +18,9 @@ class AdminLogger def log_trust_level_change(user, new_trust_level) raise Discourse::InvalidParameters.new('user is nil') unless user and user.is_a?(User) raise Discourse::InvalidParameters.new('new trust level is invalid') unless TrustLevel.levels.values.include? new_trust_level - AdminLog.create!( - action: AdminLog.actions[:change_trust_level], - admin_id: @admin.id, + StaffActionLog.create!( + action: StaffActionLog.actions[:change_trust_level], + staff_user_id: @admin.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}" ) end diff --git a/db/migrate/20130723212758_rename_admin_log.rb b/db/migrate/20130723212758_rename_admin_log.rb new file mode 100644 index 00000000000..5e33a253256 --- /dev/null +++ b/db/migrate/20130723212758_rename_admin_log.rb @@ -0,0 +1,11 @@ +class RenameAdminLog < ActiveRecord::Migration + def up + rename_table :admin_logs, :staff_action_logs + rename_column :staff_action_logs, :admin_id, :staff_user_id + end + + def down + rename_table :staff_action_logs, :admin_logs + rename_column :admin_logs, :staff_user_id, :admin_id + end +end diff --git a/lib/user_destroyer.rb b/lib/user_destroyer.rb index bd6b4aa9334..e23f60054e3 100644 --- a/lib/user_destroyer.rb +++ b/lib/user_destroyer.rb @@ -1,5 +1,3 @@ -require_dependency 'admin_logger' - # Responsible for destroying a User record class UserDestroyer @@ -20,7 +18,7 @@ class UserDestroyer user.destroy.tap do |u| if u Post.with_deleted.where(user_id: user.id).update_all("nuked_user = true") - AdminLogger.new(@admin).log_user_deletion(user) + StaffActionLogger.new(@admin).log_user_deletion(user) DiscourseHub.unregister_nickname(user.username) if SiteSetting.call_discourse_hub? MessageBus.publish "/file-change", ["refresh"], user_ids: [user.id] end diff --git a/spec/components/admin_logger_spec.rb b/spec/components/admin_logger_spec.rb deleted file mode 100644 index c88f487da6f..00000000000 --- a/spec/components/admin_logger_spec.rb +++ /dev/null @@ -1,62 +0,0 @@ -require 'spec_helper' -require_dependency 'admin_logger' - -describe AdminLogger do - - describe 'new' do - it 'raises an error when user is nil' do - expect { AdminLogger.new(nil) }.to raise_error(Discourse::InvalidParameters) - end - - it 'raises an error when user is not a User' do - expect { AdminLogger.new(5) }.to raise_error(Discourse::InvalidParameters) - end - end - - describe 'log_user_deletion' do - let(:admin) { Fabricate(:admin) } - let(:deleted_user) { Fabricate(:user) } - - subject(:log_user_deletion) { AdminLogger.new(admin).log_user_deletion(deleted_user) } - - it 'raises an error when user is nil' do - expect { AdminLogger.new(admin).log_user_deletion(nil) }.to raise_error(Discourse::InvalidParameters) - end - - it 'raises an error when user is not a User' do - expect { AdminLogger.new(admin).log_user_deletion(1) }.to raise_error(Discourse::InvalidParameters) - end - - it 'creates a new AdminLog record' do - expect { log_user_deletion }.to change { AdminLog.count }.by(1) - AdminLog.last.target_user_id.should == deleted_user.id - end - end - - describe 'log_trust_level_change' do - let(:admin) { Fabricate(:admin) } - let(:user) { Fabricate(:user) } - let(:new_trust_level) { TrustLevel.levels[:basic] } - - subject(:log_trust_level_change) { AdminLogger.new(admin).log_trust_level_change(user, new_trust_level) } - - it 'raises an error when user or trust level is nil' do - expect { AdminLogger.new(admin).log_trust_level_change(nil, new_trust_level) }.to raise_error(Discourse::InvalidParameters) - expect { AdminLogger.new(admin).log_trust_level_change(user, nil) }.to raise_error(Discourse::InvalidParameters) - end - - it 'raises an error when user is not a User' do - expect { AdminLogger.new(admin).log_trust_level_change(1, 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 { AdminLogger.new(admin).log_trust_level_change(user, max_level + 1) }.to raise_error(Discourse::InvalidParameters) - end - - it 'creates a new AdminLog record' do - expect { log_trust_level_change }.to change { AdminLog.count }.by(1) - AdminLog.last.details.should include "new trust level: #{new_trust_level}" - end - end -end diff --git a/spec/components/boost_trust_level_spec.rb b/spec/components/boost_trust_level_spec.rb index 3974aecbb62..674a540e99e 100644 --- a/spec/components/boost_trust_level_spec.rb +++ b/spec/components/boost_trust_level_spec.rb @@ -1,11 +1,10 @@ require 'spec_helper' require 'boost_trust_level' -require 'admin_logger' describe BoostTrustLevel do let(:user) { Fabricate(:user) } - let(:logger) { AdminLogger.new(Fabricate(:admin)) } + let(:logger) { StaffActionLogger.new(Fabricate(:admin)) } it "should upgrade the trust level of a user" do @@ -15,7 +14,7 @@ describe BoostTrustLevel do end it "should log the action" do - AdminLogger.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[:basic]).once boostr = BoostTrustLevel.new(user: user, level: TrustLevel.levels[:basic], logger: logger) boostr.save! end @@ -31,7 +30,7 @@ describe BoostTrustLevel do end it "should demote the user and log the action" do - AdminLogger.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[: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] @@ -49,7 +48,7 @@ describe BoostTrustLevel do end it "should not demote the user and not log the action" do - AdminLogger.any_instance.expects(:log_trust_level_change).with(user, TrustLevel.levels[:newuser]).never + StaffActionLogger.any_instance.expects(:log_trust_level_change).with(user, TrustLevel.levels[:newuser]).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/components/user_destroyer_spec.rb b/spec/components/user_destroyer_spec.rb index 357e10c7961..8d17fc68545 100644 --- a/spec/components/user_destroyer_spec.rb +++ b/spec/components/user_destroyer_spec.rb @@ -59,7 +59,7 @@ describe UserDestroyer do end it 'should not log the action' do - AdminLogger.any_instance.expects(:log_user_deletion).never + StaffActionLogger.any_instance.expects(:log_user_deletion).never destroy rescue nil end @@ -82,7 +82,7 @@ describe UserDestroyer do end it 'should log the action' do - AdminLogger.any_instance.expects(:log_user_deletion).with(@user).once + StaffActionLogger.any_instance.expects(:log_user_deletion).with(@user).once destroy end @@ -115,7 +115,7 @@ describe UserDestroyer do end it 'should not log the action' do - AdminLogger.any_instance.expects(:log_user_deletion).never + StaffActionLogger.any_instance.expects(:log_user_deletion).never destroy end diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 036e557bb0a..454041934a1 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 - AdminLogger.any_instance.expects(:log_trust_level_change).with(@another_user, 2).once + StaffActionLogger.any_instance.expects(:log_trust_level_change).with(@another_user, 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 - AdminLogger.any_instance.expects(:log_trust_level_change).with(@another_user, TrustLevel.levels[:newuser]).never + StaffActionLogger.any_instance.expects(:log_trust_level_change).with(@another_user, TrustLevel.levels[:newuser]).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/models/admin_log_spec.rb b/spec/models/admin_log_spec.rb deleted file mode 100644 index 4d006564198..00000000000 --- a/spec/models/admin_log_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require 'spec_helper' - -describe AdminLog do - # Nothing fancy going on in this model. See AdminLogger. -end diff --git a/spec/models/staff_action_log_spec.rb b/spec/models/staff_action_log_spec.rb new file mode 100644 index 00000000000..30ebbf1557a --- /dev/null +++ b/spec/models/staff_action_log_spec.rb @@ -0,0 +1,5 @@ +require 'spec_helper' + +describe StaffActionLog do + # Nothing fancy going on in this model. See StaffActionLogger. +end diff --git a/spec/services/staff_action_logger_spec.rb b/spec/services/staff_action_logger_spec.rb new file mode 100644 index 00000000000..b7d12cb7273 --- /dev/null +++ b/spec/services/staff_action_logger_spec.rb @@ -0,0 +1,61 @@ +require 'spec_helper' + +describe StaffActionLogger do + + describe 'new' do + it 'raises an error when user is nil' do + expect { described_class.new(nil) }.to raise_error(Discourse::InvalidParameters) + end + + it 'raises an error when user is not a User' do + expect { described_class.new(5) }.to raise_error(Discourse::InvalidParameters) + end + end + + describe 'log_user_deletion' do + let(:admin) { Fabricate(:admin) } + let(:deleted_user) { Fabricate(:user) } + + subject(:log_user_deletion) { described_class.new(admin).log_user_deletion(deleted_user) } + + it 'raises an error when user is nil' do + expect { described_class.new(admin).log_user_deletion(nil) }.to raise_error(Discourse::InvalidParameters) + end + + it 'raises an error when user is not a User' do + expect { described_class.new(admin).log_user_deletion(1) }.to raise_error(Discourse::InvalidParameters) + end + + it 'creates a new StaffActionLog record' do + expect { log_user_deletion }.to change { StaffActionLog.count }.by(1) + StaffActionLog.last.target_user_id.should == deleted_user.id + end + end + + describe 'log_trust_level_change' do + let(:admin) { Fabricate(:admin) } + let(:user) { Fabricate(:user) } + let(:new_trust_level) { TrustLevel.levels[:basic] } + + subject(:log_trust_level_change) { described_class.new(admin).log_trust_level_change(user, 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) + 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) + 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) + end + + it 'creates a new StaffActionLog record' do + expect { log_trust_level_change }.to change { StaffActionLog.count }.by(1) + StaffActionLog.last.details.should include "new trust level: #{new_trust_level}" + end + end +end