From 7daa40f46899c301a15ebcd0badad7bef4e892e3 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Mon, 2 Oct 2023 10:47:22 +0100 Subject: [PATCH] MDL-79466 qbank_history: Filters not applied correctly in History view Filters were not being passed from the question bank when viewing a question's history, mearning that questions not visible with the default filters applied would not have any version shown on this history view unless the required filters were re-applied manually. This change ensures the filters are passed in the URL to history.php. It also removes the Filter UI from the History view, since it doesn't make sense to change the filters that are passed. --- question/bank/history/classes/helper.php | 6 +++- .../bank/history/classes/history_action.php | 8 +++-- .../history/classes/question_history_view.php | 11 +++++++ .../behat/question_history_action.feature | 18 +++++++++-- question/bank/history/tests/helper_test.php | 32 +++++++++++++++++-- 5 files changed, 68 insertions(+), 7 deletions(-) diff --git a/question/bank/history/classes/helper.php b/question/bank/history/classes/helper.php index b9ee5f54ede..58dd7563bb0 100644 --- a/question/bank/history/classes/helper.php +++ b/question/bank/history/classes/helper.php @@ -32,14 +32,18 @@ class helper { * @param int $entryid id of the question entry * @param string $returnrul url of the page to return to * @param int $courseid id of the course + * @param ?string $filter filter param to pass to the History view * @return \moodle_url */ - public static function question_history_url(int $entryid, string $returnrul, int $courseid): \moodle_url { + public static function question_history_url(int $entryid, string $returnrul, int $courseid, ?string $filter): \moodle_url { $params = [ 'entryid' => $entryid, 'returnurl' => $returnrul, 'courseid' => $courseid ]; + if (!is_null($filter)) { + $params['filter'] = $filter; + } return new \moodle_url('/question/bank/history/history.php', $params); } diff --git a/question/bank/history/classes/history_action.php b/question/bank/history/classes/history_action.php index 100f1835f2c..057aa3ecdd5 100644 --- a/question/bank/history/classes/history_action.php +++ b/question/bank/history/classes/history_action.php @@ -45,8 +45,12 @@ class history_action extends question_action_base { } if (question_has_capability_on($question, 'use')) { - $url = helper::question_history_url($question->questionbankentryid, $this->qbank->returnurl, - $this->qbank->course->id); + $url = helper::question_history_url( + $question->questionbankentryid, + $this->qbank->returnurl, + $this->qbank->course->id, + $this->qbank->base_url()->param('filter'), + ); return [$url, 't/log', $this->strpreview]; } diff --git a/question/bank/history/classes/question_history_view.php b/question/bank/history/classes/question_history_view.php index 33a91834fef..8da85e895a6 100644 --- a/question/bank/history/classes/question_history_view.php +++ b/question/bank/history/classes/question_history_view.php @@ -177,4 +177,15 @@ class question_history_view extends view { return true; } + /** + * Override wanted_filters so that we apply the filters provided by the URL, but don't display the filter UI. + * + * @return void + */ + public function wanted_filters(): void { + $this->display_question_bank_header(); + // Add search conditions. + $this->add_standard_search_conditions(); + } + } diff --git a/question/bank/history/tests/behat/question_history_action.feature b/question/bank/history/tests/behat/question_history_action.feature index cae416064ed..ac1ca5877f0 100644 --- a/question/bank/history/tests/behat/question_history_action.feature +++ b/question/bank/history/tests/behat/question_history_action.feature @@ -38,8 +38,22 @@ Feature: Use the qbank plugin manager page for question history And I should see "Created by" And I should see "First question" And the "History" action should not exist for the "First question" question in the question bank - And I click on "#qbank-history-close" "css_element" - And the "History" action should exist for the "First question" question in the question bank + + @javascript + Scenario: Viewing history for a question in a non-default category + Given the following "question categories" exist: + | contextlevel | reference | name | + | Course | C1 | Test questions 2 | + And the following "questions" exist: + | questioncategory | qtype | name | questiontext | + | Test questions 2 | truefalse | Second question | Answer the second question | + And I am on the "Test quiz" "mod_quiz > question bank" page logged in as "admin" + And I apply question bank filter "Category" with value "Test questions 2" + And I choose "History" action for "Second question" in the question bank + Then I should see "Question history" + And "Filter 1" "fieldset" should not exist + And I should see "Second question" + And "Second question" "table_row" should exist @javascript Scenario: Delete question from the history using Edit question menu diff --git a/question/bank/history/tests/helper_test.php b/question/bank/history/tests/helper_test.php index cdc37dd823f..d68d65cae32 100644 --- a/question/bank/history/tests/helper_test.php +++ b/question/bank/history/tests/helper_test.php @@ -77,14 +77,42 @@ class helper_test extends \advanced_testcase { */ public function test_question_history_url() { $this->resetAfterTest(); - $actionurl = helper::question_history_url($this->questiondata->questionbankentryid, $this->returnurl, $this->courseid); + $filter = urlencode('filters[]'); + $actionurl = helper::question_history_url( + $this->questiondata->questionbankentryid, + $this->returnurl, + $this->courseid, + $filter, + ); $params = [ 'entryid' => $this->questiondata->questionbankentryid, 'returnurl' => $this->returnurl, - 'courseid' => $this->courseid + 'courseid' => $this->courseid, + 'filter' => $filter, ]; $expectedurl = new \moodle_url('/question/bank/history/history.php', $params); $this->assertEquals($expectedurl, $actionurl); } + /** + * Test the history action url when the filter parameter is null. + * + * @covers ::question_history_url + */ + public function test_question_history_url_null_filter() { + $this->resetAfterTest(); + $actionurl = helper::question_history_url( + $this->questiondata->questionbankentryid, + $this->returnurl, + $this->courseid, + null, + ); + $params = [ + 'entryid' => $this->questiondata->questionbankentryid, + 'returnurl' => $this->returnurl, + 'courseid' => $this->courseid, + ]; + $expectedurl = new \moodle_url('/question/bank/history/history.php', $params); + $this->assertEquals($expectedurl, $actionurl); + } }