+
{{#if target_user}}
{{#linkTo 'adminUser' target_user}}{{avatar target_user imageSize="tiny"}}{{/linkTo}}
{{target_user.username}}
- {{else}}
- —
+ {{/if}}
+ {{#if subject}}
+
{{subject}}
{{/if}}
{{unboundAgeWithTooltip created_at}}
-
{{context}}
{{{formattedDetails}}}
{{#if showFullDetails}}
@@ -24,4 +24,5 @@
{{i18n more}}
{{/if}}
+
{{context}}
diff --git a/app/assets/stylesheets/admin/admin_base.scss b/app/assets/stylesheets/admin/admin_base.scss
index 41e6738de1a..6ff9b8acef4 100644
--- a/app/assets/stylesheets/admin/admin_base.scss
+++ b/app/assets/stylesheets/admin/admin_base.scss
@@ -716,9 +716,12 @@ table {
.action {
width: 120px;
}
- .staff_user, .target_user {
+ .staff_user {
width: 100px;
}
+ .subject {
+ width: 200px;
+ }
.created_at {
width: 50px;
}
@@ -729,7 +732,7 @@ table {
text-align: center;
}
.details {
- width: 400px;
+ width: 300px;
a {
text-decoration: underline;
}
diff --git a/app/controllers/admin/site_settings_controller.rb b/app/controllers/admin/site_settings_controller.rb
index 2dcaab37cd2..e05d1425b24 100644
--- a/app/controllers/admin/site_settings_controller.rb
+++ b/app/controllers/admin/site_settings_controller.rb
@@ -8,6 +8,7 @@ class Admin::SiteSettingsController < Admin::AdminController
def update
raise ActionController::ParameterMissing.new(:value) unless params.has_key?(:value)
+ StaffActionLogger.new(current_user).log_site_setting_change(params[:id], SiteSetting.send("#{params[:id]}"), params[:value]) if SiteSetting.respond_to?(params[:id])
SiteSetting.send("#{params[:id]}=", params[:value])
render nothing: true
end
diff --git a/app/models/staff_action_log.rb b/app/models/staff_action_log.rb
index b3312a1bda5..385184fe851 100644
--- a/app/models/staff_action_log.rb
+++ b/app/models/staff_action_log.rb
@@ -9,7 +9,7 @@ class StaffActionLog < ActiveRecord::Base
validates_presence_of :action
def self.actions
- @actions ||= Enum.new(:delete_user, :change_trust_level)
+ @actions ||= Enum.new(:delete_user, :change_trust_level, :change_site_setting)
end
def self.with_filters(filters)
diff --git a/app/serializers/staff_action_log_serializer.rb b/app/serializers/staff_action_log_serializer.rb
index 0f0db26725d..35ec8e79a27 100644
--- a/app/serializers/staff_action_log_serializer.rb
+++ b/app/serializers/staff_action_log_serializer.rb
@@ -4,7 +4,10 @@ class StaffActionLogSerializer < ApplicationSerializer
:context,
:ip_address,
:email,
- :created_at
+ :created_at,
+ :subject,
+ :previous_value,
+ :new_value
has_one :staff_user, serializer: BasicUserSerializer, embed: :objects
has_one :target_user, serializer: BasicUserSerializer, embed: :objects
diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb
index 4c3cc888ae4..faf9f38436f 100644
--- a/app/services/staff_action_logger.rb
+++ b/app/services/staff_action_logger.rb
@@ -7,26 +7,39 @@ 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(
+ StaffActionLog.create( params(opts).merge({
action: StaffActionLog.actions[:delete_user],
- context: opts[:context], # should be the url from where the staff member deleted the user
- staff_user_id: @admin.id,
target_user_id: deleted_user.id,
email: deleted_user.email,
ip_address: deleted_user.ip_address,
details: [:id, :username, :name, :created_at, :trust_level, :last_seen_at, :last_emailed_at].map { |x| "#{x}: #{deleted_user.send(x)}" }.join(', ')
- )
+ }))
end
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!(
+ StaffActionLog.create!( params(opts).merge({
action: StaffActionLog.actions[:change_trust_level],
- staff_user_id: @admin.id,
target_user_id: user.id,
details: "old trust level: #{old_trust_level}, new trust level: #{new_trust_level}"
- )
+ }))
+ 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? and SiteSetting.respond_to?(setting_name)
+ StaffActionLog.create( params(opts).merge({
+ action: StaffActionLog.actions[:change_site_setting],
+ subject: setting_name,
+ previous_value: previous_value,
+ new_value: new_value
+ }))
+ end
+
+ private
+
+ def params(opts)
+ {staff_user_id: @admin.id, context: opts[:context]}
end
end
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index ccf8f58ea6b..07fdaf03c24 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -1192,12 +1192,17 @@ en:
clear_filters: "Show Everything"
staff_user: "Staff User"
target_user: "Target User"
+ subject: "Subject"
when: "When"
context: "Context"
details: "Details"
+ ip_address: "IP"
+ previous_value: "Previous"
+ new_value: "New"
actions:
delete_user: "delete user"
change_trust_level: "change trust level"
+ change_site_setting: "change site setting"
screened_emails:
title: "Screened Emails"
description: "When someone tries to create a new account, the following email addresses will be checked and the registration will be blocked, or some other action performed."
diff --git a/db/migrate/20130819192358_add_value_columns_to_staff_action_logs.rb b/db/migrate/20130819192358_add_value_columns_to_staff_action_logs.rb
new file mode 100644
index 00000000000..5ea550ece92
--- /dev/null
+++ b/db/migrate/20130819192358_add_value_columns_to_staff_action_logs.rb
@@ -0,0 +1,7 @@
+class AddValueColumnsToStaffActionLogs < ActiveRecord::Migration
+ def change
+ add_column :staff_action_logs, :subject, :text
+ add_column :staff_action_logs, :previous_value, :text
+ add_column :staff_action_logs, :new_value, :text
+ end
+end
diff --git a/spec/controllers/admin/site_settings_controller_spec.rb b/spec/controllers/admin/site_settings_controller_spec.rb
index 2f72b9b4e2f..003fb2453af 100644
--- a/spec/controllers/admin/site_settings_controller_spec.rb
+++ b/spec/controllers/admin/site_settings_controller_spec.rb
@@ -38,6 +38,13 @@ describe Admin::SiteSettingsController do
SiteSetting.expects(:'test_setting=').with('').once
xhr :put, :update, id: 'test_setting', value: ''
end
+
+ it 'logs the change' do
+ SiteSetting.stubs(:test_setting).returns('previous')
+ SiteSetting.expects(:'test_setting=').with('hello').once
+ StaffActionLogger.any_instance.expects(:log_site_setting_change).with('test_setting', 'previous', 'hello')
+ xhr :put, :update, id: 'test_setting', value: 'hello'
+ end
end
end
diff --git a/spec/services/staff_action_logger_spec.rb b/spec/services/staff_action_logger_spec.rb
index 62d24058b59..d1af03c4a18 100644
--- a/spec/services/staff_action_logger_spec.rb
+++ b/spec/services/staff_action_logger_spec.rb
@@ -2,6 +2,9 @@ require 'spec_helper'
describe StaffActionLogger do
+ let(:admin) { Fabricate(:admin) }
+ let(:logger) { described_class.new(admin) }
+
describe 'new' do
it 'raises an error when user is nil' do
expect { described_class.new(nil) }.to raise_error(Discourse::InvalidParameters)
@@ -13,17 +16,16 @@ describe StaffActionLogger do
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)
+ expect { logger.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)
+ expect { logger.log_user_deletion(1) }.to raise_error(Discourse::InvalidParameters)
end
it 'creates a new StaffActionLog record' do
@@ -33,7 +35,6 @@ describe StaffActionLogger do
end
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] }
@@ -41,18 +42,18 @@ describe StaffActionLogger do
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, old_trust_level, new_trust_level) }.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)
+ expect { logger.log_trust_level_change(nil, old_trust_level, new_trust_level) }.to raise_error(Discourse::InvalidParameters)
+ expect { logger.log_trust_level_change(user, nil, new_trust_level) }.to raise_error(Discourse::InvalidParameters)
+ expect { logger.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, old_trust_level, new_trust_level) }.to raise_error(Discourse::InvalidParameters)
+ expect { logger.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, old_trust_level, max_level + 1) }.to raise_error(Discourse::InvalidParameters)
+ expect { logger.log_trust_level_change(user, old_trust_level, max_level + 1) }.to raise_error(Discourse::InvalidParameters)
end
it 'creates a new StaffActionLog record' do
@@ -60,4 +61,16 @@ describe StaffActionLogger do
StaffActionLog.last.details.should include "new trust level: #{new_trust_level}"
end
end
+
+ describe "log_site_setting_change" do
+ it "raises an error when params are invalid" do
+ SiteSetting.stubs(:respond_to?).with('abc').returns(false)
+ expect { logger.log_site_setting_change(nil, '1', '2') }.to raise_error(Discourse::InvalidParameters)
+ expect { logger.log_site_setting_change('abc', '1', '2') }.to raise_error(Discourse::InvalidParameters)
+ end
+
+ it "creates a new StaffActionLog record" do
+ expect { logger.log_site_setting_change('title', 'Discourse', 'My Site') }.to change { StaffActionLog.count }.by(1)
+ end
+ end
end