From 8e7eef24a2e1a1eda91787d1c22e3053cd7b6500 Mon Sep 17 00:00:00 2001 From: ichuang Date: Thu, 23 Aug 2012 13:10:08 -0400 Subject: [PATCH 001/181] add manage_course_groups command to lms_migration --- .../commands/manage_course_groups.py | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 lms/djangoapps/lms_migration/management/commands/manage_course_groups.py diff --git a/lms/djangoapps/lms_migration/management/commands/manage_course_groups.py b/lms/djangoapps/lms_migration/management/commands/manage_course_groups.py new file mode 100644 index 0000000000..0043b483e0 --- /dev/null +++ b/lms/djangoapps/lms_migration/management/commands/manage_course_groups.py @@ -0,0 +1,71 @@ +#!/usr/bin/python +# +# File: manage_course_groups +# +# interactively list and edit membership in course staff and instructor groups + +import os, sys, string, re +import datetime +from getpass import getpass +import json +import readline + +from django.core.management.base import BaseCommand +from django.conf import settings +from django.contrib.auth.models import User, Group + +#----------------------------------------------------------------------------- +# get all staff groups + +class Command(BaseCommand): + help = "Manage course group membership, interactively." + + def handle(self, *args, **options): + + 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 d94ef5454999fe738cefbf9555515f23b70ed692 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Andr=C3=A9s=20Rocha?= Date: Thu, 23 Aug 2012 16:30:58 -0400 Subject: [PATCH 002/181] Added cheatsheet popup to wiki editor. --- lms/static/sass/course/wiki/_wiki.scss | 23 ++++++++- lms/templates/wiki/base.html | 27 +++++++---- lms/templates/wiki/includes/cheatsheet.html | 53 +++++++++++++++++++++ 3 files changed, 93 insertions(+), 10 deletions(-) create mode 100644 lms/templates/wiki/includes/cheatsheet.html diff --git a/lms/static/sass/course/wiki/_wiki.scss b/lms/static/sass/course/wiki/_wiki.scss index bcc2c8855d..ead58bf6a6 100644 --- a/lms/static/sass/course/wiki/_wiki.scss +++ b/lms/static/sass/course/wiki/_wiki.scss @@ -567,11 +567,30 @@ section.wiki { background: #f00 !important; } - - + #cheatsheetLink { + text-align:right; + display: float; + } + #cheatsheetModal { + width: 350px; + margin-left: 100px; + margin-top: -100px; + } + #cheatsheet-body { + background: #FFF; + text-align: left; + padding: 10px; + } + #cheatsheet-body pre{ + color: #000; + text-align: left; + background: #EEE; + margin:10px; + padding: 10px; + } /*----------------- diff --git a/lms/templates/wiki/base.html b/lms/templates/wiki/base.html index fc4a2d18d4..a346be0e3e 100644 --- a/lms/templates/wiki/base.html +++ b/lms/templates/wiki/base.html @@ -27,21 +27,29 @@ }); } - - + + {% addtoblock 'js' %} {% comment %} These scripts load at the bottom of the body {% endcomment %} - + - + {% with mathjax_mode='wiki' %} {% include "mathjax_include.html" %} {% endwith %} - + {% endaddtoblock %} - + {% endblock %} @@ -64,11 +72,14 @@ {% endfor %} {% endif %} - + {% block wiki_contents %}{% endblock %} - + {% endblock %} + + {% include "wiki/includes/cheatsheet.html" %} + {% endblock %} diff --git a/lms/templates/wiki/includes/cheatsheet.html b/lms/templates/wiki/includes/cheatsheet.html new file mode 100644 index 0000000000..d58920b814 --- /dev/null +++ b/lms/templates/wiki/includes/cheatsheet.html @@ -0,0 +1,53 @@ + From 77f928d118a3c718dd583ec4bf63bef9fad19140 Mon Sep 17 00:00:00 2001 From: kimth Date: Thu, 23 Aug 2012 17:24:59 -0400 Subject: [PATCH 003/181] Queuestate records both secret key and time of queue request --- common/lib/capa/capa/capa_problem.py | 6 +----- common/lib/capa/capa/correctmap.py | 11 ++++++----- common/lib/capa/capa/responsetypes.py | 14 ++++++++------ common/lib/xmodule/xmodule/capa_module.py | 6 ++++++ 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index 82eb330174..35c8eaf635 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -202,11 +202,7 @@ class LoncapaProblem(object): ''' Returns True if any part of the problem has been submitted to an external queue ''' - queued = False - for answer_id in self.correct_map: - if self.correct_map.is_queued(answer_id): - queued = True - return queued + return any([self.correct_map.is_queued(answer_id) for answer_id in self.correct_map]) def grade_answers(self, answers): ''' diff --git a/common/lib/capa/capa/correctmap.py b/common/lib/capa/capa/correctmap.py index eb6ef2d00c..c6fc98e62f 100644 --- a/common/lib/capa/capa/correctmap.py +++ b/common/lib/capa/capa/correctmap.py @@ -15,7 +15,8 @@ class CorrectMap(object): - msg : string (may have HTML) giving extra message response (displayed below textline or textbox) - hint : string (may have HTML) giving optional hint (displayed below textline or textbox, above msg) - hintmode : one of (None,'on_request','always') criteria for displaying hint - - queuekey : a random integer for xqueue_callback verification + - queuestate : Tuple (key, time) where key is a secret string, and time is a string dump + of a DateTime object in the format '%Y%m%d%H%M%S'. Is None when not queued Behaves as a dict. ''' @@ -31,14 +32,14 @@ class CorrectMap(object): def __iter__(self): return self.cmap.__iter__() - def set(self, answer_id=None, correctness=None, npoints=None, msg='', hint='', hintmode=None, queuekey=None): + def set(self, answer_id=None, correctness=None, npoints=None, msg='', hint='', hintmode=None, queuestate=None): if answer_id is not None: self.cmap[answer_id] = {'correctness': correctness, 'npoints': npoints, 'msg': msg, 'hint': hint, 'hintmode': hintmode, - 'queuekey': queuekey, + 'queuestate': queuestate, } def __repr__(self): @@ -67,10 +68,10 @@ class CorrectMap(object): return None def is_queued(self, answer_id): - return answer_id in self.cmap and self.cmap[answer_id]['queuekey'] is not None + return answer_id in self.cmap and self.cmap[answer_id]['queuestate'] is not None def is_right_queuekey(self, answer_id, test_key): - return answer_id in self.cmap and self.cmap[answer_id]['queuekey'] == test_key + return self.is_queued(answer_id) and self.cmap[answer_id]['queuestate'][0] == test_key def get_npoints(self, answer_id): npoints = self.get_property(answer_id, 'npoints') diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index b2d56b48ca..35b8688a7b 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -26,6 +26,7 @@ import xml.sax.saxutils as saxutils # specific library imports from calc import evaluator, UndefinedVariable from correctmap import CorrectMap +from datetime import datetime from util import * from lxml import etree from lxml.html.soupparser import fromstring as fromstring_bs # uses Beautiful Soup!!! FIXME? @@ -1026,7 +1027,7 @@ class CodeResponse(LoncapaResponse): TODO: Determines whether in synchronous or asynchronous (queued) mode ''' xml = self.xml - self.url = xml.get('url', None) # XML can override external resource (grader/queue) URL + self.url = xml.get('url', None) # TODO: XML can override external resource (grader/queue) URL self.queue_name = xml.get('queuename', self.system.xqueue['default_queuename']) # VS[compat]: @@ -1128,7 +1129,7 @@ class CodeResponse(LoncapaResponse): xheader = xqueue_interface.make_xheader(lms_callback_url=self.system.xqueue['callback_url'], lms_key=queuekey, queue_name=self.queue_name) - + # Generate body if is_list_of_files(submission): self.context.update({'submission': queuekey}) # For tracking. TODO: May want to record something else here @@ -1148,16 +1149,17 @@ class CodeResponse(LoncapaResponse): (error, msg) = qinterface.send_to_queue(header=xheader, body=json.dumps(contents)) + queuestate = (queuekey,'') cmap = CorrectMap() if error: - cmap.set(self.answer_id, queuekey=None, + cmap.set(self.answer_id, queuestate=None, msg='Unable to deliver your submission to grader. (Reason: %s.) Please try again later.' % msg) else: # Queueing mechanism flags: - # 1) Backend: Non-null CorrectMap['queuekey'] indicates that the problem has been queued + # 1) Backend: Non-null CorrectMap['queuestate'] indicates that the problem has been queued # 2) Frontend: correctness='incomplete' eventually trickles down through inputtypes.textbox # and .filesubmission to inform the browser to poll the LMS - cmap.set(self.answer_id, queuekey=queuekey, correctness='incomplete', msg=msg) + cmap.set(self.answer_id, queuestate=queuestate, correctness='incomplete', msg=msg) return cmap @@ -1180,7 +1182,7 @@ class CodeResponse(LoncapaResponse): points = 0 elif points > self.maxpoints[self.answer_id]: points = self.maxpoints[self.answer_id] - oldcmap.set(self.answer_id, npoints=points, correctness=correctness, msg=msg.replace(' ', ' '), queuekey=None) # Queuekey is consumed + oldcmap.set(self.answer_id, npoints=points, correctness=correctness, msg=msg.replace(' ', ' '), queuestate=None) # Queuestate is consumed else: log.debug('CodeResponse: queuekey %s does not match for answer_id=%s.' % (queuekey, self.answer_id)) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index d2ed3912a4..6b6c0991c5 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -462,6 +462,12 @@ class CapaModule(XModule): self.system.track_function('save_problem_check_fail', event_info) raise NotFoundError('Problem must be reset before it can be checked again') + # Problem queued. Student should not be able to submit + ''' + if self.lcp.is_queued(): + return {'success': False, 'html': 'Already queued'} + ''' + try: old_state = self.lcp.get_state() lcp_id = self.lcp.problem_id From 673cc2bbb393b3cff6d3f7c2e5e8cbd3837bd73f Mon Sep 17 00:00:00 2001 From: kimth Date: Thu, 23 Aug 2012 17:43:38 -0400 Subject: [PATCH 004/181] Keep queueuing time --- common/lib/capa/capa/correctmap.py | 3 +++ common/lib/capa/capa/responsetypes.py | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/common/lib/capa/capa/correctmap.py b/common/lib/capa/capa/correctmap.py index c6fc98e62f..4659e2d1e2 100644 --- a/common/lib/capa/capa/correctmap.py +++ b/common/lib/capa/capa/correctmap.py @@ -73,6 +73,9 @@ class CorrectMap(object): def is_right_queuekey(self, answer_id, test_key): return self.is_queued(answer_id) and self.cmap[answer_id]['queuestate'][0] == test_key + def get_queuetime_str(self, answer_id): + return self.cmap[answer_id]['queuestate'][1] if self.is_queued(answer_id) else None + def get_npoints(self, answer_id): npoints = self.get_property(answer_id, 'npoints') if npoints is not None: diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 35b8688a7b..27801ad871 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1149,7 +1149,10 @@ class CodeResponse(LoncapaResponse): (error, msg) = qinterface.send_to_queue(header=xheader, body=json.dumps(contents)) - queuestate = (queuekey,'') + # State associated with the queueing request + qtime = datetime.strftime(datetime.now(), '%Y%m%d%H%M%S') + queuestate = (queuekey, qtime) + cmap = CorrectMap() if error: cmap.set(self.answer_id, queuestate=None, From 95a7a5b08b51455463c6400666efe5acf59c7316 Mon Sep 17 00:00:00 2001 From: kimth Date: Thu, 23 Aug 2012 18:39:57 -0400 Subject: [PATCH 005/181] Student must wait XQUEUE_WAITTIME_BETWEEN_REQUESTS between queueing submissions --- common/lib/capa/capa/capa_problem.py | 16 ++++++++++++++++ common/lib/xmodule/xmodule/capa_module.py | 13 ++++++++----- lms/envs/dev.py | 1 + 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index 35c8eaf635..ee24ebd031 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -29,6 +29,7 @@ from xml.sax.saxutils import unescape import calc from correctmap import CorrectMap +from datetime import datetime import eia import inputtypes from util import contextualize_text, convert_files_to_filenames @@ -204,6 +205,21 @@ class LoncapaProblem(object): ''' return any([self.correct_map.is_queued(answer_id) for answer_id in self.correct_map]) + + def get_recentmost_queuetime(self): + ''' + Returns a DateTime object that represents the timestamp of the most recent queueing request, or None if not queued + ''' + if not self.is_queued(): + return None + + # Get a list of timestamps of all queueing requests, then convert it to a DateTime object + queuetimes = [self.correct_map.get_queuetime_str(answer_id) for answer_id in self.correct_map if self.correct_map.is_queued(answer_id)] + queuetimes = [datetime.strptime(qt,'%Y%m%d%H%M%S') for qt in queuetimes] + + return max(queuetimes) + + def grade_answers(self, answers): ''' Grade student responses. Called by capa_module.check_problem. diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 6b6c0991c5..8f570bcf6c 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -7,7 +7,8 @@ import traceback import re import sys -from datetime import timedelta +from datetime import datetime, timedelta +from django.conf import settings from lxml import etree from pkg_resources import resource_string @@ -462,11 +463,13 @@ class CapaModule(XModule): self.system.track_function('save_problem_check_fail', event_info) raise NotFoundError('Problem must be reset before it can be checked again') - # Problem queued. Student should not be able to submit - ''' + # Problem queued. Students must wait XQUEUE_WAITTIME_BETWEEN_REQUESTS if self.lcp.is_queued(): - return {'success': False, 'html': 'Already queued'} - ''' + current_time = datetime.now() + prev_submit_time = self.lcp.get_recentmost_queuetime() + if (current_time-prev_submit_time).total_seconds() < settings.XQUEUE_WAITTIME_BETWEEN_REQUESTS: + msg = 'You must wait %d seconds between queueing requests' % settings.XQUEUE_WAITTIME_BETWEEN_REQUESTS + return {'success': msg, 'html': ''} try: old_state = self.lcp.get_state() diff --git a/lms/envs/dev.py b/lms/envs/dev.py index a76e6de262..fe12ee3322 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -63,6 +63,7 @@ XQUEUE_INTERFACE = { }, "basic_auth": ('anant', 'agarwal'), } +XQUEUE_WAITTIME_BETWEEN_REQUESTS = 5 # seconds # Make the keyedcache startup warnings go away CACHE_TIMEOUT = 0 From fdecaef7eff4e4b4e83cf7ae840c80100bad1370 Mon Sep 17 00:00:00 2001 From: kimth Date: Fri, 24 Aug 2012 08:23:46 -0400 Subject: [PATCH 006/181] Fix datetime name conflict --- common/lib/xmodule/xmodule/capa_module.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 8f570bcf6c..33a2f6b4c4 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -7,7 +7,7 @@ import traceback import re import sys -from datetime import datetime, timedelta +from datetime import timedelta from django.conf import settings from lxml import etree from pkg_resources import resource_string @@ -465,10 +465,11 @@ class CapaModule(XModule): # Problem queued. Students must wait XQUEUE_WAITTIME_BETWEEN_REQUESTS if self.lcp.is_queued(): - current_time = datetime.now() + current_time = datetime.datetime.now() prev_submit_time = self.lcp.get_recentmost_queuetime() - if (current_time-prev_submit_time).total_seconds() < settings.XQUEUE_WAITTIME_BETWEEN_REQUESTS: - msg = 'You must wait %d seconds between queueing requests' % settings.XQUEUE_WAITTIME_BETWEEN_REQUESTS + waittime_between_requests = settings.XQUEUE_WAITTIME_BETWEEN_REQUESTS + if (current_time-prev_submit_time).total_seconds() < waittime_between_requests: + msg = 'You must wait at least %d seconds between queue submissions' % waittime_between_requests return {'success': msg, 'html': ''} try: From 97ab53e786c03453a69aeaa1cd7e4462ae37a9dd Mon Sep 17 00:00:00 2001 From: kimth Date: Fri, 24 Aug 2012 08:28:47 -0400 Subject: [PATCH 007/181] Queuekey --> Queuestate in tests --- common/lib/xmodule/xmodule/tests/__init__.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 2a380bb8be..5d70056d2a 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -292,7 +292,9 @@ class CodeResponseTest(unittest.TestCase): answer_ids = sorted(test_lcp.get_question_answers().keys()) numAnswers = len(answer_ids) for i in range(numAnswers): - old_cmap.update(CorrectMap(answer_id=answer_ids[i], queuekey=1000 + i)) + queuekey = 1000 + i + queuestate = (queuekey, '') + old_cmap.update(CorrectMap(answer_id=answer_ids[i], queuestate=queuestate)) # TODO: Message format inherited from ExternalResponse #correct_score_msg = "EXACT_ANSMESSAGE" @@ -326,7 +328,7 @@ class CodeResponseTest(unittest.TestCase): new_cmap = CorrectMap() new_cmap.update(old_cmap) npoints = 1 if correctness=='correct' else 0 - new_cmap.set(answer_id=answer_ids[i], npoints=npoints, correctness=correctness, msg='MESSAGE', queuekey=None) + new_cmap.set(answer_id=answer_ids[i], npoints=npoints, correctness=correctness, msg='MESSAGE', queuestate=None) test_lcp.update_score(xserver_msgs[correctness], queuekey=1000 + i) self.assertEquals(test_lcp.correct_map.get_dict(), new_cmap.get_dict()) From 088d204d9cbd0d757f66f25be982ba4ba1d8ca3a Mon Sep 17 00:00:00 2001 From: kimth Date: Fri, 24 Aug 2012 11:09:27 -0400 Subject: [PATCH 008/181] Added CodeResponse tests --- common/lib/capa/capa/responsetypes.py | 2 +- common/lib/xmodule/xmodule/tests/__init__.py | 100 ++++++++++++++--- .../xmodule/tests/test_files/coderesponse.xml | 90 ++-------------- .../coderesponse_externalresponseformat.xml | 101 ++++++++++++++++++ lms/envs/test.py | 2 +- 5 files changed, 199 insertions(+), 96 deletions(-) create mode 100644 common/lib/xmodule/xmodule/tests/test_files/coderesponse_externalresponseformat.xml diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 27801ad871..5cf45adce2 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1202,7 +1202,7 @@ class CodeResponse(LoncapaResponse): ''' Grader reply is a JSON-dump of the following dict { 'correct': True/False, - 'score': # TODO -- Partial grading + 'score': Numeric value (floating point is okay) to assign to answer 'msg': grader_msg } Returns (valid_score_msg, correct, score, msg): diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 5d70056d2a..bd5d35c75f 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -19,6 +19,7 @@ import capa.calc as calc import capa.capa_problem as lcp from capa.correctmap import CorrectMap from capa.util import convert_files_to_filenames +from datetime import datetime from xmodule import graders, x_module from xmodule.x_module import ModuleSystem from xmodule.graders import Score, aggregate_scores @@ -283,30 +284,61 @@ class CodeResponseTest(unittest.TestCase): ''' Test CodeResponse ''' + @staticmethod + def make_queuestate(key, time): + timestr = datetime.strftime(time,'%Y%m%d%H%M%S') + return (key, timestr) + + def test_is_queued(self): + ''' + Simple test of whether LoncapaProblem knows when it's been queued + ''' + problem_file = os.path.dirname(__file__) + "/test_files/coderesponse.xml" + test_lcp = lcp.LoncapaProblem(open(problem_file).read(), '1', system=i4xs) + + answer_ids = sorted(test_lcp.get_question_answers().keys()) + num_answers = len(answer_ids) + + # CodeResponse requires internal CorrectMap state. Build it now in the unqueued state + cmap = CorrectMap() + for i in range(num_answers): + cmap.update(CorrectMap(answer_id=answer_ids[i], queuestate=None)) + test_lcp.correct_map.update(cmap) + + self.assertEquals(test_lcp.is_queued(), False) + + # Now we queue the LCP + cmap = CorrectMap() + for i in range(num_answers): + queuestate = CodeResponseTest.make_queuestate(i, datetime.now()) + cmap.update(CorrectMap(answer_id=answer_ids[i], queuestate=queuestate)) + test_lcp.correct_map.update(cmap) + + self.assertEquals(test_lcp.is_queued(), True) + def test_update_score(self): + ''' + Test whether LoncapaProblem.update_score can deliver queued result to the right subproblem + ''' problem_file = os.path.dirname(__file__) + "/test_files/coderesponse.xml" test_lcp = lcp.LoncapaProblem(open(problem_file).read(), '1', system=i4xs) - # CodeResponse requires internal CorrectMap state. Build it now in the 'queued' state - old_cmap = CorrectMap() answer_ids = sorted(test_lcp.get_question_answers().keys()) - numAnswers = len(answer_ids) - for i in range(numAnswers): + num_answers = len(answer_ids) + + # CodeResponse requires internal CorrectMap state. Build it now in the queued state + old_cmap = CorrectMap() + for i in range(num_answers): queuekey = 1000 + i - queuestate = (queuekey, '') + queuestate = CodeResponseTest.make_queuestate(1000+i, datetime.now()) old_cmap.update(CorrectMap(answer_id=answer_ids[i], queuestate=queuestate)) - # TODO: Message format inherited from ExternalResponse - #correct_score_msg = "EXACT_ANSMESSAGE" - #incorrect_score_msg = "WRONG_FORMATMESSAGE" - - # New message format common to external graders + # Message format common to external graders correct_score_msg = json.dumps({'correct':True, 'score':1, 'msg':'MESSAGE'}) incorrect_score_msg = json.dumps({'correct':False, 'score':0, 'msg':'MESSAGE'}) xserver_msgs = {'correct': correct_score_msg, - 'incorrect': incorrect_score_msg, - } + 'incorrect': incorrect_score_msg,} # Incorrect queuekey, state should not be updated for correctness in ['correct', 'incorrect']: @@ -316,12 +348,12 @@ class CodeResponseTest(unittest.TestCase): test_lcp.update_score(xserver_msgs[correctness], queuekey=0) self.assertEquals(test_lcp.correct_map.get_dict(), old_cmap.get_dict()) # Deep comparison - for i in range(numAnswers): + for i in range(num_answers): self.assertTrue(test_lcp.correct_map.is_queued(answer_ids[i])) # Should be still queued, since message undelivered # Correct queuekey, state should be updated for correctness in ['correct', 'incorrect']: - for i in range(numAnswers): # Target specific answer_id's + for i in range(num_answers): # Target specific answer_id's test_lcp.correct_map = CorrectMap() test_lcp.correct_map.update(old_cmap) @@ -333,13 +365,51 @@ class CodeResponseTest(unittest.TestCase): test_lcp.update_score(xserver_msgs[correctness], queuekey=1000 + i) self.assertEquals(test_lcp.correct_map.get_dict(), new_cmap.get_dict()) - for j in range(numAnswers): + for j in range(num_answers): if j == i: self.assertFalse(test_lcp.correct_map.is_queued(answer_ids[j])) # Should be dequeued, message delivered else: self.assertTrue(test_lcp.correct_map.is_queued(answer_ids[j])) # Should be queued, message undelivered + + + def test_recentmost_queuetime(self): + ''' + Test whether the LoncapaProblem knows about the time of queue requests + ''' + problem_file = os.path.dirname(__file__) + "/test_files/coderesponse.xml" + test_lcp = lcp.LoncapaProblem(open(problem_file).read(), '1', system=i4xs) + + answer_ids = sorted(test_lcp.get_question_answers().keys()) + num_answers = len(answer_ids) + + # CodeResponse requires internal CorrectMap state. Build it now in the unqueued state + cmap = CorrectMap() + for i in range(num_answers): + cmap.update(CorrectMap(answer_id=answer_ids[i], queuestate=None)) + test_lcp.correct_map.update(cmap) + + self.assertEquals(test_lcp.get_recentmost_queuetime(), None) + + # CodeResponse requires internal CorrectMap state. Build it now in the queued state + cmap = CorrectMap() + answer_ids = sorted(test_lcp.get_question_answers().keys()) + num_answers = len(answer_ids) + for i in range(num_answers): + queuekey = 1000 + i + latest_timestamp = datetime.now() + queuestate = CodeResponseTest.make_queuestate(1000+i, latest_timestamp) + cmap.update(CorrectMap(answer_id=answer_ids[i], queuestate=queuestate)) + test_lcp.correct_map.update(cmap) + + # Queue state only tracks up to second + latest_timestamp = datetime.strptime(datetime.strftime(latest_timestamp,'%Y%m%d%H%M%S'),'%Y%m%d%H%M%S') + + self.assertEquals(test_lcp.get_recentmost_queuetime(), latest_timestamp) def test_convert_files_to_filenames(self): + ''' + Test whether file objects are converted to filenames without altering other structures + ''' problem_file = os.path.dirname(__file__) + "/test_files/coderesponse.xml" fp = open(problem_file) answers_with_file = {'1_2_1': 'String-based answer', diff --git a/common/lib/xmodule/xmodule/tests/test_files/coderesponse.xml b/common/lib/xmodule/xmodule/tests/test_files/coderesponse.xml index 42b6e0a54a..1c0bf8d4e6 100644 --- a/common/lib/xmodule/xmodule/tests/test_files/coderesponse.xml +++ b/common/lib/xmodule/xmodule/tests/test_files/coderesponse.xml @@ -9,91 +9,23 @@ Write a program to compute the square of a number - - + + def square(x): + answer + grader stuff + -Write a program to compute the cube of a number +Write a program to compute the square of a number - - + + def square(x): + answer + grader stuff + diff --git a/common/lib/xmodule/xmodule/tests/test_files/coderesponse_externalresponseformat.xml b/common/lib/xmodule/xmodule/tests/test_files/coderesponse_externalresponseformat.xml new file mode 100644 index 0000000000..42b6e0a54a --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/test_files/coderesponse_externalresponseformat.xml @@ -0,0 +1,101 @@ + + +

Code response

+ +

+

+ + +Write a program to compute the square of a number + + + + + + + + +Write a program to compute the cube of a number + + + + + + + +
+
diff --git a/lms/envs/test.py b/lms/envs/test.py index 1ab3f550b8..c164889d79 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -58,7 +58,7 @@ XQUEUE_INTERFACE = { }, "basic_auth": ('anant', 'agarwal'), } - +XQUEUE_WAITTIME_BETWEEN_REQUESTS = 5 # seconds # TODO (cpennington): We need to figure out how envs/test.py can inject things # into common.py so that we don't have to repeat this sort of thing From 3787307d93eeb04a37003490c7aa260782fb0477 Mon Sep 17 00:00:00 2001 From: kimth Date: Fri, 24 Aug 2012 11:23:26 -0400 Subject: [PATCH 009/181] Put WAITTIME_BETWEEN_REQUESTS in common rather than dev env --- lms/envs/common.py | 3 +++ lms/envs/dev.py | 1 - 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lms/envs/common.py b/lms/envs/common.py index 938c4036ae..ce08bf9666 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -86,6 +86,9 @@ DEFAULT_GROUPS = [] # If this is true, random scores will be generated for the purpose of debugging the profile graphs GENERATE_PROFILE_SCORES = False +# Used with XQueue +XQUEUE_WAITTIME_BETWEEN_REQUESTS = 5 # seconds + ############################# SET PATH INFORMATION ############################# PROJECT_ROOT = path(__file__).abspath().dirname().dirname() # /mitx/lms REPO_ROOT = PROJECT_ROOT.dirname() diff --git a/lms/envs/dev.py b/lms/envs/dev.py index fe12ee3322..a76e6de262 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -63,7 +63,6 @@ XQUEUE_INTERFACE = { }, "basic_auth": ('anant', 'agarwal'), } -XQUEUE_WAITTIME_BETWEEN_REQUESTS = 5 # seconds # Make the keyedcache startup warnings go away CACHE_TIMEOUT = 0 From 2cf612051c0cbba063527217c6dd9369b335a62a Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Fri, 24 Aug 2012 13:53:32 -0400 Subject: [PATCH 010/181] turn subdomain listings off by default in dev env --- lms/envs/dev.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/envs/dev.py b/lms/envs/dev.py index a76e6de262..97803cb47d 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -15,7 +15,7 @@ TEMPLATE_DEBUG = True MITX_FEATURES['DISABLE_START_DATES'] = True MITX_FEATURES['ENABLE_SQL_TRACKING_LOGS'] = True -MITX_FEATURES['SUBDOMAIN_COURSE_LISTINGS'] = True +MITX_FEATURES['SUBDOMAIN_COURSE_LISTINGS'] = False # Enable to test subdomains--otherwise, want all courses to show up MITX_FEATURES['SUBDOMAIN_BRANDING'] = True WIKI_ENABLED = True @@ -107,7 +107,7 @@ LMS_MIGRATION_ALLOWED_IPS = ['127.0.0.1'] MITX_FEATURES['AUTH_USE_OPENID'] = True MITX_FEATURES['BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH'] = True -INSTALLED_APPS += ('external_auth',) +INSTALLED_APPS += ('external_auth',) INSTALLED_APPS += ('django_openid_auth',) OPENID_CREATE_USERS = False From 7a67fdc4ed54cf2f766114b02f7b4c3c5284bca8 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Fri, 24 Aug 2012 13:54:46 -0400 Subject: [PATCH 011/181] Fix url_name loading - cleaned up name loading - clear fallbacks, including hashing content if no name specified - log errors in error tracker for content devs to see --- common/lib/xmodule/xmodule/modulestore/xml.py | 90 ++++++++++++++----- 1 file changed, 66 insertions(+), 24 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 3eca72987e..98260f5982 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -1,3 +1,4 @@ +import hashlib import json import logging import os @@ -43,14 +44,76 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): xmlstore: the XMLModuleStore to store the loaded modules in """ - self.unnamed_modules = 0 - self.used_slugs = set() + self.unnamed = defaultdict(int) # category -> num of new url_names for that category + self.used_names = defaultdict(set) # category -> set of used url_names self.org, self.course, self.url_name = course_id.split('/') def process_xml(xml): """Takes an xml string, and returns a XModuleDescriptor created from that xml. """ + + def make_name_unique(xml_data): + """ + Make sure that the url_name of xml_data is unique. If a previously loaded + unnamed descriptor stole this element's url_name, create a new one. + + Removes 'slug' attribute if present, and adds or overwrites the 'url_name' attribute. + """ + # VS[compat]. Take this out once course conversion is done (perhaps leave the uniqueness check) + + attr = xml_data.attrib + tag = xml_data.tag + id = lambda x: x + # Things to try to get a name, in order (key, cleaning function, remove key after reading?) + lookups = [('url_name', id, False), + ('slug', id, True), + ('name', Location.clean, False), + ('display_name', Location.clean, False)] + + url_name = None + for key, clean, remove in lookups: + if key in attr: + url_name = clean(attr[key]) + if remove: + del attr[key] + break + + def fallback_name(): + """Return the fallback name for this module. This is a function instead of a variable + because we want it to be lazy.""" + return tag + "_" + hashlib.sha1(xml).hexdigest()[:12] + + # Fallback if there was nothing we could use: + if url_name is None or url_name == "": + # use the hash of the content--the first 12 bytes should be plenty. + url_name = fallback_name() + # Don't log a warning--we don't need this in the log. Do + # put it in the error tracker--content folks need to see it. + need_uniq_names = ('problem', 'sequence', 'video', 'course', 'chapter') + + if tag in need_uniq_names: + error_tracker("ERROR: no name of any kind specified for {tag}. Student " + "state won't work right. Problem xml: '{xml}...'".format(tag=tag, xml=xml[:100])) + else: + # TODO (vshnayder): We may want to enable this once course repos are cleaned up. + # (or we may want to give up on the requirement for non-state-relevant issues...) + #error_tracker("WARNING: no name specified for module. xml='{0}...'".format(xml[:100])) + pass + + # Make sure everything is unique + if url_name in self.used_names[tag]: + msg = ("Non-unique url_name in xml. This may break content. url_name={0}. Content={1}" + .format(url_name, xml[:100])) + error_tracker("ERROR: " + msg) + log.warning(msg) + # Just set name to fallback_name--if there are multiple things with the same fallback name, + # they are actually identical, so it's fragile, but not immediately broken. + url_name = fallback_name() + + self.used_names[tag].add(url_name) + xml_data.set('url_name', url_name) + try: # VS[compat] # TODO (cpennington): Remove this once all fall 2012 courses @@ -62,32 +125,11 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): err=str(err), xml=xml)) raise - # VS[compat]. Take this out once course conversion is done - if xml_data.get('slug') is None and xml_data.get('url_name') is None: - if xml_data.get('name'): - slug = Location.clean(xml_data.get('name')) - elif xml_data.get('display_name'): - slug = Location.clean(xml_data.get('display_name')) - else: - self.unnamed_modules += 1 - slug = '{tag}_{count}'.format(tag=xml_data.tag, - count=self.unnamed_modules) - - while slug in self.used_slugs: - self.unnamed_modules += 1 - slug = '{slug}_{count}'.format(slug=slug, - count=self.unnamed_modules) - - self.used_slugs.add(slug) - # log.debug('-> slug=%s' % slug) - xml_data.set('url_name', slug) + make_name_unique(xml_data) descriptor = XModuleDescriptor.load_from_xml( etree.tostring(xml_data), self, self.org, self.course, xmlstore.default_class) - - #log.debug('==> importing descriptor location %s' % - # repr(descriptor.location)) descriptor.metadata['data_dir'] = course_dir xmlstore.modules[course_id][descriptor.location] = descriptor From dc52956c28d7b112cc4711994ad35a23bb939259 Mon Sep 17 00:00:00 2001 From: ichuang Date: Fri, 24 Aug 2012 14:09:57 -0400 Subject: [PATCH 012/181] don't list all users if too many --- .../management/commands/manage_course_groups.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/lms_migration/management/commands/manage_course_groups.py b/lms/djangoapps/lms_migration/management/commands/manage_course_groups.py index 0043b483e0..f3a39db5ca 100644 --- a/lms/djangoapps/lms_migration/management/commands/manage_course_groups.py +++ b/lms/djangoapps/lms_migration/management/commands/manage_course_groups.py @@ -36,9 +36,14 @@ class Command(BaseCommand): # users in group uall = User.objects.all() - print "----" - print "List of All Users: %s" % [str(x.username) for x in uall] - print "----" + if uall.count()<50: + print "----" + print "List of All Users: %s" % [str(x.username) for x in uall] + print "----" + else: + print "----" + print "There are %d users, which is too many to list" % uall.count() + print "----" while True: From d1760f9a22a075648190003f9647bc64ba2cbfb0 Mon Sep 17 00:00:00 2001 From: Tom Giannattasio Date: Fri, 24 Aug 2012 16:14:04 -0400 Subject: [PATCH 013/181] styled markdown modal and link --- lms/static/sass/course/wiki/_wiki.scss | 78 ++++++++++++++--- lms/templates/wiki/base.html | 5 +- lms/templates/wiki/includes/cheatsheet.html | 95 +++++++++++---------- 3 files changed, 117 insertions(+), 61 deletions(-) diff --git a/lms/static/sass/course/wiki/_wiki.scss b/lms/static/sass/course/wiki/_wiki.scss index ead58bf6a6..912b416ff6 100644 --- a/lms/static/sass/course/wiki/_wiki.scss +++ b/lms/static/sass/course/wiki/_wiki.scss @@ -567,29 +567,79 @@ section.wiki { background: #f00 !important; } + .cheatsheet { + float: right; + position: relative; + top: -26px; + font-size: 12px; + } + #cheatsheetLink { - text-align:right; - display: float; + text-align: right; + display: float; } #cheatsheetModal { - width: 350px; - margin-left: 100px; - margin-top: -100px; - } + width: 950px; + margin-left: -450px; + margin-top: -100px; + + .left-column { + margin-right: 10px; + } + + .left-column, + .right-column { + float: left; + width: 450px; + } + + .close-btn { + display: block; + position: absolute; + top: -8px; + right: -8px; + width: 30px; + height: 30px; + border-radius: 30px; + border: 1px solid #ccc; + @include linear-gradient(top, #eee, #d2d2d2); + font-size: 22px; + line-height: 28px; + color: #333; + text-align: center; + @include box-shadow(0 1px 0 #fff inset, 0 1px 2px rgba(0, 0, 0, .2)); + } + } #cheatsheet-body { - background: #FFF; - text-align: left; - padding: 10px; + background: #fff; + text-align: left; + padding: 20px; + font-size: 14px; + @include clearfix; + + h3 { + font-weight: bold; + } + + ul { + list-style: circle; + line-height: 1.4; + color: #333; + } + } + + #cheatsheet-body section + section { + margin-top: 40px; } #cheatsheet-body pre{ - color: #000; - text-align: left; - background: #EEE; - margin:10px; - padding: 10px; + color: #000; + text-align: left; + background: #eee; + padding: 10px; + font-size: 12px; } /*----------------- diff --git a/lms/templates/wiki/base.html b/lms/templates/wiki/base.html index a346be0e3e..5c7381ac05 100644 --- a/lms/templates/wiki/base.html +++ b/lms/templates/wiki/base.html @@ -42,9 +42,12 @@ diff --git a/lms/templates/wiki/includes/cheatsheet.html b/lms/templates/wiki/includes/cheatsheet.html index d58920b814..429c1ea22e 100644 --- a/lms/templates/wiki/includes/cheatsheet.html +++ b/lms/templates/wiki/includes/cheatsheet.html @@ -1,53 +1,56 @@ - + + + + From 45a64a9bd4cc33968c0cf5890eb46e5285f4e36f Mon Sep 17 00:00:00 2001 From: kimth Date: Fri, 24 Aug 2012 16:33:05 -0400 Subject: [PATCH 014/181] Hide queue length information from the student --- common/lib/capa/capa/inputtypes.py | 2 +- common/lib/capa/capa/templates/textbox.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py index ad54736359..9290503c9d 100644 --- a/common/lib/capa/capa/inputtypes.py +++ b/common/lib/capa/capa/inputtypes.py @@ -384,7 +384,7 @@ def textbox(element, value, status, render_template, msg=''): if status == 'incomplete': # Flag indicating that the problem has been queued, 'msg' is length of queue status = 'queued' queue_len = msg - msg = 'Submitted to grader. (Queue length: %s)' % queue_len + msg = 'Submitted to grader.' # For CodeMirror mode = element.get('mode','python') diff --git a/common/lib/capa/capa/templates/textbox.html b/common/lib/capa/capa/templates/textbox.html index 647f4fe4e8..19c43482a8 100644 --- a/common/lib/capa/capa/templates/textbox.html +++ b/common/lib/capa/capa/templates/textbox.html @@ -21,7 +21,7 @@
% endif
- (${state}) +
${msg|n}
From 1dcaf21f8131edb7f0de95b51fac47ac999c8303 Mon Sep 17 00:00:00 2001 From: kimth Date: Sat, 25 Aug 2012 07:57:17 -0400 Subject: [PATCH 015/181] Send idhash --- common/lib/capa/capa/responsetypes.py | 10 +++++++++- common/lib/capa/capa/xqueue_interface.py | 7 ++----- lms/djangoapps/courseware/module_render.py | 4 +++- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index b2d56b48ca..032970e324 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -16,6 +16,7 @@ import numpy import random import re import requests +import time import traceback import hashlib import abc @@ -1124,7 +1125,9 @@ class CodeResponse(LoncapaResponse): qinterface = self.system.xqueue['interface'] # Generate header - queuekey = xqueue_interface.make_hashkey(str(self.system.seed)+self.answer_id) + queuekey = xqueue_interface.make_hashkey(str(self.system.seed) + str(time.time()) + + str(self.system.xqueue['student_identifier']) + + self.answer_id) xheader = xqueue_interface.make_xheader(lms_callback_url=self.system.xqueue['callback_url'], lms_key=queuekey, queue_name=self.queue_name) @@ -1137,6 +1140,11 @@ class CodeResponse(LoncapaResponse): contents = self.payload.copy() + # Anonymized student identifier to the external grader + student_identifier_hash = xqueue_interface.make_hashkey(self.system.xqueue['student_identifier']) + student_info = {'student_identifier_hash': student_identifier_hash} + contents.update({'student_info': json.dumps(student_info)}) + # Submit request. When successful, 'msg' is the prior length of the queue if is_list_of_files(submission): contents.update({'student_response': ''}) # TODO: Is there any information we want to send here? diff --git a/common/lib/capa/capa/xqueue_interface.py b/common/lib/capa/capa/xqueue_interface.py index 2930eb682d..0dfdabf8d0 100644 --- a/common/lib/capa/capa/xqueue_interface.py +++ b/common/lib/capa/capa/xqueue_interface.py @@ -5,20 +5,17 @@ import hashlib import json import logging import requests -import time log = logging.getLogger('mitx.' + __name__) -def make_hashkey(seed=None): +def make_hashkey(seed): ''' Generate a string key by hashing ''' h = hashlib.md5() - if seed is not None: - h.update(str(seed)) - h.update(str(time.time())) + h.update(str(seed)) return h.hexdigest() diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index da9828fb12..467b147d0f 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -217,7 +217,9 @@ def _get_module(user, request, location, student_module_cache, course_id, positi xqueue = {'interface': xqueue_interface, 'callback_url': xqueue_callback_url, - 'default_queuename': xqueue_default_queuename.replace(' ', '_')} + 'default_queuename': xqueue_default_queuename.replace(' ', '_'), + 'student_identifier': user.id, + } def inner_get_module(location): """ From 27459efcf7d2fcc25c63a42a523a38aa1de397e5 Mon Sep 17 00:00:00 2001 From: kimth Date: Sat, 25 Aug 2012 08:06:17 -0400 Subject: [PATCH 016/181] Update CodeResponse test --- common/lib/xmodule/xmodule/tests/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 2a380bb8be..01462d16ac 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -35,7 +35,7 @@ i4xs = ModuleSystem( user=Mock(), filestore=fs.osfs.OSFS(os.path.dirname(os.path.realpath(__file__))+"/test_files"), debug=True, - xqueue={'interface':None, 'callback_url':'/', 'default_queuename': 'testqueue'}, + xqueue={'interface':None, 'callback_url':'/', 'default_queuename': 'testqueue', 'student_identifier': 0}, node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules") ) From f7c87aa7a1a318dae587276a489d95e0b2f91b11 Mon Sep 17 00:00:00 2001 From: kimth Date: Sat, 25 Aug 2012 09:03:47 -0400 Subject: [PATCH 017/181] Anonymous student id is a property of ModuleSystem --- common/lib/capa/capa/responsetypes.py | 8 ++++---- common/lib/xmodule/xmodule/tests/__init__.py | 5 +++-- common/lib/xmodule/xmodule/x_module.py | 11 +++++++++-- lms/djangoapps/courseware/module_render.py | 10 ++++++++-- 4 files changed, 24 insertions(+), 10 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 032970e324..fb7a69a2f2 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1123,10 +1123,11 @@ class CodeResponse(LoncapaResponse): # Prepare xqueue request #------------------------------------------------------------ qinterface = self.system.xqueue['interface'] + anonymous_student_id = self.system.anonymous_student_id # Generate header queuekey = xqueue_interface.make_hashkey(str(self.system.seed) + str(time.time()) + - str(self.system.xqueue['student_identifier']) + + anonymous_student_id + self.answer_id) xheader = xqueue_interface.make_xheader(lms_callback_url=self.system.xqueue['callback_url'], lms_key=queuekey, @@ -1134,15 +1135,14 @@ class CodeResponse(LoncapaResponse): # Generate body if is_list_of_files(submission): - self.context.update({'submission': queuekey}) # For tracking. TODO: May want to record something else here + self.context.update({'submission': ''}) # TODO: Get S3 pointer from the Queue else: self.context.update({'submission': submission}) contents = self.payload.copy() # Anonymized student identifier to the external grader - student_identifier_hash = xqueue_interface.make_hashkey(self.system.xqueue['student_identifier']) - student_info = {'student_identifier_hash': student_identifier_hash} + student_info = {'anonymous_student_id': anonymous_student_id} contents.update({'student_info': json.dumps(student_info)}) # Submit request. When successful, 'msg' is the prior length of the queue diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 01462d16ac..0d10184501 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -35,8 +35,9 @@ i4xs = ModuleSystem( user=Mock(), filestore=fs.osfs.OSFS(os.path.dirname(os.path.realpath(__file__))+"/test_files"), debug=True, - xqueue={'interface':None, 'callback_url':'/', 'default_queuename': 'testqueue', 'student_identifier': 0}, - node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules") + xqueue={'interface':None, 'callback_url':'/', 'default_queuename': 'testqueue'}, + node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules"), + anonymous_student_id = 'student' ) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index c581911c03..0dc16bd976 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -717,7 +717,8 @@ class ModuleSystem(object): filestore=None, debug=False, xqueue=None, - node_path=""): + node_path="", + anonymous_student_id=''): ''' Create a closure around the system environment. @@ -742,11 +743,16 @@ class ModuleSystem(object): at settings.DATA_DIR. xqueue - Dict containing XqueueInterface object, as well as parameters - for the specific StudentModule + for the specific StudentModule: + xqueue = {'interface': XQueueInterface object, + 'callback_url': Callback into the LMS, + 'queue_name': Target queuename in Xqueue} replace_urls - TEMPORARY - A function like static_replace.replace_urls that capa_module can use to fix up the static urls in ajax results. + + anonymous_student_id - Used for tracking modules with student id ''' self.ajax_url = ajax_url self.xqueue = xqueue @@ -758,6 +764,7 @@ class ModuleSystem(object): self.seed = user.id if user is not None else 0 self.replace_urls = replace_urls self.node_path = node_path + self.anonymous_student_id = anonymous_student_id def get(self, attr): ''' provide uniform access to attributes (like etree).''' diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 467b147d0f..b278d2615b 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -1,3 +1,4 @@ +import hashlib import json import logging import sys @@ -173,6 +174,11 @@ def _get_module(user, request, location, student_module_cache, course_id, positi if not has_access(user, descriptor, 'load'): return None + # Anonymized student identifier + h = hashlib.md5() # TODO: Seed with LMS secret key + h.update(str(user.id)) + anonymous_student_id = h.hexdigest() + #TODO Only check the cache if this module can possibly have state instance_module = None shared_module = None @@ -218,7 +224,6 @@ def _get_module(user, request, location, student_module_cache, course_id, positi xqueue = {'interface': xqueue_interface, 'callback_url': xqueue_callback_url, 'default_queuename': xqueue_default_queuename.replace(' ', '_'), - 'student_identifier': user.id, } def inner_get_module(location): @@ -243,7 +248,8 @@ def _get_module(user, request, location, student_module_cache, course_id, positi # a module is coming through get_html and is therefore covered # by the replace_static_urls code below replace_urls=replace_urls, - node_path=settings.NODE_PATH + node_path=settings.NODE_PATH, + anonymous_student_id=anonymous_student_id ) # pass position specified in URL to module through ModuleSystem system.set('position', position) From 25ea8b29905fde7d1a43e85bb2a0264d4a3ac158 Mon Sep 17 00:00:00 2001 From: kimth Date: Sat, 25 Aug 2012 09:18:10 -0400 Subject: [PATCH 018/181] Use LMS secret key to seed anonymizer --- lms/djangoapps/courseware/module_render.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index b278d2615b..5bf1cc8e97 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -175,7 +175,8 @@ def _get_module(user, request, location, student_module_cache, course_id, positi return None # Anonymized student identifier - h = hashlib.md5() # TODO: Seed with LMS secret key + h = hashlib.md5() + h.update(settings.SECRET_KEY) h.update(str(user.id)) anonymous_student_id = h.hexdigest() From c406be8c6a28db205bb2417dab1dd912f5475385 Mon Sep 17 00:00:00 2001 From: kimth Date: Sun, 26 Aug 2012 13:43:21 +0000 Subject: [PATCH 019/181] Check grader message has proper XML structure --- common/lib/capa/capa/responsetypes.py | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index b2d56b48ca..92c6f62048 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1165,7 +1165,7 @@ class CodeResponse(LoncapaResponse): (valid_score_msg, correct, points, msg) = self._parse_score_msg(score_msg) if not valid_score_msg: - oldcmap.set(self.answer_id, msg='Error: Invalid grader reply.') + oldcmap.set(self.answer_id, msg='Invalid grader reply. Please contact the course staff.') return oldcmap correctness = 'correct' if correct else 'incorrect' @@ -1203,10 +1203,10 @@ class CodeResponse(LoncapaResponse): Returns (valid_score_msg, correct, score, msg): valid_score_msg: Flag indicating valid score_msg format (Boolean) correct: Correctness of submission (Boolean) - score: # TODO: Implement partial grading + score: Points to be assigned (numeric, can be float) msg: Message from grader to display to student (string) ''' - fail = (False, False, -1, '') + fail = (False, False, 0, '') try: score_result = json.loads(score_msg) except (TypeError, ValueError): @@ -1216,7 +1216,19 @@ class CodeResponse(LoncapaResponse): for tag in ['correct', 'score', 'msg']: if not score_result.has_key(tag): return fail - return (True, score_result['correct'], score_result['score'], score_result['msg']) + + # Next, we need to check that the contents of the external grader message + # is safe for the LMS. + # 1) Make sure that the message is valid XML (proper opening/closing tags) + # 2) TODO: Is the message actually HTML? + msg = score_result['msg'] + try: + etree.fromstring(msg) + except etree.XMLSyntaxError as err: + log.error("Unable to parse external grader message as valid XML: score_msg['msg']=%s" % msg) + return fail + + return (True, score_result['correct'], score_result['score'], msg) #----------------------------------------------------------------------------- From 79568dfdf68ced51ec739ddcb7da2ea6a89e10d6 Mon Sep 17 00:00:00 2001 From: kimth Date: Sun, 26 Aug 2012 17:37:01 +0000 Subject: [PATCH 020/181] Add error logging for external grader messages --- common/lib/capa/capa/responsetypes.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 92c6f62048..e7d7406cbd 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1210,11 +1210,14 @@ class CodeResponse(LoncapaResponse): try: score_result = json.loads(score_msg) except (TypeError, ValueError): + log.error("External grader message should be a JSON-serialized dict") return fail if not isinstance(score_result, dict): + log.error("External grader message should be a JSON-serialized dict") return fail for tag in ['correct', 'score', 'msg']: if not score_result.has_key(tag): + log.error("External grader message is missing one or more required tags: 'correct', 'score', 'msg'") return fail # Next, we need to check that the contents of the external grader message From af871d92aa9165668ad920c8b54a8549fb56f652 Mon Sep 17 00:00:00 2001 From: kimth Date: Sun, 26 Aug 2012 21:15:34 +0000 Subject: [PATCH 021/181] Send ext grader the time of submission --- common/lib/capa/capa/responsetypes.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index fb7a69a2f2..580c4f979a 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -27,6 +27,7 @@ import xml.sax.saxutils as saxutils # specific library imports from calc import evaluator, UndefinedVariable from correctmap import CorrectMap +from datetime import datetime from util import * from lxml import etree from lxml.html.soupparser import fromstring as fromstring_bs # uses Beautiful Soup!!! FIXME? @@ -1141,8 +1142,11 @@ class CodeResponse(LoncapaResponse): contents = self.payload.copy() - # Anonymized student identifier to the external grader - student_info = {'anonymous_student_id': anonymous_student_id} + # Metadata related to the student submission revealed to the external grader + current_time = datetime.now() + student_info = {'anonymous_student_id': anonymous_student_id, + 'submission_time': str(current_time), + } contents.update({'student_info': json.dumps(student_info)}) # Submit request. When successful, 'msg' is the prior length of the queue From 9102c7609cc03a499dae45aa43ddc6e25623d96e Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 27 Aug 2012 10:20:16 -0400 Subject: [PATCH 022/181] Fix latent bug in access checks in get_module * All access checks now done for the "user" param, ignoring request.user - This matters for xqueue callbacks and for instructor view of student progress * An effect of this change is that if a student couldn't see their own 'progress' tab, the course instructor won't be able to see it either --- lms/djangoapps/courseware/module_render.py | 10 ++++------ lms/djangoapps/courseware/tests/tests.py | 13 +++++++++++-- lms/djangoapps/courseware/views.py | 4 ++++ 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 8a96d2533f..6c3db9bc18 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -144,8 +144,8 @@ def get_module(user, request, location, student_module_cache, course_id, positio Arguments: - user : User for whom we're getting the module - - request : current django HTTPrequest -- used in particular for auth - (This is important e.g. for prof impersonation of students in progress view) + - request : current django HTTPrequest. Note: request.user isn't used for anything--all auth + and such works based on user. - location : A Location-like object identifying the module to load - student_module_cache : a StudentModuleCache - course_id : the course_id in the context of which to load module @@ -171,12 +171,10 @@ def _get_module(user, request, location, student_module_cache, course_id, positi descriptor = modulestore().get_instance(course_id, location) # Short circuit--if the user shouldn't have access, bail without doing any work - # NOTE: Do access check on request.user -- that's who actually needs access (e.g. could be prof - # impersonating a user) - if not has_access(request.user, descriptor, 'load'): + if not has_access(user, descriptor, 'load'): return None - #TODO Only check the cache if this module can possibly have state + # Only check the cache if this module can possibly have state instance_module = None shared_module = None if user.is_authenticated(): diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index bb48dce609..88e0f53f70 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -411,8 +411,6 @@ class TestViewAuth(PageLoader): """list of urls that only instructors/staff should be able to see""" urls = reverse_urls(['instructor_dashboard','gradebook','grade_summary'], course) - urls.append(reverse('student_progress', kwargs={'course_id': course.id, - 'student_id': user(self.student).id})) return urls def check_non_staff(course): @@ -435,6 +433,17 @@ class TestViewAuth(PageLoader): print 'checking for 200 on {0}'.format(url) self.check_for_get_code(200, url) + # The student progress tab is not accessible to a student + # before launch, so the instructor view-as-student feature should return a 404 as well. + # TODO (vshnayder): If this is not the behavior we want, will need + # to make access checking smarter and understand both the effective + # user (the student), and the requesting user (the prof) + url = reverse('student_progress', kwargs={'course_id': course.id, + 'student_id': user(self.student).id}) + print 'checking for 404 on view-as-student: {0}'.format(url) + self.check_for_get_code(404, url) + + # First, try with an enrolled student print '=== Testing student access....' self.login(self.student, self.password) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 50b7a2d645..b532688037 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -333,6 +333,10 @@ def progress(request, course_id, student_id=None): course_module = get_module(student, request, course.location, student_module_cache, course_id) + # The course_module should be accessible, but check anyway just in case something went wrong: + if course_module is None: + raise Http404("No access to this course") + courseware_summary = grades.progress_summary(student, course_module, course.grader, student_module_cache) grade_summary = grades.grade(student, request, course, student_module_cache) From 7e2e90cfa58957dcf1cf41ad96d82310f1935dff Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 27 Aug 2012 11:29:13 -0400 Subject: [PATCH 023/181] move comment into function re: #549 --- common/lib/xmodule/xmodule/modulestore/xml.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 98260f5982..92eca8f5e6 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -82,11 +82,11 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): def fallback_name(): """Return the fallback name for this module. This is a function instead of a variable because we want it to be lazy.""" + # use the hash of the content--the first 12 bytes should be plenty. return tag + "_" + hashlib.sha1(xml).hexdigest()[:12] # Fallback if there was nothing we could use: if url_name is None or url_name == "": - # use the hash of the content--the first 12 bytes should be plenty. url_name = fallback_name() # Don't log a warning--we don't need this in the log. Do # put it in the error tracker--content folks need to see it. From f81f94ec674305d03ed877e34d1aa024ef922205 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 22 Aug 2012 09:59:01 -0400 Subject: [PATCH 024/181] Allow location url_names to contain ':', mapping to / on file load * New format: {tag}://{org}/{course}/{category}/{name}[@{revision}] * Updated tests, code * Added test chapter with : in url_name in toy course * added test html page with : in url_name * added a note to docs --- cms/djangoapps/github_sync/tests/__init__.py | 2 +- common/lib/xmodule/xmodule/html_module.py | 30 ++++++++++++---- .../xmodule/xmodule/modulestore/__init__.py | 36 +++++++++++++------ .../modulestore/tests/test_location.py | 32 +++++++++++------ .../lib/xmodule/xmodule/tests/test_import.py | 34 ++++++++++++++++++ common/lib/xmodule/xmodule/xml_module.py | 16 ++++++--- common/test/data/toy/chapter/secret/magic.xml | 3 ++ common/test/data/toy/course/2012_Fall.xml | 3 +- .../data/toy/html/{ => secret}/toylab.html | 0 .../data/toy/html/{ => secret}/toylab.xml | 0 common/test/data/toy/policies/2012_Fall.json | 2 +- 11 files changed, 123 insertions(+), 35 deletions(-) create mode 100644 common/test/data/toy/chapter/secret/magic.xml rename common/test/data/toy/html/{ => secret}/toylab.html (100%) rename common/test/data/toy/html/{ => secret}/toylab.xml (100%) diff --git a/cms/djangoapps/github_sync/tests/__init__.py b/cms/djangoapps/github_sync/tests/__init__.py index 581ac3cb25..e2b9a909a7 100644 --- a/cms/djangoapps/github_sync/tests/__init__.py +++ b/cms/djangoapps/github_sync/tests/__init__.py @@ -61,7 +61,7 @@ class GithubSyncTestCase(TestCase): self.assertIn( Location('i4x://edX/toy/chapter/Overview'), [child.location for child in self.import_course.get_children()]) - self.assertEquals(1, len(self.import_course.get_children())) + self.assertEquals(2, len(self.import_course.get_children())) @patch('github_sync.sync_with_github') def test_sync_all_with_github(self, sync_with_github): diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index 6c424c26f2..3f834c335c 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -4,9 +4,10 @@ import logging import os import sys from lxml import etree +from path import path from .x_module import XModule -from .xml_module import XmlDescriptor +from .xml_module import XmlDescriptor, name_to_pathname from .editing_module import EditingDescriptor from .stringify import stringify_children from .html_checker import check_html @@ -75,9 +76,19 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor): cls.clean_metadata_from_xml(definition_xml) return {'data': stringify_children(definition_xml)} else: - # html is special. cls.filename_extension is 'xml', but if 'filename' is in the definition, - # that means to load from .html - filepath = "{category}/{name}.html".format(category='html', name=filename) + # html is special. cls.filename_extension is 'xml', but + # if 'filename' is in the definition, that means to load + # from .html + # 'filename' in html pointers is a relative path + # (not same as 'html/blah.html' when the pointer is in a directory itself) + pointer_path = "{category}/{url_path}".format(category='html', + url_path=name_to_pathname(location.name)) + base = path(pointer_path).dirname() + #log.debug("base = {0}, base.dirname={1}, filename={2}".format(base, base.dirname(), filename)) + filepath = "{base}/{name}.html".format(base=base, name=filename) + #log.debug("looking for html file for {0} at {1}".format(location, filepath)) + + # VS[compat] # TODO (cpennington): If the file doesn't exist at the right path, @@ -128,13 +139,18 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor): pass # Not proper format. Write html to file, return an empty tag - filepath = u'{category}/{name}.html'.format(category=self.category, - name=self.url_name) + pathname = name_to_pathname(self.url_name) + pathdir = path(pathname).dirname() + filepath = u'{category}/{pathname}.html'.format(category=self.category, + pathname=pathname) resource_fs.makedir(os.path.dirname(filepath), allow_recreate=True) with resource_fs.open(filepath, 'w') as file: file.write(self.definition['data']) + # write out the relative name + relname = path(pathname).basename() + elt = etree.Element('html') - elt.set("filename", self.url_name) + elt.set("filename", relname) return elt diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index d6dd85deea..c3bbc1e508 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -13,18 +13,21 @@ from xmodule.errortracker import ErrorLog, make_error_tracker log = logging.getLogger('mitx.' + 'modulestore') + URL_RE = re.compile(""" (?P[^:]+):// (?P[^/]+)/ (?P[^/]+)/ (?P[^/]+)/ - (?P[^/]+) - (/(?P[^/]+))? + (?P[^@]+) + (@(?P[^/]+))? """, re.VERBOSE) # TODO (cpennington): We should decide whether we want to expand the # list of valid characters in a location INVALID_CHARS = re.compile(r"[^\w.-]") +# Names are allowed to have colons. +INVALID_CHARS_NAME = re.compile(r"[^\w.:-]") _LocationBase = namedtuple('LocationBase', 'tag org course category name revision') @@ -34,7 +37,7 @@ class Location(_LocationBase): Encodes a location. Locations representations of URLs of the - form {tag}://{org}/{course}/{category}/{name}[/{revision}] + form {tag}://{org}/{course}/{category}/{name}[@{revision}] However, they can also be represented a dictionaries (specifying each component), tuples or list (specified in order), or as strings of the url @@ -81,7 +84,7 @@ class Location(_LocationBase): location - Can be any of the following types: string: should be of the form - {tag}://{org}/{course}/{category}/{name}[/{revision}] + {tag}://{org}/{course}/{category}/{name}[@{revision}] list: should be of the form [tag, org, course, category, name, revision] @@ -99,10 +102,11 @@ class Location(_LocationBase): ommitted. Components must be composed of alphanumeric characters, or the - characters '_', '-', and '.' + characters '_', '-', and '.'. The name component is additionally allowed to have ':', + which is interpreted specially for xml storage. - Components may be set to None, which may be interpreted by some contexts - to mean wildcard selection + Components may be set to None, which may be interpreted in some contexts + to mean wildcard selection. """ @@ -116,14 +120,23 @@ class Location(_LocationBase): return _LocationBase.__new__(_cls, *([None] * 6)) def check_dict(dict_): - check_list(dict_.itervalues()) + # Order matters, so flatten out into a list + keys = ['tag', 'org', 'course', 'category', 'name', 'revision'] + list_ = [dict_[k] for k in keys] + check_list(list_) def check_list(list_): - for val in list_: - if val is not None and INVALID_CHARS.search(val) is not None: + def check(val, regexp): + if val is not None and regexp.search(val) is not None: log.debug('invalid characters val="%s", list_="%s"' % (val, list_)) raise InvalidLocationError(location) + list_ = list(list_) + for val in list_[:4] + [list_[5]]: + check(val, INVALID_CHARS) + # names allow colons + check(list_[4], INVALID_CHARS_NAME) + if isinstance(location, basestring): match = URL_RE.match(location) if match is None: @@ -162,7 +175,7 @@ class Location(_LocationBase): """ url = "{tag}://{org}/{course}/{category}/{name}".format(**self.dict()) if self.revision: - url += "/" + self.revision + url += "@" + self.revision return url def html_id(self): @@ -170,6 +183,7 @@ class Location(_LocationBase): Return a string with a version of the location that is safe for use in html id attributes """ + # TODO: is ':' ok in html ids? return "-".join(str(v) for v in self.list() if v is not None).replace('.', '_') diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_location.py b/common/lib/xmodule/xmodule/modulestore/tests/test_location.py index 70c6351685..529b1f88eb 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_location.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_location.py @@ -10,7 +10,7 @@ def check_string_roundtrip(url): def test_string_roundtrip(): check_string_roundtrip("tag://org/course/category/name") - check_string_roundtrip("tag://org/course/category/name/revision") + check_string_roundtrip("tag://org/course/category/name@revision") input_dict = { @@ -21,18 +21,28 @@ input_dict = { 'org': 'org' } + +also_valid_dict = { + 'tag': 'tag', + 'course': 'course', + 'category': 'category', + 'name': 'name:more_name', + 'org': 'org' +} + + input_list = ['tag', 'org', 'course', 'category', 'name'] input_str = "tag://org/course/category/name" -input_str_rev = "tag://org/course/category/name/revision" +input_str_rev = "tag://org/course/category/name@revision" -valid = (input_list, input_dict, input_str, input_str_rev) +valid = (input_list, input_dict, input_str, input_str_rev, also_valid_dict) invalid_dict = { 'tag': 'tag', 'course': 'course', 'category': 'category', - 'name': 'name/more_name', + 'name': 'name@more_name', 'org': 'org' } @@ -45,8 +55,9 @@ invalid_dict2 = { } invalid = ("foo", ["foo"], ["foo", "bar"], - ["foo", "bar", "baz", "blat", "foo/bar"], - "tag://org/course/category/name with spaces/revision", + ["foo", "bar", "baz", "blat:blat", "foo:bar"], # ':' ok in name, not in category + "tag://org/course/category/name with spaces@revision", + "tag://org/course/category/name/with/slashes@revision", invalid_dict, invalid_dict2) @@ -62,16 +73,15 @@ def test_dict(): assert_equals(dict(revision=None, **input_dict), Location(input_dict).dict()) input_dict['revision'] = 'revision' - assert_equals("tag://org/course/category/name/revision", Location(input_dict).url()) + assert_equals("tag://org/course/category/name@revision", Location(input_dict).url()) assert_equals(input_dict, Location(input_dict).dict()) - def test_list(): assert_equals("tag://org/course/category/name", Location(input_list).url()) assert_equals(input_list + [None], Location(input_list).list()) input_list.append('revision') - assert_equals("tag://org/course/category/name/revision", Location(input_list).url()) + assert_equals("tag://org/course/category/name@revision", Location(input_list).url()) assert_equals(input_list, Location(input_list).list()) @@ -87,8 +97,10 @@ def test_none(): def test_invalid_locations(): assert_raises(InvalidLocationError, Location, "foo") assert_raises(InvalidLocationError, Location, ["foo", "bar"]) + assert_raises(InvalidLocationError, Location, ["foo", "bar", "baz", "blat/blat", "foo"]) assert_raises(InvalidLocationError, Location, ["foo", "bar", "baz", "blat", "foo/bar"]) - assert_raises(InvalidLocationError, Location, "tag://org/course/category/name with spaces/revision") + assert_raises(InvalidLocationError, Location, "tag://org/course/category/name with spaces@revision") + assert_raises(InvalidLocationError, Location, "tag://org/course/category/name/revision") def test_equality(): diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index 3454366c1a..a369850209 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -255,3 +255,37 @@ class ImportTestCase(unittest.TestCase): two_toy_video = modulestore.get_instance(two_toy_id, location) self.assertEqual(toy_video.metadata['youtube'], "1.0:p2Q6BrNhdh8") self.assertEqual(two_toy_video.metadata['youtube'], "1.0:p2Q6BrNhdh9") + + + def test_colon_in_url_name(self): + """Ensure that colons in url_names convert to file paths properly""" + + print "Starting import" + modulestore = XMLModuleStore(DATA_DIR, eager=True, course_dirs=['toy']) + + courses = modulestore.get_courses() + self.assertEquals(len(courses), 1) + course = courses[0] + course_id = course.id + + print "course errors:" + for (msg, err) in modulestore.get_item_errors(course.location): + print msg + print err + + chapters = course.get_children() + self.assertEquals(len(chapters), 2) + + ch2 = chapters[1] + self.assertEquals(ch2.url_name, "secret:magic") + + print "Ch2 location: ", ch2.location + + also_ch2 = modulestore.get_instance(course_id, ch2.location) + self.assertEquals(ch2, also_ch2) + + print "making sure html loaded" + cloc = course.location + loc = Location(cloc.tag, cloc.org, cloc.course, 'html', 'secret:toylab') + html = modulestore.get_instance(course_id, loc) + self.assertEquals(html.display_name, "Toy lab") diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index b6f791ffc6..25dc4e0c6e 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -12,6 +12,12 @@ import sys log = logging.getLogger(__name__) +def name_to_pathname(name): + """ + Convert a location name for use in a path: replace ':' with '/'. + This allows users of the xml format to organize content into directories + """ + return name.replace(':', '/') def is_pointer_tag(xml_obj): """ @@ -245,8 +251,8 @@ class XmlDescriptor(XModuleDescriptor): # VS[compat] -- detect new-style each-in-a-file mode if is_pointer_tag(xml_object): # new style: - # read the actual definition file--named using url_name - filepath = cls._format_filepath(xml_object.tag, url_name) + # read the actual definition file--named using url_name.replace(':','/') + filepath = cls._format_filepath(xml_object.tag, name_to_pathname(url_name)) definition_xml = cls.load_file(filepath, system.resources_fs, location) else: definition_xml = xml_object # this is just a pointer, not the real definition content @@ -292,7 +298,8 @@ class XmlDescriptor(XModuleDescriptor): """If this returns True, write the definition of this descriptor to a separate file. - NOTE: Do not override this without a good reason. It is here specifically for customtag... + NOTE: Do not override this without a good reason. It is here + specifically for customtag... """ return True @@ -335,7 +342,8 @@ class XmlDescriptor(XModuleDescriptor): if self.export_to_file(): # Write the definition to a file - filepath = self.__class__._format_filepath(self.category, self.url_name) + url_path = name_to_pathname(self.url_name) + filepath = self.__class__._format_filepath(self.category, url_path) resource_fs.makedir(os.path.dirname(filepath), allow_recreate=True) with resource_fs.open(filepath, 'w') as file: file.write(etree.tostring(xml_object, pretty_print=True)) diff --git a/common/test/data/toy/chapter/secret/magic.xml b/common/test/data/toy/chapter/secret/magic.xml new file mode 100644 index 0000000000..dd16380a70 --- /dev/null +++ b/common/test/data/toy/chapter/secret/magic.xml @@ -0,0 +1,3 @@ + + diff --git a/common/test/data/toy/course/2012_Fall.xml b/common/test/data/toy/course/2012_Fall.xml index d34eb9d56a..c87fccd9ab 100644 --- a/common/test/data/toy/course/2012_Fall.xml +++ b/common/test/data/toy/course/2012_Fall.xml @@ -1,9 +1,10 @@ - + + diff --git a/common/test/data/toy/html/toylab.html b/common/test/data/toy/html/secret/toylab.html similarity index 100% rename from common/test/data/toy/html/toylab.html rename to common/test/data/toy/html/secret/toylab.html diff --git a/common/test/data/toy/html/toylab.xml b/common/test/data/toy/html/secret/toylab.xml similarity index 100% rename from common/test/data/toy/html/toylab.xml rename to common/test/data/toy/html/secret/toylab.xml diff --git a/common/test/data/toy/policies/2012_Fall.json b/common/test/data/toy/policies/2012_Fall.json index 6c501d66f8..5ea437a8e4 100644 --- a/common/test/data/toy/policies/2012_Fall.json +++ b/common/test/data/toy/policies/2012_Fall.json @@ -11,7 +11,7 @@ "display_name": "Toy Videos", "format": "Lecture Sequence" }, - "html/toylab": { + "html/secret:toylab": { "display_name": "Toy lab" }, "video/Video_Resources": { From 84732d03b6d4856f40297a42610a4a22ea63d444 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 27 Aug 2012 11:43:47 -0400 Subject: [PATCH 025/181] add note about ":" in format docs --- doc/xml-format.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/doc/xml-format.md b/doc/xml-format.md index 55bcda4480..256d4839bf 100644 --- a/doc/xml-format.md +++ b/doc/xml-format.md @@ -189,7 +189,13 @@ This video has been encoded at 4 different speeds: 0.75x, 1x, 1.25x, and 1.5x. ## More on `url_name`s -Every content element (within a course) should have a unique id. This id is formed as `{category}/{url_name}`, or automatically generated from the content if `url_name` is not specified. Categories are the different tag types ('chapter', 'problem', 'html', 'sequential', etc). Url_name is a string containing a-z, A-Z, dot (.) and _. This is what appears in urls that point to this object. +Every content element (within a course) should have a unique id. This id is formed as `{category}/{url_name}`, or automatically generated from the content if `url_name` is not specified. Categories are the different tag types ('chapter', 'problem', 'html', 'sequential', etc). Url_name is a string containing a-z, A-Z, dot (.), underscore (_), and ':'. This is what appears in urls that point to this object. + +Colon (':') is special--when looking for the content definition in an xml, ':' will be replaced with '/'. This allows organizing content into folders. For example, given the pointer tag + + + +we would look for the problem definition in `problem/conceptual/add_apples_and_oranges.xml`. (There is a technical reason why we can't just allow '/' in the url_name directly.) __IMPORTANT__: A student's state for a particular content element is tied to the element id, so the automatic id generation if only ok for elements that do not need to store any student state (e.g. verticals or customtags). For problems, sequentials, and videos, and any other element where we keep track of what the student has done and where they are at, you should specify a unique `url_name`. Of course, any content element that is split out into a file will need a `url_name` to specify where to find the definition. When the CMS comes online, it will use these ids to enable content reuse, so if there is a logical name for something, please do specify it. From 7b3098edc0eb8de1856c51c4a72e105b22fb9ab0 Mon Sep 17 00:00:00 2001 From: Tom Giannattasio Date: Mon, 27 Aug 2012 11:48:54 -0400 Subject: [PATCH 026/181] fixed cheatsheet bug on new article page --- lms/templates/wiki/base.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/templates/wiki/base.html b/lms/templates/wiki/base.html index 5c7381ac05..1c1bbc4a63 100644 --- a/lms/templates/wiki/base.html +++ b/lms/templates/wiki/base.html @@ -43,7 +43,7 @@ $(document).ready(function () { editor = $('#div_id_content div.controls'); cs = editor.prepend('

Markdown syntax is allowed. See the cheatsheet for help.

'); - cs.click(function() { + cs.find('#cheatsheetLink').click(function() { $('#cheatsheetModal').modal('show'); }); $('#cheatsheetModal .close-btn').click(function(e) { From b50f9759e1d147a799ce967f60f192092830ff5b Mon Sep 17 00:00:00 2001 From: kimth Date: Mon, 27 Aug 2012 14:08:57 -0400 Subject: [PATCH 027/181] Catch queuekey --> queuestate KeyError in conversion --- common/lib/xmodule/xmodule/capa_module.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 33a2f6b4c4..f13ab41c53 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -464,7 +464,13 @@ class CapaModule(XModule): raise NotFoundError('Problem must be reset before it can be checked again') # Problem queued. Students must wait XQUEUE_WAITTIME_BETWEEN_REQUESTS - if self.lcp.is_queued(): + try: + is_queued = self.lcp.is_queued() + except KeyError: + log.info("Caught KeyError arising from 'queuekey'-->'queuestate' conversion for CapaModule name=%s" % self.name) + is_queued = False + + if is_queued: current_time = datetime.datetime.now() prev_submit_time = self.lcp.get_recentmost_queuetime() waittime_between_requests = settings.XQUEUE_WAITTIME_BETWEEN_REQUESTS From ff38f5b390d26140f6b453450cc3d0b94480e22a Mon Sep 17 00:00:00 2001 From: kimth Date: Mon, 27 Aug 2012 14:50:03 -0400 Subject: [PATCH 028/181] Debugging correctmap state --- common/lib/capa/capa/capa_problem.py | 3 +++ common/lib/xmodule/xmodule/capa_module.py | 8 +------- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index ee24ebd031..3d9ee27ca0 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -14,6 +14,7 @@ This is used by capa_module. from __future__ import division +import json import logging import math import numpy @@ -97,6 +98,8 @@ class LoncapaProblem(object): if 'student_answers' in state: self.student_answers = state['student_answers'] if 'correct_map' in state: + print 'THK: LoncapaProblem.__init__' + print json.dumps(state['correct_map'], indent=4) self.correct_map.set_dict(state['correct_map']) if 'done' in state: self.done = state['done'] diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index f13ab41c53..33a2f6b4c4 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -464,13 +464,7 @@ class CapaModule(XModule): raise NotFoundError('Problem must be reset before it can be checked again') # Problem queued. Students must wait XQUEUE_WAITTIME_BETWEEN_REQUESTS - try: - is_queued = self.lcp.is_queued() - except KeyError: - log.info("Caught KeyError arising from 'queuekey'-->'queuestate' conversion for CapaModule name=%s" % self.name) - is_queued = False - - if is_queued: + if self.lcp.is_queued(): current_time = datetime.datetime.now() prev_submit_time = self.lcp.get_recentmost_queuetime() waittime_between_requests = settings.XQUEUE_WAITTIME_BETWEEN_REQUESTS From b53327a6a45d46aacf9c56714cf66d3a2c277c4e Mon Sep 17 00:00:00 2001 From: kimth Date: Mon, 27 Aug 2012 15:14:12 -0400 Subject: [PATCH 029/181] Correctmap state debugging continued... --- common/lib/capa/capa/capa_problem.py | 1 + common/lib/capa/capa/correctmap.py | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index 3d9ee27ca0..8edc65b9f9 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -101,6 +101,7 @@ class LoncapaProblem(object): print 'THK: LoncapaProblem.__init__' print json.dumps(state['correct_map'], indent=4) self.correct_map.set_dict(state['correct_map']) + print json.dumps(self.correct_map.get_dict(), indent=4) if 'done' in state: self.done = state['done'] diff --git a/common/lib/capa/capa/correctmap.py b/common/lib/capa/capa/correctmap.py index 4659e2d1e2..ed66643803 100644 --- a/common/lib/capa/capa/correctmap.py +++ b/common/lib/capa/capa/correctmap.py @@ -58,10 +58,11 @@ class CorrectMap(object): dict of dicts format. ''' if correct_map and not (type(correct_map[correct_map.keys()[0]]) == dict): - self.__init__() # empty current dict - for k in correct_map: self.set(k, correct_map[k]) # create new dict entries + self.__init__() # empty current dict + for k in correct_map: self.set(k, correct_map[k]) # create new dict entries else: - self.cmap = correct_map + self.__init__() + for k in correct_map: self.set(k, **correct_map[k]) def is_correct(self, answer_id): if answer_id in self.cmap: return self.cmap[answer_id]['correctness'] == 'correct' From 773a32e39f825953acea10c36deecd6177128f40 Mon Sep 17 00:00:00 2001 From: Ibrahim Awwal Date: Mon, 27 Aug 2012 12:59:07 -0700 Subject: [PATCH 030/181] Fix bug with sorting discussion forums (event was not being passed into the event handler). --- lms/static/coffee/src/discussion/discussion.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/static/coffee/src/discussion/discussion.coffee b/lms/static/coffee/src/discussion/discussion.coffee index d8df704d1c..847f5fb97f 100644 --- a/lms/static/coffee/src/discussion/discussion.coffee +++ b/lms/static/coffee/src/discussion/discussion.coffee @@ -169,7 +169,7 @@ if Backbone? url = URI($elem.attr("action")).addSearch({text: @$(".search-input").val()}) @reload($elem, url) - sort: -> + sort: (event) -> $elem = $(event.target) url = $elem.attr("sort-url") @reload($elem, url) From 1e72e1c91ec68e4d689895cb12a86732e8356bc2 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 27 Aug 2012 16:24:43 -0400 Subject: [PATCH 031/181] change 404 message --- lms/djangoapps/courseware/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index b532688037..92f6716320 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -335,7 +335,7 @@ def progress(request, course_id, student_id=None): # The course_module should be accessible, but check anyway just in case something went wrong: if course_module is None: - raise Http404("No access to this course") + raise Http404("Course does not exist") courseware_summary = grades.progress_summary(student, course_module, course.grader, student_module_cache) From 01d0697165ddeb2b3032239077c20be56497bd66 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 27 Aug 2012 16:26:35 -0400 Subject: [PATCH 032/181] Eagerly load modulestores on server startup --- common/lib/xmodule/xmodule/modulestore/django.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/common/lib/xmodule/xmodule/modulestore/django.py b/common/lib/xmodule/xmodule/modulestore/django.py index 6a7315a074..59668b4a5a 100644 --- a/common/lib/xmodule/xmodule/modulestore/django.py +++ b/common/lib/xmodule/xmodule/modulestore/django.py @@ -43,3 +43,7 @@ def modulestore(name='default'): ) return _MODULESTORES[name] + +# Initialize the modulestores immediately +for store_name in settings.MODULESTORE: + modulestore(store_name) From b45fb750b2feaff0a214228f1fa4a43a866ba817 Mon Sep 17 00:00:00 2001 From: kimth Date: Mon, 27 Aug 2012 17:12:20 -0400 Subject: [PATCH 033/181] CorrectMap allows unmatched keywords --- common/lib/capa/capa/correctmap.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/capa/capa/correctmap.py b/common/lib/capa/capa/correctmap.py index ed66643803..71f97e1321 100644 --- a/common/lib/capa/capa/correctmap.py +++ b/common/lib/capa/capa/correctmap.py @@ -32,7 +32,7 @@ class CorrectMap(object): def __iter__(self): return self.cmap.__iter__() - def set(self, answer_id=None, correctness=None, npoints=None, msg='', hint='', hintmode=None, queuestate=None): + def set(self, answer_id=None, correctness=None, npoints=None, msg='', hint='', hintmode=None, queuestate=None, **kwargs): if answer_id is not None: self.cmap[answer_id] = {'correctness': correctness, 'npoints': npoints, From d5240626528547e112c78af633c1f4494a5c6d91 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Mon, 27 Aug 2012 17:47:37 -0400 Subject: [PATCH 034/181] Put quick check so we don't load course modules on init unless we're actually running in Django --- common/lib/xmodule/xmodule/modulestore/django.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/django.py b/common/lib/xmodule/xmodule/modulestore/django.py index 59668b4a5a..d46422cbf1 100644 --- a/common/lib/xmodule/xmodule/modulestore/django.py +++ b/common/lib/xmodule/xmodule/modulestore/django.py @@ -5,8 +5,8 @@ Passes settings.MODULESTORE as kwargs to MongoModuleStore """ from __future__ import absolute_import - from importlib import import_module +from os import environ from django.conf import settings @@ -44,6 +44,7 @@ def modulestore(name='default'): return _MODULESTORES[name] -# Initialize the modulestores immediately -for store_name in settings.MODULESTORE: - modulestore(store_name) +if 'DJANGO_SETTINGS_MODULE' in environ: + # Initialize the modulestores immediately + for store_name in settings.MODULESTORE: + modulestore(store_name) From 8b0d1f09e0227fe4c4e0beed0194ae496976dc67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Andr=C3=A9s=20Rocha?= Date: Mon, 27 Aug 2012 18:30:38 -0400 Subject: [PATCH 035/181] Refactored wiki cheatsheet code. --- lms/djangoapps/course_wiki/editors.py | 14 ++++++++++++-- lms/static/js/wiki/cheatsheet.js | 9 +++++++++ lms/static/sass/course/wiki/_wiki.scss | 19 ++++++++++++------- lms/templates/wiki/base.html | 14 -------------- lms/templates/wiki/create.html | 1 + lms/templates/wiki/edit.html | 3 +++ lms/templates/wiki/includes/cheatsheet.html | 4 ++-- .../wiki/includes/editor_widget.html | 4 ++++ 8 files changed, 43 insertions(+), 25 deletions(-) create mode 100644 lms/static/js/wiki/cheatsheet.js create mode 100644 lms/templates/wiki/includes/editor_widget.html diff --git a/lms/djangoapps/course_wiki/editors.py b/lms/djangoapps/course_wiki/editors.py index a6583a8d98..c1f5b733ad 100644 --- a/lms/djangoapps/course_wiki/editors.py +++ b/lms/djangoapps/course_wiki/editors.py @@ -4,9 +4,12 @@ from django.utils.encoding import force_unicode from django.utils.html import conditional_escape from django.utils.safestring import mark_safe +from django.template.loader import render_to_string + from wiki.editors.base import BaseEditor from wiki.editors.markitup import MarkItUpAdminWidget + class CodeMirrorWidget(forms.Widget): def __init__(self, attrs=None): # The 'rows' and 'cols' attributes are required for HTML correctness. @@ -18,9 +21,15 @@ class CodeMirrorWidget(forms.Widget): def render(self, name, value, attrs=None): if value is None: value = '' + final_attrs = self.build_attrs(attrs, name=name) - return mark_safe(u'
%s
' % (flatatt(final_attrs), - conditional_escape(force_unicode(value)))) + + # TODO use the help_text field of edit form instead of rendering a template + + return render_to_string('wiki/includes/editor_widget.html', + {'attrs': mark_safe(flatatt(final_attrs)), + 'content': conditional_escape(force_unicode(value)), + }) class CodeMirror(BaseEditor): @@ -50,5 +59,6 @@ class CodeMirror(BaseEditor): "js/vendor/CodeMirror/xml.js", "js/vendor/CodeMirror/mitx_markdown.js", "js/wiki/CodeMirror.init.js", + "js/wiki/cheatsheet.js", ) diff --git a/lms/static/js/wiki/cheatsheet.js b/lms/static/js/wiki/cheatsheet.js new file mode 100644 index 0000000000..e6c8e3cdaf --- /dev/null +++ b/lms/static/js/wiki/cheatsheet.js @@ -0,0 +1,9 @@ +$(document).ready(function () { + $('#cheatsheetLink').click(function() { + $('#cheatsheetModal').modal('show'); + }); + + $('#cheatsheetModal .close-btn').click(function(e) { + $('#cheatsheetModal').modal('hide'); + }); +}); \ No newline at end of file diff --git a/lms/static/sass/course/wiki/_wiki.scss b/lms/static/sass/course/wiki/_wiki.scss index a58b8fa06c..1bc38abd9a 100644 --- a/lms/static/sass/course/wiki/_wiki.scss +++ b/lms/static/sass/course/wiki/_wiki.scss @@ -391,6 +391,18 @@ section.wiki { line-height: 1.4em; } + #div_id_content { + position: relative; + } + + #hint_id_content { + position: absolute; + top: 10px; + right: 0%; + font-size: 12px; + text-align:right; + } + .CodeMirror { background: #fafafa; border: 1px solid #c8c8c8; @@ -567,13 +579,6 @@ section.wiki { background: #f00 !important; } - .cheatsheet { - float: right; - position: relative; - top: -26px; - font-size: 12px; - } - #cheatsheetLink { text-align: right; display: float; diff --git a/lms/templates/wiki/base.html b/lms/templates/wiki/base.html index 1c1bbc4a63..8a9686f546 100644 --- a/lms/templates/wiki/base.html +++ b/lms/templates/wiki/base.html @@ -39,18 +39,6 @@ {% with mathjax_mode='wiki' %} {% include "mathjax_include.html" %} {% endwith %} - {% endaddtoblock %} {% endblock %} @@ -81,8 +69,6 @@ {% endblock %}
- {% include "wiki/includes/cheatsheet.html" %} - {% endblock %} diff --git a/lms/templates/wiki/create.html b/lms/templates/wiki/create.html index 886764ba84..745be08cf8 100644 --- a/lms/templates/wiki/create.html +++ b/lms/templates/wiki/create.html @@ -42,6 +42,7 @@ {% trans "Go back" %} + {% include "wiki/includes/cheatsheet.html" %} diff --git a/lms/templates/wiki/edit.html b/lms/templates/wiki/edit.html index f4bd7d138f..65378da4e4 100644 --- a/lms/templates/wiki/edit.html +++ b/lms/templates/wiki/edit.html @@ -40,7 +40,10 @@ + {% include "wiki/includes/cheatsheet.html" %} {% endblock %} + + diff --git a/lms/templates/wiki/includes/cheatsheet.html b/lms/templates/wiki/includes/cheatsheet.html index 429c1ea22e..6f5170e601 100644 --- a/lms/templates/wiki/includes/cheatsheet.html +++ b/lms/templates/wiki/includes/cheatsheet.html @@ -1,4 +1,4 @@ - + diff --git a/lms/templates/wiki/includes/editor_widget.html b/lms/templates/wiki/includes/editor_widget.html new file mode 100644 index 0000000000..2f5dd7ce68 --- /dev/null +++ b/lms/templates/wiki/includes/editor_widget.html @@ -0,0 +1,4 @@ + +

+ Markdown syntax is allowed. See the cheatsheet for help. +

From 57c6bfc3a6239e91caaf2237b98deb862127e27e Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Mon, 27 Aug 2012 18:46:13 -0400 Subject: [PATCH 036/181] commenting out the pre-loading of modules until we get a better way in place (that doesn't hang on deploy) --- common/lib/xmodule/xmodule/modulestore/django.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/django.py b/common/lib/xmodule/xmodule/modulestore/django.py index d46422cbf1..0b86c2fea4 100644 --- a/common/lib/xmodule/xmodule/modulestore/django.py +++ b/common/lib/xmodule/xmodule/modulestore/django.py @@ -44,7 +44,7 @@ def modulestore(name='default'): return _MODULESTORES[name] -if 'DJANGO_SETTINGS_MODULE' in environ: - # Initialize the modulestores immediately - for store_name in settings.MODULESTORE: - modulestore(store_name) +# if 'DJANGO_SETTINGS_MODULE' in environ: +# # Initialize the modulestores immediately +# for store_name in settings.MODULESTORE: +# modulestore(store_name) From c462b1a917292f400db7a2cf126622e22a04fce2 Mon Sep 17 00:00:00 2001 From: kimth Date: Mon, 27 Aug 2012 18:50:31 -0400 Subject: [PATCH 037/181] Document CorrectMap.set --- common/lib/capa/capa/correctmap.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/common/lib/capa/capa/correctmap.py b/common/lib/capa/capa/correctmap.py index 71f97e1321..112976389c 100644 --- a/common/lib/capa/capa/correctmap.py +++ b/common/lib/capa/capa/correctmap.py @@ -32,6 +32,7 @@ class CorrectMap(object): def __iter__(self): return self.cmap.__iter__() + # See the documentation for 'set_dict' for the use of kwargs def set(self, answer_id=None, correctness=None, npoints=None, msg='', hint='', hintmode=None, queuestate=None, **kwargs): if answer_id is not None: self.cmap[answer_id] = {'correctness': correctness, @@ -53,9 +54,19 @@ class CorrectMap(object): def set_dict(self, correct_map): ''' - set internal dict to provided correct_map dict - for graceful migration, if correct_map is a one-level dict, then convert it to the new - dict of dicts format. + Set internal dict of CorrectMap to provided correct_map dict + + correct_map is saved by LMS as a plaintext JSON dump of the correctmap dict. This means that + when the definition of CorrectMap (e.g. its properties) are altered, existing correct_map dict + not coincide with the newest CorrectMap format as defined by self.set. + + For graceful migration, feed the contents of each correct map to self.set, rather than + making a direct copy of the given correct_map dict. This way, the common keys between + the incoming correct_map dict and the new CorrectMap instance will be written, while + mismatched keys will be gracefully ignored. + + Special migration case: + If correct_map is a one-level dict, then convert it to the new dict of dicts format. ''' if correct_map and not (type(correct_map[correct_map.keys()[0]]) == dict): self.__init__() # empty current dict From 0cff6fa30dd3ece7af80a3b73e4f200274f84532 Mon Sep 17 00:00:00 2001 From: kimth Date: Mon, 27 Aug 2012 18:51:38 -0400 Subject: [PATCH 038/181] Remove debugging comment --- common/lib/capa/capa/capa_problem.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index 8edc65b9f9..d7bae1a36c 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -98,10 +98,7 @@ class LoncapaProblem(object): if 'student_answers' in state: self.student_answers = state['student_answers'] if 'correct_map' in state: - print 'THK: LoncapaProblem.__init__' - print json.dumps(state['correct_map'], indent=4) self.correct_map.set_dict(state['correct_map']) - print json.dumps(self.correct_map.get_dict(), indent=4) if 'done' in state: self.done = state['done'] From dca16ad007b60f093c36cbf126e986e1e85730bb Mon Sep 17 00:00:00 2001 From: kimth Date: Mon, 27 Aug 2012 19:01:36 -0400 Subject: [PATCH 039/181] CorrectMap uses dict to represent queuestate --- common/lib/capa/capa/correctmap.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/common/lib/capa/capa/correctmap.py b/common/lib/capa/capa/correctmap.py index 112976389c..07454471f0 100644 --- a/common/lib/capa/capa/correctmap.py +++ b/common/lib/capa/capa/correctmap.py @@ -15,7 +15,7 @@ class CorrectMap(object): - msg : string (may have HTML) giving extra message response (displayed below textline or textbox) - hint : string (may have HTML) giving optional hint (displayed below textline or textbox, above msg) - hintmode : one of (None,'on_request','always') criteria for displaying hint - - queuestate : Tuple (key, time) where key is a secret string, and time is a string dump + - queuestate : Dict {key:'', time:''} where key is a secret string, and time is a string dump of a DateTime object in the format '%Y%m%d%H%M%S'. Is None when not queued Behaves as a dict. @@ -83,10 +83,10 @@ class CorrectMap(object): return answer_id in self.cmap and self.cmap[answer_id]['queuestate'] is not None def is_right_queuekey(self, answer_id, test_key): - return self.is_queued(answer_id) and self.cmap[answer_id]['queuestate'][0] == test_key + return self.is_queued(answer_id) and self.cmap[answer_id]['queuestate']['key'] == test_key def get_queuetime_str(self, answer_id): - return self.cmap[answer_id]['queuestate'][1] if self.is_queued(answer_id) else None + return self.cmap[answer_id]['queuestate']['time'] if self.is_queued(answer_id) else None def get_npoints(self, answer_id): npoints = self.get_property(answer_id, 'npoints') From 50481c2a81f6659ee162a0e12b8bd69e998a52e1 Mon Sep 17 00:00:00 2001 From: kimth Date: Mon, 27 Aug 2012 19:04:36 -0400 Subject: [PATCH 040/181] CodeResponse saves queuestate as dict --- common/lib/capa/capa/responsetypes.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 5cf45adce2..c997cec30b 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1151,7 +1151,9 @@ class CodeResponse(LoncapaResponse): # State associated with the queueing request qtime = datetime.strftime(datetime.now(), '%Y%m%d%H%M%S') - queuestate = (queuekey, qtime) + queuestate = {'key': queuekey, + 'time': qtime, + } cmap = CorrectMap() if error: From a9a73561f434972515984eff34d7d30b7c3afddf Mon Sep 17 00:00:00 2001 From: John Hess Date: Mon, 27 Aug 2012 19:05:00 -0400 Subject: [PATCH 041/181] updated admin_dashboard to show count of unique students --- lms/djangoapps/dashboard/views.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/dashboard/views.py b/lms/djangoapps/dashboard/views.py index c4446bceaa..a1a3461953 100644 --- a/lms/djangoapps/dashboard/views.py +++ b/lms/djangoapps/dashboard/views.py @@ -21,11 +21,16 @@ def dashboard(request): if not request.user.is_staff: raise Http404 - query = "select count(user_id) as students, course_id from student_courseenrollment group by course_id order by students desc" + queries=[] + queries.append("select count(user_id) as students, course_id from student_courseenrollment group by course_id order by students desc;") + queries.append("select count(distinct user_id) as unique_students from student_courseenrollment;") from django.db import connection cursor = connection.cursor() - cursor.execute(query) - results = dictfetchall(cursor) + results =[] + + for query in queries: + cursor.execute(query) + results.append(dictfetchall(cursor)) return HttpResponse(json.dumps(results, indent=4)) From 98542f79ece0d1755a1a04b9041f995784227dff Mon Sep 17 00:00:00 2001 From: kimth Date: Mon, 27 Aug 2012 19:14:47 -0400 Subject: [PATCH 042/181] Define dateformat variable in xqueue_interface --- common/lib/capa/capa/capa_problem.py | 7 +++++-- common/lib/capa/capa/responsetypes.py | 2 +- common/lib/capa/capa/xqueue_interface.py | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index d7bae1a36c..9dc7658884 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -34,6 +34,7 @@ from datetime import datetime import eia import inputtypes from util import contextualize_text, convert_files_to_filenames +import xqueue_interface # to be replaced with auto-registering import responsetypes @@ -215,8 +216,10 @@ class LoncapaProblem(object): return None # Get a list of timestamps of all queueing requests, then convert it to a DateTime object - queuetimes = [self.correct_map.get_queuetime_str(answer_id) for answer_id in self.correct_map if self.correct_map.is_queued(answer_id)] - queuetimes = [datetime.strptime(qt,'%Y%m%d%H%M%S') for qt in queuetimes] + queuetime_strs = [self.correct_map.get_queuetime_str(answer_id) + for answer_id in self.correct_map + if self.correct_map.is_queued(answer_id)] + queuetimes = [datetime.strptime(qt_str, xqueue_interface.dateformat) for qt_str in queuetime_strs] return max(queuetimes) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index c997cec30b..2ab5f3eafd 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1150,7 +1150,7 @@ class CodeResponse(LoncapaResponse): body=json.dumps(contents)) # State associated with the queueing request - qtime = datetime.strftime(datetime.now(), '%Y%m%d%H%M%S') + qtime = datetime.strftime(datetime.now(), xqueue_interface.dateformat) queuestate = {'key': queuekey, 'time': qtime, } diff --git a/common/lib/capa/capa/xqueue_interface.py b/common/lib/capa/capa/xqueue_interface.py index 2930eb682d..519f63cacd 100644 --- a/common/lib/capa/capa/xqueue_interface.py +++ b/common/lib/capa/capa/xqueue_interface.py @@ -9,7 +9,7 @@ import time log = logging.getLogger('mitx.' + __name__) - +dateformat = '%Y%m%d%H%M%S' def make_hashkey(seed=None): ''' From 4468fee8e5948d63506b2bfe6e6adb5239cc8000 Mon Sep 17 00:00:00 2001 From: kimth Date: Mon, 27 Aug 2012 19:15:28 -0400 Subject: [PATCH 043/181] Define dateformat variable in xqueue_interface --- common/lib/capa/capa/capa_problem.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index 9dc7658884..3a847f8306 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -205,7 +205,7 @@ class LoncapaProblem(object): ''' Returns True if any part of the problem has been submitted to an external queue ''' - return any([self.correct_map.is_queued(answer_id) for answer_id in self.correct_map]) + return any(self.correct_map.is_queued(answer_id) for answer_id in self.correct_map) def get_recentmost_queuetime(self): From 03588c094c355422821238cf81e3aa0a49934c6e Mon Sep 17 00:00:00 2001 From: kimth Date: Mon, 27 Aug 2012 19:17:00 -0400 Subject: [PATCH 044/181] Move stdlib import location --- common/lib/capa/capa/capa_problem.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index 3a847f8306..f386c9fe24 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -14,6 +14,7 @@ This is used by capa_module. from __future__ import division +from datetime import datetime import json import logging import math @@ -30,7 +31,6 @@ from xml.sax.saxutils import unescape import calc from correctmap import CorrectMap -from datetime import datetime import eia import inputtypes from util import contextualize_text, convert_files_to_filenames From 572e88d5b20fb5a7d326623615922885c328c3af Mon Sep 17 00:00:00 2001 From: kimth Date: Mon, 27 Aug 2012 19:20:50 -0400 Subject: [PATCH 045/181] Adjust wait message --- common/lib/xmodule/xmodule/capa_module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 33a2f6b4c4..0f97de6f3f 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -469,7 +469,7 @@ class CapaModule(XModule): prev_submit_time = self.lcp.get_recentmost_queuetime() waittime_between_requests = settings.XQUEUE_WAITTIME_BETWEEN_REQUESTS if (current_time-prev_submit_time).total_seconds() < waittime_between_requests: - msg = 'You must wait at least %d seconds between queue submissions' % waittime_between_requests + msg = 'You must wait at least %d seconds between submissions' % waittime_between_requests return {'success': msg, 'html': ''} try: From 2cf4b28cf721b96704d0c82387ff371c56fce4c9 Mon Sep 17 00:00:00 2001 From: John Hess Date: Mon, 27 Aug 2012 19:22:19 -0400 Subject: [PATCH 046/181] added additional query to show number of users with any given number of registrations (power users) --- lms/djangoapps/dashboard/views.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/dashboard/views.py b/lms/djangoapps/dashboard/views.py index a1a3461953..964b3fac4a 100644 --- a/lms/djangoapps/dashboard/views.py +++ b/lms/djangoapps/dashboard/views.py @@ -24,7 +24,8 @@ def dashboard(request): queries=[] queries.append("select count(user_id) as students, course_id from student_courseenrollment group by course_id order by students desc;") queries.append("select count(distinct user_id) as unique_students from student_courseenrollment;") - + queries.append("select registrations, count(registrations) from (select count(user_id) as registrations from student_courseenrollment group by user_id) as registrations_per_user group by registrations;") + from django.db import connection cursor = connection.cursor() results =[] From a197f634ea8047da38cd7b388de230ce948a96a4 Mon Sep 17 00:00:00 2001 From: kimth Date: Mon, 27 Aug 2012 19:34:46 -0400 Subject: [PATCH 047/181] Get_queuetime_str does not return None --- common/lib/capa/capa/correctmap.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/capa/capa/correctmap.py b/common/lib/capa/capa/correctmap.py index 07454471f0..52411a8e8c 100644 --- a/common/lib/capa/capa/correctmap.py +++ b/common/lib/capa/capa/correctmap.py @@ -86,7 +86,7 @@ class CorrectMap(object): return self.is_queued(answer_id) and self.cmap[answer_id]['queuestate']['key'] == test_key def get_queuetime_str(self, answer_id): - return self.cmap[answer_id]['queuestate']['time'] if self.is_queued(answer_id) else None + return self.cmap[answer_id]['queuestate']['time'] def get_npoints(self, answer_id): npoints = self.get_property(answer_id, 'npoints') From 26051f9939c4f4597d56146c70ed743f0ede3e72 Mon Sep 17 00:00:00 2001 From: kimth Date: Mon, 27 Aug 2012 19:37:46 -0400 Subject: [PATCH 048/181] Waittime passed through ModuleSystem.xqueue --- common/lib/xmodule/xmodule/capa_module.py | 3 +-- lms/djangoapps/courseware/module_render.py | 4 +++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 0f97de6f3f..ef3de02b37 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -8,7 +8,6 @@ import re import sys from datetime import timedelta -from django.conf import settings from lxml import etree from pkg_resources import resource_string @@ -467,7 +466,7 @@ class CapaModule(XModule): if self.lcp.is_queued(): current_time = datetime.datetime.now() prev_submit_time = self.lcp.get_recentmost_queuetime() - waittime_between_requests = settings.XQUEUE_WAITTIME_BETWEEN_REQUESTS + waittime_between_requests = self.system.xqueue.waittime if (current_time-prev_submit_time).total_seconds() < waittime_between_requests: msg = 'You must wait at least %d seconds between submissions' % waittime_between_requests return {'success': msg, 'html': ''} diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index da9828fb12..0eaa1ee60f 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -217,7 +217,9 @@ def _get_module(user, request, location, student_module_cache, course_id, positi xqueue = {'interface': xqueue_interface, 'callback_url': xqueue_callback_url, - 'default_queuename': xqueue_default_queuename.replace(' ', '_')} + 'default_queuename': xqueue_default_queuename.replace(' ', '_'), + 'waittime': settings.XQUEUE_WAITTIME_BETWEEN_REQUESTS + } def inner_get_module(location): """ From 3051c6aca719be45d5cf7bc977ef4624fa3a8f3d Mon Sep 17 00:00:00 2001 From: kimth Date: Mon, 27 Aug 2012 19:41:35 -0400 Subject: [PATCH 049/181] Access system.xqueue properly as dict --- common/lib/xmodule/xmodule/capa_module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index ef3de02b37..27641af3b5 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -466,7 +466,7 @@ class CapaModule(XModule): if self.lcp.is_queued(): current_time = datetime.datetime.now() prev_submit_time = self.lcp.get_recentmost_queuetime() - waittime_between_requests = self.system.xqueue.waittime + waittime_between_requests = self.system.xqueue['waittime'] if (current_time-prev_submit_time).total_seconds() < waittime_between_requests: msg = 'You must wait at least %d seconds between submissions' % waittime_between_requests return {'success': msg, 'html': ''} From 093f08064114acf398118240bc7819c15c32a13a Mon Sep 17 00:00:00 2001 From: kimth Date: Mon, 27 Aug 2012 19:45:39 -0400 Subject: [PATCH 050/181] Filesubmission input does not reveal queue length --- common/lib/capa/capa/inputtypes.py | 2 +- common/lib/capa/capa/templates/filesubmission.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py index 9290503c9d..a323a5dbfe 100644 --- a/common/lib/capa/capa/inputtypes.py +++ b/common/lib/capa/capa/inputtypes.py @@ -351,7 +351,7 @@ def filesubmission(element, value, status, render_template, msg=''): if status == 'incomplete': # Flag indicating that the problem has been queued, 'msg' is length of queue status = 'queued' queue_len = msg - msg = 'Submitted to grader. (Queue length: %s)' % queue_len + msg = 'Submitted to grader.' context = { 'id': eid, 'state': status, 'msg': msg, 'value': value, 'queue_len': queue_len, 'allowed_files': allowed_files, diff --git a/common/lib/capa/capa/templates/filesubmission.html b/common/lib/capa/capa/templates/filesubmission.html index a859dc8458..e9fd7c5674 100644 --- a/common/lib/capa/capa/templates/filesubmission.html +++ b/common/lib/capa/capa/templates/filesubmission.html @@ -10,7 +10,7 @@ % endif - (${state}) +
${msg|n}
From 7c5879a1a58fcf291f63be0acc8943fd76db569e Mon Sep 17 00:00:00 2001 From: kimth Date: Mon, 27 Aug 2012 19:50:47 -0400 Subject: [PATCH 051/181] Use os.path.join --- common/lib/xmodule/xmodule/tests/__init__.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index bd5d35c75f..474ee1280a 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -19,6 +19,7 @@ import capa.calc as calc import capa.capa_problem as lcp from capa.correctmap import CorrectMap from capa.util import convert_files_to_filenames +from capa.xqueue_interface import dateformat from datetime import datetime from xmodule import graders, x_module from xmodule.x_module import ModuleSystem @@ -36,7 +37,7 @@ i4xs = ModuleSystem( user=Mock(), filestore=fs.osfs.OSFS(os.path.dirname(os.path.realpath(__file__))+"/test_files"), debug=True, - xqueue={'interface':None, 'callback_url':'/', 'default_queuename': 'testqueue'}, + xqueue={'interface':None, 'callback_url':'/', 'default_queuename': 'testqueue', 'waittime': 10}, node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules") ) @@ -286,14 +287,14 @@ class CodeResponseTest(unittest.TestCase): ''' @staticmethod def make_queuestate(key, time): - timestr = datetime.strftime(time,'%Y%m%d%H%M%S') - return (key, timestr) + timestr = datetime.strftime(time, dateformat) + return {'key': key, 'time': timestr} def test_is_queued(self): ''' Simple test of whether LoncapaProblem knows when it's been queued ''' - problem_file = os.path.dirname(__file__) + "/test_files/coderesponse.xml" + problem_file = os.path.join(os.path.dirname(__file__), "test_files/coderesponse.xml") test_lcp = lcp.LoncapaProblem(open(problem_file).read(), '1', system=i4xs) answer_ids = sorted(test_lcp.get_question_answers().keys()) From 2919389d8e5cabfb066960818231e0a2360949a4 Mon Sep 17 00:00:00 2001 From: kimth Date: Mon, 27 Aug 2012 19:53:10 -0400 Subject: [PATCH 052/181] Use os.path.join -- all of them... --- common/lib/xmodule/xmodule/tests/__init__.py | 41 ++++++++++---------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 474ee1280a..5b1843a632 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -295,33 +295,34 @@ class CodeResponseTest(unittest.TestCase): Simple test of whether LoncapaProblem knows when it's been queued ''' problem_file = os.path.join(os.path.dirname(__file__), "test_files/coderesponse.xml") - test_lcp = lcp.LoncapaProblem(open(problem_file).read(), '1', system=i4xs) - - answer_ids = sorted(test_lcp.get_question_answers().keys()) - num_answers = len(answer_ids) + with open(problem_file) as input_file: + test_lcp = lcp.LoncapaProblem(input_file.read(), '1', system=i4xs) + + answer_ids = sorted(test_lcp.get_question_answers().keys()) + num_answers = len(answer_ids) - # CodeResponse requires internal CorrectMap state. Build it now in the unqueued state - cmap = CorrectMap() - for i in range(num_answers): - cmap.update(CorrectMap(answer_id=answer_ids[i], queuestate=None)) - test_lcp.correct_map.update(cmap) + # CodeResponse requires internal CorrectMap state. Build it now in the unqueued state + cmap = CorrectMap() + for i in range(num_answers): + cmap.update(CorrectMap(answer_id=answer_ids[i], queuestate=None)) + test_lcp.correct_map.update(cmap) - self.assertEquals(test_lcp.is_queued(), False) + self.assertEquals(test_lcp.is_queued(), False) - # Now we queue the LCP - cmap = CorrectMap() - for i in range(num_answers): - queuestate = CodeResponseTest.make_queuestate(i, datetime.now()) - cmap.update(CorrectMap(answer_id=answer_ids[i], queuestate=queuestate)) - test_lcp.correct_map.update(cmap) + # Now we queue the LCP + cmap = CorrectMap() + for i in range(num_answers): + queuestate = CodeResponseTest.make_queuestate(i, datetime.now()) + cmap.update(CorrectMap(answer_id=answer_ids[i], queuestate=queuestate)) + test_lcp.correct_map.update(cmap) - self.assertEquals(test_lcp.is_queued(), True) + self.assertEquals(test_lcp.is_queued(), True) def test_update_score(self): ''' Test whether LoncapaProblem.update_score can deliver queued result to the right subproblem ''' - problem_file = os.path.dirname(__file__) + "/test_files/coderesponse.xml" + problem_file = os.path.join(os.path.dirname(__file__), "test_files/coderesponse.xml") test_lcp = lcp.LoncapaProblem(open(problem_file).read(), '1', system=i4xs) answer_ids = sorted(test_lcp.get_question_answers().keys()) @@ -377,7 +378,7 @@ class CodeResponseTest(unittest.TestCase): ''' Test whether the LoncapaProblem knows about the time of queue requests ''' - problem_file = os.path.dirname(__file__) + "/test_files/coderesponse.xml" + problem_file = os.path.join(os.path.dirname(__file__), "test_files/coderesponse.xml") test_lcp = lcp.LoncapaProblem(open(problem_file).read(), '1', system=i4xs) answer_ids = sorted(test_lcp.get_question_answers().keys()) @@ -411,7 +412,7 @@ class CodeResponseTest(unittest.TestCase): ''' Test whether file objects are converted to filenames without altering other structures ''' - problem_file = os.path.dirname(__file__) + "/test_files/coderesponse.xml" + problem_file = os.path.join(os.path.dirname(__file__), "test_files/coderesponse.xml") fp = open(problem_file) answers_with_file = {'1_2_1': 'String-based answer', '1_3_1': ['answer1', 'answer2', 'answer3'], From b4b8f6bc7d69d2771bf287cd2fc987807f821630 Mon Sep 17 00:00:00 2001 From: kimth Date: Mon, 27 Aug 2012 19:54:58 -0400 Subject: [PATCH 053/181] Use with open(file) structure --- common/lib/xmodule/xmodule/tests/__init__.py | 156 ++++++++++--------- 1 file changed, 79 insertions(+), 77 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 5b1843a632..5385b489b2 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -323,55 +323,56 @@ class CodeResponseTest(unittest.TestCase): Test whether LoncapaProblem.update_score can deliver queued result to the right subproblem ''' problem_file = os.path.join(os.path.dirname(__file__), "test_files/coderesponse.xml") - test_lcp = lcp.LoncapaProblem(open(problem_file).read(), '1', system=i4xs) + with open(problem_file) as input_file: + test_lcp = lcp.LoncapaProblem(input_file.read(), '1', system=i4xs) - answer_ids = sorted(test_lcp.get_question_answers().keys()) - num_answers = len(answer_ids) - - # CodeResponse requires internal CorrectMap state. Build it now in the queued state - old_cmap = CorrectMap() - for i in range(num_answers): - queuekey = 1000 + i - queuestate = CodeResponseTest.make_queuestate(1000+i, datetime.now()) - old_cmap.update(CorrectMap(answer_id=answer_ids[i], queuestate=queuestate)) - - # Message format common to external graders - correct_score_msg = json.dumps({'correct':True, 'score':1, 'msg':'MESSAGE'}) - incorrect_score_msg = json.dumps({'correct':False, 'score':0, 'msg':'MESSAGE'}) - - xserver_msgs = {'correct': correct_score_msg, - 'incorrect': incorrect_score_msg,} - - # Incorrect queuekey, state should not be updated - for correctness in ['correct', 'incorrect']: - test_lcp.correct_map = CorrectMap() - test_lcp.correct_map.update(old_cmap) # Deep copy - - test_lcp.update_score(xserver_msgs[correctness], queuekey=0) - self.assertEquals(test_lcp.correct_map.get_dict(), old_cmap.get_dict()) # Deep comparison + answer_ids = sorted(test_lcp.get_question_answers().keys()) + num_answers = len(answer_ids) + # CodeResponse requires internal CorrectMap state. Build it now in the queued state + old_cmap = CorrectMap() for i in range(num_answers): - self.assertTrue(test_lcp.correct_map.is_queued(answer_ids[i])) # Should be still queued, since message undelivered + queuekey = 1000 + i + queuestate = CodeResponseTest.make_queuestate(1000+i, datetime.now()) + old_cmap.update(CorrectMap(answer_id=answer_ids[i], queuestate=queuestate)) - # Correct queuekey, state should be updated - for correctness in ['correct', 'incorrect']: - for i in range(num_answers): # Target specific answer_id's + # Message format common to external graders + correct_score_msg = json.dumps({'correct':True, 'score':1, 'msg':'MESSAGE'}) + incorrect_score_msg = json.dumps({'correct':False, 'score':0, 'msg':'MESSAGE'}) + + xserver_msgs = {'correct': correct_score_msg, + 'incorrect': incorrect_score_msg,} + + # Incorrect queuekey, state should not be updated + for correctness in ['correct', 'incorrect']: test_lcp.correct_map = CorrectMap() - test_lcp.correct_map.update(old_cmap) + test_lcp.correct_map.update(old_cmap) # Deep copy - new_cmap = CorrectMap() - new_cmap.update(old_cmap) - npoints = 1 if correctness=='correct' else 0 - new_cmap.set(answer_id=answer_ids[i], npoints=npoints, correctness=correctness, msg='MESSAGE', queuestate=None) + test_lcp.update_score(xserver_msgs[correctness], queuekey=0) + self.assertEquals(test_lcp.correct_map.get_dict(), old_cmap.get_dict()) # Deep comparison - test_lcp.update_score(xserver_msgs[correctness], queuekey=1000 + i) - self.assertEquals(test_lcp.correct_map.get_dict(), new_cmap.get_dict()) + for i in range(num_answers): + self.assertTrue(test_lcp.correct_map.is_queued(answer_ids[i])) # Should be still queued, since message undelivered - for j in range(num_answers): - if j == i: - self.assertFalse(test_lcp.correct_map.is_queued(answer_ids[j])) # Should be dequeued, message delivered - else: - self.assertTrue(test_lcp.correct_map.is_queued(answer_ids[j])) # Should be queued, message undelivered + # Correct queuekey, state should be updated + for correctness in ['correct', 'incorrect']: + for i in range(num_answers): # Target specific answer_id's + test_lcp.correct_map = CorrectMap() + test_lcp.correct_map.update(old_cmap) + + new_cmap = CorrectMap() + new_cmap.update(old_cmap) + npoints = 1 if correctness=='correct' else 0 + new_cmap.set(answer_id=answer_ids[i], npoints=npoints, correctness=correctness, msg='MESSAGE', queuestate=None) + + test_lcp.update_score(xserver_msgs[correctness], queuekey=1000 + i) + self.assertEquals(test_lcp.correct_map.get_dict(), new_cmap.get_dict()) + + for j in range(num_answers): + if j == i: + self.assertFalse(test_lcp.correct_map.is_queued(answer_ids[j])) # Should be dequeued, message delivered + else: + self.assertTrue(test_lcp.correct_map.is_queued(answer_ids[j])) # Should be queued, message undelivered def test_recentmost_queuetime(self): @@ -379,48 +380,49 @@ class CodeResponseTest(unittest.TestCase): Test whether the LoncapaProblem knows about the time of queue requests ''' problem_file = os.path.join(os.path.dirname(__file__), "test_files/coderesponse.xml") - test_lcp = lcp.LoncapaProblem(open(problem_file).read(), '1', system=i4xs) + with open(problem_file) as input_file: + test_lcp = lcp.LoncapaProblem(input_file.read(), '1', system=i4xs) - answer_ids = sorted(test_lcp.get_question_answers().keys()) - num_answers = len(answer_ids) + answer_ids = sorted(test_lcp.get_question_answers().keys()) + num_answers = len(answer_ids) - # CodeResponse requires internal CorrectMap state. Build it now in the unqueued state - cmap = CorrectMap() - for i in range(num_answers): - cmap.update(CorrectMap(answer_id=answer_ids[i], queuestate=None)) - test_lcp.correct_map.update(cmap) - - self.assertEquals(test_lcp.get_recentmost_queuetime(), None) + # CodeResponse requires internal CorrectMap state. Build it now in the unqueued state + cmap = CorrectMap() + for i in range(num_answers): + cmap.update(CorrectMap(answer_id=answer_ids[i], queuestate=None)) + test_lcp.correct_map.update(cmap) + + self.assertEquals(test_lcp.get_recentmost_queuetime(), None) - # CodeResponse requires internal CorrectMap state. Build it now in the queued state - cmap = CorrectMap() - answer_ids = sorted(test_lcp.get_question_answers().keys()) - num_answers = len(answer_ids) - for i in range(num_answers): - queuekey = 1000 + i - latest_timestamp = datetime.now() - queuestate = CodeResponseTest.make_queuestate(1000+i, latest_timestamp) - cmap.update(CorrectMap(answer_id=answer_ids[i], queuestate=queuestate)) - test_lcp.correct_map.update(cmap) + # CodeResponse requires internal CorrectMap state. Build it now in the queued state + cmap = CorrectMap() + answer_ids = sorted(test_lcp.get_question_answers().keys()) + num_answers = len(answer_ids) + for i in range(num_answers): + queuekey = 1000 + i + latest_timestamp = datetime.now() + queuestate = CodeResponseTest.make_queuestate(1000+i, latest_timestamp) + cmap.update(CorrectMap(answer_id=answer_ids[i], queuestate=queuestate)) + test_lcp.correct_map.update(cmap) - # Queue state only tracks up to second - latest_timestamp = datetime.strptime(datetime.strftime(latest_timestamp,'%Y%m%d%H%M%S'),'%Y%m%d%H%M%S') + # Queue state only tracks up to second + latest_timestamp = datetime.strptime(datetime.strftime(latest_timestamp,'%Y%m%d%H%M%S'),'%Y%m%d%H%M%S') - self.assertEquals(test_lcp.get_recentmost_queuetime(), latest_timestamp) + self.assertEquals(test_lcp.get_recentmost_queuetime(), latest_timestamp) - def test_convert_files_to_filenames(self): - ''' - Test whether file objects are converted to filenames without altering other structures - ''' - problem_file = os.path.join(os.path.dirname(__file__), "test_files/coderesponse.xml") - fp = open(problem_file) - answers_with_file = {'1_2_1': 'String-based answer', - '1_3_1': ['answer1', 'answer2', 'answer3'], - '1_4_1': [fp, fp]} - answers_converted = convert_files_to_filenames(answers_with_file) - self.assertEquals(answers_converted['1_2_1'], 'String-based answer') - self.assertEquals(answers_converted['1_3_1'], ['answer1', 'answer2', 'answer3']) - self.assertEquals(answers_converted['1_4_1'], [fp.name, fp.name]) + def test_convert_files_to_filenames(self): + ''' + Test whether file objects are converted to filenames without altering other structures + ''' + problem_file = os.path.join(os.path.dirname(__file__), "test_files/coderesponse.xml") + with open(problem_file) as fp: + answers_with_file = {'1_2_1': 'String-based answer', + '1_3_1': ['answer1', 'answer2', 'answer3'], + '1_4_1': [fp, fp]} + answers_converted = convert_files_to_filenames(answers_with_file) + self.assertEquals(answers_converted['1_2_1'], 'String-based answer') + self.assertEquals(answers_converted['1_3_1'], ['answer1', 'answer2', 'answer3']) + self.assertEquals(answers_converted['1_4_1'], [fp.name, fp.name]) class ChoiceResponseTest(unittest.TestCase): From 6302283183fe8adff82aaa02a096be15814e47c5 Mon Sep 17 00:00:00 2001 From: kimth Date: Mon, 27 Aug 2012 19:57:46 -0400 Subject: [PATCH 054/181] Drop unnecessary .keys() --- common/lib/xmodule/xmodule/tests/__init__.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 5385b489b2..4c97710607 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -298,7 +298,7 @@ class CodeResponseTest(unittest.TestCase): with open(problem_file) as input_file: test_lcp = lcp.LoncapaProblem(input_file.read(), '1', system=i4xs) - answer_ids = sorted(test_lcp.get_question_answers().keys()) + answer_ids = sorted(test_lcp.get_question_answers()) num_answers = len(answer_ids) # CodeResponse requires internal CorrectMap state. Build it now in the unqueued state @@ -326,7 +326,7 @@ class CodeResponseTest(unittest.TestCase): with open(problem_file) as input_file: test_lcp = lcp.LoncapaProblem(input_file.read(), '1', system=i4xs) - answer_ids = sorted(test_lcp.get_question_answers().keys()) + answer_ids = sorted(test_lcp.get_question_answers()) num_answers = len(answer_ids) # CodeResponse requires internal CorrectMap state. Build it now in the queued state @@ -383,7 +383,7 @@ class CodeResponseTest(unittest.TestCase): with open(problem_file) as input_file: test_lcp = lcp.LoncapaProblem(input_file.read(), '1', system=i4xs) - answer_ids = sorted(test_lcp.get_question_answers().keys()) + answer_ids = sorted(test_lcp.get_question_answers()) num_answers = len(answer_ids) # CodeResponse requires internal CorrectMap state. Build it now in the unqueued state @@ -396,7 +396,6 @@ class CodeResponseTest(unittest.TestCase): # CodeResponse requires internal CorrectMap state. Build it now in the queued state cmap = CorrectMap() - answer_ids = sorted(test_lcp.get_question_answers().keys()) num_answers = len(answer_ids) for i in range(num_answers): queuekey = 1000 + i From 297df37fc05bfef4f674bc4661cacbe09d6969e1 Mon Sep 17 00:00:00 2001 From: kimth Date: Mon, 27 Aug 2012 19:59:09 -0400 Subject: [PATCH 055/181] Drop unnecessary iterator index --- common/lib/xmodule/xmodule/tests/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 4c97710607..b19b7e012b 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -303,8 +303,8 @@ class CodeResponseTest(unittest.TestCase): # CodeResponse requires internal CorrectMap state. Build it now in the unqueued state cmap = CorrectMap() - for i in range(num_answers): - cmap.update(CorrectMap(answer_id=answer_ids[i], queuestate=None)) + for answer_id in answer_ids: + cmap.update(CorrectMap(answer_id=answer_id, queuestate=None)) test_lcp.correct_map.update(cmap) self.assertEquals(test_lcp.is_queued(), False) From 82a0d065a76e644fe92723cae8c1fa9929a1c138 Mon Sep 17 00:00:00 2001 From: kimth Date: Mon, 27 Aug 2012 20:04:48 -0400 Subject: [PATCH 056/181] Use enumerate in loops --- common/lib/xmodule/xmodule/tests/__init__.py | 32 +++++++++----------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index b19b7e012b..a4c02a17f4 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -299,7 +299,6 @@ class CodeResponseTest(unittest.TestCase): test_lcp = lcp.LoncapaProblem(input_file.read(), '1', system=i4xs) answer_ids = sorted(test_lcp.get_question_answers()) - num_answers = len(answer_ids) # CodeResponse requires internal CorrectMap state. Build it now in the unqueued state cmap = CorrectMap() @@ -311,7 +310,7 @@ class CodeResponseTest(unittest.TestCase): # Now we queue the LCP cmap = CorrectMap() - for i in range(num_answers): + for i, answer_id in enumerate(answer_ids): queuestate = CodeResponseTest.make_queuestate(i, datetime.now()) cmap.update(CorrectMap(answer_id=answer_ids[i], queuestate=queuestate)) test_lcp.correct_map.update(cmap) @@ -327,11 +326,10 @@ class CodeResponseTest(unittest.TestCase): test_lcp = lcp.LoncapaProblem(input_file.read(), '1', system=i4xs) answer_ids = sorted(test_lcp.get_question_answers()) - num_answers = len(answer_ids) # CodeResponse requires internal CorrectMap state. Build it now in the queued state old_cmap = CorrectMap() - for i in range(num_answers): + for i, answer_id in enumerate(answer_ids): queuekey = 1000 + i queuestate = CodeResponseTest.make_queuestate(1000+i, datetime.now()) old_cmap.update(CorrectMap(answer_id=answer_ids[i], queuestate=queuestate)) @@ -351,28 +349,28 @@ class CodeResponseTest(unittest.TestCase): test_lcp.update_score(xserver_msgs[correctness], queuekey=0) self.assertEquals(test_lcp.correct_map.get_dict(), old_cmap.get_dict()) # Deep comparison - for i in range(num_answers): - self.assertTrue(test_lcp.correct_map.is_queued(answer_ids[i])) # Should be still queued, since message undelivered + for answer_id in answer_ids: + self.assertTrue(test_lcp.correct_map.is_queued(answer_id)) # Should be still queued, since message undelivered # Correct queuekey, state should be updated for correctness in ['correct', 'incorrect']: - for i in range(num_answers): # Target specific answer_id's + for i, answer_id in enumerate(answer_ids): test_lcp.correct_map = CorrectMap() test_lcp.correct_map.update(old_cmap) new_cmap = CorrectMap() new_cmap.update(old_cmap) npoints = 1 if correctness=='correct' else 0 - new_cmap.set(answer_id=answer_ids[i], npoints=npoints, correctness=correctness, msg='MESSAGE', queuestate=None) + new_cmap.set(answer_id=answer_id, npoints=npoints, correctness=correctness, msg='MESSAGE', queuestate=None) test_lcp.update_score(xserver_msgs[correctness], queuekey=1000 + i) self.assertEquals(test_lcp.correct_map.get_dict(), new_cmap.get_dict()) - for j in range(num_answers): + for j, test_id in enumerate(answer_ids): if j == i: - self.assertFalse(test_lcp.correct_map.is_queued(answer_ids[j])) # Should be dequeued, message delivered + self.assertFalse(test_lcp.correct_map.is_queued(test_id)) # Should be dequeued, message delivered else: - self.assertTrue(test_lcp.correct_map.is_queued(answer_ids[j])) # Should be queued, message undelivered + self.assertTrue(test_lcp.correct_map.is_queued(test_id)) # Should be queued, message undelivered def test_recentmost_queuetime(self): @@ -384,28 +382,26 @@ class CodeResponseTest(unittest.TestCase): test_lcp = lcp.LoncapaProblem(input_file.read(), '1', system=i4xs) answer_ids = sorted(test_lcp.get_question_answers()) - num_answers = len(answer_ids) # CodeResponse requires internal CorrectMap state. Build it now in the unqueued state cmap = CorrectMap() - for i in range(num_answers): - cmap.update(CorrectMap(answer_id=answer_ids[i], queuestate=None)) + for answer_id in answer_ids: + cmap.update(CorrectMap(answer_id=answer_id, queuestate=None)) test_lcp.correct_map.update(cmap) self.assertEquals(test_lcp.get_recentmost_queuetime(), None) # CodeResponse requires internal CorrectMap state. Build it now in the queued state cmap = CorrectMap() - num_answers = len(answer_ids) - for i in range(num_answers): + for i, answer_id in enumerate(answer_ids): queuekey = 1000 + i latest_timestamp = datetime.now() queuestate = CodeResponseTest.make_queuestate(1000+i, latest_timestamp) - cmap.update(CorrectMap(answer_id=answer_ids[i], queuestate=queuestate)) + cmap.update(CorrectMap(answer_id=answer_id, queuestate=queuestate)) test_lcp.correct_map.update(cmap) # Queue state only tracks up to second - latest_timestamp = datetime.strptime(datetime.strftime(latest_timestamp,'%Y%m%d%H%M%S'),'%Y%m%d%H%M%S') + latest_timestamp = datetime.strptime(datetime.strftime(latest_timestamp, dateformat), dateformat) self.assertEquals(test_lcp.get_recentmost_queuetime(), latest_timestamp) From 377b09e170fc7aa21aef6a2b021580602d567cda Mon Sep 17 00:00:00 2001 From: kimth Date: Mon, 27 Aug 2012 20:07:54 -0400 Subject: [PATCH 057/181] Adjust comments --- common/lib/xmodule/xmodule/capa_module.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 27641af3b5..cfe3d2d48b 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -462,14 +462,14 @@ class CapaModule(XModule): self.system.track_function('save_problem_check_fail', event_info) raise NotFoundError('Problem must be reset before it can be checked again') - # Problem queued. Students must wait XQUEUE_WAITTIME_BETWEEN_REQUESTS + # Problem queued. Students must wait a specified waittime before they are allowed to submit if self.lcp.is_queued(): current_time = datetime.datetime.now() prev_submit_time = self.lcp.get_recentmost_queuetime() waittime_between_requests = self.system.xqueue['waittime'] if (current_time-prev_submit_time).total_seconds() < waittime_between_requests: msg = 'You must wait at least %d seconds between submissions' % waittime_between_requests - return {'success': msg, 'html': ''} + return {'success': msg, 'html': ''} # Prompts a modal dialog in ajax callback try: old_state = self.lcp.get_state() From 8ed9ab44e74c82cccda8d85546234be9c22d4db6 Mon Sep 17 00:00:00 2001 From: Arjun Singh Date: Mon, 27 Aug 2012 23:48:42 -0700 Subject: [PATCH 058/181] Contextualizing javascript parameters --- common/lib/capa/capa/responsetypes.py | 38 ++++++++++--------- .../xmodule/js/src/capa/display.coffee | 3 ++ 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index b2d56b48ca..0e14ac46c1 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -384,19 +384,23 @@ class JavascriptResponse(LoncapaResponse): node_path = self.system.node_path + ":" + os.path.normpath(js_dir) tmp_env["NODE_PATH"] = node_path return tmp_env + + def call_node(self, args): + + subprocess_args = ["node"] + subprocess_args.extend(args) + + return subprocess.check_output(subprocess_args, env=self.get_node_env()) def generate_problem_state(self): generator_file = os.path.dirname(os.path.normpath(__file__)) + '/javascript_problem_generator.js' - output = subprocess.check_output(["node", - generator_file, - self.generator, - json.dumps(self.generator_dependencies), - json.dumps(str(self.system.seed)), - json.dumps(self.params) - ], - env=self.get_node_env()).strip() + output = self.call_node([generator_file, + self.generator, + json.dumps(self.generator_dependencies), + json.dumps(str(self.system.seed)), + json.dumps(self.params)]).strip() return json.loads(output) @@ -407,7 +411,8 @@ class JavascriptResponse(LoncapaResponse): for param in self.xml.xpath('//*[@id=$id]//responseparam', id=self.xml.get('id')): - params[param.get("name")] = json.loads(param.get("value")) + raw_param = param.get("value") + params[param.get("name")] = contextualize_text(raw_param, self.context) return params @@ -442,15 +447,12 @@ class JavascriptResponse(LoncapaResponse): submission = json.dumps(None) grader_file = os.path.dirname(os.path.normpath(__file__)) + '/javascript_problem_grader.js' - outputs = subprocess.check_output(["node", - grader_file, - self.grader, - json.dumps(self.grader_dependencies), - submission, - json.dumps(self.problem_state), - json.dumps(self.params) - ], - env=self.get_node_env()).split('\n') + outputs = self.call_node([grader_file, + self.grader, + json.dumps(self.grader_dependencies), + submission, + json.dumps(self.problem_state), + json.dumps(self.params)]).split('\n') all_correct = json.loads(outputs[0].strip()) evaluation = outputs[1].strip() diff --git a/common/lib/xmodule/xmodule/js/src/capa/display.coffee b/common/lib/xmodule/xmodule/js/src/capa/display.coffee index 23fa4d70fe..27d3585f96 100644 --- a/common/lib/xmodule/xmodule/js/src/capa/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/capa/display.coffee @@ -293,6 +293,9 @@ class @Problem problemState = data.data("problem_state") displayClass = window[data.data('display_class')] + if evaluation == '' + evaluation = null + container = $(element).find(".javascriptinput_container") submissionField = $(element).find(".javascriptinput_input") From 3842c39f8a3e5c7fc1c55681310cfc3ff1274e40 Mon Sep 17 00:00:00 2001 From: Arjun Singh Date: Mon, 27 Aug 2012 23:52:27 -0700 Subject: [PATCH 059/181] Fixing order that problem scores are displayed in --- lms/djangoapps/courseware/grades.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index 1af3ab1bda..7f28f3ca5c 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -17,6 +17,7 @@ log = logging.getLogger("mitx.courseware") def yield_module_descendents(module): stack = module.get_display_items() + stack.reverse() while len(stack) > 0: next_module = stack.pop() From f7ebc4dd23f1f6c0700902feaac7e68a2e98fc02 Mon Sep 17 00:00:00 2001 From: Arjun Singh Date: Tue, 28 Aug 2012 00:49:32 -0700 Subject: [PATCH 060/181] Load params from json --- common/lib/capa/capa/responsetypes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 0e14ac46c1..4d24fc6c23 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -412,7 +412,7 @@ class JavascriptResponse(LoncapaResponse): id=self.xml.get('id')): raw_param = param.get("value") - params[param.get("name")] = contextualize_text(raw_param, self.context) + params[param.get("name")] = json.loads(contextualize_text(raw_param, self.context)) return params From 02e076b7250ecf09837428a4812dc8b890c35d5f Mon Sep 17 00:00:00 2001 From: Arjun Singh Date: Tue, 28 Aug 2012 05:49:36 -0700 Subject: [PATCH 061/181] adding pygraphviz --- requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements.txt b/requirements.txt index 72b13e63ba..3376fc1a1d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -46,4 +46,5 @@ django-sekizai<0.7 django-mptt>=0.5.3 sorl-thumbnail networkx +pygraphviz -r repo-requirements.txt From ad745aa9bdfc0589631d2a4693a2818a5dc1d1b7 Mon Sep 17 00:00:00 2001 From: Arjun Singh Date: Tue, 28 Aug 2012 05:53:33 -0700 Subject: [PATCH 062/181] pygraphviz requires libgraphviz-dev on ubuntu --- create-dev-env.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/create-dev-env.sh b/create-dev-env.sh index 96f212c9b5..3664129775 100755 --- a/create-dev-env.sh +++ b/create-dev-env.sh @@ -105,7 +105,7 @@ NUMPY_VER="1.6.2" SCIPY_VER="0.10.1" BREW_FILE="$BASE/mitx/brew-formulas.txt" LOG="/var/tmp/install-$(date +%Y%m%d-%H%M%S).log" -APT_PKGS="curl git python-virtualenv build-essential python-dev gfortran liblapack-dev libfreetype6-dev libpng12-dev libxml2-dev libxslt-dev yui-compressor coffeescript graphviz" +APT_PKGS="curl git python-virtualenv build-essential python-dev gfortran liblapack-dev libfreetype6-dev libpng12-dev libxml2-dev libxslt-dev yui-compressor coffeescript graphviz libgraphviz-dev" if [[ $EUID -eq 0 ]]; then error "This script should not be run using sudo or as the root user" From 947ec74afc0cdc76b594d25edb6214c7ce912724 Mon Sep 17 00:00:00 2001 From: Arjun Singh Date: Tue, 28 Aug 2012 06:13:41 -0700 Subject: [PATCH 063/181] Let JS responses return >1 point --- common/lib/capa/capa/responsetypes.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index b2d56b48ca..d721322d6b 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -435,7 +435,11 @@ class JavascriptResponse(LoncapaResponse): (all_correct, evaluation, solution) = self.run_grader(json_submission) self.solution = solution correctness = 'correct' if all_correct else 'incorrect' - return CorrectMap(self.answer_id, correctness, msg=evaluation) + if all_correct: + points = self.get_max_score() + else: + points = 0 + return CorrectMap(self.answer_id, correctness, npoints=points, msg=evaluation) def run_grader(self, submission): if submission is None or submission == '': From 024b9db10dfb0d462488cdda9f252ef2fd8f9417 Mon Sep 17 00:00:00 2001 From: kimth Date: Tue, 28 Aug 2012 09:27:27 -0400 Subject: [PATCH 064/181] Log errors in external grader message parsing --- common/lib/capa/capa/responsetypes.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index e7d7406cbd..620152be57 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1210,13 +1210,13 @@ class CodeResponse(LoncapaResponse): try: score_result = json.loads(score_msg) except (TypeError, ValueError): - log.error("External grader message should be a JSON-serialized dict") + log.error("External grader message should be a JSON-serialized dict. Received score_msg = %s" % score_msg) return fail if not isinstance(score_result, dict): - log.error("External grader message should be a JSON-serialized dict") + log.error("External grader message should be a JSON-serialized dict. Received score_result = %s" % score_result) return fail for tag in ['correct', 'score', 'msg']: - if not score_result.has_key(tag): + if tag not in score_result: log.error("External grader message is missing one or more required tags: 'correct', 'score', 'msg'") return fail From bd2374b6fe6dd0ddbecc460f2166891a76c4ee23 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 28 Aug 2012 10:22:08 -0400 Subject: [PATCH 065/181] Remove trailing slash from ajax url in ModuleSystem * also add a check for valid location in modx_dispatch --- lms/djangoapps/courseware/module_render.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 6c3db9bc18..e238e9ca06 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -198,6 +198,8 @@ def _get_module(user, request, location, student_module_cache, course_id, positi location=descriptor.location.url(), dispatch=''), ) + # Intended use is as {ajax_url}/{dispatch_command}, so get rid of the trailing slash. + ajax_url = ajax_url.rstrip('/') # Fully qualified callback URL for external queueing system xqueue_callback_url = '{proto}://{host}'.format( @@ -410,6 +412,10 @@ def modx_dispatch(request, dispatch, location, course_id): ''' # ''' (fix emacs broken parsing) + # Check parameters and fail fast if there's a problem + if not Location.is_valid(location): + raise Http404("Invalid location") + # Check for submitted files and basic file size checks p = request.POST.copy() if request.FILES: From 9277f5df8eeaa7a13ed6f54a256d90716dc8f5e6 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 28 Aug 2012 11:27:47 -0400 Subject: [PATCH 066/181] Minor cleanups to url name checking * better error messages * better fallback names * architectural TODO for later... --- common/lib/xmodule/xmodule/modulestore/xml.py | 27 ++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 92eca8f5e6..b0910efca9 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -79,11 +79,12 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): del attr[key] break - def fallback_name(): + def fallback_name(orig_name=None): """Return the fallback name for this module. This is a function instead of a variable because we want it to be lazy.""" - # use the hash of the content--the first 12 bytes should be plenty. - return tag + "_" + hashlib.sha1(xml).hexdigest()[:12] + # append the hash of the content--the first 12 bytes should be plenty. + orig_name = "_" + orig_name if orig_name is not None else "" + return tag + orig_name + "_" + hashlib.sha1(xml).hexdigest()[:12] # Fallback if there was nothing we could use: if url_name is None or url_name == "": @@ -93,8 +94,9 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): need_uniq_names = ('problem', 'sequence', 'video', 'course', 'chapter') if tag in need_uniq_names: - error_tracker("ERROR: no name of any kind specified for {tag}. Student " - "state won't work right. Problem xml: '{xml}...'".format(tag=tag, xml=xml[:100])) + error_tracker("PROBLEM: no name of any kind specified for {tag}. Student " + "state will not be properly tracked for this module. Problem xml:" + " '{xml}...'".format(tag=tag, xml=xml[:100])) else: # TODO (vshnayder): We may want to enable this once course repos are cleaned up. # (or we may want to give up on the requirement for non-state-relevant issues...) @@ -103,13 +105,20 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): # Make sure everything is unique if url_name in self.used_names[tag]: - msg = ("Non-unique url_name in xml. This may break content. url_name={0}. Content={1}" - .format(url_name, xml[:100])) - error_tracker("ERROR: " + msg) + msg = ("Non-unique url_name in xml. This may break state tracking for content." + " url_name={0}. Content={1}".format(url_name, xml[:100])) + error_tracker("PROBLEM: " + msg) log.warning(msg) # Just set name to fallback_name--if there are multiple things with the same fallback name, # they are actually identical, so it's fragile, but not immediately broken. - url_name = fallback_name() + + # TODO (vshnayder): if the tag is a pointer tag, this will + # break the content because we won't have the right link. + # That's also a legitimate attempt to reuse the same content + # from multiple places. Once we actually allow that, we'll + # need to update this to complain about non-unique names for + # definitions, but allow multiple uses. + url_name = fallback_name(url_name) self.used_names[tag].add(url_name) xml_data.set('url_name', url_name) From e9ded1cf3639c21e283761a602e445566440937a Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 28 Aug 2012 11:28:10 -0400 Subject: [PATCH 067/181] slight refactoring to test file * was going to use to test duplicated content, but we're not supporting that for now --- common/test/data/full/chapter/Overview.xml | 2 +- common/test/data/full/video/welcome.xml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 common/test/data/full/video/welcome.xml diff --git a/common/test/data/full/chapter/Overview.xml b/common/test/data/full/chapter/Overview.xml index 89917d20da..a11a11a1e0 100644 --- a/common/test/data/full/chapter/Overview.xml +++ b/common/test/data/full/chapter/Overview.xml @@ -1,5 +1,5 @@ -