Skip to content

Commit

Permalink
MDL-76055 auth_mnet: Fix update_enrolments request
Browse files Browse the repository at this point in the history
Basically this fixes MDL-70833 that was reproduced while
testing the update_enrolments requests. Summary:

- Fixes a typo in table name preventing it to be updated ever.
- Fix outer join that was missing records.
- Stop playing and mixing ids (local and remote).
- Better control which enrolments have to be kept (previously
  they were being deleted immediately after creating them).
- Improve the coding style of inserts.
- Modernise the deletions to use sql helper and to work with 0..n ids.
  • Loading branch information
stronk7 committed Nov 2, 2022
1 parent 2987320 commit 478f423
Showing 1 changed file with 28 additions and 20 deletions.
48 changes: 28 additions & 20 deletions auth/mnet/auth.php
Original file line number Diff line number Diff line change
Expand Up @@ -510,12 +510,12 @@ function update_enrolments($username, $courses) {
c.fullname, c.shortname, c.idnumber, c.summary, c.summaryformat, c.startdate,
e.id AS enrolmentid
FROM {mnetservice_enrol_courses} c
LEFT JOIN {mnetservice_enrol_enrolments} e ON (e.hostid = c.hostid AND e.remotecourseid = c.remoteid)
WHERE e.userid = ? AND c.hostid = ?";
LEFT JOIN {mnetservice_enrol_enrolments} e ON (e.hostid = c.hostid AND e.remotecourseid = c.remoteid AND e.userid = ?)
WHERE c.hostid = ?";

$currentcourses = $DB->get_records_sql($sql, array($userid, $remoteclient->id));

$local_courseid_array = array();
$keepenrolments = array();
foreach($courses as $ix => $course) {

$course['remoteid'] = $course['id'];
Expand All @@ -525,22 +525,33 @@ function update_enrolments($username, $courses) {
// if we do not have the the information about the remote course, it is not available
// to us for remote enrolment - skip
if (array_key_exists($course['remoteid'], $currentcourses)) {
// We are going to keep this enrolment, it will be updated or inserted, but will keep it.
$keepenrolments[] = $course['id'];

// Pointer to current course:
$currentcourse =& $currentcourses[$course['remoteid']];
// We have a record - is it up-to-date?
$course['id'] = $currentcourse->id;

$saveflag = false;

foreach($course as $key => $value) {
// Only compare what is available locally, data coming from enrolment tables have
// way more information that tables used to keep the track of mnet enrolments.
if (!property_exists($currentcourse, $key)) {
continue;
}
// Don't compare ids either, they come from different databases.
if ($key === 'id') {
continue;
}

if ($currentcourse->$key != $value) {
$saveflag = true;
$currentcourse->$key = $value;
}
}

if ($saveflag) {
$DB->update_record('mnetervice_enrol_courses', $currentcourse);
$DB->update_record('mnetservice_enrol_courses', $currentcourse);
}

if (isset($currentcourse->enrolmentid) && is_numeric($currentcourse->enrolmentid)) {
Expand All @@ -551,31 +562,28 @@ function update_enrolments($username, $courses) {
continue;
}

// By this point, we should always have a $dataObj->id
$local_courseid_array[] = $course['id'];

// Do we have a record for this assignment?
if ($userisregd) {
// Yes - we know about this one already
// We don't want to do updates because the new data is probably
// 'less complete' than the data we have.
} else {
// No - create a record
$assignObj = new stdClass();
$assignObj->userid = $userid;
$assignObj->hostid = (int)$remoteclient->id;
$assignObj->remotecourseid = $course['remoteid'];
$assignObj->rolename = $course['defaultrolename'];
$assignObj->id = $DB->insert_record('mnetservice_enrol_enrolments', $assignObj);
$newenrol = new stdClass();
$newenrol->userid = $userid;
$newenrol->hostid = (int)$remoteclient->id;
$newenrol->remotecourseid = $course['remoteid'];
$newenrol->rolename = $course['defaultrolename'];
$newenrol->enroltype = 'mnet';
$newenrol->id = $DB->insert_record('mnetservice_enrol_enrolments', $newenrol);
}
}

// Clean up courses that the user is no longer enrolled in.
if (!empty($local_courseid_array)) {
$local_courseid_string = implode(', ', $local_courseid_array);
$whereclause = " userid = ? AND hostid = ? AND remotecourseid NOT IN ($local_courseid_string)";
$DB->delete_records_select('mnetservice_enrol_enrolments', $whereclause, array($userid, $remoteclient->id));
}
list($insql, $inparams) = $DB->get_in_or_equal($keepenrolments, SQL_PARAMS_NAMED, 'param', false, null);
$whereclause = ' userid = :userid AND hostid = :hostid AND remotecourseid ' . $insql;
$params = array_merge(['userid' => $userid, 'hostid' => $remoteclient->id], $inparams);
$DB->delete_records_select('mnetservice_enrol_enrolments', $whereclause, $params);
}

function prevent_local_passwords() {
Expand Down

0 comments on commit 478f423

Please sign in to comment.