django-simple-history icon indicating copy to clipboard operation
django-simple-history copied to clipboard

Bulk Delete

Open rossm6 opened this issue 4 years ago • 2 comments

Thanks for this app, I've plugged it in and it looks good for all my needs except there is no bulk_delete option. Please may you add this feature and in the meantime point to the section of the documentation which explains how I can bulk create manual audit logs for each object I'm about to delete in bulk.

I'm a little miffed why bulk_create and bulk_update were both added to this app but this wasn't.

UPDATE

I think I've got the right code now so my question has been answered. But another has now arisen, see below.

def bulk_delete_with_history(objects, model, batch_size=None, default_user=None, default_change_reason="", default_date=None):
    """
    The package `simple_history` does not log what was deleted if the items
    are deleted in bulk.  This does.
    """
    model_manager = model._default_manager
    model_manager.filter(pk__in=[obj.pk for obj in objects]).delete()

    history_manager = get_history_manager_for_model(model)

    history_type = "-"
    historical_instances = []
    for instance in objects:
        history_user = getattr(
            instance,
            "_history_user",
            default_user or history_manager.model.get_default_history_user(
                instance),
        )
        row = history_manager.model(
            history_date=getattr(
                instance, "_history_date", default_date or timezone.now()
            ),
            history_user=history_user,
            history_change_reason=get_change_reason_from_object(
                instance) or default_change_reason,
            history_type=history_type,
            **{
                field.attname: getattr(instance, field.attname)
                for field in instance._meta.fields
                if field.name not in history_manager.model._history_excluded_fields
            }
        )
        if hasattr(history_manager.model, "history_relation"):
            row.history_relation_id = instance.pk
        historical_instances.append(row)

    return history_manager.bulk_create(
        historical_instances, batch_size=batch_size
    )

The problem though is a signal is sent for each object deleted by -

model_manager.filter(pk__in=[obj.pk for obj in objects]).delete()

So I need to disconnect this signal but I'm struggling. I've tried this, where Customer is just a class of mine I'm testing.

models.signals.post_delete.disconnect(HistoricalRecords.post_delete, sender=Customer)

Any ideas? I notice you guys add the signals using models.signals.class_prepared which is unusual for third party packages as per the Django docs.

UPDATE 2

https://stackoverflow.com/questions/64089809/is-there-a-way-to-disconnect-the-post-delete-signal-in-simple-history/64092484#64092484

I made a classic mistake with disconnecting signals. One must of course disconnect with the object that was connected. This is discussed here in more detail - django signal disconnect not working

This now works because I am disconnecting with reference to the correct object.

        receiver_object = models.signals.post_delete._live_receivers(Customer)[0]
        models.signals.post_delete.disconnect(receiver_object, sender=Customer)

Still, I think it would be nice if django-simple-history provided a bulk_delete utility function like they provide one for bulk_create and bulk_update.

rossm6 avatar Sep 27 '20 13:09 rossm6

@ddabble Would be really cool to have this. Would you accept an MR with this code?

@rossm6 Would you be able to make an MR with this snippet or could I just create one with your code?

max-wittig avatar Dec 05 '23 13:12 max-wittig

@rossm6 Thanks for creating this issue and investigating the signal part 🙏🏻

We tried your proposal to disconnect the post_delete() signal but unfortunately it did not work for us (the receiver object reference found within _live_receivers was not the same than in receivers).

Only the following worked instead:

    receiver_object = None
    for receiver in models.signals.post_delete.receivers:
        (_, sender_id), receiver_ref, weak_ref = receiver
        if sender_id == id(model):
            receiver_object = receiver_ref
            break
    if receiver_object:
        assert models.signals.post_delete.disconnect(receiver_object, sender=model)

where model is a model class that we pass to the bulk_delete_with_history function.

antoineauger avatar Apr 02 '24 07:04 antoineauger