(WIP) Studio library view - Do not merge, for internal review #1
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -203,7 +203,7 @@ def xblock_view_handler(request, usage_key_string, view_name): | |
|
|
||
| if 'application/json' in accept_header: | ||
| store = modulestore() | ||
| xblock = store.get_item(usage_key, remove_branch=False) | ||
|
Member
Author
There was a problem hiding this comment. Why removing the branch information here? What are the effects on the rest of the courseware content?
Member
There was a problem hiding this comment. This change is to not remove branch information This is related to your last question. Here's why I had to add A lot of the methods on the For example, it you retrieve the library's root XBlock using then it will find and return the XBlock, but that XBlock's so the returned block ID is actually wrong and doesn't exist. This was causing all sorts of errors with studio's RESTful XBlock editing API when trying to use the library branch. However, thankfully the strip_key decorator is designed so that the end user can override its behaviour whenever necessary by passing in I think this decorator on the modulestore level didn't affect the existing studio code because the default
Member
Author
There was a problem hiding this comment. Thank you for the explanations. It could be worth including a note/question about this in the specifications, to get feedback on the approach - this will likely show up in the first code review, but it could be useful to raise the issue early. |
||
| container_views = ['container_preview', 'reorderable_container_child_preview'] | ||
|
|
||
| # wrap the generated fragment in the xmodule_editor div so that the javascript | ||
|
|
@@ -314,7 +314,7 @@ def _update_with_callback(xblock, user, old_metadata=None, old_content=None): | |
| xblock.editor_saved(user, old_metadata, old_content) | ||
|
|
||
| # Update after the callback so any changes made in the callback will get persisted. | ||
| return modulestore().update_item(xblock, user.id, remove_branch=False) | ||
|
|
||
|
|
||
| def _save_xblock(user, xblock, data=None, children_strings=None, metadata=None, nullout=None, | ||
|
|
@@ -462,7 +462,7 @@ def _create_item(request): | |
|
|
||
| store = modulestore() | ||
| with store.bulk_operations(usage_key.course_key): | ||
| parent = store.get_item(usage_key, remove_branch=False) | ||
| dest_usage_key = usage_key.replace(category=category, name=uuid4().hex) | ||
|
|
||
| # get the metadata, display_name, and definition from the request | ||
|
|
@@ -493,6 +493,7 @@ def _create_item(request): | |
| definition_data=data, | ||
| metadata=metadata, | ||
| runtime=parent.runtime, | ||
| remove_branch=False, | ||
| ) | ||
|
|
||
| # VS[compat] cdodge: This is a hack because static_tabs also have references from the course module, so | ||
|
|
@@ -518,7 +519,7 @@ def _duplicate_item(parent_usage_key, duplicate_source_usage_key, user, display_ | |
| """ | ||
| store = modulestore() | ||
| with store.bulk_operations(duplicate_source_usage_key.course_key): | ||
| source_item = store.get_item(duplicate_source_usage_key, remove_branch=False) | ||
| # Change the blockID to be unique. | ||
| dest_usage_key = source_item.location.replace(name=uuid4().hex) | ||
| category = dest_usage_key.block_type | ||
|
|
@@ -541,6 +542,7 @@ def _duplicate_item(parent_usage_key, duplicate_source_usage_key, user, display_ | |
| definition_data=source_item.get_explicitly_set_fields_by_scope(Scope.content), | ||
| metadata=duplicate_metadata, | ||
| runtime=source_item.runtime, | ||
| remove_branch=False, | ||
| ) | ||
|
|
||
| # Children are not automatically copied over (and not all xblocks have a 'children' attribute). | ||
|
|
@@ -553,7 +555,7 @@ def _duplicate_item(parent_usage_key, duplicate_source_usage_key, user, display_ | |
| store.update_item(dest_module, user.id) | ||
|
|
||
| if 'detached' not in source_item.runtime.load_block_type(category)._class_tags: | ||
| parent = store.get_item(parent_usage_key, remove_branch=False) | ||
| # 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: | ||
|
|
@@ -623,7 +625,7 @@ def _get_xblock(usage_key, user): | |
| store = modulestore() | ||
| with store.bulk_operations(usage_key.course_key): | ||
| try: | ||
| return store.get_item(usage_key, depth=None, remove_branch=False) | ||
| except ItemNotFoundError: | ||
| if usage_key.category in CREATE_IF_NOT_FOUND: | ||
| # Create a new one for certain categories only. Used for course info handouts. | ||
|
|
@@ -696,7 +698,10 @@ def safe_get_username(user_id): | |
| has_changes = modulestore().has_changes(xblock) if (is_xblock_unit or course_outline) else None | ||
|
|
||
| if graders is None: | ||
| if xblock.category != "library": | ||
| graders = CourseGradingModel.fetch(xblock.location.course_key).graders | ||
| else: | ||
| graders = [] | ||
|
|
||
| # Compute the child info first so it can be included in aggregate information for the parent | ||
| should_visit_children = include_child_info and (course_outline and not is_xblock_unit or not course_outline) | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,148 @@ | ||
| """ | ||
| Views related to content libraries. | ||
| A content library is an optional branch that contains a flat list of XBlocks | ||
| which can be re-used in the "normal" branches of the course or other courses. | ||
| """ | ||
| from __future__ import absolute_import | ||
|
|
||
| import json | ||
| import logging | ||
|
|
||
| from contentstore.views.item import create_xblock_info | ||
| from django.http import HttpResponse, HttpResponseBadRequest, Http404 | ||
| from django.contrib.auth.decorators import login_required | ||
| from django.core.exceptions import PermissionDenied | ||
| from django.conf import settings | ||
| from django.utils.translation import ugettext as _ | ||
| from django_future.csrf import ensure_csrf_cookie | ||
| from edxmako.shortcuts import render_to_response | ||
| from opaque_keys.edx.keys import CourseKey | ||
| from xmodule.library_module import LibraryDescriptor | ||
| from xmodule.modulestore.exceptions import ItemNotFoundError | ||
| from xmodule.modulestore import ModuleStoreEnum | ||
| from xmodule.modulestore.django import modulestore | ||
|
|
||
| from .access import has_course_access | ||
| from .component import get_component_templates | ||
| from util.json_request import JsonResponse | ||
|
|
||
| __all__ = ['library_handler'] | ||
|
|
||
| log = logging.getLogger(__name__) | ||
|
|
||
| LIBRARIES_ENABLED = settings.FEATURES.get('ENABLE_CONTENT_LIBRARIES', False) | ||
|
|
||
|
|
||
| @login_required | ||
| @ensure_csrf_cookie | ||
| def library_handler(request, course_key_string=None): | ||
| """ | ||
| RESTful interface to most content library related functionality. | ||
| """ | ||
| if not LIBRARIES_ENABLED: | ||
| raise Http404 # Should never happen because we test the feature in urls.py also | ||
|
|
||
| response_format = 'html' | ||
| if request.REQUEST.get('format', 'html') == 'json' or 'application/json' in request.META.get('HTTP_ACCEPT', 'text/html'): | ||
| response_format = 'json' | ||
|
|
||
| if course_key_string: | ||
| course_key = CourseKey.from_string(course_key_string) | ||
| if not has_course_access(request.user, course_key): | ||
| raise PermissionDenied() | ||
|
|
||
| if course_key.deprecated: | ||
| # Only courses stored in Split Mongo will work, and split requires Locators, not deprecated keys | ||
| return HttpResponseBadRequest("This course's modulestore does not support content libraries.") | ||
|
|
||
| if not course_key.branch: | ||
| course_key = course_key.for_branch("library") | ||
|
|
||
| store = modulestore() | ||
|
|
||
| try: | ||
| library = store.get_course(course_key, remove_branch=False) | ||
| if library is None: | ||
| raise ItemNotFoundError # Inconsistency: mixed modulestore returns None, whereas split raises exception | ||
| except ItemNotFoundError: | ||
| if store.has_course(course_key.for_branch(None)): | ||
| # There is a course, but no library [yet] | ||
| if "create" in request.GET: | ||
| # Create the library branch & root XBlock: | ||
| store.create_branch( | ||
| org=course_key.org, | ||
| course=course_key.course, | ||
| run=course_key.run, | ||
| branch='library', | ||
| user_id=request.user.id, | ||
| fields={"display_name": "New Library"}, | ||
| root_category='library', | ||
| root_block_id='library', | ||
| ) | ||
| return JsonResponse({ | ||
| "result": "success", | ||
| }) | ||
| return HttpResponse( | ||
| "<html><body>" | ||
| "No library exists for {course_id}. Would you like to create one? " | ||
| "<a href=\"?create\">Yes</a>" | ||
| "</body></html>" | ||
| .format(course_id=course_key.for_branch(None)) | ||
| ) | ||
| else: | ||
| raise Http404 | ||
|
|
||
| if not isinstance(library, LibraryDescriptor): | ||
| return HttpResponseBadRequest("Course key specified is not a library.") | ||
|
|
||
| if request.method == 'GET': | ||
| return library_blocks_view(request, library, response_format) | ||
| return HttpResponseBadRequest("Invalid request method.") | ||
|
|
||
| # List all courses with a library: | ||
| split_store = modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.split) | ||
| libraries = [] | ||
| for i in split_store.find_matching_course_indexes("library"): | ||
| libraries.append({ | ||
| "version": "{}".format(i["versions"]["library"]), | ||
| "course": i["course"], | ||
| "org": i["org"], | ||
| "run": i["run"], | ||
| }) | ||
| return JsonResponse(libraries) | ||
|
|
||
|
|
||
| def library_blocks_view(request, library, response_format): | ||
| """ | ||
| The main view of a course's content library. | ||
| Shows all the XBlocks in the library, and allows adding/editing/deleting | ||
| them. | ||
| Can be called with response_format="json" to get a JSON-formatted list of | ||
| the XBlocks in the library along with library metadata. | ||
| """ | ||
| children = library.children | ||
| if response_format == "json": | ||
| # The JSON response for this request is short and sweet: | ||
| prev_version = library.runtime.course_entry.structure['previous_version'] | ||
| return JsonResponse({ | ||
| "display_name": library.display_name, | ||
| "library_id": unicode(library.location.course_key), # library.course_id raises UndefinedContext - fix? | ||
| "version": unicode(library.runtime.course_entry.course_key.version), | ||
| "previous_version": unicode(prev_version) if prev_version else None, | ||
| "blocks": [unicode(x) for x in children], | ||
| }) | ||
|
|
||
| course = modulestore().get_course(library.location.course_key.for_branch(None)) | ||
| xblock_info = create_xblock_info(library, include_ancestor_info=False, graders=[]) | ||
|
|
||
| component_templates = get_component_templates(course) | ||
|
|
||
| return render_to_response('library.html', { | ||
| 'context_course': course, # Needed only for display of menus at top of page. | ||
| 'action': 'view', | ||
| 'xblock': library, | ||
| 'xblock_locator': library.location, | ||
| 'unit': None, | ||
| 'component_templates': json.dumps(component_templates), | ||
| 'xblock_info': xblock_info, | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| <%inherit file="base.html" /> | ||
| <%! | ||
| import json | ||
|
|
||
| from contentstore.views.helpers import xblock_studio_url, xblock_type_display_name | ||
| from django.utils.translation import ugettext as _ | ||
| %> | ||
| <%block name="title">${xblock.display_name_with_default} ${xblock_type_display_name(xblock)}</%block> | ||
| <%block name="bodyclass">is-signedin course container view-container</%block> | ||
|
|
||
| <%namespace name='static' file='static_content.html'/> | ||
|
|
||
| <%! | ||
| templates = ["basic-modal", "modal-button", "edit-xblock-modal", | ||
| "editor-mode-button", "upload-dialog", "image-modal", | ||
| "add-xblock-component", "add-xblock-component-button", "add-xblock-component-menu", | ||
| "add-xblock-component-menu-problem", "xblock-string-field-editor", "publish-xblock", "publish-history", | ||
| "unit-outline", "container-message"] | ||
| %> | ||
| <%block name="header_extras"> | ||
| % for template_name in templates: | ||
| <script type="text/template" id="${template_name}-tpl"> | ||
| <%static:include path="js/${template_name}.underscore" /> | ||
| </script> | ||
| % endfor | ||
| <link rel="stylesheet" type="text/css" href="${static.url('js/vendor/timepicker/jquery.timepicker.css')}" /> | ||
| </%block> | ||
|
|
||
| <%block name="jsextra"> | ||
| <script type='text/javascript'> | ||
| require(["domReady!", "jquery", "js/models/xblock_info", "js/views/pages/container", | ||
| "js/collections/component_template", "xmodule", "coffee/src/main", "xblock/cms.runtime.v1"], | ||
| function(doc, $, XBlockInfo, ContainerPage, ComponentTemplates, xmoduleLoader) { | ||
| var templates = new ComponentTemplates(${component_templates | n}, {parse: true}); | ||
| var mainXBlockInfo = new XBlockInfo(${json.dumps(xblock_info) | n}, {parse: true}); | ||
|
|
||
| xmoduleLoader.done(function () { | ||
| var view = new ContainerPage({ | ||
| el: $('#content'), | ||
| model: mainXBlockInfo, | ||
| action: "${action}", | ||
| templates: templates, | ||
| isUnitPage: false | ||
| }); | ||
| view.render(); | ||
| }); | ||
| }); | ||
| </script> | ||
|
|
||
| </%block> | ||
|
|
||
| <%block name="content"> | ||
|
|
||
|
|
||
| <div class="wrapper-mast wrapper"> | ||
| <header class="mast has-actions has-navigation has-subtitle"> | ||
| <div class="page-header"> | ||
| <div class="wrapper-xblock-field incontext-editor is-editable" | ||
| data-field="display_name" data-field-display-name="${_("Display Name")}"> | ||
| <h1 class="page-header-title xblock-field-value incontext-editor-value"><span class="title-value">${xblock.display_name_with_default | h}</span></h1> | ||
| </div> | ||
| </div> | ||
|
|
||
| <nav class="nav-actions"> | ||
| <h3 class="sr">${_("Page Actions")}</h3> | ||
| <ul> | ||
| <li class="action-item action-edit nav-item"> | ||
| <a href="#" class="button button-edit action-button edit-button"> | ||
| <i class="icon-pencil"></i> | ||
| <span class="action-button-text">${_("Edit")}</span> | ||
| </a> | ||
| </li> | ||
| </ul> | ||
| </nav> | ||
| </header> | ||
| </div> | ||
|
|
||
| <div class="wrapper-content wrapper"> | ||
| <div class="inner-wrapper"> | ||
| <section class="content-area"> | ||
|
|
||
| <article class="content-primary"> | ||
| <div class="container-message wrapper-message"></div> | ||
| <section class="wrapper-xblock level-page is-hidden studio-xblock-wrapper" data-locator="${xblock_locator | h}" data-course-key="${xblock_locator.course_key | h}"> | ||
| </section> | ||
| <div class="ui-loading"> | ||
| <p><span class="spin"><i class="icon-refresh"></i></span> <span class="copy">${_("Loading...")}</span></p> | ||
| </div> | ||
| </article> | ||
| <aside class="content-supplementary" role="complimentary"> | ||
| <div class="bit"> | ||
| <h3 class="title-3">${_("Adding content components")}</h3> | ||
| <p>${_("You can add compnents to the library. Help text here.")}</p> | ||
| </div> | ||
| <div class="bit external-help"> | ||
| <a href="${get_online_help_info('library')['doc_url']}" target="_blank" class="button external-help-button">${_("Learn more about content libraries")}</a> | ||
| </div> | ||
| </aside> | ||
| </section> | ||
| </div> | ||
| </div> | ||
| </%block> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why is it implied? You mention this is for the URL - what are the differences?
Also, how do you plan to handle multiple libraries in a single course? library1, library2, etc.? (If we do want it - I remember that the other approach was to create fake courses and handle everything outside of courses)
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.
Good questions. What this accomplishes is the following:
If I go to:
localhost:8001/course/course-v1:ProblemX+PR0B+2014- This is the normal course view, showing blocks in thedraft-branchbranchlocalhost:8001/library/course-v1:ProblemX+PR0B+2014- This is the new library view. With the code above, this will look for library data in thelibrarybranch by default.localhost:8001/library/course-v1:ProblemX+PR0B+2014+branch@library- This URL is the same thing, but with a CourseLocator string that explicitly specifies the branch. I found it a bit redundant and I thought it looks nicer and is more consistent without the+branch@librarypart there, so I included this code to make that optional.If we wanted to allow multiple libraries per course (I don't recommend this though), then each library would have its own branch, and you'd access them like:
localhost:8001/library/course-v1:ProblemX+PR0B+2014+branch@problems-librarylocalhost:8001/library/course-v1:ProblemX+PR0B+2014+branch@content-library1localhost:8001/library/course-v1:ProblemX+PR0B+2014+branch@content-library2