diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index 153f84e68c7..2931dbc42ec 100644 --- a/app/controllers/categories_controller.rb +++ b/app/controllers/categories_controller.rb @@ -147,10 +147,23 @@ class CategoriesController < ApplicationController guardian.ensure_can_edit!(@category) json_result(@category, serializer: CategorySerializer) do |cat| + old_category_params = category_params.dup cat.move_to(category_params[:position].to_i) if category_params[:position] category_params.delete(:position) + old_custom_fields = cat.custom_fields.dup + if category_params[:custom_fields] + category_params[:custom_fields].each do |key, value| + if value.present? + cat.custom_fields[key] = value + else + cat.custom_fields.delete(key) + end + end + end + category_params.delete(:custom_fields) + # properly null the value so the database constraint doesn't catch us category_params[:email_in] = nil if category_params[:email_in]&.blank? category_params[:minimum_required_tags] = 0 if category_params[:minimum_required_tags]&.blank? @@ -159,7 +172,12 @@ class CategoriesController < ApplicationController if result = cat.update(category_params) Scheduler::Defer.later "Log staff action change category settings" do - @staff_action_logger.log_category_settings_change(@category, category_params, old_permissions) + @staff_action_logger.log_category_settings_change( + @category, + old_category_params, + old_permissions: old_permissions, + old_custom_fields: old_custom_fields + ) end end diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb index 01c4e2bab10..ecb126b2cea 100644 --- a/app/services/staff_action_logger.rb +++ b/app/services/staff_action_logger.rb @@ -444,7 +444,7 @@ class StaffActionLogger )) end - def log_category_settings_change(category, category_params, old_permissions = nil) + def log_category_settings_change(category, category_params, old_permissions: nil, old_custom_fields: nil) validate_category(category) changed_attributes = category.previous_changes.slice(*category_params.keys) @@ -453,6 +453,12 @@ class StaffActionLogger changed_attributes.merge!(permissions: [old_permissions.to_json, category_params[:permissions].to_json]) end + if old_custom_fields && category_params[:custom_fields] + category_params[:custom_fields].each do |key, value| + changed_attributes["custom_fields[#{key}]"] = [old_custom_fields[key], value] + end + end + changed_attributes.each do |key, value| UserHistory.create!(params.merge( action: UserHistory.actions[:change_category_settings], diff --git a/spec/requests/categories_controller_spec.rb b/spec/requests/categories_controller_spec.rb index d8bb0b7a2e7..2cfed06d614 100644 --- a/spec/requests/categories_controller_spec.rb +++ b/spec/requests/categories_controller_spec.rb @@ -454,7 +454,8 @@ describe CategoriesController do category.update!( allowed_tags: ["hello", "world"], allowed_tag_groups: [tag_group_1.name], - required_tag_group_name: tag_group_2.name + required_tag_group_name: tag_group_2.name, + custom_fields: { field_1: 'hello', field_2: 'hello' } ) put "/categories/#{category.id}.json" @@ -463,20 +464,23 @@ describe CategoriesController do expect(category.tags.pluck(:name)).to contain_exactly("hello", "world") expect(category.tag_groups.pluck(:name)).to contain_exactly(tag_group_1.name) expect(category.required_tag_group).to eq(tag_group_2) + expect(category.custom_fields).to eq({ 'field_1' => 'hello', 'field_2' => 'hello' }) - put "/categories/#{category.id}.json", params: { allowed_tags: [] } + put "/categories/#{category.id}.json", params: { allowed_tags: [], custom_fields: { field_1: nil } } expect(response.status).to eq(200) category.reload expect(category.tags).to be_blank expect(category.tag_groups.pluck(:name)).to contain_exactly(tag_group_1.name) expect(category.required_tag_group).to eq(tag_group_2) + expect(category.custom_fields).to eq({ 'field_2' => 'hello' }) - put "/categories/#{category.id}.json", params: { allowed_tags: [], allowed_tag_groups: [], required_tag_group_name: nil } + put "/categories/#{category.id}.json", params: { allowed_tags: [], allowed_tag_groups: [], required_tag_group_name: nil, custom_fields: { field_1: 'hi', field_2: nil } } expect(response.status).to eq(200) category.reload expect(category.tags).to be_blank expect(category.tag_groups).to be_blank expect(category.required_tag_group).to eq(nil) + expect(category.custom_fields).to eq({ 'field_1' => 'hi' }) end end end diff --git a/spec/services/staff_action_logger_spec.rb b/spec/services/staff_action_logger_spec.rb index d40d382a727..8be97248940 100644 --- a/spec/services/staff_action_logger_spec.rb +++ b/spec/services/staff_action_logger_spec.rb @@ -352,8 +352,10 @@ describe StaffActionLogger do category.update!(attributes) - logger.log_category_settings_change(category, attributes, - category_group.group_name => category_group.permission_type + logger.log_category_settings_change( + category, + attributes, + old_permissions: { category_group.group_name => category_group.permission_type } ) expect(UserHistory.count).to eq(2) @@ -376,7 +378,11 @@ describe StaffActionLogger do old_permission = category.permissions_params category.update!(attributes) - logger.log_category_settings_change(category, attributes.merge(permissions: { "everyone" => 1 }), old_permission) + logger.log_category_settings_change( + category, + attributes.merge(permissions: { "everyone" => 1 }), + old_permissions: old_permission + ) expect(UserHistory.count).to eq(1) expect(UserHistory.find_by_subject('name').category).to eq(category)