Opened 11 years ago

Closed 5 years ago

Last modified 5 years ago

#20629 closed Cleanup/optimization (fixed)

Admonitions in custom User model documentation may be scaring off users

Reported by: Russell Keith-Magee Owned by: Tobias Kunze
Component: Documentation Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Evan Stone on django-users indicated that he was initially hesitant to use custom User models because the documentation made it sound like it was going to be painful. Specifically, he identified the following pieces of documentation:

  1. "Think carefully before handling information not directly related to authentication in your custom User Model."
  1. "It may be better to store app-specific user information in a model that has a relation with the User model. That allows each app to specify its own user data requirements without risking conflicts with other apps...."
  1. "One limitation of custom User models is that installing a custom User model will break any proxy model extending User. ..."

and

  1. "Another limitation of custom User models is that you can’t use django.contrib.auth.get_user_model() as the sender or target of "

Points 1 and 2 are asking users to consider an architectural question -- do you need a custom user model at all? If you actually *are* using a username-based login system, and you just want to track some extra information about the user, the right approach may *not* be to create a custom user model.

Points 3 and 4 are pointing out known limitations. Proxy models are a problem because they use subclassing, and they will be subclassing the wrong class; signals are a problem because at the point the signal is registered, there's no guarantee that the User model has been correctly defined. There's not much we can do about (3); (4) is something that should get cleaned up when we eventually land app refactor.

The points made by the documentation are all valid, but could perhaps be made in less threatening or alarmist tones, or clarified so that they don't seem like such big problems.

Change History (8)

comment:1 by Keith Edmiston, 11 years ago

Owner: changed from nobody to Keith Edmiston
Status: newassigned

comment:2 by Keith Edmiston, 11 years ago

Considering modification of the admonition section in the docs...from this:

Model design considerations

Think carefully before handling information not directly related to authentication in your custom User Model.

It may be better to store app-specific user information in a model that has a relation with the User model. That allows each app to specify its own user data requirements without risking conflicts with other apps. On the other hand, queries to retrieve this related information will involve a database join, which may have an effect on performance.

to this:

Model design considerations

A point to think about in your design/decision is whether you need a custom user model at all?

Before choosing to handle information not directly related to authentication in your custom User Model, consider that it may be a better choice to store app-specific user information in a model that has a relation with the User model. This approach allows each app to specify its own user data requirements without risking conflicts with other apps. Keep in mind that queries to retrieve this related information will involve a database join, which may have an effect on performance.


Let me know your thoughts and I'll make a pull request.

comment:3 by Russell Keith-Magee, 11 years ago

Looks like a good update to cover the first point, but points 2-4 are still relevant.

in reply to:  3 comment:4 by Keith Edmiston, 11 years ago

Replying to russellm:

Looks like a good update to cover the first point, but points 2-4 are still relevant.

I'm a bit confused then. I felt like the above suggestion "softened the edges" a bit for 1 & 2 both. Perhaps you can be more explicit about what it should look like in accommodating 2?

Reading your initial report above, I took it to mean that 3 wasn't something we could really do anything about here and 4 was more of a fix that will come in a later version.

Can you help me better understand how to change this to address all 4 points?

Last edited 11 years ago by Keith Edmiston (previous) (diff)

comment:5 by Tobias Kunze, 5 years ago

Owner: changed from Keith Edmiston to Tobias Kunze
Version: 1.5master

The fourth warning is no longer part of the docs. The third warning is still there, but I don't think it sounds alarmist or needs to be rephrased. I rephrased the admonition at the beginning of the custom user model implementation docs by putting the advantages first and the potential benefits of related models second.

comment:6 by Markus Holtermann, 5 years ago

Has patch: set
Triage Stage: AcceptedReady for checkin

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In eb16c726:

Fixed #20629 -- Rephrased custom user models admonition.

comment:8 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 54fcdf1:

[2.2.x] Fixed #20629 -- Rephrased custom user models admonition.

Backport of eb16c7260e573ec513d84cb586d96bdf508f3173 from master

Note: See TracTickets for help on using tickets.
Back to Top