Merge branch 'MDL-70924-39' of git://github.com/paulholden/moodle int…

…o MOODLE_39_STABLE
moodle · Mar 18, 2021 · 918d45d · 918d45d
2 parents 35b29cf + 51b2acf
commit 918d45d
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 4 deletions.
diff --git a/calendar/lib.php b/calendar/lib.php
@@ -2441,9 +2441,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
@@ -609,7 +609,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.*") {
@@ -637,6 +637,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
@@ -290,6 +290,36 @@ public function test_get_course() {
         $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();