FIX: Use unread post excerpt for topic-level bookmark excerpt (#14414)

In the user bookmark list, when we show the excerpt of the bookmark
(which is usually just the bookmarked post excerpt), we want to show
the first unread post's excerpt instead for for_topic bookmarks. This
is because when the user clicks on that bookmark link, they are taken
to the first unread post in the topic, not the OP, as per:

27699648ef
This commit is contained in:
Martin Brennan 2021-09-22 12:47:36 +10:00 committed by GitHub
parent effc3ef7b4
commit a27d2b124c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 54 additions and 1 deletions

View File

@ -109,7 +109,24 @@ class UserBookmarkSerializer < ApplicationSerializer
end
def cooked
post.cooked
@cooked ||= \
if object.for_topic && last_read_post_number.present?
for_topic_cooked_post
else
post.cooked
end
end
def for_topic_cooked_post
post_number = [last_read_post_number + 1, highest_post_number].min
posts = Post.where(topic: topic, post_type: Post.types[:regular]).order(:post_number)
first_unread_cooked = posts.where("post_number >= ?", post_number).pluck_first(:cooked)
# if first_unread_cooked is blank this likely means that the last
# read post was either deleted or is a small action post.
# in this case we should just get the last regular post and
# use that for the cooked value so we have something to show
first_unread_cooked || posts.last.cooked
end
def slug

View File

@ -30,6 +30,7 @@ RSpec.describe UserBookmarkSerializer do
expect(s.post_user_username).to eq(bookmark.post.user.username)
expect(s.post_user_name).to eq(bookmark.post.user.name)
expect(s.post_user_avatar_template).not_to eq(nil)
expect(s.excerpt).to eq(PrettyText.excerpt(post.cooked, 300, keep_emoji_images: true))
end
it "uses the correct highest_post_number column based on whether the user is staff" do
@ -45,4 +46,39 @@ RSpec.describe UserBookmarkSerializer do
expect(serializer.highest_post_number).to eq(4)
end
context "for_topic bookmarks" do
before do
bookmark.update!(for_topic: true)
end
it "uses the last_read_post_number + 1 for the for_topic bookmarks excerpt" do
next_unread_post = Fabricate(:post_with_long_raw_content, topic: bookmark.topic)
Fabricate(:post_with_external_links, topic: bookmark.topic)
bookmark.reload
TopicUser.change(user.id, bookmark.topic.id, { last_read_post_number: post.post_number })
serializer = UserBookmarkSerializer.new(bookmark, scope: Guardian.new(user))
expect(serializer.excerpt).to eq(PrettyText.excerpt(next_unread_post.cooked, 300, keep_emoji_images: true))
end
it "does not use a small post for the last unread cooked post" do
small_action_post = Fabricate(:small_action, topic: bookmark.topic)
next_unread_post = Fabricate(:post_with_long_raw_content, topic: bookmark.topic)
Fabricate(:post_with_external_links, topic: bookmark.topic)
bookmark.reload
TopicUser.change(user.id, bookmark.topic.id, { last_read_post_number: post.post_number })
serializer = UserBookmarkSerializer.new(bookmark, scope: Guardian.new(user))
expect(serializer.excerpt).to eq(PrettyText.excerpt(next_unread_post.cooked, 300, keep_emoji_images: true))
end
it "handles the last read post in the topic being a small post by getting the last read regular post" do
last_regular_post = Fabricate(:post_with_long_raw_content, topic: bookmark.topic)
small_action_post = Fabricate(:small_action, topic: bookmark.topic)
bookmark.reload
TopicUser.change(user.id, bookmark.topic.id, { last_read_post_number: small_action_post.post_number })
serializer = UserBookmarkSerializer.new(bookmark, scope: Guardian.new(user))
expect(serializer.cooked).to eq(last_regular_post.cooked)
expect(serializer.excerpt).to eq(PrettyText.excerpt(last_regular_post.cooked, 300, keep_emoji_images: true))
end
end
end