Multi-entity saving: Allow publishing a post while not saving changes to non-post entities #37383
Conversation
This file contains 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
stacimc
added
[Type] Bug
stacimc
force-pushed
the
fix/publish-post-with-nonpostEntities-deselected
branch
from
9b901e9 to
9538d1c
Compare
|
Size Change: +969 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
stacimc
force-pushed
the
fix/publish-post-with-nonpostEntities-deselected
branch
from
f870bd4 to
26e1095
Compare
ramonjd
reviewed
Dec 15, 2021
|
Nice work @stacimc This is working well for me so far and an improvement on the current experience.
✅ When I deselect the site entity the post saves, but the site logo is not set (checked in the Customizer). |
|
Thank you @stacimc Staci! Testing the PR Gutenberg Build. 1- Made a new post. Before: After - This PR: 4- Clicked Save. 1- Creating a new post. One is not stuck in the Publish process when a Site item such as the Site Logo block is unchecked. |
paaljoachim
added
the
Backport to WP 6.7 Beta/RC
|
I added the Backport label as I assume this PR will be backported to WP 5.9. (If it is merged within short period of time.) |
|
Nice! 🎉 I'll test in a moment. Chipping in my (unfinished) e2e test: #37408 |
|
Adding that we should also test the Site Editor after modifying the multi-entity saving logic.
Seems to work well! 🎉 |
Co-authored-by: Ramon <ramonjd@users.noreply.github.com>
|
Thanks for all the reviews! Updated to include the typo fix :)
@ockham I tested making changes to multiple entities and saving them individually in the site editor (eg: modify several template parts and the Site Logo, test saving everything but the Site Logo), and this is working for me. Anything else you were thinking of to test here? I would like to get the e2e tests in #37408 in with this PR, but I took a look to see if I could help with the false positive and I'm afraid I'm stumped. Are you comfortable merging this without the tests for now? |
Not really -- that sounds pretty good :)
Yeah, let's merge this as-is -- I'd rather not have it blocked by the tests, since it could take a while to sort those out, and I think we want to backport this PR to Core sooner rather than later. |
tellthemachines
removed
the
Backport to WP 6.7 Beta/RC
tellthemachines
pushed a commit
that referenced
this pull request
Dec 21, 2021
… to non-post entities (#37383) * Check for existence of setEntititesSavedStatesCallback prop * Clean up props destructuring * Update packages/editor/src/components/post-publish-button/index.js Co-authored-by: Ramon <ramonjd@users.noreply.github.com> Co-authored-by: Ramon <ramonjd@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 #36096
Description
This PR provides a quick fix for a bug which makes it impossible to publish a new post that makes changes to non-post entities (like a template part or the Site Logo), but does not save them. Eg:
Save, thenPublish.This PR resolves the error by checking for the existence of the
setEntitiesSavedStatesCallbackprop before attempting to open the pre-publish panel.Explanation of the bug
We were getting an error at this line. The problem is that when the Publish button is clicked, we check
hasNonPostEntityChangesto see if there are changes to non-post entities, and then perform some logic to open the pre-publish panel that allows you to check what entities you'd like saved (see screenshot). But if those entities aren't saved (user deselected them), they will still be dirty and the logic will attempt to run again when you hitPublishthe second time.When the
Publishbutton is rendered the second time, it's rendered by thePostPublishPaneland thesetEntitiesSavedStatesCallbackprop is not passed. It's the absence of this prop (which is used to open the pre-publish panel) that causes the error. Notably it doesn't make sense to pass it here, since at this point we don't expect to open that panel.How has this been tested?
It is important to always test with a new post, publishing for the first time.
Verify that you can reproduce the bug on trunk. Then:
Test that you can publish while deselecting non-post Entities
Publishand deselectSite Logo. HitSave, thenPublishagain.Updatebutton can be clicked), and shows the option to save Site Logo.Test that you can publish when you do save non-post Entities
Publishand do not deselect anything. HitSavethenPublishScreenshots
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).