Merge pull request #237 from edx/renzo/avoid-ghost-skus

Never save seats if LMS publication is not able to complete

ecommerce/extensions/api/serializers.py

@@ -226,19 +226,24 @@ class AtomicPublicationSerializer(serializers.Serializer):  # pylint: disable=ab
                             u'An error occurred while publishing [{course_id}] to LMS. '
                             u'No data has been saved or published.'
                         ).format(course_id=course_id)
+
                         raise Exception(message)
                 else:
                     message = (
                         u'Course [{course_id}] was not published to LMS '
                         u'because the switch [publish_course_modes_to_lms] is disabled. '
-                        u'Data has been saved, but not published.'
+                        u'To avoid ghost SKUs, data has not been saved.'
                     ).format(course_id=course_id)
-                    logger.info(message)
 
-                    return created, None, message
+                    raise Exception(message)
         except Exception as e:  # pylint: disable=broad-except
             logger.exception(u'Failed to save and publish [%s]: [%s]', course_id, e.message)
-            return False, e, e.message
+
+            user_message = (
+                u'Publication of course data to the LMS failed. '
+                u'To avoid checkout failures, this data has NOT been saved.'
+            )
+            return False, e, user_message
 
     def _flatten(self, attrs):
         """Transform a list of attribute names and values into a dictionary keyed on the names."""

ecommerce/extensions/api/v2/tests/views/test_publication.py

@@ -155,12 +155,11 @@ class AtomicPublicationTests(CourseCatalogTestMixin, UserMixin, TestCase):
 
     def test_create(self):
         """Verify that a Course and associated products can be created and published."""
-        # If LMS publication is disabled, the view should return a 201 and data should be saved.
+        # If LMS publication is disabled, the view should return a 500 and data should NOT be saved.
         response = self.client.post(self.create_path, json.dumps(self.data), JSON_CONTENT_TYPE)
-        self.assertEqual(response.status_code, 201)
-        self._assert_course_saved(self.course_id, expected=self.data)
+        self.assertEqual(response.status_code, 500)
+        self._assert_course_saved(self.course_id)
 
-        self._purge_courses()
         self._toggle_publication(True)
 
         with mock.patch.object(LMSPublisher, 'publish') as mock_publish:
@@ -181,13 +180,11 @@ class AtomicPublicationTests(CourseCatalogTestMixin, UserMixin, TestCase):
         self._purge_courses(recreate=True)
         updated_data = self._update_data()
 
-        # If LMS publication is disabled, the view should return a 200 and data should be saved.
+        # If LMS publication is disabled, the view should return a 500 and data should NOT be saved.
         response = self.client.put(self.update_path, json.dumps(updated_data), JSON_CONTENT_TYPE)
-        self.assertEqual(response.status_code, 200)
-        self._assert_course_saved(self.course_id, expected=updated_data)
+        self.assertEqual(response.status_code, 500)
+        self._assert_course_saved(self.course_id, expected=self.data)
 
-        self._purge_courses(recreate=True)
-        updated_data = self._update_data()
         self._toggle_publication(True)
 
         with mock.patch.object(LMSPublisher, 'publish') as mock_publish: