DEV: Add a new type_source field to the Reviewable model. (discou…

…rse#31325)

This change adds a new `type_source` field to the `Reviewable` model, indicating whether the Reviewable type was registered by `core`, a plugin, or an `unknown` source.

When a plugin that registered a Reviewable type is disabled, this allows us to tell the user which plugin they need to re-enable to handle any orphan reviewable items.

app/assets/javascripts/discourse/app/controllers/review-index.js

@@ -6,6 +6,7 @@ import { underscore } from "@ember/string";
 import { isPresent } from "@ember/utils";
 import { ajax } from "discourse/lib/ajax";
 import { popupAjaxError } from "discourse/lib/ajax-error";
+import { REVIEWABLE_UNKNOWN_TYPE_SOURCE } from "discourse/lib/constants";
 import discourseComputed from "discourse/lib/decorators";
 import { i18n } from "discourse-i18n";
 
@@ -44,6 +45,7 @@ export default class ReviewIndexController extends Controller {
   sort_order = null;
   additional_filters = null;
   filterScoreType = null;
+  unknownTypeSource = REVIEWABLE_UNKNOWN_TYPE_SOURCE;
 
   @discourseComputed("reviewableTypes")
   allTypes() {

app/assets/javascripts/discourse/app/lib/constants.js

@@ -117,3 +117,5 @@ export const REPORT_MODES = {
   inline_table: "inline_table",
   storage_stats: "storage_stats",
 };
+
+export const REVIEWABLE_UNKNOWN_TYPE_SOURCE = "unknown";

app/assets/javascripts/discourse/app/routes/review-index.js

@@ -39,7 +39,7 @@ export default class ReviewIndex extends DiscourseRoute {
       filterCategoryId: meta.category_id,
       filterPriority: meta.priority,
       reviewableTypes: meta.reviewable_types,
-      unknownReviewableTypes: meta.unknown_reviewable_types,
+      unknownReviewableTypes: meta.unknown_reviewable_types_and_sources,
       scoreTypes: meta.score_types,
       filterUsername: meta.username,
       filterReviewedBy: meta.reviewed_by,

app/assets/javascripts/discourse/app/templates/review-index.hbs

@@ -6,8 +6,19 @@
       }}</span>
 
     <ul>
-      {{#each this.unknownReviewableTypes as |type|}}
-        <li>{{type}}</li>
+      {{#each this.unknownReviewableTypes as |reviewable|}}
+        {{#if (eq reviewable.source this.unknownTypeSource)}}
+          <li>{{i18n
+              "review.unknown.reviewable_unknown_source"
+              reviewableType=reviewable.type
+            }}</li>
+        {{else}}
+          <li>{{i18n
+              "review.unknown.reviewable_known_source"
+              reviewableType=reviewable.type
+              pluginName=reviewable.source
+            }}</li>
+        {{/if}}
       {{/each}}
     </ul>
     <span class="text">{{htmlSafe

app/assets/javascripts/discourse/tests/fixtures/concerns/notification-types.js

@@ -42,6 +42,7 @@ export const NOTIFICATION_TYPES = {
   new_features: 37,
   admin_problems: 38,
   linked_consolidated: 39,
+  chat_watched_thread: 40,
   following: 800,
   following_created_topic: 801,
   following_replied: 802,

app/controllers/reviewables_controller.rb

@@ -74,7 +74,7 @@ def index
           total_rows_reviewables: total_rows,
           types: meta_types,
           reviewable_types: Reviewable.types,
-          unknown_reviewable_types: Reviewable.unknown_types,
+          unknown_reviewable_types_and_sources: Reviewable.unknown_types_and_sources,
           score_types:
             ReviewableScore
               .types

app/models/reviewable.rb

@@ -7,6 +7,8 @@ class Reviewable < ActiveRecord::Base
     ReviewableUser: BasicReviewableUserSerializer,
   }
 
+  UNKNOWN_TYPE_SOURCE = "unknown"
+
   self.ignored_columns = [:reviewable_by_group_id]
 
   class UpdateConflict < StandardError
@@ -42,6 +44,8 @@ def initialize(action_id, klass)
 
   validates :reject_reason, length: { maximum: 2000 }
 
+  before_save :set_type_source
+
   after_create { log_history(:created, created_by) }
 
   after_commit(on: :create) { DiscourseEvent.trigger(:reviewable_created, self) }
@@ -77,6 +81,16 @@ def self.sti_names
     self.types.map(&:sti_name)
   end
 
+  def self.source_for(type)
+    type = type.sti_name if type.is_a?(Class)
+    return UNKNOWN_TYPE_SOURCE if Reviewable.sti_names.exclude?(type)
+
+    DiscoursePluginRegistry
+      .reviewable_types_lookup
+      .find { |r| r[:klass].sti_name == type }
+      &.dig(:plugin) || "core"
+  end
+
   def self.custom_filters
     @reviewable_filters ||= []
   end
@@ -89,6 +103,10 @@ def self.clear_custom_filters!
     @reviewable_filters = []
   end
 
+  def set_type_source
+    self.type_source = Reviewable.source_for(type)
+  end
+
   def created_new!
     self.created_new = true
     self.topic = target.topic if topic.blank? && target.is_a?(Post)
@@ -766,8 +784,14 @@ def self.find_by_flagger_or_queued_post_creator(id:, user_id:)
     )
   end
 
-  def self.unknown_types
-    Reviewable.pending.distinct.pluck(:type) - Reviewable.sti_names
+  def self.unknown_types_and_sources
+    @known_sources ||= Reviewable.sti_names.map { |n| [n, Reviewable.source_for(n)] }
+
+    known_unknowns = Reviewable.pending.distinct.pluck(:type, :type_source) - @known_sources
+
+    known_unknowns
+      .map { |type, source| { type: type, source: source } }
+      .sort_by { |e| [e[:source] == UNKNOWN_TYPE_SOURCE ? 1 : 0, e[:source], e[:type]] }
   end
 
   def self.destroy_unknown_types!
@@ -804,6 +828,7 @@ def update_flag_stats(status:, user_ids:)
 #
 #  id                      :bigint           not null, primary key
 #  type                    :string           not null
+#  type_source             :string           default("unknown"), not null
 #  status                  :integer          default("pending"), not null
 #  created_by_id           :integer          not null
 #  reviewable_by_moderator :boolean          default(FALSE), not null

app/models/reviewable_flagged_post.rb

@@ -399,6 +399,7 @@ def notify_poster(performed_by)
 #
 #  id                      :bigint           not null, primary key
 #  type                    :string           not null
+#  type_source             :string           default("unknown"), not null
 #  status                  :integer          default("pending"), not null
 #  created_by_id           :integer          not null
 #  reviewable_by_moderator :boolean          default(FALSE), not null

app/models/reviewable_post.rb

@@ -142,6 +142,7 @@ def successful_transition(to_state, recalculate_score: true)
 #
 #  id                      :bigint           not null, primary key
 #  type                    :string           not null
+#  type_source             :string           default("unknown"), not null
 #  status                  :integer          default("pending"), not null
 #  created_by_id           :integer          not null
 #  reviewable_by_moderator :boolean          default(FALSE), not null

app/models/reviewable_queued_post.rb

@@ -240,6 +240,7 @@ def status_changed_from_or_to_pending?
 #
 #  id                      :bigint           not null, primary key
 #  type                    :string           not null
+#  type_source             :string           default("unknown"), not null
 #  status                  :integer          default("pending"), not null
 #  created_by_id           :integer          not null
 #  reviewable_by_moderator :boolean          default(FALSE), not null

app/models/reviewable_user.rb

@@ -102,6 +102,7 @@ def is_a_suspect_user?
 #
 #  id                      :bigint           not null, primary key
 #  type                    :string           not null
+#  type_source             :string           default("unknown"), not null
 #  status                  :integer          default("pending"), not null
 #  created_by_id           :integer          not null
 #  reviewable_by_moderator :boolean          default(FALSE), not null

app/serializers/reviewable_serializer.rb

@@ -6,6 +6,7 @@ class ReviewableSerializer < ApplicationSerializer
   attributes(
     :id,
     :type,
+    :type_source,
     :topic_id,
     :topic_url,
     :target_url,

config/locales/client.en.yml

@@ -589,6 +589,8 @@ en:
           one: "You have pending reviewables from disabled plugin:"
           other: "You have pending reviewables from disabled plugins:"
         instruction: "They cannot be properly displayed until you enable the relevant plugin. Please enable the plugin and refresh the page. Alternatively, you can ignore them. <a href='%{url}' target='_blank'>Learn more...</a>"
+        reviewable_unknown_source: "%{reviewableType} (unknown plugin)"
+        reviewable_known_source: "%{reviewableType} (from the '%{pluginName}' plugin)"
         ignore_all: "Ignore all"
         enable_plugins: "Enable plugins"
         delete_confirm: "Are you sure you want to delete all reviews created by disabled plugins?"

db/migrate/20250212045125_add_type_source_to_reviewable.rb

@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+class AddTypeSourceToReviewable < ActiveRecord::Migration[7.2]
+  def change
+    add_column :reviewables, :type_source, :string, null: false, default: "unknown"
+
+    # Migrate known reviewables to have a type_source
+    known_reviewables = {
+      "chat" => %w[ReviewableChatMessage],
+      "core" => %w[ReviewableFlaggedPost ReviewableQueuedPost ReviewableUser ReviewablePost],
+      "discourse-ai" => %w[ReviewableAiChatMessage ReviewableAiPost],
+      "discourse-akismet" => %w[
+        ReviewableAkismetPost
+        ReviewableAkismetPostVotingComment
+        ReviewableAkismetUser
+      ],
+      "discourse-antivirus" => %w[ReviewableUpload],
+      "discourse-category-experts" => %w[ReviewableCategoryExpertSuggestion],
+      "discourse-post-voting" => %w[ReviewablePostVotingComment],
+    }
+
+    known_reviewables.each do |plugin, types|
+      types.each do |type|
+        Reviewable.where(type: type, type_source: "unknown").update_all(type_source: plugin)
+      end
+    end
+  end
+end

lib/discourse_plugin_registry.rb

@@ -50,6 +50,8 @@ def self.define_filtered_register(register_name)
     define_singleton_method("register_#{register_name.to_s.singularize}") do |value, plugin|
       public_send(:"_raw_#{register_name}") << { plugin: plugin, value: value }
     end
+
+    yield(self) if block_given?
   end
 
   define_register :javascripts, Set
@@ -129,6 +131,14 @@ def self.define_filtered_register(register_name)
 
   define_filtered_register :custom_filter_mappings
 
+  define_filtered_register :reviewable_types do |singleton|
+    singleton.define_singleton_method("reviewable_types_lookup") do
+      public_send(:"_raw_reviewable_types")
+        .filter_map { |h| { plugin: h[:plugin].name, klass: h[:value] } if h[:plugin].enabled? }
+        .uniq
+    end
+  end
+
   def self.register_auth_provider(auth_provider)
     self.auth_providers << auth_provider
   end

lib/tasks/javascript.rake

@@ -169,6 +169,8 @@ task "javascript:update_constants" => :environment do
     export const USER_FIELD_FLAGS = #{UserField::FLAG_ATTRIBUTES};
 
     export const REPORT_MODES = #{Report::MODES.to_json};
+
+    export const REVIEWABLE_UNKNOWN_TYPE_SOURCE = "#{Reviewable::UNKNOWN_TYPE_SOURCE}";
   JS
 
   pretty_notifications = Notification.types.map { |n| "  #{n[0]}: #{n[1]}," }.join("\n")

spec/lib/discourse_plugin_registry_spec.rb

@@ -15,6 +15,7 @@ class TestRegistry < DiscoursePluginRegistry
     let(:plugin_class) do
       Class.new(Plugin::Instance) do
         attr_accessor :enabled
+        attr_accessor :name
         def enabled?
           @enabled
         end
@@ -52,6 +53,20 @@ def enabled?
         plugin.enabled = false
         expect(fresh_registry.test_things.length).to eq(0)
       end
+
+      it "runs the callback block" do
+        fresh_registry.define_filtered_register(:test_other_things) do |singleton|
+          singleton.define_singleton_method(:my_fun_method) { true }
+        end
+
+        fresh_registry.register_test_other_thing("mything", plugin)
+
+        plugin.enabled = true
+        expect(fresh_registry.test_other_things).to contain_exactly("mything")
+
+        expect(fresh_registry.methods.include?(:my_fun_method)).to eq(true)
+        expect(fresh_registry.my_fun_method).to eq(true)
+      end
     end
   end
 

spec/lib/plugin/instance_spec.rb

@@ -625,13 +625,22 @@ def register_locale(locale, opts)
     subject(:register_reviewable_type) { plugin_instance.register_reviewable_type(new_type) }
 
     context "when the provided class inherits from `Reviewable`" do
-      let(:new_type) { Class.new(Reviewable) }
+      let(:new_type) do
+        class MyReviewable < Reviewable
+        end
+        MyReviewable
+      end
 
       it "adds the provided class to the existing types" do
         expect { register_reviewable_type }.to change { Reviewable.types.size }.by(1)
         expect(Reviewable.types).to include(new_type)
       end
 
+      it "shows the correct source for the new type" do
+        register_reviewable_type
+        expect(Reviewable.source_for(new_type)).to eq("discourse-sample-plugin")
+      end
+
       context "when the plugin is disabled" do
         before do
           register_reviewable_type

spec/models/reviewable_spec.rb

@@ -341,6 +341,89 @@
     expect(Reviewable.valid_type?("User")).to eq(false)
   end
 
+  describe ".source_for" do
+    it "returns the correct source" do
+      expect(Reviewable.source_for(ReviewablePost)).to eq("core")
+      expect(Reviewable.source_for(ReviewableFlaggedPost)).to eq("core")
+      expect(Reviewable.source_for(ReviewableQueuedPost)).to eq("core")
+      expect(Reviewable.source_for(ReviewableUser)).to eq("core")
+      expect(Reviewable.source_for("NonExistentType")).to eq("unknown")
+    end
+  end
+
+  describe ".unknown_types_and_sources" do
+    it "returns an empty array when no unknown types are present" do
+      expect(Reviewable.unknown_types_and_sources).to eq([])
+    end
+
+    context "with reviewables of unknown type or sources" do
+      fab!(:core_type) do
+        type = Fabricate(:reviewable)
+        type.update_columns(type: "ReviewableDoesntExist", type_source: "core")
+        type
+      end
+
+      fab!(:known_core_type) do
+        type = Fabricate(:reviewable)
+        type.update_columns(type: "ReviewableFlaggedPost", type_source: "core")
+        type
+      end
+
+      fab!(:unknown_type) do
+        type = Fabricate(:reviewable)
+        type.update_columns(type: "UnknownType", type_source: "unknown")
+        type
+      end
+
+      fab!(:plugin_type) do
+        type = Fabricate(:reviewable)
+        type.update_columns(type: "PluginReviewableDoesntExist", type_source: "my-plugin")
+        type
+      end
+
+      fab!(:plugin_type2) do
+        type = Fabricate(:reviewable)
+        type.update_columns(type: "PluginReviewableStillDoesntExist", type_source: "my-plugin")
+        type
+      end
+
+      fab!(:plugin_type3) do
+        type = Fabricate(:reviewable)
+        type.update_columns(
+          type: "AnotherPluginReviewableDoesntExist",
+          type_source: "another-plugin",
+        )
+        type
+      end
+
+      fab!(:plugin_type4) do
+        type = Fabricate(:reviewable)
+        type.update_columns(type: "ThisIsGettingSilly", type_source: "zzz-last-plugin")
+        type
+      end
+
+      fab!(:unknown_type2) do
+        type = Fabricate(:reviewable)
+        type.update_columns(type: "AnotherUnknownType", type_source: "unknown")
+        type
+      end
+
+      it "returns an array of unknown types, sorted by source (with 'unknown' always last), then by type" do
+        expect(Reviewable.unknown_types_and_sources).to eq(
+          [
+            { type: "AnotherPluginReviewableDoesntExist", source: "another-plugin" },
+            { type: "ReviewableDoesntExist", source: "core" },
+            { type: "PluginReviewableDoesntExist", source: "my-plugin" },
+            { type: "PluginReviewableStillDoesntExist", source: "my-plugin" },
+            { type: "ThisIsGettingSilly", source: "zzz-last-plugin" },
+            { type: "AnotherUnknownType", source: "unknown" },
+            { type: "UnknownType", source: "unknown" },
+          ],
+        )
+      end
+    end
+  end
+
   describe "events" do
     let!(:moderator) { Fabricate(:moderator) }
     let(:reviewable) { Fabricate(:reviewable) }