diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index 95d122032..1700699db 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -27,7 +27,7 @@ def send_sms_to_provider(notification): technical_failure(notification=notification) return - if notification.status == "created": + if notification.status == NotificationStatus.CREATED: provider = provider_to_use(NotificationType.SMS, notification.international) if not provider: technical_failure(notification=notification) @@ -109,7 +109,7 @@ def send_email_to_provider(notification): if not service.active: technical_failure(notification=notification) return - if notification.status == "created": + if notification.status == NotificationStatus.CREATED: provider = provider_to_use(NotificationType.EMAIL, False) template_dict = SerialisedTemplate.from_id_and_service_id( template_id=notification.template_id, @@ -134,10 +134,9 @@ def send_email_to_provider(notification): update_notification_to_sending(notification, provider) send_email_response(notification.reference, recipient) else: - from_address = '"{}" <{}@{}>'.format( - service.name, - service.email_from, - current_app.config["NOTIFY_EMAIL_DOMAIN"], + from_address = ( + f'"{service.name}" <{service.email_from}@' + f'{current_app.config["NOTIFY_EMAIL_DOMAIN"]}>' ) reference = provider.send_email( @@ -175,10 +174,8 @@ def provider_to_use(notification_type, international=True): ] if not active_providers: - current_app.logger.error( - "{} failed as no active providers".format(notification_type) - ) - raise Exception("No active {} providers".format(notification_type)) + current_app.logger.error(f"{notification_type} failed as no active providers") + raise Exception(f"No active {notification_type} providers") # we only have sns chosen_provider = active_providers[0] @@ -240,7 +237,6 @@ def technical_failure(notification): notification.status = NotificationStatus.TECHNICAL_FAILURE dao_update_notification(notification) raise NotificationTechnicalFailureException( - "Send {} for notification id {} to provider is not allowed: service {} is inactive".format( - notification.notification_type, notification.id, notification.service_id - ) + f"Send {notification.notification_type} for notification id {notification.id} " + f"to provider is not allowed: service {notification.service_id} is inactive" ) diff --git a/tests/app/celery/test_provider_tasks.py b/tests/app/celery/test_provider_tasks.py index 532c19e3f..e9f27be3d 100644 --- a/tests/app/celery/test_provider_tasks.py +++ b/tests/app/celery/test_provider_tasks.py @@ -11,6 +11,7 @@ from app.clients.email.aws_ses import ( AwsSesClientThrottlingSendRateException, ) from app.clients.sms import SmsClientResponseException +from app.enums import NotificationStatus from app.exceptions import NotificationTechnicalFailureException @@ -55,7 +56,7 @@ def test_should_retry_and_log_warning_if_SmsClientResponseException_for_deliver_ ) mocker.patch("app.celery.provider_tasks.deliver_sms.retry") mock_logger_warning = mocker.patch("app.celery.tasks.current_app.logger.warning") - assert sample_notification.status == "created" + assert sample_notification.status == NotificationStatus.CREATED deliver_sms(sample_notification.id) @@ -75,7 +76,7 @@ def test_should_retry_and_log_exception_for_non_SmsClientResponseException_excep "app.celery.tasks.current_app.logger.exception" ) - assert sample_notification.status == "created" + assert sample_notification.status == NotificationStatus.CREATED deliver_sms(sample_notification.id) assert provider_tasks.deliver_sms.retry.called is True @@ -204,7 +205,7 @@ def test_should_retry_and_log_exception_for_deliver_email_task( deliver_email(sample_notification.id) assert provider_tasks.deliver_email.retry.called is True - assert sample_notification.status == "created" + assert sample_notification.status == NotificationStatus.CREATED assert mock_logger_exception.called @@ -232,6 +233,6 @@ def test_if_ses_send_rate_throttle_then_should_retry_and_log_warning( deliver_email(sample_notification.id) assert provider_tasks.deliver_email.retry.called is True - assert sample_notification.status == "created" + assert sample_notification.status == NotificationStatus.CREATED assert not mock_logger_exception.called assert mock_logger_warning.called diff --git a/tests/app/conftest.py b/tests/app/conftest.py index d2c1427b6..2846acfd3 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -71,7 +71,7 @@ def create_sample_notification( job=None, job_row_number=None, to_field=None, - status="created", + status=NotificationStatus.CREATED, provider_response=None, reference=None, created_at=None, @@ -456,7 +456,7 @@ def sample_notification(notify_db_session): "service": service, "template_id": template.id, "template_version": template.version, - "status": "created", + "status": NotificationStatus.CREATED, "reference": None, "created_at": created_at, "sent_at": None, @@ -499,7 +499,7 @@ def sample_email_notification(notify_db_session): "service": service, "template_id": template.id, "template_version": template.version, - "status": "created", + "status": NotificationStatus.CREATED, "reference": None, "created_at": created_at, "billable_units": 0, diff --git a/tests/app/dao/test_fact_notification_status_dao.py b/tests/app/dao/test_fact_notification_status_dao.py index 99181e811..24812f5b2 100644 --- a/tests/app/dao/test_fact_notification_status_dao.py +++ b/tests/app/dao/test_fact_notification_status_dao.py @@ -570,8 +570,8 @@ def test_fetch_notification_statuses_for_job(sample_template): ) assert {x.status: x.count for x in fetch_notification_statuses_for_job(j1.id)} == { - "created": 5, - "delivered": 2, + NotificationStatus.CREATED: 5, + NotificationStatus.DELIVERED: 2, } diff --git a/tests/app/dao/test_jobs_dao.py b/tests/app/dao/test_jobs_dao.py index f9eac45f6..c2de39181 100644 --- a/tests/app/dao/test_jobs_dao.py +++ b/tests/app/dao/test_jobs_dao.py @@ -18,7 +18,7 @@ from app.dao.jobs_dao import ( find_jobs_with_missing_rows, find_missing_row_for_job, ) -from app.enums import JobStatus +from app.enums import JobStatus, NotificationStatus from app.models import Job, NotificationType, TemplateType from tests.app.db import ( create_job, @@ -31,19 +31,29 @@ from tests.app.db import ( def test_should_count_of_statuses_for_notifications_associated_with_job( sample_template, sample_job ): - create_notification(sample_template, job=sample_job, status="created") - create_notification(sample_template, job=sample_job, status="created") - create_notification(sample_template, job=sample_job, status="created") - create_notification(sample_template, job=sample_job, status="sending") - create_notification(sample_template, job=sample_job, status="delivered") + create_notification( + sample_template, job=sample_job, status=NotificationStatus.CREATED + ) + create_notification( + sample_template, job=sample_job, status=NotificationStatus.CREATED + ) + create_notification( + sample_template, job=sample_job, status=NotificationStatus.CREATED + ) + create_notification( + sample_template, job=sample_job, status=NotificationStatus.SENDING + ) + create_notification( + sample_template, job=sample_job, status=NotificationStatus.DELIVERED + ) results = dao_get_notification_outcomes_for_job( sample_template.service_id, sample_job.id ) assert {row.status: row.count for row in results} == { - "created": 3, - "sending": 1, - "delivered": 1, + NotificationStatus.CREATED: 3, + NotificationStatus.SENDING: 1, + NotificationStatus.DELIVERED: 1, } @@ -60,13 +70,13 @@ def test_should_return_notifications_only_for_this_job(sample_template): job_1 = create_job(sample_template) job_2 = create_job(sample_template) - create_notification(sample_template, job=job_1, status="created") - create_notification(sample_template, job=job_2, status="sent") + create_notification(sample_template, job=job_1, status=NotificationStatus.CREATED) + create_notification(sample_template, job=job_2, status=NotificationStatus.SENT) results = dao_get_notification_outcomes_for_job( sample_template.service_id, job_1.id ) - assert {row.status: row.count for row in results} == {"created": 1} + assert {row.status: row.count for row in results} == {NotificationStatus.CREATED: 1} def test_should_return_notifications_only_for_this_service( diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 3aa50738e..da9aa0e10 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -1044,9 +1044,9 @@ def test_dao_fetch_todays_stats_for_service_only_includes_today(notify_db_sessio stats = dao_fetch_todays_stats_for_service(template.service_id) stats = {row.status: row.count for row in stats} - assert stats["delivered"] == 1 - assert stats["failed"] == 1 - assert stats["created"] == 1 + assert stats[NotificationStatus.DELIVERED] == 1 + assert stats[NotificationStatus.FAILED] == 1 + assert stats[NotificationStatus.CREATED] == 1 @pytest.mark.skip(reason="Need a better way to test variable DST date") @@ -1079,11 +1079,11 @@ def test_dao_fetch_todays_stats_for_service_only_includes_today_when_clocks_spri 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 not stats.get("permanent-failure") - assert not stats.get("temporary-failure") + assert NotificationStatus.DELIVERED not in stats + assert stats[NotificationStatus.FAILED] == 1 + assert stats[NotificationStatus.CREATED] == 1 + assert not stats.get(NotificationStatus.PERMANENT_FAILURE) + assert not stats.get(NotificationStatus.TEMPORARY_FAILURE) def test_dao_fetch_todays_stats_for_service_only_includes_today_during_bst( @@ -1109,10 +1109,10 @@ def test_dao_fetch_todays_stats_for_service_only_includes_today_during_bst( 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 not stats.get("permanent-failure") + assert NotificationStatus.DELIVERED not in stats + assert stats[NotificationStatus.FAILED] == 1 + assert stats[NotificationStatus.CREATED] == 1 + assert not stats.get(NotificationStatus.PERMANENT_FAILURE) def test_dao_fetch_todays_stats_for_service_only_includes_today_when_clocks_fall_back( @@ -1139,10 +1139,10 @@ def test_dao_fetch_todays_stats_for_service_only_includes_today_when_clocks_fall 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 not stats.get("permanent-failure") + assert NotificationStatus.DELIVERED not in stats + assert stats[NotificationStatus.FAILED] == 1 + assert stats[NotificationStatus.CREATED] == 1 + assert not stats.get(NotificationStatus.PERMANENT_FAILURE) def test_dao_fetch_todays_stats_for_service_only_includes_during_utc(notify_db_session): @@ -1167,10 +1167,10 @@ def test_dao_fetch_todays_stats_for_service_only_includes_during_utc(notify_db_s 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 not stats.get("permanent-failure") + assert NotificationStatus.DELIVERED not in stats + assert stats[NotificationStatus.FAILED] == 1 + assert stats[NotificationStatus.CREATED] == 1 + assert not stats.get(NotificationStatus.PERMANENT_FAILURE) def test_dao_fetch_todays_stats_for_all_services_includes_all_services( diff --git a/tests/app/db.py b/tests/app/db.py index ef5206e75..632c3a8a6 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -272,10 +272,10 @@ def create_notification( ) if status not in ( - "created", - "validation-failed", - "virus-scan-failed", - "pending-virus-check", + NotificationStatus.CREATED, + NotificationStatus.VALIDATION_FAILED, + NotificationStatus.VIRUS_SCAN_FAILED, + NotificationStatus.PENDING_VIRUS_CHECK, ): sent_at = sent_at or datetime.utcnow() updated_at = updated_at or datetime.utcnow() diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index 1f289bfe8..b49717e05 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -991,9 +991,13 @@ def test_get_jobs_should_retrieve_from_ft_notification_status_for_old_jobs( assert resp_json["data"][0]["id"] == str(job_3.id) assert resp_json["data"][0]["statistics"] == [] assert resp_json["data"][1]["id"] == str(job_2.id) - assert resp_json["data"][1]["statistics"] == [{"status": "created", "count": 1}] + assert resp_json["data"][1]["statistics"] == [ + {"status": NotificationStatus.CREATED, "count": 1}, + ] assert resp_json["data"][2]["id"] == str(job_1.id) - assert resp_json["data"][2]["statistics"] == [{"status": "delivered", "count": 6}] + assert resp_json["data"][2]["statistics"] == [ + {"status": NotificationStatus.DELIVERED, "count": 6}, + ] @freeze_time("2017-07-17 07:17") diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index 0e09cd5fb..6e8142e7c 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -32,7 +32,7 @@ def test_get_notification_by_id( assert response.status_code == 200 notification = json.loads(response.get_data(as_text=True))["data"]["notification"] - assert notification["status"] == "created" + assert notification["status"] == NotificationStatus.CREATED assert notification["template"] == { "id": str(notification_to_get.template.id), "name": notification_to_get.template.name, @@ -152,7 +152,7 @@ def test_get_all_notifications(client, sample_notification): notifications = json.loads(response.get_data(as_text=True)) assert response.status_code == 200 - assert notifications["notifications"][0]["status"] == "created" + assert notifications["notifications"][0]["status"] == NotificationStatus.CREATED assert notifications["notifications"][0]["template"] == { "id": str(sample_notification.template.id), "name": sample_notification.template.name, diff --git a/tests/app/service/test_service_data_retention_rest.py b/tests/app/service/test_service_data_retention_rest.py index 035b11b42..a6b96fc91 100644 --- a/tests/app/service/test_service_data_retention_rest.py +++ b/tests/app/service/test_service_data_retention_rest.py @@ -10,7 +10,9 @@ from tests.app.db import create_service_data_retention def test_get_service_data_retention(client, sample_service): sms_data_retention = create_service_data_retention(service=sample_service) email_data_retention = create_service_data_retention( - service=sample_service, notification_type="email", days_of_retention=10 + service=sample_service, + notification_type=NotificationType.EMAIL, + days_of_retention=10, ) response = client.get( diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index 801a18f38..fdbd9171d 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -312,7 +312,9 @@ def test_post_user_attribute_with_updated_by( mock_persist_notification = mocker.patch("app.user.rest.persist_notification") mocker.patch("app.user.rest.send_notification_to_queue") json_resp = admin_request.post( - "user.update_user_attribute", user_id=sample_user.id, _data=update_dict + "user.update_user_attribute", + user_id=sample_user.id, + _data=update_dict, ) assert json_resp["data"][user_attribute] == user_value if arguments: @@ -329,7 +331,9 @@ def test_post_user_attribute_with_updated_by_sends_notification_to_international mocker.patch("app.user.rest.send_notification_to_queue") admin_request.post( - "user.update_user_attribute", user_id=sample_user.id, _data=update_dict + "user.update_user_attribute", + user_id=sample_user.id, + _data=update_dict, ) notification = Notification.query.first() @@ -343,7 +347,9 @@ def test_archive_user(mocker, admin_request, sample_user): archive_mock = mocker.patch("app.user.rest.dao_archive_user") admin_request.post( - "user.archive_user", user_id=sample_user.id, _expected_status=204 + "user.archive_user", + user_id=sample_user.id, + _expected_status=204, ) archive_mock.assert_called_once_with(sample_user) @@ -363,7 +369,9 @@ def test_archive_user_when_user_cannot_be_archived(mocker, admin_request, sample mocker.patch("app.dao.users_dao.user_can_be_archived", return_value=False) json_resp = admin_request.post( - "user.archive_user", user_id=sample_user.id, _expected_status=400 + "user.archive_user", + user_id=sample_user.id, + _expected_status=400, ) msg = "User can’t be removed from a service - check all services have another team member with manage_settings" @@ -390,7 +398,9 @@ def test_get_user_by_email(admin_request, sample_service): def test_get_user_by_email_not_found_returns_404(admin_request, sample_user): json_resp = admin_request.get( - "user.get_by_email", email="no_user@digital.fake.gov", _expected_status=404 + "user.get_by_email", + email="no_user@digital.fake.gov", + _expected_status=404, ) assert json_resp["result"] == "error" assert json_resp["message"] == "No result found" diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 4a90c5372..b7d98fab6 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -535,7 +535,7 @@ def test_should_not_persist_or_send_notification_if_simulated_recipient( client, recipient, notification_type, sample_email_template, sample_template, mocker ): apply_async = mocker.patch( - "app.celery.provider_tasks.deliver_{}.apply_async".format(notification_type) + f"app.celery.provider_tasks.deliver_{notification_type}.apply_async" ) if notification_type == NotificationType.SMS: @@ -1194,7 +1194,7 @@ def test_post_notification_returns_201_when_content_type_is_missing_but_payload_ valid_json = { "template_id": str(template.id), } - if notification_type == "email": + if notification_type == NotificationType.EMAIL: valid_json.update({"email_address": sample_service.users[0].email_address}) else: valid_json.update({"phone_number": "+447700900855"}) diff --git a/tests/app/v2/template/test_template_schemas.py b/tests/app/v2/template/test_template_schemas.py index eff8348f2..198ad9bb6 100644 --- a/tests/app/v2/template/test_template_schemas.py +++ b/tests/app/v2/template/test_template_schemas.py @@ -80,14 +80,14 @@ invalid_json_post_args = [ valid_json_post_response = { "id": str(uuid.uuid4()), - "type": "email", + "type": TemplateType.EMAIL, "version": 1, "body": "some body", } valid_json_post_response_with_optionals = { "id": str(uuid.uuid4()), - "type": "email", + "type": TemplateType.EMAIL, "version": 1, "body": "some body", "subject": "some subject",