diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index d26b5a647..f6a2e0b0f 100644 --- a/app/dao/jobs_dao.py +++ b/app/dao/jobs_dao.py @@ -30,6 +30,11 @@ 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( @@ -157,30 +162,30 @@ def dao_get_jobs_older_than_data_retention(notification_types): @transactional def dao_cancel_letter_job(job): - if can_cancel_letter_job(job): - number_of_notifications_cancelled = Notification.query.filter( - Notification.job_id == job.id - ).update({'status': NOTIFICATION_CANCELLED, - 'updated_at': datetime.utcnow(), - 'billable_units': 0}) - job.job_status = JOB_STATUS_CANCELLED - dao_update_job(job) - return number_of_notifications_cancelled - else: - return False + number_of_notifications_cancelled = Notification.query.filter( + Notification.job_id == job.id + ).update({'status': NOTIFICATION_CANCELLED, + 'updated_at': datetime.utcnow(), + 'billable_units': 0}) + job.job_status = JOB_STATUS_CANCELLED + dao_update_job(job) + return number_of_notifications_cancelled -def can_cancel_letter_job(job): +def can_letter_job_be_cancelled(job): template = dao_get_template_by_id(job.template_id) if template.template_type != LETTER_TYPE: - return False + return "Only letter jobs can be cancelled through this endpoint. This is not a letter job." if job.job_status != JOB_STATUS_FINISHED: - return False - # Notifications are not in pending-virus-check + 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_(['created', 'pending-virus-check', 'cancelled']) + Notification.status.in_(CANCELLABLE_LETTER_STATUSES) ).count() if count_notifications != job.notification_count: - return False - return letter_can_be_cancelled(NOTIFICATION_CREATED, job.created_at) + 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: + return "Sorry, it's too late, letters have already been sent." diff --git a/app/job/rest.py b/app/job/rest.py index 9123a59da..112da1459 100644 --- a/app/job/rest.py +++ b/app/job/rest.py @@ -15,7 +15,7 @@ from app.dao.jobs_dao import ( dao_get_future_scheduled_job_by_id_and_service_id, dao_get_notification_outcomes_for_job, dao_cancel_letter_job, - can_cancel_letter_job + can_letter_job_be_cancelled ) from app.dao.fact_notification_status_dao import fetch_notification_statuses_for_job from app.dao.services_dao import dao_fetch_service_by_id @@ -63,17 +63,15 @@ def cancel_job(service_id, job_id): return get_job_by_service_and_job_id(service_id, job_id) -@job_blueprint.route('//cancel-letter-job') +@job_blueprint.route('//cancel-letter-job', methods=['POST']) def cancel_letter_job(service_id, job_id): job = dao_get_job_by_service_id_and_job_id(service_id, job_id) - if can_cancel_letter_job(job): - dao_cancel_letter_job(job) - return jsonify(), 201 + can_we_cancel = can_letter_job_be_cancelled(job) + if can_we_cancel is True: + data = dao_cancel_letter_job(job) + return jsonify(data), 200 else: - # reasons the letter can't be cancelled: - # letters are not yet saved to db --> can still cancel but later - # it's too late - return jsonify("CHANGE ME: some error to say the job can't be cancelled"), 400 + return jsonify(message=can_we_cancel), 400 @job_blueprint.route('//notifications', methods=['GET']) diff --git a/tests/app/dao/test_jobs_dao.py b/tests/app/dao/test_jobs_dao.py index 96e23d238..da84028b4 100644 --- a/tests/app/dao/test_jobs_dao.py +++ b/tests/app/dao/test_jobs_dao.py @@ -13,8 +13,7 @@ from app.dao.jobs_dao import ( dao_set_scheduled_jobs_to_pending, dao_get_future_scheduled_job_by_id_and_service_id, dao_get_notification_outcomes_for_job, - dao_get_jobs_older_than_data_retention, - dao_cancel_letter_job + dao_get_jobs_older_than_data_retention ) from app.models import ( Job, @@ -309,57 +308,3 @@ def assert_job_stat(job, result, sent, delivered, failed): assert result.sent == sent assert result.delivered == delivered assert result.failed == failed - - -@freeze_time('2019-06-13 13:00') -def test_dao_cancel_letter_job_updates_notifications_and_job_to_cancelled(sample_letter_template): - job = create_job(template=sample_letter_template, notification_count=1, job_status='finished') - notification = create_notification(template=job.template, job=job, status='created') - assert dao_cancel_letter_job(job) == 1 - assert notification.status == 'cancelled' - assert job.job_status == 'cancelled' - - -@freeze_time('2019-06-13 13:00') -def test_dao_cancel_letter_job_does_not_allow_cancel_if_notification_in_sending(sample_letter_template): - 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') - assert not dao_cancel_letter_job(job) - assert letter_1.status == 'sending' - assert letter_2.status == 'created' - assert job.job_status == 'finished' - - -def test_dao_cancel_letter_job_does_not_allow_cancel_if_letters_already_sent_to_dvla(sample_letter_template): - with freeze_time('2019-06-13 13:00'): - job = create_job(template=sample_letter_template, notification_count=1, job_status='finished') - letter = create_notification(template=job.template, job=job, status='created') - - with freeze_time('2019-06-13 17:32'): - assert not dao_cancel_letter_job(job) - assert letter.status == 'created' - assert job.job_status == 'finished' - - -@freeze_time('2019-06-13 13:00') -def test_dao_cancel_letter_job_does_not_allow_cancel_if_not_a_letter_job(sample_template): - job = create_job(template=sample_template, notification_count=1, job_status='finished') - notification = create_notification(template=job.template, job=job, status='created') - assert not dao_cancel_letter_job(job) - assert notification.status == 'created' - assert job.job_status == 'finished' - - -@freeze_time('2019-06-13 13:00') -def test_dao_cancel_letter_job_does_not_allow_cancel_if_job_not_finished(sample_letter_template): - job = create_job(template=sample_letter_template, notification_count=1, job_status="finished") - create_notification(template=job.template, job=job, status='created') - assert not dao_cancel_letter_job(job) - - -@freeze_time('2019-06-13 13:00') -def test_dao_cancel_letter_job_does_not_allow_cancel_if_notifications_not_in_db_yet(sample_letter_template): - job = create_job(template=sample_letter_template, notification_count=2, job_status='finished') - create_notification(template=job.template, job=job, status='created') - assert not dao_cancel_letter_job(job) diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index 03d4a289e..1499bb2bb 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -71,6 +71,103 @@ def test_cant_cancel_normal_job(client, sample_job, mocker): assert mock_update.call_count == 0 +@freeze_time('2019-06-13 13:00') +def test_dao_cancel_letter_job_updates_notifications_and_job_to_cancelled(sample_letter_template, admin_request): + job = create_job(template=sample_letter_template, notification_count=1, job_status='finished') + notification = create_notification(template=job.template, job=job, status='created') + response = admin_request.post( + 'job.cancel_letter_job', + service_id=job.service_id, + job_id=job.id, + ) + assert response == 1 + assert notification.status == 'cancelled' + assert job.job_status == 'cancelled' + + +@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): + 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') + response = admin_request.post( + 'job.cancel_letter_job', + service_id=job.service_id, + job_id=job.id, + _expected_status=400 + ) + assert response == "This job is still being processed. Wait a couple of minutes and try again." + assert letter_1.status == 'sending' + assert letter_2.status == 'created' + assert job.job_status == 'finished' + + +def test_dao_cancel_letter_job_does_not_allow_cancel_if_letters_already_sent_to_dvla( + sample_letter_template, admin_request +): + with freeze_time('2019-06-13 13:00'): + job = create_job(template=sample_letter_template, notification_count=1, job_status='finished') + letter = create_notification(template=job.template, job=job, status='created') + + with freeze_time('2019-06-13 17:32'): + response = admin_request.post( + 'job.cancel_letter_job', + service_id=job.service_id, + job_id=job.id, + _expected_status=400 + ) + assert response == "Sorry, it's too late, letters have already been sent." + assert letter.status == 'created' + assert job.job_status == 'finished' + + +@freeze_time('2019-06-13 13:00') +def test_dao_cancel_letter_job_does_not_allow_cancel_if_not_a_letter_job(sample_template, admin_request): + job = create_job(template=sample_template, notification_count=1, job_status='finished') + notification = create_notification(template=job.template, job=job, status='created') + response = admin_request.post( + 'job.cancel_letter_job', + service_id=job.service_id, + 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 notification.status == 'created' + assert job.job_status == 'finished' + + +@freeze_time('2019-06-13 13:00') +def test_dao_cancel_letter_job_does_not_allow_cancel_if_job_not_finished(sample_letter_template, admin_request): + job = create_job(template=sample_letter_template, notification_count=1, job_status="in progress") + letter = create_notification(template=job.template, job=job, status='created') + response = admin_request.post( + 'job.cancel_letter_job', + service_id=job.service_id, + job_id=job.id, + _expected_status=400 + ) + assert response == "This job is still being processed. Wait a couple of minutes and try again." + assert letter.status == 'created' + assert job.job_status == 'in progress' + + +@freeze_time('2019-06-13 13:00') +def test_dao_cancel_letter_job_does_not_allow_cancel_if_notifications_not_in_db_yet( + sample_letter_template, admin_request +): + job = create_job(template=sample_letter_template, notification_count=2, job_status='finished') + letter = create_notification(template=job.template, job=job, status='created') + response = admin_request.post( + 'job.cancel_letter_job', + service_id=job.service_id, + job_id=job.id, + _expected_status=400 + ) + assert response == "This job is still being processed. Wait a couple of minutes and try again." + assert letter.status == 'created' + assert job.job_status == 'finished' + + def test_create_unscheduled_job(client, sample_template, mocker, fake_uuid): mocker.patch('app.celery.tasks.process_job.apply_async') mocker.patch('app.job.rest.get_job_metadata_from_s3', return_value={