From 1b8aa6c6154c0751b946dcf9fda3ec7eeb0cacc2 Mon Sep 17 00:00:00 2001 From: Andy Chosak Date: Wed, 2 Oct 2024 11:15:26 -0400 Subject: [PATCH 1/3] Use Django storage for Wagtail deletion archive These code changes propose using Django storage to store and serve the Wagtail deletion archive from PR 8310 [0], instead of using Apache to serve those files. This simplifies the configuration and makes it easier to test locally and easier to port in future to other webserver approaches. With these changes, the deletion archive is now only available behind the Wagtail admin login, at the URL /admin/__deleted/. This URL provides a nicely formatted Wagtail report view where archives can be downloaded (instead of the current PR approach that uses Apache directory serving). With this change, Django storage configuration has been migrated to settings.STORAGES from settings.STATICFILES_STORAGE, which was deprecated in Django 4.2 and removed in Django 5.1. Additionally, with this change, cf.gov deployments that have not defined the WAGTAIL_DELETION_ARCHIVE_PATH environment variable still support importing archives; the archive-on-delete and archive download functionality is still disabled in those cases. This commit also includes a minor bugfix to the archive import template which doesn't currently properly display the destination page title. [0] https://github.com/cfpb/consumerfinance.gov/pull/8310 --- .../conf.d/wagtail_deletion.conf.sample | 7 - cfgov/cfgov/settings/base.py | 26 +++- cfgov/wagtail_deletion_archival/apps.py | 9 -- .../import_page.html | 4 +- cfgov/wagtail_deletion_archival/utils.py | 56 ++++++- cfgov/wagtail_deletion_archival/views.py | 83 ++++++++-- .../wagtail_hooks.py | 143 ++++++++---------- requirements/libraries.txt | 2 - 8 files changed, 206 insertions(+), 124 deletions(-) delete mode 100644 cfgov/apache/conf.d/wagtail_deletion.conf.sample diff --git a/cfgov/apache/conf.d/wagtail_deletion.conf.sample b/cfgov/apache/conf.d/wagtail_deletion.conf.sample deleted file mode 100644 index 65d9ab5570e..00000000000 --- a/cfgov/apache/conf.d/wagtail_deletion.conf.sample +++ /dev/null @@ -1,7 +0,0 @@ -# Serve deleted content JSON archives at /__deleted -PassEnv WAGTAIL_DELETION_ARCHIVE_PATH -LoadModule autoindex_module modules/mod_autoindex.so -Alias /__deleted/ "${WAGTAIL_DELETION_ARCHIVE_PATH}" - - Options +Indexes - diff --git a/cfgov/cfgov/settings/base.py b/cfgov/cfgov/settings/base.py index 7843bc41b0d..1734443deed 100644 --- a/cfgov/cfgov/settings/base.py +++ b/cfgov/cfgov/settings/base.py @@ -271,7 +271,26 @@ "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"): + # Archive will be available under /admin/ + whatever this next line is: + WAGTAIL_DELETION_ARCHIVE_URL = "__deleted/" + + 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 = [ @@ -363,12 +382,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", + } + # GovDelivery GOVDELIVERY_ACCOUNT_CODE = os.environ.get("GOVDELIVERY_ACCOUNT_CODE") diff --git a/cfgov/wagtail_deletion_archival/apps.py b/cfgov/wagtail_deletion_archival/apps.py index b7c5695efc0..75824eaa1d0 100644 --- a/cfgov/wagtail_deletion_archival/apps.py +++ b/cfgov/wagtail_deletion_archival/apps.py @@ -1,15 +1,6 @@ from django.apps import AppConfig -from django.conf import settings - -import fs class WagtailDeletionArchivalConfig(AppConfig): name = "wagtail_deletion_archival" label = "wagtail_deletion_archival" - filesystem_name = getattr( - settings, "WAGTAIL_DELETION_ARCHIVE_FILESYSTEM", None - ) - filesystem = ( - fs.open_fs(filesystem_name) if filesystem_name is not None else None - ) diff --git a/cfgov/wagtail_deletion_archival/templates/wagtail_deletion_archival/import_page.html b/cfgov/wagtail_deletion_archival/templates/wagtail_deletion_archival/import_page.html index 20eb0e903a2..f89b7eac5cb 100644 --- a/cfgov/wagtail_deletion_archival/templates/wagtail_deletion_archival/import_page.html +++ b/cfgov/wagtail_deletion_archival/templates/wagtail_deletion_archival/import_page.html @@ -1,9 +1,9 @@ {% extends "wagtailadmin/base.html" %} {% load i18n wagtailadmin_tags %} -{% block titletag %}Import page into {{ title }}{% endblock %} +{% block titletag %}Import page into {{ parent_page.get_admin_display_title }}{% endblock %} {% block content %} - {% include "wagtailadmin/shared/header.html" with title="Import page" subtitle=page.get_admin_display_title icon="doc-empty-inverse" %} + {% include "wagtailadmin/shared/header.html" with title="Import page" subtitle=parent_page.get_admin_display_title icon="doc-empty-inverse" %}
{% include "wagtailadmin/shared/non_field_errors.html" %} diff --git a/cfgov/wagtail_deletion_archival/utils.py b/cfgov/wagtail_deletion_archival/utils.py index f1601a44080..334b4eccda0 100644 --- a/cfgov/wagtail_deletion_archival/utils.py +++ b/cfgov/wagtail_deletion_archival/utils.py @@ -1,10 +1,16 @@ import json import logging +import os.path +import re +from urllib.parse import unquote from django.apps import apps +from django.core.files.base import ContentFile +from django.core.files.storage import InvalidStorageError, storages from django.core.serializers.json import DjangoJSONEncoder from django.db.migrations.recorder import MigrationRecorder from django.utils import timezone +from django.utils.encoding import smart_str from wagtail.coreutils import find_available_slug from wagtail.models import get_streamfield_names @@ -13,6 +19,13 @@ logger = logging.getLogger(__name__) +def get_archive_storage(): + try: + return storages["wagtail_deletion_archival"] + except InvalidStorageError: + return None + + def get_last_migration(app): Migration = MigrationRecorder.Migration app_migrations = Migration.objects.filter(app=app).order_by("-applied") @@ -24,7 +37,7 @@ def get_last_migration(app): return last_migration.name -def export_page(page): +def convert_page_to_json(page): """Get a copy of the page as JSON.""" # This includes its app_label, model, and the latest migration applied for @@ -58,10 +71,49 @@ def export_page(page): page_export, ensure_ascii=False, indent=4, cls=DjangoJSONEncoder ) - logger.info(f"Exported {page.slug} to JSON") return page_json +def make_archive_filename(page): + now = timezone.now() + return f"{page.slug}-{now.isoformat()}.json" + + +ARCHIVE_FILENAME_RE = re.compile( + # Starting with any character then - + r"^.+-" + # YYYY-MM-DDTHH:MM:SS + r"[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}" + # Optional .ffffff + r"(\.[0-9]{1,6})?" + # Optional timezone + r"([+-][0-9]{2}:[0-9]{2})?" + # .json + r"\.json$" +) + + +def export_page_signal_handler(sender, instance, **kwargs): + archive_storage = get_archive_storage() + + if not archive_storage: + return + + page = instance.specific + site = page.get_site() + page_path = unquote(page.relative_url(site)).lstrip("/") + + page_json = convert_page_to_json(page) + + page_filename = make_archive_filename(page) + target_path = smart_str(os.path.join(page_path, page_filename)) + + archived_filename = archive_storage.save( + target_path, ContentFile(page_json.encode("utf-8")) + ) + logger.info(f"Exported {page.slug} to JSON at {archived_filename}") + + def import_page(parent_page, page_json, slug=None): page_data = json.loads(page_json) diff --git a/cfgov/wagtail_deletion_archival/views.py b/cfgov/wagtail_deletion_archival/views.py index 1a9e91e232e..6b268e2539f 100644 --- a/cfgov/wagtail_deletion_archival/views.py +++ b/cfgov/wagtail_deletion_archival/views.py @@ -1,25 +1,21 @@ -from io import BytesIO +import os.path +from functools import cached_property -from django.http import FileResponse -from django.shortcuts import get_object_or_404, redirect +from django.http import FileResponse, Http404 +from django.shortcuts import get_object_or_404, redirect, reverse from django.template.response import TemplateResponse +from django.utils.html import format_html +from django.views.generic import View +from wagtail.admin.views.reports import ReportView from wagtail.models import Page from wagtail_deletion_archival.forms import ImportForm -from wagtail_deletion_archival.utils import export_page, import_page - - -def export_view(request, page_id): - page = get_object_or_404(Page, id=page_id).specific - page_json = export_page(page) - page_io = BytesIO(page_json.encode()) - response = FileResponse( - page_io, - as_attachment=True, - filename=f"{page.slug}.json", - ) - return response +from wagtail_deletion_archival.utils import ( + ARCHIVE_FILENAME_RE, + get_archive_storage, + import_page, +) def import_view(request, page_id): @@ -54,3 +50,58 @@ def import_view(request, page_id): "form": input_form, }, ) + + +class ArchiveStorageMixin: + @cached_property + def storage(self): + return get_archive_storage() + + +class ArchiveLink: + def __init__(self, path): + self.path = path + + def __str__(self): + return format_html( + '{}', + reverse( + "wagtail_deletion_archive_serve", + kwargs={"path": self.path}, + ), + self.path, + ) + + +class ArchiveIndexView(ArchiveStorageMixin, ReportView): + page_title = "Deleted Wagtail pages" + + def get_queryset(self): + return list(map(ArchiveLink, self.get_archive_files_recursive())) + + def get_archive_files_recursive(self, path=""): + archive_files = [] + + dirs, filenames = self.storage.listdir(path) + + for filename in filenames: + filename_path = os.path.join(path, filename) + if ARCHIVE_FILENAME_RE.match(filename_path): + archive_files.append(filename_path) + + for dir in dirs: + archive_files.extend( + self.get_archive_files_recursive(os.path.join(path, dir)) + ) + + return archive_files + + +class ArchiveFileView(ArchiveStorageMixin, View): + def get(self, request, path): + if not ARCHIVE_FILENAME_RE.match(path) or not self.storage.exists( + path + ): + raise Http404 + + return FileResponse(self.storage.open(path), as_attachment=True) diff --git a/cfgov/wagtail_deletion_archival/wagtail_hooks.py b/cfgov/wagtail_deletion_archival/wagtail_hooks.py index 8fdb13f6206..3475cce6cd7 100644 --- a/cfgov/wagtail_deletion_archival/wagtail_hooks.py +++ b/cfgov/wagtail_deletion_archival/wagtail_hooks.py @@ -1,105 +1,80 @@ -from urllib.parse import unquote - -from django.apps import apps from django.conf import settings from django.db.models.signals import pre_delete -from django.urls import path as django_path -from django.urls import reverse -from django.utils import timezone -from django.utils.encoding import smart_str +from django.urls import path, reverse from wagtail import hooks from wagtail.admin.widgets import Button -from fs import path as fs_path - -from wagtail_deletion_archival.utils import export_page -from wagtail_deletion_archival.views import export_view, import_view - - -def archive_page_data_receiver(sender, instance, **kwargs): - # Skip all this if settings.WAGTAIL_DELETION_ARCHIVE_FILESYSTEM is not set - if getattr(settings, "WAGTAIL_DELETION_ARCHIVE_FILESYSTEM", None) is None: - return - - fs = apps.get_app_config("wagtail_deletion_archival").filesystem - - page = instance.specific - site = page.get_site() - page_path = unquote(page.relative_url(site)) - - page_json = export_page(page) - - now = timezone.now() - page_filename = f"{page.slug}-{now.isoformat()}.json" - target_path = smart_str(fs_path.join(page_path, page_filename)) - - fs.exists(page_path) or fs.makedirs(page_path) - fs.writetext(target_path, page_json, encoding="utf8") +from wagtail_deletion_archival.utils import ( + export_page_signal_handler, + get_archive_storage, +) +from wagtail_deletion_archival.views import ( + ArchiveFileView, + ArchiveIndexView, + import_view, +) -@hooks.register("before_delete_page") -def add_archival_signal_reciever(request, page): - from wagtail.models import Page - - pre_delete.connect(archive_page_data_receiver, sender=Page) - - -@hooks.register("after_delete_page") -def remove_archival_signal_reciever(request, page): - from wagtail.models import Page +@hooks.register("register_admin_urls") +def register_archive_storage_import_url(): + return [ + path("import//", import_view, name="import_page"), + ] - pre_delete.disconnect(archive_page_data_receiver, sender=Page) +@hooks.register("register_page_header_buttons") +def page_import_button(page, user, view_name, next_url=None): + yield Button( + "Import", + reverse("import_page", args=(page.id,)), + priority=200, + icon_name="upload", + ) -@hooks.register("before_bulk_action") -def add_bulk_delete_signal_reciever( - request, action_type, objects, action_class_instance -): - from wagtail.models import Page - if action_type == "delete": - pre_delete.connect(archive_page_data_receiver, sender=Page) +if get_archive_storage(): + @hooks.register("register_admin_urls") + def register_deleted_page_archive_list_url(): + return [ + path( + settings.WAGTAIL_DELETION_ARCHIVE_URL, + ArchiveIndexView.as_view(), + ), + path( + f"{settings.WAGTAIL_DELETION_ARCHIVE_URL}", + ArchiveFileView.as_view(), + name="wagtail_deletion_archive_serve", + ), + ] -@hooks.register("after_bulk_action") -def remove_bulk_delete_signal_reciever( - request, action_type, objects, action_class_instance -): - from wagtail.models import Page + @hooks.register("before_delete_page") + def add_archival_signal_reciever(request, page): + from wagtail.models import Page - if action_type == "delete": - pre_delete.disconnect(archive_page_data_receiver, sender=Page) + pre_delete.connect(export_page_signal_handler, sender=Page) + @hooks.register("after_delete_page") + def remove_archival_signal_reciever(request, page): + from wagtail.models import Page -@hooks.register("register_page_listing_more_buttons") -def page_export_button( - page, - page_perms, - is_parent=False, - next_url=None, -): - yield Button( - "Export", - reverse("export_page", args=(page.id,)), - priority=210, - icon_name="download", - ) + pre_delete.disconnect(export_page_signal_handler, sender=Page) + @hooks.register("before_bulk_action") + def add_bulk_delete_signal_reciever( + request, action_type, objects, action_class_instance + ): + from wagtail.models import Page -@hooks.register("register_page_header_buttons") -def page_import_button(page, user, view_name, next_url=None): - yield Button( - "Import", - reverse("import_page", args=(page.id,)), - priority=200, - icon_name="upload", - ) + if action_type == "delete": + pre_delete.connect(export_page_signal_handler, sender=Page) + @hooks.register("after_bulk_action") + def remove_bulk_delete_signal_reciever( + request, action_type, objects, action_class_instance + ): + from wagtail.models import Page -@hooks.register("register_admin_urls") -def register_portable_page_admin_urls(): - return [ - django_path("export//", export_view, name="export_page"), - django_path("import//", import_view, name="import_page"), - ] + if action_type == "delete": + pre_delete.disconnect(export_page_signal_handler, sender=Page) diff --git a/requirements/libraries.txt b/requirements/libraries.txt index 73c8389bf9d..42b5b858133 100644 --- a/requirements/libraries.txt +++ b/requirements/libraries.txt @@ -18,8 +18,6 @@ django-storages==1.14.4 django-treebeard==4.7.1 edgegrid-python==1.3.1 elasticsearch<7.11 # Keep pinned to the deployed ES version -fs==2.4.16 -fs-s3fs==1.1.1 govdelivery==1.4.0 Jinja2==3.1.4 lxml==5.2.2 From 957a3fb9c6c24895f452d11c33398cd7a79c33bd Mon Sep 17 00:00:00 2001 From: Andy Chosak Date: Thu, 3 Oct 2024 10:21:16 -0400 Subject: [PATCH 2/3] Add tests, and a few minor fixups --- .env_SAMPLE | 1 - cfgov/cfgov/settings/base.py | 3 - cfgov/cfgov/settings/test.py | 3 + .../import_page.html | 2 +- .../tests/temp_storage.py | 29 ++++ .../tests/test_utils.py | 150 ++++++++++++++++-- .../tests/test_views.py | 132 ++++++++------- .../tests/test_wagtail_hooks.py | 93 ++--------- cfgov/wagtail_deletion_archival/utils.py | 8 +- cfgov/wagtail_deletion_archival/views.py | 16 +- .../wagtail_hooks.py | 90 +++++------ 11 files changed, 308 insertions(+), 219 deletions(-) create mode 100644 cfgov/wagtail_deletion_archival/tests/temp_storage.py diff --git a/.env_SAMPLE b/.env_SAMPLE index 8854a5aa069..097033f01b5 100755 --- a/.env_SAMPLE +++ b/.env_SAMPLE @@ -177,7 +177,6 @@ export HUD_API_ENDPOINT=http://localhost:8000/hud-api-replace/ # 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 ############################### diff --git a/cfgov/cfgov/settings/base.py b/cfgov/cfgov/settings/base.py index 1734443deed..2ca1290c2e2 100644 --- a/cfgov/cfgov/settings/base.py +++ b/cfgov/cfgov/settings/base.py @@ -281,9 +281,6 @@ } if WAGTAIL_DELETION_ARCHIVE_PATH := os.getenv("WAGTAIL_DELETION_ARCHIVE_PATH"): - # Archive will be available under /admin/ + whatever this next line is: - WAGTAIL_DELETION_ARCHIVE_URL = "__deleted/" - STORAGES["wagtail_deletion_archival"] = { "BACKEND": "django.core.files.storage.FileSystemStorage", "OPTIONS": { diff --git a/cfgov/cfgov/settings/test.py b/cfgov/cfgov/settings/test.py index d90370e1344..7f6bd3a706f 100644 --- a/cfgov/cfgov/settings/test.py +++ b/cfgov/cfgov/settings/test.py @@ -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", ] diff --git a/cfgov/wagtail_deletion_archival/templates/wagtail_deletion_archival/import_page.html b/cfgov/wagtail_deletion_archival/templates/wagtail_deletion_archival/import_page.html index f89b7eac5cb..32bd203fb90 100644 --- a/cfgov/wagtail_deletion_archival/templates/wagtail_deletion_archival/import_page.html +++ b/cfgov/wagtail_deletion_archival/templates/wagtail_deletion_archival/import_page.html @@ -8,7 +8,7 @@
{% include "wagtailadmin/shared/non_field_errors.html" %} -
+ {% csrf_token %}
    {% for field in form %} diff --git a/cfgov/wagtail_deletion_archival/tests/temp_storage.py b/cfgov/wagtail_deletion_archival/tests/temp_storage.py new file mode 100644 index 00000000000..149a8424f10 --- /dev/null +++ b/cfgov/wagtail_deletion_archival/tests/temp_storage.py @@ -0,0 +1,29 @@ +from tempfile import TemporaryDirectory + +from django.test import override_settings +from django.test.utils import TestContextDecorator + + +class uses_temp_archive_storage(TestContextDecorator): + def __init__(self): + super().__init__(attr_name="temp_storage", kwarg_name="temp_storage") + + def enable(self): + self.temp_storage = TemporaryDirectory() + + self.settings = override_settings( + STORAGES={ + "wagtail_deletion_archival": { + "BACKEND": "django.core.files.storage.FileSystemStorage", + "OPTIONS": { + "location": self.temp_storage.name, + }, + } + } + ) + self.settings.enable() + return self.temp_storage.name + + def disable(self): + self.settings.disable() + self.temp_storage.cleanup() diff --git a/cfgov/wagtail_deletion_archival/tests/test_utils.py b/cfgov/wagtail_deletion_archival/tests/test_utils.py index b54519689b8..da581fab9ff 100644 --- a/cfgov/wagtail_deletion_archival/tests/test_utils.py +++ b/cfgov/wagtail_deletion_archival/tests/test_utils.py @@ -1,18 +1,46 @@ import json -import logging +from glob import glob +from unittest import skipIf -from django.test import TestCase +from django.conf import settings +from django.test import SimpleTestCase, TestCase, override_settings +from django.urls import reverse from wagtail.models import Site +from wagtail.test.utils import WagtailTestUtils + +from freezegun import freeze_time from v1.models import BlogPage +from wagtail_deletion_archival.tests.temp_storage import ( + uses_temp_archive_storage, +) from wagtail_deletion_archival.utils import ( - export_page, + convert_page_to_json, + get_archive_storage, get_last_migration, import_page, + make_archive_filename, ) +class GetArchivalStorageTests(SimpleTestCase): + @override_settings(STORAGES={}) + def test_no_archival_storage(self): + self.assertIsNone(get_archive_storage()) + + @override_settings( + STORAGES={ + "wagtail_deletion_archival": { + "BACKEND": "django.core.files.storage.FileSystemStorage" + } + } + ) + def test_archive_storage(self): + self.assertIsNotNone(get_archive_storage()) + + +@skipIf(settings.SKIP_DJANGO_MIGRATIONS, "Requires Django migrations") class LastMigrationTestCase(TestCase): def test_get_last_migration_has_migrations(self): app_label = BlogPage._meta.app_label @@ -24,7 +52,8 @@ def test_get_last_migration_no_migrations(self): self.assertEqual(last_migration, "") -class ExportPageTestCase(TestCase): +@skipIf(settings.SKIP_DJANGO_MIGRATIONS, "Requires Django migrations") +class ConvertPageToJsonTestCase(TestCase): def setUp(self): self.page = BlogPage( title="Test page", @@ -46,7 +75,7 @@ def setUp(self): ) def test_export_page(self): - page_json = export_page(self.page) + page_json = convert_page_to_json(self.page) page_data = json.loads(page_json) self.assertEqual(page_data["app_label"], "v1") @@ -60,6 +89,7 @@ def test_export_page(self): ) +@skipIf(settings.SKIP_DJANGO_MIGRATIONS, "Requires Django migrations") class ImportPageTestCase(TestCase): def setUp(self): self.root_page = Site.objects.get(is_default_site=True).root_page @@ -69,11 +99,7 @@ def setUp(self): content=[("text", "Hello, world!")], live=True, ) - self.page_json = export_page(self.original_page) - logging.disable(logging.NOTSET) - - def tearDown(self): - logging.disable(logging.CRITICAL) + self.page_json = convert_page_to_json(self.original_page) def test_import_page_model_does_not_exist(self): page_json = json.dumps( @@ -105,3 +131,107 @@ def test_import_page(self): self.assertEqual(page.title, self.original_page.title) self.assertEqual(page.slug, self.original_page.slug) self.assertEqual(page.content, self.original_page.content) + + +class MakeArchiveFilenameTests(SimpleTestCase): + @freeze_time("2020-01-01") + def test_make_archive_filename(self): + self.assertEqual( + make_archive_filename("foo"), "foo-2020-01-01T00:00:00+00:00.json" + ) + + +@skipIf(settings.SKIP_DJANGO_MIGRATIONS, "Requires Django migrations") +class ArchivePageOnDeletionTestCase(TestCase, WagtailTestUtils): + def setUp(self): + self.login() + + root_page = Site.objects.get(is_default_site=True).root_page + + self.test_page1 = BlogPage(title="test page 1", slug="test-page1") + root_page.add_child(instance=self.test_page1) + + self.test_child_page = BlogPage(title="child page", slug="test-child") + self.test_page1.add_child(instance=self.test_child_page) + + self.test_page2 = BlogPage(title="test page 2", slug="test-page2") + root_page.add_child(instance=self.test_page2) + + self.test_page3 = BlogPage(title="test page 3", slug="test-page3") + root_page.add_child(instance=self.test_page3) + + @uses_temp_archive_storage() + def test_delete_page(self, temp_storage): + # Before the test no JSON files exist in the archive dir. + self.assertFalse(glob(f"{temp_storage}/**/*.json", recursive=True)) + + self.client.post( + reverse("wagtailadmin_pages:delete", args=(self.test_page1.pk,)) + ) + + # The page slug exists in the archive dir. + self.assertTrue( + glob( + f"{temp_storage}/{self.test_page1.slug}/*.json", + recursive=True, + ) + ) + + # The child page slug exists in the archive dir. + self.assertTrue( + glob( + f"{temp_storage}/{self.test_page1.slug}/{self.test_child_page.slug}/*.json", + recursive=True, + ) + ) + + @uses_temp_archive_storage() + def test_bulk_delete_page(self, temp_storage): + self.client.post( + reverse( + "wagtail_bulk_action", + args=( + "wagtailcore", + "page", + "delete", + ), + ) + + f"?id={self.test_page1.pk}&id={self.test_page2.pk}" + ) + + # Page 1, its child page, and Page 2 should all be archived. + self.assertEqual( + len( + glob( + f"{temp_storage}/**/*.json", + recursive=True, + ) + ), + 3, + ) + + @uses_temp_archive_storage() + def test_delete_page_confirmation(self, temp_storage): + self.client.get( + reverse("wagtailadmin_pages:delete", args=(self.test_page3.pk,)) + ) + self.assertFalse(glob(f"{temp_storage}/**/*.json", recursive=True)) + + +class ArchivePageOnDeletionWhenArchivingIsDisabledTestCase( + TestCase, WagtailTestUtils +): + def test_delete_works_when_archiving_is_disabled(self): + self.assertIsNone(get_archive_storage()) + + root_page = Site.objects.get(is_default_site=True).root_page + self.test_page1 = BlogPage(title="test page 1", slug="test-page1") + root_page.add_child(instance=self.test_page1) + + self.login() + self.client.post( + reverse("wagtailadmin_pages:delete", args=(self.test_page1.pk,)) + ) + + with self.assertRaises(BlogPage.DoesNotExist): + BlogPage.objects.get(slug="test-page1") diff --git a/cfgov/wagtail_deletion_archival/tests/test_views.py b/cfgov/wagtail_deletion_archival/tests/test_views.py index 207483a7d68..3b64e21087e 100644 --- a/cfgov/wagtail_deletion_archival/tests/test_views.py +++ b/cfgov/wagtail_deletion_archival/tests/test_views.py @@ -1,6 +1,10 @@ import json +import os +import os.path from io import StringIO +from unittest import skipIf +from django.conf import settings from django.test import TestCase from django.urls import reverse @@ -8,59 +12,53 @@ from wagtail.test.utils import WagtailTestUtils from v1.models import BlogPage -from wagtail_deletion_archival.utils import export_page +from wagtail_deletion_archival.tests.temp_storage import ( + uses_temp_archive_storage, +) +from wagtail_deletion_archival.utils import ( + convert_page_to_json, + get_archive_storage, + make_archive_filename, +) -class ExportViewTestCase(TestCase, WagtailTestUtils): +@skipIf(settings.SKIP_DJANGO_MIGRATIONS, "Requires Django migrations") +class ImportViewTestCase(TestCase, WagtailTestUtils): def setUp(self): self.root_page = Site.objects.get(is_default_site=True).root_page - self.page = BlogPage( - title="Test page", - slug="test-page", - content=[("text", "Hello, world!")], - live=True, - ) - self.root_page.add_child(instance=self.page) - self.page.save() - def test_export_view(self): + def test_import_view_get(self): self.login() response = self.client.get( - reverse("export_page", kwargs={"page_id": self.page.id}) + reverse( + "wagtail_deletion_archive_import", + kwargs={"page_id": self.root_page.pk}, + ), ) self.assertEqual(response.status_code, 200) + self.assertIn("form", response.context) - self.assertIn("Content-Disposition", response.headers) - self.assertEqual( - response.headers["Content-Disposition"], - 'attachment; filename="test-page.json"', - ) - - self.assertIn("Content-Type", response.headers) - self.assertEqual(response.headers["Content-Type"], "application/json") - + def post_to_import_view(self, **kwargs): + self.login() -class ImportViewTestCase(TestCase, WagtailTestUtils): - def setUp(self): - self.root_page = Site.objects.get(is_default_site=True).root_page + return self.client.post( + reverse( + "wagtail_deletion_archive_import", + kwargs={"page_id": self.root_page.pk}, + ), + kwargs, + ) def test_import_view_post_file(self): - # Export a test page test_page = BlogPage( title="Test page", slug="test-page", content=[("text", "Hello, world!")], live=True, ) - page_json = export_page(test_page) - page_io = StringIO(page_json) + page_json = convert_page_to_json(test_page) + response = self.post_to_import_view(page_file=StringIO(page_json)) - # Log in and post it - self.login() - response = self.client.post( - reverse("import_page", kwargs={"page_id": self.root_page.id}), - {"page_file": page_io}, - ) self.assertEqual(response.status_code, 302) self.root_page.refresh_from_db() @@ -76,22 +74,11 @@ def test_import_view_post_file(self): ) def test_import_view_post_invalid(self): - self.login() - response = self.client.post( - reverse("import_page", kwargs={"page_id": self.root_page.id}), - {}, - ) - self.assertEqual(response.status_code, 200) - self.assertIn("form", response.context) - self.assertIn("page_file", response.context["form"].errors) + response = self.post_to_import_view() - def test_import_view_get(self): - self.login() - response = self.client.get( - reverse("import_page", kwargs={"page_id": self.root_page.id}), - ) self.assertEqual(response.status_code, 200) self.assertIn("form", response.context) + self.assertIn("page_file", response.context["form"].errors) def test_import_view_post_app_does_not_exist(self): page_json = json.dumps( @@ -102,14 +89,8 @@ def test_import_view_post_app_does_not_exist(self): "data": {}, } ) - page_io = StringIO(page_json) + response = self.post_to_import_view(page_file=StringIO(page_json)) - # Log in and post it - self.login() - response = self.client.post( - reverse("import_page", kwargs={"page_id": self.root_page.id}), - {"page_file": page_io}, - ) self.assertTrue(len(response.context["form"].errors["page_file"]) > 0) self.assertIn( "There was an error importing this file as a page.", @@ -125,16 +106,47 @@ def test_import_view_post_model_does_not_exist(self): "data": {}, } ) - page_io = StringIO(page_json) + response = self.post_to_import_view(page_file=StringIO(page_json)) - # Log in and post it - self.login() - response = self.client.post( - reverse("import_page", kwargs={"page_id": self.root_page.id}), - {"page_file": page_io}, - ) self.assertTrue(len(response.context["form"].errors["page_file"]) > 0) self.assertIn( "There was an error importing this file as a page.", response.context["form"].errors["page_file"][0], ) + + +class ArchiveViewTests(WagtailTestUtils, TestCase): + def test_no_storage_404(self): + self.assertIsNone(get_archive_storage()) + + self.login() + response = self.client.get(reverse("wagtail_deletion_archive_index")) + self.assertEqual(response.status_code, 404) + + @uses_temp_archive_storage() + def test_valid_storage(self, temp_storage): + os.mkdir(f"{temp_storage}/subdir") + filename = make_archive_filename("test") + open(os.path.join(temp_storage, "subdir", filename), "w").close() + + self.login() + response = self.client.get(reverse("wagtail_deletion_archive_index")) + self.assertContains(response, filename) + + response = self.client.get( + reverse( + "wagtail_deletion_archive_serve", + kwargs={"path": f"subdir/{filename}"}, + ) + ) + self.assertEqual( + response["Content-Disposition"], + f'attachment; filename="{filename}"', + ) + + response = self.client.get( + reverse( + "wagtail_deletion_archive_serve", kwargs={"path": "missing"} + ) + ) + self.assertEqual(response.status_code, 404) diff --git a/cfgov/wagtail_deletion_archival/tests/test_wagtail_hooks.py b/cfgov/wagtail_deletion_archival/tests/test_wagtail_hooks.py index dc2d7fb5fcf..9c92e581dcf 100644 --- a/cfgov/wagtail_deletion_archival/tests/test_wagtail_hooks.py +++ b/cfgov/wagtail_deletion_archival/tests/test_wagtail_hooks.py @@ -1,95 +1,20 @@ -from django.apps import apps -from django.test import TestCase, override_settings -from django.urls import reverse +from django.shortcuts import reverse +from django.test import TestCase from wagtail.models import Site from wagtail.test.utils import WagtailTestUtils -from v1.models import BlogPage -from wagtail_deletion_archival.wagtail_hooks import archive_page_data_receiver - -class ArchivePageOnDeletionTestCase(TestCase, WagtailTestUtils): - def setUp(self): +class TestPageImportButton(WagtailTestUtils, TestCase): + def test_page_import_button_renders(self): self.login() root_page = Site.objects.get(is_default_site=True).root_page - - self.test_page1 = BlogPage(title="test page 1", slug="test-page1") - root_page.add_child(instance=self.test_page1) - - self.test_child_page = BlogPage(title="child page", slug="test-child") - self.test_page1.add_child(instance=self.test_child_page) - - self.test_page2 = BlogPage(title="test page 2", slug="test-page2") - root_page.add_child(instance=self.test_page2) - - self.test_page3 = BlogPage(title="test page 3", slug="test-page3") - root_page.add_child(instance=self.test_page3) - - self.fs = apps.get_app_config("wagtail_deletion_archival").filesystem - - def test_delete_page(self): - self.client.post( - reverse("wagtailadmin_pages:delete", args=(self.test_page1.pk,)) - ) - - # The page slug exists in the archive dir and has a JSON file in it - self.assertIn(self.test_page1.slug, self.fs.listdir("/")) - self.assertTrue( - self.fs.glob(f"{self.test_page1.slug}/*.json").count().files > 0 - ) - - # The child page slug exists in the archive dir and has a JSON file - self.assertIn( - self.test_child_page.slug, - self.fs.listdir(f"{self.test_page1.slug}"), + response = self.client.get( + reverse("wagtailadmin_explore", args=(root_page.pk,)) ) - self.assertTrue( - self.fs.glob( - f"{self.test_page1.slug}/" - f"{self.test_child_page.slug}/*.json" - ) - .count() - .files - > 0 - ) - - def test_bulk_delete_page(self): - self.client.post( - reverse( - "wagtail_bulk_action", - args=( - "wagtailcore", - "page", - "delete", - ), - ) - + f"?id={self.test_page1.pk}&id={self.test_page2.pk}" - ) - - # The page slug exists in the archive dir - self.assertIn(self.test_page2.slug, self.fs.listdir("/")) - - # And has a JSON file in it - self.assertTrue( - self.fs.glob(f"{self.test_page2.slug}/*.json").count().files > 0 - ) - - def test_delete_page_confirmation(self): - self.client.get( - reverse("wagtailadmin_pages:delete", args=(self.test_page3.pk,)) - ) - self.assertEqual( - self.fs.glob(f"{self.test_page3.slug}/*.json").count().files, - 0, - ) - - @override_settings(WAGTAIL_DELETION_ARCHIVE_FILESYSTEM=None) - def test_delete_page_with_no_archive_dir(self): - archive_page_data_receiver(None, self.test_page3) - # There should be no test-page3 directory anywhere in the filesystem - self.assertEqual( - self.fs.glob(f"**/{self.test_page3.slug}/").count().directories, 0 + import_url = reverse( + "wagtail_deletion_archive_import", args=(root_page.pk,) ) + self.assertContains(response, f'/", import_view, name="import_page"), - ] - - @hooks.register("register_page_header_buttons") def page_import_button(page, user, view_name, next_url=None): yield Button( "Import", - reverse("import_page", args=(page.id,)), + reverse("wagtail_deletion_archive_import", args=(page.id,)), priority=200, icon_name="upload", ) -if get_archive_storage(): +@hooks.register("register_admin_urls") +def register_admin_urls(): + archive_url_prefix = "__deleted/" - @hooks.register("register_admin_urls") - def register_deleted_page_archive_list_url(): - return [ - path( - settings.WAGTAIL_DELETION_ARCHIVE_URL, - ArchiveIndexView.as_view(), - ), - path( - f"{settings.WAGTAIL_DELETION_ARCHIVE_URL}", - ArchiveFileView.as_view(), - name="wagtail_deletion_archive_serve", - ), - ] + return [ + path( + "import//", + import_view, + name="wagtail_deletion_archive_import", + ), + path( + archive_url_prefix, + ArchiveIndexView.as_view(), + name="wagtail_deletion_archive_index", + ), + path( + f"{archive_url_prefix}", + ArchiveFileView.as_view(), + name="wagtail_deletion_archive_serve", + ), + ] - @hooks.register("before_delete_page") - def add_archival_signal_reciever(request, page): - from wagtail.models import Page - pre_delete.connect(export_page_signal_handler, sender=Page) +@hooks.register("before_delete_page") +def add_archival_signal_reciever(request, page): + pre_delete.connect(export_page_signal_handler, sender=Page) - @hooks.register("after_delete_page") - def remove_archival_signal_reciever(request, page): - from wagtail.models import Page - pre_delete.disconnect(export_page_signal_handler, sender=Page) +@hooks.register("after_delete_page") +def remove_archival_signal_reciever(request, page): + pre_delete.disconnect(export_page_signal_handler, sender=Page) - @hooks.register("before_bulk_action") - def add_bulk_delete_signal_reciever( - request, action_type, objects, action_class_instance - ): - from wagtail.models import Page - if action_type == "delete": - pre_delete.connect(export_page_signal_handler, sender=Page) +@hooks.register("before_bulk_action") +def add_bulk_delete_signal_reciever( + request, action_type, objects, action_class_instance +): + if action_type == "delete": + pre_delete.connect(export_page_signal_handler, sender=Page) - @hooks.register("after_bulk_action") - def remove_bulk_delete_signal_reciever( - request, action_type, objects, action_class_instance - ): - from wagtail.models import Page - if action_type == "delete": - pre_delete.disconnect(export_page_signal_handler, sender=Page) +@hooks.register("after_bulk_action") +def remove_bulk_delete_signal_reciever( + request, action_type, objects, action_class_instance +): + if action_type == "delete": + pre_delete.disconnect(export_page_signal_handler, sender=Page) From a9a1c09f0ed68aa4ae47fa1347d5d15262532a6a Mon Sep 17 00:00:00 2001 From: Andy Chosak Date: Thu, 3 Oct 2024 10:42:28 -0400 Subject: [PATCH 3/3] Fix production staticfiles storage --- cfgov/cfgov/settings/production.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cfgov/cfgov/settings/production.py b/cfgov/cfgov/settings/production.py index 1dac13b931f..7e35a416405 100644 --- a/cfgov/cfgov/settings/production.py +++ b/cfgov/cfgov/settings/production.py @@ -85,9 +85,9 @@ EMAIL_HOST = os.getenv("EMAIL_HOST") DEFAULT_FROM_EMAIL = "wagtail@cfpb.gov" -STATICFILES_STORAGE = ( - "django.contrib.staticfiles.storage.ManifestStaticFilesStorage" -) +STORAGES["staticfiles"] = { + "BACKEND": "django.contrib.staticfiles.storage.ManifestStaticFilesStorage" +} STATIC_ROOT = os.environ["DJANGO_STATIC_ROOT"]