Merge pull request #3064 from edx/nimisha/studio-reorder-and-hide-tabs

Nimisha/studio reorder and hide tabs

CHANGELOG.rst

@@ -5,6 +5,8 @@ These are notable changes in edx-platform.  This is a rolling list of changes,
 in roughly chronological order, most recent first.  Add your entries at or near
 the top.  Include a label indicating the component affected.
 
+Studio: Add ability to reorder Pages and hide the Wiki page. STUD-1375
+
 Blades: Added template for iFrames. BLD-611.
 
 Studio: Support for viewing built-in tabs on the Pages page. STUD-1193

cms/djangoapps/contentstore/features/pages.feature

@@ -15,10 +15,6 @@ Feature: CMS.Pages
         When I confirm the prompt
         Then I should not see any static pages
 
-    Scenario: Users can see built-in pages
-        Given I have opened the pages page in a new course
-        Then I should see the default built-in pages
-
     # Safari won't update the name properly
     @skip_safari
     Scenario: Users can edit static pages
@@ -31,7 +27,36 @@ Feature: CMS.Pages
     @skip_safari
     Scenario: Users can reorder static pages
         Given I have created two different static pages
-        When I reorder the static tabs
-        Then the static tabs are in the reverse order
+        When I drag the first static page to the last
+        Then the static pages are switched
         And I reload the page
-        Then the static tabs are in the reverse order
+        Then the static pages are switched
+
+    Scenario: Users can reorder built-in pages
+        Given I have opened the pages page in a new course
+        Then the built-in pages are in the default order
+        When I drag the first page to the last
+        Then the built-in pages are switched
+        And I reload the page
+        Then the built-in pages are switched
+
+    Scenario: Users can reorder built-in pages amongst static pages
+        Given I have created two different static pages
+        Then the pages are in the default order
+        When I drag the first page to the last
+        Then the pages are switched
+        And I reload the page
+        Then the pages are switched
+
+    Scenario: Users can toggle visibility on hideable pages
+        Given I have opened the pages page in a new course
+        Then I should see the "wiki" page as "visible"
+        When I toggle the visibility of the "wiki" page
+        Then I should see the "wiki" page as "hidden"
+        And I reload the page
+        Then I should see the "wiki" page as "hidden"
+        When I toggle the visibility of the "wiki" page
+        Then I should see the "wiki" page as "visible"
+        And I reload the page
+        Then I should see the "wiki" page as "visible"
+

cms/djangoapps/contentstore/features/pages.py

@@ -3,7 +3,10 @@
 # pylint: disable=W0613
 
 from lettuce import world, step
-from nose.tools import assert_equal  # pylint: disable=E0611
+from nose.tools import assert_equal, assert_in  # pylint: disable=E0611
+
+
+CSS_FOR_TAB_ELEMENT = "li[data-tab-id='{0}'] input.toggle-checkbox"
 
 
 @step(u'I go to the pages page$')
@@ -33,15 +36,6 @@ def not_see_any_static_pages(step):
     assert (world.is_css_not_present(pages_css, wait_time=30))
 
 
-@step(u'I should see the default built-in pages')
-def see_default_built_in_pages(step):
-    expected_pages = ['Courseware', 'Course Info', 'Discussion', 'Wiki', 'Progress']
-    pages = world.css_find("div.course-nav-tab-header h3.title")
-    assert_equal(len(expected_pages), len(pages))
-    for i, page_name in enumerate(expected_pages):
-        assert_equal(pages[i].text, page_name)
-
-
 @step(u'I "(edit|delete)" the static page$')
 def click_edit_or_delete(step, edit_or_delete):
     button_css = 'ul.component-actions a.%s-button' % edit_or_delete
@@ -60,50 +54,101 @@ def change_name(step, new_name):
     world.css_click(save_button)
 
 
-@step(u'I reorder the static tabs')
-def reorder_tabs(_step):
-    # For some reason, the drag_and_drop method did not work in this case.
-    draggables = world.css_find('.component .drag-handle')
-    source = draggables.first
-    target = draggables.last
-    source.action_chains.click_and_hold(source._element).perform()  # pylint: disable=protected-access
-    source.action_chains.move_to_element_with_offset(target._element, 0, 50).perform()  # pylint: disable=protected-access
-    source.action_chains.release().perform()
+@step(u'I drag the first static page to the last$')
+def drag_first_static_page_to_last(step):
+    drag_first_to_last_with_css('.component')
 
 
-@step(u'I have created a static page')
+@step(u'I have created a static page$')
 def create_static_page(step):
     step.given('I have opened the pages page in a new course')
     step.given('I add a new static page')
 
 
-@step(u'I have opened the pages page in a new course')
+@step(u'I have opened the pages page in a new course$')
 def open_pages_page_in_new_course(step):
     step.given('I have opened a new course in Studio')
     step.given('I go to the pages page')
 
 
-@step(u'I have created two different static pages')
+@step(u'I have created two different static pages$')
 def create_two_pages(step):
     step.given('I have created a static page')
     step.given('I "edit" the static page')
     step.given('I change the name to "First"')
     step.given('I add a new static page')
-    # Verify order of tabs
-    _verify_tab_names('First', 'Empty')
+    # Verify order of pages
+    _verify_page_names('First', 'Empty')
 
 
-@step(u'the static tabs are in the reverse order')
-def tabs_in_reverse_order(step):
-    _verify_tab_names('Empty', 'First')
+@step(u'the static pages are switched$')
+def static_pages_are_switched(step):
+    _verify_page_names('Empty', 'First')
 
 
-def _verify_tab_names(first, second):
+def _verify_page_names(first, second):
     world.wait_for(
         func=lambda _: len(world.css_find('.xmodule_StaticTabModule')) == 2,
         timeout=200,
-        timeout_msg="Timed out waiting for two tabs to be present"
+        timeout_msg="Timed out waiting for two pages to be present"
     )
-    tabs = world.css_find('.xmodule_StaticTabModule')
-    assert tabs[0].text == first
-    assert tabs[1].text == second
+    pages = world.css_find('.xmodule_StaticTabModule')
+    assert_equal(pages[0].text, first)
+    assert_equal(pages[1].text, second)
+
+
+@step(u'the built-in pages are in the default order$')
+def built_in_pages_in_default_order(step):
+    expected_pages = ['Courseware', 'Course Info', 'Discussion', 'Wiki', 'Progress']
+    see_pages_in_expected_order(expected_pages)
+
+
+@step(u'the built-in pages are switched$')
+def built_in_pages_switched(step):
+    expected_pages = ['Courseware', 'Course Info', 'Wiki', 'Progress', 'Discussion']
+    see_pages_in_expected_order(expected_pages)
+
+
+@step(u'the pages are in the default order$')
+def pages_in_default_order(step):
+    expected_pages = ['Courseware', 'Course Info', 'Discussion', 'Wiki', 'Progress', 'First', 'Empty']
+    see_pages_in_expected_order(expected_pages)
+
+
+@step(u'the pages are switched$$')
+def pages_are_switched(step):
+    expected_pages = ['Courseware', 'Course Info', 'Wiki', 'Progress', 'First', 'Empty', 'Discussion']
+    see_pages_in_expected_order(expected_pages)
+
+
+@step(u'I drag the first page to the last$')
+def drag_first_page_to_last(step):
+    drag_first_to_last_with_css('.is-movable')
+
+
+@step(u'I should see the "([^"]*)" page as "(visible|hidden)"$')
+def page_is_visible_or_hidden(step, page_id, visible_or_hidden):
+    hidden = visible_or_hidden == "hidden"
+    assert_equal(world.css_find(CSS_FOR_TAB_ELEMENT.format(page_id)).checked, hidden)
+
+
+@step(u'I toggle the visibility of the "([^"]*)" page$')
+def page_toggle_visibility(step, page_id):
+    world.css_find(CSS_FOR_TAB_ELEMENT.format(page_id))[0].click()
+
+
+def drag_first_to_last_with_css(css_class):
+    # For some reason, the drag_and_drop method did not work in this case.
+    draggables = world.css_find(css_class + ' .drag-handle')
+    source = draggables.first
+    target = draggables.last
+    source.action_chains.click_and_hold(source._element).perform()  # pylint: disable=protected-access
+    source.action_chains.move_to_element_with_offset(target._element, 0, 50).perform()  # pylint: disable=protected-access
+    source.action_chains.release().perform()
+
+
+def see_pages_in_expected_order(page_names_in_expected_order):
+    pages = world.css_find("li.course-tab")
+    assert_equal(len(page_names_in_expected_order), len(pages))
+    for i, page_name in enumerate(page_names_in_expected_order):
+        assert_in(page_name, pages[i].text)

cms/djangoapps/contentstore/management/commands/tests/test_git_export.py

@@ -150,8 +150,8 @@ class TestGitExport(CourseTestCase):
                                            '--format=%an|%ae'], cwd=cwd)
         self.assertEqual(expect_string, git_log)
 
-        # Make changes to course so there is something commit
-        self.populateCourse()
+        # Make changes to course so there is something to commit
+        self.populate_course()
         git_export_utils.export_to_git(
             self.course.id,
             'file://{0}'.format(self.bare_repo_dir),

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -400,23 +400,34 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
         course = module_store.get_item(course_location)
 
-        # reverse the ordering
-        reverse_tabs = []
+        # reverse the ordering of the static tabs
+        reverse_static_tabs = []
+        built_in_tabs = []
         for tab in course.tabs:
             if tab['type'] == 'static_tab':
-                reverse_tabs.insert(0, unicode(self._get_tab_locator(course, tab)))
-
-        self.client.ajax_post(new_location.url_reverse('tabs'), {'tabs': reverse_tabs})
+                reverse_static_tabs.insert(0, tab)
+            else:
+                built_in_tabs.append(tab)
+
+        # create the requested tab_id_locators list
+        tab_id_locators = [
+            {
+                'tab_id': tab.tab_id
+            } for tab in built_in_tabs
+        ]
+        tab_id_locators.extend([
+            {
+                'tab_locator': unicode(self._get_tab_locator(course, tab))
+            } for tab in reverse_static_tabs
+        ])
+
+        self.client.ajax_post(new_location.url_reverse('tabs'), {'tabs': tab_id_locators})
 
         course = module_store.get_item(course_location)
 
         # compare to make sure that the tabs information is in the expected order after the server call
-        course_tabs = []
-        for tab in course.tabs:
-            if tab['type'] == 'static_tab':
-                course_tabs.append(unicode(self._get_tab_locator(course, tab)))
-
-        self.assertEqual(reverse_tabs, course_tabs)
+        new_static_tabs = [tab for tab in course.tabs if (tab['type'] == 'static_tab')]
+        self.assertEqual(reverse_static_tabs, new_static_tabs)
 
     def test_static_tab_deletion(self):
         module_store, course_location, _ = self._create_static_tabs()

cms/djangoapps/contentstore/tests/test_course_settings.py

@@ -410,7 +410,7 @@ class CourseGradingTest(CourseTestCase):
         """
         Populate the course, grab a section, get the url for the assignment type access
         """
-        self.populateCourse()
+        self.populate_course()
         sections = get_modulestore(self.course_location).get_items(
             self.course_location.replace(category="sequential", name=None)
         )

cms/djangoapps/contentstore/tests/test_export_git.py

@@ -102,7 +102,7 @@ class TestExportGit(CourseTestCase):
 
         subprocess.check_output(['git', '--bare', 'init', ], cwd=bare_repo_dir)
 
-        self.populateCourse()
+        self.populate_course()
         self.course_module.giturl = 'file://{}'.format(bare_repo_dir)
         get_modulestore(self.course_module.location).update_item(self.course_module)
 

cms/djangoapps/contentstore/tests/test_orphan.py

@@ -75,7 +75,7 @@ class TestOrphan(CourseTestCase):
         """
         Test that auth restricts get and delete appropriately
         """
-        test_user_client, test_user = self.createNonStaffAuthedUserClient()
+        test_user_client, test_user = self.create_non_staff_authed_user_client()
         CourseEnrollment.enroll(test_user, self.course.location.course_id)
         locator = loc_mapper().translate_location(self.course.location.course_id, self.course.location, False, True)
         orphan_url = locator.url_reverse('orphan/', '')

cms/djangoapps/contentstore/tests/utils.py

@@ -12,6 +12,7 @@ from django.test.utils import override_settings
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 from contentstore.tests.modulestore_config import TEST_MODULESTORE
+from contentstore.utils import get_modulestore
 from xmodule.modulestore.django import loc_mapper
 
 
@@ -95,8 +96,9 @@ class CourseTestCase(ModuleStoreTestCase):
         self.course_locator = loc_mapper().translate_location(
             self.course.location.course_id, self.course.location, False, True
         )
+        self.store = get_modulestore(self.course.location)
 
-    def createNonStaffAuthedUserClient(self):
+    def create_non_staff_authed_user_client(self):
         """
         Create a non-staff user, log them in, and return the client, user to use for testing.
         """
@@ -114,7 +116,7 @@ class CourseTestCase(ModuleStoreTestCase):
         client.login(username=uname, password=password)
         return client, nonstaff
 
-    def populateCourse(self):
+    def populate_course(self):
         """
         Add 2 chapters, 4 sections, 8 verticals, 16 problems to self.course (branching 2)
         """
@@ -126,3 +128,16 @@ class CourseTestCase(ModuleStoreTestCase):
                     descend(child, stack)
 
         descend(self.course, ['chapter', 'sequential', 'vertical', 'problem'])
+
+    def reload_course(self):
+        """
+        Reloads the course object from the database
+        """
+        self.course = self.store.get_item(self.course.location)
+
+    def save_course(self):
+        """
+        Updates the course object in the database
+        """
+        self.course.save()
+        self.store.update_item(self.course, self.user.id)

cms/djangoapps/contentstore/views/course.py

@@ -918,9 +918,9 @@ def textbooks_detail_handler(request, tid, tag=None, package_id=None, branch=Non
         if not textbook:
             return JsonResponse(status=404)
         i = course.pdf_textbooks.index(textbook)
-        new_textbooks = course.pdf_textbooks[0:i]
-        new_textbooks.extend(course.pdf_textbooks[i + 1:])
-        course.pdf_textbooks = new_textbooks
+        remaining_textbooks = course.pdf_textbooks[0:i]
+        remaining_textbooks.extend(course.pdf_textbooks[i + 1:])
+        course.pdf_textbooks = remaining_textbooks
         store.update_item(course, request.user.id)
         return JsonResponse()
 

cms/djangoapps/contentstore/views/tabs.py

@@ -14,11 +14,9 @@ from edxmako.shortcuts import render_to_response
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.django import loc_mapper
 from xmodule.modulestore.locator import BlockUsageLocator
-from xmodule.tabs import CourseTabList, StaticTab, CourseTab
+from xmodule.tabs import CourseTabList, StaticTab, CourseTab, InvalidTabsException
 
-from ..utils import get_modulestore
-
-from django.utils.translation import ugettext as _
+from ..utils import get_modulestore, get_lms_link_for_item
 
 __all__ = ['tabs_handler']
 
@@ -53,83 +51,130 @@ def tabs_handler(request, tag=None, package_id=None, branch=None, version_guid=N
             raise NotImplementedError('coming soon')
         else:
             if 'tabs' in request.json:
-                def get_location_for_tab(tab):
-                    """  Returns the location (old-style) for a tab. """
-                    return loc_mapper().translate_locator_to_location(BlockUsageLocator(tab))
+                return reorder_tabs_handler(course_item, request)
+            elif 'tab_id_locator' in request.json:
+                return edit_tab_handler(course_item, request)
+            else:
+                raise NotImplementedError('Creating or changing tab content is not supported.')
 
-                tabs = request.json['tabs']
+    elif request.method == 'GET':  # assume html
+        # get all tabs from the tabs list: static tabs (a.k.a. user-created tabs) and built-in tabs
+        # present in the same order they are displayed in LMS
 
-                # get list of existing static tabs in course
-                # make sure they are the same lengths (i.e. the number of passed in tabs equals the number
-                # that we know about) otherwise we will inadvertently drop some!
-                existing_static_tabs = [t for t in course_item.tabs if t['type'] == 'static_tab']
-                if len(existing_static_tabs) != len(tabs):
-                    return JsonResponse(
-                        {"error": "number of tabs must be {}".format(len(existing_static_tabs))}, status=400
+        tabs_to_render = []
+        for tab in CourseTabList.iterate_displayable_cms(
+                course_item,
+                settings,
+        ):
+            if isinstance(tab, StaticTab):
+                # static tab needs its locator information to render itself as an xmodule
+                static_tab_loc = old_location.replace(category='static_tab', name=tab.url_slug)
+                tab.locator = loc_mapper().translate_location(
+                    course_item.location.course_id, static_tab_loc, False, True
                 )
+            tabs_to_render.append(tab)
+
+        return render_to_response('edit-tabs.html', {
+            'context_course': course_item,
+            'tabs_to_render': tabs_to_render,
+            'course_locator': locator,
+            'lms_link': get_lms_link_for_item(course_item.location),
+        })
+    else:
+        return HttpResponseNotFound()
 
-                # load all reference tabs, return BadRequest if we can't find any of them
-                tab_items = []
-                for tab in tabs:
-                    item = modulestore('direct').get_item(get_location_for_tab(tab))
-                    if item is None:
+
+def reorder_tabs_handler(course_item, request):
+    """
+    Helper function for handling reorder of tabs request
+    """
+
+    # Tabs are identified by tab_id or locators.
+    # The locators are used to identify static tabs since they are xmodules.
+    # Although all tabs have tab_ids, newly created static tabs do not know
+    # their tab_ids since the xmodule editor uses only locators to identify new objects.
+    requested_tab_id_locators = request.json['tabs']
+
+    # original tab list in original order
+    old_tab_list = course_item.tabs
+
+    # create a new list in the new order
+    new_tab_list = []
+    for tab_id_locator in requested_tab_id_locators:
+        tab = get_tab_by_tab_id_locator(old_tab_list, tab_id_locator)
+        if tab is None:
             return JsonResponse(
-                            {"error": "no tab for found location {}".format(tab)}, status=400
+                {"error": "Tab with id_locator '{0}' does not exist.".format(tab_id_locator)}, status=400
             )
+        new_tab_list.append(tab)
 
-                    tab_items.append(item)
+    # the old_tab_list may contain additional tabs that were not rendered in the UI because of
+    # global or course settings.  so add those to the end of the list.
+    non_displayed_tabs = set(old_tab_list) - set(new_tab_list)
+    new_tab_list.extend(non_displayed_tabs)
 
-                # now just go through the existing course_tabs and re-order the static tabs
-                reordered_tabs = []
-                static_tab_idx = 0
-                for tab in course_item.tabs:
-                    if isinstance(tab, StaticTab):
-                        reordered_tabs.append(
-                            StaticTab(
-                                name=tab_items[static_tab_idx].display_name,
-                                url_slug=tab_items[static_tab_idx].location.name,
-                            )
+    # validate the tabs to make sure everything is Ok (e.g., did the client try to reorder unmovable tabs?)
+    try:
+        CourseTabList.validate_tabs(new_tab_list)
+    except InvalidTabsException, exception:
+        return JsonResponse(
+            {"error": "New list of tabs is not valid: {0}.".format(str(exception))}, status=400
         )
-                        static_tab_idx += 1
-                    else:
-                        reordered_tabs.append(tab)
 
-                # OK, re-assemble the static tabs in the new order
-                course_item.tabs = reordered_tabs
+    # persist the new order of the tabs
+    course_item.tabs = new_tab_list
     modulestore('direct').update_item(course_item, request.user.id)
+
     return JsonResponse()
-            else:
-                raise NotImplementedError('Creating or changing tab content is not supported.')
-    elif request.method == 'GET':  # assume html
-        # get all tabs from the tabs list: static tabs (a.k.a. user-created tabs) and built-in tabs
-        # we do this because this is also the order in which items are displayed in the LMS
 
-        static_tabs = []
-        built_in_tabs = []
-        for tab in CourseTabList.iterate_displayable(course_item, settings, include_instructor_tab=False):
-            if isinstance(tab, StaticTab):
-                static_tab_loc = old_location.replace(category='static_tab', name=tab.url_slug)
-                static_tabs.append(modulestore('direct').get_item(static_tab_loc))
-            else:
-                built_in_tabs.append(tab)
 
-        # create a list of components for each static tab
-        components = [
-            loc_mapper().translate_location(
-                course_item.location.course_id, static_tab.location, False, True
+def edit_tab_handler(course_item, request):
+    """
+    Helper function for handling requests to edit settings of a single tab
+    """
+
+    # Tabs are identified by tab_id or locator
+    tab_id_locator = request.json['tab_id_locator']
+
+    # Find the given tab in the course
+    tab = get_tab_by_tab_id_locator(course_item.tabs, tab_id_locator)
+    if tab is None:
+        return JsonResponse(
+            {"error": "Tab with id_locator '{0}' does not exist.".format(tab_id_locator)}, status=400
         )
-            for static_tab
-            in static_tabs
-        ]
 
-        return render_to_response('edit-tabs.html', {
-            'context_course': course_item,
-            'built_in_tabs': built_in_tabs,
-            'components': components,
-            'course_locator': locator
-        })
+    if 'is_hidden' in request.json:
+        # set the is_hidden attribute on the requested tab
+        tab.is_hidden = request.json['is_hidden']
+        modulestore('direct').update_item(course_item, request.user.id)
     else:
-        return HttpResponseNotFound()
+        raise NotImplementedError('Unsupported request to edit tab: {0}'.format(request.json))
+
+    return JsonResponse()
+
+
+def get_tab_by_tab_id_locator(tab_list, tab_id_locator):
+    """
+    Look for a tab with the specified tab_id or locator.  Returns the first matching tab.
+    """
+    if 'tab_id' in tab_id_locator:
+        tab = CourseTabList.get_tab_by_id(tab_list, tab_id_locator['tab_id'])
+    elif 'tab_locator' in tab_id_locator:
+        tab = get_tab_by_locator(tab_list, tab_id_locator['tab_locator'])
+    return tab
+
+
+def get_tab_by_locator(tab_list, tab_locator):
+    """
+    Look for a tab with the specified locator.  Returns the first matching tab.
+    """
+    tab_location = loc_mapper().translate_locator_to_location(BlockUsageLocator(tab_locator))
+    item = modulestore('direct').get_item(tab_location)
+    static_tab = StaticTab(
+        name=item.display_name,
+        url_slug=item.location.name,
+    )
+    return CourseTabList.get_tab_by_id(tab_list, static_tab.tab_id)
 
 
 # "primitive" tab edit functions driven by the command line.

cms/djangoapps/contentstore/views/tests/test_course_index.py

@@ -61,7 +61,7 @@ class TestCourseIndex(CourseTestCase):
         """
         outline_url = self.course_locator.url_reverse('course/', '')
         # register a non-staff member and try to delete the course branch
-        non_staff_client, _ = self.createNonStaffAuthedUserClient()
+        non_staff_client, _ = self.create_non_staff_authed_user_client()
         response = non_staff_client.delete(outline_url, {}, HTTP_ACCEPT='application/json')
         self.assertEqual(response.status_code, 403)
 
@@ -69,7 +69,7 @@ class TestCourseIndex(CourseTestCase):
         """
         Make and register an course_staff and ensure they can access the courses
         """
-        course_staff_client, course_staff = self.createNonStaffAuthedUserClient()
+        course_staff_client, course_staff = self.create_non_staff_authed_user_client()
         for course in [self.course, self.odd_course]:
             new_location = loc_mapper().translate_location(course.location.course_id, course.location, False, True)
             permission_url = new_location.url_reverse("course_team/", course_staff.email)

cms/djangoapps/contentstore/views/tests/test_tabs.py

 """ Tests for tab functions (just primitive). """
 
+import json
 from contentstore.views import tabs
+from contentstore.tests.utils import CourseTestCase
 from django.test import TestCase
-from xmodule.modulestore.tests.factories import CourseFactory
+from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 from courseware.courses import get_course_by_id
+from xmodule.tabs import CourseTabList, WikiTab
+
+
+class TabsPageTests(CourseTestCase):
+    """Test cases for Tabs (a.k.a Pages) page"""
+
+    def setUp(self):
+        """Common setup for tests"""
+
+        # call super class to setup course, etc.
+        super(TabsPageTests, self).setUp()
+
+        # Set the URL for tests
+        self.url = self.course_locator.url_reverse('tabs')
+
+        # add a static tab to the course, for code coverage
+        ItemFactory.create(
+            parent_location=self.course_location,
+            category="static_tab",
+            display_name="Static_1"
+        )
+        self.reload_course()
+
+    def check_invalid_tab_id_response(self, resp):
+        """Verify response is an error listing the invalid_tab_id"""
+
+        self.assertEqual(resp.status_code, 400)
+        resp_content = json.loads(resp.content)
+        self.assertIn("error", resp_content)
+        self.assertIn("invalid_tab_id", resp_content['error'])
+
+    def test_not_implemented(self):
+        """Verify not implemented errors"""
+
+        # JSON GET request not supported
+        with self.assertRaises(NotImplementedError):
+            self.client.get(self.url)
+
+        # JSON POST request not supported
+        with self.assertRaises(NotImplementedError):
+            self.client.ajax_post(
+                self.url,
+                data=json.dumps({
+                    'tab_id_locator': {'tab_id': WikiTab.type},
+                    'unsupported_request': None,
+                }),
+            )
+
+        # invalid JSON POST request
+        with self.assertRaises(NotImplementedError):
+            self.client.ajax_post(
+                self.url,
+                data={'invalid_request': None},
+            )
+
+    def test_view_index(self):
+        """Basic check that the Pages page responds correctly"""
+
+        resp = self.client.get_html(self.url)
+        self.assertEqual(resp.status_code, 200)
+        self.assertIn('course-nav-list', resp.content)
+
+    def test_reorder_tabs(self):
+        """Test re-ordering of tabs"""
+
+        # get the original tab ids
+        orig_tab_ids = [tab.tab_id for tab in self.course.tabs]
+        tab_ids = list(orig_tab_ids)
+        num_orig_tabs = len(orig_tab_ids)
+
+        # make sure we have enough tabs to play around with
+        self.assertTrue(num_orig_tabs >= 5)
+
+        # reorder the last two tabs
+        tab_ids[num_orig_tabs - 1], tab_ids[num_orig_tabs - 2] = tab_ids[num_orig_tabs - 2], tab_ids[num_orig_tabs - 1]
+
+        # remove the middle tab
+        # (the code needs to handle the case where tabs requested for re-ordering is a subset of the tabs in the course)
+        removed_tab = tab_ids.pop(num_orig_tabs / 2)
+        self.assertTrue(len(tab_ids) == num_orig_tabs - 1)
+
+        # post the request
+        resp = self.client.ajax_post(
+            self.url,
+            data={'tabs': [{'tab_id': tab_id} for tab_id in tab_ids]},
+        )
+        self.assertEqual(resp.status_code, 204)
+
+        # reload the course and verify the new tab order
+        self.reload_course()
+        new_tab_ids = [tab.tab_id for tab in self.course.tabs]
+        self.assertEqual(new_tab_ids, tab_ids + [removed_tab])
+        self.assertNotEqual(new_tab_ids, orig_tab_ids)
+
+    def test_reorder_tabs_invalid_list(self):
+        """Test re-ordering of tabs with invalid tab list"""
+
+        orig_tab_ids = [tab.tab_id for tab in self.course.tabs]
+        tab_ids = list(orig_tab_ids)
+
+        # reorder the first two tabs
+        tab_ids[0], tab_ids[1] = tab_ids[1], tab_ids[0]
+
+        # post the request
+        resp = self.client.ajax_post(
+            self.url,
+            data={'tabs': [{'tab_id': tab_id} for tab_id in tab_ids]},
+        )
+        self.assertEqual(resp.status_code, 400)
+        resp_content = json.loads(resp.content)
+        self.assertIn("error", resp_content)
+
+    def test_reorder_tabs_invalid_tab(self):
+        """Test re-ordering of tabs with invalid tab"""
+
+        invalid_tab_ids = ['courseware', 'info', 'invalid_tab_id']
+
+        # post the request
+        resp = self.client.ajax_post(
+            self.url,
+            data={'tabs': [{'tab_id': tab_id} for tab_id in invalid_tab_ids]},
+        )
+        self.check_invalid_tab_id_response(resp)
+
+    def check_toggle_tab_visiblity(self, tab_type, new_is_hidden_setting):
+        """Helper method to check changes in tab visibility"""
+
+        # find the tab
+        old_tab = CourseTabList.get_tab_by_type(self.course.tabs, tab_type)
+
+        # visibility should be different from new setting
+        self.assertNotEqual(old_tab.is_hidden, new_is_hidden_setting)
+
+        # post the request
+        resp = self.client.ajax_post(
+            self.url,
+            data=json.dumps({
+                'tab_id_locator': {'tab_id': old_tab.tab_id},
+                'is_hidden': new_is_hidden_setting,
+            }),
+        )
+        self.assertEqual(resp.status_code, 204)
+
+        # reload the course and verify the new visibility setting
+        self.reload_course()
+        new_tab = CourseTabList.get_tab_by_type(self.course.tabs, tab_type)
+        self.assertEqual(new_tab.is_hidden, new_is_hidden_setting)
+
+    def test_toggle_tab_visibility(self):
+        """Test toggling of tab visiblity"""
+
+        self.check_toggle_tab_visiblity(WikiTab.type, True)
+        self.check_toggle_tab_visiblity(WikiTab.type, False)
+
+    def test_toggle_invalid_tab_visibility(self):
+        """Test toggling visibility of an invalid tab"""
+
+        # post the request
+        resp = self.client.ajax_post(
+            self.url,
+            data=json.dumps({
+                'tab_id_locator': {'tab_id': 'invalid_tab_id'}
+            }),
+        )
+        self.check_invalid_tab_id_response(resp)
 
 
 class PrimitiveTabEdit(TestCase):

cms/djangoapps/contentstore/views/tests/test_textbooks.py

@@ -56,8 +56,7 @@ class TextbookIndexTestCase(CourseTestCase):
             }
         ]
         self.course.pdf_textbooks = content
-        store = get_modulestore(self.course.location)
-        store.update_item(self.course, self.user.id)
+        self.save_course()
 
         resp = self.client.get(
             self.url,
@@ -83,12 +82,10 @@ class TextbookIndexTestCase(CourseTestCase):
         )
         self.assertEqual(resp.status_code, 200)
 
-        # reload course
-        store = get_modulestore(self.course.location)
-        course = store.get_item(self.course.location)
         # should be the same, except for added ID
         no_ids = []
-        for textbook in course.pdf_textbooks:
+        self.reload_course()
+        for textbook in self.course.pdf_textbooks:
             del textbook["id"]
             no_ids.append(textbook)
         self.assertEqual(no_ids, textbooks)
@@ -193,9 +190,7 @@ class TextbookDetailTestCase(CourseTestCase):
         self.course.pdf_textbooks = [self.textbook1, self.textbook2]
         # Save the data that we've just changed to the underlying
         # MongoKeyValueStore before we update the mongo datastore.
-        self.course.save()
-        self.store = get_modulestore(self.course.location)
-        self.store.update_item(self.course, self.user.id)
+        self.save_course()
         self.url_nonexist = self.course_locator.url_reverse("textbooks", "20")
 
     def test_get_1(self):
@@ -221,15 +216,15 @@ class TextbookDetailTestCase(CourseTestCase):
         "Delete a textbook by ID"
         resp = self.client.delete(self.url1)
         self.assertEqual(resp.status_code, 204)
-        course = self.store.get_item(self.course.location)
-        self.assertEqual(course.pdf_textbooks, [self.textbook2])
+        self.reload_course()
+        self.assertEqual(self.course.pdf_textbooks, [self.textbook2])
 
     def test_delete_nonexistant(self):
         "Delete a textbook by ID, when the ID doesn't match an existing textbook"
         resp = self.client.delete(self.url_nonexist)
         self.assertEqual(resp.status_code, 404)
-        course = self.store.get_item(self.course.location)
-        self.assertEqual(course.pdf_textbooks, [self.textbook1, self.textbook2])
+        self.reload_course()
+        self.assertEqual(self.course.pdf_textbooks, [self.textbook1, self.textbook2])
 
     def test_create_new_by_id(self):
         "Create a textbook by ID"
@@ -249,9 +244,9 @@ class TextbookDetailTestCase(CourseTestCase):
         self.assertEqual(resp2.status_code, 200)
         compare = json.loads(resp2.content)
         self.assertEqual(compare, textbook)
-        course = self.store.get_item(self.course.location)
+        self.reload_course()
         self.assertEqual(
-            course.pdf_textbooks,
+            self.course.pdf_textbooks,
             [self.textbook1, self.textbook2, textbook]
         )
 

cms/envs/common.py

@@ -43,9 +43,11 @@ FEATURES = {
 
     'GITHUB_PUSH': False,
 
-    # for consistency in user-experience, keep the value of this setting in sync with the
-    # one in lms/envs/common.py
+    # for consistency in user-experience, keep the value of the following 3 settings
+    # in sync with the ones in lms/envs/common.py
     'ENABLE_DISCUSSION_SERVICE': True,
+    'ENABLE_TEXTBOOK': True,
+    'ENABLE_STUDENT_NOTES': True,
 
     'AUTH_USE_CERTIFICATES': False,
 

cms/static/coffee/src/views/tabs.coffee

@@ -18,7 +18,8 @@ define ["jquery", "jquery.ui", "backbone", "js/views/feedback_prompt", "js/views
 
       @options.mast.find('.new-tab').on('click', @addNewTab)
       $('.add-pages .new-tab').on('click', @addNewTab)
-      @$('.components').sortable(
+      $('.toggle-checkbox').on('click', @toggleVisibilityOfTab)
+      @$('.course-nav-list').sortable(
         handle: '.drag-handle'
         update: @tabMoved
         helper: 'clone'
@@ -26,13 +27,38 @@ define ["jquery", "jquery.ui", "backbone", "js/views/feedback_prompt", "js/views
         placeholder: 'component-placeholder'
         forcePlaceholderSize: true
         axis: 'y'
-        items: '> .component'
+        items: '> .is-movable'
       )
 
+    toggleVisibilityOfTab: (event, ui) =>
+      checkbox_element = event.srcElement
+      tab_element = $(checkbox_element).parents(".course-tab")[0]
+
+      saving = new NotificationView.Mini({title: gettext("Saving…")})
+      saving.show()
+
+      $.ajax({
+        type:'POST',
+        url: @model.url(),
+        data: JSON.stringify({
+          tab_id_locator : {
+            tab_id: $(tab_element).data('tab-id'),
+            tab_locator: $(tab_element).data('locator')
+          },
+          is_hidden : $(checkbox_element).is(':checked')
+        }),
+        contentType: 'application/json'
+      }).success(=> saving.hide())
+
     tabMoved: (event, ui) =>
       tabs = []
-      @$('.component').each((idx, element) =>
-          tabs.push($(element).data('locator'))
+      @$('.course-tab').each((idx, element) =>
+          tabs.push(
+            {
+              tab_id: $(element).data('tab-id'),
+              tab_locator: $(element).data('locator')
+            }
+          )
       )
 
       analytics.track "Reordered Pages",
@@ -59,6 +85,7 @@ define ["jquery", "jquery.ui", "backbone", "js/views/feedback_prompt", "js/views
       )
 
       $('.new-component-item').before(editor.$el)
+      editor.$el.addClass('course-tab is-movable')
       editor.$el.addClass('new')
       setTimeout(=>
         editor.$el.removeClass('new')

cms/static/sass/views/_static-pages.scss

@@ -180,7 +180,7 @@
   }
 
   .component,
-  .course-nav-tab {
+  .course-nav-item {
     position: relative;
     border: 1px solid $mediumGrey;
     border-top: none;
@@ -239,7 +239,7 @@
     }
 
     .component-actions,
-    .course-nav-tab-actions {
+    .course-nav-item-actions {
       display: inline-block;
       float: right;
       margin-right: ($baseline*2);
@@ -289,30 +289,31 @@
   }
 
   // basic course nav items - overrides from above
-  .course-nav-tab {
+  .course-nav-item {
     padding: ($baseline*.75) ($baseline/4) ($baseline*.75) $baseline;
+    background: $white;
 
-    &.fixed {
+    &.is-fixed {
+      @extend %ui-disabled;
       @include transition(opacity $tmg-f2 ease-in-out 0s);
-      opacity: .7;
-
-      &:hover {
-        opacity: 1;
-      }
+      opacity: 0.5;
     }
 
-    .course-nav-tab-header {
+    .course-nav-item-header {
       display: inline-block;
       width:80%;
 
       .title {
         @extend %t-title4;
-        font-weight: 300;
-        color: $gray;
+      }
+
+      .title-sub {
+        @extend %t-title7;
+         color: $gray-l2;
       }
     }
 
-    .course-nav-tab-actions {
+    .course-nav-item-actions {
       display: inline-block;
       padding: ($baseline/10);
     }
@@ -335,7 +336,6 @@
     @include transition(background-color $tmg-s3 linear 0s);
     padding: 20px 20px 22px;
     font-size: 24px;
-    font-weight: 300;
     background: #fff;
   }
 
@@ -395,4 +395,3 @@
     outline: 0;
   }
 }
-

cms/templates/edit-tabs.html

@@ -3,6 +3,7 @@
 <%!
   from django.utils.translation import ugettext as _
   from django.core.urlresolvers import reverse
+  from xmodule.tabs import StaticTab
 %>
 <%block name="title">${_("Pages")}</%block>
 <%block name="bodyclass">is-signedin course view-static-pages</%block>
@@ -41,6 +42,9 @@
         <li class="nav-item">
           <a href="#" class="button new-button new-tab"><i class="icon-plus"></i> ${_("New Page")}</a>
         </li>
+        <li class="nav-item">
+          <a href="${lms_link}" rel="external" class="button view-button view-live-button">${_("View Live")}</a>
+        </li>
       </ul>
     </nav>
   </header>
@@ -54,39 +58,73 @@
         <article class="unit-body">
 
           <div class="tab-list">
-            <ol class="course-nav-tab-list components">
+            <ol class="course-nav-list course components">
+
+              % for tab in tabs_to_render:
+                <%
+                  css_class = "course-tab"
+                  if tab.is_movable:
+                    css_class = css_class + " is-movable"
+                  elif (not tab.is_movable) and (not tab.is_hideable):
+                    css_class = css_class + " is-fixed"
+                %>
+
+                % if isinstance(tab, StaticTab):
+                    <li class="component ${css_class}" data-locator="${tab.locator}" data-tab-id="${tab.tab_id}"></li>
+
+                % else:
+                    <li class="course-nav-item ${css_class}" data-tab-id="${tab.tab_id}">
+                      <div class="course-nav-item-header">
+
+                        % if tab.is_collection:
+
+                          <h3 class="title-sub">${_(tab.name)}</h3>
+                            <ul class="course-nav-item-children">
+                            % for item in tab.items(context_course):
+                              <li class="course-nav-item-child title">
+                                ${_(item.name)}
+                              </li>
+                            % endfor
+                            </ul>
 
-              % for tab in built_in_tabs:
-                <li class="course-nav-tab fixed">
-                  <div class="course-nav-tab-header">
+                        % else:
                           <h3 class="title">${_(tab.name)}</h3>
+                        % endif
                       </div>
-                  <div class="course-nav-tab-actions wrapper-actions-list">
+
+                      <div class="course-nav-item-actions wrapper-actions-list">
                         <ul class="actions-list">
 
                         % if tab.is_hideable:
                           <li class="action-item action-visible">
-                          <label for="[id]"><span class="sr">${_("Show this page")}</span></label>
-                          <input type="checkbox" id="[id]" class="toggle-checkbox" data-tooltip="${_('Show/hide page')}" />
+                            <label><span class="sr">${_("Show this page")}</span></label>
+                            % if tab.is_hidden:
+                              <input type="checkbox" class="toggle-checkbox" data-tooltip="${_('Show/hide page')}" checked />
+                            % else:
+                              <input type="checkbox" class="toggle-checkbox" data-tooltip="${_('Show/hide page')}" />
+                            % endif
                             <div class="action-button"><i class="icon-eye-open"></i><i class="icon-eye-close"></i></div>
                           </li>
-                   %endif
+                        % endif
 
                         </ul>
                       </div>
-                  <div class="drag-handle is-fixed" data-tooltip="${_('Cannot be reordered')}">
-                    <span class="sr">${_("Fixed page")}</span>
+
+                      % if tab.is_movable:
+                        <div class="drag-handle" data-tooltip="${_('Drag to reorder')}">
+                          <span class="sr">${_("Drag to reorder")}</span>
+                        </div>
+                      % else:
+                        <div class="drag-handle is-fixed" data-tooltip="${_('This page cannot be reordered')}">
+                          <span class="sr">${_("This page cannot be reordered")}</span>
                         </div>
+                      % endif
                     </li>
-              % endfor
 
-              % for locator in components:
-              <li class="component" data-locator="${locator}"></li>
+                 % endif
               % endfor
 
-              <li class="new-component-item">
-
-              </li>
+              <li class="new-component-item"></li>
             </ol>
           </div>
 

common/lib/xmodule/xmodule/modulestore/mongo/base.py

@@ -797,7 +797,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
             if xblock.category == 'static_tab':
                 course = self._get_course_for_item(xblock.location)
                 # find the course's reference to this tab and update the name.
-                static_tab = CourseTabList.get_tab_by_slug(course, xblock.location.name)
+                static_tab = CourseTabList.get_tab_by_slug(course.tabs, xblock.location.name)
                 # only update if changed
                 if static_tab and static_tab['name'] != xblock.display_name:
                     static_tab['name'] = xblock.display_name

common/lib/xmodule/xmodule/modulestore/xml.py

@@ -663,7 +663,7 @@ class XMLModuleStore(ModuleStoreReadBase):
                         # Hack because we need to pull in the 'display_name' for static tabs (because we need to edit them)
                         # from the course policy
                         if category == "static_tab":
-                            tab = CourseTabList.get_tab_by_slug(course=course_descriptor, url_slug=slug)
+                            tab = CourseTabList.get_tab_by_slug(tab_list=course_descriptor.tabs, url_slug=slug)
                             if tab:
                                 module.display_name = tab.name
                         module.data_dir = course_dir

lms/djangoapps/courseware/tests/test_tabs.py

@@ -45,7 +45,7 @@ class StaticTabDateTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):
     def test_get_static_tab_contents(self):
         course = get_course_by_id('edX/toy/2012_Fall')
         request = get_request_for_user(UserFactory.create())
-        tab = CourseTabList.get_tab_by_slug(course, 'resources')
+        tab = CourseTabList.get_tab_by_slug(course.tabs, 'resources')
 
         # Test render works okay
         tab_content = get_static_tab_contents(request, course, tab)

lms/djangoapps/courseware/views.py

@@ -482,7 +482,7 @@ def static_tab(request, course_id, tab_slug):
     """
     course = get_course_with_access(request.user, course_id, 'load')
 
-    tab = CourseTabList.get_tab_by_slug(course, tab_slug)
+    tab = CourseTabList.get_tab_by_slug(course.tabs, tab_slug)
     if tab is None:
         raise Http404
 

lms/envs/common.py

@@ -76,10 +76,11 @@ FEATURES = {
     'FORCE_UNIVERSITY_DOMAIN': False,  # set this to the university domain to use, as an override to HTTP_HOST
                                         # set to None to do no university selection
 
-    'ENABLE_TEXTBOOK': True,
-
-    # for consistency in user-experience, keep the value of this setting in sync with the one in cms/envs/common.py
+    # for consistency in user-experience, keep the value of the following 3 settings
+    # in sync with the corresponding ones in cms/envs/common.py
     'ENABLE_DISCUSSION_SERVICE': True,
+    'ENABLE_TEXTBOOK': True,
+    'ENABLE_STUDENT_NOTES': True,  # enables the student notes API and UI.
 
     # discussion home panel, which includes a subscription on/off setting for discussion digest emails.
     # this should remain off in production until digest notifications are online.
@@ -146,9 +147,6 @@ FEATURES = {
     # segment.io for LMS--need to explicitly turn it on for production.
     'SEGMENT_IO_LMS': False,
 
-    # Enables the student notes API and UI.
-    'ENABLE_STUDENT_NOTES': True,
-
     # Provide a UI to allow users to submit feedback from the LMS (left-hand help modal)
     'ENABLE_FEEDBACK_SUBMISSION': False,
 

lms/templates/courseware/course_navigation.html

@@ -23,7 +23,7 @@ def url_class(is_active):
 <nav class="${active_page} course-material">
   <div class="inner-wrapper">
     <ol class="course-tabs">
-      % for tab in CourseTabList.iterate_displayable(course, settings, user.is_authenticated(), has_access(user, course, 'staff'), include_instructor_tab=True):
+      % for tab in CourseTabList.iterate_displayable(course, settings, user.is_authenticated(), has_access(user, course, 'staff')):
         <%
             tab_is_active = (tab.tab_id == active_page)
             tab_image = notification_image_for_tab(tab, user, course)