From ee771daecf24797dc9b0d034c5473b252d855c64 Mon Sep 17 00:00:00 2001 From: Daniel Thee Roperto Date: Mon, 12 Sep 2016 13:57:33 +1000 Subject: [PATCH] Issue #11 - Small fixed as suggested by Brendan. --- classes/forms/outage/edit.php | 6 ++-- classes/models/outage.php | 39 ---------------------- classes/outagedb.php | 34 +------------------ classes/tables/manage.php | 17 +++++++--- edit.php | 2 +- lang/en/auth_outage.php | 7 ++-- manage.php | 2 +- renderer.php | 33 +++++++------------ tests/outagedb_test.php | 62 +++++++++++------------------------ 9 files changed, 53 insertions(+), 149 deletions(-) diff --git a/classes/forms/outage/edit.php b/classes/forms/outage/edit.php index a4e8152..9ac36e1 100644 --- a/classes/forms/outage/edit.php +++ b/classes/forms/outage/edit.php @@ -45,17 +45,17 @@ class edit extends \moodleform { $mform->addElement('hidden', 'id'); $mform->setType('id', PARAM_INT); + $mform->addElement('duration', 'warningduration', get_string('warningduration', 'auth_outage')); + $mform->addElement('date_time_selector', 'starttime', get_string('starttime', 'auth_outage')); $mform->addElement('duration', 'outageduration', get_string('outageduration', 'auth_outage')); - $mform->addElement('duration', 'warningduration', get_string('warningduration', 'auth_outage')); - $mform->addElement( 'text', 'title', get_string('title', 'auth_outage'), - 'maxlength="' . self::TITLE_MAX_CHARS . '"' + 'maxlength="' . self::TITLE_MAX_CHARS . '" size="60"' ); $mform->setType('title', PARAM_TEXT); diff --git a/classes/models/outage.php b/classes/models/outage.php index ab24dd3..320f7ab 100644 --- a/classes/models/outage.php +++ b/classes/models/outage.php @@ -28,29 +28,6 @@ namespace auth_outage\models; use auth_outage\outagelib; class outage { - private static function get_seconds_duration_string($duration) { - if (!is_int($duration)) { - throw new \InvalidArgumentException('$seconds must be an int.'); - } - - if (($duration < 60) || ($duration % 60 != 0)) { - return $duration . ' ' . get_string('durationseconds', 'auth_outage'); - } - - $duration /= 60; - if (($duration < 60) || ($duration % 60 != 0)) { - return $duration . ' ' . get_string('durationminutes', 'auth_outage'); - } - - $duration /= 60; - if (($duration < 60) || ($duration % 24 != 0)) { - return $duration . ' ' . get_string('durationhours', 'auth_outage'); - } - - $duration /= 24; - return $duration . ' ' . get_string('durationdays', 'auth_outage'); - } - /** * @var int Outage ID (auto generated by the DB). */ @@ -182,14 +159,6 @@ class outage { return $this->stoptime - $this->starttime; } - /** - * Gets the duration of the outage (start to stop, warning not included). - * @return string The duration as text, for example '6 hour(s)'. - */ - public function get_duration_string() { - return self::get_seconds_duration_string($this->get_duration()); - } - /** * Gets the warning duration from the outage (from warning time to start time). * @return int Warning duration in seconds. @@ -197,12 +166,4 @@ class outage { public function get_warning_duration() { return $this->starttime - $this->warntime; } - - /** - * Gets the warning duration from the outage (from warning time to start time). - * @return string The warning duration as text, for example '6 hour(s)'. - */ - public function get_warning_duration_string() { - return self::get_seconds_duration_string($this->get_warning_duration()); - } } diff --git a/classes/outagedb.php b/classes/outagedb.php index e211796..2726590 100644 --- a/classes/outagedb.php +++ b/classes/outagedb.php @@ -168,37 +168,6 @@ class outagedb { return (count($data) == 0) ? null : new \auth_outage\models\outage(array_shift($data)); } - /** - * Gets all active outages (including in warning period). - * @param int|null $time Timestamp considered to check for outages, null for current date/time. - * @return array An array of outages or an empty array if no active outage found. - */ - public static function get_all_active($time = null) { - global $DB; - - if ($time === null) { - $time = time(); - } - if (!is_int($time)) { - throw new \InvalidArgumentException('$time must be null or an int.'); - } - - $outages = []; - - $rs = $DB->get_recordset_select( - 'auth_outage', - '(warntime <= :datetime1 AND stoptime >= :datetime2)', - ['datetime1' => $time, 'datetime2' => $time], - 'starttime ASC, stoptime DESC, title ASC', - '*'); - foreach ($rs as $r) { - $outages[] = new outage($r); - } - $rs->close(); - - return $outages; - } - /** * Gets all future outages not in warning period. * @param int|null $time Timestamp considered to check for outages, null for current date/time. @@ -218,7 +187,7 @@ class outagedb { $rs = $DB->get_recordset_select( 'auth_outage', - 'warntime > :datetime', + 'stoptime >= :datetime', ['datetime' => $time], 'starttime ASC, stoptime DESC, title ASC', '*'); @@ -230,7 +199,6 @@ class outagedb { return $outages; } - /** * Gets all past outages. * @param int|null $time Timestamp considered to check for outages, null for current date/time. diff --git a/classes/tables/manage.php b/classes/tables/manage.php index 564117a..c6fa874 100644 --- a/classes/tables/manage.php +++ b/classes/tables/manage.php @@ -38,11 +38,11 @@ class manage extends \flexible_table { $this->define_columns(['starttime', 'stopsafter', 'warnbefore', 'title', '']); $this->define_headers([ + get_string('tableheaderwarnbefore', 'auth_outage'), get_string('tableheaderstarttime', 'auth_outage'), get_string('tableheaderstopsafter', 'auth_outage'), - get_string('tableheaderwarnbefore', 'auth_outage'), get_string('tableheadertitle', 'auth_outage'), - '', + get_string('actions'), ] ); @@ -71,6 +71,7 @@ class manage extends \flexible_table { 'target' => '_blank', ] ); + $title = $outage->get_title(); if ($editdelete) { $buttons .= \html_writer::link( new \moodle_url('/auth/outage/edit.php', ['id' => $outage->id]), @@ -90,13 +91,19 @@ class manage extends \flexible_table { ]), ['title' => get_string('delete')] ); + + $title = \html_writer::link( + new \moodle_url('/auth/outage/edit.php', ['id' => $outage->id]), + $title, + ['title' => get_string('edit')] + ); } $this->add_data([ + format_time($outage->get_warning_duration()), userdate($outage->starttime, get_string('tablerowstarts', 'auth_outage')), - $outage->get_duration_string(), - $outage->get_warning_duration_string(), - $outage->get_title(), + format_time($outage->get_duration()), + $title, $buttons, ]); } diff --git a/edit.php b/edit.php index c8c177e..1d5461e 100644 --- a/edit.php +++ b/edit.php @@ -49,7 +49,7 @@ if ($outage == null) { } $mform->set_data($outage); -$PAGE->navbar->add($outage->title); +$PAGE->navbar->add($outage->get_title()); echo $OUTPUT->header(); echo $renderer->rendersubtitle('outageedit'); $mform->display(); diff --git a/lang/en/auth_outage.php b/lang/en/auth_outage.php index e364bdd..6fbc561 100644 --- a/lang/en/auth_outage.php +++ b/lang/en/auth_outage.php @@ -52,9 +52,8 @@ $string['outagedelete'] = 'Delete Outage'; $string['outagedeletewarning'] = 'You are about to permanently delete the outage below. This cannot be undone.'; $string['outageduration'] = 'Outage Duration'; $string['outagedurationerrorinvalid'] = 'Outage duration must be positive.'; -$string['outageslistactive'] = 'Active Outages'; -$string['outageslistfuture'] = 'Future Outages'; -$string['outageslistpast'] = 'Past Outages'; +$string['outageslistfuture'] = 'Planned outages'; +$string['outageslistpast'] = 'Outage history'; $string['pluginname'] = 'Outage manager'; $string['readmore'] = 'Read More'; $string['starttime'] = 'Start date and time'; @@ -62,7 +61,7 @@ $string['tableheaderstarttime'] = 'Starts on'; $string['tableheaderstopsafter'] = 'Stops after'; $string['tableheaderwarnbefore'] = 'Warns before'; $string['tableheadertitle'] = 'Title'; -$string['tablerowstarts'] = '%d/%m/%Y %H:%M'; +$string['tablerowstarts'] = '%d %h %Y at %H:%M'; $string['textplaceholdershint'] = 'You can use {{start}} and {{stop}} as placeholders on the title/description for the actual start/stop time.'; $string['titleerrorinvalid'] = 'Title cannot be left blank.'; $string['titleerrortoolong'] = 'Title cannot have more than {$a} characters.'; diff --git a/manage.php b/manage.php index c0fa8fb..ba6bab1 100644 --- a/manage.php +++ b/manage.php @@ -33,6 +33,6 @@ $renderer = outagelib::pagesetup(); echo $OUTPUT->header(); -$renderer->renderoutagelist(outagedb::get_all_active(), outagedb::get_all_future(), outagedb::get_all_past()); +$renderer->renderoutagelist(outagedb::get_all_future(), outagedb::get_all_past()); echo $OUTPUT->footer(); diff --git a/renderer.php b/renderer.php index f7a33e9..b6d023a 100644 --- a/renderer.php +++ b/renderer.php @@ -47,15 +47,20 @@ class auth_outage_renderer extends plugin_renderer_base { * Outputs the HTML data listing all given outages. * @param array $outages Outages to list. */ - public function renderoutagelist(array $active, array $future, array $past) { + public function renderoutagelist(array $future, array $past) { global $OUTPUT; - if (!empty($active)) { - echo $this->rendersubtitle('outageslistactive'); - $table = new \auth_outage\tables\manage(); - $table->set_data($active, true); - $table->finish_output(); - } + // Add 'add' button. + $url = new moodle_url('/auth/outage/new.php'); + $img = html_writer::empty_tag('img', + ['src' => $OUTPUT->pix_url('t/add'), 'alt' => get_string('create'), 'class' => 'iconsmall']); + echo html_writer::tag('p', + html_writer::link( + $url, + $img . ' ' . get_string('outagecreate', 'auth_outage'), + ['title' => get_string('delete')] + ) + ); echo $this->rendersubtitle('outageslistfuture'); if (empty($future)) { @@ -74,20 +79,6 @@ class auth_outage_renderer extends plugin_renderer_base { $table->set_data($past, false); $table->finish_output(); } - - // Add 'add' button. - $url = new moodle_url('/auth/outage/new.php'); - $img = html_writer::empty_tag('img', - ['src' => $OUTPUT->pix_url('t/add'), 'alt' => get_string('create'), 'class' => 'iconsmall']); - echo - html_writer::empty_tag('hr') - . html_writer::tag('p', - html_writer::link( - $url, - $img . ' ' . get_string('outagecreate', 'auth_outage'), - ['title' => get_string('delete')] - ) - ); } private function renderoutage(outage $outage, $buttons) { diff --git a/tests/outagedb_test.php b/tests/outagedb_test.php index 976ae11..d00112f 100644 --- a/tests/outagedb_test.php +++ b/tests/outagedb_test.php @@ -182,42 +182,6 @@ class outagedb_test extends advanced_testcase { self::assertSame($activeid, outagedb::get_active($now)->id, 'Wrong active outage picked.'); } - public function test_getallactive() { - $this->resetAfterTest(true); - - // Have a consistent time for now (no seconds variation), helps debugging. - $now = time(); - - self::assertEquals([], outagedb::get_all(), 'Ensure there are no other outages that can affect the test.'); - self::assertEquals([], outagedb::get_all_active($now), 'There should be no active outages at this point.'); - - self::saveoutage($now, 1, 2, 3, 'An outage that starts in the future and is not in warning period.'); - self::assertEquals([], outagedb::get_all_active($now), 'No active outages yet.'); - - self::saveoutage($now, -3, -2, -1, 'An outage that is already in the past.'); - self::assertEquals([], outagedb::get_all_active($now), 'No active outages yet.'); - - $id1 = self::saveoutage($now, -2, 1, 2, 'An outage in warning period.'); - self::assertEquals([$id1], - self::createidarray(outagedb::get_all_active($now)), 'Wrong actives data.'); - - $id2 = self::saveoutage($now, -1, 2, 3, 'Another outage in warning period.'); - self::assertEquals([$id1, $id2], - self::createidarray(outagedb::get_all_active($now)), 'Wrong actives data.'); - - $id3 = self::saveoutage($now, -3, -2, 2, 'An ongoing outage.'); - self::assertEquals([$id3, $id1, $id2], - self::createidarray(outagedb::get_all_active($now)), 'Wrong actives data.'); - - $id4 = self::saveoutage($now, -3, -1, 1, 'Another ongoing outage.'); - self::assertEquals([$id3, $id4, $id1, $id2], - self::createidarray(outagedb::get_all_active($now)), 'Wrong actives data.'); - - $id5 = self::saveoutage($now, -3, -2, 1, 'Yet another ongoing outage.'); - self::assertEquals([$id3, $id5, $id4, $id1, $id2], - self::createidarray(outagedb::get_all_active($now)), 'Wrong actives data.'); - } - public function test_getallfuture() { $this->resetAfterTest(true); @@ -230,12 +194,6 @@ class outagedb_test extends advanced_testcase { self::saveoutage($now, -3, -2, -1, 'A past outage.'); self::assertEquals([], outagedb::get_all_future($now), 'No future outages yet.'); - self::saveoutage($now, -2, 1, 2, 'An outage in warning period.'); - self::assertEquals([], outagedb::get_all_future($now), 'No future outages yet.'); - - self::saveoutage($now, -3, -2, 2, 'An ongoing outage.'); - self::assertEquals([], outagedb::get_all_future($now), 'No future outages yet.'); - $id1 = self::saveoutage($now, 2, 3, 4, 'A future outage.'); self::assertEquals([$id1], self::createidarray(outagedb::get_all_future($now)), 'Wrong future data.'); @@ -247,6 +205,26 @@ class outagedb_test extends advanced_testcase { $id3 = self::saveoutage($now, 1, 3, 5, 'Yet another future outage.'); self::assertEquals([$id3, $id1, $id2], self::createidarray(outagedb::get_all_future($now)), 'Wrong future data.'); + + $id4 = self::saveoutage($now, -2, 1, 2, 'An outage in warning period.'); + self::assertEquals([$id4, $id3, $id1, $id2], + self::createidarray(outagedb::get_all_future($now)), 'Wrong future data.'); + + $id5 = self::saveoutage($now, -1, 2, 3, 'Another outage in warning period.'); + self::assertEquals([$id4, $id5, $id3, $id1, $id2], + self::createidarray(outagedb::get_all_future($now)), 'Wrong future data.'); + + $id6 = self::saveoutage($now, -3, -2, 2, 'An ongoing outage.'); + self::assertEquals([$id6, $id4, $id5, $id3, $id1, $id2], + self::createidarray(outagedb::get_all_future($now)), 'Wrong future data.'); + + $id7 = self::saveoutage($now, -3, -1, 1, 'Another ongoing outage.'); + self::assertEquals([$id6, $id7, $id4, $id5, $id3, $id1, $id2], + self::createidarray(outagedb::get_all_future($now)), 'Wrong future data.'); + + $id8 = self::saveoutage($now, -3, -2, 1, 'Yet another ongoing outage.'); + self::assertEquals([$id6, $id8, $id7, $id4, $id5, $id3, $id1, $id2], + self::createidarray(outagedb::get_all_future($now)), 'Wrong future data.'); } public function test_getallpast() {