Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trashless Wagtail #8310

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .env_SAMPLE
Original file line number Diff line number Diff line change
Expand Up @@ -176,3 +176,8 @@ export HUD_API_ENDPOINT=http://localhost:8000/hud-api-replace/
# export OIDC_OP_USER_ENDPOINT=http://localhost:8080/openid/userinfo
# export OIDC_OP_JWKS_ENDPOINT=[Not used for test OIDC provider]
# export OIDC_OP_ADMIN_ROLE=[Not supported by test OIDC provider]

###############################
# Deletion archival storage
###############################
# export WAGTAIL_DELETION_ARCHIVE_PATH=/path/to/write/deleted/content/json/files
32 changes: 30 additions & 2 deletions cfgov/cfgov/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@
"mozilla_django_oidc",
"draftail_icons",
"wagtail_footnotes",
"wagtail_deletion_archival",
)

MIDDLEWARE = (
Expand Down Expand Up @@ -270,7 +271,23 @@
"django.contrib.staticfiles.finders.FileSystemFinder",
]

STATICFILES_STORAGE = "django.contrib.staticfiles.storage.StaticFilesStorage"
STORAGES = {
"default": {
"BACKEND": "django.core.files.storage.FileSystemStorage",
},
"staticfiles": {
"BACKEND": "django.contrib.staticfiles.storage.StaticFilesStorage",
},
}

if WAGTAIL_DELETION_ARCHIVE_PATH := os.getenv("WAGTAIL_DELETION_ARCHIVE_PATH"):
STORAGES["wagtail_deletion_archival"] = {
"BACKEND": "django.core.files.storage.FileSystemStorage",
"OPTIONS": {
"location": WAGTAIL_DELETION_ARCHIVE_PATH,
}
}


# Add the frontend build output to static files.
STATICFILES_DIRS = [
Expand Down Expand Up @@ -362,12 +379,15 @@
if os.environ.get("S3_ENABLED", "False") == "True":
AWS_ACCESS_KEY_ID = os.environ["AWS_ACCESS_KEY_ID"]
AWS_SECRET_ACCESS_KEY = os.environ["AWS_SECRET_ACCESS_KEY"]
DEFAULT_FILE_STORAGE = "storages.backends.s3boto3.S3Boto3Storage"
AWS_S3_CUSTOM_DOMAIN = os.environ.get("AWS_S3_CUSTOM_DOMAIN")
MEDIA_URL = os.path.join(
AWS_STORAGE_BUCKET_NAME + ".s3.amazonaws.com", AWS_LOCATION, ""
)

STORAGES["default"] = {
"BACKEND": "storages.backends.s3boto3.S3Boto3Storage",
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change


# GovDelivery
GOVDELIVERY_ACCOUNT_CODE = os.environ.get("GOVDELIVERY_ACCOUNT_CODE")
Expand Down Expand Up @@ -793,3 +813,11 @@
# Now we do some role/group-mapping for admins and regular users
# Upstream "role" for users who get is_superuser
OIDC_OP_ADMIN_ROLE = os.environ.get("OIDC_OP_ADMIN_ROLE")


# Deletion archive
# If this is set then when Wagtail pages are deleted a JSON archive file will
# be written to this path containing the deleted page's data.
WAGTAIL_DELETION_ARCHIVE_PATH = os.environ.get("WAGTAIL_DELETION_ARCHIVE_PATH")
if WAGTAIL_DELETION_ARCHIVE_PATH is not None:
WAGTAIL_DELETION_ARCHIVE_FILESYSTEM = f"osfs://{WAGTAIL_DELETION_ARCHIVE_PATH}"
Comment on lines +816 to +823
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Deletion archive
# If this is set then when Wagtail pages are deleted a JSON archive file will
# be written to this path containing the deleted page's data.
WAGTAIL_DELETION_ARCHIVE_PATH = os.environ.get("WAGTAIL_DELETION_ARCHIVE_PATH")
if WAGTAIL_DELETION_ARCHIVE_PATH is not None:
WAGTAIL_DELETION_ARCHIVE_FILESYSTEM = f"osfs://{WAGTAIL_DELETION_ARCHIVE_PATH}"

I missed deleting this in my PR.

6 changes: 3 additions & 3 deletions cfgov/cfgov/settings/production.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@
EMAIL_HOST = os.getenv("EMAIL_HOST")
DEFAULT_FROM_EMAIL = "[email protected]"

STATICFILES_STORAGE = (
"django.contrib.staticfiles.storage.ManifestStaticFilesStorage"
)
STORAGES["staticfiles"] = {
"BACKEND": "django.contrib.staticfiles.storage.ManifestStaticFilesStorage"
}

STATIC_ROOT = os.environ["DJANGO_STATIC_ROOT"]

Expand Down
5 changes: 5 additions & 0 deletions cfgov/cfgov/settings/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@

GOVDELIVERY_API = "core.govdelivery.MockGovDelivery"

# Disable Wagtail deletion archive storage during testing.
STORAGES.pop("wagtail_deletion_archival", None)

STATICFILES_FINDERS += [
"core.testutils.mock_staticfiles.MockStaticfilesFinder",
]
Expand Down Expand Up @@ -71,3 +74,5 @@
DEPLOY_ENVIRONMENT = "test"

INSTALLED_APPS += ("tccp.tests.testapp",)

WAGTAIL_DELETION_ARCHIVE_FILESYSTEM = "mem:///archive"
Comment on lines +77 to +78
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
WAGTAIL_DELETION_ARCHIVE_FILESYSTEM = "mem:///archive"

This can also be deleted.

20 changes: 20 additions & 0 deletions cfgov/v1/migrations/0038_emailsignup_disclaimer_page_set_null.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Generated by Django 4.2.14 on 2024-08-26 13:13

from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):

dependencies = [
('wagtailcore', '0093_uploadedfile'),
('v1', '0037_add_table_caption'),
]

operations = [
migrations.AlterField(
model_name='emailsignup',
name='disclaimer_page',
field=models.ForeignKey(blank=True, help_text='Choose the page that the "See Privacy Act statement" link should go to. If in doubt, use "Generic Email Sign-Up Privacy Act Statement".', null=True, on_delete=django.db.models.deletion.SET_NULL, to='wagtailcore.page', verbose_name='Privacy Act statement'),
),
]
2 changes: 1 addition & 1 deletion cfgov/v1/models/snippets.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ class EmailSignUp(RevisionMixin, models.Model):
verbose_name="Privacy Act statement",
null=True,
blank=True,
on_delete=models.PROTECT,
on_delete=models.SET_NULL,
help_text=(
'Choose the page that the "See Privacy Act statement" link '
'should go to. If in doubt, use "Generic Email Sign-Up '
Expand Down
126 changes: 78 additions & 48 deletions cfgov/v1/tests/test_wagtail_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,54 +26,6 @@ def test_allowlist_hooks(self):
self.assertHTMLEqual(output_html, "Consumer Finance")


class TestDeleteProtections(TestCase, WagtailTestUtils):
def setUp(self):
self.login()

root_page = Site.objects.get(is_default_site=True).root_page
self.page1 = BlogPage(title="delete1", slug="delete1")
root_page.add_child(instance=self.page1)
self.page2 = BlogPage(title="delete2", slug="delete2")
root_page.add_child(instance=self.page2)

self.delete_url = reverse(
"wagtailadmin_pages:delete", args=(self.page1.id,)
)
self.bulk_delete_url = (
reverse(
"wagtail_bulk_action",
args=(
"wagtailcore",
"page",
"delete",
),
)
+ f"?id={self.page1.id}&id={self.page2.id}"
)

def test_delete_page_block(self):
response = self.client.post(self.delete_url)
self.assertRedirects(response, reverse("wagtailadmin_home"))

def test_delete_page_block_ajax(self):
response = self.client.post(
self.delete_url,
HTTP_X_REQUESTED_WITH="XMLHttpRequest",
)
self.assertEqual(response.status_code, 403)

def test_bulk_delete_block(self):
response = self.client.post(self.bulk_delete_url)
self.assertRedirects(response, reverse("wagtailadmin_home"))

def test_bulk_delete_page_block_ajax(self):
response = self.client.post(
self.bulk_delete_url,
HTTP_X_REQUESTED_WITH="XMLHttpRequest",
)
self.assertEqual(response.status_code, 403)


class TestDjangoAdminLink(TestCase, WagtailTestUtils):
def get_admin_response_for_user(self, is_staff):
credentials = {"username": "regular", "password": "regular"}
Expand Down Expand Up @@ -113,3 +65,81 @@ def test_docs_not_defined_no_link_in_admin(self):
def test_guide_defined_creates_link_in_admin(self):
InternalDocsSettings.objects.create(url="https://example.com/")
self.assertContains(self.get_admin_response(), "/admin/internal-docs/")


class LockedPageDeletionTestCase(TestCase, WagtailTestUtils):
def setUp(self):
self.login()

root_page = Site.objects.get(is_default_site=True).root_page

self.locked_page = BlogPage(title="locked page", slug="locked-page")
self.locked_page.locked = True
root_page.add_child(instance=self.locked_page)

self.unlocked_page = BlogPage(
title="unlocked page", slug="unlocked-page"
)
root_page.add_child(instance=self.unlocked_page)

def test_delete_locked_page(self):
response = self.client.post(
reverse("wagtailadmin_pages:delete", args=(self.locked_page.pk,))
)
self.assertTrue(any("message" in ctx for ctx in response.context))
self.assertEqual(
"locked page is locked and cannot be deleted.",
next(
ctx["message"] for ctx in response.context if "message" in ctx
),
)
self.assertRedirects(
response,
reverse("wagtailadmin_explore", args=(self.locked_page.pk,)),
)
self.assertEqual(
self.locked_page, BlogPage.objects.get(pk=self.locked_page.pk)
)

def test_bulk_delete_locked_page(self):
response = self.client.post(
reverse(
"wagtail_bulk_action",
args=(
"wagtailcore",
"page",
"delete",
),
)
+ f"?id={self.locked_page.pk}&id={self.unlocked_page.pk}"
)
self.assertTrue(any("message" in ctx for ctx in response.context))
self.assertIn(
"locked page is locked and cannot be deleted.",
next(
ctx["message"] for ctx in response.context if "message" in ctx
),
)
self.assertRedirects(response, reverse("wagtailadmin_home"))

def test_bulk_delete_locked_page_with_referrer(self):
response = self.client.post(
reverse(
"wagtail_bulk_action",
args=(
"wagtailcore",
"page",
"delete",
),
)
+ f"?id={self.locked_page.pk}&id={self.unlocked_page.pk}",
HTTP_REFERER="/",
)
self.assertTrue(any("message" in ctx for ctx in response.context))
self.assertIn(
"locked page is locked and cannot be deleted.",
next(
ctx["message"] for ctx in response.context if "message" in ctx
),
)
self.assertRedirects(response, "/")
55 changes: 28 additions & 27 deletions cfgov/v1/wagtail_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import re

from django.conf import settings
from django.core.exceptions import PermissionDenied
from django.shortcuts import redirect
from django.urls import path, re_path, reverse
from django.utils.html import format_html_join
Expand Down Expand Up @@ -46,32 +45,6 @@
logger = logging.getLogger(__name__)


# Inspired by https://github.com/wagtail/wagtail/blob/5fa1bd15f3b711f612628576148e904568ed8bca/wagtail/admin/auth.py#L18-L26.
def _prevent_page_deletion(request):
if request.headers.get("x-requested-with") == "XMLHttpRequest":
raise PermissionDenied

messages.error(
request,
"Page deletion is currently disabled; "
"please move your page to the Trash instead.",
)
return redirect("wagtailadmin_home")


@hooks.register("before_delete_page")
def raise_delete_error(request, page):
return _prevent_page_deletion(request)


@hooks.register("before_bulk_action")
def raise_bulk_delete_error(
request, action_type, objects, action_class_instance
):
if action_type == "delete":
return _prevent_page_deletion(request)


@hooks.register("after_delete_page")
def log_page_deletion(request, page):
logger.warning(
Expand Down Expand Up @@ -491,3 +464,31 @@ def register_internal_docs_menu_item():
attrs={"target": "_blank", "rel": "noreferrer"},
name="internal_docs_menu",
)


@hooks.register("before_delete_page")
def prevent_locked_page_deletion(request, page):
"""Prevent deletion of locked pages"""
if page.locked:
messages.warning(
request, f"{page.title} is locked and cannot be deleted."
)
return redirect("wagtailadmin_explore", page.pk)


@hooks.register("before_bulk_action")
def prevent_locked_page_bulk_deletion(
request, action_type, objects, action_class_instance
):
"""Prevent deletion of locked pages when part of a bulk action"""
if action_type == "delete":
for obj in objects:
if hasattr(obj, "locked") and obj.locked:
messages.warning(
request,
f"{obj} is locked and cannot be deleted. "
"Please remove it from the selection.",
)
if request.META.get("HTTP_REFERER"):
return redirect(request.META.get("HTTP_REFERER"))
return redirect("wagtailadmin_home")
Empty file.
6 changes: 6 additions & 0 deletions cfgov/wagtail_deletion_archival/apps.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
from django.apps import AppConfig


class WagtailDeletionArchivalConfig(AppConfig):
name = "wagtail_deletion_archival"
label = "wagtail_deletion_archival"
5 changes: 5 additions & 0 deletions cfgov/wagtail_deletion_archival/forms.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
from django import forms


class ImportForm(forms.Form):
page_file = forms.FileField(label="Page file", required=True)
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{% extends "wagtailadmin/base.html" %}
{% load i18n wagtailadmin_tags %}
{% block titletag %}Import page into {{ parent_page.get_admin_display_title }}{% endblock %}

{% block content %}
{% include "wagtailadmin/shared/header.html" with title="Import page" subtitle=parent_page.get_admin_display_title icon="doc-empty-inverse" %}

<div class="nice-padding">
{% include "wagtailadmin/shared/non_field_errors.html" %}

<form enctype="multipart/form-data" action="{% url 'wagtail_deletion_archive_import' parent_page.id %}" method="POST">
{% csrf_token %}
<ul class="fields">
{% for field in form %}
<li>
{% if field.is_hidden %}
{{ field }}
{% else %}
{% include "wagtailadmin/shared/field.html" with field=field %}
{% endif %}
</li>
{% endfor %}
<li>
<button type="submit" class="button button-longrunning" data-clicked-text="{% trans 'Uploading…' %}">{% icon name="spinner" %}<em>{% trans 'Upload' %}</em></button>
</li>
</ul>
</form>
</div>
{% endblock %}
Empty file.
Loading
Loading