From e8ef55c4461c986e26c31eaa6c1a49ca9749c374 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Tue, 10 Sep 2013 21:21:16 -0400 Subject: [PATCH] Rename StaffActionLog to UserHistory --- ...admin_logs_staff_action_logs_controller.js | 8 ++--- .../admin/models/staff_action_log.js | 4 +-- .../logs/staff_action_logs.js.handlebars | 6 ++-- .../staff_action_logs_list_item.js.handlebars | 4 +-- .../admin/staff_action_logs_controller.rb | 4 +-- .../{staff_action_log.rb => user_history.rb} | 35 +++++++++++++------ ...rializer.rb => user_history_serializer.rb} | 6 ++-- app/services/staff_action_logger.rb | 22 ++++++------ ...ename_staff_action_logs_to_user_history.rb | 17 +++++++++ ...ction_log_spec.rb => user_history_spec.rb} | 2 +- spec/services/staff_action_logger_spec.rb | 10 +++--- 11 files changed, 75 insertions(+), 43 deletions(-) rename app/models/{staff_action_log.rb => user_history.rb} (57%) rename app/serializers/{staff_action_log_serializer.rb => user_history_serializer.rb} (79%) create mode 100644 db/migrate/20130910220317_rename_staff_action_logs_to_user_history.rb rename spec/models/{staff_action_log_spec.rb => user_history_spec.rb} (77%) diff --git a/app/assets/javascripts/admin/controllers/admin_logs_staff_action_logs_controller.js b/app/assets/javascripts/admin/controllers/admin_logs_staff_action_logs_controller.js index 7ae5c9064d4..457cee475bf 100644 --- a/app/assets/javascripts/admin/controllers/admin_logs_staff_action_logs_controller.js +++ b/app/assets/javascripts/admin/controllers/admin_logs_staff_action_logs_controller.js @@ -18,11 +18,11 @@ Discourse.AdminLogsStaffActionLogsController = Ember.ArrayController.extend(Disc self.set('content', result); self.set('loading', false); }); - }.observes('filters.action_name', 'filters.staff_user', 'filters.target_user', 'filters.subject'), + }.observes('filters.action_name', 'filters.acting_user', 'filters.target_user', 'filters.subject'), filtersExists: function() { return (_.size(this.get('filters')) > 0); - }.property('filters.action_name', 'filters.staff_user', 'filters.target_user', 'filters.subject'), + }.property('filters.action_name', 'filters.acting_user', 'filters.target_user', 'filters.subject'), clearFilter: function(key) { delete this.get('filters')[key]; @@ -45,8 +45,8 @@ Discourse.AdminLogsStaffActionLogsController = Ember.ArrayController.extend(Disc } }.property('filters.action_name'), - filterByStaffUser: function(staff_user) { - this.set('filters.staff_user', staff_user.username); + filterByStaffUser: function(acting_user) { + this.set('filters.acting_user', acting_user.username); }, filterByTargetUser: function(target_user) { diff --git a/app/assets/javascripts/admin/models/staff_action_log.js b/app/assets/javascripts/admin/models/staff_action_log.js index cd0dd533e2e..8b6fe250ddc 100644 --- a/app/assets/javascripts/admin/models/staff_action_log.js +++ b/app/assets/javascripts/admin/models/staff_action_log.js @@ -43,8 +43,8 @@ Discourse.StaffActionLog = Discourse.Model.extend({ Discourse.StaffActionLog.reopenClass({ create: function(attrs) { - if (attrs.staff_user) { - attrs.staff_user = Discourse.AdminUser.create(attrs.staff_user); + if (attrs.acting_user) { + attrs.acting_user = Discourse.AdminUser.create(attrs.acting_user); } if (attrs.target_user) { attrs.target_user = Discourse.AdminUser.create(attrs.target_user); diff --git a/app/assets/javascripts/admin/templates/logs/staff_action_logs.js.handlebars b/app/assets/javascripts/admin/templates/logs/staff_action_logs.js.handlebars index f4c787b026c..5600b9d7bec 100644 --- a/app/assets/javascripts/admin/templates/logs/staff_action_logs.js.handlebars +++ b/app/assets/javascripts/admin/templates/logs/staff_action_logs.js.handlebars @@ -8,9 +8,9 @@ {{/if}} - {{#if filters.staff_user}} - - {{i18n admin.logs.staff_actions.staff_user}}: {{filters.staff_user}} + {{#if filters.acting_user}} + + {{i18n admin.logs.staff_actions.staff_user}}: {{filters.acting_user}} {{/if}} diff --git a/app/assets/javascripts/admin/templates/logs/staff_action_logs_list_item.js.handlebars b/app/assets/javascripts/admin/templates/logs/staff_action_logs_list_item.js.handlebars index 0bf40c37494..749f3197075 100644 --- a/app/assets/javascripts/admin/templates/logs/staff_action_logs_list_item.js.handlebars +++ b/app/assets/javascripts/admin/templates/logs/staff_action_logs_list_item.js.handlebars @@ -1,6 +1,6 @@
- {{#linkTo 'adminUser' staff_user}}{{avatar staff_user imageSize="tiny"}}{{/linkTo}} - {{staff_user.username}} + {{#linkTo 'adminUser' acting_user}}{{avatar acting_user imageSize="tiny"}}{{/linkTo}} + {{acting_user.username}}
{{actionName}} diff --git a/app/controllers/admin/staff_action_logs_controller.rb b/app/controllers/admin/staff_action_logs_controller.rb index c6e8bab09fc..4c84be30987 100644 --- a/app/controllers/admin/staff_action_logs_controller.rb +++ b/app/controllers/admin/staff_action_logs_controller.rb @@ -1,8 +1,8 @@ class Admin::StaffActionLogsController < Admin::AdminController def index - staff_action_logs = StaffActionLog.with_filters(params.slice(:action_name, :staff_user, :target_user, :subject)).limit(200).order('id DESC').includes(:staff_user, :target_user).to_a - render_serialized(staff_action_logs, StaffActionLogSerializer) + staff_action_logs = UserHistory.with_filters(params.slice(:action_name, :acting_user, :target_user, :subject)).only_staff_actions.limit(200).order('id DESC').includes(:acting_user, :target_user).to_a + render_serialized(staff_action_logs, UserHistorySerializer) end end diff --git a/app/models/staff_action_log.rb b/app/models/user_history.rb similarity index 57% rename from app/models/staff_action_log.rb rename to app/models/user_history.rb index a7eeb60911b..9f0291fc91d 100644 --- a/app/models/staff_action_log.rb +++ b/app/models/user_history.rb @@ -1,13 +1,15 @@ -# StaffActionLog stores information about actions that staff members have taken, -# like deleting users, changing site settings, etc. -# 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' +# UserHistory stores information about actions that users have taken, +# like deleting users, changing site settings, dimissing notifications, etc. +# Use other classes, like StaffActionLogger, to log records to this table. +class UserHistory < ActiveRecord::Base + belongs_to :acting_user, class_name: 'User' + belongs_to :target_user, class_name: 'User' - validates_presence_of :staff_user_id + validates_presence_of :acting_user_id validates_presence_of :action + scope :only_staff_actions, ->{ where("action IN (?)", UserHistory.staff_action_ids) } + def self.actions @actions ||= Enum.new( :delete_user, :change_trust_level, @@ -16,12 +18,25 @@ class StaffActionLog < ActiveRecord::Base :delete_site_customization) end + # Staff actions is a subset of all actions, used to audit actions taken by staff users. + def self.staff_actions + @staff_actions ||= [:delete_user, + :change_trust_level, + :change_site_setting, + :change_site_customization, + :delete_site_customization] + end + + def self.staff_action_ids + @staff_action_ids ||= staff_actions.map { |a| actions[a] } + end + def self.with_filters(filters) query = self - if filters[:action_name] and action_id = StaffActionLog.actions[filters[:action_name].to_sym] + if filters[:action_name] and action_id = UserHistory.actions[filters[:action_name].to_sym] query = query.where('action = ?', action_id) end - [:staff_user, :target_user].each do |key| + [:acting_user, :target_user].each do |key| if filters[key] and obj_id = User.where(username_lower: filters[key].downcase).pluck(:id) query = query.where("#{key.to_s}_id = ?", obj_id) end @@ -31,7 +46,7 @@ class StaffActionLog < ActiveRecord::Base end def new_value_is_json? - [StaffActionLog.actions[:change_site_customization], StaffActionLog.actions[:delete_site_customization]].include?(action) + [UserHistory.actions[:change_site_customization], UserHistory.actions[:delete_site_customization]].include?(action) end def previous_value_is_json? diff --git a/app/serializers/staff_action_log_serializer.rb b/app/serializers/user_history_serializer.rb similarity index 79% rename from app/serializers/staff_action_log_serializer.rb rename to app/serializers/user_history_serializer.rb index 5451af8af65..14041d65994 100644 --- a/app/serializers/staff_action_log_serializer.rb +++ b/app/serializers/user_history_serializer.rb @@ -1,4 +1,4 @@ -class StaffActionLogSerializer < ApplicationSerializer +class UserHistorySerializer < ApplicationSerializer attributes :action_name, :details, :context, @@ -9,11 +9,11 @@ class StaffActionLogSerializer < ApplicationSerializer :previous_value, :new_value - has_one :staff_user, serializer: BasicUserSerializer, embed: :objects + has_one :acting_user, serializer: BasicUserSerializer, embed: :objects has_one :target_user, serializer: BasicUserSerializer, embed: :objects def action_name - StaffActionLog.actions.key(object.action).to_s + UserHistory.actions.key(object.action).to_s end def new_value diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb index aa2bdbaa450..f291ae20efc 100644 --- a/app/services/staff_action_logger.rb +++ b/app/services/staff_action_logger.rb @@ -7,8 +7,8 @@ class StaffActionLogger def log_user_deletion(deleted_user, opts={}) raise Discourse::InvalidParameters.new('user is nil') unless deleted_user and deleted_user.is_a?(User) - StaffActionLog.create( params(opts).merge({ - action: StaffActionLog.actions[:delete_user], + UserHistory.create( params(opts).merge({ + action: UserHistory.actions[:delete_user], target_user_id: deleted_user.id, email: deleted_user.email, ip_address: deleted_user.ip_address, @@ -20,8 +20,8 @@ class StaffActionLogger 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!( params(opts).merge({ - action: StaffActionLog.actions[:change_trust_level], + UserHistory.create!( params(opts).merge({ + action: UserHistory.actions[:change_trust_level], target_user_id: user.id, details: "old trust level: #{old_trust_level}, new trust level: #{new_trust_level}" })) @@ -29,8 +29,8 @@ class StaffActionLogger def log_site_setting_change(setting_name, previous_value, new_value, opts={}) raise Discourse::InvalidParameters.new('setting_name is invalid') unless setting_name.present? and SiteSetting.respond_to?(setting_name) - StaffActionLog.create( params(opts).merge({ - action: StaffActionLog.actions[:change_site_setting], + UserHistory.create( params(opts).merge({ + action: UserHistory.actions[:change_site_setting], subject: setting_name, previous_value: previous_value, new_value: new_value @@ -41,8 +41,8 @@ class StaffActionLogger def log_site_customization_change(old_record, site_customization_params, opts={}) raise Discourse::InvalidParameters.new('site_customization_params is nil') unless site_customization_params - StaffActionLog.create( params(opts).merge({ - action: StaffActionLog.actions[:change_site_customization], + UserHistory.create( params(opts).merge({ + action: UserHistory.actions[:change_site_customization], subject: site_customization_params[:name], previous_value: old_record ? old_record.attributes.slice(*SITE_CUSTOMIZATION_LOGGED_ATTRS).to_json : nil, new_value: site_customization_params.slice(*(SITE_CUSTOMIZATION_LOGGED_ATTRS.map(&:to_sym))).to_json @@ -51,8 +51,8 @@ class StaffActionLogger def log_site_customization_destroy(site_customization, opts={}) raise Discourse::InvalidParameters.new('site_customization is nil') unless site_customization - StaffActionLog.create( params(opts).merge({ - action: StaffActionLog.actions[:delete_site_customization], + UserHistory.create( params(opts).merge({ + action: UserHistory.actions[:delete_site_customization], subject: site_customization.name, previous_value: site_customization.attributes.slice(*SITE_CUSTOMIZATION_LOGGED_ATTRS).to_json })) @@ -61,7 +61,7 @@ class StaffActionLogger private def params(opts) - {staff_user_id: @admin.id, context: opts[:context]} + {acting_user_id: @admin.id, context: opts[:context]} end end diff --git a/db/migrate/20130910220317_rename_staff_action_logs_to_user_history.rb b/db/migrate/20130910220317_rename_staff_action_logs_to_user_history.rb new file mode 100644 index 00000000000..7816133106d --- /dev/null +++ b/db/migrate/20130910220317_rename_staff_action_logs_to_user_history.rb @@ -0,0 +1,17 @@ +class RenameStaffActionLogsToUserHistory < ActiveRecord::Migration + def up + remove_index :staff_action_logs, [:staff_user_id, :id] + rename_table :staff_action_logs, :user_histories + rename_column :user_histories, :staff_user_id, :acting_user_id + execute "ALTER INDEX staff_action_logs_pkey RENAME TO user_histories_pkey" + add_index :user_histories, [:acting_user_id, :action, :id] + end + + def down + remove_index :user_histories, [:acting_user_id, :action, :id] + rename_table :user_histories, :staff_action_logs + rename_column :staff_action_logs, :acting_user_id, :staff_user_id + execute "ALTER INDEX user_histories_pkey RENAME TO staff_action_logs_pkey" + add_index :staff_action_logs, [:staff_user_id, :id] + end +end diff --git a/spec/models/staff_action_log_spec.rb b/spec/models/user_history_spec.rb similarity index 77% rename from spec/models/staff_action_log_spec.rb rename to spec/models/user_history_spec.rb index 30ebbf1557a..0a05ab41266 100644 --- a/spec/models/staff_action_log_spec.rb +++ b/spec/models/user_history_spec.rb @@ -1,5 +1,5 @@ require 'spec_helper' -describe StaffActionLog do +describe UserHistory 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 index 3259857f639..19d99bfcd34 100644 --- a/spec/services/staff_action_logger_spec.rb +++ b/spec/services/staff_action_logger_spec.rb @@ -29,8 +29,8 @@ describe StaffActionLogger do 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 + expect { log_user_deletion }.to change { UserHistory.count }.by(1) + UserHistory.last.target_user_id.should == deleted_user.id end end @@ -57,8 +57,8 @@ describe StaffActionLogger do 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}" + expect { log_trust_level_change }.to change { UserHistory.count }.by(1) + UserHistory.last.details.should include "new trust level: #{new_trust_level}" end end @@ -70,7 +70,7 @@ describe StaffActionLogger do end it "creates a new StaffActionLog record" do - expect { logger.log_site_setting_change('title', 'Discourse', 'My Site') }.to change { StaffActionLog.count }.by(1) + expect { logger.log_site_setting_change('title', 'Discourse', 'My Site') }.to change { UserHistory.count }.by(1) end end