Remove the call to the db after the notification has committed.

After the commit we issue two calls to the db to get service and get notification. This is because after the commit the ORM wants to ensure that the data model objects are the latest.

So far this is just a proof of concept, but the letter flow needs to be updated and we should be able to get rid of research mode. And it needs some tidy up.

app/notifications/process_notifications.py

@@ -120,13 +120,18 @@ 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)
-        if key_type != KEY_TYPE_TEST:
+        print("****** COMMIT *******")
-            with REDIS_GET_AND_INCR_DAILY_LIMIT_DURATION_SECONDS.time():
+        # Only keep track of the daily limit for trial mode services.
-                if redis_store.get(redis.daily_limit_cache_key(service.id)):
+        if service_in_trial_mode and key_type != KEY_TYPE_TEST:
-                    redis_store.incr(redis.daily_limit_cache_key(service.id))
+            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)
@@ -134,33 +139,41 @@ def persist_notification(
     return notification
 
 
-def send_notification_to_queue(notification, research_mode, queue=None):
+def send_notification_to_queue_detached(
-    if research_mode or notification.key_type == KEY_TYPE_TEST:
+    key_type, notification_type, notification_id, research_mode, queue=None
+):
+    if research_mode or key_type == KEY_TYPE_TEST:
         queue = QueueNames.RESEARCH_MODE
 
-    if notification.notification_type == SMS_TYPE:
+    if notification_type == SMS_TYPE:
         if not queue:
             queue = QueueNames.SEND_SMS
         deliver_task = provider_tasks.deliver_sms
-    if notification.notification_type == EMAIL_TYPE:
+    if notification_type == EMAIL_TYPE:
         if not queue:
             queue = QueueNames.SEND_EMAIL
         deliver_task = provider_tasks.deliver_email
-    if notification.notification_type == LETTER_TYPE:
+    if 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.notification_type,
+        "{} {} sent to the {} queue for delivery".format(notification_type,
-                                                         notification.id,
+                                                         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):

app/v2/notifications/create_response.py

@@ -44,3 +44,66 @@ def __create_notification_response(notification, url_root, scheduled_for):
         },
         "scheduled_for": scheduled_for if scheduled_for else None
     }
+
+
+# test detaching notification
+def create_post_sms_response_from_notification_detached(
+    notification_id, client_reference, template_id, template_version, service_id, 
+    content, from_number, url_root, scheduled_for
+):
+    resp = __create_notification_response_detached(
+        notification_id, client_reference, template_id, template_version, service_id, url_root, scheduled_for
+    )
+    resp['content'] = {
+        'from_number': from_number,
+        'body': content
+    }
+    return resp
+
+
+def create_post_email_response_from_notification_detached(
+    notification_id, client_reference, template_id, template_version, service_id, content, subject, email_from, 
+    url_root, scheduled_for):
+    resp = __create_notification_response_detached(
+        notification_id, client_reference, template_id, template_version, service_id, url_root, scheduled_for
+    )
+    resp['content'] = {
+        "from_email": email_from,
+        "body": content,
+        "subject": subject
+    }
+    return resp
+
+
+def create_post_letter_response_from_notification_detached(
+    notification_id, client_reference, template_id, template_version, service_id,
+    content, subject, url_root, scheduled_for
+):
+    resp = __create_notification_response_detached(
+        notification_id, client_reference, template_id, template_version, service_id, url_root, scheduled_for
+    )
+    resp['content'] = {
+        "body": content,
+        "subject": subject
+    }
+    return resp
+
+
+def __create_notification_response_detached(
+    notification_id, client_reference, template_id, template_version, service_id, url_root, scheduled_for
+):
+    return {
+        "id": notification_id,
+        "reference": client_reference,
+        "uri": "{}v2/notifications/{}".format(url_root, str(notification_id)),
+        'template': {
+            "id": template_id,
+            "version": template_version,
+            "uri": "{}services/{}/templates/{}".format(
+                url_root,
+                str(service_id),
+                str(template_id)
+            )
+        },
+        "scheduled_for": scheduled_for if scheduled_for else None
+    }

app/v2/notifications/post_notifications.py

@@ -46,8 +46,8 @@ from app.notifications.process_notifications import (
     persist_notification,
     persist_scheduled_notification,
     send_notification_to_queue,
-    simulated_recipient
+    simulated_recipient,
-)
+    send_notification_to_queue_detached)
 from app.notifications.validators import (
     check_if_service_can_send_files_by_email,
     check_rate_limiting,
@@ -64,8 +64,9 @@ 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_letter_response_from_notification,
-)
+    create_post_sms_response_from_notification_detached, create_post_email_response_from_notification_detached,
+    create_post_letter_response_from_notification_detached)
 from app.v2.notifications.notification_schemas import (
     post_sms_request,
     post_email_request,
@@ -151,7 +152,7 @@ def post_notification(notification_type):
     )
 
     reply_to = get_reply_to_text(notification_type, form, template)
-
+    
     if notification_type == LETTER_TYPE:
         notification = process_letter_notification(
             letter_data=form,
@@ -164,41 +165,47 @@ def post_notification(notification_type):
             form=form,
             notification_type=notification_type,
             api_key=api_user,
-            template=template,
+            template=template_with_content,
+            template_process_type=template.process_type,
             service=authenticated_service,
             reply_to_text=reply_to
         )
 
-        template_with_content.values = notification.personalisation
+    return jsonify(notification), 201
 
+
+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,
+            create_post_sms_response_from_notification_detached,
             from_number=reply_to,
         )
     elif notification_type == EMAIL_TYPE:
         create_resp_partial = functools.partial(
-            create_post_email_response_from_notification,
+            create_post_email_response_from_notification_detached,
             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,
+            create_post_letter_response_from_notification_detached,
             subject=template_with_content.subject,
         )
-
     resp = create_resp_partial(
-        notification=notification,
+        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 jsonify(resp), 201
+    return resp
 
 
-def process_sms_or_email_notification(*, form, notification_type, api_key, template, service, reply_to_text=None):
+def process_sms_or_email_notification(
-    notification_id = None
+    *, form, notification_type, api_key, template, template_process_type, service, reply_to_text=None
+):
+    notification_id = uuid.uuid4()
     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,
@@ -215,6 +222,19 @@ def process_sms_or_email_notification(*, form, notification_type, api_key, templ
         simulated=simulated
     )
 
+    key_type = api_key.key_type
+    service_in_research_mode = service.resear
+    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=None,
+        template_with_content=template)
+
     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
@@ -222,7 +242,6 @@ def process_sms_or_email_notification(*, form, notification_type, api_key, templ
         # 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:
-            notification_id = uuid.uuid4()
             notification = save_email_to_queue(
                 form=form,
                 notification_id=str(notification_id),
@@ -242,16 +261,16 @@ def process_sms_or_email_notification(*, form, notification_type, api_key, templ
                 f'Notification {notification_id} failed to save to high volume queue. Using normal flow instead'
             )
 
-    notification = persist_notification(
+    persist_notification(
         notification_id=notification_id,
         template_id=template.id,
-        template_version=template.version,
+        template_version=template._template['version'],
         recipient=form_send_to,
         service=service,
         personalisation=personalisation,
         notification_type=notification_type,
         api_key_id=api_key.id,
-        key_type=api_key.key_type,
+        key_type=key_type,
         client_reference=form.get('reference', None),
         simulated=simulated,
         reply_to_text=reply_to_text,
@@ -260,19 +279,21 @@ def process_sms_or_email_notification(*, form, notification_type, api_key, templ
 
     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
+            queue_name = QueueNames.PRIORITY if template_process_type == PRIORITY else None
-            send_notification_to_queue(
+            send_notification_to_queue_detached(
-                notification=notification,
+                key_type=key_type,
-                research_mode=service.research_mode,
+                notification_type=notification_type,
+                notification_id=notification_id,
+                research_mode=False,  # research_mode is a deprecated 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 notification
+    return resp
 
 
 def save_email_to_queue(

tests/app/v2/notifications/test_post_notifications.py

@@ -33,7 +33,7 @@ from tests.app.db import (
 )
 
 
-@pytest.mark.parametrize("reference", [None, "reference_from_client"])
+@pytest.mark.parametrize("reference", ["reference_from_client"])
 def test_post_sms_notification_returns_201(client, sample_template_with_placeholders, mocker, reference):
     mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
     data = {
@@ -44,13 +44,15 @@ def test_post_sms_notification_returns_201(client, sample_template_with_placehol
     if reference:
         data.update({"reference": reference})
     auth_header = create_authorization_header(service_id=sample_template_with_placeholders.service_id)
-
+    print("********* Start")
     response = client.post(
         path='/v2/notifications/sms',
         data=json.dumps(data),
         headers=[('Content-Type', 'application/json'), auth_header])
+    print("********* End")
     assert response.status_code == 201
     resp_json = json.loads(response.get_data(as_text=True))
+    print(resp_json)
     assert validate(resp_json, post_sms_response) == resp_json
     notifications = Notification.query.all()
     assert len(notifications) == 1