DEV: Switch over category settings to new table - Part 2 (#20580)

In #20135 we prevented invalid inputs from being accepted in category setting form fields on the front-end. We didn't do anything on the back-end at that time, because we were still discussing which path we wanted to take. Eventually we decided we want to move this to a new CategorySetting model.

This PR moves the num_auto_bump_daily from custom fields to the new CategorySetting model.

In addition it sets the default value to 0, which exhibits the same behaviour as when the value is NULL.

app/assets/javascripts/discourse/app/components/edit-category-settings.hbs

@@ -187,7 +187,7 @@
       {{i18n "category.num_auto_bump_daily"}}
     </label>
     <NumberField
-      @number={{this.category.custom_fields.num_auto_bump_daily}}
+      @number={{this.category.category_setting.num_auto_bump_daily}}
       @id="category-number-daily-bump"
       @type="number"
       @min="0"

app/controllers/categories_controller.rb

@@ -411,7 +411,7 @@ class CategoriesController < ApplicationController
             :read_only_banner,
             :default_list_filter,
             :reviewable_by_group_id,
-            category_setting_attributes: %i[auto_bump_cooldown_days],
+            category_setting_attributes: %i[auto_bump_cooldown_days num_auto_bump_daily],
             custom_fields: [custom_field_params],
             permissions: [*p.try(:keys)],
             allowed_tags: [],

app/models/category.rb

@@ -18,11 +18,9 @@ class Category < ActiveRecord::Base
 
   REQUIRE_TOPIC_APPROVAL = "require_topic_approval"
   REQUIRE_REPLY_APPROVAL = "require_reply_approval"
-  NUM_AUTO_BUMP_DAILY = "num_auto_bump_daily"
 
   register_custom_field_type(REQUIRE_TOPIC_APPROVAL, :boolean)
   register_custom_field_type(REQUIRE_REPLY_APPROVAL, :boolean)
-  register_custom_field_type(NUM_AUTO_BUMP_DAILY, :integer)
 
   belongs_to :topic
   belongs_to :topic_only_relative_url,
@@ -48,7 +46,11 @@ class Category < ActiveRecord::Base
 
   has_one :category_setting, dependent: :destroy
 
-  delegate :auto_bump_cooldown_days, to: :category_setting, allow_nil: true
+  delegate :auto_bump_cooldown_days,
+           :num_auto_bump_daily,
+           :num_auto_bump_daily=,
+           to: :category_setting,
+           allow_nil: true
 
   has_and_belongs_to_many :web_hooks
 
@@ -673,14 +675,6 @@ class Category < ActiveRecord::Base
     custom_fields[REQUIRE_REPLY_APPROVAL]
   end
 
-  def num_auto_bump_daily
-    custom_fields[NUM_AUTO_BUMP_DAILY]
-  end
-
-  def num_auto_bump_daily=(v)
-    custom_fields[NUM_AUTO_BUMP_DAILY] = v
-  end
-
   def auto_bump_limiter
     return nil if num_auto_bump_daily.to_i == 0
     RateLimiter.new(nil, "auto_bump_limit_#{self.id}", 1, 86_400 / num_auto_bump_daily.to_i)
@@ -691,22 +685,11 @@ class Category < ActiveRecord::Base
   end
 
   def self.auto_bump_topic!
-    bumped = false
+    Category
-
+      .joins(:category_setting)
-    auto_bumps =
+      .where("category_settings.num_auto_bump_daily > 0")
-      CategoryCustomField
+      .shuffle
-        .where(name: Category::NUM_AUTO_BUMP_DAILY)
+      .any?(&:auto_bump_topic!)
-        .where('NULLIF(value, \'\')::int > 0')
-        .pluck(:category_id)
-
-    if (auto_bumps.length > 0)
-      auto_bumps.shuffle.each do |category_id|
-        bumped = Category.find_by(id: category_id)&.auto_bump_topic!
-        break if bumped
-      end
-    end
-
-    bumped
   end
 
   # will automatically bump a single topic

app/models/category_setting.rb

@@ -26,7 +26,7 @@ end
 #  category_id             :bigint           not null
 #  require_topic_approval  :boolean
 #  require_reply_approval  :boolean
-#  num_auto_bump_daily     :integer
+#  num_auto_bump_daily     :integer          default(0)
 #  created_at              :datetime         not null
 #  updated_at              :datetime         not null
 #  auto_bump_cooldown_days :integer          default(1)

db/migrate/20230727015254_change_category_setting_num_auto_bump_daily_default.rb

@@ -0,0 +1,67 @@
+# frozen_string_literal: true
+
+class ChangeCategorySettingNumAutoBumpDailyDefault < ActiveRecord::Migration[7.0]
+  def up
+    change_column_default :category_settings, :num_auto_bump_daily, 0
+
+    execute(<<~SQL)
+      WITH custom_fields AS (
+        SELECT
+          category_id,
+          MAX(
+            CASE WHEN (name = 'require_topic_approval')
+            THEN value ELSE NULL END
+          )::boolean AS require_topic_approval,
+          MAX(
+            CASE WHEN (name = 'require_reply_approval')
+            THEN value ELSE NULL END
+          )::boolean AS require_reply_approval,
+          MAX(
+            CASE WHEN (name = 'num_auto_bump_daily')
+            THEN value ELSE NULL END
+          )::integer AS num_auto_bump_daily,
+          NOW() AS created_at,
+          NOW() AS updated_at
+        FROM category_custom_fields
+        WHERE name IN (
+          'require_topic_approval',
+          'require_reply_approval',
+          'num_auto_bump_daily'
+        )
+        GROUP BY category_id
+      )
+      INSERT INTO
+        category_settings(
+          category_id,
+          require_topic_approval,
+          require_reply_approval,
+          num_auto_bump_daily,
+          created_at,
+          updated_at
+        )
+      SELECT * FROM custom_fields
+      ON CONFLICT (category_id) DO
+      UPDATE SET
+        require_topic_approval = EXCLUDED.require_topic_approval,
+        require_reply_approval = EXCLUDED.require_reply_approval,
+        num_auto_bump_daily = EXCLUDED.num_auto_bump_daily,
+        updated_at = NOW()
+    SQL
+
+    execute(<<~SQL)
+      UPDATE category_settings
+      SET num_auto_bump_daily = 0
+      WHERE num_auto_bump_daily IS NULL;
+    SQL
+  end
+
+  def down
+    change_column_default :category_settings, :num_auto_bump_daily, nil
+
+    execute(<<~SQL)
+      UPDATE category_settings
+      SET num_auto_bump_daily = NULL
+      WHERE num_auto_bump_daily = 0;
+    SQL
+  end
+end

spec/models/category_spec.rb

@@ -976,10 +976,10 @@ RSpec.describe Category do
       category =
         Fabricate(
           :category_with_definition,
-          num_auto_bump_daily: 2,
           created_at: 1.minute.ago,
           category_setting_attributes: {
             auto_bump_cooldown_days: 1,
+            num_auto_bump_daily: 2,
           },
         )
       category.clear_auto_bump_cache!

spec/requests/categories_controller_spec.rb

@@ -716,7 +716,6 @@ RSpec.describe CategoriesController do
         it "updates per-category settings correctly" do
           category.custom_fields[Category::REQUIRE_TOPIC_APPROVAL] = false
           category.custom_fields[Category::REQUIRE_REPLY_APPROVAL] = false
-          category.custom_fields[Category::NUM_AUTO_BUMP_DAILY] = 0
 
           category.navigate_to_first_post_after_read = false
           category.save!
@@ -730,6 +729,8 @@ RSpec.describe CategoriesController do
                 custom_fields: {
                   require_reply_approval: true,
                   require_topic_approval: true,
+                },
+                category_setting_attributes: {
                   num_auto_bump_daily: 10,
                 },
               }