From e1dfdc0811ae9599a2ddac444d9e657361177206 Mon Sep 17 00:00:00 2001 From: jsa Date: Wed, 10 Jun 2015 10:57:54 -0400 Subject: [PATCH] Receipt page now also handles Cybersource payment failures. XCOM-398 --- lms/djangoapps/commerce/tests/test_views.py | 67 +++++++++++-- lms/djangoapps/commerce/views.py | 43 +++++++- .../shoppingcart/processors/CyberSource2.py | 97 ++++++++++++++----- lms/static/js/commerce/views/receipt_view.js | 3 +- lms/templates/commerce/checkout_receipt.html | 18 ++-- 5 files changed, 188 insertions(+), 40 deletions(-) diff --git a/lms/djangoapps/commerce/tests/test_views.py b/lms/djangoapps/commerce/tests/test_views.py index 64399a9556..0c88269c8b 100644 --- a/lms/djangoapps/commerce/tests/test_views.py +++ b/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"(\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) diff --git a/lms/djangoapps/commerce/views.py b/lms/djangoapps/commerce/views.py index efbe3c7429..a23fc7a103 100644 --- a/lms/djangoapps/commerce/views.py +++ b/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) diff --git a/lms/djangoapps/shoppingcart/processors/CyberSource2.py b/lms/djangoapps/shoppingcart/processors/CyberSource2.py index 795515acd6..83a063ace5 100644 --- a/lms/djangoapps/shoppingcart/processors/CyberSource2.py +++ b/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) diff --git a/lms/static/js/commerce/views/receipt_view.js b/lms/static/js/commerce/views/receipt_view.js index da18a49a57..0542c10ad6 100644 --- a/lms/static/js/commerce/views/receipt_view.js +++ b/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(); diff --git a/lms/templates/commerce/checkout_receipt.html b/lms/templates/commerce/checkout_receipt.html index 448fd6faf7..fc5bdf39cc 100644 --- a/lms/templates/commerce/checkout_receipt.html +++ b/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>