diff --git a/enterprise_access/apps/api/v1/views/browse_and_request.py b/enterprise_access/apps/api/v1/views/browse_and_request.py index 63f2b9e26..9bc7a9171 100644 --- a/enterprise_access/apps/api/v1/views/browse_and_request.py +++ b/enterprise_access/apps/api/v1/views/browse_and_request.py @@ -51,8 +51,9 @@ from enterprise_access.apps.subsidy_access_policy.models import SubsidyAccessPolicy from enterprise_access.apps.subsidy_request.constants import ( REUSABLE_REQUEST_STATES, - LearnerCreditAdditionalActionStates, LearnerCreditRequestActionErrorReasons, + LearnerCreditRequestActionTypes, + LearnerCreditRequestUserMessages, SegmentEvents, SubsidyRequestStates, SubsidyTypeChoices @@ -71,11 +72,7 @@ send_learner_credit_bnr_request_approve_task, send_reminder_email_for_pending_learner_credit_request ) -from enterprise_access.apps.subsidy_request.utils import ( - get_action_choice, - get_error_reason_choice, - get_user_message_choice -) +from enterprise_access.apps.subsidy_request.utils import get_error_reason_choice from enterprise_access.apps.track.segment import track_event from enterprise_access.utils import format_traceback, get_subsidy_model @@ -898,8 +895,8 @@ def create(self, request, *args, **kwargs): self._reuse_existing_request(existing_request, course_price) LearnerCreditRequestActions.create_action( learner_credit_request=existing_request, - recent_action=get_action_choice(SubsidyRequestStates.REQUESTED), - status=get_user_message_choice(SubsidyRequestStates.REQUESTED), + recent_action=LearnerCreditRequestActionTypes.REQUESTED, + status=LearnerCreditRequestUserMessages.REQUESTED, ) # Trigger admin email notification with the latest request send_learner_credit_bnr_admins_email_with_new_requests_task.delay( @@ -941,8 +938,8 @@ def create(self, request, *args, **kwargs): lcr = LearnerCreditRequest.objects.get(uuid=lcr_uuid) LearnerCreditRequestActions.create_action( learner_credit_request=lcr, - recent_action=get_action_choice(SubsidyRequestStates.REQUESTED), - status=get_user_message_choice(SubsidyRequestStates.REQUESTED), + recent_action=LearnerCreditRequestActionTypes.REQUESTED, + status=LearnerCreditRequestUserMessages.REQUESTED, ) # Trigger admin email notification with the latest request @@ -982,8 +979,8 @@ def approve(self, request, *args, **kwargs): # Log "approve" as recent action in the Request Action model. lc_request_action = LearnerCreditRequestActions.create_action( learner_credit_request=lc_request, - recent_action=get_action_choice(SubsidyRequestStates.APPROVED), - status=get_user_message_choice(SubsidyRequestStates.APPROVED), + recent_action=LearnerCreditRequestActionTypes.APPROVED, + status=LearnerCreditRequestUserMessages.APPROVED, ) try: @@ -1014,7 +1011,7 @@ def approve(self, request, *args, **kwargs): logger.exception(error_msg) # Update approve action with error reason. - lc_request_action.status = get_user_message_choice(SubsidyRequestStates.REQUESTED) + lc_request_action.status = LearnerCreditRequestUserMessages.REQUESTED lc_request_action.error_reason = get_error_reason_choice( LearnerCreditRequestActionErrorReasons.FAILED_APPROVAL ) @@ -1044,8 +1041,8 @@ def cancel(self, request, *args, **kwargs): error_msg = None lc_action = LearnerCreditRequestActions.create_action( learner_credit_request=learner_credit_request, - recent_action=get_action_choice(SubsidyRequestStates.CANCELLED), - status=get_user_message_choice(SubsidyRequestStates.CANCELLED), + recent_action=LearnerCreditRequestActionTypes.CANCELLED, + status=LearnerCreditRequestUserMessages.CANCELLED, ) try: @@ -1059,7 +1056,7 @@ def cancel(self, request, *args, **kwargs): lc_action.error_reason = get_error_reason_choice( LearnerCreditRequestActionErrorReasons.FAILED_CANCELLATION ) - lc_action.status = get_user_message_choice(SubsidyRequestStates.APPROVED) + lc_action.status = LearnerCreditRequestUserMessages.APPROVED lc_action.traceback = error_msg lc_action.save() return Response(error_msg, status=status.HTTP_422_UNPROCESSABLE_ENTITY) @@ -1081,7 +1078,7 @@ def cancel(self, request, *args, **kwargs): lc_action.error_reason = get_error_reason_choice( LearnerCreditRequestActionErrorReasons.FAILED_CANCELLATION ) - lc_action.status = get_user_message_choice(SubsidyRequestStates.APPROVED) + lc_action.status = LearnerCreditRequestUserMessages.APPROVED lc_action.traceback = error_msg lc_action.save() return Response(status=status.HTTP_422_UNPROCESSABLE_ENTITY) @@ -1102,8 +1099,8 @@ def remind(self, request, *args, **kwargs): action_instance = LearnerCreditRequestActions.create_action( learner_credit_request=learner_credit_request, - recent_action=get_action_choice(LearnerCreditAdditionalActionStates.REMINDED), - status=get_user_message_choice(LearnerCreditAdditionalActionStates.REMINDED), + recent_action=LearnerCreditRequestActionTypes.REMINDED, + status=LearnerCreditRequestUserMessages.REMINDED, ) try: @@ -1111,8 +1108,11 @@ def remind(self, request, *args, **kwargs): return Response(status=status.HTTP_200_OK) except Exception as exc: # pylint: disable=broad-except # Optionally log an errored action here if the task couldn't be queued - action_instance.status = get_user_message_choice(LearnerCreditRequestActionErrorReasons.EMAIL_ERROR) - action_instance.error_reason = str(exc) + action_instance.status = LearnerCreditRequestUserMessages.APPROVED + action_instance.error_reason = get_error_reason_choice( + LearnerCreditRequestActionErrorReasons.EMAIL_ERROR + ) + action_instance.traceback = format_traceback(exc) action_instance.save() return Response(status=status.HTTP_422_UNPROCESSABLE_ENTITY) @@ -1142,19 +1142,19 @@ def decline(self, *args, **kwargs): # Create the action instance before attempting the decline operation action_instance = LearnerCreditRequestActions.create_action( learner_credit_request=learner_credit_request, - recent_action=get_action_choice(SubsidyRequestStates.DECLINED), - status=get_user_message_choice(SubsidyRequestStates.DECLINED), + recent_action=LearnerCreditRequestActionTypes.DECLINED, + status=LearnerCreditRequestUserMessages.DECLINED, ) try: with transaction.atomic(): learner_credit_request.decline(self.user) except (ValidationError, IntegrityError, DatabaseError) as exc: - action_instance.status = get_user_message_choice(SubsidyRequestStates.REQUESTED) + action_instance.status = LearnerCreditRequestActionTypes.REQUESTED action_instance.error_reason = get_error_reason_choice( LearnerCreditRequestActionErrorReasons.FAILED_DECLINE ) - action_instance.traceback = str(exc) + action_instance.traceback = format_traceback(exc) action_instance.save() logger.exception(f"Error declining learner credit request {learner_credit_request_uuid}: {exc}") @@ -1178,11 +1178,11 @@ def decline(self, *args, **kwargs): try: unlink_users_from_enterprise_task.delay(enterprise_customer_uuid, [lms_user_id]) except (ConnectionError, TimeoutError, OSError) as exc: - action_instance.status = get_user_message_choice(SubsidyRequestStates.REQUESTED) + action_instance.status = LearnerCreditRequestActionTypes.REQUESTED action_instance.error_reason = get_error_reason_choice( LearnerCreditRequestActionErrorReasons.FAILED_DECLINE ) - action_instance.traceback = str(exc) + action_instance.traceback = format_traceback(exc) action_instance.save() logger.exception( diff --git a/enterprise_access/apps/subsidy_request/constants.py b/enterprise_access/apps/subsidy_request/constants.py index abb0d1425..4f91a9c11 100644 --- a/enterprise_access/apps/subsidy_request/constants.py +++ b/enterprise_access/apps/subsidy_request/constants.py @@ -35,6 +35,9 @@ class SubsidyRequestStates: CHOICES = COMMON_STATES + LC_REQUEST_STATES +# DEPRECATED: This class was used to add 'reminded' to the list of possible +# actions. Its functionality has been consolidated into the self-contained +# `LearnerCreditRequestActionTypes` class. class LearnerCreditAdditionalActionStates: """ Additional states specifically for LearnerCreditRequestActions. """ @@ -44,7 +47,8 @@ class LearnerCreditAdditionalActionStates: ) -# Combined choices for LearnerCreditRequestAction model +# DEPRECATED: This variable combined states from multiple classes in a way that +# was confusing and brittle. Use `LearnerCreditRequestActionTypes.CHOICES` instead. LearnerCreditRequestActionChoices = SubsidyRequestStates.CHOICES + LearnerCreditAdditionalActionStates.CHOICES @@ -85,20 +89,61 @@ class SubsidyTypeChoices: SUBSIDY_REQUEST_BULK_OPERATION_BATCH_SIZE = 100 +class LearnerCreditRequestActionTypes: + """ + Defines the set of possible values for the `recent_action` field on the + `LearnerCreditRequestActions` model. This represents the specific event + or operation that occurred (e.g., an approval, a reminder). + """ + REQUESTED = 'requested' + APPROVED = 'approved' + DECLINED = 'declined' + ERROR = 'error' + ACCEPTED = 'accepted' + CANCELLED = 'cancelled' + EXPIRED = 'expired' + REVERSED = 'reversed' + REMINDED = 'reminded' + + CHOICES = ( + (REQUESTED, "Requested"), + (APPROVED, "Approved"), + (DECLINED, "Declined"), + (ERROR, "Error"), + (ACCEPTED, "Accepted"), + (CANCELLED, "Cancelled"), + (EXPIRED, "Expired"), + (REVERSED, "Reversed"), + (REMINDED, "Reminded"), + ) + + class LearnerCreditRequestUserMessages: """ - User-facing messages for LearnerCreditRequestActions status field. - Reusing the state keys from SubsidyRequestStates but with different display messages. + Defines the set of possible values for the `status` field on the + `LearnerCreditRequestActions` model. This represents the user-facing + status label that is displayed in the UI as a result of an action. """ + REQUESTED = 'requested' + REMINDED = 'reminded' + APPROVED = 'approved' + ACCEPTED = 'accepted' + DECLINED = 'declined' + REVERSED = 'reversed' + CANCELLED = 'cancelled' + EXPIRED = 'expired' + ERROR = 'error' + CHOICES = ( - (SubsidyRequestStates.REQUESTED, "Requested"), - (LearnerCreditAdditionalActionStates.REMINDED, "Waiting For Learner"), - (SubsidyRequestStates.APPROVED, "Waiting For Learner"), - (SubsidyRequestStates.ACCEPTED, "Redeemed By Learner"), - (SubsidyRequestStates.DECLINED, "Declined"), - (SubsidyRequestStates.REVERSED, "Refunded"), - (SubsidyRequestStates.CANCELLED, "Cancelled"), - (SubsidyRequestStates.EXPIRED, "Expired"), + (REQUESTED, "Requested"), + (REMINDED, "Waiting For Learner"), + (APPROVED, "Waiting For Learner"), + (ACCEPTED, "Redeemed By Learner"), + (DECLINED, "Declined"), + (REVERSED, "Refunded"), + (CANCELLED, "Cancelled"), + (EXPIRED, "Expired"), + (ERROR, "Error"), ) diff --git a/enterprise_access/apps/subsidy_request/models.py b/enterprise_access/apps/subsidy_request/models.py index 0a3b5fde2..505475811 100644 --- a/enterprise_access/apps/subsidy_request/models.py +++ b/enterprise_access/apps/subsidy_request/models.py @@ -21,9 +21,8 @@ from enterprise_access.apps.subsidy_request.constants import ( SUBSIDY_REQUEST_BULK_OPERATION_BATCH_SIZE, - LearnerCreditAdditionalActionStates, - LearnerCreditRequestActionChoices, LearnerCreditRequestActionErrorReasons, + LearnerCreditRequestActionTypes, LearnerCreditRequestUserMessages, SubsidyRequestStates, SubsidyTypeChoices @@ -487,19 +486,19 @@ def annotate_dynamic_fields_onto_queryset(cls, queryset): ), When( Q(state=SubsidyRequestStates.REQUESTED), - then=Value(SubsidyRequestStates.REQUESTED) + then=Value(LearnerCreditRequestUserMessages.REQUESTED) ), When( Q(state=SubsidyRequestStates.DECLINED), - then=Value(SubsidyRequestStates.DECLINED) + then=Value(LearnerCreditRequestUserMessages.DECLINED) ), When( Q(state=SubsidyRequestStates.CANCELLED), - then=Value(SubsidyRequestStates.CANCELLED) + then=Value(LearnerCreditRequestUserMessages.CANCELLED) ), When( Q(state=SubsidyRequestStates.ERROR), - then=Value(SubsidyRequestStates.ERROR) + then=Value(LearnerCreditRequestUserMessages.ERROR) ), When( Q(state=SubsidyRequestStates.APPROVED), @@ -560,7 +559,7 @@ class LearnerCreditRequestActions(TimeStampedModel): blank=False, null=False, db_index=True, - choices=LearnerCreditRequestActionChoices, + choices=LearnerCreditRequestActionTypes.CHOICES, help_text="The type of action taken on the learner credit request.", ) @@ -615,7 +614,7 @@ def create_action( Args: learner_credit_request (LearnerCreditRequest): The associated learner credit request. recent_action (str): The type of action taken (must be a valid choice from - LearnerCreditRequestActionChoices). + LearnerCreditRequestActionTypes). status (str): The status message (must be a valid choice from LearnerCreditRequestUserMessages.CHOICES). error_reason (str, optional): The error reason if applicable (must be a valid choice from LearnerCreditRequestActionErrorReasons.CHOICES). diff --git a/enterprise_access/apps/subsidy_request/tests/factories.py b/enterprise_access/apps/subsidy_request/tests/factories.py index 6d03372fb..ab1e0dc04 100644 --- a/enterprise_access/apps/subsidy_request/tests/factories.py +++ b/enterprise_access/apps/subsidy_request/tests/factories.py @@ -10,7 +10,12 @@ from enterprise_access.apps.content_assignments.tests.factories import LearnerContentAssignmentFactory from enterprise_access.apps.core.tests.factories import UserFactory -from enterprise_access.apps.subsidy_request.constants import SubsidyRequestStates, SubsidyTypeChoices +from enterprise_access.apps.subsidy_request.constants import ( + LearnerCreditRequestActionTypes, + LearnerCreditRequestUserMessages, + SubsidyRequestStates, + SubsidyTypeChoices +) from enterprise_access.apps.subsidy_request.models import ( CouponCodeRequest, LearnerCreditRequest, @@ -105,8 +110,8 @@ class LearnerCreditRequestActionsFactory(factory.django.DjangoModelFactory): Test factory for the `LearnerCreditRequestActions` model. """ uuid = factory.LazyFunction(uuid4) - recent_action = get_action_choice(SubsidyRequestStates.REQUESTED) - status = get_user_message_choice(SubsidyRequestStates.REQUESTED) + recent_action = LearnerCreditRequestActionTypes.REQUESTED + status = LearnerCreditRequestUserMessages.REQUESTED learner_credit_request = factory.SubFactory(LearnerCreditRequestFactory) error_reason = None traceback = None diff --git a/enterprise_access/apps/subsidy_request/tests/test_models.py b/enterprise_access/apps/subsidy_request/tests/test_models.py index 133fb1f95..f357f12ab 100644 --- a/enterprise_access/apps/subsidy_request/tests/test_models.py +++ b/enterprise_access/apps/subsidy_request/tests/test_models.py @@ -13,8 +13,8 @@ PerLearnerEnrollmentCapLearnerCreditAccessPolicyFactory ) from enterprise_access.apps.subsidy_request.constants import ( - LearnerCreditAdditionalActionStates, LearnerCreditRequestActionErrorReasons, + LearnerCreditRequestActionTypes, LearnerCreditRequestUserMessages, SubsidyRequestStates, SubsidyTypeChoices @@ -307,8 +307,8 @@ def setUp(self): self.learner_credit_request = LearnerCreditRequestFactory(user=self.user) self.action = LearnerCreditRequestActionsFactory( learner_credit_request=self.learner_credit_request, - recent_action=SubsidyRequestStates.REQUESTED, - status=SubsidyRequestStates.REQUESTED, + recent_action=LearnerCreditRequestActionTypes.REQUESTED, + status=LearnerCreditRequestUserMessages.REQUESTED, ) self.enterprise_customer_uuid = uuid4() @@ -327,8 +327,8 @@ def test_model_creation(self): Test that a LearnerCreditRequestActions instance is created successfully. """ self.assertIsNotNone(self.action.uuid) - self.assertEqual(self.action.recent_action, SubsidyRequestStates.REQUESTED) - self.assertEqual(self.action.status, SubsidyRequestStates.REQUESTED) + self.assertEqual(self.action.recent_action, LearnerCreditRequestActionTypes.REQUESTED) + self.assertEqual(self.action.status, LearnerCreditRequestUserMessages.REQUESTED) self.assertIsNone(self.action.error_reason) self.assertIsNone(self.action.traceback) @@ -338,11 +338,11 @@ def test_reminded_action(self): """ reminded_action = LearnerCreditRequestActionsFactory( learner_credit_request=self.learner_credit_request, - recent_action=LearnerCreditAdditionalActionStates.REMINDED, - status=LearnerCreditAdditionalActionStates.REMINDED, + recent_action=LearnerCreditRequestActionTypes.REMINDED, + status=LearnerCreditRequestUserMessages.REMINDED, ) - self.assertEqual(reminded_action.recent_action, LearnerCreditAdditionalActionStates.REMINDED) - self.assertEqual(reminded_action.status, LearnerCreditAdditionalActionStates.REMINDED) + self.assertEqual(reminded_action.recent_action, LearnerCreditRequestActionTypes.REMINDED) + self.assertEqual(reminded_action.status, LearnerCreditRequestUserMessages.REMINDED) def test_error_action(self): """ @@ -350,13 +350,13 @@ def test_error_action(self): """ error_action = LearnerCreditRequestActionsFactory( learner_credit_request=self.learner_credit_request, - recent_action=SubsidyRequestStates.ERROR, - status=SubsidyRequestStates.ERROR, + recent_action=LearnerCreditRequestActionTypes.ERROR, + status=LearnerCreditRequestUserMessages.REQUESTED, error_reason=LearnerCreditRequestActionErrorReasons.FAILED_APPROVAL, traceback="An error occurred", ) - self.assertEqual(error_action.recent_action, SubsidyRequestStates.ERROR) - self.assertEqual(error_action.status, SubsidyRequestStates.ERROR) + self.assertEqual(error_action.recent_action, LearnerCreditRequestActionTypes.ERROR) + self.assertEqual(error_action.status, LearnerCreditRequestUserMessages.REQUESTED) self.assertEqual(error_action.error_reason, LearnerCreditRequestActionErrorReasons.FAILED_APPROVAL) self.assertEqual(error_action.traceback, "An error occurred") @@ -364,20 +364,18 @@ def test_model_update(self): """ Test updating a LearnerCreditRequestActions instance. """ - self.action.recent_action = SubsidyRequestStates.APPROVED - self.action.status = LearnerCreditRequestUserMessages.CHOICES[3][ - 0 - ] # APPROVED choice + self.action.recent_action = LearnerCreditRequestActionTypes.APPROVED + self.action.status = LearnerCreditRequestUserMessages.APPROVED self.action.save() updated_action = LearnerCreditRequestActions.objects.get( uuid=self.action.uuid ) self.assertEqual( - updated_action.recent_action, SubsidyRequestStates.APPROVED + updated_action.recent_action, LearnerCreditRequestActionTypes.APPROVED ) self.assertEqual( updated_action.status, - LearnerCreditRequestUserMessages.CHOICES[3][0], + LearnerCreditRequestUserMessages.APPROVED, ) def test_clean_success_when_learner_credit_config_inactive(self):