diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index cab57e68191dc42a05127f43a50fb0490abdb7ab..f36936318e3af5e4ccae4fc006ac29fec629c80d 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -140,7 +140,19 @@ def dashboard(request): if not user.is_active: message = render_to_string('registration/activate_account_notice.html', {'email': user.email}) - context = {'courses': courses, 'message': message} + + # Global staff can see what courses errored on their dashboard + staff_access = False + if has_access(user, 'global', 'staff'): + # Show any courses that errored on load + staff_access = True + errored_courses = modulestore().get_errored_courses() + + context = {'courses': courses, + 'message': message, + 'staff_access': staff_access, + 'errored_courses': errored_courses,} + return render_to_response('dashboard.html', context) diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index fb0721454c6b5669be11dfa4352985e211b9d5cb..5cc4a091650696ca86b29415fa11713ee2b3402b 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -78,7 +78,14 @@ class CourseDescriptor(SequenceDescriptor): def set_grading_policy(self, policy_str): """Parse the policy specified in policy_str, and save it""" - self._grading_policy = load_grading_policy(policy_str) + try: + self._grading_policy = load_grading_policy(policy_str) + except: + self.system.error_tracker("Failed to load grading policy") + # Setting this to an empty dictionary will lead to errors when + # grading needs to happen, but should allow course staff to see + # the error log. + self._grading_policy = {} @classmethod diff --git a/common/lib/xmodule/xmodule/graders.py b/common/lib/xmodule/xmodule/graders.py index 82dc37bf57af109ed3dd42a3af785845c7ebeeef..3f0bb63186cb9b4b8e4464603eb65187d049178c 100644 --- a/common/lib/xmodule/xmodule/graders.py +++ b/common/lib/xmodule/xmodule/graders.py @@ -1,6 +1,7 @@ import abc import json import logging +import sys from collections import namedtuple @@ -130,11 +131,11 @@ def grader_from_conf(conf): raise ValueError("Configuration has no appropriate grader class.") except (TypeError, ValueError) as error: - errorString = ("Unable to parse grader configuration:\n " + - str(subgraderconf) + - "\n Error was:\n " + str(error)) - log.critical(errorString) - raise ValueError(errorString) + # Add info and re-raise + msg = ("Unable to parse grader configuration:\n " + + str(subgraderconf) + + "\n Error was:\n " + str(error)) + raise ValueError(msg), None, sys.exc_info()[2] return WeightedSubsectionsGrader(subgraders) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 30610479b95429cca60bd4dcacf845fe015755d9..3eca72987eb144c7c155e66466fed215dcb66bcc 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -135,12 +135,12 @@ class XMLModuleStore(ModuleStoreBase): self.data_dir = path(data_dir) self.modules = defaultdict(dict) # course_id -> dict(location -> XModuleDescriptor) self.courses = {} # course_dir -> XModuleDescriptor for the course + self.errored_courses = {} # course_dir -> errorlog, for dirs that failed to load if default_class is None: self.default_class = None else: module_path, _, class_name = default_class.rpartition('.') - #log.debug('module_path = %s' % module_path) class_ = getattr(import_module(module_path), class_name) self.default_class = class_ @@ -163,18 +163,27 @@ class XMLModuleStore(ModuleStoreBase): ''' Load a course, keeping track of errors as we go along. ''' + # Special-case code here, since we don't have a location for the + # course before it loads. + # So, make a tracker to track load-time errors, then put in the right + # place after the course loads and we have its location + errorlog = make_error_tracker() + course_descriptor = None try: - # Special-case code here, since we don't have a location for the - # course before it loads. - # So, make a tracker to track load-time errors, then put in the right - # place after the course loads and we have its location - errorlog = make_error_tracker() course_descriptor = self.load_course(course_dir, errorlog.tracker) + except Exception as e: + msg = "Failed to load course '{0}': {1}".format(course_dir, str(e)) + log.exception(msg) + errorlog.tracker(msg) + + if course_descriptor is not None: self.courses[course_dir] = course_descriptor self._location_errors[course_descriptor.location] = errorlog - except: - msg = "Failed to load course '%s'" % course_dir - log.exception(msg) + else: + # Didn't load course. Instead, save the errors elsewhere. + self.errored_courses[course_dir] = errorlog + + def __unicode__(self): ''' @@ -211,6 +220,7 @@ class XMLModuleStore(ModuleStoreBase): for policy_path in paths: if not os.path.exists(policy_path): continue + log.debug("Loading grading policy from {0}".format(policy_path)) try: with open(policy_path) as grading_policy_file: policy_str = grading_policy_file.read() @@ -298,7 +308,7 @@ class XMLModuleStore(ModuleStoreBase): XModuleDescriptor.compute_inherited_metadata(course_descriptor) # Try to load grading policy - paths = [self.data_dir / 'grading_policy.json'] + paths = [self.data_dir / course_dir / 'grading_policy.json'] if policy_dir: paths = [policy_dir / 'grading_policy.json'] + paths @@ -356,6 +366,12 @@ class XMLModuleStore(ModuleStoreBase): """ return self.courses.values() + def get_errored_courses(self): + """ + Return a dictionary of course_dir -> [(msg, exception_str)], for each + course_dir where course loading failed. + """ + return dict( (k, self.errored_courses[k].errors) for k in self.errored_courses) def create_item(self, location): raise NotImplementedError("XMLModuleStores are read-only") diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index eaf70d78146970c272a90dbf920c926fea6de61a..281580cf3365a4d6247665d85d201c591ed94d94 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -63,6 +63,9 @@ def has_access(user, obj, action): if isinstance(obj, Location): return _has_access_location(user, obj, action) + if isinstance(obj, basestring): + return _has_access_string(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(): '{0}'" @@ -238,6 +241,30 @@ def _has_access_location(user, location, action): return _dispatch(checkers, action, user, location) +def _has_access_string(user, perm, action): + """ + Check if user has certain special access, specified as string. Valid strings: + + 'global' + + Valid actions: + + 'staff' -- global staff access. + """ + + def check_staff(): + if perm != 'global': + debug("Deny: invalid permission '%s'", perm) + return False + return _has_global_staff_access(user) + + checkers = { + 'staff': check_staff + } + + return _dispatch(checkers, action, user, perm) + + ##### Internal helper methods below def _dispatch(table, action, user, obj): @@ -266,6 +293,15 @@ def _course_staff_group_name(location): """ return 'staff_%s' % Location(location).course +def _has_global_staff_access(user): + if user.is_staff: + debug("Allow: user.is_staff") + return True + else: + debug("Deny: not user.is_staff") + return False + + def _has_staff_access_to_location(user, location): ''' Returns True if the given user has staff access to a location. For now this diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index b4407b7f93b57ac0aa6bedb9cb3f7dabad46c033..30ca38728a25951b0f31d33a5cdb0e49c229cca0 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -30,7 +30,6 @@ def get_course_by_id(course_id): raise Http404("Course not found.") - def get_course_with_access(user, course_id, action): """ Given a course_id, look up the corresponding course descriptor, diff --git a/lms/templates/dashboard.html b/lms/templates/dashboard.html index cfe8a0953cb3487e0632a7acbb8a8738890b66ef..770e84984109e7ce7dc7f51034ff8b687e16661c 100644 --- a/lms/templates/dashboard.html +++ b/lms/templates/dashboard.html @@ -107,6 +107,21 @@ </section> % endif + % if staff_access and len(errored_courses) > 0: + <div id="course-errors"> + <h2>Course-loading errors</h2> + + % for course_dir, errors in errored_courses.items(): + <h3>${course_dir | h}</h3> + <ul> + % for (msg, err) in errors: + <li>${msg} + <ul><li><pre>${err}</pre></li></ul> + </li> + % endfor + </ul> + % endfor + % endif </section> </section>