Split mixins between fields and methods

[kevinetienne]

Customising fields on BasicUserFieldsMixin is not easy. I have a case where I need to override
name and email.

My use case:

class User(AvatarMixin, VerifyEmailMixin, PermissionsMixin, AbstractBaseUser):
    pass

With the current implementation the solution is to copy VerifyEmailMixin and its parent class
BasicUserFieldsMixin to redefine name and email fields to avoid name clashes.

It looks like splitting the current mixins to more mixins will allow redefining fields more easily.
I suggest to make BasicUserFieldsMixin and VerifyEmailMixin children classes of some fields and methods.

Example:

class BasicUserFieldsMixin(IsStaffMixin, NameMixin, NameMethodsMixin):
    pass

class VerifyEmailMixin(BasicUserFieldsMixin, IsActiveMixin, EmailVerificationMixin, VerifyEmailMethodsMixin):
   pass

Which would allow us to create own BasicUserFieldsMixin and VerifyEmailMixin if one or more fields need changing.

[kevinetienne]

Having such a long chain of inheritance is maybe not the best solution, dealing with python mro and Django Meta class has proven not to be easy to use.

One of the downside of what I have created is that one should reimplement the checks when customising VerifyEmailMixin. Thanks @ian-foote

[kevinetienne]

@ian-foote @meshy could you have a look to this PR please?

[coveralls]

Coverage increased (+0.01%) when pulling 179bf9e on split-mixins-into-mixins into cf607ed on master.

[meshy]

Just to clarify: it's not possible to just subclass the parent, and re-define the fields?

[coveralls]

Coverage increased (+0.01%) when pulling 6f431b9 on split-mixins-into-mixins into cf607ed on master.

[kevinetienne]

@meshy unfortunately no :( The error message is the following: django.core.exceptions.FieldError: Local field 'name' in class 'User' clashes with field of similar name from base class 'VerifyEmailMixin' and then you try to subclass VerifyEmailMixin but same error :)

[kevinetienne]

it might be related to the Meta class generating something from the attributes on the class.

[coveralls]

Coverage increased (+0.01%) when pulling 6f431b9 on split-mixins-into-mixins into cf607ed on master.

[meshy]

Aaah, I guess it is. Shame. Ok then :)

[kevinetienne]

Relevant piece of code: https://github.com/django/django/blob/d66bda60590daabe21f60a532a613a31a10fedbd/django/db/models/base.py#L229-L238 (might be fixed in the future)

[LilyFirefly]

What is this one for?

[mlen108]

I don't like this syntax :(

[kevinetienne]

If feels weird and it leaves a ): on a new line, flake8 enforce this syntax..

[LilyFirefly]

An alternative is:

basic_user_mixins = (
    NameUserMethodsMixin,
    EmailUserMixin,
    DateJoinedUserMixin,
    IsStaffUserMixin,
)

class CustomBasicUserFieldsMixin(*basic_user_mixins):
    ...

[kevinetienne]

Thanks @ian-foote I like this one a lot better

[mlen108]

the ): does not need to be on new line tho

[kevinetienne]

Unfortunately the syntax with the splat produces an invalid syntax error.

[mlen108]

Who do you respond to? :)

[kevinetienne]

my last message was for @ian-foote

[coveralls]

Coverage increased (+0.01%) when pulling d1e4b30 on split-mixins-into-mixins into 8c4649a on master.

[kevinetienne]

@ian-foote @mattack108 can you merge please?