Skip to content

Commit

Permalink
Merge branch 'MDL-53169-master' of https://github.com/sammarshallou/m…
Browse files Browse the repository at this point in the history 
…oodle
  • Loading branch information
@junpataleta
junpataleta committed Sep 19, 2017
2 parents 6bfd419 + 22ef155 commit 7326663
Show file tree
Hide file tree
Showing 2 changed files with 191 additions and 16 deletions.
106 changes: 106 additions & 0 deletions enrol/tests/enrollib_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,112 @@ public function test_enrol_get_my_courses_only_enrolled_courses() {
$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
*
Expand Down
101 changes: 85 additions & 16 deletions lib/enrollib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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);

Expand Down

0 comments on commit 7326663

Please sign in to comment.