Skip to content

Commit

Permalink
Merge pull request #682 from edx/ziafazal/ENT-2543
Browse files Browse the repository at this point in the history
EnrollmentApiClient should call enrollment API with enterprise service user
  • Loading branch information
ziafazal authored Jan 21, 2020
2 parents 30a3a93 + 90be408 commit 2c12bb9
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 2c12bb9

Please sign in to comment.