Merge pull request #8477 from edx/usman/plat676-bulk_operations-2

In bulk_operations() exit, emit signals at the end.

common/lib/xmodule/xmodule/modulestore/__init__.py

@@ -221,7 +221,8 @@ class BulkOperationsMixin(object):
         """
         Clear the record for this course
         """
-        del self._active_bulk_ops.records[course_key.for_branch(None)]
+        if course_key.for_branch(None) in self._active_bulk_ops.records:
+            del self._active_bulk_ops.records[course_key.for_branch(None)]
 
     def _start_outermost_bulk_operation(self, bulk_ops_record, course_key):
         """
@@ -245,7 +246,7 @@ class BulkOperationsMixin(object):
         if bulk_ops_record.is_root:
             self._start_outermost_bulk_operation(bulk_ops_record, course_key)
 
-    def _end_outermost_bulk_operation(self, bulk_ops_record, structure_key, emit_signals=True):
+    def _end_outermost_bulk_operation(self, bulk_ops_record, structure_key):
         """
         The outermost nested bulk_operation call: do the actual end of the bulk operation.
 
@@ -269,10 +270,14 @@ class BulkOperationsMixin(object):
         if bulk_ops_record.active:
             return
 
-        self._end_outermost_bulk_operation(bulk_ops_record, structure_key, emit_signals)
+        dirty = self._end_outermost_bulk_operation(bulk_ops_record, structure_key)
 
         self._clear_bulk_ops_record(structure_key)
 
+        if emit_signals and dirty:
+            self.send_bulk_published_signal(bulk_ops_record, structure_key)
+            self.send_bulk_library_updated_signal(bulk_ops_record, structure_key)
+
     def _is_in_bulk_operation(self, course_key, ignore_case=False):
         """
         Return whether a bulk operation is active on `course_key`.

common/lib/xmodule/xmodule/modulestore/mongo/base.py

@@ -476,19 +476,17 @@ class MongoBulkOpsMixin(BulkOperationsMixin):
         # ensure it starts clean
         bulk_ops_record.dirty = False
 
-    def _end_outermost_bulk_operation(self, bulk_ops_record, structure_key, emit_signals=True):
+    def _end_outermost_bulk_operation(self, bulk_ops_record, structure_key):
         """
         Restart updating the meta-data inheritance cache for the given course or library.
         Refresh the meta-data inheritance cache now since it was temporarily disabled.
         """
+        dirty = False
         if bulk_ops_record.dirty:
             self.refresh_cached_metadata_inheritance_tree(structure_key)
-
-            if emit_signals:
-                self.send_bulk_published_signal(bulk_ops_record, structure_key)
-                self.send_bulk_library_updated_signal(bulk_ops_record, structure_key)
-
+            dirty = True
             bulk_ops_record.dirty = False  # brand spanking clean now
+        return dirty
 
     def _is_in_bulk_operation(self, course_id, ignore_case=False):
         """

common/lib/xmodule/xmodule/modulestore/split_mongo/split.py

@@ -231,7 +231,7 @@ class SplitBulkWriteMixin(BulkOperationsMixin):
         bulk_write_record.index = copy.deepcopy(bulk_write_record.initial_index)
         bulk_write_record.course_key = course_key
 
-    def _end_outermost_bulk_operation(self, bulk_write_record, structure_key, emit_signals=True):
+    def _end_outermost_bulk_operation(self, bulk_write_record, structure_key):
         """
         End the active bulk write operation on structure_key (course or library key).
         """
@@ -273,9 +273,7 @@ class SplitBulkWriteMixin(BulkOperationsMixin):
                     course_context=bulk_write_record.course_key
                 )
 
-        if dirty and emit_signals:
-            self.send_bulk_published_signal(bulk_write_record, structure_key)
-            self.send_bulk_library_updated_signal(bulk_write_record, structure_key)
+        return dirty
 
     def get_course_index(self, course_key, ignore_case=False):
         """

common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py

@@ -1989,6 +1989,49 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
             dest_store = self.store._get_modulestore_by_type(ModuleStoreEnum.Type.split)
             self.assertCoursesEqual(source_store, source_course_key, dest_store, dest_course_id)
 
+    @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
+    def test_bulk_operations_signal_firing(self, default):
+        """ Signals should be fired right before bulk_operations() exits. """
+        with MongoContentstoreBuilder().build() as contentstore:
+            self.store = MixedModuleStore(
+                contentstore=contentstore,
+                create_modulestore_instance=create_modulestore_instance,
+                mappings={},
+                signal_handler=SignalHandler(MixedModuleStore),
+                **self.OPTIONS
+            )
+            self.addCleanup(self.store.close_all_connections)
+
+            with self.store.default_store(default):
+
+                with mock_signal_receiver(SignalHandler.course_published) as receiver:
+                    self.assertEqual(receiver.call_count, 0)
+
+                    # Course creation and publication should fire the signal
+                    course = self.store.create_course('org_x', 'course_y', 'run_z', self.user_id)
+                    self.assertEqual(receiver.call_count, 1)
+                    receiver.reset_mock()
+
+                    course_key = course.id
+
+                    def _clear_bulk_ops_record(course_key):  # pylint: disable=unused-argument
+                        """ Check if the signal has been fired. """
+                        self.assertEqual(receiver.call_count, 0)
+
+                    with patch.object(
+                        self.store.thread_cache.default_store, '_clear_bulk_ops_record', wraps=_clear_bulk_ops_record
+                    ) as mock_clear_bulk_ops_record:
+
+                        with self.store.bulk_operations(course_key):
+                            categories = DIRECT_ONLY_CATEGORIES
+                            for block_type in categories:
+                                self.store.create_item(self.user_id, course_key, block_type)
+                                self.assertEqual(receiver.call_count, 0)
+
+                        self.assertEqual(mock_clear_bulk_ops_record.call_count, 1)
+
+                    self.assertEqual(receiver.call_count, 1)
+
     @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
     def test_course_publish_signal_direct_firing(self, default):
         with MongoContentstoreBuilder().build() as contentstore: