From e498e5fcfe3057d09736736f694f15e53821377b Mon Sep 17 00:00:00 2001 From: juan Date: Fri, 17 Nov 2023 17:34:24 -0500 Subject: [PATCH] feat: add new content authoring event signals --- .../core/djangoapps/content_libraries/api.py | 92 +++++- .../content_libraries/libraries_index.py | 70 +++- .../djangoapps/content_libraries/signals.py | 17 - .../content_libraries/tests/base.py | 2 +- .../tests/test_content_libraries.py | 308 +++++++++++++++++- .../tests/test_libraries_index.py | 2 +- requirements/constraints.txt | 2 +- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/testing.txt | 2 +- xmodule/modulestore/mixed.py | 78 ++++- .../tests/test_mixed_modulestore.py | 149 ++++++++- 12 files changed, 661 insertions(+), 65 deletions(-) delete mode 100644 openedx/core/djangoapps/content_libraries/signals.py diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py index 1604a4f85c47..c2b3840ab754 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api.py @@ -84,13 +84,14 @@ ContentLibraryPermission, ContentLibraryBlockImportTask, ) -from openedx.core.djangoapps.content_libraries.signals import ( +from openedx_events.content_authoring.data import ContentLibraryData, LibraryBlockData +from openedx_events.content_authoring.signals import ( CONTENT_LIBRARY_CREATED, - CONTENT_LIBRARY_UPDATED, CONTENT_LIBRARY_DELETED, + CONTENT_LIBRARY_UPDATED, LIBRARY_BLOCK_CREATED, - LIBRARY_BLOCK_UPDATED, LIBRARY_BLOCK_DELETED, + LIBRARY_BLOCK_UPDATED, ) from openedx.core.djangoapps.olx_rest_api.block_serializer import XBlockSerializer from openedx.core.djangoapps.xblock.api import get_block_display_name, load_block @@ -451,7 +452,11 @@ def create_library( ) except IntegrityError: raise LibraryAlreadyExists(slug) # lint-amnesty, pylint: disable=raise-missing-from - CONTENT_LIBRARY_CREATED.send(sender=None, library_key=ref.library_key) + CONTENT_LIBRARY_CREATED.send_event( + content_library=ContentLibraryData( + library_key=ref.library_key + ) + ) return ContentLibraryMetadata( key=ref.library_key, bundle_uuid=bundle.uuid, @@ -601,7 +606,11 @@ def update_library( assert isinstance(description, str) fields["description"] = description update_bundle(ref.bundle_uuid, **fields) - CONTENT_LIBRARY_UPDATED.send(sender=None, library_key=ref.library_key) + CONTENT_LIBRARY_UPDATED.send_event( + content_library=ContentLibraryData( + library_key=ref.library_key + ) + ) def delete_library(library_key): @@ -616,7 +625,11 @@ def delete_library(library_key): # system, which is a better state than having a reference to a library with # no backing blockstore bundle. ref.delete() - CONTENT_LIBRARY_DELETED.send(sender=None, library_key=ref.library_key) + CONTENT_LIBRARY_DELETED.send_event( + content_library=ContentLibraryData( + library_key=ref.library_key + ) + ) try: delete_bundle(bundle_uuid) except: @@ -753,7 +766,12 @@ 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, usage_key=usage_key) + LIBRARY_BLOCK_UPDATED.send_event( + library_block=LibraryBlockData( + library_key=usage_key.context_key, + usage_key=usage_key + ) + ) def create_library_block(library_key, block_type, definition_id): @@ -802,7 +820,12 @@ 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, usage_key=usage_key) + LIBRARY_BLOCK_CREATED.send_event( + library_block=LibraryBlockData( + library_key=ref.library_key, + usage_key=usage_key + ) + ) return get_library_block(usage_key) @@ -855,7 +878,12 @@ 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, usage_key=usage_key) + LIBRARY_BLOCK_DELETED.send_event( + library_block=LibraryBlockData( + library_key=lib_bundle.library_key, + usage_key=usage_key + ) + ) def create_library_block_child(parent_usage_key, block_type, definition_id): @@ -879,7 +907,12 @@ def create_library_block_child(parent_usage_key, block_type, definition_id): 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) + LIBRARY_BLOCK_UPDATED.send_event( + library_block=LibraryBlockData( + library_key=ref.library_key, + usage_key=metadata.usage_key + ) + ) return metadata @@ -929,7 +962,12 @@ 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, usage_key=usage_key) + LIBRARY_BLOCK_UPDATED.send_event( + library_block=LibraryBlockData( + library_key=lib_bundle.library_key, + usage_key=usage_key + ) + ) return LibraryXBlockStaticFile(path=file_metadata.path, url=file_metadata.url, size=file_metadata.size) @@ -950,7 +988,12 @@ 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, usage_key=usage_key) + LIBRARY_BLOCK_UPDATED.send_event( + library_block=LibraryBlockData( + library_key=lib_bundle.library_key, + usage_key=usage_key + ) + ) def get_allowed_block_types(library_key): # pylint: disable=unused-argument @@ -1043,7 +1086,11 @@ def create_bundle_link(library_key, link_id, target_opaque_key, version=None): set_draft_link(draft.uuid, link_id, target_bundle_uuid, version) # Clear the cache: 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_event( + content_library=ContentLibraryData( + library_key=library_key + ) + ) def update_bundle_link(library_key, link_id, version=None, delete=False): @@ -1067,7 +1114,11 @@ def update_bundle_link(library_key, link_id, version=None, delete=False): set_draft_link(draft.uuid, link_id, link.bundle_uuid, version) # Clear the cache: 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_event( + content_library=ContentLibraryData( + library_key=library_key + ) + ) def publish_changes(library_key): @@ -1083,7 +1134,11 @@ 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, update_blocks=True) + CONTENT_LIBRARY_UPDATED.send_event( + content_library=ContentLibraryData( + library_key=library_key + ) + ) def revert_changes(library_key): @@ -1099,7 +1154,12 @@ 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, update_blocks=True) + CONTENT_LIBRARY_UPDATED.send_event( + content_library=ContentLibraryData( + library_key=library_key, + update_blocks=True + ) + ) # Import from Courseware diff --git a/openedx/core/djangoapps/content_libraries/libraries_index.py b/openedx/core/djangoapps/content_libraries/libraries_index.py index b7a682fa5add..4f4c8f1b3cd5 100644 --- a/openedx/core/djangoapps/content_libraries/libraries_index.py +++ b/openedx/core/djangoapps/content_libraries/libraries_index.py @@ -9,16 +9,17 @@ 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 ( +from openedx_events.content_authoring.data import ContentLibraryData, LibraryBlockData +from openedx_events.content_authoring.signals import ( CONTENT_LIBRARY_CREATED, - CONTENT_LIBRARY_UPDATED, CONTENT_LIBRARY_DELETED, + CONTENT_LIBRARY_UPDATED, LIBRARY_BLOCK_CREATED, - LIBRARY_BLOCK_UPDATED, LIBRARY_BLOCK_DELETED, + LIBRARY_BLOCK_UPDATED, ) + +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.models import ContentLibrary from openedx.core.lib.blockstore_api import get_bundle @@ -242,17 +243,20 @@ def get_item_definition(cls, item): @receiver(CONTENT_LIBRARY_CREATED) @receiver(CONTENT_LIBRARY_UPDATED) -@receiver(LIBRARY_BLOCK_CREATED) -@receiver(LIBRARY_BLOCK_UPDATED) -@receiver(LIBRARY_BLOCK_DELETED) -def index_library(sender, library_key, **kwargs): # pylint: disable=unused-argument +def index_library(**kwargs): """ Index library when created or updated, or when its blocks are modified. """ + content_library = kwargs.get('content_library', None) + if not content_library or not isinstance(content_library, ContentLibraryData): + log.error('Received null or incorrect data for event') + return + library_key = content_library.library_key + update_blocks = content_library.update_blocks if ContentLibraryIndexer.indexing_is_enabled(): try: ContentLibraryIndexer.index_items([library_key]) - if kwargs.get('update_blocks', False): + if update_blocks: blocks = LibraryBlockIndexer.get_items(filter_terms={ 'library_key': str(library_key) }) @@ -262,12 +266,38 @@ def index_library(sender, library_key, **kwargs): # pylint: disable=unused-argu log.exception(e) +@receiver(LIBRARY_BLOCK_CREATED) +@receiver(LIBRARY_BLOCK_DELETED) +@receiver(LIBRARY_BLOCK_UPDATED) +def index_library_block(**kwargs): + """ + Index library when its blocks are created, modified, or deleted. + """ + library_block = kwargs.get('library_block', None) + if not library_block or not isinstance(library_block, LibraryBlockData): + log.error('Received null or incorrect data for event') + return + + library_key = library_block.library_key + if ContentLibraryIndexer.indexing_is_enabled(): + try: + ContentLibraryIndexer.index_items([library_key]) + except ElasticConnectionError as e: + log.exception(e) + + @receiver(CONTENT_LIBRARY_DELETED) -def remove_library_index(sender, library_key, **kwargs): # pylint: disable=unused-argument +def remove_library_index(**kwargs): """ Remove from index when library is deleted """ + content_library = kwargs.get('content_library', None) + if not content_library or not isinstance(content_library, ContentLibraryData): + log.error('Received null or incorrect data for event') + return + if ContentLibraryIndexer.indexing_is_enabled(): + library_key = content_library.library_key try: ContentLibraryIndexer.remove_items([library_key]) blocks = LibraryBlockIndexer.get_items(filter_terms={ @@ -280,10 +310,16 @@ def remove_library_index(sender, library_key, **kwargs): # pylint: disable=unus @receiver(LIBRARY_BLOCK_CREATED) @receiver(LIBRARY_BLOCK_UPDATED) -def index_block(sender, usage_key, **kwargs): # pylint: disable=unused-argument +def index_block(**kwargs): """ - Index block metadata when created + Index block metadata when created or updated """ + library_block = kwargs.get('library_block', None) + if not library_block or not isinstance(library_block, LibraryBlockData): + log.error('Received null or incorrect data for event') + return + + usage_key = library_block.usage_key if LibraryBlockIndexer.indexing_is_enabled(): try: LibraryBlockIndexer.index_items([usage_key]) @@ -292,10 +328,16 @@ def index_block(sender, usage_key, **kwargs): # pylint: disable=unused-argument @receiver(LIBRARY_BLOCK_DELETED) -def remove_block_index(sender, usage_key, **kwargs): # pylint: disable=unused-argument +def remove_block_index(**kwargs): """ Remove the block from the index when deleted """ + library_block = kwargs.get('library_block', None) + if not library_block or not isinstance(library_block, LibraryBlockData): + log.error('Received null or incorrect data for LIBRARY_BLOCK_DELETED') + return + + usage_key = library_block.usage_key if LibraryBlockIndexer.indexing_is_enabled(): try: LibraryBlockIndexer.remove_items([usage_key]) diff --git a/openedx/core/djangoapps/content_libraries/signals.py b/openedx/core/djangoapps/content_libraries/signals.py deleted file mode 100644 index c6fd16497a98..000000000000 --- a/openedx/core/djangoapps/content_libraries/signals.py +++ /dev/null @@ -1,17 +0,0 @@ -""" -Content libraries related signals. -""" - -from django.dispatch import Signal - -# providing_args=['library_key'] -CONTENT_LIBRARY_CREATED = Signal() -# providing_args=['library_key', 'update_blocks'] -CONTENT_LIBRARY_UPDATED = Signal() -# providing_args=['library_key'] -CONTENT_LIBRARY_DELETED = Signal() - -# Same providing_args=['library_key', 'usage_key'] for next 3 signals. -LIBRARY_BLOCK_CREATED = Signal() -LIBRARY_BLOCK_DELETED = Signal() -LIBRARY_BLOCK_UPDATED = Signal() diff --git a/openedx/core/djangoapps/content_libraries/tests/base.py b/openedx/core/djangoapps/content_libraries/tests/base.py index 34aee552ee66..997ea394b9f5 100644 --- a/openedx/core/djangoapps/content_libraries/tests/base.py +++ b/openedx/core/djangoapps/content_libraries/tests/base.py @@ -362,7 +362,7 @@ def _set_library_block_asset(self, block_key, file_name, content, expect_respons assert response.status_code == expect_response,\ 'Unexpected response code {}:\n{}'.format(response.status_code, getattr(response, 'data', '(no data)')) - def _delete_library_block_asset(self, block_key, file_name, expect_response=200): + def _delete_library_block_asset(self, block_key, file_name, expect_response=204): """ Delete a static asset file. """ url = URL_LIB_BLOCK_ASSET_FILE.format(block_key=block_key, file_name=file_name) return self._api('delete', url, None, expect_response) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py index 594c7b0f028c..f0d5b57b9014 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py @@ -2,7 +2,7 @@ Tests for Blockstore-based Content Libraries """ from uuid import UUID -from unittest.mock import patch +from unittest.mock import Mock, patch from urllib.parse import urlparse, parse_qsl import json @@ -13,6 +13,17 @@ from django.test.utils import override_settings from organizations.models import Organization from rest_framework.test import APITestCase + +from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2 +from openedx_events.content_authoring.data import ContentLibraryData, LibraryBlockData +from openedx_events.content_authoring.signals import ( + CONTENT_LIBRARY_CREATED, + CONTENT_LIBRARY_DELETED, + CONTENT_LIBRARY_UPDATED, + LIBRARY_BLOCK_CREATED, + LIBRARY_BLOCK_DELETED, + LIBRARY_BLOCK_UPDATED, +) from web_fragments.fragment import Fragment from webob import Response from xblock.core import XBlock @@ -491,7 +502,7 @@ def test_library_blocks_filters(self, is_indexing_enabled): assert len(self._get_library_blocks(lib['id'], {'text_search': 'Foo', 'block_type': 'video'})) == 0 assert len(self._get_library_blocks(lib['id'], {'text_search': 'Baz', 'block_type': 'video'})) == 1 assert len(self._get_library_blocks(lib['id'], {'text_search': 'Baz', 'block_type': ['video', 'html']})) ==\ - 2 + 2 assert len(self._get_library_blocks(lib['id'], {'block_type': 'video'})) == 1 assert len(self._get_library_blocks(lib['id'], {'block_type': 'problem'})) == 3 assert len(self._get_library_blocks(lib['id'], {'block_type': 'squirrel'})) == 0 @@ -543,8 +554,8 @@ def test_library_blocks_with_hierarchy(self): # Check the resulting OLX of the unit: assert self._get_library_block_olx(unit_block['id']) ==\ - '\n \n' \ - ' \n\n' + '\n \n' \ + ' \n\n' # The unit can see and render its children: fragment = self._render_block_view(unit_block["id"], "student_view") @@ -938,6 +949,295 @@ def test_block_types(self, slug, library_type, constrained): else: assert len(types) > 1 + def test_content_library_create_event(self): + """ + Check that CONTENT_LIBRARY_CREATED event is sent when a content library is created. + """ + event_receiver = Mock() + CONTENT_LIBRARY_CREATED.connect(event_receiver) + lib = self._create_library(slug="test_lib_event_create", title="Event Test Library", description="Testing event in library") # lint-amnesty, pylint: disable=line-too-long + library_key = LibraryLocatorV2.from_string(lib['id']) + + event_receiver.assert_called_once() + self.assertDictContainsSubset( + { + "signal": CONTENT_LIBRARY_CREATED, + "sender": None, + "content_library": ContentLibraryData( + library_key=library_key, + update_blocks=False, + ), + }, + event_receiver.call_args.kwargs + ) + + def test_content_library_update_event(self): + """ + Check that CONTENT_LIBRARY_UPDATED event is sent when a content library is updated. + """ + event_receiver = Mock() + CONTENT_LIBRARY_UPDATED.connect(event_receiver) + lib = self._create_library(slug="test_lib_event_update", title="Event Test Library", description="Testing event in library") # lint-amnesty, pylint: disable=line-too-long + + lib2 = self._update_library(lib["id"], title="New Title") + library_key = LibraryLocatorV2.from_string(lib2['id']) + + event_receiver.assert_called_once() + self.assertDictContainsSubset( + { + "signal": CONTENT_LIBRARY_UPDATED, + "sender": None, + "content_library": ContentLibraryData( + library_key=library_key, + update_blocks=False, + ), + }, + event_receiver.call_args.kwargs + ) + + def test_content_library_delete_event(self): + """ + Check that CONTENT_LIBRARY_DELETED event is sent when a content library is deleted. + """ + event_receiver = Mock() + CONTENT_LIBRARY_DELETED.connect(event_receiver) + lib = self._create_library(slug="test_lib_event_delete", title="Event Test Library", description="Testing event in library") # lint-amnesty, pylint: disable=line-too-long + library_key = LibraryLocatorV2.from_string(lib['id']) + + self._delete_library(lib["id"]) + + event_receiver.assert_called_once() + self.assertDictContainsSubset( + { + "signal": CONTENT_LIBRARY_DELETED, + "sender": None, + "content_library": ContentLibraryData( + library_key=library_key, + update_blocks=False, + ), + }, + event_receiver.call_args.kwargs + ) + + def test_library_block_create_event(self): + """ + Check that LIBRARY_BLOCK_CREATED event is sent when a library block is created. + """ + event_receiver = Mock() + LIBRARY_BLOCK_CREATED.connect(event_receiver) + lib = self._create_library(slug="test_lib_block_event_create", title="Event Test Library", description="Testing event in library") # lint-amnesty, pylint: disable=line-too-long + lib_id = lib["id"] + self._add_block_to_library(lib_id, "problem", "problem1") + + library_key = LibraryLocatorV2.from_string(lib_id) + usage_key = LibraryUsageLocatorV2( + lib_key=library_key, + block_type="problem", + usage_id="problem1" + ) + + event_receiver.assert_called_once() + self.assertDictContainsSubset( + { + "signal": LIBRARY_BLOCK_CREATED, + "sender": None, + "library_block": LibraryBlockData( + library_key=library_key, + usage_key=usage_key + ), + }, + event_receiver.call_args.kwargs + ) + + def test_library_block_olx_update_event(self): + """ + Check that LIBRARY_BLOCK_CREATED event is sent when the OLX source is updated. + """ + event_receiver = Mock() + LIBRARY_BLOCK_UPDATED.connect(event_receiver) + lib = self._create_library(slug="test_lib_block_event_olx_update", title="Event Test Library", description="Testing event in library") # lint-amnesty, pylint: disable=line-too-long + lib_id = lib["id"] + + library_key = LibraryLocatorV2.from_string(lib_id) + + block = self._add_block_to_library(lib_id, "problem", "problem1") + block_id = block["id"] + usage_key = LibraryUsageLocatorV2( + lib_key=library_key, + block_type="problem", + usage_id="problem1" + ) + + new_olx = """ + + +

This is a normal capa problem with unicode 🔥. It has "maximum attempts" set to **5**.

+ + + XBlock metadata only + XBlock data/metadata and associated static asset files + Static asset files for XBlocks and courseware + XModule metadata only + + + + """.strip() + + self._set_library_block_olx(block_id, new_olx) + + event_receiver.assert_called_once() + self.assertDictContainsSubset( + { + "signal": LIBRARY_BLOCK_UPDATED, + "sender": None, + "library_block": LibraryBlockData( + library_key=library_key, + usage_key=usage_key + ), + }, + event_receiver.call_args.kwargs + ) + + def test_library_block_child_update_event(self): + """ + Check that LIBRARY_BLOCK_CREATED event is sent when a child is created. + """ + event_receiver = Mock() + LIBRARY_BLOCK_UPDATED.connect(event_receiver) + lib = self._create_library(slug="test_lib_block_event_child_update", title="Event Test Library", description="Testing event in library") # lint-amnesty, pylint: disable=line-too-long + lib_id = lib["id"] + + library_key = LibraryLocatorV2.from_string(lib_id) + + parent_block = self._add_block_to_library(lib_id, "unit", "u1") + parent_block_id = parent_block["id"] + + self._add_block_to_library(lib["id"], "problem", "problem1", parent_block=parent_block_id) + + usage_key = LibraryUsageLocatorV2( + lib_key=library_key, + block_type="problem", + usage_id="problem1" + ) + + event_receiver.assert_called_once() + self.assertDictContainsSubset( + { + "signal": LIBRARY_BLOCK_UPDATED, + "sender": None, + "library_block": LibraryBlockData( + library_key=library_key, + usage_key=usage_key + ), + }, + event_receiver.call_args.kwargs + ) + + def test_library_block_add_asset_update_event(self): + """ + Check that LIBRARY_BLOCK_CREATED event is sent when a static asset is uploaded associated with the XBlock. + """ + event_receiver = Mock() + LIBRARY_BLOCK_UPDATED.connect(event_receiver) + lib = self._create_library(slug="test_lib_block_event_add_asset_update", title="Event Test Library", description="Testing event in library") # lint-amnesty, pylint: disable=line-too-long + lib_id = lib["id"] + + library_key = LibraryLocatorV2.from_string(lib_id) + + block = self._add_block_to_library(lib_id, "unit", "u1") + block_id = block["id"] + self._set_library_block_asset(block_id, "test.txt", b"data") + + usage_key = LibraryUsageLocatorV2( + lib_key=library_key, + block_type="unit", + usage_id="u1" + ) + + event_receiver.assert_called_once() + self.assertDictContainsSubset( + { + "signal": LIBRARY_BLOCK_UPDATED, + "sender": None, + "library_block": LibraryBlockData( + library_key=library_key, + usage_key=usage_key + ), + }, + event_receiver.call_args.kwargs + ) + + def test_library_block_del_asset_update_event(self): + """ + Check that LIBRARY_BLOCK_CREATED event is sent when a static asset is removed from XBlock. + """ + event_receiver = Mock() + LIBRARY_BLOCK_UPDATED.connect(event_receiver) + lib = self._create_library(slug="test_lib_block_event_del_asset_update", title="Event Test Library", description="Testing event in library") # lint-amnesty, pylint: disable=line-too-long + lib_id = lib["id"] + + library_key = LibraryLocatorV2.from_string(lib_id) + + block = self._add_block_to_library(lib_id, "unit", "u1") + block_id = block["id"] + self._set_library_block_asset(block_id, "test.txt", b"data") + + self._delete_library_block_asset(block_id, 'text.txt') + + usage_key = LibraryUsageLocatorV2( + lib_key=library_key, + block_type="unit", + usage_id="u1" + ) + + event_receiver.assert_called() + self.assertDictContainsSubset( + { + "signal": LIBRARY_BLOCK_UPDATED, + "sender": None, + "library_block": LibraryBlockData( + library_key=library_key, + usage_key=usage_key + ), + }, + event_receiver.call_args.kwargs + ) + + def test_library_block_delete_event(self): + """ + Check that LIBRARY_BLOCK_DELETED event is sent when a content library is deleted. + """ + event_receiver = Mock() + LIBRARY_BLOCK_DELETED.connect(event_receiver) + lib = self._create_library(slug="test_lib_block_event_delete", title="Event Test Library", description="Testing event in library") # lint-amnesty, pylint: disable=line-too-long + + lib_id = lib["id"] + library_key = LibraryLocatorV2.from_string(lib_id) + + block = self._add_block_to_library(lib_id, "problem", "problem1") + block_id = block['id'] + + usage_key = LibraryUsageLocatorV2( + lib_key=library_key, + block_type="problem", + usage_id="problem1" + ) + + self._delete_library_block(block_id) + + event_receiver.assert_called() + self.assertDictContainsSubset( + { + "signal": LIBRARY_BLOCK_DELETED, + "sender": None, + "library_block": LibraryBlockData( + library_key=library_key, + usage_key=usage_key + ), + }, + event_receiver.call_args.kwargs + ) + @elasticsearch_test class ContentLibrariesBlockstoreServiceTest( diff --git a/openedx/core/djangoapps/content_libraries/tests/test_libraries_index.py b/openedx/core/djangoapps/content_libraries/tests/test_libraries_index.py index fb6464c35c06..0e46680351af 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_libraries_index.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_libraries_index.py @@ -165,7 +165,7 @@ def verify_uncommitted_libraries(library_key, has_unpublished_changes, has_unpub self._set_library_block_asset(block["id"], "whatever.png", b"data") verify_uncommitted_libraries(library_key, True, False) commit_library_and_verify(library_key) - self._delete_library_block_asset(block["id"], "whatever.png", expect_response=204) + self._delete_library_block_asset(block["id"], "whatever.png") verify_uncommitted_libraries(library_key, True, False) commit_library_and_verify(library_key) diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 3017d343ad31..38872ccdfe56 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -100,4 +100,4 @@ pytz==2022.2.1 # openedx-events>0.13.0 causes test failures due to breaking changes # These broken tests will be fixed in a separate PR -openedx-events==0.13.0 +openedx-events==8.3.0 diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 038129222d69..2c7e75896ccc 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -759,7 +759,7 @@ openedx-calc==3.0.1 # via -r requirements/edx/base.in openedx-django-pyfs==3.2.1 # via xblock -openedx-events==0.13.0 +openedx-events==8.3.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 3e5ba143d975..406ff19b1614 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -991,7 +991,7 @@ openedx-django-pyfs==3.2.1 # via # -r requirements/edx/testing.txt # xblock -openedx-events==0.13.0 +openedx-events==8.3.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 84935527d441..e1ea3eb1187f 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -942,7 +942,7 @@ openedx-django-pyfs==3.2.1 # via # -r requirements/edx/base.txt # xblock -openedx-events==0.13.0 +openedx-events==8.3.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/xmodule/modulestore/mixed.py b/xmodule/modulestore/mixed.py index 17b5e8a0869d..7e0231a4514d 100644 --- a/xmodule/modulestore/mixed.py +++ b/xmodule/modulestore/mixed.py @@ -13,6 +13,16 @@ from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryLocator +from openedx_events.content_authoring.data import CourseData, XBlockData +from openedx_events.content_authoring.signals import ( + COURSE_CREATED, + XBLOCK_CREATED, + XBLOCK_DELETED, + XBLOCK_PUBLISHED, + XBLOCK_UPDATED +) + +from django.utils.timezone import datetime, timezone from xmodule.assetstore import AssetMetadata from . import XMODULE_FIELDS_WITH_USAGE_KEYS, ModuleStoreWriteBase @@ -667,6 +677,14 @@ def create_course(self, org, course, run, user_id, **kwargs): # lint-amnesty, p # add new course to the mapping self.mappings[course_key] = store + # .. event_implemented_name: COURSE_CREATED + COURSE_CREATED.send_event( + time=datetime.now(timezone.utc), + course=CourseData( + course_key=course_key, + ) + ) + return course @strip_key @@ -739,7 +757,17 @@ def create_item(self, user_id, course_key, block_type, block_id=None, fields=Non in the newly created block """ modulestore = self._verify_modulestore_support(course_key, 'create_item') - return modulestore.create_item(user_id, course_key, block_type, block_id=block_id, fields=fields, **kwargs) + xblock = modulestore.create_item(user_id, course_key, block_type, block_id=block_id, fields=fields, **kwargs) + # .. event_implemented_name: XBLOCK_CREATED + XBLOCK_CREATED.send_event( + time=datetime.now(timezone.utc), + xblock_info=XBlockData( + usage_key=xblock.location.for_branch(None), + block_type=block_type, + version=xblock.location + ) + ) + return xblock @strip_key @prepare_asides @@ -760,7 +788,19 @@ def create_child(self, user_id, parent_usage_key, block_type, block_id=None, fie in the newly created block """ modulestore = self._verify_modulestore_support(parent_usage_key.course_key, 'create_child') - return modulestore.create_child(user_id, parent_usage_key, block_type, block_id=block_id, fields=fields, **kwargs) # lint-amnesty, pylint: disable=line-too-long + xblock = modulestore.create_child( + user_id, parent_usage_key, block_type, block_id=block_id, fields=fields, **kwargs + ) + # .. event_implemented_name: XBLOCK_CREATED + XBLOCK_CREATED.send_event( + time=datetime.now(timezone.utc), + xblock_info=XBlockData( + usage_key=xblock.location.for_branch(None), + block_type=block_type, + version=xblock.location + ) + ) + return xblock @strip_key @prepare_asides @@ -789,7 +829,17 @@ def update_item(self, xblock, user_id, allow_not_found=False, **kwargs): # lint (content, children, and metadata) attribute the change to the given user. """ store = self._verify_modulestore_support(xblock.location.course_key, 'update_item') - return store.update_item(xblock, user_id, allow_not_found, **kwargs) + xblock = store.update_item(xblock, user_id, allow_not_found, **kwargs) + # .. event_implemented_name: XBLOCK_UPDATED + XBLOCK_UPDATED.send_event( + time=datetime.now(timezone.utc), + xblock_info=XBlockData( + usage_key=xblock.location.for_branch(None), + block_type=xblock.location.block_type, + version=xblock.location + ) + ) + return xblock @strip_key def delete_item(self, location, user_id, **kwargs): # lint-amnesty, pylint: disable=arguments-differ @@ -797,7 +847,16 @@ def delete_item(self, location, user_id, **kwargs): # lint-amnesty, pylint: dis Delete the given item from persistence. kwargs allow modulestore specific parameters. """ store = self._verify_modulestore_support(location.course_key, 'delete_item') - return store.delete_item(location, user_id=user_id, **kwargs) + item = store.delete_item(location, user_id=user_id, **kwargs) + # .. event_implemented_name: XBLOCK_DELETED + XBLOCK_DELETED.send_event( + time=datetime.now(timezone.utc), + xblock_info=XBlockData( + usage_key=location, + block_type=location.block_type, + ) + ) + return item def revert_to_published(self, location, user_id): """ @@ -911,7 +970,16 @@ def publish(self, location, user_id, **kwargs): Returns the newly published item. """ store = self._verify_modulestore_support(location.course_key, 'publish') - return store.publish(location, user_id, **kwargs) + item = store.publish(location, user_id, **kwargs) + # .. event_implemented_name: XBLOCK_PUBLISHED + XBLOCK_PUBLISHED.send_event( + time=datetime.now(timezone.utc), + xblock_info=XBlockData( + usage_key=location, + block_type=location.block_type, + ) + ) + return item @strip_key def unpublish(self, location, user_id, **kwargs): diff --git a/xmodule/modulestore/tests/test_mixed_modulestore.py b/xmodule/modulestore/tests/test_mixed_modulestore.py index 318f6dc57d9b..e4e8e64f3ec2 100644 --- a/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -15,6 +15,14 @@ from unittest.mock import Mock, call, patch import ddt +from openedx_events.content_authoring.data import CourseData, XBlockData +from openedx_events.content_authoring.signals import ( + COURSE_CREATED, + XBLOCK_CREATED, + XBLOCK_DELETED, + XBLOCK_PUBLISHED, + XBLOCK_UPDATED +) import pymongo import pytest # Mixed modulestore depends on django, so we'll manually configure some django settings @@ -109,6 +117,13 @@ class CommonMixedModuleStoreSetup(CourseComparisonTest): ], 'xblock_mixins': modulestore_options['xblock_mixins'], } + ENABLED_OPENEDX_EVENTS = [ + "org.openedx.content_authoring.course.created.v1", + "org.openedx.content_authoring.xblock.created.v1", + "org.openedx.content_authoring.xblock.updated.v1", + "org.openedx.content_authoring.xblock.deleted.v1", + "org.openedx.content_authoring.xblock.published.v1", + ] def setUp(self): """ @@ -481,7 +496,7 @@ def test_get_items_include_orphans(self, default_ms, expected_items_in_tree, orp assert {'course', 'about'}.issubset({item.location.block_type for item in items}) # pylint: disable=line-too-long # Assert that about is a detached category found in get_items assert [item.location.block_type for item in items if item.location.block_type == 'about'][0]\ - in DETACHED_XBLOCK_TYPES + in DETACHED_XBLOCK_TYPES assert len(items) == 2 # Check that orphans are not found @@ -724,6 +739,134 @@ def test_publish_automatically_after_delete_unit(self, default_ms): self.store.delete_item(vertical.location, self.user_id) assert not self._has_changes(sequential.location) + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_course_create_event(self, default_ms): + """ + Check that COURSE_CREATED event is sent when a course is created. + """ + self.initdb(default_ms) + + event_receiver = Mock() + COURSE_CREATED.connect(event_receiver) + + test_course = self.store.create_course('test_org', 'test_course', 'test_run', self.user_id) + + event_receiver.assert_called() + + self.assertDictContainsSubset( + { + "signal": COURSE_CREATED, + "sender": None, + "course": CourseData( + course_key=test_course.id, + ), + }, + event_receiver.call_args.kwargs + ) + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_xblock_create_event(self, default_ms): + """ + Check that XBLOCK_CREATED event is sent when xblock is created. + """ + self.initdb(default_ms) + + event_receiver = Mock() + XBLOCK_CREATED.connect(event_receiver) + + test_course = self.store.create_course('test_org', 'test_course', 'test_run', self.user_id) + + # create sequential to test against + sequential = self.store.create_child(self.user_id, test_course.location, 'sequential', 'test_sequential') + + event_receiver.assert_called() + + assert event_receiver.call_args.kwargs['signal'] == XBLOCK_CREATED + assert event_receiver.call_args.kwargs['xblock_info'].usage_key == sequential.location + assert event_receiver.call_args.kwargs['xblock_info'].block_type == sequential.location.block_type + assert event_receiver.call_args.kwargs['xblock_info'].version.for_branch(None) == sequential.location + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_xblock_update_event(self, default_ms): + """ + Check that XBLOCK_UPDATED event is sent when xblock is updated. + """ + self.initdb(default_ms) + + event_receiver = Mock() + XBLOCK_UPDATED.connect(event_receiver) + + test_course = self.store.create_course('test_org', 'test_course', 'test_run', self.user_id) + + # create sequential to test against + sequential = self.store.create_child(self.user_id, test_course.location, 'sequential', 'test_sequential') + + # Change the xblock + sequential.display_name = 'Updated Display Name' + self.store.update_item(sequential, user_id=self.user_id) + + event_receiver.assert_called() + + assert event_receiver.call_args.kwargs['signal'] == XBLOCK_UPDATED + assert event_receiver.call_args.kwargs['xblock_info'].usage_key == sequential.location + assert event_receiver.call_args.kwargs['xblock_info'].block_type == sequential.location.block_type + assert event_receiver.call_args.kwargs['xblock_info'].version.for_branch(None) == sequential.location + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_xblock_publish_event(self, default_ms): + """ + Check that XBLOCK_PUBLISHED event is sent when xblock is published. + """ + self.initdb(default_ms) + event_receiver = Mock() + XBLOCK_PUBLISHED.connect(event_receiver) + test_course = self.store.create_course('test_org', 'test_course', 'test_run', self.user_id) + # create sequential and vertical to test against + sequential = self.store.create_child(self.user_id, test_course.location, 'sequential', 'test_sequential') + self.store.create_child(self.user_id, sequential.location, 'vertical', 'test_vertical') + # publish sequential changes + self.store.publish(sequential.location, self.user_id) + event_receiver.assert_called() + self.assertDictContainsSubset( + { + "signal": XBLOCK_PUBLISHED, + "sender": None, + "xblock_info": XBlockData( + usage_key=sequential.location, + block_type=sequential.location.block_type, + ), + }, + event_receiver.call_args.kwargs + ) + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_xblock_delete_event(self, default_ms): + """ + Check that XBLOCK_DELETED event is sent when xblock is deleted. + """ + self.initdb(default_ms) + event_receiver = Mock() + XBLOCK_DELETED.connect(event_receiver) + test_course = self.store.create_course('test_org', 'test_course', 'test_run', self.user_id) + # create sequential and vertical to test against + sequential = self.store.create_child(self.user_id, test_course.location, 'sequential', 'test_sequential') + vertical = self.store.create_child(self.user_id, sequential.location, 'vertical', 'test_vertical') + # publish sequential changes + self.store.publish(sequential.location, self.user_id) + # delete vertical + self.store.delete_item(vertical.location, self.user_id) + event_receiver.assert_called() + self.assertDictContainsSubset( + { + "signal": XBLOCK_DELETED, + "sender": None, + "xblock_info": XBlockData( + usage_key=vertical.location, + block_type=vertical.location.block_type, + ), + }, + event_receiver.call_args.kwargs + ) + def setup_has_changes(self, default_ms): """ Common set up for has_changes tests below. @@ -1135,7 +1278,7 @@ def verify_get_parent_locations_results(self, expected_results): """ for child_location, parent_location, revision in expected_results: assert parent_location.for_branch(None) if parent_location else parent_location == \ - self.store.get_parent_location(child_location, revision=revision) + self.store.get_parent_location(child_location, revision=revision) def verify_item_parent(self, item_location, expected_parent_location, old_parent_location, is_reverted=False): """ @@ -2379,7 +2522,7 @@ def assertNumProblems(display_name, expected_number): Asserts the number of problems with the given display name is the given expected number. """ assert len(self.store.get_items(course_key.for_branch(None), settings={'display_name': display_name})) ==\ - expected_number + expected_number def assertProblemNameEquals(expected_display_name): """