feat: remove soft- and hard-deleted collections from search index

openedx/core/djangoapps/content/search/api.py

@@ -29,6 +29,7 @@
 from xmodule.modulestore.django import modulestore
 
 from .documents import (
+    DocType,
     Fields,
     meili_id_from_opaque_key,
     searchable_doc_for_course_block,
@@ -256,6 +257,28 @@ def _update_index_docs(docs) -> None:
     _wait_for_meili_tasks(tasks)
 
 
+def _delete_index_docs(docs) -> None:
+    """
+    Helper function that deletes the given documents from the search index
+
+    If there is a rebuild in progress, the document will also be removed from the new index.
+    """
+    if not docs:
+        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(docs))
+
+    tasks.append(client.index(STUDIO_INDEX_NAME).delete_documents(docs))
+
+    _wait_for_meili_tasks(tasks)
+
+
 def only_if_meilisearch_enabled(f):
     """
     Only call `f` if meilisearch is enabled
@@ -563,13 +586,19 @@ def upsert_library_block_index_doc(usage_key: UsageKey) -> None:
 
 def upsert_library_collection_index_doc(library_key: LibraryLocatorV2, collection_key: str) -> None:
     """
-    Creates or updates the document for the given Library Collection in the search index
+    Creates, updates, or deletes the document for the given Library Collection in the search index.
+
+    If the Collection is not found or disabled (i.e. soft-deleted), then delete it from the search index.
     """
-    docs = [
-        searchable_doc_for_collection(library_key, collection_key)
-    ]
+    doc = searchable_doc_for_collection(library_key, collection_key)
 
-    _update_index_docs(docs)
+    # If the doc is a "collection", then it's valid for insert/update.
+    if doc.get(Fields.type, "") == DocType.collection:
+        _update_index_docs([doc])
+
+    # Otherwise, the collection wasn't found or it's been disabled/soft-deleted, so we delete it from the index.
+    else:
+        _delete_index_docs([doc])
 
 
 def upsert_content_library_index_docs(library_key: LibraryLocatorV2) -> None:

openedx/core/djangoapps/content/search/documents.py

@@ -407,7 +407,9 @@ def searchable_doc_for_collection(
         # Collection not found, so we can only return the base doc
         pass
 
-    if collection:
+    # Disabled collections should be removed from the search index,
+    # so we intentionally don't populate the rest of the info here.
+    if collection and collection.enabled:
         assert collection.key == collection_key
 
         doc.update({

openedx/core/djangoapps/content/search/handlers.py

@@ -23,6 +23,7 @@
     LIBRARY_BLOCK_DELETED,
     LIBRARY_BLOCK_UPDATED,
     LIBRARY_COLLECTION_CREATED,
+    LIBRARY_COLLECTION_DELETED,
     LIBRARY_COLLECTION_UPDATED,
     XBLOCK_CREATED,
     XBLOCK_DELETED,
@@ -166,6 +167,7 @@ def content_library_updated_handler(**kwargs) -> None:
 
 
 @receiver(LIBRARY_COLLECTION_CREATED)
+@receiver(LIBRARY_COLLECTION_DELETED)
 @receiver(LIBRARY_COLLECTION_UPDATED)
 @only_if_meilisearch_enabled
 def library_collection_updated_handler(**kwargs) -> None:

openedx/core/djangoapps/content/search/tests/test_api.py

@@ -615,3 +615,96 @@ def test_index_tags_in_collections(self, mock_meilisearch):
             ],
             any_order=True,
         )
+
+    @override_settings(MEILISEARCH_ENABLED=True)
+    def test_delete_collection(self, mock_meilisearch):
+        """
+        Test soft-deleting, restoring, and hard-deleting a collection.
+        """
+        # Add a component to the collection
+        updated_date = datetime(2023, 6, 7, 8, 9, 10, tzinfo=timezone.utc)
+        with freeze_time(updated_date):
+            library_api.update_library_collection_components(
+                self.library.key,
+                collection_key=self.collection.key,
+                usage_keys=[
+                    self.problem1.usage_key,
+                ],
+            )
+
+        doc_collection = copy.deepcopy(self.collection_dict)
+        doc_collection["num_children"] = 1
+        doc_collection["modified"] = updated_date.timestamp()
+        doc_problem_with_collection = {
+            "id": self.doc_problem1["id"],
+            "collections": {
+                "display_name": [self.collection.title],
+                "key": [self.collection.key],
+            },
+        }
+        assert mock_meilisearch.return_value.index.return_value.update_documents.call_count == 2
+        mock_meilisearch.return_value.index.return_value.update_documents.assert_has_calls(
+            [
+                call([doc_collection]),
+                call([doc_problem_with_collection]),
+            ],
+            any_order=True,
+        )
+        mock_meilisearch.return_value.index.reset_mock()
+
+        # Soft-delete the collection
+        authoring_api.delete_collection(
+            self.collection.learning_package_id,
+            self.collection.key,
+        )
+
+        doc_collection = {
+            "id": self.collection_dict["id"],
+        }
+        doc_problem_without_collection = {
+            "id": self.doc_problem1["id"],
+            "collections": {},
+        }
+
+        mock_meilisearch.return_value.index.return_value.delete_documents.assert_called_once_with([doc_collection])
+        ## TODO: update the components in a soft-deleted collection
+        # mock_meilisearch.return_value.index.return_value.update_documents.assert_called_once_with([
+        #     doc_problem_without_collection
+        # ])
+        mock_meilisearch.return_value.index.reset_mock()
+
+        # Restore the collection
+        restored_date = datetime(2023, 8, 9, 10, 11, 12, tzinfo=timezone.utc)
+        with freeze_time(restored_date):
+            authoring_api.restore_collection(
+                self.collection.learning_package_id,
+                self.collection.key,
+            )
+
+        doc_collection = copy.deepcopy(self.collection_dict)
+        doc_collection["num_children"] = 1
+        doc_collection["modified"] = restored_date.timestamp()
+
+        mock_meilisearch.return_value.index.return_value.update_documents.assert_called_once_with([doc_collection])
+        ## TODO: update the components in a restored collection
+        # mock_meilisearch.return_value.index.return_value.update_documents.assert_called_once_with([
+        #     doc_problem_with_collection
+        # ])
+        mock_meilisearch.return_value.index.reset_mock()
+
+        # Hard-delete the collection
+        authoring_api.delete_collection(
+            self.collection.learning_package_id,
+            self.collection.key,
+            hard_delete=True,
+        )
+
+        doc_collection = {
+            "id": self.collection_dict["id"],
+        }
+
+        mock_meilisearch.return_value.index.return_value.delete_documents.assert_called_once_with([doc_collection])
+        ## TODO: update the components in a deleted collection?
+        # mock_meilisearch.return_value.index.return_value.update_documents.assert_called_once_with([
+        #     doc_problem_without_collection
+        # ])

openedx/core/djangoapps/content_libraries/signal_handlers.py

@@ -5,7 +5,7 @@
 import logging
 
 from django.conf import settings
-from django.db.models.signals import post_save
+from django.db.models.signals import post_save, post_delete
 from django.dispatch import receiver
 
 from opaque_keys import InvalidKeyError  # lint-amnesty, pylint: disable=wrong-import-order
@@ -15,6 +15,7 @@
 )
 from openedx_events.content_authoring.signals import (
     LIBRARY_COLLECTION_CREATED,
+    LIBRARY_COLLECTION_DELETED,
     LIBRARY_COLLECTION_UPDATED,
 )
 from openedx_learning.api.authoring_models import Collection
@@ -93,3 +94,22 @@ def library_collection_saved(sender, instance, created, **kwargs):
                 collection_key=instance.key,
             )
         )
+
+
+@receiver(post_delete, sender=Collection, dispatch_uid="library_collection_deleted")
+def library_collection_deleted(sender, instance, **kwargs):
+    """
+    Raises LIBRARY_COLLECTION_DELETED for the deleted Collection.
+    """
+    try:
+        library = ContentLibrary.objects.get(learning_package_id=instance.learning_package_id)
+    except ContentLibrary.DoesNotExist:
+        log.error("{instance} is not associated with a content library.")
+        return
+
+    LIBRARY_COLLECTION_DELETED.send_event(
+        library_collection=LibraryCollectionData(
+            library_key=library.library_key,
+            collection_key=instance.key,
+        )
+    )

openedx/core/djangoapps/content_libraries/tests/test_api.py

@@ -20,9 +20,11 @@
 from openedx_events.content_authoring.signals import (
     CONTENT_OBJECT_ASSOCIATIONS_CHANGED,
     LIBRARY_COLLECTION_CREATED,
+    LIBRARY_COLLECTION_DELETED,
     LIBRARY_COLLECTION_UPDATED,
 )
 from openedx_events.tests.utils import OpenEdxEventsTestMixin
+from openedx_learning.api import authoring as authoring_api
 
 from .. import api
 from ..models import ContentLibrary
@@ -264,6 +266,7 @@ class ContentLibraryCollectionsTest(ContentLibrariesRestApiTest, OpenEdxEventsTe
     ENABLED_OPENEDX_EVENTS = [
         CONTENT_OBJECT_ASSOCIATIONS_CHANGED.event_type,
         LIBRARY_COLLECTION_CREATED.event_type,
+        LIBRARY_COLLECTION_DELETED.event_type,
         LIBRARY_COLLECTION_UPDATED.event_type,
     ]
 
@@ -386,6 +389,29 @@ def test_update_library_collection_wrong_library(self):
                 self.col2.key,
             )
 
+    def test_delete_library_collection(self):
+        event_receiver = mock.Mock()
+        LIBRARY_COLLECTION_DELETED.connect(event_receiver)
+
+        authoring_api.delete_collection(
+            self.lib1.learning_package_id,
+            self.col1.key,
+            hard_delete=True,
+        )
+
+        assert event_receiver.call_count == 1
+        self.assertDictContainsSubset(
+            {
+                "signal": LIBRARY_COLLECTION_DELETED,
+                "sender": None,
+                "library_collection": LibraryCollectionData(
+                    self.lib1.library_key,
+                    collection_key="COL1",
+                ),
+            },
+            event_receiver.call_args_list[0].kwargs,
+        )
+
     def test_update_library_collection_components(self):
         assert not list(self.col1.entities.all())