From 8c499ec8a63aca04fb266b4cb2507c98589e483a Mon Sep 17 00:00:00 2001 From: Arunmozhi Date: Tue, 30 May 2023 12:55:05 +0530 Subject: [PATCH] feat: enable enrolling in invite_only courses via manage learners UI When enrollment of users to courses marked as "Invite Only" is attempted from the "Manage Learners" page in the Admin, it results in an error due to EnrollmentClosedError. This commit, adds handles that scenario by setting "force_enrollment" to True and creating a CourseEnrollmentAllowed object if the users with the specified emails didn't exist. --- .github/workflows/ci.yml | 1 - CHANGELOG.rst | 4 + enterprise/__init__.py | 2 +- enterprise/admin/forms.py | 6 + enterprise/admin/views.py | 12 +- enterprise/api_client/lms.py | 13 +- enterprise/models.py | 17 ++- .../static/enterprise/js/manage_learners.js | 36 ++++- enterprise/utils.py | 12 +- requirements/ci.txt | 2 +- test_utils/fake_enrollment_api.py | 2 +- tests/test_admin/test_view.py | 132 +++++++++++++++--- tests/test_enterprise/api/test_views.py | 65 ++++++--- tests/test_enterprise/api_client/test_lms.py | 3 +- tests/test_enterprise/test_utils.py | 6 +- 15 files changed, 253 insertions(+), 60 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 27428607a5..dbf7d46941 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,7 +4,6 @@ on: push: branches: [master] pull_request: - branches: [master] jobs: run_tests: diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 9526237dda..e12f610d94 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -17,6 +17,10 @@ Unreleased ---------- * nothing +[3.42.7] +-------- +feat: allow enrollment to invite-only courses via manage learners admin page + [3.42.6] -------- feat: allow enrollment api admin to see all enrollments diff --git a/enterprise/__init__.py b/enterprise/__init__.py index e063398869..decedbe1cb 100644 --- a/enterprise/__init__.py +++ b/enterprise/__init__.py @@ -2,6 +2,6 @@ Your project description goes here. """ -__version__ = "3.42.6" +__version__ = "3.42.7" default_app_config = "enterprise.apps.EnterpriseConfig" diff --git a/enterprise/admin/forms.py b/enterprise/admin/forms.py index fddb2408ee..8d6c9234af 100644 --- a/enterprise/admin/forms.py +++ b/enterprise/admin/forms.py @@ -67,6 +67,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 + [ @@ -128,6 +133,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 80cf8716ec..ee2a45fdbb 100644 --- a/enterprise/admin/views.py +++ b/enterprise/admin/views.py @@ -641,7 +641,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. @@ -654,6 +655,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'] @@ -667,6 +669,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: @@ -783,6 +786,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 {} @@ -797,7 +801,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(): @@ -812,7 +817,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/api_client/lms.py b/enterprise/api_client/lms.py index c0114c3ae4..3aa681d65a 100644 --- a/enterprise/api_client/lms.py +++ b/enterprise/api_client/lms.py @@ -230,7 +230,15 @@ def has_course_mode(self, course_run_id, mode): return any(course_mode for course_mode in course_modes if course_mode['slug'] == mode) @JwtLmsApiClient.refresh_token - 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. @@ -252,7 +260,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, } ) diff --git a/enterprise/models.py b/enterprise/models.py index 672d16a588..07d90cf7ec 100644 --- a/enterprise/models.py +++ b/enterprise/models.py @@ -75,9 +75,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 openedx.core.djangoapps.content.course_overviews.models import CourseOverview @@ -647,7 +648,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, diff --git a/enterprise/static/enterprise/js/manage_learners.js b/enterprise/static/enterprise/js/manage_learners.js index 5b12d4ad0b..940092467b 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,11 @@ 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 +48,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); }); }); } @@ -134,11 +139,38 @@ 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"); + 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()) { 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 76b547ade3..e48127d50f 100644 --- a/enterprise/utils.py +++ b/enterprise/utils.py @@ -1658,12 +1658,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() @@ -1678,7 +1681,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 @@ -1956,6 +1960,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. @@ -1969,6 +1974,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. @@ -1985,7 +1991,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/requirements/ci.txt b/requirements/ci.txt index 84b7c6e70c..2b3e4e9a69 100644 --- a/requirements/ci.txt +++ b/requirements/ci.txt @@ -8,7 +8,7 @@ certifi==2021.10.8 # via requests charset-normalizer==2.0.11 # via requests -codecov==2.1.12 +codecov==2.1.13 # via -r requirements/ci.in coverage==6.3.1 # via codecov diff --git a/test_utils/fake_enrollment_api.py b/test_utils/fake_enrollment_api.py index 95e7ebfc47..dbf65df3f8 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): # pylint: disable=unused-argument """ Fake implementation. """ diff --git a/tests/test_admin/test_view.py b/tests/test_admin/test_view.py index e318aea7ba..480469b1bd 100644 --- a/tests/test_admin/test_view.py +++ b/tests/test_admin/test_view.py @@ -820,7 +820,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): + 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. """ @@ -844,6 +853,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 @@ -902,7 +912,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() @@ -975,7 +986,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, { @@ -1036,20 +1048,71 @@ def test_post_multi_enroll_pending_user( """ Test that a pending learner can be enrolled in multiple courses. """ - self._post_multi_enroll( + 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") + @mock.patch("enterprise.api_client.lms.EnrollmentApiClient") + @mock.patch("enterprise.models.EnterpriseCatalogApiClient") + def test_post_enroll_no_course_detail( + self, enterprise_catalog_client, enrollment_client, course_catalog_client, track_enrollment, - False, + ): + 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) + enrollment_instance.enroll_user_in_course.assert_called_once_with( + user.username, + course_id, + mode, + 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, { + (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 + 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") - def test_post_enroll_no_course_detail( + @ddt.data(True, False) + def test_post_enroll_force_enrollment( self, + force_enrollment, enterprise_catalog_client, enrollment_client, course_catalog_client, @@ -1066,12 +1129,13 @@ def test_post_enroll_no_course_detail( user = UserFactory() course_id = "course-v1:HarvardX+CoolScience+2016" mode = "verified" - response = self._enroll_user_request(user, mode, course_id=course_id) + 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) + 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, { @@ -1085,8 +1149,6 @@ def test_post_enroll_no_course_detail( assert enrollment.course_id == course_id assert enrollment.source is not None assert enrollment.source.slug == EnterpriseEnrollmentSource.MANUAL - num_messages = len(mail.outbox) - assert num_messages == 0 @mock.patch("enterprise.utils.track_enrollment") @mock.patch("enterprise.models.CourseCatalogApiClient") @@ -1136,7 +1198,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") @@ -1170,7 +1233,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, { @@ -1211,7 +1275,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, { @@ -1256,7 +1321,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, { @@ -1570,6 +1636,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, @@ -1580,6 +1647,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() @@ -1664,8 +1732,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, @@ -1698,13 +1768,18 @@ 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, 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 = ( @@ -1723,13 +1798,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, @@ -1754,13 +1832,18 @@ 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, 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 = ( @@ -1776,12 +1859,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, @@ -1803,13 +1889,18 @@ 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, 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 = ( @@ -1824,6 +1915,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 diff --git a/tests/test_enterprise/api/test_views.py b/tests/test_enterprise/api/test_views.py index af831d3302..4fdb9f4b4d 100644 --- a/tests/test_enterprise/api/test_views.py +++ b/tests/test_enterprise/api/test_views.py @@ -67,6 +67,7 @@ ) from test_utils.decorators import mock_api_response from test_utils.factories import FAKER, PendingEnterpriseCustomerUserFactory +from test_utils.fake_enrollment_api import get_course_details from test_utils.fake_enterprise_api import get_default_branding_object fake = Faker() @@ -2600,6 +2601,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( @@ -2797,7 +2799,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. @@ -3359,6 +3362,7 @@ class TestBulkEnrollment(BaseTestEnterpriseAPIViews): 'expected_response': {'non_field_errors': ['Must include the "licenses_info" parameter in request.']}, 'expected_num_pending_licenses': 0, 'expected_events': None, + 'expected_cea': 0, }, { 'body': { @@ -3370,6 +3374,7 @@ class TestBulkEnrollment(BaseTestEnterpriseAPIViews): }, 'expected_num_pending_licenses': 0, 'expected_events': None, + 'expected_cea': 0, }, { 'body': { @@ -3381,6 +3386,7 @@ class TestBulkEnrollment(BaseTestEnterpriseAPIViews): }, 'expected_num_pending_licenses': 0, 'expected_events': None, + 'expected_cea': 0, }, { 'body': { @@ -3396,6 +3402,7 @@ class TestBulkEnrollment(BaseTestEnterpriseAPIViews): }, 'expected_num_pending_licenses': 0, 'expected_events': None, + 'expected_cea': 0, }, # Single learner, single course success { @@ -3419,6 +3426,7 @@ class TestBulkEnrollment(BaseTestEnterpriseAPIViews): }, '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 { @@ -3459,6 +3467,7 @@ class TestBulkEnrollment(BaseTestEnterpriseAPIViews): 'expected_events': [ mock.call(PATHWAY_CUSTOMER_ADMIN_ENROLLMENT, 1, 'course-v1:edX+DemoX+Demo_Course'), ], + 'expected_cea': 0, }, # Multi-learner, multi-course success { @@ -3476,12 +3485,12 @@ class TestBulkEnrollment(BaseTestEnterpriseAPIViews): }, { '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' }, ] @@ -3504,13 +3513,13 @@ class TestBulkEnrollment(BaseTestEnterpriseAPIViews): }, { '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, } @@ -3520,16 +3529,19 @@ class TestBulkEnrollment(BaseTestEnterpriseAPIViews): '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, @@ -3538,6 +3550,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. @@ -3554,11 +3567,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 + ): + 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() @@ -3573,6 +3592,7 @@ 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() @@ -3593,12 +3613,12 @@ def test_bulk_enrollment_in_bulk_courses_pending_licenses( }, { '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' }, ] @@ -3621,13 +3641,13 @@ def test_bulk_enrollment_in_bulk_courses_pending_licenses( }, { '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, } @@ -3637,7 +3657,7 @@ def test_bulk_enrollment_in_bulk_courses_pending_licenses( '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:HarvardX+CoolScience+2016') ], }, ) @@ -3672,13 +3692,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/api_client/test_lms.py b/tests/test_enterprise/api_client/test_lms.py index 6ae3715d9e..f73cc75286 100644 --- a/tests/test_enterprise/api_client/test_lms.py +++ b/tests/test_enterprise/api_client/test_lms.py @@ -94,7 +94,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, diff --git a/tests/test_enterprise/test_utils.py b/tests/test_enterprise/test_utils.py index ba3b5bb0b6..884627e886 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' @@ -243,11 +244,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_licensed_users_in_courses(ent_customer, licensed_users_info) + with mock.patch("enterprise.models.EnrollmentApiClient.get_course_details", wraps=get_course_details): + result = enroll_licensed_users_in_courses(ent_customer, licensed_users_info) self.assertEqual(result['pending'][0]['email'], 'pending-user-email@example.com') self.assertFalse(result['successes'])