Hi,
This discussion does not seems to attract much attention but I still
feel this is a nice improvement to have.
/Cordialement/Regards,
Christophe Henry/
—
Christophe Henry
https://christophe-henry.dev/
Dev. Python/JS/Android & devops
Timon-Ethics / Coopaname <http://www.coopaname.coop/>
------------------------------------------------------------------------
Le 08/10/2021 à 13:55, Christophe Henry a écrit :
Hello Django developpers,
Following the discussion I had with Mariusz Felisiak on PR #14913
<https://github.com/django/django/pull/14913/files/f7492ae19586e6fd76e235cbbe902b5da657423e#diff-e5abf5944f650c7b4cebaee079168b66a303f57e96c2cf441a0a5a0251e68524>,
I would like to suggest a small change in the way the createsuperuser
command processes values for foreign key fields of custom User models.
This all started as I was trying to override the createsuperuser
command for a Django project I work on. This project has a custom user
model with a FK field named `organisation`. As I was working on that,
I noticed that, if I passed an instance of the Organisation model for
the field organisation, I got a TypeError because createsuperuser
performs a field.clean on the value which tries to convert a model
into an integer. But if I pass an integer, I get a ValueError, because
later in the code, the command calls the the AbstractUser constructor
which expects an instance of Organisation for the organisation field.
It felt like a bug, so I opened #33151
<https://code.djangoproject.com/ticket/33151> and proposed to fix this
in a PR that simply wraps the input value in a model
<https://github.com/django/django/pull/14913/files#diff-008fad28cbf57009885eb4ba44f8460c9d58088af8bd9be3db2046cac37744f1R142>.
But, during the review, Mariusz Felisiak informs me that this
behaviour is documented
<https://docs.djangoproject.com/en/3.2/topics/auth/customizing/#django.contrib.auth.models.CustomUserManager>
as it is advised to always write a custom manager with the create_user
and create_superuser methods and any change here is a breaking change
that should be discussed on the developer's mailing list. Which is why
I would like to make my case here.
I would like to stress how easy it is to miss that part of the
documentation and run into this unexpected behaviour. Everywhere in
the framework, when I use the name of an FK field, an instance of the
model is expected. This is true for model constructors and every
method of the ORM. Which is why it was disturbing to me seeing
createsuperuser break this convention and use the name of the FK field
to pass the primary key.
This means, every time I will use a custom user model in combination
to REQUIRED_FIELDS, I will have to write a custom manager with
create_user and create_superuser methods that do this:
class CustomUserManager(UserManager):
def __normalize_fields(self, extra_fields: dict):
for field_name in extra_fields.keys():
field = self.model._meta.get_field(field_name)
field_value = extra_fields[field_name]
if field.many_to_one and not isinstance(
field_value, field.remote_field.model
):
field_value = (
int(field_value)
if not isinstance(field_value, int)
else field_value
)
extra_fields[field_name] =
field.remote_field.model(field_value)
def create_user(self, username, email=None, password=None,
**extra_fields):
self.__normalize_fields(extra_fields)
return super().create_user(username, email, password,
**extra_fields)
def create_superuser(self, username, email=None,
password=None, **extra_fields):
self.__normalize_fields(extra_fields)
return super().create_superuser(username, email, password,
**extra_fields)
Even though it's this only thing it does. IMHO, the createsuperuser
command could be — and should be — doing this itself. BTW, it's what's
being done in interactive mode
<https://github.com/django/django/blob/main/django/contrib/auth/management/commands/createsuperuser.py#L142>
to check the validity of the user input, even though the wrapping is
discarded immediatly after and only the raw value is transmitted. I
don't really see why the command performs a check in this situation
but not always.
--
/Cordialement/Regards,
Christophe Henry/
—
Christophe Henry
https://christophe-henry.dev/
Dev. Python/JS/Android & devops
Timon-Ethics / Coopaname <http://www.coopaname.coop/>
------------------------------------------------------------------------
--
You received this message because you are subscribed to the Google Groups "Django
developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-developers/d04106db-a23f-a7a4-7673-4589d0aabb34%40timon-ethics.fr.
