From 805a2a820cc6bc673b277843a74c31dc49721ccd Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Fri, 17 Mar 2023 01:01:12 -0700 Subject: [PATCH 01/16] feat: Implement paste button --- cms/djangoapps/contentstore/views/block.py | 11 +++ .../contentstore/views/component.py | 15 +++- cms/djangoapps/contentstore/views/helpers.py | 79 +++++++++++++++++ cms/static/js/views/pages/container.js | 69 ++++++++++++++- cms/static/sass/elements/_modules.scss | 16 ++++ cms/templates/container.html | 3 +- .../studio_render_children_view.html | 7 ++ .../core/djangoapps/content_staging/api.py | 85 ++++++++++++++++++- .../djangoapps/content_staging/serializers.py | 2 +- xmodule/modulestore/split_mongo/split.py | 8 +- 10 files changed, 287 insertions(+), 8 deletions(-) diff --git a/cms/djangoapps/contentstore/views/block.py b/cms/djangoapps/contentstore/views/block.py index 7c28f005011b..ea403a7c551c 100644 --- a/cms/djangoapps/contentstore/views/block.py +++ b/cms/djangoapps/contentstore/views/block.py @@ -72,6 +72,7 @@ from .helpers import ( create_xblock, get_parent_xblock, + import_staged_content_from_user_clipboard, is_unit, usage_key_with_run, xblock_primary_child_category, @@ -169,6 +170,8 @@ def xblock_handler(request, usage_key_string=None): :display_name: name for new xblock, optional :boilerplate: template name for populating fields, optional and only used if duplicate_source_locator is not present + :staged_content: use "clipboard" to paste from the OLX user's clipboard. (Incompatible with all other + fields except parent_locator) The locator (unicode representation of a UsageKey) for the created xblock (minus children) is returned. """ if usage_key_string: @@ -701,6 +704,14 @@ def _create_block(request): if not has_studio_write_access(request.user, usage_key.course_key): raise PermissionDenied() + if request.json.get('staged_content') == "clipboard": + created_xblock = import_staged_content_from_user_clipboard(parent_key=usage_key, user=request.user) + if created_xblock is None: + return HttpResponseBadRequest("User clipboard empty or invalid", content_type="text/plain") + return JsonResponse( + {'locator': str(created_xblock.location), 'courseKey': str(created_xblock.location.course_key)} + ) + category = request.json['category'] if isinstance(usage_key, LibraryUsageLocator): # Only these categories are supported at this time. diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 7f774ab161d2..19dfd6d64166 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -27,6 +27,11 @@ from cms.djangoapps.contentstore.toggles import use_new_problem_editor from openedx.core.lib.xblock_utils import get_aside_from_xblock, is_xblock_aside from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration +try: + # Technically this is a django app plugin, so we should not error if it's not installed: + import openedx.core.djangoapps.content_staging.api as content_staging_api +except ImportError: + content_staging_api = None 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 @@ -185,6 +190,12 @@ def container_handler(request, usage_key_string): break index += 1 + # Get the status of the user's clipboard so they can paste components if they have something to paste + if content_staging_api: + user_clipboard = content_staging_api.get_user_clipboard_status_json(request.user.id, request) + else: + user_clipboard = {"content": None} + return render_to_response('container.html', { 'language_code': request.LANGUAGE_CODE, 'context_course': course, # Needed only for display of menus at top of page. @@ -205,7 +216,9 @@ def container_handler(request, usage_key_string): 'xblock_info': xblock_info, 'draft_preview_link': preview_lms_link, 'published_preview_link': lms_link, - 'templates': CONTAINER_TEMPLATES + 'templates': CONTAINER_TEMPLATES, + # Status of the user's clipboard, exactly as would be returned from the "GET clipboard" REST API. + 'user_clipboard': user_clipboard, }) else: return HttpResponseBadRequest("Only supports HTML requests") diff --git a/cms/djangoapps/contentstore/views/helpers.py b/cms/djangoapps/contentstore/views/helpers.py index 398f0f6c6311..faafab03e7c1 100644 --- a/cms/djangoapps/contentstore/views/helpers.py +++ b/cms/djangoapps/contentstore/views/helpers.py @@ -3,12 +3,16 @@ """ import urllib +from lxml import etree from uuid import uuid4 from django.http import HttpResponse from django.utils.translation import gettext as _ from opaque_keys.edx.keys import UsageKey +from opaque_keys.edx.locator import DefinitionLocator, LocalId from xblock.core import XBlock +from xblock.fields import ScopeIds +from xblock.runtime import IdGenerator from xmodule.modulestore.django import modulestore from xmodule.tabs import StaticTab @@ -17,6 +21,12 @@ from common.djangoapps.student.roles import CourseCreatorRole, OrgContentCreatorRole from openedx.core.toggles import ENTRANCE_EXAMS +try: + # Technically this is a django app plugin, so we should not error if it's not installed: + import openedx.core.djangoapps.content_staging.api as content_staging_api +except ImportError: + content_staging_api = None + from ..utils import reverse_course_url, reverse_library_url, reverse_usage_url __all__ = ['event'] @@ -271,6 +281,75 @@ def create_xblock(parent_locator, user, category, display_name, boilerplate=None return created_block +class ImportIdGenerator(IdGenerator): + """ + Modulestore's IdGenerator doesn't work for importing single blocks as OLX, + so we implement our own + """ + def __init__(self, context_key): + super().__init__() + self.context_key = context_key + + def create_aside(self, definition_id, usage_id, aside_type): + """ Generate a new aside key """ + raise NotImplementedError() + + def create_usage(self, def_id) -> UsageKey: + """ Generate a new UsageKey for an XBlock """ + # Note: Split modulestore will detect this temporary ID and create a new block ID when the XBlock is saved. + return self.context_key.make_usage_key(def_id.block_type, LocalId()) + + def create_definition(self, block_type, slug=None) -> DefinitionLocator: + """ Generate a new definition_id for an XBlock """ + # Note: Split modulestore will detect this temporary ID and create a new definition ID when the XBlock is saved. + return DefinitionLocator(block_type, LocalId(block_type)) + + +def import_staged_content_from_user_clipboard(parent_key: UsageKey, user): + """ + Import a block (and any children it has) from "staged" OLX. + Does not deal with permissions or REST stuff - do that before calling this. + + Returns the newly created block on success or None if the clipboard is + empty. + """ + if not content_staging_api: + raise RuntimeError("The required content_staging app is not installed") + user_clipboard = content_staging_api.get_user_clipboard_status(user.id) + if ( + not user_clipboard or + user_clipboard.content.status != content_staging_api.StagedContentStatus.READY + ): + # Clipboard is empty or expired/error/loading + return None + source_usage_key = user_clipboard.source_usage_key # TODO: track this somewhere + block_type = user_clipboard.content.block_type + olx_str = content_staging_api.get_staged_content_olx(user_clipboard.content.id) + node = etree.fromstring(olx_str) + store = modulestore() + with store.bulk_operations(parent_key.course_key): + parent_xblock = store.get_item(parent_key) + runtime = parent_xblock.runtime + # Generate the new ID: + id_generator = ImportIdGenerator(parent_key.context_key) + def_id = id_generator.create_definition(block_type, user_clipboard.source_usage_key.block_id) + usage_id = id_generator.create_usage(def_id) + keys = ScopeIds(None, block_type, def_id, usage_id) + # parse_xml is a really messy API. We pass both 'keys' and 'id_generator' and, depending on the XBlock, either + # one may be used to determine the new XBlock's usage key, and the other will be ignored. e.g. video ignores + # 'keys' and uses 'id_generator', but the default XBlock parse_xml ignores 'id_generator' and uses 'keys'. + # For children of this block, obviously only id_generator is used. + xblock_class = runtime.load_block_type(block_type) + temp_xblock = xblock_class.parse_xml(node, runtime, keys, id_generator) + if xblock_class.has_children and temp_xblock.children: + raise NotImplementedError("We don't yet support pasting XBlocks with children") + temp_xblock.parent = parent_key + new_xblock = store.update_item(temp_xblock, user.id, allow_not_found=True) + parent_xblock.children.append(new_xblock.location) + store.update_item(parent_xblock, user.id) + return new_xblock + + def is_item_in_course_tree(item): """ Check that the item is in the course tree. diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 15c92fac6564..10da7b94b583 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -22,12 +22,14 @@ function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView 'click .move-button': 'showMoveXBlockModal', 'click .delete-button': 'deleteXBlock', 'click .show-actions-menu-button': 'showXBlockActionsMenu', - 'click .new-component-button': 'scrollToNewComponentButtons' + 'click .new-component-button': 'scrollToNewComponentButtons', + 'click .paste-component-button': 'pasteComponent', }, options: { collapsedClass: 'is-collapsed', - canEdit: true // If not specified, assume user has permission to make changes + canEdit: true, // If not specified, assume user has permission to make changes + clipboardData: { content: null }, }, view: 'container_preview', @@ -100,6 +102,7 @@ function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView } this.listenTo(Backbone, 'move:onXBlockMoved', this.onXBlockMoved); + this.clipboardBroadcastChannel = new BroadcastChannel("studio_clipboard_channel"); }, getViewParameters: function() { @@ -147,6 +150,11 @@ function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView // Re-enable Backbone events for any updated DOM elements self.delegateEvents(); + + // Show/hide the paste button + if (!self.isLibraryPage) { + self.initializePasteButton(); + } }, block_added: options && options.block_added }); @@ -182,6 +190,59 @@ function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView } }, + initializePasteButton() { + if (this.options.canEdit) { + // We should have the user's clipboard status. + const data = this.options.clipboardData; + this.refreshPasteButton(data); + // Refresh the status when something is copied on another tab: + this.clipboardBroadcastChannel.onmessage = (event) => { this.refreshPasteButton(event.data); }; + } else { + this.$(".paste-component").hide(); + } + }, + + /** + * Given the latest information about the user's clipboard, hide or show the Paste button as appropriate. + */ + refreshPasteButton(data) { + // 'data' is the same data returned by the "get clipboard status" API endpoint + // i.e. /api/content-staging/v1/clipboard/ + if (this.options.canEdit && data.content) { + // TODO: check if this is suitable for pasting into a unit + this.$(".paste-component").show(); + } else { + this.$(".paste-component").hide(); + } + }, + + /** The user has clicked on the "Paste Component button" */ + pasteComponent(event) { + event.preventDefault(); + // Get the ID of the container (usually a unit/vertical) that we're pasting into: + const parentElement = this.findXBlockElement(event.target); + const parentLocator = parentElement.data('locator'); + // Create a placeholder XBlock while we're pasting: + const $placeholderEl = $(this.createPlaceholderElement()); + const addComponentsPanel = $(event.target).closest('.paste-component').prev(); + const listPanel = addComponentsPanel.prev(); + const scrollOffset = ViewUtils.getScrollOffset(addComponentsPanel); + const placeholderElement = $placeholderEl.appendTo(listPanel); + + // Start showing a "Pasting" notification: + ViewUtils.runOperationShowingMessage(gettext('Pasting'), () => { + return $.postJSON(this.getURLRoot() + '/', { + parent_locator: parentLocator, + staged_content: "clipboard", + }).then((data) => { + this.onNewXBlock(placeholderElement, scrollOffset, false, data); + }).fail(() => { + // Remove the placeholder if the paste failed + placeholderElement.remove(); + }); + }); + }, + editXBlock: function(event, options) { event.preventDefault(); @@ -307,6 +368,8 @@ function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView const status = data.content?.status; if (status === "ready") { // The XBlock has been copied and is ready to use. + this.refreshPasteButton(data); // Update our UI + this.clipboardBroadcastChannel.postMessage(data); // And notify any other open tabs return data; } else if (status === "loading") { // The clipboard is being loaded asynchonously. @@ -316,6 +379,8 @@ function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView $.getJSON(clipboardEndpoint, (pollData) => { const newStatus = pollData.content?.status; if (newStatus === "ready") { + this.refreshPasteButton(data); + this.clipboardBroadcastChannel.postMessage(pollData); deferred.resolve(pollData); } else if (newStatus === "loading") { setTimeout(checkStatus, 1_000); diff --git a/cms/static/sass/elements/_modules.scss b/cms/static/sass/elements/_modules.scss index 9859da0b2d9a..142aea5f4e3f 100644 --- a/cms/static/sass/elements/_modules.scss +++ b/cms/static/sass/elements/_modules.scss @@ -320,6 +320,22 @@ } } } +// New "Paste component" menu, shown on the Unit page to users with a component in their clipboard +.paste-component { + margin: $baseline ($baseline/2); + + .paste-component-button { + display: block; + width: 100%; + // Override what we're extending from ui-btn-flat-outline: + &.button { + font-size: 1.5rem; + padding: 10px 0; + } + + @extend %ui-btn-flat-outline; + } +} // outline UI // -------------------- diff --git a/cms/templates/container.html b/cms/templates/container.html index d1a743b128e6..49d8903b9311 100644 --- a/cms/templates/container.html +++ b/cms/templates/container.html @@ -48,7 +48,8 @@ { isUnitPage: ${is_unit_page | n, dump_js_escaped_json}, canEdit: true, - outlineURL: "${outline_url | n, js_escaped_string}" + outlineURL: "${outline_url | n, js_escaped_string}", + clipboardData: ${user_clipboard | n, dump_js_escaped_json}, } ); require(["js/models/xblock_info", "js/views/xblock", "js/views/utils/xblock_utils", "common/js/components/utils/view_utils"], function (XBlockInfo, XBlockView, XBlockUtils, ViewUtils) { diff --git a/lms/templates/studio_render_children_view.html b/lms/templates/studio_render_children_view.html index 2050631c7fa9..aab3d1fa7f0d 100644 --- a/lms/templates/studio_render_children_view.html +++ b/lms/templates/studio_render_children_view.html @@ -1,3 +1,4 @@ +<%! from django.utils.translation import gettext as _ %> % if can_reorder:
    % endif @@ -9,4 +10,10 @@ % endif % if can_add:
    + % endif diff --git a/openedx/core/djangoapps/content_staging/api.py b/openedx/core/djangoapps/content_staging/api.py index e92c5a037862..7c6feecb1205 100644 --- a/openedx/core/djangoapps/content_staging/api.py +++ b/openedx/core/djangoapps/content_staging/api.py @@ -1,4 +1,87 @@ """ Public python API for content staging """ -# Currently, there is no public API. +from __future__ import annotations +from datetime import datetime +from typing import NamedTuple + +from django.http import HttpRequest +from opaque_keys.edx.keys import UsageKey + +from .models import UserClipboard as _UserClipboard, StagedContent as _StagedContent +from .serializers import UserClipboardSerializer as _UserClipboardSerializer + + +StagedContentStatus = _StagedContent.Status +CLIPBOARD_PURPOSE = _UserClipboard.PURPOSE + + +class StagedContentData(NamedTuple): + """ Read-only data model for StagedContent """ + id: int + user_id: int + created: datetime + purpose: str + status: StagedContentStatus + block_type: str + display_name: str + + +class UserClipboardData(NamedTuple): + """ Read-only data model for StagedContent """ + content: StagedContentData + source_usage_key: UsageKey + + +def get_user_clipboard_status(user_id: int) -> UserClipboardData: + """ Get the detailed status of the user's clipboard. """ + try: + clipboard = _UserClipboard.objects.get(user_id=user_id) + except _UserClipboard.DoesNotExist: + # This user does not have any content on their clipboard. + return None + content = clipboard.content + return UserClipboardData( + content=StagedContentData( + id=content.id, + user_id=content.user_id, + created=content.created, + purpose=content.purpose, + status=content.status, + block_type=content.block_type, + display_name=content.display_name, + ), + source_usage_key=clipboard.source_usage_key, + ) + + +def get_user_clipboard_status_json(user_id: int, request: HttpRequest = None): + """ + Get the detailed status of the user's clipboard. + This is _exactly_ the same format as returned from the + /api/content-staging/v1/clipboard/ + API endpoint. This does not return the OLX. + + (request is optional; including it will make the "olx_url" absolute instead + of relative.) + """ + try: + clipboard = _UserClipboard.objects.get(user_id=user_id) + except _UserClipboard.DoesNotExist: + # This user does not have any content on their clipboard. + return {"content": None, "source_usage_key": "", "source_context_title": ""} + serializer = _UserClipboardSerializer(clipboard, context={'request': request}) + return serializer.data + + +def get_staged_content_olx(staged_content_id: int) -> str | None: + """ + Get the OLX (as a string) for the given StagedContent. + + Does not check permissions! + """ + try: + sc = _StagedContent.objects.get(pk=staged_content_id) + return sc.olx + except _StagedContent.DoesNotExist: + return None diff --git a/openedx/core/djangoapps/content_staging/serializers.py b/openedx/core/djangoapps/content_staging/serializers.py index 2a765414d30b..f91e58a87160 100644 --- a/openedx/core/djangoapps/content_staging/serializers.py +++ b/openedx/core/djangoapps/content_staging/serializers.py @@ -16,7 +16,7 @@ class Meta: model = StagedContent fields = [ 'id', - 'user', + 'user_id', 'created', 'purpose', 'status', diff --git a/xmodule/modulestore/split_mongo/split.py b/xmodule/modulestore/split_mongo/split.py index 8e7999ae75bc..85f46bd31ce9 100644 --- a/xmodule/modulestore/split_mongo/split.py +++ b/xmodule/modulestore/split_mongo/split.py @@ -1953,9 +1953,12 @@ def update_item(self, descriptor, user_id, allow_not_found=False, force=False, * the definition, structure, nor course if they didn't change. """ partitioned_fields = self.partition_xblock_fields_by_scope(descriptor) + definition_locator = getattr(descriptor, "definition_locator", None) + if definition_locator is None and not allow_not_found: + raise AttributeError("descriptor is missing expected definition_locator from caching descriptor system") return self._update_item_from_fields( user_id, descriptor.location.course_key, BlockKey.from_usage_key(descriptor.location), - partitioned_fields, descriptor.definition_locator, allow_not_found, force, **kwargs + partitioned_fields, definition_locator, allow_not_found, force, **kwargs ) or descriptor def _update_item_from_fields(self, user_id, course_key, block_key, partitioned_fields, # pylint: disable=too-many-statements @@ -2162,7 +2165,8 @@ def _persist_subdag(self, course_key, xblock, user_id, structure_blocks, new_id) partitioned_fields = self.partition_xblock_fields_by_scope(xblock) new_def_data = self._serialize_fields(xblock.category, partitioned_fields[Scope.content]) is_updated = False - if xblock.definition_locator is None or isinstance(xblock.definition_locator.definition_id, LocalId): + current_definition_locator = getattr(xblock, "definition_locator", xblock.scope_ids.def_id) + if current_definition_locator is None or isinstance(current_definition_locator.definition_id, LocalId): xblock.definition_locator = self.create_definition_from_data( course_key, new_def_data, xblock.category, user_id ) From 6e11e054383dc2515e530d9685f303fb99f72e77 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Wed, 19 Apr 2023 17:42:07 -0700 Subject: [PATCH 02/16] chore: improve docs and add tests for python API --- .../core/djangoapps/content_staging/api.py | 11 +-- .../content_staging/tests/test_clipboard.py | 68 ++++++++++++++----- 2 files changed, 59 insertions(+), 20 deletions(-) diff --git a/openedx/core/djangoapps/content_staging/api.py b/openedx/core/djangoapps/content_staging/api.py index 7c6feecb1205..a5b41c5c30a3 100644 --- a/openedx/core/djangoapps/content_staging/api.py +++ b/openedx/core/djangoapps/content_staging/api.py @@ -33,7 +33,7 @@ class UserClipboardData(NamedTuple): source_usage_key: UsageKey -def get_user_clipboard_status(user_id: int) -> UserClipboardData: +def get_user_clipboard_status(user_id: int) -> UserClipboardData | None: """ Get the detailed status of the user's clipboard. """ try: clipboard = _UserClipboard.objects.get(user_id=user_id) @@ -57,10 +57,13 @@ def get_user_clipboard_status(user_id: int) -> UserClipboardData: def get_user_clipboard_status_json(user_id: int, request: HttpRequest = None): """ - Get the detailed status of the user's clipboard. - This is _exactly_ the same format as returned from the + Get the detailed status of the user's clipboard, in exactly the same format + as returned from the /api/content-staging/v1/clipboard/ - API endpoint. This does not return the OLX. + REST API endpoint. This version of the API is meant for "preloading" that + REST API endpoint so it can be embedded in a larger response sent to the + user's browser. If you just want to get the clipboard data from python, use + get_user_clipboard_status() instead, since it's fully typed. (request is optional; including it will make the "olx_url" absolute instead of relative.) diff --git a/openedx/core/djangoapps/content_staging/tests/test_clipboard.py b/openedx/core/djangoapps/content_staging/tests/test_clipboard.py index 1313f05d7ce6..7d27aa555379 100644 --- a/openedx/core/djangoapps/content_staging/tests/test_clipboard.py +++ b/openedx/core/djangoapps/content_staging/tests/test_clipboard.py @@ -6,12 +6,26 @@ from rest_framework.test import APIClient from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase - from xmodule.modulestore.tests.factories import ToyCourseFactory +from openedx.core.djangoapps.content_staging import api as python_api + CLIPBOARD_ENDPOINT = "/api/content-staging/v1/clipboard/" +# OLX of the video in the toy course using course_key.make_usage_key("video", "sample_video") +SAMPLE_VIDEO_OLX = """ +