Merge pull request #1440 from MITx/diana/rubric-ui-improvements

Rubric UI updates

common/lib/xmodule/xmodule/combined_open_ended_module.py

@@ -58,12 +58,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).
@@ -194,15 +188,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 = {

common/lib/xmodule/xmodule/combined_open_ended_rubric.py

@@ -29,10 +29,13 @@ class CombinedOpenEndedRubric(object):
         success = False
         try:
             rubric_categories = self.extract_categories(rubric_xml)
-            html = self.system.render_template('open_ended_rubric.html',
+            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})
             success = True
         except:
             error_message = "[render_rubric] Could not parse the rubric with xml: {0}".format(rubric_xml)
@@ -40,7 +43,7 @@ class CombinedOpenEndedRubric(object):
             raise RubricParsingError(error_message)
         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())
@@ -48,13 +51,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:

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 {
@@ -445,7 +404,6 @@ section.open-ended-child {
     div.short-form-response {
           background: #F6F6F6;
           border: 1px solid #ddd;
-          border-top: 0;
           margin-bottom: 20px;
           overflow-y: auto;
           height: 200px;
@@ -586,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;

common/lib/xmodule/xmodule/grading_service_module.py

@@ -113,7 +113,7 @@ class GradingService(object):
         try:
             if 'rubric' in response_json:
                 rubric = response_json['rubric']
-                rubric_renderer = CombinedOpenEndedRubric(self.system, False)
+                rubric_renderer = CombinedOpenEndedRubric(self.system, view_only)
                 success, rubric_html = rubric_renderer.render_rubric(rubric)
                 response_json['rubric'] = rubric_html
             return response_json

common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee

+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()
+    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
+    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
@@ -222,9 +255,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

common/lib/xmodule/xmodule/js/src/peergrading/peer_grading_problem.coffee

@@ -233,23 +233,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-1)]
-      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()
@@ -317,17 +305,11 @@ 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()
+      @grade = Rubric.get_total_score()
 
 
 
@@ -401,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)
@@ -448,28 +431,5 @@ class @PeerGradingProblem
     @submit_button.show()
 
   setup_score_selection: (max_score) =>
-
-    # first, get rid of all the old inputs, if any.
-    @score_selection_container.html("""
-    <h3>Overall Score</h3>
-    <p>Choose an overall score for this submission.</p>
-    """)
-
-    # Now create new labels and inputs for each possible score.
-    for score in [0..max_score]
-      id = 'score-' + score
-      label = """<label for="#{id}">#{score}</label>"""
-
-      input = """
-              <input type="radio" name="grade-selection" id="#{id}" value="#{score}"/>
-              """       # "  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
-
-#mock_backend = false
-#ajax_url = $('.peer-grading').data('ajax_url')
-#backend = new PeerGradingProblemBackend(ajax_url, mock_backend)
-#$(document).ready(() -> new PeerGradingProblem(backend))
+    $("input[class='score-selection']").change @graded_callback

common/lib/xmodule/xmodule/self_assessment_module.py

@@ -125,7 +125,7 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild):
         if self.state == self.INITIAL:
             return ''
 
-        rubric_renderer = CombinedOpenEndedRubric(system, True)
+        rubric_renderer = CombinedOpenEndedRubric(system, False)
         success, rubric_html = rubric_renderer.render_rubric(self.rubric)
 
         # we'll render it

common/lib/xmodule/xmodule/tests/test_combined_open_ended.py

@@ -31,9 +31,10 @@ class OpenEndedChildTest(unittest.TestCase):
         <category>
         <description>Response Quality</description>
         <option>The response is not a satisfactory answer to the question.  It either fails to address the question or does so in a limited way, with no evidence of higher-order thinking.</option>
+        <option>Second option</option>
         </category>
          </rubric></rubric>'''
-    max_score = 4
+    max_score = 1
 
     static_data = {
             'max_attempts': 20,
@@ -275,9 +276,10 @@ class CombinedOpenEndedModuleTest(unittest.TestCase):
         <category>
         <description>Response Quality</description>
         <option>The response is not a satisfactory answer to the question.  It either fails to address the question or does so in a limited way, with no evidence of higher-order thinking.</option>
+        <option>Second option</option>
         </category>
          </rubric></rubric>'''
-    max_score = 3
+    max_score = 1
 
     metadata = {'attempts': '10', 'max_score': max_score}
 

lms/static/coffee/src/staff_grading/staff_grading.coffee

@@ -212,55 +212,19 @@ class @StaffGrading
 
 
   setup_score_selection: =>
-    # first, get rid of all the old inputs, if any.
-    @grade_selection_container.html("""
-    <h3>Overall Score</h3>
-    <p>Choose an overall score for this submission.</p>
-    """)
-    # Now create new labels and inputs for each possible score.
-    for score in [0..@max_score]
-      id = 'score-' + score
-      label = """<label for="#{id}">#{score}</label>"""
-      input = """
-              <input type="radio" class="grade-selection" name="grade-selection" id="#{id}" value="#{score}"/>
-              """  # "  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()
 
 
   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 +249,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 +263,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

lms/static/sass/course.scss

@@ -44,9 +44,9 @@
 @import "course/gradebook";
 @import "course/tabs";
 @import "course/staff_grading";
+@import "course/rubric";
 @import "course/open_ended_grading";
 
-
 // instructor
 @import "course/instructor/instructor";
 

lms/static/sass/course/_rubric.scss

+.rubric {
+  margin: 40px 0px;
+  tr {
+      margin:10px 0px;
+      height: 100%;
+  }
+  td {
+      padding: 20px 0px 25px 0px;
+      height: 100%;
+      border: 1px black solid;
+      text-align: center;
+  }
+  th {
+      padding: 5px;
+      margin: 5px;
+      text-align: center;
+  }
+  .points-header th {
+      padding: 0px;
+  }
+  .rubric-label
+  {
+      position: relative;
+      padding: 0px 15px 15px 15px;
+      width: 130px;
+      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,
+  .selected-grade .rubric-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;
+      font-size: 16px;
+  }
+}
+

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; 
-      }
-  }
-    
 }
-

lms/templates/instructor/staff_grading.html

@@ -75,6 +75,7 @@
   </p>
   <p class="grade-selection-container">
   </p>
+  <h3>Written Feedback</h3>
   <textarea name="feedback" placeholder="Feedback for student (optional)"
             class="feedback-area" cols="70" ></textarea>
 </div>

lms/templates/open_ended_rubric.html

@@ -8,26 +8,33 @@
     <p>Select the criteria you feel best represents this submission in each category.</p>
     % endif
     <table class="rubric">
+        <tr class="points-header">
+            <th></th>
+        % for i in range(max_score + 1):
+            <th>   
+                ${i} points
+            </th>
+        % endfor
+        </tr>
         % for i in range(len(categories)):
             <% category = categories[i] %>
         <tr>
             <th>${category['description']}</th>
             % for j in range(len(category['options'])):
                 <% option = category['options'][j] %>
+            %if option['selected']:
+            <td class="selected-grade">
+            %else:
             <td>
+            % endif
                 % if view_only:
                     ## if this is the selected rubric block, show it highlighted
-                    % if option['selected']:
-                        <div class="view-only selected-grade">
-                    % else:
-                        <div class="view-only">
-                    % endif
+                        <div class="rubric-label">
                         ${option['text']}
-                        <div class="grade">[${option['points']} points]</div>
                         </div>
                 % else:
                     <input type="radio" class="score-selection" name="score-selection-${i}" id="score-${i}-${j}" value="${option['points']}"/>
-                    <label for="score-${i}-${j}">${option['text']}</label>
+                    <label class="rubric-label" for="score-${i}-${j}">${option['text']}</label>
                 % endif
             </td>
             % endfor

lms/templates/peer_grading/peer_grading_problem.html

@@ -52,7 +52,9 @@
           <p class="rubric-selection-container"></p>
           <p class="score-selection-container">
           </p>
-          <textarea name="feedback" placeholder="Feedback for student (optional)"
+          <h3>Written Feedback</h3>
+          <p>Please include some written feedback as well.</p>
+          <textarea name="feedback" placeholder="Feedback for student"
                     class="feedback-area" cols="70" ></textarea>
           <p class="flag-student-container">Flag this submission for review by course staff (use if the submission contains inappropriate content): <input type="checkbox" class="flag-checkbox" value="student_is_flagged"></p>
         </div>

lms/templates/self_assessment_rubric.html

@@ -2,20 +2,4 @@
     <div class="rubric">
     ${rubric | n }
     </div>
-
-    % if not read_only:
-    <div class="scoring-container">
-    <h3>Scoring</h3>
-    <p>Please select a score below:</p>
-
-    <div class="grade-selection">
-    %for i in xrange(0,max_score+1):
-    <% id = "score-{0}".format(i) %>
-        <input type="radio" class="grade-selection" name="grade-selection" value="${i}" id="${id}">
-        <label for="${id}">${i}</label>
-    %endfor
-    </div>
-    </div>
-    % endif
-
 </div>