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

Fixed #32235 -- Made ReadOnlyPasswordHashField disabled by default. #13741

Merged
merged 1 commit into from
Dec 3, 2020

Conversation

timobrembeck
Copy link
Contributor

@timobrembeck timobrembeck commented Dec 2, 2020

Ticket: #32235

Set disabled=True in contrib.auth's ReadOnlyPasswordHashField which made the custom bound_data() and has_changed() methods obsolete. This also eliminated the need for a clean_password() method in the form using that field, e.g. the UserChangeForm.

I'm not sure whether my test is reasonable - technically, there are other tests which fail if the methods are removed without setting the disabled flag. Is the disabled property a valid requirement here or just an implementation detail and what really matters is that the disabled-functionality is achieved (has_changed = False and bound_data/cleaned_data = initial)?

Thanks in advance for your feedback!

Copy link
Member

@felixxm felixxm left a comment

Choose a reason for hiding this comment

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

@timoludwig Thanks for this patch 👍 I left small comments, we should also add a short release note about changing the default value of ReadOnlyPasswordHashField.disabled in the "Backwards incompatible changes in 3.2 -> Miscellaneous".

django/contrib/auth/forms.py Outdated Show resolved Hide resolved
docs/topics/auth/customizing.txt Show resolved Hide resolved
tests/auth_tests/test_forms.py Outdated Show resolved Hide resolved
tests/auth_tests/test_forms.py Outdated Show resolved Hide resolved
@timobrembeck
Copy link
Contributor Author

@felixxm Thanks for your review! I have addressed your comments, let me know if there is anything I need to improve.

@felixxm felixxm changed the title Fixed #32235 -- Set disabled prop on ReadOnlyPasswordHashField Fixed #32235 -- Made ReadOnlyPasswordHashField disabled by default. Dec 3, 2020
@felixxm
Copy link
Member

felixxm commented Dec 3, 2020

@timoludwig Thanks 👍 Welcome aboard ⛵

@felixxm felixxm merged commit d8dfff2 into django:master Dec 3, 2020
@timobrembeck timobrembeck deleted the ticket_32235 branch December 3, 2020 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants