Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions cms/djangoapps/contentstore/signals/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ def handle_item_deleted(**kwargs):
id_list.add(block.location)

ComponentLink.objects.filter(downstream_usage_key__in=id_list).delete()
ContainerLink.objects.filter(downstream_usage_key__in=id_list).delete()


@receiver(GRADING_POLICY_CHANGED)
Expand Down
33 changes: 32 additions & 1 deletion openedx/core/djangoapps/content/search/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ def add_with_children(block):
_update_index_docs(docs)


def delete_index_doc(key: OpaqueKey) -> None:
def delete_index_doc(key: OpaqueKey, *, delete_children: bool = False) -> None:
"""
Deletes the document for the given XBlock from the search index

Expand All @@ -647,6 +647,37 @@ def delete_index_doc(key: OpaqueKey) -> None:
"""
doc = searchable_doc_for_key(key)
_delete_index_doc(doc[Fields.id])
if delete_children:
_delete_documents(f'{Fields.breadcrumbs}.{Fields.usage_key} = "{key}"')


def delete_docs_with_context_key(key: OpaqueKey) -> None:
"""
Delete all docs for given context key
"""
_delete_documents(f'{Fields.context_key} = "{key}"')


def _delete_documents(filter_query: str) -> None:
"""
Deletes all documents from the search index that match the given filter

Args:
filter (str): The query to use when filtering documents
"""
if not filter_query:
return

client = _get_meilisearch_client()
current_rebuild_index_name = _get_running_rebuild_index_name()

tasks = []
if current_rebuild_index_name:
# If there is a rebuild in progress, the document will also be removed from the new index.
tasks.append(client.index(current_rebuild_index_name).delete_documents(filter=filter_query))
tasks.append(client.index(STUDIO_INDEX_NAME).delete_documents(filter=filter_query))

_wait_for_meili_tasks(tasks)


def _delete_index_doc(doc_id) -> None:
Expand Down
3 changes: 2 additions & 1 deletion openedx/core/djangoapps/content/search/documents.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,9 @@ class Fields:
org = "org"
access_id = "access_id" # .models.SearchAccess.id
# breadcrumbs: an array of {"display_name": "..."} entries. First one is the name of the course/library itself.
# After that is the name of any parent Section/Subsection/Unit/etc.
# After that is the name of any parent Section/Subsection/Unit/etc and its usage_key.
# It's a list of dictionaries because for now we just include the name of each but in future we may add their IDs.
# Example: [{"display_name": "My course"}, {"display_name": "Section1", "usage_key": "..."}]}
breadcrumbs = "breadcrumbs"
# tags (dictionary)
# See https://blog.meilisearch.com/nested-hierarchical-facets-guide/
Expand Down
11 changes: 11 additions & 0 deletions openedx/core/djangoapps/content/search/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.djangoapps.content.search.models import SearchAccess
from openedx.core.djangoapps.content_libraries import api as lib_api
from xmodule.modulestore.django import SignalHandler

from .api import (
only_if_meilisearch_enabled,
Expand All @@ -51,6 +52,7 @@
upsert_item_containers_index_docs,
)
from .tasks import (
delete_course_index_docs,
delete_library_block_index_doc,
delete_library_container_index_doc,
delete_xblock_index_doc,
Expand Down Expand Up @@ -344,3 +346,12 @@ def handle_reindex_on_signal(**kwargs):
return

upsert_course_blocks_docs.delay(str(course_data.course_key))


@receiver(SignalHandler.course_deleted)
def listen_for_course_delete(sender, course_key, **kwargs): # pylint: disable=unused-argument
"""
Catches the signal that a course has been deleted
and removes its entry from the Course About Search index.
"""
delete_course_index_docs.delay(str(course_key))
1 change: 1 addition & 0 deletions openedx/core/djangoapps/content/search/index_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
Fields.last_published,
Fields.content + "." + Fields.problem_types,
Fields.publish_status,
Fields.breadcrumbs + "." + Fields.usage_key,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Just want to note this: we had to include breadcrumbs in filterable field list, and I think change requires a reindex.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How can we handle this kind of change in a production environment? Should we create a "migration' to force the reindex?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you run reindex_studio --init, which should run on any deployment, it will warn you that the index needs to be rebuilt, because of this code: https://github.com/openedx/edx-platform/blob/3dc96a97e99b90060c14f5b8d5b580fcef8287eb/openedx/core/djangoapps/content/search/api.py#L350-L397

But the whole thing is pretty messy right now, and we need to clean it up eventually: #36868

]

# Mark which attributes are used for keyword search, in order of importance:
Expand Down
17 changes: 16 additions & 1 deletion openedx/core/djangoapps/content/search/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ def delete_xblock_index_doc(usage_key_str: str) -> None:

log.info("Updating content index document for XBlock with id: %s", usage_key)

api.delete_index_doc(usage_key)
# Delete children index data for course blocks.
api.delete_index_doc(usage_key, delete_children=True)


@shared_task(base=LoggedTask, autoretry_for=(MeilisearchError, ConnectionError))
Expand Down Expand Up @@ -168,3 +169,17 @@ def delete_library_container_index_doc(container_key_str: str) -> None:
log.info("Deleting content index document for library block with id: %s", container_key)

api.delete_index_doc(container_key)


@shared_task(base=LoggedTask, autoretry_for=(MeilisearchError, ConnectionError))
@set_code_owner_attribute
def delete_course_index_docs(course_key_str: str) -> None:
"""
Celery task to delete the content index documents for a Course
"""
course_key = CourseKey.from_string(course_key_str)

log.info("Deleting all index documents related to course_key: %s", course_key)

# Delete children index data for course blocks.
api.delete_docs_with_context_key(course_key)
19 changes: 17 additions & 2 deletions openedx/core/djangoapps/content/search/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -618,14 +618,18 @@ def test_index_xblock_tags(self, mock_meilisearch) -> None:
@override_settings(MEILISEARCH_ENABLED=True)
def test_delete_index_xblock(self, mock_meilisearch) -> None:
"""
Test deleting an XBlock doc from the index.
Test deleting an XBlock doc and its children docs from the index.
"""
api.delete_index_doc(self.sequential.usage_key)
api.delete_index_doc(self.sequential.usage_key, delete_children=True)

mock_meilisearch.return_value.index.return_value.delete_document.assert_called_once_with(
self.doc_sequential['id']
)

mock_meilisearch.return_value.index.return_value.delete_documents.assert_called_once_with(
filter=f'breadcrumbs.usage_key = "{self.sequential.usage_key}"'
)

@override_settings(MEILISEARCH_ENABLED=True)
def test_index_library_block_metadata(self, mock_meilisearch) -> None:
"""
Expand Down Expand Up @@ -821,6 +825,17 @@ def test_delete_index_library_block(self, mock_meilisearch) -> None:
self.doc_problem1['id']
)

@override_settings(MEILISEARCH_ENABLED=True)
def test_delete_docs_with_context_key(self, mock_meilisearch) -> None:
"""
Test deleting a all Block docs from the index using context_key.
"""
api.delete_docs_with_context_key(self.course.id)

mock_meilisearch.return_value.index.return_value.delete_documents.assert_called_once_with(
filter=f'context_key = "{self.course.id}"'
)

@override_settings(MEILISEARCH_ENABLED=True)
def test_index_content_library_metadata(self, mock_meilisearch) -> None:
"""
Expand Down
Loading