Bug fix: Set default letter sender address on template creation

We spotted then when creating a new letter template it was not
having the default letter sender address added to it. This commit
fixes that bug by adding it if one exists.

Note, use of new walrus operator
https://docs.python.org/3/whatsnew/3.8.html#assignment-expressions

app/models.py

@@ -563,7 +563,7 @@ class Service(db.Model, Versioned):
 
     def get_default_letter_contact(self):
         default_letter_contact = [x for x in self.letter_contacts if x.is_default]
-        return default_letter_contact[0].contact_block if default_letter_contact else None
+        return default_letter_contact[0] if default_letter_contact else None
 
     def has_permission(self, permission):
         return permission in [p.permission for p in self.permissions]

app/schemas.py

@@ -246,9 +246,6 @@ class ServiceSchema(BaseSchema, UUIDsAsStringsMixin):
     def service_permissions(self, service):
         return [p.permission for p in service.permissions]
 
-    def get_letter_contact(self, service):
-        return service.get_default_letter_contact()
-
     class Meta(BaseSchema.Meta):
         model = models.Service
         exclude = (

app/template/rest.py

@@ -96,8 +96,11 @@ def create_template(service_id):
         errors = {'template_type': [message]}
         raise InvalidRequest(errors, 403)
 
-    if not new_template.postage and new_template.template_type == LETTER_TYPE:
-        new_template.postage = SECOND_CLASS
+    if new_template.template_type == LETTER_TYPE:
+        if default_letter_contact := fetched_service.get_default_letter_contact():
+            new_template.reply_to = default_letter_contact.id
+        if not new_template.postage:
+            new_template.postage = SECOND_CLASS
 
     new_template.service = fetched_service
 

tests/app/template/test_rest.py

@@ -146,6 +146,70 @@ def test_create_a_new_template_for_a_service_adds_postage_for_letters_only(
     assert template.postage == expected_postage
 
 
+@pytest.mark.parametrize("has_a_non_default_letter_contact_only", [True, False])
+def test_create_letter_template_adds_no_sender_address_if_no_default_exists(
+    client, sample_service, has_a_non_default_letter_contact_only
+):
+    # If the service has no letter contacts, then the default sender address is 'not set' and it will be
+    # blank for the new template
+    # However they can have a single letter contact which is stored in the DB as not the default, which
+    # results in the default sender address being 'Blank' rather than 'not set'
+    # We test both of these situations where we expect the templates sender address to empty
+    if has_a_non_default_letter_contact_only:
+        create_letter_contact(sample_service, "Edinburgh, ED1 1AA", is_default=False)
+
+    data = {
+        'name': 'my template',
+        'template_type': 'letter',
+        'subject': "my subject",
+        'content': 'template <b>content</b>',
+        'service': str(sample_service.id),
+        'created_by': str(sample_service.users[0].id)
+    }
+
+    data = json.dumps(data)
+    auth_header = create_admin_authorization_header()
+
+    response = client.post(
+        '/service/{}/template'.format(sample_service.id),
+        headers=[('Content-Type', 'application/json'), auth_header],
+        data=data
+    )
+    assert response.status_code == 201
+    created_template = json.loads(response.get_data(as_text=True))['data']
+    assert created_template['service_letter_contact'] is None
+    assert created_template['reply_to'] is None
+    assert created_template['reply_to_text'] is None
+
+
+def test_create_letter_template_adds_default_sender_address_for_letters_when_exists(
+    client, sample_service
+):
+    letter_contact = create_letter_contact(sample_service, "Edinburgh, ED1 1AA", is_default=True)
+    data = {
+        'name': 'my template',
+        'template_type': 'letter',
+        'subject': "my subject",
+        'content': 'template <b>content</b>',
+        'service': str(sample_service.id),
+        'created_by': str(sample_service.users[0].id)
+    }
+
+    data = json.dumps(data)
+    auth_header = create_admin_authorization_header()
+
+    response = client.post(
+        '/service/{}/template'.format(sample_service.id),
+        headers=[('Content-Type', 'application/json'), auth_header],
+        data=data
+    )
+    assert response.status_code == 201
+    created_template = json.loads(response.get_data(as_text=True))['data']
+    assert created_template['service_letter_contact'] == str(letter_contact.id)
+    assert created_template['reply_to'] == str(letter_contact.id)
+    assert created_template['reply_to_text'] == "Edinburgh, ED1 1AA"
+
+
 def test_create_template_should_return_400_if_folder_is_for_a_different_service(
         client, sample_service
 ):

tests/app/test_model.py

@@ -274,9 +274,9 @@ def test_service_get_default_reply_to_email_address(sample_service):
 
 
 def test_service_get_default_contact_letter(sample_service):
-    create_letter_contact(service=sample_service, contact_block='London,\nNW1A 1AA')
+    letter_contact = create_letter_contact(service=sample_service, contact_block='London,\nNW1A 1AA')
 
-    assert sample_service.get_default_letter_contact() == 'London,\nNW1A 1AA'
+    assert sample_service.get_default_letter_contact() == letter_contact
 
 
 def test_service_get_default_sms_sender(notify_db_session):