Merge pull request #374 from mitodl/mitodl/aside_block_filtering

Added capability for XBlockAsides to apply only to XBlocks that match certain conditions

CHANGELOG.rst

@@ -4,6 +4,11 @@ Change history for XBlock
 
 These are notable changes in XBlock.
 
+1.2.0 - Aside filtering
+--------------
+
+* Add capability for XBlockAsides to apply only to XBlocks that match certain conditions
+
 1.0 - Python 3
 --------------
 

script/max_pylint_violations

@@ -1,5 +1,5 @@
 #!/bin/bash
-DEFAULT_MAX=7
+DEFAULT_MAX=21
 
 pylint xblock | tee /tmp/pylint-xblock.log
 ERR=`grep -E "^[C|R|W|E]:" /tmp/pylint-xblock.log | wc -l`

xblock/VERSION.txt

@@ -1 +1 @@
-1.1.1
+1.2.0

xblock/core.py

@@ -256,6 +256,14 @@ def _decorator(func):  # pylint: disable=missing-docstring
             return func
         return _decorator
 
+    @classmethod
+    def should_apply_to_block(cls, block):  # pylint: disable=unused-argument
+        """
+        Return True if the aside should be applied to a given block. This can be overridden
+        if some aside should only wrap blocks with certain properties.
+        """
+        return True
+
     @class_lazy
     def _combined_asides(cls):  # pylint: disable=no-self-argument
         """

xblock/runtime.py

@@ -927,15 +927,21 @@ def create_aside(self, block_type, keys):
 
     def get_asides(self, block):
         """
-        Return all of the asides which might be decorating this `block`.
+        Return instances for all of the asides that will decorate this `block`.
 
         Arguments:
             block (:class:`.XBlock`): The block to render retrieve asides for.
+
+        Returns:
+            List of XBlockAside instances
         """
-        return [
+        aside_instances = [
             self.get_aside_of_type(block, aside_type)
-            for aside_type
-            in self.applicable_aside_types(block)
+            for aside_type in self.applicable_aside_types(block)
+        ]
+        return [
+            aside_instance for aside_instance in aside_instances
+            if aside_instance.should_apply_to_block(block)
         ]
 
     # pylint: disable=unused-argument

xblock/test/test_asides.py

@@ -32,6 +32,16 @@ def student_view_aside(self, block, context):  # pylint: disable=unused-argument
         """Add to the student view"""
         return Fragment(self.FRAG_CONTENT)
 
+    @XBlockAside.aside_for('studio_view')
+    def studio_view_aside(self, block, context):  # pylint: disable=unused-argument
+        """Add to the studio view"""
+        return Fragment(self.FRAG_CONTENT)
+
+    @classmethod
+    def should_apply_to_block(cls, block):
+        """Overrides base implementation for testing purposes"""
+        return block.content != 'should not apply'
+
 
 class TestInheritedAside(TestAside):
     """
@@ -66,13 +76,15 @@ def test_render_aside(self):
         """
         Test that rendering the xblock renders its aside
         """
-
         frag = self.runtime.render(self.tester, 'student_view', ["ignore"])
         self.assertIn(TestAside.FRAG_CONTENT, frag.body_html())
 
         frag = self.runtime.render(self.tester, 'author_view', ["ignore"])
         self.assertNotIn(TestAside.FRAG_CONTENT, frag.body_html())
 
+        frag = self.runtime.render(self.tester, 'studio_view', ["ignore"])
+        self.assertIn(TestAside.FRAG_CONTENT, frag.body_html())
+
     @XBlockAside.register_temp_plugin(TestAside)
     @XBlockAside.register_temp_plugin(TestInheritedAside)
     def test_inherited_aside_view(self):
@@ -88,6 +100,29 @@ def test_inherited_aside_view(self):
         self.assertNotIn(TestAside.FRAG_CONTENT, frag.body_html())
         self.assertNotIn(TestInheritedAside.FRAG_CONTENT, frag.body_html())
 
+        frag = self.runtime.render(self.tester, 'studio_view', ["ignore"])
+        self.assertIn(TestAside.FRAG_CONTENT, frag.body_html())
+        self.assertIn(TestInheritedAside.FRAG_CONTENT, frag.body_html())
+
+    @XBlockAside.register_temp_plugin(TestAside)
+    def test_aside_should_apply_to_block(self):
+        """
+        Test that should_apply_to_block returns True by default, and that it prevents
+        the aside from being applied to the given block when it returns False.
+        """
+        self.assertTrue(TestAside.should_apply_to_block(self.tester))
+        test_aside_instances = [
+            inst for inst in self.runtime.get_asides(self.tester) if isinstance(inst, TestAside)
+        ]
+        self.assertEqual(len(test_aside_instances), 1)
+
+        self.tester.content = 'should not apply'
+        self.assertFalse(TestAside.should_apply_to_block(self.tester))
+        test_aside_instances = [
+            inst for inst in self.runtime.get_asides(self.tester) if isinstance(inst, TestAside)
+        ]
+        self.assertEqual(len(test_aside_instances), 0)
+
 
 class ParsingTest(AsideRuntimeSetup, XmlTestMixin):
     """Tests of XML parsing."""