Re: [Django] #12203: ManyToManyField with through model can't be used in admin
Django
-------------------------------------+-------------------------------------
Reporter: dgouldin | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: M2M, admin, | Triage Stage: Accepted
through, through_fields |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by timgraham):
See also #26998 for how this applies to django-taggit.
--
Ticket URL: <https://code.djangoproject.com/ticket/12203#comment:18>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: M2M, admin, | Triage Stage: Accepted
through, through_fields |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Dmitry Mugtasimov):
If you also waiting for the fix and have a simple through model for some
reason you may use `auto_created = True` to trick Django Admin:
{{{
class JobTitleExperienceThrough(models.Model):
title = models.ForeignKey('JobTitle', on_delete=models.CASCADE,
related_name='experiences_through')
experience = models.ForeignKey('Experience', on_delete=models.CASCADE,
related_name='titles_trough')
class Meta:
# TODO(dmu) MEDIUM: Remove `auto_created = True` after these
issues are fixed:
# https://code.djangoproject.com/ticket/12203
and
# https://github.com/django/django/pull/10829
auto_created = True
class JobTitle(models.Model):
name = models.CharField(max_length=64)
role = models.ForeignKey('JobRole', on_delete=models.CASCADE,
related_name='titles')
experiences = models.ManyToManyField('Experience',
through='JobTitleExperienceThrough',
related_name='titles',
blank=True)
class Meta:
ordering = ('id',)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/12203#comment:19>
Django
-------------------------------------+-------------------------------------
Reporter: David Gouldin | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: M2M, admin, | Triage Stage: Accepted
through, through_fields |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Sylvain Fankhauser):
Unfortunately, the `auto_created=True` hack doesn't work if you want to
use foreign keys to the `through` model. You'll get a `(fields.E300) Field
defines a relation with model 'JobTitleExperienceThrough', which is either
not installed, or is abstract`.
--
Ticket URL: <https://code.djangoproject.com/ticket/12203#comment:20>
Django
-------------------------------------+-------------------------------------
Reporter: David Gouldin | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: M2M, admin, | Triage Stage: Accepted
through, through_fields |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: frnhr (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/12203#comment:21>
Django
-------------------------------------+-------------------------------------
Reporter: David Gouldin | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: M2M, admin, | Triage Stage: Accepted
through, through_fields |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Dmitry Mugtasimov):
For some reason I ended up with this code:
{{{
class JobTitleAdmin(admin.ModelAdmin):
def formfield_for_manytomany(self, *args, **kwargs): # pylint:
disable=arguments-differ
# TODO(dmu) MEDIUM: Remove `auto_created = True` after these
issues are fixed:
# https://code.djangoproject.com/ticket/12203
and
# https://github.com/django/django/pull/10829
# We trick Django here to avoid `./manage.py makemigrations`
produce unneeded migrations
JobTitleExperienceThrough._meta.auto_created = True # pylint:
disable=protected-access
return super().formfield_for_manytomany(*args, **kwargs)
class JobTitle(models.Model):
name = models.CharField(max_length=64)
role = models.ForeignKey('JobRole', on_delete=models.CASCADE,
related_name='titles')
experiences = models.ManyToManyField('Experience',
through='JobTitleExperienceThrough',
related_name='titles',
blank=True)
class Meta:
ordering = ('id',)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/12203#comment:22>
Django
-------------------------------------+-------------------------------------
Reporter: David Gouldin | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: M2M, admin, | Triage Stage: Accepted
through, through_fields |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Dennis):
Replying to [comment:22 Dmitry Mugtasimov]:
The trick of setting `JobTitleExperienceThrough._meta.auto_created = True`
in `formfield_for_manytomany` does indeed enable the
`ModelMultipleChoiceField` on the admin page without causing migration
issues (as far as I can see).
However, there is a dangerous side-effect of using this, in case you have
other models with a "cascading" relation directly to your explicit
through-model, e.g. `models.ForeignKey(to=JobTitleExperienceThrough,
on_delete=models.CASCADE)`:
If the initial queryset for the m2m field `JobTitle.experiences` is
filtered (e.g. as in the docs
[https://docs.djangoproject.com/en/3.0/ref/contrib/admin/#django.contrib.admin.ModelAdmin.formfield_for_manytomany]),
and, for whatever reason, the filter excludes some `Experience` objects
that have previously been assigned, then the corresponding records from
`JobTitleExperienceThrough` will be silently deleted, including any
records for other models pointing to them.
This means there is a serious potential for "silent" data loss.
This is because the many-to-many relation will now be updated using
`ManyRelatedManager.set()` (via `ModelAdmin.save_related()` ->
`form.save_m2m()` -> `BaseModelForm._save_m2m()` ->
`ManyToManyField.save_form_data()`).
Silent deletion could be prevented by setting `on_delete=models.PROTECT`
on any relation to the explicit through-model, but then you would first
have to be aware of the necessity.
When using a "real" auto-created `through` table (i.e. an implicit one)
the issue does not arise, because there is no way to set a `ForeignKey` to
the implicit `through`, as far as I know.
When using an inline for `JobTitleExperienceThrough`, this issue does not
occur.
--
Ticket URL: <https://code.djangoproject.com/ticket/12203#comment:23>
Django
-------------------------------------+-------------------------------------
| Rufer
Type: Bug | Status: assigned
Component: contrib.admin | Version: dev
Keywords: M2M, admin, | Triage Stage: Accepted
through, through_fields |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Rosana Rufer
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/12203#comment:24>
Django
-------------------------------------+-------------------------------------
Reporter: David Gouldin | Owner: Rosana
| Rufer
Type: Bug | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: M2M, admin, | Triage Stage: Accepted
through, through_fields |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Similar to #9475, I think this "through" restriction is pretty artificial
at this point (since #6707 and #9475) and the restriction can probably be
removed.
Example PR here: https://github.com/django/django/pull/18594 (I don't
plan on pushing this through myself but feel free to take it over.)
Everything should just work. Though, yes, if you filter the admin queryset
and save, it's going to delete all of the existing rows that don't match
your queryset. That's the behavior I would expect, though maybe I could
see that being confusing if Inlines/FormSets don't delete instances that
don't match your queryset? Is that the issue?
M2M docs say "For example, if an owner can own multiple cars and cars can
belong to multiple owners – a many to many relationship – you could filter
the Car foreign key field to only display the cars owned by the User"
https://docs.djangoproject.com/en/dev/ref/contrib/admin/#django.contrib.admin.ModelAdmin.formfield_for_manytomany
Inline docs refer to just `ModelAdmin.get_queryset()`
https://docs.djangoproject.com/en/dev/ref/contrib/admin/#inlinemodeladmin-
options
`ModelAdmin.get_queryset` says: " One use case for overriding this method
is to show objects owned by the logged-in user"
https://docs.djangoproject.com/en/dev/ref/contrib/admin/#django.contrib.admin.ModelAdmin.get_queryset
--
Ticket URL: <https://code.djangoproject.com/ticket/12203#comment:25>
Django
-------------------------------------+-------------------------------------
Reporter: David Gouldin | Owner: Rosana
| Rufer
Type: Bug | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: M2M, admin, | Triage Stage: Accepted
through, through_fields |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Dmytro Litvinov (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/12203#comment:26>
Django
-------------------------------------+-------------------------------------
Reporter: David Gouldin | Owner: Rosana
| Rufer
Type: Bug | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: M2M, admin, | Triage Stage: Accepted
through, through_fields |
Needs tests: 1 | Patch needs improvement: 1
-------------------------------------+-------------------------------------
* needs_docs: 0 => 1
* needs_tests: 0 => 1
Comment:
[https://github.com/django/django/pull/18612 Recent, newer PR] (so far
this is the same as Collin's PR). I added some initial comments.
--
Ticket URL: <https://code.djangoproject.com/ticket/12203#comment:27>
Django
-------------------------------------+-------------------------------------
Reporter: David Gouldin | Owner: Rosana
| Rufer
Type: Bug | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: M2M, admin, | Triage Stage: Accepted
through, through_fields |
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Replying to [comment:22 Dmitry Mugtasimov]:
>
> {{{
> class JobTitleAdmin(admin.ModelAdmin):
> def formfield_for_manytomany(self, *args, **kwargs): # pylint:
disable=arguments-differ
> # TODO(dmu) MEDIUM: Remove `auto_created = True` after these
issues are fixed:
> # https://code.djangoproject.com/ticket/12203
and
> # https://github.com/django/django/pull/10829
>
> # We trick Django here to avoid `./manage.py makemigrations`
produce unneeded migrations
> JobTitleExperienceThrough._meta.auto_created = True # pylint:
disable=protected-access
> return super().formfield_for_manytomany(*args, **kwargs)
>
>
> class JobTitle(models.Model):
> name = models.CharField(max_length=64)
> role = models.ForeignKey('JobRole', on_delete=models.CASCADE,
> related_name='titles')
> experiences = models.ManyToManyField('Experience',
through='JobTitleExperienceThrough',
> related_name='titles',
blank=True)
>
> class Meta:
> ordering = ('id',)
> }}}
didn't work so I ended up with something like this:
{{{
class UserAdmin(BaseUserAdmin):
def check(self, **kwargs):
class ThroughModelAdminChecks(self.checks_class):
def _check_field_spec_item(self, obj, field_name, label):
these issues are fixed:
#
https://code.djangoproject.com/ticket/12203 and
#
User.user_permissions.through._meta.auto_created = True #
pylint: disable=protected-access
User.groups.through._meta.auto_created = True # pylint:
disable=protected-access
return super()._check_field_spec_item(obj, field_name,
label)
return ThroughModelAdminChecks().check(self, **kwargs)
class User(AbstractUser):
groups = models.ManyToManyField(
Group,
through=GroupPermission,
verbose_name=_("groups"),
blank=True,
help_text=_(
"The groups this user belongs to. A user will get all
permissions "
"granted to each of their groups."
),
related_name="user_set",
related_query_name="user",
)
user_permissions = models.ManyToManyField(
Permission,
through=UserPermission,
verbose_name=_("user permissions"),
blank=True,
help_text=_("Specific permissions for this user."),
related_name="user_set",
related_query_name="user",
)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/12203#comment:28>