mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-02-22 19:06:51 +01:00
Code Issues Projects Releases Wiki Activity

MDL-79265 question bank: fix which questions are listed with hidden

Browse Source 
This fixes the bug where if the latest version of the question is hidden,
the question disappears, even if older versions exist that are still visible.
This commit is contained in:
Hieu Nguyen Dang 2024-07-03 15:05:31 +07:00 committed by hieuvu
parent 89e6f737cc
commit 47ce8f57b7
7 changed files with 235 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
mod/quiz/classes/question/bank/custom_view.php
View File

@ -245,9 +245,16 @@ class custom_view extends \core_question\local\bank\view {
FROM {question_versions} v
JOIN {question_bank_entries} be
ON be.id = v.questionbankentryid
WHERE be.id = qbe.id)';
$onlyready = '((' . "qv.status = '" . question_version_status::QUESTION_STATUS_READY . "'" .'))';
$this->sqlparams = [];
WHERE be.id = qbe.id AND v.status <> :substatus)';
// An additional condition is required in the subquery to account for scenarios
// where the latest version is hidden. This ensures we retrieve the previous
// "Ready" version instead of the hidden latest version.
$onlyready = '((qv.status = :status))';
$this->sqlparams = [
'status' => question_version_status::QUESTION_STATUS_READY,
'substatus' => question_version_status::QUESTION_STATUS_HIDDEN,
];
$conditions = [];
foreach ($this->searchconditions as $searchcondition) {
if ($searchcondition->where()) {

11
mod/quiz/classes/question/bank/filter/custom_category_condition_helper.php
View File

@ -102,8 +102,11 @@ class custom_category_condition_helper extends \qbank_managecategories\helper {
bool $top = false, int $showallversions = 0): array {
global $DB;
$topwhere = $top ? '' : 'AND c.parent <> 0';
$statuscondition = "AND qv.status = '". question_version_status::QUESTION_STATUS_READY . "' ";
$statuscondition = "AND qv.status = :status";
$params = [
'status' => question_version_status::QUESTION_STATUS_READY,
'substatus' => question_version_status::QUESTION_STATUS_HIDDEN,
];
$sql = "SELECT c.*,
(SELECT COUNT(1)
FROM {question} q
@ -116,7 +119,7 @@ class custom_category_condition_helper extends \qbank_managecategories\helper {
OR (qv.version = (SELECT MAX(v.version)
FROM {question_versions} v
JOIN {question_bank_entries} be ON be.id = v.questionbankentryid
WHERE be.id = qbe.id)
WHERE be.id = qbe.id AND v.status <> :substatus)
)
)
) AS questioncount
@ -124,6 +127,6 @@ class custom_category_condition_helper extends \qbank_managecategories\helper {
WHERE c.contextid IN ($contexts) $topwhere
ORDER BY $sortorder";
return $DB->get_records_sql($sql);
return $DB->get_records_sql($sql, $params);
}
}

55
mod/quiz/tests/quiz_question_bank_view_test.php
View File

@ -31,7 +31,7 @@ require_once($CFG->dirroot . '/question/editlib.php');
* @category test
* @copyright 2018 the Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
* @covers \core_question\local\bank\view
* @covers \mod_quiz\question\bank\custom_view
*/
final class quiz_question_bank_view_test extends \advanced_testcase {
@ -82,6 +82,59 @@ final class quiz_question_bank_view_test extends \advanced_testcase {
$this->assertFalse($cache->has($questiondata->id));
}
public function test_viewing_question_bank_should_not_load_hidden_question(): void {
$this->resetAfterTest();
$this->setAdminUser();
$generator = $this->getDataGenerator();
/** @var core_question_generator $questiongenerator */
$questiongenerator = $generator->get_plugin_generator('core_question');
// Create a course and a quiz.
$course = $generator->create_course();
$quiz = $this->getDataGenerator()->create_module('quiz', ['course' => $course->id]);
$context = \context_module::instance($quiz->cmid);
$cm = get_coursemodule_from_instance('quiz', $quiz->id);
// Create a question in the default category.
$contexts = new question_edit_contexts($context);
question_make_default_categories($contexts->all());
$cat = question_get_default_category($context->id);
$question = $questiongenerator->create_question('numerical', null,
['name' => 'Example question', 'category' => $cat->id]);
// Create another version.
$newversion = $questiongenerator->update_question($question, null, ['name' => 'This is the latest version']);
// Add them to the quiz.
quiz_add_quiz_question($newversion->id, $quiz);
// Generate the view.
$params = [
'qpage' => 0,
'qperpage' => 20,
'cat' => $cat->id . ',' . $context->id,
'recurse' => false,
'showhidden' => false,
'qbshowtext' => false,
'tabname' => 'editq',
];
$extraparams = ['cmid' => $cm->id];
$view = new custom_view($contexts, new \moodle_url('/'), $course, $cm, $params, $extraparams);
ob_start();
$view->display();
$html = ob_get_clean();
// Verify the output should included the latest version.
$this->assertStringContainsString('This is the latest version', $html);
$this->assertStringNotContainsString('Example question', $html);
// Delete the latest version.
question_delete_question($newversion->id);
// Verify the output should display the old version with status ready.
ob_start();
$view->display();
$html = ob_get_clean();
$this->assertStringContainsString('Example question', $html);
$this->assertStringNotContainsString('This is the latest version', $html);
}
public function test_viewing_question_bank_when_paging_out_of_limit(): void {
$this->resetAfterTest();
$this->setAdminUser();

12
question/bank/managecategories/classes/helper.php
View File

@ -308,9 +308,11 @@ class helper {
): array {
global $DB;
$topwhere = $top ? '' : 'AND c.parent <> 0';
$statuscondition = "AND (qv.status = '" . question_version_status::QUESTION_STATUS_READY . "' " .
" OR qv.status = '" . question_version_status::QUESTION_STATUS_DRAFT . "' )";
$statuscondition = "AND qv.status <> :status";
$params = [
'status' => question_version_status::QUESTION_STATUS_HIDDEN,
'substatus' => question_version_status::QUESTION_STATUS_HIDDEN,
];
$sql = "SELECT c.*,
(SELECT COUNT(1)
FROM {question} q
@ -323,7 +325,7 @@ class helper {
OR (qv.version = (SELECT MAX(v.version)
FROM {question_versions} v
JOIN {question_bank_entries} be ON be.id = v.questionbankentryid
WHERE be.id = qbe.id)
WHERE be.id = qbe.id AND v.status <> :substatus)
)
)
) AS questioncount
@ -331,7 +333,7 @@ class helper {
WHERE c.contextid IN ($contexts) $topwhere
ORDER BY $sortorder";
return $DB->get_records_sql($sql);
return $DB->get_records_sql($sql, $params);
}
/**

30
question/bank/managecategories/tests/helper_test.php
View File

@ -295,4 +295,34 @@ final class helper_test extends manage_category_test_base {
$this->assertCount($count + 1, $categorycontext);
}
}
/**
* Test that get_categories_for_contexts function returns the correct question count number.
*
* @covers ::get_categories_for_contexts
*/
public function test_question_category_question_count(): void {
global $DB;
// Create quiz.
$quiz = $this->quiz;
// Create category 1 and one hidden question.
$qcat = $this->qgenerator->create_question_category(['contextid' => $this->context->id]);
$q1 = $this->qgenerator->create_question('shortanswer', null, ['category' => $qcat->id]);
$DB->set_field('question_versions', 'status', 'hidden', ['questionid' => $q1->id]);
$contexts = new \core_question\local\bank\question_edit_contexts(\context_module::instance($quiz->cmid));
$contexts = $contexts->having_cap('moodle/question:add');
foreach ($contexts as $context) {
$contextslist[] = $context->id;
}
$contextslist = join(', ', $contextslist);
// Verify we have 0 question in category since it is hidden.
$categorycontexts = helper::get_categories_for_contexts($contextslist);
$this->assertEquals(0, reset($categorycontexts)->questioncount);
// Add an extra question.
$this->qgenerator->create_question('shortanswer', null, ['category' => $qcat->id]);
$categorycontexts = helper::get_categories_for_contexts($contextslist);
// Verify we have 1 question in category.
$this->assertEquals(1, reset($categorycontexts)->questioncount);
}
}

26
question/classes/local/bank/view.php
View File

@ -745,16 +745,16 @@ class view {
[$colname, $subsort] = $this->parse_subsort($sortname);
$sorts[] = $this->requiredcolumns[$colname]->sort_expression($sortorder == SORT_DESC, $subsort);
}
// Build the where clause.
$latestversion = 'qv.version = (SELECT MAX(v.version)
FROM {question_versions} v
JOIN {question_bank_entries} be
ON be.id = v.questionbankentryid
WHERE be.id = qbe.id)';
$this->sqlparams = [];
$conditions = [];
$showhiddenquestion = true;
// Build the where clause.
foreach ($this->searchconditions as $searchcondition) {
// TODO: Looking at the contents of params like this is not great. It is just a short-term fix.
// This will be solved properly when MDL-84433 is done.
if (array_key_exists('hidden_condition', $searchcondition->params())) {
$showhiddenquestion = false;
}
if ($searchcondition->where()) {
$conditions[] = '((' . $searchcondition->where() .'))';
}
@ -762,6 +762,18 @@ class view {
$this->sqlparams = array_merge($this->sqlparams, $searchcondition->params());
}
}
$extracondition = '';
if (!$showhiddenquestion) {
// If Show hidden question option is off, then we need get the latest version that is not hidden.
$extracondition = ' AND v.status <> :hiddenstatus';
$this->sqlparams = array_merge($this->sqlparams, ['hiddenstatus' => question_version_status::QUESTION_STATUS_HIDDEN]);
}
$latestversion = "qv.version = (SELECT MAX(v.version)
FROM {question_versions} v
JOIN {question_bank_entries} be
ON be.id = v.questionbankentryid
WHERE be.id = qbe.id $extracondition)";
// Get higher level filter condition.
$jointype = isset($this->pagevars['jointype']) ? (int)$this->pagevars['jointype'] : condition::JOINTYPE_DEFAULT;
$nonecondition = ($jointype === datafilter::JOINTYPE_NONE) ? ' NOT ' : '';

108
question/tests/question_bank_view_test.php Normal file
View File

@ -0,0 +1,108 @@
<?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 core_question;
use core_question\local\bank\question_edit_contexts;
defined('MOODLE_INTERNAL') || die();
global $CFG;
require_once($CFG->dirroot . '/question/editlib.php');
/**
* Unit tests for the question own question bank view class.
*
* @package core_question
* @copyright 2024 the Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
* @covers \core_question\local\bank\view
*/
final class question_bank_view_test extends \advanced_testcase {
public function test_viewing_question_bank_should_not_load_hidden_question(): void {
$this->resetAfterTest();
$this->setAdminUser();
$generator = $this->getDataGenerator();
/** @var core_question_generator $questiongenerator */
$questiongenerator = $generator->get_plugin_generator('core_question');
// Create a course and a quiz.
$course = $generator->create_course();
$quiz = $this->getDataGenerator()->create_module('quiz', ['course' => $course->id]);
$context = \context_module::instance($quiz->cmid);
$cm = get_coursemodule_from_instance('quiz', $quiz->id);
// Create a question in the default category.
$contexts = new question_edit_contexts($context);
question_make_default_categories($contexts->all());
$cat = question_get_default_category($context->id);
$question = $questiongenerator->create_question('numerical', null,
['name' => 'Example question', 'category' => $cat->id]);
// Create another version.
$newversion = $questiongenerator->update_question($question, null, ['name' => 'This is the latest version']);
// Add them to the quiz.
quiz_add_quiz_question($newversion->id, $quiz);
// Generate the view.
$params = [
'qpage' => 0,
'qperpage' => 20,
'cat' => $cat->id . ',' . $context->id,
'recurse' => false,
'qbshowtext' => false,
'tabname' => 'editq',
];
$extraparams = ['cmid' => $cm->id];
$view = new \core_question\local\bank\view($contexts, new \moodle_url('/'), $course, $cm, $params, $extraparams);
ob_start();
$view->display();
$html = ob_get_clean();
// Verify the output should included the latest version.
$this->assertStringContainsString('This is the latest version', $html);
$this->assertStringNotContainsString('Example question', $html);
// Delete the latest version.
question_delete_question($newversion->id);
// Verify the output should display the old version with status ready.
ob_start();
$view->display();
$html = ob_get_clean();
$this->assertStringContainsString('Example question', $html);
$this->assertStringNotContainsString('This is the latest version', $html);
// Use show hidden question filter.
$params['filter'] = [
'category' => [
'name' => 'category',
'jointype' => 1,
'values' => [$cat->id],
'filteroptions' => [],
],
'hidden' => [
'name' => 'hidden',
'jointype' => 1,
'values' => [1],
'filteroptions' => [],
],
];
$view = new \core_question\local\bank\view($contexts, new \moodle_url('/'), $course, $cm, $params, $extraparams);
ob_start();
$view->display();
$html = ob_get_clean();
$this->assertStringContainsString('This is the latest version', $html);
$this->assertStringNotContainsString('Example question', $html);
}
}