diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 1cf508b31..d29dbcc8e 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -13,7 +13,7 @@ from app.celery.tasks import process_job from app.config import QueueNames, TaskNames from app.dao.invited_org_user_dao import delete_org_invitations_created_more_than_two_days_ago from app.dao.invited_user_dao import delete_invitations_created_more_than_two_days_ago -from app.dao.jobs_dao import dao_set_scheduled_jobs_to_pending +from app.dao.jobs_dao import dao_set_scheduled_jobs_to_pending, find_jobs_with_missing_rows, find_missing_row_for_job from app.dao.jobs_dao import dao_update_job from app.dao.notifications_dao import ( is_delivery_slow_for_provider, @@ -222,3 +222,27 @@ def check_templated_letter_state(): message=msg, ticket_type=zendesk_client.TYPE_INCIDENT ) + +@notify_celery.task(name='find-missing-rows-from-completed-jobs') +def find_missing_rows_from_completed_jobs(): + jobs_and_job_size = find_jobs_with_missing_rows() + for x in jobs_and_job_size: + missing_rows = find_missing_row_for_job(jobs_and_job_size.job_id, jobs_and_job_size.notification_count) + + # job = dao_get_job_by_id(job_id) + # db_template = dao_get_template_by_id(job.template_id, job.template_version) + # + # TemplateClass = get_template_class(db_template.template_type) + # template = TemplateClass(db_template.__dict__) + # + # for row in RecipientCSV( + # s3.get_job_from_s3(str(job.service_id), str(job.id)), + # template_type=template.template_type, + # placeholders=template.placeholders + # ).get_rows(): + # if row.index == job_row_number: + # notification_id = process_row(row, template, job, job.service) + # current_app.logger.info("Process row {} for job {} created notification_id: {}".format( + # job_row_number, job_id, notification_id)) + + pass diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index 4427129a4..b84b56a68 100644 --- a/app/dao/jobs_dao.py +++ b/app/dao/jobs_dao.py @@ -8,6 +8,7 @@ from sqlalchemy import ( asc, desc, func, + and_ ) from app import db @@ -187,3 +188,38 @@ def can_letter_job_be_cancelled(job): 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_with_rows_missing = db.session.query( + func.count(Notification.id).label('count_notifications'), + Job.notification_count, + Job.id.label('job_id'), + Job.service_id + ).filter( + Job.job_status == JOB_STATUS_FINISHED + ).group_by( + Job.notification_count, + Job.id.label('job_id'), + Job.service_id + ).having( + func.count(Notification.id) != Job.notification_count + ) + + return jobs_with_rows_missing.all() + + +def find_missing_row_for_job(job_id, job_size): + expected_row_numbers = db.session.query( + func.generate_series(0, job_size-1).label('row') + ).subquery() + + query = db.session.query( + Notification.job_row_number, + expected_row_numbers.c.row.label('missing_row') + ).outerjoin( + Notification, and_(expected_row_numbers.c.row == Notification.job_row_number, Notification.job_id == job_id) + ).filter( + Notification.job_row_number == None #noqa + ) + return query.all() diff --git a/migrations/versions/0308_delete_loadtesting_provider.py b/migrations/versions/0308_delete_loadtesting_provider.py new file mode 100644 index 000000000..55daf9ce1 --- /dev/null +++ b/migrations/versions/0308_delete_loadtesting_provider.py @@ -0,0 +1,39 @@ +""" +Remove loadtesting provider + +Revision ID: 0308_delete_loadtesting_provider +Revises: 0307_delete_dm_datetime +Create Date: 2019-10-22 17:30 + +""" +import uuid +from alembic import op +from sqlalchemy.sql import text + +revision = '0308_delete_loadtesting_provider' +down_revision = '0307_delete_dm_datetime' + + +def upgrade(): + conn = op.get_bind() + conn.execute("DELETE FROM provider_details WHERE identifier = 'loadtesting'") + conn.execute("DELETE FROM provider_details_history WHERE identifier = 'loadtesting'") + + +def downgrade(): + conn = op.get_bind() + conn.execute( + text(""" + INSERT INTO + provider_details + (id, display_name, identifier, priority, notification_type, active, version, supports_international) + VALUES + (:uuid, 'Loadtesting', 'loadtesting', 100, 'sms', true, 1, false); + INSERT INTO + provider_details_history + (id, display_name, identifier, priority, notification_type, active, version, supports_international) + VALUES + (:uuid, 'Loadtesting', 'loadtesting', 100, 'sms', true, 1, false) + """), + uuid=uuid.uuid4() + ) diff --git a/tests/app/dao/test_jobs_dao.py b/tests/app/dao/test_jobs_dao.py index 3b533b263..3d32f9174 100644 --- a/tests/app/dao/test_jobs_dao.py +++ b/tests/app/dao/test_jobs_dao.py @@ -16,10 +16,13 @@ from app.dao.jobs_dao import ( dao_get_notification_outcomes_for_job, dao_set_scheduled_jobs_to_pending, dao_update_job, + find_jobs_with_missing_rows, + find_missing_row_for_job ) from app.models import ( Job, - EMAIL_TYPE, SMS_TYPE, LETTER_TYPE + EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, + JOB_STATUS_FINISHED ) from tests.app.db import create_job, create_service, create_template, create_notification @@ -409,3 +412,61 @@ def test_can_letter_job_be_cancelled_returns_false_and_error_message_if_notifica 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_find_jobs_with_missing_rows(sample_email_template): + job = create_job(template=sample_email_template, notification_count=5, job_status=JOB_STATUS_FINISHED) + for i in range(0, 4): + create_notification(job=job, job_row_number=i) + + results = find_jobs_with_missing_rows() + + assert len(results) == 1 + assert results[0] == (4, 4, job.id, job.service_id) + + +@pytest.mark.parametrize('status', ['pending', 'in progress', 'cancelled', 'scheduled']) +def test_find_jobs_with_missing_rows_doesnt_return_jobs_that_are_not_finished( + sample_email_template, status +): + job = create_job(template=sample_email_template, notification_count=5, job_status=status) + for i in range(0, 4): + create_notification(job=job, job_row_number=i) + + results = find_jobs_with_missing_rows() + + assert len(results) == 0 + + +def test_find_missing_row_for_job(sample_email_template): + job = create_job(template=sample_email_template, notification_count=5, job_status=JOB_STATUS_FINISHED) + create_notification(job=job, job_row_number=0) + create_notification(job=job, job_row_number=1) + create_notification(job=job, job_row_number=3) + create_notification(job=job, job_row_number=4) + + results = find_missing_row_for_job(job.id, 5) + assert len(results) == 1 + assert results[0].missing_row == 2 + + +def test_find_missing_row_for_job_more_than_one_missing_row(sample_email_template): + job = create_job(template=sample_email_template, notification_count=5, job_status=JOB_STATUS_FINISHED) + create_notification(job=job, job_row_number=0) + create_notification(job=job, job_row_number=1) + create_notification(job=job, job_row_number=4) + + results = find_missing_row_for_job(job.id, 5) + assert len(results) == 2 + assert results[0].missing_row == 2 + assert results[1].missing_row == 3 + + +def test_find_missing_row_for_job_return_none_when_row_isnt_missing(sample_email_template): + job = create_job(template=sample_email_template, notification_count=5, job_status=JOB_STATUS_FINISHED) + for i in range(0, 5): + create_notification(job=job, job_row_number=i) + + results = find_missing_row_for_job(job.id, 5) + print(results) + assert len(results) == 0 diff --git a/tests/app/dao/test_provider_details_dao.py b/tests/app/dao/test_provider_details_dao.py index 153f39a2e..20b662d36 100644 --- a/tests/app/dao/test_provider_details_dao.py +++ b/tests/app/dao/test_provider_details_dao.py @@ -38,7 +38,7 @@ def set_primary_sms_provider(identifier): def test_can_get_sms_non_international_providers(restore_provider_details): sms_providers = get_provider_details_by_notification_type('sms') - assert len(sms_providers) == 3 + assert len(sms_providers) == 2 assert all('sms' == prov.notification_type for prov in sms_providers) @@ -307,7 +307,7 @@ def test_dao_get_provider_stats(notify_db_session): result = dao_get_provider_stats() - assert len(result) == 5 + assert len(result) == 4 assert result[0].identifier == 'ses' assert result[0].display_name == 'AWS SES' @@ -326,9 +326,6 @@ def test_dao_get_provider_stats(notify_db_session): assert result[2].active is True assert result[2].current_month_billable_sms == 5 - assert result[3].identifier == 'loadtesting' + assert result[3].identifier == 'dvla' assert result[3].current_month_billable_sms == 0 - - assert result[4].identifier == 'dvla' - assert result[4].current_month_billable_sms == 0 - assert result[4].supports_international is False + assert result[3].supports_international is False diff --git a/tests/app/provider_details/test_rest.py b/tests/app/provider_details/test_rest.py index fd9428e47..b096d8daf 100644 --- a/tests/app/provider_details/test_rest.py +++ b/tests/app/provider_details/test_rest.py @@ -15,13 +15,12 @@ def test_get_provider_details_in_type_and_identifier_order(client, notify_db): ) assert response.status_code == 200 json_resp = json.loads(response.get_data(as_text=True))['provider_details'] - assert len(json_resp) == 5 + assert len(json_resp) == 4 assert json_resp[0]['identifier'] == 'ses' assert json_resp[1]['identifier'] == 'mmg' assert json_resp[2]['identifier'] == 'firetext' - assert json_resp[3]['identifier'] == 'loadtesting' - assert json_resp[4]['identifier'] == 'dvla' + assert json_resp[3]['identifier'] == 'dvla' def test_get_provider_details_by_id(client, notify_db): @@ -55,7 +54,7 @@ def test_get_provider_contains_correct_fields(client, sample_service, sample_tem "active", "updated_at", "supports_international", "current_month_billable_sms" } - assert len(json_resp) == 5 + assert len(json_resp) == 4 assert allowed_keys == set(json_resp[0].keys())