From 4f1d3509ff0f8958adb288ec53945fde76c31260 Mon Sep 17 00:00:00 2001 From: Karl Hobley <karl@torchbox.com> Date: Mon, 19 Dec 2016 15:53:44 +0000 Subject: [PATCH] Bulk deletion of form submissions (#3233) Remove actions column Basic implementation of bulk deletion of form submissions Moved select all checkbox Pluralised messages Hide delete button when it's not needed Test updates for multiple form submission delete Use checkbox value instead of submissionId data attribute Simplify way submission IDs are sent to the server As per https://github.com/wagtail/wagtail/pull/3233#discussion_r94417831 Removed inline styling Simplify state management logic As per: https://github.com/wagtail/wagtail/pull/3233#discussion_r94426467 Call updateActions on load to bring checkboxes and delete button in sync Release note for #3233 --- CHANGELOG.txt | 1 + docs/releases/1.9.rst | 6 ++ .../wagtailforms/confirm_delete.html | 14 ++-- .../wagtailforms/index_submissions.html | 65 +++++++++++++++- .../wagtailforms/list_submissions.html | 14 ++-- wagtail/wagtailforms/tests/test_views.py | 75 ++++++++++++------- wagtail/wagtailforms/urls.py | 2 +- wagtail/wagtailforms/views.py | 26 +++++-- 8 files changed, 157 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 6c622b9635..4e77d79ab6 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -4,6 +4,7 @@ Changelog 1.9 (xx.xx.xxxx) - IN DEVELOPMENT ~~~~~~~~~~~~~~~~ + * Form builder form submissions can now be bulk-deleted (Karl Hobley) * `get_context` methods on StreamField blocks can now access variables from the parent context (Mikael Svensson, Peter Baumgartner) * Added `before_copy_page` and `after_copy_page` hooks (Matheus Bratfisch) * View live / draft links in the admin now consistently open in a new window (Marco Fucci) diff --git a/docs/releases/1.9.rst b/docs/releases/1.9.rst index 72e21b06d0..9c9802317c 100644 --- a/docs/releases/1.9.rst +++ b/docs/releases/1.9.rst @@ -10,6 +10,12 @@ Wagtail 1.9 release notes - IN DEVELOPMENT What's new ========== +Bulk-deletion of form submissions +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Form builder form submissions can now be deleted in bulk from the form submissions index page. This feature was sponsored by St John's College, Oxford and developed by Karl Hobley. + + Accessing parent context from StreamField block ``get_context`` methods ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/wagtail/wagtailforms/templates/wagtailforms/confirm_delete.html b/wagtail/wagtailforms/templates/wagtailforms/confirm_delete.html index e023f5f022..a1826a2b07 100644 --- a/wagtail/wagtailforms/templates/wagtailforms/confirm_delete.html +++ b/wagtail/wagtailforms/templates/wagtailforms/confirm_delete.html @@ -1,19 +1,23 @@ {% extends "wagtailadmin/base.html" %} {% load i18n %} -{% block titletag %}{% blocktrans with title=page.title %}Delete {{ title }}{% endblocktrans %}{% endblock %} +{% block titletag %}{% blocktrans with title=page.title %}Delete form data {{ title }}{% endblocktrans %}{% endblock %} {% block bodyclass %}menu-explorer{% endblock %} {% block content %} - {% trans "Delete" as del_str %} + {% trans "Delete form data" as del_str %} {% include "wagtailadmin/shared/header.html" with title=del_str subtitle=page.title icon="doc-empty-inverse" %} <div class="nice-padding"> <p> - {% trans 'Are you sure you want to delete this form submission?' %} + {% blocktrans count counter=submissions.count %} + Are you sure you want to delete this form submission? + {% plural %} + Are you sure you want to delete these form submissions? + {% endblocktrans %} </p> - <form action="{% url 'wagtailforms:delete_submission' page.id submission.id %}" method="POST"> + <form action="{% url 'wagtailforms:delete_submissions' page.id %}?{{ request.GET.urlencode }}" method="POST"> {% csrf_token %} - <input type="submit" value="{% trans 'Delete it' %}" class="button serious"> + <input type="submit" value="{% trans 'Delete' %}" class="button serious"> </form> </div> {% endblock %} diff --git a/wagtail/wagtailforms/templates/wagtailforms/index_submissions.html b/wagtail/wagtailforms/templates/wagtailforms/index_submissions.html index 1d13389ad8..08c297a7e1 100644 --- a/wagtail/wagtailforms/templates/wagtailforms/index_submissions.html +++ b/wagtail/wagtailforms/templates/wagtailforms/index_submissions.html @@ -23,6 +23,63 @@ }, lang: 'lang' }); + + var selectAllCheckbox = document.getElementById('select-all'); + var deleteButton = document.getElementById('delete-submissions'); + var selectedSubmissions = {}; + + function updateActions() { + var submissionCheckboxes = $('input[type=checkbox].select-submission'); + var someSubmissionsSelected = submissionCheckboxes.is(':checked'); + var everySubmissionSelected = !submissionCheckboxes.is(':not(:checked)'); + + // Select all box state + if (everySubmissionSelected) { + // Every submission has been selected + selectAllCheckbox.checked = true; + selectAllCheckbox.indeterminate = false; + } else if (someSubmissionsSelected) { + // At least one, but not all submissions have been selected + selectAllCheckbox.checked = false; + selectAllCheckbox.indeterminate = true; + } else { + // No submissions have been selected + selectAllCheckbox.checked = false; + selectAllCheckbox.indeterminate = false; + } + + // Delete button state + if (someSubmissionsSelected) { + deleteButton.classList.remove('disabled') + deleteButton.style.visibility = "visible"; + } else { + deleteButton.classList.add('disabled') + deleteButton.style.visibility = "hidden"; + } + } + + + // Event handlers + + $(selectAllCheckbox).on('change', function() { + let checked = this.checked; + + // Update checkbox states + $('input[type=checkbox].select-submission').each(function() { + this.checked = checked; + }); + + updateActions(); + }); + + $('input[type=checkbox].select-submission').on('change', function() { + updateActions(); + }); + + // initial call to updateActions to bring delete button state in sync with checkboxes + // in the case that some checkboxes are pre-checked (which will be the case in some + // browsers when using the back button) + updateActions(); }); </script> {% endblock %} @@ -55,10 +112,12 @@ </header> <div class="nice-padding"> {% if submissions %} - {% include "wagtailforms/list_submissions.html" %} + <form action="{% url 'wagtailforms:delete_submissions' form_page.id %}" method="get"> + {% include "wagtailforms/list_submissions.html" %} - {% include "wagtailadmin/shared/pagination_nav.html" with items=submissions is_searching=False linkurl='-' %} - {# Here we pass an invalid non-empty URL name as linkurl to generate pagination links with the URL path omitted #} + {% include "wagtailadmin/shared/pagination_nav.html" with items=submissions is_searching=False linkurl='-' %} + {# Here we pass an invalid non-empty URL name as linkurl to generate pagination links with the URL path omitted #} + </form> {% else %} <p class="no-results-message">{% blocktrans with title=form_page.title %}There have been no submissions of the '{{ title }}' form.{% endblocktrans %}</p> {% endif %} diff --git a/wagtail/wagtailforms/templates/wagtailforms/list_submissions.html b/wagtail/wagtailforms/templates/wagtailforms/list_submissions.html index 77f56bea2d..abf2d81b13 100644 --- a/wagtail/wagtailforms/templates/wagtailforms/list_submissions.html +++ b/wagtail/wagtailforms/templates/wagtailforms/list_submissions.html @@ -6,25 +6,29 @@ <col /> <thead> <tr> + <th colspan="{{ data_headings|length|add:1 }}"> + <button class="button no" id="delete-submissions" style="visibility: hidden">Delete selected submissions</button> + </th> + </tr> + <tr> + <th><input type="checkbox" id="select-all" /></th> {% for heading in data_headings %} <th>{{ heading }}</th> {% endfor %} - <th>{% trans "Actions" %}</th> </tr> </thead> <tbody> {% for row in data_rows %} <tr> + <td> + <input type="checkbox" name="selected-submissions" class="select-submission" value="{{ row.model_id }}" /> + </td> {% for cell in row.fields %} <td> {{ cell }} </td> {% endfor %} <td> - <a class="button button-small button-secondary" href=" - {% url 'wagtailforms:delete_submission' form_page.id row.model_id %}"> - {% trans 'delete' %}</a> - </td> </tr> {% endfor %} </tbody> diff --git a/wagtail/wagtailforms/tests/test_views.py b/wagtail/wagtailforms/tests/test_views.py index 3fc1d48765..fffa56e550 100644 --- a/wagtail/wagtailforms/tests/test_views.py +++ b/wagtail/wagtailforms/tests/test_views.py @@ -767,11 +767,11 @@ class TestDeleteFormSubmission(TestCase, WagtailTestUtils): self.assertTrue(self.client.login(username='siteeditor', password='password')) self.form_page = Page.objects.get(url_path='/home/contact-us/') - def test_delete_submission_show_cofirmation(self): + def test_delete_submission_show_confirmation(self): response = self.client.get(reverse( - 'wagtailforms:delete_submission', - args=(self.form_page.id, FormSubmission.objects.first().id) - )) + 'wagtailforms:delete_submissions', + args=(self.form_page.id, ) + ) + '?selected-submissions={}'.format(FormSubmission.objects.first().id)) # Check show confirm page when HTTP method is GET self.assertTemplateUsed(response, 'wagtailforms/confirm_delete.html') @@ -780,22 +780,33 @@ class TestDeleteFormSubmission(TestCase, WagtailTestUtils): def test_delete_submission_with_permissions(self): response = self.client.post(reverse( - 'wagtailforms:delete_submission', - args=(self.form_page.id, FormSubmission.objects.first().id) - )) + 'wagtailforms:delete_submissions', + args=(self.form_page.id, ) + ) + '?selected-submissions={}'.format(FormSubmission.objects.first().id)) # Check that the submission is gone self.assertEqual(FormSubmission.objects.count(), 1) # Should be redirected to list of submissions self.assertRedirects(response, reverse("wagtailforms:list_submissions", args=(self.form_page.id,))) + def test_delete_multiple_submissions_with_permissions(self): + response = self.client.post(reverse( + 'wagtailforms:delete_submissions', + args=(self.form_page.id, ) + ) + '?selected-submissions={}&selected-submissions={}'.format(FormSubmission.objects.first().id, FormSubmission.objects.last().id)) + + # Check that both submissions are gone + self.assertEqual(FormSubmission.objects.count(), 0) + # Should be redirected to list of submissions + self.assertRedirects(response, reverse("wagtailforms:list_submissions", args=(self.form_page.id,))) + def test_delete_submission_bad_permissions(self): self.assertTrue(self.client.login(username="eventeditor", password="password")) response = self.client.post(reverse( - 'wagtailforms:delete_submission', - args=(self.form_page.id, FormSubmission.objects.first().id) - )) + 'wagtailforms:delete_submissions', + args=(self.form_page.id, ) + ) + '?selected-submissions={}'.format(FormSubmission.objects.first().id)) # Check that the user received a 403 response self.assertEqual(response.status_code, 403) @@ -810,9 +821,9 @@ class TestDeleteFormSubmission(TestCase, WagtailTestUtils): with self.register_hook('filter_form_submissions_for_user', construct_forms_for_user): response = self.client.post(reverse( - 'wagtailforms:delete_submission', - args=(self.form_page.id, FormSubmission.objects.first().id) - )) + 'wagtailforms:delete_submissions', + args=(self.form_page.id, ) + ) + '?selected-submissions={}'.format(FormSubmission.objects.first().id)) # An user can't delete a from submission with the hook self.assertEqual(response.status_code, 403) @@ -820,9 +831,9 @@ class TestDeleteFormSubmission(TestCase, WagtailTestUtils): # An user can delete a form submission without the hook response = self.client.post(reverse( - 'wagtailforms:delete_submission', - args=(self.form_page.id, FormSubmission.objects.first().id) - )) + 'wagtailforms:delete_submissions', + args=(self.form_page.id, ) + ) + '?selected-submissions={}'.format(CustomFormPageSubmission.objects.first().id)) self.assertEqual(FormSubmission.objects.count(), 1) self.assertRedirects(response, reverse("wagtailforms:list_submissions", args=(self.form_page.id,))) @@ -834,11 +845,12 @@ class TestDeleteCustomFormSubmission(TestCase): self.assertTrue(self.client.login(username='siteeditor', password='password')) self.form_page = Page.objects.get(url_path='/home/contact-us-one-more-time/') - def test_delete_submission_show_cofirmation(self): + def test_delete_submission_show_confirmation(self): response = self.client.get(reverse( - 'wagtailforms:delete_submission', - args=(self.form_page.id, CustomFormPageSubmission.objects.first().id) - )) + 'wagtailforms:delete_submissions', + args=(self.form_page.id, ) + ) + '?selected-submissions={}'.format(CustomFormPageSubmission.objects.first().id)) + # Check show confirm page when HTTP method is GET self.assertTemplateUsed(response, 'wagtailforms/confirm_delete.html') @@ -847,22 +859,33 @@ class TestDeleteCustomFormSubmission(TestCase): def test_delete_submission_with_permissions(self): response = self.client.post(reverse( - 'wagtailforms:delete_submission', - args=(self.form_page.id, CustomFormPageSubmission.objects.first().id) - )) + 'wagtailforms:delete_submissions', + args=(self.form_page.id, ) + ) + '?selected-submissions={}'.format(CustomFormPageSubmission.objects.first().id)) # Check that the submission is gone self.assertEqual(CustomFormPageSubmission.objects.count(), 1) # Should be redirected to list of submissions self.assertRedirects(response, reverse("wagtailforms:list_submissions", args=(self.form_page.id, ))) + def test_delete_multiple_submissions_with_permissions(self): + response = self.client.post(reverse( + 'wagtailforms:delete_submissions', + args=(self.form_page.id, ) + ) + '?selected-submissions={}&selected-submissions={}'.format(CustomFormPageSubmission.objects.first().id, CustomFormPageSubmission.objects.last().id)) + + # Check that both submissions are gone + self.assertEqual(CustomFormPageSubmission.objects.count(), 0) + # Should be redirected to list of submissions + self.assertRedirects(response, reverse("wagtailforms:list_submissions", args=(self.form_page.id,))) + def test_delete_submission_bad_permissions(self): self.assertTrue(self.client.login(username="eventeditor", password="password")) response = self.client.post(reverse( - 'wagtailforms:delete_submission', - args=(self.form_page.id, CustomFormPageSubmission.objects.first().id) - )) + 'wagtailforms:delete_submissions', + args=(self.form_page.id, ) + ) + '?selected-submissions={}'.format(CustomFormPageSubmission.objects.first().id)) # Check that the user received a 403 response self.assertEqual(response.status_code, 403) diff --git a/wagtail/wagtailforms/urls.py b/wagtail/wagtailforms/urls.py index 133675c06b..793e870b1b 100644 --- a/wagtail/wagtailforms/urls.py +++ b/wagtail/wagtailforms/urls.py @@ -7,5 +7,5 @@ from wagtail.wagtailforms import views urlpatterns = [ url(r'^$', views.index, name='index'), url(r'^submissions/(\d+)/$', views.list_submissions, name='list_submissions'), - url(r'^submissions/(\d+)/(\d+)/delete/$', views.delete_submission, name='delete_submission') + url(r'^submissions/(\d+)/delete/$', views.delete_submissions, name='delete_submissions') ] diff --git a/wagtail/wagtailforms/views.py b/wagtail/wagtailforms/views.py index 45d94a3447..0ee5b06c97 100644 --- a/wagtail/wagtailforms/views.py +++ b/wagtail/wagtailforms/views.py @@ -7,7 +7,7 @@ from django.core.exceptions import PermissionDenied from django.http import HttpResponse from django.shortcuts import get_object_or_404, redirect, render from django.utils.encoding import smart_str -from django.utils.translation import ugettext as _ +from django.utils.translation import ungettext from wagtail.utils.pagination import paginate from wagtail.wagtailadmin import messages @@ -26,22 +26,36 @@ def index(request): }) -def delete_submission(request, page_id, submission_id): +def delete_submissions(request, page_id): if not get_forms_for_user(request.user).filter(id=page_id).exists(): raise PermissionDenied page = get_object_or_404(Page, id=page_id).specific - submission = get_object_or_404(page.get_submission_class(), id=submission_id) + + # Get submissions + submission_ids = request.GET.getlist('selected-submissions') + submissions = page.get_submission_class()._default_manager.filter(id__in=submission_ids) if request.method == 'POST': - submission.delete() + count = submissions.count() + submissions.delete() + + messages.success( + request, + ungettext( + "One submission has been deleted.", + "%(count)d submissions have been deleted.", + count + ) % { + 'count': count, + } + ) - messages.success(request, _("Submission deleted.")) return redirect('wagtailforms:list_submissions', page_id) return render(request, 'wagtailforms/confirm_delete.html', { 'page': page, - 'submission': submission + 'submissions': submissions, })