-
Notifications
You must be signed in to change notification settings - Fork 153
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
NetworkSiteConnection::update_syndicated running too early in a Gutenberg/REST context #464
Comments
@lakrisgubben thanks for the very detailed issue and research on this, it's greatly appreciated! I'm pulling this into our next milestoned release to see if we can get this resolved, though if you're able to work up a PR then I'd gladly get it through review an into an upcoming release. Thanks again! |
Thanks for the reply @jeffpaul! Glad to help out with a PR on this, but to do that I'd like some input from someone more well-versed in the codebase to make sure the fix is made in a way that y'all are happy with. :) |
@lakrisgubben Thanks for bringing this to our attention. I've run across a slightly similar Gutenberg issue before but had not encountered this particular one yet. Honestly, not sure there is a great approach here, as we're mostly reliant on how Gutenberg works. This is a known issue (see here for one thread about it: WordPress/gutenberg#12903) and it sounds like they're trying to figure out a good approach for similar scenarios. Here's what I would suggest, which is pretty similar to your approach:
Let me know if you have any questions on this approach (or any concerns). I haven't tested this yet but I think, in theory, this should work and should prevent double updates from happening. |
@dkotter Thanks for the detailed reply! I took a stab at implementing a fix based on your suggestions and found another problem that I hadn't thought of before. :) Adding something like if ( \Distributor\Utils\is_using_gutenberg( $post ) && doing_action( 'save_post' ) ) {
add_action( "rest_after_insert_{$post->post_type}", array( '\Distributor\InternalConnections\NetworkSiteConnection', 'update_syndicated' ) );
return;
} makes the updating of a syndicated article work both with the classic editor and with Gutenberg using no legacy metaboxes. But it breaks when using Gutenberg with legacy metaboxes because those are saved in a separate post request run after the rest request that does not trigger the As you say there doesn't seem to be a great way to handle this situation, but adding So adding this to the if ( \Distributor\Utils\is_using_gutenberg( $post ) && doing_action( 'save_post' ) && ! isset( $_GET['meta-box-loader'] ) ) {
add_action( "rest_after_insert_{$post->post_type}", array( '\Distributor\InternalConnections\NetworkSiteConnection', 'update_syndicated' ) );
return;
} @dkotter let me know if you think this solution seems reasonable or if you can think of another way to deal with this problem and then I can make a PR for it. |
@lakrisgubben Thanks for testing that out. I knew Gutenberg made two requests if you have a custom meta box (which has caused issues on other projects) but I guess I didn't realize the It seems like that Happy to test a PR if you have time to put that together. Thanks for all the effort here! |
…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
…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
Describe the bug
When updating a post that has been distributed to another site in a multisite (internal networkconnection) using Gutenberg, updates to meta, terms, etc are not pushed. The issue here seems to be that the
update_syndicated
method in theNetworkSiteConnection
class is hooked to thesave_post
action. This works fine when using the classic editor, but when using Gutenberg and posts are saved through the REST-api, thesave_post
hook seems to fire earlier. Looking at theupdate_item
method of theWP_REST_Posts_Controller
class one can see thatwp_update_post
that triggers thesave_post
action is called on line 697, and only after that are meta, terms etc handled by the REST-api. This means that when Distributor is running theupdate_item
method, meta, terms etc haven't been updated/saved yet.As a work around I've added the following code to an mu-plugin:
This is not optimal since it means that the post will be distributed twice on save, it's no biggie either, but it makes saving a bit slower since
update_syndicated
is run twice.I'm not sure what the best solution to this problem is to make it work with both Gutenberg and the classic editor. One could of course hook the
update_syndicated
method (with some minor modifications to account for the different values sent through thesave_post
andrest_after_insert_{$post_type}
actions) to therest_after_insert_{$post_type}
action for all post_types where show_in_rest = true. This would mean that theupdate_syndicated
method would be run twice, but that could maybe be handled by aborting that method if something like( ( defined( 'REST_REQUEST' ) && REST_REQUEST ) && doing_action( 'save_post' ) )
was true. But I feel that there are probably smarter ways than that. ;)Anyways, thanks for a great plugin and I'd be happy to help out with this issue if y'all are interested.
Steps to Reproduce
Expected behavior
Meta, terms etc should be distributed on update.
Environment information
The text was updated successfully, but these errors were encountered: