From 4da08adff286c38e590bd0770604e1ec38e4bbff Mon Sep 17 00:00:00 2001 From: stvnrlly Date: Wed, 15 Feb 2023 11:53:52 -0500 Subject: [PATCH] letter_job cleanup --- app/__init__.py | 4 - app/dao/jobs_dao.py | 44 ---------- app/job/rest.py | 6 -- app/letters/__init__.py | 0 app/letters/letter_schemas.py | 17 ---- app/letters/rest.py | 25 ------ app/models.py | 6 -- app/service/rest.py | 8 -- app/v2/notifications/get_notifications.py | 29 ------- app/v2/notifications/post_notifications.py | 35 -------- tests/app/celery/test_tasks.py | 35 -------- tests/app/conftest.py | 19 ----- tests/app/dao/test_fact_billing_dao.py | 2 +- tests/app/dao/test_jobs_dao.py | 94 ---------------------- 14 files changed, 1 insertion(+), 323 deletions(-) delete mode 100644 app/letters/__init__.py delete mode 100644 app/letters/letter_schemas.py delete mode 100644 app/letters/rest.py diff --git a/app/__init__.py b/app/__init__.py index 2dc22cf92..20d93c37c 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -139,7 +139,6 @@ def register_blueprint(application): from app.inbound_number.rest import inbound_number_blueprint from app.inbound_sms.rest import inbound_sms as inbound_sms_blueprint from app.job.rest import job_blueprint - from app.letters.rest import letter_job from app.notifications.notifications_ses_callback import ( ses_callback_blueprint, ) @@ -225,9 +224,6 @@ def register_blueprint(application): email_branding_blueprint.before_request(requires_admin_auth) application.register_blueprint(email_branding_blueprint, url_prefix='/email-branding') - letter_job.before_request(requires_admin_auth) - application.register_blueprint(letter_job) - billing_blueprint.before_request(requires_admin_auth) application.register_blueprint(billing_blueprint) diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index 575f8a65b..49b12e898 100644 --- a/app/dao/jobs_dao.py +++ b/app/dao/jobs_dao.py @@ -2,23 +2,13 @@ import uuid from datetime import datetime, timedelta from flask import current_app -from notifications_utils.letter_timings import ( - CANCELLABLE_JOB_LETTER_STATUSES, - letter_can_be_cancelled, -) from sqlalchemy import and_, asc, desc, func from app import db -from app.dao.dao_utils import autocommit -from app.dao.templates_dao import dao_get_template_by_id from app.models import ( - JOB_STATUS_CANCELLED, JOB_STATUS_FINISHED, JOB_STATUS_PENDING, JOB_STATUS_SCHEDULED, - LETTER_TYPE, - NOTIFICATION_CANCELLED, - NOTIFICATION_CREATED, FactNotificationStatus, Job, Notification, @@ -183,40 +173,6 @@ def dao_get_jobs_older_than_data_retention(notification_types): return jobs -@autocommit -def dao_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 - - -def can_letter_job_be_cancelled(job): - template = dao_get_template_by_id(job.template_id) - if template.template_type != LETTER_TYPE: - return False, "Only letter jobs can be cancelled through this endpoint. This is not a letter job." - - 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 False, "We are still processing these letters, please try again in a minute." - 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 False, "It’s too late to cancel sending, these letters have already been sent." - - return True, None - - def find_jobs_with_missing_rows(): # Jobs can be a maximum of 100,000 rows. It typically takes 10 minutes to create all those notifications. # Using 20 minutes as a condition seems reasonable. diff --git a/app/job/rest.py b/app/job/rest.py index 1dd1eabfb..daf63d11f 100644 --- a/app/job/rest.py +++ b/app/job/rest.py @@ -63,12 +63,6 @@ def cancel_job(service_id, job_id): return get_job_by_service_and_job_id(service_id, job_id) -# TODO: return deprecation notice -# @job_blueprint.route('//cancel-letter-job', methods=['POST']) -# def cancel_letter_job(service_id, job_id): -# pass - - @job_blueprint.route('//notifications', methods=['GET']) def get_all_notifications_for_service_job(service_id, job_id): data = notifications_filter_schema.load(request.args) diff --git a/app/letters/__init__.py b/app/letters/__init__.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/app/letters/letter_schemas.py b/app/letters/letter_schemas.py deleted file mode 100644 index 13d10b277..000000000 --- a/app/letters/letter_schemas.py +++ /dev/null @@ -1,17 +0,0 @@ -letter_references = { - "$schema": "http://json-schema.org/draft-07/schema#", - "description": "list of letter notification references", - "type": "object", - "title": "references", - "properties": { - "references": { - "type": "array", - "items": { - "type": "string", - "pattern": "^[0-9A-Z]{16}$" - }, - "minItems": 1 - }, - }, - "required": ["references"] -} diff --git a/app/letters/rest.py b/app/letters/rest.py deleted file mode 100644 index 56dff3114..000000000 --- a/app/letters/rest.py +++ /dev/null @@ -1,25 +0,0 @@ -from flask import Blueprint - -from app.v2.errors import register_errors - -letter_job = Blueprint("letter-job", __name__) -register_errors(letter_job) - -# too many references will make SQS error (as the task can only be 256kb) -# Maybe doesn't matter anymore with Redis as the celery backing store -MAX_REFERENCES_PER_TASK = 5000 - - -# TODO: return deprecation notice -# @letter_job.route('/letters/returned', methods=['POST']) -# def create_process_returned_letters_job(): -# references = validate(request.get_json(), letter_references)['references'] - -# for start_index in range(0, len(references), MAX_REFERENCES_PER_TASK): -# process_returned_letters_list.apply_async( -# args=(references[start_index:start_index + MAX_REFERENCES_PER_TASK], ), -# queue=QueueNames.DATABASE, -# compression='zlib' -# ) - -# return jsonify(references=references), 200 diff --git a/app/models.py b/app/models.py index d46c17333..4fee1f79d 100644 --- a/app/models.py +++ b/app/models.py @@ -1331,12 +1331,6 @@ NOTIFICATION_STATUS_TYPES_ENUM = db.Enum(*NOTIFICATION_STATUS_TYPES, name='notif NOTIFICATION_STATUS_LETTER_ACCEPTED = 'accepted' NOTIFICATION_STATUS_LETTER_RECEIVED = 'received' -# TODO: delete these keywords for postage -FIRST_CLASS = 'first' -SECOND_CLASS = 'second' -EUROPE = 'europe' -REST_OF_WORLD = 'rest-of-world' - class NotificationStatusTypes(db.Model): __tablename__ = 'notification_status_types' diff --git a/app/service/rest.py b/app/service/rest.py index 3f4b5515f..d344a84d3 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -721,14 +721,6 @@ def create_one_off_notification(service_id): return jsonify(resp), 201 -# TODO: return deprecation notice -# @service_blueprint.route('//send-pdf-letter', methods=['POST']) -# def create_pdf_letter(service_id): -# data = validate(request.get_json(), send_pdf_letter_request) -# resp = send_pdf_letter_notification(service_id, data) -# return jsonify(resp), 201 - - @service_blueprint.route('//email-reply-to', methods=["GET"]) def get_email_reply_to_addresses(service_id): result = dao_get_reply_to_by_service_id(service_id) diff --git a/app/v2/notifications/get_notifications.py b/app/v2/notifications/get_notifications.py index 6fdaf3ecc..403f9742d 100644 --- a/app/v2/notifications/get_notifications.py +++ b/app/v2/notifications/get_notifications.py @@ -20,35 +20,6 @@ def get_notification_by_id(notification_id): return jsonify(notification.serialize()), 200 -# TODO: return deprecation notice -# @v2_notification_blueprint.route('//pdf', methods=['GET']) -# def get_pdf_for_notification(notification_id): -# _data = {"notification_id": notification_id} -# validate(_data, notification_by_id) -# notification = notifications_dao.get_notification_by_id( -# notification_id, authenticated_service.id, _raise=True -# ) - -# if notification.notification_type != LETTER_TYPE: -# raise BadRequestError(message="Notification is not a letter") - -# if notification.status == NOTIFICATION_VIRUS_SCAN_FAILED: -# raise BadRequestError(message='File did not pass the virus scan') - -# if notification.status == NOTIFICATION_TECHNICAL_FAILURE: -# raise BadRequestError(message='PDF not available for letters in status {}'.format(notification.status)) - -# if notification.status == NOTIFICATION_PENDING_VIRUS_CHECK: -# raise PDFNotReadyError() - -# try: -# pdf_data, metadata = get_letter_pdf_and_metadata(notification) -# except Exception: -# raise PDFNotReadyError() - -# return send_file(path_or_file=BytesIO(pdf_data), mimetype='application/pdf') - - @v2_notification_blueprint.route("", methods=['GET']) def get_notifications(): _data = request.args.to_dict(flat=False) diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index fed503912..6c6add4d8 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -66,41 +66,6 @@ POST_NOTIFICATION_JSON_PARSE_DURATION_SECONDS = Histogram( ) -# TODO: return deprecation message -# @v2_notification_blueprint.route('/{}'.format(LETTER_TYPE), methods=['POST']) -# def post_precompiled_letter_notification(): -# request_json = get_valid_json() -# if 'content' not in (request_json or {}): -# return post_notification(LETTER_TYPE) - -# form = validate(request_json, post_precompiled_letter_request) - -# # Check permission to send letters -# check_service_has_permission(LETTER_TYPE, authenticated_service.permissions) - -# check_rate_limiting(authenticated_service, api_user) - -# template = get_precompiled_letter_template(authenticated_service.id) - -# # For precompiled letters the to field will be set to Provided as PDF until the validation passes, -# # then the address of the letter will be set as the to field -# form['personalisation'] = { -# 'address_line_1': 'Provided as PDF' -# } - -# notification = process_letter_notification( -# letter_data=form, -# api_key=api_user, -# service=authenticated_service, -# template=template, -# template_with_content=None, # not required for precompiled -# reply_to_text='', # not required for precompiled -# precompiled=True -# ) - -# return jsonify(notification), 201 - - @v2_notification_blueprint.route('/', methods=['POST']) def post_notification(notification_type): with POST_NOTIFICATION_JSON_PARSE_DURATION_SECONDS.time(): diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 745fba234..0d98ff54e 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -291,41 +291,6 @@ def test_should_process_email_job_with_sender_id(email_job_with_placeholders, mo ) -@freeze_time("2016-01-01 11:09:00.061258") -def test_should_process_letter_job(sample_letter_job, mocker): - csv = """address_line_1,address_line_2,address_line_3,address_line_4,postcode,name - A1,A2,A3,A4,A_POST,Alice - """ - s3_mock = mocker.patch('app.celery.tasks.s3.get_job_and_metadata_from_s3', - return_value=(csv, {"sender_id": None})) - process_row_mock = mocker.patch('app.celery.tasks.process_row') - mocker.patch('app.celery.tasks.create_uuid', return_value="uuid") - - process_job(sample_letter_job.id) - - s3_mock.assert_called_once_with( - service_id=str(sample_letter_job.service.id), - job_id=str(sample_letter_job.id) - ) - - row_call = process_row_mock.mock_calls[0][1] - assert row_call[0].index == 0 - assert row_call[0].recipient == ['A1', 'A2', 'A3', 'A4', None, None, 'A_POST', None] - assert row_call[0].personalisation == { - 'addressline1': 'A1', - 'addressline2': 'A2', - 'addressline3': 'A3', - 'addressline4': 'A4', - 'postcode': 'A_POST' - } - assert row_call[2] == sample_letter_job - assert row_call[3] == sample_letter_job.service - - assert process_row_mock.call_count == 1 - - assert sample_letter_job.job_status == 'finished' - - def test_should_process_all_sms_job(sample_job_with_placeholdered_template, mocker): mocker.patch('app.celery.tasks.s3.get_job_and_metadata_from_s3', diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 9f9ad6bd7..afd1490dc 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -407,25 +407,6 @@ def sample_scheduled_job(sample_template_with_placeholders): ) -@pytest.fixture -def sample_letter_job(sample_letter_template): - service = sample_letter_template.service - data = { - 'id': uuid.uuid4(), - 'service_id': service.id, - 'service': service, - 'template_id': sample_letter_template.id, - 'template_version': sample_letter_template.version, - 'original_file_name': 'some.csv', - 'notification_count': 1, - 'created_at': datetime.utcnow(), - 'created_by': service.created_by, - } - job = Job(**data) - dao_create_job(job) - return job - - @pytest.fixture(scope='function') def sample_notification_with_job(notify_db_session): service = create_service(check_if_service_exists=True) diff --git a/tests/app/dao/test_fact_billing_dao.py b/tests/app/dao/test_fact_billing_dao.py index 9da182131..9331712f3 100644 --- a/tests/app/dao/test_fact_billing_dao.py +++ b/tests/app/dao/test_fact_billing_dao.py @@ -289,7 +289,7 @@ def test_get_rates_for_billing(notify_db_session): create_rate(start_date=datetime.utcnow(), value=33, notification_type='email') rates = get_rates_for_billing() - assert len(rates) == 2 + assert len(rates) == 3 @freeze_time('2017-06-01 12:00') diff --git a/tests/app/dao/test_jobs_dao.py b/tests/app/dao/test_jobs_dao.py index c64345aac..75a8de7a8 100644 --- a/tests/app/dao/test_jobs_dao.py +++ b/tests/app/dao/test_jobs_dao.py @@ -7,8 +7,6 @@ from freezegun import freeze_time from sqlalchemy.exc import IntegrityError from app.dao.jobs_dao import ( - can_letter_job_be_cancelled, - dao_cancel_letter_job, dao_create_job, dao_get_future_scheduled_job_by_id_and_service_id, dao_get_job_by_service_id_and_job_id, @@ -359,98 +357,6 @@ def assert_job_stat(job, result, sent, delivered, failed): assert result.failed == failed -@freeze_time('2019-06-13 13:00') -def test_dao_cancel_letter_job_cancels_job_and_returns_number_of_cancelled_notifications( - 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') - result = dao_cancel_letter_job(job) - assert result == 1 - assert notification.status == 'cancelled' - assert job.job_status == 'cancelled' - - -@freeze_time('2019-06-13 13:00') -def test_can_letter_job_be_cancelled_returns_true_if_job_can_be_cancelled(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') - result, errors = can_letter_job_be_cancelled(job) - assert result - assert not errors - - -@freeze_time('2019-06-13 13:00') -def test_can_letter_job_be_cancelled_returns_false_and_error_message_if_notification_status_sending( - sample_letter_template -): - job = create_job(template=sample_letter_template, notification_count=2, job_status='finished') - create_notification(template=job.template, job=job, status='sending') - create_notification(template=job.template, job=job, status='created') - result, errors = can_letter_job_be_cancelled(job) - assert not result - assert errors == "It’s too late to cancel sending, these letters have already been sent." - - -def test_can_letter_job_be_cancelled_returns_false_and_error_message_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 22:32'): - result, errors = can_letter_job_be_cancelled(job) - assert not result - assert errors == "It’s too late to cancel sending, these letters have already been sent." - assert letter.status == 'created' - assert job.job_status == 'finished' - - -@freeze_time('2019-06-13 13:00') -def test_can_letter_job_be_cancelled_returns_false_and_error_message_if_not_a_letter_job( - sample_template -): - job = create_job(template=sample_template, notification_count=1, job_status='finished') - create_notification(template=job.template, job=job, status='created') - result, errors = can_letter_job_be_cancelled(job) - assert not result - assert errors == "Only letter jobs can be cancelled through this endpoint. This is not a letter job." - - -@freeze_time('2019-06-13 13:00') -def test_can_letter_job_be_cancelled_returns_false_and_error_message_if_job_not_finished( - sample_letter_template -): - job = create_job(template=sample_letter_template, notification_count=1, job_status="in progress") - create_notification(template=job.template, job=job, status='created') - result, errors = can_letter_job_be_cancelled(job) - assert not result - assert errors == "We are still processing these letters, please try again in a minute." - - -@freeze_time('2019-06-13 13:00') -def test_can_letter_job_be_cancelled_returns_false_and_error_message_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') - result, errors = can_letter_job_be_cancelled(job) - assert not result - assert errors == "We are still processing these letters, please try again in a minute." - - -def test_can_letter_job_be_cancelled_respects_bst(sample_letter_template): - job = create_job(template=sample_letter_template, created_at=datetime(2020, 4, 9, 23, 30), job_status='finished') - create_notification(template=job.template, job=job, status='created', created_at=datetime(2020, 4, 9, 23, 32)) - - with freeze_time('2020-04-10 10:00'): - result, errors = can_letter_job_be_cancelled(job) - - assert not errors - assert result - - def test_find_jobs_with_missing_rows(sample_email_template): healthy_job = create_job(template=sample_email_template, notification_count=3,