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.
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
Fix REST API auto-draft creation #26650
Fix REST API auto-draft creation #26650
Changes from 2 commits
c6d9adc
8e2c664
8692d47
66c0977
0c3e164
b67f498
942aac1
b1d5dfa
3201a14
47b1256
5ef0082
fb426c2
582f60d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
If we aren't comparing content as in the previous implementation, @david-szabo97 has been looking into re-writing the auto-drafts if the theme version changes - #26383
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.
yes, with this simpler syncing function, it's just a matter of comparing content here and updating the auto-draft if needed.
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.
I think the updating of the auto-draft was already being done prior to #26383. The purpose of #26383 was to stop that code from running whenever it was not necessary to run it (i.e. theme version hasn't changed.) This PR seems to go backwards in that respect.
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.
It's because
gutenberg_resolve_template
callsfile_get_contents
on every template in the theme whilegutenberg_find_template_post_and_parts
only called it on the one being requested. That means if you are loading 10 templates in the site editor thenfile_get_contents
will get called 100 times.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.
Where do you see the template part update was being done before? By my own reading of the code, it was not being done anywhere.
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.
I was talking about templates actually. They were being updated inside
gutenberg_find_template_post_and_parts
if the file contents had changed. I don't know about template parts.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.
From what I can tell, the following was the closest to this. It looks like it didn't so much update the auto-draft itself, but create a new one. The current code only gets to this point if a
publish
version isn't found, but if the contents of theauto-draft
don't match the contents of the file then a new auto-draft is created:gutenberg/lib/template-loader.php
Lines 174 to 178 in 422b21c
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.
Yes, I can see that difference and it's very problematic IMO. Creating a new auto-draft when the post_content changes is probably not the best approach to synchronization.
Let's leave this syncing behavior to its own dedicated PR since it's broken in master anyway (proliferation of auto-drafts).
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.
Agreed, and that sounds like a good plan!