MDL-61804 admin: Add setting for course visibility sorting
authorAlexander Bias <alexander.bias@uni-ulm.de>
Mon, 2 Sep 2019 12:17:40 +0000 (14:17 +0200)
committerSara Arjona <sara@moodle.com>
Tue, 8 Oct 2019 09:58:39 +0000 (11:58 +0200)
admin/settings/appearance.php
admin/tool/monitor/lib.php
enrol/tests/enrollib_test.php
lang/en/admin.php
lib/enrollib.php
version.php

index dc924f7..c9d406a 100644 (file)
@@ -197,6 +197,10 @@ preferences,moodle|/user/preferences.php|t/preferences',
         'idnumber' => new lang_string('sort_idnumber', 'admin'),
     );
     $temp->add(new admin_setting_configselect('navsortmycoursessort', new lang_string('navsortmycoursessort', 'admin'), new lang_string('navsortmycoursessort_help', 'admin'), 'sortorder', $sortoptions));
         'idnumber' => new lang_string('sort_idnumber', 'admin'),
     );
     $temp->add(new admin_setting_configselect('navsortmycoursessort', new lang_string('navsortmycoursessort', 'admin'), new lang_string('navsortmycoursessort_help', 'admin'), 'sortorder', $sortoptions));
+    $temp->add(new admin_setting_configcheckbox('navsortmycourseshiddenlast',
+            new lang_string('navsortmycourseshiddenlast', 'admin'),
+            new lang_string('navsortmycourseshiddenlast_help', 'admin'),
+            1));
     $temp->add(new admin_setting_configtext('navcourselimit', new lang_string('navcourselimit', 'admin'),
         new lang_string('confignavcourselimit', 'admin'), 10, PARAM_INT));
     $temp->add(new admin_setting_configcheckbox('usesitenameforsitepages', new lang_string('usesitenameforsitepages', 'admin'), new lang_string('configusesitenameforsitepages', 'admin'), 0));
     $temp->add(new admin_setting_configtext('navcourselimit', new lang_string('navcourselimit', 'admin'),
         new lang_string('confignavcourselimit', 'admin'), 10, PARAM_INT));
     $temp->add(new admin_setting_configcheckbox('usesitenameforsitepages', new lang_string('usesitenameforsitepages', 'admin'), new lang_string('configusesitenameforsitepages', 'admin'), 0));
index 58078ec..df11ac9 100644 (file)
@@ -119,7 +119,9 @@ function tool_monitor_can_subscribe() {
  * @return array|bool Returns an array of courses or false if the user has no permission to subscribe to rules.
  */
 function tool_monitor_get_user_courses() {
  * @return array|bool Returns an array of courses or false if the user has no permission to subscribe to rules.
  */
 function tool_monitor_get_user_courses() {
-    $orderby = 'visible DESC, sortorder ASC';
+    // Get the course sorting according to the admin settings.
+    $sort = enrol_get_courses_sortingsql();
+
     $options = array();
     if (has_capability('tool/monitor:subscribe', context_system::instance())) {
         $options[0] = get_string('site');
     $options = array();
     if (has_capability('tool/monitor:subscribe', context_system::instance())) {
         $options[0] = get_string('site');
@@ -134,7 +136,7 @@ function tool_monitor_get_user_courses() {
         );
 
     $fields = implode(', ', $fieldlist);
         );
 
     $fields = implode(', ', $fieldlist);
-    if ($courses = get_user_capability_course('tool/monitor:subscribe', null, true, $fields, $orderby)) {
+    if ($courses = get_user_capability_course('tool/monitor:subscribe', null, true, $fields, $sort)) {
         foreach ($courses as $course) {
             context_helper::preload_from_record($course);
             $coursectx = context_course::instance($course->id);
         foreach ($courses as $course) {
             context_helper::preload_from_record($course);
             $coursectx = context_course::instance($course->id);
index e44cef6..9604b19 100644 (file)
@@ -58,14 +58,17 @@ class core_enrollib_testcase extends advanced_testcase {
 
         $course1 = $this->getDataGenerator()->create_course(array(
             'shortname' => 'Z',
 
         $course1 = $this->getDataGenerator()->create_course(array(
             'shortname' => 'Z',
+            'idnumber' => '123',
             'category' => $category1->id,
         ));
         $course2 = $this->getDataGenerator()->create_course(array(
             'shortname' => 'X',
             'category' => $category1->id,
         ));
         $course2 = $this->getDataGenerator()->create_course(array(
             'shortname' => 'X',
+            'idnumber' => '789',
             'category' => $category2->id,
         ));
         $course3 = $this->getDataGenerator()->create_course(array(
             'shortname' => 'Y',
             'category' => $category2->id,
         ));
         $course3 = $this->getDataGenerator()->create_course(array(
             'shortname' => 'Y',
+            'idnumber' => '456',
             'category' => $category2->id,
             'visible' => 0,
         ));
             'category' => $category2->id,
             'visible' => 0,
         ));
@@ -163,7 +166,7 @@ class core_enrollib_testcase extends advanced_testcase {
         $this->assertTrue(property_exists($course, 'timecreated'));
 
         $courses = enrol_get_all_users_courses($user2->id, false, null, 'id DESC');
         $this->assertTrue(property_exists($course, 'timecreated'));
 
         $courses = enrol_get_all_users_courses($user2->id, false, null, 'id DESC');
-        $this->assertEquals(array($course3->id, $course2->id, $course1->id), array_keys($courses));
+        $this->assertEquals(array($course2->id, $course3->id, $course1->id), array_keys($courses));
 
         // Make sure that implicit sorting defined in navsortmycoursessort is respected.
 
 
         // Make sure that implicit sorting defined in navsortmycoursessort is respected.
 
@@ -175,7 +178,54 @@ class core_enrollib_testcase extends advanced_testcase {
         // But still the explicit sorting takes precedence over the implicit one.
 
         $courses = enrol_get_all_users_courses($user1->id, false, null, 'shortname DESC');
         // But still the explicit sorting takes precedence over the implicit one.
 
         $courses = enrol_get_all_users_courses($user1->id, false, null, 'shortname DESC');
+        $this->assertEquals(array($course2->id, $course1->id, $course3->id), array_keys($courses));
+
+        // Make sure that implicit visibility sorting defined in navsortmycourseshiddenlast is respected for all course sortings.
+
+        $CFG->navsortmycoursessort = 'sortorder';
+        $CFG->navsortmycourseshiddenlast = true;
+        $courses = enrol_get_all_users_courses($user1->id);
+        $this->assertEquals(array($course2->id, $course1->id, $course3->id), array_keys($courses));
+
+        $CFG->navsortmycoursessort = 'sortorder';
+        $CFG->navsortmycourseshiddenlast = false;
+        $courses = enrol_get_all_users_courses($user1->id);
+        $this->assertEquals(array($course1->id, $course3->id, $course2->id), array_keys($courses));
+
+        $CFG->navsortmycoursessort = 'fullname';
+        $CFG->navsortmycourseshiddenlast = true;
+        $courses = enrol_get_all_users_courses($user1->id);
+        $this->assertEquals(array($course2->id, $course1->id, $course3->id), array_keys($courses));
+
+        $CFG->navsortmycoursessort = 'fullname';
+        $CFG->navsortmycourseshiddenlast = false;
+        $courses = enrol_get_all_users_courses($user1->id);
+        $this->assertEquals(array($course1->id, $course2->id, $course3->id), array_keys($courses));
+
+        $CFG->navsortmycoursessort = 'shortname';
+        $CFG->navsortmycourseshiddenlast = true;
+        $courses = enrol_get_all_users_courses($user1->id);
+        $this->assertEquals(array($course2->id, $course3->id, $course1->id), array_keys($courses));
+
+        $CFG->navsortmycoursessort = 'shortname';
+        $CFG->navsortmycourseshiddenlast = false;
+        $courses = enrol_get_all_users_courses($user1->id);
+        $this->assertEquals(array($course2->id, $course3->id, $course1->id), array_keys($courses));
+
+        $CFG->navsortmycoursessort = 'idnumber';
+        $CFG->navsortmycourseshiddenlast = true;
+        $courses = enrol_get_all_users_courses($user1->id);
+        $this->assertEquals(array($course2->id, $course1->id, $course3->id), array_keys($courses));
+
+        $CFG->navsortmycoursessort = 'idnumber';
+        $CFG->navsortmycourseshiddenlast = false;
+        $courses = enrol_get_all_users_courses($user1->id);
         $this->assertEquals(array($course1->id, $course3->id, $course2->id), array_keys($courses));
         $this->assertEquals(array($course1->id, $course3->id, $course2->id), array_keys($courses));
+
+        // But still the explicit visibility sorting takes precedence over the implicit one.
+
+        $courses = enrol_get_all_users_courses($user1->id, false, null, 'visible DESC, shortname DESC');
+        $this->assertEquals(array($course2->id, $course1->id, $course3->id), array_keys($courses));
     }
 
     public function test_enrol_user_sees_own_courses() {
     }
 
     public function test_enrol_user_sees_own_courses() {
index c31faad..72c40fc 100644 (file)
@@ -822,6 +822,8 @@ $string['navshowallcourses'] = 'Show all courses';
 $string['navshowcategories'] = 'Show course categories';
 $string['navshowmycoursecategories'] = 'Show my course categories';
 $string['navshowmycoursecategories_help'] = 'If enabled courses in the users my courses branch will be shown in categories.';
 $string['navshowcategories'] = 'Show course categories';
 $string['navshowmycoursecategories'] = 'Show my course categories';
 $string['navshowmycoursecategories_help'] = 'If enabled courses in the users my courses branch will be shown in categories.';
+$string['navsortmycourseshiddenlast'] = 'Sort my hidden courses last';
+$string['navsortmycourseshiddenlast_help'] = 'If enabled, any hidden courses will be listed after visible courses (for users who can view hidden courses). Otherwise, all courses, regardless of their visibility, will be listed according to the \'Sort my courses\' setting.';
 $string['navsortmycoursessort'] = 'Sort my courses';
 $string['navsortmycoursessort_help'] = 'This determines whether courses are listed under My courses according to the sort order (i.e. the order set in Site administration > Courses > Manage courses and categories) or alphabetically by course setting.';
 $string['never'] = 'Never';
 $string['navsortmycoursessort'] = 'Sort my courses';
 $string['navsortmycoursessort_help'] = 'This determines whether courses are listed under My courses according to the sort order (i.e. the order set in Site administration > Courses > Manage courses and categories) or alphabetically by course setting.';
 $string['never'] = 'Never';
index aac1cac..fbb5c39 100644 (file)
@@ -567,13 +567,8 @@ function enrol_get_my_courses($fields = null, $sort = null, $limit = 0, $coursei
     $offset = 0, $excludecourses = []) {
     global $DB, $USER, $CFG;
 
     $offset = 0, $excludecourses = []) {
     global $DB, $USER, $CFG;
 
-    if ($sort === null) {
-        if (empty($CFG->navsortmycoursessort)) {
-            $sort = 'visible DESC, sortorder ASC';
-        } else {
-            $sort = 'visible DESC, '.$CFG->navsortmycoursessort.' ASC';
-        }
-    }
+    // Re-Arrange the course sorting according to the admin settings.
+    $sort = enrol_get_courses_sortingsql($sort);
 
     // Guest account does not have any enrolled courses.
     if (!$allaccessible && (isguestuser() or !isloggedin())) {
 
     // Guest account does not have any enrolled courses.
     if (!$allaccessible && (isguestuser() or !isloggedin())) {
@@ -804,6 +799,41 @@ function enrol_get_course_info_icons($course, array $instances = NULL) {
     return $icons;
 }
 
     return $icons;
 }
 
+/**
+ * Returns SQL ORDER arguments which reflect the admin settings to sort my courses.
+ *
+ * @param string|null $sort SQL ORDER arguments which were originally requested (optionally).
+ * @return string SQL ORDER arguments.
+ */
+function enrol_get_courses_sortingsql($sort = null) {
+    global $CFG;
+
+    // Prepare the visible SQL fragment as empty.
+    $visible = '';
+    // Only create a visible SQL fragment if the caller didn't already pass a sort order which contains the visible field.
+    if ($sort === null || strpos($sort, 'visible') === false) {
+        // If the admin did not explicitly want to have shown and hidden courses sorted as one list, we will sort hidden
+        // courses to the end of the course list.
+        if (!isset($CFG->navsortmycourseshiddenlast) || $CFG->navsortmycourseshiddenlast == true) {
+            $visible = 'visible DESC, ';
+        }
+    }
+
+    // Only create a sortorder SQL fragment if the caller didn't already pass one.
+    if ($sort === null) {
+        // If the admin has configured a course sort order, we will use this.
+        if (!empty($CFG->navsortmycoursessort)) {
+            $sort = $CFG->navsortmycoursessort . ' ASC';
+
+            // Otherwise we will fall back to the sortorder sorting.
+        } else {
+            $sort = 'sortorder ASC';
+        }
+    }
+
+    return $visible . $sort;
+}
+
 /**
  * Returns course enrolment detailed information.
  *
 /**
  * Returns course enrolment detailed information.
  *
@@ -955,15 +985,10 @@ function enrol_user_sees_own_courses($user = null) {
  * @return array
  */
 function enrol_get_all_users_courses($userid, $onlyactive = false, $fields = null, $sort = null) {
  * @return array
  */
 function enrol_get_all_users_courses($userid, $onlyactive = false, $fields = null, $sort = null) {
-    global $CFG, $DB;
+    global $DB;
 
 
-    if ($sort === null) {
-        if (empty($CFG->navsortmycoursessort)) {
-            $sort = 'visible DESC, sortorder ASC';
-        } else {
-            $sort = 'visible DESC, '.$CFG->navsortmycoursessort.' ASC';
-        }
-    }
+    // Re-Arrange the course sorting according to the admin settings.
+    $sort = enrol_get_courses_sortingsql($sort);
 
     // Guest account does not have any courses
     if (isguestuser($userid) or empty($userid)) {
 
     // Guest account does not have any courses
     if (isguestuser($userid) or empty($userid)) {
index 4caf05b..d49e526 100644 (file)
@@ -29,7 +29,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
 
 defined('MOODLE_INTERNAL') || die();
 
-$version  = 2019100800.00;              // YYYYMMDD      = weekly release date of this DEV branch.
+$version  = 2019100800.01;              // YYYYMMDD      = weekly release date of this DEV branch.
                                         //         RR    = release increments - 00 in DEV branches.
                                         //           .XX = incremental changes.
 
                                         //         RR    = release increments - 00 in DEV branches.
                                         //           .XX = incremental changes.