Skip to content

Commit

Permalink
Merge pull request #181 from catalyst/ignore-case-option-403
Browse files Browse the repository at this point in the history 
Ignore case option 403
  • Loading branch information
@Peterburnett
Peterburnett authored Sep 9, 2024
2 parents 38b8c45 + e341bb7 commit ea8e9b0
Show file tree
Hide file tree
Showing 8 changed files with 169 additions and 11 deletions.
30 changes: 25 additions & 5 deletions classes/booking_manager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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();
Expand All @@ -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) {
Expand Down Expand Up @@ -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.
*
Expand All @@ -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.
Expand Down Expand Up @@ -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;
}
}
4 changes: 4 additions & 0 deletions classes/form/confirm_bookings_form.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand All @@ -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]),
Expand Down
3 changes: 3 additions & 0 deletions classes/form/upload_bookings_form.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions lang/en/facetoface.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
4 changes: 1 addition & 3 deletions lib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
129 changes: 129 additions & 0 deletions tests/upload_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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.'
);
}

/**
Expand All @@ -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' => '[email protected]']);
$this->getDataGenerator()->create_and_enrol($course, 'student', ['email' => '[email protected]']);

$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' => '[email protected]',
'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', '[email protected]')
),
'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' => '[email protected]']);

$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' => '[email protected]',
'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.
*/
Expand Down
5 changes: 4 additions & 1 deletion upload.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions version.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit ea8e9b0

Please sign in to comment.