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 504ff72e1..49ff5f58f 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 @@ -1801,8 +1801,7 @@ def test_create_success(self): SubsidyRequestStates.EXPIRED, SubsidyRequestStates.REVERSED, ) - @mock.patch(BNR_VIEW_PATH + '.send_learner_credit_bnr_admins_email_with_new_requests_task.delay') - def test_create_reuse_existing_request_success(self, reusable_state, mock_email_task): + def test_create_reuse_existing_request_success(self, reusable_state): """ Test that an existing request in reusable states (CANCELLED, EXPIRED, REVERSED) gets reused instead of creating a new one. @@ -1882,13 +1881,6 @@ def test_create_reuse_existing_request_success(self, reusable_state, mock_email_ assert action is not None assert action.status == get_user_message_choice(SubsidyRequestStates.REQUESTED) - # Verify email notification task was called - mock_email_task.assert_called_once_with( - str(self.policy.uuid), - str(self.policy.learner_credit_request_config.uuid), - str(existing_request.enterprise_customer_uuid) - ) - def test_overview_happy_path(self): """ Test the overview endpoint returns correct state counts. 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 93736ecd0..02202f350 100644 --- a/enterprise_access/apps/api/v1/views/browse_and_request.py +++ b/enterprise_access/apps/api/v1/views/browse_and_request.py @@ -65,7 +65,6 @@ SubsidyRequestCustomerConfiguration ) from enterprise_access.apps.subsidy_request.tasks import ( - send_learner_credit_bnr_admins_email_with_new_requests_task, send_learner_credit_bnr_request_approve_task, send_reminder_email_for_pending_learner_credit_request ) @@ -895,12 +894,6 @@ def create(self, request, *args, **kwargs): recent_action=get_action_choice(SubsidyRequestStates.REQUESTED), status=get_user_message_choice(SubsidyRequestStates.REQUESTED), ) - # Trigger admin email notification with the latest request - send_learner_credit_bnr_admins_email_with_new_requests_task.delay( - str(policy.uuid), - str(policy.learner_credit_request_config.uuid), - str(existing_request.enterprise_customer_uuid) - ) response_data = serializers.LearnerCreditRequestSerializer(existing_request).data return Response(response_data, status=status.HTTP_200_OK) except Exception as exc: # pylint: disable=broad-except @@ -938,13 +931,6 @@ def create(self, request, *args, **kwargs): recent_action=get_action_choice(SubsidyRequestStates.REQUESTED), status=get_user_message_choice(SubsidyRequestStates.REQUESTED), ) - - # Trigger admin email notification with the latest request - send_learner_credit_bnr_admins_email_with_new_requests_task.delay( - str(policy.uuid), - str(policy.learner_credit_request_config.uuid), - str(lcr.enterprise_customer_uuid) - ) except LearnerCreditRequest.DoesNotExist: logger.warning(f"LearnerCreditRequest {lcr_uuid} not found for action creation.") diff --git a/enterprise_access/apps/subsidy_request/management/commands/send_learner_credit_bnr_daily_digest.py b/enterprise_access/apps/subsidy_request/management/commands/send_learner_credit_bnr_daily_digest.py new file mode 100644 index 000000000..1b447b768 --- /dev/null +++ b/enterprise_access/apps/subsidy_request/management/commands/send_learner_credit_bnr_daily_digest.py @@ -0,0 +1,93 @@ +""" +Management command to send daily Browse & Request learner credit digest emails to enterprise admins. + +Simplified version: run once per day (e.g., via cron). It scans all BNR-enabled policies and +queues a digest task for each policy that has one or more REQUESTED learner credit requests +(open requests, regardless of creation date). Supports a --dry-run mode. +""" +import logging + +from django.core.management.base import BaseCommand + +from enterprise_access.apps.subsidy_access_policy.models import SubsidyAccessPolicy +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_learner_credit_bnr_admins_email_with_new_requests_task + +logger = logging.getLogger(__name__) + + +class Command(BaseCommand): + """Django management command to enqueue daily Browse & Request learner credit digest tasks. + + Scans active, non-retired policies with an active learner credit request config and enqueues + one Celery task per policy that has at least one open (REQUESTED) learner credit request. + Supports an optional dry-run mode for visibility without enqueuing tasks. + """ + + help = ('Queue celery tasks that send daily digest emails for Browse & Request learner credit ' + 'requests per BNR-enabled policy (simple mode).') + + LOCK_KEY_TEMPLATE = 'bnr-lc-digest-{date}' + LOCK_TIMEOUT_SECONDS = 2 * 60 * 60 # 2 hours + + def add_arguments(self, parser): # noqa: D401 - intentionally left minimal + parser.add_argument( + '--dry-run', + action='store_true', + dest='dry_run', + help='Show which tasks would be enqueued without sending them.' + ) + + def handle(self, *args, **options): + dry_run = options.get('dry_run') + + policies_qs = SubsidyAccessPolicy.objects.filter( + active=True, + retired=False, + learner_credit_request_config__isnull=False, + learner_credit_request_config__active=True, + ).select_related('learner_credit_request_config') + + total_policies = 0 + policies_with_requests = 0 + tasks_enqueued = 0 + + for policy in policies_qs.iterator(): + total_policies += 1 + config = policy.learner_credit_request_config + if not config: + continue + + num_open_requests = LearnerCreditRequest.objects.filter( + learner_credit_request_config=config, + enterprise_customer_uuid=policy.enterprise_customer_uuid, + state=SubsidyRequestStates.REQUESTED, + ).count() + if num_open_requests == 0: + continue + + policies_with_requests += 1 + if dry_run: + logger.info('[DRY RUN] Policy %s enterprise %s would enqueue digest task (%s open requests).', + policy.uuid, policy.enterprise_customer_uuid, num_open_requests) + continue + + logger.info( + 'Policy %s enterprise %s has %s open learner credit requests. Enqueuing digest task.', + policy.uuid, policy.enterprise_customer_uuid, num_open_requests + ) + send_learner_credit_bnr_admins_email_with_new_requests_task.delay( + str(policy.uuid), + str(config.uuid), + str(policy.enterprise_customer_uuid), + ) + tasks_enqueued += 1 + + summary = ( + f"BNR daily digest summary: scanned={total_policies} policies, " + f"with_requests={policies_with_requests}, tasks_enqueued={tasks_enqueued}, dry_run={dry_run}" + ) + logger.info(summary) + self.stdout.write(self.style.SUCCESS(summary)) + return 0 diff --git a/enterprise_access/apps/subsidy_request/management/commands/tests/test_send_learner_credit_bnr_daily_digest.py b/enterprise_access/apps/subsidy_request/management/commands/tests/test_send_learner_credit_bnr_daily_digest.py new file mode 100644 index 000000000..d277a963f --- /dev/null +++ b/enterprise_access/apps/subsidy_request/management/commands/tests/test_send_learner_credit_bnr_daily_digest.py @@ -0,0 +1,159 @@ +"""Tests for send_learner_credit_bnr_daily_digest management command. + +Covers command behavior for enqueuing tasks when there are open (REQUESTED) requests. +""" + +from datetime import timedelta +from unittest import mock +from uuid import uuid4 + +import pytest +from django.core.management import call_command +from django.utils import timezone + +from enterprise_access.apps.subsidy_access_policy.tests.factories import ( + PerLearnerSpendCapLearnerCreditAccessPolicyFactory +) +from enterprise_access.apps.subsidy_request.constants import SubsidyRequestStates +from enterprise_access.apps.subsidy_request.tests.factories import ( + LearnerCreditRequestConfigurationFactory, + LearnerCreditRequestFactory +) + + +@pytest.mark.django_db +class TestSendLearnerCreditBNRDailyDigestCommand: + """Tests enqueuing behavior for the BNR daily digest management command.""" + command_name = "send_learner_credit_bnr_daily_digest" + + def _make_policy_with_config( + self, + *, + active=True, + retired=False, + config_active=True, + enterprise_uuid=None + ): + """Helper to create a policy and its learner credit request config for test setups.""" + enterprise_uuid = enterprise_uuid or uuid4() + config = LearnerCreditRequestConfigurationFactory( + active=config_active + ) + policy = PerLearnerSpendCapLearnerCreditAccessPolicyFactory( + active=active, + retired=retired, + learner_credit_request_config=config, + enterprise_customer_uuid=enterprise_uuid, + ) + return policy, config + + @mock.patch( + "enterprise_access.apps.subsidy_request.management.commands." + "send_learner_credit_bnr_daily_digest." + "send_learner_credit_bnr_admins_email_with_new_requests_task.delay" + ) + def test_no_eligible_policies(self, mock_delay): + # Inactive policy + self._make_policy_with_config(active=False) + # Retired policy + self._make_policy_with_config(retired=True) + # No config + PerLearnerSpendCapLearnerCreditAccessPolicyFactory() + # Inactive config + self._make_policy_with_config(config_active=False) + + call_command(self.command_name) + mock_delay.assert_not_called() + + @mock.patch( + "enterprise_access.apps.subsidy_request.management.commands." + "send_learner_credit_bnr_daily_digest." + "send_learner_credit_bnr_admins_email_with_new_requests_task.delay" + ) + def test_eligible_policy_no_open_requests(self, mock_delay): + policy, config = self._make_policy_with_config() + LearnerCreditRequestFactory( + enterprise_customer_uuid=policy.enterprise_customer_uuid, + learner_credit_request_config=config, + state=SubsidyRequestStates.APPROVED, + ) + call_command(self.command_name) + mock_delay.assert_not_called() + + @mock.patch( + "enterprise_access.apps.subsidy_request.management.commands." + "send_learner_credit_bnr_daily_digest." + "send_learner_credit_bnr_admins_email_with_new_requests_task.delay" + ) + def test_enqueues_for_open_request(self, mock_delay): + policy, config = self._make_policy_with_config() + LearnerCreditRequestFactory( + enterprise_customer_uuid=policy.enterprise_customer_uuid, + learner_credit_request_config=config, + state=SubsidyRequestStates.REQUESTED, + ) + call_command(self.command_name) + assert mock_delay.call_count == 1 + args = mock_delay.call_args[0] + assert str(policy.uuid) == args[0] + assert str(config.uuid) == args[1] + assert str(policy.enterprise_customer_uuid) == args[2] + + @mock.patch( + "enterprise_access.apps.subsidy_request.management.commands." + "send_learner_credit_bnr_daily_digest." + "send_learner_credit_bnr_admins_email_with_new_requests_task.delay" + ) + def test_old_open_request_still_enqueues(self, mock_delay): + policy, config = self._make_policy_with_config() + req = LearnerCreditRequestFactory( + enterprise_customer_uuid=policy.enterprise_customer_uuid, + learner_credit_request_config=config, + state=SubsidyRequestStates.REQUESTED, + ) + # Make it "old" (yesterday) + req.created = timezone.now() - timedelta(days=7) + req.save(update_fields=["created"]) + + call_command(self.command_name) + assert mock_delay.call_count == 1 + + @mock.patch( + "enterprise_access.apps.subsidy_request.management.commands." + "send_learner_credit_bnr_daily_digest." + "send_learner_credit_bnr_admins_email_with_new_requests_task.delay" + ) + def test_multiple_policies_mixed(self, mock_delay): + policy_a, config_a = self._make_policy_with_config() + LearnerCreditRequestFactory( + enterprise_customer_uuid=policy_a.enterprise_customer_uuid, + learner_credit_request_config=config_a, + state=SubsidyRequestStates.REQUESTED, + ) + + self._make_policy_with_config() + + policy_c, config_c = self._make_policy_with_config() + LearnerCreditRequestFactory( + enterprise_customer_uuid=policy_c.enterprise_customer_uuid, + learner_credit_request_config=config_c, + state=SubsidyRequestStates.REQUESTED, + ) + + call_command(self.command_name) + + assert mock_delay.call_count == 2 + calls = [ + mock.call( + str(policy_a.uuid), + str(config_a.uuid), + str(policy_a.enterprise_customer_uuid), + ), + mock.call( + str(policy_c.uuid), + str(config_c.uuid), + str(policy_c.enterprise_customer_uuid), + ), + ] + actual = [c.args for c in mock_delay.call_args_list] + assert sorted(calls) == sorted([mock.call(*args) for args in actual]) diff --git a/enterprise_access/apps/subsidy_request/tasks.py b/enterprise_access/apps/subsidy_request/tasks.py index cc568d1c0..90ff5eac5 100644 --- a/enterprise_access/apps/subsidy_request/tasks.py +++ b/enterprise_access/apps/subsidy_request/tasks.py @@ -152,32 +152,33 @@ def send_learner_credit_bnr_admins_email_with_new_requests_task( policy_uuid, lc_request_config_uuid, enterprise_customer_uuid ): """ - Task to send new learner credit request emails to admins. + Daily digest email to enterprise admins for Browse & Request learner credit requests. - This task can be manually triggered from browse_and_request when a new - LearnerCreditRequest is created. It will send the latest 10 requests in - REQUESTED state to enterprise admins. + Intended to be called once per day (via a scheduler). It aggregates all open learner credit + requests in REQUESTED state for the given policy and enterprise, and sends a summary email + containing up to the 10 most recent requests plus the total count of open requests. Args: - policy_uuid (str): subsidy access policy uuid identifier - lc_request_config_uuid (str): learner credit request config uuid identifier - enterprise_customer_uuid (str): enterprise customer uuid identifier + policy_uuid (str): Subsidy access policy UUID identifier. + lc_request_config_uuid (str): Learner credit request config UUID identifier. + enterprise_customer_uuid (str): Enterprise customer UUID identifier. Raises: - HTTPError if Braze client call fails with an HTTPError + HTTPError: If Braze client call fails with an HTTPError. """ - subsidy_model = apps.get_model('subsidy_request.LearnerCreditRequest') subsidy_requests = subsidy_model.objects.filter( enterprise_customer_uuid=enterprise_customer_uuid, learner_credit_request_config__uuid=lc_request_config_uuid, state=SubsidyRequestStates.REQUESTED, ) + latest_subsidy_requests = subsidy_requests.order_by('-created')[:10] + total_open = subsidy_requests.count() - if not subsidy_requests: + if total_open == 0: logger.info( - 'No learner credit requests in REQUESTED state. Not sending new requests ' - f'email to admins for enterprise {enterprise_customer_uuid}.' + 'No open learner credit requests in REQUESTED state. Not sending digest email to admins ' + 'for enterprise %s.', enterprise_customer_uuid ) return @@ -192,10 +193,11 @@ def send_learner_credit_bnr_admins_email_with_new_requests_task( if not admin_users: logger.info( - f'No admin users found for enterprise {enterprise_customer_uuid}. ' - 'Not sending new requests email.' + 'No admin users found for enterprise %s. Not sending learner credit digest email.', + enterprise_customer_uuid ) return + braze_client = BrazeApiClient() recipients = [ braze_client.create_recipient( @@ -208,7 +210,7 @@ def send_learner_credit_bnr_admins_email_with_new_requests_task( braze_trigger_properties = { 'manage_requests_url': manage_requests_url, 'requests': [], - 'total_requests': len(subsidy_requests), + 'total_requests': total_open, 'organization': organization, } @@ -219,9 +221,9 @@ def send_learner_credit_bnr_admins_email_with_new_requests_task( }) logger.info( - f'Sending learner credit requests email to admins for enterprise {enterprise_customer_uuid}. ' - f'This includes {len(subsidy_requests)} requests. ' - f'Sending to: {admin_users}' + 'Sending learner credit DAILY DIGEST email to admins for enterprise %s. ' + 'Total open requests=%s. Sending to %s', + enterprise_customer_uuid, total_open, admin_users ) try: @@ -232,7 +234,8 @@ def send_learner_credit_bnr_admins_email_with_new_requests_task( ) except Exception: logger.exception( - f'Exception sending Braze campaign email for enterprise {enterprise_customer_uuid}.' + 'Exception sending Braze learner credit daily digest for enterprise %s.', + enterprise_customer_uuid ) raise diff --git a/enterprise_access/apps/subsidy_request/tests/test_tasks_bnr_daily_digest.py b/enterprise_access/apps/subsidy_request/tests/test_tasks_bnr_daily_digest.py new file mode 100644 index 000000000..c3a98d7a5 --- /dev/null +++ b/enterprise_access/apps/subsidy_request/tests/test_tasks_bnr_daily_digest.py @@ -0,0 +1,168 @@ +"""Tests for send_learner_credit_bnr_admins_email_with_new_requests_task (task). + +Validates payload, early exits, and slicing behavior for open (REQUESTED) requests. +""" +from datetime import timedelta +from unittest import mock +from uuid import uuid4 + +import pytest +from django.conf import settings +from django.utils import timezone + +from enterprise_access.apps.subsidy_request.constants import SubsidyRequestStates +from enterprise_access.apps.subsidy_request.tasks import send_learner_credit_bnr_admins_email_with_new_requests_task +from enterprise_access.apps.subsidy_request.tests.factories import ( + LearnerCreditRequestConfigurationFactory, + LearnerCreditRequestFactory +) + + +@pytest.mark.django_db +class TestLearnerCreditBNRDailyDigestTask: + """Tests for the BNR daily digest task.""" + def _enterprise_data(self, enterprise_uuid, slug='ent-slug', name='Ent Name'): + return { + 'uuid': enterprise_uuid, + 'slug': slug, + 'name': name, + 'admin_users': [ + {'lms_user_id': 1, 'email': 'a@example.com'}, + {'lms_user_id': 2, 'email': 'b@example.com'}, + ], + } + + @mock.patch('enterprise_access.apps.subsidy_request.tasks.BrazeApiClient') + @mock.patch('enterprise_access.apps.subsidy_request.tasks.LmsApiClient.get_enterprise_customer_data') + def test_no_open_requests_early_exit(self, mock_ent_data, mock_braze): + enterprise_uuid = uuid4() + config = LearnerCreditRequestConfigurationFactory(active=True) + mock_ent_data.return_value = self._enterprise_data(enterprise_uuid) + + # No REQUESTED requests + send_learner_credit_bnr_admins_email_with_new_requests_task( + policy_uuid=str(uuid4()), + lc_request_config_uuid=str(config.uuid), + enterprise_customer_uuid=str(enterprise_uuid), + ) + mock_braze.return_value.send_campaign_message.assert_not_called() + + @mock.patch('enterprise_access.apps.subsidy_request.tasks.BrazeApiClient') + @mock.patch('enterprise_access.apps.subsidy_request.tasks.LmsApiClient.get_enterprise_customer_data') + def test_payload_and_recipients_for_open_requests(self, mock_ent_data, mock_braze): + enterprise_uuid = uuid4() + config = LearnerCreditRequestConfigurationFactory(active=True) + mock_ent_data.return_value = self._enterprise_data(enterprise_uuid, slug='test-slug', name='Org Name') + + # Create 3 open requests with deterministic order (newest last created) + reqs = [] + for i in range(3): + r = LearnerCreditRequestFactory( + enterprise_customer_uuid=enterprise_uuid, + learner_credit_request_config=config, + state=SubsidyRequestStates.REQUESTED, + ) + # Stagger created to control ordering + r.created = timezone.now() - timedelta(minutes=10 - i) + r.save(update_fields=['created']) + reqs.append(r) + + policy_uuid = uuid4() + send_learner_credit_bnr_admins_email_with_new_requests_task( + policy_uuid=str(policy_uuid), + lc_request_config_uuid=str(config.uuid), + enterprise_customer_uuid=str(enterprise_uuid), + ) + + mock_braze.return_value.send_campaign_message.assert_called_once() + call_args = mock_braze.return_value.send_campaign_message.call_args + campaign_id = call_args[0][0] + kwargs = call_args[1] + + assert campaign_id == settings.BRAZE_LEARNER_CREDIT_BNR_NEW_REQUESTS_NOTIFICATION_CAMPAIGN + assert kwargs['trigger_properties']['total_requests'] == 3 + # Ordered by -created: reqs with highest created last in loop + emails = [item['user_email'] for item in kwargs['trigger_properties']['requests']] + titles = [item['course_title'] for item in kwargs['trigger_properties']['requests']] + expected = sorted(reqs, key=lambda r: r.created, reverse=True) + assert emails == [r.user.email for r in expected] + assert titles == [r.course_title for r in expected] + assert kwargs['trigger_properties']['manage_requests_url'].endswith( + f'/admin/learner-credit/{policy_uuid}/requests' + ) + # Recipients are created for each admin + assert len(kwargs['recipients']) == 2 + + @mock.patch('enterprise_access.apps.subsidy_request.tasks.BrazeApiClient') + @mock.patch('enterprise_access.apps.subsidy_request.tasks.LmsApiClient.get_enterprise_customer_data') + def test_limit_to_latest_10(self, mock_ent_data, mock_braze): + enterprise_uuid = uuid4() + config = LearnerCreditRequestConfigurationFactory(active=True) + mock_ent_data.return_value = self._enterprise_data(enterprise_uuid) + + # Create 12 open requests + created = [] + for i in range(12): + r = LearnerCreditRequestFactory( + enterprise_customer_uuid=enterprise_uuid, + learner_credit_request_config=config, + state=SubsidyRequestStates.REQUESTED, + ) + r.created = timezone.now() - timedelta(minutes=120 - i) + r.save(update_fields=['created']) + created.append(r) + + send_learner_credit_bnr_admins_email_with_new_requests_task( + policy_uuid=str(uuid4()), + lc_request_config_uuid=str(config.uuid), + enterprise_customer_uuid=str(enterprise_uuid), + ) + + mock_braze.return_value.send_campaign_message.assert_called_once() + sent = mock_braze.return_value.send_campaign_message.call_args[1]['trigger_properties']['requests'] + assert len(sent) == 10 + # Ensure we got the latest 10 + expected = sorted(created, key=lambda r: r.created, reverse=True)[:10] + assert [i['user_email'] for i in sent] == [r.user.email for r in expected] + + @mock.patch('enterprise_access.apps.subsidy_request.tasks.BrazeApiClient') + @mock.patch('enterprise_access.apps.subsidy_request.tasks.LmsApiClient.get_enterprise_customer_data') + def test_no_admin_users_early_exit(self, mock_ent_data, mock_braze): + enterprise_uuid = uuid4() + config = LearnerCreditRequestConfigurationFactory(active=True) + data = self._enterprise_data(enterprise_uuid) + data['admin_users'] = [] + mock_ent_data.return_value = data + + LearnerCreditRequestFactory( + enterprise_customer_uuid=enterprise_uuid, + learner_credit_request_config=config, + state=SubsidyRequestStates.REQUESTED, + ) + + send_learner_credit_bnr_admins_email_with_new_requests_task( + policy_uuid=str(uuid4()), + lc_request_config_uuid=str(config.uuid), + enterprise_customer_uuid=str(enterprise_uuid), + ) + mock_braze.return_value.send_campaign_message.assert_not_called() + + @mock.patch('enterprise_access.apps.subsidy_request.tasks.BrazeApiClient', side_effect=Exception('boom')) + @mock.patch('enterprise_access.apps.subsidy_request.tasks.LmsApiClient.get_enterprise_customer_data') + def test_braze_exception_propagates(self, mock_ent_data, _mock_braze): + enterprise_uuid = uuid4() + config = LearnerCreditRequestConfigurationFactory(active=True) + mock_ent_data.return_value = self._enterprise_data(enterprise_uuid) + + LearnerCreditRequestFactory( + enterprise_customer_uuid=enterprise_uuid, + learner_credit_request_config=config, + state=SubsidyRequestStates.REQUESTED, + ) + + with pytest.raises(Exception): + send_learner_credit_bnr_admins_email_with_new_requests_task( + policy_uuid=str(uuid4()), + lc_request_config_uuid=str(config.uuid), + enterprise_customer_uuid=str(enterprise_uuid), + )