From 57a0d7295d4fa61cebe396e7c65e0e56dff7365c Mon Sep 17 00:00:00 2001
From: Paul Craig <paul_craig_16@hotmail.com>
Date: Fri, 25 Nov 2016 14:55:29 +0000
Subject: [PATCH] Rewrite `failed` statuses

There are no more notifications whose statuses are "failed", as
the "failed" status has now been replaced with statuses that are
more specific about the nature of the failure.

However, we still want to be able to filter by failing
notifications. (ie "/v2/notifications?status=failed").

Created a `.substitute_status()` method which takes a status
string or list of status strings and, if it finds 'failure',
replaces it with the other failing status types.

This way, calling for nottifications with "?status=failed" is
internally treated as
"status = ['technical-failure', 'temporary-failure', 'permanent-failure']"
---
 app/models.py           | 50 +++++++++++++++++++++++++++++++++++++++--
 tests/app/test_model.py | 35 ++++++++++++++++++++++++++++-
 2 files changed, 82 insertions(+), 3 deletions(-)

diff --git a/app/models.py b/app/models.py
index 456fc5122..c24ce7b18 100644
--- a/app/models.py
+++ b/app/models.py
@@ -466,6 +466,12 @@ NOTIFICATION_TECHNICAL_FAILURE = 'technical-failure'
 NOTIFICATION_TEMPORARY_FAILURE = 'temporary-failure'
 NOTIFICATION_PERMANENT_FAILURE = 'permanent-failure'
 
+NOTIFICATION_STATUS_TYPES_FAILED = [
+    NOTIFICATION_TECHNICAL_FAILURE,
+    NOTIFICATION_TEMPORARY_FAILURE,
+    NOTIFICATION_PERMANENT_FAILURE,
+]
+
 NOTIFICATION_STATUS_TYPES_COMPLETED = [
     NOTIFICATION_DELIVERED,
     NOTIFICATION_FAILED,
@@ -480,7 +486,7 @@ NOTIFICATION_STATUS_TYPES_BILLABLE = [
     NOTIFICATION_FAILED,
     NOTIFICATION_TECHNICAL_FAILURE,
     NOTIFICATION_TEMPORARY_FAILURE,
-    NOTIFICATION_PERMANENT_FAILURE
+    NOTIFICATION_PERMANENT_FAILURE,
 ]
 
 NOTIFICATION_STATUS_TYPES = [
@@ -491,7 +497,7 @@ NOTIFICATION_STATUS_TYPES = [
     NOTIFICATION_FAILED,
     NOTIFICATION_TECHNICAL_FAILURE,
     NOTIFICATION_TEMPORARY_FAILURE,
-    NOTIFICATION_PERMANENT_FAILURE
+    NOTIFICATION_PERMANENT_FAILURE,
 ]
 NOTIFICATION_STATUS_TYPES_ENUM = db.Enum(*NOTIFICATION_STATUS_TYPES, name='notify_status_type')
 
@@ -558,6 +564,46 @@ class Notification(db.Model):
 
         return None
 
+    @staticmethod
+    def substitute_status(status_or_statuses):
+        """
+        static function that takes a status or list of statuses and substitutes our new failure types if it finds
+        the deprecated one
+
+        > IN
+        'failed'
+
+        < OUT
+        ['technical-failure', 'temporary-failure', 'permanent-failure']
+
+        -
+
+        > IN
+        ['failed', 'created']
+
+        < OUT
+        ['technical-failure', 'temporary-failure', 'permanent-failure', 'created']
+
+
+        :param status_or_statuses: a single status or list of statuses
+        :return: a single status or list with the current failure statuses substituted for 'failure'
+        """
+
+        def _substitute_status_str(_status):
+            return NOTIFICATION_STATUS_TYPES_FAILED if _status == NOTIFICATION_FAILED else _status
+
+        def _substitute_status_seq(_statuses):
+            if NOTIFICATION_FAILED in _statuses:
+                _statuses = list(set(
+                    NOTIFICATION_STATUS_TYPES_FAILED + [_s for _s in _statuses if _s != NOTIFICATION_FAILED]
+                ))
+            return _statuses
+
+        if isinstance(status_or_statuses, str):
+            return _substitute_status_str(status_or_statuses)
+
+        return _substitute_status_seq(status_or_statuses)
+
     def serialize(self):
 
         template_dict = {
diff --git a/tests/app/test_model.py b/tests/app/test_model.py
index ca6d11f9a..21b3df4cc 100644
--- a/tests/app/test_model.py
+++ b/tests/app/test_model.py
@@ -2,7 +2,15 @@ import pytest
 
 from app.models import (
     ServiceWhitelist,
-    MOBILE_TYPE, EMAIL_TYPE)
+    Notification,
+    MOBILE_TYPE,
+    EMAIL_TYPE,
+    NOTIFICATION_CREATED,
+    NOTIFICATION_PENDING,
+    NOTIFICATION_FAILED,
+    NOTIFICATION_TECHNICAL_FAILURE,
+    NOTIFICATION_STATUS_TYPES_FAILED
+)
 
 
 @pytest.mark.parametrize('mobile_number', [
@@ -32,3 +40,28 @@ def test_should_build_service_whitelist_from_email_address(email_address):
 def test_should_not_build_service_whitelist_from_invalid_contact(recipient_type, contact):
     with pytest.raises(ValueError):
         ServiceWhitelist.from_string('service_id', recipient_type, contact)
+
+
+@pytest.mark.parametrize('initial_statuses, expected_statuses', [
+    # passing in single statuses as strings
+    (NOTIFICATION_FAILED, NOTIFICATION_STATUS_TYPES_FAILED),
+    (NOTIFICATION_CREATED, NOTIFICATION_CREATED),
+    (NOTIFICATION_TECHNICAL_FAILURE, NOTIFICATION_TECHNICAL_FAILURE),
+    # passing in lists containing single statuses
+    ([NOTIFICATION_FAILED], NOTIFICATION_STATUS_TYPES_FAILED),
+    ([NOTIFICATION_CREATED], [NOTIFICATION_CREATED]),
+    ([NOTIFICATION_TECHNICAL_FAILURE], [NOTIFICATION_TECHNICAL_FAILURE]),
+    # passing in lists containing multiple statuses
+    ([NOTIFICATION_FAILED, NOTIFICATION_CREATED], NOTIFICATION_STATUS_TYPES_FAILED + [NOTIFICATION_CREATED]),
+    ([NOTIFICATION_CREATED, NOTIFICATION_PENDING], [NOTIFICATION_CREATED, NOTIFICATION_PENDING]),
+    ([NOTIFICATION_CREATED, NOTIFICATION_TECHNICAL_FAILURE], [NOTIFICATION_CREATED, NOTIFICATION_TECHNICAL_FAILURE]),
+    # checking we don't end up with duplicates
+    (
+        [NOTIFICATION_FAILED, NOTIFICATION_CREATED, NOTIFICATION_TECHNICAL_FAILURE],
+        NOTIFICATION_STATUS_TYPES_FAILED + [NOTIFICATION_CREATED]
+    ),
+])
+def test_status_conversion_handles_failed_statuses(initial_statuses, expected_statuses):
+    converted_statuses = Notification.substitute_status(initial_statuses)
+    assert len(converted_statuses) == len(expected_statuses)
+    assert set(converted_statuses) == set(expected_statuses)