diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index c40311d16..ef35206f5 100644 --- a/app/dao/jobs_dao.py +++ b/app/dao/jobs_dao.py @@ -9,7 +9,6 @@ from sqlalchemy import ( desc, func, ) -from sqlalchemy.testing import not_in_ from app import db from app.dao.dao_utils import transactional @@ -27,7 +26,6 @@ from app.models import ( NOTIFICATION_CANCELLED, JOB_STATUS_CANCELLED ) -from app.variables import LETTER_TEST_API_FILENAME @statsd(namespace="dao") @@ -156,36 +154,31 @@ def dao_get_jobs_older_than_data_retention(notification_types): @transactional -def dao_cancel_letter_job(service_id, job_id): - if can_cancel_letter_job(job_id, service_id): +def dao_cancel_letter_job(job): + if can_cancel_letter_job(job): number_of_notifications_cancelled = Notification.query.filter( - Notification.job_id == job_id + Notification.job_id == job.id ).update({'status': NOTIFICATION_CANCELLED, 'updated_at': datetime.utcnow(), 'billable_units': 0}) - # calling this 2x - pass job into can_cancel_letter_job - job = dao_get_job_by_service_id_and_job_id(service_id, job_id) job.job_status = JOB_STATUS_CANCELLED dao_update_job(job) return number_of_notifications_cancelled + else: + return False -def can_cancel_letter_job(job_id, service_id): - job = dao_get_job_by_service_id_and_job_id(service_id, job_id) +def can_cancel_letter_job(job): # assert is a letter job # assert job status == finished??? # Notifications are not in pending-virus-check - count_notifications_in_sending = Notification.query.filter( - Notification.job_id == job_id, - Notification.status.not_in_('created', 'pending-virus-check', 'cancelled') + count_notifications = Notification.query.filter( + Notification.job_id == job.id, + Notification.status.in_(['created', 'pending-virus-check', 'cancelled']) ).count() - if count_notifications_in_sending > 0: + if count_notifications != job.notification_count: return False - if not letter_can_be_cancelled(NOTIFICATION_CREATED, job.created_at): - return False - else: - return True - + return letter_can_be_cancelled(NOTIFICATION_CREATED, job.created_at) diff --git a/app/job/rest.py b/app/job/rest.py index 1d125c007..9123a59da 100644 --- a/app/job/rest.py +++ b/app/job/rest.py @@ -5,7 +5,6 @@ from flask import ( request, current_app ) -from notifications_utils.letter_timings import letter_can_be_cancelled from app.aws.s3 import get_job_metadata_from_s3 from app.dao.jobs_dao import ( @@ -14,7 +13,9 @@ from app.dao.jobs_dao import ( dao_get_job_by_service_id_and_job_id, dao_get_jobs_by_service_id, dao_get_future_scheduled_job_by_id_and_service_id, - dao_get_notification_outcomes_for_job, dao_cancel_letter_job + dao_get_notification_outcomes_for_job, + dao_cancel_letter_job, + can_cancel_letter_job ) from app.dao.fact_notification_status_dao import fetch_notification_statuses_for_job from app.dao.services_dao import dao_fetch_service_by_id @@ -62,6 +63,19 @@ 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') +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 + 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 + + @job_blueprint.route('//notifications', methods=['GET']) def get_all_notifications_for_service_job(service_id, job_id): data = notifications_filter_schema.load(request.args).data diff --git a/tests/app/dao/test_jobs_dao.py b/tests/app/dao/test_jobs_dao.py index 500bd90f0..731ae959a 100644 --- a/tests/app/dao/test_jobs_dao.py +++ b/tests/app/dao/test_jobs_dao.py @@ -311,15 +311,24 @@ def assert_job_stat(job, result, sent, delivered, failed): assert result.failed == failed -def test_dao_cancel_letter_job_does_not_allow_cancel_if_notification_in_sending(sample_job): - create_notification(template=sample_job.template, job=sample_job, status='sending') - create_notification(template=sample_job.template, job=sample_job, status='created') - assert not dao_cancel_letter_job(service_id=sample_job.service_id, job_id=sample_job.id) +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) + create_notification(template=job.template, job=job, status='sending') + create_notification(template=job.template, job=job, status='created') + assert not dao_cancel_letter_job(job) def test_dao_cancel_letter_job_updates_notifications_and_job_to_cancelled(sample_job): notification = create_notification(template=sample_job.template, job=sample_job, status='created') - assert dao_cancel_letter_job(service_id=sample_job.service_id, job_id=sample_job.id) == 1 + assert dao_cancel_letter_job(sample_job) == 1 assert notification.status == 'cancelled' assert sample_job.job_status == 'cancelled' + +def test_dao_cancel_letter_job_returns_false_if_too_late(): + pass + + +def test_dao_cancel_letter_returns_false_if_not_a_letter_job(): + pass +