Add filtering support for list_library_blocks API
This commit is contained in:
committed by
Kyle McCormick
parent
3233fa92d4
commit
f2ed54d72c
@@ -58,7 +58,11 @@ from xblock.exceptions import XBlockNotFoundError
|
||||
from openedx.core.djangoapps.content_libraries import permissions
|
||||
from openedx.core.djangoapps.content_libraries.constants import DRAFT_NAME
|
||||
from openedx.core.djangoapps.content_libraries.library_bundle import LibraryBundle
|
||||
from openedx.core.djangoapps.content_libraries.libraries_index import ContentLibraryIndexer, ItemNotIndexedException
|
||||
from openedx.core.djangoapps.content_libraries.libraries_index import (
|
||||
ContentLibraryIndexer,
|
||||
LibraryBlockIndexer,
|
||||
ItemNotIndexedException,
|
||||
)
|
||||
from openedx.core.djangoapps.content_libraries.models import ContentLibrary, ContentLibraryPermission
|
||||
from openedx.core.djangoapps.content_libraries.signals import (
|
||||
CONTENT_LIBRARY_CREATED,
|
||||
@@ -497,28 +501,58 @@ def delete_library(library_key):
|
||||
raise
|
||||
|
||||
|
||||
def get_library_blocks(library_key):
|
||||
def get_library_blocks(library_key, text_search=None):
|
||||
"""
|
||||
Get the list of top-level XBlocks in the specified library.
|
||||
|
||||
Returns a list of LibraryXBlockMetadata objects
|
||||
"""
|
||||
metadata = None
|
||||
ref = ContentLibrary.objects.get_by_key(library_key)
|
||||
lib_bundle = LibraryBundle(library_key, ref.bundle_uuid, draft_name=DRAFT_NAME)
|
||||
usages = lib_bundle.get_top_level_usages()
|
||||
blocks = []
|
||||
for usage_key in usages:
|
||||
# For top-level definitions, we can go from definition key to usage key using the following, but this would not
|
||||
# work for non-top-level blocks as they may have multiple usages. Top level blocks are guaranteed to have only
|
||||
# a single usage in the library, which is part of the definition of top level block.
|
||||
def_key = lib_bundle.definition_for_usage(usage_key)
|
||||
blocks.append(LibraryXBlockMetadata(
|
||||
usage_key=usage_key,
|
||||
def_key=def_key,
|
||||
display_name=get_block_display_name(def_key),
|
||||
has_unpublished_changes=lib_bundle.does_definition_have_unpublished_changes(def_key),
|
||||
))
|
||||
return blocks
|
||||
|
||||
if LibraryBlockIndexer.indexing_is_enabled():
|
||||
try:
|
||||
metadata = [
|
||||
{
|
||||
**item,
|
||||
"id": LibraryUsageLocatorV2.from_string(item['id']),
|
||||
}
|
||||
for item in LibraryBlockIndexer.get_items(usages, text_search=text_search)
|
||||
if item is not None
|
||||
]
|
||||
except (ItemNotIndexedException, KeyError, ConnectionError) as e:
|
||||
log.exception(e)
|
||||
|
||||
# If indexing is disabled, or connection to elastic failed
|
||||
if metadata is None:
|
||||
metadata = []
|
||||
for usage_key in usages:
|
||||
# For top-level definitions, we can go from definition key to usage key using the following, but this would not
|
||||
# work for non-top-level blocks as they may have multiple usages. Top level blocks are guaranteed to have only
|
||||
# a single usage in the library, which is part of the definition of top level block.
|
||||
def_key = lib_bundle.definition_for_usage(usage_key)
|
||||
display_name = get_block_display_name(def_key)
|
||||
if (text_search is None or
|
||||
text_search.lower() in display_name.lower() or
|
||||
text_search.lower() in str(usage_key).lower()):
|
||||
metadata.append({
|
||||
"id": usage_key,
|
||||
"def_key": def_key,
|
||||
"display_name": display_name,
|
||||
"has_unpublished_changes": lib_bundle.does_definition_have_unpublished_changes(def_key),
|
||||
})
|
||||
|
||||
return [
|
||||
LibraryXBlockMetadata(
|
||||
usage_key=item['id'],
|
||||
def_key=item['def_key'],
|
||||
display_name=item['display_name'],
|
||||
has_unpublished_changes=item['has_unpublished_changes'],
|
||||
)
|
||||
for item in metadata
|
||||
]
|
||||
|
||||
|
||||
def _lookup_usage_key(usage_key):
|
||||
|
||||
@@ -67,7 +67,7 @@ class SearchIndexerBase(ABC):
|
||||
"""
|
||||
searcher = SearchEngine.get_search_engine(cls.INDEX_NAME)
|
||||
response = searcher.search(doc_type=cls.DOCUMENT_TYPE, field_dictionary=kwargs, size=MAX_SIZE)
|
||||
return response["results"]
|
||||
return sorted(response["results"], key=lambda i: i['data']["id"])
|
||||
|
||||
@classmethod
|
||||
def get_items(cls, ids, text_search=None):
|
||||
@@ -101,6 +101,7 @@ class SearchIndexerBase(ABC):
|
||||
size=MAX_SIZE
|
||||
))
|
||||
else:
|
||||
# This is used only for running tests. TODO: Remove this and use elasticsearch in tests.
|
||||
response = searcher.search(
|
||||
doc_type=cls.DOCUMENT_TYPE,
|
||||
field_dictionary={"id": ids_str},
|
||||
|
||||
@@ -72,7 +72,7 @@ class LibraryXBlockMetadataSerializer(serializers.Serializer):
|
||||
"""
|
||||
id = serializers.CharField(source="usage_key", read_only=True)
|
||||
def_key = serializers.CharField(read_only=True)
|
||||
block_type = serializers.CharField(source="def_key.block_type")
|
||||
block_type = serializers.CharField(source="usage_key.block_type")
|
||||
display_name = serializers.CharField(read_only=True)
|
||||
has_unpublished_changes = serializers.BooleanField(read_only=True)
|
||||
# When creating a new XBlock in a library, the slug becomes the ID part of
|
||||
|
||||
@@ -221,9 +221,16 @@ class ContentLibrariesRestApiTest(APITestCase):
|
||||
else:
|
||||
return self._api('put', url, {"access_level": access_level}, expect_response)
|
||||
|
||||
def _get_library_blocks(self, lib_key, expect_response=200):
|
||||
def _get_library_blocks(self, lib_key, query_params_dict=None, expect_response=200):
|
||||
""" Get the list of XBlocks in the library """
|
||||
return self._api('get', URL_LIB_BLOCKS.format(lib_key=lib_key), None, expect_response)
|
||||
if query_params_dict is None:
|
||||
query_params_dict = {}
|
||||
return self._api(
|
||||
'get',
|
||||
URL_LIB_BLOCKS.format(lib_key=lib_key) + '?' + urlencode(query_params_dict),
|
||||
None,
|
||||
expect_response
|
||||
)
|
||||
|
||||
def _add_block_to_library(self, lib_key, block_type, slug, parent_block=None, expect_response=200):
|
||||
""" Add a new XBlock to the library """
|
||||
|
||||
@@ -89,7 +89,7 @@ class ContentLibrariesTest(ContentLibrariesRestApiTest):
|
||||
self._create_library(slug="Invalid Slug!", title="Library with Bad Slug", expect_response=400)
|
||||
|
||||
@ddt.data(True, False)
|
||||
@patch("openedx.core.djangoapps.content_libraries.views.LibraryRootPagination.page_size", new=2)
|
||||
@patch("openedx.core.djangoapps.content_libraries.views.LibraryApiPagination.page_size", new=2)
|
||||
def test_list_library(self, is_indexing_enabled):
|
||||
"""
|
||||
Test the /libraries API and its pagination
|
||||
@@ -247,6 +247,59 @@ class ContentLibrariesTest(ContentLibrariesRestApiTest):
|
||||
|
||||
# fin
|
||||
|
||||
@ddt.data(True, False)
|
||||
@patch("openedx.core.djangoapps.content_libraries.views.LibraryApiPagination.page_size", new=2)
|
||||
def test_list_library_blocks(self, is_indexing_enabled):
|
||||
"""
|
||||
Test the /libraries/{lib_key_str}/blocks API and its pagination
|
||||
"""
|
||||
features = settings.FEATURES
|
||||
features['ENABLE_CONTENT_LIBRARY_INDEX'] = is_indexing_enabled
|
||||
with override_settings(FEATURES=features):
|
||||
lib = self._create_library(slug="list_blocks-slug" + str(is_indexing_enabled), title="Library 1")
|
||||
block1 = self._add_block_to_library(lib["id"], "problem", "problem1")
|
||||
block2 = self._add_block_to_library(lib["id"], "unit", "unit1")
|
||||
self._add_block_to_library(lib["id"], "problem", "problem2", parent_block=block2["id"])
|
||||
|
||||
result = self._get_library_blocks(lib["id"])
|
||||
self.assertEqual(len(result), 2)
|
||||
self.assertIn(block1, result)
|
||||
|
||||
result = self._get_library_blocks(lib["id"], {'pagination': 'true'})
|
||||
self.assertEqual(len(result['results']), 2)
|
||||
self.assertEqual(result['next'], None)
|
||||
|
||||
self._add_block_to_library(lib["id"], "problem", "problem3")
|
||||
# Test pagination
|
||||
result = self._get_library_blocks(lib["id"])
|
||||
self.assertEqual(len(result), 3)
|
||||
result = self._get_library_blocks(lib["id"], {'pagination': 'true'})
|
||||
self.assertEqual(len(result['results']), 2)
|
||||
self.assertIn('page=2', result['next'])
|
||||
self.assertIn('pagination=true', result['next'])
|
||||
result = self._get_library_blocks(lib["id"], {'pagination': 'true', 'page': '2'})
|
||||
self.assertEqual(len(result['results']), 1)
|
||||
self.assertEqual(result['next'], None)
|
||||
|
||||
@ddt.data(True, False)
|
||||
def test_library_blocks_filters(self, is_indexing_enabled):
|
||||
"""
|
||||
Test the filters in the list libraries API
|
||||
"""
|
||||
features = settings.FEATURES
|
||||
features['ENABLE_CONTENT_LIBRARY_INDEX'] = is_indexing_enabled
|
||||
with override_settings(FEATURES=features):
|
||||
lib = self._create_library(slug="test-lib-blocks" + str(is_indexing_enabled), title="Title")
|
||||
block1 = self._add_block_to_library(lib["id"], "problem", "foo-bar")
|
||||
block2 = self._add_block_to_library(lib["id"], "problem", "foo-baz")
|
||||
block3 = self._add_block_to_library(lib["id"], "problem", "bar-baz")
|
||||
|
||||
self._set_library_block_olx(block1["id"], "<problem display_name=\"DisplayName\"></problem>")
|
||||
|
||||
self.assertEqual(len(self._get_library_blocks(lib["id"])), 3)
|
||||
self.assertEqual(len(self._get_library_blocks(lib["id"], {'text_search': 'Foo'})), 2)
|
||||
self.assertEqual(len(self._get_library_blocks(lib["id"], {'text_search': 'Display'})), 1)
|
||||
|
||||
def test_library_blocks_with_hierarchy(self):
|
||||
"""
|
||||
Test library blocks with children
|
||||
|
||||
@@ -66,13 +66,31 @@ def convert_exceptions(fn):
|
||||
return wrapped_fn
|
||||
|
||||
|
||||
class LibraryRootPagination(PageNumberPagination):
|
||||
class LibraryApiPagination(PageNumberPagination):
|
||||
"""
|
||||
Paginates over ContentLibraryMetadata objects.
|
||||
"""
|
||||
page_size = 50
|
||||
page_size_query_param = 'page_size'
|
||||
|
||||
apidoc_params = [
|
||||
apidocs.query_parameter(
|
||||
'pagination',
|
||||
bool,
|
||||
description="Enables paginated schema",
|
||||
),
|
||||
apidocs.query_parameter(
|
||||
'page',
|
||||
int,
|
||||
description="Page number of result. Defaults to 1",
|
||||
),
|
||||
apidocs.query_parameter(
|
||||
'page_size',
|
||||
int,
|
||||
description="Page size of the result. Defaults to 50",
|
||||
),
|
||||
]
|
||||
|
||||
|
||||
@view_auth_classes()
|
||||
class LibraryRootView(APIView):
|
||||
@@ -82,21 +100,7 @@ class LibraryRootView(APIView):
|
||||
|
||||
@apidocs.schema(
|
||||
parameters=[
|
||||
apidocs.query_parameter(
|
||||
'pagination',
|
||||
bool,
|
||||
description="Enables paginated schema",
|
||||
),
|
||||
apidocs.query_parameter(
|
||||
'page',
|
||||
int,
|
||||
description="Page number of result. Defaults to 1",
|
||||
),
|
||||
apidocs.query_parameter(
|
||||
'page_size',
|
||||
int,
|
||||
description="Page size of the result. Defaults to 50",
|
||||
),
|
||||
*LibraryApiPagination.apidoc_params,
|
||||
apidocs.query_parameter(
|
||||
'org',
|
||||
str,
|
||||
@@ -116,7 +120,7 @@ class LibraryRootView(APIView):
|
||||
org = request.query_params.get('org', None)
|
||||
text_search = request.query_params.get('text_search', None)
|
||||
|
||||
paginator = LibraryRootPagination()
|
||||
paginator = LibraryApiPagination()
|
||||
queryset = api.get_libraries_for_user(request.user, org=org)
|
||||
if text_search:
|
||||
result = api.get_metadata_from_index(queryset, text_search=text_search)
|
||||
@@ -407,14 +411,35 @@ class LibraryBlocksView(APIView):
|
||||
"""
|
||||
Views to work with XBlocks in a specific content library.
|
||||
"""
|
||||
@apidocs.schema(
|
||||
parameters=[
|
||||
*LibraryApiPagination.apidoc_params,
|
||||
apidocs.query_parameter(
|
||||
'text_search',
|
||||
str,
|
||||
description="The string used to filter libraries by searching in title, id, org, or description",
|
||||
),
|
||||
],
|
||||
)
|
||||
@convert_exceptions
|
||||
def get(self, request, lib_key_str):
|
||||
"""
|
||||
Get the list of all top-level blocks in this content library
|
||||
"""
|
||||
key = LibraryLocatorV2.from_string(lib_key_str)
|
||||
text_search = request.query_params.get('text_search', None)
|
||||
|
||||
api.require_permission_for_library_key(key, request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY)
|
||||
result = api.get_library_blocks(key)
|
||||
result = api.get_library_blocks(key, text_search=text_search)
|
||||
|
||||
# Verify `pagination` param to maintain compatibility with older
|
||||
# non pagination-aware clients
|
||||
if request.GET.get('pagination', 'false').lower() == 'true':
|
||||
paginator = LibraryApiPagination()
|
||||
result = paginator.paginate_queryset(result, request)
|
||||
serializer = LibraryXBlockMetadataSerializer(result, many=True)
|
||||
return paginator.get_paginated_response(serializer.data)
|
||||
|
||||
return Response(LibraryXBlockMetadataSerializer(result, many=True).data)
|
||||
|
||||
@convert_exceptions
|
||||
|
||||
Reference in New Issue
Block a user