From 6dfcbf4fb255d421994bf266bc717b70e195f9b0 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Mon, 18 Sep 2023 14:10:23 +0200 Subject: [PATCH] Fix CI issuesFix --- .github/workflows/ci.yml | 2 +- classes/attempt.php | 4 ++- classes/attempt_storage.php | 6 ++--- classes/custom_string_manager.php | 5 ++++ classes/embed_id.php | 3 +++ classes/embed_location.php | 2 +- classes/output/embed_iframe.php | 5 +++- classes/output/renderer.php | 6 +++-- classes/token.php | 8 +++--- classes/utils.php | 6 ++--- lib.php | 3 ++- tests/attempt_test.php | 9 ++++--- tests/behat/behat_filter_embedquestion.php | 30 ++++------------------ tests/cleanup_task_test.php | 7 ++--- tests/external_test.php | 7 ++--- tests/filter_test.php | 11 ++++---- tests/generator/lib.php | 8 +++--- tests/token_test.php | 7 ++--- tests/utils_test.php | 10 ++++---- 19 files changed, 71 insertions(+), 68 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 30e15bb..380fe77 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -93,7 +93,7 @@ jobs: - name: Moodle PHPDoc Checker if: ${{ always() }} - run: moodle-plugin-ci phpdoc --max-warnings 0 + run: moodle-plugin-ci phpdoc - name: Validating if: ${{ always() }} diff --git a/classes/attempt.php b/classes/attempt.php index b0d6af5..3a15d12 100644 --- a/classes/attempt.php +++ b/classes/attempt.php @@ -200,6 +200,8 @@ public function setup_usage_info(\question_usage_by_activity $quba, int $slot) { } /** + * Start a new attempt at a question in the current usage. + * * @param \question_usage_by_activity|null $quba usage to use. If null will continue using the same usage. */ public function start_new_attempt_at_question( @@ -410,7 +412,7 @@ public function log_view() { /** * Render the currently active question, including the required form. * - * @param \filter_embedquestion\output\renderer instance of our renderer to use. + * @param \filter_embedquestion\output\renderer $renderer instance of our renderer to use. * @return string HTML to display. */ public function render_question(\filter_embedquestion\output\renderer $renderer): string { diff --git a/classes/attempt_storage.php b/classes/attempt_storage.php index 92925ac..658bd95 100644 --- a/classes/attempt_storage.php +++ b/classes/attempt_storage.php @@ -30,7 +30,7 @@ class attempt_storage { /** - * Private constructor. Use {@link instance()} to get an instance. + * Private constructor. Use {@see instance()} to get an instance. */ protected function __construct() { } @@ -77,7 +77,7 @@ public function update_timemodified(int $qubaid): void { * Make a new usage. Will only be called if find_existing_attempt has not found anything. * * Do not try to save the new usage yet. That won't work until MDL-66685 is fixed. - * {@link new_usage_saved()} will be called once the usage id is known. + * {@see new_usage_saved()} will be called once the usage id is known. * * @param embed_id $embedid identity of the question(s) being embedded in this place. * @param embed_location $embedlocation where the question(s) are being embedded. @@ -94,7 +94,7 @@ public function make_new_usage(embed_id $embedid, embed_location $embedlocation, /** * New usage has been saved so we now know its id. * - * Called after {@link make_new_usage()} and after the at least one + * Called after {@see make_new_usage()} and after the at least one * question_attempt has been added. * * @param \question_usage_by_activity $quba the usage that has just been saved. diff --git a/classes/custom_string_manager.php b/classes/custom_string_manager.php index 5ea6e3f..b6b6797 100644 --- a/classes/custom_string_manager.php +++ b/classes/custom_string_manager.php @@ -30,6 +30,11 @@ class custom_string_manager extends \core_string_manager_standard { /** @var string language to force. */ protected $forcedlanguage; + /** + * Force all UI text to this language from now on. + * + * @param string Moodle language code, e.g. 'fr'. + */ public static function force_page_language($lang) { global $CFG; diff --git a/classes/embed_id.php b/classes/embed_id.php index 5a0ef44..731a4f6 100644 --- a/classes/embed_id.php +++ b/classes/embed_id.php @@ -24,7 +24,10 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class embed_id { + /** @var string[] Characters that need to be escaped. */ private const TO_ESCAPE = ['%', '/', '|']; + + /** @var string[] The escaped equivalents. */ private const ESCAPED = ['%25', '%2F', '%7C']; /** diff --git a/classes/embed_location.php b/classes/embed_location.php index c5c3515..b07b776 100644 --- a/classes/embed_location.php +++ b/classes/embed_location.php @@ -89,7 +89,7 @@ public static function make_from_url_params(): embed_location { } /** - * Should only be used by test code (@link report_embedquestion_generator}. + * Should only be used by test code {@see report_embedquestion_generator}. * * Make an instance with specific properies. * diff --git a/classes/output/embed_iframe.php b/classes/output/embed_iframe.php index 01edddb..e259ad9 100644 --- a/classes/output/embed_iframe.php +++ b/classes/output/embed_iframe.php @@ -28,7 +28,10 @@ */ class embed_iframe implements \renderable, \templatable { /** @var \moodle_url for the iframe src attribute. */ - private $showquestionurl; + public $showquestionurl; + + /** @var string Accessibility text for the iframe. */ + public $iframedescription = ''; /** * The error_message constructor. diff --git a/classes/output/renderer.php b/classes/output/renderer.php index 8ff7996..bdc6d4c 100644 --- a/classes/output/renderer.php +++ b/classes/output/renderer.php @@ -142,8 +142,10 @@ public function render_questionbank_link(\question_usage_by_activity $quba, int /** * Insert a template element into info element. * - * @params string $template Info template. - * @params string $childtemplate Child template. + * If the info div is not found, the content is just added at the end. + * + * @param string $template HTML of the rendered question, containing at least the info
. + * @param string $childtemplate HTML to insert at the end of, and inside, the info div. * @return string The combined template. */ protected function insert_html_into_info_section(string $template, string $childtemplate): string { diff --git a/classes/token.php b/classes/token.php index fc42439..e785066 100644 --- a/classes/token.php +++ b/classes/token.php @@ -37,7 +37,7 @@ public static function make_secret_token(embed_id $embedid): string { } /** - * Helper used by @link{add_iframe_token_to_url()}. + * Helper used by {@see add_iframe_token_to_url()}. * * @param string $otherurlparams the URL params to 'sign'. * @return string the security token. @@ -48,9 +48,9 @@ protected static function make_iframe_token(string $otherurlparams): string { } /** - * Do not call this directly. It is expected that this will only be called by @link{utils::get_show_url()}. + * Do not call this directly. It is expected that this will only be called by {@see utils::get_show_url()}. * - * @param \moodle_url $url. + * @param \moodle_url $url The URL to add the token to. */ public static function add_iframe_token_to_url(\moodle_url $url): void { $url->param('token', self::make_iframe_token($url->get_query_string(false))); @@ -59,7 +59,7 @@ public static function add_iframe_token_to_url(\moodle_url $url): void { /** * Check whether a token matches using any of the authorised keys. * - * @param string the security token to be verified. + * @param string $token the security token to be verified. * @param embed_id $embedid the embed code. * @return bool if authorized then true, otherwise false. */ diff --git a/classes/utils.php b/classes/utils.php index cd9bb3f..65125cf 100644 --- a/classes/utils.php +++ b/classes/utils.php @@ -198,7 +198,7 @@ public static function get_question_by_idnumber(int $categoryid, string $idnumbe * This method can only be called if you have already verified that * {@see has_question_versionning()} returns true. * - * @param \question_definition the question. + * @param \question_definition $question the question. * @return bool is this the latest ready version of this question? */ public static function is_latest_version(\question_definition $question): bool { @@ -423,7 +423,7 @@ public static function get_installed_language_choices(): ?array { } /** @var int Use to create unique iframe names. */ - protected static $untitilediframecounter = 0; + protected static $untitilediframecounter = 0; /** * Make a unique name, for anonymous iframes. @@ -460,7 +460,7 @@ public static function get_question_bank_url(\question_definition $question): \m FROM {question_versions} v WHERE v.questionbankentryid = ? ) - ", [$question->questionbankentryid, $question->questionbankentryid,]); + ", [$question->questionbankentryid, $question->questionbankentryid]); return new \moodle_url('/question/edit.php', [ 'courseid' => $context->instanceid, diff --git a/lib.php b/lib.php index 8ade1e1..bd5a83a 100644 --- a/lib.php +++ b/lib.php @@ -66,7 +66,8 @@ function filter_embedquestion_question_pluginfile($givencourse, $context, $compo $fs = get_file_storage(); $relativepath = implode('/', $args); $fullpath = "/{$context->id}/{$component}/{$filearea}/{$relativepath}"; - if (!$file = $fs->get_file_by_hash(sha1($fullpath)) or $file->is_directory()) { + $file = $fs->get_file_by_hash(sha1($fullpath)); + if (!$file || $file->is_directory()) { send_file_not_found(); } diff --git a/tests/attempt_test.php b/tests/attempt_test.php index e006581..de2fe8c 100644 --- a/tests/attempt_test.php +++ b/tests/attempt_test.php @@ -20,9 +20,11 @@ /** * Unit tests for the code for attempting questions. * - * @package filter_embedquestion - * @copyright 2019 The Open University - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @package filter_embedquestion + * @copyright 2019 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \filter_embedquestion\attempt + * @covers \filter_embedquestion\attempt_storage */ class attempt_test extends \advanced_testcase { @@ -274,6 +276,7 @@ public function test_question_rendering(): void { '
Not complete
Marked out of 1.00
' . '
' . ']*>Edit question
' . + '(v1 \(latest\))?' . '
' . '' . '' . diff --git a/tests/behat/behat_filter_embedquestion.php b/tests/behat/behat_filter_embedquestion.php index e443907..1b608cf 100644 --- a/tests/behat/behat_filter_embedquestion.php +++ b/tests/behat/behat_filter_embedquestion.php @@ -14,26 +14,19 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -/** - * Behat steps for filter_embedquestion. - * - * @package filter_embedquestion - * @copyright 2018 The Open University - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - // NOTE: no MOODLE_INTERNAL test because this file is required by Behat. require_once(__DIR__ . '/../../../../lib/behat/behat_base.php'); -use Behat\Mink\Exception\ExpectationException as ExpectationException, - Behat\Gherkin\Node\TableNode as TableNode; +use Behat\Mink\Exception\ExpectationException; +use Behat\Gherkin\Node\TableNode; /** * Behat steps for filter_embedquestion. * + * @package filter_embedquestion * @copyright 2018 The Open University - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class behat_filter_embedquestion extends behat_base { @@ -96,7 +89,7 @@ protected function resolve_page_instance_url(string $type, string $identifier): * * response The response that was submitted. How this is interpreted depends on * the question type. It gets passed to - * {@link core_question_generator::get_simulated_post_data_for_question_attempt()} + * {@see core_question_generator::get_simulated_post_data_for_question_attempt()} * and therefore to the un_summarise_response method of the question to decode. * * Then there should be a number of rows of data, one for each question you want to add. @@ -245,17 +238,4 @@ private function get_attempt_context(string $contextlevel, string $contextref) { return $attemptcontext; } - - /** - * Check that the given Question type already installed. - * - * @Given /^I check the "(?P(?:[^"]|\\")*)" question type already installed for embed question$/ - */ - public function check_question_type_installed($questiontype) { - $qtypes = question_bank::get_creatable_qtypes(); - if (!array_key_exists($questiontype, $qtypes)) { - // Question type not available. - throw new \Moodle\BehatExtension\Exception\SkippedException(); - } - } } diff --git a/tests/cleanup_task_test.php b/tests/cleanup_task_test.php index ce9ad92..110d454 100644 --- a/tests/cleanup_task_test.php +++ b/tests/cleanup_task_test.php @@ -21,9 +21,10 @@ /** * Unit tests for the cleanup scheduled task. * - * @package filter_embedquestion - * @copyright 2018 The Open University - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @package filter_embedquestion + * @copyright 2018 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \filter_embedquestion\task\cleanup_task */ class cleanup_task_test extends \advanced_testcase { diff --git a/tests/external_test.php b/tests/external_test.php index d54bc0b..e5995e5 100644 --- a/tests/external_test.php +++ b/tests/external_test.php @@ -20,8 +20,9 @@ * Unit tests for the external functions. * * @package filter_embedquestion - * @copyright 2018 The Open University - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @copyright 2018 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \filter_embedquestion\external */ class external_test extends \advanced_testcase { @@ -95,7 +96,7 @@ public function test_get_sharable_question_choices_only_user() { } /** - * + * Test cases for {@see test_get_embed_code_working()} and {@see test_is_authorized_secret_token()}. */ public function get_embed_code_cases(): array { return [ diff --git a/tests/filter_test.php b/tests/filter_test.php index 0c9d04d..14439ea 100644 --- a/tests/filter_test.php +++ b/tests/filter_test.php @@ -22,13 +22,14 @@ require_once($CFG->dirroot . '/filter/embedquestion/filter.php'); /** - * Unit tests for \filter_embedquestion. + * Unit tests for filter_embedquestion. * * Test the delimiter parsing used by the embedquestion filter. * - * @package \filter_embedquestion - * @copyright 2018 The Open University - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @package filter_embedquestion + * @copyright 2018 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \filter_embedquestion */ class filter_test extends \advanced_testcase { @@ -38,7 +39,7 @@ public function setUp(): void { } /** - * Data provider for {@link test_filter()}. + * Data provider for {@see test_filter()}. * @return array the test cases. */ public function get_cases_for_test_filter(): array { diff --git a/tests/generator/lib.php b/tests/generator/lib.php index 3fdeffa..60623dc 100644 --- a/tests/generator/lib.php +++ b/tests/generator/lib.php @@ -53,10 +53,10 @@ public function __construct(testing_data_generator $datagenerator) { * Do not specify both isset($overrides['category'] and $categoryrecord. * (Generally, you don't want to specify either.) * - * @param string $qtype as for {@link core_question_generator::create_question()} - * @param string|null $which as for {@link core_question_generator::create_question()} - * @param array|null $overrides as for {@link core_question_generator::create_question()}. - * @param array $categoryrecord as for {@link core_question_generator::create_question_category()}. + * @param string $qtype as for {@see core_question_generator::create_question()} + * @param string|null $which as for {@see core_question_generator::create_question()} + * @param array|null $overrides as for {@see core_question_generator::create_question()}. + * @param array $categoryrecord as for {@see core_question_generator::create_question_category()}. * @return stdClass the data for the newly created question. */ public function create_embeddable_question(string $qtype, string $which = null, diff --git a/tests/token_test.php b/tests/token_test.php index dd2e68e..4c21e9a 100644 --- a/tests/token_test.php +++ b/tests/token_test.php @@ -19,9 +19,10 @@ /** * Unit tests for the util methods. * - * @package filter_embedquestion - * @copyright 2018 The Open University - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @package filter_embedquestion + * @copyright 2018 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \filter_embedquestion\token */ class token_test extends \advanced_testcase { diff --git a/tests/utils_test.php b/tests/utils_test.php index a19d90e..7847241 100644 --- a/tests/utils_test.php +++ b/tests/utils_test.php @@ -24,13 +24,13 @@ use core_question\local\bank\question_version_status; - /** * Unit tests for the util methods. * - * @package filter_embedquestion - * @copyright 2018 The Open University - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @package filter_embedquestion + * @copyright 2018 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \filter_embedquestion\utils */ class utils_test extends \advanced_testcase { @@ -255,7 +255,7 @@ public function test_get_categories_with_sharable_question_choices_should_not_in * * @param string $qtype the question type to create an example of. * @param string|null $which as for the corresponding argument of - * {@link question_test_helper::get_question_form_data}. null for the default one. + * {@see question_test_helper::get_question_form_data}. null for the default one. * @param array|null $overrides any fields that should be different from the base example. * @return \stdClass the question data. */