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

Error importing RegistrationView when using custom user model with unique USERNAME_FIELD #234

Open
kbarnes3 opened this issue Jul 15, 2022 · 7 comments

Comments

@kbarnes3
Copy link

kbarnes3 commented Jul 15, 2022

I believe I'm hitting a bug where I can't import anything from django_registration.backends.activation.views because of a bug in django_registration.forms.RegistrationForm that is hit at import time.

Setup

Create a custom user model deriving from Django's AbstractBaseUser. Given it the following properties at a minimum:

    primary_email = models.EmailField(max_length=255, unique=True)
    USERNAME_FIELD = 'primary_email'
    EMAIL_FIELD = 'primary_email'

In your settings.py, set AUTH_USER_MODEL = 'users.MyUser'

In on of your urls.py, add this import:
from django_registration.backends.activation.views import RegistrationView

This will lead to the following crash when starting Django:

 File "root\users\urls.py", line 3, in <module>
    from django_registration.backends.activation.views import RegistrationView
  File "root\venv\lib\site-packages\django_registration\backends\activation\views.py", line 18, in <module>
    from django_registration.views import ActivationView as BaseActivationView
  File "root\venv\lib\site-packages\django_registration\views.py", line 18, in <module>
    from .forms import RegistrationForm
  File "root\venv\lib\site-packages\django_registration\forms.py", line 22, in <module>
    class RegistrationForm(UserCreationForm):
  File "root\venv\lib\site-packages\django\forms\models.py", line 327, in __new__
    raise FieldError(message)
django.core.exceptions.FieldError: Unknown field(s) (primary_email) specified for User

Since this bug in RegistrationForm is happening at import time (and it is imported by RegistrationView), it doesn't matter that I'm following the documentation for custom user models and providing my own RegistrationForm - Django is crashing too early.

I believe the bug is around RegistrationForm's meta class:

    class Meta(UserCreationForm.Meta):
        fields = [
            User.USERNAME_FIELD,
            User.get_email_field_name(),
            "password1",
            "password2",
        ]

Earlier in the file, we have User = get_user_model(), which resolves to 'users.MyUser' in this case, with User.USERNAME_FIELD and User.get_email_field_name() both evaluating to 'primary_email'. However, RegistrationForm's meta class does not override UserCreateForm.Meta's model property, which is set to the Django default User model. This User model does not have a primary_email property, leading to the above error.

I believe the fix is to set hard code User to django.contrib.auth.models.User in forms.py. This would be a breaking change however, since people with custom classes would need to provide both model and fields in their RegistrationForm's Meta class.

@ubernostrum
Copy link
Owner

I need to think about this one.

@kbarnes3
Copy link
Author

I was able to work around this by setting my EmailField to be named username. This wasn't too difficult since I'm trying to incorporate Django-Registration into a new project that hasn't been deployed yet, but I imagine it would be more difficult for established sites to do this.

@ubernostrum
Copy link
Owner

The thing that's worrying me here is that there's no such thing as a truly generic user-registration view/form for Django. The amount of stuff you can do in a custom user model just makes it impossible to write a single form that can handle anything someone might do. And the documentation on using a custom user model with django-registration already makes clear that there are times when you're going to have to write your own stuff rather than be able to use simple subclasses of the built-in forms/views.

So I'm leaning toward just doing a better job of documenting what django-registration can support, and maybe better documentation of what you need to do when your user model gets too different from that.

@kbarnes3
Copy link
Author

kbarnes3 commented Aug 8, 2022

I think a big problem with this issue is that the errors occur on import time. As it is structured now, not only do I have to write my own form, but I can't use the RegistrationView (because I can't import it) nor the URLs (because those import the RegistrationView as well). It would be nice if these errors could be deferred in some way to when the impacted classes are instantiated or something similar.

confuzeus added a commit to confuzeus/django-registration that referenced this issue Oct 28, 2022
By default, Django will use its own default User model when
initializing the CreationForm.

This will cause FieldErrors to be raised when a customer user model
is provided but doesn't contain the same fields as in Django's default
implementation.

That's why we use the custom user model to instantiate a CreationForm
instead. The implementation can now have any fields it wants as long as
it's compatible with Django's AbstractUser.

Fixes ubernostrum#234
confuzeus added a commit to confuzeus/django-registration that referenced this issue Oct 28, 2022
By default, Django will use its own default User model when
initializing the CreationForm.

This will cause FieldErrors to be raised when a customer user model
is provided but doesn't contain the same fields as in Django's default
implementation.

That's why we use the custom user model to instantiate a CreationForm
instead. The implementation can now have any fields it wants as long as
it's compatible with Django's AbstractUser.

Fixes ubernostrum#234
@confuzeus
Copy link

Why not use get_user_model in the RegistrationForm? I just submitted a PR for that but don't know if I'm missing a case where not using Django's default might break someone's code.

@ubernostrum
Copy link
Owner

Yeah, the issue is that UserCreationForm has defaulted to the built-in User forever, and I don't even know how much stuff depends on it.

If it's going to change, it'll have to change in a major version bump. I'm thinking through whether I want to do that to cover this case, since it's one that comes up but not super often, and the case of differing significantly from Django's built-in model is already one that has a lot of warnings in the docs.

@ubernostrum
Copy link
Owner

I've thought about this a lot, and I think the only way forward is to finally just give up on using Django's UserCreationForm.

That's a backwards-incompatible change, so I can't do it immediately, but once I get a release out for official Django 4.2 compatibility I'll start on a major version bump branch that will ship its own base form that doesn't hard-wire in the default Django User model.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants