MDL-31487 fix FEATURE_CONTROLS_GRADE_VISIBILITY for quiz.

This is a followup to MDL-18301. That fix missed the following points:

1. On the edit categories and items screens, all items had an eye-con to
control the visibility, even if the visibility was controlled by the
module.

2. Changing the visibility of a grade category change the visibility of
all items within it, even if the visibility was controlled by the
module.

3. The quiz ingored $cm->visible when controlling whether its grade item
was visible.

diff --git a/grade/edit/tree/action.php b/grade/edit/tree/action.php
index 33c702e..f8d18f3 100644 (file)
--- a/grade/edit/tree/action.php
+++ b/grade/edit/tree/action.php
@@ -62,6 +62,9 @@ switch ($action) {
             if ($type == 'grade' and empty($object->id)) {
                 $object->insert();
             }
+            if (!$object->can_control_visibility()) {
+                print_error('componentcontrolsvisibility', 'grades', $returnurl);
+            }
             $object->set_hidden(1, true);
         }
         break;
@@ -74,6 +77,9 @@ switch ($action) {
             if ($type == 'grade' and empty($object->id)) {
                 $object->insert();
             }
+            if (!$object->can_control_visibility()) {
+                print_error('componentcontrolsvisibility', 'grades', $returnurl);
+            }
             $object->set_hidden(0, true);
         }
         break;

diff --git a/grade/edit/tree/lib.php b/grade/edit/tree/lib.php
index 6a71f1a..e782303 100644 (file)
--- a/grade/edit/tree/lib.php
+++ b/grade/edit/tree/lib.php
@@ -26,7 +26,7 @@ class grade_edit_tree {
     public $columns = array();
 
     /**
-     * @var object $gtree          @see grade/lib.php
+     * @var grade_tree $gtree   @see grade/lib.php
      */
     public $gtree;
 

diff --git a/grade/lib.php b/grade/lib.php
index 3603ac4..7639e08 100644 (file)
--- a/grade/lib.php
+++ b/grade/lib.php
@@ -1541,6 +1541,10 @@ class grade_structure {
     public function get_hiding_icon($element, $gpr) {
         global $CFG, $OUTPUT;
 
+        if (!$element['object']->can_control_visibility()) {
+            return '';
+        }
+
         if (!has_capability('moodle/grade:manage', $this->context) and
             !has_capability('moodle/grade:hide', $this->context)) {
             return '';

diff --git a/lib/grade/grade_category.php b/lib/grade/grade_category.php
index 93d0185..190ab09 100644 (file)
--- a/lib/grade/grade_category.php
+++ b/lib/grade/grade_category.php
@@ -1616,7 +1616,9 @@ class grade_category extends grade_object {
             if ($children = grade_item::fetch_all(array('categoryid'=>$this->id))) {
 
                 foreach ($children as $child) {
-                    $child->set_hidden($hidden, $cascade);
+                    if ($child->can_control_visibility()) {
+                        $child->set_hidden($hidden, $cascade);
+                    }
                 }
             }
 

diff --git a/lib/grade/grade_item.php b/lib/grade/grade_item.php
index 9ca8c36..ce6713b 100644 (file)
--- a/lib/grade/grade_item.php
+++ b/lib/grade/grade_item.php
@@ -2097,6 +2097,6 @@ class grade_item extends grade_object {
         if (core_component::get_plugin_directory($this->itemtype, $this->itemmodule)) {
             return !plugin_supports($this->itemtype, $this->itemmodule, FEATURE_CONTROLS_GRADE_VISIBILITY, false);
         }
-        return true;
+        return parent::can_control_visibility();
     }
 }

diff --git a/lib/grade/grade_object.php b/lib/grade/grade_object.php
index 5c7e6ad..cea1c18 100644 (file)
--- a/lib/grade/grade_object.php
+++ b/lib/grade/grade_object.php
@@ -438,4 +438,13 @@ abstract class grade_object {
         $this->hidden = $hidden;
         $this->update();
     }
+
+    /**
+     * Returns whether the grade object can control the visibility of the grades.
+     *
+     * @return bool
+     */
+    public function can_control_visibility() {
+        return true;
+    }
 }

diff --git a/lib/grade/tests/fixtures/lib.php b/lib/grade/tests/fixtures/lib.php
index dcdb3c8..b7d90ef 100644 (file)
--- a/lib/grade/tests/fixtures/lib.php
+++ b/lib/grade/tests/fixtures/lib.php
@@ -105,6 +105,9 @@ abstract class grade_base_testcase extends advanced_testcase {
 
         $this->activities[6] = $this->getDataGenerator()->create_module('forum', array('course'=>$this->course->id));
         $this->course_module[6] = get_coursemodule_from_instance('forum', $this->activities[6]->id);
+
+        $this->activities[7] = $this->getDataGenerator()->create_module('quiz', array('course'=>$this->course->id));
+        $this->course_module[7] = get_coursemodule_from_instance('quiz', $this->activities[7]->id);
     }
 
     private function load_scales() {
@@ -494,6 +497,29 @@ abstract class grade_base_testcase extends advanced_testcase {
 
         $grade_item->id = $DB->insert_record('grade_items', $grade_item);
         $this->grade_items[10] = $grade_item;
+
+        // Quiz grade_item (course_module = 7).
+        // id = 11
+        $grade_item = new stdClass();
+
+        $grade_item->courseid = $this->course->id;
+        $grade_item->categoryid = $course_category->id;
+        $grade_item->itemname = 'Quiz grade item';
+        $grade_item->itemtype = 'mod';
+        $grade_item->itemmodule = $this->course_module[7]->modname;
+        $grade_item->iteminstance = $this->course_module[7]->instance;
+        $grade_item->itemnumber = 0;
+        $grade_item->gradetype = GRADE_TYPE_VALUE;
+        $grade_item->grademin = 0;
+        $grade_item->grademax = 100;
+        $grade_item->locked = 0;
+        $grade_item->iteminfo = 'Quiz grade item used for unit testing';
+        $grade_item->timecreated = time();
+        $grade_item->timemodified = time();
+        $grade_item->sortorder = 11;
+
+        $grade_item->id = $DB->insert_record('grade_items', $grade_item);
+        $this->grade_items[11] = $grade_item;
     }
 
     /**
@@ -772,5 +798,3 @@ abstract class grade_base_testcase extends advanced_testcase {
         $this->grade_outcomes[] = $grade_outcome;
     }
 }
-
-

diff --git a/lib/grade/tests/grade_category_test.php b/lib/grade/tests/grade_category_test.php
index 60ad6d2..bf15662 100644 (file)
--- a/lib/grade/tests/grade_category_test.php
+++ b/lib/grade/tests/grade_category_test.php
@@ -60,6 +60,7 @@ class core_grade_category_testcase extends grade_base_testcase {
         $this->sub_test_grade_category_set_locked();
         $this->sub_test_grade_category_is_hidden();
         $this->sub_test_grade_category_set_hidden();
+        $this->sub_test_grade_category_can_control_visibility();
 
         //this won't work until MDL-11837 is complete
         //$this->sub_test_grade_category_generate_grades();
@@ -693,6 +694,11 @@ class core_grade_category_testcase extends grade_base_testcase {
         $this->assertEquals(true, $category->grade_item->is_hidden());
     }
 
+    protected function sub_test_grade_category_can_control_visibility() {
+        $category = new grade_category($this->grade_categories[0]);
+        $this->assertTrue($category->can_control_visibility());
+    }
+
     //beware: adding a duplicate course category messes up the data in a way that's hard to recover from
     protected function sub_test_grade_category_insert_course_category() {
         $grade_category = new grade_category();

diff --git a/lib/grade/tests/grade_item_test.php b/lib/grade/tests/grade_item_test.php
index b884d8d..70ec325 100644 (file)
--- a/lib/grade/tests/grade_item_test.php
+++ b/lib/grade/tests/grade_item_test.php
@@ -64,6 +64,7 @@ class core_grade_item_testcase extends grade_base_testcase {
         $this->sub_test_grade_item_get_calculation();
         $this->sub_test_grade_item_compute();
         $this->sub_test_update_final_grade();
+        $this->sub_test_grade_item_can_control_visibility();
     }
 
     protected function sub_test_grade_item_construct() {
@@ -99,7 +100,7 @@ class core_grade_item_testcase extends grade_base_testcase {
         $last_grade_item = end($this->grade_items);
 
         $this->assertEquals($grade_item->id, $last_grade_item->id + 1);
-        $this->assertEquals(11, $grade_item->sortorder);
+        $this->assertEquals(12, $grade_item->sortorder);
 
         //keep our reference collection the same as what is in the database
         $this->grade_items[] = $grade_item;
@@ -588,4 +589,14 @@ class core_grade_item_testcase extends grade_base_testcase {
         $this->assertEquals($min, $grade_grade->rawgrademin);
         $this->assertEquals($max, $grade_grade->rawgrademax);
     }
+
+    protected function sub_test_grade_item_can_control_visibility() {
+        // Grade item 0 == Course module 0 == Assignment.
+        $grade_item = new grade_item($this->grade_items[0], false);
+        $this->assertTrue($grade_item->can_control_visibility());
+
+        // Grade item  == Course module 7 == Quiz.
+        $grade_item = new grade_item($this->grade_items[11], false);
+        $this->assertFalse($grade_item->can_control_visibility());
+    }
 }

diff --git a/mod/quiz/lib.php b/mod/quiz/lib.php
index ae23d46..c6394ad 100644 (file)
--- a/mod/quiz/lib.php
+++ b/mod/quiz/lib.php
@@ -719,6 +719,13 @@ function quiz_grade_item_update($quiz, $grades = null) {
         $params['hidden'] = 0;
     }
 
+    if (!$params['hidden']) {
+        // If the grade item is not hidden by the quiz logic, then we need to
+        // hide it if the quiz is hidden from students.
+        $cm = get_coursemodule_from_instance('quiz', $quiz->id);
+        $params['hidden'] = !$cm->visible;
+    }
+
     if ($grades  === 'reset') {
         $params['reset'] = true;
         $grades = null;