diff --git a/classes/form/oneroster_csv_form.php b/classes/form/oneroster_csv_form.php index bedad28..c1daea8 100644 --- a/classes/form/oneroster_csv_form.php +++ b/classes/form/oneroster_csv_form.php @@ -41,7 +41,7 @@ protected function definition() { // Submit button. $this->add_action_buttons( true, - 'Upload' + get_string('upload', 'enrol_oneroster') ); } } diff --git a/classes/local/csv_client.php b/classes/local/csv_client.php index 5e69b4c..6212c08 100644 --- a/classes/local/csv_client.php +++ b/classes/local/csv_client.php @@ -37,25 +37,21 @@ class csv_client implements client_interface { use root_oneroster_client; use versioned_client; - // Define constants for the base paths and types - const basepath_orgs = 'orgs'; - const basepath_schools = 'schools'; - const type_terms = 'terms'; - const type_classes = 'classes'; - const type_enrollments = 'enrollments'; - const basepath_users = 'users'; + // Define constants for the base paths and types. + const BASEPATH_ORGS = 'orgs'; + const BASEPATH_SCHOOLS = 'schools'; + const TYPE_TERMS = 'terms'; + const TYPE_CLASSES = 'classes'; + const TYPE_ENROLLMENTS = 'enrollments'; + const BASEPATH_USERS = 'users'; private $org_id; - // Define constants for keys - const academic_sessions_key = 'academicSessions'; - const orgs_key = 'orgs'; - const classes_key = 'classes'; - const enrollments_key = 'enrollments'; - const users_key = 'users'; - const periods_key = 'periods'; - const subjects_key = 'subjects'; - const subject_codes_key = 'subjectCodes'; - const grades_key = 'grades'; + // Define constants for keys. + const ACADEMIC_SESSIONS_KEY = 'academicSessions'; + const PERIODS_KEY = 'periods'; + const SUBJECTS_KEYS = 'subjects'; + const SUBJECT_CODES_KEY = 'subjectCodes'; + const GRADES_KEY = 'grades'; /** * Authenticate the client. This is a no-op for the CSV client. @@ -112,15 +108,15 @@ public function set_org_id($org_id) { */ public function execute(command $command, ?filter $filter = null): stdClass { $url = $command->get_url(''); - // Split the URL into tokens using '/' as the delimiter (eg. /schools/org-sch-222-456/terms) + // Split the URL into tokens using '/' as the delimiter (eg. /schools/org-sch-222-456/terms). $tokens = explode('/', $url); - // The second token represents the base path ('users', 'orgs', 'schools') + // The second token represents the base path ('users', 'orgs', 'schools'). $basepath = $tokens[1]; - // The third token represents the Organisation ID + // The third token represents the Organisation ID. $param = $tokens[2] ?? ''; - // The fourth token represents the type of data to fetch ('terms', 'classes', 'enrollments') + // The fourth token represents the type of data to fetch ('terms', 'classes', 'enrollments'). $type = $tokens[3] ?? ''; - // Get the organisation ID + // Get the organisation ID. $org_id = $this->org_id ?? null; if ($org_id == null) { @@ -128,21 +124,21 @@ public function execute(command $command, ?filter $filter = null): stdClass { } switch ($basepath) { - case self::basepath_orgs: - // The endpoint getAllOrgs is called to fetch all organisations + case self::BASEPATH_ORGS: + // The endpoint getAllOrgs is called to fetch all organisations. if ($param === $org_id || $param === '') { - $orgdata = $this->data[self::orgs_key]; + $orgdata = $this->data[self::BASEPATH_ORGS]; $keys = array_map(function($orgs) { return $orgs['sourcedId']; }, $orgdata); - // Combine keys and organization data into an associative array + // Combine keys and organization data into an associative array. $mapped_data = array_combine($keys, $orgdata); if (isset($mapped_data[$org_id])) { $org = (object) $mapped_data[$org_id]; - // If status and dateLastModified are not set, set them to active and the current date + // If status and dateLastModified are not set, set them to active and the current date. if ($org->status == null && $org->dateLastModified == null) { $org->status = 'active'; $org->dateLastModified = date('Y-m-d'); } - // To ensure compatibility with v1.0, set the status to 'tobedeleted' if it is 'inactive' + // To ensure compatibility with v1.0, set the status to 'tobedeleted' if it is 'inactive'. if ($org->status === 'inactive') { $org->status = 'tobedeleted'; } @@ -157,10 +153,10 @@ public function execute(command $command, ?filter $filter = null): stdClass { ]; } - case self::basepath_schools: - // The endpoint getTermsForSchool is called to fetch a list of classes in a term - if ($type === self::type_terms) { - $academicsessiondata = $this->data[self::academic_sessions_key]; + case self::BASEPATH_SCHOOLS: + // The endpoint getTermsForSchool is called to fetch a list of classes in a term. + if ($type === self::TYPE_TERMS) { + $academicsessiondata = $this->data[self::ACADEMIC_SESSIONS_KEY]; $keys = array_map(function ($schools) { return $schools['sourcedId']; }, $academicsessiondata); $mapped_data = array_combine($keys, $academicsessiondata); $academicSession = []; @@ -182,9 +178,9 @@ public function execute(command $command, ?filter $filter = null): stdClass { ]; } - if ($type === self::type_classes) { - // The endpoint getClassesForSchool is called to fetch all students for a class - $classdata = $this->data[self::classes_key]; + if ($type === self::TYPE_CLASSES) { + // The endpoint getClassesForSchool is called to fetch all students for a class. + $classdata = $this->data[self::TYPE_CLASSES]; $keys = array_map(function($schools) { return $schools['sourcedId']; }, $classdata); $mapped_data = array_combine($keys, $classdata); $classes = []; @@ -212,7 +208,7 @@ public function execute(command $command, ?filter $filter = null): stdClass { $class->periods = []; } - $objs = [self::periods_key, self::subjects_key, self::subject_codes_key, self::grades_key]; + $objs = [self::PERIODS_KEY, self::SUBJECTS_KEYS, self::SUBJECT_CODES_KEY, self::GRADES_KEY]; foreach ($objs as $obj) { if (!empty($class->$obj)) { @@ -239,9 +235,9 @@ public function execute(command $command, ?filter $filter = null): stdClass { ]; } - if ($type === self::type_enrollments) { - // The endpoint getEnrollmentsForSchool is called to fetch all enrollments in a school - $enrollmentdata = $this->data[self::enrollments_key]; + if ($type === self::TYPE_ENROLLMENTS) { + // The endpoint getEnrollmentsForSchool is called to fetch all enrollments in a school. + $enrollmentdata = $this->data[self::TYPE_ENROLLMENTS]; $keys = array_map(function($schools) { return $schools['sourcedId']; }, $enrollmentdata); $mapped_data = array_combine($keys, $enrollmentdata); $enrollments = []; @@ -266,9 +262,9 @@ public function execute(command $command, ?filter $filter = null): stdClass { ]; } - case self::basepath_users: - // The endpoint getAllUsers is called to fetch all users in a school - $usersData = $this->data[self::users_key]; + case self::BASEPATH_USERS: + // The endpoint getAllUsers is called to fetch all users in a school. + $usersData = $this->data[self::BASEPATH_USERS]; $keys = array_map(function($user) {return $user['sourcedId']; }, $usersData); $mapped_data = array_combine($keys, $usersData); $users = []; diff --git a/classes/local/csv_client_const_helper.php b/classes/local/csv_client_const_helper.php index b29f236..7eafc53 100644 --- a/classes/local/csv_client_const_helper.php +++ b/classes/local/csv_client_const_helper.php @@ -16,7 +16,7 @@ namespace enrol_oneroster\classes\local; /** - * Class csv_client_const_helper + * Class csv_client_const_helper. * * This class contains constants that are used throughout the OneRoster CSV client. * @@ -25,7 +25,7 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class csv_client_const_helper { - // Individual header constants + // Individual header constants. const HEADER_SOURCEDID = 'sourcedId'; const HEADER_STATUS = 'status'; const HEADER_DATE_LAST_MODIFIED = 'dateLastModified'; @@ -65,7 +65,7 @@ class csv_client_const_helper { const HEADER_AGENT_SOURCEDIDS = 'agentSourcedIds'; const HEADER_PASSWORD = 'password'; - // CSV file names + // CSV file names. const FILE_MANIFEST = 'manifest.csv'; const FILE_ACADEMIC_SESSIONS = 'academicSessions.csv'; const FILE_CLASSES = 'classes.csv'; @@ -73,7 +73,7 @@ class csv_client_const_helper { const FILE_ORGS = 'orgs.csv'; const FILE_USERS = 'users.csv'; - // Define constants for the types + // Define constants for the types. const DATATYPE_NULL = 'null'; const DATATYPE_GUID = 'guid'; const DATATYPE_INT = 'int'; @@ -98,7 +98,7 @@ class csv_client_const_helper { const DATATYPE_PASSWORD = 'password'; const DATATYPE_STRING = 'string'; - // Valid Enum values + // Valid Enum values. const valid_class_types = ['homeroom', 'scheduled']; const valid_roles = ['administrator', 'proctor', 'student', 'teacher']; const valid_primary_values = ['true', 'false']; @@ -107,14 +107,14 @@ class csv_client_const_helper { 'administrator', 'aide', 'guardian', 'parent', 'proctor', 'relative', 'student', 'teacher' ]; - // Valid grade codes from the Common Education Data Standards (CEDS) - // Reference: https://ceds.ed.gov/CEDSElementDetails.aspx?TermId=7100 + // Valid grade codes from the Common Education Data Standards (CEDS). + // Reference: https://ceds.ed.gov/CEDSElementDetails.aspx?TermId=7100. const valid_grade_codes = [ 'IT', 'PR', 'PK', 'TK', 'KG', '01', '02', '03', '04', '05', '06', '07', '08', '09', '10', '11', '12', '13', 'PS', 'UG', 'Other' ]; - // Header constants for each file + // Header constants for each file. const HEADER_ACADEMIC_SESSIONS = [ self::HEADER_SOURCEDID, self::HEADER_STATUS, self::HEADER_DATE_LAST_MODIFIED, self::HEADER_TITLE, self::HEADER_TYPE, self::HEADER_START_DATE, self::HEADER_END_DATE, self::HEADER_PARENT_SOURCEDID, self::HEADER_SCHOOL_YEAR @@ -140,7 +140,7 @@ class csv_client_const_helper { self::HEADER_EMAIL, self::HEADER_SMS, self::HEADER_PHONE, self::HEADER_AGENT_SOURCEDIDS, self::HEADER_GRADES, self::HEADER_PASSWORD ]; - // Required files and their headers + // Required files and their headers. const REQUIRED_FILES = [ self::FILE_ACADEMIC_SESSIONS => self::HEADER_ACADEMIC_SESSIONS, self::FILE_CLASSES => self::HEADER_CLASSES, diff --git a/classes/local/csv_client_helper.php b/classes/local/csv_client_helper.php index 837a434..cf0c4c4 100644 --- a/classes/local/csv_client_helper.php +++ b/classes/local/csv_client_helper.php @@ -26,10 +26,9 @@ */ class csv_client_helper { /** - * Get the expected data types for each file + * Get the expected data types for each file. * - * @return array An array containing the expected data types for each file - * @covers enrol_oneroster/classes/local/csv_client_const_helper + * @return array An array containing the expected data types for each file. */ public static function get_file_datatypes(): array { return [ @@ -104,10 +103,9 @@ public static function get_file_datatypes(): array { ]; } /** - * Get the validator functions for each data type + * Get the validator functions for each data type. * * @return array An array containing the validator functions for each data type - * @covers enrol_oneroster/classes/local/csv_client_const_helper */ public static function get_validator(): array { return [ @@ -137,7 +135,7 @@ public static function get_validator(): array { } /** - * Function to validate CSV headers + * Function to validate CSV headers. * * @param string $file_path Path to the CSV file * @return bool True if the headers are valid, false otherwise @@ -156,13 +154,13 @@ public static function validate_csv_headers(string $file_path): bool { } /** - * Function to check if the manifest and required files are present + * Function to check if the manifest and required files are present. * * @param string $manifest_path Path to the manifest file * @param string $tempdir Path to the temporary directory * @return array An array containing the missing files and invalid headers */ - public static function check_manifest_and_files($manifest_path, $tempdir) { + public static function check_manifest_and_files(string $manifest_path, string $tempdir): array { $invalid_headers = []; $required_files = []; @@ -197,12 +195,12 @@ public static function check_manifest_and_files($manifest_path, $tempdir) { } /** - * Function to extract CSV files to arrays + * Function to extract CSV files to arrays. * * @param string $directory Path to the directory containing the CSV files * @return array An associative array containing the CSV data */ - public static function extract_csvs_to_arrays($directory) { + public static function extract_csvs_to_arrays(string $directory): array { $csv_data = []; $files = scandir($directory); @@ -224,11 +222,11 @@ public static function extract_csvs_to_arrays($directory) { } /** - * Function to display missing and invalid files + * Function to display missing and invalid files. * * @param array $missing_files An array containing the missing files and invalid headers */ - public static function display_missing_and_invalid_files($missing_files) { + public static function display_missing_and_invalid_files(array $missing_files): void { $critical_files = [csv_client_const_helper::FILE_ACADEMIC_SESSIONS, csv_client_const_helper::FILE_CLASSES, csv_client_const_helper::FILE_ENROLLMENTS, csv_client_const_helper::FILE_ORGS, csv_client_const_helper::FILE_USERS]; if (!empty($missing_files['missing_files'])) { @@ -260,12 +258,12 @@ public static function display_missing_and_invalid_files($missing_files) { } /** - * Function to get the header for a given file + * Function to get the header for a given file. * * @param string $file_name The name of the file * @return array The header for the given file */ - public static function getHeader($file_name) { + public static function getHeader(string $file_name): array { switch ($file_name) { case csv_client_const_helper::FILE_ACADEMIC_SESSIONS: return csv_client_const_helper::HEADER_ACADEMIC_SESSIONS; @@ -283,22 +281,22 @@ public static function getHeader($file_name) { } /** - * Get the expected data types for a given file + * Get the expected data types for a given file. * - * @param string $file_name The name of the file + * @param mixed $file_name The name of the file * @return array The expected data types for the given file */ - public static function get_data_types($file_name) { + public static function get_data_types(string $file_name): array { return self::get_file_datatypes()[$file_name] ?? []; } /** - * Validate the data types of the CSV files + * Validate the data types of the CSV files. * - * @param string $directory The directory containing the CSV files + * @param array $directory The directory containing the CSV files * @return array An array containing the validity of the files, the invalid files, and error messages */ - public static function validate_csv_data_types($directory) { + public static function validate_csv_data_types(array $directory): array { $is_valid = true; $invalid_files = []; $error_messages = []; @@ -342,10 +340,10 @@ public static function validate_csv_data_types($directory) { $file_is_valid = true; foreach ($headers as $index => $header) { - $expected_types = $expected_data_types[$header] ?? ['N/A']; + $expected_types = $expected_data_types[$header] ?? [get_string('na', 'enrol_oneroster')]; $detected_type = $detected_data_types[$index]; if (!in_array($detected_type, (array)$expected_types, true)) { - $error_messages[] = "Validation failed for header '$header' in file '$file'."; + $error_messages[] = get_string('validation', 'enrol_oneroster', (object)[ 'header' => $header, 'file' => $file]); $is_valid = false; $file_is_valid = false; } @@ -365,11 +363,11 @@ public static function validate_csv_data_types($directory) { } /** - * Function to display errors for CSV data type validation + * Function to display errors for CSV data type validation. * * @param array $validation_result An array containing the validity of the files, the invalid files, and error messages */ - public static function display_validation_errors($validation_result) { + public static function display_validation_errors(array $validation_result): void { if (!empty($validation_result['error_messages'])) { echo \html_writer::tag('h3', get_string('datatype_error_messages', 'enrol_oneroster')); @@ -386,9 +384,9 @@ public static function display_validation_errors($validation_result) { } /** - * Function to validate users and configure settings for database entry - * If all users have an identifier, the users will be saved to the database - * Otherwise, no users will be saved + * Function to validate users and configure settings for database entry. + * If all users have an identifier, the users will be saved to the database. + * Otherwise, no users will be saved. * * @param array $csv_data An array containing user data, including identifiers and other relevant details. * @return bool True if all users have an identifier and password, false otherwise @@ -403,13 +401,13 @@ public static function validate_user_data(array $csv_data): bool { } /** - * Determine the data type of a value + * Determine the data type of a value. * * @param string $value The value to determine the data type of * @param array $expected_types The expected data types for the value * @return string The detected data type */ - public static function determine_data_type($value, $expected_types) { + public static function determine_data_type($value, $expected_types): mixed { if (trim($value) === '') { return csv_client_const_helper::DATATYPE_NULL; } @@ -423,12 +421,12 @@ public static function determine_data_type($value, $expected_types) { } /** - * Check if a value is a valid password + * Check if a value is a valid password. * * @param mixed $value The value to check * @return bool True if the value is a valid password, false otherwise */ - public static function is_valid_password($value) { + public static function is_valid_password($value): mixed { return check_password_policy($value, $errormsg); } @@ -450,7 +448,7 @@ public static function is_valid_human_readable_string($value): bool { } /** - * Check if a value is of type int + * Check if a value is of type int. * * @param mixed $value The value to check * @return bool True if the value is of type int, false otherwise @@ -460,7 +458,7 @@ public static function is_int_type($value): bool { } /** - * Check if a value is of type list (array of strings) + * Check if a value is of type list (array of strings). * * @param mixed $value The value to check * @return bool True if the value is of type list of strings, false otherwise @@ -483,7 +481,7 @@ public static function is_list_of_strings($value): bool { } /** - * Validate and parse subject codes + * Validate and parse subject codes. * Subject codes can be a single string or a list of strings separated by commas * * @param string $value The value to check @@ -536,8 +534,8 @@ public static function is_valid_periods(string $value): bool { } /** - * Check if a value is of type datetime - * Regular expression for ISO 8601 datetime format (YYYY-MM-DDTHH:MM:SS.SSSZ) + * Check if a value is of type datetime. + * Regular expression for ISO 8601 datetime format (YYYY-MM-DDTHH:MM:SS.SSSZ). * * If the value is in the old format (YYYY-MM-DD), it transforms to the new format (YYYY-MM-DDT23:59:59.999Z). * @@ -557,8 +555,8 @@ public static function is_datetime_type($value) { } /** - * Check if a value is of type date - * Regular expression for ISO 8601 date format (YYYY-MM-DD) + * Check if a value is of type date. + * Regular expression for ISO 8601 date format (YYYY-MM-DD). * * @param mixed $value The value to check * @return bool True if the value is of type date, false otherwise @@ -568,8 +566,8 @@ public static function is_date_type($value) { } /** - * Check if a value is of type - * Regular expression for GUID (characters: 0-9, a-z, A-Z, ., -, _, /, @) + * Check if a value is of type GUID. + * Regular expression for GUID (characters: 0-9, a-z, A-Z, ., -, _, /, @). * * @param mixed $value The value to check * @return bool True if the value is of type GUID, false otherwise @@ -579,7 +577,7 @@ public static function is_guid_type($value): bool { } /** - * Check if a value is of type status enum + * Check if a value is of type status enum. * * @param mixed $value The value to check * @return bool True if the value is of type status enum, false otherwise @@ -590,7 +588,7 @@ public static function is_status_enum_type($value): bool { } /** - * Check if a value is of type type enum + * Check if a value is of type enum. * * @param mixed $value The value to check * @return bool True if the value is of type type enum, false otherwise @@ -601,8 +599,8 @@ public static function is_type_enum($value): bool { } /** - * Check if a value is of type year - * Regular expression for year format (YYYY) + * Check if a value is of type year. + * Regular expression for year format (YYYY). * * @param mixed $value The value to check * @return bool True if the value is of type year, false otherwise @@ -612,7 +610,7 @@ public static function is_year_type($value): bool { } /** - * Check if a value is of type grade + * Check if a value is of type grade. * * @param mixed $value The value to check * @return bool True if the value is of type grade, false otherwise @@ -629,7 +627,7 @@ public static function is_valid_grades($value): bool { } /** - * Check if a value is of type grade + * Check if a value is of type grade. * * @param string $value The value to check * @return bool True if the value is of type grade, false otherwise @@ -640,7 +638,7 @@ public static function is_valid_grade(string $value): bool { } /** - * Check if a value is of type class type enum + * Check if a value is of type class type enum. * * @param mixed $value The value to check * @return bool True if the value is of type class type enum, false otherwise @@ -650,7 +648,7 @@ public static function is_class_type_enum($value): bool { } /** - * Check if a value is of type GUID list + * Check if a value is of type GUID list. * * @param mixed $value The value to check * @return bool True if the value is of type GUID list, false otherwise @@ -667,7 +665,7 @@ public static function is_valid_guid_list($value): bool { } /** - * Check if a value is of type role enum + * Check if a value is of type role enum. * * @param string $value The value to check * @return bool True if the value is of type role enum, false otherwise @@ -677,7 +675,7 @@ public static function is_role_enum(string $value): bool { } /** - * Check if a value is of type primary enum + * Check if a value is of type primary enum. * * @param mixed $value The value to check * @return bool True if the value is of type primary enum, false otherwise @@ -687,7 +685,7 @@ public static function is_primary_enum($value): bool { } /** - * Check if a value is of type org type enum + * Check if a value is of type org type enum. * * @param mixed $value The value to check * @return bool True if the value is of type org type enum, false otherwise @@ -697,7 +695,7 @@ public static function is_org_type_enum($value): bool { } /** - * Check if a value is of type email + * Check if a value is of type email. * * @param mixed $value The value to check * @return bool True if the value is of type email, false otherwise @@ -726,7 +724,7 @@ public static function is_valid_user_id($value): bool { } /** - * Check if a value is of type role user enum + * Check if a value is of type role user enum. * * @param mixed $value The value to check * @return bool True if the value is of type role user enum, false otherwise diff --git a/lang/en/enrol_oneroster.php b/lang/en/enrol_oneroster.php index 9ea7dd0..5e08760 100644 --- a/lang/en/enrol_oneroster.php +++ b/lang/en/enrol_oneroster.php @@ -93,3 +93,6 @@ $string['upload_zip_label'] = 'Upload Zip File'; $string['process_oneroster_csv'] = 'Process OneRoster CSV'; $string['set_url'] = '/enrol/oneroster/process_csv.php'; +$string['upload'] = 'Upload'; +$string['na'] = 'N/A'; +$string['validation'] = "Validation failed for header '{$a->header}' in file '{$a->file}'."; diff --git a/process_csv.php b/process_csv.php index 6b0bd82..476f2ac 100644 --- a/process_csv.php +++ b/process_csv.php @@ -28,20 +28,18 @@ require_once($CFG->libdir . '/adminlib.php'); /** - * One Roster Client + * One Roster Client. * * @package enrol_oneroster * @copyright Gustavo Amorim De Almeida, Ruben Cooper, Josh Bateson, Brayden Porter * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later * */ - admin_externalpage_setup('enrol_oneroster_csv_upload'); -$PAGE->set_url(get_string('set_url', 'enrol_oneroster')); -$PAGE->set_context(\context_system::instance()); -$PAGE->set_title(get_string('process_oneroster_csv', 'enrol_oneroster')); -$PAGE->set_heading(get_string('process_oneroster_csv', 'enrol_oneroster')); +$PAGE->set_url('/enrol/oneroster/process_csv.php'); +$PAGE->set_title('Process OneRoster CSV'); +$PAGE->set_heading('Process OneRoster CSV'); $mform = new oneroster_csv_form(); @@ -51,96 +49,82 @@ global $SESSION; if ($step == 1) { - // Step 1: Upload CSV ZIP file. $mform = new oneroster_csv_form(); if ($mform->is_cancelled()) { redirect(new \moodle_url('/admin/settings.php', ['section' => 'enrolsettingsoneroster'])); - } - - else if ($data = $mform->get_data()) { + } else if ($data = $mform->get_data()) { $uniqueid = $USER->id . '_' . time(); $tempdir = make_temp_directory('oneroster_csv/' . $uniqueid); $filecontent = $mform->get_file_content('uploadedzip'); $zipfilepath = $tempdir . '/uploadedzip.zip'; - if (file_put_contents($zipfilepath, $filecontent)) { - $zip = new \ZipArchive(); - $res = $zip->open($zipfilepath); - if ($res === true) { - $zip->extractTo($tempdir); - $zip->close(); - $manifest_path = $tempdir . '/manifest.csv'; - if (file_exists($manifest_path)) { - $missing_files = csv_client_helper::check_manifest_and_files($manifest_path, $tempdir); - if (!empty($missing_files['missing_files']) && !empty($missing_files['invalid_headers'])) { - echo $OUTPUT->header(); - csv_client_helper::display_missing_and_invalid_files($missing_files); - echo $OUTPUT->footer(); - exit; - } - - $datatype = csv_client_helper::validate_csv_data_types($tempdir); - if (!$datatype['is_valid']) { - echo $OUTPUT->header(); - csv_client_helper::display_validation_errors($datatype); - echo $OUTPUT->footer(); - exit; - } - - $csv_data = csv_client_helper::extract_csvs_to_arrays($tempdir); - $orgs = $csv_data['orgs'] ?? []; - // Prepare organization options - $orgoptions = []; - foreach ($orgs as $org) { - $orgoptions[$org['sourcedId']] = $org['name']; - } - if (count($orgoptions) == 1) { - // Only one organization, skip the selection form - $selected_org_sourcedId = array_key_first($orgoptions); - // Proceed to process the selected organization - process_selected_organization($selected_org_sourcedId, $tempdir, $csv_data); - exit; - } else { - // Display the organization selection form - $SESSION->oneroster_csv['orgoptions'] = $orgoptions; - $orgform = new oneroster_org_selection_form(null, ['orgoptions' => $orgoptions, 'tempdir' => $tempdir]); - echo $OUTPUT->header(); - $orgform->display(); - echo $OUTPUT->footer(); - exit; - } - } else { - echo $OUTPUT->header(); - echo get_string('missing_csv_files', 'enrol_oneroster') . '
'; - echo $OUTPUT->footer(); - exit; - } - } - else { - echo $OUTPUT->header(); - echo get_string('failed_to_open_zip_file', 'enrol_oneroster') . '
'; - echo $OUTPUT->footer(); - exit; - } - } - else { + + if (!file_put_contents($zipfilepath, $filecontent)) { + redirect($PAGE->url, get_string('failed_upload_zip_file', 'enrol_oneroster')); + } + + $zip = new \ZipArchive(); + $res = $zip->open($zipfilepath); + if ($res !== true) { + redirect($PAGE->url, get_string('failed_to_open_zip_file', 'enrol_oneroster')); + } + + $zip->extractTo($tempdir); + $zip->close(); + + $manifest_path = $tempdir . '/manifest.csv'; + if (!file_exists($manifest_path)) { + redirect($PAGE->url, get_string('missing_csv_files', 'enrol_oneroster')); + } + + $missing_files = csv_client_helper::check_manifest_and_files($manifest_path, $tempdir); + if (!empty($missing_files['missing_files']) || !empty($missing_files['invalid_headers'])) { + $error_message = csv_client_helper::get_missing_and_invalid_files_message($missing_files); + redirect($PAGE->url, $error_message); + } + + $datatype = csv_client_helper::validate_csv_data_types($tempdir); + if (!$datatype['is_valid']) { + $error_message = csv_client_helper::get_validation_errors_message($datatype); + redirect($PAGE->url, $error_message); + } + + $csv_data = csv_client_helper::extract_csvs_to_arrays($tempdir); + $orgs = $csv_data['orgs'] ?? []; + + // Prepare organization options. + $orgoptions = []; + foreach ($orgs as $org) { + $orgoptions[$org['sourcedId']] = $org['name']; + } + + if (count($orgoptions) == 1) { + // Only one organization, proceed directly. + $selected_org_sourcedId = array_key_first($orgoptions); + process_selected_organization($selected_org_sourcedId, $tempdir, $csv_data); + exit; + } else { + // Display the organization selection form. + $SESSION->oneroster_csv['orgoptions'] = $orgoptions; + $orgform = new oneroster_org_selection_form(null, ['orgoptions' => $orgoptions, 'tempdir' => $tempdir]); echo $OUTPUT->header(); - echo get_string('failed_upload_zip_file', 'enrol_oneroster') . '
'; + $orgform->display(); echo $OUTPUT->footer(); exit; } - } - else { + } else { echo $OUTPUT->header(); + if ($message = optional_param('message', '', PARAM_TEXT)) { + echo $OUTPUT->notification($message, 'error'); + } $mform->display(); echo $OUTPUT->footer(); } - -} +} else if ($step == 2) { // Step 2: Select Organization. - $tempdir = required_param('tempdir', PARAM_PATH); // Retrieve tempdir from submitted data - $orgoptions = $SESSION->oneroster_csv['orgoptions']; // Retrieve orgoptions from session + $tempdir = required_param('tempdir', PARAM_PATH); + $orgoptions = $SESSION->oneroster_csv['orgoptions']; $orgform = new oneroster_org_selection_form(null, ['orgoptions' => $orgoptions, 'tempdir' => $tempdir]); if ($orgform->is_cancelled()) { @@ -150,7 +134,7 @@ $selected_org_sourcedId = $orgdata->organization; $tempdir = $orgdata->tempdir; - // Proceed to process the selected organization + // Proceed to process the selected organization. process_selected_organization($selected_org_sourcedId, $tempdir); } else { diff --git a/settings.php b/settings.php index e8877b7..bd533bd 100644 --- a/settings.php +++ b/settings.php @@ -34,15 +34,6 @@ get_string('options', 'enrol_oneroster') )); - // CSV upload link. - $settings->add(new admin_setting_heading( - 'enrol_oneroster/csvupload_link', - get_string('csv_upload', 'enrol_oneroster'), - html_writer::link(new moodle_url('/enrol/oneroster/process_csv.php'), - get_string('csv_upload_process', 'enrol_oneroster'), - array('style' => 'font-size: 20px;')) - )); - // Connections settings: // - One Roster version; // - OAuth version; diff --git a/tests/process_csv_data_type_validation_test.php b/tests/process_csv_data_type_validation_test.php index 4b61fe7..53bcb98 100644 --- a/tests/process_csv_data_type_validation_test.php +++ b/tests/process_csv_data_type_validation_test.php @@ -38,7 +38,7 @@ protected function setUp(): void $this->test_dir = make_temp_directory('csvtest_dir'); $this->manifest_path = $this->test_dir . DIRECTORY_SEPARATOR . csv_client_const_helper::FILE_MANIFEST; - // Creating manifest.csv + // Creating manifest.csv. $manifest_content = [ ['propertyName', 'value'], ['file.academicSessions', 'bulk'], @@ -53,7 +53,7 @@ protected function setUp(): void } fclose($handle); - // Creating academicSessions.csv + // Creating academicSessions.csv. $academic_sessions_content = [csv_client_const_helper::HEADER_ACADEMIC_SESSIONS, ['as-trm-222-1234', 'active', '2023-05-01T18:25:43.511Z', 'Session Title', 'term', '2022-09-01', '2022-12-24', 'as-syr-222-2023', '2023'], ['as-grp-222-2345', '', '', 'Session Title', 'gradingPeriod', '2022-10-02', '2022-12-24', 'as-trm-222-1234', '2023']]; @@ -63,7 +63,7 @@ protected function setUp(): void } fclose($handle); - // Creating classes.csv + // Creating classes.csv. $classes_content = [csv_client_const_helper::HEADER_CLASSES, ['cls-222-123456', 'active', '2023-05-01T18:25:43.511Z', 'Introduction to Physics', '09,10,11', 'crs-222-2023-456-12345', 'Phys 100 - 1', 'Scheduled', 'Room 2-B', 'org-sch-222-456', 'as-trm-222-1234', 'Science, Physics, Biology', 'PHY123, ASV120', '1'], ['cls-222-123478', 'tobedeleted', '2023-05-01T18:25:43.511Z', 'History - 2', '10', 'crs-222-2023-456-23456', '2', 'Scheduled', 'Room 18-C', 'org-sch-222-456', 'as-trm-222-1234', 'History', 'HIS123', '1,2,3']]; @@ -73,7 +73,7 @@ protected function setUp(): void } fclose($handle); - // Creating enrollments.csv + // Creating enrollments.csv. $enrollments_content = [csv_client_const_helper::HEADER_ENROLLMENTS, ['enr-t-222-12345-123456', 'active', '2023-05-01T18:25:43.511Z', 'cls-222-12345', 'org-sch-222-456', 'usr-222-123456', 'teacher', 'FALSE', '2022-03-15', '2022-06-15'], ['enr-s-222-12345-987654', '', '', 'cls-222-12345', 'org-sch-222-456', 'usr-222-987654', 'student', 'FALSE', '2022-03-16', '2022-06-16'] @@ -84,7 +84,7 @@ protected function setUp(): void } fclose($handle); - // Creating orgs.csv + // Creating orgs.csv. $orgs_content = [csv_client_const_helper::HEADER_ORGS, ['org-sch-222-3456', 'active', '2023-05-01T18:25:43.511Z', 'Upper School', 'school', 'US', 'org-dpt-222-456'], ['org-sch-222-456', '', '', 'History Department', 'department', 'History', 'org-sch-222-3456'], @@ -95,8 +95,8 @@ protected function setUp(): void } fclose($handle); - // Creating users.csv - $users_content = [csv_client_const_helper::HEADER_USERS, // Header + // Creating users.csv. + $users_content = [csv_client_const_helper::HEADER_USERS, ['usr-222-123456', 'active', '2023-05-01', 'TRUE', 'org-sch-222-456', 'teacher', 'john.doe', '', 'John', 'Doe', 'Michael', '123456', 'john.doe@myschool.com', '6037778888', '6032221111', 'usr-222-66778900', '11', 'Password1*'], ['usr-222-66778899', '', '', 'TRUE', 'org-sch-222-456', 'student', 'mary.jones', '{LDAP:12}', 'Mary', 'Jones', 'Jane', '66778899', 'mary.jones@myschool.com', '6031234567', '6031234567', 'usr-222-66778900', '12', 'Password1*'], ['usr-222-66778900', 'active', '2023-05-01', 'TRUE', 'org-sch-222-456', 'parent', 'thomas.joness', '{LDAP:12},{LTI:15},{Fed:23}', 'Thomas', 'Jones', 'Paul', '66778900', 'thomas.jones@testemail.com', '6039876543', '6039876543', 'usr-222-66778899', '10', 'Password1*'] @@ -127,7 +127,7 @@ public function test_ValidateCsvDataTypes(): void { * @covers enrol_oneroster\csv_client_const_helper::get_data_types */ public function test_GetDataTypes() { - // Test academicSessions.csv data types + // Test academicSessions.csv data types. $result = csv_client_helper::get_data_types(csv_client_const_helper::FILE_ACADEMIC_SESSIONS); $expected = [ csv_client_const_helper::HEADER_SOURCEDID => csv_client_const_helper::DATATYPE_GUID, @@ -142,7 +142,7 @@ public function test_GetDataTypes() { ]; $this->assertEquals($expected, $result, 'The expected data types for academicSessions.csv do not match.'); - // Test classes.csv data types + // Test classes.csv data types. $result = csv_client_helper::get_data_types(csv_client_const_helper::FILE_CLASSES); $expected = [ csv_client_const_helper::HEADER_SOURCEDID => csv_client_const_helper::DATATYPE_GUID, @@ -162,7 +162,7 @@ public function test_GetDataTypes() { ]; $this->assertEquals($expected, $result, 'The expected data types for classes.csv do not match.'); - // Test file with no data types defined + // Test file with no data types defined. $result = csv_client_helper::get_data_types('unknown.csv'); $expected = []; $this->assertEquals($expected, $result, 'The expected data types for unknown.csv should be an empty array.'); diff --git a/tests/process_csv_file_check_test.php b/tests/process_csv_file_check_test.php index c79a40e..609cdb0 100644 --- a/tests/process_csv_file_check_test.php +++ b/tests/process_csv_file_check_test.php @@ -38,7 +38,7 @@ protected function setUp(): void $this->test_dir = make_temp_directory('csvtest_dir'); $this->manifest_path = $this->test_dir . DIRECTORY_SEPARATOR . 'manifest.csv'; - // Creating manifest.csv + // Creating manifest.csv. $manifest_content = [ ['propertyName', 'value'], ['file.academicSessions', 'bulk'], @@ -53,7 +53,7 @@ protected function setUp(): void } fclose($handle); - // Creating academicSessions.csv + // Creating academicSessions.csv. $academic_sessions_content = [csv_client_const_helper::HEADER_ACADEMIC_SESSIONS, ['as-trm-222-1234', 'active', '2023-05-01T18:25:43.511Z', 'Session Title', 'term', '2022-09-01', '2022-12-24', 'as-syr-222-2023', '2023'], ['as-grp-222-2345', '', '', 'Session Title', 'gradingPeriod', '2022-10-02', '2022-12-24', 'as-trm-222-1234', '2023']]; @@ -63,7 +63,7 @@ protected function setUp(): void } fclose($handle); - // Creating classes.csv + // Creating classes.csv. $classes_content = [csv_client_const_helper::HEADER_CLASSES, ['cls-222-123456', 'active', '2023-05-01T18:25:43.511Z', 'Introduction to Physics', '09,10,11', 'crs-222-2023-456-12345', 'Phys 100 - 1', 'Scheduled', 'Room 2-B', 'org-sch-222-456', 'as-trm-222-1234', 'Science, Physics, Biology', 'PHY123, ASV120', '1'], ['cls-222-123478', 'tobedeleted', '2023-05-01T18:25:43.511Z', 'History - 2', '10', 'crs-222-2023-456-23456', '2', 'Scheduled', 'Room 18-C', 'org-sch-222-456', 'as-trm-222-1234', 'History', 'HIS123', '1,2,3']]; @@ -73,7 +73,7 @@ protected function setUp(): void } fclose($handle); - // Creating enrollments.csv + // Creating enrollments.csv. $enrollments_content = [csv_client_const_helper::HEADER_ENROLLMENTS, ['enr-t-222-12345-123456', 'active', '2023-05-01T18:25:43.511Z', 'cls-222-12345', 'org-sch-222-456', 'usr-222-123456', 'teacher', 'FALSE', '2022-03-15', '2022-06-15'], ['enr-s-222-12345-987654', '', '', 'cls-222-12345', 'org-sch-222-456', 'usr-222-987654', 'student', 'FALSE', '2022-03-16', '2022-06-16'] @@ -84,7 +84,7 @@ protected function setUp(): void } fclose($handle); - // Creating orgs.csv + // Creating orgs.csv. $orgs_content = [csv_client_const_helper::HEADER_ORGS, ['org-sch-222-3456', 'active', '2023-05-01T18:25:43.511Z', 'Upper School', 'school', 'US', 'org-dpt-222-456'], ['org-sch-222-456', '', '', 'History Department', 'department', 'History', 'org-sch-222-3456'], @@ -95,8 +95,8 @@ protected function setUp(): void } fclose($handle); - // Creating users.csv - $users_content = [csv_client_const_helper::HEADER_USERS, // Header + // Creating users.csv. + $users_content = [csv_client_const_helper::HEADER_USERS, ['usr-222-123456', 'active', '2023-05-01', 'TRUE', 'org-sch-222-456', 'teacher', 'john.doe', '', 'John', 'Doe', 'Michael', '123456', 'john.doe@myschool.com', '6037778888', '6032221111', 'usr-222-66778900', '11', 'Password1*'], ['usr-222-66778899', '', '', 'TRUE', 'org-sch-222-456', 'student', 'mary.jones', '{LDAP:12}', 'Mary', 'Jones', 'Jane', '66778899', 'mary.jones@myschool.com', '6031234567', '6031234567', 'usr-222-66778900', '12', 'Password1*'], ['usr-222-66778900', 'active', '2023-05-01', 'TRUE', 'org-sch-222-456', 'parent', 'thomas.joness', '{LDAP:12},{LTI:15},{Fed:23}', 'Thomas', 'Jones', 'Paul', '66778900', 'thomas.jones@testemail.com', '6039876543', '6039876543', 'usr-222-66778899', '10', 'Password1*']