Add the ability to reorder Pages and hide the Wiki page.

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 reorder the static pages
+        Then the static pages are in the reverse order
         And I reload the page
-        Then the static tabs are in the reverse order
+        Then the static pages are in the reverse order
+
+    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 reorder the pages
+        Then the built-in pages are in the reverse order
+        And I reload the page
+        Then the built-in pages are in the reverse order
+
+    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 reorder the pages
+        Then the pages are in the reverse order
+        And I reload the page
+        Then the pages are in the reverse order
+
+    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,7 @@
 # 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
 
 
 @step(u'I go to the pages page$')
@@ -33,15 +33,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,15 +51,9 @@ 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 reorder the static pages')
+def reorder_static_pages(_step):
+    reorder_pages_with_css_class('.component')
 
 
 @step(u'I have created a static page')
@@ -89,21 +74,79 @@ def create_two_pages(step):
     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 in the reverse order')
+def static_pages_in_reverse_order(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 pages[0].text == first
+    assert 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 in the reverse order')
+def built_in_pages_in_reverse_order(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 in the reverse order')
+def pages_in_reverse_order(step):
+    expected_pages = ['Courseware', 'Course Info', 'Wiki', 'Progress', 'First', 'Empty', 'Discussion']
+    see_pages_in_expected_order(expected_pages)
+
+
+@step(u'I reorder the pages')
+def reorder_pages(step):
+    reorder_pages_with_css_class('.sortable-tab')
+
+
+@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 world.css_find("li[data-tab-id='{0}'] input.toggle-checkbox".format(page_id)).checked == hidden
+
+
+@step(u'I toggle the visibility of the "([^"]*)" page')
+def page_toggle_visibility(step, page_id):
+    world.css_find("li[data-tab-id='{0}'] input.toggle-checkbox".format(page_id))[0].click()
+
+
+def reorder_pages_with_css_class(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

@@ -151,7 +151,7 @@ class TestGitExport(CourseTestCase):
         self.assertEqual(expect_string, git_log)
 
         # Make changes to course so there is something commit
-        self.populateCourse()
+        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,24 @@ 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)))
+                reverse_static_tabs.insert(0, tab)
+            else:
+                built_in_tabs.append(tab)
+
+        tab_ids = [{'tab_id': tab.tab_id} for tab in (built_in_tabs + reverse_static_tabs)]
 
-        self.client.ajax_post(new_location.url_reverse('tabs'), {'tabs': reverse_tabs})
+        self.client.ajax_post(new_location.url_reverse('tabs'), {'tabs': tab_ids})
 
         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/tabs.py

@@ -14,12 +14,10 @@ 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 _
-
 __all__ = ['tabs_handler']
 
 @expect_json
@@ -53,85 +51,132 @@ 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))
-
-                tabs = request.json['tabs']
-
-                # 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
-                    )
-
-                # 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:
-                        return JsonResponse(
-                            {"error": "no tab for found location {}".format(tab)}, status=400
-                        )
-
-                    tab_items.append(item)
-
-                # 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,
-                            )
-                        )
-                        static_tab_idx += 1
-                    else:
-                        reordered_tabs.append(tab)
-
-                # OK, re-assemble the static tabs in the new order
-                course_item.tabs = reordered_tabs
-                modulestore('direct').update_item(course_item, request.user.id)
-                return JsonResponse()
+                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.')
+
     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
+        # present in the same order they are displayed in LMS
 
-        static_tabs = []
-        built_in_tabs = []
-        for tab in CourseTabList.iterate_displayable(course_item, settings, include_instructor_tab=False):
+        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)
-                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
-            )
-            for static_tab
-            in static_tabs
-        ]
+                static_tab = modulestore('direct').get_item(static_tab_loc)
+                tab.locator = loc_mapper().translate_location(
+                    course_item.location.course_id, static_tab.location, False, True
+                )
+            tabs_to_render.append(tab)
 
         return render_to_response('edit-tabs.html', {
             'context_course': course_item,
-            'built_in_tabs': built_in_tabs,
-            'components': components,
+            'tabs_to_render': tabs_to_render,
             'course_locator': locator
         })
     else:
         return HttpResponseNotFound()
 
 
+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.
+    ids_locators_of_new_tab_order = 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 ids_locators_of_new_tab_order:
+        tab = get_tab_by_tab_id_locator(old_tab_list, tab_id_locator)
+        if tab is None:
+            return JsonResponse(
+                {"error": "Tab with id_locator '{0}' does not exist.".format(tab_id_locator)}, status=400
+            )
+        new_tab_list.append(tab)
+
+    # 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)
+
+    # 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
+        )
+
+    # persist the new order of the tabs
+    course_item.tabs = new_tab_list
+    modulestore('direct').update_item(course_item, request.user.id)
+
+    return JsonResponse()
+
+
+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
+        )
+
+    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:
+        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.
 # These should be replaced/deleted by a more capable GUI someday.
 # Note that the command line UI identifies the tabs with 1-based

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={'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-tab-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/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-tab-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: '> .sortable-tab'
       )
 
+    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 sortable-tab')
       editor.$el.addClass('new')
       setTimeout(=>
         editor.$el.removeClass('new')

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>
@@ -56,37 +57,54 @@
           <div class="tab-list">
             <ol class="course-nav-tab-list components">
 
-              % for tab in built_in_tabs:
-                <li class="course-nav-tab fixed">
-                  <div class="course-nav-tab-header">
-                    <h3 class="title">${_(tab.name)}</h3>
-                  </div>
-                  <div class="course-nav-tab-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')}" />
-                          <div class="action-button"><i class="icon-eye-open"></i><i class="icon-eye-close"></i></div>
-                      </li>
-                   %endif
-
-                    </ul>
-                  </div>
-                  <div class="drag-handle is-fixed" data-tooltip="${_('Cannot be reordered')}">
-                    <span class="sr">${_("Fixed page")}</span>
-                  </div>
-                </li>
+              % for tab in tabs_to_render:
+                % if isinstance(tab, StaticTab):
+                    <li class="component course-tab sortable-tab" data-locator="${tab.locator}" data-tab-id="${tab.tab_id}"></li>
+                % else:
+
+                  <%
+                    tab_name = _(tab.name)
+                    if tab.is_collection:
+                        item_names = [_(item.name) for item in tab.items(context_course)]
+                        num_items = sum(1 for item in tab.items(context_course))
+                        tab_name = tab_name + " ({0}): {1}".format(num_items, ", ".join(item_names))
+                    css_class = "course-nav-tab course-tab"
+                    if tab.is_movable:
+                        css_class = css_class + " sortable-tab"
+                   %>
+
+                    <li class="${css_class}" data-tab-id="${tab.tab_id}">
+                      <div class="course-nav-tab-header">
+                        <h3 class="title">${tab_name}</h3>
+                      </div>
+                      <div class="course-nav-tab-actions wrapper-actions-list">
+                        <ul class="actions-list">
+
+                        % if tab.is_hideable:
+                          <li class="action-item action-visible">
+                              <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
+
+                        </ul>
+                      </div>
+
+                      % if tab.is_movable:
+                        <div class="drag-handle" data-tooltip="${_('Drag to reorder')}">
+                          <span class="sr">${_("Fixed page")}</span>
+                        </div>
+                      % endif
+                    </li>
+               % endif
               % endfor
 
-              % for locator in components:
-              <li class="component" data-locator="${locator}"></li>
-              % 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

common/lib/xmodule/xmodule/tests/test_tabs.py

@@ -14,6 +14,16 @@ class TabTestCase(unittest.TestCase):
         self.settings = MagicMock()
         self.settings.FEATURES = {}
         self.reverse = lambda name, args: "name/{0}/args/{1}".format(name, ",".join(str(a) for a in args))
+        self.books = None
+
+    def set_up_books(self, num_books):
+        """initializes the textbooks in the course and adds the given number of books to each textbook"""
+        self.books = [MagicMock() for _ in range(num_books)]
+        for book_index, book in enumerate(self.books):
+            book.title = 'Book{0}'.format(book_index)
+        self.course.textbooks = self.books
+        self.course.pdf_textbooks = self.books
+        self.course.html_textbooks = self.books
 
     def check_tab(
             self,
@@ -57,22 +67,30 @@ class TabTestCase(unittest.TestCase):
         self.check_get_and_set_methods(tab)
 
         # check to_json and from_json methods
-        serialized_tab = tab.to_json()
-        deserialized_tab = tab_class.from_json(serialized_tab)
-        self.assertEquals(serialized_tab, deserialized_tab)
+        self.check_tab_json_methods(tab)
 
         # check equality methods
+        self.check_tab_equality(tab, dict_tab)
+
+        # return tab for any additional tests
+        return tab
+
+    def check_tab_equality(self, tab, dict_tab):
+        """tests the equality methods on the given tab"""
         self.assertEquals(tab, dict_tab)  # test __eq__
         ne_dict_tab = dict_tab
         ne_dict_tab['type'] = 'fake_type'
         self.assertNotEquals(tab, ne_dict_tab)  # test __ne__: incorrect type
         self.assertNotEquals(tab, {'fake_key': 'fake_value'})  # test __ne__: missing type
 
-        # return tab for any additional tests
-        return tab
+    def check_tab_json_methods(self, tab):
+        """tests the json from and to methods on the given tab"""
+        serialized_tab = tab.to_json()
+        deserialized_tab = tab.from_json(serialized_tab)
+        self.assertEquals(serialized_tab, deserialized_tab)
 
     def check_can_display_results(self, tab, expected_value=True, for_authenticated_users_only=False, for_staff_only=False):
-        """Check can display results for various users"""
+        """checks can display results for various users"""
         if for_staff_only:
             self.assertEquals(
                 expected_value,
@@ -149,17 +167,31 @@ class WikiTestCase(TabTestCase):
         )
 
     def test_wiki_enabled(self):
+        """Test wiki tab when Enabled setting is True"""
 
         self.settings.WIKI_ENABLED = True
         tab = self.check_wiki_tab()
         self.check_can_display_results(tab)
 
     def test_wiki_enabled_false(self):
+        """Test wiki tab when Enabled setting is False"""
 
         self.settings.WIKI_ENABLED = False
         tab = self.check_wiki_tab()
         self.check_can_display_results(tab, expected_value=False)
 
+    def test_wiki_visibility(self):
+        """Test toggling of visibility of wiki tab"""
+
+        wiki_tab = tabs.WikiTab()
+        self.assertTrue(wiki_tab.is_hideable)
+        wiki_tab.is_hidden = True
+        self.assertTrue(wiki_tab['is_hidden'])
+        self.check_tab_json_methods(wiki_tab)
+        self.check_tab_equality(wiki_tab, wiki_tab.to_json())
+        wiki_tab['is_hidden'] = False
+        self.assertFalse(wiki_tab.is_hidden)
+
 
 class ExternalLinkTestCase(TabTestCase):
     """Test cases for External Link Tab."""
@@ -202,15 +234,9 @@ class TextbooksTestCase(TabTestCase):
     def setUp(self):
         super(TextbooksTestCase, self).setUp()
 
+        self.set_up_books(2)
+
         self.dict_tab = MagicMock()
-        book1 = MagicMock()
-        book2 = MagicMock()
-        book1.title = 'Book1: Algebra'
-        book2.title = 'Book2: Topology'
-        books = [book1, book2]
-        self.course.textbooks = books
-        self.course.pdf_textbooks = books
-        self.course.html_textbooks = books
         self.course.tabs = [
             tabs.CoursewareTab(),
             tabs.CourseInfoTab(),
@@ -219,7 +245,7 @@ class TextbooksTestCase(TabTestCase):
             tabs.HtmlTextbookTabs(),
         ]
         self.num_textbook_tabs = sum(1 for tab in self.course.tabs if isinstance(tab, tabs.TextbookTabsBase))
-        self.num_textbooks = self.num_textbook_tabs * len(books)
+        self.num_textbooks = self.num_textbook_tabs * len(self.books)
 
     def test_textbooks_enabled(self):
 
@@ -233,7 +259,7 @@ class TextbooksTestCase(TabTestCase):
                 book_type, book_index = tab.tab_id.split("/", 1)
                 expected_link = self.reverse(type_to_reverse_name[book_type], args=[self.course.id, book_index])
                 self.assertEqual(tab.link_func(self.course, self.reverse), expected_link)
-                self.assertTrue(tab.name.startswith('Book{0}:'.format(1 + int(book_index))))
+                self.assertTrue(tab.name.startswith('Book{0}'.format(book_index)))
                 num_textbooks_found = num_textbooks_found + 1
         self.assertEquals(num_textbooks_found, self.num_textbooks)
 
@@ -381,10 +407,11 @@ class NeedNameTestCase(unittest.TestCase):
             tabs.need_name(self.invalid_dict)
 
 
-class ValidateTabsTestCase(unittest.TestCase):
-    """Test cases for validating tabs."""
+class TabListTestCase(TabTestCase):
+    """Base class for Test cases involving tab lists."""
 
     def setUp(self):
+        super(TabListTestCase, self).setUp()
 
         # invalid tabs
         self.invalid_tabs = [
@@ -447,6 +474,12 @@ class ValidateTabsTestCase(unittest.TestCase):
             ],
         ]
 
+        self.all_valid_tab_list = tabs.CourseTabList().from_json(self.valid_tabs[1])
+
+
+class ValidateTabsTestCase(TabListTestCase):
+    """Test cases for validating tabs."""
+
     def test_validate_tabs(self):
         tab_list = tabs.CourseTabList()
         for invalid_tab_list in self.invalid_tabs:
@@ -458,7 +491,7 @@ class ValidateTabsTestCase(unittest.TestCase):
             self.assertEquals(len(from_json_result), len(valid_tab_list))
 
 
-class CourseTabListTestCase(TabTestCase):
+class CourseTabListTestCase(TabListTestCase):
     """Testing the generator method for iterating through displayable tabs"""
 
     def test_initialize_default_without_syllabus(self):
@@ -488,23 +521,51 @@ class CourseTabListTestCase(TabTestCase):
         self.assertTrue(tabs.DiscussionTab() in self.course.tabs)
 
     def test_iterate_displayable(self):
+        # enable all tab types
         self.settings.FEATURES['ENABLE_TEXTBOOK'] = True
-        self.course.tabs = [
-            tabs.CoursewareTab(),
-            tabs.CourseInfoTab(),
-            tabs.WikiTab(),
-        ]
+        self.settings.FEATURES['ENABLE_DISCUSSION_SERVICE'] = True
+        self.settings.FEATURES['ENABLE_STUDENT_NOTES'] = True
+        self.course.hide_progress_tab = False
+
+        # create 1 book per textbook type
+        self.set_up_books(1)
 
+        # initialize the course tabs to a list of all valid tabs
+        self.course.tabs = self.all_valid_tab_list
+
+        # enumerate the tabs using the CMS call
+        for i, tab in enumerate(tabs.CourseTabList.iterate_displayable_cms(
+            self.course,
+            self.settings,
+        )):
+            self.assertEquals(tab.type, self.course.tabs[i].type)
+
+        # enumerate the tabs and verify textbooks and the instructor tab
         for i, tab in enumerate(tabs.CourseTabList.iterate_displayable(
             self.course,
             self.settings,
-            include_instructor_tab=True,
         )):
-            if i == len(self.course.tabs):
+            if getattr(tab, 'is_collection_item', False):
+                # a collection item was found as a result of a collection tab
+                self.assertTrue(getattr(self.course.tabs[i], 'is_collection', False))
+            elif i == len(self.course.tabs):
+                # the last tab must be the Instructor tab
                 self.assertEquals(tab.type, tabs.InstructorTab.type)
             else:
+                # all other tabs must match the expected type
                 self.assertEquals(tab.type, self.course.tabs[i].type)
 
+    def test_get_tab_by_methods(self):
+        """tests the get_tab methods in CourseTabList"""
+        self.course.tabs = self.all_valid_tab_list
+        for tab in self.course.tabs:
+
+            # get tab by type
+            self.assertEquals(tabs.CourseTabList.get_tab_by_type(self.course.tabs, tab.type), tab)
+
+            # get tab by id
+            self.assertEquals(tabs.CourseTabList.get_tab_by_id(self.course.tabs, tab.tab_id), tab)
+
 
 class DiscussionLinkTestCase(TabTestCase):
     """Test cases for discussion link tab."""

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/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)