Merge pull request #952 from alphagov/international-delivery-receipts

update delivery receipts for countries that return them

app/dao/notifications_dao.py

@@ -8,6 +8,7 @@ from flask import current_app
 from werkzeug.datastructures import MultiDict
 from sqlalchemy import (desc, func, or_, and_, asc)
 from sqlalchemy.orm import joinedload
+from notifications_utils.international_billing_rates import INTERNATIONAL_BILLING_RATES
 
 from app import db, create_uuid
 from app.dao import days_ago
@@ -163,6 +164,10 @@ def _decide_permanent_temporary_failure(current_status, status):
     return status
 
 
+def country_records_delivery(phone_prefix):
+    return INTERNATIONAL_BILLING_RATES[phone_prefix]['attributes']['dlr'].lower() == 'yes'
+
+
 def _update_notification_status(notification, status):
     status = _decide_permanent_temporary_failure(current_status=notification.status, status=status)
     notification.status = status
@@ -182,7 +187,10 @@ def update_notification_status_by_id(notification_id, status):
             Notification.status == NOTIFICATION_SENT
         )).first()
 
-    if not notification or notification.status == NOTIFICATION_SENT:
+    if not notification:
+        return None
+
+    if notification.international and not country_records_delivery(notification.phone_prefix):
         return None
 
     return _update_notification_status(

tests/app/dao/test_notification_dao.py

@@ -15,6 +15,7 @@ from app.models import (
     NOTIFICATION_STATUS_TYPES,
     NOTIFICATION_STATUS_TYPES_FAILED,
     NOTIFICATION_SENT,
+    NOTIFICATION_DELIVERED,
     KEY_TYPE_NORMAL,
     KEY_TYPE_TEAM,
     KEY_TYPE_TEST
@@ -354,28 +355,45 @@ def test_should_update_status_by_id_if_created(notify_db, notify_db_session):
     assert updated.status == 'failed'
 
 
-def test_should_not_update_status_by_reference_if_in_sent_status(notify_db, notify_db_session):
+def test_should_not_update_status_by_reference_if_from_country_with_no_delivery_receipts(sample_template):
-    notification = sample_notification(
+    notification = create_notification(
-        notify_db,
+        sample_template,
-        notify_db_session,
         status=NOTIFICATION_SENT,
         reference='foo'
     )
 
-    update_notification_status_by_reference('foo', 'failed')
+    res = update_notification_status_by_reference('foo', 'failed')
-    assert Notification.query.get(notification.id).status == NOTIFICATION_SENT
+
+    assert res is None
+    assert notification.status == NOTIFICATION_SENT
 
 
-def test_should_not_update_status_by_id_if_in_sent_status(notify_db, notify_db_session):
+def test_should_not_update_status_by_id_if_sent_to_country_with_no_delivery_receipts(sample_template):
-    notification = sample_notification(
+    notification = create_notification(
-        notify_db,
+        sample_template,
-        notify_db_session,
+        status=NOTIFICATION_SENT,
-        status=NOTIFICATION_SENT
+        international=True,
+        phone_prefix='1'  # americans only have carrier delivery receipts
     )
 
-    update_notification_status_by_id(notification.id, 'failed')
+    res = update_notification_status_by_id(notification.id, 'delivered')
 
-    assert Notification.query.get(notification.id).status == NOTIFICATION_SENT
+    assert res is None
+    assert notification.status == NOTIFICATION_SENT
+
+
+def test_should_not_update_status_by_id_if_sent_to_country_with_no_delivery_receipts(sample_template):
+    notification = create_notification(
+        sample_template,
+        status=NOTIFICATION_SENT,
+        international=True,
+        phone_prefix='7'  # russians have full delivery receipts
+    )
+
+    res = update_notification_status_by_id(notification.id, 'delivered')
+
+    assert res == notification
+    assert notification.status == NOTIFICATION_DELIVERED
 
 
 def test_should_not_update_status_by_reference_if_not_sending(notify_db, notify_db_session):