diff --git a/cohort/lib.php b/cohort/lib.php index 6d61d04eb20..48c7c87f645 100644 --- a/cohort/lib.php +++ b/cohort/lib.php @@ -384,13 +384,13 @@ function cohort_get_cohort($cohortorid, $currentcontext, $withcustomfields = fal if ($cohort && in_array($cohort->contextid, $currentcontext->get_parent_context_ids())) { if (!$cohort->visible) { - $cohort = false; - } else { $cohortcontext = context::instance_by_id($cohort->contextid); if (!has_capability('moodle/cohort:view', $cohortcontext)) { - $cohort = false; + return false; } } + } else { + return false; } if ($cohort && $withcustomfields) { diff --git a/cohort/tests/lib_test.php b/cohort/tests/lib_test.php index f1ba6dcd4dc..8bf48250d1c 100644 --- a/cohort/tests/lib_test.php +++ b/cohort/tests/lib_test.php @@ -710,7 +710,7 @@ class lib_test extends \advanced_testcase { $cohort2 = $this->getDataGenerator()->create_cohort(); // Test cohort_get_cohort. - $result = cohort_get_cohort($cohort1->id, \context_system::instance(), true); + $result = cohort_get_cohort($cohort1->id, $coursectx, true); $this->assertObjectHasAttribute('customfields', $result); $this->assertCount(1, $result->customfields); $field = reset($result->customfields); @@ -719,7 +719,7 @@ class lib_test extends \advanced_testcase { $this->assertEquals('Test value 1', $field->get_value()); // Test custom fields are not returned if not needed. - $result = cohort_get_cohort($cohort1->id, \context_system::instance()); + $result = cohort_get_cohort($cohort1->id, $coursectx); $this->assertObjectNotHasAttribute('customfields', $result); // Test cohort_get_cohorts. @@ -939,4 +939,35 @@ class lib_test extends \advanced_testcase { } } } + + /** + * Test the behaviour of cohort_get_cohort(). + * + * @covers ::cohort_get_cohort + */ + public function test_cohort_get_cohort() { + $this->resetAfterTest(); + + $cat = $this->getDataGenerator()->create_category(); + $cat1 = $this->getDataGenerator()->create_category(['parent' => $cat->id]); + $cat2 = $this->getDataGenerator()->create_category(['parent' => $cat->id]); + + $course1 = $this->getDataGenerator()->create_course(['category' => $cat1->id, 'shortname' => 'ANON1']); + $course2 = $this->getDataGenerator()->create_course(['category' => $cat2->id, 'shortname' => 'ANON2']); + + $cohort1 = $this->getDataGenerator()->create_cohort(['contextid' => \context_coursecat::instance($cat1->id)->id]); + + $result = cohort_get_cohort($cohort1->id, \context_course::instance($course2->id)); + $this->assertFalse($result); + + $result = cohort_get_cohort($cohort1->id, \context_course::instance($course2->id), true); + $this->assertFalse($result); + + $result = cohort_get_cohort($cohort1->id, \context_course::instance($course1->id)); + $this->assertEquals($cohort1->id, $result->id); + + $result = cohort_get_cohort($cohort1->id, \context_course::instance($course1->id), true); + $this->assertEquals($cohort1->id, $result->id); + } + }