From 22ef1550873a0f1d992cf0c03255e809e5e1d146 Mon Sep 17 00:00:00 2001 From: sam marshall Date: Mon, 21 Aug 2017 17:15:30 +0100 Subject: [PATCH] MDL-53169 Lib: enrol_get_my_courses can list available courses too --- enrol/tests/enrollib_test.php | 106 ++++++++++++++++++++++++++++++++++ lib/enrollib.php | 101 +++++++++++++++++++++++++++----- 2 files changed, 191 insertions(+), 16 deletions(-) diff --git a/enrol/tests/enrollib_test.php b/enrol/tests/enrollib_test.php index 4508ae051d4..d52dc6ff07a 100644 --- a/enrol/tests/enrollib_test.php +++ b/enrol/tests/enrollib_test.php @@ -578,6 +578,112 @@ class core_enrollib_testcase extends advanced_testcase { $this->assertEquals($course2->id, $courses[$course2->id]->id); } + /** + * Tests the enrol_get_my_courses function when using the $allaccessible parameter, which + * includes a wider range of courses (enrolled courses + other accessible ones). + */ + public function test_enrol_get_my_courses_all_accessible() { + global $DB, $CFG; + + $this->resetAfterTest(true); + + // Create test user and 4 courses, two of which have guest access enabled. + $user = $this->getDataGenerator()->create_user(); + $course1 = $this->getDataGenerator()->create_course( + (object)array('shortname' => 'Z', + 'enrol_guest_status_0' => ENROL_INSTANCE_DISABLED, + 'enrol_guest_password_0' => '')); + $course2 = $this->getDataGenerator()->create_course( + (object)array('shortname' => 'Y', + 'enrol_guest_status_0' => ENROL_INSTANCE_ENABLED, + 'enrol_guest_password_0' => '')); + $course3 = $this->getDataGenerator()->create_course( + (object)array('shortname' => 'X', + 'enrol_guest_status_0' => ENROL_INSTANCE_ENABLED, + 'enrol_guest_password_0' => 'frog')); + $course4 = $this->getDataGenerator()->create_course( + (object)array('shortname' => 'W', + 'enrol_guest_status_0' => ENROL_INSTANCE_DISABLED, + 'enrol_guest_password_0' => '')); + + // User is enrolled in first course. + $this->getDataGenerator()->enrol_user($user->id, $course1->id); + + // Check enrol_get_my_courses basic use (without all accessible). + $this->setUser($user); + $courses = enrol_get_my_courses(); + $this->assertEquals([$course1->id], array_keys($courses)); + + // Turn on all accessible, now they can access the second course too. + $courses = enrol_get_my_courses(null, 'id', 0, [], true); + $this->assertEquals([$course1->id, $course2->id], array_keys($courses)); + + // Log in as guest to third course. + load_temp_course_role(context_course::instance($course3->id), $CFG->guestroleid); + $courses = enrol_get_my_courses(null, 'id', 0, [], true); + $this->assertEquals([$course1->id, $course2->id, $course3->id], array_keys($courses)); + + // Check fields parameter still works. Fields default (certain base fields). + $this->assertObjectHasAttribute('id', $courses[$course3->id]); + $this->assertObjectHasAttribute('shortname', $courses[$course3->id]); + $this->assertObjectNotHasAttribute('summary', $courses[$course3->id]); + + // Specified fields (one, string). + $courses = enrol_get_my_courses('summary', 'id', 0, [], true); + $this->assertObjectHasAttribute('id', $courses[$course3->id]); + $this->assertObjectHasAttribute('shortname', $courses[$course3->id]); + $this->assertObjectHasAttribute('summary', $courses[$course3->id]); + $this->assertObjectNotHasAttribute('summaryformat', $courses[$course3->id]); + + // Specified fields (two, string). + $courses = enrol_get_my_courses('summary, summaryformat', 'id', 0, [], true); + $this->assertObjectHasAttribute('summary', $courses[$course3->id]); + $this->assertObjectHasAttribute('summaryformat', $courses[$course3->id]); + + // Specified fields (two, array). + $courses = enrol_get_my_courses(['summary', 'summaryformat'], 'id', 0, [], true); + $this->assertObjectHasAttribute('summary', $courses[$course3->id]); + $this->assertObjectHasAttribute('summaryformat', $courses[$course3->id]); + + // Check sort parameter still works. + $courses = enrol_get_my_courses(null, 'shortname', 0, [], true); + $this->assertEquals([$course3->id, $course2->id, $course1->id], array_keys($courses)); + + // Check filter parameter still works. + $courses = enrol_get_my_courses(null, 'id', 0, [$course2->id, $course3->id, $course4->id], true); + $this->assertEquals([$course2->id, $course3->id], array_keys($courses)); + + // Check limit parameter. + $courses = enrol_get_my_courses(null, 'id', 2, [], true); + $this->assertEquals([$course1->id, $course2->id], array_keys($courses)); + + // Now try access for a different user who has manager role at system level. + $manager = $this->getDataGenerator()->create_user(); + $managerroleid = $DB->get_field('role', 'id', ['shortname' => 'manager']); + role_assign($managerroleid, $manager->id, \context_system::instance()->id); + $this->setUser($manager); + + // With default get enrolled, they don't have any courses. + $courses = enrol_get_my_courses(); + $this->assertCount(0, $courses); + + // But with all accessible, they have 4 because they have moodle/course:view everywhere. + $courses = enrol_get_my_courses(null, 'id', 0, [], true); + $this->assertEquals([$course1->id, $course2->id, $course3->id, $course4->id], + array_keys($courses)); + + // If we prohibit manager from course:view on course 1 though... + assign_capability('moodle/course:view', CAP_PROHIBIT, $managerroleid, + \context_course::instance($course1->id)); + $courses = enrol_get_my_courses(null, 'id', 0, [], true); + $this->assertEquals([$course2->id, $course3->id, $course4->id], array_keys($courses)); + + // Check for admin user, which has a slightly different query. + $this->setAdminUser(); + $courses = enrol_get_my_courses(null, 'id', 0, [], true); + $this->assertEquals([$course1->id, $course2->id, $course3->id, $course4->id], array_keys($courses)); + } + /** * test_course_users * diff --git a/lib/enrollib.php b/lib/enrollib.php index a4980394c49..e23eb1adaa2 100644 --- a/lib/enrollib.php +++ b/lib/enrollib.php @@ -548,19 +548,25 @@ function enrol_add_course_navigation(navigation_node $coursenode, $course) { * so name the fields you really need, which will * be added and uniq'd * + * If $allaccessible is true, this will additionally return courses that the current user is not + * enrolled in, but can access because they are open to the user for other reasons (course view + * permission, currently viewing course as a guest, or course allows guest access without + * password). + * * @param string|array $fields * @param string $sort * @param int $limit max number of courses * @param array $courseids the list of course ids to filter by + * @param bool $allaccessible Include courses user is not enrolled in, but can access * @return array */ function enrol_get_my_courses($fields = null, $sort = 'visible DESC,sortorder ASC', - $limit = 0, $courseids = []) { - global $DB, $USER; + $limit = 0, $courseids = [], $allaccessible = false) { + global $DB, $USER, $CFG; - // Guest account does not have any courses - if (isguestuser() or !isloggedin()) { - return(array()); + // Guest account does not have any enrolled courses. + if (!$allaccessible && (isguestuser() or !isloggedin())) { + return array(); } $basefields = array('id', 'category', 'sortorder', @@ -621,22 +627,85 @@ function enrol_get_my_courses($fields = null, $sort = 'visible DESC,sortorder AS $params = array_merge($params, $courseidsparams); } - //note: we can not use DISTINCT + text fields due to Oracle and MS limitations, that is why we have the subselect there + $courseidsql = ""; + // Logged-in, non-guest users get their enrolled courses. + if (!isguestuser() && isloggedin()) { + $courseidsql .= " + SELECT DISTINCT e.courseid + FROM {enrol} e + JOIN {user_enrolments} ue ON (ue.enrolid = e.id AND ue.userid = :userid) + WHERE ue.status = :active AND e.status = :enabled AND ue.timestart < :now1 + AND (ue.timeend = 0 OR ue.timeend > :now2)"; + $params['userid'] = $USER->id; + $params['active'] = ENROL_USER_ACTIVE; + $params['enabled'] = ENROL_INSTANCE_ENABLED; + $params['now1'] = round(time(), -2); // Improves db caching. + $params['now2'] = $params['now1']; + } + + // When including non-enrolled but accessible courses... + if ($allaccessible) { + if (is_siteadmin()) { + // Site admins can access all courses. + $courseidsql = "SELECT DISTINCT c2.id AS courseid FROM {course} c2"; + } else { + // If we used the enrolment as well, then this will be UNIONed. + if ($courseidsql) { + $courseidsql .= " UNION "; + } + + // Include courses with guest access and no password. + $courseidsql .= " + SELECT DISTINCT e.courseid + FROM {enrol} e + WHERE e.enrol = 'guest' AND e.password = '' AND e.status = :enabled2"; + $params['enabled2'] = ENROL_INSTANCE_ENABLED; + + // Include courses where the current user is currently using guest access (may include + // those which require a password). + $courseids = []; + $accessdata = get_user_accessdata($USER->id); + foreach ($accessdata['ra'] as $contextpath => $roles) { + if (array_key_exists($CFG->guestroleid, $roles)) { + // Work out the course id from context path. + $context = context::instance_by_id(preg_replace('~^.*/~', '', $contextpath)); + if ($context instanceof context_course) { + $courseids[$context->instanceid] = true; + } + } + } + + // Include courses where the current user has moodle/course:view capability. + $courses = get_user_capability_course('moodle/course:view', null, false); + if (!$courses) { + $courses = []; + } + foreach ($courses as $course) { + $courseids[$course->id] = true; + } + + // If there are any in either category, list them individually. + if ($courseids) { + list ($allowedsql, $allowedparams) = $DB->get_in_or_equal( + array_keys($courseids), SQL_PARAMS_NAMED); + $courseidsql .= " + UNION + SELECT DISTINCT c3.id AS courseid + FROM {course} c3 + WHERE c3.id $allowedsql"; + $params = array_merge($params, $allowedparams); + } + } + } + + // Note: we can not use DISTINCT + text fields due to Oracle and MS limitations, that is why + // we have the subselect there. $sql = "SELECT $coursefields $ccselect FROM {course} c - JOIN (SELECT DISTINCT e.courseid - FROM {enrol} e - JOIN {user_enrolments} ue ON (ue.enrolid = e.id AND ue.userid = :userid) - WHERE ue.status = :active AND e.status = :enabled AND ue.timestart < :now1 AND (ue.timeend = 0 OR ue.timeend > :now2) - ) en ON (en.courseid = c.id) + JOIN ($courseidsql) en ON (en.courseid = c.id) $ccjoin WHERE $wheres $orderby"; - $params['userid'] = $USER->id; - $params['active'] = ENROL_USER_ACTIVE; - $params['enabled'] = ENROL_INSTANCE_ENABLED; - $params['now1'] = round(time(), -2); // improves db caching - $params['now2'] = $params['now1']; $courses = $DB->get_records_sql($sql, $params, 0, $limit);