Skip to content

Commit

Permalink
Merge pull request openedx#33866 from open-craft/braden/copy-paste-un…
Browse files Browse the repository at this point in the history
…it-backport

Backport to Quince: copy/paste unit from within a unit in Studio
  • Loading branch information
arbrandes authored Dec 8, 2023
2 parents 19844b9 + 5c7cf44 commit 847f94b
Show file tree
Hide file tree
Showing 16 changed files with 312 additions and 73 deletions.
18 changes: 10 additions & 8 deletions cms/djangoapps/contentstore/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,16 @@ def _import_xml_node_to_parent(
# and VAL will thus make the transcript available.

child_nodes = []

if issubclass(xblock_class, XmlMixin):
# Hack: XBlocks that use "XmlMixin" have their own XML parsing behavior, and in particular if they encounter
# an XML node that has no children and has only a "url_name" attribute, they'll try to load the XML data
# from an XML file in runtime.resources_fs. But that file doesn't exist here. So we set at least one
# additional attribute here to make sure that url_name is not the only attribute; otherwise in some cases,
# XmlMixin.parse_xml will try to load an XML file that doesn't exist, giving an error. The name and value
# of this attribute don't matter and should be ignored.
node.attrib["x-is-pointer-node"] = "no"

if not xblock_class.has_children:
# No children to worry about. The XML may contain child nodes, but they're not XBlocks.
temp_xblock = xblock_class.parse_xml(node, runtime, keys, id_generator)
Expand All @@ -314,14 +324,6 @@ def _import_xml_node_to_parent(
# serialization of a child block, in order. For blocks that don't support children, their XML content/nodes
# could be anything (e.g. HTML, capa)
node_without_children = etree.Element(node.tag, **node.attrib)
if issubclass(xblock_class, XmlMixin):
# Hack: XBlocks that use "XmlMixin" have their own XML parsing behavior, and in particular if they encounter
# an XML node that has no children and has only a "url_name" attribute, they'll try to load the XML data
# from an XML file in runtime.resources_fs. But that file doesn't exist here. So we set at least one
# additional attribute here to make sure that url_name is not the only attribute; otherwise in some cases,
# XmlMixin.parse_xml will try to load an XML file that doesn't exist, giving an error. The name and value
# of this attribute don't matter and should be ignored.
node_without_children.attrib["x-is-pointer-node"] = "no"
temp_xblock = xblock_class.parse_xml(node_without_children, runtime, keys, id_generator)
child_nodes = list(node)
if xblock_class.has_children and temp_xblock.children:
Expand Down
37 changes: 37 additions & 0 deletions cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
allow users to paste XBlocks that were copied using the staged_content/clipboard
APIs.
"""
import ddt
from opaque_keys.edx.keys import UsageKey
from rest_framework.test import APIClient
from xmodule.modulestore.django import contentstore
Expand All @@ -13,6 +14,7 @@
XBLOCK_ENDPOINT = "/xblock/"


@ddt.ddt
class ClipboardPasteTestCase(ModuleStoreTestCase):
"""
Test Clipboard Paste functionality
Expand Down Expand Up @@ -99,6 +101,41 @@ def test_copy_and_paste_unit(self):
# The new block should store a reference to where it was copied from
assert dest_unit.copied_from_block == str(unit_key)

@ddt.data(
# A problem with absolutely no fields set. A previous version of copy-paste had an error when pasting this.
{"category": "problem", "display_name": None, "data": ""},
)
def test_copy_and_paste_component(self, block_args):
"""
Test copying a component (XBlock) from one course into another
"""
source_course = CourseFactory.create(display_name='Destination Course')
source_block = BlockFactory.create(parent_location=source_course.location, **block_args)

dest_course = CourseFactory.create(display_name='Destination Course')
with self.store.bulk_operations(dest_course.id):
dest_chapter = BlockFactory.create(parent=dest_course, category='chapter', display_name='Section')
dest_sequential = BlockFactory.create(parent=dest_chapter, category='sequential', display_name='Subsection')

# Copy the block
client = APIClient()
client.login(username=self.user.username, password=self.user_password)
copy_response = client.post(CLIPBOARD_ENDPOINT, {"usage_key": str(source_block.location)}, format="json")
assert copy_response.status_code == 200

# Paste the unit
paste_response = client.post(XBLOCK_ENDPOINT, {
"parent_locator": str(dest_sequential.location),
"staged_content": "clipboard",
}, format="json")
assert paste_response.status_code == 200
dest_block_key = UsageKey.from_string(paste_response.json()["locator"])

dest_block = self.store.get_item(dest_block_key)
assert dest_block.display_name == source_block.display_name
# The new block should store a reference to where it was copied from
assert dest_block.copied_from_block == str(source_block.location)

def test_paste_with_assets(self):
"""
When pasting into a different course, any required static assets should
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1394,16 +1394,17 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements
else:
xblock_info["staff_only_message"] = False

# If the ENABLE_COPY_PASTE_UNITS feature flag is enabled, we show the newer menu that allows copying/pasting
xblock_info["enable_copy_paste_units"] = ENABLE_COPY_PASTE_UNITS.is_enabled()

xblock_info[
"has_partition_group_components"
] = has_children_visible_to_specific_partition_groups(xblock)
xblock_info["user_partition_info"] = get_visibility_partition_info(
xblock, course=course
)

if course_outline or is_xblock_unit:
# If the ENABLE_COPY_PASTE_UNITS feature flag is enabled, we show the newer menu that allows copying/pasting
xblock_info["enable_copy_paste_units"] = ENABLE_COPY_PASTE_UNITS.is_enabled()

if is_xblock_unit and summary_configuration.is_enabled():
xblock_info["summary_configuration_enabled"] = summary_configuration.is_summary_enabled(xblock_info['id'])

Expand Down
1 change: 1 addition & 0 deletions cms/static/js/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ function(
$body.click(function() {
$('.nav-dd .nav-item .wrapper-nav-sub').removeClass('is-shown');
$('.nav-dd .nav-item .title').removeClass('is-selected');
$('.custom-dropdown .dropdown-options').hide();
});

$('.nav-dd .nav-item, .filterable-column .nav-item').click(function(e) {
Expand Down
27 changes: 0 additions & 27 deletions cms/static/js/spec/views/pages/container_subviews_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -581,33 +581,6 @@ describe('Container Subviews', function() {
});
});

describe('PublishHistory', function() {
var lastPublishCss = '.wrapper-last-publish';

it('renders never published when the block is unpublished', function() {
renderContainerPage(this, mockContainerXBlockHtml, {
published: false, published_on: null, published_by: null
});
expect(containerPage.$(lastPublishCss).text()).toContain('Never published');
});

it('renders the last published date and user when the block is published', function() {
renderContainerPage(this, mockContainerXBlockHtml);
fetch({
published: true, published_on: 'Jul 01, 2014 at 12:45 UTC', published_by: 'amako'
});
expect(containerPage.$(lastPublishCss).text())
.toContain('Last published Jul 01, 2014 at 12:45 UTC by amako');
});

it('renders correctly when the block is published without publish info', function() {
renderContainerPage(this, mockContainerXBlockHtml);
fetch({
published: true, published_on: null, published_by: null
});
expect(containerPage.$(lastPublishCss).text()).toContain('Previously published');
});
});

describe('Message Area', function() {
var messageSelector = '.container-message .warning',
Expand Down
6 changes: 4 additions & 2 deletions cms/static/js/views/pages/container.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView
model: this.model
});
this.messageView.render();
this.clipboardBroadcastChannel = new BroadcastChannel("studio_clipboard_channel");
// Display access message on units and split test components
if (!this.isLibraryPage) {
this.containerAccessView = new ContainerSubviews.ContainerAccess({
Expand All @@ -81,7 +82,8 @@ function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView
el: this.$('#publish-unit'),
model: this.model,
// When "Discard Changes" is clicked, the whole page must be re-rendered.
renderPage: this.render
renderPage: this.render,
clipboardBroadcastChannel: this.clipboardBroadcastChannel,
});
this.xblockPublisher.render();

Expand All @@ -105,7 +107,6 @@ function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView
}

this.listenTo(Backbone, 'move:onXBlockMoved', this.onXBlockMoved);
this.clipboardBroadcastChannel = new BroadcastChannel("studio_clipboard_channel");
},

getViewParameters: function() {
Expand Down Expand Up @@ -158,6 +159,7 @@ function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView
if (!self.isLibraryPage && !self.isLibraryContentPage) {
self.initializePasteButton();
}

},
block_added: options && options.block_added
});
Expand Down
49 changes: 48 additions & 1 deletion cms/static/js/views/pages/container_subviews.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ function($, _, gettext, BaseView, ViewUtils, XBlockViewUtils, MoveXBlockUtils, H
events: {
'click .action-publish': 'publish',
'click .action-discard': 'discardChanges',
'click .action-staff-lock': 'toggleStaffLock'
'click .action-staff-lock': 'toggleStaffLock',
'click .action-copy': 'copyToClipboard'
},

// takes XBlockInfo as a model
Expand All @@ -116,6 +117,7 @@ function($, _, gettext, BaseView, ViewUtils, XBlockViewUtils, MoveXBlockUtils, H
this.template = this.loadTemplate('publish-xblock');
this.model.on('sync', this.onSync, this);
this.renderPage = this.options.renderPage;
this.clipboardBroadcastChannel = this.options.clipboardBroadcastChannel;
},

onSync: function(model) {
Expand Down Expand Up @@ -147,6 +149,7 @@ function($, _, gettext, BaseView, ViewUtils, XBlockViewUtils, MoveXBlockUtils, H
releaseDateFrom: this.model.get('release_date_from'),
hasExplicitStaffLock: this.model.get('has_explicit_staff_lock'),
staffLockFrom: this.model.get('staff_lock_from'),
enableCopyUnit: this.model.get('enable_copy_paste_units'),
course: window.course,
HtmlUtils: HtmlUtils
})
Expand All @@ -173,6 +176,50 @@ function($, _, gettext, BaseView, ViewUtils, XBlockViewUtils, MoveXBlockUtils, H
});
},

copyToClipboard: function(e) {
e.preventDefault();
e.stopPropagation();
const clipboardEndpoint = "/api/content-staging/v1/clipboard/";
const usageKeyToCopy = this.model.get('id');
// Start showing a "Copying" notification:
ViewUtils.runOperationShowingMessage(gettext('Copying'), () => {
return $.postJSON(
clipboardEndpoint,
{ usage_key: usageKeyToCopy },
).then((data) => {
const status = data.content?.status;
if (status === "ready") {
// something that enables the paste button in the actions dropdown
this.clipboardBroadcastChannel.postMessage(data);
return data;
} else if (status === "loading") {
// The clipboard is being loaded asynchonously.
// Poll the endpoint until the copying process is complete:
const deferred = $.Deferred();
const checkStatus = () => {
$.getJSON(clipboardEndpoint, (pollData) => {
const newStatus = pollData.content?.status;
if (newStatus === "ready") {
// something that enables the paste button in actions dropdown
this.clipboardBroadcastChannel.postMessage(pollData);
deferred.resolve(pollData);
} else if (newStatus === "loading") {
setTimeout(checkStatus, 1_000);
} else {
deferred.reject();
throw new Error(`Unexpected clipboard status "${newStatus}" in successful API response.`);
}
})
}
setTimeout(checkStatus, 1_000);
return deferred;
} else {
throw new Error(`Unexpected clipboard status "${status}" in successful API response.`);
}
});
});
},

discardChanges: function(e) {
var xblockInfo = this.model,
renderPage = this.renderPage;
Expand Down
84 changes: 82 additions & 2 deletions cms/static/js/views/utils/xblock_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ function($, _, gettext, ViewUtils, ModuleUtils, XBlockInfo, StringUtils) {

var addXBlock, duplicateXBlock, deleteXBlock, createUpdateRequestData, updateXBlockField, VisibilityState,
getXBlockVisibilityClass, getXBlockListTypeClass, updateXBlockFields, getXBlockType, findXBlockInfo,
moveXBlock;
moveXBlock, pasteXBlock;

/**
* Represents the possible visibility states for an xblock:
Expand Down Expand Up @@ -69,6 +69,85 @@ function($, _, gettext, ViewUtils, ModuleUtils, XBlockInfo, StringUtils) {
});
};

pasteXBlock = function(target) {
var parentLocator = target.data('parent'),
displayName = target.data('default-name');

return ViewUtils.runOperationShowingMessage(gettext('Pasting'), () => {
return $.postJSON(ModuleUtils.getUpdateUrl(), {
parent_locator: parentLocator,
staged_content: "clipboard",
}).then((data) => {
return data;
});
}).done((data) => {
const {
conflicting_files: conflictingFiles,
error_files: errorFiles,
new_files: newFiles,
} = data.static_file_notices;

const notices = [];
if (errorFiles.length) {
notices.push((next) => new PromptView.Error({
title: gettext("Some errors occurred"),
message: (
gettext("The following required files could not be added to the course:") +
" " + errorFiles.join(", ")
),
actions: {primary: {text: gettext("OK"), click: (x) => { x.hide(); next(); }}},
}));
}
if (conflictingFiles.length) {
notices.push((next) => new PromptView.Warning({
title: gettext("You may need to update a file(s) manually"),
message: (
gettext(
"The following files already exist in this course but don't match the " +
"version used by the component you pasted:"
) + " " + conflictingFiles.join(", ")
),
actions: {primary: {text: gettext("OK"), click: (x) => { x.hide(); next(); }}},
}));
}
if (newFiles.length) {
notices.push(() => new NotificationView.Info({
title: gettext("New file(s) added to Files & Uploads."),
message: (
gettext("The following required files were imported to this course:") +
" " + newFiles.join(", ")
),
actions: {
primary: {
text: gettext('View files'),
click: function(notification) {
const article = document.querySelector('[data-course-assets]');
const assetsUrl = $(article).attr('data-course-assets');
window.location.href = assetsUrl;
return;
}
},
secondary: {
text: gettext('Dismiss'),
click: function(notification) {
return notification.hide();
}
}
}
}));
}
if (notices.length) {
// Show the notices, one at a time:
const showNext = () => {
const view = notices.shift()(showNext);
view.show();
}
// Delay to avoid conflict with the "Pasting..." notification.
setTimeout(showNext, 1250);
}
});
};

/**
* Duplicates the specified xblock element in its parent xblock.
* @param {jquery Element} xblockElement The xblock element to be duplicated.
Expand Down Expand Up @@ -308,6 +387,7 @@ function($, _, gettext, ViewUtils, ModuleUtils, XBlockInfo, StringUtils) {
getXBlockListTypeClass: getXBlockListTypeClass,
updateXBlockFields: updateXBlockFields,
getXBlockType: getXBlockType,
findXBlockInfo: findXBlockInfo
findXBlockInfo: findXBlockInfo,
pasteXBlock: pasteXBlock
};
});
4 changes: 4 additions & 0 deletions cms/static/js/views/xblock.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ function($, _, ViewUtils, BaseView, XBlock, HtmlUtils) {
'click .notification-action-button': 'fireNotificationActionEvent'
},

options: {
clipboardData: { content: null },
},

initialize: function() {
BaseView.prototype.initialize.call(this);
this.view = this.options.view;
Expand Down
Loading

0 comments on commit 847f94b

Please sign in to comment.