Merge branch 'master' of github.com:alphagov/notifications-api

app/delivery/send_to_providers.py

@@ -28,7 +28,8 @@ from app.models import (
     EMAIL_TYPE,
     NOTIFICATION_TECHNICAL_FAILURE,
     NOTIFICATION_SENT,
-    NOTIFICATION_SENDING
+    NOTIFICATION_SENDING,
+    NOTIFICATION_STATUS_TYPES_COMPLETED
 )
 
 
@@ -125,7 +126,8 @@ def send_email_to_provider(notification):
 def update_notification_to_sending(notification, provider):
     notification.sent_at = datetime.utcnow()
     notification.sent_by = provider.get_name()
-    notification.status = NOTIFICATION_SENT if notification.international else NOTIFICATION_SENDING
+    if notification.status not in NOTIFICATION_STATUS_TYPES_COMPLETED:
+        notification.status = NOTIFICATION_SENT if notification.international else NOTIFICATION_SENDING
     dao_update_notification(notification)
 
 

app/serialised_models.py

@@ -1,10 +1,13 @@
-from abc import ABC, abstractmethod
 from collections import defaultdict
 from functools import partial
 from threading import RLock
 
 import cachetools
 from notifications_utils.clients.redis import RequestCache
+from notifications_utils.serialised_model import (
+    SerialisedModel,
+    SerialisedModelCollection,
+)
 from werkzeug.utils import cached_property
 
 from app import db, redis_store
@@ -33,52 +36,6 @@ def ignore_first_argument_cache_key(cls, *args, **kwargs):
     return cachetools.keys.hashkey(*args, **kwargs)
 
 
-class SerialisedModel(ABC):
-
-    """
-    A SerialisedModel takes a dictionary, typically created by
-    serialising a database object. It then takes the value of specified
-    keys from the dictionary and adds them to itself as properties, so
-    that it can be interacted with like a normal database model object,
-    but with no risk that it will actually go back to the database.
-    """
-
-    @property
-    @abstractmethod
-    def ALLOWED_PROPERTIES(self):
-        pass
-
-    def __init__(self, _dict):
-        for property in self.ALLOWED_PROPERTIES:
-            setattr(self, property, _dict[property])
-
-    def __dir__(self):
-        return super().__dir__() + list(sorted(self.ALLOWED_PROPERTIES))
-
-
-class SerialisedModelCollection(ABC):
-
-    """
-    A SerialisedModelCollection takes a list of dictionaries, typically
-    created by serialising database objects. When iterated over it
-    returns a SerialisedModel instance for each of the items in the list.
-    """
-
-    @property
-    @abstractmethod
-    def model(self):
-        pass
-
-    def __init__(self, items):
-        self.items = items
-
-    def __bool__(self):
-        return bool(self.items)
-
-    def __getitem__(self, index):
-        return self.model(self.items[index])
-
-
 class SerialisedTemplate(SerialisedModel):
     ALLOWED_PROPERTIES = {
         'archived',

requirements-app.txt

@@ -27,7 +27,7 @@ notifications-python-client==5.5.1
 # PaaS
 awscli-cwlogs>=1.4,<1.5
 
-git+https://github.com/alphagov/notifications-utils.git@40.0.0#egg=notifications-utils==40.0.0
+git+https://github.com/alphagov/notifications-utils.git@40.2.1#egg=notifications-utils==40.2.1
 
 # gds-metrics requires prometheseus 0.2.0, override that requirement as 0.7.1 brings significant performance gains
 prometheus-client==0.7.1

requirements.txt

@@ -29,7 +29,7 @@ notifications-python-client==5.5.1
 # PaaS
 awscli-cwlogs>=1.4,<1.5
 
-git+https://github.com/alphagov/notifications-utils.git@40.0.0#egg=notifications-utils==40.0.0
+git+https://github.com/alphagov/notifications-utils.git@40.2.1#egg=notifications-utils==40.2.1
 
 # gds-metrics requires prometheseus 0.2.0, override that requirement as 0.7.1 brings significant performance gains
 prometheus-client==0.7.1
@@ -40,14 +40,14 @@ alembic==1.4.2
 amqp==1.4.9
 anyjson==0.3.3
 attrs==19.3.0
-awscli==1.18.89
+awscli==1.18.93
 bcrypt==3.1.7
 billiard==3.3.0.23
 bleach==3.1.4
 blinker==1.4
 boto==2.49.0
 boto3==1.10.38
-botocore==1.17.12
+botocore==1.17.16
 certifi==2020.6.20
 chardet==3.0.4
 click==7.1.2

tests/app/delivery/test_send_to_providers.py

@@ -9,7 +9,7 @@ from notifications_utils.recipients import validate_and_format_phone_number
 from requests import HTTPError
 
 import app
-from app import mmg_client, firetext_client
+from app import clients, mmg_client, firetext_client
 from app.dao import notifications_dao
 from app.dao.provider_details_dao import get_provider_details_by_identifier
 from app.delivery import send_to_providers
@@ -525,6 +525,20 @@ def test_should_not_update_notification_if_research_mode_on_exception(
     assert update_mock.called
 
 
+@pytest.mark.parametrize("starting_status, expected_status", [
+    ("delivered", "delivered"),
+    ("created", "sending"),
+    ("technical-failure", "technical-failure"),
+])
+def test_update_notification_to_sending_does_not_update_status_from_a_final_status(
+    sample_service, notify_db_session, starting_status, expected_status
+):
+    template = create_template(sample_service)
+    notification = create_notification(template=template, status=starting_status)
+    send_to_providers.update_notification_to_sending(notification, clients.get_client_by_name_and_type("mmg", "sms"))
+    assert notification.status == expected_status
+
+
 def __update_notification(notification_to_update, research_mode, expected_status):
     if research_mode or notification_to_update.key_type == KEY_TYPE_TEST:
         notification_to_update.status = expected_status