Posts: Unstick a post given a password through Quick Edit. - #13246
Posts: Unstick a post given a password through Quick Edit.#13246wppoland wants to merge 3 commits into
Conversation
Quick Edit posts `sticky` but never posts `visibility`, so `edit_post()` never reached the `case 'password'` branch that drops the sticky flag. A post could end up both sticky and password protected, a combination the block editor itself forbids. Set `visibility` to `password` whenever a post password is present, so the existing branch runs whatever the caller sent. Original patch by Hug0-Drelon in PR WordPress#11180. See #64810.
…value. Reproduces the Quick Edit payload: `sticky` is posted, `visibility` is not. Fails on trunk, where the post keeps both the sticky flag and the new password, and passes with the preceding change. See #64810.
|
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 Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe 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
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
Hug0-Drelon
left a comment
There was a problem hiding this comment.
Should we take the opportunity to improve code coverage for https://github.com/WordPress/wordpress-develop/pull/13246/changes#diff-7c58dcea3675e604ab0caa9263522686924e3dd881d5a36b94865dacfbca129bR309-R323?
i.e. testing with $post_data['visibility'] set to 'public'|'password'|'private'?
Otherwise, thanks for the proposed tests!
Adds the `public`, `password` and `private` cases of the visibility switch, which had no coverage before, and pins the precedence between an inferred visibility and a contradicting one sent by the caller. That last one is a behaviour change worth naming: on trunk, sending `visibility` as `public` alongside a non-empty `post_password` cleared the password. Inferring the visibility from the password applies last, so the password now wins instead. The editors never send that pair, since choosing Public clears the password field, but `edit_post()` is also reachable from bulk edit and Quick Edit. See #64810.
|
Good call, done in 5540b57. Three characterisation tests for the switch, plus a fourth that turned out to matter more than I expected.
The fourth oneWhile writing those I noticed the change alters behaviour for one input nobody has mentioned on the ticket. Inferring the visibility from the password happens after the caller's own value is read, so the two can now contradict each other and the inferred one wins. On trunk,
If that precedence is wrong, the fix is to infer only when the caller sent no VerificationFour new tests, negative control run rather than assumed. Against trunk's Whole file: 56 tests, 125 assertions, 0 failures. The 6 warnings are the pre-existing |
Trac ticket: https://core.trac.wordpress.org/ticket/64810
Opened at @audrasjb's request in comment:15: the change from #11180 plus the unit test that was only sitting in a comment there, so the ticket can lose
needs-unit-testsand be committed as one changeset.The change
@Hug0-Drelon's patch from #11180, unmodified, and committed under their authorship.Quick Edit posts
stickybut never postsvisibility.edit_post()only unsetsstickyinsidecase 'password'of thevisibilityswitch, so that branch was never reached and a post could end up both sticky and password protected, a combination the block editor itself forbids.Setting
visibilitytopasswordwhenever a post password is present makes the existing branch run whatever the caller sent.The test
test_edit_post_unsticks_a_post_when_a_password_is_set_without_visibility()reproduces the Quick Edit payload:stickyis posted,visibilityis not.Verified both directions locally, PHP 8.3 against a single site:
OK (1 test, 3 assertions)Failed asserting that true is false. A password protected post was left sticky when no explicit visibility was sent.The rest of the file is unaffected:
tests/phpunit/tests/admin/includesPost.phpruns 52 tests, 118 assertions, 0 failures. The 6 warnings are the pre-existingE_DEPRECATEDexpectation notices PHPUnit 9.6 emits across that file.phpcson both touched files: 0 errors. The single warning insrc/wp-admin/includes/post.phpis at line 901 and predates this branch.Credit
The fix is
@Hug0-Drelon's work and the props on #11180 already list hugod, wildworks, abcd95, motylanogha. If this lands instead of #11180, that props line should carry over.The open question this PR does not settle
The ticket still carries
2nd-opinion, and this PR is not a vote on it. In comment:13 @wildworks argued the opposite direction: that password protected and sticky should be allowed together in all cases, and that #11180 removes something Quick Edit could previously do.That is a real disagreement about intended behaviour, not about the code. This PR exists because @audrasjb asked for the change and the test in one place; if the ticket resolves the other way, the same test file is where the inverse expectation would go.