From 8d8e886ce0706ca442a232cab6a2afa2720b0dc4 Mon Sep 17 00:00:00 2001 From: Kathryn Dale Date: Mon, 18 May 2026 09:14:38 +0100 Subject: [PATCH 01/18] CDD-3090: Update watermark for non public charts --- cms/dashboard/models.py | 13 +++-- metrics/api/serializers/charts/common.py | 12 +++++ .../charts/single_category_charts.py | 6 +++ .../api/serializers/charts/subplot_charts.py | 17 +++++++ metrics/api/serializers/headlines.py | 12 +++++ metrics/api/serializers/help_texts.py | 7 +++ metrics/api/serializers/tables.py | 4 ++ .../views/charts/single_category_charts.py | 49 +++++++++++++++++++ metrics/domain/models/charts/common.py | 2 + .../domain/models/charts/subplot_charts.py | 6 ++- metrics/domain/models/headline.py | 4 +- .../interfaces/charts/common/chart_output.py | 35 +++++++++++++ .../charts/single_category_charts/access.py | 2 + .../charts/subplot_charts/access.py | 2 + .../interfaces/data_classification/access.py | 8 +++ 15 files changed, 172 insertions(+), 7 deletions(-) create mode 100644 metrics/interfaces/data_classification/access.py diff --git a/cms/dashboard/models.py b/cms/dashboard/models.py index f94bb4368..0928f6de4 100644 --- a/cms/dashboard/models.py +++ b/cms/dashboard/models.py @@ -12,6 +12,7 @@ from wagtail.search import index from cms import seo +from metrics.interfaces.data_classification.access import DataClassification HEADING_2: str = "h2" HEADING_3: str = "h3" @@ -33,12 +34,14 @@ MAXIMUM_URL_FIELD_LENGTH: int = 400 + class DataClassificationLevels(models.TextChoices): - OFFICIAL = "official" - OFFICIAL_SENSITIVE = "official_sensitive" - PROTECTIVE_MARKING_NOT_SET = "protective_marking_not_set" - SECRET = "secret" # nosec #noqa: S105 - TOP_SECRET = "top_secret" # nosec #noqa: S105 + OFFICIAL = (DataClassification.official.value).lower() + OFFICIAL_SENSITIVE = (DataClassification.official_sensitive.value).lower() + PROTECTIVE_MARKING_NOT_SET = (DataClassification.protective_marking_not_set.value).lower() + SECRET = (DataClassification.secret.value).lower() + TOP_SECRET = (DataClassification.top_secret.value).lower() + class UKHSAPage(Page): diff --git a/metrics/api/serializers/charts/common.py b/metrics/api/serializers/charts/common.py index decec6be7..9f774327e 100644 --- a/metrics/api/serializers/charts/common.py +++ b/metrics/api/serializers/charts/common.py @@ -94,3 +94,15 @@ class BaseChartsSerializer(serializers.Serializer): allow_null=True, default="", ) + is_public = serializers.BooleanField( + required=False, + default=False, + help_text=help_texts.IS_PUBLIC_FIELD + ) + data_classification = serializers.CharField( + required=False, + allow_blank=True, + allow_null=True, + default=None, + help_text=help_texts.DATA_CLASSIFICATION_FIELD + ) diff --git a/metrics/api/serializers/charts/single_category_charts.py b/metrics/api/serializers/charts/single_category_charts.py index 46b9d958f..f07ea9de2 100644 --- a/metrics/api/serializers/charts/single_category_charts.py +++ b/metrics/api/serializers/charts/single_category_charts.py @@ -82,6 +82,10 @@ def to_models(self, request: Request) -> ChartRequestParams: for plot in self.data["plots"]: plot["x_axis"] = x_axis plot["y_axis"] = y_axis + + # If not provided, assume data is public --> is this right? + is_public: bool = self.data.get("is_public", True) + data_classification: str | None = self.data.get("data_classification") return ChartRequestParams( plots=self.data["plots"], @@ -98,6 +102,8 @@ def to_models(self, request: Request) -> ChartRequestParams: legend_title=self.data.get("legend_title", ""), confidence_intervals=self.data.get("confidence_intervals", False), confidence_colour=self.data.get("confidence_colour", ""), + is_public = is_public, + data_classification=data_classification, request=request, ) diff --git a/metrics/api/serializers/charts/subplot_charts.py b/metrics/api/serializers/charts/subplot_charts.py index 99475d04c..e6a4aafa2 100644 --- a/metrics/api/serializers/charts/subplot_charts.py +++ b/metrics/api/serializers/charts/subplot_charts.py @@ -157,6 +157,18 @@ class SubplotChartRequestSerializer(serializers.Serializer): allow_blank=True, allow_null=True, ) + is_public = serializers.BooleanField( + required=False, + default=True, + help_text=help_texts.IS_PUBLIC_FIELD + ) + data_classification = serializers.CharField( + required=False, + allow_blank=True, + allow_null=True, + default=None, + help_text=help_texts.DATA_CLASSIFICATION_FIELD + ) chart_parameters = ChartParametersSerializer() subplots = SubplotsSerializer() @@ -196,6 +208,9 @@ def to_models(self, request: Request) -> SubplotChartRequestParameters: } ) plot["metric_value_ranges"] = metric_value_ranges + + is_public: bool = self.validated_data.get("is_public", True) + data_classification: str | None = self.validated_data.get("data_classification") return SubplotChartRequestParameters( file_format=self.validated_data["file_format"], @@ -211,6 +226,8 @@ def to_models(self, request: Request) -> SubplotChartRequestParameters: "target_threshold_label", None ), subplots=self.validated_data["subplots"], + is_public=is_public, + data_classification=data_classification, request=request, ) diff --git a/metrics/api/serializers/headlines.py b/metrics/api/serializers/headlines.py index 1e6c3989b..0ccfcea1a 100644 --- a/metrics/api/serializers/headlines.py +++ b/metrics/api/serializers/headlines.py @@ -57,6 +57,18 @@ class HeadlinesQuerySerializer(serializers.Serializer): required=False, help_text=help_texts.SEX_FIELD, ) + is_public = serializers.BooleanField( + required=False, + default=True, + help_text=help_texts.IS_PUBLIC_FIELD, + ) + data_classification = serializers.CharField( + required=False, + allow_blank=True, + allow_null=True, + default=None, + help_text=help_texts.DATA_CLASSIFICATION_FIELD, + ) def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) diff --git a/metrics/api/serializers/help_texts.py b/metrics/api/serializers/help_texts.py index e99a758cf..dc0366489 100644 --- a/metrics/api/serializers/help_texts.py +++ b/metrics/api/serializers/help_texts.py @@ -132,3 +132,10 @@ GEOGRAPHY_LIST_FORMATTING: str = """ "List of [id, name] pairs for dropdown options" """ +IS_PUBLIC_FIELD: str = """ +Whether the chart data is intended for public display. Defaults to True. +When False, a data classification watermark will be applied to the chart. +""" +DATA_CLASSIFICATION_FIELD: str = """ +The data classification watermark to apply on non-public charts, eg "OFFICIAL-SENSITIVE". +""" diff --git a/metrics/api/serializers/tables.py b/metrics/api/serializers/tables.py index 8c2e2a008..2da56902f 100644 --- a/metrics/api/serializers/tables.py +++ b/metrics/api/serializers/tables.py @@ -55,6 +55,8 @@ class TablesSerializer(serializers.Serializer): help_text=help_texts.CHART_Y_AXIS, default=DEFAULT_Y_AXIS, ) + + def __init__(self, *args, **kwargs): with contextlib.suppress(OperationalError): @@ -68,6 +70,8 @@ def to_models(self, request: Request) -> ChartRequestParams: chart_width=DEFAULT_CHART_WIDTH, x_axis=self.data.get("x_axis") or DEFAULT_X_AXIS, y_axis=self.data.get("y_axis") or DEFAULT_Y_AXIS, + is_public = self.data.get("is_public", True), + data_classification = self.data.get("data_classification"), request=request, ) diff --git a/metrics/api/views/charts/single_category_charts.py b/metrics/api/views/charts/single_category_charts.py index 2767c3982..5bde26b12 100644 --- a/metrics/api/views/charts/single_category_charts.py +++ b/metrics/api/views/charts/single_category_charts.py @@ -97,6 +97,8 @@ def post(cls, request, *args, **kwargs): | `label` | The label to assign on the legend for this individual plot | Females | No | | `line_colour` | The colour to use for the line of this individual plot | BLUE | No | | `line_type` | The type to assign for this individual plot i.e. SOLID or DASH | DASH | No | + | `is_public` | Whether the chart is for the public / non-public dashboard environment | True | Yes | + | `data_classification` | The watermark wording (only for non-public charts) | OFFICIAL-SENSITIVE | No | --- @@ -225,6 +227,51 @@ class EncodedChartsView(APIView): request=EncodedChartsRequestSerializer, responses={HTTPStatus.OK.value: EncodedChartResponseSerializer}, tags=[CHARTS_API_TAG], + examples=[ + OpenApiExample( + "COVID-19 encoded SVG example", + value={ + "file_format": "svg", + "x_axis": "date", + "y_axis": "metric", + "is_public": False, + "data_classification": "OFFICIAL-SENSITIVE", + "plots": [ + { + "topic": "COVID-19", + "metric": "COVID-19_cases_casesByDay", + "chart_type": "bar", + "date_from": "2022-01-01", + "date_to": "2023-02-01", + } + ], + }, + request_only=True, + ), + OpenApiExample( + "COVID-19 encoded SVG response example", + value={ + "last_updated": "2023-02-01", + "chart": "%3C%3Fxml%20version%3D%221.0%22%20encoding%3D%22UTF-8%22%3F%3E%3Csvg%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%20viewBox%3D%220%200%20900%20300%22%3E%3Ctext%20x%3D%22450%22%20y%3D%22150%22%20text-anchor%3D%22middle%22%3EOFFICIAL%20SENSITIVE%3C%2Ftext%3E%3C%2Fsvg%3E", + "alt_text": "There is only 1 plot on this chart. The horizontal X-axis is labelled 'date'. Whilst the vertical Y-axis is labelled 'metric'. This is a blue solid bar plot showing COVID-19 cases by day.", + "figure": { + "data": [ + { + "x": ["2023-01-01", "2023-01-02"], + "y": [100, 150], + "type": "bar", + } + ], + "layout": { + "title": "COVID-19 Cases by Day", + "xaxis": {"title": "Date"}, + "yaxis": {"title": "Cases"}, + }, + }, + }, + response_only=True, + ), + ], ) @cache_response() @require_authorisation @@ -249,6 +296,8 @@ def post(cls, request, *args, **kwargs): | `label` | The label to assign on the legend for this individual plot | Females | No | | `line_colour` | The colour to use for the line of this individual plot | BLUE | No | | `line_type` | The type to assign for this individual plot i.e. SOLID or DASH | DASH | No | + | `is_public` | Whether the chart is for the public / non-public dashboard environment | True | Yes | + | `data_classification` | The watermark wording (only for non-public charts) | OFFICIAL-SENSITIVE | No | --- diff --git a/metrics/domain/models/charts/common.py b/metrics/domain/models/charts/common.py index 317301450..fbfe185f9 100644 --- a/metrics/domain/models/charts/common.py +++ b/metrics/domain/models/charts/common.py @@ -20,6 +20,8 @@ class BaseChartRequestParams(BaseModel): request: Request | None = None confidence_intervals: bool | None = False confidence_colour: str | None = "" + is_public: bool + data_classification: str | None = None class Config: arbitrary_types_allowed = True diff --git a/metrics/domain/models/charts/subplot_charts.py b/metrics/domain/models/charts/subplot_charts.py index 63b0c7e2d..25e735645 100644 --- a/metrics/domain/models/charts/subplot_charts.py +++ b/metrics/domain/models/charts/subplot_charts.py @@ -20,7 +20,7 @@ class Subplots(BaseModel): class Config: arbitrary_types_allowed = True - + @property def rbac_permissions(self) -> Iterable["RBACPermission"]: return getattr(self.request, "rbac_permissions", []) @@ -42,6 +42,8 @@ class SubplotChartRequestParameters(BaseModel): target_threshold: float | None = None target_threshold_label: str | None = "" request: Request | None = None + is_public: bool + data_classification: str | None = None subplots: list[Subplots] @@ -131,6 +133,8 @@ def output_payload_for_tables(self) -> list[ChartRequestParams]: y_axis_minimum_value=self.y_axis_minimum_value, y_axis_maximum_value=self.y_axis_maximum_value, request=self.request, + is_public=self.is_public, + data_classification=self.data_classification, ) overall_payload.append(grouped_subplot) diff --git a/metrics/domain/models/headline.py b/metrics/domain/models/headline.py index 675c8e4e5..9a561f198 100644 --- a/metrics/domain/models/headline.py +++ b/metrics/domain/models/headline.py @@ -12,6 +12,8 @@ class HeadlineParameters(BaseModel): geography_type: str sex: str age: str + is_public: bool + data_classification: str | None = None request: Request | None = None class Config: @@ -66,7 +68,7 @@ def to_dict_for_query(self) -> dict[str, str]: "sex": self.sex_name, "rbac_permissions": self.rbac_permissions, } - + @property def rbac_permissions(self) -> Iterable["RBACPermission"]: return getattr(self.request, "rbac_permissions", []) diff --git a/metrics/interfaces/charts/common/chart_output.py b/metrics/interfaces/charts/common/chart_output.py index a48319dc8..534232054 100644 --- a/metrics/interfaces/charts/common/chart_output.py +++ b/metrics/interfaces/charts/common/chart_output.py @@ -2,7 +2,12 @@ import plotly.graph_objects as go +from metrics.interfaces.data_classification.access import DataClassification + HEX_COLOUR_BLACK = "#0b0c0c" +WATERMARK_FONT_SIZE = 40 +WATERMARK_FONT_COLOUR = "rgba(0, 0, 0, 0.25)" +WATERMARK_OPACITY = 0.58 @dataclass @@ -11,6 +16,36 @@ class ChartOutput: description: str is_headline: bool is_subplot: bool = False + is_public: bool = True + data_classification: str | None = None + + def __post_init__(self) -> None: + if not self.is_public: + self._apply_watermark() + + + def _apply_watermark(self) -> None: + """ + Adds a diagonal watermark to the Plotly figure. + + The watermark is added directly to the figure as a layout + annotation using paper coordinates, so it is consistently + rendered in static SVG exports, interactive Plotly outputs, + and any downloaded chart artefacts. + """ + watermark_text = DataClassification[self.data_classification].value + + self.figure.add_annotation( + text=watermark_text, + xref="paper", + yref="paper", + x=0.5, + y=0.5, + showarrow=False, + font={"size": WATERMARK_FONT_SIZE, "color": WATERMARK_FONT_COLOUR}, + textangle=-30, + opacity=WATERMARK_OPACITY, + ) @property def interactive_chart_figure_output(self) -> dict: diff --git a/metrics/interfaces/charts/single_category_charts/access.py b/metrics/interfaces/charts/single_category_charts/access.py index 471083c15..961fe16b3 100644 --- a/metrics/interfaces/charts/single_category_charts/access.py +++ b/metrics/interfaces/charts/single_category_charts/access.py @@ -142,6 +142,8 @@ def generate_chart_output(self) -> ChartOutput: figure=figure, description=description, is_headline=self.is_headline_data, + is_public=self.chart_request_params.is_public, + data_classification=self.chart_request_params.data_classification, ) def _build_chart_figure( diff --git a/metrics/interfaces/charts/subplot_charts/access.py b/metrics/interfaces/charts/subplot_charts/access.py index b07ff7701..9a7bb6afb 100644 --- a/metrics/interfaces/charts/subplot_charts/access.py +++ b/metrics/interfaces/charts/subplot_charts/access.py @@ -156,6 +156,8 @@ def generate_chart_output(self): description=self.build_chart_description(plots_data=plots_data), is_headline=False, is_subplot=True, + is_public=self.chart_request_params.is_public, + data_classification=self.chart_request_params.data_classification, ) @classmethod diff --git a/metrics/interfaces/data_classification/access.py b/metrics/interfaces/data_classification/access.py new file mode 100644 index 000000000..0b8c12b12 --- /dev/null +++ b/metrics/interfaces/data_classification/access.py @@ -0,0 +1,8 @@ +from enum import Enum + +class DataClassification(Enum): + official = "OFFICIAL" + official_sensitive = "OFFICIAL-SENSITIVE" + protective_marking_not_set = "PROTECTIVE MARKING NOT SET" + secret = "SECRET" # nosec #noqa: S105 + top_secret = "TOP SECRET" # nosec #noqa: S105 From 198094fbbfde2309a29380865da37554513fdc11 Mon Sep 17 00:00:00 2001 From: Kathryn Dale Date: Mon, 18 May 2026 09:15:13 +0100 Subject: [PATCH 02/18] Revert to models data classification --- cms/dashboard/models.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cms/dashboard/models.py b/cms/dashboard/models.py index 0928f6de4..333ccee95 100644 --- a/cms/dashboard/models.py +++ b/cms/dashboard/models.py @@ -36,11 +36,11 @@ class DataClassificationLevels(models.TextChoices): - OFFICIAL = (DataClassification.official.value).lower() - OFFICIAL_SENSITIVE = (DataClassification.official_sensitive.value).lower() - PROTECTIVE_MARKING_NOT_SET = (DataClassification.protective_marking_not_set.value).lower() - SECRET = (DataClassification.secret.value).lower() - TOP_SECRET = (DataClassification.top_secret.value).lower() + OFFICIAL = "official" + OFFICIAL_SENSITIVE = "official_sensitive" + PROTECTIVE_MARKING_NOT_SET = "protective_marking_not_set" + SECRET = "secret" # nosec #noqa: S105 + TOP_SECRET = "top_secret" # nosec #noqa: S105 From ab7899b9db242abdb1e5b85e82f40fc5b87013b4 Mon Sep 17 00:00:00 2001 From: Kathryn Dale Date: Mon, 18 May 2026 09:38:16 +0100 Subject: [PATCH 03/18] Wrap text for large watermarks --- metrics/interfaces/charts/common/chart_output.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/metrics/interfaces/charts/common/chart_output.py b/metrics/interfaces/charts/common/chart_output.py index 534232054..98cbc0353 100644 --- a/metrics/interfaces/charts/common/chart_output.py +++ b/metrics/interfaces/charts/common/chart_output.py @@ -1,4 +1,5 @@ from dataclasses import dataclass +import textwrap import plotly.graph_objects as go @@ -9,6 +10,11 @@ WATERMARK_FONT_COLOUR = "rgba(0, 0, 0, 0.25)" WATERMARK_OPACITY = 0.58 +def wrap_text(text: str, max_chars_per_line: int = 20) -> str: + """ + Wrap text into multiple lines using
for watermarks + """ + return "
".join(textwrap.wrap(text, width=max_chars_per_line)) @dataclass class ChartOutput: @@ -22,7 +28,7 @@ class ChartOutput: def __post_init__(self) -> None: if not self.is_public: self._apply_watermark() - + def _apply_watermark(self) -> None: """ @@ -34,9 +40,10 @@ def _apply_watermark(self) -> None: and any downloaded chart artefacts. """ watermark_text = DataClassification[self.data_classification].value + wrapped_watermark_text = wrap_text(watermark_text, 16) self.figure.add_annotation( - text=watermark_text, + text=wrapped_watermark_text, xref="paper", yref="paper", x=0.5, From cf8b0df8b9e6df8a275854777372e6fa6a78680f Mon Sep 17 00:00:00 2001 From: Kathryn Dale Date: Mon, 18 May 2026 14:07:58 +0100 Subject: [PATCH 04/18] CDD-3090: Unit testing --- metrics/domain/models/charts/common.py | 2 +- .../domain/models/charts/subplot_charts.py | 2 +- metrics/domain/models/headline.py | 2 +- .../interfaces/charts/common/chart_output.py | 3 +- .../test_single_category_downloads.py | 4 +- .../charts/test_single_category_charts.py | 2 + .../metrics/domain/models/test_headline.py | 1 + .../charts/common/test_chart_output.py | 181 ++++++++++++++++++ .../interfaces/headlines/test_access.py | 1 + 9 files changed, 192 insertions(+), 6 deletions(-) create mode 100644 tests/unit/metrics/interfaces/charts/common/test_chart_output.py diff --git a/metrics/domain/models/charts/common.py b/metrics/domain/models/charts/common.py index fbfe185f9..b610ceff1 100644 --- a/metrics/domain/models/charts/common.py +++ b/metrics/domain/models/charts/common.py @@ -20,7 +20,7 @@ class BaseChartRequestParams(BaseModel): request: Request | None = None confidence_intervals: bool | None = False confidence_colour: str | None = "" - is_public: bool + is_public: bool | None = True data_classification: str | None = None class Config: diff --git a/metrics/domain/models/charts/subplot_charts.py b/metrics/domain/models/charts/subplot_charts.py index 25e735645..3335edac6 100644 --- a/metrics/domain/models/charts/subplot_charts.py +++ b/metrics/domain/models/charts/subplot_charts.py @@ -42,7 +42,7 @@ class SubplotChartRequestParameters(BaseModel): target_threshold: float | None = None target_threshold_label: str | None = "" request: Request | None = None - is_public: bool + is_public: bool | None = True data_classification: str | None = None subplots: list[Subplots] diff --git a/metrics/domain/models/headline.py b/metrics/domain/models/headline.py index 9a561f198..3f73c16b9 100644 --- a/metrics/domain/models/headline.py +++ b/metrics/domain/models/headline.py @@ -12,7 +12,7 @@ class HeadlineParameters(BaseModel): geography_type: str sex: str age: str - is_public: bool + is_public: bool | None = True data_classification: str | None = None request: Request | None = None diff --git a/metrics/interfaces/charts/common/chart_output.py b/metrics/interfaces/charts/common/chart_output.py index 98cbc0353..c47416c54 100644 --- a/metrics/interfaces/charts/common/chart_output.py +++ b/metrics/interfaces/charts/common/chart_output.py @@ -26,7 +26,7 @@ class ChartOutput: data_classification: str | None = None def __post_init__(self) -> None: - if not self.is_public: + if (not self.is_public) and self.data_classification: self._apply_watermark() @@ -39,6 +39,7 @@ def _apply_watermark(self) -> None: rendered in static SVG exports, interactive Plotly outputs, and any downloaded chart artefacts. """ + watermark_text = DataClassification[self.data_classification].value wrapped_watermark_text = wrap_text(watermark_text, 16) diff --git a/tests/integration/metrics/api/views/downloads/test_single_category_downloads.py b/tests/integration/metrics/api/views/downloads/test_single_category_downloads.py index e477e955e..84e3ad39c 100644 --- a/tests/integration/metrics/api/views/downloads/test_single_category_downloads.py +++ b/tests/integration/metrics/api/views/downloads/test_single_category_downloads.py @@ -62,7 +62,7 @@ def _build_valid_payload(self) -> dict[str, str | list[dict[str, str]]]: "date_from": "2000-01-01", "date_to": datetime.date.today(), } - ], + ] } def _build_valid_headline_payload(self) -> dict[str, str | list[dict[str, str]]]: @@ -78,7 +78,7 @@ def _build_valid_headline_payload(self) -> dict[str, str | list[dict[str, str]]] "age": self.core_headline_data["age"], "sex": self.core_headline_data["sex"], "geography": self.core_headline_data["geography"], - "geography_type": self.core_headline_data["geography_type"], + "geography_type": self.core_headline_data["geography_type"] } ], } diff --git a/tests/unit/metrics/api/serializers/charts/test_single_category_charts.py b/tests/unit/metrics/api/serializers/charts/test_single_category_charts.py index fbf7aa36e..0c9c757ef 100644 --- a/tests/unit/metrics/api/serializers/charts/test_single_category_charts.py +++ b/tests/unit/metrics/api/serializers/charts/test_single_category_charts.py @@ -626,6 +626,7 @@ def test_to_models_returns_correct_models(self): "plots": chart_plots, "y_axis_minimum_value": 0, "y_axis_maximum_value": None, + "is_public": True } serializer = ChartsSerializer(data=valid_data_payload) @@ -644,6 +645,7 @@ def test_to_models_returns_correct_models(self): chart_width=valid_data_payload["chart_width"], x_axis=DEFAULT_X_AXIS, y_axis=DEFAULT_Y_AXIS, + is_public=True ) assert chart_plots_serialized_models == expected_chart_plots_model diff --git a/tests/unit/metrics/domain/models/test_headline.py b/tests/unit/metrics/domain/models/test_headline.py index b78ac6427..e1a0937a5 100644 --- a/tests/unit/metrics/domain/models/test_headline.py +++ b/tests/unit/metrics/domain/models/test_headline.py @@ -13,6 +13,7 @@ class TestHeadlineParameters: "stratum": "default", "sex": "all", "age": "all", + "is_public": True } @pytest.mark.parametrize( diff --git a/tests/unit/metrics/interfaces/charts/common/test_chart_output.py b/tests/unit/metrics/interfaces/charts/common/test_chart_output.py new file mode 100644 index 000000000..1b558ba55 --- /dev/null +++ b/tests/unit/metrics/interfaces/charts/common/test_chart_output.py @@ -0,0 +1,181 @@ +from unittest import mock +import pytest + +import plotly.graph_objects as go + + +from metrics.interfaces.charts.common.chart_output import ChartOutput, wrap_text + +MODULE_PATH = "metrics.interfaces.charts.common.chart_output" + +WATERMARK_FONT_SIZE = 40 +WATERMARK_FONT_COLOUR = "rgba(0, 0, 0, 0.25)" +WATERMARK_OPACITY = 0.58 + +class TestWrapText: + def test_wraps_text_into_br_separated_lines(self): + """ + Given a string longer than max_chars_per_line + When wrap_text() is called + Then the text is wrapped and joined with
+ """ + # Given + text = "This is a long string that should be wrapped properly" + max_chars = 10 + + # When + result = wrap_text(text, max_chars_per_line=max_chars) + + # Then + expected = "
".join([ + "This is a", + "long", + "string", + "that", + "should be", + "wrapped", + "properly", + ]) + assert result == expected + + def test_returns_single_line_when_text_is_short(self): + """ + Given a string shorter than max_chars_per_line + When wrap_text() is called + Then the original text is returned without
+ """ + # Given + text = "Short text" + + # When + result = wrap_text(text, max_chars_per_line=20) + + # Then + assert result == text + + def test_handles_empty_string(self): + """ + Given an empty string + When wrap_text() is called + Then an empty string is returned + """ + # Given + text = "" + + # When + result = wrap_text(text, max_chars_per_line=10) + + # Then + assert result == "" + + from unittest import mock + +class TestPostInitAppliesWatermark: + @mock.patch(f"{MODULE_PATH}.ChartOutput._apply_watermark") + def test_applies_watermark_when_not_public_and_classified( + self, spy_apply_watermark: mock.MagicMock + ): + """ + Given an instance that is not public and has data_classification + When __post_init__() is triggered + Then _apply_watermark() is called + """ + # Given / When + ChartOutput( + figure=go.Figure(), + description="Test chart", + is_headline=False, + is_public=False, + data_classification="official", + ) + + # Then + spy_apply_watermark.assert_called_once() + + @mock.patch(f"{MODULE_PATH}.ChartOutput._apply_watermark") + def test_does_not_apply_watermark_when_public( + self, spy_apply_watermark: mock.MagicMock + ): + """ + Given an instance that is public + When __post_init__() is triggered + Then _apply_watermark() is not called + """ + # Given / When + ChartOutput( + figure=go.Figure(), + description="Test chart", + is_headline=False, + data_classification="official", + ) + + # Then + spy_apply_watermark.assert_not_called() + + @mock.patch(f"{MODULE_PATH}.ChartOutput._apply_watermark") + def test_does_not_apply_watermark_when_no_classification( + self, spy_apply_watermark: mock.MagicMock + ): + """ + Given an instance that has no data_classification + When __post_init__() is triggered + Then _apply_watermark() is not called + """ + # Given / When + ChartOutput( + figure=go.Figure(), + description="Test chart", + is_headline=False, + ) + + # Then + spy_apply_watermark.assert_not_called() + +from unittest import mock +import plotly.graph_objects as go + +class TestApplyWatermark: + @mock.patch(f"{MODULE_PATH}.wrap_text") + @mock.patch(f"{MODULE_PATH}.DataClassification") + def test_adds_wrapped_watermark_annotation( + self, + mock_data_classification, + mock_wrap_text, + ): + """ + Given a ChartOutput with a valid data_classification + When _apply_watermark() is called + Then the watermark text is resolved, wrapped, and added as an annotation + """ + # Given / When (apply is called with postInit on instantiation) + figure = mock.Mock(spec=go.Figure) + + mock_data_classification.__getitem__.return_value.value = "Highly Confidential" + mock_wrap_text.return_value = "Highly
Confidential" + + instance = ChartOutput( + figure=figure, + description="Test", + is_headline=False, + is_public=False, + data_classification="official", + ) + + # Then + mock_data_classification.__getitem__.assert_called_once_with("official") + mock_wrap_text.assert_called_once_with("Highly Confidential", 16) + + figure.add_annotation.assert_called_once_with( + text="Highly
Confidential", + xref="paper", + yref="paper", + x=0.5, + y=0.5, + showarrow=False, + font={ + "size": WATERMARK_FONT_SIZE, + "color": WATERMARK_FONT_COLOUR, + }, + textangle=-30, + opacity=WATERMARK_OPACITY, + ) \ No newline at end of file diff --git a/tests/unit/metrics/interfaces/headlines/test_access.py b/tests/unit/metrics/interfaces/headlines/test_access.py index c2d87e54f..f1c2478aa 100644 --- a/tests/unit/metrics/interfaces/headlines/test_access.py +++ b/tests/unit/metrics/interfaces/headlines/test_access.py @@ -22,6 +22,7 @@ def example_headline_args() -> dict[str, str]: "stratum": "default", "age": "all", "sex": "all", + "is_public": True } From acc678405d6ed66e6f81f7a257966275b45bd4ee Mon Sep 17 00:00:00 2001 From: Kathryn Dale Date: Mon, 18 May 2026 14:08:37 +0100 Subject: [PATCH 05/18] linting --- cms/dashboard/models.py | 3 -- metrics/api/serializers/charts/common.py | 6 ++-- .../charts/single_category_charts.py | 4 +-- .../api/serializers/charts/subplot_charts.py | 8 ++--- metrics/api/serializers/tables.py | 6 ++-- .../domain/models/charts/subplot_charts.py | 2 +- metrics/domain/models/headline.py | 2 +- .../interfaces/charts/common/chart_output.py | 19 ++++++------ .../data_classification/__init__.py | 0 .../interfaces/data_classification/access.py | 7 +++-- .../test_single_category_downloads.py | 4 +-- .../charts/test_single_category_charts.py | 4 +-- .../metrics/domain/models/test_headline.py | 2 +- .../charts/common/test_chart_output.py | 30 +++++++++++-------- .../interfaces/headlines/test_access.py | 2 +- 15 files changed, 49 insertions(+), 50 deletions(-) create mode 100644 metrics/interfaces/data_classification/__init__.py diff --git a/cms/dashboard/models.py b/cms/dashboard/models.py index 333ccee95..f94bb4368 100644 --- a/cms/dashboard/models.py +++ b/cms/dashboard/models.py @@ -12,7 +12,6 @@ from wagtail.search import index from cms import seo -from metrics.interfaces.data_classification.access import DataClassification HEADING_2: str = "h2" HEADING_3: str = "h3" @@ -34,7 +33,6 @@ MAXIMUM_URL_FIELD_LENGTH: int = 400 - class DataClassificationLevels(models.TextChoices): OFFICIAL = "official" OFFICIAL_SENSITIVE = "official_sensitive" @@ -43,7 +41,6 @@ class DataClassificationLevels(models.TextChoices): TOP_SECRET = "top_secret" # nosec #noqa: S105 - class UKHSAPage(Page): """Abstract base class for all page types diff --git a/metrics/api/serializers/charts/common.py b/metrics/api/serializers/charts/common.py index 9f774327e..7fef1440c 100644 --- a/metrics/api/serializers/charts/common.py +++ b/metrics/api/serializers/charts/common.py @@ -95,14 +95,12 @@ class BaseChartsSerializer(serializers.Serializer): default="", ) is_public = serializers.BooleanField( - required=False, - default=False, - help_text=help_texts.IS_PUBLIC_FIELD + required=False, default=False, help_text=help_texts.IS_PUBLIC_FIELD ) data_classification = serializers.CharField( required=False, allow_blank=True, allow_null=True, default=None, - help_text=help_texts.DATA_CLASSIFICATION_FIELD + help_text=help_texts.DATA_CLASSIFICATION_FIELD, ) diff --git a/metrics/api/serializers/charts/single_category_charts.py b/metrics/api/serializers/charts/single_category_charts.py index f07ea9de2..2541a6753 100644 --- a/metrics/api/serializers/charts/single_category_charts.py +++ b/metrics/api/serializers/charts/single_category_charts.py @@ -82,7 +82,7 @@ def to_models(self, request: Request) -> ChartRequestParams: for plot in self.data["plots"]: plot["x_axis"] = x_axis plot["y_axis"] = y_axis - + # If not provided, assume data is public --> is this right? is_public: bool = self.data.get("is_public", True) data_classification: str | None = self.data.get("data_classification") @@ -102,7 +102,7 @@ def to_models(self, request: Request) -> ChartRequestParams: legend_title=self.data.get("legend_title", ""), confidence_intervals=self.data.get("confidence_intervals", False), confidence_colour=self.data.get("confidence_colour", ""), - is_public = is_public, + is_public=is_public, data_classification=data_classification, request=request, ) diff --git a/metrics/api/serializers/charts/subplot_charts.py b/metrics/api/serializers/charts/subplot_charts.py index e6a4aafa2..c16b860e0 100644 --- a/metrics/api/serializers/charts/subplot_charts.py +++ b/metrics/api/serializers/charts/subplot_charts.py @@ -158,16 +158,14 @@ class SubplotChartRequestSerializer(serializers.Serializer): allow_null=True, ) is_public = serializers.BooleanField( - required=False, - default=True, - help_text=help_texts.IS_PUBLIC_FIELD + required=False, default=True, help_text=help_texts.IS_PUBLIC_FIELD ) data_classification = serializers.CharField( required=False, allow_blank=True, allow_null=True, default=None, - help_text=help_texts.DATA_CLASSIFICATION_FIELD + help_text=help_texts.DATA_CLASSIFICATION_FIELD, ) chart_parameters = ChartParametersSerializer() @@ -208,7 +206,7 @@ def to_models(self, request: Request) -> SubplotChartRequestParameters: } ) plot["metric_value_ranges"] = metric_value_ranges - + is_public: bool = self.validated_data.get("is_public", True) data_classification: str | None = self.validated_data.get("data_classification") diff --git a/metrics/api/serializers/tables.py b/metrics/api/serializers/tables.py index 2da56902f..f96d4f480 100644 --- a/metrics/api/serializers/tables.py +++ b/metrics/api/serializers/tables.py @@ -55,8 +55,6 @@ class TablesSerializer(serializers.Serializer): help_text=help_texts.CHART_Y_AXIS, default=DEFAULT_Y_AXIS, ) - - def __init__(self, *args, **kwargs): with contextlib.suppress(OperationalError): @@ -70,8 +68,8 @@ def to_models(self, request: Request) -> ChartRequestParams: chart_width=DEFAULT_CHART_WIDTH, x_axis=self.data.get("x_axis") or DEFAULT_X_AXIS, y_axis=self.data.get("y_axis") or DEFAULT_Y_AXIS, - is_public = self.data.get("is_public", True), - data_classification = self.data.get("data_classification"), + is_public=self.data.get("is_public", True), + data_classification=self.data.get("data_classification"), request=request, ) diff --git a/metrics/domain/models/charts/subplot_charts.py b/metrics/domain/models/charts/subplot_charts.py index 3335edac6..3d92f3322 100644 --- a/metrics/domain/models/charts/subplot_charts.py +++ b/metrics/domain/models/charts/subplot_charts.py @@ -20,7 +20,7 @@ class Subplots(BaseModel): class Config: arbitrary_types_allowed = True - + @property def rbac_permissions(self) -> Iterable["RBACPermission"]: return getattr(self.request, "rbac_permissions", []) diff --git a/metrics/domain/models/headline.py b/metrics/domain/models/headline.py index 3f73c16b9..30b737ef8 100644 --- a/metrics/domain/models/headline.py +++ b/metrics/domain/models/headline.py @@ -68,7 +68,7 @@ def to_dict_for_query(self) -> dict[str, str]: "sex": self.sex_name, "rbac_permissions": self.rbac_permissions, } - + @property def rbac_permissions(self) -> Iterable["RBACPermission"]: return getattr(self.request, "rbac_permissions", []) diff --git a/metrics/interfaces/charts/common/chart_output.py b/metrics/interfaces/charts/common/chart_output.py index c47416c54..695c136fe 100644 --- a/metrics/interfaces/charts/common/chart_output.py +++ b/metrics/interfaces/charts/common/chart_output.py @@ -1,5 +1,5 @@ -from dataclasses import dataclass import textwrap +from dataclasses import dataclass import plotly.graph_objects as go @@ -10,11 +10,13 @@ WATERMARK_FONT_COLOUR = "rgba(0, 0, 0, 0.25)" WATERMARK_OPACITY = 0.58 + def wrap_text(text: str, max_chars_per_line: int = 20) -> str: - """ - Wrap text into multiple lines using
for watermarks - """ - return "
".join(textwrap.wrap(text, width=max_chars_per_line)) + """ + Wrap text into multiple lines using
for watermarks + """ + return "
".join(textwrap.wrap(text, width=max_chars_per_line)) + @dataclass class ChartOutput: @@ -24,12 +26,11 @@ class ChartOutput: is_subplot: bool = False is_public: bool = True data_classification: str | None = None - + def __post_init__(self) -> None: if (not self.is_public) and self.data_classification: self._apply_watermark() - - + def _apply_watermark(self) -> None: """ Adds a diagonal watermark to the Plotly figure. @@ -42,7 +43,7 @@ def _apply_watermark(self) -> None: watermark_text = DataClassification[self.data_classification].value wrapped_watermark_text = wrap_text(watermark_text, 16) - + self.figure.add_annotation( text=wrapped_watermark_text, xref="paper", diff --git a/metrics/interfaces/data_classification/__init__.py b/metrics/interfaces/data_classification/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/metrics/interfaces/data_classification/access.py b/metrics/interfaces/data_classification/access.py index 0b8c12b12..10df6bbcd 100644 --- a/metrics/interfaces/data_classification/access.py +++ b/metrics/interfaces/data_classification/access.py @@ -1,8 +1,9 @@ from enum import Enum + class DataClassification(Enum): official = "OFFICIAL" - official_sensitive = "OFFICIAL-SENSITIVE" + official_sensitive = "OFFICIAL-SENSITIVE" protective_marking_not_set = "PROTECTIVE MARKING NOT SET" - secret = "SECRET" # nosec #noqa: S105 - top_secret = "TOP SECRET" # nosec #noqa: S105 + secret = "SECRET" # nosec #noqa: S105 + top_secret = "TOP SECRET" # nosec #noqa: S105 diff --git a/tests/integration/metrics/api/views/downloads/test_single_category_downloads.py b/tests/integration/metrics/api/views/downloads/test_single_category_downloads.py index 84e3ad39c..e477e955e 100644 --- a/tests/integration/metrics/api/views/downloads/test_single_category_downloads.py +++ b/tests/integration/metrics/api/views/downloads/test_single_category_downloads.py @@ -62,7 +62,7 @@ def _build_valid_payload(self) -> dict[str, str | list[dict[str, str]]]: "date_from": "2000-01-01", "date_to": datetime.date.today(), } - ] + ], } def _build_valid_headline_payload(self) -> dict[str, str | list[dict[str, str]]]: @@ -78,7 +78,7 @@ def _build_valid_headline_payload(self) -> dict[str, str | list[dict[str, str]]] "age": self.core_headline_data["age"], "sex": self.core_headline_data["sex"], "geography": self.core_headline_data["geography"], - "geography_type": self.core_headline_data["geography_type"] + "geography_type": self.core_headline_data["geography_type"], } ], } diff --git a/tests/unit/metrics/api/serializers/charts/test_single_category_charts.py b/tests/unit/metrics/api/serializers/charts/test_single_category_charts.py index 0c9c757ef..71385de0d 100644 --- a/tests/unit/metrics/api/serializers/charts/test_single_category_charts.py +++ b/tests/unit/metrics/api/serializers/charts/test_single_category_charts.py @@ -626,7 +626,7 @@ def test_to_models_returns_correct_models(self): "plots": chart_plots, "y_axis_minimum_value": 0, "y_axis_maximum_value": None, - "is_public": True + "is_public": True, } serializer = ChartsSerializer(data=valid_data_payload) @@ -645,7 +645,7 @@ def test_to_models_returns_correct_models(self): chart_width=valid_data_payload["chart_width"], x_axis=DEFAULT_X_AXIS, y_axis=DEFAULT_Y_AXIS, - is_public=True + is_public=True, ) assert chart_plots_serialized_models == expected_chart_plots_model diff --git a/tests/unit/metrics/domain/models/test_headline.py b/tests/unit/metrics/domain/models/test_headline.py index e1a0937a5..cd39bcb18 100644 --- a/tests/unit/metrics/domain/models/test_headline.py +++ b/tests/unit/metrics/domain/models/test_headline.py @@ -13,7 +13,7 @@ class TestHeadlineParameters: "stratum": "default", "sex": "all", "age": "all", - "is_public": True + "is_public": True, } @pytest.mark.parametrize( diff --git a/tests/unit/metrics/interfaces/charts/common/test_chart_output.py b/tests/unit/metrics/interfaces/charts/common/test_chart_output.py index 1b558ba55..7a06a6470 100644 --- a/tests/unit/metrics/interfaces/charts/common/test_chart_output.py +++ b/tests/unit/metrics/interfaces/charts/common/test_chart_output.py @@ -12,6 +12,7 @@ WATERMARK_FONT_COLOUR = "rgba(0, 0, 0, 0.25)" WATERMARK_OPACITY = 0.58 + class TestWrapText: def test_wraps_text_into_br_separated_lines(self): """ @@ -27,15 +28,17 @@ def test_wraps_text_into_br_separated_lines(self): result = wrap_text(text, max_chars_per_line=max_chars) # Then - expected = "
".join([ - "This is a", - "long", - "string", - "that", - "should be", - "wrapped", - "properly", - ]) + expected = "
".join( + [ + "This is a", + "long", + "string", + "that", + "should be", + "wrapped", + "properly", + ] + ) assert result == expected def test_returns_single_line_when_text_is_short(self): @@ -67,9 +70,10 @@ def test_handles_empty_string(self): # Then assert result == "" - + from unittest import mock + class TestPostInitAppliesWatermark: @mock.patch(f"{MODULE_PATH}.ChartOutput._apply_watermark") def test_applies_watermark_when_not_public_and_classified( @@ -130,10 +134,12 @@ def test_does_not_apply_watermark_when_no_classification( # Then spy_apply_watermark.assert_not_called() - + + from unittest import mock import plotly.graph_objects as go + class TestApplyWatermark: @mock.patch(f"{MODULE_PATH}.wrap_text") @mock.patch(f"{MODULE_PATH}.DataClassification") @@ -178,4 +184,4 @@ def test_adds_wrapped_watermark_annotation( }, textangle=-30, opacity=WATERMARK_OPACITY, - ) \ No newline at end of file + ) diff --git a/tests/unit/metrics/interfaces/headlines/test_access.py b/tests/unit/metrics/interfaces/headlines/test_access.py index f1c2478aa..5e7b1e3c4 100644 --- a/tests/unit/metrics/interfaces/headlines/test_access.py +++ b/tests/unit/metrics/interfaces/headlines/test_access.py @@ -22,7 +22,7 @@ def example_headline_args() -> dict[str, str]: "stratum": "default", "age": "all", "sex": "all", - "is_public": True + "is_public": True, } From ed90c97e49372c6fb791427284c5b47084498254 Mon Sep 17 00:00:00 2001 From: Kathryn Dale Date: Mon, 18 May 2026 15:35:41 +0100 Subject: [PATCH 06/18] update comment --- metrics/api/serializers/charts/single_category_charts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metrics/api/serializers/charts/single_category_charts.py b/metrics/api/serializers/charts/single_category_charts.py index 2541a6753..01d5294c6 100644 --- a/metrics/api/serializers/charts/single_category_charts.py +++ b/metrics/api/serializers/charts/single_category_charts.py @@ -83,7 +83,7 @@ def to_models(self, request: Request) -> ChartRequestParams: plot["x_axis"] = x_axis plot["y_axis"] = y_axis - # If not provided, assume data is public --> is this right? + # If not provided, default to public data is_public: bool = self.data.get("is_public", True) data_classification: str | None = self.data.get("data_classification") From d3ffa63f283d18bcd249a9b7c504eec797a61f7d Mon Sep 17 00:00:00 2001 From: Kathryn Dale Date: Mon, 18 May 2026 16:51:21 +0100 Subject: [PATCH 07/18] wrap watermark in auth_enabled flag --- metrics/interfaces/charts/common/chart_output.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/metrics/interfaces/charts/common/chart_output.py b/metrics/interfaces/charts/common/chart_output.py index 695c136fe..3c6db6452 100644 --- a/metrics/interfaces/charts/common/chart_output.py +++ b/metrics/interfaces/charts/common/chart_output.py @@ -3,6 +3,7 @@ import plotly.graph_objects as go +from metrics.api.settings.auth import AUTH_ENABLED from metrics.interfaces.data_classification.access import DataClassification HEX_COLOUR_BLACK = "#0b0c0c" @@ -28,7 +29,7 @@ class ChartOutput: data_classification: str | None = None def __post_init__(self) -> None: - if (not self.is_public) and self.data_classification: + if (not self.is_public) and self.data_classification and AUTH_ENABLED: self._apply_watermark() def _apply_watermark(self) -> None: From 1254f436bfa9bced4d5a1c01a2dc3b847d2443fe Mon Sep 17 00:00:00 2001 From: Kathryn Dale Date: Mon, 18 May 2026 17:46:46 +0100 Subject: [PATCH 08/18] Update unit tests --- .../interfaces/charts/common/chart_output.py | 2 +- .../charts/common/test_chart_output.py | 32 +++++++++++++++---- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/metrics/interfaces/charts/common/chart_output.py b/metrics/interfaces/charts/common/chart_output.py index 3c6db6452..2af22a975 100644 --- a/metrics/interfaces/charts/common/chart_output.py +++ b/metrics/interfaces/charts/common/chart_output.py @@ -29,7 +29,7 @@ class ChartOutput: data_classification: str | None = None def __post_init__(self) -> None: - if (not self.is_public) and self.data_classification and AUTH_ENABLED: + if (not self.is_public) and (self.data_classification) and (AUTH_ENABLED): self._apply_watermark() def _apply_watermark(self) -> None: diff --git a/tests/unit/metrics/interfaces/charts/common/test_chart_output.py b/tests/unit/metrics/interfaces/charts/common/test_chart_output.py index 7a06a6470..6ebd67ec2 100644 --- a/tests/unit/metrics/interfaces/charts/common/test_chart_output.py +++ b/tests/unit/metrics/interfaces/charts/common/test_chart_output.py @@ -76,7 +76,8 @@ def test_handles_empty_string(self): class TestPostInitAppliesWatermark: @mock.patch(f"{MODULE_PATH}.ChartOutput._apply_watermark") - def test_applies_watermark_when_not_public_and_classified( + @mock.patch(f"{MODULE_PATH}.AUTH_ENABLED", True) + def test_applies_watermark_when_not_public_and_classified_and_auth_enabled( self, spy_apply_watermark: mock.MagicMock ): """ @@ -97,6 +98,7 @@ def test_applies_watermark_when_not_public_and_classified( spy_apply_watermark.assert_called_once() @mock.patch(f"{MODULE_PATH}.ChartOutput._apply_watermark") + @mock.patch(f"{MODULE_PATH}.AUTH_ENABLED", True) def test_does_not_apply_watermark_when_public( self, spy_apply_watermark: mock.MagicMock ): @@ -117,6 +119,7 @@ def test_does_not_apply_watermark_when_public( spy_apply_watermark.assert_not_called() @mock.patch(f"{MODULE_PATH}.ChartOutput._apply_watermark") + @mock.patch(f"{MODULE_PATH}.AUTH_ENABLED", True) def test_does_not_apply_watermark_when_no_classification( self, spy_apply_watermark: mock.MagicMock ): @@ -135,18 +138,35 @@ def test_does_not_apply_watermark_when_no_classification( # Then spy_apply_watermark.assert_not_called() + @mock.patch(f"{MODULE_PATH}.ChartOutput._apply_watermark") + @mock.patch(f"{MODULE_PATH}.AUTH_ENABLED", False) + def test_does_not_apply_watermark_when_no_auth_enabled( + self, spy_apply_watermark: mock.MagicMock + ): + """ + Given an instance that has no data_classification + When __post_init__() is triggered + Then _apply_watermark() is not called + """ + # Given / When + ChartOutput( + figure=go.Figure(), + description="Test chart", + is_headline=False, + is_public=False, + data_classification="official", + ) -from unittest import mock -import plotly.graph_objects as go + # Then + spy_apply_watermark.assert_not_called() class TestApplyWatermark: @mock.patch(f"{MODULE_PATH}.wrap_text") @mock.patch(f"{MODULE_PATH}.DataClassification") + @mock.patch(f"{MODULE_PATH}.AUTH_ENABLED", True) def test_adds_wrapped_watermark_annotation( - self, - mock_data_classification, - mock_wrap_text, + self, mock_data_classification, mock_wrap_text ): """ Given a ChartOutput with a valid data_classification From 44917c7251fcc6941734ab0b955a02387ab9e1a8 Mon Sep 17 00:00:00 2001 From: Matt Reynolds <18287679+mattjreynolds@users.noreply.github.com> Date: Fri, 22 May 2026 10:30:12 +0100 Subject: [PATCH 09/18] CDD-3090: Remove unused variable --- .../unit/metrics/interfaces/charts/common/test_chart_output.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/metrics/interfaces/charts/common/test_chart_output.py b/tests/unit/metrics/interfaces/charts/common/test_chart_output.py index 6ebd67ec2..3aa388c5d 100644 --- a/tests/unit/metrics/interfaces/charts/common/test_chart_output.py +++ b/tests/unit/metrics/interfaces/charts/common/test_chart_output.py @@ -179,7 +179,7 @@ def test_adds_wrapped_watermark_annotation( mock_data_classification.__getitem__.return_value.value = "Highly Confidential" mock_wrap_text.return_value = "Highly
Confidential" - instance = ChartOutput( + ChartOutput( figure=figure, description="Test", is_headline=False, From ba56e9e33d0f61e527e5283215839c30b7db8f2c Mon Sep 17 00:00:00 2001 From: Kathryn Dale Date: Thu, 28 May 2026 09:25:46 +0100 Subject: [PATCH 10/18] Make watermark scale --- metrics/interfaces/charts/common/chart_output.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/metrics/interfaces/charts/common/chart_output.py b/metrics/interfaces/charts/common/chart_output.py index 2af22a975..f68fa7ed6 100644 --- a/metrics/interfaces/charts/common/chart_output.py +++ b/metrics/interfaces/charts/common/chart_output.py @@ -7,7 +7,6 @@ from metrics.interfaces.data_classification.access import DataClassification HEX_COLOUR_BLACK = "#0b0c0c" -WATERMARK_FONT_SIZE = 40 WATERMARK_FONT_COLOUR = "rgba(0, 0, 0, 0.25)" WATERMARK_OPACITY = 0.58 @@ -34,7 +33,7 @@ def __post_init__(self) -> None: def _apply_watermark(self) -> None: """ - Adds a diagonal watermark to the Plotly figure. + Adds a horizontal watermark to the Plotly figure. The watermark is added directly to the figure as a layout annotation using paper coordinates, so it is consistently @@ -43,17 +42,21 @@ def _apply_watermark(self) -> None: """ watermark_text = DataClassification[self.data_classification].value - wrapped_watermark_text = wrap_text(watermark_text, 16) + + width = self.figure.layout.width or 800 + + WATERMARK_FONT_SIZE = max(20, int(width * 0.07)) + self.figure.add_annotation( - text=wrapped_watermark_text, + text=watermark_text, xref="paper", yref="paper", x=0.5, y=0.5, showarrow=False, font={"size": WATERMARK_FONT_SIZE, "color": WATERMARK_FONT_COLOUR}, - textangle=-30, + textangle=0, opacity=WATERMARK_OPACITY, ) From 77cae7df58601b081263f39f33d252e0bf5a457c Mon Sep 17 00:00:00 2001 From: Kathryn Dale Date: Thu, 28 May 2026 10:00:43 +0100 Subject: [PATCH 11/18] Make font size dynamic --- metrics/interfaces/charts/common/chart_output.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/metrics/interfaces/charts/common/chart_output.py b/metrics/interfaces/charts/common/chart_output.py index f68fa7ed6..1303ee24d 100644 --- a/metrics/interfaces/charts/common/chart_output.py +++ b/metrics/interfaces/charts/common/chart_output.py @@ -44,9 +44,10 @@ def _apply_watermark(self) -> None: watermark_text = DataClassification[self.data_classification].value width = self.figure.layout.width or 800 + + max_font_size = int((width * 0.9) / (len(watermark_text) * 0.5)) - WATERMARK_FONT_SIZE = max(20, int(width * 0.07)) - + WATERMARK_FONT_SIZE = max(12, min(max_font_size, 100)) self.figure.add_annotation( text=watermark_text, From fa9a44fdb9dfbfaa33c9e1ee9468b2a01661794b Mon Sep 17 00:00:00 2001 From: Kathryn Dale Date: Thu, 28 May 2026 10:13:25 +0100 Subject: [PATCH 12/18] tweak font size --- metrics/interfaces/charts/common/chart_output.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metrics/interfaces/charts/common/chart_output.py b/metrics/interfaces/charts/common/chart_output.py index 1303ee24d..9581f734d 100644 --- a/metrics/interfaces/charts/common/chart_output.py +++ b/metrics/interfaces/charts/common/chart_output.py @@ -45,7 +45,7 @@ def _apply_watermark(self) -> None: width = self.figure.layout.width or 800 - max_font_size = int((width * 0.9) / (len(watermark_text) * 0.5)) + max_font_size = int((width * 0.85) / (len(watermark_text) * 0.65)) WATERMARK_FONT_SIZE = max(12, min(max_font_size, 100)) From 47fa36ed3af4428700ac33bdaf39a4625afd5b2a Mon Sep 17 00:00:00 2001 From: Kathryn Dale Date: Fri, 29 May 2026 09:31:27 +0100 Subject: [PATCH 13/18] linting --- metrics/interfaces/charts/common/chart_output.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/metrics/interfaces/charts/common/chart_output.py b/metrics/interfaces/charts/common/chart_output.py index 9581f734d..119f0ef52 100644 --- a/metrics/interfaces/charts/common/chart_output.py +++ b/metrics/interfaces/charts/common/chart_output.py @@ -42,12 +42,12 @@ def _apply_watermark(self) -> None: """ watermark_text = DataClassification[self.data_classification].value - + width = self.figure.layout.width or 800 - + max_font_size = int((width * 0.85) / (len(watermark_text) * 0.65)) - WATERMARK_FONT_SIZE = max(12, min(max_font_size, 100)) + watermark_font_size = max(12, min(max_font_size, 100)) self.figure.add_annotation( text=watermark_text, @@ -56,7 +56,7 @@ def _apply_watermark(self) -> None: x=0.5, y=0.5, showarrow=False, - font={"size": WATERMARK_FONT_SIZE, "color": WATERMARK_FONT_COLOUR}, + font={"size": watermark_font_size, "color": WATERMARK_FONT_COLOUR}, textangle=0, opacity=WATERMARK_OPACITY, ) From 31c5ab9f66f9bfe07a7d04653cf74ecfdd2f1320 Mon Sep 17 00:00:00 2001 From: Kathryn Dale Date: Fri, 29 May 2026 10:01:20 +0100 Subject: [PATCH 14/18] Update font height --- metrics/interfaces/charts/common/chart_output.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/metrics/interfaces/charts/common/chart_output.py b/metrics/interfaces/charts/common/chart_output.py index 119f0ef52..d0fab297f 100644 --- a/metrics/interfaces/charts/common/chart_output.py +++ b/metrics/interfaces/charts/common/chart_output.py @@ -46,7 +46,6 @@ def _apply_watermark(self) -> None: width = self.figure.layout.width or 800 max_font_size = int((width * 0.85) / (len(watermark_text) * 0.65)) - watermark_font_size = max(12, min(max_font_size, 100)) self.figure.add_annotation( @@ -54,7 +53,7 @@ def _apply_watermark(self) -> None: xref="paper", yref="paper", x=0.5, - y=0.5, + y=0.8, showarrow=False, font={"size": watermark_font_size, "color": WATERMARK_FONT_COLOUR}, textangle=0, From 5809a7da11b2ef58782f4f20d352eb47df089e5c Mon Sep 17 00:00:00 2001 From: Kathryn Dale Date: Fri, 29 May 2026 10:01:27 +0100 Subject: [PATCH 15/18] Update tests --- .../charts/common/test_chart_output.py | 84 +++---------------- 1 file changed, 12 insertions(+), 72 deletions(-) diff --git a/tests/unit/metrics/interfaces/charts/common/test_chart_output.py b/tests/unit/metrics/interfaces/charts/common/test_chart_output.py index 3aa388c5d..19aa0a92a 100644 --- a/tests/unit/metrics/interfaces/charts/common/test_chart_output.py +++ b/tests/unit/metrics/interfaces/charts/common/test_chart_output.py @@ -8,72 +8,10 @@ MODULE_PATH = "metrics.interfaces.charts.common.chart_output" -WATERMARK_FONT_SIZE = 40 WATERMARK_FONT_COLOUR = "rgba(0, 0, 0, 0.25)" WATERMARK_OPACITY = 0.58 -class TestWrapText: - def test_wraps_text_into_br_separated_lines(self): - """ - Given a string longer than max_chars_per_line - When wrap_text() is called - Then the text is wrapped and joined with
- """ - # Given - text = "This is a long string that should be wrapped properly" - max_chars = 10 - - # When - result = wrap_text(text, max_chars_per_line=max_chars) - - # Then - expected = "
".join( - [ - "This is a", - "long", - "string", - "that", - "should be", - "wrapped", - "properly", - ] - ) - assert result == expected - - def test_returns_single_line_when_text_is_short(self): - """ - Given a string shorter than max_chars_per_line - When wrap_text() is called - Then the original text is returned without
- """ - # Given - text = "Short text" - - # When - result = wrap_text(text, max_chars_per_line=20) - - # Then - assert result == text - - def test_handles_empty_string(self): - """ - Given an empty string - When wrap_text() is called - Then an empty string is returned - """ - # Given - text = "" - - # When - result = wrap_text(text, max_chars_per_line=10) - - # Then - assert result == "" - - from unittest import mock - - class TestPostInitAppliesWatermark: @mock.patch(f"{MODULE_PATH}.ChartOutput._apply_watermark") @mock.patch(f"{MODULE_PATH}.AUTH_ENABLED", True) @@ -162,22 +100,24 @@ def test_does_not_apply_watermark_when_no_auth_enabled( class TestApplyWatermark: - @mock.patch(f"{MODULE_PATH}.wrap_text") @mock.patch(f"{MODULE_PATH}.DataClassification") @mock.patch(f"{MODULE_PATH}.AUTH_ENABLED", True) - def test_adds_wrapped_watermark_annotation( - self, mock_data_classification, mock_wrap_text + def test_adds_watermark_annotation( + self, mock_data_classification ): """ Given a ChartOutput with a valid data_classification When _apply_watermark() is called - Then the watermark text is resolved, wrapped, and added as an annotation + Then the watermark text is resolved, and added as a scaled annotation """ # Given / When (apply is called with postInit on instantiation) figure = mock.Mock(spec=go.Figure) + # mock layout.width for scaling logic + figure.layout.width = 800 + expected_font_size = max(12, min(int((800 * 0.85) / (len("Highly Confidential") * 0.65)), 100)) + mock_data_classification.__getitem__.return_value.value = "Highly Confidential" - mock_wrap_text.return_value = "Highly
Confidential" ChartOutput( figure=figure, @@ -187,21 +127,21 @@ def test_adds_wrapped_watermark_annotation( data_classification="official", ) + # Then mock_data_classification.__getitem__.assert_called_once_with("official") - mock_wrap_text.assert_called_once_with("Highly Confidential", 16) figure.add_annotation.assert_called_once_with( - text="Highly
Confidential", + text="Highly Confidential", xref="paper", yref="paper", x=0.5, - y=0.5, + y=0.8, showarrow=False, font={ - "size": WATERMARK_FONT_SIZE, + "size": expected_font_size, "color": WATERMARK_FONT_COLOUR, }, - textangle=-30, + textangle=0, opacity=WATERMARK_OPACITY, ) From 0f480926c83724b4a8ff606b3c45485593c9b0b7 Mon Sep 17 00:00:00 2001 From: Kathryn Dale Date: Fri, 29 May 2026 10:26:09 +0100 Subject: [PATCH 16/18] remove unused function --- metrics/interfaces/charts/common/chart_output.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/metrics/interfaces/charts/common/chart_output.py b/metrics/interfaces/charts/common/chart_output.py index d0fab297f..c75d56197 100644 --- a/metrics/interfaces/charts/common/chart_output.py +++ b/metrics/interfaces/charts/common/chart_output.py @@ -11,13 +11,6 @@ WATERMARK_OPACITY = 0.58 -def wrap_text(text: str, max_chars_per_line: int = 20) -> str: - """ - Wrap text into multiple lines using
for watermarks - """ - return "
".join(textwrap.wrap(text, width=max_chars_per_line)) - - @dataclass class ChartOutput: figure: go.Figure From ed3836196886ca325d9f183c7bc56655dbb7c1f2 Mon Sep 17 00:00:00 2001 From: Kathryn Dale Date: Fri, 29 May 2026 10:34:33 +0100 Subject: [PATCH 17/18] update import --- .../unit/metrics/interfaces/charts/common/test_chart_output.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/metrics/interfaces/charts/common/test_chart_output.py b/tests/unit/metrics/interfaces/charts/common/test_chart_output.py index 19aa0a92a..90620d0eb 100644 --- a/tests/unit/metrics/interfaces/charts/common/test_chart_output.py +++ b/tests/unit/metrics/interfaces/charts/common/test_chart_output.py @@ -4,7 +4,7 @@ import plotly.graph_objects as go -from metrics.interfaces.charts.common.chart_output import ChartOutput, wrap_text +from metrics.interfaces.charts.common.chart_output import ChartOutput MODULE_PATH = "metrics.interfaces.charts.common.chart_output" From 38e0bd87b5270581f16645f87ae33f757fea8618 Mon Sep 17 00:00:00 2001 From: Kathryn Dale Date: Fri, 29 May 2026 10:39:55 +0100 Subject: [PATCH 18/18] linting --- metrics/interfaces/charts/common/chart_output.py | 1 - .../interfaces/charts/common/test_chart_output.py | 9 ++++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/metrics/interfaces/charts/common/chart_output.py b/metrics/interfaces/charts/common/chart_output.py index c75d56197..e0be4ae74 100644 --- a/metrics/interfaces/charts/common/chart_output.py +++ b/metrics/interfaces/charts/common/chart_output.py @@ -1,4 +1,3 @@ -import textwrap from dataclasses import dataclass import plotly.graph_objects as go diff --git a/tests/unit/metrics/interfaces/charts/common/test_chart_output.py b/tests/unit/metrics/interfaces/charts/common/test_chart_output.py index 90620d0eb..51966e064 100644 --- a/tests/unit/metrics/interfaces/charts/common/test_chart_output.py +++ b/tests/unit/metrics/interfaces/charts/common/test_chart_output.py @@ -102,9 +102,7 @@ def test_does_not_apply_watermark_when_no_auth_enabled( class TestApplyWatermark: @mock.patch(f"{MODULE_PATH}.DataClassification") @mock.patch(f"{MODULE_PATH}.AUTH_ENABLED", True) - def test_adds_watermark_annotation( - self, mock_data_classification - ): + def test_adds_watermark_annotation(self, mock_data_classification): """ Given a ChartOutput with a valid data_classification When _apply_watermark() is called @@ -115,7 +113,9 @@ def test_adds_watermark_annotation( # mock layout.width for scaling logic figure.layout.width = 800 - expected_font_size = max(12, min(int((800 * 0.85) / (len("Highly Confidential") * 0.65)), 100)) + expected_font_size = max( + 12, min(int((800 * 0.85) / (len("Highly Confidential") * 0.65)), 100) + ) mock_data_classification.__getitem__.return_value.value = "Highly Confidential" @@ -127,7 +127,6 @@ def test_adds_watermark_annotation( data_classification="official", ) - # Then mock_data_classification.__getitem__.assert_called_once_with("official")