From 478f423e2d611ffbd3488c9d8aeb2f06dec48a08 Mon Sep 17 00:00:00 2001
From: "Eloy Lafuente (stronk7)" <stronk7@moodle.org>
Date: Sun, 25 Sep 2022 16:27:21 +0200
Subject: [PATCH] MDL-76055 auth_mnet: Fix update_enrolments request

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.
---
 auth/mnet/auth.php | 48 +++++++++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/auth/mnet/auth.php b/auth/mnet/auth.php
index b5c34cb8b24..ade0cc3b88a 100644
--- a/auth/mnet/auth.php
+++ b/auth/mnet/auth.php
@@ -510,12 +510,12 @@ class auth_plugin_mnet extends auth_plugin_base {
                        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'];
@@ -525,14 +525,25 @@ class auth_plugin_mnet extends auth_plugin_base {
             // 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;
@@ -540,7 +551,7 @@ class auth_plugin_mnet extends auth_plugin_base {
                 }
 
                 if ($saveflag) {
-                    $DB->update_record('mnetervice_enrol_courses', $currentcourse);
+                    $DB->update_record('mnetservice_enrol_courses', $currentcourse);
                 }
 
                 if (isset($currentcourse->enrolmentid) && is_numeric($currentcourse->enrolmentid)) {
@@ -551,9 +562,6 @@ class auth_plugin_mnet extends auth_plugin_base {
                 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
@@ -561,21 +569,21 @@ class auth_plugin_mnet extends auth_plugin_base {
                 // '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() {