Commit 026a5cea by Renzo Lucioni

Never save seats if LMS publication is not able to complete

Avoids the possibility of ghost SKUs on the LMS. XCOM-534.
parent 3c8846f7
...@@ -226,19 +226,24 @@ class AtomicPublicationSerializer(serializers.Serializer): # pylint: disable=ab ...@@ -226,19 +226,24 @@ class AtomicPublicationSerializer(serializers.Serializer): # pylint: disable=ab
u'An error occurred while publishing [{course_id}] to LMS. ' u'An error occurred while publishing [{course_id}] to LMS. '
u'No data has been saved or published.' u'No data has been saved or published.'
).format(course_id=course_id) ).format(course_id=course_id)
raise Exception(message) raise Exception(message)
else: else:
message = ( message = (
u'Course [{course_id}] was not published to LMS ' u'Course [{course_id}] was not published to LMS '
u'because the switch [publish_course_modes_to_lms] is disabled. ' 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) ).format(course_id=course_id)
logger.info(message)
return created, None, message raise Exception(message)
except Exception as e: # pylint: disable=broad-except except Exception as e: # pylint: disable=broad-except
logger.exception(u'Failed to save and publish [%s]: [%s]', course_id, e.message) 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): def _flatten(self, attrs):
"""Transform a list of attribute names and values into a dictionary keyed on the names.""" """Transform a list of attribute names and values into a dictionary keyed on the names."""
......
...@@ -155,12 +155,11 @@ class AtomicPublicationTests(CourseCatalogTestMixin, UserMixin, TestCase): ...@@ -155,12 +155,11 @@ class AtomicPublicationTests(CourseCatalogTestMixin, UserMixin, TestCase):
def test_create(self): def test_create(self):
"""Verify that a Course and associated products can be created and published.""" """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) response = self.client.post(self.create_path, json.dumps(self.data), JSON_CONTENT_TYPE)
self.assertEqual(response.status_code, 201) self.assertEqual(response.status_code, 500)
self._assert_course_saved(self.course_id, expected=self.data) self._assert_course_saved(self.course_id)
self._purge_courses()
self._toggle_publication(True) self._toggle_publication(True)
with mock.patch.object(LMSPublisher, 'publish') as mock_publish: with mock.patch.object(LMSPublisher, 'publish') as mock_publish:
...@@ -181,13 +180,11 @@ class AtomicPublicationTests(CourseCatalogTestMixin, UserMixin, TestCase): ...@@ -181,13 +180,11 @@ class AtomicPublicationTests(CourseCatalogTestMixin, UserMixin, TestCase):
self._purge_courses(recreate=True) self._purge_courses(recreate=True)
updated_data = self._update_data() 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) response = self.client.put(self.update_path, json.dumps(updated_data), JSON_CONTENT_TYPE)
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 500)
self._assert_course_saved(self.course_id, expected=updated_data) self._assert_course_saved(self.course_id, expected=self.data)
self._purge_courses(recreate=True)
updated_data = self._update_data()
self._toggle_publication(True) self._toggle_publication(True)
with mock.patch.object(LMSPublisher, 'publish') as mock_publish: with mock.patch.object(LMSPublisher, 'publish') as mock_publish:
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment