From 15965c1351ca7a3b1844369afae1ba5855789222 Mon Sep 17 00:00:00 2001 From: Martyn James Date: Thu, 5 Mar 2015 14:59:26 -0500 Subject: [PATCH] Addresses SOL-290. When display name is not present, we shall display '(Unnamed)' in its stead --- .../courseware_search/lms_result_processor.py | 19 +++++---- .../test/test_lms_result_processor.py | 39 ++++++++++++++++++- 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/lms/lib/courseware_search/lms_result_processor.py b/lms/lib/courseware_search/lms_result_processor.py index d9a2cec97b..51ffb38214 100644 --- a/lms/lib/courseware_search/lms_result_processor.py +++ b/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)) diff --git a/lms/lib/courseware_search/test/test_lms_result_processor.py b/lms/lib/courseware_search/test/test_lms_result_processor.py index c40608987b..fee8d582b9 100644 --- a/lms/lib/courseware_search/test/test_lms_result_processor.py +++ b/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)