django-modelcluster icon indicating copy to clipboard operation
django-modelcluster copied to clipboard

Data migration doesn't write to ParentalManyToManyField

Open harrislapiroff opened this issue 7 years ago • 6 comments

I ran into an issue writing a data migration for a ParentalManyToManyField today where the data migration wouldn't write any data to the tables for that field. The migration looked something like this:

# -*- coding: utf-8 -*-
# Generated by Django 1.11.11 on 2018-04-19 14:10
from __future__ import unicode_literals

from django.db import migrations


def forwards(apps, schema_editor):
    """
    Move all data from Topic, Country, and Language to TemporaryTopic,
    TemporaryCountry, and TemporaryLanguage
    """

    TemporaryTopic = apps.get_model('directory', 'TemporaryTopic')
    TemporaryCountry = apps.get_model('directory', 'TemporaryCountry')
    TemporaryLanguage = apps.get_model('directory', 'TemporaryLanguage')
    DirectoryEntry = apps.get_model('directory', 'DirectoryEntry')

    # ...

    # Move DirectoryEntry fks
    for entry in DirectoryEntry.objects.all():
        topic_titles = list(entry.topics.values_list('title', flat=True))
        country_titles = list(entry.countries.values_list('title', flat=True))
        language_titles = list(entry.languages.values_list('title', flat=True))
        entry.temporary_topics.set(
            TemporaryTopic.objects.filter(title__in=topic_titles)
        )
        entry.temporary_countries.set(
            TemporaryCountry.objects.filter(title__in=country_titles)
        )
        entry.temporary_languages.set(
            TemporaryLanguage.objects.filter(title__in=language_titles)
        )
        entry.save()

    # ...

class Migration(migrations.Migration):
    # ...
    operations = [
        migrations.RunPython(forwards),
    ]

@gasman suspects the fake model that gets built by django's migration logic doesn't inherit from ClusterableModel and is therefore missing some of the logic necessary to make this data migration work.

I ended up using a workaround in which I had a migration convert my ParentalManyToManyFields to Django's built-in ManyToManyField before the data migration and then another migration afterward to convert them back, which seems to have worked fine.

harrislapiroff avatar Apr 19 '18 16:04 harrislapiroff

Ran into the same problem. Did the data migration manually outside the migration.

grasshoppermouse avatar Feb 04 '19 19:02 grasshoppermouse

I also ran into this. The explanation above seems correct; in migrations calling save() on a model calls the base Model.save() method and thus skips the ClusterableModel.save() call that would be necessary to persist the changes being made.

@harrislapiroff I was also able to use your suggested workaround of three migration steps:

  1. AlterField to a models.ManyToManyField.
  2. RunPython that does the data migration.
  3. AlterField back to a modelcluster.fields.ParentalManyToManyField.

Another simpler option that seems to work (using an undocumented API on the field) is to manually invoke commit() on the relation, e.g.

for entry in DirectoryEntry.objects.all():
    topic_titles = list(entry.topics.values_list('title', flat=True))
    entry.temporary_topics.set(
        TemporaryTopic.objects.filter(title__in=topic_titles)
    )

    # This seems to commit the related objects, and doesn't require an entry.save() call.
    entry.temporary_topics.commit()

chosak avatar Apr 06 '20 14:04 chosak

This should be mentioned in some caveats on a frontpage I guess! :)

pySilver avatar Mar 24 '21 04:03 pySilver

In a perverse coincidence, we had been using the workaround first Initial State -> ManyToMany -> RunPython -> ParentalManyToMany, then decided to clean it up and do it the "correct way" e.g. Initial State -> ParentalManyToMany -> RunPython. After hours of confusion eventually found this issue. Definitely needs to be called out on in the README!

Perhaps Andy's workaround with calling commit() in the migration should/could be documented? Would the maintainers (@gasman ) foresee any issues with documenting that?

vsalvino avatar May 26 '22 20:05 vsalvino

Ran into this issue as well!

Stormheg avatar Sep 04 '23 09:09 Stormheg