Comment Template: Optimize the way pagination is handled for comments with replies

[gziolo]

What problem does this address?

Originally raised by @michalczaplinski in #36065 (comment) while working on the support for comments with replies:

I wonder what the best UX here should be. Like I mention in the video, if this setting is set:

we cannot easily fetch the X top level comments AND it's replies.

The extended version of the issue is explained in the video included:

newmovie.mov

The initial approach taken in #36065 looks like the following:

gutenberg/packages/block-library/src/comment-template/edit.js

Lines 141 to 148 in 93393d0

	// We convert the flat list of comments to tree.
	// Then, we show only a maximum of `queryPerPage` number of comments.
	// This is because passing `per_page` to `getEntityRecords()` does not
	// take into account nested comments.
	const comments = useMemo(
	() => convertToTree( rawComments ).slice( 0, queryPerPage ),
	[ rawComments, queryPerPage ]
	);

This fetches all comments (unless there is a limit enforced on the endpoint that would be another issue) and then constructs a tree structure representing top-level comments and their replies. The drawback is that we fetch comments that we might never use if the number of the top-level comments is higher than the number of comments per page. Another concern would be that we might want to be able to use different sorting or offset and it all should play along nicely.

What is your proposed solution?

Now the question is whether there is a way to use per_page, offset, and hierarchical params to have better control over the data fetched from the server in a way that we don't have to recreate the tree structure modeled on the server.

[luisherranz]

Another solution could be to make the _links.children items of the REST API embeddable:

{
  "id": 284,
  "_links": {
    "children": [
      {
        "embeddable": true, // <--
        "href": "https://test.site.org/wp-json/wp/v2/comments?parent=284"
      }
    ]
  }
}

Then, we could simply add _embed=true to the request and set the per_page value to the number of top-level comments.

The problem is that right now, embedding in the REST API is not recursive, so embedded comments won't embed their children.

I wonder if there's an easy solution to that problem that doesn't require adding a new recursive embedding capability to the REST API.

[gziolo]

One more thing that I discovered while testing enhancements to how we handle pagination is documented in #37196 (comment). By default, the REST API endpoint for comments returns 10 results which doesn't play along nicely with the UI controls that allow to set up to 100 top-level comments per page.

[gziolo]

I discussed this issue with @spacedmonkey and this is what I learned.

There is no simple way to get all comments in one request (even when bound to a smaller number of top-level items and their replies). You would need multiple requests, as it is an enforced performance optimization. You shouldn’t really be querying 10k items from MySQL in one request which is why requests are limited to 100 items at a time by design.

The most obvious solution that is already partially implemented is to get all comments using pagination (multiple requests), example request and construct the tree of comments in JavaScript:

https://make.wordpress.org/core/wp-json/wp/v2/comments?post=92323

The good news is that @wordpress/api-fetch implements fetchAllMiddleware middleware. You can activate it by passing per_page=-1 in the request.

The alternative would be to use the embed field set to children for top-level comments filtered with parent=0. Example:

https://make.wordpress.org/core/wp-json/wp/v2/comments?post=92323&_embed=children&parent=0

The challenge is that embeds aren't implemented for children so we would have to add some logic to:

https://github.com/WordPress/wordpress-develop/blob/954e9c153fe9d1b4f74d76bab9dd5175fd36cebc/src/wp-includes/rest-api/endpoints/class-wp-rest-comments-controller.php#L1199

Embeds are limited to 2 levels which as far as I understand is the restriction that we can't remove because of performance considerations. However, it might be a not that bad limitation because in practice you don't need to show all possible comments while editing a page template that contains the Comments Query Loop. So in the block editor, we could always show the button with the label "Show responses" if someone really needs that extended view to design their template.

[DAreRodz]

I’ve finally decided to do the second option @gziolo proposed in the previous comment: add embedded: true to children. That way, we could show replies to top-level comments with a single call.

This would be a short summary of benefits/drawbacks:

Benefits:

• Just a single call.
• Easy to handle pagination and offset.

Drawbacks:

• Up to ten replies.
• Only two levels.
• We would have to add a button in case a user wants to see more replies or more levels, etc.

Another concern I have is what would be the best UX in this case, which is something that is also being discussed in #37143. I’m talking about showing real comments vs placeholders. For me, it is not clear when to use real content or placeholders from the point of view of the user, I mean, what the user expects.