diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index 9bfab1f1f385..f61ed44421d8 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -30,6 +30,7 @@ from xmodule.contentstore.django import contentstore from xmodule.exceptions import NotFoundError from xmodule.modulestore.django import modulestore +from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.xml_block import XmlMixin from openedx.core.djangoapps.video_config.transcripts_utils import Transcript, build_components_import_path from edxval.api import ( @@ -592,7 +593,11 @@ def _import_xml_node_to_parent( raise NotImplementedError("We don't yet support pasting XBlocks with children") if node_copied_from: - _fetch_and_set_upstream_link(node_copied_from, node_copied_version, temp_xblock, user) + try: + _fetch_and_set_upstream_link(node_copied_from, node_copied_version, temp_xblock, user) + except ItemNotFoundError: + # We don't have a version of the node being copied. Skip setting upstream link + log.warning("Could not find original node '%s' in modulestore to set upstream link.", node_copied_from) # Save the XBlock into modulestore. We need to save the block and its parent for this to work: new_xblock = store.update_item(temp_xblock, user.id, allow_not_found=True) diff --git a/lms/djangoapps/learner_dashboard/tests/test_programs.py b/lms/djangoapps/learner_dashboard/tests/test_programs.py index 0662d09a41ea..f087e1fe68fc 100644 --- a/lms/djangoapps/learner_dashboard/tests/test_programs.py +++ b/lms/djangoapps/learner_dashboard/tests/test_programs.py @@ -16,6 +16,7 @@ from edx_toggles.toggles.testutils import override_waffle_flag from lti_consumer.models import LtiConfiguration +from lti_consumer.utils import CONFIG_ON_DB from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory as ModuleStoreCourseFactory from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory @@ -332,7 +333,7 @@ def test_discussion_flags_exist(self): provider_type="piazza", ) discussion_config.lti_configuration = LtiConfiguration.objects.create( - config_store=LtiConfiguration.CONFIG_ON_DB, + config_store=CONFIG_ON_DB, lti_1p1_launch_url='http://test.url', lti_1p1_client_key='test_client_key', lti_1p1_client_secret='test_client_secret', diff --git a/lms/djangoapps/learner_dashboard/tests/test_views.py b/lms/djangoapps/learner_dashboard/tests/test_views.py index 03f7da1d8c6d..378addbdf734 100644 --- a/lms/djangoapps/learner_dashboard/tests/test_views.py +++ b/lms/djangoapps/learner_dashboard/tests/test_views.py @@ -8,6 +8,7 @@ from django.urls import reverse_lazy from edx_toggles.toggles.testutils import override_waffle_flag from lti_consumer.models import LtiConfiguration +from lti_consumer.utils import CONFIG_ON_DB from markupsafe import Markup from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory as ModuleStoreCourseFactory @@ -109,7 +110,7 @@ def test_api_returns_discussions_iframe(self, staff): provider_type="piazza", ) discussion_config.lti_configuration = LtiConfiguration.objects.create( - config_store=LtiConfiguration.CONFIG_ON_DB, + config_store=CONFIG_ON_DB, lti_1p1_launch_url='http://test.url', lti_1p1_client_key='test_client_key', lti_1p1_client_secret='test_client_secret', diff --git a/openedx/core/djangoapps/course_live/plugins.py b/openedx/core/djangoapps/course_live/plugins.py index c211e27a61b0..cd4931f2f325 100644 --- a/openedx/core/djangoapps/course_live/plugins.py +++ b/openedx/core/djangoapps/course_live/plugins.py @@ -7,6 +7,7 @@ from django.contrib.auth import get_user_model from django.utils.translation import gettext_noop as _ from lti_consumer.models import LtiConfiguration +from lti_consumer.utils import CONFIG_ON_DB from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.course_apps.plugins import CourseApp @@ -51,7 +52,7 @@ def set_enabled(cls, course_key: CourseKey, enabled: bool, user: User) -> bool: configuration.enabled = enabled if not configuration.lti_configuration: configuration.lti_configuration = LtiConfiguration.objects.create( - config_store=LtiConfiguration.CONFIG_ON_DB + config_store=CONFIG_ON_DB ) configuration.save() return configuration.enabled diff --git a/openedx/core/djangoapps/course_live/serializers.py b/openedx/core/djangoapps/course_live/serializers.py index 4643f6f3bfe1..8c24d8628b00 100644 --- a/openedx/core/djangoapps/course_live/serializers.py +++ b/openedx/core/djangoapps/course_live/serializers.py @@ -4,6 +4,7 @@ from django.core.exceptions import ValidationError from django.core.validators import validate_email from lti_consumer.models import LtiConfiguration +from lti_consumer.utils import CONFIG_ON_DB from rest_framework import serializers from .models import CourseLiveConfiguration @@ -50,7 +51,7 @@ def create(self, validated_data): lti_config = validated_data.pop('lti_config', None) instance = LtiConfiguration() instance.version = 'lti_1p1' - instance.config_store = LtiConfiguration.CONFIG_ON_DB + instance.config_store = CONFIG_ON_DB for key, value in validated_data.items(): if key in set(self.Meta.fields).difference(self.Meta.read_only): @@ -69,7 +70,7 @@ def update(self, instance: LtiConfiguration, validated_data: dict) -> LtiConfigu """ Create/update a model-backed instance """ - instance.config_store = LtiConfiguration.CONFIG_ON_DB + instance.config_store = CONFIG_ON_DB lti_config = validated_data.pop('lti_config', None) if lti_config.get('additional_parameters', None): instance.lti_config['additional_parameters'] = lti_config.get('additional_parameters', {}) diff --git a/openedx/core/djangoapps/course_live/tab.py b/openedx/core/djangoapps/course_live/tab.py index 922900efdd5c..643dd804718a 100644 --- a/openedx/core/djangoapps/course_live/tab.py +++ b/openedx/core/djangoapps/course_live/tab.py @@ -4,6 +4,7 @@ from django.contrib.auth.base_user import AbstractBaseUser from django.utils.translation import gettext_lazy from lti_consumer.models import LtiConfiguration +from lti_consumer.utils import CONFIG_ON_DB from opaque_keys.edx.keys import CourseKey from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole, GlobalStaff @@ -73,7 +74,7 @@ def _get_lti_config(self, course: CourseBlock) -> LtiConfiguration: lti_1p1_client_key=provider.key, lti_1p1_client_secret=provider.secret, version='lti_1p1', - config_store=LtiConfiguration.CONFIG_ON_DB, + config_store=CONFIG_ON_DB, ) else: raise ValueError("Provider does not support global credentials") diff --git a/openedx/core/djangoapps/course_live/tests/test_tab.py b/openedx/core/djangoapps/course_live/tests/test_tab.py index 1a69434b7985..7adf57f5bdd2 100644 --- a/openedx/core/djangoapps/course_live/tests/test_tab.py +++ b/openedx/core/djangoapps/course_live/tests/test_tab.py @@ -7,6 +7,7 @@ import ddt from django.test import RequestFactory from lti_consumer.models import CourseAllowPIISharingInLTIFlag, LtiConfiguration +from lti_consumer.utils import CONFIG_ON_DB from lms.djangoapps.courseware.tests.test_tabs import TabTestCase from openedx.core.djangoapps.course_live.models import CourseLiveConfiguration @@ -25,7 +26,7 @@ def setUp(self): provider_type="zoom", ) self.course_live_config.lti_configuration = LtiConfiguration.objects.create( - config_store=LtiConfiguration.CONFIG_ON_DB, + config_store=CONFIG_ON_DB, lti_1p1_launch_url='http://test.url', lti_1p1_client_key='test_client_key', lti_1p1_client_secret='test_client_secret', diff --git a/openedx/core/djangoapps/course_live/tests/test_views.py b/openedx/core/djangoapps/course_live/tests/test_views.py index 97ed4c9ce194..7d998b15c8ff 100644 --- a/openedx/core/djangoapps/course_live/tests/test_views.py +++ b/openedx/core/djangoapps/course_live/tests/test_views.py @@ -6,6 +6,7 @@ from django.test import RequestFactory from django.urls import reverse from lti_consumer.models import CourseAllowPIISharingInLTIFlag, LtiConfiguration +from lti_consumer.utils import CONFIG_ON_DB from markupsafe import Markup from rest_framework.test import APITestCase from xmodule.modulestore import ModuleStoreEnum @@ -414,7 +415,7 @@ def test_api_returns_live_iframe(self): provider_type="zoom", ) live_config.lti_configuration = LtiConfiguration.objects.create( - config_store=LtiConfiguration.CONFIG_ON_DB, + config_store=CONFIG_ON_DB, lti_config={ "pii_share_username": 'true', "pii_share_email": 'true', diff --git a/openedx/core/djangoapps/discussions/serializers.py b/openedx/core/djangoapps/discussions/serializers.py index 5e042575a2c7..ca5cfd607af9 100644 --- a/openedx/core/djangoapps/discussions/serializers.py +++ b/openedx/core/djangoapps/discussions/serializers.py @@ -4,6 +4,7 @@ from django.core.exceptions import ValidationError from lti_consumer.api import get_lti_pii_sharing_state_for_course from lti_consumer.models import LtiConfiguration +from lti_consumer.utils import CONFIG_ON_DB from rest_framework import serializers from xmodule.modulestore.django import modulestore @@ -50,7 +51,7 @@ def update(self, instance: LtiConfiguration, validated_data: dict) -> LtiConfigu Create/update a model-backed instance """ instance = instance or LtiConfiguration() - instance.config_store = LtiConfiguration.CONFIG_ON_DB + instance.config_store = CONFIG_ON_DB pii_sharing_allowed = self.context.get('pii_sharing_allowed', False) if validated_data: for key, value in validated_data.items(): diff --git a/openedx/features/lti_course_tab/tests.py b/openedx/features/lti_course_tab/tests.py index 13827f03bbfd..e645abbacbca 100644 --- a/openedx/features/lti_course_tab/tests.py +++ b/openedx/features/lti_course_tab/tests.py @@ -6,6 +6,7 @@ import ddt from django.test import RequestFactory +from lti_consumer.utils import CONFIG_ON_DB from lti_consumer.models import CourseAllowPIISharingInLTIFlag, LtiConfiguration from lms.djangoapps.courseware.tests.test_tabs import TabTestCase @@ -26,7 +27,7 @@ def setUp(self): provider_type="yellowdig", ) self.discussion_config.lti_configuration = LtiConfiguration.objects.create( - config_store=LtiConfiguration.CONFIG_ON_DB, + config_store=CONFIG_ON_DB, lti_1p1_launch_url='http://test.url', lti_1p1_client_key='test_client_key', lti_1p1_client_secret='test_client_secret', diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index d5bea73754b9..9e10fcb78ff1 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -724,7 +724,7 @@ lazy==1.6 # lti-consumer-xblock # ora2 # xblock -lti-consumer-xblock==10.0.0 +lti-consumer-xblock @ git+https://github.com/open-craft/xblock-lti-consumer@navin/FAL-4318/reuse-config-data # via -r requirements/edx/kernel.in lxml[html-clean]==5.3.2 # via @@ -846,6 +846,7 @@ openedx-events==10.5.0 # edx-event-bus-kafka # edx-event-bus-redis # event-tracking + # lti-consumer-xblock # ora2 openedx-filters==2.1.0 # via diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index fb64d1e114f2..c097e13ec85b 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1203,7 +1203,7 @@ libsass==0.10.0 # via # -c requirements/constraints.txt # -r requirements/edx/assets.txt -lti-consumer-xblock==10.0.0 +lti-consumer-xblock @ git+https://github.com/open-craft/xblock-lti-consumer@navin/FAL-4318/reuse-config-data # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -1408,6 +1408,7 @@ openedx-events==10.5.0 # edx-event-bus-kafka # edx-event-bus-redis # event-tracking + # lti-consumer-xblock # ora2 openedx-filters==2.1.0 # via diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 4f9609d52af9..707b751b0d9f 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -883,7 +883,7 @@ lazy==1.6 # lti-consumer-xblock # ora2 # xblock -lti-consumer-xblock==10.0.0 +lti-consumer-xblock @ git+https://github.com/open-craft/xblock-lti-consumer@navin/FAL-4318/reuse-config-data # via -r requirements/edx/base.txt lxml[html-clean]==5.3.2 # via @@ -1026,6 +1026,7 @@ openedx-events==10.5.0 # edx-event-bus-kafka # edx-event-bus-redis # event-tracking + # lti-consumer-xblock # ora2 openedx-filters==2.1.0 # via diff --git a/requirements/edx/kernel.in b/requirements/edx/kernel.in index 26448abf177d..32b25a2069cd 100644 --- a/requirements/edx/kernel.in +++ b/requirements/edx/kernel.in @@ -101,7 +101,7 @@ jsonfield # Django model field for validated JSON; use laboratory # Library for testing that code refactors/infrastructure changes produce identical results importlib_metadata # Used to access entry_points in i18n_api plugin lxml[html_clean] # XML parser -lti-consumer-xblock>=9.14.2 +lti-consumer-xblock @ git+https://github.com/open-craft/xblock-lti-consumer@navin/FAL-4318/reuse-config-data mako # Primary template language used for server-side page rendering Markdown # Convert text markup to HTML; used in capa problems, forums, and course wikis meilisearch # Library to access Meilisearch search engine (will replace ElasticSearch) diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index b37720cf001d..9839390c58bf 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -924,7 +924,7 @@ lazy==1.6 # lti-consumer-xblock # ora2 # xblock -lti-consumer-xblock==10.0.0 +lti-consumer-xblock @ git+https://github.com/open-craft/xblock-lti-consumer@navin/FAL-4318/reuse-config-data # via -r requirements/edx/base.txt lxml[html-clean]==5.3.2 # via @@ -1075,6 +1075,7 @@ openedx-events==10.5.0 # edx-event-bus-kafka # edx-event-bus-redis # event-tracking + # lti-consumer-xblock # ora2 openedx-filters==2.1.0 # via diff --git a/xmodule/modulestore/__init__.py b/xmodule/modulestore/__init__.py index 3446a4c8c61b..fc4c8c8db44a 100644 --- a/xmodule/modulestore/__init__.py +++ b/xmodule/modulestore/__init__.py @@ -1425,3 +1425,10 @@ def _emit_item_deleted_signal(self, usage_key, user_id): """ if self.signal_handler: self.signal_handler.send("item_deleted", usage_key=usage_key, user_id=user_id) + + def _emit_pre_item_deleted_signal(self, usage_key, user_id): + """ + Helper method used to emit the item_deleted signal. + """ + if self.signal_handler: + self.signal_handler.send("pre_item_deleted", usage_key=usage_key, user_id=user_id) diff --git a/xmodule/modulestore/django.py b/xmodule/modulestore/django.py index c15f03a4e33d..724d0a4149a6 100644 --- a/xmodule/modulestore/django.py +++ b/xmodule/modulestore/django.py @@ -189,11 +189,19 @@ def do_my_expensive_update(course_key): course_deleted = SwitchedSignal("course_deleted") library_updated = SwitchedSignal("library_updated") item_deleted = SwitchedSignal("item_deleted") + pre_item_deleted = SwitchedSignal("pre_item_deleted") _mapping = { signal.name: signal for signal - in [pre_publish, course_published, course_deleted, library_updated, item_deleted] + in [ + pre_publish, + course_published, + course_deleted, + library_updated, + item_deleted, + pre_item_deleted, + ] } def __init__(self, modulestore_class): diff --git a/xmodule/modulestore/split_mongo/split.py b/xmodule/modulestore/split_mongo/split.py index aeacac25333e..bb00f1173e29 100644 --- a/xmodule/modulestore/split_mongo/split.py +++ b/xmodule/modulestore/split_mongo/split.py @@ -2523,6 +2523,8 @@ def delete_item(self, usage_locator, user_id, force=False): # lint-amnesty, pyl # The supplied UsageKey is of the wrong type, so it can't possibly be stored in this modulestore. raise ItemNotFoundError(usage_locator) + self._emit_pre_item_deleted_signal(usage_locator, user_id) + with self.bulk_operations(usage_locator.course_key): original_structure = self._lookup_course(usage_locator.course_key).structure block_key = BlockKey.from_usage_key(usage_locator) diff --git a/xmodule/modulestore/tests/test_mixed_modulestore.py b/xmodule/modulestore/tests/test_mixed_modulestore.py index 8c74419a0b7b..b293b6d41f2e 100644 --- a/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -1126,7 +1126,7 @@ def test_has_changes_missing_child(self, default_ms, default_branch): # check CONTENT_TAGGING_AUTO CourseWaffleFlag # Find: active_versions, 2 structures (published & draft), definition (unnecessary) # Sends: updated draft and published structures and active_versions - @ddt.data((ModuleStoreEnum.Type.split, 5, 2, 3)) + @ddt.data((ModuleStoreEnum.Type.split, 11, 2, 3)) @ddt.unpack def test_delete_item(self, default_ms, num_mysql, max_find, max_send): """ @@ -1149,7 +1149,7 @@ def test_delete_item(self, default_ms, num_mysql, max_find, max_send): # check CONTENT_TAGGING_AUTO CourseWaffleFlag # find: draft and published structures, definition (unnecessary) # sends: update published (why?), draft, and active_versions - @ddt.data((ModuleStoreEnum.Type.split, 5, 3, 3)) + @ddt.data((ModuleStoreEnum.Type.split, 11, 3, 3)) @ddt.unpack def test_delete_private_vertical(self, default_ms, num_mysql, max_find, max_send): """ @@ -1199,7 +1199,7 @@ def test_delete_private_vertical(self, default_ms, num_mysql, max_find, max_send # check CONTENT_TAGGING_AUTO CourseWaffleFlag # find: structure (cached) # send: update structure and active_versions - @ddt.data((ModuleStoreEnum.Type.split, 5, 1, 2)) + @ddt.data((ModuleStoreEnum.Type.split, 11, 1, 2)) @ddt.unpack def test_delete_draft_vertical(self, default_ms, num_mysql, max_find, max_send): """