[Django] #21832: allow USERNAME_FIELD to be a ForeignKey
Django
-------------------------+-------------------------------------------------
Reporter: | Owner: nobody
cjerdonek | Status: new
Type: New | Version: master
feature | Keywords: USERNAME_FIELD,ForeignKey,foreign
Component: | key,auth,username
contrib.auth | Has patch: 0
Severity: Normal | UI/UX: 0
Triage Stage: |
Unreviewed |
Easy pickings: 0 |
-------------------------+-------------------------------------------------
This issue is to add support for `USERNAME_FIELD` being a `ForeignKey`.
Currently, this isn't supported because, for example, the
`createsuperuser` command doesn't support inputting the corresponding
foreign key data (also see #21755 for another issue with the
`createsuperuser` command needed for this).
One use case for `USERNAME_FIELD` being a `ForeignKey` is if the username
is an email address, and you would like to have a separate table for
e-mail address related data (e.g. total e-mails sent, last e-mail sent,
etc), as well as allow changing the email address on a user account.
--
Ticket URL: <https://code.djangoproject.com/ticket/21832>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: cjerdonek | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage:
USERNAME_FIELD,ForeignKey,foreign | Unreviewed
key,auth,username | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by cjerdonek):
* cc: chris.jerdonek@… (added)
* needs_docs: => 0
* needs_tests: => 0
* needs_better_patch: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/21832#comment:1>
Django
Reporter: cjerdonek | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage:
USERNAME_FIELD,ForeignKey,foreign | Unreviewed
key,auth,username | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by anubhav9042):
Can we change the behaviour of USERNAME_FIELD
from
{{{
USERNAME_FIELD = 'somefield'
}}}
to
{{{
USERNAME_FIELD = ('somefield',)
USERNAME_FIELD = ('somefield','somemodel')
}}}
In this way, we can get to know the model in superuser command and we can
then pass an appropriate model_instance
(The tuple's second item being optional, ofc)
Thoughts??
--
Ticket URL: <https://code.djangoproject.com/ticket/21832#comment:2>
Django
Reporter: cjerdonek | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
USERNAME_FIELD,ForeignKey,foreign | Needs documentation: 0
key,auth,username | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by akaariai):
* stage: Unreviewed => Accepted
Comment:
There seems to be three different use cases here:
- "Local FK" - USERNAME_FIELD points to local foreign key, and the value
for that foreign key can be used to create the remote model. Example:
USERNAME_FIELD = 'email', email points to class Email's field 'email', and
that is the only required field of Email. This should be relatively easy
to support.
- "Remote field's value". Like above, but now the user model's email
field points to Email model's id field, but field 'email' of the remote
model is also required. This is harder to support, we need more logic so
that we know to ask the email for the related model. Syntax could be
USERNAME_FIELD = `'email__email'`.
- Remote model with multiple required fields. We need support for
validating & creating the remote model before we create the user model.
Support for REQUIRED_FIELDS = `'email__otherfield'` is required, and all
the logic to then create the related models, validation and so on.
I am marking this accepted, but I think we should only support the first
case above. It should be relatively straightforward to support. The other
cases seem to be complex and I am not sure they are worth the added value.
--
Ticket URL: <https://code.djangoproject.com/ticket/21832#comment:3>
Django
Reporter: cjerdonek | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
USERNAME_FIELD,ForeignKey,foreign | Needs documentation: 0
key,auth,username | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by anubhav9042):
I have had conversation regarding this issue with @loic and @akaariai.
I hope to work on this in GSoC this year(i.e if I am accepted).
--
Ticket URL: <https://code.djangoproject.com/ticket/21832#comment:4>
Django
Reporter: cjerdonek | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
USERNAME_FIELD,ForeignKey,foreign | Needs documentation: 0
key,auth,username | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* cc: loic@… (added)
Comment:
@akaariai summed up the situation pretty well, but to make sure everyone
is on the same page as it took us some time to figure it out on IRC:
- "Local FK":
{{{#!python
class Email(models.Model):
email = models.CharField(max_length=254)
class User(models.Model):
email = models.ForeignKey(Email, to_field='email')
USERNAME_FIELD = 'email'
}}}
- "Remote field's value":
{{{#!python
class Email(models.Model):
email = models.CharField(max_length=254)
class User(models.Model):
email = models.ForeignKey(Email)
USERNAME_FIELD = 'email_email'
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21832#comment:5>
Django
Reporter: cjerdonek | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
USERNAME_FIELD,ForeignKey,foreign | Needs documentation: 0
key,auth,username | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by cjerdonek):
For the record, in my use case the email address model has two required
fields: a "normalized" email address with the `unique` option (think
lower-cased, for example) and a non-normalized email address to store the
user's preferred spelling/casing. This lets uniqueness be enforced at the
database level. The user manager's `get_by_natural_key()` method is
overridden to normalize the given email address before querying for a
user.
Is the issue with the other cases only the creation phase? It seems like
an API to construct a model object from required fields might be worth
having anyways. Such an API could handle foreign keys recursively.
In any case, it would be good if any commits related to this issue do not
preclude or make it harder to solve more general use cases like the one I
outlined.
--
Ticket URL: <https://code.djangoproject.com/ticket/21832#comment:6>
Django
Reporter: cjerdonek | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
USERNAME_FIELD,ForeignKey,foreign | Needs documentation: 0
key,auth,username | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by russellm):
Question: Other than createsuperuser, is there anything currently
preventing this from working?
If the problem is just createsuperuer, would an acceptable fix to bail out
(gracefully) from createsuperuser if USERNAME_FIELD can't be gathered at
the command line, and leave it up to the user to open a shell and use
UserModel.objects.create_superuser()?
--
Ticket URL: <https://code.djangoproject.com/ticket/21832#comment:7>
Django
Reporter: cjerdonek | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
USERNAME_FIELD,ForeignKey,foreign | Needs documentation: 0
key,auth,username | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by cjerdonek):
> Other than createsuperuser, is there anything currently preventing this
from working?
Yes, pretty much anywhere that `USERNAME_FIELD` appears in the code needs
to be reviewed, for example `AbstractBaseUser.get_username()` and
`RemoteUserBackend.authenticate()`. In my experience, though, most of
these can be dealt with through straightforward subclassing and
overriding. However, the `createsuperuser` command was the most awkward
to deal with. I wound up overriding the command, but the solution I'm
currently using is brittle and somewhat of a hack (relying on internal
details, etc).
My hope for this issue is to reduce these barriers. In other words, I
wouldn't mind if foreign keys aren't supported out of the box, but it
would be nice if it were easier to make things work (and in a way that's
more likely to work in future versions of Django, etc).
--
Ticket URL: <https://code.djangoproject.com/ticket/21832#comment:8>
Django
Reporter: cjerdonek | Owner:
Type: New feature | anubhav9042
Component: contrib.auth | Status: assigned
Severity: Normal | Version: master
Keywords: | Resolution:
USERNAME_FIELD,ForeignKey,foreign | Triage Stage: Accepted
key,auth,username | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by anubhav9042):
* status: new => assigned
* owner: nobody => anubhav9042
Comment:
Will work on this in my GSoC project
--
Ticket URL: <https://code.djangoproject.com/ticket/21832#comment:9>
Django
Reporter: cjerdonek | Owner:
Type: New feature | anubhav9042
Component: contrib.auth | Status: assigned
Severity: Normal | Version: master
Keywords: | Resolution:
USERNAME_FIELD,ForeignKey,foreign | Triage Stage: Accepted
key,auth,username | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by anubhav9042):
Hi, I was going through the comments and documentation and I have a
question before I attempt to start this:
Why it says here in the docs for
[https://docs.djangoproject.com/en/dev/topics/auth/customizing/#django.contrib.auth.models.CustomUser.REQUIRED_FIELDS
REQUIRED_FIELDS] that "However, it will not work for `ForeignKey` fields."
Because this ticket is related to #21755 and I think when we are making
`USERNAME_FIELD` to allow `FK`, why not `REQUIRED_FIELDS`? Is there some
specific purpose as to why this is done?
--
Ticket URL: <https://code.djangoproject.com/ticket/21832#comment:10>
Django
Reporter: cjerdonek | Owner:
Type: New feature | anubhav9042
Component: contrib.auth | Status: assigned
Severity: Normal | Version: master
Keywords: | Resolution:
USERNAME_FIELD,ForeignKey,foreign | Triage Stage: Accepted
key,auth,username | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by timo):
We would probably need to fix that, yes. The documentation simply warns
that it doesn't currently work; there is no reason I know of not to try to
make it work.
--
Ticket URL: <https://code.djangoproject.com/ticket/21832#comment:11>
Django
Reporter: cjerdonek | Owner:
Type: New feature | anubhav9042
Component: contrib.auth | Status: assigned
Severity: Normal | Version: master
Keywords: | Resolution:
USERNAME_FIELD,ForeignKey,foreign | Triage Stage: Accepted
key,auth,username | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by timo):
Chris, we just committed a fix for #21755 and it's not clear to me what
additional work, if anything, should be done here. I believe the "local
FK" situation described above should work for `createsuperuser` (Anubhav
is confirming), although it requires an existing instance of that FK to
already exist. I'm not sure the effort enhancing `createsuperuser` to
allow model creation for the ForeignKeys themselves is really worth it as
I expect `createsuperuser` isn't heavily used besides creating an initial
user that can be used the login to the admin. What do you think?
--
Ticket URL: <https://code.djangoproject.com/ticket/21832#comment:12>
Django
Reporter: cjerdonek | Owner:
Type: New feature | anubhav9042
Component: contrib.auth | Status: assigned
Severity: Normal | Version: master
Keywords: | Resolution:
USERNAME_FIELD,ForeignKey,foreign | Triage Stage: Accepted
key,auth,username | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by cjerdonek):
Tim, I think you're right that it's probably not worth the effort at this
point to add full-blown support to `createsuperuser`. However, I think
the following should still be done:
1) Confirm that a good error message is being given if the FK instance
doesn't exist (e.g. if someone tries running `createsuperuser` without an
awareness of its current limitations.
2) Provide a clear way for the `createsuperuser` command to be
customized/overridden to support the case of the FK instance not existing
(e.g. by adding a hook of some sort to the public API).
I originally filed this issue because of the pain of doing (2) myself.
The approach I used was to create a command of the same name in a
different app that "shadows" the current command. The new command sub-
classed the current command. Prior to calling the base class's `handle()`
method, it runs code to prompt the user to enter the needed data to create
the FK instance. This approach was hard to come by and is brittle
because, for instance, it relies on internal details of the API. If I
recall, even the "shadowing" of a command is brittle because it depends on
the order of `INSTALLED_APPS` if I remember right.
Also, question: with the current implementation, how would the user
specify the FK instance on the command-line? Using the (integer) ID?
--
Ticket URL: <https://code.djangoproject.com/ticket/21832#comment:13>
Django
Reporter: cjerdonek | Owner:
Type: New feature | anubhav9042
Component: contrib.auth | Status: assigned
Severity: Normal | Version: master
Keywords: | Resolution:
USERNAME_FIELD,ForeignKey,foreign | Triage Stage: Accepted
key,auth,username | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by anubhav9042):
Updated the prompt and existing tests.
Also added a test to show that checks are working fine.
--
Ticket URL: <https://code.djangoproject.com/ticket/21832#comment:14>
Django
Reporter: cjerdonek | Owner:
Type: New feature | anubhav9042
Component: contrib.auth | Status: assigned
Severity: Normal | Version: master
Keywords: | Resolution:
USERNAME_FIELD,ForeignKey,foreign | Triage Stage: Accepted
key,auth,username | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by anubhav9042):
Replying to [comment:13 cjerdonek]:
> Tim, I think you're right that it's probably not worth the effort at
this point to add full-blown support to `createsuperuser`. However, I
think the following should still be done:
>
> 1) Confirm that a good error message is being given if the FK instance
doesn't exist (e.g. if someone tries running `createsuperuser` without an
awareness of its current limitations.
> 2) Provide a clear way for the `createsuperuser` command to be
customized/overridden to support the case of the FK instance not existing
(e.g. by adding a hook of some sort to the public API).
>
> I originally filed this issue because of the pain of doing (2) myself.
The approach I used was to create a command of the same name in a
different app that "shadows" the current command. The new command sub-
classed the current command. Prior to calling the base class's `handle()`
method, it runs code to prompt the user to enter the needed data to create
the FK instance. This approach was hard to come by and is brittle
because, for instance, it relies on internal details of the API. If I
recall, even the "shadowing" of a command is brittle because it depends on
the order of `INSTALLED_APPS` if I remember right.
>
> Also, question: with the current implementation, how would the user
specify the FK instance on the command-line? Using the (integer) ID?
>
>
If the instance doesn't exists, appropriate error is raised by call to
field's `clean()` method.
eg.
{{{
Email instance with pk 3 does not exists.
}}}
On the command line, the value of `to_field` is needed which is `pk` or
`id` by default. This has been specified clearly in the docs. Also the
prompt at the command line will specify which the `to_field`'s(`id` by
default) name to be entered.
eg.
{{{
Username (Email.id)
or
Username (Email.email)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21832#comment:15>
Django
Reporter: cjerdonek | Owner:
Type: New feature | anubhav9042
Component: contrib.auth | Status: assigned
Severity: Normal | Version: master
Keywords: | Resolution:
USERNAME_FIELD,ForeignKey,foreign | Triage Stage: Accepted
key,auth,username | Needs documentation: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
https://github.com/django/django/pull/2880
--
Ticket URL: <https://code.djangoproject.com/ticket/21832#comment:16>
Django
Reporter: cjerdonek | Owner:
Type: New feature | anubhav9042
Severity: Normal | Version: master
Keywords: | Resolution: fixed
USERNAME_FIELD,ForeignKey,foreign | Triage Stage: Accepted
key,auth,username | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"75ff7b8fb8335e181ad05e1d4244b4295cc7a105"]:
{{{
#!CommitTicketReference repository=""
revision="75ff7b8fb8335e181ad05e1d4244b4295cc7a105"
Fixed #21832 -- Updated prompt, tests, and docs to show that
USERNAME_FIELD supports FK after 9bc2d76.
Also added get_input_data() hook in createsuperuser.
Thanks Chris Jerdonek and Tim Graham for review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21832#comment:17>