L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-38596 Optimise SQL in preloading course contacts for number of courses

Browse Source
This commit is contained in:
Marina Glancy
2013-05-06 14:15:22 +10:00
parent 5db8f5a8f4
commit 31b0530aa6
2 changed files with 184 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lib/coursecatlib.php
+78 -12
View File
@@ -668,13 +668,13 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate {
return;
}
$managerroles = explode(',', $CFG->coursecontact);
/*
// TODO MDL-38596, this commented code is similar to get_courses_wmanagers()
// It bulk-preloads course contacts for all courses BUT it does not check enrolments
// List of ids of courses for which we need to retrieve contacts
$courseids = array_keys($courses);
// first build the array of all context ids of the courses and their categories
$allcontexts = array();
foreach (array_keys($courses) as $id) {
foreach ($courseids as $id) {
$context = context_course::instance($id);
$courses[$id]->managers = array();
foreach (preg_split('|/|', $context->path, 0, PREG_SPLIT_NO_EMPTY) as $ctxid) {
@@ -685,6 +685,7 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate {
}
}
// fetch list of all users with course contact roles in any of the courses contexts or parent contexts
list($sql1, $params1) = $DB->get_in_or_equal(array_keys($allcontexts), SQL_PARAMS_NAMED, 'ctxid');
list($sql2, $params2) = $DB->get_in_or_equal($managerroles, SQL_PARAMS_NAMED, 'rid');
list($sort, $sortparams) = users_order_by_sql('u');
@@ -698,23 +699,88 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate {
WHERE ra.contextid ". $sql1." AND ra.roleid ". $sql2."
ORDER BY r.sortorder, $sort";
$rs = $DB->get_recordset_sql($sql, $params1 + $params2 + $sortparams);
$checkenrolments = array();
foreach($rs as $ra) {
foreach ($allcontexts[$ra->contextid] as $id) {
$courses[$id]->managers[$ra->raid] = $ra;
if (!isset($checkenrolments[$id])) {
$checkenrolments[$id] = array();
}
$checkenrolments[$id][] = $ra->id;
}
}
$rs->close();
*/
list($sort, $sortparams) = users_order_by_sql('u');
foreach (array_keys($courses) as $id) {
$context = context_course::instance($id);
$courses[$id]->managers = get_role_users($managerroles, $context, true,
'ra.id AS raid, u.id, u.username, u.firstname, u.lastname, rn.name AS rolecoursealias,
r.name AS rolename, r.sortorder, r.id AS roleid, r.shortname AS roleshortname',
'r.sortorder ASC, ' . $sort, false, '', '', '', '', $sortparams);
// remove from course contacts users who are not enrolled in the course
$enrolleduserids = self::ensure_users_enrolled($checkenrolments);
foreach ($checkenrolments as $id => $userids) {
if (empty($enrolleduserids[$id])) {
$courses[$id]->managers = array();
} else if ($notenrolled = array_diff($userids, $enrolleduserids[$id])) {
foreach ($courses[$id]->managers as $raid => $ra) {
if (in_array($ra->id, $notenrolled)) {
unset($courses[$id]->managers[$raid]);
}
}
}
}
}
/**
* Verify user enrollments for multiple course-user combinations
*
* @param array $courseusers array where keys are course ids and values are array
* of users in this course whose enrolment we wish to verify
* @return array same structure as input array but values list only users from input
* who are enrolled in the course
*/
protected static function ensure_users_enrolled($courseusers) {
global $DB;
// If the input array is too big, split it into chunks
$maxcoursesinquery = 20;
if (count($courseusers) > $maxcoursesinquery) {
$rv = array();
for ($offset = 0; $offset < count($courseusers); $offset += $maxcoursesinquery) {
$chunk = array_slice($courseusers, $offset, $maxcoursesinquery, true);
$rv = $rv + self::ensure_users_enrolled($chunk);
}
return $rv;
}
// create a query verifying valid user enrolments for the number of courses
$sql = "SELECT DISTINCT e.courseid, ue.userid
FROM {user_enrolments} ue
JOIN {enrol} e ON e.id = ue.enrolid
WHERE ue.status = :active
AND e.status = :enabled
AND ue.timestart < :now1 AND (ue.timeend = 0 OR ue.timeend > :now2)";
$now = round(time(), -2); // rounding helps caching in DB
$params = array('enabled' => ENROL_INSTANCE_ENABLED,
'active' => ENROL_USER_ACTIVE,
'now1' => $now, 'now2' => $now);
$cnt = 0;
$subsqls = array();
$enrolled = array();
foreach ($courseusers as $id => $userids) {
$enrolled[$id] = array();
if (count($userids)) {
list($sql2, $params2) = $DB->get_in_or_equal($userids, SQL_PARAMS_NAMED, 'userid'.$cnt.'_');
$subsqls[] = "(e.courseid = :courseid$cnt AND ue.userid ".$sql2.")";
$params = $params + array('courseid'.$cnt => $id) + $params2;
$cnt++;
}
}
if (count($subsqls)) {
$sql .= "AND (". join(' OR ', $subsqls).")";
$rs = $DB->get_recordset_sql($sql, $params);
foreach ($rs as $record) {
$enrolled[$record->courseid][] = $record->userid;
}
$rs->close();
}
return $enrolled;
}
/**
* Retrieves number of records from course table
*
lib/tests/coursecatlib_test.php
+106
View File
@@ -426,4 +426,110 @@ class coursecatlib_testcase extends advanced_testcase {
$this->assertEquals(array($c3->id, $c6->id), array_keys($res));
$this->assertEquals(2, coursecat::search_courses_count(array('search' => 'Математика'), array()));
}
public function test_course_contacts() {
global $DB, $CFG;
$teacherrole = $DB->get_record('role', array('shortname'=>'editingteacher'));
$managerrole = $DB->get_record('role', array('shortname'=>'manager'));
$studentrole = $DB->get_record('role', array('shortname'=>'student'));
$oldcoursecontact = $CFG->coursecontact;
$CFG->coursecontact = $managerrole->id. ','. $teacherrole->id;
/**
* User is listed in course contacts for the course if he has one of the
* "course contact" roles ($CFG->coursecontact) AND is enrolled in the course.
* If the user has several roles only the highest is displayed.
*/
// Test case:
//
// == Cat1 (user2 has teacher role)
// == Cat2
// -- course21 (user2 is enrolled as manager) | [Expected] Manager: F2 L2
// -- course22 (user2 is enrolled as student) | [Expected] Teacher: F2 L2
// == Cat4 (user2 has manager role)
// -- course41 (user4 is enrolled as teacher, user5 is enrolled as manager) | [Expected] Manager: F5 L5, Teacher: F4 L4
// -- course42 (user2 is enrolled as teacher) | [Expected] Manager: F2 L2
// == Cat3 (user3 has manager role)
// -- course31 (user3 is enrolled as student) | [Expected] Manager: F3 L3
// -- course32 | [Expected]
// -- course11 (user1 is enrolled as teacher) | [Expected] Teacher: F1 L1
// -- course12 (user1 has teacher role) | [Expected]
// also user4 is enrolled as teacher but enrolment is not active
$category = $course = $enrol = $user = array();
$category[1] = coursecat::create(array('name' => 'Cat1'))->id;
$category[2] = coursecat::create(array('name' => 'Cat2', 'parent' => $category[1]))->id;
$category[3] = coursecat::create(array('name' => 'Cat3', 'parent' => $category[1]))->id;
$category[4] = coursecat::create(array('name' => 'Cat4', 'parent' => $category[2]))->id;
foreach (array(1,2,3,4) as $catid) {
foreach (array(1,2) as $courseid) {
$course[$catid][$courseid] = $this->getDataGenerator()->create_course(array('idnumber' => 'id'.$catid.$courseid,
'category' => $category[$catid]))->id;
$enrol[$catid][$courseid] = $DB->get_record('enrol', array('courseid'=>$course[$catid][$courseid], 'enrol'=>'manual'), '*', MUST_EXIST);
}
}
foreach (array(1,2,3,4,5) as $userid) {
$user[$userid] = $this->getDataGenerator()->create_user(array('firstname' => 'F'.$userid, 'lastname' => 'L'.$userid))->id;
}
$manual = enrol_get_plugin('manual');
// Cat1 (user2 has teacher role)
role_assign($teacherrole->id, $user[2], context_coursecat::instance($category[1]));
// course21 (user2 is enrolled as manager)
$manual->enrol_user($enrol[2][1], $user[2], $managerrole->id);
// course22 (user2 is enrolled as student)
$manual->enrol_user($enrol[2][2], $user[2], $studentrole->id);
// Cat4 (user2 has manager role)
role_assign($managerrole->id, $user[2], context_coursecat::instance($category[4]));
// course41 (user4 is enrolled as teacher, user5 is enrolled as manager)
$manual->enrol_user($enrol[4][1], $user[4], $teacherrole->id);
$manual->enrol_user($enrol[4][1], $user[5], $managerrole->id);
// course42 (user2 is enrolled as teacher)
$manual->enrol_user($enrol[4][2], $user[2], $teacherrole->id);
// Cat3 (user3 has manager role)
role_assign($managerrole->id, $user[3], context_coursecat::instance($category[3]));
// course31 (user3 is enrolled as student)
$manual->enrol_user($enrol[3][1], $user[3], $studentrole->id);
// course11 (user1 is enrolled as teacher)
$manual->enrol_user($enrol[1][1], $user[1], $teacherrole->id);
// -- course12 (user1 has teacher role)
// also user4 is enrolled as teacher but enrolment is not active
role_assign($teacherrole->id, $user[1], context_course::instance($course[1][2]));
$manual->enrol_user($enrol[1][2], $user[4], $teacherrole->id, 0, 0, ENROL_USER_SUSPENDED);
$allcourses = coursecat::get(0)->get_courses(array('recursive' => true, 'coursecontacts' => true, 'sort' => array('idnumber' => 1)));
// Simplify the list of contacts for each course (similar as renderer would do)
$contacts = array();
foreach (array(1,2,3,4) as $catid) {
foreach (array(1,2) as $courseid) {
$tmp = array();
foreach ($allcourses[$course[$catid][$courseid]]->get_course_contacts() as $contact) {
$tmp[] = $contact['rolename']. ': '. $contact['username'];
}
$contacts[$catid][$courseid] = join(', ', $tmp);
}
}
// Assert:
// -- course21 (user2 is enrolled as manager) | Manager: F2 L2
$this->assertEquals('Manager: F2 L2', $contacts[2][1]);
// -- course22 (user2 is enrolled as student) | Teacher: F2 L2
$this->assertEquals('Teacher: F2 L2', $contacts[2][2]);
// -- course41 (user4 is enrolled as teacher, user5 is enrolled as manager) | Manager: F5 L5, Teacher: F4 L4
$this->assertEquals('Manager: F5 L5, Teacher: F4 L4', $contacts[4][1]);
// -- course42 (user2 is enrolled as teacher) | [Expected] Manager: F2 L2
$this->assertEquals('Manager: F2 L2', $contacts[4][2]);
// -- course31 (user3 is enrolled as student) | Manager: F3 L3
$this->assertEquals('Manager: F3 L3', $contacts[3][1]);
// -- course32 |
$this->assertEquals('', $contacts[3][2]);
// -- course11 (user1 is enrolled as teacher) | Teacher: F1 L1
$this->assertEquals('Teacher: F1 L1', $contacts[1][1]);
// -- course12 (user1 has teacher role) |
$this->assertEquals('', $contacts[1][2]);
$CFG->coursecontact = $oldcoursecontact;
}
}