From d8a1b5d574787bb5c3dfa6770a8cd538676586f8 Mon Sep 17 00:00:00 2001 From: Arjun Singh Date: Wed, 8 Aug 2012 02:55:32 -0400 Subject: [PATCH 01/34] adding some styling so that the syllabus lists display reasonably --- lms/static/sass/multicourse/_course_about.scss | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lms/static/sass/multicourse/_course_about.scss b/lms/static/sass/multicourse/_course_about.scss index fcf4382109..92d19dff86 100644 --- a/lms/static/sass/multicourse/_course_about.scss +++ b/lms/static/sass/multicourse/_course_about.scss @@ -274,6 +274,17 @@ } } } + + .prerequisites, .syllabus { + ul { + li { + font: normal 1em/1.6em $serif; + } + ul { + margin: 5px 0px 10px; + } + } + } .faq { @include clearfix; From 7dd7de9566c8c7688319c401a6395be3cb1ab8ab Mon Sep 17 00:00:00 2001 From: ichuang Date: Fri, 17 Aug 2012 15:57:12 +0000 Subject: [PATCH 02/34] add gitreload to lms migration, so it doesn't have to run separately --- lms/djangoapps/lms_migration/migrate.py | 84 ++++++++++++++++++++++++- lms/urls.py | 2 + 2 files changed, 83 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/lms_migration/migrate.py b/lms/djangoapps/lms_migration/migrate.py index dfdf86b4ac..6b9d01c81c 100644 --- a/lms/djangoapps/lms_migration/migrate.py +++ b/lms/djangoapps/lms_migration/migrate.py @@ -2,13 +2,21 @@ # migration tools for content team to go from stable-edx4edx to LMS+CMS # +import json import logging +import os from pprint import pprint import xmodule.modulestore.django as xmodule_django from xmodule.modulestore.django import modulestore from django.http import HttpResponse from django.conf import settings +import track.views + +try: + from django.views.decorators.csrf import csrf_exempt +except ImportError: + from django.contrib.csrf.middleware import csrf_exempt log = logging.getLogger("mitx.lms_migrate") LOCAL_DEBUG = True @@ -18,6 +26,15 @@ def escape(s): """escape HTML special characters in string""" return str(s).replace('<','<').replace('>','>') +def getip(request): + ''' + Extract IP address of requester from header, even if behind proxy + ''' + ip = request.META.get('HTTP_X_REAL_IP','') # nginx reverse proxy + if not ip: + ip = request.META.get('REMOTE_ADDR','None') + return ip + def manage_modulestores(request,reload_dir=None): ''' Manage the static in-memory modulestores. @@ -32,9 +49,7 @@ def manage_modulestores(request,reload_dir=None): #---------------------------------------- # check on IP address of requester - ip = request.META.get('HTTP_X_REAL_IP','') # nginx reverse proxy - if not ip: - ip = request.META.get('REMOTE_ADDR','None') + ip = getip(request) if LOCAL_DEBUG: html += '

IP address: %s ' % ip @@ -108,3 +123,66 @@ def manage_modulestores(request,reload_dir=None): html += "" return HttpResponse(html) + +@csrf_exempt +def gitreload(request, reload_dir=None): + ''' + This can be used as a github WebHook Service Hook, for reloading of the content repo used by the LMS. + + If reload_dir is not None, then instruct the xml loader to reload that course directory. + ''' + html = "" + ip = getip(request) + + html += '

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

User: %s ' % request.user + + ALLOWED_IPS = ['207.97.227.253', '50.57.128.197', '108.171.174.178'] # hardcoded to github + if hasattr(settings,'ALLOWED_GITRELOAD_IPS'): # allow override in settings + ALLOWED_IPS = ALLOWED_GITRELOAD_IPS + + if not (ip in ALLOWED_IPS or 'any' in ALLOWED_IPS): + if request.user and request.user.is_staff: + log.debug('request allowed because user=%s is staff' % request.user) + else: + html += 'Permission denied' + html += "" + log.debug('request denied from %s, ALLOWED_IPS=%s' % (ip,ALLOWED_IPS)) + return HttpResponse(html) + + #---------------------------------------- + # see if request is from github (POST with JSON) + + if reload_dir is None and 'payload' in request.POST: + payload = request.POST['payload'] + log.debug("payload=%s" % payload) + gitargs = json.loads(payload) + log.debug("gitargs=%s" % gitargs) + reload_dir = gitargs['repository']['name'] + log.debug("github reload_dir=%s" % reload_dir) + gdir = settings.DATA_DIR / reload_dir + if not os.path.exists(gdir): + log.debug("====> ERROR in gitreload - no such directory %s" % reload_dir) + return HttpResponse('Error') + cmd = "cd %s; git reset --hard HEAD; git clean -f -d; git pull origin; chmod g+w course.xml" % gdir + log.debug(os.popen(cmd).read()) + if hasattr(settings,'GITRELOAD_HOOK'): # hit this hook after reload, if set + gh = settings.GITRELOAD_HOOK + if gh: + ghurl = '%s/%s' % (gh,reload_dir) + r = requests.get(ghurl) + log.debug("GITRELOAD_HOOK to %s: %s" % (ghurl, r.text)) + + #---------------------------------------- + # reload course if specified + + if reload_dir is not None: + def_ms = modulestore() + if reload_dir not in def_ms.courses: + html += "

Error: '%s' is not a valid course directory

" % reload_dir + else: + html += "

Reloaded course directory '%s'

" % reload_dir + def_ms.try_load_course(reload_dir) + track.views.server_track(request, 'reloaded %s' % reload_dir, {}, page='migrate') + + return HttpResponse(html) diff --git a/lms/urls.py b/lms/urls.py index 89ef4babc4..1ed04d1a07 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -204,6 +204,8 @@ if settings.MITX_FEATURES.get('ENABLE_LMS_MIGRATION'): urlpatterns += ( url(r'^migrate/modules$', 'lms_migration.migrate.manage_modulestores'), url(r'^migrate/reload/(?P[^/]+)$', 'lms_migration.migrate.manage_modulestores'), + url(r'^gitreload$', 'lms_migration.migrate.gitreload'), + url(r'^gitreload/(?P[^/]+)$', 'lms_migration.migrate.gitreload'), ) if settings.MITX_FEATURES.get('ENABLE_SQL_TRACKING_LOGS'): From 4365b4a092faa5d90cb22022132bb41e61ebf09b Mon Sep 17 00:00:00 2001 From: ichuang Date: Fri, 17 Aug 2012 16:17:00 +0000 Subject: [PATCH 03/34] more flexible event logs retrieval --- common/djangoapps/track/views.py | 18 +++++++++++++++--- lms/urls.py | 1 + 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/common/djangoapps/track/views.py b/common/djangoapps/track/views.py index b5f9c54665..910ed25d4b 100644 --- a/common/djangoapps/track/views.py +++ b/common/djangoapps/track/views.py @@ -84,15 +84,27 @@ def server_track(request, event_type, event, page=None): "time": datetime.datetime.utcnow().isoformat(), } - if event_type=="/event_logs" and request.user.is_staff: # don't log + if event_type.startswith("/event_logs") and request.user.is_staff: # don't log return log_event(event) @login_required @ensure_csrf_cookie -def view_tracking_log(request): +def view_tracking_log(request,args=''): if not request.user.is_staff: return redirect('/') - record_instances = TrackingLog.objects.all().order_by('-time')[0:100] + nlen = 100 + username = '' + if args: + for arg in args.split('/'): + if arg.isdigit(): + nlen = int(arg) + if arg.startswith('username='): + username = arg[9:] + + record_instances = TrackingLog.objects.all().order_by('-time') + if username: + record_instances = record_instances.filter(username=username) + record_instances = record_instances[0:nlen] return render_to_response('tracking_log.html',{'records':record_instances}) diff --git a/lms/urls.py b/lms/urls.py index 1ed04d1a07..776f08b49a 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -211,6 +211,7 @@ if settings.MITX_FEATURES.get('ENABLE_LMS_MIGRATION'): if settings.MITX_FEATURES.get('ENABLE_SQL_TRACKING_LOGS'): urlpatterns += ( url(r'^event_logs$', 'track.views.view_tracking_log'), + url(r'^event_logs/(?P.+)$', 'track.views.view_tracking_log'), ) urlpatterns = patterns(*urlpatterns) From a6c6d0f4d14836aa8bc528b5305eefc91f50a496 Mon Sep 17 00:00:00 2001 From: ichuang Date: Fri, 17 Aug 2012 14:49:59 -0400 Subject: [PATCH 04/34] MITX_FEATURES['USE_DJANGO_PIPELINE']=False in dev_ike by default --- lms/envs/dev_ike.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lms/envs/dev_ike.py b/lms/envs/dev_ike.py index 309ea1ac42..a9bd3ef97c 100644 --- a/lms/envs/dev_ike.py +++ b/lms/envs/dev_ike.py @@ -17,6 +17,9 @@ MITX_FEATURES['ENABLE_TEXTBOOK'] = False MITX_FEATURES['ENABLE_DISCUSSION'] = False MITX_FEATURES['ACCESS_REQUIRE_STAFF_FOR_COURSE'] = True # require that user be in the staff_* group to be able to enroll +MITX_FEATURES['DISABLE_START_DATES'] = True +MITX_FEATURES['USE_DJANGO_PIPELINE']=False # don't recompile scss + myhost = socket.gethostname() if ('edxvm' in myhost) or ('ocw' in myhost): MITX_FEATURES['DISABLE_LOGIN_BUTTON'] = True # auto-login with MIT certificate @@ -33,4 +36,9 @@ SECURE_PROXY_SSL_HEADER = ('HTTP_X_FORWARDED_PROTOCOL', 'https') # django 1.4 fo INSTALLED_APPS = tuple([ app for app in INSTALLED_APPS if not app.startswith('debug_toolbar') ]) MIDDLEWARE_CLASSES = tuple([ mcl for mcl in MIDDLEWARE_CLASSES if not mcl.startswith('debug_toolbar') ]) -TEMPLATE_LOADERS = tuple([ app for app in TEMPLATE_LOADERS if not app.startswith('askbot') ]) +#TEMPLATE_LOADERS = tuple([ app for app in TEMPLATE_LOADERS if not app.startswith('askbot') ]) +#TEMPLATE_LOADERS = tuple([ app for app in TEMPLATE_LOADERS if not app.startswith('mitxmako') ]) +TEMPLATE_LOADERS = ( + 'django.template.loaders.filesystem.Loader', + 'django.template.loaders.app_directories.Loader', + ) From f7d4831939dc70e8afe7a51e38e332042b7a5641 Mon Sep 17 00:00:00 2001 From: ichuang Date: Fri, 17 Aug 2012 15:20:19 -0400 Subject: [PATCH 05/34] update utility scripts for djang-1.4; will make them management cmds later --- utility-scripts/create_groups.py | 13 +++-- utility-scripts/create_user.py | 12 +++-- utility-scripts/manage_class_groups.py | 74 ++++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 10 deletions(-) create mode 100644 utility-scripts/manage_class_groups.py diff --git a/utility-scripts/create_groups.py b/utility-scripts/create_groups.py index 0e3245bb4d..8108498cb8 100644 --- a/utility-scripts/create_groups.py +++ b/utility-scripts/create_groups.py @@ -9,17 +9,20 @@ import os, sys, string, re sys.path.append(os.path.abspath('.')) os.environ['DJANGO_SETTINGS_MODULE'] = 'lms.envs.dev' -try: - from lms.envs.dev import * -except Exception as err: - print "Run this script from the top-level mitx directory (mitx_all/mitx), not a subdirectory." - sys.exit(-1) +#try: +# from lms.envs.dev import * +#except Exception as err: +# print "Run this script from the top-level mitx directory (mitx_all/mitx), not a subdirectory." +# sys.exit(-1) from django.conf import settings from django.contrib.auth.models import User, Group from path import path from lxml import etree +print "configured=",settings.configured +print settings.SETTINGS_MODULE + data_dir = settings.DATA_DIR print "data_dir = %s" % data_dir diff --git a/utility-scripts/create_user.py b/utility-scripts/create_user.py index 3ce9ce0ecf..5a52baff34 100644 --- a/utility-scripts/create_user.py +++ b/utility-scripts/create_user.py @@ -13,11 +13,13 @@ import readline sys.path.append(os.path.abspath('.')) os.environ['DJANGO_SETTINGS_MODULE'] = 'lms.envs.dev' -try: - from lms.envs.dev import * -except Exception as err: - print "Run this script from the top-level mitx directory (mitx_all/mitx), not a subdirectory." - sys.exit(-1) +#try: +# from lms.envs.dev import * +#except Exception as err: +# print "Run this script from the top-level mitx directory (mitx_all/mitx), not a subdirectory." +# sys.exit(-1) + +sys.path.append(os.path.abspath('common/djangoapps')) from student.models import UserProfile, Registration from external_auth.models import ExternalAuthMap diff --git a/utility-scripts/manage_class_groups.py b/utility-scripts/manage_class_groups.py new file mode 100644 index 0000000000..a21ec9151e --- /dev/null +++ b/utility-scripts/manage_class_groups.py @@ -0,0 +1,74 @@ +#!/usr/bin/python +# +# File: manage_class_groups +# +# list and edit membership in class staff group + +import os, sys, string, re +import datetime +from getpass import getpass +import json +import readline + +sys.path.append(os.path.abspath('.')) +os.environ['DJANGO_SETTINGS_MODULE'] = 'lms.envs.dev' + +#try: +# from lms.envs.dev import * +#except Exception as err: +# print "Run this script from the top-level mitx directory (mitx_all/mitx), not a subdirectory." +# sys.exit(-1) + +from django.conf import settings +from django.contrib.auth.models import User, Group + +#----------------------------------------------------------------------------- +# get all staff groups + +gset = Group.objects.all() + +print "Groups:" +for cnt,g in zip(range(len(gset)), gset): + print "%d. %s" % (cnt,g) + +gnum = int(raw_input('Choose group to manage (enter #): ')) + +group = gset[gnum] + +#----------------------------------------------------------------------------- +# users in group + +uall = User.objects.all() +print "----" +print "List of All Users: %s" % [str(x.username) for x in uall] +print "----" + +while True: + + print "Users in the group:" + + uset = group.user_set.all() + for cnt, u in zip(range(len(uset)), uset): + print "%d. %s" % (cnt, u) + + action = raw_input('Choose user to delete (enter #) or enter usernames (comma delim) to add: ') + + m = re.match('^[0-9]+$',action) + if m: + unum = int(action) + u = uset[unum] + print "Deleting user %s" % u + u.groups.remove(group) + + else: + for uname in action.split(','): + try: + user = User.objects.get(username=action) + except Exception as err: + print "Error %s" % err + continue + print "adding %s to group %s" % (user, group) + user.groups.add(group) + + + From b6e41dc05c56761acdd2e9c84b492e5027674de3 Mon Sep 17 00:00:00 2001 From: ichuang Date: Fri, 17 Aug 2012 15:46:09 -0400 Subject: [PATCH 06/34] fix view_tracking_log to show proper timezone --- common/djangoapps/track/views.py | 7 +++++++ lms/templates/tracking_log.html | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/track/views.py b/common/djangoapps/track/views.py index 910ed25d4b..434e75a63f 100644 --- a/common/djangoapps/track/views.py +++ b/common/djangoapps/track/views.py @@ -1,6 +1,7 @@ import json import logging import os +import pytz import datetime import dateutil.parser @@ -106,5 +107,11 @@ def view_tracking_log(request,args=''): if username: record_instances = record_instances.filter(username=username) record_instances = record_instances[0:nlen] + + # fix dtstamp + fmt = '%a %d-%b-%y %H:%M:%S' # "%Y-%m-%d %H:%M:%S %Z%z" + for rinst in record_instances: + rinst.dtstr = rinst.time.replace(tzinfo=pytz.utc).astimezone(pytz.timezone('US/Eastern')).strftime(fmt) + return render_to_response('tracking_log.html',{'records':record_instances}) diff --git a/lms/templates/tracking_log.html b/lms/templates/tracking_log.html index 66d375c2f3..24b249a583 100644 --- a/lms/templates/tracking_log.html +++ b/lms/templates/tracking_log.html @@ -3,7 +3,7 @@ % for rec in records: - + From b23bfc75bc33fbe0c3d589f8e8d259c5799a5ab2 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 19 Aug 2012 23:18:06 -0400 Subject: [PATCH 07/34] additional minor change: allow "inline" attrib on textline --- common/lib/capa/capa/inputtypes.py | 4 +++- common/lib/capa/capa/templates/textinput.html | 12 ++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py index 8c513e7aec..9de0a52e15 100644 --- a/common/lib/capa/capa/inputtypes.py +++ b/common/lib/capa/capa/inputtypes.py @@ -293,7 +293,9 @@ def textline(element, value, status, render_template, msg=""): hidden = element.get('hidden', '') # if specified, then textline is hidden and id is stored in div of name given by hidden escapedict = {'"': '"'} value = saxutils.escape(value, escapedict) # otherwise, answers with quotes in them crashes the system! - context = {'id': eid, 'value': value, 'state': status, 'count': count, 'size': size, 'msg': msg, 'hidden': hidden} + context = {'id': eid, 'value': value, 'state': status, 'count': count, 'size': size, 'msg': msg, 'hidden': hidden, + 'inline': element.get('inline',''), + } html = render_template("textinput.html", context) try: xhtml = etree.XML(html) diff --git a/common/lib/capa/capa/templates/textinput.html b/common/lib/capa/capa/templates/textinput.html index 08aa8379a7..814517d11c 100644 --- a/common/lib/capa/capa/templates/textinput.html +++ b/common/lib/capa/capa/templates/textinput.html @@ -1,6 +1,14 @@ -
+
% if state == 'unsubmitted': -
+
% elif state == 'correct':
% elif state == 'incorrect': From 660c35f63eecf84fe329d26ec3493663494b7be4 Mon Sep 17 00:00:00 2001 From: ichuang Date: Mon, 20 Aug 2012 00:19:28 -0400 Subject: [PATCH 08/34] additional changes to make inline textinput work properly --- common/lib/capa/capa/templates/textinput.html | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/common/lib/capa/capa/templates/textinput.html b/common/lib/capa/capa/templates/textinput.html index 814517d11c..63c0609107 100644 --- a/common/lib/capa/capa/templates/textinput.html +++ b/common/lib/capa/capa/templates/textinput.html @@ -10,11 +10,23 @@ % endif > % elif state == 'correct': -
+
% elif state == 'incorrect': -
+
% elif state == 'incomplete': -
+
% endif % if hidden:
From e3ecd22fa052fcce89b84d2cd8da662173317704 Mon Sep 17 00:00:00 2001 From: ichuang Date: Mon, 20 Aug 2012 18:38:05 -0400 Subject: [PATCH 09/34] github IPs in lms.envs.common; made create_user, create_groups, manage_course_groups management commands (in lms_migration) --- .../lms_migration/management/__init__.py | 0 .../management/commands/__init__.py | 0 .../management/commands/create_groups.py | 58 +++++++ .../management/commands/create_user.py | 146 +++++++++++++++++ lms/djangoapps/lms_migration/migrate.py | 4 +- lms/envs/common.py | 8 + lms/envs/dev.py | 2 + utility-scripts/create_groups.py | 49 ------ utility-scripts/create_user.py | 151 ------------------ utility-scripts/manage_class_groups.py | 74 --------- 10 files changed, 216 insertions(+), 276 deletions(-) create mode 100644 lms/djangoapps/lms_migration/management/__init__.py create mode 100644 lms/djangoapps/lms_migration/management/commands/__init__.py create mode 100644 lms/djangoapps/lms_migration/management/commands/create_groups.py create mode 100644 lms/djangoapps/lms_migration/management/commands/create_user.py delete mode 100644 utility-scripts/create_groups.py delete mode 100644 utility-scripts/create_user.py delete mode 100644 utility-scripts/manage_class_groups.py diff --git a/lms/djangoapps/lms_migration/management/__init__.py b/lms/djangoapps/lms_migration/management/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/lms_migration/management/commands/__init__.py b/lms/djangoapps/lms_migration/management/commands/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/lms_migration/management/commands/create_groups.py b/lms/djangoapps/lms_migration/management/commands/create_groups.py new file mode 100644 index 0000000000..7b52795606 --- /dev/null +++ b/lms/djangoapps/lms_migration/management/commands/create_groups.py @@ -0,0 +1,58 @@ +#!/usr/bin/python +# +# File: create_groups.py +# +# Create all staff_* groups for classes in data directory. + +import os, sys, string, re + +from django.core.management.base import BaseCommand +from django.conf import settings +from django.contrib.auth.models import User, Group +from path import path +from lxml import etree + +def create_groups(): + ''' + Create staff and instructor groups for all classes in the data_dir + ''' + + data_dir = settings.DATA_DIR + print "data_dir = %s" % data_dir + + for course_dir in os.listdir(data_dir): + + if course_dir.startswith('.'): + continue + if not os.path.isdir(path(data_dir) / course_dir): + continue + + cxfn = path(data_dir) / course_dir / 'course.xml' + try: + coursexml = etree.parse(cxfn) + except Exception as err: + print "Oops, cannot read %s, skipping" % cxfn + continue + cxmlroot = coursexml.getroot() + course = cxmlroot.get('course') # TODO (vshnayder!!): read metadata from policy file(s) instead of from course.xml + if course is None: + print "oops, can't get course id for %s" % course_dir + continue + print "course=%s for course_dir=%s" % (course,course_dir) + + create_group('staff_%s' % course) # staff group + create_group('instructor_%s' % course) # instructor group (can manage staff group list) + +def create_group(gname): + if Group.objects.filter(name=gname): + print " group exists for %s" % gname + return + g = Group(name=gname) + g.save() + print " created group %s" % gname + +class Command(BaseCommand): + help = "Create groups associated with all courses in data_dir." + + def handle(self, *args, **options): + create_groups() diff --git a/lms/djangoapps/lms_migration/management/commands/create_user.py b/lms/djangoapps/lms_migration/management/commands/create_user.py new file mode 100644 index 0000000000..333608d467 --- /dev/null +++ b/lms/djangoapps/lms_migration/management/commands/create_user.py @@ -0,0 +1,146 @@ +#!/usr/bin/python +# +# File: create_user.py +# +# Create user. Prompt for groups and ExternalAuthMap + +import os, sys, string, re +import datetime +from getpass import getpass +import json +from random import choice +import readline + +from django.core.management.base import BaseCommand +from student.models import UserProfile, Registration +from external_auth.models import ExternalAuthMap +from django.contrib.auth.models import User, Group + +class MyCompleter(object): # Custom completer + + def __init__(self, options): + self.options = sorted(options) + + def complete(self, text, state): + if state == 0: # on first trigger, build possible matches + if text: # cache matches (entries that start with entered text) + self.matches = [s for s in self.options + if s and s.startswith(text)] + else: # no text entered, all matches possible + self.matches = self.options[:] + + # return match indexed by state + try: + return self.matches[state] + except IndexError: + return None + +def GenPasswd(length=8, chars=string.letters + string.digits): + return ''.join([choice(chars) for i in range(length)]) + +#----------------------------------------------------------------------------- +# main command + +class Command(BaseCommand): + help = "Create user, interactively; can add ExternalAuthMap for MIT user if email@MIT.EDU resolves properly." + + def handle(self, *args, **options): + + while True: + uname = raw_input('username: ') + if User.objects.filter(username=uname): + print "username %s already taken" % uname + else: + break + + make_eamap = False + if raw_input('Create MIT ExternalAuth? [n] ').lower()=='y': + email = '%s@MIT.EDU' % uname + if not email.endswith('@MIT.EDU'): + print "Failed - email must be @MIT.EDU" + sys.exit(-1) + mit_domain = 'ssl:MIT' + if ExternalAuthMap.objects.filter(external_id = email, external_domain = mit_domain): + print "Failed - email %s already exists as external_id" % email + sys.exit(-1) + make_eamap = True + password = GenPasswd(12) + + # get name from kerberos + kname = os.popen("finger %s | grep 'name:'" % email).read().strip().split('name: ')[1].strip() + name = raw_input('Full name: [%s] ' % kname).strip() + if name=='': + name = kname + print "name = %s" % name + else: + while True: + password = getpass() + password2 = getpass() + if password == password2: + break + print "Oops, passwords do not match, please retry" + + while True: + email = raw_input('email: ') + if User.objects.filter(email=email): + print "email %s already taken" % email + else: + break + + name = raw_input('Full name: ') + + + user = User(username=uname, email=email, is_active=True) + user.set_password(password) + try: + user.save() + except IntegrityError: + print "Oops, failed to create user %s, IntegrityError" % user + raise + + r = Registration() + r.register(user) + + up = UserProfile(user=user) + up.name = name + up.save() + + if make_eamap: + credentials = "/C=US/ST=Massachusetts/O=Massachusetts Institute of Technology/OU=Client CA v1/CN=%s/emailAddress=%s" % (name,email) + eamap = ExternalAuthMap(external_id = email, + external_email = email, + external_domain = mit_domain, + external_name = name, + internal_password = password, + external_credentials = json.dumps(credentials), + ) + eamap.user = user + eamap.dtsignup = datetime.datetime.now() + eamap.save() + + print "User %s created successfully!" % user + + if not raw_input('Add user %s to any groups? [n] ' % user).lower()=='y': + sys.exit(0) + + print "Here are the groups available:" + + groups = [str(g.name) for g in Group.objects.all()] + print groups + + completer = MyCompleter(groups) + readline.set_completer(completer.complete) + readline.parse_and_bind('tab: complete') + + while True: + gname = raw_input("Add group (tab to autocomplete, empty line to end): ") + if not gname: + break + if not gname in groups: + print "Unknown group %s" % gname + continue + g = Group.objects.get(name=gname) + user.groups.add(g) + print "Added %s to group %s" % (user,g) + + print "Done!" diff --git a/lms/djangoapps/lms_migration/migrate.py b/lms/djangoapps/lms_migration/migrate.py index 6b9d01c81c..a7d04a655d 100644 --- a/lms/djangoapps/lms_migration/migrate.py +++ b/lms/djangoapps/lms_migration/migrate.py @@ -63,7 +63,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, status=403) #---------------------------------------- # reload course if specified @@ -137,7 +137,7 @@ def gitreload(request, reload_dir=None): html += '

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

User: %s ' % request.user - ALLOWED_IPS = ['207.97.227.253', '50.57.128.197', '108.171.174.178'] # hardcoded to github + ALLOWED_IPS = [] # allow none by default if hasattr(settings,'ALLOWED_GITRELOAD_IPS'): # allow override in settings ALLOWED_IPS = ALLOWED_GITRELOAD_IPS diff --git a/lms/envs/common.py b/lms/envs/common.py index c412a3c8cd..265c500dbf 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -254,6 +254,14 @@ USE_L10N = True # Messages MESSAGE_STORAGE = 'django.contrib.messages.storage.session.SessionStorage' +#################################### GITHUB ####################################### +# gitreload is used in LMS-workflow to pull content from github +# gitreload requests are only allowed from these IP addresses, which are +# the advertised public IPs of the github WebHook servers. +# These are listed, eg at https://github.com/MITx/mitx/admin/hooks + +ALLOWED_GITRELOAD = ['207.97.227.253', '50.57.128.197', '108.171.174.178'] + #################################### AWS ####################################### # S3BotoStorage insists on a timeout for uploaded assets. We should make it # permanent instead, but rather than trying to figure out exactly where that diff --git a/lms/envs/dev.py b/lms/envs/dev.py index 6720c2050d..b269d293dd 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -73,6 +73,8 @@ MITX_FEATURES['ENABLE_LMS_MIGRATION'] = True MITX_FEATURES['ACCESS_REQUIRE_STAFF_FOR_COURSE'] = False # require that user be in the staff_* group to be able to enroll MITX_FEATURES['USE_XQA_SERVER'] = 'http://xqa:server@content-qa.mitx.mit.edu/xqa' +INSTALLED_APPS += ('lms_migration',) + LMS_MIGRATION_ALLOWED_IPS = ['127.0.0.1'] ################################ OpenID Auth ################################# diff --git a/utility-scripts/create_groups.py b/utility-scripts/create_groups.py deleted file mode 100644 index 8108498cb8..0000000000 --- a/utility-scripts/create_groups.py +++ /dev/null @@ -1,49 +0,0 @@ -#!/usr/bin/python -# -# File: create_groups.py -# -# Create all staff_* groups for classes in data directory. - -import os, sys, string, re - -sys.path.append(os.path.abspath('.')) -os.environ['DJANGO_SETTINGS_MODULE'] = 'lms.envs.dev' - -#try: -# from lms.envs.dev import * -#except Exception as err: -# print "Run this script from the top-level mitx directory (mitx_all/mitx), not a subdirectory." -# sys.exit(-1) - -from django.conf import settings -from django.contrib.auth.models import User, Group -from path import path -from lxml import etree - -print "configured=",settings.configured -print settings.SETTINGS_MODULE - -data_dir = settings.DATA_DIR -print "data_dir = %s" % data_dir - -for course_dir in os.listdir(data_dir): - # print course_dir - if not os.path.isdir(path(data_dir) / course_dir): - continue - - cxfn = path(data_dir) / course_dir / 'course.xml' - coursexml = etree.parse(cxfn) - cxmlroot = coursexml.getroot() - course = cxmlroot.get('course') - if course is None: - print "oops, can't get course id for %s" % course_dir - continue - print "course=%s for course_dir=%s" % (course,course_dir) - - gname = 'staff_%s' % course - if Group.objects.filter(name=gname): - print "group exists for %s" % gname - continue - g = Group(name=gname) - g.save() - print "created group %s" % gname diff --git a/utility-scripts/create_user.py b/utility-scripts/create_user.py deleted file mode 100644 index 5a52baff34..0000000000 --- a/utility-scripts/create_user.py +++ /dev/null @@ -1,151 +0,0 @@ -#!/usr/bin/python -# -# File: create_user.py -# -# Create user. Prompt for groups and ExternalAuthMap - -import os, sys, string, re -import datetime -from getpass import getpass -import json -import readline - -sys.path.append(os.path.abspath('.')) -os.environ['DJANGO_SETTINGS_MODULE'] = 'lms.envs.dev' - -#try: -# from lms.envs.dev import * -#except Exception as err: -# print "Run this script from the top-level mitx directory (mitx_all/mitx), not a subdirectory." -# sys.exit(-1) - -sys.path.append(os.path.abspath('common/djangoapps')) - -from student.models import UserProfile, Registration -from external_auth.models import ExternalAuthMap -from django.contrib.auth.models import User, Group -from random import choice - -class MyCompleter(object): # Custom completer - - def __init__(self, options): - self.options = sorted(options) - - def complete(self, text, state): - if state == 0: # on first trigger, build possible matches - if text: # cache matches (entries that start with entered text) - self.matches = [s for s in self.options - if s and s.startswith(text)] - else: # no text entered, all matches possible - self.matches = self.options[:] - - # return match indexed by state - try: - return self.matches[state] - except IndexError: - return None - -def GenPasswd(length=8, chars=string.letters + string.digits): - return ''.join([choice(chars) for i in range(length)]) - -#----------------------------------------------------------------------------- -# main - -while True: - uname = raw_input('username: ') - if User.objects.filter(username=uname): - print "username %s already taken" % uname - else: - break - -make_eamap = False -if raw_input('Create MIT ExternalAuth? [n] ').lower()=='y': - email = '%s@MIT.EDU' % uname - if not email.endswith('@MIT.EDU'): - print "Failed - email must be @MIT.EDU" - sys.exit(-1) - mit_domain = 'ssl:MIT' - if ExternalAuthMap.objects.filter(external_id = email, external_domain = mit_domain): - print "Failed - email %s already exists as external_id" % email - sys.exit(-1) - make_eamap = True - password = GenPasswd(12) - - # get name from kerberos - kname = os.popen("finger %s | grep 'name:'" % email).read().strip().split('name: ')[1].strip() - name = raw_input('Full name: [%s] ' % kname).strip() - if name=='': - name = kname - print "name = %s" % name -else: - while True: - password = getpass() - password2 = getpass() - if password == password2: - break - print "Oops, passwords do not match, please retry" - - while True: - email = raw_input('email: ') - if User.objects.filter(email=email): - print "email %s already taken" % email - else: - break - - name = raw_input('Full name: ') - - -user = User(username=uname, email=email, is_active=True) -user.set_password(password) -try: - user.save() -except IntegrityError: - print "Oops, failed to create user %s, IntegrityError" % user - raise - -r = Registration() -r.register(user) - -up = UserProfile(user=user) -up.name = name -up.save() - -if make_eamap: - credentials = "/C=US/ST=Massachusetts/O=Massachusetts Institute of Technology/OU=Client CA v1/CN=%s/emailAddress=%s" % (name,email) - eamap = ExternalAuthMap(external_id = email, - external_email = email, - external_domain = mit_domain, - external_name = name, - internal_password = password, - external_credentials = json.dumps(credentials), - ) - eamap.user = user - eamap.dtsignup = datetime.datetime.now() - eamap.save() - -print "User %s created successfully!" % user - -if not raw_input('Add user %s to any groups? [n] ' % user).lower()=='y': - sys.exit(0) - -print "Here are the groups available:" - -groups = [str(g.name) for g in Group.objects.all()] -print groups - -completer = MyCompleter(groups) -readline.set_completer(completer.complete) -readline.parse_and_bind('tab: complete') - -while True: - gname = raw_input("Add group (tab to autocomplete, empty line to end): ") - if not gname: - break - if not gname in groups: - print "Unknown group %s" % gname - continue - g = Group.objects.get(name=gname) - user.groups.add(g) - print "Added %s to group %s" % (user,g) - -print "Done!" diff --git a/utility-scripts/manage_class_groups.py b/utility-scripts/manage_class_groups.py deleted file mode 100644 index a21ec9151e..0000000000 --- a/utility-scripts/manage_class_groups.py +++ /dev/null @@ -1,74 +0,0 @@ -#!/usr/bin/python -# -# File: manage_class_groups -# -# list and edit membership in class staff group - -import os, sys, string, re -import datetime -from getpass import getpass -import json -import readline - -sys.path.append(os.path.abspath('.')) -os.environ['DJANGO_SETTINGS_MODULE'] = 'lms.envs.dev' - -#try: -# from lms.envs.dev import * -#except Exception as err: -# print "Run this script from the top-level mitx directory (mitx_all/mitx), not a subdirectory." -# sys.exit(-1) - -from django.conf import settings -from django.contrib.auth.models import User, Group - -#----------------------------------------------------------------------------- -# get all staff groups - -gset = Group.objects.all() - -print "Groups:" -for cnt,g in zip(range(len(gset)), gset): - print "%d. %s" % (cnt,g) - -gnum = int(raw_input('Choose group to manage (enter #): ')) - -group = gset[gnum] - -#----------------------------------------------------------------------------- -# users in group - -uall = User.objects.all() -print "----" -print "List of All Users: %s" % [str(x.username) for x in uall] -print "----" - -while True: - - print "Users in the group:" - - uset = group.user_set.all() - for cnt, u in zip(range(len(uset)), uset): - print "%d. %s" % (cnt, u) - - action = raw_input('Choose user to delete (enter #) or enter usernames (comma delim) to add: ') - - m = re.match('^[0-9]+$',action) - if m: - unum = int(action) - u = uset[unum] - print "Deleting user %s" % u - u.groups.remove(group) - - else: - for uname in action.split(','): - try: - user = User.objects.get(username=action) - except Exception as err: - print "Error %s" % err - continue - print "adding %s to group %s" % (user, group) - user.groups.add(group) - - - From e1c9e52fc5c11e9312347f8b152ceac13711fad5 Mon Sep 17 00:00:00 2001 From: ichuang Date: Mon, 20 Aug 2012 19:18:00 -0400 Subject: [PATCH 10/34] move textinput display:inline into capa display.scss file as class --- common/lib/capa/capa/inputtypes.py | 1 + common/lib/capa/capa/templates/textinput.html | 32 ++++--------------- .../lib/xmodule/xmodule/css/capa/display.scss | 4 +++ lms/envs/dev_ike.py | 4 ++- 4 files changed, 15 insertions(+), 26 deletions(-) diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py index 9de0a52e15..9cd8d7a322 100644 --- a/common/lib/capa/capa/inputtypes.py +++ b/common/lib/capa/capa/inputtypes.py @@ -149,6 +149,7 @@ def optioninput(element, value, status, render_template, msg=''): 'state': status, 'msg': msg, 'options': osetdict, + 'inline': element.get('inline',''), } html = render_template("optioninput.html", context) diff --git a/common/lib/capa/capa/templates/textinput.html b/common/lib/capa/capa/templates/textinput.html index 63c0609107..9b66654117 100644 --- a/common/lib/capa/capa/templates/textinput.html +++ b/common/lib/capa/capa/templates/textinput.html @@ -1,32 +1,14 @@ -
+<% doinline = "inline" if inline else "" %> + +
% if state == 'unsubmitted': -
+
% elif state == 'correct': -
+
% elif state == 'incorrect': -
+
% elif state == 'incomplete': -
+
% endif % if hidden:
diff --git a/common/lib/xmodule/xmodule/css/capa/display.scss b/common/lib/xmodule/xmodule/css/capa/display.scss index 666ca57800..af0dafaa73 100644 --- a/common/lib/xmodule/xmodule/css/capa/display.scss +++ b/common/lib/xmodule/xmodule/css/capa/display.scss @@ -49,6 +49,10 @@ section.problem { } } + .inline { + display: inline; + } + div { p { &.answer { diff --git a/lms/envs/dev_ike.py b/lms/envs/dev_ike.py index a9bd3ef97c..3ae141a037 100644 --- a/lms/envs/dev_ike.py +++ b/lms/envs/dev_ike.py @@ -18,16 +18,18 @@ MITX_FEATURES['ENABLE_DISCUSSION'] = False MITX_FEATURES['ACCESS_REQUIRE_STAFF_FOR_COURSE'] = True # require that user be in the staff_* group to be able to enroll MITX_FEATURES['DISABLE_START_DATES'] = True -MITX_FEATURES['USE_DJANGO_PIPELINE']=False # don't recompile scss +# MITX_FEATURES['USE_DJANGO_PIPELINE']=False # don't recompile scss myhost = socket.gethostname() if ('edxvm' in myhost) or ('ocw' in myhost): MITX_FEATURES['DISABLE_LOGIN_BUTTON'] = True # auto-login with MIT certificate MITX_FEATURES['USE_XQA_SERVER'] = 'https://qisx.mit.edu/xqa' # needs to be ssl or browser blocks it + MITX_FEATURES['USE_DJANGO_PIPELINE']=False # don't recompile scss if ('domU' in myhost): EMAIL_BACKEND = 'django.core.mail.backends.smtp.EmailBackend' MITX_FEATURES['REROUTE_ACTIVATION_EMAIL'] = 'ichuang@mitx.mit.edu' # nonempty string = address for all activation emails + MITX_FEATURES['USE_DJANGO_PIPELINE']=False # don't recompile scss SECURE_PROXY_SSL_HEADER = ('HTTP_X_FORWARDED_PROTOCOL', 'https') # django 1.4 for nginx ssl proxy From 65ea54f7e2a07411e3f34a678458549fe6031f5b Mon Sep 17 00:00:00 2001 From: ichuang Date: Mon, 20 Aug 2012 19:23:15 -0400 Subject: [PATCH 11/34] typo in common.py ALLOWED_GITRELOAD_IPS fixed --- lms/envs/common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/envs/common.py b/lms/envs/common.py index 265c500dbf..b48dbf60ce 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -260,7 +260,7 @@ MESSAGE_STORAGE = 'django.contrib.messages.storage.session.SessionStorage' # the advertised public IPs of the github WebHook servers. # These are listed, eg at https://github.com/MITx/mitx/admin/hooks -ALLOWED_GITRELOAD = ['207.97.227.253', '50.57.128.197', '108.171.174.178'] +ALLOWED_GITRELOAD_IPS = ['207.97.227.253', '50.57.128.197', '108.171.174.178'] #################################### AWS ####################################### # S3BotoStorage insists on a timeout for uploaded assets. We should make it From 0874369dc3d00c736c0e3170672ba8c79ae90f57 Mon Sep 17 00:00:00 2001 From: Arjun Singh Date: Tue, 21 Aug 2012 01:59:01 -0700 Subject: [PATCH 12/34] Minor change in extract_choices in choicegroup to allow for html in choice text --- common/lib/capa/capa/inputtypes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py index 8c513e7aec..05027b1e1f 100644 --- a/common/lib/capa/capa/inputtypes.py +++ b/common/lib/capa/capa/inputtypes.py @@ -205,7 +205,7 @@ def extract_choices(element): raise Exception("[courseware.capa.inputtypes.extract_choices] \ Expected a tag; got %s instead" % choice.tag) - choice_text = ''.join([x.text for x in choice]) + choice_text = ''.join([etree.tostring(x) for x in choice]) choices.append((choice.get("name"), choice_text)) From 1cfed46d06b0aebe9a29062478b64b8b59b04504 Mon Sep 17 00:00:00 2001 From: Arjun Singh Date: Tue, 21 Aug 2012 05:47:26 -0700 Subject: [PATCH 13/34] Adding a syllabus page (accessible via course navigation) --- lms/djangoapps/courseware/courses.py | 26 ++++++++ lms/djangoapps/courseware/views.py | 12 ++++ lms/envs/common.py | 1 + lms/static/sass/course.scss | 1 + lms/static/sass/course/_syllabus.scss | 64 +++++++++++++++++++ .../courseware/course_navigation.html | 3 + lms/templates/courseware/syllabus.html | 24 +++++++ lms/urls.py | 2 + 8 files changed, 133 insertions(+) create mode 100644 lms/static/sass/course/_syllabus.scss create mode 100644 lms/templates/courseware/syllabus.html diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index b4407b7f93..62d05cbc04 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -142,6 +142,32 @@ def get_course_info_section(course, section_key): raise KeyError("Invalid about key " + str(section_key)) +def get_course_syllabus_section(course, section_key): + """ + This returns the snippet of html to be rendered on the syllabus page, + given the key for the section. + + Valid keys: + - syllabus + - guest_syllabus + """ + + # Many of these are stored as html files instead of some semantic + # markup. This can change without effecting this interface when we find a + # good format for defining so many snippets of text/html. + + if section_key in ['syllabus', 'guest_syllabus']: + try: + with course.system.resources_fs.open(path("syllabus") / section_key + ".html") as htmlFile: + return replace_urls(htmlFile.read().decode('utf-8'), + course.metadata['data_dir']) + except ResourceNotFoundError: + log.exception("Missing syllabus section {key} in course {url}".format( + key=section_key, url=course.location.url())) + return "! Syllabus missing !" + + raise KeyError("Invalid about key " + str(section_key)) + def get_courses_by_university(user, domain=None): ''' diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 2ab6fa0223..73529a5f88 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -219,6 +219,18 @@ def course_info(request, course_id): return render_to_response('courseware/info.html', {'course': course, 'staff_access': staff_access,}) +@ensure_csrf_cookie +def syllabus(request, course_id): + """ + Display the course's syllabus.html, or 404 if there is no such course. + + 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('courseware/syllabus.html', {'course': course, + 'staff_access': staff_access,}) def registered_for_course(course, user): '''Return CourseEnrollment if user is registered for course, else False''' diff --git a/lms/envs/common.py b/lms/envs/common.py index 52cb8c7d06..2561ad7f61 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -56,6 +56,7 @@ MITX_FEATURES = { 'ENABLE_TEXTBOOK' : True, 'ENABLE_DISCUSSION' : True, + 'ENABLE_SYLLABUS' : True, 'ENABLE_SQL_TRACKING_LOGS': False, 'ENABLE_LMS_MIGRATION': False, diff --git a/lms/static/sass/course.scss b/lms/static/sass/course.scss index d3a74cb91b..054e403289 100644 --- a/lms/static/sass/course.scss +++ b/lms/static/sass/course.scss @@ -29,6 +29,7 @@ // pages @import "course/info"; +@import "course/syllabus"; @import "course/textbook"; @import "course/profile"; @import "course/gradebook"; diff --git a/lms/static/sass/course/_syllabus.scss b/lms/static/sass/course/_syllabus.scss new file mode 100644 index 0000000000..2f2b9eedcd --- /dev/null +++ b/lms/static/sass/course/_syllabus.scss @@ -0,0 +1,64 @@ +div.syllabus { + + padding: 0px 10px; + + text-align: center; + + h1 { + @extend .top-header + } + + .notes { + width: 740px; + margin: 0px auto 10px; + } + + table { + + text-align: left; + + margin: 10px auto; + + thead { + font-weight: bold; + border-bottom: 1px solid black; + } + + tr.first { + td { + padding-top: 15px; + } + } + + td { + + vertical-align: middle; + + padding: 5px 10px; + + &.day, &.due { + white-space: nowrap; + } + + &.no_class { + text-align: center; + } + + &.important { + color: red; + } + + &.week_separator { + padding: 0px; + + hr { + margin: 10px; + } + + } + + } + + } + +} diff --git a/lms/templates/courseware/course_navigation.html b/lms/templates/courseware/course_navigation.html index 3b329c4832..7567e682dd 100644 --- a/lms/templates/courseware/course_navigation.html +++ b/lms/templates/courseware/course_navigation.html @@ -19,6 +19,9 @@ def url_class(url):
  1. Courseware
  2. Course Info
    3. +% if settings.MITX_FEATURES.get('ENABLE_SYLLABUS'): +
  3. Syllabus
    4. +% endif % if user.is_authenticated(): % if settings.MITX_FEATURES.get('ENABLE_TEXTBOOK'): % for index, textbook in enumerate(course.textbooks): diff --git a/lms/templates/courseware/syllabus.html b/lms/templates/courseware/syllabus.html new file mode 100644 index 0000000000..38c1fbd6f8 --- /dev/null +++ b/lms/templates/courseware/syllabus.html @@ -0,0 +1,24 @@ +<%inherit file="/main.html" /> +<%namespace name='static' file='/static_content.html'/> + +<%block name="headextra"> + <%static:css group='course'/> + + +<%block name="title">${course.number} Course Info + +<%include file="/courseware/course_navigation.html" args="active_page='syllabus'" /> +<%! + from courseware.courses import get_course_syllabus_section +%> + +
    +
    +

    Syllabus

    + % if user.is_authenticated(): + ${get_course_syllabus_section(course, 'syllabus')} + % else: + ${get_course_syllabus_section(course, 'guest_syllabus')} + % endif +
    +
    diff --git a/lms/urls.py b/lms/urls.py index a3c6c9cdad..9bf021ea04 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -126,6 +126,8 @@ if settings.COURSEWARE_ENABLED: #Inside the course url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/info$', 'courseware.views.course_info', name="info"), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/syllabus$', + 'courseware.views.syllabus', name="syllabus"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/book/(?P[^/]*)/$', 'staticbook.views.index', name="book"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/book/(?P[^/]*)/(?P[^/]*)$', From eeadf0ba87daaf70299847d8f178dddd78fedf56 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 20 Aug 2012 19:28:07 -0400 Subject: [PATCH 14/34] Let policies be stored in policies/{url_name}/policy.json * still backcompat with old mode --- common/lib/xmodule/xmodule/modulestore/xml.py | 9 +++++++-- .../{TT_2012_Fall.json => TT_2012_Fall/policy.json} | 0 2 files changed, 7 insertions(+), 2 deletions(-) rename common/test/data/two_toys/policies/{TT_2012_Fall.json => TT_2012_Fall/policy.json} (100%) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index e0fecb243d..769d110518 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -243,8 +243,13 @@ class XMLModuleStore(ModuleStoreBase): url_name = course_data.get('url_name', course_data.get('slug')) if url_name: - policy_path = self.data_dir / course_dir / 'policies' / '{0}.json'.format(url_name) - policy = self.load_policy(policy_path, tracker) + old_policy_path = self.data_dir / course_dir / 'policies' / url_name / 'policy.json' + policy = self.load_policy(old_policy_path, tracker) + + # VS[compat]: remove once courses use the policy dirs. + if policy == {}: + old_policy_path = self.data_dir / course_dir / 'policies' / '{0}.json'.format(url_name) + policy = self.load_policy(old_policy_path, tracker) else: policy = {} # VS[compat] : 'name' is deprecated, but support it for now... diff --git a/common/test/data/two_toys/policies/TT_2012_Fall.json b/common/test/data/two_toys/policies/TT_2012_Fall/policy.json similarity index 100% rename from common/test/data/two_toys/policies/TT_2012_Fall.json rename to common/test/data/two_toys/policies/TT_2012_Fall/policy.json From f866854411496b08f004199dbe68642a5be6ccbb Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 20 Aug 2012 20:00:32 -0400 Subject: [PATCH 15/34] Load grading policy from policy/{url_name}/grading_policy.json * with backcompat location /grading_policy.json --- common/lib/xmodule/xmodule/course_module.py | 33 +++++++-------- common/lib/xmodule/xmodule/graders.py | 16 ++++---- common/lib/xmodule/xmodule/modulestore/xml.py | 41 +++++++++++++++++-- .../lib/xmodule/xmodule/tests/test_import.py | 3 ++ .../policies/TT_2012_Fall/grading_policy.json | 35 ++++++++++++++++ 5 files changed, 98 insertions(+), 30 deletions(-) create mode 100644 common/test/data/two_toys/policies/TT_2012_Fall/grading_policy.json diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index a9f29b9a13..fb0721454c 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -18,7 +18,7 @@ class CourseDescriptor(SequenceDescriptor): class Textbook: def __init__(self, title, book_url): self.title = title - self.book_url = book_url + self.book_url = book_url self.table_of_contents = self._get_toc_from_s3() @classmethod @@ -30,11 +30,11 @@ class CourseDescriptor(SequenceDescriptor): return self.table_of_contents def _get_toc_from_s3(self): - ''' + """ Accesses the textbook's table of contents (default name "toc.xml") at the URL self.book_url Returns XML tree representation of the table of contents - ''' + """ toc_url = self.book_url + 'toc.xml' # Get the table of contents from S3 @@ -72,6 +72,15 @@ class CourseDescriptor(SequenceDescriptor): self.enrollment_start = self._try_parse_time("enrollment_start") self.enrollment_end = self._try_parse_time("enrollment_end") + # NOTE: relies on the modulestore to call set_grading_policy() right after + # init. (Modulestore is in charge of figuring out where to load the policy from) + + + def set_grading_policy(self, policy_str): + """Parse the policy specified in policy_str, and save it""" + self._grading_policy = load_grading_policy(policy_str) + + @classmethod def definition_from_xml(cls, xml_object, system): textbooks = [] @@ -87,25 +96,11 @@ class CourseDescriptor(SequenceDescriptor): @property def grader(self): - return self.__grading_policy['GRADER'] + return self._grading_policy['GRADER'] @property def grade_cutoffs(self): - return self.__grading_policy['GRADE_CUTOFFS'] - - @lazyproperty - def __grading_policy(self): - policy_string = "" - - try: - with self.system.resources_fs.open("grading_policy.json") as grading_policy_file: - policy_string = grading_policy_file.read() - except (IOError, ResourceNotFoundError): - log.warning("Unable to load course settings file from grading_policy.json in course " + self.id) - - grading_policy = load_grading_policy(policy_string) - - return grading_policy + return self._grading_policy['GRADE_CUTOFFS'] @lazyproperty def grading_context(self): diff --git a/common/lib/xmodule/xmodule/graders.py b/common/lib/xmodule/xmodule/graders.py index fca862aa9f..82dc37bf57 100644 --- a/common/lib/xmodule/xmodule/graders.py +++ b/common/lib/xmodule/xmodule/graders.py @@ -14,11 +14,11 @@ def load_grading_policy(course_policy_string): """ This loads a grading policy from a string (usually read from a file), which can be a JSON object or an empty string. - + The JSON object can have the keys GRADER and GRADE_CUTOFFS. If either is missing, it reverts to the default. """ - + default_policy_string = """ { "GRADER" : [ @@ -56,7 +56,7 @@ def load_grading_policy(course_policy_string): } } """ - + # Load the global settings as a dictionary grading_policy = json.loads(default_policy_string) @@ -64,15 +64,15 @@ def load_grading_policy(course_policy_string): course_policy = {} if course_policy_string: course_policy = json.loads(course_policy_string) - + # Override any global settings with the course settings grading_policy.update(course_policy) # Here is where we should parse any configurations, so that we can fail early grading_policy['GRADER'] = grader_from_conf(grading_policy['GRADER']) - + return grading_policy - + def aggregate_scores(scores, section_name="summary"): """ @@ -130,7 +130,9 @@ def grader_from_conf(conf): raise ValueError("Configuration has no appropriate grader class.") except (TypeError, ValueError) as error: - errorString = "Unable to parse grader configuration:\n " + str(subgraderconf) + "\n Error was:\n " + str(error) + errorString = ("Unable to parse grader configuration:\n " + + str(subgraderconf) + + "\n Error was:\n " + str(error)) log.critical(errorString) raise ValueError(errorString) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 769d110518..30610479b9 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -4,16 +4,17 @@ import os import re from collections import defaultdict +from cStringIO import StringIO 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 from xmodule.course_module import CourseDescriptor from xmodule.mako_module import MakoDescriptorSystem -from cStringIO import StringIO +from xmodule.x_module import XModuleDescriptor, XMLParsingSystem from . import ModuleStoreBase, Location from .exceptions import ItemNotFoundError @@ -202,6 +203,27 @@ class XMLModuleStore(ModuleStoreBase): return {} + def read_grading_policy(self, paths, tracker): + """Load a grading policy from the specified paths, in order, if it exists.""" + # Default to a blank policy + policy_str = "" + + for policy_path in paths: + if not os.path.exists(policy_path): + continue + try: + with open(policy_path) as grading_policy_file: + policy_str = grading_policy_file.read() + # if we successfully read the file, stop looking at backups + break + except (IOError): + msg = "Unable to load course settings file from '{0}'".format(policy_path) + tracker(msg) + log.warning(msg) + + return policy_str + + def load_course(self, course_dir, tracker): """ Load a course into this module store @@ -242,9 +264,11 @@ class XMLModuleStore(ModuleStoreBase): course = course_dir url_name = course_data.get('url_name', course_data.get('slug')) + policy_dir = None if url_name: - old_policy_path = self.data_dir / course_dir / 'policies' / url_name / 'policy.json' - policy = self.load_policy(old_policy_path, tracker) + policy_dir = self.data_dir / course_dir / 'policies' / url_name + policy_path = policy_dir / 'policy.json' + policy = self.load_policy(policy_path, tracker) # VS[compat]: remove once courses use the policy dirs. if policy == {}: @@ -273,6 +297,15 @@ class XMLModuleStore(ModuleStoreBase): # after we have the course descriptor. XModuleDescriptor.compute_inherited_metadata(course_descriptor) + # Try to load grading policy + paths = [self.data_dir / 'grading_policy.json'] + if policy_dir: + paths = [policy_dir / 'grading_policy.json'] + paths + + policy_str = self.read_grading_policy(paths, tracker) + course_descriptor.set_grading_policy(policy_str) + + log.debug('========> Done with course import from {0}'.format(course_dir)) return course_descriptor diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index 34e4767a62..3454366c1a 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -233,6 +233,9 @@ class ImportTestCase(unittest.TestCase): self.assertEqual(toy_ch.display_name, "Overview") self.assertEqual(two_toys_ch.display_name, "Two Toy Overview") + # Also check that the grading policy loaded + self.assertEqual(two_toys.grade_cutoffs['C'], 0.5999) + def test_definition_loading(self): """When two courses share the same org and course name and diff --git a/common/test/data/two_toys/policies/TT_2012_Fall/grading_policy.json b/common/test/data/two_toys/policies/TT_2012_Fall/grading_policy.json new file mode 100644 index 0000000000..13942c1715 --- /dev/null +++ b/common/test/data/two_toys/policies/TT_2012_Fall/grading_policy.json @@ -0,0 +1,35 @@ +{ + "GRADER" : [ + { + "type" : "Homework", + "min_count" : 12, + "drop_count" : 2, + "short_label" : "HW", + "weight" : 0.15 + }, + { + "type" : "Lab", + "min_count" : 12, + "drop_count" : 2, + "category" : "Labs", + "weight" : 0.15 + }, + { + "type" : "Midterm", + "name" : "Midterm Exam", + "short_label" : "Midterm", + "weight" : 0.3 + }, + { + "type" : "Final", + "name" : "Final Exam", + "short_label" : "Final", + "weight" : 0.4 + } + ], + "GRADE_CUTOFFS" : { + "A" : 0.87, + "B" : 0.7, + "C" : 0.5999 + } +} From b97a2af2a2e5d0fbcb6ffd04e007076e08dba4b5 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 21 Aug 2012 11:09:58 -0400 Subject: [PATCH 16/34] Show policy, grading policy, and top-level errors * Save errors for courses that failed to load in modulestore * global staff can see course errors on their dashboard * put policy loading errors into the error trackers * add has_access(user, 'global', 'staff'), which is equiv to user.is_staff for now --- common/djangoapps/student/views.py | 14 +++++++- common/lib/xmodule/xmodule/course_module.py | 9 ++++- common/lib/xmodule/xmodule/graders.py | 11 +++--- common/lib/xmodule/xmodule/modulestore/xml.py | 36 +++++++++++++------ lms/djangoapps/courseware/access.py | 36 +++++++++++++++++++ lms/djangoapps/courseware/courses.py | 1 - lms/templates/dashboard.html | 15 ++++++++ 7 files changed, 104 insertions(+), 18 deletions(-) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index cab57e6819..f36936318e 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -140,7 +140,19 @@ def dashboard(request): if not user.is_active: message = render_to_string('registration/activate_account_notice.html', {'email': user.email}) - context = {'courses': courses, 'message': message} + + # Global staff can see what courses errored on their dashboard + staff_access = False + if has_access(user, 'global', 'staff'): + # Show any courses that errored on load + staff_access = True + errored_courses = modulestore().get_errored_courses() + + context = {'courses': courses, + 'message': message, + 'staff_access': staff_access, + 'errored_courses': errored_courses,} + return render_to_response('dashboard.html', context) diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index fb0721454c..5cc4a09165 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -78,7 +78,14 @@ class CourseDescriptor(SequenceDescriptor): def set_grading_policy(self, policy_str): """Parse the policy specified in policy_str, and save it""" - self._grading_policy = load_grading_policy(policy_str) + try: + self._grading_policy = load_grading_policy(policy_str) + except: + self.system.error_tracker("Failed to load grading policy") + # Setting this to an empty dictionary will lead to errors when + # grading needs to happen, but should allow course staff to see + # the error log. + self._grading_policy = {} @classmethod diff --git a/common/lib/xmodule/xmodule/graders.py b/common/lib/xmodule/xmodule/graders.py index 82dc37bf57..3f0bb63186 100644 --- a/common/lib/xmodule/xmodule/graders.py +++ b/common/lib/xmodule/xmodule/graders.py @@ -1,6 +1,7 @@ import abc import json import logging +import sys from collections import namedtuple @@ -130,11 +131,11 @@ def grader_from_conf(conf): raise ValueError("Configuration has no appropriate grader class.") except (TypeError, ValueError) as error: - errorString = ("Unable to parse grader configuration:\n " + - str(subgraderconf) + - "\n Error was:\n " + str(error)) - log.critical(errorString) - raise ValueError(errorString) + # Add info and re-raise + msg = ("Unable to parse grader configuration:\n " + + str(subgraderconf) + + "\n Error was:\n " + str(error)) + raise ValueError(msg), None, sys.exc_info()[2] return WeightedSubsectionsGrader(subgraders) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 30610479b9..3eca72987e 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -135,12 +135,12 @@ class XMLModuleStore(ModuleStoreBase): self.data_dir = path(data_dir) self.modules = defaultdict(dict) # course_id -> dict(location -> XModuleDescriptor) self.courses = {} # course_dir -> XModuleDescriptor for the course + self.errored_courses = {} # course_dir -> errorlog, for dirs that failed to load if default_class is None: self.default_class = None else: module_path, _, class_name = default_class.rpartition('.') - #log.debug('module_path = %s' % module_path) class_ = getattr(import_module(module_path), class_name) self.default_class = class_ @@ -163,18 +163,27 @@ class XMLModuleStore(ModuleStoreBase): ''' Load a course, keeping track of errors as we go along. ''' + # Special-case code here, since we don't have a location for the + # course before it loads. + # So, make a tracker to track load-time errors, then put in the right + # place after the course loads and we have its location + errorlog = make_error_tracker() + course_descriptor = None try: - # Special-case code here, since we don't have a location for the - # course before it loads. - # So, make a tracker to track load-time errors, then put in the right - # place after the course loads and we have its location - errorlog = make_error_tracker() course_descriptor = self.load_course(course_dir, errorlog.tracker) + except Exception as e: + msg = "Failed to load course '{0}': {1}".format(course_dir, str(e)) + log.exception(msg) + errorlog.tracker(msg) + + if course_descriptor is not None: self.courses[course_dir] = course_descriptor self._location_errors[course_descriptor.location] = errorlog - except: - msg = "Failed to load course '%s'" % course_dir - log.exception(msg) + else: + # Didn't load course. Instead, save the errors elsewhere. + self.errored_courses[course_dir] = errorlog + + def __unicode__(self): ''' @@ -211,6 +220,7 @@ class XMLModuleStore(ModuleStoreBase): for policy_path in paths: if not os.path.exists(policy_path): continue + log.debug("Loading grading policy from {0}".format(policy_path)) try: with open(policy_path) as grading_policy_file: policy_str = grading_policy_file.read() @@ -298,7 +308,7 @@ class XMLModuleStore(ModuleStoreBase): XModuleDescriptor.compute_inherited_metadata(course_descriptor) # Try to load grading policy - paths = [self.data_dir / 'grading_policy.json'] + paths = [self.data_dir / course_dir / 'grading_policy.json'] if policy_dir: paths = [policy_dir / 'grading_policy.json'] + paths @@ -356,6 +366,12 @@ class XMLModuleStore(ModuleStoreBase): """ return self.courses.values() + def get_errored_courses(self): + """ + Return a dictionary of course_dir -> [(msg, exception_str)], for each + course_dir where course loading failed. + """ + return dict( (k, self.errored_courses[k].errors) for k in self.errored_courses) def create_item(self, location): raise NotImplementedError("XMLModuleStores are read-only") diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index eaf70d7814..281580cf33 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -63,6 +63,9 @@ def has_access(user, obj, action): if isinstance(obj, Location): return _has_access_location(user, obj, action) + if isinstance(obj, basestring): + return _has_access_string(user, obj, action) + # Passing an unknown object here is a coding error, so rather than # returning a default, complain. raise TypeError("Unknown object type in has_access(): '{0}'" @@ -238,6 +241,30 @@ def _has_access_location(user, location, action): return _dispatch(checkers, action, user, location) +def _has_access_string(user, perm, action): + """ + Check if user has certain special access, specified as string. Valid strings: + + 'global' + + Valid actions: + + 'staff' -- global staff access. + """ + + def check_staff(): + if perm != 'global': + debug("Deny: invalid permission '%s'", perm) + return False + return _has_global_staff_access(user) + + checkers = { + 'staff': check_staff + } + + return _dispatch(checkers, action, user, perm) + + ##### Internal helper methods below def _dispatch(table, action, user, obj): @@ -266,6 +293,15 @@ def _course_staff_group_name(location): """ return 'staff_%s' % Location(location).course +def _has_global_staff_access(user): + if user.is_staff: + debug("Allow: user.is_staff") + return True + else: + debug("Deny: not user.is_staff") + return False + + def _has_staff_access_to_location(user, location): ''' Returns True if the given user has staff access to a location. For now this diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index b4407b7f93..30ca38728a 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -30,7 +30,6 @@ def get_course_by_id(course_id): raise Http404("Course not found.") - def get_course_with_access(user, course_id, action): """ Given a course_id, look up the corresponding course descriptor, diff --git a/lms/templates/dashboard.html b/lms/templates/dashboard.html index cfe8a0953c..770e849841 100644 --- a/lms/templates/dashboard.html +++ b/lms/templates/dashboard.html @@ -107,6 +107,21 @@
% endif + % if staff_access and len(errored_courses) > 0: +
+

Course-loading errors

+ + % for course_dir, errors in errored_courses.items(): +

${course_dir | h}

+
    + % for (msg, err) in errors: +
  • ${msg} + 
    • ${err}
    +
    • + % endfor +
+ % endfor + % endif

From ccf2cff2bafb18dcfe6d5caeab8bd78cf6816af3 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 21 Aug 2012 13:04:33 -0400 Subject: [PATCH 17/34] bugfix: set errored_courses in default case --- common/djangoapps/student/views.py | 1 + 1 file changed, 1 insertion(+) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index f36936318e..66a860ab15 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -143,6 +143,7 @@ def dashboard(request): # Global staff can see what courses errored on their dashboard staff_access = False + errored_courses = [] if has_access(user, 'global', 'staff'): # Show any courses that errored on load staff_access = True From 2f9b2f19fc80bd276d976e21f886312a0828cc23 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 21 Aug 2012 15:01:10 -0400 Subject: [PATCH 18/34] s/[]/{}/ --- common/djangoapps/student/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 66a860ab15..0069935b0b 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -143,7 +143,7 @@ def dashboard(request): # Global staff can see what courses errored on their dashboard staff_access = False - errored_courses = [] + errored_courses = {} if has_access(user, 'global', 'staff'): # Show any courses that errored on load staff_access = True From f44d794e631cdbddca02f889e73fc6ebfe504c2b Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 20 Aug 2012 16:39:52 -0400 Subject: [PATCH 19/34] Add course_id to StudentModule * Update all uses. --- lms/djangoapps/courseware/grades.py | 9 +- .../management/commands/check_course.py | 1 + .../0004_add_field_studentmodule_course_id.py | 112 ++++++++++++++++++ lms/djangoapps/courseware/models.py | 59 +++++---- lms/djangoapps/courseware/module_render.py | 26 ++-- lms/djangoapps/courseware/views.py | 6 +- 6 files changed, 172 insertions(+), 41 deletions(-) create mode 100644 lms/djangoapps/courseware/migrations/0004_add_field_studentmodule_course_id.py diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index 3d8d8b0ebe..c913f2d5e5 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -42,7 +42,7 @@ def grade(student, request, course, student_module_cache=None): grading_context = course.grading_context if student_module_cache == None: - student_module_cache = StudentModuleCache(student, grading_context['all_descriptors']) + student_module_cache = StudentModuleCache(course.id, student, grading_context['all_descriptors']) totaled_scores = {} # This next complicated loop is just to collect the totaled_scores, which is @@ -56,7 +56,8 @@ def grade(student, request, course, student_module_cache=None): should_grade_section = False # If we haven't seen a single problem in the section, we don't have to grade it at all! We can assume 0% for moduledescriptor in section['xmoduledescriptors']: - if student_module_cache.lookup(moduledescriptor.category, moduledescriptor.location.url() ): + if student_module_cache.lookup( + course.id, moduledescriptor.category, moduledescriptor.location.url()): should_grade_section = True break @@ -64,10 +65,9 @@ def grade(student, request, course, student_module_cache=None): scores = [] # TODO: We need the request to pass into here. If we could forgo that, our arguments # would be simpler - course_id = CourseDescriptor.location_to_id(course.location) section_module = get_module(student, request, section_descriptor.location, student_module_cache, - course_id) + course.id) if section_module is None: # student doesn't have access to this module, or something else # went wrong. @@ -219,6 +219,7 @@ def get_score(user, problem, student_module_cache): # instance_module = student_module_cache.lookup(problem.category, problem.id) # if instance_module is None: # instance_module = StudentModule(module_type=problem.category, + # course_id=????, # module_state_key=problem.id, # student=user, # state=None, diff --git a/lms/djangoapps/courseware/management/commands/check_course.py b/lms/djangoapps/courseware/management/commands/check_course.py index 4b44581c3f..adb8bff709 100644 --- a/lms/djangoapps/courseware/management/commands/check_course.py +++ b/lms/djangoapps/courseware/management/commands/check_course.py @@ -84,6 +84,7 @@ class Command(BaseCommand): # TODO (cpennington): Get coursename in a legitimate way course_location = 'i4x://edx/6002xs12/course/6.002_Spring_2012' student_module_cache = StudentModuleCache.cache_for_descriptor_descendents( + course_id, sample_user, modulestore().get_item(course_location)) course = get_module(sample_user, None, course_location, student_module_cache) diff --git a/lms/djangoapps/courseware/migrations/0004_add_field_studentmodule_course_id.py b/lms/djangoapps/courseware/migrations/0004_add_field_studentmodule_course_id.py new file mode 100644 index 0000000000..71154e17fd --- /dev/null +++ b/lms/djangoapps/courseware/migrations/0004_add_field_studentmodule_course_id.py @@ -0,0 +1,112 @@ +# -*- coding: utf-8 -*- +import datetime +from south.db import db +from south.v2 import SchemaMigration +from django.db import models + + +class Migration(SchemaMigration): + + def forwards(self, orm): + # Adding field 'StudentModule.course_id' + db.add_column('courseware_studentmodule', 'course_id', + self.gf('django.db.models.fields.CharField')(default="", max_length=255, db_index=True), + keep_default=False) + + # Removing unique constraint on 'StudentModule', fields ['module_id', 'student'] + db.delete_unique('courseware_studentmodule', ['module_id', 'student_id']) + + # Adding unique constraint on 'StudentModule', fields ['course_id', 'module_state_key', 'student'] + db.create_unique('courseware_studentmodule', ['course_id', 'module_id', 'student_id']) + + + def backwards(self, orm): + # Removing unique constraint on 'StudentModule', fields ['course_id', 'module_state_key', 'student'] + db.delete_unique('courseware_studentmodule', ['course_id', 'module_id', 'student_id']) + + # Deleting field 'StudentModule.course_id' + db.delete_column('courseware_studentmodule', 'course_id') + + # Adding unique constraint on 'StudentModule', fields ['module_id', 'student'] + db.create_unique('courseware_studentmodule', ['module_id', 'student_id']) + + + models = { + 'auth.group': { + 'Meta': {'object_name': 'Group'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}), + 'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}) + }, + 'auth.permission': { + 'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'}, + 'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '50'}) + }, + 'auth.user': { + 'Meta': {'object_name': 'User'}, + 'about': ('django.db.models.fields.TextField', [], {'blank': 'True'}), + 'avatar_type': ('django.db.models.fields.CharField', [], {'default': "'n'", 'max_length': '1'}), + 'bronze': ('django.db.models.fields.SmallIntegerField', [], {'default': '0'}), + 'consecutive_days_visit_count': ('django.db.models.fields.IntegerField', [], {'default': '0'}), + 'country': ('django_countries.fields.CountryField', [], {'max_length': '2', 'blank': 'True'}), + 'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'date_of_birth': ('django.db.models.fields.DateField', [], {'null': 'True', 'blank': 'True'}), + 'display_tag_filter_strategy': ('django.db.models.fields.SmallIntegerField', [], {'default': '0'}), + 'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}), + 'email_isvalid': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'email_key': ('django.db.models.fields.CharField', [], {'max_length': '32', 'null': 'True'}), + 'email_tag_filter_strategy': ('django.db.models.fields.SmallIntegerField', [], {'default': '1'}), + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'gold': ('django.db.models.fields.SmallIntegerField', [], {'default': '0'}), + 'gravatar': ('django.db.models.fields.CharField', [], {'max_length': '32'}), + 'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'ignored_tags': ('django.db.models.fields.TextField', [], {'blank': 'True'}), + 'interesting_tags': ('django.db.models.fields.TextField', [], {'blank': 'True'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'last_seen': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'location': ('django.db.models.fields.CharField', [], {'max_length': '100', 'blank': 'True'}), + 'new_response_count': ('django.db.models.fields.IntegerField', [], {'default': '0'}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}), + 'questions_per_page': ('django.db.models.fields.SmallIntegerField', [], {'default': '10'}), + 'real_name': ('django.db.models.fields.CharField', [], {'max_length': '100', 'blank': 'True'}), + 'reputation': ('django.db.models.fields.PositiveIntegerField', [], {'default': '1'}), + 'seen_response_count': ('django.db.models.fields.IntegerField', [], {'default': '0'}), + 'show_country': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'silver': ('django.db.models.fields.SmallIntegerField', [], {'default': '0'}), + 'status': ('django.db.models.fields.CharField', [], {'default': "'w'", 'max_length': '2'}), + 'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}), + 'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'}), + 'website': ('django.db.models.fields.URLField', [], {'max_length': '200', 'blank': 'True'}) + }, + 'contenttypes.contenttype': { + 'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"}, + 'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '100'}) + }, + 'courseware.studentmodule': { + 'Meta': {'unique_together': "(('course_id', 'student', 'module_state_key'),)", 'object_name': 'StudentModule'}, + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'db_index': 'True', 'blank': 'True'}), + 'done': ('django.db.models.fields.CharField', [], {'default': "'na'", 'max_length': '8', 'db_index': 'True'}), + 'grade': ('django.db.models.fields.FloatField', [], {'db_index': 'True', 'null': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'max_grade': ('django.db.models.fields.FloatField', [], {'null': 'True', 'blank': 'True'}), + 'modified': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'db_index': 'True', 'blank': 'True'}), + 'module_state_key': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_column': "'module_id'", 'db_index': 'True'}), + 'module_type': ('django.db.models.fields.CharField', [], {'default': "'problem'", 'max_length': '32', 'db_index': 'True'}), + 'state': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'student': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}) + } + } + + complete_apps = ['courseware'] diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index 4389a5f169..4fdedfcdd3 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -22,6 +22,9 @@ from django.contrib.auth.models import User class StudentModule(models.Model): + """ + Keeps student state for a particular module in a particular course. + """ # For a homework problem, contains a JSON # object consisting of state MODULE_TYPES = (('problem', 'problem'), @@ -37,9 +40,10 @@ class StudentModule(models.Model): # Filename for homeworks, etc. module_state_key = models.CharField(max_length=255, db_index=True, db_column='module_id') student = models.ForeignKey(User, db_index=True) + course_id = models.CharField(max_length=255, db_index=True) class Meta: - unique_together = (('student', 'module_state_key'),) + unique_together = (('course_id', 'student', 'module_state_key'),) ## Internal state of the object state = models.TextField(null=True, blank=True) @@ -57,7 +61,8 @@ class StudentModule(models.Model): modified = models.DateTimeField(auto_now=True, db_index=True) def __unicode__(self): - return '/'.join([self.module_type, self.student.username, self.module_state_key, str(self.state)[:20]]) + return '/'.join([self.course_id, self.module_type, + self.student.username, self.module_state_key, str(self.state)[:20]]) # TODO (cpennington): Remove these once the LMS switches to using XModuleDescriptors @@ -67,20 +72,20 @@ class StudentModuleCache(object): """ A cache of StudentModules for a specific student """ - def __init__(self, user, descriptors, select_for_update=False): + def __init__(self, course_id, user, descriptors, select_for_update=False): ''' Find any StudentModule objects that are needed by any descriptor in descriptors. Avoids making multiple queries to the database. Note: Only modules that have store_state = True or have shared state will have a StudentModule. - + Arguments user: The user for which to fetch maching StudentModules descriptors: An array of XModuleDescriptors. 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) + module_ids = self._get_module_state_keys(descriptors) # This works around a limitation in sqlite3 on the number of parameters # that can be put into a single query @@ -89,78 +94,86 @@ class StudentModuleCache(object): for id_chunk in [module_ids[i:i + chunk_size] for i in xrange(0, len(module_ids), chunk_size)]: if select_for_update: self.cache.extend(StudentModule.objects.select_for_update().filter( + course_id=course_id, student=user, module_state_key__in=id_chunk) ) else: self.cache.extend(StudentModule.objects.filter( + course_id=course_id, student=user, module_state_key__in=id_chunk) ) else: self.cache = [] - - + + @classmethod - def cache_for_descriptor_descendents(cls, user, descriptor, depth=None, descriptor_filter=lambda descriptor: True, select_for_update=False): + def cache_for_descriptor_descendents(cls, course_id, user, descriptor, depth=None, + descriptor_filter=lambda descriptor: True, + select_for_update=False): """ + course_id: the course in the context of which we want StudentModules. + user: the django user for whom to load modules. descriptor: An XModuleDescriptor depth is the number of levels of descendent modules to load StudentModules for, in addition to 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 + descriptor_filter is a function that accepts a descriptor and return wether the StudentModule should be cached select_for_update: Flag indicating whether the rows should be locked until end of transaction """ - + def get_child_descriptors(descriptor, depth, descriptor_filter): if descriptor_filter(descriptor): descriptors = [descriptor] else: descriptors = [] - + if depth is None or depth > 0: new_depth = depth - 1 if depth is not None else depth for child in descriptor.get_children(): descriptors.extend(get_child_descriptors(child, new_depth, descriptor_filter)) - + return descriptors - - + + descriptors = get_child_descriptors(descriptor, depth, descriptor_filter) - - return StudentModuleCache(user, descriptors, select_for_update) - + + return StudentModuleCache(course_id, user, descriptors, select_for_update) + def _get_module_state_keys(self, descriptors): ''' Get a list of the state_keys needed for StudentModules required for this module descriptor - - descriptor_filter is a function that accepts a descriptor and return wether the StudentModule + + descriptor_filter is a function that accepts a descriptor and return wether the StudentModule should be cached ''' keys = [] for descriptor in descriptors: if descriptor.stores_state: keys.append(descriptor.location.url()) - + shared_state_key = getattr(descriptor, 'shared_state_key', None) if shared_state_key is not None: keys.append(shared_state_key) return keys - def lookup(self, module_type, module_state_key): + def lookup(self, course_id, module_type, module_state_key): ''' - Look for a student module with the given type and id in the cache. + Look for a student module with the given course_id, type, and id in the cache. cache -- list of student modules returns first found object, or None ''' for o in self.cache: - if o.module_type == module_type and o.module_state_key == module_state_key: + if (o.course_id == course_id and + o.module_type == module_type and + o.module_state_key == module_state_key): return o return None diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index fbd43b8247..a61e8b9e23 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -70,7 +70,8 @@ def toc_for_course(user, request, course, active_chapter, active_section, course None if this is not the case. ''' - student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(user, course, depth=2) + student_module_cache = StudentModuleCache.cache_for_descriptor_descendents( + course_id, user, course, depth=2) course = get_module(user, request, course.location, student_module_cache, course_id) chapters = list() @@ -159,12 +160,13 @@ def get_module(user, request, location, student_module_cache, course_id, positio shared_module = None if user.is_authenticated(): if descriptor.stores_state: - instance_module = student_module_cache.lookup(descriptor.category, - descriptor.location.url()) + instance_module = student_module_cache.lookup( + course_id, descriptor.category, descriptor.location.url()) shared_state_key = getattr(descriptor, 'shared_state_key', None) if shared_state_key is not None: - shared_module = student_module_cache.lookup(descriptor.category, + shared_module = student_module_cache.lookup(course_id, + descriptor.category, shared_state_key) @@ -241,7 +243,7 @@ def get_module(user, request, location, student_module_cache, course_id, positio return module -def get_instance_module(user, module, student_module_cache): +def get_instance_module(course_id, user, module, student_module_cache): """ Returns instance_module is a StudentModule specific to this module for this student, or None if this is an anonymous user @@ -252,11 +254,12 @@ def get_instance_module(user, module, student_module_cache): + str(module.id) + " which does not store state.") return None - instance_module = student_module_cache.lookup(module.category, - module.location.url()) + instance_module = student_module_cache.lookup( + course_id, module.category, module.location.url()) if not instance_module: instance_module = StudentModule( + course_id=course_id, student=user, module_type=module.category, module_state_key=module.id, @@ -285,6 +288,7 @@ def get_shared_instance_module(course_id, user, module, student_module_cache): shared_state_key) if not shared_module: shared_module = StudentModule( + course_id=course_id, student=user, module_type=descriptor.category, module_state_key=shared_state_key, @@ -317,14 +321,14 @@ def xqueue_callback(request, course_id, userid, id, dispatch): # Retrieve target StudentModule user = User.objects.get(id=userid) - student_module_cache = StudentModuleCache.cache_for_descriptor_descendents( + student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(course_id, user, modulestore().get_instance(course_id, id), depth=0, select_for_update=True) instance = get_module(user, request, id, student_module_cache, course_id) if instance is None: log.debug("No module {0} for user {1}--access denied?".format(id, user)) raise Http404 - instance_module = get_instance_module(user, instance, student_module_cache) + instance_module = get_instance_module(course_id, user, instance, student_module_cache) if instance_module is None: log.debug("Couldn't find instance of module '%s' for user '%s'", id, user) @@ -387,7 +391,7 @@ def modx_dispatch(request, dispatch, location, course_id): return HttpResponse(json.dumps({'success': file_too_big_msg})) p[fileinput_id] = inputfiles - student_module_cache = StudentModuleCache.cache_for_descriptor_descendents( + student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(course_id, request.user, modulestore().get_instance(course_id, location)) instance = get_module(request.user, request, location, student_module_cache, course_id) @@ -397,7 +401,7 @@ def modx_dispatch(request, dispatch, location, course_id): log.debug("No module {0} for user {1}--access denied?".format(location, user)) raise Http404 - instance_module = get_instance_module(request.user, instance, student_module_cache) + instance_module = get_instance_module(course_id, request.user, instance, student_module_cache) shared_module = get_shared_instance_module(course_id, request.user, instance, student_module_cache) # Don't track state for anonymous users (who don't have student modules) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 583628d1f2..6e8fe6fd23 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -149,8 +149,7 @@ def index(request, course_id, chapter=None, section=None, section_descriptor = get_section(course, chapter, section) if section_descriptor is not None: student_module_cache = StudentModuleCache.cache_for_descriptor_descendents( - request.user, - section_descriptor) + course_id, request.user, section_descriptor) module = get_module(request.user, request, section_descriptor.location, student_module_cache, course_id) @@ -310,7 +309,8 @@ def progress(request, course_id, student_id=None): raise Http404 student = User.objects.get(id=int(student_id)) - student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(request.user, course) + student_module_cache = StudentModuleCache.cache_for_descriptor_descendents( + course_id, request.user, course) course_module = get_module(request.user, request, course.location, student_module_cache, course_id) From d9c753e7ef6e31fddd26ac6a9d10dabf4694825d Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 20 Aug 2012 16:41:01 -0400 Subject: [PATCH 20/34] add test output files to gitignore --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index 3653c832fa..81d9a57d3c 100644 --- a/.gitignore +++ b/.gitignore @@ -26,3 +26,5 @@ Gemfile.lock lms/static/sass/*.css cms/static/sass/*.css lms/lib/comment_client/python +nosetests.xml +cover_html/ From ec90200b8100d8e5ee55d8b95d6d1bfd47083c96 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 20 Aug 2012 17:41:10 -0400 Subject: [PATCH 21/34] fix migration to remove out of date index * reorder index fields --- lms/djangoapps/courseware/grades.py | 10 ++++++---- .../courseware/migrations/0003_done_grade_cache.py | 2 ++ .../0004_add_field_studentmodule_course_id.py | 13 ++++++++++--- lms/djangoapps/courseware/models.py | 2 +- 4 files changed, 19 insertions(+), 8 deletions(-) diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index c913f2d5e5..6a0fedfc4b 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -76,7 +76,7 @@ def grade(student, request, course, student_module_cache=None): # TODO: We may be able to speed this up by only getting a list of children IDs from section_module # Then, we may not need to instatiate any problems if they are already in the database for module in yield_module_descendents(section_module): - (correct, total) = get_score(student, module, student_module_cache) + (correct, total) = get_score(course.id, student, module, student_module_cache) if correct is None and total is None: continue @@ -171,7 +171,9 @@ def progress_summary(student, course, grader, student_module_cache): graded = s.metadata.get('graded', False) scores = [] for module in yield_module_descendents(s): - (correct, total) = get_score(student, module, student_module_cache) + # course is a module, not a descriptor... + course_id = course.descriptor.id + (correct, total) = get_score(course_id, student, module, student_module_cache) if correct is None and total is None: continue @@ -200,7 +202,7 @@ def progress_summary(student, course, grader, student_module_cache): return chapters -def get_score(user, problem, student_module_cache): +def get_score(course_id, user, problem, student_module_cache): """ Return the score for a user on a problem, as a tuple (correct, total). @@ -215,7 +217,7 @@ def get_score(user, problem, student_module_cache): correct = 0.0 # If the ID is not in the cache, add the item - instance_module = get_instance_module(user, problem, student_module_cache) + instance_module = get_instance_module(course_id, user, problem, student_module_cache) # instance_module = student_module_cache.lookup(problem.category, problem.id) # if instance_module is None: # instance_module = StudentModule(module_type=problem.category, diff --git a/lms/djangoapps/courseware/migrations/0003_done_grade_cache.py b/lms/djangoapps/courseware/migrations/0003_done_grade_cache.py index 96b320bc8f..f29f931079 100644 --- a/lms/djangoapps/courseware/migrations/0003_done_grade_cache.py +++ b/lms/djangoapps/courseware/migrations/0003_done_grade_cache.py @@ -9,6 +9,8 @@ class Migration(SchemaMigration): def forwards(self, orm): + # NOTE (vshnayder): This constraint has the wrong field order, so it doesn't actually + # do anything. Migration 0004 actually removes this index. # Removing unique constraint on 'StudentModule', fields ['module_id', 'module_type', 'student'] db.delete_unique('courseware_studentmodule', ['module_id', 'module_type', 'student_id']) diff --git a/lms/djangoapps/courseware/migrations/0004_add_field_studentmodule_course_id.py b/lms/djangoapps/courseware/migrations/0004_add_field_studentmodule_course_id.py index 71154e17fd..7433a48b0b 100644 --- a/lms/djangoapps/courseware/migrations/0004_add_field_studentmodule_course_id.py +++ b/lms/djangoapps/courseware/migrations/0004_add_field_studentmodule_course_id.py @@ -16,13 +16,17 @@ class Migration(SchemaMigration): # Removing unique constraint on 'StudentModule', fields ['module_id', 'student'] db.delete_unique('courseware_studentmodule', ['module_id', 'student_id']) + # NOTE: manually remove this constaint (from 0001)--0003 tries, but fails. + # Removing unique constraint on 'StudentModule', fields ['module_id', 'module_type', 'student'] + db.delete_unique('courseware_studentmodule', ['student_id', 'module_id', 'module_type']) + # Adding unique constraint on 'StudentModule', fields ['course_id', 'module_state_key', 'student'] - db.create_unique('courseware_studentmodule', ['course_id', 'module_id', 'student_id']) + db.create_unique('courseware_studentmodule', ['student_id', 'module_id', 'course_id']) def backwards(self, orm): - # Removing unique constraint on 'StudentModule', fields ['course_id', 'module_state_key', 'student'] - db.delete_unique('courseware_studentmodule', ['course_id', 'module_id', 'student_id']) + # Removing unique constraint on 'StudentModule', fields ['studnet_id', 'module_state_key', 'course_id'] + db.delete_unique('courseware_studentmodule', ['student_id', 'module_id', 'course_id']) # Deleting field 'StudentModule.course_id' db.delete_column('courseware_studentmodule', 'course_id') @@ -30,6 +34,9 @@ class Migration(SchemaMigration): # Adding unique constraint on 'StudentModule', fields ['module_id', 'student'] db.create_unique('courseware_studentmodule', ['module_id', 'student_id']) + # Adding unique constraint on 'StudentModule', fields ['module_id', 'module_type', 'student'] + db.create_unique('courseware_studentmodule', ['student_id', 'module_id', 'module_type']) + models = { 'auth.group': { diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index 4fdedfcdd3..393cb0918b 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -43,7 +43,7 @@ class StudentModule(models.Model): course_id = models.CharField(max_length=255, db_index=True) class Meta: - unique_together = (('course_id', 'student', 'module_state_key'),) + unique_together = (('student', 'module_state_key', 'course_id'),) ## Internal state of the object state = models.TextField(null=True, blank=True) From d13998a13530622090634116a672a76c815c6c9d Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 20 Aug 2012 17:55:43 -0400 Subject: [PATCH 22/34] add docstring comment --- lms/djangoapps/courseware/grades.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index 6a0fedfc4b..1af3ab1bda 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -29,6 +29,8 @@ def grade(student, request, course, student_module_cache=None): output from the course grader, augmented with the final letter grade. The keys in the output are: + course: a CourseDescriptor + - grade : A final letter grade. - percent : The final percent for the class (rounded up). - section_breakdown : A breakdown of each section that makes From 9cc72c49657efa95c44a967c34ed2da4282df730 Mon Sep 17 00:00:00 2001 From: Bridger Maxwell Date: Tue, 21 Aug 2012 07:59:51 -0400 Subject: [PATCH 23/34] Victor's fixes only apply to sqlite. Added a conditional and now they run on mysql. --- .../courseware/migrations/0003_done_grade_cache.py | 2 +- .../migrations/0004_add_field_studentmodule_course_id.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/courseware/migrations/0003_done_grade_cache.py b/lms/djangoapps/courseware/migrations/0003_done_grade_cache.py index f29f931079..0ab35551c9 100644 --- a/lms/djangoapps/courseware/migrations/0003_done_grade_cache.py +++ b/lms/djangoapps/courseware/migrations/0003_done_grade_cache.py @@ -10,7 +10,7 @@ class Migration(SchemaMigration): def forwards(self, orm): # NOTE (vshnayder): This constraint has the wrong field order, so it doesn't actually - # do anything. Migration 0004 actually removes this index. + # do anything in sqlite. Migration 0004 actually removes this index for sqlite. # Removing unique constraint on 'StudentModule', fields ['module_id', 'module_type', 'student'] db.delete_unique('courseware_studentmodule', ['module_id', 'module_type', 'student_id']) diff --git a/lms/djangoapps/courseware/migrations/0004_add_field_studentmodule_course_id.py b/lms/djangoapps/courseware/migrations/0004_add_field_studentmodule_course_id.py index 7433a48b0b..ff79901824 100644 --- a/lms/djangoapps/courseware/migrations/0004_add_field_studentmodule_course_id.py +++ b/lms/djangoapps/courseware/migrations/0004_add_field_studentmodule_course_id.py @@ -16,9 +16,10 @@ class Migration(SchemaMigration): # Removing unique constraint on 'StudentModule', fields ['module_id', 'student'] db.delete_unique('courseware_studentmodule', ['module_id', 'student_id']) - # NOTE: manually remove this constaint (from 0001)--0003 tries, but fails. + # NOTE: manually remove this constaint (from 0001)--0003 tries, but fails for sqlite. # Removing unique constraint on 'StudentModule', fields ['module_id', 'module_type', 'student'] - db.delete_unique('courseware_studentmodule', ['student_id', 'module_id', 'module_type']) + if db.backend_name == "sqlite3": + db.delete_unique('courseware_studentmodule', ['student_id', 'module_id', 'module_type']) # Adding unique constraint on 'StudentModule', fields ['course_id', 'module_state_key', 'student'] db.create_unique('courseware_studentmodule', ['student_id', 'module_id', 'course_id']) From 2aa154c903362735d8c294cc8362d3bea8a276bd Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 21 Aug 2012 15:42:47 -0400 Subject: [PATCH 24/34] Initial pass at script to create links to test dirs. --- setup-test-dirs.sh | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 setup-test-dirs.sh diff --git a/setup-test-dirs.sh b/setup-test-dirs.sh new file mode 100644 index 0000000000..8b61e78394 --- /dev/null +++ b/setup-test-dirs.sh @@ -0,0 +1,42 @@ +#!/bin/bash + +# Create symlinks from ~/mitx_all/data or $ROOT/data, with root passed as first arg +# to all the test courses in mitx/common/test/data/ + +ROOT=$HOME/mitx_all + +# If there is a parameter, and it's a dir, assuming that's the path to +# the edX root dir, with data and mitx inside it +if [ -d "$1" ]; then + ROOT=$1 +fi + +if [ ! -d "$ROOT" ]; then + echo "'$ROOT' is not a directory" + exit 1 +fi + +if [ ! -d "$ROOT/mitx" ]; then + echo "'$ROOT' is not the root mitx_all directory" + exit 1 +fi + +if [ ! -d "$ROOT/data" ]; then + echo "'$ROOT' is not the root mitx_all directory" + exit 1 +fi + +echo "ROOT is $ROOT" + +cd $ROOT/data +for course in `ls ../mitx/common/test/data/` +do + # Get rid of the symlink if it already exists + echo "Make link to '$course'" + rm -f "$course" + # Create it + ln -s "../mitx/common/test/data/$course" +done + +# go back to where we came from +cd - From 8a112b0cca0194c36c71bb788f9f5667a5450de2 Mon Sep 17 00:00:00 2001 From: Kyle Fiedler Date: Tue, 21 Aug 2012 14:04:50 -0400 Subject: [PATCH 25/34] Added fix for cms sass --- cms/static/sass/_base.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cms/static/sass/_base.scss b/cms/static/sass/_base.scss index 7d16f0c6f9..410f74ee07 100644 --- a/cms/static/sass/_base.scss +++ b/cms/static/sass/_base.scss @@ -14,6 +14,10 @@ $yellow: #fff8af; $cream: #F6EFD4; $border-color: #ddd; +// edX colors +$blue: rgb(29,157,217); +$pink: rgb(182,37,104); + @mixin hide-text { background-color: transparent; border: 0; From 3273de93fe096eec7cee3e930d73fb39bb3969a8 Mon Sep 17 00:00:00 2001 From: Kyle Fiedler Date: Tue, 21 Aug 2012 15:29:53 -0400 Subject: [PATCH 26/34] make tutorial headers normal size --- lms/static/sass/course/courseware/_courseware.scss | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lms/static/sass/course/courseware/_courseware.scss b/lms/static/sass/course/courseware/_courseware.scss index 24ce46a9a3..1e7f13385a 100644 --- a/lms/static/sass/course/courseware/_courseware.scss +++ b/lms/static/sass/course/courseware/_courseware.scss @@ -58,18 +58,20 @@ div.course-wrapper { @extend h1.top-header; @include border-radius(0 4px 0 0); margin-bottom: -16px; + border-bottom: 0; h1 { margin: 0; + font-size: 1em; } h2 { float: right; - margin-right: 0; - margin-top: 8px; + margin: 12px 0 0; text-align: right; padding-right: 0; border-right: 0; + font-size: em(14, 24); } } From 1dc31ceb8b1cd8a0186b72f3158d09c0b50e4aa4 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 21 Aug 2012 16:09:35 -0400 Subject: [PATCH 27/34] fix toylab html --- common/test/data/toy/html/toylab.xml | 1 + 1 file changed, 1 insertion(+) create mode 100644 common/test/data/toy/html/toylab.xml diff --git a/common/test/data/toy/html/toylab.xml b/common/test/data/toy/html/toylab.xml new file mode 100644 index 0000000000..ab78aeb494 --- /dev/null +++ b/common/test/data/toy/html/toylab.xml @@ -0,0 +1 @@ + \ No newline at end of file From d3f1f0f8b69e2439ace92d314f056170f6b73a0f Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 21 Aug 2012 16:09:48 -0400 Subject: [PATCH 28/34] comment out debug messages --- common/lib/xmodule/xmodule/html_module.py | 2 +- common/lib/xmodule/xmodule/xml_module.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index c0b82fef90..6c424c26f2 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -86,7 +86,7 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor): # online and has imported all current (fall 2012) courses from xml if not system.resources_fs.exists(filepath): candidates = cls.backcompat_paths(filepath) - log.debug("candidates = {0}".format(candidates)) + #log.debug("candidates = {0}".format(candidates)) for candidate in candidates: if system.resources_fs.exists(candidate): filepath = candidate diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index c7042efda2..b6f791ffc6 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -38,7 +38,7 @@ def get_metadata_from_xml(xml_object, remove=True): if meta is None: return '' dmdata = meta.text - log.debug('meta for %s loaded: %s' % (xml_object,dmdata)) + #log.debug('meta for %s loaded: %s' % (xml_object,dmdata)) if remove: xml_object.remove(meta) return dmdata From a7a9abab4450501fe34185afc45ea426daca075d Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 21 Aug 2012 16:21:01 -0400 Subject: [PATCH 29/34] add a course to test start dates --- common/test/data/test_start_date/README.md | 1 + common/test/data/test_start_date/course.xml | 1 + .../data/test_start_date/course/2012_Fall.xml | 15 +++++++++++ .../data/test_start_date/html/toylab.html | 3 +++ .../test/data/test_start_date/html/toylab.xml | 1 + .../test_start_date/policies/2012_Fall.json | 27 +++++++++++++++++++ .../data/test_start_date/roots/2012_Fall.xml | 1 + 7 files changed, 49 insertions(+) create mode 100644 common/test/data/test_start_date/README.md create mode 120000 common/test/data/test_start_date/course.xml create mode 100644 common/test/data/test_start_date/course/2012_Fall.xml create mode 100644 common/test/data/test_start_date/html/toylab.html create mode 100644 common/test/data/test_start_date/html/toylab.xml create mode 100644 common/test/data/test_start_date/policies/2012_Fall.json create mode 100644 common/test/data/test_start_date/roots/2012_Fall.xml diff --git a/common/test/data/test_start_date/README.md b/common/test/data/test_start_date/README.md new file mode 100644 index 0000000000..0810107838 --- /dev/null +++ b/common/test/data/test_start_date/README.md @@ -0,0 +1 @@ +Simple course. If start dates are on, non-staff users should see Overview, but not Ch 2. diff --git a/common/test/data/test_start_date/course.xml b/common/test/data/test_start_date/course.xml new file mode 120000 index 0000000000..49041310f6 --- /dev/null +++ b/common/test/data/test_start_date/course.xml @@ -0,0 +1 @@ +roots/2012_Fall.xml \ No newline at end of file diff --git a/common/test/data/test_start_date/course/2012_Fall.xml b/common/test/data/test_start_date/course/2012_Fall.xml new file mode 100644 index 0000000000..77eca9f46c --- /dev/null +++ b/common/test/data/test_start_date/course/2012_Fall.xml @@ -0,0 +1,15 @@ + + + + + + + + +

Welcome

+ + + + diff --git a/common/test/data/test_start_date/html/toylab.html b/common/test/data/test_start_date/html/toylab.html new file mode 100644 index 0000000000..81df84bd63 --- /dev/null +++ b/common/test/data/test_start_date/html/toylab.html @@ -0,0 +1,3 @@ +Lab 2A: Superposition Experiment + +

Isn't the toy course great?

diff --git a/common/test/data/test_start_date/html/toylab.xml b/common/test/data/test_start_date/html/toylab.xml new file mode 100644 index 0000000000..ab78aeb494 --- /dev/null +++ b/common/test/data/test_start_date/html/toylab.xml @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/common/test/data/test_start_date/policies/2012_Fall.json b/common/test/data/test_start_date/policies/2012_Fall.json new file mode 100644 index 0000000000..a12ccecf1c --- /dev/null +++ b/common/test/data/test_start_date/policies/2012_Fall.json @@ -0,0 +1,27 @@ +{ + "course/2012_Fall": { + "graceperiod": "2 days 5 hours 59 minutes 59 seconds", + "start": "2011-07-17T12:00", + "display_name": "Toy Course" + }, + "chapter/Overview": { + "display_name": "Overview" + }, + "chapter/Ch2": { + "display_name": "Chapter 2", + "start": "2015-07-17T12:00" + }, + "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/test_start_date/roots/2012_Fall.xml b/common/test/data/test_start_date/roots/2012_Fall.xml new file mode 100644 index 0000000000..30dd5e0180 --- /dev/null +++ b/common/test/data/test_start_date/roots/2012_Fall.xml @@ -0,0 +1 @@ + \ No newline at end of file From 188584c77dedd5238249c79e3b4d8ca1c457231e Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 21 Aug 2012 16:21:43 -0400 Subject: [PATCH 30/34] Allow puppet to set COURSE_LISTINGS --- lms/envs/aws.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 059254bdff..a72ad92957 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -47,6 +47,7 @@ LOGGING = get_logger_config(LOG_DIR, syslog_addr=(ENV_TOKENS['SYSLOG_SERVER'], 514), debug=False) +COURSE_LISTINGS = ENV_TOKENS['COURSE_LISTINGS'] ############################## SECURE AUTH ITEMS ############################### # Secret things: passwords, access keys, etc. From b86fbdbd108b0aae353b481a9628ae58c017fca8 Mon Sep 17 00:00:00 2001 From: Arjun Singh Date: Tue, 21 Aug 2012 13:23:00 -0700 Subject: [PATCH 31/34] Adding comments about removing the syllabus tab ASAP. --- lms/djangoapps/courseware/courses.py | 3 +++ lms/envs/common.py | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 62d05cbc04..d975f48564 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -142,6 +142,9 @@ def get_course_info_section(course, section_key): raise KeyError("Invalid about key " + str(section_key)) +# TODO: Fix this such that these are pulled in as extra course-specific tabs. +# arjun will address this by the end of October if no one does so prior to +# then. def get_course_syllabus_section(course, section_key): """ This returns the snippet of html to be rendered on the syllabus page, diff --git a/lms/envs/common.py b/lms/envs/common.py index 464716940b..d6636282c7 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -55,8 +55,11 @@ MITX_FEATURES = { # course_ids (see dev_int.py for an example) 'SUBDOMAIN_COURSE_LISTINGS' : False, + # TODO: This will be removed once course-specific tabs are in place. see + # courseware/courses.py + 'ENABLE_SYLLABUS' : True, + 'ENABLE_TEXTBOOK' : True, - 'ENABLE_SYLLABUS' : True, 'ENABLE_DISCUSSION' : False, 'ENABLE_DISCUSSION_SERVICE': True, From 02ecfb473001efeb5f985150d99e074eb23f957a Mon Sep 17 00:00:00 2001 From: Arjun Singh Date: Tue, 21 Aug 2012 13:25:29 -0700 Subject: [PATCH 32/34] Adding comments about removing the syllabus tab ASAP. --- lms/djangoapps/courseware/views.py | 1 + lms/static/sass/course.scss | 2 +- lms/urls.py | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 446dd36064..460e8fd42f 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -233,6 +233,7 @@ def course_info(request, course_id): return render_to_response('courseware/info.html', {'course': course, 'staff_access': staff_access,}) +# TODO arjun: remove when custom tabs in place, see courseware/syllabus.py @ensure_csrf_cookie def syllabus(request, course_id): """ diff --git a/lms/static/sass/course.scss b/lms/static/sass/course.scss index 054e403289..93296aead6 100644 --- a/lms/static/sass/course.scss +++ b/lms/static/sass/course.scss @@ -29,7 +29,7 @@ // pages @import "course/info"; -@import "course/syllabus"; +@import "course/syllabus"; // TODO arjun replace w/ custom tabs, see courseware/courses.py @import "course/textbook"; @import "course/profile"; @import "course/gradebook"; diff --git a/lms/urls.py b/lms/urls.py index f65e45cc7a..9c3baf2f41 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -127,7 +127,7 @@ if settings.COURSEWARE_ENABLED: url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/info$', 'courseware.views.course_info', name="info"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/syllabus$', - 'courseware.views.syllabus', name="syllabus"), + 'courseware.views.syllabus', name="syllabus"), # TODO arjun remove when custom tabs in place, see courseware/courses.py url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/book/(?P[^/]*)/$', 'staticbook.views.index', name="book"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/book/(?P[^/]*)/(?P[^/]*)$', From be94e176ff521fe3fe987accc68548283a5ab803 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 21 Aug 2012 16:29:47 -0400 Subject: [PATCH 33/34] Add pytz to requirements --- requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements.txt b/requirements.txt index 7e69577073..c5cafe64b8 100644 --- a/requirements.txt +++ b/requirements.txt @@ -28,6 +28,7 @@ django-override-settings mock>=0.8, <0.9 PyYAML South +pytz django-celery django-countries django-kombu From 8be7d90c85863654c21dee06db5cf570977d81a3 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 21 Aug 2012 17:00:26 -0400 Subject: [PATCH 34/34] Address comments on #484. --- setup-test-dirs.sh | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) mode change 100644 => 100755 setup-test-dirs.sh diff --git a/setup-test-dirs.sh b/setup-test-dirs.sh old mode 100644 new mode 100755 index 8b61e78394..c4db61ec16 --- a/setup-test-dirs.sh +++ b/setup-test-dirs.sh @@ -1,27 +1,27 @@ -#!/bin/bash +#!/usr/bin/env bash # Create symlinks from ~/mitx_all/data or $ROOT/data, with root passed as first arg # to all the test courses in mitx/common/test/data/ -ROOT=$HOME/mitx_all - -# If there is a parameter, and it's a dir, assuming that's the path to -# the edX root dir, with data and mitx inside it -if [ -d "$1" ]; then - ROOT=$1 +# posix compliant sanity check +if [ -z $BASH ] || [ $BASH = "/bin/sh" ]; then +echo "Please use the bash interpreter to run this script" +exit 1 fi -if [ ! -d "$ROOT" ]; then +ROOT="${1:-$HOME/mitx_all}" + +if [[ ! -d "$ROOT" ]]; then echo "'$ROOT' is not a directory" exit 1 fi -if [ ! -d "$ROOT/mitx" ]; then +if [[ ! -d "$ROOT/mitx" ]]; then echo "'$ROOT' is not the root mitx_all directory" exit 1 fi -if [ ! -d "$ROOT/data" ]; then +if [[ ! -d "$ROOT/data" ]]; then echo "'$ROOT' is not the root mitx_all directory" exit 1 fi @@ -29,11 +29,15 @@ fi echo "ROOT is $ROOT" cd $ROOT/data -for course in `ls ../mitx/common/test/data/` + +for course in $(/bin/ls ../mitx/common/test/data/) do # Get rid of the symlink if it already exists + if [[ -L "$course" ]]; then + echo "Removing link to '$course'" + rm -f $course + fi echo "Make link to '$course'" - rm -f "$course" # Create it ln -s "../mitx/common/test/data/$course" done
datetimeusernameipaddrsourcetype
${rec.time}${rec.dtstr} ${rec.username} ${rec.ip} ${rec.event_source}