From 8bcfb1d69a52130ac11aa5d3404fbfac4c8e72f1 Mon Sep 17 00:00:00 2001 From: IrfanUddinAhmad Date: Mon, 11 Mar 2024 14:53:37 +0500 Subject: [PATCH 1/4] fix: removed waffle switch around oep-50 filter --- xmodule/tests/test_vertical.py | 6 ---- xmodule/vertical_block.py | 50 ++++++++++++---------------------- 2 files changed, 18 insertions(+), 38 deletions(-) diff --git a/xmodule/tests/test_vertical.py b/xmodule/tests/test_vertical.py index 223cf3889cc7..a3c596d253ac 100644 --- a/xmodule/tests/test_vertical.py +++ b/xmodule/tests/test_vertical.py @@ -14,12 +14,10 @@ import pytz from django.contrib.auth.models import AnonymousUser from django.test import override_settings -from edx_toggles.toggles.testutils import override_waffle_switch from fs.memoryfs import MemoryFS from openedx_filters import PipelineStep from openedx_filters.learning.filters import VerticalBlockChildRenderStarted, VerticalBlockRenderCompleted -from ..vertical_block import XBLOCK_SKILL_TAG_VERIFICATION_SWITCH from ..x_module import AUTHOR_VIEW, PUBLIC_VIEW, STUDENT_VIEW from . import prepare_block_runtime from .helpers import StubUserService @@ -362,7 +360,6 @@ def test_render_studio_view(self): }, }, ) - @override_waffle_switch(XBLOCK_SKILL_TAG_VERIFICATION_SWITCH, True) def test_vertical_block_child_render_started_filter_execution(self): """ Test the VerticalBlockChildRenderStarted filter's effects on student view. @@ -385,7 +382,6 @@ def test_vertical_block_child_render_started_filter_execution(self): }, }, ) - @override_waffle_switch(XBLOCK_SKILL_TAG_VERIFICATION_SWITCH, True) def test_vertical_block_child_render_is_skipped_on_filter_exception(self): """ Test VerticalBlockChildRenderStarted filter can be used to skip child blocks. @@ -409,7 +405,6 @@ def test_vertical_block_child_render_is_skipped_on_filter_exception(self): }, }, ) - @override_waffle_switch(XBLOCK_SKILL_TAG_VERIFICATION_SWITCH, True) def test_vertical_block_render_completed_filter_execution(self): """ Test the VerticalBlockRenderCompleted filter's execution. @@ -432,7 +427,6 @@ def test_vertical_block_render_completed_filter_execution(self): }, }, ) - @override_waffle_switch(XBLOCK_SKILL_TAG_VERIFICATION_SWITCH, True) def test_vertical_block_render_output_is_changed_on_filter_exception(self): """ Test VerticalBlockRenderCompleted filter can be used to prevent vertical block from rendering. diff --git a/xmodule/vertical_block.py b/xmodule/vertical_block.py index 10ff0a540870..2a10ae44497f 100644 --- a/xmodule/vertical_block.py +++ b/xmodule/vertical_block.py @@ -9,7 +9,6 @@ from functools import reduce import pytz -from edx_toggles.toggles import WaffleSwitch from lxml import etree from openedx_filters.learning.filters import VerticalBlockChildRenderStarted, VerticalBlockRenderCompleted from web_fragments.fragment import Fragment @@ -34,17 +33,6 @@ # HACK: This shouldn't be hard-coded to two types # OBSOLETE: This obsoletes 'type' CLASS_PRIORITY = ['video', 'problem'] -WAFFLE_NAMESPACE = 'xblocks' -# .. toggle_name: xblocks.xblock_skill_tag_verification' -# .. toggle_implementation: WaffleSwitch -# .. toggle_default: False -# .. toggle_description: Set to True to get learner verification of the skills associated with the xblock. -# .. toggle_use_cases: open_edx -# .. toggle_creation_date: 2023-10-04 -# .. toggle_target_removal_date: None -XBLOCK_SKILL_TAG_VERIFICATION_SWITCH = WaffleSwitch( # lint-amnesty, pylint: disable=toggle-missing-annotation - f'{WAFFLE_NAMESPACE}.xblock_skill_tag_verification', __name__ -) class VerticalFields: @@ -129,16 +117,15 @@ def _student_or_public_view(self, context, view): # lint-amnesty, pylint: disab child_block_context['wrap_xblock_data'] = { 'mark-completed-on-view-after-delay': complete_on_view_delay } - if XBLOCK_SKILL_TAG_VERIFICATION_SWITCH.is_enabled(): - try: - # .. filter_implemented_name: VerticalBlockChildRenderStarted - # .. filter_type: org.openedx.learning.vertical_block_child.render.started.v1 - child, child_block_context = VerticalBlockChildRenderStarted.run_filter( - block=child, context=child_block_context - ) - except VerticalBlockChildRenderStarted.PreventChildBlockRender as exc: - log.info("Skipping %s from vertical block. Reason: %s", child, exc.message) - continue + try: + # .. filter_implemented_name: VerticalBlockChildRenderStarted + # .. filter_type: org.openedx.learning.vertical_block_child.render.started.v1 + child, child_block_context = VerticalBlockChildRenderStarted.run_filter( + block=child, context=child_block_context + ) + except VerticalBlockChildRenderStarted.PreventChildBlockRender as exc: + log.info("Skipping %s from vertical block. Reason: %s", child, exc.message) + continue rendered_child = child.render(view, child_block_context) fragment.add_fragment_resources(rendered_child) @@ -180,16 +167,15 @@ def _student_or_public_view(self, context, view): # lint-amnesty, pylint: disab add_webpack_js_to_fragment(fragment, 'VerticalStudentView') fragment.initialize_js('VerticalStudentView') - if XBLOCK_SKILL_TAG_VERIFICATION_SWITCH.is_enabled(): - try: - # .. filter_implemented_name: VerticalBlockRenderCompleted - # .. filter_type: org.openedx.learning.vertical_block.render.completed.v1 - _, fragment, context, view = VerticalBlockRenderCompleted.run_filter( - block=self, fragment=fragment, context=context, view=view - ) - except VerticalBlockRenderCompleted.PreventVerticalBlockRender as exc: - log.info("VerticalBlock rendering stopped. Reason: %s", exc.message) - fragment.content = exc.message + try: + # .. filter_implemented_name: VerticalBlockRenderCompleted + # .. filter_type: org.openedx.learning.vertical_block.render.completed.v1 + _, fragment, context, view = VerticalBlockRenderCompleted.run_filter( + block=self, fragment=fragment, context=context, view=view + ) + except VerticalBlockRenderCompleted.PreventVerticalBlockRender as exc: + log.info("VerticalBlock rendering stopped. Reason: %s", exc.message) + fragment.content = exc.message return fragment From 3da1cdf01c5ee922f74a59382a30edfc36828f0b Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 19 Mar 2024 22:20:14 -0700 Subject: [PATCH 2/4] fix: handle paste of library content blocks correctly (#34274) (backport) --- cms/djangoapps/contentstore/helpers.py | 11 ++- .../views/tests/test_clipboard_paste.py | 86 ++++++++++++++++++- .../core/djangoapps/content_libraries/api.py | 12 ++- 3 files changed, 103 insertions(+), 6 deletions(-) diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index 0d66070b1120..2bfdd6574aad 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -17,6 +17,7 @@ from xmodule.contentstore.content import StaticContent from xmodule.contentstore.django import contentstore from xmodule.exceptions import NotFoundError +from xmodule.library_content_block import LibraryContentBlock from xmodule.modulestore.django import modulestore from xmodule.xml_block import XmlMixin @@ -336,8 +337,14 @@ def _import_xml_node_to_parent( new_xblock = store.update_item(temp_xblock, user_id, allow_not_found=True) parent_xblock.children.append(new_xblock.location) store.update_item(parent_xblock, user_id) - for child_node in child_nodes: - _import_xml_node_to_parent(child_node, new_xblock, store, user_id=user_id) + if isinstance(new_xblock, LibraryContentBlock): + # Special case handling for library content. If we need this for other blocks in the future, it can be made into + # an API, and we'd call new_block.studio_post_paste() instead of this code. + # In this case, we want to pull the children from the library and let library_tools assign their IDs. + new_xblock.tools.update_children(new_xblock, version=new_xblock.source_library_version) + else: + for child_node in child_nodes: + _import_xml_node_to_parent(child_node, new_xblock, store, user_id=user_id) return new_xblock diff --git a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py index a447799c2950..bdf22532fd5f 100644 --- a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py +++ b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py @@ -4,11 +4,15 @@ APIs. """ import ddt +from django.test import LiveServerTestCase from opaque_keys.edx.keys import UsageKey from rest_framework.test import APIClient -from xmodule.modulestore.django import contentstore +from xmodule.modulestore.django import contentstore, modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, upload_file_to_course -from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory, ToyCourseFactory +from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory, LibraryFactory, ToyCourseFactory + +from cms.djangoapps.contentstore.utils import reverse_usage_url +from cms.djangoapps.contentstore.tests.utils import AjaxEnabledTestClient CLIPBOARD_ENDPOINT = "/api/content-staging/v1/clipboard/" XBLOCK_ENDPOINT = "/xblock/" @@ -109,7 +113,7 @@ def test_copy_and_paste_component(self, block_args): """ Test copying a component (XBlock) from one course into another """ - source_course = CourseFactory.create(display_name='Destination Course') + source_course = CourseFactory.create(display_name='Source Course') source_block = BlockFactory.create(parent_location=source_course.location, **block_args) dest_course = CourseFactory.create(display_name='Destination Course') @@ -204,3 +208,79 @@ def test_paste_with_assets(self): source_pic2_hash = contentstore().find(source_course.id.make_asset_key("asset", "picture2.jpg")).content_digest dest_pic2_hash = contentstore().find(dest_course_key.make_asset_key("asset", "picture2.jpg")).content_digest assert source_pic2_hash != dest_pic2_hash # Because there was a conflict, this file was unchanged. + + +class ClipboardLibraryContentPasteTestCase(LiveServerTestCase, ModuleStoreTestCase): + """ + Test Clipboard Paste functionality with library content + """ + + def setUp(self): + """ + Set up a v2 Content Library and a library content block + """ + super().setUp() + self.client = AjaxEnabledTestClient() + self.client.login(username=self.user.username, password=self.user_password) + self.store = modulestore() + + def test_paste_library_content_block_v1(self): + """ + Same as the above test, but uses modulestore (v1) content library + """ + library = LibraryFactory.create() + data = { + 'parent_locator': str(library.location), + 'category': 'html', + 'display_name': 'HTML Content', + } + response = self.client.ajax_post(XBLOCK_ENDPOINT, data) + self.assertEqual(response.status_code, 200) + course = CourseFactory.create(display_name='Course') + orig_lc_block = BlockFactory.create( + parent=course, + category="library_content", + source_library_id=str(library.location.library_key), + display_name="LC Block", + publish_item=False, + ) + orig_lc_block.refresh_children() + orig_child = self.store.get_item(orig_lc_block.children[0]) + assert orig_child.display_name == "HTML Content" + # Copy a library content block that has children: + copy_response = self.client.post(CLIPBOARD_ENDPOINT, { + "usage_key": str(orig_lc_block.location) + }, format="json") + assert copy_response.status_code == 200 + + # Paste the Library content block: + paste_response = self.client.ajax_post(XBLOCK_ENDPOINT, { + "parent_locator": str(course.location), + "staged_content": "clipboard", + }) + assert paste_response.status_code == 200 + dest_lc_block_key = UsageKey.from_string(paste_response.json()["locator"]) + + # Get the ID of the new child: + dest_lc_block = self.store.get_item(dest_lc_block_key) + dest_child = self.store.get_item(dest_lc_block.children[0]) + assert dest_child.display_name == "HTML Content" + + # Importantly, the ID of the child must not changed when the library content is synced. + # Otherwise, user state saved against this child will be lost when it syncs. + dest_lc_block.refresh_children() + updated_dest_child = self.store.get_item(dest_lc_block.children[0]) + assert dest_child.location == updated_dest_child.location + + def _sync_lc_block_from_library(self, attr_name): + """ + Helper method to "sync" a Library Content Block by [re-]fetching its + children from the library. + """ + usage_key = getattr(self, attr_name).location + # It's easiest to do this via the REST API: + handler_url = reverse_usage_url('preview_handler', usage_key, kwargs={'handler': 'upgrade_and_sync'}) + response = self.client.post(handler_url) + assert response.status_code == 200 + # Now reload the block and make sure the child is in place + setattr(self, attr_name, self.store.get_item(usage_key)) # we must reload after upgrade_and_sync diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py index 737673b32e3f..d020a7635b97 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api.py @@ -91,7 +91,15 @@ from xblock.exceptions import XBlockNotFoundError from edx_rest_api_client.client import OAuthAPIClient from openedx.core.djangoapps.content_libraries import permissions -from openedx.core.djangoapps.content_libraries.constants import DRAFT_NAME, COMPLEX +# pylint: disable=unused-import +from openedx.core.djangoapps.content_libraries.constants import ( + ALL_RIGHTS_RESERVED, + CC_4_BY, + COMPLEX, + DRAFT_NAME, + PROBLEM, + VIDEO, +) from openedx.core.djangoapps.content_libraries.library_bundle import LibraryBundle from openedx.core.djangoapps.content_libraries.libraries_index import ContentLibraryIndexer, LibraryBlockIndexer from openedx.core.djangoapps.content_libraries.models import ( @@ -425,6 +433,8 @@ def create_library( allow_public_read: Allow anyone to view blocks (including source) in Studio? + library_type: Deprecated parameter, not really used. Set to COMPLEX. + Returns a ContentLibraryMetadata instance. """ assert isinstance(collection_uuid, UUID) From 7aae617836dd5d5e5069ebac10803daea26d5cd0 Mon Sep 17 00:00:00 2001 From: Stanislav Date: Wed, 20 Mar 2024 15:04:11 +0200 Subject: [PATCH 3/4] fix: Space around techers images on course about page (#34358) --- common/test/data/course_after_rename/about/overview.html | 4 ++-- common/test/data/course_before_rename/about/overview.html | 4 ++-- common/test/data/course_info_updates/about/overview.html | 4 ++-- common/test/data/manual-testing-complete/about/overview.html | 4 ++-- common/test/data/scoreable/about/overview.html | 4 ++-- xmodule/templates/about/overview.yaml | 4 ++-- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/common/test/data/course_after_rename/about/overview.html b/common/test/data/course_after_rename/about/overview.html index 0f9a54de9a4a..b20cf86c6d2f 100644 --- a/common/test/data/course_after_rename/about/overview.html +++ b/common/test/data/course_after_rename/about/overview.html @@ -14,7 +14,7 @@

Requirements

Course Staff

- Course Staff Image #1 + Course Staff Image #1

Staff Member #1

@@ -23,7 +23,7 @@

Staff Member #1

- Course Staff Image #2 + Course Staff Image #2

Staff Member #2

diff --git a/common/test/data/course_before_rename/about/overview.html b/common/test/data/course_before_rename/about/overview.html index 0f9a54de9a4a..b20cf86c6d2f 100644 --- a/common/test/data/course_before_rename/about/overview.html +++ b/common/test/data/course_before_rename/about/overview.html @@ -14,7 +14,7 @@

Requirements

Course Staff

- Course Staff Image #1 + Course Staff Image #1

Staff Member #1

@@ -23,7 +23,7 @@

Staff Member #1

- Course Staff Image #2 + Course Staff Image #2

Staff Member #2

diff --git a/common/test/data/course_info_updates/about/overview.html b/common/test/data/course_info_updates/about/overview.html index 6b4fe1a8636d..632157bfa99c 100644 --- a/common/test/data/course_info_updates/about/overview.html +++ b/common/test/data/course_info_updates/about/overview.html @@ -14,7 +14,7 @@

Prerequisites

Course Staff

- Course Staff Image #1 + Course Staff Image #1

Staff Member #1

@@ -23,7 +23,7 @@

Staff Member #1

- Course Staff Image #2 + Course Staff Image #2

Staff Member #2

diff --git a/common/test/data/manual-testing-complete/about/overview.html b/common/test/data/manual-testing-complete/about/overview.html index 6b4fe1a8636d..632157bfa99c 100644 --- a/common/test/data/manual-testing-complete/about/overview.html +++ b/common/test/data/manual-testing-complete/about/overview.html @@ -14,7 +14,7 @@

Prerequisites

Course Staff

- Course Staff Image #1 + Course Staff Image #1

Staff Member #1

@@ -23,7 +23,7 @@

Staff Member #1

- Course Staff Image #2 + Course Staff Image #2

Staff Member #2

diff --git a/common/test/data/scoreable/about/overview.html b/common/test/data/scoreable/about/overview.html index f829d3c8c177..d554b6a01843 100644 --- a/common/test/data/scoreable/about/overview.html +++ b/common/test/data/scoreable/about/overview.html @@ -14,7 +14,7 @@

Requirements

Course Staff

- Course Staff Image #1 + Course Staff Image #1

Staff Member #1

@@ -23,7 +23,7 @@

Staff Member #1

- Course Staff Image #2 + Course Staff Image #2

Staff Member #2

diff --git a/xmodule/templates/about/overview.yaml b/xmodule/templates/about/overview.yaml index 29ed75b96339..c5ddfcd97ee7 100644 --- a/xmodule/templates/about/overview.yaml +++ b/xmodule/templates/about/overview.yaml @@ -19,7 +19,7 @@ data: |

Course Staff

- Course Staff Image #1 + Course Staff Image #1

Staff Member #1

@@ -28,7 +28,7 @@ data: |
- Course Staff Image #2 + Course Staff Image #2

Staff Member #2

From e18418166b9794eff08441d55b2dd0dc1976012c Mon Sep 17 00:00:00 2001 From: Dmytro <98233552+DmytroAlipov@users.noreply.github.com> Date: Fri, 22 Mar 2024 01:00:20 +0100 Subject: [PATCH 4/4] fix: email templates caching (#34409) Authored-by: Dima Alipov --- .../core/djangoapps/theming/template_loaders.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/openedx/core/djangoapps/theming/template_loaders.py b/openedx/core/djangoapps/theming/template_loaders.py index 92326711d20a..760f1dd1adb8 100644 --- a/openedx/core/djangoapps/theming/template_loaders.py +++ b/openedx/core/djangoapps/theming/template_loaders.py @@ -27,13 +27,17 @@ class ThemeFilesystemLoader(FilesystemLoader): is_usable = True _accepts_engine_in_init = True - def __init__(self, engine, dirs=None): - if not dirs: - self.dirs = engine.dirs + def get_dirs(self): + """ + Override get_dirs method. + + Make the theme templates a priority, avoiding cashing templates for django ones. + """ + dirs = super().get_dirs() theme_dirs = self.get_theme_template_sources() - if isinstance(theme_dirs, list): - self.dirs = theme_dirs + self.dirs - super().__init__(engine, self.dirs) + if theme_dirs: + dirs = theme_dirs + dirs + return dirs @staticmethod def get_theme_template_sources():