models.E003 check incorrectly prevents duplicate ManyToMany through-self that differ by through_fields

[Simon Willison]

Let's say we're building a Twitter-style friendship model, where a user can follow other users, and can have users that follow them.

A sensible way to model that might look like this:

class User(models.Model):
    friends = models.ManyToManyField(
        'self', through='Followship', symmetrical=False,
        through_fields=('user', 'target'), related_name='+'
    )
    followers = models.ManyToManyField(
        'self', through='Followship', symmetrical=False,
        through_fields=('target', 'user'), related_name='+'
    )

class Followship(models.Model):
    user = models.ForeignKey(User, models.CASCADE, related_name='+')
    target = models.ForeignKey(User, models.CASCADE, related_name='+')

Here we have an intermediary table called "Followship", which relates a user to the target user that they are following.

We also have two helpful convenience ManyToMany fields defined on the user: user.friends.all() returns all other users that the user is following, while user.followers.all() returns the users that are following our current user.

The above models should work fine... but they don't, because Django's model checking framework throws the following error:

users.User: (models.E003) The model has two many-to-many relations through the intermediate model 'users.Followship'.

I don't think this warning is warranted in this case, because the through_fields arguments provided to the friends/followers ManyToManyFields mean that the relationships defined here make sense and the models should work correctly.

I've tested this theory by disabling the warning entirely using an over-ride class method on user, like this:

class User(models.Model):
    friends = models.ManyToManyField(
        'self', through='Followship', symmetrical=False,
        through_fields=('user', 'target'), related_name='+'
    )
    followers = models.ManyToManyField(
        'self', through='Followship', symmetrical=False,
        through_fields=('target', 'user'), related_name='+'
    )

    @classmethod
    def _check_m2m_through_same_relationship(cls):
        # Disable models.E003 check for this model
        return []

This does the trick: the check is suppressed, and the models work as expected.

I think the model check framework is being overly strict here. I think it should be modified to enable this pattern, provided the through_fields= parameters on the duplicate ManyToMany fields are present and differentiate the fields correctly.

[Simon Willison]

I've posted a pull request with a potential fix to Github: ​https://github.com/django/django/pull/6295

[Simon Willison]

As Simon Charette pointed out in a comment on the pull request, this may not be necessary. In the above example the same effect can be achieved using the related_name property, like this:

class User(models.Model):
    friends = models.ManyToManyField(
        'self', through='Followship', symmetrical=False,
        through_fields=('user', 'target'),
        related_name='followers'
    )

In which case, is there a real world use-case in which it's useful to be able to specify multiple ManyToManyFields in the way proposed by this ticket?

[Simon Charette]

The only use-case I can think of right now is working around limitations of reverse relationships (#897 seems to mention this exact workaround).

If we ever unify the reverse relationships API by exposing them as normal fields I think an explicit way of defining them would be necessary (to define options such as verbose_name).

In the meantime I'm not sure about what should be done here as I vaguely remember relying on a similar field definition in the past and it definitely worked in Django 1.2. On the other hand I wouldn't be surprised if it breaks Django in subtle ways as it seems untested (migrations come to mind here).

[Tim Graham]

In absence of a reported use case, I don't mind closing the issue for now.

[Raymond Penners]

This ticket got closed because of a missing use case. For what it is worth, our "real world use case" is the following:

class ShippingMethod(models.Model):
    ....
    to_countries = models.ManyToManyField(
        Country, through='carriers.ShippingMethodPrice',
        through_fields=('method', 'to_country'))

    # This is the 2nd ManyToManyField causing E003:
    #
    # from_countries = models.ManyToManyField(
    #     Country, through='carriers.ShippingMethodPrice',
    #     through_fields=('method', 'from_country'), related_name='+')


class ShippingMethodPrice(models.Model):
    ...
    method = models.ForeignKey(
        ShippingMethod,
        related_name='prices'
    )
    to_country = models.ForeignKey(Country)
    from_country = models.ForeignKey(Country, related_name='+')
    price = models.DecimalField()

So, basically, we have list of shipping methods, and each shipping method has a price list, containing a specific price per from/to-country combination.
Given a method, method.from_countries should represent the set of all countries that occur in the price list as a from-country.

I do not think the workaround suggested in https://code.djangoproject.com/ticket/26352#comment:2 works for us, as our case involves 3 models.

The hack to disable E003 seems to work, though a real fix (or suggestions how to receive the same result otherwise) would be appreciated.

[Tim Graham]

Does the original pull request solve it? If so, feel free to update and resend it.

[Simon Willison]

I've opened a new pull request that applies this change against the latest master of Django 2.x

​https://github.com/django/django/pull/10199

[Tim Graham <timograham@…>]

In 586a9dc4:

Fixed #26352 -- Made system check allow ManyToManyField to target the same model if through_fields differs.