MDL-9506 Resolved recursion issues in grade_tree:renumber and grade_tree::get_neighbo...

diff --git a/lib/grade/grade_tree.php b/lib/grade/grade_tree.php
index 5fc92dc06633dbd1aa2358e649f434527199e70c..8001e6e0d045681f5698fcb9feba486804b39684 100644 (file)
--- a/lib/grade/grade_tree.php
+++ b/lib/grade/grade_tree.php
@@ -398,22 +398,22 @@ class grade_tree {
 
         foreach ($elements as $key => $element) {
             $this->first_sortorder++;
+            $new_sortorder = $this->first_sortorder;
 
             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);
-                }
             } 
+            
+            if ($new_sortorder != $element['object']->sortorder) {
+                $this->need_update[$element['object']->get_item_id()] = 
+                    array('old_sortorder' => $element['object']->sortorder, 
+                          'new_sortorder' => $new_sortorder);
+            }
         }
         
         // 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.
@@ -459,7 +459,7 @@ class grade_tree {
      * @param array $array of elements to search. Defaults to $this->tree_array
      * @return int Sortorder (or null if none found)
      */
-    function get_neighbour_sortorder($element, $position, $array=null) {
+    function get_neighbour_sortorder($element, $position, $array=null, $lastsortorder=null) {
         if (empty($this->tree_array) || empty($element) || empty($position) || !in_array($position, array('previous', 'next'))) {
             return null;
         }
@@ -469,36 +469,44 @@ class grade_tree {
         if (empty($object)) {
             debugging("Invalid element given to grade_tree::get_neighbour_sortorder.");
             return false;
-        }
-        
+        } 
         if (empty($array)) {
             $array = $this->tree_array;
-        }
-
-        $lastsortorder = null;
+        } 
+        $result = null;
+    
         $returnnextelement = false;
+        $count = 0;
 
         foreach ($array as $sortorder => $child) {
+            
             if ($returnnextelement) {
                 return $sortorder;
             }
-
+            
             if ($object->get_sortorder() == $sortorder) {
                 if ($position == 'previous') {
-                    return $lastsortorder;
+                    if ($count > 0) {
+                        return $lastsortorder;
+                    } 
                 } elseif ($position == 'next') {
                     $returnnextelement = true;
                 }
                 continue;
             }
+            
+            $lastsortorder = $sortorder;
 
             if (!empty($child['children'])) {
-                return $this->get_neighbour_sortorder($element, $position, $child['children']);
-            }
+                $result = $this->get_neighbour_sortorder($element, $position, $child['children'], $lastsortorder);
+                if ($result) {
+                    break;
+                }
+            } 
 
-            $lastsortorder = $sortorder;
+            $count++;
         }
-        return null;
+        return $result;
     }
 
     /**

diff --git a/lib/simpletest/grade/simpletest/testgradetree.php b/lib/simpletest/grade/simpletest/testgradetree.php
index b2fae16e62758ebedc03460198168ceb40d558a8..790c72ecd28c6c8d7e7c4242bb09b4ed9c982e27 100644 (file)
--- a/lib/simpletest/grade/simpletest/testgradetree.php
+++ b/lib/simpletest/grade/simpletest/testgradetree.php
@@ -35,6 +35,31 @@ global $CFG;
 require_once($CFG->libdir . '/simpletest/testgradelib.php');
 
 class grade_tree_test extends gradelib_test {
+    function test_grade_tree_get_neighbour_sortorder() {
+        $tree = new grade_tree($this->courseid);
+        
+        $element = $tree->locate_element(4);
+        $this->assertEqual(3, $tree->get_neighbour_sortorder($element, 'previous'));
+        $this->assertNull($tree->get_neighbour_sortorder($element, 'next'));
+        
+        $element = $tree->locate_element(3);
+        $this->assertEqual(4, $tree->get_neighbour_sortorder($element, 'next'));
+        $this->assertNull($tree->get_neighbour_sortorder($element, 'previous'));
+        
+        $element = $tree->locate_element(1);
+        $this->assertNull($tree->get_neighbour_sortorder($element, 'previous'));
+        $this->assertEqual(7, $tree->get_neighbour_sortorder($element, 'next'));
+        
+        $element = $tree->locate_element(7);
+        $this->assertEqual(1, $tree->get_neighbour_sortorder($element, 'previous'));
+        $this->assertEqual(8, $tree->get_neighbour_sortorder($element, 'next'));
+        
+        $element = $tree->locate_element(8);
+        $this->assertEqual(7, $tree->get_neighbour_sortorder($element, 'previous'));
+        $this->assertNull($tree->get_neighbour_sortorder($element, 'next'));
+        
+    }
+    
     function test_grade_tree_renumber() {
         $tree = new grade_tree($this->courseid);
         $tree1 = $tree;
@@ -58,26 +83,6 @@ class grade_tree_test extends gradelib_test {
         $this->assertEqual('singleparentitem1', $element->element['object']->itemname); 
     }
     
-    function test_grade_tree_get_neighbour_sortorder() {
-        $tree = new grade_tree($this->courseid);
-        
-        $element = $tree->locate_element(4);
-        $this->assertEqual(3, $tree->get_neighbour_sortorder($element, 'previous'));
-        $this->assertNull($tree->get_neighbour_sortorder($element, 'next'));
-        
-        $element = $tree->locate_element(1);
-        $this->assertNull($tree->get_neighbour_sortorder($element, 'previous'));
-        $this->assertEqual(7, $tree->get_neighbour_sortorder($element, 'next'));
-        
-        $element = $tree->locate_element(7);
-        $this->assertEqual(1, $tree->get_neighbour_sortorder($element, 'previous'));
-        $this->assertEqual(8, $tree->get_neighbour_sortorder($element, 'next'));
-        
-        $element = $tree->locate_element(8);
-        $this->assertEqual(7, $tree->get_neighbour_sortorder($element, 'previous'));
-        $this->assertNull($tree->get_neighbour_sortorder($element, 'next'));
-    }
-    
     function test_grade_tree_insert_grade_subcategory() {
         $tree = new grade_tree($this->courseid);
         $grade_category = new grade_category($this->grade_categories[3]);
@@ -288,4 +293,5 @@ class grade_tree_test extends gradelib_test {
         $tree = new grade_tree($this->courseid);
         echo $tree->get_edit_tree(); 
     }
+    
 }