From 488291d20b13c0175534a88d7b2ae589a6fada06 Mon Sep 17 00:00:00 2001 From: Steve Jalim Date: Fri, 29 Nov 2024 17:16:32 +0000 Subject: [PATCH 01/10] Refactor settings to be a dict --- README.md | 24 ++++++--- wagtaildraftsharing/models.py | 10 ++-- wagtaildraftsharing/settings.py | 76 ++++++++++++++++----------- wagtaildraftsharing/snippets.py | 4 +- wagtaildraftsharing/tests/settings.py | 7 +++ wagtaildraftsharing/wagtail_hooks.py | 4 +- 6 files changed, 77 insertions(+), 48 deletions(-) diff --git a/README.md b/README.md index 83a6c3f..b39ad90 100644 --- a/README.md +++ b/README.md @@ -67,25 +67,37 @@ draft version saved since the last time the page was published ## Settings -The following settings can be added to your Django settings file: +The following settings can be added to your Django settings file as keys in a `WAGTAILDRAFTSHARING` dictionary -### ``WAGTAILDRAFTSHARING_MAX_AGE`` +``` +WAGTAILDRAFTSHARING = { + ... + "MAX_AGE": 123456, + ... +} +``` + +### ``MAX_AGE`` The default expiry time for generated links, in seconds. Defaults to 1 week. Set it to a negative value to disable expiry. -### ```WAGTAILDRAFTSHARING_ADMIN_MENU_POSITION``` +### ```ADMIN_MENU_POSITION``` Set the integer priority to control where in the admin menu the link to the list of generated links sits. Defaults to 200. -### ```WAGTAILDRAFTSHARING_VERBOSE_NAME``` +### ```VERBOSE_NAME``` Provide a different singular label for a link. Defaults to "Draftsharing Link" -### ```WAGTAILDRAFTSHARING_VERBOSE_NAME_PLURAL``` +### ```VERBOSE_NAME_PLURAL``` Provide a different plural label for a link. Defaults to "Draftsharing Links" -### ```WAGTAILDRAFTSHARING_MENU_ITEM_LABEL``` +### ```MENU_ITEM_LABEL``` Customise the label used in the page-level Action Menu for creating a link. Defaults to "Create draft sharing link", + +## Testing + +To install testing dependencies locally run `pip install -e ".[testing]"` diff --git a/wagtaildraftsharing/models.py b/wagtaildraftsharing/models.py index ed71d7f..70db66a 100644 --- a/wagtaildraftsharing/models.py +++ b/wagtaildraftsharing/models.py @@ -11,9 +11,9 @@ from wagtaildraftsharing.actions import WAGTAILDRAFTSHARING_CREATE_SHARING_LINK from wagtaildraftsharing.utils import tz_aware_utc_now -from . import settings as draftsharing_settings +from .settings import settings as draftsharing_settings -max_age = draftsharing_settings.WAGTAILDRAFTSHARING_MAX_AGE +max_age = draftsharing_settings.MAX_AGE class WagtaildraftsharingLinkManager(models.Manager): @@ -77,10 +77,8 @@ def __init__(self, *args, **kwargs): # Set the verbose names from settings, # in a way that doesn't trigger migrations - self._meta.verbose_name = draftsharing_settings.WAGTAILDRAFTSHARING_VERBOSE_NAME - self._meta.verbose_name_plural = ( - draftsharing_settings.WAGTAILDRAFTSHARING_VERBOSE_NAME_PLURAL - ) + self._meta.verbose_name = draftsharing_settings.VERBOSE_NAME + self._meta.verbose_name_plural = draftsharing_settings.VERBOSE_NAME_PLURAL def __str__(self): return f"Revision {self.revision_id} of {self.revision.content_object}" diff --git a/wagtaildraftsharing/settings.py b/wagtaildraftsharing/settings.py index e05db2f..c484438 100644 --- a/wagtaildraftsharing/settings.py +++ b/wagtaildraftsharing/settings.py @@ -1,32 +1,44 @@ -from django.conf import settings - -WAGTAILDRAFTSHARING_ADMIN_MENU_POSITION = getattr( - settings, - "WAGTAILDRAFTSHARING_ADMIN_MENU_POSITION", - 200, -) - -WAGTAILDRAFTSHARING_VERBOSE_NAME = getattr( - settings, - "WAGTAILDRAFTSHARING_VERBOSE_NAME", - "Draftsharing Link", -) - -WAGTAILDRAFTSHARING_VERBOSE_NAME_PLURAL = getattr( - settings, - "WAGTAILDRAFTSHARING_VERBOSE_NAME_PLURAL", - "Draftsharing Links", -) - -WAGTAILDRAFTSHARING_MENU_ITEM_LABEL = getattr( - settings, - "WAGTAILDRAFTSHARING_MENU_ITEM_LABEL", - "Create draft sharing link", -) - -DEFAULT_MAX_AGE = 7 * 24 * 60 * 60 # 7 days -WAGTAILDRAFTSHARING_MAX_AGE = getattr( - settings, - "WAGTAILDRAFTSHARING_MAX_AGE", - DEFAULT_MAX_AGE, -) +import dataclasses +from typing import cast + +from django.conf import settings as django_settings +from django.utils.functional import SimpleLazyObject + +_DEFAULT_MAX_AGE = 7 * 24 * 60 * 60 # 7 days + + +@dataclasses.dataclass(frozen=True) +class WagtaildraftsharingSettings: + ADMIN_MENU_POSITION: int = 200 + VERBOSE_NAME: str = "Draftsharing Link" + VERBOSE_NAME_PLURAL: str = "Draftsharing Links" + MENU_ITEM_LABEL: str = "Create draft sharing link" + MAX_AGE: int = _DEFAULT_MAX_AGE + + +def _init_settings(): + """ + Get and validate Wagtaildraftsharing settings from the Django settings. + """ + + setting_name = "WAGTAILDRAFTSHARING" + django_settings_dict = getattr(django_settings, setting_name, {}) + + overriden_defaults = {} + + for optional_setting_name in [ + "ADMIN_MENU_POSITION", + "VERBOSE_NAME", + "VERBOSE_NAME_PLURAL", + "MENU_ITEM_LABEL", + "MAX_AGE", + ]: + if optional_setting_name in django_settings_dict: + overriden_defaults[optional_setting_name] = django_settings_dict[ + optional_setting_name + ] + + return WagtaildraftsharingSettings(**overriden_defaults) + + +settings = cast(WagtaildraftsharingSettings, SimpleLazyObject(_init_settings)) diff --git a/wagtaildraftsharing/snippets.py b/wagtaildraftsharing/snippets.py index b0e5b2f..e1b6cb7 100644 --- a/wagtaildraftsharing/snippets.py +++ b/wagtaildraftsharing/snippets.py @@ -2,14 +2,14 @@ from wagtaildraftsharing.models import WagtaildraftsharingLink -from . import settings as draftsharing_settings +from .settings import settings as draftsharing_settings class WagtaildraftsharingLinkSnippetViewSet(SnippetViewSet): model = WagtaildraftsharingLink base_url_path = "wagtaildraftsharing" menu_icon = "view" - menu_order = draftsharing_settings.WAGTAILDRAFTSHARING_ADMIN_MENU_POSITION + menu_order = draftsharing_settings.ADMIN_MENU_POSITION add_to_settings_menu = False add_to_admin_menu = True list_display = ("__str__", "is_active", "created_by", "share_url") diff --git a/wagtaildraftsharing/tests/settings.py b/wagtaildraftsharing/tests/settings.py index 26e5917..b387af0 100644 --- a/wagtaildraftsharing/tests/settings.py +++ b/wagtaildraftsharing/tests/settings.py @@ -88,3 +88,10 @@ WAGTAILADMIN_BASE_URL = "http://localhost:8000" USE_TZ = True + +# Wagtaildraftsharing settings + +WAGTAILDRAFTSHARING = { + # Here is where settings would be overridden in a real project + # "MAX_AGE": 12312321, +} diff --git a/wagtaildraftsharing/wagtail_hooks.py b/wagtaildraftsharing/wagtail_hooks.py index a0913b4..312492c 100644 --- a/wagtaildraftsharing/wagtail_hooks.py +++ b/wagtaildraftsharing/wagtail_hooks.py @@ -14,7 +14,7 @@ ) from wagtaildraftsharing.snippets import WagtaildraftsharingLinkSnippetViewSet -from . import settings as draftsharing_settings +from .settings import settings as draftsharing_settings register_snippet(WagtaildraftsharingLinkSnippetViewSet) @@ -53,7 +53,7 @@ class DraftsharingPageActionMenuItem(ActionMenuItem): order = 1600 name = "action-draftsharing" icon_name = "view" - label = draftsharing_settings.WAGTAILDRAFTSHARING_MENU_ITEM_LABEL + label = draftsharing_settings.MENU_ITEM_LABEL template_name = "wagtaildraftsharing/action_menu_item.html" From 3698592e79ad78f8370bd171d242f71d3ec850a0 Mon Sep 17 00:00:00 2001 From: Steve Jalim Date: Tue, 3 Dec 2024 22:44:56 +0000 Subject: [PATCH 02/10] Allow the max age of a sharing link to be set via method arguments, not just via settings --- wagtaildraftsharing/models.py | 6 ++--- wagtaildraftsharing/settings.py | 2 +- wagtaildraftsharing/tests/test_models.py | 31 +++++++++++++++++++++++- 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/wagtaildraftsharing/models.py b/wagtaildraftsharing/models.py index 70db66a..81d6505 100644 --- a/wagtaildraftsharing/models.py +++ b/wagtaildraftsharing/models.py @@ -13,11 +13,11 @@ from .settings import settings as draftsharing_settings -max_age = draftsharing_settings.MAX_AGE - class WagtaildraftsharingLinkManager(models.Manager): - def get_or_create_for_revision(self, *, revision, user): + def get_or_create_for_revision(self, *, revision, user, max_age=None): + if max_age is None: + max_age = draftsharing_settings.MAX_AGE key = uuid.uuid4() if max_age > 0: active_until = tz_aware_utc_now() + timedelta(seconds=max_age) diff --git a/wagtaildraftsharing/settings.py b/wagtaildraftsharing/settings.py index c484438..c8c8fa2 100644 --- a/wagtaildraftsharing/settings.py +++ b/wagtaildraftsharing/settings.py @@ -7,7 +7,7 @@ _DEFAULT_MAX_AGE = 7 * 24 * 60 * 60 # 7 days -@dataclasses.dataclass(frozen=True) +@dataclasses.dataclass class WagtaildraftsharingSettings: ADMIN_MENU_POSITION: int = 200 VERBOSE_NAME: str = "Draftsharing Link" diff --git a/wagtaildraftsharing/tests/test_models.py b/wagtaildraftsharing/tests/test_models.py index c1c7f6c..c8984e6 100644 --- a/wagtaildraftsharing/tests/test_models.py +++ b/wagtaildraftsharing/tests/test_models.py @@ -54,7 +54,9 @@ def test_create_sharing_link_view__max_age_from_settings(self): for max_age, expected_expiry in max_ages_and_expected_expiries: with self.subTest(max_age=max_age, expected_expiry=expected_expiry): - with patch.object(wagtaildraftsharing.models, "max_age", max_age): + with patch.object( + wagtaildraftsharing.models.draftsharing_settings, "MAX_AGE", max_age + ): revision = self.create_revision() link = WagtaildraftsharingLink.objects.get_or_create_for_revision( @@ -67,6 +69,33 @@ def test_create_sharing_link_view__max_age_from_settings(self): expected_expiry, ) + @freeze_time(FROZEN_TIME_ISOFORMATTED) + def test_create_sharing_link_view__max_age_from_params(self): + frozen_time = datetime.datetime.fromisoformat(FROZEN_TIME_ISOFORMATTED) + + # Ensure we've got a level playing field: that the time is TZ-aware + if not is_aware(frozen_time): + self.fail("frozen_time was a naive datetime but it should not be") + + max_ages_and_expected_expiries = ( + (600, frozen_time + datetime.timedelta(seconds=600)), + (250000, frozen_time + datetime.timedelta(seconds=250000)), + (-1, None), + ) + + for max_age, expected_expiry in max_ages_and_expected_expiries: + with self.subTest(max_age=max_age, expected_expiry=expected_expiry): + revision = self.create_revision() + + link = WagtaildraftsharingLink.objects.get_or_create_for_revision( + revision=revision, user=self.test_user, max_age=max_age + ) + + assert link.active_until == expected_expiry, ( + link.active_until, + expected_expiry, + ) + class TestWagtaildraftsharingLinkModel(TestCase): def create_revision(self): From 75ea3ca761529facd5fa2c15d3880bcdcc484454 Mon Sep 17 00:00:00 2001 From: Steve Jalim Date: Wed, 4 Dec 2024 00:01:01 +0000 Subject: [PATCH 03/10] Rename max_age to max_ttl, ahead of adding min_ttl --- wagtaildraftsharing/models.py | 10 +++++----- wagtaildraftsharing/tests/test_models.py | 12 ++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/wagtaildraftsharing/models.py b/wagtaildraftsharing/models.py index 81d6505..28a9dc4 100644 --- a/wagtaildraftsharing/models.py +++ b/wagtaildraftsharing/models.py @@ -15,12 +15,12 @@ class WagtaildraftsharingLinkManager(models.Manager): - def get_or_create_for_revision(self, *, revision, user, max_age=None): - if max_age is None: - max_age = draftsharing_settings.MAX_AGE + def get_or_create_for_revision(self, *, revision, user, max_ttl=None): + if max_ttl is None: + max_ttl = draftsharing_settings.MAX_AGE key = uuid.uuid4() - if max_age > 0: - active_until = tz_aware_utc_now() + timedelta(seconds=max_age) + if max_ttl > 0: + active_until = tz_aware_utc_now() + timedelta(seconds=max_ttl) else: active_until = None sharing_link, created = WagtaildraftsharingLink.objects.get_or_create( diff --git a/wagtaildraftsharing/tests/test_models.py b/wagtaildraftsharing/tests/test_models.py index c8984e6..dde3c7b 100644 --- a/wagtaildraftsharing/tests/test_models.py +++ b/wagtaildraftsharing/tests/test_models.py @@ -52,10 +52,10 @@ def test_create_sharing_link_view__max_age_from_settings(self): (-1, None), ) - for max_age, expected_expiry in max_ages_and_expected_expiries: - with self.subTest(max_age=max_age, expected_expiry=expected_expiry): + for max_ttl, expected_expiry in max_ages_and_expected_expiries: + with self.subTest(max_ttl=max_ttl, expected_expiry=expected_expiry): with patch.object( - wagtaildraftsharing.models.draftsharing_settings, "MAX_AGE", max_age + wagtaildraftsharing.models.draftsharing_settings, "MAX_AGE", max_ttl ): revision = self.create_revision() @@ -83,12 +83,12 @@ def test_create_sharing_link_view__max_age_from_params(self): (-1, None), ) - for max_age, expected_expiry in max_ages_and_expected_expiries: - with self.subTest(max_age=max_age, expected_expiry=expected_expiry): + for max_ttl, expected_expiry in max_ages_and_expected_expiries: + with self.subTest(max_ttl=max_ttl, expected_expiry=expected_expiry): revision = self.create_revision() link = WagtaildraftsharingLink.objects.get_or_create_for_revision( - revision=revision, user=self.test_user, max_age=max_age + revision=revision, user=self.test_user, max_ttl=max_ttl ) assert link.active_until == expected_expiry, ( From 13e1b2c24a1c934e9f5789db24e23f6231682a1a Mon Sep 17 00:00:00 2001 From: Steve Jalim Date: Wed, 4 Dec 2024 00:52:31 +0000 Subject: [PATCH 04/10] Also rename MAX_AGE setting to MAX_TTL to match max_tt method argument --- README.md | 4 ++-- wagtaildraftsharing/models.py | 2 +- wagtaildraftsharing/settings.py | 6 +++--- wagtaildraftsharing/tests/settings.py | 2 +- wagtaildraftsharing/tests/test_models.py | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index b39ad90..1dbb214 100644 --- a/README.md +++ b/README.md @@ -72,12 +72,12 @@ The following settings can be added to your Django settings file as keys in a `W ``` WAGTAILDRAFTSHARING = { ... - "MAX_AGE": 123456, + "MAX_TTL": 123456, ... } ``` -### ``MAX_AGE`` +### ``MAX_TTL`` The default expiry time for generated links, in seconds. Defaults to 1 week. Set it to a negative value to disable expiry. diff --git a/wagtaildraftsharing/models.py b/wagtaildraftsharing/models.py index 28a9dc4..07ae742 100644 --- a/wagtaildraftsharing/models.py +++ b/wagtaildraftsharing/models.py @@ -17,7 +17,7 @@ class WagtaildraftsharingLinkManager(models.Manager): def get_or_create_for_revision(self, *, revision, user, max_ttl=None): if max_ttl is None: - max_ttl = draftsharing_settings.MAX_AGE + max_ttl = draftsharing_settings.MAX_TTL key = uuid.uuid4() if max_ttl > 0: active_until = tz_aware_utc_now() + timedelta(seconds=max_ttl) diff --git a/wagtaildraftsharing/settings.py b/wagtaildraftsharing/settings.py index c8c8fa2..6f6a35a 100644 --- a/wagtaildraftsharing/settings.py +++ b/wagtaildraftsharing/settings.py @@ -4,7 +4,7 @@ from django.conf import settings as django_settings from django.utils.functional import SimpleLazyObject -_DEFAULT_MAX_AGE = 7 * 24 * 60 * 60 # 7 days +_DEFAULT_MAX_TTL = 7 * 24 * 60 * 60 # 7 days @dataclasses.dataclass @@ -13,7 +13,7 @@ class WagtaildraftsharingSettings: VERBOSE_NAME: str = "Draftsharing Link" VERBOSE_NAME_PLURAL: str = "Draftsharing Links" MENU_ITEM_LABEL: str = "Create draft sharing link" - MAX_AGE: int = _DEFAULT_MAX_AGE + MAX_TTL: int = _DEFAULT_MAX_TTL def _init_settings(): @@ -31,7 +31,7 @@ def _init_settings(): "VERBOSE_NAME", "VERBOSE_NAME_PLURAL", "MENU_ITEM_LABEL", - "MAX_AGE", + "MAX_TTL", ]: if optional_setting_name in django_settings_dict: overriden_defaults[optional_setting_name] = django_settings_dict[ diff --git a/wagtaildraftsharing/tests/settings.py b/wagtaildraftsharing/tests/settings.py index b387af0..7abb3c5 100644 --- a/wagtaildraftsharing/tests/settings.py +++ b/wagtaildraftsharing/tests/settings.py @@ -93,5 +93,5 @@ WAGTAILDRAFTSHARING = { # Here is where settings would be overridden in a real project - # "MAX_AGE": 12312321, + # "MAX_TTL": 12312321, } diff --git a/wagtaildraftsharing/tests/test_models.py b/wagtaildraftsharing/tests/test_models.py index dde3c7b..a3ea09a 100644 --- a/wagtaildraftsharing/tests/test_models.py +++ b/wagtaildraftsharing/tests/test_models.py @@ -55,7 +55,7 @@ def test_create_sharing_link_view__max_age_from_settings(self): for max_ttl, expected_expiry in max_ages_and_expected_expiries: with self.subTest(max_ttl=max_ttl, expected_expiry=expected_expiry): with patch.object( - wagtaildraftsharing.models.draftsharing_settings, "MAX_AGE", max_ttl + wagtaildraftsharing.models.draftsharing_settings, "MAX_TTL", max_ttl ): revision = self.create_revision() From cfc9160139251aa84c8c3842e63d73852b0987fe Mon Sep 17 00:00:00 2001 From: Steve Jalim Date: Wed, 4 Dec 2024 00:55:24 +0000 Subject: [PATCH 05/10] Fix erroneous test names --- wagtaildraftsharing/tests/test_models.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/wagtaildraftsharing/tests/test_models.py b/wagtaildraftsharing/tests/test_models.py index a3ea09a..d4abb87 100644 --- a/wagtaildraftsharing/tests/test_models.py +++ b/wagtaildraftsharing/tests/test_models.py @@ -39,20 +39,20 @@ def create_revision(self): return earliest_revision @freeze_time(FROZEN_TIME_ISOFORMATTED) - def test_create_sharing_link_view__max_age_from_settings(self): + def test_create_sharing_link__max_ttl_from_settings(self): frozen_time = datetime.datetime.fromisoformat(FROZEN_TIME_ISOFORMATTED) # Ensure we've got a level playing field: that the time is TZ-aware if not is_aware(frozen_time): self.fail("frozen_time was a naive datetime but it should not be") - max_ages_and_expected_expiries = ( + max_ttls_and_expected_expiries = ( (300, frozen_time + datetime.timedelta(seconds=300)), (1250000, frozen_time + datetime.timedelta(seconds=1250000)), (-1, None), ) - for max_ttl, expected_expiry in max_ages_and_expected_expiries: + for max_ttl, expected_expiry in max_ttls_and_expected_expiries: with self.subTest(max_ttl=max_ttl, expected_expiry=expected_expiry): with patch.object( wagtaildraftsharing.models.draftsharing_settings, "MAX_TTL", max_ttl @@ -70,20 +70,20 @@ def test_create_sharing_link_view__max_age_from_settings(self): ) @freeze_time(FROZEN_TIME_ISOFORMATTED) - def test_create_sharing_link_view__max_age_from_params(self): + def test_create_sharing_link__max_ttl_from_params(self): frozen_time = datetime.datetime.fromisoformat(FROZEN_TIME_ISOFORMATTED) # Ensure we've got a level playing field: that the time is TZ-aware if not is_aware(frozen_time): self.fail("frozen_time was a naive datetime but it should not be") - max_ages_and_expected_expiries = ( + max_ttls_and_expected_expiries = ( (600, frozen_time + datetime.timedelta(seconds=600)), (250000, frozen_time + datetime.timedelta(seconds=250000)), (-1, None), ) - for max_ttl, expected_expiry in max_ages_and_expected_expiries: + for max_ttl, expected_expiry in max_ttls_and_expected_expiries: with self.subTest(max_ttl=max_ttl, expected_expiry=expected_expiry): revision = self.create_revision() From b828cbb4b5a93389980e11902ea0dc602bcd5aee Mon Sep 17 00:00:00 2001 From: Steve Jalim Date: Wed, 4 Dec 2024 11:27:37 +0000 Subject: [PATCH 06/10] Support making a new sharing link each time, not reusing an existing one This avoids the downside of reusing links: you might get a link that will expire soon (or has expired but won't be marked as such until it's used) --- ..._alter_wagtaildraftsharinglink_revision.py | 22 +++++ wagtaildraftsharing/models.py | 39 ++++++++- wagtaildraftsharing/tests/test_models.py | 80 ++++++++++++++----- 3 files changed, 115 insertions(+), 26 deletions(-) create mode 100644 wagtaildraftsharing/migrations/0003_alter_wagtaildraftsharinglink_revision.py diff --git a/wagtaildraftsharing/migrations/0003_alter_wagtaildraftsharinglink_revision.py b/wagtaildraftsharing/migrations/0003_alter_wagtaildraftsharinglink_revision.py new file mode 100644 index 0000000..6e4da8f --- /dev/null +++ b/wagtaildraftsharing/migrations/0003_alter_wagtaildraftsharinglink_revision.py @@ -0,0 +1,22 @@ +# Generated by Django 5.1.1 on 2024-12-04 10:56 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("wagtaildraftsharing", "0002_alter_wagtaildraftsharinglink_created_by"), + ] + + operations = [ + migrations.AlterField( + model_name="wagtaildraftsharinglink", + name="revision", + field=models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="+", + to="wagtailcore.revision", + ), + ), + ] diff --git a/wagtaildraftsharing/models.py b/wagtaildraftsharing/models.py index 07ae742..2255232 100644 --- a/wagtaildraftsharing/models.py +++ b/wagtaildraftsharing/models.py @@ -15,18 +15,49 @@ class WagtaildraftsharingLinkManager(models.Manager): - def get_or_create_for_revision(self, *, revision, user, max_ttl=None): + def _get_active_until(self, max_ttl): if max_ttl is None: max_ttl = draftsharing_settings.MAX_TTL - key = uuid.uuid4() if max_ttl > 0: active_until = tz_aware_utc_now() + timedelta(seconds=max_ttl) else: active_until = None + + return active_until + + def create_for_revision(self, *, revision, user, max_ttl=None): + # Always creates a new sharing link + sharing_link = WagtaildraftsharingLink.objects.create( + revision=revision, + key=uuid.uuid4(), + created_by=user, + active_until=self._get_active_until(max_ttl=max_ttl), + ) + log( + instance=revision.content_object, + action=WAGTAILDRAFTSHARING_CREATE_SHARING_LINK, + user=user, + revision=revision, + data={"revision": revision.id}, + ) + + return sharing_link + + def get_or_create_for_revision(self, *, revision, user, max_ttl=None): + """ + Deprecated: prefer self.create_for_revision() instead + + Creates a new sharing link if it doesn't exist, else returns an + existing one. Note that this may give you a link that is soon to + expire if it was made close to max_ttl seconds ago... + """ + + active_until = self._get_active_until(max_ttl=max_ttl) + sharing_link, created = WagtaildraftsharingLink.objects.get_or_create( revision=revision, defaults={ - "key": key, + "key": uuid.uuid4(), "created_by": user, "active_until": active_until, }, @@ -50,7 +81,7 @@ class WagtaildraftsharingLink(models.Model): editable=False, primary_key=False, ) - revision = models.OneToOneField( + revision = models.ForeignKey( "wagtailcore.Revision", on_delete=models.CASCADE, related_name="+", diff --git a/wagtaildraftsharing/tests/test_models.py b/wagtaildraftsharing/tests/test_models.py index d4abb87..9a6e972 100644 --- a/wagtaildraftsharing/tests/test_models.py +++ b/wagtaildraftsharing/tests/test_models.py @@ -53,21 +53,31 @@ def test_create_sharing_link__max_ttl_from_settings(self): ) for max_ttl, expected_expiry in max_ttls_and_expected_expiries: - with self.subTest(max_ttl=max_ttl, expected_expiry=expected_expiry): - with patch.object( - wagtaildraftsharing.models.draftsharing_settings, "MAX_TTL", max_ttl + for method_name in [ + "get_or_create_for_revision", + "create_for_revision", + ]: + with self.subTest( + max_ttl=max_ttl, + expected_expiry=expected_expiry, + method_name=method_name, ): - revision = self.create_revision() - - link = WagtaildraftsharingLink.objects.get_or_create_for_revision( - revision=revision, - user=self.test_user, - ) - - assert link.active_until == expected_expiry, ( - link.active_until, - expected_expiry, - ) + with patch.object( + wagtaildraftsharing.models.draftsharing_settings, + "MAX_TTL", + max_ttl, + ): + revision = self.create_revision() + + link = getattr(WagtaildraftsharingLink.objects, method_name)( + revision=revision, + user=self.test_user, + ) + + assert link.active_until == expected_expiry, ( + link.active_until, + expected_expiry, + ) @freeze_time(FROZEN_TIME_ISOFORMATTED) def test_create_sharing_link__max_ttl_from_params(self): @@ -84,17 +94,43 @@ def test_create_sharing_link__max_ttl_from_params(self): ) for max_ttl, expected_expiry in max_ttls_and_expected_expiries: - with self.subTest(max_ttl=max_ttl, expected_expiry=expected_expiry): - revision = self.create_revision() + for method_name in [ + "get_or_create_for_revision", + "create_for_revision", + ]: + with self.subTest( + max_ttl=max_ttl, + expected_expiry=expected_expiry, + method_name=method_name, + ): + revision = self.create_revision() - link = WagtaildraftsharingLink.objects.get_or_create_for_revision( - revision=revision, user=self.test_user, max_ttl=max_ttl - ) + link = getattr(WagtaildraftsharingLink.objects, method_name)( + revision=revision, user=self.test_user, max_ttl=max_ttl + ) + + assert link.active_until == expected_expiry, ( + link.active_until, + expected_expiry, + ) - assert link.active_until == expected_expiry, ( - link.active_until, - expected_expiry, + def test_create_sharing_link_allows_multiple_links_per_revision(self): + for method_name in [ + "get_or_create_for_revision", + "create_for_revision", + ]: + with self.subTest(method_name=method_name): + revision = self.create_revision() + link_1 = getattr(WagtaildraftsharingLink.objects, method_name)( + revision=revision, user=self.test_user + ) + link_2 = getattr(WagtaildraftsharingLink.objects, method_name)( + revision=revision, user=self.test_user ) + if method_name == "get_or_create_for_revision": + assert link_1.key == link_2.key + else: + assert link_1.key != link_2.key class TestWagtaildraftsharingLinkModel(TestCase): From 4c60d70c7115498d97f3996333e94d58687712d3 Mon Sep 17 00:00:00 2001 From: Steve Jalim Date: Wed, 4 Dec 2024 11:27:51 +0000 Subject: [PATCH 07/10] Increase default link TTL to 28 days --- wagtaildraftsharing/settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wagtaildraftsharing/settings.py b/wagtaildraftsharing/settings.py index 6f6a35a..29b487b 100644 --- a/wagtaildraftsharing/settings.py +++ b/wagtaildraftsharing/settings.py @@ -4,7 +4,7 @@ from django.conf import settings as django_settings from django.utils.functional import SimpleLazyObject -_DEFAULT_MAX_TTL = 7 * 24 * 60 * 60 # 7 days +_DEFAULT_MAX_TTL = 28 * 24 * 60 * 60 # 28 days @dataclasses.dataclass From 33ba471bf78c2320b130e0f4b82ea6df303d10ab Mon Sep 17 00:00:00 2001 From: Steve Jalim Date: Wed, 4 Dec 2024 11:28:25 +0000 Subject: [PATCH 08/10] Add extra audit logging to show when we're reusing an existing link vs generating a new one --- wagtaildraftsharing/actions.py | 13 +++++++++++++ wagtaildraftsharing/models.py | 13 ++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/wagtaildraftsharing/actions.py b/wagtaildraftsharing/actions.py index 1d86bc2..99f13dc 100644 --- a/wagtaildraftsharing/actions.py +++ b/wagtaildraftsharing/actions.py @@ -2,6 +2,7 @@ from wagtail.log_actions import LogFormatter WAGTAILDRAFTSHARING_CREATE_SHARING_LINK = "wagtaildraftsharing.create_sharing_link" +WAGTAILDRAFTSHARING_REUSE_SHARING_LINK = "wagtaildraftsharing.reuse_sharing_link" def register_wagtaildraftsharing_log_actions(actions): @@ -16,3 +17,15 @@ def format_message(self, log_entry): revision_id=log_entry.data["revision"], username=log_entry.user, ) + + @actions.register_action(WAGTAILDRAFTSHARING_REUSE_SHARING_LINK) + class ReuseSharingLink(LogFormatter): + label = _("Reuse sharing link") + + def format_message(self, log_entry): + return _( + "{username} reused sharing link for revision {revision_id}" + ).format( + revision_id=log_entry.data["revision"], + username=log_entry.user, + ) diff --git a/wagtaildraftsharing/models.py b/wagtaildraftsharing/models.py index 2255232..e6a5284 100644 --- a/wagtaildraftsharing/models.py +++ b/wagtaildraftsharing/models.py @@ -8,7 +8,10 @@ from django.utils.timezone import timedelta from wagtail.log_actions import log -from wagtaildraftsharing.actions import WAGTAILDRAFTSHARING_CREATE_SHARING_LINK +from wagtaildraftsharing.actions import ( + WAGTAILDRAFTSHARING_CREATE_SHARING_LINK, + WAGTAILDRAFTSHARING_REUSE_SHARING_LINK, +) from wagtaildraftsharing.utils import tz_aware_utc_now from .settings import settings as draftsharing_settings @@ -70,6 +73,14 @@ def get_or_create_for_revision(self, *, revision, user, max_ttl=None): revision=revision, data={"revision": revision.id}, ) + else: + log( + instance=revision.content_object, + action=WAGTAILDRAFTSHARING_REUSE_SHARING_LINK, + user=user, + revision=revision, + data={"revision": revision.id}, + ) return sharing_link From c17d3cd763cbe9a8f68812382b85a6d145bc9b88 Mon Sep 17 00:00:00 2001 From: Steve Jalim Date: Wed, 4 Dec 2024 11:29:08 +0000 Subject: [PATCH 09/10] Prep 0.2.0 release --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index c91fc7c..ffbce98 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "wagtaildraftsharing" -version = "0.1.3" +version = "0.2.0" description = "Share wagtail drafts with private URLs." readme = "README.md" requires-python = ">=3.9" From ae030bf1f59a1852f6f8c6be4c892292731e8a53 Mon Sep 17 00:00:00 2001 From: Steve Jalim Date: Wed, 4 Dec 2024 11:52:41 +0000 Subject: [PATCH 10/10] Update documentation --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 1dbb214..7df0610 100644 --- a/README.md +++ b/README.md @@ -79,7 +79,7 @@ WAGTAILDRAFTSHARING = { ### ``MAX_TTL`` -The default expiry time for generated links, in seconds. Defaults to 1 week. Set it to a negative value to disable expiry. +The default expiry time for generated links, in seconds. Defaults to 28 days. Set it to a negative value to disable expiry. ### ```ADMIN_MENU_POSITION```