From fa7431ce502193e9406ba3a244cb30e87b256261 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Wed, 25 Mar 2020 17:16:44 +0000 Subject: [PATCH] MDL-68252 question tags: fix tag editing for missingtype questions --- lib/behat/classes/behat_core_generator.php | 22 +++++++++- lib/questionlib.php | 7 ++- question/lib.php | 20 ++++++--- question/tests/behat/delete_questions.feature | 11 +++++ .../tests/behat/edit_question_tags.feature | 44 +++++++++++++++++++ question/tests/behat/edit_questions.feature | 12 +++++ 6 files changed, 106 insertions(+), 10 deletions(-) create mode 100644 question/tests/behat/edit_question_tags.feature diff --git a/lib/behat/classes/behat_core_generator.php b/lib/behat/classes/behat_core_generator.php index 253623cf8a4..08fb6635bf0 100644 --- a/lib/behat/classes/behat_core_generator.php +++ b/lib/behat/classes/behat_core_generator.php @@ -634,9 +634,18 @@ class behat_core_generator extends behat_generator_base { * We start with test_question_maker::get_question_form_data($data['qtype'], $data['template']) * and then overlay the values from any other fields of $data that are set. * + * There is a special case that allows you to set qtype to 'missingtype'. + * This creates an example of broken question, such as you might get if you + * install a question type, create some questions of that type, and then + * uninstall the question type (which is prevented through the UI but can + * still happen). This special lets tests verify that these questions are + * handled OK. + * * @param array $data the row of data from the behat script. */ protected function process_question($data) { + global $DB; + if (array_key_exists('questiontext', $data)) { $data['questiontext'] = array( 'text' => $data['questiontext'], @@ -656,7 +665,18 @@ class behat_core_generator extends behat_generator_base { $which = $data['template']; } - $this->datagenerator->get_plugin_generator('core_question')->create_question($data['qtype'], $which, $data); + $missingtypespecialcase = false; + if ($data['qtype'] === 'missingtype') { + $data['qtype'] = 'essay'; // Actual type uses here does not matter. We just need any question. + $missingtypespecialcase = true; + } + + $questiondata = $this->datagenerator->get_plugin_generator('core_question') + ->create_question($data['qtype'], $which, $data); + + if ($missingtypespecialcase) { + $DB->set_field('question', 'qtype', 'unknownqtype', ['id' => $questiondata->id]); + } } /** diff --git a/lib/questionlib.php b/lib/questionlib.php index 8b95027026d..b85cc151aaf 100644 --- a/lib/questionlib.php +++ b/lib/questionlib.php @@ -1721,8 +1721,11 @@ function question_has_capability_on($questionorid, $cap, $notused = -1) { try { $question = question_bank::load_question_data($questionid); } catch (Exception $e) { - // Let's log the exception for future debugging. - debugging($e->getMessage(), DEBUG_NORMAL, $e->getTrace()); + // Let's log the exception for future debugging, + // but not during Behat, or we can't test these cases. + if (!defined('BEHAT_SITE_RUNNING')) { + debugging($e->getMessage(), DEBUG_NORMAL, $e->getTrace()); + } // Well, at least we tried. Seems that we really have to read from DB. $question = $DB->get_record_sql('SELECT q.id, q.createdby, qc.contextid diff --git a/question/lib.php b/question/lib.php index 209d06b733d..56fda425f95 100644 --- a/question/lib.php +++ b/question/lib.php @@ -43,6 +43,7 @@ function core_question_output_fragment_tags_form($args) { $id = clean_param($args['id'], PARAM_INT); $editingcontext = $args['context']; + // Load the question and some related information. $question = $DB->get_record('question', ['id' => $id]); if ($coursecontext = $editingcontext->get_course_context(false)) { @@ -52,14 +53,19 @@ function core_question_output_fragment_tags_form($args) { $filtercourses = null; } - // Load the question tags and filter the course tags by the current - // course. - get_question_options($question, true, $filtercourses); - - $category = $question->categoryobject; + $category = $DB->get_record('question_categories', ['id' => $question->category]); $questioncontext = \context::instance_by_id($category->contextid); $contexts = new \question_edit_contexts($editingcontext); + // Load the question tags and filter the course tags by the current course. + if (core_tag_tag::is_enabled('core_question', 'question')) { + $tagobjectsbyquestion = core_tag_tag::get_items_tags('core_question', 'question', [$question->id]); + if (!empty($tagobjectsbyquestion[$question->id])) { + $tagobjects = $tagobjectsbyquestion[$question->id]; + $sortedtagobjects = question_sort_tags($tagobjects, + context::instance_by_id($category->contextid), $filtercourses); + } + } $formoptions = [ 'editingcontext' => $editingcontext, 'questioncontext' => $questioncontext, @@ -72,8 +78,8 @@ function core_question_output_fragment_tags_form($args) { 'categoryid' => $category->id, 'contextid' => $category->contextid, 'context' => $questioncontext->get_context_name(), - 'tags' => isset($question->tags) ? $question->tags : [], - 'coursetags' => isset($question->coursetags) ? $question->coursetags : [], + 'tags' => $sortedtagobjects->tags ?? [], + 'coursetags' => $sortedtagobjects->coursetags ?? [], ]; $cantag = question_has_capability_on($question, 'tag'); diff --git a/question/tests/behat/delete_questions.feature b/question/tests/behat/delete_questions.feature index 44354127ebf..26d340c056d 100644 --- a/question/tests/behat/delete_questions.feature +++ b/question/tests/behat/delete_questions.feature @@ -56,3 +56,14 @@ Feature: A teacher can delete questions in the question bank And I follow "Test quiz" And I click on "Preview quiz now" "button" And I should see "Write about whatever you want" + + @javascript + Scenario: A question can be deleted even if that question type is no longer installed + Given the following "questions" exist: + | questioncategory | qtype | name | questiontext | + | Test questions | missingtype | Broken question | Write something | + And I reload the page + When I choose "Delete" action for "Broken question" in the question bank + And I press "Delete" + And I click on "Also show old questions" "checkbox" + Then I should not see "Broken question" diff --git a/question/tests/behat/edit_question_tags.feature b/question/tests/behat/edit_question_tags.feature new file mode 100644 index 00000000000..6f7007deda0 --- /dev/null +++ b/question/tests/behat/edit_question_tags.feature @@ -0,0 +1,44 @@ +@core @core_question +Feature: A teacher can manage tags on questions in the question bank + In order to organise my questions + As a teacher + I need to be able to tag them + + Background: + Given the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | Teacher | 1 | teacher1@example.com | + And the following "courses" exist: + | fullname | shortname | format | + | Course 1 | C1 | weeks | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + And the following "question categories" exist: + | contextlevel | reference | name | + | Course | C1 | Test questions | + And the following "questions" exist: + | questioncategory | qtype | name | questiontext | + | Test questions | essay | Test question to be tagged | Write about whatever you want | + And I log in as "teacher1" + And I am on "Course 1" course homepage + And I navigate to "Question bank > Questions" in current page administration + + @javascript + Scenario: Manage tags on a question + When I choose "Manage tags" action for "Test question to be tagged" in the question bank + And I should see "Test question to be tagged" in the "Question tags" "dialogue" + And I set the field "Tags" to "my-tag" + And I click on "Save changes" "button" in the "Question tags" "dialogue" + Then I should see "my-tag" in the "Test question to be tagged" "table_row" + + @javascript + Scenario: Manage tags works even on questions of a type is no longer installed + Given the following "questions" exist: + | questioncategory | qtype | name | questiontext | + | Test questions | missingtype | Broken question | Write something | + And I reload the page + When I choose "Manage tags" action for "Broken question" in the question bank + And I set the field "Tags" to "my-tag" + And I click on "Save changes" "button" in the "Question tags" "dialogue" + Then I should see "my-tag" in the "Broken question" "table_row" diff --git a/question/tests/behat/edit_questions.feature b/question/tests/behat/edit_questions.feature index 58d020016e4..8fdb5ca08a7 100644 --- a/question/tests/behat/edit_questions.feature +++ b/question/tests/behat/edit_questions.feature @@ -53,3 +53,15 @@ Feature: A teacher can edit questions in the question bank And I set the field "ID number" to "" And I press "id_submitbutton" Then I should not see "frog" in the "Question with idnumber" "table_row" + + Scenario: If the question type is no longer installed, then most edit actions are not present + Given the following "questions" exist: + | questioncategory | qtype | name | questiontext | + | Test questions | missingtype | Broken question | Write something | + When I reload the page + Then "Edit question" "link" should not exist in the "Broken question" "table_row" + And "Duplicate" "link" should not exist in the "Broken question" "table_row" + And "Manage tags" "link" should exist in the "Broken question" "table_row" + And "Preview" "link" should not exist in the "Broken question" "table_row" + And "Delete" "link" should exist in the "Broken question" "table_row" + And "Export as Moodle XML" "link" should not exist in the "Broken question" "table_row"