diff --git a/common/lib/xmodule/xmodule/css/capa/display.scss b/common/lib/xmodule/xmodule/css/capa/display.scss index a368167448..aea59a4d63 100644 --- a/common/lib/xmodule/xmodule/css/capa/display.scss +++ b/common/lib/xmodule/xmodule/css/capa/display.scss @@ -32,6 +32,7 @@ section.problem { display: inline; } + div { p { &.answer { diff --git a/common/lib/xmodule/xmodule/css/sequence/display.scss b/common/lib/xmodule/xmodule/css/sequence/display.scss index 7b8dc5f57c..25d2c26dda 100644 --- a/common/lib/xmodule/xmodule/css/sequence/display.scss +++ b/common/lib/xmodule/xmodule/css/sequence/display.scss @@ -37,7 +37,6 @@ nav.sequence-nav { height: 44px; margin: 0 30px; @include linear-gradient(top, #ddd, #eee); - overflow: hidden; @include box-shadow(0 1px 3px rgba(0, 0, 0, .1) inset); } diff --git a/common/lib/xmodule/xmodule/modulestore/search.py b/common/lib/xmodule/xmodule/modulestore/search.py index baf3d46b57..f9901e8bfe 100644 --- a/common/lib/xmodule/xmodule/modulestore/search.py +++ b/common/lib/xmodule/xmodule/modulestore/search.py @@ -6,15 +6,14 @@ from .exceptions import (ItemNotFoundError, NoPathToItem) from . import ModuleStore, Location -def path_to_location(modulestore, location, course_name=None): +def path_to_location(modulestore, course_id, location): ''' Try to find a course_id/chapter/section[/position] path to location in modulestore. The courseware insists that the first level in the course is chapter, but any kind of module can be a "section". location: something that can be passed to Location - course_name: [optional]. If not None, restrict search to paths - in that course. + course_id: Search for paths in this course. raise ItemNotFoundError if the location doesn't exist. @@ -27,7 +26,7 @@ def path_to_location(modulestore, location, course_name=None): A location may be accessible via many paths. This method may return any valid path. - If the section is a sequence, position will be the position + If the section is a sequential or vertical, position will be the position of this location in that sequence. Otherwise, position will be None. TODO (vshnayder): Not true yet. ''' @@ -41,7 +40,7 @@ def path_to_location(modulestore, location, course_name=None): xs = xs[1] return p - def find_path_to_course(location, course_name=None): + def find_path_to_course(): '''Find a path up the location graph to a node with the specified category. @@ -69,7 +68,8 @@ def path_to_location(modulestore, location, course_name=None): # print 'Processing loc={0}, path={1}'.format(loc, path) if loc.category == "course": - if course_name is None or course_name == loc.name: + # confirm that this is the right course + if course_id == CourseDescriptor.location_to_id(loc): # Found it! path = (loc, path) return flatten(path) @@ -81,17 +81,34 @@ def path_to_location(modulestore, location, course_name=None): # If we're here, there is no path return None - path = find_path_to_course(location, course_name) + path = find_path_to_course() if path is None: - raise(NoPathToItem(location)) + raise NoPathToItem(location) n = len(path) course_id = CourseDescriptor.location_to_id(path[0]) # pull out the location names chapter = path[1].name if n > 1 else None section = path[2].name if n > 2 else None - - # TODO (vshnayder): not handling position at all yet... + # Figure out the position position = None + # This block of code will find the position of a module within a nested tree + # of modules. If a problem is on tab 2 of a sequence that's on tab 3 of a + # sequence, the resulting position is 3_2. However, no positional modules + # (e.g. sequential and videosequence) currently deal with this form of + # representing nested positions. This needs to happen before jumping to a + # module nested in more than one positional module will work. + if n > 3: + position_list = [] + for path_index in range(2, n-1): + category = path[path_index].category + if category == 'sequential' or category == 'videosequence': + section_desc = modulestore.get_instance(course_id, path[path_index]) + child_locs = [c.location for c in section_desc.get_children()] + # positions are 1-indexed, and should be strings to be consistent with + # url parsing. + position_list.append(str(child_locs.index(path[path_index+1]) + 1)) + position = "_".join(position_list) + return (course_id, chapter, section, position) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/__init__.py b/common/lib/xmodule/xmodule/modulestore/tests/__init__.py new file mode 100644 index 0000000000..126f0136e2 --- /dev/null +++ b/common/lib/xmodule/xmodule/modulestore/tests/__init__.py @@ -0,0 +1,12 @@ +from path import path + +# from ~/mitx_all/mitx/common/lib/xmodule/xmodule/modulestore/tests/ +# to ~/mitx_all/mitx/common/test +TEST_DIR = path(__file__).abspath().dirname() +for i in range(5): + TEST_DIR = TEST_DIR.dirname() +TEST_DIR = TEST_DIR / 'test' + +DATA_DIR = TEST_DIR / 'data' + + diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore.py new file mode 100644 index 0000000000..c1d1d50a53 --- /dev/null +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore.py @@ -0,0 +1,34 @@ +from nose.tools import assert_equals, assert_raises, assert_not_equals, with_setup + +from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem +from xmodule.modulestore.search import path_to_location + +def check_path_to_location(modulestore): + '''Make sure that path_to_location works: should be passed a modulestore + with the toy and simple courses loaded.''' + should_work = ( + ("i4x://edX/toy/video/Welcome", + ("edX/toy/2012_Fall", "Overview", "Welcome", None)), + ("i4x://edX/toy/chapter/Overview", + ("edX/toy/2012_Fall", "Overview", None, None)), + ) + course_id = "edX/toy/2012_Fall" + + for location, expected in should_work: + assert_equals(path_to_location(modulestore, course_id, location), expected) + + not_found = ( + "i4x://edX/toy/video/WelcomeX", "i4x://edX/toy/course/NotHome" + ) + for location in not_found: + assert_raises(ItemNotFoundError, path_to_location, modulestore, course_id, location) + + # Since our test files are valid, there shouldn't be any + # elements with no path to them. But we can look for them in + # another course. + no_path = ( + "i4x://edX/simple/video/Lost_Video", + ) + for location in no_path: + assert_raises(NoPathToItem, path_to_location, modulestore, course_id, location) + diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index 746240e763..4c593e391e 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -1,23 +1,14 @@ import pymongo from nose.tools import assert_equals, assert_raises, assert_not_equals, with_setup -from path import path from pprint import pprint from xmodule.modulestore import Location -from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError, NoPathToItem from xmodule.modulestore.mongo import MongoModuleStore from xmodule.modulestore.xml_importer import import_from_xml -from xmodule.modulestore.search import path_to_location -# from ~/mitx_all/mitx/common/lib/xmodule/xmodule/modulestore/tests/ -# to ~/mitx_all/mitx/common/test -TEST_DIR = path(__file__).abspath().dirname() -for i in range(5): - TEST_DIR = TEST_DIR.dirname() -TEST_DIR = TEST_DIR / 'test' - -DATA_DIR = TEST_DIR / 'data' +from .test_modulestore import check_path_to_location +from . import DATA_DIR HOST = 'localhost' @@ -110,27 +101,5 @@ class TestMongoModuleStore(object): def test_path_to_location(self): '''Make sure that path_to_location works''' - should_work = ( - ("i4x://edX/toy/video/Welcome", - ("edX/toy/2012_Fall", "Overview", "Welcome", None)), - ("i4x://edX/toy/chapter/Overview", - ("edX/toy/2012_Fall", "Overview", None, None)), - ) - for location, expected in should_work: - assert_equals(path_to_location(self.store, location), expected) - - not_found = ( - "i4x://edX/toy/video/WelcomeX", "i4x://edX/toy/course/NotHome" - ) - for location in not_found: - assert_raises(ItemNotFoundError, path_to_location, self.store, location) - - # Since our test files are valid, there shouldn't be any - # elements with no path to them. But we can look for them in - # another course. - no_path = ( - "i4x://edX/simple/video/Lost_Video", - ) - for location in no_path: - assert_raises(NoPathToItem, path_to_location, self.store, location, "toy") + check_path_to_location(self.store) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py b/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py new file mode 100644 index 0000000000..c4446bebb5 --- /dev/null +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py @@ -0,0 +1,16 @@ +from xmodule.modulestore import Location +from xmodule.modulestore.xml import XMLModuleStore +from xmodule.modulestore.xml_importer import import_from_xml + +from .test_modulestore import check_path_to_location +from . import DATA_DIR + +class TestXMLModuleStore(object): + def test_path_to_location(self): + """Make sure that path_to_location works properly""" + + print "Starting import" + modulestore = XMLModuleStore(DATA_DIR, course_dirs=['toy', 'simple']) + print "finished import" + + check_path_to_location(modulestore) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index b0910efca9..23a5473292 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -37,7 +37,7 @@ def clean_out_mako_templating(xml_string): class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): def __init__(self, xmlstore, course_id, course_dir, - policy, error_tracker, **kwargs): + policy, error_tracker, parent_tracker, **kwargs): """ A class that handles loading from xml. Does some munging to ensure that all elements have unique slugs. @@ -143,8 +143,8 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): xmlstore.modules[course_id][descriptor.location] = descriptor - if xmlstore.eager: - descriptor.get_children() + for child in descriptor.get_children(): + parent_tracker.add_parent(child.location, descriptor.location) return descriptor render_template = lambda: '' @@ -160,12 +160,51 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): error_tracker, process_xml, policy, **kwargs) +class ParentTracker(object): + """A simple class to factor out the logic for tracking location parent pointers.""" + def __init__(self): + """ + Init + """ + # location -> set(parents). Not using defaultdict because we care about the empty case. + self._parents = dict() + + def add_parent(self, child, parent): + """ + Add a parent of child location to the set of parents. Duplicate calls have no effect. + + child and parent must be something that can be passed to Location. + """ + child = Location(child) + parent = Location(parent) + s = self._parents.setdefault(child, set()) + s.add(parent) + + def is_known(self, child): + """ + returns True iff child has some parents. + """ + child = Location(child) + return child in self._parents + + def make_known(self, location): + """Tell the parent tracker about an object, without registering any + parents for it. Used for the top level course descriptor locations.""" + self._parents.setdefault(location, set()) + + def parents(self, child): + """ + Return a list of the parents of this child. If not is_known(child), will throw a KeyError + """ + child = Location(child) + return list(self._parents[child]) + + class XMLModuleStore(ModuleStoreBase): """ An XML backed ModuleStore """ - def __init__(self, data_dir, default_class=None, eager=False, - course_dirs=None): + def __init__(self, data_dir, default_class=None, course_dirs=None): """ Initialize an XMLModuleStore from data_dir @@ -174,15 +213,11 @@ class XMLModuleStore(ModuleStoreBase): default_class: dot-separated string defining the default descriptor class to use if none is specified in entry_points - eager: If true, load the modules children immediately to force the - entire course tree to be parsed - course_dirs: If specified, the list of course_dirs to load. Otherwise, load all course dirs """ ModuleStoreBase.__init__(self) - self.eager = eager self.data_dir = path(data_dir) self.modules = defaultdict(dict) # course_id -> dict(location -> XModuleDescriptor) self.courses = {} # course_dir -> XModuleDescriptor for the course @@ -195,10 +230,7 @@ class XMLModuleStore(ModuleStoreBase): class_ = getattr(import_module(module_path), class_name) self.default_class = class_ - # TODO (cpennington): We need a better way of selecting specific sets of - # debug messages to enable. These were drowning out important messages - #log.debug('XMLModuleStore: eager=%s, data_dir = %s' % (eager, self.data_dir)) - #log.debug('default_class = %s' % self.default_class) + self.parent_tracker = ParentTracker() # If we are specifically asked for missing courses, that should # be an error. If we are asked for "all" courses, find the ones @@ -230,6 +262,7 @@ class XMLModuleStore(ModuleStoreBase): if course_descriptor is not None: self.courses[course_dir] = course_descriptor self._location_errors[course_descriptor.location] = errorlog + self.parent_tracker.make_known(course_descriptor.location) else: # Didn't load course. Instead, save the errors elsewhere. self.errored_courses[course_dir] = errorlog @@ -348,7 +381,7 @@ class XMLModuleStore(ModuleStoreBase): course_id = CourseDescriptor.make_id(org, course, url_name) - system = ImportSystem(self, course_id, course_dir, policy, tracker) + system = ImportSystem(self, course_id, course_dir, policy, tracker, self.parent_tracker) course_descriptor = system.process_xml(etree.tostring(course_data)) @@ -459,3 +492,19 @@ class XMLModuleStore(ModuleStoreBase): metadata: A nested dictionary of module metadata """ raise NotImplementedError("XMLModuleStores are read-only") + + def get_parent_locations(self, location): + '''Find all locations that are the parents of this location. Needed + for path_to_location(). + + If there is no data at location in this modulestore, raise + ItemNotFoundError. + + returns an iterable of things that can be passed to Location. This may + be empty if there are no parents. + ''' + location = Location.ensure_fully_specified(location) + if not self.parent_tracker.is_known(location): + raise ItemNotFoundError(location) + + return self.parent_tracker.parents(location) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 89f94d8cdb..be0bdc24c2 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -6,7 +6,7 @@ from .exceptions import DuplicateItemError log = logging.getLogger(__name__) -def import_from_xml(store, data_dir, course_dirs=None, eager=True, +def import_from_xml(store, data_dir, course_dirs=None, default_class='xmodule.raw_module.RawDescriptor'): """ Import the specified xml data_dir into the "store" modulestore, @@ -19,7 +19,6 @@ def import_from_xml(store, data_dir, course_dirs=None, eager=True, module_store = XMLModuleStore( data_dir, default_class=default_class, - eager=eager, course_dirs=course_dirs ) for course_id in module_store.modules.keys(): diff --git a/common/lib/xmodule/xmodule/tests/test_export.py b/common/lib/xmodule/xmodule/tests/test_export.py index 2520d95937..826e6c9d5a 100644 --- a/common/lib/xmodule/xmodule/tests/test_export.py +++ b/common/lib/xmodule/xmodule/tests/test_export.py @@ -49,7 +49,7 @@ class RoundTripTestCase(unittest.TestCase): copytree(data_dir / course_dir, root_dir / course_dir) print "Starting import" - initial_import = XMLModuleStore(root_dir, eager=True, course_dirs=[course_dir]) + initial_import = XMLModuleStore(root_dir, course_dirs=[course_dir]) courses = initial_import.get_courses() self.assertEquals(len(courses), 1) @@ -66,7 +66,7 @@ class RoundTripTestCase(unittest.TestCase): course_xml.write(xml) print "Starting second import" - second_import = XMLModuleStore(root_dir, eager=True, course_dirs=[course_dir]) + second_import = XMLModuleStore(root_dir, course_dirs=[course_dir]) courses2 = second_import.get_courses() self.assertEquals(len(courses2), 1) diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index a369850209..e81d82bf9e 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -193,7 +193,7 @@ class ImportTestCase(unittest.TestCase): """Make sure that metadata is inherited properly""" print "Starting import" - initial_import = XMLModuleStore(DATA_DIR, eager=True, course_dirs=['toy']) + initial_import = XMLModuleStore(DATA_DIR, course_dirs=['toy']) courses = initial_import.get_courses() self.assertEquals(len(courses), 1) @@ -216,7 +216,7 @@ class ImportTestCase(unittest.TestCase): def get_course(name): print "Importing {0}".format(name) - modulestore = XMLModuleStore(DATA_DIR, eager=True, course_dirs=[name]) + modulestore = XMLModuleStore(DATA_DIR, course_dirs=[name]) courses = modulestore.get_courses() self.assertEquals(len(courses), 1) return courses[0] @@ -245,7 +245,7 @@ class ImportTestCase(unittest.TestCase): happen--locations should uniquely name definitions. But in our imperfect XML world, it can (and likely will) happen.""" - modulestore = XMLModuleStore(DATA_DIR, eager=True, course_dirs=['toy', 'two_toys']) + modulestore = XMLModuleStore(DATA_DIR, course_dirs=['toy', 'two_toys']) toy_id = "edX/toy/2012_Fall" two_toy_id = "edX/toy/TT_2012_Fall" @@ -261,7 +261,7 @@ class ImportTestCase(unittest.TestCase): """Ensure that colons in url_names convert to file paths properly""" print "Starting import" - modulestore = XMLModuleStore(DATA_DIR, eager=True, course_dirs=['toy']) + modulestore = XMLModuleStore(DATA_DIR, course_dirs=['toy']) courses = modulestore.get_courses() self.assertEquals(len(courses), 1) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 0dc16bd976..82f623e977 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -544,7 +544,13 @@ class XModuleDescriptor(Plugin, HTMLSnippet): # Put import here to avoid circular import errors from xmodule.error_module import ErrorDescriptor msg = "Error loading from xml." - log.warning(msg + " " + str(err)) + log.warning(msg + " " + str(err)[:200]) + + # Normally, we don't want lots of exception traces in our logs from common + # content problems. But if you're debugging the xml loading code itself, + # uncomment the next line. + # log.exception(msg) + system.error_tracker(msg) err_msg = msg + "\n" + exc_info_to_str(sys.exc_info()) descriptor = ErrorDescriptor.from_xml(xml_data, system, org, course, diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index dbe4ff376d..91c769f90a 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -321,7 +321,7 @@ def _has_staff_access_to_location(user, location): return True # If not global staff, is the user in the Auth group for this class? - user_groups = [x[1] for x in user.groups.values_list()] + user_groups = [g.name for g in user.groups.all()] staff_group = _course_staff_group_name(location) if staff_group in user_groups: debug("Allow: user in group %s", staff_group) diff --git a/lms/djangoapps/courseware/management/commands/clean_xml.py b/lms/djangoapps/courseware/management/commands/clean_xml.py index 9fd52178a2..425dd156c1 100644 --- a/lms/djangoapps/courseware/management/commands/clean_xml.py +++ b/lms/djangoapps/courseware/management/commands/clean_xml.py @@ -57,7 +57,6 @@ def import_with_checks(course_dir, verbose=True): # module. modulestore = XMLModuleStore(data_dir, default_class=None, - eager=True, course_dirs=course_dirs) def str_of_err(tpl): diff --git a/lms/djangoapps/courseware/management/commands/metadata_to_json.py b/lms/djangoapps/courseware/management/commands/metadata_to_json.py index dcbcdc0df3..8ef0dee7b3 100644 --- a/lms/djangoapps/courseware/management/commands/metadata_to_json.py +++ b/lms/djangoapps/courseware/management/commands/metadata_to_json.py @@ -23,7 +23,6 @@ def import_course(course_dir, verbose=True): # module. modulestore = XMLModuleStore(data_dir, default_class=None, - eager=True, course_dirs=course_dirs) def str_of_err(tpl): diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 59414274a6..b8416426b6 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -197,7 +197,6 @@ def _get_module(user, request, location, student_module_cache, course_id, positi descriptor.category, shared_state_key) - instance_state = instance_module.state if instance_module is not None else None shared_state = shared_module.state if shared_module is not None else None @@ -256,7 +255,7 @@ def _get_module(user, request, location, student_module_cache, course_id, positi # by the replace_static_urls code below replace_urls=replace_urls, node_path=settings.NODE_PATH, - anonymous_student_id=anonymous_student_id + anonymous_student_id=anonymous_student_id, ) # pass position specified in URL to module through ModuleSystem system.set('position', position) diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index 88e0f53f70..56a2384bc5 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -68,7 +68,6 @@ def xml_store_config(data_dir): 'OPTIONS': { 'data_dir': data_dir, 'default_class': 'xmodule.hidden_module.HiddenDescriptor', - 'eager': True, } } } @@ -204,7 +203,8 @@ class PageLoader(ActivateLoginTestCase): self.assertEqual(len(courses), 1) course = courses[0] self.enroll(course) - + course_id = course.id + n = 0 num_bad = 0 all_ok = True @@ -214,7 +214,8 @@ class PageLoader(ActivateLoginTestCase): print "Checking ", descriptor.location.url() #print descriptor.__class__, descriptor.location resp = self.client.get(reverse('jump_to', - kwargs={'location': descriptor.location.url()})) + kwargs={'course_id': course_id, + 'location': descriptor.location.url()})) msg = str(resp.status_code) if resp.status_code != 200: diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 92f6716320..6122ebd333 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -5,8 +5,6 @@ import itertools from functools import partial -from functools import partial - from django.conf import settings from django.core.context_processors import csrf from django.core.urlresolvers import reverse @@ -152,7 +150,7 @@ def index(request, course_id, chapter=None, section=None, course_id, request.user, section_descriptor) module = get_module(request.user, request, section_descriptor.location, - student_module_cache, course_id) + student_module_cache, course_id, position) if module is None: # User is probably being clever and trying to access something # they don't have access to. @@ -196,7 +194,7 @@ def index(request, course_id, chapter=None, section=None, @ensure_csrf_cookie -def jump_to(request, location): +def jump_to(request, course_id, location): ''' Show the page that contains a specific location. @@ -213,15 +211,18 @@ def jump_to(request, location): # Complain if there's not data for this location try: - (course_id, chapter, section, position) = path_to_location(modulestore(), location) + (course_id, chapter, section, position) = path_to_location(modulestore(), course_id, location) except ItemNotFoundError: raise Http404("No data at this location: {0}".format(location)) except NoPathToItem: raise Http404("This location is not in any class: {0}".format(location)) # Rely on index to do all error handling and access control. - return index(request, course_id, chapter, section, position) - + return redirect('courseware_position', + course_id=course_id, + chapter=chapter, + section=section, + position=position) @ensure_csrf_cookie def course_info(request, course_id): """ @@ -328,6 +329,10 @@ def progress(request, course_id, student_id=None): # NOTE: To make sure impersonation by instructor works, use # student instead of request.user in the rest of the function. + # The pre-fetching of groups is done to make auth checks not require an + # additional DB lookup (this kills the Progress page in particular). + student = User.objects.prefetch_related("groups").get(id=student.id) + student_module_cache = StudentModuleCache.cache_for_descriptor_descendents( course_id, student, course) course_module = get_module(student, request, course.location, diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index 516344d79b..1122db3ad8 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -43,7 +43,8 @@ def get_full_modules(): class_path = settings.MODULESTORE['default']['ENGINE'] module_path, _, class_name = class_path.rpartition('.') class_ = getattr(import_module(module_path), class_name) - modulestore = class_(**dict(settings.MODULESTORE['default']['OPTIONS'].items() + [('eager', True)])) + # TODO (vshnayder): wth is this doing??? + modulestore = class_(**dict(settings.MODULESTORE['default']['OPTIONS'].items())) _FULLMODULES = modulestore.modules return _FULLMODULES @@ -76,7 +77,7 @@ def initialize_discussion_info(request, course): _is_course_discussion = lambda x: x[0].dict()['category'] == 'discussion' \ and x[0].dict()['course'] == course_name - + _get_module_descriptor = operator.itemgetter(1) def _get_module(module_descriptor): @@ -135,8 +136,8 @@ class HtmlResponse(HttpResponse): def __init__(self, html=''): super(HtmlResponse, self).__init__(html, content_type='text/plain') -class ViewNameMiddleware(object): - def process_view(self, request, view_func, view_args, view_kwargs): +class ViewNameMiddleware(object): + def process_view(self, request, view_func, view_args, view_kwargs): request.view_name = view_func.__name__ class QueryCountDebugMiddleware(object): diff --git a/lms/envs/common.py b/lms/envs/common.py index 36a8d54d3c..5cd28d24d9 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -223,7 +223,6 @@ MODULESTORE = { 'OPTIONS': { 'data_dir': DATA_DIR, 'default_class': 'xmodule.hidden_module.HiddenDescriptor', - 'eager': True, } } } diff --git a/lms/static/sass/multicourse/_course_about.scss b/lms/static/sass/multicourse/_course_about.scss index 92d19dff86..3f2813ba0e 100644 --- a/lms/static/sass/multicourse/_course_about.scss +++ b/lms/static/sass/multicourse/_course_about.scss @@ -115,6 +115,30 @@ } } + a { + &:hover, &:visited { + text-decoration: none; + } + } + + strong { + @include button(shiny, $blue); + @include box-sizing(border-box); + @include border-radius(3px); + display: block; + float: left; + font: normal 1.2rem/1.6rem $sans-serif; + letter-spacing: 1px; + padding: 10px 0px; + text-transform: uppercase; + text-align: center; + width: flex-grid(3, 8); + + &:hover { + color: rgb(255,255,255); + } + } + span.register { background: lighten($blue, 20%); border: 1px solid $blue; @@ -125,7 +149,10 @@ padding: 10px 0px 8px; text-transform: uppercase; text-align: center; - width: flex-grid(12); + float: left; + margin: 1px flex-gutter(8) 0 0; + @include transition(); + width: flex-grid(5, 8); } } } diff --git a/lms/static/sass/multicourse/_dashboard.scss b/lms/static/sass/multicourse/_dashboard.scss index f37c772aef..53418bc0dd 100644 --- a/lms/static/sass/multicourse/_dashboard.scss +++ b/lms/static/sass/multicourse/_dashboard.scss @@ -261,7 +261,7 @@ padding: 12px 0px; width: 100%; - a.university { + .university { background: rgba(255,255,255, 1); border: 1px solid rgb(180,180,180); @include border-radius(3px); @@ -269,17 +269,14 @@ color: $lighter-base-font-color; display: block; font-style: italic; + font-family: $sans-serif; + font-size: 16px; font-weight: 800; @include inline-block; margin-right: 10px; + margin-bottom: 0; padding: 5px 10px; - float: left; - - &:hover { - color: $blue; - text-decoration: none; - } } h3 { @@ -306,8 +303,12 @@ background: $yellow; border: 1px solid rgb(200,200,200); @include box-shadow(0 1px 0 0 rgba(255,255,255, 0.6)); - margin-top: 16px; + margin-top: 17px; + margin-right: flex-gutter(); padding: 5px; + width: flex-grid(8); + float: left; + @include box-sizing(border-box); p { color: $lighter-base-font-color; @@ -317,93 +318,46 @@ } } - .meta { - @include clearfix; - margin-top: 22px; - position: relative; - @include transition(opacity, 0.15s, linear); - width: 100%; - - - .course-work-icon { - @include background-image(url('../images/portal-icons/pencil-icon.png')); - background-size: cover; - float: left; - height: 22px; - opacity: 0.7; - width: 22px; - } - - .complete { - float: right; - - p { - color: $lighter-base-font-color; - font-style: italic; - @include inline-block; - text-align: right; - text-shadow: 0 1px rgba(255,255,255, 0.6); - - .completeness { - color: $base-font-color; - font-weight: 700; - margin-right: 5px; - } - } - } - - .progress { - @include box-shadow(0 1px 0 0 rgba(255,255,255, 0.6)); - left: 35px; - position: absolute; - right: 130px; - - .meter { - background: rgb(245,245,245); - border: 1px solid rgb(160,160,160); - @include box-shadow(inset 0 0 3px 0 rgba(0,0,0, 0.15)); - @include box-sizing(border-box); - @include border-radius(4px); - height: 22px; - margin: 0 auto; - padding: 2px; - width: 100%; - - .meter-fill { - background: $blue; - @include background-image(linear-gradient(-45deg, rgba(255,255,255, 0.15) 25%, - transparent 25%, - transparent 50%, - rgba(255,255,255, 0.15) 50%, - rgba(255,255,255, 0.15) 75%, - transparent 75%)); - background-size: 40px 40px; - background-repeat: repeat-x; - border: 1px solid rgb(115,115,115); - @include border-radius(4px); - @include box-sizing(border-box); - content: ""; - display: block; - height: 100%; - width: 60%; - } - } - } + .enter-course { + @include button(shiny, $blue); + @include box-sizing(border-box); + @include border-radius(3px); + display: block; + float: left; + font: normal 1rem/1.6rem $sans-serif; + letter-spacing: 1px; + padding: 6px 0px; + text-transform: uppercase; + text-align: center; + margin-top: 16px; + width: flex-grid(4); } } - &:hover { + > a:hover { .cover { .shade { background: rgba(255,255,255, 0.1); @include background-image(linear-gradient(-90deg, rgba(255,255,255, 0.3) 0%, - rgba(0,0,0, 0.3) 100%)); + rgba(0,0,0, 0.3) 100%)); } .arrow { opacity: 1; } } + + .info { + background: darken(rgb(250,250,250), 5%); + @include background-image(linear-gradient(-90deg, darken(rgb(253,253,253), 3%), darken(rgb(240,240,240), 5%))); + border-color: darken(rgb(190,190,190), 10%); + + .course-status { + background: darken($yellow, 3%); + border-color: darken(rgb(200,200,200), 3%); + @include box-shadow(0 1px 0 0 rgba(255,255,255, 0.6)); + } + } } } @@ -420,5 +374,6 @@ color: #333; } } + } } diff --git a/lms/templates/dashboard.html b/lms/templates/dashboard.html index 770e849841..0f9c26611b 100644 --- a/lms/templates/dashboard.html +++ b/lms/templates/dashboard.html @@ -72,30 +72,24 @@ else: course_target = reverse('about_course', args=[course.id]) %> - -
-
❯
+ + +
+
+
❯
+
+ +
+
+

${get_course_about_section(course, 'university')}

+

${course.number} ${course.title}

+
+
+

Class Starts - ${course.start_date_text}

+
+

View Courseware

+
-
-
- ${get_course_about_section(course, 'university')} -

${course.number} ${course.title}

-
-
-

Class Starts - ${course.start_date_text} -

-
-
-
-
-
-
-
-
- ##

60% complete

-
-
-
Unregister diff --git a/lms/templates/portal/course_about.html b/lms/templates/portal/course_about.html index 4931f1fed6..bff24d597a 100644 --- a/lms/templates/portal/course_about.html +++ b/lms/templates/portal/course_about.html @@ -74,7 +74,7 @@ %if show_link: %endif - You are registered for this course (${course.number}). + You are registered for this course (${course.number}) View Courseware %if show_link: %endif diff --git a/lms/urls.py b/lms/urls.py index d88aef17f6..278239751b 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -98,8 +98,9 @@ if settings.COURSEWARE_ENABLED: urlpatterns += ( # Hook django-masquerade, allowing staff to view site as other users url(r'^masquerade/', include('masquerade.urls')), - url(r'^jump_to/(?P.*)$', 'courseware.views.jump_to', name="jump_to"), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/jump_to/(?P.*)$', + 'courseware.views.jump_to', name="jump_to"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/modx/(?P.*?)/(?P[^/]*)$', 'courseware.module_render.modx_dispatch', name='modx_dispatch'), @@ -142,6 +143,8 @@ if settings.COURSEWARE_ENABLED: 'courseware.views.index', name="courseware_chapter"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/courseware/(?P[^/]*)/(?P
[^/]*)/$', 'courseware.views.index', name="courseware_section"), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/courseware/(?P[^/]*)/(?P
[^/]*)/(?P[^/]*)/?$', + 'courseware.views.index', name="courseware_position"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/progress$', 'courseware.views.progress', name="progress"), # Takes optional student_id for instructor use--shows profile as that student sees it. @@ -164,7 +167,7 @@ if settings.COURSEWARE_ENABLED: if settings.MITX_FEATURES.get('ENABLE_DISCUSSION_SERVICE'): urlpatterns += ( - url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/news$', + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/news$', 'courseware.views.news', name="news"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/discussion/', include('django_comment_client.urls'))