Revert "Summoning heavenly logging to smite mismatches on priority assignment"

app/notifications/process_notifications.py

@@ -12,7 +12,7 @@
     validate_and_format_phone_number,
 )
 
-from app import models, redis_store
+from app import redis_store
 from app.celery import provider_tasks
 from app.celery.letters_pdf_tasks import create_letters_pdf
 from app.config import QueueNames
@@ -112,19 +112,6 @@ def persist_notification(
         billable_units=billable_units,
     )
     template = dao_get_template_by_id(template_id, template_version, use_cache=True)
-    # TODO: Remove this logging statement once debugging is complete. It is useful for understanding how templates are routed to queues.
-    current_app.logger.info(
-        "persist_notification: Routing notification %s template %s version %s: process_type_column=%r, "
-        "effective_process_type=%r, category_id=%s, category_email_process_type=%r, category_sms_process_type=%r",
-        notification.id,
-        template.id,
-        template.version,
-        template.process_type_column,
-        template.process_type,
-        template.template_category_id,
-        template.template_category.email_process_type if template.template_category else None,
-        template.template_category.sms_process_type if template.template_category else None,
-    )
     notification.queue_name = choose_queue(
         notification=notification, research_mode=service.research_mode, priority_queue=get_delivery_queue_for_template(template)
     )
@@ -280,25 +267,20 @@ def choose_queue(notification: Notification, research_mode: bool, priority_queue
     if notification.notification_type == SMS_TYPE and notification.sends_with_custom_number():
         return QueueNames.SEND_THROTTLED_SMS
 
-    if priority_queue:
-        return priority_queue
-    else:
-        override_queue: Optional[str] = None
-        match notification.notification_type:
-            case models.SMS_TYPE:
-                override_queue = QueueNames.SEND_SMS_MEDIUM
-            case models.EMAIL_TYPE:
-                override_queue = QueueNames.SEND_EMAIL_MEDIUM
-            case models.LETTER_TYPE:
-                override_queue = QueueNames.CREATE_LETTERS_PDF
-            case _:
-                raise ValueError(f"Could not determine queue for notification type {notification.notification_type!r}")
-
-        current_app.logger.info(
-            f"Notification {notification.id} had no priority queue; determined queue based on notification type and attributes to {override_queue}."
-        )
+    override_queue: Optional[str] = priority_queue
+    if notification.notification_type == SMS_TYPE:
+        if not priority_queue:
+            override_queue = QueueNames.SEND_SMS_MEDIUM
+    elif notification.notification_type == EMAIL_TYPE:
+        if not priority_queue:
+            override_queue = QueueNames.SEND_EMAIL_MEDIUM
+    elif notification.notification_type == LETTER_TYPE:
+        if not priority_queue:
+            override_queue = QueueNames.CREATE_LETTERS_PDF
 
-        return override_queue
+    if override_queue is None:
+        raise ValueError(f"Could not determine queue for notification type {notification.notification_type!r}")
+    return override_queue
 
 
 def choose_deliver_task(notification):
@@ -416,19 +398,6 @@ def persist_notifications(notifications: List[VerifiedNotification]) -> List[Not
         ):
             notification_obj.billable_units = number_of_sms_fragments(template, notification.get("personalisation"))
         service = dao_fetch_service_by_id(service_id, use_cache=True)
-        # TODO: Remove this logging statement once debugging is complete. It is useful for understanding how templates are routed to queues.
-        current_app.logger.info(
-            "persist_notifications: Routing notification %s template %s version %s: process_type_column=%r, "
-            "effective_process_type=%r, category_id=%s, category_email_process_type=%r, category_sms_process_type=%r",
-            notification_obj.id,
-            template.id,
-            template.version,
-            template.process_type_column,
-            template.process_type,
-            template.template_category_id,
-            template.template_category.email_process_type if template.template_category else None,
-            template.template_category.sms_process_type if template.template_category else None,
-        )
         notification_obj.queue_name = choose_queue(
             notification=notification_obj,
             research_mode=service.research_mode,

app/notifications/rest.py

@@ -145,7 +145,7 @@ def send_notification(notification_type: NotificationType):
                 check_sms_daily_limit(authenticated_service, 1)
     # Do not persist or send notification to the queue if it is a simulated recipient
 
-    notification = persist_notification(
+    notification_model = persist_notification(
         template_id=template.id,
         template_version=template.version,
         template_postage=template.postage,
@@ -159,30 +159,17 @@ def send_notification(notification_type: NotificationType):
         reply_to_text=template.get_reply_to_text(),
     )
     if not simulated:
-        # TODO: Remove this logging statement once debugging is complete. It is useful for understanding how templates are routed to queues.
-        current_app.logger.info(
-            "send_notification: Routing notification %s template %s version %s: process_type_column=%r, "
-            "effective_process_type=%r, category_id=%s, category_email_process_type=%r, category_sms_process_type=%r",
-            notification.id,
-            template.id,
-            template.version,
-            template.process_type_column,
-            template.process_type,
-            template.template_category_id,
-            template.template_category.email_process_type if template.template_category else None,
-            template.template_category.sms_process_type if template.template_category else None,
-        )
         send_notification_to_queue(
-            notification=notification,
+            notification=notification_model,
             research_mode=authenticated_service.research_mode,
             queue=get_delivery_queue_for_template(template),
         )
     else:
-        current_app.logger.debug("POST simulated notification for id: {}".format(notification.id))
+        current_app.logger.debug("POST simulated notification for id: {}".format(notification_model.id))
     notification_form.update({"template_version": template.version})
 
     return (
-        jsonify(data=get_notification_return_data(notification.id, notification_form, template_object)),
+        jsonify(data=get_notification_return_data(notification_model.id, notification_form, template_object)),
         201,
     )
 

app/service/send_notification.py

@@ -132,19 +132,6 @@ def send_one_off_notification(service_id, post_data):
         )
     else:
         # allow one-off sends from admin to go quicker by using normal queue instead of bulk queue
-        # TODO: Remove this logging statement once debugging is complete. It is useful for understanding how templates are routed to queues.
-        current_app.logger.info(
-            "send_one_off_notification: Routing notification %s template %s version %s: process_type_column=%r, "
-            "effective_process_type=%r, category_id=%s, category_email_process_type=%r, category_sms_process_type=%r",
-            notification.id,
-            template.id,
-            template.version,
-            template.process_type_column,
-            template.process_type,
-            template.template_category_id,
-            template.template_category.email_process_type if template.template_category else None,
-            template.template_category.sms_process_type if template.template_category else None,
-        )
         queue = get_delivery_queue_for_template(template)
         if queue == QueueNames.DELIVERY_QUEUES[template.template_type][Priorities.LOW]:
             queue = QueueNames.DELIVERY_QUEUES[template.template_type][Priorities.MEDIUM]

app/v2/notifications/post_notifications.py

@@ -62,11 +62,14 @@
 )
 from app.notifications.process_letter_notifications import create_letter_notification
 from app.notifications.process_notifications import (
+    choose_queue,
     csv_has_simulated_and_non_simulated_recipients,
+    db_save_and_send_notification,
     number_of_sms_fragments,
     persist_notification,
     persist_scheduled_notification,
     simulated_recipient,
+    transform_notification,
 )
 from app.notifications.validators import (
     check_email_annual_limit,
@@ -87,6 +90,7 @@
 from app.schemas import job_schema
 from app.service.utils import safelisted_members
 from app.sms_fragment_utils import fetch_todays_requested_sms_count
+from app.utils import get_delivery_queue_for_template
 from app.v2.errors import BadRequestError
 from app.v2.notifications import v2_notification_blueprint
 from app.v2.notifications.create_response import (
@@ -474,11 +478,33 @@
         persist_scheduled_notification(notification.id, form["scheduled_for"])
     elif not simulated:
         triage_notification_to_queues(notification_type, signed_notification_data, template)
+
         current_app.logger.info(
             f"Batch saving: {notification_type}/{template.process_type} {notification['id']} sent to buffer queue."
         )
-    else:  # Simulated and not scheduled
-        current_app.logger.debug("POST simulated notification for id: {}".format(notification["id"]))
+    else:
+        notification = transform_notification(
+            template_id=template.id,
+            template_version=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,
+            client_reference=form.get("reference", None),
+            reply_to_text=reply_to_text,
+        )
+        if not simulated:
+            notification.queue_name = choose_queue(
+                notification=notification,
+                research_mode=service.research_mode,
+                priority_queue=get_delivery_queue_for_template(template),
+            )
+            db_save_and_send_notification(notification)
+
+        else:
+            current_app.logger.debug("POST simulated notification for id: {}".format(notification.id))
 
     if not isinstance(notification, Notification):
         notification["template_id"] = notification["template"]

tests/app/v2/notifications/test_post_notifications.py

@@ -509,6 +509,7 @@ def test_returns_a_429_limit_exceeded_if_rate_limit_exceeded(
         self, notify_api, client, sample_service, mocker, notification_type, key_send_to, send_to
     ):
         sample = create_template(service=sample_service, template_type=notification_type)
+        save_mock = mocker.patch("app.v2.notifications.post_notifications.db_save_and_send_notification")
         mocker.patch(
             "app.v2.notifications.post_notifications.check_rate_limiting",
             side_effect=RateLimitError("LIMIT", "INTERVAL", "TYPE"),
@@ -532,6 +533,8 @@ def test_returns_a_429_limit_exceeded_if_rate_limit_exceeded(
         assert message == "Exceeded rate limit for key type TYPE of LIMIT requests per INTERVAL seconds"
         assert status_code == 429
 
+        assert not save_mock.called
+
     @pytest.mark.parametrize(
         "notification_type, key_send_to, send_to, expected_error_message",
         [
@@ -555,6 +558,7 @@ def test_post_notification_returns_429_when_annual_limit_exceeded(
         sample_service.sms_annual_limit = 1
         sample_service.email_annual_limit = 1
         template = create_template(service=sample_service, template_type=notification_type)
+        save_mock = mocker.patch("app.v2.notifications.post_notifications.db_save_and_send_notification")
         mocker.patch("app.v2.notifications.post_notifications.check_rate_limiting")
 
         data = {key_send_to: send_to, "template_id": str(template.id)}
@@ -591,6 +595,8 @@ def test_post_notification_returns_429_when_annual_limit_exceeded(
             assert status_code == 429
             assert message == expected_error_message
 
+            assert not save_mock.called
+
     def test_post_sms_notification_returns_400_if_not_allowed_to_send_int_sms(
         self,
         client,