-
Notifications
You must be signed in to change notification settings - Fork 11.3k
[12.x] Types: Password::reset #55564
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
base: 12.x
Are you sure you want to change the base?
Conversation
I don't think this change to interface is correct, the |
@donnysim I think this is a fair criticism. I did consider this when implementing - apologies, I should have definitely written it up in the PR description. I think the current $status = Password::reset(
$validated,
function (User $user) use ($validated) {
$user->forceFill([
'password' => Hash::make($validated['password']),
'remember_token' => Str::random(60),
])->save();
event(new PasswordReset($user));
}
);
// If the password was successfully reset, we will redirect the user back to
// the application's home authenticated view. If there is an error we can
// redirect them back to where they came from with their error message.
return $status === Password::PASSWORD_RESET
? redirect()->route('login')->with('status', __($status))
: back()->withInput($request->only('email'))
->withErrors(['email' => __($status)]); Reports:
For anyone who hasn't implemented their own Additionally, I see (interested to see if people agree) these interfaces as a way for users to ensure their code "behaves" in line with Laravel's own implementations - I think (whilst technically a very minor breaking change) that narrowing this type helps users do that, likely simplifying their own code in the process. (For once) I think the usage of PHPDocs aids this change here, as the "breaking change" is actually completely disregarded at runtime. |
I understand where you're coming from, but at the same time these components are used outside of Laravel too. Majority of cases involve application code where there is nothing to handle by the framework or a library such as breeze, as the auth code is also custom, I don't see anyone extending these without application code to handle new cases, such as external service password resets etc. that return a more detailed response rather than just a measly string. |
Hey, following on from #55109 - this PR implements the same change for the interface, and facade.