From 58bf74395c0e913d111592fb73a82f66b1a60be5 Mon Sep 17 00:00:00 2001 From: Jumana B Date: Mon, 19 Aug 2024 11:54:50 -0400 Subject: [PATCH] We want to save the reply_to_email_addresses in the cache (#2255) * start * Adding reply_to_adress in a Service * fix * Fix deserialization * add tests for testing service serilization --- app/models.py | 10 ++++- app/schemas.py | 71 +++++++++++++++++++++++++---- tests/app/dao/test_services_dao.py | 72 +++++++++++++++++++++--------- tests/app/service/test_rest.py | 44 +++++++++++++++--- 4 files changed, 159 insertions(+), 38 deletions(-) diff --git a/app/models.py b/app/models.py index e6b1ca7fe9..ac2ceb9fee 100644 --- a/app/models.py +++ b/app/models.py @@ -608,8 +608,14 @@ def from_json(cls, data): fields.pop("letter_contact_block", None) fields.pop("email_branding", None) fields["sms_daily_limit"] = fields.get("sms_daily_limit", 100) - - return cls(**fields) + reply_to_addresses = fields.get("reply_to_email_addresses", None) + fields.pop("reply_to_email_addresses", None) + current_service = cls(**fields) + # If reply_to_addresses were in the JSON, add them to the service + if reply_to_addresses: + current_service.reply_to_email_addresses = [ServiceEmailReplyTo(**addr) for addr in reply_to_addresses] + + return current_service def get_inbound_number(self): if self.inbound_number and self.inbound_number.active: diff --git a/app/schemas.py b/app/schemas.py index bbbdcf9077..bd240183b0 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -26,7 +26,7 @@ from app import db, marshmallow, models from app.dao.permissions_dao import permission_dao -from app.models import ServicePermission +from app.models import ServiceEmailReplyTo, ServicePermission from app.utils import get_template_instance @@ -267,6 +267,26 @@ class ServiceSchema(BaseSchema, UUIDsAsStringsMixin): letter_contact_block = fields.Method(serialize="get_letter_contact") go_live_at = field_for(models.Service, "go_live_at", format="%Y-%m-%d %H:%M:%S.%f") organisation_notes = field_for(models.Service, "organisation_notes") + reply_to_email_addresses = fields.Method("serialize_reply_to_email_addresses", "deserialize_reply_to_email_addresses") + + def serialize_reply_to_email_addresses(self, service): + return [ + { + "id": str(reply_to.id), + "email_address": reply_to.email_address, + "is_default": reply_to.is_default, + "archived": reply_to.archived, + } + for reply_to in service.reply_to_email_addresses + ] + + def deserialize_reply_to_email_addresses(self, value): + if isinstance(value, list): + return [ + ServiceEmailReplyTo(email_address=addr["email_address"], is_default=addr["is_default"], archived=addr["archived"]) + for addr in value + ] + return [] def get_letter_logo_filename(self, service): return service.letter_branding and service.letter_branding.filename @@ -298,7 +318,6 @@ class Meta(BaseSchema.Meta): "api_keys", "letter_contacts", "jobs", - "reply_to_email_addresses", "service_sms_senders", "templates", "updated_at", @@ -315,16 +334,49 @@ def validate_permissions(self, value): duplicates = list(set([x for x in permissions if permissions.count(x) > 1])) raise ValidationError("Duplicate Service Permission: {}".format(duplicates)) + @validates("reply_to_email_addresses") + def validate_reply_to_email_addresses(self, value): + if not value: + value = [] + + for addr in value: + if not isinstance(addr, dict): + raise ValidationError("Each reply_to_email_address must be a dictionary") + + required_keys = {"email_address", "is_default", "archived"} + if not all(key in addr for key in required_keys): + raise ValidationError(f"Each reply_to_email_address must contain keys: {required_keys}") + + if not isinstance(addr["email_address"], str): + raise ValidationError("email_address must be a string") + + if not isinstance(addr["is_default"], bool): + raise ValidationError("is_default must be a boolean") + + if not isinstance(addr["archived"], bool): + raise ValidationError("archived must be a boolean") + @pre_load() def format_for_data_model(self, in_data, **kwargs): - if isinstance(in_data, dict) and "permissions" in in_data: - str_permissions = in_data["permissions"] - permissions = [] - for p in str_permissions: - permission = ServicePermission(service_id=in_data["id"], permission=p) - permissions.append(permission) + if isinstance(in_data, dict): + if "permissions" in in_data: + str_permissions = in_data["permissions"] + permissions = [] + for p in str_permissions: + permission = ServicePermission(service_id=in_data["id"], permission=p) + permissions.append(permission) + in_data["permissions"] = permissions + + if "reply_to_email_addresses" in in_data: + reply_to_addresses = in_data["reply_to_email_addresses"] + formatted_addresses = [] + for addr in reply_to_addresses: + formatted_addr = ServiceEmailReplyTo( + email_address=addr["email_address"], is_default=addr["is_default"], archived=addr["archived"] + ) + formatted_addresses.append(formatted_addr) + in_data["reply_to_email_addresses"] = formatted_addresses - in_data["permissions"] = permissions return in_data @@ -776,6 +828,7 @@ class Meta: email_from = fields.String() created_by_id = fields.UUID() version = fields.Integer() + reply_to_email_addresses = fields.List(fields.String()) class ApiKeyHistorySchema(Schema): diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index ef57322c9c..21f15aa96a 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -81,6 +81,7 @@ create_notification, create_notification_history, create_organisation, + create_reply_to_email, create_service, create_service_with_defined_sms_sender, create_service_with_inbound_number, @@ -607,30 +608,61 @@ def test_get_service_by_id_returns_service(notify_db_session): assert dao_fetch_service_by_id(service.id).name == "testing" -def test_get_service_by_id_uses_redis_cache_when_use_cache_specified(notify_db_session, mocker): - sample_service = create_service(service_name="testing", email_from="testing") - service_json = {"data": service_schema.dump(sample_service)} +class TestServiceCache: + def test_get_service_by_id_uses_redis_cache_when_use_cache_specified(self, notify_db_session, mocker): + sample_service = create_service(service_name="testing", email_from="testing") + service_json = {"data": service_schema.dump(sample_service)} + + service_json["data"]["all_template_folders"] = ["b5035a31-b1da-42f8-b2b8-ce2acaa0b819"] + service_json["data"]["annual_billing"] = ["8676fa80-a97b-43e7-8318-ee905de2d652", "a0751f79-984b-4d9e-9edd-42457fd458e9"] + service_json["data"]["email_branding"] = "d51a41b2-c420-48a9-a8c5-e88444013020" + service_json["data"]["inbound_number"] = "aa129e4b-d37c-493a-84da-62a31a8199e3" + service_json["data"]["inbound_sms"] = ["fsdfdsfdsfdsfdsfsdf"] + service_json["data"]["service_callback_api"] = ["wfeewfwefewfewfewfewfew"] + service_json["data"]["service_data_retention"] = ["fdsfsdfsdfsdfsdfdsf"] + + mocked_redis_get = mocker.patch.object( + redis_store, + "get", + return_value=bytes( + json.dumps(service_json, default=lambda o: o.hex if isinstance(o, uuid.UUID) else None), encoding="utf-8" + ), + ) - service_json["data"]["all_template_folders"] = ["b5035a31-b1da-42f8-b2b8-ce2acaa0b819"] - service_json["data"]["annual_billing"] = ["8676fa80-a97b-43e7-8318-ee905de2d652", "a0751f79-984b-4d9e-9edd-42457fd458e9"] - service_json["data"]["email_branding"] = "d51a41b2-c420-48a9-a8c5-e88444013020" - service_json["data"]["inbound_number"] = "aa129e4b-d37c-493a-84da-62a31a8199e3" - service_json["data"]["inbound_sms"] = ["fsdfdsfdsfdsfdsfsdf"] - service_json["data"]["service_callback_api"] = ["wfeewfwefewfewfewfewfew"] - service_json["data"]["service_data_retention"] = ["fdsfsdfsdfsdfsdfdsf"] + service = dao_fetch_service_by_id(sample_service.id, use_cache=True) + + assert mocked_redis_get.called + assert str(sample_service.id) == service.id + + def test_get_service_with_reply_to_from_cache_and_db(self, notify_db_session, mocker): + sample_service = create_service(service_name="testing", email_from="testing") + create_reply_to_email(sample_service, "test@mail.com") + service_json = {"data": service_schema.dump(sample_service)} + + service_json["data"]["all_template_folders"] = ["b5035a31-b1da-42f8-b2b8-ce2acaa0b819"] + service_json["data"]["annual_billing"] = ["8676fa80-a97b-43e7-8318-ee905de2d652", "a0751f79-984b-4d9e-9edd-42457fd458e9"] + service_json["data"]["email_branding"] = "d51a41b2-c420-48a9-a8c5-e88444013020" + service_json["data"]["inbound_number"] = "aa129e4b-d37c-493a-84da-62a31a8199e3" + service_json["data"]["inbound_sms"] = ["fsdfdsfdsfdsfdsfsdf"] + service_json["data"]["service_callback_api"] = ["wfeewfwefewfewfewfewfew"] + service_json["data"]["service_data_retention"] = ["fdsfsdfsdfsdfsdfdsf"] + + mocked_redis_get = mocker.patch.object( + redis_store, + "get", + return_value=bytes( + json.dumps(service_json, default=lambda o: o.hex if isinstance(o, uuid.UUID) else None), encoding="utf-8" + ), + ) - mocked_redis_get = mocker.patch.object( - redis_store, - "get", - return_value=bytes( - json.dumps(service_json, default=lambda o: o.hex if isinstance(o, uuid.UUID) else None), encoding="utf-8" - ), - ) + service = dao_fetch_service_by_id(sample_service.id, use_cache=True) - service = dao_fetch_service_by_id(sample_service.id, use_cache=True) + assert mocked_redis_get.called + assert str(sample_service.id) == service.id + assert str(service.reply_to_email_addresses[0].id) == str(sample_service.reply_to_email_addresses[0].id) - assert mocked_redis_get.called - assert str(sample_service.id) == service.id + service = dao_fetch_service_by_id(sample_service.id, use_cache=False) + assert str(service.reply_to_email_addresses[0].id) == str(sample_service.reply_to_email_addresses[0].id) def test_create_service_returns_service_with_default_permissions(notify_db_session): diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 4c2069a6e2..a78cb13ce8 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -2623,14 +2623,15 @@ def test_is_email_from_unique_returns_400_when_email_from_does_not_exist(admin_r assert response["message"][1]["email_from"] == ["Can't be empty"] -def test_get_email_reply_to_addresses_when_there_are_no_reply_to_email_addresses(client, sample_service): - response = client.get( - "/service/{}/email-reply-to".format(sample_service.id), - headers=[create_authorization_header()], - ) +class TestServiceEmailReplyTo: + def test_get_email_reply_to_addresses_when_there_are_no_reply_to_email_addresses(self, client, sample_service): + response = client.get( + "/service/{}/email-reply-to".format(sample_service.id), + headers=[create_authorization_header()], + ) - assert json.loads(response.get_data(as_text=True)) == [] - assert response.status_code == 200 + assert json.loads(response.get_data(as_text=True)) == [] + assert response.status_code == 200 def test_get_email_reply_to_addresses_with_one_email_address(client, notify_db, notify_db_session): @@ -3223,3 +3224,32 @@ def test_get_monthly_notification_data_by_service(mocker, admin_request): dao_mock.assert_called_once_with(start_date, end_date) assert response == [] + + +class TestSerializationofServiceReplyto: + def test_get_service(self, client, sample_service): + sample_service.reply_to_email = "something@service.com" + create_reply_to_email(service=sample_service, email_address="new@service.com") + auth_header = create_authorization_header() + resp = client.get( + "/service/{}".format(sample_service.id), + headers=[auth_header], + ) + assert resp.status_code == 200 + json_resp = resp.json + assert json_resp["data"]["name"] == sample_service.name + assert json_resp["data"]["id"] == str(sample_service.id) + assert json_resp["data"]["reply_to_email_addresses"][0]["email_address"] == "new@service.com" + + def test_get_service_no_reply_to(self, client, sample_service): + sample_service.reply_to_email = "something@service.com" + auth_header = create_authorization_header() + resp = client.get( + "/service/{}".format(sample_service.id), + headers=[auth_header], + ) + assert resp.status_code == 200 + json_resp = resp.json + assert json_resp["data"]["name"] == sample_service.name + assert json_resp["data"]["id"] == str(sample_service.id) + assert json_resp["data"]["reply_to_email_addresses"] == []