MDL-11957 improving and cleanup in grade overriding

diff --git a/grade/edit/tree/calculation.php b/grade/edit/tree/calculation.php
index f7fce260d3c6b4a4b4d76d7bd6ca00e2b93e0a72..1b87ec5d94da4c56e6d5575baf340010a0b09f48 100644 (file)
--- a/grade/edit/tree/calculation.php
+++ b/grade/edit/tree/calculation.php
@@ -50,7 +50,7 @@ if (!$grade_item = grade_item::fetch(array('id'=>$id, 'courseid'=>$course->id)))
 }
 
 // module items and items without grade can not have calculation
-if (($grade_item->is_normal_item() and !$grade_item->is_outcome_item())
+if (($grade_item->is_external_item() and !$grade_item->is_outcome_item())
   or ($grade_item->gradetype != GRADE_TYPE_VALUE and $grade_item->gradetype != GRADE_TYPE_SCALE)) {
     redirect($returnurl, get_string('errornocalculationallowed', 'grades')); //TODO: localize
 }

diff --git a/grade/edit/tree/grade.php b/grade/edit/tree/grade.php
index d8d5b7dd5a9737c12d9667ac576660a6afca3ecf..52f027ce874e4baecfd70eeec49c0c9bbc25c0c8 100644 (file)
--- a/grade/edit/tree/grade.php
+++ b/grade/edit/tree/grade.php
@@ -96,11 +96,15 @@ if ($grade = get_record('grade_grades', 'itemid', $grade_item->id, 'userid', $us
 
     // always clean existing feedback - grading should not have XSS risk
     if (can_use_html_editor()) {
-        $options = new object();
-        $options->smiley  = false;
-        $options->filter  = false;
-        $options->noclean = false;
-        $grade->feedback       = format_text($grade->feedback, $grade->feedbackformat, $options);
+        if (empty($grade->feedback)) {
+            $grade->feedback  = '';
+        } else {
+            $options = new object();
+            $options->smiley  = false;
+            $options->filter  = false;
+            $options->noclean = false;
+            $grade->feedback  = format_text($grade->feedback, $grade->feedbackformat, $options);
+        }
         $grade->feedbackformat = FORMAT_HTML;
     } else {
         $grade->feedback       = clean_text($grade->feedback, $grade->feedbackformat);
@@ -136,7 +140,8 @@ if ($grade = get_record('grade_grades', 'itemid', $grade_item->id, 'userid', $us
         $grade->finalgrade = format_float($grade->finalgrade, $grade_item->get_decimals());
     }
 
-    $grade->oldgrade = $grade->finalgrade;
+    $grade->oldgrade    = $grade->finalgrade;
+    $grade->oldfeedback = $grade->feedback;
 
     $mform->set_data($grade);
 
@@ -151,8 +156,12 @@ if ($mform->is_cancelled()) {
 } else if ($data = $mform->get_data(false)) {
     $old_grade_grade = new grade_grade(array('userid'=>$data->userid, 'itemid'=>$grade_item->id), true); //might not exist yet
 
+    if (!isset($data->overridden)) {
+        $data->overridden = 0; // checkbox
+    }
+
     // fix no grade for scales
-    if (!isset($data->finalgrade) or $data->finalgrade == $data->oldgrade) {
+    if (($grade_item->is_overridable_item() and !$data->overridden) or !isset($data->finalgrade) or $data->finalgrade == $data->oldgrade) {
         $data->finalgrade = $old_grade_grade->finalgrade;
 
     } else if ($grade_item->gradetype == GRADE_TYPE_SCALE and $data->finalgrade < 1) {
@@ -162,16 +171,21 @@ if ($mform->is_cancelled()) {
         $data->finalgrade = unformat_float($data->finalgrade);
     }
 
-    if (!isset($data->feedback)) {
+    // the overriding of feedback is tricky - we have to care about external items only
+    if (!array_key_exists('feedback', $data) or $data->feedback == $data->oldfeedback) {
         $data->feedback       = $old_grade_grade->feedback;
         $data->feedbackformat = $old_grade_grade->feedbackformat;
     }
     // update final grade or feedback
-    $grade_item->update_final_grade($data->userid, $data->finalgrade, NULL, 'editgrade', $data->feedback, $data->feedbackformat);
+    $grade_item->update_final_grade($data->userid, $data->finalgrade, 'editgrade', $data->feedback, $data->feedbackformat);
 
     $grade_grade = new grade_grade(array('userid'=>$data->userid, 'itemid'=>$grade_item->id), true);
     $grade_grade->grade_item =& $grade_item; // no db fetching
 
+    if (has_capability('moodle/grade:manage', $context) or has_capability('moodle/grade:edit', $context)) {
+        $grade_grade->set_overridden($data->overridden);
+    }
+
     if (has_capability('moodle/grade:manage', $context) or has_capability('moodle/grade:hide', $context)) {
         $hidden      = empty($data->hidden) ? 0: $data->hidden;
         $hiddenuntil = empty($data->hiddenuntil) ? 0: $data->hiddenuntil;
@@ -210,13 +224,6 @@ if ($mform->is_cancelled()) {
         $grade_grade->set_excluded($data->excluded);
     }
 
-    if (isset($data->overridden) and has_capability('moodle/grade:manage', $context) or has_capability('moodle/grade:edit', $context)) {
-        // ignore overridden flag when changing final grade
-        if ($old_grade_grade->finalgrade == $grade_grade->finalgrade) {
-            $grade_grade->set_overridden($data->overridden);
-        }
-    }
-
     // detect cases when we need to do full regrading
     if ($old_grade_grade->excluded != $grade_grade->excluded) {
         $parent = $grade_item->get_parent_category();

diff --git a/grade/edit/tree/grade_form.php b/grade/edit/tree/grade_form.php
index e8a3b517127ea4017b31064ba81b64a3bd8199e2..4c12a7f7f94a159151f70dfcdbbaa3ce95ce36de 100755 (executable)
--- a/grade/edit/tree/grade_form.php
+++ b/grade/edit/tree/grade_form.php
@@ -39,12 +39,17 @@ class edit_grade_form extends moodleform {
         $mform->addElement('static', 'user', get_string('user'));
         $mform->addElement('static', 'itemname', get_string('itemname', 'grades'));
 
+        $mform->addElement('checkbox', 'overridden', get_string('overridden', 'grades'));
+        $mform->setHelpButton('overridden', array(false, get_string('overridden', 'grades'),
+                false, true, false, get_string('overriddenhelp', 'grades')));
+
         /// actual grade - numeric or scale
         if ($grade_item->gradetype == GRADE_TYPE_VALUE) {
             // numeric grade
             $mform->addElement('text', 'finalgrade', get_string('finalgrade', 'grades'));
             $mform->setHelpButton('finalgrade', array(false, get_string('finalgrade', 'grades'),
                     false, true, false, get_string('finalgradehelp', 'grades')));
+            $mform->disabledIf('finalgrade', 'overridden', 'notchecked');
 
         } else if ($grade_item->gradetype == GRADE_TYPE_SCALE) {
             // scale grade
@@ -67,11 +72,9 @@ class edit_grade_form extends moodleform {
             $mform->addElement('select', 'finalgrade', get_string('finalgrade', 'grades'), $scaleopt);
             $mform->setHelpButton('finalgrade', array(false, get_string('finalgrade', 'grades'),
                     false, true, false, get_string('finalgradehelp', 'grades')));
+            $mform->disabledIf('finalgrade', 'overridden', 'notchecked');
         }
 
-        $mform->addElement('advcheckbox', 'overridden', get_string('overridden', 'grades'));
-        $mform->setHelpButton('overridden', array(false, get_string('overridden', 'grades'),
-                false, true, false, get_string('overriddenhelp', 'grades')));
         $mform->addElement('advcheckbox', 'excluded', get_string('excluded', 'grades'));
         $mform->setHelpButton('excluded', array(false, get_string('excluded', 'grades'),
                 false, true, false, get_string('excludedhelp', 'grades')));
@@ -99,9 +102,11 @@ class edit_grade_form extends moodleform {
         $mform->setType('text', PARAM_RAW); // to be cleaned before display, no XSS risk
         $mform->addElement('format', 'feedbackformat', get_string('format'));
         $mform->setHelpButton('feedbackformat', array('textformat', get_string('helpformatting')));
+        //TODO: unfortunately we can not disable html editor for external grades when overridden off :-(
 
         // hidden params
         $mform->addElement('hidden', 'oldgrade');
+        $mform->addElement('hidden', 'oldfeedback');
 
         $mform->addElement('hidden', 'id', 0);
         $mform->setType('id', PARAM_INT);
@@ -161,6 +166,10 @@ class edit_grade_form extends moodleform {
 
         $old_grade_grade = new grade_grade(array('itemid'=>$grade_item->id, 'userid'=>$userid));
 
+        if (!$grade_item->is_overridable_item()) {
+            $mform->removeElement('overridden');
+        }
+
         if ($grade_item->is_hidden()) {
             $mform->hardFreeze('hidden');
         }

diff --git a/grade/edit/tree/item_form.php b/grade/edit/tree/item_form.php
index f22fef0ddb677c7d798bcb3cc585fae630cea885..ed952d9ec105e13f840a612a039d7b92780122ee 100644 (file)
--- a/grade/edit/tree/item_form.php
+++ b/grade/edit/tree/item_form.php
@@ -191,7 +191,7 @@ class edit_item_form extends moodleform {
                 $mform->hardFreeze('scaleid');
 
             } else {
-                if ($grade_item->is_normal_item()) {
+                if ($grade_item->is_external_item()) {
                     // following items are set up from modules and should not be overrided by user
                     $mform->hardFreeze('itemname,idnumber,gradetype,grademax,grademin,scaleid');
                     //$mform->removeElement('calculation');

diff --git a/grade/import/lib.php b/grade/import/lib.php
index fc0c675f33b59bc548b9427f8d1d783660bebbc8..255925809ef7ab1249fc99754584ce3d64b310cc 100755 (executable)
--- a/grade/import/lib.php
+++ b/grade/import/lib.php
@@ -78,7 +78,7 @@ function grade_import_commit($courseid, $importcode, $importfeedback=true, $verb
 
                 // insert each individual grade to this new grade item
                 foreach ($grades as $grade) {
-                    if (!$gradeitem->update_final_grade($grade->userid, $grade->finalgrade, 'import', NULL, $grade->feedback)) {
+                    if (!$gradeitem->update_final_grade($grade->userid, $grade->finalgrade, 'import', $grade->feedback, FORMAT_MOODLE)) {
                         $failed = true;
                         break 2;
                     }
@@ -118,7 +118,7 @@ function grade_import_commit($courseid, $importcode, $importfeedback=true, $verb
                     if (!$importfeedback) {
                         $grade->feedback = false; // ignore it
                     }
-                    if (!$gradeitem->update_final_grade($grade->userid, $grade->finalgrade, 'import', NULL, $grade->feedback)) {
+                    if (!$gradeitem->update_final_grade($grade->userid, $grade->finalgrade, 'import', $grade->feedback)) {
                         $failed = 1;
                         break 2;
                     }

diff --git a/grade/lib.php b/grade/lib.php
index c217ddaf0f3297e1fb44fd56a191f57affb1e2c6..e850926b951fe52f78d85e34b6b40c55aa5e5d99 100644 (file)
--- a/grade/lib.php
+++ b/grade/lib.php
@@ -1176,8 +1176,10 @@ class grade_tree {
         }
 
         static $stredit = null;
+        static $strfeedback = null;
         if (is_null($stredit)) {
             $stredit = get_string('edit');
+            $strfeedback = get_string('feedback');
         }
 
         $object = $element['object'];
@@ -1208,9 +1210,10 @@ class grade_tree {
                 }
                 $url = $gpr->add_url_params($url);
                 if (!empty($object->feedback)) {
-                    $feedback = format_text($object->feedback, $object->feedbackformat);
-                    $function = "return overlib('".s(ltrim($object->feedback)."', FULLHTML);");
-                    $overlib = 'onmouseover="'.$function.'" onmouseout="return nd();"';
+                    $feedback = addslashes_js(trim(format_string($object->feedback, $object->feedbackformat)));
+                    $function = "return overlib('$feedback', BORDER, 0, FGCLASS, 'feedback', "
+                              ."CAPTIONFONTCLASS, 'caption', CAPTION, '$strfeedback');";
+                    $overlib = 'onmouseover="'.s($function).'" onmouseout="return nd();"';
                 }
                 break;
 
@@ -1333,7 +1336,7 @@ class grade_tree {
             $streditcalculation = get_string('editcalculation', 'grades');
 
             // show calculation icon only when calculation possible
-            if ((!$object->is_normal_item() or $object->is_outcome_item())
+            if ((!$object->is_external_item() or $object->is_outcome_item())
               and ($object->gradetype == GRADE_TYPE_SCALE or $object->gradetype == GRADE_TYPE_VALUE)) {
                 $url = $CFG->wwwroot.'/grade/edit/tree/calculation.php?courseid='.$this->courseid.'&id='.$object->id;
                 $url = $gpr->add_url_params($url);

diff --git a/grade/report/grader/lib.php b/grade/report/grader/lib.php
index 18f3f502bbdedb448dfe400537ceec9d7199a753..d5614f8ab6b1bcc7fdcda9c8c97ff1cb61fd387d 100644 (file)
--- a/grade/report/grader/lib.php
+++ b/grade/report/grader/lib.php
@@ -160,7 +160,6 @@ class grade_report_grader extends grade_report {
         foreach ($data as $varname => $postedvalue) {
 
             $needsupdate = false;
-            $note = false; // TODO implement note??
 
             // skip, not a grade nor feedback
             if (strpos($varname, 'grade') === 0) {
@@ -210,7 +209,7 @@ class grade_report_grader extends grade_report {
                 }
             }
 
-            $grade_item->update_final_grade($userid, $finalgrade, 'gradebook', $note, $feedback);
+            $grade_item->update_final_grade($userid, $finalgrade, 'gradebook', $feedback, FORMAT_MOODLE);
         }
 
         return true;

diff --git a/lib/grade/grade_grade.php b/lib/grade/grade_grade.php
index 646324f2a0230aa7608a230df957b1cf7e56a138..3cfa6144578797bfc3237acd43c626b84fcdf1a5 100644 (file)
--- a/lib/grade/grade_grade.php
+++ b/lib/grade/grade_grade.php
@@ -243,9 +243,10 @@ class grade_grade extends grade_object {
     /**
      * Set the overridden status of grade
      * @param boolean $state requested overridden state
+     * @param boolean $refresh refresh grades from external activities if needed
      * @return boolean true is db state changed
      */
-    function set_overridden($state) {
+    function set_overridden($state, $refresh = true) {
         if (empty($this->overridden) and $state) {
             $this->overridden = time();
             $this->update();
@@ -254,6 +255,12 @@ class grade_grade extends grade_object {
         } else if (!empty($this->overridden) and !$state) {
             $this->overridden = 0;
             $this->update();
+
+            if ($refresh) {
+                //refresh when unlocking
+                $this->grade_item->refresh_grades($this->userid);
+            }
+
             return true;
         }
         return false;

diff --git a/lib/grade/grade_item.php b/lib/grade/grade_item.php
index 4ad8937a8537832370cdc876e8dad577c4ab18fd..7c428e3bc979f37abc4d1b2254616dac3be6028c 100644 (file)
--- a/lib/grade/grade_item.php
+++ b/lib/grade/grade_item.php
@@ -874,11 +874,19 @@ class grade_item extends grade_object {
     }
 
     /**
-     * Is the grade item normal - associated with module, plugin or something else?
+     * Is the grade item external - associated with module, plugin or something else?
      * @return boolean
      */
-    function is_normal_item() {
-        return ($this->itemtype != 'course' and $this->itemtype != 'category' and $this->itemtype != 'manual');
+    function is_external_item() {
+        return ($this->itemtype == 'mod');
+    }
+
+    /**
+     * Is the grade item overridable
+     * @return boolean
+     */
+    function is_overridable_item() {
+        return !$this->is_outcome_item() and ($this->is_external_item() or $this->is_calculated() or $this->is_course_item() or $this->is_category_item());
     }
 
     /**
@@ -886,7 +894,7 @@ class grade_item extends grade_object {
      * @return boolean
      */
     function is_raw_used() {
-        return ($this->is_normal_item() and !$this->is_calculated() and !$this->is_outcome_item());
+        return ($this->is_external_item() and !$this->is_calculated() and !$this->is_outcome_item());
     }
 
     /**
@@ -1291,7 +1299,7 @@ class grade_item extends grade_object {
      * @param int $feedbackformat
      * @return boolean success
      */
-    function update_final_grade($userid, $finalgrade=false, $source=NULL, $note=NULL, $feedback=false, $feedbackformat=FORMAT_MOODLE, $usermodified=null) {
+    function update_final_grade($userid, $finalgrade=false, $source=NULL, $feedback=false, $feedbackformat=FORMAT_MOODLE, $usermodified=null) {
         global $USER, $CFG;
 
         if (empty($usermodified)) {
@@ -1328,17 +1336,14 @@ class grade_item extends grade_object {
         $oldgrade->feedback       = $grade->feedback;
         $oldgrade->feedbackformat = $grade->feedbackformat;
 
-        if ($finalgrade !== false or $feedback !== false) {
-            if (($this->is_outcome_item() or $this->is_manual_item()) and !$this->is_calculated()) {
-                // final grades updated only by user - no need for overriding
-                $grade->overridden = 0;
-
-            } else {
+        // changed grade?
+        if ($finalgrade !== false) {
+            if ($this->is_overridable_item()) {
                 $grade->overridden = time();
+            } else {
+                $grade->overridden = 0;
             }
-        }
 
-        if ($finalgrade !== false)  {
             if (!is_null($finalgrade)) {
                 $finalgrade = bounded_number($this->grademin, $finalgrade, $this->grademax);
             } else {
@@ -1349,6 +1354,11 @@ class grade_item extends grade_object {
 
         // do we have comment from teacher?
         if ($feedback !== false) {
+            if ($this->is_external_item()) {
+                // external items (modules, plugins) may have own feedback
+                $grade->overridden = time();
+            }
+
             $grade->feedback       = $feedback;
             $grade->feedbackformat = $feedbackformat;
         }
@@ -1360,6 +1370,9 @@ class grade_item extends grade_object {
                 or $grade->feedback       !== $oldgrade->feedback
                 or $grade->feedbackformat !== $oldgrade->feedbackformat) {
             $result = $grade->update($source);
+        } else {
+            // no grade change
+            return $result;
         }
 
         if (!$result) {
@@ -1399,7 +1412,7 @@ class grade_item extends grade_object {
      * @param int $feedbackformat
      * @return boolean success
      */
-    function update_raw_grade($userid, $rawgrade=false, $source=NULL, $note=NULL, $feedback=false, $feedbackformat=FORMAT_MOODLE, $usermodified=null) {
+    function update_raw_grade($userid, $rawgrade=false, $source=NULL, $feedback=false, $feedbackformat=FORMAT_MOODLE, $usermodified=null) {
         global $USER;
 
         if (empty($usermodified)) {
@@ -1409,8 +1422,7 @@ class grade_item extends grade_object {
         $result = true;
 
         // calculated grades can not be updated; course and category can not be updated  because they are aggregated
-        if ($this->is_calculated() or $this->is_outcome_item() or !$this->is_normal_item()
-         or $this->gradetype == GRADE_TYPE_NONE or $this->is_locked()) {
+        if (!$this->is_raw_used() or $this->gradetype == GRADE_TYPE_NONE or $this->is_locked()) {
             return false;
         }
 

diff --git a/lib/gradelib.php b/lib/gradelib.php
index 68eef434bf34bed79aca8771f83276cf824ee1ad..9608f065f5fedcaff3a814662d48ecebc5bb6d8e 100644 (file)
--- a/lib/gradelib.php
+++ b/lib/gradelib.php
@@ -207,7 +207,7 @@ function grade_update($source, $courseid, $itemtype, $itemmodule, $iteminstance,
         }
 
         // update or insert the grade
-        if (!$grade_item->update_raw_grade($userid, $rawgrade, $source, null, $feedback, $feedbackformat, $usermodified)) {
+        if (!$grade_item->update_raw_grade($userid, $rawgrade, $source, $feedback, $feedbackformat, $usermodified)) {
             $failed = true;
         }
     }