diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index fb63f9319..acb1a4346 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -173,17 +173,25 @@ def cancel_job(service_id, job_id): @main.route("/services//jobs//cancel", methods=['GET', 'POST']) @user_has_permissions() def cancel_letter_job(service_id, job_id): - if request.method == 'POST': + job = job_api_client.get_job(service_id, job_id)['data'] + notifications = notification_api_client.get_notifications_for_service( + job['service'], job['id'] + )['notifications'] + if job['job_status'] != 'finished' or len(notifications) < job['notification_count']: + flash("We are still processing these letters, please try again in a minute.", 'try again') + return view_job(service_id, job_id) try: number_of_letters = job_api_client.cancel_letter_job(current_service.id, job_id) except HTTPError as e: flash(e.message, 'dangerous') return redirect(url_for('main.view_job', service_id=service_id, job_id=job_id)) - flash(" {} letters have been cancelled succesfully".format(number_of_letters), 'default_with_tick') + flash("Cancelled {:,.0f} letters from {}".format( + number_of_letters, job['original_file_name'] + ), 'default_with_tick') return redirect(url_for('main.service_dashboard', service_id=service_id)) - flash("Are you sure you want to cancel sending those letters?", 'cancel') + flash("Are you sure you want to cancel sending these letters?", 'cancel') return view_job(service_id, job_id) @@ -435,12 +443,10 @@ def get_job_partials(job, template): n for n in notifications["notifications"] if n["status"] not in CANCELLABLE_JOB_LETTER_STATUSES ] job_created = job["created_at"][:-6] - if job["job_status"] != "finished": - can_letter_job_be_cancelled = "This job is still being processed. Wait a couple of minutes and try again." - elif not letter_can_be_cancelled( + if not letter_can_be_cancelled( "created", datetime.strptime(job_created, '%Y-%m-%dT%H:%M:%S.%f') ) or len(not_cancellable) != 0: - can_letter_job_be_cancelled = "Cancel sending those letters" + can_letter_job_be_cancelled = False else: can_letter_job_be_cancelled = True return { diff --git a/app/templates/flash_messages.html b/app/templates/flash_messages.html index a95f2fb19..08abc1b3b 100644 --- a/app/templates/flash_messages.html +++ b/app/templates/flash_messages.html @@ -2,11 +2,18 @@ {% with messages = get_flashed_messages(with_categories=true) %} {% if messages %} {% for category, message in messages %} + {% if category in ['cancel', 'delete', 'suspend', 'resume', 'remove', 'revoke this API key'] %} + {% set delete_button_text = "Yes, {}".format(category) %} + {% elif category == 'try again' %} + {% set delete_button_text = category|capitalize %} + {% else %} + {% set delete_button_text = None %} + {% endif %}
{{ banner( message if message is string else message[0], 'default' if ((category == 'default') or (category == 'default_with_tick')) else 'dangerous', - delete_button="Yes, {}".format(category) if category in ['cancel', 'delete', 'suspend', 'resume', 'remove', 'revoke this API key'] else None, + delete_button=delete_button_text, with_tick=True if category == 'default_with_tick' else False, context=message[1] if message is not string )}} diff --git a/app/templates/views/jobs/job.html b/app/templates/views/jobs/job.html index 2325f71c9..afa13138f 100644 --- a/app/templates/views/jobs/job.html +++ b/app/templates/views/jobs/job.html @@ -25,11 +25,7 @@
- {% if can_cancel_letter_job == True %} - Cancel sending those letters - {% else %} -

Can't cancel: {{ can_cancel_letter_job }}

- {% endif %} + Cancel sending these letters {% else %}
 
diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 98b0190f9..f9083c0c1 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -261,6 +261,7 @@ def test_should_show_page_for_one_job_with_flexible_data_retention( ) assert page.find('span', {'id': 'time-left'}).text == 'Data available for 10 days' + assert "Cancel sending these letters" not in page def test_get_jobs_should_tell_user_if_more_than_one_page( @@ -490,9 +491,24 @@ def test_should_not_show_cancelled_job( def test_should_cancel_letter_job( client_request, mocker, + active_user_with_permissions ): job_id = uuid.uuid4() - mock_cancel = mocker.patch('app.main.jobs.job_api_client.cancel_letter_job') + job = job_json( + SERVICE_ONE_ID, + active_user_with_permissions, + job_id=job_id, + created_at="2019-06-20T15:30:00.000001+00:00", + job_status="finished" + ) + mocker.patch('app.job_api_client.get_job', side_effect=[{"data": job}]) + notifications_json = notification_json(SERVICE_ONE_ID, job=job, status="created", template_type="letter") + mocker.patch('app.job_api_client.get_job', side_effect=[{"data": job}]) + mocker.patch( + 'app.notification_api_client.get_notifications_for_service', + side_effect=[notifications_json] + ) + mock_cancel = mocker.patch('app.main.jobs.job_api_client.cancel_letter_job', return_value=5) client_request.post( 'main.cancel_letter_job', service_id=SERVICE_ONE_ID, @@ -539,16 +555,90 @@ def test_should_not_show_cancel_link_for_letter_job_if_too_late( job_id=str(job_id) ) - assert "Cancel sending those letters" not in page + assert "Cancel sending these letters" not in page assert page.find('p', {'id': 'printing-info'}).text.strip() == "Printed {} at 5:30pm".format(expected_fragment) -def test_dont_cancel_letter_job_when_to_early_to_cancel(): - pass +@freeze_time("2019-06-20 15:32:00.000001") +@pytest.mark.parametrize(" job_status", [ + "finished", "in progress" +]) +def test_should_show_cancel_link_for_letter_job( + client_request, + mocker, + mock_get_service_letter_template, + mock_get_service_data_retention, + active_user_with_permissions, + job_status, +): + job_id = uuid.uuid4() + job = job_json( + SERVICE_ONE_ID, + active_user_with_permissions, + job_id=job_id, + created_at="2019-06-20T15:30:00.000001+00:00", + job_status=job_status + ) + notifications_json = notification_json(SERVICE_ONE_ID, job=job, status="created", template_type="letter") + mocker.patch('app.job_api_client.get_job', side_effect=[{"data": job}]) + mocker.patch( + 'app.notification_api_client.get_notifications_for_service', + side_effect=[notifications_json] + ) + + page = client_request.get( + 'main.view_job', + service_id=SERVICE_ONE_ID, + job_id=str(job_id) + ) + + assert page.find('a', text='Cancel sending these letters').attrs["href"] == url_for( + "main.cancel_letter_job", service_id=SERVICE_ONE_ID, job_id=job_id + ) + assert page.find('p', {'id': 'printing-info'}).text.strip() == "Printing starts today at 5:30pm" -def test_page_when_user_clicks_cancel_link_letter_job_and_to_early_to_cancel(): - pass +@freeze_time("2019-06-20 15:31:00.000001") +@pytest.mark.parametrize('job_status,number_of_processed_notifications', [['in progress', 2], ['finished', 1]]) +def test_dont_cancel_letter_job_when_to_early_to_cancel( + client_request, + mocker, + mock_get_service_letter_template, + mock_get_service_data_retention, + active_user_with_permissions, + job_status, + number_of_processed_notifications, +): + job_id = uuid.uuid4() + job = job_json( + SERVICE_ONE_ID, + active_user_with_permissions, + job_id=job_id, + created_at="2019-06-20T15:30:00.000001+00:00", + job_status=job_status, + notification_count=2 + ) + mocker.patch('app.job_api_client.get_job', side_effect=[{"data": job}, {"data": job}]) + + notifications_json = notification_json( + SERVICE_ONE_ID, job=job, status="created", template_type="letter", rows=number_of_processed_notifications + ) + mocker.patch( + 'app.notification_api_client.get_notifications_for_service', + side_effect=[notifications_json, notifications_json] + ) + + mock_cancel = mocker.patch('app.main.jobs.job_api_client.cancel_letter_job') + page = client_request.post( + 'main.cancel_letter_job', + service_id=SERVICE_ONE_ID, + job_id=str(job_id), + _expected_status=200, + ) + mock_cancel.assert_not_called() + flash_message = normalize_spaces(page.find('div', class_='banner-dangerous').text) + + assert 'We are still processing these letters, please try again in a minute.' in flash_message @freeze_time("2016-01-01 00:00:00.000001")