diff --git a/app/dao/users_dao.py b/app/dao/users_dao.py index f50b70c45..7828c5c23 100644 --- a/app/dao/users_dao.py +++ b/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 diff --git a/app/service/statistics.py b/app/service/statistics.py index c8d882e8b..6ef87c9ff 100644 --- a/app/service/statistics.py +++ b/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", - "permanent-failure", - "temporary-failure", - "virus-scan-failed", + NotificationStatus.TECHNICAL_FAILURE, + NotificationStatus.PERMANENT_FAILURE, + NotificationStatus.TEMPORARY_FAILURE, + 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, - "permanent-failure": 0, - "temporary-failure": 0, - "virus-scan-failed": 0, + NotificationStatus.TECHNICAL_FAILURE: 0, + NotificationStatus.PERMANENT_FAILURE: 0, + NotificationStatus.TEMPORARY_FAILURE: 0, + NotificationStatus.VIRUS_SCAN_FAILED: 0, } return stats_dict diff --git a/migrations/versions/0410_enums_for_everything.py b/migrations/versions/0410_enums_for_everything.py index 879e3c772..e34a3621a 100644 --- a/migrations/versions/0410_enums_for_everything.py +++ b/migrations/versions/0410_enums_for_everything.py @@ -25,6 +25,7 @@ from app.enums import ( NotificationStatus, NotificationType, OrganizationType, + PermissionType, RecipientType, ServicePermissionType, TemplateProcessType, diff --git a/tests/app/celery/test_provider_tasks.py b/tests/app/celery/test_provider_tasks.py index e9f27be3d..3b02de283 100644 --- a/tests/app/celery/test_provider_tasks.py +++ b/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( diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 2846acfd3..b4714eeb8 100644 --- a/tests/app/conftest.py +++ b/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( diff --git a/tests/app/dao/test_invited_user_dao.py b/tests/app/dao/test_invited_user_dao.py index de595c415..fb1154c96 100644 --- a/tests/app/dao/test_invited_user_dao.py +++ b/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" - saved.status = "cancelled" + assert saved.status == InvitedUserStatus.PENDING + 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) diff --git a/tests/app/dao/test_jobs_dao.py b/tests/app/dao/test_jobs_dao.py index c2de39181..515d453be 100644 --- a/tests/app/dao/test_jobs_dao.py +++ b/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[1].job_status == "pending" + assert jobs[0].job_status == JobStatus.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 ): diff --git a/tests/app/dao/test_permissions_dao.py b/tests/app/dao/test_permissions_dao.py index 9d0c85c5f..77c56a2f2 100644 --- a/tests/app/dao/test_permissions_dao.py +++ b/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", - "manage_templates", - "manage_settings", - "send_texts", - "send_emails", - "manage_api_keys", - "view_activity", + PermissionType.MANAGE_USERS, + PermissionType.MANAGE_TEMPLATES, + PermissionType.MANAGE_SETTINGS, + PermissionType.SEND_TEXTS, + PermissionType.SEND_EMAILS, + PermissionType.MANAGE_API_KEYS, + PermissionType.VIEW_ACTIVITY, ] ) == sorted([i.permission for i in permissions]) diff --git a/tests/app/dao/test_users_dao.py b/tests/app/dao/test_users_dao.py index 6e4529310..548840eb1 100644 --- a/tests/app/dao/test_users_dao.py +++ b/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(service_1_user, service_1, "manage_settings", "view_activity") + create_permissions(sample_user, service_1, PermissionType.MANAGE_SETTINGS) + 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(service_2_user, service_2, "manage_settings") + create_permissions(sample_user, service_2, PermissionType.VIEW_ACTIVITY) + 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_2, sample_service, "manage_settings", "view_activity") + create_permissions(user_1, sample_service, PermissionType.MANAGE_SETTINGS) + 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(pending_user, sample_service, "view_activity") - create_permissions(inactive_user, sample_service, "send_emails", "send_texts") + create_permissions(active_user, sample_service, PermissionType.MANAGE_SETTINGS) + create_permissions(pending_user, sample_service, PermissionType.VIEW_ACTIVITY) + 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) diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index d260f47e6..cb4369a0e 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/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( diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index b49717e05..acf453c37 100644 --- a/tests/app/job/test_rest.py +++ b/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", []), diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index d7f8e169c..d622be907 100644 --- a/tests/app/service/test_rest.py +++ b/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, ) ], ) diff --git a/tests/app/service_invite/test_service_invite_rest.py b/tests/app/service_invite/test_service_invite_rest.py index 964a8acc8..482ec1944 100644 --- a/tests/app/service_invite/test_service_invite_rest.py +++ b/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( diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index e551ca4f8..c86dad592 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/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