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

@@ -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:

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):

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, "[email protected]")
+        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):

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 = "[email protected]"
+        create_reply_to_email(service=sample_service, email_address="[email protected]")
+        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"] == "[email protected]"
+
+    def test_get_service_no_reply_to(self, client, sample_service):
+        sample_service.reply_to_email = "[email protected]"
+        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"] == []