From 8c362d4df6180464f2d17fddebfe95e48b11e1f1 Mon Sep 17 00:00:00 2001
From: Brittney Exline <bexline@edx.org>
Date: Mon, 17 Jul 2017 11:44:53 -0400
Subject: [PATCH] Revert "Create EnterpriseCourseEnrollment when enrolling via
Track Selection page"
This reverts commit d940bbfd09db927f634c25ead226e965a5a8a910.
---
.../course_modes/tests/test_views.py | 89 -------------------
common/djangoapps/course_modes/views.py | 58 ++----------
openedx/features/enterprise_support/api.py | 35 +-------
.../tests/mixins/enterprise.py | 12 ---
.../enterprise_support/tests/test_api.py | 23 -----
5 files changed, 9 insertions(+), 208 deletions(-)
diff --git a/common/djangoapps/course_modes/tests/test_views.py b/common/djangoapps/course_modes/tests/test_views.py
index 7ff5ed448f3..7f7e3a7b76d 100644
--- a/common/djangoapps/course_modes/tests/test_views.py
+++ b/common/djangoapps/course_modes/tests/test_views.py
@@ -236,52 +236,6 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest
self.assertContains(response, 'Pursue a Verified Certificate')
self.assertContains(response, 'Audit This Course')
- @httpretty.activate
- @patch('course_modes.views.get_enterprise_consent_url')
- @ddt.data(
- (True, True),
- (True, False),
- (False, True),
- (False, False),
- )
- @ddt.unpack
- def test_enterprise_course_enrollment_creation(
- self,
- enterprise_enrollment_exists,
- course_in_catalog,
- get_consent_url_mock,
- ):
- for mode in ('audit', 'honor', 'verified'):
- CourseModeFactory.create(mode_slug=mode, course_id=self.course.id)
-
- catalog_integration = self.create_catalog_integration()
- UserFactory(username=catalog_integration.service_username)
-
- courses_in_catalog = [str(self.course.id)] if course_in_catalog else []
- enterprise_enrollment = {'course_id': str(self.course.id)} if enterprise_enrollment_exists else {}
-
- self.mock_course_discovery_api_for_catalog_contains(
- catalog_id=1, course_run_ids=courses_in_catalog
- )
- self.mock_enterprise_course_enrollment_get_api(**enterprise_enrollment)
- self.mock_enterprise_course_enrollment_post_api()
- self.mock_enterprise_learner_api(enable_audit_enrollment=True)
-
- get_consent_url_mock.return_value = 'http://appropriate-consent-url.com/'
-
- url = reverse('course_modes_choose', args=[unicode(self.course.id)])
-
- response = self.client.post(url, self.POST_PARAMS_FOR_COURSE_MODE['audit'])
-
- final_url = reverse('dashboard') if not course_in_catalog else 'http://appropriate-consent-url.com/'
-
- self.assertRedirects(response, final_url, fetch_redirect_response=False)
- if course_in_catalog:
- if enterprise_enrollment_exists:
- self.assertEquals(httpretty.last_request().method, 'GET')
- else:
- self.assertEquals(httpretty.last_request().method, 'POST')
-
@httpretty.activate
@ddt.data(
'',
@@ -380,7 +334,6 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest
'unsupported': {'unsupported_mode': True},
}
- @httpretty.activate
@ddt.data(
('audit', 'dashboard'),
('honor', 'dashboard'),
@@ -388,8 +341,6 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest
)
@ddt.unpack
def test_choose_mode_redirect(self, course_mode, expected_redirect):
- self.mock_enterprise_learner_api()
- self.mock_enterprise_course_enrollment_get_api()
# Create the course modes
for mode in ('audit', 'honor', 'verified'):
min_price = 0 if mode in ["honor", "audit"] else 1
@@ -412,38 +363,7 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest
self.assertRedirects(response, redirect_url)
- @httpretty.activate
- def test_choose_mode_audit_enroll_on_get(self):
- """
- Confirms that the learner will be enrolled in Audit track if it is the only possible option
- """
- self.mock_enterprise_learner_api()
- self.mock_enterprise_course_enrollment_get_api()
- # Create the course mode
- audit_mode = 'audit'
- CourseModeFactory.create(mode_slug=audit_mode, course_id=self.course.id, min_price=0)
-
- # Assert learner is not enrolled in Audit track pre-POST
- mode, is_active = CourseEnrollment.enrollment_mode_for_user(self.user, self.course.id)
- self.assertIsNone(mode)
- self.assertIsNone(is_active)
-
- # Choose the audit mode (POST request)
- choose_track_url = reverse('course_modes_choose', args=[unicode(self.course.id)])
- response = self.client.get(choose_track_url)
-
- # Assert learner is enrolled in Audit track and sent to the dashboard
- mode, is_active = CourseEnrollment.enrollment_mode_for_user(self.user, self.course.id)
- self.assertEquals(mode, audit_mode)
- self.assertTrue(is_active)
-
- redirect_url = reverse('dashboard')
- self.assertRedirects(response, redirect_url)
-
- @httpretty.activate
def test_choose_mode_audit_enroll_on_post(self):
- self.mock_enterprise_learner_api()
- self.mock_enterprise_course_enrollment_get_api()
audit_mode = 'audit'
# Create the course modes
for mode in (audit_mode, 'verified'):
@@ -478,10 +398,7 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest
self.assertEqual(mode, audit_mode)
self.assertTrue(is_active)
- @httpretty.activate
def test_remember_donation_for_course(self):
- self.mock_enterprise_learner_api()
- self.mock_enterprise_course_enrollment_get_api()
# Create the course modes
CourseModeFactory.create(mode_slug='honor', course_id=self.course.id)
CourseModeFactory.create(mode_slug='verified', course_id=self.course.id, min_price=1)
@@ -498,10 +415,7 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest
expected_amount = decimal.Decimal(self.POST_PARAMS_FOR_COURSE_MODE['verified']['contribution'])
self.assertEqual(actual_amount, expected_amount)
- @httpretty.activate
def test_successful_default_enrollment(self):
- self.mock_enterprise_learner_api()
- self.mock_enterprise_course_enrollment_get_api()
# Create the course modes
for mode in (CourseMode.DEFAULT_MODE_SLUG, 'verified'):
CourseModeFactory.create(mode_slug=mode, course_id=self.course.id)
@@ -523,10 +437,7 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest
self.assertEqual(mode, CourseMode.DEFAULT_MODE_SLUG)
self.assertEqual(is_active, True)
- @httpretty.activate
def test_unsupported_enrollment_mode_failure(self):
- self.mock_enterprise_learner_api()
- self.mock_enterprise_course_enrollment_get_api()
# Create the supported course modes
for mode in ('honor', 'verified'):
CourseModeFactory.create(mode_slug=mode, course_id=self.course.id)
diff --git a/common/djangoapps/course_modes/views.py b/common/djangoapps/course_modes/views.py
index 11043417438..f034b060fc6 100644
--- a/common/djangoapps/course_modes/views.py
+++ b/common/djangoapps/course_modes/views.py
@@ -25,7 +25,6 @@ from edxmako.shortcuts import render_to_response
from lms.djangoapps.commerce.utils import EcommerceService
from openedx.core.djangoapps.embargo import api as embargo_api
from openedx.features.enterprise_support import api as enterprise_api
-from openedx.features.enterprise_support.api import get_enterprise_consent_url
from student.models import CourseEnrollment
from third_party_auth.decorators import tpa_hint_ends_existing_session
from util import organizations_helpers as organization_api
@@ -108,16 +107,6 @@ class ChooseModeView(View):
# If there isn't a verified mode available, then there's nothing
# to do on this page. Send the user to the dashboard.
if not CourseMode.has_verified_mode(modes):
- # If the learner has arrived at this screen via the traditional enrollment workflow,
- # then they should already be enrolled in an audit mode for the course, assuming one has
- # been configured. However, alternative enrollment workflows have been introduced into the
- # system, such as third-party discovery. These workflows result in learners arriving
- # directly at this screen, and they will not necessarily be pre-enrolled in the audit mode.
- # In this particular case, Audit is the ONLY option available, and thus we need to ensure
- # that the learner is truly enrolled before we redirect them away to the dashboard.
- if len(modes) == 1 and modes.get(CourseMode.AUDIT):
- CourseEnrollment.enroll(request.user, course_key, CourseMode.AUDIT)
- return redirect(self._get_redirect_url_for_audit_enrollment(request, course_id))
return redirect(reverse('dashboard'))
# If a user has already paid, redirect them to the dashboard.
@@ -252,14 +241,19 @@ class ChooseModeView(View):
allowed_modes = CourseMode.modes_for_course_dict(course_key)
if requested_mode not in allowed_modes:
return HttpResponseBadRequest(_("Enrollment mode not supported"))
- if requested_mode in CourseMode.AUDIT_MODES:
+
+ if requested_mode == 'audit':
# If the learner has arrived at this screen via the traditional enrollment workflow,
# then they should already be enrolled in an audit mode for the course, assuming one has
# been configured. However, alternative enrollment workflows have been introduced into the
# system, such as third-party discovery. These workflows result in learners arriving
# directly at this screen, and they will not necessarily be pre-enrolled in the audit mode.
+ CourseEnrollment.enroll(request.user, course_key, CourseMode.AUDIT)
+ return redirect(reverse('dashboard'))
+
+ if requested_mode == 'honor':
CourseEnrollment.enroll(user, course_key, mode=requested_mode)
- return redirect(self._get_redirect_url_for_audit_enrollment(request, course_id))
+ return redirect(reverse('dashboard'))
mode_info = allowed_modes[requested_mode]
@@ -290,44 +284,6 @@ class ChooseModeView(View):
)
)
- def _get_redirect_url_for_audit_enrollment(self, request, course_id):
- """
- After a user has been enrolled in a course in an audit mode, determine the appropriate location
- to which they ought to be redirected, bearing in mind enterprise data sharing consent considerations.
- """
- enterprise_learner_data = enterprise_api.get_enterprise_learner_data(site=request.site, user=request.user)
-
- if enterprise_learner_data:
- enterprise_learner = enterprise_learner_data[0]
- # If we have an enterprise learner, check to see if the current course is in the enterprise's catalog.
- is_course_in_enterprise_catalog = enterprise_api.is_course_in_enterprise_catalog(
- site=request.site,
- course_id=course_id,
- enterprise_catalog_id=enterprise_learner['enterprise_customer']['catalog']
- )
- # If the course is in the catalog, check for an existing Enterprise enrollment
- if is_course_in_enterprise_catalog:
- client = enterprise_api.EnterpriseApiClient()
- if not client.get_enterprise_course_enrollment(enterprise_learner['id'], course_id):
- # If there's no existing Enterprise enrollment, create one.
- client.post_enterprise_course_enrollment(request.user.username, course_id, None)
- # Check if consent is required, and generate a redirect URL to the
- # consent service if so; this function returns None if consent
- # is not required or has already been granted.
- consent_url = get_enterprise_consent_url(
- request,
- course_id,
- user=request.user,
- return_to='dashboard',
- course_specific_return=False,
- )
- # If we got a redirect URL for consent, go there.
- if consent_url:
- return consent_url
-
- # If the enrollment isn't Enterprise-linked, or if consent isn't necessary, go to the Dashboard.
- return reverse('dashboard')
-
def _get_requested_mode(self, request_dict):
"""Get the user's requested mode
diff --git a/openedx/features/enterprise_support/api.py b/openedx/features/enterprise_support/api.py
index db58ef216d5..a3bffe22ec1 100644
--- a/openedx/features/enterprise_support/api.py
+++ b/openedx/features/enterprise_support/api.py
@@ -58,32 +58,6 @@ class EnterpriseApiClient(object):
jwt=jwt
)
- def get_enterprise_course_enrollment(self, ec_user_id, course_id):
- """
- Check for an EnterpriseCourseEnrollment linking a particular EnterpriseCustomerUser to a particular course.
- """
- params = {
- 'enterprise_customer_user': ec_user_id,
- 'course_id': course_id,
- }
- try:
- response = getattr(self.client, 'enterprise-course-enrollment').get(**params)
- except (HttpClientError, HttpServerError):
- message = (
- "An error occured while getting EnterpriseCourseEnrollment for EnterpriseCustomerUser with "
- "ID {ec_user_id} and course run {course_id}."
- ).format(
- username=username,
- course_id=course_id,
- )
- LOGGER.exception(message)
- raise EnterpriseApiException(message)
- else:
- if response.get('results'):
- return response['results'][0]
- else:
- return None
-
def post_enterprise_course_enrollment(self, username, course_id, consent_granted):
"""
Create an EnterpriseCourseEnrollment by using the corresponding serializer (for validation).
@@ -294,7 +268,7 @@ def consent_needed_for_course(user, course_id):
return consent_necessary_for_course(user, course_id)
-def get_enterprise_consent_url(request, course_id, user=None, return_to=None, course_specific_return=True):
+def get_enterprise_consent_url(request, course_id, user=None, return_to=None):
"""
Build a URL to redirect the user to the Enterprise app to provide data sharing
consent for a specific course ID.
@@ -312,15 +286,10 @@ def get_enterprise_consent_url(request, course_id, user=None, return_to=None, co
if not consent_needed_for_course(user, course_id):
return None
- if course_specific_return:
- reverse_args = (course_id,)
- else:
- reverse_args = tuple()
-
if return_to is None:
return_path = request.path
else:
- return_path = reverse(return_to, args=reverse_args)
+ return_path = reverse(return_to, args=(course_id,))
url_params = {
'course_id': course_id,
diff --git a/openedx/features/enterprise_support/tests/mixins/enterprise.py b/openedx/features/enterprise_support/tests/mixins/enterprise.py
index f4ce420e1e9..5415d73e28b 100644
--- a/openedx/features/enterprise_support/tests/mixins/enterprise.py
+++ b/openedx/features/enterprise_support/tests/mixins/enterprise.py
@@ -57,18 +57,6 @@ class EnterpriseServiceMockMixin(object):
status=500
)
- def mock_enterprise_course_enrollment_get_api(self, **kwargs):
- result = {
- 'results': [kwargs] if kwargs else []
- }
- httpretty.register_uri(
- method=httpretty.GET,
- uri=self.get_enterprise_url('enterprise-course-enrollment'),
- body=json.dumps(result),
- content_type='application/json',
- status=200
- )
-
def mock_enterprise_learner_api(
self,
catalog_id=1,
diff --git a/openedx/features/enterprise_support/tests/test_api.py b/openedx/features/enterprise_support/tests/test_api.py
index 2743222559d..052f7f434cf 100644
--- a/openedx/features/enterprise_support/tests/test_api.py
+++ b/openedx/features/enterprise_support/tests/test_api.py
@@ -194,29 +194,6 @@ class TestEnterpriseApi(unittest.TestCase):
actual_url = get_enterprise_consent_url(request_mock, course_id, return_to=return_to)
self.assertEqual(actual_url, expected_url)
- @mock.patch('openedx.features.enterprise_support.api.consent_needed_for_course')
- def test_get_enterprise_consent_url_next_provided_not_course_specific(self, needed_for_course_mock):
- """
- Verify that get_enterprise_consent_url correctly builds URLs.
- """
- needed_for_course_mock.return_value = True
-
- request_mock = mock.MagicMock(
- user=None,
- build_absolute_uri=lambda x: 'http://localhost:8000' + x # Don't do it like this in prod. Ever.
- )
-
- course_id = 'course-v1:edX+DemoX+Demo_Course'
-
- expected_url = (
- '/enterprise/grant_data_sharing_permissions?course_id=course-v1%3AedX%2BDemoX%2BDemo_'
- 'Course&failure_url=http%3A%2F%2Flocalhost%3A8000%2Fdashboard%3Fconsent_failed%3Dcou'
- 'rse-v1%253AedX%252BDemoX%252BDemo_Course&next=http%3A%2F%2Flocalhost%3A8000%2Fdashboard'
- )
-
- actual_url = get_enterprise_consent_url(request_mock, course_id, return_to='dashboard', course_specific_return=False)
- self.assertEqual(actual_url, expected_url)
-
def test_get_dashboard_consent_notification_no_param(self):
"""
Test that the output of the consent notification renderer meets expectations.
--
GitLab