Commit aaad3e27 authored by Akis Kesoglou's avatar Akis Kesoglou Committed by Ramiro Morales
Browse files

Fixed #22217 - ManyToManyField.through_fields fixes. 

- Docs description of arguments mix up.
- Keep it from erroneously masking E332 check.
- Add checks E338 and E339, tweak message of E337.
parent f4d91638

django/db/models/fields/related.py

+74 −24
Original line number Diff line number Diff line
@@ -1904,21 +1904,7 @@ class ManyToManyField(RelatedField): 
                )

            )



        elif self.rel.through_fields is not None:

            if not len(self.rel.through_fields) >= 2 or not (self.rel.through_fields[0] and self.rel.through_fields[1]):

                errors.append(

                    checks.Error(

                        ("The field is given an iterable for through_fields, "

                         "which does not provide the names for both link fields "

                         "that Django should use for the relation through model "

                         "'%s'.") % qualified_model_name,

                        hint=None,

                        obj=self,

                        id='fields.E337',

                    )

                )



        elif not isinstance(self.rel.through, six.string_types):

        else:



            assert from_model is not None, \

                "ManyToManyField with intermediate " \
@@ -2018,6 +2004,78 @@ class ManyToManyField(RelatedField): 
                            id='fields.E336',

                        )

                    )



        # Validate `through_fields`

        if self.rel.through_fields is not None:

            # Validate that we're given an iterable of at least two items

            # and that none of them is "falsy"

            if not (len(self.rel.through_fields) >= 2 and

                    self.rel.through_fields[0] and self.rel.through_fields[1]):

                errors.append(

                    checks.Error(

                        ("Field specifies 'through_fields' but does not "

                         "provide the names of the two link fields that should be "

                         "used for the relation through model "

                         "'%s'.") % qualified_model_name,

                        hint=("Make sure you specify 'through_fields' as "

                              "through_fields=('field1', 'field2')"),

                        obj=self,

                        id='fields.E337',

                    )

                )



            # Validate the given through fields -- they should be actual

            # fields on the through model, and also be foreign keys to the

            # expected models

            else:

                assert from_model is not None, \

                    "ManyToManyField with intermediate " \

                    "tables cannot be checked if you don't pass the model " \

                    "where the field is attached to."



                source, through, target = from_model, self.rel.through, self.rel.to

                source_field_name, target_field_name = self.rel.through_fields[:2]



                for field_name, related_model in ((source_field_name, source),

                                                  (target_field_name, target)):



                    possible_field_names = []

                    for f in through._meta.fields:

                        if hasattr(f, 'rel') and getattr(f.rel, 'to', None) == related_model:

                            possible_field_names.append(f.name)

                    if possible_field_names:

                        hint = ("Did you mean one of the following foreign "

                                "keys to '%s': %s?") % (related_model._meta.object_name,

                                                        ', '.join(possible_field_names))

                    else:

                        hint = None



                    try:

                        field = through._meta.get_field(field_name)

                    except FieldDoesNotExist:

                        errors.append(

                            checks.Error(

                                ("The intermediary model '%s' has no field '%s'.") % (

                                    qualified_model_name, field_name),

                                hint=hint,

                                obj=self,

                                id='fields.E338',

                            )

                        )

                    else:

                        if not (hasattr(field, 'rel') and

                                getattr(field.rel, 'to', None) == related_model):

                            errors.append(

                                checks.Error(

                                    "'%s.%s' is not a foreign key to '%s'." % (

                                        through._meta.object_name, field_name,

                                        related_model._meta.object_name),

                                    hint=hint,

                                    obj=self,

                                    id='fields.E339',

                                )

                            )



        return errors



    def deconstruct(self):
@@ -2104,9 +2162,6 @@ class ManyToManyField(RelatedField): 
                    (link_field_name is None or link_field_name == f.name):

                setattr(self, cache_attr, getattr(f, attr))

                return getattr(self, cache_attr)

        # We only reach here if we're given an invalid field name via the

        # `through_fields` argument

        raise FieldDoesNotExist(link_field_name)



    def _get_m2m_reverse_attr(self, related, attr):

        "Function that can be curried to provide the related accessor or DB column name for the m2m table"
@@ -2133,12 +2188,7 @@ class ManyToManyField(RelatedField): 
                elif link_field_name is None or link_field_name == f.name:

                    setattr(self, cache_attr, getattr(f, attr))

                    break

        try:

        return getattr(self, cache_attr)

        except AttributeError:

            # We only reach here if we're given an invalid reverse field

            # name via the `through_fields` argument

            raise FieldDoesNotExist(link_field_name)



    def value_to_string(self, obj):

        data = ''

docs/ref/checks.txt

+3 −1
Original line number Diff line number Diff line
@@ -94,7 +94,9 @@ Related Fields 
* **fields.E334**: The model is used as an intermediate model by ``<model>``, but it has more than one foreign key from ``<model>``, which is ambiguous. You must specify which foreign key Django should use via the through_fields keyword argument.

* **fields.E335**: The model is used as an intermediate model by ``<model>``, but it has more than one foreign key to ``<model>``, which is ambiguous. You must specify which foreign key Django should use via the through_fields keyword argument.

* **fields.E336**: The model is used as an intermediary model by ``<model>``, but it does not have foreign key to ``<model>`` or ``<model>``.

* **fields.E337**: The field is given an iterable for through_fields, which does not provide the names for both link fields that Django should use for the relation through <model>.

* **fields.E337**: Field specifies ``through_fields`` but does not provide the names of the two link fields that should be used for the relation through ``<model>``.

* **fields.E338**: The intermediary model ``<through model>`` has no field ``<field name>``.

* **fields.E339**: ``<model>.<field name>`` is not a foreign key to ``<model>``.



Signals

~~~~~~~

docs/ref/models/fields.txt

+6 −5
Original line number Diff line number Diff line
@@ -1353,11 +1353,11 @@ that control how the relationship functions. 


        class Group(models.Model):

            name = models.CharField(max_length=128)

            members = models.ManyToManyField(Person, through='Membership', through_fields=('person', 'group'))

            members = models.ManyToManyField(Person, through='Membership', through_fields=('group', 'person'))



        class Membership(models.Model):

            person = models.ForeignKey(Person)

            group = models.ForeignKey(Group)

            person = models.ForeignKey(Person)

            inviter = models.ForeignKey(Person, related_name="membership_invites")

            invite_reason = models.CharField(max_length=64)
@@ -1368,9 +1368,10 @@ that control how the relationship functions. 
    above.



    ``through_fields`` accepts a 2-tuple ``('field1', 'field2')``, where

    ``field1`` is the name of the foreign key to the target model (``person``

    in this case), and ``field2`` the name of the foreign key to the model the

    :class:`ManyToManyField` is defined on (``group`` in this case).

    ``field1`` is the name of the foreign key to the model the

    :class:`ManyToManyField` is defined on (``group`` in this case), and

    ``field2`` the name of the foreign key to the target model (``person``

    in this case).



    When you have more than one foreign key on an intermediary model to any

    (or even both) of the models participating in a many-to-many relationship,

docs/topics/db/models.txt

+2 −2
Original line number Diff line number Diff line
@@ -440,12 +440,12 @@ explicit declaration defines how the two models are related. 
There are a few restrictions on the intermediate model:



* Your intermediate model must contain one - and *only* one - foreign key

  to the target model (this would be ``Person`` in our example), or you must

  to the source model (this would be ``Group`` in our example), or you must

  explicitly specify the foreign keys Django should use for the relationship

  using :attr:`ManyToManyField.through_fields <ManyToManyField.through_fields>`.

  If you have more than one foreign key and ``through_fields`` is not

  specified, a validation error will be raised. A similar restriction applies

  to the foreign key to the source model (this would be ``Group`` in our

  to the foreign key to the target model (this would be ``Person`` in our

  example).



* For a model which has a many-to-many relationship to itself through an

tests/invalid_models_tests/test_relative_fields.py

+69 −17
Original line number Diff line number Diff line
@@ -240,6 +240,32 @@ class RelativeFieldTests(IsolatedModelsTestCase): 
        ]

        self.assertEqual(errors, expected)



    def test_symmetric_self_reference_with_intermediate_table_and_through_fields(self):

        """Using through_fields in a m2m with an intermediate model shouldn't mask its incompatibility with symmetry."""

        class Person(models.Model):

            # Explicit symmetrical=True.

            friends = models.ManyToManyField('self',

                symmetrical=True,

                through="Relationship",

                through_fields=('first', 'second'))



        class Relationship(models.Model):

            first = models.ForeignKey(Person, related_name="rel_from_set")

            second = models.ForeignKey(Person, related_name="rel_to_set")

            referee = models.ForeignKey(Person, related_name="referred")



        field = Person._meta.get_field('friends')

        errors = field.check(from_model=Person)

        expected = [

            Error(

                'Many-to-many fields with intermediate tables must not be symmetrical.',

                hint=None,

                obj=field,

                id='fields.E332',

            ),

        ]

        self.assertEqual(errors, expected)



    def test_foreign_key_to_abstract_model(self):

        class Model(models.Model):

            foreign_key = models.ForeignKey('AbstractModel')
@@ -1071,43 +1097,69 @@ class M2mThroughFieldsTests(IsolatedModelsTestCase): 
            ValueError, 'Cannot specify through_fields without a through model',

            models.ManyToManyField, Fan, through_fields=('f1', 'f2'))



    def test_invalid_m2m_field(self):

    def test_invalid_order(self):

        """

        Tests that providing invalid source field name to ManyToManyField.through_fields

        raises FieldDoesNotExist.

        Tests that mixing up the order of link fields to ManyToManyField.through_fields

        triggers validation errors.

        """

        class Fan(models.Model):

            pass



        class Event(models.Model):

            invitees = models.ManyToManyField(Fan, through='Invitation', through_fields=('invalid_field', 'invitee'))

            invitees = models.ManyToManyField(Fan, through='Invitation', through_fields=('invitee', 'event'))



        class Invitation(models.Model):

            event = models.ForeignKey(Event)

            invitee = models.ForeignKey(Fan)

            inviter = models.ForeignKey(Fan, related_name='+')



        with self.assertRaisesMessage(FieldDoesNotExist, 'invalid_field'):

            Event().invitees.all()

        field = Event._meta.get_field('invitees')

        errors = field.check(from_model=Event)

        expected = [

            Error(

                ("'Invitation.invitee' is not a foreign key to 'Event'."),

                hint="Did you mean one of the following foreign keys to 'Event': event?",

                obj=field,

                id='fields.E339'),

            Error(

                ("'Invitation.event' is not a foreign key to 'Fan'."),

                hint="Did you mean one of the following foreign keys to 'Fan': invitee, inviter?",

                obj=field,

                id='fields.E339'),

        ]

        self.assertEqual(expected, errors)



    def test_invalid_m2m_reverse_field(self):

    def test_invalid_field(self):

        """

        Tests that providing invalid reverse field name to ManyToManyField.through_fields

        raises FieldDoesNotExist.

        Tests that providing invalid field names to ManyToManyField.through_fields

        triggers validation errors.

        """

        class Fan(models.Model):

            pass



        class Event(models.Model):

            invitees = models.ManyToManyField(Fan, through='Invitation', through_fields=('event', 'invalid_field'))

            invitees = models.ManyToManyField(Fan, through='Invitation', through_fields=('invalid_field_1', 'invalid_field_2'))



        class Invitation(models.Model):

            event = models.ForeignKey(Event)

            invitee = models.ForeignKey(Fan)

            inviter = models.ForeignKey(Fan, related_name='+')



        with self.assertRaisesMessage(FieldDoesNotExist, 'invalid_field'):

            Event().invitees.all()

        field = Event._meta.get_field('invitees')

        errors = field.check(from_model=Event)

        expected = [

            Error(

                ("The intermediary model 'invalid_models_tests.Invitation' has no field 'invalid_field_1'."),

                hint="Did you mean one of the following foreign keys to 'Event': event?",

                obj=field,

                id='fields.E338'),

            Error(

                ("The intermediary model 'invalid_models_tests.Invitation' has no field 'invalid_field_2'."),

                hint="Did you mean one of the following foreign keys to 'Fan': invitee, inviter?",

                obj=field,

                id='fields.E338'),

        ]

        self.assertEqual(expected, errors)



    def test_explicit_field_names(self):

        """
@@ -1129,11 +1181,11 @@ class M2mThroughFieldsTests(IsolatedModelsTestCase): 
        errors = field.check(from_model=Event)

        expected = [

            Error(

                ("The field is given an iterable for through_fields, "

                 "which does not provide the names for both link fields "

                 "that Django should use for the relation through model "

                 "'invalid_models_tests.Invitation'."),

                hint=None,

                ("Field specifies 'through_fields' but does not provide the names "

                 "of the two link fields that should be used for the relation "

                 "through model 'invalid_models_tests.Invitation'."),

                hint=("Make sure you specify 'through_fields' as "

                      "through_fields=('field1', 'field2')"),

                obj=field,

                id='fields.E337')]

        self.assertEqual(expected, errors)