First draft to purge high volume data on an hourly basis.

This makes the assumption that we do not need NotificationHistory, we will need to ensure that for HighVolumeServices we do not recalculate the stats tables, either in the reporting_tasks nor in the commands.
The strategy to migrate to using this code is still up for design.
We will need to consider:
 - how to purge the data, safely, efficiently.
 - ensure we get the counts right for the stats table, the ft_notification_status table is particularly tricky, because as a notification moves from created -> sending -> delivered the row counts change for each status type. One idea is that we only write the `delivered`, `temporary-failure` and `permanent-failure` status into the table.

This is not production ready but just a draft to get the conversation going.
I also have an idea to stop returning "todays" notification data for HighVolumeServices on the dashboards/usages (except platform_admin) until this code is ready.

app/celery/letters_pdf_tasks.py

@@ -52,7 +52,7 @@ from app.models import (
     NOTIFICATION_TECHNICAL_FAILURE,
     NOTIFICATION_VALIDATION_FAILED,
     NOTIFICATION_VIRUS_SCAN_FAILED,
-    POSTAGE_TYPES)
+)
 from app.cronitor import cronitor
 
 
@@ -136,40 +136,39 @@ def collate_letter_pdfs_to_be_sent():
         hour=17, minute=30, second=0, microsecond=0
     )
 
-    for postage in POSTAGE_TYPES:
-        letters_to_print = get_key_and_size_of_letters_to_be_sent_to_print(print_run_deadline, postage)
+    letters_to_print = get_key_and_size_of_letters_to_be_sent_to_print(print_run_deadline)
 
-        for i, letters in enumerate(group_letters(letters_to_print)):
-            filenames = [letter['Key'] for letter in letters]
+    for i, letters in enumerate(group_letters(letters_to_print)):
+        filenames = [letter['Key'] for letter in letters]
 
-            hash = urlsafe_b64encode(sha512(''.join(filenames).encode()).digest())[:20].decode()
-            # eg NOTIFY.2018-12-31.001.Wjrui5nAvObjPd-3GEL-.ZIP
-            dvla_filename = 'NOTIFY.{date}.{num:03}.{hash}.ZIP'.format(
-                date=print_run_deadline.strftime("%Y-%m-%d"),
-                num=i + 1,
-                hash=hash
-            )
-
-            current_app.logger.info(
-                'Calling task zip-and-send-letter-pdfs for {} pdfs to upload {} with total size {:,} bytes'.format(
-                    len(filenames),
-                    dvla_filename,
-                    sum(letter['Size'] for letter in letters)
-                )
-            )
-            notify_celery.send_task(
-                name=TaskNames.ZIP_AND_SEND_LETTER_PDFS,
-                kwargs={
-                    'filenames_to_zip': filenames,
-                    'upload_filename': dvla_filename
-                },
-                queue=QueueNames.PROCESS_FTP,
-                compression='zlib'
+        hash = urlsafe_b64encode(sha512(''.join(filenames).encode()).digest())[:20].decode()
+        # eg NOTIFY.2018-12-31.001.Wjrui5nAvObjPd-3GEL-.ZIP
+        dvla_filename = 'NOTIFY.{date}.{num:03}.{hash}.ZIP'.format(
+            date=print_run_deadline.strftime("%Y-%m-%d"),
+            num=i + 1,
+            hash=hash
+        )
+
+        current_app.logger.info(
+            'Calling task zip-and-send-letter-pdfs for {} pdfs to upload {} with total size {:,} bytes'.format(
+                len(filenames),
+                dvla_filename,
+                sum(letter['Size'] for letter in letters)
             )
+        )
+        notify_celery.send_task(
+            name=TaskNames.ZIP_AND_SEND_LETTER_PDFS,
+            kwargs={
+                'filenames_to_zip': filenames,
+                'upload_filename': dvla_filename
+            },
+            queue=QueueNames.PROCESS_FTP,
+            compression='zlib'
+        )
 
 
-def get_key_and_size_of_letters_to_be_sent_to_print(print_run_deadline, postage):
-    letters_awaiting_sending = dao_get_letters_to_be_printed(print_run_deadline, postage)
+def get_key_and_size_of_letters_to_be_sent_to_print(print_run_deadline):
+    letters_awaiting_sending = dao_get_letters_to_be_printed(print_run_deadline)
 
     letter_pdfs = []
     for letter in letters_awaiting_sending:
@@ -184,7 +183,7 @@ def get_key_and_size_of_letters_to_be_sent_to_print(print_run_deadline, postage)
             letter_pdfs.append({"Key": letter_file_name, "Size": letter_head['ContentLength']})
         except BotoClientError as e:
             current_app.logger.exception(
-                f"Error getting letter from bucket for notification: {letter.id} with reference: {letter.reference}")
+                f"Error getting letter from bucket for notification: {letter.id} with reference: {letter.reference}", e)
 
     return letter_pdfs
 

app/celery/process_sms_client_response_tasks.py

@@ -1,20 +1,17 @@
 import uuid
-
 from datetime import datetime
+
 from flask import current_app
+from notifications_utils.statsd_decorators import statsd
 from notifications_utils.template import SMSMessageTemplate
 
-from app import statsd_client
+from app import notify_celery, statsd_client
 from app.clients import ClientException
-from app.dao import notifications_dao
 from app.clients.sms.firetext import get_firetext_responses
 from app.clients.sms.mmg import get_mmg_responses
-from app.celery.service_callback_tasks import (
-    send_delivery_status_to_service,
-    create_delivery_status_callback_data,
-)
+from app.celery.service_callback_tasks import send_delivery_status_to_service, create_delivery_status_callback_data
 from app.config import QueueNames
-from app.dao.notifications_dao import dao_update_notification
+from app.dao import notifications_dao
 from app.dao.service_callback_api_dao import get_service_delivery_status_callback_api_for_service
 from app.dao.templates_dao import dao_get_template_by_id
 from app.models import NOTIFICATION_PENDING
@@ -25,39 +22,23 @@ sms_response_mapper = {
 }
 
 
-def validate_callback_data(data, fields, client_name):
-    errors = []
-    for f in fields:
-        if not str(data.get(f, '')):
-            error = "{} callback failed: {} missing".format(client_name, f)
-            errors.append(error)
-    return errors if len(errors) > 0 else None
-
-
-def process_sms_client_response(status, provider_reference, client_name):
-    success = None
-    errors = None
+@notify_celery.task(bind=True, name="process-sms-client-response", max_retries=5, default_retry_delay=300)
+@statsd(namespace="tasks")
+def process_sms_client_response(self, status, provider_reference, client_name):
     # validate reference
-    if provider_reference == 'send-sms-code':
-        success = "{} callback succeeded: send-sms-code".format(client_name)
-        return success, errors
-
     try:
         uuid.UUID(provider_reference, version=4)
-    except ValueError:
-        errors = "{} callback with invalid reference {}".format(client_name, provider_reference)
-        return success, errors
+    except ValueError as e:
+        current_app.logger.exception(f'{client_name} callback with invalid reference {provider_reference}')
+        raise e
 
-    try:
-        response_parser = sms_response_mapper[client_name]
-    except KeyError:
-        return success, 'unknown sms client: {}'.format(client_name)
+    response_parser = sms_response_mapper[client_name]
 
-    # validate  status
+    # validate status
     try:
         notification_status = response_parser(status)
-        current_app.logger.info('{} callback return status of {} for reference: {}'.format(
-            client_name, status, provider_reference)
+        current_app.logger.info(
+            f'{client_name} callback returned status of {status} for reference: {provider_reference}'
         )
     except KeyError:
         _process_for_status(
@@ -65,14 +46,13 @@ def process_sms_client_response(status, provider_reference, client_name):
             client_name=client_name,
             provider_reference=provider_reference
         )
-        raise ClientException("{} callback failed: status {} not found.".format(client_name, status))
+        raise ClientException(f'{client_name} callback failed: status {status} not found.')
 
-    success = _process_for_status(
+    _process_for_status(
         notification_status=notification_status,
         client_name=client_name,
         provider_reference=provider_reference
     )
-    return success, errors
 
 
 def _process_for_status(notification_status, client_name, provider_reference):
@@ -114,11 +94,3 @@ def _process_for_status(notification_status, client_name, provider_reference):
             encrypted_notification = create_delivery_status_callback_data(notification, service_callback_api)
             send_delivery_status_to_service.apply_async([str(notification.id), encrypted_notification],
                                                         queue=QueueNames.CALLBACKS)
-
-    success = "{} callback succeeded. reference {} updated".format(client_name, provider_reference)
-    return success
-
-
-def set_notification_sent_by(notification, client_name):
-    notification.sent_by = client_name
-    dao_update_notification(notification)

app/celery/reporting_tasks.py

@@ -42,6 +42,8 @@ def create_nightly_billing(day_start=None):
 @notify_celery.task(name="create-nightly-billing-for-day")
 @statsd(namespace="tasks")
 def create_nightly_billing_for_day(process_day):
+    # When app.celery.scheduled_tasks.purge_high_volume_notifications starts
+    # we need to exclude HighVolumeServices from the list.
     process_day = datetime.strptime(process_day, "%Y-%m-%d").date()
 
     start = datetime.utcnow()
@@ -62,6 +64,8 @@ def create_nightly_billing_for_day(process_day):
 @cronitor("create-nightly-notification-status")
 @statsd(namespace="tasks")
 def create_nightly_notification_status():
+    # When app.celery.scheduled_tasks.purge_high_volume_notifications starts
+    # we need to exclude HighVolumeServices from the list.
     yesterday = convert_utc_to_bst(datetime.utcnow()).date() - timedelta(days=1)
 
     # email and sms
@@ -84,6 +88,8 @@ def create_nightly_notification_status():
 @notify_celery.task(name="create-nightly-notification-status-for-day")
 @statsd(namespace="tasks")
 def create_nightly_notification_status_for_day(process_day, notification_type):
+    # When app.celery.scheduled_tasks.purge_high_volume_notifications starts
+    # we need to exclude HighVolumeServices from the list.
     process_day = datetime.strptime(process_day, "%Y-%m-%d").date()
 
     start = datetime.utcnow()

app/celery/scheduled_tasks.py

@@ -5,10 +5,11 @@ from datetime import (
 
 from flask import current_app
 from notifications_utils.statsd_decorators import statsd
+from notifications_utils.timezones import convert_utc_to_bst
 from sqlalchemy import and_
 from sqlalchemy.exc import SQLAlchemyError
 
-from app import notify_celery, zendesk_client
+from app import notify_celery, zendesk_client, db
 from app.celery.tasks import (
     process_job,
     get_recipient_csv_and_template_and_sender_id,
@@ -45,7 +46,7 @@ from app.models import (
     JOB_STATUS_ERROR,
     SMS_TYPE,
     EMAIL_TYPE,
-)
+    HighVolumeService, Template, Notification, KEY_TYPE_NORMAL, FactBilling, FactNotificationStatus)
 from app.notifications.process_notifications import send_notification_to_queue
 from app.v2.errors import JobIncompleteError
 
@@ -311,3 +312,112 @@ def check_for_services_with_high_failure_rates_or_sending_to_tv_numbers():
                 message=message,
                 ticket_type=zendesk_client.TYPE_INCIDENT
             )
+
+
+def purge_high_volume_notifications():
+    # Not sure what time to use here.....
+    hour_ago = datetime.utcnow() - timedelta(hours=1)
+
+    # We could also store the id in config... however this does give us a bit of control,
+    # as in we can stop this process from happening by deleting the row.
+    # The other part of it is to potentially take this tactic for other services...
+    # perhaps hour_ago is actually the timedelta from the table.
+    services = HighVolumeService.query.all()
+
+    for service in services:
+        templates = Template.query.filter_by(
+            service_id=service.service_id,
+            # going to assume we are only dealing with emails because I don't want to deal with rates at this time.
+            template_type=EMAIL_TYPE
+        ).all()
+        for status in ['delivered', 'temporary-failure', 'permanent-failure']:
+            for template in templates:
+                del_count = Notification.query.filter(
+                    Notification.service_id == service.service_id,
+                    Notification.template_id == template.id,
+                    Notification.notification_type == template.template_type,
+                    Notification.status == status,
+                    Notification.key_type == KEY_TYPE_NORMAL,
+                    Notification.created_at < hour_ago,
+                ).delete()
+
+                bst_date = convert_utc_to_bst(hour_ago).date()
+                # upsert stat data
+                upsert_ft_billing(bst_date, del_count, service.service_id, template)
+                upsert_ft_notification_status(bst_date, del_count, service.service_id, status, template)
+
+
+def upsert_ft_notification_status(bst_date, del_count, service_id, status, template):
+    ft_status_row = FactNotificationStatus.query.filter(
+        FactNotificationStatus.service_id == service_id,
+        FactNotificationStatus.template_id == template.id,
+        FactNotificationStatus.bst_date == bst_date,
+        FactNotificationStatus.notification_type == template.template_type,
+        FactNotificationStatus.notification_status == status,
+        FactNotificationStatus.key_type == KEY_TYPE_NORMAL
+    ).first()
+    if ft_status_row:
+        # How do we deal with rows in ft_status where there are.... this isn't going to work.
+        # The current process take the days total from Notifications,
+        # deletes the row for status then inserts,
+        # this means that any rows with a created status should eventually go away.
+        # Do we stop processing the HighVolumes services in the reporting tasks?
+        # If yes, then we need to thing about the migration plan, take a snapshot of data first, etc.
+        FactNotificationStatus.query.filter(
+            FactNotificationStatus.service_id == service_id,
+            FactNotificationStatus.template_id == template.id,
+            FactNotificationStatus.bst_date == bst_date,
+            FactNotificationStatus.notification_type == template.template_type,
+            FactNotificationStatus.notification_status == status,
+            FactNotificationStatus.key_type == KEY_TYPE_NORMAL
+        ).update({"notification_count": ft_status_row.notification_count + del_count})
+    else:
+        ft_status = FactNotificationStatus(
+            bst_date=bst_date,
+            template_id=template.id,
+            service_id=service_id,
+            job_id='00000000-0000-0000-0000-000000000000',
+            notification_type=template.template_type,
+            key_type=KEY_TYPE_NORMAL,
+            notification_status=status,
+            notification_count=del_count,
+            created_at=datetime.utcnow()
+        )
+        db.session.add(ft_status)
+        db.session.commit()
+
+
+def upsert_ft_billing(bst_date, del_count, service_id, template):
+    ft_billing_row = FactBilling.query.filter(
+        FactBilling.service_id == service_id,
+        FactBilling.template_id == template.id,
+        FactBilling.bst_date == bst_date,
+        FactBilling.notification_type == template.template_type
+    ).first()
+    if not ft_billing_row:
+        # insert new row
+        ft_billing = FactBilling(
+            bst_date=bst_date,
+            service_id=service_id,
+            template_id=template.id,
+            notification_type=template.template_type,
+            provider='SES',
+            rate_multiplier=0,
+            international=False,
+            rate=0,
+            billable_units=0,
+            notifications_sent=del_count,
+            created_at=datetime.utcnow(),
+            postage='none'
+        )
+        db.session.add(ft_billing)
+        db.session.commit()
+
+    else:
+        FactBilling.query.filter(
+            FactBilling.service_id == service_id,
+            FactBilling.template_id == template.id,
+            FactBilling.bst_date == bst_date,
+            FactBilling.notification_type == template.template_type
+        ).update({'notifications_sent': ft_billing_row.notifications_sent + del_count})
+        db.session.commit()

app/celery/tasks.py

@@ -4,6 +4,7 @@ from collections import namedtuple, defaultdict
 
 from flask import current_app
 from notifications_utils.recipients import (
+    format_postcode_for_printing,
     RecipientCSV
 )
 from notifications_utils.statsd_decorators import statsd
@@ -305,6 +306,10 @@ def save_letter(
     # we store the recipient as just the first item of the person's address
     recipient = notification['personalisation']['addressline1']
 
+    notification['personalisation']['postcode'] = format_postcode_for_printing(
+        notification['personalisation']['postcode']
+    )
+
     service = dao_fetch_service_by_id(service_id)
     template = dao_get_template_by_id(notification['template'], version=notification['template_version'])
 

app/config.py

@@ -28,6 +28,7 @@ class QueueNames(object):
     CREATE_LETTERS_PDF = 'create-letters-pdf-tasks'
     CALLBACKS = 'service-callbacks'
     LETTERS = 'letter-tasks'
+    SMS_CALLBACKS = 'sms-callbacks'
     ANTIVIRUS = 'antivirus-tasks'
     SANITISE_LETTERS = 'sanitise-letter-tasks'
 
@@ -47,6 +48,7 @@ class QueueNames(object):
             QueueNames.CREATE_LETTERS_PDF,
             QueueNames.CALLBACKS,
             QueueNames.LETTERS,
+            QueueNames.SMS_CALLBACKS,
         ]
 
 

app/dao/notifications_dao.py

@@ -731,7 +731,7 @@ def notifications_not_yet_sent(should_be_sending_after_seconds, notification_typ
     return notifications
 
 
-def dao_get_letters_to_be_printed(print_run_deadline, postage):
+def dao_get_letters_to_be_printed(print_run_deadline):
     """
     Return all letters created before the print run deadline that have not yet been sent
     """
@@ -739,8 +739,7 @@ def dao_get_letters_to_be_printed(print_run_deadline, postage):
         Notification.created_at < convert_bst_to_utc(print_run_deadline),
         Notification.notification_type == LETTER_TYPE,
         Notification.status == NOTIFICATION_CREATED,
-        Notification.key_type == KEY_TYPE_NORMAL,
-        Notification.postage == postage
+        Notification.key_type == KEY_TYPE_NORMAL
     ).order_by(
         Notification.created_at
     ).all()

app/models.py

@@ -2148,3 +2148,10 @@ class ServiceContactList(db.Model):
             "created_at": created_at_in_bst.strftime("%Y-%m-%d %H:%M:%S"),
         }
         return contact_list
+
+
+class HighVolumeService(db.Model):
+    # Service that we want to purge data for hourly
+    __tablename__ = 'high_volume_service'
+
+    service_id = db.Column(UUID(as_uuid=True), primary_key=True, unique=True, index=True, nullable=False)

app/notifications/notifications_sms_callback.py

@@ -3,8 +3,9 @@ from flask import current_app
 from flask import json
 from flask import request, jsonify
 
+from app.celery.process_sms_client_response_tasks import process_sms_client_response
+from app.config import QueueNames
 from app.errors import InvalidRequest, register_errors
-from app.notifications.process_client_response import validate_callback_data, process_sms_client_response
 
 sms_callback_blueprint = Blueprint("sms_callback", __name__, url_prefix="/notifications/sms")
 register_errors(sms_callback_blueprint)
@@ -20,19 +21,21 @@ def process_mmg_response():
     if errors:
         raise InvalidRequest(errors, status_code=400)
 
-    success, errors = process_sms_client_response(status=str(data.get('status')),
-                                                  provider_reference=data.get('CID'),
-                                                  client_name=client_name)
+    status = str(data.get('status'))
+    provider_reference = data.get('CID')
+
+    process_sms_client_response.apply_async(
+        [status, provider_reference, client_name],
+        queue=QueueNames.SMS_CALLBACKS,
+    )
 
     safe_to_log = data.copy()
     safe_to_log.pop("MSISDN")
     current_app.logger.debug(
-        "Full delivery response from {} for notification: {}\n{}".format(client_name, request.form.get('CID'),
-                                                                         safe_to_log))
-    if errors:
-        raise InvalidRequest(errors, status_code=400)
-    else:
-        return jsonify(result='success', message=success), 200
+        f"Full delivery response from {client_name} for notification: {provider_reference}\n{safe_to_log}"
+    )
+
+    return jsonify(result='success'), 200
 
 
 @sms_callback_blueprint.route('/firetext', methods=['POST'])
@@ -43,15 +46,28 @@ def process_firetext_response():
                                     client_name=client_name)
     if errors:
         raise InvalidRequest(errors, status_code=400)
+
+    status = request.form.get('status')
+    provider_reference = request.form.get('reference')
+
     safe_to_log = dict(request.form).copy()
     safe_to_log.pop('mobile')
     current_app.logger.debug(
-        "Full delivery response from {} for notification: {}\n{}".format(client_name, request.form.get('reference'),
-                                                                         safe_to_log))
-    success, errors = process_sms_client_response(status=request.form.get('status'),
-                                                  provider_reference=request.form.get('reference'),
-                                                  client_name=client_name)
-    if errors:
-        raise InvalidRequest(errors, status_code=400)
-    else:
-        return jsonify(result='success', message=success), 200
+        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],
+        queue=QueueNames.SMS_CALLBACKS,
+    )
+
+    return jsonify(result='success'), 200
+
+
+def validate_callback_data(data, fields, client_name):
+    errors = []
+    for f in fields:
+        if not str(data.get(f, '')):
+            error = "{} callback failed: {} missing".format(client_name, f)
+            errors.append(error)
+    return errors if len(errors) > 0 else None

manifest.yml.j2

@@ -2,7 +2,7 @@
   'notify-api': {
     'NOTIFY_APP_NAME': 'api',
     'disk_quota': '2G',
-    'sqlalchemy_pool_size': 20,
+    'sqlalchemy_pool_size': 30,
     'routes': {
       'preview': ['api.notify.works'],
       'staging': ['api.staging-notify.works'],

migrations/versions/0319_high_volume_service.py

@@ -0,0 +1,27 @@
+"""
+
+Revision ID: 0319_high_volume_service
+Revises: 0318_service_contact_list
+Create Date: 2020-03-20 08:53:22.624516
+
+"""
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects import postgresql
+
+revision = '0319_high_volume_service'
+down_revision = '0318_service_contact_list'
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.create_table('high_volume_service',
+                    sa.Column('service_id', postgresql.UUID(as_uuid=True), nullable=False),
+                    sa.PrimaryKeyConstraint('service_id')
+                    )
+    op.create_index(op.f('ix_high_volume_service_service_id'), 'high_volume_service', ['service_id'], unique=True)
+
+
+def downgrade():
+    op.drop_index(op.f('ix_high_volume_service_service_id'), table_name='high_volume_service')
+    op.drop_table('high_volume_service')

requirements-app.txt

@@ -27,4 +27,4 @@ notifications-python-client==5.5.1
 awscli-cwlogs>=1.4,<1.5
 
 
-git+https://github.com/alphagov/notifications-utils.git@36.6.2#egg=notifications-utils==36.6.2
+git+https://github.com/alphagov/notifications-utils.git@36.9.0#egg=notifications-utils==36.9.0

requirements.txt

@@ -29,23 +29,23 @@ notifications-python-client==5.5.1
 awscli-cwlogs>=1.4,<1.5
 
 
-git+https://github.com/alphagov/notifications-utils.git@36.6.2#egg=notifications-utils==36.6.2
+git+https://github.com/alphagov/notifications-utils.git@36.9.0#egg=notifications-utils==36.9.0
 
 ## The following requirements were added by pip freeze:
 alembic==1.4.1
 amqp==1.4.9
 anyjson==0.3.3
 attrs==19.3.0
-awscli==1.18.16
+awscli==1.18.20
 bcrypt==3.1.7
 billiard==3.3.0.23
 bleach==3.1.1
 boto==2.49.0
 boto3==1.10.38
-botocore==1.15.16
+botocore==1.15.20
 certifi==2019.11.28
 chardet==3.0.4
-click==7.1
+click==7.1.1
 colorama==0.4.3
 dnspython==1.16.0
 docutils==0.15.2

scripts/paas_app_wrapper.sh

@@ -43,7 +43,7 @@ case $NOTIFY_APP_NAME in
     ;;
   delivery-worker-receipts)
     exec scripts/run_app_paas.sh celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=11 \
-    -Q ses-callbacks 2> /dev/null
+    -Q ses-callbacks,sms-callbacks 2> /dev/null
     ;;
   delivery-worker-service-callbacks)
     exec scripts/run_app_paas.sh celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=11 \

tests/app/celery/test_scheduled_tasks.py

@@ -6,6 +6,7 @@ from collections import namedtuple
 from freezegun import freeze_time
 from mock import mock
 
+from app import db
 from app.celery import scheduled_tasks
 from app.celery.scheduled_tasks import (
     check_job_status,
@@ -19,7 +20,7 @@ from app.celery.scheduled_tasks import (
     check_for_missing_rows_in_completed_jobs,
     check_for_services_with_high_failure_rates_or_sending_to_tv_numbers,
     switch_current_sms_provider_on_slow_delivery,
-)
+    purge_high_volume_notifications)
 from app.config import QueueNames, TaskNames, Config
 from app.dao.jobs_dao import dao_get_job_by_id
 from app.dao.notifications_dao import dao_get_scheduled_notifications
@@ -30,7 +31,7 @@ from app.models import (
     JOB_STATUS_FINISHED,
     NOTIFICATION_DELIVERED,
     NOTIFICATION_PENDING_VIRUS_CHECK,
-)
+    HighVolumeService, Notification, FactBilling, FactNotificationStatus)
 from app.v2.errors import JobIncompleteError
 from tests.app import load_example_csv
 
@@ -314,7 +315,7 @@ def test_replay_created_notifications(notify_db_session, sample_service, mocker)
 
 
 def test_replay_created_notifications_create_letters_pdf_tasks_for_letters_not_ready_to_send(
-        sample_letter_template, mocker
+    sample_letter_template, mocker
 ):
     mock_task = mocker.patch('app.celery.scheduled_tasks.create_letters_pdf.apply_async')
     create_notification(template=sample_letter_template, billable_units=0,
@@ -556,3 +557,56 @@ def test_check_for_services_with_high_failure_rates_or_sending_to_tv_numbers(
         subject="[test] High failure rates for sms spotted for services",
         ticket_type='incident'
     )
+
+
+@freeze_time('2020-03-19 13:30')
+def test_purge_high_volume_notifications(sample_email_template, notify_db_session):
+    # should be deleted
+    create_notification(template=sample_email_template,
+                        created_at=datetime.utcnow() - timedelta(days=4), status='delivered')
+    create_notification(template=sample_email_template,
+                        created_at=datetime.utcnow() - timedelta(hours=2), status='permanent-failure')
+
+    create_notification(template=sample_email_template,
+                        created_at=datetime.utcnow() - timedelta(hours=1, minutes=1), status='temporary-failure')
+    # should NOT be deleted
+    create_notification(template=sample_email_template,
+                        created_at=datetime.utcnow() - timedelta(minutes=59), status='temporary-failure')
+    create_notification(template=sample_email_template,
+                        created_at=datetime.utcnow() - timedelta(hours=1), status='temporary-failure')
+    create_notification(template=sample_email_template,
+                        created_at=datetime.utcnow() - timedelta(hours=1), status='delivered')
+    create_notification(template=sample_email_template,
+                        created_at=datetime.utcnow() - timedelta(hours=1), status='created')
+
+    create_notification(template=sample_email_template,
+                        created_at=datetime.utcnow() - timedelta(days=1), status='sending')
+
+    create_notification(template=sample_email_template,
+                        created_at=datetime.utcnow() - timedelta(days=1), status='technical-failure')
+
+    high_volume_service = HighVolumeService(service_id=sample_email_template.service_id)
+    db.session.add(high_volume_service)
+    db.session.commit()
+    purge_high_volume_notifications()
+
+    notifications = Notification.query.all()
+    assert len(notifications) == 6
+
+    ft_billing = FactBilling.query.all()
+    assert len(ft_billing) == 1
+    assert ft_billing[0].service_id == sample_email_template.service_id
+    assert ft_billing[0].notifications_sent == 3
+    assert str(ft_billing[0].bst_date) == '2020-03-19'
+
+    ft_status = FactNotificationStatus.query.order_by(FactNotificationStatus.notification_status).all()
+    assert len(ft_status) == 3
+    assert str(ft_status[0].bst_date) == '2020-03-19'
+    assert ft_status[0].notification_status == 'delivered'
+    assert ft_status[0].notification_count == 1
+    assert str(ft_status[1].bst_date) == '2020-03-19'
+    assert ft_status[1].notification_status == 'permanent-failure'
+    assert ft_status[1].notification_count == 1
+    assert str(ft_status[2].bst_date) == '2020-03-19'
+    assert ft_status[2].notification_status == 'temporary-failure'
+    assert ft_status[2].notification_count == 1

tests/app/celery/test_tasks.py

@@ -961,7 +961,7 @@ def test_save_letter_saves_letter_to_database(mocker, notify_db_session):
         'addressline4': 'Wibble',
         'addressline5': 'Wobble',
         'addressline6': 'Wubble',
-        'postcode': 'Flob',
+        'postcode': 'SE1 2SA',
     }
     notification_json = _notification_json(
         template=job.template,
@@ -1021,6 +1021,31 @@ def test_save_letter_saves_letter_to_database_with_correct_postage(mocker, notif
     assert notification_db.postage == postage
 
 
+def test_save_letter_saves_letter_to_database_with_formatted_postcode(mocker, notify_db_session):
+    service = create_service(service_permissions=[LETTER_TYPE])
+    template = create_template(service=service, template_type=LETTER_TYPE)
+    letter_job = create_job(template=template)
+
+    mocker.patch('app.celery.tasks.letters_pdf_tasks.create_letters_pdf.apply_async')
+    notification_json = _notification_json(
+        template=letter_job.template,
+        to='Foo',
+        personalisation={'addressline1': 'Foo', 'addressline2': 'Bar', 'postcode': 'se1 64sa'},
+        job_id=letter_job.id,
+        row_number=1
+    )
+    notification_id = uuid.uuid4()
+    save_letter(
+        letter_job.service_id,
+        notification_id,
+        encryption.encrypt(notification_json),
+    )
+
+    notification_db = Notification.query.one()
+    assert notification_db.id == notification_id
+    assert notification_db.personalisation["postcode"] == "SE16 4SA"
+
+
 def test_save_letter_saves_letter_to_database_right_reply_to(mocker, notify_db_session):
     service = create_service()
     create_letter_contact(service=service, contact_block="Address contact", is_default=True)
@@ -1037,7 +1062,7 @@ def test_save_letter_saves_letter_to_database_right_reply_to(mocker, notify_db_s
         'addressline4': 'Wibble',
         'addressline5': 'Wobble',
         'addressline6': 'Wubble',
-        'postcode': 'Flob',
+        'postcode': 'SE1 3WS',
     }
     notification_json = _notification_json(
         template=job.template,

tests/app/notifications/rest/test_callbacks.py

@@ -1,17 +1,7 @@
-import uuid
-
-from datetime import datetime
-
 import pytest
 from flask import json
-from freezegun import freeze_time
 
-import app.celery.tasks
-from app.clients import ClientException
-from app.dao.notifications_dao import (
-    get_notification_by_id
-)
-from tests.app.db import create_notification, create_service_callback_api
+from app.notifications.notifications_sms_callback import validate_callback_data
 
 
 def firetext_post(client, data):
@@ -111,15 +101,14 @@ def test_dvla_ack_calls_does_not_call_letter_notifications_task(client, mocker):
 
 
 def test_firetext_callback_should_not_need_auth(client, mocker):
-    mocker.patch('app.statsd_client.incr')
-    data = 'mobile=441234123123&status=0&reference=send-sms-code&time=2016-03-10 14:17:00'
+    mocker.patch('app.notifications.notifications_sms_callback.process_sms_client_response')
+    data = 'mobile=441234123123&status=0&reference=notification_id&time=2016-03-10 14:17:00'
 
     response = firetext_post(client, data)
     assert response.status_code == 200
 
 
 def test_firetext_callback_should_return_400_if_empty_reference(client, mocker):
-    mocker.patch('app.statsd_client.incr')
     data = 'mobile=441234123123&status=0&reference=&time=2016-03-10 14:17:00'
     response = firetext_post(client, data)
 
@@ -130,7 +119,6 @@ def test_firetext_callback_should_return_400_if_empty_reference(client, mocker):
 
 
 def test_firetext_callback_should_return_400_if_no_reference(client, mocker):
-    mocker.patch('app.statsd_client.incr')
     data = 'mobile=441234123123&status=0&time=2016-03-10 14:17:00'
     response = firetext_post(client, data)
     json_resp = json.loads(response.get_data(as_text=True))
@@ -139,19 +127,8 @@ def test_firetext_callback_should_return_400_if_no_reference(client, mocker):
     assert json_resp['message'] == ['Firetext callback failed: reference missing']
 
 
-def test_firetext_callback_should_return_200_if_send_sms_reference(client, mocker):
-    mocker.patch('app.statsd_client.incr')
-    data = 'mobile=441234123123&status=0&time=2016-03-10 14:17:00&reference=send-sms-code'
-    response = firetext_post(client, data)
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 200
-    assert json_resp['result'] == 'success'
-    assert json_resp['message'] == 'Firetext callback succeeded: send-sms-code'
-
-
 def test_firetext_callback_should_return_400_if_no_status(client, mocker):
-    mocker.patch('app.statsd_client.incr')
-    data = 'mobile=441234123123&time=2016-03-10 14:17:00&reference=send-sms-code'
+    data = 'mobile=441234123123&time=2016-03-10 14:17:00&reference=notification_id'
     response = firetext_post(client, data)
     json_resp = json.loads(response.get_data(as_text=True))
     assert response.status_code == 400
@@ -159,136 +136,24 @@ def test_firetext_callback_should_return_400_if_no_status(client, mocker):
     assert json_resp['message'] == ['Firetext callback failed: status missing']
 
 
-def test_firetext_callback_should_set_status_technical_failure_if_status_unknown(
-        client, mocker, sample_notification):
-    sample_notification.status = 'sending'
-    # mocker.patch('app.statsd_client.incr')
-    data = 'mobile=441234123123&status=99&time=2016-03-10 14:17:00&reference={}'.format(sample_notification.id)
-    with pytest.raises(ClientException) as e:
-        firetext_post(client, data)
-    assert get_notification_by_id(sample_notification.id).status == 'technical-failure'
-    assert 'Firetext callback failed: status 99 not found.' in str(e.value)
+def test_firetext_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')
 
-
-def test_firetext_callback_returns_200_when_notification_id_is_not_a_valid_uuid(client, mocker):
-    mocker.patch('app.statsd_client.incr')
-    data = 'mobile=441234123123&status=0&time=2016-03-10 14:17:00&reference=1234'
+    data = 'mobile=441234123123&status=0&time=2016-03-10 14:17:00&reference=notification_id'
     response = firetext_post(client, data)
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 400
-    assert json_resp['result'] == 'error'
-    assert json_resp['message'] == 'Firetext callback with invalid reference 1234'
-
-
-def test_callback_should_return_200_if_cannot_find_notification_id(
-    notify_db,
-    notify_db_session,
-    client,
-    mocker
-):
-    mocker.patch('app.statsd_client.incr')
-    missing_notification_id = uuid.uuid4()
-    data = 'mobile=441234123123&status=0&time=2016-03-10 14:17:00&reference={}'.format(
-        missing_notification_id)
-    response = firetext_post(client, data)
-
     json_resp = json.loads(response.get_data(as_text=True))
     assert response.status_code == 200
     assert json_resp['result'] == 'success'
 
-
-def test_firetext_callback_should_update_notification_status(
-        client, mocker, sample_notification
-):
-    mocker.patch('app.statsd_client.incr')
-    send_mock = mocker.patch(
-        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+    mock_celery.assert_called_once_with(
+        ['0', 'notification_id', 'Firetext'],
+        queue='sms-callbacks',
     )
-    sample_notification.status = 'sending'
-
-    original = get_notification_by_id(sample_notification.id)
-    assert original.status == 'sending'
-    data = 'mobile=441234123123&status=0&time=2016-03-10 14:17:00&reference={}'.format(
-        sample_notification.id)
-    response = firetext_post(client, data)
-
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 200
-    assert json_resp['result'] == 'success'
-    assert json_resp['message'] == 'Firetext callback succeeded. reference {} updated'.format(
-        sample_notification.id
-    )
-    updated = get_notification_by_id(sample_notification.id)
-    assert updated.status == 'delivered'
-    assert get_notification_by_id(sample_notification.id).status == 'delivered'
-    assert send_mock.called_once_with([sample_notification.id], queue="notify-internal-tasks")
 
 
-def test_firetext_callback_should_update_notification_status_failed(
-        client, mocker, sample_template
-):
-    mocker.patch('app.statsd_client.incr')
-    mocker.patch(
-        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
-    )
-    notification = create_notification(template=sample_template, status='sending')
-
-    original = get_notification_by_id(notification.id)
-    assert original.status == 'sending'
-
-    data = 'mobile=441234123123&status=1&time=2016-03-10 14:17:00&reference={}'.format(
-        notification.id)
-    response = firetext_post(client, data)
-
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 200
-    assert json_resp['result'] == 'success'
-    assert json_resp['message'] == 'Firetext callback succeeded. reference {} updated'.format(
-        notification.id
-    )
-    assert get_notification_by_id(notification.id).status == 'permanent-failure'
-
-
-def test_firetext_callback_should_update_notification_status_pending(client, sample_template, mocker):
-    mocker.patch('app.statsd_client.incr')
-    mocker.patch(
-        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
-    )
-    notification = create_notification(template=sample_template, status='sending')
-
-    original = get_notification_by_id(notification.id)
-    assert original.status == 'sending'
-    data = 'mobile=441234123123&status=2&time=2016-03-10 14:17:00&reference={}'.format(
-        notification.id)
-    response = firetext_post(client, data)
-
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 200
-    assert json_resp['result'] == 'success'
-    assert json_resp['message'] == 'Firetext callback succeeded. reference {} updated'.format(
-        notification.id
-    )
-    assert get_notification_by_id(notification.id).status == 'pending'
-
-
-def test_process_mmg_response_return_200_when_cid_is_send_sms_code(client):
-    data = '{"reference": "10100164", "CID": "send-sms-code", "MSISDN": "447775349060", "status": "3", \
-        "deliverytime": "2016-04-05 16:01:07"}'
-
-    response = mmg_post(client, data)
-    assert response.status_code == 200
-    json_data = json.loads(response.data)
-    assert json_data['result'] == 'success'
-    assert json_data['message'] == 'MMG callback succeeded: send-sms-code'
-
-
-def test_process_mmg_response_returns_200_when_cid_is_valid_notification_id(
-        sample_notification, client, mocker
-):
-    mocker.patch(
-        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
-    )
-    sample_notification.status = 'sending'
+def test_mmg_callback_should_not_need_auth(client, mocker, sample_notification):
+    mocker.patch('app.notifications.notifications_sms_callback.process_sms_client_response')
     data = json.dumps({"reference": "mmg_reference",
                        "CID": str(sample_notification.id),
                        "MSISDN": "447777349060",
@@ -296,93 +161,7 @@ def test_process_mmg_response_returns_200_when_cid_is_valid_notification_id(
                        "deliverytime": "2016-04-05 16:01:07"})
 
     response = mmg_post(client, data)
-
     assert response.status_code == 200
-    json_data = json.loads(response.data)
-    assert json_data['result'] == 'success'
-    assert json_data['message'] == 'MMG callback succeeded. reference {} updated'.format(sample_notification.id)
-    assert get_notification_by_id(sample_notification.id).status == 'delivered'
-
-
-def test_process_mmg_response_status_5_updates_notification_with_permanently_failed(
-    sample_notification, client, mocker
-):
-    mocker.patch(
-        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
-    )
-    sample_notification.status = 'sending'
-
-    data = json.dumps({"reference": "mmg_reference",
-                       "CID": str(sample_notification.id),
-                       "MSISDN": "447777349060",
-                       "status": 5})
-
-    response = mmg_post(client, data)
-    assert response.status_code == 200
-    json_data = json.loads(response.data)
-    assert json_data['result'] == 'success'
-    assert json_data['message'] == 'MMG callback succeeded. reference {} updated'.format(sample_notification.id)
-    assert get_notification_by_id(sample_notification.id).status == 'permanent-failure'
-
-
-def test_process_mmg_response_status_2_updates_notification_with_permanently_failed(
-    sample_notification, client, mocker
-):
-    mocker.patch(
-        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
-    )
-    sample_notification.status = 'sending'
-    data = json.dumps({"reference": "mmg_reference",
-                       "CID": str(sample_notification.id),
-                       "MSISDN": "447777349060",
-                       "status": 2})
-
-    response = mmg_post(client, data)
-    assert response.status_code == 200
-    json_data = json.loads(response.data)
-    assert json_data['result'] == 'success'
-    assert json_data['message'] == 'MMG callback succeeded. reference {} updated'.format(sample_notification.id)
-    assert get_notification_by_id(sample_notification.id).status == 'permanent-failure'
-
-
-def test_process_mmg_response_status_4_updates_notification_with_temporary_failed(
-        sample_notification, client, mocker
-):
-    mocker.patch(
-        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
-    )
-    sample_notification.status = 'sending'
-
-    data = json.dumps({"reference": "mmg_reference",
-                       "CID": str(sample_notification.id),
-                       "MSISDN": "447777349060",
-                       "status": 4})
-
-    response = mmg_post(client, data)
-    assert response.status_code == 200
-    json_data = json.loads(response.data)
-    assert json_data['result'] == 'success'
-    assert json_data['message'] == 'MMG callback succeeded. reference {} updated'.format(sample_notification.id)
-    assert get_notification_by_id(sample_notification.id).status == 'temporary-failure'
-
-
-def test_process_mmg_response_unknown_status_updates_notification_with_technical_failure(
-        sample_notification, client, mocker
-):
-    send_mock = mocker.patch(
-        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
-    )
-    sample_notification.status = 'sending'
-    data = json.dumps({"reference": "mmg_reference",
-                       "CID": str(sample_notification.id),
-                       "MSISDN": "447777349060",
-                       "status": 10})
-    create_service_callback_api(service=sample_notification.service, url="https://original_url.com")
-    with pytest.raises(ClientException) as e:
-        mmg_post(client, data)
-    assert 'MMG callback failed: status 10 not found.' in str(e.value)
-    assert get_notification_by_id(sample_notification.id).status == 'technical-failure'
-    assert send_mock.called
 
 
 def test_process_mmg_response_returns_400_for_malformed_data(client):
@@ -401,68 +180,64 @@ def test_process_mmg_response_returns_400_for_malformed_data(client):
     assert "{} callback failed: {} missing".format('MMG', 'CID') in json_data['message']
 
 
-def test_mmg_callback_returns_200_when_notification_id_not_found_or_already_updated(client):
-    data = '{"reference": "10100164", "CID": "send-sms-code", "MSISDN": "447775349060", "status": "3", \
-             "deliverytime": "2016-04-05 16:01:07"}'
+def test_mmg_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({"reference": "mmg_reference",
+                       "CID": "notification_id",
+                       "MSISDN": "447777349060",
+                       "status": "3",
+                       "deliverytime": "2016-04-05 16:01:07"})
 
     response = mmg_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(
+        ['3', 'notification_id', 'MMG'],
+        queue='sms-callbacks',
+    )
 
 
-def test_mmg_callback_returns_400_when_notification_id_is_not_a_valid_uuid(client):
-    data = '{"reference": "10100164", "CID": "1234", "MSISDN": "447775349060", "status": "3", \
-             "deliverytime": "2016-04-05 16:01:07"}'
+def test_validate_callback_data_returns_none_when_valid():
+    form = {'status': 'good',
+            'reference': 'send-sms-code'}
+    fields = ['status', 'reference']
+    client_name = 'sms client'
 
-    response = mmg_post(client, data)
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 400
-    assert json_resp['message'] == 'MMG callback with invalid reference 1234'
+    assert validate_callback_data(form, fields, client_name) is None
 
 
-def test_process_mmg_response_records_statsd(sample_notification, client, mocker):
-    with freeze_time('2001-01-01T12:00:00'):
+def test_validate_callback_data_return_errors_when_fields_are_empty():
+    form = {'monkey': 'good'}
+    fields = ['status', 'cid']
+    client_name = 'sms client'
 
-        mocker.patch('app.statsd_client.incr')
-        mocker.patch('app.statsd_client.timing_with_dates')
-        mocker.patch(
-            'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
-        )
-        sample_notification.status = 'sending'
-        sample_notification.sent_at = datetime.now()
-
-        data = json.dumps({"reference": "mmg_reference",
-                           "CID": str(sample_notification.id),
-                           "MSISDN": "447777349060",
-                           "status": "3",
-                           "deliverytime": "2016-04-05 16:01:07"})
-
-        mmg_post(client, data)
-
-        app.statsd_client.incr.assert_any_call("callback.mmg.delivered")
-        app.statsd_client.timing_with_dates.assert_any_call(
-            "callback.mmg.elapsed-time", datetime.utcnow(), sample_notification.sent_at
-        )
+    errors = validate_callback_data(form, fields, client_name)
+    assert len(errors) == 2
+    assert "{} callback failed: {} missing".format(client_name, 'status') in errors
+    assert "{} callback failed: {} missing".format(client_name, 'cid') in errors
 
 
-def test_firetext_callback_should_record_statsd(client, sample_notification, mocker):
-    with freeze_time('2001-01-01T12:00:00'):
+def test_validate_callback_data_can_handle_integers():
+    form = {'status': 00, 'cid': 'fsdfadfsdfas'}
+    fields = ['status', 'cid']
+    client_name = 'sms client'
 
-        mocker.patch('app.statsd_client.incr')
-        mocker.patch('app.statsd_client.timing_with_dates')
-        mocker.patch(
-            'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
-        )
-        sample_notification.status = 'sending'
-        sample_notification.sent_at = datetime.now()
+    result = validate_callback_data(form, fields, client_name)
+    assert result is None
 
-        data = 'mobile=441234123123&status=0&time=2016-03-10 14:17:00&code=101&reference={}'.format(
-            sample_notification.id)
-        firetext_post(client, data)
 
-        app.statsd_client.timing_with_dates.assert_any_call(
-            "callback.firetext.elapsed-time", datetime.utcnow(), sample_notification.sent_at
-        )
-        app.statsd_client.incr.assert_any_call("callback.firetext.delivered")
+def test_validate_callback_data_returns_error_for_empty_string():
+    form = {'status': '', 'cid': 'fsdfadfsdfas'}
+    fields = ['status', 'cid']
+    client_name = 'sms client'
+
+    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):

tests/app/notifications/test_process_client_response.py

@@ -1,79 +1,66 @@
 import uuid
+from datetime import datetime
 
 import pytest
+from freezegun import freeze_time
 
+from app import statsd_client
 from app.clients import ClientException
-from app.notifications.process_client_response import (
-    validate_callback_data,
-    process_sms_client_response
-)
+from app.celery.process_sms_client_response_tasks import process_sms_client_response
 from app.celery.service_callback_tasks import create_delivery_status_callback_data
+from app.models import NOTIFICATION_TECHNICAL_FAILURE
 from tests.app.db import create_service_callback_api
 
 
-def test_validate_callback_data_returns_none_when_valid():
-    form = {'status': 'good',
-            'reference': 'send-sms-code'}
-    fields = ['status', 'reference']
-    client_name = 'sms client'
-
-    assert validate_callback_data(form, fields, client_name) is None
+def test_process_sms_client_response_raises_error_if_reference_is_not_a_valid_uuid(client):
+    with pytest.raises(ValueError):
+        process_sms_client_response(
+            status='000', provider_reference='something-bad', client_name='sms-client')
 
 
-def test_validate_callback_data_return_errors_when_fields_are_empty():
-    form = {'monkey': 'good'}
-    fields = ['status', 'cid']
-    client_name = 'sms client'
+@pytest.mark.parametrize('client_name', ('Firetext', 'MMG'))
+def test_process_sms_response_raises_client_exception_for_unknown_status(
+    sample_notification,
+    mocker,
+    client_name,
+):
+    with pytest.raises(ClientException) as e:
+        process_sms_client_response(
+            status='000',
+            provider_reference=str(sample_notification.id),
+            client_name=client_name,
+        )
 
-    errors = validate_callback_data(form, fields, client_name)
-    assert len(errors) == 2
-    assert "{} callback failed: {} missing".format(client_name, 'status') in errors
-    assert "{} callback failed: {} missing".format(client_name, 'cid') in errors
+    assert f"{client_name} callback failed: status {'000'} not found." in str(e.value)
+    assert sample_notification.status == NOTIFICATION_TECHNICAL_FAILURE
 
 
-def test_validate_callback_data_can_handle_integers():
-    form = {'status': 00, 'cid': 'fsdfadfsdfas'}
-    fields = ['status', 'cid']
-    client_name = 'sms client'
+@pytest.mark.parametrize('status, sms_provider, expected_notification_status', [
+    ('0', 'Firetext', 'delivered'),
+    ('1', 'Firetext', 'permanent-failure'),
+    ('2', 'Firetext', 'pending'),
+    ('2', 'MMG', 'permanent-failure'),
+    ('3', 'MMG', 'delivered'),
+    ('4', 'MMG', 'temporary-failure'),
+    ('5', 'MMG', 'permanent-failure'),
+])
+def test_process_sms_client_response_updates_notification_status(
+    sample_notification,
+    mocker,
+    status,
+    sms_provider,
+    expected_notification_status,
+):
+    sample_notification.status = 'sending'
+    process_sms_client_response(status, str(sample_notification.id), sms_provider)
 
-    result = validate_callback_data(form, fields, client_name)
-    assert result is None
+    assert sample_notification.status == expected_notification_status
 
 
-def test_validate_callback_data_returns_error_for_empty_string():
-    form = {'status': '', 'cid': 'fsdfadfsdfas'}
-    fields = ['status', 'cid']
-    client_name = 'sms client'
-
-    result = validate_callback_data(form, fields, client_name)
-    assert result is not None
-    assert "{} callback failed: {} missing".format(client_name, 'status') in result
-
-
-def test_outcome_statistics_called_for_successful_callback(sample_notification, mocker):
+def test_sms_response_does_not_send_callback_if_notification_is_not_in_the_db(sample_service, mocker):
     mocker.patch(
-        'app.notifications.process_client_response.notifications_dao.update_notification_status_by_id',
-        return_value=sample_notification
-    )
-    send_mock = mocker.patch(
-        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
-    )
-    callback_api = create_service_callback_api(service=sample_notification.service, url="https://original_url.com")
-    reference = str(uuid.uuid4())
-
-    success, error = process_sms_client_response(status='3', provider_reference=reference, client_name='MMG')
-    assert success == "MMG callback succeeded. reference {} updated".format(str(reference))
-    assert error is None
-    encrypted_data = create_delivery_status_callback_data(sample_notification, callback_api)
-    send_mock.assert_called_once_with([str(sample_notification.id), encrypted_data],
-                                      queue="service-callbacks")
-
-
-def test_sms_resonse_does_not_call_send_callback_if_no_db_entry(sample_notification, mocker):
-    mocker.patch(
-        'app.notifications.process_client_response.notifications_dao.update_notification_status_by_id',
-        return_value=sample_notification
-    )
+        'app.celery.process_sms_client_response_tasks.get_service_delivery_status_callback_api_for_service',
+        return_value='mock-delivery-callback-for-service')
     send_mock = mocker.patch(
         'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
     )
@@ -82,63 +69,54 @@ def test_sms_resonse_does_not_call_send_callback_if_no_db_entry(sample_notificat
     send_mock.assert_not_called()
 
 
-def test_process_sms_response_return_success_for_send_sms_code_reference(mocker):
-    success, error = process_sms_client_response(
-        status='000', provider_reference='send-sms-code', client_name='sms-client')
-    assert success == "{} callback succeeded: send-sms-code".format('sms-client')
-    assert error is None
+@freeze_time('2001-01-01T12:00:00')
+def test_process_sms_client_response_records_statsd_metrics(sample_notification, client, mocker):
+    mocker.patch('app.statsd_client.incr')
+    mocker.patch('app.statsd_client.timing_with_dates')
 
+    sample_notification.status = 'sending'
+    sample_notification.sent_at = datetime.utcnow()
 
-def test_process_sms_response_does_not_send_status_update_for_pending(sample_notification, mocker):
-    send_mock = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async')
-    process_sms_client_response(
-        status='2', provider_reference=str(sample_notification.id), client_name='firetext')
-    send_mock.assert_not_called()
+    process_sms_client_response('0', str(sample_notification.id), 'Firetext')
 
-
-def test_process_sms_updates_sent_by_with_client_name_if_not_in_noti(sample_notification):
-    sample_notification.sent_by = None
-    success, error = process_sms_client_response(
-        status='3', provider_reference=str(sample_notification.id), client_name='MMG')
-    assert error is None
-    assert success == 'MMG callback succeeded. reference {} updated'.format(sample_notification.id)
-    assert sample_notification.sent_by == 'mmg'
+    statsd_client.incr.assert_any_call("callback.firetext.delivered")
+    statsd_client.timing_with_dates.assert_any_call(
+        "callback.firetext.elapsed-time", datetime.utcnow(), sample_notification.sent_at
+    )
 
 
 def test_process_sms_updates_billable_units_if_zero(sample_notification):
     sample_notification.billable_units = 0
-    success, error = process_sms_client_response(
-        status='3', provider_reference=str(sample_notification.id), client_name='MMG')
-    assert error is None
-    assert success == 'MMG callback succeeded. reference {} updated'.format(sample_notification.id)
+    process_sms_client_response('3', str(sample_notification.id), 'MMG')
+
     assert sample_notification.billable_units == 1
 
 
-def test_process_sms_does_not_update_sent_by_if_already_set(mocker, sample_notification):
-    mock_update = mocker.patch('app.notifications.process_client_response.set_notification_sent_by')
-    sample_notification.sent_by = 'MMG'
-    process_sms_client_response(
-        status='3', provider_reference=str(sample_notification.id), client_name='MMG')
-    assert not mock_update.called
+def test_process_sms_response_does_not_send_service_callback_for_pending_notifications(sample_notification, mocker):
+    mocker.patch(
+        'app.celery.process_sms_client_response_tasks.get_service_delivery_status_callback_api_for_service',
+        return_value='fake-callback')
+    send_mock = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async')
+    process_sms_client_response('2', str(sample_notification.id), 'Firetext')
+    send_mock.assert_not_called()
 
 
-def test_process_sms_response_returns_error_bad_reference(mocker):
-    success, error = process_sms_client_response(
-        status='000', provider_reference='something-bad', client_name='sms-client')
-    assert success is None
-    assert error == "{} callback with invalid reference {}".format('sms-client', 'something-bad')
+def test_outcome_statistics_called_for_successful_callback(sample_notification, mocker):
+    send_mock = mocker.patch(
+        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+    )
+    callback_api = create_service_callback_api(service=sample_notification.service, url="https://original_url.com")
+    reference = str(sample_notification.id)
+
+    process_sms_client_response('3', reference, 'MMG')
+
+    encrypted_data = create_delivery_status_callback_data(sample_notification, callback_api)
+    send_mock.assert_called_once_with([reference, encrypted_data],
+                                      queue="service-callbacks")
 
 
-def test_process_sms_response_raises_client_exception_for_unknown_sms_client(mocker):
-    success, error = process_sms_client_response(
-        status='000', provider_reference=str(uuid.uuid4()), client_name='sms-client')
+def test_process_sms_updates_sent_by_with_client_name_if_not_in_noti(sample_notification):
+    sample_notification.sent_by = None
+    process_sms_client_response('3', str(sample_notification.id), 'MMG')
 
-    assert success is None
-    assert error == 'unknown sms client: {}'.format('sms-client')
-
-
-def test_process_sms_response_raises_client_exception_for_unknown_status(mocker):
-    with pytest.raises(ClientException) as e:
-        process_sms_client_response(status='000', provider_reference=str(uuid.uuid4()), client_name='Firetext')
-
-    assert "{} callback failed: status {} not found.".format('Firetext', '000') in str(e.value)
+    assert sample_notification.sent_by == 'mmg'

tests/app/test_config.py

@@ -65,7 +65,7 @@ def test_cloudfoundry_config_has_different_defaults():
 def test_queue_names_all_queues_correct():
     # Need to ensure that all_queues() only returns queue names used in API
     queues = QueueNames.all_queues()
-    assert len(queues) == 13
+    assert len(queues) == 14
     assert set([
         QueueNames.PRIORITY,
         QueueNames.PERIODIC,
@@ -80,4 +80,5 @@ def test_queue_names_all_queues_correct():
         QueueNames.CREATE_LETTERS_PDF,
         QueueNames.CALLBACKS,
         QueueNames.LETTERS,
+        QueueNames.SMS_CALLBACKS,
     ]) == set(queues)