Merge pull request #54 from open-craft/fix-attribute-error-in-lms

Don't cause a 500 error in the LMS if get_block unexpectedly returns None

problem_builder/mentoring.py

@@ -236,7 +236,9 @@ class MentoringBlock(XBlock, StepParentMixin, StudioEditableXBlockMixin, StudioC
 
         for child_id in self.children:
             child = self.runtime.get_block(child_id)
-            if not isinstance(child, MentoringMessageBlock):
+            if child is None:  # child should not be None but it can happen due to bugs or permission issues
+                child_content += u"<p>[{}]</p>".format(self._(u"Error: Unable to load child component."))
+            elif not isinstance(child, MentoringMessageBlock):
                 try:
                     if self.is_assessment and isinstance(child, StepMixin):
                         child_fragment = child.render('assessment_step_view', context)

problem_builder/tests/integration/test_progression.py

@@ -19,7 +19,6 @@
 #
 
 # Imports ###########################################################
-
 from .base_test import MentoringBaseTest
 
 
@@ -34,8 +33,10 @@ class MentoringProgressionTest(MentoringBaseTest):
         """
         self.assertEqual(warning_dom.text, 'You need to complete the previous step before attempting this step.')
         warning_link = warning_dom.find_element_by_xpath('./*')
-        link_href = 'http://localhost:8081{}'.format(link_href)
-        self.assertEqual(warning_link.get_attribute('href'), link_href)
+        self.assertTrue(
+            warning_link.get_attribute('href').endswith(link_href),
+            "expected link href '{}' to end with '{}'".format(warning_link.get_attribute('href'), link_href)
+        )
 
     def assert_warning_is_hidden(self, mentoring):
         for elem in mentoring.find_elements_by_css_selector('.warning'):

problem_builder/tests/unit/test_mentoring.py

@@ -31,6 +31,20 @@ class TestMentoringBlock(unittest.TestCase):
 
             self.assertFalse(patched_runtime.publish.called)
 
+    def test_does_not_crash_when_get_child_is_broken(self):
+        block = MentoringBlock(MagicMock(), DictFieldData({
+            'children': ['invalid_id'],
+        }), Mock())
+
+        with patch.object(block, 'runtime') as patched_runtime:
+            patched_runtime.publish = Mock()
+            patched_runtime.service().ugettext = lambda str: str
+            patched_runtime.get_block = lambda block_id: None
+
+            fragment = block.student_view(context={})
+
+            self.assertIn('Unable to load child component', fragment.content)
+
 
 @ddt.ddt
 class TestMentoringBlockTheming(unittest.TestCase):