Merge pull request #415 from MITx/feature/victor/start-dates

Feature/victor/start dates

common/lib/xmodule/xmodule/abtest_module.py

@@ -49,9 +49,9 @@ class ABTestModule(XModule):
         return json.dumps({'group': self.group})
 
     def displayable_items(self):
-        return filter(None, [self.system.get_module(child)
-                             for child
-                             in self.definition['data']['group_content'][self.group]])
+        child_locations = self.definition['data']['group_content'][self.group]
+        children = [self.system.get_module(loc) for loc in child_locations]
+        return [c for c in children if c is not None]
 
 
 # TODO (cpennington): Use Groups should be a first class object, rather than being

common/lib/xmodule/xmodule/x_module.py

@@ -219,11 +219,11 @@ class XModule(HTMLSnippet):
         Return module instances for all the children of this module.
         '''
         if self._loaded_children is None:
+            child_locations = self.definition.get('children', [])
+            children = [self.system.get_module(loc) for loc in child_locations]
             # get_module returns None if the current user doesn't have access
             # to the location.
-            self._loaded_children = filter(None,
-                [self.system.get_module(child)
-                for child in self.definition.get('children', [])])
+            self._loaded_children = [c for c in children if c is not None]
 
         return self._loaded_children
 

lms/djangoapps/courseware/access.py

@@ -65,9 +65,10 @@ def has_access(user, obj, action):
 
     # Passing an unknown object here is a coding error, so rather than
     # returning a default, complain.
-    raise TypeError("Unknown object type in has_access().  Object type: '{}'"
+    raise TypeError("Unknown object type in has_access(): '{}'"
                     .format(type(obj)))
 
+
 # ================ Implementation helpers ================================
 
 def _has_access_course_desc(user, course, action):
@@ -83,8 +84,11 @@ def _has_access_course_desc(user, course, action):
     'staff' -- staff access to course.
     """
     def can_load():
-        "Can this user load this course?"
-        # delegate to generic descriptor check
+        "Can this user load this course?
+
+        NOTE: this is not checking whether user is actually enrolled in the course.
+        "
+        # delegate to generic descriptor check to check start dates
         return _has_access_descriptor(user, course, action)
 
     def can_enroll():
@@ -169,6 +173,12 @@ def _has_access_descriptor(user, descriptor, action):
     has_access(), it will not do the right thing.
     """
     def can_load():
+        """
+        NOTE: This does not check that the student is enrolled in the course
+        that contains this module.  We may or may not want to allow non-enrolled
+        students to see modules.  If not, views should check the course, so we
+        don't have to hit the enrollments table on every module load.
+        """
         # If start dates are off, can always load
         if settings.MITX_FEATURES['DISABLE_START_DATES']:
             debug("Allow: DISABLE_START_DATES")
@@ -196,8 +206,6 @@ def _has_access_descriptor(user, descriptor, action):
     return _dispatch(checkers, action, user, descriptor)
 
 
-
-
 def _has_access_xmodule(user, xmodule, action):
     """
     Check if user has access to this xmodule.

lms/djangoapps/courseware/views.py

@@ -110,6 +110,7 @@ def index(request, course_id, chapter=None, section=None,
      - HTTPresponse
     """
     course = get_course_with_access(request.user, course_id, 'load')
+    staff_access = has_access(request.user, course, 'staff')
     registered = registered_for_course(course, request.user)
     if not registered:
         # TODO (vshnayder): do course instructors need to be registered to see course?
@@ -123,7 +124,8 @@ def index(request, course_id, chapter=None, section=None,
             'COURSE_TITLE': course.title,
             'course': course,
             'init': '',
-            'content': ''
+            'content': '',
+            'staff_access': staff_access,
             }
 
         look_for_module = chapter is not None and section is not None
@@ -166,7 +168,8 @@ def index(request, course_id, chapter=None, section=None,
                               position=position
                               ))
             try:
-                result = render_to_response('courseware-error.html', {})
+                result = render_to_response('courseware-error.html',
+                                            {'staff_access': staff_access})
             except:
                 result = HttpResponse("There was an unrecoverable error")
 
@@ -208,8 +211,10 @@ def course_info(request, course_id):
     Assumes the course_id is in a valid format.
     """
     course = get_course_with_access(request.user, course_id, 'load')
+    staff_access = has_access(request.user, course, 'staff')
 
-    return render_to_response('info.html', {'course': course})
+    return render_to_response('info.html', {'course': course,
+                                            'staff_access': staff_access,})
 
 
 def registered_for_course(course, user):
@@ -257,13 +262,14 @@ def profile(request, course_id, student_id=None):
     Course staff are allowed to see the profiles of students in their class.
     """
     course = get_course_with_access(request.user, course_id, 'load')
+    staff_access = has_access(request.user, course, 'staff')
 
     if student_id is None or student_id == request.user.id:
         # always allowed to see your own profile
         student = request.user
     else:
         # Requesting access to a different student's profile
-        if not has_access(request.user, course, 'staff'):
+        if not staff_access:
             raise Http404
         student = User.objects.get(id=int(student_id))
 
@@ -282,8 +288,9 @@ def profile(request, course_id, student_id=None):
                'email': student.email,
                'course': course,
                'csrf': csrf(request)['csrf_token'],
-               'courseware_summary' : courseware_summary,
-               'grade_summary' : grade_summary
+               'courseware_summary': courseware_summary,
+               'grade_summary': grade_summary,
+               'staff_access': staff_access,
                }
     context.update()
 
@@ -316,7 +323,10 @@ def gradebook(request, course_id):
                      for student in enrolled_students]
 
     return render_to_response('gradebook.html', {'students': student_info,
-                                                 'course': course, 'course_id': course_id})
+                                                 'course': course,
+                                                 'course_id': course_id,
+                                                 # Checked above
+                                                 'staff_access': True,})
 
 
 @cache_control(no_cache=True, no_store=True, must_revalidate=True)
@@ -325,7 +335,8 @@ def grade_summary(request, course_id):
     course = get_course_with_access(request.user, course_id, 'staff')
 
     # For now, just a static page
-    context = {'course': course }
+    context = {'course': course,
+               'staff_access': True,}
     return render_to_response('grade_summary.html', context)
 
 
@@ -335,6 +346,7 @@ def instructor_dashboard(request, course_id):
     course = get_course_with_access(request.user, course_id, 'staff')
 
     # For now, just a static page
-    context = {'course': course }
+    context = {'course': course,
+               'staff_access': True,}
     return render_to_response('instructor_dashboard.html', context)
 

lms/djangoapps/simplewiki/views.py

@@ -10,6 +10,7 @@ from django.utils.translation import ugettext_lazy as _
 from mitxmako.shortcuts import render_to_response
 
 from courseware.courses import get_opt_course_with_access
+from courseware.access import has_access
 from xmodule.course_module import CourseDescriptor
 from xmodule.modulestore.django import modulestore
 
@@ -49,6 +50,10 @@ def update_template_dictionary(dictionary, request=None, course=None, article=No
     if request:
         dictionary.update(csrf(request))
 
+    if request and course:
+        dictionary['staff_access'] = has_access(request.user, course, 'staff')
+    else:
+        dictionary['staff_access'] = False
 
 def view(request, article_path, course_id=None):
     course = get_opt_course_with_access(request.user, course_id, 'load')

lms/djangoapps/staticbook/views.py

 from django.contrib.auth.decorators import login_required
 from mitxmako.shortcuts import render_to_response
 
+from courseware.access import has_access
 from courseware.courses import get_course_with_access
 from lxml import etree
 
 @login_required
 def index(request, course_id, page=0):
     course = get_course_with_access(request.user, course_id, 'load')
-    raw_table_of_contents = open('lms/templates/book_toc.xml', 'r') # TODO: This will need to come from S3
+    staff_access = has_access(request.user, course, 'staff')
+
+    # TODO: This will need to come from S3
+    raw_table_of_contents = open('lms/templates/book_toc.xml', 'r')
     table_of_contents = etree.parse(raw_table_of_contents).getroot()
+
     return render_to_response('staticbook.html',
                               {'page': int(page), 'course': course,
-                               'table_of_contents': table_of_contents})
+                               'table_of_contents': table_of_contents,
+                               'staff_access': staff_access})
 
 
 def index_shifted(request, course_id, page):

lms/templates/course_navigation.html

@@ -28,7 +28,7 @@ def url_class(url):
 % if user.is_authenticated():
       <li class="profile"><a href="${reverse('profile', args=[course.id])}" class="${url_class('profile')}">Profile</a></li>
 % endif
-% if has_access(user, course, 'staff'):
+% if staff_access:
       <li class="instructor"><a href="${reverse('instructor_dashboard', args=[course.id])}" class="${url_class('instructor')}">Instructor</a></li>
 % endif