diff --git a/app/main/views/send.py b/app/main/views/send.py index c532d2bed..fe36c961b 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -847,7 +847,7 @@ def send_notification(service_id, template_id): vals = ",".join(values) data = f"{data}\r\n{vals}" - filename = f"one-off:{current_user.name}:{uuid.uuid4()}.csv" + filename = f"one-off-{current_user.name}-{uuid.uuid4()}.csv" my_data = {"filename": filename, "template_id": template_id, "data": data} upload_id = s3upload(service_id, my_data) form = CsvUploadForm() @@ -873,34 +873,18 @@ def send_notification(service_id, template_id): # We have to wait for the job to run and create the notification in the database time.sleep(0.1) - notis = notification_api_client.get_notifications_for_service( + notifications = notification_api_client.get_notifications_for_service( service_id, job_id=upload_id, include_one_off=True ) attempts = 0 - while notis["total"] == 0 and attempts < 5: - notis = notification_api_client.get_notifications_for_service( + while notifications["total"] == 0 and attempts < 5: + notifications = notification_api_client.get_notifications_for_service( service_id, job_id=upload_id, include_one_off=True ) time.sleep(0.1) attempts = attempts + 1 - # TODO we are replacing the original 'one-off send' functionality with a job that - # we create on the fly. The purpose for this is to ultimately remove the phone numbers - # 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: - # - # /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: + + if notifications["total"] == 0 and attempts == 5: # This shows the job we auto-generated for the user return redirect( url_for( @@ -915,7 +899,7 @@ def send_notification(service_id, template_id): ".view_notification", service_id=service_id, from_job=upload_id, - notification_id=notis["notifications"][0]["id"], + notification_id=notifications["notifications"][0]["id"], # used to show the final step of the tour (help=3) or not show # a back link on a just sent one off notification (help=0) help=request.args.get("help"), diff --git a/app/notify_client/job_api_client.py b/app/notify_client/job_api_client.py index cdccca5c8..538bdd370 100644 --- a/app/notify_client/job_api_client.py +++ b/app/notify_client/job_api_client.py @@ -119,16 +119,16 @@ class JobApiClient(NotifyAdminAPIClient): if scheduled_for: scheduled_for = JobApiClient.convert_user_time_to_utc(scheduled_for) - data.update({"scheduled_for": scheduled_for}) + data["scheduled_for"] = scheduled_for if template_id: - data.update({"template_id": template_id}) + data["template_id"] = template_id if original_file_name: - data.update({"original_file_name": original_file_name}) + data["original_file_name"] = original_file_name if notification_count: - data.update({"notification_count": notification_count}) + data["notification_count"] = notification_count if valid: - data.update({"valid": valid}) + data["valid"] = valid data = _attach_current_user(data) job = self.post(url="/service/{}/job".format(service_id), data=data) diff --git a/app/templates/views/check/column-errors.html b/app/templates/views/check/column-errors.html index a11c4a901..a24b5de3e 100644 --- a/app/templates/views/check/column-errors.html +++ b/app/templates/views/check/column-errors.html @@ -11,7 +11,7 @@ Error {% block backLink %} -{% if recipients.__len__() == 1 and not recipients.allowed_to_send_to and not recipients.missing_column_headers %} +{% if recipients|length == 1 and not recipients.allowed_to_send_to and not recipients.missing_column_headers %} {% else %} {{ usaBackLink({ "href": back_link }) }} @@ -136,7 +136,7 @@ Error -{% if recipients.__len__() == 1 and not recipients.allowed_to_send_to and not recipients.missing_column_headers %} +{% if recipients|length == 1 and not recipients.allowed_to_send_to and not recipients.missing_column_headers %} {% else %}
diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 17691c22d..41ecb9009 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -2823,9 +2823,8 @@ def test_send_notification_shows_error_if_400( session["recipient"] = "2028675301" session["placeholders"] = {"name": "a" * 900} - # TODO This part of the test is commented out due to notify-api-679 which is - # replacing one-off sends with jobs. The new workflow is not embedded error messages into - # the page properly when the user specifies an invalid phone number + + # This now redirects to the jobs results page page = client_request.post( "main.send_notification", service_id=service_one["id"], @@ -2833,11 +2832,6 @@ def test_send_notification_shows_error_if_400( _expected_status=302, ) - # assert normalize_spaces(page.select(".banner-dangerous h1")[0].text) == expected_h1 - # assert ( - # normalize_spaces(page.select(".banner-dangerous p")[0].text) - # == expected_err_details - # ) assert not page.find("input[type=submit]") @@ -2867,24 +2861,14 @@ def test_send_notification_shows_email_error_in_trial_mode( session["recipient"] = "test@example.com" session["placeholders"] = {"date": "foo", "thing": "bar"} - # TODO This part of the test is commented out due to notify-api-679 which is - # replacing one-off sends with jobs. The new workflow is not embedded error messages into - # the page properly when the user specifies an invalid phone number - # page = client_request.post( + # Calling this means we successful ran a job so we will be redirect to the jobs page client_request.post( "main.send_notification", service_id=SERVICE_ONE_ID, template_id=fake_uuid, - # _expected_status=302, + _expected_status=302, ) - # assert normalize_spaces(page.select(".banner-dangerous h1")[0].text) == ( - # "You cannot send to this email address" - # ) - # assert normalize_spaces(page.select(".banner-dangerous p")[0].text) == ( - # "In trial mode you can only send to yourself and members of your team" - # ) - @pytest.mark.parametrize( ("endpoint", "extra_args"),