course completion: MDL-22797 Updating to use user_enrolments instead of the
authorAaron Barnes <aaronb@catalyst.net.nz>
Mon, 19 Jul 2010 03:27:52 +0000 (03:27 +0000)
committerAaron Barnes <aaronb@catalyst.net.nz>
Mon, 19 Jul 2010 03:27:52 +0000 (03:27 +0000)
old role_assignments table to find course participants

lib/completion/completion_completion.php
lib/completion/completion_criteria_duration.php
lib/completion/cron.php

index 82903a9..2be4780 100644 (file)
@@ -138,9 +138,9 @@ class completion_completion extends data_object {
      */
     public function mark_enrolled($timeenrolled = null) {
 
      */
     public function mark_enrolled($timeenrolled = null) {
 
-        if (!$this->timeenrolled) {
+        if ($this->timeenrolled === null) {
 
 
-            if (!$timeenrolled) {
+            if ($timeenrolled === null) {
                 $timeenrolled = time();
             }
 
                 $timeenrolled = time();
             }
 
@@ -219,15 +219,8 @@ class completion_completion extends data_object {
 
         global $DB;
 
 
         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
         }
 
         // Save record
index 6ad1ba2..7a42909 100644 (file)
@@ -90,8 +90,7 @@ class completion_criteria_duration extends completion_criteria {
     private function get_timeenrolled($completion) {
         global $DB;
 
     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 @@ class completion_criteria_duration extends completion_criteria {
     public function cron() {
         global $DB;
 
     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 = '
         $sql = '
-            SELECT DISTINCT
+            SELECT
                 c.id AS course,
                 cr.timeend AS date,
                 cr.id AS criteriaid,
                 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
             FROM
-                {course_completion_criteria} cr
+                {user} u
             INNER JOIN
             INNER JOIN
-                {course} c
-             ON cr.course = c.id
+                {user_enrolments} ue
+             ON ue.userid = u.id
             INNER JOIN
             INNER JOIN
-                {context} con
-             ON con.instanceid = c.id
+                {enrol} e
+             ON e.id = ue.enrolid
             INNER JOIN
             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
             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.'
             WHERE
                 cr.criteriatype = '.COMPLETION_CRITERIA_TYPE_DURATION.'
-            AND con.contextlevel = '.CONTEXT_COURSE.'
             AND c.enablecompletion = 1
             AND cc.id IS NULL
             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
         ';
 
         // 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);
             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();
             }
 
             $rs->close();
index 9c8b2e8..a216151 100644 (file)
@@ -53,72 +53,129 @@ function completion_cron() {
 function completion_cron_mark_started() {
     global $CFG, $DB;
 
 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');
     }
 
     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 = "
     $sql = "
-        SELECT DISTINCT
+        SELECT
             c.id AS course,
             c.id AS course,
-            ra.userid AS userid,
+            u.id AS userid,
             crc.id AS completionid,
             crc.id AS completionid,
-            MIN(ra.timestart) AS timestarted
+            ue.timestart,
+            ue.timeenrolled
         FROM
         FROM
-            {course} c
+            {user} u
+        INNER JOIN
+            {user_enrolments} ue
+         ON ue.userid = u.id
         INNER JOIN
         INNER JOIN
-            {context} con
-         ON con.instanceid = c.id
+            {enrol} e
+         ON e.id = ue.enrolid
         INNER JOIN
         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
         LEFT JOIN
             {course_completions} crc
          ON crc.course = c.id
-        AND crc.userid = ra.userid
+        AND crc.userid = u.id
         WHERE
         WHERE
-            con.contextlevel = ".CONTEXT_COURSE."
-        AND c.enablecompletion = 1
-        AND c.completionstartonenrol = 1
+            c.enablecompletion = 1
         AND crc.timeenrolled IS NULL
         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
         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;
     }
 
         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();
     }
 
     $rs->close();
@@ -175,7 +232,7 @@ function completion_cron_completions() {
         SELECT DISTINCT
             c.id AS course,
             cr.id AS criteriaid,
         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
             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
             {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
         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
         WHERE
-            con.contextlevel = '.CONTEXT_COURSE.'
-        AND c.enablecompletion = 1
+            c.enablecompletion = 1
         AND crc.timecompleted IS NULL
         AND crc.reaggregate > 0
         ORDER BY
         AND crc.timecompleted IS NULL
         AND crc.reaggregate > 0
         ORDER BY