Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#23604 closed Bug (fixed)

Regression in allowing to handle M2M fields from the "receiving end" - follow up to #23329

Reported by: Emmanuelle Delescolle Owned by: nobody
Component: contrib.admin Version: 1.7
Severity: Release blocker Keywords: to_field_allowed, M2M
Cc: Simon Charette Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description (last modified by Emmanuelle Delescolle)

Up until Django 1.5.8 and Django 1.6.5 it was possible to have a ManyToMany field displayed in the admin on the "receiving-end" of the relationship with the ability to add new records.
Due to the restrictiveness of the new to_field_allowed method, one cannot open the popup for adding new records to the Model where the ManyToMany relationship is defined.

I realize this usage of the admin site is undocumented but, since the current code allows to perform that from the "receiving-end" of ForeignKey fields, IMHO it should also work for ManyToMany fields.

If the descriptions sounds too vague, see http://www.lasolution.be/blog/related-manytomanyfield-django-admin-site.html for a complete step-by-step explanation on how to reproduce.

Attachments (2)

failing_test.patch (1.1 KB) - added by Emmanuelle Delescolle 5 years ago.
Failing test (patch for master)
fix.patch (820 bytes) - added by Emmanuelle Delescolle 5 years ago.
Fix

Download all attachments as: .zip

Change History (12)

Changed 5 years ago by Emmanuelle Delescolle

Attachment: failing_test.patch added

Failing test (patch for master)

Changed 5 years ago by Emmanuelle Delescolle

Attachment: fix.patch added

Fix

comment:1 Changed 5 years ago by Emmanuelle Delescolle

If this is approved I'll create pull requests for 1.6, 1.7 and master

comment:2 Changed 5 years ago by Emmanuelle Delescolle

Description: modified (diff)

comment:3 Changed 5 years ago by Simon Charette

Cc: Simon Charette added
Needs documentation: set
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

Thanks for the detailed report. The issue seems valid and the patch seems looks like the correct approach.

Could you open a PR against master only with the following changes:

  1. if opts.many_to_many and field.primary_key should do.
  2. Since this check is cheap and doesn't require access to the set of registered model I would move it before they're collected.
  3. Could you define an unreferenced model instead of relying on the Pizza one to make sure the additional checks are still covered if another model referring to Pizza is added in the future.
  4. Could you add a comment with a reference to the ticket over the assertion.
  5. Since this is a regression and will be backported you'll need to add the changes to the 1.4.16, 1.5.11, 1.6.8 and 1.7.1 release notes.

Let me know if you need additional help to land this.

comment:4 Changed 5 years ago by Emmanuelle Delescolle

Nope, I think I got it: https://github.com/django/django/pull/3311

And thanks for the quick reply.

Last edited 5 years ago by Emmanuelle Delescolle (previous) (diff)

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

Resolution: fixed
Status: newclosed

In a24cf217220dca44b7bd5b36ad9c14a96bca486e:

Fixed #23604 -- Allowed related m2m fields to be references in the admin.

Thanks Simon Charette for review.

comment:6 Changed 5 years ago by Tim Graham <timograham@…>

In f58392d8d8b68e2e7777768f39941cd995ce16e4:

[1.4.x] Fixed #23604 -- Allowed related m2m fields to be references in the admin.

Thanks Simon Charette for review.

Backport of a24cf21722 from master

comment:7 Changed 5 years ago by Tim Graham <timograham@…>

In c5c4bfa12aa0e8d4e8b46d77b6159f59330c3313:

[1.6.x] Fixed #23604 -- Allowed related m2m fields to be references in the admin.

Thanks Simon Charette for review.

Backport of a24cf21722 from master

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

In 314e9cd38f6bf72a619e5d07c99371439c33ba11:

[1.5.x] Fixed #23604 -- Allowed related m2m fields to be references in the admin.

Thanks Simon Charette for review.

Backport of a24cf21722 from master

comment:9 Changed 5 years ago by Tim Graham <timograham@…>

In f8d845910b17c119b7ed7fe8b448800bd39c17bc:

[1.7.x] Fixed #23604 -- Allowed related m2m fields to be references in the admin.

Thanks Simon Charette for review.

Backport of a24cf21722 from master

comment:10 Changed 3 years ago by Erik Romijn <erik@…>

In 29563cfb:

Added Emmanuelle Delescolle to AUTHORS.

(Fixed #23604 in 2014 and was not added before.)

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