MDL-47503 Grades: Completely remove aggregationsubcats

This setting is not compatible with combinations of aggregation methods
and the ways in which it does and does not work are not documented. It
was voted to remove it completely by the gradebook workshop, so I have
completely removed it and added a warning for all affected courses + restored
backups.

admin/settings/grades.php

@@ -112,8 +112,6 @@
         $defaults = array('value'=>0, 'forced'=>false, 'adv'=>true);
         $temp->add(new admin_setting_gradecat_combo('grade_aggregateoutcomes', new lang_string('aggregateoutcomes', 'grades'),
                     new lang_string('aggregateoutcomes_help', 'grades'), $defaults, $options));
-        $temp->add(new admin_setting_gradecat_combo('grade_aggregatesubcats', new lang_string('aggregatesubcats', 'grades'),
-                    new lang_string('aggregatesubcats_help', 'grades'), $defaults, $options));
 
         $options = array(0 => new lang_string('none'));
         for ($i=1; $i<=20; $i++) {

backup/moodle2/backup_stepslib.php

@@ -958,7 +958,7 @@ protected function define_structure() {
         $grade_category   = new backup_nested_element('grade_category', array('id'), array(
                 //'courseid',
                 'parent', 'depth', 'path', 'fullname', 'aggregation', 'keephigh',
-                'droplow', 'aggregateonlygraded', 'aggregateoutcomes', 'aggregatesubcats',
+                'droplow', 'aggregateonlygraded', 'aggregateoutcomes',
                 'timecreated', 'timemodified', 'hidden'));
 
         $letters = new backup_nested_element('grade_letters');

backup/moodle2/restore_stepslib.php

@@ -282,6 +282,11 @@ protected function process_grade_category($data) {
             }
         }
 
+        // Add a warning about a removed setting.
+        if (!empty($data->aggregatesubcats)) {
+            set_config('show_aggregatesubcats_upgrade_' . $data->courseid, 1);
+        }
+
         //need to insert a course category
         if (empty($newitemid)) {
             $newitemid = $DB->insert_record('grade_categories', $data);

grade/edit/tree/category_form.php

@@ -70,13 +70,6 @@ function definition() {
             }
         }
 
-        $mform->addElement('advcheckbox', 'aggregatesubcats', get_string('aggregatesubcats', 'grades'));
-        $mform->addHelpButton('aggregatesubcats', 'aggregatesubcats', 'grades');
-
-        if ((int)$CFG->grade_aggregatesubcats_flag & 2) {
-            $mform->setAdvanced('aggregatesubcats');
-        }
-
         $mform->addElement('text', 'keephigh', get_string('keephigh', 'grades'), 'size="3"');
         $mform->setType('keephigh', PARAM_INT);
         $mform->addHelpButton('keephigh', 'keephigh', 'grades');
@@ -360,9 +353,6 @@ function definition_after_data() {
                 if ($mform->elementExists('aggregateoutcomes')) {
                     $mform->removeElement('aggregateoutcomes');
                 }
-                if ($mform->elementExists('aggregatesubcats')) {
-                    $mform->removeElement('aggregatesubcats');
-                }
             }
 
             // If it is a course category, remove the "required" rule from the "fullname" element

grade/lib.php

@@ -467,6 +467,15 @@ function hide_natural_aggregation_upgrade_notice($courseid) {
     unset_config('show_sumofgrades_upgrade_' . $courseid);
 }
 
+/**
+ * Hide warning about changed grades during upgrade to 2.8.
+ *
+ * @param int $courseid The current course id.
+ */
+function hide_aggregatesubcats_upgrade_notice($courseid) {
+    unset_config('show_aggregatesubcats_upgrade_' . $courseid);
+}
+
 /**
  * Print warning about changed grades during upgrade to 2.8.
  *
@@ -479,11 +488,11 @@ function hide_natural_aggregation_upgrade_notice($courseid) {
 function print_natural_aggregation_upgrade_notice($courseid, $context, $return=false) {
     global $OUTPUT;
     $html = '';
-    $show = get_config('core', 'show_sumofgrades_upgrade_' . $courseid);
 
+    $show = get_config('core', 'show_sumofgrades_upgrade_' . $courseid);
     if ($show) {
         $message = get_string('sumofgradesupgradedgrades', 'grades');
-        $hidemessage = get_string('sumofgradesupgradedgradeshidemessage', 'grades');
+        $hidemessage = get_string('upgradedgradeshidemessage', 'grades');
         $urlparams = array( 'id' => $courseid,
                             'seensumofgradesupgradedgrades' => true,
                             'sesskey' => sesskey());
@@ -493,6 +502,19 @@ function print_natural_aggregation_upgrade_notice($courseid, $context, $return=f
         $html .= $goawaybutton;
     }
 
+    $show = get_config('core', 'show_aggregatesubcats_upgrade_' . $courseid);
+    if ($show) {
+        $message = get_string('aggregatesubcatsupgradedgrades', 'grades');
+        $hidemessage = get_string('upgradedgradeshidemessage', 'grades');
+        $urlparams = array( 'id' => $courseid,
+                            'seenaggregatesubcatsupgradedgrades' => true,
+                            'sesskey' => sesskey());
+        $goawayurl = new moodle_url('/grade/report/grader/index.php', $urlparams);
+        $goawaybutton = $OUTPUT->single_button($goawayurl, $hidemessage, 'get');
+        $html .= $OUTPUT->notification($message, 'notifysuccess');
+        $html .= $goawaybutton;
+    }
+
     if ($return) {
         return $html;
     } else {

grade/report/grader/index.php

@@ -132,6 +132,10 @@
 if (optional_param('seensumofgradesupgradedgrades', false, PARAM_BOOL) && confirm_sesskey()) {
     hide_natural_aggregation_upgrade_notice($courseid);
 }
+// Hide the following warning if the user told it to go away.
+if (optional_param('seenaggregatesubcatsupgradedgrades', false, PARAM_BOOL) && confirm_sesskey()) {
+    hide_aggregatesubcats_upgrade_notice($courseid);
+}
 // This shows a notice about the upgrade to Natural aggregation.
 print_natural_aggregation_upgrade_notice($COURSE->id, $context);
 

lang/en/grades.php

@@ -49,9 +49,6 @@
 $string['aggregateoutcomes'] = 'Include outcomes in aggregation';
 $string['aggregateoutcomes_help'] = 'If enabled, outcomes are included in the aggregation. This may result in an unexpected category total.';
 $string['aggregatesonly'] = 'Aggregates only';
-$string['aggregatesubcats'] = 'Aggregate including subcategories';
-$string['aggregatesubcatsshort'] = 'Include subcategories';
-$string['aggregatesubcats_help'] = 'This setting determines whether grades in subcategories are included in the aggregation.';
 $string['aggregatesum'] = 'Natural';
 $string['aggregateweightedmean'] = 'Weighted mean of grades';
 $string['aggregateweightedmean2'] = 'Simple weighted mean of grades';
@@ -193,7 +190,6 @@
 $string['errorsettinggrade'] = 'Error saving "{$a->itemname}" grade for userid {$a->userid}';
 $string['errorupdatinggradecategoryaggregateonlygraded'] = 'Error updating the "Aggregate only non-empty grades" setting of grade category ID {$a->id}';
 $string['errorupdatinggradecategoryaggregateoutcomes'] = 'Error updating the "Include outcomes in aggregation" setting of grade category ID {$a->id}';
-$string['errorupdatinggradecategoryaggregatesubcats'] = 'Error updating the "Aggregate including subcategories" setting of grade category ID {$a->id}';
 $string['errorupdatinggradecategoryaggregation'] = 'Error updating the aggregation type of grade category ID {$a->id}';
 $string['errorupdatinggradeitemaggregationcoef'] = 'Error updating the aggregation coefficient (weight or extra credit) of grade item ID {$a->id}';
 $string['eventgradedeleted'] = 'Grade deleted';
@@ -680,7 +676,8 @@
 $string['submissions'] = 'Submissions';
 $string['submittedon'] = 'Submitted: {$a}';
 $string['sumofgradesupgradedgrades'] = 'Note: The aggregation method "Sum of grades" has been changed to "Natural" as part of a site upgrade. Since "Sum of grades" was previously used in this course, it is recommended that you review this change in the gradebook.';
-$string['sumofgradesupgradedgradeshidemessage'] = 'OK';
+$string['aggregatesubcatsupgradedgrades'] = 'Note: The aggregation setting "Aggregate including subcategories" has been removed as part of a site upgrade. Since "Aggregate including subcategories" was previously used in this course, it is recommended that you review this change in the gradebook.';
+$string['upgradedgradeshidemessage'] = 'OK';
 $string['switchtofullview'] = 'Switch to full view';
 $string['switchtosimpleview'] = 'Switch to simple view';
 $string['tabs'] = 'Tabs';

lib/db/install.xml

@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8" ?>
-<XMLDB PATH="lib/db" VERSION="20141007" COMMENT="XMLDB file for core Moodle tables"
+<XMLDB PATH="lib/db" VERSION="20141017" COMMENT="XMLDB file for core Moodle tables"
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xsi:noNamespaceSchemaLocation="../../lib/xmldb/xmldb.xsd"
 >
@@ -1684,7 +1684,6 @@
         <FIELD NAME="droplow" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="Drop the X lowest items"/>
         <FIELD NAME="aggregateonlygraded" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="aggregate only graded activities"/>
         <FIELD NAME="aggregateoutcomes" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="Aggregate outcomes"/>
-        <FIELD NAME="aggregatesubcats" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="ignore subcategories in aggregation"/>
         <FIELD NAME="timecreated" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false"/>
         <FIELD NAME="timemodified" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false"/>
         <FIELD NAME="hidden" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" SEQUENCE="false"/>
@@ -1825,7 +1824,7 @@
         <FIELD NAME="droplow" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="Drop the X lowest items"/>
         <FIELD NAME="aggregateonlygraded" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="aggregate only graded items"/>
         <FIELD NAME="aggregateoutcomes" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="Aggregate outcomes"/>
-        <FIELD NAME="aggregatesubcats" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="ignore subcategories in aggregation"/>
+        <FIELD NAME="aggregatesubcats" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="This setting was removed from grade_categories. It is kept here only to preserve history."/>
         <FIELD NAME="hidden" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" SEQUENCE="false"/>
       </FIELDS>
       <KEYS>

lib/db/upgrade.php

@@ -4018,5 +4018,34 @@ function xmldb_main_upgrade($oldversion) {
         upgrade_main_savepoint(true, 2014101001.00);
     }
 
+    if ($oldversion < 2014102000.00) {
+
+        // Define field aggregatesubcats to be dropped from grade_categories.
+        $table = new xmldb_table('grade_categories');
+        $field = new xmldb_field('aggregatesubcats');
+
+        // Conditionally launch drop field aggregatesubcats.
+        if ($dbman->field_exists($table, $field)) {
+
+            $sql = 'SELECT DISTINCT courseid
+                      FROM {grade_categories}
+                     WHERE aggregatesubcats = ?';
+            $courses = $DB->get_records_sql($sql, array(1));
+
+            foreach ($courses as $course) {
+                set_config('show_aggregatesubcats_upgrade_' . $course->courseid, 1);
+                // Set each of the grade items to needing an update so that when the user visits the grade reports the
+                // figures will be updated.
+                $DB->set_field('grade_items', 'needsupdate', 1, array('courseid' => $course->courseid));
+            }
+
+
+            $dbman->drop_field($table, $field);
+        }
+
+        // Main savepoint reached.
+        upgrade_main_savepoint(true, 2014102000.00);
+    }
+
     return true;
 }

lib/grade/grade_category.php

@@ -47,7 +47,7 @@ class grade_category extends grade_object {
      */
     public $required_fields = array('id', 'courseid', 'parent', 'depth', 'path', 'fullname', 'aggregation',
                                  'keephigh', 'droplow', 'aggregateonlygraded', 'aggregateoutcomes',
-                                 'aggregatesubcats', 'timecreated', 'timemodified', 'hidden');
+                                 'timecreated', 'timemodified', 'hidden');
 
     /**
      * The course this category belongs to.
@@ -116,12 +116,6 @@ class grade_category extends grade_object {
      */
     public $aggregateoutcomes = 0;
 
-    /**
-     * Ignore subcategories when aggregating
-     * @var int $aggregatesubcats
-     */
-    public $aggregatesubcats = 0;
-
     /**
      * Array of grade_items or grade_categories nested exactly 1 level below this category
      * @var array $children
@@ -152,7 +146,7 @@ class grade_category extends grade_object {
      * List of options which can be "forced" from site settings.
      * @var array $forceable
      */
-    public $forceable = array('aggregation', 'keephigh', 'droplow', 'aggregateonlygraded', 'aggregateoutcomes', 'aggregatesubcats');
+    public $forceable = array('aggregation', 'keephigh', 'droplow', 'aggregateonlygraded', 'aggregateoutcomes');
 
     /**
      * String representing the aggregation coefficient. Variable is used as cache.
@@ -410,9 +404,8 @@ public function qualifies_for_regrading() {
         $droplowdiff     = $db_item->droplow             != $this->droplow;
         $aggonlygrddiff  = $db_item->aggregateonlygraded != $this->aggregateonlygraded;
         $aggoutcomesdiff = $db_item->aggregateoutcomes   != $this->aggregateoutcomes;
-        $aggsubcatsdiff  = $db_item->aggregatesubcats    != $this->aggregatesubcats;
 
-        return ($aggregationdiff || $keephighdiff || $droplowdiff || $aggonlygrddiff || $aggoutcomesdiff || $aggsubcatsdiff);
+        return ($aggregationdiff || $keephighdiff || $droplowdiff || $aggonlygrddiff || $aggoutcomesdiff);
     }
 
     /**
@@ -1613,8 +1606,6 @@ public static function aggregation_uses_aggregationcoef($aggregation) {
     /**
      * Recursive function to find which weight/extra credit field to use in the grade item form.
      *
-     * Inherits from a parent category if that category has aggregatesubcats set to true.
-     *
      * @param string $first Whether or not this is the first item in the recursion
      * @return string
      */
@@ -1625,8 +1616,8 @@ public function get_coefstring($first=true) {
 
         $overriding_coefstring = null;
 
-        // Stop recursing upwards if this category aggregates subcats or has no parent
-        if (!$first && !$this->aggregatesubcats) {
+        // Stop recursing upwards if this category has no parent
+        if (!$first) {
 
             if ($parent_category = $this->load_parent_category()) {
                 return $parent_category->get_coefstring(false);
@@ -1637,11 +1628,8 @@ public function get_coefstring($first=true) {
 
         } else if ($first) {
 
-            if (!$this->aggregatesubcats) {
-
-                if ($parent_category = $this->load_parent_category()) {
-                    $overriding_coefstring = $parent_category->get_coefstring(false);
-                }
+            if ($parent_category = $this->load_parent_category()) {
+                $overriding_coefstring = $parent_category->get_coefstring(false);
             }
         }
 
@@ -2010,9 +1998,6 @@ public function get_description() {
         if (!$this->aggregateonlygraded) {
             $allhelp[] = get_string('aggregatenotonlygraded', 'grades');
         }
-        if ($this->aggregatesubcats) {
-            $allhelp[] = get_string('aggregatesubcatsshort', 'grades');
-        }
         if ($allhelp) {
             return implode('. ', $allhelp) . '.';
         }

lib/grade/grade_item.php

@@ -1445,44 +1445,31 @@ public function depends_on($reset_cache=false) {
                 $params[] = GRADE_TYPE_SCALE;
             }
 
-            if ($grade_category->aggregatesubcats) {
-                // return all children excluding category items
-                $params[] = $this->courseid;
-                $params[] = '%/' . $grade_category->id . '/%';
-                $sql = "SELECT gi.id
-                          FROM {grade_items} gi
-                          JOIN {grade_categories} gc ON gi.categoryid = gc.id
-                         WHERE $gtypes
-                               $outcomes_sql
-                               AND gi.courseid = ?
-                               AND gc.path LIKE ?";
+            $params[] = $grade_category->id;
+            $params[] = $this->courseid;
+            $params[] = $grade_category->id;
+            $params[] = $this->courseid;
+            if (empty($CFG->grade_includescalesinaggregation)) {
+                $params[] = GRADE_TYPE_VALUE;
             } else {
-                $params[] = $grade_category->id;
-                $params[] = $this->courseid;
-                $params[] = $grade_category->id;
-                $params[] = $this->courseid;
-                if (empty($CFG->grade_includescalesinaggregation)) {
-                    $params[] = GRADE_TYPE_VALUE;
-                } else {
-                    $params[] = GRADE_TYPE_VALUE;
-                    $params[] = GRADE_TYPE_SCALE;
-                }
-                $sql = "SELECT gi.id
-                          FROM {grade_items} gi
-                         WHERE $gtypes
-                               AND gi.categoryid = ?
-                               AND gi.courseid = ?
-                               $outcomes_sql
-                        UNION
-
-                        SELECT gi.id
-                          FROM {grade_items} gi, {grade_categories} gc
-                         WHERE (gi.itemtype = 'category' OR gi.itemtype = 'course') AND gi.iteminstance=gc.id
-                               AND gc.parent = ?
-                               AND gi.courseid = ?
-                               AND $gtypes
-                               $outcomes_sql";
+                $params[] = GRADE_TYPE_VALUE;
+                $params[] = GRADE_TYPE_SCALE;
             }
+            $sql = "SELECT gi.id
+                      FROM {grade_items} gi
+                     WHERE $gtypes
+                           AND gi.categoryid = ?
+                           AND gi.courseid = ?
+                           $outcomes_sql
+                    UNION
+
+                    SELECT gi.id
+                      FROM {grade_items} gi, {grade_categories} gc
+                     WHERE (gi.itemtype = 'category' OR gi.itemtype = 'course') AND gi.iteminstance=gc.id
+                           AND gc.parent = ?
+                           AND gi.courseid = ?
+                           AND $gtypes
+                           $outcomes_sql";
 
             if ($children = $DB->get_records_sql($sql, $params)) {
                 $this->dependson_cache = array_keys($children);

lib/grade/tests/fixtures/lib.php

@@ -60,7 +60,6 @@ protected function setUp() {
         $CFG->grade_aggregation = -1;
         $CFG->grade_aggregateonlygraded = -1;
         $CFG->grade_aggregateoutcomes = -1;
-        $CFG->grade_aggregatesubcats = -1;
 
         $this->course = $this->getDataGenerator()->create_course();
         $this->courseid = $this->course->id;
@@ -169,7 +168,7 @@ private function load_scales() {
                                     |
                            +--------+-------------+-----------------------+
                            |                      |                       |
-             unittestcategory1               level1category       aggregatesubcatscategory
+             unittestcategory1               level1category       unittestcategory7
                   |                                                          |
          +--------+-------------+                               +------------+---------------+
          |                      |                               |                            |
@@ -256,11 +255,10 @@ private function load_grade_categories() {
 
         $grade_category = new stdClass();
 
-        $grade_category->fullname    = 'aggregatesubcatscategory';
+        $grade_category->fullname    = 'unittestcategory7';
         $grade_category->courseid    = $this->course->id;
         $grade_category->aggregation = GRADE_AGGREGATE_MEAN;
         $grade_category->aggregateonlygraded = 1;
-        $grade_category->aggregatesubcats = 1;
         $grade_category->keephigh    = 0;
         $grade_category->droplow     = 0;
         $grade_category->parent      = $course_category->id;
@@ -578,7 +576,7 @@ protected function load_grade_items() {
 
         $grade_item->courseid = $this->course->id;
         $grade_item->iteminstance = $this->grade_categories[4]->id;
-        $grade_item->itemname = 'unittestgradeitemaggregatesubcats';
+        $grade_item->itemname = 'unittestgradeitemcategory7';
         $grade_item->itemtype = 'category';
         $grade_item->gradetype = GRADE_TYPE_VALUE;
         $grade_item->needsupdate = true;