On Sat, Sep 15, 2012 at 11:59 PM, Anssi Kääriäinen <[email protected]> wrote: > On 15 syys, 15:34, Russell Keith-Magee <[email protected]> > wrote: >> Hi all, >> >> The implementation of custom User models is now ready for final >> review, prior to commit to trunk. >> >> The code is available in my Github repo, in the t3011 branch. >> >> https://github.com/freakboy3742/django/tree/t3011 >> >> The diff is available here: >> >> https://github.com/freakboy3742/django/compare/master...t3011 >> >> Documentation is also available in the auth topic guide: >> >> https://github.com/freakboy3742/django/blob/t3011/docs/topics/auth.tx... >> >> Barring the discovery of major problems, I'm aiming to commit this >> towards the end of next week. Any and all feedback is welcome. > > The above branch does not work on Python3, which seems like a blocker > issue.
Dang - I knew I forgot something. I'll put this on my list to look at. > Some questions: > - If I proxy the auth.User model when it is swapped out I get an > error from model validation: "AttributeError: type object 'UserProxy' > has no attribute '_base_manager'". The error could be cleaner. > - The last_login field is in the AbstractBaseUser, but it isn't > documented as a required field. Is this field required for something? > Is it needed as part of AbstractBaseUser? Yes, last_login is required - it's needed in order to generate password reset tokens etc. It isn't documented because we *really* want people to be subclassing AbstractBaseUser, not building their own User from scratch. > - There is no way to do single-table extension of the default User > model. Should there be a way? To me it seems this would be beneficial > when dealing with "project profiles". There is little reason I would > like to store a required employee number in another table. Maybe it > would work if current User was moved to AbstractUser, and User would > be class User(AbstractUser): class Meta: swappable = > 'AUTH_USER_MODEL'? Quick testing indicates this could work... See > commit: > https://github.com/akaariai/django/commit/08bcb4aec1ed154cefc631b8510ee13e9af0c19d > - I did a little change to REQUIRED_FIELDS handling in conjunction > with create_(super)user(). See commit: > https://github.com/akaariai/django/commit/d9f5e5addbad5e1a01f67e7358e4f5091c3cad81 > > With the above commits I can do this: > > class MyUser(AbstractUser): > employee_no = models.CharField(max_length=5) > some_other_field = models.TextField(null=True, blank=True) > REQUIRED_FIELDS = AbstractUser.REQUIRED_FIELDS + ['employee_no'] > > syncb & create_superuser commands work. Admin doesn't work directly, > but it seems you can easily subclass the default user admin & change > form to make it work. > > Single-table inheritance seems useful to me, and the added complexity > to code isn't much. To be clear -- is this just targeting the "I'm completely happy with auth.User; I just want to put XXX on the model" use case? If so, I can certainly agree with this; I'll merge these changes in. > The above questions aren't meant to be commit blockers. In my > (somewhat limited) testing the only real issue was the py3 > incompatibility, and that seems to be an easy fix. Overall, this looks > good to me. > > - Anssi > > BTW: I did also a little test to allow select_related() for user > profiles: > https://github.com/akaariai/django/commit/81f91db3234110b90572b25ca69b9012a475c460 > - the idea is that the profile models could just to > get_user_model().SELECT_RELATED_MODELS.add('user_reverse_name'). I > wonder if we want something like this now/later... Possibly; but if we do, I'd rather attack it at the model Meta level -- i.e., an analog to Meta: ordering that applies a select_related automatically to every queryset. Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
