From 63abe56a8cbc48fd9db828976ef1cd824617105a Mon Sep 17 00:00:00 2001 From: Adam Stankiewicz Date: Thu, 19 Sep 2024 16:34:29 -0400 Subject: [PATCH] fix: update `parent_content_key` and `is_assigned_course_run` on assignment re-allocation (#564) --- .../apps/content_assignments/api.py | 36 +++- .../automatically_nudge_assignments.py | 1 + .../test_automatically_nudge_assignments.py | 30 ++- .../content_assignments/tests/factories.py | 1 + .../content_assignments/tests/test_api.py | 170 +++++++++++++-- .../content_assignments/tests/test_tasks.py | 200 ++++++++++++++---- 6 files changed, 371 insertions(+), 67 deletions(-) diff --git a/enterprise_access/apps/content_assignments/api.py b/enterprise_access/apps/content_assignments/api.py index 2645ba0b..30bffbab 100644 --- a/enterprise_access/apps/content_assignments/api.py +++ b/enterprise_access/apps/content_assignments/api.py @@ -55,6 +55,7 @@ 'lms_user_id', 'learner_email', 'allocation_batch_id', 'content_quantity', 'state', 'preferred_course_run_key', 'allocated_at', 'cancelled_at', 'expired_at', 'errored_at', + 'parent_content_key', 'is_assigned_course_run', ] @@ -329,9 +330,16 @@ def allocate_assignments(assignment_configuration, learner_emails, content_key, existing_assignments_needs_update = set() # This step to find and update the preferred_course_run_key is required in order - # for nudge emails to target the start date of the new run. + # for nudge emails to target the start date of the new run. For run-based assignments, + # the preferred_course_run_key is the same as the assignment's content_key. preferred_course_run_key = _get_preferred_course_run_key(assignment_configuration, content_key) + # Determine if the assignment's content_key is a course run or a course key based + # on an associated parent content key. If the parent content key is None, then the + # assignment is for a course; otherwise, it's an assignment for a course run. + parent_content_key = _get_parent_content_key(assignment_configuration, content_key) + is_assigned_course_run = bool(parent_content_key) + # Split up the existing assignment records by state for assignment in existing_assignments: if not assignment.lms_user_id: @@ -349,7 +357,14 @@ def allocate_assignments(assignment_configuration, learner_emails, content_key, existing_assignments_needs_update.add(assignment) if assignment.state in LearnerContentAssignmentStateChoices.REALLOCATE_STATES: - _reallocate_assignment(assignment, content_quantity, allocation_batch_id, preferred_course_run_key) + _reallocate_assignment( + assignment, + content_quantity, + allocation_batch_id, + preferred_course_run_key, + parent_content_key, + is_assigned_course_run, + ) existing_assignments_needs_update.add(assignment) elif assignment.state == LearnerContentAssignmentStateChoices.ALLOCATED: # For some already-allocated assignments being re-assigned, we might still need to update the preferred @@ -357,6 +372,13 @@ def allocate_assignments(assignment_configuration, learner_emails, content_key, if assignment.preferred_course_run_key != preferred_course_run_key: assignment.preferred_course_run_key = preferred_course_run_key existing_assignments_needs_update.add(assignment) + # Update the parent_content_key and is_assigned_course_run fields if they have changed. + if assignment.parent_content_key != parent_content_key: + assignment.parent_content_key = parent_content_key + existing_assignments_needs_update.add(assignment) + if assignment.is_assigned_course_run != is_assigned_course_run: + assignment.is_assigned_course_run = is_assigned_course_run + existing_assignments_needs_update.add(assignment) learner_emails_with_existing_assignments.add(assignment.learner_email.lower()) @@ -505,7 +527,13 @@ def _get_existing_assignments_for_allocation( return existing_assignments -def _reallocate_assignment(assignment, content_quantity, allocation_batch_id, preferred_course_run_key): +def _reallocate_assignment( + assignment, + content_quantity, + allocation_batch_id, + preferred_course_run_key, + parent_content_key, + is_assigned_course_run): """ Modifies a ``LearnerContentAssignment`` record during the allocation flow. The record is **not** saved. @@ -519,6 +547,8 @@ def _reallocate_assignment(assignment, content_quantity, allocation_batch_id, pr assignment.expired_at = None assignment.errored_at = None assignment.preferred_course_run_key = preferred_course_run_key + assignment.parent_content_key = parent_content_key + assignment.is_assigned_course_run = is_assigned_course_run # Prevent invalid data from entering the database by calling the low-level full_clean() function manually. assignment.full_clean() return assignment diff --git a/enterprise_access/apps/content_assignments/management/commands/automatically_nudge_assignments.py b/enterprise_access/apps/content_assignments/management/commands/automatically_nudge_assignments.py index cd35a5da..cd9a6e7a 100644 --- a/enterprise_access/apps/content_assignments/management/commands/automatically_nudge_assignments.py +++ b/enterprise_access/apps/content_assignments/management/commands/automatically_nudge_assignments.py @@ -171,6 +171,7 @@ def handle(self, *args, **options): if datetime_start_date is not None else False ) + if not can_send_nudge_notification_in_advance: logger.info( ( diff --git a/enterprise_access/apps/content_assignments/management/commands/tests/test_automatically_nudge_assignments.py b/enterprise_access/apps/content_assignments/management/commands/tests/test_automatically_nudge_assignments.py index 00e69f50..5753d695 100644 --- a/enterprise_access/apps/content_assignments/management/commands/tests/test_automatically_nudge_assignments.py +++ b/enterprise_access/apps/content_assignments/management/commands/tests/test_automatically_nudge_assignments.py @@ -49,7 +49,9 @@ def setUp(self): assignment_configuration=self.assignment_configuration, learner_email='alice@foo.com', lms_user_id=None, - content_key='edX+edXPrivacy101', + content_key='course-v1:edX+edXPrivacy101+1T2022', + parent_content_key='edX+edXPrivacy101', + is_assigned_course_run=True, preferred_course_run_key='course-v1:edX+edXPrivacy101+1T2022', content_title='edx: Privacy 101', content_quantity=-123, @@ -61,6 +63,8 @@ def setUp(self): learner_email='bob@foo.com', lms_user_id=None, content_key='edX+edXAccessibility101', + parent_content_key=None, + is_assigned_course_run=False, preferred_course_run_key='course-v1:edX+edXAccessibility101+1T2022', content_title='edx: Accessibility 101', content_quantity=-456, @@ -72,6 +76,8 @@ def setUp(self): learner_email='rob@foo.com', lms_user_id=None, content_key='edX+edXQuadrilateral306090', + parent_content_key=None, + is_assigned_course_run=False, preferred_course_run_key='course-v1:edX+edXQuadrilateral306090+1T2022', content_title='edx: Quadrilateral 306090', content_quantity=-456, @@ -82,6 +88,8 @@ def setUp(self): learner_email='richard@foo.com', lms_user_id=None, content_key='edX+edXTesseract4D', + parent_content_key=None, + is_assigned_course_run=False, preferred_course_run_key='course-v1:edX+edXTesseract4D+1T2022', content_title='edx: Tesseract 4D', content_quantity=-456, @@ -93,6 +101,8 @@ def setUp(self): learner_email='ella@foo.com', lms_user_id=None, content_key='edX+edXIsoscelesPyramid2012', + parent_content_key=None, + is_assigned_course_run=False, preferred_course_run_key='course-v1:edX+edXIsoscelesPyramid2012+1T2022', content_title='edx: IsoscelesPyramid 2012', content_quantity=-456, @@ -103,6 +113,8 @@ def setUp(self): learner_email='bella@foo.com', lms_user_id=None, content_key='edX+edXBeeHivesAlive0220', + parent_content_key=None, + is_assigned_course_run=False, preferred_course_run_key='course-v1:edX+edXBeeHivesAlive0220+1T2022', content_title='edx: BeeHivesAlive 0220', content_quantity=-456, @@ -229,7 +241,9 @@ def test_command( }, 'course_type': 'executive-education-2u', }, - 'edX+edXPrivacy101': { + # `self.alice_assignment` is an assignment for a course run, so its run-based content_key + # should be used as the key in this dict. + 'course-v1:edX+edXPrivacy101+1T2022': { 'key': 'edX+edXPrivacy101', 'normalized_metadata': { 'start_date': start_date.strftime("%Y-%m-%dT%H:%M:%SZ"), @@ -367,7 +381,9 @@ def test_command_multiple_assignment_dates( }, 'course_type': 'executive-education-2u', }, - 'edX+edXPrivacy101': { + # `self.alice_assignment` is an assignment for a course run, so its run-based content_key + # should be used as the key in this dict. + 'course-v1:edX+edXPrivacy101+1T2022': { 'key': 'edX+edXPrivacy101', 'course_runs': [ { @@ -458,7 +474,9 @@ def test_command_multiple_assignment_course_types( }, }, }, - 'edX+edXPrivacy101': { + # `self.alice_assignment` is an assignment for a course run, so its run-based content_key + # should be used as the key in this dict. + 'course-v1:edX+edXPrivacy101+1T2022': { 'key': 'edX+edXPrivacy101', 'course_type': 'executive-education-2u', 'normalized_metadata': { @@ -587,7 +605,9 @@ def test_command_multiple_assignment_states( }, }, }, - 'edX+edXPrivacy101': { + # `self.alice_assignment` is an assignment for a course run, so its run-based content_key + # should be used as the key in this dict. + 'course-v1:edX+edXPrivacy101+1T2022': { 'key': 'edX+edXPrivacy101', 'course_type': 'executive-education-2u', 'normalized_metadata': { diff --git a/enterprise_access/apps/content_assignments/tests/factories.py b/enterprise_access/apps/content_assignments/tests/factories.py index 14692d77..e928c71d 100644 --- a/enterprise_access/apps/content_assignments/tests/factories.py +++ b/enterprise_access/apps/content_assignments/tests/factories.py @@ -38,5 +38,6 @@ class Meta: lms_user_id = factory.LazyAttribute(lambda _: FAKER.pyint()) content_key = factory.LazyAttribute(lambda _: random_content_key()) parent_content_key = factory.LazyAttribute(lambda _: random_parent_content_key()) + is_assigned_course_run = True content_title = factory.LazyAttribute(lambda _: f'{FAKER.word()}: a master class') content_quantity = factory.LazyAttribute(lambda _: FAKER.pyfloat(positive=False, right_digits=0)) diff --git a/enterprise_access/apps/content_assignments/tests/test_api.py b/enterprise_access/apps/content_assignments/tests/test_api.py index ab75ecf2..8b770ce4 100644 --- a/enterprise_access/apps/content_assignments/tests/test_api.py +++ b/enterprise_access/apps/content_assignments/tests/test_api.py @@ -42,6 +42,22 @@ def delta_t(as_string=False, **kwargs): return datetime_obj +def expirable_assignments_with_content_type(): + """ + Returns a list of tuples containing expirable assignment states and the corresponding + assignment content type (course-level or run-based). + + Each tuple contains: + - expirable_assignment_state: The current state of the assignment. + - is_assigned_course_run: Boolean indicating if the assignment is course-level (True) or run-based (False). + """ + return [ + (expirable_assignment_state, is_assigned_course_run) + for expirable_assignment_state in LearnerContentAssignmentStateChoices.EXPIRABLE_STATES + for is_assigned_course_run in [True, False] + ] + + @ddt.ddt class TestContentAssignmentApi(TestCase): """ @@ -332,23 +348,43 @@ def test_allocate_assignments_negative_quantity(self): content_price_cents, ) + # pylint: disable=too-many-statements @mock.patch('enterprise_access.apps.content_assignments.api.send_email_for_new_assignment') @mock.patch('enterprise_access.apps.content_assignments.api.create_pending_enterprise_learner_for_assignment_task') @mock.patch( 'enterprise_access.apps.content_assignments.api.get_and_cache_content_metadata', return_value=mock.MagicMock(), ) + @ddt.data( + {'is_assigned_course_run': False}, + {'is_assigned_course_run': True}, + ) + @ddt.unpack def test_allocate_assignments_happy_path( - self, mock_get_and_cache_content_metadata, mock_pending_learner_task, mock_new_assignment_email_task + self, + mock_get_and_cache_content_metadata, + mock_pending_learner_task, + mock_new_assignment_email_task, + is_assigned_course_run, ): """ Tests the allocation of new assignments against a given configuration. """ - content_key = 'demoX' - course_run_key_old = 'demoX+1T2022' - course_run_key = 'demoX+2T2023' + course_key = 'edX+DemoX' content_title = 'edx: Demo 101' content_price_cents = 100 + + # Course-level assignments + content_key = course_key + parent_content_key = None + course_run_key_old = 'course-v1:edX+DemoX+1T2022' + course_run_key = 'course-v1:edX+DemoX+2T2023' + + # Course run-level assignments + if is_assigned_course_run: + content_key = course_run_key + parent_content_key = course_key + # add a duplicate email to the input list to ensure only one # allocation occurs. # We throw a couple ALL UPPER CASE emails into the requested emails to allocate @@ -365,18 +401,25 @@ def test_allocate_assignments_happy_path( 'eugene', 'faythe', 'erin', + 'mary', 'grace', + 'xavier', + 'steve', + 'kian', ) ] mock_get_and_cache_content_metadata.return_value = { 'content_title': content_title, + 'content_key': course_key, 'course_run_key': course_run_key, } default_factory_options = { 'assignment_configuration': self.assignment_configuration, 'content_key': content_key, - 'preferred_course_run_key': course_run_key, + 'parent_content_key': parent_content_key, + 'is_assigned_course_run': is_assigned_course_run, + 'preferred_course_run_key': content_key if is_assigned_course_run else course_run_key, 'content_title': content_title, 'content_quantity': -content_price_cents, } @@ -395,6 +438,22 @@ def test_allocate_assignments_happy_path( # outdated run should trigger update. 'preferred_course_run_key': course_run_key_old, }) + # Allocated assignment SHOULD be updated because it had an outdated parent_content_key. + allocated_assignment_old_parent_content_key = LearnerContentAssignmentFactory.create(**{ + **default_factory_options, + 'learner_email': 'mary@foo.com', + 'state': LearnerContentAssignmentStateChoices.ALLOCATED, + # outdated parent_content_key should trigger update. + 'parent_content_key': None, + }) + # Allocated assignment SHOULD be updated because it had an outdated is_assigned_course_run. + allocated_assignment_old_run_based = LearnerContentAssignmentFactory.create(**{ + **default_factory_options, + 'learner_email': 'xavier@foo.com', + 'state': LearnerContentAssignmentStateChoices.ALLOCATED, + # outdated is_assigned_course_run should trigger update. + 'is_assigned_course_run': False, + }) # Allocated assignment SHOULD be updated because it had a NULL run. allocated_assignment_null_run = LearnerContentAssignmentFactory.create(**{ **default_factory_options, @@ -423,6 +482,22 @@ def test_allocate_assignments_happy_path( # outdated run should trigger update. 'preferred_course_run_key': course_run_key_old, }) + # Cancelled assignment should be updated/re-allocated (including recieving new parent_content_key). + cancelled_assignment_old_parent_content_key = LearnerContentAssignmentFactory.create(**{ + **default_factory_options, + 'learner_email': 'steve@foo.com', + 'state': LearnerContentAssignmentStateChoices.CANCELLED, + # outdated parent_content_key should trigger update. + 'parent_content_key': None, + }) + # Cancelled assignment should be updated/re-allocated (including recieving new is_assigned_course_run). + cancelled_assignment_old_run_based = LearnerContentAssignmentFactory.create(**{ + **default_factory_options, + 'learner_email': 'kian@foo.com', + 'state': LearnerContentAssignmentStateChoices.CANCELLED, + # outdated is_assigned_course_run should trigger update. + 'is_assigned_course_run': False, + }) # Errored assignment should be updated/re-allocated. errored_assignment = LearnerContentAssignmentFactory.create(**{ **default_factory_options, @@ -438,26 +513,52 @@ def test_allocate_assignments_happy_path( ) # Refresh from db to get any updates reflected in the python objects. - for record in ( + assignments_to_refresh = ( allocated_assignment, allocated_assignment_old_run, allocated_assignment_null_run, accepted_assignment, cancelled_assignment, cancelled_assignment_old_run, + cancelled_assignment_old_parent_content_key, + cancelled_assignment_old_run_based, errored_assignment, - ): + ) + if is_assigned_course_run: + assignments_to_refresh += ( + allocated_assignment_old_parent_content_key, + allocated_assignment_old_run_based, + ) + + for record in assignments_to_refresh: record.refresh_from_db() - # The allocated assignments with outdated runs, errored assignments, and cancelled assignments should be the - # only things updated. + # Create list of assignments expected to be updated, including: + # - The allocated assignments with outdated: preferred course run, parent_content_key, is_assigned_course_run. + # - Errored assignments + # - Cancelled assignments expected_updated_assignments = ( allocated_assignment_old_run, allocated_assignment_null_run, cancelled_assignment, cancelled_assignment_old_run, + cancelled_assignment_old_parent_content_key, + cancelled_assignment_old_run_based, errored_assignment, ) + + if is_assigned_course_run: + expected_updated_assignments += ( + allocated_assignment_old_parent_content_key, + allocated_assignment_old_run_based, + ) + + if is_assigned_course_run: + expected_updated_assignments += ( + allocated_assignment_old_parent_content_key, + allocated_assignment_old_run_based, + ) + self.assertEqual( {record.uuid for record in allocation_results['updated']}, {assignment.uuid for assignment in expected_updated_assignments}, @@ -470,6 +571,11 @@ def test_allocate_assignments_happy_path( allocated_assignment, accepted_assignment, ) + if not is_assigned_course_run: + expected_no_change_assignments += ( + allocated_assignment_old_parent_content_key, + allocated_assignment_old_run_based, + ) self.assertEqual( {record.uuid for record in allocation_results['no_change']}, {assignment.uuid for assignment in expected_no_change_assignments}, @@ -480,10 +586,18 @@ def test_allocate_assignments_happy_path( # The existing assignments should be 'allocated' now, except for the already accepted one self.assertEqual(cancelled_assignment.state, LearnerContentAssignmentStateChoices.ALLOCATED) self.assertEqual(cancelled_assignment_old_run.state, LearnerContentAssignmentStateChoices.ALLOCATED) + self.assertEqual( + cancelled_assignment_old_parent_content_key.state, LearnerContentAssignmentStateChoices.ALLOCATED + ) + self.assertEqual(cancelled_assignment_old_run_based.state, LearnerContentAssignmentStateChoices.ALLOCATED) self.assertEqual(errored_assignment.state, LearnerContentAssignmentStateChoices.ALLOCATED) self.assertEqual(allocated_assignment.state, LearnerContentAssignmentStateChoices.ALLOCATED) self.assertEqual(allocated_assignment_old_run.state, LearnerContentAssignmentStateChoices.ALLOCATED) self.assertEqual(allocated_assignment_null_run.state, LearnerContentAssignmentStateChoices.ALLOCATED) + self.assertEqual( + allocated_assignment_old_parent_content_key.state, LearnerContentAssignmentStateChoices.ALLOCATED + ) + self.assertEqual(allocated_assignment_old_run_based.state, LearnerContentAssignmentStateChoices.ALLOCATED) self.assertEqual(accepted_assignment.state, LearnerContentAssignmentStateChoices.ACCEPTED) # We should have created only one new, allocated assignment for eugene@foo.com @@ -836,23 +950,33 @@ def test_expire_one_assignment_automatically( mock_expired_email.delay.assert_called_once_with(assignment.uuid) @ddt.data( - *LearnerContentAssignmentStateChoices.EXPIRABLE_STATES + *expirable_assignments_with_content_type() ) + @ddt.unpack @mock.patch('enterprise_access.apps.content_assignments.api.send_assignment_automatically_expired_email') def test_expire_assignments_with_passed_enroll_by_date( self, - assignment_state, + expirable_assignment_state, + is_assigned_course_run, mock_expired_email, ): """ Tests that we expire assignments with a passed enroll_by_date """ - content_key = 'edX+DemoX' + course_key = 'edX+DemoX' course_run_key = 'course-v1:edX+DemoX+T2024' + content_key = course_key + parent_content_key = None + if is_assigned_course_run: + content_key = course_run_key + parent_content_key = course_key + assignment = LearnerContentAssignmentFactory.create( - content_key='demoX', + content_key=content_key, + parent_content_key=parent_content_key, + is_assigned_course_run=is_assigned_course_run, assignment_configuration=self.assignment_configuration, - state=assignment_state, + state=expirable_assignment_state, learner_email='larry@stooges.com', lms_user_id=12345, ) @@ -883,23 +1007,33 @@ def test_expire_assignments_with_passed_enroll_by_date( mock_expired_email.delay.assert_called_once_with(assignment.uuid) @ddt.data( - *LearnerContentAssignmentStateChoices.EXPIRABLE_STATES + *expirable_assignments_with_content_type() ) + @ddt.unpack @mock.patch('enterprise_access.apps.content_assignments.api.send_assignment_automatically_expired_email') def test_expire_assignments_with_expired_subsidy( self, - assignment_state, + expirable_assignment_state, + is_assigned_course_run, mock_expired_email, ): """ Tests that we expire assignments with an underlying subsidy that has expired. """ - content_key = 'edX+DemoX' + course_key = 'edX+DemoX' course_run_key = 'course-v1:edX+DemoX+T2024' + content_key = course_key + parent_content_key = None + if is_assigned_course_run: + content_key = course_run_key + parent_content_key = course_key + assignment = LearnerContentAssignmentFactory.create( assignment_configuration=self.assignment_configuration, content_key=content_key, - state=assignment_state, + parent_content_key=parent_content_key, + is_assigned_course_run=is_assigned_course_run, + state=expirable_assignment_state, learner_email='larry@stooges.com', lms_user_id=12345, ) diff --git a/enterprise_access/apps/content_assignments/tests/test_tasks.py b/enterprise_access/apps/content_assignments/tests/test_tasks.py index 8e037799..ae3be636 100644 --- a/enterprise_access/apps/content_assignments/tests/test_tasks.py +++ b/enterprise_access/apps/content_assignments/tests/test_tasks.py @@ -39,12 +39,15 @@ from enterprise_access.utils import get_automatic_expiration_date_and_reason from test_utils import APITestWithMocks -TEST_CONTENT_KEY = 'course:test_content' +TEST_COURSE_KEY = 'edX+DemoX' +TEST_COURSE_RUN_KEY = 'course-v1:edX+DemoX+Demo_Course' TEST_ENTERPRISE_CUSTOMER_NAME = 'test-customer-name' TEST_ENTERPRISE_UUID = uuid4() TEST_EMAIL = 'foo@bar.com' -TEST_LMS_USER_ID = 2 +TEST_LMS_USER_ID = 1 +TEST_LMS_USER_ID_2 = 2 TEST_ASSIGNMENT_UUID = uuid4() +TEST_RUN_BASED_ASSIGNMENT_UUID = uuid4() @ddt.ddt @@ -227,13 +230,21 @@ def setUpTestData(cls): 'name': TEST_ENTERPRISE_CUSTOMER_NAME, } cls.mock_content_metadata = { - 'key': TEST_CONTENT_KEY, + 'key': TEST_COURSE_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, }, + 'normalized_metadata_by_run': { + TEST_COURSE_RUN_KEY: { + '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'}, @@ -245,13 +256,24 @@ def setUp(self): super().setUp() self.course_name = 'test-course-name' self.enterprise_customer_name = TEST_ENTERPRISE_CUSTOMER_NAME - self.assignment = LearnerContentAssignmentFactory( + self.assignment_course = LearnerContentAssignmentFactory( uuid=TEST_ASSIGNMENT_UUID, - content_key=TEST_CONTENT_KEY, - learner_email='TESTING THIS EMAIL', + content_key=TEST_COURSE_KEY, + parent_content_key=None, + is_assigned_course_run=False, + learner_email='edx@example.com', lms_user_id=TEST_LMS_USER_ID, assignment_configuration=self.assignment_configuration, ) + self.assignment_course_run = LearnerContentAssignmentFactory( + uuid=TEST_RUN_BASED_ASSIGNMENT_UUID, + content_key=TEST_COURSE_RUN_KEY, + parent_content_key=TEST_COURSE_KEY, + is_assigned_course_run=True, + learner_email='learner@example.com', + lms_user_id=TEST_LMS_USER_ID_2, + assignment_configuration=self.assignment_configuration, + ) def tearDown(self): super().tearDown() @@ -263,9 +285,17 @@ def tearDown(self): @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') + @ddt.data( + {'is_assigned_course_run': False}, + {'is_assigned_course_run': True}, + ) + @ddt.unpack def test_send_cancel_email_for_pending_assignment( - self, mock_braze_client, mock_lms_client, + self, + mock_braze_client, + mock_lms_client, mock_policy_model, # pylint: disable=unused-argument + is_assigned_course_run, ): """ Verify send_cancel_email_for_pending_assignment hits braze client with expected args @@ -284,10 +314,12 @@ def test_send_cancel_email_for_pending_assignment( 'external_user_id': 1 } + assignment = self.assignment_course_run if is_assigned_course_run else self.assignment_course + 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 - send_cancel_email_for_pending_assignment(self.assignment.uuid) + send_cancel_email_for_pending_assignment(assignment.uuid) # Make sure our LMS client got called correct times and with what we expected mock_lms_client.return_value.get_enterprise_customer_data.assert_called_with( @@ -300,7 +332,7 @@ def test_send_cancel_email_for_pending_assignment( trigger_properties={ 'contact_admin_link': mock_admin_mailto, 'organization': self.enterprise_customer_name, - 'course_title': self.assignment.content_title + 'course_title': assignment.content_title }, ) assert mock_braze_client.return_value.send_campaign_message.call_count == 1 @@ -309,22 +341,32 @@ def test_send_cancel_email_for_pending_assignment( @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') - @ddt.data(True, False) + @ddt.data( + {'is_logistrated': True, 'is_assigned_course_run': False}, + {'is_logistrated': False, 'is_assigned_course_run': False}, + {'is_logistrated': True, 'is_assigned_course_run': True}, + {'is_logistrated': False, 'is_assigned_course_run': True}, + ) + @ddt.unpack def test_send_reminder_email_for_pending_assignment( self, - is_logistrated, mock_braze_client_class, mock_lms_client, mock_catalog_client, mock_subsidy_client, + is_logistrated, + is_assigned_course_run, ): """ Verify send_reminder_email_for_pending_assignment hits braze client with expected args """ mock_braze_client = mock_braze_client_class.return_value + + assignment = self.assignment_course_run if is_assigned_course_run else self.assignment_course + if not is_logistrated: - self.assignment.lms_user_id = None - self.assignment.save() + assignment.lms_user_id = None + assignment.save() admin_email = 'test@admin.com' mock_lms_client.return_value.get_enterprise_customer_data.return_value = { @@ -337,13 +379,21 @@ def test_send_reminder_email_for_pending_assignment( 'name': self.enterprise_customer_name, } mock_metadata = { - 'key': self.assignment.content_key, + 'key': assignment.content_key, 'normalized_metadata': { 'start_date': '2020-01-01 12:00:00Z', 'end_date': '2022-01-01 12:00:00Z', 'enroll_by_date': '2021-01-01 12:00:00Z', 'content_price': 123, }, + 'normalized_metadata_by_run': { + TEST_COURSE_RUN_KEY: { + 'start_date': '2020-01-01 12:00:00Z', + 'end_date': '2022-01-01 12:00:00Z', + 'enroll_by_date': '2021-01-01 12:00:00Z', + 'content_price': 123, + }, + }, 'owners': [ {'name': 'Smart Folks', 'logo_image_url': 'http://pictures.yes'}, {'name': 'Good People', 'logo_image_url': 'http://pictures.nice'}, @@ -364,7 +414,7 @@ def test_send_reminder_email_for_pending_assignment( mock_subsidy_client.retrieve_subsidy.return_value = mock_subsidy mock_braze_client.generate_mailto_link.return_value = f'mailto:{admin_email}' - send_reminder_email_for_pending_assignment(self.assignment.uuid) + send_reminder_email_for_pending_assignment(assignment.uuid) # Make sure our LMS client got called correct times and with what we expected mock_lms_client.return_value.get_enterprise_customer_data.assert_called_with( @@ -379,7 +429,7 @@ def test_send_reminder_email_for_pending_assignment( self.assertFalse(mock_braze_client.create_braze_alias.called) else: mock_braze_client.create_braze_alias.assert_called_once_with( - [self.assignment.learner_email], + [assignment.learner_email], ENTERPRISE_BRAZE_ALIAS_LABEL, ) mock_braze_client.send_campaign_message.assert_called_once_with( @@ -388,7 +438,7 @@ def test_send_reminder_email_for_pending_assignment( trigger_properties={ 'contact_admin_link': f'mailto:{admin_email}', 'organization': self.enterprise_customer_name, - 'course_title': self.assignment.content_title, + 'course_title': assignment.content_title, 'enrollment_deadline': 'Jan 01, 2021', 'start_date': 'Jan 01, 2020', 'course_partner': 'Smart Folks, Good People, and Fast Learners', @@ -402,12 +452,18 @@ def test_send_reminder_email_for_pending_assignment( @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') + @ddt.data( + {'is_assigned_course_run': False}, + {'is_assigned_course_run': True}, + ) + @ddt.unpack def test_send_email_for_new_assignment( self, mock_braze_client, mock_lms_client, mock_catalog_client, mock_subsidy_client, + is_assigned_course_run, ): """ Verify send_email_for_new_assignment hits braze client with expected args @@ -433,7 +489,9 @@ def test_send_email_for_new_assignment( mock_braze_client.return_value.create_recipient.return_value = mock_recipient mock_braze_client.return_value.generate_mailto_link.return_value = mock_admin_mailto - send_email_for_new_assignment(self.assignment.uuid) + assignment = self.assignment_course_run if is_assigned_course_run else self.assignment_course + + send_email_for_new_assignment(assignment.uuid) # Make sure our LMS client got called correct times and with what we expected mock_lms_client.return_value.get_enterprise_customer_data.assert_called_with( @@ -446,7 +504,7 @@ def test_send_email_for_new_assignment( trigger_properties={ 'contact_admin_link': mock_admin_mailto, 'organization': self.enterprise_customer_name, - 'course_title': self.assignment.content_title, + 'course_title': assignment.content_title, 'enrollment_deadline': 'Jan 01, 2021', 'start_date': 'Jan 01, 2020', 'course_partner': 'Smart Folks and Good People', @@ -464,12 +522,18 @@ def test_send_email_for_new_assignment( @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') + @ddt.data( + {'is_assigned_course_run': False}, + {'is_assigned_course_run': True}, + ) + @ddt.unpack def test_send_email_for_new_assignment_failure( self, mock_braze_client, mock_lms_client, mock_catalog_client, mock_subsidy_client, + is_assigned_course_run, ): """ Verify send_email_for_new_assignment does not change the state of the @@ -499,9 +563,11 @@ def test_send_email_for_new_assignment_failure( 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( + assignment = self.assignment_course_run if is_assigned_course_run else self.assignment_course + + send_email_for_new_assignment.delay(assignment.uuid) + self.assertEqual(assignment.state, LearnerContentAssignmentStateChoices.ALLOCATED) + self.assertTrue(assignment.actions.filter( error_reason=AssignmentActionErrors.EMAIL_ERROR, action_type=AssignmentActions.NOTIFIED, ).exists()) @@ -509,9 +575,17 @@ def test_send_email_for_new_assignment_failure( @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') + @ddt.data( + {'is_assigned_course_run': False}, + {'is_assigned_course_run': True}, + ) + @ddt.unpack def test_send_assignment_automatically_expired_email( - self, mock_braze_client, mock_lms_client, + self, + mock_braze_client, + mock_lms_client, mock_subsidy_model, # pylint: disable=unused-argument + is_assigned_course_run, ): """ Verify `send_assignment_automatically_expired_email` task work as expected @@ -534,14 +608,16 @@ def test_send_assignment_automatically_expired_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 - send_assignment_automatically_expired_email(self.assignment.uuid) + assignment = self.assignment_course_run if is_assigned_course_run else self.assignment_course + + send_assignment_automatically_expired_email(assignment.uuid) mock_braze_client.return_value.send_campaign_message.assert_any_call( 'test-assignment-expired-campaign', recipients=[mock_recipient], trigger_properties={ 'contact_admin_link': mock_admin_mailto, - 'course_title': self.assignment.content_title, + 'course_title': assignment.content_title, 'organization': self.enterprise_customer_name, }, ) @@ -551,16 +627,26 @@ def test_send_assignment_automatically_expired_email( @mock.patch('enterprise_access.apps.content_assignments.tasks.BrazeApiClient') @mock.patch('enterprise_access.apps.subsidy_access_policy.models.SubsidyAccessPolicy.subsidy_client') @mock.patch('enterprise_access.apps.content_metadata.api.EnterpriseCatalogApiClient') + @ddt.data( + {'is_assigned_course_run': False}, + {'is_assigned_course_run': True}, + ) + @ddt.unpack def test_get_action_required_by_subsidy_expires_soonest( + self, + mock_catalog_client, + mock_subsidy_client, # pylint: disable=unused-argument - self, mock_catalog_client, mock_subsidy_client, mock_braze_client_class, mock_lms_client_class + mock_braze_client_class, mock_lms_client_class, + is_assigned_course_run, ): """ Tests that the subsidy_expiration time is returned as the earliest action required by time. """ + assignment = self.assignment_course_run if is_assigned_course_run else self.assignment_course # Set the metadata enroll_by_date to tomorrow mock_metadata = { - 'key': self.assignment.content_key, + 'key': assignment.content_key, 'normalized_metadata': { 'start_date': '2020-01-01 12:00:00Z', 'end_date': '2022-01-01 12:00:00Z', @@ -581,9 +667,9 @@ def test_get_action_required_by_subsidy_expires_soonest( mock_subsidy_client.retrieve_subsidy.return_value = mock_subsidy # Add a successful notification action of now-ish - self.assignment.add_successful_notified_action() + assignment.add_successful_notified_action() - sender = BrazeCampaignSender(self.assignment) + sender = BrazeCampaignSender(assignment) action_required_by = sender.get_action_required_by_timestamp() expected_result = format_datetime_obj(yesterday, output_pattern=BRAZE_ACTION_REQUIRED_BY_TIMESTAMP_FORMAT) self.assertEqual(expected_result, action_required_by) @@ -592,22 +678,40 @@ def test_get_action_required_by_subsidy_expires_soonest( @mock.patch('enterprise_access.apps.content_assignments.tasks.BrazeApiClient') @mock.patch('enterprise_access.apps.subsidy_access_policy.models.SubsidyAccessPolicy.subsidy_client') @mock.patch('enterprise_access.apps.content_metadata.api.EnterpriseCatalogApiClient') + @ddt.data( + {'is_assigned_course_run': False}, + {'is_assigned_course_run': True}, + ) + @ddt.unpack def test_get_action_required_by_enrollment_deadline_soonest( + self, + mock_catalog_client, + mock_subsidy_client, # pylint: disable=unused-argument - self, mock_catalog_client, mock_subsidy_client, mock_braze_client_class, mock_lms_client_class + mock_braze_client_class, mock_lms_client_class, + is_assigned_course_run, ): """ Tests that the enroll_by_date is returned as the earliest action required by time. """ yesterday = now() - timedelta(days=1) + formatted_yesterday = yesterday.strftime('%Y-%m-%d %H:%M:%SZ') + assignment = self.assignment_course_run if is_assigned_course_run else self.assignment_course # Set the metadata enroll_by_date to yesterday mock_metadata = { - 'key': self.assignment.content_key, + 'key': assignment.content_key, 'normalized_metadata': { 'start_date': '2020-01-01 12:00:00Z', 'end_date': '2022-01-01 12:00:00Z', - 'enroll_by_date': yesterday.strftime('%Y-%m-%d %H:%M:%SZ'), + 'enroll_by_date': formatted_yesterday, }, + 'normalized_metadata_by_run': { + TEST_COURSE_RUN_KEY: { + 'start_date': '2020-01-01 12:00:00Z', + 'end_date': '2022-01-01 12:00:00Z', + 'enroll_by_date': formatted_yesterday, + }, + } } mock_catalog_client.return_value.catalog_content_metadata.return_value = { 'count': 1, @@ -622,9 +726,9 @@ def test_get_action_required_by_enrollment_deadline_soonest( mock_subsidy_client.retrieve_subsidy.return_value = mock_subsidy # Add a successful notification action of now-ish - self.assignment.add_successful_notified_action() + assignment.add_successful_notified_action() - sender = BrazeCampaignSender(self.assignment) + sender = BrazeCampaignSender(assignment) action_required_by = sender.get_action_required_by_timestamp() expected_result = format_datetime_obj(yesterday, output_pattern=BRAZE_ACTION_REQUIRED_BY_TIMESTAMP_FORMAT) self.assertEqual(expected_result, action_required_by) @@ -633,25 +737,39 @@ def test_get_action_required_by_enrollment_deadline_soonest( @mock.patch('enterprise_access.apps.content_assignments.tasks.BrazeApiClient') @mock.patch('enterprise_access.apps.content_metadata.api.EnterpriseCatalogApiClient') @mock.patch('enterprise_access.apps.subsidy_access_policy.models.SubsidyAccessPolicy.subsidy_client') - def test_get_action_required_by_auto_cancellation_soonest( # pylint: disable=unused-argument + @ddt.data( + {'is_assigned_course_run': False}, + {'is_assigned_course_run': True}, + ) + @ddt.unpack + def test_get_action_required_by_auto_cancellation_soonest( self, mock_subsidy_client, mock_catalog_client, - mock_braze_client_class, - mock_lms_client_class, + # pylint: disable=unused-argument + mock_braze_client_class, mock_lms_client_class, + is_assigned_course_run, ): """ Tests that the auto-cancellation date is returned as the earliest action required by time. """ the_future = now() + timedelta(days=120) + assignment = self.assignment_course_run if is_assigned_course_run else self.assignment_course # Set the metadata enroll_by_date to far in the future mock_metadata = { - 'key': self.assignment.content_key, + 'key': assignment.content_key, 'normalized_metadata': { 'start_date': '2020-01-01 12:00:00Z', 'end_date': '2022-01-01 12:00:00Z', 'enroll_by_date': the_future.strftime('%Y-%m-%d %H:%M:%SZ'), }, + 'normalized_metadata_by_run': { + TEST_COURSE_RUN_KEY: { + 'start_date': '2020-01-01 12:00:00Z', + 'end_date': '2022-01-01 12:00:00Z', + 'enroll_by_date': the_future.strftime('%Y-%m-%d %H:%M:%SZ'), + }, + }, } mock_catalog_client.return_value.catalog_content_metadata.return_value = { 'count': 1, @@ -666,13 +784,13 @@ def test_get_action_required_by_auto_cancellation_soonest( # pylint: disable=un mock_subsidy_client.retrieve_subsidy.return_value = mock_subsidy # Add a successful notification action of now-ish - self.assignment.add_successful_notified_action() + assignment.add_successful_notified_action() - sender = BrazeCampaignSender(self.assignment) + sender = BrazeCampaignSender(assignment) action_required_by = sender.get_action_required_by_timestamp() expected_result = format_datetime_obj( - get_automatic_expiration_date_and_reason(self.assignment)['date'], + get_automatic_expiration_date_and_reason(assignment)['date'], output_pattern=BRAZE_ACTION_REQUIRED_BY_TIMESTAMP_FORMAT ) self.assertEqual(expected_result, action_required_by)