DEV: Move solved custom fields into a table (#342)

Related: 
- #309
- #341

Requires:
- discourse/discourse#31954

This commit converts all use of post and topic custom fields into a dedicated table:
- migration for copying custom field into table
- swap app usage of custom fields to table

This commit does not attempt to fix issues or optimise, and does not delete old data from custom fields _yet_.

Author: nattsw

.discourse-compatibility

@@ -1,3 +1,4 @@
+< 3.5.0.beta2-dev: e82c6ae1ca38ccebb34669148f8de93a3028906e
 < 3.5.0.beta1-dev: 5450a5ef4e2ae35185320fc6af9678621026e148
 < 3.4.0.beta4-dev: 3f724bf3114cc7877fa757bc8035f13a7390c739
 < 3.4.0.beta2-dev: 1bbdfd8f5681171dc3f0e9ea93cd56997dc7938a

app/models/discourse_solved/solved_topic.rb

@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+module DiscourseSolved
+  class SolvedTopic < ActiveRecord::Base
+    self.table_name = "discourse_solved_solved_topics"
+
+    belongs_to :topic, class_name: "Topic"
+    belongs_to :answer_post, class_name: "Post", foreign_key: "answer_post_id"
+    belongs_to :accepter, class_name: "User", foreign_key: "accepter_user_id"
+    belongs_to :topic_timer
+
+    validates :topic_id, presence: true
+    validates :answer_post_id, presence: true
+    validates :accepter_user_id, presence: true
+  end
+end
+
+# == Schema Information
+#
+# Table name: discourse_solved_solved_topics
+#
+#  id               :bigint           not null, primary key
+#  topic_id         :integer          not null
+#  answer_post_id   :integer          not null
+#  accepter_user_id :integer          not null
+#  topic_timer_id   :integer
+#  created_at       :datetime         not null
+#  updated_at       :datetime         not null
+#
+# Indexes
+#
+#  index_discourse_solved_solved_topics_on_answer_post_id  (answer_post_id) UNIQUE
+#  index_discourse_solved_solved_topics_on_topic_id        (topic_id) UNIQUE
+#

app/serializers/concerns/discourse_solved/topic_answer_mixin.rb

@@ -7,7 +7,7 @@ def self.included(klass)
     end
 
     def has_accepted_answer
-      object.custom_fields[::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD].present?
+      object&.solved.present?
     end
 
     def include_has_accepted_answer?

db/fixtures/001_badges.rb

@@ -3,14 +3,13 @@
 first_solution_query = <<~SQL
   SELECT post_id, user_id, created_at AS granted_at
   FROM (
-           SELECT p.id AS post_id, p.user_id, pcf.created_at,
-                  ROW_NUMBER() OVER (PARTITION BY p.user_id ORDER BY pcf.created_at) AS row_number
-           FROM post_custom_fields pcf
-                JOIN badge_posts p ON pcf.post_id = p.id
-                JOIN topics t ON p.topic_id = t.id
-           WHERE pcf.name = 'is_accepted_answer'
-             AND p.user_id <> t.user_id -- ignore topics solved by OP
-             AND (:backfill OR p.id IN (:post_ids))
+           SELECT p.id AS post_id, p.user_id, dsst.created_at,
+              ROW_NUMBER() OVER (PARTITION BY p.user_id ORDER BY dsst.created_at) AS row_number
+           FROM discourse_solved_solved_topics dsst
+              JOIN badge_posts p ON dsst.answer_post_id = p.id
+              JOIN topics t ON p.topic_id = t.id
+           WHERE p.user_id <> t.user_id -- ignore topics solved by OP
+              AND (:backfill OR p.id IN (:post_ids))
        ) x
   WHERE row_number = 1
 SQL
@@ -32,12 +31,11 @@
 
 def solved_query_with_count(min_count)
   <<~SQL
-    SELECT p.user_id, MAX(pcf.created_at) AS granted_at
-    FROM post_custom_fields pcf
-         JOIN badge_posts p ON pcf.post_id = p.id
+    SELECT p.user_id, MAX(dsst.created_at) AS granted_at
+    FROM discourse_solved_solved_topics dsst
+         JOIN badge_posts p ON dsst.answer_post_id = p.id
          JOIN topics t ON p.topic_id = t.id
-    WHERE pcf.name = 'is_accepted_answer'
-      AND p.user_id <> t.user_id -- ignore topics solved by OP
+    WHERE p.user_id <> t.user_id -- ignore topics solved by OP
       AND (:backfill OR p.id IN (:post_ids))
     GROUP BY p.user_id
     HAVING COUNT(*) >= #{min_count}

db/migrate/20250318024824_create_discourse_solved_solved_topics.rb

@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+#
+class CreateDiscourseSolvedSolvedTopics < ActiveRecord::Migration[7.2]
+  def change
+    create_table :discourse_solved_solved_topics do |t|
+      t.integer :topic_id, null: false
+      t.integer :answer_post_id, null: false
+      t.integer :accepter_user_id, null: false
+      t.integer :topic_timer_id
+      t.timestamps
+    end
+  end
+end

db/migrate/20250318024953_copy_solved_topic_custom_field_to_discourse_solved_solved_topics.rb

@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+#
+class CopySolvedTopicCustomFieldToDiscourseSolvedSolvedTopics < ActiveRecord::Migration[7.2]
+  disable_ddl_transaction!
+
+  BATCH_SIZE = 5000
+
+  def up
+    last_id = 0
+    loop do
+      rows = DB.query(<<~SQL, last_id: last_id, batch_size: BATCH_SIZE)
+        INSERT INTO discourse_solved_solved_topics (
+          topic_id,
+          answer_post_id,
+          topic_timer_id,
+          accepter_user_id,
+          created_at,
+          updated_at
+        )
+        SELECT DISTINCT ON (tc.topic_id)
+          tc.topic_id,
+          CAST(tc.value AS INTEGER),
+          CAST(tc2.value AS INTEGER),
+          COALESCE(ua.acting_user_id, -1),
+          tc.created_at,
+          tc.updated_at
+        FROM topic_custom_fields tc
+        LEFT JOIN topic_custom_fields tc2
+          ON tc2.topic_id = tc.topic_id
+          AND tc2.name = 'solved_auto_close_topic_timer_id'
+        LEFT JOIN user_actions ua
+          ON ua.target_topic_id = tc.topic_id
+          AND ua.action_type = 15
+        WHERE tc.name = 'accepted_answer_post_id'
+          AND tc.id > :last_id
+        ORDER BY tc.topic_id, ua.created_at DESC
+        LIMIT :batch_size
+      SQL
+
+      break if rows.length == 0
+      last_id += BATCH_SIZE
+    end
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

db/migrate/20250318025147_add_index_for_discourse_solved_topics.rb

@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+#
+class AddIndexForDiscourseSolvedTopics < ActiveRecord::Migration[7.2]
+  disable_ddl_transaction!
+
+  def change
+    remove_index :discourse_solved_solved_topics,
+                 :topic_id,
+                 algorithm: :concurrently,
+                 unique: true,
+                 if_exists: true
+    remove_index :discourse_solved_solved_topics,
+                 :answer_post_id,
+                 algorithm: :concurrently,
+                 unique: true,
+                 if_exists: true
+
+    add_index :discourse_solved_solved_topics, :topic_id, unique: true, algorithm: :concurrently
+    add_index :discourse_solved_solved_topics,
+              :answer_post_id,
+              unique: true,
+              algorithm: :concurrently
+  end
+end

lib/discourse_assign/entry_point.rb

@@ -2,15 +2,13 @@
 
 module DiscourseAssign
   class EntryPoint
+    # TODO: These four plugin api usages should ideally be in the assign plugin, not the solved plugin.
+    # They have been moved here from plugin.rb as part of the custom fields migration.
+
     def self.inject(plugin)
       plugin.register_modifier(:assigns_reminder_assigned_topics_query) do |query|
         next query if !SiteSetting.ignore_solved_topics_in_assigned_reminder
-        query.where.not(
-          id:
-            TopicCustomField.where(
-              name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD,
-            ).pluck(:topic_id),
-        )
+        query.where.not(id: DiscourseSolved::SolvedTopic.select(:topic_id))
       end
 
       plugin.register_modifier(:assigned_count_for_user_query) do |query, user|

lib/discourse_dev/discourse_solved.rb

@@ -11,24 +11,24 @@ def self.populate(plugin)
 
           solved_category =
             DiscourseDev::Record.random(
-              Category.where(
+              ::Category.where(
                 read_restricted: false,
                 id: records.pluck(:id),
                 parent_category_id: nil,
               ),
             )
-          CategoryCustomField.create!(
+          ::CategoryCustomField.create!(
             category_id: solved_category.id,
             name: ::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD,
             value: "true",
           )
           puts "discourse-solved enabled on category '#{solved_category.name}' (#{solved_category.id})."
         elsif type == :topic
-          topics = Topic.where(id: records.pluck(:id))
+          topics = ::Topic.where(id: records.pluck(:id))
 
           unless SiteSetting.allow_solved_on_all_topics
             solved_category_id =
-              CategoryCustomField
+              ::CategoryCustomField
                 .where(name: ::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD, value: "true")
                 .first
                 .category_id

lib/discourse_solved/before_head_close.rb

@@ -40,10 +40,7 @@ def html
       },
     }
 
-    if accepted_answer =
-         Post.find_by(
-           id: topic.custom_fields[::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD],
-         )
+    if accepted_answer = topic.solved&.answer_post
       question_json["answerCount"] = 1
       question_json[:acceptedAnswer] = {
         "@type" => "Answer",

lib/discourse_solved/register_filters.rb

@@ -4,46 +4,38 @@ module DiscourseSolved
   class RegisterFilters
     def self.register(plugin)
       solved_callback = ->(scope) do
-        sql = <<~SQL
-      topics.id IN (
-        SELECT topic_id
-          FROM topic_custom_fields
-         WHERE name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}'
-           AND value IS NOT NULL
-      )
-    SQL
-
-        scope.where(sql).where("topics.archetype <> ?", Archetype.private_message)
+        scope.joins(
+          "INNER JOIN discourse_solved_solved_topics ON discourse_solved_solved_topics.topic_id = topics.id",
+        ).where("topics.archetype <> ?", Archetype.private_message)
       end
+
       unsolved_callback = ->(scope) do
-        scope = scope.where <<~SQL
+        scope = scope.where(<<~SQL)
           topics.id NOT IN (
             SELECT topic_id
-              FROM topic_custom_fields
-             WHERE name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}'
-               AND value IS NOT NULL
+            FROM discourse_solved_solved_topics
           )
         SQL
 
         if !SiteSetting.allow_solved_on_all_topics
           tag_ids = Tag.where(name: SiteSetting.enable_solved_tags.split("|")).pluck(:id)
 
           scope = scope.where <<~SQL, tag_ids
-        topics.id IN (
-          SELECT t.id
-            FROM topics t
-            JOIN category_custom_fields cc
-              ON t.category_id = cc.category_id
-             AND cc.name = '#{::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD}'
-             AND cc.value = 'true'
-        )
-        OR
-        topics.id IN (
-          SELECT topic_id
-            FROM topic_tags
-           WHERE tag_id IN (?)
-        )
-      SQL
+            topics.id IN (
+              SELECT t.id
+                FROM topics t
+                JOIN category_custom_fields cc
+                  ON t.category_id = cc.category_id
+                 AND cc.name = '#{::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD}'
+                 AND cc.value = 'true'
+            )
+            OR
+            topics.id IN (
+              SELECT topic_id
+                FROM topic_tags
+               WHERE tag_id IN (?)
+            )
+          SQL
         end
 
         scope.where("topics.archetype <> ?", Archetype.private_message)

lib/discourse_solved/topic_extension.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+module DiscourseSolved::TopicExtension
+  extend ActiveSupport::Concern
+
+  prepended { has_one :solved, class_name: "DiscourseSolved::SolvedTopic", dependent: :destroy }
+end

lib/discourse_solved/topic_view_serializer_extension.rb

@@ -43,12 +43,6 @@ def accepted_answer_post_info
   end
 
   def accepted_answer_post_id
-    id = object.topic.custom_fields[::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD]
-    # a bit messy but race conditions can give us an array here, avoid
-    begin
-      id && id.to_i
-    rescue StandardError
-      nil
-    end
+    object.topic.solved&.answer_post_id
   end
 end

lib/discourse_solved/user_summary_extension.rb

@@ -4,6 +4,6 @@ module DiscourseSolved::UserSummaryExtension
   extend ActiveSupport::Concern
 
   def solved_count
-    UserAction.where(user: @user).where(action_type: UserAction::SOLVED).count
+    DiscourseSolved::SolvedTopic.where(accepter: @user).count
   end
 end