Skip to content

Commit

Permalink
Merge pull request #187 from catalyst/validate-signuptype-403
Browse files Browse the repository at this point in the history 
Validate user sessions on bulk upload when f2f type is single
  • Loading branch information
@matthewhilton
matthewhilton authored Dec 4, 2024
2 parents 7d4e242 + 73e3d9d commit cacca1a
Show file tree
Hide file tree
Showing 4 changed files with 186 additions and 12 deletions.
50 changes: 50 additions & 0 deletions classes/booking_manager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,8 @@ public function validate($timenow = null): array {
$sessioncapacitycache = [];
$timenow ??= time();

$usersessions = [];

// Break into rows and validate the multiple interdependant fields together.
foreach ($this->get_iterator() as $index => $entry) {
$row = $index + 1;
Expand Down Expand Up @@ -241,6 +243,21 @@ public function validate($timenow = null): array {
$sessioncapacitycache[$session->id]['capacity']--;
$sessioncapacitycache[$session->id]['rows'][] = $row;
}

// Don't allow users to signup to another session if the signup type is not multiple.
if (isset($userid) && $entry->status !== 'cancelled' && $this->facetoface->signuptype != MOD_FACETOFACE_SIGNUP_MULTIPLE) {
if ($currusersessions = facetoface_get_user_submissions($this->f, $userid)) {
foreach ($currusersessions as $currusersession) {
if ($currusersession->sessionid != $session->id) {
$errors[] = [
$row,
new lang_string('error:addalreadysignedupattendee', 'mod_facetoface', $entry->email),
];
break;
}
}
}
}
}

// Check user enrolment into the course.
Expand Down Expand Up @@ -273,6 +290,39 @@ public function validate($timenow = null): array {
new lang_string('error:invalidstatusspecified', 'mod_facetoface', $entry->status),
];
}

// If the user exists and this isn't a cancellation, we need to store the distinct session for checking at the end.
if (isset($userid) && $entry->status !== 'cancelled') {
// Set the user sessions if it hasn't been set yet.
if (!isset($usersessions[$userid])) {
$usersessions[$userid] = [
'email' => $entry->email,
'rows' => [],
'sessions' => [],
];
}

$usersessions[$userid]['rows'][] = $row;

if ($session && !in_array($session->id, $usersessions[$userid]['sessions'])) {
$usersessions[$userid]['sessions'][] = $session->id;
}
}
}

// If the signup type is not set to multiple, we need to create errors for users being added to multiple sessions.
if ($this->facetoface->signuptype != MOD_FACETOFACE_SIGNUP_MULTIPLE) {
// Get all users being added to more than 1 session.
$doublebookedusers = array_filter($usersessions, function($us) {
return count($us['sessions']) > 1;
});
// Create errors for the user rows.
foreach ($doublebookedusers as $details) {
$errors[] = [
implode(', ', $details['rows']),
new lang_string('error:multipleusersessions', 'mod_facetoface', $details['email']),
];
}
}

// For all sessions that went over capacity, report it.
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 @@ -153,6 +153,7 @@
$string['error:sessionoverbooked'] = 'Session ID {$a->session} overbooked by {$a->amount} person(s).';
$string['error:sessiondoesnotexist'] = 'Session ID {$a} does not exist';
$string['error:userdoesnotexist'] = 'User {$a} does not exist';
$string['error:multipleusersessions'] = 'User {$a} has more than one session';
$string['error:multipleusersmatched'] = 'Multiple users matched to identifier {$a}';
$string['error:addalreadysignedupattendee'] = '{$a} is already signed-up for this Face-to-Face activity.';
$string['error:addattendee'] = 'Could not add {$a} to the session.';
Expand Down
143 changes: 133 additions & 10 deletions tests/upload_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ public function test_user_validation() {

// Generate users.
$user = $this->getDataGenerator()->create_user();
$student = $this->getDataGenerator()->create_and_enrol($course, 'student');
$student1 = $this->getDataGenerator()->create_and_enrol($course, 'student');
$student2 = $this->getDataGenerator()->create_and_enrol($course, 'student');
$student3 = $this->getDataGenerator()->create_and_enrol($course, 'student');

$this->setCurrentTimeStart();
$now = time();
Expand All @@ -156,6 +158,20 @@ public function test_user_validation() {
],
]);

$secondsession = $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 + 4 * DAYSECS, 'timefinish' => $now + 5 * DAYSECS],
],
]);

$remotesession = $generator->create_session([
'facetoface' => $anotherfacetoface->id,
'capacity' => '3',
Expand All @@ -170,6 +186,9 @@ public function test_user_validation() {
],
]);

facetoface_user_signup($session, $facetoface, $course, '',
MDL_F2F_TEXT, MDL_F2F_STATUS_BOOKED, $student3->id);

$bm = new booking_manager($facetoface->id);

$records = [
Expand All @@ -191,36 +210,52 @@ public function test_user_validation() {
],
// Test student who is enrolled into the course (no issues).
(object) [
'email' => $student->email,
'email' => $student1->email,
'session' => $session->id,
'status' => '',
'notificationtype' => '',
'discountcode' => '',
],
// Test invalid options.
(object) [
'email' => $student->email,
'email' => $student2->email,
'session' => $session->id,
'status' => 'helloworld',
'notificationtype' => 'phone',
'discountcode' => '',
],
// Test multiple rows for the same student when f2f signup type is single.
(object) [
'email' => $student2->email,
'session' => $secondsession->id,
'status' => '',
'notificationtype' => '',
'discountcode' => '',
],
// Test permissions (e.g. user not able to upload/process for a f2f activity loaded).
(object) [
'email' => $student->email,
'email' => $student2->email,
'session' => $remotesession->id,
'status' => '',
'notificationtype' => '',
'discountcode' => '',
],
// Test email does not match case-wise (in the default case sensitive mode).
(object) [
'email' => strtoupper($student->email),
'email' => strtoupper($student2->email),
'session' => $session->id,
'status' => '',
'notificationtype' => '',
'discountcode' => '',
],
// Test student already in a session when f2f signup type is single.
(object) [
'email' => $student3->email,
'session' => $secondsession->id,
'status' => '',
'notificationtype' => '',
'discountcode' => '',
],
];

$bm->load_from_array($records);
Expand Down Expand Up @@ -274,33 +309,52 @@ public function test_user_validation() {
$this->assertTrue(
$this->check_row_validation_error_exists(
$errors,
5,
'4, 5, 6, 7',
new lang_string('error:multipleusersessions', 'mod_facetoface', $student2->email)
),
'Expecting multiple sessions error for user when f2f signup type is single.'
);

$this->assertTrue(
$this->check_row_validation_error_exists(
$errors,
6,
new lang_string('error:tryingtoupdatesessionfromanothermodule', 'mod_facetoface', (object) [
'session' => $remotesession->id,
'f' => $facetoface->id,
])
),
'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))
7,
new lang_string('error:userdoesnotexist', 'mod_facetoface', strtoupper($student2->email))
),
'Expecting user to not exist because email does not match case-wise.'
);

$this->assertTrue(
$this->check_row_validation_error_exists(
$errors,
8,
new lang_string('error:addalreadysignedupattendee', 'mod_facetoface', $student3->email)
),
'Expecting user already signed up to another session error.'
);
}

/**
* Helper function to check if a specific error exists in the array of errors.
*
* @param array $errors Array of errors.
* @param int $expectedrownumber Expected row number.
* @param string $expectedrownumber Expected row number string.
* @param string $expectederrormsg Expected error message.
* @return bool True if the error exists, false otherwise.
*/
private function check_row_validation_error_exists(array $errors, int $expectedrownumber, string $expectederrormsg): bool {
private function check_row_validation_error_exists(array $errors, string $expectedrownumber, string $expectederrormsg): bool {
foreach ($errors as $error) {
// Note: row number is based on a CSV file human readable format, where there is a header and row data.
[$rownumber, $errormsg] = $error;
Expand Down Expand Up @@ -758,4 +812,73 @@ public function test_email_suppression(string $status, bool $shouldsuppress) {
$this->assertNotEmpty($emails);
}
}

/**
* Test upload processing multiple sessions with signup type set to multiple.
*/
public function test_processing_signup_multiple() {
/** @var \mod_facetoface_generator $generator */
$generator = $this->getDataGenerator()->get_plugin_generator('mod_facetoface');

$course = $this->getDataGenerator()->create_course();
// Facetoface with signuptype set to multiple.
$facetoface = $generator->create_instance(['course' => $course->id, 'signuptype' => MOD_FACETOFACE_SIGNUP_MULTIPLE]);
$student = $this->getDataGenerator()->create_and_enrol($course, 'student');

$now = time();
$record = [
'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 + 4 * DAYSECS],
],
];
$session1 = $generator->create_session($record);
$session2 = $generator->create_session($record);
$session3 = $generator->create_session($record);

// Signup student to session3 to test already signed up validating.
facetoface_user_signup($session3, $facetoface, $course, '',
MDL_F2F_TEXT, MDL_F2F_STATUS_BOOKED, $student->id);

$bm = new booking_manager($facetoface->id);

$records = [
// Test user can be added to session 1.
(object) [
'email' => $student->email,
'session' => $session1->id,
'status' => '',
'notificationtype' => '',
'discountcode' => '',
],
// Test user can be added to session 2.
(object) [
'email' => $student->email,
'session' => $session2->id,
'status' => '',
'notificationtype' => '',
'discountcode' => '',
],
];

$bm->load_from_array($records);
$errors = $bm->validate();

$this->assertEmpty($errors);
$this->assertTrue($bm->process());

$sessions = facetoface_get_user_submissions($facetoface->id, $student->id);
$this->assertCount(3, $sessions);
$sessionids = array_column($sessions, 'sessionid');
$this->assertContains($session1->id, $sessionids);
$this->assertContains($session2->id, $sessionids);
$this->assertContains($session3->id, $sessionids);
}
}
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 = 2024101100;
$plugin->release = 2024101100;
$plugin->version = 2024101101;
$plugin->release = 2024101101;
$plugin->requires = 2023100900; // Requires 4.3.
$plugin->component = 'mod_facetoface';
$plugin->maturity = MATURITY_STABLE;
Expand Down

0 comments on commit cacca1a

Please sign in to comment.