diff --git a/common/lib/xmodule/xmodule/js/src/selfassessment/display.coffee b/common/lib/xmodule/xmodule/js/src/selfassessment/display.coffee
index 951eb42fce..5b70ab29aa 100644
--- a/common/lib/xmodule/xmodule/js/src/selfassessment/display.coffee
+++ b/common/lib/xmodule/xmodule/js/src/selfassessment/display.coffee
@@ -120,7 +120,7 @@ class @SelfAssessment
if @state == 'done'
$.postWithPrefix "#{@ajax_url}/reset", {}, (response) =>
if response.success
- @answer_area.html('')
+ @answer_area.val('')
@rubric_wrapper.html('')
@hint_wrapper.html('')
@message_wrapper.html('')
diff --git a/common/lib/xmodule/xmodule/self_assessment_module.py b/common/lib/xmodule/xmodule/self_assessment_module.py
index 2edf5467b2..8498a210cd 100644
--- a/common/lib/xmodule/xmodule/self_assessment_module.py
+++ b/common/lib/xmodule/xmodule/self_assessment_module.py
@@ -7,20 +7,21 @@ Parses xml definition file--see below for exact format.
import copy
from fs.errors import ResourceNotFoundError
+import itertools
+import json
import logging
-import os
-import sys
from lxml import etree
from lxml.html import rewrite_links
from path import path
-import json
-from progress import Progress
+import os
+import sys
from pkg_resources import resource_string
from .capa_module import only_one, ComplexEncoder
from .editing_module import EditingDescriptor
from .html_checker import check_html
+from progress import Progress
from .stringify import stringify_children
from .x_module import XModule
from .xml_module import XmlDescriptor
@@ -52,6 +53,8 @@ class SelfAssessmentModule(XModule):
submissions too.)
"""
+ STATE_VERSION = 1
+
# states
INITIAL = 'initial'
ASSESSING = 'assessing'
@@ -102,35 +105,130 @@ class SelfAssessmentModule(XModule):
else:
instance_state = {}
- # Note: score responses are on scale from 0 to max_score
- self.student_answers = instance_state.get('student_answers', [])
- self.scores = instance_state.get('scores', [])
- self.hints = instance_state.get('hints', [])
+ instance_state = self.convert_state_to_current_format(instance_state)
+
+ # History is a list of tuples of (answer, score, hint), where hint may be
+ # None for any element, and score and hint can be None for the last (current)
+ # element.
+ # Scores are on scale from 0 to max_score
+ self.history = instance_state.get('history', [])
self.state = instance_state.get('state', 'initial')
+ self.attempts = instance_state.get('attempts', 0)
+ self.max_attempts = int(self.metadata.get('attempts', MAX_ATTEMPTS))
+
# Used for progress / grading. Currently get credit just for
# completion (doesn't matter if you self-assessed correct/incorrect).
-
self._max_score = int(self.metadata.get('max_score', MAX_SCORE))
- self.attempts = instance_state.get('attempts', 0)
-
- self.max_attempts = int(self.metadata.get('attempts', MAX_ATTEMPTS))
-
self.rubric = definition['rubric']
self.prompt = definition['prompt']
self.submit_message = definition['submitmessage']
self.hint_prompt = definition['hintprompt']
+
+ def latest_answer(self):
+ """None if not available"""
+ if not self.history:
+ return None
+ return self.history[-1].get('answer')
+
+ def latest_score(self):
+ """None if not available"""
+ if not self.history:
+ return None
+ return self.history[-1].get('score')
+
+ def latest_hint(self):
+ """None if not available"""
+ if not self.history:
+ return None
+ return self.history[-1].get('hint')
+
+ def new_history_entry(self, answer):
+ self.history.append({'answer': answer})
+
+ def record_latest_score(self, score):
+ """Assumes that state is right, so we're adding a score to the latest
+ history element"""
+ self.history[-1]['score'] = score
+
+ def record_latest_hint(self, hint):
+ """Assumes that state is right, so we're adding a score to the latest
+ history element"""
+ self.history[-1]['hint'] = hint
+
+
+ def change_state(self, new_state):
+ """
+ A centralized place for state changes--allows for hooks. If the
+ current state matches the old state, don't run any hooks.
+ """
+ if self.state == new_state:
+ return
+
+ self.state = new_state
+
+ if self.state == self.DONE:
+ self.attempts += 1
+
+ @staticmethod
+ def convert_state_to_current_format(old_state):
+ """
+ This module used to use a problematic state representation. This method
+ converts that into the new format.
+
+ Args:
+ old_state: dict of state, as passed in. May be old.
+
+ Returns:
+ new_state: dict of new state
+ """
+ if old_state.get('version', 0) == SelfAssessmentModule.STATE_VERSION:
+ # already current
+ return old_state
+
+ # for now, there's only one older format.
+
+ new_state = {'version': SelfAssessmentModule.STATE_VERSION}
+
+ def copy_if_present(key):
+ if key in old_state:
+ new_state[key] = old_state[key]
+
+ for to_copy in ['attempts', 'state']:
+ copy_if_present(to_copy)
+
+ # The answers, scores, and hints need to be kept together to avoid them
+ # getting out of sync.
+
+ # NOTE: Since there's only one problem with a few hundred submissions
+ # in production so far, not trying to be smart about matching up hints
+ # and submissions in cases where they got out of sync.
+
+ student_answers = old_state.get('student_answers', [])
+ scores = old_state.get('scores', [])
+ hints = old_state.get('hints', [])
+
+ new_state['history'] = [
+ {'answer': answer,
+ 'score': score,
+ 'hint': hint}
+ for answer, score, hint in itertools.izip_longest(
+ student_answers, scores, hints)]
+ return new_state
+
+
def _allow_reset(self):
"""Can the module be reset?"""
return self.state == self.DONE and self.attempts < self.max_attempts
def get_html(self):
#set context variables and render template
- if self.state != self.INITIAL and self.student_answers:
- previous_answer = self.student_answers[-1]
+ if self.state != self.INITIAL:
+ latest = self.latest_answer()
+ previous_answer = latest if latest is not None else ''
else:
previous_answer = ''
@@ -149,26 +247,19 @@ class SelfAssessmentModule(XModule):
# cdodge: perform link substitutions for any references to course static content (e.g. images)
return rewrite_links(html, self.rewrite_content_links)
- def get_score(self):
- """
- Returns dict with 'score' key
- """
- return {'score': self.get_last_score()}
-
def max_score(self):
"""
Return max_score
"""
return self._max_score
- def get_last_score(self):
+ def get_score(self):
"""
Returns the last score in the list
"""
- last_score=0
- if(len(self.scores)>0):
- last_score=self.scores[len(self.scores)-1]
- return last_score
+ score = self.latest_score()
+ return {'score': score if score is not None else 0,
+ 'total': self._max_score}
def get_progress(self):
'''
@@ -176,7 +267,7 @@ class SelfAssessmentModule(XModule):
'''
if self._max_score > 0:
try:
- return Progress(self.get_last_score(), self._max_score)
+ return Progress(self.get_score()['score'], self._max_score)
except Exception as err:
log.exception("Got bad progress")
return None
@@ -250,9 +341,10 @@ class SelfAssessmentModule(XModule):
if self.state in (self.INITIAL, self.ASSESSING):
return ''
- if self.state == self.DONE and len(self.hints) > 0:
+ if self.state == self.DONE:
# display the previous hint
- hint = self.hints[-1]
+ latest = self.latest_hint()
+ hint = latest if latest is not None else ''
else:
hint = ''
@@ -295,8 +387,9 @@ class SelfAssessmentModule(XModule):
if self.state != self.INITIAL:
return self.out_of_sync_error(get)
- self.student_answers.append(get['student_answer'])
- self.state = self.ASSESSING
+ # add new history element with answer and empty score and hint.
+ self.new_history_entry(get['student_answer'])
+ self.change_state(self.ASSESSING)
return {
'success': True,
@@ -318,27 +411,24 @@ class SelfAssessmentModule(XModule):
'message_html' only if success is true
"""
- n_answers = len(self.student_answers)
- n_scores = len(self.scores)
- if (self.state != self.ASSESSING or n_answers != n_scores + 1):
- msg = "%d answers, %d scores" % (n_answers, n_scores)
- return self.out_of_sync_error(get, msg)
+ if self.state != self.ASSESSING:
+ return self.out_of_sync_error(get)
try:
score = int(get['assessment'])
- except:
+ except ValueError:
return {'success': False, 'error': "Non-integer score value"}
- self.scores.append(score)
+ self.record_latest_score(score)
d = {'success': True,}
if score == self.max_score():
- self.state = self.DONE
+ self.change_state(self.DONE)
d['message_html'] = self.get_message_html()
d['allow_reset'] = self._allow_reset()
else:
- self.state = self.REQUEST_HINT
+ self.change_state(self.REQUEST_HINT)
d['hint_html'] = self.get_hint_html()
d['state'] = self.state
@@ -360,19 +450,15 @@ class SelfAssessmentModule(XModule):
# the same number of hints and answers.
return self.out_of_sync_error(get)
- self.hints.append(get['hint'].lower())
- self.state = self.DONE
-
- # increment attempts
- self.attempts = self.attempts + 1
+ self.record_latest_hint(get['hint'])
+ self.change_state(self.DONE)
# To the tracking logs!
event_info = {
'selfassessment_id': self.location.url(),
'state': {
- 'student_answers': self.student_answers,
- 'score': self.scores,
- 'hints': self.hints,
+ 'version': self.STATE_VERSION,
+ 'history': self.history,
}
}
self.system.track_function('save_hint', event_info)
@@ -397,7 +483,7 @@ class SelfAssessmentModule(XModule):
'success': False,
'error': 'Too many attempts.'
}
- self.state = self.INITIAL
+ self.change_state(self.INITIAL)
return {'success': True}
@@ -407,12 +493,11 @@ class SelfAssessmentModule(XModule):
"""
state = {
- 'student_answers': self.student_answers,
- 'hints': self.hints,
+ 'version': self.STATE_VERSION,
+ 'history': self.history,
'state': self.state,
- 'scores': self.scores,
'max_score': self._max_score,
- 'attempts': self.attempts
+ 'attempts': self.attempts,
}
return json.dumps(state)
diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py
index ed64c45118..a07f1ddfaf 100644
--- a/common/lib/xmodule/xmodule/tests/__init__.py
+++ b/common/lib/xmodule/xmodule/tests/__init__.py
@@ -4,7 +4,7 @@ unittests for xmodule
Run like this:
rake test_common/lib/xmodule
-
+
"""
import unittest
@@ -19,11 +19,12 @@ import xmodule
from xmodule.x_module import ModuleSystem
from mock import Mock
-i4xs = ModuleSystem(
+test_system = ModuleSystem(
ajax_url='courses/course_id/modx/a_location',
track_function=Mock(),
get_module=Mock(),
- render_template=Mock(),
+ # "render" to just the context...
+ render_template=lambda template, context: str(context),
replace_urls=Mock(),
user=Mock(),
filestore=Mock(),
diff --git a/common/lib/xmodule/xmodule/tests/test_progress.py b/common/lib/xmodule/xmodule/tests/test_progress.py
index 94a0a19d7c..cb011cdc2b 100644
--- a/common/lib/xmodule/xmodule/tests/test_progress.py
+++ b/common/lib/xmodule/xmodule/tests/test_progress.py
@@ -5,7 +5,7 @@ import unittest
from xmodule.progress import Progress
from xmodule import x_module
-from . import i4xs
+from . import test_system
class ProgressTest(unittest.TestCase):
''' Test that basic Progress objects work. A Progress represents a
@@ -133,6 +133,6 @@ class ModuleProgressTest(unittest.TestCase):
'''
def test_xmodule_default(self):
'''Make sure default get_progress exists, returns None'''
- xm = x_module.XModule(i4xs, 'a://b/c/d/e', None, {})
+ xm = x_module.XModule(test_system, 'a://b/c/d/e', None, {})
p = xm.get_progress()
self.assertEqual(p, None)
diff --git a/common/lib/xmodule/xmodule/tests/test_self_assessment.py b/common/lib/xmodule/xmodule/tests/test_self_assessment.py
new file mode 100644
index 0000000000..d89190b1e0
--- /dev/null
+++ b/common/lib/xmodule/xmodule/tests/test_self_assessment.py
@@ -0,0 +1,54 @@
+import json
+from mock import Mock
+import unittest
+
+from xmodule.self_assessment_module import SelfAssessmentModule
+from xmodule.modulestore import Location
+
+from . import test_system
+
+class SelfAssessmentTest(unittest.TestCase):
+
+ definition = {'rubric': 'A rubric',
+ 'prompt': 'Who?',
+ 'submitmessage': 'Shall we submit now?',
+ 'hintprompt': 'Consider this...',
+ }
+
+ location = Location(["i4x", "edX", "sa_test", "selfassessment",
+ "SampleQuestion"])
+
+ metadata = {'attempts': '10'}
+
+ descriptor = Mock()
+
+ def test_import(self):
+ state = json.dumps({'student_answers': ["Answer 1", "answer 2", "answer 3"],
+ 'scores': [0, 1],
+ 'hints': ['o hai'],
+ 'state': SelfAssessmentModule.ASSESSING,
+ 'attempts': 2})
+
+ module = SelfAssessmentModule(test_system, self.location,
+ self.definition, self.descriptor,
+ state, {}, metadata=self.metadata)
+
+ self.assertEqual(module.get_score()['score'], 0)
+
+ self.assertTrue('answer 3' in module.get_html())
+ self.assertFalse('answer 2' in module.get_html())
+
+ module.save_assessment({'assessment': '0'})
+ self.assertEqual(module.state, module.REQUEST_HINT)
+
+ module.save_hint({'hint': 'hint for ans 3'})
+ self.assertEqual(module.state, module.DONE)
+
+ d = module.reset({})
+ self.assertTrue(d['success'])
+ self.assertEqual(module.state, module.INITIAL)
+
+ # if we now assess as right, skip the REQUEST_HINT state
+ module.save_answer({'student_answer': 'answer 4'})
+ module.save_assessment({'assessment': '1'})
+ self.assertEqual(module.state, module.DONE)
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index 2b2e709bcb..6f3fb73356 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -233,17 +233,17 @@ class XModule(HTMLSnippet):
self._loaded_children = [c for c in children if c is not None]
return self._loaded_children
-
+
def get_children_locations(self):
'''
Returns the locations of each of child modules.
-
+
Overriding this changes the behavior of get_children and
anything that uses get_children, such as get_display_items.
-
+
This method will not instantiate the modules of the children
unless absolutely necessary, so it is cheaper to call than get_children
-
+
These children will be the same children returned by the
descriptor unless descriptor.has_dynamic_children() is true.
'''
@@ -288,8 +288,20 @@ class XModule(HTMLSnippet):
return '{}'
def get_score(self):
- ''' Score the student received on the problem.
- '''
+ """
+ Score the student received on the problem, or None if there is no
+ score.
+
+ Returns:
+ dictionary
+ {'score': integer, from 0 to get_max_score(),
+ 'total': get_max_score()}
+
+ NOTE (vshnayder): not sure if this was the intended return value, but
+ that's what it's doing now. I suspect that we really want it to just
+ return a number. Would need to change (at least) capa and
+ modx_dispatch to match if we did that.
+ """
return None
def max_score(self):
@@ -319,7 +331,7 @@ class XModule(HTMLSnippet):
get is a dictionary-like object '''
return ""
- # cdodge: added to support dynamic substitutions of
+ # cdodge: added to support dynamic substitutions of
# links for courseware assets (e.g. images). is passed through from lxml.html parser
def rewrite_content_links(self, link):
# see if we start with our format, e.g. 'xasset:'
@@ -408,7 +420,7 @@ class XModuleDescriptor(Plugin, HTMLSnippet, ResourceTemplates):
# cdodge: this is a list of metadata names which are 'system' metadata
# and should not be edited by an end-user
system_metadata_fields = [ 'data_dir' ]
-
+
# A list of descriptor attributes that must be equal for the descriptors to
# be equal
equality_attributes = ('definition', 'metadata', 'location',
@@ -562,18 +574,18 @@ class XModuleDescriptor(Plugin, HTMLSnippet, ResourceTemplates):
self,
metadata=self.metadata
)
-
-
+
+
def has_dynamic_children(self):
"""
Returns True if this descriptor has dynamic children for a given
student when the module is created.
-
+
Returns False if the children of this descriptor are the same
- children that the module will return for any student.
+ children that the module will return for any student.
"""
return False
-
+
# ================================= JSON PARSING ===========================
@staticmethod