From a1f6bd8aad1499f81bc5a0b6c064ba9458cbc19b Mon Sep 17 00:00:00 2001 From: Kathryn Dale Date: Tue, 23 Jun 2026 11:52:37 +0100 Subject: [PATCH 1/6] WIP: Filter pages --- cms/dynamic_content/blocks.py | 74 ++++++++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 1 deletion(-) diff --git a/cms/dynamic_content/blocks.py b/cms/dynamic_content/blocks.py index e57afd8ba..a3d7b9cba 100644 --- a/cms/dynamic_content/blocks.py +++ b/cms/dynamic_content/blocks.py @@ -2,6 +2,7 @@ from django.db import models from wagtail import blocks from wagtail.blocks import ( + BooleanBlock, CharBlock, ChoiceBlock, PageChooserBlock, @@ -30,6 +31,36 @@ METRIC_NUMBER_BLOCK_DATE_PREFIX_DEFAULT_TEXT = "Up to" + +def check_permissions(user_permissions, theme_id, sub_theme_id, topic_id) -> bool: + if not isinstance(user_permissions, list): + return False + + for permission in user_permissions: + permission_theme_id = permission.get("theme", {}).get("id") + permission_sub_theme_id = permission.get("sub_theme", {}).get("id") + permission_topic_id = permission.get("topic", {}).get("id") + + if permission_theme_id == "-1": + return True + + if ( + permission_theme_id == theme_id + and permission_sub_theme_id == "-1" + ): + return True + + if ( + permission_theme_id == theme_id + and permission_sub_theme_id == sub_theme_id + and (permission_topic_id in {"-1", topic_id}) + ): + return True + + return False + + + class HeadlineNumberBlockTypes(StreamBlock): headline_number = HeadlineNumberComponent(help_text=help_texts.HEADLINE_BLOCK_FIELD) trend_number = TrendNumberComponent(help_text=help_texts.TREND_BLOCK_FIELD) @@ -72,7 +103,6 @@ class PageLinkChooserBlock(PageChooserBlock): def get_api_representation(cls, value, context=None) -> str | None: if value: return value.full_url - return None @@ -205,11 +235,53 @@ class PageLink(StructBlock): help_text=help_texts.PAGE_LINK_SUB_TITLE, ) page = PageLinkChooserBlock(target_model=["topic.TopicPage"]) + + + def get_api_representation(self, value, context=None): + page = value.get("page") + if not page: + return None + + request = context.get("request") if context else None + user = getattr(request, "user", None) + user_permissions = getattr(user, "permission_sets", None) + full_user_permissions = user_permissions.permission_sets["permission_sets"] + + topic_page_details = value.specific + + if getattr(topic_page_details, "is_public", False): + page_theme = getattr(topic_page_details, "theme", None) + page_sub_theme = getattr(topic_page_details, "sub_theme", None) + page_topic = getattr(topic_page_details, "topic", None) + print(f"🦄 theme: {page_theme}") + print(f"🦄🦄 full_user_permissions: {full_user_permissions}") + + if not check_permissions( + full_user_permissions, + page_theme, + page_sub_theme, + page_topic + ): + return None + + return { + "title": value.get("title"), + "url": topic_page_details.full_url, + } + class InternalPageLinks(StreamBlock): page_link = PageLink() + + + def get_api_representation(self, value, context=None): + data = super().get_api_representation(value, context=context) + + # Remove filtered-out items (None) + return [item for item in data if item is not None] + class Meta: icon = "link" From 650f115df3e160babd02a565446d20c64d228087 Mon Sep 17 00:00:00 2001 From: Kathryn Dale Date: Wed, 24 Jun 2026 10:05:43 +0100 Subject: [PATCH 2/6] WIP: Set names to blank for filtered pages --- cms/dynamic_content/blocks.py | 88 ++++++++++++++++------------------- 1 file changed, 41 insertions(+), 47 deletions(-) diff --git a/cms/dynamic_content/blocks.py b/cms/dynamic_content/blocks.py index a3d7b9cba..12ca701e9 100644 --- a/cms/dynamic_content/blocks.py +++ b/cms/dynamic_content/blocks.py @@ -31,7 +31,6 @@ METRIC_NUMBER_BLOCK_DATE_PREFIX_DEFAULT_TEXT = "Up to" - def check_permissions(user_permissions, theme_id, sub_theme_id, topic_id) -> bool: if not isinstance(user_permissions, list): return False @@ -44,10 +43,7 @@ def check_permissions(user_permissions, theme_id, sub_theme_id, topic_id) -> boo if permission_theme_id == "-1": return True - if ( - permission_theme_id == theme_id - and permission_sub_theme_id == "-1" - ): + if permission_theme_id == theme_id and permission_sub_theme_id == "-1": return True if ( @@ -60,7 +56,6 @@ def check_permissions(user_permissions, theme_id, sub_theme_id, topic_id) -> boo return False - class HeadlineNumberBlockTypes(StreamBlock): headline_number = HeadlineNumberComponent(help_text=help_texts.HEADLINE_BLOCK_FIELD) trend_number = TrendNumberComponent(help_text=help_texts.TREND_BLOCK_FIELD) @@ -235,53 +230,52 @@ class PageLink(StructBlock): help_text=help_texts.PAGE_LINK_SUB_TITLE, ) page = PageLinkChooserBlock(target_model=["topic.TopicPage"]) - + def get_api_representation(self, value, context=None): - page = value.get("page") - if not page: - return None - - request = context.get("request") if context else None - user = getattr(request, "user", None) - user_permissions = getattr(user, "permission_sets", None) - full_user_permissions = user_permissions.permission_sets["permission_sets"] - - topic_page_details = value.specific - - if getattr(topic_page_details, "is_public", False): - page_theme = getattr(topic_page_details, "theme", None) - page_sub_theme = getattr(topic_page_details, "sub_theme", None) - page_topic = getattr(topic_page_details, "topic", None) - print(f"🦄 theme: {page_theme}") - print(f"🦄🦄 full_user_permissions: {full_user_permissions}") - - if not check_permissions( - full_user_permissions, - page_theme, - page_sub_theme, - page_topic - ): - return None - - return { - "title": value.get("title"), - "url": topic_page_details.full_url, - } - - + data = super().get_api_representation(value, context) + print(f"🦄 data: {data}") + page = value.get("page") + print(f"🦄 page: {page}") + + if not page: + data["title"] = "" + data["sub_title"] = "" + return data + + print(f"🦄 page.specific: {page.specific}") + page = page.specific + print(f"🦄 page.is_public: {page.is_public}") + + request = context.get("request") if context else None + user = getattr(request, "user", None) + print(f"🦄 request: {request}") + print(f"🦄 user: {user}") + + user_permissions = getattr(user, "permission_sets", None) + print(f"🦄 user_permissions: {user_permissions}") + full_user_permissions = ( + user_permissions.permission_sets.get("permission_sets") + if user_permissions and hasattr(user_permissions, "permission_sets") + else None + ) + + if not page.is_public: + if not check_permissions( + full_user_permissions, + getattr(page, "theme", None), + getattr(page, "sub_theme", None), + getattr(page, "topic", None), + ): + data["title"] = "" + data["sub_title"] = "" + return data + + return data class InternalPageLinks(StreamBlock): page_link = PageLink() - - - def get_api_representation(self, value, context=None): - data = super().get_api_representation(value, context=context) - # Remove filtered-out items (None) - return [item for item in data if item is not None] - - class Meta: icon = "link" From 02ca1852600f3f5297f383a4fbe013a4b5030c26 Mon Sep 17 00:00:00 2001 From: Megan Bower Date: Wed, 24 Jun 2026 15:38:10 +0100 Subject: [PATCH 3/6] CDD-3441 Add authorised flag to page --- cms/dynamic_content/blocks.py | 39 ++++++++++++++--------------------- 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/cms/dynamic_content/blocks.py b/cms/dynamic_content/blocks.py index 12ca701e9..6d1ac4776 100644 --- a/cms/dynamic_content/blocks.py +++ b/cms/dynamic_content/blocks.py @@ -231,47 +231,38 @@ class PageLink(StructBlock): ) page = PageLinkChooserBlock(target_model=["topic.TopicPage"]) - def get_api_representation(self, value, context=None): - data = super().get_api_representation(value, context) - print(f"🦄 data: {data}") - page = value.get("page") - print(f"🦄 page: {page}") + data = super().get_api_representation(value, context) + page = value.get("page") - if not page: - data["title"] = "" - data["sub_title"] = "" - return data + if not page: + data["is_authorised"] = False + return data - print(f"🦄 page.specific: {page.specific}") - page = page.specific - print(f"🦄 page.is_public: {page.is_public}") + page = page.specific + request = context.get("request") if context else None + user = getattr(request, "user", None) - request = context.get("request") if context else None - user = getattr(request, "user", None) - print(f"🦄 request: {request}") - print(f"🦄 user: {user}") + if not page.is_public: user_permissions = getattr(user, "permission_sets", None) - print(f"🦄 user_permissions: {user_permissions}") full_user_permissions = ( user_permissions.permission_sets.get("permission_sets") if user_permissions and hasattr(user_permissions, "permission_sets") else None ) - - if not page.is_public: - if not check_permissions( + if not check_permissions( full_user_permissions, getattr(page, "theme", None), getattr(page, "sub_theme", None), getattr(page, "topic", None), ): - data["title"] = "" - data["sub_title"] = "" - return data + data["is_authorised"] = False + return data + + data["is_authorised"] = True + return data - return data class InternalPageLinks(StreamBlock): page_link = PageLink() From dc4bd4bce44336725991dab0b923f1504cc8c74d Mon Sep 17 00:00:00 2001 From: Megan Bower Date: Wed, 24 Jun 2026 16:00:59 +0100 Subject: [PATCH 4/6] CDD-3441 Clear data when unauthorised --- cms/dynamic_content/blocks.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cms/dynamic_content/blocks.py b/cms/dynamic_content/blocks.py index 6d1ac4776..791a3ad52 100644 --- a/cms/dynamic_content/blocks.py +++ b/cms/dynamic_content/blocks.py @@ -237,6 +237,9 @@ def get_api_representation(self, value, context=None): if not page: data["is_authorised"] = False + data["title"] = "" + data["subtitle"] = "" + data["page"] = "" return data page = page.specific @@ -258,6 +261,9 @@ def get_api_representation(self, value, context=None): getattr(page, "topic", None), ): data["is_authorised"] = False + data["title"] = "" + data["subtitle"] = "" + data["page"] = "" return data data["is_authorised"] = True From e930b7765f5a2c14483e7b9453b5ce834461edbd Mon Sep 17 00:00:00 2001 From: Megan Bower Date: Thu, 25 Jun 2026 16:41:49 +0100 Subject: [PATCH 5/6] CDD-3441 Move check_permissions to shared file --- cms/dashboard/permissions.py | 29 +++++++++++++++++++++ cms/dashboard/viewsets.py | 30 +--------------------- cms/dynamic_content/blocks.py | 31 +++-------------------- tests/unit/cms/dashboard/test_viewsets.py | 2 +- 4 files changed, 35 insertions(+), 57 deletions(-) create mode 100644 cms/dashboard/permissions.py diff --git a/cms/dashboard/permissions.py b/cms/dashboard/permissions.py new file mode 100644 index 000000000..2635affb7 --- /dev/null +++ b/cms/dashboard/permissions.py @@ -0,0 +1,29 @@ +from cms.auth_content.constants import WILDCARD_ID_VALUE + + +def check_permissions(user_permissions, theme_id, sub_theme_id, topic_id) -> bool: + if not isinstance(user_permissions, list): + return False + + for permission in user_permissions: + permission_theme_id = permission.get("theme", {}).get("id") + permission_sub_theme_id = permission.get("sub_theme", {}).get("id") + permission_topic_id = permission.get("topic", {}).get("id") + + if permission_theme_id == WILDCARD_ID_VALUE: + return True + + if ( + permission_theme_id == theme_id + and permission_sub_theme_id == WILDCARD_ID_VALUE + ): + return True + + if ( + permission_theme_id == theme_id + and permission_sub_theme_id == sub_theme_id + and (permission_topic_id in {WILDCARD_ID_VALUE, topic_id}) + ): + return True + + return False \ No newline at end of file diff --git a/cms/dashboard/viewsets.py b/cms/dashboard/viewsets.py index 9bb4669fa..887f7fe9b 100644 --- a/cms/dashboard/viewsets.py +++ b/cms/dashboard/viewsets.py @@ -10,7 +10,7 @@ from caching.private_api.decorators import cache_response from cms.auth_content.auth_utils import is_auth_enabled -from cms.auth_content.constants import WILDCARD_ID_VALUE +from cms.dashboard.permissions import check_permissions from cms.dashboard.serializers import CMSDraftPagesSerializer, ListablePageSerializer from cms.metrics_documentation.models.child import MetricsDocumentationChildEntry from cms.topic.models import TopicPage @@ -19,34 +19,6 @@ AUTH_ENABLED = is_auth_enabled() -def check_permissions(user_permissions, theme_id, sub_theme_id, topic_id) -> bool: - if not isinstance(user_permissions, list): - return False - - for permission in user_permissions: - permission_theme_id = permission.get("theme", {}).get("id") - permission_sub_theme_id = permission.get("sub_theme", {}).get("id") - permission_topic_id = permission.get("topic", {}).get("id") - - if permission_theme_id == WILDCARD_ID_VALUE: - return True - - if ( - permission_theme_id == theme_id - and permission_sub_theme_id == WILDCARD_ID_VALUE - ): - return True - - if ( - permission_theme_id == theme_id - and permission_sub_theme_id == sub_theme_id - and (permission_topic_id in {WILDCARD_ID_VALUE, topic_id}) - ): - return True - - return False - - @extend_schema(tags=["cms"]) class CMSPagesAPIViewSet(PagesAPIViewSet): # This is the /pages (or proxy/pages env dependent endpoint) diff --git a/cms/dynamic_content/blocks.py b/cms/dynamic_content/blocks.py index 791a3ad52..051122f4a 100644 --- a/cms/dynamic_content/blocks.py +++ b/cms/dynamic_content/blocks.py @@ -14,6 +14,8 @@ ) from wagtail.snippets.blocks import SnippetChooserBlock + +from cms.dashboard.permissions import check_permissions from cms.dynamic_content import help_texts from cms.dynamic_content.components import ( HeadlineNumberComponent, @@ -31,31 +33,6 @@ METRIC_NUMBER_BLOCK_DATE_PREFIX_DEFAULT_TEXT = "Up to" -def check_permissions(user_permissions, theme_id, sub_theme_id, topic_id) -> bool: - if not isinstance(user_permissions, list): - return False - - for permission in user_permissions: - permission_theme_id = permission.get("theme", {}).get("id") - permission_sub_theme_id = permission.get("sub_theme", {}).get("id") - permission_topic_id = permission.get("topic", {}).get("id") - - if permission_theme_id == "-1": - return True - - if permission_theme_id == theme_id and permission_sub_theme_id == "-1": - return True - - if ( - permission_theme_id == theme_id - and permission_sub_theme_id == sub_theme_id - and (permission_topic_id in {"-1", topic_id}) - ): - return True - - return False - - class HeadlineNumberBlockTypes(StreamBlock): headline_number = HeadlineNumberComponent(help_text=help_texts.HEADLINE_BLOCK_FIELD) trend_number = TrendNumberComponent(help_text=help_texts.TREND_BLOCK_FIELD) @@ -238,7 +215,7 @@ def get_api_representation(self, value, context=None): if not page: data["is_authorised"] = False data["title"] = "" - data["subtitle"] = "" + data["sub_title"] = "" data["page"] = "" return data @@ -262,7 +239,7 @@ def get_api_representation(self, value, context=None): ): data["is_authorised"] = False data["title"] = "" - data["subtitle"] = "" + data["sub_title"] = "" data["page"] = "" return data diff --git a/tests/unit/cms/dashboard/test_viewsets.py b/tests/unit/cms/dashboard/test_viewsets.py index 165ccb266..a436a79c1 100644 --- a/tests/unit/cms/dashboard/test_viewsets.py +++ b/tests/unit/cms/dashboard/test_viewsets.py @@ -1,10 +1,10 @@ import pytest +from cms.dashboard.permissions import check_permissions from cms.dashboard.serializers import CMSDraftPagesSerializer, ListablePageSerializer from cms.dashboard.viewsets import ( CMSDraftPagesViewSet, CMSPagesAPIViewSet, - check_permissions, ) From e795750db6695a0d121b1ded6e601851c2a817dc Mon Sep 17 00:00:00 2001 From: Megan Bower Date: Thu, 25 Jun 2026 17:06:59 +0100 Subject: [PATCH 6/6] CDD-3441 Add unit tests for page link blocks --- tests/unit/cms/dynamic_content/test_blocks.py | 172 +++++++++++++++++- 1 file changed, 171 insertions(+), 1 deletion(-) diff --git a/tests/unit/cms/dynamic_content/test_blocks.py b/tests/unit/cms/dynamic_content/test_blocks.py index fe4fb9d79..734be9348 100644 --- a/tests/unit/cms/dynamic_content/test_blocks.py +++ b/tests/unit/cms/dynamic_content/test_blocks.py @@ -6,7 +6,7 @@ import pytest from wagtail.blocks import StructBlock, StructValue -from cms.dynamic_content.blocks import SourceLinkBlock +from cms.dynamic_content.blocks import PageLink, SourceLinkBlock class TestSourceLinkBlockClean: @@ -133,3 +133,173 @@ def test_does_not_raise_when_only_external_url_set(self): ) SourceLinkBlock._validate_only_one_of_page_or_external_url(value=value) + + +class TestPageLinkBlock: + """Tests for PageLink.get_api_representation().""" + + def test_no_page_returns_unauthorised(self): + """ + Given a value with no page set + When get_api_representation() is called + Then the response is unauthorised and fields are blanked. + """ + block = PageLink() + value = { + "title": "Test title", + "sub_title": "Test subtitle", + "page": None, + } + + result = block.get_api_representation(value=value, context={}) + + assert result["is_authorised"] is False + assert result["title"] == "" + assert result["sub_title"] == "" + assert result["page"] == "" + + def test_public_page_is_always_authorised(self): + """ + Given a public page + When get_api_representation() is called + Then the response is authorised and fields are preserved. + """ + block = PageLink() + + mock_page = mock.MagicMock() + mock_page.specific = mock_page + mock_page.is_public = True + + value = { + "title": "Test title", + "sub_title": "Test subtitle", + "page": mock_page, + } + + result = block.get_api_representation(value=value, context={}) + + assert result["is_authorised"] is True + assert result["title"] == "Test title" + assert result["sub_title"] == "Test subtitle" + + @mock.patch("cms.dynamic_content.blocks.check_permissions") + def test_non_public_page_permission_denied(self, mock_check_permissions): + """ + Given a non-public page and permissions denied + When get_api_representation() is called + Then the response is unauthorised and fields are blanked. + """ + mock_check_permissions.return_value = False + + block = PageLink() + + mock_page = mock.MagicMock() + mock_page.specific = mock_page + mock_page.is_public = False + mock_page.theme = 1 + mock_page.sub_theme = 2 + mock_page.topic = 3 + + mock_user = mock.MagicMock() + mock_user.permission_sets = mock.MagicMock() + mock_user.permission_sets.permission_sets = { + "permission_sets": [] + } + + mock_request = mock.MagicMock() + mock_request.user = mock_user + + value = { + "title": "Test title", + "sub_title": "Test subtitle", + "page": mock_page, + } + + result = block.get_api_representation( + value=value, context={"request": mock_request} + ) + + assert result["is_authorised"] is False + assert result["title"] == "" + assert result["sub_title"] == "" + assert result["page"] == "" + + mock_check_permissions.assert_called_once() + + @mock.patch("cms.dynamic_content.blocks.check_permissions") + def test_non_public_page_permission_granted(self, mock_check_permissions): + """ + Given a non-public page and permissions granted + When get_api_representation() is called + Then the response is authorised and fields are preserved. + """ + mock_check_permissions.return_value = True + + block = PageLink() + + mock_page = mock.MagicMock() + mock_page.specific = mock_page + mock_page.is_public = False + mock_page.theme = 1 + mock_page.sub_theme = 2 + mock_page.topic = 3 + mock_page.full_url = "http://test-page-url" + + mock_user = mock.MagicMock() + mock_user.permission_sets = mock.MagicMock() + mock_user.permission_sets.permission_sets = { + "permission_sets": [] + } + + mock_request = mock.MagicMock() + mock_request.user = mock_user + + value = { + "title": "Test title", + "sub_title": "Test subtitle", + "page": mock_page, + } + + result = block.get_api_representation( + value=value, context={"request": mock_request} + ) + + assert result["is_authorised"] is True + assert result["title"] == "Test title" + assert result["sub_title"] == "Test subtitle" + assert result["page"] == "http://test-page-url" + + mock_check_permissions.assert_called_once() + + @mock.patch("cms.dynamic_content.blocks.check_permissions") + def test_non_public_page_missing_request(self, mock_check_permissions): + """ + Given a non-public page without request context + When get_api_representation() is called + Then the response is unauthorised and fields are blanked. + """ + mock_check_permissions.return_value = False + + block = PageLink() + + mock_page = mock.MagicMock() + mock_page.specific = mock_page + mock_page.is_public = False + mock_page.theme = 1 + mock_page.sub_theme = 2 + mock_page.topic = 3 + + value = { + "title": "Test title", + "sub_title": "Test subtitle", + "page": mock_page, + } + + result = block.get_api_representation(value=value, context={}) + + assert result["is_authorised"] is False + assert result["title"] == "" + assert result["sub_title"] == "" + assert result["page"] == "" + + mock_check_permissions.assert_called_once() \ No newline at end of file