diff --git a/app/assets/javascripts/admin/controllers/admin-logs-staff-action-logs.js.es6 b/app/assets/javascripts/admin/controllers/admin-logs-staff-action-logs.js.es6 index c4ecd3d14..6784fa1d8 100644 --- a/app/assets/javascripts/admin/controllers/admin-logs-staff-action-logs.js.es6 +++ b/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() { - 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'), + filtersExists: Ember.computed.gt('filterCount', 0), actionFilter: function() { - if (this.get('filters.action_name')) { - return I18n.t("admin.logs.staff_actions.actions." + this.get('filters.action_name')); + var name = 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() { - return this.get('model.length') > 0; - }.property('loading', 'model.length'), + showInstructions: Ember.computed.gt('model.length', 0), + + 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]; - this.notifyPropertyChange('filters'); + var changed = {}; + + // 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) { - this.set('filters.action_name', action); + filterByAction: function(logItem) { + 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); } } diff --git a/app/assets/javascripts/admin/models/staff_action_log.js b/app/assets/javascripts/admin/models/staff_action_log.js index 75c1e3174..dc52a7b17 100644 --- a/app/assets/javascripts/admin/models/staff_action_log.js +++ b/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, diff --git a/app/assets/javascripts/admin/routes/admin-logs-staff-action-logs.js.es6 b/app/assets/javascripts/admin/routes/admin-logs-staff-action-logs.js.es6 new file mode 100644 index 000000000..bcc6a5fc1 --- /dev/null +++ b/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'); + } + } +}); diff --git a/app/assets/javascripts/admin/routes/admin_logs_routes.js b/app/assets/javascripts/admin/routes/admin_logs_routes.js index 2a7dc7230..ad54a895d 100644 --- a/app/assets/javascripts/admin/routes/admin_logs_routes.js +++ b/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. diff --git a/app/assets/javascripts/admin/templates/logs/staff_action_logs.hbs b/app/assets/javascripts/admin/templates/logs/staff_action_logs.hbs index 10ae17505..58a5ba72a 100644 --- a/app/assets/javascripts/admin/templates/logs/staff_action_logs.hbs +++ b/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> diff --git a/app/assets/javascripts/admin/templates/logs/staff_action_logs_list_item.hbs b/app/assets/javascripts/admin/templates/logs/staff_action_logs_list_item.hbs index cbcea891a..07a9b1bf8 100644 --- a/app/assets/javascripts/admin/templates/logs/staff_action_logs_list_item.hbs +++ b/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}} diff --git a/app/controllers/admin/staff_action_logs_controller.rb b/app/controllers/admin/staff_action_logs_controller.rb index e60168a52..5324aabc0 100644 --- a/app/controllers/admin/staff_action_logs_controller.rb +++ b/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 diff --git a/app/models/user_history.rb b/app/models/user_history.rb index 37722e4e5..ffe75d248 100644 --- a/app/models/user_history.rb +++ b/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 = ?', action_id) - end + query = query.where(action: filters[:action_id]) if filters[:action_id].present? + query = query.where(custom_type: filters[:custom_type]) if filters[:custom_type].present? + [: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={}) - 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) + def self.staff_filters + [: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 diff --git a/app/serializers/user_history_serializer.rb b/app/serializers/user_history_serializer.rb index b4e29e294..2af3c7dc9 100644 --- a/app/serializers/user_history_serializer.rb +++ b/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 diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb index 620c1138f..10b13092a 100644 --- a/app/services/staff_action_logger.rb +++ b/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('old trust level is invalid') 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(:user) unless user && user.is_a?(User) + raise Discourse::InvalidParameters.new(:old_trust_level) unless TrustLevel.valid? old_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 diff --git a/db/migrate/20150205172051_add_custom_type_to_user_histories.rb b/db/migrate/20150205172051_add_custom_type_to_user_histories.rb new file mode 100644 index 000000000..1706a4fa9 --- /dev/null +++ b/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 diff --git a/spec/services/staff_action_logger_spec.rb b/spec/services/staff_action_logger_spec.rb index 83bbae7e4..d5fcb126d 100644 --- a/spec/services/staff_action_logger_spec.rb +++ b/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