Merge pull request openedx#394 from MITx/feature/victor/hide-error-mo…

…dules

Hide errors from non-staff users

common/lib/xmodule/xmodule/error_module.py

@@ -27,6 +27,14 @@ def get_html(self):
             'is_staff' : self.system.is_staff,
             })
 
+    def displayable_items(self):
+        """Hide errors in the profile and table of contents for non-staff
+        users.
+        """
+        if self.system.is_staff:
+            return [self]
+        return []
+
 class ErrorDescriptor(EditingDescriptor):
     """
     Module that provides a raw editing view of broken xml.
@@ -75,7 +83,8 @@ def from_xml(cls, xml_data, system, org=None, course=None,
         # TODO (vshnayder): Do we need a unique slug here?  Just pick a random
         # 64-bit num?
         location = ['i4x', org, course, 'error', url_name]
-        metadata = {}  # stays in the xml_data
+        # real metadata stays in the xml_data, but add a display name
+        metadata = {'display_name': 'Error ' + url_name}
 
         return cls(system, definition, location=location, metadata=metadata)
 

lms/djangoapps/courseware/courses.py

@@ -8,6 +8,7 @@
 from django.http import Http404
 
 from xmodule.course_module import CourseDescriptor
+from xmodule.modulestore import Location
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from static_replace import replace_urls, try_staticfiles_lookup
@@ -178,6 +179,14 @@ def has_staff_access_to_course_id(user, course_id):
     return has_staff_access_to_course(user, loc.course)
 
 
+def has_staff_access_to_location(user, location):
+    """Helper method that checks whether the user has staff access to
+    the course of the location.
+
+    location: something that can be passed to Location
+    """
+    return has_staff_access_to_course(user, Location(location).course)
+
 def has_access_to_course(user, course):
     '''course is the .course element of a location'''
     if course.metadata.get('ispublic'):

lms/djangoapps/courseware/grades.py

@@ -146,8 +146,14 @@ def progress_summary(student, course, grader, student_module_cache):
     """
     chapters = []
     for c in course.get_children():
+        # Don't include chapters that aren't displayable (e.g. due to error)
+        if c not in c.displayable_items():
+            continue
         sections = []
         for s in c.get_children():
+            # Same for sections
+            if s not in s.displayable_items():
+                continue
             graded = s.metadata.get('graded', False)
             scores = []
             for module in yield_module_descendents(s):

lms/djangoapps/courseware/module_render.py

@@ -16,7 +16,8 @@
 from xmodule.x_module import ModuleSystem
 from xmodule_modifiers import replace_static_urls, add_histogram, wrap_xmodule
 
-from courseware.courses import has_staff_access_to_course
+from courseware.courses import (has_staff_access_to_course,
+                                has_staff_access_to_location)
 
 log = logging.getLogger("mitx.courseware")
 
@@ -182,7 +183,7 @@ def _get_module(location):
                           # a module is coming through get_html and is therefore covered
                           # by the replace_static_urls code below
                           replace_urls=replace_urls,
-                          is_staff=user.is_staff,
+                          is_staff=has_staff_access_to_location(user, location),
                           )
     # pass position specified in URL to module through ModuleSystem
     system.set('position', position)