From 7b0c8f64d5c764da3101e2d1e2e2c8d69ffd54d9 Mon Sep 17 00:00:00 2001 From: Tim Krones Date: Wed, 30 Nov 2016 10:26:25 +0100 Subject: [PATCH 1/6] Fix: Don't show visibility settings on library pages. --- cms/djangoapps/contentstore/views/item.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index df448141fde1..4a26f9b6bfc0 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -294,6 +294,7 @@ def xblock_view_handler(request, usage_key_string, view_name): elif view_name in PREVIEW_VIEWS + container_views: is_pages_view = view_name == STUDENT_VIEW # Only the "Pages" view uses student view in Studio + is_library_view = view_name == 'container_child_preview' can_edit = has_studio_write_access(request.user, usage_key.course_key) # Determine the items to be shown as reorderable. Note that the view @@ -330,6 +331,7 @@ def xblock_view_handler(request, usage_key_string, view_name): 'is_pages_view': is_pages_view, # This setting disables the recursive wrapping of xblocks 'is_unit_page': is_unit(xblock), 'can_edit': can_edit, + 'can_edit_visibility': not is_library_view, 'root_xblock': xblock if (view_name == 'container_preview') else None, 'reorderable_items': reorderable_items, 'paging': paging, From c777d8393ea4bd802267d25d75de25bcba7bef23 Mon Sep 17 00:00:00 2001 From: Tim Krones Date: Wed, 30 Nov 2016 16:36:35 +0100 Subject: [PATCH 2/6] Improve variable names. --- cms/djangoapps/contentstore/views/item.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 4a26f9b6bfc0..48a0f461c367 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -294,8 +294,8 @@ def xblock_view_handler(request, usage_key_string, view_name): elif view_name in PREVIEW_VIEWS + container_views: is_pages_view = view_name == STUDENT_VIEW # Only the "Pages" view uses student view in Studio - is_library_view = view_name == 'container_child_preview' can_edit = has_studio_write_access(request.user, usage_key.course_key) + can_edit_visibility = not view_name == 'container_child_preview' # Determine the items to be shown as reorderable. Note that the view # 'reorderable_container_child_preview' is only rendered for xblocks that @@ -331,7 +331,7 @@ def xblock_view_handler(request, usage_key_string, view_name): 'is_pages_view': is_pages_view, # This setting disables the recursive wrapping of xblocks 'is_unit_page': is_unit(xblock), 'can_edit': can_edit, - 'can_edit_visibility': not is_library_view, + 'can_edit_visibility': can_edit_visibility, 'root_xblock': xblock if (view_name == 'container_preview') else None, 'reorderable_items': reorderable_items, 'paging': paging, From a5f157ffba77e69e1c20508c1e32a6832c6ee4e0 Mon Sep 17 00:00:00 2001 From: Tim Krones Date: Wed, 30 Nov 2016 17:43:18 +0100 Subject: [PATCH 3/6] Rename "container_child_preview" to "library_container_child_preview" to reflect the fact that this view name is only used for components displayed on library pages. --- cms/djangoapps/contentstore/views/item.py | 6 ++++-- cms/static/js/views/paged_container.js | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 48a0f461c367..cca76d186f32 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -268,7 +268,9 @@ def xblock_view_handler(request, usage_key_string, view_name): if 'application/json' in accept_header: store = modulestore() xblock = store.get_item(usage_key) - container_views = ['container_preview', 'reorderable_container_child_preview', 'container_child_preview'] + container_views = [ + 'container_preview', 'reorderable_container_child_preview', 'library_container_child_preview' + ] # wrap the generated fragment in the xmodule_editor div so that the javascript # can bind to it correctly @@ -295,7 +297,7 @@ def xblock_view_handler(request, usage_key_string, view_name): elif view_name in PREVIEW_VIEWS + container_views: is_pages_view = view_name == STUDENT_VIEW # Only the "Pages" view uses student view in Studio can_edit = has_studio_write_access(request.user, usage_key.course_key) - can_edit_visibility = not view_name == 'container_child_preview' + can_edit_visibility = not view_name == 'library_container_child_preview' # Determine the items to be shown as reorderable. Note that the view # 'reorderable_container_child_preview' is only rendered for xblocks that diff --git a/cms/static/js/views/paged_container.js b/cms/static/js/views/paged_container.js index 31df130c2b1e..02a4ed068cb7 100644 --- a/cms/static/js/views/paged_container.js +++ b/cms/static/js/views/paged_container.js @@ -73,7 +73,7 @@ define(['jquery', 'underscore', 'common/js/components/utils/view_utils', 'js/vie }; }, - new_child_view: 'container_child_preview', + new_child_view: 'library_container_child_preview', render: function(options) { options = options || {}; From f0ceab200bc165c6d39ceb545dcb1ca4f5d70010 Mon Sep 17 00:00:00 2001 From: Tim Krones Date: Wed, 30 Nov 2016 19:38:11 +0100 Subject: [PATCH 4/6] Alternative approach: Pass "can_edit_visibility" info explicitly when rendering a newly added component on a library page (instead of setting this info based on view name inside "xblock_view_handler"). --- cms/djangoapps/contentstore/views/item.py | 7 +++++-- cms/static/js/views/pages/container.js | 3 ++- cms/static/js/views/pages/paged_container.js | 6 ++++-- cms/static/js/views/xblock.js | 1 + 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index cca76d186f32..663f9c6b048c 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -2,6 +2,7 @@ from __future__ import absolute_import import hashlib +import json import logging from collections import OrderedDict from datetime import datetime @@ -297,7 +298,6 @@ def xblock_view_handler(request, usage_key_string, view_name): elif view_name in PREVIEW_VIEWS + container_views: is_pages_view = view_name == STUDENT_VIEW # Only the "Pages" view uses student view in Studio can_edit = has_studio_write_access(request.user, usage_key.course_key) - can_edit_visibility = not view_name == 'library_container_child_preview' # Determine the items to be shown as reorderable. Note that the view # 'reorderable_container_child_preview' is only rendered for xblocks that @@ -327,19 +327,22 @@ def xblock_view_handler(request, usage_key_string, view_name): ) force_render = request.GET.get('force_render', None) + can_edit_visibility = request.GET.get('can_edit_visibility', None) # Set up the context to be passed to each XBlock's render method. context = { 'is_pages_view': is_pages_view, # This setting disables the recursive wrapping of xblocks 'is_unit_page': is_unit(xblock), 'can_edit': can_edit, - 'can_edit_visibility': can_edit_visibility, 'root_xblock': xblock if (view_name == 'container_preview') else None, 'reorderable_items': reorderable_items, 'paging': paging, 'force_render': force_render, } + if can_edit_visibility is not None: + context['can_edit_visibility'] = json.loads(can_edit_visibility) + fragment = get_preview_fragment(request, xblock, context) # Note that the container view recursively adds headers into the preview fragment, diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 73ee99d834bd..5d033458144c 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -302,7 +302,7 @@ define(['jquery', 'underscore', 'gettext', 'js/views/pages/base_page', 'common/j * refreshing. * @returns {jQuery promise} A promise representing the complete operation. */ - refreshChildXBlock: function(xblockElement, block_added, is_duplicate) { + refreshChildXBlock: function(xblockElement, block_added, is_duplicate, renderParameters) { var self = this, xblockInfo, TemporaryXBlockView, @@ -327,6 +327,7 @@ define(['jquery', 'underscore', 'gettext', 'js/views/pages/base_page', 'common/j el: xblockElement }); return temporaryView.render({ + data: renderParameters || {}, success: function() { self.onXBlockRefresh(temporaryView, block_added, is_duplicate); temporaryView.unbind(); // Remove the temporary view diff --git a/cms/static/js/views/pages/paged_container.js b/cms/static/js/views/pages/paged_container.js index b8317d5fcc90..80996ef3a28e 100644 --- a/cms/static/js/views/pages/paged_container.js +++ b/cms/static/js/views/pages/paged_container.js @@ -28,11 +28,13 @@ define(['jquery', 'underscore', 'gettext', 'js/views/pages/container', 'js/views refreshXBlock: function(element, block_added, is_duplicate) { var xblockElement = this.findXBlockElement(element), - rootLocator = this.xblockView.model.id; + rootLocator = this.xblockView.model.id, + renderParameters; if (xblockElement.length === 0 || xblockElement.data('locator') === rootLocator) { this.render({refresh: true, block_added: block_added}); } else { - this.refreshChildXBlock(xblockElement, block_added, is_duplicate); + renderParameters = {can_edit_visibility: false}; + this.refreshChildXBlock(xblockElement, block_added, is_duplicate, renderParameters); } }, diff --git a/cms/static/js/views/xblock.js b/cms/static/js/views/xblock.js index b90554b866e7..0b3d560b79e4 100644 --- a/cms/static/js/views/xblock.js +++ b/cms/static/js/views/xblock.js @@ -23,6 +23,7 @@ define(['jquery', 'underscore', 'common/js/components/utils/view_utils', 'js/vie url: decodeURIComponent(xblockUrl) + '/' + view, type: 'GET', cache: false, + data: options.data || {}, headers: {Accept: 'application/json'}, success: function(fragment) { self.handleXBlockFragment(fragment, options); From a2ae07b0b84862c31a0990a244213f658a5772f6 Mon Sep 17 00:00:00 2001 From: Tim Krones Date: Thu, 1 Dec 2016 11:12:09 +0100 Subject: [PATCH 5/6] XBlockView.render: Make sure "options" is defined before using it in AJAX call. --- cms/static/js/views/xblock.js | 1 + 1 file changed, 1 insertion(+) diff --git a/cms/static/js/views/xblock.js b/cms/static/js/views/xblock.js index 0b3d560b79e4..ff3cc7d4ee74 100644 --- a/cms/static/js/views/xblock.js +++ b/cms/static/js/views/xblock.js @@ -19,6 +19,7 @@ define(['jquery', 'underscore', 'common/js/components/utils/view_utils', 'js/vie view = this.view, xblockInfo = this.model, xblockUrl = xblockInfo.url(); + options = options || {}; return $.ajax({ url: decodeURIComponent(xblockUrl) + '/' + view, type: 'GET', From a239786375812ce8451b6e0bb583661de845ac6b Mon Sep 17 00:00:00 2001 From: Tim Krones Date: Thu, 1 Dec 2016 11:19:08 +0100 Subject: [PATCH 6/6] Fix quality violations. --- cms/static/js/views/pages/container.js | 6 +++--- cms/static/js/views/xblock.js | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 5d033458144c..1b6551bd6463 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -298,11 +298,11 @@ define(['jquery', 'underscore', 'gettext', 'js/views/pages/base_page', 'common/j * Refresh an xblock element inline on the page, using the specified xblockInfo. * Note that the element is removed and replaced with the newly rendered xblock. * @param xblockElement The xblock element to be refreshed. - * @param block_added Specifies if a block has been added, rather than just needs + * @param blockAdded Specifies if a block has been added, rather than just needs * refreshing. * @returns {jQuery promise} A promise representing the complete operation. */ - refreshChildXBlock: function(xblockElement, block_added, is_duplicate, renderParameters) { + refreshChildXBlock: function(xblockElement, blockAdded, isDuplicate, renderParameters) { var self = this, xblockInfo, TemporaryXBlockView, @@ -329,7 +329,7 @@ define(['jquery', 'underscore', 'gettext', 'js/views/pages/base_page', 'common/j return temporaryView.render({ data: renderParameters || {}, success: function() { - self.onXBlockRefresh(temporaryView, block_added, is_duplicate); + self.onXBlockRefresh(temporaryView, blockAdded, isDuplicate); temporaryView.unbind(); // Remove the temporary view }, initRuntimeData: this diff --git a/cms/static/js/views/xblock.js b/cms/static/js/views/xblock.js index ff3cc7d4ee74..605718c0a56e 100644 --- a/cms/static/js/views/xblock.js +++ b/cms/static/js/views/xblock.js @@ -18,13 +18,13 @@ define(['jquery', 'underscore', 'common/js/components/utils/view_utils', 'js/vie var self = this, view = this.view, xblockInfo = this.model, - xblockUrl = xblockInfo.url(); - options = options || {}; + xblockUrl = xblockInfo.url(), + data = options === 'undefined' ? {} : options.data; return $.ajax({ url: decodeURIComponent(xblockUrl) + '/' + view, type: 'GET', cache: false, - data: options.data || {}, + data: data, headers: {Accept: 'application/json'}, success: function(fragment) { self.handleXBlockFragment(fragment, options);