From 5cc8da127a7f89afe383e57945849ec4596d71d1 Mon Sep 17 00:00:00 2001 From: Ben Holt Date: Thu, 19 Sep 2019 14:59:07 -0400 Subject: [PATCH] REV-935 mobile upsell and tests (#21687) Positive test case and improvements including: requires course id to be correctly encoded in the url params, only checks entitlements of the course in question, better source for the basket url, check enrollment upgrade deadline --- .../experiments/tests/test_views_custom.py | 81 +++++++++++++++++-- lms/djangoapps/experiments/views_custom.py | 45 ++++++----- 2 files changed, 99 insertions(+), 27 deletions(-) diff --git a/lms/djangoapps/experiments/tests/test_views_custom.py b/lms/djangoapps/experiments/tests/test_views_custom.py index c375d5e3bc..5b21b21908 100644 --- a/lms/djangoapps/experiments/tests/test_views_custom.py +++ b/lms/djangoapps/experiments/tests/test_views_custom.py @@ -3,30 +3,99 @@ Tests for experimentation views """ from __future__ import absolute_import +from uuid import uuid4 +import six + from django.urls import reverse from rest_framework.test import APITestCase +from course_modes.tests.factories import CourseModeFactory from lms.djangoapps.course_blocks.transformers.tests.helpers import ModuleStoreTestCase from student.tests.factories import UserFactory +from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag +from xmodule.modulestore.tests.factories import CourseFactory + +from lms.djangoapps.experiments.views_custom import MOBILE_UPSELL_FLAG + CROSS_DOMAIN_REFERER = 'https://ecommerce.edx.org' -class Rev934Tests(APITestCase, ModuleStoreTestCase): - def test_logged_in(self): - """Test mobile app upsell API""" +class Rev934LoggedOutTests(APITestCase): + def test_not_logged_in(self): + """Test mobile app upsell API is not available if not logged in""" url = reverse('api_experiments:rev_934') - user = UserFactory() # Not-logged-in returns 401 response = self.client.get(url) self.assertEqual(response.status_code, 401) - # No-course-id returns show_upsell false + +class Rev934Tests(APITestCase, ModuleStoreTestCase): + """Test mobile app upsell API""" + @classmethod + def setUpClass(cls): + super(Rev934Tests, cls).setUpClass() + cls.url = reverse('api_experiments:rev_934') + + def setUp(self): + super(Rev934Tests, self).setUp() + user = UserFactory(username='robot-mue-1-6pnjv') # Username that hashes to bucket 1 self.client.login( username=user.username, password=UserFactory._DEFAULT_PASSWORD, # pylint: disable=protected-access ) - response = self.client.get(url) + + @override_waffle_flag(MOBILE_UPSELL_FLAG, active=False) + def test_flag_off(self): + response = self.client.get(self.url) self.assertEqual(response.status_code, 200) self.assertEqual(response.data['show_upsell'], False) + self.assertEqual(response.data['upsell_flag'], False) + + @override_waffle_flag(MOBILE_UPSELL_FLAG, active=True) + def test_no_course_id(self): + response = self.client.get(self.url) + self.assertEqual(response.status_code, 400) + + @override_waffle_flag(MOBILE_UPSELL_FLAG, active=True) + def test_bad_course_id(self): + response = self.client.get(self.url, {'course_id': 'junk'}) + self.assertEqual(response.status_code, 400) + + @override_waffle_flag(MOBILE_UPSELL_FLAG, active=True) + def test_simple_course(self): + course = CourseFactory.create() + response = self.client.get(self.url, {'course_id': course.id}) + self.assertEqual(response.status_code, 200) + expected = { + 'show_upsell': False, + 'upsell_flag': True, + 'experiment_bucket': 1, + 'user_upsell': True, + 'basket_link': None, # No sku means no basket link so no upsell + } + self.assertEqual(response.data, expected) + + @override_waffle_flag(MOBILE_UPSELL_FLAG, active=True) + def test_course(self): + course = CourseFactory.create(run='test', display_name='test') + CourseModeFactory.create( + mode_slug="verified", + course_id=course.id, + min_price=10, + sku=six.text_type(uuid4().hex) + ) + + response = self.client.get(self.url, {'course_id': course.id}) + self.assertEqual(response.status_code, 200) + result = response.data + self.assertIn('basket_url', result) + self.assertTrue(bool(result['basket_url'])) + expected = { + 'show_upsell': True, + 'price': u'$10', + 'basket_url': result['basket_url'], + # Example basket_url: u'/verify_student/upgrade/org.0/course_0/test/' + } + self.assertEqual(response.data, expected) diff --git a/lms/djangoapps/experiments/views_custom.py b/lms/djangoapps/experiments/views_custom.py index f18efbb6b7..00b8539330 100644 --- a/lms/djangoapps/experiments/views_custom.py +++ b/lms/djangoapps/experiments/views_custom.py @@ -5,10 +5,13 @@ The Discount API Views should return information about discounts that apply to t # -*- coding: utf-8 -*- from __future__ import absolute_import +import six from django.utils.decorators import method_decorator +from django.http import HttpResponseBadRequest from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser +from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from rest_framework.response import Response from rest_framework.views import APIView @@ -20,8 +23,9 @@ from openedx.core.lib.api.authentication import OAuth2AuthenticationAllowInactiv from openedx.core.lib.api.permissions import ApiKeyHeaderPermissionIsAuthenticated from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin -from course_modes.models import CourseMode -from lms.djangoapps.experiments.utils import get_base_experiment_metadata_context +from lms.djangoapps.courseware.date_summary import verified_upgrade_link_is_valid +from course_modes.models import get_cosmetic_verified_display_price +from lms.djangoapps.commerce.utils import EcommerceService from lms.djangoapps.experiments.stable_bucketing import stable_bucketing_hash_group from student.models import CourseEnrollment from track import segment @@ -92,7 +96,7 @@ class Rev934(DeveloperErrorViewMixin, APIView): "basket_url": "https://ecommerce.edx.org/basket/add?sku=abcdef" } """ - # http://localhost:18000/api/experiments/v0/custom/REV-934/?course_id=course-v1:edX+DemoX+Demo_Course + # https://courses.stage.edx.org/api/experiments/v0/custom/REV-934/?course_id=course-v1%3AedX%2BDemoX%2BDemo_Course authentication_classes = ( JwtAuthentication, @@ -112,33 +116,30 @@ class Rev934(DeveloperErrorViewMixin, APIView): 'upsell_flag': False, }) - if 'course_id' not in request.GET: - return Response({ - 'show_upsell': False, - }) + course_id = request.GET.get('course_id') + try: + course_key = CourseKey.from_string(course_id) + except InvalidKeyError: + return HttpResponseBadRequest("Missing or invalid course_id") - # HACK: the url decoding converts plus to space; put them back - course_id = request.GET.get('course_id').replace(' ', '+') - course_key = CourseKey.from_string(course_id) course = CourseOverview.get_from_id(course_key) user = request.user - enrollment = None - user_enrollments = None - has_non_audit_enrollments = False try: - user_enrollments = CourseEnrollment.objects.select_related('course').filter(user_id=user.id) - has_non_audit_enrollments = user_enrollments.exclude(mode__in=CourseMode.UPSELL_TO_VERIFIED_MODES).exists() enrollment = CourseEnrollment.objects.select_related( 'course' ).get(user_id=user.id, course_id=course.id) + user_upsell = verified_upgrade_link_is_valid(enrollment) except CourseEnrollment.DoesNotExist: - pass # Not enrolled, use the default values + user_upsell = True - context = get_base_experiment_metadata_context(course, user, enrollment, user_enrollments) + basket_link = EcommerceService().upgrade_url(user, course.id) + upgrade_price = six.text_type(get_cosmetic_verified_display_price(course)) + could_upsell = bool(user_upsell and basket_link) bucket = stable_bucketing_hash_group(MOBILE_UPSELL_EXPERIMENT, 2, user.username) - if hasattr(request, 'session') and MOBILE_UPSELL_EXPERIMENT not in request.session: + + if could_upsell and hasattr(request, 'session') and MOBILE_UPSELL_EXPERIMENT not in request.session: properties = { 'site': request.site.domain, 'app_label': 'experiments', @@ -154,16 +155,18 @@ class Rev934(DeveloperErrorViewMixin, APIView): # Mark that we've recorded this bucketing, so that we don't do it again this session request.session[MOBILE_UPSELL_EXPERIMENT] = True - show_upsell = bucket != 0 and not has_non_audit_enrollments + show_upsell = bool(bucket != 0 and could_upsell) if show_upsell: return Response({ 'show_upsell': show_upsell, - 'price': context.get('upgrade_price'), - 'basket_url': context.get('upgrade_link'), + 'price': upgrade_price, + 'basket_url': basket_link, }) else: return Response({ 'show_upsell': show_upsell, 'upsell_flag': MOBILE_UPSELL_FLAG.is_enabled(), 'experiment_bucket': bucket, + 'user_upsell': user_upsell, + 'basket_link': basket_link, })