[wp-trac] [WordPress Trac] #57223: Comments Controller : Should max/total be pre- or post- filtering? It is pre-. Reconsider?

[WordPress Trac]

#57223: Comments Controller : Should max/total be pre- or post- filtering? It is
pre-. Reconsider?
-------------------------+-----------------------------
 Reporter:  Starbuck     |      Owner:  (none)
     Type:  enhancement  |     Status:  new
 Priority:  normal       |  Milestone:  Awaiting Review
Component:  REST API     |    Version:  trunk
 Severity:  normal       |   Keywords:
  Focuses:               |
-------------------------+-----------------------------
 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
 [https://wordpress.slack.com/archives/C02RQC26G/p1669387892298079?thread_ts=1669320024.362229&cid=C02RQC26G
 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
 [https://developer.wordpress.org/reference/hooks/rest_comment_query/
 rest_comment_query filter], `total_after_filter`. Obtain and remove this
 arg from $prepared_args
 [https://core.trac.wordpress.org/browser/tags/6.1.1/src/wp-includes/rest-
 api/endpoints/class-wp-rest-comments-controller.php?rev=54846#L275 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.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/57223>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform