Hi all, I've just pushed fixes that:
* Correct the patch for Python 3 * Merges Anssi's AbstractUser base class changes * Moves skipIfCustomUser to django.contrib.auth.tests.utils * Improves the error message when you have a Proxy to a swapped class * Updates the documentation to address the feedback received to date Based on the feedback on the patch so far, it looks like we're on track to merge this later this week. Yours, Russ Magee %-) On Sun, Sep 16, 2012 at 7:15 AM, Russell Keith-Magee <russ...@keith-magee.com> wrote: > On Sat, Sep 15, 2012 at 11:59 PM, Anssi Kääriäinen > <anssi.kaariai...@thl.fi> wrote: >> On 15 syys, 15:34, Russell Keith-Magee <russ...@keith-magee.com> >> 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 django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
