Lots more cleanup.

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

app/dao/users_dao.py

@@ -9,7 +9,7 @@ from app import db
 from app.dao.dao_utils import autocommit
 from app.dao.permissions_dao import permission_dao
 from app.dao.service_user_dao import dao_get_service_users_by_user_id
-from app.enums import AuthType
+from app.enums import AuthType, PermissionType
 from app.errors import InvalidRequest
 from app.models import User, VerifyCode
 from app.utils import escape_special_characters, get_archived_db_column_value
@@ -198,7 +198,7 @@ def user_can_be_archived(user):
             return False
 
         if not any(
-            "manage_settings" in user.get_permissions(service.id)
+            PermissionType.MANAGE_SETTINGS in user.get_permissions(service.id)
             for user in other_active_users
         ):
             # no-one else has manage settings

app/service/statistics.py

@@ -28,10 +28,10 @@ def format_admin_stats(statistics):
         else:
             counts[row.notification_type]["total"] += row.count
             if row.status in (
-                "technical-failure",
+                NotificationStatus.TECHNICAL_FAILURE,
-                "permanent-failure",
+                NotificationStatus.PERMANENT_FAILURE,
-                "temporary-failure",
+                NotificationStatus.TEMPORARY_FAILURE,
-                "virus-scan-failed",
+                NotificationStatus.VIRUS_SCAN_FAILED,
             ):
                 counts[row.notification_type]["failures"][row.status] += row.count
 
@@ -47,10 +47,10 @@ def create_stats_dict():
             stats_dict[template][status] = 0
 
         stats_dict[template]["failures"] = {
-            "technical-failure": 0,
+            NotificationStatus.TECHNICAL_FAILURE: 0,
-            "permanent-failure": 0,
+            NotificationStatus.PERMANENT_FAILURE: 0,
-            "temporary-failure": 0,
+            NotificationStatus.TEMPORARY_FAILURE: 0,
-            "virus-scan-failed": 0,
+            NotificationStatus.VIRUS_SCAN_FAILED: 0,
         }
     return stats_dict
 

migrations/versions/0410_enums_for_everything.py

@@ -25,6 +25,7 @@ from app.enums import (
     NotificationStatus,
     NotificationType,
     OrganizationType,
+    PermissionType,
     RecipientType,
     ServicePermissionType,
     TemplateProcessType,

tests/app/celery/test_provider_tasks.py

@@ -106,7 +106,7 @@ def test_should_go_into_technical_error_if_exceeds_retries_on_deliver_sms_task(
         queue="retry-tasks", countdown=0
     )
 
-    assert sample_notification.status == "temporary-failure"
+    assert sample_notification.status == NotificationStatus.TEMPORARY_FAILURE
     assert mock_logger_exception.called
 
 
@@ -164,7 +164,7 @@ def test_should_go_into_technical_error_if_exceeds_retries_on_deliver_email_task
     assert str(sample_notification.id) in str(e.value)
 
     provider_tasks.deliver_email.retry.assert_called_with(queue="retry-tasks")
-    assert sample_notification.status == "technical-failure"
+    assert sample_notification.status == NotificationStatus.TECHNICAL_FAILURE
 
 
 def test_should_technical_error_and_not_retry_if_EmailClientNonRetryableException(
@@ -179,7 +179,7 @@ def test_should_technical_error_and_not_retry_if_EmailClientNonRetryableExceptio
     deliver_email(sample_notification.id)
 
     assert provider_tasks.deliver_email.retry.called is False
-    assert sample_notification.status == "technical-failure"
+    assert sample_notification.status == NotificationStatus.TECHNICAL_FAILURE
 
 
 def test_should_retry_and_log_exception_for_deliver_email_task(

tests/app/conftest.py

@@ -22,6 +22,7 @@ from app.enums import (
     JobStatus,
     KeyType,
     NotificationStatus,
+    PermissionType,
     RecipientType,
     ServicePermissionType,
     TemplateType,
@@ -588,7 +589,7 @@ def sample_invited_org_user(sample_user, sample_organization):
 @pytest.fixture(scope="function")
 def sample_user_service_permission(sample_user):
     service = create_service(user=sample_user, check_if_service_exists=True)
-    permission = "manage_settings"
+    permission = PermissionType.MANAGE_SETTINGS
 
     data = {"user": sample_user, "service": service, "permission": permission}
     p_model = Permission.query.filter_by(

tests/app/dao/test_invited_user_dao.py

@@ -12,7 +12,7 @@ from app.dao.invited_user_dao import (
     get_invited_users_for_service,
     save_invited_user,
 )
-from app.enums import InvitedUserStatus
+from app.enums import InvitedUserStatus, PermissionType
 from app.models import InvitedUser
 from tests.app.db import create_invited_user
 
@@ -109,12 +109,12 @@ def test_save_invited_user_sets_status_to_cancelled(
 ):
     assert InvitedUser.query.count() == 1
     saved = InvitedUser.query.get(sample_invited_user.id)
-    assert saved.status == "pending"
+    assert saved.status == InvitedUserStatus.PENDING
-    saved.status = "cancelled"
+    saved.status = InvitedUserStatus.CANCELLED
     save_invited_user(saved)
     assert InvitedUser.query.count() == 1
     cancelled_invited_user = InvitedUser.query.get(sample_invited_user.id)
-    assert cancelled_invited_user.status == "cancelled"
+    assert cancelled_invited_user.status == InvitedUserStatus.CANCELLED
 
 
 def test_should_delete_all_invitations_more_than_one_day_old(
@@ -195,9 +195,9 @@ def make_invitation(user, service, age=None, email_address="test@test.com"):
         email_address=email_address,
         from_user=user,
         service=service,
-        status="pending",
+        status=InvitedUserStatus.PENDING,
         created_at=datetime.utcnow() - (age or timedelta(hours=0)),
-        permissions="manage_settings",
+        permissions=PermissionType.MANAGE_SETTINGS,
         folder_permissions=[str(uuid.uuid4())],
     )
     db.session.add(verify_code)

tests/app/dao/test_jobs_dao.py

@@ -214,7 +214,7 @@ def test_get_jobs_for_service_in_processed_at_then_created_at_order(
 
 
 def test_update_job(sample_job):
-    assert sample_job.job_status == "pending"
+    assert sample_job.job_status == JobStatus.PENDING
 
     sample_job.job_status = "in progress"
 
@@ -268,8 +268,8 @@ def test_set_scheduled_jobs_to_pending_updates_rows(sample_template):
     create_job(sample_template, scheduled_for=one_hour_ago, job_status="scheduled")
     jobs = dao_set_scheduled_jobs_to_pending()
     assert len(jobs) == 2
-    assert jobs[0].job_status == "pending"
+    assert jobs[0].job_status == JobStatus.PENDING
-    assert jobs[1].job_status == "pending"
+    assert jobs[1].job_status == JobStatus.PENDING
 
 
 def test_get_future_scheduled_job_gets_a_job_yet_to_send(sample_scheduled_job):
@@ -444,7 +444,7 @@ def test_find_jobs_with_missing_rows_returns_nothing_for_a_job_completed_more_th
     assert len(results) == 0
 
 
-@pytest.mark.parametrize("status", ["pending", "in progress", "cancelled", "scheduled"])
+@pytest.mark.parametrize("status", [JobStatus.PENDING, JobStatus.IN_PROGRESS, JobStatus.CANCELLED, JobStatus.SCHEDULED,],)
 def test_find_jobs_with_missing_rows_doesnt_return_jobs_that_are_not_finished(
     sample_email_template, status
 ):

tests/app/dao/test_permissions_dao.py

@@ -1,5 +1,6 @@
 from app.dao import DAOClass
 from app.dao.permissions_dao import permission_dao
+from app.enums import PermissionType
 from tests.app.db import create_service
 
 
@@ -10,13 +11,13 @@ def test_get_permissions_by_user_id_returns_all_permissions(sample_service):
     assert len(permissions) == 7
     assert sorted(
         [
-            "manage_users",
+            PermissionType.MANAGE_USERS,
-            "manage_templates",
+            PermissionType.MANAGE_TEMPLATES,
-            "manage_settings",
+            PermissionType.MANAGE_SETTINGS,
-            "send_texts",
+            PermissionType.SEND_TEXTS,
-            "send_emails",
+            PermissionType.SEND_EMAILS,
-            "manage_api_keys",
+            PermissionType.MANAGE_API_KEYS,
-            "view_activity",
+            PermissionType.VIEW_ACTIVITY,
         ]
     ) == sorted([i.permission for i in permissions])
 

tests/app/dao/test_users_dao.py

@@ -25,7 +25,7 @@ from app.dao.users_dao import (
     update_user_password,
     user_can_be_archived,
 )
-from app.enums import AuthType
+from app.enums import AuthType, PermissionType
 from app.errors import InvalidRequest
 from app.models import User, VerifyCode
 from tests.app.db import (
@@ -208,14 +208,14 @@ def test_dao_archive_user(sample_user, sample_organization, fake_uuid):
     service_1 = create_service(service_name="Service 1")
     service_1_user = create_user(email="1@test.com")
     service_1.users = [sample_user, service_1_user]
-    create_permissions(sample_user, service_1, "manage_settings")
+    create_permissions(sample_user, service_1, PermissionType.MANAGE_SETTINGS)
-    create_permissions(service_1_user, service_1, "manage_settings", "view_activity")
+    create_permissions(service_1_user, service_1, PermissionType.MANAGE_SETTINGS, PermissionType.VIEW_ACTIVITY,)
 
     service_2 = create_service(service_name="Service 2")
     service_2_user = create_user(email="2@test.com")
     service_2.users = [sample_user, service_2_user]
-    create_permissions(sample_user, service_2, "view_activity")
+    create_permissions(sample_user, service_2, PermissionType.VIEW_ACTIVITY)
-    create_permissions(service_2_user, service_2, "manage_settings")
+    create_permissions(service_2_user, service_2, PermissionType.MANAGE_SETTINGS)
 
     # make sample_user an org member
     sample_organization.users = [sample_user]
@@ -265,10 +265,10 @@ def test_user_can_be_archived_if_the_other_service_members_have_the_manage_setti
 
     sample_service.users = [user_1, user_2, user_3]
 
-    create_permissions(user_1, sample_service, "manage_settings")
+    create_permissions(user_1, sample_service, PermissionType.MANAGE_SETTINGS)
-    create_permissions(user_2, sample_service, "manage_settings", "view_activity")
+    create_permissions(user_2, sample_service, PermissionType.MANAGE_SETTINGS, PermissionType.VIEW_ACTIVITY,)
     create_permissions(
-        user_3, sample_service, "manage_settings", "send_emails", "send_texts"
+        user_3, sample_service, PermissionType.MANAGE_SETTINGS, PermissionType.SEND_EMAILS, PermissionType.SEND_TEXTS,
     )
 
     assert len(sample_service.users) == 3
@@ -304,9 +304,9 @@ def test_user_cannot_be_archived_if_the_other_service_members_do_not_have_the_ma
 
     sample_service.users = [active_user, pending_user, inactive_user]
 
-    create_permissions(active_user, sample_service, "manage_settings")
+    create_permissions(active_user, sample_service, PermissionType.MANAGE_SETTINGS)
-    create_permissions(pending_user, sample_service, "view_activity")
+    create_permissions(pending_user, sample_service, PermissionType.VIEW_ACTIVITY)
-    create_permissions(inactive_user, sample_service, "send_emails", "send_texts")
+    create_permissions(inactive_user, sample_service, PermissionType.SEND_EMAILS, PermissionType.SEND_TEXTS,)
 
     assert len(sample_service.users) == 3
     assert not user_can_be_archived(active_user)

tests/app/delivery/test_send_to_providers.py

@@ -153,7 +153,7 @@ def test_should_not_send_email_message_when_service_is_inactive_notifcation_is_i
         send_to_providers.send_email_to_provider(sample_notification)
     assert str(sample_notification.id) in str(e.value)
     send_mock.assert_not_called()
-    assert Notification.query.get(sample_notification.id).status == "technical-failure"
+    assert Notification.query.get(sample_notification.id).status == NotificationStatus.TECHNICAL_FAILURE
 
 
 def test_should_not_send_sms_message_when_service_is_inactive_notification_is_in_tech_failure(
@@ -166,7 +166,7 @@ def test_should_not_send_sms_message_when_service_is_inactive_notification_is_in
         send_to_providers.send_sms_to_provider(sample_notification)
     assert str(sample_notification.id) in str(e.value)
     send_mock.assert_not_called()
-    assert Notification.query.get(sample_notification.id).status == "technical-failure"
+    assert Notification.query.get(sample_notification.id).status == NotificationStatus.TECHNICAL_FAILURE
 
 
 def test_send_sms_should_use_template_version_from_notification_not_latest(

tests/app/job/test_rest.py

@@ -849,11 +849,11 @@ def test_get_jobs_accepts_page_parameter(admin_request, sample_template):
 @pytest.mark.parametrize(
     "statuses_filter, expected_statuses",
     [
-        ("", JobStatus),
+        ("", list(JobStatus)),
         ("pending", [JobStatus.PENDING]),
         (
             "pending, in progress, finished, sending limits exceeded, scheduled, cancelled, ready to send, sent to dvla, error",  # noqa
-            JobStatus,
+            list(JobStatus),
         ),
         # bad statuses are accepted, just return no data
         ("foo", []),

tests/app/service/test_rest.py

@@ -1624,7 +1624,7 @@ def test_remove_user_from_service(client, sample_user_service_permission):
             Permission(
                 service_id=service.id,
                 user_id=second_user.id,
-                permission="manage_settings",
+                permission=PermissionType.MANAGE_SETTINGS,
             )
         ],
     )

tests/app/service_invite/test_service_invite_rest.py

@@ -7,7 +7,7 @@ from flask import current_app
 from freezegun import freeze_time
 from notifications_utils.url_safe_token import generate_token
 
-from app.enums import AuthType
+from app.enums import AuthType, InvitedUserStatus
 from app.models import Notification
 from tests import create_admin_authorization_header
 from tests.app.db import create_invited_user
@@ -224,7 +224,7 @@ def test_resend_expired_invite(
 
     assert response.status_code == 200
     json_resp = json.loads(response.get_data(as_text=True))["data"]
-    assert json_resp["status"] == "pending"
+    assert json_resp["status"] == InvitedUserStatus.PENDING
     assert mock_send.called
 
 
@@ -240,7 +240,7 @@ def test_update_invited_user_set_status_to_cancelled(client, sample_invited_user
 
     assert response.status_code == 200
     json_resp = json.loads(response.get_data(as_text=True))["data"]
-    assert json_resp["status"] == "cancelled"
+    assert json_resp["status"] == InvitedUserStatus.CANCELLED
 
 
 def test_update_invited_user_for_wrong_service_returns_404(

tests/app/v2/notifications/test_get_notifications.py

@@ -415,7 +415,7 @@ def test_get_all_notifications_filter_by_template_type_invalid_template_type(
 
 
 def test_get_all_notifications_filter_by_single_status(client, sample_template):
-    notification = create_notification(template=sample_template, status="pending")
+    notification = create_notification(template=sample_template, status=NotificationStatus.PENDING,)
     create_notification(template=sample_template)
 
     auth_header = create_service_authorization_header(
@@ -437,7 +437,7 @@ def test_get_all_notifications_filter_by_single_status(client, sample_template):
     assert len(json_response["notifications"]) == 1
 
     assert json_response["notifications"][0]["id"] == str(notification.id)
-    assert json_response["notifications"][0]["status"] == "pending"
+    assert json_response["notifications"][0]["status"] == NotificationStatus.PENDING
 
 
 def test_get_all_notifications_filter_by_status_invalid_status(
@@ -476,7 +476,7 @@ def test_get_all_notifications_filter_by_multiple_statuses(client, sample_templa
         ]
     ]
     failed_notification = create_notification(
-        template=sample_template, status="permanent-failure"
+        template=sample_template, status=NotificationStatus.PERMANENT_FAILURE,
     )
 
     auth_header = create_service_authorization_header(
@@ -510,7 +510,7 @@ def test_get_all_notifications_filter_by_failed_status(client, sample_template):
         status=NotificationStatus.CREATED,
     )
     failed_notifications = [
-        create_notification(template=sample_template, status="failed")
+        create_notification(template=sample_template, status=NotificationStatus.FAILED)
     ]
     auth_header = create_service_authorization_header(
         service_id=created_notification.service_id