Pull cancellable job statuses from utils and fix tests

app/dao/jobs_dao.py

@@ -2,7 +2,7 @@ import uuid
 from datetime import datetime, timedelta
 
 from flask import current_app
-from notifications_utils.letter_timings import letter_can_be_cancelled
+from notifications_utils.letter_timings import letter_can_be_cancelled, CANCELLABLE_JOB_LETTER_STATUSES
 from notifications_utils.statsd_decorators import statsd
 from sqlalchemy import (
     asc,
@@ -30,11 +30,6 @@ from app.models import (
 )
 
 
-CANCELLABLE_LETTER_STATUSES = [
-    'created', 'cancelled', 'virus-scan-failed', 'validation-failed', 'technical-failure', 'pending-virus-check'
-]
-
-
 @statsd(namespace="dao")
 def dao_get_notification_outcomes_for_job(service_id, job_id):
     return db.session.query(
@@ -176,16 +171,19 @@ def can_letter_job_be_cancelled(job):
     template = dao_get_template_by_id(job.template_id)
     if template.template_type != LETTER_TYPE:
         return "Only letter jobs can be cancelled through this endpoint. This is not a letter job."
-    if job.job_status != JOB_STATUS_FINISHED:
-        return "This job is still being processed. Wait a couple of minutes and try again."
 
-    count_notifications = Notification.query.filter(
-        Notification.job_id == job.id,
-        Notification.status.in_(CANCELLABLE_LETTER_STATUSES)
-    ).count()
-    if count_notifications != job.notification_count:
+    notifications = Notification.query.filter(
+        Notification.job_id == job.id
+    ).all()
+    count_notifications = len(notifications)
+    if job.job_status != JOB_STATUS_FINISHED or count_notifications != job.notification_count:
         return "This job is still being processed. Wait a couple of minutes and try again."
-    if letter_can_be_cancelled(NOTIFICATION_CREATED, job.created_at):
-        return True
-    else:
+    count_cancellable_notifications = len([
+        n for n in notifications if n.status in CANCELLABLE_JOB_LETTER_STATUSES
+    ])
+    if count_cancellable_notifications != job.notification_count or not letter_can_be_cancelled(
+        NOTIFICATION_CREATED, job.created_at
+    ):
         return "Sorry, it's too late, letters have already been sent."
+
+    return True

requirements-app.txt

@@ -29,6 +29,6 @@ awscli-cwlogs>=1.4,<1.5
 # Putting upgrade on hold due to v1.0.0 using sha512 instead of sha1 by default
 itsdangerous==0.24  # pyup: <1.0.0
 
-git+https://github.com/alphagov/notifications-utils.git@33.0.0#egg=notifications-utils==33.0.0
+git+https://github.com/alphagov/notifications-utils.git@33.1.0#egg=notifications-utils==33.1.0
 
 git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3

requirements.txt

@@ -31,21 +31,21 @@ awscli-cwlogs>=1.4,<1.5
 # Putting upgrade on hold due to v1.0.0 using sha512 instead of sha1 by default
 itsdangerous==0.24  # pyup: <1.0.0
 
-git+https://github.com/alphagov/notifications-utils.git@33.0.0#egg=notifications-utils==33.0.0
+git+https://github.com/alphagov/notifications-utils.git@33.1.0#egg=notifications-utils==33.1.0
 
 git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3
 
 ## The following requirements were added by pip freeze:
-alembic==1.0.10
+alembic==1.0.11
 amqp==1.4.9
 anyjson==0.3.3
 attrs==19.1.0
-awscli==1.16.185
+awscli==1.16.188
 bcrypt==3.1.7
 billiard==3.3.0.23
 bleach==3.1.0
 boto3==1.6.16
-botocore==1.12.175
+botocore==1.12.178
 certifi==2019.6.16
 chardet==3.0.4
 Click==7.0

tests/app/job/test_rest.py

@@ -86,7 +86,9 @@ def test_dao_cancel_letter_job_updates_notifications_and_job_to_cancelled(sample
 
 
 @freeze_time('2019-06-13 13:00')
-def test_dao_cancel_letter_job_does_not_allow_cancel_if_notification_in_sending(sample_letter_template, admin_request):
+def test_dao_cancel_letter_job_does_not_allow_cancel_if_notification_status_sending(
+    sample_letter_template, admin_request
+):
     job = create_job(template=sample_letter_template, notification_count=2, job_status='finished')
     letter_1 = create_notification(template=job.template, job=job, status='sending')
     letter_2 = create_notification(template=job.template, job=job, status='created')
@@ -96,7 +98,7 @@ def test_dao_cancel_letter_job_does_not_allow_cancel_if_notification_in_sending(
         job_id=job.id,
         _expected_status=400
     )
-    assert response == "This job is still being processed. Wait a couple of minutes and try again."
+    assert response["message"] == "Sorry, it's too late, letters have already been sent."
     assert letter_1.status == 'sending'
     assert letter_2.status == 'created'
     assert job.job_status == 'finished'
@@ -116,7 +118,7 @@ def test_dao_cancel_letter_job_does_not_allow_cancel_if_letters_already_sent_to_
             job_id=job.id,
             _expected_status=400
         )
-    assert response == "Sorry, it's too late, letters have already been sent."
+    assert response["message"] == "Sorry, it's too late, letters have already been sent."
     assert letter.status == 'created'
     assert job.job_status == 'finished'
 
@@ -131,7 +133,7 @@ def test_dao_cancel_letter_job_does_not_allow_cancel_if_not_a_letter_job(sample_
         job_id=job.id,
         _expected_status=400
     )
-    assert response == "Only letter jobs can be cancelled through this endpoint. This is not a letter job."
+    assert response["message"] == "Only letter jobs can be cancelled through this endpoint. This is not a letter job."
     assert notification.status == 'created'
     assert job.job_status == 'finished'
 
@@ -146,7 +148,7 @@ def test_dao_cancel_letter_job_does_not_allow_cancel_if_job_not_finished(sample_
         job_id=job.id,
         _expected_status=400
     )
-    assert response == "This job is still being processed. Wait a couple of minutes and try again."
+    assert response["message"] == "This job is still being processed. Wait a couple of minutes and try again."
     assert letter.status == 'created'
     assert job.job_status == 'in progress'
 
@@ -163,7 +165,7 @@ def test_dao_cancel_letter_job_does_not_allow_cancel_if_notifications_not_in_db_
         job_id=job.id,
         _expected_status=400
     )
-    assert response == "This job is still being processed. Wait a couple of minutes and try again."
+    assert response["message"] == "This job is still being processed. Wait a couple of minutes and try again."
     assert letter.status == 'created'
     assert job.job_status == 'finished'