Opened 3 years ago

Closed 3 years ago

Last modified 2 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 3 years ago.
Reproduction testcase

Download all attachments as: .zip

Change History (14)

comment:1 Changed 3 years ago by Baptiste Mispelon

Triage Stage: UnreviewedAccepted

Hi,

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

Thanks.

Changed 3 years ago by Baptiste Mispelon

Attachment: issue24505_testcase.diff added

Reproduction testcase

comment:2 Changed 3 years ago by Marco Fucci

Owner: changed from nobody to Marco Fucci
Status: newassigned

comment:3 Changed 3 years ago by Marco Fucci

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 Changed 3 years ago by Marco Fucci

Has patch: set

comment:5 Changed 3 years ago by Marco Fucci

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 Changed 3 years ago by Tim Graham

Patch needs improvement: set

Patch needs a rebase.

comment:7 Changed 3 years ago by Marco Fucci

Patch needs improvement: unset

Rebased

comment:8 Changed 3 years ago by Tim Graham <timograham@…>

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 Changed 3 years ago by Jeroen Dekkers

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 Changed 3 years ago by Tim Graham

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 Changed 2 years ago by Tim Graham <timograham@…>

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 Changed 2 years ago by Tim Graham <timograham@…>

In 061801e3:

Refs #24505 -- Forwardported 1.8.5 release note.

comment:13 Changed 2 years ago by Tim Graham <timograham@…>

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