Fixed #35477 -- Fixed field required validation in forms inheriting from SetPassword. #18195
Conversation
|
Thank you for the PR @fsbraun ⭐ |
f1d2ab9 to
1693f3b
Compare
|
@nessita Done! |
How do I install this version with pip? |
|
Try |
This version works. Thank you. |
There was a problem hiding this comment.
Hey @fsbraun, thanks for adding tests! Could you please rename and move each test to its specific test class? Otherwise we get tests for other forms inside SetPasswordFormTest which is about the form and not the mixin.
The relevant test cases are:
SetPasswordFormTest(forSetPasswordForm)BaseUserCreationFormTest(forBaseUserCreationForm)AdminPasswordChangeFormTest(forAdminPasswordChangeForm)
I also added a comment about an untested change. Thank you! 🌟
35cf874 to
c1673d6
Compare
…ge forms. The auth forms using SetPasswordMixin were incorrectly including the 'This field is required.' error when additional validations (e.g., overriding `clean_password1`) were performed and failed. This fix ensures accurate error reporting for password fields. Co-authored-by: Natalia <[email protected]>
There was a problem hiding this comment.
Thank you @fsbraun for the fixes.
I have made some tweaks because I wasn't comfortable with the weakening of the if guard to == "". Additionally, I removed the call to validate_password in the custom forms (in the tests) as I think it caused some confusion. It was only when I realized that the specifics of what happens inside a clean_* method are not relevant that I understood the important part: the key is to raise any validation error inside the clean field method, and so the password field is no longer present inside cleaned_data leading to the fixed issue.
Trac ticket number
ticket-35477
Branch description
When adding additional validation to the
SetPasswordform indjango.contrib.authand they fail, the from's clean method assumed they were empty since it did only check for truthiness of the passwords.This PR changes the check for empty passwords to them being empty strings.
@sarahboyce Thanks for providing those helpful tests!!
Checklist
mainbranch.