From 81174f8fd52a0bc1bd2de8053a1a64e185fbc6be Mon Sep 17 00:00:00 2001 From: 0x29a Date: Fri, 1 Mar 2024 12:06:02 +0100 Subject: [PATCH] fix: respect allow_enrollment_in_invite_only_courses 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 75ea83cc7fe42ce9d14c12d1b39d4d74698a3527) --- .../api/v1/views/enterprise_customer.py | 4 + enterprise/utils.py | 20 +++++ tests/test_enterprise/api/test_views.py | 81 ++++++++++++++++++- 3 files changed, 104 insertions(+), 1 deletion(-) diff --git a/enterprise/api/v1/views/enterprise_customer.py b/enterprise/api/v1/views/enterprise_customer.py index fcd61cc80f..6a972c5cfd 100644 --- a/enterprise/api/v1/views/enterprise_customer.py +++ b/enterprise/api/v1/views/enterprise_customer.py @@ -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, ) @@ -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: @@ -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: diff --git a/enterprise/utils.py b/enterprise/utils.py index 70f7ad6381..f7b9720604 100644 --- a/enterprise/utils.py +++ b/enterprise/utils.py @@ -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 @@ -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') @@ -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, @@ -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. diff --git a/tests/test_enterprise/api/test_views.py b/tests/test_enterprise/api/test_views.py index a72ef7ccc0..6d9fbd2400 100644 --- a/tests/test_enterprise/api/test_views.py +++ b/tests/test_enterprise/api/test_views.py @@ -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") @@ -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, @@ -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) @@ -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') @@ -4557,6 +4561,7 @@ 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. @@ -4564,6 +4569,7 @@ def test_bulk_enrollment_in_bulk_courses_existing_users( 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) @@ -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') @@ -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( @@ -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' @@ -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, ): @@ -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() @@ -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, ): @@ -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' @@ -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") @@ -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, @@ -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) @@ -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"