[Django] #29449: Regression in UserCreationForm (and UserChangeForm) for custom user models

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[Django] #29449: Regression in UserCreationForm (and UserChangeForm) for custom user models

Django
#29449: Regression in UserCreationForm (and UserChangeForm) for custom user models
-------------------------------------+-------------------------------------
               Reporter:  Sławek     |          Owner:  nobody
  Ehlert                             |
                   Type:  Bug        |         Status:  new
              Component:             |        Version:  2.1
  contrib.auth                       |       Keywords:  auth forms
               Severity:  Normal     |  UserCreationForm UserChangeForm
           Triage Stage:             |      Has patch:  0
  Unreviewed                         |
    Needs documentation:  0          |    Needs tests:  0
Patch needs improvement:  0          |  Easy pickings:  0
                  UI/UX:  0          |
-------------------------------------+-------------------------------------
 In 3333d935d2914cd80cf31f4803821ad5c0e2a51d (part of #28757 ticket)
 {{{UserCreationForm}}} (and by extension also {{{UserChangeForm}}}) was
 adjusted to support custom user models without overriding the form. The
 problem is that the implementation there assumes that the
 {{{USERNAME_FIELD}}} has to be a subclass of {{{CharField}}} since it uses
 {{{UsernameField}}} in the {{{field_classes}}} mapping.

 Now to the fun part. Django already has some tests that should fail in
 such case (for example see
 https://github.com/django/django/blob/825f0beda804e48e9197fcf3b0d909f9f548aa47/tests/auth_tests/test_management.py#L435-L491),
 but the failure is actually "swallowed" when running the whole test suite.
 When running those tests individually we can see the failure.
 Tim Graham already noticed something similar in
 https://code.djangoproject.com/ticket/28757#comment:7.
 I.e.
 {{{
  ./runtests.py auth_tests
 }}}
 works, but
 {{{
 ./runtests.py
 auth_tests.test_management.CreatesuperuserManagementCommandTestCase.test_fields_with_fk
 }}}
 doesn't. I'm not sure why the failure is hidden in the former case (I
 suspect it has to do something with the {{{override_settings}}} decorator
 and {{{importlib.reload}}}ing done in the {{{reload_auth_forms}}} function
 from the commit I've mentioned). The stacktrace from the latter case looks
 like this:
 {{{
 ERROR: test_fields_with_fk
 (auth_tests.test_management.CreatesuperuserManagementCommandTestCase)
 ----------------------------------------------------------------------
 Traceback (most recent call last):
   File "/Users/slafs/testing/django/django/django/test/utils.py", line
 364, in inner
     with self as context:
   File "/Users/slafs/testing/django/django/django/test/utils.py", line
 336, in __enter__
     return self.enable()
   File "/Users/slafs/testing/django/django/django/test/utils.py", line
 405, in enable
     setting=key, value=new_value, enter=True)
   File "/Users/slafs/testing/django/django/django/dispatch/dispatcher.py",
 line 175, in send
     for receiver in self._live_receivers(sender)
   File "/Users/slafs/testing/django/django/django/dispatch/dispatcher.py",
 line 175, in <listcomp>
     for receiver in self._live_receivers(sender)
   File "/Users/slafs/testing/django/django/django/test/signals.py", line
 195, in user_model_swapped
     from django.contrib.auth import forms
   File "/Users/slafs/testing/django/django/django/contrib/auth/forms.py",
 line 63, in <module>
     class UserCreationForm(forms.ModelForm):
   File "/Users/slafs/testing/django/django/django/forms/models.py", line
 256, in __new__
     apply_limit_choices_to=False,
   File "/Users/slafs/testing/django/django/django/forms/models.py", line
 172, in fields_for_model
     formfield = f.formfield(**kwargs)
   File
 "/Users/slafs/testing/django/django/django/db/models/fields/related.py",
 line 956, in formfield
     **kwargs,
   File
 "/Users/slafs/testing/django/django/django/db/models/fields/related.py",
 line 418, in formfield
     return super().formfield(**defaults)
   File
 "/Users/slafs/testing/django/django/django/db/models/fields/__init__.py",
 line 890, in formfield
     return form_class(**defaults)
   File "/Users/slafs/testing/django/django/django/forms/fields.py", line
 213, in __init__
     super().__init__(**kwargs)
 TypeError: __init__() got an unexpected keyword argument
 'limit_choices_to'
 }}}

 Since {{{auth_tests.CustomUserWithFK}}} model uses a {{{ForeignKey}}} as a
 {{{USERNAME_FIELD}}}, the instantiation of
 {{{django.contrib.auth.forms.UsernameField}}} is failing. However, that
 test is a bit implicit as I'm not entirely sure why Django imports
 {{{UserCreationForm}}} in the first place (the stacktrace makes me think
 it's because of the signal mechanisms).

 I'd suggest the following steps to go forward with this:

 1) Write a more explicit regression test in {{{auth_tests.test_forms}}}
 that's using {{{auth_tests.CustomUserWithFK}}} as a user model (I'll try
 to do that).
 2) Make adjustments in {{{auth_tests.test_forms}}} so that the mentioned
 tests will actually fail even when running the whole test suite (I think
 Tim Graham already tried this in
 https://code.djangoproject.com/ticket/28757#comment:8).
 3) Fix {{{UserCreationForm}}} so it supports different types of fields for
 {{{USERNAME_FIELD}}}.

 In my humble opinion this is a release blocker (I haven't marked this
 ticket as such, though).

--
Ticket URL: <https://code.djangoproject.com/ticket/29449>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/048.48dcf8393a8a7281a77ca9dd8f3cc022%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29449: Regression in UserCreationForm (and UserChangeForm) for custom user models

Django
#29449: Regression in UserCreationForm (and UserChangeForm) for custom user models
-------------------------------------+-------------------------------------
     Reporter:  Sławek Ehlert        |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  contrib.auth         |                  Version:  2.1
     Severity:  Release blocker      |               Resolution:
     Keywords:  auth forms           |             Triage Stage:  Accepted
  UserCreationForm UserChangeForm    |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Markus Holtermann):

 * severity:  Normal => Release blocker
 * stage:  Unreviewed => Accepted


Comment:

 I can confirm this.

 Using something like
 `UserModel._meta.get_field(UserModel.USERNAME_FIELD).formfield()` to
 derive the form field for a the current username field _may_ make sense.
 However, this won't work with the normalization we currently do in the
 form's `UsernameField`.

--
Ticket URL: <https://code.djangoproject.com/ticket/29449#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/063.2cd59dca8f788b3a4071e7e4ec0baf13%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29449: Regression in UserCreationForm (and UserChangeForm) for custom user models

Django
In reply to this post by Django
#29449: Regression in UserCreationForm (and UserChangeForm) for custom user models
-------------------------------------+-------------------------------------
     Reporter:  Sławek Ehlert        |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  contrib.auth         |                  Version:  2.1
     Severity:  Release blocker      |               Resolution:
     Keywords:  auth forms           |             Triage Stage:  Accepted
  UserCreationForm UserChangeForm    |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Sławek Ehlert):

 I've added some explicit tests in
 [https://github.com/django/django/pull/10020 PR#10020].

 Unfortunately it causes more failures than the ones I've introduced with
 those new tests there.

 What happens is:

 - the test I've added overrides the {{{AUTH_USER_MODEL}}} setting with a
 model that has a FK as a username field
 ({{{auth_tests.CustomUserWithFK}}}).
 - the test fails on the {{{override_settings}}} part already, since the
 {{{reload_auth_forms}}} receiver fails to reload the
 {{{django.contrib.auth.forms}}} module with that user model in place
 (fails with a similar {{{TypeError}}} that refers to 'limit_choices_to'
 argument when trying to instantiate the {{{UsernameField}}})[*]
 - because of the {{{override_settings}}} bug (see #29467) other tests
 still see the {{{auth_tests.CustomUserWithFK}}} as a current user model.

 [*] - the exact traceback for the mentioned error is:
 {{{
 ======================================================================
 ERROR: test_custom_form_username_not_charfield
 (auth_tests.test_forms.UserChangeFormTest)
 ----------------------------------------------------------------------
 Traceback (most recent call last):
   File
 "/usr/local/Cellar/python/3.6.5/Frameworks/Python.framework/Versions/3.6/lib/python3.6/unittest/case.py",
 line 59, in testPartExecutor
     yield
   File
 "/usr/local/Cellar/python/3.6.5/Frameworks/Python.framework/Versions/3.6/lib/python3.6/unittest/case.py",
 line 605, in run
     testMethod()
   File "/Users/slafs/testing/django/django/django/test/utils.py", line
 364, in inner
     with self as context:
   File "/Users/slafs/testing/django/django/django/test/utils.py", line
 336, in __enter__
     return self.enable()
   File "/Users/slafs/testing/django/django/django/test/utils.py", line
 405, in enable
     setting=key, value=new_value, enter=True)
   File "/Users/slafs/testing/django/django/django/dispatch/dispatcher.py",
 line 175, in send
     for receiver in self._live_receivers(sender)
   File "/Users/slafs/testing/django/django/django/dispatch/dispatcher.py",
 line 175, in <listcomp>
     for receiver in self._live_receivers(sender)
   File
 "/Users/slafs/testing/django/django/tests/auth_tests/test_forms.py", line
 38, in reload_auth_forms
     reload(django.contrib.auth.forms)
   File
 "/Users/slafs/.virtualenvs/django/lib/python3.6/importlib/__init__.py",
 line 166, in reload
     _bootstrap._exec(spec, module)
   File "<frozen importlib._bootstrap>", line 618, in _exec
   File "<frozen importlib._bootstrap_external>", line 678, in exec_module
   File "<frozen importlib._bootstrap>", line 219, in
 _call_with_frames_removed
   File "/Users/slafs/testing/django/django/django/contrib/auth/forms.py",
 line 63, in <module>
     class UserCreationForm(forms.ModelForm):
   File "/Users/slafs/testing/django/django/django/forms/models.py", line
 256, in __new__
     apply_limit_choices_to=False,
   File "/Users/slafs/testing/django/django/django/forms/models.py", line
 172, in fields_for_model
     formfield = f.formfield(**kwargs)
   File
 "/Users/slafs/testing/django/django/django/db/models/fields/related.py",
 line 956, in formfield
     **kwargs,
   File
 "/Users/slafs/testing/django/django/django/db/models/fields/related.py",
 line 418, in formfield
     return super().formfield(**defaults)
   File
 "/Users/slafs/testing/django/django/django/db/models/fields/__init__.py",
 line 890, in formfield
     return form_class(**defaults)
   File "/Users/slafs/testing/django/django/django/forms/fields.py", line
 213, in __init__
     super().__init__(**kwargs)
 TypeError: __init__() got an unexpected keyword argument
 'limit_choices_to'
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/29449#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/063.2fe9d1a042adc9d4f4cdf00260b4f4eb%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29449: Regression in UserCreationForm (and UserChangeForm) for custom user models

Django
In reply to this post by Django
#29449: Regression in UserCreationForm (and UserChangeForm) for custom user models
-------------------------------------+-------------------------------------
     Reporter:  Sławek Ehlert        |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  contrib.auth         |                  Version:  2.1
     Severity:  Release blocker      |               Resolution:
     Keywords:  auth forms           |             Triage Stage:  Accepted
  UserCreationForm UserChangeForm    |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Tim Graham):

 I don't understand why this issue is described as a regression. Using a
 custom user model with `UserCreationForm` and `UserChangeForm` is a new
 feature, so I wouldn't describe any issues there as a regression. A
 regression would be something that worked in older versions of Django that
 no longer works. Am I misunderstanding?

--
Ticket URL: <https://code.djangoproject.com/ticket/29449#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/063.121860db4a06d9c7f636658bd1f701c8%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29449: Regression in UserCreationForm (and UserChangeForm) for custom user models

Django
In reply to this post by Django
#29449: Regression in UserCreationForm (and UserChangeForm) for custom user models
-------------------------------------+-------------------------------------
     Reporter:  Sławek Ehlert        |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  contrib.auth         |                  Version:  2.1
     Severity:  Release blocker      |               Resolution:
     Keywords:  auth forms           |             Triage Stage:  Accepted
  UserCreationForm UserChangeForm    |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Sławek Ehlert):

 Replying to [comment:3 Tim Graham]:
 > I don't understand why this issue is described as a regression. Using a
 custom user model with `UserCreationForm` and `UserChangeForm` is a new
 feature, so I wouldn't describe any issues there as a regression. A
 regression would be something that worked in older versions of Django that
 no longer works. Am I misunderstanding?

 Nope, all good. You're correct about the naming here. My bad.

--
Ticket URL: <https://code.djangoproject.com/ticket/29449#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/063.2de9276f3603cf0398481ec9927e3f38%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29449: UserCreationForm and UserChangeForm don't work if username isn't a CharField (was: Regression in UserCreationForm (and UserChangeForm) for custom user models)

Django
In reply to this post by Django
#29449: UserCreationForm and UserChangeForm don't work if username isn't a
CharField
-------------------------------------+-------------------------------------
     Reporter:  Sławek Ehlert        |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  contrib.auth         |                  Version:  2.1
     Severity:  Release blocker      |               Resolution:
     Keywords:  auth forms           |             Triage Stage:  Accepted
  UserCreationForm UserChangeForm    |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Tim Graham):

 If a patch isn't forthcoming (the best way to avoid the issue that Markus
 pointed out in comment 1 isn't obvious to me), I think we could deescalate
 from a release blocker by documenting this limitation.

--
Ticket URL: <https://code.djangoproject.com/ticket/29449#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/063.1dc8f9fabaf69ba78a92b37e1667e2cf%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29449: UserCreationForm and UserChangeForm don't work if username isn't a CharField

Django
In reply to this post by Django
#29449: UserCreationForm and UserChangeForm don't work if username isn't a
CharField
-------------------------------------+-------------------------------------
     Reporter:  Sławek Ehlert        |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  contrib.auth         |                  Version:  2.1
     Severity:  Release blocker      |               Resolution:
     Keywords:  auth forms           |             Triage Stage:  Accepted
  UserCreationForm UserChangeForm    |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Sławek Ehlert):

 I'd like to submit a patch for that, but currently I was focusing more on
 solving #29467 first.

--
Ticket URL: <https://code.djangoproject.com/ticket/29449#comment:6>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/063.b1f64bfbe6b5a5d37b0958e1b45bb4ea%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.