From 1b2979f8e622d736e44351e3e5ff94a844de2555 Mon Sep 17 00:00:00 2001 From: Waleed Hassan Date: Sun, 12 Jul 2026 22:15:41 +0100 Subject: [PATCH] WR #490867 auth_outage Review changes - code cleanup: fix exception chaining, remove dead code, code style fixes, initialise variable, add missing PHPDoc, add database indexes --- bootstrap.php | 1 + classes/calendar/calendar.php | 9 +++++--- classes/dml/outagedb.php | 3 ++- classes/local/cli/cli_exception.php | 2 +- classes/local/cli/clibase.php | 1 - classes/local/cli/create.php | 31 ++------------------------ classes/local/cli/finish.php | 1 + classes/local/cli/waitforit.php | 1 + classes/local/controllers/infopage.php | 5 +---- classes/local/outagelib.php | 3 +++ classes/task/update_static_page.php | 5 ++++- db/install.xml | 5 +++++ 12 files changed, 27 insertions(+), 40 deletions(-) diff --git a/bootstrap.php b/bootstrap.php index 324f6d6..13ad68d 100644 --- a/bootstrap.php +++ b/bootstrap.php @@ -69,6 +69,7 @@ if (is_callable('auth_outage_bootstrap_callback')) { } // 3) Check for allowed scripts or IPs during outages. +$outageinfo = false; if (!empty($_SERVER['REQUEST_URI'])) { $rooturl = parse_url($CFG->wwwroot); $path = ''; diff --git a/classes/calendar/calendar.php b/classes/calendar/calendar.php index 3cea7a0..941df2c 100644 --- a/classes/calendar/calendar.php +++ b/classes/calendar/calendar.php @@ -40,16 +40,18 @@ class calendar { /** * Create an event on the calendar for this outage. * @param outage $outage Outage to be added to the calendar. + * @return void */ - public static function create(outage $outage) { + public static function create(outage $outage): void { calendar_event::create(self::create_data($outage)); } /** * Updates an event on the calendar based on this outage. * @param outage $outage Outage to be updated in the calendar. + * @return void */ - public static function update(outage $outage) { + public static function update(outage $outage): void { $event = self::load($outage->id); if (is_null($event)) { @@ -63,8 +65,9 @@ class calendar { /** * Removes an event from the calendar related to this outage. * @param int $outageid Id of outage to be deleted from the calendar. + * @return void */ - public static function delete($outageid) { + public static function delete(int $outageid): void { $event = self::load($outageid); // If not found (was not created before) ignore it. diff --git a/classes/dml/outagedb.php b/classes/dml/outagedb.php index 91d7943..3e39238 100644 --- a/classes/dml/outagedb.php +++ b/classes/dml/outagedb.php @@ -48,6 +48,7 @@ class outagedb { /** * Gets all outage entries. + * @return outage[] */ public static function get_all() { global $DB; @@ -361,7 +362,7 @@ class outagedb { $data = $DB->get_records_select( 'auth_outage', 'starttime <= :datetime1 AND :datetime2 <= stoptime AND finished IS NULL', - ['datetime1' => $time, 'datetime2' => $time, 'datetime3' => $time], + ['datetime1' => $time, 'datetime2' => $time], 'starttime ASC, stoptime DESC, title ASC', '*', 0, diff --git a/classes/local/cli/cli_exception.php b/classes/local/cli/cli_exception.php index ee871d0..4b9769e 100644 --- a/classes/local/cli/cli_exception.php +++ b/classes/local/cli/cli_exception.php @@ -79,6 +79,6 @@ class cli_exception extends Exception { * @param Exception|null $previous Another exception as reference or null. */ public function __construct($message, $code = 1, ?Exception $previous = null) { - parent::__construct('*ERROR* ' . $message, $code, $previous = null); + parent::__construct('*ERROR* ' . $message, $code, $previous); } } diff --git a/classes/local/cli/clibase.php b/classes/local/cli/clibase.php index a1cbc3a..b3f4e45 100644 --- a/classes/local/cli/clibase.php +++ b/classes/local/cli/clibase.php @@ -114,7 +114,6 @@ abstract class clibase { * Change session to admin user. */ protected function become_admin_user() { - global $DB; $user = get_admin(); unset($user->description); unset($user->access); diff --git a/classes/local/cli/create.php b/classes/local/cli/create.php index 3d1588c..bb4bd50 100644 --- a/classes/local/cli/create.php +++ b/classes/local/cli/create.php @@ -28,8 +28,7 @@ use coding_exception; * @copyright 2016 Catalyst IT * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class create extends clibase -{ +class create extends clibase { /** * @var mixed[] Defaults to use if given option is null. */ @@ -99,6 +98,7 @@ class create extends clibase /** * Executes the CLI. + * @throws cli_exception */ public function execute() { // Help always overrides any other parameter. @@ -270,31 +270,4 @@ class create extends clibase return $option; } - /** - * Ensures the given option is or can be converted to a bool. - * @param mixed $option The parameter to check. - * @param string $param Name of that parameter. - * @return bool The converted parameter. - * @throws cli_exception - */ - private function merge_options_check_parameters_bool($option, $param) { - if (is_bool($option)) { - return $option; - } - - if (is_string($option)) { - $option = strtoupper($option); - if (in_array($option, ['0', 'FALSE', 'NO', 'N'])) { - return false; - } - if (in_array($option, ['1', 'TRUE', 'YES', 'Y'])) { - return true; - } - } - - throw new cli_exception( - get_string('clierrorinvalidvaluenotbool', 'auth_outage', ['param' => $param]), - cli_exception::ERROR_PARAMETER_INVALID - ); - } } diff --git a/classes/local/cli/finish.php b/classes/local/cli/finish.php index cc4a9a8..fa7fdee 100644 --- a/classes/local/cli/finish.php +++ b/classes/local/cli/finish.php @@ -56,6 +56,7 @@ class finish extends clibase { /** * Executes the CLI. + * @throws cli_exception */ public function execute() { // Help always overrides any other parameter. diff --git a/classes/local/cli/waitforit.php b/classes/local/cli/waitforit.php index dd95103..555831d 100644 --- a/classes/local/cli/waitforit.php +++ b/classes/local/cli/waitforit.php @@ -101,6 +101,7 @@ class waitforit extends clibase { $outage = $this->get_outage(); while ($sleep = $this->wait_for_outage_to_start($outage)) { + $sleep = max(1, $sleep); if (is_null($this->sleepcallback)) { $this->verbose('Sleeping for ' . $sleep . ' second(s).'); sleep($sleep); diff --git a/classes/local/controllers/infopage.php b/classes/local/controllers/infopage.php index 7800ecf..b4d9548 100644 --- a/classes/local/controllers/infopage.php +++ b/classes/local/controllers/infopage.php @@ -123,10 +123,7 @@ class infopage { ]; require($CFG->dirroot . '/auth/outage/views/info/content.php'); - // Moodle 2.7 did not check for CLI mode, which was fixed later. - if (!($CFG->branch == '27' && CLI_SCRIPT)) { - echo $OUTPUT->footer(); - } + echo $OUTPUT->footer(); $CFG->svgicons = $previoussvg; } diff --git a/classes/local/outagelib.php b/classes/local/outagelib.php index 48374d6..00a7bdf 100644 --- a/classes/local/outagelib.php +++ b/classes/local/outagelib.php @@ -51,6 +51,7 @@ class outagelib { /** * Fetches page. * @param string $file file to be fetched + * @return array{contents: string|false, mime: string} */ public static function fetch_page($file) { global $CFG; @@ -69,6 +70,7 @@ class outagelib { /** * Resets inject called to allow the code to be regenerated. + * @return void */ public static function reset_injectcalled() { self::$injectcalled = false; @@ -77,6 +79,7 @@ class outagelib { /** * Given a time, usually now, when is the next outage window? * @param int $time time for next window + * @return int */ public static function get_next_window($time = null) { diff --git a/classes/task/update_static_page.php b/classes/task/update_static_page.php index 2aeff3e..1016353 100644 --- a/classes/task/update_static_page.php +++ b/classes/task/update_static_page.php @@ -37,7 +37,10 @@ class update_static_page extends scheduled_task { } /** - * Executes the event. + * Executes the task: regenerates the maintenance static page for the next scheduled outage. + * + * @throws \coding_exception + * @throws \file_exception */ public function execute() { outagelib::prepare_next_outage(); diff --git a/db/install.xml b/db/install.xml index 1dcc2a1..4f11b41 100644 --- a/db/install.xml +++ b/db/install.xml @@ -21,9 +21,14 @@ + + + + +