From 783fb23fe23e2f4e349c12d3de6ab4530efb336c Mon Sep 17 00:00:00 2001
From: Clinton Blackburn <cblackburn@edx.org>
Date: Wed, 18 Mar 2015 10:59:21 -0400
Subject: [PATCH] Fixed CourseMode Filtering Bug

Commerce code has now been updated to exclude CourseMode objects with SKU set to empty string (in addition to null).
---
 lms/djangoapps/commerce/tests.py | 34 +++++++++++++++++++++++---------
 lms/djangoapps/commerce/views.py |  3 ++-
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/lms/djangoapps/commerce/tests.py b/lms/djangoapps/commerce/tests.py
index b3f79fbd91..b1a4b67dac 100644
--- a/lms/djangoapps/commerce/tests.py
+++ b/lms/djangoapps/commerce/tests.py
@@ -203,17 +203,10 @@ class OrdersViewTests(ModuleStoreTestCase):
         # TODO Eventually we should NOT be enrolling users directly from this view.
         self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course.id))
 
-    @httpretty.activate
-    def test_course_without_sku(self):
+    def _test_course_without_sku(self):
         """
-        If the course does NOT have a SKU, the user should be enrolled in the course (under the honor mode) and
-        redirected to the user dashboard.
+        Validates the view bypasses the E-Commerce API when the course has no CourseModes with SKUs.
         """
-        # Remove SKU from all course modes
-        for course_mode in CourseMode.objects.filter(course_id=self.course.id):
-            course_mode.sku = None
-            course_mode.save()
-
         # Place an order
         self._mock_ecommerce_api()
         response = self._post_to_view()
@@ -228,6 +221,19 @@ class OrdersViewTests(ModuleStoreTestCase):
         self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course.id))
         self.assertIsInstance(httpretty.last_request(), HTTPrettyRequestEmpty)
 
+    @httpretty.activate
+    def test_course_without_sku(self):
+        """
+        If the course does NOT have a SKU, the user should be enrolled in the course (under the honor mode) and
+        redirected to the user dashboard.
+        """
+        # Remove SKU from all course modes
+        for course_mode in CourseMode.objects.filter(course_id=self.course.id):
+            course_mode.sku = None
+            course_mode.save()
+
+        self._test_course_without_sku()
+
     @httpretty.activate
     @override_settings(ECOMMERCE_API_URL=None, ECOMMERCE_API_SIGNING_KEY=None)
     def test_no_settings(self):
@@ -245,3 +251,13 @@ class OrdersViewTests(ModuleStoreTestCase):
         # Ensure that the user is not enrolled and that no calls were made to the E-Commerce API
         self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course.id))
         self.assertIsInstance(httpretty.last_request(), HTTPrettyRequestEmpty)
+
+    @httpretty.activate
+    def test_empty_sku(self):
+        """ If the CourseMode has an empty string for a SKU, the API should not be used. """
+        # Set SKU to empty string for all modes.
+        for course_mode in CourseMode.objects.filter(course_id=self.course.id):
+            course_mode.sku = ''
+            course_mode.save()
+
+        self._test_course_without_sku()
diff --git a/lms/djangoapps/commerce/views.py b/lms/djangoapps/commerce/views.py
index e029998e9e..ba4b82bdfa 100644
--- a/lms/djangoapps/commerce/views.py
+++ b/lms/djangoapps/commerce/views.py
@@ -91,7 +91,8 @@ class OrdersView(APIView):
 
         # Default to honor mode. In the future we may expand this view to support additional modes.
         mode = CourseMode.DEFAULT_MODE_SLUG
-        course_modes = CourseMode.objects.filter(course_id=course_key, mode_slug=mode, sku__isnull=False)
+        course_modes = CourseMode.objects.filter(course_id=course_key, mode_slug=mode)\
+            .exclude(sku__isnull=True).exclude(sku__exact='')
 
         # If there are no course modes with SKUs, enroll the user without contacting the external API.
         if not course_modes.exists():