Skip to content

Commit

Permalink
fix: create CEA object when enrolling using a license flow [PALM] (#19)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
0x29a authored May 8, 2024
1 parent 30e5702 commit 8281539
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 23 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/mysql8-migrations.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 29 additions & 0 deletions enterprise/api_client/lms.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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):
"""
Expand Down
11 changes: 3 additions & 8 deletions enterprise/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
9 changes: 9 additions & 0 deletions enterprise/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
13 changes: 6 additions & 7 deletions tests/test_enterprise/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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(
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()
8 changes: 3 additions & 5 deletions tests/test_enterprise/views/test_course_enrollment_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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

Expand Down
47 changes: 45 additions & 2 deletions tests/test_enterprise/views/test_grant_data_sharing_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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,
Expand All @@ -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': [
Expand All @@ -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 = {
Expand Down Expand Up @@ -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')
Expand Down

0 comments on commit 8281539

Please sign in to comment.