From 7a038330869c33e9eaa6d7a2f6730d08d48a4e47 Mon Sep 17 00:00:00 2001 From: 0x29a Date: Fri, 19 Apr 2024 15:59:09 +0200 Subject: [PATCH 1/3] fix: create CEA object when enrolling using a license flow --- enterprise/api_client/lms.py | 29 +++++++++++++++++++ enterprise/utils.py | 11 ++----- enterprise/views.py | 9 ++++++ tests/test_enterprise/test_utils.py | 11 ++++--- .../views/test_course_enrollment_view.py | 6 ++-- 5 files changed, 48 insertions(+), 18 deletions(-) diff --git a/enterprise/api_client/lms.py b/enterprise/api_client/lms.py index cb06742e69..3809185715 100644 --- a/enterprise/api_client/lms.py +++ b/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, @@ -284,6 +285,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): """ diff --git a/enterprise/utils.py b/enterprise/utils.py index 50445a49fb..75288a7fcf 100644 --- a/enterprise/utils.py +++ b/enterprise/utils.py @@ -2339,19 +2339,14 @@ def hide_price_when_zero(enterprise_customer, course_modes): 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) diff --git a/enterprise/views.py b/enterprise/views.py index 0fa0f7f97e..b33a429c6d 100644 --- a/enterprise/views.py +++ b/enterprise/views.py @@ -684,6 +684,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( diff --git a/tests/test_enterprise/test_utils.py b/tests/test_enterprise/test_utils.py index 2b3ef75a4e..0cf5e1c3da 100644 --- a/tests/test_enterprise/test_utils.py +++ b/tests/test_enterprise/test_utils.py @@ -523,10 +523,9 @@ def test_hide_course_price_when_zero(self, hide_price): self.assertEqual(non_zero_modes, processed_non_zero_modes) @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() @@ -535,9 +534,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( + mock_enrollment_api.return_value.allow_enrollment.assert_called_with( + email=self.user.email, course_id="test-course-id", - email=self.user.email ) else: - mock_cea.objects.update_or_create.assert_not_called() + mock_enrollment_api.return_value.allow_enrollment.assert_not_called() diff --git a/tests/test_enterprise/views/test_course_enrollment_view.py b/tests/test_enterprise/views/test_course_enrollment_view.py index 8ed1819d5a..ae2881245f 100644 --- a/tests/test_enterprise/views/test_course_enrollment_view.py +++ b/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( + enrollment_api_client_mock.return_value.allow_enrollment.assert_called_with( + email=self.user.email, course_id=course_id, - email=self.user.email ) assert response.status_code == 302 From 0b19535b9247b8502a2d5cd71cc3a95c880a4684 Mon Sep 17 00:00:00 2001 From: 0x29a Date: Mon, 22 Apr 2024 19:17:32 +0200 Subject: [PATCH 2/3] test: verify that allow_enrollment is called_correctly --- tests/test_enterprise/test_utils.py | 8 ++-- .../views/test_course_enrollment_view.py | 4 +- .../test_grant_data_sharing_permissions.py | 47 ++++++++++++++++++- 3 files changed, 51 insertions(+), 8 deletions(-) diff --git a/tests/test_enterprise/test_utils.py b/tests/test_enterprise/test_utils.py index 0cf5e1c3da..4fd37cccaa 100644 --- a/tests/test_enterprise/test_utils.py +++ b/tests/test_enterprise/test_utils.py @@ -534,9 +534,9 @@ def test_ensure_course_enrollment_is_allowed(self, invite_only): ensure_course_enrollment_is_allowed("test-course-id", self.user.email, mock_enrollment_api) if invite_only: - mock_enrollment_api.return_value.allow_enrollment.assert_called_with( - email=self.user.email, - course_id="test-course-id", + mock_enrollment_api.allow_enrollment.assert_called_with( + self.user.email, + "test-course-id", ) else: - mock_enrollment_api.return_value.allow_enrollment.assert_not_called() + mock_enrollment_api.allow_enrollment.assert_not_called() diff --git a/tests/test_enterprise/views/test_course_enrollment_view.py b/tests/test_enterprise/views/test_course_enrollment_view.py index ae2881245f..5ff637f2df 100644 --- a/tests/test_enterprise/views/test_course_enrollment_view.py +++ b/tests/test_enterprise/views/test_course_enrollment_view.py @@ -1663,8 +1663,8 @@ def test_post_course_specific_enrollment_view_invite_only_courses( ) enrollment_api_client_mock.return_value.allow_enrollment.assert_called_with( - email=self.user.email, - course_id=course_id, + self.user.email, + course_id, ) assert response.status_code == 302 diff --git a/tests/test_enterprise/views/test_grant_data_sharing_permissions.py b/tests/test_enterprise/views/test_grant_data_sharing_permissions.py index 642a9bcd8e..bbb339b01f 100644 --- a/tests/test_enterprise/views/test_grant_data_sharing_permissions.py +++ b/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') From f08ea923cab0f544810e0296bd6e9f7a62cd277d Mon Sep 17 00:00:00 2001 From: 0x29a Date: Mon, 22 Apr 2024 19:19:41 +0200 Subject: [PATCH 3/3] fix: xmlsec issue https://github.com/xmlsec/python-xmlsec/issues/314 --- .github/workflows/mysql8-migrations.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/mysql8-migrations.yml b/.github/workflows/mysql8-migrations.yml index 25136b8c9f..722bd95e28 100644 --- a/.github/workflows/mysql8-migrations.yml +++ b/.github/workflows/mysql8-migrations.yml @@ -60,7 +60,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 - name: Initiate Services run: | sudo /etc/init.d/mysql start