From 7ca44c74abac1c953ef7e3dfc4003509aa99e7e3 Mon Sep 17 00:00:00 2001 From: Daniel Thee Roperto Date: Thu, 15 Sep 2016 14:01:45 +1000 Subject: [PATCH] Fixed code inspections, missing phpdocs and code standards. --- classes/event/outage_created.php | 13 +++++++- classes/event/outage_deleted.php | 13 +++++++- classes/event/outage_updated.php | 14 ++++++-- classes/forms/outage/delete.php | 3 +- classes/forms/outage/edit.php | 2 +- classes/models/outage.php | 27 ++++++++-------- classes/outagedb.php | 55 ++++++++++++++++++++++++-------- classes/outagelib.php | 7 ++-- classes/tables/manage.php | 24 ++++++++++++-- clone.php | 1 - delete.php | 1 - edit.php | 1 - renderer.php | 10 ++++++ tests/outage_test.php | 10 +++--- tests/outagedb_test.php | 15 ++++----- tests/outagelib_test.php | 10 +++--- views/warningbar.js | 2 +- 17 files changed, 143 insertions(+), 65 deletions(-) diff --git a/classes/event/outage_created.php b/classes/event/outage_created.php index 28ef9d1..0339818 100644 --- a/classes/event/outage_created.php +++ b/classes/event/outage_created.php @@ -16,6 +16,8 @@ namespace auth_outage\event; +use core\event\base; + defined('MOODLE_INTERNAL') || die(); /** @@ -26,7 +28,7 @@ defined('MOODLE_INTERNAL') || die(); * @copyright Catalyst IT * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class outage_created extends \core\event\base { +class outage_created extends base { /** * Init method. */ @@ -37,11 +39,20 @@ class outage_created extends \core\event\base { $this->context = \context_system::instance(); } + /** + * Returns non-localised event description with id's for admin use only. + * + * @return string + */ public function get_description() { return "The user with the id '{$this->userid}' created a new outage title '{$this->other['title']}' " . " with id '{$this->other['id']}'."; } + /** + * Returns relevant URL, override in subclasses. + * @return \moodle_url + */ public function get_url() { return new \moodle_url('/auth/outage/list.php#auth_outage_id_' . $this->other['id']); } diff --git a/classes/event/outage_deleted.php b/classes/event/outage_deleted.php index f73a516..e1844a7 100644 --- a/classes/event/outage_deleted.php +++ b/classes/event/outage_deleted.php @@ -16,6 +16,8 @@ namespace auth_outage\event; +use core\event\base; + defined('MOODLE_INTERNAL') || die(); /** @@ -26,7 +28,7 @@ defined('MOODLE_INTERNAL') || die(); * @copyright Catalyst IT * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class outage_deleted extends \core\event\base { +class outage_deleted extends base { /** * Init method. */ @@ -37,11 +39,20 @@ class outage_deleted extends \core\event\base { $this->context = \context_system::instance(); } + /** + * Returns non-localised event description with id's for admin use only. + * + * @return string + */ public function get_description() { return "The user with the id '{$this->userid}' deleted the outage titled '{$this->other['title']}' " . "with id '{$this->other['id']}'."; } + /** + * Returns relevant URL, override in subclasses. + * @return \moodle_url + */ public function get_url() { return new \moodle_url('/auth/outage/list.php#auth_outage_id_' . $this->other['id']); } diff --git a/classes/event/outage_updated.php b/classes/event/outage_updated.php index 59a846e..797b680 100644 --- a/classes/event/outage_updated.php +++ b/classes/event/outage_updated.php @@ -16,6 +16,8 @@ namespace auth_outage\event; +use core\event\base; + defined('MOODLE_INTERNAL') || die(); /** @@ -26,7 +28,7 @@ defined('MOODLE_INTERNAL') || die(); * @copyright Catalyst IT * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class outage_updated extends \core\event\base { +class outage_updated extends base { /** * Init method. */ @@ -37,12 +39,20 @@ class outage_updated extends \core\event\base { $this->context = \context_system::instance(); } - + /** + * Returns non-localised event description with id's for admin use only. + * + * @return string + */ public function get_description() { return "The user with the id '{$this->userid}' updated the outage title '{$this->other['title']}' " . "with id '{$this->other['id']}'."; } + /** + * Returns relevant URL, override in subclasses. + * @return \moodle_url + */ public function get_url() { return new \moodle_url('/auth/outage/list.php'); } diff --git a/classes/forms/outage/delete.php b/classes/forms/outage/delete.php index 23cbcd9..2928912 100644 --- a/classes/forms/outage/delete.php +++ b/classes/forms/outage/delete.php @@ -56,5 +56,4 @@ class delete extends \moodleform { return $errors; } - -} \ No newline at end of file +} diff --git a/classes/forms/outage/edit.php b/classes/forms/outage/edit.php index 6ad9628..f4986a8 100644 --- a/classes/forms/outage/edit.php +++ b/classes/forms/outage/edit.php @@ -16,7 +16,7 @@ namespace auth_outage\forms\outage; -use \auth_outage\models\outage; +use auth_outage\models\outage; if (!defined('MOODLE_INTERNAL')) { die('Direct access to this script is forbidden.'); // It must be included from a Moodle page. diff --git a/classes/models/outage.php b/classes/models/outage.php index d258806..4fec627 100644 --- a/classes/models/outage.php +++ b/classes/models/outage.php @@ -14,19 +14,18 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . +namespace auth_outage\models; + +use auth_outage\outagelib; + /** - * An Outage object with all information about one specific outage. + * Outage class with all information about one specific outage. * * @package auth_outage * @author Daniel Thee Roperto * @copyright Catalyst IT * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ - -namespace auth_outage\models; - -use auth_outage\outagelib; - class outage { /** * @var int Outage ID (auto generated by the DB). @@ -143,14 +142,6 @@ class outage { return $this->replace_placeholders($this->title); } - /** - * Get the description with properly replaced placeholders such as {{start}} and {{stop}}. - * @return string Description. - */ - public function get_description() { - return $this->replace_placeholders($this->description); - } - /** * Returns the input string with all placeholders replaced. * @param $str string Input string. @@ -180,6 +171,14 @@ class outage { return $this->stoptime - $this->starttime; } + /** + * Get the description with properly replaced placeholders such as {{start}} and {{stop}}. + * @return string Description. + */ + public function get_description() { + return $this->replace_placeholders($this->description); + } + /** * Gets the warning duration from the outage (from warning time to start time). * @return int Warning duration in seconds. diff --git a/classes/outagedb.php b/classes/outagedb.php index 260f4a6..81557b1 100644 --- a/classes/outagedb.php +++ b/classes/outagedb.php @@ -22,7 +22,12 @@ if (!defined('MOODLE_INTERNAL')) { require_once($CFG->dirroot . '/calendar/lib.php'); +use auth_outage\event\outage_created; +use auth_outage\event\outage_deleted; +use auth_outage\event\outage_updated; use auth_outage\models\outage; +use calendar_event; +use InvalidArgumentException; /** * The DB Context to manipulate Outages. @@ -65,10 +70,10 @@ class outagedb { global $DB; if (!is_int($id)) { - throw new \InvalidArgumentException('$id must be an int.'); + throw new InvalidArgumentException('$id must be an int.'); } if ($id <= 0) { - throw new \InvalidArgumentException('$id must be positive.'); + throw new InvalidArgumentException('$id must be positive.'); } $outage = $DB->get_record('auth_outage', ['id' => $id]); @@ -100,7 +105,7 @@ class outagedb { $outage->createdby = $USER->id; // Then create it, log it and adjust its id. $outage->id = $DB->insert_record('auth_outage', $outage, true); - \auth_outage\event\outage_created::create( + outage_created::create( ['objectid' => $outage->id, 'other' => (array)$outage] )->trigger(); // Create calendar entry. @@ -110,7 +115,7 @@ class outagedb { unset($outage->createdby); $DB->update_record('auth_outage', $outage); // Log it. - \auth_outage\event\outage_updated::create( + outage_updated::create( ['objectid' => $outage->id, 'other' => (array)$outage] )->trigger(); // Update calendar entry. @@ -124,22 +129,22 @@ class outagedb { /** * Deletes an outage from the database. * - * @param $id outage Outage ID to delete + * @param int $id Outage ID to delete * @throws InvalidArgumentException If ID is not valid. */ public static function delete($id) { global $DB; if (!is_int($id)) { - throw new \InvalidArgumentException('$id must be an int.'); + throw new InvalidArgumentException('$id must be an int.'); } if ($id <= 0) { - throw new \InvalidArgumentException('$id must be positive.'); + throw new InvalidArgumentException('$id must be positive.'); } // Log it. $previous = $DB->get_record('auth_outage', ['id' => $id], '*', MUST_EXIST); - $event = \auth_outage\event\outage_deleted::create(['objectid' => $id, 'other' => (array)$previous]); + $event = outage_deleted::create(['objectid' => $id, 'other' => (array)$previous]); $event->add_record_snapshot('auth_outage', $previous); $event->trigger(); @@ -163,7 +168,7 @@ class outagedb { $time = time(); } if (!is_int($time)) { - throw new \InvalidArgumentException('$time must be null or an int.'); + throw new InvalidArgumentException('$time must be null or an int.'); } $data = $DB->get_records_select( @@ -178,7 +183,7 @@ class outagedb { // Not using $DB->get_record_select instead because there is no 'limit' parameter. // Allowing multiple records still raises an internal error. - return (count($data) == 0) ? null : new \auth_outage\models\outage(array_shift($data)); + return (count($data) == 0) ? null : new outage(array_shift($data)); } /** @@ -193,7 +198,7 @@ class outagedb { $time = time(); } if (!is_int($time)) { - throw new \InvalidArgumentException('$time must be null or an int.'); + throw new InvalidArgumentException('$time must be null or an int.'); } $outages = []; @@ -224,7 +229,7 @@ class outagedb { $time = time(); } if (!is_int($time)) { - throw new \InvalidArgumentException('$time must be null or an int.'); + throw new InvalidArgumentException('$time must be null or an int.'); } $outages = []; @@ -243,10 +248,18 @@ class outagedb { return $outages; } + /** + * Create an event on the calendar for this outage. + * @param outage $outage Outage to be added to the calendar. + */ private static function calendar_create(outage $outage) { - \calendar_event::create(self::calendar_data($outage)); + calendar_event::create(self::calendar_data($outage)); } + /** + * Updates an event on the calendar based on this outage. + * @param outage $outage Outage to be updated in the calendar. + */ private static function calendar_update(outage $outage) { $event = self::calendar_load($outage->id); @@ -258,6 +271,10 @@ class outagedb { } } + /** + * Removes an event from the calendar related to this outage. + * @param int $outageid Id of outage to be deleted from the calendar. + */ private static function calendar_delete($outageid) { $event = self::calendar_load($outageid); @@ -269,6 +286,11 @@ class outagedb { } } + /** + * Generates an array with the calendar event data based on an outage object. + * @param outage $outage Outage to use as reference for the calendar event. + * @return array Calendar event data. + */ private static function calendar_data(outage $outage) { return [ 'name' => $outage->get_title(), @@ -285,6 +307,11 @@ class outagedb { ]; } + /** + * Finds the calendar event for an specific outage. + * @param int $outageid The outage id to find in the calendar. + * @return calendar_event|null The calendar event or null if not found. + */ private static function calendar_load($outageid) { global $DB; @@ -296,6 +323,6 @@ class outagedb { IGNORE_MISSING ); - return ($event === false) ? null : \calendar_event::load($event->id); + return ($event === false) ? null : calendar_event::load($event->id); } } diff --git a/classes/outagelib.php b/classes/outagelib.php index d04f79c..3d00819 100644 --- a/classes/outagelib.php +++ b/classes/outagelib.php @@ -16,6 +16,8 @@ namespace auth_outage; +use auth_outage_renderer; + if (!defined('MOODLE_INTERNAL')) { die('Direct access to this script is forbidden.'); // It must be included from a Moodle page. } @@ -33,8 +35,7 @@ class outagelib { /** * Initializes admin pages for outage. - * - * @return \renderer_base + * @return auth_outage_renderer The outage renderer for the page. */ public static function pagesetup() { global $PAGE; @@ -45,7 +46,7 @@ class outagelib { /** * Returns the outage renderer. - * @return \renderer_base + * @return auth_outage_renderer The outage renderer. */ public static function get_renderer() { global $PAGE; diff --git a/classes/tables/manage.php b/classes/tables/manage.php index 54678db..8b1ac37 100644 --- a/classes/tables/manage.php +++ b/classes/tables/manage.php @@ -16,6 +16,9 @@ namespace auth_outage\tables; +use auth_outage\models\outage; +use flexible_table; + require_once($CFG->libdir . '/tablelib.php'); /** @@ -26,9 +29,13 @@ require_once($CFG->libdir . '/tablelib.php'); * @copyright Catalyst IT * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class manage extends \flexible_table { +class manage extends flexible_table { private static $autoid = 0; + /** + * Constructor + * @param string|null $id to be used by the table, autogenerated if null. + */ public function __construct($id = null) { global $PAGE; @@ -51,6 +58,11 @@ class manage extends \flexible_table { $this->setup(); } + /** + * Sets the data of the table. + * @param array $outages An array with outage objects. + * @param bool $editdelete If it should display the edit and delete button. + */ public function set_data(array $outages, $editdelete) { if (!is_bool($editdelete)) { throw new \InvalidArgumentException('$editdelete must be a bool.'); @@ -76,7 +88,13 @@ class manage extends \flexible_table { } } - private function set_data_buttons($outage, $editdelete) { + /** + * Create the action buttons HTML code for a specific outage. + * @param outage $outage The outage to generate the buttons. + * @param bool $editdelete If it should display the edit and delete button. + * @return string The HTML code of the action buttons. + */ + private function set_data_buttons(outage $outage, $editdelete) { global $OUTPUT; $buttons = ''; @@ -135,4 +153,4 @@ class manage extends \flexible_table { return $buttons; } -} \ No newline at end of file +} diff --git a/clone.php b/clone.php index 2adbcd4..7bb9554 100644 --- a/clone.php +++ b/clone.php @@ -23,7 +23,6 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -use auth_outage\models\outage; use auth_outage\outagedb; use auth_outage\outagelib; diff --git a/delete.php b/delete.php index f5ca3cb..7579ccc 100644 --- a/delete.php +++ b/delete.php @@ -23,7 +23,6 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -use auth_outage\models\outage; use auth_outage\outagedb; use auth_outage\outagelib; diff --git a/edit.php b/edit.php index 1d5461e..0dca59a 100644 --- a/edit.php +++ b/edit.php @@ -23,7 +23,6 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -use auth_outage\models\outage; use auth_outage\outagedb; use auth_outage\outagelib; diff --git a/renderer.php b/renderer.php index 9264096..dc9552f 100644 --- a/renderer.php +++ b/renderer.php @@ -30,6 +30,11 @@ if (!defined('MOODLE_INTERNAL')) { * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class auth_outage_renderer extends plugin_renderer_base { + /** + * Renders the subtitle of the page. + * @param string $subtitlekey Key to be used and localized. + * @return string HTML for the subtitle. + */ public function rendersubtitle($subtitlekey) { if (!is_string($subtitlekey)) { throw new \InvalidArgumentException('$subtitle is not a string.'); @@ -37,6 +42,11 @@ class auth_outage_renderer extends plugin_renderer_base { return html_writer::tag('h2', get_string($subtitlekey, 'auth_outage')); } + /** + * Renders a confirmation to delete an outage. + * @param outage $outage Outage to be deleted. + * @return string HTML for the page. + */ public function renderdeleteconfirmation(outage $outage) { return $this->rendersubtitle('outagedelete') . html_writer::tag('p', get_string('outagedeletewarning', 'auth_outage')) diff --git a/tests/outage_test.php b/tests/outage_test.php index 38c21dc..ff2b471 100644 --- a/tests/outage_test.php +++ b/tests/outage_test.php @@ -14,6 +14,10 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . +use auth_outage\models\outage; + +defined('MOODLE_INTERNAL') || die(); + /** * Tests performed on outage class. * @@ -22,12 +26,6 @@ * @copyright Catalyst IT * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ - -use auth_outage\models\outage; - -defined('MOODLE_INTERNAL') || die(); - - class outage_test extends basic_testcase { public function test_constructor() { $outage = new outage(); diff --git a/tests/outagedb_test.php b/tests/outagedb_test.php index adb83b3..2eec538 100644 --- a/tests/outagedb_test.php +++ b/tests/outagedb_test.php @@ -14,6 +14,11 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . +use auth_outage\models\outage; +use auth_outage\outagedb; + +defined('MOODLE_INTERNAL') || die(); + /** * Tests performed on outage class. * @@ -22,13 +27,6 @@ * @copyright Catalyst IT * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ - -use auth_outage\models\outage; -use auth_outage\outagedb; - -defined('MOODLE_INTERNAL') || die(); - - class outagedb_test extends advanced_testcase { /** * Ensure DB tests run as admin. @@ -40,7 +38,8 @@ class outagedb_test extends advanced_testcase { /** * Creates an array of ids in from the given outages array. - * @param $outages + * @param array $outages An array of outages. + * @return array An array with the keys of the outages as values. */ private static function createidarray(array $outages) { $ids = []; diff --git a/tests/outagelib_test.php b/tests/outagelib_test.php index ce4c5a1..4a08b5f 100644 --- a/tests/outagelib_test.php +++ b/tests/outagelib_test.php @@ -14,6 +14,10 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . +use auth_outage\outagelib; + +defined('MOODLE_INTERNAL') || die(); + /** * Tests performed on outageutils class. * @@ -22,12 +26,6 @@ * @copyright Catalyst IT * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ - -use auth_outage\outagelib; - -defined('MOODLE_INTERNAL') || die(); - - class outagelib_test extends basic_testcase { public function test_data2object() { diff --git a/views/warningbar.js b/views/warningbar.js index def3363..31dd73d 100644 --- a/views/warningbar.js +++ b/views/warningbar.js @@ -1,5 +1,5 @@ var auth_outage_countdown = { - timer: null, + timer: 0, clienttime: Date.now(), siteadmin: false, init: function (countdown, siteadmin, redirectto) {