From 536493f261a8fbaaafecfec2226f374677c80a49 Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Tue, 21 Nov 2023 00:39:35 -0300 Subject: [PATCH 01/15] refactor: inheritable studioeditableblock's callbacks for editing & duplication Solves https://github.com/openedx/edx-platform/issues/33715 Instead of applying duplicated logic after getattr, that logic is incorporated into inheritable methods of the StudioEditable block. Risks: - Assumes all cms blocks that are duplicated inherit from StudioEditableBlock. --- cms/djangoapps/contentstore/utils.py | 76 ++++--------------- cms/djangoapps/contentstore/views/block.py | 4 +- .../contentstore/views/component.py | 4 +- cms/djangoapps/contentstore/views/preview.py | 4 +- .../xblock_storage_handlers/view_handlers.py | 76 +++---------------- xmodule/library_content_block.py | 9 ++- xmodule/services.py | 48 ++++++++++++ xmodule/studio_editable.py | 70 ++++++++++------- xmodule/util/duplicate.py | 15 ++++ 9 files changed, 141 insertions(+), 165 deletions(-) create mode 100644 xmodule/util/duplicate.py diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 8e3f97eee98e..d354d012dd3f 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -15,7 +15,6 @@ from django.utils import translation from django.utils.translation import gettext as _ from help_tokens.core import HelpUrlExpert -from lti_consumer.models import CourseAllowPIISharingInLTIFlag from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.locator import LibraryLocator from openedx_events.content_authoring.data import DuplicatedXBlockData @@ -27,9 +26,7 @@ from cms.djangoapps.contentstore.toggles import exam_setting_view_enabled from common.djangoapps.course_action_state.models import CourseRerunUIStateManager from common.djangoapps.course_modes.models import CourseMode -from common.djangoapps.edxmako.services import MakoService from common.djangoapps.student import auth -from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access, STUDIO_EDIT_ROLES from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.roles import ( CourseInstructorRole, @@ -45,7 +42,6 @@ get_namespace_choices, generate_milestone_namespace ) -from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService from openedx.core import toggles as core_toggles from openedx.core.djangoapps.credit.api import get_credit_requirements, is_credit_course from openedx.core.djangoapps.discussions.config.waffle import ENABLE_PAGES_AND_RESOURCES_MICROFRONTEND @@ -79,12 +75,12 @@ use_tagging_taxonomy_list_page, ) from cms.djangoapps.models.settings.course_grading import CourseGradingModel -from xmodule.library_tools import LibraryToolsService from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.partitions.partitions_service import get_all_partitions_for_course # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.services import SettingsService, ConfigurationService, TeamsConfigurationService +from xmodule.partitions.partitions_service import get_all_partitions_for_course +from xmodule.services import load_services_for_studio +from xmodule.util.duplicate import handle_children_duplication # lint-amnesty, pylint: disable=wrong-import-order log = logging.getLogger(__name__) @@ -1055,23 +1051,17 @@ def duplicate_block( ) children_handled = False - - if hasattr(dest_block, 'studio_post_duplicate'): + if hasattr(dest_block, "studio_post_duplicate"): # Allow an XBlock to do anything fancy it may need to when duplicated from another block. - # These blocks may handle their own children or parenting if needed. Let them return booleans to - # let us know if we need to handle these or not. - load_services_for_studio(dest_block.runtime, user) - children_handled = dest_block.studio_post_duplicate(store, source_item) - - # Children are not automatically copied over (and not all xblocks have a 'children' attribute). - # Because DAGs are not fully supported, we need to actually duplicate each child as well. - if source_item.has_children and not shallow and not children_handled: - dest_block.children = dest_block.children or [] - for child in source_item.children: - dupe = duplicate_block(dest_block.location, child, user=user, is_child=True) - if dupe not in dest_block.children: # _duplicate_block may add the child for us. - dest_block.children.append(dupe) - store.update_item(dest_block, user.id) + load_services_for_studio(source_item.runtime, user) + children_handled = dest_block.studio_post_duplicate( + store, source_item, store, user, duplication_function=duplicate_block, shallow=shallow + ) + + if not children_handled: + handle_children_duplication( + store, source_item, store, user, duplication_function=duplicate_block, shallow=shallow + ) # pylint: disable=protected-access if 'detached' not in source_item.runtime.load_block_type(category)._class_tags: @@ -1173,25 +1163,6 @@ def gather_block_attributes(source_item, display_name=None, is_child=False): return duplicate_metadata, asides_to_create -def load_services_for_studio(runtime, user): - """ - Function to set some required services used for XBlock edits and studio_view. - (i.e. whenever we're not loading _prepare_runtime_for_preview.) This is required to make information - about the current user (especially permissions) available via services as needed. - """ - services = { - "user": DjangoXBlockUserService(user), - "studio_user_permissions": StudioPermissionsService(user), - "mako": MakoService(), - "settings": SettingsService(), - "lti-configuration": ConfigurationService(CourseAllowPIISharingInLTIFlag), - "teams_configuration": TeamsConfigurationService(), - "library_tools": LibraryToolsService(modulestore(), user.id) - } - - runtime._services.update(services) # lint-amnesty, pylint: disable=protected-access - - def update_course_details(request, course_key, payload, course_block): """ Utils is used to update course details. @@ -1666,24 +1637,3 @@ def get_course_videos_context(course_block, pagination_conf, course_key=None): # Cached state for transcript providers' credentials (org-specific) course_video_context['transcript_credentials'] = get_transcript_credentials_state_for_org(course.id.org) return course_video_context - - -class StudioPermissionsService: - """ - Service that can provide information about a user's permissions. - - Deprecated. To be replaced by a more general authorization service. - - Only used by LibraryContentBlock (and library_tools.py). - """ - - def __init__(self, user): - self._user = user - - def can_read(self, course_key): - """ Does the user have read access to the given course/library? """ - return has_studio_read_access(self._user, course_key) - - def can_write(self, course_key): - """ Does the user have read access to the given course/library? """ - return has_studio_write_access(self._user, course_key) diff --git a/cms/djangoapps/contentstore/views/block.py b/cms/djangoapps/contentstore/views/block.py index 91078ccd11c6..1ce1630fb768 100644 --- a/cms/djangoapps/contentstore/views/block.py +++ b/cms/djangoapps/contentstore/views/block.py @@ -27,7 +27,8 @@ ) from xmodule.modulestore.django import ( modulestore, -) # lint-amnesty, pylint: disable=wrong-import-order +) +from xmodule.services import load_services_for_studio # lint-amnesty, pylint: disable=wrong-import-order from xmodule.x_module import ( @@ -46,7 +47,6 @@ from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import ( handle_xblock, create_xblock_info, - load_services_for_studio, get_block_info, get_xblock, delete_orphans, diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index bafcdad35c71..f06aa8f5017c 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -34,14 +34,14 @@ from openedx.core.djangoapps.content_staging import api as content_staging_api from openedx.core.djangoapps.content_tagging.api import get_content_tags from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.modulestore.exceptions import ItemNotFoundError +from xmodule.services import load_services_for_studio # lint-amnesty, pylint: disable=wrong-import-order from ..toggles import use_new_unit_page from ..utils import get_lms_link_for_item, get_sibling_urls, reverse_course_url, get_unit_url from ..helpers import get_parent_xblock, is_unit, xblock_type_display_name from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import ( add_container_page_publishing_info, create_xblock_info, - load_services_for_studio, ) __all__ = [ diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index acab35471813..096de387157b 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -21,7 +21,7 @@ from xmodule.exceptions import NotFoundError, ProcessingError from xmodule.modulestore.django import XBlockI18nService, modulestore from xmodule.partitions.partitions_service import PartitionService -from xmodule.services import SettingsService, TeamsConfigurationService +from xmodule.services import SettingsService, StudioPermissionsService, TeamsConfigurationService from xmodule.studio_editable import has_author_view from xmodule.util.sandboxing import SandboxService from xmodule.util.builtin_assets import add_webpack_js_to_fragment @@ -45,7 +45,7 @@ wrap_xblock_aside ) -from ..utils import get_visibility_partition_info, StudioPermissionsService +from ..utils import get_visibility_partition_info from .access import get_user_role from .session_kv_store import SessionKeyValueStore diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index f82a6f599d11..10b2c85adcff 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -31,7 +31,6 @@ ) from edx_proctoring.exceptions import ProctoredExamNotFoundException from help_tokens.core import HelpUrlExpert -from lti_consumer.models import CourseAllowPIISharingInLTIFlag from opaque_keys.edx.locator import LibraryUsageLocator from pytz import UTC from xblock.core import XBlock @@ -41,7 +40,6 @@ from cms.djangoapps.contentstore.toggles import ENABLE_COPY_PASTE_UNITS, use_tagging_taxonomy_list_page from cms.djangoapps.models.settings.course_grading import CourseGradingModel from cms.lib.ai_aside_summary_config import AiAsideSummaryConfig -from common.djangoapps.edxmako.services import MakoService from common.djangoapps.static_replace import replace_static_urls from common.djangoapps.student.auth import ( has_studio_read_access, @@ -49,7 +47,6 @@ ) from common.djangoapps.util.date_utils import get_default_time_display from common.djangoapps.util.json_request import JsonResponse, expect_json -from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService from openedx.core.djangoapps.bookmarks import api as bookmarks_api from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE @@ -63,8 +60,9 @@ from xmodule.modulestore.draft_and_published import DIRECT_ONLY_CATEGORIES from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError from xmodule.modulestore.inheritance import own_metadata -from xmodule.services import ConfigurationService, SettingsService, TeamsConfigurationService +from xmodule.services import load_services_for_studio from xmodule.tabs import CourseTabList +from xmodule.util.duplicate import handle_children_duplication from ..utils import ( ancestor_has_staff_lock, @@ -296,46 +294,6 @@ def modify_xblock(usage_key, request): ) -class StudioPermissionsService: - """ - Service that can provide information about a user's permissions. - - Deprecated. To be replaced by a more general authorization service. - - Only used by LibraryContentBlock (and library_tools.py). - """ - - def __init__(self, user): - self._user = user - - def can_read(self, course_key): - """Does the user have read access to the given course/library?""" - return has_studio_read_access(self._user, course_key) - - def can_write(self, course_key): - """Does the user have read access to the given course/library?""" - return has_studio_write_access(self._user, course_key) - - -def load_services_for_studio(runtime, user): - """ - Function to set some required services used for XBlock edits and studio_view. - (i.e. whenever we're not loading _prepare_runtime_for_preview.) This is required to make information - about the current user (especially permissions) available via services as needed. - """ - services = { - "user": DjangoXBlockUserService(user), - "studio_user_permissions": StudioPermissionsService(user), - "mako": MakoService(), - "settings": SettingsService(), - "lti-configuration": ConfigurationService(CourseAllowPIISharingInLTIFlag), - "teams_configuration": TeamsConfigurationService(), - "library_tools": LibraryToolsService(modulestore(), user.id), - } - - runtime._services.update(services) # lint-amnesty, pylint: disable=protected-access - - def _update_with_callback(xblock, user, old_metadata=None, old_content=None): """ Updates the xblock in the modulestore. @@ -860,29 +818,17 @@ def _duplicate_block( ) children_handled = False - if hasattr(dest_block, "studio_post_duplicate"): # Allow an XBlock to do anything fancy it may need to when duplicated from another block. - # These blocks may handle their own children or parenting if needed. Let them return booleans to - # let us know if we need to handle these or not. - # TODO: Make this a proper method in the base class so we don't need getattr. - # See https://github.com/openedx/edx-platform/issues/33715 - load_services_for_studio(dest_block.runtime, user) - children_handled = dest_block.studio_post_duplicate(store, source_item) - - # Children are not automatically copied over (and not all xblocks have a 'children' attribute). - # Because DAGs are not fully supported, we need to actually duplicate each child as well. - if source_item.has_children and not children_handled: - dest_block.children = dest_block.children or [] - for child in source_item.children: - dupe = _duplicate_block( - dest_block.location, child, user=user, is_child=True - ) - if ( - dupe not in dest_block.children - ): # _duplicate_block may add the child for us. - dest_block.children.append(dupe) - store.update_item(dest_block, user.id) + load_services_for_studio(source_item.runtime, user) + children_handled = dest_block.studio_post_duplicate( + source_item, store, user, duplication_function=_duplicate_block, shallow=False + ) + + if not children_handled: + handle_children_duplication( + dest_block, source_item, store, user, duplication_function=_duplicate_block, shallow=False + ) # pylint: disable=protected-access if "detached" not in source_item.runtime.load_block_type(category)._class_tags: diff --git a/xmodule/library_content_block.py b/xmodule/library_content_block.py index e4c4af267a1f..e7542f19a873 100644 --- a/xmodule/library_content_block.py +++ b/xmodule/library_content_block.py @@ -8,6 +8,7 @@ import random from copy import copy from gettext import ngettext, gettext +from typing import Callable import bleach from django.conf import settings @@ -587,7 +588,11 @@ def children_are_syncing(self, request, suffix=''): # pylint: disable=unused-ar is_updating = False return Response(json.dumps(is_updating)) - def studio_post_duplicate(self, store, source_block): + def studio_post_duplicate( + self, + source_item, + *_, + ) -> None: # pylint: disable=unused-argument """ Used by the studio after basic duplication of a source block. We handle the children ourselves, because we have to properly reference the library upstream and set the overrides. @@ -595,7 +600,7 @@ def studio_post_duplicate(self, store, source_block): Otherwise we'll end up losing data on the next refresh. """ self._validate_sync_permissions() - self.get_tools(to_read_library_content=True).trigger_duplication(source_block=source_block, dest_block=self) + self.get_tools(to_read_library_content=True).trigger_duplication(source_block=source_item, dest_block=self) return True # Children have been handled. def _validate_library_version(self, validation, lib_tools, version, library_key): diff --git a/xmodule/services.py b/xmodule/services.py index 25c680a9653a..4835de84d44f 100644 --- a/xmodule/services.py +++ b/xmodule/services.py @@ -6,15 +6,23 @@ import inspect import logging from functools import partial +from common.djangoapps.edxmako.services import MakoService +from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService from config_models.models import ConfigurationModel from django.conf import settings from eventtracking import tracker from edx_when.field_data import DateLookupFieldData +from lti_consumer.models import CourseAllowPIISharingInLTIFlag from xblock.reference.plugins import Service from xblock.runtime import KvsFieldData +import xmodule from common.djangoapps.track import contexts +from common.djangoapps.student.auth import ( + has_studio_read_access, + has_studio_write_access, +) from lms.djangoapps.courseware.masquerade import is_masquerading_as_specific_student from xmodule.modulestore.django import modulestore @@ -308,3 +316,43 @@ def _handle_deprecated_progress_event(self, block, event): # in order to avoid duplicate work and possibly conflicting semantics. if not getattr(block, 'has_custom_completion', False): self.completion_service.submit_completion(block.scope_ids.usage_id, 1.0) + + +def load_services_for_studio(runtime, user): + """ + Function to set some required services used for XBlock edits and studio_view. + (i.e. whenever we're not loading _prepare_runtime_for_preview.) This is required to make information + about the current user (especially permissions) available via services as needed. + """ + services = { + "user": DjangoXBlockUserService(user), + "studio_user_permissions": StudioPermissionsService(user), + "mako": MakoService(), + "settings": SettingsService(), + "lti-configuration": ConfigurationService(CourseAllowPIISharingInLTIFlag), + "teams_configuration": TeamsConfigurationService(), + "library_tools": xmodule.library_tools.LibraryToolsService(modulestore(), user.id), + } + + runtime._services.update(services) # lint-amnesty, pylint: disable=protected-access + + +class StudioPermissionsService: + """ + Service that can provide information about a user's permissions. + + Deprecated. To be replaced by a more general authorization service. + + Only used by LibraryContentBlock (and library_tools.py). + """ + + def __init__(self, user): + self._user = user + + def can_read(self, course_key): + """Does the user have read access to the given course/library?""" + return has_studio_read_access(self._user, course_key) + + def can_write(self, course_key): + """Does the user have read access to the given course/library?""" + return has_studio_write_access(self._user, course_key) diff --git a/xmodule/studio_editable.py b/xmodule/studio_editable.py index 332025619eb4..08f91d7d3dac 100644 --- a/xmodule/studio_editable.py +++ b/xmodule/studio_editable.py @@ -1,7 +1,11 @@ """ Mixin to support editing in Studio. """ +from typing import Callable + from xblock.core import XBlock, XBlockMixin +from xmodule.util.duplicate import handle_children_duplication + from xmodule.x_module import AUTHOR_VIEW, STUDENT_VIEW @@ -49,35 +53,43 @@ def get_preview_view_name(block): """ return AUTHOR_VIEW if has_author_view(block) else STUDENT_VIEW - # Some parts of the code use getattr to dynamically check for the following three methods on subclasses. - # We'd like to refactor so that we can actually declare them here as overridable methods. - # For now, we leave them here as documentation. - # See https://github.com/openedx/edx-platform/issues/33715. - # - # def editor_saved(self, old_metadata, old_content) -> None: # pylint: disable=unused-argument - # """ - # Called right *before* the block is written to the DB. Can be used, e.g., to modify fields before saving. - # - # By default, is a no-op. Can be overriden in subclasses. - # """ - # - # def post_editor_saved(self, old_metadata, old_content) -> None: # pylint: disable=unused-argument - # """ - # Called right *after* the block is written to the DB. Can be used, e.g., to spin up followup tasks. - # - # By default, is a no-op. Can be overriden in subclasses. - # """ - # - # def studio_post_duplicate(self, dest_block) -> bool: # pylint: disable=unused-argument - # """ - # Called when a the block is duplicated. Can be used, e.g., for special handling of child duplication. - # - # Returns 'True' if children have been handled and thus shouldn't be handled by the standard - # duplication logic. - # - # By default, is a no-op. Can be overriden in subclasses. - # """ - # return False + def editor_saved(self, old_metadata, old_content) -> None: # pylint: disable=unused-argument + """ + Called right *before* the block is written to the DB. Can be used, e.g., to modify fields before saving. + + By default, is a no-op. Can be overriden in subclasses. + """ + + def post_editor_saved(self, old_metadata, old_content) -> None: # pylint: disable=unused-argument + """ + Called right *after* the block is written to the DB. Can be used, e.g., to spin up followup tasks. + + By default, is a no-op. Can be overriden in subclasses. + """ + + def studio_post_duplicate( + self, + source_item, + store, + user, + duplication_function: Callable[..., None], + shallow: bool, + ) -> None: # pylint: disable=unused-argument + """ + Called when a the block is duplicated. Can be used, e.g., for special handling of child duplication. + + Returns 'True' if children have been handled and thus shouldn't be handled by the standard + duplication logic. + + By default, implements standard duplication logic. + """ + self.handle_children_duplication(source_item, store, user, duplication_function, shallow) + return True + + def handle_children_duplication( + self, source_item, store, user, duplication_function: Callable[..., None], shallow: bool + ): + handle_children_duplication(self, source_item, store, user, duplication_function, shallow) def has_author_view(block): diff --git a/xmodule/util/duplicate.py b/xmodule/util/duplicate.py new file mode 100644 index 000000000000..249a44044344 --- /dev/null +++ b/xmodule/util/duplicate.py @@ -0,0 +1,15 @@ +from typing import Callable + + +def handle_children_duplication( + xblock, source_item, store, user, duplication_function: Callable[..., None], shallow: bool +): + if not source_item.has_children or shallow: + return + + xblock.children = xblock.children or [] + for child in source_item.children: + dupe = duplication_function(xblock.location, child, user=user, is_child=True) + if dupe not in xblock.children: # duplicate_fun may add the child for us. + xblock.children.append(dupe) + store.update_item(xblock, user.id) From 5901e25eaf97349006166c550c23d3de7e1f7d0a Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Fri, 24 Nov 2023 21:44:55 -0300 Subject: [PATCH 02/15] refactor: remove hasattr and add editable studio to cms xblock modules --- cms/djangoapps/contentstore/utils.py | 11 +++++------ .../xblock_storage_handlers/view_handlers.py | 11 +++++------ cms/envs/common.py | 2 ++ 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index d354d012dd3f..885e7848f04f 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -1051,12 +1051,11 @@ def duplicate_block( ) children_handled = False - if hasattr(dest_block, "studio_post_duplicate"): - # Allow an XBlock to do anything fancy it may need to when duplicated from another block. - load_services_for_studio(source_item.runtime, user) - children_handled = dest_block.studio_post_duplicate( - store, source_item, store, user, duplication_function=duplicate_block, shallow=shallow - ) + # Allow an XBlock to do anything fancy it may need to when duplicated from another block. + load_services_for_studio(source_item.runtime, user) + children_handled = dest_block.studio_post_duplicate( + store, source_item, store, user, duplication_function=duplicate_block, shallow=shallow + ) if not children_handled: handle_children_duplication( diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index 10b2c85adcff..2896318ec7a5 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -818,12 +818,11 @@ def _duplicate_block( ) children_handled = False - if hasattr(dest_block, "studio_post_duplicate"): - # Allow an XBlock to do anything fancy it may need to when duplicated from another block. - load_services_for_studio(source_item.runtime, user) - children_handled = dest_block.studio_post_duplicate( - source_item, store, user, duplication_function=_duplicate_block, shallow=False - ) + # Allow an XBlock to do anything fancy it may need to when duplicated from another block. + load_services_for_studio(source_item.runtime, user) + children_handled = dest_block.studio_post_duplicate( + source_item, store, user, duplication_function=_duplicate_block, shallow=False + ) if not children_handled: handle_children_duplication( diff --git a/cms/envs/common.py b/cms/envs/common.py index 11afd870a016..7525e5974a81 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -959,6 +959,7 @@ # Import after sys.path fixup from xmodule.modulestore.inheritance import InheritanceMixin from xmodule.x_module import XModuleMixin +from xmodule.studio_editable import StudioEditableBlock # These are the Mixins that should be added to every XBlock. # This should be moved into an XBlock Runtime/Application object @@ -969,6 +970,7 @@ XModuleMixin, EditInfoMixin, AuthoringMixin, + StudioEditableBlock, ) XBLOCK_EXTRA_MIXINS = () From 5c2dabbb94d54256edde3557940f9ea01fa00f79 Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Sat, 25 Nov 2023 00:51:29 -0300 Subject: [PATCH 03/15] fix: tests --- cms/djangoapps/contentstore/utils.py | 6 +++--- xmodule/library_content_block.py | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 885e7848f04f..4a3003d9615b 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -1054,12 +1054,12 @@ def duplicate_block( # Allow an XBlock to do anything fancy it may need to when duplicated from another block. load_services_for_studio(source_item.runtime, user) children_handled = dest_block.studio_post_duplicate( - store, source_item, store, user, duplication_function=duplicate_block, shallow=shallow + source_item, store, user, duplication_function=duplicate_block, shallow=shallow ) if not children_handled: handle_children_duplication( - store, source_item, store, user, duplication_function=duplicate_block, shallow=shallow + dest_block, store, source_item, store, user, duplication_function=duplicate_block, shallow=shallow ) # pylint: disable=protected-access @@ -1347,7 +1347,7 @@ def get_course_team(auth_user, course_key, user_perms): 'context_course': course_block, 'show_transfer_ownership_hint': auth_user in instructors and len(instructors) == 1, 'users': formatted_users, - 'allow_actions': bool(user_perms & STUDIO_EDIT_ROLES), + 'allow_actions': bool(user_perms & auth.STUDIO_EDIT_ROLES), } return course_team_context diff --git a/xmodule/library_content_block.py b/xmodule/library_content_block.py index e7542f19a873..a40656f58695 100644 --- a/xmodule/library_content_block.py +++ b/xmodule/library_content_block.py @@ -592,6 +592,7 @@ def studio_post_duplicate( self, source_item, *_, + **__, ) -> None: # pylint: disable=unused-argument """ Used by the studio after basic duplication of a source block. We handle the children From 3e9bb60f84670a7a7236de56a88e80ea6e26d35b Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Sat, 25 Nov 2023 01:06:24 -0300 Subject: [PATCH 04/15] fix: library preview --- xmodule/conditional_block.py | 1 + xmodule/studio_editable.py | 5 ++--- xmodule/vertical_block.py | 2 ++ 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/xmodule/conditional_block.py b/xmodule/conditional_block.py index 78a5b8c1c1c9..0a0690e15841 100644 --- a/xmodule/conditional_block.py +++ b/xmodule/conditional_block.py @@ -83,6 +83,7 @@ class ConditionalBlock( And my_property/my_method will be called for required blocks. """ + has_author_view = True display_name = String( display_name=_("Display Name"), diff --git a/xmodule/studio_editable.py b/xmodule/studio_editable.py index 08f91d7d3dac..e290ba11ce3e 100644 --- a/xmodule/studio_editable.py +++ b/xmodule/studio_editable.py @@ -16,7 +16,6 @@ class StudioEditableBlock(XBlockMixin): This class is only intended to be used with an XBlock! """ - has_author_view = True def render_children(self, context, fragment, can_reorder=False, can_add=False): """ @@ -53,14 +52,14 @@ def get_preview_view_name(block): """ return AUTHOR_VIEW if has_author_view(block) else STUDENT_VIEW - def editor_saved(self, old_metadata, old_content) -> None: # pylint: disable=unused-argument + def editor_saved(self, user, old_metadata, old_content) -> None: # pylint: disable=unused-argument """ Called right *before* the block is written to the DB. Can be used, e.g., to modify fields before saving. By default, is a no-op. Can be overriden in subclasses. """ - def post_editor_saved(self, old_metadata, old_content) -> None: # pylint: disable=unused-argument + def post_editor_saved(self, user, old_metadata, old_content) -> None: # pylint: disable=unused-argument """ Called right *after* the block is written to the DB. Can be used, e.g., to spin up followup tasks. diff --git a/xmodule/vertical_block.py b/xmodule/vertical_block.py index 10ff0a540870..a76f0cc16f2f 100644 --- a/xmodule/vertical_block.py +++ b/xmodule/vertical_block.py @@ -76,6 +76,8 @@ class VerticalBlock( Layout XBlock for rendering subblocks vertically. """ + has_author_view = True + resources_dir = 'assets/vertical' mako_template = 'widgets/sequence-edit.html' From 42c2037f17a6771ef638cf9dad0550ad37bfeabd Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Sat, 25 Nov 2023 01:13:10 -0300 Subject: [PATCH 05/15] style: ran darker --- xmodule/conditional_block.py | 1 + 1 file changed, 1 insertion(+) diff --git a/xmodule/conditional_block.py b/xmodule/conditional_block.py index 0a0690e15841..97ea2366c279 100644 --- a/xmodule/conditional_block.py +++ b/xmodule/conditional_block.py @@ -83,6 +83,7 @@ class ConditionalBlock( And my_property/my_method will be called for required blocks. """ + has_author_view = True display_name = String( From b4a4c96059b341d9835a996ecf1e9b388e8d9323 Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Sat, 25 Nov 2023 01:20:52 -0300 Subject: [PATCH 06/15] style: pylint fixes --- cms/djangoapps/contentstore/utils.py | 7 ++++++- .../contentstore/xblock_storage_handlers/view_handlers.py | 1 - xmodule/library_content_block.py | 1 - xmodule/util/duplicate.py | 4 ++++ 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 4a3003d9615b..f03e3daa7a36 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -1059,7 +1059,12 @@ def duplicate_block( if not children_handled: handle_children_duplication( - dest_block, store, source_item, store, user, duplication_function=duplicate_block, shallow=shallow + dest_block, + source_item, + store, + user, + duplication_function=duplicate_block, + shallow=shallow, ) # pylint: disable=protected-access diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index 2896318ec7a5..b796c5e646c4 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -54,7 +54,6 @@ from openedx.core.lib.cache_utils import request_cached from openedx.core.toggles import ENTRANCE_EXAMS from xmodule.course_block import DEFAULT_START_DATE -from xmodule.library_tools import LibraryToolsService from xmodule.modulestore import EdxJSONEncoder, ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.draft_and_published import DIRECT_ONLY_CATEGORIES diff --git a/xmodule/library_content_block.py b/xmodule/library_content_block.py index a40656f58695..eb9e6ba695c9 100644 --- a/xmodule/library_content_block.py +++ b/xmodule/library_content_block.py @@ -8,7 +8,6 @@ import random from copy import copy from gettext import ngettext, gettext -from typing import Callable import bleach from django.conf import settings diff --git a/xmodule/util/duplicate.py b/xmodule/util/duplicate.py index 249a44044344..dbbbdf7f6531 100644 --- a/xmodule/util/duplicate.py +++ b/xmodule/util/duplicate.py @@ -1,9 +1,13 @@ +""" +Utility to aid xblock duplication +""" from typing import Callable def handle_children_duplication( xblock, source_item, store, user, duplication_function: Callable[..., None], shallow: bool ): + """Implements default behaviour to handle children duplication.""" if not source_item.has_children or shallow: return From aaa26bfacdf7545862559dafde3968432b58c42f Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Wed, 29 Nov 2023 21:44:35 -0300 Subject: [PATCH 07/15] style: use *_args and **_kwargs --- xmodule/library_content_block.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/xmodule/library_content_block.py b/xmodule/library_content_block.py index eb9e6ba695c9..081dbb657871 100644 --- a/xmodule/library_content_block.py +++ b/xmodule/library_content_block.py @@ -590,9 +590,9 @@ def children_are_syncing(self, request, suffix=''): # pylint: disable=unused-ar def studio_post_duplicate( self, source_item, - *_, - **__, - ) -> None: # pylint: disable=unused-argument + *_args, + **__kwargs, + ) -> None: """ Used by the studio after basic duplication of a source block. We handle the children ourselves, because we have to properly reference the library upstream and set the overrides. @@ -601,7 +601,6 @@ def studio_post_duplicate( """ self._validate_sync_permissions() self.get_tools(to_read_library_content=True).trigger_duplication(source_block=source_item, dest_block=self) - return True # Children have been handled. def _validate_library_version(self, validation, lib_tools, version, library_key): """ From 116b013bb3b8f5a7313780a7430a2897639cf248 Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Wed, 29 Nov 2023 21:59:00 -0300 Subject: [PATCH 08/15] refactor: move load_services_for_studio back into --- cms/djangoapps/contentstore/utils.py | 65 ++++++++++++++----- cms/djangoapps/contentstore/views/block.py | 2 +- .../contentstore/views/component.py | 3 +- .../xblock_storage_handlers/view_handlers.py | 2 +- xmodule/services.py | 40 ------------ xmodule/studio_editable.py | 9 +-- 6 files changed, 53 insertions(+), 68 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index f03e3daa7a36..83fa7c8c40db 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -15,6 +15,7 @@ from django.utils import translation from django.utils.translation import gettext as _ from help_tokens.core import HelpUrlExpert +from lti_consumer.models import CourseAllowPIISharingInLTIFlag from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.locator import LibraryLocator from openedx_events.content_authoring.data import DuplicatedXBlockData @@ -26,7 +27,9 @@ from cms.djangoapps.contentstore.toggles import exam_setting_view_enabled from common.djangoapps.course_action_state.models import CourseRerunUIStateManager from common.djangoapps.course_modes.models import CourseMode +from common.djangoapps.edxmako.services import MakoService from common.djangoapps.student import auth +from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access, STUDIO_EDIT_ROLES from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.roles import ( CourseInstructorRole, @@ -42,6 +45,7 @@ get_namespace_choices, generate_milestone_namespace ) +from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService from openedx.core import toggles as core_toggles from openedx.core.djangoapps.credit.api import get_credit_requirements, is_credit_course from openedx.core.djangoapps.discussions.config.waffle import ENABLE_PAGES_AND_RESOURCES_MICROFRONTEND @@ -75,12 +79,12 @@ use_tagging_taxonomy_list_page, ) from cms.djangoapps.models.settings.course_grading import CourseGradingModel +from xmodule.library_tools import LibraryToolsService from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.partitions.partitions_service import get_all_partitions_for_course -from xmodule.services import load_services_for_studio -from xmodule.util.duplicate import handle_children_duplication # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.partitions.partitions_service import get_all_partitions_for_course # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.services import SettingsService, ConfigurationService, TeamsConfigurationService log = logging.getLogger(__name__) @@ -1050,23 +1054,12 @@ def duplicate_block( asides=asides_to_create ) - children_handled = False # Allow an XBlock to do anything fancy it may need to when duplicated from another block. load_services_for_studio(source_item.runtime, user) - children_handled = dest_block.studio_post_duplicate( + dest_block.studio_post_duplicate( source_item, store, user, duplication_function=duplicate_block, shallow=shallow ) - if not children_handled: - handle_children_duplication( - dest_block, - source_item, - store, - user, - duplication_function=duplicate_block, - shallow=shallow, - ) - # pylint: disable=protected-access if 'detached' not in source_item.runtime.load_block_type(category)._class_tags: parent = store.get_item(parent_usage_key) @@ -1167,6 +1160,25 @@ def gather_block_attributes(source_item, display_name=None, is_child=False): return duplicate_metadata, asides_to_create +def load_services_for_studio(runtime, user): + """ + Function to set some required services used for XBlock edits and studio_view. + (i.e. whenever we're not loading _prepare_runtime_for_preview.) This is required to make information + about the current user (especially permissions) available via services as needed. + """ + services = { + "user": DjangoXBlockUserService(user), + "studio_user_permissions": StudioPermissionsService(user), + "mako": MakoService(), + "settings": SettingsService(), + "lti-configuration": ConfigurationService(CourseAllowPIISharingInLTIFlag), + "teams_configuration": TeamsConfigurationService(), + "library_tools": LibraryToolsService(modulestore(), user.id) + } + + runtime._services.update(services) # lint-amnesty, pylint: disable=protected-access + + def update_course_details(request, course_key, payload, course_block): """ Utils is used to update course details. @@ -1352,7 +1364,7 @@ def get_course_team(auth_user, course_key, user_perms): 'context_course': course_block, 'show_transfer_ownership_hint': auth_user in instructors and len(instructors) == 1, 'users': formatted_users, - 'allow_actions': bool(user_perms & auth.STUDIO_EDIT_ROLES), + 'allow_actions': bool(user_perms & STUDIO_EDIT_ROLES), } return course_team_context @@ -1641,3 +1653,24 @@ def get_course_videos_context(course_block, pagination_conf, course_key=None): # Cached state for transcript providers' credentials (org-specific) course_video_context['transcript_credentials'] = get_transcript_credentials_state_for_org(course.id.org) return course_video_context + + +class StudioPermissionsService: + """ + Service that can provide information about a user's permissions. + + Deprecated. To be replaced by a more general authorization service. + + Only used by LibraryContentBlock (and library_tools.py). + """ + + def __init__(self, user): + self._user = user + + def can_read(self, course_key): + """ Does the user have read access to the given course/library? """ + return has_studio_read_access(self._user, course_key) + + def can_write(self, course_key): + """ Does the user have read access to the given course/library? """ + return has_studio_write_access(self._user, course_key) diff --git a/cms/djangoapps/contentstore/views/block.py b/cms/djangoapps/contentstore/views/block.py index 1ce1630fb768..b7b75d23b227 100644 --- a/cms/djangoapps/contentstore/views/block.py +++ b/cms/djangoapps/contentstore/views/block.py @@ -9,6 +9,7 @@ from django.http import Http404, HttpResponse from django.utils.translation import gettext as _ from django.views.decorators.http import require_http_methods +from cms.djangoapps.contentstore.utils import load_services_for_studio from opaque_keys.edx.keys import CourseKey from web_fragments.fragment import Fragment @@ -28,7 +29,6 @@ from xmodule.modulestore.django import ( modulestore, ) -from xmodule.services import load_services_for_studio # lint-amnesty, pylint: disable=wrong-import-order from xmodule.x_module import ( diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index f06aa8f5017c..b0e270c4bbc5 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -35,9 +35,8 @@ from openedx.core.djangoapps.content_tagging.api import get_content_tags from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.exceptions import ItemNotFoundError -from xmodule.services import load_services_for_studio # lint-amnesty, pylint: disable=wrong-import-order from ..toggles import use_new_unit_page -from ..utils import get_lms_link_for_item, get_sibling_urls, reverse_course_url, get_unit_url +from ..utils import get_lms_link_for_item, get_sibling_urls, load_services_for_studio, reverse_course_url, get_unit_url from ..helpers import get_parent_xblock, is_unit, xblock_type_display_name from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import ( add_container_page_publishing_info, diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index b796c5e646c4..d6e68d714998 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -59,7 +59,6 @@ from xmodule.modulestore.draft_and_published import DIRECT_ONLY_CATEGORIES from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError from xmodule.modulestore.inheritance import own_metadata -from xmodule.services import load_services_for_studio from xmodule.tabs import CourseTabList from xmodule.util.duplicate import handle_children_duplication @@ -74,6 +73,7 @@ is_currently_visible_to_students, is_self_paced, get_taxonomy_tags_widget_url, + load_services_for_studio, ) from .create_xblock import create_xblock diff --git a/xmodule/services.py b/xmodule/services.py index 4835de84d44f..48a79c85f2b2 100644 --- a/xmodule/services.py +++ b/xmodule/services.py @@ -316,43 +316,3 @@ def _handle_deprecated_progress_event(self, block, event): # in order to avoid duplicate work and possibly conflicting semantics. if not getattr(block, 'has_custom_completion', False): self.completion_service.submit_completion(block.scope_ids.usage_id, 1.0) - - -def load_services_for_studio(runtime, user): - """ - Function to set some required services used for XBlock edits and studio_view. - (i.e. whenever we're not loading _prepare_runtime_for_preview.) This is required to make information - about the current user (especially permissions) available via services as needed. - """ - services = { - "user": DjangoXBlockUserService(user), - "studio_user_permissions": StudioPermissionsService(user), - "mako": MakoService(), - "settings": SettingsService(), - "lti-configuration": ConfigurationService(CourseAllowPIISharingInLTIFlag), - "teams_configuration": TeamsConfigurationService(), - "library_tools": xmodule.library_tools.LibraryToolsService(modulestore(), user.id), - } - - runtime._services.update(services) # lint-amnesty, pylint: disable=protected-access - - -class StudioPermissionsService: - """ - Service that can provide information about a user's permissions. - - Deprecated. To be replaced by a more general authorization service. - - Only used by LibraryContentBlock (and library_tools.py). - """ - - def __init__(self, user): - self._user = user - - def can_read(self, course_key): - """Does the user have read access to the given course/library?""" - return has_studio_read_access(self._user, course_key) - - def can_write(self, course_key): - """Does the user have read access to the given course/library?""" - return has_studio_write_access(self._user, course_key) diff --git a/xmodule/studio_editable.py b/xmodule/studio_editable.py index e290ba11ce3e..df48fd8e9e2f 100644 --- a/xmodule/studio_editable.py +++ b/xmodule/studio_editable.py @@ -77,17 +77,10 @@ def studio_post_duplicate( """ Called when a the block is duplicated. Can be used, e.g., for special handling of child duplication. - Returns 'True' if children have been handled and thus shouldn't be handled by the standard - duplication logic. + Children must always be handled. In case of inheritance it can be done by running this method with super(). By default, implements standard duplication logic. """ - self.handle_children_duplication(source_item, store, user, duplication_function, shallow) - return True - - def handle_children_duplication( - self, source_item, store, user, duplication_function: Callable[..., None], shallow: bool - ): handle_children_duplication(self, source_item, store, user, duplication_function, shallow) From c5cb3f88e1de7aa3a497990d37d08d54254c87b2 Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Wed, 29 Nov 2023 23:23:50 -0300 Subject: [PATCH 09/15] refactor: move editor_saved, post_editor_saved, and studio_post_duplicate to authoringmixin --- cms/djangoapps/contentstore/views/preview.py | 4 +- .../xblock_storage_handlers/view_handlers.py | 11 +---- cms/envs/common.py | 2 - cms/lib/xblock/authoring_mixin.py | 40 +++++++++++++++++++ xmodule/conditional_block.py | 2 - xmodule/studio_editable.py | 33 +-------------- xmodule/util/duplicate.py | 19 --------- xmodule/vertical_block.py | 2 - 8 files changed, 45 insertions(+), 68 deletions(-) delete mode 100644 xmodule/util/duplicate.py diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 096de387157b..5b9632674343 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -21,7 +21,7 @@ from xmodule.exceptions import NotFoundError, ProcessingError from xmodule.modulestore.django import XBlockI18nService, modulestore from xmodule.partitions.partitions_service import PartitionService -from xmodule.services import SettingsService, StudioPermissionsService, TeamsConfigurationService +from xmodule.services import SettingsService, TeamsConfigurationService from xmodule.studio_editable import has_author_view from xmodule.util.sandboxing import SandboxService from xmodule.util.builtin_assets import add_webpack_js_to_fragment @@ -45,7 +45,7 @@ wrap_xblock_aside ) -from ..utils import get_visibility_partition_info +from ..utils import StudioPermissionsService, get_visibility_partition_info from .access import get_user_role from .session_kv_store import SessionKeyValueStore diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index d6e68d714998..c7e080fa7d10 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -60,7 +60,6 @@ from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError from xmodule.modulestore.inheritance import own_metadata from xmodule.tabs import CourseTabList -from xmodule.util.duplicate import handle_children_duplication from ..utils import ( ancestor_has_staff_lock, @@ -816,17 +815,9 @@ def _duplicate_block( asides=asides_to_create, ) - children_handled = False # Allow an XBlock to do anything fancy it may need to when duplicated from another block. load_services_for_studio(source_item.runtime, user) - children_handled = dest_block.studio_post_duplicate( - source_item, store, user, duplication_function=_duplicate_block, shallow=False - ) - - if not children_handled: - handle_children_duplication( - dest_block, source_item, store, user, duplication_function=_duplicate_block, shallow=False - ) + dest_block.studio_post_duplicate(source_item, store, user, duplication_function=_duplicate_block, shallow=False) # pylint: disable=protected-access if "detached" not in source_item.runtime.load_block_type(category)._class_tags: diff --git a/cms/envs/common.py b/cms/envs/common.py index 7525e5974a81..11afd870a016 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -959,7 +959,6 @@ # Import after sys.path fixup from xmodule.modulestore.inheritance import InheritanceMixin from xmodule.x_module import XModuleMixin -from xmodule.studio_editable import StudioEditableBlock # These are the Mixins that should be added to every XBlock. # This should be moved into an XBlock Runtime/Application object @@ -970,7 +969,6 @@ XModuleMixin, EditInfoMixin, AuthoringMixin, - StudioEditableBlock, ) XBLOCK_EXTRA_MIXINS = () diff --git a/cms/lib/xblock/authoring_mixin.py b/cms/lib/xblock/authoring_mixin.py index a3d3b3298cea..c2e336c44a3f 100644 --- a/cms/lib/xblock/authoring_mixin.py +++ b/cms/lib/xblock/authoring_mixin.py @@ -4,6 +4,7 @@ import logging +from typing import Callable from django.conf import settings from web_fragments.fragment import Fragment @@ -51,3 +52,42 @@ def visibility_view(self, _context=None): scope=Scope.settings, enforce_type=True, ) + + def editor_saved(self, user, old_metadata, old_content) -> None: # pylint: disable=unused-argument + """ + Called right *before* the block is written to the DB. Can be used, e.g., to modify fields before saving. + + By default, is a no-op. Can be overriden in subclasses. + """ + + def post_editor_saved(self, user, old_metadata, old_content) -> None: # pylint: disable=unused-argument + """ + Called right *after* the block is written to the DB. Can be used, e.g., to spin up followup tasks. + + By default, is a no-op. Can be overriden in subclasses. + """ + + def studio_post_duplicate( + self, + source_item, + store, + user, + duplication_function: Callable[..., None], + shallow: bool, + ) -> None: # pylint: disable=unused-argument + """ + Called when a the block is duplicated. Can be used, e.g., for special handling of child duplication. + + Children must always be handled. In case of inheritance it can be done by running this method with super(). + + By default, implements standard duplication logic. + """ + if not source_item.has_children or shallow: + return + + self.children = self.children or [] + for child in source_item.children: + dupe = duplication_function(self.location, child, user=user, is_child=True) + if dupe not in self.children: # duplicate_fun may add the child for us. + self.children.append(dupe) + store.update_item(self, user.id) diff --git a/xmodule/conditional_block.py b/xmodule/conditional_block.py index 97ea2366c279..78a5b8c1c1c9 100644 --- a/xmodule/conditional_block.py +++ b/xmodule/conditional_block.py @@ -84,8 +84,6 @@ class ConditionalBlock( """ - has_author_view = True - display_name = String( display_name=_("Display Name"), help=_("The display name for this component."), diff --git a/xmodule/studio_editable.py b/xmodule/studio_editable.py index df48fd8e9e2f..43fe7c05aa77 100644 --- a/xmodule/studio_editable.py +++ b/xmodule/studio_editable.py @@ -4,7 +4,6 @@ from typing import Callable from xblock.core import XBlock, XBlockMixin -from xmodule.util.duplicate import handle_children_duplication from xmodule.x_module import AUTHOR_VIEW, STUDENT_VIEW @@ -17,6 +16,8 @@ class StudioEditableBlock(XBlockMixin): This class is only intended to be used with an XBlock! """ + has_author_view = True + def render_children(self, context, fragment, can_reorder=False, can_add=False): """ Renders the children of the block with HTML appropriate for Studio. If can_reorder is True, @@ -52,36 +53,6 @@ def get_preview_view_name(block): """ return AUTHOR_VIEW if has_author_view(block) else STUDENT_VIEW - def editor_saved(self, user, old_metadata, old_content) -> None: # pylint: disable=unused-argument - """ - Called right *before* the block is written to the DB. Can be used, e.g., to modify fields before saving. - - By default, is a no-op. Can be overriden in subclasses. - """ - - def post_editor_saved(self, user, old_metadata, old_content) -> None: # pylint: disable=unused-argument - """ - Called right *after* the block is written to the DB. Can be used, e.g., to spin up followup tasks. - - By default, is a no-op. Can be overriden in subclasses. - """ - - def studio_post_duplicate( - self, - source_item, - store, - user, - duplication_function: Callable[..., None], - shallow: bool, - ) -> None: # pylint: disable=unused-argument - """ - Called when a the block is duplicated. Can be used, e.g., for special handling of child duplication. - - Children must always be handled. In case of inheritance it can be done by running this method with super(). - - By default, implements standard duplication logic. - """ - handle_children_duplication(self, source_item, store, user, duplication_function, shallow) def has_author_view(block): diff --git a/xmodule/util/duplicate.py b/xmodule/util/duplicate.py deleted file mode 100644 index dbbbdf7f6531..000000000000 --- a/xmodule/util/duplicate.py +++ /dev/null @@ -1,19 +0,0 @@ -""" -Utility to aid xblock duplication -""" -from typing import Callable - - -def handle_children_duplication( - xblock, source_item, store, user, duplication_function: Callable[..., None], shallow: bool -): - """Implements default behaviour to handle children duplication.""" - if not source_item.has_children or shallow: - return - - xblock.children = xblock.children or [] - for child in source_item.children: - dupe = duplication_function(xblock.location, child, user=user, is_child=True) - if dupe not in xblock.children: # duplicate_fun may add the child for us. - xblock.children.append(dupe) - store.update_item(xblock, user.id) diff --git a/xmodule/vertical_block.py b/xmodule/vertical_block.py index a76f0cc16f2f..10ff0a540870 100644 --- a/xmodule/vertical_block.py +++ b/xmodule/vertical_block.py @@ -76,8 +76,6 @@ class VerticalBlock( Layout XBlock for rendering subblocks vertically. """ - has_author_view = True - resources_dir = 'assets/vertical' mako_template = 'widgets/sequence-edit.html' From 619958821dc2b9c965aa6170123f52520ced63be Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Thu, 30 Nov 2023 00:53:04 -0300 Subject: [PATCH 10/15] refactor: move duplication logic into authoringmixin --- cms/djangoapps/contentstore/utils.py | 67 +++---------- .../xblock_storage_handlers/view_handlers.py | 97 ++----------------- cms/lib/xblock/authoring_mixin.py | 83 +++++++++++++++- 3 files changed, 96 insertions(+), 151 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 83fa7c8c40db..e5f73b8137e0 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -6,8 +6,7 @@ import logging from collections import defaultdict from contextlib import contextmanager -from datetime import datetime, timezone -from uuid import uuid4 +from datetime import datetime from django.conf import settings from django.core.exceptions import ValidationError @@ -18,8 +17,6 @@ from lti_consumer.models import CourseAllowPIISharingInLTIFlag from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.locator import LibraryLocator -from openedx_events.content_authoring.data import DuplicatedXBlockData -from openedx_events.content_authoring.signals import XBLOCK_DUPLICATED from milestones import api as milestones_api from pytz import UTC from xblock.fields import Scope @@ -1026,64 +1023,22 @@ def duplicate_block( Duplicate an existing xblock as a child of the supplied parent_usage_key. You can optionally specify what usage key the new duplicate block will use via dest_usage_key. - If shallow is True, does not copy children. Otherwise, this function calls itself - recursively, and will set the is_child flag to True when dealing with recursed child - blocks. + If shallow is True, does not copy children. """ store = modulestore() with store.bulk_operations(duplicate_source_usage_key.course_key): source_item = store.get_item(duplicate_source_usage_key) - if not dest_usage_key: - # Change the blockID to be unique. - dest_usage_key = source_item.location.replace(name=uuid4().hex) - - category = dest_usage_key.block_type - - duplicate_metadata, asides_to_create = gather_block_attributes( - source_item, display_name=display_name, is_child=is_child, - ) - - dest_block = store.create_item( - user.id, - dest_usage_key.course_key, - dest_usage_key.block_type, - block_id=dest_usage_key.block_id, - definition_data=source_item.get_explicitly_set_fields_by_scope(Scope.content), - metadata=duplicate_metadata, - runtime=source_item.runtime, - asides=asides_to_create - ) - - # Allow an XBlock to do anything fancy it may need to when duplicated from another block. - load_services_for_studio(source_item.runtime, user) - dest_block.studio_post_duplicate( - source_item, store, user, duplication_function=duplicate_block, shallow=shallow + return source_item.studio_duplicate( + parent_usage_key=parent_usage_key, + duplicate_source_usage_key=duplicate_source_usage_key, + user=user, + store=store, + dest_usage_key=dest_usage_key, + display_name=display_name, + shallow=shallow, + is_child=is_child, ) - # pylint: disable=protected-access - if 'detached' not in source_item.runtime.load_block_type(category)._class_tags: - parent = store.get_item(parent_usage_key) - # If source was already a child of the parent, add duplicate immediately afterward. - # Otherwise, add child to end. - if source_item.location in parent.children: - source_index = parent.children.index(source_item.location) - parent.children.insert(source_index + 1, dest_block.location) - else: - parent.children.append(dest_block.location) - store.update_item(parent, user.id) - - # .. event_implemented_name: XBLOCK_DUPLICATED - XBLOCK_DUPLICATED.send_event( - time=datetime.now(timezone.utc), - xblock_info=DuplicatedXBlockData( - usage_key=dest_block.location, - block_type=dest_block.location.block_type, - source_usage_key=duplicate_source_usage_key, - ) - ) - - return dest_block.location - def update_from_source(*, source_block, destination_block, user_id): """ diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index c7e080fa7d10..98decdb3d93b 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -10,7 +10,6 @@ """ import logging from datetime import datetime -from uuid import uuid4 from attrs import asdict from django.conf import settings @@ -18,11 +17,8 @@ from django.contrib.auth.models import User # pylint: disable=imported-auth-user from django.core.exceptions import PermissionDenied from django.http import HttpResponse, HttpResponseBadRequest -from django.utils.timezone import timezone from django.utils.translation import gettext as _ from edx_django_utils.plugins import pluggable_override -from openedx_events.content_authoring.data import DuplicatedXBlockData -from openedx_events.content_authoring.signals import XBLOCK_DUPLICATED from openedx_tagging.core.tagging import api as tagging_api from edx_proctoring.api import ( does_backend_support_onboarding, @@ -755,94 +751,15 @@ def _duplicate_block( store = modulestore() with store.bulk_operations(duplicate_source_usage_key.course_key): source_item = store.get_item(duplicate_source_usage_key) - # Change the blockID to be unique. - dest_usage_key = source_item.location.replace(name=uuid4().hex) - category = dest_usage_key.block_type - - # Update the display name to indicate this is a duplicate (unless display name provided). - # Can't use own_metadata(), b/c it converts data for JSON serialization - - # not suitable for setting metadata of the new block - duplicate_metadata = {} - for field in source_item.fields.values(): - if field.scope == Scope.settings and field.is_set_on(source_item): - duplicate_metadata[field.name] = field.read_from(source_item) - - if is_child: - display_name = ( - display_name or source_item.display_name or source_item.category - ) - - if display_name is not None: - duplicate_metadata["display_name"] = display_name - else: - if source_item.display_name is None: - duplicate_metadata["display_name"] = _("Duplicate of {0}").format( - source_item.category - ) - else: - duplicate_metadata["display_name"] = _("Duplicate of '{0}'").format( - source_item.display_name - ) - - asides_to_create = [] - for aside in source_item.runtime.get_asides(source_item): - for field in aside.fields.values(): - if field.scope in ( - Scope.settings, - Scope.content, - ) and field.is_set_on(aside): - asides_to_create.append(aside) - break - - for aside in asides_to_create: - for field in aside.fields.values(): - if field.scope not in ( - Scope.settings, - Scope.content, - ): - field.delete_from(aside) - - dest_block = store.create_item( - user.id, - dest_usage_key.course_key, - dest_usage_key.block_type, - block_id=dest_usage_key.block_id, - definition_data=source_item.get_explicitly_set_fields_by_scope( - Scope.content - ), - metadata=duplicate_metadata, - runtime=source_item.runtime, - asides=asides_to_create, - ) - - # Allow an XBlock to do anything fancy it may need to when duplicated from another block. - load_services_for_studio(source_item.runtime, user) - dest_block.studio_post_duplicate(source_item, store, user, duplication_function=_duplicate_block, shallow=False) - - # pylint: disable=protected-access - if "detached" not in source_item.runtime.load_block_type(category)._class_tags: - parent = store.get_item(parent_usage_key) - # If source was already a child of the parent, add duplicate immediately afterward. - # Otherwise, add child to end. - if source_item.location in parent.children: - source_index = parent.children.index(source_item.location) - parent.children.insert(source_index + 1, dest_block.location) - else: - parent.children.append(dest_block.location) - store.update_item(parent, user.id) - - # .. event_implemented_name: XBLOCK_DUPLICATED - XBLOCK_DUPLICATED.send_event( - time=datetime.now(timezone.utc), - xblock_info=DuplicatedXBlockData( - usage_key=dest_block.location, - block_type=dest_block.location.block_type, - source_usage_key=duplicate_source_usage_key, - ), + return source_item.studio_duplicate( + parent_usage_key=parent_usage_key, + duplicate_source_usage_key=duplicate_source_usage_key, + user=user, + store=store, + display_name=display_name, + is_child=is_child, ) - return dest_block.location - @login_required def delete_item(request, usage_key): diff --git a/cms/lib/xblock/authoring_mixin.py b/cms/lib/xblock/authoring_mixin.py index c2e336c44a3f..9310da490985 100644 --- a/cms/lib/xblock/authoring_mixin.py +++ b/cms/lib/xblock/authoring_mixin.py @@ -4,12 +4,16 @@ import logging -from typing import Callable +from datetime import datetime, timezone +from uuid import uuid4 from django.conf import settings from web_fragments.fragment import Fragment from xblock.core import XBlock, XBlockMixin from xblock.fields import String, Scope +from openedx_events.content_authoring.data import DuplicatedXBlockData +from openedx_events.content_authoring.signals import XBLOCK_DUPLICATED + log = logging.getLogger(__name__) @@ -67,16 +71,84 @@ def post_editor_saved(self, user, old_metadata, old_content) -> None: # pylint: By default, is a no-op. Can be overriden in subclasses. """ + def studio_duplicate( + self, + parent_usage_key, + duplicate_source_usage_key, + user, + store, + dest_usage_key=None, + display_name=None, + shallow=False, + is_child=False, + ): + """ + Duplicate an existing xblock as a child of the supplied parent_usage_key. You can + optionally specify what usage key the new duplicate block will use via dest_usage_key. + + If shallow is True, does not copy children. + """ + from cms.djangoapps.contentstore.utils import gather_block_attributes, load_services_for_studio + + if not dest_usage_key: + # Change the blockID to be unique. + dest_usage_key = self.location.replace(name=uuid4().hex) + + category = dest_usage_key.block_type + + duplicate_metadata, asides_to_create = gather_block_attributes( + self, + display_name=display_name, + is_child=is_child, + ) + + dest_block = store.create_item( + user.id, + dest_usage_key.course_key, + dest_usage_key.block_type, + block_id=dest_usage_key.block_id, + definition_data=self.get_explicitly_set_fields_by_scope(Scope.content), + metadata=duplicate_metadata, + runtime=self.runtime, + asides=asides_to_create, + ) + + # Allow an XBlock to do anything fancy it may need to when duplicated from another block. + load_services_for_studio(self.runtime, user) + dest_block.studio_post_duplicate(self, store, user, shallow=shallow) + # pylint: disable=protected-access + if "detached" not in self.runtime.load_block_type(category)._class_tags: + parent = store.get_item(parent_usage_key) + # If source was already a child of the parent, add duplicate immediately afterward. + # Otherwise, add child to end. + if self.location in parent.children: + source_index = parent.children.index(self.location) + parent.children.insert(source_index + 1, dest_block.location) + else: + parent.children.append(dest_block.location) + store.update_item(parent, user.id) + + # .. event_implemented_name: XBLOCK_DUPLICATED + XBLOCK_DUPLICATED.send_event( + time=datetime.now(timezone.utc), + xblock_info=DuplicatedXBlockData( + usage_key=dest_block.location, + block_type=dest_block.location.block_type, + source_usage_key=duplicate_source_usage_key, + ), + ) + + return dest_block.location + def studio_post_duplicate( self, source_item, store, user, - duplication_function: Callable[..., None], shallow: bool, ) -> None: # pylint: disable=unused-argument """ - Called when a the block is duplicated. Can be used, e.g., for special handling of child duplication. + Called when after a block is duplicated. Can be used, e.g., for special handling of child duplication. Children must always be handled. In case of inheritance it can be done by running this method with super(). @@ -87,7 +159,8 @@ def studio_post_duplicate( self.children = self.children or [] for child in source_item.children: - dupe = duplication_function(self.location, child, user=user, is_child=True) - if dupe not in self.children: # duplicate_fun may add the child for us. + child_block = store.get_item(child) + dupe = child_block.studio_duplicate(self.location, child, user=user, store=store, is_child=True) + if dupe not in self.children: # studio_duplicate may add the child for us. self.children.append(dupe) store.update_item(self, user.id) From 7f6924fa95011d59af6a59f037465a542557603b Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Thu, 30 Nov 2023 01:29:37 -0300 Subject: [PATCH 11/15] style: pylint and pep8 fixes --- xmodule/services.py | 8 -------- xmodule/studio_editable.py | 3 --- 2 files changed, 11 deletions(-) diff --git a/xmodule/services.py b/xmodule/services.py index 48a79c85f2b2..25c680a9653a 100644 --- a/xmodule/services.py +++ b/xmodule/services.py @@ -6,23 +6,15 @@ import inspect import logging from functools import partial -from common.djangoapps.edxmako.services import MakoService -from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService from config_models.models import ConfigurationModel from django.conf import settings from eventtracking import tracker from edx_when.field_data import DateLookupFieldData -from lti_consumer.models import CourseAllowPIISharingInLTIFlag from xblock.reference.plugins import Service from xblock.runtime import KvsFieldData -import xmodule from common.djangoapps.track import contexts -from common.djangoapps.student.auth import ( - has_studio_read_access, - has_studio_write_access, -) from lms.djangoapps.courseware.masquerade import is_masquerading_as_specific_student from xmodule.modulestore.django import modulestore diff --git a/xmodule/studio_editable.py b/xmodule/studio_editable.py index 43fe7c05aa77..d190c966cab8 100644 --- a/xmodule/studio_editable.py +++ b/xmodule/studio_editable.py @@ -1,8 +1,6 @@ """ Mixin to support editing in Studio. """ -from typing import Callable - from xblock.core import XBlock, XBlockMixin from xmodule.x_module import AUTHOR_VIEW, STUDENT_VIEW @@ -54,7 +52,6 @@ def get_preview_view_name(block): return AUTHOR_VIEW if has_author_view(block) else STUDENT_VIEW - def has_author_view(block): """ Returns True if the xmodule linked to the block supports "author_view". From adeec3ab392a1879609690678791e1d3c6b9cfa9 Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Thu, 30 Nov 2023 01:32:30 -0300 Subject: [PATCH 12/15] refactor: rename source_item to source_block --- cms/lib/xblock/authoring_mixin.py | 6 +++--- xmodule/library_content_block.py | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cms/lib/xblock/authoring_mixin.py b/cms/lib/xblock/authoring_mixin.py index 9310da490985..d66f9f88cd0e 100644 --- a/cms/lib/xblock/authoring_mixin.py +++ b/cms/lib/xblock/authoring_mixin.py @@ -142,7 +142,7 @@ def studio_duplicate( def studio_post_duplicate( self, - source_item, + source_block, store, user, shallow: bool, @@ -154,11 +154,11 @@ def studio_post_duplicate( By default, implements standard duplication logic. """ - if not source_item.has_children or shallow: + if not source_block.has_children or shallow: return self.children = self.children or [] - for child in source_item.children: + for child in source_block.children: child_block = store.get_item(child) dupe = child_block.studio_duplicate(self.location, child, user=user, store=store, is_child=True) if dupe not in self.children: # studio_duplicate may add the child for us. diff --git a/xmodule/library_content_block.py b/xmodule/library_content_block.py index 081dbb657871..6ba986034a49 100644 --- a/xmodule/library_content_block.py +++ b/xmodule/library_content_block.py @@ -589,7 +589,7 @@ def children_are_syncing(self, request, suffix=''): # pylint: disable=unused-ar def studio_post_duplicate( self, - source_item, + source_block, *_args, **__kwargs, ) -> None: @@ -600,7 +600,7 @@ def studio_post_duplicate( Otherwise we'll end up losing data on the next refresh. """ self._validate_sync_permissions() - self.get_tools(to_read_library_content=True).trigger_duplication(source_block=source_item, dest_block=self) + self.get_tools(to_read_library_content=True).trigger_duplication(source_block=source_block, dest_block=self) def _validate_library_version(self, validation, lib_tools, version, library_key): """ From cd78eb3b6b8fab921901237d3d1d36c7d0cf894f Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Fri, 1 Dec 2023 08:14:45 -0300 Subject: [PATCH 13/15] refactor: remove redundant hasattrs due to inheritance --- cms/djangoapps/contentstore/views/block.py | 2 +- .../xblock_storage_handlers/view_handlers.py | 9 +++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/cms/djangoapps/contentstore/views/block.py b/cms/djangoapps/contentstore/views/block.py index b7b75d23b227..810a824498f9 100644 --- a/cms/djangoapps/contentstore/views/block.py +++ b/cms/djangoapps/contentstore/views/block.py @@ -9,10 +9,10 @@ from django.http import Http404, HttpResponse from django.utils.translation import gettext as _ from django.views.decorators.http import require_http_methods -from cms.djangoapps.contentstore.utils import load_services_for_studio from opaque_keys.edx.keys import CourseKey from web_fragments.fragment import Fragment +from cms.djangoapps.contentstore.utils import load_services_for_studio from cms.lib.xblock.authoring_mixin import VISIBILITY_VIEW from common.djangoapps.edxmako.shortcuts import render_to_string from common.djangoapps.student.auth import ( diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index 98decdb3d93b..9e385e9d91a9 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -301,13 +301,10 @@ def _update_with_callback(xblock, user, old_metadata=None, old_content=None): old_metadata = own_metadata(xblock) if old_content is None: old_content = xblock.get_explicitly_set_fields_by_scope(Scope.content) - if hasattr(xblock, "editor_saved"): - load_services_for_studio(xblock.runtime, user) - xblock.editor_saved(user, old_metadata, old_content) + load_services_for_studio(xblock.runtime, user) + xblock.editor_saved(user, old_metadata, old_content) xblock_updated = modulestore().update_item(xblock, user.id) - if hasattr(xblock_updated, "post_editor_saved"): - load_services_for_studio(xblock_updated.runtime, user) - xblock_updated.post_editor_saved(user, old_metadata, old_content) + xblock_updated.post_editor_saved(user, old_metadata, old_content) return xblock_updated From 3cb2769edb72a799a8aa010366c0d378e1c2aaba Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Sun, 7 Jul 2024 11:06:51 -0400 Subject: [PATCH 14/15] revert: remove studio duplicate hooks --- cms/djangoapps/contentstore/utils.py | 814 ++++++++++++++++++++++++--- cms/lib/xblock/authoring_mixin.py | 95 +--- xmodule/library_content_block.py | 37 +- 3 files changed, 775 insertions(+), 171 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 5f2015ffa0fc..9ed3f1ce4b36 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -4,25 +4,36 @@ from __future__ import annotations import configparser import logging +import re from collections import defaultdict from contextlib import contextmanager -from datetime import datetime +from datetime import datetime, timezone +from urllib.parse import quote_plus +from uuid import uuid4 from django.conf import settings from django.core.exceptions import ValidationError from django.urls import reverse from django.utils import translation from django.utils.translation import gettext as _ +from eventtracking import tracker from help_tokens.core import HelpUrlExpert from lti_consumer.models import CourseAllowPIISharingInLTIFlag from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.locator import LibraryLocator +from openedx.core.lib.teams_config import CONTENT_GROUPS_FOR_TEAMS, TEAM_SCHEME +from openedx_events.content_authoring.data import DuplicatedXBlockData +from openedx_events.content_authoring.signals import XBLOCK_DUPLICATED +from openedx_events.learning.data import CourseNotificationData +from openedx_events.learning.signals import COURSE_NOTIFICATION_REQUESTED + from milestones import api as milestones_api from pytz import UTC from xblock.fields import Scope from cms.djangoapps.contentstore.toggles import exam_setting_view_enabled -from common.djangoapps.course_action_state.models import CourseRerunUIStateManager +from common.djangoapps.course_action_state.models import CourseRerunUIStateManager, CourseRerunState +from common.djangoapps.course_action_state.managers import CourseActionStateItemNotFoundError from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.edxmako.services import MakoService from common.djangoapps.student import auth @@ -33,6 +44,7 @@ CourseStaffRole, GlobalStaff, ) +from common.djangoapps.track import contexts from common.djangoapps.util.course import get_link_for_about_page from common.djangoapps.util.milestones_helpers import ( is_prerequisite_courses_enabled, @@ -42,8 +54,11 @@ get_namespace_choices, generate_milestone_namespace ) +from common.djangoapps.util.date_utils import get_default_time_display +from common.djangoapps.xblock_django.api import deprecated_xblocks from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService from openedx.core import toggles as core_toggles +from openedx.core.djangoapps.content_tagging.toggles import is_tagging_feature_disabled from openedx.core.djangoapps.credit.api import get_credit_requirements, is_credit_course from openedx.core.djangoapps.discussions.config.waffle import ENABLE_PAGES_AND_RESOURCES_MICROFRONTEND from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration @@ -53,6 +68,7 @@ from openedx.core.djangoapps.site_configuration.models import SiteConfiguration from openedx.core.djangoapps.models.course_details import CourseDetails from openedx.core.lib.courses import course_image_url +from openedx.core.lib.html_to_text import html_to_text from openedx.features.content_type_gating.models import ContentTypeGatingConfig from openedx.features.content_type_gating.partitions import CONTENT_TYPE_GATING_SCHEME from openedx.features.course_experience.waffle import ENABLE_COURSE_ABOUT_SIDEBAR_HTML @@ -60,24 +76,28 @@ split_library_view_on_dashboard, use_new_advanced_settings_page, use_new_course_outline_page, + use_new_certificates_page, use_new_export_page, use_new_files_uploads_page, use_new_grading_page, + use_new_group_configurations_page, use_new_course_team_page, use_new_home_page, use_new_import_page, use_new_schedule_details_page, use_new_text_editor, + use_new_textbooks_page, use_new_unit_page, use_new_updates_page, use_new_video_editor, use_new_video_uploads_page, use_new_custom_pages, - use_tagging_taxonomy_list_page, - # use_xpert_translations_component, ) from cms.djangoapps.models.settings.course_grading import CourseGradingModel +from cms.djangoapps.models.settings.course_metadata import CourseMetadata from xmodule.library_tools import LibraryToolsService +from xmodule.course_block import DEFAULT_START_DATE # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.data import CertificatesDisplayBehaviors from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order @@ -85,6 +105,7 @@ from xmodule.services import SettingsService, ConfigurationService, TeamsConfigurationService +IMPORTABLE_FILE_TYPES = ('.tar.gz', '.zip') log = logging.getLogger(__name__) @@ -416,6 +437,45 @@ def get_unit_url(course_locator, unit_locator) -> str: return unit_url +def get_certificates_url(course_locator) -> str: + """ + Gets course authoring microfrontend URL for certificates page view. + """ + certificates_url = None + if use_new_certificates_page(course_locator): + mfe_base_url = get_course_authoring_url(course_locator) + course_mfe_url = f'{mfe_base_url}/course/{course_locator}/certificates' + if mfe_base_url: + certificates_url = course_mfe_url + return certificates_url + + +def get_textbooks_url(course_locator) -> str: + """ + Gets course authoring microfrontend URL for textbooks page view. + """ + textbooks_url = None + if use_new_textbooks_page(course_locator): + mfe_base_url = get_course_authoring_url(course_locator) + course_mfe_url = f'{mfe_base_url}/course/{course_locator}/textbooks' + if mfe_base_url: + textbooks_url = course_mfe_url + return textbooks_url + + +def get_group_configurations_url(course_locator) -> str: + """ + Gets course authoring microfrontend URL for group configurations page view. + """ + group_configurations_url = None + if use_new_group_configurations_page(course_locator): + mfe_base_url = get_course_authoring_url(course_locator) + course_mfe_url = f'{mfe_base_url}/course/{course_locator}/group_configurations' + if mfe_base_url: + group_configurations_url = course_mfe_url + return group_configurations_url + + def get_custom_pages_url(course_locator) -> str: """ Gets course authoring microfrontend URL for custom pages view. @@ -429,16 +489,19 @@ def get_custom_pages_url(course_locator) -> str: return custom_pages_url -def get_taxonomy_list_url(): +def get_taxonomy_list_url() -> str | None: """ Gets course authoring microfrontend URL for taxonomy list page view. """ - taxonomy_list_url = None - if use_tagging_taxonomy_list_page(): - mfe_base_url = settings.COURSE_AUTHORING_MICROFRONTEND_URL - if mfe_base_url: - taxonomy_list_url = f'{mfe_base_url}/taxonomy-list' - return taxonomy_list_url + if is_tagging_feature_disabled(): + return None + + mfe_base_url = settings.COURSE_AUTHORING_MICROFRONTEND_URL + + if not mfe_base_url: + return None + + return f'{mfe_base_url}/taxonomies' def get_taxonomy_tags_widget_url(course_locator=None) -> str | None: @@ -447,15 +510,18 @@ def get_taxonomy_tags_widget_url(course_locator=None) -> str | None: The `content_id` needs to be appended to the end of the URL when using it. """ - taxonomy_tags_widget_url = None - # Uses the same waffle flag as taxonomy list page - if use_tagging_taxonomy_list_page(): + if is_tagging_feature_disabled(): + return None + + if course_locator: + mfe_base_url = get_course_authoring_url(course_locator) + else: mfe_base_url = settings.COURSE_AUTHORING_MICROFRONTEND_URL - if course_locator: - mfe_base_url = get_course_authoring_url(course_locator) - if mfe_base_url: - taxonomy_tags_widget_url = f'{mfe_base_url}/tagging/components/widget/' - return taxonomy_tags_widget_url + + if not mfe_base_url: + return None + + return f'{mfe_base_url}/tagging/components/widget/' def course_import_olx_validation_is_enabled(): @@ -579,6 +645,15 @@ def ancestor_has_staff_lock(xblock, parent_xblock=None): return parent_xblock.visible_to_staff_only +def get_sequence_usage_keys(course): + """ + Extracts a list of 'subsections' usage_keys + """ + return [str(subsection.location) + for section in course.get_children() + for subsection in section.get_children()] + + def reverse_url(handler_name, key_name=None, key_value=None, kwargs=None): """ Creates the URL for the given handler. @@ -771,6 +846,9 @@ def get_visibility_partition_info(xblock, course=None): if len(partition["groups"]) > 1 or any(group["selected"] for group in partition["groups"]): selectable_partitions.append(partition) + team_user_partitions = get_user_partition_info(xblock, schemes=["team"], course=course) + selectable_partitions += team_user_partitions + course_key = xblock.scope_ids.usage_id.course_key is_library = isinstance(course_key, LibraryLocator) if not is_library and ContentTypeGatingConfig.current(course_key=course_key).studio_override_enabled: @@ -1024,22 +1102,77 @@ def duplicate_block( Duplicate an existing xblock as a child of the supplied parent_usage_key. You can optionally specify what usage key the new duplicate block will use via dest_usage_key. - If shallow is True, does not copy children. + If shallow is True, does not copy children. Otherwise, this function calls itself + recursively, and will set the is_child flag to True when dealing with recursed child + blocks. """ store = modulestore() with store.bulk_operations(duplicate_source_usage_key.course_key): source_item = store.get_item(duplicate_source_usage_key) - return source_item.studio_duplicate( - parent_usage_key=parent_usage_key, - duplicate_source_usage_key=duplicate_source_usage_key, - user=user, - store=store, - dest_usage_key=dest_usage_key, - display_name=display_name, - shallow=shallow, - is_child=is_child, + if not dest_usage_key: + # Change the blockID to be unique. + dest_usage_key = source_item.location.replace(name=uuid4().hex) + + category = dest_usage_key.block_type + + duplicate_metadata, asides_to_create = gather_block_attributes( + source_item, display_name=display_name, is_child=is_child, ) + dest_block = store.create_item( + user.id, + dest_usage_key.course_key, + dest_usage_key.block_type, + block_id=dest_usage_key.block_id, + definition_data=source_item.get_explicitly_set_fields_by_scope(Scope.content), + metadata=duplicate_metadata, + runtime=source_item.runtime, + asides=asides_to_create + ) + + children_handled = False + + if hasattr(dest_block, 'studio_post_duplicate'): + # Allow an XBlock to do anything fancy it may need to when duplicated from another block. + # These blocks may handle their own children or parenting if needed. Let them return booleans to + # let us know if we need to handle these or not. + load_services_for_studio(dest_block.runtime, user) + children_handled = dest_block.studio_post_duplicate(store, source_item) + + # Children are not automatically copied over (and not all xblocks have a 'children' attribute). + # Because DAGs are not fully supported, we need to actually duplicate each child as well. + if source_item.has_children and not shallow and not children_handled: + dest_block.children = dest_block.children or [] + for child in source_item.children: + dupe = duplicate_block(dest_block.location, child, user=user, is_child=True) + if dupe not in dest_block.children: # _duplicate_block may add the child for us. + dest_block.children.append(dupe) + store.update_item(dest_block, user.id) + + # pylint: disable=protected-access + if 'detached' not in source_item.runtime.load_block_type(category)._class_tags: + parent = store.get_item(parent_usage_key) + # If source was already a child of the parent, add duplicate immediately afterward. + # Otherwise, add child to end. + if source_item.location in parent.children: + source_index = parent.children.index(source_item.location) + parent.children.insert(source_index + 1, dest_block.location) + else: + parent.children.append(dest_block.location) + store.update_item(parent, user.id) + + # .. event_implemented_name: XBLOCK_DUPLICATED + XBLOCK_DUPLICATED.send_event( + time=datetime.now(timezone.utc), + xblock_info=DuplicatedXBlockData( + usage_key=dest_block.location, + block_type=dest_block.location.block_type, + source_usage_key=duplicate_source_usage_key, + ) + ) + + return dest_block.location + def update_from_source(*, source_block, destination_block, user_id): """ @@ -1137,8 +1270,16 @@ def load_services_for_studio(runtime, user): def update_course_details(request, course_key, payload, course_block): """ - Utils is used to update course details. - It is used for both DRF and django views. + Utility function used to update course details. It is used for both DRF and legacy Django views. + + Args: + request (WSGIRequest): Django HTTP request object + course_key (CourseLocator): The course run key + payload (dict): Dictionary of course run settings + course_block (CourseBlockWithMixins): A course run instance + + Returns: + None """ from .views.entrance_exam import create_entrance_exam, delete_entrance_exam, update_entrance_exam @@ -1164,10 +1305,9 @@ def update_course_details(request, course_key, payload, course_block): if milestone["namespace"] != entrance_exam_namespace: remove_prerequisite_course(course_key, milestone) - # If the entrance exams feature has been enabled, we'll need to check for some - # feature-specific settings and handle them accordingly - # We have to be careful that we're only executing the following logic if we actually - # need to create or delete an entrance exam from the specified course + # If the entrance exams feature has been enabled, we'll need to check for some feature-specific settings and handle + # them accordingly. We have to be careful that we're only executing the following logic if we actually need to + # create or delete an entrance exam from the specified course if core_toggles.ENTRANCE_EXAMS.is_enabled(): course_entrance_exam_present = course_block.entrance_exam_enabled entrance_exam_enabled = payload.get('entrance_exam_enabled', '') == 'true' @@ -1189,12 +1329,22 @@ def update_course_details(request, course_key, payload, course_block): # If there's no entrance exam defined, we'll create a new one else: create_entrance_exam(request, course_key, entrance_exam_minimum_score_pct) - - # If the entrance exam box on the settings screen has been unchecked, - # and the course has an entrance exam attached... + # If the entrance exam box on the settings screen has been unchecked, and the course has an entrance exam + # attached... elif not entrance_exam_enabled and course_entrance_exam_present: delete_entrance_exam(request, course_key) + # Fix any potential issues with the display behavior and availability date of certificates before saving the update. + # A self-paced course run should *never* have a display behavior of anything other than "Immediately Upon Passing" + # ("early_no_info") and does not support having a certificate available date. We are aware of an issue with the + # legacy Django template-based frontend where bad data is allowed to creep into the system, which can cause + # downstream services (e.g. the Credentials IDA) to go haywire. This bad data is most often seen when a course run + # is updated from instructor-paced to self-paced (self_paced == True in our JSON payload), so we check and fix + # during these updates. Otherwise, the legacy UI seems to do the right thing. + if "self_paced" in payload and payload["self_paced"]: + payload["certificate_available_date"] = None + payload["certificates_display_behavior"] = CertificatesDisplayBehaviors.EARLY_NO_INFO + # Perform the normal update workflow for the CourseDetails model return CourseDetails.update_from_json(course_key, payload, request.user) @@ -1342,7 +1492,8 @@ def get_course_grading(course_key): 'grading_url': reverse_course_url('grading_handler', course_key), 'is_credit_course': is_credit_course(course_key), 'mfe_proctored_exam_settings_url': get_proctored_exam_settings_url(course_key), - 'course_assignment_lists': dict(course_assignment_lists) + 'course_assignment_lists': dict(course_assignment_lists), + 'default_grade_designations': settings.DEFAULT_GRADE_DESIGNATIONS } return grading_context @@ -1414,48 +1565,66 @@ def get_library_context(request, request_is_json=False): return data -def get_home_context(request): +def get_course_context(request): """ - Utils is used to get context of course home. + Utils is used to get context of course home library tab. It is used for both DRF and django views. """ from cms.djangoapps.contentstore.views.course import ( - get_allowed_organizations, - get_allowed_organizations_for_libraries, get_courses_accessible_to_user, - user_can_create_organizations, - _accessible_libraries_iter, - _get_course_creator_status, - _format_library_for_view, _process_courses_list, ENABLE_GLOBAL_STAFF_OPTIMIZATION, ) - from cms.djangoapps.contentstore.views.library import ( - LIBRARY_AUTHORING_MICROFRONTEND_URL, - LIBRARIES_ENABLED, - should_redirect_to_library_authoring_mfe, - user_can_create_library, - ) + + def format_in_process_course_view(uca): + """ + Return a dict of the data which the view requires for each unsucceeded course + """ + return { + 'display_name': uca.display_name, + 'course_key': str(uca.course_key), + 'org': uca.course_key.org, + 'number': uca.course_key.course, + 'run': uca.course_key.run, + 'is_failed': uca.state == CourseRerunUIStateManager.State.FAILED, + 'is_in_progress': uca.state == CourseRerunUIStateManager.State.IN_PROGRESS, + 'dismiss_link': reverse_course_url( + 'course_notifications_handler', + uca.course_key, + kwargs={ + 'action_state_id': uca.id, + }, + ) if uca.state == CourseRerunUIStateManager.State.FAILED else '' + } optimization_enabled = GlobalStaff().has_user(request.user) and ENABLE_GLOBAL_STAFF_OPTIMIZATION.is_enabled() org = request.GET.get('org', '') if optimization_enabled else None courses_iter, in_process_course_actions = get_courses_accessible_to_user(request, org) - user = request.user - libraries = [] - response_format = get_response_format(request) + split_archived = settings.FEATURES.get('ENABLE_SEPARATE_ARCHIVED_COURSES', False) + active_courses, archived_courses = _process_courses_list(courses_iter, in_process_course_actions, split_archived) + in_process_course_actions = [format_in_process_course_view(uca) for uca in in_process_course_actions] + return active_courses, archived_courses, in_process_course_actions - if not split_library_view_on_dashboard() and LIBRARIES_ENABLED: - accessible_libraries = _accessible_libraries_iter(user) - libraries = [_format_library_for_view(lib, request) for lib in accessible_libraries] - if split_library_view_on_dashboard() and request_response_format_is_json(request, response_format): - libraries = get_library_context(request, True)['libraries'] +def get_course_context_v2(request): + """Get context of the homepage course tab from the Studio Home.""" + + # Importing here to avoid circular imports: + # ImportError: cannot import name 'reverse_course_url' from partially initialized module + # 'cms.djangoapps.contentstore.utils' (most likely due to a circular import) + from cms.djangoapps.contentstore.views.course import ( + get_courses_accessible_to_user, + ENABLE_GLOBAL_STAFF_OPTIMIZATION, + ) def format_in_process_course_view(uca): """ - Return a dict of the data which the view requires for each unsucceeded course + Return a dict of the data which the view requires for each unsucceeded course. + + Args: + uca: CourseRerunUIStateManager object. """ return { 'display_name': uca.display_name, @@ -1474,9 +1643,50 @@ def format_in_process_course_view(uca): ) if uca.state == CourseRerunUIStateManager.State.FAILED else '' } - split_archived = settings.FEATURES.get('ENABLE_SEPARATE_ARCHIVED_COURSES', False) - active_courses, archived_courses = _process_courses_list(courses_iter, in_process_course_actions, split_archived) + optimization_enabled = GlobalStaff().has_user(request.user) and ENABLE_GLOBAL_STAFF_OPTIMIZATION.is_enabled() + + org = request.GET.get('org', '') if optimization_enabled else None + courses_iter, in_process_course_actions = get_courses_accessible_to_user(request, org) in_process_course_actions = [format_in_process_course_view(uca) for uca in in_process_course_actions] + return courses_iter, in_process_course_actions + + +def get_home_context(request, no_course=False): + """ + Utils is used to get context of course home. + It is used for both DRF and django views. + """ + + from cms.djangoapps.contentstore.views.course import ( + get_allowed_organizations, + get_allowed_organizations_for_libraries, + user_can_create_organizations, + _accessible_libraries_iter, + _get_course_creator_status, + _format_library_for_view, + ENABLE_GLOBAL_STAFF_OPTIMIZATION, + ) + from cms.djangoapps.contentstore.views.library import ( + LIBRARY_AUTHORING_MICROFRONTEND_URL, + LIBRARIES_ENABLED, + should_redirect_to_library_authoring_mfe, + user_can_create_library, + ) + + active_courses = [] + archived_courses = [] + in_process_course_actions = [] + + optimization_enabled = GlobalStaff().has_user(request.user) and ENABLE_GLOBAL_STAFF_OPTIMIZATION.is_enabled() + + user = request.user + libraries = [] + + if not no_course: + active_courses, archived_courses, in_process_course_actions = get_course_context(request) + + if not split_library_view_on_dashboard() and LIBRARIES_ENABLED and not no_course: + libraries = get_library_context(request, True)['libraries'] home_context = { 'courses': active_courses, @@ -1484,7 +1694,7 @@ def format_in_process_course_view(uca): 'archived_courses': archived_courses, 'in_process_course_actions': in_process_course_actions, 'libraries_enabled': LIBRARIES_ENABLED, - 'taxonomies_enabled': use_tagging_taxonomy_list_page(), + 'taxonomies_enabled': not is_tagging_feature_disabled(), 'redirect_to_library_authoring_mfe': should_redirect_to_library_authoring_mfe(), 'library_authoring_mfe_url': LIBRARY_AUTHORING_MICROFRONTEND_URL, 'taxonomy_list_mfe_url': get_taxonomy_list_url(), @@ -1614,6 +1824,372 @@ def get_course_videos_context(course_block, pagination_conf, course_key=None): return course_video_context +def get_course_index_context(request, course_key, course_block=None): + """ + Wrapper function to wrap _get_course_index_context in bulk operation + if course_block is None. + """ + if not course_block: + with modulestore().bulk_operations(course_key): + course_block = modulestore().get_course(course_key) + return _get_course_index_context(request, course_key, course_block) + return _get_course_index_context(request, course_key, course_block) + + +def _get_course_index_context(request, course_key, course_block): + """ + Utils is used to get context of course index outline. + It is used for both DRF and django views. + """ + + from cms.djangoapps.contentstore.views.course import ( + course_outline_initial_state, + _course_outline_json, + _deprecated_blocks_info, + ) + from openedx.core.djangoapps.content_staging import api as content_staging_api + + lms_link = get_lms_link_for_item(course_block.location) + reindex_link = None + if settings.FEATURES.get('ENABLE_COURSEWARE_INDEX', False): + if GlobalStaff().has_user(request.user): + reindex_link = f"/course/{str(course_key)}/search_reindex" + sections = course_block.get_children() + course_structure = _course_outline_json(request, course_block) + locator_to_show = request.GET.get('show', None) + + course_release_date = ( + get_default_time_display(course_block.start) + if course_block.start != DEFAULT_START_DATE + else _("Set Date") + ) + + settings_url = reverse_course_url('settings_handler', course_key) + + try: + current_action = CourseRerunState.objects.find_first(course_key=course_key, should_display=True) + except (ItemNotFoundError, CourseActionStateItemNotFoundError): + current_action = None + + deprecated_block_names = [block.name for block in deprecated_xblocks()] + deprecated_blocks_info = _deprecated_blocks_info(course_block, deprecated_block_names) + + frontend_app_publisher_url = configuration_helpers.get_value_for_org( + course_block.location.org, + 'FRONTEND_APP_PUBLISHER_URL', + settings.FEATURES.get('FRONTEND_APP_PUBLISHER_URL', False) + ) + # gather any errors in the currently stored proctoring settings. + advanced_dict = CourseMetadata.fetch(course_block) + proctoring_errors = CourseMetadata.validate_proctoring_settings(course_block, advanced_dict, request.user) + + user_clipboard = content_staging_api.get_user_clipboard_json(request.user.id, request) + course_block.discussions_settings['discussion_configuration_url'] = ( + f'{get_pages_and_resources_url(course_block.id)}/discussion/settings' + ) + + course_index_context = { + 'language_code': request.LANGUAGE_CODE, + 'context_course': course_block, + 'discussions_settings': course_block.discussions_settings, + 'lms_link': lms_link, + 'sections': sections, + 'course_structure': course_structure, + 'initial_state': course_outline_initial_state(locator_to_show, course_structure) if locator_to_show else None, # lint-amnesty, pylint: disable=line-too-long + 'initial_user_clipboard': user_clipboard, + 'rerun_notification_id': current_action.id if current_action else None, + 'course_release_date': course_release_date, + 'settings_url': settings_url, + 'reindex_link': reindex_link, + 'deprecated_blocks_info': deprecated_blocks_info, + 'notification_dismiss_url': reverse_course_url( + 'course_notifications_handler', + current_action.course_key, + kwargs={ + 'action_state_id': current_action.id, + }, + ) if current_action else None, + 'frontend_app_publisher_url': frontend_app_publisher_url, + 'mfe_proctored_exam_settings_url': get_proctored_exam_settings_url(course_block.id), + 'advance_settings_url': reverse_course_url('advanced_settings_handler', course_block.id), + 'proctoring_errors': proctoring_errors, + 'taxonomy_tags_widget_url': get_taxonomy_tags_widget_url(course_block.id), + } + + return course_index_context + + +def get_container_handler_context(request, usage_key, course, xblock): # pylint: disable=too-many-statements + """ + Utils is used to get context for container xblock requests. + It is used for both DRF and django views. + """ + + from cms.djangoapps.contentstore.views.component import ( + get_component_templates, + get_unit_tags, + CONTAINER_TEMPLATES, + LIBRARY_BLOCK_TYPES, + ) + from cms.djangoapps.contentstore.helpers import get_parent_xblock, is_unit + from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import ( + add_container_page_publishing_info, + create_xblock_info, + ) + from openedx.core.djangoapps.content_staging import api as content_staging_api + + course_sequence_ids = get_sequence_usage_keys(course) + component_templates = get_component_templates(course) + ancestor_xblocks = [] + parent = get_parent_xblock(xblock) + action = request.GET.get('action', 'view') + + is_unit_page = is_unit(xblock) + unit = xblock if is_unit_page else None + + is_first = True + block = xblock + + # Build the breadcrumbs and find the ``Unit`` ancestor + # if it is not the immediate parent. + while parent: + + if unit is None and is_unit(block): + unit = block + + # add all to nav except current xblock page + if xblock != block: + current_block = { + 'title': block.display_name_with_default, + 'children': parent.get_children(), + 'is_last': is_first + } + is_first = False + ancestor_xblocks.append(current_block) + + block = parent + parent = get_parent_xblock(parent) + + ancestor_xblocks.reverse() + + if unit is None: + raise ValueError("Could not determine unit page") + + subsection = get_parent_xblock(unit) + if subsection is None: + raise ValueError(f"Could not determine parent subsection from unit {unit.location}") + + section = get_parent_xblock(subsection) + if section is None: + raise ValueError(f"Could not determine ancestor section from unit {unit.location}") + + # for the sequence navigator + prev_url, next_url = get_sibling_urls(subsection, unit.location) + # these are quoted here because they'll end up in a query string on the page, + # and quoting with mako will trigger the xss linter... + prev_url = quote_plus(prev_url) if prev_url else None + next_url = quote_plus(next_url) if next_url else None + + show_unit_tags = not is_tagging_feature_disabled() + unit_tags = None + if show_unit_tags and is_unit_page: + unit_tags = get_unit_tags(usage_key) + + # Fetch the XBlock info for use by the container page. Note that it includes information + # about the block's ancestors and siblings for use by the Unit Outline. + xblock_info = create_xblock_info(xblock, include_ancestor_info=is_unit_page, tags=unit_tags) + + if is_unit_page: + add_container_page_publishing_info(xblock, xblock_info) + + # need to figure out where this item is in the list of children as the + # preview will need this + index = 1 + for child in subsection.get_children(): + if child.location == unit.location: + break + index += 1 + + # Get the status of the user's clipboard so they can paste components if they have something to paste + user_clipboard = content_staging_api.get_user_clipboard_json(request.user.id, request) + library_block_types = [problem_type['component'] for problem_type in LIBRARY_BLOCK_TYPES] + is_library_xblock = xblock.location.block_type in library_block_types + + context = { + 'language_code': request.LANGUAGE_CODE, + 'context_course': course, # Needed only for display of menus at top of page. + 'action': action, + 'xblock': xblock, + 'xblock_locator': xblock.location, + 'unit': unit, + 'is_unit_page': is_unit_page, + 'is_collapsible': is_library_xblock, + 'subsection': subsection, + 'section': section, + 'position': index, + 'prev_url': prev_url, + 'next_url': next_url, + 'new_unit_category': 'vertical', + 'outline_url': '{url}?format=concise'.format(url=reverse_course_url('course_handler', course.id)), + 'ancestor_xblocks': ancestor_xblocks, + 'component_templates': component_templates, + 'xblock_info': xblock_info, + 'templates': CONTAINER_TEMPLATES, + 'show_unit_tags': show_unit_tags, + # Status of the user's clipboard, exactly as would be returned from the "GET clipboard" REST API. + 'user_clipboard': user_clipboard, + 'is_fullwidth_content': is_library_xblock, + 'course_sequence_ids': course_sequence_ids, + } + return context + + +def get_textbooks_context(course): + """ + Utils is used to get context for textbooks for course. + It is used for both DRF and django views. + """ + + upload_asset_url = reverse_course_url('assets_handler', course.id) + textbook_url = reverse_course_url('textbooks_list_handler', course.id) + return { + 'context_course': course, + 'textbooks': course.pdf_textbooks, + 'upload_asset_url': upload_asset_url, + 'textbook_url': textbook_url, + } + + +def get_certificates_context(course, user): + """ + Utils is used to get context for container xblock requests. + It is used for both DRF and django views. + """ + + from cms.djangoapps.contentstore.views.certificates import CertificateManager + + course_key = course.id + certificate_url = reverse_course_url('certificates_list_handler', course_key) + course_outline_url = reverse_course_url('course_handler', course_key) + upload_asset_url = reverse_course_url('assets_handler', course_key) + activation_handler_url = reverse_course_url( + handler_name='certificate_activation_handler', + course_key=course_key + ) + course_modes = [ + mode.slug for mode in CourseMode.modes_for_course( + course_id=course_key, include_expired=True + ) if mode.slug != 'audit' + ] + + has_certificate_modes = len(course_modes) > 0 + + if has_certificate_modes: + certificate_web_view_url = get_lms_link_for_certificate_web_view( + course_key=course_key, + mode=course_modes[0] # CourseMode.modes_for_course returns default mode if doesn't find anyone. + ) + else: + certificate_web_view_url = None + + is_active, certificates = CertificateManager.is_activated(course) + context = { + 'context_course': course, + 'certificate_url': certificate_url, + 'course_outline_url': course_outline_url, + 'upload_asset_url': upload_asset_url, + 'certificates': certificates, + 'has_certificate_modes': has_certificate_modes, + 'course_modes': course_modes, + 'certificate_web_view_url': certificate_web_view_url, + 'is_active': is_active, + 'is_global_staff': GlobalStaff().has_user(user), + 'certificate_activation_handler_url': activation_handler_url, + 'mfe_proctored_exam_settings_url': get_proctored_exam_settings_url(course_key), + } + + return context + + +def get_group_configurations_context(course, store): + """ + Utils is used to get context for course's group configurations. + It is used for both DRF and django views. + """ + + from cms.djangoapps.contentstore.course_group_config import ( + COHORT_SCHEME, ENROLLMENT_SCHEME, GroupConfiguration, RANDOM_SCHEME + ) + from cms.djangoapps.contentstore.views.course import ( + are_content_experiments_enabled + ) + from xmodule.partitions.partitions import UserPartition # lint-amnesty, pylint: disable=wrong-import-order + + course_key = course.id + group_configuration_url = reverse_course_url('group_configurations_list_handler', course_key) + course_outline_url = reverse_course_url('course_handler', course_key) + should_show_experiment_groups = are_content_experiments_enabled(course) + if should_show_experiment_groups: + experiment_group_configurations = GroupConfiguration.get_split_test_partitions_with_usage(store, course) + else: + experiment_group_configurations = None + + all_partitions = GroupConfiguration.get_all_user_partition_details(store, course) + should_show_enrollment_track = False + has_content_groups = False + displayable_partitions = [] + for partition in all_partitions: + partition['read_only'] = getattr(UserPartition.get_scheme(partition['scheme']), 'read_only', False) + + if partition['scheme'] == COHORT_SCHEME: + has_content_groups = True + displayable_partitions.append(partition) + elif partition['scheme'] == CONTENT_TYPE_GATING_SCHEME: + # Add it to the front of the list if it should be shown. + if ContentTypeGatingConfig.current(course_key=course_key).studio_override_enabled: + displayable_partitions.append(partition) + elif partition['scheme'] == ENROLLMENT_SCHEME: + should_show_enrollment_track = len(partition['groups']) > 1 + + # Add it to the front of the list if it should be shown. + if should_show_enrollment_track: + displayable_partitions.insert(0, partition) + elif partition['scheme'] == TEAM_SCHEME: + should_show_team_partitions = len(partition['groups']) > 0 and CONTENT_GROUPS_FOR_TEAMS.is_enabled( + course_key + ) + if should_show_team_partitions: + displayable_partitions.append(partition) + elif partition['scheme'] != RANDOM_SCHEME: + # Experiment group configurations are handled explicitly above. We don't + # want to display their groups twice. + displayable_partitions.append(partition) + + # Set the sort-order. Higher numbers sort earlier + scheme_priority = defaultdict(lambda: -1, { + ENROLLMENT_SCHEME: 1, + CONTENT_TYPE_GATING_SCHEME: 0 + }) + displayable_partitions.sort(key=lambda p: scheme_priority[p['scheme']], reverse=True) + # Add empty content group if there is no COHORT User Partition in the list. + # This will add ability to add new groups in the view. + if not has_content_groups: + displayable_partitions.append(GroupConfiguration.get_or_create_content_group(store, course)) + + context = { + 'context_course': course, + 'group_configuration_url': group_configuration_url, + 'course_outline_url': course_outline_url, + 'experiment_group_configurations': experiment_group_configurations, + 'should_show_experiment_groups': should_show_experiment_groups, + 'all_group_configurations': displayable_partitions, + 'should_show_enrollment_track': should_show_enrollment_track, + 'mfe_proctored_exam_settings_url': get_proctored_exam_settings_url(course.id), + } + + return context + + class StudioPermissionsService: """ Service that can provide information about a user's permissions. @@ -1633,3 +2209,111 @@ def can_read(self, course_key): def can_write(self, course_key): """ Does the user have read access to the given course/library? """ return has_studio_write_access(self._user, course_key) + + +def track_course_update_event(course_key, user, course_update_content=None): + """ + Track course update event + """ + event_name = 'edx.contentstore.course_update' + event_data = {} + html_content = course_update_content.get("content", "") + str_content = re.sub(r"(\s| |//)+", " ", html_to_text(html_content)) + + event_data['content'] = str_content + event_data['date'] = course_update_content.get("date", "") + event_data['id'] = course_update_content.get("id", "") + event_data['status'] = course_update_content.get("status", "") + event_data['course_id'] = str(course_key) + event_data['user_id'] = str(user.id) + event_data['user_forums_roles'] = [ + role.name for role in user.roles.filter(course_id=str(course_key)) + ] + event_data['user_course_roles'] = [ + role.role for role in user.courseaccessrole_set.filter(course_id=str(course_key)) + ] + + context = contexts.course_context_from_course_id(course_key) + with tracker.get_tracker().context(event_name, context): + tracker.emit(event_name, event_data) + + +def send_course_update_notification(course_key, content, user): + """ + Send course update notification + """ + text_content = re.sub(r"(\s| |//)+", " ", html_to_text(content)) + course = modulestore().get_course(course_key) + extra_context = { + 'author_id': user.id, + 'course_name': course.display_name, + } + notification_data = CourseNotificationData( + course_key=course_key, + content_context={ + "course_update_content": text_content if len(text_content.strip()) < 10 else "Click here to view", + **extra_context, + }, + notification_type="course_update", + content_url=f"{settings.LMS_ROOT_URL}/courses/{str(course_key)}/course/updates", + app_name="updates", + audience_filters={}, + ) + COURSE_NOTIFICATION_REQUESTED.send_event(course_notification_data=notification_data) + + +def get_xblock_validation_messages(xblock): + """ + Retrieves validation messages for a given xblock. + + Args: + xblock: The xblock object to validate. + + Returns: + list: A list of validation error messages. + """ + validation_json = xblock.validate().to_json() + return validation_json['messages'] + + +def get_xblock_render_error(request, xblock): + """ + Checks if there are any rendering errors for a given block and return these. + + Args: + request: WSGI request object + xblock: The xblock object to rendering. + + Returns: + str: Error message which happened while rendering of xblock. + """ + from cms.djangoapps.contentstore.views.preview import _load_preview_block + from xmodule.studio_editable import has_author_view + from xmodule.x_module import AUTHOR_VIEW, STUDENT_VIEW + + def get_xblock_render_context(request, block): + """ + Return a dict of the data needs for render of each block. + """ + can_edit = has_studio_write_access(request.user, block.usage_key.course_key) + + return { + "is_unit_page": False, + "can_edit": can_edit, + "root_xblock": xblock, + "reorderable_items": set(), + "paging": None, + "force_render": None, + "item_url": "/container/{block.location}", + "tags_count_map": {}, + } + + try: + block = _load_preview_block(request, xblock) + preview_view = AUTHOR_VIEW if has_author_view(block) else STUDENT_VIEW + render_context = get_xblock_render_context(request, block) + block.render(preview_view, render_context) + except Exception as exc: # pylint: disable=broad-except + return str(exc) + + return "" diff --git a/cms/lib/xblock/authoring_mixin.py b/cms/lib/xblock/authoring_mixin.py index d66f9f88cd0e..acc11d48a260 100644 --- a/cms/lib/xblock/authoring_mixin.py +++ b/cms/lib/xblock/authoring_mixin.py @@ -26,6 +26,7 @@ class AuthoringMixin(XBlockMixin): """ Mixin class that provides authoring capabilities for XBlocks. """ + def _get_studio_resource_url(self, relative_url): """ Returns the Studio URL to a static resource. @@ -70,97 +71,3 @@ def post_editor_saved(self, user, old_metadata, old_content) -> None: # pylint: By default, is a no-op. Can be overriden in subclasses. """ - - def studio_duplicate( - self, - parent_usage_key, - duplicate_source_usage_key, - user, - store, - dest_usage_key=None, - display_name=None, - shallow=False, - is_child=False, - ): - """ - Duplicate an existing xblock as a child of the supplied parent_usage_key. You can - optionally specify what usage key the new duplicate block will use via dest_usage_key. - - If shallow is True, does not copy children. - """ - from cms.djangoapps.contentstore.utils import gather_block_attributes, load_services_for_studio - - if not dest_usage_key: - # Change the blockID to be unique. - dest_usage_key = self.location.replace(name=uuid4().hex) - - category = dest_usage_key.block_type - - duplicate_metadata, asides_to_create = gather_block_attributes( - self, - display_name=display_name, - is_child=is_child, - ) - - dest_block = store.create_item( - user.id, - dest_usage_key.course_key, - dest_usage_key.block_type, - block_id=dest_usage_key.block_id, - definition_data=self.get_explicitly_set_fields_by_scope(Scope.content), - metadata=duplicate_metadata, - runtime=self.runtime, - asides=asides_to_create, - ) - - # Allow an XBlock to do anything fancy it may need to when duplicated from another block. - load_services_for_studio(self.runtime, user) - dest_block.studio_post_duplicate(self, store, user, shallow=shallow) - # pylint: disable=protected-access - if "detached" not in self.runtime.load_block_type(category)._class_tags: - parent = store.get_item(parent_usage_key) - # If source was already a child of the parent, add duplicate immediately afterward. - # Otherwise, add child to end. - if self.location in parent.children: - source_index = parent.children.index(self.location) - parent.children.insert(source_index + 1, dest_block.location) - else: - parent.children.append(dest_block.location) - store.update_item(parent, user.id) - - # .. event_implemented_name: XBLOCK_DUPLICATED - XBLOCK_DUPLICATED.send_event( - time=datetime.now(timezone.utc), - xblock_info=DuplicatedXBlockData( - usage_key=dest_block.location, - block_type=dest_block.location.block_type, - source_usage_key=duplicate_source_usage_key, - ), - ) - - return dest_block.location - - def studio_post_duplicate( - self, - source_block, - store, - user, - shallow: bool, - ) -> None: # pylint: disable=unused-argument - """ - Called when after a block is duplicated. Can be used, e.g., for special handling of child duplication. - - Children must always be handled. In case of inheritance it can be done by running this method with super(). - - By default, implements standard duplication logic. - """ - if not source_block.has_children or shallow: - return - - self.children = self.children or [] - for child in source_block.children: - child_block = store.get_item(child) - dupe = child_block.studio_duplicate(self.location, child, user=user, store=store, is_child=True) - if dupe not in self.children: # studio_duplicate may add the child for us. - self.children.append(dupe) - store.update_item(self, user.id) diff --git a/xmodule/library_content_block.py b/xmodule/library_content_block.py index 6ba986034a49..09a5d1dee13e 100644 --- a/xmodule/library_content_block.py +++ b/xmodule/library_content_block.py @@ -9,7 +9,7 @@ from copy import copy from gettext import ngettext, gettext -import bleach +import nh3 from django.conf import settings from django.core.exceptions import ObjectDoesNotExist, PermissionDenied from django.utils.functional import classproperty @@ -221,7 +221,7 @@ def make_selection(cls, selected, children, max_count, mode): overlimit_block_keys = set() if len(selected_keys) > max_count: num_to_remove = len(selected_keys) - max_count - overlimit_block_keys = set(rand.sample(selected_keys, num_to_remove)) + overlimit_block_keys = set(rand.sample(list(selected_keys), num_to_remove)) selected_keys -= overlimit_block_keys # Do we have enough blocks now? @@ -233,7 +233,7 @@ def make_selection(cls, selected, children, max_count, mode): pool = valid_block_keys - selected_keys if mode == "random": num_to_add = min(len(pool), num_to_add) - added_block_keys = set(rand.sample(pool, num_to_add)) + added_block_keys = set(rand.sample(list(pool), num_to_add)) # We now have the correct n random children to show for this user. else: raise NotImplementedError("Unsupported mode.") @@ -451,7 +451,12 @@ def author_view(self, context): context['is_loading'] = is_updating # The following JS is used to make the "Update now" button work on the unit page and the container view: - fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/library_content_edit.js')) + if root_xblock and 'library' in root_xblock.category: + if root_xblock.source_library_id and len(root_xblock.children) > 0: + fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/library_content_edit.js')) + else: + fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/library_content_edit.js')) + fragment.initialize_js('LibraryContentAuthorView') return fragment @@ -500,7 +505,6 @@ def get_user_id(self): """ user_service = self.runtime.service(self, 'user') if user_service: - # May be None when creating bok choy test fixtures user_id = user_service.get_current_user().opt_attrs.get('edx-platform.user_id', None) else: user_id = None @@ -587,20 +591,29 @@ def children_are_syncing(self, request, suffix=''): # pylint: disable=unused-ar is_updating = False return Response(json.dumps(is_updating)) - def studio_post_duplicate( - self, - source_block, - *_args, - **__kwargs, - ) -> None: + def studio_post_duplicate(self, store, source_block): """ Used by the studio after basic duplication of a source block. We handle the children ourselves, because we have to properly reference the library upstream and set the overrides. Otherwise we'll end up losing data on the next refresh. """ + if hasattr(super(), 'studio_post_duplicate'): + super().studio_post_duplicate(store, source_block) + self._validate_sync_permissions() self.get_tools(to_read_library_content=True).trigger_duplication(source_block=source_block, dest_block=self) + return True # Children have been handled. + + def studio_post_paste(self, store, source_node) -> bool: + """ + Pull the children from the library and let library_tools assign their IDs. + """ + if hasattr(super(), 'studio_post_paste'): + super().studio_post_paste(store, source_node) + + self.sync_from_library(upgrade_to_latest=False) + return True # Children have been handled def _validate_library_version(self, validation, lib_tools, version, library_key): """ @@ -718,7 +731,7 @@ def source_library_values(self): lib_tools = self.get_tools() user_perms = self.runtime.service(self, 'studio_user_permissions') all_libraries = [ - (key, bleach.clean(name)) for key, name in lib_tools.list_available_libraries() + (key, nh3.clean(name)) for key, name in lib_tools.list_available_libraries() if user_perms.can_read(key) or self.source_library_id == str(key) ] all_libraries.sort(key=lambda entry: entry[1]) # Sort by name From 9c853999a80f6826709f1a6266912f5218fa60af Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Mon, 8 Jul 2024 03:46:11 -0400 Subject: [PATCH 15/15] style: fix pylint unused imports --- cms/lib/xblock/authoring_mixin.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/cms/lib/xblock/authoring_mixin.py b/cms/lib/xblock/authoring_mixin.py index acc11d48a260..b9057391b18b 100644 --- a/cms/lib/xblock/authoring_mixin.py +++ b/cms/lib/xblock/authoring_mixin.py @@ -4,15 +4,11 @@ import logging -from datetime import datetime, timezone -from uuid import uuid4 from django.conf import settings from web_fragments.fragment import Fragment from xblock.core import XBlock, XBlockMixin from xblock.fields import String, Scope -from openedx_events.content_authoring.data import DuplicatedXBlockData -from openedx_events.content_authoring.signals import XBLOCK_DUPLICATED log = logging.getLogger(__name__)