fix: respect allow_enrollment_in_invite_only_courses (#14)

* 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.

* fix: create CEA object when enrolling using a license flow (#18)

.github/workflows/ci.yml

@@ -4,7 +4,6 @@ on:
   push:
     branches: [master]
   pull_request:
-    branches: [master]
 
 concurrency:
   group: ci-${{ github.event.pull_request.number || github.ref }}

.github/workflows/mysql8-migrations.yml

@@ -56,7 +56,7 @@ jobs:
         pip uninstall -y mysqlclient
         pip install --no-binary mysqlclient mysqlclient
         pip uninstall -y xmlsec
-        pip install --no-binary xmlsec xmlsec
+        pip install --no-binary xmlsec xmlsec==1.3.13
         pip install backports.zoneinfo
     - name: Initiate Services
       run: |

enterprise/admin/__init__.py

@@ -202,7 +202,7 @@ class EnterpriseCustomerAdmin(DjangoObjectActions, SimpleHistoryAdmin):
         ('Integration and learning platform settings', {
             'fields': ('enable_portal_lms_configurations_screen', 'enable_portal_saml_configuration_screen',
                        'enable_slug_login', 'replace_sensitive_sso_username', 'hide_course_original_price',
-                       'enable_generation_of_api_credentials')
+                       'enable_generation_of_api_credentials', 'allow_enrollment_in_invite_only_courses')
         }),
         ('Recommended default settings for all enterprise customers', {
             'fields': ('site', 'customer_type', 'enable_learner_portal',

enterprise/api/v1/views/enterprise_customer.py

@@ -38,6 +38,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,
 )
@@ -241,6 +242,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:
@@ -254,6 +257,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:

enterprise/api_client/lms.py

@@ -6,6 +6,7 @@
 import time
 from urllib.parse import urljoin
 
+import requests
 from opaque_keys.edx.keys import CourseKey
 from requests.exceptions import (  # pylint: disable=redefined-builtin
     ConnectionError,
@@ -274,6 +275,34 @@ def get_enrolled_courses(self, username):
         response.raise_for_status()
         return response.json()
 
+    def allow_enrollment(self, email, course_id, auto_enroll=False):
+        """
+        Call the enrollment API to allow enrollment for the given email and course_id.
+
+        Args:
+            email (str): The email address of the user to be allowed to enroll in the course.
+            course_id (str): The string value of the course's unique identifier.
+            auto_enroll (bool): Whether to auto-enroll the user in the course upon registration / activation.
+
+        Returns:
+            dict: A dictionary containing details of the created CourseEnrollmentAllowed object.
+
+        """
+        api_url = self.get_api_url("enrollment_allowed")
+        response = self.client.post(
+            f"{api_url}/",
+            json={
+                'email': email,
+                'course_id': course_id,
+                'auto_enroll': auto_enroll,
+            }
+        )
+        if response.status_code == requests.codes.conflict:
+            LOGGER.info(response.json()["message"])
+        else:
+            response.raise_for_status()
+        return response.json()
+
 
 class CourseApiClient(NoAuthAPIClient):
     """

enterprise/utils.py

@@ -58,6 +58,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
@@ -2028,6 +2033,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')
@@ -2054,6 +2060,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,
@@ -2291,6 +2303,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.
@@ -2387,19 +2407,14 @@ def truncate_string(string, max_length=MAX_ALLOWED_TEXT_LENGTH):
 
 def ensure_course_enrollment_is_allowed(course_id, email, enrollment_api_client):
     """
-    Create a CourseEnrollmentAllowed object for invitation-only courses.
+    Calls the enrollment API to create a CourseEnrollmentAllowed object for
+    invitation-only courses.
 
     Arguments:
         course_id (str): ID of the course to allow enrollment
         email (str): email of the user whose enrollment should be allowed
         enrollment_api_client (:class:`enterprise.api_client.lms.EnrollmentApiClient`): Enrollment API Client
     """
-    if not CourseEnrollmentAllowed:
-        raise NotConnectedToOpenEdX()
-
     course_details = enrollment_api_client.get_course_details(course_id)
     if course_details["invite_only"]:
-        CourseEnrollmentAllowed.objects.update_or_create(
-            course_id=course_id,
-            email=email,
-        )
+        enrollment_api_client.allow_enrollment(email, course_id)

enterprise/views.py

@@ -683,6 +683,15 @@ def _enroll_learner_in_course(
                         existing_enrollment.get('mode') == constants.CourseModes.AUDIT or
                         existing_enrollment.get('is_active') is False
                 ):
+                    if enterprise_customer.allow_enrollment_in_invite_only_courses:
+                        ensure_course_enrollment_is_allowed(course_id, request.user.email, enrollment_api_client)
+                        LOGGER.info(
+                            'User {user} is allowed to enroll in Course {course_id}.'.format(
+                                user=request.user.username,
+                                course_id=course_id
+                            )
+                        )
+
                     course_mode = get_best_mode_from_course_key(course_id)
                     LOGGER.info(
                         'Retrieved Course Mode: {course_modes} for Course {course_id}'.format(

tests/test_enterprise/api/test_views.py

@@ -4437,6 +4437,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")
@@ -4445,6 +4446,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,
@@ -4464,6 +4466,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)
         response = self.client.post(
@@ -4488,6 +4491,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')
@@ -4498,13 +4502,15 @@ def test_bulk_enrollment_in_bulk_courses_existing_users(
         mock_notify_task,
         mock_track_enroll,
         mock_get_course_mode,
+        mock_get_course_details,
     ):
         """
         Tests the bulk enrollment endpoint at enroll_learners_in_courses.
 
         This tests the case where existing users are supplied, so the enrollments are fulfilled rather than pending.
         """
         mock_update_or_create_enrollment.return_value = True
+        mock_get_course_details.return_value.__getitem__.return_value.invitation_only = False
 
         user_one = factories.UserFactory(is_active=True)
         user_two = factories.UserFactory(is_active=True)
@@ -4575,6 +4581,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')
@@ -4583,12 +4590,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(
@@ -4647,6 +4657,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'
@@ -4657,6 +4668,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,
     ):
@@ -4666,6 +4678,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()
 
@@ -4768,12 +4781,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,
     ):
@@ -4782,6 +4797,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'
@@ -4878,6 +4894,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")
@@ -4886,6 +4903,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,
@@ -4905,6 +4923,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)
 
@@ -4951,12 +4970,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"