Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

course completion: MDL-22797 Updating to use user_enrolments instead …

…of the

old role_assignments table to find course participants
  • Loading branch information...
commit 89482538447f61b4ddbeaa399ef355577027dc96 1 parent 2481a04
Aaron Barnes srynot4sale authored
15 lib/completion/completion_completion.php
View
@@ -138,9 +138,9 @@ public function is_complete() {
*/
public function mark_enrolled($timeenrolled = null) {
- if (!$this->timeenrolled) {
+ if ($this->timeenrolled === null) {
- if (!$timeenrolled) {
+ if ($timeenrolled === null) {
$timeenrolled = time();
}
@@ -219,15 +219,8 @@ private function _save() {
global $DB;
- if (!$this->timeenrolled) {
- // Get users timenrolled
- // Can't find a more efficient way of doing this without alter get_users_by_capability()
- $context = get_context_instance(CONTEXT_COURSE, $this->course);
- if ($roleassignment = $DB->get_record('role_assignments', array('contextid' => $context->id, 'userid' => $this->userid))) {
- $this->timeenrolled = $roleassignment->timestart;
- } else {
- $this->timeenrolled = 0;
- }
+ if ($this->timeenrolled === null) {
+ $this->timeenrolled = 0;
}
// Save record
61 lib/completion/completion_criteria_duration.php
View
@@ -90,8 +90,7 @@ public function update_config(&$data) {
private function get_timeenrolled($completion) {
global $DB;
- $context = get_context_instance(CONTEXT_COURSE, $this->course);
- return $DB->get_field('role_assignments', 'timestart', array('contextid' => $context->id, 'userid' => $completion->userid));
+ return $DB->get_field('user_enrolments', 'timestart', array('courseid' => $this->course, 'userid' => $completion->userid));
}
/**
@@ -171,45 +170,67 @@ public function get_status($completion) {
public function cron() {
global $DB;
-//TODO: MDL-22797 completion needs to be updated to use new enrolment framework
-
- // Get all users who match meet this criteria
+ /*
+ * Get all users who match meet this criteria
+ *
+ * We can safely ignore duplicate enrolments for
+ * a user in a course here as we only care if
+ * one of the enrolments has passed the set
+ * duration.
+ */
$sql = '
- SELECT DISTINCT
+ SELECT
c.id AS course,
cr.timeend AS date,
cr.id AS criteriaid,
- ra.userid AS userid,
- (ra.timestart + cr.enrolperiod) AS timecompleted
+ u.id AS userid,
+ ue.timestart AS otimestart,
+ (ue.timestart + cr.enrolperiod) AS ctimestart,
+ ue.timeenrolled AS otimeenrolled,
+ (ue.timeenrolled + cr.enrolperiod) AS ctimeenrolled
FROM
- {course_completion_criteria} cr
+ {user} u
INNER JOIN
- {course} c
- ON cr.course = c.id
+ {user_enrolments} ue
+ ON ue.userid = u.id
INNER JOIN
- {context} con
- ON con.instanceid = c.id
+ {enrol} e
+ ON e.id = ue.enrolid
INNER JOIN
- {role_assignments} ra
- ON ra.contextid = con.id
+ {course} c
+ ON c.id = e.courseid
+ INNER JOIN
+ {course_completion_criteria} cr
+ ON c.id = cr.course
LEFT JOIN
{course_completion_crit_compl} cc
ON cc.criteriaid = cr.id
- AND cc.userid = ra.userid
+ AND cc.userid = u.id
WHERE
cr.criteriatype = '.COMPLETION_CRITERIA_TYPE_DURATION.'
- AND con.contextlevel = '.CONTEXT_COURSE.'
AND c.enablecompletion = 1
AND cc.id IS NULL
- AND ra.timestart + cr.enrolperiod < ?
+ AND
+ (
+ ue.timestart > 0 AND ue.timestart + cr.enrolperiod < ?
+ OR ue.timeenrolled > 0 AND ue.timeenrolled + cr.enrolperiod < ?
+ )
';
// Loop through completions, and mark as complete
- if ($rs = $DB->get_recordset_sql($sql, array(time()))) {
+ $now = time();
+ if ($rs = $DB->get_recordset_sql($sql, array($now, $now))) {
foreach ($rs as $record) {
$completion = new completion_criteria_completion((array)$record);
- $completion->mark_complete($record->timecompleted);
+
+ // Use time start if not 0, otherwise use timeenrolled
+ if ($record->otimestart) {
+ $completion->mark_complete($record->ctimestart);
+ }
+ else {
+ $completion->mark_complete($record->ctimeenrolled);
+ }
}
$rs->close();
151 lib/completion/cron.php
View
@@ -53,72 +53,129 @@ function completion_cron() {
function completion_cron_mark_started() {
global $CFG, $DB;
-//TODO: MDL-22797 completion needs to be updated to use new enrolment framework
-
if (debugging()) {
mtrace('Marking users as started');
}
- $roles = '';
- if (!empty($CFG->progresstrackedroles)) {
- $roles = 'AND ra.roleid IN ('.$CFG->progresstrackedroles.')';
- }
-
+ /**
+ * A quick explaination of this horrible looking query
+ *
+ * It's purpose is to locate all the active participants
+ * of a course with course completion enabled.
+ *
+ * We also only want the users with no course_completions
+ * record as this functions job is to create the missing
+ * ones :)
+ *
+ * We want to record the user's enrolment start time for the
+ * course. This gets tricky because there can be multiple
+ * enrolment plugins active in a course, hence the possibility
+ * of multiple records for each couse/user in the results
+ */
$sql = "
- SELECT DISTINCT
+ SELECT
c.id AS course,
- ra.userid AS userid,
+ u.id AS userid,
crc.id AS completionid,
- MIN(ra.timestart) AS timestarted
+ ue.timestart,
+ ue.timeenrolled
FROM
- {course} c
+ {user} u
+ INNER JOIN
+ {user_enrolments} ue
+ ON ue.userid = u.id
INNER JOIN
- {context} con
- ON con.instanceid = c.id
+ {enrol} e
+ ON e.id = ue.enrolid
INNER JOIN
- {role_assignments} ra
- ON ra.contextid = con.id
+ {course} c
+ ON c.id = e.courseid
LEFT JOIN
{course_completions} crc
ON crc.course = c.id
- AND crc.userid = ra.userid
+ AND crc.userid = u.id
WHERE
- con.contextlevel = ".CONTEXT_COURSE."
- AND c.enablecompletion = 1
- AND c.completionstartonenrol = 1
+ c.enablecompletion = 1
AND crc.timeenrolled IS NULL
- AND (ra.timeend IS NULL OR ra.timeend > ".time().")
- {$roles}
- GROUP BY
- c.id,
- ra.userid,
- crc.id
+ AND ue.status = 0
+ AND e.status = 0
+ AND u.deleted = 0
+ AND ue.timestart < ?
+ AND (ue.timeend > ? OR ue.timeend = 0)
+ AND ue.timeenrolled < ?
+ AND (ue.timeenrolled > ? OR ue.timeenrolled = 0)
ORDER BY
course,
userid
";
// Check if result is empty
- if (!$rs = $DB->get_recordset_sql($sql)) {
+ $now = time();
+ if (!$rs = $DB->get_recordset_sql($sql, array($now, $now, $now, $now))) {
return;
}
- // Grab records for current user/course
- foreach ($rs as $record) {
- $completion = new completion_completion();
- $completion->userid = $record->userid;
- $completion->course = $record->course;
- $completion->timeenrolled = $record->timestarted;
-
- if ($record->completionid) {
- $completion->id = $record->completionid;
+ /**
+ * An explaination of the following loop
+ *
+ * We are essentially doing a group by in the code here (as I can't find
+ * a decent way of doing it in the sql).
+ *
+ * Since there can be multiple enrolment plugins for each course, we can have
+ * multiple rows for each particpant in the query result. This isn't really
+ * a problem until you combine it with the fact that the enrolment plugins
+ * can save the enrol start time in either timestart or timeenrolled.
+ *
+ * The purpose of this loop is to find the earliest enrolment start time for
+ * each participant in each course.
+ */
+ $prev = null;
+ while ($rs->valid() || $prev) {
+
+ $current = $rs->current();
+
+ if (!isset($current->course)) {
+ $current = false;
+ }
+ else {
+ // Not all enrol plugins fill out timestart correctly, so use whichever
+ // is non-zero
+ $current->timeenrolled = max($current->timestart, $current->timeenrolled);
}
- $completion->mark_enrolled();
+ // If we are at the last record,
+ // or we aren't at the first and the record is for a diff user/course
+ if ($prev &&
+ (!$rs->valid() ||
+ ($current->course != $prev->course || $current->userid != $prev->userid))) {
+
+ $completion = new completion_completion();
+ $completion->userid = $prev->userid;
+ $completion->course = $prev->course;
+ $completion->timeenrolled = (string) $prev->timeenrolled;
+ $completion->timestarted = 0;
+
+ if ($prev->completionid) {
+ $completion->id = $prev->completionid;
+ }
- if (debugging()) {
- mtrace('Marked started user '.$record->userid.' in course '.$record->course);
+ $completion->mark_enrolled();
+
+ if (debugging()) {
+ mtrace('Marked started user '.$prev->userid.' in course '.$prev->course);
+ }
}
+ // Else, if this record is for the same user/course
+ elseif ($prev && $current) {
+ // Use oldest timeenrolled
+ $current->timeenrolled = min($current->timeenrolled, $prev->timeenrolled);
+ }
+
+ // Move current record to previous
+ $prev = $current;
+
+ // Move to next record
+ $rs->next();
}
$rs->close();
@@ -175,7 +232,7 @@ function completion_cron_completions() {
SELECT DISTINCT
c.id AS course,
cr.id AS criteriaid,
- ra.userid AS userid,
+ cc.userid AS userid,
cr.criteriatype AS criteriatype,
cc.timecompleted AS timecompleted
FROM
@@ -184,22 +241,14 @@ function completion_cron_completions() {
{course} c
ON cr.course = c.id
INNER JOIN
- {context} con
- ON con.instanceid = c.id
- INNER JOIN
- {role_assignments} ra
- ON ra.contextid = con.id
+ {course_completions} crc
+ ON crc.course = c.id
LEFT JOIN
{course_completion_crit_compl} cc
ON cc.criteriaid = cr.id
- AND cc.userid = ra.userid
- LEFT JOIN
- {course_completions} crc
- ON crc.course = c.id
- AND crc.userid = ra.userid
+ AND crc.userid = cc.userid
WHERE
- con.contextlevel = '.CONTEXT_COURSE.'
- AND c.enablecompletion = 1
+ c.enablecompletion = 1
AND crc.timecompleted IS NULL
AND crc.reaggregate > 0
ORDER BY
Please sign in to comment.
Something went wrong with that request. Please try again.