Use the template version at the time the notification is created or at the time the job is created.

Update notifications/sms|email endpoint to send the template version to the queue.
Update the process_job celery talk to send the template version to the queue.
When the send_sms|send_email task runs it will get the template by id and version.

Created a data migration script to add the template_vesion column for jobs and notifications.
The existing jobs and notifications are given the template_version of the current template.
There is a chance this is the wrong template version, but deemed okay since the application is not live.

Create unit test for the dao_get_template_versions method.
Rename /template/<id>/version to /template/<id>/versions which returns all versions for that template id and service id.

app/celery/tasks.py

@@ -285,6 +285,7 @@ def send_email(service_id, notification_id, from_address, encrypted_notification
         notification_db_object = Notification(
             id=notification_id,
             template_id=notification['template'],
+            template_version=notification['template_version'],
             to=notification['to'],
             service_id=service_id,
             job_id=notification.get('job', None),
@@ -301,7 +302,7 @@ def send_email(service_id, notification_id, from_address, encrypted_notification
 
         try:
             template = Template(
-                dao_get_template_by_id(notification['template']).__dict__,
+                dao_get_template_by_id(notification['template'], notification['template_version']).__dict__,
                 values=notification.get('personalisation', {})
             )
             reference = provider.send_email(
@@ -315,8 +316,8 @@ def send_email(service_id, notification_id, from_address, encrypted_notification
         except EmailClientException as e:
             current_app.logger.exception(e)
             notification_db_object.status = 'failed'
+            dao_update_notification(notification_db_object)
 
-        dao_update_notification(notification_db_object)
         current_app.logger.info(
             "Email {} created at {} sent at {}".format(notification_id, created_at, sent_at)
         )

app/dao/templates_dao.py

@@ -49,4 +49,5 @@ def dao_get_all_templates_for_service(service_id):
 
 def dao_get_template_versions(service_id, template_id):
     history_model = Template.get_history_model()
-    return history_model.query.filter_by(service_id=service_id, id=template_id).order_by(desc(history_model.version))
+    return history_model.query.filter_by(service_id=service_id, id=template_id).order_by(
+        desc(history_model.version)).all()

app/notifications/rest.py

@@ -357,7 +357,7 @@ def send_notification(notification_type):
         ), 400
 
     notification_id = create_uuid()
-
+    notification.update({"template_version": template.version})
     if notification_type == 'sms':
         send_sms.apply_async((
             service_id,

app/template/rest.py

@@ -107,18 +107,18 @@ def get_template_version(service_id, template_id, version):
         )
     )
     if errors:
-        return json_resp(result='error', message=errors), 400
+        return jsonify(result='error', message=errors), 400
     return jsonify(data=data)
 
 
-@template.route('/<uuid:template_id>/version')
+@template.route('/<uuid:template_id>/versions')
 def get_template_versions(service_id, template_id):
     data, errors = template_history_schema.dump(
-        dao_get_template_versions(service_id, template_id),
+        dao_get_template_versions(service_id=service_id, template_id=template_id),
         many=True
     )
     if errors:
-        return json_resp(result='error', message=errors), 400
+        return jsonify(result='error', message=errors), 400
     return jsonify(data=data)
 
 

migrations/versions/0014_add_template_version.py

@@ -1,14 +1,14 @@
 """empty message
 
-Revision ID: 0013_add_template_version
+Revision ID: 0014_add_template_version
-Revises: 0010_events_table
+Revises: 0012_complete_provider_details
 Create Date: 2016-05-11 16:00:51.478012
 
 """
 
 # revision identifiers, used by Alembic.
-revision = '0013_add_template_version'
+revision = '0014_add_template_version'
-down_revision = '0010_events_table'
+down_revision = '0012_complete_provider_details'
 
 from alembic import op
 import sqlalchemy as sa

tests/app/celery/test_tasks.py

@@ -114,6 +114,8 @@ def test_should_process_sms_job(sample_job, mocker, mock_celery_remove_job):
         str(sample_job.id)
     )
     assert encryption.encrypt.call_args[0][0]['to'] == '+441234123123'
+    assert encryption.encrypt.call_args[0][0]['template'] == str(sample_job.template.id)
+    assert encryption.encrypt.call_args[0][0]['template_version'] == sample_job.template.version
     assert encryption.encrypt.call_args[0][0]['personalisation'] == {}
     tasks.send_sms.apply_async.assert_called_once_with(
         (str(sample_job.service_id),
@@ -278,6 +280,8 @@ def test_should_process_email_job(sample_email_job, mocker, mock_celery_remove_j
         str(sample_email_job.id)
     )
     assert encryption.encrypt.call_args[0][0]['to'] == 'test@test.com'
+    assert encryption.encrypt.call_args[0][0]['template'] == str(sample_email_job.template.id)
+    assert encryption.encrypt.call_args[0][0]['template_version'] == sample_email_job.template.version
     assert encryption.encrypt.call_args[0][0]['personalisation'] == {}
     tasks.send_email.apply_async.assert_called_once_with(
         (
@@ -314,6 +318,9 @@ def test_should_process_all_sms_job(sample_job,
         str(sample_job_with_placeholdered_template.id)
     )
     assert encryption.encrypt.call_args[0][0]['to'] == '+441234123120'
+    assert encryption.encrypt.call_args[0][0]['template'] == str(sample_job_with_placeholdered_template.template.id)
+    assert encryption.encrypt.call_args[0][0][
+        'template_version'] == sample_job_with_placeholdered_template.template.version
     assert encryption.encrypt.call_args[0][0]['personalisation'] == {'name': 'chris'}
     tasks.send_sms.apply_async.call_count == 10
     job = jobs_dao.dao_get_job_by_id(sample_job_with_placeholdered_template.id)
@@ -321,11 +328,8 @@ def test_should_process_all_sms_job(sample_job,
 
 
 def test_should_send_template_to_correct_sms_provider_and_persist(sample_template_with_placeholders, mocker):
-    notification = {
+    notification = _notification_json(sample_template_with_placeholders,
-        "template": sample_template_with_placeholders.id,
+                                      to="+447234123123", personalisation={"name": "Jo"})
-        "to": "+447234123123",
-        "personalisation": {"name": "Jo"}
-    }
     mocker.patch('app.encryption.decrypt', return_value=notification)
     mocker.patch('app.mmg_client.send_sms')
     mocker.patch('app.mmg_client.get_name', return_value="mmg")
@@ -350,6 +354,7 @@ def test_should_send_template_to_correct_sms_provider_and_persist(sample_templat
     assert persisted_notification.id == notification_id
     assert persisted_notification.to == '+447234123123'
     assert persisted_notification.template_id == sample_template_with_placeholders.id
+    assert persisted_notification.template_version == sample_template_with_placeholders.version
     assert persisted_notification.status == 'sending'
     assert persisted_notification.created_at == now
     assert persisted_notification.sent_at > now
@@ -358,10 +363,7 @@ def test_should_send_template_to_correct_sms_provider_and_persist(sample_templat
 
 
 def test_should_send_sms_without_personalisation(sample_template, mocker):
-    notification = {
+    notification = _notification_json(sample_template, "+447234123123")
-        "template": sample_template.id,
-        "to": "+447234123123"
-    }
     mocker.patch('app.encryption.decrypt', return_value=notification)
     mocker.patch('app.mmg_client.send_sms')
     mocker.patch('app.mmg_client.get_name', return_value="mmg")
@@ -387,10 +389,8 @@ def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notif
     service = sample_service(notify_db, notify_db_session, user=user, restricted=True)
     template = sample_template(notify_db, notify_db_session, service=service)
 
-    notification = {
+    notification = _notification_json(template, "+447700900890")  # The user’s own number, but in a different format
-        "template": template.id,
+
-        "to": "+447700900890"  # The user’s own number, but in a different format
-    }
     mocker.patch('app.encryption.decrypt', return_value=notification)
     mocker.patch('app.mmg_client.send_sms')
     mocker.patch('app.mmg_client.get_name', return_value="mmg")
@@ -416,10 +416,7 @@ def test_should_not_send_sms_if_restricted_service_and_invalid_number(notify_db,
     service = sample_service(notify_db, notify_db_session, user=user, restricted=True)
     template = sample_template(notify_db, notify_db_session, service=service)
 
-    notification = {
+    notification = _notification_json(template, "07700 900849")
-        "template": template.id,
-        "to": "07700 900849"
-    }
     mocker.patch('app.encryption.decrypt', return_value=notification)
     mocker.patch('app.mmg_client.send_sms')
     mocker.patch('app.mmg_client.get_name', return_value="mmg")
@@ -436,6 +433,48 @@ def test_should_not_send_sms_if_restricted_service_and_invalid_number(notify_db,
     mmg_client.send_sms.assert_not_called()
 
 
+def test_send_sms_should_use_template_version_from_job_not_latest(sample_template, mocker):
+    notification = _notification_json(sample_template, '+447234123123')
+    mocker.patch('app.encryption.decrypt', return_value=notification)
+    mocker.patch('app.mmg_client.send_sms')
+    mocker.patch('app.mmg_client.get_name', return_value="mmg")
+    version_on_notification = sample_template.version
+
+    # Change the template
+    from app.dao.templates_dao import dao_update_template, dao_get_template_by_id
+    sample_template.content = sample_template.content + " another version of the template"
+    dao_update_template(sample_template)
+    t = dao_get_template_by_id(sample_template.id)
+    assert t.version > version_on_notification
+
+    notification_id = uuid.uuid4()
+    now = datetime.utcnow()
+    send_sms(
+        sample_template.service_id,
+        notification_id,
+        "encrypted-in-reality",
+        now.strftime(DATETIME_FORMAT)
+    )
+
+    mmg_client.send_sms.assert_called_once_with(
+        to=format_phone_number(validate_phone_number("+447234123123")),
+        content="Sample service: This is a template",
+        reference=str(notification_id)
+    )
+
+    persisted_notification = notifications_dao.get_notification(sample_template.service_id, notification_id)
+    assert persisted_notification.id == notification_id
+    assert persisted_notification.to == '+447234123123'
+    assert persisted_notification.template_id == sample_template.id
+    assert persisted_notification.template_version == version_on_notification
+    assert persisted_notification.template_version != sample_template.version
+    assert persisted_notification.created_at == now
+    assert persisted_notification.sent_at > now
+    assert persisted_notification.status == 'sending'
+    assert persisted_notification.sent_by == 'mmg'
+    assert persisted_notification.content_char_count == len("Sample service: This is a template")
+
+
 def test_should_send_email_if_restricted_service_and_valid_email(notify_db, notify_db_session, mocker):
     user = sample_user(notify_db, notify_db_session, email="test@restricted.com")
     service = sample_service(notify_db, notify_db_session, user=user, restricted=True)
@@ -445,10 +484,7 @@ def test_should_send_email_if_restricted_service_and_valid_email(notify_db, noti
         service=service,
         template_type='email')
 
-    notification = {
+    notification = _notification_json(template, "test@restricted.com")
-        "template": template.id,
-        "to": "test@restricted.com"
-    }
     mocker.patch('app.encryption.decrypt', return_value=notification)
     mocker.patch('app.aws_ses_client.send_email')
 
@@ -478,10 +514,7 @@ def test_should_not_send_email_if_restricted_service_and_invalid_email_address(n
         notify_db, notify_db_session, service=service, template_type='email', subject_line='Hello'
     )
 
-    notification = {
+    notification = _notification_json(template, to="test@example.com")
-        "template": template.id,
-        "to": "test@example.com"
-    }
     mocker.patch('app.encryption.decrypt', return_value=notification)
     mocker.patch('app.aws_ses_client.send_email')
 
@@ -499,11 +532,7 @@ def test_should_not_send_email_if_restricted_service_and_invalid_email_address(n
 
 
 def test_should_send_template_to_correct_sms_provider_and_persist_with_job_id(sample_job, mocker):
-    notification = {
+    notification = _notification_json(sample_job.template, to="+447234123123", job_id=sample_job.id)
-        "template": sample_job.template.id,
-        "job": sample_job.id,
-        "to": "+447234123123"
-    }
     mocker.patch('app.encryption.decrypt', return_value=notification)
     mocker.patch('app.mmg_client.send_sms')
     mocker.patch('app.mmg_client.get_name', return_value="mmg")
@@ -533,11 +562,7 @@ def test_should_send_template_to_correct_sms_provider_and_persist_with_job_id(sa
 
 
 def test_should_use_email_template_and_persist(sample_email_template_with_placeholders, mocker):
-    notification = {
+    notification = _notification_json(sample_email_template_with_placeholders, "my_email@my_email.com", {"name": "Jo"})
-        "template": sample_email_template_with_placeholders.id,
-        "to": "my_email@my_email.com",
-        "personalisation": {"name": "Jo"}
-    }
     mocker.patch('app.encryption.decrypt', return_value=notification)
     mocker.patch('app.aws_ses_client.send_email')
     mocker.patch('app.aws_ses_client.get_name', return_value='ses')
@@ -564,6 +589,48 @@ def test_should_use_email_template_and_persist(sample_email_template_with_placeh
     assert persisted_notification.id == notification_id
     assert persisted_notification.to == 'my_email@my_email.com'
     assert persisted_notification.template_id == sample_email_template_with_placeholders.id
+    assert persisted_notification.template_version == sample_email_template_with_placeholders.version
+    assert persisted_notification.created_at == now
+    assert persisted_notification.sent_at > now
+    assert persisted_notification.status == 'sending'
+    assert persisted_notification.sent_by == 'ses'
+
+
+def test_send_email_should_use_template_version_from_job_not_latest(sample_email_template, mocker):
+    notification = _notification_json(sample_email_template, 'my_email@my_email.com')
+    mocker.patch('app.encryption.decrypt', return_value=notification)
+    mocker.patch('app.aws_ses_client.send_email')
+    mocker.patch('app.aws_ses_client.get_name', return_value='ses')
+    version_on_notification = sample_email_template.version
+    # Change the template
+    from app.dao.templates_dao import dao_update_template, dao_get_template_by_id
+    sample_email_template.content = sample_email_template.content + " another version of the template"
+
+    dao_update_template(sample_email_template)
+    t = dao_get_template_by_id(sample_email_template.id)
+    assert t.version > version_on_notification
+    notification_id = uuid.uuid4()
+    now = datetime.utcnow()
+    send_email(
+        sample_email_template.service_id,
+        notification_id,
+        'email_from',
+        "encrypted-in-reality",
+        now.strftime(DATETIME_FORMAT)
+    )
+    aws_ses_client.send_email.assert_called_once_with(
+        "email_from",
+        "my_email@my_email.com",
+        sample_email_template.subject,
+        body="This is a template",
+        html_body=AnyStringWith("This is a template")
+    )
+
+    persisted_notification = notifications_dao.get_notification(sample_email_template.service_id, notification_id)
+    assert persisted_notification.id == notification_id
+    assert persisted_notification.to == 'my_email@my_email.com'
+    assert persisted_notification.template_id == sample_email_template.id
+    assert persisted_notification.template_version == version_on_notification
     assert persisted_notification.created_at == now
     assert persisted_notification.sent_at > now
     assert persisted_notification.status == 'sending'
@@ -571,11 +638,8 @@ def test_should_use_email_template_and_persist(sample_email_template_with_placeh
 
 
 def test_should_use_email_template_subject_placeholders(sample_email_template_with_placeholders, mocker):
-    notification = {
+    notification = _notification_json(sample_email_template_with_placeholders,
-        "template": sample_email_template_with_placeholders.id,
+                                      "my_email@my_email.com", {"name": "Jo"})
-        "to": "my_email@my_email.com",
-        "personalisation": {"name": "Jo"}
-    }
     mocker.patch('app.encryption.decrypt', return_value=notification)
     mocker.patch('app.aws_ses_client.send_email')
     mocker.patch('app.aws_ses_client.get_name', return_value='ses')
@@ -609,11 +673,7 @@ def test_should_use_email_template_subject_placeholders(sample_email_template_wi
 
 
 def test_should_use_email_template_and_persist_ses_reference(sample_email_template_with_placeholders, mocker):
-    notification = {
+    notification = _notification_json(sample_email_template_with_placeholders, "my_email@my_email.com", {"name": "Jo"})
-        "template": sample_email_template_with_placeholders.id,
-        "to": "my_email@my_email.com",
-        "personalisation": {"name": "Jo"}
-    }
     mocker.patch('app.encryption.decrypt', return_value=notification)
     mocker.patch('app.aws_ses_client.send_email', return_value='reference')
 
@@ -632,13 +692,9 @@ def test_should_use_email_template_and_persist_ses_reference(sample_email_templa
     assert persisted_notification.reference == 'reference'
 
 
-def test_should_use_email_template_and_persist_without_personalisation(
+def test_should_use_email_template_and_persist_without_personalisation(sample_email_template, mocker):
-        sample_email_template, mocker
+    mocker.patch('app.encryption.decrypt',
-):
+                 return_value=_notification_json(sample_email_template, "my_email@my_email.com"))
-    mocker.patch('app.encryption.decrypt', return_value={
-        "template": sample_email_template.id,
-        "to": "my_email@my_email.com",
-    })
     mocker.patch('app.aws_ses_client.send_email', return_value="ref")
     mocker.patch('app.aws_ses_client.get_name', return_value='ses')
 
@@ -661,10 +717,7 @@ def test_should_use_email_template_and_persist_without_personalisation(
 
 
 def test_should_persist_notification_as_failed_if_sms_client_fails(sample_template, mocker):
-    notification = {
+    notification = _notification_json(sample_template, "+447234123123")
-        "template": sample_template.id,
-        "to": "+447234123123"
-    }
     mocker.patch('app.encryption.decrypt', return_value=notification)
     mocker.patch('app.mmg_client.send_sms', side_effect=MMGClientException(mmg_error))
     mocker.patch('app.mmg_client.get_name', return_value="mmg")
@@ -687,6 +740,7 @@ def test_should_persist_notification_as_failed_if_sms_client_fails(sample_templa
     assert persisted_notification.id == notification_id
     assert persisted_notification.to == '+447234123123'
     assert persisted_notification.template_id == sample_template.id
+    assert persisted_notification.template_version == sample_template.version
     assert persisted_notification.status == 'failed'
     assert persisted_notification.created_at == now
     assert persisted_notification.sent_at > now
@@ -694,10 +748,7 @@ def test_should_persist_notification_as_failed_if_sms_client_fails(sample_templa
 
 
 def test_should_persist_notification_as_failed_if_email_client_fails(sample_email_template, mocker):
-    notification = {
+    notification = _notification_json(sample_email_template, "my_email@my_email.com")
-        "template": sample_email_template.id,
-        "to": "my_email@my_email.com"
-    }
     mocker.patch('app.encryption.decrypt', return_value=notification)
     mocker.patch('app.aws_ses_client.send_email', side_effect=AwsSesClientException())
     mocker.patch('app.aws_ses_client.get_name', return_value="ses")
@@ -724,6 +775,7 @@ def test_should_persist_notification_as_failed_if_email_client_fails(sample_emai
     assert persisted_notification.id == notification_id
     assert persisted_notification.to == 'my_email@my_email.com'
     assert persisted_notification.template_id == sample_email_template.id
+    assert persisted_notification.template_version == sample_email_template.version
     assert persisted_notification.status == 'failed'
     assert persisted_notification.created_at == now
     assert persisted_notification.sent_by == 'ses'
@@ -731,10 +783,7 @@ def test_should_persist_notification_as_failed_if_email_client_fails(sample_emai
 
 
 def test_should_not_send_sms_if_db_peristance_failed(sample_template, mocker):
-    notification = {
+    notification = _notification_json(sample_template, "+447234123123")
-        "template": sample_template.id,
-        "to": "+447234123123"
-    }
     mocker.patch('app.encryption.decrypt', return_value=notification)
     mocker.patch('app.mmg_client.send_sms')
     mocker.patch('app.db.session.add', side_effect=SQLAlchemyError())
@@ -755,10 +804,7 @@ def test_should_not_send_sms_if_db_peristance_failed(sample_template, mocker):
 
 
 def test_should_not_send_email_if_db_peristance_failed(sample_email_template, mocker):
-    notification = {
+    notification = _notification_json(sample_email_template, "my_email@my_email.com")
-        "template": sample_email_template.id,
-        "to": "my_email@my_email.com"
-    }
     mocker.patch('app.encryption.decrypt', return_value=notification)
     mocker.patch('app.aws_ses_client.send_email')
     mocker.patch('app.db.session.add', side_effect=SQLAlchemyError())
@@ -853,3 +899,16 @@ def test_email_reset_password_should_send_email(notify_db, notify_db_session, no
                                   reset_password_message['to'],
                                   "Reset password for GOV.UK Notify",
                                   message)
+
+
+def _notification_json(template, to, personalisation=None, job_id=None):
+    notification = {
+        "template": template.id,
+        "template_version": template.version,
+        "to": to,
+    }
+    if personalisation:
+        notification.update({"personalisation": personalisation})
+    if job_id:
+        notification.update({"job": job_id})
+    return notification

tests/app/conftest.py

@@ -237,13 +237,12 @@ def sample_job(notify_db,
     if template is None:
         template = sample_template(notify_db, notify_db_session,
                                    service=service)
-    job_id = uuid.uuid4()
     data = {
         'id': uuid.uuid4(),
         'service_id': service.id,
         'service': service,
         'template_id': template.id,
-        'template_version': 1,
+        'template_version': template.version,
         'original_file_name': 'some.csv',
         'notification_count': notification_count,
         'created_at': created_at,
@@ -286,6 +285,7 @@ def sample_email_job(notify_db,
         'service_id': service.id,
         'service': service,
         'template_id': template.id,
+        'template_version': template.version,
         'original_file_name': 'some.csv',
         'notification_count': 1,
         'created_by': service.created_by

tests/app/dao/test_jobs_dao.py

@@ -19,6 +19,7 @@ def test_create_job(sample_template):
         'id': job_id,
         'service_id': sample_template.service.id,
         'template_id': sample_template.id,
+        'template_version': sample_template.version,
         'original_file_name': 'some.csv',
         'notification_count': 1,
         'created_by': sample_template.created_by

tests/app/dao/test_notification_dao.py

@@ -271,7 +271,7 @@ def test_save_notification_creates_sms_and_template_stats(sample_template, sampl
     assert NotificationStatistics.query.count() == 0
     assert TemplateStatistics.query.count() == 0
 
-    data = _notification_json(sample_template, sample_job.id)
+    data = _notification_json(sample_template, job_id=sample_job.id)
 
     notification = Notification(**data)
     dao_create_notification(notification, sample_template.template_type, mmg_provider.identifier)
@@ -283,6 +283,7 @@ def test_save_notification_creates_sms_and_template_stats(sample_template, sampl
     assert data['job_id'] == notification_from_db.job_id
     assert data['service'] == notification_from_db.service
     assert data['template'] == notification_from_db.template
+    assert data['template_version'] == notification_from_db.template_version
     assert data['created_at'] == notification_from_db.created_at
     assert 'sending' == notification_from_db.status
     assert Job.query.get(sample_job.id).notifications_sent == 1
@@ -308,7 +309,7 @@ def test_save_notification_and_create_email_and_template_stats(sample_email_temp
     assert NotificationStatistics.query.count() == 0
     assert TemplateStatistics.query.count() == 0
 
-    data = _notification_json(sample_email_template, sample_job.id)
+    data = _notification_json(sample_email_template, job_id=sample_job.id)
 
     notification = Notification(**data)
     dao_create_notification(notification, sample_email_template.template_type, ses_provider.identifier)
@@ -320,6 +321,7 @@ def test_save_notification_and_create_email_and_template_stats(sample_email_temp
     assert data['job_id'] == notification_from_db.job_id
     assert data['service'] == notification_from_db.service
     assert data['template'] == notification_from_db.template
+    assert data['template_version'] == notification_from_db.template_version
     assert data['created_at'] == notification_from_db.created_at
     assert 'sending' == notification_from_db.status
     assert Job.query.get(sample_job.id).notifications_sent == 1
@@ -359,7 +361,7 @@ def test_save_notification_handles_midnight_properly(sample_template, sample_job
 @freeze_time("2016-01-01 23:59:59.999999")
 def test_save_notification_handles_just_before_midnight_properly(sample_template, sample_job, mmg_provider):
     assert Notification.query.count() == 0
-    data = _notification_json(sample_template, sample_job.id)
+    data = _notification_json(sample_template, job_id=sample_job.id)
 
     notification = Notification(**data)
     dao_create_notification(notification, sample_template.template_type, mmg_provider.identifier)
@@ -375,7 +377,7 @@ def test_save_notification_handles_just_before_midnight_properly(sample_template
 
 def test_save_notification_and_increment_email_stats(sample_email_template, sample_job, ses_provider):
     assert Notification.query.count() == 0
-    data = _notification_json(sample_email_template, sample_job.id)
+    data = _notification_json(sample_email_template, job_id=sample_job.id)
 
     notification_1 = Notification(**data)
     notification_2 = Notification(**data)
@@ -435,7 +437,7 @@ def test_not_save_notification_and_not_create_stats_on_commit_error(sample_templ
     random_id = str(uuid.uuid4())
 
     assert Notification.query.count() == 0
-    data = _notification_json(sample_template, random_id)
+    data = _notification_json(sample_template, job_id=random_id)
 
     notification = Notification(**data)
     with pytest.raises(SQLAlchemyError):
@@ -449,7 +451,7 @@ def test_not_save_notification_and_not_create_stats_on_commit_error(sample_templ
 
 def test_save_notification_and_increment_job(sample_template, sample_job, mmg_provider):
     assert Notification.query.count() == 0
-    data = _notification_json(sample_template, sample_job.id)
+    data = _notification_json(sample_template, job_id=sample_job.id)
 
     notification = Notification(**data)
     dao_create_notification(notification, sample_template.template_type, mmg_provider.identifier)
@@ -461,6 +463,7 @@ def test_save_notification_and_increment_job(sample_template, sample_job, mmg_pr
     assert data['job_id'] == notification_from_db.job_id
     assert data['service'] == notification_from_db.service
     assert data['template'] == notification_from_db.template
+    assert data['template_version'] == notification_from_db.template_version
     assert data['created_at'] == notification_from_db.created_at
     assert 'sending' == notification_from_db.status
     assert Job.query.get(sample_job.id).notifications_sent == 1
@@ -474,18 +477,7 @@ def test_save_notification_and_increment_job(sample_template, sample_job, mmg_pr
 def test_should_not_increment_job_if_notification_fails_to_persist(sample_template, sample_job, mmg_provider):
     random_id = str(uuid.uuid4())
     assert Notification.query.count() == 0
-    data = {
+    data = _notification_json(sample_template, job_id=sample_job.id, id=random_id)
-        'id': random_id,
-        'to': '+44709123456',
-        'job_id': sample_job.id,
-        'service_id': sample_template.service.id,
-        'service': sample_template.service,
-        'template': sample_template,
-        'template_id': sample_template.id,
-        'template_version': sample_template.version,
-        'created_at': datetime.utcnow(),
-        'content_char_count': 160
-    }
 
     notification_1 = Notification(**data)
     dao_create_notification(notification_1, sample_template.template_type, mmg_provider.identifier)
@@ -508,7 +500,7 @@ def test_save_notification_and_increment_correct_job(notify_db, notify_db_sessio
     job_2 = sample_job(notify_db, notify_db_session, sample_template.service)
 
     assert Notification.query.count() == 0
-    data = _notification_json(sample_template, job_1.id)
+    data = _notification_json(sample_template, job_id=job_1.id)
 
     notification = Notification(**data)
     dao_create_notification(notification, sample_template.template_type, mmg_provider.identifier)
@@ -520,6 +512,7 @@ def test_save_notification_and_increment_correct_job(notify_db, notify_db_sessio
     assert data['job_id'] == notification_from_db.job_id
     assert data['service'] == notification_from_db.service
     assert data['template'] == notification_from_db.template
+    assert data['template_version'] == notification_from_db.template_version
     assert data['created_at'] == notification_from_db.created_at
     assert 'sending' == notification_from_db.status
     assert Job.query.get(job_1.id).notifications_sent == 1
@@ -539,6 +532,7 @@ def test_save_notification_with_no_job(sample_template, mmg_provider):
     assert data['to'] == notification_from_db.to
     assert data['service'] == notification_from_db.service
     assert data['template'] == notification_from_db.template
+    assert data['template_version'] == notification_from_db.template_version
     assert data['created_at'] == notification_from_db.created_at
     assert 'sending' == notification_from_db.status
 
@@ -552,7 +546,6 @@ def test_get_notification(sample_notification):
 
 def test_save_notification_no_job_id(sample_template, mmg_provider):
     assert Notification.query.count() == 0
-    to = '+44709123456'
     data = _notification_json(sample_template)
 
     notification = Notification(**data)
@@ -564,6 +557,7 @@ def test_save_notification_no_job_id(sample_template, mmg_provider):
     assert data['to'] == notification_from_db.to
     assert data['service'] == notification_from_db.service
     assert data['template'] == notification_from_db.template
+    assert data['template_version'] == notification_from_db.template_version
     assert 'sending' == notification_from_db.status
 
 
@@ -643,7 +637,7 @@ def test_should_not_delete_failed_notifications_before_seven_days(notify_db, not
 def test_save_new_notification_creates_template_stats(sample_template, sample_job, mmg_provider):
     assert Notification.query.count() == 0
     assert TemplateStatistics.query.count() == 0
-    data = _notification_json(sample_template, sample_job.id)
+    data = _notification_json(sample_template, job_id=sample_job.id)
 
     notification = Notification(**data)
     dao_create_notification(notification, sample_template.template_type, mmg_provider.identifier)
@@ -661,7 +655,7 @@ def test_save_new_notification_creates_template_stats(sample_template, sample_jo
 def test_save_new_notification_creates_template_stats_per_day(sample_template, sample_job, mmg_provider):
     assert Notification.query.count() == 0
     assert TemplateStatistics.query.count() == 0
-    data = _notification_json(sample_template, sample_job.id)
+    data = _notification_json(sample_template, job_id=sample_job.id)
 
     notification = Notification(**data)
     dao_create_notification(notification, sample_template.template_type, mmg_provider.identifier)
@@ -697,7 +691,7 @@ def test_save_new_notification_creates_template_stats_per_day(sample_template, s
 def test_save_another_notification_increments_template_stats(sample_template, sample_job, mmg_provider):
     assert Notification.query.count() == 0
     assert TemplateStatistics.query.count() == 0
-    data = _notification_json(sample_template, sample_job.id)
+    data = _notification_json(sample_template, job_id=sample_job.id)
 
     notification_1 = Notification(**data)
     notification_2 = Notification(**data)
@@ -725,7 +719,7 @@ def test_successful_notification_inserts_followed_by_failure_does_not_increment_
     assert NotificationStatistics.query.count() == 0
     assert TemplateStatistics.query.count() == 0
 
-    data = _notification_json(sample_template, sample_job.id)
+    data = _notification_json(sample_template, job_id=sample_job.id)
 
     notification_1 = Notification(**data)
     notification_2 = Notification(**data)
@@ -768,7 +762,7 @@ def test_get_template_stats_for_service_returns_stats_in_reverse_date_order(samp
 
     template_stats = dao_get_template_statistics_for_service(sample_template.service.id)
     assert len(template_stats) == 0
-    data = _notification_json(sample_template, sample_job.id)
+    data = _notification_json(sample_template, job_id=sample_job.id)
 
     notification = Notification(**data)
     dao_create_notification(notification, sample_template.template_type, mmg_provider.identifier)
@@ -901,7 +895,7 @@ def test_sms_fragment_count(char_count, expected_sms_fragment_count):
     assert get_sms_fragment_count(char_count) == expected_sms_fragment_count
 
 
-def _notification_json(sample_template, job_id=None):
+def _notification_json(sample_template, job_id=None, id=None):
     data = {
         'to': '+44709123456',
         'service': sample_template.service,
@@ -914,4 +908,6 @@ def _notification_json(sample_template, job_id=None):
     }
     if job_id:
         data.update({'job_id': job_id})
+    if id:
+        data.update({'id': id})
     return data

tests/app/dao/test_templates_dao.py

@@ -1,11 +1,10 @@
 from sqlalchemy.orm.exc import NoResultFound
-from sqlalchemy.exc import IntegrityError
 from app.dao.templates_dao import (
     dao_create_template,
     dao_get_template_by_id_and_service_id,
     dao_get_all_templates_for_service,
-    dao_update_template
+    dao_update_template,
-)
+    dao_get_template_versions)
 from tests.app.conftest import sample_template as create_sample_template
 from app.models import Template
 import pytest
@@ -230,3 +229,19 @@ def test_get_template_history_version(sample_user, sample_service, sample_templa
         '1'
     )
     assert old_template.content == old_content
+
+
+def test_get_template_versions(sample_template):
+    original_content = sample_template.content
+    sample_template.content = 'new version'
+    dao_update_template(sample_template)
+    versions = dao_get_template_versions(service_id=sample_template.service_id, template_id=sample_template.id)
+    assert versions.__len__() == 2
+    for x in versions:
+        if x.version == 2:
+            assert x.content == 'new version'
+        else:
+            assert x.content == original_content
+    from app.schemas import (template_history_schema)
+    v = template_history_schema.load(versions, many=True)
+    assert v.__len__() == 2

tests/app/notifications/test_rest.py

@@ -3,6 +3,7 @@ import uuid
 import random
 import string
 import app.celery.tasks
+from app import encryption
 from tests import create_authorization_header
 from tests.app.conftest import sample_notification as create_sample_notification
 from tests.app.conftest import sample_job as create_sample_job
@@ -516,11 +517,10 @@ def test_send_notification_with_placeholders_replaced(notify_api, sample_templat
     with notify_api.test_request_context():
         with notify_api.test_client() as client:
             mocker.patch('app.celery.tasks.send_sms.apply_async')
-            mocker.patch('app.encryption.encrypt', return_value="something_encrypted")
 
             data = {
                 'to': '+447700900855',
-                'template': sample_template_with_placeholders.id,
+                'template': str(sample_template_with_placeholders.id),
                 'personalisation': {
                     'name': 'Jo'
                 }
@@ -533,11 +533,13 @@ def test_send_notification_with_placeholders_replaced(notify_api, sample_templat
                 headers=[('Content-Type', 'application/json'), auth_header])
 
             notification_id = json.loads(response.data)['data']['notification']['id']
+            data.update({"template_version": sample_template_with_placeholders.version})
+            encrypted_notification = encryption.encrypt(data)
 
             app.celery.tasks.send_sms.apply_async.assert_called_once_with(
                 (str(sample_template_with_placeholders.service.id),
                  notification_id,
-                 "something_encrypted",
+                 encrypted_notification,
                  "2016-01-01T11:09:00.061258"),
                 queue="sms"
             )
@@ -705,6 +707,9 @@ def test_should_allow_valid_sms_notification(notify_api, sample_template, mocker
 
             notification_id = json.loads(response.data)['data']['notification']['id']
             assert app.encryption.encrypt.call_args[0][0]['to'] == '+447700900855'
+            assert app.encryption.encrypt.call_args[0][0]['template'] == str(sample_template.id)
+            assert app.encryption.encrypt.call_args[0][0]['template_version'] == sample_template.version
+
             app.celery.tasks.send_sms.apply_async.assert_called_once_with(
                 (str(sample_template.service_id),
                  notification_id,
@@ -895,6 +900,9 @@ def test_should_allow_valid_email_notification(notify_api, sample_email_template
                 headers=[('Content-Type', 'application/json'), auth_header])
             assert response.status_code == 201
             notification_id = json.loads(response.get_data(as_text=True))['data']['notification']['id']
+            assert app.encryption.encrypt.call_args[0][0]['to'] == 'ok@ok.com'
+            assert app.encryption.encrypt.call_args[0][0]['template'] == str(sample_email_template.id)
+            assert app.encryption.encrypt.call_args[0][0]['template_version'] == sample_email_template.version
             app.celery.tasks.send_email.apply_async.assert_called_once_with(
                 (str(sample_email_template.service_id),
                  notification_id,
@@ -903,6 +911,7 @@ def test_should_allow_valid_email_notification(notify_api, sample_email_template
                  "2016-01-01T11:09:00.061258"),
                 queue="email"
             )
+
             assert response.status_code == 201
             assert notification_id
 

tests/app/template/test_rest.py

@@ -460,3 +460,27 @@ def test_update_400_for_over_limit_content(notify_api, sample_user, sample_templ
             assert (
                 'Content has a character count greater than the limit of {}'
             ).format(limit) in json_resp['message']['content']
+
+
+def test_should_return_all_template_versions_for_service_and_template_id(notify_api, 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])
+            assert resp.status_code == 200
+            resp_json = json.loads(resp.get_data(as_text=True))['data']
+            assert len(resp_json) == 3
+            for x in resp_json:
+                if x['version'] == 1:
+                    assert x['content'] == original_content
+                elif x['version'] == 2:
+                    assert x['content'] == original_content + '1'
+                else:
+                    assert x['content'] == original_content + '2'