diff --git a/calendar/lib.php b/calendar/lib.php index 3d8f0ceef71..46dfb8d450f 100644 --- a/calendar/lib.php +++ b/calendar/lib.php @@ -2442,9 +2442,7 @@ function calendar_get_default_courses($courseid = null, $fields = '*', $canmanag $prefixedfields = array_map(function($value) { return 'c.' . trim(strtolower($value)); }, $fieldlist); - if (!in_array('c.visible', $prefixedfields) && !in_array('c.*', $prefixedfields)) { - $prefixedfields[] = 'c.visible'; - } + $courses = get_courses('all', 'c.shortname', implode(',', $prefixedfields)); } else { $courses = enrol_get_users_courses($userid, true, $fields); diff --git a/lib/datalib.php b/lib/datalib.php index 64d0c34d358..da16bca3e9f 100644 --- a/lib/datalib.php +++ b/lib/datalib.php @@ -614,7 +614,7 @@ function get_course($courseid, $clone = true) { * @uses CONTEXT_COURSE * @param string|int $categoryid Either a category id or 'all' for everything * @param string $sort A field and direction to sort by - * @param string $fields The additional fields to return + * @param string $fields The additional fields to return (note that "id, category, visible" are always present) * @return array Array of courses */ function get_courses($categoryid="all", $sort="c.sortorder ASC", $fields="c.*") { @@ -642,6 +642,19 @@ function get_courses($categoryid="all", $sort="c.sortorder ASC", $fields="c.*") $ccjoin = "LEFT JOIN {context} ctx ON (ctx.instanceid = c.id AND ctx.contextlevel = :contextlevel)"; $params['contextlevel'] = CONTEXT_COURSE; + // The fields "id, category, visible" are required in the subsequent loop and must always be present. + if ($fields !== 'c.*') { + $fieldarray = array_merge( + // Split fields on comma + zero or more whitespace, merge with required fields. + preg_split('/,\s*/', $fields), [ + 'c.id', + 'c.category', + 'c.visible', + ] + ); + $fields = implode(',', array_unique($fieldarray)); + } + $sql = "SELECT $fields $ccselect FROM {course} c $ccjoin diff --git a/lib/tests/datalib_test.php b/lib/tests/datalib_test.php index 8ba4e43e6c1..860b5ea00de 100644 --- a/lib/tests/datalib_test.php +++ b/lib/tests/datalib_test.php @@ -290,6 +290,36 @@ class core_datalib_testcase extends advanced_testcase { $this->assertEquals($before + 1, $DB->perf_get_queries()); } + /** + * Test that specifying fields when calling get_courses always returns required fields "id, category, visible" + */ + public function test_get_courses_with_fields(): void { + $this->resetAfterTest(); + + $category = $this->getDataGenerator()->create_category(); + $course = $this->getDataGenerator()->create_course(['category' => $category->id]); + + // Specify "id" only. + $courses = get_courses($category->id, 'c.sortorder', 'c.id'); + $this->assertCount(1, $courses); + $this->assertEquals((object) [ + 'id' => $course->id, + 'category' => $course->category, + 'visible' => $course->visible, + ], reset($courses)); + + // Specify some optional fields. + $courses = get_courses($category->id, 'c.sortorder', 'c.id, c.shortname, c.fullname'); + $this->assertCount(1, $courses); + $this->assertEquals((object) [ + 'id' => $course->id, + 'category' => $course->category, + 'visible' => $course->visible, + 'shortname' => $course->shortname, + 'fullname' => $course->fullname, + ], reset($courses)); + } + public function test_increment_revision_number() { global $DB; $this->resetAfterTest();