Fixed #32294 -- Prevented ManyToManyField's hidden related name collisions between apps.

[manav014]

ticket-32294

I have added app_label in ManyToManyField related_name for related_name values be "+" or "foo+"

[smithdc1]

I find the leading and trailing underscore a little surprising here. PR #4382 originally introduced this code and doesn't really explain why either. 🤔

[felixxm]

Because these are names that are only used internally by Django.

[ngnpope]

I've noticed that these "internal" names leak into migrations. Making this change will cause migrations to be generated when users upgrade.

I guess that is inevitable to fix this, but perhaps the migrations need to be fixed to revert to related_name='+' when they are generated?

[felixxm]

I've noticed that these "internal" names leak into migrations. Making this change will cause migrations to be generated when users upgrade.

Are you sure? I didn't notice this in a sample project.

[felixxm]

For example:

class OtherModel(models.Model):
    pass


class OtherModelM2M(models.Model):
    m2m = models.ManyToManyField(OtherModel, related_name='+')

makemigrations didn't detect any changes with this PR.

[ngnpope]

Ok. I can confirm that it doesn't produce migrations. But it does mean that the value in existing migrations will be "wrong". I generated these operations and checked that the change didn't produce new migrations:

    operations = [
        migrations.CreateModel(
            name='Author',
            fields=[
                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
            ],
        ),
        migrations.CreateModel(
            name='Publisher',
            fields=[
                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
            ],
        ),
        migrations.CreateModel(
            name='Book',
            fields=[
                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
                ('authors', models.ManyToManyField(related_name='_book_authors_+', to='book.Author')),
                ('publisher', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='+', to='book.publisher')),
            ],
        ),
    ]

If I remove and regenerate the migrations I get the following:

    operations = [
        migrations.CreateModel(
            name='Author',
            fields=[
                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
            ],
        ),
        migrations.CreateModel(
            name='Publisher',
            fields=[
                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
            ],
        ),
        migrations.CreateModel(
            name='Book',
            fields=[
                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
                ('authors', models.ManyToManyField(related_name='_book_book_authors_+', to='book.Author')),
                ('publisher', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='+', to='book.publisher')),
            ],
        ),
    ]

So '_book_authors_+' changed to '_book_book_authors_+' when I regenerated from scratch. It doesn't seem so nice that this "internal" stuff "leaks" into migrations. Perhaps when creating migrations we should ensure the value of related_name is forced to '+' if it ends with '+'?

[ngnpope]

Perhaps when creating migrations we should ensure the value of related_name is forced to '+' if it ends with '+'?

We could edit the following in RelatedField.deconstruct() to undo the changes made in ManyToManyField.contribute_to_class():

django/django/db/models/fields/related.py

Lines 324 to 325 in 74fd233

	if self.remote_field.related_name is not None:
	kwargs['related_name'] = self.remote_field.related_name

Or handle it specially in ManyToManyField.deconstruct().

[felixxm]

@pope1ni Sure, but that's a separate issue not related with this patch.

[felixxm]

@manav014 Thanks 👍 I added a test.