From b551a32a49ba9b03aa531ad61ba1ceb40c0dd3b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chris=20Ch=C3=A1vez?= Date: Tue, 7 May 2024 13:14:33 -0500 Subject: [PATCH] fix: Stop autotagging nor indexing the course root XBlock (#34627) * fix: Multiple auto-tagging when creating a course * Avoid tagging course-block and course-info blocks * Tests * feat: Avoid create index for course blocks --- openedx/core/djangoapps/content/search/api.py | 7 +++++++ .../content/search/tests/test_api.py | 8 ++++++++ .../djangoapps/content_tagging/handlers.py | 5 +++++ .../content_tagging/tests/test_tasks.py | 19 ++++++++++++++++++- 4 files changed, 38 insertions(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index 24d69dfd9f..bbde4fc982 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -55,6 +55,8 @@ LOCK_EXPIRE = 24 * 60 * 60 # Lock expires in 24 hours MAX_ACCESS_IDS_IN_FILTER = 1_000 MAX_ORGS_IN_FILTER = 1_000 +EXCLUDED_XBLOCK_TYPES = ['course', 'course_info'] + @contextmanager def _index_rebuild_lock() -> Generator[str, None, None]: @@ -372,6 +374,7 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None: docs.append(doc) # pylint: disable=cell-var-from-loop _recurse_children(block, add_with_children) # pylint: disable=cell-var-from-loop + # Index course children _recurse_children(course, add_with_children) if docs: @@ -393,6 +396,10 @@ def upsert_xblock_index_doc(usage_key: UsageKey, recursive: bool = True) -> None recursive (bool): If True, also index all children of the XBlock """ xblock = modulestore().get_item(usage_key) + xblock_type = xblock.scope_ids.block_type + + if xblock_type in EXCLUDED_XBLOCK_TYPES: + return docs = [] diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index f156f9bbb2..cd1acc31b7 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -6,6 +6,7 @@ from __future__ import annotations import copy from unittest.mock import MagicMock, call, patch +from opaque_keys.edx.keys import UsageKey import ddt from django.test import override_settings @@ -62,6 +63,7 @@ class TestSearchApi(ModuleStoreTestCase): fields={"display_name": "Test Course"}, ) course_access, _ = SearchAccess.objects.get_or_create(context_key=self.course.id) + self.course_block_key = "block-v1:org1+test_course+test_run+type@course+block@course" # Create XBlocks self.sequential = self.store.create_child(self.user_id, self.course.location, "sequential", "test_sequential") @@ -184,6 +186,12 @@ class TestSearchApi(ModuleStoreTestCase): mock_meilisearch.return_value.index.return_value.update_documents.assert_called_once_with(expected_docs) + @override_settings(MEILISEARCH_ENABLED=True) + def test_no_index_excluded_xblocks(self, mock_meilisearch): + api.upsert_xblock_index_doc(UsageKey.from_string(self.course_block_key)) + + mock_meilisearch.return_value.index.return_value.update_document.assert_not_called() + @override_settings(MEILISEARCH_ENABLED=True) def test_index_xblock_tags(self, mock_meilisearch): """ diff --git a/openedx/core/djangoapps/content_tagging/handlers.py b/openedx/core/djangoapps/content_tagging/handlers.py index 74b4dd5b33..cc86f7e0dc 100644 --- a/openedx/core/djangoapps/content_tagging/handlers.py +++ b/openedx/core/djangoapps/content_tagging/handlers.py @@ -67,9 +67,14 @@ def auto_tag_xblock(**kwargs): if not CONTENT_TAGGING_AUTO.is_enabled(xblock_info.usage_key.course_key): return + if xblock_info.block_type == 'course_info': + # We want to add tags only to the course id, not with its XBlock + return + if xblock_info.block_type == "course": # Course update is handled by XBlock of course type update_course_tags.delay(str(xblock_info.usage_key.course_key)) + return update_xblock_tags.delay(str(xblock_info.usage_key)) diff --git a/openedx/core/djangoapps/content_tagging/tests/test_tasks.py b/openedx/core/djangoapps/content_tagging/tests/test_tasks.py index ed0cf2c060..3e396eb754 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_tasks.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_tasks.py @@ -8,7 +8,7 @@ from unittest.mock import patch from django.test import override_settings, LiveServerTestCase from django.http import HttpRequest from edx_toggles.toggles.testutils import override_waffle_flag -from openedx_tagging.core.tagging.models import Tag, Taxonomy +from openedx_tagging.core.tagging.models import Tag, Taxonomy, ObjectTag from organizations.models import Organization from common.djangoapps.student.tests.factories import UserFactory @@ -111,6 +111,23 @@ class TestAutoTagging( # type: ignore[misc] # Check if the tags are created in the Course assert self._check_tag(course.id, LANGUAGE_TAXONOMY_ID, "Polski") + def test_only_tag_course_id(self): + # Create course + course = self.store.create_course( + self.orgA.short_name, + "test_course", + "test_run", + self.user_id, + fields={"language": "pl"}, + ) + object_id = str(course.id).replace('course-v1:', '') + + # Check that only one object tag is created for the course + tags = ObjectTag.objects.filter(object_id__contains=object_id) + assert len(tags) == 1 + assert tags[0].value == "Polski" + assert tags[0].object_id == str(course.id) + @override_settings(LANGUAGE_CODE='pt-br') def test_create_course_invalid_language(self): # Create course