-
Notifications
You must be signed in to change notification settings - Fork 83
Fix visibility setting not being saved when pre-selected value differs from public #2737
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
When editing an older post, the visibility defaults to 'local' based on post date, but this computed default was only displayed in the UI without being persisted. If the user saved without changing the selection, the visibility remained unset. This fix uses useEffect to automatically sync the computed default to the meta when it differs from the stored value (and isn't the implicit 'public' default).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a bug where the computed default visibility setting was displayed in the UI but not persisted to post meta when editing older posts (>1 month old). When such posts are opened in the editor, the visibility defaults to 'local', but this value was not being saved unless the user explicitly changed the selection.
Key changes:
- Added a
useEffecthook to automatically sync computed default visibility to post meta when appropriate - Extracted
defaultVisibilitycomputation to a constant for reuse - Ensures pre-selected non-public visibility values are persisted on editor load
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/editor-plugin/plugin.js | Added useEffect hook to sync computed default visibility to meta, imported useEffect from @wordpress/element, extracted defaultVisibility as a constant |
| .github/changelog/fix-visibility-meta-sync | Added changelog entry documenting the fix as a patch-level change |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The form values were hardcoded as strings ('public', 'quiet_public', 'local')
but the PHP constants use different values ('' for public). This caused
mismatches when comparing saved values with the checked() function.
Now uses the constants directly for form values to ensure consistency.
f7910ce to
7759fc9
Compare
There was a problem hiding this comment.
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 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
|
@pfefferle I've opened a new pull request, #2738, to work on those changes. Once the pull request is ready, I'll request review from you. |
jeherve
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Save the post without changing visibility
Check post meta: wp post meta get <post_id> activitypub_content_visibility should return 'local'
This doesn't seem to work in my tests. When editing an old post where the post meta was not present, no new post meta is added.
We use undefined check because empty string '' means 'public' was explicitly saved.
I'm not sure I understand this part. If it was never explicitly saved, there would be no post meta, no?
|
If you open an old post, it should default to "local" (if it was not published before!!!). If you then update the post and save it, it should store "local" as |
|
@jeherve it is also no longer working for me :( |
WordPress returns '' (empty string) for unset post meta, not just undefined. Changed the check from strict undefined comparison to falsy check to handle both cases.
ACTIVITYPUB_CONTENT_VISIBILITY_PUBLIC is defined as '' in PHP. Updated JS to use empty string for public visibility value and added visibility constants for consistency.
PHP handles both 'public' and '' (empty string) as public visibility, so keep using 'public' in JS for RadioControl compatibility.
|
Now it works for me again. |
jeherve
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This tests well for me now on a live a site. Let's ship it! 🚢
Fixes #2716
Proposed changes:
useEffecthook to sync computed default visibility to meta when there's no stored value and the default isn't 'public'. This ensures that when editing old posts (>1 month), the 'local' default gets persisted even if the user doesn't change the selection.ACTIVITYPUB_CONTENT_VISIBILITY_PUBLIC, etc.) to ensure consistency between displayed and saved values.Other information:
Testing instructions:
wp post meta get <post_id> activitypub_content_visibilityshould return 'local'Changelog entry
Changelog Entry Details
Significance
Type
Message
Fixed visibility setting not being saved correctly in block editor and classic editor.