MDL-9506 Category and Item now set their own courseid if not set explicitly, and...

other sources. category::set_as_parent has a new constraint: cannot set over children from different courses.
Applied recursion to grade_tree::renumber(), but still some fine-tuning to do, since only grade_items are getting updated in DB.

diff --git a/lib/grade/grade_category.php b/lib/grade/grade_category.php
index da422fd1390cea58d48a1541c488ec63b19c37b0..dce60e2d44bfd01aab4f46ca9fa122454f619cb5 100644 (file)
--- a/lib/grade/grade_category.php
+++ b/lib/grade/grade_category.php
@@ -672,6 +672,11 @@ class grade_category extends grade_object {
      * @return object Grade_item
      */
     function get_grade_item() {
+        if (empty($this->id)) {
+            debugging("Attempt to obtain a grade_category's associated grade_item without the category's ID being set.");
+            return false;
+        }
+
         $grade_items = get_records_select('grade_items', "iteminstance = $this->id AND itemtype = 'category'", null, '*', 0, 1);
         
         if ($grade_items){ 
@@ -718,11 +723,12 @@ class grade_category extends grade_object {
     }
 
     /**
-     * Sets this category as the parent for the given children.
+     * Sets this category as the parent for the given children. If the category's courseid isn't set, it uses that of the children items.
      * A number of constraints are necessary:
      *    - The children must all be of the same type and at the same level
      *    - The children cannot already be top categories
-     *    - The children cannot already have a top category
+     *    - The children cannot already have a top categorya
+     *    - The children all belong to the same course
      * @param array $children An array of fully instantiated grade_category OR grade_item objects
      * @return boolean Success or Failure
      */
@@ -732,6 +738,7 @@ class grade_category extends grade_object {
         // Check type and sortorder of first child
         $first_child = current($children);
         $first_child_type = get_class($first_child);
+        $first_child_courseid = $first_child->courseid;
 
         foreach ($children as $child) {
             if (get_class($child) != $first_child_type) {
@@ -760,6 +767,11 @@ class grade_category extends grade_object {
                 debugging("Attempted to set a category over children that are neither grade_items nor grade_categories.");
                 return false;
             }                
+
+            if ($child->courseid != $first_child_courseid) {
+                debugging("Attempted to set a category over children which do not belong to the same course.");
+                return false;
+            }
         } 
 
         // We passed all the checks, time to set the category as a parent.
@@ -783,6 +795,11 @@ class grade_category extends grade_object {
         $this->load_grade_item();
         $this->grade_item->sortorder = $first_child->get_sortorder();
         
+        // If this->courseid is not set, set it to the first child's courseid
+        if (empty($this->courseid)) {
+            $this->courseid = $first_child_courseid;
+        }
+
         if (!$this->update()) {
             debugging("Could not update this category's sortorder in DB.");
             return false;
@@ -805,5 +822,16 @@ class grade_category extends grade_object {
     function get_name() {
         return $this->fullname;
     }
+
+    /**
+     * Returns this category's grade_item's id. This is specified for cases where we do not
+     * know an object's type, and want to get either an item's id or a category's item's id.
+     *
+     * @return int
+     */
+    function get_item_id() {
+        $this->load_grade_item();
+        return $this->grade_item->id;
+    }
 } 
 ?>

diff --git a/lib/grade/grade_item.php b/lib/grade/grade_item.php
index 9358d56a2db3db0e258cbb486733095e476bd0d6..d91a87a412e071d260610e10e7973f716cc12a86 100644 (file)
--- a/lib/grade/grade_item.php
+++ b/lib/grade/grade_item.php
@@ -479,6 +479,12 @@ class grade_item extends grade_object {
             $this->grademin = 0;
             $this->gradetype = GRADE_TYPE_SCALE;
         }
+
+        // If not set, infer courseid from referenced category
+        if (empty($this->courseid) && (!empty($this->iteminstance) || !empty($this->categoryid))) {
+            $this->load_category();
+            $this->courseid = $this->category->courseid;
+        }
        
         // If sortorder not given, extrapolate one
         if (empty($this->sortorder)) {
@@ -924,5 +930,15 @@ class grade_item extends grade_object {
     function get_name() {
         return $this->itemname;
     } 
+    
+    /**
+     * Returns this grade_item's id. This is specified for cases where we do not
+     * know an object's type, and want to get either an item's id or a category's item's id.
+     *
+     * @return int
+     */
+    function get_item_id() {
+        return $this->id;
+    }
 }
 ?>

diff --git a/lib/grade/grade_tree.php b/lib/grade/grade_tree.php
index a13e9c9efd22bbe284f429ef59016878feccda7d..5fc92dc06633dbd1aa2358e649f434527199e70c 100644 (file)
--- a/lib/grade/grade_tree.php
+++ b/lib/grade/grade_tree.php
@@ -93,7 +93,7 @@ class grade_tree {
         if (!empty($tree)) {
             $this->tree_array = $tree;
         } else {
-            $this->tree_array = $this->get_tree($include_grades);
+            $this->tree_array = $this->get_tree();
         }
         
         if (!empty($this->tree_array)) {
@@ -316,7 +316,7 @@ class grade_tree {
         if (isset($indices[2])) {
             $element_to_splice .= '[' . $indices[1] . "]['children']";
         }
-
+        
         eval("array_splice($element_to_splice, \$position + \$offset, 0, \$destination_array);");
 
         if (!is_object($new_element)) {
@@ -372,87 +372,59 @@ class grade_tree {
     }
 
     /**
-     * One at a time, re-assigns new sort orders for every element in the tree, starting 
-     * with a base number.
+     * One at a time, re-assigns new sort orders for every element in the tree, recursively moving
+     * down and across the tree.
+     * @param int $starting_sortorder Used by recursion to "seed" the first element in each sub-tree
+     * @param array $element A sub-tree given to each layer of recursion. If null, level 0 of recursion is assumed.
      * @return array A debugging array which shows the progression of variables throughout this method. This is very useful
      * to identify problems and implement new functionality.
      */
-    function renumber($starting_sortorder=NULL) {
+    function renumber($starting_sortorder=NULL, $elements=NULL) {
         $sortorder = $starting_sortorder;
         
-        if (empty($starting_sortorder)) { 
+        if (empty($elements) && empty($starting_sortorder)) {
             if (empty($this->first_sortorder)) {
                 debugging("The tree's first_order variable isn't set, you must provide a starting_sortorder to the renumber method.");
                 return false;
             }
             $sortorder = $this->first_sortorder - 1; 
+            $elements = $this->tree_array;
+        } elseif(!empty($elements) && empty($starting_sortorder)) {
+            debugging("Entered second level of recursion without a starting_sortorder.");
         }
-        
-        $newtree = array();
-        $topcatsortorder = 0;
-        $debug = array();
 
-        foreach ($this->tree_array as $topcat) {
-            $sortorder++; 
-            $subcatsortorder = 0;
+        $newtree = array();
+        $this->first_sortorder = $sortorder;
 
-            if (!empty($topcat['children'])) {
-                $topcatsortorder = $sortorder;
+        foreach ($elements as $key => $element) {
+            $this->first_sortorder++;
 
-                foreach ($topcat['children'] as $subcat) {
-                    $sortorder++; 
-                    
-                    if (!empty($subcat['children'])) {
-                        $subcatsortorder = $sortorder;
-                        
-                        foreach ($subcat['children'] as $item) {
-                            $sortorder++;
-                            
-                            $newtree[$topcatsortorder]['children'][$subcatsortorder]['children'][$sortorder] = $item;
-                           
-                            if ($sortorder != $item['object']->sortorder) {
-                                $this->need_update[$item['object']->id] = 
-                                    array('old_sortorder' => $item['object']->sortorder, 
-                                          'new_sortorder' => $sortorder, 
-                                          'previous_sortorder' => $this->get_neighbour_sortorder($item, 'previous'),
-                                          'next_sortorder' => $this->get_neighbour_sortorder($item, 'next'));
-                            }
-                        }
-                        $newtree[$topcatsortorder]['children'][$subcatsortorder]['object'] = $subcat['object'];
-                        $newsortorder = $subcatsortorder; 
-                    } else {
-                        $newtree[$topcatsortorder]['children'][$sortorder] = $subcat; 
-                        $newsortorder = $sortorder; 
-                    } 
-
-                    if ($newsortorder != $subcat['object']->sortorder) {
-                        $this->need_update[$subcat['object']->id] = 
-                            array('old_sortorder' => $subcat['object']->sortorder, 
-                                  'new_sortorder' => $newsortorder, 
-                                  'previous_sortorder' => $this->get_neighbour_sortorder($subcat, 'previous'),
-                                  'next_sortorder' => $this->get_neighbour_sortorder($subcat, 'next'));
-                    }
+            if (!empty($element['children'])) {
+                $newtree[$this->first_sortorder] = $element;
+                $newtree[$this->first_sortorder]['children'] = $this->renumber($this->first_sortorder, $element['children']); 
+            }  else { 
+                
+                $element['object']->previous_sortorder = $this->get_neighbour_sortorder($element, 'previous');
+                $element['object']->next_sortorder = $this->get_neighbour_sortorder($element, 'next');
+                $newtree[$this->first_sortorder] = $element; 
+                
+                if ($this->first_sortorder != $element['object']->sortorder) {
+                    $this->need_update[$element['object']->get_item_id()] = 
+                        array('old_sortorder' => $element['object']->sortorder, 
+                              'new_sortorder' => $this->first_sortorder);
                 }
-                $newtree[$topcatsortorder]['object'] = $topcat['object'];
-                $newsortorder = $topcatsortorder; 
-            } else { 
-                $newsortorder = $sortorder; 
-                $newtree[$sortorder] = $topcat; 
             } 
-            
-            if ($newsortorder != $topcat['object']->sortorder) {
-                $this->need_update[$topcat['object']->id] = 
-                    array('old_sortorder' => $topcat['object']->sortorder, 
-                          'new_sortorder' => $newsortorder, 
-                          'previous_sortorder' => $this->get_neighbour_sortorder($topcat, 'previous'),
-                          'next_sortorder' => $this->get_neighbour_sortorder($topcat, 'next'));
-            }
         }
         
-        $this->tree_array = $newtree;
-        unset($this->first_sortorder);
-        $this->build_tree_filled();
-        return $debug;
+        // If no starting sortorder was given, it means we have finished building the tree, so assign it to this->tree_array. Otherwise return the new tree.
+        if (empty($starting_sortorder)) {
+            $this->tree_array = $newtree;
+            unset($this->first_sortorder);
+            $this->build_tree_filled(); 
+            return true;
+        } else {
+            return $newtree;
+        }
     }
   
     /**
@@ -532,6 +504,9 @@ class grade_tree {
     /**
      * Static method that returns a sorted, nested array of all grade_categories and grade_items for 
      * a given course, or for the entire site if no courseid is given.
+     * @TODO Break this up in more nuclear methods
+     * @TODO Apply recursion to tree-building code (get_tree($first_parent=NULL))
+     *  NOTE the above todos are tricky in this instance because we are building two arrays simultaneously: tree_array and tree_filled
      * @return array
      */
     function get_tree() {
@@ -596,7 +571,7 @@ class grade_tree {
         }
             
         $last_topsortorder = null;
-
+        
         foreach ($topcats as $topcatid => $topcat) {
             $last_subsortorder = null;
 
@@ -945,10 +920,11 @@ class grade_tree {
         $this->need_delete = array();
         $this->need_insert = array();
 
-        foreach ($this->need_update as $id => $sortorders) {
-            if (!set_field('grade_items', 'sortorder', $sortorders['new_sortorder'], 'id', $id)) {
+        // The items' sortorder are updated
+        foreach ($this->need_update as $id => $element) {
+            if (!set_field('grade_items', 'sortorder', $element['new_sortorder'], 'id', $id)) {
                 debugging("Could not update the grade_item's sortorder in DB.");
-            }
+            } 
         } 
 
         $this->need_update = array();

diff --git a/lib/simpletest/grade/simpletest/testgradetree.php b/lib/simpletest/grade/simpletest/testgradetree.php
index 3282ee8223c695f47916893b2a322e2004d7dbc9..b2fae16e62758ebedc03460198168ceb40d558a8 100644 (file)
--- a/lib/simpletest/grade/simpletest/testgradetree.php
+++ b/lib/simpletest/grade/simpletest/testgradetree.php
@@ -35,6 +35,13 @@ global $CFG;
 require_once($CFG->libdir . '/simpletest/testgradelib.php');
 
 class grade_tree_test extends gradelib_test {
+    function test_grade_tree_renumber() {
+        $tree = new grade_tree($this->courseid);
+        $tree1 = $tree;
+        $tree->renumber();
+        $this->assertEqual($tree1->tree_array[1]['object'], $tree->tree_array[1]['object']);
+        $this->assertTrue(empty($tree->need_update));
+    }
     
     function test_grade_tree_locate_element() {
         $tree = new grade_tree($this->courseid);
@@ -188,14 +195,6 @@ class grade_tree_test extends gradelib_test {
         $this->assertEqual(48, count($tree->tree_array, COUNT_RECURSIVE));
     }
     
-    function test_grade_tree_renumber() {
-        $tree = new grade_tree($this->courseid);
-        $tree1 = $tree;
-        $tree->renumber();
-        $this->assertEqual($tree1->tree_array[1]['object'], $tree->tree_array[1]['object']);
-        $this->assertTrue(empty($tree->need_update));
-    }
-
     function test_grade_tree_remove_element() {
         $tree = new grade_tree($this->courseid);
 
@@ -289,5 +288,4 @@ class grade_tree_test extends gradelib_test {
         $tree = new grade_tree($this->courseid);
         echo $tree->get_edit_tree(); 
     }
-
 }