#21771 closed Bug (fixed)

ModelAdmin's log_deletion method behaves incorrectly compared to other log methods

Reported by: Keryn Knight <django@…> Owned by: nobody
Component: contrib.admin Version: master
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 Changed 19 months ago by bmispelon

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

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.

comment:3 Changed 18 months ago by claudep

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.
Back to Top