From a6130507c1c96ef4506e93da8d809281ddec46b3 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Wed, 28 Jan 2015 01:35:27 -0500 Subject: [PATCH] Mobile Video Summary API: add filter for get_children --- common/lib/xmodule/xmodule/x_module.py | 5 +- .../mobile_api/video_outlines/serializers.py | 20 ++++-- .../mobile_api/video_outlines/tests.py | 66 +++++++++---------- 3 files changed, 52 insertions(+), 39 deletions(-) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 892e0a99ae..70c93e0ad2 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -402,7 +402,7 @@ class XModuleMixin(XBlockMixin): else: return [self.display_name_with_default] - def get_children(self): + def get_children(self, usage_key_filter=lambda location: True): """Returns a list of XBlock instances for the children of this module""" @@ -412,6 +412,9 @@ class XModuleMixin(XBlockMixin): if getattr(self, '_child_instances', None) is None: self._child_instances = [] # pylint: disable=attribute-defined-outside-init for child_loc in self.children: + # Skip if it doesn't satisfy the filter function + if not usage_key_filter(child_loc): + continue try: child = self.runtime.get_block(child_loc) if child is None: diff --git a/lms/djangoapps/mobile_api/video_outlines/serializers.py b/lms/djangoapps/mobile_api/video_outlines/serializers.py index eabf5863e4..76ed9b972e 100644 --- a/lms/djangoapps/mobile_api/video_outlines/serializers.py +++ b/lms/djangoapps/mobile_api/video_outlines/serializers.py @@ -3,6 +3,7 @@ Serializer for video outline """ from rest_framework.reverse import reverse +from xmodule.modulestore.mongo.base import BLOCK_TYPES_WITH_CHILDREN from courseware.access import has_access from edxval.api import ( @@ -14,10 +15,10 @@ class BlockOutline(object): """ Serializes course videos, pulling data from VAL and the video modules. """ - def __init__(self, course_id, start_block, categories_to_outliner, request): + def __init__(self, course_id, start_block, block_types, request): """Create a BlockOutline using `start_block` as a starting point.""" self.start_block = start_block - self.categories_to_outliner = categories_to_outliner + self.block_types = block_types self.course_id = course_id self.request = request # needed for making full URLS self.local_cache = {} @@ -143,11 +144,11 @@ class BlockOutline(object): # from the table-of-contents. continue - if curr_block.category in self.categories_to_outliner: + if curr_block.location.block_type in self.block_types: if not has_access(user, 'load', curr_block, course_key=self.course_id): continue - summary_fn = self.categories_to_outliner[curr_block.category] + summary_fn = self.block_types[curr_block.category] block_path = list(path(curr_block)) unit_url, section_url = find_urls(curr_block) @@ -159,8 +160,17 @@ class BlockOutline(object): "summary": summary_fn(self.course_id, curr_block, self.request, self.local_cache) } + def parent_or_requested_block_type(usage_key): + """ + Returns whether the usage_key's block_type is one of self.block_types or a parent type. + """ + return ( + usage_key.block_type in self.block_types or + usage_key.block_type in BLOCK_TYPES_WITH_CHILDREN + ) + if curr_block.has_children: - for block in reversed(curr_block.get_children()): + for block in reversed(curr_block.get_children(usage_key_filter=parent_or_requested_block_type)): stack.append(block) child_to_parent[block] = curr_block diff --git a/lms/djangoapps/mobile_api/video_outlines/tests.py b/lms/djangoapps/mobile_api/video_outlines/tests.py index 7cf7ebcae4..c9ec4a7bc9 100644 --- a/lms/djangoapps/mobile_api/video_outlines/tests.py +++ b/lms/djangoapps/mobile_api/video_outlines/tests.py @@ -20,42 +20,42 @@ class TestVideoAPITestCase(MobileAPITestCase): def setUp(self): super(TestVideoAPITestCase, self).setUp() self.section = ItemFactory.create( - parent_location=self.course.location, + parent=self.course, category="chapter", display_name=u"test factory section omega \u03a9", ) self.sub_section = ItemFactory.create( - parent_location=self.section.location, + parent=self.section, category="sequential", display_name=u"test subsection omega \u03a9", ) self.unit = ItemFactory.create( - parent_location=self.sub_section.location, + parent=self.sub_section, category="vertical", metadata={'graded': True, 'format': 'Homework'}, display_name=u"test unit omega \u03a9", ) self.other_unit = ItemFactory.create( - parent_location=self.sub_section.location, + parent=self.sub_section, category="vertical", metadata={'graded': True, 'format': 'Homework'}, display_name=u"test unit omega 2 \u03a9", ) self.nameless_unit = ItemFactory.create( - parent_location=self.sub_section.location, + parent=self.sub_section, category="vertical", metadata={'graded': True, 'format': 'Homework'}, display_name=None, ) self.split_unit = ItemFactory.create( - parent_location=self.sub_section.location, + parent=self.sub_section, category="vertical", display_name=u"split test vertical\u03a9", ) self.split_test = ItemFactory.create( - parent_location=self.split_unit.location, + parent=self.split_unit, category="split_test", display_name=u"split test unit" ) @@ -120,7 +120,7 @@ class TestVideoAPITestCase(MobileAPITestCase): subid, self.course) return ItemFactory.create( - parent_location=self.unit.location, + parent=self.unit, category="video", edx_video_id=self.edx_video_id, display_name=u"test video omega \u03a9", @@ -156,27 +156,27 @@ class TestNonStandardCourseStructure(MobileAPITestCase): def setUp(self): super(TestNonStandardCourseStructure, self).setUp() self.chapter_under_course = ItemFactory.create( - parent_location=self.course.location, + parent=self.course, category="chapter", display_name=u"test factory chapter under course omega \u03a9", ) self.section_under_course = ItemFactory.create( - parent_location=self.course.location, + parent=self.course, category="sequential", display_name=u"test factory section under course omega \u03a9", ) self.section_under_chapter = ItemFactory.create( - parent_location=self.chapter_under_course.location, + parent=self.chapter_under_course, category="sequential", display_name=u"test factory section under chapter omega \u03a9", ) self.vertical_under_course = ItemFactory.create( - parent_location=self.course.location, + parent=self.course, category="vertical", display_name=u"test factory vertical under course omega \u03a9", ) self.vertical_under_section = ItemFactory.create( - parent_location=self.section_under_chapter.location, + parent=self.section_under_chapter, category="vertical", display_name=u"test factory vertical under section omega \u03a9", ) @@ -187,7 +187,7 @@ class TestNonStandardCourseStructure(MobileAPITestCase): """ self.login_and_enroll() ItemFactory.create( - parent_location=self.course.location, + parent=self.course, category="video", display_name=u"test factory video omega \u03a9", ) @@ -206,7 +206,7 @@ class TestNonStandardCourseStructure(MobileAPITestCase): """ self.login_and_enroll() ItemFactory.create( - parent_location=self.vertical_under_course.location, + parent=self.vertical_under_course, category="video", display_name=u"test factory video omega \u03a9", ) @@ -234,7 +234,7 @@ class TestNonStandardCourseStructure(MobileAPITestCase): self.login_and_enroll() ItemFactory.create( - parent_location=self.chapter_under_course.location, + parent=self.chapter_under_course, category="video", display_name=u"test factory video omega \u03a9", ) @@ -262,7 +262,7 @@ class TestNonStandardCourseStructure(MobileAPITestCase): """ self.login_and_enroll() ItemFactory.create( - parent_location=self.section_under_course.location, + parent=self.section_under_course, category="video", display_name=u"test factory video omega \u03a9", ) @@ -291,7 +291,7 @@ class TestNonStandardCourseStructure(MobileAPITestCase): self.login_and_enroll() ItemFactory.create( - parent_location=self.section_under_chapter.location, + parent=self.section_under_chapter, category="video", display_name=u"meow factory video omega \u03a9", ) @@ -323,7 +323,7 @@ class TestNonStandardCourseStructure(MobileAPITestCase): """ self.login_and_enroll() ItemFactory.create( - parent_location=self.vertical_under_section.location, + parent=self.vertical_under_section, category="video", display_name=u"test factory video omega \u03a9", ) @@ -366,19 +366,19 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileEnro self.login_and_enroll() self._create_video_with_subs() ItemFactory.create( - parent_location=self.other_unit.location, + parent=self.other_unit, category="video", display_name=u"test video omega 2 \u03a9", html5_sources=[self.html5_video_url] ) ItemFactory.create( - parent_location=self.other_unit.location, + parent=self.other_unit, category="video", display_name=u"test video omega 3 \u03a9", source=self.html5_video_url ) ItemFactory.create( - parent_location=self.unit.location, + parent=self.unit, category="video", edx_video_id=self.edx_video_id, display_name=u"test draft video omega \u03a9", @@ -405,7 +405,7 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileEnro def test_with_nameless_unit(self): self.login_and_enroll() ItemFactory.create( - parent_location=self.nameless_unit.location, + parent=self.nameless_unit, category="video", edx_video_id=self.edx_video_id, display_name=u"test draft video omega 2 \u03a9" @@ -423,7 +423,7 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileEnro """ self.login_and_enroll() ItemFactory.create( - parent_location=self.sub_section.location, + parent=self.sub_section, category="video", edx_video_id=self.edx_video_id, display_name=u"video in the sub section" @@ -446,12 +446,12 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileEnro self.login_and_enroll() ItemFactory.create( - parent_location=self.split_test.location, + parent=self.split_test, category="video", display_name=u"split test video a", ) ItemFactory.create( - parent_location=self.split_test.location, + parent=self.split_test, category="video", display_name=u"split test video b", ) @@ -465,26 +465,26 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileEnro def test_with_hidden_blocks(self): self.login_and_enroll() hidden_subsection = ItemFactory.create( - parent_location=self.section.location, + parent=self.section, category="sequential", hide_from_toc=True, ) unit_within_hidden_subsection = ItemFactory.create( - parent_location=hidden_subsection.location, + parent=hidden_subsection, category="vertical", ) hidden_unit = ItemFactory.create( - parent_location=self.sub_section.location, + parent=self.sub_section, category="vertical", hide_from_toc=True, ) ItemFactory.create( - parent_location=unit_within_hidden_subsection.location, + parent=unit_within_hidden_subsection, category="video", edx_video_id=self.edx_video_id, ) ItemFactory.create( - parent_location=hidden_unit.location, + parent=hidden_unit, category="video", edx_video_id=self.edx_video_id, ) @@ -494,7 +494,7 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileEnro def test_language(self): self.login_and_enroll() video = ItemFactory.create( - parent_location=self.nameless_unit.location, + parent=self.nameless_unit, category="video", edx_video_id=self.edx_video_id, display_name=u"test draft video omega 2 \u03a9" @@ -523,7 +523,7 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileEnro def test_transcripts(self): self.login_and_enroll() video = ItemFactory.create( - parent_location=self.nameless_unit.location, + parent=self.nameless_unit, category="video", edx_video_id=self.edx_video_id, display_name=u"test draft video omega 2 \u03a9"