From 195fa158cc25ad98722f3bbd375e54c9ccf83b95 Mon Sep 17 00:00:00 2001 From: Bridger Maxwell Date: Fri, 1 Jun 2012 16:03:30 -0400 Subject: [PATCH 01/69] Changed setup script to pull instead of clone if the repo already exists. --- create-dev-env.sh | 54 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 15 deletions(-) diff --git a/create-dev-env.sh b/create-dev-env.sh index a45fc0dba0..c0a052ba04 100755 --- a/create-dev-env.sh +++ b/create-dev-env.sh @@ -48,21 +48,45 @@ EO clone_repos() { cd "$BASE" - output "Cloning mitx" - if [[ -d "$BASE/mitx" ]]; then - mv "$BASE/mitx" "${BASE}/mitx.bak.$$" - fi - git clone git@github.com:MITx/mitx.git >>$LOG - output "Cloning askbot-devel" - if [[ -d "$BASE/askbot-devel" ]]; then - mv "$BASE/askbot-devel" "${BASE}/askbot-devel.bak.$$" - fi - git clone git@github.com:MITx/askbot-devel >>$LOG - output "Cloning data" - if [[ -d "$BASE/data" ]]; then - mv "$BASE/data" "${BASE}/data.bak.$$" - fi - hg clone ssh://hg-content@gp.mitx.mit.edu/data >>$LOG + + if [[ -d "$BASE/mitx/.git" ]]; then + output "Pulling mitx" + cd "$BASE/mitx" + git pull >>$LOG + else + output "Cloning mitx" + if [[ -d "$BASE/mitx" ]]; then + mv "$BASE/mitx" "${BASE}/mitx.bak.$$" + fi + git clone git@github.com:MITx/mitx.git >>$LOG + fi + + cd "$BASE" + if [[ -d "$BASE/askbot-devel/.git" ]]; then + output "Pulling askbot-devel" + cd "$BASE/askbot-devel" + git pull >>$LOG + else + output "Cloning askbot-devel" + if [[ -d "$BASE/askbot-devel" ]]; then + mv "$BASE/askbot-devel" "${BASE}/askbot-devel.bak.$$" + fi + git clone git@github.com:MITx/askbot-devel >>$LOG + fi + + cd "$BASE" + if [[ -d "$BASE/data/.hg" ]]; then + output "Pulling data" + cd "$BASE/data" + hg pull >>$LOG + hg update >>$LOG + else + output "Cloning data" + if [[ -d "$BASE/data" ]]; then + mv "$BASE/data" "${BASE}/data.bak.$$" + fi + hg clone ssh://hg-content@gp.mitx.mit.edu/data >>$LOG + fi } PROG=${0##*/} From 8a523997b11d3e86bcc3d07ee2b7e58c81b9e370 Mon Sep 17 00:00:00 2001 From: Bridger Maxwell Date: Thu, 7 Jun 2012 16:49:51 -0400 Subject: [PATCH 02/69] Added setting for new changes to django-pipeline. Only sass is recompiled each time now. --- lms/envs/common.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lms/envs/common.py b/lms/envs/common.py index e4c0604aa9..5cd7052f12 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -285,6 +285,8 @@ PIPELINE_CSS = { } } +PIPELINE_ALWAYS_RECOMPILE = ['sass/application.scss', 'sass/marketing.scss', 'sass/marketing-ie.scss', 'sass/print.scss'] + PIPELINE_JS = { 'application': { 'source_filenames': [pth.replace(PROJECT_ROOT / 'static/', '') for pth in glob2.glob(PROJECT_ROOT / 'static/coffee/src/**/*.coffee')], From 92c6f1e63d997f1ccbdc3cabc54d80bafa678de8 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Thu, 7 Jun 2012 16:54:05 -0400 Subject: [PATCH 03/69] Add reports for library tests --- rakefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rakefile b/rakefile index 8f49668506..0c1ead929c 100644 --- a/rakefile +++ b/rakefile @@ -74,13 +74,13 @@ end Dir["common/lib/*"].each do |lib| task_name = "test_#{lib}" - report_dir = File.join(REPORT_DIR, task_name) + report_dir = File.join(REPORT_DIR, task_name.gsub('/', '_')) directory report_dir desc "Run tests for common lib #{lib}" - task task_name do + task task_name => report_dir do ENV['NOSE_XUNIT_FILE'] = File.join(report_dir, "nosetests.xml") - sh("nosetests #{lib}") + sh("nosetests #{lib} --cover-erase --with-xunit --with-xcoverage --cover-html --cover-inclusive --cover-package #{File.basename(lib)}") end task :test => task_name end From 1d1af3be747e4cd7257e7a2d12f2da921a78524c Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Thu, 7 Jun 2012 17:00:03 -0400 Subject: [PATCH 04/69] Move coverage report html into the reports directory --- lms/envs/test.py | 2 +- rakefile | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lms/envs/test.py b/lms/envs/test.py index 1d90fecc69..941b855f5c 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -20,7 +20,7 @@ INSTALLED_APPS = [ # Nose Test Runner INSTALLED_APPS += ['django_nose'] -NOSE_ARGS = ['--cover-erase', '--with-xunit', '--with-xcoverage', '--cover-html', '--cover-inclusive'] +NOSE_ARGS = ['--cover-erase', '--with-xunit', '--with-xcoverage', '--cover-html', '--cover-inclusive', '--cover-html-dir', os.environ['NOSE_COVER_HTML_DIR']] for app in os.listdir(PROJECT_ROOT / 'djangoapps'): NOSE_ARGS += ['--cover-package', app] TEST_RUNNER = 'django_nose.NoseTestSuiteRunner' diff --git a/rakefile b/rakefile index 0c1ead929c..ce344b3fa3 100644 --- a/rakefile +++ b/rakefile @@ -66,6 +66,7 @@ end desc "Run all django tests on our djangoapps for the #{system}" task task_name => report_dir do ENV['NOSE_XUNIT_FILE'] = File.join(report_dir, "nosetests.xml") + ENV['NOSE_COVER_HTML_DIR'] = File.join(report_dir, "cover") sh(django_admin(:lms, :test, 'test', *Dir['lms/djangoapps'].each)) end task :test => task_name @@ -80,7 +81,7 @@ Dir["common/lib/*"].each do |lib| desc "Run tests for common lib #{lib}" task task_name => report_dir do ENV['NOSE_XUNIT_FILE'] = File.join(report_dir, "nosetests.xml") - sh("nosetests #{lib} --cover-erase --with-xunit --with-xcoverage --cover-html --cover-inclusive --cover-package #{File.basename(lib)}") + sh("nosetests #{lib} --cover-erase --with-xunit --with-xcoverage --cover-html --cover-inclusive --cover-package #{File.basename(lib)} --cover-html-dir #{File.join(report_dir, "cover")}") end task :test => task_name end From 374ce67e36891bcf37a6643ce2cf10302f976c33 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Thu, 7 Jun 2012 17:04:57 -0400 Subject: [PATCH 05/69] Run pep8 and pylint on all libraries, not just the lms --- rakefile | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/rakefile b/rakefile index ce344b3fa3..0cd4536834 100644 --- a/rakefile +++ b/rakefile @@ -44,17 +44,17 @@ task :default => [:pep8, :pylint, :test] directory REPORT_DIR directory LMS_REPORT_DIR -desc "Run pep8 on all of djangoapps" -task :pep8 => LMS_REPORT_DIR do - sh("pep8 --ignore=E501 lms/djangoapps | tee #{LMS_REPORT_DIR}/pep8.report") +desc "Run pep8 on all libraries" +task :pep8 => REPORT_DIR do + sh("pep8 --ignore=E501 lms/djangoapps common/lib/* | tee #{REPORT_DIR}/pep8.report") end -desc "Run pylint on all of djangoapps" -task :pylint => LMS_REPORT_DIR do - ENV['PYTHONPATH'] = 'lms/djangoapps' - Dir["lms/djangoapps/*"].each do |app| +desc "Run pylint on all libraries" +task :pylint => REPORT_DIR do + Dir["lms/djangoapps/*", "common/lib/*"].each do |app| + ENV['PYTHONPATH'] = File.dirname(app) app = File.basename(app) - sh("pylint -f parseable #{app} | tee #{LMS_REPORT_DIR}/#{app}.pylint.report") + sh("pylint -f parseable #{app} | tee #{REPORT_DIR}/#{app}.pylint.report") end end From 9d0dfd2f027829ec819609c7611bf0a1c242e32d Mon Sep 17 00:00:00 2001 From: ichuang Date: Thu, 7 Jun 2012 22:03:47 -0400 Subject: [PATCH 06/69] fix dogfood wrt lms / xmodule reorg; fix textinput_dynamath call --- common/lib/capa/inputtypes.py | 2 +- lms/lib/dogfood/check.py | 2 +- lms/lib/dogfood/views.py | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/common/lib/capa/inputtypes.py b/common/lib/capa/inputtypes.py index de3e20c1cf..3b25be3db7 100644 --- a/common/lib/capa/inputtypes.py +++ b/common/lib/capa/inputtypes.py @@ -206,7 +206,7 @@ def textline(element, value, state, render_template, msg=""): Simple text line input, with optional size specification. ''' if element.get('math') or element.get('dojs'): # 'dojs' flag is temporary, for backwards compatibility with 8.02x - return SimpleInput.xml_tags['textline_dynamath'](element,value,state,msg) + return SimpleInput.xml_tags['textline_dynamath'](element,value,state,render_template,msg) eid=element.get('id') count = int(eid.split('_')[-2])-1 # HACK size = element.get('size') diff --git a/lms/lib/dogfood/check.py b/lms/lib/dogfood/check.py index 574a01e0b0..1a3979c238 100644 --- a/lms/lib/dogfood/check.py +++ b/lms/lib/dogfood/check.py @@ -5,7 +5,7 @@ import string import traceback from django.conf import settings -import courseware.capa.capa_problem as lcp +import capa.capa_problem as lcp from dogfood.views import update_problem def GenID(length=8, chars=string.letters + string.digits): diff --git a/lms/lib/dogfood/views.py b/lms/lib/dogfood/views.py index 31d3b3ded2..17096afc70 100644 --- a/lms/lib/dogfood/views.py +++ b/lms/lib/dogfood/views.py @@ -21,7 +21,6 @@ from django.http import HttpResponse from django.shortcuts import redirect from mitxmako.shortcuts import render_to_response, render_to_string -import courseware.capa.calc import track.views from lxml import etree @@ -34,7 +33,8 @@ from util.cache import cache from util.views import accepts import courseware.content_parser as content_parser -import courseware.modules +#import courseware.modules +import xmodule log = logging.getLogger("mitx.courseware") @@ -184,7 +184,7 @@ def quickedit(request, id=None, qetemplate='quickedit.html',coursename=None): filestore = OSFS(settings.DATA_DIR + xp), #role = 'staff' if request.user.is_staff else 'student', # TODO: generalize this ) - instance=courseware.modules.get_module_class(module)(system, + instance=xmodule.get_module_class(module)(system, xml, id, state=None) From d690aebc787557695acfe6b1e47bdda8d8ce451e Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 8 Jun 2012 09:08:12 -0400 Subject: [PATCH 07/69] Set up a pylintrc file --- .pylintrc | 249 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ rakefile | 2 +- 2 files changed, 250 insertions(+), 1 deletion(-) create mode 100644 .pylintrc diff --git a/.pylintrc b/.pylintrc new file mode 100644 index 0000000000..54e2f5c8a9 --- /dev/null +++ b/.pylintrc @@ -0,0 +1,249 @@ +[MASTER] + +# Specify a configuration file. +#rcfile= + +# Python code to execute, usually for sys.path manipulation such as +# pygtk.require(). +#init-hook= + +# Profiled execution. +profile=no + +# Add files or directories to the blacklist. They should be base names, not +# paths. +ignore=CVS + +# Pickle collected data for later comparisons. +persistent=yes + +# List of plugins (as comma separated values of python modules names) to load, +# usually to register additional checkers. +load-plugins= + + +[MESSAGES CONTROL] + +# Enable the message, report, category or checker with the given id(s). You can +# either give multiple identifier separated by comma (,) or put this option +# multiple time. +#enable= + +# Disable the message, report, category or checker with the given id(s). You +# can either give multiple identifier separated by comma (,) or put this option +# multiple time (only on the command line, not in the configuration file where +# it should appear only once). +#disable= + + +[REPORTS] + +# Set the output format. Available formats are text, parseable, colorized, msvs +# (visual studio) and html +output-format=text + +# Include message's id in output +include-ids=no + +# Put messages in a separate file for each module / package specified on the +# command line instead of printing them on stdout. Reports (if any) will be +# written in a file name "pylint_global.[txt|html]". +files-output=no + +# Tells whether to display a full report or only the messages +reports=yes + +# Python expression which should return a note less than 10 (10 is the highest +# note). You have access to the variables errors warning, statement which +# respectively contain the number of errors / warnings messages and the total +# number of statements analyzed. This is used by the global evaluation report +# (RP0004). +evaluation=10.0 - ((float(5 * error + warning + refactor + convention) / statement) * 10) + +# Add a comment according to your evaluation note. This is used by the global +# evaluation report (RP0004). +comment=no + + +[TYPECHECK] + +# Tells whether missing members accessed in mixin class should be ignored. A +# mixin class is detected if its name ends with "mixin" (case insensitive). +ignore-mixin-members=yes + +# List of classes names for which member attributes should not be checked +# (useful for classes with attributes dynamically set). +ignored-classes=SQLObject + +# When zope mode is activated, add a predefined set of Zope acquired attributes +# to generated-members. +zope=no + +# List of members which are set dynamically and missed by pylint inference +# system, and so shouldn't trigger E0201 when accessed. Python regular +# expressions are accepted. +generated-members=REQUEST,acl_users,aq_parent + + +[BASIC] + +# Required attributes for module, separated by a comma +required-attributes= + +# List of builtins function names that should not be used, separated by a comma +bad-functions=map,filter,apply,input + +# Regular expression which should only match correct module names +module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$ + +# Regular expression which should only match correct module level names +const-rgx=(([A-Z_][A-Z0-9_]*)|(__.*__))$ + +# Regular expression which should only match correct class names +class-rgx=[A-Z_][a-zA-Z0-9]+$ + +# Regular expression which should only match correct function names +function-rgx=[a-z_][a-z0-9_]{2,30}$ + +# Regular expression which should only match correct method names +method-rgx=[a-z_][a-z0-9_]{2,30}$ + +# Regular expression which should only match correct instance attribute names +attr-rgx=[a-z_][a-z0-9_]{2,30}$ + +# Regular expression which should only match correct argument names +argument-rgx=[a-z_][a-z0-9_]{2,30}$ + +# Regular expression which should only match correct variable names +variable-rgx=[a-z_][a-z0-9_]{2,30}$ + +# Regular expression which should only match correct list comprehension / +# generator expression variable names +inlinevar-rgx=[A-Za-z_][A-Za-z0-9_]*$ + +# Good variable names which should always be accepted, separated by a comma +good-names=i,j,k,ex,Run,_ + +# Bad variable names which should always be refused, separated by a comma +bad-names=foo,bar,baz,toto,tutu,tata + +# Regular expression which should only match functions or classes name which do +# not require a docstring +no-docstring-rgx=__.*__ + + +[MISCELLANEOUS] + +# List of note tags to take in consideration, separated by a comma. +notes=FIXME,XXX,TODO + + +[FORMAT] + +# Maximum number of characters on a single line. +max-line-length=120 + +# Maximum number of lines in a module +max-module-lines=1000 + +# String used as indentation unit. This is usually " " (4 spaces) or "\t" (1 +# tab). +indent-string=' ' + + +[SIMILARITIES] + +# Minimum lines number of a similarity. +min-similarity-lines=4 + +# Ignore comments when computing similarities. +ignore-comments=yes + +# Ignore docstrings when computing similarities. +ignore-docstrings=yes + + +[VARIABLES] + +# Tells whether we should check for unused import in __init__ files. +init-import=no + +# A regular expression matching the beginning of the name of dummy variables +# (i.e. not used). +dummy-variables-rgx=_|dummy + +# List of additional names supposed to be defined in builtins. Remember that +# you should avoid to define new builtins when possible. +additional-builtins= + + +[IMPORTS] + +# Deprecated modules which should not be used, separated by a comma +deprecated-modules=regsub,string,TERMIOS,Bastion,rexec + +# Create a graph of every (i.e. internal and external) dependencies in the +# given file (report RP0402 must not be disabled) +import-graph= + +# Create a graph of external dependencies in the given file (report RP0402 must +# not be disabled) +ext-import-graph= + +# Create a graph of internal dependencies in the given file (report RP0402 must +# not be disabled) +int-import-graph= + + +[DESIGN] + +# Maximum number of arguments for function / method +max-args=5 + +# Argument names that match this expression will be ignored. Default to name +# with leading underscore +ignored-argument-names=_.* + +# Maximum number of locals for function / method body +max-locals=15 + +# Maximum number of return / yield for function / method body +max-returns=6 + +# Maximum number of branch for function / method body +max-branchs=12 + +# Maximum number of statements in function / method body +max-statements=50 + +# Maximum number of parents for a class (see R0901). +max-parents=7 + +# Maximum number of attributes for a class (see R0902). +max-attributes=7 + +# Minimum number of public methods for a class (see R0903). +min-public-methods=2 + +# Maximum number of public methods for a class (see R0904). +max-public-methods=20 + + +[CLASSES] + +# List of interface methods to ignore, separated by a comma. This is used for +# instance to not check methods defines in Zope's Interface base class. +ignore-iface-methods=isImplementedBy,deferred,extends,names,namesAndDescriptions,queryDescriptionFor,getBases,getDescriptionFor,getDoc,getName,getTaggedValue,getTaggedValueTags,isEqualOrExtendedBy,setTaggedValue,isImplementedByInstancesOf,adaptWith,is_implemented_by + +# List of method names used to declare (i.e. assign) instance attributes. +defining-attr-methods=__init__,__new__,setUp + +# List of valid names for the first argument in a class method. +valid-classmethod-first-arg=cls + + +[EXCEPTIONS] + +# Exceptions that will emit a warning when being caught. Defaults to +# "Exception" +overgeneral-exceptions=Exception diff --git a/rakefile b/rakefile index 0cd4536834..29446f4287 100644 --- a/rakefile +++ b/rakefile @@ -54,7 +54,7 @@ task :pylint => REPORT_DIR do Dir["lms/djangoapps/*", "common/lib/*"].each do |app| ENV['PYTHONPATH'] = File.dirname(app) app = File.basename(app) - sh("pylint -f parseable #{app} | tee #{REPORT_DIR}/#{app}.pylint.report") + sh("pylint --rcfile=.pylintrc -f parseable #{app} | tee #{REPORT_DIR}/#{app}.pylint.report") end end From c340ffe0a360b0de8721a5346b69ec7af1482c7a Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Wed, 6 Jun 2012 16:51:31 -0400 Subject: [PATCH 08/69] Fix code formatting --- .../capa/templates/textinput_dynamath.html | 81 ++++++++++--------- 1 file changed, 42 insertions(+), 39 deletions(-) diff --git a/common/lib/capa/templates/textinput_dynamath.html b/common/lib/capa/templates/textinput_dynamath.html index c50658f713..957600064a 100644 --- a/common/lib/capa/templates/textinput_dynamath.html +++ b/common/lib/capa/templates/textinput_dynamath.html @@ -2,49 +2,52 @@ ### version of textline.html which does dynammic math ###
- -
- - + + + + + + + + + +
+ + + - + % if state == 'unsubmitted': + + % elif state == 'correct': + + % elif state == 'incorrect': + + % elif state == 'incomplete': + + % endif +
+ `{::}` + + +
- % if state == 'unsubmitted': - - % elif state == 'correct': - - % elif state == 'incorrect': - - % elif state == 'incomplete': - - % endif - -
- `{::}` - - -
-## +## ## javascript for dynamic math: add this math element to the MathJax rendering queue ## also adds to global jaxset js array -## - +## + + % if msg: -
- ${msg|n} +
+ ${msg|n} % endif
From ac4f09ac4eb3eb41b243b96f947d191b692def3d Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Thu, 7 Jun 2012 16:30:52 -0400 Subject: [PATCH 09/69] Replace dynamite code with tested version --- .../capa/templates/textinput_dynamath.html | 22 +---- .../coffee/fixtures/problem_content.html | 4 + .../coffee/spec/modules/problem_spec.coffee | 38 +++++++- lms/static/coffee/src/modules/problem.coffee | 15 +++ lms/templates/mathjax_include.html | 94 ++++--------------- 5 files changed, 73 insertions(+), 100 deletions(-) diff --git a/common/lib/capa/templates/textinput_dynamath.html b/common/lib/capa/templates/textinput_dynamath.html index 957600064a..e8b26c5fcc 100644 --- a/common/lib/capa/templates/textinput_dynamath.html +++ b/common/lib/capa/templates/textinput_dynamath.html @@ -5,7 +5,7 @@
- + @@ -30,24 +30,4 @@
- -## -## javascript for dynamic math: add this math element to the MathJax rendering queue -## also adds to global jaxset js array -## - - - % if msg: -
- ${msg|n} - % endif diff --git a/lms/static/coffee/fixtures/problem_content.html b/lms/static/coffee/fixtures/problem_content.html index d2e89fed2b..ffcb5e4682 100644 --- a/lms/static/coffee/fixtures/problem_content.html +++ b/lms/static/coffee/fixtures/problem_content.html @@ -6,6 +6,10 @@
+ + + + diff --git a/lms/static/coffee/spec/modules/problem_spec.coffee b/lms/static/coffee/spec/modules/problem_spec.coffee index 6bca63cfe1..78047db3ba 100644 --- a/lms/static/coffee/spec/modules/problem_spec.coffee +++ b/lms/static/coffee/spec/modules/problem_spec.coffee @@ -1,7 +1,11 @@ describe 'Problem', -> beforeEach -> # Stub MathJax - window.MathJax = { Hub: { Queue: -> } } + window.MathJax = + Hub: jasmine.createSpyObj('MathJax.Hub', ['getAllJax', 'Queue']) + Callback: jasmine.createSpyObj('MathJax.Callback', ['After']) + @stubbedJax = root: jasmine.createSpyObj('jax.root', ['toMathML']) + MathJax.Hub.getAllJax.andReturn [@stubbedJax] window.update_schematics = -> loadFixtures 'problem.html' @@ -25,8 +29,8 @@ describe 'Problem', -> describe 'bind', -> beforeEach -> - spyOn MathJax.Hub, 'Queue' spyOn window, 'update_schematics' + MathJax.Hub.getAllJax.andReturn [@stubbedJax] @problem = new Problem 1, '/problem/url/' it 'set mathjax typeset', -> @@ -50,6 +54,12 @@ describe 'Problem', -> it 'bind the save button', -> expect($('section.action input.save')).toHandleWith 'click', @problem.save + it 'bind the math input', -> + expect($('input.math')).toHandleWith 'keyup', @problem.refreshMath + + it 'display the math input', -> + expect(@stubbedJax.root.toMathML).toHaveBeenCalled() + describe 'render', -> beforeEach -> @problem = new Problem 1, '/problem/url/' @@ -223,6 +233,30 @@ describe 'Problem', -> @problem.save() expect(window.alert).toHaveBeenCalledWith 'Saved' + describe 'refreshMath', -> + beforeEach -> + @problem = new Problem 1, '/problem/url/' + @stubbedJax.root.toMathML.andReturn '' + $('#input_example_1').val 'E=mc^2' + + describe 'when there is no exception', -> + beforeEach -> + @problem.refreshMath target: $('#input_example_1').get(0) + + it 'should convert and display the MathML object', -> + expect(MathJax.Hub.Queue).toHaveBeenCalledWith ['Text', @stubbedJax, 'E=mc^2'] + + it 'should display debug output in hidden div', -> + expect($('#input_example_1_dynamath')).toHaveValue '' + + describe 'when there is an exception', -> + beforeEach -> + @stubbedJax.root.toMathML.andThrow {restart: true} + @problem.refreshMath target: $('#input_example_1').get(0) + + it 'should queue up the exception', -> + expect(MathJax.Callback.After).toHaveBeenCalledWith [@problem.refreshMath, @stubbedJax], true + describe 'refreshAnswers', -> beforeEach -> @problem = new Problem 1, '/problem/url/' diff --git a/lms/static/coffee/src/modules/problem.coffee b/lms/static/coffee/src/modules/problem.coffee index e1fac3a1a5..c1a801d2b2 100644 --- a/lms/static/coffee/src/modules/problem.coffee +++ b/lms/static/coffee/src/modules/problem.coffee @@ -15,6 +15,7 @@ class @Problem @$('section.action input.reset').click @reset @$('section.action input.show').click @show @$('section.action input.save').click @save + @$('input.math').keyup(@refreshMath).each(@refreshMath) render: (content) -> if content @@ -62,6 +63,20 @@ class @Problem if response.success alert 'Saved' + refreshMath: (event, element) => + element = event.target unless element + target = "display_#{element.id.replace(/^input_/, '')}" + + if jax = MathJax.Hub.getAllJax(target)[0] + MathJax.Hub.Queue ['Text', jax, $(element).val()] + + try + output = jax.root.toMathML '' + $("##{element.id}_dynamath").val(output) + catch exception + throw exception unless exception.restart + MathJax.Callback.After [@refreshMath, jax], exception.restart + refreshAnswers: => @$('input.schematic').each (index, element) -> element.schematic.update_value() diff --git a/lms/templates/mathjax_include.html b/lms/templates/mathjax_include.html index 681893cadd..c789e4d299 100644 --- a/lms/templates/mathjax_include.html +++ b/lms/templates/mathjax_include.html @@ -5,82 +5,22 @@ ## ## This enables ASCIIMathJAX, and is used by js_textbox - -// })(); - -function DoUpdateMath(inputId) { - var str = document.getElementById("input_"+inputId).value; - - // make sure the input field is in the jaxset - if ($.inArray(inputId,jaxset) == -1){ - //alert('missing '+inputId); - if (document.getElementById("display_" + inputId)){ - MathJax.Hub.queue.Push(function () { - math = MathJax.Hub.getAllJax("display_" + inputId)[0]; - if (math){ - jaxset[inputId] = math; - } - }); - }; - } - - UpdateMath(str,inputId) -} - - - - - + + From a8940fc71e1a2656f863302be2e6595662120d5e Mon Sep 17 00:00:00 2001 From: John Jarvis Date: Fri, 8 Jun 2012 11:30:13 -0400 Subject: [PATCH 10/69] adding less option to skip the intro screen for ruby install --- create-dev-env.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/create-dev-env.sh b/create-dev-env.sh index c0a052ba04..2fdfd183f7 100755 --- a/create-dev-env.sh +++ b/create-dev-env.sh @@ -14,6 +14,7 @@ ouch() { script again with the -v flag. EOL + printf '\E[0m' } error() { @@ -235,7 +236,10 @@ esac output "Installing rvm and ruby" curl -sL get.rvm.io | bash -s stable source $RUBY_DIR/scripts/rvm +# skip the intro +export LESS="-E" rvm install $RUBY_VER +unset LESS virtualenv "$PYTHON_DIR" source $PYTHON_DIR/bin/activate output "Installing gem bundler" From eb4272e38670d930cb3ded0f8596bdd0f623e245 Mon Sep 17 00:00:00 2001 From: John Jarvis Date: Fri, 8 Jun 2012 11:50:57 -0400 Subject: [PATCH 11/69] converted tabs to spaces, updated LESS cmdline override --- create-dev-env.sh | 78 +++++++++++++++++++++++------------------------ 1 file changed, 38 insertions(+), 40 deletions(-) diff --git a/create-dev-env.sh b/create-dev-env.sh index 2fdfd183f7..9e644fe046 100755 --- a/create-dev-env.sh +++ b/create-dev-env.sh @@ -49,45 +49,45 @@ EO clone_repos() { cd "$BASE" - + if [[ -d "$BASE/mitx/.git" ]]; then - output "Pulling mitx" - cd "$BASE/mitx" - git pull >>$LOG - else - output "Cloning mitx" - if [[ -d "$BASE/mitx" ]]; then - mv "$BASE/mitx" "${BASE}/mitx.bak.$$" - fi - git clone git@github.com:MITx/mitx.git >>$LOG - fi - - cd "$BASE" - if [[ -d "$BASE/askbot-devel/.git" ]]; then - output "Pulling askbot-devel" - cd "$BASE/askbot-devel" - git pull >>$LOG - else - output "Cloning askbot-devel" - if [[ -d "$BASE/askbot-devel" ]]; then - mv "$BASE/askbot-devel" "${BASE}/askbot-devel.bak.$$" - fi - git clone git@github.com:MITx/askbot-devel >>$LOG - fi - - cd "$BASE" + output "Pulling mitx" + cd "$BASE/mitx" + git pull >>$LOG + else + output "Cloning mitx" + if [[ -d "$BASE/mitx" ]]; then + mv "$BASE/mitx" "${BASE}/mitx.bak.$$" + fi + git clone git@github.com:MITx/mitx.git >>$LOG + fi + + cd "$BASE" + if [[ -d "$BASE/askbot-devel/.git" ]]; then + output "Pulling askbot-devel" + cd "$BASE/askbot-devel" + git pull >>$LOG + else + output "Cloning askbot-devel" + if [[ -d "$BASE/askbot-devel" ]]; then + mv "$BASE/askbot-devel" "${BASE}/askbot-devel.bak.$$" + fi + git clone git@github.com:MITx/askbot-devel >>$LOG + fi + + cd "$BASE" if [[ -d "$BASE/data/.hg" ]]; then - output "Pulling data" - cd "$BASE/data" - hg pull >>$LOG - hg update >>$LOG - else - output "Cloning data" - if [[ -d "$BASE/data" ]]; then - mv "$BASE/data" "${BASE}/data.bak.$$" - fi - hg clone ssh://hg-content@gp.mitx.mit.edu/data >>$LOG - fi + output "Pulling data" + cd "$BASE/data" + hg pull >>$LOG + hg update >>$LOG + else + output "Cloning data" + if [[ -d "$BASE/data" ]]; then + mv "$BASE/data" "${BASE}/data.bak.$$" + fi + hg clone ssh://hg-content@gp.mitx.mit.edu/data >>$LOG + fi } PROG=${0##*/} @@ -237,9 +237,7 @@ output "Installing rvm and ruby" curl -sL get.rvm.io | bash -s stable source $RUBY_DIR/scripts/rvm # skip the intro -export LESS="-E" -rvm install $RUBY_VER -unset LESS +LESS="-E" rvm install $RUBY_VER virtualenv "$PYTHON_DIR" source $PYTHON_DIR/bin/activate output "Installing gem bundler" From b46b7daf0f8aae5454f1496cf88b8531cdb473cc Mon Sep 17 00:00:00 2001 From: John Jarvis Date: Fri, 8 Jun 2012 14:59:21 -0400 Subject: [PATCH 12/69] added a posix compliant check to make sure you are using bash to run the script, added option to use --system-site-packages for virtualenv, updated the ending instructions on how to start the server --- create-dev-env.sh | 57 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 13 deletions(-) diff --git a/create-dev-env.sh b/create-dev-env.sh index 9e644fe046..7b798c5b8c 100755 --- a/create-dev-env.sh +++ b/create-dev-env.sh @@ -1,5 +1,12 @@ -#!/bin/bash +#!/usr/bin/env bash set -e + +# posix compliant sanity check +if [ -z $BASH ] || [ $BASH = "/bin/sh" ]; then + echo "Please use the bash interpreter to run this script" + exit 1 +fi + trap "ouch" ERR ouch() { @@ -29,6 +36,7 @@ usage() { Usage: $PROG [-c] [-v] [-h] -c compile scipy and numpy + -s --system-site-packages for virtualenv -v set -x + spew -h this @@ -106,7 +114,7 @@ if [[ $EUID -eq 0 ]]; then usage exit 1 fi -ARGS=$(getopt "cvh" "$*") +ARGS=$(getopt "cvhs" "$*") if [[ $? != 0 ]]; then usage exit 1 @@ -118,6 +126,10 @@ while true; do compile=true shift ;; + -s) + systempkgs=true + shift + ;; -v) set -x verbose=true @@ -148,7 +160,7 @@ cat< + + $ rake django-admin[runserver,lms,dev,] + + If the Django development server starts properly you + should see: + + Development server is running at http://127.0.0.1:/ + Quit the server with CONTROL-C. + + Connect your browser to http://127.0.0.1: to + view the Django site. + + END exit 0 From ae44d86e2780c6e0cae137a318cde5bb23630d99 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sat, 9 Jun 2012 18:26:49 -0400 Subject: [PATCH 13/69] add msg to textinput_dynamath --- common/lib/capa/templates/textinput_dynamath.html | 3 +++ 1 file changed, 3 insertions(+) diff --git a/common/lib/capa/templates/textinput_dynamath.html b/common/lib/capa/templates/textinput_dynamath.html index e8b26c5fcc..41b9c5d172 100644 --- a/common/lib/capa/templates/textinput_dynamath.html +++ b/common/lib/capa/templates/textinput_dynamath.html @@ -30,4 +30,7 @@ + % if msg: + ${msg|n} + % endif
From 46b45969d04680b8e1cd74f389d006b8dd064b47 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sat, 9 Jun 2012 18:27:09 -0400 Subject: [PATCH 14/69] first pass in capa cleanup: - responsetype used to be instantiated multiple times(!) in capa_problem now it is instantiated once, and stored in self.responders - responsetypes.GenericResponse restructured; each superclass show now provide setup_response (and not __init__), and may provide get_max_score(); general __init__ provided to clean up superclasses. --- common/lib/capa/capa_problem.py | 191 ++++++------- common/lib/capa/responsetypes.py | 275 ++++++++++++------- common/lib/capa/util.py | 18 ++ lms/djangoapps/courseware/module_render.py | 2 +- lms/static/coffee/src/modules/problem.coffee | 2 +- 5 files changed, 287 insertions(+), 201 deletions(-) diff --git a/common/lib/capa/capa_problem.py b/common/lib/capa/capa_problem.py index b655270a9a..f790190215 100644 --- a/common/lib/capa/capa_problem.py +++ b/common/lib/capa/capa_problem.py @@ -1,6 +1,11 @@ # # File: capa/capa_problem.py # +# Nomenclature: +# +# A capa Problem is a collection of text and capa Response questions. Each Response may have one or more +# Input entry fields. The capa Problem may include a solution. +# ''' Main module which shows problems (of "capa" type). @@ -83,17 +88,32 @@ html_skip = ["numericalresponse", "customresponse", "schematicresponse", "formul class LoncapaProblem(object): + ''' + Main class for capa Problems. + ''' + def __init__(self, fileobject, id, state=None, seed=None, system=None): + ''' + Initializes capa Problem. The problem itself is defined by the XML file + pointed to by fileobject. + + Arguments: + + - filesobject : an OSFS instance: see fs.osfs + - id : string used as the identifier for this problem; often a filename (no spaces) + - state : student state (represented as a dict) + - seed : random number generator seed (int) + - system : I4xSystem instance which provides OS, rendering, and user context + + ''' + ## Initialize class variables from state - self.seed = None self.student_answers = dict() self.correct_map = dict() self.done = False self.problem_id = id self.system = system - - if seed is not None: - self.seed = seed + self.seed = seed if state: if 'seed' in state: @@ -109,22 +129,21 @@ class LoncapaProblem(object): if not self.seed: self.seed = struct.unpack('i', os.urandom(4))[0] - ## Parse XML file - if getattr(system, 'DEBUG', False): + self.fileobject = fileobject # save problem file object, so we can use for debugging information later + if getattr(system, 'DEBUG', False): # get the problem XML string from the problem file log.info("[courseware.capa.capa_problem.lcp.init] fileobject = %s" % fileobject) file_text = fileobject.read() - self.fileobject = fileobject # save it, so we can use for debugging information later - # Convert startouttext and endouttext to proper - # TODO: Do with XML operations - file_text = re.sub("startouttext\s*/", "text", file_text) + file_text = re.sub("startouttext\s*/", "text", file_text) # Convert startouttext and endouttext to proper file_text = re.sub("endouttext\s*/", "/text", file_text) - self.tree = etree.XML(file_text) - self.preprocess_problem(self.tree, correct_map=self.correct_map, answer_map=self.student_answers) + self.tree = etree.XML(file_text) # parse problem XML file into an element tree + + # construct script processor context (eg for customresponse problems) self.context = self.extract_context(self.tree, seed=self.seed) - for response in self.tree.xpath('//' + "|//".join(response_types)): - responder = response_types[response.tag](response, self.context, self.system) - responder.preprocess_response() + + # pre-parse the XML tree: modifies it to add ID's and perform some in-place transformations + # this also creates the list (self.responders) of Response instances for each question in the problem + self.preprocess_problem(self.tree, correct_map=self.correct_map, answer_map=self.student_answers) def __unicode__(self): return u"LoncapaProblem ({0})".format(self.fileobject) @@ -140,12 +159,27 @@ class LoncapaProblem(object): def get_max_score(self): ''' - TODO: multiple points for programming problems. + Return maximum score for this problem. + We do this by counting the number of answers available for each question + in the problem. If the Response for a question has a get_max_score() method + then we call that and add its return value to the count. That can be + used to give complex problems (eg programming questions) multiple points. ''' - sum = 0 - for et in entry_types: - sum = sum + self.tree.xpath('count(//' + et + ')') - return int(sum) + maxscore = 0 + for responder in self.responders: + if hasattr(responder,'get_max_score'): + try: + maxscore += responder.get_max_score() + except Exception, err: + log.error('responder %s failed to properly return from get_max_score()' % responder) + raise + else: + try: + maxscore += len(responder.get_answers()) + except: + log.error('responder %s failed to properly return get_answers()' % responder) + raise + return maxscore def get_score(self): correct = 0 @@ -166,34 +200,35 @@ class LoncapaProblem(object): of each key removed (the string before the first "_"). Thus, for example, input_ID123 -> ID123, and input_fromjs_ID123 -> fromjs_ID123 + + Calles the Response for each question in this problem, to do the actual grading. ''' self.student_answers = answers self.correct_map = dict() - problems_simple = self.extract_problems(self.tree) - for response in problems_simple: - grader = response_types[response.tag](response, self.context, self.system) - results = grader.get_score(answers) # call the responsetype instance to do the actual grading + log.info('%s: in grade_answers, answers=%s' % (self,answers)) + for responder in self.responders: + results = responder.get_score(answers) # call the responsetype instance to do the actual grading self.correct_map.update(results) return self.correct_map def get_question_answers(self): - """Returns a dict of answer_ids to answer values. If we can't generate + """Returns a dict of answer_ids to answer values. If we cannot generate an answer (this sometimes happens in customresponses), that answer_id is not included. Called by "show answers" button JSON request (see capa_module) """ answer_map = dict() - problems_simple = self.extract_problems(self.tree) # purified (flat) XML tree of just response queries - for response in problems_simple: - responder = response_types[response.tag](response, self.context, self.system) # instance of numericalresponse, customresponse,... + for responder in self.responders: results = responder.get_answers() answer_map.update(results) # dict of (id,correct_answer) + # This should be handled in each responsetype, not here. # example for the following: - for entry in problems_simple.xpath("//" + "|//".join(response_properties + entry_types)): - answer = entry.get('correct_answer') # correct answer, when specified elsewhere, eg in a textline - if answer: - answer_map[entry.get('id')] = contextualize_text(answer, self.context) + for responder in self.responders: + for entry in responder.inputfields: + answer = entry.get('correct_answer') # correct answer, when specified elsewhere, eg in a textline + if answer: + answer_map[entry.get('id')] = contextualize_text(answer, self.context) # include solutions from ... stanzas # Tentative merge; we should figure out how we want to handle hints and solutions @@ -209,17 +244,16 @@ class LoncapaProblem(object): the dicts returned by grade_answers and get_question_answers. (Though get_question_answers may only return a subset of these.""" answer_ids = [] - problems_simple = self.extract_problems(self.tree) - for response in problems_simple: - responder = response_types[response.tag](response, self.context) - if hasattr(responder, "answer_id"): - answer_ids.append(responder.answer_id) - # customresponse types can have multiple answer_ids - elif hasattr(responder, "answer_ids"): - answer_ids.extend(responder.answer_ids) - + for responder in self.responders: + answer_ids.append(responder.get_answers().keys()) return answer_ids + def get_html(self): + ''' + Main method called externally to get the HTML to be rendered for this capa Problem. + ''' + return contextualize_text(etree.tostring(self.extract_html(self.tree)[0]), self.context) + # ======= Private ======== def extract_context(self, tree, seed=struct.unpack('i', os.urandom(4))[0]): # private ''' @@ -253,9 +287,6 @@ class LoncapaProblem(object): log.exception("Error while execing code: " + code) return context - def get_html(self): - return contextualize_text(etree.tostring(self.extract_html(self.tree)[0]), self.context) - def extract_html(self, problemtree): # private ''' Helper function for get_html. Recursively converts XML tree to HTML ''' @@ -335,76 +366,34 @@ class LoncapaProblem(object): Assign sub-IDs to all entries (textline, schematic, etc.) Annoted correctness and value In-place transformation + + Also create capa Response instances for each responsetype and save as self.responders ''' response_id = 1 + self.responders = [] for response in tree.xpath('//' + "|//".join(response_types)): response_id_str = self.problem_id + "_" + str(response_id) - response.attrib['id'] = response_id_str - if response_id not in correct_map: - correct = 'unsubmitted' - response.attrib['state'] = correct - response_id = response_id + 1 + response.attrib['id'] = response_id_str # create and save ID for this response + + # if response_id not in correct_map: correct = 'unsubmitted' # unused - to be removed + # response.attrib['state'] = correct + response_id += response_id + answer_id = 1 - for entry in tree.xpath("|".join(['//' + response.tag + '[@id=$id]//' + x for x in (entry_types + solution_types)]), - id=response_id_str): - # assign one answer_id for each entry_type or solution_type + inputfields = tree.xpath("|".join(['//' + response.tag + '[@id=$id]//' + x for x in (entry_types + solution_types)]), + id=response_id_str) + for entry in inputfields: # assign one answer_id for each entry_type or solution_type entry.attrib['response_id'] = str(response_id) entry.attrib['answer_id'] = str(answer_id) entry.attrib['id'] = "%s_%i_%i" % (self.problem_id, response_id, answer_id) answer_id = answer_id + 1 + responder = response_types[response.tag](response, inputfields, self.context, self.system) # instantiate capa Response + self.responders.append(responder) # save in list in self + # ... may not be associated with any specific response; give IDs for those separately # TODO: We should make the namespaces consistent and unique (e.g. %s_problem_%i). solution_id = 1 for solution in tree.findall('.//solution'): solution.attrib['id'] = "%s_solution_%i" % (self.problem_id, solution_id) solution_id += 1 - - def extract_problems(self, problem_tree): - ''' Remove layout from the problem, and give a purified XML tree of just the problems ''' - problem_tree = copy.deepcopy(problem_tree) - tree = Element('problem') - for response in problem_tree.xpath("//" + "|//".join(response_types)): - newresponse = copy.copy(response) - for e in newresponse: - newresponse.remove(e) - # copy.copy is needed to make xpath work right. Otherwise, it starts at the root - # of the tree. We should figure out if there's some work-around - for e in copy.copy(response).xpath("//" + "|//".join(response_properties + entry_types)): - newresponse.append(e) - - tree.append(newresponse) - return tree - -if __name__ == '__main__': - problem_id = 'simpleFormula' - filename = 'simpleFormula.xml' - - problem_id = 'resistor' - filename = 'resistor.xml' - - lcp = LoncapaProblem(filename, problem_id) - - context = lcp.extract_context(lcp.tree) - problem = lcp.extract_problems(lcp.tree) - print lcp.grade_problems({'resistor_2_1': '1.0', 'resistor_3_1': '2.0'}) - #print lcp.grade_problems({'simpleFormula_2_1':'3*x^3'}) -#numericalresponse(problem, context) - -#print etree.tostring((lcp.tree)) - print '============' - print -#print etree.tostring(lcp.extract_problems(lcp.tree)) - print lcp.get_html() -#print extract_context(tree) - - - - # def handle_fr(self, element): - # problem={"answer":self.contextualize_text(answer), - # "type":"formularesponse", - # "tolerance":evaluator({},{},self.contextualize_text(tolerance)), - # "sample_range":dict(zip(variables, sranges)), - # "samples_count": numsamples, - # "id":id, - # self.questions[self.lid]=problem diff --git a/common/lib/capa/responsetypes.py b/common/lib/capa/responsetypes.py index c5683bb0bf..c0ad98baa2 100644 --- a/common/lib/capa/responsetypes.py +++ b/common/lib/capa/responsetypes.py @@ -21,40 +21,123 @@ import abc # specific library imports from calc import evaluator, UndefinedVariable -from util import contextualize_text +from util import * from lxml import etree from lxml.html.soupparser import fromstring as fromstring_bs # uses Beautiful Soup!!! FIXME? -log = logging.getLogger(__name__) +#log = logging.getLogger(__name__) +log = logging.getLogger('mitx.common.lib.capa.responsetypes') -def compare_with_tolerance(v1, v2, tol): - ''' Compare v1 to v2 with maximum tolerance tol - tol is relative if it ends in %; otherwise, it is absolute +#----------------------------------------------------------------------------- +# Exceptions + +class LoncapaProblemError(Exception): ''' - relative = "%" in tol - if relative: - tolerance_rel = evaluator(dict(),dict(),tol[:-1]) * 0.01 - tolerance = tolerance_rel * max(abs(v1), abs(v2)) - else: - tolerance = evaluator(dict(),dict(),tol) - return abs(v1-v2) <= tolerance + Error in specification of a problem + ''' + pass + +class ResponseError(Exception): + ''' + Error for failure in processing a response + ''' + pass + +class StudentInputError(Exception): + pass + +#----------------------------------------------------------------------------- +# +# Main base class for CAPA responsetypes class GenericResponse(object): + ''' + Base class for CAPA responsetypes. Each response type (ie a capa question, + which is part of a capa problem) is represented as a superclass, + which should provide the following methods: + + - get_score : evaluate the given student answers, and return a CorrectMap + - get_answers : provide a dict of the expected answers for this problem + + In addition, these methods are optional: + + - get_max_score : if defined, this is called to obtain the maximum score possible for this question + - setup_response : find and note the answer input field IDs for the response; called by __init__ + + Each response type may also specify the following attributes: + + - max_inputfields : (int) maximum number of answer input fields (checked in __init__ if not None) + - allowed_inputfields : list of allowed input fields (each a string) for this Response + - required_attributes : list of required attributes (each a string) on the main response XML stanza + + ''' __metaclass__=abc.ABCMeta # abc = Abstract Base Class + max_inputfields = None + allowed_inputfields = [] + required_attributes = [] + + def __init__(self, xml, inputfields, context, system=None): + ''' + Init is passed the following arguments: + + - xml : ElementTree of this Response + - inputfields : list of ElementTrees for each input entry field in this Response + - context : script processor context + - system : I4xSystem instance which provides OS, rendering, and user context + - __unicode__ : unicode representation of this Response + + ''' + self.xml = xml + self.inputfields = inputfields + self.context = context + self.system = system + + for abox in inputfields: + if not abox.tag in self.allowed_inputfields: + msg = "%s: cannot have input field %s" % (unicode(self),abox.tag) + msg += "\nSee XML source line %s" % getattr(xml,'sourceline','') + raise LoncapaProblemError(msg) + + if self.max_inputfields and len(inputfields)>self.max_inputfields: + msg = "%s: cannot have more than %s input fields" % (unicode(self),self.max_inputfields) + msg += "\nSee XML source line %s" % getattr(xml,'sourceline','') + raise LoncapaProblemError(msg) + + for prop in self.required_attributes: + if not xml.get(prop): + msg = "Error in problem specification: %s missing required attribute %s" % (unicode(self),prop) + msg += "\nSee XML source line %s" % getattr(xml,'sourceline','') + raise LoncapaProblemError(msg) + + self.answer_ids = [x.get('id') for x in self.inputfields] + if self.max_inputfields==1: + self.answer_id = self.answer_ids[0] # for convenience + + if hasattr(self,'setup_response'): + self.setup_response() + @abc.abstractmethod def get_score(self, student_answers): + ''' + Return a CorrectMap for the answers expected vs given. This includes + (correctness, npoints, msg) for each answer_id. + ''' pass @abc.abstractmethod def get_answers(self): + ''' + Return a dict of (answer_id,answer_text) for each answer for this question. + ''' pass #not an abstract method because plenty of responses will not want to preprocess anything, and we should not require that they override this method. - def preprocess_response(self): + def setup_response(self): pass -#Every response type needs methods "get_score" and "get_answers" + def __unicode__(self): + return 'LoncapaProblem Response %s' % self.xml.tag #----------------------------------------------------------------------------- @@ -69,30 +152,19 @@ class MultipleChoiceResponse(GenericResponse): '''}] - def __init__(self, xml, context, system=None): - self.xml = xml - self.correct_choices = xml.xpath('//*[@id=$id]//choice[@correct="true"]', - id=xml.get('id')) - self.correct_choices = [choice.get('name') for choice in self.correct_choices] - self.context = context - self.answer_field = xml.find('choicegroup') # assumes only ONE choicegroup within this response - self.answer_id = xml.xpath('//*[@id=$id]//choicegroup/@id', - id=xml.get('id')) - if not len(self.answer_id) == 1: - raise Exception("should have exactly one choice group per multiplechoicceresponse") - self.answer_id=self.answer_id[0] + max_inputfields = 1 + allowed_inputfields = ['choicegroup'] - def get_score(self, student_answers): - if self.answer_id in student_answers and student_answers[self.answer_id] in self.correct_choices: - return {self.answer_id:'correct'} - else: - return {self.answer_id:'incorrect'} + def setup_response(self): + self.mc_setup_response() # call secondary setup for MultipleChoice questions, to set name attributes - def get_answers(self): - return {self.answer_id:self.correct_choices} + # define correct choices (after calling secondary setup) + xml = self.xml + cxml = xml.xpath('//*[@id=$id]//choice[@correct="true"]',id=xml.get('id')) + self.correct_choices = [choice.get('name') for choice in cxml] - def preprocess_response(self): + def mc_setup_response(self): ''' Initialize name attributes in stanzas in the in this response. ''' @@ -107,9 +179,22 @@ class MultipleChoiceResponse(GenericResponse): i+=1 else: choice.set("name", "choice_"+choice.get("name")) - + + def get_score(self, student_answers): + ''' + grade student response. + ''' + # log.debug('%s: student_answers=%s, correct_choices=%s' % (unicode(self),student_answers,self.correct_choices)) + if self.answer_id in student_answers and student_answers[self.answer_id] in self.correct_choices: + return {self.answer_id:'correct'} + else: + return {self.answer_id:'incorrect'} + + def get_answers(self): + return {self.answer_id:self.correct_choices} + class TrueFalseResponse(MultipleChoiceResponse): - def preprocess_response(self): + def mc_setup_response(self): i=0 for response in self.xml.xpath("choicegroup"): response.set("type", "TrueFalse") @@ -140,12 +225,13 @@ class OptionResponse(GenericResponse): The location of the earth '''}] - def __init__(self, xml, context, system=None): - self.xml = xml - self.answer_fields = xml.findall('optioninput') - self.context = context + allowed_inputfields = ['optioninput'] + + def setup_response(self): + self.answer_fields = self.inputfields def get_score(self, student_answers): + # log.debug('%s: student_answers=%s' % (unicode(self),student_answers)) cmap = {} amap = self.get_answers() for aid in amap: @@ -157,17 +243,20 @@ class OptionResponse(GenericResponse): def get_answers(self): amap = dict([(af.get('id'),af.get('correct')) for af in self.answer_fields]) + # log.debug('%s: expected answers=%s' % (unicode(self),amap)) return amap #----------------------------------------------------------------------------- class NumericalResponse(GenericResponse): - def __init__(self, xml, context, system=None): - self.xml = xml - if not xml.get('answer'): - msg = "Error in problem specification: numericalresponse missing required answer attribute\n" - msg += "See XML source line %s" % getattr(xml,'sourceline','') - raise Exception,msg + + allowed_inputfields = ['textline'] + required_attributes = ['answer'] + max_inputfields = 1 + + def setup_response(self): + xml = self.xml + context = self.context self.correct_answer = contextualize_text(xml.get('answer'), context) try: self.tolerance_xml = xml.xpath('//*[@id=$id]//responseparam[@type="tolerance"]/@default', @@ -182,7 +271,7 @@ class NumericalResponse(GenericResponse): self.answer_id = None def get_score(self, student_answers): - ''' Display HTML for a numeric response ''' + '''Grade a numeric response ''' student_answer = student_answers[self.answer_id] try: correct = compare_with_tolerance (evaluator(dict(),dict(),student_answer), complex(self.correct_answer), self.tolerance) @@ -241,16 +330,11 @@ def sympy_check2(): '''}] - def __init__(self, xml, context, system=None): - self.xml = xml - self.system = system - ## CRITICAL TODO: Should cover all entrytypes - ## NOTE: xpath will look at root of XML tree, not just - ## what's in xml. @id=id keeps us in the right customresponse. - self.answer_ids = xml.xpath('//*[@id=$id]//textline/@id', - id=xml.get('id')) - self.answer_ids += [x.get('id') for x in xml.findall('textbox')] # also allow textbox inputs - self.context = context + allowed_inputfields = ['textline','textbox'] + + def setup_response(self): + xml = self.xml + context = self.context # if has an "expect" (or "answer") attribute then save that self.expect = xml.get('expect') or xml.get('answer') @@ -271,15 +355,17 @@ def sympy_check2(): cfn = xml.get('cfn') if cfn: log.debug("cfn = %s" % cfn) - if cfn in context: - self.code = context[cfn] + if cfn in self.context: + self.code = self.context[cfn] else: - print "can't find cfn in context = ",context + msg = "%s: can't find cfn in context = %s" % (unicode(self),self.context) + msg += "\nSee XML source line %s" % getattr(self.xml,'sourceline','') + raise LoncapaProblemError(msg) if not self.code: if answer is None: # raise Exception,"[courseware.capa.responsetypes.customresponse] missing code checking script! id=%s" % self.myid - print "[courseware.capa.responsetypes.customresponse] missing code checking script! id=%s" % self.myid + log.error("[courseware.capa.responsetypes.customresponse] missing code checking script! id=%s" % self.myid) self.code = '' else: answer_src = answer.get('src') @@ -294,6 +380,8 @@ def sympy_check2(): of each key removed (the string before the first "_"). ''' + log.debug('%s: student_answers=%s' % (unicode(self),student_answers)) + idset = sorted(self.answer_ids) # ordered list of answer id's try: submission = [student_answers[k] for k in idset] # ordered list of answers @@ -425,12 +513,12 @@ class SymbolicResponse(CustomResponse): Your input should be typed in as a list of lists, eg [[1,2],[3,4]]. '''}] - def __init__(self, xml, context, system=None): - xml.set('cfn','symmath_check') + + def setup_response(self): + self.xml.set('cfn','symmath_check') code = "from symmath import *" - exec code in context,context - CustomResponse.__init__(self,xml,context,system) - + exec code in self.context,self.context + CustomResponse.setup_response(self) #----------------------------------------------------------------------------- @@ -480,15 +568,13 @@ main() '''}] - def __init__(self, xml, context, system=None): - self.xml = xml - self.url = xml.get('url') or "http://eecs1.mit.edu:8889/pyloncapa" # FIXME - hardcoded URL - self.answer_ids = xml.xpath('//*[@id=$id]//textbox/@id|//*[@id=$id]//textline/@id', - id=xml.get('id')) - self.context = context - answer = xml.xpath('//*[@id=$id]//answer', - id=xml.get('id'))[0] + allowed_inputfields = ['textline','textbox'] + def setup_response(self): + xml = self.xml + self.url = xml.get('url') or "http://eecs1.mit.edu:8889/pyloncapa" # FIXME - hardcoded URL + + answer = xml.xpath('//*[@id=$id]//answer',id=xml.get('id'))[0] # FIXME - catch errors answer_src = answer.get('src') if answer_src is not None: self.code = self.system.filesystem.open('src/'+answer_src).read() @@ -590,8 +676,6 @@ main() raise Exception,'Short response from external server' return dict(zip(self.answer_ids,exans)) -class StudentInputError(Exception): - pass #----------------------------------------------------------------------------- @@ -617,8 +701,13 @@ class FormulaResponse(GenericResponse): '''}] - def __init__(self, xml, context, system=None): - self.xml = xml + allowed_inputfields = ['textline'] + required_attributes = ['answer'] + max_inputfields = 1 + + def setup_response(self): + xml = self.xml + context = self.context self.correct_answer = contextualize_text(xml.get('answer'), context) self.samples = contextualize_text(xml.get('samples'), context) try: @@ -628,14 +717,6 @@ class FormulaResponse(GenericResponse): except Exception: self.tolerance = 0 - try: - self.answer_id = xml.xpath('//*[@id=$id]//textline/@id', - id=xml.get('id'))[0] - except Exception: - self.answer_id = None - raise Exception, "[courseware.capa.responsetypes.FormulaResponse] Error: missing answer_id!!" - - self.context = context ts = xml.get('type') if ts is None: typeslist = [] @@ -648,7 +729,6 @@ class FormulaResponse(GenericResponse): else: # Default self.case_sensitive = False - def get_score(self, student_answers): variables=self.samples.split('@')[0].split(',') numsamples=int(self.samples.split('@')[1].split('#')[1]) @@ -697,13 +777,12 @@ class FormulaResponse(GenericResponse): #----------------------------------------------------------------------------- class SchematicResponse(GenericResponse): - def __init__(self, xml, context, system=None): - self.xml = xml - self.answer_ids = xml.xpath('//*[@id=$id]//schematic/@id', - id=xml.get('id')) - self.context = context - answer = xml.xpath('//*[@id=$id]//answer', - id=xml.get('id'))[0] + + allowed_inputfields = ['schematic'] + + def setup_response(self): + xml = self.xml + answer = xml.xpath('//*[@id=$id]//answer', id=xml.get('id'))[0] answer_src = answer.get('src') if answer_src is not None: self.code = self.system.filestore.open('src/'+answer_src).read() # Untested; never used @@ -740,10 +819,10 @@ class ImageResponse(GenericResponse): '''}] - def __init__(self, xml, context, system=None): - self.xml = xml - self.context = context - self.ielements = xml.findall('imageinput') + allowed_inputfields = ['imageinput'] + + def setup_response(self): + self.ielements = self.inputfields self.answer_ids = [ie.get('id') for ie in self.ielements] def get_score(self, student_answers): diff --git a/common/lib/capa/util.py b/common/lib/capa/util.py index d042aa21d3..996f6c8dac 100644 --- a/common/lib/capa/util.py +++ b/common/lib/capa/util.py @@ -1,3 +1,21 @@ +from calc import evaluator, UndefinedVariable + +#----------------------------------------------------------------------------- +# +# Utility functions used in CAPA responsetypes + +def compare_with_tolerance(v1, v2, tol): + ''' Compare v1 to v2 with maximum tolerance tol + tol is relative if it ends in %; otherwise, it is absolute + ''' + relative = "%" in tol + if relative: + tolerance_rel = evaluator(dict(),dict(),tol[:-1]) * 0.01 + tolerance = tolerance_rel * max(abs(v1), abs(v2)) + else: + tolerance = evaluator(dict(),dict(),tol) + return abs(v1-v2) <= tolerance + def contextualize_text(text, context): # private ''' Takes a string with variables. E.g. $a+$b. Does a substitution of those variables from the context ''' diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 97fd1b948c..0f82d9ba94 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -169,7 +169,7 @@ def render_x_module(user, request, xml_module, module_object_preload, position=N content = instance.get_html() # special extra information about each problem, only for users who are staff - if user.is_staff: + if False and user.is_staff: module_id = xml_module.get('id') histogram = grade_histogram(module_id) render_histogram = len(histogram) > 0 diff --git a/lms/static/coffee/src/modules/problem.coffee b/lms/static/coffee/src/modules/problem.coffee index c1a801d2b2..e1e062e949 100644 --- a/lms/static/coffee/src/modules/problem.coffee +++ b/lms/static/coffee/src/modules/problem.coffee @@ -48,7 +48,7 @@ class @Problem @$("label[for='input_#{key}_#{choice}']").attr correct_answer: 'true' else - @$("#answer_#{key}").text(value) + @$("#answer_#{key}").html(value) // needs to be html, not text, for complex solutions (eg coding) @$('.show').val 'Hide Answer' @element.addClass 'showed' else From 7b3ad553074dc323d079d0d564a0c5eeede160ae Mon Sep 17 00:00:00 2001 From: ichuang Date: Sat, 9 Jun 2012 18:35:42 -0400 Subject: [PATCH 15/69] responsetypes - fix comment --- common/lib/capa/responsetypes.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/lib/capa/responsetypes.py b/common/lib/capa/responsetypes.py index c0ad98baa2..1c09493b03 100644 --- a/common/lib/capa/responsetypes.py +++ b/common/lib/capa/responsetypes.py @@ -63,6 +63,7 @@ class GenericResponse(object): - get_max_score : if defined, this is called to obtain the maximum score possible for this question - setup_response : find and note the answer input field IDs for the response; called by __init__ + - __unicode__ : unicode representation of this Response Each response type may also specify the following attributes: @@ -85,7 +86,6 @@ class GenericResponse(object): - inputfields : list of ElementTrees for each input entry field in this Response - context : script processor context - system : I4xSystem instance which provides OS, rendering, and user context - - __unicode__ : unicode representation of this Response ''' self.xml = xml @@ -137,7 +137,7 @@ class GenericResponse(object): pass def __unicode__(self): - return 'LoncapaProblem Response %s' % self.xml.tag + return u'LoncapaProblem Response %s' % self.xml.tag #----------------------------------------------------------------------------- From 7b3c79698fb3e537da9c06a7b3233ec4fe05303e Mon Sep 17 00:00:00 2001 From: ichuang Date: Sat, 9 Jun 2012 21:29:11 -0400 Subject: [PATCH 16/69] second pass in capa cleanup: - each response can now render its own xhtml - cleaned up LoncapaProblem.extract_html --- common/lib/capa/capa_problem.py | 119 ++++++++++++------------------- common/lib/capa/inputtypes.py | 71 ++++-------------- common/lib/capa/responsetypes.py | 40 ++++++++--- 3 files changed, 89 insertions(+), 141 deletions(-) diff --git a/common/lib/capa/capa_problem.py b/common/lib/capa/capa_problem.py index f790190215..c63c13d420 100644 --- a/common/lib/capa/capa_problem.py +++ b/common/lib/capa/capa_problem.py @@ -23,7 +23,6 @@ import scipy import struct from lxml import etree -from lxml.etree import Element from xml.sax.saxutils import unescape from util import contextualize_text @@ -36,6 +35,7 @@ import eia log = logging.getLogger(__name__) +# dict of tagname, Response Class -- this should come from auto-registering response_types = {'numericalresponse': NumericalResponse, 'formularesponse': FormulaResponse, 'customresponse': CustomResponse, @@ -47,20 +47,13 @@ response_types = {'numericalresponse': NumericalResponse, 'optionresponse': OptionResponse, 'symbolicresponse': SymbolicResponse, } -entry_types = ['textline', 'schematic', 'choicegroup', 'textbox', 'imageinput', 'optioninput'] -solution_types = ['solution'] # extra things displayed after "show answers" is pressed -response_properties = ["responseparam", "answer"] # these get captured as student responses -# How to convert from original XML to HTML -# We should do this with xlst later +entry_types = ['textline', 'schematic', 'choicegroup', 'textbox', 'imageinput', 'optioninput'] +solution_types = ['solution'] # extra things displayed after "show answers" is pressed +response_properties = ["responseparam", "answer"] # these get captured as student responses + +# special problem tags which should be turned into innocuous HTML html_transforms = {'problem': {'tag': 'div'}, - "numericalresponse": {'tag': 'span'}, - "customresponse": {'tag': 'span'}, - "externalresponse": {'tag': 'span'}, - "schematicresponse": {'tag': 'span'}, - "formularesponse": {'tag': 'span'}, - "symbolicresponse": {'tag': 'span'}, - "multiplechoiceresponse": {'tag': 'span'}, "text": {'tag': 'span'}, "math": {'tag': 'span'}, } @@ -74,18 +67,6 @@ global_context = {'random': random, # These should be removed from HTML output, including all subelements html_problem_semantics = ["responseparam", "answer", "script"] -# These should be removed from HTML output, but keeping subelements -html_skip = ["numericalresponse", "customresponse", "schematicresponse", "formularesponse", "text", "externalresponse", 'symbolicresponse'] - -# removed in MC -## These should be transformed -#html_special_response = {"textline":inputtypes.textline.render, -# "schematic":inputtypes.schematic.render, -# "textbox":inputtypes.textbox.render, -# "formulainput":inputtypes.jstextline.render, -# "solution":inputtypes.solution.render, -# } - class LoncapaProblem(object): ''' @@ -142,7 +123,8 @@ class LoncapaProblem(object): self.context = self.extract_context(self.tree, seed=self.seed) # pre-parse the XML tree: modifies it to add ID's and perform some in-place transformations - # this also creates the list (self.responders) of Response instances for each question in the problem + # this also creates the dict (self.responders) of Response instances for each question in the problem. + # the dict has keys = xml subtree of Response, values = Response instance self.preprocess_problem(self.tree, correct_map=self.correct_map, answer_map=self.student_answers) def __unicode__(self): @@ -166,7 +148,7 @@ class LoncapaProblem(object): used to give complex problems (eg programming questions) multiple points. ''' maxscore = 0 - for responder in self.responders: + for responder in self.responders.values(): if hasattr(responder,'get_max_score'): try: maxscore += responder.get_max_score() @@ -182,6 +164,10 @@ class LoncapaProblem(object): return maxscore def get_score(self): + ''' + Compute score for this problem. The score is the number of points awarded. + Returns an integer, from 0 to get_max_score(). + ''' correct = 0 for key in self.correct_map: if self.correct_map[key] == u'correct': @@ -206,7 +192,7 @@ class LoncapaProblem(object): self.student_answers = answers self.correct_map = dict() log.info('%s: in grade_answers, answers=%s' % (self,answers)) - for responder in self.responders: + for responder in self.responders.values(): results = responder.get_score(answers) # call the responsetype instance to do the actual grading self.correct_map.update(results) return self.correct_map @@ -218,24 +204,14 @@ class LoncapaProblem(object): (see capa_module) """ answer_map = dict() - for responder in self.responders: + for responder in self.responders.values(): results = responder.get_answers() answer_map.update(results) # dict of (id,correct_answer) - # This should be handled in each responsetype, not here. - # example for the following: - for responder in self.responders: - for entry in responder.inputfields: - answer = entry.get('correct_answer') # correct answer, when specified elsewhere, eg in a textline - if answer: - answer_map[entry.get('id')] = contextualize_text(answer, self.context) - # include solutions from ... stanzas - # Tentative merge; we should figure out how we want to handle hints and solutions for entry in self.tree.xpath("//" + "|//".join(solution_types)): answer = etree.tostring(entry) - if answer: - answer_map[entry.get('id')] = answer + if answer: answer_map[entry.get('id')] = answer return answer_map @@ -244,7 +220,7 @@ class LoncapaProblem(object): the dicts returned by grade_answers and get_question_answers. (Though get_question_answers may only return a subset of these.""" answer_ids = [] - for responder in self.responders: + for responder in self.responders.values(): answer_ids.append(responder.get_answers().keys()) return answer_ids @@ -252,7 +228,7 @@ class LoncapaProblem(object): ''' Main method called externally to get the HTML to be rendered for this capa Problem. ''' - return contextualize_text(etree.tostring(self.extract_html(self.tree)[0]), self.context) + return contextualize_text(etree.tostring(self.extract_html(self.tree)), self.context) # ======= Private ======== def extract_context(self, tree, seed=struct.unpack('i', os.urandom(4))[0]): # private @@ -264,12 +240,11 @@ class LoncapaProblem(object): Problem XML goes to Python execution context. Runs everything in script tags ''' random.seed(self.seed) - context = {'global_context': global_context} # save global context in here also - context.update(global_context) # initialize context to have stuff in global_context - context['__builtins__'] = globals()['__builtins__'] # put globals there also - context['the_lcp'] = self # pass instance of LoncapaProblem in + context = {'global_context': global_context} # save global context in here also + context.update(global_context) # initialize context to have stuff in global_context + context['__builtins__'] = globals()['__builtins__'] # put globals there also + context['the_lcp'] = self # pass instance of LoncapaProblem in - #for script in tree.xpath('/problem/script'): for script in tree.findall('.//script'): stype = script.get('type') if stype: @@ -288,16 +263,20 @@ class LoncapaProblem(object): return context def extract_html(self, problemtree): # private - ''' Helper function for get_html. Recursively converts XML tree to HTML + ''' + Main (private) function which converts Problem XML tree to HTML. + Calls itself recursively. + + Returns Element tree of XHTML representation of problemtree. + Calls render_html of Response instances to render responses into XHTML. + + Used by get_html. ''' if problemtree.tag in html_problem_semantics: return problemid = problemtree.get('id') # my ID - # used to be - # if problemtree.tag in html_special_response: - if problemtree.tag in inputtypes.get_input_xml_tags(): # status is currently the answer for the problem ID for the input element, # but it will turn into a dict containing both the answer and any associated message @@ -334,31 +313,25 @@ class LoncapaProblem(object): use='capa_input') return render_object.get_html() # function(problemtree, value, status, msg) # render the special response (textline, schematic,...) - tree = Element(problemtree.tag) + if problemtree in self.responders: # let each Response render itself + return self.responders[problemtree].render_html(self.extract_html) + + tree = etree.Element(problemtree.tag) for item in problemtree: - subitems = self.extract_html(item) - if subitems is not None: - for subitem in subitems: - tree.append(subitem) - for (key, value) in problemtree.items(): - tree.set(key, value) + item_xhtml = self.extract_html(item) # nothing special: recurse + if item_xhtml is not None: + tree.append(item_xhtml) + + if tree.tag in html_transforms: + tree.tag = html_transforms[problemtree.tag]['tag'] + else: + for (key, value) in problemtree.items(): # copy attributes over if not innocufying + tree.set(key, value) tree.text = problemtree.text tree.tail = problemtree.tail - if problemtree.tag in html_transforms: - tree.tag = html_transforms[problemtree.tag]['tag'] - # Reset attributes. Otherwise, we get metadata in HTML - # (e.g. answers) - # TODO: We should remove and not zero them. - # I'm not sure how to do that quickly with lxml - for k in tree.keys(): - tree.set(k, "") - - # TODO: Fix. This loses Element().tail - #if problemtree.tag in html_skip: - # return tree - return [tree] + return tree def preprocess_problem(self, tree, correct_map=dict(), answer_map=dict()): # private ''' @@ -370,7 +343,7 @@ class LoncapaProblem(object): Also create capa Response instances for each responsetype and save as self.responders ''' response_id = 1 - self.responders = [] + self.responders = {} for response in tree.xpath('//' + "|//".join(response_types)): response_id_str = self.problem_id + "_" + str(response_id) response.attrib['id'] = response_id_str # create and save ID for this response @@ -389,7 +362,7 @@ class LoncapaProblem(object): answer_id = answer_id + 1 responder = response_types[response.tag](response, inputfields, self.context, self.system) # instantiate capa Response - self.responders.append(responder) # save in list in self + self.responders[response] = responder # save in list in self # ... may not be associated with any specific response; give IDs for those separately # TODO: We should make the namespaces consistent and unique (e.g. %s_problem_%i). diff --git a/common/lib/capa/inputtypes.py b/common/lib/capa/inputtypes.py index 3b25be3db7..10fbdb7f98 100644 --- a/common/lib/capa/inputtypes.py +++ b/common/lib/capa/inputtypes.py @@ -33,26 +33,17 @@ def get_input_xml_tags(): class SimpleInput():# XModule ''' Type for simple inputs -- plain HTML with a form element + State is a dictionary with optional keys: * Value * ID * Status (answered, unanswered, unsubmitted) * Feedback (dictionary containing keys for hints, errors, or other feedback from previous attempt) + ''' xml_tags = {} ## Maps tags to functions - - @classmethod - def get_xml_tags(c): - return c.xml_tags.keys() - - @classmethod - def get_uses(c): - return ['capa_input', 'capa_transform'] - - def get_html(self): - return self.xml_tags[self.tag](self.xml, self.value, self.status, self.system.render_template, self.msg) def __init__(self, system, xml, item_id = None, track_url=None, state=None, use = 'capa_input'): self.xml = xml @@ -83,49 +74,16 @@ class SimpleInput():# XModule if 'status' in state: self.status = state['status'] -## TODO -# class SimpleTransform(): -# ''' Type for simple XML to HTML transforms. Examples: -# * Math tags, which go from LON-CAPA-style m-tags to MathJAX -# ''' -# xml_tags = {} ## Maps tags to functions - -# @classmethod -# def get_xml_tags(c): -# return c.xml_tags.keys() + @classmethod + def get_xml_tags(c): + return c.xml_tags.keys() -# @classmethod -# def get_uses(c): -# return ['capa_transform'] - -# def get_html(self): -# return self.xml_tags[self.tag](self.xml, self.value, self.status, self.msg) - -# def __init__(self, system, xml, item_id = None, track_url=None, state=None, use = 'capa_input'): -# self.xml = xml -# self.tag = xml.tag -# if not state: -# state = {} -# if item_id: -# self.id = item_id -# if xml.get('id'): -# self.id = xml.get('id') -# if 'id' in state: -# self.id = state['id'] -# self.system = system - -# self.value = '' -# if 'value' in state: -# self.value = state['value'] - -# self.msg = '' -# if 'feedback' in state and 'message' in state['feedback']: -# self.msg = state['feedback']['message'] - -# self.status = 'unanswered' -# if 'status' in state: -# self.status = state['status'] + @classmethod + def get_uses(c): + return ['capa_input', 'capa_transform'] + def get_html(self): + return self.xml_tags[self.tag](self.xml, self.value, self.status, self.system.render_template, self.msg) def register_render_function(fn, names=None, cls=SimpleInput): if names is None: @@ -136,9 +94,6 @@ def register_render_function(fn, names=None, cls=SimpleInput): return fn return wrapped - - - #----------------------------------------------------------------------------- @register_render_function @@ -201,16 +156,16 @@ def choicegroup(element, value, status, render_template, msg=''): return etree.XML(html) @register_render_function -def textline(element, value, state, render_template, msg=""): +def textline(element, value, status, render_template, msg=""): ''' Simple text line input, with optional size specification. ''' if element.get('math') or element.get('dojs'): # 'dojs' flag is temporary, for backwards compatibility with 8.02x - return SimpleInput.xml_tags['textline_dynamath'](element,value,state,render_template,msg) + return SimpleInput.xml_tags['textline_dynamath'](element,value,status,render_template,msg) eid=element.get('id') count = int(eid.split('_')[-2])-1 # HACK size = element.get('size') - context = {'id':eid, 'value':value, 'state':state, 'count':count, 'size': size, 'msg': msg} + context = {'id':eid, 'value':value, 'state':status, 'count':count, 'size': size, 'msg': msg} html = render_template("textinput.html", context) return etree.XML(html) diff --git a/common/lib/capa/responsetypes.py b/common/lib/capa/responsetypes.py index 1c09493b03..bfd42814f7 100644 --- a/common/lib/capa/responsetypes.py +++ b/common/lib/capa/responsetypes.py @@ -63,7 +63,8 @@ class GenericResponse(object): - get_max_score : if defined, this is called to obtain the maximum score possible for this question - setup_response : find and note the answer input field IDs for the response; called by __init__ - - __unicode__ : unicode representation of this Response + - render_html : render this Response as HTML (must return XHTML compliant string) + - __unicode__ : unicode representation of this Response Each response type may also specify the following attributes: @@ -114,9 +115,30 @@ class GenericResponse(object): if self.max_inputfields==1: self.answer_id = self.answer_ids[0] # for convenience + self.default_answer_map = {} # dict for default answer map (provided in input elements) + for entry in self.inputfields: + answer = entry.get('correct_answer') + if answer: + self.default_answer_map[entry.get('id')] = contextualize_text(answer, self.context) + if hasattr(self,'setup_response'): self.setup_response() + def render_html(self,renderer): + ''' + Return XHTML Element tree representation of this Response. + + Arguments: + + - renderer : procedure which produces HTML given an ElementTree + ''' + tree = etree.Element('span') # render ourself as a + our content + for item in self.xml: + item_xhtml = renderer(item) # call provided procedure to do the rendering + if item_xhtml is not None: tree.append(item_xhtml) + tree.tail = self.xml.tail + return tree + @abc.abstractmethod def get_score(self, student_answers): ''' @@ -132,7 +154,6 @@ class GenericResponse(object): ''' pass - #not an abstract method because plenty of responses will not want to preprocess anything, and we should not require that they override this method. def setup_response(self): pass @@ -485,17 +506,17 @@ def sympy_check2(): ''' Give correct answer expected for this response. - capa_problem handles correct_answers from entry objects like textline, and that - is what should be used when this response has multiple entry objects. + use default_answer_map from entry elements (eg textline), + when this response has multiple entry objects. but for simplicity, if an "expect" attribute was given by the content author - ie then return it now. + ie then that. ''' if len(self.answer_ids)>1: - return {} + return self.default_answer_map if self.expect: return {self.answer_ids[0] : self.expect} - return {} + return self.default_answer_map #----------------------------------------------------------------------------- @@ -797,9 +818,8 @@ class SchematicResponse(GenericResponse): return zip(sorted(self.answer_ids), self.context['correct']) def get_answers(self): - # Since this is explicitly specified in the problem, this will - # be handled by capa_problem - return {} + # use answers provided in input elements + return self.default_answer_map #----------------------------------------------------------------------------- From c724affe316654e031fc66f8f1ebccc6a7f2df90 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sat, 9 Jun 2012 23:29:08 -0400 Subject: [PATCH 17/69] third pass in capa cleanup: correct_map -> CorrectMap - added correctmap.py with CorrectMap class - messages subsumed into CorrectMap - response get_score called with old CorrectMap so hints based on history are possible --- common/lib/capa/capa_problem.py | 83 +++++++++++++++------------- common/lib/capa/correctmap.py | 80 +++++++++++++++++++++++++++ common/lib/capa/responsetypes.py | 92 +++++++++++++++++-------------- common/lib/xmodule/capa_module.py | 25 +++------ 4 files changed, 183 insertions(+), 97 deletions(-) create mode 100644 common/lib/capa/correctmap.py diff --git a/common/lib/capa/capa_problem.py b/common/lib/capa/capa_problem.py index c63c13d420..93d5620aae 100644 --- a/common/lib/capa/capa_problem.py +++ b/common/lib/capa/capa_problem.py @@ -25,15 +25,14 @@ import struct from lxml import etree from xml.sax.saxutils import unescape -from util import contextualize_text -import inputtypes - -from responsetypes import NumericalResponse, FormulaResponse, CustomResponse, SchematicResponse, MultipleChoiceResponse, TrueFalseResponse, ExternalResponse, ImageResponse, OptionResponse, SymbolicResponse - import calc +from correctmap import CorrectMap import eia +import inputtypes +from util import contextualize_text -log = logging.getLogger(__name__) +# to be replaced with auto-registering +from responsetypes import NumericalResponse, FormulaResponse, CustomResponse, SchematicResponse, MultipleChoiceResponse, TrueFalseResponse, ExternalResponse, ImageResponse, OptionResponse, SymbolicResponse # dict of tagname, Response Class -- this should come from auto-registering response_types = {'numericalresponse': NumericalResponse, @@ -68,6 +67,12 @@ global_context = {'random': random, # These should be removed from HTML output, including all subelements html_problem_semantics = ["responseparam", "answer", "script"] +#log = logging.getLogger(__name__) +log = logging.getLogger('mitx.common.lib.capa.capa_problem') + +#----------------------------------------------------------------------------- +# main class for this module + class LoncapaProblem(object): ''' Main class for capa Problems. @@ -89,9 +94,7 @@ class LoncapaProblem(object): ''' ## Initialize class variables from state - self.student_answers = dict() - self.correct_map = dict() - self.done = False + self.do_reset() self.problem_id = id self.system = system self.seed = seed @@ -102,7 +105,7 @@ class LoncapaProblem(object): if 'student_answers' in state: self.student_answers = state['student_answers'] if 'correct_map' in state: - self.correct_map = state['correct_map'] + self.correct_map.set_dict(state['correct_map']) if 'done' in state: self.done = state['done'] @@ -125,7 +128,15 @@ class LoncapaProblem(object): # pre-parse the XML tree: modifies it to add ID's and perform some in-place transformations # this also creates the dict (self.responders) of Response instances for each question in the problem. # the dict has keys = xml subtree of Response, values = Response instance - self.preprocess_problem(self.tree, correct_map=self.correct_map, answer_map=self.student_answers) + self.preprocess_problem(self.tree, answer_map=self.student_answers) + + def do_reset(self): + ''' + Reset internal state to unfinished, with no answers + ''' + self.student_answers = dict() + self.correct_map = CorrectMap() + self.done = False def __unicode__(self): return u"LoncapaProblem ({0})".format(self.fileobject) @@ -134,9 +145,10 @@ class LoncapaProblem(object): ''' Stored per-user session data neeeded to: 1) Recreate the problem 2) Populate any student answers. ''' + return {'seed': self.seed, 'student_answers': self.student_answers, - 'correct_map': self.correct_map, + 'correct_map': self.correct_map.get_dict(), 'done': self.done} def get_max_score(self): @@ -170,8 +182,12 @@ class LoncapaProblem(object): ''' correct = 0 for key in self.correct_map: - if self.correct_map[key] == u'correct': - correct += 1 + try: + correct += self.correct_map.get_npoints(key) + except Exception,err: + log.error('key=%s, correct_map = %s' % (key,self.correct_map)) + raise + if (not self.student_answers) or len(self.student_answers) == 0: return {'score': 0, 'total': self.get_max_score()} @@ -190,12 +206,14 @@ class LoncapaProblem(object): Calles the Response for each question in this problem, to do the actual grading. ''' self.student_answers = answers - self.correct_map = dict() - log.info('%s: in grade_answers, answers=%s' % (self,answers)) + oldcmap = self.correct_map # old CorrectMap + newcmap = CorrectMap() # start new with empty CorrectMap for responder in self.responders.values(): - results = responder.get_score(answers) # call the responsetype instance to do the actual grading - self.correct_map.update(results) - return self.correct_map + results = responder.get_score(answers,oldcmap) # call the responsetype instance to do the actual grading + newcmap.update(results) + self.correct_map = newcmap + log.debug('%s: in grade_answers, answers=%s, cmap=%s' % (self,answers,newcmap)) + return newcmap def get_question_answers(self): """Returns a dict of answer_ids to answer values. If we cannot generate @@ -282,27 +300,17 @@ class LoncapaProblem(object): # but it will turn into a dict containing both the answer and any associated message # for the problem ID for the input element. status = "unsubmitted" + msg = '' if problemid in self.correct_map: - status = self.correct_map[problemtree.get('id')] + pid = problemtree.get('id') + status = self.correct_map.get_correctness(pid) + msg = self.correct_map.get_msg(pid) value = "" if self.student_answers and problemid in self.student_answers: value = self.student_answers[problemid] - #### This code is a hack. It was merged to help bring two branches - #### in sync, but should be replaced. msg should be passed in a - #### response_type - # prepare the response message, if it exists in correct_map - if 'msg' in self.correct_map: - msg = self.correct_map['msg'] - elif ('msg_%s' % problemid) in self.correct_map: - msg = self.correct_map['msg_%s' % problemid] - else: - msg = '' - # do the rendering - # This should be broken out into a helper function - # that handles all input objects render_object = inputtypes.SimpleInput(system=self.system, xml=problemtree, state={'value': value, @@ -333,7 +341,7 @@ class LoncapaProblem(object): return tree - def preprocess_problem(self, tree, correct_map=dict(), answer_map=dict()): # private + def preprocess_problem(self, tree, answer_map=dict()): # private ''' Assign IDs to all the responses Assign sub-IDs to all entries (textline, schematic, etc.) @@ -346,11 +354,8 @@ class LoncapaProblem(object): self.responders = {} for response in tree.xpath('//' + "|//".join(response_types)): response_id_str = self.problem_id + "_" + str(response_id) - response.attrib['id'] = response_id_str # create and save ID for this response - - # if response_id not in correct_map: correct = 'unsubmitted' # unused - to be removed - # response.attrib['state'] = correct - response_id += response_id + response.set('id',response_id_str) # create and save ID for this response + response_id += 1 answer_id = 1 inputfields = tree.xpath("|".join(['//' + response.tag + '[@id=$id]//' + x for x in (entry_types + solution_types)]), diff --git a/common/lib/capa/correctmap.py b/common/lib/capa/correctmap.py new file mode 100644 index 0000000000..3eac98cc3a --- /dev/null +++ b/common/lib/capa/correctmap.py @@ -0,0 +1,80 @@ +#----------------------------------------------------------------------------- +# class used to store graded responses to CAPA questions +# +# Used by responsetypes and capa_problem + +class CorrectMap(object): + ''' + Stores (correctness, npoints, msg) for each answer_id. + Behaves as a dict. + ''' + cmap = {} + + def __init__(self,*args,**kwargs): + self.set(*args,**kwargs) + + def set(self,answer_id=None,correctness=None,npoints=None,msg=''): + if answer_id is not None: + self.cmap[answer_id] = {'correctness': correctness, + 'npoints': npoints, + 'msg': msg } + + def __repr__(self): + return repr(self.cmap) + + def get_dict(self): + ''' + return dict version of self + ''' + return self.cmap + + 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. + ''' + if correct_map and not (type(correct_map[correct_map.keys()[0]])==dict): + for k in self.cmap.keys(): self.cmap.pop(k) # empty current dict + for k in correct_map: self.set(k,correct_map[k]) # create new dict entries + else: + self.cmap = correct_map + + def is_correct(self,answer_id): + if answer_id in self.cmap: return self.cmap[answer_id]['correctness'] == 'correct' + return None + + def get_npoints(self,answer_id): + if self.is_correct(answer_id): + npoints = self.cmap[answer_id].get('npoints',1) # default to 1 point if correct + return npoints or 1 + return 0 # if not correct, return 0 + + def set_property(self,answer_id,property,value): + if answer_id in self.cmap: self.cmap[answer_id][property] = value + else: self.cmap[answer_id] = {property:value} + + def get_property(self,answer_id,property,default=None): + if answer_id in self.cmap: return self.cmap[answer_id].get(property,default) + return default + + def get_correctness(self,answer_id): + return self.get_property(answer_id,'correctness') + + def get_msg(self,answer_id): + return self.get_property(answer_id,'msg','') + + def update(self,other_cmap): + ''' + Update this CorrectMap with the contents of another CorrectMap + ''' + if not isinstance(other_cmap,CorrectMap): + raise Exception('CorrectMap.update called with invalid argument %s' % other_cmap) + self.cmap.update(other_cmap.get_dict()) + + __getitem__ = cmap.__getitem__ + __iter__ = cmap.__iter__ + items = cmap.items + keys = cmap.keys + + diff --git a/common/lib/capa/responsetypes.py b/common/lib/capa/responsetypes.py index bfd42814f7..2de9e27893 100644 --- a/common/lib/capa/responsetypes.py +++ b/common/lib/capa/responsetypes.py @@ -21,6 +21,7 @@ import abc # specific library imports from calc import evaluator, UndefinedVariable +from correctmap import CorrectMap from util import * from lxml import etree from lxml.html.soupparser import fromstring as fromstring_bs # uses Beautiful Soup!!! FIXME? @@ -53,7 +54,7 @@ class StudentInputError(Exception): class GenericResponse(object): ''' Base class for CAPA responsetypes. Each response type (ie a capa question, - which is part of a capa problem) is represented as a superclass, + which is part of a capa problem) is represented as a subclass, which should provide the following methods: - get_score : evaluate the given student answers, and return a CorrectMap @@ -140,10 +141,16 @@ class GenericResponse(object): return tree @abc.abstractmethod - def get_score(self, student_answers): + def get_score(self, student_answers, old_cmap): ''' Return a CorrectMap for the answers expected vs given. This includes (correctness, npoints, msg) for each answer_id. + + Arguments: + + - student_answers : dict of (answer_id,answer) where answer = student input (string) + - old_cmap : previous CorrectMap (may be empty); useful for analyzing or recording history of responses + ''' pass @@ -201,15 +208,15 @@ class MultipleChoiceResponse(GenericResponse): else: choice.set("name", "choice_"+choice.get("name")) - def get_score(self, student_answers): + def get_score(self, student_answers, old_cmap): ''' grade student response. ''' # log.debug('%s: student_answers=%s, correct_choices=%s' % (unicode(self),student_answers,self.correct_choices)) if self.answer_id in student_answers and student_answers[self.answer_id] in self.correct_choices: - return {self.answer_id:'correct'} + return CorrectMap(self.answer_id,'correct') else: - return {self.answer_id:'incorrect'} + return CorrectMap(self.answer_id,'incorrect') def get_answers(self): return {self.answer_id:self.correct_choices} @@ -226,14 +233,14 @@ class TrueFalseResponse(MultipleChoiceResponse): else: choice.set("name", "choice_"+choice.get("name")) - def get_score(self, student_answers): + def get_score(self, student_answers, old_cmap): correct = set(self.correct_choices) answers = set(student_answers.get(self.answer_id, [])) if correct == answers: - return { self.answer_id : 'correct'} + return CorrectMap( self.answer_id , 'correct') - return {self.answer_id : 'incorrect'} + return CorrectMap(self.answer_id ,'incorrect') #----------------------------------------------------------------------------- @@ -251,15 +258,15 @@ class OptionResponse(GenericResponse): def setup_response(self): self.answer_fields = self.inputfields - def get_score(self, student_answers): + def get_score(self, student_answers, old_cmap): # log.debug('%s: student_answers=%s' % (unicode(self),student_answers)) - cmap = {} + cmap = CorrectMap() amap = self.get_answers() for aid in amap: if aid in student_answers and student_answers[aid]==amap[aid]: - cmap[aid] = 'correct' + cmap.set(aid,'correct') else: - cmap[aid] = 'incorrect' + cmap.set(aid,'incorrect') return cmap def get_answers(self): @@ -291,7 +298,7 @@ class NumericalResponse(GenericResponse): except Exception: self.answer_id = None - def get_score(self, student_answers): + def get_score(self, student_answers, old_cmap): '''Grade a numeric response ''' student_answer = student_answers[self.answer_id] try: @@ -303,9 +310,9 @@ class NumericalResponse(GenericResponse): raise StudentInputError('Invalid input -- please use a number only') if correct: - return {self.answer_id:'correct'} + return CorrectMap(self.answer_id,'correct') else: - return {self.answer_id:'incorrect'} + return CorrectMap(self.answer_id,'incorrect') def get_answers(self): return {self.answer_id:self.correct_answer} @@ -395,7 +402,7 @@ def sympy_check2(): else: self.code = answer.text - def get_score(self, student_answers): + def get_score(self, student_answers, old_cmap): ''' student_answers is a dict with everything from request.POST, but with the first part of each key removed (the string before the first "_"). @@ -495,12 +502,10 @@ def sympy_check2(): correct = ['correct']*len(idset) if ret else ['incorrect']*len(idset) # build map giving "correct"ness of the answer(s) - #correct_map = dict(zip(idset, self.context['correct'])) - correct_map = {} + correct_map = CorrectMap() for k in range(len(idset)): - correct_map[idset[k]] = correct[k] - correct_map['msg_%s' % idset[k]] = messages[k] - return correct_map + correct_map.set(idset[k], correct[k], msg=messages[k]) + return correct_map def get_answers(self): ''' @@ -642,9 +647,11 @@ main() return rxml - def get_score(self, student_answers): + def get_score(self, student_answers, old_cmap): + idset = sorted(self.answer_ids) + cmap = CorrectMap() try: - submission = [student_answers[k] for k in sorted(self.answer_ids)] + submission = [student_answers[k] for k in idset] except Exception,err: log.error('Error %s: cannot get student answer for %s; student_answers=%s' % (err,self.answer_ids,student_answers)) raise Exception,err @@ -658,9 +665,9 @@ main() except Exception, err: log.error('Error %s' % err) if self.system.DEBUG: - correct_map = dict(zip(sorted(self.answer_ids), ['incorrect'] * len(self.answer_ids) )) - correct_map['msg_%s' % self.answer_ids[0]] = '%s' % str(err).replace('<','<') - return correct_map + cmap.set_dict(dict(zip(sorted(self.answer_ids), ['incorrect'] * len(idset) ))) + cmap.set_property(self.answer_ids[0],'msg','%s' % str(err).replace('<','<')) + return cmap ad = rxml.find('awarddetail').text admap = {'EXACT_ANS':'correct', # TODO: handle other loncapa responses @@ -670,13 +677,13 @@ main() if ad in admap: self.context['correct'][0] = admap[ad] - # self.context['correct'] = ['correct','correct'] - correct_map = dict(zip(sorted(self.answer_ids), self.context['correct'])) - - # store message in correct_map - correct_map['msg_%s' % self.answer_ids[0]] = rxml.find('message').text.replace(' ',' ') + # create CorrectMap + for key in idset: + idx = idset.index(key) + msg = rxml.find('message').text.replace(' ',' ') if idx==0 else None + cmap.set(key, self.context['correct'][idx], msg=msg) - return correct_map + return cmap def get_answers(self): ''' @@ -750,7 +757,7 @@ class FormulaResponse(GenericResponse): else: # Default self.case_sensitive = False - def get_score(self, student_answers): + def get_score(self, student_answers, old_cmap): variables=self.samples.split('@')[0].split(',') numsamples=int(self.samples.split('@')[1].split('#')[1]) sranges=zip(*map(lambda x:map(float, x.split(",")), @@ -776,11 +783,11 @@ class FormulaResponse(GenericResponse): #traceback.print_exc() raise StudentInputError("Error in formula") if numpy.isnan(student_result) or numpy.isinf(student_result): - return {self.answer_id:"incorrect"} + return CorrectMap(self.answer_id, "incorrect") if not compare_with_tolerance(student_result, instructor_result, self.tolerance): - return {self.answer_id:"incorrect"} + return CorrectMap(self.answer_id, "incorrect") - return {self.answer_id:"correct"} + return CorrectMap(self.answer_id, "correct") def strip_dict(self, d): ''' Takes a dict. Returns an identical dict, with all non-word @@ -810,12 +817,13 @@ class SchematicResponse(GenericResponse): else: self.code = answer.text - def get_score(self, student_answers): + def get_score(self, student_answers, old_cmap): from capa_problem import global_context submission = [json.loads(student_answers[k]) for k in sorted(self.answer_ids)] self.context.update({'submission':submission}) exec self.code in global_context, self.context - return zip(sorted(self.answer_ids), self.context['correct']) + cmap = CorrectMap() + return cmap.set_dict(zip(sorted(self.answer_ids), self.context['correct'])) def get_answers(self): # use answers provided in input elements @@ -845,8 +853,8 @@ class ImageResponse(GenericResponse): self.ielements = self.inputfields self.answer_ids = [ie.get('id') for ie in self.ielements] - def get_score(self, student_answers): - correct_map = {} + def get_score(self, student_answers, old_cmap): + correct_map = CorrectMap() expectedset = self.get_answers() for aid in self.answer_ids: # loop through IDs of fields in our stanza @@ -869,9 +877,9 @@ class ImageResponse(GenericResponse): # answer is correct if (x,y) is within the specified rectangle if (llx <= gx <= urx) and (lly <= gy <= ury): - correct_map[aid] = 'correct' + correct_map.set(aid, 'correct') else: - correct_map[aid] = 'incorrect' + correct_map.set(aid, 'incorrect') return correct_map def get_answers(self): diff --git a/common/lib/xmodule/capa_module.py b/common/lib/xmodule/capa_module.py index 6bd7cbebdc..439982a2c1 100644 --- a/common/lib/xmodule/capa_module.py +++ b/common/lib/xmodule/capa_module.py @@ -13,6 +13,7 @@ from lxml import etree from x_module import XModule, XModuleDescriptor from capa.capa_problem import LoncapaProblem from capa.responsetypes import StudentInputError + log = logging.getLogger("mitx.courseware") #----------------------------------------------------------------------------- @@ -365,18 +366,17 @@ class Module(XModule): self.attempts = self.attempts + 1 self.lcp.done=True - success = 'correct' - for i in correct_map: - if correct_map[i]!='correct': + success = 'correct' # success = correct if ALL questions in this problem are correct + for answer_id in correct_map: + if not correct_map.is_correct(answer_id): success = 'incorrect' - event_info['correct_map']=correct_map + event_info['correct_map']=correct_map.get_dict() # log this in the tracker event_info['success']=success - self.tracker('save_problem_check', event_info) try: - html = self.get_problem_html(encapsulate=False) + html = self.get_problem_html(encapsulate=False) # render problem into HTML except Exception,err: log.error('failed to generate html') raise Exception,err @@ -430,17 +430,10 @@ class Module(XModule): self.tracker('reset_problem_fail', event_info) return "Refresh the page and make an attempt before resetting." - self.lcp.done=False - self.lcp.answers=dict() - self.lcp.correct_map=dict() - self.lcp.student_answers = dict() - - + self.lcp.do_reset() # call method in LoncapaProblem to reset itself if self.rerandomize == "always": - self.lcp.context=dict() - self.lcp.questions=dict() # Detailed info about questions in problem instance. TODO: Should be by id and not lid. - self.lcp.seed=None - + self.lcp.seed=None # reset random number generator seed (note the self.lcp.get_state() in next line) + self.lcp=LoncapaProblem(self.filestore.open(self.filename), self.item_id, self.lcp.get_state(), system=self.system) event_info['new_state']=self.lcp.get_state() From 5ac13e03aa2bcfb6b798b2730f0e9b7d7c27ad8d Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 10 Jun 2012 17:17:57 -0400 Subject: [PATCH 18/69] fourth pass in capa cleanup: - Added hints + hintmethod - hintgroup compatible with loncapa spec - also does hintfn for custom hints (can do answer history) - GenericResponse -> LoncapaResponse - moved response type tags into responsetype classes - capa_problem should use __future__ division - hints stored in CorrectMap, copied to 'feedback' in SimpleInput for display --- common/lib/capa/capa_problem.py | 40 +++--- common/lib/capa/correctmap.py | 32 ++++- common/lib/capa/inputtypes.py | 55 +++++--- common/lib/capa/responsetypes.py | 226 +++++++++++++++++++++++++------ common/lib/capa/util.py | 5 + 5 files changed, 272 insertions(+), 86 deletions(-) diff --git a/common/lib/capa/capa_problem.py b/common/lib/capa/capa_problem.py index 93d5620aae..b14001ef03 100644 --- a/common/lib/capa/capa_problem.py +++ b/common/lib/capa/capa_problem.py @@ -12,6 +12,8 @@ Main module which shows problems (of "capa" type). This is used by capa_module. ''' +from __future__ import division + import copy import logging import math @@ -32,20 +34,10 @@ import inputtypes from util import contextualize_text # to be replaced with auto-registering -from responsetypes import NumericalResponse, FormulaResponse, CustomResponse, SchematicResponse, MultipleChoiceResponse, TrueFalseResponse, ExternalResponse, ImageResponse, OptionResponse, SymbolicResponse +import responsetypes # dict of tagname, Response Class -- this should come from auto-registering -response_types = {'numericalresponse': NumericalResponse, - 'formularesponse': FormulaResponse, - 'customresponse': CustomResponse, - 'schematicresponse': SchematicResponse, - 'externalresponse': ExternalResponse, - 'multiplechoiceresponse': MultipleChoiceResponse, - 'truefalseresponse': TrueFalseResponse, - 'imageresponse': ImageResponse, - 'optionresponse': OptionResponse, - 'symbolicresponse': SymbolicResponse, - } +response_tag_dict = dict([(x.response_tag,x) for x in responsetypes.__all__]) entry_types = ['textline', 'schematic', 'choicegroup', 'textbox', 'imageinput', 'optioninput'] solution_types = ['solution'] # extra things displayed after "show answers" is pressed @@ -65,7 +57,7 @@ global_context = {'random': random, 'eia': eia} # These should be removed from HTML output, including all subelements -html_problem_semantics = ["responseparam", "answer", "script"] +html_problem_semantics = ["responseparam", "answer", "script","hintgroup"] #log = logging.getLogger(__name__) log = logging.getLogger('mitx.common.lib.capa.capa_problem') @@ -209,7 +201,7 @@ class LoncapaProblem(object): oldcmap = self.correct_map # old CorrectMap newcmap = CorrectMap() # start new with empty CorrectMap for responder in self.responders.values(): - results = responder.get_score(answers,oldcmap) # call the responsetype instance to do the actual grading + results = responder.evaluate_answers(answers,oldcmap) # call the responsetype instance to do the actual grading newcmap.update(results) self.correct_map = newcmap log.debug('%s: in grade_answers, answers=%s, cmap=%s' % (self,answers,newcmap)) @@ -248,7 +240,8 @@ class LoncapaProblem(object): ''' return contextualize_text(etree.tostring(self.extract_html(self.tree)), self.context) - # ======= Private ======== + # ======= Private Methods Below ======== + def extract_context(self, tree, seed=struct.unpack('i', os.urandom(4))[0]): # private ''' Extract content of from the problem.xml file, and exec it in the @@ -296,15 +289,17 @@ class LoncapaProblem(object): problemid = problemtree.get('id') # my ID if problemtree.tag in inputtypes.get_input_xml_tags(): - # status is currently the answer for the problem ID for the input element, - # but it will turn into a dict containing both the answer and any associated message - # for the problem ID for the input element. + status = "unsubmitted" msg = '' + hint = '' + hintmode = None if problemid in self.correct_map: pid = problemtree.get('id') status = self.correct_map.get_correctness(pid) msg = self.correct_map.get_msg(pid) + hint = self.correct_map.get_hint(pid) + hintmode = self.correct_map.get_hintmode(pid) value = "" if self.student_answers and problemid in self.student_answers: @@ -316,7 +311,10 @@ class LoncapaProblem(object): state={'value': value, 'status': status, 'id': problemtree.get('id'), - 'feedback': {'message': msg} + 'feedback': {'message': msg, + 'hint' : hint, + 'hintmode' : hintmode, + } }, use='capa_input') return render_object.get_html() # function(problemtree, value, status, msg) # render the special response (textline, schematic,...) @@ -352,7 +350,7 @@ class LoncapaProblem(object): ''' response_id = 1 self.responders = {} - for response in tree.xpath('//' + "|//".join(response_types)): + for response in tree.xpath('//' + "|//".join(response_tag_dict)): response_id_str = self.problem_id + "_" + str(response_id) response.set('id',response_id_str) # create and save ID for this response response_id += 1 @@ -366,7 +364,7 @@ class LoncapaProblem(object): entry.attrib['id'] = "%s_%i_%i" % (self.problem_id, response_id, answer_id) answer_id = answer_id + 1 - responder = response_types[response.tag](response, inputfields, self.context, self.system) # instantiate capa Response + responder = response_tag_dict[response.tag](response, inputfields, self.context, self.system) # instantiate capa Response self.responders[response] = responder # save in list in self # ... may not be associated with any specific response; give IDs for those separately diff --git a/common/lib/capa/correctmap.py b/common/lib/capa/correctmap.py index 3eac98cc3a..f694391cc6 100644 --- a/common/lib/capa/correctmap.py +++ b/common/lib/capa/correctmap.py @@ -5,7 +5,16 @@ class CorrectMap(object): ''' - Stores (correctness, npoints, msg) for each answer_id. + Stores map between answer_id and response evaluation result for each question + in a capa problem. The response evaluation result for each answer_id includes + (correctness, npoints, msg, hint, hintmode). + + - correctness : either 'correct' or 'incorrect' + - npoints : None, or integer specifying number of points awarded for this answer_id + - 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 + Behaves as a dict. ''' cmap = {} @@ -13,11 +22,14 @@ class CorrectMap(object): def __init__(self,*args,**kwargs): self.set(*args,**kwargs) - def set(self,answer_id=None,correctness=None,npoints=None,msg=''): + def set(self, answer_id=None, correctness=None, npoints=None, msg='', hint='', hintmode=None): if answer_id is not None: self.cmap[answer_id] = {'correctness': correctness, 'npoints': npoints, - 'msg': msg } + 'msg': msg, + 'hint' : hint, + 'hintmode' : hintmode, + } def __repr__(self): return repr(self.cmap) @@ -64,6 +76,20 @@ class CorrectMap(object): def get_msg(self,answer_id): return self.get_property(answer_id,'msg','') + def get_hint(self,answer_id): + return self.get_property(answer_id,'hint','') + + def get_hintmode(self,answer_id): + return self.get_property(answer_id,'hintmode',None) + + def set_hint_and_mode(self,answer_id,hint,hintmode): + ''' + - hint : (string) HTML text for hint + - hintmode : (string) mode for hint display ('always' or 'on_request') + ''' + self.set_property(answer_id,'hint',hint) + self.set_property(answer_id,'hintmode',hintmode) + def update(self,other_cmap): ''' Update this CorrectMap with the contents of another CorrectMap diff --git a/common/lib/capa/inputtypes.py b/common/lib/capa/inputtypes.py index 10fbdb7f98..1fa51f2f84 100644 --- a/common/lib/capa/inputtypes.py +++ b/common/lib/capa/inputtypes.py @@ -32,44 +32,57 @@ def get_input_xml_tags(): return SimpleInput.get_xml_tags() class SimpleInput():# XModule - ''' Type for simple inputs -- plain HTML with a form element - - State is a dictionary with optional keys: - * Value - * ID - * Status (answered, unanswered, unsubmitted) - * Feedback (dictionary containing keys for hints, errors, or other - feedback from previous attempt) - + ''' + Type for simple inputs -- plain HTML with a form element ''' xml_tags = {} ## Maps tags to functions def __init__(self, system, xml, item_id = None, track_url=None, state=None, use = 'capa_input'): + ''' + Instantiate a SimpleInput class. Arguments: + + - system : I4xSystem instance which provides OS, rendering, and user context + - xml : Element tree of this Input element + - item_id : id for this input element (assigned by capa_problem.LoncapProblem) - string + - track_url : URL used for tracking - string + - state : a dictionary with optional keys: + * Value + * ID + * Status (answered, unanswered, unsubmitted) + * Feedback (dictionary containing keys for hints, errors, or other + feedback from previous attempt) + - use : + ''' + self.xml = xml self.tag = xml.tag - if not state: - state = {} + self.system = system + if not state: state = {} + ## ID should only come from one place. ## If it comes from multiple, we use state first, XML second, and parameter ## third. Since we don't make this guarantee, we can swap this around in ## the future if there's a more logical order. - if item_id: - self.id = item_id - if xml.get('id'): - self.id = xml.get('id') - if 'id' in state: - self.id = state['id'] - self.system = system + if item_id: self.id = item_id + if xml.get('id'): self.id = xml.get('id') + if 'id' in state: self.id = state['id'] self.value = '' if 'value' in state: self.value = state['value'] self.msg = '' - if 'feedback' in state and 'message' in state['feedback']: - self.msg = state['feedback']['message'] - + feedback = state.get('feedback') + if feedback is not None: + self.msg = feedback.get('message','') + self.hint = feedback.get('hint','') + self.hintmode = feedback.get('hintmode',None) + + # put hint above msg if to be displayed + if self.hintmode == 'always': + self.msg = self.hint + ('
' if self.msg else '') + self.msg + self.status = 'unanswered' if 'status' in state: self.status = state['status'] diff --git a/common/lib/capa/responsetypes.py b/common/lib/capa/responsetypes.py index 2de9e27893..5a5296d805 100644 --- a/common/lib/capa/responsetypes.py +++ b/common/lib/capa/responsetypes.py @@ -51,7 +51,7 @@ class StudentInputError(Exception): # # Main base class for CAPA responsetypes -class GenericResponse(object): +class LoncapaResponse(object): ''' Base class for CAPA responsetypes. Each response type (ie a capa question, which is part of a capa problem) is represented as a subclass, @@ -60,22 +60,31 @@ class GenericResponse(object): - get_score : evaluate the given student answers, and return a CorrectMap - get_answers : provide a dict of the expected answers for this problem + Each subclass must also define the following attributes: + + - response_tag : xhtml tag identifying this response (used in auto-registering) + In addition, these methods are optional: - - get_max_score : if defined, this is called to obtain the maximum score possible for this question - - setup_response : find and note the answer input field IDs for the response; called by __init__ - - render_html : render this Response as HTML (must return XHTML compliant string) - - __unicode__ : unicode representation of this Response + - get_max_score : if defined, this is called to obtain the maximum score possible for this question + - setup_response : find and note the answer input field IDs for the response; called by __init__ + - check_hint_condition : check to see if the student's answers satisfy a particular condition for a hint to be displayed + - render_html : render this Response as HTML (must return XHTML compliant string) + - __unicode__ : unicode representation of this Response Each response type may also specify the following attributes: - - max_inputfields : (int) maximum number of answer input fields (checked in __init__ if not None) - - allowed_inputfields : list of allowed input fields (each a string) for this Response - - required_attributes : list of required attributes (each a string) on the main response XML stanza + - max_inputfields : (int) maximum number of answer input fields (checked in __init__ if not None) + - allowed_inputfields : list of allowed input fields (each a string) for this Response + - required_attributes : list of required attributes (each a string) on the main response XML stanza + - hint_tag : xhtml tag identifying hint associated with this response inside hintgroup ''' __metaclass__=abc.ABCMeta # abc = Abstract Base Class + response_tag = None + hint_tag = None + max_inputfields = None allowed_inputfields = [] required_attributes = [] @@ -85,7 +94,7 @@ class GenericResponse(object): Init is passed the following arguments: - xml : ElementTree of this Response - - inputfields : list of ElementTrees for each input entry field in this Response + - inputfields : ordered list of ElementTrees for each input entry field in this Response - context : script processor context - system : I4xSystem instance which provides OS, rendering, and user context @@ -112,7 +121,7 @@ class GenericResponse(object): msg += "\nSee XML source line %s" % getattr(xml,'sourceline','') raise LoncapaProblemError(msg) - self.answer_ids = [x.get('id') for x in self.inputfields] + self.answer_ids = [x.get('id') for x in self.inputfields] # ordered list of answer_id values for this response if self.max_inputfields==1: self.answer_id = self.answer_ids[0] # for convenience @@ -140,8 +149,85 @@ class GenericResponse(object): tree.tail = self.xml.tail return tree + def evaluate_answers(self,student_answers,old_cmap): + ''' + Called by capa_problem.LoncapaProblem to evaluate student answers, and to + generate hints (if any). + + Returns the new CorrectMap, with (correctness,msg,hint,hintmode) for each answer_id. + ''' + new_cmap = self.get_score(student_answers) + self.get_hints(student_answers, new_cmap, old_cmap) + return new_cmap + + def get_hints(self, student_answers, new_cmap, old_cmap): + ''' + Generate adaptive hints for this problem based on student answers, the old CorrectMap, + and the new CorrectMap produced by get_score. + + Does not return anything. + + Modifies new_cmap, by adding hints to answer_id entries as appropriate. + ''' + hintgroup = self.xml.find('hintgroup') + if hintgroup is None: return + + # hint specified by function? + hintfn = hintgroup.get('hintfn') + if hintfn: + ''' + Hint is determined by a function defined in the @@ -358,6 +463,7 @@ def sympy_check2(): '''}] + response_tag = 'customresponse' allowed_inputfields = ['textline','textbox'] def setup_response(self): @@ -402,7 +508,7 @@ def sympy_check2(): else: self.code = answer.text - def get_score(self, student_answers, old_cmap): + def get_score(self, student_answers): ''' student_answers is a dict with everything from request.POST, but with the first part of each key removed (the string before the first "_"). @@ -540,6 +646,8 @@ class SymbolicResponse(CustomResponse): '''}] + response_tag = 'symbolicresponse' + def setup_response(self): self.xml.set('cfn','symmath_check') code = "from symmath import *" @@ -548,7 +656,7 @@ class SymbolicResponse(CustomResponse): #----------------------------------------------------------------------------- -class ExternalResponse(GenericResponse): +class ExternalResponse(LoncapaResponse): ''' Grade the students input using an external server. @@ -594,6 +702,7 @@ main() '''}] + response_tag = 'externalresponse' allowed_inputfields = ['textline','textbox'] def setup_response(self): @@ -647,7 +756,7 @@ main() return rxml - def get_score(self, student_answers, old_cmap): + def get_score(self, student_answers): idset = sorted(self.answer_ids) cmap = CorrectMap() try: @@ -707,7 +816,7 @@ main() #----------------------------------------------------------------------------- -class FormulaResponse(GenericResponse): +class FormulaResponse(LoncapaResponse): ''' Checking of symbolic math response using numerical sampling. ''' @@ -729,6 +838,8 @@ class FormulaResponse(GenericResponse): '''}] + response_tag = 'formularesponse' + hint_tag = 'formulahint' allowed_inputfields = ['textline'] required_attributes = ['answer'] max_inputfields = 1 @@ -743,7 +854,7 @@ class FormulaResponse(GenericResponse): id=xml.get('id'))[0] self.tolerance = contextualize_text(self.tolerance_xml, context) except Exception: - self.tolerance = 0 + self.tolerance = '0.00001' ts = xml.get('type') if ts is None: @@ -757,11 +868,16 @@ class FormulaResponse(GenericResponse): else: # Default self.case_sensitive = False - def get_score(self, student_answers, old_cmap): - variables=self.samples.split('@')[0].split(',') - numsamples=int(self.samples.split('@')[1].split('#')[1]) + def get_score(self, student_answers): + given = student_answers[self.answer_id] + correctness = self.check_formula(self.correct_answer, given, self.samples) + return CorrectMap(self.answer_id, correctness) + + def check_formula(self,expected, given, samples): + variables=samples.split('@')[0].split(',') + numsamples=int(samples.split('@')[1].split('#')[1]) sranges=zip(*map(lambda x:map(float, x.split(",")), - self.samples.split('@')[1].split('#')[0].split(':'))) + samples.split('@')[1].split('#')[0].split(':'))) ranges=dict(zip(variables, sranges)) for i in range(numsamples): @@ -771,23 +887,26 @@ class FormulaResponse(GenericResponse): value = random.uniform(*ranges[var]) instructor_variables[str(var)] = value student_variables[str(var)] = value - instructor_result = evaluator(instructor_variables,dict(),self.correct_answer, cs = self.case_sensitive) + #log.debug('formula: instructor_vars=%s, expected=%s' % (instructor_variables,expected)) + instructor_result = evaluator(instructor_variables,dict(),expected, cs = self.case_sensitive) try: - #print student_variables,dict(),student_answers[self.answer_id] - student_result = evaluator(student_variables,dict(), - student_answers[self.answer_id], + #log.debug('formula: student_vars=%s, given=%s' % (student_variables,given)) + student_result = evaluator(student_variables, + dict(), + given, cs = self.case_sensitive) except UndefinedVariable as uv: + log.debbug('formularesponse: undefined variable in given=%s' % given) raise StudentInputError(uv.message+" not permitted in answer") - except: + except Exception, err: #traceback.print_exc() + log.debug('formularesponse: error %s in formula' % err) raise StudentInputError("Error in formula") if numpy.isnan(student_result) or numpy.isinf(student_result): - return CorrectMap(self.answer_id, "incorrect") + return "incorrect" if not compare_with_tolerance(student_result, instructor_result, self.tolerance): - return CorrectMap(self.answer_id, "incorrect") - - return CorrectMap(self.answer_id, "correct") + return "incorrect" + return "correct" def strip_dict(self, d): ''' Takes a dict. Returns an identical dict, with all non-word @@ -799,13 +918,30 @@ class FormulaResponse(GenericResponse): isinstance(d[k], numbers.Number)]) return d + def check_hint_condition(self,hxml_set,student_answers): + given = student_answers[self.answer_id] + hints_to_show = [] + for hxml in hxml_set: + samples = hxml.get('samples') + name = hxml.get('name') + correct_answer = contextualize_text(hxml.get('answer'),self.context) + try: + correctness = self.check_formula(correct_answer, given, samples) + except Exception,err: + correctness = 'incorrect' + if correctness=='correct': + hints_to_show.append(name) + log.debug('hints_to_show = %s' % hints_to_show) + return hints_to_show + def get_answers(self): return {self.answer_id:self.correct_answer} #----------------------------------------------------------------------------- -class SchematicResponse(GenericResponse): +class SchematicResponse(LoncapaResponse): + response_tag = 'schematicresponse' allowed_inputfields = ['schematic'] def setup_response(self): @@ -817,7 +953,7 @@ class SchematicResponse(GenericResponse): else: self.code = answer.text - def get_score(self, student_answers, old_cmap): + def get_score(self, student_answers): from capa_problem import global_context submission = [json.loads(student_answers[k]) for k in sorted(self.answer_ids)] self.context.update({'submission':submission}) @@ -831,7 +967,7 @@ class SchematicResponse(GenericResponse): #----------------------------------------------------------------------------- -class ImageResponse(GenericResponse): +class ImageResponse(LoncapaResponse): """ Handle student response for image input: the input is a click on an image, which produces an [x,y] coordinate pair. The click is correct if it falls @@ -847,13 +983,14 @@ class ImageResponse(GenericResponse): '''}] + response_tag = 'imageresponse' allowed_inputfields = ['imageinput'] def setup_response(self): self.ielements = self.inputfields self.answer_ids = [ie.get('id') for ie in self.ielements] - def get_score(self, student_answers, old_cmap): + def get_score(self, student_answers): correct_map = CorrectMap() expectedset = self.get_answers() @@ -884,3 +1021,10 @@ class ImageResponse(GenericResponse): def get_answers(self): return dict([(ie.get('id'),ie.get('rectangle')) for ie in self.ielements]) + +#----------------------------------------------------------------------------- +# TEMPORARY: List of all response subclasses +# FIXME: To be replaced by auto-registration + +__all__ = [ NumericalResponse, FormulaResponse, CustomResponse, SchematicResponse, MultipleChoiceResponse, TrueFalseResponse, ExternalResponse, ImageResponse, OptionResponse, SymbolicResponse ] + diff --git a/common/lib/capa/util.py b/common/lib/capa/util.py index 996f6c8dac..f1cc8f859e 100644 --- a/common/lib/capa/util.py +++ b/common/lib/capa/util.py @@ -7,6 +7,11 @@ from calc import evaluator, UndefinedVariable def compare_with_tolerance(v1, v2, tol): ''' Compare v1 to v2 with maximum tolerance tol tol is relative if it ends in %; otherwise, it is absolute + + - v1 : student result (number) + - v2 : instructor result (number) + - tol : tolerance (string or number) + ''' relative = "%" in tol if relative: From 5eda2f3a63bd6fe5edbb2772af94ecd435ed6cbb Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 10 Jun 2012 18:41:54 -0400 Subject: [PATCH 19/69] bugfixes - correctmap should reinit self.cmap on init --- common/lib/capa/capa_problem.py | 1 + common/lib/capa/correctmap.py | 16 +++++++++------- common/lib/capa/responsetypes.py | 5 +++-- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/common/lib/capa/capa_problem.py b/common/lib/capa/capa_problem.py index b14001ef03..a341f954bc 100644 --- a/common/lib/capa/capa_problem.py +++ b/common/lib/capa/capa_problem.py @@ -200,6 +200,7 @@ class LoncapaProblem(object): self.student_answers = answers oldcmap = self.correct_map # old CorrectMap newcmap = CorrectMap() # start new with empty CorrectMap + log.debug('Responders: %s' % self.responders) for responder in self.responders.values(): results = responder.evaluate_answers(answers,oldcmap) # call the responsetype instance to do the actual grading newcmap.update(results) diff --git a/common/lib/capa/correctmap.py b/common/lib/capa/correctmap.py index f694391cc6..ec3fed54c2 100644 --- a/common/lib/capa/correctmap.py +++ b/common/lib/capa/correctmap.py @@ -17,11 +17,17 @@ class CorrectMap(object): Behaves as a dict. ''' - cmap = {} - def __init__(self,*args,**kwargs): + self.cmap = dict() # start with empty dict + self.__getitem__ = self.cmap.__getitem__ + self.__iter__ = self.cmap.__iter__ + self.items = self.cmap.items + self.keys = self.cmap.keys self.set(*args,**kwargs) + def __iter__(self): + return self.cmap.__iter__() + def set(self, answer_id=None, correctness=None, npoints=None, msg='', hint='', hintmode=None): if answer_id is not None: self.cmap[answer_id] = {'correctness': correctness, @@ -47,7 +53,7 @@ class CorrectMap(object): dict of dicts format. ''' if correct_map and not (type(correct_map[correct_map.keys()[0]])==dict): - for k in self.cmap.keys(): self.cmap.pop(k) # empty current dict + 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 @@ -98,9 +104,5 @@ class CorrectMap(object): raise Exception('CorrectMap.update called with invalid argument %s' % other_cmap) self.cmap.update(other_cmap.get_dict()) - __getitem__ = cmap.__getitem__ - __iter__ = cmap.__iter__ - items = cmap.items - keys = cmap.keys diff --git a/common/lib/capa/responsetypes.py b/common/lib/capa/responsetypes.py index 5a5296d805..a63cb991cf 100644 --- a/common/lib/capa/responsetypes.py +++ b/common/lib/capa/responsetypes.py @@ -158,6 +158,7 @@ class LoncapaResponse(object): ''' new_cmap = self.get_score(student_answers) self.get_hints(student_answers, new_cmap, old_cmap) + # log.debug('new_cmap = %s' % new_cmap) return new_cmap def get_hints(self, student_answers, new_cmap, old_cmap): @@ -492,7 +493,7 @@ def sympy_check2(): if cfn in self.context: self.code = self.context[cfn] else: - msg = "%s: can't find cfn in context = %s" % (unicode(self),self.context) + msg = "%s: can't find cfn %s in context" % (unicode(self),cfn) msg += "\nSee XML source line %s" % getattr(self.xml,'sourceline','') raise LoncapaProblemError(msg) @@ -896,7 +897,7 @@ class FormulaResponse(LoncapaResponse): given, cs = self.case_sensitive) except UndefinedVariable as uv: - log.debbug('formularesponse: undefined variable in given=%s' % given) + log.debug('formularesponse: undefined variable in given=%s' % given) raise StudentInputError(uv.message+" not permitted in answer") except Exception, err: #traceback.print_exc() From 58e359e7ec80e0e8e4609070ddc2393725a1d5d3 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 10 Jun 2012 20:04:30 -0400 Subject: [PATCH 20/69] problem.coffee change: show answer -> also show solution_* & do mathjax typeset --- lms/static/coffee/src/modules/problem.coffee | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lms/static/coffee/src/modules/problem.coffee b/lms/static/coffee/src/modules/problem.coffee index e1e062e949..1f17d01405 100644 --- a/lms/static/coffee/src/modules/problem.coffee +++ b/lms/static/coffee/src/modules/problem.coffee @@ -48,11 +48,15 @@ class @Problem @$("label[for='input_#{key}_#{choice}']").attr correct_answer: 'true' else - @$("#answer_#{key}").html(value) // needs to be html, not text, for complex solutions (eg coding) + @$("#answer_#{key}").html(value) # needs to be html, not text, for complex solutions (eg coding) + @$("#solution_#{key}").html(value) # needs to be html, not text, for complex solutions (eg coding) + MathJax.Hub.Queue ["Typeset", MathJax.Hub] + MathJax.Hub.Queue ["Typeset", MathJax.Hub] @$('.show').val 'Hide Answer' @element.addClass 'showed' else @$('[id^=answer_]').text '' + @$('[id^=solution_]').text '' @$('[correct_answer]').attr correct_answer: null @element.removeClass 'showed' @$('.show').val 'Show Answer' From 989a74ba3f67bcb8b81bf93b99f4e0a71bffd633 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 10 Jun 2012 20:05:33 -0400 Subject: [PATCH 21/69] django pipeline working now (with new pip -e git+git...) --- common/lib/capa/capa_problem.py | 1 + lms/envs/dev_ike.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/common/lib/capa/capa_problem.py b/common/lib/capa/capa_problem.py index a341f954bc..16ecb9186a 100644 --- a/common/lib/capa/capa_problem.py +++ b/common/lib/capa/capa_problem.py @@ -224,6 +224,7 @@ class LoncapaProblem(object): answer = etree.tostring(entry) if answer: answer_map[entry.get('id')] = answer + log.debug('answer_map = %s' % answer_map) return answer_map def get_answer_ids(self): diff --git a/lms/envs/dev_ike.py b/lms/envs/dev_ike.py index ee5b6e831b..0930ef4651 100644 --- a/lms/envs/dev_ike.py +++ b/lms/envs/dev_ike.py @@ -27,7 +27,7 @@ DEBUG = True ENABLE_MULTICOURSE = True # set to False to disable multicourse display (see lib.util.views.mitxhome) QUICKEDIT = True -MITX_FEATURES['USE_DJANGO_PIPELINE'] = False +# MITX_FEATURES['USE_DJANGO_PIPELINE'] = False COURSE_SETTINGS = {'6.002_Spring_2012': {'number' : '6.002x', 'title' : 'Circuits and Electronics', From 2af525f19c3489905d1f57e8fc77724aee1a423c Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 10 Jun 2012 20:52:10 -0400 Subject: [PATCH 22/69] fixes to schematicresponse to work with new CorrectMap --- common/lib/capa/responsetypes.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/lib/capa/responsetypes.py b/common/lib/capa/responsetypes.py index a63cb991cf..947a66b13c 100644 --- a/common/lib/capa/responsetypes.py +++ b/common/lib/capa/responsetypes.py @@ -960,7 +960,8 @@ class SchematicResponse(LoncapaResponse): self.context.update({'submission':submission}) exec self.code in global_context, self.context cmap = CorrectMap() - return cmap.set_dict(zip(sorted(self.answer_ids), self.context['correct'])) + cmap.set_dict(dict(zip(sorted(self.answer_ids), self.context['correct']))) + return cmap def get_answers(self): # use answers provided in input elements From f4a3c54481c0d6ba7cca09d72dd09b66c6c6d6ec Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 10 Jun 2012 21:05:21 -0400 Subject: [PATCH 23/69] fix xmodule/capa tests to use new CorrectMap --- common/lib/capa/capa_problem.py | 4 ++-- common/lib/xmodule/tests.py | 34 ++++++++++++++++----------------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/common/lib/capa/capa_problem.py b/common/lib/capa/capa_problem.py index 16ecb9186a..80ec8806f3 100644 --- a/common/lib/capa/capa_problem.py +++ b/common/lib/capa/capa_problem.py @@ -200,12 +200,12 @@ class LoncapaProblem(object): self.student_answers = answers oldcmap = self.correct_map # old CorrectMap newcmap = CorrectMap() # start new with empty CorrectMap - log.debug('Responders: %s' % self.responders) + # log.debug('Responders: %s' % self.responders) for responder in self.responders.values(): results = responder.evaluate_answers(answers,oldcmap) # call the responsetype instance to do the actual grading newcmap.update(results) self.correct_map = newcmap - log.debug('%s: in grade_answers, answers=%s, cmap=%s' % (self,answers,newcmap)) + # log.debug('%s: in grade_answers, answers=%s, cmap=%s' % (self,answers,newcmap)) return newcmap def get_question_answers(self): diff --git a/common/lib/xmodule/tests.py b/common/lib/xmodule/tests.py index 69e69aa1d9..f8187269e9 100644 --- a/common/lib/xmodule/tests.py +++ b/common/lib/xmodule/tests.py @@ -1,9 +1,9 @@ # -# unittests for courseware +# unittests for xmodule (and capa) # # Note: run this using a like like this: # -# django-admin.py test --settings=envs.test_ike --pythonpath=. courseware +# django-admin.py test --settings=lms.envs.test_ike --pythonpath=. common/lib/xmodule import unittest import os @@ -96,31 +96,31 @@ class MultiChoiceTest(unittest.TestCase): multichoice_file = os.path.dirname(__file__)+"/test_files/multichoice.xml" test_lcp = lcp.LoncapaProblem(open(multichoice_file), '1', system=i4xs) correct_answers = {'1_2_1':'choice_foil3'} - self.assertEquals(test_lcp.grade_answers(correct_answers)['1_2_1'], 'correct') + self.assertEquals(test_lcp.grade_answers(correct_answers).get_correctness('1_2_1'), 'correct') false_answers = {'1_2_1':'choice_foil2'} - self.assertEquals(test_lcp.grade_answers(false_answers)['1_2_1'], 'incorrect') + self.assertEquals(test_lcp.grade_answers(false_answers).get_correctness('1_2_1'), 'incorrect') def test_MC_bare_grades(self): multichoice_file = os.path.dirname(__file__)+"/test_files/multi_bare.xml" test_lcp = lcp.LoncapaProblem(open(multichoice_file), '1', system=i4xs) correct_answers = {'1_2_1':'choice_2'} - self.assertEquals(test_lcp.grade_answers(correct_answers)['1_2_1'], 'correct') + self.assertEquals(test_lcp.grade_answers(correct_answers).get_correctness('1_2_1'), 'correct') false_answers = {'1_2_1':'choice_1'} - self.assertEquals(test_lcp.grade_answers(false_answers)['1_2_1'], 'incorrect') + self.assertEquals(test_lcp.grade_answers(false_answers).get_correctness('1_2_1'), 'incorrect') def test_TF_grade(self): truefalse_file = os.path.dirname(__file__)+"/test_files/truefalse.xml" test_lcp = lcp.LoncapaProblem(open(truefalse_file), '1', system=i4xs) correct_answers = {'1_2_1':['choice_foil2', 'choice_foil1']} - self.assertEquals(test_lcp.grade_answers(correct_answers)['1_2_1'], 'correct') + self.assertEquals(test_lcp.grade_answers(correct_answers).get_correctness('1_2_1'), 'correct') false_answers = {'1_2_1':['choice_foil1']} - self.assertEquals(test_lcp.grade_answers(false_answers)['1_2_1'], 'incorrect') + self.assertEquals(test_lcp.grade_answers(false_answers).get_correctness('1_2_1'), 'incorrect') false_answers = {'1_2_1':['choice_foil1', 'choice_foil3']} - self.assertEquals(test_lcp.grade_answers(false_answers)['1_2_1'], 'incorrect') + self.assertEquals(test_lcp.grade_answers(false_answers).get_correctness('1_2_1'), 'incorrect') false_answers = {'1_2_1':['choice_foil3']} - self.assertEquals(test_lcp.grade_answers(false_answers)['1_2_1'], 'incorrect') + self.assertEquals(test_lcp.grade_answers(false_answers).get_correctness('1_2_1'), 'incorrect') false_answers = {'1_2_1':['choice_foil1', 'choice_foil2', 'choice_foil3']} - self.assertEquals(test_lcp.grade_answers(false_answers)['1_2_1'], 'incorrect') + self.assertEquals(test_lcp.grade_answers(false_answers).get_correctness('1_2_1'), 'incorrect') class ImageResponseTest(unittest.TestCase): def test_ir_grade(self): @@ -131,8 +131,8 @@ class ImageResponseTest(unittest.TestCase): test_answers = {'1_2_1':'[500,20]', '1_2_2':'[250,300]', } - self.assertEquals(test_lcp.grade_answers(test_answers)['1_2_1'], 'correct') - self.assertEquals(test_lcp.grade_answers(test_answers)['1_2_2'], 'incorrect') + self.assertEquals(test_lcp.grade_answers(test_answers).get_correctness('1_2_1'), 'correct') + self.assertEquals(test_lcp.grade_answers(test_answers).get_correctness('1_2_2'), 'incorrect') class SymbolicResponseTest(unittest.TestCase): def test_sr_grade(self): @@ -220,8 +220,8 @@ class SymbolicResponseTest(unittest.TestCase): ''', } - self.assertEquals(test_lcp.grade_answers(correct_answers)['1_2_1'], 'correct') - self.assertEquals(test_lcp.grade_answers(wrong_answers)['1_2_1'], 'incorrect') + self.assertEquals(test_lcp.grade_answers(correct_answers).get_correctness('1_2_1'), 'correct') + self.assertEquals(test_lcp.grade_answers(wrong_answers).get_correctness('1_2_1'), 'incorrect') class OptionResponseTest(unittest.TestCase): ''' @@ -237,8 +237,8 @@ class OptionResponseTest(unittest.TestCase): test_answers = {'1_2_1':'True', '1_2_2':'True', } - self.assertEquals(test_lcp.grade_answers(test_answers)['1_2_1'], 'correct') - self.assertEquals(test_lcp.grade_answers(test_answers)['1_2_2'], 'incorrect') + self.assertEquals(test_lcp.grade_answers(test_answers).get_correctness('1_2_1'), 'correct') + self.assertEquals(test_lcp.grade_answers(test_answers).get_correctness('1_2_2'), 'incorrect') #----------------------------------------------------------------------------- # Grading tests From fdc4a14cf0a26a557e42342d0966d9feac604e08 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 10 Jun 2012 21:11:04 -0400 Subject: [PATCH 24/69] fix i4xs in tests.py; move symbolicresponse.xml test back to where it should be --- .../xmodule/test_files/{test_files => }/symbolicresponse.xml | 0 common/lib/xmodule/tests.py | 5 +++-- 2 files changed, 3 insertions(+), 2 deletions(-) rename common/lib/xmodule/test_files/{test_files => }/symbolicresponse.xml (100%) diff --git a/common/lib/xmodule/test_files/test_files/symbolicresponse.xml b/common/lib/xmodule/test_files/symbolicresponse.xml similarity index 100% rename from common/lib/xmodule/test_files/test_files/symbolicresponse.xml rename to common/lib/xmodule/test_files/symbolicresponse.xml diff --git a/common/lib/xmodule/tests.py b/common/lib/xmodule/tests.py index f8187269e9..00711ce0da 100644 --- a/common/lib/xmodule/tests.py +++ b/common/lib/xmodule/tests.py @@ -28,12 +28,13 @@ class I4xSystem(object): self.track_function = lambda x: None self.render_function = lambda x: {} # Probably incorrect self.exception404 = Exception + self.DEBUG = True def __repr__(self): return repr(self.__dict__) def __str__(self): return str(self.__dict__) -i4xs = I4xSystem +i4xs = I4xSystem() class ModelsTest(unittest.TestCase): def setUp(self): @@ -136,7 +137,7 @@ class ImageResponseTest(unittest.TestCase): class SymbolicResponseTest(unittest.TestCase): def test_sr_grade(self): - raise SkipTest() # This test fails due to dependencies on a local copy of snuggletex-webapp. Until we have figured that out, we'll just skip this test + # raise SkipTest() # This test fails due to dependencies on a local copy of snuggletex-webapp. Until we have figured that out, we'll just skip this test symbolicresponse_file = os.path.dirname(__file__)+"/test_files/symbolicresponse.xml" test_lcp = lcp.LoncapaProblem(open(symbolicresponse_file), '1', system=i4xs) correct_answers = {'1_2_1':'cos(theta)*[[1,0],[0,1]] + i*sin(theta)*[[0,1],[1,0]]', From a7d0f8322d5727b3085b95f96e61db46e6428f20 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 10 Jun 2012 21:11:43 -0400 Subject: [PATCH 25/69] SymbolicResponseTest works if snuggletex war running ; back to skipping it for now --- common/lib/xmodule/tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/tests.py b/common/lib/xmodule/tests.py index 00711ce0da..1ba423983c 100644 --- a/common/lib/xmodule/tests.py +++ b/common/lib/xmodule/tests.py @@ -137,7 +137,7 @@ class ImageResponseTest(unittest.TestCase): class SymbolicResponseTest(unittest.TestCase): def test_sr_grade(self): - # raise SkipTest() # This test fails due to dependencies on a local copy of snuggletex-webapp. Until we have figured that out, we'll just skip this test + raise SkipTest() # This test fails due to dependencies on a local copy of snuggletex-webapp. Until we have figured that out, we'll just skip this test symbolicresponse_file = os.path.dirname(__file__)+"/test_files/symbolicresponse.xml" test_lcp = lcp.LoncapaProblem(open(symbolicresponse_file), '1', system=i4xs) correct_answers = {'1_2_1':'cos(theta)*[[1,0],[0,1]] + i*sin(theta)*[[0,1],[1,0]]', From b48b33e65ecc22a22dfa03e211d0163943cb7562 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 10 Jun 2012 21:27:11 -0400 Subject: [PATCH 26/69] add FormulaResponseWithHintTest test --- common/lib/xmodule/tests.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/common/lib/xmodule/tests.py b/common/lib/xmodule/tests.py index 1ba423983c..6f054b4bfb 100644 --- a/common/lib/xmodule/tests.py +++ b/common/lib/xmodule/tests.py @@ -241,6 +241,21 @@ class OptionResponseTest(unittest.TestCase): self.assertEquals(test_lcp.grade_answers(test_answers).get_correctness('1_2_1'), 'correct') self.assertEquals(test_lcp.grade_answers(test_answers).get_correctness('1_2_2'), 'incorrect') +class FormulaResponseWithHintTest(unittest.TestCase): + ''' + Test Formula response problem with a hint + This problem also uses calc. + ''' + def test_or_grade(self): + problem_file = os.path.dirname(__file__)+"/test_files/formularesponse_with_hint.xml" + test_lcp = lcp.LoncapaProblem(open(problem_file), '1', system=i4xs) + correct_answers = {'1_2_1':'2.5*x-5.0'} + test_answers = {'1_2_1':'0.4*x-5.0'} + self.assertEquals(test_lcp.grade_answers(correct_answers).get_correctness('1_2_1'), 'correct') + cmap = test_lcp.grade_answers(test_answers) + self.assertEquals(cmap.get_correctness('1_2_1'), 'incorrect') + self.assertTrue('You have inverted' in cmap.get_hint('1_2_1')) + #----------------------------------------------------------------------------- # Grading tests From 855112f8e744520f7b3470112ea50541ed0c83ea Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 10 Jun 2012 21:59:29 -0400 Subject: [PATCH 27/69] added StringResponse (with hints) for hints, still to be done: numericalhint, optionhint no default hint processing done yet (ie hintmode = on_request) --- common/lib/capa/inputtypes.py | 4 ++++ common/lib/capa/responsetypes.py | 38 +++++++++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/common/lib/capa/inputtypes.py b/common/lib/capa/inputtypes.py index 1fa51f2f84..75588e8aea 100644 --- a/common/lib/capa/inputtypes.py +++ b/common/lib/capa/inputtypes.py @@ -176,6 +176,10 @@ def textline(element, value, status, render_template, msg=""): if element.get('math') or element.get('dojs'): # 'dojs' flag is temporary, for backwards compatibility with 8.02x return SimpleInput.xml_tags['textline_dynamath'](element,value,status,render_template,msg) eid=element.get('id') + if eid is None: + msg = 'textline has no id: it probably appears outside of a known response type' + msg += "\nSee problem XML source line %s" % getattr(element,'sourceline','') + raise Exception(msg) count = int(eid.split('_')[-2])-1 # HACK size = element.get('size') context = {'id':eid, 'value':value, 'state':status, 'count':count, 'size': size, 'msg': msg} diff --git a/common/lib/capa/responsetypes.py b/common/lib/capa/responsetypes.py index 947a66b13c..cf7310f92e 100644 --- a/common/lib/capa/responsetypes.py +++ b/common/lib/capa/responsetypes.py @@ -425,6 +425,42 @@ class NumericalResponse(LoncapaResponse): #----------------------------------------------------------------------------- +class StringResponse(LoncapaResponse): + + response_tag = 'stringresponse' + hint_tag = 'stringhint' + allowed_inputfields = ['textline'] + required_attributes = ['answer'] + max_inputfields = 1 + + def setup_response(self): + self.correct_answer = contextualize_text(self.xml.get('answer'), self.context).strip() + + def get_score(self, student_answers): + '''Grade a string response ''' + student_answer = student_answers[self.answer_id].strip() + correct = self.check_string(self.correct_answer,student_answer) + return CorrectMap(self.answer_id,'correct' if correct else 'incorrect') + + def check_string(self,expected,given): + if self.xml.get('type')=='ci': return given.lower() == expected.lower() + return given == expected + + def check_hint_condition(self,hxml_set,student_answers): + given = student_answers[self.answer_id].strip() + hints_to_show = [] + for hxml in hxml_set: + name = hxml.get('name') + correct_answer = contextualize_text(hxml.get('answer'),self.context).strip() + if self.check_string(correct_answer,given): hints_to_show.append(name) + log.debug('hints_to_show = %s' % hints_to_show) + return hints_to_show + + def get_answers(self): + return {self.answer_id:self.correct_answer} + +#----------------------------------------------------------------------------- + class CustomResponse(LoncapaResponse): ''' Custom response. The python code to be run should be in ... @@ -1028,5 +1064,5 @@ class ImageResponse(LoncapaResponse): # TEMPORARY: List of all response subclasses # FIXME: To be replaced by auto-registration -__all__ = [ NumericalResponse, FormulaResponse, CustomResponse, SchematicResponse, MultipleChoiceResponse, TrueFalseResponse, ExternalResponse, ImageResponse, OptionResponse, SymbolicResponse ] +__all__ = [ NumericalResponse, FormulaResponse, CustomResponse, SchematicResponse, MultipleChoiceResponse, TrueFalseResponse, ExternalResponse, ImageResponse, OptionResponse, SymbolicResponse, StringResponse ] From 4f6d9143c0fa30b70975b3de6dfcc364e404346f Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 10 Jun 2012 22:06:13 -0400 Subject: [PATCH 28/69] added stringresponse_with_hint test --- .../test_files/formularesponse_with_hint.xml | 45 +++++++++++++++++++ .../test_files/stringresponse_with_hint.xml | 25 +++++++++++ common/lib/xmodule/tests.py | 14 ++++++ 3 files changed, 84 insertions(+) create mode 100644 common/lib/xmodule/test_files/formularesponse_with_hint.xml create mode 100644 common/lib/xmodule/test_files/stringresponse_with_hint.xml diff --git a/common/lib/xmodule/test_files/formularesponse_with_hint.xml b/common/lib/xmodule/test_files/formularesponse_with_hint.xml new file mode 100644 index 0000000000..e5b3e28708 --- /dev/null +++ b/common/lib/xmodule/test_files/formularesponse_with_hint.xml @@ -0,0 +1,45 @@ + + + + +

Hints can be provided to students, based on the last response given, as well as the history of responses given. Here is an example of a hint produced by a Formula Response problem.

+ +

+What is the equation of the line which passess through ($x1,$y1) and +($x2,$y2)?

+ +

The correct answer is $answer. A common error is to invert the equation for the slope. Enter +$wrongans to see a hint.

+ + + + + + y = + + + + + You have inverted the slope in the question. + + + + + diff --git a/common/lib/xmodule/test_files/stringresponse_with_hint.xml b/common/lib/xmodule/test_files/stringresponse_with_hint.xml new file mode 100644 index 0000000000..86efdf0f18 --- /dev/null +++ b/common/lib/xmodule/test_files/stringresponse_with_hint.xml @@ -0,0 +1,25 @@ + +

Example: String Response Problem

+
+ + + Which US state has Lansing as its capital? + + + + + + + + + The state capital of Wisconsin is Madison. + + + The state capital of Minnesota is St. Paul. + + + The state you are looking for is also known as the 'Great Lakes State' + + + + diff --git a/common/lib/xmodule/tests.py b/common/lib/xmodule/tests.py index 6f054b4bfb..370b3befe5 100644 --- a/common/lib/xmodule/tests.py +++ b/common/lib/xmodule/tests.py @@ -256,6 +256,20 @@ class FormulaResponseWithHintTest(unittest.TestCase): self.assertEquals(cmap.get_correctness('1_2_1'), 'incorrect') self.assertTrue('You have inverted' in cmap.get_hint('1_2_1')) +class StringResponseWithHintTest(unittest.TestCase): + ''' + Test String response problem with a hint + ''' + def test_or_grade(self): + problem_file = os.path.dirname(__file__)+"/test_files/stringresponse_with_hint.xml" + test_lcp = lcp.LoncapaProblem(open(problem_file), '1', system=i4xs) + correct_answers = {'1_2_1':'Michigan'} + test_answers = {'1_2_1':'Minnesota'} + self.assertEquals(test_lcp.grade_answers(correct_answers).get_correctness('1_2_1'), 'correct') + cmap = test_lcp.grade_answers(test_answers) + self.assertEquals(cmap.get_correctness('1_2_1'), 'incorrect') + self.assertTrue('St. Paul' in cmap.get_hint('1_2_1')) + #----------------------------------------------------------------------------- # Grading tests From 6d444de05bf767f11f0662d6a64614b9a83c4aeb Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 10 Jun 2012 22:20:56 -0400 Subject: [PATCH 29/69] fix capa_problems pep8 --- common/lib/capa/capa_problem.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/common/lib/capa/capa_problem.py b/common/lib/capa/capa_problem.py index 80ec8806f3..eed6953521 100644 --- a/common/lib/capa/capa_problem.py +++ b/common/lib/capa/capa_problem.py @@ -146,7 +146,7 @@ class LoncapaProblem(object): def get_max_score(self): ''' Return maximum score for this problem. - We do this by counting the number of answers available for each question + We do this by counting the number of answers available for each question in the problem. If the Response for a question has a get_max_score() method then we call that and add its return value to the count. That can be used to give complex problems (eg programming questions) multiple points. @@ -351,7 +351,7 @@ class LoncapaProblem(object): Also create capa Response instances for each responsetype and save as self.responders ''' response_id = 1 - self.responders = {} + self.responders = {} for response in tree.xpath('//' + "|//".join(response_tag_dict)): response_id_str = self.problem_id + "_" + str(response_id) response.set('id',response_id_str) # create and save ID for this response @@ -367,7 +367,7 @@ class LoncapaProblem(object): answer_id = answer_id + 1 responder = response_tag_dict[response.tag](response, inputfields, self.context, self.system) # instantiate capa Response - self.responders[response] = responder # save in list in self + self.responders[response] = responder # save in list in self # ... may not be associated with any specific response; give IDs for those separately # TODO: We should make the namespaces consistent and unique (e.g. %s_problem_%i). From 6bcb40b52f79bb5f34091c66726d53f5c1793394 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 10 Jun 2012 22:27:40 -0400 Subject: [PATCH 30/69] capa_problem and responsetypes pep8 and pyflakes --- common/lib/capa/capa_problem.py | 5 ++--- common/lib/capa/responsetypes.py | 23 +++++++++++------------ 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/common/lib/capa/capa_problem.py b/common/lib/capa/capa_problem.py index eed6953521..516c72494b 100644 --- a/common/lib/capa/capa_problem.py +++ b/common/lib/capa/capa_problem.py @@ -14,7 +14,6 @@ This is used by capa_module. from __future__ import division -import copy import logging import math import numpy @@ -156,7 +155,7 @@ class LoncapaProblem(object): if hasattr(responder,'get_max_score'): try: maxscore += responder.get_max_score() - except Exception, err: + except Exception: log.error('responder %s failed to properly return from get_max_score()' % responder) raise else: @@ -176,7 +175,7 @@ class LoncapaProblem(object): for key in self.correct_map: try: correct += self.correct_map.get_npoints(key) - except Exception,err: + except Exception: log.error('key=%s, correct_map = %s' % (key,self.correct_map)) raise diff --git a/common/lib/capa/responsetypes.py b/common/lib/capa/responsetypes.py index cf7310f92e..32c1c6783c 100644 --- a/common/lib/capa/responsetypes.py +++ b/common/lib/capa/responsetypes.py @@ -505,7 +505,6 @@ def sympy_check2(): def setup_response(self): xml = self.xml - context = self.context # if has an "expect" (or "answer") attribute then save that self.expect = xml.get('expect') or xml.get('answer') @@ -560,7 +559,7 @@ def sympy_check2(): msg = '[courseware.capa.responsetypes.customresponse] error getting student answer from %s' % student_answers msg += '\n idset = %s, error = %s' % (idset,err) log.error(msg) - raise Exception,msg + raise Exception(msg) # global variable in context which holds the Presentation MathML from dynamic math input dynamath = [ student_answers.get(k+'_dynamath',None) for k in idset ] # ordered list of dynamath responses @@ -623,7 +622,7 @@ def sympy_check2(): log.error("oops in customresponse (cfn) error %s" % err) # print "context = ",self.context log.error(traceback.format_exc()) - raise Exception,"oops in customresponse (cfn) error %s" % err + raise Exception("oops in customresponse (cfn) error %s" % err) log.debug("[courseware.capa.responsetypes.customresponse.get_score] ret = %s" % ret) if type(ret)==dict: correct = ['correct']*len(idset) if ret['ok'] else ['incorrect']*len(idset) @@ -777,19 +776,19 @@ main() except Exception,err: msg = 'Error %s - cannot connect to external server url=%s' % (err,self.url) log.error(msg) - raise Exception, msg + raise Exception(msg) if self.system.DEBUG: log.info('response = %s' % r.text) if (not r.text ) or (not r.text.strip()): - raise Exception,'Error: no response from external server url=%s' % self.url + raise Exception('Error: no response from external server url=%s' % self.url) try: rxml = etree.fromstring(r.text) # response is XML; prase it except Exception,err: msg = 'Error %s - cannot parse response from external server r.text=%s' % (err,r.text) log.error(msg) - raise Exception, msg + raise Exception(msg) return rxml @@ -800,7 +799,7 @@ main() submission = [student_answers[k] for k in idset] except Exception,err: log.error('Error %s: cannot get student answer for %s; student_answers=%s' % (err,self.answer_ids,student_answers)) - raise Exception,err + raise Exception(err) self.context.update({'submission':submission}) @@ -817,7 +816,7 @@ main() ad = rxml.find('awarddetail').text admap = {'EXACT_ANS':'correct', # TODO: handle other loncapa responses - 'WRONG_FORMAT': 'incorrect', + 'WRONG_FORMAT': 'incorrect', } self.context['correct'] = ['correct'] if ad in admap: @@ -847,7 +846,7 @@ main() if not (len(exans)==len(self.answer_ids)): log.error('Expected %d answers from external server, only got %d!' % (len(self.answer_ids),len(exans))) - raise Exception,'Short response from external server' + raise Exception('Short response from external server') return dict(zip(self.answer_ids,exans)) @@ -964,7 +963,7 @@ class FormulaResponse(LoncapaResponse): correct_answer = contextualize_text(hxml.get('answer'),self.context) try: correctness = self.check_formula(correct_answer, given, samples) - except Exception,err: + except Exception: correctness = 'incorrect' if correctness=='correct': hints_to_show.append(name) @@ -1041,13 +1040,13 @@ class ImageResponse(LoncapaResponse): if not m: msg = 'Error in problem specification! cannot parse rectangle in %s' % (etree.tostring(self.ielements[aid], pretty_print=True)) - raise Exception,'[capamodule.capa.responsetypes.imageinput] '+msg + raise Exception('[capamodule.capa.responsetypes.imageinput] '+msg) (llx,lly,urx,ury) = [int(x) for x in m.groups()] # parse given answer m = re.match('\[([0-9]+),([0-9]+)]',given.strip().replace(' ','')) if not m: - raise Exception,'[capamodule.capa.responsetypes.imageinput] error grading %s (input=%s)' % (aid,given) + raise Exception('[capamodule.capa.responsetypes.imageinput] error grading %s (input=%s)' % (aid,given)) (gx,gy) = [int(x) for x in m.groups()] # answer is correct if (x,y) is within the specified rectangle From 6f14acee9e216399210e039a21c3730f90f58155 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 10 Jun 2012 22:34:30 -0400 Subject: [PATCH 31/69] add MITX_FEATURES['DISPLAY_HISTOGRAMS_TO_STAFF'] flag to settings --- lms/djangoapps/courseware/module_render.py | 2 +- lms/envs/common.py | 1 + lms/envs/dev_ike.py | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 0f82d9ba94..fd419a68dc 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -169,7 +169,7 @@ def render_x_module(user, request, xml_module, module_object_preload, position=N content = instance.get_html() # special extra information about each problem, only for users who are staff - if False and user.is_staff: + if settings.MITX_FEATURES.get('DISPLAY_HISTOGRAMS_TO_STAFF') and user.is_staff: module_id = xml_module.get('id') histogram = grade_histogram(module_id) render_histogram = len(histogram) > 0 diff --git a/lms/envs/common.py b/lms/envs/common.py index d583217dc5..430ffc25ab 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -37,6 +37,7 @@ PERFSTATS = False MITX_FEATURES = { 'SAMPLE' : False, 'USE_DJANGO_PIPELINE' : True, + 'DISPLAY_HISTOGRAMS_TO_STAFF' : True, } # Used for A/B testing diff --git a/lms/envs/dev_ike.py b/lms/envs/dev_ike.py index 0930ef4651..af51274433 100644 --- a/lms/envs/dev_ike.py +++ b/lms/envs/dev_ike.py @@ -28,6 +28,7 @@ ENABLE_MULTICOURSE = True # set to False to disable multicourse display (see QUICKEDIT = True # MITX_FEATURES['USE_DJANGO_PIPELINE'] = False +MITX_FEATURES['DISPLAY_HISTOGRAMS_TO_STAFF'] = False COURSE_SETTINGS = {'6.002_Spring_2012': {'number' : '6.002x', 'title' : 'Circuits and Electronics', From 17ca0e793ca77946b0a4212cc5a3550c04613ef3 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 10 Jun 2012 22:39:16 -0400 Subject: [PATCH 32/69] remove loncapa import in formularesponse_with_hint test (jenkins import path not consistent with dev machines?) --- common/lib/xmodule/test_files/formularesponse_with_hint.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/test_files/formularesponse_with_hint.xml b/common/lib/xmodule/test_files/formularesponse_with_hint.xml index e5b3e28708..90248dcf04 100644 --- a/common/lib/xmodule/test_files/formularesponse_with_hint.xml +++ b/common/lib/xmodule/test_files/formularesponse_with_hint.xml @@ -1,6 +1,6 @@ from the problem.xml file, and exec it in the context of this problem. Provides ability to randomize problems, and also set @@ -273,7 +273,7 @@ class LoncapaProblem(object): log.exception("Error while execing code: " + code) return context - def extract_html(self, problemtree): # private + def _extract_html(self, problemtree): # private ''' Main (private) function which converts Problem XML tree to HTML. Calls itself recursively. @@ -320,11 +320,11 @@ class LoncapaProblem(object): return render_object.get_html() # function(problemtree, value, status, msg) # render the special response (textline, schematic,...) if problemtree in self.responders: # let each Response render itself - return self.responders[problemtree].render_html(self.extract_html) + return self.responders[problemtree].render_html(self._extract_html) tree = etree.Element(problemtree.tag) for item in problemtree: - item_xhtml = self.extract_html(item) # nothing special: recurse + item_xhtml = self._extract_html(item) # nothing special: recurse if item_xhtml is not None: tree.append(item_xhtml) @@ -339,7 +339,7 @@ class LoncapaProblem(object): return tree - def preprocess_problem(self, tree): # private + def _preprocess_problem(self, tree): # private ''' Assign IDs to all the responses Assign sub-IDs to all entries (textline, schematic, etc.) From 5031c838ccdbefb27198e16f3dae9266006c558f Mon Sep 17 00:00:00 2001 From: ichuang Date: Tue, 12 Jun 2012 13:52:34 -0400 Subject: [PATCH 50/69] typo in correctmap --- common/lib/capa/correctmap.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/capa/correctmap.py b/common/lib/capa/correctmap.py index 63d4fb33b2..786b2f5e2d 100644 --- a/common/lib/capa/correctmap.py +++ b/common/lib/capa/correctmap.py @@ -24,7 +24,7 @@ class CorrectMap(object): self.set(*args,**kwargs) def __getitem__(self, *args, **kwargs): - return self.cmap.__getitem(*args, **kwargs) + return self.cmap.__getitem__(*args, **kwargs) def __iter__(self): return self.cmap.__iter__() From 35202817d5c102dc9786aea7c6942a05d214db0f Mon Sep 17 00:00:00 2001 From: ichuang Date: Tue, 12 Jun 2012 14:07:48 -0400 Subject: [PATCH 51/69] problem.coffee : queue mathjax typesetting just once --- lms/static/coffee/src/modules/problem.coffee | 1 - 1 file changed, 1 deletion(-) diff --git a/lms/static/coffee/src/modules/problem.coffee b/lms/static/coffee/src/modules/problem.coffee index 1f17d01405..17587b9ade 100644 --- a/lms/static/coffee/src/modules/problem.coffee +++ b/lms/static/coffee/src/modules/problem.coffee @@ -51,7 +51,6 @@ class @Problem @$("#answer_#{key}").html(value) # needs to be html, not text, for complex solutions (eg coding) @$("#solution_#{key}").html(value) # needs to be html, not text, for complex solutions (eg coding) MathJax.Hub.Queue ["Typeset", MathJax.Hub] - MathJax.Hub.Queue ["Typeset", MathJax.Hub] @$('.show').val 'Hide Answer' @element.addClass 'showed' else From 8e98e59cfa044364ed0c163339318f87269a6721 Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Tue, 12 Jun 2012 14:08:43 -0400 Subject: [PATCH 52/69] Cleanup show answer code --- lms/static/coffee/src/modules/problem.coffee | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/lms/static/coffee/src/modules/problem.coffee b/lms/static/coffee/src/modules/problem.coffee index 17587b9ade..aad41f23d4 100644 --- a/lms/static/coffee/src/modules/problem.coffee +++ b/lms/static/coffee/src/modules/problem.coffee @@ -45,17 +45,14 @@ class @Problem $.each response, (key, value) => if $.isArray(value) for choice in value - @$("label[for='input_#{key}_#{choice}']").attr - correct_answer: 'true' + @$("label[for='input_#{key}_#{choice}']").attr correct_answer: 'true' else - @$("#answer_#{key}").html(value) # needs to be html, not text, for complex solutions (eg coding) - @$("#solution_#{key}").html(value) # needs to be html, not text, for complex solutions (eg coding) + @$("#answer_#{key}, #solution_#{key}").html(value) MathJax.Hub.Queue ["Typeset", MathJax.Hub] @$('.show').val 'Hide Answer' @element.addClass 'showed' else - @$('[id^=answer_]').text '' - @$('[id^=solution_]').text '' + @$('[id^=answer_], [id^=solution_]').text '' @$('[correct_answer]').attr correct_answer: null @element.removeClass 'showed' @$('.show').val 'Show Answer' From a3561c96f79853751afce497b37a5f8c9b23fc86 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 11 Jun 2012 15:48:33 -0400 Subject: [PATCH 53/69] added TODO comment --- common/lib/xmodule/capa_module.py | 1 + 1 file changed, 1 insertion(+) diff --git a/common/lib/xmodule/capa_module.py b/common/lib/xmodule/capa_module.py index 439982a2c1..c7b52ae8ba 100644 --- a/common/lib/xmodule/capa_module.py +++ b/common/lib/xmodule/capa_module.py @@ -281,6 +281,7 @@ class Module(XModule): def answer_available(self): ''' Is the user allowed to see an answer? + TODO: simplify. ''' if self.show_answer == '': return False From c9cbc52ffada2abe6fcbfa10704ea2406cb7b2eb Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 11 Jun 2012 15:49:39 -0400 Subject: [PATCH 54/69] Add rough overview docs --- doc/README | 3 ++ doc/overview.md | 104 ++++++++++++++++++++++++++++++++++++++++++++++++ doc/testing.md | 12 ++++++ 3 files changed, 119 insertions(+) create mode 100644 doc/README create mode 100644 doc/overview.md create mode 100644 doc/testing.md diff --git a/doc/README b/doc/README new file mode 100644 index 0000000000..65f3cd2cb5 --- /dev/null +++ b/doc/README @@ -0,0 +1,3 @@ +This directory contains some high level documentation for the code. We should strive to keep it up-to-date, but don't take it as the absolute truth. + +A good place to start is 'overview' diff --git a/doc/overview.md b/doc/overview.md new file mode 100644 index 0000000000..a53e7371c2 --- /dev/null +++ b/doc/overview.md @@ -0,0 +1,104 @@ +# Documentation for edX code (mitx repo) + +This document explains the general structure of the edX platform, and defines some of the acronyms and terms you'll see flying around in the code. + +## Assumptions: + +You should be familiar with the following. If you're not, go read some docs... + + - python + - django + - javascript + - html, xml -- xpath, xslt + - css + - git + - mako templates -- we use these instead of django templates, because they support embedding real python. + +## Other relevant terms + + - CAPA -- lon-capa.org -- content management system that has defined a standard for online learning and assessment materials. Many of our materials follow this standard. + - TODO: add more details / link to relevant docs. lon-capa.org is not immediately intuitive. + - lcp = loncapa problem + + +## Parts of the system + + - LMS -- Learning Management System. The student-facing parts of the system. Handles student accounts, displaying videos, tutorials, exercies, problems, etc. + + - CMS -- Course Management System. The instructor-facing parts of the system. Allows instructors to see and modify their course, add lectures, problems, reorder things, etc. + + - Askbot -- the discussion forums. We have a custom fork of this project. We're also hoping to replace it with something better later. (e.g. need support for multiple classes, etc) + + - Data. In the data/ dir. There is currently a single `course.xml` file that describes an entire course. Speaking of which... + + - Courses. A course is broken up into Chapters ("week 1", "week 2", etc). A chapter is broken up into Sections ("Lecture 1", "Simple Circuits Exercises", "HW1", etc). A section can contain modules: Problems, Html, Videos, Verticals, or Sequences. + - Problems: specified in problem files. May have python scripts embedded to both generate random parameters and check answers. Also allows specifying things like tolerance or precision in answers + - Html: any html - often description, or links to outside resources + - Videos: links to youtube or elsewhere + - Verticals: a nesting tag: collect several videos, problems, html modules and display them vertically. + - Sequences: a sequence of modules, displayed with a horizontal navigation bar, displaying one component at a time. + - see `data/course.xml` for more examples + + +## High Level Entities in the code + +### Common libraries + +- x_modules -- generic learning modules. *x* can be sequence, video, template, html, vertical, capa, etc. These are the things that one puts inside sections in the course structure. Modules know how to render themselves to html, how to score themselves, and handle ajax calls from the front end. + - x_modules take a 'system' context parameter, which is a reference to an object that knows how to render things, track events, complain about 404s, etc. (TODO: figure out, document the necessary interface--different in `x_module.XModule.__init__` and in `x_module tests.py`) + - in `common/lib/xmodule` + +- capa modules -- defines `LoncapaProblem` and many related things. + - in `common/lib/capa` + +### LMS + +The LMS is a django site, with root in `lms/`. It runs in many different environments--the settings files are in `lms/envs`. + +- We use the Django Auth system, including the is_staff and is_superuser flags. User profiles and related code lives in `lms/djangoapps/student/`. There is support for groups of students (e.g. 'want emails about future courses', 'have unenrolled', etc) in `lms/djangoapps/student/models.py`. + +- `StudentModule` -- keeps track of where a particular student is in a module (problem, video, html)--what's their grade, have they started, are they done, etc. [This is only partly implemented so far.] + - `lms/djangoapps/courseware/models.py` + +- Core rendering path: + - `lms/urls.py` points to `courseware.views.index`, which gets module info from the course xml file, pulls list of `StudentModule` objects for this user (to avoid multiple db hits). + + - Calls `render_accordion` to render the "accordion"--the display of the course structure. + + - To render the current module, calls `module_render.py:render_x_module()`, which gets the `StudentModule` instance, and passes the `StudentModule` state and other system context to the module constructor the get an instance of the appropriate module class for this user. + + - calls the module's `.get_html()` method. If the module has nested submodules, render_x_module() will be called again for each. + + - ajax calls go to `module_render.py:modx_dispatch()`, which passes it to the module's `handle_ajax()` function, and then updates the grade and state if they changed. + +- Tracking: there is support for basic tracking of client-side events in `lms/djangoapps/track`. + +### Other modules + +- Wiki -- in `lms/djangoapps/simplewiki`. Has some markdown extentions for embedding circuits, videos, etc. + +## Testing + +See `testing.md`. + + +## QUESTIONS: + +`common/lib/capa` : what is eia, `eia.py`? Random lists of numbers? + +what is `lms/lib/dogfood`? Looks like a way to test capa problems... Is it being used? + +is lms/envs/README.txt out of date? + +## TODO: + +- Only describes backend code so far. How does the front-end work? + +- What big pieces are missing? + +- Where should reader go next? + +--- +Note: this file uses markdown. To convert to html, run: + + markdown2 overview.md > overview.html diff --git a/doc/testing.md b/doc/testing.md new file mode 100644 index 0000000000..fa134ade66 --- /dev/null +++ b/doc/testing.md @@ -0,0 +1,12 @@ +# Testing + +Testing is good. Here is some useful info about how we set up tests-- + +### Backend code: + +- TODO + +### Frontend code: + +- TODO + From 17af925a8a0016f66c1b169fe463c60c4bc3966a Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 11 Jun 2012 15:34:52 -0400 Subject: [PATCH 55/69] add function docs to module_render.py --- lms/djangoapps/courseware/module_render.py | 29 +++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index fd419a68dc..ec13dc11a5 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -89,6 +89,19 @@ def grade_histogram(module_id): return grades def get_module(user, request, xml_module, module_object_preload, position=None): + ''' Get the appropriate xmodule and StudentModule. + + Arguments: + - user : current django User + - request : current django HTTPrequest + - xml_module : lxml etree of xml subtree for the current module + - module_object_preload : list of StudentModule objects, one of which may match this module type and id + - position : extra information from URL for user-specified position within module + + Returns: + - a tuple (xmodule instance, student module, module type). + + ''' module_type=xml_module.tag module_class=xmodule.get_module_class(module_type) module_id=xml_module.get('id') #module_class.id_attribute) or "" @@ -184,7 +197,19 @@ def render_x_module(user, request, xml_module, module_object_preload, position=N return content def modx_dispatch(request, module=None, dispatch=None, id=None): - ''' Generic view for extensions. This is where AJAX calls go.''' + ''' Generic view for extensions. This is where AJAX calls go. + + Arguments: + + - request -- the django request. + - module -- the name of the module, as used in the course configuration xml. + - dispatch -- the command string to pass through to the module's handle_ajax call + (e.g. 'problem_reset'). If this string contains '?', only pass + through the part before the first '?'. + - id -- the module id. Used to look up the student module. + + TODO: why are id and module not the same? + ''' if not request.user.is_authenticated(): return redirect('/') @@ -200,6 +225,8 @@ def modx_dispatch(request, module=None, dispatch=None, id=None): oldgrade = s.grade oldstate = s.state + # TODO: if dispatch is left at default value None, this will go boom. What's the correct + # behavior? dispatch=dispatch.split('?')[0] ajax_url = settings.MITX_ROOT_URL + '/modx/'+module+'/'+id+'/' From 9f38ccb65df418b2a3972932a5b19d49f7f7482c Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 12 Jun 2012 09:31:22 -0400 Subject: [PATCH 56/69] add README.md for dogfood --- lms/lib/dogfood/README.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 lms/lib/dogfood/README.md diff --git a/lms/lib/dogfood/README.md b/lms/lib/dogfood/README.md new file mode 100644 index 0000000000..c6a7113049 --- /dev/null +++ b/lms/lib/dogfood/README.md @@ -0,0 +1 @@ +This is a library for edx4edx, allowing users to practice writing problems. From 600899c16cb357f7594b6ba3e395fe88ccb7ad97 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 12 Jun 2012 09:32:04 -0400 Subject: [PATCH 57/69] add comment/explanation to eia.py --- common/lib/capa/eia.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/common/lib/capa/eia.py b/common/lib/capa/eia.py index bc962312ec..362dc33a2d 100644 --- a/common/lib/capa/eia.py +++ b/common/lib/capa/eia.py @@ -1,3 +1,6 @@ +""" Standard resistor codes. +http://en.wikipedia.org/wiki/Electronic_color_code +""" E6=[10,15,22,33,47,68] E12=[10,12,15,18,22,27,33,39,47,56,68,82] E24=[10,12,15,18,22,27,33,39,47,56,68,82,11,13,16,20,24,30,36,43,51,62,75,91] From c3a1d7788ccc36a744d251c97d375844d48e3053 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 12 Jun 2012 09:32:41 -0400 Subject: [PATCH 58/69] clarify docstring for modx_dispatch --- lms/djangoapps/courseware/module_render.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index ec13dc11a5..fe6ebdd585 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -202,13 +202,13 @@ def modx_dispatch(request, module=None, dispatch=None, id=None): Arguments: - request -- the django request. - - module -- the name of the module, as used in the course configuration xml. + - module -- the type of the module, as used in the course configuration xml. + e.g. 'problem', 'video', etc - dispatch -- the command string to pass through to the module's handle_ajax call (e.g. 'problem_reset'). If this string contains '?', only pass through the part before the first '?'. - id -- the module id. Used to look up the student module. - - TODO: why are id and module not the same? + e.g. filenamexformularesponse ''' if not request.user.is_authenticated(): return redirect('/') From 7608003c15121026a4609925f88fc7dcc33a1757 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 12 Jun 2012 09:33:41 -0400 Subject: [PATCH 59/69] Update overview to reflect suggestions --- doc/README | 2 +- doc/overview.md | 22 +++++++++------------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/doc/README b/doc/README index 65f3cd2cb5..d40f5d988d 100644 --- a/doc/README +++ b/doc/README @@ -1,3 +1,3 @@ This directory contains some high level documentation for the code. We should strive to keep it up-to-date, but don't take it as the absolute truth. -A good place to start is 'overview' +A good place to start is 'overview.md' diff --git a/doc/overview.md b/doc/overview.md index a53e7371c2..304d5161b0 100644 --- a/doc/overview.md +++ b/doc/overview.md @@ -45,7 +45,8 @@ You should be familiar with the following. If you're not, go read some docs... ### Common libraries - x_modules -- generic learning modules. *x* can be sequence, video, template, html, vertical, capa, etc. These are the things that one puts inside sections in the course structure. Modules know how to render themselves to html, how to score themselves, and handle ajax calls from the front end. - - x_modules take a 'system' context parameter, which is a reference to an object that knows how to render things, track events, complain about 404s, etc. (TODO: figure out, document the necessary interface--different in `x_module.XModule.__init__` and in `x_module tests.py`) + - x_modules take a 'system context' parameter, which helps isolate xmodules from any particular application, so they can be used in many places. The modules should make no references to Django (though there are still a few left). The system context knows how to render things, track events, complain about 404s, etc. + - TODO: document the system context interface--it's different in `x_module.XModule.__init__` and in `x_module tests.py` (do this in the code, not here) - in `common/lib/xmodule` - capa modules -- defines `LoncapaProblem` and many related things. @@ -71,6 +72,10 @@ The LMS is a django site, with root in `lms/`. It runs in many different enviro - ajax calls go to `module_render.py:modx_dispatch()`, which passes it to the module's `handle_ajax()` function, and then updates the grade and state if they changed. + - [This diagram](https://github.com/MITx/mitx/wiki/MITx-Architecture) visually shows how the clients communicate with problems + modules. + +- See `lms/urls.py` for the wirings of urls to views. + - Tracking: there is support for basic tracking of client-side events in `lms/djangoapps/track`. ### Other modules @@ -81,22 +86,13 @@ The LMS is a django site, with root in `lms/`. It runs in many different enviro See `testing.md`. - -## QUESTIONS: - -`common/lib/capa` : what is eia, `eia.py`? Random lists of numbers? - -what is `lms/lib/dogfood`? Looks like a way to test capa problems... Is it being used? - -is lms/envs/README.txt out of date? - ## TODO: -- Only describes backend code so far. How does the front-end work? +- update lms/envs/README.txt -- What big pieces are missing? +- describe our production environment -- Where should reader go next? +- describe the front-end architecture, tools, etc. Starting point: `lms/static` --- Note: this file uses markdown. To convert to html, run: From 0f073f70c489d1a79efdfe971cdf11690ff844e5 Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Wed, 13 Jun 2012 14:37:21 -0400 Subject: [PATCH 60/69] Create Calculator and Feedback form on every page --- lms/static/coffee/spec/calculator_spec.coffee | 6 +++--- lms/static/coffee/spec/courseware_spec.coffee | 10 ---------- lms/static/coffee/src/calculator.coffee | 3 ++- lms/static/coffee/src/courseware.coffee | 2 -- lms/static/coffee/src/main.coffee | 2 ++ 5 files changed, 7 insertions(+), 16 deletions(-) diff --git a/lms/static/coffee/spec/calculator_spec.coffee b/lms/static/coffee/spec/calculator_spec.coffee index 58d7c70790..e57bff4a54 100644 --- a/lms/static/coffee/spec/calculator_spec.coffee +++ b/lms/static/coffee/spec/calculator_spec.coffee @@ -33,16 +33,16 @@ describe 'Calculator', -> describe 'toggle', -> it 'toggle the calculator and focus the input', -> spyOn $.fn, 'focus' - @calculator.toggle() + @calculator.toggle(jQuery.Event("click")) expect($('li.calc-main')).toHaveClass('open') expect($('#calculator_wrapper #calculator_input').focus).toHaveBeenCalled() it 'toggle the close button on the calculator button', -> - @calculator.toggle() + @calculator.toggle(jQuery.Event("click")) expect($('.calc')).toHaveClass('closed') - @calculator.toggle() + @calculator.toggle(jQuery.Event("click")) expect($('.calc')).not.toHaveClass('closed') describe 'helpToggle', -> diff --git a/lms/static/coffee/spec/courseware_spec.coffee b/lms/static/coffee/spec/courseware_spec.coffee index 9d938c14e1..cc64536849 100644 --- a/lms/static/coffee/spec/courseware_spec.coffee +++ b/lms/static/coffee/spec/courseware_spec.coffee @@ -5,16 +5,6 @@ describe 'Courseware', -> Courseware.start() expect(window.Navigation).toHaveBeenCalled() - it 'create the calculator', -> - spyOn(window, 'Calculator') - Courseware.start() - expect(window.Calculator).toHaveBeenCalled() - - it 'creates the FeedbackForm', -> - spyOn(window, 'FeedbackForm') - Courseware.start() - expect(window.FeedbackForm).toHaveBeenCalled() - it 'binds the Logger', -> spyOn(Logger, 'bind') Courseware.start() diff --git a/lms/static/coffee/src/calculator.coffee b/lms/static/coffee/src/calculator.coffee index 2f9b8a1aa8..18e7fba4ec 100644 --- a/lms/static/coffee/src/calculator.coffee +++ b/lms/static/coffee/src/calculator.coffee @@ -6,7 +6,8 @@ class @Calculator $('div.help-wrapper a').hover(@helpToggle).click (e) -> e.preventDefault() - toggle: -> + toggle: (event) -> + event.preventDefault() $('li.calc-main').toggleClass 'open' $('#calculator_wrapper #calculator_input').focus() if $('.calc.closed').length diff --git a/lms/static/coffee/src/courseware.coffee b/lms/static/coffee/src/courseware.coffee index e110dfe0ac..de232e05e4 100644 --- a/lms/static/coffee/src/courseware.coffee +++ b/lms/static/coffee/src/courseware.coffee @@ -4,8 +4,6 @@ class @Courseware constructor: -> Courseware.prefix = $("meta[name='path_prefix']").attr('content') new Navigation - new Calculator - new FeedbackForm Logger.bind() @bind() @render() diff --git a/lms/static/coffee/src/main.coffee b/lms/static/coffee/src/main.coffee index 9f3c759a9a..7fe23689c5 100644 --- a/lms/static/coffee/src/main.coffee +++ b/lms/static/coffee/src/main.coffee @@ -17,6 +17,8 @@ $ -> $("a[rel*=leanModal]").leanModal() $('#csrfmiddlewaretoken').attr 'value', $.cookie('csrftoken') + new Calculator + new FeedbackForm if $('body').hasClass('courseware') Courseware.start() From e14563954a8a141a8fae687f28c1423a3542329e Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Tue, 12 Jun 2012 17:16:11 -0400 Subject: [PATCH 61/69] Be specific on video progress slider handle Using `.ui-slider-handle` was too broad. --- .../coffee/spec/modules/video/video_progress_slider_spec.coffee | 2 +- .../coffee/src/modules/video/video_progress_slider.coffee | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/static/coffee/spec/modules/video/video_progress_slider_spec.coffee b/lms/static/coffee/spec/modules/video/video_progress_slider_spec.coffee index ba7d439a94..ae78048c55 100644 --- a/lms/static/coffee/spec/modules/video/video_progress_slider_spec.coffee +++ b/lms/static/coffee/spec/modules/video/video_progress_slider_spec.coffee @@ -18,7 +18,7 @@ describe 'VideoProgressSlider', -> stop: @slider.onStop it 'build the seek handle', -> - expect(@slider.handle).toBe '.ui-slider-handle' + expect(@slider.handle).toBe '.slider .ui-slider-handle' expect($.fn.qtip).toHaveBeenCalledWith content: "0:00" position: diff --git a/lms/static/coffee/src/modules/video/video_progress_slider.coffee b/lms/static/coffee/src/modules/video/video_progress_slider.coffee index 923f06b0c3..af0bbb53a6 100644 --- a/lms/static/coffee/src/modules/video/video_progress_slider.coffee +++ b/lms/static/coffee/src/modules/video/video_progress_slider.coffee @@ -17,7 +17,7 @@ class @VideoProgressSlider @buildHandle() buildHandle: -> - @handle = @$('.ui-slider-handle') + @handle = @$('.slider .ui-slider-handle') @handle.qtip content: "#{Time.format(@slider.slider('value'))}" position: From 7924e5c8f5c6c9e952b6002eb7b4b47214773287 Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Wed, 13 Jun 2012 11:46:27 -0400 Subject: [PATCH 62/69] Add video volume control to video player --- lms/static/coffee/spec/helper.coffee | 3 +- .../modules/video/video_player_spec.coffee | 14 +++ .../video/video_speed_control_spec.coffee | 2 - .../video/video_volume_control_spec.coffee | 102 ++++++++++++++++++ .../src/modules/video/video_player.coffee | 7 ++ .../modules/video/video_volume_control.coffee | 48 +++++++++ lms/static/sass/courseware/_video.scss | 80 ++++++++++++++ 7 files changed, 253 insertions(+), 3 deletions(-) create mode 100644 lms/static/coffee/spec/modules/video/video_volume_control_spec.coffee create mode 100644 lms/static/coffee/src/modules/video/video_volume_control.coffee diff --git a/lms/static/coffee/spec/helper.coffee b/lms/static/coffee/spec/helper.coffee index 3d9537f872..9b575452c7 100644 --- a/lms/static/coffee/spec/helper.coffee +++ b/lms/static/coffee/spec/helper.coffee @@ -30,7 +30,8 @@ jasmine.stubRequests = -> jasmine.stubYoutubePlayer = -> YT.Player = -> jasmine.createSpyObj 'YT.Player', ['cueVideoById', 'getVideoEmbedCode', - 'getCurrentTime', 'getPlayerState', 'loadVideoById', 'playVideo', 'pauseVideo', 'seekTo'] + 'getCurrentTime', 'getPlayerState', 'getVolume', 'setVolume', 'loadVideoById', + 'playVideo', 'pauseVideo', 'seekTo'] jasmine.stubVideoPlayer = (context, enableParts) -> enableParts = [enableParts] unless $.isArray(enableParts) diff --git a/lms/static/coffee/spec/modules/video/video_player_spec.coffee b/lms/static/coffee/spec/modules/video/video_player_spec.coffee index cdfedcb4f9..05f447b8f9 100644 --- a/lms/static/coffee/spec/modules/video/video_player_spec.coffee +++ b/lms/static/coffee/spec/modules/video/video_player_spec.coffee @@ -387,3 +387,17 @@ describe 'VideoPlayer', -> it 'delegate to the video', -> expect(@player.currentSpeed()).toEqual '3.0' + + describe 'volume', -> + beforeEach -> + @player = new VideoPlayer @video + @player.player.getVolume.andReturn 42 + + describe 'without value', -> + it 'return current volume', -> + expect(@player.volume()).toEqual 42 + + describe 'with value', -> + it 'set player volume', -> + @player.volume(60) + expect(@player.player.setVolume).toHaveBeenCalledWith(60) diff --git a/lms/static/coffee/spec/modules/video/video_speed_control_spec.coffee b/lms/static/coffee/spec/modules/video/video_speed_control_spec.coffee index 9d52ac9b0d..737460d1ce 100644 --- a/lms/static/coffee/spec/modules/video/video_speed_control_spec.coffee +++ b/lms/static/coffee/spec/modules/video/video_speed_control_spec.coffee @@ -3,8 +3,6 @@ describe 'VideoSpeedControl', -> @player = jasmine.stubVideoPlayer @ $('.speeds').remove() - afterEach -> - describe 'constructor', -> describe 'always', -> beforeEach -> diff --git a/lms/static/coffee/spec/modules/video/video_volume_control_spec.coffee b/lms/static/coffee/spec/modules/video/video_volume_control_spec.coffee new file mode 100644 index 0000000000..cbdef03ef0 --- /dev/null +++ b/lms/static/coffee/spec/modules/video/video_volume_control_spec.coffee @@ -0,0 +1,102 @@ +describe 'VideoVolumeControl', -> + beforeEach -> + @player = jasmine.stubVideoPlayer @ + $('.volume').remove() + + describe 'constructor', -> + beforeEach -> + spyOn($.fn, 'slider') + @volumeControl = new VideoVolumeControl @player + + it 'initialize previousVolume to 100', -> + expect(@volumeControl.previousVolume).toEqual 100 + + it 'render the volume control', -> + expect($('.secondary-controls').html()).toContain """ +
+ +
+
+
+
+ """ + + it 'create the slider', -> + expect($.fn.slider).toHaveBeenCalledWith + orientation: "vertical" + range: "min" + min: 0 + max: 100 + value: 100 + change: @volumeControl.onChange + slide: @volumeControl.onChange + + it 'bind the volume control', -> + expect($(@player)).toHandleWith 'ready', @volumeControl.onReady + expect($('.volume>a')).toHandleWith 'click', @volumeControl.toggleMute + + expect($('.volume')).not.toHaveClass 'open' + $('.volume').mouseenter() + expect($('.volume')).toHaveClass 'open' + $('.volume').mouseleave() + expect($('.volume')).not.toHaveClass 'open' + + describe 'onReady', -> + beforeEach -> + @volumeControl = new VideoVolumeControl @player + spyOn $.fn, 'slider' + spyOn(@player, 'volume').andReturn 60 + @volumeControl.onReady() + + it 'set the max value of the slider', -> + expect($.fn.slider).toHaveBeenCalledWith 'option', 'max', 60 + + describe 'onChange', -> + beforeEach -> + spyOn @player, 'volume' + @volumeControl = new VideoVolumeControl @player + + describe 'when the new volume is more than 0', -> + beforeEach -> + @volumeControl.onChange undefined, value: 60 + + it 'set the player volume', -> + expect(@player.volume).toHaveBeenCalledWith 60 + + it 'remote muted class', -> + expect($('.volume')).not.toHaveClass 'muted' + + describe 'when the new volume is 0', -> + beforeEach -> + @volumeControl.onChange undefined, value: 0 + + it 'set the player volume', -> + expect(@player.volume).toHaveBeenCalledWith 0 + + it 'add muted class', -> + expect($('.volume')).toHaveClass 'muted' + + describe 'toggleMute', -> + beforeEach -> + spyOn @player, 'volume' + @volumeControl = new VideoVolumeControl @player + + describe 'when the current volume is more than 0', -> + beforeEach -> + @player.volume.andReturn 60 + @volumeControl.toggleMute() + + it 'save the previous volume', -> + expect(@volumeControl.previousVolume).toEqual 60 + + it 'set the player volume', -> + expect(@player.volume).toHaveBeenCalledWith 0 + + describe 'when the current volume is 0', -> + beforeEach -> + @player.volume.andReturn 0 + @volumeControl.previousVolume = 60 + @volumeControl.toggleMute() + + it 'set the player volume to previous volume', -> + expect(@player.volume).toHaveBeenCalledWith 60 diff --git a/lms/static/coffee/src/modules/video/video_player.coffee b/lms/static/coffee/src/modules/video/video_player.coffee index 28f87b2d53..cccc6873b1 100644 --- a/lms/static/coffee/src/modules/video/video_player.coffee +++ b/lms/static/coffee/src/modules/video/video_player.coffee @@ -30,6 +30,7 @@ class @VideoPlayer render: -> new VideoControl @ new VideoCaption @, @video.youtubeId('1.0') + new VideoVolumeControl @ new VideoSpeedControl @, @video.speeds new VideoProgressSlider @ @player = new YT.Player @video.id, @@ -132,3 +133,9 @@ class @VideoPlayer currentSpeed: -> @video.speed + + volume: (value) -> + if value != undefined + @player.setVolume value + else + @player.getVolume() diff --git a/lms/static/coffee/src/modules/video/video_volume_control.coffee b/lms/static/coffee/src/modules/video/video_volume_control.coffee new file mode 100644 index 0000000000..1c0d3440d5 --- /dev/null +++ b/lms/static/coffee/src/modules/video/video_volume_control.coffee @@ -0,0 +1,48 @@ +class @VideoVolumeControl + constructor: (@player) -> + @previousVolume = 100 + @render() + @bind() + + $: (selector) -> + @player.$(selector) + + bind: -> + $(@player).bind('ready', @onReady) + @$('.volume').mouseenter -> + $(this).addClass('open') + @$('.volume').mouseleave -> + $(this).removeClass('open') + @$('.volume>a').click(@toggleMute) + + render: -> + @$('.secondary-controls').prepend """ +
+ +
+
+
+
+ """ + @slider = @$('.volume-slider').slider + orientation: "vertical" + range: "min" + min: 0 + max: 100 + value: 100 + change: @onChange + slide: @onChange + + onReady: => + @slider.slider 'option', 'max', @player.volume() + + onChange: (event, ui) => + @player.volume ui.value + @$('.secondary-controls .volume').toggleClass 'muted', ui.value == 0 + + toggleMute: => + if @player.volume() > 0 + @previousVolume = @player.volume() + @slider.slider 'option', 'value', 0 + else + @slider.slider 'option', 'value', @previousVolume diff --git a/lms/static/sass/courseware/_video.scss b/lms/static/sass/courseware/_video.scss index 7130cc8b7b..f6c6ac5a98 100644 --- a/lms/static/sass/courseware/_video.scss +++ b/lms/static/sass/courseware/_video.scss @@ -286,6 +286,86 @@ section.course-content { } } + div.volume { + float: left; + position: relative; + + &.open { + .volume-slider-container { + display: block; + opacity: 1; + } + } + + &.muted { + &>a { + // TODO: Replace this with muted speaker icon + background: url('../images/closed-arrow.png') 10px center no-repeat; + } + } + + &>a { + // TODO: Replace this with speaker icon + background: url('../images/open-arrow.png') 10px center no-repeat; + border-right: 1px solid #000; + @include box-shadow(1px 0 0 #555, inset 1px 0 0 #555); + @include clearfix(); + color: #fff; + cursor: pointer; + display: block; + height: 46px; + margin-right: 0; + padding-left: 15px; + position: relative; + @include transition(); + -webkit-font-smoothing: antialiased; + width: 30px; + + &:hover, &:active, &:focus { + background-color: #444; + } + } + + .volume-slider-container { + @include box-shadow(inset 1px 0 0 #555, 0 3px 0 #444); + @include transition(); + background-color: #444; + border: 1px solid #000; + bottom: 46px; + display: none; + opacity: 0; + position: absolute; + width: 35px; + height: 125px; + margin-left: 5px; + z-index: 10; + + .volume-slider { + height: 100px; + border: 0; + width: 5px; + margin: 14px auto; + + a.ui-slider-handle { + background: $mit-red url(../images/slider-handle.png) center center no-repeat; + @include background-size(50%); + border: 1px solid darken($mit-red, 20%); + @include border-radius(15px); + @include box-shadow(inset 0 1px 0 lighten($mit-red, 10%)); + cursor: pointer; + height: 15px; + left: -6px; + @include transition(height 2.0s ease-in-out, width 2.0s ease-in-out); + width: 15px; + } + + .ui-slider-range { + background: #666; + } + } + } + } + a.add-fullscreen { background: url(../images/fullscreen.png) center no-repeat; border-right: 1px solid #000; From 98ed7384908e49fcd772190c10c5de86b3cfca79 Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Wed, 13 Jun 2012 12:12:17 -0400 Subject: [PATCH 63/69] Make sure we don't show volume control on iOS --- lms/static/coffee/spec/helper.coffee | 7 +- .../modules/video/video_player_spec.coffee | 111 +++++++++++------- .../src/modules/video/video_player.coffee | 2 +- 3 files changed, 73 insertions(+), 47 deletions(-) diff --git a/lms/static/coffee/spec/helper.coffee b/lms/static/coffee/spec/helper.coffee index 9b575452c7..1bb92160ce 100644 --- a/lms/static/coffee/spec/helper.coffee +++ b/lms/static/coffee/spec/helper.coffee @@ -33,14 +33,14 @@ jasmine.stubYoutubePlayer = -> 'getCurrentTime', 'getPlayerState', 'getVolume', 'setVolume', 'loadVideoById', 'playVideo', 'pauseVideo', 'seekTo'] -jasmine.stubVideoPlayer = (context, enableParts) -> +jasmine.stubVideoPlayer = (context, enableParts, createPlayer=true) -> enableParts = [enableParts] unless $.isArray(enableParts) suite = context.suite currentPartName = suite.description while suite = suite.parentSuite enableParts.push currentPartName - for part in ['VideoCaption', 'VideoSpeedControl', 'VideoProgressSlider'] + for part in ['VideoCaption', 'VideoSpeedControl', 'VideoVolumeControl', 'VideoProgressSlider'] unless $.inArray(part, enableParts) >= 0 spyOn window, part @@ -49,7 +49,8 @@ jasmine.stubVideoPlayer = (context, enableParts) -> YT.Player = undefined context.video = new Video 'example', '.75:abc123,1.0:def456' jasmine.stubYoutubePlayer() - return new VideoPlayer context.video + if createPlayer + return new VideoPlayer context.video spyOn(window, 'onunload') diff --git a/lms/static/coffee/spec/modules/video/video_player_spec.coffee b/lms/static/coffee/spec/modules/video/video_player_spec.coffee index 05f447b8f9..ebcb2cc009 100644 --- a/lms/static/coffee/spec/modules/video/video_player_spec.coffee +++ b/lms/static/coffee/spec/modules/video/video_player_spec.coffee @@ -1,6 +1,6 @@ describe 'VideoPlayer', -> beforeEach -> - jasmine.stubVideoPlayer @ + jasmine.stubVideoPlayer @, [], false afterEach -> YT.Player = undefined @@ -11,69 +11,94 @@ describe 'VideoPlayer', -> spyOn YT, 'Player' $.fn.qtip.andCallFake -> $(this).data('qtip', true) - $('.video').append $('
') - @player = new VideoPlayer @video + $('.video').append $('
') - it 'instanticate current time to zero', -> - expect(@player.currentTime).toEqual 0 + describe 'always', -> + beforeEach -> + @player = new VideoPlayer @video - it 'set the element', -> - expect(@player.element).toBe '#video_example' + it 'instanticate current time to zero', -> + expect(@player.currentTime).toEqual 0 - it 'create video control', -> - expect(window.VideoControl).toHaveBeenCalledWith @player + it 'set the element', -> + expect(@player.element).toBe '#video_example' - it 'create video caption', -> - expect(window.VideoCaption).toHaveBeenCalledWith @player, 'def456' + it 'create video control', -> + expect(window.VideoControl).toHaveBeenCalledWith @player - it 'create video speed control', -> - expect(window.VideoSpeedControl).toHaveBeenCalledWith @player, ['0.75', '1.0'] + it 'create video caption', -> + expect(window.VideoCaption).toHaveBeenCalledWith @player, 'def456' - it 'create video progress slider', -> - expect(window.VideoProgressSlider).toHaveBeenCalledWith @player + it 'create video speed control', -> + expect(window.VideoSpeedControl).toHaveBeenCalledWith @player, ['0.75', '1.0'] - it 'create Youtube player', -> - expect(YT.Player).toHaveBeenCalledWith 'example' - playerVars: - controls: 0 - wmode: 'transparent' - rel: 0 - showinfo: 0 - enablejsapi: 1 - videoId: 'def456' - events: - onReady: @player.onReady - onStateChange: @player.onStateChange + it 'create video progress slider', -> + expect(window.VideoProgressSlider).toHaveBeenCalledWith @player - it 'bind to seek event', -> - expect($(@player)).toHandleWith 'seek', @player.onSeek + it 'create Youtube player', -> + expect(YT.Player).toHaveBeenCalledWith 'example' + playerVars: + controls: 0 + wmode: 'transparent' + rel: 0 + showinfo: 0 + enablejsapi: 1 + videoId: 'def456' + events: + onReady: @player.onReady + onStateChange: @player.onStateChange - it 'bind to updatePlayTime event', -> - expect($(@player)).toHandleWith 'updatePlayTime', @player.onUpdatePlayTime + it 'bind to seek event', -> + expect($(@player)).toHandleWith 'seek', @player.onSeek - it 'bidn to speedChange event', -> - expect($(@player)).toHandleWith 'speedChange', @player.onSpeedChange + it 'bind to updatePlayTime event', -> + expect($(@player)).toHandleWith 'updatePlayTime', @player.onUpdatePlayTime - it 'bind to play event', -> - expect($(@player)).toHandleWith 'play', @player.onPlay + it 'bidn to speedChange event', -> + expect($(@player)).toHandleWith 'speedChange', @player.onSpeedChange - it 'bind to paused event', -> - expect($(@player)).toHandleWith 'pause', @player.onPause + it 'bind to play event', -> + expect($(@player)).toHandleWith 'play', @player.onPlay - it 'bind to ended event', -> - expect($(@player)).toHandleWith 'ended', @player.onPause + it 'bind to paused event', -> + expect($(@player)).toHandleWith 'pause', @player.onPause - it 'bind to key press', -> - expect($(document)).toHandleWith 'keyup', @player.bindExitFullScreen + it 'bind to ended event', -> + expect($(@player)).toHandleWith 'ended', @player.onPause - it 'bind to fullscreen switching button', -> - expect($('.add-fullscreen')).toHandleWith 'click', @player.toggleFullScreen + it 'bind to key press', -> + expect($(document)).toHandleWith 'keyup', @player.bindExitFullScreen + + it 'bind to fullscreen switching button', -> + console.debug $('.add-fullscreen') + expect($('.add-fullscreen')).toHandleWith 'click', @player.toggleFullScreen describe 'when not on a touch based device', -> + beforeEach -> + spyOn(window, 'onTouchBasedDevice').andReturn false + $('.add-fullscreen, .hide-subtitles').removeData 'qtip' + @player = new VideoPlayer @video + it 'add the tooltip to fullscreen and subtitle button', -> expect($('.add-fullscreen')).toHaveData 'qtip' expect($('.hide-subtitles')).toHaveData 'qtip' + it 'create video volume control', -> + expect(window.VideoVolumeControl).toHaveBeenCalledWith @player + + describe 'when on a touch based device', -> + beforeEach -> + spyOn(window, 'onTouchBasedDevice').andReturn true + $('.add-fullscreen, .hide-subtitles').removeData 'qtip' + @player = new VideoPlayer @video + + it 'does not add the tooltip to fullscreen and subtitle button', -> + expect($('.add-fullscreen')).not.toHaveData 'qtip' + expect($('.hide-subtitles')).not.toHaveData 'qtip' + + it 'does not create video volume control', -> + expect(window.VideoVolumeControl).not.toHaveBeenCalled() + describe 'onReady', -> beforeEach -> @video.embed() diff --git a/lms/static/coffee/src/modules/video/video_player.coffee b/lms/static/coffee/src/modules/video/video_player.coffee index cccc6873b1..0dd52a128b 100644 --- a/lms/static/coffee/src/modules/video/video_player.coffee +++ b/lms/static/coffee/src/modules/video/video_player.coffee @@ -30,7 +30,7 @@ class @VideoPlayer render: -> new VideoControl @ new VideoCaption @, @video.youtubeId('1.0') - new VideoVolumeControl @ + new VideoVolumeControl @ unless onTouchBasedDevice() new VideoSpeedControl @, @video.speeds new VideoProgressSlider @ @player = new YT.Player @video.id, From b97f82df8f9b6e4e17aa020553f2b54fa77e1217 Mon Sep 17 00:00:00 2001 From: Kyle Fiedler Date: Wed, 13 Jun 2012 15:13:40 -0400 Subject: [PATCH 64/69] Added some styles to clean up volume slider --- lms/static/sass/courseware/_video.scss | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lms/static/sass/courseware/_video.scss b/lms/static/sass/courseware/_video.scss index f6c6ac5a98..87092fdc54 100644 --- a/lms/static/sass/courseware/_video.scss +++ b/lms/static/sass/courseware/_video.scss @@ -299,14 +299,12 @@ section.course-content { &.muted { &>a { - // TODO: Replace this with muted speaker icon - background: url('../images/closed-arrow.png') 10px center no-repeat; + background: url('../images/mute.png') 10px center no-repeat; } } - &>a { - // TODO: Replace this with speaker icon - background: url('../images/open-arrow.png') 10px center no-repeat; + > a { + background: url('../images/volume.png') 10px center no-repeat; border-right: 1px solid #000; @include box-shadow(1px 0 0 #555, inset 1px 0 0 #555); @include clearfix(); @@ -335,9 +333,9 @@ section.course-content { display: none; opacity: 0; position: absolute; - width: 35px; + width: 45px; height: 125px; - margin-left: 5px; + margin-left: -1px; z-index: 10; .volume-slider { @@ -345,6 +343,9 @@ section.course-content { border: 0; width: 5px; margin: 14px auto; + background: #666; + border: 1px solid #000; + @include box-shadow(0 1px 0 #333); a.ui-slider-handle { background: $mit-red url(../images/slider-handle.png) center center no-repeat; @@ -360,7 +361,7 @@ section.course-content { } .ui-slider-range { - background: #666; + background: #ddd; } } } From ed69957f6251a2f4b9c6a9221678b7c7ab8da01e Mon Sep 17 00:00:00 2001 From: Kyle Fiedler Date: Wed, 13 Jun 2012 15:14:34 -0400 Subject: [PATCH 65/69] Added volume images --- lms/static/images/mute.png | Bin 0 -> 185 bytes lms/static/images/volume.png | Bin 0 -> 448 bytes 2 files changed, 0 insertions(+), 0 deletions(-) create mode 100644 lms/static/images/mute.png create mode 100644 lms/static/images/volume.png diff --git a/lms/static/images/mute.png b/lms/static/images/mute.png new file mode 100644 index 0000000000000000000000000000000000000000..70a604965c53597ff1dc1cceef5bd2feb2126999 GIT binary patch literal 185 zcmeAS@N?(olHy`uVBq!ia0vp^Qa~)h!3HFsG`&3vq#8V3978H@y}jnk<&YrKnmA>C z>5T@Sl(d9oHm{q#owXW<+j;LluwpLt?y;D(FZlcq*}J>?I`%x;toXh+fX&ue#EF?r zEKH-_^P8cQ;Fbi3d0a=?RnIJ{)+jNQ-lQnKt8@cvPtS#!3|R(uCU_-2Xq&<>x(3Kv i@VITt@9EENIUlF;mg^=uRyD@`an3ww)+60)#>l; zhR8IYdCt_S_w~KrxHEIV`Yq<1_xin=dH3ED*L7_(2Ih2bTQUS3ke$I|M5X`q?ckH5 zH_%Xss0SPyTo-e_S<6RM$>013JO(^KnfM$6XB0W0#Y7YV_kiOQb%8Iipz>~z;XSLl zL{(s(-$ITSoifjWUe%qjdk59opjj~h9+V(R6b9ey623Rkn_&dzqeWy%^kI?Lh*y2Z zhNxBa)2h2>eo?fDJhR_c`3CbBTcRuW=PF-hp0*{r3=l0dcWjB0>~~eZ$^6xZs8RHB z)g_qU*bq&LzE3Us6!Uf8g5?UoGZfWA!3;=i5s86Xl^u!AHX=kubbtec>n2=p_6t4ALwOwK qAD~H!Y!Vg!%?LC5A~FePb^ZryA2Qnec4!p<0000 Date: Thu, 14 Jun 2012 12:16:29 -0400 Subject: [PATCH 66/69] Use existential operator --- lms/static/coffee/src/modules/video/video_player.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/static/coffee/src/modules/video/video_player.coffee b/lms/static/coffee/src/modules/video/video_player.coffee index 0dd52a128b..20df378267 100644 --- a/lms/static/coffee/src/modules/video/video_player.coffee +++ b/lms/static/coffee/src/modules/video/video_player.coffee @@ -135,7 +135,7 @@ class @VideoPlayer @video.speed volume: (value) -> - if value != undefined + if value? @player.setVolume value else @player.getVolume() From 5ad2824c59a47a267d180e36286475a2d7c85d02 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 7 Jun 2012 17:16:56 -0400 Subject: [PATCH 67/69] fix typo in comment in student view --- lms/djangoapps/student/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/student/views.py b/lms/djangoapps/student/views.py index eb71f5ba6a..a157da7bef 100644 --- a/lms/djangoapps/student/views.py +++ b/lms/djangoapps/student/views.py @@ -89,7 +89,7 @@ def login_user(request, error=""): @ensure_csrf_cookie def logout_user(request): - ''' HTTP request to log in the user. Redirects to marketing page''' + ''' HTTP request to log out the user. Redirects to marketing page''' logout(request) return redirect('/') From 15ac575301eb4fb63ead0ca62c4ab497ed934505 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 14 Jun 2012 11:28:52 -0400 Subject: [PATCH 68/69] added rednose to requirements (adds color output to nose) --- requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements.txt b/requirements.txt index 9dc67682fb..a62baad94b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -25,3 +25,4 @@ newrelic glob2 django_nose nosexcover +rednose From e061e8642433e95151c3b355efa09059d5f8a41f Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 14 Jun 2012 17:07:50 -0400 Subject: [PATCH 69/69] Clean and refactor courseware/views.py and module_render.py * Refactor index() so that it makes sense to me and hopefully others :) * Rename preloaded cache of student modules to student_module_cache * Fix line length and whitespace throughout * add docstrings and other comments * a few behavior-preserving tweaks to the code to make it clearer. * Separate codepaths for with-module and without-module in index view * Remove default chapter + section, since they don't exist anyway in course.xml --- lms/djangoapps/courseware/module_render.py | 227 +++++++++------- lms/djangoapps/courseware/views.py | 301 +++++++++++++-------- 2 files changed, 316 insertions(+), 212 deletions(-) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index fe6ebdd585..7c4de7d499 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -26,8 +26,29 @@ class I4xSystem(object): This is an abstraction such that x_modules can function independent of the courseware (e.g. import into other types of courseware, LMS, or if we want to have a sandbox server for user-contributed content) + + I4xSystem objects are passed to x_modules to provide access to system + functionality. ''' - def __init__(self, ajax_url, track_function, render_function, render_template, filestore=None): + def __init__(self, ajax_url, track_function, render_function, + render_template, filestore=None): + ''' + Create a closure around the system environment. + + ajax_url - the url where ajax calls to the encapsulating module go. + track_function - function of (event_type, event), intended for logging + or otherwise tracking the event. + TODO: Not used, and has inconsistent args in different + files. Update or remove. + render_function - function that takes (module_xml) and renders it, + returning a dictionary with a context for rendering the + module to html. Dictionary will contain keys 'content' + and 'type'. + render_template - a function that takes (template_file, context), and returns + rendered html. + filestore - A filestore ojbect. Defaults to an instance of OSFS based at + settings.DATA_DIR. + ''' self.ajax_url = ajax_url self.track_function = track_function if not filestore: @@ -35,37 +56,47 @@ class I4xSystem(object): else: self.filestore = filestore if settings.DEBUG: - log.info("[courseware.module_render.I4xSystem] filestore path = %s" % filestore) + log.info("[courseware.module_render.I4xSystem] filestore path = %s", + filestore) self.render_function = render_function self.render_template = render_template self.exception404 = Http404 self.DEBUG = settings.DEBUG - def get(self,attr): # uniform access to attributes (like etree) + def get(self, attr): + ''' provide uniform access to attributes (like etree).''' return self.__dict__.get(attr) - def set(self,attr,val): # uniform access to attributes (like etree) + + def set(self,attr,val): + '''provide uniform access to attributes (like etree)''' self.__dict__[attr] = val + def __repr__(self): return repr(self.__dict__) + def __str__(self): return str(self.__dict__) -def object_cache(cache, user, module_type, module_id): - # We don't look up on user -- all queries include user - # Additional lookup would require a DB hit the way Django - # is broken. +def smod_cache_lookup(cache, module_type, module_id): + ''' + Look for a student module with the given type and id in the cache. + + cache -- list of student modules + + returns first found object, or None + ''' for o in cache: - if o.module_type == module_type and \ - o.module_id == module_id: + if o.module_type == module_type and o.module_id == module_id: return o return None def make_track_function(request): ''' We want the capa problem (and other modules) to be able to track/log what happens inside them without adding dependencies on - Django or the rest of the codebase. We do this by passing a - tracking function to them. This generates a closure for each request - that gives a clean interface on both sides. + Django or the rest of the codebase. + + To do this in a clean way, we pass a tracking function to the module, + which calls it to log events. ''' import track.views @@ -75,85 +106,91 @@ def make_track_function(request): def grade_histogram(module_id): ''' Print out a histogram of grades on a given problem. - Part of staff member debug info. + Part of staff member debug info. ''' from django.db import connection cursor = connection.cursor() - cursor.execute("select courseware_studentmodule.grade,COUNT(courseware_studentmodule.student_id) from courseware_studentmodule where courseware_studentmodule.module_id=%s group by courseware_studentmodule.grade", [module_id]) + q = """SELECT courseware_studentmodule.grade, + COUNT(courseware_studentmodule.student_id) + FROM courseware_studentmodule + WHERE courseware_studentmodule.module_id=%s + GROUP BY courseware_studentmodule.grade""" + # Passing module_id this way prevents sql-injection. + cursor.execute(q, [module_id]) grades = list(cursor.fetchall()) - grades.sort(key=lambda x:x[0]) # Probably not necessary - if (len(grades) == 1 and grades[0][0] is None): + grades.sort(key=lambda x: x[0]) # Add ORDER BY to sql query? + if len(grades) == 1 and grades[0][0] is None: return [] return grades -def get_module(user, request, xml_module, module_object_preload, position=None): - ''' Get the appropriate xmodule and StudentModule. +def get_module(user, request, module_xml, student_module_cache, position=None): + ''' Get an instance of the xmodule class corresponding to module_xml, + setting the state based on an existing StudentModule, or creating one if none + exists. Arguments: - user : current django User - request : current django HTTPrequest - - xml_module : lxml etree of xml subtree for the current module - - module_object_preload : list of StudentModule objects, one of which may match this module type and id - - position : extra information from URL for user-specified position within module + - module_xml : lxml etree of xml subtree for the requested module + - student_module_cache : list of StudentModule objects, one of which may + match this module type and id + - position : extra information from URL for user-specified + position within module Returns: - a tuple (xmodule instance, student module, module type). - ''' - module_type=xml_module.tag - module_class=xmodule.get_module_class(module_type) - module_id=xml_module.get('id') #module_class.id_attribute) or "" + module_type = module_xml.tag + module_class = xmodule.get_module_class(module_type) + module_id = module_xml.get('id') - # Grab state from database - smod = object_cache(module_object_preload, - user, - module_type, - module_id) + # Grab xmodule state from StudentModule cache + smod = smod_cache_lookup(student_module_cache, module_type, module_id) + state = smod.state if smod else None - if not smod: # If nothing in the database... - state=None - else: - state = smod.state - - # get coursename if stored + # get coursename if present in request coursename = multicourse_settings.get_coursename_from_request(request) if coursename and settings.ENABLE_MULTICOURSE: - xp = multicourse_settings.get_course_xmlpath(coursename) # path to XML for the course + # path to XML for the course + xp = multicourse_settings.get_course_xmlpath(coursename) data_root = settings.DATA_DIR + xp else: data_root = settings.DATA_DIR - # Create a new instance - ajax_url = settings.MITX_ROOT_URL + '/modx/'+module_type+'/'+module_id+'/' + # Setup system context for module instance + ajax_url = settings.MITX_ROOT_URL + '/modx/' + module_type + '/' + module_id + '/' + def render_function(module_xml): + return render_x_module(user, request, module_xml, student_module_cache, position) system = I4xSystem(track_function = make_track_function(request), - render_function = lambda x: render_x_module(user, request, x, module_object_preload, position), + render_function = render_function, render_template = render_to_string, ajax_url = ajax_url, filestore = OSFS(data_root), ) - system.set('position',position) # pass URL specified position along to module, through I4xSystem - instance=module_class(system, - etree.tostring(xml_module), - module_id, - state=state) + # pass position specified in URL to module through I4xSystem + system.set('position', position) + instance = module_class(system, + etree.tostring(module_xml), + module_id, + state=state) - # If instance wasn't already in the database, and this - # isn't a guest user, create it + # If StudentModule for this instance wasn't already in the database, + # and this isn't a guest user, create it. if not smod and user.is_authenticated(): - smod=StudentModule(student=user, - module_type = module_type, - module_id=module_id, - state=instance.get_state()) + smod = StudentModule(student=user, module_type = module_type, + module_id=module_id, state=instance.get_state()) smod.save() - module_object_preload.append(smod) + # Add to cache. The caller and the system context have references + # to it, so the change persists past the return + student_module_cache.append(smod) return (instance, smod, module_type) -def render_x_module(user, request, xml_module, module_object_preload, position=None): +def render_x_module(user, request, module_xml, student_module_cache, position=None): ''' Generic module for extensions. This renders to HTML. modules include sequential, vertical, problem, video, html @@ -164,37 +201,36 @@ def render_x_module(user, request, xml_module, module_object_preload, position=N - user : current django User - request : current django HTTPrequest - - xml_module : lxml etree of xml subtree for the current module - - module_object_preload : list of StudentModule objects, one of which may match this module type and id + - module_xml : lxml etree of xml subtree for the current module + - student_module_cache : list of StudentModule objects, one of which may match this module type and id - position : extra information from URL for user-specified position within module Returns: - - dict which is context for HTML rendering of the specified module - + - dict which is context for HTML rendering of the specified module. Will have + key 'content', and will have 'type' key if passed a valid module. ''' - if xml_module==None : - return {"content":""} + if module_xml is None : + return {"content": ""} - (instance, smod, module_type) = get_module(user, request, xml_module, module_object_preload, position) + (instance, smod, module_type) = get_module( + user, request, module_xml, student_module_cache, position) - # Grab content content = instance.get_html() # special extra information about each problem, only for users who are staff if settings.MITX_FEATURES.get('DISPLAY_HISTOGRAMS_TO_STAFF') and user.is_staff: - module_id = xml_module.get('id') + module_id = module_xml.get('id') histogram = grade_histogram(module_id) render_histogram = len(histogram) > 0 - content=content+render_to_string("staff_problem_info.html", {'xml':etree.tostring(xml_module), - 'module_id' : module_id, - 'histogram': json.dumps(histogram), - 'render_histogram' : render_histogram}) + staff_context = {'xml': etree.tostring(module_xml), + 'module_id': module_id, + 'histogram': json.dumps(histogram), + 'render_histogram': render_histogram} + content += render_to_string("staff_problem_info.html", staff_context) - content = {'content':content, - 'type':module_type} - - return content + context = {'content': content, 'type': module_type} + return context def modx_dispatch(request, module=None, dispatch=None, id=None): ''' Generic view for extensions. This is where AJAX calls go. @@ -210,32 +246,38 @@ def modx_dispatch(request, module=None, dispatch=None, id=None): - id -- the module id. Used to look up the student module. e.g. filenamexformularesponse ''' + # ''' (fix emacs broken parsing) if not request.user.is_authenticated(): return redirect('/') + # python concats adjacent strings + error_msg = ("We're sorry, this module is temporarily unavailable." + "Our staff is working to fix it as soon as possible") + + # Grab the student information for the module from the database s = StudentModule.objects.filter(student=request.user, module_id=id) - #s = StudentModule.get_with_caching(request.user, id) - if len(s) == 0 or s is None: - log.debug("Couldnt find module for user and id " + str(module) + " " + str(request.user) + " "+ str(id)) + + # s = StudentModule.get_with_caching(request.user, id) + if s is None or len(s) == 0: + log.debug("Couldn't find module '%s' for user '%s' and id '%s'", + module, request.user, id) raise Http404 s = s[0] oldgrade = s.grade oldstate = s.state - # TODO: if dispatch is left at default value None, this will go boom. What's the correct - # behavior? - dispatch=dispatch.split('?')[0] + # If there are arguments, get rid of them + if '?' in dispatch: + dispatch = dispatch.split('?')[0] - ajax_url = settings.MITX_ROOT_URL + '/modx/'+module+'/'+id+'/' - - # get coursename if stored + ajax_url = '{root}/modx/{module}/{id}'.format(root = settings.MITX_ROOT_URL, + module=module, id=id) coursename = multicourse_settings.get_coursename_from_request(request) - if coursename and settings.ENABLE_MULTICOURSE: - xp = multicourse_settings.get_course_xmlpath(coursename) # path to XML for the course + xp = multicourse_settings.get_course_xmlpath(coursename) data_root = settings.DATA_DIR + xp else: data_root = settings.DATA_DIR @@ -244,11 +286,13 @@ def modx_dispatch(request, module=None, dispatch=None, id=None): try: xml = content_parser.module_xml(request.user, module, 'id', id, coursename) except: - log.exception("Unable to load module during ajax call. module=%s, dispatch=%s, id=%s" % (module, dispatch, id)) + log.exception( + "Unable to load module during ajax call. module=%s, dispatch=%s, id=%s", + module, dispatch, id) if accepts(request, 'text/html'): return render_to_response("module-error.html", {}) else: - response = HttpResponse(json.dumps({'success': "We're sorry, this module is temporarily unavailable. Our staff is working to fix it as soon as possible"})) + response = HttpResponse(json.dumps({'success': error_msg})) return response # Create the module @@ -260,24 +304,23 @@ def modx_dispatch(request, module=None, dispatch=None, id=None): ) try: - instance=xmodule.get_module_class(module)(system, - xml, - id, - state=oldstate) + module_class = xmodule.get_module_class(module) + instance = module_class(system, xml, id, state=oldstate) except: log.exception("Unable to load module instance during ajax call") if accepts(request, 'text/html'): return render_to_response("module-error.html", {}) else: - response = HttpResponse(json.dumps({'success': "We're sorry, this module is temporarily unavailable. Our staff is working to fix it as soon as possible"})) + response = HttpResponse(json.dumps({'success': error_msg})) return response # Let the module handle the AJAX - ajax_return=instance.handle_ajax(dispatch, request.POST) + ajax_return = instance.handle_ajax(dispatch, request.POST) + # Save the state back to the database - s.state=instance.get_state() + s.state = instance.get_state() if instance.get_score(): - s.grade=instance.get_score()['score'] + s.grade = instance.get_score()['score'] if s.grade != oldgrade or s.state != oldstate: s.save() # Return whatever the module wanted to return to the client/caller diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index fcd0104455..7bc8b323ce 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -41,66 +41,71 @@ def gradebook(request): coursename = multicourse_settings.get_coursename_from_request(request) student_objects = User.objects.all()[:100] - student_info = [{'username' :s.username, - 'id' : s.id, + student_info = [{'username': s.username, + 'id': s.id, 'email': s.email, - 'grade_info' : grades.grade_sheet(s,coursename), - 'realname' : UserProfile.objects.get(user = s).name + 'grade_info': grades.grade_sheet(s, coursename), + 'realname': UserProfile.objects.get(user = s).name } for s in student_objects] - return render_to_response('gradebook.html',{'students':student_info}) + return render_to_response('gradebook.html', {'students': student_info}) @login_required @cache_control(no_cache=True, no_store=True, must_revalidate=True) -def profile(request, student_id = None): +def profile(request, student_id=None): ''' User profile. Show username, location, etc, as well as grades . We need to allow the user to change some of these settings .''' if student_id is None: student = request.user - else: + else: if 'course_admin' not in content_parser.user_groups(request.user): raise Http404 student = User.objects.get( id = int(student_id)) - user_info = UserProfile.objects.get(user=student) # request.user.profile_cache # + user_info = UserProfile.objects.get(user=student) # request.user.profile_cache # coursename = multicourse_settings.get_coursename_from_request(request) - context={'name':user_info.name, - 'username':student.username, - 'location':user_info.location, - 'language':user_info.language, - 'email':student.email, - 'format_url_params' : content_parser.format_url_params, - 'csrf':csrf(request)['csrf_token'] - } - context.update(grades.grade_sheet(student,coursename)) + context = {'name': user_info.name, + 'username': student.username, + 'location': user_info.location, + 'language': user_info.language, + 'email': student.email, + 'format_url_params': content_parser.format_url_params, + 'csrf': csrf(request)['csrf_token'] + } + context.update(grades.grade_sheet(student, coursename)) return render_to_response('profile.html', context) -def render_accordion(request,course,chapter,section): + +def render_accordion(request, course, chapter, section): ''' Draws navigation bar. Takes current position in accordion as parameter. Returns (initialization_javascript, content)''' if not course: course = "6.002 Spring 2012" - toc=content_parser.toc_from_xml(content_parser.course_file(request.user,course), chapter, section) - active_chapter=1 + toc = content_parser.toc_from_xml( + content_parser.course_file(request.user, course), chapter, section) + + active_chapter = 1 for i in range(len(toc)): if toc[i]['active']: - active_chapter=i - context=dict([['active_chapter',active_chapter], - ['toc',toc], - ['course_name',course], - ['format_url_params',content_parser.format_url_params], - ['csrf',csrf(request)['csrf_token']]] + \ + active_chapter = i + + context=dict([('active_chapter', active_chapter), + ('toc', toc), + ('course_name', course), + ('format_url_params', content_parser.format_url_params), + ('csrf', csrf(request)['csrf_token'])] + template_imports.items()) - return render_to_string('accordion.html',context) + return render_to_string('accordion.html', context) + @cache_control(no_cache=True, no_store=True, must_revalidate=True) def render_section(request, section): - ''' TODO: Consolidate with index + ''' TODO: Consolidate with index ''' user = request.user if not settings.COURSEWARE_ENABLED: @@ -120,15 +125,15 @@ def render_section(request, section): } module_ids = dom.xpath("//@id") - + if user.is_authenticated(): - module_object_preload = list(StudentModule.objects.filter(student=user, + student_module_cache = list(StudentModule.objects.filter(student=user, module_id__in=module_ids)) else: - module_object_preload = [] - + student_module_cache = [] + try: - module = render_x_module(user, request, dom, module_object_preload) + module = render_x_module(user, request, dom, student_module_cache) except: log.exception("Unable to load module") context.update({ @@ -138,18 +143,67 @@ def render_section(request, section): return render_to_response('courseware.html', context) context.update({ - 'init':module.get('init_js', ''), - 'content':module['content'], + 'init': module.get('init_js', ''), + 'content': module['content'], }) result = render_to_response('courseware.html', context) return result +def get_course(request, course): + ''' Figure out what the correct course is. + + Needed to preserve backwards compatibility with non-multi-course version. + TODO: Can this go away once multicourse becomes standard? + ''' + + if course==None: + if not settings.ENABLE_MULTICOURSE: + course = "6.002 Spring 2012" + elif 'coursename' in request.session: + course = request.session['coursename'] + else: + course = settings.COURSE_DEFAULT + return course + +def get_module_xml(user, course, chapter, section): + ''' Look up the module xml for the given course/chapter/section path. + + Takes the user to look up the course file. + + Returns None if there was a problem, or the lxml etree for the module. + ''' + try: + # this is the course.xml etree + dom = content_parser.course_file(user, course) + except: + log.exception("Unable to parse courseware xml") + return None + + # this is the module's parent's etree + path = "//course[@name=$course]/chapter[@name=$chapter]//section[@name=$section]" + dom_module = dom.xpath(path, course=course, chapter=chapter, section=section) + + module_wrapper = dom_module[0] if len(dom_module) > 0 else None + if module_wrapper is None: + module = None + elif module_wrapper.get("src"): + module = content_parser.section_file( + user=user, section=module_wrapper.get("src"), coursename=course) + else: + # Copy the element out of the module's etree + module = etree.XML(etree.tostring(module_wrapper[0])) + return module + @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) -def index(request, course=None, chapter="Using the System", section="Hints",position=None): +def index(request, course=None, chapter=None, section=None, + position=None): ''' Displays courseware accordion, and any associated content. + If course, chapter, and section aren't all specified, just returns + the accordion. If they are specified, returns an error if they don't + point to a valid module. Arguments: @@ -162,110 +216,113 @@ def index(request, course=None, chapter="Using the System", section="Hints",posi Returns: - HTTPresponse + ''' + def clean(s): + ''' Fixes URLs -- we convert spaces to _ in URLs to prevent + funny encoding characters and keep the URLs readable. This undoes + that transformation. - ''' - user = request.user + TODO: Properly replace underscores. (Q: what is properly?) + ''' + return s.replace('_', ' ') + + def get_submodule_ids(module_xml): + ''' + Get a list with ids of the modules within this module. + ''' + return module_xml.xpath("//@id") + + def preload_student_modules(module_xml): + ''' + Find any StudentModule objects for this user that match + one of the given module_ids. Used as a cache to avoid having + each rendered module hit the db separately. + + Returns the list, or None on error. + ''' + if request.user.is_authenticated(): + module_ids = get_submodule_ids(module_xml) + return list(StudentModule.objects.filter(student=request.user, + module_id__in=module_ids)) + else: + return [] + + def get_module_context(): + ''' + Look up the module object and render it. If all goes well, returns + {'init': module-init-js, 'content': module-rendered-content} + + If there's an error, returns + {'content': module-error message} + ''' + # Can't modify variables of outer scope, so need new ones + chapter_ = clean(chapter) + section_ = clean(section) + + user = request.user + + module_xml = get_module_xml(user, course, chapter_, section_) + if module_xml is None: + log.exception("couldn't get module_xml: course/chapter/section: '%s/%s/%s'", + course, chapter_, section_) + return {'content' : render_to_string("module-error.html", {})} + + student_module_cache = preload_student_modules(module_xml) + + try: + module_context = render_x_module(user, request, module_xml, + student_module_cache, position) + except: + log.exception("Unable to load module") + return {'content' : render_to_string("module-error.html", {})} + + return {'init': module_context.get('init_js', ''), + 'content': module_context['content']} + if not settings.COURSEWARE_ENABLED: return redirect('/') - if course==None: - if not settings.ENABLE_MULTICOURSE: - course = "6.002 Spring 2012" - elif 'coursename' in request.session: - course = request.session['coursename'] - else: - course = settings.COURSE_DEFAULT - - # Fixes URLs -- we don't get funny encoding characters from spaces - # so they remain readable - ## TODO: Properly replace underscores - course=course.replace("_"," ") - chapter=chapter.replace("_"," ") - section=section.replace("_"," ") - - # use multicourse module to determine if "course" is valid - #if course!=settings.COURSE_NAME.replace('_',' '): + course = clean(get_course(request, course)) if not multicourse_settings.is_valid_course(course): return redirect('/') - request.session['coursename'] = course # keep track of current course being viewed in django's request.session - - try: - # this is the course.xml etree - dom = content_parser.course_file(user,course) # also pass course to it, for course-specific XML path - except: - log.exception("Unable to parse courseware xml") - return render_to_response('courseware-error.html', {}) - - # this is the module's parent's etree - dom_module = dom.xpath("//course[@name=$course]/chapter[@name=$chapter]//section[@name=$section]", - course=course, chapter=chapter, section=section) - - #print "DM", dom_module - - if len(dom_module) == 0: - module_wrapper = None - else: - module_wrapper = dom_module[0] - - if module_wrapper is None: - module = None - elif module_wrapper.get("src"): - module = content_parser.section_file(user=user, section=module_wrapper.get("src"), coursename=course) - else: - # this is the module's etree - module = etree.XML(etree.tostring(module_wrapper[0])) # Copy the element out of the tree - - module_ids = [] - if module is not None: - module_ids = module.xpath("//@id", - course=course, chapter=chapter, section=section) - - if user.is_authenticated(): - module_object_preload = list(StudentModule.objects.filter(student=user, - module_id__in=module_ids)) - else: - module_object_preload = [] + # keep track of current course being viewed in django's request.session + request.session['coursename'] = course context = { 'csrf': csrf(request)['csrf_token'], 'accordion': render_accordion(request, course, chapter, section), - 'COURSE_TITLE':multicourse_settings.get_course_title(course), + 'COURSE_TITLE': multicourse_settings.get_course_title(course), + 'init': '', + 'content': '' } - try: - module_context = render_x_module(user, request, module, module_object_preload, position) - except: - log.exception("Unable to load module") - context.update({ - 'init': '', - 'content': render_to_string("module-error.html", {}), - }) - return render_to_response('courseware.html', context) - - context.update({ - 'init': module_context.get('init_js', ''), - 'content': module_context['content'], - }) + look_for_module = chapter is not None and section is not None + if look_for_module: + context.update(get_module_context()) result = render_to_response('courseware.html', context) return result def jump_to(request, probname=None): ''' - Jump to viewing a specific problem. The problem is specified by a problem name - currently the filename (minus .xml) - of the problem. Maybe this should change to a more generic tag, eg "name" given as an attribute in . + Jump to viewing a specific problem. The problem is specified by a + problem name - currently the filename (minus .xml) of the problem. + Maybe this should change to a more generic tag, eg "name" given as + an attribute in . - We do the jump by (1) reading course.xml to find the first instance of with the given filename, then - (2) finding the parent element of the problem, then (3) rendering that parent element with a specific computed position - value (if it is ). + We do the jump by (1) reading course.xml to find the first + instance of with the given filename, then (2) finding + the parent element of the problem, then (3) rendering that parent + element with a specific computed position value (if it is + ). ''' # get coursename if stored coursename = multicourse_settings.get_coursename_from_request(request) # begin by getting course.xml tree - xml = content_parser.course_file(request.user,coursename) + xml = content_parser.course_file(request.user, coursename) # look for problem of given name pxml = xml.xpath('//problem[@filename="%s"]' % probname) @@ -279,12 +336,16 @@ def jump_to(request, probname=None): section = None branch = parent for k in range(4): # max depth of recursion - if branch.tag=='section': section = branch.get('name') - if branch.tag=='chapter': chapter = branch.get('name') + if branch.tag == 'section': + section = branch.get('name') + if branch.tag == 'chapter': + chapter = branch.get('name') branch = branch.getparent() position = None - if parent.tag=='sequential': - position = parent.index(pxml)+1 # position in sequence - - return index(request,course=coursename,chapter=chapter,section=section,position=position) + if parent.tag == 'sequential': + position = parent.index(pxml) + 1 # position in sequence + + return index(request, + course=coursename, chapter=chapter, + section=section, position=position)