Custom tabs

* specify in tabs list in course policy
  - active page tracking now done in tabs.py
  - properly handle the fact that there may be multiple textbooks

* Still need:
  - wiki pages
  - (if that's delayed, special-case syllabus support)

common/lib/xmodule/xmodule/course_module.py

 from fs.errors import ResourceNotFoundError
-import time
 import logging
-import requests
 from lxml import etree
 from path import path # NOTE (THK): Only used for detecting presence of syllabus
+import requests
+import time
 
 from xmodule.util.decorators import lazyproperty
 from xmodule.graders import load_grading_policy
@@ -135,6 +135,13 @@ class CourseDescriptor(SequenceDescriptor):
         return self._grading_policy['GRADE_CUTOFFS']
 
     @property
+    def tabs(self):
+        """
+        Return the tabs config, as a python object, or None if not specified.
+        """
+        return self.metadata.get('tabs')
+
+    @property
     def show_calculator(self):
         return self.metadata.get("show_calculator", None) == "Yes"
 

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

@@ -414,7 +414,6 @@ class XMLModuleStore(ModuleStoreBase):
             policy_str = self.read_grading_policy(paths, tracker)
             course_descriptor.set_grading_policy(policy_str)
 
-
             log.debug('========> Done with course import from {0}'.format(course_dir))
             return course_descriptor
 

doc/xml-format.md

@@ -219,6 +219,13 @@ Values are dictionaries of the form {"metadata-key" : "metadata-value"}.
 * The order in which things appear does not matter, though it may be helpful to organize the file in the same order as things appear in the content.
 * NOTE: json is picky about commas.  If you have trailing commas before closing braces, it will complain and refuse to parse the file.  This can be irritating at first.
 
+Supported fields at the course level:
+
+* "start" -- specify the start date for the course.  Format-by-example: "2012-09-05T12:00".
+* "enrollment_start", "enrollment_end" -- when can students enroll?  (if not specified, can enroll anytime).   Same format as "start".
+* "tabs" -- have custom tabs in the courseware.  See below for details on config.
+* TODO: there are others
+
 ### Grading policy file contents
 
 TODO: This needs to be improved, but for now here's a sketch of how grading works:
@@ -340,7 +347,45 @@ If you look at some older xml, you may see some tags or metadata attributes that
 
 # Static links
 
-if your content links (e.g. in an html file)  to `"static/blah/ponies.jpg"`, we will look for this in `YOUR_COURSE_DIR/blah/ponies.jpg`.  Note that this is not looking in a `static/` subfolder in your course dir.  This may (should?) change at some point.   Links that include `/course` will be rewritten to the root of your course in the courseware (e.g. `courses/{org}/{course}/{url_name}/` in the current url structure).  This is useful for linking to the course wiki, for example.
+If your content links (e.g. in an html file)  to `"static/blah/ponies.jpg"`, we will look for this...
+
+* If your course dir has a `static/` subdirectory, we will look in `YOUR_COURSE_DIR/static/blah/ponies.jpg`.   This is the prefered organization, as it does not expose anything except what's in `static/` to the world.
+*  If your course dir does not have a `static/` subdirectory, we will look in `YOUR_COURSE_DIR/blah/ponies.jpg`.  This is the old organization, and requires that the web server allow access to everything in the couse dir.  To switch to the new organization, move all your static content into a new `static/` dir  (e.g. if you currently have things in `images/`, `css/`, and `special/`, create a dir called `static/`, and move `images/, css/, and special/` there).
+
+Links that include `/course` will be rewritten to the root of your course in the courseware (e.g. `courses/{org}/{course}/{url_name}/` in the current url structure).  This is useful for linking to the course wiki, for example.
+
+# Tabs
+
+If you want to customize the courseware tabs displayed for your course, specify a "tabs" list in the course-level policy.  e.g.:
+
+"tabs" : [
+{ "type": "courseware"},     # no name--always "Courseware" for consistency between courses
+{"name": "Course Info",
+ "type": course_info"},
+{"name": "My Discussion",
+ "type": external_"link",
+ "link": "http://www.mydiscussion.org/blah"},
+{"name": "Progress",
+"type": "Progress"},
+{"name": "Wonderwiki",
+ "type": "wiki"},
+{"type": "textbooks"}   # generates one tab per textbook, taking names from the textbook titles
+]
+
+
+* If you specify any tabs, you must specify all tabs.  They will appear in the order given.
+* The first two tabs must have types "courseware" and "course_info", in that order.  Otherwise, we'll refuse to load the course.
+* An Instructor tab will be automatically added at the end for course staff users.
+
+## Supported tab types:
+
+* "courseware".  No other parameters.
+* "course_info".  Parameter "name".
+* "wiki". Parameter "name".
+* "discussion".  Parameter "name".
+* "external_link".  Parameters "name", "link".
+* "textbooks".  No parameters--generates tab names from book titles.
+* "progress".  Parameter "name".
 
 # Tips for content developers
 

lms/djangoapps/courseware/tabs.py

+"""
+Tabs configuration.  By the time the tab is being rendered, it's just a name,
+link, and css class (CourseTab tuple).  Tabs are specified in course policy.
+Each tab has a type, and possibly some type-specific parameters.
+
+To add a new tab type, add a TabImpl to the VALID_TAB_TYPES dict below--it will
+contain a validation function that checks whether config for the tab type is
+valid, and a generator function that takes the config, user, and course, and
+actually generates the CourseTab.
+"""
+
+from collections import namedtuple
+import logging
+
+from django.conf import settings
+from django.core.urlresolvers import reverse
+
+from courseware.access import has_access
+
+log = logging.getLogger(__name__)
+
+class InvalidTabsException(Exception):
+    """
+    A complaint about invalid tabs.
+    """
+    pass
+
+CourseTab = namedtuple('CourseTab', 'name link is_active')
+
+# encapsulate implementation for a tab:
+#  - a validation function: takes the config dict and raises
+#    InvalidTabsException if required fields are missing or otherwise
+#    wrong.  (e.g. "is there a 'name' field?).  Validators can assume
+#    that the type field is valid.
+#
+#  - a function that takes a config, a user, and a course, and active_page and
+#    return a list of CourseTabs.  (e.g. "return a CourseTab with specified
+#    name, link to courseware, and is_active=True/False").  The function can
+#    assume that it is only called with configs of the appropriate type that
+#    have passed the corresponding validator.
+TabImpl = namedtuple('TabImpl', 'validator generator')
+
+
+#####  Generators for various tabs.
+
+def _tab(name, view_name, is_active, extra_args=[]):
+    """Return a CourseTab when link is reverse of css class with course_id"""
+    return CourseTab(name, reverse(class_name, args=[course.id]), class_name)
+
+def _courseware(tab, user, course, active_page):
+    link = reverse('courseware', args=[course.id])
+    return [CourseTab('Courseware', link, active_page == "courseware")]
+
+def _course_info(tab, user, course, active_page):
+    link = reverse('info', args=[course.id])
+    return [CourseTab(tab['name'], link, active_page == "info")]
+
+def _progress(tab, user, course, active_page):
+    if user.is_authenticated():
+        link = reverse('progress', args=[course.id])
+        return [CourseTab(tab['name'], link, active_page == "progress")]
+    return []
+
+def _wiki(tab, user, course, active_page):
+    if settings.WIKI_ENABLED:
+        link = reverse('course_wiki', args=[course.id])
+        return [CourseTab(tab['name'], link, active_page == 'wiki')]
+    return []
+
+def _discussion(tab, user, course, active_page):
+    """
+    This tab format only supports the new Berkeley discussion forums.
+    """
+    if settings.MITX_FEATURES.get('ENABLE_DISCUSSION_SERVICE'):
+        link = reverse('django_comment_client.forum.views.forum_form_discussion',
+                              args=[course.id])
+        return [CourseTab(tab['name'], link, active_page=='discussion')]
+    return []
+
+def _external_link(tab, user, course, active_page):
+    # external links are never active
+    return [CourseTab(tab['name'], tab['link'], False)]
+
+
+def _textbooks(tab, user, course, active_page):
+    """
+    Generates one tab per textbook.  Only displays if user is authenticated.
+    """
+    if user.is_authenticated() and settings.MITX_FEATURES.get('ENABLE_TEXTBOOK'):
+        # since there can be more than one textbook, active_page is e.g. "book/0".
+        return [CourseTab(textbook.title, reverse('book', args=[course.id, index]),
+                          active_page=="textbook/{0}".format(index))
+                for index, textbook in enumerate(course.textbooks)]
+    return []
+
+#### Validators
+
+
+def key_checker(expected_keys):
+    """
+    Returns a function that checks that specified keys are present in a dict
+    """
+    def check(d):
+        for k in expected_keys:
+            if k not in d:
+                raise InvalidTabsException("Key {0} not present in {1}"
+                                           .format(k, d))
+    return check
+
+
+need_name = key_checker(['name'])
+
+def null_validator(d):
+    """
+    Don't check anything--use for tabs that don't need any params. (e.g. textbook)
+    """
+    pass
+
+##### The main tab config dict.
+
+# type -> TabImpl
+VALID_TAB_TYPES = {
+    'courseware': TabImpl(null_validator, _courseware),
+    'course_info': TabImpl(need_name, _course_info),
+    'wiki': TabImpl(need_name, _wiki),
+    'discussion': TabImpl(need_name, _discussion),
+    'external_link': TabImpl(key_checker(['name', 'link']), _external_link),
+    'textbooks': TabImpl(null_validator, _textbooks),
+    'progress': TabImpl(need_name, _progress),
+    }
+
+
+### External interface below this.
+
+def validate_tabs(course):
+    """
+    Check that the tabs set for the specified course is valid.  If it
+    isn't, raise InvalidTabsException with the complaint.
+
+    Specific rules checked:
+    - if no tabs specified, that's fine
+    - if tabs specified, first two must have type 'courseware' and 'course_info', in that order.
+    - All the tabs must have a type in VALID_TAB_TYPES.
+
+    """
+    tabs = course.tabs
+    if tabs is None:
+        return
+
+    if len(tabs) < 2:
+        raise InvalidTabsException("Expected at least two tabs.  tabs: '{0}'".format(tabs))
+    if tabs[0]['type'] != 'courseware':
+        raise InvalidTabsException(
+            "Expected first tab to have type 'courseware'.  tabs: '{0}'".format(tabs))
+    if tabs[1]['type'] != 'course_info':
+        raise InvalidTabsException(
+            "Expected second tab to have type 'course_info'.  tabs: '{0}'".format(tabs))
+    for t in tabs:
+        if t['type'] not in VALID_TAB_TYPES:
+            raise InvalidTabsException("Unknown tab type {0}. Known types: {1}"
+                                       .format(t['type'], VALID_TAB_TYPES))
+        # the type-specific validator checks the rest of the tab config
+        VALID_TAB_TYPES[t['type']].validator(t)
+
+    # Possible other checks: make sure tabs that should only appear once (e.g. courseware)
+    # are actually unique (otherwise, will break active tag code)
+
+
+def get_course_tabs(user, course, active_page):
+    """
+    Return the tabs to show a particular user, as a list of CourseTab items.
+    """
+    if not course.tabs:
+        return get_default_tabs(user, course, active_page)
+
+    # TODO (vshnayder): There needs to be a place to call this right after course
+    # load, but not from inside xmodule, since that doesn't (and probably
+    # shouldn't) know about the details of what tabs are supported, etc.
+    validate_tabs(course)
+
+    tabs = []
+    for tab in course.tabs:
+        # expect handlers to return lists--handles things that are turned off
+        # via feature flags, and things like 'textbook' which might generate
+        # multiple tabs.
+        gen = VALID_TAB_TYPES[tab['type']].generator
+        tabs.extend(gen(tab, user, course, active_page))
+
+    # Instructor tab is special--automatically added if user is staff for the course
+    if has_access(user, course, 'staff'):
+        tabs.append(CourseTab('Instructor',
+                              reverse('instructor_dashboard', args=[course.id]),
+                              active_page == 'instructor'))
+    return tabs
+
+
+def get_default_tabs(user, course, active_page):
+
+    # When calling the various _tab methods, can omit the 'type':'blah' from the
+    # first arg, since that's only used for dispatch
+    tabs = []
+    tabs.extend(_courseware({''}, user, course, active_page))
+    tabs.extend(_course_info({'name': 'Course Info'}, user, course, active_page))
+
+    if hasattr(course, 'syllabus_present') and course.syllabus_present:
+        link = reverse('syllabus', args=[course.id])
+        tabs.append(CourseTab('Syllabus', link, active_page=='syllabus'))
+
+    tabs.extend(_textbooks({}, user, course, active_page))
+
+    ## If they have a discussion link specified, use that even if we feature
+    ## flag discussions off. Disabling that is mostly a server safety feature
+    ## at this point, and we don't need to worry about external sites.
+    if course.discussion_link:
+        tabs.append(CourseTab('Discussion', course.discussion_link, active_page == 'discussion'))
+    elif settings.MITX_FEATURES.get('ENABLE_DISCUSSION_SERVICE'):
+        link = reverse('django_comment_client.forum.views.forum_form_discussion',
+                              args=[course.id])
+        tabs.append(CourseTab('Discussion', link, active_page == 'discussion'))
+    elif settings.MITX_FEATURES.get('ENABLE_DISCUSSION'):
+        ## This is Askbot, which we should be retiring soon...
+        tabs.append(CourseTab('Discussion', reverse('questions'), active_page == 'discussion'))
+
+    tabs.extend(_wiki({'name': 'Wiki', 'type': 'wiki'}, user, course, active_page))
+
+    if user.is_authenticated() and not course.hide_progress_tab:
+        tabs.extend(_progress({'name': 'Progress'}, user, course, active_page))
+
+    if has_access(user, course, 'staff'):
+        link = reverse('instructor_dashboard', args=[course.id])
+        tabs.append(CourseTab('Instructor', link, active_page=='instructor'))
+
+    return tabs

lms/djangoapps/staticbook/views.py

@@ -11,11 +11,13 @@ def index(request, course_id, book_index, page=0):
     course = get_course_with_access(request.user, course_id, 'load')
     staff_access = has_access(request.user, course, 'staff')
 
-    textbook = course.textbooks[int(book_index)]
+    book_index = int(book_index)
+    textbook = course.textbooks[book_index]
     table_of_contents = textbook.table_of_contents
 
     return render_to_response('staticbook.html',
-                              {'page': int(page), 'course': course, 'book_url': textbook.book_url,
+                              {'book_index': book_index, 'page': int(page),
+                               'course': course, 'book_url': textbook.book_url,
                                'table_of_contents': table_of_contents,
                                'staff_access': staff_access})
 

lms/templates/courseware/course_navigation.html

@@ -6,54 +6,21 @@ if active_page == None and active_page_context is not UNDEFINED:
   # If active_page is not passed in as an argument, it may be in the context as active_page_context
   active_page = active_page_context
 
-def url_class(url):
-  if url == active_page:
+def url_class(is_active):
+  if is_active:
     return "active"
   return ""
 %>
-<%! from django.core.urlresolvers import reverse %>
-<%! from courseware.access import has_access %>
+<%! from courseware.tabs import get_course_tabs %>
 
 <nav class="${active_page} course-material">
   <div class="inner-wrapper">
     <ol class="course-tabs">
-      <li class="courseware"><a href="${reverse('courseware', args=[course.id])}" class="${url_class('courseware')}">Courseware</a></li>
-      <li class="info"><a href="${reverse('info', args=[course.id])}" class="${url_class('info')}">Course Info</a></li>
-    % if hasattr(course,'syllabus_present') and course.syllabus_present:
-      <li class="syllabus"><a href="${reverse('syllabus', args=[course.id])}" class="${url_class('syllabus')}">Syllabus</a></li>
-    % endif
-    % if user.is_authenticated():
-      % if settings.MITX_FEATURES.get('ENABLE_TEXTBOOK'):
-        % for index, textbook in enumerate(course.textbooks):
-            <li class="book"><a href="${reverse('book', args=[course.id, index])}" class="${url_class('book')}">${textbook.title}</a></li>
+      % for tab in get_course_tabs(user, course, active_page):
+          <li>
+             <a href="${tab.link | h}" class="${url_class(tab.is_active)}">${tab.name | h}</a>
+          </li>
       % endfor
-      % endif
-
-      ## If they have a discussion link specified, use that even if we feature
-      ## flag discussions off. Disabling that is mostly a server safety feature
-      ## at this point, and we don't need to worry about external sites.
-      % if course.discussion_link:
-        <li class="discussion"><a href="${course.discussion_link}">Discussion</a></li>
-      % elif settings.MITX_FEATURES.get('ENABLE_DISCUSSION_SERVICE'):
-        <li class="discussion"><a href="${reverse('django_comment_client.forum.views.forum_form_discussion', args=[course.id])}" class="${url_class('discussion')}">Discussion</a></li>
-        ## <li class="news"><a href="${reverse('news', args=[course.id])}" class="${url_class('news')}">News</a></li>
-      % endif
-
-      ## This is Askbot, which we should be retiring soon...
-      % if settings.MITX_FEATURES.get('ENABLE_DISCUSSION'):
-        <li class="discussion"><a href="${reverse('questions')}">Discussion</a></li>
-      % endif
-    % endif
-
-    % if settings.WIKI_ENABLED:
-      <li class="wiki"><a href="${reverse('course_wiki', args=[course.id])}" class="${url_class('wiki')}">Wiki</a></li>
-    % endif
-    % if user.is_authenticated() and not course.hide_progress_tab:
-      <li class="profile"><a href="${reverse('progress', args=[course.id])}" class="${url_class('progress')}">Progress</a></li>
-    % endif
-    % if staff_access:
-      <li class="instructor"><a href="${reverse('instructor_dashboard', args=[course.id])}" class="${url_class('instructor')}">Instructor</a></li>
-    % endif
     <%block name="extratabs" />
     </ol>
   </div>

lms/templates/staticbook.html

@@ -61,7 +61,7 @@ $("#open_close_accordion a").click(function(){
 </script>
 </%block>
 
-<%include file="/courseware/course_navigation.html" args="active_page='book'" />
+<%include file="/courseware/course_navigation.html" args="active_page='textbook/{0}'.format(book_index)" />
 
 <section class="container">
   <div class="book-wrapper">