IntenseWebs/discourse
3
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-02-25 18:55:32 -06:00
Code Issues Packages Projects Releases Wiki Activity

FIX: Remove last_unread_post excerpt logic for bookmarks (#17979)

Browse Source 
The logic to determine what post excerpt to show for
a topic-level bookmark based on the last unread post
was complex and slow, so we decided to remove it and
always just use the first post excerpt.

This commit also fixes an issue where a couple of
instances of for_topic were missed when doing the
Bookmarkable refactors, so:

1. Clicking the topic bookmark link was not taking
   the user to the last unread post
2. When replying to a topic where there was a topic
   level bookmark with the auto delete preference
   of "on owner reply", we were not removing the
   bookmark from the UI correctly.

A test has been added for the former, the latter would
be quite time-consuming to test and not really worth
it considering it's quite an edge case UI bug.
This commit is contained in:
Martin Brennan
2022-08-19 09:35:25 +10:00
committed by GitHub
parent df9e546f5d
commit 49a70a37f1
6 changed files with 55 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
spec/requests/users_controller_spec.rb
View File

@@ -5485,10 +5485,7 @@ RSpec.describe UsersController do
let!(:post) { Fabricate(:post, topic: topic) }
let!(:bookmark) { Fabricate(:bookmark, name: 'Test', user: user, bookmarkable: topic) }
it "uses the last_read_post_number + 1 for the bookmarks excerpt" do
next_unread_post = Fabricate(:post_with_long_raw_content, topic: bookmark.bookmarkable)
Fabricate(:post_with_external_links, topic: bookmark.bookmarkable)
bookmark.reload
it "uses the first post of the topic for the bookmarks excerpt" do
TopicUser.change(user.id, bookmark.bookmarkable.id, { last_read_post_number: post.post_number })
sign_in(user)
@@ -5496,42 +5493,10 @@ RSpec.describe UsersController do
get "/u/#{user.username}/bookmarks.json"
expect(response.status).to eq(200)
bookmark_list = response.parsed_body["user_bookmark_list"]["bookmarks"]
expected_excerpt = PrettyText.excerpt(next_unread_post.cooked, 300, keep_emoji_images: true)
expected_excerpt = PrettyText.excerpt(topic.first_post.cooked, 300, keep_emoji_images: true)
expect(bookmark_list.first["excerpt"]).to eq(expected_excerpt)
end
it "does not use a small post for the last unread cooked post" do
small_action_post = Fabricate(:small_action, topic: bookmark.bookmarkable)
next_unread_post = Fabricate(:post_with_long_raw_content, topic: bookmark.bookmarkable)
Fabricate(:post_with_external_links, topic: bookmark.bookmarkable)
bookmark.reload
TopicUser.change(user.id, bookmark.bookmarkable.id, { last_read_post_number: post.post_number })
sign_in(user)
get "/u/#{user.username}/bookmarks.json"
expect(response.status).to eq(200)
bookmark_list = response.parsed_body["user_bookmark_list"]["bookmarks"]
expect(bookmark_list.first["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.bookmarkable)
small_action_post = Fabricate(:small_action, topic: bookmark.bookmarkable)
bookmark.reload
topic.reload
TopicUser.change(user.id, bookmark.bookmarkable.id, { last_read_post_number: small_action_post.post_number })
sign_in(user)
get "/u/#{user.username}/bookmarks.json"
expect(response.status).to eq(200)
bookmark_list = response.parsed_body["user_bookmark_list"]["bookmarks"]
expect(bookmark_list.first["excerpt"]).to eq(PrettyText.excerpt(last_regular_post.cooked, 300, keep_emoji_images: true))
end
describe "bookmarkable_url" do
context "with the link_to_first_unread_post option" do
it "is a full topic URL to the first unread post in the topic when the option is set" do