Remove option to override receipt and cancel page URLs

These overrides are the result of an incomplete understanding of how to provide custom redirect URLs to CyberSource and other payment processors. Knowing now that we'll always use the URLs configured in settings, these overrides provide no value and only serve to complicate our code.

ecommerce/extensions/payment/processors.py

@@ -32,17 +32,13 @@ class BasePaymentProcessor(object):  # pragma: no cover
     NAME = None
 
     @abc.abstractmethod
-    def get_transaction_parameters(self, basket, receipt_page_url=None, cancel_page_url=None, **kwargs):
+    def get_transaction_parameters(self, basket):
         """
         Generate a dictionary of signed parameters required for this processor to complete a transaction.
 
         Arguments:
             basket (Basket): The basket of products being purchased.
 
-        Keyword Arguments:
-            receipt_page_url (unicode): If provided, overrides the receipt page URL normally used by this processor.
-            cancel_page_url (unicode): If provided, overrides the cancellation page URL normally used by this processor.
-
         Returns:
             dict: Payment processor-specific parameters required to complete a transaction.
         """
@@ -57,9 +53,11 @@ class BasePaymentProcessor(object):  # pragma: no cover
             1. Verify the validity of the response.
             2. Create PaymentEvents and Sources for successful payments.
 
-        Args:
-            response (dict): Dictionary of parameters received from the payment processor.
-            basket (Basket): Basket being purchased via the payment processor.
+        Arguments:
+            response (dict): Dictionary of parameters received from the payment processor
+
+        Keyword Arguments:
+            basket (Basket): Basket whose contents have been purchased via the payment processor
         """
         raise NotImplementedError
 
@@ -77,7 +75,7 @@ class BasePaymentProcessor(object):  # pragma: no cover
             dict: Payment processor configuration
 
         Raises:
-            KeyError: If no settings found for this payment processor.
+            KeyError: If no settings found for this payment processor
         """
         return settings.PAYMENT_PROCESSOR_CONFIG[self.NAME]
 
@@ -86,11 +84,11 @@ class BasePaymentProcessor(object):  # pragma: no cover
         Save the processor's response to the database for auditing.
 
         Arguments:
-            transaction_id (string): Identifier for the transaction on the payment processor's servers.
             response (dict): Response received from the payment processor
 
         Keyword Arguments:
-            basket (Basket): Basket associated with the payment event (e.g. being purchased)
+            transaction_id (string): Identifier for the transaction on the payment processor's servers
+            basket (Basket): Basket associated with the payment event (e.g., being purchased)
 
         Return
             PaymentProcessorResponse
@@ -120,23 +118,17 @@ class Cybersource(BasePaymentProcessor):
         self.access_key = configuration['access_key']
         self.secret_key = configuration['secret_key']
         self.payment_page_url = configuration['payment_page_url']
-        self.receipt_page_url = configuration.get('receipt_page_url')
-        self.cancel_page_url = configuration.get('cancel_page_url')
+        self.receipt_page_url = configuration['receipt_page_url']
+        self.cancel_page_url = configuration['cancel_page_url']
         self.language_code = settings.LANGUAGE_CODE
 
-    def get_transaction_parameters(self, basket, receipt_page_url=None, cancel_page_url=None, **kwargs):
+    def get_transaction_parameters(self, basket):
         """
         Generate a dictionary of signed parameters CyberSource requires to complete a transaction.
 
         Arguments:
             basket (Basket): The basket of products being purchased.
 
-        Keyword Arguments:
-            receipt_page_url (unicode): If provided, overrides the receipt page URL on the Secure Acceptance
-                profile in use for this transaction.
-            cancel_page_url (unicode): If provided, overrides the cancellation page URL on the Secure Acceptance
-                profile in use for this transaction.
-
         Returns:
             dict: CyberSource-specific parameters required to complete a transaction, including a signature.
         """
@@ -152,22 +144,13 @@ class Cybersource(BasePaymentProcessor):
             u'reference_number': unicode(basket.id),
             u'amount': unicode(basket.total_incl_tax),
             u'currency': basket.currency,
-            u'consumer_id': basket.owner.username
+            u'consumer_id': basket.owner.username,
+            u'override_custom_receipt_page': u'{}?basket_id={}'.format(self.receipt_page_url, basket.id),
+            u'override_custom_cancel_page': self.cancel_page_url,
         }
 
         # TODO Include edX-specific data (e.g. course_id, seat type)
 
-        # Allow the URL overrides passed directly to this method to override those pulled from settings.
-        cancel_page_url = cancel_page_url or self.cancel_page_url
-        if cancel_page_url:
-            parameters[u'override_custom_cancel_page'] = cancel_page_url
-
-        if self.receipt_page_url and not receipt_page_url:
-            receipt_page_url = u'{}?basket_id={}'.format(self.receipt_page_url, basket.id)
-
-        if receipt_page_url:
-            parameters[u'override_custom_receipt_page'] = receipt_page_url
-
         # Sign all fields
         signed_field_names = parameters.keys()
         parameters[u'signed_field_names'] = u','.join(sorted(signed_field_names))

ecommerce/extensions/payment/tests/processors.py

@@ -4,10 +4,10 @@ from ecommerce.extensions.payment.processors import BasePaymentProcessor
 class DummyProcessor(BasePaymentProcessor):
     NAME = 'dummy'
 
-    def handle_processor_response(self, response, basket=None):
+    def get_transaction_parameters(self, basket):
         pass
 
-    def get_transaction_parameters(self, basket, receipt_page_url=None, cancel_page_url=None, **kwargs):
+    def handle_processor_response(self, response, basket=None):
         pass
 
     def is_signature_valid(self, response):

ecommerce/extensions/payment/tests/test_processors.py

@@ -68,15 +68,13 @@ class CybersourceTests(CybersourceMixin, PaymentProcessorTestCaseMixin, TestCase
     processor_class = processors.Cybersource
     processor_name = 'cybersource'
 
-    def assert_valid_transaction_parameters(self, cancel_page_url=None, receipt_page_url=None):
-        """ Validates the transaction parameters returned by get_transaction_parameters(). """
+    def test_get_transaction_parameters(self):
+        """ Verify the processor returns the appropriate parameters required to complete a transaction. """
 
         # Patch the datetime object so that we can validate the signed_date_time field
         with mock.patch.object(processors.datetime, u'datetime', mock.Mock(wraps=datetime.datetime)) as mocked_datetime:
             mocked_datetime.utcnow.return_value = self.PI_DAY
-            actual = self.processor.get_transaction_parameters(self.basket,
-                                                               cancel_page_url=cancel_page_url,
-                                                               receipt_page_url=receipt_page_url)
+            actual = self.processor.get_transaction_parameters(self.basket)
 
         configuration = settings.PAYMENT_PROCESSOR_CONFIG[self.processor_name]
         access_key = configuration[u'access_key']
@@ -93,19 +91,11 @@ class CybersourceTests(CybersourceMixin, PaymentProcessorTestCaseMixin, TestCase
             u'reference_number': unicode(self.basket.id),
             u'amount': unicode(self.basket.total_incl_tax),
             u'currency': self.basket.currency,
-            u'consumer_id': self.basket.owner.username
+            u'consumer_id': self.basket.owner.username,
+            u'override_custom_receipt_page': u'{}?basket_id={}'.format(self.processor.receipt_page_url, self.basket.id),
+            u'override_custom_cancel_page': self.processor.cancel_page_url,
         }
 
-        cancel_page_url = cancel_page_url or self.processor.cancel_page_url
-        if cancel_page_url:
-            expected[u'override_custom_cancel_page'] = cancel_page_url
-
-        if self.processor.receipt_page_url and not receipt_page_url:
-            receipt_page_url = u'{}?basket_id={}'.format(self.processor.receipt_page_url, self.basket.id)
-
-        if receipt_page_url:
-            expected[u'override_custom_receipt_page'] = receipt_page_url
-
         signed_field_names = expected.keys() + [u'transaction_uuid']
         expected[u'signed_field_names'] = u','.join(sorted(signed_field_names))
 
@@ -183,26 +173,3 @@ class CybersourceTests(CybersourceMixin, PaymentProcessorTestCaseMixin, TestCase
         response = self.generate_notification(self.processor.secret_key, self.basket, auth_amount=u'0.00')
         self.assertRaises(PartialAuthorizationError, self.processor.handle_processor_response, response,
                           basket=self.basket)
-
-    def test_get_transaction_parameters(self):
-        """ Verify the processor returns the appropriate parameters required to complete a transaction. """
-
-        # Test with settings overrides
-        self.processor.receipt_page_url = u'http://example.com/receipt/'
-        self.processor.cancel_page_url = u'http://example.com/cancel/'
-        self.assert_valid_transaction_parameters()
-
-        # Test with receipt page override
-        self.assert_valid_transaction_parameters(receipt_page_url=u'http://example.com/receipt/')
-
-        # Test with cancel page override
-        self.assert_valid_transaction_parameters(cancel_page_url=u'http://example.com/cancel/')
-
-        # Test with both overrides
-        self.assert_valid_transaction_parameters(cancel_page_url=u'http://example.com/cancel/',
-                                                 receipt_page_url=u'http://example.com/receipt/')
-
-        # Test without overrides
-        self.processor.receipt_page_url = None
-        self.processor.cancel_page_url = None
-        self.assert_valid_transaction_parameters()