Opened 10 years ago

Closed 10 years ago

Last modified 9 years ago

#24505 closed Bug (fixed)

Multiple ManyToManyFields to same "to" model with related_name set to '+' mix up badly

Reported by: Gergely Polonkai Owned by: Marco Fucci
Component: Database layer (models, ORM) Version: 1.7
Severity: Normal Keywords:
Cc: jeroen@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have a model, ProductField, which has a fillable_by and an approvable_by field. Both fields are defined as

ManyToManyField(to=Group, related_name='+')

Now when I add a group to fillable_by and another one to approvable_by, the fillable_by gets overridden by approvable_by. I cannot find anything related to this in the documentation, but it seems a pretty bad behaviour to me.

The problem still exists in latest 1.7.7 version.

Attachments (1)

issue24505_testcase.diff (1013 bytes ) - added by Baptiste Mispelon 10 years ago.
Reproduction testcase

Download all attachments as: .zip

Change History (14)

comment:1 by Baptiste Mispelon, 10 years ago

Triage Stage: UnreviewedAccepted

Hi,

I can confirm that something strange is going on.
I'm attaching a testcase patch to reproduce the issue.

Thanks.

by Baptiste Mispelon, 10 years ago

Attachment: issue24505_testcase.diff added

Reproduction testcase

comment:2 by Marco Fucci, 10 years ago

Owner: changed from nobody to Marco Fucci
Status: newassigned

comment:3 by Marco Fucci, 10 years ago

PR: https://github.com/django/django/pull/4382

"This fixes a bug with multiple many to many fields to the same to model and related_name set to +.

This happens because Django is still using backwards relations internally so multiple fields with the same related_name are sometimes overridden and the last field defined wins.

There is still a bug with Django not throwing an exception when defining m2m fields with the same related_name and backwards relation enabled but I think it's slightly different from this one and should be managed differently.

Ideally we would refactor the ORM so that it doesn't use backwards relations internally when disabled but this particular fix was easy to implement (although hard to find) so I would be happy with it.

I had to change a few tests so I would like somebody to take a look at them and double-check that it makes sense if possible."

Thanks to Marc Tamlyn for the help and bmispelon for the TestCase.

comment:4 by Marco Fucci, 10 years ago

Has patch: set

comment:5 by Marco Fucci, 10 years ago

I did a bit of research after jtiai pointed me in the right direction and it seems that we've had the same problem before.

To fix the problem, we documented a workaround where related_names had to be unique, even the ones ending in + #15932.

We then thought we had fixed it in #21375 so we deleted the documented workaround #21491.

Finally, it seems that the bug is back and addressed here.

comment:6 by Tim Graham, 10 years ago

Patch needs improvement: set

Patch needs a rebase.

comment:7 by Marco Fucci, 10 years ago

Patch needs improvement: unset

Rebased

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

Resolution: fixed
Status: assignedclosed

In 4ee08958:

Fixed #24505 -- Fixed clash with hidden m2m fields.

Added support for multiple m2m fields with the same 'to' model
and with related_name set to '+'.

comment:9 by Jeroen Dekkers, 9 years ago

Cc: jeroen@… added

I just hit this bug with 1.8 and spend a lot of time to figure out what was going on. Given that the fix is pretty trivial, would it be possible to backport it to 1.8?

comment:10 by Tim Graham, 9 years ago

If you'd like to send a pull request, I'll merge it. Please include a mention in the 1.8.3 release notes.

comment:11 by Tim Graham <timograham@…>, 9 years ago

In 0e2d3b9:

[1.8.x] Fixed #24505 -- Fixed clash with hidden m2m fields.

Added support for multiple m2m fields with the same 'to' model
and with related_name set to '+'.

Backport of 4ee08958f154594b538207a53c1d457687b3f7ae from master

comment:12 by Tim Graham <timograham@…>, 9 years ago

In 061801e3:

Refs #24505 -- Forwardported 1.8.5 release note.

comment:13 by Tim Graham <timograham@…>, 9 years ago

In 3569e9d4:

[1.9.x] Refs #24505 -- Forwardported 1.8.5 release note.

Backport of 061801e3dfb3f88550cdaeef1a6dd1c24c13d53d from master

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