MDL-65276 core: fix CiBoT complains

This commit is contained in:
Simey Lameze
2019-04-08 10:30:09 +08:00
parent 97fddc26f4
commit 846c585910
4 changed files with 143 additions and 179 deletions
+3 -1
View File
@@ -77,7 +77,9 @@ define('COMPLETION_CRITERIA_TYPE_ROLE', 7);
define('COMPLETION_CRITERIA_TYPE_COURSE', 8);
/**
* Criteria type constant to class name mapping
* Criteria type constant to class name mapping.
*
* This global variable would be improved if it was implemented as a cache.
*/
global $COMPLETION_CRITERIA_TYPES;
$COMPLETION_CRITERIA_TYPES = array(
+52 -91
View File
@@ -52,103 +52,66 @@ class completion_daily_task extends scheduled_task {
if (debugging()) {
mtrace('Marking users as started');
}
// This causes it to default to everyone (if there is no student role).
$sqlroles = '';
if (!empty($CFG->gradebookroles)) {
$roles = ' AND ra.roleid IN ('.$CFG->gradebookroles.')';
} else {
// This causes it to default to everyone (if there is no student role)
$roles = '';
$sqlroles = ' AND ra.roleid IN (' . $CFG->gradebookroles.')';
}
/**
* 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
c.id AS course,
u.id AS userid,
crc.id AS completionid,
ue.timestart AS timeenrolled,
ue.timecreated
FROM
{user} u
INNER JOIN
{user_enrolments} ue
ON ue.userid = u.id
INNER JOIN
{enrol} e
ON e.id = ue.enrolid
INNER JOIN
{course} c
ON c.id = e.courseid
INNER JOIN
{role_assignments} ra
ON ra.userid = u.id
LEFT JOIN
{course_completions} crc
ON crc.course = c.id
AND crc.userid = u.id
WHERE
c.enablecompletion = 1
AND crc.timeenrolled IS NULL
AND ue.status = 0
AND e.status = 0
AND u.deleted = 0
AND ue.timestart < ?
AND (ue.timeend > ? OR ue.timeend = 0)
$roles
ORDER BY
course,
userid
";
// 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 c.id AS course, u.id AS userid, crc.id AS completionid, ue.timestart AS timeenrolled,
ue.timecreated
FROM {user} u
INNER JOIN {user_enrolments} ue ON ue.userid = u.id
INNER JOIN {enrol} e ON e.id = ue.enrolid
INNER JOIN {course} c ON c.id = e.courseid
INNER JOIN {role_assignments} ra ON ra.userid = u.id
LEFT JOIN {course_completions} crc ON crc.course = c.id AND crc.userid = u.id
WHERE c.enablecompletion = 1
AND crc.timeenrolled IS NULL
AND ue.status = 0
AND e.status = 0
AND u.deleted = 0
AND ue.timestart < ?
AND (ue.timeend > ? OR ue.timeend = 0)
$sqlroles
ORDER BY course, userid";
$now = time();
$rs = $DB->get_recordset_sql($sql, array($now, $now, $now, $now));
// Check if result is empty
$rs = $DB->get_recordset_sql($sql, [$now, $now, $now, $now]);
// Check if result is empty.
if (!$rs->valid()) {
$rs->close(); // Not going to iterate (but exit), close rs
// Not going to iterate (but exit), close rs.
$rs->close();
return;
}
/**
* 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.
*/
// 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 participant 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 the 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
} else {
// Not all enrol plugins fill out timestart correctly, so use whichever is non-zero.
$current->timeenrolled = max($current->timecreated, $current->timeenrolled);
}
// 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() ||
// 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 = new \completion_completion();
$completion->userid = $prev->userid;
$completion->course = $prev->course;
$completion->timeenrolled = (string) $prev->timeenrolled;
@@ -158,22 +121,20 @@ class completion_daily_task extends scheduled_task {
$completion->id = $prev->completionid;
}
$completion->mark_enrolled();
if (debugging()) {
mtrace('Marked started user '.$prev->userid.' in course '.$prev->course);
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
} else if ($prev && $current) {
// Else, if this record is for the same user/course use oldest timeenrolled.
$current->timeenrolled = min($current->timeenrolled, $prev->timeenrolled);
}
// Move current record to previous
// Move current record to previous.
$prev = $current;
// Move to next record
// Move to next record.
$rs->next();
}
$rs->close();
}
}
}
+87 -86
View File
@@ -51,10 +51,11 @@ class completion_regular_task extends scheduled_task {
// Process each criteria type.
foreach ($COMPLETION_CRITERIA_TYPES as $type) {
$object = 'completion_criteria_'.$type;
require_once $CFG->dirroot.'/completion/criteria/'.$object.'.php';
$object = 'completion_criteria_' . $type;
require_once($CFG->dirroot . '/completion/criteria/' . $object . '.php');
$class = new $object();
// Run the criteria type's cron method, if it has one
// Run the criteria type's cron method, if it has one.
if (method_exists($class, 'cron')) {
if (debugging()) {
mtrace('Running '.$object.'->cron()');
@@ -66,132 +67,132 @@ class completion_regular_task extends scheduled_task {
if (debugging()) {
mtrace('Aggregating completions');
}
// Save time started
// Save time started.
$timestarted = time();
// Grab all criteria and their associated criteria completions
$sql = '
SELECT DISTINCT
c.id AS course,
cr.id AS criteriaid,
crc.userid AS userid,
cr.criteriatype AS criteriatype,
cc.timecompleted AS timecompleted
FROM
{course_completion_criteria} cr
INNER JOIN
{course} c
ON cr.course = c.id
INNER JOIN
{course_completions} crc
ON crc.course = c.id
LEFT JOIN
{course_completion_crit_compl} cc
ON cc.criteriaid = cr.id
AND crc.userid = cc.userid
WHERE
c.enablecompletion = 1
AND crc.timecompleted IS NULL
AND crc.reaggregate > 0
AND crc.reaggregate < :timestarted
ORDER BY
course,
userid
';
$rs = $DB->get_recordset_sql($sql, array('timestarted' => $timestarted));
// Check if result is empty
// Grab all criteria and their associated criteria completions.
$sql = 'SELECT DISTINCT c.id AS course, cr.id AS criteriaid, crc.userid AS userid,
cr.criteriatype AS criteriatype, cc.timecompleted AS timecompleted
FROM {course_completion_criteria} cr
INNER JOIN {course} c ON cr.course = c.id
INNER JOIN {course_completions} crc ON crc.course = c.id
LEFT JOIN {course_completion_crit_compl} cc ON cc.criteriaid = cr.id AND crc.userid = cc.userid
WHERE c.enablecompletion = 1
AND crc.timecompleted IS NULL
AND crc.reaggregate > 0
AND crc.reaggregate < :timestarted
ORDER BY course, userid';
$rs = $DB->get_recordset_sql($sql, ['timestarted' => $timestarted]);
// Check if result is empty.
if (!$rs->valid()) {
$rs->close(); // Not going to iterate (but exit), close rs
$rs->close();
return;
}
$current_user = null;
$current_course = null;
$completions = array();
$currentuser = null;
$currentcourse = null;
$completions = [];
while (1) {
// Grab records for current user/course
// Grab records for current user/course.
foreach ($rs as $record) {
// If we are still grabbing the same users completions
if ($record->userid === $current_user && $record->course === $current_course) {
// If we are still grabbing the same users completions.
if ($record->userid === $currentuser && $record->course === $currentcourse) {
$completions[$record->criteriaid] = $record;
} else {
break;
}
}
// Aggregate
// Aggregate.
if (!empty($completions)) {
if (debugging()) {
mtrace('Aggregating completions for user '.$current_user.' in course '.$current_course);
mtrace('Aggregating completions for user ' . $currentuser . ' in course ' . $currentcourse);
}
// Get course info object
$info = new \completion_info((object)array('id' => $current_course));
// Setup aggregation
// Get course info object.
$info = new \completion_info((object)['id' => $currentcourse]);
// Setup aggregation.
$overall = $info->get_aggregation_method();
$activity = $info->get_aggregation_method(COMPLETION_CRITERIA_TYPE_ACTIVITY);
$prerequisite = $info->get_aggregation_method(COMPLETION_CRITERIA_TYPE_COURSE);
$role = $info->get_aggregation_method(COMPLETION_CRITERIA_TYPE_ROLE);
$overall_status = null;
$activity_status = null;
$prerequisite_status = null;
$role_status = null;
// Get latest timecompleted
$overallstatus = null;
$activitystatus = null;
$prerequisitestatus = null;
$rolestatus = null;
// Get latest timecompleted.
$timecompleted = null;
// Check each of the criteria
// Check each of the criteria.
foreach ($completions as $params) {
$timecompleted = max($timecompleted, $params->timecompleted);
$completion = new \completion_criteria_completion((array)$params, false);
// Handle aggregation special cases
// Handle aggregation special cases.
if ($params->criteriatype == COMPLETION_CRITERIA_TYPE_ACTIVITY) {
completion_cron_aggregate($activity, $completion->is_complete(), $activity_status);
completion_cron_aggregate($activity, $completion->is_complete(), $activitystatus);
} else if ($params->criteriatype == COMPLETION_CRITERIA_TYPE_COURSE) {
completion_cron_aggregate($prerequisite, $completion->is_complete(), $prerequisite_status);
completion_cron_aggregate($prerequisite, $completion->is_complete(), $prerequisitestatus);
} else if ($params->criteriatype == COMPLETION_CRITERIA_TYPE_ROLE) {
completion_cron_aggregate($role, $completion->is_complete(), $role_status);
completion_cron_aggregate($role, $completion->is_complete(), $rolestatus);
} else {
completion_cron_aggregate($overall, $completion->is_complete(), $overall_status);
completion_cron_aggregate($overall, $completion->is_complete(), $overallstatus);
}
}
// Include role criteria aggregation in overall aggregation
if ($role_status !== null) {
completion_cron_aggregate($overall, $role_status, $overall_status);
// Include role criteria aggregation in overall aggregation.
if ($rolestatus !== null) {
completion_cron_aggregate($overall, $rolestatus, $overallstatus);
}
// Include activity criteria aggregation in overall aggregation
if ($activity_status !== null) {
completion_cron_aggregate($overall, $activity_status, $overall_status);
// Include activity criteria aggregation in overall aggregation.
if ($activitystatus !== null) {
completion_cron_aggregate($overall, $activitystatus, $overallstatus);
}
// Include prerequisite criteria aggregation in overall aggregation
if ($prerequisite_status !== null) {
completion_cron_aggregate($overall, $prerequisite_status, $overall_status);
// Include prerequisite criteria aggregation in overall aggregation.
if ($prerequisitestatus !== null) {
completion_cron_aggregate($overall, $prerequisitestatus, $overallstatus);
}
// If aggregation status is true, mark course complete for user
if ($overall_status) {
// If aggregation status is true, mark course complete for user.
if ($overallstatus) {
if (debugging()) {
mtrace('Marking complete');
}
$ccompletion = new \completion_completion(array('course' => $params->course, 'userid' => $params->userid));
$ccompletion = new \completion_completion([
'course' => $params->course,
'userid' => $params->userid
]);
$ccompletion->mark_complete($timecompleted);
}
}
// If this is the end of the recordset, break the loop
// If this is the end of the recordset, break the loop.
if (!$rs->valid()) {
$rs->close();
break;
}
// New/next user, update user details, reset completions
$current_user = $record->userid;
$current_course = $record->course;
$completions = array();
// New/next user, update user details, reset completions.
$currentuser = $record->userid;
$currentcourse = $record->course;
$completions = [];
$completions[$record->criteriaid] = $record;
}
// Mark all users as aggregated
$sql = "
UPDATE
{course_completions}
SET
reaggregate = 0
WHERE
reaggregate < :timestarted
AND reaggregate > 0
";
$DB->execute($sql, array('timestarted' => $timestarted));
// Mark all users as aggregated.
$sql = "UPDATE {course_completions}
SET reaggregate = 0
WHERE reaggregate < :timestarted
AND reaggregate > 0";
$DB->execute($sql, ['timestarted' => $timestarted]);
}
}
+1 -1
View File
@@ -1384,7 +1384,7 @@ function completion_cron_aggregate($method, $data, &$state) {
} else {
$state = false;
}
} elseif ($method == COMPLETION_AGGREGATION_ANY) {
} else if ($method == COMPLETION_AGGREGATION_ANY) {
if ($data) {
$state = true;
} else if (!$data && $state === null) {