Skip to content
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

Run NetworkSiteConnection::update_syndicated on correct hook when using Gutenberg #518

Merged

Conversation

lakrisgubben
Copy link
Contributor

Description of the Change

Add a check to the NetworkSiteConnection::update_syndicated method to see if the current post is saved through Gutenberg. If so add a new action that runs the same method but on the rest_after_insert_{$post_type} hook to fix an issue where updates to terms/meta etc where not beeing syndicated.

For a deeper discussion about what problems this is trying to solve see issue #464

Alternate Designs

Had some back-and-forth on the #464 issue, but this solution seemed like the most "elegant" to a problem without any ideal solutions.

Benefits

Terms/meta etc. will be syndicated when updating an already syndicated post, no matter if it's written using the classic editor, Gutenberg without legacy metaboxes or Gutenberg with legacy metaboxes.

Possible Drawbacks

Maybe if WP changed how the legacy metaboxes are saved this would need to be reworked, but I'm having a hard time seeing them change that functionality.

Verification Process

Tested this with the classic editor and Gutenberg with and without legacy metaboxes and terms/meta etc are all syndicated correctly when updating a post. See #464 for more details on the bug and how to trigger it.

Checklist:

  • [x ] I have read the CONTRIBUTING document.
  • [ x] My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Applicable Issues

Fixes #464

@lakrisgubben
Copy link
Contributor Author

Regarding the PHPCS warning on line 591 in includes/classes/InternalConnections/NetworkSiteConnection.php: "Processing form data without nonce verification." Since we're not actually processing any form data – just checking if a $_GET variable isset – I'm inclined to add a phpcs:ignore statement to that line. But happy to do something else if y'all think that's better. :)

@jeffpaul jeffpaul added the type:bug Something isn't working. label Feb 11, 2020
@jeffpaul jeffpaul added this to the 2.0.0 milestone Feb 11, 2020
dkotter
dkotter previously approved these changes Feb 14, 2020
@dkotter
Copy link
Collaborator

dkotter commented Feb 14, 2020

@lakrisgubben Thanks for this, looks great and in my testing, things update correctly now when using Gutenberg. If you want to add that ignore statement so the tests pass, I think we'll be good here.

Thanks again!

…ng Gutenberg

Add a check to the NetworkSiteConnection::update_syndicated method to see if the current post is saved through Gutenberg. If so add a new action that runs the same method but on the rest_after_insert_{$post_type} hook to fix an issue where updates to terms/meta etc where not beeing syndicated. Fixes 10up#464
@lakrisgubben
Copy link
Contributor Author

thanks @dkotter! I've added the phpcs:ignore statement now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NetworkSiteConnection::update_syndicated running too early in a Gutenberg/REST context
3 participants