MDL-68284 gradebook: Prevent exposing hidden quiz grade item
authorSagar Ghimire <gmrsagar@gmail.com>
Mon, 24 Aug 2020 01:40:49 +0000 (11:40 +1000)
committerJohn Yao <johnyao@catalyst-au.net>
Fri, 16 Oct 2020 04:06:34 +0000 (15:06 +1100)
grade/edit/tree/item.php
grade/edit/tree/item_form.php
lib/db/upgrade.php
version.php

index 2ece98f..64ddf7f 100644 (file)
@@ -133,8 +133,11 @@ if ($mform->is_cancelled()) {
         $data->grademin = 0;
     }
 
         $data->grademin = 0;
     }
 
-    $hidden      = empty($data->hidden) ? 0: $data->hidden;
-    $hiddenuntil = empty($data->hiddenuntil) ? 0: $data->hiddenuntil;
+    $hide = empty($data->hiddenuntil) ? 0 : $data->hiddenuntil;
+    if (!$hide) {
+        $hide = empty($data->hidden) ? 0 : $data->hidden;
+    }
+
     unset($data->hidden);
     unset($data->hiddenuntil);
 
     unset($data->hidden);
     unset($data->hiddenuntil);
 
@@ -155,45 +158,43 @@ if ($mform->is_cancelled()) {
         $data->aggregationcoef2 = $defaults['aggregationcoef2'];
     }
 
         $data->aggregationcoef2 = $defaults['aggregationcoef2'];
     }
 
-    $grade_item = new grade_item(array('id'=>$id, 'courseid'=>$courseid));
-    $oldmin = $grade_item->grademin;
-    $oldmax = $grade_item->grademax;
-    grade_item::set_properties($grade_item, $data);
-    $grade_item->outcomeid = null;
+    $gradeitem = new grade_item(array('id' => $id, 'courseid' => $courseid));
+    $oldmin = $gradeitem->grademin;
+    $oldmax = $gradeitem->grademax;
+    grade_item::set_properties($gradeitem, $data);
+    $gradeitem->outcomeid = null;
 
     // Handle null decimals value
     if (!property_exists($data, 'decimals') or $data->decimals < 0) {
 
     // Handle null decimals value
     if (!property_exists($data, 'decimals') or $data->decimals < 0) {
-        $grade_item->decimals = null;
+        $gradeitem->decimals = null;
     }
 
     }
 
-    if (empty($grade_item->id)) {
-        $grade_item->itemtype = 'manual'; // all new items to be manual only
-        $grade_item->insert();
+    if (empty($gradeitem->id)) {
+        $gradeitem->itemtype = 'manual'; // All new items to be manual only.
+        $gradeitem->insert();
 
         // set parent if needed
         if (isset($data->parentcategory)) {
 
         // set parent if needed
         if (isset($data->parentcategory)) {
-            $grade_item->set_parent($data->parentcategory, false);
+            $gradeitem->set_parent($data->parentcategory, false);
         }
 
     } else {
         }
 
     } else {
-        $grade_item->update();
+        $gradeitem->update();
 
         if (!empty($data->rescalegrades) && $data->rescalegrades == 'yes') {
 
         if (!empty($data->rescalegrades) && $data->rescalegrades == 'yes') {
-            $newmin = $grade_item->grademin;
-            $newmax = $grade_item->grademax;
-            $grade_item->rescale_grades_keep_percentage($oldmin, $oldmax, $newmin, $newmax, 'gradebook');
+            $newmin = $gradeitem->grademin;
+            $newmax = $gradeitem->grademax;
+            $gradeitem->rescale_grades_keep_percentage($oldmin, $oldmax, $newmin, $newmax, 'gradebook');
         }
     }
 
         }
     }
 
-    // update hiding flag
-    if ($hiddenuntil) {
-        $grade_item->set_hidden($hiddenuntil, false);
-    } else {
-        $grade_item->set_hidden($hidden, false);
+    if ($item->cancontrolvisibility) {
+        // Update hiding flag.
+        $gradeitem->set_hidden($hide, false);
     }
 
     }
 
-    $grade_item->set_locktime($locktime); // locktime first - it might be removed when unlocking
-    $grade_item->set_locked($locked, false, true);
+    $gradeitem->set_locktime($locktime); // Locktime first - it might be removed when unlocking.
+    $gradeitem->set_locked($locked, false, true);
 
     redirect($returnurl);
 }
 
     redirect($returnurl);
 }
index f762cc7..fe5f58a 100644 (file)
@@ -181,10 +181,9 @@ class edit_item_form extends moodleform {
 
         /// hiding
         if ($item->cancontrolvisibility) {
 
         /// hiding
         if ($item->cancontrolvisibility) {
-            // advcheckbox is not compatible with disabledIf!
-            $mform->addElement('checkbox', 'hidden', get_string('hidden', 'grades'));
+            $mform->addElement('advcheckbox', 'hidden', get_string('hidden', 'grades'), '', [], [0, 1]);
             $mform->addElement('date_time_selector', 'hiddenuntil', get_string('hiddenuntil', 'grades'), array('optional'=>true));
             $mform->addElement('date_time_selector', 'hiddenuntil', get_string('hiddenuntil', 'grades'), array('optional'=>true));
-            $mform->disabledIf('hidden', 'hiddenuntil[off]', 'notchecked');
+            $mform->disabledIf('hidden', 'hiddenuntil[enabled]', 'checked');
         } else {
             $mform->addElement('static', 'hidden', get_string('hidden', 'grades'),
                     get_string('componentcontrolsvisibility', 'grades'));
         } else {
             $mform->addElement('static', 'hidden', get_string('hidden', 'grades'),
                     get_string('componentcontrolsvisibility', 'grades'));
index 54eecc8..4c437ed 100644 (file)
@@ -2762,5 +2762,27 @@ function xmldb_main_upgrade($oldversion) {
         upgrade_main_savepoint(true, 2020100700.00);
     }
 
         upgrade_main_savepoint(true, 2020100700.00);
     }
 
+    if ($oldversion < 2020101300.01) {
+        // Script to fix incorrect records of "hidden" field in existing grade items.
+        $sql = "SELECT cm.instance, cm.course
+                  FROM {course_modules} cm
+                  JOIN {modules} m ON m.id = cm.module
+                 WHERE m.name = :module AND cm.visible = :visible";
+        $hidequizlist = $DB->get_recordset_sql($sql, ['module' => 'quiz', 'visible' => 0]);
+
+        foreach ($hidequizlist as $hidequiz) {
+            $params = [
+                'itemmodule'    => 'quiz',
+                'courseid'      => $hidequiz->course,
+                'iteminstance'  => $hidequiz->instance,
+            ];
+
+            $DB->set_field('grade_items', 'hidden', 1, $params);
+        }
+        $hidequizlist->close();
+
+        upgrade_main_savepoint(true, 2020101300.01);
+    }
+
     return true;
 }
     return true;
 }
index a7d851b..4b0925e 100644 (file)
@@ -29,7 +29,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
 
 defined('MOODLE_INTERNAL') || die();
 
-$version  = 2020101300.00;              // YYYYMMDD      = weekly release date of this DEV branch.
+$version  = 2020101300.01;              // YYYYMMDD      = weekly release date of this DEV branch.
                                         //         RR    = release increments - 00 in DEV branches.
                                         //           .XX = incremental changes.
 $release  = '3.10dev+ (Build: 20201013)';// Human-friendly version name
                                         //         RR    = release increments - 00 in DEV branches.
                                         //           .XX = incremental changes.
 $release  = '3.10dev+ (Build: 20201013)';// Human-friendly version name