Added a test

By added the notification_type to the filter we get a better performance on the query. Especially when selecting for NotificationHistory.

app/celery/letters_pdf_tasks.py

@@ -42,6 +42,7 @@ from app.models import (
     NOTIFICATION_TECHNICAL_FAILURE,
     NOTIFICATION_VALIDATION_FAILED,
     NOTIFICATION_VIRUS_SCAN_FAILED,
+    LETTER_TYPE
 )
 from app.cronitor import cronitor
 
@@ -216,7 +217,7 @@ def group_letters(letter_pdfs):
 def sanitise_letter(self, filename):
     try:
         reference = get_reference_from_filename(filename)
-        notification = dao_get_notification_by_reference(reference)
+        notification = dao_get_notification_by_reference(reference=reference, notification_type=LETTER_TYPE)
 
         current_app.logger.info('Notification ID {} Virus scan passed: {}'.format(notification.id, filename))
 
@@ -352,7 +353,7 @@ def _move_invalid_letter_and_update_status(
 def process_virus_scan_failed(filename):
     move_failed_pdf(filename, ScanErrorType.FAILURE)
     reference = get_reference_from_filename(filename)
-    notification = dao_get_notification_by_reference(reference)
+    notification = dao_get_notification_by_reference(reference=reference, notification_type=LETTER_TYPE)
     updated_count = update_letter_pdf_status(reference, NOTIFICATION_VIRUS_SCAN_FAILED, billable_units=0)
 
     if updated_count != 1:
@@ -371,7 +372,7 @@ def process_virus_scan_failed(filename):
 def process_virus_scan_error(filename):
     move_failed_pdf(filename, ScanErrorType.ERROR)
     reference = get_reference_from_filename(filename)
-    notification = dao_get_notification_by_reference(reference)
+    notification = dao_get_notification_by_reference(reference=reference, notification_type=LETTER_TYPE)
     updated_count = update_letter_pdf_status(reference, NOTIFICATION_TECHNICAL_FAILURE, billable_units=0)
 
     if updated_count != 1:

app/celery/process_ses_receipts_tasks.py

@@ -10,7 +10,7 @@ from app import notify_celery, statsd_client
 from app.config import QueueNames
 from app.clients.email.aws_ses import get_aws_responses
 from app.dao import notifications_dao
-from app.models import NOTIFICATION_SENDING, NOTIFICATION_PENDING
+from app.models import NOTIFICATION_SENDING, NOTIFICATION_PENDING, EMAIL_TYPE
 
 from app.notifications.notifications_ses_callback import (
     determine_notification_bounce_type,
@@ -39,7 +39,9 @@ def process_ses_results(self, response):
         reference = ses_message['mail']['messageId']
 
         try:
-            notification = notifications_dao.dao_get_notification_or_history_by_reference(reference=reference)
+            notification = notifications_dao.dao_get_notification_or_history_by_reference(
+                reference=reference, notification_type=EMAIL_TYPE
+            )
         except NoResultFound:
             message_time = iso8601.parse_date(ses_message['mail']['timestamp']).replace(tzinfo=None)
             if datetime.utcnow() - message_time < timedelta(minutes=5):

app/celery/tasks.py

@@ -536,7 +536,7 @@ def update_letter_notification(filename, temporary_failures, update):
 
 
 def check_billable_units(notification_update):
-    notification = dao_get_notification_or_history_by_reference(notification_update.reference)
+    notification = dao_get_notification_or_history_by_reference(notification_update.reference, LETTER_TYPE)
 
     if int(notification_update.page_count) != notification.billable_units:
         msg = 'Notification with id {} has {} billable_units but DVLA says page count is {}'.format(

app/dao/notifications_dao.py

@@ -650,33 +650,29 @@ def dao_get_notifications_by_recipient_or_reference(
 
 
 @statsd(namespace="dao")
-def dao_get_notification_by_reference(reference):
+def dao_get_notification_by_reference(reference, notification_type):
     return Notification.query.filter(
-        Notification.reference == reference
+        Notification.reference == reference,
+        Notification.notification_type == notification_type
     ).one()
 
 
 @statsd(namespace="dao")
-def dao_get_notification_or_history_by_reference(reference):
+def dao_get_notification_or_history_by_reference(reference, notification_type):
     try:
         # This try except is necessary because in test keys and research mode does not create notification history.
         # Otherwise we could just search for the NotificationHistory object
         return Notification.query.filter(
-            Notification.reference == reference
+            Notification.reference == reference,
+            Notification.notification_type == notification_type
         ).one()
     except NoResultFound:
         return NotificationHistory.query.filter(
-            NotificationHistory.reference == reference
+            NotificationHistory.reference == reference,
+            NotificationHistory.notification_type == notification_type
         ).one()
 
 
-@statsd(namespace="dao")
-def dao_get_notifications_by_references(references):
-    return Notification.query.filter(
-        Notification.reference.in_(references)
-    ).all()
-
-
 @statsd(namespace="dao")
 def dao_created_scheduled_notification(scheduled_notification):
     db.session.add(scheduled_notification)

app/notifications/notifications_ses_callback.py

@@ -5,7 +5,7 @@ from app.dao.notifications_dao import dao_get_notification_or_history_by_referen
 from app.dao.service_callback_api_dao import (
     get_service_delivery_status_callback_api_for_service, get_service_complaint_callback_api_for_service
 )
-from app.models import Complaint
+from app.models import Complaint, EMAIL_TYPE
 from app.celery.service_callback_tasks import (
     send_delivery_status_to_service,
     send_complaint_to_service,
@@ -33,7 +33,7 @@ def handle_complaint(ses_message):
     except KeyError as e:
         current_app.logger.exception("Complaint from SES failed to get reference from message", e)
         return
-    notification = dao_get_notification_or_history_by_reference(reference)
+    notification = dao_get_notification_or_history_by_reference(reference, EMAIL_TYPE)
     ses_complaint = ses_message.get('complaint', None)
 
     complaint = Complaint(

app/notifications/process_letter_notifications.py

@@ -5,9 +5,7 @@ from app.models import LETTER_TYPE
 from app.notifications.process_notifications import persist_notification
 
 
-def create_letter_notification(
-    letter_data, template, api_key, status, reply_to_text=None, billable_units=None, updated_at=None
-):
+def create_letter_notification(letter_data, template, api_key, status, reply_to_text=None, billable_units=None):
     notification = persist_notification(
         template_id=template.id,
         template_version=template.version,
@@ -26,7 +24,6 @@ def create_letter_notification(
         status=status,
         reply_to_text=reply_to_text,
         billable_units=billable_units,
-        postage=letter_data.get('postage'),
-        updated_at=updated_at
+        postage=letter_data.get('postage')
     )
     return notification

app/notifications/process_notifications.py

@@ -79,8 +79,7 @@ def persist_notification(
     billable_units=None,
     postage=None,
     template_postage=None,
-    document_download_count=None,
-    updated_at=None
+    document_download_count=None
 ):
     notification_created_at = created_at or datetime.utcnow()
     if not notification_id:
@@ -106,7 +105,6 @@ def persist_notification(
         reply_to_text=reply_to_text,
         billable_units=billable_units,
         document_download_count=document_download_count,
-        updated_at=updated_at
     )
 
     if notification_type == SMS_TYPE:
@@ -122,17 +120,13 @@ def persist_notification(
         notification.postage = postage or template_postage
         notification.normalised_to = ''.join(notification.to.split()).lower()
 
-    # Get service attributes before the commit
-    service_in_trial_mode = service.restricted
-    service_id = service.id
-
     # if simulated create a Notification model to return but do not persist the Notification to the dB
     if not simulated:
         dao_create_notification(notification)
-        # Only keep track of the daily limit for trial mode services.
-        if service_in_trial_mode and key_type != KEY_TYPE_TEST:
-            if redis_store.get(redis.daily_limit_cache_key(service_id)):
-                redis_store.incr(redis.daily_limit_cache_key(service_id))
+        if key_type != KEY_TYPE_TEST:
+            with REDIS_GET_AND_INCR_DAILY_LIMIT_DURATION_SECONDS.time():
+                if redis_store.get(redis.daily_limit_cache_key(service.id)):
+                    redis_store.incr(redis.daily_limit_cache_key(service.id))
 
         current_app.logger.info(
             "{} {} created at {}".format(notification_type, notification_id, notification_created_at)
@@ -140,43 +134,35 @@ def persist_notification(
     return notification
 
 
-def send_notification_to_queue_detached(
-    key_type, notification_type, notification_id, research_mode, queue=None
-):
-    if research_mode or key_type == KEY_TYPE_TEST:
+def send_notification_to_queue(notification, research_mode, queue=None):
+    if research_mode or notification.key_type == KEY_TYPE_TEST:
         queue = QueueNames.RESEARCH_MODE
 
-    if notification_type == SMS_TYPE:
+    if notification.notification_type == SMS_TYPE:
         if not queue:
             queue = QueueNames.SEND_SMS
         deliver_task = provider_tasks.deliver_sms
-    if notification_type == EMAIL_TYPE:
+    if notification.notification_type == EMAIL_TYPE:
         if not queue:
             queue = QueueNames.SEND_EMAIL
         deliver_task = provider_tasks.deliver_email
-    if notification_type == LETTER_TYPE:
+    if notification.notification_type == LETTER_TYPE:
         if not queue:
             queue = QueueNames.CREATE_LETTERS_PDF
         deliver_task = get_pdf_for_templated_letter
 
     try:
-        deliver_task.apply_async([str(notification_id)], queue=queue)
+        deliver_task.apply_async([str(notification.id)], queue=queue)
     except Exception:
-        dao_delete_notifications_by_id(notification_id)
+        dao_delete_notifications_by_id(notification.id)
         raise
 
     current_app.logger.debug(
-        "{} {} sent to the {} queue for delivery".format(notification_type,
-                                                         notification_id,
+        "{} {} sent to the {} queue for delivery".format(notification.notification_type,
+                                                         notification.id,
                                                          queue))
 
 
-def send_notification_to_queue(notification, research_mode, queue=None):
-    send_notification_to_queue_detached(
-        notification.key_type, notification.notification_type, notification.id, research_mode, queue
-    )
-
-
 def simulated_recipient(to_address, notification_type):
     if notification_type == SMS_TYPE:
         formatted_simulated_numbers = [

app/schemas.py

@@ -235,18 +235,6 @@ class ServiceSchema(BaseSchema):
             'letter_contacts',
             'complaints',
             'data_retention',
-            'all_template_folders',
-            'annual_billing',
-            'contact_list',
-            'crown',
-            'inbound_number',
-            'inbound_sms',
-            'letter_logo_filename',
-            'rate_limit',
-            'returned_letters',
-            'users',
-            'version',
-            'whitelist',
         )
         strict = True
 
@@ -301,18 +289,7 @@ class DetailedServiceSchema(BaseSchema):
             'sms_sender',
             'permissions',
             'inbound_number',
-            'inbound_sms',
-            'all_template_folders',
-            'annual_billing',
-            'contact_list',
-            'created_by',
-            'crown',
-            'letter_logo_filename',
-            'rate_limit',
-            'returned_letters',
-            'users',
-            'version',
-            'whitelist',
+            'inbound_sms'
         )
 
 

app/v2/notifications/create_response.py

@@ -1,70 +1,45 @@
 
 
-def create_post_sms_response_from_notification(
-    notification_id, client_reference, template_id, template_version, service_id,
-    content, from_number, url_root, scheduled_for
-):
-    resp = __create_notification_response(
-        notification_id, client_reference, template_id, template_version, service_id, url_root, scheduled_for
-    )
-    resp['content'] = {
+def create_post_sms_response_from_notification(notification, content, from_number, url_root, scheduled_for):
+    noti = __create_notification_response(notification, url_root, scheduled_for)
+    noti['content'] = {
         'from_number': from_number,
         'body': content
     }
-    return resp
+    return noti
 
 
-def create_post_email_response_from_notification(
-    notification_id,
-    client_reference,
-    template_id,
-    template_version,
-    service_id,
-    content,
-    subject,
-    email_from,
-    url_root,
-    scheduled_for
-):
-    resp = __create_notification_response(
-        notification_id, client_reference, template_id, template_version, service_id, url_root, scheduled_for
-    )
-    resp['content'] = {
+def create_post_email_response_from_notification(notification, content, subject, email_from, url_root, scheduled_for):
+    noti = __create_notification_response(notification, url_root, scheduled_for)
+    noti['content'] = {
         "from_email": email_from,
         "body": content,
         "subject": subject
     }
-    return resp
+    return noti
 
 
-def create_post_letter_response_from_notification(
-    notification_id, client_reference, template_id, template_version, service_id,
-    content, subject, url_root, scheduled_for
-):
-    resp = __create_notification_response(
-        notification_id, client_reference, template_id, template_version, service_id, url_root, scheduled_for
-    )
-    resp['content'] = {
+def create_post_letter_response_from_notification(notification, content, subject, url_root, scheduled_for):
+    noti = __create_notification_response(notification, url_root, scheduled_for)
+    noti['content'] = {
         "body": content,
         "subject": subject
     }
-    return resp
+    return noti
 
 
-def __create_notification_response(
-    notification_id, client_reference, template_id, template_version, service_id, url_root, scheduled_for
-):
+def __create_notification_response(notification, url_root, scheduled_for):
     return {
-        "id": notification_id,
-        "reference": client_reference,
-        "uri": "{}v2/notifications/{}".format(url_root, str(notification_id)),
+        "id": notification.id,
+        "reference": notification.client_reference,
+        "uri": "{}v2/notifications/{}".format(url_root, str(notification.id)),
         'template': {
-            "id": template_id,
-            "version": template_version,
+            "id": notification.template_id,
+            "version": notification.template_version,
             "uri": "{}services/{}/templates/{}".format(
                 url_root,
-                str(service_id),
-                str(template_id)
+                str(notification.service_id),
+                str(notification.template_id)
             )
         },
         "scheduled_for": scheduled_for if scheduled_for else None

app/v2/notifications/post_notifications.py

@@ -15,13 +15,14 @@ from app import (
     notify_celery,
     document_download_client,
     encryption,
-    DATETIME_FORMAT,
-    db)
+    DATETIME_FORMAT
+)
 from app.celery.letters_pdf_tasks import get_pdf_for_templated_letter, sanitise_letter
 from app.celery.research_mode_tasks import create_fake_letter_response_file
 from app.celery.tasks import save_api_email
 from app.clients.document_download import DocumentDownloadError
 from app.config import QueueNames, TaskNames
+from app.dao.notifications_dao import update_notification_status_by_reference
 from app.dao.templates_dao import get_precompiled_letter_template
 from app.letters.utils import upload_letter_pdf
 from app.models import (
@@ -44,8 +45,9 @@ from app.notifications.process_letter_notifications import (
 from app.notifications.process_notifications import (
     persist_notification,
     persist_scheduled_notification,
-    simulated_recipient,
-    send_notification_to_queue_detached)
+    send_notification_to_queue,
+    simulated_recipient
+)
 from app.notifications.validators import (
     check_if_service_can_send_files_by_email,
     check_rate_limiting,
@@ -60,8 +62,10 @@ from app.schema_validation import validate
 from app.v2.errors import BadRequestError, ValidationError
 from app.v2.notifications import v2_notification_blueprint
 from app.v2.notifications.create_response import (
-    create_post_sms_response_from_notification, create_post_email_response_from_notification,
-    create_post_letter_response_from_notification)
+    create_post_sms_response_from_notification,
+    create_post_email_response_from_notification,
+    create_post_letter_response_from_notification
+)
 from app.v2.notifications.notification_schemas import (
     post_sms_request,
     post_email_request,
@@ -104,12 +108,17 @@ def post_precompiled_letter_notification():
         letter_data=form,
         api_key=api_user,
         template=template,
-        template_with_content=None,  # not required for precompiled
         reply_to_text=reply_to,
         precompiled=True
     )
 
-    return jsonify(notification), 201
+    resp = {
+        'id': notification.id,
+        'reference': notification.client_reference,
+        'postage': notification.postage
+    }
+
+    return jsonify(resp), 201
 
 
 @v2_notification_blueprint.route('/<notification_type>', methods=['POST'])
@@ -148,7 +157,6 @@ def post_notification(notification_type):
             letter_data=form,
             api_key=api_user,
             template=template,
-            template_with_content=template_with_content,
             reply_to_text=reply_to
         )
     else:
@@ -156,19 +164,41 @@ def post_notification(notification_type):
             form=form,
             notification_type=notification_type,
             api_key=api_user,
-            template=template_with_content,
-            template_process_type=template.process_type,
+            template=template,
             service=authenticated_service,
             reply_to_text=reply_to
         )
 
-    return jsonify(notification), 201
+        template_with_content.values = notification.personalisation
+
+    if notification_type == SMS_TYPE:
+        create_resp_partial = functools.partial(
+            create_post_sms_response_from_notification,
+            from_number=reply_to,
+        )
+    elif notification_type == EMAIL_TYPE:
+        create_resp_partial = functools.partial(
+            create_post_email_response_from_notification,
+            subject=template_with_content.subject,
+            email_from='{}@{}'.format(authenticated_service.email_from, current_app.config['NOTIFY_EMAIL_DOMAIN']),
+        )
+    elif notification_type == LETTER_TYPE:
+        create_resp_partial = functools.partial(
+            create_post_letter_response_from_notification,
+            subject=template_with_content.subject,
+        )
+
+    resp = create_resp_partial(
+        notification=notification,
+        url_root=request.url_root,
+        scheduled_for=scheduled_for,
+        content=template_with_content.content_with_placeholders_filled_in,
+    )
+    return jsonify(resp), 201
 
 
-def process_sms_or_email_notification(
-    *, form, notification_type, api_key, template, template_process_type, service, reply_to_text=None
-):
-    notification_id = uuid.uuid4()
+def process_sms_or_email_notification(*, form, notification_type, api_key, template, service, reply_to_text=None):
+    notification_id = None
     form_send_to = form['email_address'] if notification_type == EMAIL_TYPE else form['phone_number']
 
     send_to = validate_and_format_recipient(send_to=form_send_to,
@@ -184,46 +214,27 @@ def process_sms_or_email_notification(
         service,
         simulated=simulated
     )
-    if document_download_count:
-        # We changed personalisation which means we need to update the content
-        template.values = personalisation
-    api_key_id = api_key.id
-    key_type = api_key.key_type
-    service_in_research_mode = service.research_mode
-    template_version = template._template['version']
-    resp = create_response_for_post_notification(
-        notification_id=notification_id,
-        client_reference=form.get('reference', None),
-        template_id=template.id,
-        template_version=template._template['version'],
-        service_id=service.id,
-        notification_type=notification_type,
-        reply_to=reply_to_text,
-        scheduled_for=form.get("scheduled_for", None),
-        template_with_content=template)
 
-    if str(service.id) in current_app.config.get('HIGH_VOLUME_SERVICE') and key_type == KEY_TYPE_NORMAL \
+    if str(service.id) in current_app.config.get('HIGH_VOLUME_SERVICE') and api_key.key_type == KEY_TYPE_NORMAL \
        and notification_type == EMAIL_TYPE:
         # Put GOV.UK Email notifications onto a queue
         # To take the pressure off the db for API requests put the notification for our high volume service onto a queue
         # the task will then save the notification, then call send_notification_to_queue.
         # We know that this team does not use the GET request, but relies on callbacks to get the status updates.
         try:
-            db.session.commit()
-            save_email_to_queue(
+            notification_id = uuid.uuid4()
+            notification = save_email_to_queue(
                 form=form,
                 notification_id=str(notification_id),
                 notification_type=notification_type,
-                api_key_id=api_key_id,
-                key_type=key_type,
-                template_id=template.id,
-                template_version=template_version,
+                api_key=api_key,
+                template=template,
                 service_id=service.id,
                 personalisation=personalisation,
                 document_download_count=document_download_count,
                 reply_to_text=reply_to_text
             )
-            return resp
+            return notification
         except SQSError:
             # if SQS cannot put the task on the queue, it's probably because the notification body was too long and it
             # went over SQS's 256kb message limit. If so, we
@@ -231,16 +242,16 @@ def process_sms_or_email_notification(
                 f'Notification {notification_id} failed to save to high volume queue. Using normal flow instead'
             )
 
-    persist_notification(
+    notification = persist_notification(
         notification_id=notification_id,
         template_id=template.id,
-        template_version=template_version,
+        template_version=template.version,
         recipient=form_send_to,
         service=service,
         personalisation=personalisation,
         notification_type=notification_type,
-        api_key_id=api_key_id,
-        key_type=key_type,
+        api_key_id=api_key.id,
+        key_type=api_key.key_type,
         client_reference=form.get('reference', None),
         simulated=simulated,
         reply_to_text=reply_to_text,
@@ -249,21 +260,19 @@ def process_sms_or_email_notification(
 
     scheduled_for = form.get("scheduled_for", None)
     if scheduled_for:
-        persist_scheduled_notification(notification_id, form["scheduled_for"])
+        persist_scheduled_notification(notification.id, form["scheduled_for"])
     else:
         if not simulated:
-            queue_name = QueueNames.PRIORITY if template_process_type == PRIORITY else None
-            send_notification_to_queue_detached(
-                key_type=key_type,
-                notification_type=notification_type,
-                notification_id=notification_id,
-                research_mode=service_in_research_mode,  # research_mode is deprecated
+            queue_name = QueueNames.PRIORITY if template.process_type == PRIORITY else None
+            send_notification_to_queue(
+                notification=notification,
+                research_mode=service.research_mode,
                 queue=queue_name
             )
         else:
-            current_app.logger.debug("POST simulated notification for id: {}".format(notification_id))
+            current_app.logger.debug("POST simulated notification for id: {}".format(notification.id))
 
-    return resp
+    return notification
 
 
 def save_email_to_queue(
@@ -271,26 +280,23 @@ def save_email_to_queue(
     notification_id,
     form,
     notification_type,
-    api_key_id,
-    key_type,
-    template_id,
-    template_version,
+    api_key,
+    template,
     service_id,
     personalisation,
     document_download_count,
     reply_to_text=None
 ):
-    db.session.commit()
     data = {
         "id": notification_id,
-        "template_id": str(template_id),
-        "template_version": template_version,
+        "template_id": str(template.id),
+        "template_version": template.version,
         "to": form['email_address'],
         "service_id": str(service_id),
         "personalisation": personalisation,
         "notification_type": notification_type,
-        "api_key_id": str(api_key_id),
-        "key_type": key_type,
+        "api_key_id": str(api_key.id),
+        "key_type": api_key.key_type,
         "client_reference": form.get('reference', None),
         "reply_to_text": reply_to_text,
         "document_download_count": document_download_count,
@@ -335,9 +341,7 @@ def process_document_uploads(personalisation_data, service, simulated=False):
     return personalisation_data, len(file_keys)
 
 
-def process_letter_notification(
-    *, letter_data, api_key, template, template_with_content, reply_to_text, precompiled=False
-):
+def process_letter_notification(*, letter_data, api_key, template, reply_to_text, precompiled=False):
     if api_key.key_type == KEY_TYPE_TEAM:
         raise BadRequestError(message='Cannot send letters with a team api key', status_code=403)
 
@@ -350,68 +354,21 @@ def process_letter_notification(
                                                         template=template,
                                                         reply_to_text=reply_to_text)
 
-    validate_address(api_key, letter_data)
-
-    test_key = api_key.key_type == KEY_TYPE_TEST
-
-    status = NOTIFICATION_CREATED
-    updated_at = None
-    if test_key:
-        # if we don't want to actually send the letter, then start it off in SENDING so we don't pick it up
-        if current_app.config['NOTIFY_ENVIRONMENT'] in ['preview', 'development']:
-            status = NOTIFICATION_SENDING
-        # mark test letter as delivered and do not create a fake response later
-        else:
-            status = NOTIFICATION_DELIVERED
-            updated_at = datetime.utcnow()
-
-    queue = QueueNames.CREATE_LETTERS_PDF if not test_key else QueueNames.RESEARCH_MODE
-
-    notification = create_letter_notification(letter_data=letter_data,
-                                              template=template,
-                                              api_key=api_key,
-                                              status=status,
-                                              reply_to_text=reply_to_text,
-                                              updated_at=updated_at
-                                              )
-
-    get_pdf_for_templated_letter.apply_async(
-        [str(notification.id)],
-        queue=queue
-    )
-
-    if test_key and current_app.config['NOTIFY_ENVIRONMENT'] in ['preview', 'development']:
-        create_fake_letter_response_file.apply_async(
-            (notification.reference,),
-            queue=queue
-        )
-    resp = create_response_for_post_notification(
-        notification_id=notification.id,
-        client_reference=notification.client_reference,
-        template_id=notification.template_id,
-        template_version=notification.template_version,
-        notification_type=notification.notification_type,
-        reply_to=reply_to_text,
-        scheduled_for=letter_data.get('scheduled_for', None),
-        service_id=notification.service_id,
-        template_with_content=template_with_content
-    )
-    return resp
-
-
-def validate_address(api_key, letter_data):
     address = PostalAddress.from_personalisation(
         letter_data['personalisation'],
         allow_international_letters=api_key.service.has_permission(INTERNATIONAL_LETTERS),
     )
+
     if not address.has_enough_lines:
         raise ValidationError(
             message=f'Address must be at least {PostalAddress.MIN_LINES} lines'
         )
+
     if address.has_too_many_lines:
         raise ValidationError(
             message=f'Address must be no more than {PostalAddress.MAX_LINES} lines'
         )
+
     if not address.has_valid_last_line:
         if address.allow_international_letters:
             raise ValidationError(
@@ -421,6 +378,34 @@ def validate_address(api_key, letter_data):
             message='Must be a real UK postcode'
         )
 
+    test_key = api_key.key_type == KEY_TYPE_TEST
+
+    # if we don't want to actually send the letter, then start it off in SENDING so we don't pick it up
+    status = NOTIFICATION_CREATED if not test_key else NOTIFICATION_SENDING
+    queue = QueueNames.CREATE_LETTERS_PDF if not test_key else QueueNames.RESEARCH_MODE
+
+    notification = create_letter_notification(letter_data=letter_data,
+                                              template=template,
+                                              api_key=api_key,
+                                              status=status,
+                                              reply_to_text=reply_to_text)
+
+    get_pdf_for_templated_letter.apply_async(
+        [str(notification.id)],
+        queue=queue
+    )
+
+    if test_key:
+        if current_app.config['NOTIFY_ENVIRONMENT'] in ['preview', 'development']:
+            create_fake_letter_response_file.apply_async(
+                (notification.reference,),
+                queue=queue
+            )
+        else:
+            update_notification_status_by_reference(notification.reference, NOTIFICATION_DELIVERED)
+
+    return notification
+
 
 def process_precompiled_letter_notifications(*, letter_data, api_key, template, reply_to_text):
     try:
@@ -435,12 +420,6 @@ def process_precompiled_letter_notifications(*, letter_data, api_key, template,
                                               status=status,
                                               reply_to_text=reply_to_text)
 
-    resp = {
-        'id': notification.id,
-        'reference': notification.client_reference,
-        'postage': notification.postage
-    }
-
     filename = upload_letter_pdf(notification, letter_content, precompiled=True)
 
     current_app.logger.info('Calling task scan-file for {}'.format(filename))
@@ -459,7 +438,7 @@ def process_precompiled_letter_notifications(*, letter_data, api_key, template,
             queue=QueueNames.LETTERS
         )
 
-    return resp
+    return notification
 
 
 def get_reply_to_text(notification_type, form, template):
@@ -484,31 +463,3 @@ def get_reply_to_text(notification_type, form, template):
         reply_to = template.get_reply_to_text()
 
     return reply_to
-
-
-def create_response_for_post_notification(notification_id, client_reference, template_id, template_version, service_id,
-                                          notification_type, reply_to, scheduled_for,
-                                          template_with_content):
-    if notification_type == SMS_TYPE:
-        create_resp_partial = functools.partial(
-            create_post_sms_response_from_notification,
-            from_number=reply_to,
-        )
-    elif notification_type == EMAIL_TYPE:
-        create_resp_partial = functools.partial(
-            create_post_email_response_from_notification,
-            subject=template_with_content.subject,
-            email_from='{}@{}'.format(authenticated_service.email_from, current_app.config['NOTIFY_EMAIL_DOMAIN']),
-        )
-    elif notification_type == LETTER_TYPE:
-        create_resp_partial = functools.partial(
-            create_post_letter_response_from_notification,
-            subject=template_with_content.subject,
-        )
-    resp = create_resp_partial(
-        notification_id, client_reference, template_id, template_version, service_id,
-        url_root=request.url_root,
-        scheduled_for=scheduled_for,
-        content=template_with_content.content_with_placeholders_filled_in,
-    )
-    return resp

requirements-app.txt

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

requirements.txt

@@ -28,7 +28,7 @@ notifications-python-client==5.5.1
 # PaaS
 awscli-cwlogs>=1.4,<1.5
 
-git+https://github.com/alphagov/notifications-utils.git@39.6.0#egg=notifications-utils==39.6.0
+git+https://github.com/alphagov/notifications-utils.git@39.4.4#egg=notifications-utils==39.4.4
 
 # gds-metrics requires prometheseus 0.2.0, override that requirement as 0.7.1 brings significant performance gains
 prometheus-client==0.7.1
@@ -39,15 +39,14 @@ alembic==1.4.2
 amqp==1.4.9
 anyjson==0.3.3
 attrs==19.3.0
-awscli==1.18.82
+awscli==1.18.75
 bcrypt==3.1.7
 billiard==3.3.0.23
 bleach==3.1.4
 blinker==1.4
 boto==2.49.0
 boto3==1.10.38
-botocore==1.17.5
-cachetools==4.1.0
+botocore==1.16.25
 certifi==2020.4.5.2
 chardet==3.0.4
 click==7.1.2
@@ -79,7 +78,7 @@ python-json-logger==0.1.11
 pytz==2020.1
 PyYAML==5.3.1
 redis==3.5.3
-requests==2.24.0
+requests==2.23.0
 rsa==3.4.2
 s3transfer==0.3.3
 six==1.15.0

tests/app/dao/notification_dao/test_notification_dao.py

@@ -28,7 +28,6 @@ from app.dao.notifications_dao import (
     update_notification_status_by_id,
     update_notification_status_by_reference,
     dao_get_notification_by_reference,
-    dao_get_notifications_by_references,
     dao_get_notification_or_history_by_reference,
     notifications_not_yet_sent,
 )
@@ -1613,7 +1612,7 @@ def test_dao_update_notifications_by_reference_updates_history_when_one_of_two_n
 
 def test_dao_get_notification_by_reference_with_one_match_returns_notification(sample_letter_template, notify_db):
     create_notification(template=sample_letter_template, reference='REF1')
-    notification = dao_get_notification_by_reference('REF1')
+    notification = dao_get_notification_by_reference('REF1', 'letter')
 
     assert notification.reference == 'REF1'
 
@@ -1623,30 +1622,25 @@ def test_dao_get_notification_by_reference_with_multiple_matches_raises_error(sa
     create_notification(template=sample_letter_template, reference='REF1')
 
     with pytest.raises(SQLAlchemyError):
-        dao_get_notification_by_reference('REF1')
+        dao_get_notification_by_reference('REF1', 'letter')
 
 
 def test_dao_get_notification_by_reference_with_no_matches_raises_error(notify_db):
     with pytest.raises(SQLAlchemyError):
-        dao_get_notification_by_reference('REF1')
+        dao_get_notification_by_reference('REF1', 'email')
 
 
-def test_dao_get_notifications_by_references(sample_template):
-    create_notification(template=sample_template, reference='noref')
-    notification_1 = create_notification(template=sample_template, reference='ref')
-    notification_2 = create_notification(template=sample_template, reference='ref')
-
-    notifications = dao_get_notifications_by_references(['ref'])
-    assert len(notifications) == 2
-    assert notifications[0].id in [notification_1.id, notification_2.id]
-    assert notifications[1].id in [notification_1.id, notification_2.id]
+def test_dao_get_notification_by_reference_with_no_matches_for_type_raises_error(sample_email_template):
+    create_notification(template=sample_email_template, reference='REF1')
+    with pytest.raises(SQLAlchemyError):
+        dao_get_notification_by_reference('REF1', 'letter')
 
 
 def test_dao_get_notification_or_history_by_reference_with_one_match_returns_notification(
         sample_letter_template
 ):
     create_notification(template=sample_letter_template, reference='REF1')
-    notification = dao_get_notification_or_history_by_reference('REF1')
+    notification = dao_get_notification_or_history_by_reference('REF1', 'letter')
 
     assert notification.reference == 'REF1'
 
@@ -1658,12 +1652,18 @@ def test_dao_get_notification_or_history_by_reference_with_multiple_matches_rais
     create_notification(template=sample_letter_template, reference='REF1')
 
     with pytest.raises(SQLAlchemyError):
-        dao_get_notification_or_history_by_reference('REF1')
+        dao_get_notification_or_history_by_reference('REF1', 'letter')
 
 
-def test_dao_get_notification_or_history_by_reference_with_no_matches_raises_error(notify_db):
+def test_dao_get_notification_or_history_by_reference_with_no_matches_raises_error(sample_letter_template):
+    create_notification(template=sample_letter_template, reference='REF1')
     with pytest.raises(SQLAlchemyError):
-        dao_get_notification_or_history_by_reference('REF1')
+        dao_get_notification_or_history_by_reference('REF1', 'email')
+
+
+def test_dao_get_notification_or_history_by_reference_with_no_matches_for_type_raises_error(notify_db):
+    with pytest.raises(SQLAlchemyError):
+        dao_get_notification_or_history_by_reference('REF1', 'email')
 
 
 @pytest.mark.parametrize("notification_type",

tests/app/notifications/test_process_notification.py

@@ -23,7 +23,7 @@ from app.notifications.process_notifications import (
 )
 from notifications_utils.recipients import validate_and_format_phone_number, validate_and_format_email_address
 from app.v2.errors import BadRequestError
-from tests.app.db import create_service, create_template, create_api_key
+from tests.app.db import create_service, create_template
 
 
 def test_create_content_for_notification_passes(sample_email_template):
@@ -51,7 +51,8 @@ def test_create_content_for_notification_allows_additional_personalisation(sampl
 
 
 @freeze_time("2016-01-01 11:09:00.061258")
-def test_persist_notification_creates_and_save_to_db(sample_template, sample_api_key, sample_job):
+def test_persist_notification_creates_and_save_to_db(sample_template, sample_api_key, sample_job, mocker):
+    mocked_redis = mocker.patch('app.notifications.process_notifications.redis_store.get')
 
     assert Notification.query.count() == 0
     assert NotificationHistory.query.count() == 0
@@ -90,6 +91,8 @@ def test_persist_notification_creates_and_save_to_db(sample_template, sample_api
     assert notification_from_db.created_by_id == notification.created_by_id
     assert notification_from_db.reply_to_text == sample_template.service.get_default_sms_sender()
 
+    mocked_redis.assert_called_once_with(str(sample_template.service_id) + "-2016-01-01-count")
+
 
 def test_persist_notification_throws_exception_when_missing_template(sample_api_key):
     assert Notification.query.count() == 0
@@ -154,9 +157,10 @@ def test_persist_notification_does_not_increment_cache_if_test_key(
 
 
 @freeze_time("2016-01-01 11:09:00.061258")
-def test_persist_notification_with_optionals(sample_job, sample_api_key):
+def test_persist_notification_with_optionals(sample_job, sample_api_key, mocker):
     assert Notification.query.count() == 0
     assert NotificationHistory.query.count() == 0
+    mocked_redis = mocker.patch('app.notifications.process_notifications.redis_store.get')
     n_id = uuid.uuid4()
     created_at = datetime.datetime(2016, 11, 11, 16, 8, 18)
     persist_notification(
@@ -182,7 +186,7 @@ def test_persist_notification_with_optionals(sample_job, sample_api_key):
     persisted_notification.job_id == sample_job.id
     assert persisted_notification.job_row_number == 10
     assert persisted_notification.created_at == created_at
-
+    mocked_redis.assert_called_once_with(str(sample_job.service_id) + "-2016-01-01-count")
     assert persisted_notification.client_reference == "ref from client"
     assert persisted_notification.reference is None
     assert persisted_notification.international is False
@@ -193,77 +197,44 @@ def test_persist_notification_with_optionals(sample_job, sample_api_key):
 
 
 @freeze_time("2016-01-01 11:09:00.061258")
-def test_persist_notification_doesnt_touch_cache_for_old_keys_that_dont_exist(notify_db_session, mocker):
-    service = create_service(restricted=True)
-    template = create_template(service=service)
-    api_key = create_api_key(service=service)
+def test_persist_notification_doesnt_touch_cache_for_old_keys_that_dont_exist(sample_template, sample_api_key, mocker):
     mock_incr = mocker.patch('app.notifications.process_notifications.redis_store.incr')
     mocker.patch('app.notifications.process_notifications.redis_store.get', return_value=None)
     mocker.patch('app.notifications.process_notifications.redis_store.get_all_from_hash', return_value=None)
 
     persist_notification(
-        template_id=template.id,
-        template_version=template.version,
+        template_id=sample_template.id,
+        template_version=sample_template.version,
         recipient='+447111111111',
-        service=template.service,
+        service=sample_template.service,
         personalisation={},
         notification_type='sms',
-        api_key_id=api_key.id,
-        key_type=api_key.key_type,
+        api_key_id=sample_api_key.id,
+        key_type=sample_api_key.key_type,
         reference="ref"
     )
     mock_incr.assert_not_called()
 
 
 @freeze_time("2016-01-01 11:09:00.061258")
-def test_persist_notification_increments_cache_if_key_exists_and_for_trial_service(
-    notify_db_session, mocker
-):
-    service = create_service(restricted=True)
-    template = create_template(service=service)
-    api_key = create_api_key(service=service)
+def test_persist_notification_increments_cache_if_key_exists(sample_template, sample_api_key, mocker):
     mock_incr = mocker.patch('app.notifications.process_notifications.redis_store.incr')
     mocker.patch('app.notifications.process_notifications.redis_store.get', return_value=1)
     mocker.patch('app.notifications.process_notifications.redis_store.get_all_from_hash',
-                 return_value={template.id, 1})
+                 return_value={sample_template.id, 1})
 
     persist_notification(
-        template_id=template.id,
-        template_version=template.version,
+        template_id=sample_template.id,
+        template_version=sample_template.version,
         recipient='+447111111122',
-        service=template.service,
+        service=sample_template.service,
         personalisation={},
         notification_type='sms',
-        api_key_id=api_key.id,
-        key_type=api_key.key_type,
+        api_key_id=sample_api_key.id,
+        key_type=sample_api_key.key_type,
         reference="ref2")
 
-    mock_incr.assert_called_once_with(str(service.id) + "-2016-01-01-count", )
-
-
-def test_persist_notification_does_not_increments_cache_live_service(
-    notify_db_session, mocker
-):
-    service = create_service(restricted=False)
-    template = create_template(service=service)
-    api_key = create_api_key(service=service)
-    mock_incr = mocker.patch('app.notifications.process_notifications.redis_store.incr')
-    mocker.patch('app.notifications.process_notifications.redis_store.get', return_value=1)
-    mocker.patch('app.notifications.process_notifications.redis_store.get_all_from_hash',
-                 return_value={template.id, 1})
-
-    persist_notification(
-        template_id=template.id,
-        template_version=template.version,
-        recipient='+447111111122',
-        service=template.service,
-        personalisation={},
-        notification_type='sms',
-        api_key_id=api_key.id,
-        key_type=api_key.key_type,
-        reference="ref2")
-
-    assert not mock_incr.called
+    mock_incr.assert_called_once_with(str(sample_template.service_id) + "-2016-01-01-count", )
 
 
 @pytest.mark.parametrize((

tests/app/service/test_rest.py

@@ -235,6 +235,7 @@ def test_get_service_by_id(admin_request, sample_service):
     assert json_resp['data']['email_branding'] is None
     assert 'branding' not in json_resp['data']
     assert json_resp['data']['prefix_sms'] is True
+    assert json_resp['data']['letter_logo_filename'] is None
 
 
 @pytest.mark.parametrize('detailed', [True, False])
@@ -346,6 +347,7 @@ def test_create_service(
     assert json_resp['data']['name'] == 'created service'
     assert json_resp['data']['email_from'] == 'created.service'
     assert not json_resp['data']['research_mode']
+    assert json_resp['data']['rate_limit'] == 3000
     assert json_resp['data']['letter_branding'] is None
     assert json_resp['data']['count_as_live'] is expected_count_as_live
 
@@ -1223,6 +1225,7 @@ def test_add_existing_user_to_another_service_with_all_permissions(
             )
             assert resp.status_code == 200
             json_resp = resp.json
+            assert str(user_to_add.id) in json_resp['data']['users']
 
             # check user has all permissions
             auth_header = create_authorization_header()

tests/app/v2/notifications/test_post_letter_notifications.py

@@ -245,7 +245,6 @@ def test_post_letter_notification_with_test_key_creates_pdf_and_sets_status_to_d
     fake_create_letter_task.assert_called_once_with([str(notification.id)], queue='research-mode-tasks')
     assert not fake_create_dvla_response_task.called
     assert notification.status == NOTIFICATION_DELIVERED
-    assert notification.updated_at is not None
 
 
 @pytest.mark.parametrize('env', [

tests/app/v2/notifications/test_post_notifications.py

@@ -49,7 +49,6 @@ def test_post_sms_notification_returns_201(client, sample_template_with_placehol
         path='/v2/notifications/sms',
         data=json.dumps(data),
         headers=[('Content-Type', 'application/json'), auth_header])
-
     assert response.status_code == 201
     resp_json = json.loads(response.get_data(as_text=True))
     assert validate(resp_json, post_sms_response) == resp_json
@@ -425,7 +424,7 @@ def test_returns_a_429_limit_exceeded_if_rate_limit_exceeded(
 ):
     sample = create_template(service=sample_service, template_type=notification_type)
     persist_mock = mocker.patch('app.v2.notifications.post_notifications.persist_notification')
-    deliver_mock = mocker.patch('app.v2.notifications.post_notifications.send_notification_to_queue_detached')
+    deliver_mock = mocker.patch('app.v2.notifications.post_notifications.send_notification_to_queue')
     mocker.patch(
         'app.v2.notifications.post_notifications.check_rate_limiting',
         side_effect=RateLimitError("LIMIT", "INTERVAL", "TYPE"))
@@ -1006,14 +1005,11 @@ def test_post_notifications_saves_email_normally_if_save_email_to_queue_fails(cl
         "template_id": template.id,
         "personalisation": {"message": "Dear citizen, have a nice day"}
     }
-
-    print("******** Start")
     response = client.post(
         path='/v2/notifications/email',
         data=json.dumps(data),
         headers=[('Content-Type', 'application/json'), create_authorization_header(service_id=service.id)]
     )
-    print("********** End")
 
     json_resp = response.get_json()