Merge pull request #3766 from edx/zub/bugfix/std1616-editdraft

create draft request gets already created version if it exists

cms/djangoapps/contentstore/views/helpers.py

@@ -38,11 +38,20 @@ def render_from_lms(template_name, dictionary, context=None, namespace='main'):
     return render_to_string(template_name, dictionary, context, namespace="lms." + namespace)
 
 
-def _xmodule_recurse(item, action):
-    for child in item.get_children():
-        _xmodule_recurse(child, action)
+def _xmodule_recurse(item, action, ignore_exception=()):
+    """
+    Recursively apply provided action on item and its children
 
-    action(item)
+    ignore_exception (Exception Object): A optional argument; when passed ignores the corresponding
+        exception raised during xmodule recursion,
+    """
+    for child in item.get_children():
+        _xmodule_recurse(child, action, ignore_exception)
+
+    try:
+        return action(item)
+    except ignore_exception:
+        return
 
 
 def get_parent_xblock(xblock):

cms/djangoapps/contentstore/views/item.py

@@ -21,7 +21,7 @@ from xblock.fragment import Fragment
 
 import xmodule
 from xmodule.modulestore.django import modulestore, loc_mapper
-from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError
+from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError, DuplicateItemError
 from xmodule.modulestore.inheritance import own_metadata
 from xmodule.modulestore.locator import BlockUsageLocator
 from xmodule.modulestore import Location
@@ -321,11 +321,19 @@ def _save_item(request, usage_loc, item_location, data=None, children=None, meta
 
     if publish:
         if publish == 'make_private':
-            _xmodule_recurse(existing_item, lambda i: modulestore().unpublish(i.location))
+            _xmodule_recurse(
+                existing_item,
+                lambda i: modulestore().unpublish(i.location),
+                ignore_exception=ItemNotFoundError
+            )
         elif publish == 'create_draft':
             # This recursively clones the existing item location to a draft location (the draft is
             # implicit, because modulestore is a Draft modulestore)
-            _xmodule_recurse(existing_item, lambda i: modulestore().convert_to_draft(i.location))
+            _xmodule_recurse(
+                existing_item,
+                lambda i: modulestore().convert_to_draft(i.location),
+                ignore_exception=DuplicateItemError
+            )
 
     if data:
         # TODO Allow any scope.content fields not just "data" (exactly like the get below this)

cms/djangoapps/contentstore/views/tests/test_item.py

@@ -632,6 +632,80 @@ class TestEditItem(ItemTest):
         draft = self.get_item_from_modulestore(self.problem_locator, True)
         self.assertEqual(draft.due, datetime(2077, 10, 10, 4, 0, tzinfo=UTC))
 
+    def test_create_draft_with_multiple_requests(self):
+        """
+        Create a draft request returns already created version if it exists.
+        """
+        # Make problem public.
+        self.client.ajax_post(
+            self.problem_update_url,
+            data={'publish': 'make_public'}
+        )
+        self.assertIsNotNone(self.get_item_from_modulestore(self.problem_locator, False))
+        # Now make it draft, which means both versions will exist.
+        self.client.ajax_post(
+            self.problem_update_url,
+            data={
+                'publish': 'create_draft'
+            }
+        )
+        self.assertIsNotNone(self.get_item_from_modulestore(self.problem_locator, False))
+        draft_1 = self.get_item_from_modulestore(self.problem_locator, True)
+        self.assertIsNotNone(draft_1)
+
+        # Now check that when a user sends request to create a draft when there is already a draft version then
+        # user gets that already created draft instead of getting 'DuplicateItemError' exception.
+        self.client.ajax_post(
+            self.problem_update_url,
+            data={
+                'publish': 'create_draft'
+            }
+        )
+        draft_2 = self.get_item_from_modulestore(self.problem_locator, True)
+        self.assertIsNotNone(draft_2)
+        self.assertEqual(draft_1, draft_2)
+
+
+    def test_make_private_with_multiple_requests(self):
+        """
+        Make private requests gets proper response even if xmodule is already made private.
+        """
+        # Make problem public.
+        self.client.ajax_post(
+            self.problem_update_url,
+            data={'publish': 'make_public'}
+        )
+        self.assertIsNotNone(self.get_item_from_modulestore(self.problem_locator, False))
+
+        # Now make it private, and check that its published version not exists
+        resp = self.client.ajax_post(
+            self.problem_update_url,
+            data={
+                'publish': 'make_private'
+            }
+        )
+        self.assertEqual(resp.status_code, 200)
+        with self.assertRaises(ItemNotFoundError):
+            self.get_item_from_modulestore(self.problem_locator, False)
+        draft_1 = self.get_item_from_modulestore(self.problem_locator, True)
+        self.assertIsNotNone(draft_1)
+
+        # Now check that when a user sends request to make it private when it already is private then
+        # user gets that private version instead of getting 'ItemNotFoundError' exception.
+        self.client.ajax_post(
+            self.problem_update_url,
+            data={
+                'publish': 'make_private'
+            }
+        )
+        self.assertEqual(resp.status_code, 200)
+        with self.assertRaises(ItemNotFoundError):
+            self.get_item_from_modulestore(self.problem_locator, False)
+        draft_2 = self.get_item_from_modulestore(self.problem_locator, True)
+        self.assertIsNotNone(draft_2)
+        self.assertEqual(draft_1, draft_2)
+
+
     def test_published_and_draft_contents_with_update(self):
         """ Create a draft and publish it then modify the draft and check that published content is not modified """