Log detailed sms delivery status for mmg from process_sms_client_response task.

Also log detailed delivery status for firetext in the same place in addition
to it being logged from notifications_dao.

Logging detailed delivery statuses will help us see why messages
fail to deliver. In the future we could persist detailed delivery
status in the database.

tests/app/clients/test_firetext.py

@@ -8,19 +8,25 @@ import requests_mock
 from app.clients.sms.firetext import get_firetext_responses, SmsClientResponseException, FiretextClientResponseException
 
 
-def test_should_return_correct_details_for_delivery():
-    get_firetext_responses('0') == 'delivered'
+@pytest.mark.parametrize('code, result', [
+    (None, ('delivered', None)), ('000', ('delivered', None))
+])
+def test_get_firetext_responses_should_return_correct_details_for_delivery(code, result):
+    assert get_firetext_responses('0', code) == result
 
 
-def test_should_return_correct_details_for_bounced():
-    get_firetext_responses('1') == 'permanent-failure'
+@pytest.mark.parametrize('code, result', [
+    (None, ('permanent-failure', None)), ('401', ('permanent-failure', 'Message Rejected'))
+])
+def test_get_firetext_responses_should_return_correct_details_for_bounced(code, result):
+    assert get_firetext_responses('1', code) == result
 
 
-def test_should_return_correct_details_for_complaint():
-    get_firetext_responses('2') == 'pending'
+def test_get_firetext_responses_should_return_correct_details_for_complaint():
+    assert get_firetext_responses('2') == ('pending', None)
 
 
-def test_should_be_none_if_unrecognised_status_code():
+def test_get_firetext_responses_should_be_none_if_unrecognised_status_code():
     with pytest.raises(KeyError) as e:
         get_firetext_responses('99')
     assert '99' in str(e.value)

tests/app/clients/test_mmg.py

@@ -9,20 +9,31 @@ from app.clients.sms import SmsClientResponseException
 from app.clients.sms.mmg import get_mmg_responses, MMGClientResponseException
 
 
-def test_should_return_correct_details_for_delivery():
-    get_mmg_responses('3') == 'delivered'
+@pytest.mark.parametrize('substatus, result', [
+    (None, ('delivered', None)), ('5', ('delivered', 'Delivered to handset'))
+])
+def test_get_mmg_responses_should_return_correct_details_for_delivery(substatus, result):
+    assert get_mmg_responses('3', substatus) == result
 
 
-def test_should_return_correct_details_for_temporary_failure():
-    get_mmg_responses('4') == 'temporary-failure'
+@pytest.mark.parametrize('substatus, result', [
+    (None, ('temporary-failure', None)), ('15', ('temporary-failure', 'Expired'))
+])
+def test_get_mmg_responses_should_return_correct_details_for_temporary_failure(substatus, result):
+    assert get_mmg_responses('4', substatus) == result
 
 
-@pytest.mark.parametrize('status', ['5', '2'])
-def test_should_return_correct_details_for_bounced(status):
-    get_mmg_responses(status) == 'permanent-failure'
+@pytest.mark.parametrize('status, substatus, result', [
+    ('2', None, ('permanent-failure', None)),
+    ('2', '12', ('permanent-failure', "Illegal equipment")),
+    ('5', None, ('permanent-failure', None)),
+    ('5', '20', ('permanent-failure', 'Rejected by anti-flooding mechanism'))
+])
+def test_get_mmg_responses_should_return_correct_details_for_bounced(status, substatus, result):
+    assert get_mmg_responses(status, substatus) == result
 
 
-def test_should_be_raise_if_unrecognised_status_code():
+def test_get_mmg_responses_should_be_raise_if_unrecognised_status_code():
     with pytest.raises(KeyError) as e:
         get_mmg_responses('99')
     assert '99' in str(e.value)

tests/app/notifications/rest/test_callbacks.py

@@ -203,6 +203,7 @@ def test_mmg_callback_should_return_200_and_call_task_with_valid_data(client, mo
                        "CID": "notification_id",
                        "MSISDN": "447777349060",
                        "status": "3",
+                       "substatus": "5",
                        "deliverytime": "2016-04-05 16:01:07"})
 
     response = mmg_post(client, data)
@@ -212,7 +213,7 @@ def test_mmg_callback_should_return_200_and_call_task_with_valid_data(client, mo
     assert json_data['result'] == 'success'
 
     mock_celery.assert_called_once_with(
-        ['3', 'notification_id', 'MMG'],
+        ['3', 'notification_id', 'MMG', '5'],
         queue='sms-callbacks',
     )
 

tests/app/notifications/test_process_client_response.py

@@ -35,25 +35,31 @@ def test_process_sms_response_raises_client_exception_for_unknown_status(
     assert sample_notification.status == NOTIFICATION_TECHNICAL_FAILURE
 
 
-@pytest.mark.parametrize('status, sms_provider, expected_notification_status', [
-    ('0', 'Firetext', 'delivered'),
-    ('1', 'Firetext', 'permanent-failure'),
-    ('2', 'Firetext', 'pending'),
-    ('2', 'MMG', 'permanent-failure'),
-    ('3', 'MMG', 'delivered'),
-    ('4', 'MMG', 'temporary-failure'),
-    ('5', 'MMG', 'permanent-failure'),
+@pytest.mark.parametrize('status, substatus, sms_provider, expected_notification_status, reason', [
+    ('0', None, 'Firetext', 'delivered', None),
+    ('1', '101', 'Firetext', 'permanent-failure', 'Unknown Subscriber'),
+    ('2', '102', 'Firetext', 'pending', 'Absent Subscriber'),
+    ('2', '1', 'MMG', 'permanent-failure', "Number does not exist"),
+    ('3', '2', 'MMG', 'delivered', "Delivered to operator"),
+    ('4', '27', 'MMG', 'temporary-failure', "Absent Subscriber"),
+    ('5', '13', 'MMG', 'permanent-failure', "Sender id blacklisted"),
 ])
 def test_process_sms_client_response_updates_notification_status(
     sample_notification,
     mocker,
     status,
+    substatus,
     sms_provider,
     expected_notification_status,
+    reason
 ):
+    mock_logger = mocker.patch('app.celery.tasks.current_app.logger.info')
     sample_notification.status = 'sending'
-    process_sms_client_response(status, str(sample_notification.id), sms_provider)
 
+    process_sms_client_response(status, str(sample_notification.id), sms_provider, substatus)
+
+    message = f"{sms_provider} callback returned status of {expected_notification_status}: {reason} for reference: {sample_notification.id}"  # noqa
+    mock_logger.assert_any_call(message)
     assert sample_notification.status == expected_notification_status