diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 55ec0fd71..c22f8bac2 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -143,7 +143,7 @@ def process_job(job_id): dao_update_job(job) template = Template( - dao_get_template_by_id(job.template_id).__dict__ + dao_get_template_by_id(job.template_id, job.template_version).__dict__ ) for recipient, personalisation in RecipientCSV( @@ -154,6 +154,7 @@ def process_job(job_id): encrypted = encryption.encrypt({ 'template': str(template.id), + 'template_version': job.template_version, 'job': str(job.id), 'to': recipient, 'personalisation': { @@ -219,7 +220,7 @@ def send_sms(service_id, notification_id, encrypted_notification, created_at): 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', {}), prefix=service.name ) @@ -228,6 +229,7 @@ def send_sms(service_id, notification_id, encrypted_notification, created_at): 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), diff --git a/app/job/rest.py b/app/job/rest.py index 1e658ccaf..381bf0c6c 100644 --- a/app/job/rest.py +++ b/app/job/rest.py @@ -14,6 +14,8 @@ from app.dao.services_dao import ( dao_fetch_service_by_id ) +from app.dao.templates_dao import (dao_get_template_by_id) + from app.schemas import job_schema from app.celery.tasks import process_job @@ -49,6 +51,8 @@ def create_job(service_id): data.update({ "service": service_id }) + template = dao_get_template_by_id(data['template']) + data.update({"template_version": template.version}) job, errors = job_schema.load(data) if errors: return jsonify(result="error", message=errors), 400 diff --git a/app/models.py b/app/models.py index 3aae8ad2b..e524a14bb 100644 --- a/app/models.py +++ b/app/models.py @@ -224,6 +224,7 @@ class Job(db.Model): service = db.relationship('Service', backref=db.backref('jobs', lazy='dynamic')) template_id = db.Column(UUID(as_uuid=True), db.ForeignKey('templates.id'), index=True, unique=False) template = db.relationship('Template', backref=db.backref('jobs', lazy='dynamic')) + template_version = db.Column(db.Integer, nullable=False) created_at = db.Column( db.DateTime, index=False, @@ -301,6 +302,7 @@ class Notification(db.Model): service = db.relationship('Service') template_id = db.Column(UUID(as_uuid=True), db.ForeignKey('templates.id'), index=True, unique=False) template = db.relationship('Template') + template_version = db.Column(db.Integer, nullable=False) content_char_count = db.Column(db.Integer, nullable=True) created_at = db.Column( db.DateTime, diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 04e38a9e6..a3a973e74 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -245,6 +245,7 @@ def sample_job(notify_db, 'service_id': service.id, 'service': service, 'template_id': template.id, + 'template_version': 1, 'original_file_name': 'some.csv', 'notification_count': notification_count, 'created_at': created_at, @@ -342,6 +343,7 @@ def sample_notification(notify_db, 'service_id': service.id, 'service': service, 'template': template, + 'template_version': template.version, 'status': status, 'reference': reference, 'created_at': created_at, diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index 083947d1e..14a00695d 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -45,14 +45,7 @@ def test_should_by_able_to_update_reference_by_id(sample_notification): def test_should_by_able_to_update_status_by_reference(sample_email_template, ses_provider_name): - data = { - 'to': '+44709123456', - 'service': sample_email_template.service, - 'service_id': sample_email_template.service.id, - 'template': sample_email_template, - 'template_id': sample_email_template.id, - 'created_at': datetime.utcnow() - } + data = _notification_json(sample_email_template) notification = Notification(**data) dao_create_notification( @@ -108,14 +101,7 @@ def test_should_be_able_to_record_statistics_failure_for_sms(sample_notification def test_should_be_able_to_record_statistics_failure_for_email(sample_email_template, ses_provider_name): - data = { - 'to': '+44709123456', - 'service': sample_email_template.service, - 'service_id': sample_email_template.service.id, - 'template': sample_email_template, - 'template_id': sample_email_template.id, - 'created_at': datetime.utcnow() - } + data = _notification_json(sample_email_template) notification = Notification(**data) dao_create_notification(notification, sample_email_template.template_type, ses_provider_name) @@ -144,15 +130,7 @@ def test_should_return_zero_count_if_no_notification_with_reference(): def test_should_be_able_to_get_statistics_for_a_service(sample_template, mmg_provider_name): - data = { - 'to': '+44709123456', - 'service': sample_template.service, - 'service_id': sample_template.service.id, - 'template': sample_template, - 'template_id': sample_template.id, - 'created_at': datetime.utcnow(), - 'content_char_count': 160 - } + data = _notification_json(sample_template) notification = Notification(**data) dao_create_notification(notification, sample_template.template_type, mmg_provider_name) @@ -172,16 +150,7 @@ def test_should_be_able_to_get_statistics_for_a_service(sample_template, mmg_pro def test_should_be_able_to_get_statistics_for_a_service_for_a_day(sample_template, mmg_provider_name): now = datetime.utcnow() - data = { - 'to': '+44709123456', - 'service': sample_template.service, - 'service_id': sample_template.service.id, - 'template': sample_template, - 'template_id': sample_template.id, - 'created_at': now, - 'content_char_count': 160 - } - + data = _notification_json(sample_template) notification = Notification(**data) dao_create_notification(notification, sample_template.template_type, mmg_provider_name) stat = dao_get_notification_statistics_for_service_and_day( @@ -198,16 +167,7 @@ def test_should_be_able_to_get_statistics_for_a_service_for_a_day(sample_templat def test_should_return_none_if_no_statistics_for_a_service_for_a_day(sample_template, mmg_provider_name): - now = datetime.utcnow() - data = { - 'to': '+44709123456', - 'service': sample_template.service, - 'service_id': sample_template.service.id, - 'template': sample_template, - 'template_id': sample_template.id, - 'created_at': now, - 'content_char_count': 160 - } + data = _notification_json(sample_template) notification = Notification(**data) dao_create_notification(notification, sample_template.template_type, mmg_provider_name) @@ -217,15 +177,7 @@ def test_should_return_none_if_no_statistics_for_a_service_for_a_day(sample_temp def test_should_be_able_to_get_all_statistics_for_a_service(sample_template, mmg_provider_name): - data = { - 'to': '+44709123456', - 'service': sample_template.service, - 'service_id': sample_template.service.id, - 'template': sample_template, - 'template_id': sample_template.id, - 'created_at': datetime.utcnow(), - 'content_char_count': 160 - } + data = _notification_json(sample_template) notification_1 = Notification(**data) notification_2 = Notification(**data) @@ -241,14 +193,7 @@ def test_should_be_able_to_get_all_statistics_for_a_service(sample_template, mmg def test_should_be_able_to_get_all_statistics_for_a_service_for_several_days(sample_template, mmg_provider_name): - data = { - 'to': '+44709123456', - 'service': sample_template.service, - 'service_id': sample_template.service.id, - 'template': sample_template, - 'template_id': sample_template.id, - 'content_char_count': 160 - } + data = _notification_json(sample_template) today = datetime.utcnow() yesterday = datetime.utcnow() - timedelta(days=1) @@ -288,14 +233,7 @@ def test_should_be_empty_list_if_no_statistics_for_a_service(sample_service): def test_should_be_able_to_get_all_statistics_for_a_service_for_several_days_previous(sample_template, mmg_provider_name): - data = { - 'to': '+44709123456', - 'service': sample_template.service, - 'service_id': sample_template.service.id, - 'template': sample_template, - 'template_id': sample_template.id, - 'content_char_count': 160 - } + data = _notification_json(sample_template) today = datetime.utcnow() seven_days_ago = datetime.utcnow() - timedelta(days=7) @@ -333,16 +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 = { - 'to': '+44709123456', - 'job_id': sample_job.id, - 'service': sample_template.service, - 'service_id': sample_template.service.id, - 'template': sample_template, - 'template_id': sample_template.id, - 'created_at': datetime.utcnow(), - 'content_char_count': 160 - } + data = _notification_json(sample_template, sample_job.id) notification = Notification(**data) dao_create_notification(notification, sample_template.template_type, mmg_provider_name) @@ -379,16 +308,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 = { - 'to': '+44709123456', - 'job_id': sample_job.id, - 'service': sample_email_template.service, - 'service_id': sample_email_template.service.id, - 'template': sample_email_template, - 'template_id': sample_email_template.id, - 'created_at': datetime.utcnow(), - 'content_char_count': 160 - } + data = _notification_json(sample_email_template, sample_job.id) notification = Notification(**data) dao_create_notification(notification, sample_email_template.template_type, ses_provider_name) @@ -422,16 +342,7 @@ def test_save_notification_and_create_email_and_template_stats(sample_email_temp @freeze_time("2016-01-01 00:00:00.000000") def test_save_notification_handles_midnight_properly(sample_template, sample_job, mmg_provider_name): assert Notification.query.count() == 0 - data = { - 'to': '+44709123456', - 'job_id': sample_job.id, - 'service': sample_template.service, - 'service_id': sample_template.service.id, - 'template': sample_template, - 'template_id': sample_template.id, - 'created_at': datetime.utcnow(), - 'content_char_count': 160 - } + data = _notification_json(sample_template, sample_job.id) notification = Notification(**data) dao_create_notification(notification, sample_template.template_type, mmg_provider_name) @@ -448,16 +359,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_name): assert Notification.query.count() == 0 - data = { - 'to': '+44709123456', - 'job_id': sample_job.id, - 'service': sample_template.service, - 'service_id': sample_template.service.id, - 'template': sample_template, - 'template_id': sample_template.id, - 'created_at': datetime.utcnow(), - 'content_char_count': 160 - } + data = _notification_json(sample_template, sample_job.id) notification = Notification(**data) dao_create_notification(notification, sample_template.template_type, mmg_provider_name) @@ -473,16 +375,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_name): assert Notification.query.count() == 0 - data = { - 'to': '+44709123456', - 'job_id': sample_job.id, - 'service': sample_email_template.service, - 'service_id': sample_email_template.service.id, - 'template': sample_email_template, - 'template_id': sample_email_template.id, - 'created_at': datetime.utcnow(), - 'content_char_count': 160 - } + data = _notification_json(sample_email_template, sample_job.id) notification_1 = Notification(**data) notification_2 = Notification(**data) @@ -511,16 +404,7 @@ def test_save_notification_and_increment_email_stats(sample_email_template, samp def test_save_notification_and_increment_sms_stats(sample_template, sample_job, mmg_provider_name): assert Notification.query.count() == 0 - data = { - 'to': '+44709123456', - 'job_id': sample_job.id, - 'service': sample_template.service, - 'service_id': sample_template.service.id, - 'template': sample_template, - 'template_id': sample_template.id, - 'created_at': datetime.utcnow(), - 'content_char_count': 160 - } + data = _notification_json(sample_template, sample_job.id) notification_1 = Notification(**data) notification_2 = Notification(**data) @@ -551,16 +435,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 = { - 'to': '+44709123456', - 'job_id': random_id, - 'service': sample_template.service, - 'service_id': sample_template.service.id, - 'template': sample_template, - 'template_id': sample_template.id, - 'created_at': datetime.utcnow(), - 'content_char_count': 160 - } + data = _notification_json(sample_template, random_id) notification = Notification(**data) with pytest.raises(SQLAlchemyError): @@ -574,16 +449,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_name): assert Notification.query.count() == 0 - data = { - 'to': '+44709123456', - 'job_id': sample_job.id, - 'service': sample_template.service, - 'service_id': sample_template.service.id, - 'template': sample_template, - 'template_id': sample_template.id, - 'created_at': datetime.utcnow(), - 'content_char_count': 160 - } + data = _notification_json(sample_template, sample_job.id) notification = Notification(**data) dao_create_notification(notification, sample_template.template_type, mmg_provider_name) @@ -607,16 +473,15 @@ 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_name): random_id = str(uuid.uuid4()) - assert Notification.query.count() == 0 - data = { + data = data = { 'id': random_id, 'to': '+44709123456', - 'job_id': sample_job.id, - 'service_id': sample_template.service.id, 'service': sample_template.service, + 'service_id': sample_template.service.id, 'template': sample_template, 'template_id': sample_template.id, + 'template_version': sample_template.version, 'created_at': datetime.utcnow(), 'content_char_count': 160 } @@ -642,16 +507,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 = { - 'to': '+44709123456', - 'job_id': job_1.id, - 'service_id': sample_template.service.id, - 'service': sample_template.service, - 'template': sample_template, - 'template_id': sample_template.id, - 'created_at': datetime.utcnow(), - 'content_char_count': 160 - } + data = _notification_json(sample_template, job_1.id) notification = Notification(**data) dao_create_notification(notification, sample_template.template_type, mmg_provider_name) @@ -671,15 +527,7 @@ def test_save_notification_and_increment_correct_job(notify_db, notify_db_sessio def test_save_notification_with_no_job(sample_template, mmg_provider_name): assert Notification.query.count() == 0 - data = { - 'to': '+44709123456', - 'service_id': sample_template.service.id, - 'service': sample_template.service, - 'template': sample_template, - 'template_id': sample_template.id, - 'created_at': datetime.utcnow(), - 'content_char_count': 160 - } + data = _notification_json(sample_template) notification = Notification(**data) dao_create_notification(notification, sample_template.template_type, mmg_provider_name) @@ -704,15 +552,7 @@ def test_get_notification(sample_notification): def test_save_notification_no_job_id(sample_template, mmg_provider_name): assert Notification.query.count() == 0 to = '+44709123456' - data = { - 'to': to, - 'service_id': sample_template.service.id, - 'service': sample_template.service, - 'template': sample_template, - 'template_id': sample_template.id, - 'created_at': datetime.utcnow(), - 'content_char_count': 160 - } + data = _notification_json(sample_template) notification = Notification(**data) dao_create_notification(notification, sample_template.template_type, mmg_provider_name) @@ -802,16 +642,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_name): assert Notification.query.count() == 0 assert TemplateStatistics.query.count() == 0 - data = { - 'to': '+44709123456', - 'job_id': sample_job.id, - 'service': sample_template.service, - 'service_id': sample_template.service.id, - 'template': sample_template, - 'template_id': sample_template.id, - 'created_at': datetime.utcnow(), - 'content_char_count': 160 - } + data = _notification_json(sample_template, sample_job.id) notification = Notification(**data) dao_create_notification(notification, sample_template.template_type, mmg_provider_name) @@ -829,16 +660,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_name): assert Notification.query.count() == 0 assert TemplateStatistics.query.count() == 0 - data = { - 'to': '+44709123456', - 'job_id': sample_job.id, - 'service': sample_template.service, - 'service_id': sample_template.service.id, - 'template': sample_template, - 'template_id': sample_template.id, - 'created_at': datetime.utcnow(), - 'content_char_count': 160 - } + data = _notification_json(sample_template, sample_job.id) notification = Notification(**data) dao_create_notification(notification, sample_template.template_type, mmg_provider_name) @@ -874,16 +696,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_name): assert Notification.query.count() == 0 assert TemplateStatistics.query.count() == 0 - data = { - 'to': '+44709123456', - 'job_id': sample_job.id, - 'service': sample_template.service, - 'service_id': sample_template.service.id, - 'template': sample_template, - 'template_id': sample_template.id, - 'created_at': datetime.utcnow(), - 'content_char_count': 160 - } + data = _notification_json(sample_template, sample_job.id) notification_1 = Notification(**data) notification_2 = Notification(**data) @@ -911,16 +724,7 @@ def test_successful_notification_inserts_followed_by_failure_does_not_increment_ assert NotificationStatistics.query.count() == 0 assert TemplateStatistics.query.count() == 0 - data = { - 'to': '+44709123456', - 'job_id': sample_job.id, - 'service': sample_template.service, - 'service_id': sample_template.service.id, - 'template': sample_template, - 'template_id': sample_template.id, - 'created_at': datetime.utcnow(), - 'content_char_count': 160 - } + data = _notification_json(sample_template, sample_job.id) notification_1 = Notification(**data) notification_2 = Notification(**data) @@ -963,16 +767,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 = { - 'to': '+44709123456', - 'job_id': sample_job.id, - 'service': sample_template.service, - 'service_id': sample_template.service.id, - 'template': sample_template, - 'template_id': sample_template.id, - 'created_at': datetime.utcnow(), - 'content_char_count': 160 - } + data = _notification_json(sample_template, sample_job.id) notification = Notification(**data) dao_create_notification(notification, sample_template.template_type, mmg_provider_name) @@ -1103,3 +898,19 @@ def test_should_limit_notifications_return_by_day_limit_plus_one(notify_db, noti ]) 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): + data = { + 'to': '+44709123456', + 'service': sample_template.service, + 'service_id': sample_template.service.id, + 'template': sample_template, + 'template_id': sample_template.id, + 'template_version': sample_template.version, + 'created_at': datetime.utcnow(), + 'content_char_count': 160 + } + if job_id: + data.update({'job_id': job_id}) + return data \ No newline at end of file diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index 9733c932a..b297d5f50 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -113,6 +113,7 @@ def test_create_job_returns_400_if_missing_data(notify_api, sample_template, moc with notify_api.test_client() as client: mocker.patch('app.celery.tasks.process_job.apply_async') data = { + 'template': str(sample_template.id) } path = '/service/{}/job'.format(sample_template.service.id) auth_header = create_authorization_header(service_id=sample_template.service.id) @@ -132,12 +133,35 @@ def test_create_job_returns_400_if_missing_data(notify_api, sample_template, moc assert 'Missing data for required field.' in resp_json['message']['id'] +def test_create_job_returns_404_if_template_does_not_exist(notify_api, sample_service, mocker): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + mocker.patch('app.celery.tasks.process_job.apply_async') + data = { + 'template': str(sample_service.id) + } + path = '/service/{}/job'.format(sample_service.id) + auth_header = create_authorization_header(service_id=sample_service.id) + headers = [('Content-Type', 'application/json'), auth_header] + response = client.post( + path, + data=json.dumps(data), + headers=headers) + + resp_json = json.loads(response.get_data(as_text=True)) + assert response.status_code == 404 + + app.celery.tasks.process_job.apply_async.assert_not_called() + assert resp_json['result'] == 'error' + assert resp_json['message'] == 'No result found' + + def test_create_job_returns_404_if_missing_service(notify_api, sample_template, mocker): with notify_api.test_request_context(): with notify_api.test_client() as client: mocker.patch('app.celery.tasks.process_job.apply_async') random_id = str(uuid.uuid4()) - data = {} + data = {'template': str(sample_template.id)} path = '/service/{}/job'.format(random_id) auth_header = create_authorization_header(service_id=sample_template.service.id) headers = [('Content-Type', 'application/json'), auth_header]