-
Notifications
You must be signed in to change notification settings - Fork 4.3k
feat: [FC-0070] Create a new Studio view for rendering whole Unit in an iframe #35587
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
| from django.http import Http404, HttpResponseBadRequest | ||
| from django.shortcuts import redirect | ||
| from django.utils.translation import gettext as _ | ||
| from django.views.decorators.clickjacking import xframe_options_exempt | ||
| from django.views.decorators.http import require_GET | ||
| from opaque_keys import InvalidKeyError | ||
| from opaque_keys.edx.keys import UsageKey | ||
|
|
@@ -35,7 +36,8 @@ | |
|
|
||
| __all__ = [ | ||
| 'container_handler', | ||
| 'component_handler' | ||
| 'component_handler', | ||
| 'container_embed_handler', | ||
| ] | ||
|
|
||
| log = logging.getLogger(__name__) | ||
|
|
@@ -141,6 +143,36 @@ def container_handler(request, usage_key_string): # pylint: disable=too-many-st | |
| return HttpResponseBadRequest("Only supports HTML requests") | ||
|
|
||
|
|
||
| @require_GET | ||
| @login_required | ||
| @xframe_options_exempt | ||
| def container_embed_handler(request, usage_key_string): # pylint: disable=too-many-statements | ||
| """ | ||
| Returns an HttpResponse with HTML content for the container XBlock. | ||
| The returned HTML is a chromeless rendering of the XBlock. | ||
|
|
||
| GET | ||
| html: returns the HTML page for editing a container | ||
| json: not currently supported | ||
| """ | ||
|
|
||
| # Avoiding a circular dependency | ||
| from ..utils import get_container_handler_context | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [question]: Is this to avoid a circular dependency?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it is. I added a comment for it. |
||
|
|
||
| try: | ||
| usage_key = UsageKey.from_string(usage_key_string) | ||
| except InvalidKeyError: # Raise Http404 on invalid 'usage_key_string' | ||
| return HttpResponseBadRequest() | ||
| with modulestore().bulk_operations(usage_key.course_key): | ||
| try: | ||
| course, xblock, lms_link, preview_lms_link = _get_item_in_course(request, usage_key) | ||
| except ItemNotFoundError: | ||
| raise Http404 # lint-amnesty, pylint: disable=raise-missing-from | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [request] An invalid key should be a 400, not a 404, since it's a malformed request.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updated |
||
|
|
||
| container_handler_context = get_container_handler_context(request, usage_key, course, xblock) | ||
| return render_to_response('container_chromeless.html', container_handler_context) | ||
|
|
||
|
|
||
| def get_component_templates(courselike, library=False): # lint-amnesty, pylint: disable=too-many-statements | ||
| """ | ||
| Returns the applicable component templates that can be used by the specified course or library. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -207,7 +207,7 @@ function($, _, Backbone, gettext, BasePage, | |
|
|
||
| renderAddXBlockComponents: function() { | ||
| var self = this; | ||
| if (self.options.canEdit) { | ||
| if (self.options.canEdit && !self.options.isIframeEmbed) { | ||
| this.$('.add-xblock-component').each(function(index, element) { | ||
| var component = new AddXBlockComponent({ | ||
| el: element, | ||
|
|
@@ -222,7 +222,7 @@ function($, _, Backbone, gettext, BasePage, | |
| }, | ||
|
|
||
| initializePasteButton() { | ||
| if (this.options.canEdit) { | ||
| if (this.options.canEdit && !self.options.isIframeEmbed) { | ||
| // We should have the user's clipboard status. | ||
| const data = this.options.clipboardData; | ||
| this.refreshPasteButton(data); | ||
|
|
@@ -239,7 +239,7 @@ function($, _, Backbone, gettext, BasePage, | |
| refreshPasteButton(data) { | ||
| // Do not perform any changes on paste button since they are not | ||
| // rendered on Library or LibraryContent pages | ||
| if (!this.isLibraryPage && !this.isLibraryContentPage) { | ||
| if (!this.isLibraryPage && !this.isLibraryContentPage && !self.options.isIframeEmbed) { | ||
| // '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) { | ||
|
|
@@ -273,6 +273,18 @@ function($, _, Backbone, gettext, BasePage, | |
| /** The user has clicked on the "Paste Component button" */ | ||
| pasteComponent(event) { | ||
| event.preventDefault(); | ||
| try { | ||
| if (this.options.isIframeEmbed) { | ||
| window.parent.postMessage( | ||
| { | ||
| type: 'pasteComponent', | ||
| payload: {} | ||
| }, document.referrer | ||
| ); | ||
| } | ||
| } catch (e) { | ||
| console.error(e); | ||
| } | ||
| // 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'); | ||
|
|
@@ -365,6 +377,18 @@ function($, _, Backbone, gettext, BasePage, | |
|
|
||
| editXBlock: function(event, options) { | ||
| event.preventDefault(); | ||
| try { | ||
| if (this.options.isIframeEmbed) { | ||
| window.parent.postMessage( | ||
| { | ||
| type: 'editXBlock', | ||
| payload: {} | ||
| }, document.referrer | ||
| ); | ||
| } | ||
| } catch (e) { | ||
| console.error(e); | ||
| } | ||
|
|
||
| if (!options || options.view !== 'visibility_view') { | ||
| const primaryHeader = $(event.target).closest('.xblock-header-primary, .nav-actions'); | ||
|
|
@@ -432,66 +456,43 @@ function($, _, Backbone, gettext, BasePage, | |
| }); | ||
| }, | ||
|
|
||
| duplicateXBlock: function(event) { | ||
| event.preventDefault(); | ||
| this.duplicateComponent(this.findXBlockElement(event.target)); | ||
| }, | ||
|
|
||
| openManageTags: function(event) { | ||
| try { | ||
| if (this.options.isIframeEmbed) { | ||
| window.parent.postMessage( | ||
| { | ||
| type: 'openManageTags', | ||
| payload: {} | ||
| }, document.referrer | ||
| ); | ||
| } | ||
| } catch (e) { | ||
| console.error(e); | ||
| } | ||
| const taxonomyTagsWidgetUrl = this.model.get('taxonomy_tags_widget_url'); | ||
| const contentId = this.findXBlockElement(event.target).data('locator'); | ||
|
|
||
| TaggingDrawerUtils.openDrawer(taxonomyTagsWidgetUrl, contentId); | ||
| }, | ||
|
|
||
| showMoveXBlockModal: function(event) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't exactly understand why some functions were completely removed from this file?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some methods were duplicated, so I just removed the extra ones
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Strange, can you assume what is the reason for it, have you noticed anything strange in behavior on the legacy Unit page in CMS after the removal of these functions?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume these methods were duplicated by mistake during some rebasing/merging because they are equal.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| var xblockElement = this.findXBlockElement(event.target), | ||
| parentXBlockElement = xblockElement.parents('.studio-xblock-wrapper'), | ||
| modal = new MoveXBlockModal({ | ||
| sourceXBlockInfo: XBlockUtils.findXBlockInfo(xblockElement, this.model), | ||
| sourceParentXBlockInfo: XBlockUtils.findXBlockInfo(parentXBlockElement, this.model), | ||
| XBlockURLRoot: this.getURLRoot(), | ||
| outlineURL: this.options.outlineURL | ||
| }); | ||
|
|
||
| event.preventDefault(); | ||
| modal.show(); | ||
| }, | ||
|
|
||
| deleteXBlock: function(event) { | ||
| event.preventDefault(); | ||
| this.deleteComponent(this.findXBlockElement(event.target)); | ||
| }, | ||
|
|
||
| createPlaceholderElement: function() { | ||
| return $('<div/>', {class: 'studio-xblock-wrapper'}); | ||
| }, | ||
|
|
||
| createComponent: function(template, target) { | ||
| // A placeholder element is created in the correct location for the new xblock | ||
| // and then onNewXBlock will replace it with a rendering of the xblock. Note that | ||
| // for xblocks that can't be replaced inline, the entire parent will be refreshed. | ||
| var parentElement = this.findXBlockElement(target), | ||
| parentLocator = parentElement.data('locator'), | ||
| buttonPanel = target.closest('.add-xblock-component'), | ||
| listPanel = buttonPanel.prev(), | ||
| scrollOffset = ViewUtils.getScrollOffset(buttonPanel), | ||
| $placeholderEl = $(this.createPlaceholderElement()), | ||
| requestData = _.extend(template, { | ||
| parent_locator: parentLocator | ||
| }), | ||
| placeholderElement; | ||
| placeholderElement = $placeholderEl.appendTo(listPanel); | ||
| return $.postJSON(this.getURLRoot() + '/', requestData, | ||
| _.bind(this.onNewXBlock, this, placeholderElement, scrollOffset, false)) | ||
| .fail(function() { | ||
| // Remove the placeholder if the update failed | ||
| placeholderElement.remove(); | ||
| }); | ||
| }, | ||
|
|
||
| copyXBlock: function(event) { | ||
| event.preventDefault(); | ||
| try { | ||
| if (this.options.isIframeEmbed) { | ||
| window.parent.postMessage( | ||
| { | ||
| type: 'copyXBlock', | ||
| payload: {} | ||
| }, document.referrer | ||
| ); | ||
| } | ||
| } catch (e) { | ||
| console.error(e); | ||
| } | ||
| const clipboardEndpoint = "/api/content-staging/v1/clipboard/"; | ||
| const element = this.findXBlockElement(event.target); | ||
| const usageKeyToCopy = element.data('locator'); | ||
|
|
@@ -535,48 +536,63 @@ function($, _, Backbone, gettext, BasePage, | |
| }); | ||
| }, | ||
|
|
||
| duplicateComponent: function(xblockElement) { | ||
| // A placeholder element is created in the correct location for the duplicate xblock | ||
| // and then onNewXBlock will replace it with a rendering of the xblock. Note that | ||
| // for xblocks that can't be replaced inline, the entire parent will be refreshed. | ||
| var self = this, | ||
| parentElement = self.findXBlockElement(xblockElement.parent()), | ||
| scrollOffset = ViewUtils.getScrollOffset(xblockElement), | ||
| $placeholderEl = $(self.createPlaceholderElement()), | ||
| placeholderElement; | ||
|
|
||
| placeholderElement = $placeholderEl.insertAfter(xblockElement); | ||
| XBlockUtils.duplicateXBlock(xblockElement, parentElement) | ||
| .done(function(data) { | ||
| self.onNewXBlock(placeholderElement, scrollOffset, true, data); | ||
| }) | ||
| .fail(function() { | ||
| // Remove the placeholder if the update failed | ||
| placeholderElement.remove(); | ||
| }); | ||
| }, | ||
|
|
||
| duplicateXBlock: function(event) { | ||
| event.preventDefault(); | ||
| try { | ||
| if (this.options.isIframeEmbed) { | ||
| window.parent.postMessage( | ||
| { | ||
| type: 'duplicateXBlock', | ||
| payload: {} | ||
| }, document.referrer | ||
| ); | ||
| } | ||
| } catch (e) { | ||
| console.error(e); | ||
| } | ||
| this.duplicateComponent(this.findXBlockElement(event.target)); | ||
| }, | ||
|
|
||
| showMoveXBlockModal: function(event) { | ||
| try { | ||
| if (this.options.isIframeEmbed) { | ||
| window.parent.postMessage( | ||
| { | ||
| type: 'showMoveXBlockModal', | ||
| payload: {} | ||
| }, document.referrer | ||
| ); | ||
| } | ||
| } catch (e) { | ||
| console.error(e); | ||
| } | ||
| var xblockElement = this.findXBlockElement(event.target), | ||
| parentXBlockElement = xblockElement.parents('.studio-xblock-wrapper'), | ||
| modal = new MoveXBlockModal({ | ||
| sourceXBlockInfo: XBlockUtils.findXBlockInfo(xblockElement, this.model), | ||
| sourceParentXBlockInfo: XBlockUtils.findXBlockInfo(parentXBlockElement, this.model), | ||
| XBlockURLRoot: this.getURLRoot(), | ||
| outlineURL: this.options.outlineURL | ||
| }); | ||
| sourceXBlockInfo: XBlockUtils.findXBlockInfo(xblockElement, this.model), | ||
| sourceParentXBlockInfo: XBlockUtils.findXBlockInfo(parentXBlockElement, this.model), | ||
| XBlockURLRoot: this.getURLRoot(), | ||
| outlineURL: this.options.outlineURL | ||
| }); | ||
|
|
||
| event.preventDefault(); | ||
| modal.show(); | ||
| }, | ||
|
|
||
| deleteXBlock: function(event) { | ||
| event.preventDefault(); | ||
| try { | ||
| if (this.options.isIframeEmbed) { | ||
| window.parent.postMessage( | ||
| { | ||
| type: 'deleteXBlock', | ||
| payload: {} | ||
| }, document.referrer | ||
| ); | ||
| } | ||
| } catch (e) { | ||
| console.error(e); | ||
| } | ||
| this.deleteComponent(this.findXBlockElement(event.target)); | ||
| }, | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like we are missing a decorator that is required to use this view in an Iframe. Please add
xframe_options_exemptas it is done in LMS: https://github.com/openedx/edx-platform/blob/master/lms/djangoapps/courseware/views/views.py#L1546.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
xframe_options_exemptdecorator added.