Merge pull request #7242 from edx/mjames/SOL-290

Addresses SOL-290.

lms/lib/courseware_search/lms_result_processor.py

@@ -4,6 +4,7 @@ This file contains implementation override of SearchResultProcessor which will a
     * Confirms user access to object
 """
 from django.core.urlresolvers import reverse
+from django.utils.translation import ugettext as _
 
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 from search.result_processor import SearchResultProcessor
@@ -12,6 +13,8 @@ from xmodule.modulestore.search import path_to_location
 
 from courseware.access import has_access
 
+UNNAMED_MODULE_NAME = _("(Unnamed)")
+
 
 class LmsSearchResultProcessor(SearchResultProcessor):
 
@@ -66,25 +69,25 @@ class LmsSearchResultProcessor(SearchResultProcessor):
         # TODO: update whern changes to "cohorted-courseware" branch are merged in
         (course_key, chapter, section, position) = path_to_location(self.get_module_store(), self.get_usage_key())
 
-        def get_display_name(category, item_id):
-            """ helper to get display name from object """
-            item = self.get_item(course_key.make_usage_key(category, item_id))
-            return getattr(item, "display_name", None)
+        def get_display_name(item_key):
+            """ gets display name from object's key """
+            item = self.get_item(item_key)
+            display_name = getattr(item, "display_name", None)
+            return display_name if display_name else UNNAMED_MODULE_NAME
 
         def get_position_name(section, position):
             """ helper to fetch name corresponding to the position therein """
             pos = int(position)
             section_item = self.get_item(course_key.make_usage_key("sequential", section))
             if section_item.has_children and len(section_item.children) >= pos:
-                item = self.get_item(section_item.children[pos - 1])
-                return getattr(item, "display_name", None)
+                return get_display_name(section_item.children[pos - 1])
             return None
 
         location_description = []
         if chapter:
-            location_description.append(get_display_name("chapter", chapter))
+            location_description.append(get_display_name(course_key.make_usage_key("chapter", chapter)))
         if section:
-            location_description.append(get_display_name("sequential", section))
+            location_description.append(get_display_name(course_key.make_usage_key("sequential", section)))
         if position:
             location_description.append(get_position_name(section, position))
 

lms/lib/courseware_search/test/test_lms_result_processor.py

@@ -6,7 +6,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 
 from courseware.tests.factories import UserFactory
 
-from lms.lib.courseware_search.lms_result_processor import LmsSearchResultProcessor
+from lms.lib.courseware_search.lms_result_processor import LmsSearchResultProcessor, UNNAMED_MODULE_NAME
 
 
 class LmsSearchResultProcessorTestCase(ModuleStoreTestCase):
@@ -40,9 +40,25 @@ class LmsSearchResultProcessorTestCase(ModuleStoreTestCase):
             display_name='Test Unit',
         )
         self.html = ItemFactory.create(
-            parent=self.vertical, category='html',
+            parent=self.vertical,
+            category='html',
             display_name='Test Html control',
         )
+        self.ghost_subsection = ItemFactory.create(
+            parent=self.section,
+            category='sequential',
+            display_name=None,
+        )
+        self.ghost_vertical = ItemFactory.create(
+            parent=self.ghost_subsection,
+            category='vertical',
+            display_name=None,
+        )
+        self.ghost_html = ItemFactory.create(
+            parent=self.ghost_vertical,
+            category='html',
+            display_name='Ghost Html control',
+        )
 
     def setUp(self):
         # from nose.tools import set_trace
@@ -137,3 +153,22 @@ class LmsSearchResultProcessorTestCase(ModuleStoreTestCase):
         )
 
         self.assertEqual(srp.should_remove(self.global_staff), False)
+
+    def test_missing_display_name(self):
+        """
+        Tests that we get the correct name to include when there is an empty or null display name
+        """
+        srp = LmsSearchResultProcessor(
+            {
+                "course": unicode(self.course.id),
+                "id": unicode(self.ghost_html.scope_ids.usage_id),
+                "content": {"text": "This is html test text"}
+            },
+            "test"
+        )
+
+        location = srp.location
+        self.assertEqual(len(location), 3)
+        self.assertEqual(location[0], self.section.display_name)
+        self.assertEqual(location[1], UNNAMED_MODULE_NAME)
+        self.assertEqual(location[2], UNNAMED_MODULE_NAME)