FIX: Update only passed custom fields (#14357)

It used to replace custom fields instead of updating only the custom
fields that were passed. The changes to custom fields will also be
logged.

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
 

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],

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

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)