From a27d2b124c75ffbf39276facacf146301081a5b3 Mon Sep 17 00:00:00 2001 From: Martin Brennan <mjrbrennan@gmail.com> Date: Wed, 22 Sep 2021 12:47:36 +1000 Subject: [PATCH] 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: 27699648ef8a73d20f0729e97b8ca684513edfd5 --- app/serializers/user_bookmark_serializer.rb | 19 +++++++++- .../user_bookmark_serializer_spec.rb | 36 +++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/app/serializers/user_bookmark_serializer.rb b/app/serializers/user_bookmark_serializer.rb index d408878adfd..1edb26adb69 100644 --- a/app/serializers/user_bookmark_serializer.rb +++ b/app/serializers/user_bookmark_serializer.rb @@ -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 diff --git a/spec/serializers/user_bookmark_serializer_spec.rb b/spec/serializers/user_bookmark_serializer_spec.rb index 8376c98cd3e..41ac72fd17e 100644 --- a/spec/serializers/user_bookmark_serializer_spec.rb +++ b/spec/serializers/user_bookmark_serializer_spec.rb @@ -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