diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index 8698feb6d9c6..76b809a96dec 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -423,3 +423,63 @@ def test_inheritance_in_locked_subsection(self): def test_no_inheritance_for_orphan(self): """Tests that an orphaned xblock does not inherit staff lock""" self.assertFalse(utils.ancestor_has_staff_lock(self.orphan)) + + +class GroupVisibilityTest(CourseTestCase): + """ + Test content group access rules. + """ + def setUp(self): + super(GroupVisibilityTest, self).setUp() + + chapter = ItemFactory.create(category='chapter', parent_location=self.course.location) + sequential = ItemFactory.create(category='sequential', parent_location=chapter.location) + vertical = ItemFactory.create(category='vertical', parent_location=sequential.location) + html = ItemFactory.create(category='html', parent_location=vertical.location) + problem = ItemFactory.create( + category='problem', parent_location=vertical.location, data="" + ) + self.sequential = self.store.get_item(sequential.location) + self.vertical = self.store.get_item(vertical.location) + self.html = self.store.get_item(html.location) + self.problem = self.store.get_item(problem.location) + + def set_group_access(self, xblock, value): + """ Sets group_access to specified value and calls update_item to persist the change. """ + xblock.group_access = value + self.store.update_item(xblock, self.user.id) + + def test_no_visibility_set(self): + """ Tests when group_access has not been set on anything. """ + + def verify_all_components_visible_to_all(): # pylint: disable=invalid-name + """ Verifies when group_access has not been set on anything. """ + for item in (self.sequential, self.vertical, self.html, self.problem): + self.assertFalse(utils.has_children_visible_to_specific_content_groups(item)) + self.assertFalse(utils.is_visible_to_specific_content_groups(item)) + + verify_all_components_visible_to_all() + + # Test with group_access set to Falsey values. + self.set_group_access(self.vertical, {1: []}) + self.set_group_access(self.html, {2: None}) + + verify_all_components_visible_to_all() + + def test_sequential_and_problem_have_group_access(self): + """ Tests when group_access is set on a few different components. """ + self.set_group_access(self.sequential, {1: [0]}) + # This is a no-op. + self.set_group_access(self.vertical, {1: []}) + self.set_group_access(self.problem, {2: [3, 4]}) + + # Note that "has_children_visible_to_specific_content_groups" only checks immediate children. + self.assertFalse(utils.has_children_visible_to_specific_content_groups(self.sequential)) + self.assertTrue(utils.has_children_visible_to_specific_content_groups(self.vertical)) + self.assertFalse(utils.has_children_visible_to_specific_content_groups(self.html)) + self.assertFalse(utils.has_children_visible_to_specific_content_groups(self.problem)) + + self.assertTrue(utils.is_visible_to_specific_content_groups(self.sequential)) + self.assertFalse(utils.is_visible_to_specific_content_groups(self.vertical)) + self.assertFalse(utils.is_visible_to_specific_content_groups(self.html)) + self.assertTrue(utils.is_visible_to_specific_content_groups(self.problem)) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 914ad1ec65d4..b64bbc74ca3b 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -178,6 +178,36 @@ def is_currently_visible_to_students(xblock): return True +def has_children_visible_to_specific_content_groups(xblock): + """ + Returns True if this xblock has children that are limited to specific content groups. + Note that this method is not recursive (it does not check grandchildren). + """ + if not xblock.has_children: + return False + + for child in xblock.get_children(): + if is_visible_to_specific_content_groups(child): + return True + + return False + + +def is_visible_to_specific_content_groups(xblock): + """ + Returns True if this xblock has visibility limited to specific content groups. + """ + if not xblock.group_access: + return False + for __, value in xblock.group_access.iteritems(): + # value should be a list of group IDs. If it is an empty list or None, the xblock is visible + # to all groups in that particular partition. So if value is a truthy value, the xblock is + # restricted in some way. + if value: + return True + return False + + def find_release_date_source(xblock): """ Finds the ancestor of xblock that set its release date. diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 70a470f9dc76..db5e9847e89d 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -21,7 +21,7 @@ from contentstore.utils import get_lms_link_for_item from contentstore.views.helpers import get_parent_xblock, is_unit, xblock_type_display_name -from contentstore.views.item import create_xblock_info +from contentstore.views.item import create_xblock_info, add_container_page_publishing_info from opaque_keys.edx.keys import UsageKey @@ -177,8 +177,9 @@ def container_handler(request, usage_key_string): # about the block's ancestors and siblings for use by the Unit Outline. xblock_info = create_xblock_info(xblock, include_ancestor_info=is_unit_page) - # Create the link for preview. - preview_lms_base = settings.FEATURES.get('PREVIEW_LMS_BASE') + if is_unit_page: + add_container_page_publishing_info(xblock, xblock_info) + # need to figure out where this item is in the list of children as the # preview will need this index = 1 diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index cbc40b6f31e9..a0d8113d1888 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -1343,7 +1343,7 @@ def group_configurations_list_handler(request, course_key_string): 'context_course': course, 'group_configuration_url': group_configuration_url, 'course_outline_url': course_outline_url, - 'configurations': configurations if should_show_group_configurations_page(course) else None, + 'configurations': configurations, }) elif "application/json" in request.META.get('HTTP_ACCEPT'): if request.method == 'POST': @@ -1422,16 +1422,6 @@ def group_configurations_detail_handler(request, course_key_string, group_config return JsonResponse(status=204) -def should_show_group_configurations_page(course): - """ - Returns true if Studio should show the "Group Configurations" page for the specified course. - """ - return ( - SPLIT_TEST_COMPONENT_TYPE in ADVANCED_COMPONENT_TYPES and - SPLIT_TEST_COMPONENT_TYPE in course.advanced_modules - ) - - def _get_course_creator_status(user): """ Helper method for returning the course creator status for a particular user, diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index a15fb26c5ff7..9b7a9803d397 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -39,7 +39,7 @@ from student.auth import has_course_author_access from contentstore.utils import find_release_date_source, find_staff_lock_source, is_currently_visible_to_students, \ - ancestor_has_staff_lock + ancestor_has_staff_lock, has_children_visible_to_specific_content_groups from contentstore.views.helpers import is_unit, xblock_studio_url, xblock_primary_child_category, \ xblock_type_display_name, get_parent_xblock from contentstore.views.preview import get_preview_fragment @@ -47,8 +47,11 @@ from models.settings.course_grading import CourseGradingModel from cms.lib.xblock.runtime import handler_url, local_resource_url from opaque_keys.edx.keys import UsageKey, CourseKey +from cms.lib.xblock.authoring_mixin import VISIBILITY_VIEW -__all__ = ['orphan_handler', 'xblock_handler', 'xblock_view_handler', 'xblock_outline_handler'] +__all__ = [ + 'orphan_handler', 'xblock_handler', 'xblock_view_handler', 'xblock_outline_handler', 'xblock_container_handler' +] log = logging.getLogger(__name__) @@ -58,7 +61,6 @@ NEVER = lambda x: False ALWAYS = lambda x: True - # In order to allow descriptors to use a handler url, we need to # monkey-patch the x_module library. # TODO: Remove this code when Runtimes are no longer created by modulestores @@ -142,8 +144,8 @@ def xblock_handler(request, usage_key_string): return JsonResponse(CourseGradingModel.get_section_grader_type(usage_key)) # TODO: pass fields to _get_module_info and only return those with modulestore().bulk_operations(usage_key.course_key): - rsp = _get_module_info(_get_xblock(usage_key, request.user)) - return JsonResponse(rsp) + response = _get_module_info(_get_xblock(usage_key, request.user)) + return JsonResponse(response) else: return HttpResponse(status=406) @@ -216,14 +218,14 @@ def xblock_view_handler(request, usage_key_string, view_name): request_token=request_token(request), )) - if view_name == STUDIO_VIEW: + if view_name in (STUDIO_VIEW, VISIBILITY_VIEW): try: - fragment = xblock.render(STUDIO_VIEW) + fragment = xblock.render(view_name) # catch exceptions indiscriminately, since after this point they escape the # dungeon and surface as uneditable, unsaveable, and undeletable # component-goblins. except Exception as exc: # pylint: disable=broad-except - log.debug("unable to render studio_view for %r", xblock, exc_info=True) + log.debug("Unable to render %s for %r", view_name, xblock, exc_info=True) fragment = Fragment(render_to_string('html_error.html', {'message': str(exc)})) elif view_name in (PREVIEW_VIEWS + container_views): @@ -301,6 +303,32 @@ def xblock_outline_handler(request, usage_key_string): return Http404 +# pylint: disable=unused-argument +@require_http_methods(("GET")) +@login_required +@expect_json +def xblock_container_handler(request, usage_key_string): + """ + The restful handler for requests for XBlock information about the block and its children. + This is used by the container page in particular to get additional information about publish state + and ancestor state. + """ + usage_key = usage_key_with_run(usage_key_string) + + if not has_course_author_access(request.user, usage_key.course_key): + raise PermissionDenied() + + response_format = request.REQUEST.get('format', 'html') + if response_format == 'json' or 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'): + with modulestore().bulk_operations(usage_key.course_key): + response = _get_module_info( + _get_xblock(usage_key, request.user), include_ancestor_info=True, include_publishing_info=True + ) + return JsonResponse(response) + else: + return Http404 + + def _update_with_callback(xblock, user, old_metadata=None, old_content=None): """ Updates the xblock in the modulestore. @@ -635,7 +663,7 @@ def _get_xblock(usage_key, user): return JsonResponse({"error": "Can't find item by location: " + unicode(usage_key)}, 404) -def _get_module_info(xblock, rewrite_static_links=True): +def _get_module_info(xblock, rewrite_static_links=True, include_ancestor_info=False, include_publishing_info=False): """ metadata, data, id representation of a leaf module fetcher. :param usage_key: A UsageKey @@ -653,7 +681,12 @@ def _get_module_info(xblock, rewrite_static_links=True): 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) + xblock_info = create_xblock_info( + xblock, data=data, metadata=own_metadata(xblock), include_ancestor_info=include_ancestor_info + ) + if include_publishing_info: + add_container_page_publishing_info(xblock, xblock_info) + return xblock_info def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=False, include_child_info=False, @@ -673,24 +706,6 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F In addition, an optional include_children_predicate argument can be provided to define whether or not a particular xblock should have its children included. """ - - def safe_get_username(user_id): - """ - Guard against bad user_ids, like the infamous "**replace_user**". - Note that this will ignore our special known IDs (ModuleStoreEnum.UserID). - We should consider adding special handling for those values. - - :param user_id: the user id to get the username of - :return: username, or None if the user does not exist or user_id is None - """ - if user_id: - try: - return User.objects.get(id=user_id).username - except: # pylint: disable=bare-except - pass - - return None - 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 @@ -710,8 +725,6 @@ def safe_get_username(user_id): else: child_info = None - # Treat DEFAULT_START_DATE as a magic number that means the release date has not been set - release_date = get_default_time_display(xblock.start) if xblock.start != DEFAULT_START_DATE else None if xblock.category != 'course': visibility_state = _compute_visibility_state(xblock, child_info, is_xblock_unit and has_changes) else: @@ -727,7 +740,7 @@ def safe_get_username(user_id): "published_on": get_default_time_display(xblock.published_on) if 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, + "release_date": _get_release_date(xblock), "visibility_state": visibility_state, "has_explicit_staff_lock": xblock.fields['visible_to_staff_only'].is_set_on(xblock), "start": xblock.fields['start'].to_json(xblock.start), @@ -751,19 +764,6 @@ def safe_get_username(user_id): else: xblock_info["ancestor_has_staff_lock"] = False - # Currently, 'edited_by', 'published_by', and 'release_date_from' are only used by the - # container page when rendering a unit. Since they are expensive to compute, only include them for units - # that are not being rendered on the course outline. - if is_xblock_unit and not course_outline: - xblock_info["edited_by"] = safe_get_username(xblock.subtree_edited_by) - xblock_info["published_by"] = safe_get_username(xblock.published_by) - xblock_info["currently_visible_to_students"] = is_currently_visible_to_students(xblock) - if release_date: - xblock_info["release_date_from"] = _get_release_date_from(xblock) - if visibility_state == VisibilityState.staff_only: - xblock_info["staff_lock_from"] = _get_staff_lock_from(xblock) - else: - xblock_info["staff_lock_from"] = None if course_outline: if xblock_info["has_explicit_staff_lock"]: xblock_info["staff_only_message"] = True @@ -775,6 +775,40 @@ def safe_get_username(user_id): return xblock_info +def add_container_page_publishing_info(xblock, xblock_info): # pylint: disable=invalid-name + """ + Adds information about the xblock's publish state to the supplied + xblock_info for the container page. + """ + def safe_get_username(user_id): + """ + Guard against bad user_ids, like the infamous "**replace_user**". + Note that this will ignore our special known IDs (ModuleStoreEnum.UserID). + We should consider adding special handling for those values. + + :param user_id: the user id to get the username of + :return: username, or None if the user does not exist or user_id is None + """ + if user_id: + try: + return User.objects.get(id=user_id).username + except: # pylint: disable=bare-except + pass + + return None + + xblock_info["edited_by"] = safe_get_username(xblock.subtree_edited_by) + xblock_info["published_by"] = safe_get_username(xblock.published_by) + xblock_info["currently_visible_to_students"] = is_currently_visible_to_students(xblock) + xblock_info["has_content_group_components"] = has_children_visible_to_specific_content_groups(xblock) + if xblock_info["release_date"]: + xblock_info["release_date_from"] = _get_release_date_from(xblock) + if xblock_info["visibility_state"] == VisibilityState.staff_only: + xblock_info["staff_lock_from"] = _get_staff_lock_from(xblock) + else: + xblock_info["staff_lock_from"] = None + + class VisibilityState(object): """ Represents the possible visibility states for an xblock: @@ -894,6 +928,14 @@ def _create_xblock_child_info(xblock, course_outline, graders, include_children_ return child_info +def _get_release_date(xblock): + """ + Returns the release date for the xblock, or None if the release date has never been set. + """ + # Treat DEFAULT_START_DATE as a magic number that means the release date has not been set + return get_default_time_display(xblock.start) if xblock.start != DEFAULT_START_DATE else None + + def _get_release_date_from(xblock): """ Returns a string representation of the section or subsection that sets the xblock's release date diff --git a/cms/djangoapps/contentstore/views/tests/test_group_configurations.py b/cms/djangoapps/contentstore/views/tests/test_group_configurations.py index 5c0e5130d179..411c17625a55 100644 --- a/cms/djangoapps/contentstore/views/tests/test_group_configurations.py +++ b/cms/djangoapps/contentstore/views/tests/test_group_configurations.py @@ -208,17 +208,6 @@ def test_view_index_ok(self): self.assertContains(response, 'First name') self.assertContains(response, 'Group C') - def test_view_index_disabled(self): - """ - Check that group configuration page is not displayed when turned off. - """ - if SPLIT_TEST_COMPONENT_TYPE in self.course.advanced_modules: - self.course.advanced_modules.remove(SPLIT_TEST_COMPONENT_TYPE) - self.store.update_item(self.course, self.user.id) - - resp = self.client.get(self._url()) - self.assertContains(resp, "module is disabled") - def test_unsupported_http_accept_header(self): """ Test if not allowed header present in request. diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 0ec957fb4483..6b5ef558a779 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -18,7 +18,9 @@ component_handler, get_component_templates ) -from contentstore.views.item import create_xblock_info, ALWAYS, VisibilityState, _xblock_type_and_display_name +from contentstore.views.item import ( + create_xblock_info, ALWAYS, VisibilityState, _xblock_type_and_display_name, add_container_page_publishing_info +) from contentstore.tests.utils import CourseTestCase from student.tests.factories import UserFactory from xmodule.capa_module import CapaDescriptor @@ -100,20 +102,9 @@ def _get_container_preview(self, usage_key): return html, resources @ddt.data( - # chapter explanation: - # 1-3. get course, chapter, chapter's children, - # 4-7. chapter's published grandchildren, chapter's draft grandchildren, published & then draft greatgrand - # 8 compute chapter's parent - # 9 get chapter's parent - # 10-16. run queries 2-8 again - # 17-19. compute seq, vert, and problem's parents (odd since it's going down; so, it knows) - # 20-22. get course 3 times - # 23. get chapter - # 24. compute chapter's parent (course) - # 25. compute course's parent (None) - (1, 20, 20, 26, 26), - (2, 21, 21, 29, 28), - (3, 22, 22, 32, 30), + (1, 16, 14, 15, 11), + (2, 16, 14, 15, 11), + (3, 16, 14, 15, 11), ) @ddt.unpack def test_get_query_count(self, branching_factor, chapter_queries, section_queries, unit_queries, problem_queries): @@ -128,6 +119,17 @@ def test_get_query_count(self, branching_factor, chapter_queries, section_querie with check_mongo_calls(problem_queries): self.client.get(reverse_usage_url('xblock_handler', self.populated_usage_keys['problem'][-1])) + @ddt.data( + (1, 26), + (2, 28), + (3, 30), + ) + @ddt.unpack + def test_container_get_query_count(self, branching_factor, unit_queries,): + self.populate_course(branching_factor) + with check_mongo_calls(unit_queries): + self.client.get(reverse_usage_url('xblock_container_handler', self.populated_usage_keys['vertical'][-1])) + def test_get_vertical(self): # Add a vertical resp = self.create_xblock(category='vertical') @@ -1330,6 +1332,7 @@ def test_vertical_xblock_info(self): include_children_predicate=ALWAYS, include_ancestor_info=True ) + add_container_page_publishing_info(vertical, xblock_info) self.validate_vertical_xblock_info(xblock_info) def test_component_xblock_info(self): @@ -1450,10 +1453,6 @@ def validate_xblock_info_consistency(self, xblock_info, has_ancestor_info=False, ) else: self.assertIsNone(xblock_info.get('child_info', None)) - if xblock_info['category'] == 'vertical' and not course_outline: - self.assertEqual(xblock_info['edited_by'], 'testuser') - else: - self.assertIsNone(xblock_info.get('edited_by', None)) class TestXBlockPublishingInfo(ItemTest): @@ -1474,7 +1473,8 @@ def _create_child(self, parent, category, display_name, publish_item=False, staf ) if staff_only: self._enable_staff_only(child.location) - return child + # In case the staff_only state was set, return the updated xblock. + return modulestore().get_item(child.location) def _get_child_xblock_info(self, xblock_info, index): """ @@ -1563,12 +1563,6 @@ def _verify_explicit_staff_lock_state(self, xblock_info, expected_state, path=No """ self._verify_xblock_info_state(xblock_info, 'has_explicit_staff_lock', expected_state, path, should_equal) - def _verify_staff_lock_from_state(self, xblock_info, expected_state, path=None, should_equal=True): - """ - Verify the staff_lock_from state of an item in the xblock_info. - """ - self._verify_xblock_info_state(xblock_info, 'staff_lock_from', expected_state, path, should_equal) - def test_empty_chapter(self): empty_chapter = self._create_child(self.course, 'chapter', "Empty Chapter") xblock_info = self._get_xblock_info(empty_chapter.location) @@ -1658,7 +1652,7 @@ def test_staff_only_section(self): """ chapter = self._create_child(self.course, 'chapter', "Test Chapter", staff_only=True) sequential = self._create_child(chapter, 'sequential', "Test Sequential") - self._create_child(sequential, 'vertical', "Unit") + vertical = self._create_child(sequential, 'vertical', "Unit") xblock_info = self._get_xblock_info(chapter.location) self._verify_visibility_state(xblock_info, VisibilityState.staff_only) self._verify_visibility_state(xblock_info, VisibilityState.staff_only, path=self.FIRST_SUBSECTION_PATH) @@ -1668,7 +1662,9 @@ def test_staff_only_section(self): self._verify_explicit_staff_lock_state(xblock_info, False, path=self.FIRST_SUBSECTION_PATH) self._verify_explicit_staff_lock_state(xblock_info, False, path=self.FIRST_UNIT_PATH) - self._verify_staff_lock_from_state(xblock_info, _xblock_type_and_display_name(chapter), path=self.FIRST_UNIT_PATH) + vertical_info = self._get_xblock_info(vertical.location) + add_container_page_publishing_info(vertical, vertical_info) + self.assertEqual(_xblock_type_and_display_name(chapter), vertical_info["staff_lock_from"]) def test_no_staff_only_section(self): """ @@ -1689,7 +1685,7 @@ def test_staff_only_subsection(self): """ chapter = self._create_child(self.course, 'chapter', "Test Chapter") sequential = self._create_child(chapter, 'sequential', "Test Sequential", staff_only=True) - self._create_child(sequential, 'vertical', "Unit") + vertical = self._create_child(sequential, 'vertical', "Unit") xblock_info = self._get_xblock_info(chapter.location) self._verify_visibility_state(xblock_info, VisibilityState.staff_only) self._verify_visibility_state(xblock_info, VisibilityState.staff_only, path=self.FIRST_SUBSECTION_PATH) @@ -1699,7 +1695,9 @@ def test_staff_only_subsection(self): self._verify_explicit_staff_lock_state(xblock_info, True, path=self.FIRST_SUBSECTION_PATH) self._verify_explicit_staff_lock_state(xblock_info, False, path=self.FIRST_UNIT_PATH) - self._verify_staff_lock_from_state(xblock_info, _xblock_type_and_display_name(sequential), path=self.FIRST_UNIT_PATH) + vertical_info = self._get_xblock_info(vertical.location) + add_container_page_publishing_info(vertical, vertical_info) + self.assertEqual(_xblock_type_and_display_name(sequential), vertical_info["staff_lock_from"]) def test_no_staff_only_subsection(self): """ @@ -1717,7 +1715,7 @@ def test_no_staff_only_subsection(self): def test_staff_only_unit(self): chapter = self._create_child(self.course, 'chapter', "Test Chapter") sequential = self._create_child(chapter, 'sequential', "Test Sequential") - unit = self._create_child(sequential, 'vertical', "Unit", staff_only=True) + vertical = self._create_child(sequential, 'vertical', "Unit", staff_only=True) xblock_info = self._get_xblock_info(chapter.location) self._verify_visibility_state(xblock_info, VisibilityState.staff_only) self._verify_visibility_state(xblock_info, VisibilityState.staff_only, path=self.FIRST_SUBSECTION_PATH) @@ -1727,7 +1725,9 @@ def test_staff_only_unit(self): self._verify_explicit_staff_lock_state(xblock_info, False, path=self.FIRST_SUBSECTION_PATH) self._verify_explicit_staff_lock_state(xblock_info, True, path=self.FIRST_UNIT_PATH) - self._verify_staff_lock_from_state(xblock_info, _xblock_type_and_display_name(unit), path=self.FIRST_UNIT_PATH) + vertical_info = self._get_xblock_info(vertical.location) + add_container_page_publishing_info(vertical, vertical_info) + self.assertEqual(_xblock_type_and_display_name(vertical), vertical_info["staff_lock_from"]) def test_unscheduled_section_with_live_subsection(self): chapter = self._create_child(self.course, 'chapter', "Test Chapter") diff --git a/cms/envs/common.py b/cms/envs/common.py index 6b044d388d73..9d86835ad308 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -37,6 +37,7 @@ from warnings import simplefilter from lms.djangoapps.lms_xblock.mixin import LmsBlockMixin +from cms.lib.xblock.authoring_mixin import AuthoringMixin from dealer.git import git from xmodule.modulestore.edit_info import EditInfoMixin @@ -263,7 +264,13 @@ # This should be moved into an XBlock Runtime/Application object # once the responsibility of XBlock creation is moved out of modulestore - cpennington -XBLOCK_MIXINS = (LmsBlockMixin, InheritanceMixin, XModuleMixin, EditInfoMixin) +XBLOCK_MIXINS = ( + LmsBlockMixin, + InheritanceMixin, + XModuleMixin, + EditInfoMixin, + AuthoringMixin, +) # Allow any XBlock in Studio # You should also enable the ALLOW_ALL_ADVANCED_COMPONENTS feature flag, so that diff --git a/cms/lib/xblock/authoring_mixin.py b/cms/lib/xblock/authoring_mixin.py new file mode 100644 index 000000000000..a4bbf472e8e5 --- /dev/null +++ b/cms/lib/xblock/authoring_mixin.py @@ -0,0 +1,49 @@ +""" +Mixin class that provides authoring capabilities for XBlocks. +""" + +import logging + +from xblock.core import XBlock +from xblock.fields import XBlockMixin +from xblock.fragment import Fragment + +log = logging.getLogger(__name__) + +VISIBILITY_VIEW = 'visibility_view' + + +@XBlock.needs("i18n") +class AuthoringMixin(XBlockMixin): + """ + Mixin class that provides authoring capabilities for XBlocks. + """ + _services_requested = { + 'i18n': 'need', + } + + def _get_studio_resource_url(self, relative_url): + """ + Returns the Studio URL to a static resource. + """ + # TODO: is there a cleaner way to do this? + from cms.envs.common import STATIC_URL + return STATIC_URL + relative_url + + def visibility_view(self, _context=None): + """ + Render the view to manage an xblock's visibility settings in Studio. + Args: + _context: Not actively used for this view. + Returns: + (Fragment): An HTML fragment for editing the visibility of this XBlock. + """ + fragment = Fragment() + from contentstore.utils import reverse_course_url + fragment.add_content(self.system.render_template('visibility_editor.html', { + 'xblock': self, + 'manage_groups_url': reverse_course_url('group_configurations_list_handler', self.location.course_key), + })) + fragment.add_javascript_url(self._get_studio_resource_url('/js/xblock/authoring.js')) + fragment.initialize_js('VisibilityEditorInit') + return fragment diff --git a/cms/lib/xblock/test/test_authoring_mixin.py b/cms/lib/xblock/test/test_authoring_mixin.py new file mode 100644 index 000000000000..574c5c761f15 --- /dev/null +++ b/cms/lib/xblock/test/test_authoring_mixin.py @@ -0,0 +1,121 @@ +from openedx.core.djangoapps.course_groups.partition_scheme import CohortPartitionScheme +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.partitions.partitions import Group, UserPartition + + +class AuthoringMixinTestCase(ModuleStoreTestCase): + """ + Tests the studio authoring XBlock mixin. + """ + def setUp(self): + """ + Create a simple course with a video component. + """ + super(AuthoringMixinTestCase, self).setUp() + self.course = CourseFactory.create() + chapter = ItemFactory.create( + category='chapter', + parent_location=self.course.location, + display_name='Test Chapter' + ) + sequential = ItemFactory.create( + category='sequential', + parent_location=chapter.location, + display_name='Test Sequential' + ) + self.vertical = ItemFactory.create( + category='vertical', + parent_location=sequential.location, + display_name='Test Vertical' + ) + self.video = ItemFactory.create( + category='video', + parent_location=self.vertical.location, + display_name='Test Vertical' + ) + self.pet_groups = [Group(1, 'Cat Lovers'), Group(2, 'Dog Lovers')] + + def create_cohorted_content_groups(self, groups): + """ + Create a cohorted content partition with specified groups. + """ + self.content_partition = UserPartition( + 1, + 'Content Groups', + 'Contains Groups for Cohorted Courseware', + groups, + scheme_id='cohort' + ) + self.course.user_partitions = [self.content_partition] + self.store.update_item(self.course, self.user.id) + + def set_staff_only(self, item): + """Make an item visible to staff only.""" + item.visible_to_staff_only = True + self.store.update_item(item, self.user.id) + + def set_group_access(self, item, group_ids): + """ + Set group_access for the specified item to the specified group + ids within the content partition. + """ + item.group_access[self.content_partition.id] = group_ids + self.store.update_item(item, self.user.id) + + def verify_visibility_view_contains(self, item, substrings): + """ + Verify that an item's visibility view returns an html string + containing all the expected substrings. + """ + html = item.visibility_view().body_html() + for string in substrings: + self.assertIn(string, html) + + def test_html_no_partition(self): + self.verify_visibility_view_contains(self.video, 'You have not set up any groups to manage visibility with.') + + def test_html_empty_partition(self): + self.create_cohorted_content_groups([]) + self.verify_visibility_view_contains(self.video, 'You have not set up any groups to manage visibility with.') + + def test_html_populated_partition(self): + self.create_cohorted_content_groups(self.pet_groups) + self.verify_visibility_view_contains(self.video, ['Cat Lovers', 'Dog Lovers']) + + def test_html_no_partition_staff_locked(self): + self.set_staff_only(self.vertical) + self.verify_visibility_view_contains(self.video, ['You have not set up any groups to manage visibility with.']) + + def test_html_empty_partition_staff_locked(self): + self.create_cohorted_content_groups([]) + self.set_staff_only(self.vertical) + self.verify_visibility_view_contains(self.video, 'You have not set up any groups to manage visibility with.') + + def test_html_populated_partition_staff_locked(self): + self.create_cohorted_content_groups(self.pet_groups) + self.set_staff_only(self.vertical) + self.verify_visibility_view_contains( + self.video, ['The Unit this component is contained in is hidden from students.', 'Cat Lovers', 'Dog Lovers'] + ) + + def test_html_false_content_group(self): + self.create_cohorted_content_groups(self.pet_groups) + self.set_group_access(self.video, ['false_group_id']) + self.verify_visibility_view_contains( + self.video, ['Cat Lovers', 'Dog Lovers', 'Content group no longer exists.'] + ) + + def test_html_false_content_group_staff_locked(self): + self.create_cohorted_content_groups(self.pet_groups) + self.set_staff_only(self.vertical) + self.set_group_access(self.video, ['false_group_id']) + self.verify_visibility_view_contains( + self.video, + [ + 'Cat Lovers', + 'Dog Lovers', + 'The Unit this component is contained in is hidden from students.', + 'Content group no longer exists.' + ] + ) diff --git a/cms/static/coffee/src/views/module_edit.coffee b/cms/static/coffee/src/views/module_edit.coffee index b839ade44698..b04495f8c52a 100644 --- a/cms/static/coffee/src/views/module_edit.coffee +++ b/cms/static/coffee/src/views/module_edit.coffee @@ -52,7 +52,5 @@ define ["jquery", "underscore", "gettext", "xblock/runtime.v1", clickEditButton: (event) -> event.preventDefault() - modal = new EditXBlockModal({ - view: 'student_view' - }); + modal = new EditXBlockModal(); modal.edit(this.$el, self.model, { refresh: _.bind(@render, this) }) diff --git a/cms/static/js/factories/container.js b/cms/static/js/factories/container.js index 93cdeb8fd991..0f8c102b797e 100644 --- a/cms/static/js/factories/container.js +++ b/cms/static/js/factories/container.js @@ -1,13 +1,13 @@ define([ - 'jquery', 'js/models/xblock_info', 'js/views/pages/container', + 'jquery', 'js/models/xblock_container_info', 'js/views/pages/container', 'js/collections/component_template', 'xmodule', 'coffee/src/main', 'xblock/cms.runtime.v1' ], -function($, XBlockInfo, ContainerPage, ComponentTemplates, xmoduleLoader) { +function($, XBlockContainerInfo, ContainerPage, ComponentTemplates, xmoduleLoader) { 'use strict'; return function (componentTemplates, XBlockInfoJson, action, isUnitPage) { var templates = new ComponentTemplates(componentTemplates, {parse: true}), - mainXBlockInfo = new XBlockInfo(XBlockInfoJson, {parse: true}); + mainXBlockInfo = new XBlockContainerInfo(XBlockInfoJson, {parse: true}); xmoduleLoader.done(function () { var view = new ContainerPage({ diff --git a/cms/static/js/models/custom_sync_xblock_info.js b/cms/static/js/models/custom_sync_xblock_info.js new file mode 100644 index 000000000000..da3b6b0c7a04 --- /dev/null +++ b/cms/static/js/models/custom_sync_xblock_info.js @@ -0,0 +1,10 @@ +define(["js/models/xblock_info"], + function(XBlockInfo) { + var CustomSyncXBlockInfo = XBlockInfo.extend({ + sync: function(method, model, options) { + options.url = (this.urlRoots[method] || this.urlRoot) + '/' + this.get('id'); + return XBlockInfo.prototype.sync.call(this, method, model, options); + } + }); + return CustomSyncXBlockInfo; + }); diff --git a/cms/static/js/models/xblock_container_info.js b/cms/static/js/models/xblock_container_info.js new file mode 100644 index 000000000000..f60f491d3803 --- /dev/null +++ b/cms/static/js/models/xblock_container_info.js @@ -0,0 +1,9 @@ +define(["js/models/custom_sync_xblock_info"], + function(CustomSyncXBlockInfo) { + var XBlockContainerInfo = CustomSyncXBlockInfo.extend({ + urlRoots: { + 'read': '/xblock/container' + } + }); + return XBlockContainerInfo; + }); diff --git a/cms/static/js/models/xblock_info.js b/cms/static/js/models/xblock_info.js index d2324313b5de..4e643f7d5fad 100644 --- a/cms/static/js/models/xblock_info.js +++ b/cms/static/js/models/xblock_info.js @@ -32,7 +32,8 @@ function(Backbone, _, str, ModuleUtils) { */ 'edited_on':null, /** - * User who last edited the xblock or any of its descendants. + * User who last edited the xblock or any of its descendants. Will only be present if + * publishing info was explicitly requested. */ 'edited_by':null, /** @@ -44,7 +45,8 @@ function(Backbone, _, str, ModuleUtils) { */ 'published_on': null, /** - * User who last published the xblock, or null if never published. + * User who last published the xblock, or null if never published. Will only be present if + * publishing info was explicitly requested. */ 'published_by': null, /** @@ -70,12 +72,14 @@ function(Backbone, _, str, ModuleUtils) { /** * The xblock which is determining the release date. For instance, for a unit, * this will either be the parent subsection or the grandparent section. - * This can be null if the release date is unscheduled. + * This can be null if the release date is unscheduled. Will only be present if + * publishing info was explicitly requested. */ 'release_date_from':null, /** * True if this xblock is currently visible to students. This is computed server-side - * so that the logic isn't duplicated on the client. + * so that the logic isn't duplicated on the client. Will only be present if + * publishing info was explicitly requested. */ 'currently_visible_to_students': null, /** @@ -114,13 +118,20 @@ function(Backbone, _, str, ModuleUtils) { /** * The xblock which is determining the staff lock value. For instance, for a unit, * this will either be the parent subsection or the grandparent section. - * This can be null if the xblock has no inherited staff lock. + * This can be null if the xblock has no inherited staff lock. Will only be present if + * publishing info was explicitly requested. */ 'staff_lock_from': null, /** * True iff this xblock should display a "Contains staff only content" message. */ - 'staff_only_message': null + 'staff_only_message': null, + /** + * True iff this xblock is a unit, and it has children that are only visible to certain + * content groups. Note that this is not a recursive property. Will only be present if + * publishing info was explicitly requested. + */ + 'has_content_group_components': null }, initialize: function () { diff --git a/cms/static/js/models/xblock_outline_info.js b/cms/static/js/models/xblock_outline_info.js index da34adb219c7..b90001c98e0d 100644 --- a/cms/static/js/models/xblock_outline_info.js +++ b/cms/static/js/models/xblock_outline_info.js @@ -1,6 +1,6 @@ -define(["js/models/xblock_info"], - function(XBlockInfo) { - var XBlockOutlineInfo = XBlockInfo.extend({ +define(["js/models/custom_sync_xblock_info"], + function(CustomSyncXBlockInfo) { + var XBlockOutlineInfo = CustomSyncXBlockInfo.extend({ urlRoots: { 'read': '/xblock/outline' @@ -8,15 +8,6 @@ define(["js/models/xblock_info"], createChild: function(response) { return new XBlockOutlineInfo(response, { parse: true }); - }, - - sync: function(method, model, options) { - var urlRoot = this.urlRoots[method]; - if (!urlRoot) { - urlRoot = this.urlRoot; - } - options.url = urlRoot + '/' + this.get('id'); - return XBlockInfo.prototype.sync.call(this, method, model, options); } }); return XBlockOutlineInfo; diff --git a/cms/static/js/spec/views/pages/container_spec.js b/cms/static/js/spec/views/pages/container_spec.js index 8077c0ed7afa..78a1901dc3ff 100644 --- a/cms/static/js/spec/views/pages/container_spec.js +++ b/cms/static/js/spec/views/pages/container_spec.js @@ -11,7 +11,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'), + mockXBlockVisibilityEditorHtml = readFixtures('mock/mock-xblock-visibility-editor.underscore'); beforeEach(function () { var newDisplayName = 'New Display Name'; @@ -208,6 +209,21 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel }); expect(EditHelpers.isShowingModal()).toBeTruthy(); }); + + it('can show a visibility modal for a child xblock', function() { + var visibilityButtons; + renderContainerPage(this, mockContainerXBlockHtml); + visibilityButtons = containerPage.$('.wrapper-xblock .visibility-button'); + expect(visibilityButtons.length).toBe(6); + visibilityButtons[0].click(); + expect(str.startsWith(lastRequest().url, '/xblock/locator-component-A1/visibility_view')) + .toBeTruthy(); + AjaxHelpers.respondWithJson(requests, { + html: mockXBlockVisibilityEditorHtml, + resources: [] + }); + expect(EditHelpers.isShowingModal()).toBeTruthy(); + }); }); describe("Editing an xmodule", function() { diff --git a/cms/static/js/spec/views/pages/container_subviews_spec.js b/cms/static/js/spec/views/pages/container_subviews_spec.js index 50653efcc726..0f9b681be5ef 100644 --- a/cms/static/js/spec/views/pages/container_subviews_spec.js +++ b/cms/static/js/spec/views/pages/container_subviews_spec.js @@ -80,7 +80,8 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel describe("PreviewActionController", function () { var viewPublishedCss = '.button-view', - previewCss = '.button-preview'; + previewCss = '.button-preview', + visibilityNoteCss = '.note-visibility'; it('renders correctly for unscheduled unit', function () { renderContainerPage(this, mockContainerXBlockHtml); @@ -109,6 +110,18 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel fetch({published: false, has_changes: false}); expect(containerPage.$(previewCss)).not.toHaveClass(disabledCss); }); + + it('updates when has_content_group_components attribute changes', function () { + renderContainerPage(this, mockContainerXBlockHtml); + fetch({has_content_group_components: false}); + expect(containerPage.$(visibilityNoteCss).length).toBe(0); + + fetch({has_content_group_components: true}); + expect(containerPage.$(visibilityNoteCss).length).toBe(1); + + fetch({has_content_group_components: false}); + expect(containerPage.$(visibilityNoteCss).length).toBe(0); + }); }); describe("Publisher", function () { diff --git a/cms/static/js/spec/views/xblock_editor_spec.js b/cms/static/js/spec/views/xblock_editor_spec.js index 59116c603bbe..d681db4b5e82 100644 --- a/cms/static/js/spec/views/xblock_editor_spec.js +++ b/cms/static/js/spec/views/xblock_editor_spec.js @@ -85,7 +85,7 @@ define([ "jquery", "underscore", "js/common_helpers/ajax_helpers", "js/spec_help }); // Give the mock xblock a save method... editor.xblock.save = window.MockDescriptor.save; - editor.model.save(editor.getXModuleData()); + editor.model.save(editor.getXBlockFieldData()); request = requests[requests.length - 1]; response = JSON.parse(request.requestBody); expect(response.metadata.display_name).toBe(testDisplayName); diff --git a/cms/static/js/views/metadata.js b/cms/static/js/views/metadata.js index 1bc80fe91b29..938240287d59 100644 --- a/cms/static/js/views/metadata.js +++ b/cms/static/js/views/metadata.js @@ -49,7 +49,7 @@ function(BaseView, _, MetadataModel, AbstractEditor, FileUpload, UploadDialog, V }, /** - * Returns the just the modified metadata values, in the format used to persist to the server. + * Returns just the modified metadata values, in the format used to persist to the server. */ getModifiedMetadataValues: function () { var modified_values = {}; diff --git a/cms/static/js/views/modals/base_modal.js b/cms/static/js/views/modals/base_modal.js index eb543295ed6c..fb02299484de 100644 --- a/cms/static/js/views/modals/base_modal.js +++ b/cms/static/js/views/modals/base_modal.js @@ -1,5 +1,23 @@ /** * This is a base modal implementation that provides common utilities. + * + * A modal implementation should override the following methods: + * + * getTitle(): + * returns the title for the modal. + * getHTMLContent(): + * returns the HTML content to be shown inside the modal. + * + * A modal implementation should also provide the following options: + * + * modalName: A string identifying the modal. + * modalType: A string identifying the type of the modal. + * modalSize: A string, either 'sm', 'med', or 'lg' indicating the + * size of the modal. + * viewSpecificClasses: A string of CSS classes to be attached to + * the modal window. + * addSaveButton: A boolean indicating whether to include a save + * button on the modal. */ define(["jquery", "underscore", "gettext", "js/views/baseview"], function($, _, gettext, BaseView) { @@ -41,7 +59,7 @@ define(["jquery", "underscore", "gettext", "js/views/baseview"], name: this.options.modalName, type: this.options.modalType, size: this.options.modalSize, - title: this.options.title, + title: this.getTitle(), viewSpecificClasses: this.options.viewSpecificClasses })); this.addActionButtons(); @@ -49,6 +67,10 @@ define(["jquery", "underscore", "gettext", "js/views/baseview"], this.parentElement.append(this.$el); }, + getTitle: function() { + return this.options.title; + }, + renderContents: function() { var contentHtml = this.getContentHtml(); this.$('.modal-content').html(contentHtml); diff --git a/cms/static/js/views/modals/edit_xblock.js b/cms/static/js/views/modals/edit_xblock.js index 67e9de6f88e1..76ad99dd1613 100644 --- a/cms/static/js/views/modals/edit_xblock.js +++ b/cms/static/js/views/modals/edit_xblock.js @@ -6,6 +6,8 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", "js/views/utils/view_utils", "js/models/xblock_info", "js/views/xblock_editor"], function($, _, gettext, BaseModal, ViewUtils, XBlockInfo, XBlockEditorView) { + "strict mode"; + var EditXBlockModal = BaseModal.extend({ events : { "click .action-save": "save", @@ -15,7 +17,10 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", "js/vie options: $.extend({}, BaseModal.prototype.options, { modalName: 'edit-xblock', addSaveButton: true, - viewSpecificClasses: 'modal-editor confirm' + view: 'studio_view', + viewSpecificClasses: 'modal-editor confirm', + // Translators: "title" is the name of the current component being edited. + titleFormat: gettext("Editing: %(title)s") }), initialize: function() { @@ -56,7 +61,8 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", "js/vie displayXBlock: function() { this.editorView = new XBlockEditorView({ el: this.$('.xblock-editor'), - model: this.xblockInfo + model: this.xblockInfo, + view: this.options.view }); this.editorView.render({ success: _.bind(this.onDisplayXBlock, this) @@ -88,7 +94,7 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", "js/vie // If the xblock is not using custom buttons then choose which buttons to show if (!editorView.hasCustomButtons()) { // If the xblock does not support save then disable the save button - if (!editorView.xblock.save) { + if (!this.canSave()) { this.disableSave(); } this.getActionBar().show(); @@ -98,6 +104,10 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", "js/vie this.resize(); }, + canSave: function() { + return this.editorView.xblock.save || this.editorView.xblock.collectFieldData; + }, + disableSave: function() { var saveButton = this.getActionButton('save'), cancelButton = this.getActionButton('cancel'); @@ -111,7 +121,7 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", "js/vie if (!displayName) { displayName = gettext('Component'); } - return interpolate(gettext("Editing: %(title)s"), { title: displayName }, true); + return interpolate(this.options.titleFormat, { title: displayName }, true); }, addDefaultModes: function() { @@ -146,10 +156,10 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", "js/vie var self = this, editorView = this.editorView, xblockInfo = this.xblockInfo, - data = editorView.getXModuleData(); + data = editorView.getXBlockFieldData(); event.preventDefault(); if (data) { - ViewUtils.runOperationShowingMessage(gettext('Saving…'), + ViewUtils.runOperationShowingMessage(gettext('Saving'), function() { return xblockInfo.save(data); }).done(function() { diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 7a62e535919d..4d9c834d9e96 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -15,6 +15,7 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views events: { "click .edit-button": "editXBlock", + "click .visibility-button": "editVisibilitySettings", "click .duplicate-button": "duplicateXBlock", "click .delete-button": "deleteXBlock" }, @@ -136,10 +137,10 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views }); }, - editXBlock: function(event) { + editXBlock: function(event, options) { var xblockElement = this.findXBlockElement(event.target), self = this, - modal = new EditXBlockModal({ }); + modal = new EditXBlockModal(options); event.preventDefault(); modal.edit(xblockElement, this.model, { @@ -149,6 +150,16 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views }); }, + editVisibilitySettings: function(event) { + this.editXBlock(event, { + view: 'visibility_view', + // Translators: "title" is the name of the current component being edited. + titleFormat: gettext("Editing visibility for: %(title)s"), + viewSpecificClasses: '', + modalSize: 'med' + }); + }, + duplicateXBlock: function(event) { event.preventDefault(); this.duplicateComponent(this.findXBlockElement(event.target)); diff --git a/cms/static/js/views/pages/container_subviews.js b/cms/static/js/views/pages/container_subviews.js index 1957633f3bff..552deb9eee4b 100644 --- a/cms/static/js/views/pages/container_subviews.js +++ b/cms/static/js/views/pages/container_subviews.js @@ -100,7 +100,8 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/utils/ onSync: function(model) { if (ViewUtils.hasChangedAttributes(model, [ - 'has_changes', 'published', 'edited_on', 'edited_by', 'visibility_state', 'has_explicit_staff_lock' + 'has_changes', 'published', 'edited_on', 'edited_by', 'visibility_state', + 'has_explicit_staff_lock', 'has_content_group_components' ])) { this.render(); } @@ -120,7 +121,8 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/utils/ releaseDate: this.model.get('release_date'), releaseDateFrom: this.model.get('release_date_from'), hasExplicitStaffLock: this.model.get('has_explicit_staff_lock'), - staffLockFrom: this.model.get('staff_lock_from') + staffLockFrom: this.model.get('staff_lock_from'), + hasContentGroupComponents: this.model.get('has_content_group_components') })); return this; diff --git a/cms/static/js/views/pages/course_outline.js b/cms/static/js/views/pages/course_outline.js index a812eb7b02fa..4e815fd08bd3 100644 --- a/cms/static/js/views/pages/course_outline.js +++ b/cms/static/js/views/pages/course_outline.js @@ -26,7 +26,7 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views }); this.model.on('change', this.setCollapseExpandVisibility, this); $('.dismiss-button').bind('click', ViewUtils.deleteNotificationHandler(function () { - $('.wrapper-alert-announcement').removeClass('is-shown').addClass('is-hidden') + $('.wrapper-alert-announcement').removeClass('is-shown').addClass('is-hidden'); })); }, diff --git a/cms/static/js/views/xblock_editor.js b/cms/static/js/views/xblock_editor.js index 7cdf2de34a75..e549fa6b54ae 100644 --- a/cms/static/js/views/xblock_editor.js +++ b/cms/static/js/views/xblock_editor.js @@ -89,17 +89,23 @@ define(["jquery", "underscore", "gettext", "js/views/xblock", "js/views/metadata }, /** - * Returns the data saved for the xmodule. Note that this *does not* work for XBlocks. + * Returns the updated field data for the xblock. Note that this works for all + * XModules as well as for XBlocks that provide a 'collectFieldData' API. */ - getXModuleData: function() { + getXBlockFieldData: function() { var xblock = this.xblock, metadataEditor = this.getMetadataEditor(), data = null; - if (xblock.save) { + // If the xblock supports returning its field data then collect it + if (xblock.collectFieldData) { + data = xblock.collectFieldData(); + // ... else if this is an XModule then call its save method + } else if (xblock.save) { data = xblock.save(); if (metadataEditor) { data.metadata = _.extend(data.metadata || {}, this.getChangedMetadata()); } + // ... else log an error } else { console.error('Cannot save xblock as it has no save method'); } diff --git a/cms/static/js/xblock/authoring.js b/cms/static/js/xblock/authoring.js new file mode 100644 index 000000000000..dfc8e7cc0dee --- /dev/null +++ b/cms/static/js/xblock/authoring.js @@ -0,0 +1,48 @@ +/** + * Client-side logic to support XBlock authoring. + */ +(function($) { + 'use strict'; + + function VisibilityEditorView(runtime, element) { + this.getGroupAccess = function() { + var groupAccess, userPartitionId, selectedGroupIds; + if (element.find('.visibility-level-all').prop('checked')) { + return {}; + } + userPartitionId = element.find('.wrapper-visibility-specific').data('user-partition-id').toString(); + selectedGroupIds = []; + element.find('.field-visibility-content-group input:checked').each(function(index, input) { + selectedGroupIds.push(parseInt($(input).val())); + }); + groupAccess = {}; + groupAccess[userPartitionId] = selectedGroupIds; + return groupAccess; + }; + + element.find('.field-visibility-level input').change(function(event) { + if ($(event.target).hasClass('visibility-level-all')) { + element.find('.field-visibility-content-group input').prop('checked', false); + } + }); + element.find('.field-visibility-content-group input').change(function(event) { + element.find('.visibility-level-all').prop('checked', false); + element.find('.visibility-level-specific').prop('checked', true); + }); + } + + VisibilityEditorView.prototype.collectFieldData = function collectFieldData() { + return { + metadata: { + "group_access": this.getGroupAccess() + } + }; + }; + + function initializeVisibilityEditor(runtime, element) { + return new VisibilityEditorView(runtime, element); + } + + // XBlock initialization functions must be global + window.VisibilityEditorInit = initializeVisibilityEditor; +})($); diff --git a/cms/static/sass/_variables.scss b/cms/static/sass/_variables.scss index c310f4296f1c..9542cb8cbf43 100644 --- a/cms/static/sass/_variables.scss +++ b/cms/static/sass/_variables.scss @@ -165,6 +165,7 @@ $color-ready: $green; $color-warning: $orange-l2; $color-error: $red-l2; $color-staff-only: $black; +$color-visibility-set: $black; $color-heading-base: $gray-d2; $color-copy-base: $gray-l1; diff --git a/cms/static/sass/elements/_forms.scss b/cms/static/sass/elements/_forms.scss index 5979c92ff740..e80944c1f66e 100644 --- a/cms/static/sass/elements/_forms.scss +++ b/cms/static/sass/elements/_forms.scss @@ -1,7 +1,24 @@ // studio - elements - forms // ==================== +// element-specific utilities +// -------------------- +// UI: checkbox/radio inputs +%input-tickable { + + ~ label { + color: $color-copy-base; + } + + // STATE: checked/selected + &:checked ~ label { + @extend %t-strong; + color: $ui-action-primary-color-focus; + } +} + // forms - general +// -------------------- input[type="text"], input[type="email"], input[type="password"], @@ -99,8 +116,18 @@ form { } } + // CASE: checkbox input + .field-checkbox .input-checkbox { + @extend %input-tickable; + } + + // CASE: radio input + .field-radio .input-radio { + @extend %input-tickable; + } + // CASE: file input - input[type=file] { + input[type="file"] { @extend %t-copy-sub1; } diff --git a/cms/static/sass/elements/_modal-window.scss b/cms/static/sass/elements/_modal-window.scss index c80ff868625e..1ade1439069f 100644 --- a/cms/static/sass/elements/_modal-window.scss +++ b/cms/static/sass/elements/_modal-window.scss @@ -52,6 +52,45 @@ } } + // UI: summary messages + .summary-message { + margin-bottom: $baseline; + padding: ($baseline*0.75); + background: $gray-d3; + + .icon, .copy { + display: inline-block; + vertical-align: top; + } + + .icon { + @extend %t-icon4; + @include margin-right($baseline/2); + color: $white; + } + + .copy { + @extend %t-copy-sub1; + max-width: 85%; + color: $white; + } + } + + // CASE: Warning summary message + .summary-message-warning { + border-top: ($baseline/5) solid $color-warning; + + .icon { + color: $color-warning; + } + } + + // visual dividers + .divider-visual { + margin: ($baseline*0.75) 0; + border: ($baseline/20) solid $gray-l4; + } + // sections within a modal .modal-section { margin-bottom: ($baseline*0.75); @@ -64,11 +103,20 @@ .modal-section-title { @extend %t-title6; margin: 0 0 ($baseline/2) 0; - border-bottom: 1px solid $gray-l4; + border-bottom: ($baseline/10) solid $gray-l4; padding-bottom: ($baseline/4); color: $gray-d2; } + .modal-subsection-title { + @extend %t-title8; + @extend %t-strong; + margin-bottom: ($baseline/4); + text-transform: uppercase; + letter-spacing: 0.1; + color: $gray-l2; + } + .modal-section-content { .list-fields, .list-actions { @@ -234,143 +282,6 @@ } } - // outline: edit item settings - .wrapper-modal-window-bulkpublish-section, - .wrapper-modal-window-bulkpublish-subsection, - .wrapper-modal-window-bulkpublish-unit, - .course-outline-modal { - - .list-fields { - - .field { - display: inline-block; - vertical-align: top; - margin-right: ($baseline/2); - margin-bottom: ($baseline/4); - - label { - @extend %t-copy-sub1; - @extend %t-strong; - @include transition(color $tmg-f3 ease-in-out 0s); - margin: 0 0 ($baseline/4) 0; - - &.is-focused { - color: $blue; - } - } - - - input, textarea { - @extend %t-copy-base; - @include transition(all $tmg-f2 ease-in-out 0s); - height: 100%; - width: 100%; - padding: ($baseline/2); - - // CASE: long length - &.long { - width: 100%; - } - - // CASE: short length - &.short { - width: 25%; - } - } - - // CASE: specific release + due times/dates - .start-date, - .start-time, - .due-date, - .due-time { - width: ($baseline*7); - } - - .tip { - @extend %t-copy-sub1; - @include transition(color, 0.15s, ease-in-out); - display: block; - margin-top: ($baseline/4); - color: $gray-l2; - } - - .tip-warning { - color: $gray-d2; - } - } - - // CASE: type-based input - .field-text { - - // TODO: refactor the _forms.scss partial to allow for this area to inherit from it - label, input, textarea { - display: block; - } - } - - // CASE: select input - .field-select { - - .label, .input { - display: inline-block; - vertical-align: middle; - } - - .label { - margin-right: ($baseline/2); - } - - .input { - width: 100%; - } - - // CASE: checkbox input - .field-checkbox { - - .label, label { - margin-bottom: 0; - } - } - } - } - - - - // UI: grading section - .edit-settings-grading { - - .grading-type { - margin-bottom: $baseline; - } - } - - // UI: staff lock section - .edit-staff-lock { - - .checkbox-cosmetic .input-checkbox { - @extend %cont-text-sr; - - // CASE: unchecked - ~ .tip-warning { - display: block; - } - - // CASE: checked - &:checked { - - ~ .tip-warning { - display: none; - } - } - } - - // needed to override poorly scoped margin-bottom on any label element in a view (from _forms.scss) - .checkbox-cosmetic .label { - margin-bottom: 0; - } - } - } - // xblock custom actions .modal-window .editor-with-buttons { margin-bottom: ($baseline*3); @@ -390,7 +301,7 @@ } - // special overrides for video module editor/hidden tab editors + // MODAL TYPE: component - video modal (includes special overrides for xblock-related editing view) .modal-lg.modal-type-video { .modal-content { @@ -513,4 +424,225 @@ opacity: 0.5; filter: alpha(opacity=50); } + + // MODAL TYPE: component - visibility modal + .xblock-visibility_view { + + .visibility-controls-secondary { + max-height: 100%; + overflow-y: scroll; + @include margin(($baseline*0.75), 0, 0, $baseline); + } + + .visibility-controls-group { + @extend %wipe-last-child; + margin-bottom: $baseline; + } + + // UI: form fields + .list-fields { + + .field { + @extend %wipe-last-child; + margin-bottom: ($baseline/4); + + label { + @extend %t-copy-sub1; + } + } + + // UI: radio and checkbox inputs + .field-radio, .field-checkbox { + + label { + @include margin-left($baseline/4); + } + } + } + + // CASE: content group has been removed + .field-visibility-content-group.was-removed { + + .input-checkbox:checked ~ label { + color: $color-error; + } + + .note { + @extend %t-copy-sub2; + @extend %t-regular; + display: block; + color: $color-error; + } + } + + // CASE: no groups configured for visibility + .is-not-configured { + @extend %no-content; + padding: ($baseline); + @include text-align(left); // reset for %no-content's default styling + + .title { + @extend %t-title6; + font-weight: 600; // needed for poorly scoped .title rule in modals + margin: 0 0 ($baseline/2) 0; // needed for poorly scoped .title rule in modals + } + + .copy { + @extend %t-copy-sub1; + + p { + @extend %wipe-last-child; + margin-bottom: $baseline; + } + } + + &.has-actions { + + .actions { + margin-top: $baseline; + } + + .action { + @include margin-left(0); // reset for %no-content's default styling + } + } + } + } + + // MODAL TYPE: outline - edit item settings + .wrapper-modal-window-bulkpublish-section, + .wrapper-modal-window-bulkpublish-subsection, + .wrapper-modal-window-bulkpublish-unit, + .course-outline-modal { + + .list-fields { + + .field { + display: inline-block; + vertical-align: top; + @include margin-right($baseline/2); + margin-bottom: ($baseline/4); + + label { + @extend %t-copy-sub1; + @extend %t-strong; + @include transition(color $tmg-f3 ease-in-out 0s); + margin: 0 0 ($baseline/4) 0; + + &.is-focused { + color: $blue; + } + } + + + input, textarea { + @extend %t-copy-base; + @include transition(all $tmg-f2 ease-in-out 0s); + height: 100%; + width: 100%; + padding: ($baseline/2); + + // CASE: long length + &.long { + width: 100%; + } + + // CASE: short length + &.short { + width: 25%; + } + } + + // CASE: specific release + due times/dates + .start-date, + .start-time, + .due-date, + .due-time { + width: ($baseline*7); + } + + .tip { + @extend %t-copy-sub1; + @include transition(color, 0.15s, ease-in-out); + display: block; + margin-top: ($baseline/4); + color: $gray-l2; + } + + .tip-warning { + color: $gray-d2; + } + } + + // CASE: type-based input + .field-text { + + // TODO: refactor the _forms.scss partial to allow for this area to inherit from it + label, input, textarea { + display: block; + } + } + + // CASE: select input + .field-select { + + .label, .input { + display: inline-block; + vertical-align: middle; + } + + .label { + @include margin-right($baseline/2); + } + + .input { + width: 100%; + } + + // CASE: checkbox input + .field-checkbox { + + .label, label { + margin-bottom: 0; + } + } + } + } + + + + // UI: grading section + .edit-settings-grading { + + .grading-type { + margin-bottom: $baseline; + } + } + + // UI: staff lock section + .edit-staff-lock { + + .checkbox-cosmetic .input-checkbox { + @extend %cont-text-sr; + + // CASE: unchecked + ~ .tip-warning { + display: block; + } + + // CASE: checked + &:checked { + + ~ .tip-warning { + display: none; + } + } + } + + // needed to override poorly scoped margin-bottom on any label element in a view (from _forms.scss) + .checkbox-cosmetic .label { + margin-bottom: 0; + } + } + } } diff --git a/cms/static/sass/elements/_xblocks.scss b/cms/static/sass/elements/_xblocks.scss index d98506f6dfb2..16f9d16bb66e 100644 --- a/cms/static/sass/elements/_xblocks.scss +++ b/cms/static/sass/elements/_xblocks.scss @@ -119,42 +119,50 @@ // ==================== - // UI: xblocks - calls-to-action - .wrapper-xblock .header-actions { + .wrapper-xblock { - .actions-list { + // UI: xblocks - calls-to-action + .header-actions .actions-list { @extend %actions-list; } - } - // UI: xblock is collapsible - .wrapper-xblock.is-collapsible, - .wrapper-xblock.xblock-type-container { + // CASE: xblock is collapsible + &.is-collapsible, + &.xblock-type-container { - [class^="icon-"] { - font-style: normal; - } + [class^="icon-"] { + font-style: normal; + } - .expand-collapse { - @extend %expand-collapse; - margin: 0 ($baseline/4); - height: ($baseline*1.25); - width: $baseline; + .expand-collapse { + @extend %expand-collapse; + margin: 0 ($baseline/4); + height: ($baseline*1.25); + width: $baseline; - &:focus { - outline: 0; + &:focus { + outline: 0; + } } - } - .action-view { + .action-view { + + .action-button { + transition: none; + } - .action-button { - transition: none; + .action-button-text { + padding-right: ($baseline/5); + padding-left: 0; + } } + } + + // CASE: xblock has specific visibility based on content groups set + &.has-group-visibility-set { - .action-button-text { - padding-right: ($baseline/5); - padding-left: 0; + .action-visibility .visibility-button.visibility-button { // needed to cascade in front of overscoped header-actions CSS rule + color: $color-visibility-set; } } } diff --git a/cms/static/sass/views/_container.scss b/cms/static/sass/views/_container.scss index 7780d0485078..f240b9c2e0fb 100644 --- a/cms/static/sass/views/_container.scss +++ b/cms/static/sass/views/_container.scss @@ -6,7 +6,20 @@ // ==================== +// view-specific utilities +// -------------------- +%status-value-base { + @extend %t-title7; + @extend %t-strong; +} + +%status-value-sub1 { + @extend %t-title8; + display: block; +} + // UI: container page view +// -------------------- .view-container { @extend %two-col-1; @@ -102,6 +115,7 @@ @extend %t-title8; } + // UI: publishing details/summary .bit-publishing { @extend %bar-module; @@ -159,19 +173,18 @@ .wrapper-release { .release-date { - @extend %t-strong; + @extend %status-value-base; } .release-with { - @extend %t-title8; - display: block; + @extend %status-value-sub1; } } .wrapper-visibility { .copy { - @extend %t-strong; + @extend %status-value-base; margin-bottom: ($baseline/10); } @@ -181,15 +194,23 @@ } .inherited-from { - @extend %t-title8; - display: block; + @extend %status-value-sub1; } + // UI: note about specific access + .note-visibility { + @extend %status-value-sub1; + .icon { + @include margin-right($baseline/4); + } + } } .wrapper-pub-actions { - padding: ($baseline*.75); + border-top: 1px solid $gray-l4; + margin-top: ($baseline/2); + padding: $baseline ($baseline*0.75) ($baseline*0.75) ($baseline*0.75); .action-publish { @extend %btn-primary-blue; @@ -209,7 +230,6 @@ } } } - } // versioning widget @@ -244,8 +264,7 @@ .wrapper-unit-id { .unit-id-value { - @extend %cont-text-wrap; - @extend %t-copy-sub1; + @extend %status-value-base; display: inline-block; width: 100%; } diff --git a/cms/templates/base.html b/cms/templates/base.html index d02d85fb0e68..ad4919ddd349 100644 --- a/cms/templates/base.html +++ b/cms/templates/base.html @@ -80,6 +80,9 @@
+ + <%block name="modal_placeholder"> + <%block name="jsextra">