Skip to content

Commit

Permalink
EnrollmentApiClient should call enrollment API with enterprise servic…
Browse files Browse the repository at this point in the history
…e user

EnrollmentApiClient should call enrollment API with enterprise service user instead of the user being enrolled manually to avoid 403 errors.

added unit test

added unit test to support changes

initialize enrollmentapi client with service user

Updated version and Changelog
  • Loading branch information
ziafazal committed Jan 21, 2020
1 parent 30a3a93 commit 90be408
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 6 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ Change Log

.. There should always be an "Unreleased" section for changes pending release.
[2.1.01] - 2020-01-21
---------------------

* Initialized EnrollmentApiClient with enterprise service worker user

[2.1.0] - 2020-01-16
--------------------

Expand Down
2 changes: 1 addition & 1 deletion enterprise/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@

from __future__ import absolute_import, unicode_literals

__version__ = "2.1.00"
__version__ = "2.1.01"

default_app_config = "enterprise.apps.EnterpriseConfig" # pylint: disable=invalid-name
4 changes: 3 additions & 1 deletion enterprise/admin/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
from enterprise.utils import (
get_configuration_value_for_site,
get_ecommerce_worker_user,
get_enterprise_worker_user,
send_email_notification_message,
track_enrollment,
)
Expand Down Expand Up @@ -443,7 +444,8 @@ def enroll_user(cls, enterprise_customer, user, course_mode, *course_ids):
enterprise_customer=enterprise_customer,
user_id=user.id
)
enrollment_client = EnrollmentApiClient(user)
worker_user = get_enterprise_worker_user()
enrollment_client = EnrollmentApiClient(worker_user)
succeeded = True
for course_id in course_ids:
try:
Expand Down
7 changes: 5 additions & 2 deletions enterprise/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
NotConnectedToOpenEdX,
get_configuration_value,
get_ecommerce_worker_user,
get_enterprise_worker_user,
)
from enterprise.validators import validate_image_extension, validate_image_size

Expand Down Expand Up @@ -734,7 +735,8 @@ def enroll(self, course_run_id, mode, cohort=None, source_slug=None):
"""
Enroll a user into a course track, and register an enterprise course enrollment.
"""
enrollment_api_client = EnrollmentApiClient(self.user)
worker_user = get_enterprise_worker_user()
enrollment_api_client = EnrollmentApiClient(worker_user)
# Check to see if the user's already enrolled and we have an enterprise course enrollment to track it.
course_enrollment = enrollment_api_client.get_course_enrollment(self.username, course_run_id) or {}
enrolled_in_course = course_enrollment and course_enrollment.get('is_active', False)
Expand Down Expand Up @@ -797,7 +799,8 @@ def unenroll(self, course_run_id):
"""
Unenroll a user from a course track.
"""
enrollment_api_client = EnrollmentApiClient(self.user)
worker_user = get_enterprise_worker_user()
enrollment_api_client = EnrollmentApiClient(worker_user)
if enrollment_api_client.unenroll_user_from_course(self.username, course_run_id):
EnterpriseCourseEnrollment.objects.filter(enterprise_customer_user=self, course_id=course_run_id).delete()
return True
Expand Down
8 changes: 7 additions & 1 deletion tests/test_admin/test_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,9 @@ def setUp(self):
"""
super(BaseTestEnterpriseCustomerManageLearnersView, self).setUp()
self.user = UserFactory.create(is_staff=True, is_active=True, id=1)
self.worker_user = UserFactory(
username=settings.ENTERPRISE_SERVICE_WORKER_USERNAME, is_staff=True, is_active=True
)
self.user.set_password("QWERTY")
self.user.save()
self.enterprise_customer = EnterpriseCustomerFactory()
Expand Down Expand Up @@ -634,7 +637,7 @@ def _enroll_user_request(self, user, mode, course_id="", program_id="", notify=T
@mock.patch("enterprise.admin.views.EcommerceApiClient")
@mock.patch("enterprise.admin.views.track_enrollment")
@mock.patch("enterprise.models.CourseCatalogApiClient")
@mock.patch("enterprise.admin.views.EnrollmentApiClient")
@mock.patch("enterprise.admin.views.EnrollmentApiClient", autospec=True)
@mock.patch("enterprise.admin.forms.EnrollmentApiClient")
@ddt.data(
(True, True),
Expand Down Expand Up @@ -680,6 +683,9 @@ def test_post_enroll_user(
ecommerce_api_client_mock.assert_not_called()
else:
ecommerce_api_client_mock.assert_called_once()
# Assert EnrollmentApiClient is initialized with enterprise worker user since enrollment API raise
# error if EnrollmentApiClient is initialized with same user who is going to be enrolled
views_client.assert_called_once_with(self.worker_user)
views_instance.enroll_user_in_course.assert_called_once_with(
user.username,
course_id,
Expand Down
24 changes: 23 additions & 1 deletion tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from pytest import mark, raises
from testfixtures import LogCapture

from django.conf import settings
from django.core.exceptions import ValidationError
from django.core.files import File
from django.core.files.storage import Storage
Expand Down Expand Up @@ -357,6 +358,15 @@ class TestEnterpriseCustomerUser(unittest.TestCase):
Tests of the EnterpriseCustomerUser model.
"""

def setUp(self):
"""
Test set up
"""
super(TestEnterpriseCustomerUser, self).setUp()
self.worker_user = factories.UserFactory(
username=settings.ENTERPRISE_SERVICE_WORKER_USERNAME, is_staff=True, is_active=True
)

@ddt.data(str, repr)
def test_string_conversion(self, method):
"""
Expand Down Expand Up @@ -425,7 +435,7 @@ def test_get_remote_id(self, provider_id, expected_value, called, mock_third_par
assert mock_third_party_api.return_value.get_remote_id.call_count == 0

@mock.patch('enterprise.utils.segment')
@mock.patch('enterprise.models.EnrollmentApiClient')
@mock.patch('enterprise.models.EnrollmentApiClient', autospec=True)
@mock.patch('enterprise.models.EnterpriseCustomerUser.create_order_for_enrollment')
@ddt.data('audit', 'verified')
def test_enroll_learner(self, course_mode, enrollment_order_mock, enrollment_api_client_mock, analytics_mock):
Expand All @@ -442,6 +452,18 @@ def test_enroll_learner(self, course_mode, enrollment_order_mock, enrollment_api
else:
enrollment_order_mock.assert_not_called()

enrollment_api_client_mock.assert_called_once_with(self.worker_user)

@mock.patch('enterprise.models.EnrollmentApiClient', autospec=True)
def test_unenroll_client_user(self, enrollment_api_client_mock):
"""
Verify that `EnrollmentApiClient` is created with correct user in `EnterpriseCustomerUser.unenroll`.
"""
enterprise_customer_user = factories.EnterpriseCustomerUserFactory()
enterprise_customer_user.unenroll('course-v1:edX+DemoX+Demo_Course')
enrollment_api_client_mock.return_value.unenroll_user_from_course.assert_called_once()
enrollment_api_client_mock.assert_called_once_with(self.worker_user)

@mock.patch('enterprise.models.EnrollmentApiClient')
@mock.patch('enterprise.models.EnterpriseCustomerUser.create_order_for_enrollment')
def test_enroll_learner_already_enrolled(self, enrollment_order_mock, enrollment_api_client_mock):
Expand Down

0 comments on commit 90be408

Please sign in to comment.