Merge pull request #137 from edx/renzo/relay-order-id

Relay order number to payment processors

ecommerce/extensions/api/data.py

@@ -9,7 +9,6 @@ Product = get_model('catalogue', 'Product')
 
 Selector = get_class('partner.strategy', 'Selector')
 NoShippingRequired = get_class('shipping.methods', 'NoShippingRequired')
-OrderNumberGenerator = get_class('order.utils', 'OrderNumberGenerator')
 OrderTotalCalculator = get_class('checkout.calculators', 'OrderTotalCalculator')
 
 
@@ -55,13 +54,12 @@ def get_order_metadata(basket):
         dict: Containing an order number, a shipping method, a shipping charge,
             and a Price object representing the order total.
     """
-    number = OrderNumberGenerator().order_number(basket)
     shipping_method = NoShippingRequired()
     shipping_charge = shipping_method.calculate(basket)
     total = OrderTotalCalculator().calculate(basket, shipping_charge)
 
     metadata = {
-        AC.KEYS.ORDER_NUMBER: number,
+        AC.KEYS.ORDER_NUMBER: basket.order_number,
         AC.KEYS.SHIPPING_METHOD: shipping_method,
         AC.KEYS.SHIPPING_CHARGE: shipping_charge,
         AC.KEYS.ORDER_TOTAL: total,

ecommerce/extensions/basket/__init__.py

+default_app_config = 'ecommerce.extensions.basket.config.BasketConfig'  # pragma: no cover

ecommerce/extensions/basket/admin.py

+from oscar.apps.basket.admin import *  # noqa pylint: disable=wildcard-import,unused-wildcard-import

ecommerce/extensions/basket/config.py

+from oscar.apps.basket import config
+
+
+class BasketConfig(config.BasketConfig):
+    name = 'ecommerce.extensions.basket'

ecommerce/extensions/basket/migrations/0001_initial.py

+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import models, migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='Basket',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('status', models.CharField(default='Open', max_length=128, verbose_name='Status', choices=[('Open', 'Open - currently active'), ('Merged', 'Merged - superceded by another basket'), ('Saved', 'Saved - for items to be purchased later'), ('Frozen', 'Frozen - the basket cannot be modified'), ('Submitted', 'Submitted - has been ordered at the checkout')])),
+                ('date_created', models.DateTimeField(auto_now_add=True, verbose_name='Date created')),
+                ('date_merged', models.DateTimeField(blank=True, verbose_name='Date merged', null=True)),
+                ('date_submitted', models.DateTimeField(blank=True, verbose_name='Date submitted', null=True)),
+            ],
+            options={
+                'verbose_name_plural': 'Baskets',
+                'verbose_name': 'Basket',
+                'abstract': False,
+            },
+            bases=(models.Model,),
+        ),
+        migrations.CreateModel(
+            name='Line',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('line_reference', models.SlugField(max_length=128, verbose_name='Line Reference')),
+                ('quantity', models.PositiveIntegerField(default=1, verbose_name='Quantity')),
+                ('price_currency', models.CharField(default='GBP', max_length=12, verbose_name='Currency')),
+                ('price_excl_tax', models.DecimalField(max_digits=12, decimal_places=2, verbose_name='Price excl. Tax', null=True)),
+                ('price_incl_tax', models.DecimalField(max_digits=12, decimal_places=2, verbose_name='Price incl. Tax', null=True)),
+                ('date_created', models.DateTimeField(auto_now_add=True, verbose_name='Date Created')),
+            ],
+            options={
+                'verbose_name_plural': 'Basket lines',
+                'verbose_name': 'Basket line',
+                'abstract': False,
+            },
+            bases=(models.Model,),
+        ),
+        migrations.CreateModel(
+            name='LineAttribute',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('value', models.CharField(max_length=255, verbose_name='Value')),
+                ('line', models.ForeignKey(verbose_name='Line', related_name='attributes', to='basket.Line')),
+            ],
+            options={
+                'verbose_name_plural': 'Line attributes',
+                'verbose_name': 'Line attribute',
+                'abstract': False,
+            },
+            bases=(models.Model,),
+        ),
+    ]

ecommerce/extensions/basket/migrations/0002_auto_20140827_1705.py

+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import models, migrations
+from django.conf import settings
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('partner', '0001_initial'),
+        ('catalogue', '0001_initial'),
+        ('basket', '0001_initial'),
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='lineattribute',
+            name='option',
+            field=models.ForeignKey(verbose_name='Option', to='catalogue.Option'),
+            preserve_default=True,
+        ),
+        migrations.AddField(
+            model_name='line',
+            name='basket',
+            field=models.ForeignKey(verbose_name='Basket', related_name='lines', to='basket.Basket'),
+            preserve_default=True,
+        ),
+        migrations.AddField(
+            model_name='line',
+            name='product',
+            field=models.ForeignKey(verbose_name='Product', related_name='basket_lines', to='catalogue.Product'),
+            preserve_default=True,
+        ),
+        migrations.AddField(
+            model_name='line',
+            name='stockrecord',
+            field=models.ForeignKey(related_name='basket_lines', to='partner.StockRecord'),
+            preserve_default=True,
+        ),
+        migrations.AlterUniqueTogether(
+            name='line',
+            unique_together=set([('basket', 'line_reference')]),
+        ),
+        migrations.AddField(
+            model_name='basket',
+            name='owner',
+            field=models.ForeignKey(verbose_name='Owner', related_name='baskets', to=settings.AUTH_USER_MODEL, null=True),
+            preserve_default=True,
+        ),
+    ]

ecommerce/extensions/basket/migrations/0003_basket_vouchers.py

+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import models, migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('voucher', '0001_initial'),
+        ('basket', '0002_auto_20140827_1705'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='basket',
+            name='vouchers',
+            field=models.ManyToManyField(blank=True, verbose_name='Vouchers', to='voucher.Voucher', null=True),
+            preserve_default=True,
+        ),
+    ]

ecommerce/extensions/basket/migrations/0004_auto_20141007_2032.py

+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import models, migrations
+import oscar.core.utils
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('basket', '0003_basket_vouchers'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='line',
+            name='price_currency',
+            field=models.CharField(default=oscar.core.utils.get_default_currency, max_length=12, verbose_name='Currency'),
+        ),
+    ]

ecommerce/extensions/basket/models.py

+from oscar.apps.basket.abstract_models import AbstractBasket
+from oscar.core.loading import get_class
+
+
+OrderNumberGenerator = get_class('order.utils', 'OrderNumberGenerator')
+
+
+class Basket(AbstractBasket):
+    @property
+    def order_number(self):
+        return OrderNumberGenerator().order_number(self)
+
+
+from oscar.apps.basket.models import *  # noqa pylint: disable=wildcard-import,unused-wildcard-import

ecommerce/extensions/basket/tests/test_models.py

+from django.test import TestCase
+from oscar.core.loading import get_class
+from oscar.test import factories
+
+
+OrderNumberGenerator = get_class('order.utils', 'OrderNumberGenerator')
+
+
+class BasketTests(TestCase):
+    def setUp(self):
+        super(BasketTests, self).setUp()
+        self.basket = factories.create_basket()
+
+    def test_order_number_generation(self):
+        """Verify that an instance of Basket can generate its own order number."""
+        expected = OrderNumberGenerator().order_number(self.basket)
+        self.assertEqual(self.basket.order_number, expected)

ecommerce/extensions/order/tests/test_utils.py

 """Test Order Utility classes """
-from unittest import TestCase
-from django.test import override_settings
+from django.test import TestCase, override_settings
 from oscar.test.newfactories import BasketFactory
 
 from ecommerce.extensions.order.utils import OrderNumberGenerator
@@ -9,15 +8,26 @@ from ecommerce.extensions.order.utils import OrderNumberGenerator
 class UtilsTest(TestCase):
     """Unit tests for the order utility functions and classes. """
 
-    ORDER_NUMBER_PREFIX = "Zoidberg"
+    ORDER_NUMBER_PREFIX = 'FOO'
 
     @override_settings(ORDER_NUMBER_PREFIX=ORDER_NUMBER_PREFIX)
-    def create_order_number(self):
-        """Test creating order numbers"""
-        basket = BasketFactory()
-        next_basket = BasketFactory()
-        new_order_number = OrderNumberGenerator.order_number(basket)
-        next_order_number = OrderNumberGenerator.order_number(next_basket)
-        self.assertIn(self.ORDER_NUMBER_PREFIX, new_order_number)
-        self.assertIn(self.ORDER_NUMBER_PREFIX, next_order_number)
-        self.assertNotEqual(new_order_number, next_order_number)
+    def test_order_number_generation(self):
+        """
+        Verify that order numbers are generated correctly, and that they can
+        be converted back into basket IDs when necessary.
+        """
+        first_basket = BasketFactory()
+        second_basket = BasketFactory()
+
+        first_order_number = OrderNumberGenerator().order_number(first_basket)
+        second_order_number = OrderNumberGenerator().order_number(second_basket)
+
+        self.assertIn(self.ORDER_NUMBER_PREFIX, first_order_number)
+        self.assertIn(self.ORDER_NUMBER_PREFIX, second_order_number)
+        self.assertNotEqual(first_order_number, second_order_number)
+
+        first_basket_id = OrderNumberGenerator().basket_id(first_order_number)
+        second_basket_id = OrderNumberGenerator().basket_id(second_order_number)
+
+        self.assertEqual(first_basket_id, first_basket.id)
+        self.assertEqual(second_basket_id, second_basket.id)

ecommerce/extensions/order/utils.py

@@ -7,22 +7,38 @@ class OrderNumberGenerator(object):
 
     We need this as the order number is often required for payment
     which takes place before the order model has been created.
-
     """
+    OFFSET = 100000
 
-    @staticmethod
-    def order_number(basket):
+    def order_number(self, basket):
         """Create an order number with a configured prefix.
 
         Creates a unique order number with a configured prefix.
 
-        Args:
+        Arguments:
             basket (Basket): Used to construct the order ID.
 
         Returns:
-            String: representation of the order 'number' with a configured prefix.
+            str: Representation of the order 'number' with a configured prefix.
 
         """
-        prefix = getattr(settings, 'ORDER_NUMBER_PREFIX', 'OSCR')
-        order_id = str(100000 + basket.id)
-        return u"{prefix}-{order_id}".format(prefix=prefix, order_id=order_id)
+        order_id = basket.id + self.OFFSET
+        order_number = u'{prefix}-{order_id}'.format(prefix=settings.ORDER_NUMBER_PREFIX, order_id=order_id)
+
+        return order_number
+
+    def basket_id(self, order_number):
+        """Inverse of order number generation.
+
+        Given an order number, returns the basket ID used when generating it.
+
+        Arguments:
+            order_number (str): An order number.
+
+        Returns:
+            int: The basket ID used to generate the provided order number.
+        """
+        order_id = int(order_number.lstrip(u'{prefix}-'.format(prefix=settings.ORDER_NUMBER_PREFIX)))
+        basket_id = order_id - self.OFFSET
+
+        return basket_id

ecommerce/extensions/payment/processors/cybersource.py

 """ CyberSource payment processing. """
-
 import datetime
 from decimal import Decimal
 import logging
@@ -19,6 +18,7 @@ from ecommerce.extensions.payment.exceptions import (InvalidSignatureError, Inva
 from ecommerce.extensions.payment.helpers import sign
 from ecommerce.extensions.payment.processors import BasePaymentProcessor
 
+
 logger = logging.getLogger(__name__)
 
 PaymentEvent = get_model('order', 'PaymentEvent')
@@ -82,10 +82,11 @@ class Cybersource(BasePaymentProcessor):
             u'signed_date_time': datetime.datetime.utcnow().strftime(ISO_8601_FORMAT),
             u'locale': self.language_code,
             u'transaction_type': u'sale',
-            u'reference_number': unicode(basket.id),
+            u'reference_number': basket.order_number,
             u'amount': unicode(basket.total_incl_tax),
             u'currency': basket.currency,
             u'consumer_id': basket.owner.username,
+            # TODO: Update once LMS receipt page is able to look up orders by order number.
             u'override_custom_receipt_page': u'{}?basket_id={}'.format(self.receipt_page_url, basket.id),
             u'override_custom_cancel_page': self.cancel_page_url,
         }
@@ -244,7 +245,7 @@ class Cybersource(BasePaymentProcessor):
             purchase_totals.currency = currency
             purchase_totals.grandTotalAmount = unicode(amount)
 
-            response = client.service.runTransaction(merchantID=self.merchant_id, merchantReferenceCode=order.basket.id,
+            response = client.service.runTransaction(merchantID=self.merchant_id, merchantReferenceCode=order.number,
                                                      orderRequestToken=order_request_token,
                                                      ccCreditService=credit_service,
                                                      purchaseTotals=purchase_totals)

ecommerce/extensions/payment/processors/paypal.py

 """ PayPal payment processing. """
-
 from decimal import Decimal
 import logging
 from urlparse import urljoin
@@ -13,6 +12,7 @@ import paypalrestsdk
 from ecommerce.extensions.order.constants import PaymentEventTypeName
 from ecommerce.extensions.payment.processors import BasePaymentProcessor
 
+
 logger = logging.getLogger(__name__)
 
 PaymentEvent = get_model('order', 'PaymentEvent')
@@ -90,7 +90,7 @@ class Paypal(BasePaymentProcessor):
                         for line in basket.all_lines()
                     ],
                 },
-                'invoice_number': unicode(basket.id),
+                'invoice_number': basket.order_number,
             }],
         }
 

ecommerce/extensions/payment/tests/mixins.py

@@ -12,6 +12,7 @@ from suds.sudsobject import Factory
 from ecommerce.extensions.payment.constants import CARD_TYPES
 from ecommerce.extensions.payment.helpers import sign
 
+
 Order = get_model('order', 'Order')
 PaymentProcessorResponse = get_model('payment', 'PaymentProcessorResponse')
 
@@ -99,7 +100,7 @@ class CybersourceMixin(object):
                               **kwargs):
         """ Generates a dict containing the API reply fields expected to be received from CyberSource. """
 
-        req_reference_number = kwargs.get('req_reference_number', unicode(basket.id))
+        req_reference_number = kwargs.get('req_reference_number', basket.order_number)
         total = unicode(basket.total_incl_tax)
         auth_amount = auth_amount or total
         notification = {
@@ -206,7 +207,6 @@ class PaypalMixin(object):
     def mock_payment_creation_response(self, basket, state=PAYMENT_CREATION_STATE, approval_url=APPROVAL_URL,
                                        find=False):
         total = unicode(basket.total_incl_tax)
-
         payment_creation_response = {
             u'create_time': u'2015-05-04T18:18:27Z',
             u'id': self.PAYMENT_ID,
@@ -249,7 +249,7 @@ class PaypalMixin(object):
                         for line in basket.all_lines()
                     ],
                 },
-                u'invoice_number': unicode(basket.id),
+                u'invoice_number': basket.order_number,
                 u'related_resources': []
             }],
             u'update_time': u'2015-05-04T18:18:27Z'
@@ -275,7 +275,6 @@ class PaypalMixin(object):
 
     def mock_payment_execution_response(self, basket, state=PAYMENT_EXECUTION_STATE):
         total = unicode(basket.total_incl_tax)
-
         payment_execution_response = {
             u'create_time': u'2015-05-04T15:55:27Z',
             u'id': self.PAYMENT_ID,
@@ -324,7 +323,7 @@ class PaypalMixin(object):
                         for line in basket.all_lines()
                     ],
                 },
-                u'invoice_number': unicode(basket.id),
+                u'invoice_number': basket.order_number,
                 u'related_resources': [{
                     u'sale': {
                         u'amount': {
@@ -381,4 +380,4 @@ class PaypalMixin(object):
         mode = settings.PAYMENT_PROCESSOR_CONFIG['paypal']['mode']
         root = u'https://api.sandbox.paypal.com' if mode == 'sandbox' else u'https://api.paypal.com'
 
-        return root + path
+        return urljoin(root, path)

ecommerce/extensions/payment/tests/test_processors.py

@@ -29,6 +29,7 @@ from ecommerce.extensions.payment.processors.paypal import Paypal
 from ecommerce.extensions.payment.tests.mixins import PaymentEventsMixin, CybersourceMixin, PaypalMixin
 from ecommerce.extensions.refund.tests.mixins import RefundTestMixin
 
+
 PaymentEvent = get_model('order', 'PaymentEvent')
 PaymentEventType = get_model('order', 'PaymentEventType')
 Source = get_model('payment', 'Source')
@@ -114,7 +115,7 @@ class CybersourceTests(CybersourceMixin, PaymentProcessorTestCaseMixin, TestCase
             u'signed_date_time': self.PI_DAY.strftime(ISO_8601_FORMAT),
             u'locale': settings.LANGUAGE_CODE,
             u'transaction_type': u'sale',
-            u'reference_number': unicode(self.basket.id),
+            u'reference_number': self.basket.order_number,
             u'amount': unicode(self.basket.total_incl_tax),
             u'currency': self.basket.currency,
             u'consumer_id': self.basket.owner.username,

ecommerce/extensions/payment/tests/test_views.py

 """ Tests of the Payment Views. """
-
 import ddt
+from django.conf import settings
 from django.core.urlresolvers import reverse
 from django.test import TestCase
 from django.test.client import RequestFactory
@@ -9,7 +9,7 @@ import httpretty
 import mock
 from oscar.apps.order.exceptions import UnableToPlaceOrder
 from oscar.apps.payment.exceptions import PaymentError
-from oscar.core.loading import get_model, get_class
+from oscar.core.loading import get_class, get_model
 from oscar.test import factories
 from oscar.test.contextmanagers import mock_signal_receiver
 
@@ -19,6 +19,7 @@ from ecommerce.extensions.payment.processors.paypal import Paypal
 from ecommerce.extensions.payment.tests.mixins import PaymentEventsMixin, CybersourceMixin, PaypalMixin
 from ecommerce.extensions.payment.views import CybersourceNotifyView, PaypalPaymentExecutionView
 
+
 Basket = get_model('basket', 'Basket')
 Order = get_model('order', 'Order')
 PaymentEvent = get_model('order', 'PaymentEvent')
@@ -148,15 +149,15 @@ class CybersourceNotifyViewTests(CybersourceMixin, PaymentEventsMixin, TestCase)
         self.assert_processor_response_recorded(self.processor_name, notification[u'transaction_id'], notification,
                                                 basket=self.basket)
 
-    @ddt.data('abc', '1986', '')
-    def test_invalid_basket(self, basket_id):
+    def test_invalid_basket(self):
         """ When payment is accepted for a non-existent basket, log an error and record the response. """
+        order_number = '{}-{}'.format(settings.ORDER_NUMBER_PREFIX, 101986)
 
         notification = self.generate_notification(
             self.processor.secret_key,
             self.basket,
             billing_address=self.billing_address,
-            req_reference_number=basket_id,
+            req_reference_number=order_number,
         )
         response = self.client.post(reverse('cybersource_notify'), notification)
 

ecommerce/extensions/payment/views.py

@@ -17,12 +17,14 @@ from ecommerce.extensions.payment.exceptions import InvalidSignatureError
 from ecommerce.extensions.payment.processors.cybersource import Cybersource
 from ecommerce.extensions.payment.processors.paypal import Paypal
 
+
 logger = logging.getLogger(__name__)
 
 Basket = get_model('basket', 'Basket')
 BillingAddress = get_model('order', 'BillingAddress')
 Country = get_model('address', 'Country')
 NoShippingRequired = get_class('shipping.methods', 'NoShippingRequired')
+OrderNumberGenerator = get_class('order.utils', 'OrderNumberGenerator')
 OrderTotalCalculator = get_class('checkout.calculators', 'OrderTotalCalculator')
 PaymentProcessorResponse = get_model('payment', 'PaymentProcessorResponse')
 
@@ -76,7 +78,8 @@ class CybersourceNotifyView(EdxOrderPlacementMixin, View):
 
         try:
             transaction_id = cybersource_response.get('transaction_id')
-            basket_id = cybersource_response.get('req_reference_number')
+            order_number = cybersource_response.get('req_reference_number')
+            basket_id = OrderNumberGenerator().basket_id(order_number)
 
             basket = self._get_basket(basket_id)
 
@@ -113,7 +116,6 @@ class CybersourceNotifyView(EdxOrderPlacementMixin, View):
 
         try:
             user = basket.owner
-            order_number = self.generate_order_number(basket)
             self.handle_order_placement(order_number, user, basket, None, shipping_method, shipping_charge,
                                         billing_address, order_total)
         except UnableToPlaceOrder:
@@ -160,7 +162,10 @@ class PaypalPaymentExecutionView(EdxOrderPlacementMixin, View):
 
         try:
             user = basket.owner
-            order_number = self.generate_order_number(basket)
+            # Given a basket, order number generation is idempotent. Although we've already
+            # generated this order number once before, it's faster to generate it again
+            # than to retrieve an invoice number from PayPal.
+            order_number = basket.order_number
 
             self.handle_order_placement(
                 order_number=order_number,

ecommerce/settings/_oscar.py

@@ -24,6 +24,7 @@ OSCAR_APPS = [
     'ecommerce.extensions.refund',
 ] + get_core_apps([
     'ecommerce.extensions.analytics',
+    'ecommerce.extensions.basket',
     'ecommerce.extensions.catalogue',
     'ecommerce.extensions.checkout',
     'ecommerce.extensions.dashboard',