#57436 closed feature request (fixed)
Add hook to wp_set_password
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.2 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Users | Keywords: | has-patch needs-dev-note add-to-field-guide has-unit-tests commit |
Focuses: | Cc: |
Description
In WordPress 4.4 an action was added for the password reset option, however, I am working on a Single Sign-On application and need to sync the password every time it is updated. Many plugins call wp_set_password
directly, so I'd like to propose adding an action to the end of the function to catch all instances of password setting.
Attachments (3)
Change History (26)
#1
@
2 years ago
- Keywords has-patch changes-requested added
- Milestone changed from Awaiting Review to 6.2
- Owner set to audrasjb
- Severity changed from trivial to normal
- Status changed from new to reviewing
- Type changed from enhancement to feature request
Hello and thanks for the ticket!
Looks like a simple and relevant feature and it has a patch.
Let's move this for 6.2 consideration.
However, the patch should be updated to add a Docblock for this new action :)
#3
@
2 years ago
Thanks for this new patch @tanner-m!
Looks almost good :)
Sorry for nitpicking but we have two small things to address before committing this, to make sure it's compliant to WordPress Coding Standards:
- Add a period at the end of the short description
- Put the
@since
mention between the shortdesc and the parameters
Just like this:
/** * Fires after the password is set. * * @since 6.2.0 * * @param string $password The plain text password just set. * @param mixed $user_id The ID of the user whose password was just reset. */ do_action( 'wp_set_password', $password, $user_id );
#5
@
2 years ago
- Keywords needs-dev-note add-to-field-guide commit added; changes-requested removed
Alright, this new patch looks good to me, thanks!
Adding needs-dev-note
workflow keyword to not forget to mention this change in a WP 6.2 Dev Note :)
#6
@
2 years ago
I was wondering whether the action should be triggered before or after clean_user_cache()
but your implementation (after) looks like the best option, since it's better to clean the cached data before allowing operations to plugin authors. They can always re-clean cached data in their function, depending on what they are doing with the hook.
👆 Perhaps something worth mentioning in the dev note?
#8
@
2 years ago
- Keywords needs-unit-tests added; commit removed
- Resolution fixed deleted
- Status changed from closed to reopened
Hmm. Reopening as it would be nice to add unit tests for this new hook.
This ticket was mentioned in PR #3844 on WordPress/wordpress-develop by @audrasjb.
2 years ago
#9
- Keywords has-unit-tests added; needs-unit-tests removed
#11
@
2 years ago
PR3844 adds a unit test for this new hook.
By the way, @SergeyBiryukov thanks for the docblock fix!
@SergeyBiryukov commented on PR #3844:
2 years ago
#12
Thanks for working on the test!
We should be able to test that the action works as expected by checking the call count via MockAction
:
public function test_wp_set_password_action() { $action = new MockAction(); add_action( 'wp_set_password', array( $action, 'action' ) ); wp_set_password( 'A simple password', self::$user_id ); $this->assertSame( 1, $action->get_call_count() ); }
This approach is used in other tests and seems simpler that writing a meta value 🙂
@SergeyBiryukov commented on PR #3844:
2 years ago
#13
Thanks for working on the test!
We should be able to test that the action works as expected by checking the call count via MockAction
:
public function test_wp_set_password_action() { $action = new MockAction(); add_action( 'wp_set_password', array( $action, 'action' ) ); wp_set_password( 'A simple password', self::$user_id ); $this->assertSame( 1, $action->get_call_count() ); }
This approach is used in other tests and seems simpler that writing a meta value 🙂
@audrasjb commented on PR #3844:
2 years ago
#14
Oh ok, thanks @SergeyBiryukov 👍
I need to keep myself more up to date on the functions that can be used. Do you have a starting point resource in mind to discover the functions that can be used within WP core tests?
This ticket was mentioned in Slack in #core by mukeshpanchal27. View the logs.
2 years ago
#16
@
2 years ago
- Keywords changes-requested added
This ticket was reviewed during the recent bug scrub. I've reviewed the PR and left a couple of small additions. Adding changes-requested
to note the current status of the ticket and PR.
Additional props: @mukesh27
This ticket was mentioned in Slack in #core by mukeshpanchal27. View the logs.
2 years ago
This ticket was mentioned in PR #4020 on WordPress/wordpress-develop by @costdev.
2 years ago
#18
_This is an update of #3844 to change "Test" to "Tests" in the docblock and to add a @covers
annotation_
Trac ticket: https://core.trac.wordpress.org/ticket/57436
#19
@
2 years ago
I've submitted PR 4020 to make small adjustments to PR 3844.
@audrasjb Is this one ready for commit?
Additional props: @mukesh27
@audrasjb commented on PR #3844:
2 years ago
#20
Closing in favor of https://github.com/WordPress/wordpress-develop/pull/4020
@audrasjb commented on PR #4020:
2 years ago
#23
committed in https://core.trac.wordpress.org/changeset/55250
Patch 1