From 8d627fe64f747b737b8301538d66b1ebf91a11d3 Mon Sep 17 00:00:00 2001 From: Arunmozhi Date: Thu, 4 May 2023 16:46:26 +0530 Subject: [PATCH 1/9] chore: version bump --- enterprise/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/enterprise/__init__.py b/enterprise/__init__.py index ecc16a8e63..6523679720 100644 --- a/enterprise/__init__.py +++ b/enterprise/__init__.py @@ -2,6 +2,6 @@ Your project description goes here. """ -__version__ = "3.65.3" +__version__ = "3.66.0" default_app_config = "enterprise.apps.EnterpriseConfig" From 25a899996923ea8205f04d7f76996a735ffa73d3 Mon Sep 17 00:00:00 2001 From: Arunmozhi Date: Sat, 6 May 2023 13:59:37 +0530 Subject: [PATCH 2/9] feat: create CourseEnrollmentAllowed entries for pending enrollments When creating pending enrollments for non-existant users, we also check to see if the course is "invite_only". If the course is invite only, then we create corresponding CourseEnrollmentAllowed objects. This fixes the issue when the enterprise creates pending enrollment, but the user cannot enroll to the course as platform rejects the enrollment request due to missing CEA for the user. --- enterprise/models.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/enterprise/models.py b/enterprise/models.py index 4908234a89..66d7a195b2 100644 --- a/enterprise/models.py +++ b/enterprise/models.py @@ -78,9 +78,10 @@ ) try: - from common.djangoapps.student.models import CourseEnrollment + from common.djangoapps.student.models import CourseEnrollment, CourseEnrollmentAllowed except ImportError: CourseEnrollment = None + CourseEnrollmentAllowed = None try: from common.djangoapps.entitlements.models import CourseEntitlement @@ -673,7 +674,21 @@ def enroll_user_pending_registration_with_status(self, email, course_mode, *cour license_uuid = None new_enrollments = {} + enrollment_api_client = EnrollmentApiClient() + for course_id in course_ids: + # Check if the course is "Invite Only" and add CEA if it is. + course_details = enrollment_api_client.get_course_details(course_id) + + if course_details["invite_only"]: + if not CourseEnrollmentAllowed: + raise NotConnectedToOpenEdX() + + CourseEnrollmentAllowed.objects.update_or_create( + email=email, + course_id=course_id + ) + __, created = PendingEnrollment.objects.update_or_create( user=pending_ecu, course_id=course_id, From f36343dd3783c6ba913d1783e098534d5f1f2431 Mon Sep 17 00:00:00 2001 From: Arunmozhi Date: Sat, 6 May 2023 14:05:46 +0530 Subject: [PATCH 3/9] fix: update tests broken due to CourseEnrollmentAllowed change in models --- tests/test_enterprise/api/test_views.py | 80 ++++++++++++++++--------- tests/test_enterprise/test_utils.py | 6 +- 2 files changed, 57 insertions(+), 29 deletions(-) diff --git a/tests/test_enterprise/api/test_views.py b/tests/test_enterprise/api/test_views.py index f8cd6dcfce..8dcb364664 100644 --- a/tests/test_enterprise/api/test_views.py +++ b/tests/test_enterprise/api/test_views.py @@ -69,6 +69,7 @@ ) from test_utils.factories import FAKER, EnterpriseCustomerUserFactory, PendingEnterpriseCustomerUserFactory, UserFactory from test_utils.fake_enterprise_api import get_default_branding_object +from test_utils.fake_enrollment_api import get_course_details fake = Faker() @@ -2532,7 +2533,7 @@ def test_enterprise_customer_contains_content_items_no_query_params(self): @ddt.ddt @mark.django_db -class TestEnterpriesCustomerCourseEnrollments(BaseTestEnterpriseAPIViews): +class TestEnterpriseCustomerCourseEnrollments(BaseTestEnterpriseAPIViews): """ Test the Enteprise Customer course enrollments detail route """ @@ -2856,6 +2857,7 @@ def test_enterprise_customer_course_enrollments_detail_success( True, enable_autocohorting=True ) + mock_enrollment_client.return_value.get_course_details = get_course_details # Make the call! response = self.client.post( @@ -3053,7 +3055,8 @@ def test_enterprise_customer_course_enrollments_detail_multiple( get_course_enrollment=mock.Mock( side_effect=[None, {'is_active': True, 'mode': VERIFIED_SUBSCRIPTION_COURSE_MODE}] ), - enroll_user_in_course=mock.Mock() + enroll_user_in_course=mock.Mock(), + get_course_details=get_course_details ) # Set up catalog_contains_course response. @@ -3973,6 +3976,7 @@ def _create_user_and_enterprise_customer(self, email, password): }, 'expected_num_pending_licenses': 1, 'expected_events': [mock.call(PATHWAY_CUSTOMER_ADMIN_ENROLLMENT, 1, 'course-v1:edX+DemoX+Demo_Course')], + 'expected_cea': 0, }, # Validation failure cases { @@ -3981,6 +3985,7 @@ def _create_user_and_enterprise_customer(self, email, password): 'expected_response': {'non_field_errors': ['Must include the `enrollment_info` parameter in request.']}, 'expected_num_pending_licenses': 0, 'expected_events': None, + 'expected_cea': 0, }, { 'body': { @@ -3992,6 +3997,7 @@ def _create_user_and_enterprise_customer(self, email, password): }, 'expected_num_pending_licenses': 0, 'expected_events': None, + 'expected_cea': 0, }, { 'body': { @@ -4009,6 +4015,7 @@ def _create_user_and_enterprise_customer(self, email, password): }, 'expected_num_pending_licenses': 0, 'expected_events': None, + 'expected_cea': 0, }, { 'body': { @@ -4034,6 +4041,7 @@ def _create_user_and_enterprise_customer(self, email, password): }, 'expected_num_pending_licenses': 0, 'expected_events': None, + 'expected_cea': 0, }, { 'body': { @@ -4052,6 +4060,7 @@ def _create_user_and_enterprise_customer(self, email, password): }, 'expected_num_pending_licenses': 0, 'expected_events': None, + 'expected_cea': 0, }, { 'body': { @@ -4067,6 +4076,7 @@ def _create_user_and_enterprise_customer(self, email, password): }, 'expected_num_pending_licenses': 0, 'expected_events': None, + 'expected_cea': 0, }, # Single learner, single course success { @@ -4090,6 +4100,7 @@ def _create_user_and_enterprise_customer(self, email, password): }, 'expected_num_pending_licenses': 1, 'expected_events': [mock.call(PATHWAY_CUSTOMER_ADMIN_ENROLLMENT, 1, 'course-v1:edX+DemoX+Demo_Course')], + 'expected_cea': 0, }, # Multi-learner, single course success { @@ -4130,6 +4141,7 @@ def _create_user_and_enterprise_customer(self, email, password): 'expected_events': [ mock.call(PATHWAY_CUSTOMER_ADMIN_ENROLLMENT, 1, 'course-v1:edX+DemoX+Demo_Course'), ], + 'expected_cea': 0, }, # Multi-learner, multi-course success { @@ -4147,12 +4159,12 @@ def _create_user_and_enterprise_customer(self, email, password): }, { 'email': 'abc@test.com', - 'course_run_key': 'course-v2:edX+DemoX+Second_Demo_Course', + 'course_run_key': 'course-v1:EnterpriseX+Training+2017', 'license_uuid': '5a88bdcade7c4ecb838f8111b68e18ac' }, { 'email': 'xyz@test.com', - 'course_run_key': 'course-v2:edX+DemoX+Second_Demo_Course', + 'course_run_key': 'course-v1:EnterpriseX+Training+2017', 'license_uuid': '2c58acdade7c4ede838f7111b42e18ac' }, ] @@ -4175,13 +4187,13 @@ def _create_user_and_enterprise_customer(self, email, password): }, { 'email': 'abc@test.com', - 'course_run_key': 'course-v2:edX+DemoX+Second_Demo_Course', + 'course_run_key': 'course-v1:EnterpriseX+Training+2017', 'created': True, 'activation_link': None, }, { 'email': 'xyz@test.com', - 'course_run_key': 'course-v2:edX+DemoX+Second_Demo_Course', + 'course_run_key': 'course-v1:EnterpriseX+Training+2017', 'created': True, 'activation_link': None, } @@ -4191,8 +4203,9 @@ def _create_user_and_enterprise_customer(self, email, password): 'expected_num_pending_licenses': 4, 'expected_events': [ mock.call(PATHWAY_CUSTOMER_ADMIN_ENROLLMENT, 1, 'course-v1:edX+DemoX+Demo_Course'), - mock.call(PATHWAY_CUSTOMER_ADMIN_ENROLLMENT, 1, 'course-v2:edX+DemoX+Second_Demo_Course') + mock.call(PATHWAY_CUSTOMER_ADMIN_ENROLLMENT, 1, 'course-v1:EnterpriseX+Training+2017') ], + 'expected_cea': 2, }, { 'body': { @@ -4209,12 +4222,12 @@ def _create_user_and_enterprise_customer(self, email, password): }, { 'email': 'abc@test.com', - 'course_run_key': 'course-v2:edX+DemoX+Second_Demo_Course', + 'course_run_key': 'course-v1:EnterpriseX+Training+2017', 'license_uuid': '5a88bdcade7c4ecb838f8111b68e18ac' }, { 'email': 'xyz@test.com', - 'course_run_key': 'course-v2:edX+DemoX+Second_Demo_Course', + 'course_run_key': 'course-v1:EnterpriseX+Training+2017', 'license_uuid': '2c58acdade7c4ede838f7111b42e18ac' }, ] @@ -4237,13 +4250,13 @@ def _create_user_and_enterprise_customer(self, email, password): }, { 'email': 'abc@test.com', - 'course_run_key': 'course-v2:edX+DemoX+Second_Demo_Course', + 'course_run_key': 'course-v1:EnterpriseX+Training+2017', 'created': True, 'activation_link': None, }, { 'email': 'xyz@test.com', - 'course_run_key': 'course-v2:edX+DemoX+Second_Demo_Course', + 'course_run_key': 'course-v1:EnterpriseX+Training+2017', 'created': True, 'activation_link': None, } @@ -4253,16 +4266,19 @@ def _create_user_and_enterprise_customer(self, email, password): 'expected_num_pending_licenses': 4, 'expected_events': [ mock.call(PATHWAY_CUSTOMER_ADMIN_ENROLLMENT, 1, 'course-v1:edX+DemoX+Demo_Course'), - mock.call(PATHWAY_CUSTOMER_ADMIN_ENROLLMENT, 1, 'course-v2:edX+DemoX+Second_Demo_Course') + mock.call(PATHWAY_CUSTOMER_ADMIN_ENROLLMENT, 1, 'course-v1:EnterpriseX+Training+2017') ], + 'expected_cea': 2, }, ) @ddt.unpack @mock.patch('enterprise.api.v1.views.get_best_mode_from_course_key') @mock.patch('enterprise.api.v1.views.track_enrollment') @mock.patch("enterprise.models.EnterpriseCustomer.notify_enrolled_learners") + @mock.patch("enterprise.models.CourseEnrollmentAllowed") def test_bulk_enrollment_in_bulk_courses_pending_licenses( self, + mock_cea, mock_notify_task, mock_track_enroll, mock_get_course_mode, @@ -4271,6 +4287,7 @@ def test_bulk_enrollment_in_bulk_courses_pending_licenses( expected_response, expected_num_pending_licenses, expected_events, + expected_cea, ): """ Tests the bulk enrollment endpoint at enroll_learners_in_courses. @@ -4287,11 +4304,17 @@ def test_bulk_enrollment_in_bulk_courses_pending_licenses( mock_get_course_mode.return_value = VERIFIED_SUBSCRIPTION_COURSE_MODE self.assertEqual(len(PendingEnrollment.objects.all()), 0) - response = self.client.post( - settings.TEST_SERVER + ENTERPRISE_CUSTOMER_BULK_ENROLL_LEARNERS_IN_COURSES_ENDPOINT, - data=json.dumps(body), - content_type='application/json', - ) + + with mock.patch("enterprise.models.EnrollmentApiClient.get_course_details", wraps=get_course_details) as mock_client: + # mock_client = mock.Mock( + # get_course_details=get_course_details + # ) + response = self.client.post( + settings.TEST_SERVER + ENTERPRISE_CUSTOMER_BULK_ENROLL_LEARNERS_IN_COURSES_ENDPOINT, + data=json.dumps(body), + content_type='application/json', + ) + self.assertEqual(response.status_code, expected_code) if expected_response: response_json = response.json() @@ -4306,6 +4329,8 @@ def test_bulk_enrollment_in_bulk_courses_pending_licenses( else: mock_track_enroll.assert_not_called() + self.assertEqual(mock_cea.objects.update_or_create.call_count, expected_cea) + # no notifications to be sent unless 'notify' specifically asked for in payload mock_notify_task.assert_not_called() @@ -4648,12 +4673,12 @@ def test_bulk_enrollment_includes_fulfillment_source_uuid( }, { 'email': 'abc@test.com', - 'course_run_key': 'course-v2:edX+DemoX+Second_Demo_Course', + 'course_run_key': 'course-v1:HarvardX+CoolScience+2016', 'license_uuid': '5a88bdcade7c4ecb838f8111b68e18ac' }, { 'email': 'xyz@test.com', - 'course_run_key': 'course-v2:edX+DemoX+Second_Demo_Course', + 'course_run_key': 'course-v1:HarvardX+CoolScience+2016', 'license_uuid': '2c58acdade7c4ede838f7111b42e18ac' }, ] @@ -4676,13 +4701,13 @@ def test_bulk_enrollment_includes_fulfillment_source_uuid( }, { 'email': 'abc@test.com', - 'course_run_key': 'course-v2:edX+DemoX+Second_Demo_Course', + 'course_run_key': 'course-v1:HarvardX+CoolScience+2016', 'created': True, 'activation_link': None, }, { 'email': 'xyz@test.com', - 'course_run_key': 'course-v2:edX+DemoX+Second_Demo_Course', + 'course_run_key': 'course-v1:HarvardX+CoolScience+2016', 'created': True, 'activation_link': None, } @@ -4727,13 +4752,14 @@ def test_bulk_enrollment_with_notification( self.assertEqual(len(PendingEnrollment.objects.all()), 0) - response = self.client.post( - settings.TEST_SERVER + ENTERPRISE_CUSTOMER_BULK_ENROLL_LEARNERS_IN_COURSES_ENDPOINT, - data=json.dumps(body), - content_type='application/json', - ) - self.assertEqual(response.status_code, expected_code) + with mock.patch("enterprise.models.EnrollmentApiClient.get_course_details", wraps=get_course_details): + response = self.client.post( + settings.TEST_SERVER + ENTERPRISE_CUSTOMER_BULK_ENROLL_LEARNERS_IN_COURSES_ENDPOINT, + data=json.dumps(body), + content_type='application/json', + ) + self.assertEqual(response.status_code, expected_code) response_json = response.json() self.assertEqual(expected_response, response_json) self.assertEqual(len(PendingEnrollment.objects.all()), expected_num_pending_licenses) diff --git a/tests/test_enterprise/test_utils.py b/tests/test_enterprise/test_utils.py index cba4eb3bbe..163cac48b2 100644 --- a/tests/test_enterprise/test_utils.py +++ b/tests/test_enterprise/test_utils.py @@ -21,6 +21,7 @@ serialize_notification_content, ) from test_utils import FAKE_UUIDS, TEST_PASSWORD, TEST_USERNAME, factories +from test_utils.fake_enrollment_api import get_course_details LMS_BASE_URL = 'https://lms.base.url' @@ -401,11 +402,12 @@ def test_enroll_pending_licensed_users_in_courses_succeeds(self): ) licensed_users_info = [{ 'email': 'pending-user-email@example.com', - 'course_run_key': 'course-key-v1', + 'course_run_key': 'course-v1:edX+DemoX+Demo_Course', 'course_mode': 'verified', 'license_uuid': '5b77bdbade7b4fcb838f8111b68e18ae' }] - result = enroll_subsidy_users_in_courses(ent_customer, licensed_users_info) + with mock.patch("enterprise.models.EnrollmentApiClient.get_course_details", wraps=get_course_details): + result = enroll_subsidy_users_in_courses(ent_customer, licensed_users_info) self.assertEqual(result['pending'][0]['email'], 'pending-user-email@example.com') self.assertFalse(result['successes']) From 3d027e8c7513cadbffd6670d800517f26f6dea7f Mon Sep 17 00:00:00 2001 From: Arunmozhi Date: Sun, 7 May 2023 07:47:50 +0530 Subject: [PATCH 4/9] fix: update the admin view tests failing due to cea --- tests/test_admin/test_view.py | 45 +++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/tests/test_admin/test_view.py b/tests/test_admin/test_view.py index 5c1bec505b..287d5a2445 100644 --- a/tests/test_admin/test_view.py +++ b/tests/test_admin/test_view.py @@ -1111,13 +1111,17 @@ def test_post_multi_enroll_pending_user( """ Test that a pending learner can be enrolled in multiple courses. """ - self._post_multi_enroll( - enterprise_catalog_client, - enrollment_client, - course_catalog_client, - track_enrollment, - False, - ) + with mock.patch( + 'enterprise.models.EnrollmentApiClient.get_course_details', + wraps=fake_enrollment_api.get_course_details, + ): + self._post_multi_enroll( + enterprise_catalog_client, + enrollment_client, + course_catalog_client, + track_enrollment, + False, + ) @mock.patch("enterprise.utils.track_enrollment") @mock.patch("enterprise.models.CourseCatalogApiClient") @@ -1771,8 +1775,10 @@ def test_post_successful_test(self): @mock.patch("enterprise.models.CourseCatalogApiClient") @mock.patch("enterprise.api_client.lms.EnrollmentApiClient") @mock.patch("enterprise.models.EnterpriseCatalogApiClient") + @mock.patch("enterprise.models.CourseEnrollmentAllowed") def test_post_link_and_enroll( self, + mock_cea, enterprise_catalog_client, enrollment_client, course_catalog_client, @@ -1805,7 +1811,11 @@ def test_post_link_and_enroll( course_id = "course-v1:EnterpriseX+Training+2017" course_mode = "professional" - response = self._perform_request(columns, data, course=course_id, course_mode=course_mode) + with mock.patch( + 'enterprise.models.EnrollmentApiClient.get_course_details', + wraps=fake_enrollment_api.get_course_details, + ): + response = self._perform_request(columns, data, course=course_id, course_mode=course_mode) enrollment_instance.enroll_user_in_course.assert_called_once_with( user.username, @@ -1830,13 +1840,16 @@ def test_post_link_and_enroll( assert pending_enrollment.sales_force_id == sales_force_id num_messages = len(mail.outbox) assert num_messages == 2 + mock_cea.objects.update_or_create.assert_called_once() @mock.patch("enterprise.utils.track_enrollment") @mock.patch("enterprise.models.CourseCatalogApiClient") @mock.patch("enterprise.api_client.lms.EnrollmentApiClient") @mock.patch("enterprise.models.EnterpriseCatalogApiClient") + @mock.patch("enterprise.models.CourseEnrollmentAllowed") def test_post_link_and_enroll_no_course_details( self, + mock_cea, enterprise_catalog_client, enrollment_client, course_catalog_client, @@ -1861,7 +1874,11 @@ def test_post_link_and_enroll_no_course_details( course_id = "course-v1:EnterpriseX+Training+2017" course_mode = "professional" - response = self._perform_request(columns, data, course=course_id, course_mode=course_mode) + with mock.patch( + 'enterprise.models.EnrollmentApiClient.get_course_details', + wraps=fake_enrollment_api.get_course_details, + ): + response = self._perform_request(columns, data, course=course_id, course_mode=course_mode) enrollment_instance.enroll_user_in_course.assert_called_once_with( user.username, @@ -1883,12 +1900,15 @@ def test_post_link_and_enroll_no_course_details( assert PendingEnterpriseCustomerUser.objects.all()[0].pendingenrollment_set.all()[0].course_id == course_id num_messages = len(mail.outbox) assert num_messages == 0 + mock_cea.objects.update_or_create.assert_called_once() @mock.patch("enterprise.utils.track_enrollment") @mock.patch("enterprise.api_client.lms.EnrollmentApiClient") @mock.patch("enterprise.models.EnterpriseCatalogApiClient") + @mock.patch("enterprise.models.CourseEnrollmentAllowed") def test_post_link_and_enroll_no_notification( self, + mock_cea, enterprise_catalog_client, enrollment_client, track_enrollment, @@ -1910,7 +1930,11 @@ def test_post_link_and_enroll_no_notification( course_id = "course-v1:EnterpriseX+Training+2017" course_mode = "professional" - response = self._perform_request(columns, data, course=course_id, course_mode=course_mode, notify=False) + with mock.patch( + 'enterprise.models.EnrollmentApiClient.get_course_details', + wraps=fake_enrollment_api.get_course_details, + ): + response = self._perform_request(columns, data, course=course_id, course_mode=course_mode, notify=False) enrollment_instance.enroll_user_in_course.assert_called_once_with( user.username, @@ -1931,6 +1955,7 @@ def test_post_link_and_enroll_no_notification( assert PendingEnterpriseCustomerUser.objects.all()[0].pendingenrollment_set.all()[0].course_id == course_id num_messages = len(mail.outbox) assert num_messages == 0 + mock_cea.objects.update_or_create.assert_called_once() @mark.django_db From c5df1b0621f17354d8cbfc280af17bae34213330 Mon Sep 17 00:00:00 2001 From: Arunmozhi Date: Tue, 9 May 2023 17:05:58 +0530 Subject: [PATCH 5/9] feat: adds force enrollment checkbox in manage learners page --- enterprise/admin/forms.py | 6 ++ enterprise/admin/views.py | 9 ++- enterprise/api_client/lms.py | 6 +- .../static/enterprise/js/manage_learners.js | 14 +++- enterprise/utils.py | 12 ++- test_utils/fake_enrollment_api.py | 2 +- tests/test_admin/test_view.py | 80 ++++++++++++++++--- 7 files changed, 108 insertions(+), 21 deletions(-) diff --git a/enterprise/admin/forms.py b/enterprise/admin/forms.py index 288a6b65d6..ba74eadb9e 100644 --- a/enterprise/admin/forms.py +++ b/enterprise/admin/forms.py @@ -68,6 +68,11 @@ class ManageLearnersForm(forms.Form): label=_("Enroll these learners in this course"), required=False, help_text=_("To enroll learners in a course, enter a course ID."), ) + force_enrollment = forms.BooleanField( + label=_("Force Enrollment"), + help_text=_("The selected course is 'Invite Only'. Only staff can enroll learners to this course."), + required=False, + ) course_mode = forms.ChoiceField( label=_("Course enrollment track"), required=False, choices=BLANK_CHOICE_DASH + [ @@ -130,6 +135,7 @@ class Fields: REASON = "reason" SALES_FORCE_ID = "sales_force_id" DISCOUNT = "discount" + FORCE_ENROLLMENT = "force_enrollment" class CsvColumns: """ diff --git a/enterprise/admin/views.py b/enterprise/admin/views.py index 893f30a2e4..3752deb023 100644 --- a/enterprise/admin/views.py +++ b/enterprise/admin/views.py @@ -676,7 +676,8 @@ def _enroll_users( notify=True, enrollment_reason=None, sales_force_id=None, - discount=0.0 + discount=0.0, + force_enrollment=False ): """ Enroll the users with the given email addresses to the course. @@ -689,6 +690,7 @@ def _enroll_users( mode: The enrollment mode the users will be enrolled in the course with course_id: The ID of the course in which we want to enroll notify: Whether to notify (by email) the users that have been enrolled + force_enrollment: Force enrollment into "Invite Only" courses """ pending_messages = [] paid_modes = ['verified', 'professional'] @@ -702,6 +704,7 @@ def _enroll_users( enrollment_reason=enrollment_reason, discount=discount, sales_force_id=sales_force_id, + force_enrollment=force_enrollment, ) all_successes = succeeded + pending if notify: @@ -818,6 +821,7 @@ def post(self, request, customer_uuid): sales_force_id = manage_learners_form.cleaned_data.get(ManageLearnersForm.Fields.SALES_FORCE_ID) course_mode = manage_learners_form.cleaned_data.get(ManageLearnersForm.Fields.COURSE_MODE) course_id = None + force_enrollment = manage_learners_form.cleaned_data.get(ManageLearnersForm.Fields.FORCE_ENROLLMENT) if not course_id_with_emails: course_details = manage_learners_form.cleaned_data.get(ManageLearnersForm.Fields.COURSE) or {} @@ -832,7 +836,8 @@ def post(self, request, customer_uuid): notify=notify, enrollment_reason=manual_enrollment_reason, sales_force_id=sales_force_id, - discount=discount + discount=discount, + force_enrollment=force_enrollment, ) else: for course_id, emails in course_id_with_emails.items(): diff --git a/enterprise/api_client/lms.py b/enterprise/api_client/lms.py index 47e08edb49..ca08c2b499 100644 --- a/enterprise/api_client/lms.py +++ b/enterprise/api_client/lms.py @@ -128,7 +128,7 @@ def has_course_mode(self, course_run_id, mode): course_modes = self.get_course_modes(course_run_id) return any(course_mode for course_mode in course_modes if course_mode['slug'] == mode) - def enroll_user_in_course(self, username, course_id, mode, cohort=None, enterprise_uuid=None): + def enroll_user_in_course(self, username, course_id, mode, cohort=None, enterprise_uuid=None, force_enrollment=False): """ Call the enrollment API to enroll the user in the course specified by course_id. @@ -138,6 +138,7 @@ def enroll_user_in_course(self, username, course_id, mode, cohort=None, enterpri mode (str): The enrollment mode which should be used for the enrollment cohort (str): Add the user to this named cohort enterprise_uuid (str): Add course enterprise uuid + force_enrollment (bool): Force the enrollment even if course is Invite Only Returns: dict: A dictionary containing details of the enrollment, including course details, mode, username, etc. @@ -152,7 +153,8 @@ def enroll_user_in_course(self, username, course_id, mode, cohort=None, enterpri 'is_active': True, 'mode': mode, 'cohort': cohort, - 'enterprise_uuid': str(enterprise_uuid) + 'enterprise_uuid': str(enterprise_uuid), + 'force_enrollment': force_enrollment, } ) response.raise_for_status() diff --git a/enterprise/static/enterprise/js/manage_learners.js b/enterprise/static/enterprise/js/manage_learners.js index 5b12d4ad0b..48723789c9 100644 --- a/enterprise/static/enterprise/js/manage_learners.js +++ b/enterprise/static/enterprise/js/manage_learners.js @@ -9,7 +9,7 @@ function makeOption(name, value) { return $("").text(name).val(value); } -function fillModeDropdown(data) { +function updateCourseData(data) { /* Given a set of data fetched from the enrollment API, populate the Course Mode dropdown with those options that are valid for the course entered in the @@ -19,6 +19,12 @@ function fillModeDropdown(data) { var previous_value = $course_mode.val(); applyModes(data.course_modes); $course_mode.val(previous_value); + /* + * If the course is invite-only, show the force enrollment box. + */ + if (data.invite_only) { + $("#id_force_enrollment").parent().show(); + } } function applyModes(modes) { @@ -43,7 +49,7 @@ function loadCourseModes(success, failure) { return; } $.ajax({method: 'get', url: enrollmentApiRoot + "course/" + courseId}) - .done(success || fillModeDropdown) + .done(success || updateCourseData) .fail(failure || function (err, jxHR, errstat) { disableMode(disableReason); }); }); } @@ -139,6 +145,10 @@ function loadPage() { } else if (programEnrollment.$control.val()) { programEnrollment.$control.trigger("input"); } + + // hide the force_invite_only checkbox by default + $("#id_force_enrollment").parent().hide(); + $("#learner-management-form").submit(addCheckedLearnersToEnrollBox); } diff --git a/enterprise/utils.py b/enterprise/utils.py index 96ca99750b..166c294bd1 100644 --- a/enterprise/utils.py +++ b/enterprise/utils.py @@ -1700,12 +1700,15 @@ def enroll_user(enterprise_customer, user, course_mode, *course_ids, **kwargs): user: The user model object who needs to be enrolled in the course course_mode: The string representation of the mode with which the enrollment should be created *course_ids: An iterable containing any number of course IDs to eventually enroll the user in. - kwargs: Should contain enrollment_client if it's already been instantiated and should be passed in. + kwargs: Contains optional params such as: + - enrollment_client, if it's already been instantiated and should be passed in + - force_enrollment, if the course is "Invite Only" and the "force_enrollment" is needed Returns: Boolean: Whether or not enrollment succeeded for all courses specified """ enrollment_client = kwargs.pop('enrollment_client', None) + force_enrollment = kwargs.pop('force_enrollment', False) if not enrollment_client: from enterprise.api_client.lms import EnrollmentApiClient # pylint: disable=import-outside-toplevel enrollment_client = EnrollmentApiClient() @@ -1720,7 +1723,8 @@ def enroll_user(enterprise_customer, user, course_mode, *course_ids, **kwargs): user.username, course_id, course_mode, - enterprise_uuid=str(enterprise_customer_user.enterprise_customer.uuid) + enterprise_uuid=str(enterprise_customer_user.enterprise_customer.uuid), + force_enrollment=force_enrollment, ) except HttpClientError as exc: # Check if user is already enrolled then we should ignore exception @@ -2063,6 +2067,7 @@ def enroll_users_in_course( enrollment_reason=None, discount=0.0, sales_force_id=None, + force_enrollment=False, ): """ Enroll existing users in a course, and create a pending enrollment for nonexisting users. @@ -2076,6 +2081,7 @@ def enroll_users_in_course( enrollment_reason (str): A reason for enrollment. discount (Decimal): Percentage discount for enrollment. sales_force_id (str): Salesforce opportunity id. + force_enrollment (bool): Force enrollment into 'Invite Only' courses. Returns: successes: A list of users who were successfully enrolled in the course. @@ -2092,7 +2098,7 @@ def enroll_users_in_course( failures = [] for user in existing_users: - succeeded = enroll_user(enterprise_customer, user, course_mode, course_id) + succeeded = enroll_user(enterprise_customer, user, course_mode, course_id, force_enrollment=force_enrollment) if succeeded: successes.append(user) if enrollment_requester and enrollment_reason: diff --git a/test_utils/fake_enrollment_api.py b/test_utils/fake_enrollment_api.py index 700cc31d38..d06eaaf36b 100644 --- a/test_utils/fake_enrollment_api.py +++ b/test_utils/fake_enrollment_api.py @@ -150,7 +150,7 @@ def get_course_details(course_id): return None -def enroll_user_in_course(user, course_id, mode, cohort=None, enterprise_uuid=None): +def enroll_user_in_course(user, course_id, mode, cohort=None, enterprise_uuid=None, force_enrollment=False): """ Fake implementation. """ diff --git a/tests/test_admin/test_view.py b/tests/test_admin/test_view.py index 287d5a2445..e70c24444a 100644 --- a/tests/test_admin/test_view.py +++ b/tests/test_admin/test_view.py @@ -894,7 +894,7 @@ def test_post_existing_pending_record_with_another_enterprise_customer(self): self._test_post_existing_record_response(response) assert PendingEnterpriseCustomerUser.objects.filter(user_email=email).count() == 2 - def _enroll_user_request(self, user, mode, course_id="", notify=True, reason="tests", discount=0.0): + def _enroll_user_request(self, user, mode, course_id="", notify=True, reason="tests", discount=0.0, force_enrollment=False): """ Perform post request to log in and submit the form to enroll a user. """ @@ -919,6 +919,7 @@ def _enroll_user_request(self, user, mode, course_id="", notify=True, reason="te ManageLearnersForm.Fields.NOTIFY: notify, ManageLearnersForm.Fields.REASON: reason, ManageLearnersForm.Fields.DISCOUNT: discount, + ManageLearnersForm.Fields.FORCE_ENROLLMENT: force_enrollment, }) return response @@ -977,7 +978,8 @@ def test_post_enroll_user( user.username, course_id, mode, - enterprise_uuid=str(self.enterprise_customer.uuid) + enterprise_uuid=str(self.enterprise_customer.uuid), + force_enrollment=False ) if enrollment_exists: track_enrollment.assert_not_called() @@ -1050,7 +1052,8 @@ def _post_multi_enroll( user.username, course_id, mode, - enterprise_uuid=str(self.enterprise_customer.uuid) + enterprise_uuid=str(self.enterprise_customer.uuid), + force_enrollment=False, ) track_enrollment.assert_called_with('admin-enrollment', user.id, course_id) self._assert_django_messages(response, { @@ -1150,7 +1153,8 @@ def test_post_enroll_no_course_detail( user.username, course_id, mode, - enterprise_uuid=str(self.enterprise_customer.uuid) + enterprise_uuid=str(self.enterprise_customer.uuid), + force_enrollment=False ) track_enrollment.assert_called_once_with('admin-enrollment', user.id, course_id) self._assert_django_messages(response, { @@ -1167,6 +1171,51 @@ def test_post_enroll_no_course_detail( num_messages = len(mail.outbox) assert num_messages == 0 + @mock.patch("enterprise.utils.track_enrollment") + @mock.patch("enterprise.models.CourseCatalogApiClient") + @mock.patch("enterprise.api_client.lms.EnrollmentApiClient") + @mock.patch("enterprise.models.EnterpriseCatalogApiClient") + @ddt.data(True, False) + def test_post_enroll_force_enrollment( + self, + force_enrollment, + enterprise_catalog_client, + enrollment_client, + course_catalog_client, + track_enrollment, + ): + catalog_instance = course_catalog_client.return_value + catalog_instance.get_course_run.return_value = {} + enrollment_instance = enrollment_client.return_value + enrollment_instance.enroll_user_in_course.side_effect = fake_enrollment_api.enroll_user_in_course + enrollment_instance.get_course_details.side_effect = fake_enrollment_api.get_course_details + enterprise_catalog_instance = enterprise_catalog_client.return_value + enterprise_catalog_instance.enterprise_contains_content_items.return_value = True + + user = UserFactory() + course_id = "course-v1:HarvardX+CoolScience+2016" + mode = "verified" + response = self._enroll_user_request(user, mode, course_id=course_id, force_enrollment=force_enrollment) + enrollment_instance.enroll_user_in_course.assert_called_once_with( + user.username, + course_id, + mode, + enterprise_uuid=str(self.enterprise_customer.uuid), + force_enrollment=force_enrollment + ) + track_enrollment.assert_called_once_with('admin-enrollment', user.id, course_id) + self._assert_django_messages(response, { + (messages.SUCCESS, "1 learner was enrolled in {}.".format(course_id)), + }) + all_enterprise_enrollments = EnterpriseCourseEnrollment.objects.all() + num_enterprise_enrollments = len(all_enterprise_enrollments) + assert num_enterprise_enrollments == 1 + enrollment = all_enterprise_enrollments[0] + assert enrollment.enterprise_customer_user.user == user + assert enrollment.course_id == course_id + assert enrollment.source is not None + assert enrollment.source.slug == EnterpriseEnrollmentSource.MANUAL + @mock.patch("enterprise.utils.track_enrollment") @mock.patch("enterprise.models.CourseCatalogApiClient") @mock.patch("enterprise.api_client.lms.EnrollmentApiClient") @@ -1215,7 +1264,8 @@ def test_post_enroll_course_when_enrollment_closed( user.username, course_id, mode, - enterprise_uuid=str(self.enterprise_customer.uuid) + enterprise_uuid=str(self.enterprise_customer.uuid), + force_enrollment=False ) @mock.patch("enterprise.utils.track_enrollment") @@ -1249,7 +1299,8 @@ def test_post_enroll_course_when_enrollment_closed_mode_changed( user.username, course_id, mode, - enterprise_uuid=str(self.enterprise_customer.uuid) + enterprise_uuid=str(self.enterprise_customer.uuid), + force_enrollment=False ) track_enrollment.assert_not_called() self._assert_django_messages(response, { @@ -1290,7 +1341,8 @@ def test_post_enroll_course_when_enrollment_closed_no_sce_exists( user.username, course_id, mode, - enterprise_uuid=str(self.enterprise_customer.uuid) + enterprise_uuid=str(self.enterprise_customer.uuid), + force_enrollment=False ) track_enrollment.assert_not_called() self._assert_django_messages(response, { @@ -1335,7 +1387,8 @@ def test_post_enroll_with_missing_course_start_date( user.username, course_id, mode, - enterprise_uuid=str(self.enterprise_customer.uuid) + enterprise_uuid=str(self.enterprise_customer.uuid), + force_enrollment=False ) track_enrollment.assert_called_once_with('admin-enrollment', user.id, course_id) self._assert_django_messages(response, { @@ -1681,6 +1734,7 @@ def test_post_create_course_enrollments( enrollment_requester=ANY, enterprise_customer=ANY, sales_force_id=ANY, + force_enrollment=ANY, ) enroll_users_in_course_mock.assert_any_call( course_id=second_course_id, @@ -1691,6 +1745,7 @@ def test_post_create_course_enrollments( enrollment_requester=ANY, enterprise_customer=ANY, sales_force_id=ANY, + force_enrollment=ANY, ) else: enroll_users_in_course_mock.assert_not_called() @@ -1821,7 +1876,8 @@ def test_post_link_and_enroll( user.username, course_id, course_mode, - enterprise_uuid=str(self.enterprise_customer.uuid) + enterprise_uuid=str(self.enterprise_customer.uuid), + force_enrollment=False ) track_enrollment.assert_called_once_with('admin-enrollment', user.id, course_id) pending_user_message = ( @@ -1884,7 +1940,8 @@ def test_post_link_and_enroll_no_course_details( user.username, course_id, course_mode, - enterprise_uuid=str(self.enterprise_customer.uuid) + enterprise_uuid=str(self.enterprise_customer.uuid), + force_enrollment=False ) track_enrollment.assert_called_once_with('admin-enrollment', user.id, course_id) pending_user_message = ( @@ -1940,7 +1997,8 @@ def test_post_link_and_enroll_no_notification( user.username, course_id, course_mode, - enterprise_uuid=str(self.enterprise_customer.uuid) + enterprise_uuid=str(self.enterprise_customer.uuid), + force_enrollment=False ) track_enrollment.assert_called_once_with('admin-enrollment', user.id, course_id) pending_user_message = ( From ff48dff81655b5b6c23811878f33c79dae34c577 Mon Sep 17 00:00:00 2001 From: Arunmozhi Date: Tue, 9 May 2023 19:43:32 +0530 Subject: [PATCH 6/9] feat: add force enrollment for csv upload --- enterprise/admin/views.py | 3 ++- enterprise/static/enterprise/js/manage_learners.js | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/enterprise/admin/views.py b/enterprise/admin/views.py index 3752deb023..ba9c5bd890 100644 --- a/enterprise/admin/views.py +++ b/enterprise/admin/views.py @@ -852,7 +852,8 @@ def post(self, request, customer_uuid): notify=notify, enrollment_reason=manual_enrollment_reason, sales_force_id=sales_force_id, - discount=discount + discount=discount, + force_enrollment=force_enrollment, ) # Redirect to GET if everything went smooth. diff --git a/enterprise/static/enterprise/js/manage_learners.js b/enterprise/static/enterprise/js/manage_learners.js index 48723789c9..cd2c312aba 100644 --- a/enterprise/static/enterprise/js/manage_learners.js +++ b/enterprise/static/enterprise/js/manage_learners.js @@ -140,6 +140,17 @@ function loadPage() { programEnrollment.$control.oldValue = null; }); + $("#id_bulk_upload_csv").change(function(e) { + if (e.target.value) { + var force_enrollment = $("#id_force_enrollment"); + force_enrollment.parent().show(); + force_enrollment.siblings(".helptext")[0].innerHTML = gettext( + "If any of the courses in the CSV file are marked 'Invite Only', " + + "this should be enabled for the enrollments to go through in those courses." + ); + } + }); + if (courseEnrollment.$control.val()) { courseEnrollment.$control.trigger("input"); } else if (programEnrollment.$control.val()) { From aff0098a4617c0caec451d943b18af4483c6cbec Mon Sep 17 00:00:00 2001 From: Arunmozhi Date: Fri, 12 May 2023 14:46:55 +0530 Subject: [PATCH 7/9] docs: update changelog and a comment --- CHANGELOG.rst | 5 +++++ enterprise/admin/views.py | 2 +- .../static/enterprise/js/manage_learners.js | 17 ++++++++++++++--- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index d72d7d3216..b5c583dd7f 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -18,6 +18,11 @@ Unreleased * Switch from ``edx-sphinx-theme`` to ``sphinx-book-theme`` since the former is deprecated +[3.66.0] +-------- +feat: enable enrolling leaners to invite_only courses via manage learners view + + [3.65.2] -------- feat: extending the enterprise fulfillment api serializer to contain more info diff --git a/enterprise/admin/views.py b/enterprise/admin/views.py index ba9c5bd890..128980f9bd 100644 --- a/enterprise/admin/views.py +++ b/enterprise/admin/views.py @@ -677,7 +677,7 @@ def _enroll_users( enrollment_reason=None, sales_force_id=None, discount=0.0, - force_enrollment=False + force_enrollment=False, ): """ Enroll the users with the given email addresses to the course. diff --git a/enterprise/static/enterprise/js/manage_learners.js b/enterprise/static/enterprise/js/manage_learners.js index cd2c312aba..940092467b 100644 --- a/enterprise/static/enterprise/js/manage_learners.js +++ b/enterprise/static/enterprise/js/manage_learners.js @@ -19,9 +19,8 @@ function updateCourseData(data) { var previous_value = $course_mode.val(); applyModes(data.course_modes); $course_mode.val(previous_value); - /* - * If the course is invite-only, show the force enrollment box. - */ + + // If the course is invite-only, show the force enrollment box. if (data.invite_only) { $("#id_force_enrollment").parent().show(); } @@ -140,6 +139,18 @@ function loadPage() { programEnrollment.$control.oldValue = null; }); + // NOTE: As the course details won't be fetched for course id in the CSV + // file, this has a potential side-effect of enrolling learners into the courses + // which might be marked as closed for reasons other then being "Invite Only". + // + // This is considered as a reasonable tradeoff at the time of this addition. + // Currently, the EnrollmentListView does not support invitation only courses. + // This problem does not happen in the Instructor Dashboard because it doesn't + // invoke access checks when calling the enroll method. Modifying the enroll method + // is a high-risk change, and it seems that the API will need some changes in + // the near future anyway - when the Instructor Dashboard is converted into an + // MFE (it could be an excellent opportunity to eliminate many legacy behaviors + // there, too). $("#id_bulk_upload_csv").change(function(e) { if (e.target.value) { var force_enrollment = $("#id_force_enrollment"); From 110bdb03345991566a8ae1d6c7df8baa2e7455e1 Mon Sep 17 00:00:00 2001 From: Arunmozhi Date: Fri, 12 May 2023 17:14:23 +0530 Subject: [PATCH 8/9] fix: lint and other quality issues --- enterprise/api_client/lms.py | 10 +++++++++- enterprise/utils.py | 2 +- test_utils/fake_enrollment_api.py | 3 ++- tests/test_admin/test_view.py | 11 ++++++++++- tests/test_enterprise/api/test_views.py | 10 +++++----- tests/test_enterprise/api_client/test_lms.py | 3 ++- 6 files changed, 29 insertions(+), 10 deletions(-) diff --git a/enterprise/api_client/lms.py b/enterprise/api_client/lms.py index ca08c2b499..cb06742e69 100644 --- a/enterprise/api_client/lms.py +++ b/enterprise/api_client/lms.py @@ -128,7 +128,15 @@ def has_course_mode(self, course_run_id, mode): course_modes = self.get_course_modes(course_run_id) return any(course_mode for course_mode in course_modes if course_mode['slug'] == mode) - def enroll_user_in_course(self, username, course_id, mode, cohort=None, enterprise_uuid=None, force_enrollment=False): + def enroll_user_in_course( + self, + username, + course_id, + mode, + cohort=None, + enterprise_uuid=None, + force_enrollment=False, + ): """ Call the enrollment API to enroll the user in the course specified by course_id. diff --git a/enterprise/utils.py b/enterprise/utils.py index 166c294bd1..4139552e7f 100644 --- a/enterprise/utils.py +++ b/enterprise/utils.py @@ -1702,7 +1702,7 @@ def enroll_user(enterprise_customer, user, course_mode, *course_ids, **kwargs): *course_ids: An iterable containing any number of course IDs to eventually enroll the user in. kwargs: Contains optional params such as: - enrollment_client, if it's already been instantiated and should be passed in - - force_enrollment, if the course is "Invite Only" and the "force_enrollment" is needed + - force_enrollment, if the course is "Invite Only" and the "force_enrollment" is needed Returns: Boolean: Whether or not enrollment succeeded for all courses specified diff --git a/test_utils/fake_enrollment_api.py b/test_utils/fake_enrollment_api.py index d06eaaf36b..7db03d5f80 100644 --- a/test_utils/fake_enrollment_api.py +++ b/test_utils/fake_enrollment_api.py @@ -150,7 +150,8 @@ def get_course_details(course_id): return None -def enroll_user_in_course(user, course_id, mode, cohort=None, enterprise_uuid=None, force_enrollment=False): +def enroll_user_in_course(user, course_id, mode, cohort=None, enterprise_uuid=None, force_enrollment=False): # pylint: disable=unused-argument + """ Fake implementation. """ diff --git a/tests/test_admin/test_view.py b/tests/test_admin/test_view.py index e70c24444a..465058ead4 100644 --- a/tests/test_admin/test_view.py +++ b/tests/test_admin/test_view.py @@ -894,7 +894,16 @@ def test_post_existing_pending_record_with_another_enterprise_customer(self): self._test_post_existing_record_response(response) assert PendingEnterpriseCustomerUser.objects.filter(user_email=email).count() == 2 - def _enroll_user_request(self, user, mode, course_id="", notify=True, reason="tests", discount=0.0, force_enrollment=False): + def _enroll_user_request( + self, + user, + mode, + course_id="", + notify=True, + reason="tests", + discount=0.0, + force_enrollment=False + ): """ Perform post request to log in and submit the form to enroll a user. """ diff --git a/tests/test_enterprise/api/test_views.py b/tests/test_enterprise/api/test_views.py index 8dcb364664..a3c36f537c 100644 --- a/tests/test_enterprise/api/test_views.py +++ b/tests/test_enterprise/api/test_views.py @@ -68,8 +68,8 @@ update_program_with_enterprise_context, ) from test_utils.factories import FAKER, EnterpriseCustomerUserFactory, PendingEnterpriseCustomerUserFactory, UserFactory -from test_utils.fake_enterprise_api import get_default_branding_object from test_utils.fake_enrollment_api import get_course_details +from test_utils.fake_enterprise_api import get_default_branding_object fake = Faker() @@ -4305,10 +4305,10 @@ def test_bulk_enrollment_in_bulk_courses_pending_licenses( self.assertEqual(len(PendingEnrollment.objects.all()), 0) - with mock.patch("enterprise.models.EnrollmentApiClient.get_course_details", wraps=get_course_details) as mock_client: - # mock_client = mock.Mock( - # get_course_details=get_course_details - # ) + with mock.patch( + "enterprise.models.EnrollmentApiClient.get_course_details", + wraps=get_course_details + ): response = self.client.post( settings.TEST_SERVER + ENTERPRISE_CUSTOMER_BULK_ENROLL_LEARNERS_IN_COURSES_ENDPOINT, data=json.dumps(body), diff --git a/tests/test_enterprise/api_client/test_lms.py b/tests/test_enterprise/api_client/test_lms.py index abaa1307d0..d15f27725b 100644 --- a/tests/test_enterprise/api_client/test_lms.py +++ b/tests/test_enterprise/api_client/test_lms.py @@ -99,7 +99,8 @@ def test_enroll_user_in_course(): 'mode': mode, 'cohort': cohort, 'is_active': True, - 'enterprise_uuid': 'None' + 'enterprise_uuid': 'None', + 'force_enrollment': False } responses.add( responses.POST, From 75b9f10374f277e4ee32bacb4409cca9a1fb5214 Mon Sep 17 00:00:00 2001 From: Edward Zarecor Date: Wed, 5 Jul 2023 15:49:45 +0200 Subject: [PATCH 9/9] fix: stray underline --- CHANGELOG.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 7eac69ded5..5190cad815 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -17,7 +17,6 @@ Unreleased ---------- feat: enable enrolling leaners to invite_only courses via manage learners view -======= [3.68.1] -------- fix: pick first object from CourseDetails