Issue #46 - Added tests for every class, except outputs. Small bugs/invalid parameter detection fixed together.

This commit is contained in:
Daniel Thee Roperto
2016-09-26 19:34:27 +10:00
parent 619a5a663f
commit 8791727ba4
28 changed files with 1090 additions and 239 deletions

View File

@@ -20,7 +20,6 @@ use auth_outage\local\outage;
use calendar_event;
defined('MOODLE_INTERNAL') || die();
require_once($CFG->dirroot.'/calendar/lib.php');
/**
@@ -34,6 +33,7 @@ require_once($CFG->dirroot.'/calendar/lib.php');
class calendar {
/**
* Private constructor, use static methods instead.
* @codeCoverageIgnore
*/
private function __construct() {
}
@@ -42,22 +42,23 @@ class calendar {
* Create an event on the calendar for this outage.
* @param outage $outage Outage to be added to the calendar.
*/
public static function calendar_create(outage $outage) {
calendar_event::create(self::calendar_data($outage));
public static function create(outage $outage) {
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.
* @SuppressWarnings("comment") Allow this test to have as many tests as necessary.
*/
public static function calendar_update(outage $outage) {
$event = self::calendar_load($outage->id);
public static function update(outage $outage) {
$event = self::load($outage->id);
if (is_null($event)) {
debugging('Cannot update calendar entry for outage #'.$outage->id.', event not found. Creating it...');
self::calendar_create($outage);
self::create($outage);
} else {
$event->update(self::calendar_data($outage));
$event->update(self::create_data($outage));
}
}
@@ -65,8 +66,8 @@ class calendar {
* Removes an event from the calendar related to this outage.
* @param int $outageid Id of outage to be deleted from the calendar.
*/
public static function calendar_delete($outageid) {
$event = self::calendar_load($outageid);
public static function delete($outageid) {
$event = self::load($outageid);
// If not found (was not created before) ignore it.
if (is_null($event)) {
@@ -81,7 +82,7 @@ class calendar {
* @param outage $outage Outage to use as reference for the calendar event.
* @return mixed[] Calendar event data.
*/
private static function calendar_data(outage $outage) {
private static function create_data(outage $outage) {
return [
'name' => $outage->get_title(),
'description' => $outage->get_description(),
@@ -102,7 +103,7 @@ class calendar {
* @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) {
public static function load($outageid) {
global $DB;
$event = $DB->get_record_select(

View File

@@ -40,6 +40,7 @@ require_once($CFG->dirroot.'/calendar/lib.php');
class outagedb {
/**
* Private constructor, use static methods instead.
* @codeCoverageIgnore
*/
private function __construct() {
}
@@ -62,7 +63,7 @@ class outagedb {
}
/**
* @param $id int Outage id to get.
* @param int $id Outage id to get.
* @return outage|null Returns the outage or null if not found.
* @throws coding_exception
*/
@@ -106,7 +107,7 @@ class outagedb {
['objectid' => $outage->id, 'other' => (array)$outage]
)->trigger();
// Create calendar entry.
calendar::calendar_create($outage);
calendar::create($outage);
} else {
// Remove the createdby field so it does not get updated.
unset($outage->createdby);
@@ -116,7 +117,7 @@ class outagedb {
['objectid' => $outage->id, 'other' => (array)$outage]
)->trigger();
// Update calendar entry.
calendar::calendar_update($outage);
calendar::update($outage);
}
// Trigger outages modified events.
@@ -147,7 +148,7 @@ class outagedb {
// Delete it and remove from calendar.
$DB->delete_records('auth_outage', ['id' => $id]);
calendar::calendar_delete($id);
calendar::delete($id);
// Trigger events.
outagelib::outages_modified();

98
classes/form/baseform.php Normal file
View File

@@ -0,0 +1,98 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
namespace auth_outage\form;
use moodleform;
defined('MOODLE_INTERNAL') || die();
require_once($CFG->libdir.'/formslib.php');
/**
* Outage base for forms, extends Moodle form to fix a but in the validation method.
*
* @package auth_outage
* @author Daniel Thee Roperto <daniel.roperto@catalyst-au.net>
* @copyright 2016 Catalyst IT
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
abstract class baseform extends moodleform {
/**
* Validate the form.
*
* You almost always want to call {@link is_validated} instead of this
* because it calls {@link definition_after_data} first, before validating the form,
* which is what you want in 99% of cases.
*
* This is provided as a separate function for those special cases where
* you want the form validated before definition_after_data is called
* for example, to selectively add new elements depending on a no_submit_button press,
* but only when the form is valid when the no_submit_button is pressed,
*
* @param bool $validateonnosubmit optional, defaults to false. The default behaviour
* is NOT to validate the form when a no submit button has been pressed.
* pass true here to override this behaviour
*
* @return bool true if form data valid
* @SuppressWarnings(PHPMD) It is better to not refactor this method as it is linked to its parent functionality.
* @codeCoverageIgnore
*/
public function validate_defined_fields($validateonnosubmit = false) {
// One validation NOT is enough (if mocking). See parent method.
$mform =& $this->_form;
if ($this->no_submit_button_pressed() && empty($validateonnosubmit)) {
return false;
}
$internalval = $mform->validate();
$files = [];
$fileval = $this->_validate_files($files);
// Check draft files for validation and flag them if required files are not in draft area.
$draftfilevalue = $this->validate_draft_files();
if ($fileval !== true && $draftfilevalue !== true) {
$fileval = array_merge($fileval, $draftfilevalue);
} else if ($draftfilevalue !== true) {
$fileval = $draftfilevalue;
} //default is file_val, so no need to assign.
if ($fileval !== true) {
if (!empty($fileval)) {
foreach ($fileval as $element => $msg) {
$mform->setElementError($element, $msg);
}
}
$fileval = false;
}
$data = $mform->exportValues();
$moodleval = $this->validation($data, $files);
if ((is_array($moodleval) && count($moodleval) !== 0)) {
// Non-empty array means errors.
foreach ($moodleval as $element => $msg) {
$mform->setElementError($element, $msg);
}
$moodleval = false;
} else {
// Anything else means validation ok.
$moodleval = true;
}
$validated = ($internalval and $moodleval and $fileval);
return $validated;
}
}

View File

@@ -32,8 +32,7 @@ defined('MOODLE_INTERNAL') || die();
*/
class delete extends \moodleform {
/**
* {@inheritDoc}
* @see moodleform::definition()
* Defines the form elements.
*/
public function definition() {
$mform = $this->_form;

View File

@@ -16,9 +16,9 @@
namespace auth_outage\form\outage;
use auth_outage\form\baseform;
use auth_outage\local\outage;
use coding_exception;
use moodleform;
defined('MOODLE_INTERNAL') || die();
@@ -32,7 +32,7 @@ require_once($CFG->libdir.'/formslib.php');
* @copyright 2016 Catalyst IT
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class edit extends moodleform {
class edit extends baseform {
const TITLE_MAX_CHARS = 100;
/**

View File

@@ -284,6 +284,7 @@ class create extends clibase {
}
}
throw new cli_exception(get_string('clierrorinvalidvalue', 'auth_outage', ['param' => $param]));
throw new cli_exception(get_string('clierrorinvalidvalue', 'auth_outage', ['param' => $param]),
cli_exception::ERROR_PARAMETER_INVALID);
}
}

View File

@@ -74,10 +74,11 @@ class infopage {
* Given the HTML code for the static page, find the outage id for that page.
* @param string $html Static info page HTML.
* @return int|null Outage id or null if not found.
* @throws coding_exception
*/
public static function find_outageid_from_infopage($html) {
if (!is_string($html)) {
throw new coding_exception('$html must be a string.', $html);
throw new coding_exception('$html must be a string.', gettype($html));
}
$output = [];
@@ -103,10 +104,7 @@ class infopage {
$info = new infopage(['outage' => $outage, 'static' => true]);
$html = $info->get_output();
// Sanity check before writing/overwriting old file.
if (!is_string($html) || ($html == '') || (html_to_text($html) == '')) {
throw new invalid_state_exception('Sanity check failed. Invalid contents on $html.');
}
self::save_static_page_sanitycheck($html);
$dir = dirname($file);
if (!file_exists($dir) || !is_dir($dir)) {
@@ -117,7 +115,9 @@ class infopage {
/**
* Updates the static info page by (re)creating or deleting it as needed.
* @param null $file
* @param string|null $file File to update. Null to use default.
* @throws coding_exception
* @throws file_exception
*/
public static function update_static_page($file = null) {
if (is_null($file)) {
@@ -149,19 +149,33 @@ class infopage {
return $CFG->dataroot.'/climaintenance.template.html';
}
/**
* Ensures the data to create the page is valid.
* It should never be invalid, but if it is we will get a blank page while the maintenance is ongoing and the
* system administrators may become frustrated to understand why it is not working, let's not provoke them!
* @param string $html The HTML to check.
* @throws invalid_state_exception
*/
public static function save_static_page_sanitycheck($html) {
if (!is_string($html) || (trim($html) == '') || (trim(html_to_text($html)) == '')) {
throw new invalid_state_exception('Sanity check failed. Invalid contents on $html.');
}
}
/**
* Generates and returns the HTML for the info page.
* @return string HTML for the info page.
*/
public function get_output() {
ob_start();
$output = null;
try {
// TODO what if redirection occurs here?
$this->output();
return ob_get_contents();
$output = ob_get_contents();
} finally {
ob_end_clean();
}
return $output;
}
/**
@@ -213,6 +227,7 @@ class infopage {
/**
* Adjusts the fields according to the given parameters.
* @param mixed[] $params
* @throws coding_exception
*/
private function set_parameters(array $params) {
if (!is_null($params['outage']) && !($params['outage'] instanceof outage)) {
@@ -220,7 +235,7 @@ class infopage {
}
if (!is_null($params['id']) && !is_null($params['outage']) && ($params['id'] !== $params['outage']->id)) {
throw new coding_exception('Provided id and outage->id do not match.', $params);
throw new coding_exception('Provided id and outage->id do not match.', $params['id'].'/'.$params['outage']->id);
}
if (is_null($params['id']) && is_null($params['outage'])) {

View File

@@ -20,7 +20,6 @@ use auth_outage\dml\outagedb;
use auth_outage\local\controllers\infopage;
use auth_outage\output\renderer;
use Exception;
use moodle_url;
use stdClass;
defined('MOODLE_INTERNAL') || die();
@@ -34,17 +33,14 @@ defined('MOODLE_INTERNAL') || die();
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class outagelib {
private static $initialized = false;
private static $injected = false;
/**
* Initializes admin pages for outage.
* @return renderer The outage renderer for the page.
* Calls inject even if it was already called before.
*/
public static function page_setup() {
global $PAGE;
admin_externalpage_setup('auth_outage_manage');
$PAGE->set_url(new moodle_url('/auth/outage/manage.php'));
return renderer::get();
public static function reinject() {
self::$injected = false;
self::inject();
}
/**
@@ -54,13 +50,18 @@ class outagelib {
global $CFG;
// Many hooks can call it, execute only once.
if (self::$initialized) {
if (self::$injected) {
return;
}
self::$initialized = true;
self::$injected = true;
// Ensure we do not kill the whole website in case of an error.
try {
// Ensure no exceptions break the code.
if (PHPUNIT_TEST && optional_param('auth_outage_break_code', false, PARAM_INT)) {
(new stdClass())->invalidfield;
}
// Check for a previewing outage, then for an active outage.
$previewid = optional_param('auth_outage_preview', null, PARAM_INT);
$time = time();
@@ -74,7 +75,8 @@ class outagelib {
return;
}
// Delta is in seconds, setting the time our warning bar will consider relative to the outage start time.
$time = $active->starttime + optional_param('auth_outage_delta', 0, PARAM_INT);
$delta = optional_param('auth_outage_delta', 0, PARAM_INT);
$time = $active->starttime + $delta;
if (!$active->is_active($time)) {
return;
}
@@ -134,10 +136,8 @@ class outagelib {
} else {
$message = get_config('moodle', 'maintenance_message');
if ($message) {
if (!defined('PHPUNIT_TEST') || !PHPUNIT_TEST) {
error_log('Disabling $CFG->maintenance_message to allow our template page to take place.');
error_log('Previous value: '.$message);
}
debugging('Disabling $CFG->maintenance_message to allow our template page to take place.');
debugging('Previous value: '.$message);
// We cannot do much if forced config, but the logs will show the error.
unset_config('maintenance_message');
}