FEATURE: Allow plugins to log staff actions

app/assets/javascripts/admin/controllers/admin-logs-staff-action-logs.js.es6

@@ -1,70 +1,97 @@
-/**
-  This controller supports the interface for listing staff action logs in the admin section.
-
-  @class AdminLogsStaffActionLogsController
-  @extends Ember.ArrayController
-  @namespace Discourse
-  @module Discourse
-**/
 import { outputExportResult } from 'discourse/lib/export-result';
 
 export default Ember.ArrayController.extend(Discourse.Presence, {
   loading: false,
-  filters: {},
+  filters: null,
 
-  show: function() {
+  filtersExists: Ember.computed.gt('filterCount', 0),
-    var self = this;
-    this.set('loading', true);
-    Discourse.URL.set('queryParams', this.get('filters')); // TODO: doesn't work
-    Discourse.StaffActionLog.findAll(this.get('filters')).then(function(result) {
-      self.set('model', result);
-      self.set('loading', false);
-    });
-  }.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.acting_user', 'filters.target_user', 'filters.subject'),
 
   actionFilter: function() {
-    if (this.get('filters.action_name')) {
+    var name = this.get('filters.action_name');
-      return I18n.t("admin.logs.staff_actions.actions." + this.get('filters.action_name'));
+    if (name) {
+      return I18n.t("admin.logs.staff_actions.actions." + name);
     } else {
       return null;
     }
   }.property('filters.action_name'),
 
-  showInstructions: function() {
+  showInstructions: Ember.computed.gt('model.length', 0),
-    return this.get('model.length') > 0;
+
-  }.property('loading', 'model.length'),
+  refresh: function() {
+    var self = this;
+    this.set('loading', true);
+
+    var filters = this.get('filters'),
+        params = {},
+        count = 0;
+
+    // Don't send null values
+    Object.keys(filters).forEach(function(k) {
+      var val = filters.get(k);
+      if (val) {
+        params[k] = val;
+        count += 1;
+      }
+    });
+    this.set('filterCount', count);
+
+    Discourse.StaffActionLog.findAll(params).then(function(result) {
+      self.set('model', result);
+    }).finally(function() {
+      self.set('loading', false);
+    });
+  },
+
+  resetFilters: function() {
+    this.set('filters', Ember.Object.create());
+    this.refresh();
+  }.on('init'),
+
+  _changeFilters: function(props) {
+    this.get('filters').setProperties(props);
+    this.refresh();
+  },
 
   actions: {
     clearFilter: function(key) {
-      delete this.get('filters')[key];
+      var changed = {};
-      this.notifyPropertyChange('filters');
+
+      // Special case, clear all action related stuff
+      if (key === 'actionFilter') {
+        changed.action_name = null;
+        changed.action_id = null;
+        changed.custom_type = null;
+      } else {
+        changed[key] = null;
+      }
+      this._changeFilters(changed);
     },
 
     clearAllFilters: function() {
-      this.set('filters', {});
+      this.resetFilters();
     },
 
-    filterByAction: function(action) {
+    filterByAction: function(logItem) {
-      this.set('filters.action_name', action);
+      this._changeFilters({
+        action_name: logItem.get('action_name'),
+        action_id: logItem.get('action'),
+        custom_type: logItem.get('custom_type')
+      });
     },
 
     filterByStaffUser: function(acting_user) {
-      this.set('filters.acting_user', acting_user.username);
+      this._changeFilters({ acting_user: acting_user.username });
     },
 
     filterByTargetUser: function(target_user) {
-      this.set('filters.target_user', target_user.username);
+      this._changeFilters({ target_user: target_user.username });
     },
 
     filterBySubject: function(subject) {
-      this.set('filters.subject', subject);
+      this._changeFilters({ subject: subject });
     },
 
-    exportStaffActionLogs: function(subject) {
+    exportStaffActionLogs: function() {
       Discourse.ExportCsv.exportStaffActionLogs().then(outputExportResult);
     }
   }

app/assets/javascripts/admin/models/staff_action_log.js

@@ -1,11 +1,3 @@
-/**
-  Represents an action taken by a staff member that has been logged.
-
-  @class StaffActionLog
-  @extends Discourse.Model
-  @namespace Discourse
-  @module Discourse
-**/
 Discourse.StaffActionLog = Discourse.Model.extend({
   showFullDetails: false,
 

app/assets/javascripts/admin/routes/admin-logs-staff-action-logs.js.es6

@@ -0,0 +1,23 @@
+export default Discourse.Route.extend({
+  // TODO: make this automatic using an `{{outlet}}`
+  renderTemplate: function() {
+    this.render('admin/templates/logs/staff_action_logs', {into: 'adminLogs'});
+  },
+
+  setupController: function(controller) {
+    controller.resetFilters();
+    controller.refresh();
+  },
+
+  actions: {
+    showDetailsModal: function(logRecord) {
+      Discourse.Route.showModal(this, 'admin_staff_action_log_details', logRecord);
+      this.controllerFor('modal').set('modalClass', 'log-details-modal');
+    },
+
+    showCustomDetailsModal: function(logRecord) {
+      Discourse.Route.showModal(this, logRecord.action_name + '_details', logRecord);
+      this.controllerFor('modal').set('modalClass', 'tabbed-modal log-details-modal');
+    }
+  }
+});

app/assets/javascripts/admin/routes/admin_logs_routes.js

@@ -12,47 +12,6 @@ Discourse.AdminLogsIndexRoute = Discourse.Route.extend({
   }
 });
 
-/**
-  The route that lists staff actions that were logged.
-
-  @class AdminLogsStaffActionLogsRoute
-  @extends Discourse.Route
-  @namespace Discourse
-  @module Discourse
-**/
-Discourse.AdminLogsStaffActionLogsRoute = Discourse.Route.extend({
-  renderTemplate: function() {
-    this.render('admin/templates/logs/staff_action_logs', {into: 'adminLogs'});
-  },
-
-  setupController: function(controller) {
-    var queryParams = Discourse.URL.get('queryParams');
-    if (queryParams) {
-      controller.set('filters', queryParams);
-    }
-    return controller.show();
-  },
-
-  actions: {
-    showDetailsModal: function(logRecord) {
-      Discourse.Route.showModal(this, 'admin_staff_action_log_details', logRecord);
-      this.controllerFor('modal').set('modalClass', 'log-details-modal');
-    },
-
-    showCustomDetailsModal: function(logRecord) {
-      Discourse.Route.showModal(this, logRecord.action_name + '_details', logRecord);
-      this.controllerFor('modal').set('modalClass', 'tabbed-modal log-details-modal');
-    }
-  },
-
-  deactivate: function() {
-    this._super();
-
-    // Clear any filters when we leave the route
-    Discourse.URL.set('queryParams', null);
-  }
-});
-
 /**
   The route that lists blocked email addresses.
 

app/assets/javascripts/admin/templates/logs/staff_action_logs.hbs

@@ -3,33 +3,33 @@
     <span class="label">{{i18n 'admin.logs.staff_actions.clear_filters'}}</span>
   </a>
   {{#if actionFilter}}
-    <a {{action "clearFilter" "action_name"}} class="filter">
+    <a {{action "clearFilter" "actionFilter"}} class="filter">
       <span class="label">{{i18n 'admin.logs.action'}}</span>: {{actionFilter}}
-      <i class="fa fa-times-circle"></i>
+      {{fa-icon "times-circle"}}
     </a>
   {{/if}}
   {{#if filters.acting_user}}
     <a {{action "clearFilter" "acting_user"}} class="filter">
       <span class="label">{{i18n 'admin.logs.staff_actions.staff_user'}}</span>: {{filters.acting_user}}
-      <i class="fa fa-times-circle"></i>
+      {{fa-icon "times-circle"}}
     </a>
   {{/if}}
   {{#if filters.target_user}}
     <a {{action "clearFilter" "target_user"}} class="filter">
       <span class="label">{{i18n 'admin.logs.staff_actions.target_user'}}</span>: {{filters.target_user}}
-      <i class="fa fa-times-circle"></i>
+      {{fa-icon "times-circle"}}
     </a>
   {{/if}}
   {{#if filters.subject}}
     <a {{action "clearFilter" "subject"}} class="filter">
       <span class="label">{{i18n 'admin.logs.staff_actions.subject'}}</span>: {{filters.subject}}
-      <i class="fa fa-times-circle"></i>
+      {{fa-icon "times-circle"}}
     </a>
   {{/if}}
 </div>
 
 <div class="pull-right">
-  <button class="btn" {{action "exportStaffActionLogs"}} title="{{i18n 'admin.export_csv.button_title.staff_action'}}">{{fa-icon "download"}}{{i18n 'admin.export_csv.button_text'}}</button>
+  {{d-button action="exportStaffActionLogs" label="admin.export_csv.button_text" icon="download"}}
 </div>
 </br>
 

app/assets/javascripts/admin/templates/logs/staff_action_logs_list_item.hbs

@@ -3,7 +3,7 @@
   <a {{action "filterByStaffUser" acting_user}} class="btn btn-small">{{acting_user.username}}</a>
 </div>
 <div class="col value action">
-  <a {{action "filterByAction" action_name}} class="btn btn-small">{{actionName}}</a>
+  <a {{action "filterByAction" this}} class="btn btn-small">{{actionName}}</a>
 </div>
 <div class="col value subject">
   {{#if target_user}}

app/controllers/admin/staff_action_logs_controller.rb

@@ -1,7 +1,8 @@
 class Admin::StaffActionLogsController < Admin::AdminController
 
   def index
-    staff_action_logs = UserHistory.staff_action_records(current_user, params.slice(:action_name, :acting_user, :target_user, :subject)).to_a
+    filters = params.slice(*UserHistory.staff_filters)
+    staff_action_logs = UserHistory.staff_action_records(current_user, filters).to_a
     render_serialized(staff_action_logs, UserHistorySerializer)
   end
 

app/models/user_history.rb

@@ -35,7 +35,9 @@ class UserHistory < ActiveRecord::Base
                           :delete_topic,
                           :impersonate,
                           :roll_up,
-                          :change_username)
+                          :change_username,
+                          :custom,
+                          :custom_staff)
   end
 
   # Staff actions is a subset of all actions, used to audit actions taken by staff users.
@@ -54,7 +56,8 @@ class UserHistory < ActiveRecord::Base
                         :delete_topic,
                         :impersonate,
                         :roll_up,
-                        :change_username]
+                        :change_username,
+                        :custom_staff]
   end
 
   def self.staff_action_ids
@@ -67,9 +70,9 @@ class UserHistory < ActiveRecord::Base
 
   def self.with_filters(filters)
     query = self
-    if filters[:action_name] and action_id = UserHistory.actions[filters[:action_name].to_sym]
+    query = query.where(action: filters[:action_id]) if filters[:action_id].present?
-      query = query.where('action = ?', action_id)
+    query = query.where(custom_type: filters[:custom_type]) if filters[:custom_type].present?
-    end
+
     [: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}_id = ?", obj_id)
@@ -90,8 +93,13 @@ class UserHistory < ActiveRecord::Base
     result.exists?
   end
 
-  def self.staff_action_records(viewer, opts={})
+  def self.staff_filters
-    query = self.with_filters(opts.slice(:action_name, :acting_user, :target_user, :subject)).only_staff_actions.limit(200).order('id DESC').includes(:acting_user, :target_user)
+    [:action_id, :custom_type, :acting_user, :target_user, :subject]
+  end
+
+  def self.staff_action_records(viewer, opts=nil)
+    opts ||= {}
+    query = self.with_filters(opts.slice(*staff_filters)).only_staff_actions.limit(200).order('id DESC').includes(:acting_user, :target_user)
     query = query.where(admin_only: false) unless viewer && viewer.admin?
     query
   end

app/serializers/user_history_serializer.rb

@@ -9,13 +9,16 @@ class UserHistorySerializer < ApplicationSerializer
              :previous_value,
              :new_value,
              :topic_id,
-             :post_id
+             :post_id,
+             :action,
+             :custom_type
 
   has_one :acting_user, serializer: BasicUserSerializer, embed: :objects
   has_one :target_user, serializer: BasicUserSerializer, embed: :objects
 
   def action_name
-    UserHistory.actions.key(object.action).to_s
+    key = UserHistory.actions.key(object.action)
+    [:custom, :custom_staff].include?(key) ? object.custom_type : key.to_s
   end
 
   def new_value

app/services/staff_action_logger.rb

@@ -1,13 +1,17 @@
 # Responsible for logging the actions of admins and moderators.
 class StaffActionLogger
 
+  def self.base_attrs
+    [:topic_id, :post_id, :context, :subject, :ip_address, :previous_value, :new_value]
+  end
+
   def initialize(admin)
     @admin = admin
-    raise Discourse::InvalidParameters.new('admin is nil') unless @admin && @admin.is_a?(User)
+    raise Discourse::InvalidParameters.new(:admin) unless @admin && @admin.is_a?(User)
   end
 
   def log_user_deletion(deleted_user, opts={})
-    raise Discourse::InvalidParameters.new('user is nil') unless deleted_user && deleted_user.is_a?(User)
+    raise Discourse::InvalidParameters.new(:deleted_user) unless deleted_user && deleted_user.is_a?(User)
     UserHistory.create( params(opts).merge({
       action: UserHistory.actions[:delete_user],
       email: deleted_user.email,
@@ -16,8 +20,25 @@ class StaffActionLogger
     }))
   end
 
+  def log_custom(custom_type, details=nil)
+    raise Discourse::InvalidParameters.new(:custom_type) unless custom_type
+
+    details ||= {}
+
+    attrs = {}
+    StaffActionLogger.base_attrs.each do |attr|
+      attrs[attr] = details.delete(attr) if details.has_key?(attr)
+    end
+    attrs[:details] = details.map {|r| "#{r[0]}: #{r[1]}"}.join("\n")
+    attrs[:acting_user_id] = @admin.id
+    attrs[:action] = UserHistory.actions[:custom_staff]
+    attrs[:custom_type] = custom_type
+
+    UserHistory.create(attrs)
+  end
+
   def log_post_deletion(deleted_post, opts={})
-    raise Discourse::InvalidParameters.new("post is nil") unless deleted_post && deleted_post.is_a?(Post)
+    raise Discourse::InvalidParameters.new(:deleted_post) unless deleted_post && deleted_post.is_a?(Post)
 
     topic = deleted_post.topic || Topic.with_deleted.find(deleted_post.topic_id)
 
@@ -38,7 +59,7 @@ class StaffActionLogger
   end
 
   def log_topic_deletion(deleted_topic, opts={})
-    raise Discourse::InvalidParameters.new("topic is nil") unless deleted_topic && deleted_topic.is_a?(Topic)
+    raise Discourse::InvalidParameters.new(:deleted_topic) unless deleted_topic && deleted_topic.is_a?(Topic)
 
     details = [
       "id: #{deleted_topic.id}",
@@ -59,9 +80,9 @@ class StaffActionLogger
   end
 
   def log_trust_level_change(user, old_trust_level, new_trust_level, opts={})
-    raise Discourse::InvalidParameters.new('user is nil') unless user && user.is_a?(User)
+    raise Discourse::InvalidParameters.new(:user) unless user && user.is_a?(User)
-    raise Discourse::InvalidParameters.new('old trust level is invalid') unless TrustLevel.valid? old_trust_level
+    raise Discourse::InvalidParameters.new(:old_trust_level) unless TrustLevel.valid? old_trust_level
-    raise Discourse::InvalidParameters.new('new trust level is invalid') unless TrustLevel.valid? new_trust_level
+    raise Discourse::InvalidParameters.new(:new_trust_level) unless TrustLevel.valid? new_trust_level
     UserHistory.create!( params(opts).merge({
       action: UserHistory.actions[:change_trust_level],
       target_user_id: user.id,
@@ -70,7 +91,7 @@ class StaffActionLogger
   end
 
   def log_site_setting_change(setting_name, previous_value, new_value, opts={})
-    raise Discourse::InvalidParameters.new('setting_name is invalid') unless setting_name.present? && SiteSetting.respond_to?(setting_name)
+    raise Discourse::InvalidParameters.new(:setting_name) unless setting_name.present? && SiteSetting.respond_to?(setting_name)
     UserHistory.create( params(opts).merge({
       action: UserHistory.actions[:change_site_setting],
       subject: setting_name,
@@ -82,7 +103,7 @@ class StaffActionLogger
   SITE_CUSTOMIZATION_LOGGED_ATTRS = ['stylesheet', 'header', 'position', 'enabled', 'key']
 
   def log_site_customization_change(old_record, site_customization_params, opts={})
-    raise Discourse::InvalidParameters.new('site_customization_params is nil') unless site_customization_params
+    raise Discourse::InvalidParameters.new(:site_customization_params) unless site_customization_params
     UserHistory.create( params(opts).merge({
       action: UserHistory.actions[:change_site_customization],
       subject: site_customization_params[:name],
@@ -92,7 +113,7 @@ class StaffActionLogger
   end
 
   def log_site_customization_destroy(site_customization, opts={})
-    raise Discourse::InvalidParameters.new('site_customization is nil') unless site_customization
+    raise Discourse::InvalidParameters.new(:site_customization) unless site_customization
     UserHistory.create( params(opts).merge({
       action: UserHistory.actions[:delete_site_customization],
       subject: site_customization.name,
@@ -101,7 +122,7 @@ class StaffActionLogger
   end
 
   def log_username_change(user, old_username, new_username, opts={})
-    raise Discourse::InvalidParameters.new('user is nil') unless user
+    raise Discourse::InvalidParameters.new(:user) unless user
     UserHistory.create( params(opts).merge({
       action: UserHistory.actions[:change_username],
       target_user_id: user.id,
@@ -111,7 +132,7 @@ class StaffActionLogger
   end
 
   def log_user_suspend(user, reason, opts={})
-    raise Discourse::InvalidParameters.new('user is nil') unless user
+    raise Discourse::InvalidParameters.new(:user) unless user
     UserHistory.create( params(opts).merge({
       action: UserHistory.actions[:suspend_user],
       target_user_id: user.id,
@@ -120,7 +141,7 @@ class StaffActionLogger
   end
 
   def log_user_unsuspend(user, opts={})
-    raise Discourse::InvalidParameters.new('user is nil') unless user
+    raise Discourse::InvalidParameters.new(:user) unless user
     UserHistory.create( params(opts).merge({
       action: UserHistory.actions[:unsuspend_user],
       target_user_id: user.id
@@ -128,7 +149,7 @@ class StaffActionLogger
   end
 
   def log_badge_grant(user_badge, opts={})
-    raise Discourse::InvalidParameters.new('user_badge is nil') unless user_badge
+    raise Discourse::InvalidParameters.new(:user_badge) unless user_badge
     UserHistory.create( params(opts).merge({
       action: UserHistory.actions[:grant_badge],
       target_user_id: user_badge.user_id,
@@ -137,7 +158,7 @@ class StaffActionLogger
   end
 
   def log_badge_revoke(user_badge, opts={})
-    raise Discourse::InvalidParameters.new('user_badge is nil') unless user_badge
+    raise Discourse::InvalidParameters.new(:user_badge) unless user_badge
     UserHistory.create( params(opts).merge({
       action: UserHistory.actions[:revoke_badge],
       target_user_id: user_badge.user_id,
@@ -146,7 +167,7 @@ class StaffActionLogger
   end
 
   def log_check_email(user, opts={})
-    raise Discourse::InvalidParameters.new('user is nil') unless user
+    raise Discourse::InvalidParameters.new(:user) unless user
     UserHistory.create(params(opts).merge({
       action: UserHistory.actions[:check_email],
       target_user_id: user.id
@@ -162,7 +183,7 @@ class StaffActionLogger
   end
 
   def log_impersonate(user, opts={})
-    raise Discourse::InvalidParameters.new("user is nil") unless user
+    raise Discourse::InvalidParameters.new(:user) unless user
     UserHistory.create(params(opts).merge({
       action: UserHistory.actions[:impersonate],
       target_user_id: user.id
@@ -178,7 +199,8 @@ class StaffActionLogger
 
   private
 
-    def params(opts)
+    def params(opts=nil)
+      opts ||= {}
       { acting_user_id: @admin.id, context: opts[:context] }
     end
 

db/migrate/20150205172051_add_custom_type_to_user_histories.rb

@@ -0,0 +1,5 @@
+class AddCustomTypeToUserHistories < ActiveRecord::Migration
+  def change
+    add_column :user_histories, :custom_type, :string
+  end
+end

spec/services/staff_action_logger_spec.rb

@@ -245,4 +245,23 @@ describe StaffActionLogger do
       log_record.details.should == subnets.join(", ")
     end
   end
+
+  describe 'log_custom' do
+    it "raises an error when `custom_type` is missing" do
+      expect { logger.log_custom(nil) }.to raise_error(Discourse::InvalidParameters)
+    end
+
+    it "creates the UserHistory record" do
+      logged = logger.log_custom('clicked_something', {
+        evil: 'trout',
+        clicked_on: 'thing',
+        topic_id: 1234
+      })
+      logged.should be_valid
+      logged.details.should == "evil: trout\nclicked_on: thing"
+      logged.action.should == UserHistory.actions[:custom_staff]
+      logged.custom_type.should == 'clicked_something'
+      logged.topic_id.should === 1234
+    end
+  end
 end