From 3d962d49cbf3f789e9daa03b564fae2bb67d4a18 Mon Sep 17 00:00:00 2001 From: sam marshall Date: Fri, 1 Aug 2014 17:07:17 +0100 Subject: [PATCH] MDL-44725 Availability: Replace groupmembersonly - core_availability (2) 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 | 14 +++++------ availability/classes/info_module.php | 8 +------ .../condition/group/classes/frontend.php | 8 ------- .../condition/grouping/classes/frontend.php | 8 ------- availability/tests/info_test.php | 24 ++++--------------- 5 files changed, 13 insertions(+), 49 deletions(-) diff --git a/availability/classes/info.php b/availability/classes/info.php index b7c6d3d070cd4..1f31f1b30cd20 100644 --- a/availability/classes/info.php +++ b/availability/classes/info.php @@ -403,19 +403,19 @@ protected function update_dependency_id($table, $oldid, $newid) { * 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 @@ public static function convert_legacy_fields($rec, $section, $modgroupmembersonl // 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"' . diff --git a/availability/classes/info_module.php b/availability/classes/info_module.php index af4ae5f3ee78b..1bff351737b52 100644 --- a/availability/classes/info_module.php +++ b/availability/classes/info_module.php @@ -113,8 +113,7 @@ public function filter_user_list(array $users) { * 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 @@ public static function is_user_visible($cmorid, $userid = 0, $checkcourse = true $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); diff --git a/availability/condition/group/classes/frontend.php b/availability/condition/group/classes/frontend.php index 499d1a452848f..a6f1fd23ecdde 100644 --- a/availability/condition/group/classes/frontend.php +++ b/availability/condition/group/classes/frontend.php @@ -79,14 +79,6 @@ protected function allow_add($course, \cm_info $cm = null, \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; } diff --git a/availability/condition/grouping/classes/frontend.php b/availability/condition/grouping/classes/frontend.php index 456fb26033b6e..f8f21862426dc 100644 --- a/availability/condition/grouping/classes/frontend.php +++ b/availability/condition/grouping/classes/frontend.php @@ -74,14 +74,6 @@ protected function allow_add($course, \cm_info $cm = null, \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' diff --git a/availability/tests/info_test.php b/availability/tests/info_test.php index d0165679735aa..0ee25ee32fb7f 100644 --- a/availability/tests/info_test.php +++ b/availability/tests/info_test.php @@ -166,7 +166,6 @@ public function test_is_user_visible() { // 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 @@ public function test_is_user_visible() { $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 @@ public function test_is_user_visible() { // 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 @@ public function test_convert_legacy_fields() { '{"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 @@ public function test_convert_legacy_fields() { $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}' . ']}',