[Django] #23604: Regression in allowing to handle M2M fields from the "receiving end" - follow up to #23329
Django
follow up to #23329
-------------------------------+----------------------------------
Reporter: nanuxbe | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 1.7
Severity: Normal | Keywords: to_field_allowed,M2M
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------+----------------------------------
Up until Django 1.5.8 and Django 1.6.5 in 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/23604>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
follow up to #23329
Reporter: nanuxbe | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 1.7
Keywords: | Triage Stage:
to_field_allowed,M2M | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by nanuxbe):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
If this is approved I'll create pull requests for 1.6, 1.7 and master
--
Ticket URL: <https://code.djangoproject.com/ticket/23604#comment:1>
Django
follow up to #23329
Reporter: nanuxbe | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 1.7
Keywords: | Triage Stage:
to_field_allowed,M2M | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Description changed by nanuxbe:
Old description:
> Up until Django 1.5.8 and Django 1.6.5 in 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.
New description:
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.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/23604#comment:2>
Django
follow up to #23329
Reporter: nanuxbe | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 1.7
Keywords: to_field_allowed,M2M | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Changes (by charettes):
* cc: charettes (added)
* needs_docs: 0 => 1
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/23604#comment:3>
Django
follow up to #23329
Reporter: nanuxbe | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 1.7
Keywords: to_field_allowed,M2M | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Comment (by nanuxbe):
Nope, I think I git it: https://github.com/django/django/pull/3311
And thanks for the quick reply.
--
Ticket URL: <https://code.djangoproject.com/ticket/23604#comment:4>
Django
follow up to #23329
Reporter: nanuxbe | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 1.7
Severity: Release blocker | Resolution: fixed
Keywords: to_field_allowed,M2M | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"a24cf217220dca44b7bd5b36ad9c14a96bca486e"]:
{{{
#!CommitTicketReference repository=""
revision="a24cf217220dca44b7bd5b36ad9c14a96bca486e"
Fixed #23604 -- Allowed related m2m fields to be references in the admin.
Thanks Simon Charette for review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23604#comment:5>
Django
follow up to #23329
Reporter: nanuxbe | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 1.7
Severity: Release blocker | Resolution: fixed
Keywords: to_field_allowed,M2M | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"f58392d8d8b68e2e7777768f39941cd995ce16e4"]:
{{{
#!CommitTicketReference repository=""
revision="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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23604#comment:6>
Django
follow up to #23329
Reporter: nanuxbe | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 1.7
Severity: Release blocker | Resolution: fixed
Keywords: to_field_allowed,M2M | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"c5c4bfa12aa0e8d4e8b46d77b6159f59330c3313"]:
{{{
#!CommitTicketReference repository=""
revision="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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23604#comment:7>
Django
follow up to #23329
Reporter: nanuxbe | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 1.7
Severity: Release blocker | Resolution: fixed
Keywords: to_field_allowed,M2M | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"314e9cd38f6bf72a619e5d07c99371439c33ba11"]:
{{{
#!CommitTicketReference repository=""
revision="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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23604#comment:8>
Django
follow up to #23329
Reporter: nanuxbe | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 1.7
Severity: Release blocker | Resolution: fixed
Keywords: to_field_allowed,M2M | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"f8d845910b17c119b7ed7fe8b448800bd39c17bc"]:
{{{
#!CommitTicketReference repository=""
revision="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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23604#comment:9>
Django
follow up to #23329
Reporter: Emmanuelle | Owner: nobody
Delescolle |
Type: Bug | Status: closed
Component: contrib.admin | Version: 1.7
Severity: Release blocker | Resolution: fixed
to_field_allowed,M2M |
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Comment (by Erik Romijn <erik@…>):
In [changeset:"29563cfb80d91d8710752f6abfff27a40ed53062" 29563cfb]:
{{{
#!CommitTicketReference repository=""
revision="29563cfb80d91d8710752f6abfff27a40ed53062"
Added Emmanuelle Delescolle to AUTHORS.
(Fixed #23604 in 2014 and was not added before.)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23604#comment:10>