Re: AbstractUser to get more abstract
Hi all - I updated Russ's new wiki page to include the work I've done so far: https://code.djangoproject.com/wiki/ContribEmailAuth Again, the patch I've been working on is here: https://github.com/tanderegg/django/tree/ticket_20824_master Please let me know if you have any feedback. The only real other option (that I can see) would be to do something more extensive like django-authtools ( https://django-authtools.readthedocs.org/en/latest/). I can write up a description of this approach as well if that would be useful, although I did write up a description of the difference between my code and the two other existing django apps in a post earlier in this thread. Thanks, Tim On Wed, Sep 18, 2013 at 1:42 AM, Russell Keith-Magee < russ...@keith-magee.com> wrote: > > On Wed, Sep 18, 2013 at 1:27 PM, Luke Sneeringer wrote: > >> Russell, >> I would *love* to do the work for the email-login analogue you describe. >> I actually proposed just such a thing a few months ago but was rebuffed. >> > > I'm sorry to hear this. Out of interest, did a member of the core team > actually say "no", or was it just a matter of proposing something and not > getting traction? > > If it was the latter, it's important to remember that the core team are > all volunteers, and sometimes the spare time of the core team doesn't > necessarily match up with the spare time of volunteers in the wider > community. As a result, well intentioned and desired work sometimes gets > ignored. It's not (necessarily) being ignored because it was bad -- often > it's just "we don't have enough cycles *right now*. > > >> However, I think this would be extremely useful. Also, I am, in fact, >> looking to get involved with Django development, as I haven't quite >> navigated the hurdles successfully. >> >> I do have one request, though. Is there a core developer that would be >> willing to "mentor" my work on this, so I can make sure I am writing >> something worthy of acceptance? >> > > Well, I'm willing to mentor the effort. > > Like I've indicated, the first step isn't to write code at all - it's to > get a good summary of the state of play of existing implementations. > Multiple people have already taken a swing at an implementation; before we > commit to one particular codebase, we need to understand what has already > been offered by the community. > > Yours, > Russ Magee %-) > > -- > You received this message because you are subscribed to a topic in the > Google Groups "Django developers" group. > To unsubscribe from this topic, visit > https://groups.google.com/d/topic/django-developers/rb7v9kVAK3I/unsubscribe > . > To unsubscribe from this group and all its topics, send an email to > django-developers+unsubscr...@googlegroups.com. > To post to this group, send email to django-developers@googlegroups.com. > Visit this group at http://groups.google.com/group/django-developers. > For more options, visit https://groups.google.com/groups/opt_out. > -- You received this message because you are subscribed to the Google Groups "Django developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at http://groups.google.com/group/django-developers. For more options, visit https://groups.google.com/groups/opt_out.
Re: AbstractUser to get more abstract
Hi Luke - I just wanted to clarify the approach I'm using - The issue of whether or not the EmailUser is in contrib.auth or in a new app contrib.auth_email is a separate issue from code duplication. Either way, you'd have to create new versions of UserCreationForm, UserChangeForm, UserAdmin, and UserManager, because each of those classes will only work with the default auth.User model (as currently written). So there's really two independent questions at play here: 1) Should a new EmailUser model be in contrib.auth or contrib.auth_email? Based on the current recommended way to use swappable, EmailUser should be in its own app contrib.auth_email. This could be changed by "blessing" the usage of swappable that Aaron was talking about (which does seem to work), in which case EmailUser could be in the contrib.auth app without conflicts. Testing would have to be done to make sure this works in all cases. 2) Should contrib.auth undergo refactoring to reduce the amount of code duplication required to create a custom user model? This would require modifying the two forms, admin class, and user manager to support custom models without further subclassing (except to add new functionality) or replacement. I did this for UserManager in my alternative patch ( https://github.com/tanderegg/django/compare/ticket_20824_refactor_master), which was fairly straightforward although a little awkard since the current public api requires a separate parameter passed to create_user for email, even if 'email' is also being used as the username. The forms and admin class would be a bit harder, it could be done but the result would probably be a bit convoluted, and they still would not cover all use cases of course. Better would be to create abstract base classes for those forms and UserAdmin that cover essential functionality, and then allow custom models to inhereit from those, which would reduce code duplication fairly substantially. If we used abstract base classes though, the custom user model would still require defining custom forms and admin classes as well, they would just be much simpler. -Tim On Fri, Sep 20, 2013 at 2:47 PM, Luke Sneeringer wrote: > > On Sep 20, 2013, at 10:58 AM, gavinw...@gmail.com wrote: > > > > No other User model needs to [set swappable] > > > > This would still be the case. Only models that want to conditionally > load themselves would set swappable. User models in application code > probably wouldn't set it, because the project will always use that one user > model. > > This is my understanding too. If you set a user model in individual > project code, there's no point in defining it as swappable, as that project > is always going to want to use it. > > If you're writing the authtools app, you probably do want to define the > user model or models you offer as swappable, but that seems like it's still > true now. > > > > ... made a special case of "Swappable models in the same app". > > > > I'm not sure where there's a special case. Swappable works for this > without any modifications, see authtools. > > > > > *Any* model can be swapped in as a substitute user. > > > > Yep. Nothing needs to change to keep this possible. > > > > > If we were to go down this path, the logical extension (to my mind) > would be to validate that you're not swapping in a model that hasn't > declared itself as swappable > > > > Why would you want to validate this? Swappable only controls loading of > the model its set on, there is no action-at-a-distance on other models. > > I, too, don't really understand this assumption. The status quo works just > fine, and makes complete sense to me. > > > I don't think we should focus so much on the optional part of the > proposal. The really important part is to refactor the existing code to > work better with custom users. Once this happens, projects like authtools > can trivially add an EmailUser in a few lines of code, so it's not so > important that there's one in core. > > Eh, I pretty passionately believe that this particular use case needs to > be in core. It's **really** common. But, the point that you make is why I > am passionate about not just making an auth_email app, because I see the > other approach as *also* having more value in simplifying some other > more-simple user substitutes. > > L > > -- > You received this message because you are subscribed to a topic in the > Google Groups "Django developers" group. > To unsubscribe from this topic, visit > https://groups.google.com/d/topic/django-developers/rb7v9kVAK3I/unsubscribe > . > To unsubscribe from this group and all its topics, send an email to > django-developers+unsubscr...@googlegroups.com. > To post to this group, send email to django-developers@googlegroups.com. > Visit this group at http://groups.google.com/group/django-developers. > For more options, visit https://groups.google.com/groups/opt_out. > -- You received this message because you are subscribed to the Google Groups "Django develop
Re: AbstractUser to get more abstract
>
>
>
>- I believe that we should add an AbstractEmailUser as a subclass of
>AbstractBaseUser, and make AbstractUser subclass it.
> - So, right now we have AbstractBaseUser -> AbstractUser -> User.
> - My desired hierarchy would be:
> - AbstractBaseUser
> - AbstractEmailUser
>- AbstractUser
> - User
>- EmailUser
> - I assume this is mostly intuitive, but under my proposed
> hierarchy, everything that AbstractUser has now would be moved to
> AbstractEmailUser *except for* the username field.
> AbstractEmailUser would have USERNAME_FIELD set to 'email', and
> REQUIRED_FIELDS set to empty tuple. AbstractUser would have
> USERNAME_FIELD
> set to 'username' and REQUIRED_FIELDS set to ('email',).
> - Both User and EmailUser would have the swappable Meta option set
> to 'AUTH_USER_MODEL'.
>
>
One thing to note here, is that AbstractEmailUser would have to define the
email field with unique=True in order for it to be used as USERNAME_FIELD.
This would mean that AbstractUser and User would fundamentally change so
that email has to be unique. This could definitely wreck havoc with
existing apps that don't have this requirement.
One solution would be to have an intermediate class (AbstractNamedUser or
something) that both AbstractEmailUser and AbstractUser inherit from, and
each would define their own email fields. The other would be to stub out
the email field (email = '') on AbstractEmailUser and actually implement it
in each descendent class. I like the former approach myself, since it
abstracts out name handling and would make it easier to create custom users
that handle names differently. It might make sense to put
AbstractEmailUser above AbstractNamedUser in the inheritance chain, and use
the stubbing technique too.
>
>- I would then propose taking forms that hard-declare username as a
>field (e.g. UserCreationForm, but there are others) and instead determine
>their username field using the `fields` meta option in ModelForm.
> - An example in the case of the UserCreationForm would be: fields =
> (model.USERNAME_FIELD, 'password1', 'password2')
> - There's a potential land mine in here which is that the
> `clean_username` method is much less straightforward: we'd either have
> to
> determine the method name programmatically in class construction or get
> around the issue through double-defining it. I would favor the former
> because then the form is useful without modification to more third-party
> use cases. However, this is one of the (few, IMO) cases where there
> actually is a complexity hit, and I believe this problem should be
> noted as
> a disadvantage of my approach.
>
> The clean_username method is technically unnesseccary, it just provides a
better error message than the ORM would. There might be a way to
generalize this differently. The other issue is that username is hard
coded into these forms as a regex validation. That could perhaps be moved
to the AbstractUser class itself.
>
>- I haven't looked at views as thoroughly, but I believe them to be
>more straightforward. I will do this dive soon.
>
> In my experimentation, I haven't had to touch the views at all to make
things work. I may have missed something though!
The only other part that would have to change is UserAdmin, although it may
end up being simpler just to require a custom Admin class for every custom
user model, since it seems to me abstracting that would be tricky.
-Tim
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
For more options, visit https://groups.google.com/groups/opt_out.
