3 ms·

The whole USERNAME_FIELD / REQUIRED_FIELDS thing feels clunkier that it should be. Isn't defining REQUIRED_FIELDS redundant, since this information could be in

The whole USERNAME_FIELD / REQUIRED_FIELDS thing feels clunkier that it should be. Isn't defining REQUIRED_FIELDS redundant, since this information could be introspected from the fields?

pydanny 14y ago
I agree it's a bit clunky, but... Making REQUIRED_FIELDS figure itself out from introspection means we all risk getting hurt by unanticipated edge cases. I would rather deal with a small bit of clunk than magical code touching my user model.
- hcarvalhoalves 14y ago
  The point is: what does REQUIRED_FIELDS do? Model validation? Form validation? Can you put a blank=True field on REQUIRED_FIELDS? Can you not put a blank=False on REQUIRED_FIELDS? It's duplicating behavior in two different places, and it's not entirely clear what it does, or what overrides what. This is much worse than introspection, because you'll have to patch Django to fix any inconsistencies instead of just changing your model definition / inheriting forms accordingly.
  - pydanny 14y ago
    See https://docs.djangoproject.com/en/dev/topics/auth/#django.contrib.auth.models.CustomUser.User.REQUIRED_FIELDS https://docs.djangoproject.com/en/dev/topics/auth/#django.co... The quick version is that it's not duplicating behavior, the documentation is simply still undergoing work. We'll try and make it a bit more clear in the days to come. The long version is that during user creation sometimes you have fields that for whatever reason are not required by the database but are required for the user to enter in. This is because user creation is often more complex than it seems on the surface, and there is already existing django.contrib.auth.views that need this sort of configuration. REQUIRED_FIELDS gives you the control to force the user to enter data while not not altering the database. Is it ideal? Not really. But having dealt with user creation through email, OAuth, OpenID, LDAP, SOAP, and worse, having this as a configurable setting is better than an auto-magic introspection. Finally, I'll return to the documentation issue. I'm officially duly noting that the documentation on this feature needs work. I've opened ticket #19402 to address this issue. See https://code.djangoproject.com/ticket/19402 https://code.djangoproject.com/ticket/19402
    - ProblemFactory 14y ago
      > The long version is that during user creation sometimes you have fields that for whatever reason are not required by the database but are required for the user to enter in. This is because user creation is often more complex than it seems on the surface, and there is already existing django.contrib.auth.views that need this sort of configuration. REQUIRED_FIELDS gives you the control to force the user to enter data while not not altering the database. Could you please explain this further? Is REQUIRED_FIELDS intended for use cases where some users are created by import/migration, but others by sign-up forms (with more requirements)? From grepping the 1.5beta1 source code, it appears that the only place where REQUIRED_FIELDS is actually read is in contrib/auth/management/commands/createsuperuser.py.
      - pydanny 14y ago
        Rather than get into a huge discussion here, I'm going to spend my time working the ticket I mentioned. This isn't production ready code/documentation, it's a BETA release. Your issue has been noted and we are digging in. :-)
freakboy3742 14y ago
I'm the author of the feature, so I figured I should give some explanation. REQUIRED_FIELDS is used internally in one place -- in the createsuperuser management command. When you run that command, you are prompted for the minimal set of fields to create a user object. REQUIRED_FIELDS is that list of fields. Could this be used elsewhere? Sure. A model form, for example, could use this list as the minimal data to create a user object. Could this be replaced with some model introspection? Possibly. On my first pass through, this is exactly what I tried to do. And the code got messy really quickly. At a first approximation, any field with blank=False and no default is essentially "required". However, what about BooleanFields? BooleanFields have an automatically default to False, but that doesn't necessarily mean that False is an appropriate value. And what about custom fields that use the same "built-in default" pattern? There's also a possible edge case where a field strictly isn't required (e.g., it's blankable) but from a business logic perspective, you always want to have a value. Ultimately, I came to the conclusion that the mess was worth it. That said, I'm not 100% opposed to cleaning it up. Essentially, if someone can make createsuperuser work without the need for REQUIRED_FIELDS, and the code is robust and isn't a mess, I'm willing to look at the patch before 1.5 final. However, I'm not planning on looking at this problem myself.
- aptwebapps 14y ago
  "I'm the author of the feature ... " Thank you.

The whole USERNAME_FIELD / REQUIRED_FIELDS thing feels clunkier that it should be. Isn't defining REQUIRED_FIELDS redundant, since this information could be introspected from the fields?

pydanny 14y ago

I agree it's a bit clunky, but... Making REQUIRED_FIELDS figure itself out from introspection means we all risk getting hurt by unanticipated edge cases. I would rather deal with a small bit of clunk than magical code touching my user model.

hcarvalhoalves 14y ago
The point is: what does REQUIRED_FIELDS do? Model validation? Form validation? Can you put a blank=True field on REQUIRED_FIELDS? Can you not put a blank=False on REQUIRED_FIELDS? It's duplicating behavior in two different places, and it's not entirely clear what it does, or what overrides what. This is much worse than introspection, because you'll have to patch Django to fix any inconsistencies instead of just changing your model definition / inheriting forms accordingly.
- pydanny 14y ago
  See https://docs.djangoproject.com/en/dev/topics/auth/#django.contrib.auth.models.CustomUser.User.REQUIRED_FIELDS https://docs.djangoproject.com/en/dev/topics/auth/#django.co... The quick version is that it's not duplicating behavior, the documentation is simply still undergoing work. We'll try and make it a bit more clear in the days to come. The long version is that during user creation sometimes you have fields that for whatever reason are not required by the database but are required for the user to enter in. This is because user creation is often more complex than it seems on the surface, and there is already existing django.contrib.auth.views that need this sort of configuration. REQUIRED_FIELDS gives you the control to force the user to enter data while not not altering the database. Is it ideal? Not really. But having dealt with user creation through email, OAuth, OpenID, LDAP, SOAP, and worse, having this as a configurable setting is better than an auto-magic introspection. Finally, I'll return to the documentation issue. I'm officially duly noting that the documentation on this feature needs work. I've opened ticket #19402 to address this issue. See https://code.djangoproject.com/ticket/19402 https://code.djangoproject.com/ticket/19402
  - ProblemFactory 14y ago
    > The long version is that during user creation sometimes you have fields that for whatever reason are not required by the database but are required for the user to enter in. This is because user creation is often more complex than it seems on the surface, and there is already existing django.contrib.auth.views that need this sort of configuration. REQUIRED_FIELDS gives you the control to force the user to enter data while not not altering the database. Could you please explain this further? Is REQUIRED_FIELDS intended for use cases where some users are created by import/migration, but others by sign-up forms (with more requirements)? From grepping the 1.5beta1 source code, it appears that the only place where REQUIRED_FIELDS is actually read is in contrib/auth/management/commands/createsuperuser.py.
    - pydanny 14y ago
      Rather than get into a huge discussion here, I'm going to spend my time working the ticket I mentioned. This isn't production ready code/documentation, it's a BETA release. Your issue has been noted and we are digging in. :-)

freakboy3742 14y ago

I'm the author of the feature, so I figured I should give some explanation. REQUIRED_FIELDS is used internally in one place -- in the createsuperuser management command. When you run that command, you are prompted for the minimal set of fields to create a user object. REQUIRED_FIELDS is that list of fields. Could this be used elsewhere? Sure. A model form, for example, could use this list as the minimal data to create a user object. Could this be replaced with some model introspection? Possibly. On my first pass through, this is exactly what I tried to do. And the code got messy really quickly. At a first approximation, any field with blank=False and no default is essentially "required". However, what about BooleanFields? BooleanFields have an automatically default to False, but that doesn't necessarily mean that False is an appropriate value. And what about custom fields that use the same "built-in default" pattern? There's also a possible edge case where a field strictly isn't required (e.g., it's blankable) but from a business logic perspective, you always want to have a value. Ultimately, I came to the conclusion that the mess was worth it. That said, I'm not 100% opposed to cleaning it up. Essentially, if someone can make createsuperuser work without the need for REQUIRED_FIELDS, and the code is robust and isn't a mess, I'm willing to look at the patch before 1.5 final. However, I'm not planning on looking at this problem myself.

aptwebapps 14y ago
"I'm the author of the feature ... " Thank you.