Merge pull request #341 from alphagov/no-jobs-for-archived-templates

Prevent creating jobs for deleted templates

app/job/rest.py

@@ -16,7 +16,7 @@ from app.dao.services_dao import (
 
 from app.dao.templates_dao import (dao_get_template_by_id)
 
-from app.schemas import job_schema
+from app.schemas import job_schema, unarchived_template_schema
 
 from app.celery.tasks import process_job
 
@@ -52,6 +52,12 @@ def create_job(service_id):
         "service": service_id
     })
     template = dao_get_template_by_id(data['template'])
+
+    errors = unarchived_template_schema.validate({'archived': template.archived})
+
+    if errors:
+        return jsonify(result='error', message=errors), 400
+
     data.update({"template_version": template.version})
     job, errors = job_schema.load(data)
     if errors:

app/notifications/rest.py

@@ -1,4 +1,4 @@
-from datetime import datetime, date
+from datetime import datetime
 import statsd
 import itertools
 from flask import (
@@ -30,6 +30,7 @@ from app.schemas import (
     notifications_filter_schema,
     notifications_statistics_schema,
     day_schema,
+    unarchived_template_schema
 )
 from app.celery.tasks import send_sms, send_email
 
@@ -328,6 +329,10 @@ def send_notification(notification_type):
         service_id=service_id
     )
 
+    errors = unarchived_template_schema.validate({'archived': template.archived})
+    if errors:
+        return jsonify(result='error', message=errors), 400
+
     template_object = Template(template.__dict__, notification.get('personalisation', {}))
     if template_object.missing_data:
         return jsonify(

app/schemas.py

@@ -381,6 +381,15 @@ class WeekAggregateNotificationStatisticsSchema(ma.Schema):
         _validate_positive_number(value)
 
 
+class UnarchivedTemplateSchema(BaseSchema):
+    archived = fields.Boolean(required=True)
+
+    @validates_schema
+    def validate_archived(self, data):
+        if data['archived']:
+            raise ValidationError('Template has been deleted', 'template')
+
+
 user_schema = UserSchema()
 user_schema_load_json = UserSchema(load_json=True)
 service_schema = ServiceSchema()
@@ -413,3 +422,4 @@ from_to_date_schema = FromToDateSchema()
 provider_details_schema = ProviderDetailsSchema()
 week_aggregate_notification_statistics_schema = WeekAggregateNotificationStatisticsSchema()
 day_schema = DaySchema()
+unarchived_template_schema = UnarchivedTemplateSchema()

tests/app/job/test_rest.py

@@ -4,6 +4,7 @@ import app.celery.tasks
 
 from tests import create_authorization_header
 from tests.app.conftest import sample_job as create_job
+from app.dao.templates_dao import dao_update_template
 
 
 def test_get_jobs(notify_api, notify_db, notify_db_session, sample_template):
@@ -178,6 +179,31 @@ def test_create_job_returns_404_if_missing_service(notify_api, sample_template,
             assert resp_json['message'] == 'No result found'
 
 
+def test_create_job_returns_400_if_archived_template(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')
+            sample_template.archived = True
+            dao_update_template(sample_template)
+            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)
+            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 == 400
+
+            app.celery.tasks.process_job.apply_async.assert_not_called()
+            assert resp_json['result'] == 'error'
+            assert 'Template has been deleted' in resp_json['message']['template']
+
+
 def _setup_jobs(notify_db, notify_db_session, template, number_of_jobs=5):
     for i in range(number_of_jobs):
         create_job(

tests/app/notifications/test_rest.py

@@ -13,7 +13,7 @@ from tests.app.conftest import sample_email_template as create_sample_email_temp
 from tests.app.conftest import sample_template as create_sample_template
 from flask import (json, current_app, url_for)
 from app.models import Service
-from app.dao.templates_dao import dao_get_all_templates_for_service
+from app.dao.templates_dao import dao_get_all_templates_for_service, dao_update_template
 from app.dao.services_dao import dao_update_service
 from app.dao.notifications_dao import get_notification_by_id, dao_get_notification_statistics_for_service
 from freezegun import freeze_time
@@ -564,6 +564,28 @@ def test_send_notification_with_placeholders_replaced(notify_api, sample_templat
             assert encryption.decrypt(app.celery.tasks.send_sms.apply_async.call_args[0][0][2]) == data
 
 
+def test_should_not_send_notification_for_archived_template(notify_api, sample_template, mocker):
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            sample_template.archived = True
+            dao_update_template(sample_template)
+            limit = current_app.config.get('SMS_CHAR_COUNT_LIMIT')
+            json_data = json.dumps({
+                'to': '+447700900855',
+                'template': sample_template.id
+            })
+            endpoint = url_for('notifications.send_notification', notification_type='sms')
+            auth_header = create_authorization_header(service_id=sample_template.service.id)
+
+            resp = client.post(
+                path=endpoint,
+                data=json_data,
+                headers=[('Content-Type', 'application/json'), auth_header])
+            assert resp.status_code == 400
+            json_resp = json.loads(resp.get_data(as_text=True))
+            assert 'Template has been deleted' in json_resp['message']['template']
+
+
 def test_send_notification_with_missing_personalisation(notify_api, sample_template_with_placeholders, mocker):
     with notify_api.test_request_context():
         with notify_api.test_client() as client: