Skip to content

Commit

Permalink
Merge branch 'wip-MDL-38596-master' of git://github.com/marinaglancy/…
Browse files Browse the repository at this point in the history 
…moodle
  • Loading branch information
Damyon Wiese committed May 7, 2013
2 parents e4e8d57 + 36ba8fd commit fc07589
Show file tree
Hide file tree
Showing 4 changed files with 221 additions and 13 deletions.
1 change: 1 addition & 0 deletions lang/en/cache.php
View file
Expand Up @@ -40,6 +40,7 @@
$string['cachedef_config'] = 'Config settings';
$string['cachedef_coursecat'] = 'Course categories lists for particular user';
$string['cachedef_coursecatrecords'] = 'Course categories records';
$string['cachedef_coursecontacts'] = 'List of course contacts';
$string['cachedef_coursecattree'] = 'Course categories tree';
$string['cachedef_databasemeta'] = 'Database meta information';
$string['cachedef_eventinvalidation'] = 'Event invalidation';
Expand Down
121 changes: 108 additions & 13 deletions lib/coursecatlib.php
View file
Expand Up @@ -37,6 +37,8 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate {
/** @var coursecat stores pseudo category with id=0. Use coursecat::get(0) to retrieve */
protected static $coursecat0;

const CACHE_COURSE_CONTACTS_TTL = 3600; // do not fetch course contacts more often than once per hour

/** @var array list of all fields and their short name and default value for caching */
protected static $coursecatfields = array(
'id' => array('id', 0),
Expand Down Expand Up @@ -668,13 +670,33 @@ public static function preload_course_contacts(&$courses) {
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
$cache = cache::make('core', 'coursecontacts');
$cacheddata = $cache->get_many(array_merge(array('basic'), array_keys($courses)));
// check if cache was set for the current course contacts and it is not yet expired
if (empty($cacheddata['basic']) || $cacheddata['basic']['roles'] !== $CFG->coursecontact ||
$cacheddata['basic']['lastreset'] < time() - self::CACHE_COURSE_CONTACTS_TTL) {
// reset cache
$cache->purge();
$cache->set('basic', array('roles' => $CFG->coursecontact, 'lastreset' => time()));
$cacheddata = $cache->get_many(array_merge(array('basic'), array_keys($courses)));
}
$courseids = array();
foreach (array_keys($courses) as $id) {
if ($cacheddata[$id] !== false) {
$courses[$id]->managers = $cacheddata[$id];
} else {
$courseids[] = $id;
}
}

// $courseids now stores list of ids of courses for which we still need to retrieve contacts
if (empty($courseids)) {
return;
}

// 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) {
Expand All @@ -685,6 +707,7 @@ public static function preload_course_contacts(&$courses) {
}
}

// 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');
Expand All @@ -698,21 +721,93 @@ public static function preload_course_contacts(&$courses) {
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]);
}
}
}
}

// set the cache
$values = array();
foreach ($courseids as $id) {
$values[$id] = $courses[$id]->managers;
}
$cache->set_many($values);
}

/**
* 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;
}

/**
Expand Down Expand Up @@ -2222,4 +2317,4 @@ public function sort_by_many_fields($a, $b) {
}
return 0;
}
}
}
6 changes: 6 additions & 0 deletions lib/db/caches.php
View file
Expand Up @@ -224,6 +224,12 @@
'changesincoursecat',
),
),
// Cache course contacts for the courses
'coursecontacts' => array(
'mode' => cache_store::MODE_APPLICATION,
'persistent' => true,
'simplekeys' => true,
),
// Used to store data for repositories to avoid repetitive DB queries within one request
'repositories' => array(
'mode' => cache_store::MODE_REQUEST,
Expand Down
106 changes: 106 additions & 0 deletions lib/tests/coursecatlib_test.php
View file
Expand Up @@ -426,4 +426,110 @@ public function test_get_search_courses() {
$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;
}
}

0 comments on commit fc07589

Please sign in to comment.