#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 )
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)
Change History (12)
by , 10 years ago
Attachment: | failing_test.patch added |
---|
comment:2 by , 10 years ago
Description: | modified (diff) |
---|
comment:3 by , 10 years ago
Cc: | added |
---|---|
Needs documentation: | set |
Severity: | Normal → Release blocker |
Triage Stage: | Unreviewed → Accepted |
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:
if opts.many_to_many and field.primary_key
should do.- Since this check is cheap and doesn't require access to the set of registered model I would move it before they're collected.
- 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 toPizza
is added in the future. - Could you add a comment with a reference to the ticket over the assertion.
- 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 by , 10 years ago
Nope, I think I got it: https://github.com/django/django/pull/3311
And thanks for the quick reply.
comment:5 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Failing test (patch for master)