Can't cancel letter job if job not processed yet

Also add more tests for showing or not the cancel those letters link

Also check if all notifications already in database

Upgrade delete button text logic to handle more cases

Also corrections following review

app/main/views/jobs.py

@@ -173,17 +173,25 @@ def cancel_job(service_id, job_id):
 @main.route("/services/<service_id>/jobs/<uuid:job_id>/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 {

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 %}
       <div class="bottom-gutter">
         {{ 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
         )}}

app/templates/views/jobs/job.html

@@ -25,11 +25,7 @@
       <div class="js-stick-at-bottom-when-scrolling">
         <div class="page-footer">
           <span class="page-footer-delete-link page-footer-delete-link-without-button">
-            {% if can_cancel_letter_job == True %}
-              <a href="{{ url_for('main.cancel_letter_job', service_id=current_service.id, job_id=job_id) }}">Cancel sending those letters</a>
-            {% else %}
-              <p> Can't cancel: {{ can_cancel_letter_job }} </p>
-            {% endif %}
+              <a href="{{ url_for('main.cancel_letter_job', service_id=current_service.id, job_id=job_id) }}">Cancel sending these letters</a>
           </span>
     {% else %}
       <div>&nbsp;</div>

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")