From ba8781ddf460694b9cfabc4773c150a351491a2d Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Mon, 12 Nov 2012 14:07:06 -0500 Subject: [PATCH 1/6] Modify to store scores instead of correct/incorrect, remove student ability to edit response --- .../js/src/selfassessment/display.coffee | 1 + .../xmodule/xmodule/self_assessment_module.py | 60 ++++++++++++------- 2 files changed, 38 insertions(+), 23 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/src/selfassessment/display.coffee b/common/lib/xmodule/xmodule/js/src/selfassessment/display.coffee index 51d6b76ebb..9f10357234 100644 --- a/common/lib/xmodule/xmodule/js/src/selfassessment/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/selfassessment/display.coffee @@ -59,6 +59,7 @@ class @SelfAssessment save_answer: (event) => event.preventDefault() if @state == 'initial' + @answer_area.attr("disabled",true) data = {'student_answer' : @answer_area.val()} $.postWithPrefix "#{@ajax_url}/save_answer", data, (response) => if response.success diff --git a/common/lib/xmodule/xmodule/self_assessment_module.py b/common/lib/xmodule/xmodule/self_assessment_module.py index f208cd9c7e..46bfcc8aa1 100644 --- a/common/lib/xmodule/xmodule/self_assessment_module.py +++ b/common/lib/xmodule/xmodule/self_assessment_module.py @@ -33,8 +33,8 @@ log = logging.getLogger("mitx.courseware") # attempts specified in xml definition overrides this. MAX_ATTEMPTS = 1 -# Set maximum available number of points. Should be set to 1 for now due to assessment handling, -# which only allows for correct/incorrect. +# Set maximum available number of points. +# Overriden by max_score specified in xml. MAX_SCORE = 1 class SelfAssessmentModule(XModule): @@ -68,13 +68,19 @@ class SelfAssessmentModule(XModule): """ Definition file should have 4 blocks -- prompt, rubric, submitmessage, hintprompt, - and one optional attribute, attempts, which should be an integer that defaults to 1. + and two optional attributes: + attempts, which should be an integer that defaults to 1. If it's > 1, the student will be able to re-submit after they see - the rubric. Note: all the submissions are stored. + the rubric. + max_score, which should be an integer that defaults to 1. + It defines the maximum number of points a student can get. Assumed to be integer scale + from 0 to max_score, with an interval of 1. + + Note: all the submissions are stored. Sample file: - + Insert prompt text here. (arbitrary html) @@ -96,16 +102,16 @@ class SelfAssessmentModule(XModule): else: instance_state = {} - # Note: assessment responses are 'incorrect'/'correct' + # Note: score responses are on scale from 0 to max_score self.student_answers = instance_state.get('student_answers', []) - self.assessment = instance_state.get('assessment', []) + self.scores = instance_state.get('scores', []) self.hints = instance_state.get('hints', []) self.state = instance_state.get('state', 'initial') # Used for progress / grading. Currently get credit just for # completion (doesn't matter if you self-assessed correct/incorrect). - self.score = instance_state.get('score', 0) + self._max_score = instance_state.get('max_score', MAX_SCORE) self.attempts = instance_state.get('attempts', 0) @@ -131,6 +137,7 @@ class SelfAssessmentModule(XModule): 'initial_message': self.get_message_html(), 'state': self.state, 'allow_reset': allow_reset, + 'max_score' : self._max_score, } html = self.system.render_template('self_assessment_prompt.html', context) @@ -141,7 +148,7 @@ class SelfAssessmentModule(XModule): """ Returns dict with 'score' key """ - return {'score': self.score} + return {'score': self.get_last_score()} def max_score(self): """ @@ -149,13 +156,22 @@ class SelfAssessmentModule(XModule): """ return self._max_score + def get_last_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 + def get_progress(self): ''' - For now, just return score / max_score + For now, just return last score / max_score ''' if self._max_score > 0: try: - return Progress(self.score, self._max_score) + return Progress(self.get_last_score(), self._max_score) except Exception as err: log.exception("Got bad progress") return None @@ -285,10 +301,14 @@ class SelfAssessmentModule(XModule): """ if (self.state != self.ASSESSING or - len(self.student_answers) != len(self.assessment) + 1): + len(self.student_answers) != len(self.scores) + 1): return self.out_of_sync_error(get) - self.assessment.append(get['assessment'].lower()) + try: + self.scores.append(int(get['assessment'])) + except: + return {'success': False, 'error': "Non-integer score value"} + self.state = self.REQUEST_HINT # TODO: return different hint based on assessment value... @@ -304,14 +324,12 @@ class SelfAssessmentModule(XModule): with the error key only present if success is False and message_html only if True. ''' - if self.state != self.REQUEST_HINT or len(self.assessment) != len(self.hints) + 1: + if self.state != self.REQUEST_HINT or len(self.scores) != len(self.hints) + 1: return self.out_of_sync_error(get) self.hints.append(get['hint'].lower()) self.state = self.DONE - # Points are assigned for completion, so always set to 1 - points = 1 # increment attempts self.attempts = self.attempts + 1 @@ -320,9 +338,8 @@ class SelfAssessmentModule(XModule): 'selfassessment_id': self.location.url(), 'state': { 'student_answers': self.student_answers, - 'assessment': self.assessment, + 'score': self.scores, 'hints': self.hints, - 'score': points, } } self.system.track_function('save_hint', event_info) @@ -353,17 +370,14 @@ class SelfAssessmentModule(XModule): def get_instance_state(self): """ - Get the current assessment, points, and state + Get the current score and state """ - #Assign points based on completion. May want to change to assessment-based down the road. - points = 1 state = { 'student_answers': self.student_answers, - 'assessment': self.assessment, 'hints': self.hints, 'state': self.state, - 'score': points, + 'scores': self.scores, 'max_score': self._max_score, 'attempts': self.attempts } From d5702fc712194f0ee0fa242cea463d0428d13c33 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Mon, 12 Nov 2012 14:19:04 -0500 Subject: [PATCH 2/6] Add in ability to define score points --- common/lib/xmodule/xmodule/self_assessment_module.py | 7 ++++--- lms/templates/self_assessment_rubric.html | 5 +++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/common/lib/xmodule/xmodule/self_assessment_module.py b/common/lib/xmodule/xmodule/self_assessment_module.py index 46bfcc8aa1..699cd0f9ac 100644 --- a/common/lib/xmodule/xmodule/self_assessment_module.py +++ b/common/lib/xmodule/xmodule/self_assessment_module.py @@ -112,7 +112,7 @@ class SelfAssessmentModule(XModule): # Used for progress / grading. Currently get credit just for # completion (doesn't matter if you self-assessed correct/incorrect). - self._max_score = instance_state.get('max_score', MAX_SCORE) + self._max_score = int(self.metadata.get('max_score', MAX_SCORE)) self.attempts = instance_state.get('attempts', 0) @@ -137,7 +137,6 @@ class SelfAssessmentModule(XModule): 'initial_message': self.get_message_html(), 'state': self.state, 'allow_reset': allow_reset, - 'max_score' : self._max_score, } html = self.system.render_template('self_assessment_prompt.html', context) @@ -225,7 +224,9 @@ class SelfAssessmentModule(XModule): return '' # we'll render it - context = {'rubric': self.rubric} + context = {'rubric': self.rubric, + 'max_score' : self._max_score, + } if self.state == self.ASSESSING: context['read_only'] = False diff --git a/lms/templates/self_assessment_rubric.html b/lms/templates/self_assessment_rubric.html index 479c19a027..5bcb3bba93 100644 --- a/lms/templates/self_assessment_rubric.html +++ b/lms/templates/self_assessment_rubric.html @@ -6,8 +6,9 @@ % if not read_only: % endif From 39210fa95f6075c4f8da2863d6537b693b10fc31 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Mon, 12 Nov 2012 14:20:44 -0500 Subject: [PATCH 3/6] Move point where input box is disabled --- common/lib/xmodule/xmodule/js/src/selfassessment/display.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/js/src/selfassessment/display.coffee b/common/lib/xmodule/xmodule/js/src/selfassessment/display.coffee index 9f10357234..a16a6b3d58 100644 --- a/common/lib/xmodule/xmodule/js/src/selfassessment/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/selfassessment/display.coffee @@ -59,10 +59,10 @@ class @SelfAssessment save_answer: (event) => event.preventDefault() if @state == 'initial' - @answer_area.attr("disabled",true) data = {'student_answer' : @answer_area.val()} $.postWithPrefix "#{@ajax_url}/save_answer", data, (response) => if response.success + @answer_area.attr("disabled",true) @rubric_wrapper.html(response.rubric_html) @state = 'assessing' @find_assessment_elements() From 6e5a6f9753401674d708fd64cabd0776a85e4d2c Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 12 Nov 2012 16:23:29 -0500 Subject: [PATCH 4/6] make textboxes read-only when that makes sense. - also make reset hide the original output --- .../js/src/selfassessment/display.coffee | 9 +++++-- .../xmodule/xmodule/self_assessment_module.py | 27 ++++++++++++------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/src/selfassessment/display.coffee b/common/lib/xmodule/xmodule/js/src/selfassessment/display.coffee index a16a6b3d58..a40c405e4a 100644 --- a/common/lib/xmodule/xmodule/js/src/selfassessment/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/selfassessment/display.coffee @@ -21,7 +21,6 @@ class @SelfAssessment @find_assessment_elements() @find_hint_elements() - @rebind() # locally scoped jquery. @@ -33,16 +32,22 @@ class @SelfAssessment @submit_button.unbind('click') @submit_button.show() @reset_button.hide() + @hint_area.attr('disabled', false) if @state == 'initial' + @answer_area.attr("disabled", false) @submit_button.prop('value', 'Submit') @submit_button.click @save_answer else if @state == 'assessing' + @answer_area.attr("disabled", true) @submit_button.prop('value', 'Submit assessment') @submit_button.click @save_assessment else if @state == 'request_hint' + @answer_area.attr("disabled", true) @submit_button.prop('value', 'Submit hint') @submit_button.click @save_hint else if @state == 'done' + @answer_area.attr("disabled", true) + @hint_area.attr('disabled', true) @submit_button.hide() if @allow_reset @reset_button.show() @@ -62,7 +67,6 @@ class @SelfAssessment data = {'student_answer' : @answer_area.val()} $.postWithPrefix "#{@ajax_url}/save_answer", data, (response) => if response.success - @answer_area.attr("disabled",true) @rubric_wrapper.html(response.rubric_html) @state = 'assessing' @find_assessment_elements() @@ -110,6 +114,7 @@ class @SelfAssessment if @state == 'done' $.postWithPrefix "#{@ajax_url}/reset", {}, (response) => if response.success + @answer_area.html('') @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 699cd0f9ac..9de1d999ee 100644 --- a/common/lib/xmodule/xmodule/self_assessment_module.py +++ b/common/lib/xmodule/xmodule/self_assessment_module.py @@ -125,7 +125,10 @@ class SelfAssessmentModule(XModule): def get_html(self): #set context variables and render template - previous_answer = self.student_answers[-1] if self.student_answers else '' + if self.state != self.INITIAL and self.student_answers: + previous_answer = self.student_answers[-1] + else: + previous_answer = '' allow_reset = self.state == self.DONE and self.attempts < self.max_attempts context = { @@ -207,12 +210,12 @@ class SelfAssessmentModule(XModule): }) return json.dumps(d, cls=ComplexEncoder) - def out_of_sync_error(self, get): + def out_of_sync_error(self, get, msg=''): """ return dict out-of-sync error message, and also log. """ - log.warning("Assessment module state out sync. state: %r, get: %r", - self.state, get) + log.warning("Assessment module state out sync. state: %r, get: %r. %s", + self.state, get, msg) return {'success': False, 'error': 'The problem state got out-of-sync'} @@ -244,8 +247,12 @@ class SelfAssessmentModule(XModule): if self.state in (self.INITIAL, self.ASSESSING): return '' - # else we'll render it - hint = self.hints[-1] if len(self.hints) > 0 else '' + if self.state == self.REQUEST_HINT and len(self.hints) > 0: + # display the previous hint + hint = self.hints[-1] + else: + hint = '' + context = {'hint_prompt': self.hint_prompt, 'hint': hint} @@ -301,9 +308,11 @@ class SelfAssessmentModule(XModule): with 'error' only present if 'success' is False, and 'hint_html' only if success is true """ - if (self.state != self.ASSESSING or - len(self.student_answers) != len(self.scores) + 1): - return self.out_of_sync_error(get) + 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) try: self.scores.append(int(get['assessment'])) From 92ab5af627717a487f7330db6953b95244244cbb Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 12 Nov 2012 16:23:40 -0500 Subject: [PATCH 5/6] bigger error messages --- lms/static/sass/course/courseware/_courseware.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lms/static/sass/course/courseware/_courseware.scss b/lms/static/sass/course/courseware/_courseware.scss index 55a7be9acf..350665dbce 100644 --- a/lms/static/sass/course/courseware/_courseware.scss +++ b/lms/static/sass/course/courseware/_courseware.scss @@ -262,5 +262,9 @@ section.self-assessment { margin-bottom: 5px; } + .error { + font-size: 14px; + font-weight: bold; + } } From 3f95116ac104e7e92d2d1c21d0e94db6e144a824 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 12 Nov 2012 16:23:58 -0500 Subject: [PATCH 6/6] only ask for hints if assessed as wrong --- .../js/src/selfassessment/display.coffee | 20 +++++--- .../xmodule/xmodule/self_assessment_module.py | 51 ++++++++++++++----- 2 files changed, 50 insertions(+), 21 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/src/selfassessment/display.coffee b/common/lib/xmodule/xmodule/js/src/selfassessment/display.coffee index a40c405e4a..951eb42fce 100644 --- a/common/lib/xmodule/xmodule/js/src/selfassessment/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/selfassessment/display.coffee @@ -72,7 +72,7 @@ class @SelfAssessment @find_assessment_elements() @rebind() else - @errors_area.html(response.message) + @errors_area.html(response.error) else @errors_area.html('Problem state got out of sync. Try reloading the page.') @@ -82,12 +82,18 @@ class @SelfAssessment data = {'assessment' : @assessment.find(':selected').text()} $.postWithPrefix "#{@ajax_url}/save_assessment", data, (response) => if response.success - @hint_wrapper.html(response.hint_html) - @state = 'request_hint' - @find_hint_elements() + @state = response.state + + if @state == 'request_hint' + @hint_wrapper.html(response.hint_html) + @find_hint_elements() + else if @state == 'done' + @message_wrapper.html(response.message_html) + @allow_reset = response.allow_reset + @rebind() else - @errors_area.html(response.message) + @errors_area.html(response.error) else @errors_area.html('Problem state got out of sync. Try reloading the page.') @@ -104,7 +110,7 @@ class @SelfAssessment @allow_reset = response.allow_reset @rebind() else - @errors_area.html(response.message) + @errors_area.html(response.error) else @errors_area.html('Problem state got out of sync. Try reloading the page.') @@ -122,6 +128,6 @@ class @SelfAssessment @rebind() @reset_button.hide() else - @errors_area.html(response.message) + @errors_area.html(response.error) else @errors_area.html('Problem state got out of sync. Try reloading the page.') diff --git a/common/lib/xmodule/xmodule/self_assessment_module.py b/common/lib/xmodule/xmodule/self_assessment_module.py index 9de1d999ee..2edf5467b2 100644 --- a/common/lib/xmodule/xmodule/self_assessment_module.py +++ b/common/lib/xmodule/xmodule/self_assessment_module.py @@ -123,6 +123,10 @@ class SelfAssessmentModule(XModule): self.submit_message = definition['submitmessage'] self.hint_prompt = definition['hintprompt'] + 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: @@ -130,7 +134,6 @@ class SelfAssessmentModule(XModule): else: previous_answer = '' - allow_reset = self.state == self.DONE and self.attempts < self.max_attempts context = { 'prompt': self.prompt, 'previous_answer': previous_answer, @@ -139,7 +142,7 @@ class SelfAssessmentModule(XModule): 'initial_hint': self.get_hint_html(), 'initial_message': self.get_message_html(), 'state': self.state, - 'allow_reset': allow_reset, + 'allow_reset': self._allow_reset(), } html = self.system.render_template('self_assessment_prompt.html', context) @@ -247,12 +250,12 @@ class SelfAssessmentModule(XModule): if self.state in (self.INITIAL, self.ASSESSING): return '' - if self.state == self.REQUEST_HINT and len(self.hints) > 0: + if self.state == self.DONE and len(self.hints) > 0: # display the previous hint hint = self.hints[-1] else: hint = '' - + context = {'hint_prompt': self.hint_prompt, 'hint': hint} @@ -286,7 +289,7 @@ class SelfAssessmentModule(XModule): # they won't see the reset button once they're out of attempts. return { 'success': False, - 'message': 'Too many attempts.' + 'error': 'Too many attempts.' } if self.state != self.INITIAL: @@ -302,10 +305,17 @@ class SelfAssessmentModule(XModule): def save_assessment(self, get): """ - Save the assessment. + Save the assessment. If the student said they're right, don't ask for a + hint, and go straight to the done state. Otherwise, do ask for a hint. - Returns a dict { 'success': bool, 'hint_html': hint_html 'error': error-msg}, - with 'error' only present if 'success' is False, and 'hint_html' only if success is true + Returns a dict { 'success': bool, 'state': state, + + 'hint_html': hint_html OR 'message_html': html and 'allow_reset', + + 'error': error-msg}, + + with 'error' only present if 'success' is False, and 'hint_html' or + 'message_html' only if success is true """ n_answers = len(self.student_answers) @@ -315,14 +325,25 @@ class SelfAssessmentModule(XModule): return self.out_of_sync_error(get, msg) try: - self.scores.append(int(get['assessment'])) + score = int(get['assessment']) except: return {'success': False, 'error': "Non-integer score value"} - self.state = self.REQUEST_HINT + self.scores.append(score) + + d = {'success': True,} + + if score == self.max_score(): + self.state = self.DONE + d['message_html'] = self.get_message_html() + d['allow_reset'] = self._allow_reset() + else: + self.state = self.REQUEST_HINT + d['hint_html'] = self.get_hint_html() + + d['state'] = self.state + return d - # TODO: return different hint based on assessment value... - return {'success': True, 'hint_html': self.get_hint_html()} def save_hint(self, get): ''' @@ -334,7 +355,9 @@ class SelfAssessmentModule(XModule): with the error key only present if success is False and message_html only if True. ''' - if self.state != self.REQUEST_HINT or len(self.scores) != len(self.hints) + 1: + if self.state != self.REQUEST_HINT: + # Note: because we only ask for hints on wrong answers, may not have + # the same number of hints and answers. return self.out_of_sync_error(get) self.hints.append(get['hint'].lower()) @@ -356,7 +379,7 @@ class SelfAssessmentModule(XModule): return {'success': True, 'message_html': self.get_message_html(), - 'allow_reset': self.attempts < self.max_attempts} + 'allow_reset': self._allow_reset()} def reset(self, get):