Skip to content

Commit 221c35a

Browse files
ojnadjarmojnadjarm
committed
ARLO-78: Changing select for autocomplete and fixing merge contact issue
1 parent dac445d commit 221c35a

4 files changed

Lines changed: 139 additions & 31 deletions

File tree

‎admin/contactmergefailure.php‎

Lines changed: 65 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -27,49 +27,87 @@
2727
require_once($CFG->libdir . '/tablelib.php');
2828

2929
$id = required_param('id', PARAM_INT);
30-
admin_externalpage_setup('enrolsettingsarlocontactmergefailure',
31-
null, ['id' => $id], '/enrol/arlo/admin/contactmergefailure.php');
30+
$action = optional_param('action', '', PARAM_ALPHA);
31+
admin_externalpage_setup(
32+
'enrolsettingsarlocontactmergefailure',
33+
null,
34+
['id' => $id],
35+
'/enrol/arlo/admin/contactmergefailure.php'
36+
);
3237
$registration = new \enrol_arlo\local\persistent\registration_persistent($id);
3338
$contact = $registration->get_contact();
3439
$event = $registration->get_event();
3540
$onlineactivity = $registration->get_online_activity();
3641
$code = ($event) ? $event->get('code') : $onlineactivity->get('code');
3742
$returnurl = new moodle_url('/enrol/arlo/admin/unsuccessfulenrolments.php');
3843
$output = $PAGE->get_renderer('enrol_arlo');
44+
// Check for failed contact merge requests first.
45+
$contactmergerequests = \enrol_arlo\local\persistent\contact_merge_request_persistent::get_records(
46+
['destinationcontactid' => $contact->get('sourceid'), 'mergefailed' => 1]
47+
);
48+
if (!$contactmergerequests) {
49+
redirect($returnurl);
50+
}
51+
// Just deal with first.
52+
$contactmergerequest = reset($contactmergerequests);
53+
$sourcecontact = $contactmergerequest->get_source_contact();
54+
$destinationcontact = $contactmergerequest->get_destination_contact();
55+
// When both contacts resolve to the same Moodle user there is nothing to merge,
56+
// so the merge request can be safely marked as complete.
57+
$sameuser = $sourcecontact && $destinationcontact
58+
&& $sourcecontact->get('userid') > 0
59+
&& $sourcecontact->get('userid') == $destinationcontact->get('userid');
60+
if ($action === 'markcomplete' && confirm_sesskey()) {
61+
if (!$sameuser || !enrol_is_enabled('arlo')) {
62+
throw new moodle_exception('invalidrecord');
63+
}
64+
// Mark the merge request as applied.
65+
$contactmergerequest->set('active', 0);
66+
$contactmergerequest->set('mergefailed', 0);
67+
$contactmergerequest->update();
68+
// Re-attempt the enrolment now that the merge request is resolved.
69+
$plugin = \enrol_arlo\api::get_enrolment_plugin();
70+
$enrolmentinstance = $plugin::get_instance_record($registration->get('enrolid'), MUST_EXIST);
71+
$result = \enrol_arlo\local\job\memberships_job::process_enrolment_registration(
72+
$enrolmentinstance,
73+
$registration
74+
);
75+
$message = ($result) ? get_string('success') : get_string('failed', 'enrol_arlo');
76+
redirect($returnurl, $message, 1);
77+
}
3978
echo $OUTPUT->header();
4079
$params = [
4180
'fullname' => $contact->get('firstname') . ' ' . $contact->get('lastname'),
42-
'code' => $code
81+
'code' => $code,
4382
];
4483
$heading = get_string('unsuccessfulenrolmentof', 'enrol_arlo', $params);
4584
echo $OUTPUT->heading(format_string($heading), 3);
46-
// Check for failed contact merge requests first.
47-
$contactmergerequests = \enrol_arlo\local\persistent\contact_merge_request_persistent::get_records(
48-
['destinationcontactid' => $contact->get('sourceid'), 'mergefailed' => 1]
49-
);
50-
if ($contactmergerequests) {
51-
// Just deal with first.
52-
$contactmergerequest = reset($contactmergerequests);
53-
$sourcecontact = new \enrol_arlo\output\contact($contactmergerequest->get_source_contact(), 'source');
54-
$destinationcontact = new \enrol_arlo\output\contact($contactmergerequest->get_destination_contact(), 'destination');
55-
echo $OUTPUT->heading(get_string('contactmergerequestfailure', 'enrol_arlo'), 3);
56-
echo html_writer::start_div('container');
57-
echo html_writer::start_div('row');
58-
echo html_writer::start_div('col-sm-6');
59-
echo $output->render($sourcecontact);
60-
echo html_writer::end_div();
61-
echo html_writer::start_div('col-sm-6');
62-
echo $output->render($destinationcontact);
63-
echo html_writer::end_div();
64-
echo html_writer::end_div();
65-
echo html_writer::end_div();
66-
} else {
67-
redirect($returnurl);
85+
$sourcecontactoutput = new \enrol_arlo\output\contact($sourcecontact, 'source');
86+
$destinationcontactoutput = new \enrol_arlo\output\contact($destinationcontact, 'destination');
87+
echo $OUTPUT->heading(get_string('contactmergerequestfailure', 'enrol_arlo'), 3);
88+
echo html_writer::start_div('container');
89+
echo html_writer::start_div('row');
90+
echo html_writer::start_div('col-sm-6');
91+
echo $output->render($sourcecontactoutput);
92+
echo html_writer::end_div();
93+
echo html_writer::start_div('col-sm-6');
94+
echo $output->render($destinationcontactoutput);
95+
echo html_writer::end_div();
96+
echo html_writer::end_div();
97+
echo html_writer::end_div();
98+
if ($sameuser) {
99+
echo $OUTPUT->notification(get_string('contactmergerequestsameuser', 'enrol_arlo'), 'info');
100+
$markcompleteurl = new moodle_url($PAGE->url, ['action' => 'markcomplete', 'sesskey' => sesskey()]);
101+
$markcompletebutton = new single_button($markcompleteurl, get_string('markmergecomplete', 'enrol_arlo'), 'post');
102+
$markcompletebutton->add_confirm_action(get_string('markmergecompleteconfirm', 'enrol_arlo'));
103+
echo $OUTPUT->render($markcompletebutton);
68104
}
69105
echo html_writer::start_div('row float-right');
70106
echo html_writer::start_tag('h4');
71-
echo $OUTPUT->action_link($returnurl,
72-
get_string('returntounsucessfulenrolments', 'enrol_arlo'));
107+
echo $OUTPUT->action_link(
108+
$returnurl,
109+
get_string('returntounsucessfulenrolments', 'enrol_arlo')
110+
);
73111
echo html_writer::end_tag('h4');
74112
echo html_writer::end_div();
75113
echo $OUTPUT->footer();

‎lang/en/enrol_arlo.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,10 @@
8181
$string['codeprimary'] = 'Code primary';
8282
$string['contactmergerequestfailure'] = 'Failure to apply contact merge request';
8383
$string['contactmergerequestfailures'] = 'Contact merge request failures';
84+
$string['contactmergerequestsameuser'] = 'Both contacts are linked to the same Moodle user account, probably due to a user merge inside Moodle or a change of email address. There is nothing to merge, so it is safe to mark this merge as complete.';
8485
$string['contactmergefailurereport'] = 'Contact merge report';
86+
$string['markmergecomplete'] = 'Mark merge as complete';
87+
$string['markmergecompleteconfirm'] = 'Both contacts are linked to the same Moodle user, so there is nothing to merge. Mark this merge request as complete?';
8588
$string['communications'] = 'Communications';
8689
$string['completed'] = 'Completed';
8790
$string['courseid'] = 'Course ID';

‎lib.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -701,7 +701,7 @@ public function edit_instance_form($instance, MoodleQuickForm $mform, $context)
701701
$eventoptions = [
702702
$persistent->get('sourceguid') => $persistent->get('code')
703703
];
704-
$mform->addElement('select', 'arloevent', get_string('event', 'enrol_arlo'),
704+
$mform->addElement('autocomplete', 'arloevent', get_string('event', 'enrol_arlo'),
705705
$eventoptions);
706706
$mform->setConstant('arloevent', $instance->customchar3);
707707
$mform->hardFreeze('arloevent', $instance->customchar3);
@@ -723,7 +723,7 @@ public function edit_instance_form($instance, MoodleQuickForm $mform, $context)
723723
$eventoptions = [
724724
$persistent->get('sourceguid') => $persistent->get('code')
725725
];
726-
$mform->addElement('select', 'arloonlineactivity', get_string('onlineactivity',
726+
$mform->addElement('autocomplete', 'arloonlineactivity', get_string('onlineactivity',
727727
'enrol_arlo'), $eventoptions);
728728
$mform->setConstant('arloonlineactivity', $instance->customchar3);
729729
$mform->hardFreeze('arloonlineactivity', $instance->customchar3);
@@ -743,12 +743,12 @@ public function edit_instance_form($instance, MoodleQuickForm $mform, $context)
743743
$mform->addElement('select', 'arlotype', get_string('type', 'enrol_arlo'), $typeoptions);
744744
// Event selector.
745745
array_unshift($eventoptions, get_string('choose') . '...');
746-
$mform->addElement('select', 'arloevent', get_string('event', 'enrol_arlo'), $eventoptions);
746+
$mform->addElement('autocomplete', 'arloevent', get_string('event', 'enrol_arlo'), $eventoptions);
747747
$mform->disabledIf('arloevent', 'arlotype', 'eq', arlo_type::ONLINEACTIVITY);
748748
$mform->disabledIf('arloevent', 'arlotype', 'eq', 0);
749749
// Online Activity selector.
750750
array_unshift($onlineactivityoptions, get_string('choose') . '...');
751-
$mform->addElement('select', 'arloonlineactivity',
751+
$mform->addElement('autocomplete', 'arloonlineactivity',
752752
get_string('onlineactivity', 'enrol_arlo'), $onlineactivityoptions);
753753
$mform->disabledIf('arloonlineactivity', 'arlotype', 'eq', arlo_type::EVENT);
754754
$mform->disabledIf('arloonlineactivity', 'arlotype', 'eq', 0);

‎tests/contact_merge_requests_test.php‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,73 @@ public function test_both_source_and_destination_have_enrolments() {
108108
$this->assertEquals(false, $result);
109109
}
110110

111+
/**
112+
* A merge request where both contacts already resolve to the same Moodle user.
113+
*
114+
* @covers \enrol_arlo\local\handler\contact_merge_requests_handler::apply_all_merge_requests
115+
*/
116+
public function test_source_and_destination_share_same_user(): void {
117+
global $CFG, $DB;
118+
require_once($CFG->dirroot . '/enrol/arlo/lib.php');
119+
/** @var enrol_arlo_generator $plugingenerator */
120+
$plugingenerator = $this->getDataGenerator()->get_plugin_generator('enrol_arlo');
121+
$this->resetAfterTest();
122+
123+
// One Moodle user, email already updated to the new address.
124+
$userinfo = new stdClass();
125+
$userinfo->firstname = 'Jane';
126+
$userinfo->lastname = 'Doe';
127+
$userinfo->email = 'jane.new@example.com';
128+
$user = $this->getDataGenerator()->create_user($userinfo);
129+
130+
// Old contact, old email, linked to the user.
131+
$sourceinfo = new stdClass();
132+
$sourceinfo->firstname = 'Jane';
133+
$sourceinfo->lastname = 'Doe';
134+
$sourceinfo->email = 'jane.old@example.com';
135+
136+
$sourcecontact = $plugingenerator->create_contact($sourceinfo);
137+
138+
// Associate contact and user.
139+
$sourcecontact->set('userid', $user->id);
140+
$sourcecontact->update();
141+
142+
// New contact, new email, matched to the same user.
143+
$destinationinfo = new stdClass();
144+
$destinationinfo->firstname = 'Jane';
145+
$destinationinfo->lastname = 'Doe';
146+
$destinationinfo->email = 'jane.new@example.com';
147+
148+
$destinationcontact = $plugingenerator->create_contact($destinationinfo);
149+
150+
// Associate contact and user.
151+
$destinationcontact->set('userid', $user->id);
152+
$destinationcontact->update();
153+
154+
// Create a contact merge request.
155+
$contactmergerequest = $plugingenerator->create_contact_merge_request($sourcecontact, $destinationcontact);
156+
157+
// A single enrolment, counted on both sides because it is the same user.
158+
$manualplugin = enrol_get_plugin('manual');
159+
160+
$studentrole = $DB->get_record('role', ['shortname' => 'student']);
161+
$this->assertNotEmpty($studentrole);
162+
163+
$category = $this->getDataGenerator()->create_category();
164+
$course = $this->getDataGenerator()->create_course(['category' => $category->id]);
165+
$manualinstance = $DB->get_record('enrol', ['courseid' => $course->id, 'enrol' => 'manual'], '*', MUST_EXIST);
166+
167+
$manualplugin->enrol_user($manualinstance, $user->id, $studentrole->id);
168+
169+
$handler = new contact_merge_requests_handler($destinationcontact);
170+
$result = $handler->apply_all_merge_requests();
171+
172+
// Current behaviour: the merge fails even though there is nothing to merge.
173+
$this->assertEquals(false, $result);
174+
$contactmergerequest->read();
175+
$this->assertEquals(1, $contactmergerequest->get('mergefailed'));
176+
}
177+
111178
public function test_source_has_user_and_destination_has_enrolments() {
112179
global $CFG, $DB;
113180
require_once($CFG->dirroot . '/enrol/arlo/lib.php');

0 commit comments

Comments
 (0)