Merge pull request #8488 from edx/patch/2015-06-11

Patch/2015 06 11

lms/djangoapps/commerce/tests/test_views.py

@@ -4,10 +4,11 @@ import json
 from uuid import uuid4
 from nose.plugins.attrib import attr
 
-from ddt import ddt, data
+import ddt
 from django.core.urlresolvers import reverse
 from django.test import TestCase
 from django.test.utils import override_settings
+import mock
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
 
@@ -35,7 +36,7 @@ class UserMixin(object):
 
 
 @attr('shard_1')
-@ddt
+@ddt.ddt
 @override_settings(ECOMMERCE_API_URL=TEST_API_URL, ECOMMERCE_API_SIGNING_KEY=TEST_API_SIGNING_KEY)
 class BasketsViewTests(EnrollmentEventTestMixin, UserMixin, ModuleStoreTestCase):
     """
@@ -102,7 +103,7 @@ class BasketsViewTests(EnrollmentEventTestMixin, UserMixin, ModuleStoreTestCase)
         self.client.logout()
         self.assertEqual(403, self._post_to_view().status_code)
 
-    @data('delete', 'get', 'put')
+    @ddt.data('delete', 'get', 'put')
     def test_post_required(self, method):
         """
         Verify that the view only responds to POST operations.
@@ -157,7 +158,7 @@ class BasketsViewTests(EnrollmentEventTestMixin, UserMixin, ModuleStoreTestCase)
         else:
             self.assertResponsePaymentData(response)
 
-    @data(True, False)
+    @ddt.data(True, False)
     def test_course_with_honor_seat_sku(self, user_is_active):
         """
         If the course has a SKU, the view should get authorization from the E-Commerce API before enrolling
@@ -172,7 +173,7 @@ class BasketsViewTests(EnrollmentEventTestMixin, UserMixin, ModuleStoreTestCase)
         with mock_create_basket(response=return_value):
             self._test_successful_ecommerce_api_call()
 
-    @data(True, 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
@@ -341,11 +342,63 @@ class BasketOrderViewTests(UserMixin, TestCase):
 
 
 @attr('shard_1')
-class ReceiptViewTests(TestCase):
+@ddt.ddt
+class ReceiptViewTests(UserMixin, TestCase):
     """ Tests for the receipt view. """
 
     def test_login_required(self):
         """ The view should redirect to the login page if the user is not logged in. """
         self.client.logout()
-        response = self.client.get(reverse('commerce:checkout_receipt'))
+        response = self.client.post(reverse('commerce:checkout_receipt'))
         self.assertEqual(response.status_code, 302)
+
+    def post_to_receipt_page(self, post_data):
+        """ DRY helper """
+        response = self.client.post(reverse('commerce:checkout_receipt'), params={'basket_id': 1}, data=post_data)
+        self.assertEqual(response.status_code, 200)
+        return response
+
+    @ddt.data('decision', 'reason_code', 'signed_field_names', None)
+    def test_is_cybersource(self, post_key):
+        """
+        Ensure the view uses three specific POST keys to detect a request initiated by Cybersource.
+        """
+        self._login()
+        post_data = {'decision': 'REJECT', 'reason_code': '200', 'signed_field_names': 'dummy'}
+        if post_key is not None:
+            # a key will be missing; we will not expect the receipt page to handle a cybersource decision
+            del post_data[post_key]
+            expected_pattern = r"<title>(\s+)Receipt"
+        else:
+            expected_pattern = r"<title>(\s+)Payment Failed"
+        response = self.post_to_receipt_page(post_data)
+        self.assertRegexpMatches(response.content, expected_pattern)
+
+    @ddt.data('ACCEPT', 'REJECT', 'ERROR')
+    def test_cybersource_decision(self, decision):
+        """
+        Ensure the view renders a page appropriately depending on the Cybersource decision.
+        """
+        self._login()
+        post_data = {'decision': decision, 'reason_code': '200', 'signed_field_names': 'dummy'}
+        expected_pattern = r"<title>(\s+)Receipt" if decision == 'ACCEPT' else r"<title>(\s+)Payment Failed"
+        response = self.post_to_receipt_page(post_data)
+        self.assertRegexpMatches(response.content, expected_pattern)
+
+    @ddt.data(True, False)
+    @mock.patch('commerce.views.is_user_payment_error')
+    def test_cybersource_message(self, is_user_message_expected, mock_is_user_payment_error):
+        """
+        Ensure that the page displays the right message for the reason_code (it
+        may be a user error message or a system error message).
+        """
+        mock_is_user_payment_error.return_value = is_user_message_expected
+        self._login()
+        response = self.post_to_receipt_page({'decision': 'REJECT', 'reason_code': '99', 'signed_field_names': 'dummy'})
+        self.assertTrue(mock_is_user_payment_error.called)
+        self.assertTrue(mock_is_user_payment_error.call_args[0][0], '99')
+
+        user_message = "There was a problem with this transaction"
+        system_message = "A system error occurred while processing your payment"
+        self.assertRegexpMatches(response.content, user_message if is_user_message_expected else system_message)
+        self.assertNotRegexpMatches(response.content, user_message if not is_user_message_expected else system_message)

lms/djangoapps/commerce/views.py

@@ -3,7 +3,6 @@ import logging
 
 from django.conf import settings
 from django.contrib.auth.decorators import login_required
-from django.views.decorators.cache import cache_page
 from django.views.decorators.csrf import csrf_exempt
 from ecommerce_api_client import exceptions
 from opaque_keys import InvalidKeyError
@@ -26,6 +25,8 @@ from student.models import CourseEnrollment
 from openedx.core.lib.api.authentication import SessionAuthenticationAllowInactiveUser
 from util.json_request import JsonResponse
 from verify_student.models import SoftwareSecurePhotoVerification
+from shoppingcart.processors.CyberSource2 import is_user_payment_error
+from django.utils.translation import ugettext as _
 
 
 log = logging.getLogger(__name__)
@@ -137,7 +138,6 @@ class BasketsView(APIView):
 
 
 @csrf_exempt
-@cache_page(1800)
 def checkout_cancel(_request):
     """ Checkout/payment cancellation view. """
     context = {'payment_support_email': microsite.get_value('payment_support_email', settings.PAYMENT_SUPPORT_EMAIL)}
@@ -148,9 +148,46 @@ def checkout_cancel(_request):
 @login_required
 def checkout_receipt(request):
     """ Receipt view. """
+
+    page_title = _('Receipt')
+    is_payment_complete = True
+    payment_support_email = microsite.get_value('payment_support_email', settings.PAYMENT_SUPPORT_EMAIL)
+    payment_support_link = '<a href=\"mailto:{email}\">{email}</a>'.format(email=payment_support_email)
+
+    is_cybersource = all(k in request.POST for k in ('signed_field_names', 'decision', 'reason_code'))
+    if is_cybersource and request.POST['decision'] != 'ACCEPT':
+        # Cybersource may redirect users to this view if it couldn't recover
+        # from an error while capturing payment info.
+        is_payment_complete = False
+        page_title = _('Payment Failed')
+        reason_code = request.POST['reason_code']
+        # if the problem was with the info submitted by the user, we present more detailed messages.
+        if is_user_payment_error(reason_code):
+            error_summary = _("There was a problem with this transaction. You have not been charged.")
+            error_text = _(
+                "Make sure your information is correct, or try again with a different card or another form of payment."
+            )
+        else:
+            error_summary = _("A system error occurred while processing your payment. You have not been charged.")
+            error_text = _("Please wait a few minutes and then try again.")
+        for_help_text = _("For help, contact {payment_support_link}.").format(payment_support_link=payment_support_link)
+    else:
+        # if anything goes wrong rendering the receipt, it indicates a problem fetching order data.
+        error_summary = _("An error occurred while creating your receipt.")
+        error_text = None  # nothing particularly helpful to say if this happens.
+        for_help_text = _(
+            "If your course does not appear on your dashboard, contact {payment_support_link}."
+        ).format(payment_support_link=payment_support_link)
+
     context = {
+        'page_title': page_title,
+        'is_payment_complete': is_payment_complete,
         'platform_name': microsite.get_value('platform_name', settings.PLATFORM_NAME),
-        'verified': SoftwareSecurePhotoVerification.verification_valid_or_pending(request.user).exists()
+        'verified': SoftwareSecurePhotoVerification.verification_valid_or_pending(request.user).exists(),
+        'error_summary': error_summary,
+        'error_text': error_text,
+        'for_help_text': for_help_text,
+        'payment_support_email': payment_support_email,
     }
     return render_to_response('commerce/checkout_receipt.html', context)
 

lms/djangoapps/shoppingcart/processors/CyberSource2.py

@@ -32,7 +32,7 @@ from collections import OrderedDict, defaultdict
 from decimal import Decimal, InvalidOperation
 from hashlib import sha256
 from django.conf import settings
-from django.utils.translation import ugettext as _
+from django.utils.translation import ugettext as _, ugettext_noop
 from edxmako.shortcuts import render_to_string
 from shoppingcart.models import Order
 from shoppingcart.processors.exceptions import *
@@ -41,6 +41,10 @@ from microsite_configuration import microsite
 
 log = logging.getLogger(__name__)
 
+# Translators: this text appears when an unfamiliar error code occurs during payment,
+# for which we don't know a user-friendly message to display in advance.
+DEFAULT_REASON = ugettext_noop("UNKNOWN REASON")
+
 
 def process_postpay_callback(params):
     """
@@ -582,18 +586,29 @@ CARDTYPE_MAP.update(
     }
 )
 
-REASONCODE_MAP = defaultdict(lambda: "UNKNOWN REASON")
+
+# Note: these messages come directly from official Cybersource documentation at:
+# http://apps.cybersource.com/library/documentation/dev_guides/CC_Svcs_SO_API/html/wwhelp/wwhimpl/js/html/wwhelp.htm#href=reason_codes.html
+REASONCODE_MAP = defaultdict(lambda: DEFAULT_REASON)
 REASONCODE_MAP.update(
     {
         '100': _('Successful transaction.'),
+        '101': _('The request is missing one or more required fields.'),
         '102': _('One or more fields in the request contains invalid data.'),
         '104': dedent(_(
             """
-            The access_key and transaction_uuid fields for this authorization request matches the access_key and
-            transaction_uuid of another authorization request that you sent in the last 15 minutes.
-            Possible fix: retry the payment after 15 minutes.
+            The merchant reference code for this authorization request matches the merchant reference code of another
+            authorization request that you sent within the past 15 minutes.
+            Possible action: Resend the request with a unique merchant reference code.
             """)),
         '110': _('Only a partial amount was approved.'),
+        '150': _('General system failure.'),
+        '151': dedent(_(
+            """
+            The request was received but there was a server timeout. This error does not include timeouts between the
+            client and the server.
+            """)),
+        '152': _('The request was received, but a service did not finish running in time.'),
         '200': dedent(_(
             """
             The authorization request was approved by the issuing bank but declined by CyberSource
@@ -603,63 +618,101 @@ REASONCODE_MAP.update(
             """
             The issuing bank has questions about the request. You do not receive an
             authorization code programmatically, but you might receive one verbally by calling the processor.
-            Possible fix: retry with another form of payment
+            Possible action: retry with another form of payment.
             """)),
         '202': dedent(_(
             """
             Expired card. You might also receive this if the expiration date you
             provided does not match the date the issuing bank has on file.
-            Possible fix: retry with another form of payment
+            Possible action: retry with another form of payment.
             """)),
         '203': dedent(_(
             """
             General decline of the card. No other information provided by the issuing bank.
-            Possible fix: retry with another form of payment
+            Possible action: retry with another form of payment.
             """)),
-        '204': _('Insufficient funds in the account. Possible fix: retry with another form of payment'),
+        '204': _('Insufficient funds in the account. Possible action: retry with another form of payment.'),
         # 205 was Stolen or lost card.  Might as well not show this message to the person using such a card.
-        '205': _('Stolen or lost card'),
-        '207': _('Issuing bank unavailable. Possible fix: retry again after a few minutes'),
+        '205': _('Stolen or lost card.'),
+        '207': _('Issuing bank unavailable. Possible action: retry again after a few minutes.'),
         '208': dedent(_(
             """
             Inactive card or card not authorized for card-not-present transactions.
-            Possible fix: retry with another form of payment
+            Possible action: retry with another form of payment.
             """)),
-        '210': _('The card has reached the credit limit. Possible fix: retry with another form of payment'),
-        '211': _('Invalid card verification number (CVN). Possible fix: retry with another form of payment'),
+        '209': _('CVN did not match.'),
+        '210': _('The card has reached the credit limit. Possible action: retry with another form of payment.'),
+        '211': _('Invalid card verification number (CVN). Possible action: retry with another form of payment.'),
         # 221 was The customer matched an entry on the processor's negative file.
         # Might as well not show this message to the person using such a card.
         '221': _('The customer matched an entry on the processors negative file.'),
-        '222': _('Account frozen. Possible fix: retry with another form of payment'),
+        '222': _('Account frozen. Possible action: retry with another form of payment.'),
         '230': dedent(_(
             """
             The authorization request was approved by the issuing bank but declined by
             CyberSource because it did not pass the CVN check.
-            Possible fix: retry with another form of payment
+            Possible action: retry with another form of payment.
             """)),
-        '231': _('Invalid account number. Possible fix: retry with another form of payment'),
+        '231': _('Invalid account number. Possible action: retry with another form of payment.'),
         '232': dedent(_(
             """
             The card type is not accepted by the payment processor.
-            Possible fix: retry with another form of payment
+            Possible action: retry with another form of payment.
             """)),
-        '233': _('General decline by the processor.  Possible fix: retry with another form of payment'),
+        '233': _('General decline by the processor.  Possible action: retry with another form of payment.'),
         '234': _(
             "There is a problem with the information in your CyberSource account.  Please let us know at {0}"
         ).format(settings.PAYMENT_SUPPORT_EMAIL),
-        '236': _('Processor Failure.  Possible fix: retry the payment'),
+        '235': _('The requested capture amount exceeds the originally authorized amount.'),
+        '236': _('Processor Failure.  Possible action: retry the payment'),
+        '237': _('The authorization has already been reversed.'),
+        '238': _('The authorization has already been captured.'),
+        '239': _('The requested transaction amount must match the previous transaction amount.'),
         '240': dedent(_(
             """
             The card type sent is invalid or does not correlate with the credit card number.
-            Possible fix: retry with the same card or another form of payment
+            Possible action: retry with the same card or another form of payment.
+            """)),
+        '241': _('The request ID is invalid.'),
+        '242': dedent(_(
+            """
+            You requested a capture, but there is no corresponding, unused authorization record. Occurs if there was
+            not a previously successful authorization request or if the previously successful authorization has already
+            been used by another capture request.
+            """)),
+        '243': _('The transaction has already been settled or reversed.'),
+        '246': dedent(_(
+            """
+            Either the capture or credit is not voidable because the capture or credit information has already been
+            submitted to your processor, or you requested a void for a type of transaction that cannot be voided.
             """)),
+        '247': _('You requested a credit for a capture that was previously voided.'),
+        '250': _('The request was received, but there was a timeout at the payment processor.'),
+        '254': _('Stand-alone credits are not allowed.'),
         '475': _('The cardholder is enrolled for payer authentication'),
         '476': _('Payer authentication could not be authenticated'),
         '520': dedent(_(
             """
             The authorization request was approved by the issuing bank but declined by CyberSource based
             on your legacy Smart Authorization settings.
-            Possible fix: retry with a different form of payment.
+            Possible action: retry with a different form of payment.
             """)),
     }
 )
+
+
+def is_user_payment_error(reason_code):
+    """
+    Decide, based on the reason_code, whether or not it signifies a problem
+    with something the user did (rather than a system error beyond the user's
+    control).
+
+    This function is used to determine whether we can/should show the user a
+    message with suggested actions to fix the problem, or simply apologize and
+    ask her to try again later.
+    """
+    reason_code = str(reason_code)
+    if reason_code not in REASONCODE_MAP or REASONCODE_MAP[reason_code] == DEFAULT_REASON:
+        return False
+
+    return (200 <= int(reason_code) <= 233) or int(reason_code) in (101, 102, 240)

lms/static/js/commerce/views/receipt_view.js

@@ -50,8 +50,9 @@ var edx = edx || {};
             var self = this,
                 orderId = $.url('?basket_id') || $.url('?payment-order-num');
 
-            if (orderId) {
+            if (orderId && this.$el.data('is-payment-complete')==='True') {
                 // Get the order details
+                self.$el.removeClass('hidden');
                 self.getReceiptData(orderId).then(self.renderReceipt, self.renderError);
             } else {
                 self.renderError();

lms/static/js/spec/verify_student/make_payment_step_view_spec.js

@@ -103,11 +103,15 @@ define([
                 expect($el.length).toEqual(_.size(buttons));
                 _.each(buttons, function( expectedText, expectedId ) {
                     var buttonEl = $( '#' + expectedId );
+
                     buttonEl.removeAttr('disabled');
                     expect( buttonEl.length ).toEqual( 1 );
                     expect( buttonEl[0] ).toHaveClass( 'payment-button' );
                     expect( buttonEl[0] ).toHaveText( expectedText );
+
                     buttonEl[0].click();
+                    expect( buttonEl[0] ).toHaveClass( 'is-selected' );
+                    expectPaymentButtonEnabled( false );
                     expect(requests[requests.length - 1].requestBody.split('&')).toContain('processor=' + expectedId);
                 });
             };
@@ -136,15 +140,15 @@ define([
 
             it( 'provides working payment buttons for a single processor', function() {
                 createView({processors: ['cybersource']});
-                checkPaymentButtons( AjaxHelpers.requests(this), {cybersource: "Pay with Credit Card"});
+                checkPaymentButtons( AjaxHelpers.requests(this), {cybersource: "Checkout"});
             });
 
             it( 'provides working payment buttons for multiple processors', function() {
                 createView({processors: ['cybersource', 'paypal', 'other']});
                 checkPaymentButtons( AjaxHelpers.requests(this), {
-                    cybersource: "Pay with Credit Card",
-                    paypal: "Pay with PayPal",
-                    other: "Pay with other"
+                    cybersource: "Checkout",
+                    paypal: "Checkout with PayPal",
+                    other: "Checkout with other"
                 });
             });
 

lms/static/js/verify_student/views/make_payment_step_view.js

@@ -45,13 +45,13 @@ var edx = edx || {};
 
         _getPaymentButtonText: function(processorName) {
             if (processorName.toLowerCase().substr(0, 11)=='cybersource') {
-                return gettext('Pay with Credit Card');
+                return gettext('Checkout');
             } else if (processorName.toLowerCase()=='paypal') {
-                return gettext('Pay with PayPal');
+                return gettext('Checkout with PayPal');
             } else {
                 // This is mainly for testing as no other processors are supported right now.
                 // Translators: 'processor' is the name of a third-party payment processing vendor (example: "PayPal")
-                return interpolate_text(gettext('Pay with {processor}'), {processor: processorName});
+                return interpolate_text(gettext('Checkout with {processor}'), {processor: processorName});
             }
         },
 
@@ -134,6 +134,8 @@ var edx = edx || {};
             // Disable the payment button to prevent multiple submissions
             this.setPaymentEnabled( false );
 
+            $( event.target ).toggleClass( 'is-selected' );
+
             // Create the order for the amount
             $.ajax({
                 url: '/verify_student/create_order/',
@@ -194,6 +196,8 @@ var edx = edx || {};
 
             // Re-enable the button so the user can re-try
             this.setPaymentEnabled( true );
+
+            $( '.payment-button' ).toggleClass( 'is-selected', false );
         },
 
         getPaymentAmount: function() {

lms/static/sass/_developer.scss

@@ -67,6 +67,10 @@
   .payment-button {
     float: right;
     @include margin-left( ($baseline/2) );
+    
+    &.is-selected {
+      background: $m-blue-d3 !important;
+    }
   }
 }
 

lms/templates/commerce/checkout_receipt.html

@@ -6,9 +6,7 @@ from django.utils.translation import ugettext as _
 <%inherit file="../main.html" />
 <%block name="bodyclass">register verification-process step-requirements</%block>
 
-<%block name="pagetitle">
-${_("Receipt")}
-</%block>
+<%block name="pagetitle">${page_title}</%block>
 
 <%block name="header_extras">
 <script type="text/template" id="receipt-tpl">
@@ -33,11 +31,17 @@ ${_("Receipt")}
             <i class="msg-icon icon fa fa-exclamation-triangle" aria-hidden="true"></i>
             <div class="msg-content">
                 <h3 class="title">
-                    <span class="sr">${ _("Error:") }</span>
-                    ${ _("Error") }
+                    <span class="sr">${error_summary}</span>
+                    ${error_summary}
                 </h3>
+%if error_text:
                 <div class="copy">
-                    <p>${ _("Could not retrieve payment information") }</p>
+                    <p>${error_text}</p>
+                    <br/>
+                </div>
+%endif
+                <div class="msg">
+                    <p>${for_help_text}</p>
                 </div>
             </div>
         </div>
@@ -46,7 +50,7 @@ ${_("Receipt")}
 
 <div class="container">
     <section class="wrapper carousel">
-        <div id="receipt-container" class="pay-and-verify" data-platform-name='${platform_name}' data-verified='${verified}'>
+        <div id="receipt-container" class="pay-and-verify hidden" data-is-payment-complete='${is_payment_complete}' data-platform-name='${platform_name}' data-verified='${verified}'>
             <h1>${_("Loading Order Data...")}</h1>
             <span>${ _("Please wait while we retrieve your order details.") }</span>
         </div>