From 73e2cb14209a77d1a2e21f3c7771d026f70895d0 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 28 Oct 2014 22:23:26 -0700 Subject: [PATCH 01/23] Studio support for creating and editing libraries (PR 6046) SOL-1, SOL-2, SOL-3 --- cms/djangoapps/contentstore/utils.py | 7 + cms/djangoapps/contentstore/views/__init__.py | 1 + .../contentstore/views/component.py | 10 + cms/djangoapps/contentstore/views/course.py | 26 +++ cms/djangoapps/contentstore/views/helpers.py | 5 +- cms/djangoapps/contentstore/views/item.py | 19 +- cms/djangoapps/contentstore/views/library.py | 185 +++++++++++++++ .../views/tests/test_course_index.py | 25 ++- .../contentstore/views/tests/test_helpers.py | 7 +- .../contentstore/views/tests/test_item.py | 51 ++++- .../contentstore/views/tests/test_library.py | 185 +++++++++++++++ cms/envs/bok_choy.env.json | 3 +- cms/envs/test.py | 3 + cms/static/js/factories/library.js | 23 ++ cms/static/js/index.js | 112 ++++++++-- .../js/spec/views/pages/course_rerun_spec.js | 4 +- cms/static/js/spec/views/pages/index_spec.js | 84 ++++++- .../js/views/utils/create_course_utils.js | 49 +--- .../js/views/utils/create_library_utils.js | 129 +++++++++++ cms/static/js/views/utils/view_utils.js | 61 ++++- cms/static/sass/elements/_forms.scss | 2 +- cms/static/sass/views/_dashboard.scss | 39 +++- cms/templates/base.html | 2 + cms/templates/container.html | 7 - cms/templates/index.html | 127 ++++++++++- .../js/mock/mock-index-page.underscore | 60 +++++ cms/templates/library.html | 66 ++++++ cms/urls.py | 6 + .../modulestore/split_mongo/split_draft.py | 8 +- .../modulestore/tests/test_libraries.py | 11 + common/static/js/xblock/core.js | 4 +- common/test/acceptance/fixtures/base.py | 196 ++++++++++++++++ common/test/acceptance/fixtures/course.py | 211 ++---------------- common/test/acceptance/fixtures/library.py | 92 ++++++++ .../test/acceptance/pages/studio/container.py | 8 +- common/test/acceptance/pages/studio/index.py | 72 ++++++ .../test/acceptance/pages/studio/library.py | 97 ++++++++ common/test/acceptance/pages/studio/utils.py | 24 ++ .../tests/studio/base_studio_test.py | 48 ++++ .../tests/studio/test_studio_home.py | 67 ++++++ .../tests/studio/test_studio_library.py | 104 +++++++++ 41 files changed, 1945 insertions(+), 295 deletions(-) create mode 100644 cms/djangoapps/contentstore/views/library.py create mode 100644 cms/djangoapps/contentstore/views/tests/test_library.py create mode 100644 cms/static/js/factories/library.js create mode 100644 cms/static/js/views/utils/create_library_utils.js create mode 100644 cms/templates/library.html create mode 100644 common/test/acceptance/fixtures/base.py create mode 100644 common/test/acceptance/fixtures/library.py create mode 100644 common/test/acceptance/pages/studio/library.py create mode 100644 common/test/acceptance/tests/studio/test_studio_home.py create mode 100644 common/test/acceptance/tests/studio/test_studio_library.py diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 914ad1ec65d4..e6c6d458cc7a 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -296,6 +296,13 @@ def reverse_course_url(handler_name, course_key, kwargs=None): return reverse_url(handler_name, 'course_key_string', course_key, kwargs) +def reverse_library_url(handler_name, library_key, kwargs=None): + """ + Creates the URL for handlers that use library_keys as URL parameters. + """ + return reverse_url(handler_name, 'library_key_string', library_key, kwargs) + + def reverse_usage_url(handler_name, usage_key, kwargs=None): """ Creates the URL for handlers that use usage_keys as URL parameters. diff --git a/cms/djangoapps/contentstore/views/__init__.py b/cms/djangoapps/contentstore/views/__init__.py index 48ff107f117c..ae78c9953239 100644 --- a/cms/djangoapps/contentstore/views/__init__.py +++ b/cms/djangoapps/contentstore/views/__init__.py @@ -12,6 +12,7 @@ from .helpers import * from .item import * from .import_export import * +from .library import * from .preview import * from .public import * from .export_git import * diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 70a470f9dc76..9768542ea8b8 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -56,6 +56,15 @@ ADVANCED_PROBLEM_TYPES = settings.ADVANCED_PROBLEM_TYPES +CONTAINER_TEMPATES = [ + "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" +] + + def _advanced_component_types(): """ Return advanced component types which can be created. @@ -202,6 +211,7 @@ def container_handler(request, usage_key_string): 'xblock_info': xblock_info, 'draft_preview_link': preview_lms_link, 'published_preview_link': lms_link, + 'templates': CONTAINER_TEMPATES }) else: return HttpResponseBadRequest("Only supports HTML requests") diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 6548607eb197..d6b2b67aad97 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -38,6 +38,7 @@ add_extra_panel_tab, remove_extra_panel_tab, reverse_course_url, + reverse_library_url, reverse_usage_url, reverse_url, remove_all_instructors, @@ -56,6 +57,7 @@ ADVANCED_COMPONENT_TYPES, ) from contentstore.tasks import rerun_course +from .library import LIBRARIES_ENABLED from .item import create_xblock_info from course_creators.views import get_course_creator_status, add_user_with_status_unrequested from contentstore import utils @@ -341,6 +343,14 @@ def _accessible_courses_list_from_groups(request): return courses_list.values(), in_process_course_actions +def _accessible_libraries_list(user): + """ + List all libraries available to the logged in user by iterating through all libraries + """ + # No need to worry about ErrorDescriptors - split's get_libraries() never returns them. + return [lib for lib in modulestore().get_libraries() if has_course_author_access(user, lib.location)] + + @login_required @ensure_csrf_cookie def course_listing(request): @@ -360,6 +370,8 @@ def course_listing(request): # so fallback to iterating through all courses courses, in_process_course_actions = _accessible_courses_list(request) + libraries = _accessible_libraries_list(request.user) if LIBRARIES_ENABLED else [] + def format_course_for_view(course): """ Return a dict of the data which the view requires for each course @@ -396,6 +408,18 @@ def format_in_process_course_view(uca): ) if uca.state == CourseRerunUIStateManager.State.FAILED else '' } + def format_library_for_view(library): + """ + Return a dict of the data which the view requires for each library + """ + return { + 'display_name': library.display_name, + 'library_key': unicode(library.location.library_key), + 'url': reverse_library_url('library_handler', unicode(library.location.library_key)), + 'org': library.display_org_with_default, + 'number': library.display_number_with_default, + } + # remove any courses in courses that are also in the in_process_course_actions list in_process_action_course_keys = [uca.course_key for uca in in_process_course_actions] courses = [ @@ -409,6 +433,8 @@ def format_in_process_course_view(uca): return render_to_response('index.html', { 'courses': courses, 'in_process_course_actions': in_process_course_actions, + 'libraries_enabled': LIBRARIES_ENABLED, + 'libraries': [format_library_for_view(lib) for lib in libraries], 'user': request.user, 'request_course_creator_url': reverse('contentstore.views.request_course_creator'), 'course_creator_status': _get_course_creator_status(request.user), diff --git a/cms/djangoapps/contentstore/views/helpers.py b/cms/djangoapps/contentstore/views/helpers.py index 34ef869f170f..3769c81978fd 100644 --- a/cms/djangoapps/contentstore/views/helpers.py +++ b/cms/djangoapps/contentstore/views/helpers.py @@ -13,7 +13,7 @@ from edxmako.shortcuts import render_to_string, render_to_response from xblock.core import XBlock from xmodule.modulestore.django import modulestore -from contentstore.utils import reverse_course_url, reverse_usage_url +from contentstore.utils import reverse_course_url, reverse_library_url, reverse_usage_url __all__ = ['edge', 'event', 'landing'] @@ -106,6 +106,9 @@ def xblock_studio_url(xblock, parent_xblock=None): url=reverse_course_url('course_handler', xblock.location.course_key), usage_key=urllib.quote(unicode(xblock.location)) ) + elif category == 'library': + library_key = xblock.location.course_key + return reverse_library_url('library_handler', library_key) else: return reverse_usage_url('container_handler', xblock.location) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index ffbcd68d18ad..815b67795157 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -47,6 +47,7 @@ from models.settings.course_grading import CourseGradingModel from cms.lib.xblock.runtime import handler_url, local_resource_url, get_asides from opaque_keys.edx.keys import UsageKey, CourseKey +from opaque_keys.edx.locator import LibraryUsageLocator __all__ = ['orphan_handler', 'xblock_handler', 'xblock_view_handler', 'xblock_outline_handler'] @@ -650,7 +651,9 @@ def _get_module_info(xblock, rewrite_static_links=True): ) # Pre-cache has changes for the entire course because we'll need it for the ancestor info - modulestore().has_changes(modulestore().get_course(xblock.location.course_key, depth=None)) + # Except library blocks which don't [yet] use draft/publish + if not isinstance(xblock.location, LibraryUsageLocator): + modulestore().has_changes(modulestore().get_course(xblock.location.course_key, depth=None)) # Note that children aren't being returned until we have a use case. return create_xblock_info(xblock, data=data, metadata=own_metadata(xblock), include_ancestor_info=True) @@ -691,12 +694,16 @@ def safe_get_username(user_id): return None + is_library_block = isinstance(xblock.location, LibraryUsageLocator) is_xblock_unit = is_unit(xblock, parent_xblock) - # this should not be calculated for Sections and Subsections on Unit page - has_changes = modulestore().has_changes(xblock) if (is_xblock_unit or course_outline) else None + # this should not be calculated for Sections and Subsections on Unit page or for library blocks + has_changes = modulestore().has_changes(xblock) if (is_xblock_unit or course_outline) and not is_library_block else None if graders is None: - graders = CourseGradingModel.fetch(xblock.location.course_key).graders + if not is_library_block: + 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) @@ -716,7 +723,7 @@ def safe_get_username(user_id): visibility_state = _compute_visibility_state(xblock, child_info, is_xblock_unit and has_changes) else: visibility_state = None - published = modulestore().has_published_version(xblock) + published = modulestore().has_published_version(xblock) if not is_library_block else None xblock_info = { "id": unicode(xblock.location), @@ -724,7 +731,7 @@ def safe_get_username(user_id): "category": xblock.category, "edited_on": get_default_time_display(xblock.subtree_edited_on) if xblock.subtree_edited_on else None, "published": published, - "published_on": get_default_time_display(xblock.published_on) if xblock.published_on else None, + "published_on": get_default_time_display(xblock.published_on) if published and xblock.published_on else None, "studio_url": xblock_studio_url(xblock, parent_xblock), "released_to_students": datetime.now(UTC) > xblock.start, "release_date": release_date, diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py new file mode 100644 index 000000000000..15e54a37ce35 --- /dev/null +++ b/cms/djangoapps/contentstore/views/library.py @@ -0,0 +1,185 @@ +""" +Views related to content libraries. +A content library is a structure containing XBlocks which can be re-used in the +multiple courses. +""" +from __future__ import absolute_import + +import json +import logging + +from contentstore.views.item import create_xblock_info +from contentstore.utils import reverse_library_url +from django.http import HttpResponseNotAllowed, 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.views.decorators.http import require_http_methods +from django_future.csrf import ensure_csrf_cookie +from edxmako.shortcuts import render_to_response +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.locator import LibraryLocator, LibraryUsageLocator +from xmodule.modulestore.exceptions import DuplicateCourseError +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore + +from .component import get_component_templates, CONTAINER_TEMPATES +from student.auth import has_course_author_access +from student.roles import CourseCreatorRole +from student import auth +from util.json_request import expect_json, JsonResponse, JsonResponseBadRequest + +__all__ = ['library_handler'] + +log = logging.getLogger(__name__) + +LIBRARIES_ENABLED = settings.FEATURES.get('ENABLE_CONTENT_LIBRARIES', False) + + +@login_required +@ensure_csrf_cookie +@require_http_methods(('GET', 'POST')) +def library_handler(request, library_key_string=None): + """ + RESTful interface to most content library related functionality. + """ + if not LIBRARIES_ENABLED: + log.exception("Attempted to use the content library API when the libraries feature is disabled.") + raise Http404 # Should never happen because we test the feature in urls.py also + + if library_key_string is not None and request.method == 'POST': + return HttpResponseNotAllowed(("POST",)) + + if request.method == 'POST': + return _create_library(request) + + # request method is get, since only GET and POST are allowed by @require_http_methods(('GET', 'POST')) + if library_key_string: + return _display_library(library_key_string, request) + + return _list_libraries(request) + + +def _display_library(library_key_string, request): + """ + Displays single library + """ + library_key = CourseKey.from_string(library_key_string) + if not isinstance(library_key, LibraryLocator): + log.exception("Non-library key passed to content libraries API.") # Should never happen due to url regex + raise Http404 # This is not a library + if not has_course_author_access(request.user, library_key): + log.exception(u"User %s tried to access library %s without permission", request.user.username, unicode(library_key)) + raise PermissionDenied() + + library = modulestore().get_library(library_key) + if library is None: + log.exception(u"Library %s not found", unicode(library_key)) + raise Http404 + + response_format = 'html' + if request.REQUEST.get('format', 'html') == 'json' or 'application/json' in request.META.get('HTTP_ACCEPT', 'text/html'): + response_format = 'json' + + return library_blocks_view(library, response_format) + + +def _list_libraries(request): + """ + List all accessible libraries + """ + lib_info = [ + { + "display_name": lib.display_name, + "library_key": unicode(lib.location.library_key), + } + for lib in modulestore().get_libraries() + if has_course_author_access(request.user, lib.location.library_key) + ] + return JsonResponse(lib_info) + + +@expect_json +def _create_library(request): + """ + Helper method for creating a new library. + """ + if not auth.has_access(request.user, CourseCreatorRole()): + log.exception(u"User %s tried to create a library without permission", request.user.username) + raise PermissionDenied() + display_name = None + try: + display_name = request.json['display_name'] + org = request.json['org'] + library = request.json.get('number', None) + if library is None: + library = request.json['library'] + store = modulestore() + with store.default_store(ModuleStoreEnum.Type.split): + new_lib = store.create_library( + org=org, + library=library, + user_id=request.user.id, + fields={"display_name": display_name}, + ) + except KeyError as error: + log.exception("Unable to create library - missing required JSON key.") + return JsonResponseBadRequest({ + "ErrMsg": _("Unable to create library - missing required field '{field}'".format(field=error.message)) + }) + except InvalidKeyError as error: + log.exception("Unable to create library - invalid key.") + return JsonResponseBadRequest({ + "ErrMsg": _("Unable to create library '{name}'.\n\n{err}").format(name=display_name, err=error.message)} + ) + except DuplicateCourseError: + log.exception("Unable to create library - one already exists with the same key.") + return JsonResponseBadRequest({ + 'ErrMsg': _( + 'There is already a library defined with the same ' + 'organization and library code. Please ' + 'change either organization or library code to be unique.' + ) + }) + + lib_key_str = unicode(new_lib.location.library_key) + return JsonResponse({ + 'url': reverse_library_url('library_handler', lib_key_str), + 'library_key': lib_key_str, + }) + + +def library_blocks_view(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. + """ + assert isinstance(library.location.library_key, LibraryLocator) + assert isinstance(library.location, LibraryUsageLocator) + + 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.course_id), + "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], + }) + + xblock_info = create_xblock_info(library, include_ancestor_info=False, graders=[]) + component_templates = get_component_templates(library) + + return render_to_response('library.html', { + 'context_library': library, + 'component_templates': json.dumps(component_templates), + 'xblock_info': xblock_info, + 'templates': CONTAINER_TEMPATES + }) diff --git a/cms/djangoapps/contentstore/views/tests/test_course_index.py b/cms/djangoapps/contentstore/views/tests/test_course_index.py index b0ea997b4d80..965879e15428 100644 --- a/cms/djangoapps/contentstore/views/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/views/tests/test_course_index.py @@ -6,7 +6,7 @@ import datetime from contentstore.tests.utils import CourseTestCase -from contentstore.utils import reverse_course_url, add_instructor +from contentstore.utils import reverse_course_url, reverse_library_url, add_instructor from student.auth import has_course_author_access from contentstore.views.course import course_outline_initial_state from contentstore.views.item import create_xblock_info, VisibilityState @@ -14,7 +14,7 @@ from util.date_utils import get_default_time_display from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, LibraryFactory from opaque_keys.edx.locator import CourseLocator from student.tests.factories import UserFactory from course_action_state.managers import CourseRerunUIStateManager @@ -61,6 +61,27 @@ def check_index_and_outline(self, authed_client): course_menu_link = outline_parsed.find_class('nav-course-courseware-outline')[0] self.assertEqual(course_menu_link.find("a").get("href"), link.get("href")) + def test_libraries_on_course_index(self): + """ + Test getting the list of libraries from the course listing page + """ + # Add a library: + lib1 = LibraryFactory.create() + + index_url = '/course/' + index_response = self.client.get(index_url, {}, HTTP_ACCEPT='text/html') + parsed_html = lxml.html.fromstring(index_response.content) + library_link_elements = parsed_html.find_class('library-link') + self.assertEqual(len(library_link_elements), 1) + link = library_link_elements[0] + self.assertEqual( + link.get("href"), + reverse_library_url('library_handler', lib1.location.library_key), + ) + # now test that url + outline_response = self.client.get(link.get("href"), {}, HTTP_ACCEPT='text/html') + self.assertEqual(outline_response.status_code, 200) + def test_is_staff_access(self): """ Test that people with is_staff see the courses and can navigate into them diff --git a/cms/djangoapps/contentstore/views/tests/test_helpers.py b/cms/djangoapps/contentstore/views/tests/test_helpers.py index 034a9002fb76..576ea388081d 100644 --- a/cms/djangoapps/contentstore/views/tests/test_helpers.py +++ b/cms/djangoapps/contentstore/views/tests/test_helpers.py @@ -4,7 +4,7 @@ from contentstore.tests.utils import CourseTestCase from contentstore.views.helpers import xblock_studio_url, xblock_type_display_name -from xmodule.modulestore.tests.factories import ItemFactory +from xmodule.modulestore.tests.factories import ItemFactory, LibraryFactory from django.utils import http @@ -50,6 +50,11 @@ def test_xblock_studio_url(self): display_name="My Video") self.assertIsNone(xblock_studio_url(video)) + # Verify library URL + library = LibraryFactory.create() + expected_url = u'/library/{}'.format(unicode(library.location.library_key)) + self.assertEqual(xblock_studio_url(library), expected_url) + def test_xblock_type_display_name(self): # Verify chapter type display name diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index b496a7ffc4f9..3f4fa86cba0c 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -24,7 +24,8 @@ from xmodule.capa_module import CapaDescriptor from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.factories import ItemFactory, check_mongo_calls +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import ItemFactory, LibraryFactory, check_mongo_calls from xmodule.x_module import STUDIO_VIEW, STUDENT_VIEW from xblock.exceptions import NoSuchHandlerError from opaque_keys.edx.keys import UsageKey, CourseKey @@ -1420,6 +1421,54 @@ def validate_xblock_info_consistency(self, xblock_info, has_ancestor_info=False, self.assertIsNone(xblock_info.get('edited_by', None)) +class TestLibraryXBlockInfo(ModuleStoreTestCase): + """ + Unit tests for XBlock Info for XBlocks in a content library + """ + def setUp(self): + super(TestLibraryXBlockInfo, self).setUp() + user_id = self.user.id + self.library = LibraryFactory.create() + self.top_level_html = ItemFactory.create( + parent_location=self.library.location, category='html', user_id=user_id, publish_item=False + ) + self.vertical = ItemFactory.create( + parent_location=self.library.location, category='vertical', user_id=user_id, publish_item=False + ) + self.child_html = ItemFactory.create( + parent_location=self.vertical.location, category='html', display_name='Test HTML Child Block', user_id=user_id, publish_item=False + ) + + def test_lib_xblock_info(self): + html_block = modulestore().get_item(self.top_level_html.location) + xblock_info = create_xblock_info(html_block) + self.validate_component_xblock_info(xblock_info, html_block) + self.assertIsNone(xblock_info.get('child_info', None)) + + def test_lib_child_xblock_info(self): + html_block = modulestore().get_item(self.child_html.location) + xblock_info = create_xblock_info(html_block, include_ancestor_info=True, include_child_info=True) + self.validate_component_xblock_info(xblock_info, html_block) + self.assertIsNone(xblock_info.get('child_info', None)) + ancestors = xblock_info['ancestor_info']['ancestors'] + self.assertEqual(len(ancestors), 2) + self.assertEqual(ancestors[0]['category'], 'vertical') + self.assertEqual(ancestors[0]['id'], unicode(self.vertical.location)) + self.assertEqual(ancestors[1]['category'], 'library') + + def validate_component_xblock_info(self, xblock_info, original_block): + """ + Validate that the xblock info is correct for the test component. + """ + self.assertEqual(xblock_info['category'], original_block.category) + self.assertEqual(xblock_info['id'], unicode(original_block.location)) + self.assertEqual(xblock_info['display_name'], original_block.display_name) + self.assertIsNone(xblock_info.get('has_changes', None)) + self.assertIsNone(xblock_info.get('published', None)) + self.assertIsNone(xblock_info.get('published_on', None)) + self.assertIsNone(xblock_info.get('graders', None)) + + class TestXBlockPublishingInfo(ItemTest): """ Unit tests for XBlock's outline handling. diff --git a/cms/djangoapps/contentstore/views/tests/test_library.py b/cms/djangoapps/contentstore/views/tests/test_library.py new file mode 100644 index 000000000000..8cae9710873f --- /dev/null +++ b/cms/djangoapps/contentstore/views/tests/test_library.py @@ -0,0 +1,185 @@ +""" +Unit tests for contentstore.views.library + +More important high-level tests are in contentstore/tests/test_libraries.py +""" +from contentstore.tests.utils import AjaxEnabledTestClient, parse_json +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import LibraryFactory +from mock import patch +from opaque_keys.edx.locator import CourseKey, LibraryLocator +import ddt + +LIBRARY_REST_URL = '/library/' # URL for GET/POST requests involving libraries + + +def make_url_for_lib(key): + """ Get the RESTful/studio URL for testing the given library """ + if isinstance(key, LibraryLocator): + key = unicode(key) + return LIBRARY_REST_URL + key + + +@ddt.ddt +class UnitTestLibraries(ModuleStoreTestCase): + """ + Unit tests for library views + """ + + def setUp(self): + user_password = super(UnitTestLibraries, self).setUp() + + self.client = AjaxEnabledTestClient() + self.client.login(username=self.user.username, password=user_password) + + ###################################################### + # Tests for /library/ - list and create libraries: + + @patch("contentstore.views.library.LIBRARIES_ENABLED", False) + def test_with_libraries_disabled(self): + """ + The library URLs should return 404 if libraries are disabled. + """ + response = self.client.get_json(LIBRARY_REST_URL) + self.assertEqual(response.status_code, 404) + + def test_list_libraries(self): + """ + Test that we can GET /library/ to list all libraries visible to the current user. + """ + # Create some more libraries + libraries = [LibraryFactory.create() for _ in range(0, 3)] + lib_dict = dict([(lib.location.library_key, lib) for lib in libraries]) + + response = self.client.get_json(LIBRARY_REST_URL) + self.assertEqual(response.status_code, 200) + lib_list = parse_json(response) + self.assertEqual(len(lib_list), len(libraries)) + for entry in lib_list: + self.assertIn("library_key", entry) + self.assertIn("display_name", entry) + key = CourseKey.from_string(entry["library_key"]) + self.assertIn(key, lib_dict) + self.assertEqual(entry["display_name"], lib_dict[key].display_name) + del lib_dict[key] # To ensure no duplicates are matched + + @ddt.data("delete", "put") + def test_bad_http_verb(self, verb): + """ + We should get an error if we do weird requests to /library/ + """ + response = getattr(self.client, verb)(LIBRARY_REST_URL) + self.assertEqual(response.status_code, 405) + + def test_create_library(self): + """ Create a library. """ + response = self.client.ajax_post(LIBRARY_REST_URL, { + 'org': 'org', + 'library': 'lib', + 'display_name': "New Library", + }) + self.assertEqual(response.status_code, 200) + # That's all we check. More detailed tests are in contentstore.tests.test_libraries... + + @patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': True}) + def test_lib_create_permission(self): + """ + Users who aren't given course creator roles shouldn't be able to create + libraries either. + """ + self.client.logout() + ns_user, password = self.create_non_staff_user() + self.client.login(username=ns_user.username, password=password) + + response = self.client.ajax_post(LIBRARY_REST_URL, { + 'org': 'org', 'library': 'lib', 'display_name': "New Library", + }) + self.assertEqual(response.status_code, 403) + + @ddt.data( + {}, + {'org': 'org'}, + {'library': 'lib'}, + {'org': 'C++', 'library': 'lib', 'display_name': 'Lib with invalid characters in key'}, + {'org': 'Org', 'library': 'Wh@t?', 'display_name': 'Lib with invalid characters in key'}, + ) + def test_create_library_invalid(self, data): + """ + Make sure we are prevented from creating libraries with invalid keys/data + """ + response = self.client.ajax_post(LIBRARY_REST_URL, data) + self.assertEqual(response.status_code, 400) + + def test_no_duplicate_libraries(self): + """ + We should not be able to create multiple libraries with the same key + """ + lib = LibraryFactory.create() + lib_key = lib.location.library_key + response = self.client.ajax_post(LIBRARY_REST_URL, { + 'org': lib_key.org, + 'library': lib_key.library, + 'display_name': "A Duplicate key, same as 'lib'", + }) + self.assertIn('already a library defined', parse_json(response)['ErrMsg']) + self.assertEqual(response.status_code, 400) + + ###################################################### + # Tests for /library/:lib_key/ - get a specific library as JSON or HTML editing view + + def test_get_lib_info(self): + """ + Test that we can get data about a library (in JSON format) using /library/:key/ + """ + # Create a library + lib_key = LibraryFactory.create().location.library_key + # Re-load the library from the modulestore, explicitly including version information: + lib = self.store.get_library(lib_key, remove_version=False, remove_branch=False) + version = lib.location.library_key.version_guid + self.assertNotEqual(version, None) + + response = self.client.get_json(make_url_for_lib(lib_key)) + self.assertEqual(response.status_code, 200) + info = parse_json(response) + self.assertEqual(info['display_name'], lib.display_name) + self.assertEqual(info['library_id'], unicode(lib_key)) + self.assertEqual(info['previous_version'], None) + self.assertNotEqual(info['version'], None) + self.assertNotEqual(info['version'], '') + self.assertEqual(info['version'], unicode(version)) + + def test_get_lib_edit_html(self): + """ + Test that we can get the studio view for editing a library using /library/:key/ + """ + lib = LibraryFactory.create() + + response = self.client.get(make_url_for_lib(lib.location.library_key)) + self.assertEqual(response.status_code, 200) + self.assertIn("' + errorMessage + '

'); $('.new-course-save').addClass('is-disabled'); }); }; - var cancelNewCourse = function (e) { - e.preventDefault(); - $('.new-course-button').removeClass('is-disabled'); - $('.wrapper-create-course').removeClass('is-shown'); - // Clear out existing fields and errors - _.each( - ['.new-course-name', '.new-course-org', '.new-course-number', '.new-course-run'], - function (field) { - $(field).val(''); - } - ); - $('#course_creation_error').html(''); - $('.wrap-error').removeClass('is-shown'); - $('.new-course-save').off('click'); + var makeCancelHandler = function (addType) { + return function(e) { + e.preventDefault(); + $('.new-'+addType+'-button').removeClass('is-disabled'); + $('.wrapper-create-'+addType).removeClass('is-shown'); + // Clear out existing fields and errors + $('#create-'+addType+'-form input[type=text]').val(''); + $('#'+addType+'_creation_error').html(''); + $('.create-'+addType+' .wrap-error').removeClass('is-shown'); + $('.new-'+addType+'-save').off('click'); + }; }; var addNewCourse = function (e) { @@ -73,18 +89,70 @@ define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape", "js/vie var $courseName = $('.new-course-name'); $courseName.focus().select(); $('.new-course-save').on('click', saveNewCourse); - $cancelButton.bind('click', cancelNewCourse); + $cancelButton.bind('click', makeCancelHandler('course')); CancelOnEscape($cancelButton); CreateCourseUtils.configureHandlers(); }; + var saveNewLibrary = function (e) { + e.preventDefault(); + + if (CreateLibraryUtils.hasInvalidRequiredFields()) { + return; + } + + var $newLibraryForm = $(this).closest('#create-library-form'); + var display_name = $newLibraryForm.find('.new-library-name').val(); + var org = $newLibraryForm.find('.new-library-org').val(); + var number = $newLibraryForm.find('.new-library-number').val(); + + var lib_info = { + org: org, + number: number, + display_name: display_name, + }; + + analytics.track('Created a Library', lib_info); + CreateLibraryUtils.createLibrary(lib_info, function (errorMessage) { + $('.create-library .wrap-error').addClass('is-shown'); + $('#library_creation_error').html('

' + errorMessage + '

'); + $('.new-library-save').addClass('is-disabled'); + }); + }; + + var addNewLibrary = function (e) { + e.preventDefault(); + $('.new-library-button').addClass('is-disabled'); + $('.new-library-save').addClass('is-disabled'); + var $newLibrary = $('.wrapper-create-library').addClass('is-shown'); + var $cancelButton = $newLibrary.find('.new-library-cancel'); + var $libraryName = $('.new-library-name'); + $libraryName.focus().select(); + $('.new-library-save').on('click', saveNewLibrary); + $cancelButton.bind('click', makeCancelHandler('library')); + CancelOnEscape($cancelButton); + + CreateLibraryUtils.configureHandlers(); + }; + + var showTab = function(tab) { + return function(e) { + e.preventDefault(); + $('.courses-tab').toggleClass('active', tab === 'courses'); + $('.libraries-tab').toggleClass('active', tab === 'libraries'); + }; + }; + var onReady = function () { $('.new-course-button').bind('click', addNewCourse); + $('.new-library-button').bind('click', addNewLibrary); $('.dismiss-button').bind('click', ViewUtils.deleteNotificationHandler(function () { ViewUtils.reload(); })); $('.action-reload').bind('click', ViewUtils.reload); + $('#course-index-tabs .courses-tab').bind('click', showTab('courses')); + $('#course-index-tabs .libraries-tab').bind('click', showTab('libraries')); }; domReady(onReady); diff --git a/cms/static/js/spec/views/pages/course_rerun_spec.js b/cms/static/js/spec/views/pages/course_rerun_spec.js index bcf881b98685..320f8d6557ec 100644 --- a/cms/static/js/spec/views/pages/course_rerun_spec.js +++ b/cms/static/js/spec/views/pages/course_rerun_spec.js @@ -49,12 +49,12 @@ define(["jquery", "js/common_helpers/ajax_helpers", "js/spec_helpers/view_helper describe("Field validation", function () { it("returns a message for an empty string", function () { - var message = CreateCourseUtils.validateRequiredField(''); + var message = ViewUtils.validateRequiredField(''); expect(message).not.toBe(''); }); it("does not return a message for a non empty string", function () { - var message = CreateCourseUtils.validateRequiredField('edX'); + var message = ViewUtils.validateRequiredField('edX'); expect(message).toBe(''); }); }); diff --git a/cms/static/js/spec/views/pages/index_spec.js b/cms/static/js/spec/views/pages/index_spec.js index 2a7974cd591d..52b71a834957 100644 --- a/cms/static/js/spec/views/pages/index_spec.js +++ b/cms/static/js/spec/views/pages/index_spec.js @@ -2,7 +2,7 @@ define(["jquery", "js/common_helpers/ajax_helpers", "js/spec_helpers/view_helper "js/views/utils/view_utils"], function ($, AjaxHelpers, ViewHelpers, IndexUtils, ViewUtils) { describe("Course listing page", function () { - var mockIndexPageHTML = readFixtures('mock/mock-index-page.underscore'), fillInFields; + var mockIndexPageHTML = readFixtures('mock/mock-index-page.underscore'); var fillInFields = function (org, number, run, name) { $('.new-course-org').val(org); @@ -11,6 +11,12 @@ define(["jquery", "js/common_helpers/ajax_helpers", "js/spec_helpers/view_helper $('.new-course-name').val(name); }; + var fillInLibraryFields = function(org, number, name) { + $('.new-library-org').val(org).keyup(); + $('.new-library-number').val(number).keyup(); + $('.new-library-name').val(name).keyup(); + }; + beforeEach(function () { ViewHelpers.installMockAnalytics(); appendSetFixtures(mockIndexPageHTML); @@ -57,9 +63,83 @@ define(["jquery", "js/common_helpers/ajax_helpers", "js/spec_helpers/view_helper AjaxHelpers.respondWithJson(requests, { ErrMsg: 'error message' }); - expect($('.wrap-error')).toHaveClass('is-shown'); + expect($('.create-course .wrap-error')).toHaveClass('is-shown'); expect($('#course_creation_error')).toContainText('error message'); expect($('.new-course-save')).toHaveClass('is-disabled'); }); + + it("saves new libraries", function () { + var requests = AjaxHelpers.requests(this); + var redirectSpy = spyOn(ViewUtils, 'redirect'); + $('.new-library-button').click(); + fillInLibraryFields('DemoX', 'DM101', 'Demo library'); + $('.new-library-save').click(); + AjaxHelpers.expectJsonRequest(requests, 'POST', '/library/', { + org: 'DemoX', + number: 'DM101', + display_name: 'Demo library' + }); + AjaxHelpers.respondWithJson(requests, { + url: 'dummy_test_url' + }); + expect(redirectSpy).toHaveBeenCalledWith('dummy_test_url'); + }); + + it("displays an error when a required field is blank", function () { + var requests = AjaxHelpers.requests(this); + var requests_count = requests.length; + $('.new-library-button').click(); + var values = ['DemoX', 'DM101', 'Demo library']; + // Try making each of these three values empty one at a time and ensure the form won't submit: + for (var i=0; i characters.'); - // Ensure that org/course_num/run < 65 chars. + // Ensure that org, course_num and run passes checkTotalKeyLengthViolations validateTotalCourseItemsLength = function () { - var totalLength = _.reduce( + ViewUtils.checkTotalKeyLengthViolations( + selectors, classes, [selectors.org, selectors.number, selectors.run], - function (sum, ele) { - return sum + $(ele).val().length; - }, 0 + keyLengthViolationMessage ); - if (totalLength > 65) { - $(selectors.errorWrapper).addClass(classes.shown).removeClass(classes.hiding); - $(selectors.errorMessage).html('

' + gettext('The combined length of the organization, course number, and course run fields cannot be more than 65 characters.') + '

'); - $(selectors.save).addClass(classes.disabled); - } - else { - $(selectors.errorWrapper).removeClass(classes.shown).addClass(classes.hiding); - } }; setNewCourseFieldInErr = function (el, msg) { @@ -117,7 +90,7 @@ define(["jquery", "underscore", "gettext", "js/views/utils/view_utils"], if (event.keyCode === 9) { return; } - var error = validateCourseItemEncoding($ele.val()); + var error = validateURLItemEncoding($ele.val(), $(selectors.allowUnicode).val() === 'True'); setNewCourseFieldInErr($ele.parent(), error); validateTotalCourseItemsLength(); if (!validateFilledFields()) { @@ -138,8 +111,6 @@ define(["jquery", "underscore", "gettext", "js/views/utils/view_utils"], }; return { - validateRequiredField: validateRequiredField, - validateCourseItemEncoding: validateCourseItemEncoding, validateTotalCourseItemsLength: validateTotalCourseItemsLength, setNewCourseFieldInErr: setNewCourseFieldInErr, hasInvalidRequiredFields: hasInvalidRequiredFields, diff --git a/cms/static/js/views/utils/create_library_utils.js b/cms/static/js/views/utils/create_library_utils.js new file mode 100644 index 000000000000..b53f7c8b5398 --- /dev/null +++ b/cms/static/js/views/utils/create_library_utils.js @@ -0,0 +1,129 @@ +/** + * Provides utilities for validating libraries during creation. + */ +define(["jquery", "underscore", "gettext", "js/views/utils/view_utils"], + function ($, _, gettext, ViewUtils) { + "use strict"; + return function (selectors, classes) { + var validateTotalKeyLength, setNewLibraryFieldInErr, hasInvalidRequiredFields, + createLibrary, validateFilledFields, configureHandlers; + + var validateRequiredField = ViewUtils.validateRequiredField; + var validateURLItemEncoding = ViewUtils.validateURLItemEncoding; + + var keyLengthViolationMessage = gettext("The combined length of the organization and library code fields cannot be more than <%=limit%> characters."); + + // Ensure that org/librarycode passes validateTotalKeyLength check + validateTotalKeyLength = function () { + ViewUtils.checkTotalKeyLengthViolations( + selectors, classes, + [selectors.org, selectors.number], + keyLengthViolationMessage + ); + }; + + setNewLibraryFieldInErr = function (element, message) { + if (message) { + element.addClass(classes.error); + element.children(selectors.tipError).addClass(classes.showing).removeClass(classes.hiding).text(message); + $(selectors.save).addClass(classes.disabled); + } + else { + element.removeClass(classes.error); + element.children(selectors.tipError).addClass(classes.hiding).removeClass(classes.showing); + // One "error" div is always present, but hidden or shown + if ($(selectors.error).length === 1) { + $(selectors.save).removeClass(classes.disabled); + } + } + }; + + // One final check for empty values + hasInvalidRequiredFields = function () { + return _.reduce( + [selectors.name, selectors.org, selectors.number], + function (acc, element) { + var $element = $(element); + var error = validateRequiredField($element.val()); + setNewLibraryFieldInErr($element.parent(), error); + return error ? true : acc; + }, + false + ); + }; + + createLibrary = function (libraryInfo, errorHandler) { + $.postJSON( + '/library/', + libraryInfo + ).done(function (data) { + ViewUtils.redirect(data.url); + }).fail(function(jqXHR, textStatus, errorThrown) { + var reason = errorThrown; + if (jqXHR.responseText) { + try { + var detailedReason = $.parseJSON(jqXHR.responseText).ErrMsg; + if (detailedReason) { + reason = detailedReason; + } + } catch (e) {} + } + errorHandler(reason); + }); + }; + + // Ensure that all fields are not empty + validateFilledFields = function () { + return _.reduce( + [selectors.org, selectors.number, selectors.name], + function (acc, element) { + var $element = $(element); + return $element.val().length !== 0 ? acc : false; + }, + true + ); + }; + + // Handle validation asynchronously + configureHandlers = function () { + _.each( + [selectors.org, selectors.number], + function (element) { + var $element = $(element); + $element.on('keyup', function (event) { + // Don't bother showing "required field" error when + // the user tabs into a new field; this is distracting + // and unnecessary + if (event.keyCode === $.ui.keyCode.TAB) { + return; + } + var error = validateURLItemEncoding($element.val(), $(selectors.allowUnicode).val() === 'True'); + setNewLibraryFieldInErr($element.parent(), error); + validateTotalKeyLength(); + if (!validateFilledFields()) { + $(selectors.save).addClass(classes.disabled); + } + }); + } + ); + var $name = $(selectors.name); + $name.on('keyup', function () { + var error = validateRequiredField($name.val()); + setNewLibraryFieldInErr($name.parent(), error); + validateTotalKeyLength(); + if (!validateFilledFields()) { + $(selectors.save).addClass(classes.disabled); + } + }); + }; + + return { + validateTotalKeyLength: validateTotalKeyLength, + setNewLibraryFieldInErr: setNewLibraryFieldInErr, + hasInvalidRequiredFields: hasInvalidRequiredFields, + createLibrary: createLibrary, + validateFilledFields: validateFilledFields, + configureHandlers: configureHandlers + }; + }; + }); diff --git a/cms/static/js/views/utils/view_utils.js b/cms/static/js/views/utils/view_utils.js index 27d969f523f7..3f06a4f62942 100644 --- a/cms/static/js/views/utils/view_utils.js +++ b/cms/static/js/views/utils/view_utils.js @@ -5,7 +5,11 @@ define(["jquery", "underscore", "gettext", "js/views/feedback_notification", "js function ($, _, gettext, NotificationView, PromptView) { var toggleExpandCollapse, showLoadingIndicator, hideLoadingIndicator, confirmThenRunOperation, runOperationShowingMessage, disableElementWhileRunning, getScrollOffset, setScrollOffset, - setScrollTop, redirect, reload, hasChangedAttributes, deleteNotificationHandler; + setScrollTop, redirect, reload, hasChangedAttributes, deleteNotificationHandler, + validateRequiredField, validateURLItemEncoding, validateTotalKeyLength, checkTotalKeyLengthViolations; + + // see https://openedx.atlassian.net/browse/TNL-889 for what is it and why it's 65 + var MAX_SUM_KEY_LENGTH = 65; /** * Toggles the expanded state of the current element. @@ -173,6 +177,55 @@ define(["jquery", "underscore", "gettext", "js/views/feedback_notification", "js return false; }; + /** + * Helper method for course/library creation - verifies a required field is not blank. + */ + validateRequiredField = function (msg) { + return msg.length === 0 ? gettext('Required field.') : ''; + }; + + /** + * Helper method for course/library creation. + * Check that a course (org, number, run) doesn't use any special characters + */ + validateURLItemEncoding = function (item, allowUnicode) { + var required = validateRequiredField(item); + if (required) { + return required; + } + if (allowUnicode) { + if (/\s/g.test(item)) { + return gettext('Please do not use any spaces in this field.'); + } + } + else { + if (item !== encodeURIComponent(item)) { + return gettext('Please do not use any spaces or special characters in this field.'); + } + } + return ''; + }; + + // Ensure that sum length of key field values <= ${MAX_SUM_KEY_LENGTH} chars. + validateTotalKeyLength = function (key_field_selectors) { + var totalLength = _.reduce( + key_field_selectors, + function (sum, ele) { return sum + $(ele).val().length;}, + 0 + ); + return totalLength <= MAX_SUM_KEY_LENGTH; + }; + + checkTotalKeyLengthViolations = function(selectors, classes, key_field_selectors, message_tpl) { + if (!validateTotalKeyLength(key_field_selectors)) { + $(selectors.errorWrapper).addClass(classes.shown).removeClass(classes.hiding); + $(selectors.errorMessage).html('

' + _.template(message_tpl, {limit: MAX_SUM_KEY_LENGTH}) + '

'); + $(selectors.save).addClass(classes.disabled); + } else { + $(selectors.errorWrapper).removeClass(classes.shown).addClass(classes.hiding); + } + }; + return { 'toggleExpandCollapse': toggleExpandCollapse, 'showLoadingIndicator': showLoadingIndicator, @@ -186,6 +239,10 @@ define(["jquery", "underscore", "gettext", "js/views/feedback_notification", "js 'setScrollOffset': setScrollOffset, 'redirect': redirect, 'reload': reload, - 'hasChangedAttributes': hasChangedAttributes + 'hasChangedAttributes': hasChangedAttributes, + 'validateRequiredField': validateRequiredField, + 'validateURLItemEncoding': validateURLItemEncoding, + 'validateTotalKeyLength': validateTotalKeyLength, + 'checkTotalKeyLengthViolations': checkTotalKeyLengthViolations }; }); diff --git a/cms/static/sass/elements/_forms.scss b/cms/static/sass/elements/_forms.scss index 5979c92ff740..9befe26c7abd 100644 --- a/cms/static/sass/elements/_forms.scss +++ b/cms/static/sass/elements/_forms.scss @@ -394,7 +394,6 @@ form { // ELEM: form wrapper .wrapper-create-element { height: 0; - margin-bottom: $baseline; opacity: 0.0; pointer-events: none; overflow: hidden; @@ -405,6 +404,7 @@ form { &.is-shown { height: auto; // define a specific height for the animating version of this UI to work properly + margin-bottom: $baseline; opacity: 1.0; pointer-events: auto; } diff --git a/cms/static/sass/views/_dashboard.scss b/cms/static/sass/views/_dashboard.scss index 494500fd4d28..1a1430f0127e 100644 --- a/cms/static/sass/views/_dashboard.scss +++ b/cms/static/sass/views/_dashboard.scss @@ -289,10 +289,42 @@ // ==================== + // Course/Library tabs + #course-index-tabs { + margin: 0; + font-size: 1.4rem; + + li { + display: inline-block; + line-height: $baseline*2; + margin: 0 10px; + + &.active, &:hover { + border-bottom: 4px solid $blue; + } + + a { + color: $blue; + cursor: pointer; + display: inline-block; + } + + &.active a { + color: $gray-d2; + } + } + } + // ELEM: course listings - .courses { - margin: $baseline 0; + .courses-tab, .libraries-tab { + display: none; + + &.active { + display: block; + } + } + .courses, .libraries { .title { @extend %t-title6; margin-bottom: $baseline; @@ -311,7 +343,6 @@ } .list-courses { - margin-top: $baseline; border-radius: 3px; border: 1px solid $gray-l2; background: $white; @@ -622,7 +653,7 @@ // course listings - .create-course { + .create-course, .create-library { .row { @include clearfix(); diff --git a/cms/templates/base.html b/cms/templates/base.html index 8a8a70c0ded2..0b31ebf4239c 100644 --- a/cms/templates/base.html +++ b/cms/templates/base.html @@ -21,6 +21,8 @@ % if context_course: <% ctx_loc = context_course.location %> ${context_course.display_name_with_default | h} | + % elif context_library: + ${context_library.display_name_with_default | h} | % endif edX Studio diff --git a/cms/templates/container.html b/cms/templates/container.html index b24cffe2ea10..663184a9b192 100644 --- a/cms/templates/container.html +++ b/cms/templates/container.html @@ -18,13 +18,6 @@ <%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: +% endfor + + +<%block name="requirejs"> + require(["js/factories/library"], function(LibraryFactory) { + LibraryFactory( + ${component_templates | n}, + ${json.dumps(xblock_info) | n} + ); + }); + + +<%block name="content"> + + +
+
+
+

${context_library.display_name_with_default | h}

+
+
+
+ +
+
+
+ +
+
+ +
+

${_("Loading")}

+
+
+ +
+
+
+ diff --git a/cms/urls.py b/cms/urls.py index 7e06f5033909..0f0a898e98b3 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -110,6 +110,12 @@ url(r'^i18n.js$', 'django.views.i18n.javascript_catalog', js_info_dict), ) +if settings.FEATURES.get('ENABLE_CONTENT_LIBRARIES'): + LIBRARY_KEY_PATTERN = r'(?Plibrary-v1:[^/+]+\+[^/+]+)' + urlpatterns += ( + url(r'^library/{}?$'.format(LIBRARY_KEY_PATTERN), + 'contentstore.views.library_handler', name='library_handler'), + ) if settings.FEATURES.get('ENABLE_EXPORT_GIT'): urlpatterns += (url( diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py index bb59da2a7d76..12c2f4c84b0f 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -5,7 +5,7 @@ from xmodule.modulestore.split_mongo.split import SplitMongoModuleStore, EXCLUDE_ALL from xmodule.exceptions import InvalidVersionError from xmodule.modulestore import ModuleStoreEnum -from xmodule.modulestore.exceptions import InsufficientSpecificationError +from xmodule.modulestore.exceptions import InsufficientSpecificationError, ItemNotFoundError from xmodule.modulestore.draft_and_published import ( ModuleStoreDraftAndPublished, DIRECT_ONLY_CATEGORIES, UnsupportedRevisionError ) @@ -409,7 +409,11 @@ def convert_to_draft(self, location, user_id): pass def _get_head(self, xblock, branch): - course_structure = self._lookup_course(xblock.location.course_key.for_branch(branch)).structure + try: + course_structure = self._lookup_course(xblock.location.course_key.for_branch(branch)).structure + except ItemNotFoundError: + # There is no published version xblock container, e.g. Library + return None return self._get_block_from_structure(course_structure, BlockKey.from_usage_key(xblock.location)) def _get_version(self, block): diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py b/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py index 1447b0238414..13df157602e9 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py @@ -206,3 +206,14 @@ def test_library_author_view(self): with patch('xmodule.x_module.descriptor_global_get_asides', lambda block: []): result = library.render(AUTHOR_VIEW, context) self.assertIn(message, result.content) + + def test_xblock_in_lib_have_published_version_returns_false(self): + library = LibraryFactory.create(modulestore=self.store) + block = ItemFactory.create( + category="html", + parent_location=library.location, + user_id=self.user_id, + publish_item=False, + modulestore=self.store, + ) + self.assertFalse(self.store.has_published_version(block)) diff --git a/common/static/js/xblock/core.js b/common/static/js/xblock/core.js index ffef2b27627b..99b2ae0489b0 100644 --- a/common/static/js/xblock/core.js +++ b/common/static/js/xblock/core.js @@ -23,10 +23,10 @@ if (runtime && version && initFnName) { return new window[runtime]['v' + version]; } else { - if (!runtime || !version || !initFnName) { + if (runtime || version || initFnName) { var elementTag = $('
').append($element.clone()).html(); console.log('Block ' + elementTag + ' is missing data-runtime, data-runtime-version or data-init, and can\'t be initialized'); - } + } // else this XBlock doesn't have a JS init function. return null; } } diff --git a/common/test/acceptance/fixtures/base.py b/common/test/acceptance/fixtures/base.py new file mode 100644 index 000000000000..0f2e2723839e --- /dev/null +++ b/common/test/acceptance/fixtures/base.py @@ -0,0 +1,196 @@ +""" +Common code shared by course and library fixtures. +""" +import re +import requests +import json +from lazy import lazy + +from . import STUDIO_BASE_URL + + +class StudioApiLoginError(Exception): + """ + Error occurred while logging in to the Studio API. + """ + pass + + +class StudioApiFixture(object): + """ + Base class for fixtures that use the Studio restful API. + """ + def __init__(self): + # Info about the auto-auth user used to create the course/library. + self.user = {} + + @lazy + def session(self): + """ + Log in as a staff user, then return a `requests` `session` object for the logged in user. + Raises a `StudioApiLoginError` if the login fails. + """ + # Use auto-auth to retrieve the session for a logged in user + session = requests.Session() + response = session.get(STUDIO_BASE_URL + "/auto_auth?staff=true") + + # Return the session from the request + if response.ok: + # auto_auth returns information about the newly created user + # capture this so it can be used by by the testcases. + user_pattern = re.compile(r'Logged in user {0} \({1}\) with password {2} and user_id {3}'.format( + r'(?P\S+)', r'(?P[^\)]+)', r'(?P\S+)', r'(?P\d+)')) + user_matches = re.match(user_pattern, response.text) + if user_matches: + self.user = user_matches.groupdict() + + return session + + else: + msg = "Could not log in to use Studio restful API. Status code: {0}".format(response.status_code) + raise StudioApiLoginError(msg) + + @lazy + def session_cookies(self): + """ + Log in as a staff user, then return the cookies for the session (as a dict) + Raises a `StudioApiLoginError` if the login fails. + """ + return {key: val for key, val in self.session.cookies.items()} + + @lazy + def headers(self): + """ + Default HTTP headers dict. + """ + return { + 'Content-type': 'application/json', + 'Accept': 'application/json', + 'X-CSRFToken': self.session_cookies.get('csrftoken', '') + } + + +class FixtureError(Exception): + """ + Error occurred while installing a course or library fixture. + """ + pass + + +class XBlockContainerFixture(StudioApiFixture): + """ + Base class for course and library fixtures. + """ + + def __init__(self): + self.children = [] + super(XBlockContainerFixture, self).__init__() + + def add_children(self, *args): + """ + Add children XBlock to the container. + Each item in `args` is an `XBlockFixtureDesc` object. + + Returns the fixture to allow chaining. + """ + self.children.extend(args) + return self + + def _create_xblock_children(self, parent_loc, xblock_descriptions): + """ + Recursively create XBlock children. + """ + for desc in xblock_descriptions: + loc = self.create_xblock(parent_loc, desc) + self._create_xblock_children(loc, desc.children) + + def create_xblock(self, parent_loc, xblock_desc): + """ + Create an XBlock with `parent_loc` (the location of the parent block) + and `xblock_desc` (an `XBlockFixtureDesc` instance). + """ + create_payload = { + 'category': xblock_desc.category, + 'display_name': xblock_desc.display_name, + } + + if parent_loc is not None: + create_payload['parent_locator'] = parent_loc + + # Create the new XBlock + response = self.session.post( + STUDIO_BASE_URL + '/xblock/', + data=json.dumps(create_payload), + headers=self.headers, + ) + + if not response.ok: + msg = "Could not create {0}. Status was {1}".format(xblock_desc, response.status_code) + raise FixtureError(msg) + + try: + loc = response.json().get('locator') + xblock_desc.locator = loc + except ValueError: + raise FixtureError("Could not decode JSON from '{0}'".format(response.content)) + + # Configure the XBlock + response = self.session.post( + STUDIO_BASE_URL + '/xblock/' + loc, + data=xblock_desc.serialize(), + headers=self.headers, + ) + + if response.ok: + return loc + else: + raise FixtureError("Could not update {0}. Status code: {1}".format(xblock_desc, response.status_code)) + + def _update_xblock(self, locator, data): + """ + Update the xblock at `locator`. + """ + # Create the new XBlock + response = self.session.put( + "{}/xblock/{}".format(STUDIO_BASE_URL, locator), + data=json.dumps(data), + headers=self.headers, + ) + + if not response.ok: + msg = "Could not update {} with data {}. Status was {}".format(locator, data, response.status_code) + raise FixtureError(msg) + + def _encode_post_dict(self, post_dict): + """ + Encode `post_dict` (a dictionary) as UTF-8 encoded JSON. + """ + return json.dumps({ + k: v.encode('utf-8') if isinstance(v, basestring) else v + for k, v in post_dict.items() + }) + + def get_nested_xblocks(self, category=None): + """ + Return a list of nested XBlocks for the container that can be filtered by + category. + """ + xblocks = self._get_nested_xblocks(self) + if category: + xblocks = [x for x in xblocks if x.category == category] + return xblocks + + def _get_nested_xblocks(self, xblock_descriptor): + """ + Return a list of nested XBlocks for the container. + """ + xblocks = list(xblock_descriptor.children) + for child in xblock_descriptor.children: + xblocks.extend(self._get_nested_xblocks(child)) + return xblocks + + def _publish_xblock(self, locator): + """ + Publish the xblock at `locator`. + """ + self._update_xblock(locator, {'publish': 'make_public'}) diff --git a/common/test/acceptance/fixtures/course.py b/common/test/acceptance/fixtures/course.py index 69836fbee048..1e5bca8a337f 100644 --- a/common/test/acceptance/fixtures/course.py +++ b/common/test/acceptance/fixtures/course.py @@ -4,77 +4,17 @@ import mimetypes import json -import re + import datetime -import requests + from textwrap import dedent from collections import namedtuple from path import path -from lazy import lazy + from opaque_keys.edx.keys import CourseKey from . import STUDIO_BASE_URL - - -class StudioApiLoginError(Exception): - """ - Error occurred while logging in to the Studio API. - """ - pass - - -class StudioApiFixture(object): - """ - Base class for fixtures that use the Studio restful API. - """ - def __init__(self): - # Info about the auto-auth user used to create the course. - self.user = {} - - @lazy - def session(self): - """ - Log in as a staff user, then return a `requests` `session` object for the logged in user. - Raises a `StudioApiLoginError` if the login fails. - """ - # Use auto-auth to retrieve the session for a logged in user - session = requests.Session() - response = session.get(STUDIO_BASE_URL + "/auto_auth?staff=true") - - # Return the session from the request - if response.ok: - # auto_auth returns information about the newly created user - # capture this so it can be used by by the testcases. - user_pattern = re.compile('Logged in user {0} \({1}\) with password {2} and user_id {3}'.format( - '(?P\S+)', '(?P[^\)]+)', '(?P\S+)', '(?P\d+)')) - user_matches = re.match(user_pattern, response.text) - if user_matches: - self.user = user_matches.groupdict() - - return session - - else: - msg = "Could not log in to use Studio restful API. Status code: {0}".format(response.status_code) - raise StudioApiLoginError(msg) - - @lazy - def session_cookies(self): - """ - Log in as a staff user, then return the cookies for the session (as a dict) - Raises a `StudioApiLoginError` if the login fails. - """ - return {key: val for key, val in self.session.cookies.items()} - - @lazy - def headers(self): - """ - Default HTTP headers dict. - """ - return { - 'Content-type': 'application/json', - 'Accept': 'application/json', - 'X-CSRFToken': self.session_cookies.get('csrftoken', '') - } +from .base import XBlockContainerFixture, FixtureError class XBlockFixtureDesc(object): @@ -105,7 +45,7 @@ def __init__(self, category, display_name, data=None, metadata=None, grader_type def add_children(self, *args): """ Add child XBlocks to this XBlock. - Each item in `args` is an `XBlockFixtureDescriptor` object. + Each item in `args` is an `XBlockFixtureDesc` object. Returns the `xblock_desc` instance to allow chaining. """ @@ -154,14 +94,7 @@ def __str__(self): CourseUpdateDesc = namedtuple("CourseUpdateDesc", ['date', 'content']) -class CourseFixtureError(Exception): - """ - Error occurred while installing a course fixture. - """ - pass - - -class CourseFixture(StudioApiFixture): +class CourseFixture(XBlockContainerFixture): """ Fixture for ensuring that a course exists. @@ -181,6 +114,7 @@ def __init__(self, org, number, run, display_name, start_date=None, end_date=Non These have the same meaning as in the Studio restful API /course end-point. """ + super(CourseFixture, self).__init__() self._course_dict = { 'org': org, 'number': number, @@ -202,7 +136,6 @@ def __init__(self, org, number, run, display_name, start_date=None, end_date=Non self._updates = [] self._handouts = [] - self.children = [] self._assets = [] self._advanced_settings = {} self._course_key = None @@ -213,16 +146,6 @@ def __str__(self): """ return "".format(**self._course_dict) - def add_children(self, *args): - """ - Add children XBlock to the course. - Each item in `args` is an `XBlockFixtureDescriptor` object. - - Returns the course fixture to allow chaining. - """ - self.children.extend(args) - return self - def add_update(self, update): """ Add an update to the course. `update` should be a `CourseUpdateDesc`. @@ -252,7 +175,7 @@ def install(self): """ Create the course and XBlocks within the course. This is NOT an idempotent method; if the course already exists, this will - raise a `CourseFixtureError`. You should use unique course identifiers to avoid + raise a `FixtureError`. You should use unique course identifiers to avoid conflicts between tests. """ self._create_course() @@ -308,18 +231,18 @@ def _create_course(self): err = response.json().get('ErrMsg') except ValueError: - raise CourseFixtureError( + raise FixtureError( "Could not parse response from course request as JSON: '{0}'".format( response.content)) # This will occur if the course identifier is not unique if err is not None: - raise CourseFixtureError("Could not create course {0}. Error message: '{1}'".format(self, err)) + raise FixtureError("Could not create course {0}. Error message: '{1}'".format(self, err)) if response.ok: self._course_key = response.json()['course_key'] else: - raise CourseFixtureError( + raise FixtureError( "Could not create course {0}. Status was {1}".format( self._course_dict, response.status_code)) @@ -333,14 +256,14 @@ def _configure_course(self): response = self.session.get(url, headers=self.headers) if not response.ok: - raise CourseFixtureError( + raise FixtureError( "Could not retrieve course details. Status was {0}".format( response.status_code)) try: details = response.json() except ValueError: - raise CourseFixtureError( + raise FixtureError( "Could not decode course details as JSON: '{0}'".format(details) ) @@ -354,7 +277,7 @@ def _configure_course(self): ) if not response.ok: - raise CourseFixtureError( + raise FixtureError( "Could not update course details to '{0}' with {1}: Status was {2}.".format( self._course_details, url, response.status_code)) @@ -382,7 +305,7 @@ def _install_course_handouts(self): response = self.session.post(url, data=payload, headers=self.headers) if not response.ok: - raise CourseFixtureError( + raise FixtureError( "Could not update course handouts with {0}. Status was {1}".format(url, response.status_code)) def _install_course_updates(self): @@ -399,14 +322,14 @@ def _install_course_updates(self): response = self.session.post(url, headers=self.headers, data=payload) if not response.ok: - raise CourseFixtureError( + raise FixtureError( "Could not add update to course: {0} with {1}. Status was {2}".format( update, url, response.status_code)) def _upload_assets(self): """ Upload assets - :raise CourseFixtureError: + :raise FixtureError: """ url = STUDIO_BASE_URL + self._assets_url @@ -426,7 +349,7 @@ def _upload_assets(self): upload_response = self.session.post(url, files=files, headers=headers) if not upload_response.ok: - raise CourseFixtureError('Could not upload {asset_name} with {url}. Status code: {code}'.format( + raise FixtureError('Could not upload {asset_name} with {url}. Status code: {code}'.format( asset_name=asset_name, url=url, code=upload_response.status_code)) def _add_advanced_settings(self): @@ -442,7 +365,7 @@ def _add_advanced_settings(self): ) if not response.ok: - raise CourseFixtureError( + raise FixtureError( "Could not update advanced details to '{0}' with {1}: Status was {2}.".format( self._advanced_settings, url, response.status_code)) @@ -450,101 +373,7 @@ def _create_xblock_children(self, parent_loc, xblock_descriptions): """ Recursively create XBlock children. """ - for desc in xblock_descriptions: - loc = self.create_xblock(parent_loc, desc) - self._create_xblock_children(loc, desc.children) - + super(CourseFixture, self)._create_xblock_children(parent_loc, xblock_descriptions) self._publish_xblock(parent_loc) - def get_nested_xblocks(self, category=None): - """ - Return a list of nested XBlocks for the course that can be filtered by - category. - """ - xblocks = self._get_nested_xblocks(self) - if category: - xblocks = filter(lambda x: x.category == category, xblocks) - return xblocks - - def _get_nested_xblocks(self, xblock_descriptor): - """ - Return a list of nested XBlocks for the course. - """ - xblocks = list(xblock_descriptor.children) - for child in xblock_descriptor.children: - xblocks.extend(self._get_nested_xblocks(child)) - return xblocks - - def create_xblock(self, parent_loc, xblock_desc): - """ - Create an XBlock with `parent_loc` (the location of the parent block) - and `xblock_desc` (an `XBlockFixtureDesc` instance). - """ - create_payload = { - 'category': xblock_desc.category, - 'display_name': xblock_desc.display_name, - } - - if parent_loc is not None: - create_payload['parent_locator'] = parent_loc - - # Create the new XBlock - response = self.session.post( - STUDIO_BASE_URL + '/xblock/', - data=json.dumps(create_payload), - headers=self.headers, - ) - - if not response.ok: - msg = "Could not create {0}. Status was {1}".format(xblock_desc, response.status_code) - raise CourseFixtureError(msg) - try: - loc = response.json().get('locator') - xblock_desc.locator = loc - except ValueError: - raise CourseFixtureError("Could not decode JSON from '{0}'".format(response.content)) - - # Configure the XBlock - response = self.session.post( - STUDIO_BASE_URL + '/xblock/' + loc, - data=xblock_desc.serialize(), - headers=self.headers, - ) - - if response.ok: - return loc - else: - raise CourseFixtureError( - "Could not update {0}. Status code: {1}".format( - xblock_desc, response.status_code)) - - def _publish_xblock(self, locator): - """ - Publish the xblock at `locator`. - """ - self._update_xblock(locator, {'publish': 'make_public'}) - - def _update_xblock(self, locator, data): - """ - Update the xblock at `locator`. - """ - # Create the new XBlock - response = self.session.put( - "{}/xblock/{}".format(STUDIO_BASE_URL, locator), - data=json.dumps(data), - headers=self.headers, - ) - - if not response.ok: - msg = "Could not update {} with data {}. Status was {}".format(locator, data, response.status_code) - raise CourseFixtureError(msg) - - def _encode_post_dict(self, post_dict): - """ - Encode `post_dict` (a dictionary) as UTF-8 encoded JSON. - """ - return json.dumps({ - k: v.encode('utf-8') if isinstance(v, basestring) else v - for k, v in post_dict.items() - }) diff --git a/common/test/acceptance/fixtures/library.py b/common/test/acceptance/fixtures/library.py new file mode 100644 index 000000000000..f97b8e9fc222 --- /dev/null +++ b/common/test/acceptance/fixtures/library.py @@ -0,0 +1,92 @@ +""" +Fixture to create a Content Library +""" + +from opaque_keys.edx.keys import CourseKey + +from . import STUDIO_BASE_URL +from .base import XBlockContainerFixture, FixtureError + + +class LibraryFixture(XBlockContainerFixture): + """ + Fixture for ensuring that a library exists. + + WARNING: This fixture is NOT idempotent. To avoid conflicts + between tests, you should use unique library identifiers for each fixture. + """ + + def __init__(self, org, number, display_name): + """ + Configure the library fixture to create a library with + """ + super(LibraryFixture, self).__init__() + self.library_info = { + 'org': org, + 'number': number, + 'display_name': display_name + } + + self._library_key = None + super(LibraryFixture, self).__init__() + + def __str__(self): + """ + String representation of the library fixture, useful for debugging. + """ + return "".format(**self.library_info) + + def install(self): + """ + Create the library and XBlocks within the library. + This is NOT an idempotent method; if the library already exists, this will + raise a `FixtureError`. You should use unique library identifiers to avoid + conflicts between tests. + """ + self._create_library() + self._create_xblock_children(self.library_location, self.children) + + return self + + @property + def library_key(self): + """ + Get the LibraryLocator for this library, as a string. + """ + return self._library_key + + @property + def library_location(self): + """ + Return the locator string for the LibraryRoot XBlock that is the root of the library hierarchy. + """ + lib_key = CourseKey.from_string(self._library_key) + return unicode(lib_key.make_usage_key('library', 'library')) + + def _create_library(self): + """ + Create the library described in the fixture. + Will fail if the library already exists. + """ + response = self.session.post( + STUDIO_BASE_URL + '/library/', + data=self._encode_post_dict(self.library_info), + headers=self.headers + ) + + if response.ok: + self._library_key = response.json()['library_key'] + else: + try: + err_msg = response.json().get('ErrMsg') + except ValueError: + err_msg = "Unknown Error" + raise FixtureError( + "Could not create library {}. Status was {}, error was: {}".format(self.library_info, response.status_code, err_msg) + ) + + def create_xblock(self, parent_loc, xblock_desc): + # Disable publishing for library XBlocks: + xblock_desc.publish = "not-applicable" + + return super(LibraryFixture, self).create_xblock(parent_loc, xblock_desc) diff --git a/common/test/acceptance/pages/studio/container.py b/common/test/acceptance/pages/studio/container.py index e65d55146f0c..20cf140ed117 100644 --- a/common/test/acceptance/pages/studio/container.py +++ b/common/test/acceptance/pages/studio/container.py @@ -6,7 +6,7 @@ from bok_choy.promise import Promise, EmptyPromise from . import BASE_URL -from utils import click_css, confirm_prompt +from .utils import click_css, confirm_prompt, type_in_codemirror class ContainerPage(PageObject): @@ -362,6 +362,12 @@ def open_basic_tab(self): """ self._click_button('basic_tab') + def set_codemirror_text(self, text, index=0): + """ + Set the text of a CodeMirror editor that is part of this xblock's settings. + """ + type_in_codemirror(self, index, text, find_prefix='$("{}").find'.format(self.editor_selector)) + def save_settings(self): """ Click on settings Save button. diff --git a/common/test/acceptance/pages/studio/index.py b/common/test/acceptance/pages/studio/index.py index af163eca6852..aed9a5faae23 100644 --- a/common/test/acceptance/pages/studio/index.py +++ b/common/test/acceptance/pages/studio/index.py @@ -28,6 +28,13 @@ def course_runs(self): def has_processing_courses(self): return self.q(css='.courses-processing').present + @property + def page_subheader(self): + """ + Get the text of the introductory copy seen below the Welcome header. ("Here are all of...") + """ + return self.q(css='.content-primary .introduction .copy p').first.text[0] + def create_rerun(self, display_name): """ Clicks the create rerun link of the course specified by display_name. @@ -40,3 +47,68 @@ def click_course_run(self, run): Clicks on the course with run given by run. """ self.q(css='.course-run .value').filter(lambda el: el.text == run)[0].click() + + def has_new_library_button(self): + """ + (bool) is the "New Library" button present? + """ + return self.q(css='.new-library-button').present + + def click_new_library(self): + """ + Click on the "New Library" button + """ + self.q(css='.new-library-button').click() + + def is_new_library_form_visible(self): + """ + Is the new library form visisble? + """ + return self.q(css='.wrapper-create-library').visible + + def fill_new_library_form(self, display_name, org, number): + """ + Fill out the form to create a new library. + Must have called click_new_library() first. + """ + field = lambda fn: self.q(css='.wrapper-create-library #new-library-{}'.format(fn)) + field('name').fill(display_name) + field('org').fill(org) + field('number').fill(number) + + def is_new_library_form_valid(self): + """ + IS the new library form ready to submit? + """ + return ( + self.q(css='.wrapper-create-library .new-library-save:not(.is-disabled)').present and + not self.q(css='.wrapper-create-library .wrap-error.is-shown').present + ) + + def submit_new_library_form(self): + """ + Submit the new library form. + """ + self.q(css='.wrapper-create-library .new-library-save').click() + + def list_libraries(self): + """ + List all the libraries found on the page's list of libraries. + """ + self.q(css='#course-index-tabs .libraries-tab a').click() # Workaround Selenium/Firefox bug: `.text` property is broken on invisible elements + div2info = lambda element: { + 'name': element.find_element_by_css_selector('.course-title').text, + 'org': element.find_element_by_css_selector('.course-org .value').text, + 'number': element.find_element_by_css_selector('.course-num .value').text, + 'url': element.find_element_by_css_selector('a.library-link').get_attribute('href'), + } + return self.q(css='.libraries li.course-item').map(div2info).results + + def has_library(self, **kwargs): + """ + Does the page's list of libraries include a library matching kwargs? + """ + for lib in self.list_libraries(): + if all([lib[key] == kwargs[key] for key in kwargs]): + return True + return False diff --git a/common/test/acceptance/pages/studio/library.py b/common/test/acceptance/pages/studio/library.py new file mode 100644 index 000000000000..e87c556da968 --- /dev/null +++ b/common/test/acceptance/pages/studio/library.py @@ -0,0 +1,97 @@ +""" +Library edit page in Studio +""" + +from bok_choy.page_object import PageObject +from .container import XBlockWrapper +from ...tests.helpers import disable_animations +from .utils import confirm_prompt, wait_for_notification +from . import BASE_URL + + +class LibraryPage(PageObject): + """ + Library page in Studio + """ + + def __init__(self, browser, locator): + super(LibraryPage, self).__init__(browser) + self.locator = locator + + @property + def url(self): + """ + URL to the library edit page for the given library. + """ + return "{}/library/{}".format(BASE_URL, unicode(self.locator)) + + def is_browser_on_page(self): + """ + Returns True iff the browser has loaded the library edit page. + """ + return self.q(css='body.view-library').present + + def get_header_title(self): + """ + The text of the main heading (H1) visible on the page. + """ + return self.q(css='h1.page-header-title').text + + def wait_until_ready(self): + """ + When the page first loads, there is a loading indicator and most + functionality is not yet available. This waits for that loading to + finish. + + Always call this before using the page. It also disables animations + for improved test reliability. + """ + self.wait_for_ajax() + self.wait_for_element_invisibility('.ui-loading', 'Wait for the page to complete its initial loading of XBlocks via AJAX') + disable_animations(self) + + @property + def xblocks(self): + """ + Return a list of xblocks loaded on the container page. + """ + return self._get_xblocks() + + def click_duplicate_button(self, xblock_id): + """ + Click on the duplicate button for the given XBlock + """ + self._action_btn_for_xblock_id(xblock_id, "duplicate").click() + wait_for_notification(self) + self.wait_for_ajax() + + def click_delete_button(self, xblock_id, confirm=True): + """ + Click on the delete button for the given XBlock + """ + self._action_btn_for_xblock_id(xblock_id, "delete").click() + if confirm: + confirm_prompt(self) # this will also wait_for_notification() + self.wait_for_ajax() + + def _get_xblocks(self): + """ + Create an XBlockWrapper for each XBlock div found on the page. + """ + prefix = '.wrapper-xblock.level-page ' + return self.q(css=prefix + XBlockWrapper.BODY_SELECTOR).map(lambda el: XBlockWrapper(self.browser, el.get_attribute('data-locator'))).results + + def _div_for_xblock_id(self, xblock_id): + """ + Given an XBlock's usage locator as a string, return the WebElement for + that block's wrapper div. + """ + return self.q(css='.wrapper-xblock.level-page .studio-xblock-wrapper').filter(lambda el: el.get_attribute('data-locator') == xblock_id) + + def _action_btn_for_xblock_id(self, xblock_id, action): + """ + Given an XBlock's usage locator as a string, return one of its action + buttons. + action is 'edit', 'duplicate', or 'delete' + """ + return self._div_for_xblock_id(xblock_id)[0].find_element_by_css_selector('.header-actions .{action}-button.action-button'.format(action=action)) diff --git a/common/test/acceptance/pages/studio/utils.py b/common/test/acceptance/pages/studio/utils.py index a94f50ba6fa1..dd8ec091a347 100644 --- a/common/test/acceptance/pages/studio/utils.py +++ b/common/test/acceptance/pages/studio/utils.py @@ -103,6 +103,30 @@ def add_advanced_component(page, menu_index, name): click_css(page, component_css, 0) +def add_component(page, item_type, specific_type): + """ + Click one of the "Add New Component" buttons. + + item_type should be "advanced", "html", "problem", or "video" + + specific_type is required for some types and should be something like + "Blank Common Problem". + """ + btn = page.q(css='.add-xblock-component .add-xblock-component-button[data-type={}]'.format(item_type)) + multiple_templates = btn.filter(lambda el: 'multiple-templates' in el.get_attribute('class')).present + btn.click() + if multiple_templates: + sub_template_menu_div_selector = '.new-component-{}'.format(item_type) + page.wait_for_element_visibility(sub_template_menu_div_selector, 'Wait for the templates sub-menu to appear') + page.wait_for_element_invisibility('.add-xblock-component .new-component', 'Wait for the add component menu to disappear') + + all_options = page.q(css='.new-component-{} ul.new-component-template li a span'.format(item_type)) + chosen_option = all_options.filter(lambda el: el.text == specific_type).first + chosen_option.click() + wait_for_notification(page) + page.wait_for_ajax() + + @js_defined('window.jQuery') def type_in_codemirror(page, index, text, find_prefix="$"): script = """ diff --git a/common/test/acceptance/tests/studio/base_studio_test.py b/common/test/acceptance/tests/studio/base_studio_test.py index fa07533fba86..ec94f7f058c5 100644 --- a/common/test/acceptance/tests/studio/base_studio_test.py +++ b/common/test/acceptance/tests/studio/base_studio_test.py @@ -1,5 +1,10 @@ +""" +Base classes used by studio tests. +""" +from bok_choy.web_app_test import WebAppTest from ...pages.studio.auto_auth import AutoAuthPage from ...fixtures.course import CourseFixture +from ...fixtures.library import LibraryFixture from ..helpers import UniqueCourseTest from ...pages.studio.overview import CourseOutlinePage from ...pages.studio.utils import verify_ordering @@ -98,3 +103,46 @@ def do_action_and_verify(self, action, expected_ordering): # Reload the page to see that the change was persisted. container = self.go_to_nested_container_page() verify_ordering(self, container, expected_ordering) + + +class StudioLibraryTest(WebAppTest): + """ + Base class for all Studio library tests. + """ + + def setUp(self, is_staff=False): # pylint: disable=arguments-differ + """ + Install a library with no content using a fixture. + """ + super(StudioLibraryTest, self).setUp() + fixture = LibraryFixture( + 'test_org', + self.unique_id, + 'Test Library {}'.format(self.unique_id), + ) + self.populate_library_fixture(fixture) + fixture.install() + self.library_info = fixture.library_info + self.library_key = fixture.library_key + self.user = fixture.user + self.log_in(self.user, is_staff) + + def populate_library_fixture(self, library_fixture): + """ + Populate the children of the test course fixture. + """ + pass + + def log_in(self, user, is_staff=False): + """ + Log in as the user that created the library. + By default the user will not have staff access unless is_staff is passed as True. + """ + auth_page = AutoAuthPage( + self.browser, + staff=is_staff, + username=user.get('username'), + email=user.get('email'), + password=user.get('password') + ) + auth_page.visit() diff --git a/common/test/acceptance/tests/studio/test_studio_home.py b/common/test/acceptance/tests/studio/test_studio_home.py new file mode 100644 index 000000000000..9dc9b0249716 --- /dev/null +++ b/common/test/acceptance/tests/studio/test_studio_home.py @@ -0,0 +1,67 @@ +""" +Acceptance tests for Home Page (My Courses / My Libraries). +""" +from bok_choy.web_app_test import WebAppTest +from opaque_keys.edx.locator import LibraryLocator + +from ...pages.studio.auto_auth import AutoAuthPage +from ...pages.studio.library import LibraryPage +from ...pages.studio.index import DashboardPage + + +class CreateLibraryTest(WebAppTest): + """ + Test that we can create a new content library on the studio home page. + """ + + def setUp(self): + """ + Load the helper for the home page (dashboard page) + """ + super(CreateLibraryTest, self).setUp() + + self.auth_page = AutoAuthPage(self.browser, staff=True) + self.dashboard_page = DashboardPage(self.browser) + + def test_subheader(self): + """ + From the home page: + Verify that subheader is correct + """ + self.auth_page.visit() + self.dashboard_page.visit() + + self.assertIn("courses and libraries", self.dashboard_page.page_subheader) + + def test_create_library(self): + """ + From the home page: + Click "New Library" + Fill out the form + Submit the form + We should be redirected to the edit view for the library + Return to the home page + The newly created library should now appear in the list of libraries + """ + name = "New Library Name" + org = "TestOrgX" + number = "TESTLIB" + + self.auth_page.visit() + self.dashboard_page.visit() + self.assertFalse(self.dashboard_page.has_library(name=name, org=org, number=number)) + self.assertTrue(self.dashboard_page.has_new_library_button()) + + self.dashboard_page.click_new_library() + self.assertTrue(self.dashboard_page.is_new_library_form_visible()) + self.dashboard_page.fill_new_library_form(name, org, number) + self.assertTrue(self.dashboard_page.is_new_library_form_valid()) + self.dashboard_page.submit_new_library_form() + + # The next page is the library edit view; make sure it loads: + lib_page = LibraryPage(self.browser, LibraryLocator(org, number)) + lib_page.wait_for_page() + + # Then go back to the home page and make sure the new library is listed there: + self.dashboard_page.visit() + self.assertTrue(self.dashboard_page.has_library(name=name, org=org, number=number)) diff --git a/common/test/acceptance/tests/studio/test_studio_library.py b/common/test/acceptance/tests/studio/test_studio_library.py new file mode 100644 index 000000000000..d5ad890e376c --- /dev/null +++ b/common/test/acceptance/tests/studio/test_studio_library.py @@ -0,0 +1,104 @@ +""" +Acceptance tests for Content Libraries in Studio +""" + +from .base_studio_test import StudioLibraryTest +from ...pages.studio.utils import add_component +from ...pages.studio.library import LibraryPage + + +class LibraryEditPageTest(StudioLibraryTest): + """ + Test the functionality of the library edit page. + """ + def setUp(self): # pylint: disable=arguments-differ + """ + Ensure a library exists and navigate to the library edit page. + """ + super(LibraryEditPageTest, self).setUp(is_staff=True) + self.lib_page = LibraryPage(self.browser, self.library_key) + self.lib_page.visit() + self.lib_page.wait_until_ready() + + def test_page_header(self): + """ + Scenario: Ensure that the library's name is displayed in the header and title. + Given I have a library in Studio + And I navigate to Library Page in Studio + Then I can see library name in page header title + And I can see library name in browser page title + """ + self.assertIn(self.library_info['display_name'], self.lib_page.get_header_title()) + self.assertIn(self.library_info['display_name'], self.browser.title) + + def test_add_duplicate_delete_actions(self): + """ + Scenario: Ensure that we can add an HTML block, duplicate it, then delete the original. + Given I have a library in Studio with no XBlocks + And I navigate to Library Page in Studio + Then there are no XBlocks displayed + When I add Text XBlock + Then one XBlock is displayed + When I duplicate first XBlock + Then two XBlocks are displayed + And those XBlocks locators' are different + When I delete first XBlock + Then one XBlock is displayed + And displayed XBlock are second one + """ + self.assertEqual(len(self.lib_page.xblocks), 0) + + # Create a new block: + add_component(self.lib_page, "html", "Text") + self.assertEqual(len(self.lib_page.xblocks), 1) + first_block_id = self.lib_page.xblocks[0].locator + + # Duplicate the block: + self.lib_page.click_duplicate_button(first_block_id) + self.assertEqual(len(self.lib_page.xblocks), 2) + second_block_id = self.lib_page.xblocks[1].locator + self.assertNotEqual(first_block_id, second_block_id) + + # Delete the first block: + self.lib_page.click_delete_button(first_block_id, confirm=True) + self.assertEqual(len(self.lib_page.xblocks), 1) + self.assertEqual(self.lib_page.xblocks[0].locator, second_block_id) + + def test_add_edit_xblock(self): + """ + Scenario: Ensure that we can add an XBlock, edit it, then see the resulting changes. + Given I have a library in Studio with no XBlocks + And I navigate to Library Page in Studio + Then there are no XBlocks displayed + When I add Multiple Choice XBlock + Then one XBlock is displayed + When I edit first XBlock + And I go to basic tab + And set it's text to a fairly trivial question about Battlestar Galactica + And save XBlock + Then one XBlock is displayed + And first XBlock student content contains at least part of text I set + """ + self.assertEqual(len(self.lib_page.xblocks), 0) + # Create a new problem block: + add_component(self.lib_page, "problem", "Multiple Choice") + self.assertEqual(len(self.lib_page.xblocks), 1) + problem_block = self.lib_page.xblocks[0] + # Edit it: + problem_block.edit() + problem_block.open_basic_tab() + problem_block.set_codemirror_text( + """ + >>Who is "Starbuck"?<< + (x) Kara Thrace + ( ) William Adama + ( ) Laura Roslin + ( ) Lee Adama + ( ) Gaius Baltar + """ + ) + problem_block.save_settings() + # Check that the save worked: + self.assertEqual(len(self.lib_page.xblocks), 1) + problem_block = self.lib_page.xblocks[0] + self.assertIn("Laura Roslin", problem_block.student_content) From 6963e8eb3d261a5b6d94897002e9abeb9cf17b54 Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Tue, 2 Dec 2014 17:58:34 +0000 Subject: [PATCH 02/23] Removed the ability to add Discussion and advanced components to Content Libraries. --- .../contentstore/views/component.py | 20 ++++++++--- cms/djangoapps/contentstore/views/item.py | 7 ++++ cms/djangoapps/contentstore/views/library.py | 2 +- .../contentstore/views/tests/test_item.py | 35 +++++++++++++++++++ .../contentstore/views/tests/test_library.py | 14 ++++++++ .../tests/studio/test_studio_library.py | 7 +++- 6 files changed, 78 insertions(+), 7 deletions(-) diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 9768542ea8b8..90f1dde26723 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -217,9 +217,9 @@ def container_handler(request, usage_key_string): return HttpResponseBadRequest("Only supports HTML requests") -def get_component_templates(course): +def get_component_templates(courselike, library=False): """ - Returns the applicable component templates that can be used by the specified course. + Returns the applicable component templates that can be used by the specified course or library. """ def create_template_dict(name, cat, boilerplate_name=None, is_common=False): """ @@ -250,7 +250,13 @@ def create_template_dict(name, cat, boilerplate_name=None, is_common=False): categories = set() # The component_templates array is in the order of "advanced" (if present), followed # by the components in the order listed in COMPONENT_TYPES. - for category in COMPONENT_TYPES: + component_types = COMPONENT_TYPES[:] + + # Libraries do not support discussions + if library: + component_types = [component for component in component_types if component != 'discussion'] + + for category in component_types: templates_for_category = [] component_class = _load_mixed_class(category) # add the default template with localized display name @@ -264,7 +270,7 @@ def create_template_dict(name, cat, boilerplate_name=None, is_common=False): if hasattr(component_class, 'templates'): for template in component_class.templates(): filter_templates = getattr(component_class, 'filter_templates', None) - if not filter_templates or filter_templates(template, course): + if not filter_templates or filter_templates(template, courselike): templates_for_category.append( create_template_dict( _(template['metadata'].get('display_name')), @@ -289,11 +295,15 @@ def create_template_dict(name, cat, boilerplate_name=None, is_common=False): "display_name": component_display_names[category] }) + # Libraries do not support advanced components at this time. + if library: + return component_templates + # Check if there are any advanced modules specified in the course policy. # These modules should be specified as a list of strings, where the strings # are the names of the modules in ADVANCED_COMPONENT_TYPES that should be # enabled for the course. - course_advanced_keys = course.advanced_modules + course_advanced_keys = courselike.advanced_modules advanced_component_templates = {"type": "advanced", "templates": [], "display_name": _("Advanced")} advanced_component_types = _advanced_component_types() # Set component types according to course policy file diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 815b67795157..d3123aa0d04d 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -461,6 +461,13 @@ def _create_item(request): if not has_course_author_access(request.user, usage_key.course_key): raise PermissionDenied() + if isinstance(usage_key, LibraryUsageLocator): + # Only these categories are supported at this time. + if category not in ['html', 'problem', 'video']: + return HttpResponseBadRequest( + "Category '%s' not supported for Libraries" % category, content_type='text/plain' + ) + store = modulestore() with store.bulk_operations(usage_key.course_key): parent = store.get_item(usage_key) diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index 15e54a37ce35..1fdc8381a8f4 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -175,7 +175,7 @@ def library_blocks_view(library, response_format): }) xblock_info = create_xblock_info(library, include_ancestor_info=False, graders=[]) - component_templates = get_component_templates(library) + component_templates = get_component_templates(library, library=True) return render_to_response('library.html', { 'context_library': library, diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 3f4fa86cba0c..d6d913a59489 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -1469,6 +1469,41 @@ def validate_component_xblock_info(self, xblock_info, original_block): self.assertIsNone(xblock_info.get('graders', None)) +class TestLibraryXBlockCreation(ItemTest): + """ + Tests the adding of XBlocks to Library + """ + def test_add_xblock(self): + """ + Verify we can add an XBlock to a Library. + """ + lib = LibraryFactory.create() + self.create_xblock(parent_usage_key=lib.location, display_name='Test', category="html") + lib = self.store.get_library(lib.location.library_key) + self.assertTrue(lib.children) + xblock_locator = lib.children[0] + self.assertEqual(self.store.get_item(xblock_locator).display_name, 'Test') + + def test_no_add_discussion(self): + """ + Verify we cannot add a discussion module to a Library. + """ + lib = LibraryFactory.create() + response = self.create_xblock(parent_usage_key=lib.location, display_name='Test', category='discussion') + self.assertEqual(response.status_code, 400) + lib = self.store.get_library(lib.location.library_key) + self.assertFalse(lib.children) + + def test_no_add_advanced(self): + lib = LibraryFactory.create() + lib.advanced_modules = ['lti'] + lib.save() + response = self.create_xblock(parent_usage_key=lib.location, display_name='Test', category='lti') + self.assertEqual(response.status_code, 400) + lib = self.store.get_library(lib.location.library_key) + self.assertFalse(lib.children) + + class TestXBlockPublishingInfo(ItemTest): """ Unit tests for XBlock's outline handling. diff --git a/cms/djangoapps/contentstore/views/tests/test_library.py b/cms/djangoapps/contentstore/views/tests/test_library.py index 8cae9710873f..9ab5bc06ccf0 100644 --- a/cms/djangoapps/contentstore/views/tests/test_library.py +++ b/cms/djangoapps/contentstore/views/tests/test_library.py @@ -4,6 +4,7 @@ More important high-level tests are in contentstore/tests/test_libraries.py """ from contentstore.tests.utils import AjaxEnabledTestClient, parse_json +from contentstore.views.component import get_component_templates from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import LibraryFactory from mock import patch @@ -183,3 +184,16 @@ def test_no_access(self): lib = LibraryFactory.create() response = self.client.get(make_url_for_lib(lib.location.library_key)) self.assertEqual(response.status_code, 403) + + def test_get_component_templates(self): + """ + Verify that templates for adding discussion and advanced components to + content libraries are not provided. + """ + lib = LibraryFactory.create() + lib.advanced_modules = ['lti'] + lib.save() + templates = [template['type'] for template in get_component_templates(lib, library=True)] + self.assertIn('problem', templates) + self.assertNotIn('discussion', templates) + self.assertNotIn('advanced', templates) diff --git a/common/test/acceptance/tests/studio/test_studio_library.py b/common/test/acceptance/tests/studio/test_studio_library.py index d5ad890e376c..b505ac140dbc 100644 --- a/common/test/acceptance/tests/studio/test_studio_library.py +++ b/common/test/acceptance/tests/studio/test_studio_library.py @@ -1,7 +1,6 @@ """ Acceptance tests for Content Libraries in Studio """ - from .base_studio_test import StudioLibraryTest from ...pages.studio.utils import add_component from ...pages.studio.library import LibraryPage @@ -102,3 +101,9 @@ def test_add_edit_xblock(self): self.assertEqual(len(self.lib_page.xblocks), 1) problem_block = self.lib_page.xblocks[0] self.assertIn("Laura Roslin", problem_block.student_content) + + def test_no_discussion_button(self): + """ + Ensure the UI is not loaded for adding discussions. + """ + self.assertFalse(self.browser.find_elements_by_css_selector('span.large-discussion-icon')) From 459b65143e2f3d06d5b9c9cdb41a13af003c0acd Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Mon, 3 Nov 2014 20:20:29 +0700 Subject: [PATCH 03/23] Paging for LibraryView added with JS tests. --- cms/djangoapps/contentstore/views/item.py | 18 +- cms/static/coffee/spec/main.coffee | 1 + cms/static/js/factories/container.js | 22 +- cms/static/js/factories/library.js | 22 +- .../js/spec/views/library_container_spec.js | 489 ++++++++++ .../js/spec/views/pages/container_spec.js | 893 +++++++++--------- cms/static/js/views/container.js | 4 + cms/static/js/views/library_container.js | 164 ++++ cms/static/js/views/pages/container.js | 52 +- cms/static/js/views/paging_footer.js | 6 + cms/static/sass/elements/_pagination.scss | 119 +++ cms/static/sass/elements/_xblocks.scss | 31 + cms/static/sass/style-app-extend1-rtl.scss | 1 + cms/static/sass/style-app-extend1.scss | 1 + cms/static/sass/views/_assets.scss | 122 +-- cms/templates/container.html | 5 +- ...ontainer-paged-after-add-xblock.underscore | 283 ++++++ .../mock-container-paged-xblock.underscore | 257 +++++ cms/templates/library.html | 8 +- .../xmodule/xmodule/library_root_xblock.py | 54 +- .../xmodule/video_module/video_handlers.py | 1 - .../studio_render_paged_children_view.html | 23 + 22 files changed, 1963 insertions(+), 613 deletions(-) create mode 100644 cms/static/js/spec/views/library_container_spec.js create mode 100644 cms/static/js/views/library_container.js create mode 100644 cms/static/sass/elements/_pagination.scss create mode 100644 cms/templates/js/mock/mock-container-paged-after-add-xblock.underscore create mode 100644 cms/templates/js/mock/mock-container-paged-xblock.underscore create mode 100644 lms/templates/studio_render_paged_children_view.html diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index d3123aa0d04d..5d05a27e1781 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -238,12 +238,28 @@ def xblock_view_handler(request, usage_key_string, view_name): if view_name == 'reorderable_container_child_preview': reorderable_items.add(xblock.location) + paging = None + try: + if request.REQUEST.get('enable_paging', 'false') == 'true': + paging = { + 'page_number': int(request.REQUEST.get('page_number', 0)), + 'page_size': int(request.REQUEST.get('page_size', 0)), + } + except ValueError: + log.exception( + "Couldn't parse paging parameters: enable_paging: %s, page_number: %s, page_size: %s", + request.REQUEST.get('enable_paging', 'false'), + request.REQUEST.get('page_number', 0), + request.REQUEST.get('page_size', 0) + ) + # 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), 'root_xblock': xblock if (view_name == 'container_preview') else None, - 'reorderable_items': reorderable_items + 'reorderable_items': reorderable_items, + 'paging': paging } fragment = get_preview_fragment(request, xblock, context) diff --git a/cms/static/coffee/spec/main.coffee b/cms/static/coffee/spec/main.coffee index b9b3546dbff3..e2ba18a82198 100644 --- a/cms/static/coffee/spec/main.coffee +++ b/cms/static/coffee/spec/main.coffee @@ -232,6 +232,7 @@ define([ "js/spec/views/assets_spec", "js/spec/views/baseview_spec", "js/spec/views/container_spec", + "js/spec/views/library_container_spec", "js/spec/views/group_configuration_spec", "js/spec/views/paging_spec", "js/spec/views/unit_outline_spec", diff --git a/cms/static/js/factories/container.js b/cms/static/js/factories/container.js index 93cdeb8fd991..ea48bb2a989f 100644 --- a/cms/static/js/factories/container.js +++ b/cms/static/js/factories/container.js @@ -1,22 +1,20 @@ define([ - 'jquery', 'js/models/xblock_info', 'js/views/pages/container', + 'jquery', 'underscore', 'js/models/xblock_info', 'js/views/pages/container', 'js/collections/component_template', 'xmodule', 'coffee/src/main', 'xblock/cms.runtime.v1' ], -function($, XBlockInfo, ContainerPage, ComponentTemplates, xmoduleLoader) { +function($, _, XBlockInfo, ContainerPage, ComponentTemplates, xmoduleLoader) { 'use strict'; - return function (componentTemplates, XBlockInfoJson, action, isUnitPage) { - var templates = new ComponentTemplates(componentTemplates, {parse: true}), - mainXBlockInfo = new XBlockInfo(XBlockInfoJson, {parse: true}); - - xmoduleLoader.done(function () { - var view = new ContainerPage({ + return function (componentTemplates, XBlockInfoJson, action, options) { + var main_options = { el: $('#content'), - model: mainXBlockInfo, + model: new XBlockInfo(XBlockInfoJson, {parse: true}), action: action, - templates: templates, - isUnitPage: isUnitPage - }); + templates: new ComponentTemplates(componentTemplates, {parse: true}) + }; + + xmoduleLoader.done(function () { + var view = new ContainerPage(_.extend(main_options, options)); view.render(); }); }; diff --git a/cms/static/js/factories/library.js b/cms/static/js/factories/library.js index 2729a3cf279d..e7834f60ef3c 100644 --- a/cms/static/js/factories/library.js +++ b/cms/static/js/factories/library.js @@ -1,22 +1,20 @@ define([ - 'jquery', 'js/models/xblock_info', 'js/views/pages/container', + 'jquery', 'underscore', 'js/models/xblock_info', 'js/views/pages/container', 'js/collections/component_template', 'xmodule', 'coffee/src/main', 'xblock/cms.runtime.v1' ], -function($, XBlockInfo, ContainerPage, ComponentTemplates, xmoduleLoader) { +function($, _, XBlockInfo, ContainerPage, ComponentTemplates, xmoduleLoader) { 'use strict'; - return function (componentTemplates, XBlockInfoJson) { - var templates = new ComponentTemplates(componentTemplates, {parse: true}), - mainXBlockInfo = new XBlockInfo(XBlockInfoJson, {parse: true}); + return function (componentTemplates, XBlockInfoJson, options) { + var main_options = { + el: $('#content'), + model: new XBlockInfo(XBlockInfoJson, {parse: true}), + templates: new ComponentTemplates(componentTemplates, {parse: true}), + action: 'view' + }; xmoduleLoader.done(function () { - var view = new ContainerPage({ - el: $('#content'), - model: mainXBlockInfo, - action: "view", - templates: templates, - isUnitPage: false - }); + var view = new ContainerPage(_.extend(main_options, options)); view.render(); }); }; diff --git a/cms/static/js/spec/views/library_container_spec.js b/cms/static/js/spec/views/library_container_spec.js new file mode 100644 index 000000000000..2d39cdc35819 --- /dev/null +++ b/cms/static/js/spec/views/library_container_spec.js @@ -0,0 +1,489 @@ +define([ "jquery", "underscore", "js/common_helpers/ajax_helpers", "URI", "js/models/xblock_info", + "js/views/library_container", "js/views/paging_header", "js/views/paging_footer"], + function ($, _, AjaxHelpers, URI, XBlockInfo, PagedContainer, PagingContainer, PagingFooter) { + + var htmlResponseTpl = _.template('' + + '
' + ); + + function getResponseHtml(options){ + return '
' + + '
' + + htmlResponseTpl(options) + + '
' + + '
' + } + + var PAGE_SIZE = 3; + + var mockFirstPage = { + resources: [], + html: getResponseHtml({ + start: 0, + displayed: PAGE_SIZE, + total: PAGE_SIZE + 1 + }) + }; + + var mockSecondPage = { + resources: [], + html: getResponseHtml({ + start: PAGE_SIZE, + displayed: 1, + total: PAGE_SIZE + 1 + }) + }; + + var mockEmptyPage = { + resources: [], + html: getResponseHtml({ + start: 0, + displayed: 0, + total: 0 + }) + }; + + var respondWithMockPage = function(requests) { + var requestIndex = requests.length - 1; + var request = requests[requestIndex]; + var url = new URI(request.url); + var queryParameters = url.query(true); // Returns an object with each query parameter stored as a value + var page = queryParameters.page_number; + var response = page === "0" ? mockFirstPage : mockSecondPage; + AjaxHelpers.respondWithJson(requests, response, requestIndex); + }; + + var MockPagingView = PagedContainer.extend({ + view: 'container_preview', + el: $("
"), + model: new XBlockInfo({}, {parse: true}) + }); + + describe("Paging Container", function() { + var pagingContainer; + + beforeEach(function () { + var feedbackTpl = readFixtures('system-feedback.underscore'); + setFixtures($(" + + +
+ +
+
+
+
+
+ + + Expand or Collapse + + Group A +
+
+
    +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ + + Expand or Collapse + + Group B +
+
+
    +
  • + +
    • +
+
+
+
+ +
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
+ diff --git a/cms/templates/js/mock/mock-container-paged-xblock.underscore b/cms/templates/js/mock/mock-container-paged-xblock.underscore new file mode 100644 index 000000000000..2314bb892577 --- /dev/null +++ b/cms/templates/js/mock/mock-container-paged-xblock.underscore @@ -0,0 +1,257 @@ +
+
+
+ Test Container +
+
+
    +
+
+
+
+
+
+ + + + +
+ +
+
+
+
+
+ + + Expand or Collapse + + Group A +
+
+
    +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ + + Expand or Collapse + + Group B +
+
+
    +
  • + +
    • +
+
+
+
+ +
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/cms/templates/library.html b/cms/templates/library.html index 70bd836baddd..dc9baa5736c3 100644 --- a/cms/templates/library.html +++ b/cms/templates/library.html @@ -22,8 +22,12 @@ <%block name="requirejs"> require(["js/factories/library"], function(LibraryFactory) { LibraryFactory( - ${component_templates | n}, - ${json.dumps(xblock_info) | n} + ${component_templates | n}, ${json.dumps(xblock_info) | n}, + { + isUnitPage: false, + enable_paging: true, + page_size: 10 + } ); }); diff --git a/common/lib/xmodule/xmodule/library_root_xblock.py b/common/lib/xmodule/xmodule/library_root_xblock.py index dc00aaa97fa9..497a145b79bd 100644 --- a/common/lib/xmodule/xmodule/library_root_xblock.py +++ b/common/lib/xmodule/xmodule/library_root_xblock.py @@ -3,10 +3,10 @@ """ import logging -from .studio_editable import StudioEditableModule from xblock.core import XBlock from xblock.fields import Scope, String, List from xblock.fragment import Fragment +from xmodule.studio_editable import StudioEditableModule log = logging.getLogger(__name__) @@ -42,29 +42,55 @@ def __str__(self): def author_view(self, context): """ - Renders the Studio preview view, which supports drag and drop. + Renders the Studio preview view. """ fragment = Fragment() + self.render_children(context, fragment, can_reorder=False, can_add=True) + return fragment + + def render_children(self, context, fragment, can_reorder=False, can_add=False): # pylint: disable=unused-argument + """ + Renders the children of the module with HTML appropriate for Studio. If can_reorder is True, + then the children will be rendered to support drag and drop. + """ contents = [] - for child_key in self.children: # pylint: disable=E1101 - context['reorderable_items'].add(child_key) + paging = context.get('paging', None) + + children_count = len(self.children) # pylint: disable=no-member + item_start, item_end = 0, children_count + + # TODO sort children + if paging: + page_number = paging.get('page_number', 0) + raw_page_size = paging.get('page_size', None) + page_size = raw_page_size if raw_page_size is not None else children_count + item_start, item_end = page_size * page_number, page_size * (page_number + 1) + + children_to_show = self.children[item_start:item_end] # pylint: disable=no-member + + for child_key in children_to_show: # pylint: disable=E1101 child = self.runtime.get_block(child_key) - rendered_child = self.runtime.render_child(child, StudioEditableModule.get_preview_view_name(child), context) + child_view_name = StudioEditableModule.get_preview_view_name(child) + rendered_child = self.runtime.render_child(child, child_view_name, context) fragment.add_frag_resources(rendered_child) contents.append({ - 'id': unicode(child_key), - 'content': rendered_child.content, + 'id': child.location.to_deprecated_string(), + 'content': rendered_child.content }) - fragment.add_content(self.runtime.render_template("studio_render_children_view.html", { - 'items': contents, - 'xblock_context': context, - 'can_add': True, - 'can_reorder': True, - })) - return fragment + fragment.add_content( + self.runtime.render_template("studio_render_paged_children_view.html", { + 'items': contents, + 'xblock_context': context, + 'can_add': can_add, + 'can_reorder': False, + 'first_displayed': item_start, + 'total_children': children_count, + 'displayed_children': len(children_to_show) + }) + ) @property def display_org_with_default(self): diff --git a/common/lib/xmodule/xmodule/video_module/video_handlers.py b/common/lib/xmodule/xmodule/video_module/video_handlers.py index 9e9db860ca54..1ba427c35785 100644 --- a/common/lib/xmodule/xmodule/video_module/video_handlers.py +++ b/common/lib/xmodule/xmodule/video_module/video_handlers.py @@ -155,7 +155,6 @@ def get_static_transcript(self, request): if transcript_name: # Get the asset path for course - asset_path = None course = self.descriptor.runtime.modulestore.get_course(self.course_id) if course.static_asset_path: asset_path = course.static_asset_path diff --git a/lms/templates/studio_render_paged_children_view.html b/lms/templates/studio_render_paged_children_view.html new file mode 100644 index 000000000000..fe5b5403e1ab --- /dev/null +++ b/lms/templates/studio_render_paged_children_view.html @@ -0,0 +1,23 @@ +<%! from django.utils.translation import ugettext as _ %> + +<%namespace name='static' file='static_content.html'/> + +% for template_name in ["paging-header", "paging-footer"]: + +% endfor + +
+ +
+ +% for item in items: + ${item['content']} +% endfor + +% if can_add: +
+% endif + +
From 523e8d98a92c9e7d8a4adc49d1c25ed55151242f Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Thu, 4 Dec 2014 21:25:52 +0000 Subject: [PATCH 04/23] Added tests for Library pagination. --- .../test/acceptance/pages/studio/library.py | 53 ++++++ .../tests/studio/test_studio_library.py | 161 ++++++++++++++++++ 2 files changed, 214 insertions(+) diff --git a/common/test/acceptance/pages/studio/library.py b/common/test/acceptance/pages/studio/library.py index e87c556da968..5572b1a91e43 100644 --- a/common/test/acceptance/pages/studio/library.py +++ b/common/test/acceptance/pages/studio/library.py @@ -3,6 +3,7 @@ """ from bok_choy.page_object import PageObject +from selenium.webdriver.common.keys import Keys from .container import XBlockWrapper from ...tests.helpers import disable_animations from .utils import confirm_prompt, wait_for_notification @@ -74,6 +75,58 @@ def click_delete_button(self, xblock_id, confirm=True): confirm_prompt(self) # this will also wait_for_notification() self.wait_for_ajax() + def nav_disabled(self, position, arrows=('next', 'previous')): + """ + Verifies that pagination nav is disabled. Position can be 'top' or 'bottom'. + + To specify a specific arrow, pass an iterable with a single element, 'next' or 'previous'. + """ + return all([ + self.q(css='nav.%s * a.%s-page-link.is-disabled' % (position, arrow)) + for arrow in arrows + ]) + + def move_back(self, position): + """ + Clicks one of the forward nav buttons. Position can be 'top' or 'bottom'. + """ + self.q(css='nav.%s * a.previous-page-link' % position)[0].click() + self.wait_until_ready() + + def move_forward(self, position): + """ + Clicks one of the forward nav buttons. Position can be 'top' or 'bottom'. + """ + self.q(css='nav.%s * a.next-page-link' % position)[0].click() + self.wait_until_ready() + + def revisit(self): + """ + Visit the page's URL, instead of refreshing, so that a new state is created. + """ + self.browser.get(self.browser.current_url) + self.wait_until_ready() + + def go_to_page(self, number): + """ + Enter a number into the page number input field, and then try to navigate to it. + """ + page_input = self.q(css="#page-number-input")[0] + page_input.click() + page_input.send_keys(str(number)) + page_input.send_keys(Keys.RETURN) + self.wait_until_ready() + + def check_page_unchanged(self, first_block_name): + """ + Used to make sure that a page has not transitioned after a bogus number is given. + """ + if not self.xblocks[0].name == first_block_name: + return False + if not self.q(css='#page-number-input')[0].get_attribute('value') == '': + return False + return True + def _get_xblocks(self): """ Create an XBlockWrapper for each XBlock div found on the page. diff --git a/common/test/acceptance/tests/studio/test_studio_library.py b/common/test/acceptance/tests/studio/test_studio_library.py index b505ac140dbc..5529f3603293 100644 --- a/common/test/acceptance/tests/studio/test_studio_library.py +++ b/common/test/acceptance/tests/studio/test_studio_library.py @@ -1,11 +1,14 @@ """ Acceptance tests for Content Libraries in Studio """ +from ddt import ddt, data + from .base_studio_test import StudioLibraryTest from ...pages.studio.utils import add_component from ...pages.studio.library import LibraryPage +@ddt class LibraryEditPageTest(StudioLibraryTest): """ Test the functionality of the library edit page. @@ -107,3 +110,161 @@ def test_no_discussion_button(self): Ensure the UI is not loaded for adding discussions. """ self.assertFalse(self.browser.find_elements_by_css_selector('span.large-discussion-icon')) + + def test_library_pagination(self): + """ + Scenario: Ensure that adding several XBlocks to a library results in pagination. + Given that I have a library in Studio with no XBlocks + And I create 10 Multiple Choice XBlocks + Then 10 are displayed. + When I add one more Multiple Choice XBlock + Then 1 XBlock will be displayed + When I delete that XBlock + Then 10 are displayed. + """ + self.assertEqual(len(self.lib_page.xblocks), 0) + for _ in range(0, 10): + add_component(self.lib_page, "problem", "Multiple Choice") + self.assertEqual(len(self.lib_page.xblocks), 10) + add_component(self.lib_page, "problem", "Multiple Choice") + self.assertEqual(len(self.lib_page.xblocks), 1) + self.lib_page.click_delete_button(self.lib_page.xblocks[0].locator) + self.assertEqual(len(self.lib_page.xblocks), 10) + + @data('top', 'bottom') + def test_nav_present_but_disabled(self, position): + """ + Scenario: Ensure that the navigation buttons aren't active when there aren't enough XBlocks. + Given that I have a library in Studio with no XBlocks + The Navigation buttons should be disabled. + When I add 5 multiple Choice XBlocks + The Navigation buttons should be disabled. + """ + self.assertEqual(len(self.lib_page.xblocks), 0) + self.assertTrue(self.lib_page.nav_disabled(position)) + for _ in range(0, 5): + add_component(self.lib_page, "problem", "Multiple Choice") + self.assertTrue(self.lib_page.nav_disabled(position)) + + @data('top', 'bottom') + def test_nav_buttons(self, position): + """ + Scenario: Ensure that the navigation buttons work. + Given that I have a library in Studio with no XBlocks + And I create 10 Multiple Choice XBlocks + And I create 10 Checkbox XBlocks + And I create 10 Dropdown XBlocks + And I revisit the page + The previous button should be disabled. + The first XBlock should be a Multiple Choice XBlock + Then if I hit the next button + The first XBlock should be a Checkboxes XBlock + Then if I hit the next button + The first XBlock should be a Dropdown XBlock + And the next button should be disabled + Then if I hit the previous button + The first XBlock should be an Checkboxes XBlock + Then if I hit the previous button + The first XBlock should be a Multipe Choice XBlock + And the previous button should be disabled + """ + self.assertEqual(len(self.lib_page.xblocks), 0) + block_types = [('problem', 'Multiple Choice'), ('problem', 'Checkboxes'), ('problem', 'Dropdown')] + for block_type in block_types: + for _ in range(0, 10): + add_component(self.lib_page, *block_type) + + # Don't refresh, as that may contain additional state. + self.lib_page.revisit() + + # Check forward navigation + self.assertTrue(self.lib_page.nav_disabled(position, ['previous'])) + self.assertEqual(self.lib_page.xblocks[0].name, 'Multiple Choice') + self.lib_page.move_forward(position) + self.assertEqual(self.lib_page.xblocks[0].name, 'Checkboxes') + self.lib_page.move_forward(position) + self.assertEqual(self.lib_page.xblocks[0].name, 'Dropdown') + self.lib_page.nav_disabled(position, ['next']) + + # Check backward navigation + self.lib_page.move_back(position) + self.assertEqual(self.lib_page.xblocks[0].name, 'Checkboxes') + self.lib_page.move_back(position) + self.assertEqual(self.lib_page.xblocks[0].name, 'Multiple Choice') + self.assertTrue(self.lib_page.nav_disabled(position, ['previous'])) + + def test_arbitrary_page_selection(self): + """ + Scenario: I can pick a specific page number of a Library at will. + Given that I have a library in Studio with no XBlocks + And I create 10 Multiple Choice XBlocks + And I create 10 Checkboxes XBlocks + And I create 10 Dropdown XBlocks + And I create 10 Numerical Input XBlocks + And I revisit the page + When I go to the 3rd page + The first XBlock should be a Dropdown XBlock + When I go to the 4th Page + The first XBlock should be a Numerical Input XBlock + When I go to the 1st page + The first XBlock should be a Multiple Choice XBlock + When I go to the 2nd page + The first XBlock should be a Checkboxes XBlock + """ + self.assertEqual(len(self.lib_page.xblocks), 0) + block_types = [ + ('problem', 'Multiple Choice'), ('problem', 'Checkboxes'), ('problem', 'Dropdown'), + ('problem', 'Numerical Input'), + ] + for block_type in block_types: + for _ in range(0, 10): + add_component(self.lib_page, *block_type) + + # Don't refresh, as that may contain additional state. + self.lib_page.revisit() + self.lib_page.go_to_page(3) + self.assertEqual(self.lib_page.xblocks[0].name, 'Dropdown') + self.lib_page.go_to_page(4) + self.assertEqual(self.lib_page.xblocks[0].name, 'Numerical Input') + self.lib_page.go_to_page(1) + self.assertEqual(self.lib_page.xblocks[0].name, 'Multiple Choice') + self.lib_page.go_to_page(2) + self.assertEqual(self.lib_page.xblocks[0].name, 'Checkboxes') + + def test_bogus_page_selection(self): + """ + Scenario: I can't pick a nonsense page number of a Library + Given that I have a library in Studio with no XBlocks + And I create 10 Multiple Choice XBlocks + And I create 10 Checkboxes XBlocks + And I create 10 Dropdown XBlocks + And I create 10 Numerical Input XBlocks + And I revisit the page + When I attempt to go to the 'a'th page + The input field will be cleared and no change of XBlocks will be made + When I attempt to visit the 5th page + The input field will be cleared and no change of XBlocks will be made + When I attempt to visit the -1st page + The input field will be cleared and no change of XBlocks will be made + When I attempt to visit the 0th page + The input field will be cleared and no change of XBlocks will be made + """ + self.assertEqual(len(self.lib_page.xblocks), 0) + block_types = [ + ('problem', 'Multiple Choice'), ('problem', 'Checkboxes'), ('problem', 'Dropdown'), + ('problem', 'Numerical Input'), + ] + for block_type in block_types: + for _ in range(0, 10): + add_component(self.lib_page, *block_type) + + self.lib_page.revisit() + self.assertEqual(self.lib_page.xblocks[0].name, 'Multiple Choice') + self.lib_page.go_to_page('a') + self.assertTrue(self.lib_page.check_page_unchanged('Multiple Choice')) + self.lib_page.go_to_page(-1) + self.assertTrue(self.lib_page.check_page_unchanged('Multiple Choice')) + self.lib_page.go_to_page(5) + self.assertTrue(self.lib_page.check_page_unchanged('Multiple Choice')) + self.lib_page.go_to_page(0) + self.assertTrue(self.lib_page.check_page_unchanged('Multiple Choice')) From 26e9399b39b16381de57e97bde7f2061c09ea0f5 Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Mon, 8 Dec 2014 22:22:02 +0000 Subject: [PATCH 05/23] Addressed notes from reviewers on Library Pagination. --- cms/djangoapps/contentstore/views/item.py | 5 +- .../js/spec/views/pages/container_spec.js | 32 +- cms/static/js/views/library_container.js | 71 +++-- cms/static/js/views/pages/container.js | 23 +- cms/static/sass/elements/_pagination.scss | 8 +- cms/static/sass/elements/_xblocks.scss | 2 +- ...ontainer-paged-after-add-xblock.underscore | 283 ------------------ .../js/mock/mock-xblock-paged.underscore | 21 ++ .../xmodule/xmodule/library_root_xblock.py | 3 +- .../xmodule/video_module/video_handlers.py | 1 + 10 files changed, 113 insertions(+), 336 deletions(-) delete mode 100644 cms/templates/js/mock/mock-container-paged-after-add-xblock.underscore create mode 100644 cms/templates/js/mock/mock-xblock-paged.underscore diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 5d05a27e1781..7f68dcd78481 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -207,6 +207,7 @@ def xblock_view_handler(request, usage_key_string, view_name): store = modulestore() xblock = store.get_item(usage_key) container_views = ['container_preview', 'reorderable_container_child_preview'] + library = isinstance(usage_key, LibraryUsageLocator) # wrap the generated fragment in the xmodule_editor div so that the javascript # can bind to it correctly @@ -235,7 +236,7 @@ def xblock_view_handler(request, usage_key_string, view_name): # are being shown in a reorderable container, so the xblock is automatically # added to the list. reorderable_items = set() - if view_name == 'reorderable_container_child_preview': + if not library and view_name == 'reorderable_container_child_preview': reorderable_items.add(xblock.location) paging = None @@ -259,7 +260,7 @@ def xblock_view_handler(request, usage_key_string, view_name): 'is_unit_page': is_unit(xblock), 'root_xblock': xblock if (view_name == 'container_preview') else None, 'reorderable_items': reorderable_items, - 'paging': paging + 'paging': paging, } fragment = get_preview_fragment(request, xblock, context) diff --git a/cms/static/js/spec/views/pages/container_spec.js b/cms/static/js/spec/views/pages/container_spec.js index 6f4b4baf46af..d5ec6938dc25 100644 --- a/cms/static/js/spec/views/pages/container_spec.js +++ b/cms/static/js/spec/views/pages/container_spec.js @@ -273,7 +273,7 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel }); describe("xblock operations", function () { - var getGroupElement, + var getGroupElement, paginated, NUM_COMPONENTS_PER_GROUP = 3, GROUP_TO_TEST = "A", allComponentsInGroup = _.map( _.range(NUM_COMPONENTS_PER_GROUP), @@ -282,6 +282,11 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel } ); + paginated = function () { + return containerPage.enable_paging; + }; + + getGroupElement = function () { return containerPage.$("[data-locator='locator-group-" + GROUP_TO_TEST + "']"); }; @@ -294,6 +299,7 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel promptSpy = EditHelpers.createPromptSpy(); }); + clickDelete = function (componentIndex, clickNo) { // find all delete buttons for the given group @@ -307,21 +313,25 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel EditHelpers.confirmPrompt(promptSpy, clickNo); }; - deleteComponent = function (componentIndex) { + deleteComponent = function (componentIndex, requestOffset) { clickDelete(componentIndex); AjaxHelpers.respondWithJson(requests, {}); // second to last request contains given component's id (to delete the component) AjaxHelpers.expectJsonRequest(requests, 'DELETE', '/xblock/locator-component-' + GROUP_TO_TEST + (componentIndex + 1), - null, requests.length - 2); + null, requests.length - requestOffset); // final request to refresh the xblock info AjaxHelpers.expectJsonRequest(requests, 'GET', '/xblock/locator-container'); }; deleteComponentWithSuccess = function (componentIndex) { - deleteComponent(componentIndex); + var deleteOffset; + + deleteOffset = paginated() ? 3 : 2; + + deleteComponent(componentIndex, deleteOffset); // verify the new list of components within the group expectComponents( @@ -350,9 +360,16 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel containerPage.$('.delete-button').first().click(); EditHelpers.confirmPrompt(promptSpy); AjaxHelpers.respondWithJson(requests, {}); + var deleteOffset; + + if (paginated()) { + deleteOffset = 3; + } else { + deleteOffset = 2; + } // expect the second to last request to be a delete of the xblock AjaxHelpers.expectJsonRequest(requests, 'DELETE', '/xblock/locator-broken-javascript', - null, requests.length - 2); + null, requests.length - deleteOffset); // expect the last request to be a fetch of the xblock info for the parent container AjaxHelpers.expectJsonRequest(requests, 'GET', '/xblock/locator-container'); }); @@ -511,7 +528,7 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel }); describe('Template Picker', function () { - var showTemplatePicker, verifyCreateHtmlComponent; + var showTemplatePicker, verifyCreateHtmlComponent, call_count; showTemplatePicker = function () { containerPage.$('.new-component .new-component-type a.multiple-templates')[0].click(); @@ -519,6 +536,7 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel verifyCreateHtmlComponent = function (test, templateIndex, expectedRequest) { var xblockCount; + // call_count = paginated() ? 18: 10; renderContainerPage(test, mockContainerXBlockHtml); showTemplatePicker(); xblockCount = containerPage.$('.studio-xblock-wrapper').length; @@ -557,6 +575,6 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel { enable_paging: true, page_size: 42 }, { initial: 'mock/mock-container-paged-xblock.underscore', - add_response: 'mock/mock-container-paged-after-add-xblock.underscore' + add_response: 'mock/mock-xblock-paged.underscore' }); }); diff --git a/cms/static/js/views/library_container.js b/cms/static/js/views/library_container.js index b655833289c9..a8c15999ab3f 100644 --- a/cms/static/js/views/library_container.js +++ b/cms/static/js/views/library_container.js @@ -1,19 +1,16 @@ -define(["jquery", "underscore", "js/views/xblock", "js/utils/module", "gettext", "js/views/feedback_notification", +define(["jquery", "underscore", "js/views/container", "js/utils/module", "gettext", "js/views/feedback_notification", "js/views/paging_header", "js/views/paging_footer"], - function ($, _, XBlockView, ModuleUtils, gettext, NotificationView, PagingHeader, PagingFooter) { - var LibraryContainerView = XBlockView.extend({ + function ($, _, ContainerView, ModuleUtils, gettext, NotificationView, PagingHeader, PagingFooter) { + var LibraryContainerView = ContainerView.extend({ // Store the request token of the first xblock on the page (which we know was rendered by Studio when // the page was generated). Use that request token to filter out user-defined HTML in any // child xblocks within the page. - requestToken: "", initialize: function(options){ var self = this; - XBlockView.prototype.initialize.call(this); + ContainerView.prototype.initialize.call(this); this.page_size = this.options.page_size || 10; - if (options) { - this.page_reload_callback = options.page_reload_callback; - } + this.page_reload_callback = options.page_reload_callback || function () {}; // emulating Backbone.paginator interface this.collection = { currentPage: 0, @@ -30,9 +27,6 @@ define(["jquery", "underscore", "js/views/xblock", "js/utils/module", "gettext", render: function(options) { var eff_options = options || {}; - if (eff_options.block_added) { - this.collection.currentPage = this.getPageCount(this.collection.totalCount+1) - 1; - } eff_options.page_number = typeof eff_options.page_number !== "undefined" ? eff_options.page_number : this.collection.currentPage; @@ -53,9 +47,8 @@ define(["jquery", "underscore", "js/views/xblock", "js/utils/module", "gettext", success: function(fragment) { self.handleXBlockFragment(fragment, options); self.processPaging({ requested_page: options.page_number }); - if (options.paging && self.page_reload_callback){ - self.page_reload_callback(self.$el); - } + // This is expected to render the add xblock components menu. + self.page_reload_callback(self.$el) } }); }, @@ -69,12 +62,12 @@ define(["jquery", "underscore", "js/views/xblock", "js/utils/module", "gettext", }, getPageCount: function(total_count){ - if (total_count==0) return 1; + if (total_count===0) return 1; return Math.ceil(total_count / this.page_size); }, setPage: function(page_number) { - this.render({ page_number: page_number, paging: true }); + this.render({ page_number: page_number}); }, nextPage: function() { @@ -129,32 +122,54 @@ define(["jquery", "underscore", "js/views/xblock", "js/utils/module", "gettext", }, xblockReady: function () { - XBlockView.prototype.xblockReady.call(this); + ContainerView.prototype.xblockReady.call(this); this.requestToken = this.$('div.xblock').first().data('request-token'); }, - refresh: function() { }, + refresh: function(block_added) { + if (block_added) { + this.collection.totalCount += 1; + this.collection._size +=1; + if (this.collection.totalCount == 1) { + this.render(); + return + } + this.collection.totalPages = this.getPageCount(this.collection.totalCount); + var new_page = this.collection.totalPages - 1; + // If we're on a new page due to overflow, or this is the first item, set the page. + if (((this.collection.currentPage) != new_page) || this.collection.totalCount == 1) { + this.setPage(new_page); + } else { + this.pagingHeader.render(); + this.pagingFooter.render(); + } + } + }, acknowledgeXBlockDeletion: function (locator){ this.notifyRuntime('deleted-child', locator); this.collection._size -= 1; this.collection.totalCount -= 1; - // pages are counted from 0 - thus currentPage == 1 if we're on second page - if (this.collection._size == 0 && this.collection.currentPage >= 1) { - this.setPage(this.collection.currentPage - 1); - this.collection.totalPages -= 1; - } - else { + var current_page = this.collection.currentPage; + var total_pages = this.getPageCount(this.collection.totalCount); + this.collection.totalPages = total_pages; + // Starts counting from 0 + if ((current_page + 1) > total_pages) { + // The number of total pages has changed. Move down. + // Also, be mindful of the off-by-one. + this.setPage(total_pages - 1) + } else if ((current_page + 1) != total_pages) { + // Refresh page to get any blocks shifted from the next page. + this.setPage(current_page) + } else { + // We're on the last page, just need to update the numbers in the + // pagination interface. this.pagingHeader.render(); this.pagingFooter.render(); } }, - makeRequestSpecificSelector: function(selector) { - return 'div.xblock[data-request-token="' + this.requestToken + '"] > ' + selector; - }, - sortDisplayName: function() { return "Date added"; // TODO add support for sorting } diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index c63e7f00f6e0..771e9afb1b89 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -119,8 +119,11 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views // Notify the runtime that the page has been successfully shown xblockView.notifyRuntime('page-shown', self); - // Render the add buttons - self.renderAddXBlockComponents(); + // Render the add buttons. Paged containers should do this on their own. + if (!self.enable_paging) { + // Render the add buttons + self.renderAddXBlockComponents(); + } // Refresh the views now that the xblock is visible self.onXBlockRefresh(xblockView); @@ -141,8 +144,8 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views return this.xblockView.model.urlRoot; }, - onXBlockRefresh: function(xblockView) { - this.xblockView.refresh(); + onXBlockRefresh: function(xblockView, block_added) { + this.xblockView.refresh(block_added); // Update publish and last modified information from the server. this.model.fetch(); }, @@ -274,10 +277,10 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views rootLocator = this.xblockView.model.id; if (xblockElement.length === 0 || xblockElement.data('locator') === rootLocator) { this.render({refresh: true, block_added: block_added}); - } else if (parentElement.hasClass('reorderable-container')) { - this.refreshChildXBlock(xblockElement); + } else if (parentElement.hasClass('reorderable-container') || this.enable_paging) { + this.refreshChildXBlock(xblockElement, block_added); } else { - this.refreshXBlock(this.findXBlockElement(parentElement), block_added); + this.refreshXBlock(this.findXBlockElement(parentElement)); } }, @@ -285,9 +288,11 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views * 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 + * refreshing. * @returns {jQuery promise} A promise representing the complete operation. */ - refreshChildXBlock: function(xblockElement) { + refreshChildXBlock: function(xblockElement, block_added) { var self = this, xblockInfo, TemporaryXBlockView, @@ -313,7 +318,7 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views }); return temporaryView.render({ success: function() { - self.onXBlockRefresh(temporaryView); + self.onXBlockRefresh(temporaryView, block_added); temporaryView.unbind(); // Remove the temporary view } }); diff --git a/cms/static/sass/elements/_pagination.scss b/cms/static/sass/elements/_pagination.scss index f3ba465b8086..379d8785e315 100644 --- a/cms/static/sass/elements/_pagination.scss +++ b/cms/static/sass/elements/_pagination.scss @@ -2,7 +2,7 @@ // ========================== %pagination { - @include clearfix; + @include clearfix(); display: inline-block; width: flex-grid(3, 12); @@ -48,7 +48,7 @@ } .nav-label { - @extend .sr; + @extend %cont-text-sr; } .pagination-form, @@ -89,7 +89,7 @@ .page-number-label, .submit-pagination-form { - @extend .sr; + @extend %cont-text-sr; } .page-number-input { @@ -116,4 +116,4 @@ } } } -} \ No newline at end of file +} diff --git a/cms/static/sass/elements/_xblocks.scss b/cms/static/sass/elements/_xblocks.scss index 938f000d8e53..0473d748e7d6 100644 --- a/cms/static/sass/elements/_xblocks.scss +++ b/cms/static/sass/elements/_xblocks.scss @@ -105,7 +105,7 @@ .container-paging-header { .meta-wrap { - margin: $baseline $baseline/2; + margin: $baseline ($baseline/2); } .meta { @extend %t-copy-sub2; diff --git a/cms/templates/js/mock/mock-container-paged-after-add-xblock.underscore b/cms/templates/js/mock/mock-container-paged-after-add-xblock.underscore deleted file mode 100644 index cb260c9bca9d..000000000000 --- a/cms/templates/js/mock/mock-container-paged-after-add-xblock.underscore +++ /dev/null @@ -1,283 +0,0 @@ - -
-
-
- Test Container -
-
-
    -
-
-
-
-
-
- - - - -
- -
-
-
-
-
- - - Expand or Collapse - - Group A -
-
-
    -
  • - -
    • -
-
-
-
-
-
-
-
-
-
-
-
    -
  • - -
    • -
  • - -
    • -
  • - -
    • -
  • - -
    • -
-
-
-
-
-
-
-
-
-
-
-
-
    -
  • - -
    • -
  • - -
    • -
  • - -
    • -
  • - -
    • -
-
-
-
-
-
-
-
-
-
-
-
-
    -
  • - -
    • -
  • - -
    • -
  • - -
    • -
  • - -
    • -
-
-
-
-
-
-
-
-
-
-
-
-
    -
  • - -
    • -
  • - -
    • -
  • - -
    • -
  • - -
    • -
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
- - - Expand or Collapse - - Group B -
-
-
    -
  • - -
    • -
-
-
-
- -
-
-
-
-
-
-
-
    -
  • - -
    • -
  • - -
    • -
  • - -
    • -
  • - -
    • -
-
-
-
-
-
-
-
-
-
-
-
-
    -
  • - -
    • -
  • - -
    • -
  • - -
    • -
  • - -
    • -
-
-
-
-
-
-
-
-
-
-
-
-
    -
  • - -
    • -
  • - -
    • -
  • - -
    • -
  • - -
    • -
-
-
-
-
-
-
-
-
-
-
-
-
-
diff --git a/cms/templates/js/mock/mock-xblock-paged.underscore b/cms/templates/js/mock/mock-xblock-paged.underscore new file mode 100644 index 000000000000..c6c2c881d8d2 --- /dev/null +++ b/cms/templates/js/mock/mock-xblock-paged.underscore @@ -0,0 +1,21 @@ +
+
+
+ Mock XBlock +
+
+
    +
+ + Add Missing Groups + +
+
+
+
+

Mock XBlock

+
+
+
diff --git a/common/lib/xmodule/xmodule/library_root_xblock.py b/common/lib/xmodule/xmodule/library_root_xblock.py index 497a145b79bd..3118f9a25842 100644 --- a/common/lib/xmodule/xmodule/library_root_xblock.py +++ b/common/lib/xmodule/xmodule/library_root_xblock.py @@ -76,7 +76,7 @@ def render_children(self, context, fragment, can_reorder=False, can_add=False): fragment.add_frag_resources(rendered_child) contents.append({ - 'id': child.location.to_deprecated_string(), + 'id': unicode(child.location), 'content': rendered_child.content }) @@ -85,7 +85,6 @@ def render_children(self, context, fragment, can_reorder=False, can_add=False): 'items': contents, 'xblock_context': context, 'can_add': can_add, - 'can_reorder': False, 'first_displayed': item_start, 'total_children': children_count, 'displayed_children': len(children_to_show) diff --git a/common/lib/xmodule/xmodule/video_module/video_handlers.py b/common/lib/xmodule/xmodule/video_module/video_handlers.py index 1ba427c35785..9e9db860ca54 100644 --- a/common/lib/xmodule/xmodule/video_module/video_handlers.py +++ b/common/lib/xmodule/xmodule/video_module/video_handlers.py @@ -155,6 +155,7 @@ def get_static_transcript(self, request): if transcript_name: # Get the asset path for course + asset_path = None course = self.descriptor.runtime.modulestore.get_course(self.course_id) if course.static_asset_path: asset_path = course.static_asset_path From 1581a398248e54a85505706a216ccc6a9a4c2eb5 Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Thu, 11 Dec 2014 19:53:08 +0000 Subject: [PATCH 06/23] Factored out Pagination into its own Container view. --- cms/static/js/views/library_container.js | 180 +---------------------- cms/static/js/views/paged_container.js | 158 ++++++++++++++++++++ cms/static/js/views/paging.js | 40 +---- cms/static/js/views/paging_mixin.js | 37 +++++ 4 files changed, 202 insertions(+), 213 deletions(-) create mode 100644 cms/static/js/views/paged_container.js create mode 100644 cms/static/js/views/paging_mixin.js diff --git a/cms/static/js/views/library_container.js b/cms/static/js/views/library_container.js index a8c15999ab3f..7c48e83cee8f 100644 --- a/cms/static/js/views/library_container.js +++ b/cms/static/js/views/library_container.js @@ -1,179 +1,7 @@ -define(["jquery", "underscore", "js/views/container", "js/utils/module", "gettext", "js/views/feedback_notification", +define(["jquery", "underscore", "js/views/paged_container", "js/utils/module", "gettext", "js/views/feedback_notification", "js/views/paging_header", "js/views/paging_footer"], - function ($, _, ContainerView, ModuleUtils, gettext, NotificationView, PagingHeader, PagingFooter) { - var LibraryContainerView = ContainerView.extend({ - // Store the request token of the first xblock on the page (which we know was rendered by Studio when - // the page was generated). Use that request token to filter out user-defined HTML in any - // child xblocks within the page. - - initialize: function(options){ - var self = this; - ContainerView.prototype.initialize.call(this); - this.page_size = this.options.page_size || 10; - this.page_reload_callback = options.page_reload_callback || function () {}; - // emulating Backbone.paginator interface - this.collection = { - currentPage: 0, - totalPages: 0, - totalCount: 0, - sortDirection: "desc", - start: 0, - _size: 0, - - bind: function() {}, // no-op - size: function() { return self.collection._size; } - }; - }, - - render: function(options) { - var eff_options = options || {}; - eff_options.page_number = typeof eff_options.page_number !== "undefined" - ? eff_options.page_number - : this.collection.currentPage; - return this.renderPage(eff_options); - }, - - renderPage: function(options){ - var self = this, - view = this.view, - xblockInfo = this.model, - xblockUrl = xblockInfo.url(); - return $.ajax({ - url: decodeURIComponent(xblockUrl) + "/" + view, - type: 'GET', - cache: false, - data: this.getRenderParameters(options.page_number), - headers: { Accept: 'application/json' }, - success: function(fragment) { - self.handleXBlockFragment(fragment, options); - self.processPaging({ requested_page: options.page_number }); - // This is expected to render the add xblock components menu. - self.page_reload_callback(self.$el) - } - }); - }, - - getRenderParameters: function(page_number) { - return { - enable_paging: true, - page_size: this.page_size, - page_number: page_number - }; - }, - - getPageCount: function(total_count){ - if (total_count===0) return 1; - return Math.ceil(total_count / this.page_size); - }, - - setPage: function(page_number) { - this.render({ page_number: page_number}); - }, - - nextPage: function() { - var collection = this.collection, - currentPage = collection.currentPage, - lastPage = collection.totalPages - 1; - if (currentPage < lastPage) { - this.setPage(currentPage + 1); - } - }, - - previousPage: function() { - var collection = this.collection, - currentPage = collection.currentPage; - if (currentPage > 0) { - this.setPage(currentPage - 1); - } - }, - - processPaging: function(options){ - var $element = this.$el.find('.xblock-container-paging-parameters'), - total = $element.data('total'), - displayed = $element.data('displayed'), - start = $element.data('start'); - - this.collection.currentPage = options.requested_page; - this.collection.totalCount = total; - this.collection.totalPages = this.getPageCount(total); - this.collection.start = start; - this.collection._size = displayed; - - this.processPagingHeaderAndFooter(); - }, - - processPagingHeaderAndFooter: function(){ - if (this.pagingHeader) - this.pagingHeader.undelegateEvents(); - if (this.pagingFooter) - this.pagingFooter.undelegateEvents(); - - this.pagingHeader = new PagingHeader({ - view: this, - el: this.$el.find('.container-paging-header') - }); - this.pagingFooter = new PagingFooter({ - view: this, - el: this.$el.find('.container-paging-footer') - }); - - this.pagingHeader.render(); - this.pagingFooter.render(); - }, - - xblockReady: function () { - ContainerView.prototype.xblockReady.call(this); - - this.requestToken = this.$('div.xblock').first().data('request-token'); - }, - - refresh: function(block_added) { - if (block_added) { - this.collection.totalCount += 1; - this.collection._size +=1; - if (this.collection.totalCount == 1) { - this.render(); - return - } - this.collection.totalPages = this.getPageCount(this.collection.totalCount); - var new_page = this.collection.totalPages - 1; - // If we're on a new page due to overflow, or this is the first item, set the page. - if (((this.collection.currentPage) != new_page) || this.collection.totalCount == 1) { - this.setPage(new_page); - } else { - this.pagingHeader.render(); - this.pagingFooter.render(); - } - } - }, - - acknowledgeXBlockDeletion: function (locator){ - this.notifyRuntime('deleted-child', locator); - this.collection._size -= 1; - this.collection.totalCount -= 1; - var current_page = this.collection.currentPage; - var total_pages = this.getPageCount(this.collection.totalCount); - this.collection.totalPages = total_pages; - // Starts counting from 0 - if ((current_page + 1) > total_pages) { - // The number of total pages has changed. Move down. - // Also, be mindful of the off-by-one. - this.setPage(total_pages - 1) - } else if ((current_page + 1) != total_pages) { - // Refresh page to get any blocks shifted from the next page. - this.setPage(current_page) - } else { - // We're on the last page, just need to update the numbers in the - // pagination interface. - this.pagingHeader.render(); - this.pagingFooter.render(); - } - }, - - sortDisplayName: function() { - return "Date added"; // TODO add support for sorting - } - }); - + function ($, _, PagedContainerView) { + // To be extended with Library-specific features later. + var LibraryContainerView = PagedContainerView; return LibraryContainerView; }); // end define(); diff --git a/cms/static/js/views/paged_container.js b/cms/static/js/views/paged_container.js new file mode 100644 index 000000000000..cd7590156a17 --- /dev/null +++ b/cms/static/js/views/paged_container.js @@ -0,0 +1,158 @@ +define(["jquery", "underscore", "js/views/container", "js/utils/module", "gettext", + "js/views/feedback_notification", "js/views/paging_header", "js/views/paging_footer", "js/views/paging_mixin"], + function ($, _, ContainerView, ModuleUtils, gettext, NotificationView, PagingHeader, PagingFooter, PagingMixin) { + var PagedContainerView = ContainerView.extend(PagingMixin).extend({ + initialize: function(options){ + var self = this; + ContainerView.prototype.initialize.call(this); + this.page_size = this.options.page_size || 10; + this.page_reload_callback = options.page_reload_callback || function () {}; + // emulating Backbone.paginator interface + this.collection = { + currentPage: 0, + totalPages: 0, + totalCount: 0, + sortDirection: "desc", + start: 0, + _size: 0, + + bind: function() {}, // no-op + size: function() { return self.collection._size; } + }; + }, + + render: function(options) { + var eff_options = options || {}; + eff_options.page_number = typeof eff_options.page_number !== "undefined" + ? eff_options.page_number + : this.collection.currentPage; + return this.renderPage(eff_options); + }, + + renderPage: function(options){ + var self = this, + view = this.view, + xblockInfo = this.model, + xblockUrl = xblockInfo.url(); + return $.ajax({ + url: decodeURIComponent(xblockUrl) + "/" + view, + type: 'GET', + cache: false, + data: this.getRenderParameters(options.page_number), + headers: { Accept: 'application/json' }, + success: function(fragment) { + self.handleXBlockFragment(fragment, options); + self.processPaging({ requested_page: options.page_number }); + // This is expected to render the add xblock components menu. + self.page_reload_callback(self.$el) + } + }); + }, + + getRenderParameters: function(page_number) { + return { + enable_paging: true, + page_size: this.page_size, + page_number: page_number + }; + }, + + getPageCount: function(total_count){ + if (total_count===0) return 1; + return Math.ceil(total_count / this.page_size); + }, + + setPage: function(page_number) { + this.render({ page_number: page_number}); + }, + + processPaging: function(options){ + var $element = this.$el.find('.xblock-container-paging-parameters'), + total = $element.data('total'), + displayed = $element.data('displayed'), + start = $element.data('start'); + + this.collection.currentPage = options.requested_page; + this.collection.totalCount = total; + this.collection.totalPages = this.getPageCount(total); + this.collection.start = start; + this.collection._size = displayed; + + this.processPagingHeaderAndFooter(); + }, + + processPagingHeaderAndFooter: function(){ + if (this.pagingHeader) + this.pagingHeader.undelegateEvents(); + if (this.pagingFooter) + this.pagingFooter.undelegateEvents(); + + this.pagingHeader = new PagingHeader({ + view: this, + el: this.$el.find('.container-paging-header') + }); + this.pagingFooter = new PagingFooter({ + view: this, + el: this.$el.find('.container-paging-footer') + }); + + this.pagingHeader.render(); + this.pagingFooter.render(); + }, + + xblockReady: function () { + ContainerView.prototype.xblockReady.call(this); + + this.requestToken = this.$('div.xblock').first().data('request-token'); + }, + + refresh: function(block_added) { + if (block_added) { + this.collection.totalCount += 1; + this.collection._size +=1; + if (this.collection.totalCount == 1) { + this.render(); + return + } + this.collection.totalPages = this.getPageCount(this.collection.totalCount); + var new_page = this.collection.totalPages - 1; + // If we're on a new page due to overflow, or this is the first item, set the page. + if (((this.collection.currentPage) != new_page) || this.collection.totalCount == 1) { + this.setPage(new_page); + } else { + this.pagingHeader.render(); + this.pagingFooter.render(); + } + } + }, + + acknowledgeXBlockDeletion: function (locator){ + this.notifyRuntime('deleted-child', locator); + this.collection._size -= 1; + this.collection.totalCount -= 1; + var current_page = this.collection.currentPage; + var total_pages = this.getPageCount(this.collection.totalCount); + this.collection.totalPages = total_pages; + // Starts counting from 0 + if ((current_page + 1) > total_pages) { + // The number of total pages has changed. Move down. + // Also, be mindful of the off-by-one. + this.setPage(total_pages - 1) + } else if ((current_page + 1) != total_pages) { + // Refresh page to get any blocks shifted from the next page. + this.setPage(current_page) + } else { + // We're on the last page, just need to update the numbers in the + // pagination interface. + this.pagingHeader.render(); + this.pagingFooter.render(); + } + }, + + sortDisplayName: function() { + return "Date added"; // TODO add support for sorting + } + }); + + return PagedContainerView; + }); // end define(); diff --git a/cms/static/js/views/paging.js b/cms/static/js/views/paging.js index c6c3a491ca04..c4d9b1b602f5 100644 --- a/cms/static/js/views/paging.js +++ b/cms/static/js/views/paging.js @@ -1,7 +1,7 @@ -define(["underscore", "js/views/baseview", "js/views/feedback_alert", "gettext"], - function(_, BaseView, AlertView, gettext) { +define(["underscore", "js/views/baseview", "js/views/feedback_alert", "gettext", "js/views/paging_mixin"], + function(_, BaseView, AlertView, gettext, PagingMixin) { - var PagingView = BaseView.extend({ + var PagingView = BaseView.extend(PagingMixin).extend({ // takes a Backbone Paginator as a model sortableColumns: {}, @@ -21,43 +21,10 @@ define(["underscore", "js/views/baseview", "js/views/feedback_alert", "gettext"] this.$('#' + sortColumn).addClass('current-sort'); }, - setPage: function(page) { - var self = this, - collection = self.collection, - oldPage = collection.currentPage; - collection.goTo(page, { - reset: true, - success: function() { - window.scrollTo(0, 0); - }, - error: function(collection) { - collection.currentPage = oldPage; - self.onError(); - } - }); - }, - onError: function() { // Do nothing by default }, - nextPage: function() { - var collection = this.collection, - currentPage = collection.currentPage, - lastPage = collection.totalPages - 1; - if (currentPage < lastPage) { - this.setPage(currentPage + 1); - } - }, - - previousPage: function() { - var collection = this.collection, - currentPage = collection.currentPage; - if (currentPage > 0) { - this.setPage(currentPage - 1); - } - }, - /** * Registers information about a column that can be sorted. * @param columnName The element name of the column. @@ -110,6 +77,5 @@ define(["underscore", "js/views/baseview", "js/views/feedback_alert", "gettext"] this.setPage(0); } }); - return PagingView; }); // end define(); diff --git a/cms/static/js/views/paging_mixin.js b/cms/static/js/views/paging_mixin.js new file mode 100644 index 000000000000..d2c1700e5d64 --- /dev/null +++ b/cms/static/js/views/paging_mixin.js @@ -0,0 +1,37 @@ +define(["jquery", "underscore"], + function ($, _) { + var PagedMixin = { + setPage: function (page) { + var self = this, + collection = self.collection, + oldPage = collection.currentPage; + collection.goTo(page, { + reset: true, + success: function () { + window.scrollTo(0, 0); + }, + error: function (collection) { + collection.currentPage = oldPage; + self.onError(); + } + }); + }, + nextPage: function() { + var collection = this.collection, + currentPage = collection.currentPage, + lastPage = collection.totalPages - 1; + if (currentPage < lastPage) { + this.setPage(currentPage + 1); + } + }, + + previousPage: function() { + var collection = this.collection, + currentPage = collection.currentPage; + if (currentPage > 0) { + this.setPage(currentPage - 1); + } + } + }; + return PagedMixin; + }); From be3371ee85cb629f91705c2ae045cf418d3a286a Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Fri, 12 Dec 2014 19:19:56 +0000 Subject: [PATCH 07/23] Addressed further review notes for Library Pagination --- cms/djangoapps/contentstore/views/item.py | 19 +- cms/static/coffee/spec/main.coffee | 2 +- cms/static/js/factories/container.js | 10 +- cms/static/js/factories/library.js | 11 +- ...tainer_spec.js => paged_container_spec.js} | 4 +- .../js/spec/views/pages/container_spec.js | 43 ++-- cms/static/js/views/container.js | 2 + cms/static/js/views/library_container.js | 5 +- cms/static/js/views/paged_container.js | 44 ++-- cms/static/js/views/pages/container.js | 50 ++-- cms/static/js/views/pages/paged_container.js | 36 +++ cms/static/js/views/paging_footer.js | 2 + cms/static/js/views/paging_mixin.js | 4 +- cms/templates/library.html | 1 - .../xmodule/xmodule/library_root_xblock.py | 5 +- .../test/acceptance/pages/studio/library.py | 56 +---- .../acceptance/pages/studio/pagination.py | 62 +++++ .../tests/studio/test_studio_library.py | 222 ++++++++++-------- 18 files changed, 334 insertions(+), 244 deletions(-) rename cms/static/js/spec/views/{library_container_spec.js => paged_container_spec.js} (99%) create mode 100644 cms/static/js/views/pages/paged_container.js create mode 100644 common/test/acceptance/pages/studio/pagination.py diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 7f68dcd78481..c1f627009cd0 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -206,8 +206,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) - container_views = ['container_preview', 'reorderable_container_child_preview'] - library = isinstance(usage_key, LibraryUsageLocator) + container_views = ['container_preview', 'reorderable_container_child_preview', 'container_child_preview'] # wrap the generated fragment in the xmodule_editor div so that the javascript # can bind to it correctly @@ -236,7 +235,7 @@ def xblock_view_handler(request, usage_key_string, view_name): # are being shown in a reorderable container, so the xblock is automatically # added to the list. reorderable_items = set() - if not library and view_name == 'reorderable_container_child_preview': + if view_name == 'reorderable_container_child_preview': reorderable_items.add(xblock.location) paging = None @@ -247,11 +246,15 @@ def xblock_view_handler(request, usage_key_string, view_name): 'page_size': int(request.REQUEST.get('page_size', 0)), } except ValueError: - log.exception( - "Couldn't parse paging parameters: enable_paging: %s, page_number: %s, page_size: %s", - request.REQUEST.get('enable_paging', 'false'), - request.REQUEST.get('page_number', 0), - request.REQUEST.get('page_size', 0) + return HttpResponse( + content="Couldn't parse paging parameters: enable_paging: " + "%s, page_number: %s, page_size: %s".format( + request.REQUEST.get('enable_paging', 'false'), + request.REQUEST.get('page_number', 0), + request.REQUEST.get('page_size', 0) + ), + status=400, + content_type="text/plain", ) # Set up the context to be passed to each XBlock's render method. diff --git a/cms/static/coffee/spec/main.coffee b/cms/static/coffee/spec/main.coffee index e2ba18a82198..ba732d20c166 100644 --- a/cms/static/coffee/spec/main.coffee +++ b/cms/static/coffee/spec/main.coffee @@ -232,7 +232,7 @@ define([ "js/spec/views/assets_spec", "js/spec/views/baseview_spec", "js/spec/views/container_spec", - "js/spec/views/library_container_spec", + "js/spec/views/paged_container_spec", "js/spec/views/group_configuration_spec", "js/spec/views/paging_spec", "js/spec/views/unit_outline_spec", diff --git a/cms/static/js/factories/container.js b/cms/static/js/factories/container.js index ea48bb2a989f..429ae58f5151 100644 --- a/cms/static/js/factories/container.js +++ b/cms/static/js/factories/container.js @@ -7,11 +7,11 @@ function($, _, XBlockInfo, ContainerPage, ComponentTemplates, xmoduleLoader) { 'use strict'; return function (componentTemplates, XBlockInfoJson, action, options) { var main_options = { - el: $('#content'), - model: new XBlockInfo(XBlockInfoJson, {parse: true}), - action: action, - templates: new ComponentTemplates(componentTemplates, {parse: true}) - }; + el: $('#content'), + model: new XBlockInfo(XBlockInfoJson, {parse: true}), + action: action, + templates: new ComponentTemplates(componentTemplates, {parse: true}) + }; xmoduleLoader.done(function () { var view = new ContainerPage(_.extend(main_options, options)); diff --git a/cms/static/js/factories/library.js b/cms/static/js/factories/library.js index e7834f60ef3c..76ac47413ddc 100644 --- a/cms/static/js/factories/library.js +++ b/cms/static/js/factories/library.js @@ -1,20 +1,21 @@ define([ - 'jquery', 'underscore', 'js/models/xblock_info', 'js/views/pages/container', - 'js/collections/component_template', 'xmodule', 'coffee/src/main', + 'jquery', 'underscore', 'js/models/xblock_info', 'js/views/pages/paged_container', + 'js/views/library_container', 'js/collections/component_template', 'xmodule', 'coffee/src/main', 'xblock/cms.runtime.v1' ], -function($, _, XBlockInfo, ContainerPage, ComponentTemplates, xmoduleLoader) { +function($, _, XBlockInfo, PagedContainerPage, LibraryContainerView, ComponentTemplates, xmoduleLoader) { 'use strict'; return function (componentTemplates, XBlockInfoJson, options) { var main_options = { el: $('#content'), model: new XBlockInfo(XBlockInfoJson, {parse: true}), templates: new ComponentTemplates(componentTemplates, {parse: true}), - action: 'view' + action: 'view', + viewClass: LibraryContainerView }; xmoduleLoader.done(function () { - var view = new ContainerPage(_.extend(main_options, options)); + var view = new PagedContainerPage(_.extend(main_options, options)); view.render(); }); }; diff --git a/cms/static/js/spec/views/library_container_spec.js b/cms/static/js/spec/views/paged_container_spec.js similarity index 99% rename from cms/static/js/spec/views/library_container_spec.js rename to cms/static/js/spec/views/paged_container_spec.js index 2d39cdc35819..524f88e552f7 100644 --- a/cms/static/js/spec/views/library_container_spec.js +++ b/cms/static/js/spec/views/paged_container_spec.js @@ -1,6 +1,6 @@ define([ "jquery", "underscore", "js/common_helpers/ajax_helpers", "URI", "js/models/xblock_info", - "js/views/library_container", "js/views/paging_header", "js/views/paging_footer"], - function ($, _, AjaxHelpers, URI, XBlockInfo, PagedContainer, PagingContainer, PagingFooter) { + "js/views/paged_container", "js/views/paging_header", "js/views/paging_footer"], + function ($, _, AjaxHelpers, URI, XBlockInfo, PagedContainer, PagingHeader, PagingFooter) { var htmlResponseTpl = _.template('' + '
' diff --git a/cms/static/js/spec/views/pages/container_spec.js b/cms/static/js/spec/views/pages/container_spec.js index d5ec6938dc25..ce862aac7d7f 100644 --- a/cms/static/js/spec/views/pages/container_spec.js +++ b/cms/static/js/spec/views/pages/container_spec.js @@ -1,7 +1,7 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_helpers", "js/common_helpers/template_helpers", "js/spec_helpers/edit_helpers", - "js/views/pages/container", "js/models/xblock_info", "jquery.simulate"], - function ($, _, str, AjaxHelpers, TemplateHelpers, EditHelpers, ContainerPage, XBlockInfo) { + "js/views/pages/container", "js/views/pages/paged_container", "js/models/xblock_info"], + function ($, _, str, AjaxHelpers, TemplateHelpers, EditHelpers, ContainerPage, PagedContainerPage, XBlockInfo) { function parameterized_suite(label, global_page_options, fixtures) { describe(label + " ContainerPage", function () { @@ -13,7 +13,8 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel mockBadContainerXBlockHtml = readFixtures('mock/mock-bad-javascript-container-xblock.underscore'), mockBadXBlockContainerXBlockHtml = readFixtures('mock/mock-bad-xblock-container-xblock.underscore'), mockUpdatedContainerXBlockHtml = readFixtures('mock/mock-updated-container-xblock.underscore'), - mockXBlockEditorHtml = readFixtures('mock/mock-xblock-editor.underscore'); + mockXBlockEditorHtml = readFixtures('mock/mock-xblock-editor.underscore'), + PageClass = fixtures.page; beforeEach(function () { var newDisplayName = 'New Display Name'; @@ -62,7 +63,7 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel templates: EditHelpers.mockComponentTemplates, el: $('#content') }; - return new ContainerPage(_.extend(options || {}, global_page_options, default_options)); + return new PageClass(_.extend(options || {}, global_page_options, default_options)); }; renderContainerPage = function (test, html, options) { @@ -273,7 +274,7 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel }); describe("xblock operations", function () { - var getGroupElement, paginated, + var getGroupElement, paginated, getDeleteOffset, NUM_COMPONENTS_PER_GROUP = 3, GROUP_TO_TEST = "A", allComponentsInGroup = _.map( _.range(NUM_COMPONENTS_PER_GROUP), @@ -283,9 +284,13 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel ); paginated = function () { - return containerPage.enable_paging; + return containerPage instanceof PagedContainerPage; }; + getDeleteOffset = function () { + // Paginated containers will make an additional AJAX request. + return paginated() ? 3 : 2; + }; getGroupElement = function () { return containerPage.$("[data-locator='locator-group-" + GROUP_TO_TEST + "']"); @@ -316,8 +321,6 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel deleteComponent = function (componentIndex, requestOffset) { clickDelete(componentIndex); AjaxHelpers.respondWithJson(requests, {}); - - // second to last request contains given component's id (to delete the component) AjaxHelpers.expectJsonRequest(requests, 'DELETE', '/xblock/locator-component-' + GROUP_TO_TEST + (componentIndex + 1), null, requests.length - requestOffset); @@ -329,8 +332,7 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel deleteComponentWithSuccess = function (componentIndex) { var deleteOffset; - deleteOffset = paginated() ? 3 : 2; - + deleteOffset = getDeleteOffset(); deleteComponent(componentIndex, deleteOffset); // verify the new list of components within the group @@ -356,17 +358,12 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel }); it("can delete an xblock with broken JavaScript", function () { + var deleteOffset = getDeleteOffset(); renderContainerPage(this, mockBadContainerXBlockHtml); containerPage.$('.delete-button').first().click(); EditHelpers.confirmPrompt(promptSpy); AjaxHelpers.respondWithJson(requests, {}); - var deleteOffset; - if (paginated()) { - deleteOffset = 3; - } else { - deleteOffset = 2; - } // expect the second to last request to be a delete of the xblock AjaxHelpers.expectJsonRequest(requests, 'DELETE', '/xblock/locator-broken-javascript', null, requests.length - deleteOffset); @@ -528,7 +525,7 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel }); describe('Template Picker', function () { - var showTemplatePicker, verifyCreateHtmlComponent, call_count; + var showTemplatePicker, verifyCreateHtmlComponent; showTemplatePicker = function () { containerPage.$('.new-component .new-component-type a.multiple-templates')[0].click(); @@ -536,7 +533,6 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel verifyCreateHtmlComponent = function (test, templateIndex, expectedRequest) { var xblockCount; - // call_count = paginated() ? 18: 10; renderContainerPage(test, mockContainerXBlockHtml); showTemplatePicker(); xblockCount = containerPage.$('.studio-xblock-wrapper').length; @@ -568,12 +564,17 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel } parameterized_suite("Non paged", - { enable_paging: false }, - { initial: 'mock/mock-container-xblock.underscore', add_response: 'mock/mock-xblock.underscore' } + { }, + { + page: ContainerPage, + initial: 'mock/mock-container-xblock.underscore', + add_response: 'mock/mock-xblock.underscore' + } ); parameterized_suite("Paged", - { enable_paging: true, page_size: 42 }, + { page_size: 42 }, { + page: PagedContainerPage, initial: 'mock/mock-container-paged-xblock.underscore', add_response: 'mock/mock-xblock-paged.underscore' }); diff --git a/cms/static/js/views/container.js b/cms/static/js/views/container.js index ec89208b4435..a99993fe5da3 100644 --- a/cms/static/js/views/container.js +++ b/cms/static/js/views/container.js @@ -9,6 +9,8 @@ define(["jquery", "underscore", "js/views/xblock", "js/utils/module", "gettext", // child xblocks within the page. requestToken: "", + new_child_view: 'reorderable_container_child_preview', + xblockReady: function () { XBlockView.prototype.xblockReady.call(this); var reorderableClass, reorderableContainer, diff --git a/cms/static/js/views/library_container.js b/cms/static/js/views/library_container.js index 7c48e83cee8f..ea09c69c8929 100644 --- a/cms/static/js/views/library_container.js +++ b/cms/static/js/views/library_container.js @@ -1,6 +1,5 @@ -define(["jquery", "underscore", "js/views/paged_container", "js/utils/module", "gettext", "js/views/feedback_notification", - "js/views/paging_header", "js/views/paging_footer"], - function ($, _, PagedContainerView) { +define(["js/views/paged_container"], + function (PagedContainerView) { // To be extended with Library-specific features later. var LibraryContainerView = PagedContainerView; return LibraryContainerView; diff --git a/cms/static/js/views/paged_container.js b/cms/static/js/views/paged_container.js index cd7590156a17..a8cd7aec3242 100644 --- a/cms/static/js/views/paged_container.js +++ b/cms/static/js/views/paged_container.js @@ -5,9 +5,13 @@ define(["jquery", "underscore", "js/views/container", "js/utils/module", "gettex initialize: function(options){ var self = this; ContainerView.prototype.initialize.call(this); - this.page_size = this.options.page_size || 10; - this.page_reload_callback = options.page_reload_callback || function () {}; - // emulating Backbone.paginator interface + this.page_size = this.options.page_size; + // Reference to the page model + this.page = options.page; + // XBlocks are rendered via Django views and templates rather than underscore templates, and so don't + // have a Backbone model for us to manipulate in a backbone collection. Here, we emulate the interface + // of backbone.paginator so that we can use the Paging Header and Footer with this page. As a + // consequence, however, we have to manipulate its members manually. this.collection = { currentPage: 0, totalPages: 0, @@ -15,18 +19,23 @@ define(["jquery", "underscore", "js/views/container", "js/utils/module", "gettex sortDirection: "desc", start: 0, _size: 0, - - bind: function() {}, // no-op + // Paging header and footer expect this to be a Backbone model they can listen to for changes, but + // they cannot. Provide the bind function for them, but have it do nothing. + bind: function() {}, + // size() on backbone collections shows how many objects are in the collection, or in the case + // of paginator, on the current page. size: function() { return self.collection._size; } }; }, + new_child_view: 'container_child_preview', + render: function(options) { - var eff_options = options || {}; - eff_options.page_number = typeof eff_options.page_number !== "undefined" - ? eff_options.page_number + options = options || {}; + options.page_number = typeof options.page_number !== "undefined" + ? options.page_number : this.collection.currentPage; - return this.renderPage(eff_options); + return this.renderPage(options); }, renderPage: function(options){ @@ -43,16 +52,15 @@ define(["jquery", "underscore", "js/views/container", "js/utils/module", "gettex success: function(fragment) { self.handleXBlockFragment(fragment, options); self.processPaging({ requested_page: options.page_number }); - // This is expected to render the add xblock components menu. - self.page_reload_callback(self.$el) + self.page.renderAddXBlockComponents() } }); }, getRenderParameters: function(page_number) { return { - enable_paging: true, page_size: this.page_size, + enable_paging: true, page_number: page_number }; }, @@ -67,6 +75,8 @@ define(["jquery", "underscore", "js/views/container", "js/utils/module", "gettex }, processPaging: function(options){ + // We have the Django template sneak us the pagination information, + // and we load it from a div here. var $element = this.$el.find('.xblock-container-paging-parameters'), total = $element.data('total'), displayed = $element.data('displayed'), @@ -82,6 +92,8 @@ define(["jquery", "underscore", "js/views/container", "js/utils/module", "gettex }, processPagingHeaderAndFooter: function(){ + // Rendering the container view detaches the header and footer from the DOM. + // It's just as easy to recreate them as it is to try to shove them back into the tree. if (this.pagingHeader) this.pagingHeader.undelegateEvents(); if (this.pagingFooter) @@ -100,12 +112,6 @@ define(["jquery", "underscore", "js/views/container", "js/utils/module", "gettex this.pagingFooter.render(); }, - xblockReady: function () { - ContainerView.prototype.xblockReady.call(this); - - this.requestToken = this.$('div.xblock').first().data('request-token'); - }, - refresh: function(block_added) { if (block_added) { this.collection.totalCount += 1; @@ -150,7 +156,7 @@ define(["jquery", "underscore", "js/views/container", "js/utils/module", "gettex }, sortDisplayName: function() { - return "Date added"; // TODO add support for sorting + return gettext("Date added"); // TODO add support for sorting } }); diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 771e9afb1b89..406e6e9b0354 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -3,10 +3,10 @@ * This page allows the user to understand and manipulate the xblock and its children. */ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views/utils/view_utils", - "js/views/container", "js/views/library_container", "js/views/xblock", "js/views/components/add_xblock", "js/views/modals/edit_xblock", + "js/views/container", "js/views/xblock", "js/views/components/add_xblock", "js/views/modals/edit_xblock", "js/models/xblock_info", "js/views/xblock_string_field_editor", "js/views/pages/container_subviews", "js/views/unit_outline", "js/views/utils/xblock_utils"], - function ($, _, gettext, BasePage, ViewUtils, ContainerView, PagedContainerView, XBlockView, AddXBlockComponent, + function ($, _, gettext, BasePage, ViewUtils, ContainerView, XBlockView, AddXBlockComponent, EditXBlockModal, XBlockInfo, XBlockStringFieldEditor, ContainerSubviews, UnitOutlineView, XBlockUtils) { 'use strict'; @@ -25,12 +25,16 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views view: 'container_preview', + defaultViewClass: ContainerView, + + // Overridable by subclasses-- determines whether the XBlock component + // addition menu is added on initialization. You may set this to false + // if your subclass handles it. + components_on_init: true, + initialize: function(options) { BasePage.prototype.initialize.call(this, options); - this.enable_paging = options.enable_paging || false; - if (this.enable_paging) { - this.page_size = options.page_size || 10; - } + this.viewClass = options.viewClass || this.defaultViewClass; this.nameEditor = new XBlockStringFieldEditor({ el: this.$('.wrapper-xblock-field'), model: this.model @@ -75,28 +79,18 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views } }, - getXBlockView: function(){ - var self = this, - parameters = { - el: this.$('.wrapper-xblock'), - model: this.model, - view: this.view - }; - - if (this.enable_paging) { - parameters = _.extend(parameters, { - page_size: this.page_size, - page_reload_callback: function($element) { - self.renderAddXBlockComponents(); - } - }); - return new PagedContainerView(parameters); - } - else { - return new ContainerView(parameters); + getViewParameters: function () { + return { + el: this.$('.wrapper-xblock'), + model: this.model, + view: this.view } }, + getXBlockView: function(){ + return new this.viewClass(this.getViewParameters()); + }, + render: function(options) { var self = this, xblockView = this.xblockView, @@ -120,7 +114,7 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views xblockView.notifyRuntime('page-shown', self); // Render the add buttons. Paged containers should do this on their own. - if (!self.enable_paging) { + if (self.components_on_init) { // Render the add buttons self.renderAddXBlockComponents(); } @@ -277,7 +271,7 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views rootLocator = this.xblockView.model.id; if (xblockElement.length === 0 || xblockElement.data('locator') === rootLocator) { this.render({refresh: true, block_added: block_added}); - } else if (parentElement.hasClass('reorderable-container') || this.enable_paging) { + } else if (parentElement.hasClass('reorderable-container')) { this.refreshChildXBlock(xblockElement, block_added); } else { this.refreshXBlock(this.findXBlockElement(parentElement)); @@ -313,7 +307,7 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views }); temporaryView = new TemporaryXBlockView({ model: xblockInfo, - view: 'reorderable_container_child_preview', + view: self.xblockView.new_child_view, el: xblockElement }); return temporaryView.render({ diff --git a/cms/static/js/views/pages/paged_container.js b/cms/static/js/views/pages/paged_container.js new file mode 100644 index 000000000000..916bf3005e43 --- /dev/null +++ b/cms/static/js/views/pages/paged_container.js @@ -0,0 +1,36 @@ +/** + * PagedXBlockContainerPage is a variant of XBlockContainerPage that supports Pagination. + */ +define(["jquery", "underscore", "gettext", "js/views/pages/container", "js/views/paged_container"], + function ($, _, gettext, XBlockContainerPage, PagedContainerView) { + 'use strict'; + var PagedXBlockContainerPage = XBlockContainerPage.extend({ + + defaultViewClass: PagedContainerView, + components_on_init: false, + + initialize: function (options){ + this.page_size = options.page_size || 10; + XBlockContainerPage.prototype.initialize.call(this, options); + }, + + getViewParameters: function () { + return _.extend(XBlockContainerPage.prototype.getViewParameters.call(this), { + page_size: this.page_size, + page: this + }); + }, + + refreshXBlock: function(element, block_added) { + var xblockElement = this.findXBlockElement(element), + rootLocator = this.xblockView.model.id; + if (xblockElement.length === 0 || xblockElement.data('locator') === rootLocator) { + this.render({refresh: true, block_added: block_added}); + } else { + this.refreshChildXBlock(xblockElement, block_added); + } + } + + }); + return PagedXBlockContainerPage; + }); diff --git a/cms/static/js/views/paging_footer.js b/cms/static/js/views/paging_footer.js index 5a42c1d03c92..4ec3501d4482 100644 --- a/cms/static/js/views/paging_footer.js +++ b/cms/static/js/views/paging_footer.js @@ -44,6 +44,8 @@ define(["underscore", "js/views/baseview"], function(_, BaseView) { if (pageNumber <= 0) { pageNumber = false; } + // If we still have a page number by this point, + // and it's not the current page, load it. if (pageNumber && pageNumber !== currentPage) { view.setPage(pageNumber - 1); } diff --git a/cms/static/js/views/paging_mixin.js b/cms/static/js/views/paging_mixin.js index d2c1700e5d64..16d518f856d4 100644 --- a/cms/static/js/views/paging_mixin.js +++ b/cms/static/js/views/paging_mixin.js @@ -1,5 +1,5 @@ -define(["jquery", "underscore"], - function ($, _) { +define([], + function () { var PagedMixin = { setPage: function (page) { var self = this, diff --git a/cms/templates/library.html b/cms/templates/library.html index dc9baa5736c3..d367c333d270 100644 --- a/cms/templates/library.html +++ b/cms/templates/library.html @@ -25,7 +25,6 @@ ${component_templates | n}, ${json.dumps(xblock_info) | n}, { isUnitPage: false, - enable_paging: true, page_size: 10 } ); diff --git a/common/lib/xmodule/xmodule/library_root_xblock.py b/common/lib/xmodule/xmodule/library_root_xblock.py index 3118f9a25842..6a58cf1b1d18 100644 --- a/common/lib/xmodule/xmodule/library_root_xblock.py +++ b/common/lib/xmodule/xmodule/library_root_xblock.py @@ -50,8 +50,7 @@ def author_view(self, context): def render_children(self, context, fragment, can_reorder=False, can_add=False): # pylint: disable=unused-argument """ - Renders the children of the module with HTML appropriate for Studio. If can_reorder is True, - then the children will be rendered to support drag and drop. + Renders the children of the module with HTML appropriate for Studio. Reordering is not supported. """ contents = [] @@ -77,7 +76,7 @@ def render_children(self, context, fragment, can_reorder=False, can_add=False): contents.append({ 'id': unicode(child.location), - 'content': rendered_child.content + 'content': rendered_child.content, }) fragment.add_content( diff --git a/common/test/acceptance/pages/studio/library.py b/common/test/acceptance/pages/studio/library.py index 5572b1a91e43..64f93f21167e 100644 --- a/common/test/acceptance/pages/studio/library.py +++ b/common/test/acceptance/pages/studio/library.py @@ -3,14 +3,14 @@ """ from bok_choy.page_object import PageObject -from selenium.webdriver.common.keys import Keys +from ...pages.studio.pagination import PaginatedMixin from .container import XBlockWrapper from ...tests.helpers import disable_animations from .utils import confirm_prompt, wait_for_notification from . import BASE_URL -class LibraryPage(PageObject): +class LibraryPage(PageObject, PaginatedMixin): """ Library page in Studio """ @@ -75,58 +75,6 @@ def click_delete_button(self, xblock_id, confirm=True): confirm_prompt(self) # this will also wait_for_notification() self.wait_for_ajax() - def nav_disabled(self, position, arrows=('next', 'previous')): - """ - Verifies that pagination nav is disabled. Position can be 'top' or 'bottom'. - - To specify a specific arrow, pass an iterable with a single element, 'next' or 'previous'. - """ - return all([ - self.q(css='nav.%s * a.%s-page-link.is-disabled' % (position, arrow)) - for arrow in arrows - ]) - - def move_back(self, position): - """ - Clicks one of the forward nav buttons. Position can be 'top' or 'bottom'. - """ - self.q(css='nav.%s * a.previous-page-link' % position)[0].click() - self.wait_until_ready() - - def move_forward(self, position): - """ - Clicks one of the forward nav buttons. Position can be 'top' or 'bottom'. - """ - self.q(css='nav.%s * a.next-page-link' % position)[0].click() - self.wait_until_ready() - - def revisit(self): - """ - Visit the page's URL, instead of refreshing, so that a new state is created. - """ - self.browser.get(self.browser.current_url) - self.wait_until_ready() - - def go_to_page(self, number): - """ - Enter a number into the page number input field, and then try to navigate to it. - """ - page_input = self.q(css="#page-number-input")[0] - page_input.click() - page_input.send_keys(str(number)) - page_input.send_keys(Keys.RETURN) - self.wait_until_ready() - - def check_page_unchanged(self, first_block_name): - """ - Used to make sure that a page has not transitioned after a bogus number is given. - """ - if not self.xblocks[0].name == first_block_name: - return False - if not self.q(css='#page-number-input')[0].get_attribute('value') == '': - return False - return True - def _get_xblocks(self): """ Create an XBlockWrapper for each XBlock div found on the page. diff --git a/common/test/acceptance/pages/studio/pagination.py b/common/test/acceptance/pages/studio/pagination.py new file mode 100644 index 000000000000..a976149c37dd --- /dev/null +++ b/common/test/acceptance/pages/studio/pagination.py @@ -0,0 +1,62 @@ +""" +Mixin to include for Paginated container pages +""" +from selenium.webdriver.common.keys import Keys + + +class PaginatedMixin(object): + """ + Mixin class used for paginated page tests. + """ + def nav_disabled(self, position, arrows=('next', 'previous')): + """ + Verifies that pagination nav is disabled. Position can be 'top' or 'bottom'. + + `top` is the header, `bottom` is the footer. + + To specify a specific arrow, pass an iterable with a single element, 'next' or 'previous'. + """ + return all([ + self.q(css='nav.%s * a.%s-page-link.is-disabled' % (position, arrow)) + for arrow in arrows + ]) + + def move_back(self, position): + """ + Clicks one of the forward nav buttons. Position can be 'top' or 'bottom'. + """ + self.q(css='nav.%s * a.previous-page-link' % position)[0].click() + self.wait_until_ready() + + def move_forward(self, position): + """ + Clicks one of the forward nav buttons. Position can be 'top' or 'bottom'. + """ + self.q(css='nav.%s * a.next-page-link' % position)[0].click() + self.wait_until_ready() + + def go_to_page(self, number): + """ + Enter a number into the page number input field, and then try to navigate to it. + """ + page_input = self.q(css="#page-number-input")[0] + page_input.click() + page_input.send_keys(str(number)) + page_input.send_keys(Keys.RETURN) + self.wait_until_ready() + + def get_page_number(self): + """ + Returns the page number as the page represents it, in string form. + """ + return self.q(css="span.current-page")[0].get_attribute('innerHTML') + + def check_page_unchanged(self, first_block_name): + """ + Used to make sure that a page has not transitioned after a bogus number is given. + """ + if not self.xblocks[0].name == first_block_name: + return False + if not self.q(css='#page-number-input')[0].get_attribute('value') == '': + return False + return True diff --git a/common/test/acceptance/tests/studio/test_studio_library.py b/common/test/acceptance/tests/studio/test_studio_library.py index 5529f3603293..491c9093d0fc 100644 --- a/common/test/acceptance/tests/studio/test_studio_library.py +++ b/common/test/acceptance/tests/studio/test_studio_library.py @@ -4,6 +4,7 @@ from ddt import ddt, data from .base_studio_test import StudioLibraryTest +from ...fixtures.course import XBlockFixtureDesc from ...pages.studio.utils import add_component from ...pages.studio.library import LibraryPage @@ -137,109 +138,64 @@ def test_nav_present_but_disabled(self, position): Scenario: Ensure that the navigation buttons aren't active when there aren't enough XBlocks. Given that I have a library in Studio with no XBlocks The Navigation buttons should be disabled. - When I add 5 multiple Choice XBlocks + When I add a multiple choice problem The Navigation buttons should be disabled. """ self.assertEqual(len(self.lib_page.xblocks), 0) self.assertTrue(self.lib_page.nav_disabled(position)) - for _ in range(0, 5): - add_component(self.lib_page, "problem", "Multiple Choice") + add_component(self.lib_page, "problem", "Multiple Choice") self.assertTrue(self.lib_page.nav_disabled(position)) - @data('top', 'bottom') - def test_nav_buttons(self, position): + +@ddt +class LibraryNavigationTest(StudioLibraryTest): + """ + Test common Navigation actions + """ + def setUp(self): # pylint: disable=arguments-differ """ - Scenario: Ensure that the navigation buttons work. - Given that I have a library in Studio with no XBlocks - And I create 10 Multiple Choice XBlocks - And I create 10 Checkbox XBlocks - And I create 10 Dropdown XBlocks - And I revisit the page - The previous button should be disabled. - The first XBlock should be a Multiple Choice XBlock - Then if I hit the next button - The first XBlock should be a Checkboxes XBlock - Then if I hit the next button - The first XBlock should be a Dropdown XBlock - And the next button should be disabled - Then if I hit the previous button - The first XBlock should be an Checkboxes XBlock - Then if I hit the previous button - The first XBlock should be a Multipe Choice XBlock - And the previous button should be disabled + Ensure a library exists and navigate to the library edit page. """ - self.assertEqual(len(self.lib_page.xblocks), 0) - block_types = [('problem', 'Multiple Choice'), ('problem', 'Checkboxes'), ('problem', 'Dropdown')] - for block_type in block_types: - for _ in range(0, 10): - add_component(self.lib_page, *block_type) - - # Don't refresh, as that may contain additional state. - self.lib_page.revisit() - - # Check forward navigation - self.assertTrue(self.lib_page.nav_disabled(position, ['previous'])) - self.assertEqual(self.lib_page.xblocks[0].name, 'Multiple Choice') - self.lib_page.move_forward(position) - self.assertEqual(self.lib_page.xblocks[0].name, 'Checkboxes') - self.lib_page.move_forward(position) - self.assertEqual(self.lib_page.xblocks[0].name, 'Dropdown') - self.lib_page.nav_disabled(position, ['next']) + super(LibraryNavigationTest, self).setUp(is_staff=True) + self.lib_page = LibraryPage(self.browser, self.library_key) + self.lib_page.visit() + self.lib_page.wait_until_ready() - # Check backward navigation - self.lib_page.move_back(position) - self.assertEqual(self.lib_page.xblocks[0].name, 'Checkboxes') - self.lib_page.move_back(position) - self.assertEqual(self.lib_page.xblocks[0].name, 'Multiple Choice') - self.assertTrue(self.lib_page.nav_disabled(position, ['previous'])) + def populate_library_fixture(self, library_fixture): + """ + Create four pages worth of XBlocks, and offset by one so each is named + after the number they should be in line by the user's perception. + """ + # pylint: disable=attribute-defined-outside-init + self.blocks = [XBlockFixtureDesc('html', str(i)) for i in xrange(1, 41)] + library_fixture.add_children(*self.blocks) def test_arbitrary_page_selection(self): """ Scenario: I can pick a specific page number of a Library at will. - Given that I have a library in Studio with no XBlocks - And I create 10 Multiple Choice XBlocks - And I create 10 Checkboxes XBlocks - And I create 10 Dropdown XBlocks - And I create 10 Numerical Input XBlocks - And I revisit the page + Given that I have a library in Studio with 40 XBlocks When I go to the 3rd page - The first XBlock should be a Dropdown XBlock + The first XBlock should be the 21st XBlock When I go to the 4th Page - The first XBlock should be a Numerical Input XBlock + The first XBlock should be the 31st XBlock When I go to the 1st page - The first XBlock should be a Multiple Choice XBlock + The first XBlock should be the 1st XBlock When I go to the 2nd page - The first XBlock should be a Checkboxes XBlock + The first XBlock should be the 11th XBlock """ - self.assertEqual(len(self.lib_page.xblocks), 0) - block_types = [ - ('problem', 'Multiple Choice'), ('problem', 'Checkboxes'), ('problem', 'Dropdown'), - ('problem', 'Numerical Input'), - ] - for block_type in block_types: - for _ in range(0, 10): - add_component(self.lib_page, *block_type) - - # Don't refresh, as that may contain additional state. - self.lib_page.revisit() self.lib_page.go_to_page(3) - self.assertEqual(self.lib_page.xblocks[0].name, 'Dropdown') + self.assertEqual(self.lib_page.xblocks[0].name, '21') self.lib_page.go_to_page(4) - self.assertEqual(self.lib_page.xblocks[0].name, 'Numerical Input') + self.assertEqual(self.lib_page.xblocks[0].name, '31') self.lib_page.go_to_page(1) - self.assertEqual(self.lib_page.xblocks[0].name, 'Multiple Choice') + self.assertEqual(self.lib_page.xblocks[0].name, '1') self.lib_page.go_to_page(2) - self.assertEqual(self.lib_page.xblocks[0].name, 'Checkboxes') + self.assertEqual(self.lib_page.xblocks[0].name, '11') def test_bogus_page_selection(self): """ Scenario: I can't pick a nonsense page number of a Library - Given that I have a library in Studio with no XBlocks - And I create 10 Multiple Choice XBlocks - And I create 10 Checkboxes XBlocks - And I create 10 Dropdown XBlocks - And I create 10 Numerical Input XBlocks - And I revisit the page + Given that I have a library in Studio with 40 XBlocks When I attempt to go to the 'a'th page The input field will be cleared and no change of XBlocks will be made When I attempt to visit the 5th page @@ -249,22 +205,104 @@ def test_bogus_page_selection(self): When I attempt to visit the 0th page The input field will be cleared and no change of XBlocks will be made """ - self.assertEqual(len(self.lib_page.xblocks), 0) - block_types = [ - ('problem', 'Multiple Choice'), ('problem', 'Checkboxes'), ('problem', 'Dropdown'), - ('problem', 'Numerical Input'), - ] - for block_type in block_types: - for _ in range(0, 10): - add_component(self.lib_page, *block_type) - - self.lib_page.revisit() - self.assertEqual(self.lib_page.xblocks[0].name, 'Multiple Choice') + self.assertEqual(self.lib_page.xblocks[0].name, '1') self.lib_page.go_to_page('a') - self.assertTrue(self.lib_page.check_page_unchanged('Multiple Choice')) + self.assertTrue(self.lib_page.check_page_unchanged('1')) self.lib_page.go_to_page(-1) - self.assertTrue(self.lib_page.check_page_unchanged('Multiple Choice')) + self.assertTrue(self.lib_page.check_page_unchanged('1')) self.lib_page.go_to_page(5) - self.assertTrue(self.lib_page.check_page_unchanged('Multiple Choice')) + self.assertTrue(self.lib_page.check_page_unchanged('1')) self.lib_page.go_to_page(0) - self.assertTrue(self.lib_page.check_page_unchanged('Multiple Choice')) + self.assertTrue(self.lib_page.check_page_unchanged('1')) + + @data('top', 'bottom') + def test_nav_buttons(self, position): + """ + Scenario: Ensure that the navigation buttons work. + Given that I have a library in Studio with 40 XBlocks + The previous button should be disabled. + The first XBlock should be the 1st XBlock + Then if I hit the next button + The first XBlock should be the 11th XBlock + Then if I hit the next button + The first XBlock should be the 21st XBlock + Then if I hit the next button + The first XBlock should be the 31st XBlock + And the next button should be disabled + Then if I hit the previous button + The first XBlock should be the 21st XBlock + Then if I hit the previous button + The first XBlock should be the 11th XBlock + Then if I hit the previous button + The first XBlock should be the 1st XBlock + And the previous button should be disabled + """ + # Check forward navigation + self.assertTrue(self.lib_page.nav_disabled(position, ['previous'])) + self.assertEqual(self.lib_page.xblocks[0].name, '1') + self.lib_page.move_forward(position) + self.assertEqual(self.lib_page.xblocks[0].name, '11') + self.lib_page.move_forward(position) + self.assertEqual(self.lib_page.xblocks[0].name, '21') + self.lib_page.move_forward(position) + self.assertEqual(self.lib_page.xblocks[0].name, '31') + self.lib_page.nav_disabled(position, ['next']) + + # Check backward navigation + self.lib_page.move_back(position) + self.assertEqual(self.lib_page.xblocks[0].name, '21') + self.lib_page.move_back(position) + self.assertEqual(self.lib_page.xblocks[0].name, '11') + self.lib_page.move_back(position) + self.assertEqual(self.lib_page.xblocks[0].name, '1') + self.assertTrue(self.lib_page.nav_disabled(position, ['previous'])) + + def test_library_pagination(self): + """ + Scenario: Ensure that adding several XBlocks to a library results in pagination. + Given that I have a library in Studio with 40 XBlocks + Then 10 are displayed + And the first XBlock will be the 1st one + And I'm on the 1st page + When I add 1 Multiple Choice XBlock + Then 1 XBlock will be displayed + And I'm on the 5th page + The first XBlock will be the newest one + When I delete that XBlock + Then 10 are displayed + And I'm on the 4th page + And the first XBlock is the 31st one + And the last XBlock is the 40th one. + """ + self.assertEqual(len(self.lib_page.xblocks), 10) + self.assertEqual(self.lib_page.get_page_number(), '1') + self.assertEqual(self.lib_page.xblocks[0].name, '1') + add_component(self.lib_page, "problem", "Multiple Choice") + self.assertEqual(len(self.lib_page.xblocks), 1) + self.assertEqual(self.lib_page.get_page_number(), '5') + self.assertEqual(self.lib_page.xblocks[0].name, "Multiple Choice") + self.lib_page.click_delete_button(self.lib_page.xblocks[0].locator) + self.assertEqual(len(self.lib_page.xblocks), 10) + self.assertEqual(self.lib_page.get_page_number(), '4') + self.assertEqual(self.lib_page.xblocks[0].name, '31') + self.assertEqual(self.lib_page.xblocks[-1].name, '40') + + def test_delete_shifts_blocks(self): + """ + Scenario: Ensure that removing an XBlock shifts other blocks back. + Given that I have a library in Studio with 40 XBlocks + Then 10 are displayed + And I will be on the first page + When I delete the third XBlock + There will be 10 displayed + And the first XBlock will be the first one + And the last XBlock will be the 11th one + And I will be on the first page + """ + self.assertEqual(len(self.lib_page.xblocks), 10) + self.assertEqual(self.lib_page.get_page_number(), '1') + self.lib_page.click_delete_button(self.lib_page.xblocks[2].locator, confirm=True) + self.assertEqual(len(self.lib_page.xblocks), 10) + self.assertEqual(self.lib_page.xblocks[0].name, '1') + self.assertEqual(self.lib_page.xblocks[-1].name, '11') + self.assertEqual(self.lib_page.get_page_number(), '1') From 219eeb6a6d3e7090d22812f647615151d6f3e13f Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 28 Oct 2014 23:10:40 -0700 Subject: [PATCH 08/23] Library Content XModule --- cms/envs/common.py | 1 + common/lib/xmodule/setup.py | 1 + .../xmodule/xmodule/library_content_module.py | 441 ++++++++++++++++++ .../xmodule/public/js/library_content_edit.js | 24 + lms/djangoapps/courseware/models.py | 4 + lms/templates/library-block-author-view.html | 17 + lms/templates/staff_problem_info.html | 2 +- 7 files changed, 489 insertions(+), 1 deletion(-) create mode 100644 common/lib/xmodule/xmodule/library_content_module.py create mode 100644 common/lib/xmodule/xmodule/public/js/library_content_edit.js create mode 100644 lms/templates/library-block-author-view.html diff --git a/cms/envs/common.py b/cms/envs/common.py index 397620ef864b..c81a392134ce 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -743,6 +743,7 @@ 'word_cloud', 'graphical_slider_tool', 'lti', + 'library_content', # XBlocks from pmitros repos are prototypes. They should not be used # except for edX Learning Sciences experiments on edge.edx.org without # further work to make them robust, maintainable, finalize data formats, diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index f0721e91a47f..f2b548efe9c0 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -11,6 +11,7 @@ "discuss = xmodule.backcompat_module:TranslateCustomTagDescriptor", "html = xmodule.html_module:HtmlDescriptor", "image = xmodule.backcompat_module:TranslateCustomTagDescriptor", + "library_content = xmodule.library_content_module:LibraryContentDescriptor", "error = xmodule.error_module:ErrorDescriptor", "peergrading = xmodule.peer_grading_module:PeerGradingDescriptor", "poll_question = xmodule.poll_module:PollDescriptor", diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py new file mode 100644 index 000000000000..092bc2a93117 --- /dev/null +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -0,0 +1,441 @@ +""" +LibraryContent: The XBlock used to include blocks from a library in a course. +""" +from bson.objectid import ObjectId +from collections import namedtuple +from copy import copy +import hashlib +from .mako_module import MakoModuleDescriptor +from opaque_keys.edx.locator import LibraryLocator +import random +from webob import Response +from xblock.core import XBlock +from xblock.fields import Scope, String, List, Integer, Boolean +from xblock.fragment import Fragment +from xmodule.modulestore.exceptions import ItemNotFoundError +from xmodule.x_module import XModule, STUDENT_VIEW +from xmodule.studio_editable import StudioEditableModule, StudioEditableDescriptor +from .xml_module import XmlDescriptor +from pkg_resources import resource_string + +# Make '_' a no-op so we can scrape strings +_ = lambda text: text + + +def enum(**enums): + """ enum helper in lieu of enum34 """ + return type('Enum', (), enums) + + +class LibraryVersionReference(namedtuple("LibraryVersionReference", "library_id version")): + """ + A reference to a specific library, with an optional version. + The version is used to find out when the LibraryContentXBlock was last + updated with the latest content from the library. + + library_id is a LibraryLocator + version is an ObjectId or None + """ + def __new__(cls, library_id, version=None): + # pylint: disable=super-on-old-class + if not isinstance(library_id, LibraryLocator): + library_id = LibraryLocator.from_string(library_id) + if library_id.version_guid: + assert (version is None) or (version == library_id.version_guid) + if not version: + version = library_id.version_guid + library_id = library_id.for_version(None) + if version and not isinstance(version, ObjectId): + version = ObjectId(version) + return super(LibraryVersionReference, cls).__new__(cls, library_id, version) + + @staticmethod + def from_json(value): + """ + Implement from_json to convert from JSON + """ + return LibraryVersionReference(*value) + + def to_json(self): + """ + Implement to_json to convert value to JSON + """ + # TODO: Is there anyway for an xblock to *store* an ObjectId as + # part of the List() field value? + return [unicode(self.library_id), unicode(self.version) if self.version else None] # pylint: disable=no-member + + +class LibraryList(List): + """ + Special List class for listing references to content libraries. + Is simply a list of LibraryVersionReference tuples. + """ + def from_json(self, values): + """ + Implement from_json to convert from JSON. + + values might be a list of lists, or a list of strings + Normally the runtime gives us: + [[u'library-v1:ProblemX+PR0B', '5436ffec56c02c13806a4c1b'], ...] + But the studio editor gives us: + [u'library-v1:ProblemX+PR0B,5436ffec56c02c13806a4c1b', ...] + """ + def parse(val): + """ Convert this list entry from its JSON representation """ + if isinstance(val, basestring): + val = val.strip(' []') + parts = val.rsplit(',', 1) + val = [parts[0], parts[1] if len(parts) > 1 else None] + return LibraryVersionReference.from_json(val) + return [parse(v) for v in values] + + def to_json(self, values): + """ + Implement to_json to convert value to JSON + """ + return [lvr.to_json() for lvr in values] + + +class LibraryContentFields(object): + """ + Fields for the LibraryContentModule. + + Separated out for now because they need to be added to the module and the + descriptor. + """ + # Please note the display_name of each field below is used in + # common/test/acceptance/pages/studio/overview.py:StudioLibraryContentXBlockEditModal + # to locate input elements - keep synchronized + display_name = String( + display_name=_("Display Name"), + help=_("Display name for this module"), + default="Library Content", + scope=Scope.settings, + ) + source_libraries = LibraryList( + display_name=_("Libraries"), + help=_("Enter a library ID for each library from which you want to draw content."), + default=[], + scope=Scope.settings, + ) + mode = String( + help=_("Determines how content is drawn from the library"), + default="random", + values=[ + {"display_name": _("Choose n at random"), "value": "random"} + # Future addition: Choose a new random set of n every time the student refreshes the block, for self tests + # Future addition: manually selected blocks + ], + scope=Scope.settings, + ) + max_count = Integer( + display_name=_("Count"), + help=_("Enter the number of components to display to each student."), + default=1, + scope=Scope.settings, + ) + filters = String(default="") # TBD + has_score = Boolean( + display_name=_("Scored"), + help=_("Set this value to True if this module is either a graded assignment or a practice problem."), + default=False, + scope=Scope.settings, + ) + selected = List( + # This is a list of (block_type, block_id) tuples used to record which random/first set of matching blocks was selected per user + default=[], + scope=Scope.user_state, + ) + has_children = True + + +def _get_library(modulestore, library_key): + """ + Given a library key like "library-v1:ProblemX+PR0B", return the + 'library' XBlock with meta-information about the library. + + Returns None on error. + """ + if not isinstance(library_key, LibraryLocator): + library_key = LibraryLocator.from_string(library_key) + assert library_key.version_guid is None + + # TODO: Is this too tightly coupled to split? May need to abstract this into a service + # provided by the CMS runtime. + try: + library = modulestore.get_library(library_key, remove_version=False) + except ItemNotFoundError: + return None + # We need to know the library's version so ensure it's set in library.location.library_key.version_guid + assert library.location.library_key.version_guid is not None + return library + + +#pylint: disable=abstract-method +class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule): + """ + An XBlock whose children are chosen dynamically from a content library. + Can be used to create randomized assessments among other things. + + Note: technically, all matching blocks from the content library are added + as children of this block, but only a subset of those children are shown to + any particular student. + """ + def selected_children(self): + """ + Returns a set() of block_ids indicating which of the possible children + have been selected to display to the current user. + + This reads and updates the "selected" field, which has user_state scope. + + Note: self.selected and the return value contain block_ids. To get + actual BlockUsageLocators, it is necessary to use self.children, + because the block_ids alone do not specify the block type. + """ + if hasattr(self, "_selected_set"): + # Already done: + return self._selected_set # pylint: disable=access-member-before-definition + # Determine which of our children we will show: + selected = set(tuple(k) for k in self.selected) # set of (block_type, block_id) tuples + valid_block_keys = set([(c.block_type, c.block_id) for c in self.children]) # pylint: disable=no-member + # Remove any selected blocks that are no longer valid: + selected -= (selected - valid_block_keys) + # If max_count has been decreased, we may have to drop some previously selected blocks: + while len(selected) > self.max_count: + selected.pop() + # Do we have enough blocks now? + num_to_add = self.max_count - len(selected) + if num_to_add > 0: + # We need to select [more] blocks to display to this user: + if self.mode == "random": + pool = valid_block_keys - selected + num_to_add = min(len(pool), num_to_add) + selected |= set(random.sample(pool, num_to_add)) + # We now have the correct n random children to show for this user. + else: + raise NotImplementedError("Unsupported mode.") + # Save our selections to the user state, to ensure consistency: + self.selected = list(selected) # TODO: this doesn't save from the LMS "Progress" page. + # Cache the results + self._selected_set = selected # pylint: disable=attribute-defined-outside-init + return selected + + def _get_selected_child_blocks(self): + """ + Generator returning XBlock instances of the children selected for the + current user. + """ + for block_type, block_id in self.selected_children(): + yield self.runtime.get_block(self.location.course_key.make_usage_key(block_type, block_id)) + + def student_view(self, context): + fragment = Fragment() + contents = [] + child_context = {} if not context else copy(context) + + for child in self._get_selected_child_blocks(): + for displayable in child.displayable_items(): + rendered_child = displayable.render(STUDENT_VIEW, child_context) + fragment.add_frag_resources(rendered_child) + contents.append({ + 'id': displayable.location.to_deprecated_string(), + 'content': rendered_child.content + }) + + fragment.add_content(self.system.render_template('vert_module.html', { + 'items': contents, + 'xblock_context': context, + })) + return fragment + + def author_view(self, context): + """ + Renders the Studio views. + Normal studio view: displays library status and has an "Update" button. + Studio container view: displays a preview of all possible children. + """ + fragment = Fragment() + root_xblock = context.get('root_xblock') + is_root = root_xblock and root_xblock.location == self.location + + if is_root: + # User has clicked the "View" link. Show a preview of all possible children: + if self.children: # pylint: disable=no-member + self.render_children(context, fragment, can_reorder=False, can_add=False) + else: + fragment.add_content(u'

{}

'.format( + _('No matching content found in library, no library configured, or not yet loaded from library.') + )) + else: + # When shown on a unit page, don't show any sort of preview - just the status of this block. + LibraryStatus = enum( # pylint: disable=invalid-name + NONE=0, # no library configured + INVALID=1, # invalid configuration or library has been deleted/corrupted + OK=2, # library configured correctly and should be working fine + ) + UpdateStatus = enum( # pylint: disable=invalid-name + CANNOT=0, # Cannot update - library is not set, invalid, deleted, etc. + NEEDED=1, # An update is needed - prompt the user to update + UP_TO_DATE=2, # No update necessary - library is up to date + ) + library_names = [] + library_status = LibraryStatus.OK + update_status = UpdateStatus.UP_TO_DATE + if self.source_libraries: + for library_key, version in self.source_libraries: + library = _get_library(self.runtime.descriptor_runtime.modulestore, library_key) + if library is None: + library_status = LibraryStatus.INVALID + update_status = UpdateStatus.CANNOT + break + library_names.append(library.display_name) + latest_version = library.location.library_key.version_guid + if version is None or version != latest_version: + update_status = UpdateStatus.NEEDED + # else library is up to date. + else: + library_status = LibraryStatus.NONE + update_status = UpdateStatus.CANNOT + fragment.add_content(self.system.render_template('library-block-author-view.html', { + 'library_status': library_status, + 'LibraryStatus': LibraryStatus, + 'update_status': update_status, + 'UpdateStatus': UpdateStatus, + 'library_names': library_names, + 'max_count': self.max_count, + 'mode': self.mode, + 'num_children': len(self.children), # pylint: disable=no-member + })) + fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/library_content_edit.js')) + fragment.initialize_js('LibraryContentAuthorView') + return fragment + + def get_child_descriptors(self): + """ + Return only the subset of our children relevant to the current student. + """ + return list(self._get_selected_child_blocks()) + + +@XBlock.wants('user') +class LibraryContentDescriptor(LibraryContentFields, MakoModuleDescriptor, XmlDescriptor, StudioEditableDescriptor): + """ + Descriptor class for LibraryContentModule XBlock. + """ + module_class = LibraryContentModule + mako_template = 'widgets/metadata-edit.html' + js = {'coffee': [resource_string(__name__, 'js/src/vertical/edit.coffee')]} + js_module_name = "VerticalDescriptor" + + @XBlock.handler + def refresh_children(self, request, suffix): # pylint: disable=unused-argument + """ + Refresh children: + This method is to be used when any of the libraries that this block + references have been updated. It will re-fetch all matching blocks from + the libraries, and copy them as children of this block. The children + will be given new block_ids, but the definition ID used should be the + exact same definition ID used in the library. + + This method will update this block's 'source_libraries' field to store + the version number of the libraries used, so we easily determine if + this block is up to date or not. + """ + user_id = self.runtime.service(self, 'user').user_id + root_children = [] + + store = self.system.modulestore + with store.bulk_operations(self.location.course_key): + # Currently, ALL children are essentially deleted and then re-added + # in a way that preserves their block_ids (and thus should preserve + # student data, grades, analytics, etc.) + # Once course-level field overrides are implemented, this will + # change to a more conservative implementation. + + # First, delete all our existing children to avoid block_id conflicts when we add them: + for child in self.children: # pylint: disable=access-member-before-definition + store.delete_item(child, user_id) + + # Now add all matching children, and record the library version we use: + new_libraries = [] + for library_key, old_version in self.source_libraries: # pylint: disable=unused-variable + library = _get_library(self.system.modulestore, library_key) # pylint: disable=protected-access + + def copy_children_recursively(from_block): + """ + Internal method to copy blocks from the library recursively + """ + new_children = [] + for child_key in from_block.children: + child = store.get_item(child_key, depth=9) + # We compute a block_id for each matching child block found in the library. + # block_ids are unique within any branch, but are not unique per-course or globally. + # We need our block_ids to be consistent when content in the library is updated, so + # we compute block_id as a hash of three pieces of data: + unique_data = "{}:{}:{}".format( + self.location.block_id, # Must not clash with other usages of the same library in this course + unicode(library_key.for_version(None)).encode("utf-8"), # The block ID below is only unique within a library, so we need this too + child_key.block_id, # Child block ID. Should not change even if the block is edited. + ) + child_block_id = hashlib.sha1(unique_data).hexdigest()[:20] + fields = {} + for field in child.fields.itervalues(): + if field.scope == Scope.settings and field.is_set_on(child): + fields[field.name] = field.read_from(child) + if child.has_children: + fields['children'] = copy_children_recursively(from_block=child) + new_child_info = store.create_item( + user_id, + self.location.course_key, + child_key.block_type, + block_id=child_block_id, + definition_locator=child.definition_locator, + runtime=self.system, + fields=fields, + ) + new_children.append(new_child_info.location) + return new_children + root_children.extend(copy_children_recursively(from_block=library)) + new_libraries.append(LibraryVersionReference(library_key, library.location.library_key.version_guid)) + self.source_libraries = new_libraries + self.children = root_children # pylint: disable=attribute-defined-outside-init + self.system.modulestore.update_item(self, user_id) + return Response() + + def has_dynamic_children(self): + """ + Inform the runtime that our children vary per-user. + See get_child_descriptors() above + """ + return True + + def get_content_titles(self): + """ + Returns list of friendly titles for our selected children only; without + thi, all possible children's titles would be seen in the sequence bar in + the LMS. + + This overwrites the get_content_titles method included in x_module by default. + """ + titles = [] + for child in self._xmodule.get_child_descriptors(): + titles.extend(child.get_content_titles()) + return titles + + @classmethod + def definition_from_xml(cls, xml_object, system): + """ XML support not yet implemented. """ + raise NotImplementedError + + def definition_to_xml(self, resource_fs): + """ XML support not yet implemented. """ + raise NotImplementedError + + @classmethod + def from_xml(cls, xml_data, system, id_generator): + """ XML support not yet implemented. """ + raise NotImplementedError + + def export_to_xml(self, resource_fs): + """ XML support not yet implemented. """ + raise NotImplementedError diff --git a/common/lib/xmodule/xmodule/public/js/library_content_edit.js b/common/lib/xmodule/xmodule/public/js/library_content_edit.js new file mode 100644 index 000000000000..9a84a214049c --- /dev/null +++ b/common/lib/xmodule/xmodule/public/js/library_content_edit.js @@ -0,0 +1,24 @@ +/* JavaScript for editing operations that can be done on LibraryContentXBlock */ +window.LibraryContentAuthorView = function (runtime, element) { + $(element).find('.library-update-btn').on('click', function(e) { + e.preventDefault(); + // Update the XBlock with the latest matching content from the library: + runtime.notify('save', { + state: 'start', + element: element, + message: gettext('Updating with latest library content') + }); + $.post(runtime.handlerUrl(element, 'refresh_children')).done(function() { + runtime.notify('save', { + state: 'end', + element: element + }); + // runtime.refreshXBlock(element); + // The above does not work, because this XBlock's runtime has no reference + // to the page (XBlockContainerPage). Only the Vertical XBlock's runtime has + // a reference to the page, and we have no way of getting a reference to it. + // So instead we: + location.reload(); + }); + }); +}; diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index 56818d4e2eea..d1f1f45b89ed 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -32,6 +32,10 @@ class StudentModule(models.Model): MODULE_TYPES = (('problem', 'problem'), ('video', 'video'), ('html', 'html'), + ('course', 'course'), + ('chapter', 'Section'), + ('sequential', 'Subsection'), + ('library_content', 'Library Content'), ) ## These three are the key for the object module_type = models.CharField(max_length=32, choices=MODULE_TYPES, default='problem', db_index=True) diff --git a/lms/templates/library-block-author-view.html b/lms/templates/library-block-author-view.html new file mode 100644 index 000000000000..521946a903db --- /dev/null +++ b/lms/templates/library-block-author-view.html @@ -0,0 +1,17 @@ +<%! +from django.utils.translation import ugettext as _ +%> +
+ % if library_status == LibraryStatus.OK: +

${_('This component will be replaced by {mode} {max_count} components from the {num_children} matching components from {lib_names}.').format(mode=mode, max_count=max_count, num_children=num_children, lib_names=', '.join(library_names))}

+ % if update_status == UpdateStatus.NEEDED: +

${_('This component is out of date.')} ↻ ${_('Update now with latest components from the library')}

+ % elif update_status == UpdateStatus.UP_TO_DATE: +

${_(u'✓ Up to date.')}

+ % endif + % elif library_status == LibraryStatus.NONE: +

${_('No library or filters configured. Press "Edit" to configure.')}

+ % else: +

${_('Library is invalid, corrupt, or has been deleted.')}

+ % endif +
diff --git a/lms/templates/staff_problem_info.html b/lms/templates/staff_problem_info.html index 75d2789d7c6e..f486bfc6f65f 100644 --- a/lms/templates/staff_problem_info.html +++ b/lms/templates/staff_problem_info.html @@ -4,7 +4,7 @@ ## The JS for this is defined in xqa_interface.html ${block_content} -%if location.category in ['problem','video','html','combinedopenended','graphical_slider_tool']: +%if location.category in ['problem','video','html','combinedopenended','graphical_slider_tool', 'library_content']: % if edit_link:
Edit From d2ae624ce752666937e6c29bb8da9a2119c577e8 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Sat, 1 Nov 2014 19:47:28 -0700 Subject: [PATCH 09/23] Unit and integration tests of content libraries --- .../contentstore/tests/test_libraries.py | 256 ++++++++++++++++++ .../xmodule/tests/test_library_content.py | 142 ++++++++++ 2 files changed, 398 insertions(+) create mode 100644 cms/djangoapps/contentstore/tests/test_libraries.py create mode 100644 common/lib/xmodule/xmodule/tests/test_library_content.py diff --git a/cms/djangoapps/contentstore/tests/test_libraries.py b/cms/djangoapps/contentstore/tests/test_libraries.py new file mode 100644 index 000000000000..b6c6119ed1b0 --- /dev/null +++ b/cms/djangoapps/contentstore/tests/test_libraries.py @@ -0,0 +1,256 @@ +""" +Content library unit tests that require the CMS runtime. +""" +from contentstore.tests.utils import AjaxEnabledTestClient, parse_json +from contentstore.utils import reverse_usage_url +from contentstore.views.preview import _load_preview_module +from contentstore.views.tests.test_library import LIBRARY_REST_URL +import ddt +from xmodule.library_content_module import LibraryVersionReference +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.tests import get_test_system +from mock import Mock +from opaque_keys.edx.locator import CourseKey, LibraryLocator + + +@ddt.ddt +class TestLibraries(ModuleStoreTestCase): + """ + High-level tests for libraries + """ + def setUp(self): + user_password = super(TestLibraries, self).setUp() + + self.client = AjaxEnabledTestClient() + self.client.login(username=self.user.username, password=user_password) + + self.lib_key = self._create_library() + self.library = modulestore().get_library(self.lib_key) + + def _create_library(self, org="org", library="lib", display_name="Test Library"): + """ + Helper method used to create a library. Uses the REST API. + """ + response = self.client.ajax_post(LIBRARY_REST_URL, { + 'org': org, + 'library': library, + 'display_name': display_name, + }) + self.assertEqual(response.status_code, 200) + lib_info = parse_json(response) + lib_key = CourseKey.from_string(lib_info['library_key']) + self.assertIsInstance(lib_key, LibraryLocator) + return lib_key + + def _add_library_content_block(self, course, library_key, other_settings=None): + """ + Helper method to add a LibraryContent block to a course. + The block will be configured to select content from the library + specified by library_key. + other_settings can be a dict of Scope.settings fields to set on the block. + """ + return ItemFactory.create( + category='library_content', + parent_location=course.location, + user_id=self.user.id, + publish_item=False, + source_libraries=[LibraryVersionReference(library_key)], + **(other_settings or {}) + ) + + def _refresh_children(self, lib_content_block): + """ + Helper method: Uses the REST API to call the 'refresh_children' handler + of a LibraryContent block + """ + if 'user' not in lib_content_block.runtime._services: # pylint: disable=protected-access + lib_content_block.runtime._services['user'] = Mock(user_id=self.user.id) # pylint: disable=protected-access + handler_url = reverse_usage_url('component_handler', lib_content_block.location, kwargs={'handler': 'refresh_children'}) + response = self.client.ajax_post(handler_url) + self.assertEqual(response.status_code, 200) + return modulestore().get_item(lib_content_block.location) + + @ddt.data( + (2, 1, 1), + (2, 2, 2), + (2, 20, 2), + ) + @ddt.unpack + def test_max_items(self, num_to_create, num_to_select, num_expected): + """ + Test the 'max_count' property of LibraryContent blocks. + """ + for _ in range(0, num_to_create): + ItemFactory.create(category="html", parent_location=self.library.location, user_id=self.user.id, publish_item=False) + + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + lc_block = self._add_library_content_block(course, self.lib_key, {'max_count': num_to_select}) + self.assertEqual(len(lc_block.children), 0) + lc_block = self._refresh_children(lc_block) + + # Now, we want to make sure that .children has the total # of potential + # children, and that get_child_descriptors() returns the actual children + # chosen for a given student. + # In order to be able to call get_child_descriptors(), we must first + # call bind_for_student: + lc_block.bind_for_student(get_test_system(), lc_block._field_data) # pylint: disable=protected-access + self.assertEqual(len(lc_block.children), num_to_create) + self.assertEqual(len(lc_block.get_child_descriptors()), num_expected) + + def test_consistent_children(self): + """ + Test that the same student will always see the same selected child block + """ + session_data = {} + + def bind_module(descriptor): + """ + Helper to use the CMS's module system so we can access student-specific fields. + """ + request = Mock(user=self.user, session=session_data) + return _load_preview_module(request, descriptor) # pylint: disable=protected-access + + # Create many blocks in the library and add them to a course: + for num in range(0, 8): + ItemFactory.create( + data="This is #{}".format(num + 1), + category="html", parent_location=self.library.location, user_id=self.user.id, publish_item=False + ) + + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + lc_block = self._add_library_content_block(course, self.lib_key, {'max_count': 1}) + lc_block_key = lc_block.location + lc_block = self._refresh_children(lc_block) + + def get_child_of_lc_block(block): + """ + Fetch the child shown to the current user. + """ + children = block.get_child_descriptors() + self.assertEqual(len(children), 1) + return children[0] + + # Check which child a student will see: + bind_module(lc_block) + chosen_child = get_child_of_lc_block(lc_block) + chosen_child_defn_id = chosen_child.definition_locator.definition_id + lc_block.save() + + modulestore().update_item(lc_block, self.user.id) + + # Now re-load the block and try again: + def check(): + """ + Confirm that chosen_child is still the child seen by the test student + """ + for _ in range(0, 6): # Repeat many times b/c blocks are randomized + lc_block = modulestore().get_item(lc_block_key) # Reload block from the database + bind_module(lc_block) + current_child = get_child_of_lc_block(lc_block) + self.assertEqual(current_child.location, chosen_child.location) + self.assertEqual(current_child.data, chosen_child.data) + self.assertEqual(current_child.definition_locator.definition_id, chosen_child_defn_id) + + check() + # Refresh the children: + lc_block = self._refresh_children(lc_block) + # Now re-load the block and try yet again, in case refreshing the children changed anything: + check() + + def test_definition_shared_with_library(self): + """ + Test that the same block definition is used for the library and course[s] + """ + block1 = ItemFactory.create(category="html", parent_location=self.library.location, user_id=self.user.id, publish_item=False) + def_id1 = block1.definition_locator.definition_id + block2 = ItemFactory.create(category="html", parent_location=self.library.location, user_id=self.user.id, publish_item=False) + def_id2 = block2.definition_locator.definition_id + self.assertNotEqual(def_id1, def_id2) + + # Next, create a course: + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + # Add a LibraryContent block to the course: + lc_block = self._add_library_content_block(course, self.lib_key) + lc_block = self._refresh_children(lc_block) + for child_key in lc_block.children: + child = modulestore().get_item(child_key) + def_id = child.definition_locator.definition_id + self.assertIn(def_id, (def_id1, def_id2)) + + def test_fields(self): + """ + Test that blocks used from a library have the same field values as + defined by the library author. + """ + data_value = "A Scope.content value" + name_value = "A Scope.settings value" + lib_block = ItemFactory.create( + category="html", + parent_location=self.library.location, + user_id=self.user.id, + publish_item=False, + display_name=name_value, + data=data_value, + ) + self.assertEqual(lib_block.data, data_value) + self.assertEqual(lib_block.display_name, name_value) + + # Next, create a course: + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + # Add a LibraryContent block to the course: + lc_block = self._add_library_content_block(course, self.lib_key) + lc_block = self._refresh_children(lc_block) + course_block = modulestore().get_item(lc_block.children[0]) + + self.assertEqual(course_block.data, data_value) + self.assertEqual(course_block.display_name, name_value) + + def test_block_with_children(self): + """ + Test that blocks used from a library can have children. + """ + data_value = "A Scope.content value" + name_value = "A Scope.settings value" + # In the library, create a vertical block with a child: + vert_block = ItemFactory.create( + category="vertical", + parent_location=self.library.location, + user_id=self.user.id, + publish_item=False, + ) + child_block = ItemFactory.create( + category="html", + parent_location=vert_block.location, + user_id=self.user.id, + publish_item=False, + display_name=name_value, + data=data_value, + ) + self.assertEqual(child_block.data, data_value) + self.assertEqual(child_block.display_name, name_value) + + # Next, create a course: + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + # Add a LibraryContent block to the course: + lc_block = self._add_library_content_block(course, self.lib_key) + lc_block = self._refresh_children(lc_block) + self.assertEqual(len(lc_block.children), 1) + course_vert_block = modulestore().get_item(lc_block.children[0]) + self.assertEqual(len(course_vert_block.children), 1) + course_child_block = modulestore().get_item(course_vert_block.children[0]) + + self.assertEqual(course_child_block.data, data_value) + self.assertEqual(course_child_block.display_name, name_value) diff --git a/common/lib/xmodule/xmodule/tests/test_library_content.py b/common/lib/xmodule/xmodule/tests/test_library_content.py new file mode 100644 index 000000000000..2b52386e3740 --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/test_library_content.py @@ -0,0 +1,142 @@ +# -*- coding: utf-8 -*- +""" +Basic unit tests for LibraryContentModule + +Higher-level tests are in `cms/djangoapps/contentstore/tests/test_libraries.py`. +""" +import ddt +from xmodule.library_content_module import LibraryVersionReference +from xmodule.modulestore.tests.factories import LibraryFactory, CourseFactory, ItemFactory +from xmodule.modulestore.tests.utils import MixedSplitTestCase +from xmodule.tests import get_test_system +from xmodule.validation import StudioValidationMessage + + +@ddt.ddt +class TestLibraries(MixedSplitTestCase): + """ + Basic unit tests for LibraryContentModule (library_content_module.py) + """ + def setUp(self): + super(TestLibraries, self).setUp() + + self.library = LibraryFactory.create(modulestore=self.store) + self.lib_blocks = [ + ItemFactory.create( + category="html", + parent_location=self.library.location, + user_id=self.user_id, + publish_item=False, + metadata={"data": "Hello world from block {}".format(i), }, + modulestore=self.store, + ) + for i in range(1, 5) + ] + self.course = CourseFactory.create(modulestore=self.store) + self.chapter = ItemFactory.create( + category="chapter", + parent_location=self.course.location, + user_id=self.user_id, + modulestore=self.store, + ) + self.sequential = ItemFactory.create( + category="sequential", + parent_location=self.chapter.location, + user_id=self.user_id, + modulestore=self.store, + ) + self.vertical = ItemFactory.create( + category="vertical", + parent_location=self.sequential.location, + user_id=self.user_id, + modulestore=self.store, + ) + self.lc_block = ItemFactory.create( + category="library_content", + parent_location=self.vertical.location, + user_id=self.user_id, + modulestore=self.store, + metadata={ + 'max_count': 1, + 'source_libraries': [LibraryVersionReference(self.library.location.library_key)] + } + ) + + def _bind_course_module(self, module): + """ + Bind a module (part of self.course) so we can access student-specific data. + """ + module_system = get_test_system(course_id=self.course.location.course_key) + module_system.descriptor_runtime = module.runtime + + def get_module(descriptor): + """Mocks module_system get_module function""" + sub_module_system = get_test_system(course_id=self.course.location.course_key) + sub_module_system.get_module = get_module + sub_module_system.descriptor_runtime = descriptor.runtime + descriptor.bind_for_student(sub_module_system, descriptor._field_data) # pylint: disable=protected-access + return descriptor + + module_system.get_module = get_module + module.xmodule_runtime = module_system + + def test_lib_content_block(self): + """ + Test that blocks from a library are copied and added as children + """ + # Check that the LibraryContent block has no children initially + # Normally the children get added when the "source_libraries" setting + # is updated, but the way we do it through a factory doesn't do that. + self.assertEqual(len(self.lc_block.children), 0) + # Update the LibraryContent module: + self.lc_block.refresh_children(None, None) + # Check that all blocks from the library are now children of the block: + self.assertEqual(len(self.lc_block.children), len(self.lib_blocks)) + + def test_children_seen_by_a_user(self): + """ + Test that each student sees only one block as a child of the LibraryContent block. + """ + self.lc_block.refresh_children(None, None) + self.lc_block = self.store.get_item(self.lc_block.location) + self._bind_course_module(self.lc_block) + # Make sure the runtime knows that the block's children vary per-user: + self.assertTrue(self.lc_block.has_dynamic_children()) + + self.assertEqual(len(self.lc_block.children), len(self.lib_blocks)) + + # Check how many children each user will see: + self.assertEqual(len(self.lc_block.get_child_descriptors()), 1) + # Check that get_content_titles() doesn't return titles for hidden/unused children + self.assertEqual(len(self.lc_block.get_content_titles()), 1) + + def test_validation(self): + """ + Test that the validation method of LibraryContent blocks is working. + """ + # When source_libraries is blank, the validation summary should say this block needs to be configured: + self.lc_block.source_libraries = [] + result = self.lc_block.validate() + self.assertFalse(result) # Validation fails due to at least one warning/message + self.assertTrue(result.summary) + self.assertEqual(StudioValidationMessage.NOT_CONFIGURED, result.summary.type) + + # When source_libraries references a non-existent library, we should get an error: + self.lc_block.source_libraries = [LibraryVersionReference("library-v1:BAD+WOLF")] + result = self.lc_block.validate() + self.assertFalse(result) # Validation fails due to at least one warning/message + self.assertTrue(result.summary) + self.assertEqual(StudioValidationMessage.ERROR, result.summary.type) + self.assertIn("invalid", result.summary.text) + + # When source_libraries is set but the block needs to be updated, the summary should say so: + self.lc_block.source_libraries = [LibraryVersionReference(self.library.location.library_key)] + result = self.lc_block.validate() + self.assertFalse(result) # Validation fails due to at least one warning/message + self.assertTrue(result.summary) + self.assertEqual(StudioValidationMessage.WARNING, result.summary.type) + self.assertIn("out of date", result.summary.text) + + # Now if we update the block, all validation should pass: + self.lc_block.refresh_children(None, None) + self.assertTrue(self.lc_block.validate()) From d1681d05b4b596140d01c7ef635817f22644be56 Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Mon, 8 Dec 2014 15:16:42 +0700 Subject: [PATCH 10/23] LibraryContent bok choy acceptance tests --- .../xmodule/xmodule/library_content_module.py | 1 + common/test/acceptance/fixtures/course.py | 2 - common/test/acceptance/fixtures/library.py | 1 + common/test/acceptance/pages/lms/library.py | 37 ++++ .../test/acceptance/pages/studio/library.py | 179 +++++++++++++++++- .../test/acceptance/tests/lms/test_library.py | 169 +++++++++++++++++ .../tests/studio/base_studio_test.py | 6 +- .../tests/studio/test_studio_library.py | 4 +- .../studio/test_studio_library_container.py | 133 +++++++++++++ 9 files changed, 521 insertions(+), 11 deletions(-) create mode 100644 common/test/acceptance/pages/lms/library.py create mode 100644 common/test/acceptance/tests/lms/test_library.py create mode 100644 common/test/acceptance/tests/studio/test_studio_library_container.py diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 092bc2a93117..2d1e386847b3 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -119,6 +119,7 @@ class LibraryContentFields(object): scope=Scope.settings, ) mode = String( + display_name=_("Mode"), help=_("Determines how content is drawn from the library"), default="random", values=[ diff --git a/common/test/acceptance/fixtures/course.py b/common/test/acceptance/fixtures/course.py index 1e5bca8a337f..656a12a9658a 100644 --- a/common/test/acceptance/fixtures/course.py +++ b/common/test/acceptance/fixtures/course.py @@ -375,5 +375,3 @@ def _create_xblock_children(self, parent_loc, xblock_descriptions): """ super(CourseFixture, self)._create_xblock_children(parent_loc, xblock_descriptions) self._publish_xblock(parent_loc) - - diff --git a/common/test/acceptance/fixtures/library.py b/common/test/acceptance/fixtures/library.py index f97b8e9fc222..5692c078dbd5 100644 --- a/common/test/acceptance/fixtures/library.py +++ b/common/test/acceptance/fixtures/library.py @@ -27,6 +27,7 @@ def __init__(self, org, number, display_name): 'display_name': display_name } + self.display_name = display_name self._library_key = None super(LibraryFixture, self).__init__() diff --git a/common/test/acceptance/pages/lms/library.py b/common/test/acceptance/pages/lms/library.py new file mode 100644 index 000000000000..8655fae79f55 --- /dev/null +++ b/common/test/acceptance/pages/lms/library.py @@ -0,0 +1,37 @@ +""" +Library Content XBlock Wrapper +""" +from bok_choy.page_object import PageObject + + +class LibraryContentXBlockWrapper(PageObject): + """ + A PageObject representing a wrapper around a LibraryContent block seen in the LMS + """ + url = None + BODY_SELECTOR = '.xblock-student_view div' + + def __init__(self, browser, locator): + super(LibraryContentXBlockWrapper, self).__init__(browser) + self.locator = locator + + def is_browser_on_page(self): + return self.q(css='{}[data-id="{}"]'.format(self.BODY_SELECTOR, self.locator)).present + + def _bounded_selector(self, selector): + """ + Return `selector`, but limited to this particular block's context + """ + return '{}[data-id="{}"] {}'.format( + self.BODY_SELECTOR, + self.locator, + selector + ) + + @property + def children_contents(self): + """ + Gets contents of all child XBlocks as list of strings + """ + child_blocks = self.q(css=self._bounded_selector("div[data-id]")) + return frozenset(child.text for child in child_blocks) diff --git a/common/test/acceptance/pages/studio/library.py b/common/test/acceptance/pages/studio/library.py index 64f93f21167e..3151324cd079 100644 --- a/common/test/acceptance/pages/studio/library.py +++ b/common/test/acceptance/pages/studio/library.py @@ -3,8 +3,12 @@ """ from bok_choy.page_object import PageObject -from ...pages.studio.pagination import PaginatedMixin +from bok_choy.promise import EmptyPromise +from selenium.webdriver.common.keys import Keys +from selenium.webdriver.support.select import Select +from .overview import CourseOutlineModal from .container import XBlockWrapper +from ...pages.studio.pagination import PaginatedMixin from ...tests.helpers import disable_animations from .utils import confirm_prompt, wait_for_notification from . import BASE_URL @@ -48,7 +52,10 @@ def wait_until_ready(self): for improved test reliability. """ self.wait_for_ajax() - self.wait_for_element_invisibility('.ui-loading', 'Wait for the page to complete its initial loading of XBlocks via AJAX') + self.wait_for_element_invisibility( + '.ui-loading', + 'Wait for the page to complete its initial loading of XBlocks via AJAX' + ) disable_animations(self) @property @@ -80,14 +87,18 @@ def _get_xblocks(self): Create an XBlockWrapper for each XBlock div found on the page. """ prefix = '.wrapper-xblock.level-page ' - return self.q(css=prefix + XBlockWrapper.BODY_SELECTOR).map(lambda el: XBlockWrapper(self.browser, el.get_attribute('data-locator'))).results + return self.q(css=prefix + XBlockWrapper.BODY_SELECTOR).map( + lambda el: XBlockWrapper(self.browser, el.get_attribute('data-locator')) + ).results def _div_for_xblock_id(self, xblock_id): """ Given an XBlock's usage locator as a string, return the WebElement for that block's wrapper div. """ - return self.q(css='.wrapper-xblock.level-page .studio-xblock-wrapper').filter(lambda el: el.get_attribute('data-locator') == xblock_id) + return self.q(css='.wrapper-xblock.level-page .studio-xblock-wrapper').filter( + lambda el: el.get_attribute('data-locator') == xblock_id + ) def _action_btn_for_xblock_id(self, xblock_id, action): """ @@ -95,4 +106,162 @@ def _action_btn_for_xblock_id(self, xblock_id, action): buttons. action is 'edit', 'duplicate', or 'delete' """ - return self._div_for_xblock_id(xblock_id)[0].find_element_by_css_selector('.header-actions .{action}-button.action-button'.format(action=action)) + return self._div_for_xblock_id(xblock_id)[0].find_element_by_css_selector( + '.header-actions .{action}-button.action-button'.format(action=action) + ) + + +class StudioLibraryContentXBlockEditModal(CourseOutlineModal, PageObject): + """ + Library Content XBlock Modal edit window + """ + url = None + MODAL_SELECTOR = ".wrapper-modal-window-edit-xblock" + + # Labels used to identify the fields on the edit modal: + LIBRARY_LABEL = "Libraries" + COUNT_LABEL = "Count" + SCORED_LABEL = "Scored" + + def is_browser_on_page(self): + """ + Check that we are on the right page in the browser. + """ + return self.is_shown() + + @property + def library_key(self): + """ + Gets value of first library key input + """ + library_key_input = self.get_metadata_input(self.LIBRARY_LABEL) + if library_key_input is not None: + return library_key_input.get_attribute('value').strip(',') + return None + + @library_key.setter + def library_key(self, library_key): + """ + Sets value of first library key input, creating it if necessary + """ + library_key_input = self.get_metadata_input(self.LIBRARY_LABEL) + if library_key_input is None: + library_key_input = self._add_library_key() + if library_key is not None: + # can't use lib_text.clear() here as input get deleted by client side script + library_key_input.send_keys(Keys.HOME) + library_key_input.send_keys(Keys.SHIFT, Keys.END) + library_key_input.send_keys(library_key) + else: + library_key_input.clear() + EmptyPromise(lambda: self.library_key == library_key, "library_key is updated in modal.").fulfill() + + @property + def count(self): + """ + Gets value of children count input + """ + return int(self.get_metadata_input(self.COUNT_LABEL).get_attribute('value')) + + @count.setter + def count(self, count): + """ + Sets value of children count input + """ + count_text = self.get_metadata_input(self.COUNT_LABEL) + count_text.clear() + count_text.send_keys(count) + EmptyPromise(lambda: self.count == count, "count is updated in modal.").fulfill() + + @property + def scored(self): + """ + Gets value of scored select + """ + value = self.get_metadata_input(self.SCORED_LABEL).get_attribute('value') + if value == 'True': + return True + elif value == 'False': + return False + raise ValueError("Unknown value {value} set for {label}".format(value=value, label=self.SCORED_LABEL)) + + @scored.setter + def scored(self, scored): + """ + Sets value of scored select + """ + select_element = self.get_metadata_input(self.SCORED_LABEL) + select_element.click() + scored_select = Select(select_element) + scored_select.select_by_value(str(scored)) + EmptyPromise(lambda: self.scored == scored, "scored is updated in modal.").fulfill() + + def _add_library_key(self): + """ + Adds library key input + """ + wrapper = self._get_metadata_element(self.LIBRARY_LABEL) + add_button = wrapper.find_element_by_xpath(".//a[contains(@class, 'create-action')]") + add_button.click() + return self._get_list_inputs(wrapper)[0] + + def _get_list_inputs(self, list_wrapper): + """ + Finds nested input elements (useful for List and Dict fields) + """ + return list_wrapper.find_elements_by_xpath(".//input[@type='text']") + + def _get_metadata_element(self, metadata_key): + """ + Gets metadata input element (a wrapper div for List and Dict fields) + """ + metadata_inputs = self.find_css(".metadata_entry .wrapper-comp-setting label.setting-label") + target_label = [elem for elem in metadata_inputs if elem.text == metadata_key][0] + label_for = target_label.get_attribute('for') + return self.find_css("#" + label_for)[0] + + def get_metadata_input(self, metadata_key): + """ + Gets input/select element for given field + """ + element = self._get_metadata_element(metadata_key) + if element.tag_name == 'div': + # List or Dict field - return first input + # TODO support multiple values + inputs = self._get_list_inputs(element) + element = inputs[0] if inputs else None + return element + + +class StudioLibraryContainerXBlockWrapper(XBlockWrapper): + """ + Wraps :class:`.container.XBlockWrapper` for use with LibraryContent blocks + """ + url = None + + @classmethod + def from_xblock_wrapper(cls, xblock_wrapper): + """ + Factory method: creates :class:`.StudioLibraryContainerXBlockWrapper` from :class:`.container.XBlockWrapper` + """ + return cls(xblock_wrapper.browser, xblock_wrapper.locator) + + @property + def header_text(self): + """ + Gets library content text + """ + return self.get_body_paragraphs().first.text[0] + + def get_body_paragraphs(self): + """ + Gets library content body paragraphs + """ + return self.q(css=self._bounded_selector(".xblock-message-area p")) + + def refresh_children(self): + """ + Click "Update now..." button + """ + refresh_button = self.q(css=self._bounded_selector(".library-update-btn")) + refresh_button.click() diff --git a/common/test/acceptance/tests/lms/test_library.py b/common/test/acceptance/tests/lms/test_library.py new file mode 100644 index 000000000000..78d699faa6fd --- /dev/null +++ b/common/test/acceptance/tests/lms/test_library.py @@ -0,0 +1,169 @@ +# -*- coding: utf-8 -*- +""" +End-to-end tests for LibraryContent block in LMS +""" +import ddt + +from ..helpers import UniqueCourseTest +from ...pages.studio.auto_auth import AutoAuthPage +from ...pages.studio.overview import CourseOutlinePage +from ...pages.studio.library import StudioLibraryContentXBlockEditModal, StudioLibraryContainerXBlockWrapper +from ...pages.lms.courseware import CoursewarePage +from ...pages.lms.library import LibraryContentXBlockWrapper +from ...pages.common.logout import LogoutPage +from ...fixtures.course import CourseFixture, XBlockFixtureDesc +from ...fixtures.library import LibraryFixture + +SECTION_NAME = 'Test Section' +SUBSECTION_NAME = 'Test Subsection' +UNIT_NAME = 'Test Unit' + + +@ddt.ddt +class LibraryContentTest(UniqueCourseTest): + """ + Test courseware. + """ + USERNAME = "STUDENT_TESTER" + EMAIL = "student101@example.com" + + STAFF_USERNAME = "STAFF_TESTER" + STAFF_EMAIL = "staff101@example.com" + + def setUp(self): + """ + Set up library, course and library content XBlock + """ + super(LibraryContentTest, self).setUp() + + self.courseware_page = CoursewarePage(self.browser, self.course_id) + + self.course_outline = CourseOutlinePage( + self.browser, + self.course_info['org'], + self.course_info['number'], + self.course_info['run'] + ) + + self.library_fixture = LibraryFixture('test_org', self.unique_id, 'Test Library {}'.format(self.unique_id)) + self.library_fixture.add_children( + XBlockFixtureDesc("html", "Html1", data='html1'), + XBlockFixtureDesc("html", "Html2", data='html2'), + XBlockFixtureDesc("html", "Html3", data='html3'), + ) + + self.library_fixture.install() + self.library_info = self.library_fixture.library_info + self.library_key = self.library_fixture.library_key + + # Install a course with library content xblock + self.course_fixture = CourseFixture( + self.course_info['org'], self.course_info['number'], + self.course_info['run'], self.course_info['display_name'] + ) + + library_content_metadata = { + 'source_libraries': [self.library_key], + 'mode': 'random', + 'max_count': 1, + 'has_score': False + } + + self.lib_block = XBlockFixtureDesc('library_content', "Library Content", metadata=library_content_metadata) + + self.course_fixture.add_children( + XBlockFixtureDesc('chapter', SECTION_NAME).add_children( + XBlockFixtureDesc('sequential', SUBSECTION_NAME).add_children( + XBlockFixtureDesc('vertical', UNIT_NAME).add_children( + self.lib_block + ) + ) + ) + ) + + self.course_fixture.install() + + def _refresh_library_content_children(self, count=1): + """ + Performs library block refresh in Studio, configuring it to show {count} children + """ + unit_page = self._go_to_unit_page(True) + library_container_block = StudioLibraryContainerXBlockWrapper.from_xblock_wrapper(unit_page.xblocks[0]) + modal = StudioLibraryContentXBlockEditModal(library_container_block.edit()) + modal.count = count + library_container_block.save_settings() + library_container_block.refresh_children() + self._go_to_unit_page(change_login=False) + unit_page.wait_for_page() + unit_page.publish_action.click() + unit_page.wait_for_ajax() + self.assertIn("Published and Live", unit_page.publish_title) + + @property + def library_xblocks_texts(self): + """ + Gets texts of all xblocks in library + """ + return frozenset(child.data for child in self.library_fixture.children) + + def _go_to_unit_page(self, change_login=True): + """ + Open unit page in Studio + """ + if change_login: + LogoutPage(self.browser).visit() + self._auto_auth(self.STAFF_USERNAME, self.STAFF_EMAIL, True) + self.course_outline.visit() + subsection = self.course_outline.section(SECTION_NAME).subsection(SUBSECTION_NAME) + return subsection.toggle_expand().unit(UNIT_NAME).go_to() + + def _goto_library_block_page(self, block_id=None): + """ + Open library page in LMS + """ + self.courseware_page.visit() + block_id = block_id if block_id is not None else self.lib_block.locator + #pylint: disable=attribute-defined-outside-init + self.library_content_page = LibraryContentXBlockWrapper(self.browser, block_id) + + def _auto_auth(self, username, email, staff): + """ + Logout and login with given credentials. + """ + AutoAuthPage(self.browser, username=username, email=email, + course_id=self.course_id, staff=staff).visit() + + @ddt.data(1, 2, 3) + def test_shows_random_xblocks_from_configured(self, count): + """ + Scenario: Ensures that library content shows {count} random xblocks from library in LMS + Given I have a library, a course and a LibraryContent block in that course + When I go to studio unit page for library content xblock as staff + And I set library content xblock to display {count} random children + And I refresh library content xblock and pulbish unit + When I go to LMS courseware page for library content xblock as student + Then I can see {count} random xblocks from the library + """ + self._refresh_library_content_children(count=count) + self._auto_auth(self.USERNAME, self.EMAIL, False) + self._goto_library_block_page() + children_contents = self.library_content_page.children_contents + self.assertEqual(len(children_contents), count) + self.assertLessEqual(children_contents, self.library_xblocks_texts) + + def test_shows_all_if_max_set_to_greater_value(self): + """ + Scenario: Ensures that library content shows {count} random xblocks from library in LMS + Given I have a library, a course and a LibraryContent block in that course + When I go to studio unit page for library content xblock as staff + And I set library content xblock to display more children than library have + And I refresh library content xblock and pulbish unit + When I go to LMS courseware page for library content xblock as student + Then I can see all xblocks from the library + """ + self._refresh_library_content_children(count=10) + self._auto_auth(self.USERNAME, self.EMAIL, False) + self._goto_library_block_page() + children_contents = self.library_content_page.children_contents + self.assertEqual(len(children_contents), 3) + self.assertEqual(children_contents, self.library_xblocks_texts) diff --git a/common/test/acceptance/tests/studio/base_studio_test.py b/common/test/acceptance/tests/studio/base_studio_test.py index ec94f7f058c5..02fdcbe99849 100644 --- a/common/test/acceptance/tests/studio/base_studio_test.py +++ b/common/test/acceptance/tests/studio/base_studio_test.py @@ -109,8 +109,9 @@ class StudioLibraryTest(WebAppTest): """ Base class for all Studio library tests. """ + as_staff = True - def setUp(self, is_staff=False): # pylint: disable=arguments-differ + def setUp(self): # pylint: disable=arguments-differ """ Install a library with no content using a fixture. """ @@ -122,10 +123,11 @@ def setUp(self, is_staff=False): # pylint: disable=arguments-differ ) self.populate_library_fixture(fixture) fixture.install() + self.library_fixture = fixture self.library_info = fixture.library_info self.library_key = fixture.library_key self.user = fixture.user - self.log_in(self.user, is_staff) + self.log_in(self.user, self.as_staff) def populate_library_fixture(self, library_fixture): """ diff --git a/common/test/acceptance/tests/studio/test_studio_library.py b/common/test/acceptance/tests/studio/test_studio_library.py index 491c9093d0fc..b0d6cffb1aed 100644 --- a/common/test/acceptance/tests/studio/test_studio_library.py +++ b/common/test/acceptance/tests/studio/test_studio_library.py @@ -18,7 +18,7 @@ def setUp(self): # pylint: disable=arguments-differ """ Ensure a library exists and navigate to the library edit page. """ - super(LibraryEditPageTest, self).setUp(is_staff=True) + super(LibraryEditPageTest, self).setUp() self.lib_page = LibraryPage(self.browser, self.library_key) self.lib_page.visit() self.lib_page.wait_until_ready() @@ -156,7 +156,7 @@ def setUp(self): # pylint: disable=arguments-differ """ Ensure a library exists and navigate to the library edit page. """ - super(LibraryNavigationTest, self).setUp(is_staff=True) + super(LibraryNavigationTest, self).setUp() self.lib_page = LibraryPage(self.browser, self.library_key) self.lib_page.visit() self.lib_page.wait_until_ready() diff --git a/common/test/acceptance/tests/studio/test_studio_library_container.py b/common/test/acceptance/tests/studio/test_studio_library_container.py new file mode 100644 index 000000000000..7bb712c779b5 --- /dev/null +++ b/common/test/acceptance/tests/studio/test_studio_library_container.py @@ -0,0 +1,133 @@ +""" +Acceptance tests for Library Content in LMS +""" +import ddt +from .base_studio_test import StudioLibraryTest, ContainerBase +from ...pages.studio.library import StudioLibraryContentXBlockEditModal, StudioLibraryContainerXBlockWrapper +from ...fixtures.course import XBlockFixtureDesc + +SECTION_NAME = 'Test Section' +SUBSECTION_NAME = 'Test Subsection' +UNIT_NAME = 'Test Unit' + + +@ddt.ddt +class StudioLibraryContainerTest(ContainerBase, StudioLibraryTest): + """ + Test Library Content block in LMS + """ + def setUp(self): + """ + Install library with some content and a course using fixtures + """ + super(StudioLibraryContainerTest, self).setUp() + self.outline.visit() + subsection = self.outline.section(SECTION_NAME).subsection(SUBSECTION_NAME) + self.unit_page = subsection.toggle_expand().unit(UNIT_NAME).go_to() + + def populate_library_fixture(self, library_fixture): + """ + Populate the children of the test course fixture. + """ + library_fixture.add_children( + XBlockFixtureDesc("html", "Html1"), + XBlockFixtureDesc("html", "Html2"), + XBlockFixtureDesc("html", "Html3"), + ) + + def populate_course_fixture(self, course_fixture): + """ Install a course with sections/problems, tabs, updates, and handouts """ + library_content_metadata = { + 'source_libraries': [self.library_key], + 'mode': 'random', + 'max_count': 1, + 'has_score': False + } + + course_fixture.add_children( + XBlockFixtureDesc('chapter', SECTION_NAME).add_children( + XBlockFixtureDesc('sequential', SUBSECTION_NAME).add_children( + XBlockFixtureDesc('vertical', UNIT_NAME).add_children( + XBlockFixtureDesc('library_content', "Library Content", metadata=library_content_metadata) + ) + ) + ) + ) + + def _get_library_xblock_wrapper(self, xblock): + """ + Wraps xblock into :class:`...pages.studio.library.StudioLibraryContainerXBlockWrapper` + """ + return StudioLibraryContainerXBlockWrapper.from_xblock_wrapper(xblock) + + @ddt.data( + ('library-v1:111+111', 1, True), + ('library-v1:edX+L104', 2, False), + ('library-v1:OtherX+IDDQD', 3, True), + ) + @ddt.unpack + def test_can_edit_metadata(self, library_key, max_count, scored): + """ + Scenario: Given I have a library, a course and library content xblock in a course + When I go to studio unit page for library content block + And I edit library content metadata and save it + Then I can ensure that data is persisted + """ + library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) + edit_modal = StudioLibraryContentXBlockEditModal(library_container.edit()) + edit_modal.library_key = library_key + edit_modal.count = max_count + edit_modal.scored = scored + + library_container.save_settings() # saving settings + + # open edit window again to verify changes are persistent + edit_modal = StudioLibraryContentXBlockEditModal(library_container.edit()) + self.assertEqual(edit_modal.library_key, library_key) + self.assertEqual(edit_modal.count, max_count) + self.assertEqual(edit_modal.scored, scored) + + def test_no_library_shows_library_not_configured(self): + """ + Scenario: Given I have a library, a course and library content xblock in a course + When I go to studio unit page for library content block + And I edit set library key to none + Then I can see that library content block is misconfigured + """ + expected_text = 'No library or filters configured. Press "Edit" to configure.' + library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) + + # precondition check - assert library is configured before we remove it + self.assertNotIn(expected_text, library_container.header_text) + + edit_modal = StudioLibraryContentXBlockEditModal(library_container.edit()) + edit_modal.library_key = None + + library_container.save_settings() + + self.assertIn(expected_text, library_container.header_text) + + @ddt.data( + 'library-v1:111+111', + 'library-v1:edX+L104', + ) + def test_set_missing_library_shows_correct_label(self, library_key): + """ + Scenario: Given I have a library, a course and library content xblock in a course + When I go to studio unit page for library content block + And I edit set library key to non-existent library + Then I can see that library content block is misconfigured + """ + expected_text = "Library is invalid, corrupt, or has been deleted." + + library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) + + # precondition check - assert library is configured before we remove it + self.assertNotIn(expected_text, library_container.header_text) + + edit_modal = StudioLibraryContentXBlockEditModal(library_container.edit()) + edit_modal.library_key = library_key + + library_container.save_settings() + + self.assertIn(expected_text, library_container.header_text) From e312c15a8d91a1550df9ea970e7f23df008e17ba Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Wed, 10 Dec 2014 13:02:38 -0800 Subject: [PATCH 11/23] Friendly error message when library key is invalid --- cms/djangoapps/contentstore/views/item.py | 5 ++-- .../contentstore/views/tests/test_item.py | 23 +++++++++++++++++++ .../xmodule/xmodule/library_content_module.py | 17 +++++++++++--- 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index c1f627009cd0..7b893486eedd 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -427,8 +427,9 @@ def _save_xblock(user, xblock, data=None, children_strings=None, metadata=None, else: try: value = field.from_json(value) - except ValueError: - return JsonResponse({"error": "Invalid data"}, 400) + except ValueError as verr: + reason = _("Invalid data ({details})").format(details=verr.message) if verr.message else _("Invalid data") + return JsonResponse({"error": reason}, 400) field.write_to(xblock, value) # update the xblock and call any xblock callbacks diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index d6d913a59489..6b595ae00755 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -894,6 +894,29 @@ def test_publish_states_of_nested_xblocks(self): self._verify_published_with_draft(unit_usage_key) self._verify_published_with_draft(html_usage_key) + def test_field_value_errors(self): + """ + Test that if the user's input causes a ValueError on an XBlock field, + we provide a friendly error message back to the user. + """ + response = self.create_xblock(parent_usage_key=self.seq_usage_key, category='video') + video_usage_key = self.response_usage_key(response) + update_url = reverse_usage_url('xblock_handler', video_usage_key) + + response = self.client.ajax_post( + update_url, + data={ + 'id': unicode(video_usage_key), + 'metadata': { + 'saved_video_position': "Not a valid relative time", + }, + } + ) + self.assertEqual(response.status_code, 400) + parsed = json.loads(response.content) + self.assertIn("error", parsed) + self.assertIn("Incorrect RelativeTime value", parsed["error"]) # See xmodule/fields.py + class TestEditSplitModule(ItemTest): """ diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 2d1e386847b3..4233429e0b2f 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -1,11 +1,12 @@ """ LibraryContent: The XBlock used to include blocks from a library in a course. """ -from bson.objectid import ObjectId +from bson.objectid import ObjectId, InvalidId from collections import namedtuple from copy import copy import hashlib from .mako_module import MakoModuleDescriptor +from opaque_keys import InvalidKeyError from opaque_keys.edx.locator import LibraryLocator import random from webob import Response @@ -46,7 +47,10 @@ def __new__(cls, library_id, version=None): version = library_id.version_guid library_id = library_id.for_version(None) if version and not isinstance(version, ObjectId): - version = ObjectId(version) + try: + version = ObjectId(version) + except InvalidId: + raise ValueError(version) return super(LibraryVersionReference, cls).__new__(cls, library_id, version) @staticmethod @@ -86,7 +90,14 @@ def parse(val): val = val.strip(' []') parts = val.rsplit(',', 1) val = [parts[0], parts[1] if len(parts) > 1 else None] - return LibraryVersionReference.from_json(val) + try: + return LibraryVersionReference.from_json(val) + except InvalidKeyError: + try: + friendly_val = val[0] # Just get the library key part, not the version + except IndexError: + friendly_val = unicode(val) + raise ValueError(_('"{value}" is not a valid library ID.').format(value=friendly_val)) return [parse(v) for v in values] def to_json(self, values): From eecc6f1032ab55d249d7c4289f9c5cf64caaa91e Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Wed, 26 Nov 2014 21:48:08 +0000 Subject: [PATCH 12/23] Added explanation to container view of Library Block. --- common/lib/xmodule/xmodule/library_content_module.py | 7 ++++++- lms/templates/library-block-author-preview-header.html | 10 ++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 lms/templates/library-block-author-preview-header.html diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 4233429e0b2f..753d1938556a 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -251,7 +251,7 @@ def student_view(self, context): fragment.add_frag_resources(rendered_child) contents.append({ 'id': displayable.location.to_deprecated_string(), - 'content': rendered_child.content + 'content': rendered_child.content, }) fragment.add_content(self.system.render_template('vert_module.html', { @@ -273,6 +273,11 @@ def author_view(self, context): if is_root: # User has clicked the "View" link. Show a preview of all possible children: if self.children: # pylint: disable=no-member + fragment.add_content(self.system.render_template("library-block-author-preview-header.html", { + 'max_count': self.max_count, + 'display_name': self.display_name or self.url_name, + 'mode': self.mode, + })) self.render_children(context, fragment, can_reorder=False, can_add=False) else: fragment.add_content(u'

{}

'.format( diff --git a/lms/templates/library-block-author-preview-header.html b/lms/templates/library-block-author-preview-header.html new file mode 100644 index 000000000000..4596281b6702 --- /dev/null +++ b/lms/templates/library-block-author-preview-header.html @@ -0,0 +1,10 @@ +<%! from django.utils.translation import ugettext as _ %> +
+
+

+ + ${_('Showing all matching content eligible to be added into {display_name}. Each student will be assigned {mode} {max_count} components from this list.').format(max_count=max_count, display_name=display_name, mode=mode)} + +

+
+
From a7b577d1737daca865f29daff94578b7f058a0a2 Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Wed, 26 Nov 2014 23:57:16 +0000 Subject: [PATCH 13/23] Made errors on Library blocks use validate functionality. --- .../xmodule/xmodule/library_content_module.py | 77 ++++++++++++------- lms/templates/library-block-author-view.html | 6 +- 2 files changed, 52 insertions(+), 31 deletions(-) diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 753d1938556a..476c97b0112d 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -14,6 +14,7 @@ from xblock.fields import Scope, String, List, Integer, Boolean from xblock.fragment import Fragment from xmodule.modulestore.exceptions import ItemNotFoundError +from xmodule.validation import StudioValidationMessage, StudioValidation from xmodule.x_module import XModule, STUDENT_VIEW from xmodule.studio_editable import StudioEditableModule, StudioEditableDescriptor from .xml_module import XmlDescriptor @@ -260,6 +261,40 @@ def student_view(self, context): })) return fragment + def validate(self): + """ + Validates the state of this Library Content Module Instance. This + is the override of the general XBlock method, and it will also ask + its superclass to validate. + """ + validation = super(LibraryContentModule, self).validate() + if not isinstance(validation, StudioValidation): + validation = StudioValidation.copy(validation) + if not self.source_libraries: + validation.set_summary( + StudioValidationMessage( + StudioValidationMessage.NOT_CONFIGURED, + _(u"A library has not yet been selected."), + action_class='edit-button', + action_label=_(u"Select a Library") + ) + ) + return validation + for library_key, version in self.source_libraries: # pylint: disable=unused-variable + library = _get_library(self.runtime.descriptor_runtime.modulestore, library_key) + if library is None: + validation.set_summary( + StudioValidationMessage( + StudioValidationMessage.ERROR, + _(u'Library is invalid, corrupt, or has been deleted.'), + action_class='edit-button', + action_label=_(u"Edit Library List") + ) + ) + break + + return validation + def author_view(self, context): """ Renders the Studio views. @@ -284,41 +319,31 @@ def author_view(self, context): _('No matching content found in library, no library configured, or not yet loaded from library.') )) else: - # When shown on a unit page, don't show any sort of preview - just the status of this block. - LibraryStatus = enum( # pylint: disable=invalid-name - NONE=0, # no library configured - INVALID=1, # invalid configuration or library has been deleted/corrupted - OK=2, # library configured correctly and should be working fine - ) UpdateStatus = enum( # pylint: disable=invalid-name CANNOT=0, # Cannot update - library is not set, invalid, deleted, etc. NEEDED=1, # An update is needed - prompt the user to update UP_TO_DATE=2, # No update necessary - library is up to date ) + # When shown on a unit page, don't show any sort of preview - just the status of this block. + library_ok = bool(self.source_libraries) # True if at least one source library is defined library_names = [] - library_status = LibraryStatus.OK update_status = UpdateStatus.UP_TO_DATE - if self.source_libraries: - for library_key, version in self.source_libraries: - library = _get_library(self.runtime.descriptor_runtime.modulestore, library_key) - if library is None: - library_status = LibraryStatus.INVALID - update_status = UpdateStatus.CANNOT - break - library_names.append(library.display_name) - latest_version = library.location.library_key.version_guid - if version is None or version != latest_version: - update_status = UpdateStatus.NEEDED - # else library is up to date. - else: - library_status = LibraryStatus.NONE - update_status = UpdateStatus.CANNOT + for library_key, version in self.source_libraries: + library = _get_library(self.runtime.descriptor_runtime.modulestore, library_key) + if library is None: + update_status = UpdateStatus.CANNOT + library_ok = False + break + library_names.append(library.display_name) + latest_version = library.location.library_key.version_guid + if version is None or version != latest_version: + update_status = UpdateStatus.NEEDED + fragment.add_content(self.system.render_template('library-block-author-view.html', { - 'library_status': library_status, - 'LibraryStatus': LibraryStatus, - 'update_status': update_status, - 'UpdateStatus': UpdateStatus, 'library_names': library_names, + 'library_ok': library_ok, + 'UpdateStatus': UpdateStatus, + 'update_status': update_status, 'max_count': self.max_count, 'mode': self.mode, 'num_children': len(self.children), # pylint: disable=no-member diff --git a/lms/templates/library-block-author-view.html b/lms/templates/library-block-author-view.html index 521946a903db..ce1542cc5888 100644 --- a/lms/templates/library-block-author-view.html +++ b/lms/templates/library-block-author-view.html @@ -2,16 +2,12 @@ from django.utils.translation import ugettext as _ %>
- % if library_status == LibraryStatus.OK: + % if library_ok:

${_('This component will be replaced by {mode} {max_count} components from the {num_children} matching components from {lib_names}.').format(mode=mode, max_count=max_count, num_children=num_children, lib_names=', '.join(library_names))}

% if update_status == UpdateStatus.NEEDED:

${_('This component is out of date.')} ↻ ${_('Update now with latest components from the library')}

% elif update_status == UpdateStatus.UP_TO_DATE:

${_(u'✓ Up to date.')}

% endif - % elif library_status == LibraryStatus.NONE: -

${_('No library or filters configured. Press "Edit" to configure.')}

- % else: -

${_('Library is invalid, corrupt, or has been deleted.')}

% endif
From dfd0d70ab8209a57b88cf5c3155fbac25d1c4c6d Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Wed, 10 Dec 2014 14:08:51 -0800 Subject: [PATCH 14/23] Move update link to the validation area --- .../xmodule/xmodule/library_content_module.py | 65 +++++++++---------- .../xmodule/public/js/library_content_edit.js | 12 +++- .../test/acceptance/pages/studio/container.py | 39 +++++++++++ .../test/acceptance/pages/studio/library.py | 11 +--- .../studio/test_studio_library_container.py | 47 ++++++++++---- .../library-block-author-preview-header.html | 2 +- lms/templates/library-block-author-view.html | 9 +-- 7 files changed, 117 insertions(+), 68 deletions(-) diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 476c97b0112d..d9e28e93fd09 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -1,3 +1,4 @@ +# -*- coding: utf-8 -*- """ LibraryContent: The XBlock used to include blocks from a library in a course. """ @@ -280,9 +281,21 @@ def validate(self): ) ) return validation - for library_key, version in self.source_libraries: # pylint: disable=unused-variable + for library_key, version in self.source_libraries: library = _get_library(self.runtime.descriptor_runtime.modulestore, library_key) - if library is None: + if library is not None: + latest_version = library.location.library_key.version_guid + if version is None or version != latest_version: + validation.set_summary( + StudioValidationMessage( + StudioValidationMessage.WARNING, + _(u'This component is out of date. The library has new content.'), + action_class='library-update-btn', # TODO: change this to action_runtime_event='...' once the unit page supports that feature. + action_label=_(u"↻ Update now") + ) + ) + break + else: validation.set_summary( StudioValidationMessage( StudioValidationMessage.ERROR, @@ -298,7 +311,7 @@ def validate(self): def author_view(self, context): """ Renders the Studio views. - Normal studio view: displays library status and has an "Update" button. + Normal studio view: If block is properly configured, displays library status summary Studio container view: displays a preview of all possible children. """ fragment = Fragment() @@ -311,45 +324,25 @@ def author_view(self, context): fragment.add_content(self.system.render_template("library-block-author-preview-header.html", { 'max_count': self.max_count, 'display_name': self.display_name or self.url_name, - 'mode': self.mode, })) self.render_children(context, fragment, can_reorder=False, can_add=False) - else: - fragment.add_content(u'

{}

'.format( - _('No matching content found in library, no library configured, or not yet loaded from library.') - )) else: - UpdateStatus = enum( # pylint: disable=invalid-name - CANNOT=0, # Cannot update - library is not set, invalid, deleted, etc. - NEEDED=1, # An update is needed - prompt the user to update - UP_TO_DATE=2, # No update necessary - library is up to date - ) # When shown on a unit page, don't show any sort of preview - just the status of this block. - library_ok = bool(self.source_libraries) # True if at least one source library is defined library_names = [] - update_status = UpdateStatus.UP_TO_DATE - for library_key, version in self.source_libraries: + for library_key, version in self.source_libraries: # pylint: disable=unused-variable library = _get_library(self.runtime.descriptor_runtime.modulestore, library_key) - if library is None: - update_status = UpdateStatus.CANNOT - library_ok = False - break - library_names.append(library.display_name) - latest_version = library.location.library_key.version_guid - if version is None or version != latest_version: - update_status = UpdateStatus.NEEDED - - fragment.add_content(self.system.render_template('library-block-author-view.html', { - 'library_names': library_names, - 'library_ok': library_ok, - 'UpdateStatus': UpdateStatus, - 'update_status': update_status, - 'max_count': self.max_count, - 'mode': self.mode, - 'num_children': len(self.children), # pylint: disable=no-member - })) - fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/library_content_edit.js')) - fragment.initialize_js('LibraryContentAuthorView') + if library is not None: + library_names.append(library.display_name) + + if library_names: + fragment.add_content(self.system.render_template('library-block-author-view.html', { + 'library_names': library_names, + 'max_count': self.max_count, + 'num_children': len(self.children), # pylint: disable=no-member + })) + # The following JS is used to make the "Update now" button work on the unit page and the container view: + fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/library_content_edit.js')) + fragment.initialize_js('LibraryContentAuthorView') return fragment def get_child_descriptors(self): diff --git a/common/lib/xmodule/xmodule/public/js/library_content_edit.js b/common/lib/xmodule/xmodule/public/js/library_content_edit.js index 9a84a214049c..2db019feddf2 100644 --- a/common/lib/xmodule/xmodule/public/js/library_content_edit.js +++ b/common/lib/xmodule/xmodule/public/js/library_content_edit.js @@ -1,6 +1,14 @@ -/* JavaScript for editing operations that can be done on LibraryContentXBlock */ +/* JavaScript for special editing operations that can be done on LibraryContentXBlock */ window.LibraryContentAuthorView = function (runtime, element) { - $(element).find('.library-update-btn').on('click', function(e) { + "use strict"; + var usage_id = $(element).data('usage-id'); + // The "Update Now" button is not a child of 'element', as it is in the validation message area + // But it is still inside this xblock's wrapper element, which we can easily find: + var $wrapper = $(element).parents('*[data-locator="'+usage_id+'"]'); + + // We can't bind to the button itself because in the bok choy test environment, + // it may not yet exist at this point in time... not sure why. + $wrapper.on('click', '.library-update-btn', function(e) { e.preventDefault(); // Update the XBlock with the latest matching content from the library: runtime.notify('save', { diff --git a/common/test/acceptance/pages/studio/container.py b/common/test/acceptance/pages/studio/container.py index 20cf140ed117..dc93be67f561 100644 --- a/common/test/acceptance/pages/studio/container.py +++ b/common/test/acceptance/pages/studio/container.py @@ -333,6 +333,45 @@ def children(self): grand_locators = [grandkid.locator for grandkid in grandkids] return [descendant for descendant in descendants if descendant.locator not in grand_locators] + @property + def has_validation_message(self): + """ Is a validation warning/error/message shown? """ + return self.q(css=self._bounded_selector('.xblock-message.validation')).present + + def _validation_paragraph(self, css_class): + """ Helper method to return the

element of a validation warning """ + return self.q(css=self._bounded_selector('.xblock-message.validation p.{}'.format(css_class))) + + @property + def has_validation_warning(self): + """ Is a validation warning shown? """ + return self._validation_paragraph('warning').present + + @property + def has_validation_error(self): + """ Is a validation error shown? """ + return self._validation_paragraph('error').present + + @property + def has_validation_not_configured_warning(self): + """ Is a validation "not configured" message shown? """ + return self._validation_paragraph('not-configured').present + + @property + def validation_warning_text(self): + """ Get the text of the validation warning. """ + return self._validation_paragraph('warning').text[0] + + @property + def validation_error_text(self): + """ Get the text of the validation error. """ + return self._validation_paragraph('error').text[0] + + @property + def validation_not_configured_warning_text(self): + """ Get the text of the validation "not configured" message. """ + return self._validation_paragraph('not-configured').text[0] + @property def preview_selector(self): return self._bounded_selector('.xblock-student_view,.xblock-author_view') diff --git a/common/test/acceptance/pages/studio/library.py b/common/test/acceptance/pages/studio/library.py index 3151324cd079..ea7f2299f961 100644 --- a/common/test/acceptance/pages/studio/library.py +++ b/common/test/acceptance/pages/studio/library.py @@ -246,13 +246,6 @@ def from_xblock_wrapper(cls, xblock_wrapper): """ return cls(xblock_wrapper.browser, xblock_wrapper.locator) - @property - def header_text(self): - """ - Gets library content text - """ - return self.get_body_paragraphs().first.text[0] - def get_body_paragraphs(self): """ Gets library content body paragraphs @@ -263,5 +256,7 @@ def refresh_children(self): """ Click "Update now..." button """ - refresh_button = self.q(css=self._bounded_selector(".library-update-btn")) + btn_selector = self._bounded_selector(".library-update-btn") + refresh_button = self.q(css=btn_selector) refresh_button.click() + self.wait_for_element_absence(btn_selector, 'Wait for the XBlock to reload') diff --git a/common/test/acceptance/tests/studio/test_studio_library_container.py b/common/test/acceptance/tests/studio/test_studio_library_container.py index 7bb712c779b5..ba66bdd7b1ec 100644 --- a/common/test/acceptance/tests/studio/test_studio_library_container.py +++ b/common/test/acceptance/tests/studio/test_studio_library_container.py @@ -94,40 +94,61 @@ def test_no_library_shows_library_not_configured(self): And I edit set library key to none Then I can see that library content block is misconfigured """ - expected_text = 'No library or filters configured. Press "Edit" to configure.' + expected_text = 'A library has not yet been selected.' + expected_action = 'Select a Library' library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) - # precondition check - assert library is configured before we remove it - self.assertNotIn(expected_text, library_container.header_text) + # precondition check - the library block should be configured before we remove the library setting + self.assertFalse(library_container.has_validation_not_configured_warning) edit_modal = StudioLibraryContentXBlockEditModal(library_container.edit()) edit_modal.library_key = None - library_container.save_settings() - self.assertIn(expected_text, library_container.header_text) + self.assertTrue(library_container.has_validation_not_configured_warning) + self.assertIn(expected_text, library_container.validation_not_configured_warning_text) + self.assertIn(expected_action, library_container.validation_not_configured_warning_text) - @ddt.data( - 'library-v1:111+111', - 'library-v1:edX+L104', - ) - def test_set_missing_library_shows_correct_label(self, library_key): + def test_set_missing_library_shows_correct_label(self): """ Scenario: Given I have a library, a course and library content xblock in a course When I go to studio unit page for library content block And I edit set library key to non-existent library Then I can see that library content block is misconfigured """ + nonexistent_lib_key = 'library-v1:111+111' expected_text = "Library is invalid, corrupt, or has been deleted." library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) # precondition check - assert library is configured before we remove it - self.assertNotIn(expected_text, library_container.header_text) + self.assertFalse(library_container.has_validation_error) edit_modal = StudioLibraryContentXBlockEditModal(library_container.edit()) - edit_modal.library_key = library_key + edit_modal.library_key = nonexistent_lib_key library_container.save_settings() - self.assertIn(expected_text, library_container.header_text) + self.assertTrue(library_container.has_validation_error) + self.assertIn(expected_text, library_container.validation_error_text) + + def test_out_of_date_message(self): + """ + Scenario: Given I have a library, a course and library content xblock in a course + When I go to studio unit page for library content block + Then I can see that library content block needs to be updated + When I click on the update link + Then I can see that the content no longer needs to be updated + """ + expected_text = "This component is out of date. The library has new content." + library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) + + self.assertTrue(library_container.has_validation_warning) + self.assertIn(expected_text, library_container.validation_warning_text) + + library_container.refresh_children() + + self.unit_page.wait_for_page() # Wait for the page to reload + library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) + + self.assertFalse(library_container.has_validation_message) diff --git a/lms/templates/library-block-author-preview-header.html b/lms/templates/library-block-author-preview-header.html index 4596281b6702..ad76623deb6a 100644 --- a/lms/templates/library-block-author-preview-header.html +++ b/lms/templates/library-block-author-preview-header.html @@ -3,7 +3,7 @@

- ${_('Showing all matching content eligible to be added into {display_name}. Each student will be assigned {mode} {max_count} components from this list.').format(max_count=max_count, display_name=display_name, mode=mode)} + ${_('Showing all matching content eligible to be added into {display_name}. Each student will be assigned {max_count} component[s] drawn randomly from this list.').format(max_count=max_count, display_name=display_name)}

diff --git a/lms/templates/library-block-author-view.html b/lms/templates/library-block-author-view.html index ce1542cc5888..46202aa2a9c6 100644 --- a/lms/templates/library-block-author-view.html +++ b/lms/templates/library-block-author-view.html @@ -2,12 +2,5 @@ from django.utils.translation import ugettext as _ %>
- % if library_ok: -

${_('This component will be replaced by {mode} {max_count} components from the {num_children} matching components from {lib_names}.').format(mode=mode, max_count=max_count, num_children=num_children, lib_names=', '.join(library_names))}

- % if update_status == UpdateStatus.NEEDED: -

${_('This component is out of date.')} ↻ ${_('Update now with latest components from the library')}

- % elif update_status == UpdateStatus.UP_TO_DATE: -

${_(u'✓ Up to date.')}

- % endif - % endif +

${_('This component will be replaced by {max_count} component[s] randomly chosen from the {num_children} matching components in {lib_names}.').format(mode=mode, max_count=max_count, num_children=num_children, lib_names=', '.join(library_names))}

From e20b904b6b64ef65ac1393bc92709832ec71f644 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Wed, 10 Dec 2014 20:59:49 -0800 Subject: [PATCH 15/23] Fix: don't need to reload the whole page to refresh_children from the container view --- .../xmodule/public/js/library_content_edit.js | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/common/lib/xmodule/xmodule/public/js/library_content_edit.js b/common/lib/xmodule/xmodule/public/js/library_content_edit.js index 2db019feddf2..89011789b99b 100644 --- a/common/lib/xmodule/xmodule/public/js/library_content_edit.js +++ b/common/lib/xmodule/xmodule/public/js/library_content_edit.js @@ -1,10 +1,11 @@ /* JavaScript for special editing operations that can be done on LibraryContentXBlock */ window.LibraryContentAuthorView = function (runtime, element) { "use strict"; - var usage_id = $(element).data('usage-id'); + var $element = $(element); + var usage_id = $element.data('usage-id'); // The "Update Now" button is not a child of 'element', as it is in the validation message area // But it is still inside this xblock's wrapper element, which we can easily find: - var $wrapper = $(element).parents('*[data-locator="'+usage_id+'"]'); + var $wrapper = $element.parents('*[data-locator="'+usage_id+'"]'); // We can't bind to the button itself because in the bok choy test environment, // it may not yet exist at this point in time... not sure why. @@ -21,12 +22,15 @@ window.LibraryContentAuthorView = function (runtime, element) { state: 'end', element: element }); - // runtime.refreshXBlock(element); - // The above does not work, because this XBlock's runtime has no reference - // to the page (XBlockContainerPage). Only the Vertical XBlock's runtime has - // a reference to the page, and we have no way of getting a reference to it. - // So instead we: - location.reload(); + if ($element.closest('.wrapper-xblock').is(':not(.level-page)')) { + // We are on a course unit page. The notify('save') should refresh this block, + // but that is only working on the container page view of this block. + // Why? On the unit page, this XBlock's runtime has no reference to the + // XBlockContainerPage - only the top-level XBlock (a vertical) runtime does. + // But unfortunately there is no way to get a reference to our parent block's + // JS 'runtime' object. So instead we must refresh the whole page: + location.reload(); + } }); }); }; From 66f5f8f25799817d60d9bc3a9a207ec304ce233d Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Thu, 11 Dec 2014 00:40:08 -0800 Subject: [PATCH 16/23] Refresh children automatically when library setting is changed --- .../xmodule/xmodule/library_content_module.py | 24 ++++++++++++++++--- .../test/acceptance/pages/studio/container.py | 8 +++++++ .../test/acceptance/tests/lms/test_library.py | 1 - .../studio/test_studio_library_container.py | 22 ++++++++++++----- 4 files changed, 45 insertions(+), 10 deletions(-) diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index d9e28e93fd09..f89f147330dd 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -363,7 +363,7 @@ class LibraryContentDescriptor(LibraryContentFields, MakoModuleDescriptor, XmlDe js_module_name = "VerticalDescriptor" @XBlock.handler - def refresh_children(self, request, suffix): # pylint: disable=unused-argument + def refresh_children(self, request, suffix, update_db=True): # pylint: disable=unused-argument """ Refresh children: This method is to be used when any of the libraries that this block @@ -375,8 +375,12 @@ def refresh_children(self, request, suffix): # pylint: disable=unused-argument This method will update this block's 'source_libraries' field to store the version number of the libraries used, so we easily determine if this block is up to date or not. + + If update_db is True (default), this will explicitly persist the changes + to the modulestore by calling update_item() """ - user_id = self.runtime.service(self, 'user').user_id + user_service = self.runtime.service(self, 'user') + user_id = user_service.user_id if user_service else None # May be None when creating bok choy test fixtures root_children = [] store = self.system.modulestore @@ -395,6 +399,8 @@ def refresh_children(self, request, suffix): # pylint: disable=unused-argument new_libraries = [] for library_key, old_version in self.source_libraries: # pylint: disable=unused-variable library = _get_library(self.system.modulestore, library_key) # pylint: disable=protected-access + if library is None: + raise ValueError("Required library not found.") def copy_children_recursively(from_block): """ @@ -434,9 +440,21 @@ def copy_children_recursively(from_block): new_libraries.append(LibraryVersionReference(library_key, library.location.library_key.version_guid)) self.source_libraries = new_libraries self.children = root_children # pylint: disable=attribute-defined-outside-init - self.system.modulestore.update_item(self, user_id) + if update_db: + self.system.modulestore.update_item(self, user_id) return Response() + def editor_saved(self, user, old_metadata, old_content): + """ + If source_libraries has been edited, refresh_children automatically. + """ + old_source_libraries = LibraryList().from_json(old_metadata.get('source_libraries', [])) + if set(old_source_libraries) != set(self.source_libraries): + try: + self.refresh_children(None, None, update_db=False) # update_db=False since update_item() is about to be called anyways + except ValueError: + pass # The validation area will display an error message, no need to do anything now. + def has_dynamic_children(self): """ Inform the runtime that our children vary per-user. diff --git a/common/test/acceptance/pages/studio/container.py b/common/test/acceptance/pages/studio/container.py index dc93be67f561..d8a760cac972 100644 --- a/common/test/acceptance/pages/studio/container.py +++ b/common/test/acceptance/pages/studio/container.py @@ -309,6 +309,14 @@ def student_content(self): """ return self.q(css=self._bounded_selector('.xblock-student_view'))[0].text + @property + def author_content(self): + """ + Returns the text content of the xblock as displayed on the container page. + (For blocks which implement a distinct author_view). + """ + return self.q(css=self._bounded_selector('.xblock-author_view'))[0].text + @property def name(self): titles = self.q(css=self._bounded_selector(self.NAME_SELECTOR)).text diff --git a/common/test/acceptance/tests/lms/test_library.py b/common/test/acceptance/tests/lms/test_library.py index 78d699faa6fd..f83e6b94e9d5 100644 --- a/common/test/acceptance/tests/lms/test_library.py +++ b/common/test/acceptance/tests/lms/test_library.py @@ -92,7 +92,6 @@ def _refresh_library_content_children(self, count=1): modal = StudioLibraryContentXBlockEditModal(library_container_block.edit()) modal.count = count library_container_block.save_settings() - library_container_block.refresh_children() self._go_to_unit_page(change_login=False) unit_page.wait_for_page() unit_page.publish_action.click() diff --git a/common/test/acceptance/tests/studio/test_studio_library_container.py b/common/test/acceptance/tests/studio/test_studio_library_container.py index ba66bdd7b1ec..6e8fddeb8efb 100644 --- a/common/test/acceptance/tests/studio/test_studio_library_container.py +++ b/common/test/acceptance/tests/studio/test_studio_library_container.py @@ -136,19 +136,29 @@ def test_out_of_date_message(self): """ Scenario: Given I have a library, a course and library content xblock in a course When I go to studio unit page for library content block + Then I update the library being used + Then I refresh the page Then I can see that library content block needs to be updated When I click on the update link Then I can see that the content no longer needs to be updated """ expected_text = "This component is out of date. The library has new content." - library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) + library_block = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) + + self.assertFalse(library_block.has_validation_warning) + self.assertIn("3 matching components", library_block.author_content) + + self.library_fixture.create_xblock(self.library_fixture.library_location, XBlockFixtureDesc("html", "Html4")) - self.assertTrue(library_container.has_validation_warning) - self.assertIn(expected_text, library_container.validation_warning_text) + self.unit_page.visit() # Reload the page - library_container.refresh_children() + self.assertTrue(library_block.has_validation_warning) + self.assertIn(expected_text, library_block.validation_warning_text) + + library_block.refresh_children() self.unit_page.wait_for_page() # Wait for the page to reload - library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) + library_block = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) - self.assertFalse(library_container.has_validation_message) + self.assertFalse(library_block.has_validation_message) + self.assertIn("4 matching components", library_block.author_content) From c2f757ffe0dfe52f2f42ee39a403e538e0fa344b Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Thu, 11 Dec 2014 15:10:38 -0800 Subject: [PATCH 17/23] Fix greedy intrusion of split_test documentation --- cms/templates/container.html | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cms/templates/container.html b/cms/templates/container.html index de395457f24b..e36914506b1b 100644 --- a/cms/templates/container.html +++ b/cms/templates/container.html @@ -103,7 +103,7 @@

${_("Page Actions")}