Comments Controller : Should max/total be pre- or post- filtering? It is pre-. Reconsider?

[Starbuck]

Ref get_items for the REST Comments Controller: https://core.trac.wordpress.org/browser/tags/6.1.1/src/wp-includes/rest-api/endpoints/class-wp-rest-comments-controller.php?rev=54846#L186

Skip down to line 277. The query is run. Read permissions are checked, and at 284 protected comments are removed from the result.

Now at 291, $total_comments and $max_pages come from the original query result, not the whittled-down list. So there might be 100 original results, maybe 10 that this user can see, but the $response still shows 100.

The question here is, should the returned value be a count of the records/pages that are potentially available to all, or the count that's available to this user.

I'm thinking the latter. Consider #57221 : With this scenario the client can get an invalid TotalPages response and then come back to request the last page, which isn't actually a valid page number. Many of us have seen anomalies like this before and it can be frustrating. Some developers will setup a loop from 1 to count(x), and error out with fewer records. There's a lesson to be learned there, but perhaps we can improve it.

A comments response includes headers with totals and with links to previous/next pages - all of these values can be invalid.

As ​noted by @spacedmonkey, it would be a breaking change to return post-filter totals where existing apps may expect pre-filter totals. For example, it's legitimate to display "X available comments of Y total". So I'll not request a change to existing behaviour, but options to modify it.

The Suggestion

Document a new argument that can be passed into the ​rest_comment_query filter, total_after_filter. Obtain and remove this arg from $prepared_args here at line 275 to avoid passing to WP_Comment_Query.

If that argument is present, use the post-filter numbers for totals at line 291.

Whether or not that argument is present, add two new response headers after line 307, X-WP-TotalAvailable and X-WP-TotalAvailablePages, which may have values different from the others.

If the above is valid, I'd suggest a similar enhancement to return additional or different prev/next header links, but this would not be as easy and I'll defer discussion for later (or maybe another ticket).

And now, the proverbial can of worms...

Comments are not the only data type subject to this scenario, and it's not unique to the REST API either. I have not yet done a deep enquiry to see if pre- or post-filter totals are returned by various interfaces. I am curious if anyone believes there is justification to expand the scope of this to new tickets for other classes and APIs.

[Starbuck]

Intentionally left out of OP, not sure if this should be in a separate ticket yet: At line 301 a similar situation exists. Let's preface this all with "it looks to me like ... and I might be wrong ..."

If the original query returns no results, and the request is looking for a specific page, the query is re-run with no pagination limits, and there is no post-query check for permissions on the comments. So the total number of comments and pages are based on the entire set available for the current post, and not on the records available to the current user ... and all comments are returned without permissions filtering, simply because the query was wrong, perhaps mis-led by the data that is being provided here. This seems like a real bug. Untested, not verified, sorry.