DEV: Change Bookmarkable registration to DiscoursePluginRegistry (#20556)

Similar spirit to e195e6f614,
this moves the Bookmarkable registration to DiscoursePluginRegistry
so plugins which are not enabled do not register additional
bookmarkable classes.

app/models/bookmark.rb

@@ -1,36 +1,21 @@
 # frozen_string_literal: true
 
 class Bookmark < ActiveRecord::Base
-  cattr_accessor :registered_bookmarkables
-  self.registered_bookmarkables = []
+  DEFAULT_BOOKMARKABLES = [
+    RegisteredBookmarkable.new(PostBookmarkable),
+    RegisteredBookmarkable.new(TopicBookmarkable),
+  ]
+
+  def self.registered_bookmarkables
+    Set.new(DEFAULT_BOOKMARKABLES | DiscoursePluginRegistry.bookmarkables)
+  end
 
   def self.registered_bookmarkable_from_type(type)
     Bookmark.registered_bookmarkables.find { |bm| bm.model.name == type }
   end
 
-  def self.register_bookmarkable(bookmarkable_klass)
-    return if Bookmark.registered_bookmarkable_from_type(bookmarkable_klass.model.name).present?
-    Bookmark.registered_bookmarkables << RegisteredBookmarkable.new(bookmarkable_klass)
-  end
-
-  ##
-  # This is called when the app loads, similar to AdminDashboardData.reset_problem_checks,
-  # so the default Post and Topic bookmarkables are registered on
-  # boot.
-  #
-  # This method also can be used in testing to reset bookmarkables between
-  # tests. It will also fire multiple times in development mode because
-  # classes are not cached.
-  def self.reset_bookmarkables
-    self.registered_bookmarkables = []
-
-    Bookmark.register_bookmarkable(PostBookmarkable)
-    Bookmark.register_bookmarkable(TopicBookmarkable)
-  end
-  reset_bookmarkables
-
   def self.valid_bookmarkable_types
-    Bookmark.registered_bookmarkables.map(&:model).map(&:to_s)
+    Bookmark.registered_bookmarkables.map { |bm| bm.model.to_s }
   end
 
   belongs_to :user

app/services/base_bookmarkable.rb

@@ -2,7 +2,7 @@
 
 ##
 # Anything that we want to be able to bookmark must be registered as a
-# bookmarkable type using Bookmark.register_bookmarkable(bookmarkable_klass),
+# bookmarkable type using Plugin::Instance#register_bookmarkable(bookmarkable_klass),
 # where the bookmarkable_klass is a class that implements this BaseBookmarkable
 # interface. Some examples are TopicBookmarkable and PostBookmarkable.
 #

app/services/registered_bookmarkable.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 #
-# Should only be created via the Bookmark.register_bookmarkable
+# Should only be created via the Plugin::Instance#register_bookmarkable
 # method; this is used to let the BookmarkQuery class query and
 # search additional bookmarks for the user bookmark list, and
 # also to enumerate on the registered RegisteredBookmarkable types.
@@ -11,7 +11,7 @@
 # when trying to save the Bookmark record. All things that are bookmarkable
 # must be registered in this way.
 #
-# See Bookmark#reset_bookmarkables for some examples on how registering
+# See Plugin::Instance#register_bookmarkable for some examples on how registering
 # bookmarkables works.
 #
 # See BaseBookmarkable for documentation on what return types should be

lib/discourse_plugin_registry.rb

@@ -109,6 +109,7 @@ class DiscoursePluginRegistry
   define_filtered_register :search_groups_set_query_callbacks
 
   define_filtered_register :about_stat_groups
+  define_filtered_register :bookmarkables
 
   def self.register_auth_provider(auth_provider)
     self.auth_providers << auth_provider

lib/plugin/instance.rb

@@ -1221,6 +1221,16 @@ class Plugin::Instance
     DiscoursePluginRegistry.register_user_destroyer_on_content_deletion_callback(callback, self)
   end
 
+  ##
+  # Register a class that implements [BaseBookmarkable], which represents another
+  # [ActiveRecord::Model] that may be bookmarked via the [Bookmark] model's
+  # polymorphic association. The class handles create and destroy hooks, querying,
+  # and reminders among other things.
+  def register_bookmarkable(klass)
+    return if Bookmark.registered_bookmarkable_from_type(klass.model.name).present?
+    DiscoursePluginRegistry.register_bookmarkable(RegisteredBookmarkable.new(klass), self)
+  end
+
   protected
 
   def self.js_path

plugins/chat/plugin.rb

@@ -280,7 +280,6 @@ after_initialize do
     Category.prepend Chat::CategoryExtension
     User.prepend Chat::UserExtension
     Jobs::UserEmail.prepend Chat::UserEmailExtension
-    Bookmark.register_bookmarkable(ChatMessageBookmarkable)
   end
 
   if Oneboxer.respond_to?(:register_local_handler)
@@ -777,6 +776,8 @@ after_initialize do
   register_user_destroyer_on_content_deletion_callback(
     Proc.new { |user| Jobs.enqueue(:delete_user_messages, user_id: user.id) },
   )
+
+  register_bookmarkable(ChatMessageBookmarkable)
 end
 
 if Rails.env == "test"

plugins/chat/spec/lib/chat_message_bookmarkable_spec.rb

@@ -11,10 +11,12 @@ describe ChatMessageBookmarkable do
   fab!(:channel) { Fabricate(:category_channel) }
 
   before do
-    Bookmark.register_bookmarkable(ChatMessageBookmarkable)
+    register_test_bookmarkable(ChatMessageBookmarkable)
     UserChatChannelMembership.create(chat_channel: channel, user: user, following: true)
   end
 
+  after { DiscoursePluginRegistry.reset! }
+
   let!(:message1) { Fabricate(:chat_message, chat_channel: channel) }
   let!(:message2) { Fabricate(:chat_message, chat_channel: channel) }
   let!(:bookmark1) do

plugins/chat/spec/models/chat_message_spec.rb

@@ -499,8 +499,6 @@ describe ChatMessage do
     end
 
     describe "bookmarks" do
-      before { Bookmark.register_bookmarkable(ChatMessageBookmarkable) }
-
       it "destroys bookmarks" do
         message_1 = Fabricate(:chat_message)
         bookmark_1 = Fabricate(:bookmark, bookmarkable: message_1)

plugins/chat/spec/plugin_helper.rb

@@ -18,9 +18,6 @@ module ChatSystemHelpers
     end
 
     Group.refresh_automatic_groups!
-
-    # this is reset after each test
-    Bookmark.register_bookmarkable(ChatMessageBookmarkable)
   end
 
   def chat_thread_chain_bootstrap(channel:, users:, messages_count: 4)

spec/lib/bookmark_query_spec.rb

@@ -12,7 +12,6 @@ RSpec.describe BookmarkQuery do
 
   describe "#list_all" do
     before do
-      Bookmark.reset_bookmarkables
       register_test_bookmarkable
 
       Fabricate(:topic_user, user: user, topic: post_bookmark.bookmarkable.topic)
@@ -20,14 +19,14 @@ RSpec.describe BookmarkQuery do
       user_bookmark
     end
 
+    after { DiscoursePluginRegistry.reset! }
+
     let(:post_bookmark) { Fabricate(:bookmark, user: user, bookmarkable: Fabricate(:post)) }
     let(:topic_bookmark) { Fabricate(:bookmark, user: user, bookmarkable: Fabricate(:topic)) }
     let(:user_bookmark) do
       Fabricate(:bookmark, user: user, bookmarkable: Fabricate(:user, username: "bookmarkqueen"))
     end
 
-    after { Bookmark.reset_bookmarkables }
-
     it "returns all the bookmarks for a user" do
       expect(bookmark_query.list_all.count).to eq(3)
     end
@@ -77,9 +76,7 @@ RSpec.describe BookmarkQuery do
         )
       end
 
-      before { Bookmark.reset_bookmarkables }
-
-      after { Bookmark.reset_bookmarkables }
+      after { DiscoursePluginRegistry.reset! }
 
       let(:bookmark3) do
         Fabricate(:bookmark, user: user, name: "Check up later", bookmarkable: Fabricate(:post))

spec/models/bookmark_spec.rb

@@ -4,6 +4,8 @@ RSpec.describe Bookmark do
   fab!(:post) { Fabricate(:post) }
 
   describe "Validations" do
+    after { DiscoursePluginRegistry.reset! }
+
     it "does not allow a user to create a bookmark with only one polymorphic column" do
       user = Fabricate(:user)
       bm = Bookmark.create(bookmarkable_id: post.id, user: user)

spec/models/user_bookmark_list_spec.rb

@@ -13,6 +13,8 @@ RSpec.describe UserBookmarkList do
     user_bookmark
   end
 
+  after { DiscoursePluginRegistry.reset! }
+
   let(:post_bookmark) { Fabricate(:bookmark, user: user, bookmarkable: Fabricate(:post)) }
   let(:topic_bookmark) { Fabricate(:bookmark, user: user, bookmarkable: Fabricate(:topic)) }
   let(:user_bookmark) { Fabricate(:bookmark, user: user, bookmarkable: Fabricate(:user)) }

spec/rails_helper.rb

@@ -145,9 +145,6 @@ module TestSetup
     # Don't queue badge grant in test mode
     BadgeGranter.disable_queue
 
-    # Make sure the default Post and Topic bookmarkables are registered
-    Bookmark.reset_bookmarkables
-
     OmniAuth.config.test_mode = false
   end
 end

spec/requests/users_controller_spec.rb

@@ -5947,7 +5947,7 @@ RSpec.describe UsersController do
       bookmark3 && bookmark4
     end
 
-    after { Bookmark.registered_bookmarkables = [] }
+    after { DiscoursePluginRegistry.reset! }
 
     let(:bookmark1) { Fabricate(:bookmark, user: user1, bookmarkable: Fabricate(:post)) }
     let(:bookmark2) { Fabricate(:bookmark, user: user1, bookmarkable: Fabricate(:topic)) }

spec/serializers/user_bookmark_list_serializer_spec.rb

@@ -11,6 +11,8 @@ RSpec.describe UserBookmarkListSerializer do
       user_bookmark
     end
 
+    after { DiscoursePluginRegistry.reset! }
+
     let(:post_bookmark) { Fabricate(:bookmark, user: user, bookmarkable: Fabricate(:post)) }
     let(:topic_bookmark) { Fabricate(:bookmark, user: user, bookmarkable: Fabricate(:topic)) }
     let(:user_bookmark) { Fabricate(:bookmark, user: user, bookmarkable: Fabricate(:user)) }

spec/support/bookmarkable_helper.rb

@@ -72,6 +72,9 @@ class UserTestBookmarkable < BaseBookmarkable
   end
 end
 
-def register_test_bookmarkable
-  Bookmark.register_bookmarkable(UserTestBookmarkable)
+def register_test_bookmarkable(klass = UserTestBookmarkable)
+  DiscoursePluginRegistry.register_bookmarkable(
+    RegisteredBookmarkable.new(klass),
+    stub(enabled?: true),
+  )
 end