From dea2c43af21e2feae7c0165d958a24ae8e8be09f Mon Sep 17 00:00:00 2001 From: ahmad-hassan-dev Date: Thu, 25 Sep 2025 14:54:01 +0500 Subject: [PATCH] feat: bulk remind for lcr --- .../apps/api/serializers/__init__.py | 1 + .../apps/api/serializers/subsidy_requests.py | 48 ++----- .../v1/tests/test_browse_and_request_views.py | 133 ++++++++++++++++++ .../apps/api/v1/views/browse_and_request.py | 81 +++++++++-- enterprise_access/apps/subsidy_request/api.py | 45 ++++++ .../apps/subsidy_request/models.py | 23 ++- .../apps/subsidy_request/tasks.py | 4 + 7 files changed, 286 insertions(+), 49 deletions(-) create mode 100644 enterprise_access/apps/subsidy_request/api.py diff --git a/enterprise_access/apps/api/serializers/__init__.py b/enterprise_access/apps/api/serializers/__init__.py index b59777f2d..8df96a768 100644 --- a/enterprise_access/apps/api/serializers/__init__.py +++ b/enterprise_access/apps/api/serializers/__init__.py @@ -51,6 +51,7 @@ LearnerCreditRequestApproveRequestSerializer, LearnerCreditRequestCancelSerializer, LearnerCreditRequestDeclineSerializer, + LearnerCreditRequestRemindAllSerializer, LearnerCreditRequestRemindSerializer, LearnerCreditRequestSerializer, LicenseRequestSerializer, diff --git a/enterprise_access/apps/api/serializers/subsidy_requests.py b/enterprise_access/apps/api/serializers/subsidy_requests.py index 0483ecb30..35ca1b84e 100644 --- a/enterprise_access/apps/api/serializers/subsidy_requests.py +++ b/enterprise_access/apps/api/serializers/subsidy_requests.py @@ -342,45 +342,25 @@ def get_learner_credit_request(self): class LearnerCreditRequestRemindSerializer(serializers.Serializer): """ - Request serializer to validate remind endpoint for a LearnerCreditRequest. + Request serializer to validate remind endpoint for a list of LearnerCreditRequests. For view: LearnerCreditRequestViewSet.remind """ - learner_credit_request_uuid = serializers.UUIDField( + learner_credit_request_uuids = serializers.ListField( + child=serializers.UUIDField(), required=True, - help_text="The UUID of the LearnerCreditRequest to be reminded." + allow_empty=False, + help_text="A list of LearnerCreditRequest UUIDs to be reminded." ) - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - self._learner_credit_request = None - - def validate_learner_credit_request_uuid(self, value): - """ - Validate that the learner credit request exists, has an associated assignment, - and is in a state where a reminder is appropriate. - """ - try: - learner_credit_request = LearnerCreditRequest.objects.select_related('assignment').get(uuid=value) - except LearnerCreditRequest.DoesNotExist as exc: - raise serializers.ValidationError(f"Learner credit request with uuid {value} not found.") from exc - - if learner_credit_request.state != SubsidyRequestStates.APPROVED: - raise serializers.ValidationError( - f"Cannot send a reminder for a request in the '{learner_credit_request.state}' state. " - "Reminders can only be sent for 'APPROVED' requests." - ) - - if not learner_credit_request.assignment: - raise serializers.ValidationError( - f"The learner credit request with uuid {value} does not have an associated assignment." - ) - self._learner_credit_request = learner_credit_request - return value +class LearnerCreditRequestRemindAllSerializer(serializers.Serializer): + """ + Request serializer to validate remind-all endpoint for LearnerCreditRequests. - def get_learner_credit_request(self): - """ - Return the already-fetched learner credit request object. - """ - return getattr(self, '_learner_credit_request', None) + For view: LearnerCreditRequestViewSet.remind_all + """ + policy_uuid = serializers.UUIDField( + required=True, + help_text='The UUID of the policy to which the requests belong.', + ) diff --git a/enterprise_access/apps/api/v1/tests/test_browse_and_request_views.py b/enterprise_access/apps/api/v1/tests/test_browse_and_request_views.py index 95d868512..1c5d07d68 100644 --- a/enterprise_access/apps/api/v1/tests/test_browse_and_request_views.py +++ b/enterprise_access/apps/api/v1/tests/test_browse_and_request_views.py @@ -2788,6 +2788,139 @@ def test_approve_policy_lock_failure( assert self.user_request_1.state == SubsidyRequestStates.REQUESTED assert self.user_request_1.assignment is None + @mock.patch( + 'enterprise_access.apps.api.v1.views.browse_and_request.subsidy_request_api.remind_learner_credit_requests' + ) + def test_remind_success(self, mock_remind_api): + """ + Test that the remind endpoint returns 200 OK when all requests are remindable. + """ + self.set_jwt_cookie([{ + 'system_wide_role': SYSTEM_ENTERPRISE_ADMIN_ROLE, + 'context': str(self.enterprise_customer_uuid_1) + }]) + remindable_request = LearnerCreditRequestFactory( + enterprise_customer_uuid=self.enterprise_customer_uuid_1, + state=SubsidyRequestStates.APPROVED, + assignment=LearnerContentAssignmentFactory( + assignment_configuration=self.assignment_config + ), + ) + mock_remind_api.return_value = { + 'remindable_requests': [remindable_request], + 'non_remindable_requests': [] + } + url = reverse('api:v1:learner-credit-requests-remind') + data = { + 'enterprise_customer_uuid': str(self.enterprise_customer_uuid_1), + 'learner_credit_request_uuids': [str(remindable_request.uuid)] + } + response = self.client.post(url, data) + assert response.status_code == status.HTTP_200_OK + mock_remind_api.assert_called_once() + + @mock.patch( + 'enterprise_access.apps.api.v1.views.browse_and_request.subsidy_request_api.remind_learner_credit_requests' + ) + def test_remind_no_remindable_requests(self, mock_remind_api): + """ + Test that the remind endpoint returns 200 OK when all requests are remindable. + """ + self.set_jwt_cookie([{ + 'system_wide_role': SYSTEM_ENTERPRISE_ADMIN_ROLE, + 'context': str(self.enterprise_customer_uuid_1) + }]) + remindable_request = LearnerCreditRequestFactory( + enterprise_customer_uuid=self.enterprise_customer_uuid_1, + state=SubsidyRequestStates.REQUESTED, + assignment=LearnerContentAssignmentFactory( + assignment_configuration=self.assignment_config + ), + ) + mock_remind_api.return_value = { + 'remindable_requests': [], + 'non_remindable_requests': [remindable_request] + } + url = reverse('api:v1:learner-credit-requests-remind') + data = { + 'enterprise_customer_uuid': str(self.enterprise_customer_uuid_1), + 'learner_credit_request_uuids': [str(remindable_request.uuid)] + } + response = self.client.post(url, data) + assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY + mock_remind_api.assert_called_once() + + @mock.patch( + 'enterprise_access.apps.api.v1.views.browse_and_request.subsidy_request_api.remind_learner_credit_requests' + ) + def test_remind_all_success(self, mock_remind_api): + """ + Test that the remind-all endpoint returns 202 ACCEPTED when all requests are remindable. + """ + self.set_jwt_cookie([{ + 'system_wide_role': SYSTEM_ENTERPRISE_ADMIN_ROLE, + 'context': str(self.enterprise_customer_uuid_1) + }]) + remindable_request = LearnerCreditRequestFactory( + enterprise_customer_uuid=self.enterprise_customer_uuid_1, + state=SubsidyRequestStates.APPROVED, + assignment=LearnerContentAssignmentFactory( + assignment_configuration=self.assignment_config + ), + learner_credit_request_config=self.learner_credit_config, + ) + mock_remind_api.return_value = { + 'remindable_requests': [remindable_request], + 'non_remindable_requests': [] + } + url = reverse('api:v1:learner-credit-requests-remind-all') + data = { + 'enterprise_customer_uuid': str(self.enterprise_customer_uuid_1), + 'policy_uuid': str(self.policy.uuid) + } + response = self.client.post(url, data) + assert response.status_code == status.HTTP_202_ACCEPTED + mock_remind_api.assert_called_once() + + @mock.patch( + 'enterprise_access.apps.api.v1.views.browse_and_request.subsidy_request_api.remind_learner_credit_requests' + ) + def test_remind_all_non_remindable_requests(self, mock_remind_api): + """ + Test that remind-all returns 422 if there are non-remindable requests. + """ + self.set_jwt_cookie([{ + 'system_wide_role': SYSTEM_ENTERPRISE_ADMIN_ROLE, + 'context': str(self.enterprise_customer_uuid_1) + }]) + remindable_request = LearnerCreditRequestFactory( + enterprise_customer_uuid=self.enterprise_customer_uuid_1, + state=SubsidyRequestStates.APPROVED, + assignment=LearnerContentAssignmentFactory( + assignment_configuration=self.assignment_config + ), + learner_credit_request_config=self.learner_credit_config, + ) + non_remindable_request = LearnerCreditRequestFactory( + enterprise_customer_uuid=self.enterprise_customer_uuid_1, + state=SubsidyRequestStates.REQUESTED, + assignment=None, + learner_credit_request_config=self.learner_credit_config, + ) + mock_remind_api.return_value = { + 'remindable_requests': [remindable_request], + 'non_remindable_requests': [non_remindable_request] + } + url = reverse('api:v1:learner-credit-requests-remind-all') + data = { + 'enterprise_customer_uuid': str(self.enterprise_customer_uuid_1), + 'policy_uuid': str(self.policy.uuid) + } + response = self.client.post(url, data) + + assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY + mock_remind_api.assert_called_once() + def test_cancel_invalid_request_uuid(self): """ Test cancel with invalid UUID format returns 400. 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..3f2c8fd27 100644 --- a/enterprise_access/apps/api/v1/views/browse_and_request.py +++ b/enterprise_access/apps/api/v1/views/browse_and_request.py @@ -49,6 +49,7 @@ from enterprise_access.apps.subsidy_access_policy.api import approve_learner_credit_request_via_policy from enterprise_access.apps.subsidy_access_policy.exceptions import SubisidyAccessPolicyRequestApprovalError from enterprise_access.apps.subsidy_access_policy.models import SubsidyAccessPolicy +from enterprise_access.apps.subsidy_request import api as subsidy_request_api from enterprise_access.apps.subsidy_request.constants import ( REUSABLE_REQUEST_STATES, LearnerCreditAdditionalActionStates, @@ -1093,27 +1094,79 @@ def cancel(self, request, *args, **kwargs): @action(detail=False, url_path="remind", methods=["post"]) def remind(self, request, *args, **kwargs): """ - Remind a Learner that their LearnerCreditRequest is Approved and waiting for their action. + Send reminders to a list of learners with associated ``LearnerCreditRequests`` + record by list of uuids. + + This action is idempotent and will only send reminders for requests + that are in a valid, remindable state (e.g., 'APPROVED'). """ serializer = serializers.LearnerCreditRequestRemindSerializer(data=request.data) serializer.is_valid(raise_exception=True) - learner_credit_request = serializer.get_learner_credit_request() - assignment = learner_credit_request.assignment - - 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), + request_uuids = serializer.validated_data['learner_credit_request_uuids'] + learner_credit_requests = self.get_queryset().select_related('assignment').filter( + uuid__in=request_uuids ) + if len(learner_credit_requests) != len(set(request_uuids)): + return Response( + status=status.HTTP_404_NOT_FOUND + ) + try: - send_reminder_email_for_pending_learner_credit_request.delay(assignment.uuid) + response = subsidy_request_api.remind_learner_credit_requests(learner_credit_requests) + if response.get('non_remindable_requests'): + return Response( + status=status.HTTP_422_UNPROCESSABLE_ENTITY + ) 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.save() + except Exception: # pylint: disable=broad-except + return Response(status=status.HTTP_422_UNPROCESSABLE_ENTITY) + + @permission_required( + constants.REQUESTS_ADMIN_ACCESS_PERMISSION, + fn=get_enterprise_uuid_from_request_data, + ) + @action(detail=False, url_path="remind-all", methods=["post"], pagination_class=None) + def remind_all(self, request, *args, **kwargs): + """ + Send reminders for all selected learner credit requests that are in a remindable state. + + This endpoint respects the filters applied in the request (e.g., by policy_uuid), + allowing admins to send bulk reminders to a specific subset of requests. + + ``` + Raises: + 404 if no remindable learner credit requests were found + 422 if any of the learner credit requests threw an error (not found or not remindable) + ``` + """ + serializer = serializers.LearnerCreditRequestRemindAllSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + policy_uuid = serializer.validated_data['policy_uuid'] + + # A request is only remindable if it is in the 'APPROVED' state. + learner_credit_requests = self.get_queryset().filter( + state=SubsidyRequestStates.APPROVED, + learner_credit_request_config__learner_credit_config__uuid=policy_uuid + ) + + if not learner_credit_requests.exists(): + return Response(status=status.HTTP_404_NOT_FOUND) + + try: + response = subsidy_request_api.remind_learner_credit_requests(learner_credit_requests) + if non_remindable_requests := response.get('non_remindable_requests'): + # This is very unlikely to occur, because we filter down to only the remindable + # requests before calling `remind_learner_credit_requests()`, and that function + # only declares requests to be non-remindable if they are not + # in the set of remindable states. + logger.error( + 'There were non-remindable requests in remind-all: %s', + non_remindable_requests, + ) + return Response(status=status.HTTP_422_UNPROCESSABLE_ENTITY) + return Response(status=status.HTTP_202_ACCEPTED) + except Exception: # pylint: disable=broad-except return Response(status=status.HTTP_422_UNPROCESSABLE_ENTITY) @permission_required( diff --git a/enterprise_access/apps/subsidy_request/api.py b/enterprise_access/apps/subsidy_request/api.py new file mode 100644 index 000000000..f0f4129d0 --- /dev/null +++ b/enterprise_access/apps/subsidy_request/api.py @@ -0,0 +1,45 @@ +""" +Primary Python API for interacting with Subsidy Request +records and business logic. +""" +import logging +from typing import Iterable + +from enterprise_access.apps.subsidy_request.constants import SubsidyRequestStates +from enterprise_access.apps.subsidy_request.models import LearnerCreditRequest +from enterprise_access.apps.subsidy_request.tasks import send_reminder_email_for_pending_learner_credit_request + +logger = logging.getLogger(__name__) + + +def remind_learner_credit_requests(requests: Iterable[LearnerCreditRequest]) -> dict: + """ + Bulk remind for Learner Credit Requests. + + This filters for requests that are in a remindable state and triggers a Celery + task to send a reminder email for each one. + + Args: + requests: An iterable of LearnerCreditRequest objects. + + Returns: + A dict containing lists of 'remindable_requests' and 'non_remindable_requests' requests. + """ + # A request is only remindable if it is APPROVED and has an associated assignment. + remindable_requests = { + req for req in requests + if req.state == SubsidyRequestStates.APPROVED and req.assignment_id is not None + } + + non_remindable_requests = set(requests) - remindable_requests + + logger.info(f'Skipping {len(non_remindable_requests)} non-remindable learner credit requests.') + logger.info(f'Queueing reminders for {len(remindable_requests)} learner credit requests.') + + for req in remindable_requests: + send_reminder_email_for_pending_learner_credit_request.delay(req.assignment.uuid) + + return { + 'remindable_requests': list(remindable_requests), + 'non_remindable_requests': list(non_remindable_requests), + } diff --git a/enterprise_access/apps/subsidy_request/models.py b/enterprise_access/apps/subsidy_request/models.py index 0a3b5fde2..0aa97a18a 100644 --- a/enterprise_access/apps/subsidy_request/models.py +++ b/enterprise_access/apps/subsidy_request/models.py @@ -29,7 +29,8 @@ SubsidyTypeChoices ) from enterprise_access.apps.subsidy_request.tasks import update_course_info_for_subsidy_request_task -from enterprise_access.utils import localized_utcnow +from enterprise_access.apps.subsidy_request.utils import get_action_choice, get_user_message_choice +from enterprise_access.utils import format_traceback, localized_utcnow class SubsidyRequest(TimeStampedModel, SoftDeletableModel): @@ -435,6 +436,26 @@ def approve(self, reviewer): self.reviewed_at = localized_utcnow() self.save() + def add_successful_reminded_action(self): + """ + Adds a successful "reminded" LearnerCreditRequestActions for this request. + """ + return self.actions.create( + recent_action=get_action_choice(LearnerCreditAdditionalActionStates.REMINDED), + status=get_user_message_choice(LearnerCreditAdditionalActionStates.REMINDED), + ) + + def add_errored_reminded_action(self, exc): + """ + Adds an errored "reminded" LearnerCreditRequestActions for this request. + """ + return self.actions.create( + recent_action=get_action_choice(LearnerCreditAdditionalActionStates.REMINDED), + status=get_user_message_choice(LearnerCreditAdditionalActionStates.REMINDED), + error_reason=LearnerCreditRequestActionErrorReasons.EMAIL_ERROR, + traceback=format_traceback(exc), + ) + @classmethod def annotate_dynamic_fields_onto_queryset(cls, queryset): """ diff --git a/enterprise_access/apps/subsidy_request/tasks.py b/enterprise_access/apps/subsidy_request/tasks.py index 6b7cad39d..50f9f9999 100644 --- a/enterprise_access/apps/subsidy_request/tasks.py +++ b/enterprise_access/apps/subsidy_request/tasks.py @@ -84,6 +84,7 @@ def log_errored_action(self, learner_credit_request, exc): f'Enterprise ID: {learner_credit_request.enterprise_customer_uuid}, ' f'Exception: {exc}' ) + learner_credit_request.add_errored_reminded_action(exc) # pylint: disable=abstract-method @@ -396,6 +397,9 @@ def send_reminder_email_for_pending_learner_credit_request(assignment_uuid): braze_trigger_properties, campaign_uuid, ) + + if hasattr(assignment, 'credit_request') and assignment.credit_request: + assignment.credit_request.add_successful_reminded_action() logger.info(f'Sent braze campaign reminder uuid={campaign_uuid} message for assignment {assignment}')