From 090769a06926fa6afde2d2087bc5179907edfd0d Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Fri, 21 Jun 2019 16:55:38 +0100 Subject: [PATCH] Pull cancellable job statuses from utils and fix tests --- app/dao/jobs_dao.py | 30 ++++++++++++++---------------- requirements-app.txt | 2 +- requirements.txt | 8 ++++---- tests/app/job/test_rest.py | 14 ++++++++------ 4 files changed, 27 insertions(+), 27 deletions(-) diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index f6a2e0b0f..d47aefca5 100644 --- a/app/dao/jobs_dao.py +++ b/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 diff --git a/requirements-app.txt b/requirements-app.txt index 460c9bf4c..542c10758 100644 --- a/requirements-app.txt +++ b/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 diff --git a/requirements.txt b/requirements.txt index d3348afc0..0bc17e2d8 100644 --- a/requirements.txt +++ b/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 diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index 1499bb2bb..0609cef40 100644 --- a/tests/app/job/test_rest.py +++ b/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'