[Django] #21771: ModelAdmin's log_deletion method behaves incorrectly compared to other log methods
Django
methods
-----------------------------------------+--------------------
Reporter: Keryn Knight <django@…> | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------------+--------------------
The current implementation of
[https://github.com/django/django/blob/8f04f53dd847896f49a9bc367bb7269984ebdb6e/django/contrib/admin/options.py#L713
log_deletion] makes use of `self.model` to figure out the appropriate
`content_type`, even though `object` is passed in. The `log_addition` and
`log_change` methods correctly use the provided object to ascertain the
type.
I can only assume that at some point obj wasn't intended/guaranteed to
exist at the point `log_deletion` is called, but in both places it's used
([https://github.com/django/django/blob/8f04f53dd847896f49a9bc367bb7269984ebdb6e/django/contrib/admin/options.py#L1580
ModelAdmin.delete_view],
[https://github.com/django/django/blob/17c66e6fe77bc5eb2bd9e6f8f032a105d3cac3d7/django/contrib/admin/actions.py#L48,
actions.delete_selected]) there is already a requirement (or at least an
assumption) that the object exists.
I consider this to be a bug, because it means that the following use case
doesn't work, which is how I came across it:
{{{
class MyModelAdmin(ModelAdmin):
def log_deletion(self, request, object, object_repr):
super(MyModelAdmin, self).log_deletion(request, object,
object_repr)
super(MyModelAdmin, self).log_deletion(request,
object.content_object, object_repr)
}}}
where `object.content_object` is a GFK back to the "owner" of the object.
Instead of getting two deletion messages, to the instance and the
GFK/parent, you end up with two for the instance, because the content_type
of the GFK/parent is never used.
The equivalent code for the `log_addition` and `log_change` methods
appears to work.
--
Ticket URL: <https://code.djangoproject.com/ticket/21771>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
methods
Reporter: Keryn Knight | Owner: nobody
<django@…> | Status: new
Type: Bug | Version: master
Component: contrib.admin | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by bmispelon):
* needs_better_patch: => 0
* stage: Unreviewed => Accepted
* needs_tests: => 0
* needs_docs: => 0
Comment:
Hi,
From what I can tell, this is a remnant of a limitation that existed when
the feature was introduced (6ba64896622213ce44ace2c605e4eaafed1e9fc5).
According to the original docstring, `log_deletion` used to be called
**after** the object had been deleted and I'm guessing that's why
`self.model` was used instead of `object`.
However, according to the current docstring, `log_deletion` is now called
**before** deletion so it should be safe to used `object` here was well.
--
Ticket URL: <https://code.djangoproject.com/ticket/21771#comment:1>
Django
methods
Reporter: Keryn Knight | Owner: nobody
<django@…> | Status: new
Type: Bug | Version: master
Component: contrib.admin | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by alasdair):
Pull request https://github.com/django/django/pull/2251
--
Ticket URL: <https://code.djangoproject.com/ticket/21771#comment:2>
Django
methods
Reporter: Keryn Knight | Owner: nobody
Type: Bug | Version: master
Component: contrib.admin | Resolution: fixed
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
PR pushed in [1b29d3289473a6c3ce565c0ddc1bed4d5b5ac9a3]
--
Ticket URL: <https://code.djangoproject.com/ticket/21771#comment:3>