Restored code to share sms creation logic between:

- sms for API calls
- sms for Jobs

app/notifications/rest.py

@@ -45,37 +45,24 @@ def get_notifications(notification_id):
 
 @notifications.route('/sms', methods=['POST'])
 def create_sms_notification():
-    notification, errors = sms_template_notification_schema.load(request.get_json())
+    return base_create_sms_notification(expects_job=False)
-    if errors:
-        return jsonify(result="error", message=errors), 400
-
-    template = templates_dao.dao_get_template_by_id_and_service_id(
-        template_id=notification['template'],
-        service_id=api_user['client']
-    )
-
-    if not template:
-        return jsonify(result="error", message={'template': ['Template not found']}), 400
-
-    service = services_dao.dao_fetch_service_by_id(api_user['client'])
-
-    if service.restricted:
-        if notification['to'] not in [user.email_address for user in service.users]:
-            return jsonify(result="error", message={'to': ['Invalid phone number for restricted service']}), 400
-
-    notification_id = create_notification_id()
-
-    send_sms.apply_async((
-        api_user['client'],
-        notification_id,
-        encryption.encrypt(notification)),
-        queue='sms')
-    return jsonify({'notification_id': notification_id}), 201
 
 
 @notifications.route('/sms/service/<service_id>', methods=['POST'])
 def create_sms_for_service(service_id):
-    notification, errors = job_sms_template_notification_schema.load(request.get_json())
+    return base_create_sms_notification(service_id, expects_job=True)
+
+
+def base_create_sms_notification(service_id=None, expects_job=False):
+    if not service_id:
+        service_id = api_user['client']
+
+    if expects_job:
+        schema = job_sms_template_notification_schema
+    else:
+        schema = sms_template_notification_schema
+
+    notification, errors = schema.load(request.get_json())
     if errors:
         return jsonify(result="error", message=errors), 400
 
@@ -85,14 +72,18 @@ def create_sms_for_service(service_id):
     )
 
     if not template:
-        return jsonify(result="error", message={'template': ['Template or service not found']}), 400
+        return jsonify(
+            result="error",
+            message={
+                'template': ['Template {} not found for service {}'.format(notification['template'], service_id)]
+            }
+        ), 400
 
-    job_id = notification['job']
+    if expects_job:
+        job = jobs_dao.get_job(service_id, notification['job'])
 
-    job = jobs_dao.get_job(service_id, job_id)
+        if not job:
-
+            return jsonify(result="error", message={'job': ['Job {} not found'.format(notification['job'])]}), 400
-    if not job:
-        return jsonify(result="error", message={'job': ['Job not found']}), 400
 
     service = services_dao.dao_fetch_service_by_id(service_id)
 

tests/app/notifications/test_rest.py

@@ -180,7 +180,8 @@ def test_send_notification_invalid_template_id(notify_api, sample_template, mock
 
             assert response.status_code == 400
             assert len(json_resp['message'].keys()) == 1
-            assert 'Template not found' in json_resp['message']['template']
+            test_string = 'Template {} not found for service {}'.format(9999, sample_template.service.id)
+            assert test_string in json_resp['message']['template']
 
 
 def test_send_notification_invalid_service_id_on_job_sms(notify_api, sample_job, mocker):
@@ -212,7 +213,8 @@ def test_send_notification_invalid_service_id_on_job_sms(notify_api, sample_job,
 
             assert response.status_code == 400
             assert len(json_resp['message'].keys()) == 1
-            assert 'Template or service not found' in json_resp['message']['template']
+            test_string = 'Template {} not found for service {}'.format(sample_job.template.id, service_id)
+            assert test_string in json_resp['message']['template']
 
 
 def test_send_notification_invalid_template_id_on_job_sms(notify_api, sample_job, mocker):
@@ -242,18 +244,19 @@ def test_send_notification_invalid_template_id_on_job_sms(notify_api, sample_job
 
             assert response.status_code == 400
             assert len(json_resp['message'].keys()) == 1
-            assert 'Template or service not found' in json_resp['message']['template']
+            test_string = 'Template {} not found for service {}'.format(9999, sample_job.service.id)
+            assert test_string in json_resp['message']['template']
 
 
 def test_send_notification_invalid_job_id_on_job_sms(notify_api, sample_template, mocker):
     with notify_api.test_request_context():
         with notify_api.test_client() as client:
             mocker.patch('app.celery.tasks.send_sms.apply_async')
-
+            job_id = uuid.uuid4()
             data = {
                 'to': '+441234123123',
                 'template': sample_template.id,
-                'job': uuid.uuid4()
+                'job': job_id
 
             }
 
@@ -273,7 +276,8 @@ def test_send_notification_invalid_job_id_on_job_sms(notify_api, sample_template
 
             assert response.status_code == 400
             assert len(json_resp['message'].keys()) == 1
-            assert 'Job not found' in json_resp['message']['job']
+            test_string = 'Job {} not found'.format(job_id)
+            assert test_string in json_resp['message']['job']
 
 
 def test_prevents_sending_to_any_mobile_on_restricted_service(notify_api, sample_template, mocker):
@@ -374,7 +378,8 @@ def test_should_not_allow_template_from_another_service(notify_api, service_fact
             app.celery.tasks.send_sms.apply_async.assert_not_called()
 
             assert response.status_code == 400
-            assert 'Template not found' in json_resp['message']['template']
+            test_string = 'Template {} not found for service {}'.format(service_2_templates[0].id, service_1.id)
+            assert test_string in json_resp['message']['template']
 
 
 def test_should_not_allow_template_from_another_service_on_job_sms(
@@ -418,7 +423,8 @@ def test_should_not_allow_template_from_another_service_on_job_sms(
             app.celery.tasks.send_sms.apply_async.assert_not_called()
 
             assert response.status_code == 400
-            assert 'Template or service not found' in json_resp['message']['template']
+            test_string = 'Template {} not found for service {}'.format(service_2_templates[0].id, service_1.id)
+            assert test_string in json_resp['message']['template']
 
 
 def test_should_allow_valid_sms_notification(notify_api, sample_template, mocker):