diff --git a/enterprise/models.py b/enterprise/models.py index f5758a1a4..1971214ca 100644 --- a/enterprise/models.py +++ b/enterprise/models.py @@ -2097,7 +2097,7 @@ def is_audit_enrollment(self): @property def license(self): """ - Returns the license associated with this enterprise course enrollment if one exists. + Returns the license fulfillment associated with this enterprise course enrollment if one exists. """ try: associated_license = self.licensedenterprisecourseenrollment_enrollment_fulfillment # pylint: disable=no-member @@ -2105,6 +2105,24 @@ def license(self): associated_license = None return associated_license + @property + def learner_credit_fulfillment(self): + """ + Returns the Learner Credit fulfillment associated with this enterprise course enrollment if one exists. + """ + try: + associated_fulfillment = self.learnercreditenterprisecourseenrollment_enrollment_fulfillment # pylint: disable=no-member + except LearnerCreditEnterpriseCourseEnrollment.DoesNotExist: + associated_fulfillment = None + return associated_fulfillment + + @property + def fulfillment(self): + """ + Find and return the related EnterpriseFulfillmentSource subclass, or None. + """ + return self.license or self.learner_credit_fulfillment + @cached_property def course_enrollment(self): """ @@ -2196,6 +2214,44 @@ def get_enterprise_uuids_with_user_and_course(cls, user_id, course_run_id, is_cu ) return [] + def set_unenrolled(self, desired_unenrolled): + """ + Idempotently set this object's fields to appear (un)enrolled and (un)saved-for-later. + + Also, attempt to revoke any related fulfillment, which in turn is also idempotent. + + This method and the fulfillment's revoke() call each other!!! If you edit either method, make sure to preserve + base cases that terminate infinite recursion. + """ + changed = False + if desired_unenrolled: + if not self.unenrolled or not self.saved_for_later: + self.saved_for_later = True + self.unenrolled = True + self.unenrolled_at = localized_utcnow() + changed = True + else: + if self.unenrolled or self.saved_for_later: + self.saved_for_later = False + self.unenrolled = False + self.unenrolled_at = None + changed = True + if changed: + LOGGER.info( + f"Marking EnterpriseCourseEnrollment as unenrolled={desired_unenrolled} " + f"for LMS user {self.enterprise_customer_user.user_id} " + f"and course {self.course_id}" + ) + self.save() + # Find and revoke/reactivate any related fulfillment. + # By only updating the related object on updates to self, we prevent infinite recursion. + if fulfillment := self.fulfillment: + if desired_unenrolled: + fulfillment.revoke() + # Fulfillment reactivation on ECE reenrollment is unsupported. We'd need to collect a + # transaction UUID from the caller, but the caller at the time of writing is not aware of any + # transaction. + def __str__(self): """ Create string representation of the enrollment. @@ -2285,34 +2341,49 @@ def enterprise_customer_user(self): def revoke(self): """ - Marks this object as revoked and marks the associated EnterpriseCourseEnrollment - as "saved for later". This object and the associated EnterpriseCourseEnrollment are both saved. + Idempotently unenroll/revoke this fulfillment and associated EnterpriseCourseEnrollment. - Subclasses may override this function to additionally emit revocation events. + This method and EnterpriseCourseEnrollment.set_unenrolled() call each other!!! If you edit either method, make + sure to preserve base cases that terminate infinite recursion. - TODO: revoke entitlements as well? - """ - if self.enterprise_course_enrollment: - self.enterprise_course_enrollment.saved_for_later = True - self.enterprise_course_enrollment.unenrolled = True - self.enterprise_course_enrollment.unenrolled_at = localized_utcnow() - self.enterprise_course_enrollment.save() + Notes: + * This object and the associated EnterpriseCourseEnrollment may both be saved. + * Subclasses may override this function to additionally emit revocation events. - self.is_revoked = True - self.save() + Returns: + bool: True if self.is_revoked was changed. + """ + changed = False + if not self.is_revoked: + LOGGER.info(f"Marking fulfillment {str(self)} as revoked.") + changed = True + self.is_revoked = True + self.save() + # Find and unenroll any related EnterpriseCourseEnrollment. + # By only updating the related object on updates to self, we prevent infinite recursion. + if ece := self.enterprise_course_enrollment: + # TODO: revoke entitlements as well? + ece.set_unenrolled(True) + return changed def reactivate(self, **kwargs): """ Idempotently reactivates this enterprise fulfillment source. - """ - if self.enterprise_course_enrollment: - self.enterprise_course_enrollment.saved_for_later = False - self.enterprise_course_enrollment.unenrolled = False - self.enterprise_course_enrollment.unenrolled_at = None - self.enterprise_course_enrollment.save() - self.is_revoked = False - self.save() + Returns: + bool: True if self.is_revoked was changed. + """ + changed = False + if self.is_revoked: + LOGGER.info(f"Marking fulfillment {str(self)} as reactivated.") + changed = True + self.is_revoked = False + self.save() + # Find and REenroll any related EnterpriseCourseEnrollment. + # By only updating the related object on updates to self, we prevent infinite recursion. + if ece := self.enterprise_course_enrollment: + ece.set_unenrolled(False) + return changed def __str__(self): """ @@ -2337,8 +2408,9 @@ def revoke(self): """ Revoke this LearnerCreditEnterpriseCourseEnrollment, and emit a revoked event. """ - super().revoke() - send_learner_credit_course_enrollment_revoked_event(self) + if changed := super().revoke(): + send_learner_credit_course_enrollment_revoked_event(self) + return changed def reactivate(self, transaction_id=None, **kwargs): """ @@ -2354,7 +2426,7 @@ def reactivate(self, transaction_id=None, **kwargs): f"getting this enrollment for free." ) self.transaction_id = transaction_id - super().reactivate() + return super().reactivate() transaction_id = models.UUIDField( primary_key=False, diff --git a/enterprise/signals.py b/enterprise/signals.py index 8813b95a4..4e076c6ec 100644 --- a/enterprise/signals.py +++ b/enterprise/signals.py @@ -17,7 +17,6 @@ from enterprise.utils import ( NotConnectedToOpenEdX, get_default_catalog_content_filter, - localized_utcnow, unset_enterprise_learner_language, unset_language_of_all_enterprise_learners, ) @@ -364,14 +363,7 @@ def course_enrollment_changed_receiver(sender, **kwargs): # pylint: disable= enterprise_customer_user__user_id=enrollment.user.id, ).first() if enterprise_enrollment and enrollment.is_active: - logger.info( - f"Marking EnterpriseCourseEnrollment as enrolled (unenrolled_at=NULL) for user {enrollment.user} and " - f"course {enrollment.course.course_key}" - ) - enterprise_enrollment.unenrolled = False - enterprise_enrollment.unenrolled_at = None - enterprise_enrollment.saved_for_later = False - enterprise_enrollment.save() + enterprise_enrollment.set_unenrolled(False) # Note: If the CourseEnrollment is being flipped to is_active=False, then this handler is a no-op. # In that case, the `enterprise_unenrollment_receiver` signal handler below will run. @@ -386,13 +378,7 @@ def enterprise_unenrollment_receiver(sender, **kwargs): # pylint: disable=un enterprise_customer_user__user_id=enrollment.user.id, ).first() if enterprise_enrollment: - logger.info( - f"Marking EnterpriseCourseEnrollment as unenrolled for user {enrollment.user} and " - f"course {enrollment.course.course_key}" - ) - enterprise_enrollment.unenrolled = True - enterprise_enrollment.unenrolled_at = localized_utcnow() - enterprise_enrollment.save() + enterprise_enrollment.set_unenrolled(True) def create_enterprise_enrollment_receiver(sender, instance, **kwargs): # pylint: disable=unused-argument diff --git a/tests/test_enterprise/api/test_views.py b/tests/test_enterprise/api/test_views.py index 6e8efe7e4..27c9493d8 100644 --- a/tests/test_enterprise/api/test_views.py +++ b/tests/test_enterprise/api/test_views.py @@ -4152,6 +4152,7 @@ def test_requested_recently_unenrolled_subsidy_fulfillment(self): # user. Because the requesting user is an operator user, they should be able to see this enrollment. second_lc_enrollment = factories.LearnerCreditEnterpriseCourseEnrollmentFactory( enterprise_course_enrollment=second_enterprise_course_enrollment, + is_revoked=True, ) self.enterprise_course_enrollment.unenrolled = True @@ -4209,6 +4210,7 @@ def test_recently_unenrolled_fulfillment_endpoint_can_filter_for_modified_after( ) old_learner_credit_enrollment = factories.LearnerCreditEnterpriseCourseEnrollmentFactory( enterprise_course_enrollment=old_enterprise_course_enrollment, + is_revoked=True, ) response = self.client.get( reverse('enterprise-subsidy-fulfillment-unenrolled') + self.unenrolled_after_filter diff --git a/tests/test_enterprise/test_signals.py b/tests/test_enterprise/test_signals.py index 5e5b5a949..b335a7431 100644 --- a/tests/test_enterprise/test_signals.py +++ b/tests/test_enterprise/test_signals.py @@ -41,6 +41,7 @@ EnterpriseCustomerFactory, EnterpriseCustomerUserFactory, EnterpriseGroupMembershipFactory, + LearnerCreditEnterpriseCourseEnrollmentFactory, PendingEnrollmentFactory, PendingEnterpriseCustomerAdminUserFactory, PendingEnterpriseCustomerUserFactory, @@ -830,6 +831,7 @@ def test_delete_enterprise_learner_role_assignment_no_user_associated(self): @mark.django_db +@ddt.ddt class TestCourseEnrollmentSignals(TestCase): """ Tests signals associated with CourseEnrollments (that are found in edx-platform). @@ -897,17 +899,27 @@ def test_create_ece_receiver_does_not_call_task_if_ecu_not_exists(self, mock_tas create_enterprise_enrollment_receiver(sender, instance, **kwargs) mock_task.assert_not_called() - def test_course_enrollment_changed_receiver(self): + @ddt.data(True, False) + def test_course_enrollment_changed_receiver(self, create_related_lc_fulfillment): """ Test receiver that is supposed to handle course enrollments being reactivated (re-enrolled). """ # Create an unenrolled EnterpriseCourseEnrollment. enterprise_enrollment = EnterpriseCourseEnrollmentFactory( enterprise_customer_user=self.enterprise_customer_user, + saved_for_later=True, unenrolled=True, unenrolled_at=datetime.now() - timedelta(days=1), ) + # Optionally create a revoked LearnerCreditEnterpriseCourseEnrollment. + lc_fulfillment = None + if create_related_lc_fulfillment: + lc_fulfillment = LearnerCreditEnterpriseCourseEnrollmentFactory( + enterprise_course_enrollment=enterprise_enrollment, + is_revoked=True, + ) + # Simulate a previously inactive course enrollment being re-activated. mock_enrollment_data = mock.Mock() mock_enrollment_data.course.course_key = enterprise_enrollment.course_id @@ -921,20 +933,35 @@ def test_course_enrollment_changed_receiver(self): # Make sure the previously inactive enterprise enrollment has now been re-activated in response to the system # enrollment being re-activated. enterprise_enrollment.refresh_from_db() + assert enterprise_enrollment.saved_for_later is False assert enterprise_enrollment.unenrolled is False assert enterprise_enrollment.unenrolled_at is None - def test_enterprise_unenrollment_receiver(self): + # Make sure the related LC fulfillment is SILL REVOKED. Fulfillment re-activation is unsupported. + if create_related_lc_fulfillment: + lc_fulfillment.refresh_from_db() + assert lc_fulfillment.is_revoked is True + + @ddt.data(True, False) + def test_enterprise_unenrollment_receiver(self, create_related_lc_fulfillment): """ Test receiver that is supposed to handle course enrollments being deactivated (unenrolled). """ # Create an enrolled EnterpriseCourseEnrollment. enterprise_enrollment = EnterpriseCourseEnrollmentFactory( enterprise_customer_user=self.enterprise_customer_user, + saved_for_later=False, unenrolled=None, unenrolled_at=None, ) + # Optionally create an enrolled LearnerCreditEnterpriseCourseEnrollment. + lc_fulfillment = None + if create_related_lc_fulfillment: + lc_fulfillment = LearnerCreditEnterpriseCourseEnrollmentFactory( + enterprise_course_enrollment=enterprise_enrollment, + ) + # Simulate a previously active course enrollment being deactivated. mock_enrollment_data = mock.Mock() mock_enrollment_data.course.course_key = enterprise_enrollment.course_id @@ -948,9 +975,15 @@ def test_enterprise_unenrollment_receiver(self): # Make sure the previously active enterprise enrollment has now been deactivated in response to the system # enrollment being deactivated. enterprise_enrollment.refresh_from_db() + assert enterprise_enrollment.saved_for_later is True assert enterprise_enrollment.unenrolled is True assert enterprise_enrollment.unenrolled_at is not None + # Make sure the related LC fulfillment is also revoked, and a signal sent. + if create_related_lc_fulfillment: + lc_fulfillment.refresh_from_db() + assert lc_fulfillment.is_revoked is True + @mark.django_db class TestEnterpriseCatalogSignals(unittest.TestCase):