From 361def81c2f9c524b4c8057841e50924d2ae7b83 Mon Sep 17 00:00:00 2001 From: Tomasz Knapik Date: Fri, 6 Jul 2018 14:45:01 +0100 Subject: [PATCH] Wrap deleting page into database transaction Currently queries executed in the hooks don't run in the transaction with the page deletion query and it's harder to write hook without copying the whole view if you want to keep queries running in the hooks integral with page deletion. --- CHANGELOG.txt | 1 + docs/releases/2.3.rst | 1 + wagtail/admin/views/pages.py | 36 +++++++++++++++++++----------------- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 6bf7038533..cbed23e9d2 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -10,6 +10,7 @@ Changelog * EmbedBlock now validates against recognised embed providers on save (Bertrand Bordage) * Fix: Respect next param on login (Loic Teixeira) * Fix: InlinePanel now handles relations that specify a related_query_name (Aram Dulyan) + * Fix: before_delete_page / after_delete_page hooks now run within the same database transaction as the page deletion (Tomasz Knapik) 2.2.1 (13.08.2018) diff --git a/docs/releases/2.3.rst b/docs/releases/2.3.rst index 8634aa6ff9..1ffb57b8a4 100644 --- a/docs/releases/2.3.rst +++ b/docs/releases/2.3.rst @@ -32,6 +32,7 @@ Bug fixes * Respect next param on login (Loic Teixeira) * InlinePanel now handles relations that specify a related_query_name (Aram Dulyan) + * before_delete_page / after_delete_page hooks now run within the same database transaction as the page deletion (Tomasz Knapik) Upgrade considerations diff --git a/wagtail/admin/views/pages.py b/wagtail/admin/views/pages.py index e39562c86c..e8ac2d0782 100644 --- a/wagtail/admin/views/pages.py +++ b/wagtail/admin/views/pages.py @@ -2,6 +2,7 @@ from time import time from django.contrib.contenttypes.models import ContentType from django.core.exceptions import PermissionDenied +from django.db import transaction from django.db.models import Count from django.http import Http404, HttpResponse, JsonResponse from django.http.request import QueryDict @@ -541,27 +542,28 @@ def delete(request, page_id): if not page.permissions_for_user(request.user).can_delete(): raise PermissionDenied - for fn in hooks.get_hooks('before_delete_page'): - result = fn(request, page) - if hasattr(result, 'status_code'): - return result - - next_url = get_valid_next_url_from_request(request) - - if request.method == 'POST': - parent_id = page.get_parent().id - page.delete() - - messages.success(request, _("Page '{0}' deleted.").format(page.get_admin_display_title())) - - for fn in hooks.get_hooks('after_delete_page'): + with transaction.atomic(): + for fn in hooks.get_hooks('before_delete_page'): result = fn(request, page) if hasattr(result, 'status_code'): return result - if next_url: - return redirect(next_url) - return redirect('wagtailadmin_explore', parent_id) + next_url = get_valid_next_url_from_request(request) + + if request.method == 'POST': + parent_id = page.get_parent().id + page.delete() + + messages.success(request, _("Page '{0}' deleted.").format(page.get_admin_display_title())) + + for fn in hooks.get_hooks('after_delete_page'): + result = fn(request, page) + if hasattr(result, 'status_code'): + return result + + if next_url: + return redirect(next_url) + return redirect('wagtailadmin_explore', parent_id) return render(request, 'wagtailadmin/pages/confirm_delete.html', { 'page': page,