Merge pull request #730 from alphagov/letter-templates

Add ‘letter’ as a possible template type

app/schemas.py

@@ -212,8 +212,7 @@ class TemplateSchema(BaseTemplateSchema):
 
     @validates_schema
     def validate_type(self, data):
-        template_type = data.get('template_type')
+        if data.get('template_type') in [models.EMAIL_TYPE, models.LETTER_TYPE]:
-        if template_type and template_type == 'email':
             subject = data.get('subject')
             if not subject or subject.strip() == '':
                 raise ValidationError('Invalid template subject', 'subject')

tests/app/conftest.py

@@ -180,7 +180,7 @@ def sample_template(notify_db,
         'created_by': created_by,
         'archived': archived
     }
-    if template_type == 'email':
+    if template_type in ['email', 'letter']:
         data.update({
             'subject': subject_line
         })

tests/app/dao/test_templates_dao.py

@@ -10,31 +10,21 @@ from app.models import Template, TemplateHistory
 import pytest
 
 
-def test_create_template(sample_service, sample_user):
+@pytest.mark.parametrize('template_type, subject', [
+    ('sms', None),
+    ('email', 'subject'),
+    ('letter', 'subject'),
+])
+def test_create_template(sample_service, sample_user, template_type, subject):
     data = {
         'name': 'Sample Template',
-        'template_type': "sms",
+        'template_type': template_type,
-        'content': "Template content",
-        'service': sample_service,
-        'created_by': sample_user
-    }
-    template = Template(**data)
-    dao_create_template(template)
-
-    assert Template.query.count() == 1
-    assert len(dao_get_all_templates_for_service(sample_service.id)) == 1
-    assert dao_get_all_templates_for_service(sample_service.id)[0].name == 'Sample Template'
-
-
-def test_create_email_template(sample_service, sample_user):
-    data = {
-        'name': 'Sample Template',
-        'template_type': "email",
-        'subject': "subject",
         'content': "Template content",
         'service': sample_service,
         'created_by': sample_user
     }
+    if subject:
+        data.update({'subject': subject})
     template = Template(**data)
     dao_create_template(template)
 

tests/app/template/test_rest.py

@@ -1,3 +1,4 @@
+import pytest
 import json
 import random
 import string
@@ -8,16 +9,23 @@ from tests.app.conftest import sample_template as create_sample_template
 from app.dao.templates_dao import dao_get_template_by_id
 
 
-def test_should_create_a_new_sms_template_for_a_service(notify_api, sample_user, sample_service):
+@pytest.mark.parametrize('template_type, subject', [
-    with notify_api.test_request_context():
+    ('sms', None),
-        with notify_api.test_client() as client:
+    ('email', 'subject'),
+    ('letter', 'subject'),
+])
+def test_should_create_a_new_template_for_a_service(
+    client, sample_user, sample_service, template_type, subject
+):
     data = {
         'name': 'my template',
-                'template_type': 'sms',
+        'template_type': template_type,
         'content': 'template <b>content</b>',
         'service': str(sample_service.id),
         'created_by': str(sample_user.id)
     }
+    if subject:
+        data.update({'subject': subject})
     data = json.dumps(data)
     auth_header = create_authorization_header()
 
@@ -29,47 +37,18 @@ def test_should_create_a_new_sms_template_for_a_service(notify_api, sample_user,
     assert response.status_code == 201
     json_resp = json.loads(response.get_data(as_text=True))
     assert json_resp['data']['name'] == 'my template'
-            assert json_resp['data']['template_type'] == 'sms'
+    assert json_resp['data']['template_type'] == template_type
     assert json_resp['data']['content'] == 'template content'
     assert json_resp['data']['service'] == str(sample_service.id)
     assert json_resp['data']['id']
     assert json_resp['data']['version'] == 1
+    if subject:
+        assert json_resp['data']['subject'] == 'subject'
+    else:
         assert not json_resp['data']['subject']
 
 
-def test_should_create_a_new_email_template_for_a_service(notify_api, sample_user, sample_service):
+def test_should_be_error_if_service_does_not_exist_on_create(client, sample_user, fake_uuid):
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
-            data = {
-                'name': 'my template',
-                'template_type': 'email',
-                'subject': 'subject',
-                'content': 'template <b>content</b>',
-                'service': str(sample_service.id),
-                'created_by': str(sample_user.id)
-            }
-            data = json.dumps(data)
-            auth_header = create_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
-            json_resp = json.loads(response.get_data(as_text=True))
-            assert json_resp['data']['name'] == 'my template'
-            assert json_resp['data']['template_type'] == 'email'
-            assert json_resp['data']['content'] == 'template content'
-            assert json_resp['data']['service'] == str(sample_service.id)
-            assert json_resp['data']['subject'] == 'subject'
-            assert json_resp['data']['version'] == 1
-            assert json_resp['data']['id']
-
-
-def test_should_be_error_if_service_does_not_exist_on_create(notify_api, sample_user, fake_uuid):
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
     data = {
         'name': 'my template',
         'template_type': 'sms',
@@ -91,9 +70,7 @@ def test_should_be_error_if_service_does_not_exist_on_create(notify_api, sample_
     assert json_resp['message'] == 'No result found'
 
 
-def test_should_error_if_created_by_missing(notify_api, sample_user, sample_service):
+def test_should_error_if_created_by_missing(client, sample_user, sample_service):
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
     service_id = str(sample_service.id)
     data = {
         'name': 'my template',
@@ -114,9 +91,7 @@ def test_should_error_if_created_by_missing(notify_api, sample_user, sample_serv
     assert json_resp['result'] == 'error'
 
 
-def test_should_be_error_if_service_does_not_exist_on_update(notify_api, fake_uuid):
+def test_should_be_error_if_service_does_not_exist_on_update(client, fake_uuid):
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
     data = {
         'name': 'my template'
     }
@@ -134,12 +109,11 @@ def test_should_be_error_if_service_does_not_exist_on_update(notify_api, fake_uu
     assert json_resp['message'] == 'No result found'
 
 
-def test_must_have_a_subject_on_an_email_template(notify_api, sample_user, sample_service):
+@pytest.mark.parametrize('template_type', ['email', 'letter'])
-    with notify_api.test_request_context():
+def test_must_have_a_subject_on_an_email_or_letter_template(client, sample_user, sample_service, template_type):
-        with notify_api.test_client() as client:
     data = {
         'name': 'my template',
-                'template_type': 'email',
+        'template_type': template_type,
         'content': 'template content',
         'service': str(sample_service.id),
         'created_by': str(sample_user.id)
@@ -158,9 +132,7 @@ def test_must_have_a_subject_on_an_email_template(notify_api, sample_user, sampl
     assert json_resp['message'] == {'subject': ['Invalid template subject']}
 
 
-def test_update_should_update_a_template(notify_api, sample_user, sample_template):
+def test_update_should_update_a_template(client, sample_user, sample_template):
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
     data = {
         'content': 'my template has new content <script type="text/javascript">alert("foo")</script>',
         'created_by': str(sample_user.id)
@@ -182,9 +154,7 @@ def test_update_should_update_a_template(notify_api, sample_user, sample_templat
     assert update_json_resp['data']['version'] == 2
 
 
-def test_should_be_able_to_archive_template(notify_api, sample_template):
+def test_should_be_able_to_archive_template(client, sample_template):
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
     data = {
         'name': sample_template.name,
         'template_type': sample_template.template_type,
@@ -208,9 +178,7 @@ def test_should_be_able_to_archive_template(notify_api, sample_template):
     assert Template.query.first().archived
 
 
-def test_should_be_able_to_get_all_templates_for_a_service(notify_api, sample_user, sample_service):
+def test_should_be_able_to_get_all_templates_for_a_service(client, sample_user, sample_service):
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
     data = {
         'name': 'my template 1',
         'template_type': 'email',
@@ -260,9 +228,7 @@ def test_should_be_able_to_get_all_templates_for_a_service(notify_api, sample_us
     assert update_json_resp['data'][1]['created_at']
 
 
-def test_should_get_only_templates_for_that_service(notify_api, sample_user, service_factory):
+def test_should_get_only_templates_for_that_service(client, sample_user, service_factory):
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
 
     service_1 = service_factory.get('service 1', email_from='service.1')
     service_2 = service_factory.get('service 2', email_from='service.2')
@@ -337,19 +303,23 @@ def test_should_get_only_templates_for_that_service(notify_api, sample_user, ser
             None,
             'hello ((name)) we’ve received your ((thing))',
             'sms'
+        ),
+        (
+            'about your ((thing))',
+            'hello ((name)) we’ve received your ((thing))',
+            'letter'
         )
     ]
 )
 def test_should_get_a_single_template(
     notify_db,
-    notify_api,
+    client,
     sample_user,
     service_factory,
     subject,
     content,
     template_type
 ):
-    with notify_api.test_request_context(), notify_api.test_client() as client:
 
     template = create_sample_template(
         notify_db, notify_db.session, subject_line=subject, content=content, template_type=template_type
@@ -403,7 +373,7 @@ def test_should_get_a_single_template(
 )
 def test_should_preview_a_single_template(
     notify_db,
-    notify_api,
+    client,
     sample_user,
     service_factory,
     subject,
@@ -413,7 +383,6 @@ def test_should_preview_a_single_template(
     expected_content,
     expected_error
 ):
-    with notify_api.test_request_context(), notify_api.test_client() as client:
 
     template = create_sample_template(
         notify_db, notify_db.session, subject_line=subject, content=content, template_type='email'
@@ -435,9 +404,7 @@ def test_should_preview_a_single_template(
         assert content['subject'] == expected_subject
 
 
-def test_should_return_empty_array_if_no_templates_for_service(notify_api, sample_service):
+def test_should_return_empty_array_if_no_templates_for_service(client, sample_service):
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
 
     auth_header = create_authorization_header()
 
@@ -451,9 +418,7 @@ def test_should_return_empty_array_if_no_templates_for_service(notify_api, sampl
     assert len(json_resp['data']) == 0
 
 
-def test_should_return_404_if_no_templates_for_service_with_id(notify_api, sample_service, fake_uuid):
+def test_should_return_404_if_no_templates_for_service_with_id(client, sample_service, fake_uuid):
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
 
     auth_header = create_authorization_header()
 
@@ -468,9 +433,8 @@ def test_should_return_404_if_no_templates_for_service_with_id(notify_api, sampl
     assert json_resp['message'] == 'No result found'
 
 
-def test_create_400_for_over_limit_content(notify_api, sample_user, sample_service, fake_uuid):
+def test_create_400_for_over_limit_content(client, notify_api, sample_user, sample_service, fake_uuid):
-    with notify_api.test_request_context():
+
-        with notify_api.test_client() as client:
     limit = notify_api.config.get('SMS_CHAR_COUNT_LIMIT')
     content = ''.join(random.choice(string.ascii_uppercase + string.digits) for _ in range(limit + 1))
     data = {
@@ -495,9 +459,8 @@ def test_create_400_for_over_limit_content(notify_api, sample_user, sample_servi
     ).format(limit) in json_resp['message']['content']
 
 
-def test_update_400_for_over_limit_content(notify_api, sample_user, sample_template):
+def test_update_400_for_over_limit_content(client, notify_api, sample_user, sample_template):
-    with notify_api.test_request_context():
+
-        with notify_api.test_client() as client:
     limit = notify_api.config.get('SMS_CHAR_COUNT_LIMIT')
     json_data = json.dumps({
         'content': ''.join(random.choice(string.ascii_uppercase + string.digits) for _ in range(limit + 1)),
@@ -516,15 +479,14 @@ def test_update_400_for_over_limit_content(notify_api, sample_user, sample_templ
     ).format(limit) in json_resp['message']['content']
 
 
-def test_should_return_all_template_versions_for_service_and_template_id(notify_api, sample_template):
+def test_should_return_all_template_versions_for_service_and_template_id(client, sample_template):
     original_content = sample_template.content
     from app.dao.templates_dao import dao_update_template
     sample_template.content = original_content + '1'
     dao_update_template(sample_template)
     sample_template.content = original_content + '2'
     dao_update_template(sample_template)
-    with notify_api.test_request_context():
+
-        with notify_api.test_client() as client:
     auth_header = create_authorization_header()
     resp = client.get('/service/{}/template/{}/versions'.format(sample_template.service_id, sample_template.id),
                       headers=[('Content-Type', 'application/json'), auth_header])
@@ -540,9 +502,8 @@ def test_should_return_all_template_versions_for_service_and_template_id(notify_
             assert x['content'] == original_content + '2'
 
 
-def test_update_does_not_create_new_version_when_there_is_no_change(notify_api, sample_template):
+def test_update_does_not_create_new_version_when_there_is_no_change(client, sample_template):
-    with notify_api.test_request_context():
+
-        with notify_api.test_client() as client:
     auth_header = create_authorization_header()
     data = {
         'template_type': sample_template.template_type,
@@ -552,5 +513,6 @@ def test_update_does_not_create_new_version_when_there_is_no_change(notify_api,
                        data=json.dumps(data),
                        headers=[('Content-Type', 'application/json'), auth_header])
     assert resp.status_code == 200
+
     template = dao_get_template_by_id(sample_template.id)
     assert template.version == 1