Opened 11 years ago
Closed 11 years ago
#21771 closed Bug (fixed)
ModelAdmin's log_deletion method behaves incorrectly compared to other log methods
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The current implementation of 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 (ModelAdmin.delete_view, 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.
Change History (3)
comment:1 by , 11 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:3 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
PR pushed in [1b29d3289473a6c3ce565c0ddc1bed4d5b5ac9a3]
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 whyself.model
was used instead ofobject
.However, according to the current docstring,
log_deletion
is now called before deletion so it should be safe to usedobject
here was well.