Arlo 77

.github/workflows/moodle-ci.yml

@@ -8,7 +8,7 @@ jobs:
 
     services:
       postgres:
-        image: postgres:13
+        image: postgres:16
         env:
           POSTGRES_USER: 'postgres'
           POSTGRES_HOST_AUTH_METHOD: 'trust'
@@ -19,9 +19,9 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        php: ['8.0', '8.1']
+        php: ['8.3', '8.4']
         include:
-          - moodle-branch: 'MOODLE_401_STABLE'
+          - moodle-branch: 'MOODLE_502_STABLE'
 
     steps:
       - name: Check out repository code
@@ -39,7 +39,7 @@ jobs:
 
       - name: Initialise moodle-plugin-ci
         run: |
-          composer create-project -n --no-dev --prefer-dist moodlehq/moodle-plugin-ci ci ^3
+          composer create-project -n --no-dev --prefer-dist moodlehq/moodle-plugin-ci ci ^4
           echo $(cd ci/bin; pwd) >> $GITHUB_PATH
           echo $(cd ci/vendor/bin; pwd) >> $GITHUB_PATH
           sudo locale-gen en_AU.UTF-8

classes/adminsettings/configarlostatus.php

@@ -99,25 +99,27 @@ public static function api_status_render() {
         $description = '';
         if (200 == $apistatus) {
             if ($useimageiconclass) {
-                $statusicon = $OUTPUT->image_icon('t/go', get_string('ok', 'enrol_arlo'));
+                $statusicon = $OUTPUT->image_icon('t/completion_complete', get_string('ok', 'enrol_arlo'));
+
             } else {
-                $statusicon = $OUTPUT->pix_icon('t/go', get_string('ok', 'enrol_arlo'));
+                $statusicon = $OUTPUT->pix_icon('t/completion_complete', get_string('ok', 'enrol_arlo'));
+
             }
             $reason = get_string('apistatusok', 'enrol_arlo', userdate($apilastrequested));
         } else if (0 == $apistatus || ($apistatus >= 400 && $apistatus < 499)) {
             if ($useimageiconclass) {
-                $statusicon = $OUTPUT->image_icon('t/stop', get_string('notok', 'enrol_arlo'));
+                $statusicon = $OUTPUT->image_icon('t/completion_fail', get_string('notok', 'enrol_arlo'), null);
             } else {
-                $statusicon = $OUTPUT->pix_icon('t/stop', get_string('notok', 'enrol_arlo'));
+                $statusicon = $OUTPUT->pix_icon('t/completion_fail', get_string('notok', 'enrol_arlo'));
             }
             $reason = get_string('apistatusclienterror', 'enrol_arlo');
             $url = new \moodle_url('/enrol/arlo/admin/apirequests.php');
             $description = get_string('pleasecheckrequestlog', 'enrol_arlo', $url->out());
         } else if ($apistatus >= 500 && $apistatus < 599) {
             if ($useimageiconclass) {
-                $statusicon = $OUTPUT->image_icon('t/stop', get_string('notok', 'enrol_arlo'));
+                $statusicon = $OUTPUT->image_icon('t/completion_fail', get_string('notok', 'enrol_arlo'), null);
             } else {
-                $statusicon = $OUTPUT->pix_icon('t/stop', get_string('notok', 'enrol_arlo'));
+                $statusicon = $OUTPUT->pix_icon('t/completion_fail', get_string('notok', 'enrol_arlo'));
             }
             $reason = get_string('apistatusservererror', 'enrol_arlo');
             $url = new \moodle_url('/enrol/arlo/admin/apirequests.php');

classes/api.php

@@ -60,7 +60,7 @@ class api {
      * @throws \coding_exception
      * @throws moodle_exception
      */
-    public static function api_callable(progress_trace $trace = null) {
+    public static function api_callable(?progress_trace $trace = null) {
         if (is_null($trace)) {
             $trace = new null_progress_trace();
         }
@@ -115,7 +115,7 @@ public static function get_enrolment_plugin() {
      * @throws \dml_exception
      * @throws moodle_exception
      */
-    public static function run_outcome_jobs($limit = 100, progress_trace $trace = null) {
+    public static function run_outcome_jobs($limit = 100, ?progress_trace $trace = null) {
         global $DB;
         if (!static::api_callable($trace)) {
             return false;
@@ -181,7 +181,7 @@ public static function run_outcome_jobs($limit = 100, progress_trace $trace = nu
      * @throws \dml_exception
      * @throws moodle_exception
      */
-    public static function run_scheduled_jobs($area, $type, $time = null, $limit = 50, progress_trace $trace = null) {
+    public static function run_scheduled_jobs($area, $type, $time = null, $limit = 50, ?progress_trace $trace = null) {
         global $DB;
         if (!static::api_callable($trace)) {
             return false;
@@ -281,7 +281,7 @@ public static function run_scheduled_jobs($area, $type, $time = null, $limit = 5
      * @throws \dml_exception
      * @throws moodle_exception
      */
-    public static function run_site_jobs(progress_trace $trace = null) {
+    public static function run_site_jobs(?progress_trace $trace = null) {
         global $DB;
         if (!static::api_callable($trace)) {
             return false;
@@ -330,7 +330,7 @@ public static function run_site_jobs(progress_trace $trace = null) {
      * @throws \coding_exception
      * @throws \dml_exception
      */
-    public static function run_cleanup(progress_trace $trace = null) {
+    public static function run_cleanup(?progress_trace $trace = null) {
         global $DB;
         if (is_null($trace)) {
             $trace = new null_progress_trace();

classes/local/generator/username_generator.php

@@ -73,7 +73,7 @@ class username_generator {
      * @param array|null $options
      * @throws coding_exception
      */
-    public function __construct($data = null, string $order = null, array $options = null) {
+    public function __construct($data = null, ?string $order = null, ?array $options = null) {
         $this->register_default_formats();
         if (!is_null($data)) {
             $this->add_data($data);

classes/local/job/memberships_job.php

@@ -548,7 +548,7 @@ public static function match_user_from_contact($contact) {
      */
     public static function process_enrolment_registration(stdClass $enrolmentinstance,
                                                           registration_persistent $registration,
-                                                          contact_persistent $contact = null) {
+                                                          ?contact_persistent $contact = null) {
         // Load plugin class instance.
         $plugin = api::get_enrolment_plugin();
         // Get plugin config.
@@ -706,34 +706,38 @@ public static function save_resource_information_to_persistents($enrolmentinstan
             throw new moodle_exception('missingresource',
                 null, null,  null, 'course'); // Course is Event ot Online Activity.
         }
-        // Check for existing registration record.
-        $registration = registration_persistent::get_record(
-            ['sourceguid' => $sourceguid]
-        );
-        if (!$registration) {
-            // Now we try by user and enrolment instance.
-            $contact = contact_persistent::get_record(
-                ['sourceid' => $contactresource->ContactID]
+        // ARLO-77: one registration row per (userid, enrolid). Look up by user+course when we
+        // already know the user, falling back to the incoming sourceguid.
+        $existingcontact = contact_persistent::get_record(['sourceid' => $contactresource->ContactID]);
+        $registration = null;
+        if ($existingcontact && $existingcontact->get('userid') > 0) {
+            $registration = registration_persistent::get_record(
+                ['userid' => $existingcontact->get('userid'), 'enrolid' => $enrolmentinstance->id]
             );
-            if (!empty($contact)) {
-                $registration = registration_persistent::get_record(
-                    ['userid' => $contact->get('userid'), 'enrolid' => $enrolmentinstance->id]
-                );
-                if (!empty($registration)) {
-                    // We don't want to re-process the registrations if it hasn't been modified since the last sync.
-                    $lastsourcemodifieddb = $registration->get('sourcemodified');
-                    $lastsourcemodifiedapi = $resource->LastModifiedDateTime;
-                    // It must be newer, if has the same timestamp we already processed it.
-                    if ($lastsourcemodifieddb > $lastsourcemodifiedapi) {
-                        return [$registration, $contactresource, true];
-                    }
+        }
+        $registration = $registration ?: registration_persistent::get_record(['sourceguid' => $sourceguid]);
+
+        if ($registration) {
+            // Nothing newer to apply - short-circuit so the outcomes-push -> memberships-poll
+            // loop doesn't re-run the enrolment pipeline every cycle.
+            if ($registration->get('sourcemodified') >= $resource->LastModifiedDateTime) {
+                return [$registration, $existingcontact, true];
+            }
+            // Pair switched to a new Arlo registration ID. Drop any orphan row already holding
+            // the incoming sourceguid (UNIQUE index) before updating in place.
+            if ($registration->get('sourceguid') !== $sourceguid) {
+                $orphan = registration_persistent::get_record(['sourceguid' => $sourceguid]);
+                if ($orphan && $orphan->get('id') != $registration->get('id')) {
+                    $orphan->delete();
                 }
+                $registration->set('sourceguid', $sourceguid);
+                $registration->set('sourceid', $sourceid);
             }
-
+        } else {
             $registration = new registration_persistent();
             $registration->set('sourceid', $sourceid);
             $registration->set('sourceguid', $sourceguid);
-        } 
+        }
         $registration->set('enrolid', $enrolmentinstance->id);
         $registration->set('attendance', $resource->Attendance);
         $registration->set('outcome', $resource->Outcome);
@@ -754,8 +758,8 @@ public static function save_resource_information_to_persistents($enrolmentinstan
             $registration->set('sourceonlineactivityid', $onlineactivityresource->OnlineActivityID);
             $registration->set('sourceonlineactivityguid', $onlineactivityresource->UniqueIdentifier);
         }
-        // Check for existing contact record.
-        $contact = $registration->get_contact();
+        // Check for existing contact record. Reuse the lookup we did above when available.
+        $contact = $existingcontact ?: $registration->get_contact();
         // Create new contact.
         if (!$contact) {
             $contact = new contact_persistent();
@@ -785,8 +789,7 @@ public static function save_resource_information_to_persistents($enrolmentinstan
         $registration->set('errormessage', '');
         $registration->set('enrolmentfailure', 0);
         $registration->save();
-        // Return registration and contact persistents.
-        return array($registration, $contact);
+        return [$registration, $contact, false];
     }
 
 }

classes/local/persistent/user_persistent.php

@@ -171,7 +171,7 @@ protected static function define_properties() {
      * @throws \dml_exception
      * @throws coding_exception
      */
-    public function __construct($id = 0, stdClass $record = null) {
+    public function __construct($id = 0, ?stdClass $record = null) {
         global $DB;
 
         if ($id > 0) {

classes/manager.php

@@ -34,7 +34,7 @@ class manager {
     /** @var \progress_trace  */
     private static $trace;
 
-    public function __construct(\progress_trace $trace = null) {
+    public function __construct(?\progress_trace $trace = null) {
         // Raise limits, so this script can be interrupted without problems.
         \core_php_time_limit::raise();
         raise_memory_limit(MEMORY_HUGE);

classes/persistent.php

@@ -58,7 +58,7 @@ abstract class persistent {
      * @param int $id If set, this is the id of an existing record, used to load the data.
      * @param stdClass $record If set will be passed to {@link self::from_record()}.
      */
-    public function __construct($id = 0, stdClass $record = null) {
+    public function __construct($id = 0, ?stdClass $record = null) {
         global $CFG;
 
         if ($id > 0) {
@@ -946,7 +946,7 @@ public static function record_exists($id) {
      * @param array $params
      * @return bool
      */
-    public static function record_exists_select($select, array $params = null) {
+    public static function record_exists_select($select, ?array $params = null) {
         global $DB;
         return $DB->record_exists_select(static::TABLE, $select, $params);
     }

lib.php

@@ -227,7 +227,7 @@ public function add_default_instance($course) {
      * @throws moodle_exception
      * @throws required_capability_exception
      */
-    public function add_instance($course, array $fields = null) {
+    public function add_instance($course, ?array $fields = null) {
         $pluginconfig = new arlo_plugin_config();
         $fields['roleid'] = $pluginconfig->get('roleid');
         $fields['customchar1'] = $pluginconfig->get('platform');

tests/contact_merge_requests_test.php

@@ -29,7 +29,7 @@
 use enrol_arlo\local\persistent\user_persistent;
 use enrol_arlo\local\handler\contact_merge_requests_handler;
 
-class enrol_arlo_contact_merge_requests_testcase extends advanced_testcase {
+class contact_merge_requests_test extends advanced_testcase {
 
     public function test_no_merge_requests() {
         global $CFG, $DB;

tests/generator/lib.php

@@ -114,7 +114,7 @@ public function setup_plugin($platformdomain = NULL) {
      * @throws \enrol_arlo\invalid_persistent_exception
      * @throws coding_exception
      */
-    public function create_contact(stdClass $data = null) {
+    public function create_contact(?stdClass $data = null) {
         $randomnumber = rand();
         $datetime = $this->get_arlo_type_datetime();
         $contact = new contact_persistent();
@@ -143,7 +143,7 @@ public function create_contact(stdClass $data = null) {
      * @return event_persistent
      * @throws coding_exception
      */
-    public function create_event(event_template_persistent $template, stdClass $data = null) {
+    public function create_event(event_template_persistent $template, ?stdClass $data = null) {
         $randomnumber = rand();
         $datetimeformat = $this->get_arlo_datetime_format();
         $date = new DateTime(
@@ -241,7 +241,7 @@ public function create_event_enrolment_instance($course, event_persistent $event
     public function create_event_registration(contact_persistent $contact,
                                               event_persistent $event,
                                               $enrolmentinstance = null,
-                                              stdClass $data = null) {
+                                              ?stdClass $data = null) {
         $randomnumber = rand();
         $datetime = $this->get_arlo_type_datetime();
         $registration = new registration_persistent();