FEATURE: Log staff actions for Category changes.

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)) {

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

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

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

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)
 #

app/serializers/user_history_serializer.rb

@@ -10,6 +10,7 @@ class UserHistorySerializer < ApplicationSerializer
              :new_value,
              :topic_id,
              :post_id,
+             :category_id,
              :action,
              :custom_type
 

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

config/locales/client.en.yml

@@ -2163,6 +2163,7 @@ en:
         ip_address: "IP"
         topic_id: "Topic ID"
         post_id: "Post ID"
+        category_id: "Category ID"
         delete: 'Delete'
         edit: 'Edit'
         save: 'Save'
@@ -2203,6 +2204,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."

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

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
 

spec/fabricators/category_group_fabricator.rb

@@ -0,0 +1,5 @@
+Fabricator(:category_group) do
+  category
+  group
+  permission_type 1
+end

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
 

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