Merge pull request #2520 from alphagov/fix-bug

Fix a bug introduced when refactoring some code.

app/delivery/send_to_providers.py

@@ -51,8 +51,8 @@ def send_sms_to_provider(notification):
         )
 
         if service.research_mode or notification.key_type == KEY_TYPE_TEST:
-            send_sms_response(provider.get_name(), str(notification.id), notification.to)
             update_notification_to_sending(notification, provider)
+            send_sms_response(provider.get_name(), str(notification.id), notification.to)
 
         else:
             try:

tests/app/delivery/test_send_to_providers.py

@@ -229,7 +229,7 @@ def test_should_call_send_sms_response_task_if_research_mode(
     assert not persisted_notification.personalisation
 
 
-def test_should_leave_as_created_if_fake_callback_function_fails(sample_notification, mocker):
+def test_should_have_sending_status_if_fake_callback_function_fails(sample_notification, mocker):
     mocker.patch('app.delivery.send_to_providers.send_sms_response', side_effect=HTTPError)
 
     sample_notification.key_type = KEY_TYPE_TEST
@@ -238,9 +238,8 @@ def test_should_leave_as_created_if_fake_callback_function_fails(sample_notifica
         send_to_providers.send_sms_to_provider(
             sample_notification
         )
-    assert sample_notification.status == 'created'
+    assert sample_notification.status == 'sending'
-    assert sample_notification.sent_at is None
+    assert sample_notification.sent_by == 'mmg'
-    assert sample_notification.sent_by is None
 
 
 def test_should_not_send_to_provider_when_status_is_not_created(
@@ -507,27 +506,34 @@ def test_should_not_update_notification_if_research_mode_on_exception(
 
     persisted_notification = notifications_dao.get_notification_by_id(sample_notification.id)
     assert persisted_notification.billable_units == 0
-    assert not update_mock.called
+    assert update_mock.called
 
 
-@pytest.mark.parametrize('research_mode,key_type, billable_units', [
+def __update_notification(notification_to_update, research_mode, expected_status):
-    (True, KEY_TYPE_NORMAL, 0),
+    if research_mode or notification_to_update.key_type == KEY_TYPE_TEST:
-    (False, KEY_TYPE_NORMAL, 1),
+        notification_to_update.status = expected_status
-    (False, KEY_TYPE_TEST, 0),
+
-    (True, KEY_TYPE_TEST, 0),
+
-    (True, KEY_TYPE_TEAM, 0),
+@pytest.mark.parametrize('research_mode,key_type, billable_units, expected_status', [
-    (False, KEY_TYPE_TEAM, 1)
+    (True, KEY_TYPE_NORMAL, 0, 'delivered'),
+    (False, KEY_TYPE_NORMAL, 1, 'sending'),
+    (False, KEY_TYPE_TEST, 0, 'sending'),
+    (True, KEY_TYPE_TEST, 0, 'sending'),
+    (True, KEY_TYPE_TEAM, 0, 'delivered'),
+    (False, KEY_TYPE_TEAM, 1, 'sending')
 ])
-def test_should_update_billable_units_according_to_research_mode_and_key_type(
+def test_should_update_billable_units_and_status_according_to_research_mode_and_key_type(
     sample_template,
     mocker,
     research_mode,
     key_type,
-    billable_units
+    billable_units,
+    expected_status
 ):
     notification = create_notification(template=sample_template, billable_units=0, status='created', key_type=key_type)
     mocker.patch('app.mmg_client.send_sms')
-    mocker.patch('app.delivery.send_to_providers.send_sms_response')
+    mocker.patch('app.delivery.send_to_providers.send_sms_response',
+                 side_effect=__update_notification(notification, research_mode, expected_status))
 
     if research_mode:
         sample_template.service.research_mode = True
@@ -536,6 +542,7 @@ def test_should_update_billable_units_according_to_research_mode_and_key_type(
         notification
     )
     assert notification.billable_units == billable_units
+    assert notification.status == expected_status
 
 
 def test_should_set_notification_billable_units_if_sending_to_provider_fails(