rework how we display error message for disallowed phone numbers

app/main/views/send.py

@@ -4,16 +4,7 @@ import uuid
 from string import ascii_uppercase
 from zipfile import BadZipFile
 
-from flask import (
+from flask import abort, flash, redirect, render_template, request, session, url_for
-    abort,
-    current_app,
-    flash,
-    redirect,
-    render_template,
-    request,
-    session,
-    url_for,
-)
 from flask_login import current_user
 from notifications_python_client.errors import HTTPError
 from notifications_utils import SMS_CHAR_COUNT_LIMIT
@@ -497,7 +488,6 @@ def _check_messages(service_id, template_id, upload_id, preview_row):
     remaining_messages = current_service.message_limit - notification_count
 
     contents = s3download(service_id, upload_id)
-
     db_template = current_service.get_template_with_user_permission_or_403(
         template_id, current_user
     )
@@ -857,25 +847,25 @@ def send_notification(service_id, template_id):
     vals = ",".join(values)
     data = f"{data}\r\n{vals}"
 
-    filename = f"{uuid.uuid4()}.csv"
+    filename = f"one-off:{current_user.name}:{uuid.uuid4()}.csv"
-    my_data = {"file_name": filename, "template_id": template_id, "data": data}
+    my_data = {"filename": filename, "template_id": template_id, "data": data}
     upload_id = s3upload(service_id, my_data)
     form = CsvUploadForm()
     form.file.data = my_data
     form.file.name = filename
-    job = None
+
-    try:
+    check_message_output = check_messages(service_id, template_id, upload_id, 2)
-        job = job_api_client.create_job(
+    if "You cannot send to" in check_message_output:
-            upload_id,
+        return check_messages(service_id, template_id, upload_id, 2)
-            service_id,
+    job_api_client.create_job(
-            scheduled_for="",
+        upload_id,
-            template_id=template_id,
+        service_id,
-            original_file_name=filename,
+        scheduled_for="",
-            notification_count=1,
+        template_id=template_id,
-            valid="True",
+        original_file_name=filename,
-        )
+        notification_count=1,
-    except Exception as e:
+        valid="True",
-        current_app.logger.error(e)
+    )
 
     session.pop("recipient")
     session.pop("placeholders")
@@ -892,42 +882,31 @@ def send_notification(service_id, template_id):
         )
         time.sleep(0.1)
         attempts = attempts + 1
-
+    # TODO we are replacing the original 'one-off send' functionality with a job that
-    # TODO need some UI magic so the error message is displayed properly
+    # we create on the fly.  The purpose for this is to ultimately remove the phone numbers
-    # and we don't just barf an exception
+    # from the db.  However, by running a job we no longer get error messages we used to get.
+    # If the user is in trial mode and trying to send to a phone number they are not allowed
+    # to send to, right now they will see that their job started and the only way they will
+    # know something went wrong, is to sit and watch the status sit as pending for 3 hours
+    # and ultimately switch to failed with no reason why.
+    #
+    # In future, we should block the user from sending to phone numbers they aren't allowed
+    # to send to.
+    # the way to do that would be to make this available to the front end:
+    #
+    # <api>/service/utils/service_allowed_to_send_to
+    #
+    # After that the UI should be making this call as part of the phone number validation
+    # A user in trial mode should not be able to 'send message' to a phone number they are not
+    # allowed to send to
     if notis["total"] == 0 and attempts == 5:
-        # raise Exception(
+        # This shows the job we auto-generated for the user
-        #    "Could not send notification. Please check that you can send to that phone number"
+        return redirect(
-        # )
+            url_for(
-
+                "main.view_job",
-        db_template = current_service.get_template_with_user_permission_or_403(
+                service_id=service_id,
-            template_id, current_user
+                job_id=upload_id,
-        )
+            )
-
-        return render_template(
-            "views/notifications/notification.html",
-            finished=True,
-            notification_status="failed",
-            error_message="This is bogus",
-            uploaded_file_name="Report",
-            template=db_template,
-            job=job,
-            # updates_url=url_for(
-            #    ".view_notification_updates",
-            #    service_id=service_id,
-            #    notification_id=notification["id"],
-            #    status=request.args.get("status"),
-            #    help=get_help_argument(),
-            # ),
-            # partials=get_single_notification_partials(notification),
-            # created_by=notification.get("created_by"),
-            created_at="2023-12-15 00:00:00",
-            # updated_at=notification["updated_at"],
-            # help=get_help_argument(),
-            # notification_id=notification["id"],
-            # can_receive_inbound=(current_service.has_permission("inbound_sms")),
-            # sent_with_test_key=(notification.get("key_type") == KEY_TYPE_TEST),
-            # back_link=back_link,
         )
 
     return redirect(

tests/app/main/views/test_send.py

@@ -2117,11 +2117,13 @@ def test_route_permissions_send_check_notifications(
     route,
     response_code,
     method,
+    mock_create_job,
 ):
     with client_request.session_transaction() as session:
         session["recipient"] = "2028675301"
         session["placeholders"] = {"name": "a"}
 
+    mocker.patch("app.main.views.send.check_messages")
     mocker.patch(
         "app.notification_api_client.get_notifications_for_service",
         return_value=FAKE_ONE_OFF_NOTIFICATION,
@@ -2657,6 +2659,8 @@ def test_send_notification_submits_data(
         return_value=FAKE_ONE_OFF_NOTIFICATION,
     )
 
+    mocker.patch("app.main.views.send.check_messages", return_value="")
+
     client_request.post(
         "main.send_notification", service_id=SERVICE_ONE_ID, template_id=fake_uuid
     )
@@ -2671,11 +2675,13 @@ def test_send_notification_clears_session(
     mock_send_notification,
     mock_get_service_template,
     mocker,
+    mock_create_job,
 ):
     with client_request.session_transaction() as session:
         session["recipient"] = "2028675301"
         session["placeholders"] = {"a": "b"}
 
+    mocker.patch("app.main.views.send.check_messages")
     mocker.patch(
         "app.notification_api_client.get_notifications_for_service",
         return_value=FAKE_ONE_OFF_NOTIFICATION,
@@ -2730,11 +2736,14 @@ def test_send_notification_redirects_to_view_page(
     extra_args,
     extra_redirect_args,
     mocker,
+    mock_create_job,
 ):
     with client_request.session_transaction() as session:
         session["recipient"] = "2028675301"
         session["placeholders"] = {"a": "b"}
 
+    mocker.patch("app.main.views.send.check_messages")
+
     mocker.patch(
         "app.notification_api_client.get_notifications_for_service",
         return_value=FAKE_ONE_OFF_NOTIFICATION,
@@ -2783,6 +2792,7 @@ def test_send_notification_shows_error_if_400(
     fake_uuid,
     mocker,
     mock_get_service_template_with_placeholders,
+    mock_create_job,
     exception_msg,
     expected_h1,
     expected_err_details,
@@ -2790,11 +2800,17 @@ def test_send_notification_shows_error_if_400(
     class MockHTTPError(HTTPError):
         message = exception_msg
 
+    mocker.patch(
+        "app.main.views.send.check_messages",
+    )
+
     mocker.patch(
         "app.notification_api_client.get_notifications_for_service",
         return_value=FAKE_ONE_OFF_NOTIFICATION,
     )
 
+    mocker.patch("app.s3_client.s3_csv_client.s3upload")
+
     mocker.patch(
         "app.notification_api_client.send_notification",
         side_effect=MockHTTPError(),
@@ -2810,7 +2826,7 @@ def test_send_notification_shows_error_if_400(
         "main.send_notification",
         service_id=service_one["id"],
         template_id=fake_uuid,
-        # _expected_status=200,
+        _expected_status=302,
     )
 
     # assert normalize_spaces(page.select(".banner-dangerous h1")[0].text) == expected_h1
@@ -2826,11 +2842,13 @@ def test_send_notification_shows_email_error_in_trial_mode(
     fake_uuid,
     mocker,
     mock_get_service_email_template,
+    mock_create_job,
 ):
     class MockHTTPError(HTTPError):
         message = TRIAL_MODE_MSG
         status_code = 400
 
+    mocker.patch("app.main.views.send.check_messages")
     mocker.patch(
         "app.notification_api_client.get_notifications_for_service",
         return_value=FAKE_ONE_OFF_NOTIFICATION,

tests/conftest.py

@@ -1396,7 +1396,15 @@ def mock_check_verify_code_code_expired(mocker):
 
 @pytest.fixture()
 def mock_create_job(mocker, api_user_active):
-    def _create(job_id, service_id, scheduled_for=None):
+    def _create(
+        job_id,
+        service_id,
+        scheduled_for=None,
+        template_id=None,
+        original_file_name=None,
+        notification_count=None,
+        valid=None,
+    ):
         return job_json(
             service_id,
             api_user_active,