From ae439310e19331d061040f340c0d55e56a17b80e Mon Sep 17 00:00:00 2001 From: lenacom Date: Fri, 25 Dec 2015 18:03:28 +0300 Subject: [PATCH 01/10] New optional parameters for course blocks API: lti_url, block_types --- lms/djangoapps/course_api/blocks/api.py | 13 +++++++++++++ lms/djangoapps/course_api/blocks/forms.py | 2 ++ lms/djangoapps/course_api/blocks/serializers.py | 8 +++++++- .../course_api/blocks/tests/test_api.py | 16 ++++++++++++++-- .../course_api/blocks/tests/test_forms.py | 1 + .../course_api/blocks/tests/test_serializers.py | 5 ++--- lms/djangoapps/course_api/blocks/views.py | 15 ++++++++++++--- .../core/lib/block_structure/block_structure.py | 2 +- 8 files changed, 52 insertions(+), 10 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index 12636fef52..bbfd3d827d 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -20,6 +20,7 @@ def get_blocks( block_counts=None, student_view_data=None, return_type='dict', + block_types=None ): """ Return a serialized representation of the course blocks. @@ -61,6 +62,18 @@ def get_blocks( # transform blocks = get_course_blocks(user, usage_key, transformers) + # filter blocks by types + if block_types and len(block_types) == 0: + block_types = None + if block_types: + block_keys_to_remove = [] + for block_key in blocks: + block_type = blocks.get_xblock_field(block_key, 'category') + if not block_type in block_types: + block_keys_to_remove.append(block_key) + for block_key in block_keys_to_remove: + blocks.remove_block(block_key, True) + # serialize serializer_context = { 'request': request, diff --git a/lms/djangoapps/course_api/blocks/forms.py b/lms/djangoapps/course_api/blocks/forms.py index d77a148f81..ff6a746f59 100644 --- a/lms/djangoapps/course_api/blocks/forms.py +++ b/lms/djangoapps/course_api/blocks/forms.py @@ -31,6 +31,7 @@ class BlockListGetForm(Form): student_view_data = MultiValueField(required=False) usage_key = CharField(required=True) username = CharField(required=False) + block_types = MultiValueField(required=False) def clean_depth(self): """ @@ -88,6 +89,7 @@ class BlockListGetForm(Form): 'student_view_data', 'block_counts', 'nav_depth', + 'block_types', ] for additional_field in additional_requested_fields: field_value = cleaned_data.get(additional_field) diff --git a/lms/djangoapps/course_api/blocks/serializers.py b/lms/djangoapps/course_api/blocks/serializers.py index f0682f3041..84b863367c 100644 --- a/lms/djangoapps/course_api/blocks/serializers.py +++ b/lms/djangoapps/course_api/blocks/serializers.py @@ -44,6 +44,13 @@ class BlockSerializer(serializers.Serializer): # pylint: disable=abstract-metho ), } + if 'lti_url' in self.context['requested_fields']: + data['lti_url'] = reverse( + 'lti_provider_launch', + kwargs={'course_id': unicode(block_key.course_key), 'usage_id': unicode(block_key)}, + request=self.context['request'], + ) + # add additional requested fields that are supported by the various transformers for supported_field in SUPPORTED_FIELDS: if supported_field.requested_field_name in self.context['requested_fields']: @@ -70,7 +77,6 @@ class BlockDictSerializer(serializers.Serializer): # pylint: disable=abstract-m Serializer that formats a BlockStructure object to a dictionary, rather than a list, of blocks """ - root = serializers.CharField(source='root_block_usage_key') blocks = serializers.SerializerMethodField() def get_blocks(self, structure): diff --git a/lms/djangoapps/course_api/blocks/tests/test_api.py b/lms/djangoapps/course_api/blocks/tests/test_api.py index b4ab1112a5..c5e017ea3c 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_api.py +++ b/lms/djangoapps/course_api/blocks/tests/test_api.py @@ -35,7 +35,6 @@ class TestGetBlocks(EnableTransformerRegistryMixin, SharedModuleStoreTestCase): def test_basic(self): blocks = get_blocks(self.request, self.course.location, self.user) - self.assertEquals(blocks['root'], unicode(self.course.location)) # subtract for (1) the orphaned course About block and (2) the hidden Html block self.assertEquals(len(blocks['blocks']), len(self.store.get_items(self.course.id)) - 2) @@ -63,7 +62,6 @@ class TestGetBlocks(EnableTransformerRegistryMixin, SharedModuleStoreTestCase): sequential_block = self.store.get_item(self.course.id.make_usage_key('sequential', 'sequential_y1')) blocks = get_blocks(self.request, sequential_block.location, self.user) - self.assertEquals(blocks['root'], unicode(sequential_block.location)) self.assertEquals(len(blocks['blocks']), 5) for block_type, block_name, is_inside_of_structure in ( @@ -77,3 +75,17 @@ class TestGetBlocks(EnableTransformerRegistryMixin, SharedModuleStoreTestCase): self.assertIn(unicode(block.location), blocks['blocks']) else: self.assertNotIn(unicode(block.location), blocks['blocks']) + + def test_filtering_by_block_types(self): + sequential_block = self.store.get_item(self.course.id.make_usage_key('sequential', 'sequential_y1')) + + block_types = ['problem'] + blocks = get_blocks(self.request, sequential_block.location, self.user, block_types=block_types) + + for block_type, block_name in ( + ('problem', 'problem_y1a_1'), + ('problem', 'problem_y1a_2'), + ('problem', 'problem_y1a_3'), + ): + block = self.store.get_item(self.course.id.make_usage_key(block_type, block_name)) + self.assertIn(unicode(block.location), blocks['blocks']) diff --git a/lms/djangoapps/course_api/blocks/tests/test_forms.py b/lms/djangoapps/course_api/blocks/tests/test_forms.py index 78f94e4b71..efa397e693 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_forms.py +++ b/lms/djangoapps/course_api/blocks/tests/test_forms.py @@ -60,6 +60,7 @@ class TestBlockListGetForm(EnableTransformerRegistryMixin, FormTestMixin, Shared 'usage_key': usage_key, 'username': self.student.username, 'user': self.student, + 'block_types': set(), } def assert_raises_permission_denied(self): diff --git a/lms/djangoapps/course_api/blocks/tests/test_serializers.py b/lms/djangoapps/course_api/blocks/tests/test_serializers.py index f3a8691c28..c41de62a31 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_serializers.py +++ b/lms/djangoapps/course_api/blocks/tests/test_serializers.py @@ -70,6 +70,7 @@ class TestBlockSerializerBase(EnableTransformerRegistryMixin, SharedModuleStoreT 'block_counts', 'student_view_data', 'student_view_multi_device', + 'lti_url', ]) def assert_extended_block(self, serialized_block): @@ -81,6 +82,7 @@ class TestBlockSerializerBase(EnableTransformerRegistryMixin, SharedModuleStoreT 'id', 'type', 'lms_web_url', 'student_view_url', 'display_name', 'graded', 'block_counts', 'student_view_multi_device', + 'lti_url', }, set(serialized_block.iterkeys()), ) @@ -136,9 +138,6 @@ class TestBlockDictSerializer(TestBlockSerializerBase): def test_basic(self): serializer = self.create_serializer() - # verify root - self.assertEquals(serializer.data['root'], unicode(self.block_structure.root_block_usage_key)) - # verify blocks for block_key_string, serialized_block in serializer.data['blocks'].iteritems(): self.assertEquals(serialized_block['id'], block_key_string) diff --git a/lms/djangoapps/course_api/blocks/views.py b/lms/djangoapps/course_api/blocks/views.py index 8cbc4dcee4..50b846e85b 100644 --- a/lms/djangoapps/course_api/blocks/views.py +++ b/lms/djangoapps/course_api/blocks/views.py @@ -30,9 +30,10 @@ class BlocksView(DeveloperErrorViewMixin, ListAPIView): GET /api/courses/v1/blocks//? username=anjali &depth=all - &requested_fields=graded,format,student_view_multi_device + &requested_fields=graded,format,student_view_multi_device,lti_url &block_counts=video &student_view_data=video + &block_types=problem,html **Parameters**: @@ -85,6 +86,11 @@ class BlocksView(DeveloperErrorViewMixin, ListAPIView): Example: return_type=dict + * block_types: (list) Requested types of blocks. Possible values include sequential, + vertical, html, problem, video, and discussion. + + Example: block_types=vertical,html + **Response Values** The following fields are returned with a successful response. @@ -147,6 +153,7 @@ class BlocksView(DeveloperErrorViewMixin, ListAPIView): if the student_view_url and the student_view_data fields are not supported. + * lti_url: The block URL for an LTI consumer. """ def list(self, request, usage_key_string): # pylint: disable=arguments-differ @@ -177,7 +184,8 @@ class BlocksView(DeveloperErrorViewMixin, ListAPIView): params.cleaned_data['requested_fields'], params.cleaned_data.get('block_counts', []), params.cleaned_data.get('student_view_data', []), - params.cleaned_data['return_type'] + params.cleaned_data['return_type'], + params.cleaned_data.get('block_types', None), ) ) except ItemNotFoundError as exception: @@ -198,9 +206,10 @@ class BlocksInCourseView(BlocksView): GET /api/courses/v1/blocks/?course_id= &username=anjali &depth=all - &requested_fields=graded,format,student_view_multi_device + &requested_fields=graded,format,student_view_multi_device,lti_url &block_counts=video &student_view_data=video + &block_types=problem,html **Parameters**: diff --git a/openedx/core/lib/block_structure/block_structure.py b/openedx/core/lib/block_structure/block_structure.py index a526612d78..cd2422285f 100644 --- a/openedx/core/lib/block_structure/block_structure.py +++ b/openedx/core/lib/block_structure/block_structure.py @@ -70,7 +70,7 @@ class BlockStructure(object): traversal since it's the more common case and we currently need to support DAGs. """ - return self.topological_traversal() + return self.get_block_keys() #--- Block structure relation methods ---# From f048a28fa0b008d1acbc774cfb31945932587a7c Mon Sep 17 00:00:00 2001 From: lenacom Date: Mon, 29 Feb 2016 16:23:15 +0300 Subject: [PATCH 02/10] added ending comma --- lms/djangoapps/course_api/blocks/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index bbfd3d827d..c3dce3269b 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -20,7 +20,7 @@ def get_blocks( block_counts=None, student_view_data=None, return_type='dict', - block_types=None + block_types=None, ): """ Return a serialized representation of the course blocks. From 236cf4e30d7b4f7e02c3a45f79fb228c3cbc40c4 Mon Sep 17 00:00:00 2001 From: lenacom Date: Mon, 29 Feb 2016 19:12:44 +0300 Subject: [PATCH 03/10] removed check for 'root' --- lms/djangoapps/course_api/blocks/tests/test_views.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lms/djangoapps/course_api/blocks/tests/test_views.py b/lms/djangoapps/course_api/blocks/tests/test_views.py index 700077baef..15f820f88e 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_views.py +++ b/lms/djangoapps/course_api/blocks/tests/test_views.py @@ -173,7 +173,6 @@ class TestBlocksView(EnableTransformerRegistryMixin, SharedModuleStoreTestCase): def test_basic(self): response = self.verify_response() - self.assertEquals(response.data['root'], unicode(self.course_usage_key)) self.verify_response_block_dict(response) for block_key_string, block_data in response.data['blocks'].iteritems(): block_key = deserialize_usage_key(block_key_string, self.course_key) From 016c8200ae6db7dde7cbada2ecac1aa699cdfb2d Mon Sep 17 00:00:00 2001 From: lenacom Date: Tue, 1 Mar 2016 09:39:24 +0300 Subject: [PATCH 04/10] fixed comments --- lms/djangoapps/course_api/blocks/api.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index c3dce3269b..c6d389f6ff 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -45,6 +45,8 @@ def get_blocks( which blocks to return their student_view_data. return_type (string): Possible values are 'dict' or 'list'. Indicates the format for returning the blocks. + block_types (list): Optional list of names of block types which to + return blocks. """ # create ordered list of transformers, adding BlocksAPITransformer at end. transformers = BlockStructureTransformers() @@ -63,13 +65,11 @@ def get_blocks( blocks = get_course_blocks(user, usage_key, transformers) # filter blocks by types - if block_types and len(block_types) == 0: - block_types = None if block_types: block_keys_to_remove = [] for block_key in blocks: block_type = blocks.get_xblock_field(block_key, 'category') - if not block_type in block_types: + if block_type not in block_types: block_keys_to_remove.append(block_key) for block_key in block_keys_to_remove: blocks.remove_block(block_key, True) From 85f494db155d13edf3e7f98e833482763f3a8798 Mon Sep 17 00:00:00 2001 From: lenacom Date: Tue, 1 Mar 2016 13:02:57 +0300 Subject: [PATCH 05/10] fixed comment --- openedx/core/lib/block_structure/block_structure.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/openedx/core/lib/block_structure/block_structure.py b/openedx/core/lib/block_structure/block_structure.py index cd2422285f..c0a40b46da 100644 --- a/openedx/core/lib/block_structure/block_structure.py +++ b/openedx/core/lib/block_structure/block_structure.py @@ -66,9 +66,9 @@ class BlockStructure(object): def __iter__(self): """ - The default iterator for a block structure is a topological - traversal since it's the more common case and we currently - need to support DAGs. + The default iterator for a block structure is get_block_keys() + since we need to filter blocks as a list. + A topological traversal can be used to support DAGs. """ return self.get_block_keys() From f07bc1ce28dd4d490d5bf34ff6b1b282346c4246 Mon Sep 17 00:00:00 2001 From: lenacom Date: Wed, 9 Mar 2016 16:52:02 +0300 Subject: [PATCH 06/10] fixed comments --- lms/djangoapps/course_api/blocks/api.py | 12 ++++----- lms/djangoapps/course_api/blocks/forms.py | 4 +-- .../course_api/blocks/serializers.py | 1 + .../course_api/blocks/tests/test_api.py | 26 ++++++++++++------- .../course_api/blocks/tests/test_forms.py | 2 +- .../blocks/tests/test_serializers.py | 6 +++-- .../course_api/blocks/tests/test_views.py | 1 + lms/djangoapps/course_api/blocks/views.py | 13 +++++----- 8 files changed, 39 insertions(+), 26 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index c6d389f6ff..aa6bf61f71 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -20,7 +20,7 @@ def get_blocks( block_counts=None, student_view_data=None, return_type='dict', - block_types=None, + block_type_filter=None, ): """ Return a serialized representation of the course blocks. @@ -45,8 +45,8 @@ def get_blocks( which blocks to return their student_view_data. return_type (string): Possible values are 'dict' or 'list'. Indicates the format for returning the blocks. - block_types (list): Optional list of names of block types which to - return blocks. + block_type_filter (list): Optional list of block type names used to filter + the final result of returned blocks. """ # create ordered list of transformers, adding BlocksAPITransformer at end. transformers = BlockStructureTransformers() @@ -65,14 +65,14 @@ def get_blocks( blocks = get_course_blocks(user, usage_key, transformers) # filter blocks by types - if block_types: + if block_type_filter: block_keys_to_remove = [] for block_key in blocks: block_type = blocks.get_xblock_field(block_key, 'category') - if block_type not in block_types: + if block_type not in block_type_filter: block_keys_to_remove.append(block_key) for block_key in block_keys_to_remove: - blocks.remove_block(block_key, True) + blocks.remove_block(block_key, keep_descendants=True) # serialize serializer_context = { diff --git a/lms/djangoapps/course_api/blocks/forms.py b/lms/djangoapps/course_api/blocks/forms.py index ff6a746f59..6d214b0314 100644 --- a/lms/djangoapps/course_api/blocks/forms.py +++ b/lms/djangoapps/course_api/blocks/forms.py @@ -31,7 +31,7 @@ class BlockListGetForm(Form): student_view_data = MultiValueField(required=False) usage_key = CharField(required=True) username = CharField(required=False) - block_types = MultiValueField(required=False) + block_type_filter = MultiValueField(required=False) def clean_depth(self): """ @@ -89,7 +89,7 @@ class BlockListGetForm(Form): 'student_view_data', 'block_counts', 'nav_depth', - 'block_types', + 'block_type_filter', ] for additional_field in additional_requested_fields: field_value = cleaned_data.get(additional_field) diff --git a/lms/djangoapps/course_api/blocks/serializers.py b/lms/djangoapps/course_api/blocks/serializers.py index 84b863367c..3f69703c7d 100644 --- a/lms/djangoapps/course_api/blocks/serializers.py +++ b/lms/djangoapps/course_api/blocks/serializers.py @@ -77,6 +77,7 @@ class BlockDictSerializer(serializers.Serializer): # pylint: disable=abstract-m Serializer that formats a BlockStructure object to a dictionary, rather than a list, of blocks """ + root = serializers.CharField(source='root_block_usage_key') blocks = serializers.SerializerMethodField() def get_blocks(self, structure): diff --git a/lms/djangoapps/course_api/blocks/tests/test_api.py b/lms/djangoapps/course_api/blocks/tests/test_api.py index c5e017ea3c..02f73dd882 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_api.py +++ b/lms/djangoapps/course_api/blocks/tests/test_api.py @@ -12,6 +12,8 @@ from xmodule.modulestore.tests.factories import SampleCourseFactory from ..api import get_blocks +import re + class TestGetBlocks(EnableTransformerRegistryMixin, SharedModuleStoreTestCase): """ @@ -35,6 +37,7 @@ class TestGetBlocks(EnableTransformerRegistryMixin, SharedModuleStoreTestCase): def test_basic(self): blocks = get_blocks(self.request, self.course.location, self.user) + self.assertEquals(blocks['root'], unicode(self.course.location)) # subtract for (1) the orphaned course About block and (2) the hidden Html block self.assertEquals(len(blocks['blocks']), len(self.store.get_items(self.course.id)) - 2) @@ -62,6 +65,7 @@ class TestGetBlocks(EnableTransformerRegistryMixin, SharedModuleStoreTestCase): sequential_block = self.store.get_item(self.course.id.make_usage_key('sequential', 'sequential_y1')) blocks = get_blocks(self.request, sequential_block.location, self.user) + self.assertEquals(blocks['root'], unicode(sequential_block.location)) self.assertEquals(len(blocks['blocks']), 5) for block_type, block_name, is_inside_of_structure in ( @@ -79,13 +83,17 @@ class TestGetBlocks(EnableTransformerRegistryMixin, SharedModuleStoreTestCase): def test_filtering_by_block_types(self): sequential_block = self.store.get_item(self.course.id.make_usage_key('sequential', 'sequential_y1')) - block_types = ['problem'] - blocks = get_blocks(self.request, sequential_block.location, self.user, block_types=block_types) + # not filtered blocks + blocks = get_blocks(self.request, sequential_block.location, self.user) + self.assertEquals(len(blocks['blocks']), 5) + found_not_problem = False + for key in blocks['blocks']: + if not re.search(r'/problem/', key): + found_not_problem = True + self.assertTrue(found_not_problem) - for block_type, block_name in ( - ('problem', 'problem_y1a_1'), - ('problem', 'problem_y1a_2'), - ('problem', 'problem_y1a_3'), - ): - block = self.store.get_item(self.course.id.make_usage_key(block_type, block_name)) - self.assertIn(unicode(block.location), blocks['blocks']) + # filtered blocks + blocks = get_blocks(self.request, sequential_block.location, self.user, block_type_filter=['problem']) + self.assertEquals(len(blocks['blocks']), 3) + for key in blocks['blocks']: + self.assertTrue(re.search(r'/problem/', key)) diff --git a/lms/djangoapps/course_api/blocks/tests/test_forms.py b/lms/djangoapps/course_api/blocks/tests/test_forms.py index efa397e693..2c39a1e508 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_forms.py +++ b/lms/djangoapps/course_api/blocks/tests/test_forms.py @@ -60,7 +60,7 @@ class TestBlockListGetForm(EnableTransformerRegistryMixin, FormTestMixin, Shared 'usage_key': usage_key, 'username': self.student.username, 'user': self.student, - 'block_types': set(), + 'block_type_filter': set(), } def assert_raises_permission_denied(self): diff --git a/lms/djangoapps/course_api/blocks/tests/test_serializers.py b/lms/djangoapps/course_api/blocks/tests/test_serializers.py index c41de62a31..4223d2446a 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_serializers.py +++ b/lms/djangoapps/course_api/blocks/tests/test_serializers.py @@ -118,8 +118,7 @@ class TestBlockSerializer(TestBlockSerializerBase): def test_additional_requested_fields(self): self.add_additional_requested_fields() serializer = self.create_serializer() - for serialized_block in serializer.data: - self.assert_extended_block(serialized_block) + self.assertEqual(5, len(serializer.data)) class TestBlockDictSerializer(TestBlockSerializerBase): @@ -138,6 +137,9 @@ class TestBlockDictSerializer(TestBlockSerializerBase): def test_basic(self): serializer = self.create_serializer() + # verify root + self.assertEquals(serializer.data['root'], unicode(self.block_structure.root_block_usage_key)) + # verify blocks for block_key_string, serialized_block in serializer.data['blocks'].iteritems(): self.assertEquals(serialized_block['id'], block_key_string) diff --git a/lms/djangoapps/course_api/blocks/tests/test_views.py b/lms/djangoapps/course_api/blocks/tests/test_views.py index 15f820f88e..700077baef 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_views.py +++ b/lms/djangoapps/course_api/blocks/tests/test_views.py @@ -173,6 +173,7 @@ class TestBlocksView(EnableTransformerRegistryMixin, SharedModuleStoreTestCase): def test_basic(self): response = self.verify_response() + self.assertEquals(response.data['root'], unicode(self.course_usage_key)) self.verify_response_block_dict(response) for block_key_string, block_data in response.data['blocks'].iteritems(): block_key = deserialize_usage_key(block_key_string, self.course_key) diff --git a/lms/djangoapps/course_api/blocks/views.py b/lms/djangoapps/course_api/blocks/views.py index 50b846e85b..47425108cf 100644 --- a/lms/djangoapps/course_api/blocks/views.py +++ b/lms/djangoapps/course_api/blocks/views.py @@ -33,7 +33,7 @@ class BlocksView(DeveloperErrorViewMixin, ListAPIView): &requested_fields=graded,format,student_view_multi_device,lti_url &block_counts=video &student_view_data=video - &block_types=problem,html + &block_type_filter=problem,html **Parameters**: @@ -86,10 +86,11 @@ class BlocksView(DeveloperErrorViewMixin, ListAPIView): Example: return_type=dict - * block_types: (list) Requested types of blocks. Possible values include sequential, - vertical, html, problem, video, and discussion. + * block_type_filter: (list) Requested types of blocks used to filter the final result + of returned blocks. Possible values include sequential,vertical, html, problem, + video, and discussion. - Example: block_types=vertical,html + Example: block_type_filter=vertical,html **Response Values** @@ -185,7 +186,7 @@ class BlocksView(DeveloperErrorViewMixin, ListAPIView): params.cleaned_data.get('block_counts', []), params.cleaned_data.get('student_view_data', []), params.cleaned_data['return_type'], - params.cleaned_data.get('block_types', None), + params.cleaned_data.get('block_type_filter', None), ) ) except ItemNotFoundError as exception: @@ -209,7 +210,7 @@ class BlocksInCourseView(BlocksView): &requested_fields=graded,format,student_view_multi_device,lti_url &block_counts=video &student_view_data=video - &block_types=problem,html + &block_type_filter=problem,html **Parameters**: From 3aafbe591a99ed68ff4548e99d02ba005d331ff2 Mon Sep 17 00:00:00 2001 From: lenacom Date: Wed, 9 Mar 2016 16:57:45 +0300 Subject: [PATCH 07/10] fixed test changed by mistake --- lms/djangoapps/course_api/blocks/tests/test_serializers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/course_api/blocks/tests/test_serializers.py b/lms/djangoapps/course_api/blocks/tests/test_serializers.py index 4223d2446a..179970b30b 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_serializers.py +++ b/lms/djangoapps/course_api/blocks/tests/test_serializers.py @@ -118,7 +118,8 @@ class TestBlockSerializer(TestBlockSerializerBase): def test_additional_requested_fields(self): self.add_additional_requested_fields() serializer = self.create_serializer() - self.assertEqual(5, len(serializer.data)) + for serialized_block in serializer.data: + self.assert_extended_block(serialized_block) class TestBlockDictSerializer(TestBlockSerializerBase): From 23b617492c822910d547c957938cdfbe6ae03915 Mon Sep 17 00:00:00 2001 From: lenacom Date: Wed, 9 Mar 2016 18:07:47 +0300 Subject: [PATCH 08/10] removed a trailing space --- lms/djangoapps/course_api/blocks/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index aa6bf61f71..d69be19c46 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -45,7 +45,7 @@ def get_blocks( which blocks to return their student_view_data. return_type (string): Possible values are 'dict' or 'list'. Indicates the format for returning the blocks. - block_type_filter (list): Optional list of block type names used to filter + block_type_filter (list): Optional list of block type names used to filter the final result of returned blocks. """ # create ordered list of transformers, adding BlocksAPITransformer at end. From 2f2c6fc5dafb9d609e25fb917834f9511110e342 Mon Sep 17 00:00:00 2001 From: lenacom Date: Thu, 10 Mar 2016 11:21:06 +0300 Subject: [PATCH 09/10] fixed comments --- lms/djangoapps/course_api/blocks/api.py | 8 ++++---- lms/djangoapps/course_api/blocks/forms.py | 4 ++-- lms/djangoapps/course_api/blocks/tests/test_api.py | 11 +++++------ lms/djangoapps/course_api/blocks/tests/test_forms.py | 2 +- lms/djangoapps/course_api/blocks/views.py | 12 ++++++------ 5 files changed, 18 insertions(+), 19 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index d69be19c46..ab8fd53bbc 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -20,7 +20,7 @@ def get_blocks( block_counts=None, student_view_data=None, return_type='dict', - block_type_filter=None, + block_types_filter=None, ): """ Return a serialized representation of the course blocks. @@ -45,7 +45,7 @@ def get_blocks( which blocks to return their student_view_data. return_type (string): Possible values are 'dict' or 'list'. Indicates the format for returning the blocks. - block_type_filter (list): Optional list of block type names used to filter + block_types_filter (list): Optional list of block type names used to filter the final result of returned blocks. """ # create ordered list of transformers, adding BlocksAPITransformer at end. @@ -65,11 +65,11 @@ def get_blocks( blocks = get_course_blocks(user, usage_key, transformers) # filter blocks by types - if block_type_filter: + if block_types_filter: block_keys_to_remove = [] for block_key in blocks: block_type = blocks.get_xblock_field(block_key, 'category') - if block_type not in block_type_filter: + if block_type not in block_types_filter: block_keys_to_remove.append(block_key) for block_key in block_keys_to_remove: blocks.remove_block(block_key, keep_descendants=True) diff --git a/lms/djangoapps/course_api/blocks/forms.py b/lms/djangoapps/course_api/blocks/forms.py index 6d214b0314..76363e6679 100644 --- a/lms/djangoapps/course_api/blocks/forms.py +++ b/lms/djangoapps/course_api/blocks/forms.py @@ -31,7 +31,7 @@ class BlockListGetForm(Form): student_view_data = MultiValueField(required=False) usage_key = CharField(required=True) username = CharField(required=False) - block_type_filter = MultiValueField(required=False) + block_types_filter = MultiValueField(required=False) def clean_depth(self): """ @@ -89,7 +89,7 @@ class BlockListGetForm(Form): 'student_view_data', 'block_counts', 'nav_depth', - 'block_type_filter', + 'block_types_filter', ] for additional_field in additional_requested_fields: field_value = cleaned_data.get(additional_field) diff --git a/lms/djangoapps/course_api/blocks/tests/test_api.py b/lms/djangoapps/course_api/blocks/tests/test_api.py index 02f73dd882..4a7d4ec0b5 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_api.py +++ b/lms/djangoapps/course_api/blocks/tests/test_api.py @@ -12,8 +12,6 @@ from xmodule.modulestore.tests.factories import SampleCourseFactory from ..api import get_blocks -import re - class TestGetBlocks(EnableTransformerRegistryMixin, SharedModuleStoreTestCase): """ @@ -84,16 +82,17 @@ class TestGetBlocks(EnableTransformerRegistryMixin, SharedModuleStoreTestCase): sequential_block = self.store.get_item(self.course.id.make_usage_key('sequential', 'sequential_y1')) # not filtered blocks - blocks = get_blocks(self.request, sequential_block.location, self.user) + blocks = get_blocks(self.request, sequential_block.location, self.user, requested_fields=['type']) self.assertEquals(len(blocks['blocks']), 5) found_not_problem = False for key in blocks['blocks']: - if not re.search(r'/problem/', key): + if blocks['blocks'][key]['type'] != 'problem': found_not_problem = True self.assertTrue(found_not_problem) # filtered blocks - blocks = get_blocks(self.request, sequential_block.location, self.user, block_type_filter=['problem']) + blocks = get_blocks(self.request, sequential_block.location, self.user, + block_types_filter=['problem'], requested_fields=['type']) self.assertEquals(len(blocks['blocks']), 3) for key in blocks['blocks']: - self.assertTrue(re.search(r'/problem/', key)) + self.assertEqual(blocks['blocks'][key]['type'], 'problem') diff --git a/lms/djangoapps/course_api/blocks/tests/test_forms.py b/lms/djangoapps/course_api/blocks/tests/test_forms.py index 2c39a1e508..01c2fc0dcd 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_forms.py +++ b/lms/djangoapps/course_api/blocks/tests/test_forms.py @@ -60,7 +60,7 @@ class TestBlockListGetForm(EnableTransformerRegistryMixin, FormTestMixin, Shared 'usage_key': usage_key, 'username': self.student.username, 'user': self.student, - 'block_type_filter': set(), + 'block_types_filter': set(), } def assert_raises_permission_denied(self): diff --git a/lms/djangoapps/course_api/blocks/views.py b/lms/djangoapps/course_api/blocks/views.py index 47425108cf..f6e73ed928 100644 --- a/lms/djangoapps/course_api/blocks/views.py +++ b/lms/djangoapps/course_api/blocks/views.py @@ -33,7 +33,7 @@ class BlocksView(DeveloperErrorViewMixin, ListAPIView): &requested_fields=graded,format,student_view_multi_device,lti_url &block_counts=video &student_view_data=video - &block_type_filter=problem,html + &block_types_filter=problem,html **Parameters**: @@ -86,11 +86,11 @@ class BlocksView(DeveloperErrorViewMixin, ListAPIView): Example: return_type=dict - * block_type_filter: (list) Requested types of blocks used to filter the final result - of returned blocks. Possible values include sequential,vertical, html, problem, + * block_types_filter: (list) Requested types of blocks used to filter the final result + of returned blocks. Possible values include sequential, vertical, html, problem, video, and discussion. - Example: block_type_filter=vertical,html + Example: block_types_filter=vertical,html **Response Values** @@ -186,7 +186,7 @@ class BlocksView(DeveloperErrorViewMixin, ListAPIView): params.cleaned_data.get('block_counts', []), params.cleaned_data.get('student_view_data', []), params.cleaned_data['return_type'], - params.cleaned_data.get('block_type_filter', None), + params.cleaned_data.get('block_types_filter', None), ) ) except ItemNotFoundError as exception: @@ -210,7 +210,7 @@ class BlocksInCourseView(BlocksView): &requested_fields=graded,format,student_view_multi_device,lti_url &block_counts=video &student_view_data=video - &block_type_filter=problem,html + &block_types_filter=problem,html **Parameters**: From 7e4f3fd00c4425397680752a8609456b5cb8fdd0 Mon Sep 17 00:00:00 2001 From: lenacom Date: Thu, 10 Mar 2016 16:47:35 +0300 Subject: [PATCH 10/10] fixed comment --- lms/djangoapps/course_api/blocks/tests/test_api.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/tests/test_api.py b/lms/djangoapps/course_api/blocks/tests/test_api.py index 4a7d4ec0b5..8cbea0bd16 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_api.py +++ b/lms/djangoapps/course_api/blocks/tests/test_api.py @@ -85,8 +85,8 @@ class TestGetBlocks(EnableTransformerRegistryMixin, SharedModuleStoreTestCase): blocks = get_blocks(self.request, sequential_block.location, self.user, requested_fields=['type']) self.assertEquals(len(blocks['blocks']), 5) found_not_problem = False - for key in blocks['blocks']: - if blocks['blocks'][key]['type'] != 'problem': + for block in blocks['blocks'].itervalues(): + if block['type'] != 'problem': found_not_problem = True self.assertTrue(found_not_problem) @@ -94,5 +94,5 @@ class TestGetBlocks(EnableTransformerRegistryMixin, SharedModuleStoreTestCase): blocks = get_blocks(self.request, sequential_block.location, self.user, block_types_filter=['problem'], requested_fields=['type']) self.assertEquals(len(blocks['blocks']), 3) - for key in blocks['blocks']: - self.assertEqual(blocks['blocks'][key]['type'], 'problem') + for block in blocks['blocks'].itervalues(): + self.assertEqual(block['type'], 'problem')