Skip to content

Remove unused snippets delete-multiple view #10058

@laymonage

Description

@laymonage

The feature to delete multiple snippets was added in #4610 (see also #3802). It was implemented using a combination of custom JavaScript and refactoring the delete view to handle multiple objects, which is then hooked to another url pattern. Later, the view was refactored to be a subclass of the generic DeleteView in #8346, retaining the multiple objects logic.

As of #8574, deleting multiple snippets is implemented using the bulk actions framework. The delete bulk action uses its own view, and the multiple delete mechanism on the listing view was rerouted to use the bulk actions feature. This makes the delete multiple logic on the snippets' DeleteView redundant, and in fact there's no way to access the view other than through the URL directly:

path("multiple/delete/", self.delete_view, name="delete-multiple"),
path("delete/<str:pk>/", self.delete_view, name="delete"),

I'd suggest that we remove the multiple objects handling logic from the view, so it can reuse the code from the generic delete view instead:

def get_object(self, queryset=None):
# DeleteView requires either a pk kwarg or a positional arg, but we use
# an `id` query param for multiple objects. We need to explicitly override
# this so that we don't have to override `post()`.
return None
def get_objects(self):
# Replaces get_object to allow returning multiple objects instead of just one
if self.pk:
return [get_object_or_404(self.model, pk=unquote(self.pk))]
ids = self.request.GET.getlist("id")
objects = self.model.objects.filter(pk__in=ids)
return objects

In addition, the delete-multiple URL should also be removed.

Reducing this code duplication would allow us to use less code when implementing #10008.

Given that this code has been left unused since 4.0, I think it's fair to remove this without a deprecation process in 5.0.

Additionally, looks like the {before,after}_delete_snippet hook wasn't included in #8574. This means that realistically the hook can only be triggered when deleting a single snippet. I'm not sure what approach we should do here:

  • Call the {before,after}_delete_snippet hooks inside the delete bulk action view.
  • However, bulk actions have their own hooks, so maybe we could just instruct people to use that instead?
    • Although these hooks called in form_valid instead of the beginning of dispatch(), so the before_bulk_action hook won't get called on a GET request...

I also noticed that the docs that the hooks accept a queryset of instances, when in reality it's not always the case: when there's only a single snippet, the hooks are passed a list of the single instance, not a queryset. This has been the case since the hooks were introduced in #6269 though, so probably not a big deal.

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions