Skip to content

Commit

Permalink
feat: failed notifications for assignments leave state as allocated
Browse files Browse the repository at this point in the history
When the assignment email notification task fails, the corresponding
assignment is now NOT moved to an errored state, but instead left
in an allocated state. Furthermore, the admin-serializer for the assignment
now indicates a "failed" _learner_ state, and an error reason related
to the failed notification attempt.
ENT-9037
  • Loading branch information
iloveagent57 committed Sep 4, 2024
1 parent 8fbda7f commit 4ec1a6e
Show file tree
Hide file tree
Showing 5 changed files with 158 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
48 changes: 48 additions & 0 deletions enterprise_access/apps/api/v1/tests/test_assignment_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

from enterprise_access.apps.content_assignments.constants import (
NUM_DAYS_BEFORE_AUTO_EXPIRATION,
AssignmentActionErrors,
AssignmentActions,
AssignmentAutomaticExpiredReason,
AssignmentLearnerStates,
Expand Down Expand Up @@ -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') == '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)},
Expand Down
14 changes: 14 additions & 0 deletions enterprise_access/apps/content_assignments/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
8 changes: 8 additions & 0 deletions enterprise_access/apps/content_assignments/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 appear as "Waiting on learner..." 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):
Expand Down
111 changes: 82 additions & 29 deletions enterprise_access/apps/content_assignments/tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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')
Expand Down

0 comments on commit 4ec1a6e

Please sign in to comment.