diff --git a/app/assets/javascripts/admin/models/staff_action_log.js b/app/assets/javascripts/admin/models/staff_action_log.js
index dc52a7b17..0dde77366 100644
--- a/app/assets/javascripts/admin/models/staff_action_log.js
+++ b/app/assets/javascripts/admin/models/staff_action_log.js
@@ -11,6 +11,7 @@ Discourse.StaffActionLog = Discourse.Model.extend({
formatted += this.format('admin.logs.ip_address', 'ip_address');
formatted += this.format('admin.logs.topic_id', 'topic_id');
formatted += this.format('admin.logs.post_id', 'post_id');
+ formatted += this.format('admin.logs.category_id', 'category_id');
if (!this.get('useCustomModalForDetails')) {
formatted += this.format('admin.logs.staff_actions.new_value', 'new_value');
formatted += this.format('admin.logs.staff_actions.previous_value', 'previous_value');
@@ -19,7 +20,7 @@ Discourse.StaffActionLog = Discourse.Model.extend({
if (this.get('details')) formatted += Handlebars.Utils.escapeExpression(this.get('details')) + '<br/>';
}
return formatted;
- }.property('ip_address', 'email', 'topic_id', 'post_id'),
+ }.property('ip_address', 'email', 'topic_id', 'post_id', 'category_id'),
format: function(label, propertyName) {
if (this.get(propertyName)) {
diff --git a/app/assets/javascripts/discourse/controllers/edit-category.js.es6 b/app/assets/javascripts/discourse/controllers/edit-category.js.es6
index bfda086b6..d77783bc0 100644
--- a/app/assets/javascripts/discourse/controllers/edit-category.js.es6
+++ b/app/assets/javascripts/discourse/controllers/edit-category.js.es6
@@ -1,5 +1,6 @@
import ModalFunctionality from 'discourse/mixins/modal-functionality';
import DiscourseURL from 'discourse/lib/url';
+import { extractError } from 'discourse/lib/ajax-error';
// Modal for editing / creating a category
export default Ember.Controller.extend(ModalFunctionality, {
@@ -67,17 +68,13 @@ export default Ember.Controller.extend(ModalFunctionality, {
this.set('saving', true);
model.set('parentCategory', parentCategory);
- self.set('saving', false);
this.get('model').save().then(function(result) {
+ self.set('saving', false);
self.send('closeModal');
model.setProperties({slug: result.category.slug, id: result.category.id });
DiscourseURL.redirectTo("/c/" + Discourse.Category.slugFor(model));
}).catch(function(error) {
- if (error && error.responseText) {
- self.flash($.parseJSON(error.responseText).errors[0], 'error');
- } else {
- self.flash(I18n.t('generic_error'), 'error');
- }
+ self.flash(extractError(error), 'error');
self.set('saving', false);
});
},
@@ -94,13 +91,7 @@ export default Ember.Controller.extend(ModalFunctionality, {
self.send('closeModal');
DiscourseURL.redirectTo("/categories");
}, function(error){
-
- if (error && error.responseText) {
- self.flash($.parseJSON(error.responseText).errors[0]);
- } else {
- self.flash(I18n.t('generic_error'));
- }
-
+ self.flash(extractError(error), 'error');
self.send('reopenModal');
self.displayErrors([I18n.t("category.delete_error")]);
self.set('deleting', false);
diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb
index b63ec503d..52437468d 100644
--- a/app/controllers/categories_controller.rb
+++ b/app/controllers/categories_controller.rb
@@ -4,6 +4,7 @@ class CategoriesController < ApplicationController
before_filter :ensure_logged_in, except: [:index, :show, :redirect]
before_filter :fetch_category, only: [:show, :update, :destroy]
+ before_filter :initialize_staff_action_logger, only: [:create, :update, :destroy]
skip_before_filter :check_xhr, only: [:index, :redirect]
def redirect
@@ -81,10 +82,18 @@ class CategoriesController < ApplicationController
position = category_params.delete(:position)
@category = Category.create(category_params.merge(user: current_user))
- return render_json_error(@category) unless @category.save
- @category.move_to(position.to_i) if position
- render_serialized(@category, CategorySerializer)
+ if @category.save
+ @category.move_to(position.to_i) if position
+
+ Scheduler::Defer.later "Log staff action create category" do
+ @staff_action_logger.log_category_creation(@category)
+ end
+
+ render_serialized(@category, CategorySerializer)
+ else
+ return render_json_error(@category) unless @category.save
+ end
end
def update
@@ -103,8 +112,15 @@ class CategoriesController < ApplicationController
end
category_params.delete(:position)
+ old_permissions = Category.find(@category.id).permissions_params
- cat.update_attributes(category_params)
+ if result = cat.update_attributes(category_params)
+ Scheduler::Defer.later "Log staff action change category settings" do
+ @staff_action_logger.log_category_settings_change(@category, category_params, old_permissions)
+ end
+ end
+
+ result
end
end
@@ -133,6 +149,10 @@ class CategoriesController < ApplicationController
guardian.ensure_can_delete!(@category)
@category.destroy
+ Scheduler::Defer.later "Log staff action delete category" do
+ @staff_action_logger.log_category_deletion(@category)
+ end
+
render json: success_json
end
@@ -175,4 +195,8 @@ class CategoriesController < ApplicationController
def fetch_category
@category = Category.find_by(slug: params[:id]) || Category.find_by(id: params[:id].to_i)
end
+
+ def initialize_staff_action_logger
+ @staff_action_logger = StaffActionLogger.new(current_user)
+ end
end
diff --git a/app/models/category.rb b/app/models/category.rb
index 7e550aef2..68eed7090 100644
--- a/app/models/category.rb
+++ b/app/models/category.rb
@@ -283,6 +283,14 @@ SQL
set_permissions(permissions)
end
+ def permissions_params
+ hash = {}
+ category_groups.includes(:group).each do |category_group|
+ hash[category_group.group_name] = category_group.permission_type
+ end
+ hash
+ end
+
def apply_permissions
if @permissions
category_groups.destroy_all
diff --git a/app/models/category_group.rb b/app/models/category_group.rb
index fa9fd3a21..849fcba02 100644
--- a/app/models/category_group.rb
+++ b/app/models/category_group.rb
@@ -2,6 +2,8 @@ class CategoryGroup < ActiveRecord::Base
belongs_to :category
belongs_to :group
+ delegate :name, to: :group, prefix: true
+
def self.permission_types
@permission_types ||= Enum.new(:full, :create_post, :readonly)
end
diff --git a/app/models/user_history.rb b/app/models/user_history.rb
index 77867f7d4..038df0e9c 100644
--- a/app/models/user_history.rb
+++ b/app/models/user_history.rb
@@ -7,6 +7,7 @@ class UserHistory < ActiveRecord::Base
belongs_to :post
belongs_to :topic
+ belongs_to :category
validates_presence_of :action
@@ -39,7 +40,10 @@ class UserHistory < ActiveRecord::Base
:custom,
:custom_staff,
:anonymize_user,
- :reviewed_post)
+ :reviewed_post,
+ :change_category_settings,
+ :delete_category,
+ :create_category)
end
# Staff actions is a subset of all actions, used to audit actions taken by staff users.
@@ -61,7 +65,10 @@ class UserHistory < ActiveRecord::Base
:change_username,
:custom_staff,
:anonymize_user,
- :reviewed_post]
+ :reviewed_post,
+ :change_category_settings,
+ :delete_category,
+ :create_category]
end
def self.staff_action_ids
@@ -144,11 +151,13 @@ end
# admin_only :boolean default(FALSE)
# post_id :integer
# custom_type :string(255)
+# category_id :integer
#
# Indexes
#
# index_user_histories_on_acting_user_id_and_action_and_id (acting_user_id,action,id)
# index_user_histories_on_action_and_id (action,id)
+# index_user_histories_on_category_id (category_id)
# index_user_histories_on_subject_and_id (subject,id)
# index_user_histories_on_target_user_id_and_id (target_user_id,id)
#
diff --git a/app/serializers/user_history_serializer.rb b/app/serializers/user_history_serializer.rb
index 2af3c7dc9..039e25c61 100644
--- a/app/serializers/user_history_serializer.rb
+++ b/app/serializers/user_history_serializer.rb
@@ -10,6 +10,7 @@ class UserHistorySerializer < ApplicationSerializer
:new_value,
:topic_id,
:post_id,
+ :category_id,
:action,
:custom_type
diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb
index cd681eea9..cbb56693a 100644
--- a/app/services/staff_action_logger.rb
+++ b/app/services/staff_action_logger.rb
@@ -210,6 +210,64 @@ class StaffActionLogger
}))
end
+ def log_category_settings_change(category, category_params, old_permissions=nil)
+ validate_category(category)
+
+ changed_attributes = category.previous_changes.slice(*category_params.keys)
+
+ if old_permissions != category_params[:permissions]
+ changed_attributes.merge!({ permissions: [old_permissions.to_json, category_params[:permissions].to_json] })
+ end
+
+ changed_attributes.each do |key, value|
+ UserHistory.create(params.merge({
+ action: UserHistory.actions[:change_category_settings],
+ category_id: category.id,
+ context: category.url,
+ subject: key,
+ previous_value: value[0],
+ new_value: value[1]
+ }))
+ end
+ end
+
+ def log_category_deletion(category)
+ validate_category(category)
+
+ details = [
+ "created_at: #{category.created_at}",
+ "name: #{category.name}",
+ "permissions: #{category.permissions_params}"
+ ]
+
+ if parent_category = category.parent_category
+ details << "parent_category: #{parent_category.name}"
+ end
+
+ UserHistory.create(params.merge({
+ action: UserHistory.actions[:delete_category],
+ category_id: category.id,
+ details: details.join("\n"),
+ context: category.url
+ }))
+ end
+
+ def log_category_creation(category)
+ validate_category(category)
+
+ details = [
+ "created_at: #{category.created_at}",
+ "name: #{category.name}"
+ ]
+
+ UserHistory.create(params.merge({
+ action: UserHistory.actions[:create_category],
+ details: details.join("\n"),
+ category_id: category.id,
+ context: category.url
+ }))
+ end
+
private
def params(opts=nil)
@@ -217,4 +275,8 @@ class StaffActionLogger
{ acting_user_id: @admin.id, context: opts[:context] }
end
+ def validate_category(category)
+ raise Discourse::InvalidParameters.new(:category) unless category && category.is_a?(Category)
+ end
+
end
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 4dd0b0cf5..f5fa11808 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -2173,6 +2173,7 @@ en:
ip_address: "IP"
topic_id: "Topic ID"
post_id: "Post ID"
+ category_id: "Category ID"
delete: 'Delete'
edit: 'Edit'
save: 'Save'
@@ -2213,6 +2214,9 @@ en:
impersonate: "impersonate"
anonymize_user: "anonymize user"
roll_up: "roll up IP blocks"
+ change_category_settings: "change category settings"
+ delete_category: "delete category"
+ create_category: "create category"
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/20150917071017_add_category_id_to_user_histories.rb b/db/migrate/20150917071017_add_category_id_to_user_histories.rb
new file mode 100644
index 000000000..117edef54
--- /dev/null
+++ b/db/migrate/20150917071017_add_category_id_to_user_histories.rb
@@ -0,0 +1,6 @@
+class AddCategoryIdToUserHistories < ActiveRecord::Migration
+ def change
+ add_column :user_histories, :category_id, :integer
+ add_index :user_histories, :category_id
+ end
+end
diff --git a/spec/controllers/categories_controller_spec.rb b/spec/controllers/categories_controller_spec.rb
index f03cfd05e..e545faedd 100644
--- a/spec/controllers/categories_controller_spec.rb
+++ b/spec/controllers/categories_controller_spec.rb
@@ -64,6 +64,7 @@ describe CategoriesController do
expect(category.slug).to eq("hello-cat")
expect(category.color).to eq("ff0")
expect(category.auto_close_hours).to eq(72)
+ expect(UserHistory.count).to eq(1)
end
end
end
@@ -90,6 +91,7 @@ describe CategoriesController do
it "deletes the record" do
Guardian.any_instance.expects(:can_delete_category?).returns(true)
expect { xhr :delete, :destroy, id: @category.slug}.to change(Category, :count).by(-1)
+ expect(UserHistory.count).to eq(1)
end
end
@@ -215,6 +217,17 @@ describe CategoriesController do
expect(@category.auto_close_hours).to eq(72)
expect(@category.custom_fields).to eq({"dancing" => "frogs"})
end
+
+ it 'logs the changes correctly' do
+ xhr :put , :update, id: @category.id, name: 'new name',
+ color: @category.color, text_color: @category.text_color,
+ slug: @category.slug,
+ permissions: {
+ "everyone" => CategoryGroup.permission_types[:create_post]
+ }
+
+ expect(UserHistory.count).to eq(2)
+ end
end
end
diff --git a/spec/fabricators/category_group_fabricator.rb b/spec/fabricators/category_group_fabricator.rb
new file mode 100644
index 000000000..898825b80
--- /dev/null
+++ b/spec/fabricators/category_group_fabricator.rb
@@ -0,0 +1,5 @@
+Fabricator(:category_group) do
+ category
+ group
+ permission_type 1
+end
diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb
index a00529fb2..8fdf5952a 100644
--- a/spec/models/category_spec.rb
+++ b/spec/models/category_spec.rb
@@ -36,6 +36,15 @@ describe Category do
end
end
+ describe "permissions_params" do
+ it "returns the right group names and permission type" do
+ category = Fabricate(:category)
+ group = Fabricate(:group)
+ category_group = Fabricate(:category_group, category: category, group: group)
+ expect(category.permissions_params).to eq({ "#{group.name}" => category_group.permission_type })
+ end
+ end
+
describe "topic_create_allowed and post_create_allowed" do
it "works" do
diff --git a/spec/services/staff_action_logger_spec.rb b/spec/services/staff_action_logger_spec.rb
index cb7d6e175..00eb85eee 100644
--- a/spec/services/staff_action_logger_spec.rb
+++ b/spec/services/staff_action_logger_spec.rb
@@ -271,4 +271,81 @@ describe StaffActionLogger do
expect(logged.topic_id).to be === 1234
end
end
+
+ describe 'log_category_settings_change' do
+ let(:category) { Fabricate(:category, name: 'haha') }
+ let(:category_group) { Fabricate(:category_group, category: category, permission_type: 1) }
+
+ it "raises an error when category is missing" do
+ expect { logger.log_category_settings_change(nil, nil) }.to raise_error(Discourse::InvalidParameters)
+ end
+
+ it "creates new UserHistory records" do
+ attributes = {
+ name: 'new_name',
+ permissions: { category_group.group_name => 2 }
+ }
+
+ category.update!(attributes)
+
+ logger.log_category_settings_change(category, attributes,
+ { category_group.group_name => category_group.permission_type }
+ )
+
+ expect(UserHistory.count).to eq(2)
+
+ permission_user_history = UserHistory.find_by_subject('permissions')
+ expect(permission_user_history.category_id).to eq(category.id)
+ expect(permission_user_history.previous_value).to eq({ category_group.group_name => 1 }.to_json)
+ expect(permission_user_history.new_value).to eq({ category_group.group_name => 2 }.to_json)
+ expect(permission_user_history.action).to eq(UserHistory.actions[:change_category_settings])
+ expect(permission_user_history.context).to eq(category.url)
+
+ name_user_history = UserHistory.find_by_subject('name')
+ expect(name_user_history.category).to eq(category)
+ expect(name_user_history.previous_value).to eq('haha')
+ expect(name_user_history.new_value).to eq('new_name')
+ end
+ end
+
+ describe 'log_category_deletion' do
+ let(:parent_category) { Fabricate(:category) }
+ let(:category) { Fabricate(:category, parent_category: parent_category) }
+
+ it "raises an error when category is missing" do
+ expect { logger.log_category_deletion(nil) }.to raise_error(Discourse::InvalidParameters)
+ end
+
+ it "creates a new UserHistory record" do
+ logger.log_category_deletion(category)
+
+ expect(UserHistory.count).to eq(1)
+ user_history = UserHistory.last
+
+ expect(user_history.subject).to eq(nil)
+ expect(user_history.category).to eq(category)
+ expect(user_history.details).to include("parent_category: #{parent_category.name}")
+ expect(user_history.context).to eq(category.url)
+ expect(user_history.action).to eq(UserHistory.actions[:delete_category])
+ end
+ end
+
+ describe 'log_category_creation' do
+ let(:category) { Fabricate(:category) }
+
+ it "raises an error when category is missing" do
+ expect { logger.log_category_deletion(nil) }.to raise_error(Discourse::InvalidParameters)
+ end
+
+ it "creates a new UserHistory record" do
+ logger.log_category_creation(category)
+
+ expect(UserHistory.count).to eq(1)
+ user_history = UserHistory.last
+
+ expect(user_history.category).to eq(category)
+ expect(user_history.context).to eq(category.url)
+ expect(user_history.action).to eq(UserHistory.actions[:create_category])
+ end
+ end
end