Merge branch 'MDL-35762_fix' of git://github.com/andyjdavis/moodle
authorEloy Lafuente (stronk7) <stronk7@moodle.org>
Tue, 6 Nov 2012 15:13:47 +0000 (16:13 +0100)
committerEloy Lafuente (stronk7) <stronk7@moodle.org>
Tue, 6 Nov 2012 15:13:47 +0000 (16:13 +0100)
grade/report/user/lib.php
lib/modinfolib.php
lib/tests/modinfolib_test.php [new file with mode: 0644]

index 8b7cfeb..d5932c5 100644 (file)
@@ -348,13 +348,17 @@ class grade_report_user extends grade_report {
                     ($this->showhiddenitems == GRADE_REPORT_USER_HIDE_UNTIL && !$grade_grade->is_hiddenuntil()))) {
                 $hide = true;
             } else if (!empty($grade_object->itemmodule) && !empty($grade_object->iteminstance)) {
-                // The grade object can be marked visible but still be hidden if "enablegroupmembersonly"
-                // is on and it's an activity assigned to a grouping the user is not in.
+                // The grade object can be marked visible but still be hidden if...
+                //  1) "enablegroupmembersonly" is on and the activity is assigned to a grouping the user is not in.
+                //  2) the student cannot see the activity due to conditional access and its set to be hidden entirely.
                 $instances = $this->gtree->modinfo->get_instances_of($grade_object->itemmodule);
                 if (!empty($instances[$grade_object->iteminstance])) {
                     $cm = $instances[$grade_object->iteminstance];
-                    if ($cm->is_user_access_restricted_by_group()) {
-                        $hide = true;
+                    if (!$cm->uservisible) {
+                        // Further checks are required to determine whether the activity is entirely hidden or just greyed out.
+                        if ($cm->is_user_access_restricted_by_group() || $cm->is_user_access_restricted_by_conditional_access()) {
+                            $hide = true;
+                        }
                     }
                 }
             }
index ec02959..884d72a 100644 (file)
@@ -1128,8 +1128,12 @@ class cm_info extends stdClass {
     }
 
     /**
-     * Works out whether activity is visible *for current user* - if this is false, they
-     * aren't allowed to access it.
+     * Works out whether activity is available to the current user
+     *
+     * If the activity is unavailable, additional checks are required to determine if its hidden or greyed out
+     *
+     * @see is_user_access_restricted_by_group()
+     * @see is_user_access_restricted_by_conditional_access()
      * @return void
      */
     private function update_user_visible() {
@@ -1137,39 +1141,76 @@ class cm_info extends stdClass {
         $modcontext = context_module::instance($this->id);
         $userid = $this->modinfo->get_user_id();
         $this->uservisible = true;
-        // Check visibility/availability conditions.
+
+        // If the user cannot access the activity set the uservisible flag to false.
+        // Additional checks are required to determine whether the activity is entirely hidden or just greyed out.
         if ((!$this->visible or !$this->available) and
                 !has_capability('moodle/course:viewhiddenactivities', $modcontext, $userid)) {
-            // If the activity is hidden or unavailable, and you don't have viewhiddenactivities,
-            // set it so that user can't see or access it.
+
             $this->uservisible = false;
         }
-        // Check group membership. The grouping option makes the activity
-        // completely invisible as it does not apply to the user at all.
+
+        // Check group membership.
         if ($this->is_user_access_restricted_by_group()) {
-            $this->uservisible = false;
-            // Ensure activity is completely hidden from user.
+
+             $this->uservisible = false;
+            // Ensure activity is completely hidden from the user.
             $this->showavailability = 0;
         }
     }
 
     /**
-     * Checks whether the module group settings restrict the user access.
-     * @return bool true if the user access is restricted
+     * Checks whether the module's group settings restrict the current user's access
+     *
+     * @return bool True if the user access is restricted
      */
     public function is_user_access_restricted_by_group() {
         global $CFG;
+
+        if (!empty($CFG->enablegroupmembersonly) and !empty($this->groupmembersonly)) {
+            $modcontext = context_module::instance($this->id);
+            $userid = $this->modinfo->get_user_id();
+            if (!has_capability('moodle/site:accessallgroups', $modcontext, $userid)) {
+                // If the activity has 'group members only' and you don't have accessallgroups...
+                $groups = $this->modinfo->get_groups($this->groupingid);
+                if (empty($groups)) {
+                    // ...and you don't belong to a group, then set it so you can't see/access it
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+
+    /**
+     * Checks whether the module's conditional access settings mean that the user cannot see the activity at all
+     *
+     * @return bool True if the user cannot see the module. False if the activity is either available or should be greyed out.
+     */
+    public function is_user_access_restricted_by_conditional_access() {
+        global $CFG, $USER;
+
+        if (empty($CFG->enableavailability)) {
+            return false;
+        }
+
+        // If module will always be visible anyway (but greyed out), don't bother checking anything else
+        if ($this->showavailability == CONDITION_STUDENTVIEW_SHOW) {
+            return false;
+        }
+
+        // Can the user see hidden modules?
         $modcontext = context_module::instance($this->id);
         $userid = $this->modinfo->get_user_id();
-        if (!empty($CFG->enablegroupmembersonly) and !empty($this->groupmembersonly)
-                and !has_capability('moodle/site:accessallgroups', $modcontext, $userid)) {
-            // If the activity has 'group members only' and you don't have accessallgroups...
-            $groups = $this->modinfo->get_groups($this->groupingid);
-            if (empty($groups)) {
-                // ...and you don't belong to a group, then set it so you can't see/access it
-                return true;
-            }
+        if (has_capability('moodle/course:viewhiddenactivities', $modcontext, $userid)) {
+            return false;
         }
+
+        // Is the module hidden due to unmet conditions?
+        if (!$this->available) {
+            return true;
+        }
+
         return false;
     }
 
diff --git a/lib/tests/modinfolib_test.php b/lib/tests/modinfolib_test.php
new file mode 100644 (file)
index 0000000..34a90a5
--- /dev/null
@@ -0,0 +1,184 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Unit tests for lib/modinfolib.php.
+ *
+ * @package    core
+ * @category   phpunit
+ * @copyright  2012 Andrew Davis
+ */
+
+defined('MOODLE_INTERNAL') || die();
+
+global $CFG;
+require_once($CFG->libdir . '/modinfolib.php');
+require_once($CFG->libdir . '/conditionlib.php');
+
+/**
+ * Unit tests for modinfolib.php
+ *
+ * @copyright 2012 Andrew Davis
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class modinfolib_testcase extends advanced_testcase {
+
+    /**
+     * Test is_user_access_restricted_by_group()
+     *
+     * The underlying groups system is more thoroughly tested in lib/tests/grouplib_test.php
+     */
+    public function test_is_user_access_restricted_by_group() {
+        global $DB, $CFG, $USER;
+
+        $this->resetAfterTest(true);
+
+        // Create a course
+        $course = $this->getDataGenerator()->create_course();
+        $coursecontext = context_course::instance($course->id);
+
+        // Create a mod_assign instance
+        $assign = $this->getDataGenerator()->create_module('assign', array('course'=>$course->id));
+        $cm_info = get_fast_modinfo($course)->instances['assign'][$assign->id];
+
+        // Create and enrol a student
+        // Enrolment is necessary for groups to work
+        $studentrole = $DB->get_record('role', array('shortname'=>'student'), '*', MUST_EXIST);
+        $student = $this->getDataGenerator()->create_user();
+        role_assign($studentrole->id, $student->id, $coursecontext);
+        $enrolplugin = enrol_get_plugin('manual');
+        $enrolplugin->add_instance($course);
+        $enrolinstances = enrol_get_instances($course->id, false);
+        foreach ($enrolinstances as $enrolinstance) {
+            if ($enrolinstance->enrol === 'manual') {
+                break;
+            }
+        }
+        $enrolplugin->enrol_user($enrolinstance, $student->id);
+
+        // Switch to a student and reload the context info
+        $this->setUser($student);
+        $cm_info = $this->refresh_cm_info($course, $assign);
+
+        // Create up a teacher
+        $teacherrole = $DB->get_record('role', array('shortname'=>'editingteacher'), '*', MUST_EXIST);
+        $teacher = $this->getDataGenerator()->create_user();
+        role_assign($teacherrole->id, $teacher->id, $coursecontext);
+
+        // Create 2 groupings
+        $grouping1 = $this->getDataGenerator()->create_grouping(array('courseid' => $course->id, 'name' => 'grouping1'));
+        $grouping2 = $this->getDataGenerator()->create_grouping(array('courseid' => $course->id, 'name' => 'grouping2'));
+
+        // Create 2 groups and put them in the groupings
+        $group1 = $this->getDataGenerator()->create_group(array('courseid' => $course->id, 'idnumber' => 'group1'));
+        groups_assign_grouping($grouping1->id, $group1->id);
+        $group2 = $this->getDataGenerator()->create_group(array('courseid' => $course->id, 'idnumber' => 'group2'));
+        groups_assign_grouping($grouping2->id, $group2->id);
+
+        // If groups are disabled, the activity isn't restricted.
+        $CFG->enablegroupmembersonly = false;
+        $this->assertFalse($cm_info->is_user_access_restricted_by_group());
+
+        // If groups are on but "group members only" is off, the activity isn't restricted.
+        $CFG->enablegroupmembersonly = true;
+        $cm_info->groupmembersonly = NOGROUPS;
+        $this->assertFalse($cm_info->is_user_access_restricted_by_group());
+
+        // If "group members only" is on but user is in the wrong group, the activity is restricted.
+        $cm_info->groupmembersonly = SEPARATEGROUPS;
+        $cm_info->groupingid = $grouping1->id;
+        $this->assertTrue(groups_add_member($group2, $USER));
+        $this->assertTrue($cm_info->is_user_access_restricted_by_group());
+
+        // If the user is in the required group, the activity isn't restricted.
+        groups_remove_member($group2, $USER);
+        $this->assertTrue(groups_add_member($group1, $USER));
+        $cm_info = $this->refresh_cm_info($course, $assign);
+        $this->assertFalse($cm_info->is_user_access_restricted_by_group());
+
+        // Switch to a teacher and reload the context info
+        $this->setUser($teacher);
+        $cm_info = $this->refresh_cm_info($course, $assign);
+
+        // If the user isn't in the required group but has 'moodle/site:accessallgroups', the activity isn't restricted.
+        $this->assertTrue(has_capability('moodle/site:accessallgroups', $coursecontext));
+        $this->assertFalse($cm_info->is_user_access_restricted_by_group());
+    }
+
+    /**
+     * Test is_user_access_restricted_by_conditional_access()
+     *
+     * The underlying conditional access system is more thoroughly tested in lib/tests/conditionlib_test.php
+     */
+    public function test_is_user_access_restricted_by_conditional_access() {
+        global $DB, $CFG;
+
+        $this->resetAfterTest(true);
+
+        // Create a course and a mod_assign instance
+        $course = $this->getDataGenerator()->create_course();
+        $assign = $this->getDataGenerator()->create_module('assign', array('course'=>$course->id));
+        $cm_info = get_fast_modinfo($course)->instances['assign'][$assign->id];
+
+        // Set up a teacher
+        $coursecontext = context_course::instance($course->id);
+        $teacherrole = $DB->get_record('role', array('shortname'=>'editingteacher'), '*', MUST_EXIST);
+        $teacher = $this->getDataGenerator()->create_user();
+        role_assign($teacherrole->id, $teacher->id, $coursecontext);
+
+        // Mark the activity as unavailable (due to unmet conditions)
+        // Testing of the code that normally turns this flag on and off is done in conditionlib_test.php
+        $cm_info->available = false;
+        // Set the activity to be hidden entirely if it is unavailable to the user
+        $cm_info->showavailability = CONDITION_STUDENTVIEW_HIDE;
+
+        // If conditional availability is disabled the activity will always be unrestricted
+        $CFG->enableavailability = false;
+        $this->assertFalse($cm_info->is_user_access_restricted_by_conditional_access());
+
+        // Turn on conditional availability
+        $CFG->enableavailability = true;
+
+        // The unavailable, hidden entirely activity should now be restricted
+        $this->assertTrue($cm_info->is_user_access_restricted_by_conditional_access());
+
+        // If the activity is available it should not be restricted
+        $cm_info->available = true;
+        $this->assertFalse($cm_info->is_user_access_restricted_by_conditional_access());
+
+        // If the activity is unavailable and set to be greyed out it should not be restricted
+        $cm_info->available = false;
+        $cm_info->showavailability = CONDITION_STUDENTVIEW_SHOW;
+        $this->assertFalse($cm_info->is_user_access_restricted_by_conditional_access());
+
+        // If the activity is unavailable and set to be hidden entirely its restricted unless user has 'moodle/course:viewhiddenactivities'
+        $cm_info->available = false;
+        $cm_info->showavailability = CONDITION_STUDENTVIEW_HIDE;
+        
+        // Switch to a teacher and reload the context info
+        $this->setUser($teacher);
+        $cm_info = $this->refresh_cm_info($course, $assign);
+
+        $this->assertTrue(has_capability('moodle/course:viewhiddenactivities', $coursecontext));
+        $this->assertFalse($cm_info->is_user_access_restricted_by_conditional_access());
+    }
+    
+    private function refresh_cm_info($course, $assign) {
+        $reset = 'reset';
+        get_fast_modinfo($reset);
+        return get_fast_modinfo($course)->instances['assign'][$assign->id];
+    }
+}