Skip to content

Commit 786110a

Browse files
ojnadjarmojnadjarm
authored
Merge pull request #276 from ArloSoftware/ARLO-77
Arlo 77
2 parents 907944d + fcf1d38 commit 786110a

14 files changed

Lines changed: 236 additions & 56 deletions

File tree

‎.github/workflows/moodle-ci.yml‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ jobs:
88

99
services:
1010
postgres:
11-
image: postgres:13
11+
image: postgres:16
1212
env:
1313
POSTGRES_USER: 'postgres'
1414
POSTGRES_HOST_AUTH_METHOD: 'trust'
@@ -19,9 +19,9 @@ jobs:
1919
strategy:
2020
fail-fast: false
2121
matrix:
22-
php: ['8.0', '8.1']
22+
php: ['8.3', '8.4']
2323
include:
24-
- moodle-branch: 'MOODLE_401_STABLE'
24+
- moodle-branch: 'MOODLE_502_STABLE'
2525

2626
steps:
2727
- name: Check out repository code
@@ -39,7 +39,7 @@ jobs:
3939

4040
- name: Initialise moodle-plugin-ci
4141
run: |
42-
composer create-project -n --no-dev --prefer-dist moodlehq/moodle-plugin-ci ci ^3
42+
composer create-project -n --no-dev --prefer-dist moodlehq/moodle-plugin-ci ci ^4
4343
echo $(cd ci/bin; pwd) >> $GITHUB_PATH
4444
echo $(cd ci/vendor/bin; pwd) >> $GITHUB_PATH
4545
sudo locale-gen en_AU.UTF-8

‎classes/adminsettings/configarlostatus.php‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,25 +99,27 @@ public static function api_status_render() {
9999
$description = '';
100100
if (200 == $apistatus) {
101101
if ($useimageiconclass) {
102-
$statusicon = $OUTPUT->image_icon('t/go', get_string('ok', 'enrol_arlo'));
102+
$statusicon = $OUTPUT->image_icon('t/completion_complete', get_string('ok', 'enrol_arlo'));
103+
103104
} else {
104-
$statusicon = $OUTPUT->pix_icon('t/go', get_string('ok', 'enrol_arlo'));
105+
$statusicon = $OUTPUT->pix_icon('t/completion_complete', get_string('ok', 'enrol_arlo'));
106+
105107
}
106108
$reason = get_string('apistatusok', 'enrol_arlo', userdate($apilastrequested));
107109
} else if (0 == $apistatus || ($apistatus >= 400 && $apistatus < 499)) {
108110
if ($useimageiconclass) {
109-
$statusicon = $OUTPUT->image_icon('t/stop', get_string('notok', 'enrol_arlo'));
111+
$statusicon = $OUTPUT->image_icon('t/completion_fail', get_string('notok', 'enrol_arlo'), null);
110112
} else {
111-
$statusicon = $OUTPUT->pix_icon('t/stop', get_string('notok', 'enrol_arlo'));
113+
$statusicon = $OUTPUT->pix_icon('t/completion_fail', get_string('notok', 'enrol_arlo'));
112114
}
113115
$reason = get_string('apistatusclienterror', 'enrol_arlo');
114116
$url = new \moodle_url('/enrol/arlo/admin/apirequests.php');
115117
$description = get_string('pleasecheckrequestlog', 'enrol_arlo', $url->out());
116118
} else if ($apistatus >= 500 && $apistatus < 599) {
117119
if ($useimageiconclass) {
118-
$statusicon = $OUTPUT->image_icon('t/stop', get_string('notok', 'enrol_arlo'));
120+
$statusicon = $OUTPUT->image_icon('t/completion_fail', get_string('notok', 'enrol_arlo'), null);
119121
} else {
120-
$statusicon = $OUTPUT->pix_icon('t/stop', get_string('notok', 'enrol_arlo'));
122+
$statusicon = $OUTPUT->pix_icon('t/completion_fail', get_string('notok', 'enrol_arlo'));
121123
}
122124
$reason = get_string('apistatusservererror', 'enrol_arlo');
123125
$url = new \moodle_url('/enrol/arlo/admin/apirequests.php');

‎classes/api.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ class api {
6060
* @throws \coding_exception
6161
* @throws moodle_exception
6262
*/
63-
public static function api_callable(progress_trace $trace = null) {
63+
public static function api_callable(?progress_trace $trace = null) {
6464
if (is_null($trace)) {
6565
$trace = new null_progress_trace();
6666
}
@@ -115,7 +115,7 @@ public static function get_enrolment_plugin() {
115115
* @throws \dml_exception
116116
* @throws moodle_exception
117117
*/
118-
public static function run_outcome_jobs($limit = 100, progress_trace $trace = null) {
118+
public static function run_outcome_jobs($limit = 100, ?progress_trace $trace = null) {
119119
global $DB;
120120
if (!static::api_callable($trace)) {
121121
return false;
@@ -181,7 +181,7 @@ public static function run_outcome_jobs($limit = 100, progress_trace $trace = nu
181181
* @throws \dml_exception
182182
* @throws moodle_exception
183183
*/
184-
public static function run_scheduled_jobs($area, $type, $time = null, $limit = 50, progress_trace $trace = null) {
184+
public static function run_scheduled_jobs($area, $type, $time = null, $limit = 50, ?progress_trace $trace = null) {
185185
global $DB;
186186
if (!static::api_callable($trace)) {
187187
return false;
@@ -281,7 +281,7 @@ public static function run_scheduled_jobs($area, $type, $time = null, $limit = 5
281281
* @throws \dml_exception
282282
* @throws moodle_exception
283283
*/
284-
public static function run_site_jobs(progress_trace $trace = null) {
284+
public static function run_site_jobs(?progress_trace $trace = null) {
285285
global $DB;
286286
if (!static::api_callable($trace)) {
287287
return false;
@@ -330,7 +330,7 @@ public static function run_site_jobs(progress_trace $trace = null) {
330330
* @throws \coding_exception
331331
* @throws \dml_exception
332332
*/
333-
public static function run_cleanup(progress_trace $trace = null) {
333+
public static function run_cleanup(?progress_trace $trace = null) {
334334
global $DB;
335335
if (is_null($trace)) {
336336
$trace = new null_progress_trace();

‎classes/local/generator/username_generator.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ class username_generator {
7373
* @param array|null $options
7474
* @throws coding_exception
7575
*/
76-
public function __construct($data = null, string $order = null, array $options = null) {
76+
public function __construct($data = null, ?string $order = null, ?array $options = null) {
7777
$this->register_default_formats();
7878
if (!is_null($data)) {
7979
$this->add_data($data);

‎classes/local/job/memberships_job.php‎

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -548,7 +548,7 @@ public static function match_user_from_contact($contact) {
548548
*/
549549
public static function process_enrolment_registration(stdClass $enrolmentinstance,
550550
registration_persistent $registration,
551-
contact_persistent $contact = null) {
551+
?contact_persistent $contact = null) {
552552
// Load plugin class instance.
553553
$plugin = api::get_enrolment_plugin();
554554
// Get plugin config.
@@ -706,34 +706,38 @@ public static function save_resource_information_to_persistents($enrolmentinstan
706706
throw new moodle_exception('missingresource',
707707
null, null, null, 'course'); // Course is Event ot Online Activity.
708708
}
709-
// Check for existing registration record.
710-
$registration = registration_persistent::get_record(
711-
['sourceguid' => $sourceguid]
712-
);
713-
if (!$registration) {
714-
// Now we try by user and enrolment instance.
715-
$contact = contact_persistent::get_record(
716-
['sourceid' => $contactresource->ContactID]
709+
// ARLO-77: one registration row per (userid, enrolid). Look up by user+course when we
710+
// already know the user, falling back to the incoming sourceguid.
711+
$existingcontact = contact_persistent::get_record(['sourceid' => $contactresource->ContactID]);
712+
$registration = null;
713+
if ($existingcontact && $existingcontact->get('userid') > 0) {
714+
$registration = registration_persistent::get_record(
715+
['userid' => $existingcontact->get('userid'), 'enrolid' => $enrolmentinstance->id]
717716
);
718-
if (!empty($contact)) {
719-
$registration = registration_persistent::get_record(
720-
['userid' => $contact->get('userid'), 'enrolid' => $enrolmentinstance->id]
721-
);
722-
if (!empty($registration)) {
723-
// We don't want to re-process the registrations if it hasn't been modified since the last sync.
724-
$lastsourcemodifieddb = $registration->get('sourcemodified');
725-
$lastsourcemodifiedapi = $resource->LastModifiedDateTime;
726-
// It must be newer, if has the same timestamp we already processed it.
727-
if ($lastsourcemodifieddb > $lastsourcemodifiedapi) {
728-
return [$registration, $contactresource, true];
729-
}
717+
}
718+
$registration = $registration ?: registration_persistent::get_record(['sourceguid' => $sourceguid]);
719+
720+
if ($registration) {
721+
// Nothing newer to apply - short-circuit so the outcomes-push -> memberships-poll
722+
// loop doesn't re-run the enrolment pipeline every cycle.
723+
if ($registration->get('sourcemodified') >= $resource->LastModifiedDateTime) {
724+
return [$registration, $existingcontact, true];
725+
}
726+
// Pair switched to a new Arlo registration ID. Drop any orphan row already holding
727+
// the incoming sourceguid (UNIQUE index) before updating in place.
728+
if ($registration->get('sourceguid') !== $sourceguid) {
729+
$orphan = registration_persistent::get_record(['sourceguid' => $sourceguid]);
730+
if ($orphan && $orphan->get('id') != $registration->get('id')) {
731+
$orphan->delete();
730732
}
733+
$registration->set('sourceguid', $sourceguid);
734+
$registration->set('sourceid', $sourceid);
731735
}
732-
736+
} else {
733737
$registration = new registration_persistent();
734738
$registration->set('sourceid', $sourceid);
735739
$registration->set('sourceguid', $sourceguid);
736-
}
740+
}
737741
$registration->set('enrolid', $enrolmentinstance->id);
738742
$registration->set('attendance', $resource->Attendance);
739743
$registration->set('outcome', $resource->Outcome);
@@ -754,8 +758,8 @@ public static function save_resource_information_to_persistents($enrolmentinstan
754758
$registration->set('sourceonlineactivityid', $onlineactivityresource->OnlineActivityID);
755759
$registration->set('sourceonlineactivityguid', $onlineactivityresource->UniqueIdentifier);
756760
}
757-
// Check for existing contact record.
758-
$contact = $registration->get_contact();
761+
// Check for existing contact record. Reuse the lookup we did above when available.
762+
$contact = $existingcontact ?: $registration->get_contact();
759763
// Create new contact.
760764
if (!$contact) {
761765
$contact = new contact_persistent();
@@ -785,8 +789,7 @@ public static function save_resource_information_to_persistents($enrolmentinstan
785789
$registration->set('errormessage', '');
786790
$registration->set('enrolmentfailure', 0);
787791
$registration->save();
788-
// Return registration and contact persistents.
789-
return array($registration, $contact);
792+
return [$registration, $contact, false];
790793
}
791794

792795
}

‎classes/local/persistent/user_persistent.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ protected static function define_properties() {
171171
* @throws \dml_exception
172172
* @throws coding_exception
173173
*/
174-
public function __construct($id = 0, stdClass $record = null) {
174+
public function __construct($id = 0, ?stdClass $record = null) {
175175
global $DB;
176176

177177
if ($id > 0) {

‎classes/manager.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class manager {
3434
/** @var \progress_trace */
3535
private static $trace;
3636

37-
public function __construct(\progress_trace $trace = null) {
37+
public function __construct(?\progress_trace $trace = null) {
3838
// Raise limits, so this script can be interrupted without problems.
3939
\core_php_time_limit::raise();
4040
raise_memory_limit(MEMORY_HUGE);

‎classes/persistent.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ abstract class persistent {
5858
* @param int $id If set, this is the id of an existing record, used to load the data.
5959
* @param stdClass $record If set will be passed to {@link self::from_record()}.
6060
*/
61-
public function __construct($id = 0, stdClass $record = null) {
61+
public function __construct($id = 0, ?stdClass $record = null) {
6262
global $CFG;
6363

6464
if ($id > 0) {
@@ -946,7 +946,7 @@ public static function record_exists($id) {
946946
* @param array $params
947947
* @return bool
948948
*/
949-
public static function record_exists_select($select, array $params = null) {
949+
public static function record_exists_select($select, ?array $params = null) {
950950
global $DB;
951951
return $DB->record_exists_select(static::TABLE, $select, $params);
952952
}

‎lib.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ public function add_default_instance($course) {
227227
* @throws moodle_exception
228228
* @throws required_capability_exception
229229
*/
230-
public function add_instance($course, array $fields = null) {
230+
public function add_instance($course, ?array $fields = null) {
231231
$pluginconfig = new arlo_plugin_config();
232232
$fields['roleid'] = $pluginconfig->get('roleid');
233233
$fields['customchar1'] = $pluginconfig->get('platform');

‎tests/contact_merge_requests_test.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
use enrol_arlo\local\persistent\user_persistent;
3030
use enrol_arlo\local\handler\contact_merge_requests_handler;
3131

32-
class enrol_arlo_contact_merge_requests_testcase extends advanced_testcase {
32+
class contact_merge_requests_test extends advanced_testcase {
3333

3434
public function test_no_merge_requests() {
3535
global $CFG, $DB;

0 commit comments

Comments
 (0)