If the sms client provider sends more than one delivery receipt only take the update for the the first one.

Only update the notification.status if status = sending.

app/dao/notifications_dao.py

@@ -219,8 +219,9 @@ def dao_update_notification(notification):
 
 
 def update_notification_status_by_id(notification_id, status, notification_statistics_status):
-    count = db.session.query(Notification).filter_by(
+    count = db.session.query(Notification).filter(
-        id=notification_id
+        Notification.id == notification_id,
+        Notification.status == 'sending'
     ).update({
         Notification.status: status
     })

app/notifications/process_client_response.py

@@ -58,9 +58,10 @@ def process_sms_client_response(status, reference, client_name):
                                                                         notification_status,
                                                                         notification_statistics_status)
     if update_success == 0:
-        status_error = "{} callback failed: notification {} not found. Status {}".format(client_name,
+        status_error = "{} callback failed: notification {} either not found or already updated " \
-                                                                                         reference,
+                       "from sending. Status {}".format(client_name,
-                                                                                         notification_status_message)
+                                                        reference,
+                                                        notification_status_message)
         return success, status_error
 
     if not notification_success:

tests/app/dao/test_notification_dao.py

@@ -73,15 +73,18 @@ def test_should_by_able_to_update_status_by_id(sample_notification):
     count = update_notification_status_by_id(sample_notification.id, 'delivered', 'delivered')
     assert count == 1
     assert Notification.query.get(sample_notification.id).status == 'delivered'
-    assert NotificationStatistics.query.filter_by(
+    stats = NotificationStatistics.query.filter_by(service_id=sample_notification.service.id).one()
-        service_id=sample_notification.service.id
+    assert stats.sms_delivered == 1
-    ).one().sms_delivered == 1
+    assert stats.sms_requested == 1
-    assert NotificationStatistics.query.filter_by(
+    assert stats.sms_failed == 0
-        service_id=sample_notification.service.id
+
-    ).one().sms_requested == 1
+
-    assert NotificationStatistics.query.filter_by(
+def test_should_not_update_status_by_id_if_not_sending(notify_db, notify_db_session):
-        service_id=sample_notification.service.id
+    notification = sample_notification(notify_db, notify_db_session, status='delivered')
-    ).one().sms_failed == 0
+    assert Notification.query.get(notification.id).status == 'delivered'
+    count = update_notification_status_by_id(notification.id, 'failed', 'failure')
+    assert count == 0
+    assert Notification.query.get(notification.id).status == 'delivered'
 
 
 def test_should_not_update_status_one_notification_status_is_delivered(sample_email_template, ses_provider):
@@ -100,15 +103,10 @@ def test_should_not_update_status_one_notification_status_is_delivered(sample_em
 
     update_notification_status_by_reference('reference', 'delivered', 'temporary-failure')
     assert Notification.query.get(notification.id).status == 'delivered'
-    assert NotificationStatistics.query.filter_by(
+    stats = NotificationStatistics.query.filter_by(service_id=sample_email_template.service.id).one()
-        service_id=sample_email_template.service.id
+    assert stats.emails_delivered == 1
-    ).one().emails_delivered == 1
+    assert stats.emails_requested == 1
-    assert NotificationStatistics.query.filter_by(
+    assert stats.emails_failed == 0
-        service_id=sample_email_template.service.id
-    ).one().emails_requested == 1
-    assert NotificationStatistics.query.filter_by(
-        service_id=sample_email_template.service.id
-    ).one().emails_failed == 0
 
 
 def test_should_be_able_to_record_statistics_failure_for_sms(sample_notification):
@@ -116,15 +114,10 @@ def test_should_be_able_to_record_statistics_failure_for_sms(sample_notification
     count = update_notification_status_by_id(sample_notification.id, 'delivered', 'failure')
     assert count == 1
     assert Notification.query.get(sample_notification.id).status == 'delivered'
-    assert NotificationStatistics.query.filter_by(
+    stats = NotificationStatistics.query.filter_by(service_id=sample_notification.service.id).one()
-        service_id=sample_notification.service.id
+    assert stats.sms_delivered == 0
-    ).one().sms_delivered == 0
+    assert stats.sms_requested == 1
-    assert NotificationStatistics.query.filter_by(
+    assert stats.sms_failed == 1
-        service_id=sample_notification.service.id
-    ).one().sms_requested == 1
-    assert NotificationStatistics.query.filter_by(
-        service_id=sample_notification.service.id
-    ).one().sms_failed == 1
 
 
 def test_should_be_able_to_record_statistics_failure_for_email(sample_email_template, ses_provider):
@@ -137,15 +130,10 @@ def test_should_be_able_to_record_statistics_failure_for_email(sample_email_temp
     count = update_notification_status_by_reference('reference', 'failed', 'failure')
     assert count == 1
     assert Notification.query.get(notification.id).status == 'failed'
-    assert NotificationStatistics.query.filter_by(
+    stats = NotificationStatistics.query.filter_by(service_id=notification.service.id).one()
-        service_id=notification.service.id
+    assert stats.emails_delivered == 0
-    ).one().emails_delivered == 0
+    assert stats.emails_requested == 1
-    assert NotificationStatistics.query.filter_by(
+    assert stats.emails_failed == 1
-        service_id=notification.service.id
-    ).one().emails_requested == 1
-    assert NotificationStatistics.query.filter_by(
-        service_id=notification.service.id
-    ).one().emails_failed == 1
 
 
 def test_should_return_zero_count_if_no_notification_with_id():

tests/app/notifications/test_rest.py

@@ -1158,7 +1158,8 @@ def test_firetext_callback_should_return_404_if_cannot_find_notification_id(noti
             json_resp = json.loads(response.get_data(as_text=True))
             assert response.status_code == 400
             assert json_resp['result'] == 'error'
-            assert json_resp['message'] == 'Firetext callback failed: notification {} not found. Status {}'.format(
+            assert json_resp['message'] == 'Firetext callback failed: notification {} either not found ' \
+                                           'or already updated from sending. Status {}'.format(
                 missing_notification_id,
                 'Delivered'
             )
@@ -1221,9 +1222,9 @@ def test_firetext_callback_should_update_notification_status_failed(notify_api,
 def test_firetext_callback_should_update_notification_status_sent(notify_api, notify_db, notify_db_session):
     with notify_api.test_request_context():
         with notify_api.test_client() as client:
-            notification = create_sample_notification(notify_db, notify_db_session, status='delivered')
+            notification = create_sample_notification(notify_db, notify_db_session, status='sending')
             original = get_notification_by_id(notification.id)
-            assert original.status == 'delivered'
+            assert original.status == 'sending'
 
             response = client.post(
                 path='/notifications/sms/firetext',
@@ -1248,9 +1249,9 @@ def test_firetext_callback_should_update_notification_status_sent(notify_api, no
 def test_firetext_callback_should_update_multiple_notification_status_sent(notify_api, notify_db, notify_db_session):
     with notify_api.test_request_context():
         with notify_api.test_client() as client:
-            notification1 = create_sample_notification(notify_db, notify_db_session, status='delivered')
+            notification1 = create_sample_notification(notify_db, notify_db_session, status='sending')
-            notification2 = create_sample_notification(notify_db, notify_db_session, status='delivered')
+            notification2 = create_sample_notification(notify_db, notify_db_session, status='sending')
-            notification3 = create_sample_notification(notify_db, notify_db_session, status='delivered')
+            notification3 = create_sample_notification(notify_db, notify_db_session, status='sending')
 
             client.post(
                 path='/notifications/sms/firetext',
@@ -1330,8 +1331,8 @@ def test_process_mmg_response_status_5_updates_notification_with_permanently_fai
         assert get_notification_by_id(sample_notification.id).status == 'permanent-failure'
 
 
-def test_process_mmg_response_status_2_or_4_updates_notification_with_temporary_failed(notify_api,
+def test_process_mmg_response_status_2_updates_notification_with_temporary_failed(notify_api,
-                                                                                       sample_notification):
+                                                                                  sample_notification):
     with notify_api.test_client() as client:
         data = json.dumps({"reference": "mmg_reference",
                            "CID": str(sample_notification.id),
@@ -1347,6 +1348,10 @@ def test_process_mmg_response_status_2_or_4_updates_notification_with_temporary_
         assert json_data['message'] == 'MMG callback succeeded. reference {} updated'.format(sample_notification.id)
         assert get_notification_by_id(sample_notification.id).status == 'temporary-failure'
 
+
+def test_process_mmg_response_status_4_updates_notification_with_temporary_failed(notify_api,
+                                                                                  sample_notification):
+    with notify_api.test_client() as client:
         data = json.dumps({"reference": "mmg_reference",
                            "CID": str(sample_notification.id),
                            "MSISDN": "447777349060",
@@ -1510,21 +1515,21 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
                 notify_db_session,
                 template=sample_email_template,
                 reference='ref1',
-                status='delivered')
+                status='sending')
 
             notification2 = create_sample_notification(
                 notify_db,
                 notify_db_session,
                 template=sample_email_template,
                 reference='ref2',
-                status='delivered')
+                status='sending')
 
             notification3 = create_sample_notification(
                 notify_db,
                 notify_db_session,
                 template=sample_email_template,
                 reference='ref2',
-                status='delivered')
+                status='sending')
 
             client.post(
                 path='/notifications/sms/firetext',
@@ -1766,7 +1771,7 @@ def test_firetext_callback_should_record_statsd(notify_api, notify_db, notify_db
     with notify_api.test_request_context():
         with notify_api.test_client() as client:
             mocker.patch('app.statsd_client.incr')
-            notification = create_sample_notification(notify_db, notify_db_session, status='delivered')
+            notification = create_sample_notification(notify_db, notify_db_session, status='sending')
 
             client.post(
                 path='/notifications/sms/firetext',