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 22208836c5
This commit is contained in:
Martin Brennan 2021-09-15 11:29:22 +10:00 committed by GitHub
parent 98866e138b
commit d1d2298a4c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 65 additions and 10 deletions

View File

@ -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
#

View File

@ -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

View File

@ -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