MDL-10875 Corrected bug in averaging plus a couple of typos

diff --git a/grade/lib.php b/grade/lib.php
index bd3faafb7bad3fef3cb5776b60b93f24f2d023f7..627ca0772f3ba8964eee6770ee56dcaa8a100904 100644 (file)
--- a/grade/lib.php
+++ b/grade/lib.php
@@ -442,6 +442,12 @@ function grade_to_percentage($gradeval, $grademin, $grademax) {
     if ($grademin >= $grademax) {
         debugging("The minimum grade ($grademin) was higher than or equal to the maximum grade ($grademax)!!! Cannot proceed with computation.");
     }
+
+    // If given gradevalue is lower than grademin, adjust it to grademin
+    if ($gradeval < $grademin || empty($gradeval)) {
+        $gradeval = $grademin;
+    }
+
     $offset_value = $gradeval - $grademin;
     $offset_max = $grademax - $grademin;
     $factor = 100 / $offset_max;

diff --git a/grade/report/grader/lib.php b/grade/report/grader/lib.php
index df24783972ba6880ae2749be52ec6407abc47079..c1d2b69e03f41bc6dd4d06cbc751a672dcdc2787 100644 (file)
--- a/grade/report/grader/lib.php
+++ b/grade/report/grader/lib.php
@@ -708,7 +708,7 @@ class grade_report_grader extends grade_report {
 
                     if ($gradedisplaytype == GRADE_REPORT_GRADE_DISPLAY_TYPE_PERCENTAGE) {
                         if (!is_null($gradeval)) {
-                            $gradeval = grade_grade::standardise_score($gradeval, $grademin, $grademax, 0, 100);
+                            $gradeval = grade_to_percentage($gradeval, $grademin, $grademax);
                         }
                         $percentsign = '%';
                     }
@@ -791,16 +791,12 @@ class grade_report_grader extends grade_report {
             $groupwheresql = null;
         }
 
-        if ($meanselection == GRADE_AGGREGATE_MEAN_GRADED) {
-            $totalcount = 0;
-        } else {
-            $totalcount = $this->get_numusers(false);
-        }
+        $totalcount = $this->get_numusers(false);
 
         if ($showaverages) {
 
             // the first join on user is needed for groupsql
-            $SQL = "SELECT g.itemid, SUM(g.finalgrade) as sum, COUNT(DISTINCT(u.id)) as count
+            $SQL = "SELECT g.itemid, SUM(g.finalgrade) as sum
                 FROM {$CFG->prefix}grade_items gi LEFT JOIN
                      {$CFG->prefix}grade_grades g ON gi.id = g.itemid LEFT JOIN
                      {$CFG->prefix}user u ON g.userid = u.id
@@ -816,15 +812,9 @@ class grade_report_grader extends grade_report {
                      )
                 GROUP BY g.itemid";
             $sum_array = array();
-            $count_array = array();
             if ($sums = get_records_sql($SQL)) {
                 foreach ($sums as $itemid => $csum) {
                     $sum_array[$itemid] = $csum->sum;
-                    if ($totalcount) {
-                        $count_array[$itemid] = $totalcount;
-                    } else {
-                        $count_array[$itemid] = $csum->count;
-                    }
                 }
             }
 
@@ -832,6 +822,37 @@ class grade_report_grader extends grade_report {
 
             $columncount=1;
             foreach ($this->items as $item) {
+                if (empty($sum_array[$item->id])) {
+                    $sum_array[$item->id] = 0;
+                }
+
+                // MDL-10875 Empty grades must be evaluated as grademin, NOT always 0
+                // This query returns a count of ungraded grades (NULL finalgrade OR no matching record in grade_grades table)
+                $SQL = "SELECT COUNT(*) AS count FROM {$CFG->prefix}user u
+                         WHERE u.id NOT IN
+                           (SELECT userid FROM {$CFG->prefix}grade_grades
+                             WHERE itemid = $item->id
+                               AND finalgrade IS NOT NULL
+                           )
+                           AND u.id IN (
+                             SELECT DISTINCT(u.id)
+                              FROM {$CFG->prefix}user u LEFT JOIN
+                                   {$CFG->prefix}role_assignments ra ON u.id = ra.userid
+                             WHERE ra.roleid in ($this->gradebookroles)
+                               AND ra.contextid ".get_related_contexts_string($this->context)."
+                           )";
+
+                $ungraded_count = get_field_sql($SQL);
+
+                if ($meanselection == GRADE_AGGREGATE_MEAN_GRADED) {
+                    $mean_count = $totalcount - $ungraded_count;
+                } else { // Bump up the sum by the number of ungraded items * grademin
+                    if (isset($sum_array[$item->id])) {
+                        $sum_array[$item->id] += $ungraded_count * $item->grademin;
+                    }
+                    $mean_count = $totalcount;
+                }
+
                 $decimalpoints = $this->get_pref('decimalpoints', $item->id);
                 // Determine which display type to use for this average
                 $gradedisplaytype = $this->get_pref('gradedisplaytype', $item->id);
@@ -847,7 +868,7 @@ class grade_report_grader extends grade_report {
                     $decimalpoints = $averagesdecimalpoints;
                 }
 
-                if (empty($count_array[$item->id]) || !isset($sum_array[$item->id])) {
+                if (!isset($sum_array[$item->id]) || $mean_count == 0) {
                     $avghtml .= '<td class="cell c' . $columncount++.'">-</td>';
                 } else {
                     $sum = $sum_array[$item->id];
@@ -855,24 +876,23 @@ class grade_report_grader extends grade_report {
                     if ($item->scaleid) {
                         if ($grouponly) {
                             $finalsum = $sum_array[$item->id];
-                            $finalavg = $finalsum/$count_array[$item->id];
+                            $finalavg = $finalsum/$mean_count;
                         } else {
-                            $finalavg = $sum/$count_array[$item->id];
+                            $finalavg = $sum/$mean_count;
                         }
-
                         $scaleval = round($finalavg);
                         $scale_object = new grade_scale(array('id' => $item->scaleid), false);
                         $gradehtml = $scale_object->get_nearest_item($scaleval);
                         $rawvalue = $scaleval;
                     } else {
-                        $gradeval = format_float($sum/$count_array[$item->id], $decimalpoints);
+                        $gradeval = format_float($sum/$mean_count, $decimalpoints);
                         $gradehtml = $gradeval;
                         $rawvalue = $gradeval;
                     }
 
                     if ($displaytype == GRADE_REPORT_GRADE_DISPLAY_TYPE_PERCENTAGE) {
-                        $gradeval = grade_grade::standardise_score($rawvalue, $item->grademin, $item->grademax, 0, 100);
-                        $gradehtml = number_format($gradeval, $decimalpoints) . '%';
+                        $gradeval = grade_to_percentage($rawvalue, $item->grademin, $item->grademax);
+                        $gradehtml = number_format(format_float($gradeval, $decimalpoints), $decimalpoints) . '%';
                     } elseif ($displaytype == GRADE_REPORT_GRADE_DISPLAY_TYPE_LETTER) {
                         $letters = grade_report::get_grade_letters();
                         $gradehtml = grade_grade::get_letter($letters, $gradeval, $item->grademin, $item->grademax);

diff --git a/grade/report/grader/preferences.php b/grade/report/grader/preferences.php
index b190c7dd2c3906496af85ddf002513cba003ad7f..f188334a52fb1311171b27f908095f18bd542da8 100644 (file)
--- a/grade/report/grader/preferences.php
+++ b/grade/report/grader/preferences.php
@@ -49,7 +49,7 @@ if ($data = data_submitted()) {
                 set_user_preference('calendar_persistflt', intval($value));
                 break;
             default:
-                if ($value == GRADE_REPORT_PREFERENCE_DEFAULT || empty($value)) {
+                if ($value == GRADE_REPORT_PREFERENCE_DEFAULT || strlen($value) == 0) {
                     unset_user_preference($preference);
                 } else {
                     set_user_preference($preference, $value);

diff --git a/lib/dmllib.php b/lib/dmllib.php
index b6ba720dc79e5e5a2c3453ec93773dbabbcbb3b8..399d8f8e8dd9dd379e54cfb5e40c8e5d37ccc4de 100644 (file)
--- a/lib/dmllib.php
+++ b/lib/dmllib.php
@@ -413,7 +413,7 @@ function get_record($table, $field1, $value1, $field2='', $value2='', $field3=''
 
     $record = get_record_sql('SELECT '.$fields.' FROM '. $CFG->prefix . $table .' '. $select);
 
-    // If we're caching records, store this one 
+    // If we're caching records, store this one
     // (supposing we got something - we don't cache failures)
     if ($docache) {
         if (isset($record)) {
@@ -436,7 +436,7 @@ function get_record($table, $field1, $value1, $field2='', $value2='', $field3=''
  * @uses $CFG
  * @uses $db
  * @param string $sql The SQL string you wish to be executed, should normally only return one record.
- * @param bool $expectmultiple If the SQL cannot be written to conviniently return just one record,
+ * @param bool $expectmultiple If the SQL cannot be written to conveniently return just one record,
  *      set this to true to hide the debug message.
  * @param bool $nolimit sometimes appending ' LIMIT 1' to the SQL causes an error. Set this to true
  *      to stop your SQL being modified. This argument should probably be deprecated.
@@ -1247,7 +1247,7 @@ function set_field_select($table, $newfield, $newvalue, $select, $localcall = fa
         $update = "$newfield = '$newvalue'";
     }
 
-/// Arriving here, standard update 
+/// Arriving here, standard update
     return $db->Execute('UPDATE '. $CFG->prefix . $table .' SET '.$update.' '.$select);
 }
 
@@ -1585,7 +1585,7 @@ function update_record($table, $dataobject) {
     $count = 0;
     $update = '';
 
-/// Only if we have fields to be updated (this will prevent both wrong updates + 
+/// Only if we have fields to be updated (this will prevent both wrong updates +
 /// updates of only LOBs in Oracle
     if ($numddd) {
         foreach ($ddd as $key => $value) {
@@ -1950,7 +1950,7 @@ function sql_primary_role_subselect() {
             FROM '.$CFG->prefix.'role_assignments ra
             INNER JOIN '.$CFG->prefix.'role r ON ra.roleid = r.id
             INNER JOIN '.$CFG->prefix.'context c ON ra.contextid = c.id
-            WHERE NOT EXISTS ( 
+            WHERE NOT EXISTS (
                               SELECT 1
                               FROM '.$CFG->prefix.'role_assignments i_ra
                               INNER JOIN '.$CFG->prefix.'role i_r ON i_ra.roleid = i_r.id

diff --git a/lib/moodlelib.php b/lib/moodlelib.php
index ec8a62b9204c8290c75910e8bfb7a5bbb4ccea53..a5c1b06580a51bccd26a6f7d18c3e3d11d1e6e7f 100644 (file)
--- a/lib/moodlelib.php
+++ b/lib/moodlelib.php
@@ -5888,10 +5888,10 @@ function format_float($float, $decimalpoints=1) {
 }
 
 /**
- * Convers locale specific floating point/comma number back to standard PHP float value
+ * Converts locale specific floating point/comma number back to standard PHP float value
  * Do NOT try to do any math operations before this conversion on any user submitted floats!
  *
- * @param  string $locale_float locale aware flaot represenation
+ * @param  string $locale_float locale aware float representation
  */
 function unformat_float($locale_float) {
     $locale_float = trim($locale_float);
@@ -5907,7 +5907,7 @@ function unformat_float($locale_float) {
 
 /**
  * Given a simple array, this shuffles it up just like shuffle()
- * Unlike PHP's shuffle() ihis function works on any machine.
+ * Unlike PHP's shuffle() this function works on any machine.
  *
  * @param array $array The array to be rearranged
  * @return array