From 309ac7aa5c1f217f0f5b85cbbbeba0938e4e0268 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Thu, 31 Jan 2013 13:27:46 -0500 Subject: [PATCH 01/37] Refactor rubric CSS into a single file. --- .../css/combinedopenended/display.scss | 41 --------------- lms/static/sass/course.scss | 1 + lms/static/sass/course/_rubric.scss | 52 +++++++++++++++++++ lms/static/sass/course/_staff_grading.scss | 47 +---------------- 4 files changed, 54 insertions(+), 87 deletions(-) create mode 100644 lms/static/sass/course/_rubric.scss diff --git a/common/lib/xmodule/xmodule/css/combinedopenended/display.scss b/common/lib/xmodule/xmodule/css/combinedopenended/display.scss index a4045c9dad..1917471879 100644 --- a/common/lib/xmodule/xmodule/css/combinedopenended/display.scss +++ b/common/lib/xmodule/xmodule/css/combinedopenended/display.scss @@ -231,47 +231,6 @@ div.result-container { } } -div.result-container, section.open-ended-child { - .rubric { - margin-bottom:25px; - tr { - margin:10px 0px; - height: 100%; - } - td { - padding: 20px 0px 25px 0px; - margin: 10px 0px; - height: 100%; - } - th { - padding: 5px; - margin: 5px; - } - label, - .view-only { - margin:2px; - position: relative; - padding: 10px 15px 25px 15px; - width: 145px; - height:100%; - display: inline-block; - min-height: 50px; - min-width: 50px; - background-color: #CCC; - font-size: .85em; - } - .grade { - position: absolute; - bottom:0px; - right:0px; - margin:10px; - } - .selected-grade { - background: #666; - color: white; - } - } -} section.open-ended-child { @media print { diff --git a/lms/static/sass/course.scss b/lms/static/sass/course.scss index e900e589b2..d5f620be82 100644 --- a/lms/static/sass/course.scss +++ b/lms/static/sass/course.scss @@ -44,6 +44,7 @@ @import "course/gradebook"; @import "course/tabs"; @import "course/staff_grading"; +@import "course/rubric"; // instructor @import "course/instructor/instructor"; diff --git a/lms/static/sass/course/_rubric.scss b/lms/static/sass/course/_rubric.scss new file mode 100644 index 0000000000..c82d929fac --- /dev/null +++ b/lms/static/sass/course/_rubric.scss @@ -0,0 +1,52 @@ +.rubric { + padding: 40px 0px; + tr { + margin:10px 0px; + height: 100%; + } + td { + padding: 20px 0px 25px 0px; + height: 100%; + border: 1px black solid; + } + th { + padding: 5px; + margin: 5px; + text-align: center; + } + .points-header th { + padding: 0px; + } + label, + .view-only { + margin:2px; + position: relative; + padding: 15px 15px 25px 15px; + width: 130px; + height:100%; + min-height: 50px; + min-width: 50px; + font-size: .9em; + background-color: white; + display: block; + } + .grade { + position: absolute; + bottom:0px; + right:0px; + margin:10px; + } + .selected-grade { + background: #666; + color: white; + } + input[type=radio]:checked + label { + background: #666; + color: white; } + input[class='score-selection'] { + position: relative; + margin-left: 10px; + font-size: 16px; + } +} + diff --git a/lms/static/sass/course/_staff_grading.scss b/lms/static/sass/course/_staff_grading.scss index 177bd9e5e2..4d4da484de 100644 --- a/lms/static/sass/course/_staff_grading.scss +++ b/lms/static/sass/course/_staff_grading.scss @@ -12,7 +12,7 @@ div.peer-grading{ label { margin: 10px; padding: 5px; - display: inline-block; + @include inline-block; min-width: 50px; background-color: #CCC; text-size: 1.5em; @@ -176,49 +176,4 @@ div.peer-grading{ } } padding: 40px; - .rubric { - tr { - margin:10px 0px; - height: 100%; - } - td { - padding: 20px 0px 25px 0px; - height: 100%; - } - th { - padding: 5px; - margin: 5px; - } - label, - .view-only { - margin:2px; - position: relative; - padding: 15px 15px 25px 15px; - width: 150px; - height:100%; - display: inline-block; - min-height: 50px; - min-width: 50px; - background-color: #CCC; - font-size: .9em; - } - .grade { - position: absolute; - bottom:0px; - right:0px; - margin:10px; - } - .selected-grade { - background: #666; - color: white; - } - input[type=radio]:checked + label { - background: #666; - color: white; } - input[class='score-selection'] { - display: none; - } - } - } - From 683976d7adf4eebedd84894d79b13572072f8dfb Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Thu, 31 Jan 2013 13:28:19 -0500 Subject: [PATCH 02/37] Add scores to the top of the rubric, remove from individual cells --- common/lib/xmodule/xmodule/combined_open_ended_rubric.py | 5 ++++- lms/templates/open_ended_rubric.html | 9 ++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_rubric.py b/common/lib/xmodule/xmodule/combined_open_ended_rubric.py index 4380e32d5b..50ec22f033 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_rubric.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_rubric.py @@ -25,10 +25,13 @@ class CombinedOpenEndedRubric(object): ''' try: rubric_categories = self.extract_categories(rubric_xml) + max_scores = map((lambda cat: cat['options'][-1]['points']), rubric_categories) + max_score = max(max_scores) html = self.system.render_template('open_ended_rubric.html', {'categories' : rubric_categories, 'has_score': self.has_score, - 'view_only': self.view_only}) + 'view_only': self.view_only, + 'max_score': max_score}) except: raise RubricParsingError("[render_rubric] Could not parse the rubric with xml: {0}".format(rubric_xml)) return html diff --git a/lms/templates/open_ended_rubric.html b/lms/templates/open_ended_rubric.html index 8d40c7d2b8..b92ad04bde 100644 --- a/lms/templates/open_ended_rubric.html +++ b/lms/templates/open_ended_rubric.html @@ -8,6 +8,14 @@

Select the criteria you feel best represents this submission in each category.

% endif + + + % for i in range(max_score + 1): + + % endfor + % for i in range(len(categories)): <% category = categories[i] %> @@ -23,7 +31,6 @@
% endif ${option['text']} -
[${option['points']} points]
% else: From 2e0f90081eb12c822b603861cbf86e8e34601866 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Thu, 31 Jan 2013 14:23:17 -0500 Subject: [PATCH 03/37] Make rubric cleaner and visually simpler --- lms/static/sass/course/_rubric.scss | 17 ++++++++--------- lms/templates/open_ended_rubric.html | 6 +++--- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/lms/static/sass/course/_rubric.scss b/lms/static/sass/course/_rubric.scss index c82d929fac..9aa0ca2f2a 100644 --- a/lms/static/sass/course/_rubric.scss +++ b/lms/static/sass/course/_rubric.scss @@ -1,5 +1,5 @@ .rubric { - padding: 40px 0px; + margin: 40px 0px; tr { margin:10px 0px; height: 100%; @@ -8,6 +8,7 @@ padding: 20px 0px 25px 0px; height: 100%; border: 1px black solid; + text-align: center; } th { padding: 5px; @@ -17,13 +18,11 @@ .points-header th { padding: 0px; } - label, - .view-only { - margin:2px; + .rubric-label + { position: relative; - padding: 15px 15px 25px 15px; + padding: 15px 15px 25px; width: 130px; - height:100%; min-height: 50px; min-width: 50px; font-size: .9em; @@ -40,9 +39,9 @@ background: #666; color: white; } - input[type=radio]:checked + label { - background: #666; - color: white; } + input[type=radio]:checked + .rubric-label { + background: white; + color: $base-font-color; } input[class='score-selection'] { position: relative; margin-left: 10px; diff --git a/lms/templates/open_ended_rubric.html b/lms/templates/open_ended_rubric.html index b92ad04bde..a2d8d6945c 100644 --- a/lms/templates/open_ended_rubric.html +++ b/lms/templates/open_ended_rubric.html @@ -26,15 +26,15 @@ % if view_only: ## if this is the selected rubric block, show it highlighted % if option['selected']: -
+
% else: -
+
% endif ${option['text']}
% else: - + % endif % endfor From 6575386d6992bea4e68332370d0e9de13180c861 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 1 Feb 2013 08:43:55 -0500 Subject: [PATCH 04/37] Refactor rubric JS so that we don't have to keep duplicating this code. --- .../js/src/combinedopenended/display.coffee | 29 +++++++++++++++ .../peer_grading/peer_grading_problem.coffee | 15 ++------ .../src/staff_grading/staff_grading.coffee | 37 ++++--------------- 3 files changed, 41 insertions(+), 40 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee b/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee index 594efe2f9b..c4560559c8 100644 --- a/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee @@ -1,3 +1,32 @@ +class @Rubric + constructor: () -> + + # finds the scores for each rubric category + @get_score_list: () => + # find the number of categories: + num_categories = $('table.rubric tr').length + + score_lst = [] + # get the score for each one + for i in [0..(num_categories-2)] + score = $("input[name='score-selection-#{i}']:checked").val() + score_lst.push(score) + + return score_lst + + @get_total_score: () -> + score_lst = @get_score_list() + + @check_complete: () -> + # check to see whether or not any categories have not been scored + num_categories = $('table.rubric tr').length + # -2 because we want to skip the header + for i in [0..(num_categories-2)] + score = $("input[name='score-selection-#{i}']:checked").val() + if score == undefined + return false + return true + class @CombinedOpenEnded constructor: (element) -> @element=element diff --git a/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee b/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee index c4b87eb30e..525891bb03 100644 --- a/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee +++ b/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee @@ -239,7 +239,7 @@ class PeerGradingProblem score_lst = [] # get the score for each one - for i in [0..(num_categories-1)] + for i in [0..(num_categories-2)] score = $("input[name='score-selection-#{i}']:checked").val() score_lst.push(score) @@ -315,17 +315,10 @@ class PeerGradingProblem # called after a grade is selected on the interface graded_callback: (event) => - @grade = $("input[name='grade-selection']:checked").val() - if @grade == undefined - return # check to see whether or not any categories have not been scored - num_categories = $('table.rubric tr').length - for i in [0..(num_categories-1)] - score = $("input[name='score-selection-#{i}']:checked").val() - if score == undefined - return - # show button if we have scores for all categories - @show_submit_button() + if Rubric.check_complete(): + # show button if we have scores for all categories + @show_submit_button() diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee index 005a8e682e..2d3cafd3e7 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -232,35 +232,14 @@ class @StaffGrading graded_callback: () => - @grade = $("input[name='grade-selection']:checked").val() - if @grade == undefined - return - # check to see whether or not any categories have not been scored - num_categories = $('table.rubric tr').length - for i in [0..(num_categories-1)] - score = $("input[name='score-selection-#{i}']:checked").val() - if score == undefined - return - # show button if we have scores for all categories - @state = state_graded - @submit_button.show() + # show button if we have scores for all categories + if Rubric.check_complete() + @state = state_graded + @submit_button.show() set_button_text: (text) => @action_button.attr('value', text) - # finds the scores for each rubric category - get_score_list: () => - # find the number of categories: - num_categories = $('table.rubric tr').length - - score_lst = [] - # get the score for each one - for i in [0..(num_categories-1)] - score = $("input[name='score-selection-#{i}']:checked").val() - score_lst.push(score) - - return score_lst - ajax_callback: (response) => # always clear out errors and messages on transition. @error_msg = '' @@ -285,8 +264,8 @@ class @StaffGrading skip_and_get_next: () => data = - score: @grade - rubric_scores: @get_score_list() + score: Rubric.get_total_score() + rubric_scores: Rubric.get_score_list() feedback: @feedback_area.val() submission_id: @submission_id location: @location @@ -299,8 +278,8 @@ class @StaffGrading submit_and_get_next: () -> data = - score: @grade - rubric_scores: @get_score_list() + score: Rubric.get_total_score() + rubric_scores: Rubric.get_score_list() feedback: @feedback_area.val() submission_id: @submission_id location: @location From f05bda764470b80873937cb8573cf3db27228137 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 1 Feb 2013 08:44:08 -0500 Subject: [PATCH 05/37] Visual updates to rubric --- lms/static/sass/course/_rubric.scss | 3 ++- lms/templates/open_ended_rubric.html | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lms/static/sass/course/_rubric.scss b/lms/static/sass/course/_rubric.scss index 9aa0ca2f2a..722a790e6d 100644 --- a/lms/static/sass/course/_rubric.scss +++ b/lms/static/sass/course/_rubric.scss @@ -35,7 +35,8 @@ right:0px; margin:10px; } - .selected-grade { + .selected-grade, + .selected-grade .rubric-label { background: #666; color: white; } diff --git a/lms/templates/open_ended_rubric.html b/lms/templates/open_ended_rubric.html index a2d8d6945c..eb3fc564b4 100644 --- a/lms/templates/open_ended_rubric.html +++ b/lms/templates/open_ended_rubric.html @@ -22,14 +22,14 @@
% for j in range(len(category['options'])): <% option = category['options'][j] %> + %if option['selected']: +
+ ${i} points +
${category['description']} + %else: + % endif % if view_only: ## if this is the selected rubric block, show it highlighted - % if option['selected']: -
- % else:
- % endif ${option['text']}
% else: From 0ec3be18155d5136aeade3139686f04c07b6d8a4 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 1 Feb 2013 10:17:16 -0500 Subject: [PATCH 06/37] Calculate the total score from the rubric. --- .../xmodule/xmodule/js/src/combinedopenended/display.coffee | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee b/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee index c4560559c8..576fb7290d 100644 --- a/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee @@ -16,6 +16,10 @@ class @Rubric @get_total_score: () -> score_lst = @get_score_list() + tot = 0 + for score in score_lst + tot += parseInt(score) + return tot @check_complete: () -> # check to see whether or not any categories have not been scored From cb44918f4958ebef6cd70bb8d1e760c534f046ea Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 1 Feb 2013 10:31:46 -0500 Subject: [PATCH 07/37] Remove duplicate Javascript and remove total grade selection. --- .../peer_grading/peer_grading_problem.coffee | 35 ++----------------- .../src/staff_grading/staff_grading.coffee | 15 -------- 2 files changed, 3 insertions(+), 47 deletions(-) diff --git a/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee b/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee index 525891bb03..f4b9bdbe78 100644 --- a/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee +++ b/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee @@ -232,23 +232,11 @@ class PeerGradingProblem fetch_submission_essay: () => @backend.post('get_next_submission', {location: @location}, @render_submission) - # finds the scores for each rubric category - get_score_list: () => - # find the number of categories: - num_categories = $('table.rubric tr').length - - score_lst = [] - # get the score for each one - for i in [0..(num_categories-2)] - score = $("input[name='score-selection-#{i}']:checked").val() - score_lst.push(score) - - return score_lst construct_data: () -> data = - rubric_scores: @get_score_list() - score: @grade + rubric_scores: Rubric.get_score_list() + score: Rubric.get_total_score() location: @location submission_id: @essay_id_input.val() submission_key: @submission_key_input.val() @@ -316,7 +304,7 @@ class PeerGradingProblem # called after a grade is selected on the interface graded_callback: (event) => # check to see whether or not any categories have not been scored - if Rubric.check_complete(): + if Rubric.check_complete() # show button if we have scores for all categories @show_submit_button() @@ -439,25 +427,8 @@ class PeerGradingProblem setup_score_selection: (max_score) => - # first, get rid of all the old inputs, if any. - @score_selection_container.html(""" -

Overall Score

-

Choose an overall score for this submission.

- """) - - # Now create new labels and inputs for each possible score. - for score in [0..max_score] - id = 'score-' + score - label = """""" - - input = """ - - """ # " fix broken parsing in emacs - @score_selection_container.append(input + label) - # And now hook up an event handler again $("input[name='score-selection']").change @graded_callback - $("input[name='grade-selection']").change @graded_callback diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee index 2d3cafd3e7..117388bab0 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -212,21 +212,6 @@ class @StaffGrading setup_score_selection: => - # first, get rid of all the old inputs, if any. - @grade_selection_container.html(""" -

Overall Score

-

Choose an overall score for this submission.

- """) - # Now create new labels and inputs for each possible score. - for score in [0..@max_score] - id = 'score-' + score - label = """""" - input = """ - - """ # " fix broken parsing in emacs - @grade_selection_container.append(input + label) - $('.grade-selection').click => @graded_callback() - @score_selection_container.html(@rubric) $('.score-selection').click => @graded_callback() From 1fef6b161b767fddc88aac4194cfac7792b82cd0 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 1 Feb 2013 11:21:56 -0500 Subject: [PATCH 08/37] Add in some better encouragement to write feedback --- lms/templates/instructor/staff_grading.html | 1 + lms/templates/peer_grading/peer_grading_problem.html | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/lms/templates/instructor/staff_grading.html b/lms/templates/instructor/staff_grading.html index 56aed5a54a..dcfece34b8 100644 --- a/lms/templates/instructor/staff_grading.html +++ b/lms/templates/instructor/staff_grading.html @@ -75,6 +75,7 @@

+

Written Feedback

diff --git a/lms/templates/peer_grading/peer_grading_problem.html b/lms/templates/peer_grading/peer_grading_problem.html index cb9ed1c0fb..ae630f118e 100644 --- a/lms/templates/peer_grading/peer_grading_problem.html +++ b/lms/templates/peer_grading/peer_grading_problem.html @@ -70,7 +70,9 @@

- From 2b764eebad0dd892f894ca3abb7f68b0be3e63c1 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 1 Feb 2013 11:27:53 -0500 Subject: [PATCH 09/37] Make the rubric for self-assessment selectable and remove the separate grade selection. --- .../js/src/combinedopenended/display.coffee | 6 +++--- .../xmodule/xmodule/self_assessment_module.py | 2 +- .../src/peer_grading/peer_grading_problem.coffee | 1 - lms/templates/self_assessment_rubric.html | 16 ---------------- 4 files changed, 4 insertions(+), 21 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee b/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee index 576fb7290d..9add338137 100644 --- a/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee @@ -208,9 +208,9 @@ class @CombinedOpenEnded save_assessment: (event) => event.preventDefault() - if @child_state == 'assessing' - checked_assessment = @$('input[name="grade-selection"]:checked') - data = {'assessment' : checked_assessment.val()} + if @child_state == 'assessing' && Rubric.check_complete() + checked_assessment = Rubric.get_total_score() + data = {'assessment' : checked_assessment} $.postWithPrefix "#{@ajax_url}/save_assessment", data, (response) => if response.success @child_state = response.state diff --git a/common/lib/xmodule/xmodule/self_assessment_module.py b/common/lib/xmodule/xmodule/self_assessment_module.py index fb1d306708..a288fa55b3 100644 --- a/common/lib/xmodule/xmodule/self_assessment_module.py +++ b/common/lib/xmodule/xmodule/self_assessment_module.py @@ -122,7 +122,7 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild): if self.state == self.INITIAL: return '' - rubric_renderer = CombinedOpenEndedRubric(system, True) + rubric_renderer = CombinedOpenEndedRubric(system, False) rubric_html = rubric_renderer.render_rubric(self.rubric) # we'll render it diff --git a/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee b/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee index f4b9bdbe78..f803c74c7b 100644 --- a/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee +++ b/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee @@ -426,7 +426,6 @@ class PeerGradingProblem @submit_button.show() setup_score_selection: (max_score) => - # And now hook up an event handler again $("input[name='score-selection']").change @graded_callback diff --git a/lms/templates/self_assessment_rubric.html b/lms/templates/self_assessment_rubric.html index b4fc125232..2986c5041a 100644 --- a/lms/templates/self_assessment_rubric.html +++ b/lms/templates/self_assessment_rubric.html @@ -2,20 +2,4 @@
${rubric | n }
- - % if not read_only: -
-

Scoring

-

Please select a score below:

- -
- %for i in xrange(0,max_score+1): - <% id = "score-{0}".format(i) %> - - - %endfor -
-
- % endif - From 91d9bc05be2f20a8b09ebe1de9b6ae8edf2018d0 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 1 Feb 2013 13:06:12 -0500 Subject: [PATCH 10/37] Fix bug in the callback --- lms/static/coffee/src/peer_grading/peer_grading_problem.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee b/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee index f803c74c7b..05d0189ac8 100644 --- a/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee +++ b/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee @@ -427,7 +427,7 @@ class PeerGradingProblem setup_score_selection: (max_score) => # And now hook up an event handler again - $("input[name='score-selection']").change @graded_callback + $("input[class='score-selection']").change @graded_callback From 00ffbc070adeb605d58efce4478969d620c06b0e Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 1 Feb 2013 14:34:09 -0500 Subject: [PATCH 11/37] Make rubrics spacing smaller and fix a bug in the grading service renderer --- lms/djangoapps/open_ended_grading/grading_service.py | 2 +- lms/static/sass/course/_rubric.scss | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/open_ended_grading/grading_service.py b/lms/djangoapps/open_ended_grading/grading_service.py index f65554a9d6..8e6209bf38 100644 --- a/lms/djangoapps/open_ended_grading/grading_service.py +++ b/lms/djangoapps/open_ended_grading/grading_service.py @@ -115,7 +115,7 @@ class GradingService(object): response_json = json.loads(response) if 'rubric' in response_json: rubric = response_json['rubric'] - rubric_renderer = CombinedOpenEndedRubric(self.system, False) + rubric_renderer = CombinedOpenEndedRubric(self.system, view_only) rubric_html = rubric_renderer.render_rubric(rubric) response_json['rubric'] = rubric_html return response_json diff --git a/lms/static/sass/course/_rubric.scss b/lms/static/sass/course/_rubric.scss index 722a790e6d..5048d70253 100644 --- a/lms/static/sass/course/_rubric.scss +++ b/lms/static/sass/course/_rubric.scss @@ -21,7 +21,7 @@ .rubric-label { position: relative; - padding: 15px 15px 25px; + padding: 0px 15px 15px 15px; width: 130px; min-height: 50px; min-width: 50px; From 69d5c005c30f64346b1f190b04e090f4c29436b3 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Thu, 31 Jan 2013 13:27:46 -0500 Subject: [PATCH 12/37] Refactor rubric CSS into a single file. --- .../css/combinedopenended/display.scss | 41 --------------- lms/static/sass/course.scss | 1 + lms/static/sass/course/_rubric.scss | 52 +++++++++++++++++++ lms/static/sass/course/_staff_grading.scss | 47 +---------------- 4 files changed, 54 insertions(+), 87 deletions(-) create mode 100644 lms/static/sass/course/_rubric.scss diff --git a/common/lib/xmodule/xmodule/css/combinedopenended/display.scss b/common/lib/xmodule/xmodule/css/combinedopenended/display.scss index a4045c9dad..1917471879 100644 --- a/common/lib/xmodule/xmodule/css/combinedopenended/display.scss +++ b/common/lib/xmodule/xmodule/css/combinedopenended/display.scss @@ -231,47 +231,6 @@ div.result-container { } } -div.result-container, section.open-ended-child { - .rubric { - margin-bottom:25px; - tr { - margin:10px 0px; - height: 100%; - } - td { - padding: 20px 0px 25px 0px; - margin: 10px 0px; - height: 100%; - } - th { - padding: 5px; - margin: 5px; - } - label, - .view-only { - margin:2px; - position: relative; - padding: 10px 15px 25px 15px; - width: 145px; - height:100%; - display: inline-block; - min-height: 50px; - min-width: 50px; - background-color: #CCC; - font-size: .85em; - } - .grade { - position: absolute; - bottom:0px; - right:0px; - margin:10px; - } - .selected-grade { - background: #666; - color: white; - } - } -} section.open-ended-child { @media print { diff --git a/lms/static/sass/course.scss b/lms/static/sass/course.scss index e900e589b2..d5f620be82 100644 --- a/lms/static/sass/course.scss +++ b/lms/static/sass/course.scss @@ -44,6 +44,7 @@ @import "course/gradebook"; @import "course/tabs"; @import "course/staff_grading"; +@import "course/rubric"; // instructor @import "course/instructor/instructor"; diff --git a/lms/static/sass/course/_rubric.scss b/lms/static/sass/course/_rubric.scss new file mode 100644 index 0000000000..c82d929fac --- /dev/null +++ b/lms/static/sass/course/_rubric.scss @@ -0,0 +1,52 @@ +.rubric { + padding: 40px 0px; + tr { + margin:10px 0px; + height: 100%; + } + td { + padding: 20px 0px 25px 0px; + height: 100%; + border: 1px black solid; + } + th { + padding: 5px; + margin: 5px; + text-align: center; + } + .points-header th { + padding: 0px; + } + label, + .view-only { + margin:2px; + position: relative; + padding: 15px 15px 25px 15px; + width: 130px; + height:100%; + min-height: 50px; + min-width: 50px; + font-size: .9em; + background-color: white; + display: block; + } + .grade { + position: absolute; + bottom:0px; + right:0px; + margin:10px; + } + .selected-grade { + background: #666; + color: white; + } + input[type=radio]:checked + label { + background: #666; + color: white; } + input[class='score-selection'] { + position: relative; + margin-left: 10px; + font-size: 16px; + } +} + diff --git a/lms/static/sass/course/_staff_grading.scss b/lms/static/sass/course/_staff_grading.scss index 177bd9e5e2..4d4da484de 100644 --- a/lms/static/sass/course/_staff_grading.scss +++ b/lms/static/sass/course/_staff_grading.scss @@ -12,7 +12,7 @@ div.peer-grading{ label { margin: 10px; padding: 5px; - display: inline-block; + @include inline-block; min-width: 50px; background-color: #CCC; text-size: 1.5em; @@ -176,49 +176,4 @@ div.peer-grading{ } } padding: 40px; - .rubric { - tr { - margin:10px 0px; - height: 100%; - } - td { - padding: 20px 0px 25px 0px; - height: 100%; - } - th { - padding: 5px; - margin: 5px; - } - label, - .view-only { - margin:2px; - position: relative; - padding: 15px 15px 25px 15px; - width: 150px; - height:100%; - display: inline-block; - min-height: 50px; - min-width: 50px; - background-color: #CCC; - font-size: .9em; - } - .grade { - position: absolute; - bottom:0px; - right:0px; - margin:10px; - } - .selected-grade { - background: #666; - color: white; - } - input[type=radio]:checked + label { - background: #666; - color: white; } - input[class='score-selection'] { - display: none; - } - } - } - From 7e345ce51278810337f15101d931c5cebf35aac8 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Thu, 31 Jan 2013 13:28:19 -0500 Subject: [PATCH 13/37] Add scores to the top of the rubric, remove from individual cells --- common/lib/xmodule/xmodule/combined_open_ended_rubric.py | 5 ++++- lms/templates/open_ended_rubric.html | 9 ++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_rubric.py b/common/lib/xmodule/xmodule/combined_open_ended_rubric.py index 4380e32d5b..50ec22f033 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_rubric.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_rubric.py @@ -25,10 +25,13 @@ class CombinedOpenEndedRubric(object): ''' try: rubric_categories = self.extract_categories(rubric_xml) + max_scores = map((lambda cat: cat['options'][-1]['points']), rubric_categories) + max_score = max(max_scores) html = self.system.render_template('open_ended_rubric.html', {'categories' : rubric_categories, 'has_score': self.has_score, - 'view_only': self.view_only}) + 'view_only': self.view_only, + 'max_score': max_score}) except: raise RubricParsingError("[render_rubric] Could not parse the rubric with xml: {0}".format(rubric_xml)) return html diff --git a/lms/templates/open_ended_rubric.html b/lms/templates/open_ended_rubric.html index 8d40c7d2b8..b92ad04bde 100644 --- a/lms/templates/open_ended_rubric.html +++ b/lms/templates/open_ended_rubric.html @@ -8,6 +8,14 @@

Select the criteria you feel best represents this submission in each category.

% endif + + + % for i in range(max_score + 1): + + % endfor + % for i in range(len(categories)): <% category = categories[i] %> @@ -23,7 +31,6 @@
% endif ${option['text']} -
[${option['points']} points]
% else: From 1fde3c5ec5821b0fe69ac4799bb319659bbb9fa2 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Thu, 31 Jan 2013 14:23:17 -0500 Subject: [PATCH 14/37] Make rubric cleaner and visually simpler --- lms/static/sass/course/_rubric.scss | 17 ++++++++--------- lms/templates/open_ended_rubric.html | 6 +++--- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/lms/static/sass/course/_rubric.scss b/lms/static/sass/course/_rubric.scss index c82d929fac..9aa0ca2f2a 100644 --- a/lms/static/sass/course/_rubric.scss +++ b/lms/static/sass/course/_rubric.scss @@ -1,5 +1,5 @@ .rubric { - padding: 40px 0px; + margin: 40px 0px; tr { margin:10px 0px; height: 100%; @@ -8,6 +8,7 @@ padding: 20px 0px 25px 0px; height: 100%; border: 1px black solid; + text-align: center; } th { padding: 5px; @@ -17,13 +18,11 @@ .points-header th { padding: 0px; } - label, - .view-only { - margin:2px; + .rubric-label + { position: relative; - padding: 15px 15px 25px 15px; + padding: 15px 15px 25px; width: 130px; - height:100%; min-height: 50px; min-width: 50px; font-size: .9em; @@ -40,9 +39,9 @@ background: #666; color: white; } - input[type=radio]:checked + label { - background: #666; - color: white; } + input[type=radio]:checked + .rubric-label { + background: white; + color: $base-font-color; } input[class='score-selection'] { position: relative; margin-left: 10px; diff --git a/lms/templates/open_ended_rubric.html b/lms/templates/open_ended_rubric.html index b92ad04bde..a2d8d6945c 100644 --- a/lms/templates/open_ended_rubric.html +++ b/lms/templates/open_ended_rubric.html @@ -26,15 +26,15 @@ % if view_only: ## if this is the selected rubric block, show it highlighted % if option['selected']: -
+
% else: -
+
% endif ${option['text']}
% else: - + % endif % endfor From 92b7dbdc69e7b2fc177dd2ff2085df22a4d24a25 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 1 Feb 2013 08:43:55 -0500 Subject: [PATCH 15/37] Refactor rubric JS so that we don't have to keep duplicating this code. --- .../js/src/combinedopenended/display.coffee | 29 +++++++++++++++ .../peer_grading/peer_grading_problem.coffee | 15 ++------ .../src/staff_grading/staff_grading.coffee | 37 ++++--------------- 3 files changed, 41 insertions(+), 40 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee b/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee index 594efe2f9b..c4560559c8 100644 --- a/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee @@ -1,3 +1,32 @@ +class @Rubric + constructor: () -> + + # finds the scores for each rubric category + @get_score_list: () => + # find the number of categories: + num_categories = $('table.rubric tr').length + + score_lst = [] + # get the score for each one + for i in [0..(num_categories-2)] + score = $("input[name='score-selection-#{i}']:checked").val() + score_lst.push(score) + + return score_lst + + @get_total_score: () -> + score_lst = @get_score_list() + + @check_complete: () -> + # check to see whether or not any categories have not been scored + num_categories = $('table.rubric tr').length + # -2 because we want to skip the header + for i in [0..(num_categories-2)] + score = $("input[name='score-selection-#{i}']:checked").val() + if score == undefined + return false + return true + class @CombinedOpenEnded constructor: (element) -> @element=element diff --git a/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee b/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee index c4b87eb30e..525891bb03 100644 --- a/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee +++ b/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee @@ -239,7 +239,7 @@ class PeerGradingProblem score_lst = [] # get the score for each one - for i in [0..(num_categories-1)] + for i in [0..(num_categories-2)] score = $("input[name='score-selection-#{i}']:checked").val() score_lst.push(score) @@ -315,17 +315,10 @@ class PeerGradingProblem # called after a grade is selected on the interface graded_callback: (event) => - @grade = $("input[name='grade-selection']:checked").val() - if @grade == undefined - return # check to see whether or not any categories have not been scored - num_categories = $('table.rubric tr').length - for i in [0..(num_categories-1)] - score = $("input[name='score-selection-#{i}']:checked").val() - if score == undefined - return - # show button if we have scores for all categories - @show_submit_button() + if Rubric.check_complete(): + # show button if we have scores for all categories + @show_submit_button() diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee index 005a8e682e..2d3cafd3e7 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -232,35 +232,14 @@ class @StaffGrading graded_callback: () => - @grade = $("input[name='grade-selection']:checked").val() - if @grade == undefined - return - # check to see whether or not any categories have not been scored - num_categories = $('table.rubric tr').length - for i in [0..(num_categories-1)] - score = $("input[name='score-selection-#{i}']:checked").val() - if score == undefined - return - # show button if we have scores for all categories - @state = state_graded - @submit_button.show() + # show button if we have scores for all categories + if Rubric.check_complete() + @state = state_graded + @submit_button.show() set_button_text: (text) => @action_button.attr('value', text) - # finds the scores for each rubric category - get_score_list: () => - # find the number of categories: - num_categories = $('table.rubric tr').length - - score_lst = [] - # get the score for each one - for i in [0..(num_categories-1)] - score = $("input[name='score-selection-#{i}']:checked").val() - score_lst.push(score) - - return score_lst - ajax_callback: (response) => # always clear out errors and messages on transition. @error_msg = '' @@ -285,8 +264,8 @@ class @StaffGrading skip_and_get_next: () => data = - score: @grade - rubric_scores: @get_score_list() + score: Rubric.get_total_score() + rubric_scores: Rubric.get_score_list() feedback: @feedback_area.val() submission_id: @submission_id location: @location @@ -299,8 +278,8 @@ class @StaffGrading submit_and_get_next: () -> data = - score: @grade - rubric_scores: @get_score_list() + score: Rubric.get_total_score() + rubric_scores: Rubric.get_score_list() feedback: @feedback_area.val() submission_id: @submission_id location: @location From 0bae98398202d798f69154aeaa77e99581739f13 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 1 Feb 2013 08:44:08 -0500 Subject: [PATCH 16/37] Visual updates to rubric --- lms/static/sass/course/_rubric.scss | 3 ++- lms/templates/open_ended_rubric.html | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lms/static/sass/course/_rubric.scss b/lms/static/sass/course/_rubric.scss index 9aa0ca2f2a..722a790e6d 100644 --- a/lms/static/sass/course/_rubric.scss +++ b/lms/static/sass/course/_rubric.scss @@ -35,7 +35,8 @@ right:0px; margin:10px; } - .selected-grade { + .selected-grade, + .selected-grade .rubric-label { background: #666; color: white; } diff --git a/lms/templates/open_ended_rubric.html b/lms/templates/open_ended_rubric.html index a2d8d6945c..eb3fc564b4 100644 --- a/lms/templates/open_ended_rubric.html +++ b/lms/templates/open_ended_rubric.html @@ -22,14 +22,14 @@
% for j in range(len(category['options'])): <% option = category['options'][j] %> + %if option['selected']: +
+ ${i} points +
${category['description']} + %else: + % endif % if view_only: ## if this is the selected rubric block, show it highlighted - % if option['selected']: -
- % else:
- % endif ${option['text']}
% else: From ba30c2ce58b9478483ced3857b39a4d1d9b88235 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 1 Feb 2013 10:17:16 -0500 Subject: [PATCH 17/37] Calculate the total score from the rubric. --- .../xmodule/xmodule/js/src/combinedopenended/display.coffee | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee b/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee index c4560559c8..576fb7290d 100644 --- a/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee @@ -16,6 +16,10 @@ class @Rubric @get_total_score: () -> score_lst = @get_score_list() + tot = 0 + for score in score_lst + tot += parseInt(score) + return tot @check_complete: () -> # check to see whether or not any categories have not been scored From 745c05277521ef57593de2071b3afeeb4e786a24 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 1 Feb 2013 10:31:46 -0500 Subject: [PATCH 18/37] Remove duplicate Javascript and remove total grade selection. --- .../peer_grading/peer_grading_problem.coffee | 35 ++----------------- .../src/staff_grading/staff_grading.coffee | 15 -------- 2 files changed, 3 insertions(+), 47 deletions(-) diff --git a/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee b/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee index 525891bb03..f4b9bdbe78 100644 --- a/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee +++ b/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee @@ -232,23 +232,11 @@ class PeerGradingProblem fetch_submission_essay: () => @backend.post('get_next_submission', {location: @location}, @render_submission) - # finds the scores for each rubric category - get_score_list: () => - # find the number of categories: - num_categories = $('table.rubric tr').length - - score_lst = [] - # get the score for each one - for i in [0..(num_categories-2)] - score = $("input[name='score-selection-#{i}']:checked").val() - score_lst.push(score) - - return score_lst construct_data: () -> data = - rubric_scores: @get_score_list() - score: @grade + rubric_scores: Rubric.get_score_list() + score: Rubric.get_total_score() location: @location submission_id: @essay_id_input.val() submission_key: @submission_key_input.val() @@ -316,7 +304,7 @@ class PeerGradingProblem # called after a grade is selected on the interface graded_callback: (event) => # check to see whether or not any categories have not been scored - if Rubric.check_complete(): + if Rubric.check_complete() # show button if we have scores for all categories @show_submit_button() @@ -439,25 +427,8 @@ class PeerGradingProblem setup_score_selection: (max_score) => - # first, get rid of all the old inputs, if any. - @score_selection_container.html(""" -

Overall Score

-

Choose an overall score for this submission.

- """) - - # Now create new labels and inputs for each possible score. - for score in [0..max_score] - id = 'score-' + score - label = """""" - - input = """ - - """ # " fix broken parsing in emacs - @score_selection_container.append(input + label) - # And now hook up an event handler again $("input[name='score-selection']").change @graded_callback - $("input[name='grade-selection']").change @graded_callback diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee index 2d3cafd3e7..117388bab0 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -212,21 +212,6 @@ class @StaffGrading setup_score_selection: => - # first, get rid of all the old inputs, if any. - @grade_selection_container.html(""" -

Overall Score

-

Choose an overall score for this submission.

- """) - # Now create new labels and inputs for each possible score. - for score in [0..@max_score] - id = 'score-' + score - label = """""" - input = """ - - """ # " fix broken parsing in emacs - @grade_selection_container.append(input + label) - $('.grade-selection').click => @graded_callback() - @score_selection_container.html(@rubric) $('.score-selection').click => @graded_callback() From 68fc794a9757b66ddd92166a5529d02050115849 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 1 Feb 2013 11:21:56 -0500 Subject: [PATCH 19/37] Add in some better encouragement to write feedback --- lms/templates/instructor/staff_grading.html | 1 + lms/templates/peer_grading/peer_grading_problem.html | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/lms/templates/instructor/staff_grading.html b/lms/templates/instructor/staff_grading.html index 56aed5a54a..dcfece34b8 100644 --- a/lms/templates/instructor/staff_grading.html +++ b/lms/templates/instructor/staff_grading.html @@ -75,6 +75,7 @@

+

Written Feedback

diff --git a/lms/templates/peer_grading/peer_grading_problem.html b/lms/templates/peer_grading/peer_grading_problem.html index cb9ed1c0fb..ae630f118e 100644 --- a/lms/templates/peer_grading/peer_grading_problem.html +++ b/lms/templates/peer_grading/peer_grading_problem.html @@ -70,7 +70,9 @@

- From 1c5c54c862fb106b1efda09a1da35367e7fe35cc Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 1 Feb 2013 11:27:53 -0500 Subject: [PATCH 20/37] Make the rubric for self-assessment selectable and remove the separate grade selection. --- .../js/src/combinedopenended/display.coffee | 6 +++--- .../xmodule/xmodule/self_assessment_module.py | 2 +- .../src/peer_grading/peer_grading_problem.coffee | 1 - lms/templates/self_assessment_rubric.html | 16 ---------------- 4 files changed, 4 insertions(+), 21 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee b/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee index 576fb7290d..9add338137 100644 --- a/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee @@ -208,9 +208,9 @@ class @CombinedOpenEnded save_assessment: (event) => event.preventDefault() - if @child_state == 'assessing' - checked_assessment = @$('input[name="grade-selection"]:checked') - data = {'assessment' : checked_assessment.val()} + if @child_state == 'assessing' && Rubric.check_complete() + checked_assessment = Rubric.get_total_score() + data = {'assessment' : checked_assessment} $.postWithPrefix "#{@ajax_url}/save_assessment", data, (response) => if response.success @child_state = response.state diff --git a/common/lib/xmodule/xmodule/self_assessment_module.py b/common/lib/xmodule/xmodule/self_assessment_module.py index fb1d306708..a288fa55b3 100644 --- a/common/lib/xmodule/xmodule/self_assessment_module.py +++ b/common/lib/xmodule/xmodule/self_assessment_module.py @@ -122,7 +122,7 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild): if self.state == self.INITIAL: return '' - rubric_renderer = CombinedOpenEndedRubric(system, True) + rubric_renderer = CombinedOpenEndedRubric(system, False) rubric_html = rubric_renderer.render_rubric(self.rubric) # we'll render it diff --git a/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee b/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee index f4b9bdbe78..f803c74c7b 100644 --- a/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee +++ b/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee @@ -426,7 +426,6 @@ class PeerGradingProblem @submit_button.show() setup_score_selection: (max_score) => - # And now hook up an event handler again $("input[name='score-selection']").change @graded_callback diff --git a/lms/templates/self_assessment_rubric.html b/lms/templates/self_assessment_rubric.html index b4fc125232..2986c5041a 100644 --- a/lms/templates/self_assessment_rubric.html +++ b/lms/templates/self_assessment_rubric.html @@ -2,20 +2,4 @@
${rubric | n }
- - % if not read_only: -
-

Scoring

-

Please select a score below:

- -
- %for i in xrange(0,max_score+1): - <% id = "score-{0}".format(i) %> - - - %endfor -
-
- % endif - From 71c233af15d4cc0c06af8bfd3d263a8678f9b7d0 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 1 Feb 2013 13:06:12 -0500 Subject: [PATCH 21/37] Fix bug in the callback --- lms/static/coffee/src/peer_grading/peer_grading_problem.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee b/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee index f803c74c7b..05d0189ac8 100644 --- a/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee +++ b/lms/static/coffee/src/peer_grading/peer_grading_problem.coffee @@ -427,7 +427,7 @@ class PeerGradingProblem setup_score_selection: (max_score) => # And now hook up an event handler again - $("input[name='score-selection']").change @graded_callback + $("input[class='score-selection']").change @graded_callback From a85d43a8170d6b00d1a9232ce665fd558d83bb1f Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 1 Feb 2013 14:34:09 -0500 Subject: [PATCH 22/37] Make rubrics spacing smaller and fix a bug in the grading service renderer --- lms/djangoapps/open_ended_grading/grading_service.py | 2 +- lms/static/sass/course/_rubric.scss | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/open_ended_grading/grading_service.py b/lms/djangoapps/open_ended_grading/grading_service.py index f65554a9d6..8e6209bf38 100644 --- a/lms/djangoapps/open_ended_grading/grading_service.py +++ b/lms/djangoapps/open_ended_grading/grading_service.py @@ -115,7 +115,7 @@ class GradingService(object): response_json = json.loads(response) if 'rubric' in response_json: rubric = response_json['rubric'] - rubric_renderer = CombinedOpenEndedRubric(self.system, False) + rubric_renderer = CombinedOpenEndedRubric(self.system, view_only) rubric_html = rubric_renderer.render_rubric(rubric) response_json['rubric'] = rubric_html return response_json diff --git a/lms/static/sass/course/_rubric.scss b/lms/static/sass/course/_rubric.scss index 722a790e6d..5048d70253 100644 --- a/lms/static/sass/course/_rubric.scss +++ b/lms/static/sass/course/_rubric.scss @@ -21,7 +21,7 @@ .rubric-label { position: relative; - padding: 15px 15px 25px; + padding: 0px 15px 15px 15px; width: 130px; min-height: 50px; min-width: 50px; From 4b59e7d5513657bb48b55b6a40c0d163933ce078 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Mon, 4 Feb 2013 10:34:00 -0500 Subject: [PATCH 23/37] Fix max score check for the updated rubric UI stuff. --- .../lib/xmodule/xmodule/combined_open_ended_module.py | 9 ++------- .../lib/xmodule/xmodule/combined_open_ended_rubric.py | 10 +++++++++- .../xmodule/xmodule/css/combinedopenended/display.scss | 7 +------ 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index 14a59c9004..7ecca35107 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -165,15 +165,10 @@ class CombinedOpenEndedModule(XModule): # completion (doesn't matter if you self-assessed correct/incorrect). self._max_score = int(self.metadata.get('max_score', MAX_SCORE)) - if self._max_score > MAX_SCORE_ALLOWED: - error_message = "Max score {0} is higher than max score allowed {1} for location {2}".format(self._max_score, - MAX_SCORE_ALLOWED, location) - log.error(error_message) - raise IncorrectMaxScoreError(error_message) rubric_renderer = CombinedOpenEndedRubric(system, True) rubric_string = stringify_children(definition['rubric']) - rubric_renderer.check_if_rubric_is_parseable(rubric_string, location, MAX_SCORE_ALLOWED) + rubric_renderer.check_if_rubric_is_parseable(rubric_string, location, MAX_SCORE_ALLOWED, self._max_score) #Static data is passed to the child modules to render self.static_data = { @@ -700,4 +695,4 @@ class CombinedOpenEndedDescriptor(XmlDescriptor, EditingDescriptor): for child in ['task']: add_child(child) - return elt \ No newline at end of file + return elt diff --git a/common/lib/xmodule/xmodule/combined_open_ended_rubric.py b/common/lib/xmodule/xmodule/combined_open_ended_rubric.py index 7c772c1596..dc08199511 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_rubric.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_rubric.py @@ -39,7 +39,7 @@ class CombinedOpenEndedRubric(object): raise RubricParsingError("[render_rubric] Could not parse the rubric with xml: {0}".format(rubric_xml)) return success, html - def check_if_rubric_is_parseable(self, rubric_string, location, max_score_allowed): + def check_if_rubric_is_parseable(self, rubric_string, location, max_score_allowed, max_score): success, rubric_feedback = self.render_rubric(rubric_string) if not success: error_message = "Could not parse rubric : {0} for location {1}".format(rubric_string, location.url()) @@ -47,13 +47,21 @@ class CombinedOpenEndedRubric(object): raise RubricParsingError(error_message) rubric_categories = self.extract_categories(rubric_string) + total = 0 for category in rubric_categories: + total = total + len(category['options']) - 1 if len(category['options']) > (max_score_allowed + 1): error_message = "Number of score points in rubric {0} higher than the max allowed, which is {1}".format( len(category['options']), max_score_allowed) log.error(error_message) raise RubricParsingError(error_message) + if total != max_score: + error_msg = "The max score {0} for problem {1} does not match the total number of points in the rubric {2}".format( + max_score, location, total) + log.error(error_msg) + raise RubricParsingError(error_msg) + def extract_categories(self, element): ''' Contstruct a list of categories such that the structure looks like: diff --git a/common/lib/xmodule/xmodule/css/combinedopenended/display.scss b/common/lib/xmodule/xmodule/css/combinedopenended/display.scss index 5f7b46f7cf..eba2910144 100644 --- a/common/lib/xmodule/xmodule/css/combinedopenended/display.scss +++ b/common/lib/xmodule/xmodule/css/combinedopenended/display.scss @@ -405,8 +405,8 @@ section.open-ended-child { padding: 9px; background: #F6F6F6; border: 1px solid #ddd; - border-top: 0; margin-bottom: 20px; + min-height: 5em; @include clearfix; } @@ -544,11 +544,6 @@ section.open-ended-child { } .submission_feedback { - // background: #F3F3F3; - // border: 1px solid #ddd; - // @include border-radius(3px); - // padding: 8px 12px; - // margin-top: 10px; @include inline-block; font-style: italic; margin: 8px 0 0 10px; From f9cca8befdb7794b79486ba2ef9c6132309c0894 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Mon, 4 Feb 2013 11:51:05 -0500 Subject: [PATCH 24/37] Starting a framework for handling the closure of an open ended problem. Currently incomplete. --- .../xmodule/combined_open_ended_module.py | 20 ++++++++++++++----- .../lib/xmodule/xmodule/open_ended_module.py | 9 ++++----- common/lib/xmodule/xmodule/openendedchild.py | 7 ++++++- .../xmodule/xmodule/self_assessment_module.py | 9 +++------ .../xmodule/tests/test_combined_open_ended.py | 10 ++++++++-- .../xmodule/tests/test_self_assessment.py | 6 +++++- 6 files changed, 41 insertions(+), 20 deletions(-) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index 14a59c9004..e9b3d1d4d0 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -188,6 +188,15 @@ class CombinedOpenEndedModule(XModule): self.task_xml = definition['task_xml'] self.setup_next_task() + def closed(self): + return True + #''' Is the student still allowed to submit answers? ''' + #if self.attempts == self.max_attempts: + # return True + #if self.close_date is not None and datetime.datetime.utcnow() > self.close_date: + # return True + + def get_tag_name(self, xml): """ Gets the tag name of a given xml block. @@ -269,7 +278,7 @@ class CombinedOpenEndedModule(XModule): self.current_task_parsed_xml = self.current_task_descriptor.definition_from_xml(etree_xml, self.system) if current_task_state is None and self.current_task_number == 0: self.current_task = child_task_module(self.system, self.location, - self.current_task_parsed_xml, self.current_task_descriptor, self.static_data) + self.current_task_parsed_xml, self.current_task_descriptor, self.static_data, self) self.task_states.append(self.current_task.get_instance_state()) self.state = self.ASSESSING elif current_task_state is None and self.current_task_number > 0: @@ -285,7 +294,7 @@ class CombinedOpenEndedModule(XModule): }) self.current_task = child_task_module(self.system, self.location, self.current_task_parsed_xml, self.current_task_descriptor, self.static_data, - instance_state=current_task_state) + self, instance_state=current_task_state) self.task_states.append(self.current_task.get_instance_state()) self.state = self.ASSESSING else: @@ -293,10 +302,11 @@ class CombinedOpenEndedModule(XModule): current_task_state = self.overwrite_state(current_task_state) self.current_task = child_task_module(self.system, self.location, self.current_task_parsed_xml, self.current_task_descriptor, self.static_data, - instance_state=current_task_state) + self, instance_state=current_task_state) return True + def check_allow_reset(self): """ Checks to see if the student has passed the criteria to move to the next module. If not, sets @@ -404,7 +414,7 @@ class CombinedOpenEndedModule(XModule): task_parsed_xml = task_descriptor.definition_from_xml(etree_xml, self.system) task = children['modules'][task_type](self.system, self.location, task_parsed_xml, task_descriptor, - self.static_data, instance_state=task_state) + self.static_data, self, instance_state=task_state) last_response = task.latest_answer() last_score = task.latest_score() last_post_assessment = task.latest_post_assessment(self.system) @@ -700,4 +710,4 @@ class CombinedOpenEndedDescriptor(XmlDescriptor, EditingDescriptor): for child in ['task']: add_child(child) - return elt \ No newline at end of file + return elt diff --git a/common/lib/xmodule/xmodule/open_ended_module.py b/common/lib/xmodule/xmodule/open_ended_module.py index 94d45d96e3..2d6970e162 100644 --- a/common/lib/xmodule/xmodule/open_ended_module.py +++ b/common/lib/xmodule/xmodule/open_ended_module.py @@ -548,13 +548,12 @@ class OpenEndedModule(openendedchild.OpenEndedChild): @param system: modulesystem @return: Success indicator """ - if self.attempts > self.max_attempts: - # If too many attempts, prevent student from saving answer and - # seeing rubric. In normal use, students shouldn't see this because - # they won't see the reset button once they're out of attempts. + # Once we close the problem, we should not allow students + # to save answers + if self.closed(): return { 'success': False, - 'error': 'Too many attempts.' + 'error': 'Problem is closed.' } if self.state != self.INITIAL: diff --git a/common/lib/xmodule/xmodule/openendedchild.py b/common/lib/xmodule/xmodule/openendedchild.py index 7151ac0723..8c35fb0cae 100644 --- a/common/lib/xmodule/xmodule/openendedchild.py +++ b/common/lib/xmodule/xmodule/openendedchild.py @@ -73,7 +73,7 @@ class OpenEndedChild(object): 'done': 'Problem complete', } - def __init__(self, system, location, definition, descriptor, static_data, + def __init__(self, system, location, definition, descriptor, static_data, parent, instance_state=None, shared_state=None, **kwargs): # Load instance state if instance_state is not None: @@ -87,6 +87,8 @@ class OpenEndedChild(object): # Scores are on scale from 0 to max_score self.history = instance_state.get('history', []) + self.parent = parent + self.state = instance_state.get('state', self.INITIAL) self.created = instance_state.get('created', False) @@ -116,6 +118,9 @@ class OpenEndedChild(object): """ pass + def closed(self): + return self.parent.closed() + def latest_answer(self): """Empty string if not available""" if not self.history: diff --git a/common/lib/xmodule/xmodule/self_assessment_module.py b/common/lib/xmodule/xmodule/self_assessment_module.py index 38a60e11f5..bb8b46559d 100644 --- a/common/lib/xmodule/xmodule/self_assessment_module.py +++ b/common/lib/xmodule/xmodule/self_assessment_module.py @@ -189,14 +189,11 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild): Dictionary with keys 'success' and either 'error' (if not success), or 'rubric_html' (if success). """ - # Check to see if attempts are less than max - if self.attempts > self.max_attempts: - # If too many attempts, prevent student from saving answer and - # seeing rubric. In normal use, students shouldn't see this because - # they won't see the reset button once they're out of attempts. + # Check to see if this problem is closed + if self.closed(): return { 'success': False, - 'error': 'Too many attempts.' + 'error': 'This problem is now closed.' } if self.state != self.INITIAL: diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index c89f5ee848..69c502cd5d 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -43,11 +43,14 @@ class OpenEndedChildTest(unittest.TestCase): 'accept_file_upload' : False, } definition = Mock() + parent = Mock() + parent.closed.return_value = False; descriptor = Mock() def setUp(self): self.openendedchild = OpenEndedChild(test_system, self.location, - self.definition, self.descriptor, self.static_data, self.metadata) + self.definition, self.descriptor, self.static_data, + self.parent, self.metadata) def test_latest_answer_empty(self): @@ -166,6 +169,8 @@ class OpenEndedModuleTest(unittest.TestCase): ''') definition = {'oeparam': oeparam} descriptor = Mock() + parent = Mock() + parent.closed.return_value = False; def setUp(self): test_system.location = self.location @@ -173,7 +178,8 @@ class OpenEndedModuleTest(unittest.TestCase): self.mock_xqueue.send_to_queue.return_value=(None, "Message") test_system.xqueue = {'interface':self.mock_xqueue, 'callback_url':'/', 'default_queuename': 'testqueue', 'waittime': 1} self.openendedmodule = OpenEndedModule(test_system, self.location, - self.definition, self.descriptor, self.static_data, self.metadata) + self.definition, self.descriptor, self.static_data, + self.parent, self.metadata) def test_message_post(self): get = {'feedback': 'feedback text', diff --git a/common/lib/xmodule/xmodule/tests/test_self_assessment.py b/common/lib/xmodule/xmodule/tests/test_self_assessment.py index c5fb82e412..74018cf101 100644 --- a/common/lib/xmodule/xmodule/tests/test_self_assessment.py +++ b/common/lib/xmodule/xmodule/tests/test_self_assessment.py @@ -30,6 +30,8 @@ class SelfAssessmentTest(unittest.TestCase): metadata = {'attempts': '10'} descriptor = Mock() + parent = Mock() + parent.closed.return_value = False def setUp(self): state = json.dumps({'student_answers': ["Answer 1", "answer 2", "answer 3"], @@ -49,7 +51,8 @@ class SelfAssessmentTest(unittest.TestCase): self.module = SelfAssessmentModule(test_system, self.location, self.definition, self.descriptor, - static_data, state, metadata=self.metadata) + static_data, self.parent, + state, metadata=self.metadata) def test_get_html(self): html = self.module.get_html(test_system) @@ -72,6 +75,7 @@ class SelfAssessmentTest(unittest.TestCase): # if we now assess as right, skip the REQUEST_HINT state self.module.save_answer({'student_answer': 'answer 4'}, test_system) + self.parent.closed.assert_called_with() self.module.save_assessment({'assessment': '1'}, test_system) self.assertEqual(self.module.state, self.module.DONE) From 97a32f8ced0d4979022c5513eedeec4f4e0a9ee1 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Mon, 4 Feb 2013 12:45:40 -0500 Subject: [PATCH 25/37] Remove extra styling. --- common/lib/xmodule/xmodule/css/combinedopenended/display.scss | 1 - 1 file changed, 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/css/combinedopenended/display.scss b/common/lib/xmodule/xmodule/css/combinedopenended/display.scss index eba2910144..efd282329d 100644 --- a/common/lib/xmodule/xmodule/css/combinedopenended/display.scss +++ b/common/lib/xmodule/xmodule/css/combinedopenended/display.scss @@ -406,7 +406,6 @@ section.open-ended-child { background: #F6F6F6; border: 1px solid #ddd; margin-bottom: 20px; - min-height: 5em; @include clearfix; } From 85abc435159d4db466f04c24460b1a53f13f78f6 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Mon, 4 Feb 2013 14:02:42 -0500 Subject: [PATCH 26/37] Add in the ability to handle due dates and grace periods --- .../xmodule/combined_open_ended_module.py | 31 +++++++++++++++---- common/lib/xmodule/xmodule/timeparse.py | 25 +++++++++++++++ 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index e9b3d1d4d0..4c3e8da8c6 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -7,7 +7,11 @@ from lxml import etree from lxml.html import rewrite_links from path import path import os +import dateutil +import dateutil.parser +import datetime import sys +from timeparse import parse_timedelta from pkg_resources import resource_string @@ -155,12 +159,27 @@ class CombinedOpenEndedModule(XModule): self.attempts = instance_state.get('attempts', 0) + #Allow reset is true if student has failed the criteria to move to the next child task self.allow_reset = instance_state.get('ready_to_reset', False) self.max_attempts = int(self.metadata.get('attempts', MAX_ATTEMPTS)) self.is_scored = self.metadata.get('is_graded', IS_SCORED) in TRUE_DICT self.accept_file_upload = self.metadata.get('accept_file_upload', ACCEPT_FILE_UPLOAD) in TRUE_DICT + display_due_date_string = self.metadata.get('due', None) + if display_due_date_string is not None: + self.display_due_date = dateutil.parser.parse(display_due_date_string) + else: + self.display_due_date = None + + grace_period_string = self.metadata.get('graceperiod', None) + if grace_period_string is not None and self.display_due_date: + self.grace_period = parse_timedelta(grace_period_string) + self.close_date = self.display_due_date + self.grace_period + else: + self.grace_period = None + self.close_date = self.display_due_date + # 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)) @@ -189,12 +208,12 @@ class CombinedOpenEndedModule(XModule): self.setup_next_task() def closed(self): - return True - #''' Is the student still allowed to submit answers? ''' - #if self.attempts == self.max_attempts: - # return True - #if self.close_date is not None and datetime.datetime.utcnow() > self.close_date: - # return True + ''' Is the student still allowed to submit answers? ''' + if self.attempts == self.max_attempts: + return True + if self.close_date is not None and datetime.datetime.utcnow() > self.close_date: + return True + def get_tag_name(self, xml): diff --git a/common/lib/xmodule/xmodule/timeparse.py b/common/lib/xmodule/xmodule/timeparse.py index 36c0f725e5..605662654d 100644 --- a/common/lib/xmodule/xmodule/timeparse.py +++ b/common/lib/xmodule/xmodule/timeparse.py @@ -2,9 +2,14 @@ Helper functions for handling time in the format we like. """ import time +import re +from datetime import timedelta TIME_FORMAT = "%Y-%m-%dT%H:%M" +TIMEDELTA_REGEX = re.compile(r'^((?P\d+?) day(?:s?))?(\s)?((?P\d+?) hour(?:s?))?(\s)?((?P\d+?) minute(?:s)?)?(\s)?((?P\d+?) second(?:s)?)?$') + + def parse_time(time_str): """ Takes a time string in TIME_FORMAT @@ -20,3 +25,23 @@ def stringify_time(time_struct): Convert a time struct to a string """ return time.strftime(TIME_FORMAT, time_struct) + +def parse_timedelta(time_str): + """ + time_str: A string with the following components: + day[s] (optional) + hour[s] (optional) + minute[s] (optional) + second[s] (optional) + + Returns a datetime.timedelta parsed from the string + """ + parts = TIMEDELTA_REGEX.match(time_str) + if not parts: + return + parts = parts.groupdict() + time_params = {} + for (name, param) in parts.iteritems(): + if param: + time_params[name] = int(param) + return timedelta(**time_params) From 7860e013adbceddb8066a26ff035917f9c3931ac Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Mon, 4 Feb 2013 14:25:23 -0500 Subject: [PATCH 27/37] Fix unit tests to handle new max score requirements. --- .../lib/xmodule/xmodule/tests/test_combined_open_ended.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index c89f5ee848..e7f927fc9e 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -30,9 +30,10 @@ class OpenEndedChildTest(unittest.TestCase): Response Quality + ''' - max_score = 4 + max_score = 1 static_data = { 'max_attempts': 20, @@ -270,9 +271,10 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): Response Quality + ''' - max_score = 3 + max_score = 1 metadata = {'attempts': '10', 'max_score': max_score} From 37900c3f7642c93ab2cf8b94427592c43a7adb4c Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Tue, 5 Feb 2013 15:37:20 -0500 Subject: [PATCH 28/37] Add more error logging for date parsing and make the closure checking more robust --- .../xmodule/combined_open_ended_module.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index 4c3e8da8c6..d85b0f6e17 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -168,14 +168,22 @@ class CombinedOpenEndedModule(XModule): display_due_date_string = self.metadata.get('due', None) if display_due_date_string is not None: - self.display_due_date = dateutil.parser.parse(display_due_date_string) + try: + self.display_due_date = dateutil.parser.parse(display_due_date_string) + except ValueError: + log.error("Could not parse due date {0} for location {1}".format(display_due_date_string, location)) + raise else: self.display_due_date = None grace_period_string = self.metadata.get('graceperiod', None) if grace_period_string is not None and self.display_due_date: - self.grace_period = parse_timedelta(grace_period_string) - self.close_date = self.display_due_date + self.grace_period + try: + self.grace_period = parse_timedelta(grace_period_string) + self.close_date = self.display_due_date + self.grace_period + except: + log.error("Error parsing the grace period {0} for location {1}".format(grace_period_string, location)) + raise else: self.grace_period = None self.close_date = self.display_due_date @@ -209,7 +217,7 @@ class CombinedOpenEndedModule(XModule): def closed(self): ''' Is the student still allowed to submit answers? ''' - if self.attempts == self.max_attempts: + if self.attempts >= self.max_attempts: return True if self.close_date is not None and datetime.datetime.utcnow() > self.close_date: return True From 018412ca262718054ef6736473fa66c8a886bbb2 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Tue, 5 Feb 2013 15:41:23 -0500 Subject: [PATCH 29/37] Return False to make this code clearer --- common/lib/xmodule/xmodule/combined_open_ended_module.py | 1 + 1 file changed, 1 insertion(+) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index d85b0f6e17..ebcb044783 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -221,6 +221,7 @@ class CombinedOpenEndedModule(XModule): return True if self.close_date is not None and datetime.datetime.utcnow() > self.close_date: return True + return False From c70531ae27514aeebbc97e7d047452976af05b47 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Tue, 5 Feb 2013 16:00:16 -0500 Subject: [PATCH 30/37] Move closed check logic into the OpenEndedChild --- .../xmodule/combined_open_ended_module.py | 18 +++++------------- common/lib/xmodule/xmodule/openendedchild.py | 9 +++++---- 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index ebcb044783..15a101e876 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -210,20 +210,12 @@ class CombinedOpenEndedModule(XModule): 'rubric': definition['rubric'], 'display_name': self.display_name, 'accept_file_upload': self.accept_file_upload, + 'close_date': self.close_date } self.task_xml = definition['task_xml'] self.setup_next_task() - def closed(self): - ''' Is the student still allowed to submit answers? ''' - if self.attempts >= self.max_attempts: - return True - if self.close_date is not None and datetime.datetime.utcnow() > self.close_date: - return True - return False - - def get_tag_name(self, xml): """ @@ -306,7 +298,7 @@ class CombinedOpenEndedModule(XModule): self.current_task_parsed_xml = self.current_task_descriptor.definition_from_xml(etree_xml, self.system) if current_task_state is None and self.current_task_number == 0: self.current_task = child_task_module(self.system, self.location, - self.current_task_parsed_xml, self.current_task_descriptor, self.static_data, self) + self.current_task_parsed_xml, self.current_task_descriptor, self.static_data) self.task_states.append(self.current_task.get_instance_state()) self.state = self.ASSESSING elif current_task_state is None and self.current_task_number > 0: @@ -322,7 +314,7 @@ class CombinedOpenEndedModule(XModule): }) self.current_task = child_task_module(self.system, self.location, self.current_task_parsed_xml, self.current_task_descriptor, self.static_data, - self, instance_state=current_task_state) + instance_state=current_task_state) self.task_states.append(self.current_task.get_instance_state()) self.state = self.ASSESSING else: @@ -330,7 +322,7 @@ class CombinedOpenEndedModule(XModule): current_task_state = self.overwrite_state(current_task_state) self.current_task = child_task_module(self.system, self.location, self.current_task_parsed_xml, self.current_task_descriptor, self.static_data, - self, instance_state=current_task_state) + instance_state=current_task_state) return True @@ -442,7 +434,7 @@ class CombinedOpenEndedModule(XModule): task_parsed_xml = task_descriptor.definition_from_xml(etree_xml, self.system) task = children['modules'][task_type](self.system, self.location, task_parsed_xml, task_descriptor, - self.static_data, self, instance_state=task_state) + self.static_data, instance_state=task_state) last_response = task.latest_answer() last_score = task.latest_score() last_post_assessment = task.latest_post_assessment(self.system) diff --git a/common/lib/xmodule/xmodule/openendedchild.py b/common/lib/xmodule/xmodule/openendedchild.py index 8c35fb0cae..9fa8959c95 100644 --- a/common/lib/xmodule/xmodule/openendedchild.py +++ b/common/lib/xmodule/xmodule/openendedchild.py @@ -73,7 +73,7 @@ class OpenEndedChild(object): 'done': 'Problem complete', } - def __init__(self, system, location, definition, descriptor, static_data, parent, + def __init__(self, system, location, definition, descriptor, static_data, instance_state=None, shared_state=None, **kwargs): # Load instance state if instance_state is not None: @@ -87,8 +87,6 @@ class OpenEndedChild(object): # Scores are on scale from 0 to max_score self.history = instance_state.get('history', []) - self.parent = parent - self.state = instance_state.get('state', self.INITIAL) self.created = instance_state.get('created', False) @@ -100,6 +98,7 @@ class OpenEndedChild(object): self.rubric = static_data['rubric'] self.display_name = static_data['display_name'] self.accept_file_upload = static_data['accept_file_upload'] + self.close_date = static_data['close_date'] # Used for progress / grading. Currently get credit just for # completion (doesn't matter if you self-assessed correct/incorrect). @@ -119,7 +118,9 @@ class OpenEndedChild(object): pass def closed(self): - return self.parent.closed() + if self.close_date is not None and datetime.utcnow() > self.close_date: + return True + return False def latest_answer(self): """Empty string if not available""" From 31c89e6d031494e737ab9358869365db8cffb4fc Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Tue, 5 Feb 2013 16:06:14 -0500 Subject: [PATCH 31/37] Consolidate error messages. --- common/lib/xmodule/xmodule/open_ended_module.py | 8 +++----- common/lib/xmodule/xmodule/openendedchild.py | 16 ++++++++++++++++ .../xmodule/xmodule/self_assessment_module.py | 8 +++----- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/common/lib/xmodule/xmodule/open_ended_module.py b/common/lib/xmodule/xmodule/open_ended_module.py index 2d6970e162..9d2e3e6a54 100644 --- a/common/lib/xmodule/xmodule/open_ended_module.py +++ b/common/lib/xmodule/xmodule/open_ended_module.py @@ -550,11 +550,9 @@ class OpenEndedModule(openendedchild.OpenEndedChild): """ # Once we close the problem, we should not allow students # to save answers - if self.closed(): - return { - 'success': False, - 'error': 'Problem is closed.' - } + closed, msg = self.check_if_closed() + if closed: + return msg if self.state != self.INITIAL: return self.out_of_sync_error(get) diff --git a/common/lib/xmodule/xmodule/openendedchild.py b/common/lib/xmodule/xmodule/openendedchild.py index 9fa8959c95..185472d0da 100644 --- a/common/lib/xmodule/xmodule/openendedchild.py +++ b/common/lib/xmodule/xmodule/openendedchild.py @@ -122,6 +122,22 @@ class OpenEndedChild(object): return True return False + def check_if_closed(self): + if self.closed(): + return True, { + 'success': False, + 'error': 'This problem is now closed.' + } + elif self.attempts > self.max_attempts: + return True, { + 'success': False, + 'error': 'Too many attempts.' + } + else: + return False, {} + + + def latest_answer(self): """Empty string if not available""" if not self.history: diff --git a/common/lib/xmodule/xmodule/self_assessment_module.py b/common/lib/xmodule/xmodule/self_assessment_module.py index bb8b46559d..14d5c31fc2 100644 --- a/common/lib/xmodule/xmodule/self_assessment_module.py +++ b/common/lib/xmodule/xmodule/self_assessment_module.py @@ -190,11 +190,9 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild): or 'rubric_html' (if success). """ # Check to see if this problem is closed - if self.closed(): - return { - 'success': False, - 'error': 'This problem is now closed.' - } + closed, msg = self.check_if_closed() + if closed: + return msg if self.state != self.INITIAL: return self.out_of_sync_error(get) From a7588410b2fccf86759c8c6ab406d2240abb056d Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Tue, 5 Feb 2013 16:11:04 -0500 Subject: [PATCH 32/37] Fix tests --- .../xmodule/xmodule/tests/test_combined_open_ended.py | 10 ++++------ .../lib/xmodule/xmodule/tests/test_self_assessment.py | 6 ++---- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index 69c502cd5d..e58e8c024b 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -41,16 +41,15 @@ class OpenEndedChildTest(unittest.TestCase): 'max_score': max_score, 'display_name': 'Name', 'accept_file_upload' : False, + 'close_date': None } definition = Mock() - parent = Mock() - parent.closed.return_value = False; descriptor = Mock() def setUp(self): self.openendedchild = OpenEndedChild(test_system, self.location, self.definition, self.descriptor, self.static_data, - self.parent, self.metadata) + self.metadata) def test_latest_answer_empty(self): @@ -158,6 +157,7 @@ class OpenEndedModuleTest(unittest.TestCase): 'max_score': max_score, 'display_name': 'Name', 'accept_file_upload': False, + 'close_date': None } oeparam = etree.XML(''' @@ -169,8 +169,6 @@ class OpenEndedModuleTest(unittest.TestCase): ''') definition = {'oeparam': oeparam} descriptor = Mock() - parent = Mock() - parent.closed.return_value = False; def setUp(self): test_system.location = self.location @@ -179,7 +177,7 @@ class OpenEndedModuleTest(unittest.TestCase): test_system.xqueue = {'interface':self.mock_xqueue, 'callback_url':'/', 'default_queuename': 'testqueue', 'waittime': 1} self.openendedmodule = OpenEndedModule(test_system, self.location, self.definition, self.descriptor, self.static_data, - self.parent, self.metadata) + self.metadata) def test_message_post(self): get = {'feedback': 'feedback text', diff --git a/common/lib/xmodule/xmodule/tests/test_self_assessment.py b/common/lib/xmodule/xmodule/tests/test_self_assessment.py index 74018cf101..3cae123ffe 100644 --- a/common/lib/xmodule/xmodule/tests/test_self_assessment.py +++ b/common/lib/xmodule/xmodule/tests/test_self_assessment.py @@ -30,8 +30,6 @@ class SelfAssessmentTest(unittest.TestCase): metadata = {'attempts': '10'} descriptor = Mock() - parent = Mock() - parent.closed.return_value = False def setUp(self): state = json.dumps({'student_answers': ["Answer 1", "answer 2", "answer 3"], @@ -47,11 +45,12 @@ class SelfAssessmentTest(unittest.TestCase): 'max_score': 1, 'display_name': "Name", 'accept_file_upload' : False, + 'close_date': None } self.module = SelfAssessmentModule(test_system, self.location, self.definition, self.descriptor, - static_data, self.parent, + static_data, state, metadata=self.metadata) def test_get_html(self): @@ -75,7 +74,6 @@ class SelfAssessmentTest(unittest.TestCase): # if we now assess as right, skip the REQUEST_HINT state self.module.save_answer({'student_answer': 'answer 4'}, test_system) - self.parent.closed.assert_called_with() self.module.save_assessment({'assessment': '1'}, test_system) self.assertEqual(self.module.state, self.module.DONE) From 6e7287dd8e47622b8cf6c0bd738507e53432e59c Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Tue, 5 Feb 2013 17:36:06 -0500 Subject: [PATCH 33/37] Fix some bugs in the peer grading problem code. --- .../xmodule/js/src/peergrading/peer_grading_problem.coffee | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common/lib/xmodule/xmodule/js/src/peergrading/peer_grading_problem.coffee b/common/lib/xmodule/xmodule/js/src/peergrading/peer_grading_problem.coffee index 1be8cbbb00..deeb82900b 100644 --- a/common/lib/xmodule/xmodule/js/src/peergrading/peer_grading_problem.coffee +++ b/common/lib/xmodule/xmodule/js/src/peergrading/peer_grading_problem.coffee @@ -309,6 +309,7 @@ class @PeerGradingProblem if Rubric.check_complete() # show button if we have scores for all categories @show_submit_button() + @grade = Rubric.get_total_score() @@ -382,6 +383,7 @@ class @PeerGradingProblem # render common information between calibration and grading render_submission_data: (response) => @content_panel.show() + @error_container.hide() @submission_container.append(@make_paragraphs(response.student_response)) @prompt_container.html(response.prompt) From f9304a88cb486024224670c0f0c470f01af95f73 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Wed, 6 Feb 2013 09:15:24 -0500 Subject: [PATCH 34/37] Remove unneeded error type. --- common/lib/xmodule/xmodule/combined_open_ended_module.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index 7ecca35107..ed2e25ef01 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -54,10 +54,6 @@ HUMAN_TASK_TYPE = { 'openended' : "External Grader", } -class IncorrectMaxScoreError(Exception): - def __init__(self, msg): - self.msg = msg - class CombinedOpenEndedModule(XModule): """ This is a module that encapsulates all open ended grading (self assessment, peer assessment, etc). From f568e49265c35a23f74fbec5070c25a3c76fa2ae Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 6 Feb 2013 15:20:02 -0500 Subject: [PATCH 35/37] Fix oe version tests --- common/lib/xmodule/xmodule/tests/test_combined_open_ended.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index 556bf2953b..3fee05e602 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -320,7 +320,7 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): descriptor = Mock() def setUp(self): - self.combinedoe = CombinedOpenEndedV1Module(test_system, self.location, self.definition, self.descriptor, self.static_data, metadata=self.metadata) + self.combinedoe = CombinedOpenEndedV1Module(test_system, self.location, self.definition, self.descriptor, static_data = self.static_data, metadata=self.metadata) def test_get_tag_name(self): name = self.combinedoe.get_tag_name("Tag") From 733faebaf0bd1a3c4c52c38438409e0490203de8 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 6 Feb 2013 15:28:05 -0500 Subject: [PATCH 36/37] Fix git issue --- common/lib/xmodule/xmodule/tests/test_combined_open_ended.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index cb30af7901..2981ce9833 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -159,11 +159,8 @@ class OpenEndedModuleTest(unittest.TestCase): 'max_score': max_score, 'display_name': 'Name', 'accept_file_upload': False, -<<<<<<< HEAD 'rewrite_content_links' : "", -======= - 'close_date': None ->>>>>>> origin/master + 'close_date': None, } oeparam = etree.XML(''' From cf81fb27e80fdc7bb71ad1ac1f418aed48d46259 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 6 Feb 2013 15:55:39 -0500 Subject: [PATCH 37/37] Add in close date to metadata --- common/lib/xmodule/xmodule/combined_open_ended_modulev1.py | 1 + common/lib/xmodule/xmodule/tests/test_combined_open_ended.py | 1 + 2 files changed, 2 insertions(+) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_modulev1.py b/common/lib/xmodule/xmodule/combined_open_ended_modulev1.py index 1081ef9fba..8bd7df86c1 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_modulev1.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_modulev1.py @@ -203,6 +203,7 @@ class CombinedOpenEndedV1Module(): 'rubric': definition['rubric'], 'display_name': self.display_name, 'accept_file_upload': self.accept_file_upload, + 'close_date' : self.close_date, } self.task_xml = definition['task_xml'] diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index 2981ce9833..c2b27e4953 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -292,6 +292,7 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): 'display_name': 'Name', 'accept_file_upload' : False, 'rewrite_content_links' : "", + 'close_date' : "", } oeparam = etree.XML('''