Skip to content

Commit

Permalink
Merge pull request #2152 from openedx/pwnage101/ENT-8991-2
Browse files Browse the repository at this point in the history
feat: atomically re-activate ECE when re-activating SCE
  • Loading branch information
pwnage101 authored Jun 21, 2024
2 parents 30c0dbf + f6b1c5b commit 18d68f3
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 5 deletions.
2 changes: 2 additions & 0 deletions enterprise/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -2289,6 +2289,7 @@ def revoke(self):
"""
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()

Expand All @@ -2301,6 +2302,7 @@ def reactivate(self, **kwargs):
"""
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()

Expand Down
32 changes: 31 additions & 1 deletion enterprise/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,11 @@

try:
from common.djangoapps.student.models import CourseEnrollment
from openedx_events.learning.signals import COURSE_UNENROLLMENT_COMPLETED
from openedx_events.learning.signals import COURSE_ENROLLMENT_CHANGED, COURSE_UNENROLLMENT_COMPLETED

except ImportError:
CourseEnrollment = None
COURSE_ENROLLMENT_CHANGED = None
COURSE_UNENROLLMENT_COMPLETED = None

logger = getLogger(__name__)
Expand Down Expand Up @@ -350,6 +351,31 @@ def delete_enterprise_catalog_data(sender, instance, **kwargs): # pylint: di
break


def course_enrollment_changed_receiver(sender, **kwargs): # pylint: disable=unused-argument
"""
Handle when a course enrollment is (de/re)activated.
Importantly, if a student.CourseEnrollment is being reactivated, take this opportunity to atomically reactivate
the corresponding EnterpriseCourseEnrollment.
"""
enrollment = kwargs.get('enrollment')
enterprise_enrollment = models.EnterpriseCourseEnrollment.objects.filter(
course_id=enrollment.course.course_key,
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()
# 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.


def enterprise_unenrollment_receiver(sender, **kwargs): # pylint: disable=unused-argument
"""
Mark the EnterpriseCourseEnrollment object as unenrolled when a user unenrolls from a course.
Expand All @@ -364,6 +390,7 @@ def enterprise_unenrollment_receiver(sender, **kwargs): # pylint: disable=un
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()

Expand Down Expand Up @@ -434,3 +461,6 @@ def generate_default_orchestration_record_display_name(sender, instance, **kwarg

if COURSE_UNENROLLMENT_COMPLETED is not None:
COURSE_UNENROLLMENT_COMPLETED.connect(enterprise_unenrollment_receiver)

if COURSE_ENROLLMENT_CHANGED is not None:
COURSE_ENROLLMENT_CHANGED.connect(course_enrollment_changed_receiver)
68 changes: 64 additions & 4 deletions tests/test_enterprise/test_signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import unittest
from collections import OrderedDict
from datetime import datetime
from datetime import datetime, timedelta
from unittest import mock

import ddt
Expand All @@ -25,12 +25,18 @@
SystemWideEnterpriseRole,
SystemWideEnterpriseUserRoleAssignment,
)
from enterprise.signals import create_enterprise_enrollment_receiver, handle_user_post_save
from enterprise.signals import (
course_enrollment_changed_receiver,
create_enterprise_enrollment_receiver,
enterprise_unenrollment_receiver,
handle_user_post_save,
)
from integrated_channels.integrated_channel.models import OrphanedContentTransmissions
from test_utils import EmptyCacheMixin
from test_utils.factories import (
ContentMetadataItemTransmissionFactory,
EnterpriseCatalogQueryFactory,
EnterpriseCourseEnrollmentFactory,
EnterpriseCustomerCatalogFactory,
EnterpriseCustomerFactory,
EnterpriseCustomerUserFactory,
Expand Down Expand Up @@ -844,7 +850,7 @@ def setUp(self):
super().setUp()

@mock.patch('enterprise.tasks.create_enterprise_enrollment.apply_async')
def test_receiver_calls_task_if_ecu_exists(self, mock_task):
def test_create_ece_receiver_calls_task_if_ecu_exists(self, mock_task):
"""
Receiver should call a task
if user tied to the CourseEnrollment that is handed into the function
Expand All @@ -869,7 +875,7 @@ def test_receiver_calls_task_if_ecu_exists(self, mock_task):
mock_task.assert_called_once_with((str(instance.course_id), self.enterprise_customer_user.id), countdown=42)

@mock.patch('enterprise.tasks.create_enterprise_enrollment.apply_async')
def test_receiver_does_not_call_task_if_ecu_not_exists(self, mock_task):
def test_create_ece_receiver_does_not_call_task_if_ecu_not_exists(self, mock_task):
"""
Receiver should NOT call a task
if user tied to the CourseEnrollment that is handed into the function
Expand All @@ -891,6 +897,60 @@ def test_receiver_does_not_call_task_if_ecu_not_exists(self, mock_task):
create_enterprise_enrollment_receiver(sender, instance, **kwargs)
mock_task.assert_not_called()

def test_course_enrollment_changed_receiver(self):
"""
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,
unenrolled=True,
unenrolled_at=datetime.now() - timedelta(days=1),
)

# Simulate a previously inactive course enrollment being re-activated.
mock_enrollment_data = mock.Mock()
mock_enrollment_data.course.course_key = enterprise_enrollment.course_id
mock_enrollment_data.user.id = self.enterprise_customer_user.user.id
mock_enrollment_data.is_active = True
kwargs = {
'enrollment': mock_enrollment_data,
}
course_enrollment_changed_receiver(mock.Mock(), **kwargs)

# 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.unenrolled is False
assert enterprise_enrollment.unenrolled_at is None

def test_enterprise_unenrollment_receiver(self):
"""
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,
unenrolled=None,
unenrolled_at=None,
)

# Simulate a previously active course enrollment being deactivated.
mock_enrollment_data = mock.Mock()
mock_enrollment_data.course.course_key = enterprise_enrollment.course_id
mock_enrollment_data.user.id = self.enterprise_customer_user.user.id
mock_enrollment_data.is_active = False
kwargs = {
'enrollment': mock_enrollment_data,
}
enterprise_unenrollment_receiver(mock.Mock(), **kwargs)

# 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.unenrolled is True
assert enterprise_enrollment.unenrolled_at is not None


@mark.django_db
class TestEnterpriseCatalogSignals(unittest.TestCase):
Expand Down

0 comments on commit 18d68f3

Please sign in to comment.