From: skodak <skodak>
Date: Wed, 29 Apr 2009 13:42:03 +0000 (+0000)
Subject: MDL-19008 extra credis should not be dropped when drop low or keep high specified... 
X-Git-Url: http://git.mjollnir.org/gw?a=commitdiff_plain;h=a9e38ac8a7365dd0086c0612162ffb827a826c6b;p=moodle.git

MDL-19008 extra credis should not be dropped when drop low or keep high specified (including unit tests)
---

diff --git a/lib/grade/grade_category.php b/lib/grade/grade_category.php
index 5fcb5b77d8..e02035f4a5 100644
--- a/lib/grade/grade_category.php
+++ b/lib/grade/grade_category.php
@@ -558,7 +558,7 @@ class grade_category extends grade_object {
         }
 
         // limit and sort
-        $this->apply_limit_rules($grade_values);
+        $this->apply_limit_rules($grade_values, $items);
         asort($grade_values, SORT_NUMERIC);
 
         // let's see we have still enough grades to do any statistics
@@ -734,7 +734,7 @@ class grade_category extends grade_object {
             }
         }
         // apply droplow and keephigh
-        $this->apply_limit_rules($maxes);
+        $this->apply_limit_rules($maxes, $items);
         $max = array_sum($maxes);
 
         // update db if anything changed
@@ -780,7 +780,7 @@ class grade_category extends grade_object {
             }
         }
 
-        $this->apply_limit_rules($grade_values);
+        $this->apply_limit_rules($grade_values, $items);
 
         $sum = array_sum($grade_values);
         $grade->finalgrade = $this->grade_item->bounded_grade($sum);
@@ -796,27 +796,45 @@ class grade_category extends grade_object {
     /**
      * Given an array of grade values (numerical indices), applies droplow or keephigh
      * rules to limit the final array.
-     * @param array $grade_values
+     * @param array $grade_values itemid=>$grade_value float
+     * @param array $items grade titem objects
      * @return array Limited grades.
      */
-    public function apply_limit_rules(&$grade_values) {
-        arsort($grade_values, SORT_NUMERIC);
+    public function apply_limit_rules(&$grade_values, $items) {
+        $extraused = $this->is_extracredit_used();
+
         if (!empty($this->droplow)) {
-            for ($i = 0; $i < $this->droplow; $i++) {
-                if (empty($grade_values)) {
-                    // nothing to remove
-                    return;
+            asort($grade_values, SORT_NUMERIC);
+            $dropped = 0;
+            foreach ($grade_values as $itemid=>$value) {
+                if ($dropped < $this->droplow) {
+                    if ($extraused and $items[$itemid]->aggregationcoef > 0) {
+                        // no drop low for extra credits
+                    } else {
+                        unset($grade_values[$itemid]);
+                        $dropped++;
+                    }
+                } else {
+                    // we have dropped enough
+                    break;
                 }
-                array_pop($grade_values);
             }
-        } elseif (!empty($this->keephigh)) {
-            while (count($grade_values) > $this->keephigh) {
-                array_pop($grade_values);
+
+        } else if (!empty($this->keephigh)) {
+            arsort($grade_values, SORT_NUMERIC);
+            $kept = 0;
+            foreach ($grade_values as $itemid=>$value) {
+                if ($extraused and $items[$itemid]->aggregationcoef > 0) {
+                    // we keep all extra credits
+                } else if ($kept < $this->keephigh) {
+                    $kept++;
+                } else {
+                    unset($grade_values[$itemid]);
+                }
             }
         }
     }
 
-
     /**
      * Returns true if category uses special aggregation coeficient
      * @return boolean true if coeficient used
diff --git a/lib/grade/grade_grade.php b/lib/grade/grade_grade.php
index 102058c357..242380851c 100644
--- a/lib/grade/grade_grade.php
+++ b/lib/grade/grade_grade.php
@@ -651,7 +651,7 @@ class grade_grade extends grade_object {
                             }
 
                             // limit and sort
-                            $grade_category->apply_limit_rules($values);
+                            $grade_category->apply_limit_rules($values, $grade_items);
                             asort($values, SORT_NUMERIC);
 
                             // let's see we have still enough grades to do any statistics
diff --git a/lib/grade/simpletest/testgradecategory.php b/lib/grade/simpletest/testgradecategory.php
index ee352e1da6..0b7f15a769 100755
--- a/lib/grade/simpletest/testgradecategory.php
+++ b/lib/grade/simpletest/testgradecategory.php
@@ -248,23 +248,61 @@ class grade_category_test extends grade_test {
     }
 
     function test_grade_category_apply_limit_rules() {
-        $category = new grade_category();
-        $grades = array(5.374, 9.4743, 2.5474, 7.3754);
+        $items[$this->grade_items[0]->id] = new grade_item($this->grade_items[0], false);
+        $items[$this->grade_items[1]->id] = new grade_item($this->grade_items[1], false);
+        $items[$this->grade_items[2]->id] = new grade_item($this->grade_items[2], false);
+        $items[$this->grade_items[4]->id] = new grade_item($this->grade_items[4], false);
 
+        $category = new grade_category();
         $category->droplow = 2;
-        $category->apply_limit_rules($grades);
-        sort($grades, SORT_NUMERIC);
-        $this->assertEqual(array(7.3754, 9.4743), $grades);
+        $grades = array($this->grade_items[0]->id=>5.374,
+                        $this->grade_items[1]->id=>9.4743,
+                        $this->grade_items[2]->id=>2.5474,
+                        $this->grade_items[4]->id=>7.3754);
+        $category->apply_limit_rules($grades, $items);
+        $this->assertEqual(count($grades), 2);
+        $this->assertEqual($grades[$this->grade_items[1]->id], 9.4743);
+        $this->assertEqual($grades[$this->grade_items[4]->id], 7.3754);
 
         $category = new grade_category();
-        $grades = array(5.374, 9.4743, 2.5474, 7.3754);
-
         $category->keephigh = 1;
         $category->droplow = 0;
-        $category->apply_limit_rules($grades);
+        $grades = array($this->grade_items[0]->id=>5.374,
+                        $this->grade_items[1]->id=>9.4743,
+                        $this->grade_items[2]->id=>2.5474,
+                        $this->grade_items[4]->id=>7.3754);
+        $category->apply_limit_rules($grades, $items);
         $this->assertEqual(count($grades), 1);
         $grade = reset($grades);
         $this->assertEqual(9.4743, $grade);
+
+        $category = new grade_category();
+        $category->droplow     = 2;
+        $category->aggregation = GRADE_AGGREGATE_SUM;
+        $items[$this->grade_items[2]->id]->aggregationcoef = 1;
+        $grades = array($this->grade_items[0]->id=>5.374,
+                        $this->grade_items[1]->id=>9.4743,
+                        $this->grade_items[2]->id=>2.5474,
+                        $this->grade_items[4]->id=>7.3754);
+
+        $category->apply_limit_rules($grades, $items);
+        $this->assertEqual(count($grades), 2);
+        $this->assertEqual($grades[$this->grade_items[1]->id], 9.4743);
+        $this->assertEqual($grades[$this->grade_items[2]->id], 2.5474);
+
+        $category = new grade_category();
+        $category->keephigh = 1;
+        $category->droplow = 0;
+        $category->aggregation = GRADE_AGGREGATE_SUM;
+        $items[$this->grade_items[2]->id]->aggregationcoef = 1;
+        $grades = array($this->grade_items[0]->id=>5.374,
+                        $this->grade_items[1]->id=>9.4743,
+                        $this->grade_items[2]->id=>2.5474,
+                        $this->grade_items[4]->id=>7.3754);
+        $category->apply_limit_rules($grades, $items);
+        $this->assertEqual(count($grades), 2);
+        $this->assertEqual($grades[$this->grade_items[1]->id], 9.4743);
+        $this->assertEqual($grades[$this->grade_items[2]->id], 2.5474);
     }
 
     /**