DEV: Add for_topic column to bookmarks (#14343)

DEV: Add for_topic column to bookmarks (#14343)

This new column will be used to indicate that a bookmark is at the topic level. The first post of a topic can be bookmarked twice after this change – with for_topic set to true and with for_topic set to false.

A later PR will use this column for logic to bookmark the topic, and then topic-level bookmark links will take you to the last unread post in the topic.

See also 22208836c5f2adcf8c85a62d27b67968743b20f0

diff --git a/app/models/bookmark.rb b/app/models/bookmark.rb
index 74a0d27..69219cf 100644
--- a/app/models/bookmark.rb
+++ b/app/models/bookmark.rb
@@ -46,7 +46,14 @@ class Bookmark < ActiveRecord::Base
   validates :name, length: { maximum: 100 }
 
   def unique_per_post_for_user
-    if Bookmark.exists?(user_id: user_id, post_id: post_id)
+    is_first_post = Post.where(id: post_id).pluck_first(:post_number) == 1
+    exists = if is_first_post
+      Bookmark.exists?(user_id: user_id, post_id: post_id, for_topic: for_topic)
+    else
+      Bookmark.exists?(user_id: user_id, post_id: post_id)
+    end
+
+    if exists
       self.errors.add(:base, I18n.t("bookmarks.errors.already_bookmarked_post"))
     end
   end
@@ -116,6 +123,10 @@ class Bookmark < ActiveRecord::Base
     joins(:post).where(user_id: user_id, posts: { topic_id: topic_id })
   }
 
+  def self.find_for_topic_by_user(topic_id, user_id)
+    for_user_in_topic(user_id, topic_id).where(for_topic: true).first
+  end
+
   def self.count_per_day(opts = nil)
     opts ||= {}
     result = where('bookmarks.created_at >= ?', opts[:start_date] || (opts[:since_days_ago] || 30).days.ago)
@@ -170,14 +181,15 @@ end
 #  reminder_set_at        :datetime
 #  auto_delete_preference :integer          default(0), not null
 #  pinned                 :boolean          default(FALSE)
+#  for_topic              :boolean          default(FALSE), not null
 #
 # Indexes
 #
-#  index_bookmarks_on_post_id              (post_id)
-#  index_bookmarks_on_reminder_at          (reminder_at)
-#  index_bookmarks_on_reminder_set_at      (reminder_set_at)
-#  index_bookmarks_on_reminder_type        (reminder_type)
-#  index_bookmarks_on_topic_id             (topic_id)
-#  index_bookmarks_on_user_id              (user_id)
-#  index_bookmarks_on_user_id_and_post_id  (user_id,post_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)
+#  index_bookmarks_on_reminder_type                      (reminder_type)
+#  index_bookmarks_on_topic_id                           (topic_id)
+#  index_bookmarks_on_user_id                            (user_id)
+#  index_bookmarks_on_user_id_and_post_id_and_for_topic  (user_id,post_id,for_topic) UNIQUE
 #
diff --git a/db/migrate/20210913032326_add_for_topic_to_bookmarks.rb b/db/migrate/20210913032326_add_for_topic_to_bookmarks.rb
new file mode 100644
index 0000000..2660cf6
--- /dev/null
+++ b/db/migrate/20210913032326_add_for_topic_to_bookmarks.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class AddForTopicToBookmarks < ActiveRecord::Migration[6.1]
+  def change
+    add_column :bookmarks, :for_topic, :boolean, default: false, null: false
+    add_index :bookmarks, [:user_id, :post_id, :for_topic], unique: true
+    remove_index :bookmarks, [:user_id, :post_id]
+  end
+end
diff --git a/spec/models/bookmark_spec.rb b/spec/models/bookmark_spec.rb
index d617e02..2e7386d 100644
--- a/spec/models/bookmark_spec.rb
+++ b/spec/models/bookmark_spec.rb
@@ -5,8 +5,8 @@ require 'rails_helper'
 describe Bookmark do
   fab!(:post) { Fabricate(:post) }
 
-  context 'validations' do
-    it 'does not allow user to bookmark a post twice' do
+  context "validations" do
+    it "does not allow user to bookmark a post twice, enforces unique bookmark per post, user, and for_topic" do
       bookmark = Fabricate(:bookmark, post: post)
       user = bookmark.user
 
@@ -17,6 +17,40 @@ describe Bookmark do
 
       expect(bookmark_2.valid?).to eq(false)
     end
+
+    it "allows a user to bookmark a post twice if it is the first post and for_topic is different" do
+      post.update!(post_number: 1)
+      bookmark = Fabricate(:bookmark, post: post, for_topic: false)
+      user = bookmark.user
+
+      bookmark_2 = Fabricate(:bookmark,
+        post: post,
+        user: user,
+        for_topic: true
+      )
+
+      expect(bookmark_2.valid?).to eq(true)
+
+      bookmark_3 = Fabricate.build(:bookmark,
+        post: post,
+        user: user,
+        for_topic: true
+      )
+
+      expect(bookmark_3.valid?).to eq(false)
+    end
+  end
+
+  describe "#find_for_topic_by_user" do
+    it "gets the for_topic bookmark for a user for a specific topic" do
+      user = Fabricate(:user)
+      post.update!(post_number: 1)
+      bookmark = Fabricate(:bookmark, user: user)
+      bookmark_2 = Fabricate(:bookmark, user: user, post: post, for_topic: true)
+      expect(Bookmark.find_for_topic_by_user(post.topic_id, user.id)).to eq(bookmark_2)
+      bookmark_2.update!(for_topic: false)
+      expect(Bookmark.find_for_topic_by_user(post.topic_id, user.id)).to eq(nil)
+    end
   end
 
   describe "#cleanup!" do

GitHub sha: d1d2298a4cee0b021db66ff116a665ee8ae27111

This commit appears in #14343 which was approved by lis2. It was merged by martin.

This commit has been mentioned on Discourse Meta. There might be relevant details there: