MDL-9506 Extracted the code that applies droplow and keephigh rules to aggregated values, into its own little method. Unit testing this method revealed a coding error which I have rectified.

lib/grade/grade_category.php

@@ -389,6 +389,27 @@ class grade_category extends grade_object {
         return true;
     }
 
+    /**
+     * Given an array of grade values (numerical indices), applies droplow or keephigh
+     * rules to limit the final array.
+     * @param array $grades
+     * @return array Limited grades.
+     */
+    function apply_limit_rules($grades) {
+        rsort($grades, SORT_NUMERIC);
+        if (!empty($this->droplow)) {
+            for ($i = 0; $i < $this->droplow; $i++) {
+                array_pop($grades);
+            }
+        } elseif (!empty($this->keephigh)) { 
+            while (count($grades) > $this->keephigh) {
+                array_pop($grades);                    
+            }
+        }
+        sort($grades, SORT_NUMERIC);
+        return $grades;
+    }
+
     /**
      * Given an array of arrays of values, standardised from 0 to 1 and indexed by userid, 
      * uses this category's aggregation method to 
@@ -415,21 +436,8 @@ class grade_category extends grade_object {
 
         foreach ($pooled_grades as $userid => $grades) {
             $aggregated_value = null;
-            // Sort grades from lowest to largest
-            sort($grades, SORT_NUMERIC);
             
-            // Apply droplow or keephigh rule
-            if (!empty($this->droplow)) {
-                $reversed_grades = array_reverse($grades);
-                for ($i = 0; $i < $this->droplow; $i++) {
-                    array_pop($reversed_grades);
-                }
-                $grades = array_reverse($reversed_grades);
-            } elseif (!empty($this->keephigh)) { 
-                for ($i = 0; $i < $this->keephigh; $i++) {
-                    array_pop($grades);
-                }
-            }
+            $grades = $this->apply_limit_rules($grades);
             
             if (count($grades) > 1) {
 

lib/simpletest/grade/simpletest/testgradecategory.php

@@ -219,6 +219,9 @@ class grade_category_test extends gradelib_test {
     } 
 
     function test_grade_category_set_as_parent() {
+        global $CFG;
+        $debuglevel = $CFG->debug;
+
         // There are 4 constraints which, if violated, should return false and trigger a debugging message. Test each of them
         $grade_category = new grade_category();
         $grade_category->fullname    = 'new topcategory';
@@ -231,22 +234,30 @@ class grade_category_test extends gradelib_test {
         $child2 = new grade_category();
         $child2->grade_item = new grade_item();
         $child2->grade_item->sortorder = 2;
+        $CFG->debug = 2;
         $this->assertFalse($grade_category->set_as_parent(array($child1, $child2)));
+        $CFG->debug = $debuglevel;
 
         // 2. Non-consecutive children
         $child1 = new grade_item();
         $child2 = new grade_item();
         $child1->sortorder = 1;
         $child2->sortorder = 3;
+        $CFG->debug = 2;
         $this->assertFalse($grade_category->set_as_parent(array($child1, $child2)));
+        $CFG->debug = $debuglevel;
         
         // 3. Child is a top category
         $child1 = new grade_category($this->grade_categories[0]);
+        $CFG->debug = 2;
         $this->assertFalse($grade_category->set_as_parent(array($child1)));
+        $CFG->debug = $debuglevel;
 
         // 4. Child already has a top category
         $child1 = new grade_item($this->grade_items[0]);
+        $CFG->debug = 2;
         $this->assertFalse($grade_category->set_as_parent(array($child1)));
+        $CFG->debug = $debuglevel;
 
         // Now test setting parent correctly
         $child1 = new grade_item();
@@ -259,5 +270,19 @@ class grade_category_test extends gradelib_test {
         $child2->insert();
         $this->assertTrue($grade_category->set_as_parent(array($child1, $child2)));
     }
+
+    function test_grade_category_apply_limit_rules() {
+        $category = new grade_category();
+        $grades = array(5.374, 9.4743, 2.5474, 7.3754);
+        
+        $category->droplow = 2;
+        $result = $category->apply_limit_rules(fullclone($grades));
+        $this->assertEqual(array(7.3754, 9.4743), $result);
+        
+        $category->keephigh = 1;
+        $category->droplow = 0;
+        $result = $category->apply_limit_rules(fullclone($grades));
+        $this->assertEqual(array(9.4743), $result); 
+    }
 } 
 ?>

lib/simpletest/grade/simpletest/testgradetree.php

@@ -218,5 +218,15 @@ class grade_tree_test extends gradelib_test {
 
     function test_grade_tree_build_tree_filled() {
         $tree = new grade_tree($this->courseid);
+        echo $tree->display_grades() . "<br />";
+
+        $element = $tree->tree_array[7];
+        $tree->remove_element(7);
+        $tree->renumber();
+        echo $tree->display_grades() . "<br />";
+
+        $tree->insert_element($element, 4);
+        $tree->renumber();
+        echo $tree->display_grades() . "<br />";
     }
 }