From 39b5a5032fbbbffcfa4db1bb798c2fab9a106720 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 7 Nov 2025 15:12:28 +0000 Subject: [PATCH 01/10] Add a strict variant of `JSONModel` This will raise an error if one of the attributes is missing from the dictionary. This is the same way that the serialised models work in the API. --- app/models/__init__.py | 4 +++- tests/app/models/test_base_model.py | 13 ++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/app/models/__init__.py b/app/models/__init__.py index 396114b441..ec1250249f 100644 --- a/app/models/__init__.py +++ b/app/models/__init__.py @@ -13,7 +13,7 @@ class JSONModelMeta(SerialisedModelMeta, ABCMeta): @total_ordering -class JSONModel(SerialisedModel, ABC, metaclass=JSONModelMeta): +class StrictJSONModel(SerialisedModel, ABC, metaclass=JSONModelMeta): @property @abstractmethod def __sort_attribute__(self): @@ -42,6 +42,8 @@ def __eq__(self, other): def __hash__(self): return hash(self.id) + +class JSONModel(StrictJSONModel): def __init__(self, _dict): # in the case of a bad request _dict may be `None` self._dict = _dict or {} diff --git a/tests/app/models/test_base_model.py b/tests/app/models/test_base_model.py index 06eff72b67..8cff6876fb 100644 --- a/tests/app/models/test_base_model.py +++ b/tests/app/models/test_base_model.py @@ -1,6 +1,6 @@ import pytest -from app.models import JSONModel +from app.models import JSONModel, StrictJSONModel def test_looks_up_from_dict(): @@ -58,6 +58,17 @@ class Custom(JSONModel): assert str(e.value) == "'Custom' object has no attribute 'foo'" +def test_strict_model_raises_keyerror_if_item_missing_from_dict_on_instantiation(): + class Custom(StrictJSONModel): + foo: str + __sort_attribute__ = "foo" + + with pytest.raises(KeyError) as e: + Custom({}) + + assert str(e.value) == "'foo'" + + @pytest.mark.parametrize( "json_response", ( From d80a1d26dbfe5542ccaca716084b20bb274fb12a Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 7 Nov 2025 15:13:44 +0000 Subject: [PATCH 02/10] Use StrictJSONModel for API keys --- app/models/api_key.py | 4 ++-- tests/__init__.py | 11 ++++++++-- tests/app/main/forms/test_create_key_form.py | 21 ++++++++++++-------- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/app/models/api_key.py b/app/models/api_key.py index 8f279b60f8..83b9fac075 100644 --- a/app/models/api_key.py +++ b/app/models/api_key.py @@ -3,11 +3,11 @@ from flask import abort -from app.models import JSONModel, ModelList +from app.models import ModelList, StrictJSONModel from app.notify_client.api_key_api_client import api_key_api_client -class APIKey(JSONModel): +class APIKey(StrictJSONModel): created_at: datetime created_by: Any expiry_date: datetime diff --git a/tests/__init__.py b/tests/__init__.py index 131a4a67cc..db12474dab 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -382,8 +382,15 @@ def template_version_json(service_id, id_, created_by, version=1, created_at=Non return template -def api_key_json(id_, name, expiry_date=None, key_type="normal"): - return {"id": id_, "name": name, "expiry_date": expiry_date, "key_type": key_type} +def api_key_json(id_, name, expiry_date=None, created_at=None, created_by=None, key_type="normal"): + return { + "id": id_, + "name": name, + "expiry_date": expiry_date, + "key_type": key_type, + "created_at": created_at, + "created_by": created_by, + } def invite_json( diff --git a/tests/app/main/forms/test_create_key_form.py b/tests/app/main/forms/test_create_key_form.py index b7ea82b3af..3e9b412e36 100644 --- a/tests/app/main/forms/test_create_key_form.py +++ b/tests/app/main/forms/test_create_key_form.py @@ -1,8 +1,11 @@ +from uuid import uuid4 + import pytest from werkzeug.datastructures import MultiDict from app.main.forms import CreateKeyForm from app.models.api_key import APIKeys +from tests import api_key_json @pytest.mark.parametrize( @@ -22,14 +25,16 @@ def test_return_validation_error_when_key_name_exists( "app.models.api_key.api_key_api_client.get_api_keys", return_value={ "apiKeys": [ - { - "name": "some key", - "expiry_date": expiry_date, - }, - { - "name": "another key", - "expiry_date": None, - }, + api_key_json( + id_=str(uuid4()), + name="some key", + expiry_date=expiry_date, + ), + api_key_json( + id_=str(uuid4()), + name="another key", + expiry_date=None, + ), ] }, ) From 4087a77acb4df2dabea4886cb4f9849664a60577 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 7 Nov 2025 15:14:36 +0000 Subject: [PATCH 03/10] Use StrictJSONModel for contact lists --- app/models/contact_list.py | 4 ++-- tests/app/models/test_contact_list.py | 3 ++- tests/conftest.py | 14 +++++++------- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/app/models/contact_list.py b/app/models/contact_list.py index 71118f1016..0fc39a478b 100644 --- a/app/models/contact_list.py +++ b/app/models/contact_list.py @@ -8,7 +8,7 @@ from notifications_utils.recipients import RecipientCSV from werkzeug.utils import cached_property -from app.models import JSONModel, ModelList +from app.models import ModelList, StrictJSONModel from app.models.job import PaginatedJobsAndScheduledJobs from app.notify_client.contact_list_api_client import contact_list_api_client from app.s3_client.s3_csv_client import ( @@ -20,7 +20,7 @@ from app.utils.templates import get_sample_template -class ContactList(JSONModel): +class ContactList(StrictJSONModel): id: Any created_at: datetime created_by: Any diff --git a/tests/app/models/test_contact_list.py b/tests/app/models/test_contact_list.py index 0b9d8ec79b..e3c2e3cfa1 100644 --- a/tests/app/models/test_contact_list.py +++ b/tests/app/models/test_contact_list.py @@ -1,9 +1,10 @@ from app.models.contact_list import ContactList from app.models.job import PaginatedJobs +from tests import contact_list_json def test_get_jobs(mock_get_jobs): - contact_list = ContactList({"id": "a", "service_id": "b"}) + contact_list = ContactList(contact_list_json(id_="a", service_id="b")) assert isinstance(contact_list.get_jobs(page=123), PaginatedJobs) # mock_get_jobs mocks the underlying API client method, not # contact_list.get_jobs diff --git a/tests/conftest.py b/tests/conftest.py index bb85aaaa1e..accf6760ac 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1913,13 +1913,13 @@ def _create( row_count, template_type, ): - return { - "service_id": service_id, - "upload_id": upload_id, - "original_file_name": original_file_name, - "row_count": row_count, - "template_type": template_type, - } + return contact_list_json( + id_=upload_id, + service_id=service_id, + original_file_name=original_file_name, + row_count=row_count, + template_type=template_type, + ) return mocker.patch( "app.contact_list_api_client.create_contact_list", From 3c02006ea37ef6e10bdf9c44d8fcdfacdec88d0d Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 7 Nov 2025 15:15:22 +0000 Subject: [PATCH 04/10] Use StrictJSONModel for letter rates --- app/models/letter_rates.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/letter_rates.py b/app/models/letter_rates.py index c33d25b00f..39c85e2ea9 100644 --- a/app/models/letter_rates.py +++ b/app/models/letter_rates.py @@ -1,11 +1,11 @@ from datetime import datetime from typing import Any -from app.models import JSONModel, ModelList +from app.models import ModelList, StrictJSONModel from app.notify_client.letter_rate_api_client import letter_rate_api_client -class LetterRate(JSONModel): +class LetterRate(StrictJSONModel): sheet_count: int rate: float post_class: Any From ae75ceb10a784cdefe4d84e49dbf8d32e0ad4063 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 7 Nov 2025 15:15:36 +0000 Subject: [PATCH 05/10] Use StrictJSONModel for text message rates --- app/models/sms_rate.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/sms_rate.py b/app/models/sms_rate.py index 015955a7b2..3d4f67b471 100644 --- a/app/models/sms_rate.py +++ b/app/models/sms_rate.py @@ -1,11 +1,11 @@ from datetime import datetime from app.formatters import format_pennies_as_currency -from app.models import JSONModel +from app.models import StrictJSONModel from app.notify_client.sms_rate_client import sms_rate_api_client -class SMSRate(JSONModel): +class SMSRate(StrictJSONModel): rate: float valid_from: datetime From 4a65fe70e4be8d9026c086a259e11b510e35ec26 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 7 Nov 2025 15:17:16 +0000 Subject: [PATCH 06/10] Use StrictJSONModel for report requests --- app/models/report_request.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/report_request.py b/app/models/report_request.py index c488e6bb73..55b58ba909 100644 --- a/app/models/report_request.py +++ b/app/models/report_request.py @@ -5,11 +5,11 @@ from notifications_utils.s3 import s3download from app import report_request_api_client -from app.models import JSONModel +from app.models import StrictJSONModel from app.s3_client import check_s3_object_exists -class ReportRequest(JSONModel): +class ReportRequest(StrictJSONModel): id: Any user_id: Any service_id: Any From 3929dfa7f659b6934f6fd5ac90ae091a3b4f65e2 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 7 Nov 2025 16:22:28 +0000 Subject: [PATCH 07/10] Use StrictJSONModel for service join requests --- app/models/service.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/models/service.py b/app/models/service.py index 1d54d324b2..1dc0629a5b 100644 --- a/app/models/service.py +++ b/app/models/service.py @@ -13,7 +13,7 @@ SIGN_IN_METHOD_TEXT, SIGN_IN_METHOD_TEXT_OR_EMAIL, ) -from app.models import JSONModel +from app.models import JSONModel, StrictJSONModel from app.models.api_key import APIKeys from app.models.branding import EmailBranding, LetterBranding from app.models.contact_list import ContactLists @@ -707,7 +707,7 @@ class Services(SerialisedModelCollection): model = Service -class ServiceJoinRequest(JSONModel): +class ServiceJoinRequest(StrictJSONModel): id: Any requester: Any service_id: Any @@ -717,8 +717,6 @@ class ServiceJoinRequest(JSONModel): reason: str status: str contacted_service_users: list[str] - requested_service: Any - permissions: list[str] __sort_attribute__ = "id" From 6bf0041e096c4728faee9727145cdf26bb9d3b2d Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 10 Feb 2026 16:39:41 +0000 Subject: [PATCH 08/10] Use StrictJSONModel for template email files --- app/models/template_email_file.py | 4 ++-- tests/app/main/views/test_send.py | 2 ++ tests/app/main/views/test_templates.py | 3 +++ tests/app/utils/test_templates.py | 18 +++++++++++++++++- 4 files changed, 24 insertions(+), 3 deletions(-) diff --git a/app/models/template_email_file.py b/app/models/template_email_file.py index 816c0c4cac..197ef7c58f 100644 --- a/app/models/template_email_file.py +++ b/app/models/template_email_file.py @@ -5,7 +5,7 @@ from notifications_utils.base64_uuid import uuid_to_base64 from notifications_utils.serialised_model import SerialisedModelCollection -from app.models import JSONModel +from app.models import StrictJSONModel from app.s3_client.s3_template_email_file_upload_client import upload_template_email_file_to_s3 @@ -13,7 +13,7 @@ def _get_file_location(file_id: uuid, service_id: uuid) -> str: return f"{service_id}/{file_id}" -class TemplateEmailFile(JSONModel): +class TemplateEmailFile(StrictJSONModel): id: Any service_id: Any template_id: Any diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 8abd965f72..156de6eba9 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -1570,12 +1570,14 @@ def test_send_one_off_has_correct_page_title( "retention_period": 26, "id": str(uuid.UUID(int=1, version=4)), "link_text": None, + "validate_users_email": True, }, { "filename": "picture.png", "retention_period": 90, "id": str(uuid.UUID(int=2, version=4)), "link_text": None, + "validate_users_email": True, }, ], "((one)) ((example.pdf)) ((two)) ((picture.png))", diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index dad27642dd..f40be47284 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -4737,6 +4737,7 @@ def test_letter_attachment_preview_image_shows_overlay_when_content_outside_prin "retention_period": 26, "id": str(uuid.UUID(int=1, version=4)), "link_text": None, + "validate_users_email": True, } ], "Manage files", @@ -4752,12 +4753,14 @@ def test_letter_attachment_preview_image_shows_overlay_when_content_outside_prin "retention_period": 26, "id": str(uuid.UUID(int=1, version=4)), "link_text": None, + "validate_users_email": True, }, { "filename": "picture.png", "retention_period": 90, "id": str(uuid.UUID(int=2, version=4)), "link_text": None, + "validate_users_email": True, }, ], "Manage files", diff --git a/tests/app/utils/test_templates.py b/tests/app/utils/test_templates.py index dc492f043c..9957a0c12f 100644 --- a/tests/app/utils/test_templates.py +++ b/tests/app/utils/test_templates.py @@ -1219,6 +1219,7 @@ def test_TemplateChange_placeholders_removed(old_template, new_template, placeho "retention_period": 26, "id": str(UUID(int=1, version=4)), "link_text": None, + "validate_users_email": True, } ], } @@ -1235,6 +1236,7 @@ def test_TemplateChange_placeholders_removed(old_template, new_template, placeho "retention_period": 26, "id": str(UUID(int=1, version=4)), "link_text": None, + "validate_users_email": True, } ], } @@ -1256,6 +1258,7 @@ def test_TemplateChange_placeholders_removed(old_template, new_template, placeho "retention_period": 26, "id": str(UUID(int=1, version=4)), "link_text": None, + "validate_users_email": True, } ], } @@ -1272,6 +1275,7 @@ def test_TemplateChange_placeholders_removed(old_template, new_template, placeho "retention_period": 26, "id": str(UUID(int=1, version=4)), "link_text": None, + "validate_users_email": True, } ], } @@ -1293,12 +1297,14 @@ def test_TemplateChange_placeholders_removed(old_template, new_template, placeho "retention_period": 26, "id": str(UUID(int=1, version=4)), "link_text": None, + "validate_users_email": True, }, { "filename": "3.pdf", "retention_period": 26, "id": str(UUID(int=2, version=4)), "link_text": None, + "validate_users_email": True, }, ], } @@ -1315,12 +1321,14 @@ def test_TemplateChange_placeholders_removed(old_template, new_template, placeho "retention_period": 26, "id": str(UUID(int=1, version=4)), "link_text": None, + "validate_users_email": True, }, { "filename": "3.pdf", "retention_period": 26, "id": str(UUID(int=2, version=4)), "link_text": None, + "validate_users_email": True, }, ], } @@ -1341,7 +1349,15 @@ def test_TemplateChange_email_files_removed( @pytest.mark.parametrize("service_has_api_keys", (True, False)) def test_TemplateChange_email_files_and_placeholders_removed(service_has_api_keys, fake_uuid): - email_file_data = {"filename": "2.pdf", "retention_period": 26, "id": fake_uuid, "link_text": None} + email_file_data = { + "filename": "2.pdf", + "retention_period": 26, + "id": fake_uuid, + "link_text": None, + "validate_users_email": True, + "service_id": fake_uuid, + "template_id": fake_uuid, + } old_template = EmailPreviewTemplate( { "id": fake_uuid, From 08aa3b325287e8a2412fdc07ecc7d03724d3fdc4 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 10 Feb 2026 16:50:40 +0000 Subject: [PATCH 09/10] Use StrictJSONModel for letter attachments --- app/utils/templates.py | 4 ++-- tests/app/utils/test_templates.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/utils/templates.py b/app/utils/templates.py index 31b84881ec..c7ddccffb4 100644 --- a/app/utils/templates.py +++ b/app/utils/templates.py @@ -17,7 +17,7 @@ from ordered_set import OrderedSet from app.extensions import redis_client -from app.models import JSONModel +from app.models import StrictJSONModel from app.models.template_email_file import TemplateEmailFiles from app.notify_client import cache @@ -317,7 +317,7 @@ def placeholders(self): return OrderedSet([placeholder for placeholder in self.all_placeholders if placeholder not in self.filenames]) -class LetterAttachment(JSONModel): +class LetterAttachment(StrictJSONModel): id: Any original_filename: Any page_count: Any diff --git a/tests/app/utils/test_templates.py b/tests/app/utils/test_templates.py index 9957a0c12f..e28d6ca403 100644 --- a/tests/app/utils/test_templates.py +++ b/tests/app/utils/test_templates.py @@ -651,7 +651,7 @@ def test_letter_image_template_marks_first_page_of_attachment(mocker, fake_uuid) "content": "Content", "subject": "Subject", "template_type": "letter", - "letter_attachment": {"id": fake_uuid, "page_count": 3}, + "letter_attachment": {"id": fake_uuid, "page_count": 3, "original_filename": "example.pdf"}, }, image_url="http://example.com/endpoint.png", ) From 53ddc8b42c7638d35722bd7514baa16f631b452b Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 10 Feb 2026 17:06:05 +0000 Subject: [PATCH 10/10] Use StrictJSONModel for unsubscribe request reports --- app/models/unsubscribe_requests_report.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/unsubscribe_requests_report.py b/app/models/unsubscribe_requests_report.py index 7bb67f5e39..56d38ee768 100644 --- a/app/models/unsubscribe_requests_report.py +++ b/app/models/unsubscribe_requests_report.py @@ -4,12 +4,12 @@ from flask import abort from app.formatters import format_date_human, format_time, get_human_day -from app.models import JSONModel, ModelList +from app.models import ModelList, StrictJSONModel from app.notify_client.service_api_client import service_api_client from app.utils.time import to_utc_string -class UnsubscribeRequestsReport(JSONModel): +class UnsubscribeRequestsReport(StrictJSONModel): service_id: Any count: int batch_id: Any