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

[11.x] Trigger event on password reset link send #51223

Closed
wants to merge 6 commits into from

Conversation

Muffinman
Copy link
Contributor

This PR adds are new event called PasswordResetLinkSent, which is triggered by the PasswordBroker directly after the Notification is queued.

We had a need for this ourselves, and this seemed to be the only missing event in the auth / reset chain, and seems like something that may be useful for others.

Note that it's possible to trigger your own event within the application layer at least a couple of different ways, using middleware or within a Notification event listener, but those solutions seem messy comparatively.

When looking at the other Auth related events already supported I think doing this natively makes sense.

Note: I decided not to trigger the event if $callback is passed in, since we will not know how this is handled within the callback.

This seemed to be the only missing event in the auth / reset chain.
@Muffinman
Copy link
Contributor Author

Meant to submit this as Draft, will resubmit after tests are passing.

@Muffinman Muffinman closed this Apr 26, 2024
@Muffinman Muffinman reopened this Apr 29, 2024
@taylorotwell
Copy link
Member

We can't call the global event helper from this the Illuminate\Auth package.

@Muffinman
Copy link
Contributor Author

@taylorotwell to be clear, are you saying this is an impossible problem to solve within the Auth package?

Or would you accept a PR to add helpers to the Illuminate/Auth package composer.json or otherwise conditionally call the event when the presence of event helper is detected?

@Muffinman
Copy link
Contributor Author

I think I could have another go at this, adding setDispatcher methods as per the StatefulGuard class, but I think that will end up being a much bigger PR and I don't hold out much hope of it being accepted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants