diff --git a/student_auto_feed/add_drop_report.php b/student_auto_feed/add_drop_report.php index b49e2d3..25584f6 100644 --- a/student_auto_feed/add_drop_report.php +++ b/student_auto_feed/add_drop_report.php @@ -126,13 +126,16 @@ public static function close() { } /** - * Verify that DB connection resource is OK + * Verify that DB connection resource/instance is OK + * + * PHP < 8.1: self::$db is a resource + * PHP >= 8.1: self::$db is an instanceof \PgSql\Connection * * @access private - * @return bool true when DB connection resource is OK, false otherwise. + * @return bool true when DB connection resource/instance is OK, false otherwise. */ private static function check() { - return is_resource(self::$db) && pg_connection_status(self::$db) === PGSQL_CONNECTION_OK; + return (is_resource(self::$db) || self::$db instanceof \PgSql\Connection) && pg_connection_status(self::$db) === PGSQL_CONNECTION_OK; } /** @@ -148,7 +151,7 @@ public static function get_courses($term) { // Undergraduate courses from DB. $sql = "SELECT course FROM courses WHERE term=$1 AND status=1"; - $params = array($term); + $params = [$term]; $res = pg_query_params(self::$db, $sql, $params); if ($res === false) die("Failed to retrieve course list from DB\n"); @@ -171,7 +174,7 @@ public static function get_mapped_courses($term) { // mapped courses from DB $sql = "SELECT course, mapped_course FROM mapped_courses WHERE term=$1"; - $params = array($term); + $params = [$term]; $res = pg_query_params(self::$db, $sql, $params); if ($res === false) { die("Failed to retrieve mapped courses from DB\n"); @@ -199,15 +202,15 @@ public static function count_enrollments($term, $course_list, $mapped_courses) { die("Not connected to DB when querying course enrollments\n"); } - $course_enrollments = array(); - $manual_flags = array(); + $course_enrollments = []; + $manual_flags = []; foreach ($course_list as $course) { $grad_course = array_search($course, $mapped_courses); if ($grad_course === false) { // COURSE HAS NO GRAD SECTION (not mapped). $sql = "SELECT COUNT(*) FROM courses_users WHERE term=$1 AND course=$2 AND user_group=4 AND registration_section IS NOT NULL"; - $params = array($term, $course); + $params = [$term, $course]; $res = pg_query_params(self::$db, $sql, $params); if ($res === false) die("Failed to lookup enrollments for {$course}\n"); @@ -222,7 +225,7 @@ public static function count_enrollments($term, $course_list, $mapped_courses) { } else { // UNDERGRADUATE SECTION $sql = "SELECT COUNT(*) FROM courses_users WHERE term=$1 AND course=$2 AND user_group=4 AND registration_section='1'"; - $params = array($term, $course); + $params = [$term, $course]; $res = pg_query_params(self::$db, $sql, $params); if ($res === false) die("Failed to lookup enrollments for {$course}\n"); @@ -254,7 +257,7 @@ public static function count_enrollments($term, $course_list, $mapped_courses) { // Courses make up array keys. Sort by courses. ksort($course_enrollments); ksort($manual_flags); - return array($course_enrollments, $manual_flags); + return [$course_enrollments, $manual_flags]; } } @@ -282,7 +285,7 @@ public static function write_temp_csv($course_enrollments) { } foreach($course_enrollments as $course=>$num_students) { - fputcsv($fh, array($course, $num_students), CSV_DELIM_CHAR); + fputcsv($fh, [$course, $num_students], CSV_DELIM_CHAR); } fclose($fh); chmod($tmp_path . $tmp_file, 0660); @@ -305,7 +308,7 @@ public static function read_temp_csv() { unlink($tmp_path . $tmp_file); // remove tmp file. array_walk($csv, 'callbacks::str_getcsv_cb'); - // return array of array('course' => enrollment). e.g. ('csci1000' => 100) + // return array of ['course' => enrollment]. e.g. ['csci1000' => 100] return array_combine(array_column($csv, 0), array_column($csv, 1)); } @@ -321,9 +324,11 @@ public static function compile_report($prev_course_enrollments, $course_enrollme // Compile stats $date = date("F j, Y"); $time = date("g:i A"); - $report = "Student autofeed counts report for {$date} at {$time}\n"; - $report .= "NOTE: Difference and ratio do not account for the manual flag.\n"; - $report .= "COURSE YESTERDAY TODAY MANUAL DIFFERENCE RATIO\n"; + $report = <<$course_enrollment) { // Calculate data @@ -363,7 +368,7 @@ public static function send_report($term, $report) { $from = ADD_DROP_FROM_EMAIL; $subject = "Submitty Autofeed Add/Drop Report For {$date}"; $report = str_replace("\n", "\r\n", $report); // needed for email formatting - $is_sent = mail($to, $subject, $report, array('from' => $from)); + $is_sent = mail($to, $subject, $report, ['from' => $from]); if (!$is_sent) { $report = str_replace("\r\n", "\n", $report); // revert back since not being emailed. fprintf(STDERR, "Add/Drop report could not be emailed.\n%s", $report); diff --git a/student_auto_feed/config.php b/student_auto_feed/config.php index ab107bc..40432d7 100644 --- a/student_auto_feed/config.php +++ b/student_auto_feed/config.php @@ -5,7 +5,7 @@ * config.php script used by submitty_student_auto_feed * By Peter Bailie, Systems Programmer (RPI dept of computer science) * - * Requires minimum PHP version 7.1 with pgsql extension. + * Requires minimum PHP version 7.3 with pgsql extension. * * Configuration of submitty_student_auto_feed is structured through a series * of named constants. @@ -127,9 +127,6 @@ //Set to true, if Submitty is using SAML for authentication. define('PROCESS_SAML', true); -//Allows "\r" EOL encoding. This is rare but exists (e.g. Excel for Macintosh). -ini_set('auto_detect_line_endings', true); - /* DATA SOURCING -------------------------------------------------------------- * The Student Autofeed provides helper scripts to retrieve the CSV file for * processing. Shell script ssaf.sh is used to invoke one of the helper diff --git a/student_auto_feed/crn_copymap.php b/student_auto_feed/crn_copymap.php index 01d2a92..c882672 100644 --- a/student_auto_feed/crn_copymap.php +++ b/student_auto_feed/crn_copymap.php @@ -58,7 +58,7 @@ private function write_mappings($args) { $len = count($source_sections); for ($i = 0; $i < $len; $i++) { - $row = array($source_course, $source_sections[$i], $dest_course, $dest_sections[$i]); + $row = [$source_course, $source_sections[$i], $dest_course, $dest_sections[$i]]; fputcsv($fh, $row, ","); } @@ -66,10 +66,10 @@ private function write_mappings($args) { } private function get_mappings($sections) { - if ($sections === "" || $sections === "all") return array($sections); + if ($sections === "" || $sections === "all") return [$sections]; $arr = explode(",", $sections); - $expanded = array(); + $expanded = []; foreach($arr as $val) { if (preg_match("/(\d+)\-(\d+)/", $val, $matches) === 1) { $expanded = array_merge($expanded, range((int) $matches[1], (int) $matches[2])); @@ -100,10 +100,12 @@ class cli { Arguments: -h, --help, help Show this help message. term Term code of courses and sections being mapped. Required. - course-a Original course - sections Section list, or "all" of preceding course - course-b Course being copied to - sections For course-b, this can be ommited when course-a sections is "all" + course-a Original course. + sections Section list or "all" for course-a. + course-b Course being copied to. + sections Section list or "all" for course-b. + + Course-b sections can be ommited when course-a sections is "all"\n ARGS_LIST; /** @@ -115,7 +117,7 @@ class cli { */ public static function parse_args() { global $argc, $argv; - $matches = array(); + $matches = []; switch(true) { // Check for request for help @@ -130,7 +132,7 @@ public static function parse_args() { case preg_match("/^[\w\d\-]+$/", $argv[2], $matches['source']['course']) !== 1: case preg_match("/^\d+(?:(?:,|\-)\d+)*$|^all$/", $argv[3], $matches['source']['sections']) !== 1: case preg_match("/^[\w\d\-]+$/", $argv[4], $matches['dest']['course']) !== 1: - case preg_match("/^\d+(?:(?:,|\-)\d+)*$|^(?:all)?$/", $argv[5], $matches['dest']['sections']) !== 1: + case preg_match("/^\d+(?:(?:,|\-)\d+)*$|^(?:all)?$/", $argv[5] ?? "", $matches['dest']['sections']) !== 1: self::print_usage(); exit; } diff --git a/student_auto_feed/readme.md b/student_auto_feed/readme.md index f6e1d3a..720714d 100644 --- a/student_auto_feed/readme.md +++ b/student_auto_feed/readme.md @@ -10,7 +10,7 @@ policies and practices.__ Detailed instructions can be found at [http://submitty.org/sysadmin/student\_auto\_feed](http://submitty.org/sysadmin/student_auto_feed) -Requirements: PHP 7.1 or higher with pgsql extension. `imap_remote.php` also +Requirements: PHP 7.3 or higher with pgsql extension. `imap_remote.php` also requires the imap extension. This system is intended to be platform agnostic, but has been developed and tested with Ubuntu Linux. diff --git a/student_auto_feed/ssaf_cli.php b/student_auto_feed/ssaf_cli.php index 1ae11ad..6a85223 100644 --- a/student_auto_feed/ssaf_cli.php +++ b/student_auto_feed/ssaf_cli.php @@ -4,20 +4,19 @@ /** class to parse command line arguments */ class cli_args { /** @var array Holds all CLI argument flags and their values */ - private static $args = array(); + private static $args = []; /** @var string usage help message */ private static $help_usage = "Usage: submitty_student_auto_feed.php [-h | --help] [-a auth str] (-t term code)\n"; /** @var string short description help message */ private static $help_short_desc = "Read student enrollment CSV and upsert to Submitty database.\n"; /** @var string argument list help message */ private static $help_args_list = << $row['mapped_course'], 'mapped_section' => $row['mapped_section'] - ); + ]; } } @@ -110,7 +110,7 @@ public static function get_enrollment_count($semester, $course) { return false; } - $results = self::run_query(sql::GET_COURSE_ENROLLMENT_COUNT, array($semester, $course)); + $results = self::run_query(sql::GET_COURSE_ENROLLMENT_COUNT, [$semester, $course]); if ($results === false) { self::$error .= "Error while retrieving course enrollment counts."; return false; @@ -166,14 +166,14 @@ public static function upsert($semester, $course, $rows) : bool { // Do upsert of course enrollment data. foreach($rows as $row) { - $users_params = array( + $users_params = [ $row[COLUMN_USER_ID], $row[COLUMN_NUMERIC_ID], $row[COLUMN_FIRSTNAME], $row[COLUMN_LASTNAME], $row[COLUMN_PREFERREDNAME], $row[COLUMN_EMAIL] - ); + ]; // Determine registration type for courses_users table // Registration type code has already been validated by now. @@ -189,7 +189,7 @@ public static function upsert($semester, $course, $rows) : bool { break; } - $courses_users_params = array( + $courses_users_params = [ $semester, $course, $row[COLUMN_USER_ID], @@ -197,11 +197,11 @@ public static function upsert($semester, $course, $rows) : bool { $row[COLUMN_SECTION], $registration_type, "FALSE" - ); + ]; - $reg_sections_params = array($semester, $course, $row[COLUMN_SECTION], $row[COLUMN_REG_ID]); - $tmp_table_params = array($row[COLUMN_USER_ID]); - $dropped_users_params = array($semester, $course); + $reg_sections_params = [$semester, $course, $row[COLUMN_SECTION], $row[COLUMN_REG_ID]]; + $tmp_table_params = [$row[COLUMN_USER_ID]]; + $dropped_users_params = [$semester, $course]; // Upsert queries // If any query returns false, we need to rollback and bail out. @@ -247,8 +247,16 @@ public static function upsert($semester, $course, $rows) : bool { // PRIVATE STATIC FUNCTIONS ------------------------------------------------ + /** + * Verify that DB connection resource/instance is OK + * + * PHP < 8.1: self::$db is a resource + * PHP >= 8.1: self::$db is an instanceof \PgSql\Connection + * + * @return bool true when DB connection resource/instance is OK, false otherwise. + */ private static function check() : bool { - if (!is_resource(self::$db) || pg_connection_status(self::$db) !== PGSQL_CONNECTION_OK) { + if ((!is_resource(self::$db) && !(self::$db instanceof \PgSql\Connection)) || pg_connection_status(self::$db) !== PGSQL_CONNECTION_OK) { self::$error = "No DB connection."; return false; } @@ -273,8 +281,8 @@ private static function run_query($sql, $params = null) { return false; } - if (is_null($params)) $params = array(); - else if (!is_array($params)) $params = array($params); + if (is_null($params)) $params = []; + elseif (is_scalar($params)) $params = [$params]; $res = pg_query_params(self::$db, $sql, $params); if ($res === false) { diff --git a/student_auto_feed/ssaf_sql.php b/student_auto_feed/ssaf_sql.php index 15c6a4d..9ad5ea7 100644 --- a/student_auto_feed/ssaf_sql.php +++ b/student_auto_feed/ssaf_sql.php @@ -19,92 +19,92 @@ class sql { // SELECT queries public const GET_COURSES = <<'' - THEN EXCLUDED.user_email - ELSE users.user_email - END -/* AUTH: "AUTO_FEED" */ -SQL; + INSERT INTO users ( + user_id, + user_numeric_id, + user_givenname, + user_familyname, + user_preferred_givenname, + user_email + ) VALUES ($1, $2, $3, $4, $5, $6) + ON CONFLICT (user_id) DO UPDATE + SET user_numeric_id=EXCLUDED.user_numeric_id, + user_givenname=EXCLUDED.user_givenname, + user_familyname=EXCLUDED.user_familyname, + user_preferred_givenname= + CASE WHEN users.user_updated=FALSE + AND users.instructor_updated=FALSE + AND COALESCE(users.user_preferred_givenname, '')='' + THEN EXCLUDED.user_preferred_givenname + ELSE users.user_preferred_givenname + END, + user_email= + CASE WHEN COALESCE(EXCLUDED.user_email, '')<>'' + THEN EXCLUDED.user_email + ELSE users.user_email + END + /* AUTH: "AUTO_FEED" */ + SQL; // UPSERT courses_users table public const UPSERT_COURSES_USERS = << $b[COLUMN_USER_ID]; }); - $user_ids = array(); - $d_rows = array(); + $user_ids = []; + $d_rows = []; $are_all_unique = true; // Unless proven FALSE $length = count($rows); for ($i = 1; $i < $length; $i++) { diff --git a/student_auto_feed/submitty_student_auto_feed.php b/student_auto_feed/submitty_student_auto_feed.php index cae91c0..9f1b9f2 100755 --- a/student_auto_feed/submitty_student_auto_feed.php +++ b/student_auto_feed/submitty_student_auto_feed.php @@ -52,7 +52,8 @@ public function __construct() { $opts = cli_args::parse_args(); if (array_key_exists('l', $opts)) { $this->log_it("Logging test requested. There is no actual error to report."); - exit; + $this->shutdown(); + exit(0); } $this->semester = $opts['t']; @@ -69,11 +70,13 @@ public function __construct() { if (!$this->open_data_csv(CSV_FILE)) { // Error is already in log queue. + $this->shutdown(); exit(1); } if (!db::open($db_host, $db_user, $db_password)) { - $this->log_it("Error: Cannot connect to Submitty DB"); + $this->log_it("Error: Cannot connect to Submitty DB."); + $this->shutdown(); exit(1); } @@ -82,6 +85,7 @@ public function __construct() { $this->course_list = db::get_course_list($this->semester, $error); if ($this->course_list === false) { $this->log_it($error); + $this->shutdown(); exit(1); } @@ -89,6 +93,7 @@ public function __construct() { $this->mapped_courses = db::get_mapped_courses($this->semester, $error); if ($this->mapped_courses === false) { $this->log_it($error); + $this->shutdown(); exit(1); } @@ -96,11 +101,15 @@ public function __construct() { $this->crn_copymap = $this->read_crn_copymap(); // Init other properties. - $this->invalid_courses = array(); - $this->data = array(); + $this->invalid_courses = []; + $this->data = []; } public function __destruct() { + $this->shutdown(); + } + + private function shutdown() { db::close(); $this->close_data_csv(); @@ -168,10 +177,10 @@ private function get_csv_data() { } $graded_reg_codes = STUDENT_REGISTERED_CODES; - $audit_reg_codes = is_null(STUDENT_AUDIT_CODES) ? array() : STUDENT_AUDIT_CODES; - $latedrop_reg_codes = is_null(STUDENT_LATEDROP_CODES) ? array() : STUDENT_LATEDROP_CODES; + $audit_reg_codes = is_null(STUDENT_AUDIT_CODES) ? [] : STUDENT_AUDIT_CODES; + $latedrop_reg_codes = is_null(STUDENT_LATEDROP_CODES) ? [] : STUDENT_LATEDROP_CODES; $all_valid_reg_codes = array_merge($graded_reg_codes, $audit_reg_codes, $latedrop_reg_codes); - $unexpected_term_codes = array(); + $unexpected_term_codes = []; // Read and assign csv rows into $this->data array $row = fgetcsv($this->fh, 0, CSV_DELIM_CHAR); @@ -320,12 +329,12 @@ private function check_for_duplicate_user_ids() { */ private function check_for_excessive_dropped_users() { $is_validated = true; - $invalid_courses = array(); // intentional local array + $invalid_courses = []; // intentional local array $ratio = 0; $diff = 0; foreach($this->data as $course => $rows) { if (!validate::check_for_excessive_dropped_users($rows, $this->semester, $course, $diff, $ratio)) { - $invalid_courses[] = array('course' => $course, 'diff' => $diff, 'ratio' => round(abs($ratio), 3)); + $invalid_courses[] = ['course' => $course, 'diff' => $diff, 'ratio' => round(abs($ratio), 3)]; $is_validated = false; } } @@ -333,7 +342,7 @@ private function check_for_excessive_dropped_users() { if (!empty($invalid_courses)) { usort($invalid_courses, function($a, $b) { return $a['course'] <=> $b['course']; }); $msg = "The following course(s) have an excessive ratio of dropped students.\n Stats show mapped courses combined in base courses.\n"; - array_unshift($invalid_courses, array('course' => "COURSE", 'diff' => "DIFF", 'ratio' => "RATIO")); // Header + array_unshift($invalid_courses, ['course' => "COURSE", 'diff' => "DIFF", 'ratio' => "RATIO"]); // Header foreach ($invalid_courses as $invalid_course) { $msg .= " " . str_pad($invalid_course['course'], 18, " ", STR_PAD_RIGHT) . @@ -407,7 +416,7 @@ private function invalidate_courses() { */ private function read_crn_copymap() { // Skip this function and return empty copymap array when CRN_COPYMAP_FILE is null - if (is_null(CRN_COPYMAP_FILE)) return array(); + if (is_null(CRN_COPYMAP_FILE)) return []; // Insert "_{$this->semester}" right before file extension. // e.g. When term is "f23", "/path/to/copymap.csv" becomes "/path/to/copymap_f23.csv" @@ -415,24 +424,24 @@ private function read_crn_copymap() { if (!is_file($filename)) { $this->log_it("crn copymap file not found: {$filename}"); - return array(); + return []; } $fh = fopen($filename, 'r'); if ($fh === false) { $this->log_it("Failed to open crn copymap file: {$filename}"); - return array(); + return []; } // source course == $row[0] // source section == $row[1] // dest course == $row[2] // dest section == $row[3] - $arr = array(); + $arr = []; $row = fgetcsv($fh, 0, ","); while (!feof($fh)) { if (in_array($row[2], $this->course_list, true)) { - $arr[$row[0]][$row[1]] = array('course' => $row[2], 'section' => $row[3]); + $arr[$row[0]][$row[1]] = ['course' => $row[2], 'section' => $row[3]]; } else { $this->log_it("Duplicated course {$row[2]} not created in Submitty."); }