Skip to content

Commit

Permalink
fix: respect allow_enrollment_in_invite_only_courses
Browse files Browse the repository at this point in the history
Fixes `enroll_learners_in_courses` endpoint (and
`enroll_subsidy_users_in_courses` utility function) to respect
enterprise customer's `allow_enrollment_in_invite_only_courses` flag.

(cherry picked from commit 75ea83c)
  • Loading branch information
0x29a committed Mar 25, 2024
1 parent d1c1995 commit 81174f8
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 1 deletion.
4 changes: 4 additions & 0 deletions enterprise/api/v1/views/enterprise_customer.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
from enterprise.utils import (
enroll_subsidy_users_in_courses,
get_best_mode_from_course_key,
get_course_details_from_course_keys,
track_enrollment,
validate_email_to_link,
)
Expand Down Expand Up @@ -245,6 +246,8 @@ def enroll_learners_in_courses(self, request, pk):
for course_run in course_runs_modes:
course_runs_modes[course_run] = get_best_mode_from_course_key(course_run)

course_details = get_course_details_from_course_keys(course_runs_modes.keys())

emails = set()

for info in enrollments_info:
Expand All @@ -258,6 +261,7 @@ def enroll_learners_in_courses(self, request, pk):
else:
emails.add(info['email'])
info['course_mode'] = course_runs_modes[info['course_run_key']]
info['invitation_only'] = course_details[info['course_run_key']].invitation_only

for email in emails:
try:
Expand Down
20 changes: 20 additions & 0 deletions enterprise/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@
CourseUserGroup = None
CourseEnrollmentError = None

try:
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
except ImportError:
CourseOverview = None

try:
from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.student.models import CourseEnrollmentAllowed
Expand Down Expand Up @@ -2031,6 +2036,7 @@ def enroll_subsidy_users_in_courses(enterprise_customer, subsidy_users_info, dis
user_id = subsidy_user_info.get('user_id')
user_email = subsidy_user_info['email'].strip().lower() if 'email' in subsidy_user_info else None
course_mode = subsidy_user_info.get('course_mode')
invitation_only = subsidy_user_info.get('invitation_only')
course_run_key = subsidy_user_info.get('course_run_key')
license_uuid = subsidy_user_info.get('license_uuid')
transaction_id = subsidy_user_info.get('transaction_id')
Expand All @@ -2057,6 +2063,12 @@ def enroll_subsidy_users_in_courses(enterprise_customer, subsidy_users_info, dis
enrollment_source = enterprise_enrollment_source_model().get_source(
enterprise_enrollment_source_model().CUSTOMER_ADMIN
)
if invitation_only and enterprise_customer.allow_enrollment_in_invite_only_courses:
CourseEnrollmentAllowed.objects.update_or_create(
course_id=course_run_key,
email=user.email,
)

succeeded, created, source_uuid = customer_admin_enroll_user_with_status(
enterprise_customer,
user,
Expand Down Expand Up @@ -2296,6 +2308,14 @@ def get_best_mode_from_course_key(course_key):
return CourseModes.AUDIT


def get_course_details_from_course_keys(course_keys):
"""
Helper to get a mapping of course keys to course details.
"""
course_overviews = CourseOverview.objects.filter(id__in=course_keys)
return {str(course_overview.id): course_overview for course_overview in course_overviews}


def parse_lms_api_datetime(datetime_string, datetime_format=LMS_API_DATETIME_FORMAT):
"""
Parse a received datetime into a timezone-aware, Python datetime object.
Expand Down
81 changes: 80 additions & 1 deletion tests/test_enterprise/api/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -4485,6 +4485,7 @@ def tearDown(self):
},
)
@ddt.unpack
@mock.patch('enterprise.api.v1.views.enterprise_customer.get_course_details_from_course_keys')
@mock.patch('enterprise.api.v1.views.enterprise_customer.get_best_mode_from_course_key')
@mock.patch('enterprise.api.v1.views.enterprise_customer.track_enrollment')
@mock.patch("enterprise.models.EnterpriseCustomer.notify_enrolled_learners")
Expand All @@ -4495,6 +4496,7 @@ def test_bulk_enrollment_in_bulk_courses_pending_licenses(
mock_notify_task,
mock_track_enroll,
mock_get_course_mode,
mock_get_course_details,
body,
expected_code,
expected_response,
Expand All @@ -4515,6 +4517,7 @@ def test_bulk_enrollment_in_bulk_courses_pending_licenses(
permission = Permission.objects.get(name='Can add Enterprise Customer')
self.user.user_permissions.add(permission)
mock_get_course_mode.return_value = VERIFIED_SUBSCRIPTION_COURSE_MODE
mock_get_course_details.return_value.__getitem__.return_value.invitation_only = False

self.assertEqual(len(PendingEnrollment.objects.all()), 0)

Expand Down Expand Up @@ -4547,6 +4550,7 @@ def test_bulk_enrollment_in_bulk_courses_pending_licenses(
# no notifications to be sent unless 'notify' specifically asked for in payload
mock_notify_task.assert_not_called()

@mock.patch('enterprise.api.v1.views.enterprise_customer.get_course_details_from_course_keys')
@mock.patch('enterprise.api.v1.views.enterprise_customer.get_best_mode_from_course_key')
@mock.patch('enterprise.api.v1.views.enterprise_customer.track_enrollment')
@mock.patch('enterprise.models.EnterpriseCustomer.notify_enrolled_learners')
Expand All @@ -4557,13 +4561,15 @@ def test_bulk_enrollment_in_bulk_courses_existing_users(
mock_notify_task,
mock_track_enroll,
mock_get_course_mode,
mock_get_course_details,
):
"""
Tests the bulk enrollment endpoint at enroll_learners_in_courses.
This tests the case where existing users are supplied, so the enrollments are fulfilled rather than pending.
"""
mock_update_or_create_enrollment.return_value = True
mock_get_course_details.return_value.__getitem__.return_value.invitation_only = False

user_one = factories.UserFactory(is_active=True)
user_two = factories.UserFactory(is_active=True)
Expand Down Expand Up @@ -4634,6 +4640,7 @@ def test_bulk_enrollment_in_bulk_courses_existing_users(

assert mock_update_or_create_enrollment.call_count == 2

@mock.patch('enterprise.api.v1.views.enterprise_customer.get_course_details_from_course_keys')
@mock.patch('enterprise.api.v1.views.enterprise_customer.get_best_mode_from_course_key')
@mock.patch('enterprise.api.v1.views.enterprise_customer.track_enrollment')
@mock.patch('enterprise.models.EnterpriseCustomer.notify_enrolled_learners')
Expand All @@ -4642,12 +4649,15 @@ def test_bulk_enrollment_in_bulk_courses_nonexisting_user_id(
mock_notify_task,
mock_track_enroll,
mock_get_course_mode,
mock_get_course_details,
):
"""
Tests the bulk enrollment endpoint at enroll_learners_in_courses.
This tests the case where a non-existent user_id is supplied, so an error should occur.
"""
mock_get_course_details.return_value.__getitem__.return_value.invitation_only = False

user = factories.UserFactory(is_active=True)

factories.EnterpriseCustomerFactory(
Expand Down Expand Up @@ -4706,6 +4716,7 @@ def test_bulk_enrollment_in_bulk_courses_nonexisting_user_id(
},
)
@ddt.unpack
@mock.patch("enterprise.api.v1.views.enterprise_customer.get_course_details_from_course_keys")
@mock.patch("enterprise.api.v1.views.enterprise_subsidy_fulfillment.enrollment_api")
@mock.patch(
'enterprise.api.v1.views.enterprise_customer.get_best_mode_from_course_key'
Expand All @@ -4716,6 +4727,7 @@ def test_bulk_enrollment_enroll_after_cancel(
mock_platform_enrollment,
mock_get_course_mode,
mock_update_or_create_enrollment,
mock_get_course_details,
old_transaction_id,
new_transaction_id,
):
Expand All @@ -4725,6 +4737,7 @@ def test_bulk_enrollment_enroll_after_cancel(
"""
mock_platform_enrollment.return_value = True
mock_get_course_mode.return_value = VERIFIED_SUBSCRIPTION_COURSE_MODE
mock_get_course_details.return_value.__getitem__.return_value.invitation_only = False
# Needed for the cancel endpoint:
mock_update_or_create_enrollment.update_enrollment.return_value = mock.Mock()

Expand Down Expand Up @@ -4827,12 +4840,14 @@ def test_bulk_enrollment_enroll_after_cancel(
},
)
@ddt.unpack
@mock.patch('enterprise.api.v1.views.enterprise_customer.get_course_details_from_course_keys')
@mock.patch('enterprise.api.v1.views.enterprise_customer.get_best_mode_from_course_key')
@mock.patch('enterprise.utils.lms_update_or_create_enrollment')
def test_bulk_enrollment_includes_fulfillment_source_uuid(
self,
mock_get_course_mode,
mock_update_or_create_enrollment,
mock_get_course_details,
body,
fulfillment_source,
):
Expand All @@ -4841,6 +4856,7 @@ def test_bulk_enrollment_includes_fulfillment_source_uuid(
generated subsidized enrollment uuid value as part of the response payload.
"""
mock_update_or_create_enrollment.return_value = True
mock_get_course_details.return_value.__getitem__.return_value.invitation_only = False

user, _, enterprise_customer = self._create_user_and_enterprise_customer(
body.get('enrollments_info')[0].get('email'), 'test_password'
Expand Down Expand Up @@ -4937,6 +4953,7 @@ def test_bulk_enrollment_includes_fulfillment_source_uuid(
},
)
@ddt.unpack
@mock.patch('enterprise.api.v1.views.enterprise_customer.get_course_details_from_course_keys')
@mock.patch('enterprise.api.v1.views.enterprise_customer.get_best_mode_from_course_key')
@mock.patch('enterprise.api.v1.views.enterprise_customer.track_enrollment')
@mock.patch("enterprise.models.EnterpriseCustomer.notify_enrolled_learners")
Expand All @@ -4945,6 +4962,7 @@ def test_bulk_enrollment_with_notification(
mock_notify_task,
mock_track_enroll,
mock_get_course_mode,
mock_get_course_details,
body,
expected_code,
expected_response,
Expand All @@ -4964,6 +4982,7 @@ def test_bulk_enrollment_with_notification(
permission = Permission.objects.get(name='Can add Enterprise Customer')
self.user.user_permissions.add(permission)
mock_get_course_mode.return_value = VERIFIED_SUBSCRIPTION_COURSE_MODE
mock_get_course_details.return_value.__getitem__.return_value.invitation_only = False

self.assertEqual(len(PendingEnrollment.objects.all()), 0)

Expand Down Expand Up @@ -5011,12 +5030,72 @@ def _make_call(course_run, enrolled_learners):

mock_notify_task.assert_has_calls(mock_calls, any_order=True)

@mock.patch('enterprise.utils.CourseEnrollmentAllowed')
@mock.patch('enterprise.api.v1.views.enterprise_customer.get_course_details_from_course_keys')
@mock.patch('enterprise.api.v1.views.enterprise_customer.get_best_mode_from_course_key')
@mock.patch('enterprise.utils.lms_update_or_create_enrollment')
def test_bulk_enrollment_invitation_only(
self,
mock_update_or_create_enrollment,
mock_get_course_mode,
mock_get_course_details,
mock_cea,
):
"""
Tests that bulk enrollment endpoint creates CourseEnrollmentAllowed object when enterprise customer allows
enrollment in invitation only courses and the course is invitation only.
"""
mock_update_or_create_enrollment.return_value = True
mock_get_course_details.return_value.__getitem__.return_value.invitation_only = True
mock_get_course_mode.return_value = VERIFIED_SUBSCRIPTION_COURSE_MODE

user, _, enterprise_customer = self._create_user_and_enterprise_customer("abc@test.com", "test_password")
course_id = 'course-v1:edX+DemoX+Demo_Course'
body = {
'enrollments_info': [
{
'user_id': user.id,
'course_run_key': course_id,
'license_uuid': '5a88bdcade7c4ecb838f8111b68e18ac'
},
]
}

def enroll():
self.client.post(
settings.TEST_SERVER + reverse(
'enterprise-customer-enroll-learners-in-courses', (enterprise_customer.uuid,)
),
data=json.dumps(body),
content_type='application/json',
)

enroll()
mock_cea.objects.update_or_create.assert_not_called()

enterprise_customer.allow_enrollment_in_invite_only_courses = True
enterprise_customer.save()

enroll()
mock_cea.objects.update_or_create.assert_called_with(
course_id=course_id,
email=user.email
)

@mock.patch('enterprise.api.v1.views.enterprise_customer.get_course_details_from_course_keys')
@mock.patch('enterprise.api.v1.views.enterprise_customer.enroll_subsidy_users_in_courses')
@mock.patch('enterprise.api.v1.views.enterprise_customer.get_best_mode_from_course_key')
def test_enroll_learners_in_courses_partial_failure(self, mock_get_course_mode, mock_enroll_user):
def test_enroll_learners_in_courses_partial_failure(
self,
mock_get_course_mode,
mock_enroll_user,
mock_get_course_details,
):
"""
Tests that bulk users bulk enrollment endpoint properly handles partial failures.
"""
mock_get_course_details.return_value.__getitem__.return_value.invitation_only = True

ent_customer = factories.EnterpriseCustomerFactory(
uuid=FAKE_UUIDS[0],
name="test_enterprise"
Expand Down

0 comments on commit 81174f8

Please sign in to comment.