Meta Inheritance for default_permissions

[Clayton Daley]

I add Read/Browse permissions to my models that I enforce in my views. For some reason, default_permissions isn't inheriting in the way I would expect. Nor does the Meta Inheritance section of the documentation say anything to suggest this should not be valid.

Given something like:

class BreadMetaMixin(object):
    # ACTION_EDIT aliases "change" for BREAD semantics, see http://paul-m-jones.com/archives/291
    default_permissions = (ACTION_BROWSE, ACTION_READ, ACTION_EDIT, ACTION_ADD, ACTION_DELETE)

class Encounter(AbstractParentModel):
    class Meta(BreadMetaMixin, AbstractParentModel.Meta):
        pass

class Admission(Encounter):
    class Meta(Encounter.Meta):
        # also reproduced with non-proxy model
        proxy = True

When I makemigrations, Encounter detects the new permissions...

...
    operations = [
        migrations.CreateModel(
            name='Encounter',
...
            options={
                'default_permissions': ('browse', 'read', 'change', 'add', 'delete'),
                'abstract': False,
            },
...

but Admissions does not:

...
    operations = [
        migrations.CreateModel(
            name='Admission',
...
            options={
                'abstract': False,
                'proxy': True,  # also verified with non-proxy model
                'indexes': [],
            },
...

It's possible that this hasn't come up because it's masked by #11154, but it affects non-proxy models as well. I noticed it because I'm working around #11154 using a migration like:

def add_proxy_permissions(apps, schema_editor):
    """Workaround https://code.djangoproject.com/ticket/11154 by manually creating the required permissions."""
    ContentType = apps.get_model('contenttypes', 'ContentType')
    Permission = apps.get_model('auth', 'Permission')
    for model in ['Admission']:
        class_ = apps.get_model('task_manager', model)
        content_type = ContentType.objects.get_for_model(class_, for_concrete_model=False)
        for perm in class_._meta.default_permissions:
            Permission.objects.get_or_create(
                content_type=content_type,
                codename='{}_{}'.format(perm, model.lower()),
            )

This works great on two meta models where BreadMetaMixin is included directly (e.g. UserProxy wraps auth.models.User and includes BreadMetaMixin). When I moved the code over to Admission, however, it wasn't working correctly. If I mixin BreadMetaMixin directly, it works fine so I've narrowed it down to a Meta Inheritance issue.

[Carlton Gibson]

This is expected behaviour.

The ​Proxy models docs say:

Proxy models inherit Meta attributes in the same way as regular models.

Here they link to the ​Meta and and multi-table inheritance section which says:

... it doesn’t make sense for a child class to inherit from its parent’s Meta class... However, there are a few limited cases where the child inherits behavior from the parent: if the child does not specify an ordering attribute or a get_latest_by attribute, it will inherit these from its parent.

default_permissions is not amongst the listed exceptions, so there is no reason to expect it would be inherited.

[Clayton Daley]

The section you cited says "child class to inherit from its parent’s Meta class" which I interpret as the following:

class Parent(models.Model):
    class Meta:
        <stuff here>

class Child(Parent):
    # no explicit meta definition

In my case, the Meta class is explicitly a subclass of the parent's Meta:

class Encounter(AbstractParentModel):
    class Meta(BreadMetaMixin, AbstractParentModel.Meta):
        pass

class Admission(Encounter):
    class Meta(Encounter.Meta):  # EXPLICIT inheritance
        pass

So this should be covered by the Meta Inheritance documentation (​https://docs.djangoproject.com/en/2.0/topics/db/models/#meta-inheritance). This section specifically says:

If the child wants to extend the parent’s Meta class, it can subclass it... Django does make one adjustment to the Meta class of an abstract base class: before installing the Meta attribute, it sets abstract=False.

It seems to me that it's also ignoring default_permissions. At minimum, this is a documentation issue.

[Carlton Gibson]

Replying to Clayton Daley:

The section you cited says "child class to inherit from its parent’s Meta class" ...

Those words go " it doesn’t make sense for a child class to inherit from its parent’s Meta class". Don't you think the "it doesn't make sense" is clear enough?

It follows "All the Meta options have already been applied to the parent class and applying them again would normally only lead to contradictory behavior".

What else do you want it to say?

[Clayton Daley]

I only cited that section to point out that it describes a different case... implicit inheritance of a parent's Meta Class. In that case, the documentation is fine and I believe the behavior is expected.

I followed the Meta Inheritance section which states "If the child wants to extend the parent’s Meta class, it can subclass it". I wanted to extend it. I subclassed it. In this section, it states that only one change is made... abstract is set to False. It does not state that default_permissions are ignored. But they seem to be.

[Carlton Gibson]

That section if for abstract inheritance, one of the three types (the others being multi-table and proxy). It's saying that when you inherit from an abstract model Meta is inherited (and can be overridden.)

Your case is Proxy inheritance, which links to the multi-table rules for inheritance (which I cited above). It's important not to conflate the different types of inheritance.

I appreciate it's complex and not always as expected every time, especially when the examples become more developed, as are yours.

This still seems to be expected behaviour. I'm going to close this as such. If you have a concrete suggestion for the docs—there's always the possibility of improvement—we'd be very happy to review a PR referencing this issue.

[Clayton Daley]

Thanks. I finally see the chain of references that would have brought me to that conclusion. It sounds like Meta Inheritance is supported under few, very specific conditions... something like:

• A is the parent of B
• A.Meta is the parent of B.Meta
• A.Meta is abstract

If a concise set of conditions could be described, would you be open to a PR that threw ImproperlyConfigured under other conditions? This seems like an ideal case for an exception because Meta Inheritance is a best practice in some cases (i.e. abstract inheritance) and deliberately ineffective in others (Multi-Table, Proxy). A reasonably informative error message would eliminate the need to even go to the documentation.

[Carlton Gibson]

Hi Clayton.

I'm struggling to think what such a PR would look like — but yes — happy to look at one. (It's always easier to think about with actual code.)

My only initial thought would be to use a system check, rather than an ImproperlyConfigured.

If you come up with something call the PR Ref #29386 -- ... and @carltongibson me on GitHub. I'll have a look.