Clean up models, add some error handling

common/djangoapps/course_modes/models.py

@@ -62,7 +62,7 @@ class CourseMode(models.Model):
         """
         modes = cls.modes_for_course(course_id)
 
-        matched = filter(lambda m: m.slug == mode_slug, modes)
+        matched = [m for m in modes if m.slug == mode_slug]
         if matched:
             return matched[0]
         else:

lms/djangoapps/shoppingcart/migrations/0001_initial.py

@@ -59,7 +59,6 @@ class Migration(SchemaMigration):
         ))
         db.send_create_signal('shoppingcart', ['CertificateItem'])
 
-
     def backwards(self, orm):
         # Deleting model 'Order'
         db.delete_table('shoppingcart_order')
@@ -73,7 +72,6 @@ class Migration(SchemaMigration):
         # Deleting model 'CertificateItem'
         db.delete_table('shoppingcart_certificateitem')
 
-
     models = {
         'auth.group': {
             'Meta': {'object_name': 'Group'},
@@ -165,4 +163,4 @@ class Migration(SchemaMigration):
         }
     }
 
-    complete_apps = ['shoppingcart']
+    complete_apps = ['shoppingcart']

lms/djangoapps/shoppingcart/migrations/0002_auto__add_field_paidcourseregistration_mode.py

@@ -13,12 +13,10 @@ class Migration(SchemaMigration):
                       self.gf('django.db.models.fields.SlugField')(default='honor', max_length=50),
                       keep_default=False)
 
-
     def backwards(self, orm):
         # Deleting field 'PaidCourseRegistration.mode'
         db.delete_column('shoppingcart_paidcourseregistration', 'mode')
 
-
     models = {
         'auth.group': {
             'Meta': {'object_name': 'Group'},
@@ -111,4 +109,4 @@ class Migration(SchemaMigration):
         }
     }
 
-    complete_apps = ['shoppingcart']
+    complete_apps = ['shoppingcart']

lms/djangoapps/shoppingcart/migrations/0003_auto__del_field_orderitem_line_cost.py

@@ -11,14 +11,12 @@ class Migration(SchemaMigration):
         # Deleting field 'OrderItem.line_cost'
         db.delete_column('shoppingcart_orderitem', 'line_cost')
 
-
     def backwards(self, orm):
         # Adding field 'OrderItem.line_cost'
         db.add_column('shoppingcart_orderitem', 'line_cost',
                       self.gf('django.db.models.fields.DecimalField')(default=0.0, max_digits=30, decimal_places=2),
                       keep_default=False)
 
-
     models = {
         'auth.group': {
             'Meta': {'object_name': 'Group'},
@@ -110,4 +108,4 @@ class Migration(SchemaMigration):
         }
     }
 
-    complete_apps = ['shoppingcart']
+    complete_apps = ['shoppingcart']

lms/djangoapps/shoppingcart/models.py

@@ -5,6 +5,7 @@ from django.db import models
 from django.core.exceptions import ObjectDoesNotExist
 from django.contrib.auth.models import User
 from django.utils.translation import ugettext as _
+from django.db import transaction
 from model_utils.managers import InheritanceManager
 from courseware.courses import get_course_about_section
 
@@ -113,8 +114,8 @@ class Order(models.Model):
         orderitems = OrderItem.objects.filter(order=self).select_subclasses()
         for item in orderitems:
             item.status = 'purchased'
-            item.purchased_callback()
             item.save()
+            item.purchased_callback()
 
 
 class OrderItem(models.Model):
@@ -138,23 +139,23 @@ class OrderItem(models.Model):
 
     @property
     def line_cost(self):
+        """ Return the total cost of this OrderItem """
         return self.qty * self.unit_cost
 
     @classmethod
-    def add_to_order(cls, *args, **kwargs):
+    def add_to_order(cls, order, *args, **kwargs):
         """
         A suggested convenience function for subclasses.
 
-        NOTE: This does not add anything items to the cart. That is left up to the subclasses
+        NOTE: This does not add anything to the cart. That is left up to the
+        subclasses to implement for themselves
         """
         # this is a validation step to verify that the currency of the item we
         # are adding is the same as the currency of the order we are adding it
         # to
-        if isinstance(args[0], Order):
-            currency = kwargs['currency'] if 'currency' in kwargs else 'usd'
-            order = args[0]
-            if order.currency != currency and order.orderitem_set.count() > 0:
-                raise InvalidCartItem(_("Trying to add a different currency into the cart"))
+        currency = kwargs.get('currency', 'usd')
+        if order.currency != currency and order.orderitem_set.exists():
+            raise InvalidCartItem(_("Trying to add a different currency into the cart"))
 
     def purchased_callback(self):
         """
@@ -180,6 +181,7 @@ class PaidCourseRegistration(OrderItem):
                              for item in order.orderitem_set.all().select_subclasses("paidcourseregistration")]
 
     @classmethod
+    @transaction.commit_on_success
     def add_to_order(cls, order, course_id, mode_slug=CourseMode.DEFAULT_MODE_SLUG, cost=None, currency=None):
         """
         A standardized way to create these objects, with sensible defaults filled in.
@@ -254,6 +256,7 @@ class CertificateItem(OrderItem):
     mode = models.SlugField()
 
     @classmethod
+    @transaction.commit_on_success
     def add_to_order(cls, order, course_id, cost, mode, currency='usd'):
         """
         Add a CertificateItem to an order