From 3f9a6e9e105fbea365523f6630698209482043e0 Mon Sep 17 00:00:00 2001 From: Evan Parish <104009494+EvanParish@users.noreply.github.com> Date: Wed, 8 Jan 2025 15:12:37 -0600 Subject: [PATCH] #2191 - Remove Technical-Failure from Stats --- .talismanrc | 4 + app/dao/fact_notification_status_dao.py | 13 -- app/service/statistics.py | 6 +- tests/app/celery/test_tasks.py | 7 +- .../dao/test_fact_notification_status_dao.py | 10 +- tests/app/dao/test_ft_billing_dao.py | 86 ++++---- tests/app/dao/test_services_dao.py | 24 +-- .../test_process_notifications.py | 2 - tests/app/platform_stats/test_rest.py | 3 - .../schemas/v0/email_notification.json | 1 - .../schemas/v0/sms_notification.json | 1 - tests/app/service/test_callback_rest.py | 9 +- tests/app/service/test_statistics.py | 183 +++++++++++------- .../notifications/test_get_notifications.py | 1 - 14 files changed, 189 insertions(+), 161 deletions(-) diff --git a/.talismanrc b/.talismanrc index c124f84ec2..8b6a8e4141 100644 --- a/.talismanrc +++ b/.talismanrc @@ -5,6 +5,10 @@ fileignoreconfig: checksum: 128bde997e5a0f41e6bac5a5dfe3180f93b33bca138d4b4eb64443af7b1b15bd - filename: tests/app/dao/test_api_key_dao.py checksum: 5bab4eaddf8760c502111ae3e5f9f8bee59482d99f053f94598e8c77bd10b7b6 +- filename: tests/app/dao/test_ft_billing_dao.py + checksum: dc425af859e49b9822259fad20d0ff99f3fde5c0237728fe73e5c9db2b78591b +- filename: tests/app/dao/test_services_dao.py + checksum: 572fb3a52e89ae19c5a94be881bcbfe0247bae112bce58d4ae1e9eff5a5c91b7 - filename: tests/app/v2/notifications/test_get_notifications.py checksum: 167aa80c8aac01566b3cf627cdaa839feac9dc77f0b13b6d0a008035aa8e15f1 version: "1.0" diff --git a/app/dao/fact_notification_status_dao.py b/app/dao/fact_notification_status_dao.py index 16c24cb25e..aba4b1335b 100644 --- a/app/dao/fact_notification_status_dao.py +++ b/app/dao/fact_notification_status_dao.py @@ -16,7 +16,6 @@ NOTIFICATION_CANCELLED, NOTIFICATION_CREATED, NOTIFICATION_DELIVERED, - NOTIFICATION_FAILED, NOTIFICATION_SENDING, NOTIFICATION_SENT, NOTIFICATION_TEMPORARY_FAILURE, @@ -815,18 +814,6 @@ def fetch_monthly_notification_statuses_per_service( else_=0, ) ).label('count_delivered'), - # TODO 2191 - remove this after technical-failure is removed from the codebase - func.sum( - case( - [ - ( - FactNotificationStatus.notification_status.in_(['technical-failure', NOTIFICATION_FAILED]), - FactNotificationStatus.notification_count, - ) - ], - else_=0, - ) - ).label('count_technical_failure'), func.sum( case( [ diff --git a/app/service/statistics.py b/app/service/statistics.py index 1bd21603fb..120c660669 100644 --- a/app/service/statistics.py +++ b/app/service/statistics.py @@ -29,8 +29,7 @@ def format_admin_stats(statistics): counts[row.notification_type]['test-key'] += row.count else: counts[row.notification_type]['total'] += row.count - # TODO 2191 - remove this after technical-failure is removed from the codebase - if row.status in ('technical-failure', 'permanent-failure', 'temporary-failure', 'virus-scan-failed'): + if row.status in ('permanent-failure', 'temporary-failure', 'virus-scan-failed'): counts[row.notification_type]['failures'][row.status] += row.count return counts @@ -45,8 +44,6 @@ def create_stats_dict(): stats_dict[template][status] = 0 stats_dict[template]['failures'] = { - # TODO 2191 - remove this after technical-failure is removed from the codebase - 'technical-failure': 0, 'permanent-failure': 0, 'temporary-failure': 0, 'virus-scan-failed': 0, @@ -94,7 +91,6 @@ def _update_statuses_from_row( update_dict['delivered'] += row.count elif row.status in ( 'failed', - 'technical-failure', 'temporary-failure', 'permanent-failure', 'validation-failed', diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 894f9a91f7..06829c104a 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -531,7 +531,6 @@ def test_should_put_save_sms_task_in_research_mode_queue_if_research_mode_servic assert mocked_deliver_sms.called -@pytest.mark.serial def test_should_save_sms_if_restricted_service_and_valid_number( notify_db_session, mocker, @@ -551,7 +550,6 @@ def test_should_save_sms_if_restricted_service_and_valid_number( notification_id = uuid4() encrypt_notification = encryption.encrypt(notification) - # Intermittently makes the status 'technical-failure' save_sms( service.id, notification_id, @@ -878,7 +876,6 @@ def test_should_use_email_template_and_persist( ) -@pytest.mark.serial def test_save_email_should_use_template_version_from_job_not_latest( notify_db_session, sample_template, @@ -902,7 +899,6 @@ def test_save_email_should_use_template_version_from_job_not_latest( notification_id = uuid4() - # Intermittently makes the status 'technical-failure' save_email( template.service_id, notification_id, @@ -926,7 +922,6 @@ def test_save_email_should_use_template_version_from_job_not_latest( ) -@pytest.mark.serial def test_should_use_email_template_subject_placeholders( notify_db_session, sample_template, @@ -944,7 +939,7 @@ def test_should_use_email_template_subject_placeholders( notification_id = uuid4() now = datetime.utcnow() - # Intermittently makes the status 'technical-failure' + save_email( template.service_id, notification_id, diff --git a/tests/app/dao/test_fact_notification_status_dao.py b/tests/app/dao/test_fact_notification_status_dao.py index 618352a233..fda0a6be63 100644 --- a/tests/app/dao/test_fact_notification_status_dao.py +++ b/tests/app/dao/test_fact_notification_status_dao.py @@ -1063,8 +1063,8 @@ def test_fetch_monthly_notification_statuses_per_service( assert len(results) == 6 # column order: date, service_id, service_name, notifaction_type, count_sending, count_delivered, - # count_technical_failure, count_temporary_failure, count_permanent_failure, count_sent - assert [x for x in results[0]] == [date(2019, 3, 1), service_one.id, service_one.name, EMAIL_TYPE, 4, 0, 0, 0, 3, 0] + # count_temporary_failure, count_permanent_failure, count_sent + assert [x for x in results[0]] == [date(2019, 3, 1), service_one.id, service_one.name, EMAIL_TYPE, 4, 0, 0, 3, 0] assert [x for x in results[1]] == [ date(2019, 3, 1), service_one.id, @@ -1075,9 +1075,8 @@ def test_fetch_monthly_notification_statuses_per_service( 0, 0, 0, - 0, ] - assert [x for x in results[2]] == [date(2019, 3, 1), service_one.id, service_one.name, SMS_TYPE, 0, 0, 0, 0, 0, 1] + assert [x for x in results[2]] == [date(2019, 3, 1), service_one.id, service_one.name, SMS_TYPE, 0, 0, 0, 0, 1] assert [x for x in results[3]] == [ date(2019, 3, 1), service_two.id, @@ -1086,7 +1085,6 @@ def test_fetch_monthly_notification_statuses_per_service( 0, 0, 0, - 0, 2, 0, ] @@ -1100,7 +1098,6 @@ def test_fetch_monthly_notification_statuses_per_service( 0, 0, 0, - 0, ] assert [x for x in results[5]] == [ date(2019, 4, 1), @@ -1109,7 +1106,6 @@ def test_fetch_monthly_notification_statuses_per_service( LETTER_TYPE, 0, 0, - 0, 10, 0, 0, diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index 9cbbf97727..3e2b7e64f7 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -15,7 +15,12 @@ KEY_TYPE_TEAM, KEY_TYPE_TEST, LETTER_TYPE, + NOTIFICATION_CREATED, + NOTIFICATION_DELIVERED, + NOTIFICATION_PENDING, + NOTIFICATION_SENDING, NOTIFICATION_STATUS_TYPES, + NOTIFICATION_TEMPORARY_FAILURE, SMS_TYPE, ) from app.dao.fact_billing_dao import ( @@ -89,14 +94,15 @@ def test_fetch_billing_data_for_today_includes_data_with_the_right_status( ): service = sample_service() template = sample_template(service=service, template_type=EMAIL_TYPE) - sample_notification(template=template, status='created') - sample_notification(template=template, status='technical-failure') + # these statuses need to be ones that are not in NOTIFICATION_STATUS_TYPES_BILLABLE + sample_notification(template=template, status=NOTIFICATION_CREATED) + sample_notification(template=template, status=NOTIFICATION_PENDING) today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(today, service.id) assert isinstance(results, list) and not results, 'Should be an empty list' - for status in ('delivered', 'sending', 'temporary-failure'): + for status in (NOTIFICATION_DELIVERED, NOTIFICATION_SENDING, NOTIFICATION_TEMPORARY_FAILURE): sample_notification(template=template, status=status) results = fetch_billing_data_for_day(today, service.id) @@ -114,7 +120,7 @@ def test_fetch_billing_data_for_today_includes_data_with_the_right_key_type( template = sample_template(service=service, template_type=EMAIL_TYPE) for key_type in (KEY_TYPE_NORMAL, KEY_TYPE_TEST, KEY_TYPE_TEAM): - sample_notification(template=template, status='delivered', key_type=key_type) + sample_notification(template=template, status=NOTIFICATION_DELIVERED, key_type=key_type) today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(today, service.id) @@ -132,11 +138,11 @@ def test_fetch_billing_data_for_today_includes_data_with_the_right_date( process_day = datetime(2018, 4, 1, 13, 30, 0) service = sample_service() template = sample_template(service=service, template_type=EMAIL_TYPE) - sample_notification(template=template, status='delivered', created_at=process_day) - sample_notification(template=template, status='delivered', created_at=datetime(2018, 4, 1, 4, 23, 23)) + sample_notification(template=template, status=NOTIFICATION_DELIVERED, created_at=process_day) + sample_notification(template=template, status=NOTIFICATION_DELIVERED, created_at=datetime(2018, 4, 1, 4, 23, 23)) - sample_notification(template=template, status='delivered', created_at=datetime(2018, 3, 31, 20, 23, 23)) - sample_notification(template=template, status='sending', created_at=process_day + timedelta(days=1)) + sample_notification(template=template, status=NOTIFICATION_DELIVERED, created_at=datetime(2018, 3, 31, 20, 23, 23)) + sample_notification(template=template, status=NOTIFICATION_SENDING, created_at=process_day + timedelta(days=1)) day_under_test = convert_utc_to_local_timezone(process_day) results = fetch_billing_data_for_day(day_under_test, service.id) @@ -162,7 +168,7 @@ def test_fetch_nightly_billing_counts_retrieves_correct_data_within_process_day( # Create 3 SMS notifications for the given process date. sample_notification( template=template2, - status='delivered', + status=NOTIFICATION_DELIVERED, created_at=process_day, billing_code='test_code', sms_sender_id=service.service_sms_senders[0].id, @@ -171,7 +177,7 @@ def test_fetch_nightly_billing_counts_retrieves_correct_data_within_process_day( ) sample_notification( template=template1, - status='delivered', + status=NOTIFICATION_DELIVERED, created_at=process_day, sms_sender_id=service.service_sms_senders[0].id, segments_count=5, @@ -179,7 +185,7 @@ def test_fetch_nightly_billing_counts_retrieves_correct_data_within_process_day( ) sample_notification( template=template1, - status='delivered', + status=NOTIFICATION_DELIVERED, created_at=datetime(2018, 4, 1, 4, 23, 23), sms_sender_id=service.service_sms_senders[0].id, segments_count=4, @@ -189,7 +195,7 @@ def test_fetch_nightly_billing_counts_retrieves_correct_data_within_process_day( # Create 2 SMS notifications not for the given process date. sample_notification( template=template1, - status='delivered', + status=NOTIFICATION_DELIVERED, created_at=datetime(2018, 3, 31, 20, 23, 23), sms_sender_id=service.service_sms_senders[0].id, segments_count=1, @@ -197,7 +203,7 @@ def test_fetch_nightly_billing_counts_retrieves_correct_data_within_process_day( ) sample_notification( template=template1, - status='sending', + status=NOTIFICATION_SENDING, created_at=process_day + timedelta(days=1), sms_sender_id=service.service_sms_senders[0].id, segments_count=1, @@ -232,8 +238,8 @@ def test_fetch_billing_data_for_day_is_grouped_by_template_and_notification_type service = sample_service() email_template = sample_template(service=service, template_type=EMAIL_TYPE) sms_template = sample_template(service=service, template_type=SMS_TYPE) - sample_notification(template=email_template, status='delivered') - sample_notification(template=sms_template, status='delivered') + sample_notification(template=email_template, status=NOTIFICATION_DELIVERED) + sample_notification(template=sms_template, status=NOTIFICATION_DELIVERED) today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(today, service.id) @@ -249,11 +255,11 @@ def test_fetch_billing_data_for_day_is_grouped_by_service( ): service_1 = sample_service() email_template = sample_template(service=service_1) - sample_notification(template=email_template, status='delivered') + sample_notification(template=email_template, status=NOTIFICATION_DELIVERED) service_2 = sample_service() sms_template = sample_template(service=service_2) - sample_notification(template=sms_template, status='delivered') + sample_notification(template=sms_template, status=NOTIFICATION_DELIVERED) today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(today, (service_1.id, service_2.id)) @@ -270,8 +276,8 @@ def test_fetch_billing_data_for_day_is_grouped_by_provider( ): service = sample_service() template = sample_template(service=service) - sample_notification(template=template, status='delivered', sent_by='mmg') - sample_notification(template=template, status='delivered', sent_by='firetext') + sample_notification(template=template, status=NOTIFICATION_DELIVERED, sent_by='mmg') + sample_notification(template=template, status=NOTIFICATION_DELIVERED, sent_by='firetext') today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(today, service.id) @@ -289,8 +295,8 @@ def test_fetch_billing_data_for_day_is_grouped_by_rate_mulitplier( ): service = sample_service() template = sample_template(service=service) - sample_notification(template=template, status='delivered', rate_multiplier=1) - sample_notification(template=template, status='delivered', rate_multiplier=2) + sample_notification(template=template, status=NOTIFICATION_DELIVERED, rate_multiplier=1) + sample_notification(template=template, status=NOTIFICATION_DELIVERED, rate_multiplier=2) today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(today, service.id) @@ -307,8 +313,8 @@ def test_fetch_billing_data_for_day_is_grouped_by_international( ): service = sample_service() template = sample_template(service=service) - sample_notification(template=template, status='delivered', international=True) - sample_notification(template=template, status='delivered', international=False) + sample_notification(template=template, status=NOTIFICATION_DELIVERED, international=True) + sample_notification(template=template, status=NOTIFICATION_DELIVERED, international=False) today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(today, service.id) @@ -326,12 +332,12 @@ def test_fetch_billing_data_for_day_is_grouped_by_notification_type( sms_template = sample_template(service=service, template_type=SMS_TYPE) email_template = sample_template(service=service, template_type=EMAIL_TYPE) letter_template = sample_template(service=service, template_type=LETTER_TYPE) - sample_notification(template=sms_template, status='delivered') - sample_notification(template=sms_template, status='delivered') - sample_notification(template=sms_template, status='delivered') - sample_notification(template=email_template, status='delivered') - sample_notification(template=email_template, status='delivered') - sample_notification(template=letter_template, status='delivered') + sample_notification(template=sms_template, status=NOTIFICATION_DELIVERED) + sample_notification(template=sms_template, status=NOTIFICATION_DELIVERED) + sample_notification(template=sms_template, status=NOTIFICATION_DELIVERED) + sample_notification(template=email_template, status=NOTIFICATION_DELIVERED) + sample_notification(template=email_template, status=NOTIFICATION_DELIVERED) + sample_notification(template=letter_template, status=NOTIFICATION_DELIVERED) today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(today, service.id) @@ -348,10 +354,10 @@ def test_fetch_billing_data_for_day_groups_by_postage( service = sample_service() letter_template = sample_template(service=service, template_type=LETTER_TYPE) email_template = sample_template(service=service, template_type=EMAIL_TYPE) - sample_notification(template=letter_template, status='delivered', postage='first') - sample_notification(template=letter_template, status='delivered', postage='first') - sample_notification(template=letter_template, status='delivered', postage='second') - sample_notification(template=email_template, status='delivered') + sample_notification(template=letter_template, status=NOTIFICATION_DELIVERED, postage='first') + sample_notification(template=letter_template, status=NOTIFICATION_DELIVERED, postage='first') + sample_notification(template=letter_template, status=NOTIFICATION_DELIVERED, postage='second') + sample_notification(template=email_template, status=NOTIFICATION_DELIVERED) today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(today, service.id) @@ -366,8 +372,8 @@ def test_fetch_billing_data_for_day_sets_postage_for_emails_and_sms_to_none( service = sample_service() sms_template = sample_template(service=service, template_type=SMS_TYPE) email_template = sample_template(service=service, template_type=EMAIL_TYPE) - sample_notification(template=sms_template, status='delivered') - sample_notification(template=email_template, status='delivered') + sample_notification(template=sms_template, status=NOTIFICATION_DELIVERED) + sample_notification(template=email_template, status=NOTIFICATION_DELIVERED) today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(today, service.id) @@ -392,10 +398,10 @@ def test_fetch_billing_data_for_day_uses_notification_history( service = sample_service() sms_template = sample_template(service=service, template_type=SMS_TYPE) sample_notification_history( - template=sms_template, status='delivered', created_at=datetime.utcnow() - timedelta(days=8) + template=sms_template, status=NOTIFICATION_DELIVERED, created_at=datetime.utcnow() - timedelta(days=8) ) sample_notification_history( - template=sms_template, status='delivered', created_at=datetime.utcnow() - timedelta(days=8) + template=sms_template, status=NOTIFICATION_DELIVERED, created_at=datetime.utcnow() - timedelta(days=8) ) results = fetch_billing_data_for_day( @@ -414,8 +420,8 @@ def test_fetch_billing_data_for_day_returns_list_for_given_service( service_2 = sample_service() template = sample_template(service=service) template_2 = sample_template(service=service_2) - sample_notification(template=template, status='delivered') - sample_notification(template=template_2, status='delivered') + sample_notification(template=template, status=NOTIFICATION_DELIVERED) + sample_notification(template=template_2, status=NOTIFICATION_DELIVERED) today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(today, service.id) @@ -573,7 +579,7 @@ def test_fetch_monthly_billing_for_year_adds_data_for_today( notification_type=EMAIL_TYPE, rate=0.162, ) - sample_notification(template=template, status='delivered') + sample_notification(template=template, status=NOTIFICATION_DELIVERED) stmt = select(func.count()).select_from(FactBilling).where(FactBilling.service_id == service.id) diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index c63a610982..637068fa04 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -17,6 +17,8 @@ KEY_TYPE_TEAM, KEY_TYPE_TEST, LETTER_TYPE, + NOTIFICATION_CREATED, + NOTIFICATION_PERMANENT_FAILURE, SES_PROVIDER, SMS_TYPE, ) @@ -78,7 +80,7 @@ def service_status_mappings(stats: list) -> dict: """ Takes a stats list from the `dao_fetch_todays_stats_for_all_services` query and maps status counts per service - {'service one': {'created': X, 'sent': Y, 'permanent-failure': Z}, 'service two': {'created': A, 'delivered': B} + {'service one': {NOTIFICATION_CREATED: X, 'sent': Y, 'permanent-failure': Z}, 'service two': {NOTIFICATION_CREATED: A, 'delivered': B} """ status_count_mapping = {} for row in stats: @@ -1093,25 +1095,25 @@ def test_fetch_stats_counts_correctly( sms_template = sample_template(service=service) email_template = sample_template(service=service, template_type=EMAIL_TYPE) # two created email, one failed email, and one created sms - sample_notification(template=email_template, status='created', api_key=api_key) - sample_notification(template=email_template, status='created', api_key=api_key) - sample_notification(template=email_template, status='technical-failure', api_key=api_key) - sample_notification(template=sms_template, status='created', api_key=api_key) + sample_notification(template=email_template, status=NOTIFICATION_CREATED, api_key=api_key) + sample_notification(template=email_template, status=NOTIFICATION_CREATED, api_key=api_key) + sample_notification(template=email_template, status=NOTIFICATION_PERMANENT_FAILURE, api_key=api_key) + sample_notification(template=sms_template, status=NOTIFICATION_CREATED, api_key=api_key) stats = dao_fetch_stats_for_service(sms_template.service_id, 7) stats = sorted(stats, key=lambda x: (x.notification_type, x.status)) assert len(stats) == 3 assert stats[0].notification_type == EMAIL_TYPE - assert stats[0].status == 'created' + assert stats[0].status == NOTIFICATION_CREATED assert stats[0].count == 2 assert stats[1].notification_type == EMAIL_TYPE - assert stats[1].status == 'technical-failure' + assert stats[1].status == NOTIFICATION_PERMANENT_FAILURE assert stats[1].count == 1 assert stats[2].notification_type == SMS_TYPE - assert stats[2].status == 'created' + assert stats[2].status == NOTIFICATION_CREATED assert stats[2].count == 1 @@ -1137,7 +1139,7 @@ def test_fetch_stats_counts_should_ignore_team_key( stats = dao_fetch_stats_for_service(template.service_id, 7) assert len(stats) == 1 assert stats[0].notification_type == SMS_TYPE - assert stats[0].status == 'created' + assert stats[0].status == NOTIFICATION_CREATED assert stats[0].count == 3 @@ -1159,14 +1161,14 @@ def test_fetch_stats_for_today_only_includes_today( with freeze_time('2001-01-02T12:00:00'): # right_now - sample_notification(template=template, to_field='3', status='created') + sample_notification(template=template, to_field='3', status=NOTIFICATION_CREATED) stats = dao_fetch_todays_stats_for_service(template.service_id) stats = {row.status: row.count for row in stats} assert 'delivered' not in stats assert stats['failed'] == 1 - assert stats['created'] == 1 + assert stats[NOTIFICATION_CREATED] == 1 @pytest.mark.parametrize( diff --git a/tests/app/notifications/test_process_notifications.py b/tests/app/notifications/test_process_notifications.py index e494f1acef..2905229b42 100644 --- a/tests/app/notifications/test_process_notifications.py +++ b/tests/app/notifications/test_process_notifications.py @@ -86,7 +86,6 @@ def test_create_content_for_notification_allows_additional_personalisation( create_content_for_notification(db_template, {'name': 'Bobby', 'Additional placeholder': 'Data'}) -@pytest.mark.serial @freeze_time('2016-01-01 11:09:00.061258') def test_persist_notification_creates_and_save_to_db( notify_db_session, @@ -115,7 +114,6 @@ def test_persist_notification_creates_and_save_to_db( 'personalisation': {}, } - # Intermittently makes the status 'technical-failure' # Cleaned by the template cleanup persist_notification(**data) diff --git a/tests/app/platform_stats/test_rest.py b/tests/app/platform_stats/test_rest.py index 5e3ac53acc..f89593e949 100644 --- a/tests/app/platform_stats/test_rest.py +++ b/tests/app/platform_stats/test_rest.py @@ -70,7 +70,6 @@ def test_get_platform_stats_with_real_query( 'virus-scan-failed': 0, 'temporary-failure': 0, 'permanent-failure': 0, - 'technical-failure': 0, }, 'total': 4, 'test-key': 0, @@ -80,7 +79,6 @@ def test_get_platform_stats_with_real_query( 'virus-scan-failed': 0, 'temporary-failure': 0, 'permanent-failure': 0, - 'technical-failure': 0, }, 'total': 0, 'test-key': 0, @@ -90,7 +88,6 @@ def test_get_platform_stats_with_real_query( 'virus-scan-failed': 0, 'temporary-failure': 0, 'permanent-failure': 0, - 'technical-failure': 0, }, 'total': 11, 'test-key': 1, diff --git a/tests/app/public_contracts/schemas/v0/email_notification.json b/tests/app/public_contracts/schemas/v0/email_notification.json index 1bc1f54977..3e7577a3c9 100644 --- a/tests/app/public_contracts/schemas/v0/email_notification.json +++ b/tests/app/public_contracts/schemas/v0/email_notification.json @@ -35,7 +35,6 @@ "delivered", "pending", "failed", - "technical-failure", "temporary-failure", "permanent-failure" ] diff --git a/tests/app/public_contracts/schemas/v0/sms_notification.json b/tests/app/public_contracts/schemas/v0/sms_notification.json index 588903add9..d94eb47aad 100644 --- a/tests/app/public_contracts/schemas/v0/sms_notification.json +++ b/tests/app/public_contracts/schemas/v0/sms_notification.json @@ -35,7 +35,6 @@ "delivered", "pending", "failed", - "technical-failure", "temporary-failure", "permanent-failure" ] diff --git a/tests/app/service/test_callback_rest.py b/tests/app/service/test_callback_rest.py index 9e94d78750..0fc55c1019 100644 --- a/tests/app/service/test_callback_rest.py +++ b/tests/app/service/test_callback_rest.py @@ -13,6 +13,7 @@ DELIVERY_STATUS_CALLBACK_TYPE, INBOUND_SMS_CALLBACK_TYPE, MANAGE_SETTINGS, + NOTIFICATION_PERMANENT_FAILURE, NOTIFICATION_STATUS_TYPES_COMPLETED, QUEUE_CHANNEL_TYPE, WEBHOOK_CHANNEL_TYPE, @@ -553,7 +554,9 @@ def test_update_service_callback_raises_400_when_wrong_request( request_data, ): service = sample_service() - service_callback_api = sample_service_callback(service=service, notification_statuses=['technical-failure']) + service_callback_api = sample_service_callback( + service=service, notification_statuses=[NOTIFICATION_PERMANENT_FAILURE] + ) response = client.post( url_for( @@ -579,7 +582,9 @@ def test_update_service_callback_raises_400_when_invalid_status( sample_service, ): service = sample_service() - service_callback_api = sample_service_callback(service=service, notification_statuses=['technical-failure']) + service_callback_api = sample_service_callback( + service=service, notification_statuses=[NOTIFICATION_PERMANENT_FAILURE] + ) data = { 'notification_statuses': ['nonexistent-status'], diff --git a/tests/app/service/test_statistics.py b/tests/app/service/test_statistics.py index 1f2bae4054..060629a6ca 100644 --- a/tests/app/service/test_statistics.py +++ b/tests/app/service/test_statistics.py @@ -5,6 +5,22 @@ from freezegun import freeze_time import pytest +from app.constants import ( + EMAIL_TYPE, + LETTER_TYPE, + NOTIFICATION_CANCELLED, + NOTIFICATION_CREATED, + NOTIFICATION_DELIVERED, + NOTIFICATION_FAILED, + NOTIFICATION_PENDING_VIRUS_CHECK, + NOTIFICATION_PERMANENT_FAILURE, + NOTIFICATION_SENDING, + NOTIFICATION_SENT, + NOTIFICATION_TEMPORARY_FAILURE, + NOTIFICATION_VALIDATION_FAILED, + NOTIFICATION_VIRUS_SCAN_FAILED, + SMS_TYPE, +) from app.service.statistics import ( format_admin_stats, format_statistics, @@ -24,27 +40,30 @@ { 'empty': ([], [0, 0, 0], [0, 0, 0], [0, 0, 0]), 'always_increment_requested': ( - [StatsRow('email', 'delivered', 1), StatsRow('email', 'failed', 1)], + [StatsRow(EMAIL_TYPE, NOTIFICATION_DELIVERED, 1), StatsRow(EMAIL_TYPE, NOTIFICATION_FAILED, 1)], [2, 1, 1], [0, 0, 0], [0, 0, 0], ), 'dont_mix_template_types': ( - [StatsRow('email', 'delivered', 1), StatsRow('sms', 'delivered', 1), StatsRow('letter', 'delivered', 1)], + [ + StatsRow(EMAIL_TYPE, NOTIFICATION_DELIVERED, 1), + StatsRow(SMS_TYPE, NOTIFICATION_DELIVERED, 1), + StatsRow(LETTER_TYPE, NOTIFICATION_DELIVERED, 1), + ], [1, 1, 0], [1, 1, 0], [1, 1, 0], ), 'convert_fail_statuses_to_failed': ( [ - StatsRow('email', 'failed', 1), - StatsRow('email', 'technical-failure', 1), - StatsRow('email', 'temporary-failure', 1), - StatsRow('email', 'permanent-failure', 1), - StatsRow('letter', 'validation-failed', 1), - StatsRow('letter', 'virus-scan-failed', 1), - StatsRow('letter', 'permanent-failure', 1), - StatsRow('letter', 'cancelled', 1), + StatsRow(EMAIL_TYPE, NOTIFICATION_FAILED, 1), + StatsRow(EMAIL_TYPE, NOTIFICATION_TEMPORARY_FAILURE, 1), + StatsRow(EMAIL_TYPE, NOTIFICATION_PERMANENT_FAILURE, 2), + StatsRow(LETTER_TYPE, NOTIFICATION_VALIDATION_FAILED, 1), + StatsRow(LETTER_TYPE, NOTIFICATION_VIRUS_SCAN_FAILED, 1), + StatsRow(LETTER_TYPE, NOTIFICATION_PERMANENT_FAILURE, 1), + StatsRow(LETTER_TYPE, NOTIFICATION_CANCELLED, 1), ], [4, 0, 4], [0, 0, 0], @@ -52,16 +71,16 @@ ), 'convert_sent_to_delivered': ( [ - StatsRow('sms', 'sending', 1), - StatsRow('sms', 'delivered', 1), - StatsRow('sms', 'sent', 1), + StatsRow(SMS_TYPE, NOTIFICATION_SENDING, 1), + StatsRow(SMS_TYPE, NOTIFICATION_DELIVERED, 1), + StatsRow(SMS_TYPE, NOTIFICATION_SENT, 1), ], [0, 0, 0], [3, 2, 0], [0, 0, 0], ), 'handles_none_rows': ( - [StatsRow('sms', 'sending', 1), StatsRow(None, None, None)], + [StatsRow(SMS_TYPE, NOTIFICATION_SENDING, 1), StatsRow(None, None, None)], [0, 0, 0], [1, 0, 0], [0, 0, 0], @@ -71,53 +90,55 @@ def test_format_statistics(stats, email_counts, sms_counts, letter_counts): ret = format_statistics(stats) - assert ret['email'] == {status: count for status, count in zip(['requested', 'delivered', 'failed'], email_counts)} + assert ret[EMAIL_TYPE] == { + status: count for status, count in zip(['requested', NOTIFICATION_DELIVERED, NOTIFICATION_FAILED], email_counts) + } - assert ret['sms'] == {status: count for status, count in zip(['requested', 'delivered', 'failed'], sms_counts)} + assert ret[SMS_TYPE] == { + status: count for status, count in zip(['requested', NOTIFICATION_DELIVERED, NOTIFICATION_FAILED], sms_counts) + } - assert ret['letter'] == { - status: count for status, count in zip(['requested', 'delivered', 'failed'], letter_counts) + assert ret[LETTER_TYPE] == { + status: count + for status, count in zip(['requested', NOTIFICATION_DELIVERED, NOTIFICATION_FAILED], letter_counts) } def test_create_zeroed_stats_dicts(): assert create_zeroed_stats_dicts() == { - 'sms': {'requested': 0, 'delivered': 0, 'failed': 0}, - 'email': {'requested': 0, 'delivered': 0, 'failed': 0}, - 'letter': {'requested': 0, 'delivered': 0, 'failed': 0}, + SMS_TYPE: {'requested': 0, NOTIFICATION_DELIVERED: 0, NOTIFICATION_FAILED: 0}, + EMAIL_TYPE: {'requested': 0, NOTIFICATION_DELIVERED: 0, NOTIFICATION_FAILED: 0}, + LETTER_TYPE: {'requested': 0, NOTIFICATION_DELIVERED: 0, NOTIFICATION_FAILED: 0}, } def test_create_stats_dict(): assert create_stats_dict() == { - 'sms': { + SMS_TYPE: { 'total': 0, 'test-key': 0, 'failures': { - 'technical-failure': 0, - 'permanent-failure': 0, - 'temporary-failure': 0, - 'virus-scan-failed': 0, + NOTIFICATION_PERMANENT_FAILURE: 0, + NOTIFICATION_TEMPORARY_FAILURE: 0, + NOTIFICATION_VIRUS_SCAN_FAILED: 0, }, }, - 'email': { + EMAIL_TYPE: { 'total': 0, 'test-key': 0, 'failures': { - 'technical-failure': 0, - 'permanent-failure': 0, - 'temporary-failure': 0, - 'virus-scan-failed': 0, + NOTIFICATION_PERMANENT_FAILURE: 0, + NOTIFICATION_TEMPORARY_FAILURE: 0, + NOTIFICATION_VIRUS_SCAN_FAILED: 0, }, }, - 'letter': { + LETTER_TYPE: { 'total': 0, 'test-key': 0, 'failures': { - 'technical-failure': 0, - 'permanent-failure': 0, - 'temporary-failure': 0, - 'virus-scan-failed': 0, + NOTIFICATION_PERMANENT_FAILURE: 0, + NOTIFICATION_TEMPORARY_FAILURE: 0, + NOTIFICATION_VIRUS_SCAN_FAILED: 0, }, }, } @@ -125,46 +146,46 @@ def test_create_stats_dict(): def test_format_admin_stats_only_includes_test_key_notifications_in_test_key_section(): rows = [ - NewStatsRow('email', 'technical-failure', 'test', 3), - NewStatsRow('sms', 'permanent-failure', 'test', 4), - NewStatsRow('letter', 'virus-scan-failed', 'test', 5), + NewStatsRow(EMAIL_TYPE, NOTIFICATION_PERMANENT_FAILURE, 'test', 3), + NewStatsRow(SMS_TYPE, NOTIFICATION_PERMANENT_FAILURE, 'test', 4), + NewStatsRow(LETTER_TYPE, NOTIFICATION_VIRUS_SCAN_FAILED, 'test', 5), ] stats_dict = format_admin_stats(rows) - assert stats_dict['email']['total'] == 0 - assert stats_dict['email']['failures']['technical-failure'] == 0 - assert stats_dict['email']['test-key'] == 3 + assert stats_dict[EMAIL_TYPE]['total'] == 0 + assert stats_dict[EMAIL_TYPE]['failures'][NOTIFICATION_PERMANENT_FAILURE] == 0 + assert stats_dict[EMAIL_TYPE]['test-key'] == 3 - assert stats_dict['sms']['total'] == 0 - assert stats_dict['sms']['failures']['permanent-failure'] == 0 - assert stats_dict['sms']['test-key'] == 4 + assert stats_dict[SMS_TYPE]['total'] == 0 + assert stats_dict[SMS_TYPE]['failures'][NOTIFICATION_PERMANENT_FAILURE] == 0 + assert stats_dict[SMS_TYPE]['test-key'] == 4 - assert stats_dict['letter']['total'] == 0 - assert stats_dict['letter']['failures']['virus-scan-failed'] == 0 - assert stats_dict['letter']['test-key'] == 5 + assert stats_dict[LETTER_TYPE]['total'] == 0 + assert stats_dict[LETTER_TYPE]['failures'][NOTIFICATION_VIRUS_SCAN_FAILED] == 0 + assert stats_dict[LETTER_TYPE]['test-key'] == 5 def test_format_admin_stats_counts_non_test_key_notifications_correctly(): rows = [ - NewStatsRow('email', 'technical-failure', 'normal', 1), - NewStatsRow('email', 'created', 'team', 3), - NewStatsRow('sms', 'temporary-failure', 'normal', 6), - NewStatsRow('sms', 'sent', 'normal', 2), - NewStatsRow('letter', 'pending-virus-check', 'normal', 1), + NewStatsRow(EMAIL_TYPE, NOTIFICATION_PERMANENT_FAILURE, 'normal', 1), + NewStatsRow(EMAIL_TYPE, NOTIFICATION_CREATED, 'team', 3), + NewStatsRow(SMS_TYPE, NOTIFICATION_TEMPORARY_FAILURE, 'normal', 6), + NewStatsRow(SMS_TYPE, NOTIFICATION_SENT, 'normal', 2), + NewStatsRow(LETTER_TYPE, NOTIFICATION_PENDING_VIRUS_CHECK, 'normal', 1), ] stats_dict = format_admin_stats(rows) - assert stats_dict['email']['total'] == 4 - assert stats_dict['email']['failures']['technical-failure'] == 1 + assert stats_dict[EMAIL_TYPE]['total'] == 4 + assert stats_dict[EMAIL_TYPE]['failures'][NOTIFICATION_PERMANENT_FAILURE] == 1 - assert stats_dict['sms']['total'] == 8 - assert stats_dict['sms']['failures']['permanent-failure'] == 0 + assert stats_dict[SMS_TYPE]['total'] == 8 + assert stats_dict[SMS_TYPE]['failures'][NOTIFICATION_PERMANENT_FAILURE] == 0 - assert stats_dict['letter']['total'] == 1 + assert stats_dict[LETTER_TYPE]['total'] == 1 def _stats(requested, delivered, failed): - return {'requested': requested, 'delivered': delivered, 'failed': failed} + return {'requested': requested, NOTIFICATION_DELIVERED: delivered, NOTIFICATION_FAILED: failed} @pytest.mark.parametrize( @@ -196,17 +217,37 @@ def test_create_empty_monthly_notification_status_stats_dict(year, expected_year output = create_empty_monthly_notification_status_stats_dict(year) assert sorted(output.keys()) == expected_years for v in output.values(): - assert v == {'sms': {}, 'email': {}, 'letter': {}} + assert v == {SMS_TYPE: {}, EMAIL_TYPE: {}, LETTER_TYPE: {}} @freeze_time('2018-06-01 04:59:59') # This test assumes the local timezone is EST def test_add_monthly_notification_status_stats(): row_data = [ - {'month': datetime(2018, 4, 1), 'notification_type': 'sms', 'notification_status': 'sending', 'count': 1}, - {'month': datetime(2018, 4, 1), 'notification_type': 'sms', 'notification_status': 'delivered', 'count': 2}, - {'month': datetime(2018, 4, 1), 'notification_type': 'email', 'notification_status': 'sending', 'count': 4}, - {'month': datetime(2018, 5, 1), 'notification_type': 'sms', 'notification_status': 'sending', 'count': 8}, + { + 'month': datetime(2018, 4, 1), + 'notification_type': SMS_TYPE, + 'notification_status': NOTIFICATION_SENDING, + 'count': 1, + }, + { + 'month': datetime(2018, 4, 1), + 'notification_type': SMS_TYPE, + 'notification_status': NOTIFICATION_DELIVERED, + 'count': 2, + }, + { + 'month': datetime(2018, 4, 1), + 'notification_type': EMAIL_TYPE, + 'notification_status': NOTIFICATION_SENDING, + 'count': 4, + }, + { + 'month': datetime(2018, 5, 1), + 'notification_type': SMS_TYPE, + 'notification_status': NOTIFICATION_SENDING, + 'count': 8, + }, ] rows = [] for r in row_data: @@ -217,15 +258,19 @@ def test_add_monthly_notification_status_stats(): data = create_empty_monthly_notification_status_stats_dict(2018) # this data won't be affected - data['2018-05']['email']['sending'] = 32 + data['2018-05'][EMAIL_TYPE][NOTIFICATION_SENDING] = 32 # this data will get combined with the 8 from row_data - data['2018-05']['sms']['sending'] = 16 + data['2018-05'][SMS_TYPE][NOTIFICATION_SENDING] = 16 add_monthly_notification_status_stats(data, rows) assert data == { - '2018-04': {'sms': {'sending': 1, 'delivered': 2}, 'email': {'sending': 4}, 'letter': {}}, - '2018-05': {'sms': {'sending': 24}, 'email': {'sending': 32}, 'letter': {}}, - '2018-06': {'sms': {}, 'email': {}, 'letter': {}}, + '2018-04': { + SMS_TYPE: {NOTIFICATION_SENDING: 1, NOTIFICATION_DELIVERED: 2}, + EMAIL_TYPE: {NOTIFICATION_SENDING: 4}, + LETTER_TYPE: {}, + }, + '2018-05': {SMS_TYPE: {NOTIFICATION_SENDING: 24}, EMAIL_TYPE: {NOTIFICATION_SENDING: 32}, LETTER_TYPE: {}}, + '2018-06': {SMS_TYPE: {}, EMAIL_TYPE: {}, LETTER_TYPE: {}}, } diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index 81c83704a7..b2e10b46a5 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/tests/app/v2/notifications/test_get_notifications.py @@ -865,7 +865,6 @@ def test_get_all_notifications_renames_letter_statuses(client, sample_letter_not ('sending', 'accepted'), ('delivered', 'received'), ('pending', 'pending'), - ('technical-failure', 'technical-failure'), ], ) def test_get_notifications_renames_letter_statuses(client, sample_letter_template, db_status, expected_status):