Skip to content

Posts with long threads cannot load earlier comments

Marcelo Rivera requested to merge fix/focused-comments-load-2662 into master

Summary

Closes front#2662 (closed)

This fixes an issue with pagination when following a notification over a tag or reply in a comment thread, where there's more than one page of comments.

loadNext should be string

We handle pagination with a guid for previous pages and a token for next pages. The problem with this is they get inverted depending on the $descending param. This means that they can both be a guid and a token, so we need to accept both and check which one is which in our Repository

includeOffset was always set to true

We don't seem to use this anymore in v2, so I left it there for compatibility issues. That said, unless specified by the user, include_offset should be true by default if focused_urn is also truthy.

offset was always calculated from the last comment in the page

Before this fix, the offset was calculated like this:

$offset = count($comments) <= $limit ? '' : $comments[count($comments) - 1]->getGuid();

This only applies for descending (inverted) lists, though. If the list is inverted, we need to take it from the beginning:

$offset = count($comments) >= 0 ? $comments[0]->getGuid() : '';

Why is this necessary?

$offset is set to load-previous in case descending is false. If the offset guid was the last one in the page, then the "load previous" button would re-load the same page we already have

Steps to test

For this we'll need two users.

  1. Log in as user 1
  2. Create an activity
  3. Comment like crazy (20 comments, maybe?)
  4. Create one more comment, but this time tag user 2
  5. Log in as user 2, go to your notifications and click on it. There should be a "Load" button on the beginning of the list.

Here's an activity you can use for testing: https://fix-focused-comments-load-2662.minds.io/newsfeed/1099373604681814024 Note that I've tagged xandertest in comment 19. You should have a notification that goes directly to that thread, and you can use that for testing pagination there too.

Estimated Regression Scope

The very worst case would be comments repeating at the end of each page (which, in this case, it would be in the beginning, since they get reversed).

Edited by Marcelo Rivera

Merge request reports