diff --git a/common/djangoapps/course_modes/views.py b/common/djangoapps/course_modes/views.py index 36eefc87b7..15f3027a27 100644 --- a/common/djangoapps/course_modes/views.py +++ b/common/djangoapps/course_modes/views.py @@ -55,6 +55,16 @@ class ChooseModeView(View): upgrade = request.GET.get('upgrade', False) request.session['attempting_upgrade'] = upgrade + # TODO (ECOM-188): Once the A/B test of decoupled/verified flows + # completes, we can remove this flag. + # The A/B test framework will reload the page with the ?separate-verified GET param + # set if the user is in the experimental condition. We then store this flag + # in a session variable so downstream views can check it. + if request.GET.get('separate-verified', False): + request.session['separate-verified'] = True + elif request.GET.get('disable-separate-verified', False) and 'separate-verified' in request.session: + del request.session['separate-verified'] + enrollment_mode, is_active = CourseEnrollment.enrollment_mode_for_user(request.user, course_key) modes = CourseMode.modes_for_course_dict(course_key) @@ -63,7 +73,9 @@ class ChooseModeView(View): # to the usual "choose your track" page. has_enrolled_professional = (enrollment_mode == "professional" and is_active) if "professional" in modes and not has_enrolled_professional: - if settings.FEATURES.get('SEPARATE_VERIFICATION_FROM_PAYMENT'): + # TODO (ECOM-188): Once the A/B test of separating verification / payment completes, + # we can remove the check for the session variable. + if settings.FEATURES.get('SEPARATE_VERIFICATION_FROM_PAYMENT') and request.session.get('separate-verified', False): return redirect( reverse( 'verify_student_start_flow', @@ -180,7 +192,9 @@ class ChooseModeView(View): donation_for_course[unicode(course_key)] = amount_value request.session["donation_for_course"] = donation_for_course - if settings.FEATURES.get('SEPARATE_VERIFICATION_FROM_PAYMENT'): + # TODO (ECOM-188): Once the A/B test of separate verification flow completes, + # we can remove the check for the session variable. + if settings.FEATURES.get('SEPARATE_VERIFICATION_FROM_PAYMENT') and request.session.get('separate-verified', False): return redirect( reverse( 'verify_student_start_flow', diff --git a/common/djangoapps/student/tests/test_verification_status.py b/common/djangoapps/student/tests/test_verification_status.py index c13e44d548..32b3317bbd 100644 --- a/common/djangoapps/student/tests/test_verification_status.py +++ b/common/djangoapps/student/tests/test_verification_status.py @@ -50,6 +50,12 @@ class TestCourseVerificationStatus(UrlResetMixin, ModuleStoreTestCase): success = self.client.login(username=self.user.username, password="edx") self.assertTrue(success, msg="Did not log in successfully") + # Use the URL with the querystring param to put the user + # in the experimental track. + # TODO (ECOM-188): Once the A/B test of decoupling verified / payment + # completes, we can remove the querystring param. + self.dashboard_url = reverse('dashboard') + '?separate-verified=1' + def test_enrolled_as_non_verified(self): self._setup_mode_and_enrollment(None, "honor") @@ -92,7 +98,7 @@ class TestCourseVerificationStatus(UrlResetMixin, ModuleStoreTestCase): def test_need_to_verify_expiration(self): self._setup_mode_and_enrollment(self.FUTURE, "verified") - response = self.client.get(reverse('dashboard')) + response = self.client.get(self.dashboard_url) self.assertContains(response, self.BANNER_ALT_MESSAGES[VERIFY_STATUS_NEED_TO_VERIFY]) self.assertContains(response, "You only have 4 days left to verify for this course.") @@ -122,7 +128,7 @@ class TestCourseVerificationStatus(UrlResetMixin, ModuleStoreTestCase): self._assert_course_verification_status(VERIFY_STATUS_APPROVED) # Check that the "verification good until" date is displayed - response = self.client.get(reverse('dashboard')) + response = self.client.get(self.dashboard_url) self.assertContains(response, attempt.expiration_datetime.strftime("%m/%d/%Y")) def test_missed_verification_deadline(self): @@ -237,7 +243,7 @@ class TestCourseVerificationStatus(UrlResetMixin, ModuleStoreTestCase): AssertionError """ - response = self.client.get(reverse('dashboard')) + response = self.client.get(self.dashboard_url) # Sanity check: verify that the course is on the page self.assertContains(response, unicode(self.course.id)) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 4c8b08d7ec..f5f75485c8 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -568,13 +568,21 @@ def dashboard(request): # # If a course is not included in this dictionary, # there is no verification messaging to display. - if settings.FEATURES.get("SEPARATE_VERIFICATION_FROM_PAYMENT"): + # + # TODO (ECOM-188): After the A/B test completes, we can remove the check + # for the GET param and the session var. + # The A/B test framework will set the GET param for users in the experimental + # group; we then set the session var so downstream views can check this. + if settings.FEATURES.get("SEPARATE_VERIFICATION_FROM_PAYMENT") and request.GET.get('separate-verified', False): + request.session['separate-verified'] = True verify_status_by_course = check_verify_status_by_course( user, course_enrollment_pairs, all_course_modes ) else: + if request.GET.get('disable-separate-verified', False) and 'separate-verified' in request.session: + del request.session['separate-verified'] verify_status_by_course = {} cert_statuses = { diff --git a/lms/djangoapps/shoppingcart/models.py b/lms/djangoapps/shoppingcart/models.py index d959294774..b736238dd1 100644 --- a/lms/djangoapps/shoppingcart/models.py +++ b/lms/djangoapps/shoppingcart/models.py @@ -655,8 +655,7 @@ class OrderItem(TimeStampedModel): """ return {} - @property - def additional_instruction_text(self): + def additional_instruction_text(self, **kwargs): # pylint: disable=unused-argument """ Individual instructions for this order item. @@ -1378,8 +1377,7 @@ class CertificateItem(OrderItem): "dashboard_url": reverse('dashboard'), } - @property - def additional_instruction_text(self): + def additional_instruction_text(self, **kwargs): refund_reminder = _( "You have up to two weeks into the course to unenroll from the Verified Certificate option " "and receive a full refund. To receive your refund, contact {billing_email}. " @@ -1387,7 +1385,18 @@ class CertificateItem(OrderItem): "Please do NOT include your credit card information." ).format(billing_email=settings.PAYMENT_SUPPORT_EMAIL) - if settings.FEATURES.get('SEPARATE_VERIFICATION_FROM_PAYMENT'): + # TODO (ECOM-188): When running the A/B test for + # separating the verified / payment flow, we want to add some extra instructions + # for users in the experimental group. In order to know the user is in the experimental + # group, we need to check a session variable. But at this point in the code, + # we're so deep into the request handling stack that we don't have access to the request. + # The approach taken here is to have the email template check the request session + # and pass in a kwarg to this function if it's set. The template already has + # access to the request (via edxmako middleware), so we don't need to change + # too much to make this work. Once the A/B test completes, though, we should + # clean this up by removing the `**kwargs` param and skipping the check + # for the session variable. + if settings.FEATURES.get('SEPARATE_VERIFICATION_FROM_PAYMENT') and kwargs.get('separate_verification'): domain = microsite.get_value('SITE_NAME', settings.SITE_NAME) path = reverse('verify_student_verify_later', kwargs={'course_id': unicode(self.course_id)}) verification_url = "http://{domain}{path}".format(domain=domain, path=path) @@ -1542,8 +1551,7 @@ class Donation(OrderItem): """ return self.pk_with_subclass, set([self._tax_deduction_msg()]) - @property - def additional_instruction_text(self): + def additional_instruction_text(self, **kwargs): # pylint: disable=unused-argument """Provide information about tax-deductible donations in the confirmation email. Returns: diff --git a/lms/djangoapps/shoppingcart/tests/test_models.py b/lms/djangoapps/shoppingcart/tests/test_models.py index 2032e29bf6..394e4a4f4e 100644 --- a/lms/djangoapps/shoppingcart/tests/test_models.py +++ b/lms/djangoapps/shoppingcart/tests/test_models.py @@ -249,7 +249,7 @@ class OrderTest(UrlResetMixin, ModuleStoreTestCase): self.assertEquals('Order Payment Confirmation', mail.outbox[0].subject) self.assertIn(settings.PAYMENT_SUPPORT_EMAIL, mail.outbox[0].body) self.assertIn(unicode(cart.total_cost), mail.outbox[0].body) - self.assertIn(item.additional_instruction_text, mail.outbox[0].body) + self.assertIn(item.additional_instruction_text(), mail.outbox[0].body) # Assert Google Analytics event fired for purchase. self.mock_tracker.track.assert_called_once_with( # pylint: disable=maybe-no-member @@ -280,7 +280,7 @@ class OrderTest(UrlResetMixin, ModuleStoreTestCase): self.assertEquals(len(mail.outbox), 1) # Verify that the verification reminder appears in the sent email. - self.assertIn(item.additional_instruction_text, mail.outbox[0].body) + self.assertIn(item.additional_instruction_text(), mail.outbox[0].body) def test_purchase_item_failure(self): # once again, we're testing against the specific implementation of diff --git a/lms/djangoapps/shoppingcart/tests/test_views.py b/lms/djangoapps/shoppingcart/tests/test_views.py index 9a2e0d54fa..f32c0a82a7 100644 --- a/lms/djangoapps/shoppingcart/tests/test_views.py +++ b/lms/djangoapps/shoppingcart/tests/test_views.py @@ -1286,6 +1286,12 @@ class ReceiptRedirectTest(UrlResetMixin, ModuleStoreTestCase): ) self.cart.purchase() + # Set the session flag indicating that the user is in the + # experimental group + session = self.client.session + session['separate-verified'] = True + session.save() + # Visit the receipt page url = reverse('shoppingcart.views.show_receipt', args=[self.cart.id]) resp = self.client.get(url) @@ -1302,6 +1308,28 @@ class ReceiptRedirectTest(UrlResetMixin, ModuleStoreTestCase): self.assertRedirects(resp, redirect_url) + @patch.dict(settings.FEATURES, {'SEPARATE_VERIFICATION_FROM_PAYMENT': True}) + def test_no_redirect_if_not_in_experimental_group(self): + # Purchase a verified certificate + CertificateItem.add_to_order( + self.cart, + self.course_key, + self.COST, + 'verified' + ) + self.cart.purchase() + + # We do NOT set the session flag indicating that the user is in + # the experimental group. + + # Visit the receipt page + url = reverse('shoppingcart.views.show_receipt', args=[self.cart.id]) + resp = self.client.get(url) + + # Since the user is not in the experimental group, expect + # that we see the usual receipt page (no redirect) + self.assertEqual(resp.status_code, 200) + @override_settings(MODULESTORE=MODULESTORE_CONFIG) @patch.dict('django.conf.settings.FEATURES', {'ENABLE_PAID_COURSE_REGISTRATION': True}) diff --git a/lms/djangoapps/shoppingcart/views.py b/lms/djangoapps/shoppingcart/views.py index bb200d51b6..85a4636df9 100644 --- a/lms/djangoapps/shoppingcart/views.py +++ b/lms/djangoapps/shoppingcart/views.py @@ -798,7 +798,7 @@ def _show_receipt_html(request, order): # TODO (ECOM-188): Once the A/B test of separate verified / payment flow # completes, implement this in a more general way. For now, # we simply redirect to the new receipt page (in verify_student). - if settings.FEATURES.get('SEPARATE_VERIFICATION_FROM_PAYMENT'): + if settings.FEATURES.get('SEPARATE_VERIFICATION_FROM_PAYMENT') and request.session.get('separate-verified', False): if receipt_template == 'shoppingcart/verified_cert_receipt.html': url = reverse( 'verify_student_payment_confirmation', diff --git a/lms/djangoapps/verify_student/views.py b/lms/djangoapps/verify_student/views.py index 5e1fec7ed7..eec973b598 100644 --- a/lms/djangoapps/verify_student/views.py +++ b/lms/djangoapps/verify_student/views.py @@ -754,15 +754,14 @@ def create_order(request): """ Submit PhotoVerification and create a new Order for this verified cert """ - # TODO (ECOM-188): Once the A/B test of separating the payment/verified flow - # has completed, we can remove this flag and delete the photo verification - # step entirely (since it will be handled in a separate view). - submit_photo = True - if settings.FEATURES.get("SEPARATE_VERIFICATION_FROM_PAYMENT"): - submit_photo = ( - 'face_image' in request.POST and - 'photo_id_image' in request.POST - ) + # Only submit photos if photo data is provided by the client. + # TODO (ECOM-188): Once the A/B test of decoupling verified / payment + # completes, we may be able to remove photo submission from this step + # entirely. + submit_photo = ( + 'face_image' in request.POST and + 'photo_id_image' in request.POST + ) if ( submit_photo and not diff --git a/lms/templates/dashboard/_dashboard_course_listing.html b/lms/templates/dashboard/_dashboard_course_listing.html index 5c16627360..21121933eb 100644 --- a/lms/templates/dashboard/_dashboard_course_listing.html +++ b/lms/templates/dashboard/_dashboard_course_listing.html @@ -54,7 +54,7 @@ from student.helpers import ( % endif % if settings.FEATURES.get('ENABLE_VERIFIED_CERTIFICATES'): % if enrollment.mode == "verified": - % if settings.FEATURES.get('SEPARATE_VERIFICATION_FROM_PAYMENT'): + % if settings.FEATURES.get('SEPARATE_VERIFICATION_FROM_PAYMENT') and request.session.get('separate-verified', False): % if verification_status.get('status') in [VERIFY_STATUS_NEED_TO_VERIFY, VERIFY_STATUS_SUBMITTED]: ${_("Enrolled as: ")} @@ -131,7 +131,7 @@ from student.helpers import ( <%include file='_dashboard_certificate_information.html' args='cert_status=cert_status,course=course, enrollment=enrollment'/> % endif - % if settings.FEATURES.get('SEPARATE_VERIFICATION_FROM_PAYMENT'): + % if settings.FEATURES.get('SEPARATE_VERIFICATION_FROM_PAYMENT') and request.session.get('separate-verified', True): % if verification_status.get('status') in [VERIFY_STATUS_NEED_TO_VERIFY, VERIFY_STATUS_SUBMITTED, VERIFY_STATUS_APPROVED] and not is_course_blocked:
% if verification_status['status'] == VERIFY_STATUS_NEED_TO_VERIFY: @@ -180,7 +180,7 @@ from student.helpers import (