send to send-sms-tasks and send-email-tasks instead of send-tasks

app/celery/tasks.py

@@ -182,7 +182,7 @@ def send_sms(self,
 
         provider_tasks.deliver_sms.apply_async(
             [str(saved_notification.id)],
-            queue=QueueNames.SEND_COMBINED if not service.research_mode else QueueNames.RESEARCH_MODE
+            queue=QueueNames.SEND_SMS if not service.research_mode else QueueNames.RESEARCH_MODE
         )
 
         current_app.logger.info(
@@ -227,7 +227,7 @@ def send_email(self,
 
         provider_tasks.deliver_email.apply_async(
             [str(saved_notification.id)],
-            queue=QueueNames.SEND_COMBINED if not service.research_mode else QueueNames.RESEARCH_MODE
+            queue=QueueNames.SEND_EMAIL if not service.research_mode else QueueNames.RESEARCH_MODE
         )
 
         current_app.logger.info("Email {} created at {}".format(saved_notification.id, created_at))

app/delivery/rest.py

@@ -24,16 +24,20 @@ def send_notification_to_provider(notification_id):
         send_response(
             send_to_providers.send_email_to_provider,
             provider_tasks.deliver_email,
-            notification)
+            notification,
+            QueueNames.SEND_EMAIL
+        )
     else:
         send_response(
             send_to_providers.send_sms_to_provider,
             provider_tasks.deliver_sms,
-            notification)
+            notification,
+            QueueNames.SEND_SMS
+        )
     return jsonify({}), 204
 
 
-def send_response(send_call, task_call, notification):
+def send_response(send_call, task_call, notification, queue):
     try:
         send_call(notification)
     except Exception as e:
@@ -42,4 +46,4 @@ def send_response(send_call, task_call, notification):
                 notification.id,
                 notification.notification_type),
             e)
-        task_call.apply_async((str(notification.id)), queue=QueueNames.SEND_COMBINED)
+        task_call.apply_async((str(notification.id)), queue=queue)

app/notifications/process_notifications.py

@@ -100,12 +100,14 @@ def persist_notification(
 def send_notification_to_queue(notification, research_mode, queue=None):
     if research_mode or notification.key_type == KEY_TYPE_TEST:
         queue = QueueNames.RESEARCH_MODE
-    elif not queue:
-        queue = QueueNames.SEND_COMBINED
 
     if notification.notification_type == SMS_TYPE:
+        if not queue:
+            queue = QueueNames.SEND_SMS
         deliver_task = provider_tasks.deliver_sms
     if notification.notification_type == EMAIL_TYPE:
+        if not queue:
+            queue = QueueNames.SEND_EMAIL
         deliver_task = provider_tasks.deliver_email
 
     try:

tests/app/celery/test_scheduled_tasks.py

@@ -468,7 +468,7 @@ def test_should_send_all_scheduled_notifications_to_deliver_queue(sample_templat
 
     send_scheduled_notifications()
 
-    mocked.apply_async.assert_called_once_with([str(message_to_deliver.id)], queue='send-tasks')
+    mocked.apply_async.assert_called_once_with([str(message_to_deliver.id)], queue='send-sms-tasks')
     scheduled_notifications = dao_get_scheduled_notifications()
     assert not scheduled_notifications
 

tests/app/celery/test_tasks.py

@@ -422,7 +422,7 @@ def test_should_send_template_to_correct_sms_task_and_persist(sample_template_wi
     assert persisted_notification.notification_type == 'sms'
     mocked_deliver_sms.assert_called_once_with(
         [str(persisted_notification.id)],
-        queue="send-tasks"
+        queue="send-sms-tasks"
     )
 
 
@@ -483,7 +483,7 @@ def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notif
     assert persisted_notification.notification_type == 'sms'
     provider_tasks.deliver_sms.apply_async.assert_called_once_with(
         [str(persisted_notification.id)],
-        queue="send-tasks"
+        queue="send-sms-tasks"
     )
 
 
@@ -509,7 +509,7 @@ def test_should_send_sms_if_restricted_service_and_non_team_number_with_test_key
     persisted_notification = Notification.query.one()
     mocked_deliver_sms.assert_called_once_with(
         [str(persisted_notification.id)],
-        queue="send-tasks"
+        queue="send-sms-tasks"
     )
 
 
@@ -537,7 +537,7 @@ def test_should_send_email_if_restricted_service_and_non_team_email_address_with
     persisted_notification = Notification.query.one()
     mocked_deliver_email.assert_called_once_with(
         [str(persisted_notification.id)],
-        queue="send-tasks"
+        queue="send-email-tasks"
     )
 
 
@@ -641,7 +641,7 @@ def test_should_send_sms_template_to_and_persist_with_job_id(sample_job, sample_
 
     provider_tasks.deliver_sms.apply_async.assert_called_once_with(
         [str(persisted_notification.id)],
-        queue="send-tasks"
+        queue="send-sms-tasks"
     )
 
 
@@ -738,7 +738,7 @@ def test_should_use_email_template_and_persist(sample_email_template_with_placeh
     assert persisted_notification.notification_type == 'email'
 
     provider_tasks.deliver_email.apply_async.assert_called_once_with(
-        [str(persisted_notification.id)], queue='send-tasks')
+        [str(persisted_notification.id)], queue='send-email-tasks')
 
 
 def test_send_email_should_use_template_version_from_job_not_latest(sample_email_template, mocker):
@@ -769,7 +769,7 @@ def test_send_email_should_use_template_version_from_job_not_latest(sample_email
     assert not persisted_notification.sent_by
     assert persisted_notification.notification_type == 'email'
     provider_tasks.deliver_email.apply_async.assert_called_once_with([str(persisted_notification.id)],
-                                                                     queue='send-tasks')
+                                                                     queue='send-email-tasks')
 
 
 def test_should_use_email_template_subject_placeholders(sample_email_template_with_placeholders, mocker):
@@ -795,7 +795,7 @@ def test_should_use_email_template_subject_placeholders(sample_email_template_wi
     assert not persisted_notification.reference
     assert persisted_notification.notification_type == 'email'
     provider_tasks.deliver_email.apply_async.assert_called_once_with(
-        [str(persisted_notification.id)], queue='send-tasks'
+        [str(persisted_notification.id)], queue='send-email-tasks'
     )
 
 
@@ -823,7 +823,7 @@ def test_should_use_email_template_and_persist_without_personalisation(sample_em
     assert not persisted_notification.reference
     assert persisted_notification.notification_type == 'email'
     provider_tasks.deliver_email.apply_async.assert_called_once_with([str(persisted_notification.id)],
-                                                                     queue='send-tasks')
+                                                                     queue='send-email-tasks')
 
 
 def test_send_sms_should_go_to_retry_queue_if_database_errors(sample_template, mocker):

tests/app/delivery/test_rest.py

@@ -78,7 +78,7 @@ def test_should_call_deliver_sms_task_if_send_sms_to_provider_fails(notify_api,
             )
         app.delivery.send_to_providers.send_sms_to_provider.assert_called_with(sample_notification)
         app.celery.provider_tasks.deliver_sms.apply_async.assert_called_with(
-            (str(sample_notification.id)), queue='send-tasks'
+            (str(sample_notification.id)), queue='send-sms-tasks'
         )
         assert response.status_code == 204
 
@@ -100,6 +100,6 @@ def test_should_call_deliver_email_task_if_send_email_to_provider_fails(
             )
         app.delivery.send_to_providers.send_email_to_provider.assert_called_with(sample_email_notification)
         app.celery.provider_tasks.deliver_email.apply_async.assert_called_with(
-            (str(sample_email_notification.id)), queue='send-tasks'
+            (str(sample_email_notification.id)), queue='send-email-tasks'
         )
         assert response.status_code == 204

tests/app/notifications/rest/test_send_notification.py

@@ -132,7 +132,7 @@ def test_send_notification_with_placeholders_replaced(notify_api, sample_email_t
 
             mocked.assert_called_once_with(
                 [notification_id],
-                queue="send-tasks"
+                queue="send-email-tasks"
             )
             assert response.status_code == 201
             assert response_data['body'] == u'Hello Jo\nThis is an email from GOV.\u200BUK'
@@ -342,7 +342,7 @@ def test_should_allow_valid_sms_notification(notify_api, sample_template, mocker
             response_data = json.loads(response.data)['data']
             notification_id = response_data['notification']['id']
 
-            mocked.assert_called_once_with([notification_id], queue='send-tasks')
+            mocked.assert_called_once_with([notification_id], queue='send-sms-tasks')
             assert response.status_code == 201
             assert notification_id
             assert 'subject' not in response_data
@@ -395,7 +395,7 @@ def test_should_allow_valid_email_notification(notify_api, sample_email_template
             notification_id = response_data['notification']['id']
             app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with(
                 [notification_id],
-                queue="send-tasks"
+                queue="send-email-tasks"
             )
 
             assert response.status_code == 201
@@ -593,7 +593,10 @@ def test_should_send_email_if_team_api_key_and_a_service_user(notify_api, sample
             data=json.dumps(data),
             headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))])
 
-        app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with([fake_uuid], queue='send-tasks')
+        app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with(
+            [fake_uuid],
+            queue='send-email-tasks'
+        )
         assert response.status_code == 201
 
 
@@ -689,57 +692,67 @@ def test_should_send_sms_if_team_api_key_and_a_service_user(notify_api, sample_t
             data=json.dumps(data),
             headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))])
 
-        app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with([fake_uuid], queue='send-tasks')
+        app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with([fake_uuid], queue='send-sms-tasks')
         assert response.status_code == 201
 
 
-@pytest.mark.parametrize('template_type',
+@pytest.mark.parametrize('template_type,queue_name', [
-                         [SMS_TYPE, EMAIL_TYPE])
+    (SMS_TYPE, 'send-sms-tasks'),
-def test_should_persist_notification(notify_api, sample_template,
+    (EMAIL_TYPE, 'send-email-tasks')
-                                     sample_email_template,
+])
-                                     template_type,
+def test_should_persist_notification(
-                                     fake_uuid, mocker):
+    client,
-    with notify_api.test_request_context(), notify_api.test_client() as client:
+    sample_template,
-        mocked = mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(template_type))
+    sample_email_template,
-        mocker.patch('app.dao.notifications_dao.create_uuid', return_value=fake_uuid)
+    fake_uuid,
-        template = sample_template if template_type == SMS_TYPE else sample_email_template
+    mocker,
-        to = sample_template.service.created_by.mobile_number if template_type == SMS_TYPE \
+    template_type,
-            else sample_email_template.service.created_by.email_address
+    queue_name
-        data = {
+):
-            'to': to,
+    mocked = mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(template_type))
-            'template': template.id
+    mocker.patch('app.dao.notifications_dao.create_uuid', return_value=fake_uuid)
-        }
+    template = sample_template if template_type == SMS_TYPE else sample_email_template
-        api_key = ApiKey(
+    to = sample_template.service.created_by.mobile_number if template_type == SMS_TYPE \
-            service=template.service,
+        else sample_email_template.service.created_by.email_address
-            name='team_key',
+    data = {
-            created_by=template.created_by,
+        'to': to,
-            key_type=KEY_TYPE_TEAM)
+        'template': template.id
-        save_model_api_key(api_key)
+    }
-        auth_header = create_jwt_token(secret=api_key.secret, client_id=str(api_key.service_id))
+    api_key = ApiKey(
+        service=template.service,
+        name='team_key',
+        created_by=template.created_by,
+        key_type=KEY_TYPE_TEAM)
+    save_model_api_key(api_key)
+    auth_header = create_jwt_token(secret=api_key.secret, client_id=str(api_key.service_id))
 
-        response = client.post(
+    response = client.post(
-            path='/notifications/{}'.format(template_type),
+        path='/notifications/{}'.format(template_type),
-            data=json.dumps(data),
+        data=json.dumps(data),
-            headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))])
+        headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))])
 
-        mocked.assert_called_once_with([fake_uuid], queue='send-tasks')
+    mocked.assert_called_once_with([fake_uuid], queue=queue_name)
-        assert response.status_code == 201
+    assert response.status_code == 201
 
-        notification = notifications_dao.get_notification_by_id(fake_uuid)
+    notification = notifications_dao.get_notification_by_id(fake_uuid)
-        assert notification.to == to
+    assert notification.to == to
-        assert notification.template_id == template.id
+    assert notification.template_id == template.id
-        assert notification.notification_type == template_type
+    assert notification.notification_type == template_type
 
 
-@pytest.mark.parametrize('template_type',
+@pytest.mark.parametrize('template_type,queue_name', [
-                         [SMS_TYPE, EMAIL_TYPE])
+    (SMS_TYPE, 'send-sms-tasks'),
+    (EMAIL_TYPE, 'send-email-tasks')
+])
 def test_should_delete_notification_and_return_error_if_sqs_fails(
-        client,
+    client,
-        sample_email_template,
+    sample_email_template,
-        sample_template,
+    sample_template,
-        fake_uuid,
+    fake_uuid,
-        mocker,
+    mocker,
-        template_type):
+    template_type,
+    queue_name
+):
     mocked = mocker.patch(
         'app.celery.provider_tasks.deliver_{}.apply_async'.format(template_type),
         side_effect=Exception("failed to talk to SQS")
@@ -768,7 +781,7 @@ def test_should_delete_notification_and_return_error_if_sqs_fails(
         )
     assert str(e.value) == 'failed to talk to SQS'
 
-    mocked.assert_called_once_with([fake_uuid], queue='send-tasks')
+    mocked.assert_called_once_with([fake_uuid], queue=queue_name)
     assert not notifications_dao.get_notification_by_id(fake_uuid)
     assert not NotificationHistory.query.get(fake_uuid)
 
@@ -1119,7 +1132,7 @@ def test_should_allow_store_original_number_on_sms_notification(client, sample_t
     response_data = json.loads(response.data)['data']
     notification_id = response_data['notification']['id']
 
-    mocked.assert_called_once_with([notification_id], queue='send-tasks')
+    mocked.assert_called_once_with([notification_id], queue='send-sms-tasks')
     assert response.status_code == 201
     assert notification_id
     notifications = Notification.query.all()

tests/app/notifications/test_process_notification.py

@@ -214,9 +214,9 @@ def test_persist_notification_increments_cache_if_key_exists(sample_template, sa
                          [(True, None, 'research-mode-tasks', 'sms', 'normal'),
                           (True, None, 'research-mode-tasks', 'email', 'normal'),
                           (True, None, 'research-mode-tasks', 'email', 'team'),
-                          (False, None, 'send-tasks', 'sms', 'normal'),
+                          (False, None, 'send-sms-tasks', 'sms', 'normal'),
-                          (False, None, 'send-tasks', 'email', 'normal'),
+                          (False, None, 'send-email-tasks', 'email', 'normal'),
-                          (False, None, 'send-tasks', 'sms', 'team'),
+                          (False, None, 'send-sms-tasks', 'sms', 'team'),
                           (False, None, 'research-mode-tasks', 'sms', 'test'),
                           (True, 'notify-internal-tasks', 'research-mode-tasks', 'email', 'normal'),
                           (False, 'notify-internal-tasks', 'notify-internal-tasks', 'sms', 'normal'),