Merge pull request #6410 from edx/will/decouple-verified-ab-test

Decouple verified/payment A/B test setup

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',

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))

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 = {

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.
 
@@ -1383,8 +1382,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}. "
@@ -1392,7 +1390,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)
@@ -1547,8 +1556,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:

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

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})

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',

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

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]:
             <span class="sts-enrollment" title="${_("Your verification is pending")}">
               <span class="label">${_("Enrolled as: ")}</span>
@@ -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:
         <div class="message message-status is-shown">
           % if verification_status['status'] == VERIFY_STATUS_NEED_TO_VERIFY:
@@ -180,7 +180,7 @@ from student.helpers import (
 
               <ul class="actions message-actions">
                 <li class="action-item">
-                  % if settings.FEATURES.get('SEPARATE_VERIFICATION_FROM_PAYMENT'):
+                  % if settings.FEATURES.get('SEPARATE_VERIFICATION_FROM_PAYMENT') and request.session.get('separate-verified', False):
                   <a class="action action-upgrade" href="${reverse('verify_student_upgrade_and_verify', kwargs={'course_id': unicode(course.id)})}" data-course-id="${course.id | h}" data-user="${user.username | h}">
                   % else:
                   <a class="action action-upgrade" href="${reverse('course_modes_choose', kwargs={'course_id': unicode(course.id)})}?upgrade=True" data-course-id="${course.id | h}" data-user="${user.username | h}">

lms/templates/emails/order_confirmation_email.txt

@@ -39,5 +39,5 @@ ${order.bill_to_country.upper()}
 % endif
 
 % for order_item in order_items:
-${order_item.additional_instruction_text}
+${order_item.additional_instruction_text(separate_verification=getattr(request, 'session', {}).get('separate-verified', False))}
 % endfor