From b73b4ac73e5502324324b33721c65e0028e010a4 Mon Sep 17 00:00:00 2001 From: Beverly Nguyen Date: Fri, 4 Jul 2025 16:37:04 -0700 Subject: [PATCH 1/6] PART 1: Created and implemented enums throughout the codebase to replace hardcoded status strings, improving type safety and reducing the risk of typos. --- app/enums.py | 74 ++++++++++++++++++++++ app/main/views/invites.py | 9 +-- app/main/views/jobs.py | 3 +- app/main/views/register.py | 5 +- app/main/views/service_settings.py | 7 +- app/models/job.py | 21 +++--- app/models/user.py | 5 +- app/notify_client/api_key_api_client.py | 7 +- app/notify_client/invite_api_client.py | 5 +- app/notify_client/job_api_client.py | 23 +++---- app/notify_client/org_invite_api_client.py | 5 +- app/status/views/healthcheck.py | 13 ++-- 12 files changed, 132 insertions(+), 45 deletions(-) create mode 100644 app/enums.py diff --git a/app/enums.py b/app/enums.py new file mode 100644 index 000000000..48474d6dc --- /dev/null +++ b/app/enums.py @@ -0,0 +1,74 @@ +from enum import Enum + + +class NotificationStatus(Enum): + CREATED = "created" + PENDING = "pending" + SENDING = "sending" + + DELIVERED = "delivered" + SENT = "sent" + + FAILED = "failed" + TEMPORARY_FAILURE = "temporary-failure" + PERMANENT_FAILURE = "permanent-failure" + TECHNICAL_FAILURE = "technical-failure" + VALIDATION_FAILED = "validation-failed" + CANCELLED = "cancelled" + + +class ApiKeyType(Enum): + NORMAL = "normal" + TEAM = "team" + TEST = "test" + + +class JobStatus(Enum): + PENDING = "pending" + IN_PROGRESS = "in progress" + FINISHED = "finished" + SENDING_LIMITS_EXCEEDED = "sending limits exceeded" + SCHEDULED = "scheduled" + CANCELLED = "cancelled" + READY_TO_SEND = "ready to send" + SENT_TO_DVLA = "sent to dvla" + + +class InvitedUserStatus(Enum): + ACCEPTED = "accepted" + CANCELLED = "cancelled" + EXPIRED = "expired" + + +class InvitedOrgUserStatus(Enum): + ACCEPTED = "accepted" + CANCELLED = "cancelled" + + +class VerificationStatus(Enum): + PENDING = "pending" + SUCCESS = "success" + + +class HealthStatus(Enum): + OK = "ok" + ERROR = "error" + + +class AuthType(Enum): + EMAIL_AUTH = "email_auth" + SMS_AUTH = "sms_auth" + + +# TODO: UserRole enum +# class UserRole(Enum): +# ADMIN = "admin" +# USER = "user" +# GUEST = "guest" + + +# TODO: NotificationType enum +# class NotificationType(Enum): +# EMAIL = "email" +# SMS = "sms" +# PUSH = "push" diff --git a/app/main/views/invites.py b/app/main/views/invites.py index 420630535..1cc9a04c1 100644 --- a/app/main/views/invites.py +++ b/app/main/views/invites.py @@ -6,6 +6,7 @@ from app.main import main from app.models.organization import Organization from app.models.service import Service from app.models.user import InvitedOrgUser, InvitedUser, OrganizationUsers, User, Users +from app.enums import InvitedUserStatus, InvitedOrgUserStatus @main.route("/invitation/") @@ -29,14 +30,14 @@ def accept_invite(token): abort(403) - if invited_user.status == "cancelled": + if invited_user.status == InvitedUserStatus.CANCELLED.value: service = Service.from_id(invited_user.service) return render_template( "views/cancelled-invitation.html", from_user=invited_user.from_user.name, service_name=service.name, ) - if invited_user.status == "accepted": + if invited_user.status == InvitedUserStatus.ACCEPTED.value: session.pop("invited_user_id", None) service = Service.from_id(invited_user.service) return redirect( @@ -104,7 +105,7 @@ def accept_org_invite(token): abort(403) - if invited_org_user.status == "cancelled": + if invited_org_user.status == InvitedOrgUserStatus.CANCELLED.value: organization = Organization.from_id(invited_org_user.organization) return render_template( "views/cancelled-invitation.html", @@ -112,7 +113,7 @@ def accept_org_invite(token): organization_name=organization.name, ) - if invited_org_user.status == "accepted": + if invited_org_user.status == InvitedOrgUserStatus.ACCEPTED.value: session.pop("invited_org_user_id", None) return redirect( url_for("main.organization_dashboard", org_id=invited_org_user.organization) diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index d4ed28c18..8ba519f04 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -23,6 +23,7 @@ from app import ( notification_api_client, service_api_client, ) +from app.enums import JobStatus from app.formatters import get_time_left, message_count_noun from app.main import main from app.main.forms import SearchNotificationsForm @@ -398,7 +399,7 @@ def get_job_partials(job): counts=_get_job_counts(job), status=filter_args["status"], notifications_deleted=( - job.status == "finished" and not notifications["notifications"] + job.status == JobStatus.FINISHED.value and not notifications["notifications"] ), ) service_data_retention_days = current_service.get_days_of_retention( diff --git a/app/main/views/register.py b/app/main/views/register.py index 14858d3ec..14ddef5e6 100644 --- a/app/main/views/register.py +++ b/app/main/views/register.py @@ -15,6 +15,7 @@ from flask import ( ) from app import redis_client, user_api_client +from app.enums import InvitedUserStatus from app.main import main from app.main.forms import ( RegisterUserFromOrgInviteForm, @@ -254,14 +255,14 @@ def get_invited_user_email_address(invited_user_id): def invited_user_accept_invite(invited_user_id): invited_user = InvitedUser.by_id(invited_user_id) - if invited_user.status == "expired": + if invited_user.status == InvitedUserStatus.EXPIRED.value: current_app.logger.error("User invitation has expired") flash( "Your invitation has expired; please contact the person who invited you for additional help." ) abort(401) - if invited_user.status == "cancelled": + if invited_user.status == InvitedUserStatus.CANCELLED.value: current_app.logger.error("User invitation has been cancelled") flash( "Your invitation is no longer valid; please contact the person who invited you for additional help." diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index fd15b1854..0c51dbf02 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -52,6 +52,7 @@ from app.utils import DELIVERED_STATUSES, FAILURE_STATUSES, SENDING_STATUSES from app.utils.time import parse_naive_dt from app.utils.user import user_has_permissions, user_is_platform_admin from notifications_python_client.errors import HTTPError +from app.enums import VerificationStatus PLATFORM_ADMIN_SERVICE_PERMISSIONS = OrderedDict( [ @@ -397,10 +398,10 @@ def get_service_verify_reply_to_address_partials(service_id, notification_id): if replace: existing = current_service.get_email_reply_to_address(replace) existing_is_default = existing["is_default"] - verification_status = "pending" + verification_status = VerificationStatus.PENDING.value is_default = True if (request.args.get("is_default", False) == "True") else False if notification["status"] in DELIVERED_STATUSES: - verification_status = "success" + verification_status = VerificationStatus.SUCCESS.value if notification["to"] not in [ i["email_address"] for i in current_service.email_reply_to_addresses ]: @@ -441,7 +442,7 @@ def get_service_verify_reply_to_address_partials(service_id, notification_id): first_email_address=first_email_address, replace=replace, ), - "stop": 0 if verification_status == "pending" else 1, + "stop": 0 if verification_status == VerificationStatus.PENDING.value else 1, } diff --git a/app/models/job.py b/app/models/job.py index b31af24fe..4ebbb2d4b 100644 --- a/app/models/job.py +++ b/app/models/job.py @@ -1,5 +1,6 @@ from werkzeug.utils import cached_property +from app.enums import JobStatus, NotificationStatus from app.models import JSONModel, ModelList, PaginatedModelList from app.notify_client.job_api_client import job_api_client from app.notify_client.notification_api_client import notification_api_client @@ -32,11 +33,11 @@ class Job(JSONModel): @property def cancelled(self): - return self.status == "cancelled" + return self.status == JobStatus.CANCELLED.value @property def scheduled(self): - return self.status == "scheduled" + return self.status == JobStatus.SCHEDULED.value @property def scheduled_for(self): @@ -61,16 +62,18 @@ class Job(JSONModel): @property def notifications_delivered(self): - return self._aggregate_statistics("delivered", "sent") + return self._aggregate_statistics( + NotificationStatus.DELIVERED.value, NotificationStatus.SENT.value + ) @property def notifications_failed(self): return self._aggregate_statistics( - "failed", - "technical-failure", - "temporary-failure", - "permanent-failure", - "cancelled", + NotificationStatus.FAILED.value, + NotificationStatus.TECHNICAL_FAILURE.value, + NotificationStatus.TEMPORARY_FAILURE.value, + NotificationStatus.PERMANENT_FAILURE.value, + NotificationStatus.CANCELLED.value, ) @property @@ -95,7 +98,7 @@ class Job(JSONModel): @property def still_processing(self): - return self.status != "finished" or self.percentage_complete < 100 + return self.status != JobStatus.FINISHED.value or self.percentage_complete < 100 @cached_property def finished_processing(self): diff --git a/app/models/user.py b/app/models/user.py index 88babe2fa..5d5d961ec 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -22,6 +22,7 @@ from app.utils.user_permissions import ( translate_permissions_from_db_to_ui, ) from notifications_python_client.errors import HTTPError +from app.enums import InvitedUserStatus def _get_service_id_from_view_args(): @@ -566,7 +567,7 @@ class InvitedUser(JSONModel): # current_app.logger.warning( # f"Checking invited user {self.id} for permissions: {permissions}" # ) - if self.status == "cancelled": + if self.status == InvitedUserStatus.CANCELLED.value: return False return set(self.permissions) > set(permissions) @@ -574,7 +575,7 @@ class InvitedUser(JSONModel): # current_app.logger.warn( # f"Checking invited user {self.id} for permission: {permission} on service {service_id}" # ) - if self.status == "cancelled": + if self.status == InvitedUserStatus.CANCELLED.value: return False return self.service == service_id and permission in self.permissions diff --git a/app/notify_client/api_key_api_client.py b/app/notify_client/api_key_api_client.py index 0383e8f8c..20b323bb6 100644 --- a/app/notify_client/api_key_api_client.py +++ b/app/notify_client/api_key_api_client.py @@ -1,9 +1,10 @@ from app.notify_client import NotifyAdminAPIClient, _attach_current_user +from app.enums import ApiKeyType # must match key types in notifications-api/app/models.py -KEY_TYPE_NORMAL = "normal" -KEY_TYPE_TEAM = "team" -KEY_TYPE_TEST = "test" +KEY_TYPE_NORMAL = ApiKeyType.NORMAL.value +KEY_TYPE_TEAM = ApiKeyType.TEAM.value +KEY_TYPE_TEST = ApiKeyType.TEST.value class ApiKeyApiClient(NotifyAdminAPIClient): diff --git a/app/notify_client/invite_api_client.py b/app/notify_client/invite_api_client.py index 4ed5eee11..4bc877f44 100644 --- a/app/notify_client/invite_api_client.py +++ b/app/notify_client/invite_api_client.py @@ -11,6 +11,7 @@ from app.utils.user_permissions import ( translate_permissions_from_ui_to_db, ) from notifications_utils.url_safe_token import generate_token +from app.enums import InvitedUserStatus class InviteApiClient(NotifyAdminAPIClient): @@ -94,7 +95,7 @@ class InviteApiClient(NotifyAdminAPIClient): return self.get(url=f"/invite/service/check/{token}")["data"] def cancel_invited_user(self, service_id, invited_user_id): - data = {"status": "cancelled"} + data = {"status": InvitedUserStatus.CANCELLED.value} data = _attach_current_user(data) self.post(url=f"/service/{service_id}/invite/{invited_user_id}", data=data) @@ -131,7 +132,7 @@ class InviteApiClient(NotifyAdminAPIClient): @cache.delete("service-{service_id}") @cache.delete("user-{invited_user_id}") def accept_invite(self, service_id, invited_user_id): - data = {"status": "accepted"} + data = {"status": InvitedUserStatus.ACCEPTED.value} self.post(url=f"/service/{service_id}/invite/{invited_user_id}", data=data) diff --git a/app/notify_client/job_api_client.py b/app/notify_client/job_api_client.py index 9a06e16bf..47f42aeeb 100644 --- a/app/notify_client/job_api_client.py +++ b/app/notify_client/job_api_client.py @@ -4,21 +4,22 @@ from zoneinfo import ZoneInfo from app.extensions import redis_client from app.notify_client import NotifyAdminAPIClient, _attach_current_user, cache from app.utils.csv import get_user_preferred_timezone +from app.enums import JobStatus class JobApiClient(NotifyAdminAPIClient): JOB_STATUSES = { - "scheduled", - "pending", - "in progress", - "finished", - "cancelled", - "sending limits exceeded", - "ready to send", - "sent to dvla", + JobStatus.SCHEDULED.value, + JobStatus.PENDING.value, + JobStatus.IN_PROGRESS.value, + JobStatus.FINISHED.value, + JobStatus.CANCELLED.value, + JobStatus.SENDING_LIMITS_EXCEEDED.value, + JobStatus.READY_TO_SEND.value, + JobStatus.SENT_TO_DVLA.value, } - SCHEDULED_JOB_STATUS = "scheduled" - CANCELLED_JOB_STATUS = "cancelled" + SCHEDULED_JOB_STATUS = JobStatus.SCHEDULED.value + CANCELLED_JOB_STATUS = JobStatus.CANCELLED.value NON_CANCELLED_JOB_STATUSES = JOB_STATUSES - {CANCELLED_JOB_STATUS} NON_SCHEDULED_JOB_STATUSES = JOB_STATUSES - { SCHEDULED_JOB_STATUS, @@ -57,7 +58,7 @@ class JobApiClient(NotifyAdminAPIClient): job["original_file_name"], ) for job in self.get_jobs(service_id, limit_days=0)["data"] - if job["job_status"] != "cancelled" + if job["job_status"] != JobStatus.CANCELLED.value ) def get_page_of_jobs(self, service_id, *, page, statuses=None, limit_days=None): diff --git a/app/notify_client/org_invite_api_client.py b/app/notify_client/org_invite_api_client.py index d85787748..38071e77a 100644 --- a/app/notify_client/org_invite_api_client.py +++ b/app/notify_client/org_invite_api_client.py @@ -1,4 +1,5 @@ from app.notify_client import NotifyAdminAPIClient, _attach_current_user +from app.enums import InvitedOrgUserStatus class OrgInviteApiClient(NotifyAdminAPIClient): @@ -33,7 +34,7 @@ class OrgInviteApiClient(NotifyAdminAPIClient): return resp["data"] def cancel_invited_user(self, org_id, invited_user_id): - data = {"status": "cancelled"} + data = {"status": InvitedOrgUserStatus.CANCELLED.value} data = _attach_current_user(data) self.post( url="/organization/{0}/invite/{1}".format(org_id, invited_user_id), @@ -41,7 +42,7 @@ class OrgInviteApiClient(NotifyAdminAPIClient): ) def accept_invite(self, org_id, invited_user_id): - data = {"status": "accepted"} + data = {"status": InvitedOrgUserStatus.ACCEPTED.value} self.post( url="/organization/{0}/invite/{1}".format(org_id, invited_user_id), data=data, diff --git a/app/status/views/healthcheck.py b/app/status/views/healthcheck.py index 6bb62cf03..f22d209da 100644 --- a/app/status/views/healthcheck.py +++ b/app/status/views/healthcheck.py @@ -5,6 +5,7 @@ from flask import current_app, jsonify, request from redis import RedisError from app import status_api_client, version +from app.enums import HealthStatus from app.extensions import redis_client from app.status import status from notifications_python_client.errors import HTTPError @@ -13,16 +14,16 @@ from notifications_python_client.errors import HTTPError @status.route("/_status", methods=["GET"]) def show_status(): if request.args.get("elb", None) or request.args.get("simple", None): - return jsonify(status="ok"), 200 + return jsonify(status=HealthStatus.OK.value), 200 else: try: api_status = status_api_client.get_status() except HTTPError as err: current_app.logger.exception("API failed to respond") - return jsonify(status="error", message=str(err.message)), 500 + return jsonify(status=HealthStatus.ERROR.value, message=str(err.message)), 500 return ( jsonify( - status="ok", + status=HealthStatus.OK.value, api=api_status, git_commit=version.__git_commit__, build_time=version.__time__, @@ -60,10 +61,10 @@ def show_redis_status(): ) except HTTPError as err: current_app.logger.exception("API failed to respond") - return jsonify(status="error", message=str(err.message)), 500 + return jsonify(status=HealthStatus.ERROR.value, message=str(err.message)), 500 return ( jsonify( - status="ok", + status=HealthStatus.OK.value, api=api_status, git_commit=version.__git_commit__, build_time=version.__time__, @@ -76,7 +77,7 @@ def show_redis_status(): ) return ( jsonify( - status=f"error: {err}", + status=f"{HealthStatus.ERROR.value}: {err}", api=api_status, git_commit=version.__git_commit__, build_time=version.__time__, From 8a41de8f0779a94c5d134b812a15cc6a682bdcdd Mon Sep 17 00:00:00 2001 From: Beverly Nguyen Date: Fri, 4 Jul 2025 17:22:21 -0700 Subject: [PATCH 2/6] Using StrEnum to drop .value --- app/enums.py | 18 +++++++++--------- app/main/views/invites.py | 8 ++++---- app/main/views/register.py | 4 ++-- app/models/job.py | 18 +++++++++--------- app/notify_client/api_key_api_client.py | 6 +++--- app/notify_client/job_api_client.py | 20 ++++++++++---------- 6 files changed, 37 insertions(+), 37 deletions(-) diff --git a/app/enums.py b/app/enums.py index 48474d6dc..f5a8e7f64 100644 --- a/app/enums.py +++ b/app/enums.py @@ -1,7 +1,7 @@ -from enum import Enum +from enum import Enum, StrEnum -class NotificationStatus(Enum): +class NotificationStatus(StrEnum): CREATED = "created" PENDING = "pending" SENDING = "sending" @@ -17,13 +17,13 @@ class NotificationStatus(Enum): CANCELLED = "cancelled" -class ApiKeyType(Enum): +class ApiKeyType(StrEnum): NORMAL = "normal" TEAM = "team" TEST = "test" -class JobStatus(Enum): +class JobStatus(StrEnum): PENDING = "pending" IN_PROGRESS = "in progress" FINISHED = "finished" @@ -34,28 +34,28 @@ class JobStatus(Enum): SENT_TO_DVLA = "sent to dvla" -class InvitedUserStatus(Enum): +class InvitedUserStatus(StrEnum): ACCEPTED = "accepted" CANCELLED = "cancelled" EXPIRED = "expired" -class InvitedOrgUserStatus(Enum): +class InvitedOrgUserStatus(StrEnum): ACCEPTED = "accepted" CANCELLED = "cancelled" -class VerificationStatus(Enum): +class VerificationStatus(StrEnum): PENDING = "pending" SUCCESS = "success" -class HealthStatus(Enum): +class HealthStatus(StrEnum): OK = "ok" ERROR = "error" -class AuthType(Enum): +class AuthType(StrEnum): EMAIL_AUTH = "email_auth" SMS_AUTH = "sms_auth" diff --git a/app/main/views/invites.py b/app/main/views/invites.py index 1cc9a04c1..8aa2c73dd 100644 --- a/app/main/views/invites.py +++ b/app/main/views/invites.py @@ -30,14 +30,14 @@ def accept_invite(token): abort(403) - if invited_user.status == InvitedUserStatus.CANCELLED.value: + if invited_user.status == InvitedUserStatus.CANCELLED: service = Service.from_id(invited_user.service) return render_template( "views/cancelled-invitation.html", from_user=invited_user.from_user.name, service_name=service.name, ) - if invited_user.status == InvitedUserStatus.ACCEPTED.value: + if invited_user.status == InvitedUserStatus.ACCEPTED: session.pop("invited_user_id", None) service = Service.from_id(invited_user.service) return redirect( @@ -105,7 +105,7 @@ def accept_org_invite(token): abort(403) - if invited_org_user.status == InvitedOrgUserStatus.CANCELLED.value: + if invited_org_user.status == InvitedOrgUserStatus.CANCELLED: organization = Organization.from_id(invited_org_user.organization) return render_template( "views/cancelled-invitation.html", @@ -113,7 +113,7 @@ def accept_org_invite(token): organization_name=organization.name, ) - if invited_org_user.status == InvitedOrgUserStatus.ACCEPTED.value: + if invited_org_user.status == InvitedOrgUserStatus.ACCEPTED: session.pop("invited_org_user_id", None) return redirect( url_for("main.organization_dashboard", org_id=invited_org_user.organization) diff --git a/app/main/views/register.py b/app/main/views/register.py index 14ddef5e6..cd5b1682d 100644 --- a/app/main/views/register.py +++ b/app/main/views/register.py @@ -255,14 +255,14 @@ def get_invited_user_email_address(invited_user_id): def invited_user_accept_invite(invited_user_id): invited_user = InvitedUser.by_id(invited_user_id) - if invited_user.status == InvitedUserStatus.EXPIRED.value: + if invited_user.status == InvitedUserStatus.EXPIRED: current_app.logger.error("User invitation has expired") flash( "Your invitation has expired; please contact the person who invited you for additional help." ) abort(401) - if invited_user.status == InvitedUserStatus.CANCELLED.value: + if invited_user.status == InvitedUserStatus.CANCELLED: current_app.logger.error("User invitation has been cancelled") flash( "Your invitation is no longer valid; please contact the person who invited you for additional help." diff --git a/app/models/job.py b/app/models/job.py index 4ebbb2d4b..adf59945b 100644 --- a/app/models/job.py +++ b/app/models/job.py @@ -33,11 +33,11 @@ class Job(JSONModel): @property def cancelled(self): - return self.status == JobStatus.CANCELLED.value + return self.status == JobStatus.CANCELLED @property def scheduled(self): - return self.status == JobStatus.SCHEDULED.value + return self.status == JobStatus.SCHEDULED @property def scheduled_for(self): @@ -63,17 +63,17 @@ class Job(JSONModel): @property def notifications_delivered(self): return self._aggregate_statistics( - NotificationStatus.DELIVERED.value, NotificationStatus.SENT.value + NotificationStatus.DELIVERED, NotificationStatus.SENT ) @property def notifications_failed(self): return self._aggregate_statistics( - NotificationStatus.FAILED.value, - NotificationStatus.TECHNICAL_FAILURE.value, - NotificationStatus.TEMPORARY_FAILURE.value, - NotificationStatus.PERMANENT_FAILURE.value, - NotificationStatus.CANCELLED.value, + NotificationStatus.FAILED, + NotificationStatus.TECHNICAL_FAILURE, + NotificationStatus.TEMPORARY_FAILURE, + NotificationStatus.PERMANENT_FAILURE, + NotificationStatus.CANCELLED, ) @property @@ -98,7 +98,7 @@ class Job(JSONModel): @property def still_processing(self): - return self.status != JobStatus.FINISHED.value or self.percentage_complete < 100 + return self.status != JobStatus.FINISHED or self.percentage_complete < 100 @cached_property def finished_processing(self): diff --git a/app/notify_client/api_key_api_client.py b/app/notify_client/api_key_api_client.py index 20b323bb6..f56a2c307 100644 --- a/app/notify_client/api_key_api_client.py +++ b/app/notify_client/api_key_api_client.py @@ -2,9 +2,9 @@ from app.notify_client import NotifyAdminAPIClient, _attach_current_user from app.enums import ApiKeyType # must match key types in notifications-api/app/models.py -KEY_TYPE_NORMAL = ApiKeyType.NORMAL.value -KEY_TYPE_TEAM = ApiKeyType.TEAM.value -KEY_TYPE_TEST = ApiKeyType.TEST.value +KEY_TYPE_NORMAL = ApiKeyType.NORMAL +KEY_TYPE_TEAM = ApiKeyType.TEAM +KEY_TYPE_TEST = ApiKeyType.TEST class ApiKeyApiClient(NotifyAdminAPIClient): diff --git a/app/notify_client/job_api_client.py b/app/notify_client/job_api_client.py index 47f42aeeb..953d21e51 100644 --- a/app/notify_client/job_api_client.py +++ b/app/notify_client/job_api_client.py @@ -9,17 +9,17 @@ from app.enums import JobStatus class JobApiClient(NotifyAdminAPIClient): JOB_STATUSES = { - JobStatus.SCHEDULED.value, - JobStatus.PENDING.value, - JobStatus.IN_PROGRESS.value, - JobStatus.FINISHED.value, - JobStatus.CANCELLED.value, - JobStatus.SENDING_LIMITS_EXCEEDED.value, - JobStatus.READY_TO_SEND.value, - JobStatus.SENT_TO_DVLA.value, + JobStatus.SCHEDULED, + JobStatus.PENDING, + JobStatus.IN_PROGRESS, + JobStatus.FINISHED, + JobStatus.CANCELLED, + JobStatus.SENDING_LIMITS_EXCEEDED, + JobStatus.READY_TO_SEND, + JobStatus.SENT_TO_DVLA, } - SCHEDULED_JOB_STATUS = JobStatus.SCHEDULED.value - CANCELLED_JOB_STATUS = JobStatus.CANCELLED.value + SCHEDULED_JOB_STATUS = JobStatus.SCHEDULED + CANCELLED_JOB_STATUS = JobStatus.CANCELLED NON_CANCELLED_JOB_STATUSES = JOB_STATUSES - {CANCELLED_JOB_STATUS} NON_SCHEDULED_JOB_STATUSES = JOB_STATUSES - { SCHEDULED_JOB_STATUS, From 9d9fa1c3940891e939283a8f574e2f92dd585c3d Mon Sep 17 00:00:00 2001 From: Beverly Nguyen Date: Fri, 4 Jul 2025 17:30:30 -0700 Subject: [PATCH 3/6] removed .value because we have StrEnum --- app/enums.py | 10 +++++----- app/main/views/activity.py | 8 ++++---- app/main/views/jobs.py | 2 +- app/main/views/service_settings.py | 6 +++--- app/models/user.py | 4 ++-- app/notify_client/invite_api_client.py | 4 ++-- app/status/views/healthcheck.py | 12 ++++++------ 7 files changed, 23 insertions(+), 23 deletions(-) diff --git a/app/enums.py b/app/enums.py index f5a8e7f64..02fbb0e9a 100644 --- a/app/enums.py +++ b/app/enums.py @@ -1,4 +1,4 @@ -from enum import Enum, StrEnum +from enum import StrEnum class NotificationStatus(StrEnum): @@ -60,15 +60,15 @@ class AuthType(StrEnum): SMS_AUTH = "sms_auth" -# TODO: UserRole enum -# class UserRole(Enum): +# TODO: +# class UserRole(StrEnum): # ADMIN = "admin" # USER = "user" # GUEST = "guest" -# TODO: NotificationType enum -# class NotificationType(Enum): +# TODO: +# class NotificationType(StrEnum): # EMAIL = "email" # SMS = "sms" # PUSH = "push" diff --git a/app/main/views/activity.py b/app/main/views/activity.py index a06a69ddf..e43df4e04 100644 --- a/app/main/views/activity.py +++ b/app/main/views/activity.py @@ -1,6 +1,7 @@ from flask import abort, render_template, request, url_for from app import current_service, job_api_client +from app.enums import NotificationStatus from app.formatters import get_time_left from app.main import main from app.utils.pagination import ( @@ -78,8 +79,7 @@ def handle_pagination(jobs, service_id, page): return prev_page, next_page, pagination -JOB_STATUS_DELIVERED = "delivered" -JOB_STATUS_FAILED = "failed" + def get_job_statistics(job, status): @@ -109,8 +109,8 @@ def create_job_dict_entry(job): "activity_time": activity_time, "created_by": job.get("created_by"), "template_name": job.get("template_name"), - "delivered_count": get_job_statistics(job, JOB_STATUS_DELIVERED), - "failed_count": get_job_statistics(job, JOB_STATUS_FAILED), + "delivered_count": get_job_statistics(job, NotificationStatus.DELIVERED), + "failed_count": get_job_statistics(job, NotificationStatus.FAILED), } diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 8ba519f04..d9e282122 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -399,7 +399,7 @@ def get_job_partials(job): counts=_get_job_counts(job), status=filter_args["status"], notifications_deleted=( - job.status == JobStatus.FINISHED.value and not notifications["notifications"] + job.status == JobStatus.FINISHED and not notifications["notifications"] ), ) service_data_retention_days = current_service.get_days_of_retention( diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 0c51dbf02..899451844 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -398,10 +398,10 @@ def get_service_verify_reply_to_address_partials(service_id, notification_id): if replace: existing = current_service.get_email_reply_to_address(replace) existing_is_default = existing["is_default"] - verification_status = VerificationStatus.PENDING.value + verification_status = VerificationStatus.PENDING is_default = True if (request.args.get("is_default", False) == "True") else False if notification["status"] in DELIVERED_STATUSES: - verification_status = VerificationStatus.SUCCESS.value + verification_status = VerificationStatus.SUCCESS if notification["to"] not in [ i["email_address"] for i in current_service.email_reply_to_addresses ]: @@ -442,7 +442,7 @@ def get_service_verify_reply_to_address_partials(service_id, notification_id): first_email_address=first_email_address, replace=replace, ), - "stop": 0 if verification_status == VerificationStatus.PENDING.value else 1, + "stop": 0 if verification_status == VerificationStatus.PENDING else 1, } diff --git a/app/models/user.py b/app/models/user.py index 5d5d961ec..b6f732f07 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -567,7 +567,7 @@ class InvitedUser(JSONModel): # current_app.logger.warning( # f"Checking invited user {self.id} for permissions: {permissions}" # ) - if self.status == InvitedUserStatus.CANCELLED.value: + if self.status == InvitedUserStatus.CANCELLED: return False return set(self.permissions) > set(permissions) @@ -575,7 +575,7 @@ class InvitedUser(JSONModel): # current_app.logger.warn( # f"Checking invited user {self.id} for permission: {permission} on service {service_id}" # ) - if self.status == InvitedUserStatus.CANCELLED.value: + if self.status == InvitedUserStatus.CANCELLED: return False return self.service == service_id and permission in self.permissions diff --git a/app/notify_client/invite_api_client.py b/app/notify_client/invite_api_client.py index 4bc877f44..700c1c701 100644 --- a/app/notify_client/invite_api_client.py +++ b/app/notify_client/invite_api_client.py @@ -95,7 +95,7 @@ class InviteApiClient(NotifyAdminAPIClient): return self.get(url=f"/invite/service/check/{token}")["data"] def cancel_invited_user(self, service_id, invited_user_id): - data = {"status": InvitedUserStatus.CANCELLED.value} + data = {"status": InvitedUserStatus.CANCELLED} data = _attach_current_user(data) self.post(url=f"/service/{service_id}/invite/{invited_user_id}", data=data) @@ -132,7 +132,7 @@ class InviteApiClient(NotifyAdminAPIClient): @cache.delete("service-{service_id}") @cache.delete("user-{invited_user_id}") def accept_invite(self, service_id, invited_user_id): - data = {"status": InvitedUserStatus.ACCEPTED.value} + data = {"status": InvitedUserStatus.ACCEPTED} self.post(url=f"/service/{service_id}/invite/{invited_user_id}", data=data) diff --git a/app/status/views/healthcheck.py b/app/status/views/healthcheck.py index f22d209da..7e06d823f 100644 --- a/app/status/views/healthcheck.py +++ b/app/status/views/healthcheck.py @@ -14,16 +14,16 @@ from notifications_python_client.errors import HTTPError @status.route("/_status", methods=["GET"]) def show_status(): if request.args.get("elb", None) or request.args.get("simple", None): - return jsonify(status=HealthStatus.OK.value), 200 + return jsonify(status=HealthStatus.OK), 200 else: try: api_status = status_api_client.get_status() except HTTPError as err: current_app.logger.exception("API failed to respond") - return jsonify(status=HealthStatus.ERROR.value, message=str(err.message)), 500 + return jsonify(status=HealthStatus.ERROR, message=str(err.message)), 500 return ( jsonify( - status=HealthStatus.OK.value, + status=HealthStatus.OK, api=api_status, git_commit=version.__git_commit__, build_time=version.__time__, @@ -61,10 +61,10 @@ def show_redis_status(): ) except HTTPError as err: current_app.logger.exception("API failed to respond") - return jsonify(status=HealthStatus.ERROR.value, message=str(err.message)), 500 + return jsonify(status=HealthStatus.ERROR, message=str(err.message)), 500 return ( jsonify( - status=HealthStatus.OK.value, + status=HealthStatus.OK, api=api_status, git_commit=version.__git_commit__, build_time=version.__time__, @@ -77,7 +77,7 @@ def show_redis_status(): ) return ( jsonify( - status=f"{HealthStatus.ERROR.value}: {err}", + status=f"{HealthStatus.ERROR}: {err}", api=api_status, git_commit=version.__git_commit__, build_time=version.__time__, From ee2f777904db5860f54a0efb10ddb8873842850b Mon Sep 17 00:00:00 2001 From: Beverly Nguyen Date: Fri, 4 Jul 2025 17:33:25 -0700 Subject: [PATCH 4/6] remove healthcheck.py changes --- app/enums.py | 5 ----- app/status/views/healthcheck.py | 13 ++++++------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/app/enums.py b/app/enums.py index 02fbb0e9a..405c28cdf 100644 --- a/app/enums.py +++ b/app/enums.py @@ -50,11 +50,6 @@ class VerificationStatus(StrEnum): SUCCESS = "success" -class HealthStatus(StrEnum): - OK = "ok" - ERROR = "error" - - class AuthType(StrEnum): EMAIL_AUTH = "email_auth" SMS_AUTH = "sms_auth" diff --git a/app/status/views/healthcheck.py b/app/status/views/healthcheck.py index 7e06d823f..6bb62cf03 100644 --- a/app/status/views/healthcheck.py +++ b/app/status/views/healthcheck.py @@ -5,7 +5,6 @@ from flask import current_app, jsonify, request from redis import RedisError from app import status_api_client, version -from app.enums import HealthStatus from app.extensions import redis_client from app.status import status from notifications_python_client.errors import HTTPError @@ -14,16 +13,16 @@ from notifications_python_client.errors import HTTPError @status.route("/_status", methods=["GET"]) def show_status(): if request.args.get("elb", None) or request.args.get("simple", None): - return jsonify(status=HealthStatus.OK), 200 + return jsonify(status="ok"), 200 else: try: api_status = status_api_client.get_status() except HTTPError as err: current_app.logger.exception("API failed to respond") - return jsonify(status=HealthStatus.ERROR, message=str(err.message)), 500 + return jsonify(status="error", message=str(err.message)), 500 return ( jsonify( - status=HealthStatus.OK, + status="ok", api=api_status, git_commit=version.__git_commit__, build_time=version.__time__, @@ -61,10 +60,10 @@ def show_redis_status(): ) except HTTPError as err: current_app.logger.exception("API failed to respond") - return jsonify(status=HealthStatus.ERROR, message=str(err.message)), 500 + return jsonify(status="error", message=str(err.message)), 500 return ( jsonify( - status=HealthStatus.OK, + status="ok", api=api_status, git_commit=version.__git_commit__, build_time=version.__time__, @@ -77,7 +76,7 @@ def show_redis_status(): ) return ( jsonify( - status=f"{HealthStatus.ERROR}: {err}", + status=f"error: {err}", api=api_status, git_commit=version.__git_commit__, build_time=version.__time__, From fa9e45fbcde6de6f1d1e8261dda1750062d1df99 Mon Sep 17 00:00:00 2001 From: Beverly Nguyen Date: Fri, 4 Jul 2025 17:42:20 -0700 Subject: [PATCH 5/6] .value removed --- app/notify_client/org_invite_api_client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/notify_client/org_invite_api_client.py b/app/notify_client/org_invite_api_client.py index 38071e77a..d6c8fe186 100644 --- a/app/notify_client/org_invite_api_client.py +++ b/app/notify_client/org_invite_api_client.py @@ -34,7 +34,7 @@ class OrgInviteApiClient(NotifyAdminAPIClient): return resp["data"] def cancel_invited_user(self, org_id, invited_user_id): - data = {"status": InvitedOrgUserStatus.CANCELLED.value} + data = {"status": InvitedOrgUserStatus.CANCELLED} data = _attach_current_user(data) self.post( url="/organization/{0}/invite/{1}".format(org_id, invited_user_id), @@ -42,7 +42,7 @@ class OrgInviteApiClient(NotifyAdminAPIClient): ) def accept_invite(self, org_id, invited_user_id): - data = {"status": InvitedOrgUserStatus.ACCEPTED.value} + data = {"status": InvitedOrgUserStatus.ACCEPTED} self.post( url="/organization/{0}/invite/{1}".format(org_id, invited_user_id), data=data, From 3130b1399c7fd15d232e8e39bc056be34edbbde1 Mon Sep 17 00:00:00 2001 From: Beverly Nguyen Date: Fri, 4 Jul 2025 17:52:03 -0700 Subject: [PATCH 6/6] isort . --- app/main/views/activity.py | 3 --- app/main/views/invites.py | 2 +- app/main/views/service_settings.py | 2 +- app/models/user.py | 2 +- app/notify_client/api_key_api_client.py | 2 +- app/notify_client/invite_api_client.py | 2 +- app/notify_client/job_api_client.py | 4 ++-- app/notify_client/org_invite_api_client.py | 2 +- app/utils/__init__.py | 19 ++++++++++++------- 9 files changed, 20 insertions(+), 18 deletions(-) diff --git a/app/main/views/activity.py b/app/main/views/activity.py index e43df4e04..661d55afb 100644 --- a/app/main/views/activity.py +++ b/app/main/views/activity.py @@ -79,9 +79,6 @@ def handle_pagination(jobs, service_id, page): return prev_page, next_page, pagination - - - def get_job_statistics(job, status): statistics = job.get("statistics", []) for stat in statistics: diff --git a/app/main/views/invites.py b/app/main/views/invites.py index 8aa2c73dd..6831f1db4 100644 --- a/app/main/views/invites.py +++ b/app/main/views/invites.py @@ -2,11 +2,11 @@ from flask import abort, flash, redirect, render_template, session, url_for from flask_login import current_user from markupsafe import Markup +from app.enums import InvitedOrgUserStatus, InvitedUserStatus from app.main import main from app.models.organization import Organization from app.models.service import Service from app.models.user import InvitedOrgUser, InvitedUser, OrganizationUsers, User, Users -from app.enums import InvitedUserStatus, InvitedOrgUserStatus @main.route("/invitation/") diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 899451844..5615df690 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -21,6 +21,7 @@ from app import ( organizations_client, service_api_client, ) +from app.enums import VerificationStatus from app.event_handlers import ( create_archive_service_event, create_resume_service_event, @@ -52,7 +53,6 @@ from app.utils import DELIVERED_STATUSES, FAILURE_STATUSES, SENDING_STATUSES from app.utils.time import parse_naive_dt from app.utils.user import user_has_permissions, user_is_platform_admin from notifications_python_client.errors import HTTPError -from app.enums import VerificationStatus PLATFORM_ADMIN_SERVICE_PERMISSIONS = OrderedDict( [ diff --git a/app/models/user.py b/app/models/user.py index b6f732f07..4ed4a62da 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -5,6 +5,7 @@ from flask import abort, current_app, request, session from flask_login import AnonymousUserMixin, UserMixin, login_user, logout_user from werkzeug.utils import cached_property +from app.enums import InvitedUserStatus from app.event_handlers import ( create_add_user_to_service_event, create_set_user_permissions_event, @@ -22,7 +23,6 @@ from app.utils.user_permissions import ( translate_permissions_from_db_to_ui, ) from notifications_python_client.errors import HTTPError -from app.enums import InvitedUserStatus def _get_service_id_from_view_args(): diff --git a/app/notify_client/api_key_api_client.py b/app/notify_client/api_key_api_client.py index f56a2c307..540693673 100644 --- a/app/notify_client/api_key_api_client.py +++ b/app/notify_client/api_key_api_client.py @@ -1,5 +1,5 @@ -from app.notify_client import NotifyAdminAPIClient, _attach_current_user from app.enums import ApiKeyType +from app.notify_client import NotifyAdminAPIClient, _attach_current_user # must match key types in notifications-api/app/models.py KEY_TYPE_NORMAL = ApiKeyType.NORMAL diff --git a/app/notify_client/invite_api_client.py b/app/notify_client/invite_api_client.py index 700c1c701..a9a6695fb 100644 --- a/app/notify_client/invite_api_client.py +++ b/app/notify_client/invite_api_client.py @@ -5,13 +5,13 @@ from urllib.parse import unquote from flask import current_app, request from app import redis_client +from app.enums import InvitedUserStatus from app.notify_client import NotifyAdminAPIClient, _attach_current_user, cache from app.utils.user_permissions import ( all_ui_permissions, translate_permissions_from_ui_to_db, ) from notifications_utils.url_safe_token import generate_token -from app.enums import InvitedUserStatus class InviteApiClient(NotifyAdminAPIClient): diff --git a/app/notify_client/job_api_client.py b/app/notify_client/job_api_client.py index 953d21e51..0e04cfc74 100644 --- a/app/notify_client/job_api_client.py +++ b/app/notify_client/job_api_client.py @@ -1,10 +1,10 @@ import datetime from zoneinfo import ZoneInfo +from app.enums import JobStatus from app.extensions import redis_client from app.notify_client import NotifyAdminAPIClient, _attach_current_user, cache from app.utils.csv import get_user_preferred_timezone -from app.enums import JobStatus class JobApiClient(NotifyAdminAPIClient): @@ -58,7 +58,7 @@ class JobApiClient(NotifyAdminAPIClient): job["original_file_name"], ) for job in self.get_jobs(service_id, limit_days=0)["data"] - if job["job_status"] != JobStatus.CANCELLED.value + if job["job_status"] != JobStatus.CANCELLED ) def get_page_of_jobs(self, service_id, *, page, statuses=None, limit_days=None): diff --git a/app/notify_client/org_invite_api_client.py b/app/notify_client/org_invite_api_client.py index d6c8fe186..10b596f40 100644 --- a/app/notify_client/org_invite_api_client.py +++ b/app/notify_client/org_invite_api_client.py @@ -1,5 +1,5 @@ -from app.notify_client import NotifyAdminAPIClient, _attach_current_user from app.enums import InvitedOrgUserStatus +from app.notify_client import NotifyAdminAPIClient, _attach_current_user class OrgInviteApiClient(NotifyAdminAPIClient): diff --git a/app/utils/__init__.py b/app/utils/__init__.py index 6e9c8aa88..b9aaaaa88 100644 --- a/app/utils/__init__.py +++ b/app/utils/__init__.py @@ -7,16 +7,21 @@ from ordered_set import OrderedSet from werkzeug.datastructures import MultiDict from werkzeug.routing import RequestRedirect +from app.enums import NotificationStatus from notifications_utils.field import Field -SENDING_STATUSES = ["created", "pending", "sending"] -DELIVERED_STATUSES = ["delivered", "sent"] +SENDING_STATUSES = [ + NotificationStatus.CREATED, + NotificationStatus.PENDING, + NotificationStatus.SENDING, +] +DELIVERED_STATUSES = [NotificationStatus.DELIVERED, NotificationStatus.SENT] FAILURE_STATUSES = [ - "failed", - "temporary-failure", - "permanent-failure", - "technical-failure", - "validation-failed", + NotificationStatus.FAILED, + NotificationStatus.TEMPORARY_FAILURE, + NotificationStatus.PERMANENT_FAILURE, + NotificationStatus.TECHNICAL_FAILURE, + NotificationStatus.VALIDATION_FAILED, ] REQUESTED_STATUSES = SENDING_STATUSES + DELIVERED_STATUSES + FAILURE_STATUSES