-
Notifications
You must be signed in to change notification settings - Fork 800
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
Sync: Update User Sync Module #8506
Conversation
|
||
return; | ||
} | ||
do_action( 'jetpack_sync_register_user', $user_id ); |
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.
Do we need to add the Documentation comment here?
/** | ||
* Fires when the client needs to sync an updated user | ||
* | ||
* @since 4.2.0 | ||
* | ||
* @param object The Sanitized WP_User object | ||
* @param array state Since 5.8 | ||
* @param array state Since 5.8 |
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.
lets update this to 5.8.0
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.
Tests well! Awesome work
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.
Do you think you could make those changes in the future PRs, so things get categorized properly?
Thank you.
/** | ||
* Fires when a new user is registered on a site | ||
* | ||
* @since 4.9.0 |
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.
Thank you for adding documentation! Could you also add the @module sync
text to you Sync hooks, so it gets categorized by the parser?
https://developer.jetpack.com/module/sync/
Thank you!
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.
noted, do you have time for a quick PR to add the missing ones? 😊
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.
Not right now, no, unfortunately, but I'll keep it in mind :)
* Changelog 5.8: create base for changelog. * Update 5.8 release post link * fix 5.8 release date * Updates to plugin description * Changelog: add #8499 * Changelog: add #8506 * Changelog: add #8509 * Changelog: add #8516 * Changelog: add #8517 * Changelog: add #8523 * Changelog: add #8547 * Changelog: add #8496 * Changelog: add #8584 * Changelog: add #8595 * Changelog: add #8445 * Changelog: add #8431 * Changelog: add #8284 * Changelog: add #8270 * Changelog: add #8124 * Changelog: add #8581 * Changelog: add #8463 * Changelog: add #8568 (#8646) * Updates to testing list and changelog * Changelog: add #8443 * Changelog: add #8459 * Changelog: add #8469 * Changelog: add #8464 * Changelog: add #8478 and #8479 * Changelog: add #8483 * Changelog: add #8488 * Changelog: add #8513 * Changelog: add #8555 * Changelog: add #8565 * Changelog: add #8601 * Changelog: add #8612 * Changelog: add first pass at Search items. * Changelog: add more info to help test Search. * Changelog: add #8144 * Changelog: add #8313 * Changelog: add #8419 * Changelog: add #8465 * Changelog: add #8515 * Changelog: add #8587 * Changelog: add #8591 * Changelog: add #8659 * Changelog: add #8661 * Changelog: add #8671 * Changelog: add 5.7.1 to archived changelog too. * Reverted changes to readme, removed entry about backups.
This PR removes duplicate synced actions.
To test:
With this diff applied try
adding a new user
editing a user
adding / editing via API
editing your profile
add existing user to multi-site
invite user -> user accepts invite
user registers via registration form
delete user
Ensure stuff is syncing properly upstream.