Merge lp://staging/~twom/canonical-identity-provider/django-1.10 into lp://staging/canonical-identity-provider/release
Proposed by
Tom Wardill
| Status: | Merged |
|---|---|
| Merged at revision: | 1603 |
| Proposed branch: | lp://staging/~twom/canonical-identity-provider/django-1.10 |
| Merge into: | lp://staging/canonical-identity-provider/release |
| Prerequisite: | lp://staging/~twom/canonical-identity-provider/django-1.9-deprecation-warnings |
| Diff against target: |
287 lines (+40/-37) 7 files modified
config-manager.txt (+1/-1) requirements.txt (+1/-1) scripts/generate-random-accounts (+10/-2) src/identityprovider/admin.py (+13/-11) src/identityprovider/tests/test_forms.py (+1/-0) src/identityprovider/tests/test_widgets.py (+7/-11) src/identityprovider/widgets.py (+7/-11) |
| To merge this branch: | bzr merge lp://staging/~twom/canonical-identity-provider/django-1.10 |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Ricardo Kirkner (community) | Approve | ||
|
Review via email:
|
|||
This proposal supersedes a proposal from 2018-02-20.
Description of the change
Upgrade to django 1.10.
Not intended for landing, just for review before moving on to 1.11.
Some changes around read only fields in the admin and handling of default attributes in forms.
To post a comment you must log in.
Revision history for this message
| Ricardo Kirkner (ricardokirkner) | # |
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
<pindonga> twom, on the django 1.10 mp, how is the password_setting change related to django 1.10?
<pindonga> same for the 'choices' stuff
<twom> the password field is defined as 'editable=False'
<twom> pre 1.10, you could still define a ModelForm field with that name and it would work
<twom> with 1.10, that is invalid and gives an error
<twom> the choices changes are due to an API change for widgets, 1.10 removes the `choices` keyword on the `render` method, and only allows it to be given to the `init`
<pindonga> ack
<pindonga> finally, why is the change necessary in the test to pass twofactor_
<pindonga> so, I just added a comment, I think would be good to address before landing
<twom> the old behaviour was that on form submission (or save), django would reset the values of any fields that were missing to the default defined on the form.
<twom> new behaviour is that it will keep the existing value
<twom> so that test broke as it was assuming that the value of the twofactor_required field would implicitly change
<twom> as we only appear use that form in the admin, where all the values are rendered, we can just update the data we are giving to the form save in the test
<twom> pindonga: I see the comment, I actually do that on line 38, just missed taking the `chdir` out. I'll have another look at seeing if it can be run from anywhere in the morning
<pindonga> ack, if you remove the chdir, I'm fine with ti
<twom> cool, I'll do that.
<twom> pindonga: thanks for the review :)
<pindonga> sure
<pindonga> let me know when you pushed and I'll rubber stamp it
<twom> just pushed the chdir change