Skip to content

[Form] Fix PasswordHasherListener to work with empty data #49208

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

Merged
merged 1 commit into from
Feb 4, 2023

Conversation

1ed
Copy link
Contributor

@1ed 1ed commented Feb 2, 2023

Q A
Branch? 6.2
Bug fix? yes
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

This fixes the case when a new user is created by the form,
for example in a registration form. Before this the listener
thrown an exception, because when it tried to get the user
it was not available yet. With this the user resolved later
at the root form when empty data already ran. Other then that
the logic is the same just refactored a code a bit to make it
more readable.

@1ed 1ed force-pushed the fix-form-password-hasher branch from 791d794 to 6494370 Compare February 2, 2023 20:49
@OskarStark OskarStark changed the title [Form] Fix PasswordHasherListener to work with empty data [Form] Fix PasswordHasherListener to work with empty data Feb 3, 2023
@fabpot
Copy link
Member

fabpot commented Feb 4, 2023

Thank you @1ed.

@fabpot fabpot merged commit ad14daf into symfony:6.2 Feb 4, 2023
@1ed 1ed deleted the fix-form-password-hasher branch February 4, 2023 09:13
@@ -77,6 +77,42 @@ public function testPasswordHashSuccess()
$this->assertSame($user->getPassword(), $hashedPassword);
}

public function testPasswordHashSuccessWitnEmptyData()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

testPasswordHashSuccessWitnEmptyData

should be:

testPasswordHashSuccessWithEmptyData

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch, but it's already fixed in the 6.2 branch :) (see d88d5b3)

@fabpot fabpot mentioned this pull request Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants