diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 1c357a0588..e37fde4276 100644 --- a/CHANGELOG.rst +++ b/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 diff --git a/cms/djangoapps/contentstore/features/pages.feature b/cms/djangoapps/contentstore/features/pages.feature index e0c4fa0899..890c62301f 100644 --- a/cms/djangoapps/contentstore/features/pages.feature +++ b/cms/djangoapps/contentstore/features/pages.feature @@ -27,26 +27,26 @@ Feature: CMS.Pages @skip_safari Scenario: Users can reorder static pages Given I have created two different static pages - When I reorder the static pages - Then the static pages 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 pages 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 reorder the pages - Then the built-in pages are in the reverse 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 in the reverse order + 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 reorder the pages - Then the pages are in the reverse order + When I drag the first page to the last + Then the pages are switched And I reload the page - Then the pages are in the reverse order + Then the pages are switched Scenario: Users can toggle visibility on hideable pages Given I have opened the pages page in a new course diff --git a/cms/djangoapps/contentstore/features/pages.py b/cms/djangoapps/contentstore/features/pages.py index f91eb3bd6a..e86177282d 100644 --- a/cms/djangoapps/contentstore/features/pages.py +++ b/cms/djangoapps/contentstore/features/pages.py @@ -6,6 +6,9 @@ from lettuce import world, step 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$') def go_to_static(step): menu_css = 'li.nav-course-courseware' @@ -51,24 +54,24 @@ def change_name(step, new_name): world.css_click(save_button) -@step(u'I reorder the static pages') -def reorder_static_pages(_step): - reorder_pages_with_css_class('.component') +@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') @@ -78,8 +81,8 @@ def create_two_pages(step): _verify_page_names('First', 'Empty') -@step(u'the static pages are in the reverse order') -def static_pages_in_reverse_order(step): +@step(u'the static pages are switched$') +def static_pages_are_switched(step): _verify_page_names('Empty', 'First') @@ -90,51 +93,51 @@ def _verify_page_names(first, second): timeout_msg="Timed out waiting for two pages to be present" ) pages = world.css_find('.xmodule_StaticTabModule') - assert pages[0].text == first - assert pages[1].text == second + assert_equal(pages[0].text, first) + assert_equal(pages[1].text, second) -@step(u'the built-in pages are in the default order') +@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): +@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') +@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): +@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 reorder the pages') -def reorder_pages(step): - reorder_pages_with_css_class('.sortable-tab') +@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 world.css_find("li[data-tab-id='{0}'] input.toggle-checkbox".format(page_id)).checked == hidden + assert_equal(world.css_find(CSS_FOR_TAB_ELEMENT.format(page_id)).checked, hidden) -@step(u'I toggle the visibility of the "([^"]*)" page') +@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() + world.css_find(CSS_FOR_TAB_ELEMENT.format(page_id))[0].click() -def reorder_pages_with_css_class(css_class): +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 @@ -149,4 +152,3 @@ def see_pages_in_expected_order(page_names_in_expected_order): 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) - diff --git a/cms/djangoapps/contentstore/management/commands/tests/test_git_export.py b/cms/djangoapps/contentstore/management/commands/tests/test_git_export.py index 1fa81e9a64..9b7f4a7665 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_git_export.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_git_export.py @@ -150,7 +150,7 @@ class TestGitExport(CourseTestCase): '--format=%an|%ae'], cwd=cwd) self.assertEqual(expect_string, git_log) - # Make changes to course so there is something commit + # Make changes to course so there is something to commit self.populate_course() git_export_utils.export_to_git( self.course.id, diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index ca23c4e51d..0b028cdca0 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -409,9 +409,19 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): else: built_in_tabs.append(tab) - tab_ids = [{'tab_id': tab.tab_id} for tab in (built_in_tabs + reverse_static_tabs)] + # 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_ids}) + self.client.ajax_post(new_location.url_reverse('tabs'), {'tabs': tab_id_locators}) course = module_store.get_item(course_location) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index ce577cb903..02afd2ad13 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/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() diff --git a/cms/djangoapps/contentstore/views/tabs.py b/cms/djangoapps/contentstore/views/tabs.py index 695d390138..988fe64894 100644 --- a/cms/djangoapps/contentstore/views/tabs.py +++ b/cms/djangoapps/contentstore/views/tabs.py @@ -16,7 +16,7 @@ from xmodule.modulestore.django import loc_mapper from xmodule.modulestore.locator import BlockUsageLocator from xmodule.tabs import CourseTabList, StaticTab, CourseTab, InvalidTabsException -from ..utils import get_modulestore +from ..utils import get_modulestore, get_lms_link_for_item __all__ = ['tabs_handler'] @@ -69,16 +69,16 @@ def tabs_handler(request, tag=None, package_id=None, branch=None, version_guid=N 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_tab = modulestore('direct').get_item(static_tab_loc) tab.locator = loc_mapper().translate_location( - course_item.location.course_id, static_tab.location, False, True + 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 + 'course_locator': locator, + 'lms_link': get_lms_link_for_item(course_item.location), }) else: return HttpResponseNotFound() @@ -93,14 +93,14 @@ def reorder_tabs_handler(course_item, request): # 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'] + 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 ids_locators_of_new_tab_order: + 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( diff --git a/cms/djangoapps/contentstore/views/tests/test_tabs.py b/cms/djangoapps/contentstore/views/tests/test_tabs.py index 9e92c4f27d..fd04089bc6 100644 --- a/cms/djangoapps/contentstore/views/tests/test_tabs.py +++ b/cms/djangoapps/contentstore/views/tests/test_tabs.py @@ -48,14 +48,17 @@ class TabsPageTests(CourseTestCase): with self.assertRaises(NotImplementedError): self.client.ajax_post( self.url, - data={'tab_id': WikiTab.type, 'unsupported_request': None} + 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} + data={'invalid_request': None}, ) def test_view_index(self): @@ -63,7 +66,7 @@ class TabsPageTests(CourseTestCase): resp = self.client.get_html(self.url) self.assertEqual(resp.status_code, 200) - self.assertIn('course-nav-tab-list', resp.content) + self.assertIn('course-nav-list', resp.content) def test_reorder_tabs(self): """Test re-ordering of tabs""" @@ -87,7 +90,7 @@ class TabsPageTests(CourseTestCase): # post the request resp = self.client.ajax_post( self.url, - data={'tabs': [{'tab_id': tab_id} for tab_id in tab_ids]} + data={'tabs': [{'tab_id': tab_id} for tab_id in tab_ids]}, ) self.assertEqual(resp.status_code, 204) @@ -109,7 +112,7 @@ class TabsPageTests(CourseTestCase): # post the request resp = self.client.ajax_post( self.url, - data={'tabs': [{'tab_id': tab_id} for tab_id in tab_ids]} + data={'tabs': [{'tab_id': tab_id} for tab_id in tab_ids]}, ) self.assertEqual(resp.status_code, 400) resp_content = json.loads(resp.content) @@ -123,7 +126,7 @@ class TabsPageTests(CourseTestCase): # post the request resp = self.client.ajax_post( self.url, - data={'tabs': [{'tab_id': tab_id} for tab_id in invalid_tab_ids]} + data={'tabs': [{'tab_id': tab_id} for tab_id in invalid_tab_ids]}, ) self.check_invalid_tab_id_response(resp) @@ -141,7 +144,7 @@ class TabsPageTests(CourseTestCase): self.url, data=json.dumps({ 'tab_id_locator': {'tab_id': old_tab.tab_id}, - 'is_hidden': new_is_hidden_setting + 'is_hidden': new_is_hidden_setting, }), ) self.assertEqual(resp.status_code, 204) diff --git a/cms/envs/common.py b/cms/envs/common.py index 6e2618a65e..56d3fcc62e 100644 --- a/cms/envs/common.py +++ b/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, diff --git a/cms/static/coffee/src/views/tabs.coffee b/cms/static/coffee/src/views/tabs.coffee index 0262886685..83f520f7ae 100644 --- a/cms/static/coffee/src/views/tabs.coffee +++ b/cms/static/coffee/src/views/tabs.coffee @@ -19,7 +19,7 @@ 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) $('.toggle-checkbox').on('click', @toggleVisibilityOfTab) - @$('.course-nav-tab-list').sortable( + @$('.course-nav-list').sortable( handle: '.drag-handle' update: @tabMoved helper: 'clone' @@ -27,7 +27,7 @@ define ["jquery", "jquery.ui", "backbone", "js/views/feedback_prompt", "js/views placeholder: 'component-placeholder' forcePlaceholderSize: true axis: 'y' - items: '> .sortable-tab' + items: '> .is-movable' ) toggleVisibilityOfTab: (event, ui) => @@ -85,7 +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('course-tab is-movable') editor.$el.addClass('new') setTimeout(=> editor.$el.removeClass('new') diff --git a/cms/templates/edit-tabs.html b/cms/templates/edit-tabs.html index 0339e76833..2e3ce617af 100644 --- a/cms/templates/edit-tabs.html +++ b/cms/templates/edit-tabs.html @@ -40,7 +40,10 @@

${_("Page Actions")}

@@ -58,39 +61,34 @@
    % 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): -
  1. +
  2. + % else: - - <% - tab_name = _(tab.name) - item_names_formatted = "" - item_names = [] - num_items = 0 - 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)) - css_class = "course-nav-item course-nav-tab course-tab" - if tab.is_movable: - css_class = css_class + " sortable-tab" - %> - - % if tab.is_hideable or tab.is_movable: -
  3. +
  4. - % if tab.is_collection: -

    ${tab_name}

    -
      - % for item_name in item_names: -
    • - ${item_name} -
    • - % endfor -
    + % if tab.is_collection: + +

    ${_(tab.name)}

    +
      + % for item in tab.items(context_course): +
    • + ${_(item.name)} +
    • + % endfor +
    % else: -

    ${tab_name}

    +

    ${_(tab.name)}

    % endif
    @@ -99,13 +97,13 @@ % if tab.is_hideable:
  5. - - % if tab.is_hidden: - - % else: - - % endif -
    + + % if tab.is_hidden: + + % else: + + % endif +
  6. % endif @@ -123,30 +121,7 @@ % endif - % else: -
  7. -
    -

    ${tab_name}

    - - % if tab.is_collection: -
      - % for item_name in item_names: -
    • - ${item_name} -
    • - % endfor -
    - % endif - -
    - -
    - ${_("This page cannot be reordered")} -
    -
  8. - % endif - - % endif + % endif % endfor
  9. diff --git a/common/lib/xmodule/xmodule/tabs.py b/common/lib/xmodule/xmodule/tabs.py index c215d56f34..cd1f5101db 100644 --- a/common/lib/xmodule/xmodule/tabs.py +++ b/common/lib/xmodule/xmodule/tabs.py @@ -37,7 +37,7 @@ class CourseTab(object): # pylint: disable=incomplete-protocol # Class property that specifies whether the tab is a collection of other tabs is_collection = False - def __init__(self, name, tab_id, link_func, tab): + def __init__(self, name, tab_id, link_func): """ Initializes class members with values passed in by subclasses. @@ -57,8 +57,6 @@ class CourseTab(object): # pylint: disable=incomplete-protocol self.link_func = link_func - self.is_hidden = tab.get('is_hidden', False) if tab else False - def can_display(self, course, settings, is_user_authenticated, is_user_staff): # pylint: disable=unused-argument """ Determines whether the tab should be displayed in the UI for the given course and a particular user. @@ -105,8 +103,6 @@ class CourseTab(object): # pylint: disable=incomplete-protocol return self.name elif key == 'type': return self.type - elif key == 'is_hidden': - return self.is_hidden elif key == 'tab_id': return self.tab_id else: @@ -123,8 +119,6 @@ class CourseTab(object): # pylint: disable=incomplete-protocol self.name = value elif key == 'tab_id': self.tab_id = value - elif key == 'is_hidden': - self.is_hidden = value else: raise KeyError('Key {0} cannot be set in tab {1}'.format(key, self.to_json())) @@ -142,8 +136,8 @@ class CourseTab(object): # pylint: disable=incomplete-protocol # allow tabs without names; if a name is required, its presence was checked in the validator. name_is_eq = (other.get('name') is None or self.name == other['name']) - # only compare the persisted/serialized members: 'type', 'name', and 'is_hidden' - return self.type == other.get('type') and name_is_eq and self.is_hidden == other.get('is_hidden', False) + # only compare the persisted/serialized members: 'type' and 'name' + return self.type == other.get('type') and name_is_eq def __ne__(self, other): """ @@ -152,12 +146,12 @@ class CourseTab(object): # pylint: disable=incomplete-protocol return not (self == other) @classmethod - def validate(cls, tab, raise_error=True): + def validate(cls, tab_dict, raise_error=True): """ Validates the given dict-type tab object to ensure it contains the expected keys. This method should be overridden by subclasses that require certain keys to be persisted in the tab. """ - return key_checker(['type'])(tab, raise_error) + return key_checker(['type'])(tab_dict, raise_error) def to_json(self): """ @@ -167,13 +161,10 @@ class CourseTab(object): # pylint: disable=incomplete-protocol Returns: a dictionary with keys for the properties of the CourseTab object. """ - to_json_val = {'type': self.type, 'name': self.name} - if self.is_hidden: - to_json_val.update({'is_hidden': True}) - return to_json_val + return {'type': self.type, 'name': self.name} @staticmethod - def from_json(tab): + def from_json(tab_dict): """ Deserializes a CourseTab from a json-like representation. @@ -206,15 +197,15 @@ class CourseTab(object): # pylint: disable=incomplete-protocol 'instructor': InstructorTab, # not persisted } - tab_type = tab.get('type') + tab_type = tab_dict.get('type') if tab_type not in sub_class_types: raise InvalidTabsException( 'Unknown tab type {0}. Known types: {1}'.format(tab_type, sub_class_types) ) - tab_class = sub_class_types[tab['type']] - tab_class.validate(tab) - return tab_class(tab=tab) + tab_class = sub_class_types[tab_dict['type']] + tab_class.validate(tab_dict) + return tab_class(tab_dict=tab_dict) class AuthenticatedCourseTab(CourseTab): @@ -233,6 +224,44 @@ class StaffTab(AuthenticatedCourseTab): return is_user_staff +class HideableTab(CourseTab): + """ + Abstract class for tabs that are hideable + """ + is_hideable = True + + def __init__(self, name, tab_id, link_func, tab_dict): + super(HideableTab, self).__init__( + name=name, + tab_id=tab_id, + link_func=link_func, + ) + self.is_hidden = tab_dict.get('is_hidden', False) if tab_dict else False + + def __getitem__(self, key): + if key == 'is_hidden': + return self.is_hidden + else: + return super(HideableTab, self).__getitem__(key) + + def __setitem__(self, key, value): + if key == 'is_hidden': + self.is_hidden = value + else: + super(HideableTab, self).__setitem__(key, value) + + def to_json(self): + to_json_val = super(HideableTab, self).to_json() + if self.is_hidden: + to_json_val.update({'is_hidden': True}) + return to_json_val + + def __eq__(self, other): + if not super(HideableTab, self).__eq__(other): + return False + return self.is_hidden == other.get('is_hidden', False) + + class CoursewareTab(CourseTab): """ A tab containing the course content. @@ -241,13 +270,12 @@ class CoursewareTab(CourseTab): type = 'courseware' is_movable = False - def __init__(self, tab=None): + def __init__(self, tab_dict=None): # pylint: disable=unused-argument super(CoursewareTab, self).__init__( # Translators: 'Courseware' refers to the tab in the courseware that leads to the content of a course name=_('Courseware'), # support fixed name for the courseware tab tab_id=self.type, link_func=link_reverse_func(self.type), - tab=tab, ) @@ -259,18 +287,17 @@ class CourseInfoTab(CourseTab): type = 'course_info' is_movable = False - def __init__(self, tab=None): + def __init__(self, tab_dict=None): super(CourseInfoTab, self).__init__( # Translators: "Course Info" is the name of the course's information and updates page - name=tab['name'] if tab else _('Course Info'), + name=tab_dict['name'] if tab_dict else _('Course Info'), tab_id='info', link_func=link_reverse_func('info'), - tab=tab, ) @classmethod - def validate(cls, tab, raise_error=True): - return super(CourseInfoTab, cls).validate(tab, raise_error) and need_name(tab, raise_error) + def validate(cls, tab_dict, raise_error=True): + return super(CourseInfoTab, cls).validate(tab_dict, raise_error) and need_name(tab_dict, raise_error) class ProgressTab(AuthenticatedCourseTab): @@ -280,46 +307,44 @@ class ProgressTab(AuthenticatedCourseTab): type = 'progress' - def __init__(self, tab=None): + def __init__(self, tab_dict=None): super(ProgressTab, self).__init__( # Translators: "Progress" is the name of the student's course progress page - name=tab['name'] if tab else _('Progress'), + name=tab_dict['name'] if tab_dict else _('Progress'), tab_id=self.type, link_func=link_reverse_func(self.type), - tab=tab, ) def can_display(self, course, settings, is_user_authenticated, is_user_staff): return not course.hide_progress_tab @classmethod - def validate(cls, tab, raise_error=True): - return super(ProgressTab, cls).validate(tab, raise_error) and need_name(tab, raise_error) + def validate(cls, tab_dict, raise_error=True): + return super(ProgressTab, cls).validate(tab_dict, raise_error) and need_name(tab_dict, raise_error) -class WikiTab(CourseTab): +class WikiTab(HideableTab): """ - A tab containing the course wiki. + A tab_dict containing the course wiki. """ type = 'wiki' - is_hideable = True - def __init__(self, tab=None): + def __init__(self, tab_dict=None): super(WikiTab, self).__init__( # Translators: "Wiki" is the name of the course's wiki page - name=tab['name'] if tab else _('Wiki'), + name=tab_dict['name'] if tab_dict else _('Wiki'), tab_id=self.type, link_func=link_reverse_func('course_wiki'), - tab=tab, + tab_dict=tab_dict, ) def can_display(self, course, settings, is_user_authenticated, is_user_staff): return settings.WIKI_ENABLED @classmethod - def validate(cls, tab, raise_error=True): - return super(WikiTab, cls).validate(tab, raise_error) and need_name(tab, raise_error) + def validate(cls, tab_dict, raise_error=True): + return super(WikiTab, cls).validate(tab_dict, raise_error) and need_name(tab_dict, raise_error) class DiscussionTab(CourseTab): @@ -329,21 +354,20 @@ class DiscussionTab(CourseTab): type = 'discussion' - def __init__(self, tab=None): + def __init__(self, tab_dict=None): super(DiscussionTab, self).__init__( # Translators: "Discussion" is the title of the course forum page - name=tab['name'] if tab else _('Discussion'), + name=tab_dict['name'] if tab_dict else _('Discussion'), tab_id=self.type, link_func=link_reverse_func('django_comment_client.forum.views.forum_form_discussion'), - tab=tab, ) def can_display(self, course, settings, is_user_authenticated, is_user_staff): return settings.FEATURES.get('ENABLE_DISCUSSION_SERVICE') @classmethod - def validate(cls, tab, raise_error=True): - return super(DiscussionTab, cls).validate(tab, raise_error) and need_name(tab, raise_error) + def validate(cls, tab_dict, raise_error=True): + return super(DiscussionTab, cls).validate(tab_dict, raise_error) and need_name(tab_dict, raise_error) class LinkTab(CourseTab): @@ -352,13 +376,12 @@ class LinkTab(CourseTab): """ link_value = '' - def __init__(self, name, tab_id, link_value, tab): + def __init__(self, name, tab_id, link_value): self.link_value = link_value super(LinkTab, self).__init__( name=name, tab_id=tab_id, link_func=link_value_func(self.link_value), - tab=tab, ) def __getitem__(self, key): @@ -384,8 +407,8 @@ class LinkTab(CourseTab): return self.link_value == other.get('link') @classmethod - def validate(cls, tab, raise_error=True): - return super(LinkTab, cls).validate(tab, raise_error) and key_checker(['link'])(tab, raise_error) + def validate(cls, tab_dict, raise_error=True): + return super(LinkTab, cls).validate(tab_dict, raise_error) and key_checker(['link'])(tab_dict, raise_error) class ExternalDiscussionTab(LinkTab): @@ -395,13 +418,12 @@ class ExternalDiscussionTab(LinkTab): type = 'external_discussion' - def __init__(self, tab=None, link_value=None): + def __init__(self, tab_dict=None, link_value=None): super(ExternalDiscussionTab, self).__init__( # Translators: 'Discussion' refers to the tab in the courseware that leads to the discussion forums name=_('Discussion'), tab_id='discussion', - link_value=tab['link'] if tab else link_value, - tab=tab, + link_value=tab_dict['link'] if tab_dict else link_value, ) @@ -411,12 +433,11 @@ class ExternalLinkTab(LinkTab): """ type = 'external_link' - def __init__(self, tab): + def __init__(self, tab_dict): super(ExternalLinkTab, self).__init__( - name=tab['name'], + name=tab_dict['name'], tab_id=None, # External links are never active. - link_value=tab['link'], - tab=tab, + link_value=tab_dict['link'], ) @@ -427,16 +448,15 @@ class StaticTab(CourseTab): type = 'static_tab' @classmethod - def validate(cls, tab, raise_error=True): - return super(StaticTab, cls).validate(tab, raise_error) and key_checker(['name', 'url_slug'])(tab, raise_error) + def validate(cls, tab_dict, raise_error=True): + return super(StaticTab, cls).validate(tab_dict, raise_error) and key_checker(['name', 'url_slug'])(tab_dict, raise_error) - def __init__(self, tab=None, name=None, url_slug=None): - self.url_slug = tab['url_slug'] if tab else url_slug + def __init__(self, tab_dict=None, name=None, url_slug=None): + self.url_slug = tab_dict['url_slug'] if tab_dict else url_slug super(StaticTab, self).__init__( - name=tab['name'] if tab else name, + name=tab_dict['name'] if tab_dict else name, tab_id='static_tab_{0}'.format(self.url_slug), link_func=lambda course, reverse_func: reverse_func(self.type, args=[course.id, self.url_slug]), - tab=tab, ) def __getitem__(self, key): @@ -481,12 +501,12 @@ class TextbookTabsBase(AuthenticatedCourseTab): """ is_collection = True - def __init__(self, tab_id, tab): + def __init__(self, tab_id): # Translators: 'Textbooks' refers to the tab in the course that leads to the course' textbooks super(TextbookTabsBase, self).__init__( name=_("Textbooks"), tab_id=tab_id, - link_func=None, tab=tab + link_func=None, ) @abstractmethod @@ -504,10 +524,9 @@ class TextbookTabs(TextbookTabsBase): """ type = 'textbooks' - def __init__(self, tab=None): + def __init__(self, tab_dict=None): # pylint: disable=unused-argument super(TextbookTabs, self).__init__( tab_id=self.type, - tab=tab, ) def can_display(self, course, settings, is_user_authenticated, is_user_staff): @@ -519,7 +538,6 @@ class TextbookTabs(TextbookTabsBase): name=textbook.title, tab_id='textbook/{0}'.format(index), link_func=lambda course, reverse_func: reverse_func('book', args=[course.id, index]), - tab=None ) @@ -529,10 +547,9 @@ class PDFTextbookTabs(TextbookTabsBase): """ type = 'pdf_textbooks' - def __init__(self, tab=None): + def __init__(self, tab_dict=None): # pylint: disable=unused-argument super(PDFTextbookTabs, self).__init__( tab_id=self.type, - tab=tab, ) def items(self, course): @@ -541,7 +558,6 @@ class PDFTextbookTabs(TextbookTabsBase): name=textbook['tab_title'], tab_id='pdftextbook/{0}'.format(index), link_func=lambda course, reverse_func: reverse_func('pdf_book', args=[course.id, index]), - tab=None ) @@ -551,10 +567,9 @@ class HtmlTextbookTabs(TextbookTabsBase): """ type = 'html_textbooks' - def __init__(self, tab=None): + def __init__(self, tab_dict=None): # pylint: disable=unused-argument super(HtmlTextbookTabs, self).__init__( tab_id=self.type, - tab=tab, ) def items(self, course): @@ -563,7 +578,6 @@ class HtmlTextbookTabs(TextbookTabsBase): name=textbook['tab_title'], tab_id='htmltextbook/{0}'.format(index), link_func=lambda course, reverse_func: reverse_func('html_book', args=[course.id, index]), - tab=None ) @@ -580,14 +594,13 @@ class StaffGradingTab(StaffTab, GradingTab): """ type = 'staff_grading' - def __init__(self, tab=None): + def __init__(self, tab_dict=None): # pylint: disable=unused-argument super(StaffGradingTab, self).__init__( # Translators: "Staff grading" appears on a tab that allows # staff to view open-ended problems that require staff grading name=_("Staff grading"), tab_id=self.type, link_func=link_reverse_func(self.type), - tab=tab, ) @@ -597,14 +610,13 @@ class PeerGradingTab(AuthenticatedCourseTab, GradingTab): """ type = 'peer_grading' - def __init__(self, tab=None): + def __init__(self, tab_dict=None): # pylint: disable=unused-argument super(PeerGradingTab, self).__init__( # Translators: "Peer grading" appears on a tab that allows # students to view open-ended problems that require grading name=_("Peer grading"), tab_id=self.type, link_func=link_reverse_func(self.type), - tab=tab, ) @@ -614,14 +626,13 @@ class OpenEndedGradingTab(AuthenticatedCourseTab, GradingTab): """ type = 'open_ended' - def __init__(self, tab=None): + def __init__(self, tab_dict=None): # pylint: disable=unused-argument super(OpenEndedGradingTab, self).__init__( # Translators: "Open Ended Panel" appears on a tab that, when clicked, opens up a panel that # displays information about open-ended problems that a user has submitted or needs to grade name=_("Open Ended Panel"), tab_id=self.type, link_func=link_reverse_func('open_ended_notifications'), - tab=tab, ) @@ -634,13 +645,12 @@ class SyllabusTab(CourseTab): def can_display(self, course, settings, is_user_authenticated, is_user_staff): return hasattr(course, 'syllabus_present') and course.syllabus_present - def __init__(self, tab=None): + def __init__(self, tab_dict=None): # pylint: disable=unused-argument super(SyllabusTab, self).__init__( # Translators: "Syllabus" appears on a tab that, when clicked, opens the syllabus of the course. name=_('Syllabus'), tab_id=self.type, link_func=link_reverse_func(self.type), - tab=tab, ) @@ -653,17 +663,16 @@ class NotesTab(AuthenticatedCourseTab): def can_display(self, course, settings, is_user_authenticated, is_user_staff): return settings.FEATURES.get('ENABLE_STUDENT_NOTES') - def __init__(self, tab=None): + def __init__(self, tab_dict=None): super(NotesTab, self).__init__( - name=tab['name'], + name=tab_dict['name'], tab_id=self.type, link_func=link_reverse_func(self.type), - tab=tab, ) @classmethod - def validate(cls, tab, raise_error=True): - return super(NotesTab, cls).validate(tab, raise_error) and need_name(tab, raise_error) + def validate(cls, tab_dict, raise_error=True): + return super(NotesTab, cls).validate(tab_dict, raise_error) and need_name(tab_dict, raise_error) class InstructorTab(StaffTab): @@ -672,14 +681,13 @@ class InstructorTab(StaffTab): """ type = 'instructor' - def __init__(self, tab=None): + def __init__(self, tab_dict=None): # pylint: disable=unused-argument super(InstructorTab, self).__init__( # Translators: 'Instructor' appears on the tab that leads to the instructor dashboard, which is # a portal where an instructor can get data and perform various actions on their course name=_('Instructor'), tab_id=self.type, link_func=link_reverse_func('instructor_dashboard'), - tab=tab, ) @@ -771,7 +779,9 @@ class CourseTabList(List): the given user with the provided access settings. """ for tab in course.tabs: - if tab.can_display(course, settings, is_user_authenticated, is_user_staff) and not tab.is_hidden: + if tab.can_display( + course, settings, is_user_authenticated, is_user_staff + ) and (not tab.is_hideable or not tab.is_hidden): if tab.is_collection: for item in tab.items(course): yield item @@ -787,11 +797,14 @@ class CourseTabList(List): settings ): """ - Generator method for iterating through all tabs that can be displayed for the given course and - the given user with the provided access settings. + Generator method for iterating through all tabs that can be displayed for the given course + with the provided settings. """ for tab in course.tabs: if tab.can_display(course, settings, is_user_authenticated=True, is_user_staff=True): + if tab.is_collection and not len(list(tab.items(course))): + # do not yield collections that have no items + continue yield tab @classmethod @@ -862,7 +875,7 @@ class CourseTabList(List): Overrides the from_json method to de-serialize the CourseTab objects from a json-like representation. """ self.validate_tabs(values) - return [CourseTab.from_json(tab) for tab in values] + return [CourseTab.from_json(tab_dict) for tab_dict in values] #### Link Functions diff --git a/common/lib/xmodule/xmodule/tests/test_tabs.py b/common/lib/xmodule/xmodule/tests/test_tabs.py index 86f1c1e026..311cced077 100644 --- a/common/lib/xmodule/xmodule/tests/test_tabs.py +++ b/common/lib/xmodule/xmodule/tests/test_tabs.py @@ -17,7 +17,7 @@ class TabTestCase(unittest.TestCase): 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""" + """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) @@ -76,7 +76,7 @@ class TabTestCase(unittest.TestCase): return tab def check_tab_equality(self, tab, dict_tab): - """tests the equality methods on the given 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' @@ -84,13 +84,13 @@ class TabTestCase(unittest.TestCase): self.assertNotEquals(tab, {'fake_key': 'fake_value'}) # test __ne__: missing type def check_tab_json_methods(self, tab): - """tests the json from and to methods on the given 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): - """checks can display results for various users""" + """Checks can display results for various users""" if for_staff_only: self.assertEquals( expected_value, @@ -108,7 +108,7 @@ class TabTestCase(unittest.TestCase): ) def check_get_and_set_methods(self, tab): - """test __getitem__ and __setitem__ calls""" + """Test __getitem__ and __setitem__ calls""" self.assertEquals(tab['type'], tab.type) self.assertEquals(tab['tab_id'], tab.tab_id) with self.assertRaises(KeyError): @@ -120,7 +120,7 @@ class TabTestCase(unittest.TestCase): tab['invalid_key'] = 'New Value' def check_get_and_set_method_for_key(self, tab, key): - """test __getitem__ and __setitem__ for the given key""" + """Test __getitem__ and __setitem__ for the given key""" old_value = tab[key] new_value = 'New Value' tab[key] = new_value @@ -540,7 +540,7 @@ class CourseTabListTestCase(TabListTestCase): )): self.assertEquals(tab.type, self.course.tabs[i].type) - # enumerate the tabs and verify textbooks and the instructor tab + # enumerate the tabs and verify textbooks and the instructor tab for i, tab in enumerate(tabs.CourseTabList.iterate_displayable( self.course, self.settings, @@ -555,8 +555,21 @@ class CourseTabListTestCase(TabListTestCase): # all other tabs must match the expected type self.assertEquals(tab.type, self.course.tabs[i].type) + # test including non-empty collections + self.assertIn( + tabs.HtmlTextbookTabs(), + list(tabs.CourseTabList.iterate_displayable_cms(self.course, self.settings)), + ) + + # test not including empty collections + self.course.html_textbooks = [] + self.assertNotIn( + tabs.HtmlTextbookTabs(), + list(tabs.CourseTabList.iterate_displayable_cms(self.course, self.settings)), + ) + def test_get_tab_by_methods(self): - """tests the get_tab methods in CourseTabList""" + """Tests the get_tab methods in CourseTabList""" self.course.tabs = self.all_valid_tab_list for tab in self.course.tabs: @@ -587,7 +600,7 @@ class DiscussionLinkTestCase(TabTestCase): @staticmethod def _reverse(course): - """custom reverse function""" + """Custom reverse function""" def reverse_discussion_link(viewname, args): """reverse lookup for discussion link""" if viewname == "django_comment_client.forum.views.forum_form_discussion" and args == [course.id]: diff --git a/lms/envs/common.py b/lms/envs/common.py index 4cef6a88d6..e61ee8740d 100644 --- a/lms/envs/common.py +++ b/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,