Merge pull request #32849 from openedx/ea/ent-7453

chore: remove lms_enroll_user_in_course

openedx/features/enterprise_support/enrollments/tests/test_utils.py

@@ -1,25 +1,24 @@
 """
 Test the enterprise support utils.
 """
-import ddt
 from unittest import mock
 from unittest.case import TestCase
 
 from django.core.exceptions import ObjectDoesNotExist
-from django.test import override_settings
 from opaque_keys.edx.keys import CourseKey
 
 from openedx.core.djangoapps.course_groups.cohorts import CourseUserGroup
-from openedx.core.djangoapps.enrollments.errors import CourseEnrollmentError, CourseEnrollmentExistsError
+from openedx.core.djangoapps.enrollments.errors import (
+    CourseEnrollmentError,
+    CourseEnrollmentExistsError,
+)
 from openedx.core.djangolib.testing.utils import skip_unless_lms
 from openedx.features.enterprise_support.enrollments.exceptions import (
     CourseIdMissingException,
     UserDoesNotExistException
 )
-from openedx.features.enterprise_support.enrollments.utils import (
-    lms_enroll_user_in_course,
-    lms_update_or_create_enrollment,
-)
+from openedx.features.enterprise_support.enrollments.utils import lms_update_or_create_enrollment
+
 COURSE_STRING = 'course-v1:OpenEdX+OutlineCourse+Run3'
 ENTERPRISE_UUID = 'enterprise_uuid'
 COURSE_ID = CourseKey.from_string(COURSE_STRING)
@@ -29,7 +28,6 @@ COURSE_MODE = 'verified'
 
 
 @skip_unless_lms
-@ddt.ddt
 class EnrollmentUtilsTest(TestCase):
     """
     Test enterprise support utils.
@@ -41,221 +39,97 @@ class EnrollmentUtilsTest(TestCase):
         self.a_user.id = USER_ID
         self.a_user.username = USERNAME
 
-    def run_test_with_setting(
-        self,
-        setting,
-        mock_update_create_enroll,
-        mock_enroll_user,
-        test_function_true,
-        test_function_false,
-    ):
-        """
-        Run a test with a setting.
-        """
-        with override_settings(
-            ENABLE_ENTERPRISE_BACKEND_EMET_AUTO_UPGRADE_ENROLLMENT_MODE=setting
-        ):
-            if setting:
-                return test_function_true(mock_update_create_enroll)
-            return test_function_false(mock_enroll_user)
-
-    @mock.patch('openedx.features.enterprise_support.enrollments.utils.lms_enroll_user_in_course')
-    @mock.patch('openedx.features.enterprise_support.enrollments.utils.lms_update_or_create_enrollment')
-    @ddt.data(True, False)
-    def test_validation_of_inputs_course_id(self, setting_value, mock_update_create_enroll, mock_enroll_user):
-        test_function_true = lambda mock_fn: lms_update_or_create_enrollment(
-            USERNAME, None, COURSE_MODE, is_active=True, enterprise_uuid=ENTERPRISE_UUID
-        )
-        test_function_false = lambda mock_fn: lms_enroll_user_in_course(USERNAME, None, COURSE_MODE, ENTERPRISE_UUID)
+    def test_validation_of_inputs_course_id(self):
         with self.assertRaises(CourseIdMissingException):
-            self.run_test_with_setting(
-                setting_value,
-                mock_update_create_enroll,
-                mock_enroll_user,
-                test_function_true,
-                test_function_false
+            lms_update_or_create_enrollment(
+                USERNAME, None, COURSE_MODE, is_active=True, enterprise_uuid=ENTERPRISE_UUID
             )
 
-    @mock.patch('openedx.features.enterprise_support.enrollments.utils.lms_enroll_user_in_course')
-    @mock.patch('openedx.features.enterprise_support.enrollments.utils.lms_update_or_create_enrollment')
-    @ddt.data(True, False)
-    def test_validation_of_inputs_user_not_provided(self, setting_value, mock_update_create_enroll, mock_enroll_user):
-        test_function_true = lambda mock_fn: lms_update_or_create_enrollment(
-            None, COURSE_ID, COURSE_MODE, is_active=True, enterprise_uuid=ENTERPRISE_UUID
-        )
-        test_function_false = lambda mock_fn: lms_enroll_user_in_course(None, COURSE_ID, COURSE_MODE, ENTERPRISE_UUID)
+    def test_validation_of_inputs_user_not_provided(self):
         with self.assertRaises(UserDoesNotExistException):
-            self.run_test_with_setting(
-                setting_value,
-                mock_update_create_enroll,
-                mock_enroll_user,
-                test_function_true,
-                test_function_false
+            lms_update_or_create_enrollment(
+                None, COURSE_ID, COURSE_MODE, is_active=True, enterprise_uuid=ENTERPRISE_UUID
             )
 
-    @mock.patch('openedx.features.enterprise_support.enrollments.utils.lms_enroll_user_in_course')
-    @mock.patch('openedx.features.enterprise_support.enrollments.utils.lms_update_or_create_enrollment')
     @mock.patch('openedx.features.enterprise_support.enrollments.utils.User.objects.get')
     @mock.patch('openedx.features.enterprise_support.enrollments.utils.transaction')
-    @ddt.data(True, False)
     def test_validation_of_inputs_user_not_found(
         self,
-        setting_value,
         mock_tx,
         mock_user_model,
-        mock_update_create_enroll,
-        mock_enroll_user
     ):
         mock_tx.return_value.atomic.side_effect = None
         mock_user_model.side_effect = ObjectDoesNotExist()
-        test_function_true = lambda mock_fn: lms_update_or_create_enrollment(
-            USERNAME, COURSE_ID, COURSE_MODE, is_active=True, enterprise_uuid=ENTERPRISE_UUID
-        )
-        test_function_false = lambda mock_fn: lms_enroll_user_in_course(
-            USERNAME,
-            COURSE_ID,
-            COURSE_MODE,
-            ENTERPRISE_UUID
-        )
         with self.assertRaises(UserDoesNotExistException):
-            self.run_test_with_setting(
-                setting_value,
-                mock_update_create_enroll,
-                mock_enroll_user,
-                test_function_true,
-                test_function_false
+            lms_update_or_create_enrollment(
+                USERNAME, COURSE_ID, COURSE_MODE, is_active=True, enterprise_uuid=ENTERPRISE_UUID
             )
 
-    @mock.patch('openedx.features.enterprise_support.enrollments.utils.lms_enroll_user_in_course')
-    @mock.patch('openedx.features.enterprise_support.enrollments.utils.lms_update_or_create_enrollment')
     @mock.patch('openedx.features.enterprise_support.enrollments.utils.enrollment_api.add_enrollment')
     @mock.patch('openedx.features.enterprise_support.enrollments.utils.enrollment_api.get_enrollment')
     @mock.patch('openedx.features.enterprise_support.enrollments.utils.User.objects.get')
     @mock.patch('openedx.features.enterprise_support.enrollments.utils.transaction')
-    @ddt.data(True, False)
     def test_course_enrollment_error_raises(
         self,
-        setting_value,
         mock_tx,
         mock_user_model,
         mock_get_enrollment_api,
         mock_add_enrollment_api,
-        mock_update_create_enroll,
-        mock_enroll_user
     ):
-        test_function_true = lambda mock_fn: lms_update_or_create_enrollment(
-            USERNAME, COURSE_ID, COURSE_MODE, is_active=True, enterprise_uuid=ENTERPRISE_UUID
-        )
-        test_function_false = lambda mock_fn: lms_enroll_user_in_course(
-            USERNAME,
-            COURSE_ID,
-            COURSE_MODE,
-            ENTERPRISE_UUID
-        )
-
         mock_add_enrollment_api.side_effect = CourseEnrollmentError("test")
         mock_tx.return_value.atomic.side_effect = None
 
         mock_user_model.return_value = self.a_user
-
-        enrollment_response = {'mode': COURSE_MODE, 'is_active': True} if not setting_value else None
-        mock_get_enrollment_api.return_value = enrollment_response
+        mock_get_enrollment_api.return_value = None
         with self.assertRaises(CourseEnrollmentError):
-            self.run_test_with_setting(
-                setting_value,
-                mock_update_create_enroll,
-                mock_enroll_user,
-                test_function_true,
-                test_function_false
+            lms_update_or_create_enrollment(
+                USERNAME, COURSE_ID, COURSE_MODE, is_active=True, enterprise_uuid=ENTERPRISE_UUID
             )
             mock_get_enrollment_api.assert_called_once_with(USERNAME, str(COURSE_ID))
 
-    @mock.patch('openedx.features.enterprise_support.enrollments.utils.lms_enroll_user_in_course')
-    @mock.patch('openedx.features.enterprise_support.enrollments.utils.lms_update_or_create_enrollment')
     @mock.patch('openedx.features.enterprise_support.enrollments.utils.enrollment_api.add_enrollment')
     @mock.patch('openedx.features.enterprise_support.enrollments.utils.enrollment_api.get_enrollment')
     @mock.patch('openedx.features.enterprise_support.enrollments.utils.User.objects.get')
     @mock.patch('openedx.features.enterprise_support.enrollments.utils.transaction')
-    @ddt.data(True, False)
     def test_course_group_error_raises(
         self,
-        setting_value,
         mock_tx,
         mock_user_model,
         mock_get_enrollment_api,
         mock_add_enrollment_api,
-        mock_update_create_enroll,
-        mock_enroll_user
     ):
-        test_function_true = lambda mock_fn: lms_update_or_create_enrollment(
-            USERNAME, COURSE_ID, COURSE_MODE, is_active=True, enterprise_uuid=ENTERPRISE_UUID
-        )
-        test_function_false = lambda mock_fn: lms_enroll_user_in_course(
-            USERNAME,
-            COURSE_ID,
-            COURSE_MODE,
-            ENTERPRISE_UUID
-        )
         mock_add_enrollment_api.side_effect = CourseUserGroup.DoesNotExist()
         mock_tx.return_value.atomic.side_effect = None
 
         mock_user_model.return_value = self.a_user
-        enrollment_response = {'mode': COURSE_MODE, 'is_active': True} if not setting_value else None
-        mock_get_enrollment_api.return_value = enrollment_response
+        mock_get_enrollment_api.return_value = None
         with self.assertRaises(CourseUserGroup.DoesNotExist):
-            self.run_test_with_setting(
-                setting_value,
-                mock_update_create_enroll,
-                mock_enroll_user,
-                test_function_true,
-                test_function_false
+            lms_update_or_create_enrollment(
+                USERNAME, COURSE_ID, COURSE_MODE, is_active=True, enterprise_uuid=ENTERPRISE_UUID
             )
         mock_get_enrollment_api.assert_called_once_with(USERNAME, str(COURSE_ID))
 
-    @mock.patch('openedx.features.enterprise_support.enrollments.utils.lms_enroll_user_in_course')
-    @mock.patch('openedx.features.enterprise_support.enrollments.utils.lms_update_or_create_enrollment')
     @mock.patch('openedx.features.enterprise_support.enrollments.utils.enrollment_api.add_enrollment')
     @mock.patch('openedx.features.enterprise_support.enrollments.utils.enrollment_api.get_enrollment')
     @mock.patch('openedx.features.enterprise_support.enrollments.utils.User.objects.get')
     @mock.patch('openedx.features.enterprise_support.enrollments.utils.transaction')
-    @ddt.data(True, False)
     def test_calls_enrollment_and_cohort_apis(
         self,
-        setting,
         mock_tx,
         mock_user_model,
         mock_get_enrollment_api,
         mock_add_enrollment_api,
-        mock_update_create_enroll,
-        mock_enroll_user,
     ):
-        test_function_true = lambda mock_fn: lms_update_or_create_enrollment(
-            USERNAME, COURSE_ID, COURSE_MODE, is_active=True, enterprise_uuid=ENTERPRISE_UUID
-        )
-        test_function_false = lambda mock_fn: lms_enroll_user_in_course(
-            USERNAME,
-            COURSE_ID,
-            COURSE_MODE,
-            ENTERPRISE_UUID
-        )
         expected_response = {'mode': COURSE_MODE, 'is_active': True}
-        enrollment_response = {'mode': COURSE_MODE, 'is_active': True}
 
         mock_add_enrollment_api.return_value = expected_response
         mock_tx.return_value.atomic.side_effect = None
 
         mock_user_model.return_value = self.a_user
+        mock_get_enrollment_api.return_value = None
 
-        if setting:
-            mock_get_enrollment_api.return_value = None
-        else:
-            mock_get_enrollment_api.return_value = enrollment_response
-        response = self.run_test_with_setting(
-            setting,
-            mock_update_create_enroll,
-            mock_enroll_user,
-            test_function_true,
-            test_function_false
+        response = lms_update_or_create_enrollment(
+            USERNAME, COURSE_ID, COURSE_MODE, is_active=True, enterprise_uuid=ENTERPRISE_UUID
         )
         assert response == expected_response
         mock_add_enrollment_api.assert_called_once_with(
@@ -268,32 +142,17 @@ class EnrollmentUtilsTest(TestCase):
         )
         mock_get_enrollment_api.assert_called_once_with(USERNAME, str(COURSE_ID))
 
-    @mock.patch('openedx.features.enterprise_support.enrollments.utils.lms_enroll_user_in_course')
-    @mock.patch('openedx.features.enterprise_support.enrollments.utils.lms_update_or_create_enrollment')
     @mock.patch('openedx.features.enterprise_support.enrollments.utils.enrollment_api.add_enrollment')
     @mock.patch('openedx.features.enterprise_support.enrollments.utils.enrollment_api.get_enrollment')
     @mock.patch('openedx.features.enterprise_support.enrollments.utils.User.objects.get')
     @mock.patch('openedx.features.enterprise_support.enrollments.utils.transaction')
-    @ddt.data(True, False)
     def test_existing_enrollment_does_not_fail(
         self,
-        setting_value,
         mock_tx,
         mock_user_model,
         mock_get_enrollment_api,
         mock_add_enrollment_api,
-        mock_update_create_enroll,
-        mock_enroll_user,
     ):
-        test_function_true = lambda mock_fn: lms_update_or_create_enrollment(
-            USERNAME, COURSE_ID, COURSE_MODE, is_active=True, enterprise_uuid=ENTERPRISE_UUID
-        )
-        test_function_false = lambda mock_fn: lms_enroll_user_in_course(
-            USERNAME,
-            COURSE_ID,
-            COURSE_MODE,
-            ENTERPRISE_UUID
-        )
         expected_response = {'mode': COURSE_MODE, 'is_active': True}
         enrollment_response = {'mode': COURSE_MODE, 'is_active': True}
 
@@ -301,29 +160,13 @@ class EnrollmentUtilsTest(TestCase):
         mock_tx.return_value.atomic.side_effect = None
 
         mock_get_enrollment_api.return_value = enrollment_response
-
         mock_user_model.return_value = self.a_user
 
-        response = self.run_test_with_setting(
-            setting_value,
-            mock_update_create_enroll,
-            mock_enroll_user,
-            test_function_true,
-            test_function_false
+        response = lms_update_or_create_enrollment(
+            USERNAME, COURSE_ID, COURSE_MODE, is_active=True, enterprise_uuid=ENTERPRISE_UUID
         )
-        if setting_value:
-            mock_add_enrollment_api.assert_not_called()
-            assert response == expected_response
-        else:
-            mock_add_enrollment_api.assert_called_once_with(
-                USERNAME,
-                str(COURSE_ID),
-                mode=COURSE_MODE,
-                is_active=True,
-                enrollment_attributes=None,
-                enterprise_uuid=ENTERPRISE_UUID,
-            )
-            assert response is None
+        mock_add_enrollment_api.assert_not_called()
+        assert response == expected_response
         mock_get_enrollment_api.assert_called_once()
 
     @mock.patch('openedx.features.enterprise_support.enrollments.utils.enrollment_api.update_enrollment')

openedx/features/enterprise_support/enrollments/utils.py

@@ -22,51 +22,6 @@ from openedx.features.enterprise_support.enrollments.exceptions import (
 log = logging.getLogger(__name__)
 
 
-def lms_enroll_user_in_course(
-    username,
-    course_id,
-    mode,
-    enterprise_uuid,
-    is_active=True,
-):
-    """
-    Temporarily keeping the original enrollment function to help with deployment
-    """
-    user = _validate_enrollment_inputs(username, course_id)
-
-    with transaction.atomic():
-        try:
-            response = enrollment_api.add_enrollment(
-                username,
-                str(course_id),
-                mode=mode,
-                is_active=is_active,
-                enrollment_attributes=None,
-                enterprise_uuid=enterprise_uuid,
-            )
-            log.info('The user [%s] has been enrolled in course run [%s].', username, course_id)
-            return response
-        except CourseEnrollmentExistsError as error:  # pylint: disable=unused-variable
-            log.warning('An enrollment already exists for user [%s] in course run [%s].', username, course_id)
-            return None
-        except CourseEnrollmentError as error:
-            log.exception("An error occurred while creating the new course enrollment for user "
-                          "[%s] in course run [%s]", username, course_id)
-            raise error
-        finally:
-            # Assumes that the ecommerce service uses an API key to authenticate.
-            current_enrollment = enrollment_api.get_enrollment(username, str(course_id))
-            audit_log(
-                'enrollment_change_requested',
-                course_id=str(course_id),
-                requested_mode=mode,
-                actual_mode=current_enrollment['mode'] if current_enrollment else None,
-                requested_activation=is_active,
-                actual_activation=current_enrollment['is_active'] if current_enrollment else None,
-                user_id=user.id
-            )
-
-
 def lms_update_or_create_enrollment(
     username,
     course_id,

requirements/constraints.txt

@@ -27,7 +27,7 @@ django-storages==1.13.2
 # The team that owns this package will manually bump this package rather than having it pulled in automatically.
 # This is to allow them to better control its deployment and to do it in a process that works better
 # for them.
-edx-enterprise==4.3.0
+edx-enterprise==4.3.1
 
 # 1. django-oauth-toolkit version >=2.0.0 has breaking changes. More details
 #    mentioned on this issue https://github.com/openedx/edx-platform/issues/32884

requirements/edx/base.txt

@@ -486,7 +486,7 @@ edx-drf-extensions==8.10.0
     #   edx-when
     #   edxval
     #   openedx-learning
-edx-enterprise==4.3.0
+edx-enterprise==4.3.1
     # via
     #   -c requirements/edx/../constraints.txt
     #   -r requirements/edx/kernel.in

requirements/edx/development.txt

@@ -754,7 +754,7 @@ edx-drf-extensions==8.10.0
     #   edx-when
     #   edxval
     #   openedx-learning
-edx-enterprise==4.3.0
+edx-enterprise==4.3.1
     # via
     #   -c requirements/edx/../constraints.txt
     #   -r requirements/edx/doc.txt

requirements/edx/doc.txt

@@ -560,7 +560,7 @@ edx-drf-extensions==8.10.0
     #   edx-when
     #   edxval
     #   openedx-learning
-edx-enterprise==4.3.0
+edx-enterprise==4.3.1
     # via
     #   -c requirements/edx/../constraints.txt
     #   -r requirements/edx/base.txt

requirements/edx/testing.txt

@@ -590,7 +590,7 @@ edx-drf-extensions==8.10.0
     #   edx-when
     #   edxval
     #   openedx-learning
-edx-enterprise==4.3.0
+edx-enterprise==4.3.1
     # via
     #   -c requirements/edx/../constraints.txt
     #   -r requirements/edx/base.txt