10up
unifying the dt_push_post args
Description of the Change
dt_push_post hook isn't passing the same set of arguments in WordPressExternalConnection.php and NetworkSiteConnection.php which needs to be fixed as this will cause issues when we use dt_push_post action.
/**
* Fires after a post is pushed via Distributor before `restore_current_blog()`.
*
* @hook dt_push_post
*
* @param {WP_Post} $new_post The newly created post.
* @param {WP_Post} $post_id The original post.
* @param {array} $args The arguments passed into wp_insert_post.
* @param {Connection} $this The Distributor connection being pushed to.
*/
do_action( 'dt_push_post', $new_post, $post, $args, $this );
Passing the new and old post objects instead of id.
Benefits
Passing same arguments would be benefit the users to use the hooks for internal and external site pushes.
Possible Drawbacks
Existing hook attachments may break, as we are changing the arguments.
Verification Process
- Pull and push should work on internal connections
- Pull and push should work on external conenctions
Checklist:
- [x] I have read the CONTRIBUTING document.
- [x] My code follows the code style of this project.
- [x] My change requires a change to the documentation.
- [x] I have updated the documentation accordingly.
- [ ] I have added tests to cover my change.
- [x] All new and existing tests passed.
Applicable Issues
https://github.com/10up/distributor/issues/123
Changelog Entry
Arguments for dt_push_post hook is changed, it might break the existing plugin customisation if any site has.
Changing existing hooks feels dangerous to me, as OP mentioned, this will cause the backward compatibility issue, IMO we can't merge this unless we can address that problem. cc @dkotter
@dinhtungdu yes I agree it is a breaking change. But considering the current implementation users can't use these hooks reliably. I would love to join the backward compatibility issue discussion.
As this was discussed on the original ticket https://github.com/10up/distributor/issues/123#issuecomment-1188921367 and a different approach decided upon, I'm going to close this PR off so the commit history doesn't include the initial idea.
