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

Provide ResetPassword notification toMailUsing function with the full resetUrl. #52940

Closed
wants to merge 2 commits into from

Conversation

35grain
Copy link

@35grain 35grain commented Sep 26, 2024

Allows overwriting the resetUrl function using createUrlUsing callback in a similar way verificationUrl can be overwritten with createUrlUsing to be included in toMailUsing in VerifyEmail.

With the current implementation, in order to include a custom URL in toMailUsing callback, the createUrlUsing callback is of no use.

… resetUrl.

Allows overwriting the resetUrl using createUrlUsing in a similar way verificationUrl can be overwritten with createUrlUsing to be included in toMailUsing.
@crynobone
Copy link
Member

This was attempted before and the failed tests are in place to avoid this breaking change.

This should prevent existing implementations from breaking.
@35grain
Copy link
Author

35grain commented Sep 26, 2024

Now it's failing due to Route [password.reset] not defined... that must be related to something else, no?

@crynobone
Copy link
Member

Now it's failing due to Route [password.reset] not defined... that must be related to something else, no?

#39681 The tests was added because this exact PR keep popping up and we want to avoid causing breaking change.

@35grain
Copy link
Author

35grain commented Sep 26, 2024

I understand you want to avoid causing breaking changes, but if this keeps coming up, then perhaps there is a way to implement this in a safe way then? Closing this due to lack of interest on the maintainer's part.

@35grain 35grain closed this Sep 26, 2024
@35grain 35grain deleted the patch-1 branch September 26, 2024 09:24
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.

2 participants