update _xmodule_recurse to ignore some exception

cms/djangoapps/contentstore/views/helpers.py

@@ -4,6 +4,7 @@ from django.http import HttpResponse
 from django.shortcuts import redirect
 from edxmako.shortcuts import render_to_string, render_to_response
 from xmodule.modulestore.django import loc_mapper, modulestore
+from xmodule.modulestore.exceptions import DuplicateItemError, ItemNotFoundError
 
 __all__ = ['edge', 'event', 'landing']
 
@@ -36,9 +37,22 @@ 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):
+def _xmodule_recurse(item, action, ignore_exception=None):
+    """
+    Recursively apply provided action on item and its children
+
+    ignore_duplicate_exception (str): A optional argument; when set to a certain value then ignores the corresponding
+        exception raised while xmodule recursion
+    """
     for child in item.get_children():
-        _xmodule_recurse(child, action)
+        _xmodule_recurse(child, action, ignore_exception)
+
+    if ignore_exception in ['ItemNotFoundError', 'DuplicateItemError']:
+        # In case of valid provided exception; ignore it and continue recursion
+        try:
+            return action(item)
+        except (ItemNotFoundError, DuplicateItemError):
+            return
 
     action(item)
 

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, DuplicateItemError
+from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError
 from xmodule.modulestore.inheritance import own_metadata
 from xmodule.modulestore.locator import BlockUsageLocator
 from xmodule.modulestore import Location
@@ -310,15 +310,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)
-            try:
-                _xmodule_recurse(existing_item, lambda i: modulestore().convert_to_draft(i.location))
-            except DuplicateItemError:
-                # Since there is already a draft version of this module so no need to create another
-                pass
+            _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

@@ -654,7 +654,7 @@ class TestEditItem(ItemTest):
         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.
+        # user gets that already created draft instead of getting 'DuplicateItemError' exception.
         self.client.ajax_post(
             self.problem_update_url,
             data={
@@ -665,6 +665,47 @@ class TestEditItem(ItemTest):
         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 """