Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#23422 closed Bug (wontfix)

Cannot add Permission to Group in data migration

Reported by: Tomas Walch Owned by: nobody
Component: Migrations Version: 1.7
Severity: Normal Keywords:
Cc: bronger@…, scott.woodall@…, hv@…, TZanke, frnhr, adehnert, alan.justino@…, django@…, info@… Triage Stage: Unreviewed
Has patch: no Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

As initial_data fixtures are now deprecated I want to move my things there to migration files instead. One thing I do is to create auth.Group(s) and give these some custom permissions that I have defined on the app's models. This does not seem to be possible as, from what I can tell, the permissions and content types are created in a signal AFTER all the Migrations are run. Neither custom nor standard permissions can be found.

A Group and its permissions are data and the initialization thereof should therefore be possible to achieve through the migration system since its supposed to replace the fixtures.

my_new_group.permissions.add(
    Permission.objects.get_by_natural_key('add_my_model', 'my_app', 'my_model')
)

This gives:

django.contrib.contenttypes.models.DoesNotExist: ContentType matching query does not exist.

even though I created the model in the previous migration step. There are no ContentTypes ate all yet.

Change History (27)

comment:1 Changed 5 years ago by chris cauley

I'm a bit new to the migrations system, but I was able to verify this. I created a new app called "my_app" and a model called "MyModel". I created the first migration and then the following data migration:

# -*- coding: utf-8 -*-                                                                                                                      
from __future__ import unicode_literals

from django.db import models, migrations

def make_permissions(apps,schema_editor):
    Group = apps.get_model("auth","Group")
    Permission = apps.get_model("auth","Permission")
    my_new_group = Group(name="test_group")
    my_new_group.save()
    my_new_group.permissions.add(
        Permission.objects.get(codename='add_mymodel',content_type__app_label='my_app')
    )

class Migration(migrations.Migration):

    dependencies = [
        ('my_app', '0001_initial'),
        ('auth', '0001_initial'),
    ]

    operations = [
        migrations.RunPython(make_permissions,reverse_code=lambda *args,**kwargs: True)
    ]

If I run these separately (python manage.py migrate my_app 0001;python manage.py migrate my_app 0002), everything is cool. Together it breaks (python manage.py migrate my_app). You could solve this yourself by doing a get_or_create on the content type and on the permission, but that is by no means an adequate solution.

comment:2 Changed 5 years ago by Andrew Godwin

Resolution: wontfix
Status: newclosed

This is a limitation of the signalling system we use to create ContentTypes; if we fixed this to work, there would then be issues where the ContentTypes were created too early.

I would suggest that the data migration uses ContentType.objects.get_for_model to get the ContentType before making the Permission, as this creates the ContentType if necessary and has internal caching too.

comment:3 Changed 4 years ago by Nicals

I understand the won't fix, but a way to solve this group permissions problem would be appreciated. I'm blocked on a project by this...

comment:4 Changed 4 years ago by Ilya Baryshev

I figure out ContentType.objects.get_for_model won't work "as is" in datamigration, because only default model managers are available.

comment:5 Changed 4 years ago by Carl Meyer

If using ContentType.objects.get_for_model is not possible, perhaps calling django.contrib.auth.management.create_permissions(...) in the migration would work?

comment:6 Changed 4 years ago by John Whitlock

Here's an example of calling create_permissions directly from a data migration, as suggested by carljm:

def make_permissions(apps, schema_editor, with_create_permissions=True):
    Group = apps.get_model("auth", "Group")
    Permission = apps.get_model("auth", "Permission")
    try:
        perm = Permission.objects.get(
            codename='add_mymodel', content_type__app_label='my_app')
    except Permission.DoesNotExist:
        if with_create_permissions:
            # Manually run create_permissions
            from django.contrib.auth.management import create_permissions
            assert not getattr(apps, 'models_module', None)
            apps.models_module = True
            create_permissions(apps, verbosity=0)
            apps.models_module = None
            return make_permissions(
                apps, schema_editor, with_create_permissions=False)
        else:
            raise
    my_new_group = Group.objects.create(name="test_group")
    my_new_group.permissions.add(perm)

Feels a little hacky, but it solved this wontfix for me . I'd prefer a migration configuration that says "stop migrating, emit post_migrate signals, and restart migrations."

Last edited 4 years ago by John Whitlock (previous) (diff)

comment:7 Changed 4 years ago by Torsten Bronger

South used to have send_pending_create_signals(). A django equivalent would be useful.

comment:8 Changed 4 years ago by Torsten Bronger

Cc: bronger@… added

comment:9 Changed 4 years ago by Collin Anderson

One thing we could do is have a post_migration signal that runs after _every_ migration. That way it's consistent.

comment:10 Changed 4 years ago by Markus Holtermann

You might want to keep an eye on #24100

comment:11 Changed 4 years ago by Scott Woodall

Cc: scott.woodall@… added

comment:12 Changed 4 years ago by Thomas Güttler

I am not happy with this issue being "wontfix".

A helper method create_missing_content_types_and_permissions() would be nice.

comment:13 Changed 4 years ago by Thomas Güttler

Cc: hv@… added

comment:14 Changed 4 years ago by TZanke

Cc: TZanke added

comment:15 Changed 4 years ago by Dmitry Mugtasimov

I am not happy with this issue being "wontfix", too.

comment:16 Changed 4 years ago by antialiasis

Chiming in with another +1 on the not-wontfixing - this is stopping me from running tests.

comment:17 Changed 4 years ago by frnhr

Cc: frnhr added

Same problem seems to be affecting Generic relations in migrations, since they too depend on Content type framework...

Last edited 4 years ago by frnhr (previous) (diff)

comment:18 Changed 4 years ago by frnhr

The workaround jwhitlock proposes with calling create_permissions from migration fixed one of my problematic migration, but not another. The other migration is calling loaddata command to do a one-time fixture import, and it was failing because required ContentType objects haven't been created. So it seems to be another face of the same problem.

Here is a more dirty workaround until an official solution becomes available. I think it has better chances of fixing the problem in case solution with create_permissions fails:

if ContentType.objects.filter(**whats_needed_for_this_migration).count() < 1:
    print("Running another migrate command from migration to create ContentType objects:")
    call_command('migrate', 'users', '0002')  # this should cite migration from the same app, just prior to this one
    print("Finished with the migrate command.")

I'm guessing the only thing really needed from the migrate management command in this situation is:

emit_post_migrate_signal(created_models, self.verbosity, self.interactive, connection.alias)

but not sure how to populate the arguments.

I have no idea if using call_command("migrate") from a migration (of all places...) can be problematic, but in my particular case there don't seem to be any problems.

The lengths we have to go to... Please willfix this.

Last edited 4 years ago by frnhr (previous) (diff)

comment:19 Changed 4 years ago by adehnert

Cc: adehnert added

comment:20 in reply to:  12 ; Changed 3 years ago by Alan Justino da Silva

Needs documentation: set

Replying to guettli:

I am not happy with this issue being "wontfix".

A helper method create_missing_content_types_and_permissions() would be nice.

Found! This helper method exists already: django.core.management.sql.emit_post_migrate_signal

from django.core.management.sql import emit_post_migrate_signal

def create_group(apps, schema_editor):
    db_alias = schema_editor.connection.alias
    try:
        emit_post_migrate_signal(2, False, 'default', db_alias)
    except TypeError:  # Django < 1.8
        emit_post_migrate_signal([], 2, False, 'default', db_alias)

    Group = apps.get_model('auth', 'Group')
    Permission = apps.get_model('auth', 'Permission')
    (...)

I suggest this behavior and "how to deal with" to be documented on the Permissions page.

Is this acceptable instead of a wontfix ?

comment:21 Changed 3 years ago by Alan Justino da Silva

Cc: alan.justino@… added

comment:22 Changed 3 years ago by Daniel Hahler

Cc: django@… added

comment:23 in reply to:  20 Changed 3 years ago by cmichal

Replying to alanjds:

Is this acceptable instead of a wontfix ?

way better. it works for me, thanks!

one minor thing is that when you add group permission for the first time you need to pass it's id, for the second time Permission object is also good - strange...

m

comment:24 Changed 3 years ago by Rene Dohmen

I can confirm that it also works for me.

0001.py added custom permissions
0002.py assigned custom permissions to a group

Without the workaround by guettli in 0002.py: the custom permissions weren't available.

Thanks

R

comment:25 in reply to:  20 Changed 3 years ago by Denis

Replying to alanjds:

I've failed with Django 1.9, even after adapting emit_post_migrate_signal to its new signature:

    db_alias = schema_editor.connection.alias
    try:
        # Django 1.9
        emit_post_migrate_signal(2, False, db_alias)
    except TypeError:
        # Django < 1.9
        try:
            # Django 1.8
            emit_post_migrate_signal(2, False, 'default', db_alias)
        except TypeError:  # Django < 1.8
            emit_post_migrate_signal([], 2, False, 'default', db_alias)

This led me to this most significant part of stack trace and error:

...
  File "<venv>/local/lib/python2.7/site-packages/django/core/management/sql.py", line 50, in emit_post_migrate_signal
    using=db)
  File "<venv>/local/lib/python2.7/site-packages/django/dispatch/dispatcher.py", line 192, in send
    response = receiver(signal=self, sender=sender, **named)
  File "<venv>/olivia/local/lib/python2.7/site-packages/django/contrib/sites/management.py", line 20, in create_default_site
    if not Site.objects.using(using).exists():
...
django.db.utils.OperationalError: no such table: django_site

Has anyone succeeded with 1.9?

UPD: Sorry for the panic. This could be easily fixed supplying dependencies of the migration with sites. So you'll need at least that in the dependencies to keep going:

        ('contenttypes', '__latest__'),
        ('sites', '__latest__'),
Last edited 3 years ago by Denis (previous) (diff)

comment:26 Changed 3 years ago by Simon Charette

FWIW I started to work on this issue (which is related to #24100 to #24067) on a branch.

The plan here is to make the pre_migrate signal dispatch its plan (#24100) and inject CreatePermision operations after CreateContentType operations (that are injected after CreateModel operations just like RenameContentType operations are meant to be injected after RenameModel operations in order to solve #24067).

I'll try to get this into 1.10.

Last edited 3 years ago by Simon Charette (previous) (diff)

comment:27 Changed 3 years ago by Paul Sanchez

Cc: info@… added
Note: See TracTickets for help on using tickets.
Back to Top