Skip to content

Comment REST API route: add support for filtering by multiple statuses. - #9870

Open
adamsilverstein wants to merge 33 commits into
WordPress:trunkfrom
adamsilverstein:add/comment-rest-api-types-support
Open

adamsilverstein wants to merge 33 commits into
WordPress:trunkfrom
adamsilverstein:add/comment-rest-api-types-support

Conversation

@adamsilverstein

@adamsilverstein adamsilverstein commented Sep 14, 2025 •

Copy link
Copy Markdown
Member

Add support for filtering the comments collection endpoint by multiple statuses, eg. /wp/v2/comments?status=approve,hold or status[]=approve&status[]=hold. WP_Comment_Query already accepts an array of statuses, so this brings the REST endpoint in line with what the underlying query supports. Single string values continue to work as before.

Originally extracted from the Notes work, see WordPress/gutenberg#71271 (WordPress/gutenberg#71271 (comment))

Changes:

  • Define the status collection param as an array of strings, each sanitized with sanitize_key, following the pattern of the posts controller.
  • Update the status check in get_items_permissions_check to handle array values. Behavior is unchanged: unauthenticated requests can only use the default approve status, anything else requires edit_posts.
  • Add tests covering single, multiple, comma separated and all status queries, sanitization of the param, and the authorization boundary.
  • Regenerate the QUnit REST schema fixture.

Note: I did not add an enum of allowed values here - WP_Comment_Query explicitly supports custom comment statuses and the endpoint currently accepts any string (eg. spam and trash work today for authorized users), so an enum limited to the registered statuses would be a back compat break.

Testing instructions

Test in WordPress Playground

  1. On a test site, create some comments and mark a few as approved and a few as pending (the Comments screen works for this).
  2. As an authorized user (I used an application password), request multiple statuses and verify comments from both lists are returned:
    curl --user admin:APP_PASSWORD "https://example.test/wp-json/wp/v2/comments?status=approve,hold"
  3. Request a single status, eg. ?status=hold, and verify only pending comments are returned.
  4. Without authentication, verify ?status=approve still returns approved comments and ?status=approve,hold returns a rest_forbidden_param error.
  5. Run the unit tests: npm run test:php -- --filter WP_Test_REST_Comments_Controller

Trac ticket: https://core.trac.wordpress.org/ticket/63982


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@github-actions

Copy link
Copy Markdown

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@adamsilverstein adamsilverstein changed the title Add/comment rest api types support Comment REST API route: add support for status as array Sep 15, 2025
@adamsilverstein
adamsilverstein marked this pull request as ready for review September 15, 2025 05:18
@github-actions

github-actions Bot commented Sep 15, 2025 •

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props adamsilverstein, westonruter, swissspidy, timothyblynjacobs, mukesh27, wildworks.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@adamsilverstein adamsilverstein changed the title Comment REST API route: add support for status as array Comment REST API route: add support multiple statuses Sep 15, 2025
Comment thread tests/phpunit/tests/rest-api/rest-comments-controller.php Outdated
@t-hamano

Copy link
Copy Markdown
Contributor

We might not need to extend the REST API itself. See WordPress/gutenberg#71271 (comment)

Co-authored-by: Pascal Birchler <pascal.birchler@gmail.com>
@adamsilverstein

adamsilverstein commented Sep 15, 2025 •

Copy link
Copy Markdown
Member Author

We might not need to extend the REST API itself. See WordPress/gutenberg#71271 (comment)

Good point; that said its probably worth adding as its a simple change and reflects what the underlying core comments class already supports (so it feels like a bug or oversight that it isn't supported already). While we don't need it yet, I could easily see us or a plugin wanting this.

Comment thread src/wp-includes/rest-api/endpoints/class-wp-rest-comments-controller.php Outdated
Comment thread src/wp-includes/rest-api/endpoints/class-wp-rest-comments-controller.php Outdated
Comment thread src/wp-includes/rest-api/endpoints/class-wp-rest-comments-controller.php Outdated
…roller.php

Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
Comment thread tests/phpunit/tests/rest-api/rest-comments-controller.php Outdated
Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
@adamsilverstein adamsilverstein changed the title Comment REST API route: add support multiple statuses [WIP] Comment REST API route: add support multiple statuses Sep 18, 2025
With the status collection param now an array, the sanitized value never
string-matches 'approve', so unauthenticated requests using the default
status were rejected. Compare against the parsed list instead, define the
schema as an array of strings per review feedback, and regenerate the
QUnit fixture to match.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

@westonruter westonruter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to add this test: #9870 (comment)

Otherwise, pre-approving.

The unauthenticated 401 boundary was only covered for the array form of
the status parameter. Since the permission check now relies on the
sanitize callback having parsed the raw value, the comma-separated form
needs its own case so it cannot regress independently. Convert the test
to a data provider covering both forms.
Copilot AI review requested due to automatic review settings July 23, 2026 22:52
array_filter() without a callback drops all falsy strings, including
'0'. WP_Comment_Query treats a literal '0' status as a direct
comment_approved match (how held comments are stored), so ?status=0 is
a working query today and must survive sanitization. Filter only empty
strings instead.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings July 23, 2026 22:59

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comment thread src/wp-includes/rest-api/endpoints/class-wp-rest-comments-controller.php Outdated
adamsilverstein and others added 3 commits September 28, 2026 14:50
Resolve the conflict in the comments controller, where trunk added
check_target_post_permission() at the same spot the branch added
sanitize_comment_statuses(). Both methods are kept.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GqepyjmFSryBt88j6f6ubY
Trunk is now 7.2-alpha, so the new method ships in 7.2.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GqepyjmFSryBt88j6f6ubY
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DxbbMKz3sa741SjHuE1ZLS
@t-hamano

t-hamano commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

@adamsilverstein, this PR looks good to merge, but do you have any concerns?

The `status` collection param is now typed as an array, but its default
was still the string 'approve'. The permission checks compare against
`array( 'approve' )`, so they only matched because sanitization coerced
the default; declaring it as an array keeps the schema consistent with
the param type and removes that implicit dependency. The generated
QUnit schema fixture is updated to match.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B6M4x5WcvJVMgMqxH6qBKv
@adamsilverstein

Copy link
Copy Markdown
Member Author

@adamsilverstein, this PR looks good to merge, but do you have any concerns?

I think its ready, sweeping for unaddressed feedback.

@adamsilverstein

Copy link
Copy Markdown
Member Author

@adamsilverstein, this PR looks good to merge, but do you have any concerns?

Looks good to me!

@t-hamano t-hamano left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good from my end. It might be worth noting the definition change under miscellaneous dev notes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants