[Django] #20244: PermissionsMixin should define related name of groups and user_permissions related name

132 views
Skip to first unread message

Django

unread,
Apr 11, 2013, 7:04:10 AM4/11/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
------------------------------+--------------------
Reporter: bdauvergne | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 0 | UI/UX: 0
------------------------------+--------------------
Third party expecting a group.user_set or permission.user_set reverse
relationships are broken by custom user model whose class name is not
'User'.

An easy work around would be to add a related_name='user_set' to the
definitions of those two fields.

{{{
diff --git a/django/contrib/auth/models.py b/django/contrib/auth/models.py
index 5709d25..e0a2dba 100644
--- a/django/contrib/auth/models.py
+++ b/django/contrib/auth/models.py
@@ -294,10 +294,12 @@ class PermissionsMixin(models.Model):
groups = models.ManyToManyField(Group, verbose_name=_('groups'),
blank=True, help_text=_('The groups this user belongs to. A user
will '
'get all permissions granted to each of '
- 'his/her group.'))
+ 'his/her group.'),
+ related_name='user_set')
user_permissions = models.ManyToManyField(Permission,
verbose_name=_('user permissions'), blank=True,
- help_text='Specific permissions for this user.')
+ help_text='Specific permissions for this user.',
+ related_name='user_set')

class Meta:
abstract = True

}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/20244>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Apr 17, 2013, 3:41:31 AM4/17/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
------------------------------+--------------------------------------

Reporter: bdauvergne | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
------------------------------+--------------------------------------
Changes (by bdauvergne):

* needs_better_patch: => 0
* has_patch: 1 => 0
* needs_tests: => 0
* needs_docs: => 0


Comment:

My proposition is not good: when you do not use related_name you get
`user_set` for the name of the related field and `user` for the name of
the field to use in query filters, but if you defined `user_set` as the
related name, you will have `user_set` for the related field name but you
will also have `user_set` for the related field name in query filters
(example: `Q(group__user_set=user)`); that's no good ;(

So I maintain that custom user model break third party code expecting a
fixed name for the reverse relation between group and the user model
(exemple: django-cms); but that the current `related_name` option
behaviour does not allow to simulate the normal behaviour, so I have no
solution to propose.

--
Ticket URL: <https://code.djangoproject.com/ticket/20244#comment:1>

Django

unread,
May 18, 2013, 7:30:56 AM5/18/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
------------------------------+------------------------------------

Reporter: bdauvergne | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------
Changes (by lrekucki):

* stage: Unreviewed => Accepted


--
Ticket URL: <https://code.djangoproject.com/ticket/20244#comment:2>

Django

unread,
May 19, 2013, 7:40:03 AM5/19/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
------------------------------+------------------------------------

Reporter: bdauvergne | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------

Comment (by russellm):

For the record - this is a backwards incompatible change. However,
discussing this with the core devs at the DjangoCon sprints, we're
comfortable with making this change. The affected demographic is:

* people who have started a project on 1.5
* who have a custom user model
* that isn't named User
* who are doing their own internal modifications of groups and
permissions.

If this ticket had been raised pre 1.5, it would have been a no-brainer
change, so for the sake of practicality, we should do it now before this
is a deeply embedded API problem.

So - the patch for this needs to include a backwards compatibility entry
in the release notes.

--
Ticket URL: <https://code.djangoproject.com/ticket/20244#comment:3>

Django

unread,
May 20, 2013, 4:04:57 AM5/20/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
---------------------------------+------------------------------------

Reporter: bdauvergne | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Changes (by mjtamlyn):

* cc: marc.tamlyn@… (added)
* severity: Normal => Release blocker


Comment:

Upgrading this to a release blocker so it definitely gets done in 1.6.

--
Ticket URL: <https://code.djangoproject.com/ticket/20244#comment:4>

Django

unread,
May 20, 2013, 7:20:17 AM5/20/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
---------------------------------+------------------------------------

Reporter: bdauvergne | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Changes (by benjaoming):

* cc: benjaoming@… (added)


--
Ticket URL: <https://code.djangoproject.com/ticket/20244#comment:5>

Django

unread,
May 20, 2013, 9:16:13 AM5/20/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
---------------------------------+------------------------------------

Reporter: bdauvergne | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------

Comment (by russellm):

The core team discussion didn't cover the point raised in the first
comment - that reverse lookups in the ORM are an issue here, and that
didn't come up in the core team discussion.

The default handling names the attribute user_set, but lookups use
user!__XXX. However, when you move to an explicit related_name, that name
is used for both the attribute and the look kwarg. This means we'd
actually be breaking running code that was using reverse relations -- any
query on group!__user_set for example. We may need to take this to django-
dev for further discussion.

--
Ticket URL: <https://code.djangoproject.com/ticket/20244#comment:6>

Django

unread,
May 20, 2013, 10:56:40 AM5/20/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
---------------------------------+------------------------------------

Reporter: bdauvergne | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------

Comment (by benjaoming):

Hi Russell! Thanks for your attention to this. The process of supporting
the new custom user models as a reusable apps dev has been a bit of a
strain. Until further notice about where the dev discussion takes place,
here are my 2 cents...

The pitch: I think this issue and others affect the idea of models being
''swappable'' in the sense of that term. If we cannot achieve a proper
transparent swap, then all these swapped cases become hacks with nasty
implications. One example is `if django.VERSION>=(1,5):
User=get_user_model() else ...` in every place that's using a User model
for Django 1.5 -- can we even count that? :) In addition to that, we will
need conditions on everything that the swapped model does not
transparently let its swapped replacement do. This makes both forwards and
backwards compatibility become very painful.

We need the swapped model to behave like its swappable predecessor in
certain cases, and those specifically pertain some abstract class that we
have stated a contract about, e.g. AbstractBaseUser or AbstractUser.
Everytime we notice a case where a swapped user model that has all
features of AbstractUser and yet does not work, I would say should result
in a new issue.

The solution draft: I would propose that when related_name is not set, the
swapped out model should ''always'' dictate the name for both the related
attribute and query lookups. So, in case User has been swapped by
CustomUser, the automatic naming mechanism that triggers when related_name
is not set, will consider User and not CustomUser.

The cost: If this is introduced, then only existing custom user models
with their unset related_names will break -- and honestly, that's far less
severe than breaking everything else :)

Oh, and a related idea: In the future, consider a Warning to be fired when
defining a relation with a swappable/swapped model and there is no
explicit related_name.

--
Ticket URL: <https://code.djangoproject.com/ticket/20244#comment:7>

Django

unread,
May 21, 2013, 5:33:10 AM5/21/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
---------------------------------+------------------------------------

Reporter: bdauvergne | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------

Comment (by mjtamlyn):

I'm not convinced that any other swappable model with a foreign key
pointing away should behave any differently. If your pluggable application
depends on the behaviour of an relationship pointing away from `User` (so
already doesn't work with `contrib.auth.User`) then it becomes part of
your extended user contract that you have this functionality.

I'm not sure a warning that you're doing this is a good idea either, as
someone in a totally custom environment without `User` like models linked
to external applications may have perfectly reasonable reasons for wanting
a different name.

I am however happy to put a recommendation into the documentation that
where sensible you should call your pluggable user `myapp.User` to reduce
these kind of problems when you change it.

--
Ticket URL: <https://code.djangoproject.com/ticket/20244#comment:8>

Django

unread,
May 23, 2013, 5:59:13 AM5/23/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
---------------------------------+------------------------------------

Reporter: bdauvergne | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------

Comment (by mjtamlyn):

Ok, having now understood was Russell was saying in his last comment, I
don't think we can make this change - it's too backwardsly incompatible
due to the fact that setting the related name changes the filter lookups.

It should be theoretically possible to implement the restriction
benjaoming was suggesting, that swappable models automatically set their
related attributes and query lookups based on the model they're swapping
out. Personally I'm not in favour of this as it will likely cause more
confusion for most users. Just because I've swapped out user in my small
project for something called Profile doesn't mean that all of the normal
rules about automatic naming should break.

As a result, I think the best solution is to document that wherever
possible you should call your swapped out model `User`, and give the
reasons why. In an external application which requires doing lookups based
on `Group`, `Permission` or anything else the swappable user relates to
should upgrade this recommendation to a requirement. By depending on the
presence of `PermissionsMixin`'s relationships, you are in fact already
extending the contract for what a swappable user means. To my mind,
including "this must be called `User`" is a reasonable restriction.

I have considered putting a proxy on `Group` and `Permission` to find the
swappable model under a default name. This falls over somewhat when the
user model is extended to add e.g. `User.admin_groups`, a m2m to groups
the user can administer. Which relationship is now the "default" the proxy
would point to?

I agree it would be nicer if we could make a concrete resolution here
which would make swappable models easier in external applications. However
it is more important to maintain both backwards compatibility and
adaptability of the current system. I don't want a restriction that you
*have* to name your model `User`, but equally I don't see much reason why
if you want to integrate into a larger ecosystem of pluggable parts you
shouldn't have to. Once migrations come around it should be much easier to
rename models anyway if you do come across this problem half way through a
project.

Documentation PR is at https://github.com/django/django/pull/1203

--
Ticket URL: <https://code.djangoproject.com/ticket/20244#comment:9>

Django

unread,
May 23, 2013, 7:56:13 AM5/23/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
---------------------------------+------------------------------------

Reporter: bdauvergne | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------

Comment (by benjaoming):

I agree with mjtamlyn's immediate changes to the docs. Maybe it should
also note that the reason has to do with an unresolved issue in automatic
naming of reverse lookups and related queryset attributes on
PermissionMixin. Also, I'm pretty sure that people are using
AUTH_USER_MODEL like crazy, because they think it's the way to go... so I
will suggest a docs change after this one, that makes clear that the user
model is not supposed to be swapped out in order to add profile
information. Just to do some collateral.

''In the longer run...''

I don't think that restricting the model class name is the way to go in
the long run. It looks like a 1999 javascript library doc :) Should we
raise exception `SwappedInModelMustBeSameName` if a model is swapped out
by another model that doesn't have the same name? ;)

Regarding reverse lookups, `Q(user__XXX)` and `object.user_set`, please
note that this is _ONLY_ a suggestion to maintain the the swapped out
model's reverse names in case no related_name parameter is given to the
relationship. As soon as related_name is set for a relation to a swappable
model, this will be respected! The real issue is that this has not been
done on relations in PermissionMixin. There's no turning back from this
because supposedly people have already deployed tons of custom user models
that aren't named User, and how should they then refactor their code?
Renaming tables and models is a real mess.

Btw. a quick count to illustrate the issue: django-cms relies on the
"user_set" naming 16 places at least + "user__" and "__user" 13 places at
least.

mjtamlyn:
> In an external application which requires doing lookups based on Group,


Permission or anything else the swappable user relates to should upgrade
this recommendation to a requirement.

I don't agree to this, though. There should be two cases which constitute
a contract: 1) If related_name is set, then that's the contract. Same as
any other relation or 2) If related name is not set, then always default
to the swappable model's name (Django 1.5). In Django 1.6, we could raise
an exception instead of having case 2)... 1.6 release note could prompt
people to change all "user" lookups to "user_set" if we do
related_name="user_set" in PermissionMixin.

mjtamlyn:


> By depending on the presence of PermissionsMixin's relationships, you
are in fact already extending the contract for what a swappable user
means.

No, I don't think so. If we needed relations with no contracts at all, we
would be using generic relationships. A swappable model has to be
something that we can assume something about, and if it is swapped out by
e.g. a model that does not implement PermissionMixin, then the developer
should be acknowledging that assumptions made by huge parts of the
reusable app ecology would be violated. If, on the other hand, the Django
docs did not seriously promote that swapped out user models should
implement PermissionsMixin, then reusable apps would no longer be able to
depend on that contract, and that would be a major blow. Especially
considering that there are security and privacy issues at stake.

--
Ticket URL: <https://code.djangoproject.com/ticket/20244#comment:10>

Django

unread,
May 23, 2013, 9:07:45 AM5/23/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
---------------------------------+------------------------------------

Reporter: bdauvergne | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------

Comment (by mjtamlyn):

Ok, on further discussion with Russell and reading your comments, here's
another idea.

Revisiting the idea of solidifying the functionality in
`PermissionsMixin`, I think this is the right way to go. The problem is
that at present setting `related_name` will also change the filter
lookups, which is a backwards incompatibility we can't accept. However
what we could do is extend the API for relations so that they can also
define this extra customisation point - a new kwarg called something like
`related_filter_name`. This would allow us to pin `PermissionsMixin` in a
backwards-compat-to-1.4 way, and only break the small subset of use cases
discussed before, which the core discussion in Warsaw deemed acceptable.

To my mind, this is also a good thing to do anyway as at present we have
automatic naming functionality which cannot be exactly reproduced by
explicit code. IMO this is a bug with the design.

Would this satisfy the requirements you need for Django CMS? It seems to
me at least the DjCMS is requiring PermissionsMixin (and contrib.auth for
that matter) for this functionality anyway - so this would at least ensure
that any mode defined as a subclass of `AbstractUser` (not
`AbstractBaseUser`) would work seamlessly.

To respond to one of your other points, I think pluggable user models are
exactly the right solution to "I have this one extra char field I need to
add to User" kind of profile information problems - not an additional
table and all of the database complexity that introduces. But that's
another discussion ;)

--
Ticket URL: <https://code.djangoproject.com/ticket/20244#comment:11>

Django

unread,
May 23, 2013, 1:42:40 PM5/23/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
---------------------------------+------------------------------------

Reporter: bdauvergne | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------

Comment (by ptone):

It would seem a reasonable API restriction for Groups in the specific
case, and swappable models in the general case to say that related_names
take the name of the swapped model

so group.user_set would remain, in this case 'user' is the role of the
model, not the name of the spec

The relatively easy place to set this would be {{{get_accessor_name}}}

https://github.com/django/django/blob/dffdca1109a2111f104f2419d081c0f971537fec/django/db/models/related.py#L50

However there is a big catch/flaw. The record keeping of swapped models
only goes in one direction. When a custom user is installed, we can ask
the User model if it is swapped, and find out from the setting what it is
swapped with. But we can not take an instance of the CustomUser and ask
what it was swapped_for. Without knowing what it was swapped for,
{{{get_accessor_name}}} has no way to know the name of the model it was
swapped in for.

If we could figure out a sane way to get a {{{swapped_for}}} attribute,
then something like this might work:

{{{

def get_accessor_name(self):
# This method encapsulates the logic that decides what name to
give an
# accessor descriptor that retrieves related many-to-one or
# many-to-many objects. It uses the lower-cased object_name +
"_set",
# but this can be overridden with the "related_name" option.
model_name = self.opts.swapped_for or self.opts.model_name
if self.field.rel.multiple:
# If this is a symmetrical m2m relation on self, there is no
reverse accessor.
if getattr(self.field.rel, 'symmetrical', False) and
self.model == self.parent_model:
return None
return self.field.rel.related_name or (model_name + '_set')
else:
return self.field.rel.related_name or model_name
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/20244#comment:12>

Django

unread,
May 23, 2013, 1:52:13 PM5/23/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
---------------------------------+------------------------------------

Reporter: bdauvergne | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Changes (by ptone):

* cc: preston@… (added)


--
Ticket URL: <https://code.djangoproject.com/ticket/20244#comment:13>

Django

unread,
May 23, 2013, 3:58:48 PM5/23/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
---------------------------------+------------------------------------

Reporter: bdauvergne | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------

Comment (by mjtamlyn):

This will only solve the reverse relationship descriptor, we would still
need to do additional work to get the correct {{{related_query_name}}} for
filter lookups. It seems to me your proposed solution isn't much different
to fixing the names, except that it future proofs the mechanism for any
other possible swappable models in the future.

--
Ticket URL: <https://code.djangoproject.com/ticket/20244#comment:14>

Django

unread,
May 24, 2013, 12:20:27 AM5/24/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
---------------------------------+------------------------------------

Reporter: bdauvergne | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------

Comment (by russellm):

@mjtamlyn For me, that's the point -- it's ultimately *exactly* the same
solution as manually setting the related_name and related_query_name,
except that it's done automatically for any swapped model, since end users
will need to do this consistently wherever a model is swapped out. As an
aside, adding related_query_name handling may also be worthwhile, but
that's orthogonal to this work if we can get the automated swapped_for
version working.

--
Ticket URL: <https://code.djangoproject.com/ticket/20244#comment:15>

Django

unread,
May 24, 2013, 11:23:15 PM5/24/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
---------------------------------+------------------------------------

Reporter: bdauvergne | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------

Comment (by ptone):

Looking at how this might possibly work, and haven't found any great
options.

I think this will have to happen at the level of the app_cache, after all
models are loaded. It can traverse the loaded models, looking for those
that are 'swappable' then checking for the appropriate setting, then
setting a 'swapped_for' attribute on swapped in model. {{{swapped_for}}}
as a term is already used in _swapped where I probably would have used
{{{swapped_with}}}, but given this is all private API, we can change at
will if it makes sense.

There isn't really any way I can see to discover this at Model creation
time.

@russelm if you get some time in a bit to look at this and get a start, or
at least a plan of attack, I can probably help finish up and/or review in
the AM my time.

--
Ticket URL: <https://code.djangoproject.com/ticket/20244#comment:16>

Django

unread,
Jun 22, 2013, 9:28:24 AM6/22/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
---------------------------------+------------------------------------

Reporter: bdauvergne | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------

Comment (by andrewgodwin):

While I understand that a nice generic solution would be great, I'm much
more in favour of just adding a `related_query_name` parameter to get this
bug fixed with the least amount of code and getting it out of the way of
blocking a release.

If you want the automatic solution I'm going to need someone to step up
and do it in the next week before beta, otherwise I'll land a patch for
the related_query_name parameter and go with that solution as it's a nice
minimum fix to the overall blocker here (which is that PermissionsMixin
doesn't work right).

--
Ticket URL: <https://code.djangoproject.com/ticket/20244#comment:17>

Django

unread,
Jun 27, 2013, 9:11:34 AM6/27/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
---------------------------------+----------------------------------------
Reporter: bdauvergne | Owner: andrewgodwin
Type: Bug | Status: assigned
Component: contrib.auth | Version: 1.5

Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+----------------------------------------
Changes (by andrewgodwin):

* status: new => assigned
* owner: nobody => andrewgodwin


--
Ticket URL: <https://code.djangoproject.com/ticket/20244#comment:18>

Django

unread,
Jun 27, 2013, 9:45:38 AM6/27/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
---------------------------------+----------------------------------------
Reporter: bdauvergne | Owner: andrewgodwin
Type: Bug | Status: assigned
Component: contrib.auth | Version: 1.5

Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+----------------------------------------

Comment (by Andrew Godwin <andrew@…>):

In [changeset:"99b467f272da91b8894dc90d793d8d2c40b78d8c"]:
{{{
#!CommitTicketReference repository=""
revision="99b467f272da91b8894dc90d793d8d2c40b78d8c"
Add related_query_name to ForeignKey/M2M. Refs #20244
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/20244#comment:19>

Django

unread,
Jun 27, 2013, 10:44:37 AM6/27/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
---------------------------------+----------------------------------------
Reporter: bdauvergne | Owner: andrewgodwin
Type: Bug | Status: closed
Component: contrib.auth | Version: 1.5
Severity: Release blocker | Resolution: fixed

Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+----------------------------------------
Changes (by Andrew Godwin <andrew@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"f325f86971bf7fc30e3daafc9835f66cd7951e3a"]:
{{{
#!CommitTicketReference repository=""
revision="f325f86971bf7fc30e3daafc9835f66cd7951e3a"
Fixed #20244: PermissionsMixin now defines a related_query_name for M2Ms
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/20244#comment:20>

Django

unread,
Oct 11, 2013, 8:29:11 AM10/11/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
---------------------------------+----------------------------------------
Reporter: bdauvergne | Owner: andrewgodwin
Type: Bug | Status: closed
Component: contrib.auth | Version: 1.5

Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+----------------------------------------

Comment (by antialiasis@…):

This change makes it incredibly difficult to migrate from the default user
model to a custom user model with permissions. Which, yes, I realize
Django recommends against, but given projects and requirements change it
should definitely be kept in mind that this might need to be possible.

Various blogs and StackOverflow questions already describe how to migrate
to a custom user model using South. These methods almost universally call
for creating your custom user model while AUTH_USER_MODEL is still set to
'auth.User', then performing a migration to one way or another get the
users from the auth_user table into a table your new user model can use,
and then switching AUTH_USER_MODEL over to the new model. However, with
this change, you can't have two models that inherit from PermissionsMixin
exist in your project at the same time at any point, because it doesn't
pass model validation:


{{{
CommandError: One or more models did not validate:
auth.user: Accessor for m2m field 'groups' clashes with related m2m field
'Group.user_set'. Add a related_name argument
to the definition for 'groups'.
auth.user: Accessor for m2m field 'user_permissions' clashes with related
m2m field 'Permission.user_set'. Add a related
_name argument to the definition for 'user_permissions'.
accounts.user: Accessor for m2m field 'groups' clashes with related m2m
field 'Group.user_set'. Add a related_name
argument to the definition for 'groups'.
accounts.user: Accessor for m2m field 'user_permissions' clashes with
related m2m field 'Permission.user_set'. Add
a related_name argument to the definition for 'user_permissions'.
}}}

And without having both models active at the same time, South can't
migrate data between them or otherwise do anything useful.

Migrating to custom user models is already enough of a quagmire. Is there
no way to avoid this headache without breaking third-party apps that use
user_set/user in related queries?

--
Ticket URL: <https://code.djangoproject.com/ticket/20244#comment:21>

Django

unread,
Oct 11, 2013, 1:47:40 PM10/11/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
---------------------------------+----------------------------------------
Reporter: bdauvergne | Owner: andrewgodwin
Type: Bug | Status: closed
Component: contrib.auth | Version: 1.5

Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+----------------------------------------

Comment (by mjtamlyn):

During that migration point, it should be straightforwards enough to copy
the two fields form PermissionsMixin into your custom model, add a
`related_name`, and then remove them and re-add PermissionsMixin again
afterwards. Its worth noting that the same problem you're experiencing
would occur with any custom User model named User, irrespective of these
changes. What the change guarantees is that when your transitional period
is complete, django and other third party applications have a defined api
to use if they require Group or Permission.

--
Ticket URL: <https://code.djangoproject.com/ticket/20244#comment:22>

Django

unread,
Dec 19, 2013, 5:16:13 PM12/19/13
to django-...@googlegroups.com
#20244: PermissionsMixin should define related name of groups and user_permissions
related name
---------------------------------+----------------------------------------
Reporter: bdauvergne | Owner: andrewgodwin
Type: Bug | Status: closed
Component: contrib.auth | Version: 1.5

Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 1

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+----------------------------------------
Changes (by anonymous):

* needs_docs: 0 => 1


Comment:

Should this be added to the custom user model docs or 1.6 release notes?
It would be good to know about the changes to `related_name` and addition
of `query_filter_name` are coded into PermissionsMixin (the same applies
to the `user_permissions` ManyToManyField).

Example: My app's user model subclasses `contrib.auth.AbstractUser` and
has a model that relates to `contrib.auth.Group`. I came across this while
checking for compatibility with django 1.6 when
`MyModel.objects.filter(groups__myuser=user_obj)` came back as an invalid
query.

--
Ticket URL: <https://code.djangoproject.com/ticket/20244#comment:23>

Reply all
Reply to author
Forward
0 new messages