Thanks for the input, that clears up some details which bring me on agreement for the first suggestion. Op 2 feb. 2016 21:51 schreef "Shai Berger" <s...@platonix.com>:
> 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. > -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" 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 https://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAFGzxkzUXWyZcB%3D2pBvDyxVfZZ2EmQ%3Dx_UX4N6M%3DyxOKEbfk%3DQ%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
