Hi Sergei,
Two notes:
1) For Abstract base classes, Meta is inherited by default[1] which would make
the permissions inherited by default. For multi-table inheritance, this is not
the case[2].
2) The second option as you presented it, where AssignedTask.Meta's
permissions are added to its parent's permissions, rather than replacing them,
appears unpythonic to me. Python, consistently, requires such accumulation in
inheritence to be explicit.
So, I think I would go with Tim's first suggestion. It means that, if BaseTask
is concrete, then, indeed, its subclasses might need some boilerplate. Note
that such boilerplate may be reduced by other means -- e.g. defining a base-
class for all the Meta's as an independent, non-nested class. At least in
principle, you could even give this base-Meta a metaclass that will implement
the unpythonic accumulating-instead-of-overriding inheritence; but I don't
think Django should do something like that.
HTH,
Shai.
[1] https://docs.djangoproject.com/en/1.9/topics/db/models/#meta-inheritance
[2] https://docs.djangoproject.com/en/1.9/topics/db/models/#meta-and-multi-
table-inheritance
On Tuesday 02 February 2016 15:52:19 Sergei Maertens wrote:
> Hi all,
>
> I was browsing tickets and stumbled
> upon https://code.djangoproject.com/ticket/10686, which was accepted except
> for lack of documentation. The patch in the mentioned ticket makes it so
> that a Model class that's used as base class with extra permissions:
>
> class BaseTask(models.Model):
> ...
> class Meta:
> permissions = (
> ('view_%(class)s', 'Can view %(class)s'),
> )
>
> will make those permissions specific for any child classes:
>
> class TodoTask(BaseTask):
> pass
>
>
> class AssignedTask(BaseTask):
> assignee = models.ForeignKey(settings.AUTH_USER_MODEL)
>
> class Meta:
> permissions = (
> ('reassign_assignedtask', 'Can reassign assigned task')
> )
>
>
>
>
> With the current patch, TodoTask would have one extra permission:
> view_todotask, and AssignedTask would have two: view_assignedtask and
> reassign_assignedtask.
>
> Now there were some concerns on the pull request, and input is needed:
>
>
> - in its current state, Django will not inherit the base class
> permissions, so AssignedTask would end up only with the
> reassign_assignedtask permission (so missing view_assignedtask). The
> patch ensures that inheritance is applied, and adds an extra Meta option:
> inherit_permissions. Setting that to False yields the current behaviour -
> no base class permissions are inherited. Disadvantages of this approach: -
> yet another Meta attribute
> - more tight coupling between Meta and django.contrib.permissions
> (pointed out by Tim Graham)
> - 'hidden' second feature within the initial enhancee
>
> Two alternatives to avoid having to add inherit_permisisons have been
> proposed by Tim:
>
> class AssignedTask(BaseTask):
> assignee = models.ForeignKey(settings.AUTH_USER_MODEL)
>
> class Meta:
> permissions = BaseTask.Meta.permissions + (
> ('reassign_assignedtask', 'Can reassign assigned task')
> )
>
> It looks like the inheritance of permissions would not be a default then,
> which would require the other model to be changed to:
>
> class TodoTask(BaseTask):
> class Meta:
> permissions = BaseTask.Meta.permissions
>
>
> In my opinition this introduces boilerplate which you get for free if the
> permissions are inherited by default. On the upside, it is more explicit.
>
> Another suggestion was to use good old fashioned Python inheritance:
>
> class TodoTask(BaseTask):
> class Meta(BaseTask.Meta):
> pass
>
>
> class AssignedTask(BaseTask):
> assignee = models.ForeignKey(settings.AUTH_USER_MODEL)
>
> class Meta(BaseTask.Meta):
> permissions = (
> ('reassign_assignedtask', 'Can reassign assigned task')
> )
>
>
> The latter two suggestions would ofcourse require fixing the patch, but
> that's an implementation detail.
>
> Input would be greatly valued.
