From 808c70caf525587b088b7e7c67478eb4f6b6688e Mon Sep 17 00:00:00 2001 From: kimth Date: Wed, 15 Aug 2012 09:39:13 -0400 Subject: [PATCH 01/46] Quick workaround courses hierarchy --- lms/djangoapps/courseware/views.py | 17 ++++++++++++++++- lms/urls.py | 1 + 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 00ab45e605..1ea0a8407e 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -1,5 +1,6 @@ import json import logging +import re import urllib import itertools @@ -8,7 +9,7 @@ from django.core.context_processors import csrf from django.core.urlresolvers import reverse from django.contrib.auth.models import User from django.contrib.auth.decorators import login_required -from django.http import Http404, HttpResponse +from django.http import Http404, HttpResponse, HttpResponseRedirect from django.shortcuts import redirect from mitxmako.shortcuts import render_to_response, render_to_string #from django.views.decorators.csrf import ensure_csrf_cookie @@ -35,6 +36,20 @@ log = logging.getLogger("mitx.courseware") template_imports = {'urllib': urllib} + +def course_redirect(request, course_path): + ''' + Allows course XML to refer to its own directory structure without knowing the multicourse hierarchy. + Prepends the multicourse path to the requested course-internal path. + If the course_id cannot be determined, redirect to multicourse page. (NOTE: is Http404 more appropriate?) + ''' + referer = request.META['HTTP_REFERER'] + match = re.search(r'/courses/(?P[^/]+/[^/]+/[^/]+)/', referer) + courses_path = '/courses/' + if match is not None: + courses_path += match.group('course_id') + '/' + course_path + return HttpResponseRedirect(courses_path) + def user_groups(user): """ TODO (vshnayder): This is not used. When we have a new plan for groups, adjust appropriately. diff --git a/lms/urls.py b/lms/urls.py index aaeba1b51e..af49aa2f3c 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -124,6 +124,7 @@ if settings.COURSEWARE_ENABLED: 'courseware.views.course_about', name="about_course"), #Inside the course + url(r'^course/(?P.*)$','courseware.views.course_redirect'), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/info$', 'courseware.views.course_info', name="info"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/book$', From 9a14af4ba1e6580674ee02febdfd77e959f17807 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 15 Aug 2012 10:27:07 -0400 Subject: [PATCH 02/46] address comment on #412 --- common/lib/xmodule/xmodule/abtest_module.py | 6 +++--- common/lib/xmodule/xmodule/x_module.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/common/lib/xmodule/xmodule/abtest_module.py b/common/lib/xmodule/xmodule/abtest_module.py index ca00db4c9a..ceca6ff9ed 100644 --- a/common/lib/xmodule/xmodule/abtest_module.py +++ b/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 diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 06449dc37f..1063c5428b 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/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 From 9425bbba609029c44d8633afd044aba8dd205ff0 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 15 Aug 2012 10:28:08 -0400 Subject: [PATCH 03/46] Address other comment on #413 - don't call has_access directly from template, pass a staff_access variable instead --- lms/djangoapps/courseware/access.py | 2 +- lms/djangoapps/courseware/views.py | 30 +++++++++++++++++++--------- lms/djangoapps/simplewiki/views.py | 5 +++++ lms/djangoapps/staticbook/views.py | 10 ++++++++-- lms/templates/course_navigation.html | 2 +- 5 files changed, 36 insertions(+), 13 deletions(-) diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 9605c827de..1494de6662 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -65,7 +65,7 @@ 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 ================================ diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index ab63872170..98de50c089 100644 --- a/lms/djangoapps/courseware/views.py +++ b/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) diff --git a/lms/djangoapps/simplewiki/views.py b/lms/djangoapps/simplewiki/views.py index 2ee76a1868..192035fcde 100644 --- a/lms/djangoapps/simplewiki/views.py +++ b/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, 'load') + else: + dictionary['staff_access'] = False def view(request, article_path, course_id=None): course = get_opt_course_with_access(request.user, course_id, 'load') diff --git a/lms/djangoapps/staticbook/views.py b/lms/djangoapps/staticbook/views.py index aec3fb1448..2e19ab6425 100644 --- a/lms/djangoapps/staticbook/views.py +++ b/lms/djangoapps/staticbook/views.py @@ -1,17 +1,23 @@ 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): diff --git a/lms/templates/course_navigation.html b/lms/templates/course_navigation.html index dd1c8d93b9..9e93b2fb14 100644 --- a/lms/templates/course_navigation.html +++ b/lms/templates/course_navigation.html @@ -28,7 +28,7 @@ def url_class(url): % if user.is_authenticated():
  • Profile
    • % endif -% if has_access(user, course, 'staff'): +% if staff_access:
  • Instructor
    • % endif From 9805b416550e4c7784acb1e96829da70f6e15c1e Mon Sep 17 00:00:00 2001 From: kimth Date: Wed, 15 Aug 2012 10:46:33 -0400 Subject: [PATCH 04/46] row --> rows --- lms/djangoapps/courseware/models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index 261140dec7..4389a5f169 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -77,7 +77,7 @@ class StudentModuleCache(object): Arguments user: The user for which to fetch maching StudentModules descriptors: An array of XModuleDescriptors. - select_for_update: Flag indicating whether the row should be locked until end of transaction + select_for_update: Flag indicating whether the rows should be locked until end of transaction ''' if user.is_authenticated(): module_ids = self._get_module_state_keys(descriptors) @@ -110,7 +110,7 @@ class StudentModuleCache(object): the supplied descriptor. If depth is None, load all descendent StudentModules descriptor_filter is a function that accepts a descriptor and return wether the StudentModule should be cached - select_for_update: Flag indicating whether the row should be locked until end of transaction + select_for_update: Flag indicating whether the rows should be locked until end of transaction """ def get_child_descriptors(descriptor, depth, descriptor_filter): From a2057f9ea42d5ded3e1e5f15057ee470932a387b Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 15 Aug 2012 11:47:01 -0400 Subject: [PATCH 05/46] fix docstring --- common/lib/xmodule/xmodule/xml_module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 399d5d3f91..3c2f3269ca 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -166,7 +166,7 @@ class XmlDescriptor(XModuleDescriptor): Subclasses should not need to override this except in special cases (e.g. html module)''' - # VS[compat] -- the filename tag should go away once everything is + # VS[compat] -- the filename attr should go away once everything is # converted. (note: make sure html files still work once this goes away) filename = xml_object.get('filename') if filename is None: From 346d5b91a1c07aad0b51fca09fbac2aa1014d586 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Wed, 15 Aug 2012 10:58:20 -0400 Subject: [PATCH 06/46] implement subdomain-based course displays --- common/djangoapps/external_auth/views.py | 7 ++- common/djangoapps/student/views.py | 7 ++- lms/djangoapps/courseware/courses.py | 78 +++++++++++++++++++++++- lms/djangoapps/courseware/views.py | 6 +- lms/envs/common.py | 6 ++ 5 files changed, 95 insertions(+), 9 deletions(-) diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py index 0425f3e158..35e59db0ca 100644 --- a/common/djangoapps/external_auth/views.py +++ b/common/djangoapps/external_auth/views.py @@ -1,3 +1,4 @@ +import functools import json import logging import random @@ -156,7 +157,7 @@ def edXauth_signup(request, eamap=None): log.debug('ExtAuth: doing signup for %s' % eamap.external_email) - return student_views.main_index(extra_context=context) + return student_views.main_index(request, extra_context=context) #----------------------------------------------------------------------------- # MIT SSL @@ -206,7 +207,7 @@ def edXauth_ssl_login(request): pass if not cert: # no certificate information - go onward to main index - return student_views.main_index() + return student_views.main_index(request) (user, email, fullname) = ssl_dn_extract_info(cert) @@ -216,4 +217,4 @@ def edXauth_ssl_login(request): credentials=cert, email=email, fullname=fullname, - retfun = student_views.main_index) + retfun = functools.partial(student_views.main_index, request)) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index b6aa62e03d..e02a5f4562 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -68,9 +68,9 @@ def index(request): from external_auth.views import edXauth_ssl_login return edXauth_ssl_login(request) - return main_index(user=request.user) + return main_index(request, user=request.user) -def main_index(extra_context = {}, user=None): +def main_index(request, extra_context={}, user=None): ''' Render the edX main page. @@ -93,7 +93,8 @@ def main_index(extra_context = {}, user=None): entry.summary = soup.getText() # The course selection work is done in courseware.courses. - universities = get_courses_by_university(None) + universities = get_courses_by_university(None, + domain=request.META['HTTP_HOST']) context = {'universities': universities, 'entries': entries} context.update(extra_context) return render_to_response('index.html', context) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 2e74853760..5818252cf1 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -2,8 +2,8 @@ from collections import defaultdict from fs.errors import ResourceNotFoundError from functools import wraps import logging -from path import path +from path import path from django.conf import settings from django.http import Http404 @@ -142,19 +142,95 @@ def get_course_info_section(course, section_key): raise KeyError("Invalid about key " + str(section_key)) +<<<<<<< HEAD def get_courses_by_university(user): +======= +def course_staff_group_name(course): + ''' + course should be either a CourseDescriptor instance, or a string (the + .course entry of a Location) + ''' + if isinstance(course, str) or isinstance(course, unicode): + coursename = course + else: + # should be a CourseDescriptor, so grab its location.course: + coursename = course.location.course + return 'staff_%s' % coursename + +def has_staff_access_to_course(user, course): + ''' + Returns True if the given user has staff access to the course. + This means that user is in the staff_* group, or is an overall admin. + TODO (vshnayder): this needs to be changed to allow per-course_id permissions, not per-course + (e.g. staff in 2012 is different from 2013, but maybe some people always have access) + + course is the course field of the location being accessed. + ''' + if user is None or (not user.is_authenticated()) or course is None: + return False + if user.is_staff: + return True + + # note this is the Auth group, not UserTestGroup + user_groups = [x[1] for x in user.groups.values_list()] + staff_group = course_staff_group_name(course) + if staff_group in user_groups: + return True + return False + +def has_staff_access_to_course_id(user, course_id): + """Helper method that takes a course_id instead of a course name""" + loc = CourseDescriptor.id_to_location(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'): + return True + return has_staff_access_to_course(user,course) + + +def get_courses_by_university(user, domain=None): +>>>>>>> implement subdomain-based course displays ''' Returns dict of lists of courses available, keyed by course.org (ie university). Courses are sorted by course.number. ''' + subdomain = domain.split(".")[0] + # TODO: Clean up how 'error' is done. # filter out any courses that errored. + if settings.MITX_FEATURES.get('SUBDOMAIN_COURSE_LISTINGS'): + if subdomain in settings.COURSE_LISTINGS: + visible_courses = settings.COURSE_LISTINGS[subdomain] + else: + visible_courses = frozenset(settings.COURSE_LISTINGS['default']) + courses = [c for c in modulestore().get_courses() if isinstance(c, CourseDescriptor)] courses = sorted(courses, key=lambda course: course.number) universities = defaultdict(list) for course in courses: +<<<<<<< HEAD if has_access(user, course, 'see_exists'): universities[course.org].append(course) +======= + if settings.MITX_FEATURES.get('ACCESS_REQUIRE_STAFF_FOR_COURSE'): + if not has_access_to_course(user,course): + continue + if settings.MITX_FEATURES.get('SUBDOMAIN_COURSE_LISTINGS'): + if course.id not in visible_courses: + continue + universities[course.org].append(course) +>>>>>>> implement subdomain-based course displays return universities diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index ab63872170..b6a56830e0 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -63,7 +63,8 @@ def courses(request): ''' Render "find courses" page. The course selection work is done in courseware.courses. ''' - universities = get_courses_by_university(request.user) + universities = get_courses_by_university(request.user, + domain=request.META['HTTP_HOST']) return render_to_response("courses.html", {'universities': universities}) @@ -241,7 +242,8 @@ def university_profile(request, org_id): raise Http404("University Profile not found for {0}".format(org_id)) # Only grab courses for this org... - courses = get_courses_by_university(request.user)[org_id] + courses = get_courses_by_university(request.user, + domain=request.META['HTTP_HOST'])[org_id] context = dict(courses=courses, org_id=org_id) template_file = "university_profile/{0}.html".format(org_id).lower() diff --git a/lms/envs/common.py b/lms/envs/common.py index 303e73ea81..45818c0ff2 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -49,6 +49,11 @@ MITX_FEATURES = { ## Doing so will cause all courses to be released on production 'DISABLE_START_DATES': False, # When True, all courses will be active, regardless of start date + # When True, will only publicly list courses by the subdomain. Expects you + # to define COURSE_LISTINGS, a dictionary mapping subdomains to lists of + # course_ids (see dev_int.py for an example) + 'SUBDOMAIN_COURSE_LISTINGS' : False, + 'ENABLE_TEXTBOOK' : True, 'ENABLE_DISCUSSION' : True, @@ -61,6 +66,7 @@ MITX_FEATURES = { 'ACCESS_REQUIRE_STAFF_FOR_COURSE': False, 'AUTH_USE_OPENID': False, 'AUTH_USE_MIT_CERTIFICATES' : False, + } # Used for A/B testing From 7386d66c0f354a089d36d473c5a3a0786d792f17 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Wed, 15 Aug 2012 11:11:54 -0400 Subject: [PATCH 07/46] Add dev_int.py config for testing course listings by subdomain --- lms/envs/dev_int.py | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 lms/envs/dev_int.py diff --git a/lms/envs/dev_int.py b/lms/envs/dev_int.py new file mode 100644 index 0000000000..83c7fb0aba --- /dev/null +++ b/lms/envs/dev_int.py @@ -0,0 +1,32 @@ +""" +This enables use of course listings by subdomain. To see it in action, point the +following domains to 127.0.0.1 in your /etc/hosts file: + + berkeley.dev + harvard.dev + mit.dev + +Note that OS X has a bug where using *.local domains is excruciatingly slow, so +use *.dev domains instead for local testing. +""" +from .dev import * + +MITX_FEATURES['SUBDOMAIN_COURSE_LISTINGS'] = True + +COURSE_LISTINGS = { + 'default' : ['BerkeleyX/CS169.1x/2012_Fall', + 'BerkeleyX/CS188.1x/2012_Fall', + 'HarvardX/CS50x/2012', + 'HarvardX/PH207x/2012_Fall' + 'MITx/3.091x/2012_Fall', + 'MITx/6.002x/2012_Fall', + 'MITx/6.00x/2012_Fall'], + + 'berkeley': ['BerkeleyX/CS169.1x/2012_Fall', + 'BerkeleyX/CS188.1x/2012_Fall'], + + 'harvard' : ['HarvardX/CS50x/2012'], + + 'mit' : ['MITx/3.091x/2012_Fall', + 'MITx/6.00x/2012_Fall'] +} From d0f2641890c835aa7263f125ec9fd71a4ff5fac9 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Wed, 15 Aug 2012 11:12:44 -0400 Subject: [PATCH 08/46] Account for the fact that sometimes we don't get HTTP_HOST (like for tests) --- common/djangoapps/student/views.py | 2 +- lms/djangoapps/courseware/courses.py | 20 +++++++++----------- lms/djangoapps/courseware/views.py | 4 ++-- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index e02a5f4562..a99b46fd13 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -94,7 +94,7 @@ def main_index(request, extra_context={}, user=None): # The course selection work is done in courseware.courses. universities = get_courses_by_university(None, - domain=request.META['HTTP_HOST']) + domain=request.META.get('HTTP_HOST')) context = {'universities': universities, 'entries': entries} context.update(extra_context) return render_to_response('index.html', context) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 5818252cf1..02144ac3e0 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -205,19 +205,18 @@ def get_courses_by_university(user, domain=None): Returns dict of lists of courses available, keyed by course.org (ie university). Courses are sorted by course.number. ''' - subdomain = domain.split(".")[0] - # TODO: Clean up how 'error' is done. # filter out any courses that errored. - if settings.MITX_FEATURES.get('SUBDOMAIN_COURSE_LISTINGS'): - if subdomain in settings.COURSE_LISTINGS: - visible_courses = settings.COURSE_LISTINGS[subdomain] - else: - visible_courses = frozenset(settings.COURSE_LISTINGS['default']) - courses = [c for c in modulestore().get_courses() if isinstance(c, CourseDescriptor)] courses = sorted(courses, key=lambda course: course.number) + + if domain and settings.MITX_FEATURES.get('SUBDOMAIN_COURSE_LISTINGS'): + subdomain = settings.COURSE_LISTINGS.get(domain.split(".")[0], 'default') + visible_courses = frozenset(settings.COURSE_LISTINGS[subdomain]) + else: + visible_courses = frozenset(c.id for c in courses) + universities = defaultdict(list) for course in courses: <<<<<<< HEAD @@ -227,9 +226,8 @@ def get_courses_by_university(user, domain=None): if settings.MITX_FEATURES.get('ACCESS_REQUIRE_STAFF_FOR_COURSE'): if not has_access_to_course(user,course): continue - if settings.MITX_FEATURES.get('SUBDOMAIN_COURSE_LISTINGS'): - if course.id not in visible_courses: - continue + if course.id not in visible_courses: + continue universities[course.org].append(course) >>>>>>> implement subdomain-based course displays return universities diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index b6a56830e0..eaeb667b3e 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -64,7 +64,7 @@ def courses(request): Render "find courses" page. The course selection work is done in courseware.courses. ''' universities = get_courses_by_university(request.user, - domain=request.META['HTTP_HOST']) + domain=request.META.get('HTTP_HOST')) return render_to_response("courses.html", {'universities': universities}) @@ -243,7 +243,7 @@ def university_profile(request, org_id): # Only grab courses for this org... courses = get_courses_by_university(request.user, - domain=request.META['HTTP_HOST'])[org_id] + domain=request.META.get('HTTP_HOST'))[org_id] context = dict(courses=courses, org_id=org_id) template_file = "university_profile/{0}.html".format(org_id).lower() From 7f8f70297179628250d72b3e4666d48f19700678 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Wed, 15 Aug 2012 11:21:15 -0400 Subject: [PATCH 09/46] Fix silly error that was pulling the wrong info from COURSE_LISTINGS --- lms/djangoapps/courseware/courses.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 02144ac3e0..3eaff318a5 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -212,7 +212,9 @@ def get_courses_by_university(user, domain=None): courses = sorted(courses, key=lambda course: course.number) if domain and settings.MITX_FEATURES.get('SUBDOMAIN_COURSE_LISTINGS'): - subdomain = settings.COURSE_LISTINGS.get(domain.split(".")[0], 'default') + subdomain = domain.split(".")[0] + if subdomain not in settings.COURSE_LISTINGS: + subdomain = 'default' visible_courses = frozenset(settings.COURSE_LISTINGS[subdomain]) else: visible_courses = frozenset(c.id for c in courses) From 7d90617c5e9fb898609334c9c67d3a2dea062792 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Wed, 15 Aug 2012 12:21:59 -0400 Subject: [PATCH 10/46] Merge with master --- lms/djangoapps/courseware/courses.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 3eaff318a5..e142614f46 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -142,9 +142,6 @@ def get_course_info_section(course, section_key): raise KeyError("Invalid about key " + str(section_key)) -<<<<<<< HEAD -def get_courses_by_university(user): -======= def course_staff_group_name(course): ''' course should be either a CourseDescriptor instance, or a string (the @@ -200,7 +197,6 @@ def has_access_to_course(user, course): def get_courses_by_university(user, domain=None): ->>>>>>> implement subdomain-based course displays ''' Returns dict of lists of courses available, keyed by course.org (ie university). Courses are sorted by course.number. @@ -221,16 +217,10 @@ def get_courses_by_university(user, domain=None): universities = defaultdict(list) for course in courses: -<<<<<<< HEAD - if has_access(user, course, 'see_exists'): - universities[course.org].append(course) -======= - if settings.MITX_FEATURES.get('ACCESS_REQUIRE_STAFF_FOR_COURSE'): - if not has_access_to_course(user,course): - continue + if not has_access(user, course, 'see_exists'): + continue if course.id not in visible_courses: continue universities[course.org].append(course) ->>>>>>> implement subdomain-based course displays return universities From 7c6b9eba1c1a6004f4d93c8b84b64702a2dd92ac Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 15 Aug 2012 12:55:13 -0400 Subject: [PATCH 11/46] Note that 'load' access does not check for enrollment --- lms/djangoapps/courseware/access.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 1494de6662..760ae55fb4 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -68,6 +68,7 @@ def has_access(user, obj, action): 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. From c4afddfd384cd858779061174e40e237a8cb01ab Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 15 Aug 2012 12:56:01 -0400 Subject: [PATCH 12/46] Fix staff access check in simplewiki --- lms/djangoapps/simplewiki/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/simplewiki/views.py b/lms/djangoapps/simplewiki/views.py index 192035fcde..ac807b13ed 100644 --- a/lms/djangoapps/simplewiki/views.py +++ b/lms/djangoapps/simplewiki/views.py @@ -51,7 +51,7 @@ def update_template_dictionary(dictionary, request=None, course=None, article=No dictionary.update(csrf(request)) if request and course: - dictionary['staff_access'] = has_access(request.user, course, 'load') + dictionary['staff_access'] = has_access(request.user, course, 'staff') else: dictionary['staff_access'] = False From bbe94325080f6c3a9c23553c6ea2892ea2221caa Mon Sep 17 00:00:00 2001 From: Kyle Fiedler Date: Tue, 14 Aug 2012 13:58:18 -0400 Subject: [PATCH 13/46] Lots of askbot fixes --- .../templates/question/question_controls.html | 3 +- lms/askbot/skins/mitx/templates/base.html | 7 +- .../templates/meta/html_head_stylesheets.html | 1 - lms/static/sass/course/base/_base.scss | 6 +- .../sass/course/discussion/_answers.scss | 4 - .../sass/course/discussion/_modals.scss | 1 - .../course/discussion/_question-view.scss | 1 - .../sass/course/discussion/_questions.scss | 75 +++++++-------- .../sass/course/discussion/_sidebar.scss | 93 +++++++++---------- lms/static/sass/course/discussion/_tags.scss | 32 +------ 10 files changed, 90 insertions(+), 133 deletions(-) diff --git a/lms/askbot/skins/common/templates/question/question_controls.html b/lms/askbot/skins/common/templates/question/question_controls.html index af30d43419..2409371c65 100644 --- a/lms/askbot/skins/common/templates/question/question_controls.html +++ b/lms/askbot/skins/common/templates/question/question_controls.html @@ -40,6 +40,5 @@ {% endif %} {% if request.user|can_delete_post(question) %}{{ pipe() }} - {% if question.deleted %}{% trans %}undelete{% endtrans %}{% else %}✖{% endif %} - + {% if question.deleted %}{% trans %}undelete{% endtrans %}{% else %}delete{% endif %} {% endif %} diff --git a/lms/askbot/skins/mitx/templates/base.html b/lms/askbot/skins/mitx/templates/base.html index a344009c60..4c5a36bd46 100644 --- a/lms/askbot/skins/mitx/templates/base.html +++ b/lms/askbot/skins/mitx/templates/base.html @@ -18,11 +18,14 @@ {% include "widgets/system_messages.html" %} {% include "debug_header.html" %} {% include "widgets/header.html" %} {# Logo, user tool navigation and meta navitation #} - {# include "widgets/secondary_header.html" #} {# Scope selector, search input and ask button #} -
    +
    + {# include "widgets/secondary_header.html" #} {# Scope selector, search input and ask button #} + +
    {% block body %} {% endblock %} +
    {% if settings.FOOTER_MODE == 'default' %} diff --git a/lms/askbot/skins/mitx/templates/meta/html_head_stylesheets.html b/lms/askbot/skins/mitx/templates/meta/html_head_stylesheets.html index 3ec11b59fd..a6a3d3cd46 100644 --- a/lms/askbot/skins/mitx/templates/meta/html_head_stylesheets.html +++ b/lms/askbot/skins/mitx/templates/meta/html_head_stylesheets.html @@ -1,4 +1,3 @@ {% load extra_filters_jinja %} - {{ 'application' | compressed_css }} {{ 'course' | compressed_css }} diff --git a/lms/static/sass/course/base/_base.scss b/lms/static/sass/course/base/_base.scss index 034e047754..90c8fdc3e2 100644 --- a/lms/static/sass/course/base/_base.scss +++ b/lms/static/sass/course/base/_base.scss @@ -2,7 +2,7 @@ body { min-width: 980px; } -body, h1, h2, h3, h4, h5, h6, p, p a:link, p a:visited, a { +body, h1, h2, h3, h4, h5, h6, p, p a:link, p a:visited, a, label { text-align: left; font-family: $sans-serif; } @@ -27,6 +27,10 @@ form { } } +img { + max-width: 100%; +} + ::selection, ::-moz-selection, ::-webkit-selection { background:#444; color:#fff; diff --git a/lms/static/sass/course/discussion/_answers.scss b/lms/static/sass/course/discussion/_answers.scss index 8ab22aa833..27043be68d 100644 --- a/lms/static/sass/course/discussion/_answers.scss +++ b/lms/static/sass/course/discussion/_answers.scss @@ -141,10 +141,6 @@ div.answer-actions { a { cursor: pointer; text-decoration: none; - - &.question-delete { - color: $mit-red; - } } } diff --git a/lms/static/sass/course/discussion/_modals.scss b/lms/static/sass/course/discussion/_modals.scss index 5a7e6db1e5..f1d1fd78cf 100644 --- a/lms/static/sass/course/discussion/_modals.scss +++ b/lms/static/sass/course/discussion/_modals.scss @@ -1,5 +1,4 @@ // Style for modal boxes that pop up to notify the user of various events - .vote-notification { background-color: darken($mit-red, 7%); @include border-radius(4px); diff --git a/lms/static/sass/course/discussion/_question-view.scss b/lms/static/sass/course/discussion/_question-view.scss index 4c2acaf9be..1bee024607 100644 --- a/lms/static/sass/course/discussion/_question-view.scss +++ b/lms/static/sass/course/discussion/_question-view.scss @@ -90,7 +90,6 @@ div.question-header { a { &.question-delete { - color: $mit-red; text-decoration: none; cursor: pointer; } diff --git a/lms/static/sass/course/discussion/_questions.scss b/lms/static/sass/course/discussion/_questions.scss index 4f855cd092..a699dc6c5e 100644 --- a/lms/static/sass/course/discussion/_questions.scss +++ b/lms/static/sass/course/discussion/_questions.scss @@ -9,6 +9,7 @@ div.question-list-header { h1 { margin: 0; + font-size: 1em; > a.light-button { float: right; @@ -49,8 +50,11 @@ div.question-list-header { nav { @extend .action-link; float: right; + font-size: em(16, 24); a { + font-size: 1em; + &.on span{ font-weight: bold; } @@ -103,26 +107,15 @@ div.question-list-header { ul.question-list, div#question-list { width: flex-grid(9,9); + padding-left: 0; + margin: 0; li.single-question { border-bottom: 1px solid #eee; list-style: none; - padding: 10px lh(); - margin-left: (-(lh())); + padding: lh() 0; width: 100%; - &:hover { - background: #F3F3F3; - - ul.tags li { - background: #ddd; - - &:before { - border-color: transparent #ddd transparent transparent; - } - } - } - &:first-child { border-top: 0; } @@ -133,14 +126,19 @@ ul.question-list, div#question-list { &.question-body { @include box-sizing(border-box); margin-right: flex-gutter(); - width: flex-grid(5.5,9); + width: flex-grid(5,9); h2 { - font-size: 16px; + font-size: em(20); font-weight: bold; letter-spacing: 0; - margin: 0px 0 15px 0; + margin: 0 0 lh() 0; text-transform: none; + line-height: lh(); + + a { + line-height: lh(); + } } p.excerpt { @@ -151,40 +149,40 @@ ul.question-list, div#question-list { div.user-info { display: inline-block; vertical-align: top; - margin-bottom: 10px; + margin: lh() 0 0 0; + line-height: lh(); span.relative-time { font-weight: normal; } - - a { - color: $mit-red; - } } ul.tags { display: inline-block; + margin: lh() 0 0 0; + padding: 0; } } &.question-meta { float: right; - margin-top: 10px; - width: flex-grid(3.5,9); - + width: flex-grid(3,9); ul { - text-align: right; + @include clearfix; + margin: 0; + padding: 0; + list-style: none; li { - border: 1px solid #ddd; + border: 1px solid lighten($border-color, 10%); + @include box-sizing(border-box); @include box-shadow(0 1px 0 #fff); - display: inline-block; height:60px; - @include linear-gradient(#fff, #f5f5f5); - margin-right: 10px; - width: 60px; + float: left; + margin-right: flex-gutter(3); + width: flex-grid(1,3); &:last-child { margin-right: 0px; @@ -196,31 +194,22 @@ ul.question-list, div#question-list { } } - &.views { - } - &.answers { &.accepted { - - @include linear-gradient(#fff, lighten( #c4dfbe, 12% )); - border-color: #c4dfbe; + border-color: lighten($border-color, 10%); span, div { color: darken(#c4dfbe, 35%); } } + &.no-answers { - - span, div { - color: lighten($mit-red, 20%); + color: $pink; } } } - &.votes { - } - span, div { @include box-sizing(border-box); color: #888; diff --git a/lms/static/sass/course/discussion/_sidebar.scss b/lms/static/sass/course/discussion/_sidebar.scss index 5ff8ce2c55..947f8eb722 100644 --- a/lms/static/sass/course/discussion/_sidebar.scss +++ b/lms/static/sass/course/discussion/_sidebar.scss @@ -2,11 +2,8 @@ div.discussion-wrapper aside { @extend .sidebar; - border-left: 1px solid #d3d3d3; - @include border-radius(0 4px 4px 0); - border-right: 1px solid #f6f6f6; - @include box-shadow(inset 1px 0 0 #f6f6f6); - padding: lh(); + border-left: 1px solid $border-color; + border-right: 0; width: flex-grid(3); &.main-sidebar { @@ -15,12 +12,16 @@ div.discussion-wrapper aside { h1 { @extend .bottom-border; - margin: (-(lh())) (-(lh())) 0; padding: lh(.5) lh(); + margin-bottom: em(16, 20); } h2 { - color: #4D4D4D; + color: #3C3C3C; + font-size: 1em; + font-style: normal; + font-weight: bold; + margin-bottom: 1em; &.first { margin-top: 0px; @@ -36,6 +37,9 @@ div.discussion-wrapper aside { input[type="submit"] { width: 27%; float: right; + text-align: center; + padding-left: 0; + padding-right: 0; } input[type="text"] { @@ -45,24 +49,24 @@ div.discussion-wrapper aside { div.box { display: block; - margin: lh(.5) 0; + padding: lh(.5) lh(); + border-top: 1px solid lighten($border-color, 10%); - &:last-child { - @include box-shadow(none); - border: 0; + &:first-child { + border-top: 0; } - h2 { - text-transform: uppercase; - font-weight: bold; - font-size: 14px; - letter-spacing: 1px; + ul#related-tags { + position: relative; + left: -10px; - &:not(.first) { - @include box-shadow(inset 0 1px 0 #eee); - border-top: 1px solid #d3d3d3; - margin: 0 (-(lh())) 0; - padding: lh(.5) lh(); + li { + border-bottom: 0; + background: #eee; + + a:hover { + background: transparent; + } } } @@ -85,9 +89,6 @@ div.discussion-wrapper aside { } } - img.gravatar { - @include border-radius(3px); - } } &.tag-selector { @@ -100,17 +101,18 @@ div.discussion-wrapper aside { div.search-box { margin-top: lh(.5); + input { @include box-sizing(border-box); display: inline; } input[type='submit'] { - @include box-shadow(none); - opacity: 0.5; background: url(../images/askbot/search-icon.png) no-repeat center; border: 0; + @include box-shadow(none); margin-left: 3px; + opacity: 0.5; position: absolute; text-indent: -9999px; width: 24px; @@ -144,17 +146,6 @@ div.discussion-wrapper aside { } div#tagSelector { - h2 { - @include box-shadow(inset 0 1px 0 #eee); - border-top: 1px solid #d3d3d3; - margin: 0 (-(lh())) 0; - padding: lh(.5) lh(); - text-transform: uppercase; - font-weight: bold; - font-size: 14px; - letter-spacing: 1px; - } - ul { margin: 0; } @@ -167,11 +158,17 @@ div.discussion-wrapper aside { p.choice { @include inline-block(); margin-right: lh(.5); + margin-top: 0; } } + + label { + font-style: normal; + font-weight: 400; + } } - // Question view sopecific + // Question view specific div.follow-buttons { margin-top: 20px; @@ -187,12 +184,15 @@ div.discussion-wrapper aside { div.question-stats { + border-top: 0; + ul { color: #777; list-style: none; li { padding: 7px 0 0; + border: 0; &:last-child { @include box-shadow(none); @@ -273,6 +273,7 @@ div.discussion-wrapper aside { margin-left: 8%; } } + div.markdown ul li { margin: 20px 0; @@ -286,19 +287,15 @@ div.discussion-wrapper aside { } div.view-profile { - h2 { - border-top: 0; - @include box-shadow(none); - } + border-top: 0; a { - width: 100%; - @include box-sizing(border-box); - text-align: center; - padding: 10px; - display: block; - margin-top: 10px; @extend .light-button; + @include box-sizing(border-box); + display: block; + text-align: center; + width: 100%; + margin-top: lh(.5); &:first-child { margin-top: 0; diff --git a/lms/static/sass/course/discussion/_tags.scss b/lms/static/sass/course/discussion/_tags.scss index a8d4d0f034..fd05ed0df0 100644 --- a/lms/static/sass/course/discussion/_tags.scss +++ b/lms/static/sass/course/discussion/_tags.scss @@ -10,19 +10,17 @@ ul.tags { li { background: #eee; - @include border-radius(4px); - @include box-shadow(0px 1px 0px #ccc); color: #555; display: inline-block; font-size: 12px; margin-bottom: 5px; margin-left: 15px; - padding: 3px 10px 5px 5px; + padding: 6px 10px 6px 5px; &:before { border-color:transparent #eee transparent transparent; border-style:solid; - border-width:12px 12px 12px 0; + border-width:12px 10px 12px 0; content:""; height:0; left:-10px; @@ -31,25 +29,6 @@ ul.tags { width:0; } - span.delete-icon, div.delete-icon { - background: #555; - @include border-radius(0 4px 4px 0); - clear: none; - color: #eee; - cursor: pointer; - display: inline; - float: none; - left: 10px; - opacity: 0.5; - padding: 4px 6px; - position: relative; - top: 1px; - - &:hover { - opacity: 1; - } - } - a { color: #555; text-decoration: none; @@ -61,11 +40,4 @@ ul.tags { span.tag-number { display: none; - // @include border-radius(3px); - // background: #555; - // font-size: 10px; - // margin: 0 3px; - // padding: 2px 5px; - // color: #eee; - // opacity: 0.5; } From 0804ff7a75908a50f4414333d243067763137868 Mon Sep 17 00:00:00 2001 From: Kyle Fiedler Date: Tue, 14 Aug 2012 15:33:55 -0400 Subject: [PATCH 14/46] Added styles for single question and profile view --- .../templates/question/answer_controls.html | 2 +- lms/static/images/askbot/comment-vote-up.png | Bin 191 -> 0 bytes .../{ => old}/vote-arrow-down-activate.png | Bin .../images/askbot/old/vote-arrow-down.png | Bin 0 -> 216 bytes .../{ => old}/vote-arrow-up-activate.png | Bin .../images/askbot/old/vote-arrow-up.png | Bin 0 -> 200 bytes lms/static/images/askbot/vote-arrow-down.png | Bin 216 -> 1155 bytes lms/static/images/askbot/vote-arrow-up.png | Bin 200 -> 1138 bytes .../sass/course/discussion/_profile.scss | 28 +++------ .../course/discussion/_question-view.scss | 57 +++++++++--------- .../sass/course/discussion/_questions.scss | 1 + .../sass/course/discussion/_sidebar.scss | 17 +++--- lms/static/sass/course/discussion/_tags.scss | 1 + 13 files changed, 51 insertions(+), 55 deletions(-) delete mode 100644 lms/static/images/askbot/comment-vote-up.png rename lms/static/images/askbot/{ => old}/vote-arrow-down-activate.png (100%) create mode 100644 lms/static/images/askbot/old/vote-arrow-down.png rename lms/static/images/askbot/{ => old}/vote-arrow-up-activate.png (100%) create mode 100644 lms/static/images/askbot/old/vote-arrow-up.png diff --git a/lms/askbot/skins/common/templates/question/answer_controls.html b/lms/askbot/skins/common/templates/question/answer_controls.html index 52c4836e1e..f1896e0d95 100644 --- a/lms/askbot/skins/common/templates/question/answer_controls.html +++ b/lms/askbot/skins/common/templates/question/answer_controls.html @@ -31,7 +31,7 @@ {% spaceless %} - {% if answer.deleted %}{% trans %}undelete{% endtrans %}{% else %}✖{% endif %} + {% if answer.deleted %}{% trans %}undelete{% endtrans %}{% else %}delete{% endif %} {% endspaceless %} {% endif %} diff --git a/lms/static/images/askbot/comment-vote-up.png b/lms/static/images/askbot/comment-vote-up.png deleted file mode 100644 index 05dc84e12e306a2cedc65c3835669bc4199a1fb6..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 191 zcmeAS@N?(olHy`uVBq!ia0vp^0wB!61|;P_|4#%`ZJsWUAr-gYUQ*<14&Z5cXg{NJ zUBeAewFxnd*M7-1H5at*OidDU-NDJUcWt$b>xLHbX^}_Ne=qm#n)x@C--YRkOu#x0 zUD22C85cSlmNQQ}%Q(e5f#ZFr`zv3@b8`gdeSRQQ^uwN67#H diff --git a/lms/static/images/askbot/vote-arrow-down-activate.png b/lms/static/images/askbot/old/vote-arrow-down-activate.png similarity index 100% rename from lms/static/images/askbot/vote-arrow-down-activate.png rename to lms/static/images/askbot/old/vote-arrow-down-activate.png diff --git a/lms/static/images/askbot/old/vote-arrow-down.png b/lms/static/images/askbot/old/vote-arrow-down.png new file mode 100644 index 0000000000000000000000000000000000000000..e67524077a4650997d321967e4f0b62833ecad6a GIT binary patch literal 216 zcmeAS@N?(olHy`uVBq!ia0vp^!a&T!!3HE7F0?fPsrjBRjv*DdmYy->YBrEKTBxYN zF_$ZrJ2tkubIw8LFLLt^hQ0`}X_)+gmn+iyxtv1q@+N`#52Bee&piJSFSjTD*__k& zo?TSjzPPNgL-v9Kn@Em?fFo|KA{Et9+p$gh#o; z&5WV;!o~jr+>KJca$lUUA83ydSY!KU+nXStg@vu}?x;!}6n+(`c;Lxdqkr2zFyA*a Q2RfC()78&qol`;+09svAV*mgE literal 0 HcmV?d00001 diff --git a/lms/static/images/askbot/vote-arrow-up-activate.png b/lms/static/images/askbot/old/vote-arrow-up-activate.png similarity index 100% rename from lms/static/images/askbot/vote-arrow-up-activate.png rename to lms/static/images/askbot/old/vote-arrow-up-activate.png diff --git a/lms/static/images/askbot/old/vote-arrow-up.png b/lms/static/images/askbot/old/vote-arrow-up.png new file mode 100644 index 0000000000000000000000000000000000000000..a35946cc5100ad2fcf91e7f5aea14eb79633864f GIT binary patch literal 200 zcmV;(05|`MP)`OK?rd?H-2_k7H;8p0jHme>iJDc8m_3{GrBzBHEL4Z+pnV2uU1K^_l;mRJcH zRKH?5M#!l67E94V3-ma;u@Eh^!V~d4c7`^%h|UBw%z;zdsUGeC0000O>EOv95*dWp&}}*Orwm$lgp|hEphBLPGcJCGpf-2nS2RpfoxAno6rNJv6I9EH!T=NX44beiYIwx6p;|>4Orx@Hrs>D$7ipkt zY5HJ7i79p#RrCY%4w{(H7uETyn$qZzePGxVh=74G1fDTtx`LOcS9Jxl56lb=Rw1~W zre6d#rHq5Dd#&S%I+j#-!Eq6Sh`4hmhF-*U`&$YUa#csSv2K|lP=wRg zEKbuT(ibTh_PVU;u9b-tjPan&u+dnMQVXakuZ9}NI@-kr^fKQ66?ThrHew3MwPqca zG_KSixUz+;gCMq?qGiprsyJS;u;o@P8)PSV@U{X~-3$zSRvC&SIIhK{#%urI6<0*D5%O#~)D&FEsmO5)76Suh9OYTrxZZHgnO*~7; z(ceefm}41WHEBU_A4{UG-WpeH9}C}>%aCB0ptb*Lb<0HNC)lnJE;+1^J~GL?J7lmO z{n~T#Q9R2@nWA@Z`uWPrNas1~YE!*@wzQ-7bgApXR_?DisV4t3{7t;@TBBS)^6vMU zZ^VU7jl(y>hr4_z?&|Wts(ZqPt-rV^341eVq`{B06`MS6B zwhXSGgz+sK^HlS#tuFQF32;01^@s6fY6450001=Nkli=EADSr z1<%~X^wgl##FWaylc_cg49qH-ArU1JzCKpT`MG+DAT@dwxdlMo3=B5*6$OdO*{LN8 zNvY|XdA3ULckfqH$V{%1*XSQL?vFu&J;D8jzb> zlBiITo0C^;Rbi_HHrEQs1_|pcDS(xfWZNo192Makpx~Tel&WB=XRMoSU}&gdW~OIo zVrph)sH0$HU}&Uo07PcGh9*{~W>!Y#3Q(W~w5=#5%__*n4QdyVXRDM^Qc_^0uU}qX zu2*iXmtT~wZ)j<02{OaTNEfI=x41H|B(Xv_uUHvof=g;~a#3bMNoIbY0?5R~r2Ntn zTP2`NAzsKWfE$}v3=Jk=fazBx7U&!58GyV5Q|Rl9UukYGTy=3tP%6T`SPd=?sVqp< z4@xc0FD*(2MqHXQ$f^P>=c3falKi5O{QMkPC z!8&|>tvvIJOA_;vQ$1a5m4IgGWoD*W89TW;8@rksxVV~`7#g}7I$D^!x*A&=x;Yz} zyBJs+!}Pl3Czs}?=9R$orXchh;?xUD47mkBn_W_iGRsm^+=}vZ6~Lah%EaOpS3?UY zb5jdTpm|2v-2%~@g2gRRy^c8b>H{644~kl(sD=pv(+`LVPq;u1Jn5(A0n>XCFk!p* z?<@soiYJ~fjv*DdlK%YvZ_m7y;q=Dj<15l6gqcMd6%Si9{xA|L4osRMA|TI}uE zBC34TfLXzg^9Yke4%-p6fN!2FWG=S-bd3GN(4pJ#LrUO&!zOu$r++#P*NAjnXa2~h zP{#Pk+@mOeL6XAG(txciJej>VGkr8s@MC+FrSOg+DTniey1;R#*xQ^(vK;2H9@#&U zH=6CpJqH=KBiS7utZyA^q&vhJHwp-_D@%uQABlCSnbnfQy6KCeK)u`dXS_z2HM^en zTsbql&xY delta 185 zcmV;q07n1v2*?4D7k>;01^@s6fY6450001wNkl`OK?rd?H-2_k7H; z8p0jHme>iJDc8m_3{GrBzBHEL4Z+pnV2uU1K^_l;mRJcHR5HI}IY!8+_!djiK@0Ra ny0H)~w89hdJa&dQxQNaKG|Yih+NmDy00000NkvXXu0mjfQ1(zD diff --git a/lms/static/sass/course/discussion/_profile.scss b/lms/static/sass/course/discussion/_profile.scss index 010a03ffd6..f20b51b72b 100644 --- a/lms/static/sass/course/discussion/_profile.scss +++ b/lms/static/sass/course/discussion/_profile.scss @@ -9,9 +9,9 @@ body.user-profile-page { } ul.sub-info { - // border-top: 1px solid #ddd; margin-top: lh(); list-style: none; + padding: 0; > li { display: table-cell; @@ -57,6 +57,7 @@ body.user-profile-page { ul { list-style: none; + padding: 0; &.user-stats-table { list-style: none; @@ -72,37 +73,28 @@ body.user-profile-page { margin-bottom: 30px; li { - background-position: 10px center; + background-position: 10px -10px; background-repeat: no-repeat; - @include border-radius(4px); display: inline-block; - height: 20px; - padding: 10px 10px 10px 40px; + padding: 2px 10px 2px 40px; + margin-bottom: lh(.5); + border: 1px solid lighten($border-color, 10%); &.up { - background-color:#d1e3a8; - background-image: url(../images/askbot/vote-arrow-up-activate.png); + background-image: url(../images/askbot/vote-arrow-up.png); margin-right: 6px; - - span.vote-count { - color: #3f6c3e; - } } &.down { - background-image: url(../images/askbot/vote-arrow-down-activate.png); - background-color:#eac6ad; - - span.vote-count { - color: $mit-red; - } - + background-image: url(../images/askbot/vote-arrow-down.png); } } } &.badges { @include inline-block(); + padding: 0; + margin: 0; a { background-color: #e3e3e3; diff --git a/lms/static/sass/course/discussion/_question-view.scss b/lms/static/sass/course/discussion/_question-view.scss index 1bee024607..b369ceabe0 100644 --- a/lms/static/sass/course/discussion/_question-view.scss +++ b/lms/static/sass/course/discussion/_question-view.scss @@ -1,15 +1,16 @@ // Styles for the single question view div.question-header { + @include clearfix(); div.official-stamp { background: $mit-red; color: #fff; font-size: 12px; + margin-left: -1px; margin-top: 10px; padding: 2px 5px; text-align: center; - margin-left: -1px; } div.vote-buttons { @@ -19,40 +20,40 @@ div.question-header { width: flex-grid(0.7,9); ul { - li { - background-position: center; - background-repeat: no-repeat; - cursor: pointer; - font-weight: bold; - height: 20px; - list-style: none; - padding: 10px; - text-align: center; - width: 70%; + padding: 0; + margin: 0; - &.post-vote { - @include border-radius(4px); - @include box-shadow(inset 0 1px 0px #fff); - } + li { + background-repeat: no-repeat; + color: #999; + font-size: em(20); + font-weight: bold; + list-style: none; + text-align: center; &.question-img-upvote, &.answer-img-upvote { background-image: url(../images/askbot/vote-arrow-up.png); - @include box-shadow(inset 0 1px 0px rgba(255, 255, 255, 0.5)); + background-position: center 0; + cursor: pointer; + height: 12px; + margin-bottom: lh(.5); &:hover, &.on { - background-color:#d1e3a8; - border-color: darken(#D1E3A8, 20%); - background-image: url(../images/askbot/vote-arrow-up-activate.png); + background-image: url(../images/askbot/vote-arrow-up.png); + background-position: center -22px; } } &.question-img-downvote, &.answer-img-downvote { + cursor: pointer; background-image: url(../images/askbot/vote-arrow-down.png); + background-position: center 0; + height: 12px; + margin-top: lh(.5); &:hover, &.on { - background-color:#EAC6AD; - border-color: darken(#EAC6AD, 20%); - background-image: url(../images/askbot/vote-arrow-down-activate.png); + background-image: url(../images/askbot/vote-arrow-down.png); + background-position: center -22px; } } } @@ -66,6 +67,11 @@ div.question-header { h1 { margin-top: 0; + font-weight: 100; + + a { + font-weight: 100; + } } div.meta-bar { @@ -89,10 +95,8 @@ div.question-header { width: flex-grid(4,8); a { - &.question-delete { - text-decoration: none; - cursor: pointer; - } + @extend a:link; + cursor: pointer; } span.sep { @@ -333,7 +337,6 @@ div.question-header { } div.controls { - border-top: 1px solid #efefef; text-align: right; a { diff --git a/lms/static/sass/course/discussion/_questions.scss b/lms/static/sass/course/discussion/_questions.scss index a699dc6c5e..9b7335e62b 100644 --- a/lms/static/sass/course/discussion/_questions.scss +++ b/lms/static/sass/course/discussion/_questions.scss @@ -10,6 +10,7 @@ div.question-list-header { h1 { margin: 0; font-size: 1em; + font-weight: 100; > a.light-button { float: right; diff --git a/lms/static/sass/course/discussion/_sidebar.scss b/lms/static/sass/course/discussion/_sidebar.scss index 947f8eb722..cac219b8cf 100644 --- a/lms/static/sass/course/discussion/_sidebar.scss +++ b/lms/static/sass/course/discussion/_sidebar.scss @@ -216,19 +216,20 @@ div.discussion-wrapper aside { } div.karma { - background: #eee; - border: 1px solid #D3D3D3; - @include border-radius(3px); + border: 1px solid $border-color; @include box-sizing(border-box); - @include box-shadow(inset 0 0 0 1px #fff, 0 1px 0 #fff); padding: lh(.4) 0; text-align: center; width: flex-grid(1, 3); float: right; - strong { - display: block; - font-style: 20px; + p { + text-align: center; + + strong { + display: block; + font-style: 20px; + } } } @@ -255,8 +256,6 @@ div.discussion-wrapper aside { overflow: visible; ul { - font-size: 14px; - h2 { margin:0 (-(lh())) 5px (-(lh())); padding: lh(.5) lh(); diff --git a/lms/static/sass/course/discussion/_tags.scss b/lms/static/sass/course/discussion/_tags.scss index fd05ed0df0..5cf78a4816 100644 --- a/lms/static/sass/course/discussion/_tags.scss +++ b/lms/static/sass/course/discussion/_tags.scss @@ -3,6 +3,7 @@ ul.tags { list-style: none; display: inline; + padding: 0; li, a { position: relative; From ff1f090737d1e6377bd5ced1ac4e6b71886022a8 Mon Sep 17 00:00:00 2001 From: Kyle Fiedler Date: Wed, 15 Aug 2012 12:48:14 -0400 Subject: [PATCH 15/46] Added more fixes for askbot also fixed some of the extra padding --- lms/static/sass/course/_textbook.scss | 4 +-- lms/static/sass/course/base/_extends.scss | 7 +++-- .../sass/course/courseware/_courseware.scss | 3 +- .../sass/course/discussion/_answers.scss | 18 ++++++++--- .../sass/course/discussion/_badges.scss | 21 ++++++++----- .../sass/course/discussion/_discussion.scss | 2 +- lms/static/sass/course/discussion/_forms.scss | 11 ++++++- .../course/discussion/_question-view.scss | 30 ++++++++++--------- .../sass/course/discussion/_questions.scss | 19 +++++++----- .../sass/course/discussion/_sidebar.scss | 22 +++++--------- lms/static/sass/shared/_forms.scss | 1 + 11 files changed, 83 insertions(+), 55 deletions(-) diff --git a/lms/static/sass/course/_textbook.scss b/lms/static/sass/course/_textbook.scss index 8e88f8befd..8bbfa67b1c 100644 --- a/lms/static/sass/course/_textbook.scss +++ b/lms/static/sass/course/_textbook.scss @@ -10,7 +10,6 @@ div.book-wrapper { font-size: em(14); .chapter-number { - } .chapter { @@ -81,9 +80,8 @@ div.book-wrapper { section.book { @extend .content; - padding-bottom: 0; padding-right: 0; - padding-top: 0; + padding-left: lh(); nav { @extend .clearfix; diff --git a/lms/static/sass/course/base/_extends.scss b/lms/static/sass/course/base/_extends.scss index 04eaf73094..a438b93150 100644 --- a/lms/static/sass/course/base/_extends.scss +++ b/lms/static/sass/course/base/_extends.scss @@ -12,10 +12,13 @@ h1.top-header { @include box-shadow(inset 0 1px 0 #fff); color: #666; cursor: pointer; - font: normal $body-font-size $body-font-family; + font: 400 $body-font-size $body-font-family; @include linear-gradient(#fff, lighten(#888, 40%)); padding: 4px 8px; text-decoration: none; + text-shadow: none; + text-transform: none; + letter-spacing: 0; -webkit-font-smoothing: antialiased; &:hover, &:focus { @@ -28,7 +31,7 @@ h1.top-header { .content { @include box-sizing(border-box); display: table-cell; - padding: lh(); + padding-right: lh(); vertical-align: top; width: flex-grid(9) + flex-gutter(); diff --git a/lms/static/sass/course/courseware/_courseware.scss b/lms/static/sass/course/courseware/_courseware.scss index 198902c146..24bea4dd84 100644 --- a/lms/static/sass/course/courseware/_courseware.scss +++ b/lms/static/sass/course/courseware/_courseware.scss @@ -12,7 +12,8 @@ div.course-wrapper { section.course-content { @extend .content; - @include border-radius(0 4px 4px 0); + padding-right: 0; + padding-left: lh(); h1 { margin: 0 0 lh(); diff --git a/lms/static/sass/course/discussion/_answers.scss b/lms/static/sass/course/discussion/_answers.scss index 27043be68d..73660dd336 100644 --- a/lms/static/sass/course/discussion/_answers.scss +++ b/lms/static/sass/course/discussion/_answers.scss @@ -7,9 +7,16 @@ div.answer-controls { padding-left: flex-grid(1.1); width: 100%; + div.answer-count { display: inline-block; float: left; + + h1 { + margin-bottom: 0; + font-size: em(24); + font-weight: 100; + } } div.answer-sort { @@ -18,7 +25,7 @@ div.answer-controls { nav { float: right; - margin-top: 34px; + margin-top: 10px; a { &.on span{ @@ -44,8 +51,9 @@ div.answer-block { width: 100%; img.answer-img-accept { - margin: 10px 0px 10px 16px; + margin: 10px 0px 10px 11px; } + div.answer-container { @extend div.question-container; @@ -130,17 +138,19 @@ div.answer-own { div.answer-actions { margin: 0; - padding:8px 8px 8px 0; + padding:8px 0 8px 8px; text-align: right; border-top: 1px solid #efefef; span.sep { - color: #EDDFAA; + color: $border-color; } a { cursor: pointer; text-decoration: none; + @extend a:link; + font-size: em(14); } } diff --git a/lms/static/sass/course/discussion/_badges.scss b/lms/static/sass/course/discussion/_badges.scss index d74dd93d13..65d8cbf513 100644 --- a/lms/static/sass/course/discussion/_badges.scss +++ b/lms/static/sass/course/discussion/_badges.scss @@ -22,6 +22,8 @@ div#award-list{ } ul.badge-list { + padding-left: 0; + li.badge { border-bottom: 1px solid #eee; @extend .clearfix; @@ -70,12 +72,17 @@ ul.badge-list { .bronze, .badge3 { color: #cc9933; } -div.badge-desc { - > div { - margin-bottom: 20px; - span { - font-size: 18px; - @include border-radius(10px); - } + +div.discussion-wrapper aside { + div.badge-desc { + border-top: 0; + + > div { + margin-bottom: 20px; + span { + font-size: 18px; + @include border-radius(10px); + } + } } } diff --git a/lms/static/sass/course/discussion/_discussion.scss b/lms/static/sass/course/discussion/_discussion.scss index 7b0aa601d9..e12f308ae2 100644 --- a/lms/static/sass/course/discussion/_discussion.scss +++ b/lms/static/sass/course/discussion/_discussion.scss @@ -8,7 +8,7 @@ body.askbot { @include box-sizing(border-box); display: table-cell; min-width: 650px; - padding: lh(); + padding-right: lh(); vertical-align: top; width: flex-grid(9) + flex-gutter(); diff --git a/lms/static/sass/course/discussion/_forms.scss b/lms/static/sass/course/discussion/_forms.scss index ae02ab3b20..43ba07df19 100644 --- a/lms/static/sass/course/discussion/_forms.scss +++ b/lms/static/sass/course/discussion/_forms.scss @@ -5,6 +5,11 @@ form.answer-form { border-top: 1px solid #ddd; overflow: hidden; padding-left: flex-grid(1.1); + padding-top: lh(); + + p { + margin-bottom: lh(); + } textarea { @include box-sizing(border-box); @@ -121,7 +126,6 @@ form.question-form { border: none; padding: 15px 0 0 0; - input[type="text"] { @include box-sizing(border-box); width: flex-grid(6); @@ -131,6 +135,11 @@ form.question-form { margin-top: 10px; } + input[value="Cancel"] { + @extend .light-button; + float: right; + } + div#question-list { background-color: rgba(255,255,255,0.95); @include box-sizing(border-box); diff --git a/lms/static/sass/course/discussion/_question-view.scss b/lms/static/sass/course/discussion/_question-view.scss index b369ceabe0..f7657dbf97 100644 --- a/lms/static/sass/course/discussion/_question-view.scss +++ b/lms/static/sass/course/discussion/_question-view.scss @@ -68,16 +68,18 @@ div.question-header { h1 { margin-top: 0; font-weight: 100; + line-height: 1.1em; a { font-weight: 100; + line-height: 1.1em; } } div.meta-bar { border-bottom: 1px solid #eee; display: block; - margin: 10px 0; + margin: lh(.5) 0 lh(); overflow: hidden; padding: 5px 0 10px; @@ -158,7 +160,7 @@ div.question-header { } div.change-date { - font-size: 12px; + font-size: em(14); margin-bottom: 2px; } @@ -182,13 +184,13 @@ div.question-header { display: inline-block; padding: 0 0 3% 0; width: 100%; + margin-top: lh(2); div.comments-content { - font-size: 13px; - background: #efefef; + border-top: 1px solid lighten($border-color, 10%); .block { - border-top: 1px solid #ddd; + border-top: 1px solid lighten($border-color, 10%); padding: 15px; display: block; @@ -200,10 +202,10 @@ div.question-header { padding-top: 10px; span.official-comment { - background: $mit-red; + background: $pink; color: #fff; display: block; - font-size: 12px; + font-size: em(12); margin: 0 0 10px -5%; padding:2px 5px 2px 5%; text-align: left; @@ -215,6 +217,10 @@ div.question-header { form.post-comments { padding: 15px; + button { + color: #fff; + } + button:last-child { margin-left: 10px; @extend .light-button; @@ -235,7 +241,6 @@ div.question-header { border: none; @include box-shadow(none); display: inline-block; - margin-top: -8px; padding:0 2% 0 0; text-align: center; width: 5%; @@ -281,16 +286,14 @@ div.question-header { } div.comment-delete { - // display: inline; - color: $mit-red; + @extend a:link; cursor: pointer; - font-size: 15px; } div.comment-edit { @include transform(rotate(50deg)); cursor: pointer; - font-size: 16px; + a.edit-icon { color: #555; text-decoration: none; @@ -308,13 +311,13 @@ div.question-header { div.comment-meta { text-align: right; + margin-top: lh(.5); a.author { font-weight: bold; } a.edit { - font-size: 12px; padding: 2px 10px; } } @@ -341,7 +344,6 @@ div.question-header { a { display: inline-block; - font-size: 12px; margin: 10px 10px 10px 0; } } diff --git a/lms/static/sass/course/discussion/_questions.scss b/lms/static/sass/course/discussion/_questions.scss index 9b7335e62b..1b77231bba 100644 --- a/lms/static/sass/course/discussion/_questions.scss +++ b/lms/static/sass/course/discussion/_questions.scss @@ -1,19 +1,24 @@ // Styles for the default question list view div.question-list-header { + @extend h1.top-header; display: block; margin-bottom: 0px; + padding-bottom: lh(.5); overflow: hidden; width: flex-grid(9,9); - @extend h1.top-header; h1 { margin: 0; font-size: 1em; font-weight: 100; + padding-bottom: lh(.5); > a.light-button { float: right; + font-size: em(14, 24); + letter-spacing: 0; + font-weight: 400; } } @@ -87,6 +92,7 @@ div.question-list-header { a { color: #555; + font-size: em(14, 24); } } @@ -95,12 +101,10 @@ div.question-list-header { } ul.tags { - li { - background: #fff; - - &:before { - border-color: transparent #fff transparent transparent; - } + span, div { + line-height: 1em; + margin-left: 6px; + cursor: pointer; } } } @@ -155,6 +159,7 @@ ul.question-list, div#question-list { span.relative-time { font-weight: normal; + line-height: lh(); } } diff --git a/lms/static/sass/course/discussion/_sidebar.scss b/lms/static/sass/course/discussion/_sidebar.scss index cac219b8cf..2418792b4f 100644 --- a/lms/static/sass/course/discussion/_sidebar.scss +++ b/lms/static/sass/course/discussion/_sidebar.scss @@ -264,24 +264,16 @@ div.discussion-wrapper aside { } div.question-tips, div.markdown { - ul { - margin-left: 8%; - } - + ul, ol { - margin-left: 8%; - } - } - - div.markdown ul li { - margin: 20px 0; - - &:first-child { margin: 0; - } + padding: 0; - ol li { - margin: 0; + li { + border-bottom: 0; + line-height: lh(); + margin-bottom: em(8); + } } } diff --git a/lms/static/sass/shared/_forms.scss b/lms/static/sass/shared/_forms.scss index 842ffb0086..d6a5f482e3 100644 --- a/lms/static/sass/shared/_forms.scss +++ b/lms/static/sass/shared/_forms.scss @@ -42,6 +42,7 @@ textarea { input[type="submit"], input[type="button"], +button, .button { @include border-radius(3px); @include button(shiny, $blue); From 6d7225bebb9396f4fbcb351704a1cd6a1f9e96d7 Mon Sep 17 00:00:00 2001 From: Kyle Fiedler Date: Wed, 15 Aug 2012 13:50:37 -0400 Subject: [PATCH 16/46] Added some final touches to askbot --- .../sass/course/discussion/_sidebar.scss | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/lms/static/sass/course/discussion/_sidebar.scss b/lms/static/sass/course/discussion/_sidebar.scss index 2418792b4f..7ee435f8af 100644 --- a/lms/static/sass/course/discussion/_sidebar.scss +++ b/lms/static/sass/course/discussion/_sidebar.scss @@ -63,9 +63,15 @@ div.discussion-wrapper aside { li { border-bottom: 0; background: #eee; + padding: 6px 10px 6px 5px; - a:hover { - background: transparent; + a { + padding: 0; + line-height: 12px; + + &:hover { + background: transparent; + } } } } @@ -113,6 +119,7 @@ div.discussion-wrapper aside { @include box-shadow(none); margin-left: 3px; opacity: 0.5; + padding: 6px 0 0; position: absolute; text-indent: -9999px; width: 24px; @@ -133,15 +140,22 @@ div.discussion-wrapper aside { } input#clear { - @include box-shadow(none); - @include border-radius(15px); + background: none; border: none; - background: #bbb; - color: #fff; + @include border-radius(0); + @include box-shadow(none); + color: #999; display: inline; - font-size: 10px; - margin-left: -25px; + font-size: 12px; + font-weight: bold; + height: 19px; + line-height: 1em; + margin: { + left: -25px; + top: 8px; + } padding: 2px 5px; + text-shadow: none; } } From 573c58d02b6cb5894624a0298564efef1742de08 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 15 Aug 2012 13:59:25 -0400 Subject: [PATCH 17/46] Fix broken docstring --- lms/djangoapps/courseware/access.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 760ae55fb4..e588f807da 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -84,10 +84,11 @@ def _has_access_course_desc(user, course, action): 'staff' -- staff access to course. """ def can_load(): - "Can this user load this course? + """ + 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) From e524f3bf6c930c2af7a980aa5ecbe9ca40a716e2 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 15 Aug 2012 14:12:32 -0400 Subject: [PATCH 18/46] Point at the new urls for the sandbox queue server LB --- lms/envs/dev.py | 2 +- lms/envs/test.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/envs/dev.py b/lms/envs/dev.py index 882a82b8f0..8457b50374 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -54,7 +54,7 @@ CACHES = { } XQUEUE_INTERFACE = { - "url": "http://xqueue.sandbox.edx.org", + "url": "http://sandbox-xqueue.edx.org", "django_auth": { "username": "lms", "password": "***REMOVED***" diff --git a/lms/envs/test.py b/lms/envs/test.py index 187cb5c68e..11534b3f4d 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -51,7 +51,7 @@ GITHUB_REPO_ROOT = ENV_ROOT / "data" XQUEUE_INTERFACE = { - "url": "http://xqueue.sandbox.edx.org", + "url": "http://sandbox-xqueue.edx.org", "django_auth": { "username": "lms", "password": "***REMOVED***" From 0b4b8ecfdc91be3e2cee9eac801cee08b45af08b Mon Sep 17 00:00:00 2001 From: kimth Date: Wed, 15 Aug 2012 14:20:35 -0400 Subject: [PATCH 19/46] courseware.get_module passes down course_id triplet (org, course, run) --- lms/djangoapps/courseware/module_render.py | 24 ++++++++++++++-------- lms/djangoapps/courseware/views.py | 23 +++++---------------- lms/urls.py | 1 - 3 files changed, 21 insertions(+), 27 deletions(-) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 29b73dda96..33f131e576 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -48,7 +48,7 @@ def make_track_function(request): return f -def toc_for_course(user, request, course, active_chapter, active_section): +def toc_for_course(user, request, course, active_chapter, active_section, course_id=None): ''' Create a table of contents from the module store @@ -71,7 +71,7 @@ def toc_for_course(user, request, course, active_chapter, active_section): ''' student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(user, course, depth=2) - course = get_module(user, request, course.location, student_module_cache) + course = get_module(user, request, course.location, student_module_cache, course_id=course_id) chapters = list() for chapter in course.get_display_items(): @@ -127,7 +127,7 @@ def get_section(course_module, chapter, section): return section_module -def get_module(user, request, location, student_module_cache, position=None): +def get_module(user, request, location, student_module_cache, position=None, course_id=None): ''' Get an instance of the xmodule class identified by location, setting the state based on an existing StudentModule, or creating one if none exists. @@ -144,6 +144,14 @@ def get_module(user, request, location, student_module_cache, position=None): ''' descriptor = modulestore().get_item(location) + + # NOTE: + # A 'course_id' is understood to be the triplet (org, course, run), for example + # (MITx, 6.002x, 2012_Spring). + # At the moment generic XModule does not contain enough information to replicate + # the triplet (it is missing 'run'), so we must pass down course_id + if course_id is None: + course_id = descriptor.location.course_id # Will NOT produce (org, course, run) for non-CourseModule's # Short circuit--if the user shouldn't have access, bail without doing any work if not has_access(user, descriptor, 'load'): @@ -167,7 +175,7 @@ def get_module(user, request, location, student_module_cache, position=None): # Setup system context for module instance ajax_url = reverse('modx_dispatch', - kwargs=dict(course_id=descriptor.location.course_id, + kwargs=dict(course_id=course_id, id=descriptor.location.url(), dispatch=''), ) @@ -175,7 +183,7 @@ def get_module(user, request, location, student_module_cache, position=None): # Fully qualified callback URL for external queueing system xqueue_callback_url = request.build_absolute_uri('/')[:-1] # Trailing slash provided by reverse xqueue_callback_url += reverse('xqueue_callback', - kwargs=dict(course_id=descriptor.location.course_id, + kwargs=dict(course_id=course_id, userid=str(user.id), id=descriptor.location.url(), dispatch='score_update'), @@ -195,7 +203,7 @@ def get_module(user, request, location, student_module_cache, position=None): Delegate to get_module. It does an access check, so may return None """ return get_module(user, request, location, - student_module_cache, position) + student_module_cache, position, course_id=course_id) # TODO (cpennington): When modules are shared between courses, the static # prefix is going to have to be specific to the module, not the directory @@ -309,7 +317,7 @@ def xqueue_callback(request, course_id, userid, id, dispatch): student_module_cache = StudentModuleCache.cache_for_descriptor_descendents( user, modulestore().get_item(id), depth=0, acquire_lock=True) - instance = get_module(user, request, id, student_module_cache) + instance = get_module(user, request, id, student_module_cache, course_id=course_id) if instance is None: log.debug("No module {} for user {}--access denied?".format(id, user)) raise Http404 @@ -370,7 +378,7 @@ def modx_dispatch(request, dispatch=None, id=None, course_id=None): p[inputfile_id] = inputfile student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(request.user, modulestore().get_item(id)) - instance = get_module(request.user, request, id, student_module_cache) + instance = get_module(request.user, request, id, student_module_cache, course_id=course_id) if instance is None: # Either permissions just changed, or someone is trying to be clever # and load something they shouldn't have access to. diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index dd22327f24..cbd706e155 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -35,19 +35,6 @@ log = logging.getLogger("mitx.courseware") template_imports = {'urllib': urllib} -def course_redirect(request, course_path): - ''' - Allows course XML to refer to its own directory structure without knowing the multicourse hierarchy. - Prepends the multicourse path to the requested course-internal path. - If the course_id cannot be determined, redirect to multicourse page. (NOTE: is Http404 more appropriate?) - ''' - referer = request.META['HTTP_REFERER'] - match = re.search(r'/courses/(?P[^/]+/[^/]+/[^/]+)/', referer) - courses_path = '/courses/' - if match is not None: - courses_path += match.group('course_id') + '/' + course_path - return HttpResponseRedirect(courses_path) - def user_groups(user): """ TODO (vshnayder): This is not used. When we have a new plan for groups, adjust appropriately. @@ -82,7 +69,7 @@ def courses(request): return render_to_response("courses.html", {'universities': universities}) -def render_accordion(request, course, chapter, section): +def render_accordion(request, course, chapter, section, course_id=None): ''' Draws navigation bar. Takes current position in accordion as parameter. @@ -93,7 +80,7 @@ def render_accordion(request, course, chapter, section): Returns the html string''' # grab the table of contents - toc = toc_for_course(request.user, request, course, chapter, section) + toc = toc_for_course(request.user, request, course, chapter, section, course_id=course_id) context = dict([('toc', toc), ('course_id', course.id), @@ -134,7 +121,7 @@ def index(request, course_id, chapter=None, section=None, try: context = { 'csrf': csrf(request)['csrf_token'], - 'accordion': render_accordion(request, course, chapter, section), + 'accordion': render_accordion(request, course, chapter, section, course_id=course_id), 'COURSE_TITLE': course.title, 'course': course, 'init': '', @@ -150,7 +137,7 @@ def index(request, course_id, chapter=None, section=None, section_descriptor) module = get_module(request.user, request, section_descriptor.location, - student_module_cache) + student_module_cache, course_id=course_id) if module is None: # User is probably being clever and trying to access something # they don't have access to. @@ -285,7 +272,7 @@ def profile(request, course_id, student_id=None): user_info = UserProfile.objects.get(user=student) student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(request.user, course) - course_module = get_module(request.user, request, course.location, student_module_cache) + course_module = get_module(request.user, request, course.location, student_module_cache, course_id=course_id) courseware_summary = grades.progress_summary(student, course_module, course.grader, student_module_cache) grade_summary = grades.grade(request.user, request, course, student_module_cache) diff --git a/lms/urls.py b/lms/urls.py index af49aa2f3c..aaeba1b51e 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -124,7 +124,6 @@ if settings.COURSEWARE_ENABLED: 'courseware.views.course_about', name="about_course"), #Inside the course - url(r'^course/(?P.*)$','courseware.views.course_redirect'), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/info$', 'courseware.views.course_info', name="info"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/book$', From f1f80cb1f4bb2394ec8335d5e93094881b277b81 Mon Sep 17 00:00:00 2001 From: kimth Date: Wed, 15 Aug 2012 14:26:41 -0400 Subject: [PATCH 20/46] Drop HttpResponseRedirect import --- lms/djangoapps/courseware/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index cbd706e155..6da6720622 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -9,7 +9,7 @@ from django.core.context_processors import csrf from django.core.urlresolvers import reverse from django.contrib.auth.models import User from django.contrib.auth.decorators import login_required -from django.http import Http404, HttpResponse, HttpResponseRedirect +from django.http import Http404, HttpResponse from django.shortcuts import redirect from mitxmako.shortcuts import render_to_response, render_to_string #from django.views.decorators.csrf import ensure_csrf_cookie From f535f44e625663d21f26d8879f293206ec60cf39 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Wed, 15 Aug 2012 14:32:36 -0400 Subject: [PATCH 21/46] Fix typo that caused two classes to not get loaded --- lms/envs/dev_int.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/envs/dev_int.py b/lms/envs/dev_int.py index 83c7fb0aba..12123e12d4 100644 --- a/lms/envs/dev_int.py +++ b/lms/envs/dev_int.py @@ -17,7 +17,7 @@ COURSE_LISTINGS = { 'default' : ['BerkeleyX/CS169.1x/2012_Fall', 'BerkeleyX/CS188.1x/2012_Fall', 'HarvardX/CS50x/2012', - 'HarvardX/PH207x/2012_Fall' + 'HarvardX/PH207x/2012_Fall', 'MITx/3.091x/2012_Fall', 'MITx/6.002x/2012_Fall', 'MITx/6.00x/2012_Fall'], From a4d67bab33e23f5369b8e390e01b7c697b675ec8 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 15 Aug 2012 14:36:06 -0400 Subject: [PATCH 22/46] Add support metadata in policy.json * if there is a policy.json in the course dir, read it * file format is a dict with keys {category}/{url_name}, and values metadata dictionaries * apply the policy, overwriting keys that are in the xml * then do metadata inheritance, inheriting any overwritten keys. * also a management cmd to generate a policy.json from a course dir. --- common/lib/xmodule/xmodule/modulestore/xml.py | 30 +++++- common/lib/xmodule/xmodule/x_module.py | 26 +++++ .../management/commands/metadata_to_json.py | 98 +++++++++++++++++++ 3 files changed, 152 insertions(+), 2 deletions(-) create mode 100644 lms/djangoapps/courseware/management/commands/metadata_to_json.py diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 8c4c373d4f..0c0d750882 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -1,3 +1,4 @@ +import json import logging import os import re @@ -149,7 +150,7 @@ class XMLModuleStore(ModuleStoreBase): for course_dir in course_dirs: self.try_load_course(course_dir) - def try_load_course(self,course_dir): + def try_load_course(self, course_dir): ''' Load a course, keeping track of errors as we go along. ''' @@ -170,7 +171,27 @@ class XMLModuleStore(ModuleStoreBase): ''' String representation - for debugging ''' - return 'data_dir=%s, %d courses, %d modules' % (self.data_dir,len(self.courses),len(self.modules)) + return 'data_dir=%s, %d courses, %d modules' % ( + self.data_dir, len(self.courses), len(self.modules)) + + def load_policy(self, policy_path, tracker): + """ + Attempt to read a course policy from policy_path. If the file + exists, but is invalid, log an error and return {}. + + If the policy loads correctly, returns the deserialized version. + """ + if not os.path.exists(policy_path): + return {} + try: + with open(policy_path) as f: + return json.load(f) + except (IOError, ValueError) as err: + msg = "Error loading course policy from {}".format(policy_path) + tracker(msg) + log.warning(msg + " " + str(err)) + return {} + def load_course(self, course_dir, tracker): """ @@ -214,6 +235,11 @@ class XMLModuleStore(ModuleStoreBase): system = ImportSystem(self, org, course, course_dir, tracker) course_descriptor = system.process_xml(etree.tostring(course_data)) + policy_path = self.data_dir / course_dir / 'policy.json' + + policy = self.load_policy(policy_path, tracker) + XModuleDescriptor.apply_policy(course_descriptor, policy) + # NOTE: The descriptors end up loading somewhat bottom up, which # breaks metadata inheritance via get_children(). Instead # (actually, in addition to, for now), we do a final inheritance pass diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 06449dc37f..af3f04e366 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -298,6 +298,14 @@ class XModule(HTMLSnippet): return "" +def policy_key(location): + """ + Get the key for a location in a policy file. (Since the policy file is + specific to a course, it doesn't need the full location url). + """ + return '{cat}/{name}'.format(cat=location.category, name=location.name) + + class XModuleDescriptor(Plugin, HTMLSnippet): """ An XModuleDescriptor is a specification for an element of a course. This @@ -416,6 +424,24 @@ class XModuleDescriptor(Plugin, HTMLSnippet): return dict((k,v) for k,v in self.metadata.items() if k not in self._inherited_metadata) + + @staticmethod + def apply_policy(node, policy): + """ + Given a descriptor, traverse all its descendants and update its metadata + with the policy. + + Notes: + - this does not propagate inherited metadata. The caller should + call compute_inherited_metadata after applying the policy. + - metadata specified in the policy overrides metadata in the xml + """ + k = policy_key(node.location) + if k in policy: + node.metadata.update(policy[k]) + for c in node.get_children(): + XModuleDescriptor.apply_policy(c, policy) + @staticmethod def compute_inherited_metadata(node): """Given a descriptor, traverse all of its descendants and do metadata diff --git a/lms/djangoapps/courseware/management/commands/metadata_to_json.py b/lms/djangoapps/courseware/management/commands/metadata_to_json.py new file mode 100644 index 0000000000..0f48e93319 --- /dev/null +++ b/lms/djangoapps/courseware/management/commands/metadata_to_json.py @@ -0,0 +1,98 @@ +""" +A script to walk a course xml tree, generate a dictionary of all the metadata, +and print it out as a json dict. +""" +import os +import sys +import json + +from collections import OrderedDict +from path import path + +from django.core.management.base import BaseCommand + +from xmodule.modulestore.xml import XMLModuleStore +from xmodule.x_module import policy_key + +def import_course(course_dir, verbose=True): + course_dir = path(course_dir) + data_dir = course_dir.dirname() + course_dirs = [course_dir.basename()] + + # No default class--want to complain if it doesn't find plugins for any + # module. + modulestore = XMLModuleStore(data_dir, + default_class=None, + eager=True, + course_dirs=course_dirs) + + def str_of_err(tpl): + (msg, exc_str) = tpl + return '{msg}\n{exc}'.format(msg=msg, exc=exc_str) + + courses = modulestore.get_courses() + + n = len(courses) + if n != 1: + sys.stderr.write('ERROR: Expect exactly 1 course. Loaded {n}: {lst}\n'.format( + n=n, lst=courses)) + return None + + course = courses[0] + errors = modulestore.get_item_errors(course.location) + if len(errors) != 0: + sys.stderr.write('ERRORs during import: {}\n'.format('\n'.join(map(str_of_err, errors)))) + + return course + +def node_metadata(node): + # make a copy + to_export = ('format', 'display_name', + 'graceperiod', 'showanswer', 'rerandomize', + 'start', 'due', 'graded', 'hide_from_toc', + 'ispublic', 'xqa_key') + + orig = node.own_metadata + d = {k: orig[k] for k in to_export if k in orig} + return d + +def get_metadata(course): + d = OrderedDict({}) + queue = [course] + while len(queue) > 0: + node = queue.pop() + d[policy_key(node.location)] = node_metadata(node) + # want to print first children first, so put them at the end + # (we're popping from the end) + queue.extend(reversed(node.get_children())) + return d + + +def print_metadata(course_dir, output): + course = import_course(course_dir) + if course: + meta = get_metadata(course) + result = json.dumps(meta, indent=4) + if output: + with file(output, 'w') as f: + f.write(result) + else: + print result + + +class Command(BaseCommand): + help = """Imports specified course.xml and prints its +metadata as a json dict. + +Usage: metadata_to_json PATH-TO-COURSE-DIR OUTPUT-PATH + +if OUTPUT-PATH isn't given, print to stdout. +""" + def handle(self, *args, **options): + n = len(args) + if n < 1 or n > 2: + print Command.help + return + + output_path = args[1] if n > 1 else None + print_metadata(args[0], output_path) From 1c2b6e80885e200c96e770c1b54b24c71b536608 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Wed, 15 Aug 2012 14:39:56 -0400 Subject: [PATCH 23/46] Remove stuff that I should have deleted during the rebase --- lms/djangoapps/courseware/courses.py | 53 ---------------------------- 1 file changed, 53 deletions(-) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index e142614f46..f0b82a3c9c 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -142,59 +142,6 @@ def get_course_info_section(course, section_key): raise KeyError("Invalid about key " + str(section_key)) -def course_staff_group_name(course): - ''' - course should be either a CourseDescriptor instance, or a string (the - .course entry of a Location) - ''' - if isinstance(course, str) or isinstance(course, unicode): - coursename = course - else: - # should be a CourseDescriptor, so grab its location.course: - coursename = course.location.course - return 'staff_%s' % coursename - -def has_staff_access_to_course(user, course): - ''' - Returns True if the given user has staff access to the course. - This means that user is in the staff_* group, or is an overall admin. - TODO (vshnayder): this needs to be changed to allow per-course_id permissions, not per-course - (e.g. staff in 2012 is different from 2013, but maybe some people always have access) - - course is the course field of the location being accessed. - ''' - if user is None or (not user.is_authenticated()) or course is None: - return False - if user.is_staff: - return True - - # note this is the Auth group, not UserTestGroup - user_groups = [x[1] for x in user.groups.values_list()] - staff_group = course_staff_group_name(course) - if staff_group in user_groups: - return True - return False - -def has_staff_access_to_course_id(user, course_id): - """Helper method that takes a course_id instead of a course name""" - loc = CourseDescriptor.id_to_location(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'): - return True - return has_staff_access_to_course(user,course) - def get_courses_by_university(user, domain=None): ''' From 02d970c31e8def30d5b116e95f42f078ad9c1cd0 Mon Sep 17 00:00:00 2001 From: kimth Date: Wed, 15 Aug 2012 14:45:12 -0400 Subject: [PATCH 24/46] Map '/course/' urls to the course's root in the multicourse hierarchy --- common/djangoapps/xmodule_modifiers.py | 11 +++++++++++ lms/djangoapps/courseware/module_render.py | 6 +++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 0aeaa59d69..4b3050e227 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -34,6 +34,17 @@ def wrap_xmodule(get_html, module, template): return _get_html +def replace_course_urls(get_html, course_id, module): + """ + Updates the supplied module with a new get_html function that wraps + the old get_html function and substitutes urls of the form /course/... + with urls that are /courses//... + """ + @wraps(get_html) + def _get_html(): + return replace_urls(get_html(), staticfiles_prefix='/courses/'+course_id, replace_prefix='/course/') + return _get_html + def replace_static_urls(get_html, prefix, module): """ Updates the supplied module with a new get_html function that wraps diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 33f131e576..b84bdb2310 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -19,7 +19,7 @@ from xmodule.exceptions import NotFoundError from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore from xmodule.x_module import ModuleSystem -from xmodule_modifiers import replace_static_urls, add_histogram, wrap_xmodule +from xmodule_modifiers import replace_course_urls, replace_static_urls, add_histogram, wrap_xmodule log = logging.getLogger("mitx.courseware") @@ -233,6 +233,10 @@ def get_module(user, request, location, student_module_cache, position=None, cou module.metadata['data_dir'], module ) + # Allow URLs of the form '/course/' refer to the root of multicourse directory + # hierarchy of this course + module.get_html = replace_course_urls(module.get_html, course_id, module) + if settings.MITX_FEATURES.get('DISPLAY_HISTOGRAMS_TO_STAFF'): if has_access(user, module, 'staff'): module.get_html = add_histogram(module.get_html, module, user) From 58932bb2f7a37ee91939b7c482aa77c34696cc71 Mon Sep 17 00:00:00 2001 From: kimth Date: Wed, 15 Aug 2012 15:23:14 -0400 Subject: [PATCH 25/46] Drop import of re --- lms/djangoapps/courseware/views.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 20a1443f53..f5a93475b9 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -1,6 +1,5 @@ import json import logging -import re import urllib import itertools From ce29b5aec20daa4c04150770a887beb87656e55f Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 15 Aug 2012 16:06:42 -0400 Subject: [PATCH 26/46] Special case handling of course url_names in policy.json --- common/lib/xmodule/xmodule/modulestore/xml.py | 10 ++++++++-- common/lib/xmodule/xmodule/x_module.py | 4 ++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 0c0d750882..9fc0ba29ca 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -234,10 +234,16 @@ class XMLModuleStore(ModuleStoreBase): system = ImportSystem(self, org, course, course_dir, tracker) - course_descriptor = system.process_xml(etree.tostring(course_data)) policy_path = self.data_dir / course_dir / 'policy.json' - policy = self.load_policy(policy_path, tracker) + # Special case -- need to change the url_name of the course before + # it gets loaded, so its location and other fields are right + if 'course_url_name' in policy: + new_url_name = policy['course_url_name'] + log.info("changing course url_name to {}".format(new_url_name)) + course_data.set('url_name', new_url_name) + + course_descriptor = system.process_xml(etree.tostring(course_data)) XModuleDescriptor.apply_policy(course_descriptor, policy) # NOTE: The descriptors end up loading somewhat bottom up, which diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index af3f04e366..d68dd61e3d 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -303,6 +303,10 @@ def policy_key(location): Get the key for a location in a policy file. (Since the policy file is specific to a course, it doesn't need the full location url). """ + # Special case--we need to be able to override the url_name on a course, + # so special case where we look for the course descriptor + if location.category == 'course': + return 'course' return '{cat}/{name}'.format(cat=location.category, name=location.name) From 3519ea6b2fcdc2247640b92309ff13492770fa76 Mon Sep 17 00:00:00 2001 From: kimth Date: Wed, 15 Aug 2012 17:14:27 -0400 Subject: [PATCH 27/46] Don't display 'None points' on problem rendering --- lms/templates/problem.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/templates/problem.html b/lms/templates/problem.html index 6363274d24..ed49b3bd5d 100644 --- a/lms/templates/problem.html +++ b/lms/templates/problem.html @@ -1,7 +1,7 @@ <%namespace name='static' file='static_content.html'/>

    ${ problem['name'] } - % if problem['weight'] != 1: + % if problem['weight'] != 1 and problem['weight'] != None: : ${ problem['weight'] } points % endif

    From f717b0421f250bd2fb7de8652a1e652e9fcf5b4c Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 15 Aug 2012 21:39:10 -0400 Subject: [PATCH 28/46] fix bug in backcompat section replacement (s/sequence/sequential/) --- common/lib/xmodule/xmodule/backcompat_module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/backcompat_module.py b/common/lib/xmodule/xmodule/backcompat_module.py index c49f23b99e..d9f10ffba0 100644 --- a/common/lib/xmodule/xmodule/backcompat_module.py +++ b/common/lib/xmodule/xmodule/backcompat_module.py @@ -74,7 +74,7 @@ class SemanticSectionDescriptor(XModuleDescriptor): return system.process_xml(etree.tostring(xml_object[0])) else: - xml_object.tag = 'sequence' + xml_object.tag = 'sequential' return system.process_xml(etree.tostring(xml_object)) From f39e58962508bf47313dee576cd39bb6b4a498a0 Mon Sep 17 00:00:00 2001 From: ichuang Date: Wed, 15 Aug 2012 21:57:33 -0400 Subject: [PATCH 29/46] do replace_urls within capa_module get_answer (eg for img in s) --- common/lib/xmodule/xmodule/capa_module.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 46e02542c8..e27126c549 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -376,14 +376,17 @@ class CapaModule(XModule): ''' For the "show answer" button. - TODO: show answer events should be logged here, not just in the problem.js - Returns the answers: {'answers' : answers} ''' + event_info = dict() + event_info['problem_id'] = self.location.url() + self.system.track_function('show_answer', event_info) if not self.answer_available(): raise NotFoundError('Answer is not available') else: answers = self.lcp.get_question_answers() + # answers (eg ) may have embedded images + answers = dict( (k,self.system.replace_urls(answers[k], self.metadata['data_dir'])) for k in answers ) return {'answers': answers} # Figure out if we should move these to capa_problem? From 033211a5d18e226a2a3e9754438f186b55f59bde Mon Sep 17 00:00:00 2001 From: ichuang Date: Wed, 15 Aug 2012 22:03:00 -0400 Subject: [PATCH 30/46] doc update - LMS migrate reload --- doc/development.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doc/development.md b/doc/development.md index 590a935405..cb71278c40 100644 --- a/doc/development.md +++ b/doc/development.md @@ -66,3 +66,9 @@ To run a single nose test: Very handy: if you uncomment the `--pdb` argument in `NOSE_ARGS` in `lms/envs/test.py`, it will drop you into pdb on error. This lets you go up and down the stack and see what the values of the variables are. Check out http://docs.python.org/library/pdb.html + +## Content development + +If you change course content, while running the LMS in dev mode, it is unnecessary to restart to refresh the modulestore. + +Instead, hit /migrate/modules to see a list of all modules loaded, and click on links (eg /migrate/reload/edx4edx) to reload a course. From ae09598d9a388b261b1cc2b72e1d58942d4c5cf4 Mon Sep 17 00:00:00 2001 From: Matthew Mongeau Date: Sun, 12 Aug 2012 20:10:34 -0400 Subject: [PATCH 31/46] Display textbooks in course navigation, handle in urls and views. --- common/lib/xmodule/setup.py | 1 + common/lib/xmodule/xmodule/course_module.py | 5 ++++ common/lib/xmodule/xmodule/textbook_module.py | 27 +++++++++++++++++++ lms/djangoapps/staticbook/views.py | 8 +++--- lms/templates/course_navigation.html | 8 +++--- lms/urls.py | 4 +-- 6 files changed, 43 insertions(+), 10 deletions(-) create mode 100644 common/lib/xmodule/xmodule/textbook_module.py diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index 8a0a6bb139..31918c0250 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -31,6 +31,7 @@ setup( "section = xmodule.backcompat_module:SemanticSectionDescriptor", "sequential = xmodule.seq_module:SequenceDescriptor", "slides = xmodule.backcompat_module:TranslateCustomTagDescriptor", + "textbook = xmodule.textbook_module:TextbookDescriptor", "vertical = xmodule.vertical_module:VerticalDescriptor", "video = xmodule.video_module:VideoDescriptor", "videodev = xmodule.backcompat_module:TranslateCustomTagDescriptor", diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 11d4e090f9..8674b8b443 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -7,6 +7,7 @@ from xmodule.graders import load_grading_policy from xmodule.modulestore import Location from xmodule.seq_module import SequenceDescriptor, SequenceModule from xmodule.timeparse import parse_time, stringify_time +from xmodule.textbook_module import TextbookDescriptor log = logging.getLogger(__name__) @@ -53,6 +54,10 @@ class CourseDescriptor(SequenceDescriptor): return grading_policy + @property + def textbooks(self): + return [child for child in self.get_children() if type(child) == TextbookDescriptor] + @lazyproperty def grading_context(self): diff --git a/common/lib/xmodule/xmodule/textbook_module.py b/common/lib/xmodule/xmodule/textbook_module.py new file mode 100644 index 0000000000..bd1c422643 --- /dev/null +++ b/common/lib/xmodule/xmodule/textbook_module.py @@ -0,0 +1,27 @@ +from xmodule.x_module import XModule +from xmodule.xml_module import XmlDescriptor +from lxml import etree + +class TextbookModule(XModule): + def __init__(self, system, location, definition, instance_state=None, + shared_state=None, **kwargs): + XModule.__init__(self, system, location, definition, + instance_state, shared_state, **kwargs) + +class TextbookDescriptor(XmlDescriptor): + + module_class = TextbookModule + + def __init__(self, system, definition=None, **kwargs): + super(TextbookDescriptor, self).__init__(system, definition, **kwargs) + self.title = self.metadata["title"] + + @classmethod + def definition_from_xml(cls, xml_object, system): + return { 'children': [] } + + @property + def table_of_contents(self): + raw_table_of_contents = open(self.metadata['table_of_contents_url'], 'r') # TODO: This will need to come from S3 + table_of_contents = etree.parse(raw_table_of_contents).getroot() + return table_of_contents diff --git a/lms/djangoapps/staticbook/views.py b/lms/djangoapps/staticbook/views.py index 2e19ab6425..aaafb60dd8 100644 --- a/lms/djangoapps/staticbook/views.py +++ b/lms/djangoapps/staticbook/views.py @@ -6,19 +6,17 @@ from courseware.courses import get_course_with_access from lxml import etree @login_required -def index(request, course_id, page=0): +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') - # 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() + textbook = course.textbooks[int(book_index)] + table_of_contents = textbook.table_of_contents return render_to_response('staticbook.html', {'page': int(page), 'course': course, 'table_of_contents': table_of_contents, 'staff_access': staff_access}) - def index_shifted(request, course_id, page): return index(request, course_id=course_id, page=int(page) + 24) diff --git a/lms/templates/course_navigation.html b/lms/templates/course_navigation.html index 9e93b2fb14..b75c12064d 100644 --- a/lms/templates/course_navigation.html +++ b/lms/templates/course_navigation.html @@ -16,8 +16,10 @@ def url_class(url):
  • Course Info
    • % if user.is_authenticated(): % if settings.MITX_FEATURES.get('ENABLE_TEXTBOOK'): -
  • Textbook
    • -% endif + % for index, textbook in enumerate(course.textbooks): +
  • ${textbook.title}
    • + % endfor +% endif % if settings.MITX_FEATURES.get('ENABLE_DISCUSSION'):
  • Discussion
    • % endif @@ -34,4 +36,4 @@ def url_class(url): - \ No newline at end of file + diff --git a/lms/urls.py b/lms/urls.py index aaeba1b51e..6850b65644 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -126,9 +126,9 @@ if settings.COURSEWARE_ENABLED: #Inside the course url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/info$', 'courseware.views.course_info', name="info"), - url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/book$', + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/book/(?P[^/]*)/$', 'staticbook.views.index', name="book"), - url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/book/(?P[^/]*)$', + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/book/(?P[^/]*)/(?P[^/]*)$', 'staticbook.views.index'), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/book-shifted/(?P[^/]*)$', 'staticbook.views.index_shifted'), From 34010a38a822394e9d611117285a2fe03166c82d Mon Sep 17 00:00:00 2001 From: Matthew Mongeau Date: Wed, 15 Aug 2012 11:35:39 -0400 Subject: [PATCH 32/46] Fix textbook module. --- common/lib/xmodule/xmodule/textbook_module.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/textbook_module.py b/common/lib/xmodule/xmodule/textbook_module.py index bd1c422643..d0f7400cfc 100644 --- a/common/lib/xmodule/xmodule/textbook_module.py +++ b/common/lib/xmodule/xmodule/textbook_module.py @@ -3,11 +3,14 @@ from xmodule.xml_module import XmlDescriptor from lxml import etree class TextbookModule(XModule): - def __init__(self, system, location, definition, instance_state=None, + def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, + XModule.__init__(self, system, location, definition, descriptor, instance_state, shared_state, **kwargs) + def get_display_items(self): + return [] + class TextbookDescriptor(XmlDescriptor): module_class = TextbookModule From 4a7b163ce34628ac5392e27ca682cc7487459d94 Mon Sep 17 00:00:00 2001 From: Matthew Mongeau Date: Wed, 15 Aug 2012 11:37:44 -0400 Subject: [PATCH 33/46] Keep textbooks out of displayable items. --- common/lib/xmodule/xmodule/textbook_module.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/common/lib/xmodule/xmodule/textbook_module.py b/common/lib/xmodule/xmodule/textbook_module.py index d0f7400cfc..e0d0730627 100644 --- a/common/lib/xmodule/xmodule/textbook_module.py +++ b/common/lib/xmodule/xmodule/textbook_module.py @@ -11,6 +11,9 @@ class TextbookModule(XModule): def get_display_items(self): return [] + def displayable_items(self): + return [] + class TextbookDescriptor(XmlDescriptor): module_class = TextbookModule From 1bb8d9366dd344f49507dc432c2355d42d22c2d5 Mon Sep 17 00:00:00 2001 From: kimth Date: Thu, 16 Aug 2012 11:27:29 -0400 Subject: [PATCH 34/46] Xqueue URL is https not http --- lms/envs/dev.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/envs/dev.py b/lms/envs/dev.py index 8457b50374..6720c2050d 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -54,7 +54,7 @@ CACHES = { } XQUEUE_INTERFACE = { - "url": "http://sandbox-xqueue.edx.org", + "url": "https://sandbox-xqueue.edx.org", "django_auth": { "username": "lms", "password": "***REMOVED***" From ad7061e41f1f32e1cb48a784e887dfa77986b0bb Mon Sep 17 00:00:00 2001 From: Kyle Fiedler Date: Wed, 15 Aug 2012 15:44:19 -0400 Subject: [PATCH 35/46] Added styles to grade out the top and bottom of the sidebar border and fixed spacing with the video and the sequence nav --- .../lib/xmodule/xmodule/css/sequence/display.scss | 2 +- common/lib/xmodule/xmodule/css/video/display.scss | 2 +- lms/djangoapps/courseware/access.py | 4 ++++ lms/static/sass/course/_info.scss | 7 ++++++- lms/static/sass/course/_profile.scss | 5 +++++ lms/static/sass/course/base/_base.scss | 4 ++++ lms/static/sass/course/base/_extends.scss | 14 +++++++++++++- lms/static/sass/course/discussion/_sidebar.scss | 5 +++++ 8 files changed, 39 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/css/sequence/display.scss b/common/lib/xmodule/xmodule/css/sequence/display.scss index 97a8e0e4b3..a745129a68 100644 --- a/common/lib/xmodule/xmodule/css/sequence/display.scss +++ b/common/lib/xmodule/xmodule/css/sequence/display.scss @@ -4,7 +4,7 @@ nav.sequence-nav { @extend .topbar; border-bottom: 1px solid $border-color; @include border-top-right-radius(4px); - margin: (-(lh())) (-(lh())) lh() (-(lh())); + margin: 0 0 lh() (-(lh())); position: relative; ol { diff --git a/common/lib/xmodule/xmodule/css/video/display.scss b/common/lib/xmodule/xmodule/css/video/display.scss index 8d0c4ac522..9e32de941a 100644 --- a/common/lib/xmodule/xmodule/css/video/display.scss +++ b/common/lib/xmodule/xmodule/css/video/display.scss @@ -4,7 +4,7 @@ div.video { border-bottom: 1px solid #e1e1e1; border-top: 1px solid #e1e1e1; display: block; - margin: 0 (-(lh())); + margin: 0 0 0 (-(lh())); padding: 6px lh(); article.video-wrapper { diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index e588f807da..69d9839045 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -84,8 +84,12 @@ def _has_access_course_desc(user, course, action): 'staff' -- staff access to course. """ def can_load(): +<<<<<<< Updated upstream """ Can this user load this course? +======= + """Can this user load this course? +>>>>>>> Stashed changes NOTE: this is not checking whether user is actually enrolled in the course. """ diff --git a/lms/static/sass/course/_info.scss b/lms/static/sass/course/_info.scss index 1651ad4da8..e68e386696 100644 --- a/lms/static/sass/course/_info.scss +++ b/lms/static/sass/course/_info.scss @@ -69,10 +69,15 @@ div.info-wrapper { section.handouts { @extend .sidebar; border-left: 1px solid $border-color; - @include border-radius(0 4px 4px 0); border-right: 0; + @include border-radius(0 4px 4px 0); @include box-shadow(none); + &:after { + left: -1px; + right: auto; + } + h1 { @extend .bottom-border; margin-bottom: 0; diff --git a/lms/static/sass/course/_profile.scss b/lms/static/sass/course/_profile.scss index 006bd902e5..5b1d6ee068 100644 --- a/lms/static/sass/course/_profile.scss +++ b/lms/static/sass/course/_profile.scss @@ -8,6 +8,11 @@ div.profile-wrapper { @include border-radius(0px 4px 4px 0); border-right: 0; + &:after { + left: -1px; + right: auto; + } + header { @extend .bottom-border; margin: 0; diff --git a/lms/static/sass/course/base/_base.scss b/lms/static/sass/course/base/_base.scss index 90c8fdc3e2..9f0be9c298 100644 --- a/lms/static/sass/course/base/_base.scss +++ b/lms/static/sass/course/base/_base.scss @@ -31,6 +31,10 @@ img { max-width: 100%; } +.container { + padding: em(40) 0; +} + ::selection, ::-moz-selection, ::-webkit-selection { background:#444; color:#fff; diff --git a/lms/static/sass/course/base/_extends.scss b/lms/static/sass/course/base/_extends.scss index a438b93150..78618df83d 100644 --- a/lms/static/sass/course/base/_extends.scss +++ b/lms/static/sass/course/base/_extends.scss @@ -49,6 +49,18 @@ h1.top-header { vertical-align: top; width: flex-grid(3); + &:after { + width: 1px; + height: 100%; + @include position(absolute, 0px -1px 0px 0); + content: ""; + @include background-image(linear-gradient(top, #fff, rgba(#fff, 0)), linear-gradient(top, rgba(#fff, 0), #fff)); + background-position: top, bottom; + @include background-size(1px 20px); + background-repeat: no-repeat; + display: block; + } + h1, h2 { font-size: em(20); font-weight: 100; @@ -137,7 +149,7 @@ h1.top-header { position: absolute; right: -1px; text-indent: -9999px; - top: 6px; + top: 12px; width: 16px; z-index: 99; diff --git a/lms/static/sass/course/discussion/_sidebar.scss b/lms/static/sass/course/discussion/_sidebar.scss index 7ee435f8af..59dcfaf449 100644 --- a/lms/static/sass/course/discussion/_sidebar.scss +++ b/lms/static/sass/course/discussion/_sidebar.scss @@ -6,6 +6,11 @@ div.discussion-wrapper aside { border-right: 0; width: flex-grid(3); + &:after { + left: -1px; + right: auto; + } + &.main-sidebar { min-width:200px; } From 7f72004b50bbc4078ab7f15395643cc9fc985f9b Mon Sep 17 00:00:00 2001 From: Kyle Fiedler Date: Wed, 15 Aug 2012 15:53:37 -0400 Subject: [PATCH 36/46] Fix conflicts --- lms/djangoapps/courseware/access.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 69d9839045..e588f807da 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -84,12 +84,8 @@ def _has_access_course_desc(user, course, action): 'staff' -- staff access to course. """ def can_load(): -<<<<<<< Updated upstream """ Can this user load this course? -======= - """Can this user load this course? ->>>>>>> Stashed changes NOTE: this is not checking whether user is actually enrolled in the course. """ From 5e2f676153899b3333aa5111865e4aefd2210a48 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 16 Aug 2012 11:36:05 -0400 Subject: [PATCH 37/46] backcompat warnings --- common/lib/xmodule/xmodule/backcompat_module.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/backcompat_module.py b/common/lib/xmodule/xmodule/backcompat_module.py index d9f10ffba0..06713d3432 100644 --- a/common/lib/xmodule/xmodule/backcompat_module.py +++ b/common/lib/xmodule/xmodule/backcompat_module.py @@ -21,6 +21,7 @@ def process_includes(fn): xml_object = etree.fromstring(xml_data) next_include = xml_object.find('include') while next_include is not None: + system.error_tracker("WARNING: the tag is deprecated, and will go away.") file = next_include.get('file') parent = next_include.getparent() @@ -67,6 +68,8 @@ class SemanticSectionDescriptor(XModuleDescriptor): the child element """ xml_object = etree.fromstring(xml_data) + system.error_tracker("WARNING: the <{}> tag is deprecated. Please do not use in new content." + .format(xml_object.tag)) if len(xml_object) == 1: for (key, val) in xml_object.items(): @@ -83,10 +86,14 @@ class TranslateCustomTagDescriptor(XModuleDescriptor): def from_xml(cls, xml_data, system, org=None, course=None): """ Transforms the xml_data from <$custom_tag attr="" attr=""/> to - $custom_tag + """ xml_object = etree.fromstring(xml_data) + system.error_tracker('WARNING: the <{tag}> tag is deprecated. ' + 'Instead, use . ' + .format(xml_object.tag)) + tag = xml_object.tag xml_object.tag = 'customtag' xml_object.attrib['impl'] = tag From 691744e3599fc537c9b4f11c0902454660d3722b Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 16 Aug 2012 11:37:54 -0400 Subject: [PATCH 38/46] Use .display_name instead of metadata['display_name'] * the former does a fallback if metadata['display_name'] isn't set. --- common/lib/xmodule/xmodule/capa_module.py | 4 ++-- common/lib/xmodule/xmodule/course_module.py | 2 +- common/lib/xmodule/xmodule/seq_module.py | 4 ++-- lms/djangoapps/lms_migration/migrate.py | 12 ++++++------ 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 46e02542c8..791ec8be45 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -237,7 +237,7 @@ class CapaModule(XModule): else: raise - content = {'name': self.metadata['display_name'], + content = {'name': self.display_name, 'html': html, 'weight': self.weight, } @@ -464,7 +464,7 @@ class CapaModule(XModule): return {'success': msg} log.exception("Error in capa_module problem checking") raise Exception("error in capa_module") - + self.attempts = self.attempts + 1 self.lcp.done = True diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 11d4e090f9..04b38b55ad 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -140,7 +140,7 @@ class CourseDescriptor(SequenceDescriptor): @property def title(self): - return self.metadata['display_name'] + return self.display_name @property def number(self): diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 2986c948d3..fee4d53700 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -76,7 +76,7 @@ class SequenceModule(XModule): contents.append({ 'content': child.get_html(), 'title': "\n".join( - grand_child.metadata['display_name'].strip() + grand_child.display_name.strip() for grand_child in child.get_children() if 'display_name' in grand_child.metadata ), @@ -107,7 +107,7 @@ class SequenceModule(XModule): class SequenceDescriptor(MakoModuleDescriptor, XmlDescriptor): mako_template = 'widgets/sequence-edit.html' module_class = SequenceModule - + stores_state = True # For remembering where in the sequence the student is @classmethod diff --git a/lms/djangoapps/lms_migration/migrate.py b/lms/djangoapps/lms_migration/migrate.py index 2bf893507b..dfdf86b4ac 100644 --- a/lms/djangoapps/lms_migration/migrate.py +++ b/lms/djangoapps/lms_migration/migrate.py @@ -35,7 +35,7 @@ def manage_modulestores(request,reload_dir=None): ip = request.META.get('HTTP_X_REAL_IP','') # nginx reverse proxy if not ip: ip = request.META.get('REMOTE_ADDR','None') - + if LOCAL_DEBUG: html += '

    IP address: %s ' % ip html += '

    User: %s ' % request.user @@ -48,7 +48,7 @@ def manage_modulestores(request,reload_dir=None): html += 'Permission denied' html += "" log.debug('request denied, ALLOWED_IPS=%s' % ALLOWED_IPS) - return HttpResponse(html) + return HttpResponse(html) #---------------------------------------- # reload course if specified @@ -74,10 +74,10 @@ def manage_modulestores(request,reload_dir=None): #---------------------------------------- dumpfields = ['definition','location','metadata'] - + for cdir, course in def_ms.courses.items(): html += '
    ' - html += '

    Course: %s (%s)

    ' % (course.metadata['display_name'],cdir) + html += '

    Course: %s (%s)

    ' % (course.display_name,cdir) for field in dumpfields: data = getattr(course,field) @@ -89,7 +89,7 @@ def manage_modulestores(request,reload_dir=None): html += '' else: html += '
    • %s
    ' % escape(data) - + #---------------------------------------- @@ -107,4 +107,4 @@ def manage_modulestores(request,reload_dir=None): log.debug('def_ms=%s' % unicode(def_ms)) html += "" - return HttpResponse(html) + return HttpResponse(html) From 00d9ecd6001d3441931687485b2b81d36c3ce6fd Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 16 Aug 2012 11:41:19 -0400 Subject: [PATCH 39/46] New policy organization: * course roots live in roots/{url_name}.xml - one is linked from course.xml * policies live in policies/url_name.json - loaded based on course url_name * Updated to pass policy through into xml parsing, so it takes effect before descriptor constructors are called. * Update toy test course to new structure, fix up tests --- .../xmodule/modulestore/tests/test_mongo.py | 4 +-- common/lib/xmodule/xmodule/modulestore/xml.py | 23 ++++++++-------- .../lib/xmodule/xmodule/tests/test_import.py | 4 +-- common/lib/xmodule/xmodule/x_module.py | 26 +++---------------- common/lib/xmodule/xmodule/xml_module.py | 7 ++++- common/test/data/toy/course.xml | 10 +------ common/test/data/toy/course/2012_Fall.xml | 9 +++++++ common/test/data/toy/policies/2012_Fall.json | 23 ++++++++++++++++ common/test/data/toy/roots/2012_Fall.xml | 1 + 9 files changed, 59 insertions(+), 48 deletions(-) mode change 100644 => 120000 common/test/data/toy/course.xml create mode 100644 common/test/data/toy/course/2012_Fall.xml create mode 100644 common/test/data/toy/policies/2012_Fall.json create mode 100644 common/test/data/toy/roots/2012_Fall.xml diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index 24f0441ee0..cb94444b7a 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -112,8 +112,8 @@ class TestMongoModuleStore(object): should_work = ( ("i4x://edX/toy/video/Welcome", ("edX/toy/2012_Fall", "Overview", "Welcome", None)), - ("i4x://edX/toy/html/toylab", - ("edX/toy/2012_Fall", "Overview", "Toy_Videos", 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) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 9fc0ba29ca..e52126a2c8 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -31,7 +31,8 @@ def clean_out_mako_templating(xml_string): return xml_string class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): - def __init__(self, xmlstore, org, course, course_dir, error_tracker, **kwargs): + def __init__(self, xmlstore, org, course, course_dir, + policy, error_tracker, **kwargs): """ A class that handles loading from xml. Does some munging to ensure that all elements have unique slugs. @@ -97,7 +98,7 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): MakoDescriptorSystem.__init__(self, load_item, resources_fs, error_tracker, render_template, **kwargs) XMLParsingSystem.__init__(self, load_item, resources_fs, - error_tracker, process_xml, **kwargs) + error_tracker, process_xml, policy, **kwargs) class XMLModuleStore(ModuleStoreBase): @@ -184,6 +185,7 @@ class XMLModuleStore(ModuleStoreBase): if not os.path.exists(policy_path): return {} try: + log.debug("Loading policy from {}".format(policy_path)) with open(policy_path) as f: return json.load(f) except (IOError, ValueError) as err: @@ -232,19 +234,16 @@ class XMLModuleStore(ModuleStoreBase): tracker(msg) course = course_dir - system = ImportSystem(self, org, course, course_dir, tracker) + url_name = course_data.get('url_name') + if url_name: + policy_path = self.data_dir / course_dir / 'policies' / '{}.json'.format(url_name) + policy = self.load_policy(policy_path, tracker) + else: + policy = {} - policy_path = self.data_dir / course_dir / 'policy.json' - policy = self.load_policy(policy_path, tracker) - # Special case -- need to change the url_name of the course before - # it gets loaded, so its location and other fields are right - if 'course_url_name' in policy: - new_url_name = policy['course_url_name'] - log.info("changing course url_name to {}".format(new_url_name)) - course_data.set('url_name', new_url_name) + system = ImportSystem(self, org, course, course_dir, policy, tracker) course_descriptor = system.process_xml(etree.tostring(course_data)) - XModuleDescriptor.apply_policy(course_descriptor, policy) # NOTE: The descriptors end up loading somewhat bottom up, which # breaks metadata inheritance via get_children(). Instead diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index 1da618f6a4..dfa75f9137 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -42,9 +42,9 @@ class DummySystem(XMLParsingSystem): descriptor.get_children() return descriptor - + policy = {} XMLParsingSystem.__init__(self, load_item, self.resources_fs, - self.errorlog.tracker, process_xml) + self.errorlog.tracker, process_xml, policy) def render_template(self, template, context): raise Exception("Shouldn't be called") diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index d68dd61e3d..f598204454 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -303,10 +303,6 @@ def policy_key(location): Get the key for a location in a policy file. (Since the policy file is specific to a course, it doesn't need the full location url). """ - # Special case--we need to be able to override the url_name on a course, - # so special case where we look for the course descriptor - if location.category == 'course': - return 'course' return '{cat}/{name}'.format(cat=location.category, name=location.name) @@ -429,23 +425,6 @@ class XModuleDescriptor(Plugin, HTMLSnippet): if k not in self._inherited_metadata) - @staticmethod - def apply_policy(node, policy): - """ - Given a descriptor, traverse all its descendants and update its metadata - with the policy. - - Notes: - - this does not propagate inherited metadata. The caller should - call compute_inherited_metadata after applying the policy. - - metadata specified in the policy overrides metadata in the xml - """ - k = policy_key(node.location) - if k in policy: - node.metadata.update(policy[k]) - for c in node.get_children(): - XModuleDescriptor.apply_policy(c, policy) - @staticmethod def compute_inherited_metadata(node): """Given a descriptor, traverse all of its descendants and do metadata @@ -701,16 +680,19 @@ class DescriptorSystem(object): class XMLParsingSystem(DescriptorSystem): - def __init__(self, load_item, resources_fs, error_tracker, process_xml, **kwargs): + def __init__(self, load_item, resources_fs, error_tracker, process_xml, policy, **kwargs): """ load_item, resources_fs, error_tracker: see DescriptorSystem + policy: a policy dictionary for overriding xml metadata + process_xml: Takes an xml string, and returns a XModuleDescriptor created from that xml """ DescriptorSystem.__init__(self, load_item, resources_fs, error_tracker, **kwargs) self.process_xml = process_xml + self.policy = policy class ModuleSystem(object): diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 3c2f3269ca..c7042efda2 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -1,4 +1,4 @@ -from xmodule.x_module import XModuleDescriptor +from xmodule.x_module import (XModuleDescriptor, policy_key) from xmodule.modulestore import Location from lxml import etree import json @@ -270,6 +270,11 @@ class XmlDescriptor(XModuleDescriptor): log.debug('Error %s in loading metadata %s' % (err,dmdata)) metadata['definition_metadata_err'] = str(err) + # Set/override any metadata specified by policy + k = policy_key(location) + if k in system.policy: + metadata.update(system.policy[k]) + return cls( system, definition, diff --git a/common/test/data/toy/course.xml b/common/test/data/toy/course.xml deleted file mode 100644 index 270a1eb27f..0000000000 --- a/common/test/data/toy/course.xml +++ /dev/null @@ -1,9 +0,0 @@ - - - - - - - diff --git a/common/test/data/toy/course.xml b/common/test/data/toy/course.xml new file mode 120000 index 0000000000..49041310f6 --- /dev/null +++ b/common/test/data/toy/course.xml @@ -0,0 +1 @@ +roots/2012_Fall.xml \ No newline at end of file diff --git a/common/test/data/toy/course/2012_Fall.xml b/common/test/data/toy/course/2012_Fall.xml new file mode 100644 index 0000000000..d34eb9d56a --- /dev/null +++ b/common/test/data/toy/course/2012_Fall.xml @@ -0,0 +1,9 @@ + + + + + + + diff --git a/common/test/data/toy/policies/2012_Fall.json b/common/test/data/toy/policies/2012_Fall.json new file mode 100644 index 0000000000..6c501d66f8 --- /dev/null +++ b/common/test/data/toy/policies/2012_Fall.json @@ -0,0 +1,23 @@ +{ + "course/2012_Fall": { + "graceperiod": "2 days 5 hours 59 minutes 59 seconds", + "start": "2015-07-17T12:00", + "display_name": "Toy Course" + }, + "chapter/Overview": { + "display_name": "Overview" + }, + "videosequence/Toy_Videos": { + "display_name": "Toy Videos", + "format": "Lecture Sequence" + }, + "html/toylab": { + "display_name": "Toy lab" + }, + "video/Video_Resources": { + "display_name": "Video Resources" + }, + "video/Welcome": { + "display_name": "Welcome" + } +} diff --git a/common/test/data/toy/roots/2012_Fall.xml b/common/test/data/toy/roots/2012_Fall.xml new file mode 100644 index 0000000000..b71528809b --- /dev/null +++ b/common/test/data/toy/roots/2012_Fall.xml @@ -0,0 +1 @@ + \ No newline at end of file From c02313a0990b6e3dd21f4c43d726cc0e2b5c8820 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 16 Aug 2012 11:43:40 -0400 Subject: [PATCH 40/46] hackish cleanup script --- common/xml_cleanup.py | 112 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100755 common/xml_cleanup.py diff --git a/common/xml_cleanup.py b/common/xml_cleanup.py new file mode 100755 index 0000000000..8e794b97c2 --- /dev/null +++ b/common/xml_cleanup.py @@ -0,0 +1,112 @@ +#!/usr/bin/env python + +""" +Victor's xml cleanup script. A big pile of useful hacks. Do not use +without carefully reading the code and deciding that this is what you want. + +In particular, the remove-meta option is only intended to be used after pulling out a policy +using the metadata_to_json management command. +""" + +import os, fnmatch, re, sys +from lxml import etree +from collections import defaultdict + +INVALID_CHARS = re.compile(r"[^\w.-]") + +def clean(value): + """ + Return value, made into a form legal for locations + """ + return re.sub('_+', '_', INVALID_CHARS.sub('_', value)) + + +# category -> set of url_names for that category that we've already seen +used_names = defaultdict(set) + +def clean_unique(category, name): + cleaned = clean(name) + if cleaned not in used_names[category]: + used_names[category].add(cleaned) + return cleaned + x = 1 + while cleaned + str(x) in used_names[category]: + x += 1 + + # Found one! + cleaned = cleaned + str(x) + used_names[category].add(cleaned) + return cleaned + +def cleanup(filepath, remove_meta): + # Keys that are exported to the policy file, and so + # can be removed from the xml afterward + to_remove = ('format', 'display_name', + 'graceperiod', 'showanswer', 'rerandomize', + 'start', 'due', 'graded', 'hide_from_toc', + 'ispublic', 'xqa_key') + + try: + print "Cleaning {}".format(filepath) + with open(filepath) as f: + parser = etree.XMLParser(remove_comments=False) + xml = etree.parse(filepath, parser=parser) + except: + print "Error parsing file {}".format(filepath) + return + + for node in xml.iter(tag=etree.Element): + attrs = node.attrib + if 'url_name' in attrs: + used_names[node.tag].add(attrs['url_name']) + if 'name' in attrs: + # Replace name with an identical display_name, and a unique url_name + name = attrs['name'] + attrs['display_name'] = name + attrs['url_name'] = clean_unique(node.tag, name) + del attrs['name'] + + if 'url_name' in attrs and 'slug' in attrs: + print "WARNING: {} has both slug and url_name" + + if ('url_name' in attrs and 'filename' in attrs and + len(attrs)==2 and attrs['url_name'] == attrs['filename']): + # This is a pointer tag in disguise. Get rid of the filename. + print 'turning {}.{} into a pointer tag'.format(node.tag, attrs['url_name']) + del attrs['filename'] + + if remove_meta: + for attr in to_remove: + if attr in attrs: + del attrs[attr] + + + with open(filepath, "w") as f: + f.write(etree.tostring(xml)) + + +def find_replace(directory, filePattern, remove_meta): + for path, dirs, files in os.walk(os.path.abspath(directory)): + for filename in fnmatch.filter(files, filePattern): + filepath = os.path.join(path, filename) + cleanup(filepath, remove_meta) + + +def main(args): + usage = "xml_cleanup [dir] [remove-meta]" + n = len(args) + if n < 1 or n > 2 or (n == 2 and args[1] != 'remove-meta'): + print usage + return + + remove_meta = False + if n == 2: + remove_meta = True + + find_replace(args[0], '*.xml', remove_meta) + + +if __name__ == '__main__': + main(sys.argv[1:]) + + From 61478f4ba0202a97ee3cb1c9969047c19054eb33 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 16 Aug 2012 11:46:56 -0400 Subject: [PATCH 41/46] fix string format bug --- common/lib/xmodule/xmodule/backcompat_module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/backcompat_module.py b/common/lib/xmodule/xmodule/backcompat_module.py index 06713d3432..ed2bdb837a 100644 --- a/common/lib/xmodule/xmodule/backcompat_module.py +++ b/common/lib/xmodule/xmodule/backcompat_module.py @@ -92,7 +92,7 @@ class TranslateCustomTagDescriptor(XModuleDescriptor): xml_object = etree.fromstring(xml_data) system.error_tracker('WARNING: the <{tag}> tag is deprecated. ' 'Instead, use . ' - .format(xml_object.tag)) + .format(tag=xml_object.tag)) tag = xml_object.tag xml_object.tag = 'customtag' From 945632362b410f6cccaf7279910ebd7ecb61d52d Mon Sep 17 00:00:00 2001 From: Matthew Mongeau Date: Thu, 16 Aug 2012 11:49:03 -0400 Subject: [PATCH 42/46] Get rid of TextbookModule and TextbookDescriptor. --- common/lib/xmodule/xmodule/course_module.py | 35 +++++++++++++++---- common/lib/xmodule/xmodule/textbook_module.py | 33 ----------------- 2 files changed, 28 insertions(+), 40 deletions(-) delete mode 100644 common/lib/xmodule/xmodule/textbook_module.py diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 8674b8b443..b04a0cdf69 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -1,22 +1,38 @@ from fs.errors import ResourceNotFoundError import time import logging +from lxml import etree from xmodule.util.decorators import lazyproperty from xmodule.graders import load_grading_policy from xmodule.modulestore import Location from xmodule.seq_module import SequenceDescriptor, SequenceModule from xmodule.timeparse import parse_time, stringify_time -from xmodule.textbook_module import TextbookDescriptor log = logging.getLogger(__name__) - class CourseDescriptor(SequenceDescriptor): module_class = SequenceModule + class Textbook: + def __init__(self, title, table_of_contents_url): + self.title = title + self.table_of_contents_url = table_of_contents_url + + @classmethod + def from_xml_object(cls, xml_object): + return cls(xml_object.get('title'), xml_object.get('table_of_contents_url')) + + @property + def table_of_contents(self): + raw_table_of_contents = open(self.table_of_contents_url, 'r') # TODO: This will need to come from S3 + table_of_contents = etree.parse(raw_table_of_contents).getroot() + return table_of_contents + + def __init__(self, system, definition=None, **kwargs): super(CourseDescriptor, self).__init__(system, definition, **kwargs) + self.textbooks = self.definition['textbooks'] msg = None if self.start is None: @@ -29,6 +45,16 @@ class CourseDescriptor(SequenceDescriptor): self.enrollment_start = self._try_parse_time("enrollment_start") self.enrollment_end = self._try_parse_time("enrollment_end") + @classmethod + def definition_from_xml(cls, xml_object, system): + textbooks = [] + for textbook in xml_object.findall("textbook"): + textbooks.append(cls.Textbook.from_xml_object(textbook)) + xml_object.remove(textbook) + definition = super(CourseDescriptor, cls).definition_from_xml(xml_object, system) + definition['textbooks'] = textbooks + return definition + def has_started(self): return time.gmtime() > self.start @@ -54,11 +80,6 @@ class CourseDescriptor(SequenceDescriptor): return grading_policy - @property - def textbooks(self): - return [child for child in self.get_children() if type(child) == TextbookDescriptor] - - @lazyproperty def grading_context(self): """ diff --git a/common/lib/xmodule/xmodule/textbook_module.py b/common/lib/xmodule/xmodule/textbook_module.py deleted file mode 100644 index e0d0730627..0000000000 --- a/common/lib/xmodule/xmodule/textbook_module.py +++ /dev/null @@ -1,33 +0,0 @@ -from xmodule.x_module import XModule -from xmodule.xml_module import XmlDescriptor -from lxml import etree - -class TextbookModule(XModule): - def __init__(self, system, location, definition, descriptor, instance_state=None, - shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, descriptor, - instance_state, shared_state, **kwargs) - - def get_display_items(self): - return [] - - def displayable_items(self): - return [] - -class TextbookDescriptor(XmlDescriptor): - - module_class = TextbookModule - - def __init__(self, system, definition=None, **kwargs): - super(TextbookDescriptor, self).__init__(system, definition, **kwargs) - self.title = self.metadata["title"] - - @classmethod - def definition_from_xml(cls, xml_object, system): - return { 'children': [] } - - @property - def table_of_contents(self): - raw_table_of_contents = open(self.metadata['table_of_contents_url'], 'r') # TODO: This will need to come from S3 - table_of_contents = etree.parse(raw_table_of_contents).getroot() - return table_of_contents From 7553dac2f36a1fcd21bb23e7f99bff63b6577f54 Mon Sep 17 00:00:00 2001 From: ichuang Date: Thu, 16 Aug 2012 11:50:04 -0400 Subject: [PATCH 43/46] etree parser needs to be set on each call in modulestore load_course --- common/lib/xmodule/xmodule/modulestore/xml.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 0c0d750882..5fed33220a 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -6,6 +6,7 @@ import re from fs.osfs import OSFS from importlib import import_module from lxml import etree +from lxml.html import HtmlComment from path import path from xmodule.errortracker import ErrorLog, make_error_tracker from xmodule.x_module import XModuleDescriptor, XMLParsingSystem @@ -16,9 +17,10 @@ from cStringIO import StringIO from . import ModuleStoreBase, Location from .exceptions import ItemNotFoundError -etree.set_default_parser( - etree.XMLParser(dtd_validation=False, load_dtd=False, - remove_comments=True, remove_blank_text=True)) +edx_xml_parser = etree.XMLParser(dtd_validation=False, load_dtd=False, + remove_comments=True, remove_blank_text=True) + +etree.set_default_parser(edx_xml_parser) log = logging.getLogger('mitx.' + __name__) @@ -209,7 +211,7 @@ class XMLModuleStore(ModuleStoreBase): # been imported into the cms from xml course_file = StringIO(clean_out_mako_templating(course_file.read())) - course_data = etree.parse(course_file).getroot() + course_data = etree.parse(course_file,parser=edx_xml_parser).getroot() org = course_data.get('org') From 4ff80287f7bd0e88c87cba546e03bb39b5dbca8b Mon Sep 17 00:00:00 2001 From: Matthew Mongeau Date: Thu, 16 Aug 2012 11:52:10 -0400 Subject: [PATCH 44/46] Remove TextbookDescriptor from setup. --- common/lib/xmodule/setup.py | 1 - 1 file changed, 1 deletion(-) diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index 31918c0250..8a0a6bb139 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -31,7 +31,6 @@ setup( "section = xmodule.backcompat_module:SemanticSectionDescriptor", "sequential = xmodule.seq_module:SequenceDescriptor", "slides = xmodule.backcompat_module:TranslateCustomTagDescriptor", - "textbook = xmodule.textbook_module:TextbookDescriptor", "vertical = xmodule.vertical_module:VerticalDescriptor", "video = xmodule.video_module:VideoDescriptor", "videodev = xmodule.backcompat_module:TranslateCustomTagDescriptor", From 24ed4a4daeca4e59c0ccc412c8c97baf33558f4a Mon Sep 17 00:00:00 2001 From: Kyle Fiedler Date: Thu, 16 Aug 2012 12:06:20 -0400 Subject: [PATCH 45/46] MAnage images --- lms/static/images/askbot/comment-vote-up.png | Bin 0 -> 191 bytes .../askbot/old/vote-arrow-down-activate.png | Bin 214 -> 0 bytes lms/static/images/askbot/old/vote-arrow-down.png | Bin 216 -> 0 bytes .../images/askbot/old/vote-arrow-up-activate.png | Bin 211 -> 0 bytes lms/static/images/askbot/old/vote-arrow-up.png | Bin 200 -> 0 bytes 5 files changed, 0 insertions(+), 0 deletions(-) create mode 100644 lms/static/images/askbot/comment-vote-up.png delete mode 100644 lms/static/images/askbot/old/vote-arrow-down-activate.png delete mode 100644 lms/static/images/askbot/old/vote-arrow-down.png delete mode 100644 lms/static/images/askbot/old/vote-arrow-up-activate.png delete mode 100644 lms/static/images/askbot/old/vote-arrow-up.png diff --git a/lms/static/images/askbot/comment-vote-up.png b/lms/static/images/askbot/comment-vote-up.png new file mode 100644 index 0000000000000000000000000000000000000000..05dc84e12e306a2cedc65c3835669bc4199a1fb6 GIT binary patch literal 191 zcmeAS@N?(olHy`uVBq!ia0vp^0wB!61|;P_|4#%`ZJsWUAr-gYUQ*<14&Z5cXg{NJ zUBeAewFxnd*M7-1H5at*OidDU-NDJUcWt$b>xLHbX^}_Ne=qm#n)x@C--YRkOu#x0 zUD22C85cSlmNQQ}%Q(e5f#ZFr`zv3@b8`gdeSRQQ^uwN67#H literal 0 HcmV?d00001 diff --git a/lms/static/images/askbot/old/vote-arrow-down-activate.png b/lms/static/images/askbot/old/vote-arrow-down-activate.png deleted file mode 100644 index 354c49dca6d3ad51b074b6a392a8780231c9c28b..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 214 zcmeAS@N?(olHy`uVBq!ia0vp^!a&T!!3HE7F0?fPskxpmjv*Dd)?VJo*FN zqJ1!IgYRpha}OBaJ>=Quq{Y5L=#ImNQU>-7n}k|^h_n_6Bwu;tUs^MvU}KBG{!*Da z8@W39ry4z5_)ztV^7BJ_6BgbSm=&NBAIuiaoZ-q@Ys@9azp-Zi^+di`CIQVkqHYHT ze7Q1Q_GOudBs7IF>1yn|z1iA8J;E)m$3S|k`jPp67-S_DNbTHplQ)rHX+f&ghs8jL OGI+ZBxvXYBrEKTBxYN zF_$ZrJ2tkubIw8LFLLt^hQ0`}X_)+gmn+iyxtv1q@+N`#52Bee&piJSFSjTD*__k& zo?TSjzPPNgL-v9Kn@Em?fFo|KA{Et9+p$gh#o; z&5WV;!o~jr+>KJca$lUUA83ydSY!KU+nXStg@vu}?x;!}6n+(`c;Lxdqkr2zFyA*a Q2RfC()78&qol`;+09svAV*mgE diff --git a/lms/static/images/askbot/old/vote-arrow-up-activate.png b/lms/static/images/askbot/old/vote-arrow-up-activate.png deleted file mode 100644 index aa411c70e14f5678bf656179be61f87a07fb5f78..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 211 zcmV;^04)EBP)fsdMzI#hMKKm4)X;WG53GGpa2zi!&m|R;M&LjrUTfsNg(Q_y>S2l N002ovPDHLkV1kv$RlxuN diff --git a/lms/static/images/askbot/old/vote-arrow-up.png b/lms/static/images/askbot/old/vote-arrow-up.png deleted file mode 100644 index a35946cc5100ad2fcf91e7f5aea14eb79633864f..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 200 zcmV;(05|`MP)`OK?rd?H-2_k7H;8p0jHme>iJDc8m_3{GrBzBHEL4Z+pnV2uU1K^_l;mRJcH zRKH?5M#!l67E94V3-ma;u@Eh^!V~d4c7`^%h|UBw%z;zdsUGeC0000 Date: Thu, 16 Aug 2012 12:06:37 -0400 Subject: [PATCH 46/46] Nest textbooks under data --- common/lib/xmodule/xmodule/course_module.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index b04a0cdf69..d003ff42fd 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -32,7 +32,7 @@ class CourseDescriptor(SequenceDescriptor): def __init__(self, system, definition=None, **kwargs): super(CourseDescriptor, self).__init__(system, definition, **kwargs) - self.textbooks = self.definition['textbooks'] + self.textbooks = self.definition['data']['textbooks'] msg = None if self.start is None: @@ -52,7 +52,7 @@ class CourseDescriptor(SequenceDescriptor): textbooks.append(cls.Textbook.from_xml_object(textbook)) xml_object.remove(textbook) definition = super(CourseDescriptor, cls).definition_from_xml(xml_object, system) - definition['textbooks'] = textbooks + definition.setdefault('data', {})['textbooks'] = textbooks return definition def has_started(self):