Skip to content

Add failing precondition tests: unspam/untrash must not demote a non-spam/trash comment - #1566

Draft
jkmassel wants to merge 1 commit into
comments-unspam-untrashfrom
comments-unspam-untrash-precondition-tests
Draft

Add failing precondition tests: unspam/untrash must not demote a non-spam/trash comment#1566
jkmassel wants to merge 1 commit into
comments-unspam-untrashfrom
comments-unspam-untrash-precondition-tests

Conversation

@jkmassel

Copy link
Copy Markdown
Contributor

Description

Follow-up to #1564, stacked on its branch. unspam()/untrash() blindly POST status=unspam|untrash. WordPress core (wp_unspam_comment/wp_untrash_comment) restores the status saved in _wp_trash_meta_status, and defaults to hold when that meta is absent — so calling either endpoint on a comment that is not currently spam/trash returns HTTP 200 and silently demotes an approved comment to pending (verified against WordPress 6.8.1), rather than the HTTP 500 the endpoint's doc comment claims. The status-only 500 branch in WP_REST_Comments_Controller::update_item is in fact unreachable for a real request, because prepare_item_for_database always populates comment_author_IP — which also means the call overwrites the comment's stored author IP with the caller's.

These are aspirational tests that currently fail — they encode the contract we want (reject the call, surface an error, leave the comment untouched) so #1564 can be adjusted to make them pass. Opened as a draft because CI will be red until a precondition guard lands.

Changes

  • Add unspam_on_non_spam_comment_should_error_without_demoting and untrash_on_non_trashed_comment_should_error_without_demoting to wp_api_integration_tests/tests/test_comments_mut.rs.
  • Each asserts the comment is left Approved (no silent demotion) and that the call returns an error rather than a bogus success.

Test plan

  • Verified locally against the WordPress 6.8.1 test server: both fail today with left: Hold, right: Approved.
  • Both pass once a client-side precondition guard is added to unspam()/untrash() (refuse when the comment isn't currently spam/trash). Realistic options: fetch-then-act inside the method, accept an expected-current-status argument, or enforce at a higher layer.
  • If a silent no-op success is preferred over an error, drop the assert!(result.is_err(), …) line — the == Approved assertion alone still pins "no demotion".

Changelog

  • N/A — test-only, no library behavior change (the fix that makes these pass will carry the changelog entry).

…omments

unspam()/untrash() blindly POST status=unspam|untrash. WordPress core (wp_unspam_comment/wp_untrash_comment) restores the status saved in _wp_trash_meta_status and defaults to hold when that meta is absent — so calling either endpoint on a comment that is not currently spam/trash returns HTTP 200 and silently demotes an approved comment to pending (verified against WordPress 6.8.1), rather than the HTTP 500 the endpoint's doc comment claims.

These two integration tests encode the desired contract — refuse the call and surface an error, leaving the comment untouched — and fail against the current implementation. Stacked on #1564.
@jkmassel jkmassel self-assigned this Aug 13, 2026
@wpmobilebot

Copy link
Copy Markdown
Collaborator

⚠️ CHANGELOG.md was not updated in this PR.

Every PR should add an entry under the ## [Unreleased] section of CHANGELOG.md describing the change for our users. The format follows Keep a Changelog 1.0.0 — use one of:

  • ### Added — for new features
  • ### Changed — for changes in existing functionality (prefix **BREAKING:** if breaking)
  • ### Deprecated — for soon-to-be-removed features
  • ### Removed — for now-removed features
  • ### Fixed — for any bug fixes
  • ### Security — for vulnerability fixes

If the change genuinely has no user-visible impact (e.g. CI-only tweaks, internal refactors), add a short entry under ### Changed noting that.

@wpmobilebot

Copy link
Copy Markdown
Collaborator

XCFramework Build

This PR's XCFramework is available for testing. Add to your Package.swift:

.package(url: "https://github.com/automattic/wordpress-rs", branch: "pr-build/1566")

Built from 3dc7526

@crazytonyli

Copy link
Copy Markdown
Contributor

I wonder if this should be fixed on the core side? Given the library just implements the endpoints. In practice, the client should only perform these actions on spam-ed and trash-ed comments, like wp-admin.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants