MDL-44725 Availability: Replace groupmembersonly - core_availability (2)
authorsam marshall <s.marshall@open.ac.uk>
Fri, 1 Aug 2014 16:07:17 +0000 (17:07 +0100)
committersam marshall <s.marshall@open.ac.uk>
Tue, 2 Sep 2014 12:03:09 +0000 (13:03 +0100)
Remove groupmembersonly usage in the core_availability API, and change
the update code (used in backup) so that it considers groupmembersonly
when restoring old backups.

availability/classes/info.php
availability/classes/info_module.php
availability/condition/group/classes/frontend.php
availability/condition/grouping/classes/frontend.php
availability/tests/info_test.php

index b7c6d3d..1f31f1b 100644 (file)
@@ -403,19 +403,19 @@ abstract class info {
      * Supported fields: availablefrom, availableuntil, showavailability
      * (and groupingid for sections).
      *
-     * If you enable $modgroupmembersonly, then it also supports the
-     * groupmembersonly field for modules. This is off by default because
-     * we are not yet moving the groupmembersonly option into this new API.
+     * It also supports the groupmembersonly field for modules. This part was
+     * optional in 2.7 but now always runs (because groupmembersonly has been
+     * removed).
      *
      * @param \stdClass $rec Object possibly containing legacy fields
      * @param bool $section True if this is a section
-     * @param bool $modgroupmembersonly True if groupmembersonly is converted for mods
+     * @param bool $modgroupmembersonlyignored Ignored option, previously used
      * @return string|null New availability value or null if none
      */
-    public static function convert_legacy_fields($rec, $section, $modgroupmembersonly = false) {
+    public static function convert_legacy_fields($rec, $section, $modgroupmembersonlyignored = false) {
         // Do nothing if the fields are not set.
         if (empty($rec->availablefrom) && empty($rec->availableuntil) &&
-                (!$modgroupmembersonly || empty($rec->groupmembersonly)) &&
+                (empty($rec->groupmembersonly)) &&
                 (!$section || empty($rec->groupingid))) {
             return null;
         }
@@ -426,7 +426,7 @@ abstract class info {
 
         // Groupmembersonly condition (if enabled) for modules, groupingid for
         // sections.
-        if (($modgroupmembersonly && !empty($rec->groupmembersonly)) ||
+        if (!empty($rec->groupmembersonly) ||
                 (!empty($rec->groupingid) && $section)) {
             if (!empty($rec->groupingid)) {
                 $conditions[] = '{"type":"grouping"' .
index af4ae5f..1bff351 100644 (file)
@@ -113,8 +113,7 @@ class info_module extends info {
      * Checks if an activity is visible to the given user.
      *
      * Unlike other checks in the availability system, this check includes the
-     * $cm->visible flag and also (if enabled) the groupmembersonly feature.
-     * It is equivalent to $cm->uservisible.
+     * $cm->visible flag. It is equivalent to $cm->uservisible.
      *
      * If you have already checked (or do not care whether) the user has access
      * to the course, you can set $checkcourse to false to save it checking
@@ -160,11 +159,6 @@ class info_module extends info {
             $cm = $DB->get_record('course_modules', array('id' => $cmorid), '*', MUST_EXIST);
         }
 
-        // Check the groupmembersonly feature.
-        if (!groups_course_module_visible($cm, $userid)) {
-            return false;
-        }
-
         // If requested, check user can access the course.
         if ($checkcourse) {
             $coursecontext = \context_course::instance($cm->course);
index 499d1a4..a6f1fd2 100644 (file)
@@ -79,14 +79,6 @@ class frontend extends \core_availability\frontend {
             \section_info $section = null) {
         global $CFG;
 
-        // If groupmembersonly is turned on, then you can only add group
-        // restrictions on sections (which don't use groupmembersonly) and
-        // not on modules. This is to avoid confusion - otherwise
-        // there would be two ways to add restrictions based on groups.
-        if (is_null($section) && $CFG->enablegroupmembersonly) {
-            return false;
-        }
-
         // Only show this option if there are some groups.
         return count($this->get_all_groups($course->id)) > 0;
     }
index 456fb26..f8f2186 100644 (file)
@@ -74,14 +74,6 @@ class frontend extends \core_availability\frontend {
             \section_info $section = null) {
         global $CFG, $DB;
 
-        // If groupmembersonly is turned on, then you can only add group
-        // restrictions on sections (which don't use groupmembersonly) and
-        // not on modules. This is to avoid confusion - otherwise
-        // there would be two ways to add restrictions based on groups.
-        if (is_null($section) && $CFG->enablegroupmembersonly) {
-            return false;
-        }
-
         // Check if groupings are in use for the course. (Unlike the 'group'
         // condition there is no case where you might want to set up the
         // condition before you set a grouping - there is no 'any grouping'
index d016567..0ee25ee 100644 (file)
@@ -166,7 +166,6 @@ class info_testcase extends \advanced_testcase {
         // 1. Availability restriction (mock, set to fail).
         // 2. Availability restriction on section (mock, set to fail).
         // 3. Actually visible.
-        // 4. With groupmembersonly flag.
         $generator = $this->getDataGenerator();
         $course = $generator->create_course(
                 array('numsections' => 1), array('createsections' => true));
@@ -177,7 +176,6 @@ class info_testcase extends \advanced_testcase {
         $pages[1] = $pagegen->create_instance($rec);
         $pages[2] = $pagegen->create_instance($rec);
         $pages[3] = $pagegen->create_instance($rec);
-        $pages[4] = $pagegen->create_instance($rec, array('groupmembersonly' => 1));
         $modinfo = get_fast_modinfo($course);
         $section = $modinfo->get_section_info(1);
         $cm = $modinfo->get_cm($pages[2]->cmid);
@@ -250,18 +248,6 @@ class info_testcase extends \advanced_testcase {
         // section's availability.
         $this->assertFalse(info_module::is_user_visible($pages[1]->cmid, $student->id, false));
         $this->assertFalse(info_module::is_user_visible($pages[2]->cmid, $student->id, false));
-
-        // Groupmembersonly uses a different flag.
-        $CFG->enableavailability = false;
-        $this->assertTrue(info_module::is_user_visible($pages[4]->cmid, $student->id, false));
-        $CFG->enablegroupmembersonly = true;
-        $this->assertFalse(info_module::is_user_visible($pages[4]->cmid, $student->id, false));
-
-        // There is no way to clear a static cache used in grouplib, so test the
-        // positive case on an identical but different user.
-        $group = $generator->create_group(array('courseid' => $course->id));
-        groups_add_member($group, $student2);
-        $this->assertTrue(info_module::is_user_visible($pages[4]->cmid, $student2->id, false));
     }
 
     /**
@@ -278,18 +264,17 @@ class info_testcase extends \advanced_testcase {
                 '{"op":"&","showc":[false],"c":[{"type":"grouping","id":7}]}',
                 info::convert_legacy_fields($rec, true));
 
-        // Check groupmembersonly with grouping - only if flag enabled.
+        // Check groupmembersonly with grouping.
         $rec->groupmembersonly = 1;
         $this->assertEquals(
                 '{"op":"&","showc":[false],"c":[{"type":"grouping","id":7}]}',
-                info::convert_legacy_fields($rec, false, true));
-        $this->assertNull(info::convert_legacy_fields($rec, false));
+                info::convert_legacy_fields($rec, false));
 
         // Check groupmembersonly without grouping.
         $rec->groupingid = 0;
         $this->assertEquals(
                 '{"op":"&","showc":[false],"c":[{"type":"group"}]}',
-                info::convert_legacy_fields($rec, false, true));
+                info::convert_legacy_fields($rec, false));
 
         // Check start date.
         $rec->groupmembersonly = 0;
@@ -317,7 +302,8 @@ class info_testcase extends \advanced_testcase {
         $rec->groupmembersonly = 1;
         $rec->availablefrom = 123;
         $this->assertEquals(
-                '{"op":"&","showc":[true,false],"c":[' .
+                '{"op":"&","showc":[false,true,false],"c":[' .
+                '{"type":"grouping","id":7},' .
                 '{"type":"date","d":">=","t":123},' .
                 '{"type":"date","d":"<","t":456}' .
                 ']}',