From 2e248a2f9a56ae42c4ee2c115a1b058927956483 Mon Sep 17 00:00:00 2001 From: Hercilio Ortiz Date: Tue, 12 Aug 2025 20:03:40 -0300 Subject: [PATCH 01/18] Feat: Connecting Wagtail page to URLPath model --- baseapp_pages/graphql/__init__.py | 2 + baseapp_pages/graphql/object_types.py | 95 +++++++++++------ baseapp_pages/urlpath_interfaces.py | 51 +++++++++ baseapp_pages/urlpath_registry.py | 32 ++++++ baseapp_wagtail/base/apps.py | 9 ++ baseapp_wagtail/base/graphql/interfaces.py | 53 ++++++++++ baseapp_wagtail/base/graphql/object_types.py | 47 +-------- baseapp_wagtail/base/models.py | 17 +-- baseapp_wagtail/base/urlpath/__init__.py | 0 baseapp_wagtail/base/urlpath/urlpath_sync.py | 105 +++++++++++++++++++ baseapp_wagtail/base/wagtail_hooks.py | 22 ++++ baseapp_wagtail/settings.py | 2 +- testproject/settings.py | 2 +- 13 files changed, 354 insertions(+), 83 deletions(-) create mode 100644 baseapp_pages/urlpath_interfaces.py create mode 100644 baseapp_pages/urlpath_registry.py create mode 100644 baseapp_wagtail/base/graphql/interfaces.py create mode 100644 baseapp_wagtail/base/urlpath/__init__.py create mode 100644 baseapp_wagtail/base/urlpath/urlpath_sync.py diff --git a/baseapp_pages/graphql/__init__.py b/baseapp_pages/graphql/__init__.py index dd93305c..8b77eb97 100644 --- a/baseapp_pages/graphql/__init__.py +++ b/baseapp_pages/graphql/__init__.py @@ -1,4 +1,6 @@ from .mutations import PageCreate, PageEdit # noqa + +# TODO: (ho) review this file. from .object_types import ( # noqa MetadataObjectType, PageInterface, diff --git a/baseapp_pages/graphql/object_types.py b/baseapp_pages/graphql/object_types.py index 99384251..b3fad413 100644 --- a/baseapp_pages/graphql/object_types.py +++ b/baseapp_pages/graphql/object_types.py @@ -11,6 +11,7 @@ from baseapp_comments.graphql.object_types import CommentsInterface from baseapp_core.graphql import DjangoObjectType, LanguagesEnum, ThumbnailField from baseapp_pages.models import AbstractPage, Metadata, URLPath +from baseapp_pages.urlpath_registry import urlpath_registry from ..meta import AbstractMetadataObjectType @@ -41,37 +42,11 @@ def resolve_metadata(cls, instance, info, **kwargs): raise NotImplementedError -class URLPathNode(DjangoObjectType): - target = graphene.Field(PageInterface) - language = graphene.Field(LanguagesEnum) - - class Meta: - interfaces = (relay.Node,) - model = URLPath - fields = ( - "id", - "pk", - "path", - "language", - "is_active", - "created", - "modified", - "target", - ) - filter_fields = { - "id": ["exact"], - } +class UrlPathTargetInterface(graphene.Interface): + data = graphene.Field(PageInterface) - def resolve_target(self, info, **kwargs): - if isinstance(self.target, AbstractPage): - if not info.context.user.has_perm(f"{page_app_label}.view_page", self.target): - return None - return self.target - - @classmethod - def get_queryset(cls, queryset, info): - MAX_COMPLEXITY = 3 - return optimize(queryset, info, max_complexity=MAX_COMPLEXITY) + def resolve_data(self, info): + return self class PageFilter(django_filters.FilterSet): @@ -83,7 +58,7 @@ class Meta: class BasePageObjectType: metadata = graphene.Field(lambda: MetadataObjectType) status = graphene.Field(PageStatusEnum) - title = graphene.String() + title = graphene.String(required=True) body = graphene.String() class Meta: @@ -162,3 +137,61 @@ def is_type_of(cls, root, info): if isinstance(root, AbstractMetadataObjectType): return True return super().is_type_of(root, info) + + +class PagesPageObjectType(graphene.ObjectType): + class Meta: + interfaces = (UrlPathTargetInterface,) + name = "PagesPage" + + +class TargetAvailableTypes(graphene.Union): + class Meta: + types = (PagesPageObjectType, *urlpath_registry.get_all_types()) + + @classmethod + def resolve_type(cls, instance, info): + if isinstance(instance, AbstractPage): + return PageObjectType + + graphql_type = urlpath_registry.get_type(instance) + if graphql_type: + return graphql_type + + return None + + +class URLPathNode(DjangoObjectType): + target = graphene.Field(TargetAvailableTypes) + language = graphene.Field(LanguagesEnum) + + class Meta: + interfaces = (relay.Node,) + model = URLPath + fields = ( + "id", + "pk", + "path", + "language", + "is_active", + "created", + "modified", + "target", + ) + filter_fields = { + "id": ["exact"], + } + + def resolve_target(self, info, **kwargs): + if isinstance(self.target, AbstractPage): + if not info.context.user.has_perm(f"{page_app_label}.view_page", self.target): + return None + # For other targets, we rely on the registry to provide the correct GraphQL type + # Permission checking would need to be implemented in the foreign app's GraphQL types + + return self.target + + @classmethod + def get_queryset(cls, queryset, info): + MAX_COMPLEXITY = 3 + return optimize(queryset, info, max_complexity=MAX_COMPLEXITY) diff --git a/baseapp_pages/urlpath_interfaces.py b/baseapp_pages/urlpath_interfaces.py new file mode 100644 index 00000000..5a445406 --- /dev/null +++ b/baseapp_pages/urlpath_interfaces.py @@ -0,0 +1,51 @@ +from typing import Optional, Type + +from django.contrib.auth.models import AbstractUser +from django.contrib.contenttypes.fields import GenericRelation +from django.db import models + +from baseapp_pages.models import URLPath + + +class URLPathTargetMixin(models.Model): + """Mixin for models that want to be URLPath targets""" + + url_paths = GenericRelation( + URLPath, + content_type_field="target_content_type", + object_id_field="target_object_id", + ) + + class Meta: + abstract = True + + def get_graphql_object_type(self) -> Type: + raise NotImplementedError + + def get_permission_check(self, user: AbstractUser) -> bool: + return True + + def create_url_path(self, path: str, language: Optional[str] = None, is_active: bool = True): + if not self.pk: + raise ValueError("Save the instance before creating URL paths.") + return self.url_paths.create(path=path, language=language, is_active=is_active) + + def update_url_path(self, path: str, language: Optional[str] = None, is_active: bool = True): + if not self.pk: + raise ValueError("Save the instance before updating URL paths.") + primary_path = self.url_path + if primary_path: + primary_path.path = path + primary_path.language = language + primary_path.is_active = is_active + primary_path.save() + else: + self.create_url_path(path, language, is_active) + + def deactivate_url_paths(self): + if self.pk: + self.url_paths.update(is_active=False) + + def delete_url_paths(self): + if self.pk: + self.url_paths.all().delete() diff --git a/baseapp_pages/urlpath_registry.py b/baseapp_pages/urlpath_registry.py new file mode 100644 index 00000000..c1c2a606 --- /dev/null +++ b/baseapp_pages/urlpath_registry.py @@ -0,0 +1,32 @@ +from typing import Dict, List, Type + +from django.contrib.contenttypes.models import ContentType +from django.db.models import Model + + +class URLPathRegistry: + def __init__(self): + self._graphql_types: Dict[str, Type] = {} + + def register_type(self, model_class: Type[Model], graphql_type: Type): + content_type = ContentType.objects.get_for_model(model_class) + key = f"{content_type.app_label}.{content_type.model}" + self._graphql_types[key] = graphql_type + + def get_type(self, instance: Model) -> Type: + content_type = ContentType.objects.get_for_model(instance) + key = f"{content_type.app_label}.{content_type.model}" + return self._graphql_types.get(key) + + def get_all_types(self) -> List[Type]: + return list(set(self._graphql_types.values())) + + +# Global registry instance +urlpath_registry = URLPathRegistry() + + +def register_urlpath_type(model_class: Type[Model], graphql_type: Type): + """Decorator to register a GraphQL type for a model""" + urlpath_registry.register_type(model_class, graphql_type) + return model_class diff --git a/baseapp_wagtail/base/apps.py b/baseapp_wagtail/base/apps.py index b104f017..e6aaad2b 100644 --- a/baseapp_wagtail/base/apps.py +++ b/baseapp_wagtail/base/apps.py @@ -5,3 +5,12 @@ class WagtailConfig(AppConfig): name = "baseapp_wagtail.base" verbose_name = "BaseApp Wagtail - Base" label = "baseapp_wagtail_base" + + def ready(self): + from grapple.registry import registry + + from baseapp_pages.urlpath_registry import urlpath_registry + from baseapp_wagtail.base.graphql.object_types import WagtailPageObjectType + + for model_type in registry.pages.keys(): + urlpath_registry.register_type(model_type, WagtailPageObjectType) diff --git a/baseapp_wagtail/base/graphql/interfaces.py b/baseapp_wagtail/base/graphql/interfaces.py new file mode 100644 index 00000000..bf9ca4cb --- /dev/null +++ b/baseapp_wagtail/base/graphql/interfaces.py @@ -0,0 +1,53 @@ +import graphene +from grapple.types.interfaces import PageInterface + +from baseapp_comments.graphql.object_types import CommentsInterface +from baseapp_notifications.graphql.object_types import NotificationsInterface +from baseapp_reactions.graphql.object_types import ReactionsInterface +from baseapp_reports.graphql.object_types import ReportsInterface + +# TODO: Fix in next story +# https://app.approvd.io/silverlogic/BA/stories/36399 +# As per @ap, these interfaces shouldn't be necessary. They are currently necessary because +# the baseapp interfaces classes inherit from RelayNode, but they shouldn't! +# +# Also if we return -1 for resolve_id, it means we are not Relay compliant. This can cause bugs. + + +class WagtailCommentsInterface(CommentsInterface): + id = graphene.ID() + + def resolve_id(self, info, **kwargs): + return str(self.id) if self.id is not None else -1 + + +class WagtailReactionsInterface(ReactionsInterface): + id = graphene.ID() + + def resolve_id(self, info, **kwargs): + return str(self.id) if self.id is not None else -1 + + +class WagtailNotificationsInterfaceInterface(NotificationsInterface): + id = graphene.ID() + + def resolve_id(self, info, **kwargs): + return str(self.id) if self.id is not None else -1 + + +class WagtailReportsInterfaceInterface(ReportsInterface): + id = graphene.ID() + + def resolve_id(self, info, **kwargs): + return str(self.id) if self.id is not None else -1 + + +class WagtailPageInterface(PageInterface): + pass + + +class URLPathTargetWagtailInterface(graphene.Interface): + data = graphene.Field(WagtailPageInterface) + + def resolve_data(self, info): + return self diff --git a/baseapp_wagtail/base/graphql/object_types.py b/baseapp_wagtail/base/graphql/object_types.py index 3bb94058..0a9bf6c5 100644 --- a/baseapp_wagtail/base/graphql/object_types.py +++ b/baseapp_wagtail/base/graphql/object_types.py @@ -1,46 +1,9 @@ import graphene -from grapple.types.interfaces import PageInterface -from baseapp_comments.graphql.object_types import CommentsInterface -from baseapp_notifications.graphql.object_types import NotificationsInterface -from baseapp_reactions.graphql.object_types import ReactionsInterface -from baseapp_reports.graphql.object_types import ReportsInterface +from baseapp_wagtail.base.graphql.interfaces import URLPathTargetWagtailInterface -# TODO: Fix in next story -# https://app.approvd.io/silverlogic/BA/stories/36399 -# As per @ap, these interfaces shouldn't be necessary. They are currently necessary because -# the baseapp interfaces classes inherit from RelayNode, but they shouldn't! -# -# Also if we return -1 for resolve_id, it means we are not Relay compliant. This can cause bugs. - -class WagtailCommentsInterface(CommentsInterface): - id = graphene.ID() - - def resolve_id(self, info, **kwargs): - return str(self.id) if self.id is not None else -1 - - -class WagtailReactionsInterface(ReactionsInterface): - id = graphene.ID() - - def resolve_id(self, info, **kwargs): - return str(self.id) if self.id is not None else -1 - - -class WagtailNotificationsInterfaceInterface(NotificationsInterface): - id = graphene.ID() - - def resolve_id(self, info, **kwargs): - return str(self.id) if self.id is not None else -1 - - -class WagtailReportsInterfaceInterface(ReportsInterface): - id = graphene.ID() - - def resolve_id(self, info, **kwargs): - return str(self.id) if self.id is not None else -1 - - -class WagtailPageInterface(PageInterface): - pass +class WagtailPageObjectType(graphene.ObjectType): + class Meta: + interfaces = (URLPathTargetWagtailInterface,) + name = "WagtailPage" diff --git a/baseapp_wagtail/base/models.py b/baseapp_wagtail/base/models.py index 66193a45..57bee091 100644 --- a/baseapp_wagtail/base/models.py +++ b/baseapp_wagtail/base/models.py @@ -11,6 +11,7 @@ from baseapp_comments.models import CommentableModel from baseapp_core.graphql.models import RelayModel +from baseapp_pages.urlpath_interfaces import URLPathTargetMixin from baseapp_reactions.models import ReactableModel from baseapp_reports.models import ReportableModel @@ -46,7 +47,7 @@ class Meta: abstract = True -class DefaultPageModel(HeadlessPageMixin, Page, metaclass=HeadlessPageBase): +class DefaultPageModel(URLPathTargetMixin, HeadlessPageMixin, Page, metaclass=HeadlessPageBase): featured_image = FeaturedImageStreamField.create() body = None @@ -65,13 +66,13 @@ class DefaultPageModel(HeadlessPageMixin, Page, metaclass=HeadlessPageBase): index.AutocompleteField("body"), ] - class Meta: - abstract = True - graphql_fields = [ GraphQLStreamfield("featured_image"), ] + class Meta: + abstract = True + class BaseStandardPage( DefaultPageModel, CommentableModel, ReactableModel, ReportableModel, RelayModel @@ -91,8 +92,8 @@ class Meta: ] graphql_interfaces = [ - "baseapp_wagtail.base.graphql.object_types.WagtailCommentsInterface", - "baseapp_wagtail.base.graphql.object_types.WagtailReactionsInterface", - "baseapp_wagtail.base.graphql.object_types.WagtailNotificationsInterfaceInterface", - "baseapp_wagtail.base.graphql.object_types.WagtailReportsInterfaceInterface", + "baseapp_wagtail.base.graphql.interfaces.WagtailCommentsInterface", + "baseapp_wagtail.base.graphql.interfaces.WagtailReactionsInterface", + "baseapp_wagtail.base.graphql.interfaces.WagtailNotificationsInterfaceInterface", + "baseapp_wagtail.base.graphql.interfaces.WagtailReportsInterfaceInterface", ] diff --git a/baseapp_wagtail/base/urlpath/__init__.py b/baseapp_wagtail/base/urlpath/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/baseapp_wagtail/base/urlpath/urlpath_sync.py b/baseapp_wagtail/base/urlpath/urlpath_sync.py new file mode 100644 index 00000000..39642442 --- /dev/null +++ b/baseapp_wagtail/base/urlpath/urlpath_sync.py @@ -0,0 +1,105 @@ +from typing import Optional + +from django.db.models import Model +from wagtail.models import Page + +from baseapp_pages.urlpath_interfaces import URLPathTargetMixin + + +class WagtailURLPathSync: + page: Page + urlpath_model: Optional[Model] + + def __init__(self, page: Page): + self.page = page + self.urlpath_model = None + self._load_urlpath_model() + + def _load_urlpath_model(self): + try: + from baseapp_pages.models import URLPath + + self.urlpath_model = URLPath + except ImportError: + self.urlpath_model = None + + def create_urlpath(self): + if not self._can_sync(): + return + + wagtail_path = self._get_wagtail_path() + if not wagtail_path: + return + + if self.urlpath_model.objects.filter(path=wagtail_path).exists(): + wagtail_path = self._generate_unique_path(wagtail_path) + + try: + # Use the mixin method + self.page.create_url_path( + path=wagtail_path, language=self.page.locale.language_code, is_active=self.page.live + ) + except Exception: + return + + def deactivate_urlpath(self): + if not self._can_sync(): + return + + try: + # Use the mixin method + self.page.deactivate_url_paths() + except Exception: + return + + def update_urlpath(self): + if not self._can_sync(): + return + + wagtail_path = self._get_wagtail_path() + if not wagtail_path: + return + + try: + # Use the mixin method + self.page.update_url_path( + path=wagtail_path, language=self.page.locale.language_code, is_active=self.page.live + ) + except Exception: + return + + def delete_urlpath(self): + if not self._can_sync(): + return + + try: + # Use the mixin method + self.page.delete_url_paths() + except Exception: + return + + def _can_sync(self) -> bool: + return self._is_available() and self._is_urlpath_target() + + def _is_urlpath_target(self) -> bool: + return isinstance(self.page, URLPathTargetMixin) + + def _is_available(self) -> bool: + return self.urlpath_model is not None + + def _get_wagtail_path(self) -> Optional[str]: + url_parts = self.page.get_url_parts() + if not url_parts: + return None + _, _, page_path = url_parts + return page_path + + def _generate_unique_path(self, base_path: str) -> str: + counter = 1 + new_path = base_path + + while self.urlpath_model.objects.filter(path=new_path).exists(): + new_path = f"{base_path}-{counter}" + counter += 1 + + return new_path diff --git a/baseapp_wagtail/base/wagtail_hooks.py b/baseapp_wagtail/base/wagtail_hooks.py index 6bfc1375..2c4fc934 100644 --- a/baseapp_wagtail/base/wagtail_hooks.py +++ b/baseapp_wagtail/base/wagtail_hooks.py @@ -2,6 +2,8 @@ from wagtail import hooks from wagtail.rich_text import LinkHandler +from baseapp_wagtail.base.urlpath.urlpath_sync import WagtailURLPathSync + @hooks.register("register_rich_text_features") def register_core_features(features): @@ -34,3 +36,23 @@ class EmailLinkHandler(LinkHandler): def expand_db_attributes(cls, attrs): href = attrs["href"] return '' % escape(href) + + +@hooks.register("after_create_page") +def create_urlpath_for_page(request, page): + WagtailURLPathSync(page).create_urlpath() + + +@hooks.register("after_publish_page") +def update_urlpath_on_publish(request, page): + WagtailURLPathSync(page).update_urlpath() + + +@hooks.register("after_unpublish_page") +def deactivate_urlpath_on_unpublish(request, page): + WagtailURLPathSync(page).deactivate_urlpath() + + +@hooks.register("after_delete_page") +def delete_urlpath_on_delete(request, page): + WagtailURLPathSync(page).delete_urlpath() diff --git a/baseapp_wagtail/settings.py b/baseapp_wagtail/settings.py index dd1dfbd5..09df7613 100644 --- a/baseapp_wagtail/settings.py +++ b/baseapp_wagtail/settings.py @@ -71,7 +71,7 @@ "baseapp_wagtail_base", "baseapp_wagtail_medias", ], - "PAGE_INTERFACE": "baseapp_wagtail.base.graphql.object_types.WagtailPageInterface", + "PAGE_INTERFACE": "baseapp_wagtail.base.graphql.interfaces.WagtailPageInterface", } WAGTAILIMAGES_IMAGE_MODEL = "baseapp_wagtail_medias.CustomImage" diff --git a/testproject/settings.py b/testproject/settings.py index af4fd60a..4b66f6cb 100644 --- a/testproject/settings.py +++ b/testproject/settings.py @@ -48,8 +48,8 @@ "testproject.profiles", "testproject.base", "testproject.e2e", - *WAGTAIL_INSTALLED_INTERNAL_APPS, *WAGTAIL_INSTALLED_APPS, + *WAGTAIL_INSTALLED_INTERNAL_APPS, "baseapp_wagtail.tests", "baseapp_pdf", ] From 6219b0e41a65e133b18e3fb66173579a478e2db6 Mon Sep 17 00:00:00 2001 From: Hercilio Ortiz Date: Wed, 13 Aug 2025 11:26:35 -0300 Subject: [PATCH 02/18] Update: Adding Wagtail object type compatible with PageInterface --- baseapp_pages/graphql/object_types.py | 30 +++--------- baseapp_pages/models.py | 34 +++++++++++++ baseapp_pages/urlpath_interfaces.py | 51 -------------------- baseapp_wagtail/base/apps.py | 9 ---- baseapp_wagtail/base/graphql/__init__.py | 0 baseapp_wagtail/base/graphql/object_types.py | 22 +++++++-- baseapp_wagtail/base/models.py | 14 ++++-- baseapp_wagtail/base/urlpath/urlpath_sync.py | 6 +-- testproject/graphql.py | 7 ++- 9 files changed, 77 insertions(+), 96 deletions(-) delete mode 100644 baseapp_pages/urlpath_interfaces.py create mode 100644 baseapp_wagtail/base/graphql/__init__.py diff --git a/baseapp_pages/graphql/object_types.py b/baseapp_pages/graphql/object_types.py index b3fad413..46f25ed8 100644 --- a/baseapp_pages/graphql/object_types.py +++ b/baseapp_pages/graphql/object_types.py @@ -11,7 +11,6 @@ from baseapp_comments.graphql.object_types import CommentsInterface from baseapp_core.graphql import DjangoObjectType, LanguagesEnum, ThumbnailField from baseapp_pages.models import AbstractPage, Metadata, URLPath -from baseapp_pages.urlpath_registry import urlpath_registry from ..meta import AbstractMetadataObjectType @@ -25,6 +24,11 @@ class PageInterface(relay.Node): url_paths = graphene.List(lambda: URLPathNode) metadata = graphene.Field(lambda: MetadataObjectType) + def resolve_type(self, info): + if hasattr(self, "get_graphql_object_type"): + return self.get_graphql_object_type() + return None + @classmethod def resolve_url_path(cls, instance, info, **kwargs): return instance.url_path @@ -139,30 +143,8 @@ def is_type_of(cls, root, info): return super().is_type_of(root, info) -class PagesPageObjectType(graphene.ObjectType): - class Meta: - interfaces = (UrlPathTargetInterface,) - name = "PagesPage" - - -class TargetAvailableTypes(graphene.Union): - class Meta: - types = (PagesPageObjectType, *urlpath_registry.get_all_types()) - - @classmethod - def resolve_type(cls, instance, info): - if isinstance(instance, AbstractPage): - return PageObjectType - - graphql_type = urlpath_registry.get_type(instance) - if graphql_type: - return graphql_type - - return None - - class URLPathNode(DjangoObjectType): - target = graphene.Field(TargetAvailableTypes) + target = graphene.Field(PageInterface) language = graphene.Field(LanguagesEnum) class Meta: diff --git a/baseapp_pages/models.py b/baseapp_pages/models.py index 8079612a..be7f2b0a 100644 --- a/baseapp_pages/models.py +++ b/baseapp_pages/models.py @@ -1,6 +1,9 @@ +from typing import Optional, Type + import pghistory import swapper from django.conf import settings +from django.contrib.auth.models import AbstractUser from django.contrib.contenttypes.fields import GenericForeignKey, GenericRelation from django.contrib.contenttypes.models import ContentType from django.db import models @@ -104,6 +107,37 @@ def url_path(self): Q(is_active=True), Q(language=get_language()) | Q(language__isnull=True) ).first() + def get_graphql_object_type(self) -> Type: + raise NotImplementedError + + def get_permission_check(self, user: AbstractUser) -> bool: + return True + + def create_url_path(self, path: str, language: Optional[str] = None, is_active: bool = True): + if not self.pk: + raise ValueError("Save the instance before creating URL paths.") + return self.url_paths.create(path=path, language=language, is_active=is_active) + + def update_url_path(self, path: str, language: Optional[str] = None, is_active: bool = True): + if not self.pk: + raise ValueError("Save the instance before updating URL paths.") + primary_path = self.url_path + if primary_path: + primary_path.path = path + primary_path.language = language + primary_path.is_active = is_active + primary_path.save() + else: + self.create_url_path(path, language, is_active) + + def deactivate_url_paths(self): + if self.pk: + self.url_paths.update(is_active=False) + + def delete_url_paths(self): + if self.pk: + self.url_paths.all().delete() + class AbstractPage(PageMixin, TimeStampedModel, RelayModel, CommentableModel): user = models.ForeignKey( diff --git a/baseapp_pages/urlpath_interfaces.py b/baseapp_pages/urlpath_interfaces.py deleted file mode 100644 index 5a445406..00000000 --- a/baseapp_pages/urlpath_interfaces.py +++ /dev/null @@ -1,51 +0,0 @@ -from typing import Optional, Type - -from django.contrib.auth.models import AbstractUser -from django.contrib.contenttypes.fields import GenericRelation -from django.db import models - -from baseapp_pages.models import URLPath - - -class URLPathTargetMixin(models.Model): - """Mixin for models that want to be URLPath targets""" - - url_paths = GenericRelation( - URLPath, - content_type_field="target_content_type", - object_id_field="target_object_id", - ) - - class Meta: - abstract = True - - def get_graphql_object_type(self) -> Type: - raise NotImplementedError - - def get_permission_check(self, user: AbstractUser) -> bool: - return True - - def create_url_path(self, path: str, language: Optional[str] = None, is_active: bool = True): - if not self.pk: - raise ValueError("Save the instance before creating URL paths.") - return self.url_paths.create(path=path, language=language, is_active=is_active) - - def update_url_path(self, path: str, language: Optional[str] = None, is_active: bool = True): - if not self.pk: - raise ValueError("Save the instance before updating URL paths.") - primary_path = self.url_path - if primary_path: - primary_path.path = path - primary_path.language = language - primary_path.is_active = is_active - primary_path.save() - else: - self.create_url_path(path, language, is_active) - - def deactivate_url_paths(self): - if self.pk: - self.url_paths.update(is_active=False) - - def delete_url_paths(self): - if self.pk: - self.url_paths.all().delete() diff --git a/baseapp_wagtail/base/apps.py b/baseapp_wagtail/base/apps.py index e6aaad2b..b104f017 100644 --- a/baseapp_wagtail/base/apps.py +++ b/baseapp_wagtail/base/apps.py @@ -5,12 +5,3 @@ class WagtailConfig(AppConfig): name = "baseapp_wagtail.base" verbose_name = "BaseApp Wagtail - Base" label = "baseapp_wagtail_base" - - def ready(self): - from grapple.registry import registry - - from baseapp_pages.urlpath_registry import urlpath_registry - from baseapp_wagtail.base.graphql.object_types import WagtailPageObjectType - - for model_type in registry.pages.keys(): - urlpath_registry.register_type(model_type, WagtailPageObjectType) diff --git a/baseapp_wagtail/base/graphql/__init__.py b/baseapp_wagtail/base/graphql/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/baseapp_wagtail/base/graphql/object_types.py b/baseapp_wagtail/base/graphql/object_types.py index 0a9bf6c5..9fe04355 100644 --- a/baseapp_wagtail/base/graphql/object_types.py +++ b/baseapp_wagtail/base/graphql/object_types.py @@ -1,9 +1,25 @@ import graphene +from graphene import relay -from baseapp_wagtail.base.graphql.interfaces import URLPathTargetWagtailInterface +from baseapp_pages.graphql import PageInterface +from baseapp_wagtail.base.graphql.interfaces import WagtailPageInterface -class WagtailPageObjectType(graphene.ObjectType): +class WagtailURLPathObjectType(graphene.ObjectType): + data = graphene.Field(WagtailPageInterface) + class Meta: - interfaces = (URLPathTargetWagtailInterface,) + interfaces = ( + relay.Node, + PageInterface, + ) name = "WagtailPage" + + @classmethod + def resolve_type(cls, instance, info): + return cls + + def resolve_data(self, info): + return self + + # TODO: (BA-2636) Solve the metadata interface. diff --git a/baseapp_wagtail/base/models.py b/baseapp_wagtail/base/models.py index 57bee091..53d3c864 100644 --- a/baseapp_wagtail/base/models.py +++ b/baseapp_wagtail/base/models.py @@ -11,7 +11,7 @@ from baseapp_comments.models import CommentableModel from baseapp_core.graphql.models import RelayModel -from baseapp_pages.urlpath_interfaces import URLPathTargetMixin +from baseapp_pages.models import PageMixin from baseapp_reactions.models import ReactableModel from baseapp_reports.models import ReportableModel @@ -47,7 +47,7 @@ class Meta: abstract = True -class DefaultPageModel(URLPathTargetMixin, HeadlessPageMixin, Page, metaclass=HeadlessPageBase): +class DefaultPageModel(HeadlessPageMixin, Page, PageMixin, RelayModel, metaclass=HeadlessPageBase): featured_image = FeaturedImageStreamField.create() body = None @@ -70,13 +70,17 @@ class DefaultPageModel(URLPathTargetMixin, HeadlessPageMixin, Page, metaclass=He GraphQLStreamfield("featured_image"), ] + @classmethod + def get_graphql_object_type(cls): + from baseapp_wagtail.base.graphql.object_types import WagtailURLPathObjectType + + return WagtailURLPathObjectType + class Meta: abstract = True -class BaseStandardPage( - DefaultPageModel, CommentableModel, ReactableModel, ReportableModel, RelayModel -): +class BaseStandardPage(DefaultPageModel, CommentableModel, ReactableModel, ReportableModel): body = PageBodyStreamField.create( StandardPageStreamBlock(required=False), ) diff --git a/baseapp_wagtail/base/urlpath/urlpath_sync.py b/baseapp_wagtail/base/urlpath/urlpath_sync.py index 39642442..372823e3 100644 --- a/baseapp_wagtail/base/urlpath/urlpath_sync.py +++ b/baseapp_wagtail/base/urlpath/urlpath_sync.py @@ -3,8 +3,6 @@ from django.db.models import Model from wagtail.models import Page -from baseapp_pages.urlpath_interfaces import URLPathTargetMixin - class WagtailURLPathSync: page: Page @@ -82,7 +80,9 @@ def _can_sync(self) -> bool: return self._is_available() and self._is_urlpath_target() def _is_urlpath_target(self) -> bool: - return isinstance(self.page, URLPathTargetMixin) + from baseapp_pages.models import PageMixin + + return isinstance(self.page, PageMixin) def _is_available(self) -> bool: return self.urlpath_model is not None diff --git a/testproject/graphql.py b/testproject/graphql.py index 70973d57..870d5d50 100644 --- a/testproject/graphql.py +++ b/testproject/graphql.py @@ -31,6 +31,7 @@ from baseapp_reports.graphql.mutations import ReportsMutations from baseapp_reports.graphql.queries import ReportsQueries from baseapp_wagtail.base.graphql.mutations import WagtailMutation +from baseapp_wagtail.base.graphql.object_types import WagtailURLPathObjectType from baseapp_wagtail.base.graphql.queries import WagtailQuery from baseapp_wagtail.base.graphql.subscriptions import WagtailSubscription from testproject.users.graphql.queries import UsersQueries @@ -85,5 +86,9 @@ class Subscription( schema = graphene.Schema( - query=Query, mutation=Mutation, subscription=Subscription, types=list(registry.models.values()) + query=Query, + mutation=Mutation, + subscription=Subscription, + # TODO: (BA-2636) Unify this inside of the wagtail package. + types=list(registry.models.values()) + [WagtailURLPathObjectType], ) From 385dce8eabf66e876f129fa753894960bcbc00e7 Mon Sep 17 00:00:00 2001 From: Hercilio Ortiz Date: Wed, 13 Aug 2025 11:35:45 -0300 Subject: [PATCH 03/18] Update adding smail rule for formatting urls --- baseapp_wagtail/base/urlpath/urlpath_sync.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/baseapp_wagtail/base/urlpath/urlpath_sync.py b/baseapp_wagtail/base/urlpath/urlpath_sync.py index 372823e3..a2d28717 100644 --- a/baseapp_wagtail/base/urlpath/urlpath_sync.py +++ b/baseapp_wagtail/base/urlpath/urlpath_sync.py @@ -92,6 +92,9 @@ def _get_wagtail_path(self) -> Optional[str]: if not url_parts: return None _, _, page_path = url_parts + # TODO: (BA-2636) Add this "url formatter" in the baseapp_pages package. + if page_path and page_path.endswith("/"): + page_path = page_path[:-1] return page_path def _generate_unique_path(self, base_path: str) -> str: From 16516a62787325bc16b5180d936bbd01817f8cb1 Mon Sep 17 00:00:00 2001 From: Hercilio Ortiz Date: Wed, 13 Aug 2025 11:51:47 -0300 Subject: [PATCH 04/18] Update: Adding metadata to Wagtail url path object type --- baseapp_wagtail/base/graphql/object_types.py | 24 +++++++++++++++++++- baseapp_wagtail/base/urlpath/urlpath_sync.py | 1 + 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/baseapp_wagtail/base/graphql/object_types.py b/baseapp_wagtail/base/graphql/object_types.py index 9fe04355..76dabf43 100644 --- a/baseapp_wagtail/base/graphql/object_types.py +++ b/baseapp_wagtail/base/graphql/object_types.py @@ -2,6 +2,7 @@ from graphene import relay from baseapp_pages.graphql import PageInterface +from baseapp_pages.graphql.object_types import AbstractMetadataObjectType from baseapp_wagtail.base.graphql.interfaces import WagtailPageInterface @@ -22,4 +23,25 @@ def resolve_type(cls, instance, info): def resolve_data(self, info): return self - # TODO: (BA-2636) Solve the metadata interface. + @classmethod + def resolve_metadata(cls, instance, info, **kwargs): + # TODO: (BA-2636) Review metadata for Wagtail pages. + return WagtailMetadata(instance) + + +class WagtailMetadata(AbstractMetadataObjectType): + @property + def meta_title(self): + return self.instance.title + + @property + def meta_description(self): + return None + + @property + def meta_og_type(self): + return "wagtail" + + @property + def meta_og_image(self): + return None diff --git a/baseapp_wagtail/base/urlpath/urlpath_sync.py b/baseapp_wagtail/base/urlpath/urlpath_sync.py index a2d28717..34e04092 100644 --- a/baseapp_wagtail/base/urlpath/urlpath_sync.py +++ b/baseapp_wagtail/base/urlpath/urlpath_sync.py @@ -11,6 +11,7 @@ class WagtailURLPathSync: def __init__(self, page: Page): self.page = page self.urlpath_model = None + # TODO: (BA-2636) Home page is failing. self._load_urlpath_model() def _load_urlpath_model(self): From 3d9b04425eddb65b78e593ea44a926826268e94f Mon Sep 17 00:00:00 2001 From: Hercilio Ortiz Date: Wed, 13 Aug 2025 11:53:49 -0300 Subject: [PATCH 05/18] Bugfix: Small adjustiment in object_types --- baseapp_wagtail/base/graphql/object_types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/baseapp_wagtail/base/graphql/object_types.py b/baseapp_wagtail/base/graphql/object_types.py index 76dabf43..96824624 100644 --- a/baseapp_wagtail/base/graphql/object_types.py +++ b/baseapp_wagtail/base/graphql/object_types.py @@ -26,7 +26,7 @@ def resolve_data(self, info): @classmethod def resolve_metadata(cls, instance, info, **kwargs): # TODO: (BA-2636) Review metadata for Wagtail pages. - return WagtailMetadata(instance) + return WagtailMetadata(instance, info) class WagtailMetadata(AbstractMetadataObjectType): From a6f3f3a1ee9a3d1ca06ef03b06f75c016e6089b0 Mon Sep 17 00:00:00 2001 From: Hercilio Ortiz Date: Wed, 13 Aug 2025 13:59:32 -0300 Subject: [PATCH 06/18] Update: Fixing meta_og_type --- baseapp_wagtail/base/graphql/object_types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/baseapp_wagtail/base/graphql/object_types.py b/baseapp_wagtail/base/graphql/object_types.py index 96824624..3565b9b2 100644 --- a/baseapp_wagtail/base/graphql/object_types.py +++ b/baseapp_wagtail/base/graphql/object_types.py @@ -40,7 +40,7 @@ def meta_description(self): @property def meta_og_type(self): - return "wagtail" + return "article" @property def meta_og_image(self): From 72ed1d0d44b60091a29373509e0ba019d6ed1c0a Mon Sep 17 00:00:00 2001 From: Hercilio Ortiz Date: Wed, 13 Aug 2025 21:48:46 -0300 Subject: [PATCH 07/18] Update: Adding checkers before coupling with other packages --- baseapp_wagtail/base/graphql/interfaces.py | 62 +++++++------------ baseapp_wagtail/base/graphql/object_types.py | 42 ++++++++----- baseapp_wagtail/base/models.py | 44 ++++++++++--- ...ndardpage_is_reactions_enabled_and_more.py | 25 ++++++++ 4 files changed, 107 insertions(+), 66 deletions(-) create mode 100644 testproject/base/migrations/0005_remove_standardpage_is_reactions_enabled_and_more.py diff --git a/baseapp_wagtail/base/graphql/interfaces.py b/baseapp_wagtail/base/graphql/interfaces.py index bf9ca4cb..e25acb17 100644 --- a/baseapp_wagtail/base/graphql/interfaces.py +++ b/baseapp_wagtail/base/graphql/interfaces.py @@ -1,53 +1,37 @@ import graphene +from django.apps import apps from grapple.types.interfaces import PageInterface -from baseapp_comments.graphql.object_types import CommentsInterface -from baseapp_notifications.graphql.object_types import NotificationsInterface -from baseapp_reactions.graphql.object_types import ReactionsInterface -from baseapp_reports.graphql.object_types import ReportsInterface +from baseapp_core.graphql.models import RelayModel -# TODO: Fix in next story -# https://app.approvd.io/silverlogic/BA/stories/36399 -# As per @ap, these interfaces shouldn't be necessary. They are currently necessary because -# the baseapp interfaces classes inherit from RelayNode, but they shouldn't! -# -# Also if we return -1 for resolve_id, it means we are not Relay compliant. This can cause bugs. +if apps.is_installed("baseapp_comments"): + from baseapp_comments.graphql.object_types import CommentsInterface + class WagtailCommentsInterface(CommentsInterface): + """ + Wagtail-specific comments interface for Wagtail page types that do not support + relay.Node. -class WagtailCommentsInterface(CommentsInterface): - id = graphene.ID() + Extends baseapp_comments.graphql.CommentsInterface to enable comments on Wagtail + pages with dynamic or non-relay-compliant id fields, avoiding relay.Node conflicts. - def resolve_id(self, info, **kwargs): - return str(self.id) if self.id is not None else -1 + If needed, an intermediate object type could be used to connect CommentsInterface. + @see baseapp_wagtail.base.graphql.object_types.WagtailURLPathObjectType + """ -class WagtailReactionsInterface(ReactionsInterface): - id = graphene.ID() + id = graphene.ID() - def resolve_id(self, info, **kwargs): - return str(self.id) if self.id is not None else -1 - - -class WagtailNotificationsInterfaceInterface(NotificationsInterface): - id = graphene.ID() - - def resolve_id(self, info, **kwargs): - return str(self.id) if self.id is not None else -1 - - -class WagtailReportsInterfaceInterface(ReportsInterface): - id = graphene.ID() - - def resolve_id(self, info, **kwargs): - return str(self.id) if self.id is not None else -1 + def resolve_id(self, info, **kwargs): + if isinstance(self, RelayModel): + return self.relay_id + raise ValueError("WagtailCommentsInterface can only be used with RelayModel instances.") class WagtailPageInterface(PageInterface): - pass - + """ + Wagtail-specific page interface that extends Grapple's PageInterface to avoid + conflicts with the baseapp_pages.graphql.PageInterface. + """ -class URLPathTargetWagtailInterface(graphene.Interface): - data = graphene.Field(WagtailPageInterface) - - def resolve_data(self, info): - return self + pass diff --git a/baseapp_wagtail/base/graphql/object_types.py b/baseapp_wagtail/base/graphql/object_types.py index 3565b9b2..4077a529 100644 --- a/baseapp_wagtail/base/graphql/object_types.py +++ b/baseapp_wagtail/base/graphql/object_types.py @@ -1,10 +1,15 @@ import graphene +from django.apps import apps from graphene import relay -from baseapp_pages.graphql import PageInterface -from baseapp_pages.graphql.object_types import AbstractMetadataObjectType from baseapp_wagtail.base.graphql.interfaces import WagtailPageInterface +wagtail_url_path_object_type_interfaces = [] +if apps.is_installed("baseapp_pages"): + from baseapp_pages.graphql import PageInterface + + wagtail_url_path_object_type_interfaces.append(PageInterface) + class WagtailURLPathObjectType(graphene.ObjectType): data = graphene.Field(WagtailPageInterface) @@ -12,7 +17,7 @@ class WagtailURLPathObjectType(graphene.ObjectType): class Meta: interfaces = ( relay.Node, - PageInterface, + *wagtail_url_path_object_type_interfaces, ) name = "WagtailPage" @@ -25,23 +30,26 @@ def resolve_data(self, info): @classmethod def resolve_metadata(cls, instance, info, **kwargs): - # TODO: (BA-2636) Review metadata for Wagtail pages. - return WagtailMetadata(instance, info) + if apps.is_installed("baseapp_pages"): + from baseapp_pages.graphql.object_types import AbstractMetadataObjectType + class WagtailMetadata(AbstractMetadataObjectType): + # TODO: (BA-2635) Complete the metadata for Wagtail pages when implementing the story BA-2635. + @property + def meta_title(self): + return self.instance.title -class WagtailMetadata(AbstractMetadataObjectType): - @property - def meta_title(self): - return self.instance.title + @property + def meta_description(self): + return None - @property - def meta_description(self): - return None + @property + def meta_og_type(self): + return "article" - @property - def meta_og_type(self): - return "article" + @property + def meta_og_image(self): + return None - @property - def meta_og_image(self): + return WagtailMetadata(instance, info) return None diff --git a/baseapp_wagtail/base/models.py b/baseapp_wagtail/base/models.py index 53d3c864..76f0cb10 100644 --- a/baseapp_wagtail/base/models.py +++ b/baseapp_wagtail/base/models.py @@ -1,5 +1,6 @@ from urllib.parse import urlparse +from django.apps import apps from django.conf import settings from django.utils.translation import gettext as _ from grapple.models import GraphQLStreamfield @@ -9,11 +10,7 @@ from wagtail.search import index from wagtail_headless_preview.models import HeadlessPreviewMixin -from baseapp_comments.models import CommentableModel from baseapp_core.graphql.models import RelayModel -from baseapp_pages.models import PageMixin -from baseapp_reactions.models import ReactableModel -from baseapp_reports.models import ReportableModel from .stream_fields import ( FeaturedImageStreamField, @@ -47,7 +44,19 @@ class Meta: abstract = True -class DefaultPageModel(HeadlessPageMixin, Page, PageMixin, RelayModel, metaclass=HeadlessPageBase): +default_page_model_inheritances = [] + +if apps.is_installed("baseapp_pages"): + from baseapp_pages.models import PageMixin + + default_page_model_inheritances.append(PageMixin) + +default_page_model_inheritances.append(RelayModel) + + +class DefaultPageModel( + HeadlessPageMixin, Page, *default_page_model_inheritances, metaclass=HeadlessPageBase +): featured_image = FeaturedImageStreamField.create() body = None @@ -70,6 +79,8 @@ class DefaultPageModel(HeadlessPageMixin, Page, PageMixin, RelayModel, metaclass GraphQLStreamfield("featured_image"), ] + graphql_interfaces = [] + @classmethod def get_graphql_object_type(cls): from baseapp_wagtail.base.graphql.object_types import WagtailURLPathObjectType @@ -80,7 +91,23 @@ class Meta: abstract = True -class BaseStandardPage(DefaultPageModel, CommentableModel, ReactableModel, ReportableModel): +base_standard_page_model_inheritances = [] + +if apps.is_installed("baseapp_comments"): + from baseapp_comments.models import CommentableModel + + base_standard_page_model_inheritances.append(CommentableModel) + + +base_standard_page_model_graphql_interfaces = [] + +if apps.is_installed("baseapp_comments"): + base_standard_page_model_graphql_interfaces.append( + "baseapp_wagtail.base.graphql.interfaces.WagtailCommentsInterface", + ) + + +class BaseStandardPage(DefaultPageModel, *base_standard_page_model_inheritances): body = PageBodyStreamField.create( StandardPageStreamBlock(required=False), ) @@ -96,8 +123,5 @@ class Meta: ] graphql_interfaces = [ - "baseapp_wagtail.base.graphql.interfaces.WagtailCommentsInterface", - "baseapp_wagtail.base.graphql.interfaces.WagtailReactionsInterface", - "baseapp_wagtail.base.graphql.interfaces.WagtailNotificationsInterfaceInterface", - "baseapp_wagtail.base.graphql.interfaces.WagtailReportsInterfaceInterface", + *base_standard_page_model_graphql_interfaces, ] diff --git a/testproject/base/migrations/0005_remove_standardpage_is_reactions_enabled_and_more.py b/testproject/base/migrations/0005_remove_standardpage_is_reactions_enabled_and_more.py new file mode 100644 index 00000000..fe641a53 --- /dev/null +++ b/testproject/base/migrations/0005_remove_standardpage_is_reactions_enabled_and_more.py @@ -0,0 +1,25 @@ +# Generated by Django 5.1.11 on 2025-08-14 00:40 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("base", "0004_alter_standardpage_featured_image"), + ] + + operations = [ + migrations.RemoveField( + model_name="standardpage", + name="is_reactions_enabled", + ), + migrations.RemoveField( + model_name="standardpage", + name="reactions_count", + ), + migrations.RemoveField( + model_name="standardpage", + name="reports_count", + ), + ] From 808c81c42bbf9fb92a7a87b9d4b8e69ee6b81b25 Mon Sep 17 00:00:00 2001 From: Hercilio Ortiz Date: Thu, 14 Aug 2025 09:04:09 -0300 Subject: [PATCH 08/18] Update: Solving TODOs --- baseapp_pages/graphql/__init__.py | 2 - baseapp_pages/models.py | 7 ++- baseapp_pages/utils/__init__.py | 0 baseapp_pages/utils/url_path_formater.py | 53 ++++++++++++++++++++ baseapp_wagtail/base/graphql/object_types.py | 7 +++ baseapp_wagtail/base/models.py | 29 +++++++++++ baseapp_wagtail/base/urlpath/urlpath_sync.py | 35 +++++++------ baseapp_wagtail/base/wagtail_hooks.py | 50 +++++++++--------- testproject/graphql.py | 11 ++-- 9 files changed, 145 insertions(+), 49 deletions(-) create mode 100644 baseapp_pages/utils/__init__.py create mode 100644 baseapp_pages/utils/url_path_formater.py diff --git a/baseapp_pages/graphql/__init__.py b/baseapp_pages/graphql/__init__.py index 8b77eb97..dd93305c 100644 --- a/baseapp_pages/graphql/__init__.py +++ b/baseapp_pages/graphql/__init__.py @@ -1,6 +1,4 @@ from .mutations import PageCreate, PageEdit # noqa - -# TODO: (ho) review this file. from .object_types import ( # noqa MetadataObjectType, PageInterface, diff --git a/baseapp_pages/models.py b/baseapp_pages/models.py index be7f2b0a..0b4ab807 100644 --- a/baseapp_pages/models.py +++ b/baseapp_pages/models.py @@ -17,6 +17,7 @@ from baseapp_comments.models import CommentableModel from baseapp_core.graphql.models import RelayModel from baseapp_core.models import random_name_in +from baseapp_pages.utils.url_path_formater import URLPathFormater class URLPath(TimeStampedModel, RelayModel): @@ -116,14 +117,16 @@ def get_permission_check(self, user: AbstractUser) -> bool: def create_url_path(self, path: str, language: Optional[str] = None, is_active: bool = True): if not self.pk: raise ValueError("Save the instance before creating URL paths.") - return self.url_paths.create(path=path, language=language, is_active=is_active) + return self.url_paths.create( + path=URLPathFormater(path)(), language=language, is_active=is_active + ) def update_url_path(self, path: str, language: Optional[str] = None, is_active: bool = True): if not self.pk: raise ValueError("Save the instance before updating URL paths.") primary_path = self.url_path if primary_path: - primary_path.path = path + primary_path.path = URLPathFormater(path)() primary_path.language = language primary_path.is_active = is_active primary_path.save() diff --git a/baseapp_pages/utils/__init__.py b/baseapp_pages/utils/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/baseapp_pages/utils/url_path_formater.py b/baseapp_pages/utils/url_path_formater.py new file mode 100644 index 00000000..5ef1a4c0 --- /dev/null +++ b/baseapp_pages/utils/url_path_formater.py @@ -0,0 +1,53 @@ +import re + + +class URLPathFormater: + """ + Ensures the path is valid: + - Always starts with a single slash + - Never ends with a slash (unless it's just '/') + - Never has double slashes + - Is at least '/' + Raises ValueError if the format is not compatible. + """ + + path: str + + def __init__(self, path: str): + self.path = path + + def __call__(self) -> str: + return self.format() + + def format(self) -> str: + self._clean_path() + self._replace_multiple_slashes() + self._ensure_starts_with_slash() + self._remove_trailing_slash() + self._validate_path() + + return self.path + + def _clean_path(self) -> str: + self.path = self.path.strip() + if not self.path: + raise ValueError("Path cannot be empty.") + + def _replace_multiple_slashes(self) -> str: + self.path = re.sub(r"/+", "/", self.path) + + def _ensure_starts_with_slash(self) -> str: + if not self.path.startswith("/"): + self.path = "/" + self.path + + def _remove_trailing_slash(self) -> str: + if len(self.path) > 1 and self.path.endswith("/"): + self.path = self.path[:-1] + + def _validate_path(self) -> str: + if not self.path.startswith("/"): + raise ValueError("Path must start with a slash ('/').") + if "//" in self.path: + raise ValueError("Path must not contain double slashes ('//').") + if len(self.path) > 1 and self.path.endswith("/"): + raise ValueError("Path must not end with a slash unless it is root ('/').") diff --git a/baseapp_wagtail/base/graphql/object_types.py b/baseapp_wagtail/base/graphql/object_types.py index 4077a529..21252299 100644 --- a/baseapp_wagtail/base/graphql/object_types.py +++ b/baseapp_wagtail/base/graphql/object_types.py @@ -1,6 +1,7 @@ import graphene from django.apps import apps from graphene import relay +from grapple.registry import registry from baseapp_wagtail.base.graphql.interfaces import WagtailPageInterface @@ -53,3 +54,9 @@ def meta_og_image(self): return WagtailMetadata(instance, info) return None + + +BASEAPP_WAGTAIL_TYPES = [ + *registry.models.values(), + WagtailURLPathObjectType, +] diff --git a/baseapp_wagtail/base/models.py b/baseapp_wagtail/base/models.py index 76f0cb10..c6d10767 100644 --- a/baseapp_wagtail/base/models.py +++ b/baseapp_wagtail/base/models.py @@ -1,7 +1,10 @@ +from typing import Optional from urllib.parse import urlparse from django.apps import apps from django.conf import settings +from django.db.models import Q +from django.utils.translation import get_language from django.utils.translation import gettext as _ from grapple.models import GraphQLStreamfield from wagtail.admin.panels import FieldPanel @@ -81,6 +84,32 @@ class DefaultPageModel( graphql_interfaces = [] + @property + def pages_url_path(self): + """ + baseapp_pages.models.PageMixin.url_path alternative. + Defines a new property because wagtail pages are have have a defined "url_path" property. + """ + return self.url_paths.filter( + Q(is_active=True), Q(language=get_language()) | Q(language__isnull=True) + ).first() + + def update_url_path(self, path: str, language: Optional[str] = None, is_active: bool = True): + """ + Overrides the baseapp_pages.models.PageMixin.update_url_path method. + This is necessary in order to use the new "pages_url_path" property. + """ + from baseapp_pages.utils.url_path_formater import URLPathFormater + + primary_path = self.pages_url_path + if primary_path: + primary_path.path = URLPathFormater(path)() + primary_path.language = language + primary_path.is_active = is_active + primary_path.save() + else: + self.create_url_path(path, language, is_active) + @classmethod def get_graphql_object_type(cls): from baseapp_wagtail.base.graphql.object_types import WagtailURLPathObjectType diff --git a/baseapp_wagtail/base/urlpath/urlpath_sync.py b/baseapp_wagtail/base/urlpath/urlpath_sync.py index 34e04092..245fecf5 100644 --- a/baseapp_wagtail/base/urlpath/urlpath_sync.py +++ b/baseapp_wagtail/base/urlpath/urlpath_sync.py @@ -1,8 +1,12 @@ +import logging from typing import Optional +from django.apps import apps from django.db.models import Model from wagtail.models import Page +logger = logging.getLogger(__name__) + class WagtailURLPathSync: page: Page @@ -11,7 +15,6 @@ class WagtailURLPathSync: def __init__(self, page: Page): self.page = page self.urlpath_model = None - # TODO: (BA-2636) Home page is failing. self._load_urlpath_model() def _load_urlpath_model(self): @@ -38,7 +41,8 @@ def create_urlpath(self): self.page.create_url_path( path=wagtail_path, language=self.page.locale.language_code, is_active=self.page.live ) - except Exception: + except Exception as e: + logger.error(f"(Wagtail urlpath sync) Error creating urlpath: {e}") return def deactivate_urlpath(self): @@ -46,9 +50,9 @@ def deactivate_urlpath(self): return try: - # Use the mixin method self.page.deactivate_url_paths() - except Exception: + except Exception as e: + logger.error(f"(Wagtail urlpath sync) Error deactivating urlpath: {e}") return def update_urlpath(self): @@ -60,11 +64,11 @@ def update_urlpath(self): return try: - # Use the mixin method self.page.update_url_path( path=wagtail_path, language=self.page.locale.language_code, is_active=self.page.live ) - except Exception: + except Exception as e: + logger.error(f"(Wagtail urlpath sync) Error updating urlpath: {e}") return def delete_urlpath(self): @@ -72,30 +76,29 @@ def delete_urlpath(self): return try: - # Use the mixin method self.page.delete_url_paths() - except Exception: + except Exception as e: + logger.error(f"(Wagtail urlpath sync) Error deleting urlpath: {e}") return def _can_sync(self) -> bool: return self._is_available() and self._is_urlpath_target() - def _is_urlpath_target(self) -> bool: - from baseapp_pages.models import PageMixin - - return isinstance(self.page, PageMixin) - def _is_available(self) -> bool: return self.urlpath_model is not None + def _is_urlpath_target(self) -> bool: + if apps.is_installed("baseapp_pages"): + from baseapp_pages.models import PageMixin + + return isinstance(self.page, PageMixin) + return False + def _get_wagtail_path(self) -> Optional[str]: url_parts = self.page.get_url_parts() if not url_parts: return None _, _, page_path = url_parts - # TODO: (BA-2636) Add this "url formatter" in the baseapp_pages package. - if page_path and page_path.endswith("/"): - page_path = page_path[:-1] return page_path def _generate_unique_path(self, base_path: str) -> str: diff --git a/baseapp_wagtail/base/wagtail_hooks.py b/baseapp_wagtail/base/wagtail_hooks.py index 2c4fc934..252030ee 100644 --- a/baseapp_wagtail/base/wagtail_hooks.py +++ b/baseapp_wagtail/base/wagtail_hooks.py @@ -5,13 +5,6 @@ from baseapp_wagtail.base.urlpath.urlpath_sync import WagtailURLPathSync -@hooks.register("register_rich_text_features") -def register_core_features(features): - features.default_features.append("blockquote") - features.register_link_type(ExternalLinkHandler) - features.register_link_type(EmailLinkHandler) - - @hooks.register("register_schema_query") def register_schema_query(query_mixins): for query_mixin in query_mixins: @@ -20,24 +13,6 @@ def register_schema_query(query_mixins): query_mixins.remove(query_mixin) -class ExternalLinkHandler(LinkHandler): - identifier = "external" - - @classmethod - def expand_db_attributes(cls, attrs): - href = attrs["href"] - return '' % escape(href) - - -class EmailLinkHandler(LinkHandler): - identifier = "email" - - @classmethod - def expand_db_attributes(cls, attrs): - href = attrs["href"] - return '' % escape(href) - - @hooks.register("after_create_page") def create_urlpath_for_page(request, page): WagtailURLPathSync(page).create_urlpath() @@ -56,3 +31,28 @@ def deactivate_urlpath_on_unpublish(request, page): @hooks.register("after_delete_page") def delete_urlpath_on_delete(request, page): WagtailURLPathSync(page).delete_urlpath() + + +@hooks.register("register_rich_text_features") +def register_core_features(features): + features.default_features.append("blockquote") + features.register_link_type(ExternalLinkHandler) + features.register_link_type(EmailLinkHandler) + + +class ExternalLinkHandler(LinkHandler): + identifier = "external" + + @classmethod + def expand_db_attributes(cls, attrs): + href = attrs["href"] + return '' % escape(href) + + +class EmailLinkHandler(LinkHandler): + identifier = "email" + + @classmethod + def expand_db_attributes(cls, attrs): + href = attrs["href"] + return '' % escape(href) diff --git a/testproject/graphql.py b/testproject/graphql.py index 870d5d50..b0efdb63 100644 --- a/testproject/graphql.py +++ b/testproject/graphql.py @@ -2,7 +2,6 @@ from graphene import relay from graphene.relay.node import NodeField as RelayNodeField from graphene_django.debug import DjangoDebug -from grapple.registry import registry from baseapp.activity_log.graphql.queries import ActivityLogQueries from baseapp.content_feed.graphql.mutations import ContentFeedMutations @@ -31,7 +30,7 @@ from baseapp_reports.graphql.mutations import ReportsMutations from baseapp_reports.graphql.queries import ReportsQueries from baseapp_wagtail.base.graphql.mutations import WagtailMutation -from baseapp_wagtail.base.graphql.object_types import WagtailURLPathObjectType +from baseapp_wagtail.base.graphql.object_types import BASEAPP_WAGTAIL_TYPES from baseapp_wagtail.base.graphql.queries import WagtailQuery from baseapp_wagtail.base.graphql.subscriptions import WagtailSubscription from testproject.users.graphql.queries import UsersQueries @@ -85,10 +84,14 @@ class Subscription( pass +schema_types = [ + *BASEAPP_WAGTAIL_TYPES, +] + + schema = graphene.Schema( query=Query, mutation=Mutation, subscription=Subscription, - # TODO: (BA-2636) Unify this inside of the wagtail package. - types=list(registry.models.values()) + [WagtailURLPathObjectType], + types=schema_types, ) From 4ad2815e536d5cb1946926ae9922c80e14a6994d Mon Sep 17 00:00:00 2001 From: Hercilio Ortiz Date: Thu, 14 Aug 2025 09:16:00 -0300 Subject: [PATCH 09/18] Cleaning some unneeded code --- baseapp_pages/graphql/object_types.py | 12 +------- baseapp_pages/urlpath_registry.py | 32 -------------------- baseapp_wagtail/base/urlpath/urlpath_sync.py | 16 +++++----- 3 files changed, 10 insertions(+), 50 deletions(-) delete mode 100644 baseapp_pages/urlpath_registry.py diff --git a/baseapp_pages/graphql/object_types.py b/baseapp_pages/graphql/object_types.py index 46f25ed8..909c38b9 100644 --- a/baseapp_pages/graphql/object_types.py +++ b/baseapp_pages/graphql/object_types.py @@ -46,13 +46,6 @@ def resolve_metadata(cls, instance, info, **kwargs): raise NotImplementedError -class UrlPathTargetInterface(graphene.Interface): - data = graphene.Field(PageInterface) - - def resolve_data(self, info): - return self - - class PageFilter(django_filters.FilterSet): class Meta: model = Page @@ -62,7 +55,7 @@ class Meta: class BasePageObjectType: metadata = graphene.Field(lambda: MetadataObjectType) status = graphene.Field(PageStatusEnum) - title = graphene.String(required=True) + title = graphene.String() body = graphene.String() class Meta: @@ -168,9 +161,6 @@ def resolve_target(self, info, **kwargs): if isinstance(self.target, AbstractPage): if not info.context.user.has_perm(f"{page_app_label}.view_page", self.target): return None - # For other targets, we rely on the registry to provide the correct GraphQL type - # Permission checking would need to be implemented in the foreign app's GraphQL types - return self.target @classmethod diff --git a/baseapp_pages/urlpath_registry.py b/baseapp_pages/urlpath_registry.py deleted file mode 100644 index c1c2a606..00000000 --- a/baseapp_pages/urlpath_registry.py +++ /dev/null @@ -1,32 +0,0 @@ -from typing import Dict, List, Type - -from django.contrib.contenttypes.models import ContentType -from django.db.models import Model - - -class URLPathRegistry: - def __init__(self): - self._graphql_types: Dict[str, Type] = {} - - def register_type(self, model_class: Type[Model], graphql_type: Type): - content_type = ContentType.objects.get_for_model(model_class) - key = f"{content_type.app_label}.{content_type.model}" - self._graphql_types[key] = graphql_type - - def get_type(self, instance: Model) -> Type: - content_type = ContentType.objects.get_for_model(instance) - key = f"{content_type.app_label}.{content_type.model}" - return self._graphql_types.get(key) - - def get_all_types(self) -> List[Type]: - return list(set(self._graphql_types.values())) - - -# Global registry instance -urlpath_registry = URLPathRegistry() - - -def register_urlpath_type(model_class: Type[Model], graphql_type: Type): - """Decorator to register a GraphQL type for a model""" - urlpath_registry.register_type(model_class, graphql_type) - return model_class diff --git a/baseapp_wagtail/base/urlpath/urlpath_sync.py b/baseapp_wagtail/base/urlpath/urlpath_sync.py index 245fecf5..6b9c3b8f 100644 --- a/baseapp_wagtail/base/urlpath/urlpath_sync.py +++ b/baseapp_wagtail/base/urlpath/urlpath_sync.py @@ -11,18 +11,20 @@ class WagtailURLPathSync: page: Page urlpath_model: Optional[Model] + is_baseapp_pages_installed: bool def __init__(self, page: Page): self.page = page self.urlpath_model = None + self.is_baseapp_pages_installed = apps.is_installed("baseapp_pages") self._load_urlpath_model() def _load_urlpath_model(self): - try: + if self.is_baseapp_pages_installed: from baseapp_pages.models import URLPath self.urlpath_model = URLPath - except ImportError: + else: self.urlpath_model = None def create_urlpath(self): @@ -82,17 +84,17 @@ def delete_urlpath(self): return def _can_sync(self) -> bool: - return self._is_available() and self._is_urlpath_target() + return ( + self.is_baseapp_pages_installed and self._is_available() and self._is_urlpath_target() + ) def _is_available(self) -> bool: return self.urlpath_model is not None def _is_urlpath_target(self) -> bool: - if apps.is_installed("baseapp_pages"): - from baseapp_pages.models import PageMixin + from baseapp_pages.models import PageMixin - return isinstance(self.page, PageMixin) - return False + return isinstance(self.page, PageMixin) def _get_wagtail_path(self) -> Optional[str]: url_parts = self.page.get_url_parts() From 8a6acd6b4fe64a72a81c5844091f0c84ea293d73 Mon Sep 17 00:00:00 2001 From: Hercilio Ortiz Date: Fri, 15 Aug 2025 07:58:47 -0300 Subject: [PATCH 10/18] Update: Changes due unit tests validations --- baseapp_pages/graphql/object_types.py | 5 - baseapp_pages/graphql/queries.py | 2 + baseapp_pages/models.py | 6 +- baseapp_pages/tests/test_model_page_mixin.py | 89 ++++++ .../tests/test_utils_url_path_formatter.py | 46 +++ ...path_formater.py => url_path_formatter.py} | 2 +- baseapp_wagtail/base/graphql/interfaces.py | 26 +- baseapp_wagtail/base/graphql/object_types.py | 61 ++-- baseapp_wagtail/base/models.py | 11 +- baseapp_wagtail/base/tests/__init__.py | 0 .../test_graphql_comments_integration.py | 288 ++++++++++++++++++ .../base/tests/test_graphql_urlpath_query.py | 262 ++++++++++++++++ baseapp_wagtail/tests/mixins.py | 11 +- setup.cfg | 2 +- 14 files changed, 758 insertions(+), 53 deletions(-) create mode 100644 baseapp_pages/tests/test_model_page_mixin.py create mode 100644 baseapp_pages/tests/test_utils_url_path_formatter.py rename baseapp_pages/utils/{url_path_formater.py => url_path_formatter.py} (98%) create mode 100644 baseapp_wagtail/base/tests/__init__.py create mode 100644 baseapp_wagtail/base/tests/test_graphql_comments_integration.py create mode 100644 baseapp_wagtail/base/tests/test_graphql_urlpath_query.py diff --git a/baseapp_pages/graphql/object_types.py b/baseapp_pages/graphql/object_types.py index 909c38b9..de379c7e 100644 --- a/baseapp_pages/graphql/object_types.py +++ b/baseapp_pages/graphql/object_types.py @@ -24,11 +24,6 @@ class PageInterface(relay.Node): url_paths = graphene.List(lambda: URLPathNode) metadata = graphene.Field(lambda: MetadataObjectType) - def resolve_type(self, info): - if hasattr(self, "get_graphql_object_type"): - return self.get_graphql_object_type() - return None - @classmethod def resolve_url_path(cls, instance, info, **kwargs): return instance.url_path diff --git a/baseapp_pages/graphql/queries.py b/baseapp_pages/graphql/queries.py index fc47d771..c17143ea 100644 --- a/baseapp_pages/graphql/queries.py +++ b/baseapp_pages/graphql/queries.py @@ -37,5 +37,7 @@ def resolve_url_path(self, info, path): ).first() if active_url_path: url_path = active_url_path + else: + url_path = None return url_path diff --git a/baseapp_pages/models.py b/baseapp_pages/models.py index 0b4ab807..b549cd52 100644 --- a/baseapp_pages/models.py +++ b/baseapp_pages/models.py @@ -17,7 +17,7 @@ from baseapp_comments.models import CommentableModel from baseapp_core.graphql.models import RelayModel from baseapp_core.models import random_name_in -from baseapp_pages.utils.url_path_formater import URLPathFormater +from baseapp_pages.utils.url_path_formatter import URLPathFormatter class URLPath(TimeStampedModel, RelayModel): @@ -118,7 +118,7 @@ def create_url_path(self, path: str, language: Optional[str] = None, is_active: if not self.pk: raise ValueError("Save the instance before creating URL paths.") return self.url_paths.create( - path=URLPathFormater(path)(), language=language, is_active=is_active + path=URLPathFormatter(path)(), language=language, is_active=is_active ) def update_url_path(self, path: str, language: Optional[str] = None, is_active: bool = True): @@ -126,7 +126,7 @@ def update_url_path(self, path: str, language: Optional[str] = None, is_active: raise ValueError("Save the instance before updating URL paths.") primary_path = self.url_path if primary_path: - primary_path.path = URLPathFormater(path)() + primary_path.path = URLPathFormatter(path)() primary_path.language = language primary_path.is_active = is_active primary_path.save() diff --git a/baseapp_pages/tests/test_model_page_mixin.py b/baseapp_pages/tests/test_model_page_mixin.py new file mode 100644 index 00000000..df1f4527 --- /dev/null +++ b/baseapp_pages/tests/test_model_page_mixin.py @@ -0,0 +1,89 @@ +import pytest +import swapper +from django.contrib.auth import get_user_model + +from baseapp_pages.models import URLPath +from baseapp_pages.utils.url_path_formatter import URLPathFormatter + +from .factories import PageFactory + +pytestmark = pytest.mark.django_db + +Page = swapper.load_model("baseapp_pages", "Page") +User = get_user_model() + + +def make_page_mixin_instance(): + return PageFactory() + + +def test_url_path_property_returns_active_path(monkeypatch): + page = make_page_mixin_instance() + url_path_active = page.create_url_path("/test-path", language="en", is_active=True) + page.create_url_path("/test-path-inactive", language="en", is_active=False) + + assert page.url_path == url_path_active + + +def test_url_path_property_returns_none_if_no_active(): + page = make_page_mixin_instance() + # No url paths yet + assert page.url_path is None + # Add only inactive + page.create_url_path("/inactive", language="en", is_active=False) + assert page.url_path is None + + +def test_create_url_path_requires_saved_instance(): + page = Page() # not saved + with pytest.raises(ValueError): + page.create_url_path("/should-fail") + + +def test_create_url_path_creates_urlpath(): + page = make_page_mixin_instance() + url_path = page.create_url_path("/created", language="en", is_active=True) + assert isinstance(url_path, URLPath) + assert url_path.path == URLPathFormatter("/created")() + assert url_path.language == "en" + assert url_path.is_active is True + assert url_path.target == page + + +def test_update_url_path_updates_existing(monkeypatch): + page = make_page_mixin_instance() + url_path = page.create_url_path("/old", language="en", is_active=True) + page.update_url_path("/new", language="fr", is_active=False) + url_path.refresh_from_db() + assert url_path.path == URLPathFormatter("/new")() + assert url_path.language == "fr" + assert url_path.is_active is False + + +def test_update_url_path_creates_if_none_exists(): + page = make_page_mixin_instance() + assert page.url_path is None + page.update_url_path("/created", language="en", is_active=True) + url_path = page.url_path + assert url_path is not None + assert url_path.path == URLPathFormatter("/created")() + assert url_path.language == "en" + assert url_path.is_active is True + + +def test_deactivate_url_paths_sets_all_inactive(): + page = make_page_mixin_instance() + page.create_url_path("/a", language="en", is_active=True) + page.create_url_path("/b", language="fr", is_active=True) + page.deactivate_url_paths() + for url_path in page.url_paths.all(): + assert url_path.is_active is False + + +def test_delete_url_paths_deletes_all(): + page = make_page_mixin_instance() + page.create_url_path("/a", language="en", is_active=True) + page.create_url_path("/b", language="fr", is_active=True) + assert page.url_paths.count() == 2 + page.delete_url_paths() + assert page.url_paths.count() == 0 diff --git a/baseapp_pages/tests/test_utils_url_path_formatter.py b/baseapp_pages/tests/test_utils_url_path_formatter.py new file mode 100644 index 00000000..dde5202e --- /dev/null +++ b/baseapp_pages/tests/test_utils_url_path_formatter.py @@ -0,0 +1,46 @@ +import pytest + +from baseapp_pages.utils.url_path_formatter import URLPathFormatter + + +@pytest.mark.parametrize( + "input_path,expected", + [ + ("/", "/"), + (" / ", "/"), + ("test", "/test"), + ("/test", "/test"), + ("//test", "/test"), + ("/test/", "/test"), + ("//test//foo//bar//", "/test/foo/bar"), + ("foo/bar", "/foo/bar"), + ("/foo/bar", "/foo/bar"), + ("foo/bar/", "/foo/bar"), + ("foo//bar", "/foo/bar"), + ("//foo//bar//", "/foo/bar"), + ("//foo//bar//baz//", "/foo/bar/baz"), + (" /foo/bar/ ", "/foo/bar"), + ], +) +def test_url_path_formatter_valid(input_path, expected): + assert URLPathFormatter(input_path)() == expected + + +@pytest.mark.parametrize( + "bad_path,error_msg", + [ + ("", "Path cannot be empty."), + (" ", "Path cannot be empty."), + ], +) +def test_url_path_formatter_raises_on_empty(bad_path, error_msg): + with pytest.raises(ValueError) as exc: + URLPathFormatter(bad_path)() + assert error_msg in str(exc.value) + + +def test_url_path_formatter_invalid_no_leading_slash_after_format(): + f = URLPathFormatter("foo") + f.path = "foo" # forcibly break invariant + with pytest.raises(ValueError): + f._validate_path() diff --git a/baseapp_pages/utils/url_path_formater.py b/baseapp_pages/utils/url_path_formatter.py similarity index 98% rename from baseapp_pages/utils/url_path_formater.py rename to baseapp_pages/utils/url_path_formatter.py index 5ef1a4c0..27054aec 100644 --- a/baseapp_pages/utils/url_path_formater.py +++ b/baseapp_pages/utils/url_path_formatter.py @@ -1,7 +1,7 @@ import re -class URLPathFormater: +class URLPathFormatter: """ Ensures the path is valid: - Always starts with a single slash diff --git a/baseapp_wagtail/base/graphql/interfaces.py b/baseapp_wagtail/base/graphql/interfaces.py index e25acb17..b5a51048 100644 --- a/baseapp_wagtail/base/graphql/interfaces.py +++ b/baseapp_wagtail/base/graphql/interfaces.py @@ -4,6 +4,23 @@ from baseapp_core.graphql.models import RelayModel +""" +Baseapp Interfaces Compatibility + +Wagtail page type IDs are set at runtime, so the graphene ID field is not required by default. This +conflicts with relay.Node, which expects an ID to always be present. Since multiple baseapp +interfaces extend relay.Node, this can cause issues. + +To fix this, we extend the original interfaces and explicitly resolve the ID field, as shown in +WagtailCommentsInterface. Because Wagtail page IDs always exist, this approach ensures compatibility +with relay.Node and avoids ID-related problems. + +However, this approach only works when querying the page directly. When the Wagtail page is +referenced dynamically—such as via content_type and target id (e.g., as a comment target) — the +original interfaces must also be included in WagtailPageObjectType. This ensures the connection and +ID resolution work correctly in these dynamic GraphQL structures. +""" + if apps.is_installed("baseapp_comments"): from baseapp_comments.graphql.object_types import CommentsInterface @@ -11,20 +28,13 @@ class WagtailCommentsInterface(CommentsInterface): """ Wagtail-specific comments interface for Wagtail page types that do not support relay.Node. - - Extends baseapp_comments.graphql.CommentsInterface to enable comments on Wagtail - pages with dynamic or non-relay-compliant id fields, avoiding relay.Node conflicts. - - If needed, an intermediate object type could be used to connect CommentsInterface. - - @see baseapp_wagtail.base.graphql.object_types.WagtailURLPathObjectType """ id = graphene.ID() def resolve_id(self, info, **kwargs): if isinstance(self, RelayModel): - return self.relay_id + return self.id raise ValueError("WagtailCommentsInterface can only be used with RelayModel instances.") diff --git a/baseapp_wagtail/base/graphql/object_types.py b/baseapp_wagtail/base/graphql/object_types.py index 21252299..29e03d51 100644 --- a/baseapp_wagtail/base/graphql/object_types.py +++ b/baseapp_wagtail/base/graphql/object_types.py @@ -1,21 +1,39 @@ import graphene from django.apps import apps from graphene import relay +from graphene_django import DjangoObjectType from grapple.registry import registry from baseapp_wagtail.base.graphql.interfaces import WagtailPageInterface +from baseapp_wagtail.base.models import DefaultPageModel wagtail_url_path_object_type_interfaces = [] + if apps.is_installed("baseapp_pages"): from baseapp_pages.graphql import PageInterface wagtail_url_path_object_type_interfaces.append(PageInterface) +if apps.is_installed("baseapp_comments"): + from baseapp_comments.graphql.object_types import CommentsInterface + + wagtail_url_path_object_type_interfaces.append(CommentsInterface) + -class WagtailURLPathObjectType(graphene.ObjectType): +class WagtailPageObjectType(DjangoObjectType): + """ + Object type for connecting Wagtail pages with other baseapp interfaces. Use this when Wagtail + pages must be retrieved via different baseapp interfaces. + If only the Wagtail page interface is needed, use the graphql_interfaces attribute in the + Wagtail page models. Note: You may need to extend the interface and resolve the id field as + shown in WagtailCommentsInterface. + """ + + id = graphene.ID(required=True) data = graphene.Field(WagtailPageInterface) class Meta: + model = DefaultPageModel interfaces = ( relay.Node, *wagtail_url_path_object_type_interfaces, @@ -23,8 +41,13 @@ class Meta: name = "WagtailPage" @classmethod - def resolve_type(cls, instance, info): - return cls + def is_type_of(cls, root, info): + if isinstance(root, DefaultPageModel): + return True + return super().is_type_of(root, info) + + def resolve_id(self, info): + return self.id def resolve_data(self, info): return self @@ -32,31 +55,19 @@ def resolve_data(self, info): @classmethod def resolve_metadata(cls, instance, info, **kwargs): if apps.is_installed("baseapp_pages"): - from baseapp_pages.graphql.object_types import AbstractMetadataObjectType - - class WagtailMetadata(AbstractMetadataObjectType): - # TODO: (BA-2635) Complete the metadata for Wagtail pages when implementing the story BA-2635. - @property - def meta_title(self): - return self.instance.title - - @property - def meta_description(self): - return None - - @property - def meta_og_type(self): - return "article" - - @property - def meta_og_image(self): - return None - - return WagtailMetadata(instance, info) + from baseapp_pages.graphql.object_types import MetadataObjectType + + # TODO: (BA-2635) Complete the metadata for Wagtail pages when implementing the story BA-2635. + return MetadataObjectType( + meta_title=instance.title, + meta_description=None, + meta_og_image=None, + meta_og_type="article", + ) return None BASEAPP_WAGTAIL_TYPES = [ *registry.models.values(), - WagtailURLPathObjectType, + WagtailPageObjectType, ] diff --git a/baseapp_wagtail/base/models.py b/baseapp_wagtail/base/models.py index c6d10767..dc622fcd 100644 --- a/baseapp_wagtail/base/models.py +++ b/baseapp_wagtail/base/models.py @@ -4,7 +4,6 @@ from django.apps import apps from django.conf import settings from django.db.models import Q -from django.utils.translation import get_language from django.utils.translation import gettext as _ from grapple.models import GraphQLStreamfield from wagtail.admin.panels import FieldPanel @@ -91,7 +90,7 @@ def pages_url_path(self): Defines a new property because wagtail pages are have have a defined "url_path" property. """ return self.url_paths.filter( - Q(is_active=True), Q(language=get_language()) | Q(language__isnull=True) + Q(is_active=True), Q(language=self.locale.language_code) | Q(language__isnull=True) ).first() def update_url_path(self, path: str, language: Optional[str] = None, is_active: bool = True): @@ -99,11 +98,11 @@ def update_url_path(self, path: str, language: Optional[str] = None, is_active: Overrides the baseapp_pages.models.PageMixin.update_url_path method. This is necessary in order to use the new "pages_url_path" property. """ - from baseapp_pages.utils.url_path_formater import URLPathFormater + from baseapp_pages.utils.url_path_formatter import URLPathFormatter primary_path = self.pages_url_path if primary_path: - primary_path.path = URLPathFormater(path)() + primary_path.path = URLPathFormatter(path)() primary_path.language = language primary_path.is_active = is_active primary_path.save() @@ -112,9 +111,7 @@ def update_url_path(self, path: str, language: Optional[str] = None, is_active: @classmethod def get_graphql_object_type(cls): - from baseapp_wagtail.base.graphql.object_types import WagtailURLPathObjectType - - return WagtailURLPathObjectType + return None class Meta: abstract = True diff --git a/baseapp_wagtail/base/tests/__init__.py b/baseapp_wagtail/base/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/baseapp_wagtail/base/tests/test_graphql_comments_integration.py b/baseapp_wagtail/base/tests/test_graphql_comments_integration.py new file mode 100644 index 00000000..3016b59a --- /dev/null +++ b/baseapp_wagtail/base/tests/test_graphql_comments_integration.py @@ -0,0 +1,288 @@ +from rest_framework import status + +from baseapp_comments.tests.factories import CommentFactory +from baseapp_wagtail.tests.mixins import TestPageContextMixin +from baseapp_wagtail.tests.utils.graphql_helpers import GraphqlHelper +from testproject.base.models import StandardPage + + +class WagtailCommentsIntegrationTests(GraphqlHelper, TestPageContextMixin): + page_model = StandardPage + + def setUp(self): + super().setUp() + self.page.save_revision().publish() + + def test_wagtail_page_has_comments_interface(self): + response = self.query( + """ + query Page($id: ID!) { + page(id: $id) { + id + title + ... on StandardPage { + commentsCount { + total + main + replies + pinned + reported + } + isCommentsEnabled + } + } + } + """, + variables={"id": self.page.id}, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + content = response.json() + + data = content["data"]["page"] + self.assertIsNotNone(data["commentsCount"]) + self.assertEqual(data["commentsCount"]["total"], 0) + self.assertEqual(data["commentsCount"]["main"], 0) + self.assertEqual(data["commentsCount"]["replies"], 0) + self.assertEqual(data["commentsCount"]["pinned"], 0) + self.assertEqual(data["commentsCount"]["reported"], 0) + self.assertTrue(data["isCommentsEnabled"]) + + def test_create_comment_on_wagtail_page(self): + response = self.query( + """ + mutation CommentCreate($input: CommentCreateInput!) { + commentCreate(input: $input) { + comment { + node { + id + body + target { + id + commentsCount { + total + replies + } + } + } + } + errors { + field + messages + } + } + } + """, + variables={ + "input": { + "targetObjectId": self.page.relay_id, + "body": "This is a test comment on a Wagtail page", + } + }, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + content = response.json() + + self.assertIsNone(content["data"]["commentCreate"]["errors"]) + + comment_data = content["data"]["commentCreate"]["comment"]["node"] + self.assertEqual(comment_data["body"], "This is a test comment on a Wagtail page") + + target = comment_data["target"] + self.assertEqual(target["id"], str(self.page.id)) + comments_count = target["commentsCount"] + self.assertEqual(comments_count["total"], 1) + self.assertEqual(comments_count["replies"], 0) + + def test_create_reply_comment_on_wagtail_page(self): + parent_comment = CommentFactory(target=self.page) + + response = self.query( + """ + mutation CommentCreate($input: CommentCreateInput!) { + commentCreate(input: $input) { + comment { + node { + id + body + inReplyTo { + id + body + } + target { + ... on WagtailPage { + id + commentsCount { + total + main + replies + } + } + } + } + } + errors { + field + messages + } + } + } + """, + variables={ + "input": { + "targetObjectId": self.page.relay_id, + "body": "This is a reply to the parent comment", + "inReplyToId": parent_comment.relay_id, + } + }, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + content = response.json() + + self.assertIsNone(content["data"]["commentCreate"]["errors"]) + + comment_data = content["data"]["commentCreate"]["comment"]["node"] + self.assertEqual(comment_data["body"], "This is a reply to the parent comment") + + in_reply_to = comment_data["inReplyTo"] + self.assertEqual(in_reply_to["id"], parent_comment.relay_id) + self.assertEqual(in_reply_to["body"], parent_comment.body) + + target = comment_data["target"] + comments_count = target["commentsCount"] + self.assertEqual(comments_count["total"], 2) + self.assertEqual(comments_count["main"], 1) + self.assertEqual(comments_count["replies"], 1) + + def test_query_comments_on_wagtail_page(self): + CommentFactory(target=self.page, body="First comment") + CommentFactory(target=self.page, body="Second comment") + + response = self.query( + """ + query PageComments($id: ID!) { + page(id: $id) { + id + title + ... on StandardPage { + comments { + edges { + node { + id + body + user { + id + } + created + } + } + } + commentsCount { + total + main + } + } + } + } + """, + variables={"id": self.page.id}, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + content = response.json() + + data = content["data"]["page"] + comments = data["comments"]["edges"] + + self.assertEqual(len(comments), 2) + self.assertEqual(data["commentsCount"]["total"], 2) + self.assertEqual(data["commentsCount"]["main"], 2) + + comment_bodies = [edge["node"]["body"] for edge in comments] + self.assertIn("First comment", comment_bodies) + self.assertIn("Second comment", comment_bodies) + + def test_comments_disabled_on_wagtail_page(self): + self.page.is_comments_enabled = False + self.page.save() + + response = self.query( + """ + mutation CommentCreate($input: CommentCreateInput!) { + commentCreate(input: $input) { + comment { + node { + id + body + } + } + errors { + field + messages + } + } + } + """, + variables={ + "input": { + "targetObjectId": self.page.relay_id, + "body": "This comment should not be created", + } + }, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + content = response.json() + + errors = content["errors"] + self.assertIsNotNone(errors) + self.assertEqual(errors[0]["extensions"]["code"], "permission_required") + + def test_comments_ordering_and_pagination(self): + CommentFactory(target=self.page, body="Comment 1") + CommentFactory(target=self.page, body="Comment 2") + CommentFactory(target=self.page, body="Comment 3") + + response = self.query( + """ + query PageComments($id: ID!, $first: Int!) { + page(id: $id) { + id + ... on StandardPage { + comments(first: $first) { + edges { + node { + id + body + created + } + cursor + } + pageInfo { + hasNextPage + hasPreviousPage + } + } + commentsCount { + total + } + } + } + } + """, + variables={"id": self.page.id, "first": 2}, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + content = response.json() + + data = content["data"]["page"] + comments = data["comments"]["edges"] + page_info = data["comments"]["pageInfo"] + + self.assertEqual(len(comments), 2) + self.assertTrue(page_info["hasNextPage"]) + self.assertFalse(page_info["hasPreviousPage"]) + self.assertEqual(data["commentsCount"]["total"], 3) diff --git a/baseapp_wagtail/base/tests/test_graphql_urlpath_query.py b/baseapp_wagtail/base/tests/test_graphql_urlpath_query.py new file mode 100644 index 00000000..b0b9ee33 --- /dev/null +++ b/baseapp_wagtail/base/tests/test_graphql_urlpath_query.py @@ -0,0 +1,262 @@ +from rest_framework import status +from wagtail.models import Locale + +from baseapp_wagtail.base.urlpath.urlpath_sync import WagtailURLPathSync +from baseapp_wagtail.tests.mixins import TestPageContextMixin +from baseapp_wagtail.tests.utils.graphql_helpers import GraphqlHelper +from testproject.base.models import StandardPage + + +class WagtailURLPathObjectTypeTests(GraphqlHelper, TestPageContextMixin): + def setUp(self): + super().setUp() + self.page.save_revision().publish() + WagtailURLPathSync(self.page).create_urlpath() + self.url_path = self.page.pages_url_path + + def test_urlpath_query_returns_wagtail_page_data(self): + response = self.query( + """ + query Page($path: String!) { + urlPath(path: $path) { + path + target { + __typename + ... on WagtailPage { + data { + id + title + ... on PageForTests { + body { + ... on RichTextBlock { + value + } + } + } + } + } + } + } + } + """, + variables={"path": self.url_path.path}, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + content = response.json() + + self.assertIsNotNone(content["data"]["urlPath"]) + self.assertEqual(content["data"]["urlPath"]["path"], self.url_path.path) + + target = content["data"]["urlPath"]["target"] + self.assertEqual(target["__typename"], "WagtailPage") + + data = target["data"] + self.assertEqual(data["id"], str(self.page.id)) + self.assertEqual(data["title"], self.page.title) + + def test_urlpath_query_with_metadata(self): + response = self.query( + """ + query Page($path: String!) { + urlPath(path: $path) { + target { + ... on WagtailPage { + data { + ... on PageForTests { + metadata { + metaTitle + metaDescription + metaOgType + } + } + } + } + } + } + } + """, + variables={"path": self.url_path.path}, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + content = response.json() + + data = content["data"]["urlPath"]["target"]["data"] + metadata = data["metadata"] + self.assertIsNotNone(metadata) + self.assertEqual(metadata["metaTitle"], self.page.title) + self.assertEqual(metadata["metaOgType"], "article") + + def test_urlpath_query_with_standard_page(self): + standard_page = StandardPage( + title="Standard Test Page", + slug="standard-test-page", + path=f"{self.site.root_page.path}0002", + depth=self.site.root_page.depth + 1, + ) + self.site.root_page.add_child(instance=standard_page) + standard_page.save_revision().publish() + WagtailURLPathSync(standard_page).create_urlpath() + standard_url_path = standard_page.pages_url_path + + response = self.query( + """ + query Page($path: String!) { + urlPath(path: $path) { + path + target { + __typename + ... on WagtailPage { + data { + id + title + ... on StandardPage { + path + } + } + } + } + } + } + """, + variables={"path": standard_url_path.path}, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + content = response.json() + + self.assertIsNotNone(content["data"]["urlPath"]) + target = content["data"]["urlPath"]["target"] + self.assertEqual(target["__typename"], "WagtailPage") + + data = target["data"] + self.assertEqual(data["id"], str(standard_page.id)) + self.assertEqual(data["title"], standard_page.title) + + def test_urlpath_query_with_comments_interface(self): + standard_page = StandardPage( + title="Standard Test Page", + slug="standard-test-page", + path=f"{self.site.root_page.path}0002", + depth=self.site.root_page.depth + 1, + ) + self.site.root_page.add_child(instance=standard_page) + standard_page.save_revision().publish() + WagtailURLPathSync(standard_page).create_urlpath() + standard_url_path = standard_page.pages_url_path + + response = self.query( + """ + query Page($path: String!) { + urlPath(path: $path) { + target { + ... on WagtailPage { + data { + ... on StandardPage { + commentsCount { + total + } + } + } + } + } + } + } + """, + variables={"path": standard_url_path.path}, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual( + response.json()["data"]["urlPath"]["target"]["data"]["commentsCount"]["total"], 0 + ) + + def test_urlpath_query_inactive_path(self): + self.page.update_url_path(path="/inactive-page", language="en", is_active=False) + + response = self.query( + """ + query Page($path: String!) { + urlPath(path: $path) { + path + target { + ... on WagtailPage { + data { + id + title + } + } + } + } + } + """, + variables={"path": "/inactive-page"}, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertIsNone(response.json()["data"]["urlPath"]) + + def test_urlpath_query_nonexistent_path(self): + response = self.query( + """ + query Page($path: String!) { + urlPath(path: $path) { + path + target { + ... on WagtailPage { + data { + id + title + } + } + } + } + } + """, + variables={"path": "/nonexistent-page"}, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + content = response.json() + + self.assertIsNone(content["data"]["urlPath"]) + + def test_urlpath_query_with_language_specific_path(self): + standard_page = StandardPage( + title="Standard Test Page", + slug="standard-test-page", + path=f"{self.site.root_page.path}0002", + depth=self.site.root_page.depth + 1, + locale=Locale.objects.get_or_create(language_code="pt")[0], + ) + self.site.root_page.add_child(instance=standard_page) + standard_page.save_revision().publish() + WagtailURLPathSync(standard_page).create_urlpath() + pt_url_path = standard_page.pages_url_path + + response = self.query( + """ + query Page($path: String!) { + urlPath(path: $path) { + path + target { + ... on WagtailPage { + data { + id + title + } + } + } + } + } + """, + variables={"path": pt_url_path.path}, + headers={"HTTP_ACCEPT_LANGUAGE": "pt"}, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + content = response.json() + + self.assertIsNotNone(content["data"]["urlPath"]) + self.assertEqual(content["data"]["urlPath"]["path"], pt_url_path.path) diff --git a/baseapp_wagtail/tests/mixins.py b/baseapp_wagtail/tests/mixins.py index 54149fc4..56a2f886 100644 --- a/baseapp_wagtail/tests/mixins.py +++ b/baseapp_wagtail/tests/mixins.py @@ -1,4 +1,5 @@ import urllib.parse +from typing import Type from django.test import TestCase, override_settings from django.urls import reverse @@ -17,13 +18,17 @@ class WagtailBasicMixin(WagtailPageTestCase, WagtailTestUtils, TestCase): class TestPageContextMixin(WagtailBasicMixin): + page_model: Type[Page] = PageForTests + root_page: PageForTests + page: PageForTests + @classmethod def setUpTestData(cls): cls.user = UserFactory() root_page = Page.get_first_root_node() if not root_page: LocaleFactory(language_code="en") - root_page = PageForTests( + root_page = cls.page_model( title="Root", slug="root", depth=1, @@ -40,7 +45,7 @@ def setUpTestData(cls): "site_name": "localhost", }, ) - cls.page = PageForTests( + cls.page = cls.page_model( title="My Page", slug="mypage", depth=cls.site.root_page.depth + 1, @@ -49,7 +54,7 @@ def setUpTestData(cls): cls.site.root_page.add_child(instance=cls.page) def _reload_the_page(self): - self.page = PageForTests.objects.get(id=self.page.id) + self.page = self.page_model.objects.get(id=self.page.id) def _get_edit_page(self, page): response = self.client.get(reverse("wagtailadmin_pages:edit", args=[page.id])) diff --git a/setup.cfg b/setup.cfg index a7d62abf..cfbd6151 100644 --- a/setup.cfg +++ b/setup.cfg @@ -76,7 +76,7 @@ wagtail = wagtail == 7.0.1 wagtail-headless-preview == 0.8 # TODO: Update to the original repo once the PR is merged (https://github.com/torchbox/wagtail-grapple/pull/423) - wagtail_grapple @ git+https://github.com/Hercilio1/wagtail-grapple@602faa94a1fd216f05daf739e39fd72d9908eb37 + wagtail_grapple @ git+https://github.com/Hercilio1/wagtail-grapple@874e3d3a6b677ee0b579511f83c75f537b0fa5b1 socialauth = hashids == 1.3.1 rest-social-auth >= 8.1.0 From bbbd1cd4934552fcffcea013976ebee5f49e5426 Mon Sep 17 00:00:00 2001 From: Hercilio Ortiz Date: Fri, 15 Aug 2025 09:47:01 -0300 Subject: [PATCH 11/18] Update: Adding unit tests for the urlpath sync process --- .../base/tests/test_graphql_urlpath_query.py | 17 ++- .../base/tests/test_urlpath_sync.py | 66 ++++++++++ .../base/tests/test_urlpath_sync_hooks.py | 121 ++++++++++++++++++ baseapp_wagtail/base/urlpath/urlpath_sync.py | 8 +- baseapp_wagtail/tests/mixins.py | 91 ++++++++++++- 5 files changed, 288 insertions(+), 15 deletions(-) create mode 100644 baseapp_wagtail/base/tests/test_urlpath_sync.py create mode 100644 baseapp_wagtail/base/tests/test_urlpath_sync_hooks.py diff --git a/baseapp_wagtail/base/tests/test_graphql_urlpath_query.py b/baseapp_wagtail/base/tests/test_graphql_urlpath_query.py index b0b9ee33..9db4a118 100644 --- a/baseapp_wagtail/base/tests/test_graphql_urlpath_query.py +++ b/baseapp_wagtail/base/tests/test_graphql_urlpath_query.py @@ -7,7 +7,7 @@ from testproject.base.models import StandardPage -class WagtailURLPathObjectTypeTests(GraphqlHelper, TestPageContextMixin): +class WagtailURLPathQueryTests(GraphqlHelper, TestPageContextMixin): def setUp(self): super().setUp() self.page.save_revision().publish() @@ -63,13 +63,12 @@ def test_urlpath_query_with_metadata(self): target { ... on WagtailPage { data { - ... on PageForTests { - metadata { - metaTitle - metaDescription - metaOgType - } - } + title + } + metadata { + metaTitle + metaDescription + metaOgType } } } @@ -82,7 +81,7 @@ def test_urlpath_query_with_metadata(self): self.assertEqual(response.status_code, status.HTTP_200_OK) content = response.json() - data = content["data"]["urlPath"]["target"]["data"] + data = content["data"]["urlPath"]["target"] metadata = data["metadata"] self.assertIsNotNone(metadata) self.assertEqual(metadata["metaTitle"], self.page.title) diff --git a/baseapp_wagtail/base/tests/test_urlpath_sync.py b/baseapp_wagtail/base/tests/test_urlpath_sync.py new file mode 100644 index 00000000..a3d0d988 --- /dev/null +++ b/baseapp_wagtail/base/tests/test_urlpath_sync.py @@ -0,0 +1,66 @@ +from baseapp_pages.models import URLPath +from baseapp_pages.tests.factories import URLPathFactory +from baseapp_wagtail.base.urlpath.urlpath_sync import WagtailURLPathSync +from baseapp_wagtail.tests.mixins import TestPageContextMixin + + +class TestWagtailURLPathSyncIntegration(TestPageContextMixin): + def test_create_urlpath_creates_inactive_urlpath(self): + self.page.live = False + self.page.save() + sync = WagtailURLPathSync(self.page) + sync.create_urlpath() + urlpath = URLPath.objects.get(path="/mypage") + self.assertEqual(urlpath.path, "/mypage") + self.assertFalse(urlpath.is_active) + self.assertEqual(urlpath.target, self.page) + + def test_create_urlpath_creates_unique_path(self): + URLPathFactory(path="/mypage", is_active=False) + sync = WagtailURLPathSync(self.page) + sync.create_urlpath() + self.assertTrue(URLPath.objects.filter(path="/mypage-1").exists()) + + def test_update_urlpath_sets_active(self): + sync = WagtailURLPathSync(self.page) + sync.create_urlpath() + self.page.save_revision().publish() + sync.update_urlpath() + urlpath = URLPath.objects.get(path="/mypage") + self.assertTrue(urlpath.is_active) + + def test_deactivate_urlpath_sets_inactive(self): + sync = WagtailURLPathSync(self.page) + sync.create_urlpath() + self.page.save_revision().publish() + sync.update_urlpath() + sync.deactivate_urlpath() + urlpath = URLPath.objects.get(path="/mypage") + self.assertFalse(urlpath.is_active) + + def test_delete_urlpath_removes_urlpath(self): + sync = WagtailURLPathSync(self.page) + sync.create_urlpath() + self.assertTrue(URLPath.objects.filter(path="/mypage").exists()) + sync.delete_urlpath() + self.assertFalse(URLPath.objects.filter(path="/mypage").exists()) + + def test_generate_unique_path(self): + sync = WagtailURLPathSync(self.page) + URLPathFactory(path="/mypage", target=self.page, is_active=False) + URLPathFactory(path="/mypage-1", target=self.page, is_active=False) + unique_path = sync._generate_unique_path("/mypage") + self.assertEqual(unique_path, "/mypage-2") + + def test_can_sync_true_for_page_mixin(self): + sync = WagtailURLPathSync(self.page) + self.assertTrue(sync._can_sync()) + + def test_can_sync_false_if_not_urlpath_target(self): + from wagtail.models import Page + + root = Page.objects.get(id=self.root_page.id) + plain_page = Page(title="Plain", slug="plain", depth=2, path="00010002") + root.add_child(instance=plain_page) + sync = WagtailURLPathSync(plain_page) + self.assertFalse(sync._can_sync()) diff --git a/baseapp_wagtail/base/tests/test_urlpath_sync_hooks.py b/baseapp_wagtail/base/tests/test_urlpath_sync_hooks.py new file mode 100644 index 00000000..2de7b3c4 --- /dev/null +++ b/baseapp_wagtail/base/tests/test_urlpath_sync_hooks.py @@ -0,0 +1,121 @@ +from rest_framework import status + +from baseapp_pages.models import URLPath +from baseapp_wagtail.tests.mixins import TestAdminActionsMixin +from baseapp_wagtail.tests.models import PageForTests + + +class URLPathSyncHooksTests(TestAdminActionsMixin): + def test_urlpath_creation_on_page_create(self): + response = self._post_new_page({"slug": "test-page-2"}) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + new_page = self._get_page_by_slug("test-page-2") + self.assertIsNotNone(new_page) + + urlpath = new_page.pages_url_path + self.assertIsNone(urlpath) + + urlpath = URLPath.objects.get(path="/mypage/test-page-2") + self.assertEqual(urlpath.path, "/mypage/test-page-2") + self.assertFalse(urlpath.is_active) + + def test_urlpath_creation_on_page_create_and_publish(self): + response = self._post_new_page({"action-publish": "action-publish", "slug": "test-page-2"}) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + new_page = self._get_page_by_slug("test-page-2") + self.assertIsNotNone(new_page) + + urlpath = new_page.pages_url_path + self.assertIsNotNone(urlpath) + self.assertEqual(urlpath.path, "/mypage/test-page-2") + self.assertTrue(urlpath.is_active) + + def test_urlpath_update_on_page_publish(self): + self.assertIsNone(self.page.pages_url_path) + + response = self._post_publish_page(self.page) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + self._reload_the_page() + + urlpath = self.page.pages_url_path + self.assertTrue(urlpath.is_active) + + def test_urlpath_deactivation_on_page_unpublish(self): + response = self._post_publish_page(self.page) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + self._reload_the_page() + + urlpath = self.page.pages_url_path + self.assertTrue(urlpath.is_active) + + response = self._post_unpublish_page(self.page) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + self._reload_the_page() + + urlpath = self.page.pages_url_path + self.assertIsNone(urlpath) + + urlpath = URLPath.objects.get(path="/test-page") + self.assertFalse(urlpath.is_active) + + def test_urlpath_deletion_on_page_delete(self): + response = self._post_new_page({"action-publish": "action-publish", "slug": "test-page-2"}) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + page = self._get_page_by_slug("test-page-2") + + urlpath = page.pages_url_path + self.assertTrue(urlpath.is_active) + + response = self._post_delete_page(page) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + self.assertFalse(PageForTests.objects.filter(id=page.id).exists()) + + urlpath = URLPath.objects.filter(path="/mypage/test-page-2").exists() + self.assertFalse(urlpath) + + def test_editting_page_already_published_not_changing_existing_urlpath(self): + response = self._post_new_page({"action-publish": "action-publish", "slug": "test-page-2"}) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + page = self._get_page_by_slug("test-page-2") + + urlpath = page.pages_url_path + self.assertTrue(urlpath.is_active) + + edit_data = { + "title": "Updated Page Title", + "slug": "updated-page-slug", + } + + response = self._post_edit_page(page, edit_data) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + urlpath = page.pages_url_path + self.assertEqual(urlpath.path, "/mypage/test-page-2") + + def test_urlpath_path_update_on_page_edit(self): + response = self._post_new_page({"action-publish": "action-publish", "slug": "test-page-2"}) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + page = self._get_page_by_slug("test-page-2") + + urlpath = page.pages_url_path + self.assertTrue(urlpath.is_active) + + edit_data = { + "title": "Updated Page Title", + "slug": "updated-page-slug", + } + + response = self._post_publish_page(page, edit_data) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + urlpath = page.pages_url_path + self.assertEqual(urlpath.path, "/mypage/updated-page-slug") diff --git a/baseapp_wagtail/base/urlpath/urlpath_sync.py b/baseapp_wagtail/base/urlpath/urlpath_sync.py index 6b9c3b8f..06fc799e 100644 --- a/baseapp_wagtail/base/urlpath/urlpath_sync.py +++ b/baseapp_wagtail/base/urlpath/urlpath_sync.py @@ -39,8 +39,8 @@ def create_urlpath(self): wagtail_path = self._generate_unique_path(wagtail_path) try: - # Use the mixin method - self.page.create_url_path( + # The created action can be triggered after the publish action when the page is created. + self.page.update_url_path( path=wagtail_path, language=self.page.locale.language_code, is_active=self.page.live ) except Exception as e: @@ -101,6 +101,10 @@ def _get_wagtail_path(self) -> Optional[str]: if not url_parts: return None _, _, page_path = url_parts + if self.is_baseapp_pages_installed: + from baseapp_pages.utils.url_path_formatter import URLPathFormatter + + return URLPathFormatter(page_path)() return page_path def _generate_unique_path(self, base_path: str) -> str: diff --git a/baseapp_wagtail/tests/mixins.py b/baseapp_wagtail/tests/mixins.py index 56a2f886..40fbc428 100644 --- a/baseapp_wagtail/tests/mixins.py +++ b/baseapp_wagtail/tests/mixins.py @@ -7,7 +7,6 @@ from wagtail.test.utils import WagtailPageTestCase, WagtailTestUtils import baseapp_wagtail.medias.tests.factories as medias_factories -from baseapp_core.tests.factories import UserFactory from baseapp_wagtail.tests.factories.wagtail_factories import LocaleFactory from baseapp_wagtail.tests.models import PageForTests @@ -24,7 +23,6 @@ class TestPageContextMixin(WagtailBasicMixin): @classmethod def setUpTestData(cls): - cls.user = UserFactory() root_page = Page.get_first_root_node() if not root_page: LocaleFactory(language_code="en") @@ -53,22 +51,107 @@ def setUpTestData(cls): ) cls.site.root_page.add_child(instance=cls.page) + def setUp(self): + super().setUp() + self.user = self.login() + def _reload_the_page(self): self.page = self.page_model.objects.get(id=self.page.id) + +class TestAdminActionsMixin(TestPageContextMixin): + def setUp(self): + super().setUp() + self.image = medias_factories.ImageFactory() + + def _get_page_by_slug(self, slug=None): + slug = slug or self._get_page_slug() + return self.page_model.objects.get(slug=slug) + + def _get_page_slug(self): + return "test-page" + def _get_edit_page(self, page): response = self.client.get(reverse("wagtailadmin_pages:edit", args=[page.id])) return response + def _post_new_page(self, extra_data=None): + post_data = self._get_page_data(extra_data) + response = self.client.post( + reverse( + "wagtailadmin_pages:add", + args=("tests", self.page_model.__name__.lower(), self.page.id), + ), + post_data, + follow=True, + ) + return response + + def _post_edit_page(self, page, extra_data=None): + post_data = self._get_page_data(extra_data) + response = self.client.post( + reverse("wagtailadmin_pages:edit", args=[page.id]), + post_data, + follow=True, + ) + return response + + def _post_publish_page(self, page, extra_data=None): + publish_data = { + "action-publish": "action-publish", + } + if extra_data: + publish_data.update(extra_data) + return self._post_edit_page(page, publish_data) + + def _post_unpublish_page(self, page): + response = self.client.post( + reverse("wagtailadmin_pages:unpublish", args=[page.id]), + {}, + follow=True, + ) + return response + + def _post_delete_page(self, page): + response = self.client.post( + reverse("wagtailadmin_pages:delete", args=[page.id]), + {}, + follow=True, + ) + return response + + def _get_page_data(self, extra_data): + post_data = { + "title": "Test page!", + "slug": self._get_page_slug(), + "featured_image-count": "0", + "body": [ + ( + "rich_text_block", + { + "value": "Hello", + }, + ), + ], + "body-count": "1", + "body-0-deleted": "", + "body-0-order": "0", + "body-0-type": "text", + "body-0-value": "hello world", + } + post_data.update(self._get_featured_image_raw_data()) + if extra_data: + post_data.update(extra_data) + return post_data + def _get_featured_image_raw_data(self): - image = medias_factories.ImageFactory() return { "featured_image-count": "1", "featured_image-0-deleted": "", "featured_image-0-order": "0", "featured_image-0-type": "featured_image", "featured_image-0-id": "random-id", - "featured_image-0-value-image": image.id, + "featured_image-0-value-image": self.image.id, "featured_image-0-value-alt_text": "", "featured_image-0-value-attribution": "", "featured_image-0-value-caption": "", From 5ec6c815cf0bca964d18ed2507c48603ec282eb0 Mon Sep 17 00:00:00 2001 From: Hercilio Ortiz Date: Fri, 15 Aug 2025 09:56:12 -0300 Subject: [PATCH 12/18] bugfix: fixing the create urlpath sync --- baseapp_wagtail/base/urlpath/urlpath_sync.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/baseapp_wagtail/base/urlpath/urlpath_sync.py b/baseapp_wagtail/base/urlpath/urlpath_sync.py index 06fc799e..e4db837f 100644 --- a/baseapp_wagtail/base/urlpath/urlpath_sync.py +++ b/baseapp_wagtail/base/urlpath/urlpath_sync.py @@ -35,8 +35,9 @@ def create_urlpath(self): if not wagtail_path: return - if self.urlpath_model.objects.filter(path=wagtail_path).exists(): - wagtail_path = self._generate_unique_path(wagtail_path) + if not hasattr(self.page, "url_paths") or not self.page.url_paths.exists(): + if self.urlpath_model.objects.filter(path=wagtail_path).exists(): + wagtail_path = self._generate_unique_path(wagtail_path) try: # The created action can be triggered after the publish action when the page is created. From 91d4b96899eb99f3c94bc6d4e9430e20284b4bd5 Mon Sep 17 00:00:00 2001 From: Hercilio Ortiz Date: Fri, 15 Aug 2025 12:01:47 -0300 Subject: [PATCH 13/18] Bugfix: fixing duplicated id issue --- .../base/blocks/basic_blocks/custom_image_block/block.py | 3 +++ baseapp_wagtail/base/graphql/object_types.py | 4 ---- baseapp_wagtail/base/models.py | 8 +++----- .../base/tests/test_graphql_comments_integration.py | 2 +- 4 files changed, 7 insertions(+), 10 deletions(-) diff --git a/baseapp_wagtail/base/blocks/basic_blocks/custom_image_block/block.py b/baseapp_wagtail/base/blocks/basic_blocks/custom_image_block/block.py index 41152a96..9934ae45 100644 --- a/baseapp_wagtail/base/blocks/basic_blocks/custom_image_block/block.py +++ b/baseapp_wagtail/base/blocks/basic_blocks/custom_image_block/block.py @@ -6,6 +6,9 @@ @register_streamfield_block class CustomImageBlock(StructBlock): + # TODO (wagtail) Relay.Node creates the id based on the page model (e.g. StandardPage-{img_id}). + # This is a problem when the page has the same id as the image, causing two identical node ids for two different objects. + # The idea of this TODO is to try to fix this behavior from the baseapp_wagtail side. graphql_fields = [ GraphQLImage("image"), GraphQLString("alt_text"), diff --git a/baseapp_wagtail/base/graphql/object_types.py b/baseapp_wagtail/base/graphql/object_types.py index 29e03d51..d6849485 100644 --- a/baseapp_wagtail/base/graphql/object_types.py +++ b/baseapp_wagtail/base/graphql/object_types.py @@ -29,7 +29,6 @@ class WagtailPageObjectType(DjangoObjectType): shown in WagtailCommentsInterface. """ - id = graphene.ID(required=True) data = graphene.Field(WagtailPageInterface) class Meta: @@ -46,9 +45,6 @@ def is_type_of(cls, root, info): return True return super().is_type_of(root, info) - def resolve_id(self, info): - return self.id - def resolve_data(self, info): return self diff --git a/baseapp_wagtail/base/models.py b/baseapp_wagtail/base/models.py index dc622fcd..1fa5f89e 100644 --- a/baseapp_wagtail/base/models.py +++ b/baseapp_wagtail/base/models.py @@ -100,7 +100,9 @@ def update_url_path(self, path: str, language: Optional[str] = None, is_active: """ from baseapp_pages.utils.url_path_formatter import URLPathFormatter - primary_path = self.pages_url_path + primary_path = ( + self.pages_url_path or self.url_paths.filter(language=self.locale.language_code).first() + ) if primary_path: primary_path.path = URLPathFormatter(path)() primary_path.language = language @@ -109,10 +111,6 @@ def update_url_path(self, path: str, language: Optional[str] = None, is_active: else: self.create_url_path(path, language, is_active) - @classmethod - def get_graphql_object_type(cls): - return None - class Meta: abstract = True diff --git a/baseapp_wagtail/base/tests/test_graphql_comments_integration.py b/baseapp_wagtail/base/tests/test_graphql_comments_integration.py index 3016b59a..4b4c9f4f 100644 --- a/baseapp_wagtail/base/tests/test_graphql_comments_integration.py +++ b/baseapp_wagtail/base/tests/test_graphql_comments_integration.py @@ -90,7 +90,7 @@ def test_create_comment_on_wagtail_page(self): self.assertEqual(comment_data["body"], "This is a test comment on a Wagtail page") target = comment_data["target"] - self.assertEqual(target["id"], str(self.page.id)) + self.assertIsNotNone(target["id"], self.page.relay_id) comments_count = target["commentsCount"] self.assertEqual(comments_count["total"], 1) self.assertEqual(comments_count["replies"], 0) From f851b8593eff3666229aac5f215fe735fc270cf7 Mon Sep 17 00:00:00 2001 From: Hercilio Ortiz Date: Mon, 18 Aug 2025 12:11:16 -0300 Subject: [PATCH 14/18] freeze setuptools_scm to 8.3.1 --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index b912c978..206cd3d8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,5 +1,5 @@ [build-system] -requires = ['setuptools>=64', 'wheel', 'setuptools_scm[toml]>=8'] +requires = ['setuptools>=64', 'wheel', 'setuptools_scm[toml]==8.3.1'] build-backend = 'setuptools.build_meta' [tool.setuptools_scm] From 2b0e8482ceb8590293a9436295deecccf7fcbcac Mon Sep 17 00:00:00 2001 From: Hercilio Ortiz Date: Mon, 18 Aug 2025 14:58:31 -0300 Subject: [PATCH 15/18] Remove setuptools_scm frozen version --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 206cd3d8..1c84c764 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,5 +1,5 @@ [build-system] -requires = ['setuptools>=64', 'wheel', 'setuptools_scm[toml]==8.3.1'] +requires = ['setuptools>=80', 'wheel', 'setuptools_scm[toml]>=8'] build-backend = 'setuptools.build_meta' [tool.setuptools_scm] From d42bf037bb5545a3fb05f4a4ad2bdbc72786b15e Mon Sep 17 00:00:00 2001 From: Hercilio Ortiz Date: Tue, 19 Aug 2025 15:15:18 -0300 Subject: [PATCH 16/18] Update: Improving the sync process btw Wagtail Pages and URLPath --- baseapp_pages/models.py | 2 +- baseapp_wagtail/base/apps.py | 3 + baseapp_wagtail/base/models.py | 34 +++- baseapp_wagtail/base/signals.py | 22 +++ .../base/tests/test_graphql_urlpath_query.py | 5 - .../base/tests/test_urlpath_page_models.py | 21 +++ .../base/tests/test_urlpath_sync.py | 145 +++++++++++++--- .../base/tests/test_urlpath_sync_hooks.py | 158 +++++++----------- .../base/tests/test_urlpath_sync_signals.py | 19 +++ baseapp_wagtail/base/urlpath/urlpath_sync.py | 103 ++++++++---- baseapp_wagtail/base/wagtail_hooks.py | 20 +-- baseapp_wagtail/settings.py | 2 +- 12 files changed, 347 insertions(+), 187 deletions(-) create mode 100644 baseapp_wagtail/base/signals.py create mode 100644 baseapp_wagtail/base/tests/test_urlpath_page_models.py create mode 100644 baseapp_wagtail/base/tests/test_urlpath_sync_signals.py diff --git a/baseapp_pages/models.py b/baseapp_pages/models.py index b549cd52..61ae9d3f 100644 --- a/baseapp_pages/models.py +++ b/baseapp_pages/models.py @@ -124,7 +124,7 @@ def create_url_path(self, path: str, language: Optional[str] = None, is_active: def update_url_path(self, path: str, language: Optional[str] = None, is_active: bool = True): if not self.pk: raise ValueError("Save the instance before updating URL paths.") - primary_path = self.url_path + primary_path = self.url_path or self.url_paths.first() if primary_path: primary_path.path = URLPathFormatter(path)() primary_path.language = language diff --git a/baseapp_wagtail/base/apps.py b/baseapp_wagtail/base/apps.py index b104f017..6da2d2d3 100644 --- a/baseapp_wagtail/base/apps.py +++ b/baseapp_wagtail/base/apps.py @@ -5,3 +5,6 @@ class WagtailConfig(AppConfig): name = "baseapp_wagtail.base" verbose_name = "BaseApp Wagtail - Base" label = "baseapp_wagtail_base" + + def ready(self): + import baseapp_wagtail.base.signals # noqa: F401 diff --git a/baseapp_wagtail/base/models.py b/baseapp_wagtail/base/models.py index 1fa5f89e..0886651d 100644 --- a/baseapp_wagtail/base/models.py +++ b/baseapp_wagtail/base/models.py @@ -1,8 +1,9 @@ from typing import Optional -from urllib.parse import urlparse +from urllib.parse import urljoin, urlparse from django.apps import apps from django.conf import settings +from django.core.exceptions import ValidationError from django.db.models import Q from django.utils.translation import gettext as _ from grapple.models import GraphQLStreamfield @@ -42,6 +43,14 @@ def _has_no_domain(self, url: str) -> bool: parsed_url = urlparse(url) return not parsed_url.netloc + @classmethod + def get_front_url_path(cls, page) -> str: + url_parts = page.get_url_parts() + if not url_parts: + return None + _, _, page_path = url_parts + return page_path + class Meta: abstract = True @@ -100,9 +109,7 @@ def update_url_path(self, path: str, language: Optional[str] = None, is_active: """ from baseapp_pages.utils.url_path_formatter import URLPathFormatter - primary_path = ( - self.pages_url_path or self.url_paths.filter(language=self.locale.language_code).first() - ) + primary_path = self.pages_url_path or self.url_paths.first() if primary_path: primary_path.path = URLPathFormatter(path)() primary_path.language = language @@ -111,6 +118,25 @@ def update_url_path(self, path: str, language: Optional[str] = None, is_active: else: self.create_url_path(path, language, is_active) + def clean(self): + super().clean() + self._check_urlpath_is_unique() + + def _check_urlpath_is_unique(self): + from baseapp_wagtail.base.urlpath.urlpath_sync import WagtailURLPathSync + + parent_path = self.get_front_url_path(self.get_parent()) if self.get_parent() else "/" + path = urljoin(parent_path, self.slug) + + if WagtailURLPathSync(self).exists_urlpath(path): + raise ValidationError( + { + "slug": _( + "The url path generated from the slug is already in use by another page. Please try a different slug." + ) + } + ) + class Meta: abstract = True diff --git a/baseapp_wagtail/base/signals.py b/baseapp_wagtail/base/signals.py new file mode 100644 index 00000000..50d8140f --- /dev/null +++ b/baseapp_wagtail/base/signals.py @@ -0,0 +1,22 @@ +from django.dispatch import receiver +from wagtail.signals import page_published, page_unpublished, post_page_move + +from baseapp_wagtail.base.urlpath.urlpath_sync import WagtailURLPathSync + + +@receiver(page_published) +def update_urlpath_on_publish(sender, instance, revision, **kwargs): + if instance.scheduled_revision: + WagtailURLPathSync(instance).create_or_update_urlpath_draft() + else: + WagtailURLPathSync(instance).publish_urlpath() + + +@receiver(post_page_move) +def update_urlpath_on_move(sender, instance, **kwargs): + WagtailURLPathSync(instance.specific).update_urlpath() + + +@receiver(page_unpublished) +def deactivate_urlpath_on_unpublish(sender, instance, **kwargs): + WagtailURLPathSync(instance).deactivate_urlpath() diff --git a/baseapp_wagtail/base/tests/test_graphql_urlpath_query.py b/baseapp_wagtail/base/tests/test_graphql_urlpath_query.py index 9db4a118..51dc322d 100644 --- a/baseapp_wagtail/base/tests/test_graphql_urlpath_query.py +++ b/baseapp_wagtail/base/tests/test_graphql_urlpath_query.py @@ -1,7 +1,6 @@ from rest_framework import status from wagtail.models import Locale -from baseapp_wagtail.base.urlpath.urlpath_sync import WagtailURLPathSync from baseapp_wagtail.tests.mixins import TestPageContextMixin from baseapp_wagtail.tests.utils.graphql_helpers import GraphqlHelper from testproject.base.models import StandardPage @@ -11,7 +10,6 @@ class WagtailURLPathQueryTests(GraphqlHelper, TestPageContextMixin): def setUp(self): super().setUp() self.page.save_revision().publish() - WagtailURLPathSync(self.page).create_urlpath() self.url_path = self.page.pages_url_path def test_urlpath_query_returns_wagtail_page_data(self): @@ -96,7 +94,6 @@ def test_urlpath_query_with_standard_page(self): ) self.site.root_page.add_child(instance=standard_page) standard_page.save_revision().publish() - WagtailURLPathSync(standard_page).create_urlpath() standard_url_path = standard_page.pages_url_path response = self.query( @@ -142,7 +139,6 @@ def test_urlpath_query_with_comments_interface(self): ) self.site.root_page.add_child(instance=standard_page) standard_page.save_revision().publish() - WagtailURLPathSync(standard_page).create_urlpath() standard_url_path = standard_page.pages_url_path response = self.query( @@ -231,7 +227,6 @@ def test_urlpath_query_with_language_specific_path(self): ) self.site.root_page.add_child(instance=standard_page) standard_page.save_revision().publish() - WagtailURLPathSync(standard_page).create_urlpath() pt_url_path = standard_page.pages_url_path response = self.query( diff --git a/baseapp_wagtail/base/tests/test_urlpath_page_models.py b/baseapp_wagtail/base/tests/test_urlpath_page_models.py new file mode 100644 index 00000000..7565844b --- /dev/null +++ b/baseapp_wagtail/base/tests/test_urlpath_page_models.py @@ -0,0 +1,21 @@ +from django.core.exceptions import ValidationError + +from baseapp_pages.tests.factories import URLPathFactory +from baseapp_wagtail.tests.mixins import TestPageContextMixin + + +class URLPathPageModelsTests(TestPageContextMixin): + def test_urlpath_slug_validation_with_existing_urlpath(self): + URLPathFactory(path="/mypage") + self.page.slug = "mypage" + with self.assertRaises(ValidationError): + self.page.save() + + def test_urlpath_slug_validation_with_existing_urlpath_for_same_page(self): + URLPathFactory(path="/mypage", target=self.page) + self.page.slug = "mypage" + self.page.save() + + def test_urlpath_slug_validation_without_existing_urlpath(self): + self.page.slug = "mypage" + self.page.save() diff --git a/baseapp_wagtail/base/tests/test_urlpath_sync.py b/baseapp_wagtail/base/tests/test_urlpath_sync.py index a3d0d988..f3eb6aac 100644 --- a/baseapp_wagtail/base/tests/test_urlpath_sync.py +++ b/baseapp_wagtail/base/tests/test_urlpath_sync.py @@ -1,56 +1,130 @@ -from baseapp_pages.models import URLPath from baseapp_pages.tests.factories import URLPathFactory -from baseapp_wagtail.base.urlpath.urlpath_sync import WagtailURLPathSync +from baseapp_wagtail.base.urlpath.urlpath_sync import ( + SlugAlreadyTakenError, + WagtailURLPathSync, +) from baseapp_wagtail.tests.mixins import TestPageContextMixin class TestWagtailURLPathSyncIntegration(TestPageContextMixin): - def test_create_urlpath_creates_inactive_urlpath(self): + def test_create_or_update_urlpath_draft_creates_inactive_urlpath(self): self.page.live = False self.page.save() sync = WagtailURLPathSync(self.page) - sync.create_urlpath() - urlpath = URLPath.objects.get(path="/mypage") - self.assertEqual(urlpath.path, "/mypage") + sync.create_or_update_urlpath_draft() + + urlpath = self.page.url_paths.filter(is_active=False).first() + self.assertIsNotNone(urlpath) self.assertFalse(urlpath.is_active) self.assertEqual(urlpath.target, self.page) - def test_create_urlpath_creates_unique_path(self): - URLPathFactory(path="/mypage", is_active=False) + def test_create_or_update_urlpath_draft_raises_error_if_path_taken(self): + other_page = self.page_model( + title="Other Page", + slug="otherpage", + depth=self.site.root_page.depth + 1, + path=f"{self.site.root_page.path}0002", + ) + self.site.root_page.add_child(instance=other_page) + + URLPathFactory(path="/mypage", target=other_page, is_active=False) + sync = WagtailURLPathSync(self.page) - sync.create_urlpath() - self.assertTrue(URLPath.objects.filter(path="/mypage-1").exists()) + with self.assertRaises(SlugAlreadyTakenError): + sync.create_or_update_urlpath_draft() - def test_update_urlpath_sets_active(self): + def test_create_or_update_urlpath_draft_updates_existing_draft(self): sync = WagtailURLPathSync(self.page) - sync.create_urlpath() - self.page.save_revision().publish() - sync.update_urlpath() - urlpath = URLPath.objects.get(path="/mypage") + + sync.create_or_update_urlpath_draft() + initial_urlpath = self.page.url_paths.filter(is_active=False).first() + initial_path = initial_urlpath.path + + # Change page slug and create draft again + self.page.slug = "newslug" + self.page.save() + sync.create_or_update_urlpath_draft() + + updated_urlpath = self.page.url_paths.filter(is_active=False).first() + self.assertEqual(updated_urlpath.id, initial_urlpath.id) + self.assertNotEqual(updated_urlpath.path, initial_path) + + def test_publish_urlpath_sets_active(self): + sync = WagtailURLPathSync(self.page) + + sync.create_or_update_urlpath_draft() + self.assertFalse(self.page.url_paths.filter(is_active=True).exists()) + + sync.publish_urlpath() + + urlpath = self.page.url_paths.filter(is_active=True).first() + self.assertIsNotNone(urlpath) self.assertTrue(urlpath.is_active) + def test_publish_urlpath_deletes_old_active_paths(self): + sync = WagtailURLPathSync(self.page) + + sync.create_or_update_urlpath_draft() + sync.publish_urlpath() + initial_active = self.page.url_paths.filter(is_active=True).first() + + self.page.slug = "newslug" + self.page.save() + sync.create_or_update_urlpath_draft() + sync.publish_urlpath() + + self.assertFalse(self.page.url_paths.filter(id=initial_active.id, is_active=True).exists()) + + new_active = self.page.url_paths.filter(is_active=True).first() + self.assertIsNotNone(new_active) + self.assertNotEqual(new_active.id, initial_active.id) + def test_deactivate_urlpath_sets_inactive(self): sync = WagtailURLPathSync(self.page) - sync.create_urlpath() - self.page.save_revision().publish() - sync.update_urlpath() + + sync.create_or_update_urlpath_draft() + sync.publish_urlpath() + self.assertTrue(self.page.url_paths.filter(is_active=True).exists()) + sync.deactivate_urlpath() - urlpath = URLPath.objects.get(path="/mypage") - self.assertFalse(urlpath.is_active) + + self.assertFalse(self.page.url_paths.filter(is_active=True).exists()) def test_delete_urlpath_removes_urlpath(self): sync = WagtailURLPathSync(self.page) - sync.create_urlpath() - self.assertTrue(URLPath.objects.filter(path="/mypage").exists()) + + sync.create_or_update_urlpath_draft() + self.assertTrue(self.page.url_paths.exists()) + sync.delete_urlpath() - self.assertFalse(URLPath.objects.filter(path="/mypage").exists()) - def test_generate_unique_path(self): + self.assertFalse(self.page.url_paths.exists()) + + def test_exists_urlpath_returns_true_if_path_taken(self): + sync = WagtailURLPathSync(self.page) + + other_page = self.page_model( + title="Other Page", + slug="otherpage", + depth=self.site.root_page.depth + 1, + path=f"{self.site.root_page.path}0002", + ) + self.site.root_page.add_child(instance=other_page) + URLPathFactory(path="/mypage", target=other_page, is_active=False) + + self.assertTrue(sync.exists_urlpath("/mypage")) + + def test_exists_urlpath_returns_false_if_path_not_taken(self): + sync = WagtailURLPathSync(self.page) + + self.assertFalse(sync.exists_urlpath("/nonexistent")) + + def test_exists_urlpath_returns_false_if_path_taken_by_same_target(self): sync = WagtailURLPathSync(self.page) + URLPathFactory(path="/mypage", target=self.page, is_active=False) - URLPathFactory(path="/mypage-1", target=self.page, is_active=False) - unique_path = sync._generate_unique_path("/mypage") - self.assertEqual(unique_path, "/mypage-2") + + self.assertFalse(sync.exists_urlpath("/mypage")) def test_can_sync_true_for_page_mixin(self): sync = WagtailURLPathSync(self.page) @@ -64,3 +138,20 @@ def test_can_sync_false_if_not_urlpath_target(self): root.add_child(instance=plain_page) sync = WagtailURLPathSync(plain_page) self.assertFalse(sync._can_sync()) + + def test_can_sync_false_if_baseapp_pages_not_installed(self): + sync = WagtailURLPathSync(self.page) + self.assertTrue(sync._can_sync()) + + def test_get_wagtail_path_formats_path_correctly(self): + sync = WagtailURLPathSync(self.page) + wagtail_path = sync._get_wagtail_path() + + self.assertIsNotNone(wagtail_path) + self.assertIn("mypage", wagtail_path) + + def test_path_formatting_uses_urlpath_formatter(self): + sync = WagtailURLPathSync(self.page) + + formatted_path = sync._format_path("/test/path") + self.assertEqual(formatted_path, "/test/path") # URLPathFormatter should format this diff --git a/baseapp_wagtail/base/tests/test_urlpath_sync_hooks.py b/baseapp_wagtail/base/tests/test_urlpath_sync_hooks.py index 2de7b3c4..d80e6b7f 100644 --- a/baseapp_wagtail/base/tests/test_urlpath_sync_hooks.py +++ b/baseapp_wagtail/base/tests/test_urlpath_sync_hooks.py @@ -1,121 +1,75 @@ +from datetime import timedelta + +from django.utils import timezone from rest_framework import status -from baseapp_pages.models import URLPath from baseapp_wagtail.tests.mixins import TestAdminActionsMixin -from baseapp_wagtail.tests.models import PageForTests class URLPathSyncHooksTests(TestAdminActionsMixin): - def test_urlpath_creation_on_page_create(self): - response = self._post_new_page({"slug": "test-page-2"}) - self.assertEqual(response.status_code, status.HTTP_200_OK) - - new_page = self._get_page_by_slug("test-page-2") - self.assertIsNotNone(new_page) - - urlpath = new_page.pages_url_path - self.assertIsNone(urlpath) - - urlpath = URLPath.objects.get(path="/mypage/test-page-2") - self.assertEqual(urlpath.path, "/mypage/test-page-2") - self.assertFalse(urlpath.is_active) - - def test_urlpath_creation_on_page_create_and_publish(self): - response = self._post_new_page({"action-publish": "action-publish", "slug": "test-page-2"}) - self.assertEqual(response.status_code, status.HTTP_200_OK) - - new_page = self._get_page_by_slug("test-page-2") - self.assertIsNotNone(new_page) - - urlpath = new_page.pages_url_path - self.assertIsNotNone(urlpath) - self.assertEqual(urlpath.path, "/mypage/test-page-2") - self.assertTrue(urlpath.is_active) - - def test_urlpath_update_on_page_publish(self): - self.assertIsNone(self.page.pages_url_path) - - response = self._post_publish_page(self.page) + def test_scheduled_publish_creates_scheduled_revision(self): + go_live_at = timezone.now() + timedelta(hours=1) + + response = self._post_publish_page( + self.page, + { + "slug": "scheduled-page", + "go_live_at": go_live_at.strftime("%Y-%m-%d %H:%M:%S"), + }, + ) self.assertEqual(response.status_code, status.HTTP_200_OK) self._reload_the_page() - urlpath = self.page.pages_url_path - self.assertTrue(urlpath.is_active) - - def test_urlpath_deactivation_on_page_unpublish(self): - response = self._post_publish_page(self.page) - self.assertEqual(response.status_code, status.HTTP_200_OK) - - self._reload_the_page() + self.assertIsNotNone(self.page.scheduled_revision) + self.assertEqual(self.page.slug, "scheduled-page") - urlpath = self.page.pages_url_path - self.assertTrue(urlpath.is_active) + def test_scheduled_publish_hook_integration(self): + go_live_at = timezone.now() + timedelta(hours=1) - response = self._post_unpublish_page(self.page) + response = self._post_publish_page( + self.page, + { + "slug": "hook-test-page", + "go_live_at": go_live_at.strftime("%Y-%m-%d %H:%M:%S"), + }, + ) self.assertEqual(response.status_code, status.HTTP_200_OK) self._reload_the_page() - urlpath = self.page.pages_url_path - self.assertIsNone(urlpath) - - urlpath = URLPath.objects.get(path="/test-page") - self.assertFalse(urlpath.is_active) - - def test_urlpath_deletion_on_page_delete(self): - response = self._post_new_page({"action-publish": "action-publish", "slug": "test-page-2"}) - self.assertEqual(response.status_code, status.HTTP_200_OK) - - page = self._get_page_by_slug("test-page-2") + self.assertIsNotNone(self.page.scheduled_revision) + + urlpath_draft = self.page.url_paths.filter(is_active=False).first() + self.assertIsNotNone(urlpath_draft) + self.assertFalse(urlpath_draft.is_active) + + def test_scheduled_publish_hook_handles_multiple_scheduled_pages(self): + go_live_at_1 = timezone.now() + timedelta(hours=1) + go_live_at_2 = timezone.now() + timedelta(hours=2) + + response1 = self._post_publish_page( + self.page, + { + "slug": "scheduled-page-1", + "go_live_at": go_live_at_1.strftime("%Y-%m-%d %H:%M:%S"), + }, + ) + self.assertEqual(response1.status_code, status.HTTP_200_OK) + + response2 = self._post_publish_page( + self.page, + { + "slug": "scheduled-page-2", + "go_live_at": go_live_at_2.strftime("%Y-%m-%d %H:%M:%S"), + }, + ) + self.assertEqual(response2.status_code, status.HTTP_200_OK) - urlpath = page.pages_url_path - self.assertTrue(urlpath.is_active) - - response = self._post_delete_page(page) - self.assertEqual(response.status_code, status.HTTP_200_OK) - - self.assertFalse(PageForTests.objects.filter(id=page.id).exists()) - - urlpath = URLPath.objects.filter(path="/mypage/test-page-2").exists() - self.assertFalse(urlpath) - - def test_editting_page_already_published_not_changing_existing_urlpath(self): - response = self._post_new_page({"action-publish": "action-publish", "slug": "test-page-2"}) - self.assertEqual(response.status_code, status.HTTP_200_OK) - - page = self._get_page_by_slug("test-page-2") - - urlpath = page.pages_url_path - self.assertTrue(urlpath.is_active) - - edit_data = { - "title": "Updated Page Title", - "slug": "updated-page-slug", - } - - response = self._post_edit_page(page, edit_data) - self.assertEqual(response.status_code, status.HTTP_200_OK) - - urlpath = page.pages_url_path - self.assertEqual(urlpath.path, "/mypage/test-page-2") - - def test_urlpath_path_update_on_page_edit(self): - response = self._post_new_page({"action-publish": "action-publish", "slug": "test-page-2"}) - self.assertEqual(response.status_code, status.HTTP_200_OK) - - page = self._get_page_by_slug("test-page-2") - - urlpath = page.pages_url_path - self.assertTrue(urlpath.is_active) - - edit_data = { - "title": "Updated Page Title", - "slug": "updated-page-slug", - } + self._reload_the_page() - response = self._post_publish_page(page, edit_data) - self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertIsNotNone(self.page.scheduled_revision) - urlpath = page.pages_url_path - self.assertEqual(urlpath.path, "/mypage/updated-page-slug") + urlpath_draft = self.page.url_paths.filter(is_active=False).first() + self.assertIsNotNone(urlpath_draft) + self.assertFalse(urlpath_draft.is_active) diff --git a/baseapp_wagtail/base/tests/test_urlpath_sync_signals.py b/baseapp_wagtail/base/tests/test_urlpath_sync_signals.py new file mode 100644 index 00000000..6f13e8e5 --- /dev/null +++ b/baseapp_wagtail/base/tests/test_urlpath_sync_signals.py @@ -0,0 +1,19 @@ +from baseapp_wagtail.base.urlpath.urlpath_sync import WagtailURLPathSync +from baseapp_wagtail.tests.mixins import TestPageContextMixin + + +class URLPathSyncSignalsTests(TestPageContextMixin): + def test_publish_page_creates_urlpath(self): + self.page.save_revision().publish() + self._reload_the_page() + + urlpath = self.page.url_paths.filter(is_active=True).first() + self.assertIsNotNone(urlpath) + self.assertEqual(urlpath.path, "/mypage") + + def test_publish_page_with_existing_urlpath_drafs(self): + WagtailURLPathSync(self.page).create_or_update_urlpath_draft() + self.assertEqual(self.page.url_paths.filter(is_active=False).count(), 1) + + self.page.save_revision().publish() + self.assertEqual(self.page.url_paths.all().count(), 1) diff --git a/baseapp_wagtail/base/urlpath/urlpath_sync.py b/baseapp_wagtail/base/urlpath/urlpath_sync.py index e4db837f..8622dcbc 100644 --- a/baseapp_wagtail/base/urlpath/urlpath_sync.py +++ b/baseapp_wagtail/base/urlpath/urlpath_sync.py @@ -1,19 +1,27 @@ import logging from typing import Optional +from urllib.parse import urljoin from django.apps import apps from django.db.models import Model -from wagtail.models import Page + +from baseapp_wagtail.base.models import DefaultPageModel logger = logging.getLogger(__name__) +class SlugAlreadyTakenError(Exception): + """Exception raised when a slug/path is already taken by a different target.""" + + pass + + class WagtailURLPathSync: - page: Page + page: DefaultPageModel urlpath_model: Optional[Model] is_baseapp_pages_installed: bool - def __init__(self, page: Page): + def __init__(self, page: DefaultPageModel): self.page = page self.urlpath_model = None self.is_baseapp_pages_installed = apps.is_installed("baseapp_pages") @@ -27,7 +35,7 @@ def _load_urlpath_model(self): else: self.urlpath_model = None - def create_urlpath(self): + def create_or_update_urlpath_draft(self): if not self._can_sync(): return @@ -35,27 +43,44 @@ def create_urlpath(self): if not wagtail_path: return - if not hasattr(self.page, "url_paths") or not self.page.url_paths.exists(): - if self.urlpath_model.objects.filter(path=wagtail_path).exists(): - wagtail_path = self._generate_unique_path(wagtail_path) + if self._is_path_taken_by_different_target(wagtail_path): + raise SlugAlreadyTakenError(f"Slug '{wagtail_path}' is already taken by another page") try: - # The created action can be triggered after the publish action when the page is created. - self.page.update_url_path( - path=wagtail_path, language=self.page.locale.language_code, is_active=self.page.live - ) + live_version = self.page.url_paths.filter(is_active=True).first() + if not live_version: + self.page.update_url_path( + path=wagtail_path, language=self.page.locale.language_code, is_active=False + ) + elif live_version.path != wagtail_path: + if url_path := self.page.url_paths.filter(is_active=False).first(): + url_path.path = wagtail_path + url_path.language = self.page.locale.language_code + url_path.is_active = False + url_path.save() + else: + self.page.create_url_path( + path=wagtail_path, language=self.page.locale.language_code, is_active=False + ) except Exception as e: logger.error(f"(Wagtail urlpath sync) Error creating urlpath: {e}") return - def deactivate_urlpath(self): + def publish_urlpath(self): if not self._can_sync(): return + wagtail_path = self._get_wagtail_path() + if not wagtail_path: + return + try: - self.page.deactivate_url_paths() + self.page.url_paths.filter(is_active=True).delete() + self.page.update_url_path( + path=wagtail_path, language=self.page.locale.language_code, is_active=True + ) except Exception as e: - logger.error(f"(Wagtail urlpath sync) Error deactivating urlpath: {e}") + logger.error(f"(Wagtail urlpath sync) Error publishing urlpath: {e}") return def update_urlpath(self): @@ -74,6 +99,16 @@ def update_urlpath(self): logger.error(f"(Wagtail urlpath sync) Error updating urlpath: {e}") return + def deactivate_urlpath(self): + if not self._can_sync(): + return + + try: + self.page.deactivate_url_paths() + except Exception as e: + logger.error(f"(Wagtail urlpath sync) Error deactivating urlpath: {e}") + return + def delete_urlpath(self): if not self._can_sync(): return @@ -84,6 +119,14 @@ def delete_urlpath(self): logger.error(f"(Wagtail urlpath sync) Error deleting urlpath: {e}") return + def exists_urlpath(self, path: str) -> bool: + if not self._can_sync(): + return False + + path = self._format_path(path) + + return self._is_path_taken_by_different_target(path) + def _can_sync(self) -> bool: return ( self.is_baseapp_pages_installed and self._is_available() and self._is_urlpath_target() @@ -97,23 +140,23 @@ def _is_urlpath_target(self) -> bool: return isinstance(self.page, PageMixin) - def _get_wagtail_path(self) -> Optional[str]: - url_parts = self.page.get_url_parts() - if not url_parts: - return None - _, _, page_path = url_parts - if self.is_baseapp_pages_installed: - from baseapp_pages.utils.url_path_formatter import URLPathFormatter + def _is_path_taken_by_different_target(self, path: str) -> bool: + path = self._format_path(path) - return URLPathFormatter(page_path)() - return page_path + existing_urlpath = self.urlpath_model.objects.filter(path=path).first() + if existing_urlpath and existing_urlpath.target != self.page: + return True - def _generate_unique_path(self, base_path: str) -> str: - counter = 1 - new_path = base_path + return False - while self.urlpath_model.objects.filter(path=new_path).exists(): - new_path = f"{base_path}-{counter}" - counter += 1 + def _format_path(self, path: str) -> str: + if self.is_baseapp_pages_installed: + from baseapp_pages.utils.url_path_formatter import URLPathFormatter - return new_path + return URLPathFormatter(path)() + return path + + def _get_wagtail_path(self) -> Optional[str]: + parent_path = self.page.get_front_url_path(self.page.get_parent()) + page_path = urljoin(parent_path, self.page.slug) + return self._format_path(page_path) diff --git a/baseapp_wagtail/base/wagtail_hooks.py b/baseapp_wagtail/base/wagtail_hooks.py index 252030ee..e572e646 100644 --- a/baseapp_wagtail/base/wagtail_hooks.py +++ b/baseapp_wagtail/base/wagtail_hooks.py @@ -13,24 +13,10 @@ def register_schema_query(query_mixins): query_mixins.remove(query_mixin) -@hooks.register("after_create_page") -def create_urlpath_for_page(request, page): - WagtailURLPathSync(page).create_urlpath() - - @hooks.register("after_publish_page") -def update_urlpath_on_publish(request, page): - WagtailURLPathSync(page).update_urlpath() - - -@hooks.register("after_unpublish_page") -def deactivate_urlpath_on_unpublish(request, page): - WagtailURLPathSync(page).deactivate_urlpath() - - -@hooks.register("after_delete_page") -def delete_urlpath_on_delete(request, page): - WagtailURLPathSync(page).delete_urlpath() +def save_urlpath_draft_on_schedule_publish(request, page): + if page.scheduled_revision: + WagtailURLPathSync(page.scheduled_revision.as_object()).create_or_update_urlpath_draft() @hooks.register("register_rich_text_features") diff --git a/baseapp_wagtail/settings.py b/baseapp_wagtail/settings.py index 09df7613..f0211fe5 100644 --- a/baseapp_wagtail/settings.py +++ b/baseapp_wagtail/settings.py @@ -56,7 +56,7 @@ if "FRONT_URL" not in globals(): FRONT_URL = env("FRONT_URL", "", required=False) -FRONT_HEADLESS_URL = urljoin(FRONT_URL, env("WAGTAIL_FRONT_URL_PATH", default="/pages")) +FRONT_HEADLESS_URL = urljoin(FRONT_URL, env("WAGTAIL_FRONT_URL_PATH", default="/")) FRONT_PAGE_PREVIEW_URL = urljoin( FRONT_URL, env("WAGTAIL_FRONT_PAGE_PREVIEW_URL_PATH", default="/page-preview") ) From 7e7c709668d272c9607def2e72f86590c60172fc Mon Sep 17 00:00:00 2001 From: Hercilio Ortiz Date: Tue, 19 Aug 2025 15:27:20 -0300 Subject: [PATCH 17/18] Refactor: changing name of the urlpath_exists method --- baseapp_wagtail/base/models.py | 2 +- baseapp_wagtail/base/tests/test_urlpath_sync.py | 12 ++++++------ baseapp_wagtail/base/urlpath/urlpath_sync.py | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/baseapp_wagtail/base/models.py b/baseapp_wagtail/base/models.py index 0886651d..fc4e2851 100644 --- a/baseapp_wagtail/base/models.py +++ b/baseapp_wagtail/base/models.py @@ -128,7 +128,7 @@ def _check_urlpath_is_unique(self): parent_path = self.get_front_url_path(self.get_parent()) if self.get_parent() else "/" path = urljoin(parent_path, self.slug) - if WagtailURLPathSync(self).exists_urlpath(path): + if WagtailURLPathSync(self).urlpath_exists(path): raise ValidationError( { "slug": _( diff --git a/baseapp_wagtail/base/tests/test_urlpath_sync.py b/baseapp_wagtail/base/tests/test_urlpath_sync.py index f3eb6aac..4d11383e 100644 --- a/baseapp_wagtail/base/tests/test_urlpath_sync.py +++ b/baseapp_wagtail/base/tests/test_urlpath_sync.py @@ -100,7 +100,7 @@ def test_delete_urlpath_removes_urlpath(self): self.assertFalse(self.page.url_paths.exists()) - def test_exists_urlpath_returns_true_if_path_taken(self): + def test_urlpath_exists_returns_true_if_path_taken(self): sync = WagtailURLPathSync(self.page) other_page = self.page_model( @@ -112,19 +112,19 @@ def test_exists_urlpath_returns_true_if_path_taken(self): self.site.root_page.add_child(instance=other_page) URLPathFactory(path="/mypage", target=other_page, is_active=False) - self.assertTrue(sync.exists_urlpath("/mypage")) + self.assertTrue(sync.urlpath_exists("/mypage")) - def test_exists_urlpath_returns_false_if_path_not_taken(self): + def test_urlpath_exists_returns_false_if_path_not_taken(self): sync = WagtailURLPathSync(self.page) - self.assertFalse(sync.exists_urlpath("/nonexistent")) + self.assertFalse(sync.urlpath_exists("/nonexistent")) - def test_exists_urlpath_returns_false_if_path_taken_by_same_target(self): + def test_urlpath_exists_returns_false_if_path_taken_by_same_target(self): sync = WagtailURLPathSync(self.page) URLPathFactory(path="/mypage", target=self.page, is_active=False) - self.assertFalse(sync.exists_urlpath("/mypage")) + self.assertFalse(sync.urlpath_exists("/mypage")) def test_can_sync_true_for_page_mixin(self): sync = WagtailURLPathSync(self.page) diff --git a/baseapp_wagtail/base/urlpath/urlpath_sync.py b/baseapp_wagtail/base/urlpath/urlpath_sync.py index 8622dcbc..79bd4f2d 100644 --- a/baseapp_wagtail/base/urlpath/urlpath_sync.py +++ b/baseapp_wagtail/base/urlpath/urlpath_sync.py @@ -119,7 +119,7 @@ def delete_urlpath(self): logger.error(f"(Wagtail urlpath sync) Error deleting urlpath: {e}") return - def exists_urlpath(self, path: str) -> bool: + def urlpath_exists(self, path: str) -> bool: if not self._can_sync(): return False From 505e2991a6d542052ea58c3482d6ba66a1cccb27 Mon Sep 17 00:00:00 2001 From: Hercilio Ortiz Date: Tue, 19 Aug 2025 23:07:52 -0300 Subject: [PATCH 18/18] Fixing typo --- baseapp_wagtail/base/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/baseapp_wagtail/base/models.py b/baseapp_wagtail/base/models.py index fc4e2851..b5538f8b 100644 --- a/baseapp_wagtail/base/models.py +++ b/baseapp_wagtail/base/models.py @@ -96,7 +96,7 @@ class DefaultPageModel( def pages_url_path(self): """ baseapp_pages.models.PageMixin.url_path alternative. - Defines a new property because wagtail pages are have have a defined "url_path" property. + Defines a new property because wagtail pages already have a defined "url_path" property. """ return self.url_paths.filter( Q(is_active=True), Q(language=self.locale.language_code) | Q(language__isnull=True)