Newsletter: fix the editor warning about unsaved changes after changing post settings - #50998
Open
arcangelini wants to merge 7 commits into
Open
Newsletter: fix the editor warning about unsaved changes after changing post settings#50998arcangelini wants to merge 7 commits into
arcangelini wants to merge 7 commits into
Conversation
Contributor
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
github-actions
Bot
added
[Block] Subscriptions
[Plugin] Jetpack
Contributor
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Jetpack plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Code Coverage SummaryCoverage changed in 2 files.
1 file is newly checked for coverage.
|
The editor's save cycle also completes when preSavePost rejects and no request runs, and the outgoing payload is snapshotted before the request starts, so neither the success check nor the sent-meta snapshot could be trusted to decide whether a staged meta value was really saved. Both paths could revert a pending edit.
The staged copy losing a key was the only reason the hook restored missing keys, and that restore cancelled a deletion made while a save was in flight. Writing 0 clears the tier for real -- omitting the key from the REST payload never did -- and keeps the staged key set complete, so the hook can leave every non-CRDT difference alone.
Nothing but a save response ever moves `_crdt_document`: peers never sync it, and only a non-autosave save runs the prePersist that mints a new one. Watching that value replaces the save-transition tracking, the autosave and save-error guards, and the ref, and it can no longer write an undefined snapshot back. Also moves the hook out of memberships/, since nothing in it is specific to newsletters, and trims the tests to the three branches that matter.
Disabling the options renders them with a native `disabled` and no tabindex, so the whole group drops out of the tab order. Fall back to the current value as text, the way the access panel already does.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes NL-797
Alternative to #50924. Same issue, fixed one layer down so it covers both panels NL-797 reports.
Proposed changes
useClearPhantomMetaDirt. Once a save comes back, it copies the collaboration snapshot the server returned over the stale one the editor is still holding, and the post goes back to "Saved". It touches nothing else, and only runs when a save request actually went out and succeeded.saveEditedEntityRecordcall from the Newsletter toggle so the setting rides the normal editor save. That call also silently converted a brand-new post into a draft, because core forcesstatus: 'draft'on auto-drafts during save.disabledprop, which was set onToggleGroupControlwhere it does nothing instead of on its options.0to the post's newsletter tier instead of deleting the tier field. Deleting it only ever worked inside the browser — the field was left out of the save entirely, so the old tier stayed on the post. A post switched to "Anyone subscribed" could quietly still be carrying a paid tier. Writing0clears it for real. Nothing looks different in the editor:0and "no tier" are already treated the same by everything that reads it.Root cause
Collaboration serializes a fresh
_crdt_documentinto the save payload after the meta edit was staged (prePersistPostType). core-data compares the wholemetaobject at once, so the staged copy matches neither the response record nor the sent edits, and the edits reducer keeps it. Each further save mints another snapshot, so it never reconciles.The fix stays deliberately narrow. The hook only ever puts back
_crdt_document, a field nothing in the editor can edit, so restoring the server's copy cannot throw away something a writer typed. Every other difference is left alone: the save payload is captured before the request goes out and is never visible afterwards, so a value the server changed looks exactly like an edit that was never sent. The one case that used to need special handling here —useSetAccessdeleting the tier field — is fixed at the source instead.This belongs upstream in Gutenberg — the comparison at
core-data/src/reducer.jscan never match onceprePersistrewritesmeta. The hook is a no-op when collaboration is off and a no-op again once that lands.Testing instructions
Needs real-time collaboration, which is on by default on WordPress.com. Self-hosted: install Gutenberg and enable Settings → Writing → Collaboration.
Reproduce first, on trunk:
Then on this branch, both flows should return to "Saved" after pressing Save, and navigating away should be silent.
Also confirm nothing unrelated is swallowed:
And that a new post still behaves:
And the tier change, which needs at least two paid newsletter tiers:
Does this pull request change what data or activity we track or use?
No.