From 6779214a8fb088a161e64c155c3cbdcbcdbcdf9d Mon Sep 17 00:00:00 2001 From: Matthew Hilton Date: Thu, 5 Sep 2024 08:58:03 +1000 Subject: [PATCH 1/3] feat: add case insensitive upload option --- classes/booking_manager.php | 30 +++++- classes/form/confirm_bookings_form.php | 4 + classes/form/upload_bookings_form.php | 3 + lang/en/facetoface.php | 1 + tests/upload_test.php | 129 +++++++++++++++++++++++++ upload.php | 5 +- 6 files changed, 166 insertions(+), 6 deletions(-) diff --git a/classes/booking_manager.php b/classes/booking_manager.php index af97e53..53cc314 100644 --- a/classes/booking_manager.php +++ b/classes/booking_manager.php @@ -56,6 +56,9 @@ class booking_manager { /** @var bool When true, confirmation emails are not sent. */ private $suppressemail = false; + /** @var bool Will ignore case when matching users */ + private $caseinsensitive = false; + /** * Constructor for the booking manager. * @param int $f The facetoface module ID. @@ -165,7 +168,6 @@ private function get_iterator(): \Generator { * @return array An array of errors. */ public function validate($timenow = null): array { - global $DB; $errors = []; $sessioncapacitycache = []; $timenow ??= time(); @@ -180,7 +182,7 @@ public function validate($timenow = null): array { $entry->discountcode = $entry->discountcode ?? ''; // Validate and get user. - $userids = $DB->get_records('user', ['email' => $entry->email], 'id'); + $userids = $this->match_users($entry->email, 'id'); // Multiple matched, ambiguous which is the real one. if (count($userids) > 1) { @@ -293,6 +295,18 @@ public function validate($timenow = null): array { return $errors; } + /** + * Match users for a given email, taking into account case sensitivity. + * @param string $email + * @param string $fields fields to return + * @return array of users, with specified fields + */ + private function match_users(string $email, string $fields): array { + global $DB; + $equals = $DB->sql_equal('email', ':email', !$this->caseinsensitive); + return $DB->get_records_select('user', $equals, ['email' => $email], 'id', $fields); + } + /** * Transform notification type to internal representation. * @@ -318,15 +332,13 @@ private function transform_notification_type($type) { * @throws moodle_exception */ public function process() { - global $DB; - if (!empty($this->validate())) { throw new moodle_exception('error:cannotprocessbookingsvalidationerrorsexist', 'facetoface'); } // Records should be valid at this point. foreach ($this->get_iterator() as $entry) { - $user = $DB->get_record('user', ['email' => $entry->email]); + $user = current($this->match_users($entry->email, '*')); $session = facetoface_get_session($entry->session); // Get signup type. @@ -400,4 +412,12 @@ public function process() { public function suppress_email() { $this->suppressemail = true; } + + /** + * Sets case insensitive match value + * @param bool $value + */ + public function set_case_insensitive(bool $value) { + $this->caseinsensitive = $value; + } } diff --git a/classes/form/confirm_bookings_form.php b/classes/form/confirm_bookings_form.php index 047118c..834a95a 100644 --- a/classes/form/confirm_bookings_form.php +++ b/classes/form/confirm_bookings_form.php @@ -43,6 +43,7 @@ public function definition() { $mform = $this->_form; $fileid = $this->_customdata['fileid'] ?? 0; $f = $this->_customdata['f'] ?? 0; + $caseinsensitive = $this->_customdata['caseinsensitive'] ?? true; // Suppress email checkbox. $mform->addElement('advcheckbox', 'suppressemail', get_string('suppressemail', 'facetoface'), '', [], [0, 1]); @@ -57,6 +58,9 @@ public function definition() { $mform->addElement('hidden', 'fileid', $fileid); $mform->setType('fileid', PARAM_INT); + $mform->addElement('hidden', 'caseinsensitive', $caseinsensitive); + $mform->setType('caseinsensitive', PARAM_BOOL); + $backurl = new moodle_url('/mod/facetoface/upload.php', ['f' => $f]); $htmlbuttons = $OUTPUT->render((new single_button( new moodle_url('/mod/facetoface/upload.php', ['f' => $f, 'fileid' => $fileid, 'process' => 1]), diff --git a/classes/form/upload_bookings_form.php b/classes/form/upload_bookings_form.php index 20a65e2..4fdad84 100644 --- a/classes/form/upload_bookings_form.php +++ b/classes/form/upload_bookings_form.php @@ -63,6 +63,9 @@ public function definition() { $mform->addElement('static', 'csvuploadhelp', '', nl2br(get_string('facetoface:uploadbookingsfiledesc', 'mod_facetoface'))); + $mform->addElement('advcheckbox', 'caseinsensitive', get_string('caseinsensitive', 'mod_facetoface')); + $mform->setDefault('caseinsensitive', true); + // The facetoface module ID. $mform->addElement('hidden', 'f'); $mform->setType('f', PARAM_INT); diff --git a/lang/en/facetoface.php b/lang/en/facetoface.php index 284a140..9cdd69e 100644 --- a/lang/en/facetoface.php +++ b/lang/en/facetoface.php @@ -833,3 +833,4 @@ $string['waitliststatus'] = 'You have a place on the waitlist of the following session'; $string['addtoallsessions'] = 'Add users to all (upcoming) sessions'; $string['addtoallsessions_help'] = 'Use this option if you want to add users to all upcoming Face-to-Face sessions. When this uption is toggled, the selected users will be added to this session and all other future sessions in the activity.'; +$string['caseinsensitive'] = 'Case insensitive'; diff --git a/tests/upload_test.php b/tests/upload_test.php index b7a4a83..a475849 100644 --- a/tests/upload_test.php +++ b/tests/upload_test.php @@ -213,6 +213,14 @@ public function test_user_validation() { 'notificationtype' => '', 'discountcode' => '', ], + // Test email does not match case-wise (in the default case sensitive mode). + (object) [ + 'email' => strtoupper($student->email), + 'session' => $session->id, + 'status' => '', + 'notificationtype' => '', + 'discountcode' => '', + ], ]; $bm->load_from_array($records); @@ -274,6 +282,14 @@ public function test_user_validation() { ), 'Expecting permission check conflict due to session->facetoface + facetoface id mismatcherror.' ); + $this->assertTrue( + $this->check_row_validation_error_exists( + $errors, + 6, + new lang_string('error:userdoesnotexist', 'mod_facetoface', strtoupper($student->email)) + ), + 'Expecting user to not exist because email does not match case-wise.' + ); } /** @@ -296,6 +312,119 @@ private function check_row_validation_error_exists(array $errors, int $expectedr return false; } + /** + * Tests uploading booking fails if it matches multiple users when ignoring case. + */ + public function test_processing_booking_case_insensitive_match_multiple() { + /** @var \mod_facetoface_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('mod_facetoface'); + + $course = $this->getDataGenerator()->create_course(); + $facetoface = $generator->create_instance(['course' => $course->id]); + + // Create two users, with emails that would be the same when made lowercase. + $this->getDataGenerator()->create_and_enrol($course, 'student', ['email' => 'test@test.com']); + $this->getDataGenerator()->create_and_enrol($course, 'student', ['email' => 'TEST@test.com']); + + $this->setCurrentTimeStart(); + $now = time(); + $session = $generator->create_session([ + 'facetoface' => $facetoface->id, + 'capacity' => '3', + 'allowoverbook' => '0', + 'details' => 'xyz', + 'duration' => '3.0', + 'normalcost' => '111', + 'discountcost' => '11', + 'allowcancellations' => '0', + 'sessiondates' => [ + ['timestart' => $now + 3 * DAYSECS, 'timefinish' => $now + 2 * DAYSECS], + ], + ]); + + $bm = new booking_manager($facetoface->id); + $record = (object) [ + 'email' => 'TEST@test.com', + 'session' => $session->id, + 'status' => 'booked', + 'notificationtype' => 'ical', + 'discountcode' => 'mycode', + ]; + $records = [$record]; + $bm->set_case_insensitive(true); + + $bm->load_from_array($records); + + $errors = $bm->validate(); + $this->assertNotEmpty($errors); + + $this->assertTrue( + $this->check_row_validation_error_exists( + $errors, + 1, + new lang_string('error:multipleusersmatched', 'mod_facetoface', 'TEST@test.com') + ), + 'Expecting to error due to matching multiple users when ignoring case.', + ); + } + + /** + * Tests uploading a booking where the emails should match regardless of case. + */ + public function test_processing_booking_case_insensitive() { + /** @var \mod_facetoface_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('mod_facetoface'); + + $course = $this->getDataGenerator()->create_course(); + $facetoface = $generator->create_instance(['course' => $course->id]); + $this->getDataGenerator()->create_and_enrol($course, 'student', ['email' => 'test@test.com']); + + $this->setCurrentTimeStart(); + $now = time(); + $session = $generator->create_session([ + 'facetoface' => $facetoface->id, + 'capacity' => '3', + 'allowoverbook' => '0', + 'details' => 'xyz', + 'duration' => '1.5', // One and half hours. + 'normalcost' => '111', + 'discountcost' => '11', + 'allowcancellations' => '0', + 'sessiondates' => [ + ['timestart' => $now + 3 * DAYSECS, 'timefinish' => $now + 2 * DAYSECS], + ], + ]); + + $bm = new booking_manager($facetoface->id); + $record = (object) [ + 'email' => 'TEST@test.com', + 'session' => $session->id, + 'status' => 'booked', + 'notificationtype' => 'ical', + 'discountcode' => 'MYSPECIALCODE', + ]; + $records = [$record]; + $bm->set_case_insensitive(true); + + $bm->load_from_array($records); + + $errors = $bm->validate(); + $this->assertEmpty($errors); + $this->assertTrue($bm->process()); + + // Check users are as expected. + $users = facetoface_get_attendees($session->id); + $this->assertCount(1, $users); + $this->assertEquals(strtolower($record->email), strtolower(current($users)->email)); + $this->assertEquals($record->discountcode, current($users)->discountcode); + $this->assertEquals(MDL_F2F_ICAL, current($users)->notificationtype); + $this->assertEquals(MDL_F2F_STATUS_BOOKED, current($users)->statuscode); + + // Re-booking the same user shouldn't cause any isssues. Run the validate again and check. + $errors = $bm->validate(); + $this->assertEmpty($errors); + } + /** * Test upload processing to ensure the happy path is working as expected, and users can be booked into a session. */ diff --git a/upload.php b/upload.php index 840a7e2..c7746e1 100644 --- a/upload.php +++ b/upload.php @@ -36,6 +36,7 @@ $validate = optional_param('validate', 0, PARAM_INT); // Whether or not the user wants to process the upload (after verification). $process = optional_param('process', 0, PARAM_INT); // Whether or not the user wants to process the upload (after verification). $step = optional_param('step', '', PARAM_ALPHA); // The current step in the process. +$caseinsensitive = optional_param('caseinsensitive', false, PARAM_BOOL); // If emails should match a user case insensitively. if (!$facetoface = $DB->get_record('facetoface', ['id' => $f])) { throw new moodle_exception('error:incorrectfacetofaceid', 'facetoface'); @@ -63,10 +64,11 @@ $data = $mform->get_data(); $fileid = $data->csvfile ?: 0; - $mform = new confirm_bookings_form(null, ['f' => $f, 'fileid' => $fileid]); + $mform = new confirm_bookings_form(null, ['f' => $f, 'fileid' => $fileid, 'caseinsensitive' => $caseinsensitive]); $bm = new booking_manager($f); $bm->load_from_file($fileid); + $bm->set_case_insensitive($caseinsensitive); // Validate entries. $errors = $bm->validate(); @@ -76,6 +78,7 @@ // Form submitted, and ready for processing -> process. $bm = new booking_manager($f); $bm->load_from_file($fileid); + $bm->set_case_insensitive($caseinsensitive); // Get the options selected by the user at confirm time. $confirmdata = (new confirm_bookings_form(null))->get_data(); From d4d33352c820a5a4d4b38a730d7dfde541ebfee3 Mon Sep 17 00:00:00 2001 From: Matthew Hilton Date: Thu, 5 Sep 2024 08:58:14 +1000 Subject: [PATCH 2/3] chore: fix type in phpdoc --- lib.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib.php b/lib.php index 8710a8e..e4ca059 100644 --- a/lib.php +++ b/lib.php @@ -91,9 +91,7 @@ /** * Returns the list of possible facetoface status. - * - * @param int $statuscode One of the MDL_F2F_STATUS* constants - * @return string $string Human readable code + * @return array $string Human readable code */ function facetoface_statuses() { // This array must match the status codes above, and the values From e341bb7b289c0881a055e1d8e1639805a318abe2 Mon Sep 17 00:00:00 2001 From: Matthew Hilton Date: Thu, 5 Sep 2024 09:03:43 +1000 Subject: [PATCH 3/3] chore: bump version --- version.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/version.php b/version.php index 9b0000a..f5e6a48 100644 --- a/version.php +++ b/version.php @@ -30,8 +30,8 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2024070900; -$plugin->release = 2024070900; +$plugin->version = 2024090600; +$plugin->release = 2024090600; $plugin->requires = 2023100900; // Requires 4.3. $plugin->component = 'mod_facetoface'; $plugin->maturity = MATURITY_STABLE;