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

* fix: create CEA object when enrolling using a license flow
* test: verify that allow_enrollment is called_correctly
* fix: xmlsec issue
    xmlsec/python-xmlsec#314
* build: enable CI for pull requests
* style: fix some pycodestyle issues

Author: 0x29a

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

@@ -2407,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

@@ -4512,7 +4512,6 @@ def test_bulk_enrollment_in_bulk_courses_existing_users(
         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)
 
@@ -5001,6 +5000,7 @@ def test_bulk_enrollment_invitation_only(
                 },
             ]
         }
+
         def enroll():
             self.client.post(
                 settings.TEST_SERVER + reverse(

tests/test_enterprise/test_utils.py

@@ -539,10 +539,9 @@ def test_truncate_string(self):
         self.assertEqual(len(truncated_string), MAX_ALLOWED_TEXT_LENGTH)
 
     @ddt.data(True, False)
-    @mock.patch("enterprise.utils.CourseEnrollmentAllowed")
-    def test_ensure_course_enrollment_is_allowed(self, invite_only, mock_cea):
+    def test_ensure_course_enrollment_is_allowed(self, invite_only):
         """
-        Test that the CourseEnrollmentAllowed is created only for the "invite_only" courses.
+        Test that the enrollment allow endpoint is called for the "invite_only" courses.
         """
         self.create_user()
         mock_enrollment_api = mock.Mock()
@@ -551,9 +550,9 @@ def test_ensure_course_enrollment_is_allowed(self, invite_only, mock_cea):
         ensure_course_enrollment_is_allowed("test-course-id", self.user.email, mock_enrollment_api)
 
         if invite_only:
-            mock_cea.objects.update_or_create.assert_called_with(
-                course_id="test-course-id",
-                email=self.user.email
+            mock_enrollment_api.allow_enrollment.assert_called_with(
+                self.user.email,
+                "test-course-id",
             )
         else:
-            mock_cea.objects.update_or_create.assert_not_called()
+            mock_enrollment_api.allow_enrollment.assert_not_called()

tests/test_enterprise/views/test_course_enrollment_view.py

@@ -1623,10 +1623,8 @@ def test_post_course_specific_enrollment_view_premium_mode(
     @mock.patch('enterprise.views.EnrollmentApiClient')
     @mock.patch('enterprise.views.get_data_sharing_consent')
     @mock.patch('enterprise.utils.Registry')
-    @mock.patch('enterprise.utils.CourseEnrollmentAllowed')
     def test_post_course_specific_enrollment_view_invite_only_courses(
             self,
-            mock_cea,
             registry_mock,
             get_data_sharing_consent_mock,
             enrollment_api_client_mock,
@@ -1664,9 +1662,9 @@ def test_post_course_specific_enrollment_view_invite_only_courses(
             }
         )
 
-        mock_cea.objects.update_or_create.assert_called_with(
-            course_id=course_id,
-            email=self.user.email
+        enrollment_api_client_mock.return_value.allow_enrollment.assert_called_with(
+            self.user.email,
+            course_id,
         )
         assert response.status_code == 302
 

tests/test_enterprise/views/test_grant_data_sharing_permissions.py

@@ -90,6 +90,30 @@ def _assert_enterprise_linking_messages(self, response, user_is_active=True):
                 'You will not be able to log back into your account until you have activated it.'
             )
 
+    def _assert_allow_enrollment_is_called_correctly(
+        self,
+        mock_enrollment_api_client,
+        license_is_present,
+        course_invite_only,
+        enrollment_in_invite_only_courses_allowed,
+    ):
+        """
+        Verify that the allow_enrollment endpoint is called only when:
+            - License is present
+            - Course is invite only
+            - Enrollment in invite only courses is allowed
+        """
+        if license_is_present:
+            if course_invite_only:
+                if enrollment_in_invite_only_courses_allowed:
+                    mock_enrollment_api_client.return_value.allow_enrollment.assert_called_once()
+                else:
+                    mock_enrollment_api_client.return_value.allow_enrollment.assert_not_called()
+            else:
+                mock_enrollment_api_client.return_value.allow_enrollment.assert_not_called()
+        else:
+            mock_enrollment_api_client.return_value.allow_enrollment.assert_not_called()
+
     @mock.patch('enterprise.views.render', side_effect=fake_render)
     @mock.patch('enterprise.models.EnterpriseCatalogApiClient')
     @mock.patch('enterprise.api_client.discovery.CourseCatalogApiServiceClient')
@@ -398,12 +422,21 @@ def test_get_course_specific_consent_not_needed(
     @mock.patch('enterprise.views.get_best_mode_from_course_key')
     @mock.patch('enterprise.api_client.discovery.CourseCatalogApiServiceClient')
     @ddt.data(
-        str(uuid.uuid4()),
-        '',
+        (str(uuid.uuid4()), True, True),
+        (str(uuid.uuid4()), True, False),
+        (str(uuid.uuid4()), False, True),
+        (str(uuid.uuid4()), False, False),
+        ('', True, True),
+        ('', True, False),
+        ('', False, True),
+        ('', False, False),
     )
+    @ddt.unpack
     def test_get_course_specific_data_sharing_consent_not_enabled(
             self,
             license_uuid,
+            course_invite_only,
+            allow_enrollment_in_invite_only_courses,
             course_catalog_api_client_mock,
             mock_get_course_mode,
             mock_enrollment_api_client,
@@ -414,6 +447,7 @@ def test_get_course_specific_data_sharing_consent_not_enabled(
         enterprise_customer = EnterpriseCustomerFactory(
             name='Starfleet Academy',
             enable_data_sharing_consent=False,
+            allow_enrollment_in_invite_only_courses=allow_enrollment_in_invite_only_courses,
         )
         content_filter = {
             'key': [
@@ -432,6 +466,8 @@ def test_get_course_specific_data_sharing_consent_not_enabled(
         course_catalog_api_client_mock.return_value.program_exists.return_value = True
         course_catalog_api_client_mock.return_value.get_course_id.return_value = course_id
 
+        mock_enrollment_api_client.return_value.get_course_details.return_value = {"invite_only": course_invite_only}
+
         course_mode = 'verified'
         mock_get_course_mode.return_value = course_mode
         mock_enrollment_api_client.return_value.get_course_enrollment.return_value = {
@@ -467,6 +503,13 @@ def test_get_course_specific_data_sharing_consent_not_enabled(
         else:
             assert not mock_enrollment_api_client.return_value.enroll_user_in_course.called
 
+        self._assert_allow_enrollment_is_called_correctly(
+            mock_enrollment_api_client,
+            bool(license_uuid),
+            course_invite_only,
+            allow_enrollment_in_invite_only_courses
+        )
+
     @mock.patch('enterprise.views.render', side_effect=fake_render)
     @mock.patch('enterprise.views.get_best_mode_from_course_key')
     @mock.patch('enterprise.models.EnterpriseCatalogApiClient')