From 32679b0423855fe1d19f98a36c35da377e6991fc Mon Sep 17 00:00:00 2001 From: ayub-khan Date: Fri, 25 Aug 2017 16:02:14 +0500 Subject: [PATCH] Removed waffle flag on commerce api for default_enrollment_mode Removed ecommerce calls for default_enrollment_mode order creation Removed tests related to those LEARNER-2294 --- .../commerce/api/v0/tests/test_views.py | 157 ++---------------- lms/djangoapps/commerce/api/v0/views.py | 95 +---------- lms/djangoapps/commerce/tests/mocks.py | 18 -- lms/djangoapps/commerce/utils.py | 3 - 4 files changed, 18 insertions(+), 255 deletions(-) diff --git a/lms/djangoapps/commerce/api/v0/tests/test_views.py b/lms/djangoapps/commerce/api/v0/tests/test_views.py index 19394e2641..2e2a864130 100644 --- a/lms/djangoapps/commerce/api/v0/tests/test_views.py +++ b/lms/djangoapps/commerce/api/v0/tests/test_views.py @@ -5,7 +5,6 @@ from datetime import datetime, timedelta from uuid import uuid4 import ddt -import httpretty import mock import pytz from django.conf import settings @@ -15,15 +14,13 @@ from django.test.utils import override_settings from edx_rest_api_client import exceptions from nose.plugins.attrib import attr -from commerce.api.v0.views import SAILTHRU_CAMPAIGN_COOKIE, STOP_BASKET_CREATION_FLAG +from commerce.api.v0.views import SAILTHRU_CAMPAIGN_COOKIE from commerce.constants import Messages -from commerce.tests import TEST_BASKET_ID, TEST_ORDER_NUMBER, TEST_PAYMENT_DATA -from commerce.tests.mocks import mock_basket_order, mock_create_basket +from commerce.tests.mocks import mock_basket_order from commerce.tests.test_views import UserMixin from course_modes.models import CourseMode from enrollment.api import get_enrollment from openedx.core.djangoapps.embargo.test_utils import restrict_course -from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag from openedx.core.lib.django_test_client_utils import get_absolute_url from student.models import CourseEnrollment from course_modes.tests.factories import CourseModeFactory @@ -42,7 +39,7 @@ UTM_COOKIE_CONTENTS = { @ddt.ddt class BasketsViewTests(EnrollmentEventTestMixin, UserMixin, ModuleStoreTestCase): """ - Tests for the commerce orders view. + Tests for the commerce Baskets view. """ def _post_to_view(self, course_id=None, marketing_email_opt_in=False, include_utm_cookie=False): """ @@ -69,22 +66,6 @@ class BasketsViewTests(EnrollmentEventTestMixin, UserMixin, ModuleStoreTestCase) actual = json.loads(response.content)['detail'] self.assertEqual(actual, expected_msg) - def assertResponsePaymentData(self, response): - """ Asserts correctness of a JSON body containing payment information. """ - actual_response = json.loads(response.content) - self.assertEqual(actual_response, TEST_PAYMENT_DATA) - - def assertValidEcommerceInternalRequestErrorResponse(self, response): - """ Asserts the response is a valid response sent when the E-Commerce API is unavailable. """ - self.assertEqual(response.status_code, 500) - actual = json.loads(response.content)['detail'] - self.assertIn('Call to E-Commerce API failed', actual) - - def assertUserNotEnrolled(self): - """ Asserts that the user is NOT enrolled in the course, and that an enrollment event was NOT fired. """ - self.assertFalse(CourseEnrollment.is_enrolled(self.user, self.course.id)) - self.assert_no_events_were_emitted() - def setUp(self): super(BasketsViewTests, self).setUp() self.url = reverse('commerce_api:v0:baskets:create') @@ -147,102 +128,15 @@ class BasketsViewTests(EnrollmentEventTestMixin, UserMixin, ModuleStoreTestCase) self.assertEqual(406, self.client.post(self.url, {}).status_code) self.assertEqual(406, self.client.post(self.url, {'not_course_id': ''}).status_code) - def test_ecommerce_api_timeout(self): - """ - If the call to the E-Commerce API times out, the view should log an error and return an HTTP 503 status. - """ - with mock_create_basket(exception=exceptions.Timeout): - response = self._post_to_view() - - self.assertValidEcommerceInternalRequestErrorResponse(response) - self.assertUserNotEnrolled() - - def test_ecommerce_api_error(self): - """ - If the E-Commerce API raises an error, the view should return an HTTP 503 status. - """ - with mock_create_basket(exception=exceptions.SlumberBaseException): - response = self._post_to_view() - - self.assertValidEcommerceInternalRequestErrorResponse(response) - self.assertUserNotEnrolled() - - def _test_successful_ecommerce_api_call(self, is_completed=True, utm_tracking_present=False): - """ - Verifies that the view contacts the E-Commerce API with the correct data and headers. - """ - with mock.patch('commerce.api.v0.views.audit_log') as mock_audit_log: - response = self._post_to_view(include_utm_cookie=utm_tracking_present) - - # Verify that an audit message was logged - self.assertTrue(mock_audit_log.called) - - # Validate the response content - if is_completed: - msg = Messages.ORDER_COMPLETED.format(order_number=TEST_ORDER_NUMBER) - self.assertResponseMessage(response, msg) - else: - self.assertResponsePaymentData(response) - - # Make sure ecommerce API call forwards Sailthru cookie - self.assertIn('{}=sailthru id'.format(SAILTHRU_CAMPAIGN_COOKIE), httpretty.last_request().headers['cookie']) - - # Check that UTM tracking cookie is passed along in request to ecommerce for attribution - if utm_tracking_present: - cookie_string = '{cookie_name}={cookie_contents}'.format( - cookie_name=UTM_COOKIE_NAME, cookie_contents=json.dumps(UTM_COOKIE_CONTENTS)) - self.assertIn(cookie_string, httpretty.last_request().headers['cookie']) - - @override_waffle_flag(STOP_BASKET_CREATION_FLAG, active=False) @ddt.data(True, False) - def test_course_with_honor_seat_sku(self, user_is_active): + def test_course_for_active_and_inactive_user(self, user_is_active): """ - If the course has a SKU, the view should get authorization from the E-Commerce API before enrolling - the user in the course. If authorization is approved, the user should be redirected to the user dashboard. - """ - - # Set user's active flag - self.user.is_active = user_is_active - self.user.save() # pylint: disable=no-member - - return_value = {'id': TEST_BASKET_ID, 'payment_data': None, 'order': {'number': TEST_ORDER_NUMBER}} - with mock_create_basket(response=return_value): - # Test that call without utm tracking works - self._test_successful_ecommerce_api_call() - with mock.patch('student.models.RegistrationCookieConfiguration.current') as config: - instance = config.return_value - instance.utm_cookie_name = UTM_COOKIE_NAME - - # Test that call with cookie passes cookie along - self._test_successful_ecommerce_api_call(utm_tracking_present=True) - - @override_waffle_flag(STOP_BASKET_CREATION_FLAG, active=False) - @ddt.data(True, False) - def test_course_with_paid_seat_sku(self, user_is_active): - """ - If the course has a SKU, the view should return data that the client - will use to redirect the user to an external payment processor. + Test course enrollment for active and inactive user. """ # Set user's active flag self.user.is_active = user_is_active self.user.save() # pylint: disable=no-member - - return_value = {'id': TEST_BASKET_ID, 'payment_data': TEST_PAYMENT_DATA, 'order': None} - with mock_create_basket(response=return_value): - self._test_successful_ecommerce_api_call(is_completed=False) - - @override_waffle_flag(STOP_BASKET_CREATION_FLAG, active=True) - @ddt.data(True, False) - def test_course_without_creating_order(self, user_is_active): - """ - If the course has a SKU, and the STOP_BASKET_CREATION waffle flag is on, - the enrollment should happen without contacting ecommerce api - """ - # Set user's active flag - self.user.is_active = user_is_active - self.user.save() # pylint: disable=no-member - with mock_create_basket(expect_called=False): - response = self._post_to_view() + response = self._post_to_view() # Validate the response content self.assertEqual(response.status_code, 200) @@ -254,11 +148,9 @@ class BasketsViewTests(EnrollmentEventTestMixin, UserMixin, ModuleStoreTestCase) def _test_course_without_sku(self, enrollment_mode=CourseMode.DEFAULT_MODE_SLUG): """ - Validates the view bypasses the E-Commerce API when the course has no CourseModes with SKUs. + Validates the view when course has no CourseModes with SKUs. """ - # Place an order - with mock_create_basket(expect_called=False): - response = self._post_to_view() + response = self._post_to_view() # Validate the response content self.assertEqual(response.status_code, 200) @@ -299,22 +191,6 @@ class BasketsViewTests(EnrollmentEventTestMixin, UserMixin, ModuleStoreTestCase) # We should be enrolled in honor mode self._test_course_without_sku(enrollment_mode=CourseMode.HONOR) - @override_settings(ECOMMERCE_API_URL=None) - def test_ecommerce_service_not_configured(self): - """ - If the E-Commerce Service is not configured, the view should enroll the user. - """ - with mock_create_basket(expect_called=False): - response = self._post_to_view() - - # Validate the response - self.assertEqual(response.status_code, 200) - msg = Messages.NO_ECOM_API.format(username=self.user.username, course_id=self.course.id) - self.assertResponseMessage(response, msg) - - # Ensure that the user is not enrolled and that no calls were made to the E-Commerce API - self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course.id)) - def assertProfessionalModeBypassed(self): """ Verifies that the view returns HTTP 406 when a course with no honor or audit mode is encountered. """ @@ -323,9 +199,7 @@ class BasketsViewTests(EnrollmentEventTestMixin, UserMixin, ModuleStoreTestCase) sku_string = uuid4().hex.decode('ascii') CourseModeFactory.create(course_id=self.course.id, mode_slug=mode, mode_display_name=mode, sku=sku_string, bulk_sku='BULK-{}'.format(sku_string)) - - with mock_create_basket(expect_called=False): - response = self._post_to_view() + response = self._post_to_view() # The view should return an error status code self.assertEqual(response.status_code, 406) @@ -376,9 +250,6 @@ class BasketsViewTests(EnrollmentEventTestMixin, UserMixin, ModuleStoreTestCase) self.assertFalse(CourseEnrollment.is_enrolled(self.user, self.course.id)) self.assertIsNotNone(get_enrollment(self.user.username, unicode(self.course.id))) - with mock_create_basket(): - self._test_successful_ecommerce_api_call(is_completed=False) - @mock.patch('commerce.api.v0.views.update_email_opt_in') @ddt.data(*itertools.product((False, True), (False, True), (False, True))) @ddt.unpack @@ -395,21 +266,17 @@ class BasketsViewTests(EnrollmentEventTestMixin, UserMixin, ModuleStoreTestCase) if is_exception: mock_update.side_effect = Exception("boink") - return_value = {'id': TEST_BASKET_ID, 'payment_data': None, 'order': {'number': TEST_ORDER_NUMBER}} - with mock_create_basket(response=return_value, expect_called=has_sku): - response = self._post_to_view(marketing_email_opt_in=is_opt_in) + response = self._post_to_view(marketing_email_opt_in=is_opt_in) self.assertEqual(mock_update.called, is_opt_in) self.assertEqual(response.status_code, 200) def test_closed_course(self): """ - Ensure that the view does not attempt to create a basket for closed - courses. + Verifies that the view returns HTTP 406 when a course is closed. """ self.course.enrollment_end = datetime.now(pytz.UTC) - timedelta(days=1) modulestore().update_item(self.course, self.user.id) # pylint:disable=no-member - with mock_create_basket(expect_called=False): - self.assertEqual(self._post_to_view().status_code, 406) + self.assertEqual(self._post_to_view().status_code, 406) @attr(shard=1) diff --git a/lms/djangoapps/commerce/api/v0/views.py b/lms/djangoapps/commerce/api/v0/views.py index d6ff0fd663..181cf81ed4 100644 --- a/lms/djangoapps/commerce/api/v0/views.py +++ b/lms/djangoapps/commerce/api/v0/views.py @@ -1,7 +1,6 @@ """ API v0 views. """ import logging -import requests from edx_rest_api_client import exceptions from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey @@ -11,9 +10,7 @@ from rest_framework.status import HTTP_406_NOT_ACCEPTABLE, HTTP_409_CONFLICT from rest_framework.views import APIView from commerce.constants import Messages -from commerce.exceptions import InvalidResponseError -from commerce.http import DetailResponse, InternalRequestErrorResponse -from commerce.utils import COMMERCE_API_WAFFLE_FLAG_NAMESPACE +from commerce.http import DetailResponse from course_modes.models import CourseMode from courseware import courses from enrollment.api import add_enrollment @@ -21,15 +18,12 @@ from enrollment.views import EnrollmentCrossDomainSessionAuth from openedx.core.djangoapps.commerce.utils import ecommerce_api_client from openedx.core.djangoapps.embargo import api as embargo_api from openedx.core.djangoapps.user_api.preferences.api import update_email_opt_in -from openedx.core.djangoapps.waffle_utils import WaffleFlag from openedx.core.lib.api.authentication import OAuth2AuthenticationAllowInactiveUser -from openedx.core.lib.log_utils import audit_log -from student.models import CourseEnrollment, RegistrationCookieConfiguration +from student.models import CourseEnrollment from util.json_request import JsonResponse log = logging.getLogger(__name__) SAILTHRU_CAMPAIGN_COOKIE = 'sailthru_bid' -STOP_BASKET_CREATION_FLAG = WaffleFlag(COMMERCE_API_WAFFLE_FLAG_NAMESPACE, 'stop_basket_creation') class BasketsView(APIView): @@ -85,7 +79,7 @@ class BasketsView(APIView): def post(self, request, *args, **kwargs): """ - Attempt to enroll the user, and if needed, create the basket. + Attempt to enroll the user. """ user = request.user valid, course_key, error = self._is_data_valid(request) @@ -120,11 +114,7 @@ class BasketsView(APIView): # Accept either honor or audit as an enrollment mode to # maintain backwards compatibility with existing courses default_enrollment_mode = audit_mode or honor_mode - - if not default_enrollment_mode: - msg = Messages.NO_DEFAULT_ENROLLMENT_MODE.format(course_id=course_id) - return DetailResponse(msg, status=HTTP_406_NOT_ACCEPTABLE) - elif not default_enrollment_mode.sku or STOP_BASKET_CREATION_FLAG.is_enabled(): + if default_enrollment_mode: msg = Messages.ENROLL_DIRECTLY.format( username=user.username, course_id=course_id @@ -141,81 +131,8 @@ class BasketsView(APIView): self._handle_marketing_opt_in(request, course_key, user) return DetailResponse(msg) else: - return self._create_basket_to_order(request, user, course_key, default_enrollment_mode) - - def _add_request_cookie_to_api_session(self, server_session, request, cookie_name): - """ Add cookie from user request into server session """ - user_cookie = None - if cookie_name: - user_cookie = request.COOKIES.get(cookie_name) - if user_cookie: - server_cookie = {cookie_name: user_cookie} - if server_session.cookies: - requests.utils.add_dict_to_cookiejar(server_session.cookies, server_cookie) - else: - server_session.cookies = requests.utils.cookiejar_from_dict(server_cookie) - - def _create_basket_to_order(self, request, user, course_key, default_enrollment_mode): - """ - Connect to the ecommerce service to create the basket and the order to do the enrollment - """ - # Setup the API - course_id = unicode(course_key) - try: - api_session = requests.Session() - api = ecommerce_api_client(user, session=api_session) - except ValueError: - self._enroll(course_key, user) - msg = Messages.NO_ECOM_API.format(username=user.username, course_id=course_id) - log.debug(msg) - return DetailResponse(msg) - - response = None - - # Make the API call - try: - # Pass along Sailthru campaign id - self._add_request_cookie_to_api_session(api_session, request, SAILTHRU_CAMPAIGN_COOKIE) - - # Pass along UTM tracking info - utm_cookie_name = RegistrationCookieConfiguration.current().utm_cookie_name - self._add_request_cookie_to_api_session(api_session, request, utm_cookie_name) - - response_data = api.baskets.post({ - 'products': [{'sku': default_enrollment_mode.sku}], - 'checkout': True, - }) - - payment_data = response_data["payment_data"] - if payment_data: - # Pass data to the client to begin the payment flow. - response = JsonResponse(payment_data) - elif response_data['order']: - # The order was completed immediately because there is no charge. - msg = Messages.ORDER_COMPLETED.format(order_number=response_data['order']['number']) - log.debug(msg) - response = DetailResponse(msg) - else: - msg = u'Unexpected response from basket endpoint.' - log.error( - msg + u' Could not enroll user %(username)s in course %(course_id)s.', - {'username': user.id, 'course_id': course_id}, - ) - raise InvalidResponseError(msg) - except (exceptions.SlumberBaseException, exceptions.Timeout) as ex: - log.exception(ex.message) - return InternalRequestErrorResponse(ex.message) - finally: - audit_log( - 'checkout_requested', - course_id=course_id, - mode=default_enrollment_mode.slug, - processor_name=None, - user_id=user.id - ) - - self._handle_marketing_opt_in(request, course_key, user) - return response + msg = Messages.NO_DEFAULT_ENROLLMENT_MODE.format(course_id=course_id) + return DetailResponse(msg, status=HTTP_406_NOT_ACCEPTABLE) class BasketOrderView(APIView): diff --git a/lms/djangoapps/commerce/tests/mocks.py b/lms/djangoapps/commerce/tests/mocks.py index 199c2e405f..1ce677842a 100644 --- a/lms/djangoapps/commerce/tests/mocks.py +++ b/lms/djangoapps/commerce/tests/mocks.py @@ -80,24 +80,6 @@ class mock_ecommerce_api_endpoint(object): httpretty.reset() -class mock_create_basket(mock_ecommerce_api_endpoint): - """ Mocks calls to E-Commerce API client basket creation method. """ - - default_response = { - 'id': 7, - 'order': {'number': '100004'}, # never both None. - 'payment_data': { - 'payment_processor_name': 'test-processor', - 'payment_form_data': {}, - 'payment_page_url': 'http://example.com/pay', - }, - } - method = httpretty.POST - - def get_path(self): - return '/baskets/' - - class mock_basket_order(mock_ecommerce_api_endpoint): """ Mocks calls to E-Commerce API client basket order method. """ diff --git a/lms/djangoapps/commerce/utils.py b/lms/djangoapps/commerce/utils.py index 465de98697..7de0e6d6f1 100644 --- a/lms/djangoapps/commerce/utils.py +++ b/lms/djangoapps/commerce/utils.py @@ -7,9 +7,6 @@ from django.conf import settings from commerce.models import CommerceConfiguration from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers -from openedx.core.djangoapps.waffle_utils import WaffleFlagNamespace - -COMMERCE_API_WAFFLE_FLAG_NAMESPACE = WaffleFlagNamespace(name='commerce_api') def is_account_activation_requirement_disabled():