From 1fc0620a4f4c11c8a0f1d6d7afca5fbd17941999 Mon Sep 17 00:00:00 2001 From: Kathryn Dale Date: Tue, 23 Jun 2026 11:52:37 +0100 Subject: [PATCH 01/17] 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 3c620b9f05a18adb9f84b63d03045b3e08d4e911 Mon Sep 17 00:00:00 2001 From: Kathryn Dale Date: Wed, 24 Jun 2026 10:05:43 +0100 Subject: [PATCH 02/17] 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 80df03fc646aea78d9653c8b3b65bcb206f8fcf1 Mon Sep 17 00:00:00 2001 From: Megan Bower Date: Wed, 24 Jun 2026 15:38:10 +0100 Subject: [PATCH 03/17] 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 c29072fb8dff511ccae62f8b06dd73a092f72684 Mon Sep 17 00:00:00 2001 From: Megan Bower Date: Wed, 24 Jun 2026 16:00:59 +0100 Subject: [PATCH 04/17] 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 aa4c38b2a22de98e8677e61f7999aff310d6a487 Mon Sep 17 00:00:00 2001 From: Megan Bower Date: Wed, 1 Jul 2026 16:13:44 +0100 Subject: [PATCH 05/17] CDD-3441 Add debugging --- cms/dynamic_content/blocks.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/cms/dynamic_content/blocks.py b/cms/dynamic_content/blocks.py index 791a3ad52..18ee53143 100644 --- a/cms/dynamic_content/blocks.py +++ b/cms/dynamic_content/blocks.py @@ -1,3 +1,5 @@ +import logging + from django.core.exceptions import ValidationError from django.db import models from wagtail import blocks @@ -22,6 +24,10 @@ ) from validation.url import validate_https_scheme + +logger = logging.getLogger(__name__) + + MINIMUM_ROWS_NUMBER_BLOCK_COUNT: int = 1 MAXIMUM_ROWS_NUMBER_BLOCK_COUNT: int = 2 @@ -249,6 +255,11 @@ def get_api_representation(self, value, context=None): if not page.is_public: user_permissions = getattr(user, "permission_sets", None) + logger.warning(f"user_permissions raw: {user_permissions}") + logger.warning(f"type: {type(user_permissions)}") + + if hasattr(user_permissions, "permission_sets"): + logger.warning(f"inner permission_sets: {user_permissions.permission_sets}") full_user_permissions = ( user_permissions.permission_sets.get("permission_sets") if user_permissions and hasattr(user_permissions, "permission_sets") @@ -260,6 +271,7 @@ def get_api_representation(self, value, context=None): getattr(page, "sub_theme", None), getattr(page, "topic", None), ): + print("🦄 check_permissions failed") data["is_authorised"] = False data["title"] = "" data["subtitle"] = "" From 978b10e881f87ad95e117bfefbf4be0f632fcd65 Mon Sep 17 00:00:00 2001 From: Megan Bower Date: Wed, 1 Jul 2026 16:32:29 +0100 Subject: [PATCH 06/17] CDD-3441 Reference the user_permissions dict --- cms/dynamic_content/blocks.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cms/dynamic_content/blocks.py b/cms/dynamic_content/blocks.py index 18ee53143..93ae02aa2 100644 --- a/cms/dynamic_content/blocks.py +++ b/cms/dynamic_content/blocks.py @@ -261,10 +261,12 @@ def get_api_representation(self, value, context=None): if hasattr(user_permissions, "permission_sets"): logger.warning(f"inner permission_sets: {user_permissions.permission_sets}") full_user_permissions = ( - user_permissions.permission_sets.get("permission_sets") - if user_permissions and hasattr(user_permissions, "permission_sets") + user_permissions.get("permission_sets") + if user_permissions else None ) + logger.warning(f"Permissions passed to check: {full_user_permissions}") + logger.warning(f"Page topic: {getattr(page, 'topic', None)}") if not check_permissions( full_user_permissions, getattr(page, "theme", None), From ff016ace6e6de9640240680623caa2837549ef81 Mon Sep 17 00:00:00 2001 From: Megan Bower Date: Wed, 1 Jul 2026 17:17:56 +0100 Subject: [PATCH 07/17] CDD-3441 Add page link unit tests --- cms/dynamic_content/blocks.py | 11 +- tests/unit/cms/dynamic_content/test_blocks.py | 172 +++++++++++++++++- 2 files changed, 173 insertions(+), 10 deletions(-) diff --git a/cms/dynamic_content/blocks.py b/cms/dynamic_content/blocks.py index 93ae02aa2..da201df04 100644 --- a/cms/dynamic_content/blocks.py +++ b/cms/dynamic_content/blocks.py @@ -244,7 +244,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 @@ -255,28 +255,21 @@ def get_api_representation(self, value, context=None): if not page.is_public: user_permissions = getattr(user, "permission_sets", None) - logger.warning(f"user_permissions raw: {user_permissions}") - logger.warning(f"type: {type(user_permissions)}") - if hasattr(user_permissions, "permission_sets"): - logger.warning(f"inner permission_sets: {user_permissions.permission_sets}") full_user_permissions = ( user_permissions.get("permission_sets") if user_permissions else None ) - logger.warning(f"Permissions passed to check: {full_user_permissions}") - logger.warning(f"Page topic: {getattr(page, 'topic', None)}") if not check_permissions( full_user_permissions, getattr(page, "theme", None), getattr(page, "sub_theme", None), getattr(page, "topic", None), ): - print("🦄 check_permissions failed") data["is_authorised"] = False data["title"] = "" - data["subtitle"] = "" + data["sub_title"] = "" data["page"] = "" return data diff --git a/tests/unit/cms/dynamic_content/test_blocks.py b/tests/unit/cms/dynamic_content/test_blocks.py index fe4fb9d79..5f250300b 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 are 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": [] + } + + mock_request = mock.MagicMock() + mock_request.user = mock_user + + value = { + "title": "Test title", + "sub_title": "Test subtitle", + "page": mock_page, + } + + context = {"request": mock_request} + + result = block.get_api_representation(value=value, context=context) + + 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 are 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": [] + } + + mock_request = mock.MagicMock() + mock_request.user = mock_user + + value = { + "title": "Test title", + "sub_title": "Test subtitle", + "page": mock_page, + } + + context = {"request": mock_request} + + result = block.get_api_representation(value=value, context=context) + + 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 and no request in 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 From c68e6b0befbb01fe4dd4c82f8813e696eb83ba73 Mon Sep 17 00:00:00 2001 From: Megan Bower Date: Wed, 1 Jul 2026 17:23:17 +0100 Subject: [PATCH 08/17] CDD-3441 Format & lint --- cms/dynamic_content/blocks.py | 17 ++++++----------- tests/unit/cms/dynamic_content/test_blocks.py | 14 +++++--------- 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/cms/dynamic_content/blocks.py b/cms/dynamic_content/blocks.py index da201df04..a3643e95b 100644 --- a/cms/dynamic_content/blocks.py +++ b/cms/dynamic_content/blocks.py @@ -4,7 +4,6 @@ from django.db import models from wagtail import blocks from wagtail.blocks import ( - BooleanBlock, CharBlock, ChoiceBlock, PageChooserBlock, @@ -24,7 +23,6 @@ ) from validation.url import validate_https_scheme - logger = logging.getLogger(__name__) @@ -252,21 +250,18 @@ def get_api_representation(self, value, context=None): request = context.get("request") if context else None user = getattr(request, "user", None) - if not page.is_public: user_permissions = getattr(user, "permission_sets", None) full_user_permissions = ( - user_permissions.get("permission_sets") - if user_permissions - else None + user_permissions.get("permission_sets") if user_permissions else None ) if not check_permissions( - full_user_permissions, - getattr(page, "theme", None), - getattr(page, "sub_theme", None), - getattr(page, "topic", None), - ): + full_user_permissions, + getattr(page, "theme", None), + getattr(page, "sub_theme", None), + getattr(page, "topic", None), + ): data["is_authorised"] = False data["title"] = "" data["sub_title"] = "" diff --git a/tests/unit/cms/dynamic_content/test_blocks.py b/tests/unit/cms/dynamic_content/test_blocks.py index 5f250300b..b180eb58e 100644 --- a/tests/unit/cms/dynamic_content/test_blocks.py +++ b/tests/unit/cms/dynamic_content/test_blocks.py @@ -202,9 +202,7 @@ def test_non_public_page_permission_denied(self, mock_check_permissions): mock_user = mock.MagicMock() mock_user.permission_sets = mock.MagicMock() - mock_user.permission_sets = { - "permission_sets": [] - } + mock_user.permission_sets = {"permission_sets": []} mock_request = mock.MagicMock() mock_request.user = mock_user @@ -243,13 +241,11 @@ def test_non_public_page_permission_granted(self, mock_check_permissions): mock_page.theme = 1 mock_page.sub_theme = 2 mock_page.topic = 3 - mock_page.full_url = "http://test-page-url" + mock_page.full_url = "https://test-page-url" mock_user = mock.MagicMock() mock_user.permission_sets = mock.MagicMock() - mock_user.permission_sets = { - "permission_sets": [] - } + mock_user.permission_sets = {"permission_sets": []} mock_request = mock.MagicMock() mock_request.user = mock_user @@ -267,7 +263,7 @@ def test_non_public_page_permission_granted(self, mock_check_permissions): assert result["is_authorised"] is True assert result["title"] == "Test title" assert result["sub_title"] == "Test subtitle" - assert result["page"] == "http://test-page-url" + assert result["page"] == "https://test-page-url" mock_check_permissions.assert_called_once() @@ -302,4 +298,4 @@ def test_non_public_page_missing_request(self, mock_check_permissions): assert result["sub_title"] == "" assert result["page"] == "" - mock_check_permissions.assert_called_once() \ No newline at end of file + mock_check_permissions.assert_called_once() From fb8ab2c9da9638d53584e44c48a898af62dfa36a Mon Sep 17 00:00:00 2001 From: Megan Bower Date: Wed, 1 Jul 2026 17:39:13 +0100 Subject: [PATCH 09/17] CDD-3441 check_permissions unit tests --- tests/unit/cms/dynamic_content/test_blocks.py | 121 +++++++++++++++++- 1 file changed, 120 insertions(+), 1 deletion(-) diff --git a/tests/unit/cms/dynamic_content/test_blocks.py b/tests/unit/cms/dynamic_content/test_blocks.py index b180eb58e..4b3560855 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 PageLink, SourceLinkBlock +from cms.dynamic_content.blocks import PageLink, SourceLinkBlock, check_permissions class TestSourceLinkBlockClean: @@ -299,3 +299,122 @@ def test_non_public_page_missing_request(self, mock_check_permissions): assert result["page"] == "" mock_check_permissions.assert_called_once() + + +class TestCheckPermissions: + + def test_returns_false_if_not_list(self): + """ + Given user_permissions is not a list + Then function returns False + """ + assert check_permissions(None, "1", "2", "3") is False + assert check_permissions({}, "1", "2", "3") is False + + def test_global_wildcard_theme_allows_access(self): + """ + Given a permission with theme = '-1' + Then access is always allowed + """ + permissions = [ + { + "theme": {"id": "-1"}, + "sub_theme": {"id": "999"}, + "topic": {"id": "999"}, + } + ] + + assert check_permissions(permissions, "1", "2", "3") is True + + def test_theme_match_with_subtheme_wildcard(self): + """ + Given matching theme and sub_theme wildcard (-1) + Then access is allowed + """ + permissions = [ + { + "theme": {"id": "1"}, + "sub_theme": {"id": "-1"}, + "topic": {"id": "999"}, + } + ] + + assert check_permissions(permissions, "1", "2", "3") is True + + def test_full_match_allows_access(self): + """ + Given theme, sub_theme and topic match + Then access is allowed + """ + permissions = [ + { + "theme": {"id": "1"}, + "sub_theme": {"id": "2"}, + "topic": {"id": "3"}, + } + ] + + assert check_permissions(permissions, "1", "2", "3") is True + + def test_topic_wildcard_allows_access(self): + """ + Given matching theme + sub_theme and topic wildcard (-1) + Then access is allowed + """ + permissions = [ + { + "theme": {"id": "1"}, + "sub_theme": {"id": "2"}, + "topic": {"id": "-1"}, + } + ] + + assert check_permissions(permissions, "1", "2", "3") is True + + def test_no_matching_permissions_returns_false(self): + """ + Given no permissions match + Then access is denied + """ + permissions = [ + { + "theme": {"id": "10"}, + "sub_theme": {"id": "20"}, + "topic": {"id": "30"}, + } + ] + + assert check_permissions(permissions, "1", "2", "3") is False + + def test_multiple_permissions_one_valid(self): + """ + Given multiple permission sets and one is valid + Then access is allowed + """ + permissions = [ + { + "theme": {"id": "10"}, + "sub_theme": {"id": "20"}, + "topic": {"id": "30"}, + }, + { + "theme": {"id": "1"}, + "sub_theme": {"id": "2"}, + "topic": {"id": "3"}, + }, + ] + + assert check_permissions(permissions, "1", "2", "3") is True + + def test_missing_keys_do_not_break_logic(self): + """ + Given malformed permission entries + Then function safely ignores them + """ + permissions = [ + {}, + {"theme": {}}, + {"theme": {"id": "1"}}, # incomplete + ] + + assert check_permissions(permissions, "1", "2", "3") is False \ No newline at end of file From a35f3c0b02b0997f2596d6386fc28406463dda65 Mon Sep 17 00:00:00 2001 From: Megan Bower Date: Thu, 2 Jul 2026 09:25:49 +0100 Subject: [PATCH 10/17] CDD-3441 Format & lint --- tests/unit/cms/dynamic_content/test_blocks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/cms/dynamic_content/test_blocks.py b/tests/unit/cms/dynamic_content/test_blocks.py index 4b3560855..3bb90e4e9 100644 --- a/tests/unit/cms/dynamic_content/test_blocks.py +++ b/tests/unit/cms/dynamic_content/test_blocks.py @@ -417,4 +417,4 @@ def test_missing_keys_do_not_break_logic(self): {"theme": {"id": "1"}}, # incomplete ] - assert check_permissions(permissions, "1", "2", "3") is False \ No newline at end of file + assert check_permissions(permissions, "1", "2", "3") is False From f014516b9aec6111b7072f1e2107b3f16cdcca6f Mon Sep 17 00:00:00 2001 From: Megan Bower Date: Thu, 2 Jul 2026 09:27:31 +0100 Subject: [PATCH 11/17] CDD-3441 Remove temp logging --- cms/dynamic_content/blocks.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/cms/dynamic_content/blocks.py b/cms/dynamic_content/blocks.py index a3643e95b..5cd17f2c2 100644 --- a/cms/dynamic_content/blocks.py +++ b/cms/dynamic_content/blocks.py @@ -1,5 +1,3 @@ -import logging - from django.core.exceptions import ValidationError from django.db import models from wagtail import blocks @@ -23,8 +21,6 @@ ) from validation.url import validate_https_scheme -logger = logging.getLogger(__name__) - MINIMUM_ROWS_NUMBER_BLOCK_COUNT: int = 1 MAXIMUM_ROWS_NUMBER_BLOCK_COUNT: int = 2 From aeabbfdc6d826cea4c4f255e07a97fa0ad972778 Mon Sep 17 00:00:00 2001 From: Megan Bower Date: Thu, 2 Jul 2026 09:46:32 +0100 Subject: [PATCH 12/17] CDD-3441 Format & lint --- cms/dynamic_content/blocks.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cms/dynamic_content/blocks.py b/cms/dynamic_content/blocks.py index 5cd17f2c2..e40257faa 100644 --- a/cms/dynamic_content/blocks.py +++ b/cms/dynamic_content/blocks.py @@ -21,7 +21,6 @@ ) from validation.url import validate_https_scheme - MINIMUM_ROWS_NUMBER_BLOCK_COUNT: int = 1 MAXIMUM_ROWS_NUMBER_BLOCK_COUNT: int = 2 From 57bf034df0d0a2ae88da9007ce87c766724617d7 Mon Sep 17 00:00:00 2001 From: Megan Bower Date: Thu, 2 Jul 2026 14:00:26 +0100 Subject: [PATCH 13/17] CDD-3441 Use common check permissions --- cms/dynamic_content/blocks.py | 36 +----- tests/unit/cms/dynamic_content/test_blocks.py | 119 ------------------ tests/unit/common/auth/test_permissions.py | 37 ++++++ 3 files changed, 43 insertions(+), 149 deletions(-) diff --git a/cms/dynamic_content/blocks.py b/cms/dynamic_content/blocks.py index e40257faa..43f1c7d0a 100644 --- a/cms/dynamic_content/blocks.py +++ b/cms/dynamic_content/blocks.py @@ -1,5 +1,6 @@ from django.core.exceptions import ValidationError from django.db import models +from common.auth.permissions import check_page_permissions from wagtail import blocks from wagtail.blocks import ( CharBlock, @@ -30,31 +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 - - 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) @@ -251,11 +227,11 @@ def get_api_representation(self, value, context=None): full_user_permissions = ( user_permissions.get("permission_sets") if user_permissions else None ) - if not check_permissions( - full_user_permissions, - getattr(page, "theme", None), - getattr(page, "sub_theme", None), - getattr(page, "topic", None), + if not check_page_permissions( + permission_sets=full_user_permissions, + theme_id=getattr(page, "theme", None), + sub_theme_id=getattr(page, "sub_theme", None), + topic_id=getattr(page, "topic", None), ): data["is_authorised"] = False data["title"] = "" diff --git a/tests/unit/cms/dynamic_content/test_blocks.py b/tests/unit/cms/dynamic_content/test_blocks.py index 3bb90e4e9..5529cce96 100644 --- a/tests/unit/cms/dynamic_content/test_blocks.py +++ b/tests/unit/cms/dynamic_content/test_blocks.py @@ -299,122 +299,3 @@ def test_non_public_page_missing_request(self, mock_check_permissions): assert result["page"] == "" mock_check_permissions.assert_called_once() - - -class TestCheckPermissions: - - def test_returns_false_if_not_list(self): - """ - Given user_permissions is not a list - Then function returns False - """ - assert check_permissions(None, "1", "2", "3") is False - assert check_permissions({}, "1", "2", "3") is False - - def test_global_wildcard_theme_allows_access(self): - """ - Given a permission with theme = '-1' - Then access is always allowed - """ - permissions = [ - { - "theme": {"id": "-1"}, - "sub_theme": {"id": "999"}, - "topic": {"id": "999"}, - } - ] - - assert check_permissions(permissions, "1", "2", "3") is True - - def test_theme_match_with_subtheme_wildcard(self): - """ - Given matching theme and sub_theme wildcard (-1) - Then access is allowed - """ - permissions = [ - { - "theme": {"id": "1"}, - "sub_theme": {"id": "-1"}, - "topic": {"id": "999"}, - } - ] - - assert check_permissions(permissions, "1", "2", "3") is True - - def test_full_match_allows_access(self): - """ - Given theme, sub_theme and topic match - Then access is allowed - """ - permissions = [ - { - "theme": {"id": "1"}, - "sub_theme": {"id": "2"}, - "topic": {"id": "3"}, - } - ] - - assert check_permissions(permissions, "1", "2", "3") is True - - def test_topic_wildcard_allows_access(self): - """ - Given matching theme + sub_theme and topic wildcard (-1) - Then access is allowed - """ - permissions = [ - { - "theme": {"id": "1"}, - "sub_theme": {"id": "2"}, - "topic": {"id": "-1"}, - } - ] - - assert check_permissions(permissions, "1", "2", "3") is True - - def test_no_matching_permissions_returns_false(self): - """ - Given no permissions match - Then access is denied - """ - permissions = [ - { - "theme": {"id": "10"}, - "sub_theme": {"id": "20"}, - "topic": {"id": "30"}, - } - ] - - assert check_permissions(permissions, "1", "2", "3") is False - - def test_multiple_permissions_one_valid(self): - """ - Given multiple permission sets and one is valid - Then access is allowed - """ - permissions = [ - { - "theme": {"id": "10"}, - "sub_theme": {"id": "20"}, - "topic": {"id": "30"}, - }, - { - "theme": {"id": "1"}, - "sub_theme": {"id": "2"}, - "topic": {"id": "3"}, - }, - ] - - assert check_permissions(permissions, "1", "2", "3") is True - - def test_missing_keys_do_not_break_logic(self): - """ - Given malformed permission entries - Then function safely ignores them - """ - permissions = [ - {}, - {"theme": {}}, - {"theme": {"id": "1"}}, # incomplete - ] - - assert check_permissions(permissions, "1", "2", "3") is False diff --git a/tests/unit/common/auth/test_permissions.py b/tests/unit/common/auth/test_permissions.py index cf077e2b0..37a17e90b 100644 --- a/tests/unit/common/auth/test_permissions.py +++ b/tests/unit/common/auth/test_permissions.py @@ -1090,6 +1090,7 @@ def test_check_page_permissions_invalid_access( "user_permissions, theme_id, sub_theme_id, topic_id", [ ([{}], "10", "20", "30"), + ("invalid", "10", "20", "30"), (None, "10", "20", "30"), ([{"sub_theme": {"id": "-1"}, "topic": {"id": "-1"}}], "10", "20", "30"), ( @@ -1125,3 +1126,39 @@ def test_check_page_permissions_with_missing_values( sub_theme_id=sub_theme_id, topic_id=topic_id, ) + + @pytest.mark.parametrize( + "user_permissions", + [ + ["invalid"], + [123], + [None], + [{"theme": {"id": "10"}}, "invalid"], + ], + ) + def test_check_page_permissions_non_dict_permission_entry(user_permissions): + assert not check_page_permissions( + permission_sets=user_permissions, + theme_id="10", + sub_theme_id="20", + topic_id="30", + ) + + @pytest.mark.parametrize( + "theme_id, sub_theme_id, topic_id", + [ + (None, "20", "30"), + ("10", None, "30"), + ("10", "20", None), + ("", "20", "30"), + ], + ) + def test_check_page_permissions_invalid_resource_ids( + theme_id, sub_theme_id, topic_id + ): + assert not check_page_permissions( + permission_sets=[{"theme": {"id": "-1"}}], + theme_id=theme_id, + sub_theme_id=sub_theme_id, + topic_id=topic_id, + ) From f2de3cc56d4deb8eb9d2b16d6c1551e675a9fbe5 Mon Sep 17 00:00:00 2001 From: Megan Bower Date: Thu, 2 Jul 2026 14:11:30 +0100 Subject: [PATCH 14/17] CDD-3441 Update unit tests & temp add check_permissions back in --- cms/dynamic_content/blocks.py | 29 ++++++++++++++++++++-- tests/unit/common/auth/test_permissions.py | 4 +-- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/cms/dynamic_content/blocks.py b/cms/dynamic_content/blocks.py index 43f1c7d0a..24c8f1aa0 100644 --- a/cms/dynamic_content/blocks.py +++ b/cms/dynamic_content/blocks.py @@ -31,6 +31,31 @@ 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) @@ -227,8 +252,8 @@ def get_api_representation(self, value, context=None): full_user_permissions = ( user_permissions.get("permission_sets") if user_permissions else None ) - if not check_page_permissions( - permission_sets=full_user_permissions, + if not check_permissions( + user_permissions=full_user_permissions, theme_id=getattr(page, "theme", None), sub_theme_id=getattr(page, "sub_theme", None), topic_id=getattr(page, "topic", None), diff --git a/tests/unit/common/auth/test_permissions.py b/tests/unit/common/auth/test_permissions.py index 37a17e90b..bdae742ff 100644 --- a/tests/unit/common/auth/test_permissions.py +++ b/tests/unit/common/auth/test_permissions.py @@ -1136,7 +1136,7 @@ def test_check_page_permissions_with_missing_values( [{"theme": {"id": "10"}}, "invalid"], ], ) - def test_check_page_permissions_non_dict_permission_entry(user_permissions): + def test_check_page_permissions_non_dict_permission_entry(self, user_permissions): assert not check_page_permissions( permission_sets=user_permissions, theme_id="10", @@ -1154,7 +1154,7 @@ def test_check_page_permissions_non_dict_permission_entry(user_permissions): ], ) def test_check_page_permissions_invalid_resource_ids( - theme_id, sub_theme_id, topic_id + self, theme_id, sub_theme_id, topic_id ): assert not check_page_permissions( permission_sets=[{"theme": {"id": "-1"}}], From 90a024ad5cbcdfe3f83e534427c0dc0bac4663a0 Mon Sep 17 00:00:00 2001 From: Megan Bower Date: Thu, 2 Jul 2026 14:14:15 +0100 Subject: [PATCH 15/17] CDD-3441 Remove circular import --- cms/dynamic_content/blocks.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cms/dynamic_content/blocks.py b/cms/dynamic_content/blocks.py index 24c8f1aa0..fa71eaa5d 100644 --- a/cms/dynamic_content/blocks.py +++ b/cms/dynamic_content/blocks.py @@ -1,6 +1,5 @@ from django.core.exceptions import ValidationError from django.db import models -from common.auth.permissions import check_page_permissions from wagtail import blocks from wagtail.blocks import ( CharBlock, From 78615d8ecc1a398dafa8c73c2538a6a8511d073f Mon Sep 17 00:00:00 2001 From: Megan Bower Date: Thu, 2 Jul 2026 14:26:00 +0100 Subject: [PATCH 16/17] CDD-3441 Remove invalid test case --- tests/unit/common/auth/test_permissions.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/common/auth/test_permissions.py b/tests/unit/common/auth/test_permissions.py index bdae742ff..4caa97093 100644 --- a/tests/unit/common/auth/test_permissions.py +++ b/tests/unit/common/auth/test_permissions.py @@ -1150,7 +1150,6 @@ def test_check_page_permissions_non_dict_permission_entry(self, user_permissions (None, "20", "30"), ("10", None, "30"), ("10", "20", None), - ("", "20", "30"), ], ) def test_check_page_permissions_invalid_resource_ids( From f307e441f4b9a1aa099223e51476930fdb2c18d9 Mon Sep 17 00:00:00 2001 From: Megan Bower Date: Thu, 2 Jul 2026 16:38:54 +0100 Subject: [PATCH 17/17] CDD-3441 Use shared check page permissions --- cms/dynamic_content/blocks.py | 30 ++----------------- tests/unit/cms/dynamic_content/test_blocks.py | 26 ++++++++-------- 2 files changed, 16 insertions(+), 40 deletions(-) diff --git a/cms/dynamic_content/blocks.py b/cms/dynamic_content/blocks.py index fa71eaa5d..43f1c7d0a 100644 --- a/cms/dynamic_content/blocks.py +++ b/cms/dynamic_content/blocks.py @@ -1,5 +1,6 @@ from django.core.exceptions import ValidationError from django.db import models +from common.auth.permissions import check_page_permissions from wagtail import blocks from wagtail.blocks import ( CharBlock, @@ -30,31 +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 - - 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) @@ -251,8 +227,8 @@ def get_api_representation(self, value, context=None): full_user_permissions = ( user_permissions.get("permission_sets") if user_permissions else None ) - if not check_permissions( - user_permissions=full_user_permissions, + if not check_page_permissions( + permission_sets=full_user_permissions, theme_id=getattr(page, "theme", None), sub_theme_id=getattr(page, "sub_theme", None), topic_id=getattr(page, "topic", None), diff --git a/tests/unit/cms/dynamic_content/test_blocks.py b/tests/unit/cms/dynamic_content/test_blocks.py index 5529cce96..5bbadaaa5 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 PageLink, SourceLinkBlock, check_permissions +from cms.dynamic_content.blocks import PageLink, SourceLinkBlock class TestSourceLinkBlockClean: @@ -182,14 +182,14 @@ def test_public_page_is_always_authorised(self): 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): + @mock.patch("cms.dynamic_content.blocks.check_page_permissions") + def test_non_public_page_permission_denied(self, mock_check_page_permissions): """ Given a non-public page and permissions are denied When get_api_representation() is called Then the response is unauthorised and fields are blanked. """ - mock_check_permissions.return_value = False + mock_check_page_permissions.return_value = False block = PageLink() @@ -222,16 +222,16 @@ def test_non_public_page_permission_denied(self, mock_check_permissions): assert result["sub_title"] == "" assert result["page"] == "" - mock_check_permissions.assert_called_once() + mock_check_page_permissions.assert_called_once() - @mock.patch("cms.dynamic_content.blocks.check_permissions") - def test_non_public_page_permission_granted(self, mock_check_permissions): + @mock.patch("cms.dynamic_content.blocks.check_page_permissions") + def test_non_public_page_permission_granted(self, mock_check_page_permissions): """ Given a non-public page and permissions are granted When get_api_representation() is called Then the response is authorised and fields are preserved. """ - mock_check_permissions.return_value = True + mock_check_page_permissions.return_value = True block = PageLink() @@ -265,16 +265,16 @@ def test_non_public_page_permission_granted(self, mock_check_permissions): assert result["sub_title"] == "Test subtitle" assert result["page"] == "https://test-page-url" - mock_check_permissions.assert_called_once() + mock_check_page_permissions.assert_called_once() - @mock.patch("cms.dynamic_content.blocks.check_permissions") - def test_non_public_page_missing_request(self, mock_check_permissions): + @mock.patch("cms.dynamic_content.blocks.check_page_permissions") + def test_non_public_page_missing_request(self, mock_check_page_permissions): """ Given a non-public page and no request in context When get_api_representation() is called Then the response is unauthorised and fields are blanked. """ - mock_check_permissions.return_value = False + mock_check_page_permissions.return_value = False block = PageLink() @@ -298,4 +298,4 @@ def test_non_public_page_missing_request(self, mock_check_permissions): assert result["sub_title"] == "" assert result["page"] == "" - mock_check_permissions.assert_called_once() + mock_check_page_permissions.assert_called_once()