FEATURE: Go to unread post for topic-level bookmarks (PR #14263)

Instead of going to the OP of the topic for topic-level bookmarks when clicking on the bookmark in the quick access menu or on the user bookmark list, this commit takes the user to the last unread post in the topic instead. This should be generally more useful than landing on the unchanging OP.

To make this work nicely, I needed to add the last_read_post_number to the BookmarkQuery based on the TopicUser association. It should not add too much extra weight to the query, because it is limited to the user that we are fetching bookmarks for.

Also fixed an issue where the bookmark serializer highest_post_number was not taking into account whether the user was staff, which is when we should use highest_staff_post_number instead.

GitHub

Does this need to be a computed property when it is not based off any other property?

I found this property name confusing because I thought a link is being returned. Instead, it returns a Topic object.

    scope.is_staff? ? topic.highest_staff_post_number : topic.highest_post_number

Just want to confirm that topic_users have been eager loaded? Otherwise, this is going to cause an N+1 queries problem.

Also, I wonder if we need to load all the topic_users of a topic when the bookmark is already scoped to a single user. Feels like the fetching of this data can be optimized further.

Sorry XD Just read bookmark_query.rb.

      bookmark = bookmark_query.list_all.find do |b|
        b.topic_id == bookmark1.topic_id
      end

      expect(bookmark.topic_users.map(&:user_id)).to contain_exactly(user.id)

Minor but I find this easier to follow.

    list = BookmarkQuery.new(user: bookmark.user).list_all.to_ary
    serializer = UserBookmarkSerializer.new(list.last, scope: Guardian.new(user))
    
    expect(serializer.highest_post_number).to eq(3)
    
    user.update!(admin: true)
    serializer = UserBookmarkSerializer.new(list.last, scope: Guardian.new(user))
    
    expect(serializer.highest_post_number).to eq(4)

I suspect this will work so we don’t have to query for things twice.

O this is still a draft xD I’m sorry didn’t notice.

@tgxworld haha a little too eager, not sure if I am using this yet, I did open it outside of Draft mode at first so maybe you saw it then