Commit a90f7ab1 by Will Daly

Enter the new verification / payment flow using a GET param

Allow disabling of experiment
parent 234691e4
...@@ -55,6 +55,16 @@ class ChooseModeView(View): ...@@ -55,6 +55,16 @@ class ChooseModeView(View):
upgrade = request.GET.get('upgrade', False) upgrade = request.GET.get('upgrade', False)
request.session['attempting_upgrade'] = upgrade 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) enrollment_mode, is_active = CourseEnrollment.enrollment_mode_for_user(request.user, course_key)
modes = CourseMode.modes_for_course_dict(course_key) modes = CourseMode.modes_for_course_dict(course_key)
...@@ -63,7 +73,9 @@ class ChooseModeView(View): ...@@ -63,7 +73,9 @@ class ChooseModeView(View):
# to the usual "choose your track" page. # to the usual "choose your track" page.
has_enrolled_professional = (enrollment_mode == "professional" and is_active) has_enrolled_professional = (enrollment_mode == "professional" and is_active)
if "professional" in modes and not has_enrolled_professional: 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( return redirect(
reverse( reverse(
'verify_student_start_flow', 'verify_student_start_flow',
...@@ -180,7 +192,9 @@ class ChooseModeView(View): ...@@ -180,7 +192,9 @@ class ChooseModeView(View):
donation_for_course[unicode(course_key)] = amount_value donation_for_course[unicode(course_key)] = amount_value
request.session["donation_for_course"] = donation_for_course 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( return redirect(
reverse( reverse(
'verify_student_start_flow', 'verify_student_start_flow',
......
...@@ -50,6 +50,12 @@ class TestCourseVerificationStatus(UrlResetMixin, ModuleStoreTestCase): ...@@ -50,6 +50,12 @@ class TestCourseVerificationStatus(UrlResetMixin, ModuleStoreTestCase):
success = self.client.login(username=self.user.username, password="edx") success = self.client.login(username=self.user.username, password="edx")
self.assertTrue(success, msg="Did not log in successfully") 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): def test_enrolled_as_non_verified(self):
self._setup_mode_and_enrollment(None, "honor") self._setup_mode_and_enrollment(None, "honor")
...@@ -92,7 +98,7 @@ class TestCourseVerificationStatus(UrlResetMixin, ModuleStoreTestCase): ...@@ -92,7 +98,7 @@ class TestCourseVerificationStatus(UrlResetMixin, ModuleStoreTestCase):
def test_need_to_verify_expiration(self): def test_need_to_verify_expiration(self):
self._setup_mode_and_enrollment(self.FUTURE, "verified") 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, self.BANNER_ALT_MESSAGES[VERIFY_STATUS_NEED_TO_VERIFY])
self.assertContains(response, "You only have 4 days left to verify for this course.") self.assertContains(response, "You only have 4 days left to verify for this course.")
...@@ -122,7 +128,7 @@ class TestCourseVerificationStatus(UrlResetMixin, ModuleStoreTestCase): ...@@ -122,7 +128,7 @@ class TestCourseVerificationStatus(UrlResetMixin, ModuleStoreTestCase):
self._assert_course_verification_status(VERIFY_STATUS_APPROVED) self._assert_course_verification_status(VERIFY_STATUS_APPROVED)
# Check that the "verification good until" date is displayed # 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")) self.assertContains(response, attempt.expiration_datetime.strftime("%m/%d/%Y"))
def test_missed_verification_deadline(self): def test_missed_verification_deadline(self):
...@@ -237,7 +243,7 @@ class TestCourseVerificationStatus(UrlResetMixin, ModuleStoreTestCase): ...@@ -237,7 +243,7 @@ class TestCourseVerificationStatus(UrlResetMixin, ModuleStoreTestCase):
AssertionError AssertionError
""" """
response = self.client.get(reverse('dashboard')) response = self.client.get(self.dashboard_url)
# Sanity check: verify that the course is on the page # Sanity check: verify that the course is on the page
self.assertContains(response, unicode(self.course.id)) self.assertContains(response, unicode(self.course.id))
......
...@@ -568,13 +568,21 @@ def dashboard(request): ...@@ -568,13 +568,21 @@ def dashboard(request):
# #
# If a course is not included in this dictionary, # If a course is not included in this dictionary,
# there is no verification messaging to display. # 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( verify_status_by_course = check_verify_status_by_course(
user, user,
course_enrollment_pairs, course_enrollment_pairs,
all_course_modes all_course_modes
) )
else: else:
if request.GET.get('disable-separate-verified', False) and 'separate-verified' in request.session:
del request.session['separate-verified']
verify_status_by_course = {} verify_status_by_course = {}
cert_statuses = { cert_statuses = {
......
...@@ -655,8 +655,7 @@ class OrderItem(TimeStampedModel): ...@@ -655,8 +655,7 @@ class OrderItem(TimeStampedModel):
""" """
return {} return {}
@property def additional_instruction_text(self, **kwargs): # pylint: disable=unused-argument
def additional_instruction_text(self):
""" """
Individual instructions for this order item. Individual instructions for this order item.
...@@ -1378,8 +1377,7 @@ class CertificateItem(OrderItem): ...@@ -1378,8 +1377,7 @@ class CertificateItem(OrderItem):
"dashboard_url": reverse('dashboard'), "dashboard_url": reverse('dashboard'),
} }
@property def additional_instruction_text(self, **kwargs):
def additional_instruction_text(self):
refund_reminder = _( refund_reminder = _(
"You have up to two weeks into the course to unenroll from the Verified Certificate option " "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}. " "and receive a full refund. To receive your refund, contact {billing_email}. "
...@@ -1387,7 +1385,18 @@ class CertificateItem(OrderItem): ...@@ -1387,7 +1385,18 @@ class CertificateItem(OrderItem):
"Please do NOT include your credit card information." "Please do NOT include your credit card information."
).format(billing_email=settings.PAYMENT_SUPPORT_EMAIL) ).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) domain = microsite.get_value('SITE_NAME', settings.SITE_NAME)
path = reverse('verify_student_verify_later', kwargs={'course_id': unicode(self.course_id)}) path = reverse('verify_student_verify_later', kwargs={'course_id': unicode(self.course_id)})
verification_url = "http://{domain}{path}".format(domain=domain, path=path) verification_url = "http://{domain}{path}".format(domain=domain, path=path)
...@@ -1542,8 +1551,7 @@ class Donation(OrderItem): ...@@ -1542,8 +1551,7 @@ class Donation(OrderItem):
""" """
return self.pk_with_subclass, set([self._tax_deduction_msg()]) return self.pk_with_subclass, set([self._tax_deduction_msg()])
@property def additional_instruction_text(self, **kwargs): # pylint: disable=unused-argument
def additional_instruction_text(self):
"""Provide information about tax-deductible donations in the confirmation email. """Provide information about tax-deductible donations in the confirmation email.
Returns: Returns:
......
...@@ -249,7 +249,7 @@ class OrderTest(UrlResetMixin, ModuleStoreTestCase): ...@@ -249,7 +249,7 @@ class OrderTest(UrlResetMixin, ModuleStoreTestCase):
self.assertEquals('Order Payment Confirmation', mail.outbox[0].subject) self.assertEquals('Order Payment Confirmation', mail.outbox[0].subject)
self.assertIn(settings.PAYMENT_SUPPORT_EMAIL, mail.outbox[0].body) self.assertIn(settings.PAYMENT_SUPPORT_EMAIL, mail.outbox[0].body)
self.assertIn(unicode(cart.total_cost), 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. # Assert Google Analytics event fired for purchase.
self.mock_tracker.track.assert_called_once_with( # pylint: disable=maybe-no-member self.mock_tracker.track.assert_called_once_with( # pylint: disable=maybe-no-member
...@@ -280,7 +280,7 @@ class OrderTest(UrlResetMixin, ModuleStoreTestCase): ...@@ -280,7 +280,7 @@ class OrderTest(UrlResetMixin, ModuleStoreTestCase):
self.assertEquals(len(mail.outbox), 1) self.assertEquals(len(mail.outbox), 1)
# Verify that the verification reminder appears in the sent email. # 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): def test_purchase_item_failure(self):
# once again, we're testing against the specific implementation of # once again, we're testing against the specific implementation of
......
...@@ -1286,6 +1286,12 @@ class ReceiptRedirectTest(UrlResetMixin, ModuleStoreTestCase): ...@@ -1286,6 +1286,12 @@ class ReceiptRedirectTest(UrlResetMixin, ModuleStoreTestCase):
) )
self.cart.purchase() 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 # Visit the receipt page
url = reverse('shoppingcart.views.show_receipt', args=[self.cart.id]) url = reverse('shoppingcart.views.show_receipt', args=[self.cart.id])
resp = self.client.get(url) resp = self.client.get(url)
...@@ -1302,6 +1308,28 @@ class ReceiptRedirectTest(UrlResetMixin, ModuleStoreTestCase): ...@@ -1302,6 +1308,28 @@ class ReceiptRedirectTest(UrlResetMixin, ModuleStoreTestCase):
self.assertRedirects(resp, redirect_url) 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) @override_settings(MODULESTORE=MODULESTORE_CONFIG)
@patch.dict('django.conf.settings.FEATURES', {'ENABLE_PAID_COURSE_REGISTRATION': True}) @patch.dict('django.conf.settings.FEATURES', {'ENABLE_PAID_COURSE_REGISTRATION': True})
......
...@@ -798,7 +798,7 @@ def _show_receipt_html(request, order): ...@@ -798,7 +798,7 @@ def _show_receipt_html(request, order):
# TODO (ECOM-188): Once the A/B test of separate verified / payment flow # TODO (ECOM-188): Once the A/B test of separate verified / payment flow
# completes, implement this in a more general way. For now, # completes, implement this in a more general way. For now,
# we simply redirect to the new receipt page (in verify_student). # 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': if receipt_template == 'shoppingcart/verified_cert_receipt.html':
url = reverse( url = reverse(
'verify_student_payment_confirmation', 'verify_student_payment_confirmation',
......
...@@ -754,11 +754,10 @@ def create_order(request): ...@@ -754,11 +754,10 @@ def create_order(request):
""" """
Submit PhotoVerification and create a new Order for this verified cert 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 # Only submit photos if photo data is provided by the client.
# has completed, we can remove this flag and delete the photo verification # TODO (ECOM-188): Once the A/B test of decoupling verified / payment
# step entirely (since it will be handled in a separate view). # completes, we may be able to remove photo submission from this step
submit_photo = True # entirely.
if settings.FEATURES.get("SEPARATE_VERIFICATION_FROM_PAYMENT"):
submit_photo = ( submit_photo = (
'face_image' in request.POST and 'face_image' in request.POST and
'photo_id_image' in request.POST 'photo_id_image' in request.POST
......
...@@ -54,7 +54,7 @@ from student.helpers import ( ...@@ -54,7 +54,7 @@ from student.helpers import (
% endif % endif
% if settings.FEATURES.get('ENABLE_VERIFIED_CERTIFICATES'): % if settings.FEATURES.get('ENABLE_VERIFIED_CERTIFICATES'):
% if enrollment.mode == "verified": % 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]: % 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="sts-enrollment" title="${_("Your verification is pending")}">
<span class="label">${_("Enrolled as: ")}</span> <span class="label">${_("Enrolled as: ")}</span>
...@@ -131,7 +131,7 @@ from student.helpers import ( ...@@ -131,7 +131,7 @@ from student.helpers import (
<%include file='_dashboard_certificate_information.html' args='cert_status=cert_status,course=course, enrollment=enrollment'/> <%include file='_dashboard_certificate_information.html' args='cert_status=cert_status,course=course, enrollment=enrollment'/>
% endif % 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.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"> <div class="message message-status is-shown">
% if verification_status['status'] == VERIFY_STATUS_NEED_TO_VERIFY: % if verification_status['status'] == VERIFY_STATUS_NEED_TO_VERIFY:
...@@ -180,7 +180,7 @@ from student.helpers import ( ...@@ -180,7 +180,7 @@ from student.helpers import (
<ul class="actions message-actions"> <ul class="actions message-actions">
<li class="action-item"> <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}"> <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: % 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}"> <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}">
......
...@@ -39,5 +39,5 @@ ${order.bill_to_country.upper()} ...@@ -39,5 +39,5 @@ ${order.bill_to_country.upper()}
% endif % endif
% for order_item in order_items: % 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 % endfor
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment