Merge pull request #198 from ArloSoftware/staging

Version 4.1.2 into master

.github/workflows/moodle-ci.yml

@@ -19,9 +19,9 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
+        php: ['8.0', '8.1']
         include:
-          - php: '8.0'
-            moodle-branch: 'MOODLE_401_STABLE'
+          - moodle-branch: 'MOODLE_401_STABLE'
 
     steps:
       - name: Check out repository code

classes/local/job/memberships_job.php

@@ -547,7 +547,7 @@ public static function process_enrolment_registration(stdClass $enrolmentinstanc
                         }
                         $user->set('phone1', $contact->get('phonemobile'));
                         $user->set('phone2', $contact->get('phonework'));
-                        $user->create();
+                        $user->create_user();
                         // Important must associate user with contact.
                         $contact->set('userid', $user->get('id'));
                         $contact->save();
@@ -575,7 +575,7 @@ public static function process_enrolment_registration(stdClass $enrolmentinstanc
                 }
                 $user->set('phone1', $contact->get('phonemobile'));
                 $user->set('phone2', $contact->get('phonework'));
-                $user->update();
+                $user->update_user();
 
             }
             // Important must associate user with registration.

classes/local/job/outcomes_job.php

@@ -137,12 +137,23 @@ public function run() {
         $lockfactory = static::get_lock_factory();
         $lock = $lockfactory->get_lock($this->get_lock_resource(), self::TIME_LOCK_TIMEOUT);
         if ($lock) {
-            $limit = $pluginconfig->get('outcomejobdefaultlimit');
-            $registrations = registration_persistent::get_records(
-                ['enrolid' => $enrolmentinstance->id, 'updatesource' => 1],
-                'timelastrequest', 'ASC', 0, $limit
-            );
-            $course = get_course($enrolmentinstance->courseid);
+            try {
+                $limit = $pluginconfig->get('outcomejobdefaultlimit');
+                $registrations = registration_persistent::get_records(
+                    ['enrolid' => $enrolmentinstance->id, 'updatesource' => 1],
+                    'timelastrequest', 'ASC', 0, $limit
+                );
+                $course = get_course($enrolmentinstance->courseid);
+            } catch (Exception $exception) {
+                // Update scheduling information on persistent after successfull save.
+                $jobpersistent->set('timelastrequest', time());
+                $jobpersistent->save();
+                // Log error and release lock. Rethrow exception.
+                $this->add_error($exception->getMessage());
+                $lock->release();
+                throw $exception;
+            }
+
             if (!$registrations) {
                 // Update scheduling information on persistent after successfull save.
                 $jobpersistent->set('timelastrequest', time());
@@ -161,20 +172,20 @@ public function run() {
                         if (!empty($data)) {
                             $this->trace->output(implode(',', $data));
                             external::patch_registration_resource($sourceregistration, $data);
+                            $registrationpersistent->set('timelastrequest', time());
+                            // Reset update flag.
+                            $registrationpersistent->set('updatesource', 0);
+                            $registrationpersistent->save();
                         }
                     } catch (Exception $exception) {
                         debugging($exception->getMessage(), DEBUG_DEVELOPER);
                         $this->add_error($exception->getMessage());
                         $registrationpersistent->set('errormessage', $exception->getMessage());
-                    } finally {
-                        $registrationpersistent->set('timelastrequest', time());
-                        // Reset update flag.
-                        $registrationpersistent->set('updatesource', 0);
-                        $registrationpersistent->save();
+                    } finally {                        
                         // Update scheduling information on persistent after successfull save.
                         $jobpersistent->set('timelastrequest', time());
                         $jobpersistent->save();
-                        $lock->release();
+                        // DO NOT release lock here. This is a foreach loop!
                     }
                 }
             }

classes/local/persistent/registration_persistent.php

@@ -224,7 +224,7 @@ public function get_online_activity() {
      * @throws coding_exception
      */
     protected function set_progresspercent($value) {
-        return $this->raw_set('progresspercent', round($value, 0));
+        return $this->raw_set('progresspercent', round($value ?? 0, 0));
     }
 
     /**

classes/local/persistent/user_persistent.php

@@ -464,6 +464,47 @@ protected function before_create() {
         $this->set('confirmed', 1);
     }
 
+    /**
+     * Creates the user data
+     * 
+     */
+    public function create_user() {
+        global $CFG, $USER;
+        if ($this->raw_get('id')) {
+            // The validation methods rely on the ID to know if we're updating or not, the ID should be
+            // falsy whenever we are creating an object.
+            throw new coding_exception('Cannot create an object that has an ID defined.');
+        }
+
+        if (!$this->is_valid()) {
+            throw new invalid_persistent_exception($this->get_errors());
+        }
+
+        // Before create hook.
+        $this->before_create();
+
+        // We can safely set those values bypassing the validation because we know what we're doing.
+        $now = time();
+        $this->raw_set('timecreated', $now);
+        $this->raw_set('timemodified', $now);
+        $this->raw_set('usermodified', $USER->id);
+
+        $record = $this->to_record();
+        unset($record->id);
+
+        require_once($CFG->dirroot . '/user/lib.php');
+        $id = user_create_user($record, true, false);
+        $this->raw_set('id', $id);
+
+        // We ensure that this is flagged as validated.
+        $this->validated = true;
+
+        // After create hook.
+        $this->after_create();
+
+        return $this;
+    }
+
     /**
      * Set user context, email and trigger new user event.
      *
@@ -473,8 +514,6 @@ protected function before_create() {
     protected function after_create() {
         $pluginconfig = api::get_enrolment_plugin()->get_plugin_config();
         $newuserid = $this->get('id');
-        // Create a context for this user.
-        context_user::instance($newuserid);
         // Send email. TODO refactor messaging.
         $manager = new manager();
         if ($pluginconfig->get('emailsendnewaccountdetails')) {
@@ -492,6 +531,43 @@ protected function after_create() {
         \core\event\user_created::create_from_userid($newuserid)->trigger();
     }
 
+    /**
+     * Update the existing record in the DB.
+     *
+     * @return bool True on success.
+     */
+    public function update_user() {
+        global $USER, $CFG;
+
+        if ($this->raw_get('id') <= 0) {
+            throw new coding_exception('id is required to update');
+        } else if (!$this->is_valid()) {
+            throw new invalid_persistent_exception($this->get_errors());
+        }
+
+        // Before update hook.
+        $this->before_update();
+
+        // We can safely set those values after the validation because we know what we're doing.
+        $this->raw_set('timemodified', time());
+        $this->raw_set('usermodified', $USER->id);
+
+        $record = $this->to_record();
+        unset($record->timecreated);
+
+        require_once($CFG->dirroot . '/user/lib.php');
+        // Save the record.
+        $result = user_update_user($record, true, false);
+
+        // We ensure that this is flagged as validated.
+        $this->validated = true;
+
+        // After update hook.
+        $this->after_update($result);
+
+        return $result;
+    }
+
     /**
      * Fire update event.
      *

classes/local/tablesql/unsuccessful_enrolments_table_sql.php

@@ -265,9 +265,17 @@ public function col_actions($values) {
                 );
                 $contactmergerequest = reset($contactmergerequests);
                 $sourcecontact = $contactmergerequest->get_source_contact();
-                $sourceenrolments = $sourcecontact->get_associated_user()->has_course_enrolments();
-                $destinationenrolments = $contact->get_associated_user()->has_course_enrolments();
-                if (!($sourceenrolments && $destinationenrolments)) {
+
+                // Merge failed errors are misbehaving and sometimes have no source contact.
+                // Minor patch while that is fixed.
+                $sourceenrolments = false;
+                $destinationenrolments = false;
+                if (!empty($sourcecontact)) {
+                    $sourceenrolments = $sourcecontact->get_associated_user()->has_course_enrolments();
+                    $destinationenrolments = $contact->get_associated_user()->has_course_enrolments();
+                }
+
+                if (empty($sourcecontact) || !($sourceenrolments && $destinationenrolments)) {
                     $url = new moodle_url('/enrol/arlo/admin/reattemptenrolment.php');
                     $url->param('id', $values->id);
                     $text = get_string('reattemptenrolment', 'enrol_arlo');

classes/manager.php

@@ -258,7 +258,7 @@ public function email_coursewelcome($instance, $user) {
         $a->fullname = fullname($user);
         $a->email = $user->email;
         $a->forgotpasswordurl = "$CFG->wwwroot/login/forgot_password.php";
-        if (trim($instance->customtext1) !== '') {
+        if (isset($instance->customtext1) && trim($instance->customtext1) !== '') {
             $message = $instance->customtext1;
             $key = array(
                 '{$a->coursename}',

classes/persistent.php

@@ -247,11 +247,14 @@ protected static function define_properties() {
     final public static function properties_definition() {
         global $CFG;
 
-        static $def = null;
-        if ($def !== null) {
-            return $def;
+        static $cachedef = [];
+        if (isset($cachedef[static::class])) {
+            return $cachedef[static::class];
         }
 
+        $cachedef[static::class] = static::define_properties();
+        $def = &$cachedef[static::class];
+
         $def = static::define_properties();
         $def['id'] = array(
             'default' => 0,

version.php

@@ -27,9 +27,9 @@
 defined('MOODLE_INTERNAL') || die();
 
 $plugin            = new stdClass();
-$plugin->version   = 2023062000;
+$plugin->version   = 2023091900;
 $plugin->requires  = 2022112800;        // See http://docs.moodle.org/dev/Moodle_Version
 $plugin->component = 'enrol_arlo';      // Full name of the plugin (used for diagnostics).
-$plugin->release   = '4.1.1';          // Human-friendly version name.
+$plugin->release   = '4.1.2';          // Human-friendly version name.
 $plugin->maturity  = MATURITY_STABLE;   // This version's maturity level.
 $plugin->dependencies = [];