Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

through_fields not reflected in test models #418

Open
jaredlockhart opened this issue Jan 11, 2024 · 0 comments
Open

through_fields not reflected in test models #418

jaredlockhart opened this issue Jan 11, 2024 · 0 comments

Comments

@jaredlockhart
Copy link

jaredlockhart commented Jan 11, 2024

Hi,

I just stumbled on a weird little edge case. I am migrating a ManyToMany field over to a through model, and specifying both through and through_fields on the ManyToMany field on the parent model, example:

class ThroughModel(models.Model):
    parent_experiment = models.ForeignKey(
        "NimbusExperiment",
        on_delete=models.CASCADE,
    )
    child_experiment = models.ForeignKey(
        "NimbusExperiment",
        on_delete=models.CASCADE,
    )
    branch_slug = models.SlugField(
        max_length=NimbusConstants.MAX_SLUG_LEN, null=True, blank=True
    )

class NimbusExperiment(models.Model):
    required_experiments = models.ManyToManyField["NimbusExperiment"](
        "NimbusExperiment",
        related_name="required_by",
        blank=True,
        verbose_name="Required Experiments",
        through=ThroughModel,
        through_fields=("parent_experiment", "child_experiment"),
    )

and then using this package to write a test roughly like

    def prepare(self):
        """Prepare some data before the migration."""
        User = self.old_state.apps.get_model("auth", "User")
        NimbusExperiment = self.old_state.apps.get_model(
            "experiments", "NimbusExperiment"
        )

        user = User.objects.create(email="[email protected]")

        parent_experiment = NimbusExperiment.objects.create(
            owner=user,
            name="test parent experiment",
            slug="test-parent-experiment",
            application=NimbusConstants.Application.DESKTOP,
            status=NimbusConstants.Status.DRAFT,
            publish_status=NimbusConstants.PublishStatus.IDLE,
            published_dto="{}",
        )
        required_experiment = NimbusExperiment.objects.create(
            owner=user,
            name="test required experiment",
            slug="test-required-experiment",
            application=NimbusConstants.Application.DESKTOP,
            status=NimbusConstants.Status.DRAFT,
            publish_status=NimbusConstants.PublishStatus.IDLE,
            published_dto="{}",
        )
        parent_experiment.required_experiments.add(required_experiment)

    def test_migration(self):
        """Run the test itself."""
        NimbusExperiment = self.new_state.apps.get_model(
            "experiments", "NimbusExperiment"
        )
     
        parent_experiment = NimbusExperiment.objects.get(slug="test-parent-experiment")

        self.assertEqual(
            set(
                parent_experiment.required_experiments.all().values_list(
                    "slug", flat=True
                )
            ),
            {"test-required-experiment"},
        )

however this test was failing and I couldn't see any obvious reason why. Digging deeper I found that the generated SQL for the query parent_experiment.required_experiments.all() in the test looked like

...WHERE "experiments_nimbusexperimentbranchthroughrequired"."child_experiment_id" = 1

whereas it should be

...WHERE "experiments_nimbusexperimentbranchthroughrequired"."parent_experiment_id" = 1

and what's interesting is that I get the correct behaviour in ./manage.py shell but not if I pdb inside the test, so something is going wrong inside the test.

Further investigation lead me to find that inside the test I see

ipdb> required_field = NimbusExperiment._meta.many_to_many[-1]
ipdb> required_field
<django.db.models.fields.related.ManyToManyField: required_experiments>
ipdb> required_field.remote_field.through_fields
ipdb> required_field.remote_field.through_fields is None
True

whereas in ./manage.py shell I see

In [9]: required_field.remote_field.through_fields
Out[9]: ('parent_experiment', 'child_experiment')

so somehow the through_fields is None inside the test context and not the tuple it should be, which is causing the relevant method in django here to fall back to the case of looping over fields and finding the first one that matches the model, and finding child_experiment first as we can see here inside a pdb inside the test:

ipdb> NimbusExperimentBranchThroughRequired._meta.fields
(<django.db.models.fields.AutoField: id>, <django.db.models.fields.SlugField: branch_slug>, <django.db.models.fields.related.ForeignKey: child_experiment>, <django.db.models.fields.related.ForeignKey: parent_experiment>)

So I was able to rectify this by explicitly setting through_fields like so in the test:

    def test_migration(self):
        """Run the test itself."""
        NimbusExperiment = self.new_state.apps.get_model(
            "experiments", "NimbusExperiment"
        )
        required_field = next(
            f
            for f in NimbusExperiment._meta.many_to_many
            if f.name == "required_experiments"
        )
        required_field.remote_field.through_fields = (
            "parent_experiment",
            "child_experiment",
        )

        parent_experiment = NimbusExperiment.objects.get(slug="test-parent-experiment")

        self.assertEqual(
            set(
                parent_experiment.required_experiments.all().values_list(
                    "slug", flat=True
                )
            ),
            {"test-required-experiment"},
        )

but I also wanted to file it here. Looking through this repo I couldn't find anything that would clearly cause this behaviour, so I'm not sure why through_fields would be None inside the test context, but hopefully the maintainers of this project will have a better idea.

Aside from that I just want to say thank you for this library and we use it constantly and it's made testing migrations much easier 🙏

Oh and this is on django-test-migrations 1.3.0 and Django 3.2.23 (I know this is old)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant