Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 20 additions & 8 deletions openedx/core/djangoapps/content/search/documents.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ class Fields:
published_content = "content"
published_num_children = "num_children"

# List of children keys
child_usage_keys = "child_usage_keys"

# Note: new fields or values can be added at any time, but if they need to be indexed for filtering or keyword
# search, the index configuration will need to be changed, which is only done as part of the 'reindex_studio'
# command (changing those settings on an large active index is not recommended).
Expand Down Expand Up @@ -656,16 +659,28 @@ def searchable_doc_for_container(
elif container.has_unpublished_changes:
publish_status = PublishStatus.modified

container_type = lib_api.ContainerType(container_key.container_type)

def get_child_keys(children):
match container_type:
case lib_api.ContainerType.Unit:
return [
str(child.usage_key)
for child in children
]
case lib_api.ContainerType.Subsection | lib_api.ContainerType.Section:
return [
str(child.container_key)
for child in children
]

doc.update({
Fields.display_name: container.display_name,
Fields.created: container.created.timestamp(),
Fields.modified: container.modified.timestamp(),
Fields.num_children: len(draft_children),
Fields.content: {
"child_usage_keys": [
str(child.usage_key)
for child in draft_children
],
Fields.child_usage_keys: get_child_keys(draft_children)
},
Fields.publish_status: publish_status,
Fields.last_published: container.last_published.timestamp() if container.last_published else None,
Expand All @@ -683,10 +698,7 @@ def searchable_doc_for_container(
Fields.published_display_name: container.published_display_name,
Fields.published_num_children: len(published_children),
Fields.published_content: {
"child_usage_keys": [
str(child.usage_key)
for child in published_children
],
Fields.child_usage_keys: get_child_keys(published_children),
},
}

Expand Down
158 changes: 146 additions & 12 deletions openedx/core/djangoapps/content/search/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class TestSearchApi(ModuleStoreTestCase):
MODULESTORE = TEST_DATA_SPLIT_MODULESTORE

def setUp(self):
# pylint: disable=too-many-statements
super().setUp()
self.user = UserFactory.create()
self.user_id = self.user.id
Expand Down Expand Up @@ -219,7 +220,7 @@ def setUp(self):
"breadcrumbs": [{"display_name": "Library"}],
}

# Create a unit:
# Create a container:
with freeze_time(self.created_date):
self.unit = library_api.create_container(
library_key=self.library.key,
Expand All @@ -229,6 +230,23 @@ def setUp(self):
user_id=None,
)
self.unit_key = "lct:org1:lib:unit:unit-1"
self.subsection = library_api.create_container(
self.library.key,
container_type=library_api.ContainerType.Subsection,
slug="subsection-1",
title="Subsection 1",
user_id=None,
)
self.subsection_key = "lct:org1:lib:subsection:subsection-1"
self.section = library_api.create_container(
self.library.key,
container_type=library_api.ContainerType.Section,
slug="section-1",
title="Section 1",
user_id=None,
)
self.section_key = "lct:org1:lib:section:section-1"

self.unit_dict = {
"id": "lctorg1libunitunit-1-e4527f7c",
"block_id": "unit-1",
Expand All @@ -249,6 +267,46 @@ def setUp(self):
"breadcrumbs": [{"display_name": "Library"}],
# "published" is not set since we haven't published it yet
}
self.subsection_dict = {
"id": "lctorg1libsubsectionsubsection-1-cf808309",
"block_id": "subsection-1",
"block_type": "subsection",
"usage_key": self.subsection_key,
"type": "library_container",
"display_name": "Subsection 1",
# description is not set for containers
"num_children": 0,
"content": {"child_usage_keys": []},
"publish_status": "never",
"context_key": "lib:org1:lib",
"org": "org1",
"created": self.created_date.timestamp(),
"modified": self.created_date.timestamp(),
"last_published": None,
"access_id": lib_access.id,
"breadcrumbs": [{"display_name": "Library"}],
# "published" is not set since we haven't published it yet
}
self.section_dict = {
"id": "lctorg1libsectionsection-1-dc4791a4",
"block_id": "section-1",
"block_type": "section",
"usage_key": self.section_key,
"type": "library_container",
"display_name": "Section 1",
# description is not set for containers
"num_children": 0,
"content": {"child_usage_keys": []},
"publish_status": "never",
"context_key": "lib:org1:lib",
"org": "org1",
"created": self.created_date.timestamp(),
"modified": self.created_date.timestamp(),
"last_published": None,
"access_id": lib_access.id,
"breadcrumbs": [{"display_name": "Library"}],
# "published" is not set since we haven't published it yet
}

@override_settings(MEILISEARCH_ENABLED=False)
def test_reindex_meilisearch_disabled(self, mock_meilisearch):
Expand Down Expand Up @@ -278,6 +336,12 @@ def test_reindex_meilisearch(self, mock_meilisearch):
doc_unit = copy.deepcopy(self.unit_dict)
doc_unit["tags"] = {}
doc_unit["collections"] = {'display_name': [], 'key': []}
doc_subsection = copy.deepcopy(self.subsection_dict)
doc_subsection["tags"] = {}
doc_subsection["collections"] = {'display_name': [], 'key': []}
doc_section = copy.deepcopy(self.section_dict)
doc_section["tags"] = {}
doc_section["collections"] = {'display_name': [], 'key': []}

api.rebuild_index()
assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 4
Expand All @@ -286,7 +350,7 @@ def test_reindex_meilisearch(self, mock_meilisearch):
call([doc_sequential, doc_vertical]),
call([doc_problem1, doc_problem2]),
call([doc_collection]),
call([doc_unit]),
call([doc_unit, doc_subsection, doc_section]),
],
any_order=True,
)
Expand All @@ -312,6 +376,12 @@ def test_reindex_meilisearch_incremental(self, mock_meilisearch):
doc_unit = copy.deepcopy(self.unit_dict)
doc_unit["tags"] = {}
doc_unit["collections"] = {"display_name": [], "key": []}
doc_subsection = copy.deepcopy(self.subsection_dict)
doc_subsection["tags"] = {}
doc_subsection["collections"] = {'display_name': [], 'key': []}
doc_section = copy.deepcopy(self.section_dict)
doc_section["tags"] = {}
doc_section["collections"] = {'display_name': [], 'key': []}

api.rebuild_index(incremental=True)
assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 4
Expand All @@ -320,7 +390,7 @@ def test_reindex_meilisearch_incremental(self, mock_meilisearch):
call([doc_sequential, doc_vertical]),
call([doc_problem1, doc_problem2]),
call([doc_collection]),
call([doc_unit]),
call([doc_unit, doc_subsection, doc_section]),
],
any_order=True,
)
Expand Down Expand Up @@ -894,15 +964,23 @@ def test_delete_collection(self, mock_meilisearch):
any_order=True,
)

@ddt.data(
"unit",
"subsection",
"section",
)
@override_settings(MEILISEARCH_ENABLED=True)
def test_delete_index_container(self, mock_meilisearch):
def test_delete_index_container(self, container_type, mock_meilisearch):
"""
Test delete a container index.
"""
library_api.delete_container(self.unit.container_key)
container = getattr(self, container_type)
container_dict = getattr(self, f"{container_type}_dict")

library_api.delete_container(container.container_key)

mock_meilisearch.return_value.index.return_value.delete_document.assert_called_once_with(
self.unit_dict["id"],
container_dict["id"],
)

@override_settings(MEILISEARCH_ENABLED=True)
Expand All @@ -914,22 +992,30 @@ def test_index_library_container_metadata(self, mock_meilisearch):

mock_meilisearch.return_value.index.return_value.update_documents.assert_called_once_with([self.unit_dict])

@ddt.data(
("unit", "lctorg1libunitunit-1-e4527f7c"),
("subsection", "lctorg1libsubsectionsubsection-1-cf808309"),
("section", "lctorg1libsectionsection-1-dc4791a4"),
)
@ddt.unpack
@override_settings(MEILISEARCH_ENABLED=True)
def test_index_tags_in_containers(self, mock_meilisearch):
# Tag collection
tagging_api.tag_object(self.unit_key, self.taxonomyA, ["one", "two"])
tagging_api.tag_object(self.unit_key, self.taxonomyB, ["three", "four"])
def test_index_tags_in_containers(self, container_type, container_id, mock_meilisearch):
container_key = getattr(self, f"{container_type}_key")

# Tag container
tagging_api.tag_object(container_key, self.taxonomyA, ["one", "two"])
tagging_api.tag_object(container_key, self.taxonomyB, ["three", "four"])

# Build expected docs with tags at each stage
doc_unit_with_tags1 = {
"id": "lctorg1libunitunit-1-e4527f7c",
"id": container_id,
"tags": {
'taxonomy': ['A'],
'level0': ['A > one', 'A > two']
}
}
doc_unit_with_tags2 = {
"id": "lctorg1libunitunit-1-e4527f7c",
"id": container_id,
"tags": {
'taxonomy': ['A', 'B'],
'level0': ['A > one', 'A > two', 'B > four', 'B > three']
Expand Down Expand Up @@ -975,3 +1061,51 @@ def test_block_in_units(self, mock_meilisearch):
],
any_order=True,
)

@override_settings(MEILISEARCH_ENABLED=True)
def test_units_in_subsection(self, mock_meilisearch):
with freeze_time(self.created_date):
library_api.update_container_children(
LibraryContainerLocator.from_string(self.subsection_key),
[LibraryContainerLocator.from_string(self.unit_key)],
None,
)

# TODO verify subsections in units
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's left to do here? This looks similar to the units case above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was missing to add to the index the sections and subsections to which it is contained. Navin has advanced this part in #36762 (comment). I'll review it and update the tests.


new_subsection_dict = {
**self.subsection_dict,
"num_children": 1,
'content': {'child_usage_keys': [self.unit_key]}
}
assert mock_meilisearch.return_value.index.return_value.update_documents.call_count == 1
mock_meilisearch.return_value.index.return_value.update_documents.assert_has_calls(
[
call([new_subsection_dict]),
],
any_order=True,
)

@override_settings(MEILISEARCH_ENABLED=True)
def test_section_in_usbsections(self, mock_meilisearch):
with freeze_time(self.created_date):
library_api.update_container_children(
LibraryContainerLocator.from_string(self.section_key),
[LibraryContainerLocator.from_string(self.subsection_key)],
None,
)

# TODO verify section in subsections
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


new_section_dict = {
**self.section_dict,
"num_children": 1,
'content': {'child_usage_keys': [self.subsection_key]}
}
assert mock_meilisearch.return_value.index.return_value.update_documents.call_count == 1
mock_meilisearch.return_value.index.return_value.update_documents.assert_has_calls(
[
call([new_section_dict]),
],
any_order=True,
)
47 changes: 39 additions & 8 deletions openedx/core/djangoapps/content/search/tests/test_documents.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""
Tests for the Studio content search documents (what gets stored in the index)
"""
import ddt
from dataclasses import replace
from datetime import datetime, timezone

Expand Down Expand Up @@ -42,6 +43,7 @@


@skip_unless_cms
@ddt.ddt
class StudioDocumentsTest(SharedModuleStoreTestCase):
"""
Tests for the Studio content search documents (what gets stored in the
Expand Down Expand Up @@ -100,6 +102,26 @@ def setUpClass(cls):
cls.container_key = LibraryContainerLocator.from_string(
"lct:edX:2012_Fall:unit:unit1",
)
cls.subsection = library_api.create_container(
cls.library.key,
container_type=library_api.ContainerType.Subsection,
slug="subsection1",
title="A Subsection in the Search Index",
user_id=None,
)
cls.subsection_key = LibraryContainerLocator.from_string(
"lct:edX:2012_Fall:subsection:subsection1",
)
cls.section = library_api.create_container(
cls.library.key,
container_type=library_api.ContainerType.Section,
slug="section1",
title="A Section in the Search Index",
user_id=None,
)
cls.section_key = LibraryContainerLocator.from_string(
"lct:edX:2012_Fall:section:section1",
)

# Add the problem block to the collection
library_api.update_library_collection_items(
Expand Down Expand Up @@ -130,6 +152,8 @@ def setUpClass(cls):
tagging_api.tag_object(str(cls.library_block.usage_key), cls.difficulty_tags, tags=["Normal"])
tagging_api.tag_object(str(cls.collection_key), cls.difficulty_tags, tags=["Normal"])
tagging_api.tag_object(str(cls.container_key), cls.difficulty_tags, tags=["Normal"])
tagging_api.tag_object(str(cls.subsection_key), cls.difficulty_tags, tags=["Normal"])
tagging_api.tag_object(str(cls.section_key), cls.difficulty_tags, tags=["Normal"])

@property
def toy_course_access_id(self):
Expand Down Expand Up @@ -514,21 +538,28 @@ def test_collection_with_published_library(self):
}
}

def test_draft_container(self):
@ddt.data(
("container", "unit1", "unit", "edd13a0c"),
("subsection", "subsection1", "subsection", "c6c172be"),
("section", "section1", "section", "79ee8fa2"),
)
@ddt.unpack
def test_draft_container_1(self, container_name, container_slug, container_type, doc_id):
"""
Test creating a search document for a draft-only container
"""
doc = searchable_doc_for_container(self.container.container_key)
doc.update(searchable_doc_tags(self.container.container_key))
container = getattr(self, container_name)
doc = searchable_doc_for_container(container.container_key)
doc.update(searchable_doc_tags(container.container_key))

assert doc == {
"id": "lctedx2012_fallunitunit1-edd13a0c",
"block_id": "unit1",
"block_type": "unit",
"usage_key": "lct:edX:2012_Fall:unit:unit1",
"id": f"lctedx2012_fall{container_type}{container_slug}-{doc_id}",
"block_id": container_slug,
"block_type": container_type,
"usage_key": f"lct:edX:2012_Fall:{container_type}:{container_slug}",
"type": "library_container",
"org": "edX",
"display_name": "A Unit in the Search Index",
"display_name": container.display_name,
# description is not set for containers
"num_children": 0,
"content": {
Expand Down
Loading
Loading