diff --git a/enterprise_access/apps/api/serializers/content_assignments/assignment.py b/enterprise_access/apps/api/serializers/content_assignments/assignment.py index b2eddc72..77b8503f 100644 --- a/enterprise_access/apps/api/serializers/content_assignments/assignment.py +++ b/enterprise_access/apps/api/serializers/content_assignments/assignment.py @@ -184,8 +184,12 @@ def get_error_reason(self, assignment): Resolves the error reason for the assignment, if any, for display purposes based on any associated actions. """ - # If the assignment is not in an errored state, there should be no error reason. - if assignment.state != LearnerContentAssignmentStateChoices.ERRORED: + # If the assignment is not in an errored or allocated state, + # there should be no error reason. + if assignment.state not in ( + LearnerContentAssignmentStateChoices.ERRORED, + LearnerContentAssignmentStateChoices.ALLOCATED, + ): return None # Assignment is an errored state, so determine the appropriate error reason so clients don't need to. diff --git a/enterprise_access/apps/api/v1/tests/test_assignment_views.py b/enterprise_access/apps/api/v1/tests/test_assignment_views.py index 327fe9e0..c156d214 100644 --- a/enterprise_access/apps/api/v1/tests/test_assignment_views.py +++ b/enterprise_access/apps/api/v1/tests/test_assignment_views.py @@ -12,6 +12,7 @@ from enterprise_access.apps.content_assignments.constants import ( NUM_DAYS_BEFORE_AUTO_EXPIRATION, + AssignmentActionErrors, AssignmentActions, AssignmentAutomaticExpiredReason, AssignmentLearnerStates, @@ -448,6 +449,53 @@ def test_retrieve(self, role_context_dict, mock_subsidy_record, mock_catalog_cli }, } + @ddt.data( + # A good admin role, and with a context matching the main testing customer. + {'system_wide_role': SYSTEM_ENTERPRISE_ADMIN_ROLE, 'context': str(TEST_ENTERPRISE_UUID)}, + # A good operator role, and with a context matching the main testing customer. + {'system_wide_role': SYSTEM_ENTERPRISE_OPERATOR_ROLE, 'context': str(TEST_ENTERPRISE_UUID)}, + ) + @mock.patch('enterprise_access.apps.content_metadata.api.EnterpriseCatalogApiClient', autospec=True) + @mock.patch.object(SubsidyAccessPolicy, 'subsidy_record', autospec=True) + def test_retrieve_allocated_with_notification_error( + self, role_context_dict, mock_subsidy_record, mock_catalog_client + ): + assignment = LearnerContentAssignmentFactory( + state=LearnerContentAssignmentStateChoices.ALLOCATED, + lms_user_id=98123, + transaction_uuid=None, + assignment_configuration=self.assignment_configuration, + content_key='edX+edXPrivacy101', + content_quantity=-321, + content_title='edx: Privacy 101' + ) + assignment.add_errored_notified_action(Exception('foo')) + + # Set the JWT-based auth that we'll use for every request. + self.set_jwt_cookie([role_context_dict]) + + # Mock results from the catalog content metadata API endpoint. + mock_catalog_client.return_value.catalog_content_metadata.return_value = self.mock_catalog_result + + # Mock results from the subsidy record. + mock_subsidy_record.return_value = self.mock_subsidy_record + + # Setup and call the retrieve endpoint. + detail_kwargs = { + 'assignment_configuration_uuid': str(TEST_ASSIGNMENT_CONFIG_UUID), + 'uuid': str(assignment.uuid), + } + detail_url = reverse('api:v1:admin-assignments-detail', kwargs=detail_kwargs) + response = self.client.get(detail_url) + + assert response.status_code == status.HTTP_200_OK + assert response.json().get('state') == LearnerContentAssignmentStateChoices.ALLOCATED + assert response.json().get('learner_state') == AssignmentLearnerStates.FAILED + assert response.json().get('error_reason') == { + 'action_type': AssignmentActions.NOTIFIED, + 'error_reason': AssignmentActionErrors.EMAIL_ERROR, + } + @ddt.data( # A good admin role, and with a context matching the main testing customer. {'system_wide_role': SYSTEM_ENTERPRISE_ADMIN_ROLE, 'context': str(TEST_ENTERPRISE_UUID)}, diff --git a/enterprise_access/apps/content_assignments/models.py b/enterprise_access/apps/content_assignments/models.py index 1d8a41a3..9b381bed 100644 --- a/enterprise_access/apps/content_assignments/models.py +++ b/enterprise_access/apps/content_assignments/models.py @@ -784,9 +784,23 @@ def annotate_dynamic_fields_onto_queryset(cls, queryset): error_reason__isnull=True, completed_at__isnull=False, ) + ), + # ... or if they have an errored notification. + has_errored_notification=Exists( + LearnerContentAssignmentAction.objects.filter( + assignment=OuterRef('uuid'), + action_type=AssignmentActions.NOTIFIED, + error_reason__isnull=False, + ) ) ).annotate( learner_state=Case( + When( + Q(state=LearnerContentAssignmentStateChoices.ALLOCATED) & + Q(has_errored_notification=True) & + Q(has_notification=False), + then=Value(AssignmentLearnerStates.FAILED), + ), When( Q(state=LearnerContentAssignmentStateChoices.ALLOCATED) & Q(has_notification=False), then=Value(AssignmentLearnerStates.NOTIFYING), diff --git a/enterprise_access/apps/content_assignments/tasks.py b/enterprise_access/apps/content_assignments/tasks.py index 0ae95208..2e2a1bff 100644 --- a/enterprise_access/apps/content_assignments/tasks.py +++ b/enterprise_access/apps/content_assignments/tasks.py @@ -458,6 +458,14 @@ class SendNotificationEmailTask(BaseAssignmentRetryAndErrorActionTask): def add_errored_action(self, assignment, exc): assignment.add_errored_notified_action(exc) + def progress_state_on_failure(self, assignment): + """ + Skip progressing the assignment state to `failed` (keeping it `allocated`) + so that the assignment remains functional and redeemable + for learners and appears as actionable to admins. + """ + logger.info('NOT progressing the assignment state to failed for notification failures.') + @shared_task(base=SendNotificationEmailTask) def send_email_for_new_assignment(new_assignment_uuid): diff --git a/enterprise_access/apps/content_assignments/tests/test_tasks.py b/enterprise_access/apps/content_assignments/tests/test_tasks.py index c8f2ed5b..8e037799 100644 --- a/enterprise_access/apps/content_assignments/tests/test_tasks.py +++ b/enterprise_access/apps/content_assignments/tests/test_tasks.py @@ -39,6 +39,8 @@ from enterprise_access.utils import get_automatic_expiration_date_and_reason from test_utils import APITestWithMocks +TEST_CONTENT_KEY = 'course:test_content' +TEST_ENTERPRISE_CUSTOMER_NAME = 'test-customer-name' TEST_ENTERPRISE_UUID = uuid4() TEST_EMAIL = 'foo@bar.com' TEST_LMS_USER_ID = 2 @@ -215,13 +217,37 @@ def setUpTestData(cls): assignment_configuration=cls.assignment_configuration, spend_limit=10000000, ) + cls.mock_enterprise_customer_data = { + 'uuid': TEST_ENTERPRISE_UUID, + 'slug': 'test-slug', + 'admin_users': [{ + 'email': 'test@admin.com', + 'lms_user_id': 1 + }], + 'name': TEST_ENTERPRISE_CUSTOMER_NAME, + } + cls.mock_content_metadata = { + 'key': TEST_CONTENT_KEY, + 'normalized_metadata': { + 'start_date': '2020-01-01T12:00:00Z', + 'end_date': '2022-01-01 12:00:00Z', + 'enroll_by_date': '2021-01-01T12:00:00Z', + 'content_price': 123, + }, + 'owners': [ + {'name': 'Smart Folks', 'logo_image_url': 'http://pictures.yes'}, + {'name': 'Good People', 'logo_image_url': 'http://pictures.nice'}, + ], + 'card_image_url': 'https://itsanimage.com', + } def setUp(self): super().setUp() self.course_name = 'test-course-name' - self.enterprise_customer_name = 'test-customer-name' + self.enterprise_customer_name = TEST_ENTERPRISE_CUSTOMER_NAME self.assignment = LearnerContentAssignmentFactory( uuid=TEST_ASSIGNMENT_UUID, + content_key=TEST_CONTENT_KEY, learner_email='TESTING THIS EMAIL', lms_user_id=TEST_LMS_USER_ID, assignment_configuration=self.assignment_configuration, @@ -386,36 +412,13 @@ def test_send_email_for_new_assignment( """ Verify send_email_for_new_assignment hits braze client with expected args """ - admin_email = 'test@admin.com' - mock_lms_client.return_value.get_enterprise_customer_data.return_value = { - 'uuid': TEST_ENTERPRISE_UUID, - 'slug': 'test-slug', - 'admin_users': [{ - 'email': admin_email, - 'lms_user_id': 1 - }], - 'name': self.enterprise_customer_name, - } + mock_lms_client.return_value.get_enterprise_customer_data.return_value = self.mock_enterprise_customer_data mock_recipient = { 'external_user_id': 1 } - mock_metadata = { - 'key': self.assignment.content_key, - 'normalized_metadata': { - 'start_date': '2020-01-01T12:00:00Z', - 'end_date': '2022-01-01 12:00:00Z', - 'enroll_by_date': '2021-01-01T12:00:00Z', - 'content_price': 123, - }, - 'owners': [ - {'name': 'Smart Folks', 'logo_image_url': 'http://pictures.yes'}, - {'name': 'Good People', 'logo_image_url': 'http://pictures.nice'}, - ], - 'card_image_url': 'https://itsanimage.com', - } mock_catalog_client.return_value.catalog_content_metadata.return_value = { 'count': 1, - 'results': [mock_metadata] + 'results': [self.mock_content_metadata] } # Set the subsidy expiration time to tomorrow @@ -425,7 +428,8 @@ def test_send_email_for_new_assignment( } mock_subsidy_client.retrieve_subsidy.return_value = mock_subsidy - mock_admin_mailto = f'mailto:{admin_email}' + admin_email = self.mock_enterprise_customer_data['admin_users'][0]['email'] + mock_admin_mailto = f"mailto:{admin_email}" mock_braze_client.return_value.create_recipient.return_value = mock_recipient mock_braze_client.return_value.generate_mailto_link.return_value = mock_admin_mailto @@ -446,13 +450,62 @@ def test_send_email_for_new_assignment( 'enrollment_deadline': 'Jan 01, 2021', 'start_date': 'Jan 01, 2020', 'course_partner': 'Smart Folks and Good People', - 'course_card_image': 'https://itsanimage.com', - 'learner_portal_link': '{}/{}'.format(settings.ENTERPRISE_LEARNER_PORTAL_URL, 'test-slug'), + 'course_card_image': self.mock_content_metadata['card_image_url'], + 'learner_portal_link': '{}/{}'.format( + settings.ENTERPRISE_LEARNER_PORTAL_URL, + self.mock_enterprise_customer_data['slug'] + ), 'action_required_by_timestamp': '2021-01-01T12:00:00Z' }, ) assert mock_braze_client.return_value.send_campaign_message.call_count == 1 + @mock.patch('enterprise_access.apps.subsidy_access_policy.models.SubsidyAccessPolicy.subsidy_client') + @mock.patch('enterprise_access.apps.content_metadata.api.EnterpriseCatalogApiClient') + @mock.patch('enterprise_access.apps.content_assignments.tasks.LmsApiClient') + @mock.patch('enterprise_access.apps.content_assignments.tasks.BrazeApiClient') + def test_send_email_for_new_assignment_failure( + self, + mock_braze_client, + mock_lms_client, + mock_catalog_client, + mock_subsidy_client, + ): + """ + Verify send_email_for_new_assignment does not change the state of the + assignment record on failure. + """ + mock_lms_client.return_value.get_enterprise_customer_data.return_value = self.mock_enterprise_customer_data + mock_recipient = { + 'external_user_id': 1 + } + mock_catalog_client.return_value.catalog_content_metadata.return_value = { + 'count': 1, + 'results': [self.mock_content_metadata] + } + + # Set the subsidy expiration time to tomorrow + mock_subsidy = { + 'uuid': self.policy.subsidy_uuid, + 'expiration_datetime': (now() + timedelta(days=1)).strftime('%Y-%m-%d %H:%M:%SZ'), + } + mock_subsidy_client.retrieve_subsidy.return_value = mock_subsidy + + braze_client_instance = mock_braze_client.return_value + braze_client_instance.send_campaign_message.side_effect = Exception('foo') + + admin_email = self.mock_enterprise_customer_data['admin_users'][0]['email'] + mock_admin_mailto = f"mailto:{admin_email}" + braze_client_instance.create_recipient.return_value = mock_recipient + braze_client_instance.generate_mailto_link.return_value = mock_admin_mailto + + send_email_for_new_assignment.delay(self.assignment.uuid) + self.assertEqual(self.assignment.state, LearnerContentAssignmentStateChoices.ALLOCATED) + self.assertTrue(self.assignment.actions.filter( + error_reason=AssignmentActionErrors.EMAIL_ERROR, + action_type=AssignmentActions.NOTIFIED, + ).exists()) + @mock.patch('enterprise_access.apps.subsidy_access_policy.models.SubsidyAccessPolicy.objects') @mock.patch('enterprise_access.apps.content_assignments.tasks.LmsApiClient') @mock.patch('enterprise_access.apps.content_assignments.tasks.BrazeApiClient')