diff --git a/openedx/features/enterprise_support/enrollments/tests/test_utils.py b/openedx/features/enterprise_support/enrollments/tests/test_utils.py index d0588b2f01..35a9331186 100644 --- a/openedx/features/enterprise_support/enrollments/tests/test_utils.py +++ b/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') diff --git a/openedx/features/enterprise_support/enrollments/utils.py b/openedx/features/enterprise_support/enrollments/utils.py index f44de3e2d4..1a8a105f4b 100644 --- a/openedx/features/enterprise_support/enrollments/utils.py +++ b/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, diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 62fb756778..58a103c543 100644 --- a/requirements/constraints.txt +++ b/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 diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 1d6a357b5f..f0a5c00b1e 100644 --- a/requirements/edx/base.txt +++ b/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 diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index b9d4d194b9..d38dfac599 100644 --- a/requirements/edx/development.txt +++ b/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 diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index d95033fe68..b27cca0f46 100644 --- a/requirements/edx/doc.txt +++ b/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 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 4d6c5defa8..18134e0ea5 100644 --- a/requirements/edx/testing.txt +++ b/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