Skip to content

Commit

Permalink
feat: force all enrollments (#2763)
Browse files Browse the repository at this point in the history
* feat: force all enrollments

* linting

* fix test
  • Loading branch information
asadali145 authored Sep 25, 2023
1 parent eef62a2 commit 3efb080
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 65 deletions.
15 changes: 3 additions & 12 deletions courses/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,8 @@
from requests.exceptions import ConnectionError as RequestsConnectionError, HTTPError

from courses.constants import ENROLL_CHANGE_STATUS_DEFERRED
from courses.models import (
CourseRun,
CourseRunEnrollment,
ProgramEnrollment,
)
from courseware.api import (
enroll_in_edx_course_runs,
unenroll_edx_course_run,
)
from courses.models import CourseRun, CourseRunEnrollment, ProgramEnrollment
from courseware.api import enroll_in_edx_course_runs, unenroll_edx_course_run
from courseware.exceptions import (
EdxApiEnrollErrorException,
EdxEnrollmentCreateError,
Expand Down Expand Up @@ -100,7 +93,6 @@ def create_run_enrollments(
keep_failed_enrollments=False,
order=None,
company=None,
force_enrollment=False,
): # pylint: disable=too-many-arguments
"""
Creates local records of a user's enrollment in course runs, and attempts to enroll them
Expand All @@ -122,7 +114,7 @@ def create_run_enrollments(
"""
successful_enrollments = []
try:
enroll_in_edx_course_runs(user, runs, force_enrollment=force_enrollment)
enroll_in_edx_course_runs(user, runs)
except (
EdxApiEnrollErrorException,
UnknownEdxApiEnrollException,
Expand Down Expand Up @@ -348,7 +340,6 @@ def defer_enrollment(
order=from_enrollment.order,
company=from_enrollment.company,
keep_failed_enrollments=keep_failed_enrollments,
force_enrollment=force,
)
if to_enrollments:
from_enrollment = deactivate_run_enrollment(
Expand Down
34 changes: 9 additions & 25 deletions courses/api_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,7 @@ def key_func(enrollment):


@pytest.mark.django_db
@pytest.mark.parametrize("force_enrollment", [True, False])
def test_create_run_enrollments(mocker, user, force_enrollment):
def test_create_run_enrollments(mocker, user):
"""
create_run_enrollments should call the edX API to create enrollments, create or reactivate local
enrollment records, and notify enrolled users via email
Expand All @@ -143,11 +142,9 @@ def test_create_run_enrollments(mocker, user, force_enrollment):
)

successful_enrollments, edx_request_success = create_run_enrollments(
user, runs, order=order, company=company, force_enrollment=force_enrollment
)
patched_edx_enroll.assert_called_once_with(
user, runs, force_enrollment=force_enrollment
user, runs, order=order, company=company
)
patched_edx_enroll.assert_called_once_with(user, runs)
assert patched_send_enrollment_email.call_count == num_runs
assert edx_request_success is True
assert len(successful_enrollments) == num_runs
Expand All @@ -165,8 +162,7 @@ def test_create_run_enrollments(mocker, user, force_enrollment):
"exception_cls",
[NoEdxApiAuthError, HTTPError, RequestsConnectionError, OpenEdXOAuth2Error],
)
@pytest.mark.parametrize("force_enrollment", [True, False])
def test_create_run_enrollments_api_fail(mocker, user, exception_cls, force_enrollment):
def test_create_run_enrollments_api_fail(mocker, user, exception_cls):
"""
create_run_enrollments should log a message and still create local enrollment records when certain exceptions
are raised if a flag is set to true
Expand All @@ -185,11 +181,8 @@ def test_create_run_enrollments_api_fail(mocker, user, exception_cls, force_enro
order=None,
company=None,
keep_failed_enrollments=True,
force_enrollment=force_enrollment,
)
patched_edx_enroll.assert_called_once_with(
user, [run], force_enrollment=force_enrollment
)
patched_edx_enroll.assert_called_once_with(user, [run])
patched_log_exception.assert_called_once()
patched_send_enrollment_email.assert_not_called()
assert len(successful_enrollments) == 1
Expand All @@ -198,7 +191,6 @@ def test_create_run_enrollments_api_fail(mocker, user, exception_cls, force_enro

@pytest.mark.django_db
@pytest.mark.parametrize("keep_failed_enrollments", [True, False])
@pytest.mark.parametrize("force_enrollment", [True, False])
@pytest.mark.parametrize(
"exception_cls,inner_exception",
[
Expand All @@ -210,7 +202,6 @@ def test_create_run_enrollments_enroll_api_fail(
mocker,
user,
keep_failed_enrollments,
force_enrollment,
exception_cls,
inner_exception,
): # pylint: disable=too-many-arguments
Expand Down Expand Up @@ -240,11 +231,8 @@ def test_create_run_enrollments_enroll_api_fail(
order=None,
company=None,
keep_failed_enrollments=keep_failed_enrollments,
force_enrollment=force_enrollment,
)
patched_edx_enroll.assert_called_once_with(
user, runs, force_enrollment=force_enrollment
)
patched_edx_enroll.assert_called_once_with(user, runs)
if keep_failed_enrollments:
patched_log_exception.assert_called_once()
else:
Expand All @@ -256,8 +244,7 @@ def test_create_run_enrollments_enroll_api_fail(


@pytest.mark.django_db
@pytest.mark.parametrize("force_enrollment", [True, False])
def test_create_run_enrollments_creation_fail(mocker, user, force_enrollment):
def test_create_run_enrollments_creation_fail(mocker, user):
"""
create_run_enrollments should log a message and send an admin email if there's an error during the
creation of local enrollment records
Expand All @@ -273,11 +260,9 @@ def test_create_run_enrollments_creation_fail(mocker, user, force_enrollment):
patched_mail_api = mocker.patch("courses.api.mail_api")

successful_enrollments, edx_request_success = create_run_enrollments(
user, runs, order=None, company=None, force_enrollment=force_enrollment
)
patched_edx_enroll.assert_called_once_with(
user, runs, force_enrollment=force_enrollment
user, runs, order=None, company=None
)
patched_edx_enroll.assert_called_once_with(user, runs)
patched_log_exception.assert_called_once()
patched_mail_api.send_course_run_enrollment_email.assert_not_called()
patched_mail_api.send_enrollment_failure_message.assert_called_once()
Expand Down Expand Up @@ -479,7 +464,6 @@ def test_defer_enrollment(mocker, keep_failed_enrollments, force_enrollment):
order=order,
company=company,
keep_failed_enrollments=keep_failed_enrollments,
force_enrollment=force_enrollment,
)
patched_deactivate_enrollments.assert_called_once_with(
existing_enrollment,
Expand Down
13 changes: 5 additions & 8 deletions courseware/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from django.conf import settings
from django.contrib.auth import get_user_model
from django.core.exceptions import ImproperlyConfigured
from django.db import transaction, IntegrityError
from django.db import IntegrityError, transaction
from django.shortcuts import reverse
from edx_api.client import EdxApi
from oauth2_provider.models import AccessToken, Application
Expand Down Expand Up @@ -616,13 +616,14 @@ def get_enrollment(user: User, course_run: CourseRun):
)


def enroll_in_edx_course_runs(user, course_runs, force_enrollment=False):
def enroll_in_edx_course_runs(user, course_runs, force_enrollment=True):
"""
Enrolls a user in edx course runs
Args:
user (users.models.User): The user to enroll
course_runs (iterable of CourseRun): The course runs to enroll in
force_enrollment (bool): Force enrollments in edX
Returns:
list of edx_api.enrollments.models.Enrollment:
Expand All @@ -632,12 +633,8 @@ def enroll_in_edx_course_runs(user, course_runs, force_enrollment=False):
EdxApiEnrollErrorException: Raised if the underlying edX API HTTP request fails
UnknownEdxApiEnrollException: Raised if an unknown error was encountered during the edX API request
"""
username = None
if force_enrollment:
edx_client = get_edx_api_service_client()
username = user.username
else:
edx_client = get_edx_api_client(user)
edx_client = get_edx_api_service_client()
username = user.username
results = []
for course_run in course_runs:
try:
Expand Down
34 changes: 14 additions & 20 deletions courseware/api_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@

from courses.factories import CourseRunEnrollmentFactory, CourseRunFactory
from courseware.api import (
OpenEdxUser,
ACCESS_TOKEN_HEADER_NAME,
OPENEDX_AUTH_DEFAULT_TTL_IN_SECONDS,
OpenEdxUser,
create_edx_auth_token,
create_edx_user,
create_user,
Expand Down Expand Up @@ -417,38 +417,31 @@ def test_get_edx_api_client(mocker, settings, user):
)


@pytest.mark.parametrize("force_enrollment", [True, False])
def test_enroll_in_edx_course_runs(mocker, user, force_enrollment):
def test_enroll_in_edx_course_runs(mocker, user):
"""Tests that enroll_in_edx_course_runs uses the EdxApi client to enroll in course runs"""
mock_client = mocker.MagicMock()
enroll_return_values = ["result1", "result2"]
mock_client.enrollments.create_student_enrollment = mocker.Mock(
side_effect=enroll_return_values
)
if force_enrollment:
mocker.patch(
"courseware.api.get_edx_api_service_client", return_value=mock_client
)
else:
mocker.patch("courseware.api.get_edx_api_client", return_value=mock_client)
mocker.patch("courseware.api.get_edx_api_service_client", return_value=mock_client)
course_runs = CourseRunFactory.build_batch(2)
enroll_results = enroll_in_edx_course_runs(
user,
course_runs,
force_enrollment=force_enrollment,
)
expected_username = user.username if force_enrollment else None
expected_username = user.username
mock_client.enrollments.create_student_enrollment.assert_any_call(
course_runs[0].courseware_id,
mode=EDX_ENROLLMENT_PRO_MODE,
username=expected_username,
force_enrollment=force_enrollment,
force_enrollment=True,
)
mock_client.enrollments.create_student_enrollment.assert_any_call(
course_runs[1].courseware_id,
mode=EDX_ENROLLMENT_PRO_MODE,
username=expected_username,
force_enrollment=force_enrollment,
force_enrollment=True,
)
assert enroll_results == enroll_return_values

Expand All @@ -463,22 +456,22 @@ def test_enroll_in_edx_course_runs_audit(mocker, user, error_text):
side_effect=[HTTPError(response=pro_enrollment_response), audit_result]
)
patched_log_error = mocker.patch("courseware.api.log.error")
mocker.patch("courseware.api.get_edx_api_client", return_value=mock_client)
mocker.patch("courseware.api.get_edx_api_service_client", return_value=mock_client)

course_run = CourseRunFactory.build()
results = enroll_in_edx_course_runs(user, [course_run])
assert mock_client.enrollments.create_student_enrollment.call_count == 2
mock_client.enrollments.create_student_enrollment.assert_any_call(
course_run.courseware_id,
mode=EDX_ENROLLMENT_PRO_MODE,
username=None,
force_enrollment=False,
username=user.username,
force_enrollment=True,
)
mock_client.enrollments.create_student_enrollment.assert_any_call(
course_run.courseware_id,
mode=EDX_ENROLLMENT_AUDIT_MODE,
username=None,
force_enrollment=False,
username=user.username,
force_enrollment=True,
)
assert results == [audit_result]
patched_log_error.assert_called_once()
Expand All @@ -494,14 +487,14 @@ def test_enroll_pro_api_fail(mocker, user):
mock_client.enrollments.create_student_enrollment = mocker.Mock(
side_effect=HTTPError(response=pro_enrollment_response)
)
mocker.patch("courseware.api.get_edx_api_client", return_value=mock_client)
mocker.patch("courseware.api.get_edx_api_service_client", return_value=mock_client)
course_run = CourseRunFactory.build()

with pytest.raises(EdxApiEnrollErrorException):
enroll_in_edx_course_runs(user, [course_run])


def test_enroll_pro_unknown_fail(mocker, user):
def test_enroll_pro_unknown_fail(mocker, user, settings):
"""
Tests that enroll_in_edx_course_runs raises an UnknownEdxApiEnrollException if an unexpected exception
is encountered
Expand All @@ -512,6 +505,7 @@ def test_enroll_pro_unknown_fail(mocker, user):
)
mocker.patch("courseware.api.get_edx_api_client", return_value=mock_client)
course_run = CourseRunFactory.build()
settings.OPENEDX_SERVICE_WORKER_API_TOKEN = "mock_api_token"

with pytest.raises(UnknownEdxApiEnrollException):
enroll_in_edx_course_runs(user, [course_run])
Expand Down

0 comments on commit 3efb080

Please sign in to comment.