Repository navigation
Force visibility to password when editing a post in quick edit. - #11180
Hug0-Drelon wants to merge 3 commits into
Conversation
|
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: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. Props Bot commandsYou can trigger Props Bot actions by commenting on this PR:
|
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. |
There was a problem hiding this comment.
Pull request overview
Ensures Quick Edit requests correctly treat posts with a post_password as “password-protected” visibility so that edit_post() applies the existing visibility handling (notably unsetting sticky) for password-protected posts.
Changes:
- In
edit_post(), infervisibility = 'password'whenpost_passwordis present in the incoming post data.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Password visibility. | ||
| if ( ! empty( $post_data['post_password'] ) ) { | ||
| $post_data['visibility'] = 'password'; | ||
| } |
| // Password visibility. | ||
| if ( ! empty( $post_data['post_password'] ) ) { | ||
| $post_data['visibility'] = 'password'; | ||
| } |
|
Hi @Hug0-Drelon, do you have the bandwidth to address the reviews and add some test coverage for this PR? |
|
The ticket carries It reproduces Quick Edit's actual payload: /**
* Ensures that giving a sticky post a password through edit_post() unsticks it,
* even when the caller never sends an explicit `visibility` value.
*
* The block editor sends `visibility`, so `edit_post()` reaches the
* `case 'password'` branch and drops the `sticky` flag. Quick Edit never sends
* it, so a post could end up both sticky and password protected, a combination
* the editor itself forbids.
*
* @ticket 64810
*
* @covers ::edit_post
*/
public function test_edit_post_unsticks_a_post_when_a_password_is_set_without_visibility() {
wp_set_current_user( self::$admin_id );
$post_id = self::factory()->post->create(
array(
'post_status' => 'publish',
'post_author' => self::$admin_id,
)
);
stick_post( $post_id );
$this->assertTrue( is_sticky( $post_id ), 'The post was not sticky to begin with: check test setup.' );
// Mirrors Quick Edit, which posts `sticky` but no `visibility`.
edit_post(
array(
'post_ID' => $post_id,
'post_title' => 'Protected and sticky',
'post_status' => 'publish',
'post_password' => 'secret',
'sticky' => 'sticky',
)
);
$this->assertSame(
'secret',
get_post( $post_id )->post_password,
'The password was not saved: check test setup.'
);
$this->assertFalse(
is_sticky( $post_id ),
'A password protected post was left sticky when no explicit visibility was sent.'
);
}It goes in Verified against a local build: Whole file still green with the patch applied: 52 tests, 118 assertions, 0 failures (6 warnings, all pre-existing PHPUnit 10 deprecation notices in Also attached to the ticket as |
|
Rather than leave the test as a paste, I've opened it as a commit against this branch: Hug0-Drelon#1. Merging it there brings the test into this PR so the fix and its coverage land as one changeset, and nothing else on your side changes. The fix stays yours; I only wrote the test. |
|
@Hug0-Drelon this is the unit test @wildworks asked for in comment:6 when they marked #64810 It reproduces what Quick Edit actually posts, Ready to cherry-pick onto this branch as-is: diff --git a/tests/phpunit/tests/admin/includesPost.php b/tests/phpunit/tests/admin/includesPost.php
index d9d39d8da727d..dac5c8bc86dfa 100644
--- a/tests/phpunit/tests/admin/includesPost.php
+++ b/tests/phpunit/tests/admin/includesPost.php
@@ -1371,4 +1371,52 @@ public function test_user_get_refreshed_metabox_nonce() {
$this->assertNotEmpty( $response['wp-refresh-metabox-loader-nonces']['replace']['_wpnonce'] );
$this->assertNotEmpty( $response['wp-refresh-metabox-loader-nonces']['replace']['metabox_loader_nonce'] );
}
+
+ /**
+ * Ensures that giving a sticky post a password through edit_post() unsticks it,
+ * even when the caller never sends an explicit `visibility` value.
+ *
+ * The block editor sends `visibility`, so `edit_post()` reaches the
+ * `case 'password'` branch and drops the `sticky` flag. Quick Edit never sends
+ * it, so a post could end up both sticky and password protected, a combination
+ * the editor itself forbids.
+ *
+ * @ticket 64810
+ *
+ * @covers ::edit_post
+ */
+ public function test_edit_post_unsticks_a_post_when_a_password_is_set_without_visibility() {
+ wp_set_current_user( self::$admin_id );
+
+ $post_id = self::factory()->post->create(
+ array(
+ 'post_status' => 'publish',
+ 'post_author' => self::$admin_id,
+ )
+ );
+
+ stick_post( $post_id );
+ $this->assertTrue( is_sticky( $post_id ), 'The post was not sticky to begin with: check test setup.' );
+
+ // Mirrors Quick Edit, which posts `sticky` but no `visibility`.
+ edit_post(
+ array(
+ 'post_ID' => $post_id,
+ 'post_title' => 'Protected and sticky',
+ 'post_status' => 'publish',
+ 'post_password' => 'secret',
+ 'sticky' => 'sticky',
+ )
+ );
+
+ $this->assertSame(
+ 'secret',
+ get_post( $post_id )->post_password,
+ 'The password was not saved: check test setup.'
+ );
+ $this->assertFalse(
+ is_sticky( $post_id ),
+ 'A password protected post was left sticky when no explicit visibility was sent.'
+ );
+ }
}It is yours to take, and it belongs in this changeset rather than a separate one, since it only passes with the fix in place. Ticket credit sorts itself out through the props bot either way. |
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.
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.
|
I completely agree with @wildworks' assessment in https://core.trac.wordpress.org/ticket/64810#comment:29. Allowing a post to be both password protected and sticky across all code paths is the right architectural direction. Historically, this restriction seems to have originated merely as an artifact of the Classic Editor's Publish metabox UI (where the sticky checkbox was nested under the "Public" radio button), which was then ported into From a product and technical standpoint:
I have implemented and tested this unified approach on branch
Ready to open this as an alternative PR or adapt it here if the consensus is to move forward with allowing sticky password-protected posts. |
Visibility is never set for password when using quick edit, thus
edit_post()cannot unset sticky status in https://github.com/WordPress/wordpress-develop/blob/6.9.1/src/wp-admin/includes/post.php#L308-L322.Looking at it, I thought that
wp_ajax_inline_save()would handle that like it does forprivatestatus. But it might be dead code around https://github.com/WordPress/wordpress-develop/blob/6.9.1/src/wp-admin/includes/ajax-actions.php#L2117-L2122.Removing it doesn't affect the feature (remove sticky status when a post is private).
Trac ticket: https://core.trac.wordpress.org/ticket/64810
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.