DEV: Add polymorphic bookmarkable columns (#15454)

We are planning on attaching bookmarks to more and
more other models, so it makes sense to make a polymorphic
relationship to handle this. This commit adds the new
columns and backfills them in the bookmark table, and
makes sure that any new bookmark changes fill in the columns
via DB triggers.

This way we can gradually change the frontend and backend
to use these new columns, and eventually delete the
old post_id and for_topic columns in `bookmarks`.

app/models/bookmark.rb

@@ -4,6 +4,7 @@ class Bookmark < ActiveRecord::Base
   belongs_to :user
   belongs_to :post
   has_one :topic, through: :post
+  belongs_to :bookmarkable, polymorphic: true
 
   delegate :topic_id, to: :post
 
@@ -171,9 +172,12 @@ end
 #  auto_delete_preference :integer          default(0), not null
 #  pinned                 :boolean          default(FALSE)
 #  for_topic              :boolean          default(FALSE), not null
+#  bookmarkable_id        :integer
+#  bookmarkable_type      :string
 #
 # Indexes
 #
+#  idx_bookmarks_user_polymorphic_unique                 (user_id,bookmarkable_type,bookmarkable_id) UNIQUE
 #  index_bookmarks_on_post_id                            (post_id)
 #  index_bookmarks_on_reminder_at                        (reminder_at)
 #  index_bookmarks_on_reminder_set_at                    (reminder_set_at)

app/models/post.rb

@@ -46,6 +46,9 @@ class Post < ActiveRecord::Base
   has_many :uploads, through: :post_uploads
 
   has_one :post_stat
+
+  # When we are ready we can add as: :bookmarkable here to use the
+  # polymorphic association.
   has_many :bookmarks
 
   has_one :incoming_email

app/models/topic.rb

@@ -203,7 +203,15 @@ class Topic < ActiveRecord::Base
   belongs_to :category
   has_many :category_users, through: :category
   has_many :posts
+
+  # When we are ready we can add as: :bookmarkable here to use the
+  # polymorphic association.
+  #
+  # At that time we may also want to make another association for example
+  # :topic_bookmarks that get all of the bookmarks for that topic's bookmarkable id
+  # and type, because this one gets all of the post bookmarks.
   has_many :bookmarks, through: :posts
+
   has_many :ordered_posts, -> { order(post_number: :asc) }, class_name: "Post"
   has_many :topic_allowed_users
   has_many :topic_allowed_groups

db/migrate/20220104053343_add_bookmark_polymorphic_columns.rb

@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+class AddBookmarkPolymorphicColumns < ActiveRecord::Migration[6.1]
+  def change
+    add_column :bookmarks, :bookmarkable_id, :integer
+    add_column :bookmarks, :bookmarkable_type, :string
+
+    add_index :bookmarks, [:user_id, :bookmarkable_type, :bookmarkable_id], name: "idx_bookmarks_user_polymorphic_unique", unique: true
+  end
+end

db/migrate/20220105024605_add_trigger_for_polymorphic_bookmark_columns_to_sync_data.rb

@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+class AddTriggerForPolymorphicBookmarkColumnsToSyncData < ActiveRecord::Migration[6.1]
+  def up
+    DB.exec <<~SQL
+      CREATE OR REPLACE FUNCTION sync_bookmarks_polymorphic_column_data()
+      RETURNS TRIGGER
+      LANGUAGE PLPGSQL AS $rcr$
+      BEGIN
+        IF NEW.for_topic
+        THEN
+          NEW.bookmarkable_id = (SELECT topic_id FROM posts WHERE posts.id = NEW.post_id);
+          NEW.bookmarkable_type = 'Topic';
+        ELSE
+          NEW.bookmarkable_id = NEW.post_id;
+          NEW.bookmarkable_type = 'Post';
+        END IF;
+        RETURN NEW;
+      END
+      $rcr$;
+    SQL
+
+    DB.exec <<~SQL
+      CREATE TRIGGER bookmarks_polymorphic_data_sync
+      BEFORE INSERT OR UPDATE OF post_id, for_topic ON bookmarks
+      FOR EACH ROW
+      EXECUTE FUNCTION sync_bookmarks_polymorphic_column_data();
+    SQL
+
+    # sync data that already exists in the table
+    DB.exec(<<~SQL)
+      UPDATE bookmarks
+      SET bookmarkable_id = post_id, bookmarkable_type = 'Post'
+      WHERE NOT bookmarks.for_topic
+    SQL
+    DB.exec(<<~SQL)
+      UPDATE bookmarks
+      SET bookmarkable_id = posts.topic_id, bookmarkable_type = 'Topic'
+      FROM posts
+      WHERE bookmarks.for_topic AND posts.id = bookmarks.post_id
+    SQL
+  end
+
+  def down
+    DB.exec("DROP TRIGGER IF EXISTS bookmarks_polymorphic_data_sync")
+    DB.exec("DROP FUNCTION IF EXISTS sync_bookmarks_polymorphic_column_data")
+  end
+end

spec/lib/bookmark_manager_spec.rb

@@ -12,12 +12,14 @@ RSpec.describe BookmarkManager do
   subject { described_class.new(user) }
 
   describe ".create" do
-    it "creates the bookmark for the user" do
+    it "creates the bookmark for the user with the correct polymorphic type" do
       subject.create(post_id: post.id, name: name)
       bookmark = Bookmark.find_by(user: user)
 
       expect(bookmark.post_id).to eq(post.id)
       expect(bookmark.topic_id).to eq(post.topic_id)
+      expect(bookmark.bookmarkable_id).to eq(post.id)
+      expect(bookmark.bookmarkable_type).to eq("Post")
     end
 
     it "allows creating a bookmark for the topic and for the first post" do
@@ -27,6 +29,8 @@ RSpec.describe BookmarkManager do
       expect(bookmark.post_id).to eq(post.id)
       expect(bookmark.topic_id).to eq(post.topic_id)
       expect(bookmark.for_topic).to eq(true)
+      expect(bookmark.bookmarkable_id).to eq(post.topic_id)
+      expect(bookmark.bookmarkable_type).to eq("Topic")
 
       subject.create(post_id: post.id, name: name)
       bookmark = Bookmark.find_by(user: user, post_id: post.id, for_topic: false)
@@ -34,6 +38,8 @@ RSpec.describe BookmarkManager do
       expect(bookmark.post_id).to eq(post.id)
       expect(bookmark.topic_id).to eq(post.topic_id)
       expect(bookmark.for_topic).to eq(false)
+      expect(bookmark.bookmarkable_id).to eq(post.id)
+      expect(bookmark.bookmarkable_type).to eq("Post")
     end
 
     it "errors when creating a for_topic bookmark for a post that is not the first one" do