Degrade inbound number not associated with service message from error to warning

app/celery/letters_pdf_tasks.py

@@ -172,18 +172,15 @@ def get_key_and_size_of_letters_to_be_sent_to_print(print_run_deadline):
 
     letter_pdfs = []
     for letter in letters_awaiting_sending:
-        try:
-            letter_file_name = get_letter_pdf_filename(
-                reference=letter.reference,
-                crown=letter.service.crown,
-                sending_date=letter.created_at,
-                postage=letter.postage
-            )
-            letter_head = s3.head_s3_object(current_app.config['LETTERS_PDF_BUCKET_NAME'], letter_file_name)
-            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}", e)
+        letter_file_name = get_letter_pdf_filename(
+            reference=letter.reference,
+            crown=letter.service.crown,
+            sending_date=letter.created_at,
+            postage=letter.postage
+        )
+
+        letter_head = s3.head_s3_object(current_app.config['LETTERS_PDF_BUCKET_NAME'], letter_file_name)
+        letter_pdfs.append({"Key": letter_file_name, "Size": letter_head['ContentLength']})
 
     return letter_pdfs
 

app/celery/scheduled_tasks.py

@@ -205,7 +205,7 @@ def replay_created_notifications():
 
         current_app.logger.info(msg)
         for letter in letters:
-            create_letters_pdf.apply_async([str(letter.id)], queue=QueueNames.LETTERS)
+            create_letters_pdf.apply_async([letter.id], queue=QueueNames.LETTERS)
 
 
 @notify_celery.task(name='check-precompiled-letter-state')

app/celery/tasks.py

@@ -4,7 +4,6 @@ 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
@@ -306,10 +305,6 @@ 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/commands.py

@@ -11,7 +11,7 @@ from click_datetime import Datetime as click_dt
 from flask import current_app, json
 from notifications_utils.recipients import RecipientCSV
 from notifications_utils.template import SMSMessageTemplate
-from sqlalchemy.exc import IntegrityError, SQLAlchemyError
+from sqlalchemy.exc import IntegrityError
 from sqlalchemy.orm.exc import NoResultFound
 from notifications_utils.statsd_decorators import statsd
 
@@ -45,6 +45,7 @@ from app.dao.templates_dao import dao_get_template_by_id
 from app.dao.users_dao import delete_model_user, delete_user_verify_codes, get_user_by_email
 from app.models import (
     PROVIDERS,
+    NOTIFICATION_CREATED,
     KEY_TYPE_TEST,
     SMS_TYPE,
     EMAIL_TYPE,
@@ -56,10 +57,6 @@ from app.models import (
     Service,
     EmailBranding,
     LetterBranding,
-    NOTIFICATION_CREATED,
-    NOTIFICATION_DELIVERED,
-    NOTIFICATION_PERMANENT_FAILURE,
-    NOTIFICATION_TEMPORARY_FAILURE,
 )
 from app.performance_platform.processing_time import send_processing_time_for_start_and_end
 from app.utils import get_london_midnight_in_utc, get_midnight_for_day_before
@@ -904,105 +901,3 @@ def process_row_from_job(job_id, job_row_number):
             notification_id = process_row(row, template, job, job.service)
             current_app.logger.info("Process row {} for job {} created notification_id: {}".format(
                 job_row_number, job_id, notification_id))
-
-
-@notify_command(name='delete-high-volume-service-data')
-@click.option('-i', '--service_id', required=True, help='Service id of the high volume service')
-@click.option('-s', '--start_date', required=True, type=click_dt(format='%Y-%m-%d %H'),
-              help='Start date of process YYYY-MM-DD HH:mm')
-@click.option('-e', '--end_date', required=True, type=click_dt(format='%Y-%m-%d %H'),
-              help='End date of process YYYY-MM-DD HH:MM')
-@click.option('-t', '--notification_type', required=False, default='email',
-              help='Notification type of the data to delete')
-def delete_high_volume_service_data(service_id, start_date, end_date, notification_type):
-    str_date = start_date.strftime('%Y_%m_%d_%H')
-    str_end_date = end_date.strftime('%Y_%m_%d_%H')
-
-    bkup_tble_name = f'back_up_notifications_{str_date}_to_{str_end_date}'
-
-    print(f"""Creating back up {bkup_tble_name} starting at: {datetime.utcnow()}
-          for {notification_type} notifications for service: {service_id}, starting at {start_date} and {end_date}
-         """)
-
-    _create_notification_bkup_table(bkup_tble_name, end_date, notification_type, service_id, start_date)
-
-    hour_start = start_date
-    hour_end = hour_start + timedelta(hours=1)
-
-    terminate_statuses = [NOTIFICATION_DELIVERED, NOTIFICATION_TEMPORARY_FAILURE, NOTIFICATION_PERMANENT_FAILURE]
-    delete_query = Notification.query.filter(
-        Notification.notification_type == notification_type,
-        Notification.service_id == service_id,
-        Notification.created_at >= hour_start,
-        Notification.created_at <= hour_end,
-        Notification.status.in_(terminate_statuses)
-    )
-    # Iterate hour by hour
-    del_count = 0
-    while hour_start < end_date:
-        del_count += delete_query.delete(synchronize_session=False)
-        db.session.commit()
-        # print(hour_start, hour_end)
-        # increment hour
-        hour_end = hour_end + timedelta(hours=1)
-        hour_start = hour_start + timedelta(hours=1)
-        delete_query = Notification.query.filter(
-            Notification.notification_type == notification_type,
-            Notification.service_id == service_id,
-            Notification.created_at >= hour_start,
-            Notification.created_at <= hour_end,
-            Notification.status.in_(terminate_statuses)
-        )
-
-    print(f"""Completed deleting {del_count} from notifications
-              for {notification_type} notifications for
-              service: {service_id}, starting at {start_date} and {end_date}
-             """)
-
-
-def _create_notification_bkup_table(bkup_tble_name, end_date, notification_type, service_id, start_date):
-    try:
-        create_tbl_sql = f"""
-            CREATE TABLE {bkup_tble_name} AS 
-                SELECT *
-                  FROM notifications
-                 WHERE service_id = :service_id
-                   AND notification_type = :notification_type
-                   AND created_at >= :start_date
-                   AND created_at <= :end_date
-                   AND key_type = 'normal'
-                   AND notification_status in ('delivered', 'permanent-failure', 'temporary-failure')
-        """
-        input_params = {
-            "service_id": service_id,
-            "notification_type": notification_type,
-            "start_date": start_date,
-            "end_date": end_date
-        }
-        db.session.execute(create_tbl_sql, input_params)
-        db.session.commit()
-    except SQLAlchemyError as e:
-        db.session.commit()  # terminate previous transaction
-        # This query isn't quite right yet, if the notifications are already deleted still get 0.
-        # however it doesn't cause any harm, because there is nothing to delete.
-        # But it will also return 0 if the rows exist in notifications
-        qry = f""" Select count(*) from notifications 
-          WHERE service_id = :service_id
-          AND notification_type = :notification_type
-          AND created_at >= :start_date
-          AND created_at <= :end_date
-          AND key_type = 'normal'
-          UNION 
-          SELECT count(*) 
-            FROM {bkup_tble_name} 
-        """
-        result = db.session.execute(qry, input_params).fetchall()
-        if result[0][0] == result[1][0]:
-            print("Table and data already exists, keep going")
-            return
-        else:
-            # This will throw the exception if the data is already deleted or partically deleted...
-            # but gives us a chance to see what's happending.
-            print(f"Table already exists but row counts are inconsistent is missing bail out. "
-                  f"There are {result[0][0]} rows in notifications and {result[1][0]} rows in {bkup_tble_name}")
-            raise e

app/config.py

@@ -28,7 +28,6 @@ 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'
 
@@ -48,7 +47,6 @@ class QueueNames(object):
             QueueNames.CREATE_LETTERS_PDF,
             QueueNames.CALLBACKS,
             QueueNames.LETTERS,
-            QueueNames.SMS_CALLBACKS,
         ]
 
 

app/dao/notifications_dao.py

@@ -307,14 +307,19 @@ def delete_notifications_older_than_retention_by_type(notification_type, qry_lim
     ).all()
     deleted = 0
     for f in flexible_data_retention:
-        current_app.logger.info(
-            "Deleting {} notifications for service id: {}".format(notification_type, f.service_id))
-
-        day_to_delete_backwards_from = get_london_midnight_in_utc(
+        days_of_retention = get_london_midnight_in_utc(
             convert_utc_to_bst(datetime.utcnow()).date()) - timedelta(days=f.days_of_retention)
 
-        deleted += _move_notifications_to_notification_history(
-            notification_type, f.service_id, day_to_delete_backwards_from, qry_limit)
+        if notification_type == LETTER_TYPE:
+            _delete_letters_from_s3(
+                notification_type, f.service_id, days_of_retention, qry_limit
+            )
+
+        insert_update_notification_history(notification_type, days_of_retention, f.service_id)
+
+        current_app.logger.info(
+            "Deleting {} notifications for service id: {}".format(notification_type, f.service_id))
+        deleted += _delete_notifications(notification_type, days_of_retention, f.service_id, qry_limit)
 
     current_app.logger.info(
         'Deleting {} notifications for services without flexible data retention'.format(notification_type))
@@ -324,54 +329,18 @@ def delete_notifications_older_than_retention_by_type(notification_type, qry_lim
     service_ids_to_purge = db.session.query(Service.id).filter(Service.id.notin_(services_with_data_retention)).all()
 
     for service_id in service_ids_to_purge:
-        deleted += _move_notifications_to_notification_history(
-            notification_type, service_id, seven_days_ago, qry_limit)
+        if notification_type == LETTER_TYPE:
+            _delete_letters_from_s3(
+                notification_type, service_id, seven_days_ago, qry_limit
+            )
+        insert_update_notification_history(notification_type, seven_days_ago, service_id)
+        deleted += _delete_notifications(notification_type, seven_days_ago, service_id, qry_limit)
 
     current_app.logger.info('Finished deleting {} notifications'.format(notification_type))
 
     return deleted
 
 
-def _move_notifications_to_notification_history(notification_type, service_id, day_to_delete_backwards_from, qry_limit):
-    deleted = 0
-    if notification_type == LETTER_TYPE:
-        _delete_letters_from_s3(
-            notification_type, service_id, day_to_delete_backwards_from, qry_limit
-        )
-
-    stop = -1  # exclusive, we want to include 0
-    step = -1
-    for hour_delta in range(23, stop, step):
-        # We find the timestamp we want to delete all notifications backwards from
-        # We then start 23 hours ago, and do an insert notification history before deleting all notifications older
-        # We then look 22 hours ago, do an insert notifications history before deleting all notifications older
-        # We continue this until we reach the original timestamp we wanted to delete notifications backwardsfrom
-        # This enables us to break this into smaller database queries
-        timestamp_to_delete_backwards_from = day_to_delete_backwards_from - timedelta(hours=hour_delta)
-
-        if service_id == '539d63a1-701d-400d-ab11-f3ee2319d4d4':
-            current_app.logger.info(
-                "Beginning insert_update_notification_history for GOV.UK Email from {} backwards".format(
-                    timestamp_to_delete_backwards_from
-                )
-            )
-
-        insert_update_notification_history(notification_type, timestamp_to_delete_backwards_from, service_id, qry_limit)
-
-        if service_id == '539d63a1-701d-400d-ab11-f3ee2319d4d4':
-            current_app.logger.info(
-                "Beginning _delete_notifications for GOV.UK Email {} backwards".format(
-                    timestamp_to_delete_backwards_from
-                )
-            )
-
-        deleted += _delete_notifications(
-            notification_type, timestamp_to_delete_backwards_from, service_id, qry_limit
-        )
-
-    return deleted
-
-
 def _delete_notifications(notification_type, date_to_delete_from, service_id, query_limit):
     subquery = db.session.query(
         Notification.id
@@ -420,8 +389,6 @@ def insert_update_notification_history(notification_type, date_to_delete_from, s
         Notification.service_id == service_id,
         Notification.created_at < date_to_delete_from,
         Notification.key_type != KEY_TYPE_TEST
-    ).order_by(
-        Notification.created_at
     )
     notifications_count = notification_query.count()
 

app/job/rest.py

@@ -141,6 +141,7 @@ def create_job(service_id):
         raise InvalidRequest("Create job is not allowed: service is inactive ", 403)
 
     data = request.get_json()
+
     data.update({
         "service": service_id
     })

app/notifications/notifications_sms_callback.py

@@ -3,9 +3,8 @@ 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)
@@ -21,21 +20,19 @@ def process_mmg_response():
     if errors:
         raise InvalidRequest(errors, status_code=400)
 
-    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,
-    )
+    success, errors = process_sms_client_response(status=str(data.get('status')),
+                                                  provider_reference=data.get('CID'),
+                                                  client_name=client_name)
 
     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
+        "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
 
 
 @sms_callback_blueprint.route('/firetext', methods=['POST'])
@@ -46,28 +43,15 @@ 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(
-        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
+        "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

app/notifications/process_client_response.py

@@ -1,17 +1,20 @@
 import uuid
-from datetime import datetime
 
+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 notify_celery, statsd_client
+from app import 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 import notifications_dao
+from app.dao.notifications_dao import dao_update_notification
 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
@@ -22,23 +25,39 @@ sms_response_mapper = {
 }
 
 
-@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):
+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
     # 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 as e:
-        current_app.logger.exception(f'{client_name} callback with invalid reference {provider_reference}')
-        raise e
+    except ValueError:
+        errors = "{} callback with invalid reference {}".format(client_name, provider_reference)
+        return success, errors
 
-    response_parser = sms_response_mapper[client_name]
+    try:
+        response_parser = sms_response_mapper[client_name]
+    except KeyError:
+        return success, 'unknown sms client: {}'.format(client_name)
 
-    # validate status
+    # validate  status
     try:
         notification_status = response_parser(status)
-        current_app.logger.info(
-            f'{client_name} callback returned status of {status} for reference: {provider_reference}'
+        current_app.logger.info('{} callback return status of {} for reference: {}'.format(
+            client_name, status, provider_reference)
         )
     except KeyError:
         _process_for_status(
@@ -46,13 +65,14 @@ def process_sms_client_response(self, status, provider_reference, client_name):
             client_name=client_name,
             provider_reference=provider_reference
         )
-        raise ClientException(f'{client_name} callback failed: status {status} not found.')
+        raise ClientException("{} callback failed: status {} not found.".format(client_name, status))
 
-    _process_for_status(
+    success = _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):
@@ -94,3 +114,11 @@ 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/notifications/receive_notifications.py

@@ -149,7 +149,7 @@ def fetch_potential_service(inbound_number, provider_name):
     service = dao_fetch_service_by_inbound_number(inbound_number)
 
     if not service:
-        current_app.logger.error('Inbound number "{}" from {} not associated with a service'.format(
+        current_app.logger.warning('Inbound number "{}" from {} not associated with a service'.format(
             inbound_number, provider_name
         ))
         statsd_client.incr('inbound.{}.failed'.format(provider_name))

app/notifications/validators.py

@@ -50,8 +50,7 @@ def check_service_over_daily_message_limit(key_type, service):
 
 def check_rate_limiting(service, api_key):
     check_service_over_api_rate_limit(service, api_key)
-    # Reduce queries to the notifications table
-    # check_service_over_daily_message_limit(api_key.key_type, service)
+    check_service_over_daily_message_limit(api_key.key_type, service)
 
 
 def check_template_is_for_notification_type(notification_type, template_type):

app/schemas.py

@@ -381,7 +381,6 @@ class JobSchema(BaseSchema):
         ServiceSchema, attribute="service", dump_to="service_name", only=["name"], dump_only=True)
 
     template_type = fields.Method('get_template_type', dump_only=True)
-    contact_list_id = field_for(models.Job, 'contact_list_id')
 
     def get_template_type(self, job):
         return job.template.template_type

manifest.yml.j2

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

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.9.0#egg=notifications-utils==36.9.0
+git+https://github.com/alphagov/notifications-utils.git@36.6.2#egg=notifications-utils==36.6.2

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.9.0#egg=notifications-utils==36.9.0
+git+https://github.com/alphagov/notifications-utils.git@36.6.2#egg=notifications-utils==36.6.2
 
 ## 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.20
+awscli==1.18.16
 bcrypt==3.1.7
 billiard==3.3.0.23
 bleach==3.1.1
 boto==2.49.0
 boto3==1.10.38
-botocore==1.15.20
+botocore==1.15.16
 certifi==2019.11.28
 chardet==3.0.4
-click==7.1.1
+click==7.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,sms-callbacks 2> /dev/null
+    -Q ses-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_letters_pdf_tasks.py

@@ -315,48 +315,6 @@ def test_get_key_and_size_of_letters_to_be_sent_to_print(notify_api, mocker, sam
     ]
 
 
-@freeze_time('2020-02-17 18:00:00')
-def test_get_key_and_size_of_letters_to_be_sent_to_print_catches_exception(
-    notify_api, mocker, sample_letter_template
-):
-    create_notification(
-        template=sample_letter_template,
-        status='created',
-        reference='ref0',
-        created_at=(datetime.now() - timedelta(hours=2))
-    )
-
-    create_notification(
-        template=sample_letter_template,
-        status='created',
-        reference='ref1',
-        created_at=(datetime.now() - timedelta(hours=3))
-    )
-    error_response = {
-        'Error': {
-            'Code': 'FileNotFound',
-            'Message': 'some error message from amazon',
-            'Type': 'Sender'
-        }
-    }
-    mock_head_s3_object = mocker.patch('app.celery.tasks.s3.head_s3_object', side_effect=[
-        {'ContentLength': 2},
-        ClientError(error_response, "File not found")
-    ])
-
-    results = get_key_and_size_of_letters_to_be_sent_to_print(datetime.now() - timedelta(minutes=30))
-
-    assert mock_head_s3_object.call_count == 2
-    mock_head_s3_object.assert_has_calls(
-        [
-            call(current_app.config['LETTERS_PDF_BUCKET_NAME'], '2020-02-17/NOTIFY.REF1.D.2.C.C.20200217150000.PDF'),
-            call(current_app.config['LETTERS_PDF_BUCKET_NAME'], '2020-02-17/NOTIFY.REF0.D.2.C.C.20200217160000.PDF'),
-        ]
-    )
-
-    assert results == [{'Key': '2020-02-17/NOTIFY.REF1.D.2.C.C.20200217150000.PDF', 'Size': 2}]
-
-
 @pytest.mark.parametrize('time_to_run_task', [
     "2020-02-17 18:00:00",  # after 5:30pm
     "2020-02-18 02:00:00",  # the next day after midnight, before 5:30pm we expect the same results

tests/app/celery/test_scheduled_tasks.py

@@ -329,8 +329,8 @@ def test_replay_created_notifications_create_letters_pdf_tasks_for_letters_not_r
 
     replay_created_notifications()
 
-    calls = [call([str(notification_1.id)], queue=QueueNames.LETTERS),
-             call([str(notification_2.id)], queue=QueueNames.LETTERS),
+    calls = [call([notification_1.id], queue=QueueNames.LETTERS),
+             call([notification_2.id], queue=QueueNames.LETTERS),
              ]
     mock_task.assert_has_calls(calls, any_order=True)
 

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': 'SE1 2SA',
+        'postcode': 'Flob',
     }
     notification_json = _notification_json(
         template=job.template,
@@ -1021,31 +1021,6 @@ 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)
@@ -1062,7 +1037,7 @@ def test_save_letter_saves_letter_to_database_right_reply_to(mocker, notify_db_s
         'addressline4': 'Wibble',
         'addressline5': 'Wobble',
         'addressline6': 'Wubble',
-        'postcode': 'SE1 3WS',
+        'postcode': 'Flob',
     }
     notification_json = _notification_json(
         template=job.template,

tests/app/job/test_rest.py

@@ -13,12 +13,7 @@ from app.models import JOB_STATUS_TYPES, JOB_STATUS_PENDING
 
 from tests import create_authorization_header
 from tests.conftest import set_config
-from tests.app.db import (
-    create_ft_notification_status,
-    create_job,
-    create_notification,
-    create_service_contact_list
-)
+from tests.app.db import create_ft_notification_status, create_job, create_notification
 
 
 def test_get_job_with_invalid_service_id_returns404(client, sample_service):
@@ -238,30 +233,6 @@ def test_create_scheduled_job(client, sample_template, mocker, fake_uuid):
     assert resp_json['data']['notification_count'] == 1
 
 
-def test_create_job_with_contact_list_id(client, mocker, sample_template, fake_uuid):
-    mocker.patch('app.celery.tasks.process_job.apply_async')
-    mocker.patch('app.job.rest.get_job_metadata_from_s3', return_value={
-        'template_id': str(sample_template.id)
-    })
-    contact_list = create_service_contact_list()
-    data = {
-        'id': fake_uuid,
-        'valid': 'True',
-        'original_file_name': contact_list.original_file_name,
-        'created_by': str(sample_template.service.users[0].id),
-        'notification_count': 100,
-        'contact_list_id': str(contact_list.id),
-    }
-    response = client.post(
-        f'/service/{sample_template.service_id}/job',
-        data=json.dumps(data),
-        headers=[('Content-Type', 'application/json'), create_authorization_header()])
-    resp_json = response.get_json()
-    assert response.status_code == 201
-    assert resp_json['data']['contact_list_id'] == str(contact_list.id)
-    assert resp_json['data']['original_file_name'] == 'EmergencyContactList.xls'
-
-
 def test_create_job_returns_403_if_service_is_not_active(client, fake_uuid, sample_service, mocker):
     sample_service.active = False
     mock_job_dao = mocker.patch("app.dao.jobs_dao.dao_create_job")
@@ -712,7 +683,6 @@ def test_get_jobs(admin_request, sample_template):
         'template_type': 'sms',
         'template_version': 1,
         'updated_at': None,
-        'contact_list_id': None
     }
 
 

tests/app/notifications/rest/test_callbacks.py

@@ -1,7 +1,17 @@
+import uuid
+
+from datetime import datetime
+
 import pytest
 from flask import json
+from freezegun import freeze_time
 
-from app.notifications.notifications_sms_callback import validate_callback_data
+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
 
 
 def firetext_post(client, data):
@@ -101,14 +111,15 @@ 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.notifications.notifications_sms_callback.process_sms_client_response')
-    data = 'mobile=441234123123&status=0&reference=notification_id&time=2016-03-10 14:17:00'
+    mocker.patch('app.statsd_client.incr')
+    data = 'mobile=441234123123&status=0&reference=send-sms-code&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)
 
@@ -119,6 +130,7 @@ 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))
@@ -127,8 +139,19 @@ 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):
-    data = 'mobile=441234123123&time=2016-03-10 14:17:00&reference=notification_id'
+    mocker.patch('app.statsd_client.incr')
+    data = 'mobile=441234123123&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 == 400
@@ -136,24 +159,136 @@ 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_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_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)
 
-    data = 'mobile=441234123123&status=0&time=2016-03-10 14:17:00&reference=notification_id'
+
+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'
     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'
 
-    mock_celery.assert_called_once_with(
-        ['0', 'notification_id', 'Firetext'],
-        queue='sms-callbacks',
+
+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'
     )
+    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_mmg_callback_should_not_need_auth(client, mocker, sample_notification):
-    mocker.patch('app.notifications.notifications_sms_callback.process_sms_client_response')
+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'
     data = json.dumps({"reference": "mmg_reference",
                        "CID": str(sample_notification.id),
                        "MSISDN": "447777349060",
@@ -161,7 +296,93 @@ def test_mmg_callback_should_not_need_auth(client, mocker, sample_notification):
                        "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):
@@ -180,64 +401,68 @@ 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_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"})
+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"}'
 
     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_validate_callback_data_returns_none_when_valid():
-    form = {'status': 'good',
-            'reference': 'send-sms-code'}
-    fields = ['status', 'reference']
-    client_name = 'sms client'
+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"}'
 
-    assert validate_callback_data(form, fields, client_name) is None
+    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'
 
 
-def test_validate_callback_data_return_errors_when_fields_are_empty():
-    form = {'monkey': 'good'}
-    fields = ['status', 'cid']
-    client_name = 'sms client'
+def test_process_mmg_response_records_statsd(sample_notification, client, mocker):
+    with freeze_time('2001-01-01T12:00:00'):
 
-    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
+        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
+        )
 
 
-def test_validate_callback_data_can_handle_integers():
-    form = {'status': 00, 'cid': 'fsdfadfsdfas'}
-    fields = ['status', 'cid']
-    client_name = 'sms client'
+def test_firetext_callback_should_record_statsd(client, sample_notification, mocker):
+    with freeze_time('2001-01-01T12:00:00'):
 
-    result = validate_callback_data(form, fields, client_name)
-    assert result is None
+        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 = 'mobile=441234123123&status=0&time=2016-03-10 14:17:00&code=101&reference={}'.format(
+            sample_notification.id)
+        firetext_post(client, data)
 
-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
+        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 _sample_sns_s3_callback(filename):

tests/app/notifications/rest/test_send_notification.py

@@ -18,7 +18,7 @@ from app.dao.services_dao import dao_update_service
 from app.dao.api_key_dao import save_model_api_key
 from app.errors import InvalidRequest
 from app.models import Template
-from app.v2.errors import RateLimitError
+from app.v2.errors import RateLimitError, TooManyRequestsError
 
 from tests import create_authorization_header
 from tests.app.db import (
@@ -404,6 +404,69 @@ def test_should_allow_valid_email_notification(notify_api, sample_email_template
             assert response_data['template_version'] == sample_email_template.version
 
 
+@freeze_time("2016-01-01 12:00:00.061258")
+def test_should_block_api_call_if_over_day_limit_for_live_service(
+        notify_db_session,
+        notify_api,
+        mocker):
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            mocker.patch(
+                'app.notifications.validators.check_service_over_daily_message_limit',
+                side_effect=TooManyRequestsError(1)
+            )
+            mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
+            service = create_service(message_limit=1)
+            email_template = create_template(service, template_type=EMAIL_TYPE)
+            create_notification(template=email_template)
+
+            data = {
+                'to': 'ok@ok.com',
+                'template': str(email_template.id)
+            }
+
+            auth_header = create_authorization_header(service_id=service.id)
+
+            response = client.post(
+                path='/notifications/email',
+                data=json.dumps(data),
+                headers=[('Content-Type', 'application/json'), auth_header])
+            json.loads(response.get_data(as_text=True))
+            assert response.status_code == 429
+
+
+@freeze_time("2016-01-01 12:00:00.061258")
+def test_should_block_api_call_if_over_day_limit_for_restricted_service(
+        notify_db_session,
+        notify_api,
+        mocker):
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
+            mocker.patch(
+                'app.notifications.validators.check_service_over_daily_message_limit',
+                side_effect=TooManyRequestsError(1)
+            )
+            service = create_service(restricted=True, message_limit=1)
+            email_template = create_template(service, template_type=EMAIL_TYPE)
+            create_notification(template=email_template)
+
+            data = {
+                'to': 'ok@ok.com',
+                'template': str(email_template.id)
+            }
+
+            auth_header = create_authorization_header(service_id=service.id)
+
+            response = client.post(
+                path='/notifications/email',
+                data=json.dumps(data),
+                headers=[('Content-Type', 'application/json'), auth_header])
+            json.loads(response.get_data(as_text=True))
+
+            assert response.status_code == 429
+
+
 @pytest.mark.parametrize('restricted', [True, False])
 @freeze_time("2016-01-01 12:00:00.061258")
 def test_should_allow_api_call_if_under_day_limit_regardless_of_type(

tests/app/notifications/test_process_client_response.py

@@ -1,66 +1,79 @@
 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.celery.process_sms_client_response_tasks import process_sms_client_response
+from app.notifications.process_client_response import (
+    validate_callback_data,
+    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_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_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
 
 
-@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,
-        )
+def test_validate_callback_data_return_errors_when_fields_are_empty():
+    form = {'monkey': 'good'}
+    fields = ['status', 'cid']
+    client_name = 'sms client'
 
-    assert f"{client_name} callback failed: status {'000'} not found." in str(e.value)
-    assert sample_notification.status == NOTIFICATION_TECHNICAL_FAILURE
+    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
 
 
-@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)
+def test_validate_callback_data_can_handle_integers():
+    form = {'status': 00, 'cid': 'fsdfadfsdfas'}
+    fields = ['status', 'cid']
+    client_name = 'sms client'
 
-    assert sample_notification.status == expected_notification_status
+    result = validate_callback_data(form, fields, client_name)
+    assert result is None
 
 
-def test_sms_response_does_not_send_callback_if_notification_is_not_in_the_db(sample_service, mocker):
+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):
     mocker.patch(
-        'app.celery.process_sms_client_response_tasks.get_service_delivery_status_callback_api_for_service',
-        return_value='mock-delivery-callback-for-service')
+        '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
+    )
     send_mock = mocker.patch(
         'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
     )
@@ -69,54 +82,63 @@ def test_sms_response_does_not_send_callback_if_notification_is_not_in_the_db(sa
     send_mock.assert_not_called()
 
 
-@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()
-
-    process_sms_client_response('0', str(sample_notification.id), 'Firetext')
-
-    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_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
 
 
-def test_process_sms_updates_billable_units_if_zero(sample_notification):
-    sample_notification.billable_units = 0
-    process_sms_client_response('3', str(sample_notification.id), 'MMG')
-
-    assert sample_notification.billable_units == 1
-
-
-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')
+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('2', str(sample_notification.id), 'Firetext')
+    process_sms_client_response(
+        status='2', provider_reference=str(sample_notification.id), client_name='firetext')
     send_mock.assert_not_called()
 
 
-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_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')
-
+    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'
+
+
+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)
+    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_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_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')
+
+    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)

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) == 14
+    assert len(queues) == 13
     assert set([
         QueueNames.PRIORITY,
         QueueNames.PERIODIC,
@@ -80,5 +80,4 @@ def test_queue_names_all_queues_correct():
         QueueNames.CREATE_LETTERS_PDF,
         QueueNames.CALLBACKS,
         QueueNames.LETTERS,
-        QueueNames.SMS_CALLBACKS,
     ]) == set(queues)