Re: [Django] #897: Bi-Directional ManyToMany in Admin
Django
---------------------------------------+-------------------------------
Reporter: anonymous | Owner: nobody
Type: New feature | Status: new
Milestone: | Component: contrib.admin
Version: | Severity: Normal
Resolution: | Keywords:
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
---------------------------------------+-------------------------------
Changes (by CarstenF):
* cc: carsten.fuchs@… (added)
* easy: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:26>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
---------------------------------------+-------------------------------
Reporter: anonymous | Owner: nobody
Type: New feature | Status: new
Milestone: | Component: contrib.admin
Version: | Severity: Normal
Resolution: | Keywords:
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
---------------------------------------+-------------------------------
Changes (by croepha):
* ui_ux: => 0
Comment:
etienned's solution did not work for me, I am doing this instead:
{{{
class User(models.Model):
groups = models.ManyToManyField('Group',
db_table='testapp_user_groups')
class Group(models.Model):
users = models.ManyToManyField('User',
db_table=User.groups.field.db_table)
Group.users.through._meta.managed = False
}}}
This will also work for non symmetrical self M2M relationships:
{{{
class User(models.Model):
supervisors = models.ManyToManyField('self',
related_name='underlings_set', db_table='testapp_user_supervisors')
underlings = models.ManyToManyField('self',
related_name='supervisors_set', db_table=supervisors.db_table)
underlings._get_m2m_attr = supervisors._get_m2m_reverse_attr
underlings._get_m2m_reverse_attr = supervisors._get_m2m_attr
User.underlings.through._meta.managed = False
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:27>
Django
---------------------------------------+-------------------------------
Reporter: anonymous | Owner: nobody
Type: New feature | Status: new
Milestone: | Component: contrib.admin
Version: | Severity: Normal
Resolution: | Keywords:
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------+-------------------------------
Comment (by anonymous):
Check this out
{{{
class Test1(models.Model):
tests2 = models.ManyToManyField('Test2', blank=True)
class Test2(models.Model):
tests1 = models.ManyToManyField(Test1, through=Test1.tests2.through,
blank=True)
}}}
I guess this is the most native way
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:28>
Django
Reporter: anonymous | Owner: nobody
Type: New feature | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by CB):
To update comment 28:
This works great, but breaks south. So here's a fix for that:
{{{
class ReverseManyToManyField(models.ManyToManyField):
pass
try:
import south
except ImportError:
pass
else:
from south.modelsinspector import add_ignored_fields
add_ignored_fields([".*\.ReverseManyToManyField$",])
class Test1(models.Model):
tests2 = models.ManyToManyField('Test2', blank=True)
class Test2(models.Model):
tests1 = models.ReverseManyToManyField(Test1,
through=Test1.tests2.through, blank=True)
}}}
A possible Django enchancement would be adding this field (or rather, the
results of it's contribute_to_class, I think) instead of the
ReverseManyRelatedObjectsDescriptor
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:29>
Django
-------------------------------+------------------------------------
Reporter: anonymous | Owner: nobody
Type: New feature | Status: new
Component: contrib.admin | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by rhunwicks):
Using Django 1.5.1 I was getting:
{{{
app.test1: Reverse query name for m2m field 'tests2' clashes with m2m
field 'Test2.tests1'. Add a related_name argument to the definition for
'tests2'
}}}
This is almost certainly because my field and model name is the same.
Either way, I have solved it by adding `related_name` to `Test1.tests2`
and taking the opportunity to suppress it from `Test2`:
{{{
class Test1(models.Model):
tests2 = models.ManyToManyField('Test2', related_name='test2_set+',
blank=True)
class Test2(models.Model):
tests1 = models.ReverseManyToManyField(Test1,
through=Test1.tests2.through, blank=True)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:30>
Django
-------------------------------+------------------------------------
Reporter: anonymous | Owner: nobody
Type: New feature | Status: new
Component: contrib.admin | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by kux):
For people who still bump into this, it might be worth checking
https://github.com/kux/django-admin-extend
It provides a mechanism for injecting bidirectional many-to-many fields in
ModelAdmins that have already been defined by other apps.
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:31>
Django
-------------------------------+------------------------------------
Reporter: anonymous | Owner: nobody
Type: New feature | Status: new
Component: contrib.admin | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by Koen Biermans <koen@…>):
This seems to be more or less the same as #10964. I have a more generic
solution for it on that ticket that allows you to use the reverse
descriptor for M2M or nullable FK (like 'book_set') in the fields set for
any modelform. It is also available in admin (also for filter_horizontal
and filter_vertical).
Should one of these be considered as a duplicate?
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:32>
Django
-------------------------------+------------------------------------
Reporter: anonymous | Owner: nobody
Type: New feature | Status: new
Component: contrib.admin | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
* cc: cmawebsite@… (added)
Comment:
I started trying to get the patches from #10964 applying cleanly (and
correctly) to master, but I think it's worth waiting for the new _meta
options API. https://github.com/django/django/pull/2894
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:33>
Django
-------------------------------+------------------------------------
Reporter: anonymous | Owner: nobody
Type: New feature | Status: new
Component: contrib.admin | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by jschneier):
The new _meta options api has landed. I'm very interested in seeing this
make it across the finish line and would be happy to do the bulk of the
work with a little bit of direction. (Not sure where the best place to
post that kind of query).
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:34>
Django
-------------------------------+------------------------------------
Reporter: anonymous | Owner: nobody
Type: New feature | Status: new
Component: contrib.admin | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by collinanderson):
Awesome. As some have mentioned, I think the next basic step make auto
created reverse related fields (especially ManyToManyRel) into actual
fields.
After that, we'd need to allow ManyToManyRel to be used in any model form.
Here's what I think would be a start. (Doesn't work at all)
https://github.com/django/django/pull/3927/files
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:35>
Django
-------------------------------+------------------------------------
Reporter: anonymous | Owner: nobody
Type: New feature | Status: new
Component: contrib.admin | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by collinanderson):
#24317 should fix this.
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:36>
Django
-------------------------------+------------------------------------
Reporter: anonymous | Owner: nobody
Type: New feature | Status: new
Component: contrib.admin | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by auvipy):
Replying to [comment:36 collinanderson]:
> #24317 should fix this.
then the issue should be closed?
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:37>
Django
-------------------------------+------------------------------------
Reporter: anonymous | Owner: nobody
Type: New feature | Status: new
Component: contrib.admin | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by claudep):
Note the "should". I'd rather see the result of #24317 once committed
before closing this one.
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:38>
Django
-------------------------------+------------------------------------
Type: New feature | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
* status: new => assigned
* owner: nobody => auvipy
* version: => master
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:39>
Django
-------------------------------+------------------------------------
Reporter: anonymous | Owner:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Changes (by auvipy):
* owner: auvipy =>
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:40>
Django
-------------------------------+------------------------------------
Reporter: anonymous | Owner:
Type: New feature | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by Chronial):
This snippet provides a workaround for this: https://snipt.net/chrisdpratt
/symmetrical-manytomany-filter-horizontal-in-django-admin/
(and does it at the form level instead of with models)
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:41>
Django
-------------------------------+------------------------------------
Reporter: anonymous | Owner:
Type: New feature | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
* cc: mmitar@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:42>
Django
-------------------------------+------------------------------------
Reporter: anonymous | Owner:
Type: New feature | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by mitar):
The form approach does not work correctly because in the history view of
the model in admin there are no entries for changed reverse M2M fields.
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:43>
Django
-------------------------------+------------------------------------
Type: New feature | Status: new
Component: contrib.admin | Version:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by Ramiro Morales):
Isn't this use case covered by this admin functionality?:
https://docs.djangoproject.com/en/2.1/ref/contrib/admin/#working-with-
many-to-many-models
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:44>
Django
-------------------------------+------------------------------------
Reporter: anonymous | Owner: nobody
Type: New feature | Status: new
Component: contrib.admin | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by Collin Anderson):
It can be done with inlines, but not with easy left/right select widget.
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:45>
Django
-------------------------------+------------------------------------
Reporter: anonymous | Owner: nobody
Type: New feature | Status: new
Component: contrib.admin | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Old description:
> Allow amnytomany relationships to be defined both ways
> E.G
>
> {{{
> class ItemType(meta.Model):
> name = meta.CharField(maxlength=100)
> descritpion = meta.CharField(maxlength=250)
>
> class PropertyType(meta.Model):
> name = meta.CharField(maxlength=100)
> itemtypes = meta.ManyToManyField(ItemType)
> }}}
>
> Excellent. When I make a new property type in the admin screens I get a
> mutiselect window for item types.
>
> What I want to be able to do however is have this work back the other
> way too so that whe I create a new item i can specify what property
> types apply.
>
> Thanks
New description:
Allow manytomany relationships to be defined both ways
E.G
{{{
class ItemType(meta.Model):
name = meta.CharField(maxlength=100)
descritpion = meta.CharField(maxlength=250)
class PropertyType(meta.Model):
name = meta.CharField(maxlength=100)
itemtypes = meta.ManyToManyField(ItemType)
}}}
Excellent. When I make a new property type in the admin screen I get a
multiselect window for item types.
What I want to be able to do however is have this work back the other
way too so that when I create a new item I can specify what property
types apply.
Thanks
--
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:47>
Django
-------------------------------+------------------------------------
Reporter: anonymous | Owner: nobody
Type: New feature | Status: new
Component: contrib.admin | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
For clarity and to help whoever might take this up in the future:
This is still an issue in Django ''5.2.dev20240621100134''.
Given the following models
{{{
from django.db import models
class Student(models.Model):
name = models.CharField(max_length=50)
age = models.PositiveIntegerField()
def __str__(self) -> str:
return self.name
class Course(models.Model):
name = models.CharField(max_length=20)
students = models.ManyToManyField(
to=Student,
related_name="courses",
related_query_name="course",
)
def __str__(self) -> str:
return self.name
}}}
and the admin.py
{{{
from django.contrib import admin
from django.contrib.admin import ModelAdmin
from core.models import Course, Student
@admin.register(Course)
class CourseAdmin(ModelAdmin): ...
@admin.register(Student)
class StudentAdmin(ModelAdmin):
filter_horizontal = [
"course",
]
}}}
It raises an error
{{{
ERRORS:
<class 'core.admin.StudentAdmin'>: (admin.E020) The value of
'filter_horizontal[0]' must be a many-to-many field.
}}}
This error is raised by
**django.contrib.admin.checks.BaseModelAdminChecks._check_filter_item**.
However, getting the widget to render would require modifying
**ModelAdmin.get_form** as well.
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:48>
Django
-------------------------------+------------------------------------
Reporter: anonymous | Owner: nobody
Type: New feature | Status: new
Component: contrib.admin | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
* cc: Emmanuel Katchy (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:49>
Django
-------------------------------+------------------------------------
Reporter: anonymous | Owner: nobody
Type: New feature | Status: new
Component: contrib.admin | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
I was not aware of this very old issue and opened a duplicate where I
posted a workaround to make this work:
https://code.djangoproject.com/ticket/35878#comment:1
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:50>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 5.1
Severity: Release blocker | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Shai Berger):
* cc: Shai Berger (added)
* severity: Normal => Release blocker
* type: New feature => Bug
* version: => 5.1
Comment:
I was very surprised to see this 3-digit ticket, because I saw the
functionality mostly work on Django 2.2 to 4.2.
The issue described in comment:48 was in fact introduced in 5.1, while
fixing #35056: A check rejecting M2M fields with through models on the
admin was crashing on reverse relations, and the fix for it rejects
**all** reverse M2M fields from the admin, whether they do or don't have a
through relation.
This change in {{{django/contrib/admin/checks.py}}} should fix the check
itself, but some tests would need to be added too...
{{{#!diff
535c535
< if not field.many_to_many or isinstance(field,
models.ManyToManyRel):
---
> if not field.many_to_many:
539c539,544
< elif not field.remote_field.through._meta.auto_created:
---
> through = (
> field.through
> if isinstance(field, models.ManyToManyRel)
> else field.remote_field.through
> )
> if not through._meta.auto_created:
}}}
In the current state, this breaks existing Django projects -- it's a
regression introduced in the last release, so as far as I understand, it
merits Release Blocker status. I'm just not sure if it's better to make
the Release Blocker a new bug, leaving just whatever part is left of this
one ([comment:43 history?]) here.
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:51>
Django
-------------------------------+------------------------------------
Reporter: anonymous | Owner: nobody
Component: contrib.admin | Version: 5.1
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Sarah Boyce):
* cc: Mariusz Felisiak (added)
* severity: Release blocker => Normal
* type: Bug => New feature
Comment:
I think technically this never worked on 5.0, if I install 5.0 I get the
error fixed by #35056 and then for versions 5.0.1+, I get the error
described in the comment
In which case, 5.1 hasn't broken anything. So, I don't think this is a
release blocker.
> ... leaving just whatever part is left of this one (history?) here.
It wouldn't just be the history that needs fixing as part of the ticket,
but also tests need to be added to cover the support of the feature itself
(which is likely why any partial support could get reverted between
versions)
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:52>
Django
-------------------------------+------------------------------------
Reporter: anonymous | Owner: nobody
Type: New feature | Status: new
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
* version: 5.1 => dev
Comment:
I agree with Sarah's assessment. The workarounds mentioned since the
ticket was created do not work in Django 5.0 either. Considering that:
1. The ticket remains open (i.e., it has not been officially resolved),
2. Django 5.1 did not introduce any new issues related to this ticket, and
3. There are no tests ensuring the minimal desired behavior.
I believe this should still be considered a missing feature that requires
proper documentation and tests, at least. I'm open to further evaluate a
PR that would more officially document the cleanest "workaround" with some
tests as an interim step.
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:53>
Django
-------------------------------+------------------------------------
Reporter: anonymous | Owner: nobody
Type: New feature | Status: new
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Yep, I noted that the behavior changed after 5.0, and I missed that it had
already changed by 5.0.1.
Sorry about that bit of noise.
However, I'm not sure I understand this last bit:
Replying to [comment:53 Natalia Bidart]:
requires proper documentation and tests, at least. I'm open to further
evaluate a PR that would more officially document the cleanest
"workaround" with some tests as an interim step.
docs) whose production-code component is, essentially, the snippet I added
above, to restore the 4.2 functionality?
--
Ticket URL: <https://code.djangoproject.com/ticket/897#comment:54>