makinacorpus / django-safedelete

Mask your objects instead of deleting them from your database.
https://django-safedelete.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
677 stars 122 forks source link

HARD_DELETE in SafeDeleteQueryset bypasses custom delete logic #231

Open ColemanDunn opened 1 year ago

ColemanDunn commented 1 year ago

When hard deleting a queryset, custom delete logic (aka overriding the delete method in your model with a call to super) is not called, but it is when doing hard delete (force_policy=HARD_DELETE). Any idea why? Is the way around this just to loop over the objects manually. Is it possible to make this not bypass the model's delete method?

For reference, here is the code that shows SafeDeleteQueryset.delete bypassing the model's delete method for when the policy is HARD_DELETE, but normally for the other deletes.

` def delete(self, force_policy: Optional[int] = None) -> Tuple[int, Dict[str, int]]: """Overrides bulk delete behaviour.

    .. note::
        The current implementation loses performance on bulk deletes in order
        to safely delete objects according to the deletion policies set.

    .. seealso::
        :py:func:`safedelete.models.SafeDeleteModel.delete`
    """
    assert self.query.can_filter(), "Cannot use 'limit' or 'offset' with delete."

    if force_policy == NO_DELETE:
        return (0, {})
    elif force_policy == HARD_DELETE:
        return self.hard_delete_policy_action()
    else:
        deleted_counter: Counter = Counter()
        # TODO: Replace this by bulk update if we can
        for obj in self.all():
            _, delete_response = obj.delete(force_policy=force_policy)
            deleted_counter.update(delete_response)
        self._result_cache = None
        return sum(deleted_counter.values()), dict(deleted_counter)
delete.alters_data = True  # type: ignore

def hard_delete_policy_action(self) -> Tuple[int, Dict[str, int]]:
    # Normally hard-delete the objects.
    self.query._filter_visibility()
    return super().delete()

`

Gagaro commented 1 year ago

Yes that's the default Django behavior.

We iterate over the objects otherwise to be able to handle all other policies. That's clearly not the most optimized way.