Skip to content

REST API: Sanitize comment statuses in a single pass. - #14752

Open
adamsilverstein wants to merge 1 commit into
WordPress:trunkfrom
adamsilverstein:fix/comment-statuses-single-pass
Open

adamsilverstein wants to merge 1 commit into
WordPress:trunkfrom
adamsilverstein:fix/comment-statuses-single-pass

Conversation

@adamsilverstein

@adamsilverstein adamsilverstein commented Oct 22, 2026 •

Copy link
Copy Markdown
Member

Follow up to #9870 (comment) - thanks @dmsnell for the suggestion.

Claude made the change @dmsnell sketched out, summary below:

sanitize_comment_statuses() made five passes over the input (wp_parse_list, array_map, array_unique, array_filter, array_values), building a new array on each one. This replaces them with a single loop that sanitizes, skips empty values, and dedupes as it goes.

Behavior should be identical. The loop still keeps a literal '0', since held comments store comment_approved = '0', and still drops non-scalar array entries the way wp_parse_list() did. Non-array input still goes through wp_parse_list(), so comma and space separated strings work the same as before.

The data provider gets three new cases: values that only collide after sanitizing (Approve, APPROVE), empty values inside an array, and non-scalar entries.

Testing

  • Run npm run test:php -- --filter test_sanitize_comment_statuses (not run locally yet, leaning on CI for the PHPUnit run).
  • PHPCS passes on both changed files.
  • The old and new implementations were run side by side in a standalone PHP script over 15 inputs (the provider cases plus booleans, ints, padded strings and an empty array) and returned the same result for every one.

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

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5.5
Used for: Claude Code wrote copied the code, wrote tests and this description, starting from the reviewer's snippet. I reviewed.


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.

🤖 Generated with Claude Code

https://claude.ai/code/session_015KyhSZWbcaixA5gHomFjXT

Commit message

REST API: Sanitize comment statuses in a single pass.

Replace the chain of array functions in `WP_REST_Comments_Controller::sanitize_comment_statuses()` with one loop to avoid building intermediate arrays. Behavior is unchanged.

Follow-up to [64494].

Props dmsnell, westonruter.
See #63982.

Replace the chained array_map/array_unique/array_filter/array_values calls in sanitize_comment_statuses() with one loop, avoiding the intermediate arrays. Behavior is unchanged; add test cases covering duplicates that only collide after sanitizing, empty values inside an array, and non-scalar values.

Follow-up to a review suggestion on WordPress#9870.

See #63982.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_015KyhSZWbcaixA5gHomFjXT
@github-actions

github-actions Bot commented Oct 22, 2026 •

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.

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

Props adamsilverstein, westonruter, dmsnell.

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


Props Bot commands

You can trigger Props Bot actions by commenting on this PR:

  • @propsbot add <username> to give props to a GitHub user. Requires write access.
  • @propsbot remove <username> to leave a user out of the props. Requires write access.
  • @propsbot refresh to refresh this list. Adding the props-bot label works too.

@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

  • 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

Copy link
Copy Markdown
Member Author

Code-wise this looks good to me.

@dmsnell

dmsnell commented Oct 22, 2026

Copy link
Copy Markdown
Member

Claude Code wrote the code

not that I really care, but it appears that Claude copied my code verbatim, which is more like “Claude Code copy/pasted the code” 🤷‍♂️

@adamsilverstein

Copy link
Copy Markdown
Member Author

Claude Code wrote the code

not that I really care, but it appears that Claude copied my code verbatim, which is more like “Claude Code copy/pasted the code” 🤷‍♂️

Description updated.

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.

3 participants