Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#23862 closed Bug (fixed)

ManyToManyRel.get_related_field() doesn't account for defined through to_field

Reported by: Simon Charette Owned by: Simon Charette
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

While investigating edge cases to_field_allowed has to cope with I stumbled upon the following unusual scenario:

class Ingredient(models.Model):
    iname = models.CharField(max_length=20, unique=True)

class Recipe(models.Model):
    rname = models.CharField(max_length=20, unique=True)
    ingredients = models.ManyToManyField(
        Ingredient, through='RecipeIngredient', related_name='recipes'
    )

class RecipeIngredient(models.Model):
    ingredient = models.ForeignKey(Ingredient, to_field='iname')
    recipe = models.ForeignKey(Recipe, to_field='rname')

Which is not correctly handled by the actual ManyToManyRel.get_related_field() implementation that assumes this is always the primary key on the target model.

Since ManyToManyRel is an implementation detail the provided test is against ManyToManyField.get_choices() method which relies on it. The patch also includes tests for related managers interaction since this reference scenario was not tested in m2m_through yet.

The fact that the implementation doesn't actually return the correct field means that we don't have to backport the required to_field_allowed adjustments (which I'll submit as another ticket/patch to facilitate backports of #23754 and #23839) since it never worked in previous Django versions.

Change History (5)

comment:1 by Anssi Kääriäinen, 10 years ago

Patch needs improvement: set
Triage Stage: UnreviewedAccepted

comment:2 by Simon Charette, 10 years ago

Patch needs improvement: unset

Updated patch.

comment:3 by Anssi Kääriäinen, 10 years ago

Triage Stage: AcceptedReady for checkin

comment:4 by Tim Graham <timograham@…>, 10 years ago

Resolution: fixed
Status: newclosed

In c7087bc777a078f71a0705d9d0eba51d2679a206:

Fixed #23862 -- Made ManyToManyRel.get_related_field() respect to_field.

comment:5 by Simon Charette <charette.s@…>, 10 years ago

In 3a9aa155e2f7326df669953980ac87e78e932c43:

Fixed #23915 -- Made sure m2m fields through non-pk to_field are allowed in the admin.

refs #23754, #23862

Note: See TracTickets for help on using tickets.
Back to Top