More fixes, removing literal "created" from code.

Signed-off-by: Cliff Hill <Clifford.hill@gsa.gov>

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(
+            from_address = (
-                service.name,
+                f'"{service.name}" <{service.email_from}@'
-                service.email_from,
+                f'{current_app.config["NOTIFY_EMAIL_DOMAIN"]}>'
-                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(
+        current_app.logger.error(f"{notification_type} failed as no active providers")
-            "{} failed as no active providers".format(notification_type)
+        raise Exception(f"No active {notification_type} providers")
-        )
-        raise Exception("No active {} providers".format(notification_type))
 
     # 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(
+        f"Send {notification.notification_type} for notification id {notification.id} "
-            notification.notification_type, notification.id, notification.service_id
+        f"to provider is not allowed: service {notification.service_id} is inactive"
-        )
     )

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

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,

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,
+        NotificationStatus.CREATED: 5,
-        "delivered": 2,
+        NotificationStatus.DELIVERED: 2,
     }
 
 

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(
-    create_notification(sample_template, job=sample_job, status="created")
+        sample_template, job=sample_job, status=NotificationStatus.CREATED
-    create_notification(sample_template, job=sample_job, status="created")
+    )
-    create_notification(sample_template, job=sample_job, status="sending")
+    create_notification(
-    create_notification(sample_template, job=sample_job, status="delivered")
+        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,
+        NotificationStatus.CREATED: 3,
-        "sending": 1,
+        NotificationStatus.SENDING: 1,
-        "delivered": 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_1, status=NotificationStatus.CREATED)
-    create_notification(sample_template, job=job_2, status="sent")
+    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(

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[NotificationStatus.DELIVERED] == 1
-    assert stats["failed"] == 1
+    assert stats[NotificationStatus.FAILED] == 1
-    assert stats["created"] == 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 NotificationStatus.DELIVERED not in stats
-    assert stats["failed"] == 1
+    assert stats[NotificationStatus.FAILED] == 1
-    assert stats["created"] == 1
+    assert stats[NotificationStatus.CREATED] == 1
-    assert not stats.get("permanent-failure")
+    assert not stats.get(NotificationStatus.PERMANENT_FAILURE)
-    assert not stats.get("temporary-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 NotificationStatus.DELIVERED not in stats
-    assert stats["failed"] == 1
+    assert stats[NotificationStatus.FAILED] == 1
-    assert stats["created"] == 1
+    assert stats[NotificationStatus.CREATED] == 1
-    assert not stats.get("permanent-failure")
+    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 NotificationStatus.DELIVERED not in stats
-    assert stats["failed"] == 1
+    assert stats[NotificationStatus.FAILED] == 1
-    assert stats["created"] == 1
+    assert stats[NotificationStatus.CREATED] == 1
-    assert not stats.get("permanent-failure")
+    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 NotificationStatus.DELIVERED not in stats
-    assert stats["failed"] == 1
+    assert stats[NotificationStatus.FAILED] == 1
-    assert stats["created"] == 1
+    assert stats[NotificationStatus.CREATED] == 1
-    assert not stats.get("permanent-failure")
+    assert not stats.get(NotificationStatus.PERMANENT_FAILURE)
 
 
 def test_dao_fetch_todays_stats_for_all_services_includes_all_services(

tests/app/db.py

@@ -272,10 +272,10 @@ def create_notification(
         )
 
     if status not in (
-        "created",
+        NotificationStatus.CREATED,
-        "validation-failed",
+        NotificationStatus.VALIDATION_FAILED,
-        "virus-scan-failed",
+        NotificationStatus.VIRUS_SCAN_FAILED,
-        "pending-virus-check",
+        NotificationStatus.PENDING_VIRUS_CHECK,
     ):
         sent_at = sent_at or datetime.utcnow()
         updated_at = updated_at or datetime.utcnow()

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")

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,

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(

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"

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"})

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",