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/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/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/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 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 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" 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 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/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 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/__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, + ), ] }, ) 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/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", ( 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/app/utils/test_templates.py b/tests/app/utils/test_templates.py index dc492f043c..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", ) @@ -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, 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",