Change to research mode.

Previously research mode created a task to fake the callback from the providers. This meant there is an extra task for each notification than would be generated in a live situation.

This PR changes that, so that a research mode/test key notification calls the callback API rather than make a task to do that .

This ensures that the flow for research mode more closely mimics that of live, and removes a task from the process so we can more accurately test throughput,

app/celery/research_mode_tasks.py

@@ -15,7 +15,6 @@ perm_fail_email = "perm-fail@simulator.notify"
 temp_fail_email = "temp-fail@simulator.notify"
 
 
-@notify_celery.task(name="send-mmg-response")
 def send_sms_response(provider, reference, to):
     if provider == "mmg":
         body = mmg_callback(reference, to)
@@ -37,7 +36,6 @@ def send_sms_response(provider, reference, to):
     make_request(SMS_TYPE, provider, body, headers)
 
 
-@notify_celery.task(name="send-ses-response")
 def send_email_response(provider, reference, to):
     if to == perm_fail_email:
         body = ses_hard_bounce_callback(reference)

app/delivery/send_to_providers.py

@@ -33,11 +33,11 @@ def send_sms_to_provider(notification):
             prefix=service.name,
             sender=service.sms_sender
         )
+
         if service.research_mode or notification.key_type == KEY_TYPE_TEST:
-            send_sms_response.apply_async(
-                (provider.get_name(), str(notification.id), notification.to), queue='research-mode'
-            )
             notification.billable_units = 0
+            update_notification(notification, provider)
+            send_sms_response(provider.get_name(), str(notification.id), notification.to)
         else:
             try:
                 provider.send_sms(
@@ -51,11 +51,7 @@ def send_sms_to_provider(notification):
                 raise e
             else:
                 notification.billable_units = template.fragment_count
-
-        notification.sent_at = datetime.utcnow()
-        notification.sent_by = provider.get_name()
-        notification.status = 'sending'
-        dao_update_notification(notification)
+                update_notification(notification, provider)
 
         current_app.logger.info(
             "SMS {} sent to provider {} at {}".format(notification.id, provider.get_name(), notification.sent_at)
@@ -86,10 +82,10 @@ def send_email_to_provider(notification):
 
         if service.research_mode or notification.key_type == KEY_TYPE_TEST:
             reference = str(create_uuid())
-            send_email_response.apply_async(
-                (provider.get_name(), reference, notification.to), queue='research-mode'
-            )
             notification.billable_units = 0
+            notification.reference = reference
+            update_notification(notification, provider)
+            send_email_response(provider.get_name(), reference, notification.to)
         else:
             from_address = '"{}" <{}@{}>'.format(service.name, service.email_from,
                                                  current_app.config['NOTIFY_EMAIL_DOMAIN'])
@@ -101,12 +97,8 @@ def send_email_to_provider(notification):
                 html_body=str(html_email),
                 reply_to_address=service.reply_to_email_address,
             )
-
             notification.reference = reference
-        notification.sent_at = datetime.utcnow()
-        notification.sent_by = provider.get_name(),
-        notification.status = 'sending'
-        dao_update_notification(notification)
+            update_notification(notification, provider)
 
         current_app.logger.info(
             "Email {} sent to provider at {}".format(notification.id, notification.sent_at)
@@ -115,6 +107,13 @@ def send_email_to_provider(notification):
         statsd_client.timing("email.total-time", delta_milliseconds)
 
 
+def update_notification(notification, provider):
+    notification.sent_at = datetime.utcnow()
+    notification.sent_by = provider.get_name()
+    notification.status = 'sending'
+    dao_update_notification(notification)
+
+
 def provider_to_use(notification_type, notification_id):
     active_providers_in_order = [
         provider for provider in get_provider_details_by_notification_type(notification_type) if provider.active

tests/app/delivery/test_send_to_providers.py

@@ -165,7 +165,7 @@ def test_should_call_send_sms_response_task_if_research_mode(notify_db, sample_s
                                                              research_mode, key_type):
     mocker.patch('app.mmg_client.send_sms')
     mocker.patch('app.mmg_client.get_name', return_value="mmg")
-    mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async')
+    mocker.patch('app.delivery.send_to_providers.send_sms_response')
 
     if research_mode:
         sample_service.research_mode = True
@@ -178,8 +178,8 @@ def test_should_call_send_sms_response_task_if_research_mode(notify_db, sample_s
         sample_notification
     )
     assert not mmg_client.send_sms.called
-    send_to_providers.send_sms_response.apply_async.assert_called_once_with(
-        ('mmg', str(sample_notification.id), sample_notification.to), queue='research-mode'
+    app.delivery.send_to_providers.send_sms_response.assert_called_once_with(
+        'mmg', str(sample_notification.id), sample_notification.to
     )
 
     persisted_notification = notifications_dao.get_notification_by_id(sample_notification.id)
@@ -200,7 +200,7 @@ def test_should_set_billable_units_to_zero_in_research_mode_or_test_key(
 
     mocker.patch('app.mmg_client.send_sms')
     mocker.patch('app.mmg_client.get_name', return_value="mmg")
-    mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async')
+    mocker.patch('app.delivery.send_to_providers.send_sms_response')
     if research_mode:
         sample_service.research_mode = True
         notify_db.session.add(sample_service)
@@ -222,14 +222,14 @@ def test_should_not_send_to_provider_when_status_is_not_created(notify_db, notif
                                        status='sending')
     mocker.patch('app.mmg_client.send_sms')
     mocker.patch('app.mmg_client.get_name', return_value="mmg")
-    mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async')
+    mocker.patch('app.delivery.send_to_providers.send_sms_response')
 
     send_to_providers.send_sms_to_provider(
         notification
     )
 
     app.mmg_client.send_sms.assert_not_called()
-    app.celery.research_mode_tasks.send_sms_response.apply_async.assert_not_called()
+    app.delivery.send_to_providers.send_sms_response.assert_not_called()
 
 
 def test_should_send_sms_sender_from_service_if_present(
@@ -284,7 +284,7 @@ def test_send_email_to_provider_should_call_research_mode_task_response_task_if_
     mocker.patch('app.uuid.uuid4', return_value=reference)
     mocker.patch('app.aws_ses_client.send_email')
     mocker.patch('app.aws_ses_client.get_name', return_value="ses")
-    mocker.patch('app.celery.research_mode_tasks.send_email_response.apply_async')
+    mocker.patch('app.delivery.send_to_providers.send_email_response')
 
     if research_mode:
         sample_service.research_mode = True
@@ -295,9 +295,7 @@ def test_send_email_to_provider_should_call_research_mode_task_response_task_if_
         notification
     )
     assert not app.aws_ses_client.send_email.called
-    send_to_providers.send_email_response.apply_async.assert_called_once_with(
-        ('ses', str(reference), 'john@smith.com'), queue="research-mode"
-    )
+    app.delivery.send_to_providers.send_email_response.assert_called_once_with('ses', str(reference), 'john@smith.com')
     persisted_notification = Notification.query.filter_by(id=notification.id).one()
 
     assert persisted_notification.to == 'john@smith.com'
@@ -320,14 +318,14 @@ def test_send_email_to_provider_should_not_send_to_provider_when_status_is_not_c
                                        status='sending')
     mocker.patch('app.aws_ses_client.send_email')
     mocker.patch('app.aws_ses_client.get_name', return_value="ses")
-    mocker.patch('app.celery.research_mode_tasks.send_email_response.apply_async')
+    mocker.patch('app.delivery.send_to_providers.send_email_response')
 
     send_to_providers.send_sms_to_provider(
         notification
     )
 
     app.aws_ses_client.send_email.assert_not_called()
-    app.celery.research_mode_tasks.send_email_response.apply_async.assert_not_called()
+    app.delivery.send_to_providers.send_email_response.assert_not_called()
 
 
 def test_send_email_should_use_service_reply_to_email(
@@ -420,7 +418,7 @@ def test_get_logo_url_works_for_different_environments(base_url, expected_url):
 def test_should_not_set_billable_units_if_research_mode(notify_db, sample_service, sample_notification, mocker):
     mocker.patch('app.mmg_client.send_sms')
     mocker.patch('app.mmg_client.get_name', return_value="mmg")
-    mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async')
+    mocker.patch('app.delivery.send_to_providers.send_sms_response')
 
     sample_service.research_mode = True
     notify_db.session.add(sample_service)
@@ -454,7 +452,7 @@ def test_should_update_billable_units_according_to_research_mode_and_key_type(no
 
     mocker.patch('app.mmg_client.send_sms')
     mocker.patch('app.mmg_client.get_name', return_value="mmg")
-    mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async')
+    mocker.patch('app.delivery.send_to_providers.send_sms_response')
     if research_mode:
         sample_service.research_mode = True
         notify_db.session.add(sample_service)