remove sandbox

we haven't used this in five years. we set up sandbox once but never
actually had any concrete uses for spinning up an entire extra infra
setup outside the main deploy pipeline. none of these buckets exist and
no doubt the AWS infra is significantly out of date too.

README.md

@@ -11,9 +11,9 @@ Contains:
 
 We run python 3.9 both locally and in production.
 
-### pycurl
+### psycopg2
 
-See https://github.com/alphagov/notifications-manuals/wiki/Getting-started#pycurl
+[Follow these instructions on Mac M1 machines](https://github.com/psycopg/psycopg2/issues/1216#issuecomment-1068150544).
 
 ### AWS credentials
 
@@ -29,6 +29,7 @@ export NOTIFY_ENVIRONMENT='development'
 
 export MMG_API_KEY='MMG_API_KEY'
 export FIRETEXT_API_KEY='FIRETEXT_ACTUAL_KEY'
+export REACH_API_KEY='REACH_API_KEY'
 export NOTIFICATION_QUEUE_PREFIX='YOUR_OWN_PREFIX'
 
 export FLASK_APP=application.py
@@ -45,6 +46,7 @@ Things to change:
 ```
 notify-pass credentials/firetext
 notify-pass credentials/mmg
+notify-pass credentials/reach
 ```
 
 ### Postgres

app/__init__.py

@@ -36,6 +36,7 @@ from app.clients.email.aws_ses import AwsSesClient
 from app.clients.email.aws_ses_stub import AwsSesStubClient
 from app.clients.sms.firetext import FiretextClient
 from app.clients.sms.mmg import MMGClient
+from app.clients.sms.reach import ReachClient
 
 
 class SQLAlchemy(_SQLAlchemy):
@@ -56,6 +57,7 @@ ma = Marshmallow()
 notify_celery = NotifyCelery()
 firetext_client = FiretextClient()
 mmg_client = MMGClient()
+reach_client = ReachClient()
 aws_ses_client = AwsSesClient()
 aws_ses_stub_client = AwsSesStubClient()
 encryption = Encryption()
@@ -98,6 +100,7 @@ def create_app(application):
     logging.init_app(application, statsd_client)
     firetext_client.init_app(application, statsd_client=statsd_client)
     mmg_client.init_app(application, statsd_client=statsd_client)
+    reach_client.init_app(application, statsd_client=statsd_client)
 
     aws_ses_client.init_app(application.config['AWS_REGION'], statsd_client=statsd_client)
     aws_ses_stub_client.init_app(
@@ -107,7 +110,10 @@ def create_app(application):
     )
     # If a stub url is provided for SES, then use the stub client rather than the real SES boto client
     email_clients = [aws_ses_stub_client] if application.config['SES_STUB_URL'] else [aws_ses_client]
-    notification_provider_clients.init_app(sms_clients=[firetext_client, mmg_client], email_clients=email_clients)
+    notification_provider_clients.init_app(
+        sms_clients=[firetext_client, mmg_client, reach_client],
+        email_clients=email_clients
+    )
 
     notify_celery.init_app(application)
     encryption.init_app(application)
@@ -277,48 +283,34 @@ def register_blueprint(application):
 
 def register_v2_blueprints(application):
     from app.authentication.auth import requires_auth
-    from app.v2.broadcast.post_broadcast import (
-        v2_broadcast_blueprint as post_broadcast,
+    from app.v2.broadcast.post_broadcast import v2_broadcast_blueprint
+    from app.v2.inbound_sms.get_inbound_sms import v2_inbound_sms_blueprint
+    from app.v2.notifications import (  # noqa
+        get_notifications,
+        post_notifications,
+        v2_notification_blueprint,
     )
-    from app.v2.inbound_sms.get_inbound_sms import (
-        v2_inbound_sms_blueprint as get_inbound_sms,
-    )
-    from app.v2.notifications.get_notifications import (
-        v2_notification_blueprint as get_notifications,
-    )
-    from app.v2.notifications.post_notifications import (
-        v2_notification_blueprint as post_notifications,
-    )
-    from app.v2.template.get_template import (
-        v2_template_blueprint as get_template,
-    )
-    from app.v2.template.post_template import (
-        v2_template_blueprint as post_template,
-    )
-    from app.v2.templates.get_templates import (
-        v2_templates_blueprint as get_templates,
+    from app.v2.template import (  # noqa
+        get_template,
+        post_template,
+        v2_template_blueprint,
     )
+    from app.v2.templates.get_templates import v2_templates_blueprint
 
-    post_notifications.before_request(requires_auth)
-    application.register_blueprint(post_notifications)
+    v2_notification_blueprint.before_request(requires_auth)
+    application.register_blueprint(v2_notification_blueprint)
 
-    get_notifications.before_request(requires_auth)
-    application.register_blueprint(get_notifications)
+    v2_templates_blueprint.before_request(requires_auth)
+    application.register_blueprint(v2_templates_blueprint)
 
-    get_templates.before_request(requires_auth)
-    application.register_blueprint(get_templates)
+    v2_template_blueprint.before_request(requires_auth)
+    application.register_blueprint(v2_template_blueprint)
 
-    get_template.before_request(requires_auth)
-    application.register_blueprint(get_template)
+    v2_inbound_sms_blueprint.before_request(requires_auth)
+    application.register_blueprint(v2_inbound_sms_blueprint)
 
-    post_template.before_request(requires_auth)
-    application.register_blueprint(post_template)
-
-    get_inbound_sms.before_request(requires_auth)
-    application.register_blueprint(get_inbound_sms)
-
-    post_broadcast.before_request(requires_auth)
-    application.register_blueprint(post_broadcast)
+    v2_broadcast_blueprint.before_request(requires_auth)
+    application.register_blueprint(v2_broadcast_blueprint)
 
 
 def init_app(app):

app/broadcast_message/rest.py

@@ -2,14 +2,12 @@ import iso8601
 from flask import Blueprint, jsonify, request
 from notifications_utils.template import BroadcastMessageTemplate
 
+from app.broadcast_message import utils as broadcast_utils
 from app.broadcast_message.broadcast_message_schema import (
     create_broadcast_message_schema,
     update_broadcast_message_schema,
     update_broadcast_message_status_schema,
 )
-from app.broadcast_message.utils import (
-    validate_and_update_broadcast_message_status,
-)
 from app.dao.broadcast_message_dao import (
     dao_get_broadcast_message_by_id_and_service_id,
     dao_get_broadcast_messages_for_service,
@@ -162,6 +160,6 @@ def update_broadcast_message_status(service_id, broadcast_message_id):
                 status_code=400
             )
 
-    validate_and_update_broadcast_message_status(broadcast_message, new_status, updating_user)
+    broadcast_utils.update_broadcast_message_status(broadcast_message, new_status, updating_user)
 
     return jsonify(broadcast_message.serialize()), 200

app/broadcast_message/utils.py

@@ -1,7 +1,12 @@
+import inspect
 from datetime import datetime
 
 from flask import current_app
+from notifications_utils.clients.zendesk.zendesk_client import (
+    NotifySupportTicket,
+)
 
+from app import zendesk_client
 from app.celery.broadcast_message_tasks import send_broadcast_event
 from app.config import QueueNames
 from app.dao.dao_utils import dao_save_object
@@ -13,7 +18,31 @@ from app.models import (
 )
 
 
-def validate_and_update_broadcast_message_status(broadcast_message, new_status, updating_user=None, api_key_id=None):
+def update_broadcast_message_status(broadcast_message, new_status, updating_user=None, api_key_id=None):
+    _validate_broadcast_update(broadcast_message, new_status, updating_user)
+
+    if new_status == BroadcastStatusType.BROADCASTING:
+        broadcast_message.approved_at = datetime.utcnow()
+        broadcast_message.approved_by = updating_user
+
+    if new_status == BroadcastStatusType.CANCELLED:
+        broadcast_message.cancelled_at = datetime.utcnow()
+        broadcast_message.cancelled_by = updating_user
+        broadcast_message.cancelled_by_api_key_id = api_key_id
+
+    current_app.logger.info(
+        f'broadcast_message {broadcast_message.id} moving from {broadcast_message.status} to {new_status}'
+    )
+    broadcast_message.status = new_status
+
+    dao_save_object(broadcast_message)
+    _create_p1_zendesk_alert(broadcast_message)
+
+    if new_status in {BroadcastStatusType.BROADCASTING, BroadcastStatusType.CANCELLED}:
+        _create_broadcast_event(broadcast_message)
+
+
+def _validate_broadcast_update(broadcast_message, new_status, updating_user):
     if new_status not in BroadcastStatusType.ALLOWED_STATUS_TRANSITIONS[broadcast_message.status]:
         raise InvalidRequest(
             f'Cannot move broadcast_message {broadcast_message.id} from {broadcast_message.status} to {new_status}',
@@ -32,24 +61,39 @@ def validate_and_update_broadcast_message_status(broadcast_message, new_status,
                 f'broadcast_message {broadcast_message.id} has no selected areas and so cannot be broadcasted.',
                 status_code=400
             )
-        else:
-            broadcast_message.approved_at = datetime.utcnow()
-            broadcast_message.approved_by = updating_user
 
-    if new_status == BroadcastStatusType.CANCELLED:
-        broadcast_message.cancelled_at = datetime.utcnow()
-        broadcast_message.cancelled_by = updating_user
-        broadcast_message.cancelled_by_api_key_id = api_key_id
 
-    current_app.logger.info(
-        f'broadcast_message {broadcast_message.id} moving from {broadcast_message.status} to {new_status}'
+def _create_p1_zendesk_alert(broadcast_message):
+    if current_app.config['NOTIFY_ENVIRONMENT'] != 'live':
+        return
+
+    if broadcast_message.status != BroadcastStatusType.BROADCASTING:
+        return
+
+    message = inspect.cleandoc(f"""
+        Broadcast Sent
+
+        https://www.notifications.service.gov.uk/services/{broadcast_message.service_id}/current-alerts/{broadcast_message.id}
+
+        Sent on channel {broadcast_message.service.broadcast_channel} to {broadcast_message.areas["names"]}.
+
+        Content starts "{broadcast_message.content[:100]}".
+
+        Follow the runbook to check the broadcast went out OK:
+        https://docs.google.com/document/d/1J99yOlfp4nQz6et0w5oJVqi-KywtIXkxrEIyq_g2XUs/edit#heading=h.lzr9aq5b4wg
+    """)
+
+    ticket = NotifySupportTicket(
+        subject='Live broadcast sent',
+        message=message,
+        ticket_type=NotifySupportTicket.TYPE_INCIDENT,
+        technical_ticket=True,
+        org_id=current_app.config['BROADCAST_ORGANISATION_ID'],
+        org_type='central',
+        service_id=str(broadcast_message.service_id),
+        p1=True
     )
-    broadcast_message.status = new_status
-
-    dao_save_object(broadcast_message)
-
-    if new_status in {BroadcastStatusType.BROADCASTING, BroadcastStatusType.CANCELLED}:
-        _create_broadcast_event(broadcast_message)
+    zendesk_client.send_ticket_to_zendesk(ticket)
 
 
 def _create_broadcast_event(broadcast_message):

app/celery/broadcast_message_tasks.py

@@ -1,11 +1,8 @@
 from datetime import datetime
 
 from flask import current_app
-from notifications_utils.clients.zendesk.zendesk_client import (
-    NotifySupportTicket,
-)
 
-from app import cbc_proxy_client, notify_celery, zendesk_client
+from app import cbc_proxy_client, notify_celery
 from app.clients.cbc_proxy import CBCProxyRetryableException
 from app.config import QueueNames, TaskNames
 from app.dao.broadcast_message_dao import (
@@ -126,37 +123,6 @@ def check_event_makes_sense_in_sequence(broadcast_event, provider):
 def send_broadcast_event(broadcast_event_id):
     broadcast_event = dao_get_broadcast_event_by_id(broadcast_event_id)
 
-    if (
-        current_app.config['NOTIFY_ENVIRONMENT'] == 'live' and
-        broadcast_event.message_type == BroadcastEventMessageType.ALERT
-    ):
-        broadcast_message = broadcast_event.broadcast_message
-        # raise a zendesk ticket to alert team that broadcast is going out.
-        message = '\n'.join([
-            'Broadcast Sent',
-            '',
-            f'https://www.notifications.service.gov.uk/services/{broadcast_message.service_id}/current-alerts/{broadcast_message.id}',  # noqa
-            '',
-            f'This broacast has been sent on channel {broadcast_message.service.broadcast_channel}.',
-            f'This broadcast is targeted at areas {broadcast_message.areas.get("names", [])}.',  # noqa
-            ''
-            f'This broadcast\'s content starts "{broadcast_message.content[:100]}"'
-            '',
-            'If this alert is not expected refer to the runbook for instructions.',
-            'https://docs.google.com/document/d/1J99yOlfp4nQz6et0w5oJVqi-KywtIXkxrEIyq_g2XUs',
-        ])
-        ticket = NotifySupportTicket(
-            subject='Live broadcast sent',
-            message=message,
-            ticket_type=NotifySupportTicket.TYPE_INCIDENT,
-            technical_ticket=True,
-            org_id=current_app.config['BROADCAST_ORGANISATION_ID'],
-            org_type='central',
-            service_id=str(broadcast_message.service_id)
-        )
-        zendesk_client.send_ticket_to_zendesk(ticket)
-        current_app.logger.error(message)
-
     notify_celery.send_task(
         name=TaskNames.PUBLISH_GOVUK_ALERTS,
         queue=QueueNames.GOVUK_ALERTS

app/celery/process_sms_client_response_tasks.py

@@ -8,6 +8,7 @@ from app import notify_celery, statsd_client
 from app.clients import ClientException
 from app.clients.sms.firetext import get_firetext_responses
 from app.clients.sms.mmg import get_mmg_responses
+from app.clients.sms.reach import get_reach_responses
 from app.dao import notifications_dao
 from app.dao.templates_dao import dao_get_template_by_id
 from app.models import NOTIFICATION_PENDING
@@ -17,7 +18,8 @@ from app.notifications.notifications_ses_callback import (
 
 sms_response_mapper = {
     'MMG': get_mmg_responses,
-    'Firetext': get_firetext_responses
+    'Firetext': get_firetext_responses,
+    'Reach': get_reach_responses
 }
 
 

app/celery/provider_tasks.py

@@ -24,7 +24,8 @@ def deliver_sms(self, notification_id):
     except Exception as e:
         if isinstance(e, SmsClientResponseException):
             current_app.logger.warning(
-                "SMS notification delivery for id: {} failed".format(notification_id)
+                "SMS notification delivery for id: {} failed".format(notification_id),
+                exc_info=True
             )
         else:
             current_app.logger.exception(

app/celery/scheduled_tasks.py

@@ -8,6 +8,7 @@ from sqlalchemy import between
 from sqlalchemy.exc import SQLAlchemyError
 
 from app import db, notify_celery, zendesk_client
+from app.aws import s3
 from app.celery.broadcast_message_tasks import trigger_link_test
 from app.celery.letters_pdf_tasks import get_pdf_for_templated_letter
 from app.celery.tasks import (
@@ -45,6 +46,7 @@ from app.dao.services_dao import (
     dao_find_services_with_high_failure_rates,
 )
 from app.dao.users_dao import delete_codes_older_created_more_than_a_day_ago
+from app.letters.utils import generate_letter_pdf_filename
 from app.models import (
     EMAIL_TYPE,
     JOB_STATUS_ERROR,
@@ -207,14 +209,37 @@ def replay_created_notifications():
 
 @notify_celery.task(name='check-if-letters-still-pending-virus-check')
 def check_if_letters_still_pending_virus_check():
-    letters = dao_precompiled_letters_still_pending_virus_check()
+    letters = []
+
+    for letter in dao_precompiled_letters_still_pending_virus_check():
+        # find letter in the scan bucket
+        filename = generate_letter_pdf_filename(
+            letter.reference,
+            letter.created_at,
+            ignore_folder=True,
+            postage=letter.postage
+        )
+
+        if s3.file_exists(current_app.config['LETTERS_SCAN_BUCKET_NAME'], filename):
+            current_app.logger.warning(
+                f'Letter id {letter.id} got stuck in pending-virus-check. Sending off for scan again.'
+            )
+            notify_celery.send_task(
+                name=TaskNames.SCAN_FILE,
+                kwargs={'filename': filename},
+                queue=QueueNames.ANTIVIRUS,
+            )
+        else:
+            letters.append(letter)
 
     if len(letters) > 0:
         letter_ids = [(str(letter.id), letter.reference) for letter in letters]
 
-        msg = """{} precompiled letters have been pending-virus-check for over 90 minutes. Follow runbook to resolve:
-            https://github.com/alphagov/notifications-manuals/wiki/Support-Runbook#Deal-with-letter-pending-virus-scan-for-90-minutes.
-            Notifications: {}""".format(len(letters), sorted(letter_ids))
+        msg = f"""{len(letters)} precompiled letters have been pending-virus-check for over 90 minutes.
+            We couldn't find them in the scan bucket. We'll need to find out where the files are and kick them off
+            again or move them to technical failure.
+
+            Notifications: {sorted(letter_ids)}"""
 
         if current_app.config['NOTIFY_ENVIRONMENT'] in ['live', 'production', 'test']:
             ticket = NotifySupportTicket(

app/clients/email/__init__.py

@@ -28,5 +28,6 @@ class EmailClient(Client):
     def send_email(self, *args, **kwargs):
         raise NotImplementedError('TODO Need to implement.')
 
-    def get_name(self):
+    @property
+    def name(self):
         raise NotImplementedError('TODO Need to implement.')

app/clients/email/aws_ses.py

@@ -59,11 +59,11 @@ class AwsSesClient(EmailClient):
     def init_app(self, region, statsd_client, *args, **kwargs):
         self._client = boto3.client('ses', region_name=region)
         super(AwsSesClient, self).__init__(*args, **kwargs)
-        self.name = 'ses'
         self.statsd_client = statsd_client
 
-    def get_name(self):
-        return self.name
+    @property
+    def name(self):
+        return 'ses'
 
     def send_email(self,
                    source,

app/clients/email/aws_ses_stub.py

@@ -13,12 +13,12 @@ class AwsSesStubClientException(EmailClientException):
 
 class AwsSesStubClient(EmailClient):
     def init_app(self, region, statsd_client, stub_url):
-        self.name = 'ses'
         self.statsd_client = statsd_client
         self.url = stub_url
 
-    def get_name(self):
-        return self.name
+    @property
+    def name(self):
+        return 'ses'
 
     def send_email(self,
                    source,

app/clients/sms/__init__.py

@@ -1,3 +1,5 @@
+from time import monotonic
+
 from app.clients import Client, ClientException
 
 
@@ -10,7 +12,7 @@ class SmsClientResponseException(ClientException):
         self.message = message
 
     def __str__(self):
-        return "Message {}".format(self.message)
+        return f"SMS client error ({self.message})"
 
 
 class SmsClient(Client):
@@ -18,8 +20,52 @@ class SmsClient(Client):
     Base Sms client for sending smss.
     '''
 
-    def send_sms(self, *args, **kwargs):
+    def init_app(self, current_app, statsd_client):
+        self.current_app = current_app
+        self.statsd_client = statsd_client
+        self.from_number = self.current_app.config.get('FROM_NUMBER')
+
+    def record_outcome(self, success):
+        log_message = "Provider request for {} {}".format(
+            self.name,
+            "succeeded" if success else "failed",
+        )
+
+        if success:
+            self.current_app.logger.info(log_message)
+            self.statsd_client.incr(f"clients.{self.name}.success")
+        else:
+            self.statsd_client.incr(f"clients.{self.name}.error")
+            self.current_app.logger.warning(log_message)
+
+    def send_sms(self, to, content, reference, international, sender):
+        start_time = monotonic()
+
+        if sender is None:
+            # temporary log to see if the following ternary is necessary
+            # or if it's safe to remove it - keep for 1-2 weeks
+            self.current_app.logger.warning(
+                f"send_sms called with 'sender' of 'None' for {reference}"
+            )
+
+        sender = self.from_number if sender is None else sender
+
+        try:
+            response = self.try_send_sms(to, content, reference, international, sender)
+            self.record_outcome(True)
+        except SmsClientResponseException as e:
+            self.record_outcome(False)
+            raise e
+        finally:
+            elapsed_time = monotonic() - start_time
+            self.statsd_client.timing(f"clients.{self.name}.request-time", elapsed_time)
+            self.current_app.logger.info(f"{self.name} request for {reference} finished in {elapsed_time}")
+
+        return response
+
+    def try_send_sms(self, *args, **kwargs):
         raise NotImplementedError('TODO Need to implement.')
 
-    def get_name(self):
+    @property
+    def name(self):
         raise NotImplementedError('TODO Need to implement.')

app/clients/sms/firetext.py

@@ -1,6 +1,5 @@
 import json
 import logging
-from time import monotonic
 
 from requests import RequestException, request
 
@@ -45,63 +44,30 @@ def get_message_status_and_reason_from_firetext_code(detailed_status_code):
     return firetext_codes[detailed_status_code]['status'], firetext_codes[detailed_status_code]['reason']
 
 
-class FiretextClientResponseException(SmsClientResponseException):
-    def __init__(self, response, exception):
-        status_code = response.status_code if response is not None else 504
-        text = response.text if response is not None else "Gateway Time-out"
-        self.status_code = status_code
-        self.text = text
-        self.exception = exception
-
-    def __str__(self):
-        return "Code {} text {} exception {}".format(self.status_code, self.text, str(self.exception))
-
-
 class FiretextClient(SmsClient):
     '''
     FireText sms client.
     '''
 
-    def init_app(self, current_app, statsd_client, *args, **kwargs):
-        super(SmsClient, self).__init__(*args, **kwargs)
-        self.current_app = current_app
-        self.api_key = current_app.config.get('FIRETEXT_API_KEY')
-        self.international_api_key = current_app.config.get('FIRETEXT_INTERNATIONAL_API_KEY')
-        self.from_number = current_app.config.get('FROM_NUMBER')
-        self.name = 'firetext'
-        self.url = current_app.config.get('FIRETEXT_URL')
-        self.statsd_client = statsd_client
+    def init_app(self, *args, **kwargs):
+        super().init_app(*args, **kwargs)
+        self.api_key = self.current_app.config.get('FIRETEXT_API_KEY')
+        self.international_api_key = self.current_app.config.get('FIRETEXT_INTERNATIONAL_API_KEY')
+        self.url = self.current_app.config.get('FIRETEXT_URL')
 
-    def get_name(self):
-        return self.name
+    @property
+    def name(self):
+        return 'firetext'
 
-    def record_outcome(self, success, response):
-        status_code = response.status_code if response else 503
-
-        log_message = "API {} request {} on {} response status_code {}".format(
-            "POST",
-            "succeeded" if success else "failed",
-            self.url,
-            status_code
-        )
-
-        if success:
-            self.current_app.logger.info(log_message)
-            self.statsd_client.incr("clients.firetext.success")
-        else:
-            self.statsd_client.incr("clients.firetext.error")
-            self.current_app.logger.warning(log_message)
-
-    def send_sms(self, to, content, reference, international, sender=None):
+    def try_send_sms(self, to, content, reference, international, sender):
         data = {
             "apiKey": self.international_api_key if international else self.api_key,
-            "from": self.from_number if sender is None else sender,
+            "from": sender,
             "to": to.replace('+', ''),
             "message": content,
             "reference": reference
         }
 
-        start_time = monotonic()
         try:
             response = request(
                 "POST",
@@ -113,16 +79,10 @@ class FiretextClient(SmsClient):
             try:
                 json.loads(response.text)
                 if response.json()['code'] != 0:
-                    raise ValueError()
-            except (ValueError, AttributeError) as e:
-                self.record_outcome(False, response)
-                raise FiretextClientResponseException(response=response, exception=e)
-            self.record_outcome(True, response)
-        except RequestException as e:
-            self.record_outcome(False, e.response)
-            raise FiretextClientResponseException(response=e.response, exception=e)
-        finally:
-            elapsed_time = monotonic() - start_time
-            self.current_app.logger.info("Firetext request for {} finished in {}".format(reference, elapsed_time))
-            self.statsd_client.timing("clients.firetext.request-time", elapsed_time)
+                    raise ValueError("Expected 'code' to be '0'")
+            except (ValueError, AttributeError):
+                raise SmsClientResponseException("Invalid response JSON")
+        except RequestException:
+            raise SmsClientResponseException("Request failed")
+
         return response

app/clients/sms/mmg.py

@@ -1,5 +1,4 @@
 import json
-from time import monotonic
 
 from requests import RequestException, request
 
@@ -69,45 +68,25 @@ class MMGClient(SmsClient):
     MMG sms client
     '''
 
-    def init_app(self, current_app, statsd_client, *args, **kwargs):
-        super(SmsClient, self).__init__(*args, **kwargs)
-        self.current_app = current_app
-        self.api_key = current_app.config.get('MMG_API_KEY')
-        self.from_number = current_app.config.get('FROM_NUMBER')
-        self.name = 'mmg'
-        self.statsd_client = statsd_client
-        self.mmg_url = current_app.config.get('MMG_URL')
+    def init_app(self, *args, **kwargs):
+        super().init_app(*args, **kwargs)
+        self.api_key = self.current_app.config.get('MMG_API_KEY')
+        self.mmg_url = self.current_app.config.get('MMG_URL')
 
-    def record_outcome(self, success, response):
-        status_code = response.status_code if response else 503
-        log_message = "API {} request {} on {} response status_code {}".format(
-            "POST",
-            "succeeded" if success else "failed",
-            self.mmg_url,
-            status_code
-        )
+    @property
+    def name(self):
+        return 'mmg'
 
-        if success:
-            self.current_app.logger.info(log_message)
-            self.statsd_client.incr("clients.mmg.success")
-        else:
-            self.statsd_client.incr("clients.mmg.error")
-            self.current_app.logger.warning(log_message)
-
-    def get_name(self):
-        return self.name
-
-    def send_sms(self, to, content, reference, international, multi=True, sender=None):
+    def try_send_sms(self, to, content, reference, international, sender):
         data = {
             "reqType": "BULK",
             "MSISDN": to,
             "msg": content,
-            "sender": self.from_number if sender is None else sender,
+            "sender": sender,
             "cid": reference,
-            "multi": multi
+            "multi": True
         }
 
-        start_time = monotonic()
         try:
             response = request(
                 "POST",
@@ -123,16 +102,9 @@ class MMGClient(SmsClient):
             response.raise_for_status()
             try:
                 json.loads(response.text)
-            except (ValueError, AttributeError) as e:
-                self.record_outcome(False, response)
-                raise MMGClientResponseException(response=response, exception=e)
-            self.record_outcome(True, response)
-        except RequestException as e:
-            self.record_outcome(False, e.response)
-            raise MMGClientResponseException(response=e.response, exception=e)
-        finally:
-            elapsed_time = monotonic() - start_time
-            self.statsd_client.timing("clients.mmg.request-time", elapsed_time)
-            self.current_app.logger.info("MMG request for {} finished in {}".format(reference, elapsed_time))
+            except (ValueError, AttributeError):
+                raise SmsClientResponseException("Invalid response JSON")
+        except RequestException:
+            raise SmsClientResponseException("Request failed")
 
         return response

app/clients/sms/reach.py

@@ -0,0 +1,52 @@
+import json
+
+from requests import RequestException, request
+
+from app.clients.sms import SmsClient, SmsClientResponseException
+
+
+def get_reach_responses(status, detailed_status_code=None):
+    if status == 'TODO-d':
+        return ("delivered", "TODO: Delivered")
+    elif status == 'TODO-tf':
+        return ("temporary-failure", "TODO: Temporary failure")
+    elif status == 'TODO-pf':
+        return ("permanent-failure", "TODO: Permanent failure")
+    else:
+        raise KeyError
+
+
+class ReachClient(SmsClient):
+    def init_app(self, *args, **kwargs):
+        super().init_app(*args, **kwargs)
+        self.url = self.current_app.config.get('REACH_URL')
+
+    @property
+    def name(self):
+        return 'reach'
+
+    def try_send_sms(self, to, content, reference, international, sender):
+        data = {
+            # TODO
+        }
+
+        try:
+            response = request(
+                "POST",
+                self.url,
+                data=json.dumps(data),
+                headers={
+                    'Content-Type': 'application/json',
+                },
+                timeout=60
+            )
+
+            response.raise_for_status()
+            try:
+                json.loads(response.text)
+            except (ValueError, AttributeError):
+                raise SmsClientResponseException("Invalid response JSON")
+        except RequestException:
+            raise SmsClientResponseException("Request failed")
+
+        return response

app/commands.py

@@ -807,9 +807,27 @@ def populate_annual_billing_with_defaults(year, missing_services_only):
         active_services = Service.query.filter(
             Service.active
         ).all()
+    previous_year = year - 1
+    services_with_zero_free_allowance = db.session.query(AnnualBilling.service_id).filter(
+        AnnualBilling.financial_year_start == previous_year,
+        AnnualBilling.free_sms_fragment_limit == 0
+    ).all()
 
     for service in active_services:
-        set_default_free_allowance_for_service(service, year)
+
+        # If a service has free_sms_fragment_limit for the previous year
+        # set the free allowance for this year to 0 as well.
+        # Else use the default free allowance for the service.
+        if service.id in [x.service_id for x in services_with_zero_free_allowance]:
+            print(f'update service {service.id} to 0')
+            dao_create_or_update_annual_billing_for_year(
+                service_id=service.id,
+                free_sms_fragment_limit=0,
+                financial_year_start=year
+            )
+        else:
+            print(f'update service {service.id} with default')
+            set_default_free_allowance_for_service(service, year)
 
 
 @click.option('-u', '--user-id', required=True)
@@ -833,6 +851,7 @@ def local_dev_broadcast_permissions(user_id):
                 'reject_broadcasts', 'cancel_broadcasts',  # required to create / approve
                 'create_broadcasts', 'approve_broadcasts',  # minimum for testing
                 'manage_templates',  # unlikely but might be useful
+                'view_activity',  # normally added on invite / service creation
             ]
         ]
 

app/config.py

@@ -161,8 +161,8 @@ class Config(object):
 
     # these should always add up to 100%
     SMS_PROVIDER_RESTING_POINTS = {
-        'mmg': 50,
-        'firetext': 50
+        'mmg': 60,
+        'firetext': 40
     }
 
     NOTIFY_SERVICE_ID = 'd6aa2c68-a2d9-4437-ab19-3ae8eb202553'
@@ -380,6 +380,7 @@ class Config(object):
     # these environment vars aren't defined in the manifest so to set them on paas use `cf set-env`
     MMG_URL = os.environ.get("MMG_URL", "https://api.mmg.co.uk/jsonv2a/api.php")
     FIRETEXT_URL = os.environ.get("FIRETEXT_URL", "https://www.firetext.co.uk/api/sendsms/json")
+    REACH_URL = os.environ.get("REACH_URL", "TODO")
     SES_STUB_URL = os.environ.get("SES_STUB_URL")
 
     AWS_REGION = 'eu-west-1'
@@ -432,7 +433,7 @@ class Development(Config):
 
     ANTIVIRUS_ENABLED = os.getenv('ANTIVIRUS_ENABLED') == '1'
 
-    API_HOST_NAME = "http://localhost:6011"
+    API_HOST_NAME = os.getenv('API_HOST_NAME', 'http://localhost:6011')
     API_RATE_LIMIT_ENABLED = True
     DVLA_EMAIL_ADDRESSES = ['success@simulator.amazonses.com']
 
@@ -481,6 +482,7 @@ class Test(Development):
 
     MMG_URL = 'https://example.com/mmg'
     FIRETEXT_URL = 'https://example.com/firetext'
+    REACH_URL = 'https://example.com/reach'
 
     CBC_PROXY_ENABLED = True
     DVLA_EMAIL_ADDRESSES = ['success@simulator.amazonses.com', 'success+2@simulator.amazonses.com']
@@ -544,21 +546,6 @@ class CloudFoundryConfig(Config):
     pass
 
 
-# CloudFoundry sandbox
-class Sandbox(CloudFoundryConfig):
-    NOTIFY_EMAIL_DOMAIN = 'notify.works'
-    NOTIFY_ENVIRONMENT = 'sandbox'
-    CSV_UPLOAD_BUCKET_NAME = 'cf-sandbox-notifications-csv-upload'
-    CONTACT_LIST_BUCKET_NAME = 'cf-sandbox-contact-list'
-    LETTERS_PDF_BUCKET_NAME = 'cf-sandbox-letters-pdf'
-    TEST_LETTERS_BUCKET_NAME = 'cf-sandbox-test-letters'
-    DVLA_RESPONSE_BUCKET_NAME = 'notify.works-ftp'
-    LETTERS_PDF_BUCKET_NAME = 'cf-sandbox-letters-pdf'
-    LETTERS_SCAN_BUCKET_NAME = 'cf-sandbox-letters-scan'
-    INVALID_PDF_BUCKET_NAME = 'cf-sandbox-letters-invalid-pdf'
-    FROM_NUMBER = 'sandbox'
-
-
 configs = {
     'development': Development,
     'test': Test,
@@ -566,5 +553,4 @@ configs = {
     'production': Live,
     'staging': Staging,
     'preview': Preview,
-    'sandbox': Sandbox
 }

app/dao/annual_billing_dao.py

@@ -58,34 +58,42 @@ def set_default_free_allowance_for_service(service, year_start=None):
         'central': {
             2020: 250_000,
             2021: 150_000,
+            2022: 40_000,
         },
         'local': {
             2020: 25_000,
             2021: 25_000,
+            2022: 20_000,
         },
         'nhs_central': {
             2020: 250_000,
             2021: 150_000,
+            2022: 40_000,
         },
         'nhs_local': {
             2020: 25_000,
             2021: 25_000,
+            2022: 20_000,
         },
         'nhs_gp': {
             2020: 25_000,
             2021: 10_000,
+            2022: 10_000,
         },
         'emergency_service': {
             2020: 25_000,
             2021: 25_000,
+            2022: 20_000,
         },
         'school_or_college': {
             2020: 25_000,
             2021: 10_000,
+            2022: 10_000,
         },
         'other': {
             2020: 25_000,
             2021: 10_000,
+            2022: 10_000,
         },
     }
     if not year_start:
@@ -93,8 +101,8 @@ def set_default_free_allowance_for_service(service, year_start=None):
     # handle cases where the year is less than 2020 or greater than 2021
     if year_start < 2020:
         year_start = 2020
-    if year_start > 2021:
-        year_start = 2021
+    if year_start > 2022:
+        year_start = 2022
     if service.organisation_type:
         free_allowance = default_free_sms_fragment_limits[service.organisation_type][year_start]
     else:

app/dao/notifications_dao.py

@@ -775,15 +775,18 @@ def dao_precompiled_letters_still_pending_virus_check():
 def _duplicate_update_warning(notification, status):
     current_app.logger.info(
         (
-            'Duplicate callback received. Notification id {id} received a status update to {new_status}'
-            ' from {old_status} for {type} sent by {sent_by}. This happened {time_diff} after being first set.'
+            'Duplicate callback received for service {service_id}. '
+            'Notification ID {id} with type {type} sent by {sent_by}. '
+            'New status was {new_status}, current status is {old_status}. '
+            'This happened {time_diff} after being first set.'
         ).format(
             id=notification.id,
             old_status=notification.status,
             new_status=status,
             time_diff=datetime.utcnow() - (notification.updated_at or notification.created_at),
             type=notification.notification_type,
-            sent_by=notification.sent_by
+            sent_by=notification.sent_by,
+            service_id=notification.service_id
         )
     )
 

app/dao/provider_details_dao.py

@@ -36,6 +36,8 @@ def dao_get_provider_versions(provider_id):
         id=provider_id
     ).order_by(
         desc(ProviderDetailsHistory.version)
+    ).limit(
+        100  # limit results instead of adding pagination
     ).all()
 
 
@@ -84,7 +86,8 @@ def dao_reduce_sms_provider_priority(identifier, *, time_threshold):
     amount_to_reduce_by = 10
     providers_list = _get_sms_providers_for_update(time_threshold)
 
-    if not providers_list:
+    if len(providers_list) < 2:
+        current_app.logger.info("Not adjusting providers, number of active providers is less than 2.")
         return
 
     providers = {provider.identifier: provider for provider in providers_list}

app/delivery/send_to_providers.py

@@ -60,7 +60,7 @@ def send_sms_to_provider(notification):
         key_type = notification.key_type
         if service.research_mode or notification.key_type == KEY_TYPE_TEST:
             update_notification_to_sending(notification, provider)
-            send_sms_response(provider.get_name(), str(notification.id), notification.to)
+            send_sms_response(provider.name, str(notification.id), notification.to)
 
         else:
             try:
@@ -82,7 +82,7 @@ def send_sms_to_provider(notification):
             except Exception as e:
                 notification.billable_units = template.fragment_count
                 dao_update_notification(notification)
-                dao_reduce_sms_provider_priority(provider.get_name(), time_threshold=timedelta(minutes=1))
+                dao_reduce_sms_provider_priority(provider.name, time_threshold=timedelta(minutes=1))
                 raise e
             else:
                 notification.billable_units = template.fragment_count
@@ -158,7 +158,7 @@ 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.sent_by = provider.name
     if notification.status not in NOTIFICATION_STATUS_TYPES_COMPLETED:
         notification.status = NOTIFICATION_SENT if notification.international else NOTIFICATION_SENDING
     dao_update_notification(notification)

app/notifications/notifications_sms_callback.py

@@ -1,4 +1,4 @@
-from flask import Blueprint, current_app, json, jsonify, request
+from flask import Blueprint, json, jsonify, request
 
 from app.celery.process_sms_client_response_tasks import (
     process_sms_client_response,
@@ -30,12 +30,6 @@ def process_mmg_response():
         queue=QueueNames.SMS_CALLBACKS,
     )
 
-    safe_to_log = data.copy()
-    safe_to_log.pop("MSISDN")
-    current_app.logger.debug(
-        f"Full delivery response from {client_name} for notification: {provider_reference}\n{safe_to_log}"
-    )
-
     return jsonify(result='success'), 200
 
 
@@ -52,12 +46,28 @@ def process_firetext_response():
     detailed_status_code = request.form.get('code')
     provider_reference = request.form.get('reference')
 
-    safe_to_log = dict(request.form).copy()
-    safe_to_log.pop('mobile')
-    current_app.logger.debug(
-        f"Full delivery response from {client_name} for notification: {provider_reference}\n{safe_to_log}"
+    process_sms_client_response.apply_async(
+        [status, provider_reference, client_name, detailed_status_code],
+        queue=QueueNames.SMS_CALLBACKS,
     )
 
+    return jsonify(result='success'), 200
+
+
+@sms_callback_blueprint.route('/reach', methods=['POST'])
+def process_reach_response():
+    client_name = 'Reach'
+
+    # TODO: validate request
+    errors = None
+
+    if errors:
+        raise InvalidRequest(errors, status_code=400)
+
+    status = 'TODO-d'  # TODO
+    detailed_status_code = 'something'  # TODO
+    provider_reference = 'notification_id'  # TODO
+
     process_sms_client_response.apply_async(
         [status, provider_reference, client_name, detailed_status_code],
         queue=QueueNames.SMS_CALLBACKS,

app/v2/broadcast/post_broadcast.py

@@ -6,10 +6,8 @@ from notifications_utils.template import BroadcastMessageTemplate
 from sqlalchemy.orm.exc import MultipleResultsFound
 
 from app import api_user, authenticated_service, redis_store
+from app.broadcast_message import utils as broadcast_utils
 from app.broadcast_message.translators import cap_xml_to_dict
-from app.broadcast_message.utils import (
-    validate_and_update_broadcast_message_status,
-)
 from app.dao.broadcast_message_dao import (
     dao_get_broadcast_message_by_references_and_service_id,
 )
@@ -121,7 +119,7 @@ def _cancel_or_reject_broadcast(references_to_original_broadcast, service_id):
         new_status = BroadcastStatusType.REJECTED
     else:
         new_status = BroadcastStatusType.CANCELLED
-    validate_and_update_broadcast_message_status(
+    broadcast_utils.update_broadcast_message_status(
         broadcast_message,
         new_status,
         api_key_id=api_user.id

app/v2/notifications/get_notifications.py

@@ -55,7 +55,7 @@ def get_pdf_for_notification(notification_id):
     except Exception:
         raise PDFNotReadyError()
 
-    return send_file(filename_or_fp=BytesIO(pdf_data), mimetype='application/pdf')
+    return send_file(path_or_file=BytesIO(pdf_data), mimetype='application/pdf')
 
 
 @v2_notification_blueprint.route("", methods=['GET'])

manifest.yml.j2

@@ -27,9 +27,9 @@
       'STATSD_HOST': None
     },
     'routes': {
-      'preview': ['api.notify.works/notifications/sms/mmg', 'api.notify.works/notifications/sms/firetext'],
-      'staging': ['api.staging-notify.works/notifications/sms/mmg', 'api.staging-notify.works/notifications/sms/firetext'],
-      'production': ['api.notifications.service.gov.uk/notifications/sms/mmg', 'api.notifications.service.gov.uk/notifications/sms/firetext'],
+      'preview': ['api.notify.works/notifications/sms/mmg', 'api.notify.works/notifications/sms/firetext', 'api.notify.works/notifications/sms/reach'],
+      'staging': ['api.staging-notify.works/notifications/sms/mmg', 'api.staging-notify.works/notifications/sms/firetext', 'api.staging-notify.works/notifications/sms/reach'],
+      'production': ['api.notifications.service.gov.uk/notifications/sms/mmg', 'api.notifications.service.gov.uk/notifications/sms/firetext', 'api.notifications.service.gov.uk/notifications/sms/reach'],
     },
     'health-check-type': 'port',
     'health-check-invocation-timeout': 3,

migrations/versions/0367_add_reach.py

@@ -0,0 +1,54 @@
+"""
+
+Revision ID: 0367_add_reach
+Revises: 0366_letter_rates_2022
+Create Date: 2022-03-24 16:00:00
+
+"""
+import itertools
+import uuid
+from datetime import datetime
+
+from alembic import op
+from sqlalchemy.sql import text
+
+from app.models import LetterRate
+
+
+revision = '0367_add_reach'
+down_revision = '0366_letter_rates_2022'
+
+
+def upgrade():
+    conn = op.get_bind()
+    conn.execute(
+        """
+        INSERT INTO provider_details (
+            id,
+            display_name,
+            identifier,
+            priority,
+            notification_type,
+            active,
+            version,
+            created_by_id
+        )
+        VALUES (
+            '{}',
+            'Reach',
+            'reach',
+            0,
+            'sms',
+            false,
+            1,
+            null
+        )
+        """.format(
+            str(uuid.uuid4()),
+        )
+    )
+
+
+def downgrade():
+    conn = op.get_bind()
+    conn.execute("DELETE FROM provider_details WHERE identifier = 'reach'")

requirements.in

@@ -1,5 +1,5 @@
 # Run `make freeze-requirements` to update requirements.txt
-# with package version changes made in requirements-app.txt
+# with package version changes made in requirements.in
 
 cffi==1.15.0
 celery[sqs]==5.2.3
@@ -7,24 +7,24 @@ Flask-Bcrypt==0.7.1
 flask-marshmallow==0.14.0
 Flask-Migrate==3.1.0
 git+https://github.com/mitsuhiko/flask-sqlalchemy.git@500e732dd1b975a56ab06a46bd1a20a21e682262#egg=Flask-SQLAlchemy==2.3.2.dev20190108
-Flask==1.1.2
+Flask==2.1.0
 click-datetime==0.2
 # Should be pinned until a new gunicorn release greater than 20.1.0 comes out. (Due to eventlet v0.33 compatibility issues)
 git+https://github.com/benoitc/gunicorn.git@1299ea9e967a61ae2edebe191082fd169b864c64#egg=gunicorn[eventlet]==20.1.0
 iso8601==1.0.2
-itsdangerous==1.1.0
+itsdangerous==2.1.2
 jsonschema==3.2.0
 marshmallow-sqlalchemy==0.23.1 # pyup: <0.24.0 # marshmallow v3 throws errors
 marshmallow==2.21.0 # pyup: <3 # v3 throws errors
 psycopg2-binary==2.9.3
-PyJWT==2.0.1
-SQLAlchemy==1.4.10
+PyJWT==2.3.0
+SQLAlchemy==1.4.32
 strict-rfc3339==0.7
 rfc3987==1.3.8
 cachetools==4.2.1
-beautifulsoup4==4.9.3
+beautifulsoup4==4.10.0
 lxml==4.8.0
-Werkzeug==2.0.2
+Werkzeug==2.0.3
 
 notifications-python-client==6.3.0
 

requirements.txt

@@ -18,7 +18,7 @@ awscli-cwlogs==1.4.6
     # via -r requirements.in
 bcrypt==3.2.0
     # via flask-bcrypt
-beautifulsoup4==4.9.3
+beautifulsoup4==4.10.0
     # via -r requirements.in
 billiard==3.6.4.0
     # via celery
@@ -75,7 +75,7 @@ docutils==0.15.2
     # via awscli
 eventlet==0.33.0
     # via gunicorn
-flask==1.1.2
+flask==2.1.0
     # via
     #   -r requirements.in
     #   flask-bcrypt
@@ -103,16 +103,16 @@ geojson==2.5.0
 govuk-bank-holidays==0.10
     # via notifications-utils
 greenlet==1.1.2
-    # via
-    #   eventlet
-    #   sqlalchemy
+    # via eventlet
 gunicorn @ git+https://github.com/benoitc/gunicorn.git@1299ea9e967a61ae2edebe191082fd169b864c64
     # via -r requirements.in
 idna==3.3
     # via requests
+importlib-metadata==4.11.3
+    # via flask
 iso8601==1.0.2
     # via -r requirements.in
-itsdangerous==1.1.0
+itsdangerous==2.1.2
     # via
     #   -r requirements.in
     #   flask
@@ -168,7 +168,7 @@ pyasn1==0.4.8
     # via rsa
 pycparser==2.20
     # via cffi
-pyjwt==2.0.1
+pyjwt==2.3.0
     # via
     #   -r requirements.in
     #   notifications-python-client
@@ -226,7 +226,7 @@ smartypants==2.0.1
     # via notifications-utils
 soupsieve==2.2.1
     # via beautifulsoup4
-sqlalchemy==1.4.10
+sqlalchemy==1.4.32
     # via
     #   -r requirements.in
     #   alembic
@@ -248,10 +248,12 @@ wcwidth==0.2.5
     # via prompt-toolkit
 webencodings==0.5.1
     # via bleach
-werkzeug==2.0.2
+werkzeug==2.0.3
     # via
     #   -r requirements.in
     #   flask
+zipp==3.8.0
+    # via importlib-metadata
 
 # The following packages are considered to be unsafe in a requirements file:
 # setuptools

requirements_for_test.txt

@@ -2,13 +2,13 @@
 flake8==4.0.1
 flake8-bugbear==22.1.11
 isort==5.10.1
-moto==3.0.5
+moto==3.0.7
 pytest==7.0.1
 pytest-env==0.6.2
 pytest-mock==3.7.0
 pytest-cov==3.0.0
 pytest-xdist==2.5.0
-freezegun==1.1.0
+freezegun==1.2.0
 requests-mock==1.9.3
 # used for creating manifest file locally
 jinja2-cli[yaml]==0.8.1

scripts/run_with_docker.sh

@@ -8,8 +8,9 @@ source environment.sh
 # this script should be run from within your virtualenv so you can access the aws cli
 AWS_ACCESS_KEY_ID=${AWS_ACCESS_KEY_ID:-"$(aws configure get aws_access_key_id)"}
 AWS_SECRET_ACCESS_KEY=${AWS_SECRET_ACCESS_KEY:-"$(aws configure get aws_secret_access_key)"}
-: "${SQLALCHEMY_DATABASE_URI:=postgresql://postgres@host.docker.internal/notification_api}"
+SQLALCHEMY_DATABASE_URI="postgresql://postgres@host.docker.internal/notification_api"
 REDIS_URL="redis://host.docker.internal:6379"
+API_HOST_NAME="http://host.docker.internal:6011"
 
 docker run -it --rm \
   -e AWS_ACCESS_KEY_ID=$AWS_ACCESS_KEY_ID \
@@ -17,6 +18,7 @@ docker run -it --rm \
   -e SQLALCHEMY_DATABASE_URI=$SQLALCHEMY_DATABASE_URI \
   -e REDIS_ENABLED=${REDIS_ENABLED:-0} \
   -e REDIS_URL=$REDIS_URL \
+  -e API_HOST_NAME=$API_HOST_NAME \
   -v $(pwd):/home/vcap/app \
   ${DOCKER_IMAGE_NAME} \
   ${@}

tests/app/broadcast_message/test_utils.py

@@ -1,7 +1,8 @@
 import pytest
 
 from app.broadcast_message.utils import (
-    validate_and_update_broadcast_message_status,
+    _create_p1_zendesk_alert,
+    update_broadcast_message_status,
 )
 from app.errors import InvalidRequest
 from app.models import (
@@ -15,9 +16,10 @@ from tests.app.db import (
     create_template,
     create_user,
 )
+from tests.conftest import set_config
 
 
-def test_validate_and_update_broadcast_message_status_stores_approved_by_and_approved_at_and_queues_task(
+def test_update_broadcast_message_status_stores_approved_by_and_approved_at_and_queues_task(
     sample_broadcast_service,
     mocker
 ):
@@ -34,7 +36,7 @@ def test_validate_and_update_broadcast_message_status_stores_approved_by_and_app
     sample_broadcast_service.users.append(approver)
     mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async')
 
-    validate_and_update_broadcast_message_status(
+    update_broadcast_message_status(
         broadcast_message, BroadcastStatusType.BROADCASTING, approver
     )
 
@@ -54,7 +56,7 @@ def test_validate_and_update_broadcast_message_status_stores_approved_by_and_app
     assert alert_event.transmitted_content == {"body": "emergency broadcast"}
 
 
-def test_validate_and_update_broadcast_message_status_for_cancelling_broadcast_from_admin_interface(
+def test_update_broadcast_message_status_for_cancelling_broadcast_from_admin_interface(
     sample_broadcast_service,
     mocker,
 ):
@@ -71,7 +73,7 @@ def test_validate_and_update_broadcast_message_status_for_cancelling_broadcast_f
 
     mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async')
 
-    validate_and_update_broadcast_message_status(
+    update_broadcast_message_status(
         broadcast_message, BroadcastStatusType.CANCELLED, updating_user=canceller, api_key_id=None
     )
 
@@ -89,7 +91,7 @@ def test_validate_and_update_broadcast_message_status_for_cancelling_broadcast_f
     assert alert_event.message_type == BroadcastEventMessageType.CANCEL
 
 
-def test_validate_and_update_broadcast_message_status_for_cancelling_broadcast_from_API_call(
+def test_update_broadcast_message_status_for_cancelling_broadcast_from_API_call(
     sample_broadcast_service,
     mocker,
 ):
@@ -105,7 +107,7 @@ def test_validate_and_update_broadcast_message_status_for_cancelling_broadcast_f
     )
     mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async')
 
-    validate_and_update_broadcast_message_status(
+    update_broadcast_message_status(
         broadcast_message, BroadcastStatusType.CANCELLED, updating_user=None, api_key_id=api_key.id
     )
 
@@ -123,7 +125,7 @@ def test_validate_and_update_broadcast_message_status_for_cancelling_broadcast_f
     assert alert_event.message_type == BroadcastEventMessageType.CANCEL
 
 
-def test_validate_and_update_broadcast_message_status_for_rejecting_broadcast_via_admin_interface(
+def test_update_broadcast_message_status_for_rejecting_broadcast_via_admin_interface(
     sample_broadcast_service,
     mocker
 ):
@@ -138,7 +140,7 @@ def test_validate_and_update_broadcast_message_status_for_rejecting_broadcast_vi
     )
     mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async')
 
-    validate_and_update_broadcast_message_status(
+    update_broadcast_message_status(
         broadcast_message, BroadcastStatusType.REJECTED, updating_user=sample_broadcast_service.created_by
     )
 
@@ -151,7 +153,7 @@ def test_validate_and_update_broadcast_message_status_for_rejecting_broadcast_vi
     assert len(broadcast_message.events) == 0
 
 
-def test_validate_and_update_broadcast_message_status_for_rejecting_broadcast_from_API_call(
+def test_update_broadcast_message_status_for_rejecting_broadcast_from_API_call(
     sample_broadcast_service,
     mocker
 ):
@@ -167,7 +169,7 @@ def test_validate_and_update_broadcast_message_status_for_rejecting_broadcast_fr
     )
     mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async')
 
-    validate_and_update_broadcast_message_status(
+    update_broadcast_message_status(
         broadcast_message, BroadcastStatusType.REJECTED, api_key_id=api_key.id
     )
 
@@ -209,7 +211,7 @@ def test_validate_and_update_broadcast_message_status_for_rejecting_broadcast_fr
     (BroadcastStatusType.CANCELLED, BroadcastStatusType.BROADCASTING),
     (BroadcastStatusType.CANCELLED, BroadcastStatusType.COMPLETED),
 ])
-def test_validate_and_update_broadcast_message_status_restricts_status_transitions_to_explicit_list(
+def test_update_broadcast_message_status_restricts_status_transitions_to_explicit_list(
     sample_broadcast_service,
     mocker,
     current_status,
@@ -222,14 +224,14 @@ def test_validate_and_update_broadcast_message_status_restricts_status_transitio
     mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async')
 
     with pytest.raises(expected_exception=InvalidRequest) as e:
-        validate_and_update_broadcast_message_status(broadcast_message, new_status, approver)
+        update_broadcast_message_status(broadcast_message, new_status, approver)
 
     assert mock_task.called is False
     assert f'from {current_status} to {new_status}' in str(e.value)
 
 
 @pytest.mark.parametrize('is_platform_admin', [True, False])
-def test_validate_and_update_broadcast_message_status_rejects_approval_from_creator(
+def test_update_broadcast_message_status_rejects_approval_from_creator(
     sample_broadcast_service,
     mocker,
     is_platform_admin
@@ -241,7 +243,7 @@ def test_validate_and_update_broadcast_message_status_rejects_approval_from_crea
     mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async')
 
     with pytest.raises(expected_exception=InvalidRequest) as e:
-        validate_and_update_broadcast_message_status(
+        update_broadcast_message_status(
             broadcast_message, BroadcastStatusType.BROADCASTING, creator_and_approver
         )
 
@@ -249,7 +251,7 @@ def test_validate_and_update_broadcast_message_status_rejects_approval_from_crea
     assert 'cannot approve their own broadcast' in str(e.value)
 
 
-def test_validate_and_update_broadcast_message_status_rejects_approval_of_broadcast_with_no_areas(
+def test_update_broadcast_message_status_rejects_approval_of_broadcast_with_no_areas(
     admin_request,
     sample_broadcast_service,
     mocker
@@ -261,13 +263,13 @@ def test_validate_and_update_broadcast_message_status_rejects_approval_of_broadc
     mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async')
 
     with pytest.raises(expected_exception=InvalidRequest) as e:
-        validate_and_update_broadcast_message_status(broadcast, BroadcastStatusType.BROADCASTING, approver)
+        update_broadcast_message_status(broadcast, BroadcastStatusType.BROADCASTING, approver)
 
     assert mock_task.called is False
     assert f'broadcast_message {broadcast.id} has no selected areas and so cannot be broadcasted.' in str(e.value)
 
 
-def test_validate_and_update_broadcast_message_status_allows_trial_mode_services_to_approve_own_message(
+def test_update_broadcast_message_status_allows_trial_mode_services_to_approve_own_message(
     notify_db,
     sample_broadcast_service,
     mocker
@@ -282,7 +284,7 @@ def test_validate_and_update_broadcast_message_status_allows_trial_mode_services
     creator_and_approver = sample_broadcast_service.created_by
     mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async')
 
-    validate_and_update_broadcast_message_status(
+    update_broadcast_message_status(
         broadcast_message, BroadcastStatusType.BROADCASTING, creator_and_approver
     )
 
@@ -298,7 +300,7 @@ def test_validate_and_update_broadcast_message_status_allows_trial_mode_services
     (True, False),
     (False, True),
 ])
-def test_validate_and_update_broadcast_message_status_when_broadcast_message_is_stubbed_or_service_not_live(
+def test_update_broadcast_message_status_when_broadcast_message_is_stubbed_or_service_not_live(
     admin_request,
     sample_broadcast_service,
     mocker,
@@ -319,7 +321,7 @@ def test_validate_and_update_broadcast_message_status_when_broadcast_message_is_
 
     sample_broadcast_service.restricted = service_restricted_before_approval
 
-    validate_and_update_broadcast_message_status(
+    update_broadcast_message_status(
         broadcast_message, BroadcastStatusType.BROADCASTING, approver
     )
     assert broadcast_message.status == BroadcastStatusType.BROADCASTING
@@ -331,7 +333,7 @@ def test_validate_and_update_broadcast_message_status_when_broadcast_message_is_
     assert len(mock_task.mock_calls) == 0
 
 
-def test_validate_and_update_broadcast_message_status_creates_event_with_correct_content_if_broadcast_has_no_template(
+def test_update_broadcast_message_status_creates_event_with_correct_content_if_broadcast_has_no_template(
     admin_request,
     sample_broadcast_service,
     mocker
@@ -350,7 +352,7 @@ def test_validate_and_update_broadcast_message_status_creates_event_with_correct
     sample_broadcast_service.users.append(approver)
     mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async')
 
-    validate_and_update_broadcast_message_status(
+    update_broadcast_message_status(
         broadcast_message, BroadcastStatusType.BROADCASTING, approver
     )
 
@@ -362,3 +364,93 @@ def test_validate_and_update_broadcast_message_status_creates_event_with_correct
     mock_task.assert_called_once_with(kwargs={'broadcast_event_id': str(alert_event.id)}, queue='broadcast-tasks')
 
     assert alert_event.transmitted_content == {"body": "tailor made emergency broadcast content"}
+
+
+def test_update_broadcast_message_status_creates_zendesk_ticket(
+    mocker,
+    notify_api,
+    sample_broadcast_service
+):
+    broadcast_message = create_broadcast_message(
+        service=sample_broadcast_service,
+        content='tailor made emergency broadcast content',
+        status=BroadcastStatusType.PENDING_APPROVAL,
+        areas={"names": ["England", "Scotland"], "simple_polygons": ['polygons']}
+    )
+    approver = create_user(email='approver@gov.uk')
+    sample_broadcast_service.users.append(approver)
+
+    mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async')
+    mock_send_ticket_to_zendesk = mocker.patch(
+        'app.broadcast_message.utils.zendesk_client.send_ticket_to_zendesk',
+        autospec=True,
+    )
+
+    with set_config(notify_api, 'NOTIFY_ENVIRONMENT', 'live'):
+        update_broadcast_message_status(
+            broadcast_message, BroadcastStatusType.BROADCASTING, approver
+        )
+
+    mock_send_ticket_to_zendesk.assert_called_once()
+
+
+def test_create_p1_zendesk_alert(sample_broadcast_service, mocker, notify_api):
+    broadcast_message = create_broadcast_message(
+        service=sample_broadcast_service,
+        content='tailor made emergency broadcast content',
+        status=BroadcastStatusType.BROADCASTING,
+        areas={"names": ["England", "Scotland"]}
+    )
+
+    mock_send_ticket_to_zendesk = mocker.patch(
+        'app.broadcast_message.utils.zendesk_client.send_ticket_to_zendesk',
+        autospec=True,
+    )
+
+    with set_config(notify_api, 'NOTIFY_ENVIRONMENT', 'live'):
+        _create_p1_zendesk_alert(broadcast_message)
+
+    ticket = mock_send_ticket_to_zendesk.call_args_list[0].args[0]
+    assert ticket.subject == 'Live broadcast sent'
+    assert ticket.ticket_type == 'incident'
+    assert str(broadcast_message.id) in ticket.message
+    assert "Sent on channel severe to ['England', 'Scotland']" in ticket.message
+    assert 'Content starts "tailor made emergency' in ticket.message
+
+
+def test_create_p1_zendesk_alert_doesnt_alert_when_cancelling(mocker, notify_api, sample_broadcast_service):
+    broadcast_message = create_broadcast_message(
+        service=sample_broadcast_service,
+        content='tailor made emergency broadcast content',
+        status=BroadcastStatusType.CANCELLED,
+        areas={"names": ["England", "Scotland"]}
+    )
+
+    mock_send_ticket_to_zendesk = mocker.patch(
+        'app.broadcast_message.utils.zendesk_client.send_ticket_to_zendesk',
+        autospec=True,
+    )
+
+    with set_config(notify_api, 'NOTIFY_ENVIRONMENT', 'live'):
+        _create_p1_zendesk_alert(broadcast_message)
+
+    mock_send_ticket_to_zendesk.assert_not_called()
+
+
+def test_create_p1_zendesk_alert_doesnt_alert_on_staging(mocker, notify_api, sample_broadcast_service):
+    broadcast_message = create_broadcast_message(
+        service=sample_broadcast_service,
+        content='tailor made emergency broadcast content',
+        status=BroadcastStatusType.BROADCASTING,
+        areas={"names": ["England", "Scotland"]}
+    )
+
+    mock_send_ticket_to_zendesk = mocker.patch(
+        'app.broadcast_message.utils.zendesk_client.send_ticket_to_zendesk',
+        autospec=True,
+    )
+
+    with set_config(notify_api, 'NOTIFY_ENVIRONMENT', 'staging'):
+        _create_p1_zendesk_alert(broadcast_message)
+
+    mock_send_ticket_to_zendesk.assert_not_called()

tests/app/celery/test_broadcast_message_tasks.py

@@ -3,11 +3,7 @@ from unittest.mock import ANY, Mock, call
 
 import pytest
 from celery.exceptions import Retry
-from flask import current_app
 from freezegun import freeze_time
-from notifications_utils.clients.zendesk.zendesk_client import (
-    NotifySupportTicket,
-)
 
 from app.celery.broadcast_message_tasks import (
     BroadcastIntegrityError,
@@ -41,10 +37,6 @@ def test_send_broadcast_event_queues_up_for_active_providers(mocker, notify_api,
 
     mocker.patch('app.celery.broadcast_message_tasks.notify_celery.send_task')
 
-    mock_send_ticket_to_zendesk = mocker.patch(
-        'app.celery.broadcast_message_tasks.zendesk_client.send_ticket_to_zendesk',
-        autospec=True,
-    )
     mock_send_broadcast_provider_message = mocker.patch(
         'app.celery.broadcast_message_tasks.send_broadcast_provider_message',
     )
@@ -57,9 +49,6 @@ def test_send_broadcast_event_queues_up_for_active_providers(mocker, notify_api,
         call(kwargs={'broadcast_event_id': event.id, 'provider': 'vodafone'}, queue='broadcast-tasks')
     ]
 
-    # we're on test env so this isn't called
-    assert mock_send_ticket_to_zendesk.called is False
-
 
 @pytest.mark.parametrize('message_status', [
     BroadcastStatusType.BROADCASTING,
@@ -71,10 +60,6 @@ def test_send_broadcast_event_calls_publish_govuk_alerts_task(
     template = create_template(sample_broadcast_service, BROADCAST_TYPE)
     broadcast_message = create_broadcast_message(template, status=message_status)
     event = create_broadcast_event(broadcast_message)
-    mocker.patch(
-        'app.celery.broadcast_message_tasks.zendesk_client.send_ticket_to_zendesk',
-        autospec=True,
-    )
     mocker.patch(
         'app.celery.broadcast_message_tasks.send_broadcast_provider_message',
     )
@@ -137,104 +122,6 @@ def test_send_broadcast_event_does_nothing_if_provider_set_on_service_isnt_enabl
     assert mock_send_broadcast_provider_message.apply_async.called is False
 
 
-@pytest.mark.parametrize('area_data,expected_message', [
-    ({'names': ['England', 'Scotland']}, ['England', 'Scotland']),
-    ({}, [])
-])
-def test_send_broadcast_event_creates_zendesk(
-    area_data,
-    expected_message,
-    mocker,
-    notify_api,
-    sample_broadcast_service
-):
-    template = create_template(sample_broadcast_service, BROADCAST_TYPE)
-    broadcast_message = create_broadcast_message(
-        template,
-        status=BroadcastStatusType.BROADCASTING,
-        areas={**area_data, 'simple_polygons': []},
-    )
-    event = create_broadcast_event(broadcast_message)
-    mock_create_ticket = mocker.spy(NotifySupportTicket, '__init__')
-    mocker.patch('app.celery.broadcast_message_tasks.notify_celery.send_task')
-
-    mock_send_ticket_to_zendesk = mocker.patch(
-        'app.celery.broadcast_message_tasks.zendesk_client.send_ticket_to_zendesk',
-        autospec=True,
-    )
-
-    mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_provider_message')
-
-    with set_config(notify_api, 'NOTIFY_ENVIRONMENT', 'live'):
-        send_broadcast_event(event.id)
-
-    mock_create_ticket.assert_called_once_with(
-        ANY,
-        subject='Live broadcast sent',
-        message=ANY,
-        ticket_type='incident',
-        technical_ticket=True,
-        org_id=current_app.config['BROADCAST_ORGANISATION_ID'],
-        org_type='central',
-        service_id=str(sample_broadcast_service.id)
-    )
-    ticket_message = mock_create_ticket.call_args_list[0][1]['message']
-
-    assert str(broadcast_message.id) in ticket_message
-    assert 'channel severe' in ticket_message
-    assert f"areas {expected_message}" in ticket_message
-    # the start of the content from the broadcast template
-    assert "Dear Sir/Madam" in ticket_message
-
-    mock_send_ticket_to_zendesk.assert_called_once()
-
-
-def test_send_broadcast_event_doesnt_create_zendesk_when_cancelling(mocker, notify_api, sample_broadcast_service):
-    template = create_template(sample_broadcast_service, BROADCAST_TYPE)
-    broadcast_message = create_broadcast_message(
-        template,
-        status=BroadcastStatusType.BROADCASTING,
-        areas={'areas': ['wd20-S13002775', 'wd20-S13002773'], 'simple_polygons': []},
-    )
-    create_broadcast_event(broadcast_message, message_type=BroadcastEventMessageType.ALERT)
-    cancel_event = create_broadcast_event(broadcast_message, message_type=BroadcastEventMessageType.CANCEL)
-
-    mocker.patch('app.celery.broadcast_message_tasks.notify_celery.send_task')
-
-    mock_send_ticket_to_zendesk = mocker.patch(
-        'app.celery.broadcast_message_tasks.zendesk_client.send_ticket_to_zendesk',
-        autospec=True,
-    )
-    mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_provider_message')
-
-    with set_config(notify_api, 'NOTIFY_ENVIRONMENT', 'live'):
-        send_broadcast_event(cancel_event.id)
-
-    assert mock_send_ticket_to_zendesk.called is False
-
-
-def test_send_broadcast_event_doesnt_create_zendesk_on_staging(mocker, notify_api, sample_broadcast_service):
-    template = create_template(sample_broadcast_service, BROADCAST_TYPE)
-    broadcast_message = create_broadcast_message(template, status=BroadcastStatusType.BROADCASTING)
-    event = create_broadcast_event(broadcast_message)
-
-    mocker.patch('app.celery.broadcast_message_tasks.notify_celery.send_task')
-
-    mock_send_ticket_to_zendesk = mocker.patch(
-        'app.celery.broadcast_message_tasks.zendesk_client.send_ticket_to_zendesk',
-        autospec=True,
-    )
-    mock_send_broadcast_provider_message = mocker.patch(
-        'app.celery.broadcast_message_tasks.send_broadcast_provider_message',
-    )
-
-    with set_config(notify_api, 'NOTIFY_ENVIRONMENT', 'staging'):
-        send_broadcast_event(event.id)
-
-    assert mock_send_broadcast_provider_message.apply_async.called is True
-    assert mock_send_ticket_to_zendesk.called is False
-
-
 @freeze_time('2020-08-01 12:00')
 @pytest.mark.parametrize('provider,provider_capitalised', [
     ['ee', 'EE'],

tests/app/celery/test_process_sms_client_response_tasks.py

@@ -18,7 +18,7 @@ def test_process_sms_client_response_raises_error_if_reference_is_not_a_valid_uu
             status='000', provider_reference='something-bad', client_name='sms-client')
 
 
-@pytest.mark.parametrize('client_name', ('Firetext', 'MMG'))
+@pytest.mark.parametrize('client_name', ('Firetext', 'MMG', 'Reach'))
 def test_process_sms_response_raises_client_exception_for_unknown_status(
     sample_notification,
     mocker,
@@ -43,6 +43,9 @@ def test_process_sms_response_raises_client_exception_for_unknown_status(
     ('3', '2', 'MMG', 'delivered', "Delivered to operator"),
     ('4', '27', 'MMG', 'temporary-failure', "Absent Subscriber"),
     ('5', '13', 'MMG', 'permanent-failure', "Sender id blacklisted"),
+    ('TODO-d', None, 'Reach', 'delivered', "TODO: Delivered"),
+    ('TODO-tf', None, 'Reach', 'temporary-failure', "TODO: Temporary failure"),
+    ('TODO-pf', None, 'Reach', 'permanent-failure', "TODO: Permanent failure"),
 ])
 def test_process_sms_client_response_updates_notification_status(
     sample_notification,

tests/app/celery/test_scheduled_tasks.py

@@ -366,9 +366,43 @@ def test_check_job_status_task_does_not_raise_error(sample_template):
 
 
 @freeze_time("2019-05-30 14:00:00")
-def test_check_if_letters_still_pending_virus_check(mocker, sample_letter_template):
-    mock_logger = mocker.patch('app.celery.tasks.current_app.logger.error')
+def test_check_if_letters_still_pending_virus_check_restarts_scan_for_stuck_letters(
+        mocker,
+        sample_letter_template
+):
+    mock_file_exists = mocker.patch('app.aws.s3.file_exists', return_value=True)
     mock_create_ticket = mocker.spy(NotifySupportTicket, '__init__')
+    mock_celery = mocker.patch('app.celery.scheduled_tasks.notify_celery.send_task')
+
+    create_notification(
+        template=sample_letter_template,
+        status=NOTIFICATION_PENDING_VIRUS_CHECK,
+        created_at=datetime.utcnow() - timedelta(seconds=5401),
+        reference='one'
+    )
+    expected_filename = 'NOTIFY.ONE.D.2.C.20190530122959.PDF'
+
+    check_if_letters_still_pending_virus_check()
+
+    mock_file_exists.assert_called_once_with('test-letters-scan', expected_filename)
+
+    mock_celery.assert_called_once_with(
+        name=TaskNames.SCAN_FILE,
+        kwargs={'filename': expected_filename},
+        queue=QueueNames.ANTIVIRUS
+    )
+
+    assert mock_create_ticket.called is False
+
+
+@freeze_time("2019-05-30 14:00:00")
+def test_check_if_letters_still_pending_virus_check_raises_zendesk_if_files_cant_be_found(
+        mocker,
+        sample_letter_template
+):
+    mock_file_exists = mocker.patch('app.aws.s3.file_exists', return_value=False)
+    mock_create_ticket = mocker.spy(NotifySupportTicket, '__init__')
+    mock_celery = mocker.patch('app.celery.scheduled_tasks.notify_celery.send_task')
     mock_send_ticket_to_zendesk = mocker.patch(
         'app.celery.scheduled_tasks.zendesk_client.send_ticket_to_zendesk',
         autospec=True,
@@ -391,22 +425,24 @@ def test_check_if_letters_still_pending_virus_check(mocker, sample_letter_templa
 
     check_if_letters_still_pending_virus_check()
 
-    id_references = sorted([(str(notification_1.id), notification_1.reference),
-                            (str(notification_2.id), notification_2.reference)])
+    assert mock_file_exists.call_count == 2
+    mock_file_exists.assert_has_calls([
+        call('test-letters-scan', 'NOTIFY.ONE.D.2.C.20190530122959.PDF'),
+        call('test-letters-scan', 'NOTIFY.TWO.D.2.C.20190529183320.PDF'),
+    ], any_order=True)
+    assert mock_celery.called is False
 
-    message = """2 precompiled letters have been pending-virus-check for over 90 minutes. Follow runbook to resolve:
-            https://github.com/alphagov/notifications-manuals/wiki/Support-Runbook#Deal-with-letter-pending-virus-scan-for-90-minutes.
-            Notifications: {}""".format(id_references)
-
-    mock_logger.assert_called_once_with(message)
     mock_create_ticket.assert_called_once_with(
         ANY,
         subject='[test] Letters still pending virus check',
-        message=message,
+        message=ANY,
         ticket_type='incident',
         technical_ticket=True,
         ticket_categories=['notify_letters']
     )
+    assert '2 precompiled letters have been pending-virus-check' in mock_create_ticket.call_args.kwargs['message']
+    assert f'{(str(notification_1.id), notification_1.reference)}' in mock_create_ticket.call_args.kwargs['message']
+    assert f'{(str(notification_2.id), notification_2.reference)}' in mock_create_ticket.call_args.kwargs['message']
     mock_send_ticket_to_zendesk.assert_called_once()
 
 

tests/app/clients/test_firetext.py

@@ -2,11 +2,9 @@ from urllib.parse import parse_qs
 
 import pytest
 import requests_mock
-from requests import HTTPError
 from requests.exceptions import ConnectTimeout, ReadTimeout
 
 from app.clients.sms.firetext import (
-    FiretextClientResponseException,
     SmsClientResponseException,
     get_firetext_responses,
 )
@@ -36,7 +34,7 @@ def test_get_firetext_responses_raises_KeyError_if_unrecognised_status_code():
     assert '99' in str(e.value)
 
 
-def test_send_sms_successful_returns_firetext_response(mocker, mock_firetext_client):
+def test_try_send_sms_successful_returns_firetext_response(mocker, mock_firetext_client):
     to = content = reference = 'foo'
     response_dict = {
         'data': [],
@@ -47,7 +45,7 @@ def test_send_sms_successful_returns_firetext_response(mocker, mock_firetext_cli
 
     with requests_mock.Mocker() as request_mock:
         request_mock.post('https://example.com/firetext', json=response_dict, status_code=200)
-        response = mock_firetext_client.send_sms(to, content, reference, False)
+        response = mock_firetext_client.try_send_sms(to, content, reference, False, 'sender')
 
     response_json = response.json()
     assert response.status_code == 200
@@ -55,7 +53,7 @@ def test_send_sms_successful_returns_firetext_response(mocker, mock_firetext_cli
     assert response_json['description'] == 'SMS successfully queued'
 
 
-def test_send_sms_calls_firetext_correctly(mocker, mock_firetext_client):
+def test_try_send_sms_calls_firetext_correctly(mocker, mock_firetext_client):
     to = '+447234567890'
     content = 'my message'
     reference = 'my reference'
@@ -65,7 +63,7 @@ def test_send_sms_calls_firetext_correctly(mocker, mock_firetext_client):
 
     with requests_mock.Mocker() as request_mock:
         request_mock.post('https://example.com/firetext', json=response_dict, status_code=200)
-        mock_firetext_client.send_sms(to, content, reference, False)
+        mock_firetext_client.try_send_sms(to, content, reference, False, 'bar')
 
     assert request_mock.call_count == 1
     assert request_mock.request_history[0].url == 'https://example.com/firetext'
@@ -79,7 +77,7 @@ def test_send_sms_calls_firetext_correctly(mocker, mock_firetext_client):
     assert request_args['reference'][0] == reference
 
 
-def test_send_sms_calls_firetext_correctly_for_international(mocker, mock_firetext_client):
+def test_try_send_sms_calls_firetext_correctly_for_international(mocker, mock_firetext_client):
     to = '+607234567890'
     content = 'my message'
     reference = 'my reference'
@@ -89,7 +87,7 @@ def test_send_sms_calls_firetext_correctly_for_international(mocker, mock_firete
 
     with requests_mock.Mocker() as request_mock:
         request_mock.post('https://example.com/firetext', json=response_dict, status_code=200)
-        mock_firetext_client.send_sms(to, content, reference, True)
+        mock_firetext_client.try_send_sms(to, content, reference, True, 'bar')
 
     assert request_mock.call_count == 1
     assert request_mock.request_history[0].url == 'https://example.com/firetext'
@@ -103,7 +101,7 @@ def test_send_sms_calls_firetext_correctly_for_international(mocker, mock_firete
     assert request_args['reference'][0] == reference
 
 
-def test_send_sms_raises_if_firetext_rejects(mocker, mock_firetext_client):
+def test_try_send_sms_raises_if_firetext_rejects(mocker, mock_firetext_client):
     to = content = reference = 'foo'
     response_dict = {
         'data': [],
@@ -114,60 +112,48 @@ def test_send_sms_raises_if_firetext_rejects(mocker, mock_firetext_client):
 
     with pytest.raises(SmsClientResponseException) as exc, requests_mock.Mocker() as request_mock:
         request_mock.post('https://example.com/firetext', json=response_dict, status_code=200)
-        mock_firetext_client.send_sms(to, content, reference, False)
+        mock_firetext_client.try_send_sms(to, content, reference, False, 'sender')
 
-    assert exc.value.status_code == 200
-    assert '"description": "Some kind of error"' in exc.value.text
-    assert '"code": 1' in exc.value.text
+    assert "Invalid response JSON" in str(exc.value)
 
 
-def test_send_sms_raises_if_firetext_rejects_with_unexpected_data(mocker, mock_firetext_client):
+def test_try_send_sms_raises_if_firetext_rejects_with_unexpected_data(mocker, mock_firetext_client):
     to = content = reference = 'foo'
     response_dict = {"something": "gone bad"}
 
     with pytest.raises(SmsClientResponseException) as exc, requests_mock.Mocker() as request_mock:
         request_mock.post('https://example.com/firetext', json=response_dict, status_code=400)
-        mock_firetext_client.send_sms(to, content, reference, False)
+        mock_firetext_client.try_send_sms(to, content, reference, False, 'sender')
 
-    assert exc.value.status_code == 400
-    assert exc.value.text == '{"something": "gone bad"}'
-    assert type(exc.value.exception) == HTTPError
+    assert "Request failed" in str(exc.value)
 
 
-def test_send_sms_override_configured_shortcode_with_sender(mocker, mock_firetext_client):
-    to = '+447234567890'
-    content = 'my message'
-    reference = 'my reference'
-    response_dict = {
-        'code': 0,
-    }
-    sender = 'fromservice'
+def test_try_send_sms_raises_if_firetext_fails_to_return_json(notify_api, mock_firetext_client):
+    to = content = reference = 'foo'
+    response_dict = 'NOT AT ALL VALID JSON {"key" : "value"}}'
 
-    with requests_mock.Mocker() as request_mock:
-        request_mock.post('https://example.com/firetext', json=response_dict, status_code=200)
-        mock_firetext_client.send_sms(to, content, reference, False, sender=sender)
+    with pytest.raises(SmsClientResponseException) as exc, requests_mock.Mocker() as request_mock:
+        request_mock.post('https://example.com/firetext', text=response_dict, status_code=200)
+        mock_firetext_client.try_send_sms(to, content, reference, False, 'sender')
 
-    request_args = parse_qs(request_mock.request_history[0].text)
-    assert request_args['from'][0] == 'fromservice'
+    assert "Invalid response JSON" in str(exc.value)
 
 
-def test_send_sms_raises_if_firetext_rejects_with_connect_timeout(rmock, mock_firetext_client):
+def test_try_send_sms_raises_if_firetext_rejects_with_connect_timeout(rmock, mock_firetext_client):
     to = content = reference = 'foo'
 
-    with pytest.raises(FiretextClientResponseException) as exc:
+    with pytest.raises(SmsClientResponseException) as exc:
         rmock.register_uri('POST', 'https://example.com/firetext', exc=ConnectTimeout)
-        mock_firetext_client.send_sms(to, content, reference, False)
+        mock_firetext_client.try_send_sms(to, content, reference, False, 'sender')
 
-    assert exc.value.status_code == 504
-    assert exc.value.text == 'Gateway Time-out'
+    assert "Request failed" in str(exc.value)
 
 
-def test_send_sms_raises_if_firetext_rejects_with_read_timeout(rmock, mock_firetext_client):
+def test_try_send_sms_raises_if_firetext_rejects_with_read_timeout(rmock, mock_firetext_client):
     to = content = reference = 'foo'
 
-    with pytest.raises(FiretextClientResponseException) as exc:
+    with pytest.raises(SmsClientResponseException) as exc:
         rmock.register_uri('POST', 'https://example.com/firetext', exc=ReadTimeout)
-        mock_firetext_client.send_sms(to, content, reference, False)
+        mock_firetext_client.try_send_sms(to, content, reference, False, 'sender')
 
-    assert exc.value.status_code == 504
-    assert exc.value.text == 'Gateway Time-out'
+    assert "Request failed" in str(exc.value)

tests/app/clients/test_mmg.py

@@ -1,11 +1,9 @@
 import pytest
 import requests_mock
-from requests import HTTPError
 from requests.exceptions import ConnectTimeout, ReadTimeout
 
 from app import mmg_client
-from app.clients.sms import SmsClientResponseException
-from app.clients.sms.mmg import MMGClientResponseException, get_mmg_responses
+from app.clients.sms.mmg import SmsClientResponseException, get_mmg_responses
 
 
 @pytest.mark.parametrize('detailed_status_code, result', [
@@ -38,20 +36,20 @@ def test_get_mmg_responses_raises_KeyError_if_unrecognised_status_code():
     assert '99' in str(e.value)
 
 
-def test_send_sms_successful_returns_mmg_response(notify_api, mocker):
+def test_try_send_sms_successful_returns_mmg_response(notify_api, mocker):
     to = content = reference = 'foo'
     response_dict = {'Reference': 12345678}
 
     with requests_mock.Mocker() as request_mock:
         request_mock.post('https://example.com/mmg', json=response_dict, status_code=200)
-        response = mmg_client.send_sms(to, content, reference, False)
+        response = mmg_client.try_send_sms(to, content, reference, False, 'sender')
 
     response_json = response.json()
     assert response.status_code == 200
     assert response_json['Reference'] == 12345678
 
 
-def test_send_sms_calls_mmg_correctly(notify_api, mocker):
+def test_try_send_sms_calls_mmg_correctly(notify_api, mocker):
     to = '+447234567890'
     content = 'my message'
     reference = 'my reference'
@@ -59,7 +57,7 @@ def test_send_sms_calls_mmg_correctly(notify_api, mocker):
 
     with requests_mock.Mocker() as request_mock:
         request_mock.post('https://example.com/mmg', json=response_dict, status_code=200)
-        mmg_client.send_sms(to, content, reference, False)
+        mmg_client.try_send_sms(to, content, reference, False, 'testing')
 
     assert request_mock.call_count == 1
     assert request_mock.request_history[0].url == 'https://example.com/mmg'
@@ -74,7 +72,7 @@ def test_send_sms_calls_mmg_correctly(notify_api, mocker):
     assert request_args['multi'] is True
 
 
-def test_send_sms_raises_if_mmg_rejects(notify_api, mocker):
+def test_try_send_sms_raises_if_mmg_rejects(notify_api, mocker):
     to = content = reference = 'foo'
     response_dict = {
         'Error': 206,
@@ -83,59 +81,37 @@ def test_send_sms_raises_if_mmg_rejects(notify_api, mocker):
 
     with pytest.raises(SmsClientResponseException) as exc, requests_mock.Mocker() as request_mock:
         request_mock.post('https://example.com/mmg', json=response_dict, status_code=400)
-        mmg_client.send_sms(to, content, reference, False)
+        mmg_client.try_send_sms(to, content, reference, False, 'sender')
 
-    assert exc.value.status_code == 400
-    assert '"Error": 206' in exc.value.text
-    assert '"Description": "Some kind of error"' in exc.value.text
-    assert type(exc.value.exception) == HTTPError
+    assert "Request failed" in str(exc.value)
 
 
-def test_send_sms_override_configured_shortcode_with_sender(notify_api, mocker):
-    to = '+447234567890'
-    content = 'my message'
-    reference = 'my reference'
-    response_dict = {'Reference': 12345678}
-    sender = 'fromservice'
-
-    with requests_mock.Mocker() as request_mock:
-        request_mock.post('https://example.com/mmg', json=response_dict, status_code=200)
-        mmg_client.send_sms(to, content, reference, False, sender=sender)
-
-    request_args = request_mock.request_history[0].json()
-    assert request_args['sender'] == 'fromservice'
-
-
-def test_send_sms_raises_if_mmg_fails_to_return_json(notify_api, mocker):
+def test_try_send_sms_raises_if_mmg_fails_to_return_json(notify_api, mocker):
     to = content = reference = 'foo'
     response_dict = 'NOT AT ALL VALID JSON {"key" : "value"}}'
 
     with pytest.raises(SmsClientResponseException) as exc, requests_mock.Mocker() as request_mock:
         request_mock.post('https://example.com/mmg', text=response_dict, status_code=200)
-        mmg_client.send_sms(to, content, reference, False)
+        mmg_client.try_send_sms(to, content, reference, False, 'sender')
 
-    assert 'Code 200 text NOT AT ALL VALID JSON {"key" : "value"}} exception Expecting value: line 1 column 1 (char 0)' in str(exc.value)  # noqa
-    assert exc.value.status_code == 200
-    assert exc.value.text == 'NOT AT ALL VALID JSON {"key" : "value"}}'
+    assert "Invalid response JSON" in str(exc.value)
 
 
-def test_send_sms_raises_if_mmg_rejects_with_connect_timeout(rmock):
+def test_try_send_sms_raises_if_mmg_rejects_with_connect_timeout(rmock):
     to = content = reference = 'foo'
 
-    with pytest.raises(MMGClientResponseException) as exc:
+    with pytest.raises(SmsClientResponseException) as exc:
         rmock.register_uri('POST', 'https://example.com/mmg', exc=ConnectTimeout)
-        mmg_client.send_sms(to, content, reference, False)
+        mmg_client.try_send_sms(to, content, reference, False, 'sender')
 
-    assert exc.value.status_code == 504
-    assert exc.value.text == 'Gateway Time-out'
+    assert "Request failed" in str(exc.value)
 
 
-def test_send_sms_raises_if_mmg_rejects_with_read_timeout(rmock):
+def test_try_send_sms_raises_if_mmg_rejects_with_read_timeout(rmock):
     to = content = reference = 'foo'
 
-    with pytest.raises(MMGClientResponseException) as exc:
+    with pytest.raises(SmsClientResponseException) as exc:
         rmock.register_uri('POST', 'https://example.com/mmg', exc=ReadTimeout)
-        mmg_client.send_sms(to, content, reference, False)
+        mmg_client.try_send_sms(to, content, reference, False, 'sender')
 
-    assert exc.value.status_code == 504
-    assert exc.value.text == 'Gateway Time-out'
+    assert "Request failed" in str(exc.value)

tests/app/clients/test_reach.py

@@ -0,0 +1,84 @@
+import pytest
+import requests_mock
+from requests.exceptions import ConnectTimeout, ReadTimeout
+
+from app import reach_client
+from app.clients.sms import SmsClientResponseException
+
+# TODO: tests for get_reach_responses
+
+
+def test_try_send_sms_successful_returns_reach_response(notify_api, mocker):
+    to = content = reference = 'foo'
+    response_dict = {}  # TODO
+
+    with requests_mock.Mocker() as request_mock:
+        request_mock.post('https://example.com/reach', json=response_dict, status_code=200)
+        response = reach_client.try_send_sms(to, content, reference, False, 'sender')
+
+    # response_json = response.json()
+    assert response.status_code == 200
+    # TODO: assertions
+
+
+def test_try_send_sms_calls_reach_correctly(notify_api, mocker):
+    to = '+447234567890'
+    content = 'my message'
+    reference = 'my reference'
+    response_dict = {}  # TODO
+
+    with requests_mock.Mocker() as request_mock:
+        request_mock.post('https://example.com/reach', json=response_dict, status_code=200)
+        reach_client.try_send_sms(to, content, reference, False, 'sender')
+
+    assert request_mock.call_count == 1
+    assert request_mock.request_history[0].url == 'https://example.com/reach'
+    assert request_mock.request_history[0].method == 'POST'
+
+    # request_args = request_mock.request_history[0].json()
+    # TODO: assertions
+
+
+def test_try_send_sms_raises_if_reach_rejects(notify_api, mocker):
+    to = content = reference = 'foo'
+    response_dict = {
+        'Error': 206,
+        'Description': 'Some kind of error'
+    }
+
+    with pytest.raises(SmsClientResponseException) as exc, requests_mock.Mocker() as request_mock:
+        request_mock.post('https://example.com/reach', json=response_dict, status_code=400)
+        reach_client.try_send_sms(to, content, reference, False, 'sender')
+
+    assert "Request failed" in str(exc)
+
+
+def test_try_send_sms_raises_if_reach_fails_to_return_json(notify_api, mocker):
+    to = content = reference = 'foo'
+    response_dict = 'NOT AT ALL VALID JSON {"key" : "value"}}'
+
+    with pytest.raises(SmsClientResponseException) as exc, requests_mock.Mocker() as request_mock:
+        request_mock.post('https://example.com/reach', text=response_dict, status_code=200)
+        reach_client.try_send_sms(to, content, reference, False, 'sender')
+
+    assert 'Invalid response JSON' in str(exc.value)
+
+
+def test_try_send_sms_raises_if_reach_rejects_with_connect_timeout(rmock):
+    to = content = reference = 'foo'
+
+    with pytest.raises(SmsClientResponseException) as exc:
+        rmock.register_uri('POST', 'https://example.com/reach', exc=ConnectTimeout)
+        reach_client.try_send_sms(to, content, reference, False, 'sender')
+
+    assert 'Request failed' in str(exc.value)
+
+
+def test_try_send_sms_raises_if_reach_rejects_with_read_timeout(rmock):
+    to = content = reference = 'foo'
+
+    with pytest.raises(SmsClientResponseException) as exc:
+        rmock.register_uri('POST', 'https://example.com/reach', exc=ReadTimeout)
+        reach_client.try_send_sms(to, content, reference, False, 'sender')
+
+    assert 'Request failed' in str(exc.value)

tests/app/clients/test_sms.py

@@ -0,0 +1,66 @@
+import pytest
+
+from app import statsd_client
+from app.clients.sms import SmsClient, SmsClientResponseException
+
+
+@pytest.fixture
+def fake_client(notify_api):
+    class FakeSmsClient(SmsClient):
+        @property
+        def name(self):
+            return 'fake'
+
+    fake_client = FakeSmsClient()
+    fake_client.init_app(notify_api, statsd_client)
+    return fake_client
+
+
+def test_send_sms(fake_client, mocker):
+    mock_send = mocker.patch.object(fake_client, 'try_send_sms')
+
+    fake_client.send_sms(
+        to='to',
+        content='content',
+        reference='reference',
+        international=False,
+        sender=None,
+    )
+
+    mock_send.assert_called_with(
+        'to', 'content', 'reference', False, 'testing'
+    )
+
+
+def test_send_sms_error(fake_client, mocker):
+    mocker.patch.object(
+        fake_client, 'try_send_sms', side_effect=SmsClientResponseException('error')
+    )
+
+    with pytest.raises(SmsClientResponseException):
+        fake_client.send_sms(
+            to='to',
+            content='content',
+            reference='reference',
+            international=False,
+            sender=None,
+        )
+
+
+def test_send_sms_override_configured_shortcode_with_sender(
+    fake_client,
+    mocker
+):
+    mock_send = mocker.patch.object(fake_client, 'try_send_sms')
+
+    fake_client.send_sms(
+        to='to',
+        content='content',
+        reference='reference',
+        international=False,
+        sender='sender'
+    )
+
+    mock_send.assert_called_with(
+        'to', 'content', 'reference', False, 'sender'
+    )

tests/app/dao/test_annual_billing_dao.py

@@ -66,7 +66,12 @@ def test_dao_update_annual_billing_for_future_years(notify_db_session, sample_se
                           ('other', 2020, 25000),
                           (None, 2020, 25000),
                           ('central', 2019, 250000),
-                          ('school_or_college', 2022, 10000)
+                          ('school_or_college', 2022, 10000),
+                          ('central', 2022, 40000),
+                          ('local', 2022, 20000),
+                          ('nhs_local', 2022, 20000),
+                          ('emergency_service', 2022, 20000),
+                          ('central', 2023, 40000),
                           ])
 def test_set_default_free_allowance_for_service(notify_db_session, org_type, year, expected_default):
 

tests/app/dao/test_fact_notification_status_dao.py

@@ -611,6 +611,7 @@ def test_get_total_notifications_for_date_range(sample_service):
     assert results[0] == ("2021-03-01", 15, 20, 3)
 
 
+@freeze_time('2022-03-31T18:00:00')
 @pytest.mark.parametrize('created_at_utc,process_day,expected_count', [
     # Clocks change on the 27th of March 2022, so the query needs to look at the
     # time range 00:00 - 23:00 (UTC) thereafter.

tests/app/dao/test_provider_details_dao.py

@@ -40,7 +40,7 @@ def set_primary_sms_provider(identifier):
 
 def test_can_get_sms_non_international_providers(notify_db_session):
     sms_providers = get_provider_details_by_notification_type('sms')
-    assert len(sms_providers) == 2
+    assert len(sms_providers) > 0
     assert all('sms' == prov.notification_type for prov in sms_providers)
 
 
@@ -53,8 +53,8 @@ def test_can_get_sms_international_providers(notify_db_session):
 
 def test_can_get_sms_providers_in_order_of_priority(notify_db_session):
     providers = get_provider_details_by_notification_type('sms', False)
-
-    assert providers[0].priority < providers[1].priority
+    priorities = [provider.priority for provider in providers]
+    assert priorities == sorted(priorities)
 
 
 def test_can_get_email_providers_in_order_of_priority(notify_db_session):
@@ -234,7 +234,7 @@ def test_reduce_sms_provider_priority_does_nothing_if_providers_have_recently_ch
     mocker,
     restore_provider_details,
 ):
-    mock_get_providers = mocker.patch('app.dao.provider_details_dao._get_sms_providers_for_update', return_value=None)
+    mock_get_providers = mocker.patch('app.dao.provider_details_dao._get_sms_providers_for_update', return_value=[])
     mock_adjust = mocker.patch('app.dao.provider_details_dao._adjust_provider_priority')
 
     dao_reduce_sms_provider_priority('firetext', time_threshold=timedelta(minutes=5))
@@ -243,6 +243,20 @@ def test_reduce_sms_provider_priority_does_nothing_if_providers_have_recently_ch
     assert mock_adjust.called is False
 
 
+def test_reduce_sms_provider_priority_does_nothing_if_there_is_only_one_active_provider(
+    mocker,
+    restore_provider_details,
+):
+    firetext = get_provider_details_by_identifier('firetext')
+    firetext.active = False
+
+    mock_adjust = mocker.patch('app.dao.provider_details_dao._adjust_provider_priority')
+
+    dao_reduce_sms_provider_priority('firetext', time_threshold=timedelta(minutes=5))
+
+    assert mock_adjust.called is False
+
+
 @pytest.mark.parametrize('existing_mmg, existing_firetext, new_mmg, new_firetext', [
     (50, 50, 60, 40),  # not just 50/50 - 60/40 specifically
     (65, 35, 60, 40),  # doesn't overshoot if there's less than 10 difference
@@ -318,27 +332,26 @@ def test_dao_get_provider_stats(notify_db_session):
     create_ft_billing('2018-06-15', sms_template_1, provider='firetext', billable_unit=1)
     create_ft_billing('2018-06-28', sms_template_2, provider='mmg', billable_unit=2)
 
-    result = dao_get_provider_stats()
+    results = dao_get_provider_stats()
 
-    assert len(result) == 4
+    assert len(results) > 0
 
-    assert result[0].identifier == 'ses'
-    assert result[0].display_name == 'AWS SES'
-    assert result[0].created_by_name is None
-    assert result[0].current_month_billable_sms == 0
+    ses = next(result for result in results if result.identifier == 'ses')
+    firetext = next(result for result in results if result.identifier == 'firetext')
+    mmg = next(result for result in results if result.identifier == 'mmg')
 
-    assert result[1].identifier == 'firetext'
-    assert result[1].notification_type == 'sms'
-    assert result[1].supports_international is False
-    assert result[1].active is True
-    assert result[1].current_month_billable_sms == 5
+    assert ses.display_name == 'AWS SES'
+    assert ses.created_by_name is None
+    assert ses.current_month_billable_sms == 0
 
-    assert result[2].identifier == 'mmg'
-    assert result[2].display_name == 'MMG'
-    assert result[2].supports_international is True
-    assert result[2].active is True
-    assert result[2].current_month_billable_sms == 4
+    assert firetext.display_name == 'Firetext'
+    assert firetext.notification_type == 'sms'
+    assert firetext.supports_international is False
+    assert firetext.active is True
+    assert firetext.current_month_billable_sms == 5
 
-    assert result[3].identifier == 'dvla'
-    assert result[3].current_month_billable_sms == 0
-    assert result[3].supports_international is False
+    assert mmg.identifier == 'mmg'
+    assert mmg.display_name == 'MMG'
+    assert mmg.supports_international is True
+    assert mmg.active is True
+    assert mmg.current_month_billable_sms == 4

tests/app/delivery/test_send_to_providers.py

@@ -54,7 +54,7 @@ def test_provider_to_use_should_return_random_provider(mocker, notify_db_session
     ret = send_to_providers.provider_to_use('sms', international=False)
 
     mock_choices.assert_called_once_with([mmg, firetext], weights=[25, 75])
-    assert ret.get_name() == 'mmg'
+    assert ret.name == 'mmg'
 
 
 def test_provider_to_use_should_cache_repeated_calls(mocker, notify_db_session):
@@ -89,7 +89,7 @@ def test_provider_to_use_should_only_return_mmg_for_international(
     ret = send_to_providers.provider_to_use('sms', international=True)
 
     mock_choices.assert_called_once_with([mmg], weights=[100])
-    assert ret.get_name() == 'mmg'
+    assert ret.name == 'mmg'
 
 
 def test_provider_to_use_should_only_return_active_providers(mocker, restore_provider_details):
@@ -101,7 +101,7 @@ def test_provider_to_use_should_only_return_active_providers(mocker, restore_pro
     ret = send_to_providers.provider_to_use('sms')
 
     mock_choices.assert_called_once_with([firetext], weights=[100])
-    assert ret.get_name() == 'firetext'
+    assert ret.name == 'firetext'
 
 
 def test_provider_to_use_raises_if_no_active_providers(mocker, restore_provider_details):

tests/app/notifications/test_notifications_letter_callbacks.py

@@ -0,0 +1,99 @@
+import pytest
+from flask import json
+
+
+def dvla_post(client, data):
+    return client.post(
+        path='/notifications/letter/dvla',
+        data=data,
+        headers=[('Content-Type', 'application/json')]
+    )
+
+
+def test_dvla_callback_returns_400_with_invalid_request(client):
+    data = json.dumps({"foo": "bar"})
+    response = dvla_post(client, data)
+    assert response.status_code == 400
+
+
+def test_dvla_callback_autoconfirms_subscription(client, mocker):
+    autoconfirm_mock = mocker.patch('app.notifications.notifications_letter_callback.autoconfirm_subscription')
+
+    data = _sns_confirmation_callback()
+    response = dvla_post(client, data)
+    assert response.status_code == 200
+    assert autoconfirm_mock.called
+
+
+def test_dvla_callback_autoconfirm_does_not_call_update_letter_notifications_task(client, mocker):
+    autoconfirm_mock = mocker.patch('app.notifications.notifications_letter_callback.autoconfirm_subscription')
+    update_task = \
+        mocker.patch('app.notifications.notifications_letter_callback.update_letter_notifications_statuses.apply_async')
+
+    data = _sns_confirmation_callback()
+    response = dvla_post(client, data)
+
+    assert response.status_code == 200
+    assert autoconfirm_mock.called
+    assert not update_task.called
+
+
+def test_dvla_callback_calls_does_not_update_letter_notifications_task_with_invalid_file_type(client, mocker):
+    update_task = \
+        mocker.patch('app.notifications.notifications_letter_callback.update_letter_notifications_statuses.apply_async')
+
+    data = _sample_sns_s3_callback("bar.txt")
+    response = dvla_post(client, data)
+
+    assert response.status_code == 200
+    assert not update_task.called
+
+
+@pytest.mark.parametrize("filename",
+                         ['Notify-20170411153023-rs.txt', 'Notify-20170411153023-rsp.txt'])
+def test_dvla_rs_and_rsp_txt_file_callback_calls_update_letter_notifications_task(client, mocker, filename):
+    update_task = mocker.patch(
+        'app.notifications.notifications_letter_callback.update_letter_notifications_statuses.apply_async')
+    daily_sorted_counts_task = mocker.patch(
+        'app.notifications.notifications_letter_callback.record_daily_sorted_counts.apply_async')
+    data = _sample_sns_s3_callback(filename)
+    response = dvla_post(client, data)
+
+    assert response.status_code == 200
+    assert update_task.called
+    update_task.assert_called_with([filename], queue='notify-internal-tasks')
+    daily_sorted_counts_task.assert_called_with([filename], queue='notify-internal-tasks')
+
+
+def test_dvla_ack_calls_does_not_call_letter_notifications_task(client, mocker):
+    update_task = mocker.patch(
+        'app.notifications.notifications_letter_callback.update_letter_notifications_statuses.apply_async')
+    daily_sorted_counts_task = mocker.patch(
+        'app.notifications.notifications_letter_callback.record_daily_sorted_counts.apply_async')
+    data = _sample_sns_s3_callback('bar.ack.txt')
+    response = dvla_post(client, data)
+
+    assert response.status_code == 200
+    update_task.assert_not_called()
+    daily_sorted_counts_task.assert_not_called()
+
+
+def _sample_sns_s3_callback(filename):
+    message_contents = '''{"Records":[{"eventVersion":"2.0","eventSource":"aws:s3","awsRegion":"eu-west-1","eventTime":"2017-05-16T11:38:41.073Z","eventName":"ObjectCreated:Put","userIdentity":{"principalId":"some-p-id"},"requestParameters":{"sourceIPAddress":"8.8.8.8"},"responseElements":{"x-amz-request-id":"some-r-id","x-amz-id-2":"some-x-am-id"},"s3":{"s3SchemaVersion":"1.0","configurationId":"some-c-id","bucket":{"name":"some-bucket","ownerIdentity":{"principalId":"some-p-id"},"arn":"some-bucket-arn"},
+            "object":{"key":"%s"}}}]}''' % (filename)  # noqa
+    return json.dumps({
+        "SigningCertURL": "foo.pem",
+        "UnsubscribeURL": "bar",
+        "Signature": "some-signature",
+        "Type": "Notification",
+        "Timestamp": "2016-05-03T08:35:12.884Z",
+        "SignatureVersion": "1",
+        "MessageId": "6adbfe0a-d610-509a-9c47-af894e90d32d",
+        "Subject": "Amazon S3 Notification",
+        "TopicArn": "sample-topic-arn",
+        "Message": message_contents
+    })
+
+
+def _sns_confirmation_callback():
+    return b'{\n    "Type": "SubscriptionConfirmation",\n    "MessageId": "165545c9-2a5c-472c-8df2-7ff2be2b3b1b",\n    "Token": "2336412f37fb687f5d51e6e241d09c805a5a57b30d712f794cc5f6a988666d92768dd60a747ba6f3beb71854e285d6ad02428b09ceece29417f1f02d609c582afbacc99c583a916b9981dd2728f4ae6fdb82efd087cc3b7849e05798d2d2785c03b0879594eeac82c01f235d0e717736",\n    "TopicArn": "arn:aws:sns:us-west-2:123456789012:MyTopic",\n    "Message": "You have chosen to subscribe to the topic arn:aws:sns:us-west-2:123456789012:MyTopic.\\nTo confirm the subscription, visit the SubscribeURL included in this message.",\n    "SubscribeURL": "https://sns.us-west-2.amazonaws.com/?Action=ConfirmSubscription&TopicArn=arn:aws:sns:us-west-2:123456789012:MyTopic&Token=2336412f37fb687f5d51e6e241d09c805a5a57b30d712f794cc5f6a988666d92768dd60a747ba6f3beb71854e285d6ad02428b09ceece29417f1f02d609c582afbacc99c583a916b9981dd2728f4ae6fdb82efd087cc3b7849e05798d2d2785c03b0879594eeac82c01f235d0e717736",\n    "Timestamp": "2012-04-26T20:45:04.751Z",\n    "SignatureVersion": "1",\n    "Signature": "EXAMPLEpH+DcEwjAPg8O9mY8dReBSwksfg2S7WKQcikcNKWLQjwu6A4VbeS0QHVCkhRS7fUQvi2egU3N858fiTDN6bkkOxYDVrY0Ad8L10Hs3zH81mtnPk5uvvolIC1CXGu43obcgFxeL3khZl8IKvO61GWB6jI9b5+gLPoBc1Q=",\n    "SigningCertURL": "https://sns.us-west-2.amazonaws.com/SimpleNotificationService-f3ecfb7224c7233fe7bb5f59f96de52f.pem"\n}'  # noqa

tests/app/notifications/test_notifications_sms_callbacks.py

@@ -1,4 +1,3 @@
-import pytest
 from flask import json
 
 from app.notifications.notifications_sms_callback import validate_callback_data
@@ -8,96 +7,21 @@ def firetext_post(client, data):
     return client.post(
         path='/notifications/sms/firetext',
         data=data,
-        headers=[
-            ('Content-Type', 'application/x-www-form-urlencoded'),
-            ('X-Forwarded-For', '203.0.113.195, 70.41.3.18, 150.172.238.178')  # fake IPs
-        ])
+        headers=[('Content-Type', 'application/x-www-form-urlencoded')])
 
 
 def mmg_post(client, data):
     return client.post(
         path='/notifications/sms/mmg',
         data=data,
-        headers=[
-            ('Content-Type', 'application/json'),
-            ('X-Forwarded-For', '203.0.113.195, 70.41.3.18, 150.172.238.178')  # fake IPs
-        ])
+        headers=[('Content-Type', 'application/json')])
 
 
-def dvla_post(client, data):
+def reach_post(client, data):
     return client.post(
-        path='/notifications/letter/dvla',
+        path='/notifications/sms/reach',
         data=data,
-        headers=[('Content-Type', 'application/json')]
-    )
-
-
-def test_dvla_callback_returns_400_with_invalid_request(client):
-    data = json.dumps({"foo": "bar"})
-    response = dvla_post(client, data)
-    assert response.status_code == 400
-
-
-def test_dvla_callback_autoconfirms_subscription(client, mocker):
-    autoconfirm_mock = mocker.patch('app.notifications.notifications_letter_callback.autoconfirm_subscription')
-
-    data = _sns_confirmation_callback()
-    response = dvla_post(client, data)
-    assert response.status_code == 200
-    assert autoconfirm_mock.called
-
-
-def test_dvla_callback_autoconfirm_does_not_call_update_letter_notifications_task(client, mocker):
-    autoconfirm_mock = mocker.patch('app.notifications.notifications_letter_callback.autoconfirm_subscription')
-    update_task = \
-        mocker.patch('app.notifications.notifications_letter_callback.update_letter_notifications_statuses.apply_async')
-
-    data = _sns_confirmation_callback()
-    response = dvla_post(client, data)
-
-    assert response.status_code == 200
-    assert autoconfirm_mock.called
-    assert not update_task.called
-
-
-def test_dvla_callback_calls_does_not_update_letter_notifications_task_with_invalid_file_type(client, mocker):
-    update_task = \
-        mocker.patch('app.notifications.notifications_letter_callback.update_letter_notifications_statuses.apply_async')
-
-    data = _sample_sns_s3_callback("bar.txt")
-    response = dvla_post(client, data)
-
-    assert response.status_code == 200
-    assert not update_task.called
-
-
-@pytest.mark.parametrize("filename",
-                         ['Notify-20170411153023-rs.txt', 'Notify-20170411153023-rsp.txt'])
-def test_dvla_rs_and_rsp_txt_file_callback_calls_update_letter_notifications_task(client, mocker, filename):
-    update_task = mocker.patch(
-        'app.notifications.notifications_letter_callback.update_letter_notifications_statuses.apply_async')
-    daily_sorted_counts_task = mocker.patch(
-        'app.notifications.notifications_letter_callback.record_daily_sorted_counts.apply_async')
-    data = _sample_sns_s3_callback(filename)
-    response = dvla_post(client, data)
-
-    assert response.status_code == 200
-    assert update_task.called
-    update_task.assert_called_with([filename], queue='notify-internal-tasks')
-    daily_sorted_counts_task.assert_called_with([filename], queue='notify-internal-tasks')
-
-
-def test_dvla_ack_calls_does_not_call_letter_notifications_task(client, mocker):
-    update_task = mocker.patch(
-        'app.notifications.notifications_letter_callback.update_letter_notifications_statuses.apply_async')
-    daily_sorted_counts_task = mocker.patch(
-        'app.notifications.notifications_letter_callback.record_daily_sorted_counts.apply_async')
-    data = _sample_sns_s3_callback('bar.ack.txt')
-    response = dvla_post(client, data)
-
-    assert response.status_code == 200
-    update_task.assert_not_called()
-    daily_sorted_counts_task.assert_not_called()
+        headers=[('Content-Type', 'application/json')])
 
 
 def test_firetext_callback_should_not_need_auth(client, mocker):
@@ -218,6 +142,24 @@ def test_mmg_callback_should_return_200_and_call_task_with_valid_data(client, mo
     )
 
 
+# TODO: more tests about edge cases for this provider
+def test_reach_callback_should_return_200_and_call_task_with_valid_data(client, mocker):
+    mock_celery = mocker.patch(
+        'app.notifications.notifications_sms_callback.process_sms_client_response.apply_async')
+    data = json.dumps({"data": "TODO"})
+
+    response = reach_post(client, data)
+
+    assert response.status_code == 200
+    json_data = json.loads(response.data)
+    assert json_data['result'] == 'success'
+
+    mock_celery.assert_called_once_with(
+        ['TODO-d', 'notification_id', 'Reach', 'something'],
+        queue='sms-callbacks',
+    )
+
+
 def test_validate_callback_data_returns_none_when_valid():
     form = {'status': 'good',
             'reference': 'send-sms-code'}
@@ -255,24 +197,3 @@ def test_validate_callback_data_returns_error_for_empty_string():
     result = validate_callback_data(form, fields, client_name)
     assert result is not None
     assert "{} callback failed: {} missing".format(client_name, 'status') in result
-
-
-def _sample_sns_s3_callback(filename):
-    message_contents = '''{"Records":[{"eventVersion":"2.0","eventSource":"aws:s3","awsRegion":"eu-west-1","eventTime":"2017-05-16T11:38:41.073Z","eventName":"ObjectCreated:Put","userIdentity":{"principalId":"some-p-id"},"requestParameters":{"sourceIPAddress":"8.8.8.8"},"responseElements":{"x-amz-request-id":"some-r-id","x-amz-id-2":"some-x-am-id"},"s3":{"s3SchemaVersion":"1.0","configurationId":"some-c-id","bucket":{"name":"some-bucket","ownerIdentity":{"principalId":"some-p-id"},"arn":"some-bucket-arn"},
-            "object":{"key":"%s"}}}]}''' % (filename)  # noqa
-    return json.dumps({
-        "SigningCertURL": "foo.pem",
-        "UnsubscribeURL": "bar",
-        "Signature": "some-signature",
-        "Type": "Notification",
-        "Timestamp": "2016-05-03T08:35:12.884Z",
-        "SignatureVersion": "1",
-        "MessageId": "6adbfe0a-d610-509a-9c47-af894e90d32d",
-        "Subject": "Amazon S3 Notification",
-        "TopicArn": "sample-topic-arn",
-        "Message": message_contents
-    })
-
-
-def _sns_confirmation_callback():
-    return b'{\n    "Type": "SubscriptionConfirmation",\n    "MessageId": "165545c9-2a5c-472c-8df2-7ff2be2b3b1b",\n    "Token": "2336412f37fb687f5d51e6e241d09c805a5a57b30d712f794cc5f6a988666d92768dd60a747ba6f3beb71854e285d6ad02428b09ceece29417f1f02d609c582afbacc99c583a916b9981dd2728f4ae6fdb82efd087cc3b7849e05798d2d2785c03b0879594eeac82c01f235d0e717736",\n    "TopicArn": "arn:aws:sns:us-west-2:123456789012:MyTopic",\n    "Message": "You have chosen to subscribe to the topic arn:aws:sns:us-west-2:123456789012:MyTopic.\\nTo confirm the subscription, visit the SubscribeURL included in this message.",\n    "SubscribeURL": "https://sns.us-west-2.amazonaws.com/?Action=ConfirmSubscription&TopicArn=arn:aws:sns:us-west-2:123456789012:MyTopic&Token=2336412f37fb687f5d51e6e241d09c805a5a57b30d712f794cc5f6a988666d92768dd60a747ba6f3beb71854e285d6ad02428b09ceece29417f1f02d609c582afbacc99c583a916b9981dd2728f4ae6fdb82efd087cc3b7849e05798d2d2785c03b0879594eeac82c01f235d0e717736",\n    "Timestamp": "2012-04-26T20:45:04.751Z",\n    "SignatureVersion": "1",\n    "Signature": "EXAMPLEpH+DcEwjAPg8O9mY8dReBSwksfg2S7WKQcikcNKWLQjwu6A4VbeS0QHVCkhRS7fUQvi2egU3N858fiTDN6bkkOxYDVrY0Ad8L10Hs3zH81mtnPk5uvvolIC1CXGu43obcgFxeL3khZl8IKvO61GWB6jI9b5+gLPoBc1Q=",\n    "SigningCertURL": "https://sns.us-west-2.amazonaws.com/SimpleNotificationService-f3ecfb7224c7233fe7bb5f59f96de52f.pem"\n}'  # noqa

tests/app/organisation/test_rest.py

@@ -508,6 +508,7 @@ def test_post_link_service_to_organisation(admin_request, sample_service):
     assert sample_service.organisation_type == 'central'
 
 
+@freeze_time('2021-09-24 13:30')
 def test_post_link_service_to_organisation_inserts_annual_billing(admin_request, sample_service):
     data = {
         'service_id': str(sample_service.id)
@@ -551,6 +552,7 @@ def test_post_link_service_to_organisation_rollback_service_if_annual_billing_up
     assert len(AnnualBilling.query.all()) == 0
 
 
+@freeze_time('2021-09-24 13:30')
 def test_post_link_service_to_another_org(
         admin_request, sample_service, sample_organisation):
     data = {

tests/app/provider_details/test_rest.py

@@ -10,8 +10,8 @@ from tests.app.db import create_ft_billing
 def test_get_provider_details_returns_all_providers(admin_request, notify_db_session):
     json_resp = admin_request.get('provider_details.get_providers')['provider_details']
 
-    assert len(json_resp) == 4
-    assert {x['identifier'] for x in json_resp} == {'ses', 'firetext', 'mmg', 'dvla'}
+    assert len(json_resp) > 0
+    assert {'ses', 'firetext', 'mmg', 'dvla'} < {x['identifier'] for x in json_resp}
 
 
 def test_get_provider_details_by_id(client, notify_db):
@@ -45,7 +45,7 @@ def test_get_provider_contains_correct_fields(client, sample_template):
         "active", "updated_at", "supports_international",
         "current_month_billable_sms"
     }
-    assert len(json_resp) == 4
+    assert len(json_resp) > 0
     assert allowed_keys == set(json_resp[0].keys())
 
 

tests/app/test_commands.py

@@ -1,10 +1,14 @@
+import pytest
+
 from app.commands import (
     insert_inbound_numbers_from_file,
     local_dev_broadcast_permissions,
+    populate_annual_billing_with_defaults,
 )
 from app.dao.inbound_numbers_dao import dao_get_available_inbound_numbers
 from app.dao.services_dao import dao_add_user_to_service
-from tests.app.db import create_user
+from app.models import AnnualBilling
+from tests.app.db import create_annual_billing, create_service, create_user
 
 
 def test_insert_inbound_numbers_from_file(notify_db_session, notify_api, tmpdir):
@@ -36,3 +40,43 @@ def test_local_dev_broadcast_permissions(
 
     assert len(user.get_permissions(sample_service.id)) == 0
     assert len(user.get_permissions(sample_broadcast_service.id)) > 0
+
+
+@pytest.mark.parametrize("organisation_type, expected_allowance",
+                         [('central', 40000),
+                          ('local', 20000),
+                          ('nhs_gp', 10000)])
+def test_populate_annual_billing_with_defaults(
+        notify_db_session, notify_api, organisation_type, expected_allowance
+):
+    service = create_service(service_name=organisation_type, organisation_type=organisation_type)
+
+    notify_api.test_cli_runner().invoke(
+        populate_annual_billing_with_defaults, ['-y', 2022]
+    )
+
+    results = AnnualBilling.query.filter(
+        AnnualBilling.financial_year_start == 2022,
+        AnnualBilling.service_id == service.id
+    ).all()
+
+    assert len(results) == 1
+    assert results[0].free_sms_fragment_limit == expected_allowance
+
+
+def test_populate_annual_billing_with_defaults_sets_free_allowance_to_zero_if_previous_year_is_zero(
+        notify_db_session, notify_api
+):
+    service = create_service(organisation_type='central')
+    create_annual_billing(service_id=service.id, free_sms_fragment_limit=0, financial_year_start=2021)
+    notify_api.test_cli_runner().invoke(
+        populate_annual_billing_with_defaults, ['-y', 2022]
+    )
+
+    results = AnnualBilling.query.filter(
+        AnnualBilling.financial_year_start == 2022,
+        AnnualBilling.service_id == service.id
+    ).all()
+
+    assert len(results) == 1
+    assert results[0].free_sms_fragment_limit == 0

tests/app/v2/broadcast/test_post_broadcast.py

@@ -124,7 +124,7 @@ def test_valid_post_cap_xml_broadcast_returns_201(
     [True, "cancelled"],
     [False, "rejected"]
 ])
-def test_valid_cancel_broadcast_request_calls_validate_and_update_broadcast_message_status_and_returns_201(
+def test_valid_cancel_broadcast_request_calls_update_broadcast_message_status_and_returns_201(
     client,
     sample_broadcast_service,
     mocker,
@@ -153,7 +153,9 @@ def test_valid_cancel_broadcast_request_calls_validate_and_update_broadcast_mess
     if is_approved:
         broadcast_message.status = 'broadcasting'
 
-    mock_update = mocker.patch('app.v2.broadcast.post_broadcast.validate_and_update_broadcast_message_status')
+    mock_update = mocker.patch(
+        'app.v2.broadcast.post_broadcast.broadcast_utils.update_broadcast_message_status'
+    )
 
     # cancel broadcast
     response_for_cancel = client.post(

tests/app/v2/test_errors.py

@@ -128,7 +128,7 @@ def test_bad_method(app_for_test):
 
             assert response.status_code == 405
 
-            assert response.json == {
+            assert response.get_json(force=True) == {
                 "result": "error",
                 "message": "The method is not allowed for the requested URL."
             }