MDL-10364 fixed de/normalization of calculation formulas in item edit form

grade/report/grader/category.php

@@ -47,7 +47,8 @@ $context = get_context_instance(CONTEXT_COURSE, $course->id);
 $returnurl = 'category.php?id='.$course->id;
 
 // get the grading tree object
-$gtree = new grade_tree($courseid, false);
+// note: total must be first for moving to work correctly, if you want it last moving code must be rewritten!
+$gtree = new grade_tree($courseid, false, false, false);
 
 if (empty($eid)) {
     $element = null;

grade/report/grader/edit_item.php

@@ -20,7 +20,9 @@ $returnurl = 'category.php?id='.$course->id;
 
 $mform = new edit_item_form(qualified_me(), array('id'=>$id));
 if ($item = get_record('grade_items', 'id', $id, 'courseid', $course->id)) {
+    $item->calculation = grade_item::denormalize_formula($item->calculation, $course->id);
     $mform->set_data($item);
+
 } else {
     $mform->set_data(array('courseid'=>$course->id, 'itemtype'=>'manual'));
 }
@@ -33,11 +35,15 @@ if ($mform->is_cancelled()) {
         $data->checkbox = 0; // work around the missing value if checkbox not selected
     }
 
+    if (array_key_exists('calculation', $data)) {
+        $data->calculation = grade_item::normalize_formula($data->calculation, $course->id);
+    }
+
     $grade_item = new grade_item(array('id'=>$id, 'courseid'=>$course->id));
     grade_item::set_properties($grade_item, $data);
 
     if (empty($grade_item->id)) {
-        $grade_item->itemtype = 'manual'; // for all new items to be manual only
+        $grade_item->itemtype = 'manual'; // all new items to be manual only
         $grade_item->insert();
 
     } else {

lib/grade/grade_item.php

@@ -811,19 +811,12 @@ class grade_item extends grade_object {
          * Also if user changes the idnumber the formula does not need to be updated.
          */
 
-        // first detect if we need to update calculation formula from [idnumber] to [#giXXX#] (after backup, etc.)
+        // first detect if we need to change calculation formula from [idnumber] to [#giXXX#] (after backup, etc.)
-        if (!$this->calculation_normalized and preg_match_all('/\[(?!#gi)(.*?)\]/', $this->calculation, $matches)) {
+        if (!$this->calculation_normalized and preg_match('/\[(?!#gi)(.*?)\]/', $this->calculation)) {
-            foreach ($matches[1] as $idnumber) {
+            $this->set_calculation($this->calculation);
-                if ($grade_item = grade_item::fetch(array('courseid'=>$this->courseid, 'idnumber'=>$idnumber))) {
-                    $this->calculation = str_replace('['.$grade_item->idnumber.']', '[#gi'.$grade_item->id.'#]', $this->calculation);
-                }
-            }
-            $this->update(); // update in db if needed
-            $this->calculation_normalized = true;
-            return !empty($this->calculation);
         }
 
-        return true;
+        return !empty($this->calculation);
     }
 
     /**
@@ -832,19 +825,7 @@ class grade_item extends grade_object {
      */
     function get_calculation() {
         if ($this->is_calculated()) {
-            $formula = $this->calculation;
+            return grade_item::denormalize_formula($this->calculation, $this->courseid);
-            // denormalize formula - convert [#giXX#] to [idnumber]
-            if (preg_match_all('/\[#gi([0-9]+)#\]/', $formula, $matches)) {
-                foreach ($matches[1] as $id) {
-                    if ($grade_item = grade_item::fetch(array('id'=>$id))) {
-                        if (!empty($grade_item->idnumber)) {
-                            $formula = str_replace('[#gi'.$grade_item->id.'#]', '['.$grade_item->idnumber.']', $formula);
-                        }
-                    }
-                }
-            }
-
-            return $formula;
 
         } else {
             return NULL;
@@ -859,28 +840,57 @@ class grade_item extends grade_object {
      * @return boolean success
      */
     function set_calculation($formula) {
+        $this->calculation = grade_item::normalize_formula($formula, $this->courseid);
+        $this->calculation_normalized = true;
+        return $this->update();
+    }
+
+    /**
+     * Denormalizes the calculation formula to [idnumber] form
+     * @param string $formula
+     * @return string denormalized string
+     */
+    function denormalize_formula($formula, $courseid) {
+        if (empty($formula)) {
+            return '';
+        }
+
+        // denormalize formula - convert [#giXX#] to [idnumber]
+        if (preg_match_all('/\[#gi([0-9]+)#\]/', $formula, $matches)) {
+            foreach ($matches[1] as $id) {
+                if ($grade_item = grade_item::fetch(array('id'=>$id, 'courseid'=>$courseid))) {
+                    if (!empty($grade_item->idnumber)) {
+                        $formula = str_replace('[#gi'.$grade_item->id.'#]', '['.$grade_item->idnumber.']', $formula);
+                    }
+                }
+            }
+        }
+
+        return $formula;
+
+    }
+
+    /**
+     * Normalizes the calculation formula to [#giXX#] form
+     * @param string $formula
+     * @return string normalized string
+     */
+    function normalize_formula($formula, $courseid) {
         $formula = trim($formula);
 
         if (empty($formula)) {
-            $this->calculation = NULL;
+            return NULL;
 
-        } else {
-            if (strpos($formula, '=') !== 0) {
-                $formula = '='.$formula;
-            }
-
-            // normalize formula - we want grade item ids [#giXXX#] instead of [idnumber]
-            if ($grade_items = grade_item::fetch_all(array('courseid'=>$this->courseid))) {
-                foreach ($grade_items as $grade_item) {
-                    $formula = str_replace('['.$grade_item->idnumber.']', '[#gi'.$grade_item->id.'#]', $formula);
-                }
-            }
-
-            $this->calculation = $formula;
         }
 
-        $this->calculation_normalized = true;
+        // normalize formula - we want grade item ids [#giXXX#] instead of [idnumber]
-        return $this->update();
+        if ($grade_items = grade_item::fetch_all(array('courseid'=>$courseid))) {
+            foreach ($grade_items as $grade_item) {
+                $formula = str_replace('['.$grade_item->idnumber.']', '[#gi'.$grade_item->id.'#]', $formula);
+            }
+        }
+
+        return $formula;
     }
 
     /**