Merge branch 'master' into timmc/misc-metric-attribute
# Conflicts: # cms/envs/common.py # lms/envs/common.py
This commit is contained in:
@@ -58,7 +58,7 @@ 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, LibraryNotIndexedException
|
||||
from openedx.core.djangoapps.content_libraries.libraries_index import ContentLibraryIndexer, LibraryBlockIndexer
|
||||
from openedx.core.djangoapps.content_libraries.models import ContentLibrary, ContentLibraryPermission
|
||||
from openedx.core.djangoapps.content_libraries.signals import (
|
||||
CONTENT_LIBRARY_CREATED,
|
||||
@@ -114,6 +114,10 @@ class InvalidNameError(ValueError):
|
||||
""" The specified name/identifier is not valid """
|
||||
|
||||
|
||||
class LibraryPermissionIntegrityError(IntegrityError):
|
||||
""" Thrown when an operation would cause insane permissions. """
|
||||
|
||||
|
||||
# Models:
|
||||
|
||||
@attr.s
|
||||
@@ -220,15 +224,18 @@ class AccessLevel:
|
||||
NO_ACCESS = None
|
||||
|
||||
|
||||
def get_libraries_for_user(user):
|
||||
def get_libraries_for_user(user, org=None):
|
||||
"""
|
||||
Return content libraries that the user has permission to view.
|
||||
"""
|
||||
qs = ContentLibrary.objects.all()
|
||||
if org:
|
||||
qs = ContentLibrary.objects.filter(org__short_name=org)
|
||||
else:
|
||||
qs = ContentLibrary.objects.all()
|
||||
return permissions.perms[permissions.CAN_VIEW_THIS_CONTENT_LIBRARY].filter(user, qs)
|
||||
|
||||
|
||||
def get_metadata_from_index(queryset):
|
||||
def get_metadata_from_index(queryset, text_search=None):
|
||||
"""
|
||||
Take a list of ContentLibrary objects and return metadata stored in
|
||||
ContentLibraryIndex.
|
||||
@@ -236,15 +243,33 @@ def get_metadata_from_index(queryset):
|
||||
metadata = None
|
||||
if ContentLibraryIndexer.indexing_is_enabled():
|
||||
try:
|
||||
library_keys = [lib.library_key for lib in queryset]
|
||||
metadata = ContentLibraryIndexer.get_libraries(library_keys)
|
||||
except (LibraryNotIndexedException, KeyError, ElasticConnectionError) as e:
|
||||
library_keys = [str(lib.library_key) for lib in queryset]
|
||||
metadata = ContentLibraryIndexer.get_items(library_keys, text_search=text_search)
|
||||
metadata_dict = {
|
||||
item["id"]: item
|
||||
for item in metadata
|
||||
}
|
||||
metadata = [
|
||||
metadata_dict[key]
|
||||
if key in metadata_dict
|
||||
else None
|
||||
for key in library_keys
|
||||
]
|
||||
except ElasticConnectionError as e:
|
||||
log.exception(e)
|
||||
|
||||
# If ContentLibraryIndex is not available, we query blockstore for a limited set of metadata
|
||||
if metadata is None:
|
||||
uuids = [lib.bundle_uuid for lib in queryset]
|
||||
bundles = get_bundles(uuids)
|
||||
bundles = get_bundles(uuids=uuids, text_search=text_search)
|
||||
|
||||
if text_search:
|
||||
# Bundle APIs can't apply text_search on a bundle's org, so including those results here
|
||||
queryset_org_search = queryset.filter(org__short_name__icontains=text_search)
|
||||
if queryset_org_search.exists():
|
||||
uuids_org_search = [lib.bundle_uuid for lib in queryset_org_search]
|
||||
bundles += get_bundles(uuids=uuids_org_search)
|
||||
|
||||
bundle_dict = {
|
||||
bundle.uuid: {
|
||||
'uuid': bundle.uuid,
|
||||
@@ -254,7 +279,12 @@ def get_metadata_from_index(queryset):
|
||||
}
|
||||
for bundle in bundles
|
||||
}
|
||||
metadata = [bundle_dict[uuid] for uuid in uuids]
|
||||
metadata = [
|
||||
bundle_dict[uuid]
|
||||
if uuid in bundle_dict
|
||||
else None
|
||||
for uuid in uuids
|
||||
]
|
||||
|
||||
libraries = [
|
||||
ContentLibraryMetadata(
|
||||
@@ -271,6 +301,7 @@ def get_metadata_from_index(queryset):
|
||||
has_unpublished_deletes=metadata[i].get('has_unpublished_deletes'),
|
||||
)
|
||||
for i, lib in enumerate(queryset)
|
||||
if metadata[i] is not None
|
||||
]
|
||||
return libraries
|
||||
|
||||
@@ -385,6 +416,22 @@ def get_library_team(library_key):
|
||||
]
|
||||
|
||||
|
||||
def get_library_user_permissions(library_key, user):
|
||||
"""
|
||||
Fetch the specified user's access information. Will return None if no
|
||||
permissions have been granted.
|
||||
"""
|
||||
ref = ContentLibrary.objects.get_by_key(library_key)
|
||||
grant = ref.permission_grants.filter(user=user).first()
|
||||
if grant is None:
|
||||
return None
|
||||
return ContentLibraryPermissionEntry(
|
||||
user=grant.user,
|
||||
group=grant.group,
|
||||
access_level=grant.access_level,
|
||||
)
|
||||
|
||||
|
||||
def set_library_user_permissions(library_key, user, access_level):
|
||||
"""
|
||||
Change the specified user's level of access to this library.
|
||||
@@ -392,6 +439,10 @@ def set_library_user_permissions(library_key, user, access_level):
|
||||
access_level should be one of the AccessLevel values defined above.
|
||||
"""
|
||||
ref = ContentLibrary.objects.get_by_key(library_key)
|
||||
current_grant = get_library_user_permissions(library_key, user)
|
||||
if current_grant and current_grant.access_level == AccessLevel.ADMIN_LEVEL:
|
||||
if not ref.permission_grants.filter(access_level=AccessLevel.ADMIN_LEVEL).exclude(user_id=user.id).exists():
|
||||
raise LibraryPermissionIntegrityError(_('Cannot change or remove the access level for the only admin.'))
|
||||
if access_level is None:
|
||||
ref.permission_grants.filter(user=user).delete()
|
||||
else:
|
||||
@@ -480,28 +531,62 @@ 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
|
||||
"""
|
||||
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
|
||||
metadata = None
|
||||
if LibraryBlockIndexer.indexing_is_enabled():
|
||||
try:
|
||||
filter_terms = {
|
||||
'library_key': [str(library_key)],
|
||||
'is_child': [False],
|
||||
}
|
||||
metadata = [
|
||||
{
|
||||
**item,
|
||||
"id": LibraryUsageLocatorV2.from_string(item['id']),
|
||||
}
|
||||
for item in LibraryBlockIndexer.get_items(filter_terms=filter_terms, text_search=text_search)
|
||||
if item is not None
|
||||
]
|
||||
except (ElasticConnectionError) as e:
|
||||
log.exception(e)
|
||||
|
||||
# If indexing is disabled, or connection to elastic failed
|
||||
if metadata is None:
|
||||
metadata = []
|
||||
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()
|
||||
|
||||
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):
|
||||
@@ -571,7 +656,7 @@ def set_library_block_olx(usage_key, new_olx_str):
|
||||
write_draft_file(draft.uuid, metadata.def_key.olx_path, new_olx_str.encode('utf-8'))
|
||||
# Clear the bundle cache so everyone sees the new block immediately:
|
||||
BundleCache(metadata.def_key.bundle_uuid, draft_name=DRAFT_NAME).clear()
|
||||
LIBRARY_BLOCK_UPDATED.send(sender=None, library_key=usage_key.context_key)
|
||||
LIBRARY_BLOCK_UPDATED.send(sender=None, library_key=usage_key.context_key, usage_key=usage_key)
|
||||
|
||||
|
||||
def create_library_block(library_key, block_type, definition_id):
|
||||
@@ -613,7 +698,7 @@ def create_library_block(library_key, block_type, definition_id):
|
||||
# Clear the bundle cache so everyone sees the new block immediately:
|
||||
BundleCache(ref.bundle_uuid, draft_name=DRAFT_NAME).clear()
|
||||
# Now return the metadata about the new block:
|
||||
LIBRARY_BLOCK_CREATED.send(sender=None, library_key=ref.library_key)
|
||||
LIBRARY_BLOCK_CREATED.send(sender=None, library_key=ref.library_key, usage_key=usage_key)
|
||||
return get_library_block(usage_key)
|
||||
|
||||
|
||||
@@ -666,7 +751,7 @@ def delete_library_block(usage_key, remove_from_parent=True):
|
||||
pass
|
||||
# Clear the bundle cache so everyone sees the deleted block immediately:
|
||||
lib_bundle.cache.clear()
|
||||
LIBRARY_BLOCK_DELETED.send(sender=None, library_key=lib_bundle.library_key)
|
||||
LIBRARY_BLOCK_DELETED.send(sender=None, library_key=lib_bundle.library_key, usage_key=usage_key)
|
||||
|
||||
|
||||
def create_library_block_child(parent_usage_key, block_type, definition_id):
|
||||
@@ -689,6 +774,8 @@ def create_library_block_child(parent_usage_key, block_type, definition_id):
|
||||
include_data = XBlockInclude(link_id=None, block_type=block_type, definition_id=definition_id, usage_hint=None)
|
||||
parent_block.runtime.add_child_include(parent_block, include_data)
|
||||
parent_block.save()
|
||||
ref = ContentLibrary.objects.get_by_key(parent_usage_key.context_key)
|
||||
LIBRARY_BLOCK_UPDATED.send(sender=None, library_key=ref.library_key, usage_key=metadata.usage_key)
|
||||
return metadata
|
||||
|
||||
|
||||
@@ -738,7 +825,7 @@ def add_library_block_static_asset_file(usage_key, file_name, file_content):
|
||||
file_metadata = blockstore_cache.get_bundle_file_metadata_with_cache(
|
||||
bundle_uuid=def_key.bundle_uuid, path=file_path, draft_name=DRAFT_NAME,
|
||||
)
|
||||
LIBRARY_BLOCK_UPDATED.send(sender=None, library_key=lib_bundle.library_key)
|
||||
LIBRARY_BLOCK_UPDATED.send(sender=None, library_key=lib_bundle.library_key, usage_key=usage_key)
|
||||
return LibraryXBlockStaticFile(path=file_metadata.path, url=file_metadata.url, size=file_metadata.size)
|
||||
|
||||
|
||||
@@ -759,7 +846,7 @@ def delete_library_block_static_asset_file(usage_key, file_name):
|
||||
write_draft_file(draft.uuid, file_path, contents=None)
|
||||
# Clear the bundle cache so everyone sees the new file immediately:
|
||||
lib_bundle.cache.clear()
|
||||
LIBRARY_BLOCK_UPDATED.send(sender=None, library_key=lib_bundle.library_key)
|
||||
LIBRARY_BLOCK_UPDATED.send(sender=None, library_key=lib_bundle.library_key, usage_key=usage_key)
|
||||
|
||||
|
||||
def get_allowed_block_types(library_key): # pylint: disable=unused-argument
|
||||
@@ -886,7 +973,7 @@ def publish_changes(library_key):
|
||||
return # If there is no draft, no action is needed.
|
||||
LibraryBundle(library_key, ref.bundle_uuid).cache.clear()
|
||||
LibraryBundle(library_key, ref.bundle_uuid, draft_name=DRAFT_NAME).cache.clear()
|
||||
CONTENT_LIBRARY_UPDATED.send(sender=None, library_key=library_key)
|
||||
CONTENT_LIBRARY_UPDATED.send(sender=None, library_key=library_key, update_blocks=True)
|
||||
|
||||
|
||||
def revert_changes(library_key):
|
||||
@@ -902,4 +989,4 @@ def revert_changes(library_key):
|
||||
else:
|
||||
return # If there is no draft, no action is needed.
|
||||
LibraryBundle(library_key, ref.bundle_uuid, draft_name=DRAFT_NAME).cache.clear()
|
||||
CONTENT_LIBRARY_UPDATED.send(sender=None, library_key=library_key)
|
||||
CONTENT_LIBRARY_UPDATED.send(sender=None, library_key=library_key, update_blocks=True)
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
1. Index libraries in elasticsearch
|
||||
-----------------------------------
|
||||
|
||||
Status
|
||||
------
|
||||
|
||||
Accepted
|
||||
|
||||
Context
|
||||
-------
|
||||
|
||||
The new content libraries reside in blockstore instead of edx-platform's models,
|
||||
which means that we are no longer able to query databases to get complete
|
||||
metadata quickly about one or more libraries/xblock anymore. Blockstore can't
|
||||
index them either because most of the data resides on the filesystem, S3 or
|
||||
other non-queryable stores. The current method to get the metadata of a library
|
||||
involves requesting blockstore for the metadata, which in turn reads metadata
|
||||
from files stored in the above mentioned storage systems. This process is
|
||||
repeated for every library if data is required for a list of libraries. A
|
||||
similar process is followed for XBlocks too.
|
||||
|
||||
This is a very inefficient way to fetch metadata for a list of
|
||||
libraries/xblocks, and makes it even harder to filter/query them.
|
||||
|
||||
Decision
|
||||
--------
|
||||
|
||||
Index the libraries and xblocks in elasticsearch to make them queryable. These
|
||||
indexes are updated whenever a library or XBlock is updated through the studio.
|
||||
A management command ``redindex_content_library`` is also added for clearing
|
||||
indexes or reindex libraries manually.
|
||||
|
||||
Given that elasticsearch hasn't been a required dependency of studio till now,
|
||||
fallbacks have been implemented in case elastic is down or hasn't been enabled
|
||||
yet.
|
||||
|
||||
Consequences
|
||||
------------
|
||||
|
||||
List APIs are significantly faster and are able to support filtering and
|
||||
searching now that the metadata can be queried using elasticsearch. This also
|
||||
means that if the indexes are empty or outdated, the API results would be too.
|
||||
|
||||
Signal handlers update the indexes whenever libraries and xblocks are created,
|
||||
updated or deleted through the studio. But if they are modified directly at the
|
||||
source or without using the studio APIs, then the indexes will get out of date
|
||||
too until reindexing is performed using management commands or another
|
||||
modification operation causes a reindex.
|
||||
|
||||
The fallback method described above returns only a subset of the complete
|
||||
response usually returned by Elastic. Attributes which require scanning through
|
||||
multiple files are exempted from this minimal response.
|
||||
|
||||
We use schema versions to avoid querying old indexes, which could otherwise
|
||||
result in unforeseen errors. This version is incremented every time the
|
||||
structure of the schema changes. This also means that a reindexing will be
|
||||
needed after any upgrade which changes an index schema.
|
||||
@@ -1,11 +1,14 @@
|
||||
""" Code to allow indexing content libraries """
|
||||
|
||||
import logging
|
||||
from abc import ABC, abstractmethod
|
||||
|
||||
from django.conf import settings
|
||||
from django.dispatch import receiver
|
||||
from elasticsearch.exceptions import ConnectionError as ElasticConnectionError
|
||||
from search.elastic import _translate_hits, RESERVED_CHARACTERS
|
||||
from search.search_engine_base import SearchEngine
|
||||
from opaque_keys.edx.locator import LibraryUsageLocatorV2
|
||||
|
||||
from openedx.core.djangoapps.content_libraries.constants import DRAFT_NAME
|
||||
from openedx.core.djangoapps.content_libraries.signals import (
|
||||
@@ -25,116 +28,234 @@ log = logging.getLogger(__name__)
|
||||
MAX_SIZE = 10000 # 10000 is the maximum records elastic is able to return in a single result. Defaults to 10.
|
||||
|
||||
|
||||
class LibraryNotIndexedException(Exception):
|
||||
class SearchIndexerBase(ABC):
|
||||
"""
|
||||
Library supplied wasn't indexed in ElasticSearch
|
||||
Abstract Base Class for implementing library search indexers.
|
||||
"""
|
||||
|
||||
|
||||
class ContentLibraryIndexer:
|
||||
"""
|
||||
Class to perform indexing for blockstore-based content libraries
|
||||
"""
|
||||
|
||||
INDEX_NAME = "content_library_index"
|
||||
LIBRARY_DOCUMENT_TYPE = "content_library"
|
||||
|
||||
INDEX_NAME = None
|
||||
DOCUMENT_TYPE = None
|
||||
ENABLE_INDEXING_KEY = None
|
||||
SCHEMA_VERSION = 0
|
||||
SEARCH_KWARGS = {
|
||||
# Set this to True or 'wait_for' if immediate refresh is required after any update.
|
||||
# See elastic docs for more information.
|
||||
'refresh': False
|
||||
}
|
||||
|
||||
@classmethod
|
||||
def index_libraries(cls, library_keys):
|
||||
@abstractmethod
|
||||
def get_item_definition(cls, item):
|
||||
"""
|
||||
Returns a serializable dictionary which can be stored in elasticsearch.
|
||||
"""
|
||||
|
||||
@classmethod
|
||||
def index_items(cls, items):
|
||||
"""
|
||||
Index the specified libraries. If they already exist, replace them with new ones.
|
||||
"""
|
||||
searcher = SearchEngine.get_search_engine(cls.INDEX_NAME)
|
||||
items = [cls.get_item_definition(item) for item in items]
|
||||
return searcher.index(cls.DOCUMENT_TYPE, items, **cls.SEARCH_KWARGS)
|
||||
|
||||
library_dicts = []
|
||||
|
||||
for library_key in library_keys:
|
||||
ref = ContentLibrary.objects.get_by_key(library_key)
|
||||
lib_bundle = LibraryBundle(library_key, ref.bundle_uuid, draft_name=DRAFT_NAME)
|
||||
num_blocks = len(lib_bundle.get_top_level_usages())
|
||||
last_published = lib_bundle.get_last_published_time()
|
||||
last_published_str = None
|
||||
if last_published:
|
||||
last_published_str = last_published.strftime('%Y-%m-%dT%H:%M:%SZ')
|
||||
(has_unpublished_changes, has_unpublished_deletes) = lib_bundle.has_changes()
|
||||
|
||||
bundle_metadata = get_bundle(ref.bundle_uuid)
|
||||
|
||||
# NOTE: Increment ContentLibraryIndexer.SCHEMA_VERSION if the following schema is updated to avoid dealing
|
||||
# with outdated indexes which might cause errors due to missing/invalid attributes.
|
||||
library_dict = {
|
||||
"schema_version": ContentLibraryIndexer.SCHEMA_VERSION,
|
||||
"id": str(library_key),
|
||||
"uuid": str(bundle_metadata.uuid),
|
||||
"title": bundle_metadata.title,
|
||||
"description": bundle_metadata.description,
|
||||
"num_blocks": num_blocks,
|
||||
"version": bundle_metadata.latest_version,
|
||||
"last_published": last_published_str,
|
||||
"has_unpublished_changes": has_unpublished_changes,
|
||||
"has_unpublished_deletes": has_unpublished_deletes,
|
||||
@classmethod
|
||||
def get_items(cls, ids=None, filter_terms=None, text_search=None):
|
||||
"""
|
||||
Retrieve a list of items from the index.
|
||||
Arguments:
|
||||
ids - List of ids to be searched for in the index
|
||||
filter_terms - Dictionary of filters to be applied
|
||||
text_search - String which is used to do a text search in the supported indexes.
|
||||
"""
|
||||
if filter_terms is None:
|
||||
filter_terms = {}
|
||||
if ids is not None:
|
||||
filter_terms = {
|
||||
"id": [str(item) for item in ids],
|
||||
"schema_version": [cls.SCHEMA_VERSION],
|
||||
**filter_terms,
|
||||
}
|
||||
library_dicts.append(library_dict)
|
||||
|
||||
return searcher.index(cls.LIBRARY_DOCUMENT_TYPE, library_dicts)
|
||||
if text_search:
|
||||
response = cls._perform_elastic_search(filter_terms, text_search)
|
||||
else:
|
||||
searcher = SearchEngine.get_search_engine(cls.INDEX_NAME)
|
||||
response = searcher.search(doc_type=cls.DOCUMENT_TYPE, field_dictionary=filter_terms, size=MAX_SIZE)
|
||||
|
||||
response = [result["data"] for result in response["results"]]
|
||||
return sorted(response, key=lambda i: i["id"])
|
||||
|
||||
@classmethod
|
||||
def get_libraries(cls, library_keys):
|
||||
def remove_items(cls, ids):
|
||||
"""
|
||||
Retrieve a list of libraries from the index
|
||||
Remove the provided ids from the index
|
||||
"""
|
||||
searcher = SearchEngine.get_search_engine(cls.INDEX_NAME)
|
||||
library_keys_str = [str(key) for key in library_keys]
|
||||
response = searcher.search(
|
||||
doc_type=cls.LIBRARY_DOCUMENT_TYPE,
|
||||
field_dictionary={
|
||||
"id": library_keys_str,
|
||||
"schema_version": ContentLibraryIndexer.SCHEMA_VERSION
|
||||
},
|
||||
size=MAX_SIZE,
|
||||
)
|
||||
|
||||
# Search results may not retain the original order of keys - we use this
|
||||
# dict to construct a list in the original order of library_keys
|
||||
response_dict = {
|
||||
result["data"]["id"]: result["data"]
|
||||
for result in response["results"]
|
||||
}
|
||||
if len(response_dict) != len(library_keys_str):
|
||||
missing = set(library_keys_str) - set(response_dict.keys())
|
||||
raise LibraryNotIndexedException("Keys not found in index: {}".format(missing))
|
||||
return [
|
||||
response_dict[key]
|
||||
for key in library_keys_str
|
||||
]
|
||||
ids_str = [str(i) for i in ids]
|
||||
searcher.remove(cls.DOCUMENT_TYPE, ids_str, **cls.SEARCH_KWARGS)
|
||||
|
||||
@classmethod
|
||||
def remove_libraries(cls, library_keys):
|
||||
def remove_all_items(cls):
|
||||
"""
|
||||
Remove the provided library_keys from the index
|
||||
Remove all items from the index
|
||||
"""
|
||||
searcher = SearchEngine.get_search_engine(cls.INDEX_NAME)
|
||||
ids_str = [str(key) for key in library_keys]
|
||||
searcher.remove(cls.LIBRARY_DOCUMENT_TYPE, ids_str)
|
||||
|
||||
@classmethod
|
||||
def remove_all_libraries(cls):
|
||||
"""
|
||||
Remove all libraries from the index
|
||||
"""
|
||||
searcher = SearchEngine.get_search_engine(cls.INDEX_NAME)
|
||||
response = searcher.search(doc_type=cls.LIBRARY_DOCUMENT_TYPE, filter_dictionary={}, size=MAX_SIZE)
|
||||
response = searcher.search(doc_type=cls.DOCUMENT_TYPE, filter_dictionary={}, size=MAX_SIZE)
|
||||
ids = [result["data"]["id"] for result in response["results"]]
|
||||
searcher.remove(cls.LIBRARY_DOCUMENT_TYPE, ids)
|
||||
searcher.remove(cls.DOCUMENT_TYPE, ids, **cls.SEARCH_KWARGS)
|
||||
|
||||
@classmethod
|
||||
def indexing_is_enabled(cls):
|
||||
"""
|
||||
Checks to see if the indexing feature is enabled
|
||||
"""
|
||||
return settings.FEATURES.get("ENABLE_CONTENT_LIBRARY_INDEX", False)
|
||||
return settings.FEATURES.get(cls.ENABLE_INDEXING_KEY, False)
|
||||
|
||||
@classmethod
|
||||
def _perform_elastic_search(cls, filter_terms, text_search):
|
||||
"""
|
||||
Build a query and search directly on elasticsearch
|
||||
"""
|
||||
searcher = SearchEngine.get_search_engine(cls.INDEX_NAME)
|
||||
return _translate_hits(searcher._es.search( # pylint: disable=protected-access
|
||||
doc_type=cls.DOCUMENT_TYPE,
|
||||
index=searcher.index_name,
|
||||
body=cls.build_elastic_query(filter_terms, text_search),
|
||||
size=MAX_SIZE
|
||||
))
|
||||
|
||||
@staticmethod
|
||||
def build_elastic_query(filter_terms, text_search):
|
||||
"""
|
||||
Build and return an elastic query for doing text search on a library
|
||||
"""
|
||||
# Remove reserved characters (and ") from the text to prevent unexpected errors.
|
||||
text_search_normalised = text_search.translate(text_search.maketrans('', '', RESERVED_CHARACTERS + '"'))
|
||||
text_search_normalised = text_search.replace('-', ' ')
|
||||
# Wrap with asterix to enable partial matches
|
||||
text_search_normalised = "*{}*".format(text_search_normalised)
|
||||
terms = [
|
||||
{
|
||||
'terms': {
|
||||
item: filter_terms[item]
|
||||
}
|
||||
}
|
||||
for item in filter_terms
|
||||
]
|
||||
return {
|
||||
'query': {
|
||||
'filtered': {
|
||||
'query': {
|
||||
'bool': {
|
||||
'should': [
|
||||
{
|
||||
'query_string': {
|
||||
'query': text_search_normalised,
|
||||
"fields": ["content.*"],
|
||||
"minimum_should_match": "100%",
|
||||
},
|
||||
},
|
||||
# Add a special wildcard search for id, as it contains a ":" character which is
|
||||
# filtered out in query_string
|
||||
{
|
||||
'wildcard': {
|
||||
'id': {
|
||||
'value': '*{}*'.format(text_search),
|
||||
}
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
'filter': {
|
||||
'bool': {
|
||||
'must': terms
|
||||
}
|
||||
}
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
class ContentLibraryIndexer(SearchIndexerBase):
|
||||
"""
|
||||
Class to perform indexing for blockstore-based content libraries
|
||||
"""
|
||||
|
||||
INDEX_NAME = "content_library_index"
|
||||
ENABLE_INDEXING_KEY = "ENABLE_CONTENT_LIBRARY_INDEX"
|
||||
DOCUMENT_TYPE = "content_library"
|
||||
SCHEMA_VERSION = 0
|
||||
|
||||
@classmethod
|
||||
def get_item_definition(cls, item):
|
||||
ref = ContentLibrary.objects.get_by_key(item)
|
||||
lib_bundle = LibraryBundle(item, ref.bundle_uuid, draft_name=DRAFT_NAME)
|
||||
num_blocks = len(lib_bundle.get_top_level_usages())
|
||||
last_published = lib_bundle.get_last_published_time()
|
||||
last_published_str = None
|
||||
if last_published:
|
||||
last_published_str = last_published.strftime('%Y-%m-%dT%H:%M:%SZ')
|
||||
(has_unpublished_changes, has_unpublished_deletes) = lib_bundle.has_changes()
|
||||
|
||||
bundle_metadata = get_bundle(ref.bundle_uuid)
|
||||
|
||||
# NOTE: Increment ContentLibraryIndexer.SCHEMA_VERSION if the following schema is updated to avoid dealing
|
||||
# with outdated indexes which might cause errors due to missing/invalid attributes.
|
||||
return {
|
||||
"schema_version": ContentLibraryIndexer.SCHEMA_VERSION,
|
||||
"id": str(item),
|
||||
"uuid": str(bundle_metadata.uuid),
|
||||
"title": bundle_metadata.title,
|
||||
"description": bundle_metadata.description,
|
||||
"num_blocks": num_blocks,
|
||||
"version": bundle_metadata.latest_version,
|
||||
"last_published": last_published_str,
|
||||
"has_unpublished_changes": has_unpublished_changes,
|
||||
"has_unpublished_deletes": has_unpublished_deletes,
|
||||
# only 'content' field is analyzed by elastisearch, and allows text-search
|
||||
"content": {
|
||||
"id": str(item),
|
||||
"title": bundle_metadata.title,
|
||||
"description": bundle_metadata.description,
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
class LibraryBlockIndexer(SearchIndexerBase):
|
||||
"""
|
||||
Class to perform indexing on the XBlocks in content libraries.
|
||||
"""
|
||||
|
||||
INDEX_NAME = "content_library_index"
|
||||
ENABLE_INDEXING_KEY = "ENABLE_CONTENT_LIBRARY_INDEX"
|
||||
DOCUMENT_TYPE = "content_library_block"
|
||||
SCHEMA_VERSION = 0
|
||||
|
||||
@classmethod
|
||||
def get_item_definition(cls, item):
|
||||
from openedx.core.djangoapps.content_libraries.api import get_block_display_name, _lookup_usage_key
|
||||
|
||||
def_key, lib_bundle = _lookup_usage_key(item)
|
||||
is_child = item in lib_bundle.get_bundle_includes().keys()
|
||||
|
||||
# NOTE: Increment LibraryBlockIndexer.SCHEMA_VERSION if the following schema is updated to avoid dealing
|
||||
# with outdated indexes which might cause errors due to missing/invalid attributes.
|
||||
return {
|
||||
"schema_version": LibraryBlockIndexer.SCHEMA_VERSION,
|
||||
"id": str(item),
|
||||
"library_key": str(lib_bundle.library_key),
|
||||
"is_child": is_child,
|
||||
"def_key": str(def_key),
|
||||
"display_name": get_block_display_name(def_key),
|
||||
"block_type": def_key.block_type,
|
||||
"has_unpublished_changes": lib_bundle.does_definition_have_unpublished_changes(def_key),
|
||||
# only 'content' field is analyzed by elastisearch, and allows text-search
|
||||
"content": {
|
||||
"id": str(item),
|
||||
"display_name": get_block_display_name(def_key),
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
@receiver(CONTENT_LIBRARY_CREATED)
|
||||
@@ -148,7 +269,13 @@ def index_library(sender, library_key, **kwargs): # pylint: disable=unused-argu
|
||||
"""
|
||||
if ContentLibraryIndexer.indexing_is_enabled():
|
||||
try:
|
||||
ContentLibraryIndexer.index_libraries([library_key])
|
||||
ContentLibraryIndexer.index_items([library_key])
|
||||
if kwargs.get('update_blocks', False):
|
||||
blocks = LibraryBlockIndexer.get_items(filter_terms={
|
||||
'library_key': str(library_key)
|
||||
})
|
||||
usage_keys = [LibraryUsageLocatorV2.from_string(block['id']) for block in blocks]
|
||||
LibraryBlockIndexer.index_items(usage_keys)
|
||||
except ElasticConnectionError as e:
|
||||
log.exception(e)
|
||||
|
||||
@@ -160,6 +287,35 @@ def remove_library_index(sender, library_key, **kwargs): # pylint: disable=unus
|
||||
"""
|
||||
if ContentLibraryIndexer.indexing_is_enabled():
|
||||
try:
|
||||
ContentLibraryIndexer.remove_libraries([library_key])
|
||||
ContentLibraryIndexer.remove_items([library_key])
|
||||
blocks = LibraryBlockIndexer.get_items(filter_terms={
|
||||
'library_key': str(library_key)
|
||||
})
|
||||
LibraryBlockIndexer.remove_items([block['id'] for block in blocks])
|
||||
except ElasticConnectionError as e:
|
||||
log.exception(e)
|
||||
|
||||
|
||||
@receiver(LIBRARY_BLOCK_CREATED)
|
||||
@receiver(LIBRARY_BLOCK_UPDATED)
|
||||
def index_block(sender, usage_key, **kwargs): # pylint: disable=unused-argument
|
||||
"""
|
||||
Index block metadata when created
|
||||
"""
|
||||
if LibraryBlockIndexer.indexing_is_enabled():
|
||||
try:
|
||||
LibraryBlockIndexer.index_items([usage_key])
|
||||
except ConnectionError as e:
|
||||
log.exception(e)
|
||||
|
||||
|
||||
@receiver(LIBRARY_BLOCK_DELETED)
|
||||
def remove_block_index(sender, usage_key, **kwargs): # pylint: disable=unused-argument
|
||||
"""
|
||||
Remove the block from the index when deleted
|
||||
"""
|
||||
if LibraryBlockIndexer.indexing_is_enabled():
|
||||
try:
|
||||
LibraryBlockIndexer.remove_items([usage_key])
|
||||
except ConnectionError as e:
|
||||
log.exception(e)
|
||||
|
||||
@@ -162,16 +162,23 @@ class LibraryBundle(object):
|
||||
except KeyError:
|
||||
return None
|
||||
|
||||
def get_all_usages(self):
|
||||
"""
|
||||
Get usage keys of all the blocks in this bundle
|
||||
"""
|
||||
usage_keys = []
|
||||
for olx_file_path in self.get_olx_files():
|
||||
block_type, usage_id, _unused = olx_file_path.split('/')
|
||||
usage_key = LibraryUsageLocatorV2(self.library_key, block_type, usage_id)
|
||||
usage_keys.append(usage_key)
|
||||
|
||||
return usage_keys
|
||||
|
||||
def get_top_level_usages(self):
|
||||
"""
|
||||
Get the set of usage keys in this bundle that have no parent.
|
||||
"""
|
||||
own_usage_keys = []
|
||||
for olx_file_path in self.get_olx_files():
|
||||
block_type, usage_id, _unused = olx_file_path.split('/')
|
||||
usage_key = LibraryUsageLocatorV2(self.library_key, block_type, usage_id)
|
||||
own_usage_keys.append(usage_key)
|
||||
|
||||
own_usage_keys = self.get_all_usages()
|
||||
usage_keys_with_parents = self.get_bundle_includes().keys()
|
||||
return [usage_key for usage_key in own_usage_keys if usage_key not in usage_keys_with_parents]
|
||||
|
||||
|
||||
@@ -7,7 +7,9 @@ from textwrap import dedent
|
||||
|
||||
from django.core.management import BaseCommand
|
||||
from opaque_keys.edx.locator import LibraryLocatorV2
|
||||
from openedx.core.djangoapps.content_libraries.libraries_index import ContentLibraryIndexer
|
||||
from openedx.core.djangoapps.content_libraries.api import DRAFT_NAME
|
||||
from openedx.core.djangoapps.content_libraries.libraries_index import ContentLibraryIndexer, LibraryBlockIndexer
|
||||
from openedx.core.djangoapps.content_libraries.library_bundle import LibraryBundle
|
||||
from openedx.core.djangoapps.content_libraries.models import ContentLibrary
|
||||
|
||||
from cms.djangoapps.contentstore.management.commands.prompt import query_yes_no
|
||||
@@ -57,7 +59,8 @@ class Command(BaseCommand):
|
||||
if options['clear-all']:
|
||||
if options['force'] or query_yes_no(self.CONFIRMATION_PROMPT_CLEAR, default="no"):
|
||||
logging.info("Removing all libraries from the index")
|
||||
ContentLibraryIndexer.remove_all_libraries()
|
||||
ContentLibraryIndexer.remove_all_items()
|
||||
LibraryBlockIndexer.remove_all_items()
|
||||
return
|
||||
|
||||
if options['all']:
|
||||
@@ -70,4 +73,9 @@ class Command(BaseCommand):
|
||||
logging.info("Indexing libraries: {}".format(options['library_ids']))
|
||||
library_keys = list(map(LibraryLocatorV2.from_string, options['library_ids']))
|
||||
|
||||
ContentLibraryIndexer.index_libraries(library_keys)
|
||||
ContentLibraryIndexer.index_items(library_keys)
|
||||
|
||||
for library_key in library_keys:
|
||||
ref = ContentLibrary.objects.get_by_key(library_key)
|
||||
lib_bundle = LibraryBundle(library_key, ref.bundle_uuid, draft_name=DRAFT_NAME)
|
||||
LibraryBlockIndexer.index_items(lib_bundle.get_all_usages())
|
||||
|
||||
@@ -57,12 +57,21 @@ class ContentLibraryPermissionLevelSerializer(serializers.Serializer):
|
||||
access_level = serializers.ChoiceField(choices=ContentLibraryPermission.ACCESS_LEVEL_CHOICES)
|
||||
|
||||
|
||||
class ContentLibraryAddPermissionByEmailSerializer(serializers.Serializer):
|
||||
"""
|
||||
Serializer for adding a new user and granting their access level via their email address.
|
||||
"""
|
||||
access_level = serializers.ChoiceField(choices=ContentLibraryPermission.ACCESS_LEVEL_CHOICES)
|
||||
email = serializers.EmailField()
|
||||
|
||||
|
||||
class ContentLibraryPermissionSerializer(ContentLibraryPermissionLevelSerializer):
|
||||
"""
|
||||
Serializer for a ContentLibraryPermission object, which grants either a user
|
||||
or a group permission to view a content library.
|
||||
"""
|
||||
user_id = serializers.IntegerField(source="user.id", allow_null=True)
|
||||
email = serializers.EmailField(source="user.email", read_only=True, default=None)
|
||||
username = serializers.CharField(source="user.username", read_only=True, default=None)
|
||||
group_name = serializers.CharField(source="group.name", allow_null=True, allow_blank=False, default=None)
|
||||
|
||||
|
||||
@@ -72,7 +81,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
|
||||
|
||||
@@ -5,8 +5,8 @@ Content libraries related signals.
|
||||
from django.dispatch import Signal
|
||||
|
||||
CONTENT_LIBRARY_CREATED = Signal(providing_args=['library_key'])
|
||||
CONTENT_LIBRARY_UPDATED = Signal(providing_args=['library_key'])
|
||||
CONTENT_LIBRARY_UPDATED = Signal(providing_args=['library_key', 'update_blocks'])
|
||||
CONTENT_LIBRARY_DELETED = Signal(providing_args=['library_key'])
|
||||
LIBRARY_BLOCK_CREATED = Signal(providing_args=['library_key'])
|
||||
LIBRARY_BLOCK_DELETED = Signal(providing_args=['library_key'])
|
||||
LIBRARY_BLOCK_UPDATED = Signal(providing_args=['library_key'])
|
||||
LIBRARY_BLOCK_CREATED = Signal(providing_args=['library_key', 'usage_key'])
|
||||
LIBRARY_BLOCK_DELETED = Signal(providing_args=['library_key', 'usage_key'])
|
||||
LIBRARY_BLOCK_UPDATED = Signal(providing_args=['library_key', 'usage_key'])
|
||||
|
||||
@@ -4,14 +4,18 @@ Tests for Blockstore-based Content Libraries
|
||||
"""
|
||||
from contextlib import contextmanager
|
||||
from io import BytesIO
|
||||
from mock import patch
|
||||
from urllib.parse import urlencode
|
||||
import unittest
|
||||
|
||||
from django.conf import settings
|
||||
from django.test.utils import override_settings
|
||||
from organizations.models import Organization
|
||||
from rest_framework.test import APITestCase, APIClient
|
||||
from search.search_engine_base import SearchEngine
|
||||
|
||||
from student.tests.factories import UserFactory
|
||||
from openedx.core.djangoapps.content_libraries.libraries_index import MAX_SIZE
|
||||
from openedx.core.djangolib.testing.utils import skip_unless_cms
|
||||
from openedx.core.lib import blockstore_api
|
||||
|
||||
@@ -26,7 +30,7 @@ URL_LIB_LINKS = URL_LIB_DETAIL + 'links/' # Get the list of links in this libra
|
||||
URL_LIB_COMMIT = URL_LIB_DETAIL + 'commit/' # Commit (POST) or revert (DELETE) all pending changes to this library
|
||||
URL_LIB_BLOCKS = URL_LIB_DETAIL + 'blocks/' # Get the list of XBlocks in this library, or add a new one
|
||||
URL_LIB_TEAM = URL_LIB_DETAIL + 'team/' # Get the list of users/groups authorized to use this library
|
||||
URL_LIB_TEAM_USER = URL_LIB_TEAM + 'user/{user_id}/' # Add/edit/remove a user's permission to use this library
|
||||
URL_LIB_TEAM_USER = URL_LIB_TEAM + 'user/{username}/' # Add/edit/remove a user's permission to use this library
|
||||
URL_LIB_TEAM_GROUP = URL_LIB_TEAM + 'group/{group_name}/' # Add/edit/remove a group's permission to use this library
|
||||
URL_LIB_BLOCK = URL_PREFIX + 'blocks/{block_key}/' # Get data about a block, or delete it
|
||||
URL_LIB_BLOCK_OLX = URL_LIB_BLOCK + 'olx/' # Get or set the OLX of the specified XBlock
|
||||
@@ -42,6 +46,44 @@ URL_BLOCK_METADATA_URL = '/api/xblock/v2/xblocks/{block_key}/'
|
||||
requires_blockstore = unittest.skipUnless(settings.RUN_BLOCKSTORE_TESTS, "Requires a running Blockstore server")
|
||||
|
||||
|
||||
def elasticsearch_test(func):
|
||||
"""
|
||||
Decorator for tests which connect to elasticsearch when needed
|
||||
"""
|
||||
# This is disabled by default. Set to True if the elasticsearch engine is needed to test parts of code.
|
||||
if settings.ENABLE_ELASTICSEARCH_FOR_TESTS:
|
||||
func = override_settings(SEARCH_ENGINE="search.elastic.ElasticSearchEngine")(func)
|
||||
func = override_settings(ELASTIC_SEARCH_CONFIG=[{
|
||||
'use_ssl': settings.TEST_ELASTICSEARCH_USE_SSL,
|
||||
'host': settings.TEST_ELASTICSEARCH_HOST,
|
||||
'port': settings.TEST_ELASTICSEARCH_PORT,
|
||||
}])(func)
|
||||
func = patch("openedx.core.djangoapps.content_libraries.libraries_index.SearchIndexerBase.SEARCH_KWARGS", new={
|
||||
'refresh': 'wait_for'
|
||||
})(func)
|
||||
return func
|
||||
else:
|
||||
@classmethod
|
||||
def mock_perform(cls, filter_terms, text_search):
|
||||
# pylint: disable=no-member
|
||||
return SearchEngine.get_search_engine(cls.INDEX_NAME).search(
|
||||
doc_type=cls.DOCUMENT_TYPE,
|
||||
field_dictionary=filter_terms,
|
||||
query_string=text_search,
|
||||
size=MAX_SIZE
|
||||
)
|
||||
|
||||
func = patch(
|
||||
"openedx.core.djangoapps.content_libraries.libraries_index.SearchIndexerBase.SEARCH_KWARGS",
|
||||
new={}
|
||||
)(func)
|
||||
func = patch(
|
||||
"openedx.core.djangoapps.content_libraries.libraries_index.SearchIndexerBase._perform_elastic_search",
|
||||
new=mock_perform
|
||||
)(func)
|
||||
return func
|
||||
|
||||
|
||||
@requires_blockstore
|
||||
@skip_unless_cms # Content Libraries REST API is only available in Studio
|
||||
class ContentLibrariesRestApiTest(APITestCase):
|
||||
@@ -124,10 +166,12 @@ class ContentLibrariesRestApiTest(APITestCase):
|
||||
yield
|
||||
self.client = old_client # pylint: disable=attribute-defined-outside-init
|
||||
|
||||
def _create_library(self, slug, title, description="", expect_response=200):
|
||||
def _create_library(self, slug, title, description="", org=None, expect_response=200):
|
||||
""" Create a library """
|
||||
if org is None:
|
||||
org = self.organization.short_name
|
||||
return self._api('post', URL_LIB_CREATE, {
|
||||
"org": self.organization.short_name,
|
||||
"org": org,
|
||||
"slug": slug,
|
||||
"title": title,
|
||||
"description": description,
|
||||
@@ -181,13 +225,25 @@ class ContentLibrariesRestApiTest(APITestCase):
|
||||
""" Get the list of users/groups authorized to use this library """
|
||||
return self._api('get', URL_LIB_TEAM.format(lib_key=lib_key), None, expect_response)
|
||||
|
||||
def _set_user_access_level(self, lib_key, user_id, access_level, expect_response=200):
|
||||
def _get_user_access_level(self, lib_key, username, expect_response=200):
|
||||
""" Fetch a user's access level """
|
||||
url = URL_LIB_TEAM_USER.format(lib_key=lib_key, username=username)
|
||||
return self._api('get', url, None, expect_response)
|
||||
|
||||
def _add_user_by_email(self, lib_key, email, access_level, expect_response=200):
|
||||
""" Add a user of a specified permission level by their email address. """
|
||||
url = URL_LIB_TEAM.format(lib_key=lib_key)
|
||||
return self._api('post', url, {"access_level": access_level, "email": email}, expect_response)
|
||||
|
||||
def _set_user_access_level(self, lib_key, username, access_level, expect_response=200):
|
||||
""" Change the specified user's access level """
|
||||
url = URL_LIB_TEAM_USER.format(lib_key=lib_key, user_id=user_id)
|
||||
if access_level is None:
|
||||
return self._api('delete', url, None, expect_response)
|
||||
else:
|
||||
return self._api('put', url, {"access_level": access_level}, expect_response)
|
||||
url = URL_LIB_TEAM_USER.format(lib_key=lib_key, username=username)
|
||||
return self._api('put', url, {"access_level": access_level}, expect_response)
|
||||
|
||||
def _remove_user_access(self, lib_key, username, expect_response=200):
|
||||
""" Should effectively be the same as the above with access_level=None, but using the delete HTTP verb. """
|
||||
url = URL_LIB_TEAM_USER.format(lib_key=lib_key, username=username)
|
||||
return self._api('delete', url, None, expect_response)
|
||||
|
||||
def _set_group_access_level(self, lib_key, group_name, access_level, expect_response=200):
|
||||
""" Change the specified group's access level """
|
||||
@@ -197,9 +253,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 """
|
||||
|
||||
@@ -2,7 +2,6 @@
|
||||
"""
|
||||
Tests for Blockstore-based Content Libraries
|
||||
"""
|
||||
import unittest
|
||||
from uuid import UUID
|
||||
|
||||
import ddt
|
||||
@@ -10,13 +9,14 @@ from django.conf import settings
|
||||
from django.contrib.auth.models import Group
|
||||
from django.test.utils import override_settings
|
||||
from mock import patch
|
||||
from organizations.models import Organization
|
||||
|
||||
from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrariesRestApiTest
|
||||
from openedx.core.djangoapps.content_libraries.api import BlockLimitReachedError
|
||||
from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrariesRestApiTest, elasticsearch_test
|
||||
from student.tests.factories import UserFactory
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
@elasticsearch_test
|
||||
class ContentLibrariesTest(ContentLibrariesRestApiTest):
|
||||
"""
|
||||
General tests for Blockstore-based Content Libraries
|
||||
@@ -87,8 +87,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)
|
||||
@override_settings(SEARCH_ENGINE="search.tests.mock_search_engine.MockSearchEngine")
|
||||
@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
|
||||
@@ -104,7 +103,8 @@ class ContentLibrariesTest(ContentLibrariesRestApiTest):
|
||||
|
||||
result = self._list_libraries()
|
||||
self.assertEqual(len(result), 2)
|
||||
self.assertEqual(result[0], lib1)
|
||||
self.assertIn(lib1, result)
|
||||
self.assertIn(lib2, result)
|
||||
result = self._list_libraries({'pagination': 'true'})
|
||||
self.assertEqual(len(result['results']), 2)
|
||||
self.assertEqual(result['next'], None)
|
||||
@@ -127,6 +127,31 @@ class ContentLibrariesTest(ContentLibrariesRestApiTest):
|
||||
self.assertEqual(len(result['results']), 1)
|
||||
self.assertEqual(result['next'], None)
|
||||
|
||||
@ddt.data(True, False)
|
||||
def test_library_filters(self, is_indexing_enabled):
|
||||
"""
|
||||
Test the filters in the list libraries API
|
||||
"""
|
||||
with override_settings(FEATURES={**settings.FEATURES, 'ENABLE_CONTENT_LIBRARY_INDEX': is_indexing_enabled}):
|
||||
self._create_library(slug="test-lib1", title="Foo", description="Bar")
|
||||
self._create_library(slug="test-lib2", title="Library-Title-2", description="Bar2")
|
||||
self._create_library(slug="l3", title="Library-Title-3", description="Description")
|
||||
|
||||
Organization.objects.get_or_create(
|
||||
short_name="org-test",
|
||||
defaults={"name": "Content Libraries Tachyon Exploration & Survey Team"},
|
||||
)
|
||||
self._create_library(slug="l4", title="Library-Title-4", description="Library-Description", org='org-test')
|
||||
self._create_library(slug="l5", title="Library-Title-5", description="Library-Description", org='org-test')
|
||||
|
||||
self.assertEqual(len(self._list_libraries()), 5)
|
||||
self.assertEqual(len(self._list_libraries({'org': 'org-test'})), 2)
|
||||
self.assertEqual(len(self._list_libraries({'text_search': 'test-lib'})), 2)
|
||||
self.assertEqual(len(self._list_libraries({'text_search': 'library-title'})), 4)
|
||||
self.assertEqual(len(self._list_libraries({'text_search': 'bar'})), 2)
|
||||
self.assertEqual(len(self._list_libraries({'text_search': 'org-tes'})), 2)
|
||||
self.assertEqual(len(self._list_libraries({'org': 'org-test', 'text_search': 'library-title-4'})), 1)
|
||||
|
||||
# General Content Library XBlock tests:
|
||||
|
||||
def test_library_blocks(self):
|
||||
@@ -218,6 +243,56 @@ 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
|
||||
"""
|
||||
with override_settings(FEATURES={**settings.FEATURES, 'ENABLE_CONTENT_LIBRARY_INDEX': is_indexing_enabled}):
|
||||
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
|
||||
"""
|
||||
with override_settings(FEATURES={**settings.FEATURES, 'ENABLE_CONTENT_LIBRARY_INDEX': is_indexing_enabled}):
|
||||
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")
|
||||
self._add_block_to_library(lib["id"], "problem", "foo-baz")
|
||||
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
|
||||
@@ -278,6 +353,7 @@ class ContentLibrariesTest(ContentLibrariesRestApiTest):
|
||||
author_group_member = UserFactory.create(username="GroupMember", email="groupmember@example.com")
|
||||
author_group_member.groups.add(group)
|
||||
random_user = UserFactory.create(username="Random", email="random@example.com")
|
||||
never_added = UserFactory.create(username="Never", email="never@example.com")
|
||||
|
||||
# Library CRUD #########################################################
|
||||
|
||||
@@ -292,22 +368,32 @@ class ContentLibrariesTest(ContentLibrariesRestApiTest):
|
||||
# By default, the creator of a new library is the only admin
|
||||
data = self._get_library_team(lib_id)
|
||||
self.assertEqual(len(data), 1)
|
||||
self.assertDictContainsEntries(data[0], {"user_id": admin.pk, "group_name": None, "access_level": "admin"})
|
||||
self.assertDictContainsEntries(data[0], {
|
||||
"username": admin.username, "group_name": None, "access_level": "admin",
|
||||
})
|
||||
|
||||
# Add the other users to the content library:
|
||||
self._set_user_access_level(lib_id, author.pk, access_level="author")
|
||||
self._set_user_access_level(lib_id, reader.pk, access_level="read")
|
||||
self._set_user_access_level(lib_id, author.username, access_level="author")
|
||||
# Delete it, add it again.
|
||||
self._remove_user_access(lib_id, author.username)
|
||||
self._set_user_access_level(lib_id, author.username, access_level="author")
|
||||
# Add one of them via the email-based creation endpoint.
|
||||
self._add_user_by_email(lib_id, reader.email, access_level="read")
|
||||
self._set_group_access_level(lib_id, group.name, access_level="author")
|
||||
|
||||
team_response = self._get_library_team(lib_id)
|
||||
self.assertEqual(len(team_response), 4)
|
||||
# We'll use this one later.
|
||||
reader_grant = {"username": reader.username, "group_name": None, "access_level": "read"}
|
||||
# The response should also always be sorted in a specific order (by username and group name):
|
||||
expected_response = [
|
||||
{"user_id": None, "group_name": "group1", "access_level": "author"},
|
||||
{"user_id": admin.pk, "group_name": None, "access_level": "admin"},
|
||||
{"user_id": author.pk, "group_name": None, "access_level": "author"},
|
||||
{"user_id": reader.pk, "group_name": None, "access_level": "read"},
|
||||
{"username": None, "group_name": "group1", "access_level": "author"},
|
||||
{"username": admin.username, "group_name": None, "access_level": "admin"},
|
||||
{"username": author.username, "group_name": None, "access_level": "author"},
|
||||
reader_grant,
|
||||
]
|
||||
from pprint import pprint
|
||||
pprint(team_response)
|
||||
for entry, expected in zip(team_response, expected_response):
|
||||
self.assertDictContainsEntries(entry, expected)
|
||||
|
||||
@@ -315,6 +401,7 @@ class ContentLibrariesTest(ContentLibrariesRestApiTest):
|
||||
with self.as_user(random_user):
|
||||
self._get_library(lib_id, expect_response=403)
|
||||
self._get_library_team(lib_id, expect_response=403)
|
||||
self._add_user_by_email(lib_id, never_added.email, access_level="read", expect_response=403)
|
||||
|
||||
# But every authorized user can:
|
||||
for user in [admin, author, author_group_member]:
|
||||
@@ -322,19 +409,25 @@ class ContentLibrariesTest(ContentLibrariesRestApiTest):
|
||||
self._get_library(lib_id)
|
||||
data = self._get_library_team(lib_id)
|
||||
self.assertEqual(data, team_response)
|
||||
data = self._get_user_access_level(lib_id, reader.username)
|
||||
self.assertEqual(data, {**reader_grant, 'username': 'Reader', 'email': 'reader@example.com'})
|
||||
|
||||
# A user with only read permission can get data about the library but not the team:
|
||||
with self.as_user(reader):
|
||||
self._get_library(lib_id)
|
||||
self._get_library_team(lib_id, expect_response=403)
|
||||
self._get_user_access_level(lib_id, author.username, expect_response=403)
|
||||
self._add_user_by_email(lib_id, never_added.email, access_level="read", expect_response=403)
|
||||
|
||||
# Users without admin access cannot delete the library nor change its team:
|
||||
for user in [author, reader, author_group_member, random_user]:
|
||||
with self.as_user(user):
|
||||
self._delete_library(lib_id, expect_response=403)
|
||||
self._set_user_access_level(lib_id, author.pk, access_level="admin", expect_response=403)
|
||||
self._set_user_access_level(lib_id, admin.pk, access_level=None, expect_response=403)
|
||||
self._set_user_access_level(lib_id, random_user.pk, access_level="read", expect_response=403)
|
||||
self._set_user_access_level(lib_id, author.username, access_level="admin", expect_response=403)
|
||||
self._set_user_access_level(lib_id, admin.username, access_level=None, expect_response=403)
|
||||
self._set_user_access_level(lib_id, random_user.username, access_level="read", expect_response=403)
|
||||
self._remove_user_access(lib_id, admin.username, expect_response=403)
|
||||
self._add_user_by_email(lib_id, never_added.email, access_level="read", expect_response=403)
|
||||
|
||||
# Users with author access (or higher) can edit the library's properties:
|
||||
with self.as_user(author):
|
||||
@@ -403,6 +496,22 @@ class ContentLibrariesTest(ContentLibrariesRestApiTest):
|
||||
self._commit_library_changes(lib_id)
|
||||
self._revert_library_changes(lib_id) # This is a no-op after the commit, but should still have 200 response
|
||||
|
||||
def test_no_lockout(self):
|
||||
"""
|
||||
Test that administrators cannot be removed if they are the only administrator granted access.
|
||||
"""
|
||||
admin = UserFactory.create(username="Admin", email="admin@example.com")
|
||||
successor = UserFactory.create(username="Successor", email="successor@example.com")
|
||||
with self.as_user(admin):
|
||||
lib = self._create_library(slug="permtest", title="Permission Test Library", description="Testing")
|
||||
# Fail to downgrade permissions.
|
||||
self._remove_user_access(lib_key=lib['id'], username=admin.username, expect_response=400)
|
||||
# Promote another user.
|
||||
self._set_user_access_level(
|
||||
lib_key=lib['id'], username=successor.username, access_level="admin",
|
||||
)
|
||||
self._remove_user_access(lib_key=lib['id'], username=admin.username)
|
||||
|
||||
def test_library_blocks_with_links(self):
|
||||
"""
|
||||
Test that libraries can link to XBlocks in other content libraries
|
||||
|
||||
@@ -6,23 +6,25 @@ from django.conf import settings
|
||||
from django.core.management import call_command
|
||||
from django.test.utils import override_settings
|
||||
from mock import patch
|
||||
from opaque_keys.edx.locator import LibraryLocatorV2
|
||||
from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2
|
||||
from search.search_engine_base import SearchEngine
|
||||
|
||||
from openedx.core.djangoapps.content_libraries.libraries_index import ContentLibraryIndexer, LibraryNotIndexedException
|
||||
from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrariesRestApiTest
|
||||
from openedx.core.djangoapps.content_libraries.libraries_index import ContentLibraryIndexer, LibraryBlockIndexer
|
||||
from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrariesRestApiTest, elasticsearch_test
|
||||
|
||||
|
||||
@override_settings(FEATURES={**settings.FEATURES, 'ENABLE_CONTENT_LIBRARY_INDEX': True})
|
||||
@override_settings(SEARCH_ENGINE="search.tests.mock_search_engine.MockSearchEngine")
|
||||
class ContentLibraryIndexerIndexer(ContentLibrariesRestApiTest):
|
||||
@elasticsearch_test
|
||||
class ContentLibraryIndexerTest(ContentLibrariesRestApiTest):
|
||||
"""
|
||||
Tests the operation of ContentLibraryIndexer
|
||||
"""
|
||||
|
||||
@elasticsearch_test
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
ContentLibraryIndexer.remove_all_libraries()
|
||||
ContentLibraryIndexer.remove_all_items()
|
||||
LibraryBlockIndexer.remove_all_items()
|
||||
self.searcher = SearchEngine.get_search_engine(ContentLibraryIndexer.INDEX_NAME)
|
||||
|
||||
def test_index_libraries(self):
|
||||
@@ -32,12 +34,9 @@ class ContentLibraryIndexerIndexer(ContentLibrariesRestApiTest):
|
||||
result1 = self._create_library(slug="test-lib-index-1", title="Title 1", description="Description")
|
||||
result2 = self._create_library(slug="test-lib-index-2", title="Title 2", description="Description")
|
||||
|
||||
response = self.searcher.search(doc_type=ContentLibraryIndexer.LIBRARY_DOCUMENT_TYPE, filter_dictionary={})
|
||||
self.assertEqual(response['total'], 2)
|
||||
|
||||
for result in [result1, result2]:
|
||||
library_key = LibraryLocatorV2.from_string(result['id'])
|
||||
response = ContentLibraryIndexer.get_libraries([library_key])[0]
|
||||
response = ContentLibraryIndexer.get_items([library_key])[0]
|
||||
|
||||
self.assertEqual(response['id'], result['id'])
|
||||
self.assertEqual(response['title'], result['title'])
|
||||
@@ -53,33 +52,33 @@ class ContentLibraryIndexerIndexer(ContentLibrariesRestApiTest):
|
||||
"""
|
||||
Test that outdated indexes aren't retrieved
|
||||
"""
|
||||
result = self._create_library(slug="test-lib-schemaupdates-1", title="Title 1", description="Description")
|
||||
library_key = LibraryLocatorV2.from_string(result['id'])
|
||||
|
||||
ContentLibraryIndexer.get_libraries([library_key])
|
||||
with patch("openedx.core.djangoapps.content_libraries.libraries_index.ContentLibraryIndexer.SCHEMA_VERSION",
|
||||
new=0):
|
||||
result = self._create_library(slug="test-lib-schemaupdates-1", title="Title 1", description="Description")
|
||||
library_key = LibraryLocatorV2.from_string(result['id'])
|
||||
self.assertEqual(len(ContentLibraryIndexer.get_items([library_key])), 1)
|
||||
|
||||
with patch("openedx.core.djangoapps.content_libraries.libraries_index.ContentLibraryIndexer.SCHEMA_VERSION",
|
||||
new=1):
|
||||
with self.assertRaises(LibraryNotIndexedException):
|
||||
ContentLibraryIndexer.get_libraries([library_key])
|
||||
self.assertEqual(len(ContentLibraryIndexer.get_items([library_key])), 0)
|
||||
|
||||
call_command("reindex_content_library", all=True, quiet=True)
|
||||
call_command("reindex_content_library", all=True, force=True)
|
||||
|
||||
ContentLibraryIndexer.get_libraries([library_key])
|
||||
self.assertEqual(len(ContentLibraryIndexer.get_items([library_key])), 1)
|
||||
|
||||
def test_remove_all_libraries(self):
|
||||
"""
|
||||
Test if remove_all_libraries() deletes all libraries
|
||||
Test if remove_all_items() deletes all libraries
|
||||
"""
|
||||
self._create_library(slug="test-lib-rm-all-1", title="Title 1", description="Description")
|
||||
self._create_library(slug="test-lib-rm-all-2", title="Title 2", description="Description")
|
||||
lib1 = self._create_library(slug="test-lib-rm-all-1", title="Title 1", description="Description")
|
||||
lib2 = self._create_library(slug="test-lib-rm-all-2", title="Title 2", description="Description")
|
||||
library_key1 = LibraryLocatorV2.from_string(lib1['id'])
|
||||
library_key2 = LibraryLocatorV2.from_string(lib2['id'])
|
||||
|
||||
response = self.searcher.search(doc_type=ContentLibraryIndexer.LIBRARY_DOCUMENT_TYPE, filter_dictionary={})
|
||||
self.assertEqual(response['total'], 2)
|
||||
self.assertEqual(len(ContentLibraryIndexer.get_items([library_key1, library_key2])), 2)
|
||||
|
||||
ContentLibraryIndexer.remove_all_libraries()
|
||||
response = self.searcher.search(doc_type=ContentLibraryIndexer.LIBRARY_DOCUMENT_TYPE, filter_dictionary={})
|
||||
self.assertEqual(response['total'], 0)
|
||||
ContentLibraryIndexer.remove_all_items()
|
||||
self.assertEqual(len(ContentLibraryIndexer.get_items()), 0)
|
||||
|
||||
def test_update_libraries(self):
|
||||
"""
|
||||
@@ -90,7 +89,7 @@ class ContentLibraryIndexerIndexer(ContentLibrariesRestApiTest):
|
||||
|
||||
self._update_library(lib['id'], title="New Title", description="New Title")
|
||||
|
||||
response = ContentLibraryIndexer.get_libraries([library_key])[0]
|
||||
response = ContentLibraryIndexer.get_items([library_key])[0]
|
||||
|
||||
self.assertEqual(response['id'], lib['id'])
|
||||
self.assertEqual(response['title'], "New Title")
|
||||
@@ -103,8 +102,8 @@ class ContentLibraryIndexerIndexer(ContentLibrariesRestApiTest):
|
||||
self.assertEqual(response['has_unpublished_deletes'], False)
|
||||
|
||||
self._delete_library(lib['id'])
|
||||
with self.assertRaises(LibraryNotIndexedException):
|
||||
ContentLibraryIndexer.get_libraries([library_key])
|
||||
self.assertEqual(ContentLibraryIndexer.get_items([library_key]), [])
|
||||
ContentLibraryIndexer.get_items([library_key])
|
||||
|
||||
def test_update_library_blocks(self):
|
||||
"""
|
||||
@@ -114,9 +113,9 @@ class ContentLibraryIndexerIndexer(ContentLibrariesRestApiTest):
|
||||
"""
|
||||
Commit library changes, and verify that there are no uncommited changes anymore
|
||||
"""
|
||||
last_published = ContentLibraryIndexer.get_libraries([library_key])[0]['last_published']
|
||||
last_published = ContentLibraryIndexer.get_items([library_key])[0]['last_published']
|
||||
self._commit_library_changes(str(library_key))
|
||||
response = ContentLibraryIndexer.get_libraries([library_key])[0]
|
||||
response = ContentLibraryIndexer.get_items([library_key])[0]
|
||||
self.assertEqual(response['has_unpublished_changes'], False)
|
||||
self.assertEqual(response['has_unpublished_deletes'], False)
|
||||
self.assertGreaterEqual(response['last_published'], last_published)
|
||||
@@ -126,7 +125,7 @@ class ContentLibraryIndexerIndexer(ContentLibrariesRestApiTest):
|
||||
"""
|
||||
Verify uncommitted changes and deletes in the index
|
||||
"""
|
||||
response = ContentLibraryIndexer.get_libraries([library_key])[0]
|
||||
response = ContentLibraryIndexer.get_items([library_key])[0]
|
||||
self.assertEqual(response['has_unpublished_changes'], has_unpublished_changes)
|
||||
self.assertEqual(response['has_unpublished_deletes'], has_unpublished_deletes)
|
||||
return response
|
||||
@@ -178,3 +177,105 @@ class ContentLibraryIndexerIndexer(ContentLibrariesRestApiTest):
|
||||
#Verify reverting uncommitted changes
|
||||
self._revert_library_changes(lib["id"])
|
||||
verify_uncommitted_libraries(library_key, False, False)
|
||||
|
||||
|
||||
@override_settings(FEATURES={**settings.FEATURES, 'ENABLE_CONTENT_LIBRARY_INDEX': True})
|
||||
@elasticsearch_test
|
||||
class LibraryBlockIndexerTest(ContentLibrariesRestApiTest):
|
||||
"""
|
||||
Tests the operation of LibraryBlockIndexer
|
||||
"""
|
||||
|
||||
@elasticsearch_test
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
ContentLibraryIndexer.remove_all_items()
|
||||
LibraryBlockIndexer.remove_all_items()
|
||||
self.searcher = SearchEngine.get_search_engine(LibraryBlockIndexer.INDEX_NAME)
|
||||
|
||||
def test_index_block(self):
|
||||
"""
|
||||
Test if libraries are being indexed correctly
|
||||
"""
|
||||
lib = self._create_library(slug="test-lib-index-1", title="Title 1", description="Description")
|
||||
block1 = self._add_block_to_library(lib['id'], "problem", "problem1")
|
||||
block2 = self._add_block_to_library(lib['id'], "problem", "problem2")
|
||||
|
||||
self.assertEqual(len(LibraryBlockIndexer.get_items()), 2)
|
||||
|
||||
for block in [block1, block2]:
|
||||
usage_key = LibraryUsageLocatorV2.from_string(block['id'])
|
||||
response = LibraryBlockIndexer.get_items([usage_key])[0]
|
||||
|
||||
self.assertEqual(response['id'], block['id'])
|
||||
self.assertEqual(response['def_key'], block['def_key'])
|
||||
self.assertEqual(response['block_type'], block['block_type'])
|
||||
self.assertEqual(response['display_name'], block['display_name'])
|
||||
self.assertEqual(response['has_unpublished_changes'], block['has_unpublished_changes'])
|
||||
|
||||
def test_schema_updates(self):
|
||||
"""
|
||||
Test that outdated indexes aren't retrieved
|
||||
"""
|
||||
lib = self._create_library(slug="test-lib--block-schemaupdates-1", title="Title 1", description="Description")
|
||||
with patch("openedx.core.djangoapps.content_libraries.libraries_index.LibraryBlockIndexer.SCHEMA_VERSION",
|
||||
new=0):
|
||||
block = self._add_block_to_library(lib['id'], "problem", "problem1")
|
||||
self.assertEqual(len(LibraryBlockIndexer.get_items([block['id']])), 1)
|
||||
|
||||
with patch("openedx.core.djangoapps.content_libraries.libraries_index.LibraryBlockIndexer.SCHEMA_VERSION",
|
||||
new=1):
|
||||
self.assertEqual(len(LibraryBlockIndexer.get_items([block['id']])), 0)
|
||||
|
||||
call_command("reindex_content_library", all=True, force=True)
|
||||
|
||||
self.assertEqual(len(LibraryBlockIndexer.get_items([block['id']])), 1)
|
||||
|
||||
def test_remove_all_items(self):
|
||||
"""
|
||||
Test if remove_all_items() deletes all libraries
|
||||
"""
|
||||
lib1 = self._create_library(slug="test-lib-rm-all", title="Title 1", description="Description")
|
||||
self._add_block_to_library(lib1['id'], "problem", "problem1")
|
||||
self._add_block_to_library(lib1['id'], "problem", "problem2")
|
||||
self.assertEqual(len(LibraryBlockIndexer.get_items()), 2)
|
||||
|
||||
LibraryBlockIndexer.remove_all_items()
|
||||
self.assertEqual(len(LibraryBlockIndexer.get_items()), 0)
|
||||
|
||||
def test_crud_block(self):
|
||||
"""
|
||||
Test that CRUD operations on blocks are reflected in the index
|
||||
"""
|
||||
lib = self._create_library(slug="test-lib-crud-block", title="Title", description="Description")
|
||||
block = self._add_block_to_library(lib['id'], "problem", "problem1")
|
||||
|
||||
# Update OLX, verify updates in index
|
||||
self._set_library_block_olx(block["id"], '<problem display_name="new_name"/>')
|
||||
response = LibraryBlockIndexer.get_items([block['id']])[0]
|
||||
self.assertEqual(response['display_name'], "new_name")
|
||||
self.assertEqual(response['has_unpublished_changes'], True)
|
||||
|
||||
# Verify has_unpublished_changes after committing library
|
||||
self._commit_library_changes(lib['id'])
|
||||
response = LibraryBlockIndexer.get_items([block['id']])[0]
|
||||
self.assertEqual(response['has_unpublished_changes'], False)
|
||||
|
||||
# Verify has_unpublished_changes after reverting library
|
||||
self._set_library_block_asset(block["id"], "whatever.png", b"data")
|
||||
response = LibraryBlockIndexer.get_items([block['id']])[0]
|
||||
self.assertEqual(response['has_unpublished_changes'], True)
|
||||
|
||||
self._revert_library_changes(lib['id'])
|
||||
response = LibraryBlockIndexer.get_items([block['id']])[0]
|
||||
self.assertEqual(response['has_unpublished_changes'], False)
|
||||
|
||||
# Verify that deleting block removes it from index
|
||||
self._delete_library_block(block['id'])
|
||||
self.assertEqual(LibraryBlockIndexer.get_items([block['id']]), [])
|
||||
|
||||
# Verify that deleting a library removes its blocks from index too
|
||||
self._add_block_to_library(lib['id'], "problem", "problem1")
|
||||
LibraryBlockIndexer.get_items([block['id']])
|
||||
self._delete_library(lib['id'])
|
||||
self.assertEqual(LibraryBlockIndexer.get_items([block['id']]), [])
|
||||
|
||||
@@ -31,7 +31,7 @@ urlpatterns = [
|
||||
# Get the list of users/groups who have permission to view/edit/administer this library:
|
||||
url(r'^team/$', views.LibraryTeamView.as_view()),
|
||||
# Add/Edit (PUT) or remove (DELETE) a user's permission to use this library
|
||||
url(r'^team/user/(?P<user_id>\d+)/$', views.LibraryTeamUserView.as_view()),
|
||||
url(r'^team/user/(?P<username>[^/]+)/$', views.LibraryTeamUserView.as_view()),
|
||||
# Add/Edit (PUT) or remove (DELETE) a group's permission to use this library
|
||||
url(r'^team/group/(?P<group_name>[^/]+)/$', views.LibraryTeamGroupView.as_view()),
|
||||
])),
|
||||
|
||||
@@ -7,6 +7,7 @@ import logging
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.contrib.auth.models import Group
|
||||
from django.shortcuts import get_object_or_404
|
||||
from django.utils.translation import ugettext as _
|
||||
import edx_api_doc_tools as apidocs
|
||||
from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2
|
||||
from organizations.models import Organization
|
||||
@@ -31,6 +32,7 @@ from openedx.core.djangoapps.content_libraries.serializers import (
|
||||
LibraryXBlockOlxSerializer,
|
||||
LibraryXBlockStaticFileSerializer,
|
||||
LibraryXBlockStaticFilesSerializer,
|
||||
ContentLibraryAddPermissionByEmailSerializer,
|
||||
)
|
||||
from openedx.core.lib.api.view_utils import view_auth_classes
|
||||
|
||||
@@ -66,13 +68,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,20 +102,16 @@ class LibraryRootView(APIView):
|
||||
|
||||
@apidocs.schema(
|
||||
parameters=[
|
||||
*LibraryApiPagination.apidoc_params,
|
||||
apidocs.query_parameter(
|
||||
'pagination',
|
||||
bool,
|
||||
description="Enables paginated schema",
|
||||
'org',
|
||||
str,
|
||||
description="The organization short-name used to filter libraries",
|
||||
),
|
||||
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",
|
||||
'text_search',
|
||||
str,
|
||||
description="The string used to filter libraries by searching in title, id, org, or description",
|
||||
),
|
||||
],
|
||||
)
|
||||
@@ -103,10 +119,19 @@ class LibraryRootView(APIView):
|
||||
"""
|
||||
Return a list of all content libraries that the user has permission to view.
|
||||
"""
|
||||
paginator = LibraryRootPagination()
|
||||
queryset = api.get_libraries_for_user(request.user)
|
||||
paginated_qs = paginator.paginate_queryset(queryset, request)
|
||||
result = api.get_metadata_from_index(paginated_qs)
|
||||
org = request.query_params.get('org', None)
|
||||
text_search = request.query_params.get('text_search', None)
|
||||
|
||||
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)
|
||||
result = paginator.paginate_queryset(result, request)
|
||||
else:
|
||||
# We can paginate queryset early and prevent fetching unneeded metadata
|
||||
paginated_qs = paginator.paginate_queryset(queryset, request)
|
||||
result = api.get_metadata_from_index(paginated_qs)
|
||||
|
||||
serializer = ContentLibraryMetadataSerializer(result, many=True)
|
||||
# Verify `pagination` param to maintain compatibility with older
|
||||
# non pagination-aware clients
|
||||
@@ -188,6 +213,33 @@ class LibraryTeamView(APIView):
|
||||
Note also the 'allow_public_' settings which can be edited by PATCHing the
|
||||
library itself (LibraryDetailsView.patch).
|
||||
"""
|
||||
@convert_exceptions
|
||||
def post(self, request, lib_key_str):
|
||||
"""
|
||||
Add a user to this content library via email, with permissions specified in the
|
||||
request body.
|
||||
"""
|
||||
key = LibraryLocatorV2.from_string(lib_key_str)
|
||||
api.require_permission_for_library_key(key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY_TEAM)
|
||||
serializer = ContentLibraryAddPermissionByEmailSerializer(data=request.data)
|
||||
serializer.is_valid(raise_exception=True)
|
||||
try:
|
||||
user = User.objects.get(email=serializer.validated_data.get('email'))
|
||||
except User.DoesNotExist:
|
||||
raise ValidationError({'email': _('We could not find a user with that email address.')})
|
||||
grant = api.get_library_user_permissions(key, user)
|
||||
if grant:
|
||||
return Response(
|
||||
{'email': [_('This user already has access to this library.')]},
|
||||
status=status.HTTP_400_BAD_REQUEST,
|
||||
)
|
||||
try:
|
||||
api.set_library_user_permissions(key, user, access_level=serializer.validated_data["access_level"])
|
||||
except api.LibraryPermissionIntegrityError as err:
|
||||
raise ValidationError(detail=str(err))
|
||||
grant = api.get_library_user_permissions(key, user)
|
||||
return Response(ContentLibraryPermissionSerializer(grant).data)
|
||||
|
||||
@convert_exceptions
|
||||
def get(self, request, lib_key_str):
|
||||
"""
|
||||
@@ -207,7 +259,7 @@ class LibraryTeamUserView(APIView):
|
||||
library.
|
||||
"""
|
||||
@convert_exceptions
|
||||
def put(self, request, lib_key_str, user_id):
|
||||
def put(self, request, lib_key_str, username):
|
||||
"""
|
||||
Add a user to this content library, with permissions specified in the
|
||||
request body.
|
||||
@@ -216,20 +268,40 @@ class LibraryTeamUserView(APIView):
|
||||
api.require_permission_for_library_key(key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY_TEAM)
|
||||
serializer = ContentLibraryPermissionLevelSerializer(data=request.data)
|
||||
serializer.is_valid(raise_exception=True)
|
||||
user = get_object_or_404(User, pk=int(user_id))
|
||||
api.set_library_user_permissions(key, user, access_level=serializer.validated_data["access_level"])
|
||||
return Response({})
|
||||
user = get_object_or_404(User, username=username)
|
||||
try:
|
||||
api.set_library_user_permissions(key, user, access_level=serializer.validated_data["access_level"])
|
||||
except api.LibraryPermissionIntegrityError as err:
|
||||
raise ValidationError(detail=str(err))
|
||||
grant = api.get_library_user_permissions(key, user)
|
||||
return Response(ContentLibraryPermissionSerializer(grant).data)
|
||||
|
||||
@convert_exceptions
|
||||
def delete(self, request, lib_key_str, user_id):
|
||||
def get(self, request, lib_key_str, username):
|
||||
"""
|
||||
Gets the current permissions settings for a particular user.
|
||||
"""
|
||||
key = LibraryLocatorV2.from_string(lib_key_str)
|
||||
api.require_permission_for_library_key(key, request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY_TEAM)
|
||||
user = get_object_or_404(User, username=username)
|
||||
grant = api.get_library_user_permissions(key, user)
|
||||
if not grant:
|
||||
raise NotFound
|
||||
return Response(ContentLibraryPermissionSerializer(grant).data)
|
||||
|
||||
@convert_exceptions
|
||||
def delete(self, request, lib_key_str, username):
|
||||
"""
|
||||
Remove the specified user's permission to access or edit this content
|
||||
library.
|
||||
"""
|
||||
key = LibraryLocatorV2.from_string(lib_key_str)
|
||||
api.require_permission_for_library_key(key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY_TEAM)
|
||||
user = get_object_or_404(User, pk=int(user_id))
|
||||
api.set_library_user_permissions(key, user, access_level=None)
|
||||
user = get_object_or_404(User, username=username)
|
||||
try:
|
||||
api.set_library_user_permissions(key, user, access_level=None)
|
||||
except api.LibraryPermissionIntegrityError as err:
|
||||
raise ValidationError(detail=str(err))
|
||||
return Response({})
|
||||
|
||||
|
||||
@@ -253,14 +325,14 @@ class LibraryTeamGroupView(APIView):
|
||||
return Response({})
|
||||
|
||||
@convert_exceptions
|
||||
def delete(self, request, lib_key_str, user_id):
|
||||
def delete(self, request, lib_key_str, username):
|
||||
"""
|
||||
Remove the specified user's permission to access or edit this content
|
||||
library.
|
||||
"""
|
||||
key = LibraryLocatorV2.from_string(lib_key_str)
|
||||
api.require_permission_for_library_key(key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY_TEAM)
|
||||
group = get_object_or_404(Group, pk=int(user_id))
|
||||
group = get_object_or_404(Group, username=username)
|
||||
api.set_library_group_permissions(key, group, access_level=None)
|
||||
return Response({})
|
||||
|
||||
@@ -388,14 +460,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
|
||||
|
||||
@@ -414,9 +414,10 @@ class TestModuleStoreSerializer(TestDumpToNeo4jCommandBase):
|
||||
mock_graph = MockGraph()
|
||||
mock_graph_constructor.return_value = mock_graph
|
||||
mock_selector_class.return_value = MockNodeSelector(mock_graph)
|
||||
mock_credentials = mock.Mock()
|
||||
# mocking is thorwing error in kombu serialzier and its not require here any more.
|
||||
credentials = {}
|
||||
|
||||
submitted, skipped = self.mss.dump_courses_to_neo4j(mock_credentials)
|
||||
submitted, skipped = self.mss.dump_courses_to_neo4j(credentials)
|
||||
|
||||
self.assertCourseDump(
|
||||
mock_graph,
|
||||
@@ -441,9 +442,10 @@ class TestModuleStoreSerializer(TestDumpToNeo4jCommandBase):
|
||||
mock_graph = MockGraph(transaction_errors=True)
|
||||
mock_graph_constructor.return_value = mock_graph
|
||||
mock_selector_class.return_value = MockNodeSelector(mock_graph)
|
||||
mock_credentials = mock.Mock()
|
||||
# mocking is thorwing error in kombu serialzier and its not require here any more.
|
||||
credentials = {}
|
||||
|
||||
submitted, skipped = self.mss.dump_courses_to_neo4j(mock_credentials)
|
||||
submitted, skipped = self.mss.dump_courses_to_neo4j(credentials)
|
||||
|
||||
self.assertCourseDump(
|
||||
mock_graph,
|
||||
@@ -472,17 +474,18 @@ class TestModuleStoreSerializer(TestDumpToNeo4jCommandBase):
|
||||
mock_graph = MockGraph()
|
||||
mock_graph_constructor.return_value = mock_graph
|
||||
mock_selector_class.return_value = MockNodeSelector(mock_graph)
|
||||
mock_credentials = mock.Mock()
|
||||
# mocking is thorwing error in kombu serialzier and its not require here any more.
|
||||
credentials = {}
|
||||
|
||||
# run once to warm the cache
|
||||
self.mss.dump_courses_to_neo4j(
|
||||
mock_credentials, override_cache=override_cache
|
||||
credentials, override_cache=override_cache
|
||||
)
|
||||
|
||||
# when run the second time, only dump courses if the cache override
|
||||
# is enabled
|
||||
submitted, __ = self.mss.dump_courses_to_neo4j(
|
||||
mock_credentials, override_cache=override_cache
|
||||
credentials, override_cache=override_cache
|
||||
)
|
||||
self.assertEqual(len(submitted), expected_number_courses)
|
||||
|
||||
@@ -496,10 +499,11 @@ class TestModuleStoreSerializer(TestDumpToNeo4jCommandBase):
|
||||
mock_graph = MockGraph()
|
||||
mock_graph_constructor.return_value = mock_graph
|
||||
mock_selector_class.return_value = MockNodeSelector(mock_graph)
|
||||
mock_credentials = mock.Mock()
|
||||
# mocking is thorwing error in kombu serialzier and its not require here any more.
|
||||
credentials = {}
|
||||
|
||||
# run once to warm the cache
|
||||
submitted, skipped = self.mss.dump_courses_to_neo4j(mock_credentials)
|
||||
submitted, skipped = self.mss.dump_courses_to_neo4j(credentials)
|
||||
self.assertEqual(len(submitted), len(self.course_strings))
|
||||
|
||||
# simulate one of the courses being published
|
||||
@@ -507,7 +511,7 @@ class TestModuleStoreSerializer(TestDumpToNeo4jCommandBase):
|
||||
update_block_structure_on_course_publish(None, self.course.id)
|
||||
|
||||
# make sure only the published course was dumped
|
||||
submitted, __ = self.mss.dump_courses_to_neo4j(mock_credentials)
|
||||
submitted, __ = self.mss.dump_courses_to_neo4j(credentials)
|
||||
self.assertEqual(len(submitted), 1)
|
||||
self.assertEqual(submitted[0], six.text_type(self.course.id))
|
||||
|
||||
|
||||
@@ -20,7 +20,7 @@ from rest_framework.response import Response
|
||||
from rest_framework.views import APIView
|
||||
|
||||
from course_modes.models import CourseMode
|
||||
from edxnotes.helpers import is_feature_enabled
|
||||
from lms.djangoapps.edxnotes.helpers import is_feature_enabled
|
||||
from lms.djangoapps.course_api.api import course_detail
|
||||
from lms.djangoapps.courseware.access import has_access
|
||||
from lms.djangoapps.courseware.access_response import (
|
||||
|
||||
@@ -33,5 +33,7 @@ COURSE_GRADE_NOW_FAILED = Signal(
|
||||
]
|
||||
)
|
||||
|
||||
# Signal that indicates that a user has become verified
|
||||
# Signal that indicates that a user has become verified for certificate purposes
|
||||
LEARNER_NOW_VERIFIED = Signal(providing_args=['user'])
|
||||
|
||||
USER_ACCOUNT_ACTIVATED = Signal(providing_args=["user"]) # Signal indicating email verification
|
||||
|
||||
@@ -111,22 +111,18 @@ class Command(BaseCommand):
|
||||
if os.path.exists(file_path):
|
||||
raise CommandError("File already exists at '{path}'".format(path=file_path))
|
||||
|
||||
# Retrieve all the courses for the org.
|
||||
# If we were given a specific list of courses to include,
|
||||
# filter out anything not in that list.
|
||||
courses = self._get_courses_for_org(org_list)
|
||||
only_courses = options.get("courses")
|
||||
|
||||
if only_courses is not None:
|
||||
only_courses = [
|
||||
CourseKey.from_string(course_key.strip())
|
||||
for course_key in only_courses.split(",")
|
||||
]
|
||||
courses = list(set(courses) & set(only_courses))
|
||||
if only_courses is None:
|
||||
# Retrieve all the courses for the org.
|
||||
# If we were given a specific list of courses to include,
|
||||
# filter out anything not in that list.
|
||||
courses = self._get_courses_for_org(org_list)
|
||||
|
||||
# Add in organizations from the course keys, to ensure
|
||||
# we're including orgs with different capitalizations
|
||||
org_list = list(set(org_list) | set(course.org for course in courses))
|
||||
# Add in organizations from the course keys, to ensure we're including orgs with different capitalizations
|
||||
org_list = list(set(org_list) | set(course.org for course in courses))
|
||||
else:
|
||||
courses = list(set(only_courses.split(",")))
|
||||
|
||||
# If no courses are found, abort
|
||||
if not courses:
|
||||
@@ -270,7 +266,7 @@ class Command(BaseCommand):
|
||||
row_count += 1
|
||||
|
||||
# Log the number of rows we processed
|
||||
LOGGER.info("Retrieved {num_rows} records.".format(num_rows=row_count))
|
||||
LOGGER.info("Retrieved {num_rows} records for orgs {org}.".format(num_rows=row_count, org=org_aliases))
|
||||
|
||||
def _iterate_results(self, cursor):
|
||||
"""
|
||||
|
||||
@@ -382,26 +382,6 @@ class LoginTest(SiteMixin, CacheIsolationTestCase):
|
||||
response, _audit_log = self._login_response(self.user_email, 'wrong_password')
|
||||
self._assert_response(response, success=False, value='Too many failed login attempts')
|
||||
|
||||
def test_login_ratelimited(self):
|
||||
"""
|
||||
Test that login endpoint is IP ratelimited and only allow 5 requests
|
||||
per 5 minutes per IP.
|
||||
"""
|
||||
for i in range(5):
|
||||
password = u'test_password{0}'.format(i)
|
||||
response, _audit_log = self._login_response(self.user_email, password)
|
||||
self._assert_response(response, success=False)
|
||||
|
||||
response, _audit_log = self._login_response(self.user_email, self.password)
|
||||
self.assertEqual(response.status_code, 403)
|
||||
|
||||
# now reset the time to 6 min from now in future and verify that it will
|
||||
# allow another request from same IP and user can successfully login
|
||||
reset_time = datetime.datetime.utcnow() + datetime.timedelta(seconds=361)
|
||||
with freeze_time(reset_time):
|
||||
response, _audit_log = self._login_response(self.user_email, self.password)
|
||||
self._assert_response(response, success=True)
|
||||
|
||||
@patch.dict("django.conf.settings.FEATURES", {"DISABLE_SET_JWT_COOKIES_FOR_TESTS": False})
|
||||
def test_login_refresh(self):
|
||||
def _assert_jwt_cookie_present(response):
|
||||
|
||||
@@ -191,14 +191,14 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
|
||||
self.request_password_reset(200)
|
||||
# now reset the time to 1 min from now in future and change the email and
|
||||
# verify that it will allow another request from same IP
|
||||
for status in [200, 403]:
|
||||
reset_time = datetime.now(UTC) + timedelta(seconds=61)
|
||||
with freeze_time(reset_time):
|
||||
reset_time = datetime.now(UTC) + timedelta(seconds=61)
|
||||
with freeze_time(reset_time):
|
||||
for status in [200, 403]:
|
||||
self.request_password_reset(status)
|
||||
|
||||
# Even changing the IP will not allow more than two requests for same email.
|
||||
new_ip = "8.8.8.8"
|
||||
self.request_password_reset(403, new_ip=new_ip)
|
||||
# Even changing the IP will not allow more than two requests for same email.
|
||||
new_ip = "8.8.8.8"
|
||||
self.request_password_reset(403, new_ip=new_ip)
|
||||
|
||||
cache.clear()
|
||||
|
||||
|
||||
@@ -0,0 +1,35 @@
|
||||
Waffle Utils Extraction
|
||||
***********************
|
||||
|
||||
Status
|
||||
======
|
||||
|
||||
Accepted
|
||||
|
||||
Context
|
||||
=======
|
||||
|
||||
The waffle utilities in this app were created in edx-platform, but are generally useful across IDAs.
|
||||
|
||||
Decision
|
||||
========
|
||||
|
||||
These utilities will be be moved to `edx/edx-toggles`_ so that they can be used by other IDAs. Additionally, the shared library will use the module name ``toggles``, rather than ``waffle_utils``, so it can more generally include non-waffle based toggle utilities as well.
|
||||
|
||||
.. _edx/edx-toggles: https://github.com/edx/edx-toggles
|
||||
|
||||
Consequences
|
||||
============
|
||||
|
||||
* Rollout plan required to deprecate and update class references.
|
||||
* See ADR 0003-leave-course-waffle-flag for the decision to leave the CourseWaffleFlag behind.
|
||||
* See ADR 0004-waffle-util-namespacing for decision to change namespacing implementation before extraction.
|
||||
* The toggle state endpoint, which is meant to be a Django Plugin, could be extracted as a separate step. This requires some additional work:
|
||||
|
||||
* Finishing out work around Django Plugin capabilities in edx-django-utils.
|
||||
* Adding ability to document CourseWaffleFlag from edx-platform. Note: we may lose the ability to find course override data for toggles no longer in use, by looping through the entire model, unless we add a hook for this from the edx-toggles version.
|
||||
|
||||
* The helper `get_instance_module_name`_ should probably move to `edx_django_utils/monitoring/code_owner`_. It could be considered hacky, but is quite useful. It needs to work whether the class definition is in a library or an IDA, and whether the instance declaration is in a library or an IDA.
|
||||
|
||||
.. _get_instance_module_name: https://github.com/edx/edx-platform/blob/a8c3413a32510dc45301d0c462bf706a5f7ba487/openedx/core/djangoapps/waffle_utils/__init__.py#L521
|
||||
.. _edx_django_utils/monitoring/code_owner: https://github.com/edx/edx-django-utils/tree/master/edx_django_utils/monitoring/code_owner
|
||||
@@ -0,0 +1,31 @@
|
||||
Leave CourseWaffleFlag
|
||||
**********************
|
||||
|
||||
Status
|
||||
======
|
||||
|
||||
Accepted
|
||||
|
||||
Context
|
||||
=======
|
||||
|
||||
It was decided in 0002-waffle-utils-extraction to remove waffle_utils to the shared library edx-toggles. However, moving the class CourseWaffleFlag would be complicated due to its model data, and it is unclear how much usage it would get in other IDAs.
|
||||
|
||||
Decision
|
||||
========
|
||||
|
||||
It has been decided to leave CourseWaffleFlag inside edx-platform for the time being, and to delay its extraction until the work seems warranted (i.e. another IDA actually wants to make use of it).
|
||||
|
||||
Consequences
|
||||
============
|
||||
|
||||
* The toggle state endpoint will need to be updated to allow WaffleFlag subclasses to add state data.
|
||||
|
||||
Rejected Alternative
|
||||
====================
|
||||
|
||||
The alternative would be to extract CourseWaffleFlag to the shared library. As noted, this can be decided in the future. If extraction were to be pursued, some things to consider would be:
|
||||
|
||||
* Ensuring course override data is properly migrated from the old to new method of defining course overrides. It is possible that this migration would need to be documented and maintained for the Open edX release as well.
|
||||
* Given the data migration needed, it might be practical to migrate course override data from a ConfigurationModel to a Django Setting (using remote config) before extraction.
|
||||
* The toggle state report uses the term `course_id` for the course overrides, but this is actually a `course_run_id` in the context of other IDAs. Will its name be configurable per IDA?
|
||||
@@ -0,0 +1,38 @@
|
||||
Waffle Util Namespacing
|
||||
***********************
|
||||
|
||||
Status
|
||||
======
|
||||
|
||||
Accepted
|
||||
|
||||
Context
|
||||
=======
|
||||
|
||||
The toggle classes WaffleFlag and WaffleSwitch rely on several namespace classes (WaffleNamespace, WaffleSwitchNamespace, and WaffleFlagNamespace). In order to create a WaffleFlag (or WaffleSwitch), you must first create a Namespace object.
|
||||
|
||||
Other IDAs have active waffle flags and switches that don't use any namespacing, like this `example switch in ecommerce`_. Once WaffleFlag and WaffleSwitch are extracted to be used in other IDAs (see 0002-waffle-utils-extraction), the required namespace class will make this transition more difficult.
|
||||
|
||||
Additionally, the fully qualified waffle name, including the namespace, is required in code annotations and the django admin. Since it needs to be manually reconstructed by the developer, it has lead to copy/paste issues that are also difficult to lint.
|
||||
|
||||
Lastly, the namespace classes contain a lot of logic, but in effect, they only are used to ensure the flag name has a prefix like '<NAMESPACE_NAME>.<FLAG_NAME>'.
|
||||
|
||||
.. _example switch in ecommerce: https://github.com/edx/ecommerce/blob/e899c78325ac492d0a2b1ea0aab4d5e230262b8f/ecommerce/extensions/dashboard/users/views.py#L21
|
||||
|
||||
Decision
|
||||
========
|
||||
|
||||
Change the interface to WaffleFlag and WaffleSwitch to simply take the complete flag name, rather than a Namespace object.
|
||||
|
||||
The constructor can assert that the name includes a `.` to help remind people to use some form of prefixed namespace. However, an optional argument with a name like `skip_namespace_assertion=True` could be used to skip this assertion, enabling a simpler transition for existing flags and switches that don't meet this requirement.
|
||||
|
||||
Consequences
|
||||
============
|
||||
|
||||
This change will enable WaffleFlag, WaffleSwitch, and all subclasses to have a simpler interface. In addition to a simpler constructor, we will no longer need to differentiate between an instance's namespaced and non-namespaced name.
|
||||
|
||||
A possible rollout plan would be to introduce WaffleFlag and WaffleSwitch classes with the new interface when they are added into edx-toggles, and deprecate the old versions in edx-platform. This would enable us to reuse the same class names with a new import, for an iterative rollout.
|
||||
|
||||
Although it would be nice to update CourseWaffleFlag to have a similar interface for consistency, it is a lower priority if it is not moving to edx-toggles. See 0003-leave-course-waffle-flag.rst.
|
||||
|
||||
This change needs be documented for the next Open edX release.
|
||||
Reference in New Issue
Block a user