MDL-76757 Question bank: Can not delete the question completely

This commit is contained in:
danghieu1407 2023-07-10 14:30:01 +07:00
parent ef93325f27
commit ab4cb28883
10 changed files with 397 additions and 32 deletions

View File

@ -100,8 +100,13 @@ $string['defaultfor'] = 'Default for {$a}';
$string['defaultinfofor'] = 'The default category for questions shared in context \'{$a}\'.';
$string['defaultmarkmustbepositive'] = 'The default mark must be positive.';
$string['deletecoursecategorywithquestions'] = 'There are questions in the question bank associated with this course category. If you proceed, they will be deleted. You may wish to move them first, using the question bank interface.';
$string['deletequestioncheck'] = 'Are you absolutely sure you want to delete \'{$a}\'?';
$string['deletequestionscheck'] = 'Are you absolutely sure you want to delete the following questions?<br /><br />{$a}';
$string['deletequestioncheck'] = 'This will delete the following question and all its versions:<br /><br />{$a}';
$string['deletequestionscheck'] = 'This will delete the following questions and all their versions:<br /><br />{$a}';
$string['deleteselectedquestioncheck'] = 'This will delete selected versions of the following question:<br /><br />{$a}';
$string['deletequestiontitle'] = 'Delete question?';
$string['deletequestiontitle_plural'] = 'Delete questions?';
$string['deleteversiontitle'] = 'Delete selected version?';
$string['deleteversiontitle_plural'] = 'Delete selected versions?';
$string['deletingbehaviour'] = 'Deleting question behaviour \'{$a}\'';
$string['deletingqtype'] = 'Deleting question type \'{$a}\'';
$string['didnotmatchanyanswer'] = '[Did not match any answer]';
@ -288,9 +293,10 @@ $string['questioncategory'] = 'Question category';
$string['questioncatsfor'] = 'Question categories for \'{$a}\'';
$string['questiondoesnotexist'] = 'This question does not exist';
$string['questionname'] = 'Question name';
$string['questionnameandquestionversion'] = '{$a->name} v{$a->version}';
$string['questionno'] = 'Question {$a}';
$string['questionsaveerror'] = 'Errors occur during saving question - ({$a})';
$string['questionsinuse'] = '(* Questions marked with an asterisk are used somewhere, for example in a quiz. Therefore, if you proceed, these questions will not really be deleted, they will just be hidden.)';
$string['questionsinuse'] = '* Denotes questions which can\'t be deleted because they are in use. Instead, they will be hidden in the question bank unless you select \'Show old questions\'.';
$string['questionsmovedto'] = 'Questions still in use moved to "{$a}" in the parent course category.';
$string['questionsrescuedfrom'] = 'Questions saved from context {$a}.';
$string['questionsrescuedfrominfo'] = 'These questions (some of which may be hidden) were saved when context {$a} was deleted because they are still used by some quizzes or other activities.';

View File

@ -96,6 +96,9 @@ class delete_action_column extends menu_action_column_base {
'q' . $question->id => 1,
'sesskey' => sesskey());
$deleteparams = array_merge($deleteparams, $this->returnparams);
if ($this->qbank->base_url()->get_param('deleteall')) {
$deleteparams['deleteall'] = 1;
}
$url = new \moodle_url($this->deletequestionurl, $deleteparams);
return [$url, 't/delete', $this->strdelete];
}

View File

@ -0,0 +1,117 @@
<?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 qbank_deletequestion;
/**
* Class helper of qbank_deletequestion.
*
* @package qbank_deletequestion
* @copyright 2023 The Open University
* @since Moodle 4.2
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class helper {
/**
* Get the confirmation message of delete question.
*
* @param array $questionids List of id questions.
* @param bool $deleteallversions Delete all question version or not.
* @return array List confirmation message.
*/
public static function get_delete_confirmation_message(array $questionids, bool $deleteallversions): array {
global $DB;
$questionnames = '';
$inuse = false;
$hasmutipleversions = false;
$questionversions = [];
$countselectedquestion = count($questionids);
if ($deleteallversions) {
$listofquestions = \question_bank::get_all_versions_of_questions($questionids);
foreach ($listofquestions as $questionbankentry) {
if (count($questionbankentry) > 1 && !$hasmutipleversions) {
$hasmutipleversions = true;
}
// Flip the array to list question by question id. [ qid => qversion ].
$questionversions += array_flip($questionbankentry);
}
// Flatten an array.
$questionids = array_merge(...$listofquestions);
}
[$questionsql, $params] = $DB->get_in_or_equal($questionids, SQL_PARAMS_NAMED);
$questions = $DB->get_records_select('question', 'id ' . $questionsql, $params,
'name ASC', 'id, name');
foreach ($questions as $question) {
if (questions_in_use([$question->id])) {
$questionnames .= '* ';
$inuse = true;
}
$questionname = format_string($question->name);
if (isset($questionversions[$question->id])) {
$a = new \stdClass();
$a->name = $questionname;
$a->version = $questionversions[$question->id];
$questionnames .= get_string('questionnameandquestionversion',
'question', $a) . '<br />';
} else {
$questionnames .= $questionname . '<br />';
}
}
if ($inuse) {
$questionnames .= '<br />'.get_string('questionsinuse', 'question');
}
$confirmtitle = [
'confirmtitle' => $countselectedquestion > 1 ? get_string('deleteversiontitle_plural',
'question') : get_string('deleteversiontitle', 'question'),
];
$message = get_string('deleteselectedquestioncheck', 'question', $questionnames);
if ($deleteallversions) {
$confirmtitle = [
'confirmtitle' => get_string('deletequestiontitle', 'question'),
];
$message = get_string('deletequestioncheck', 'question', $questionnames);
if ($countselectedquestion > 1) {
$confirmtitle = [
'confirmtitle' => get_string('deletequestiontitle_plural', 'question'),
];
$message = get_string('deletequestionscheck', 'question', $questionnames);
}
}
return [$confirmtitle, $message];
}
/**
* Delete questions has (single/multiple) version.
*
* @param array $questionids List of questionid.
* @param bool $deleteallversions Delete all question version or not.
*/
public static function delete_questions(array $questionids, bool $deleteallversions): void {
if ($deleteallversions) {
// Get all the question id from multidimensional array.
$listofquestions = \question_bank::get_all_versions_of_questions($questionids);
// Flatten an array.
$questionids = array_merge(...$listofquestions);
}
foreach ($questionids as $questionid) {
$questionid = (int) $questionid;
question_require_capability_on($questionid, 'edit');
question_delete_question($questionid);
}
}
}

View File

@ -33,6 +33,7 @@ $deleteselected = optional_param('deleteselected', false, PARAM_BOOL);
$returnurl = optional_param('returnurl', 0, PARAM_LOCALURL);
$cmid = optional_param('cmid', 0, PARAM_INT);
$courseid = optional_param('courseid', 0, PARAM_INT);
$deleteall = optional_param('deleteall', false, PARAM_BOOL);
if ($returnurl) {
$returnurl = new moodle_url($returnurl);
@ -79,12 +80,7 @@ if ($deleteselected && ($confirm = optional_param('confirm', '', PARAM_ALPHANUM)
$deleteselected = required_param('deleteselected', PARAM_RAW);
if ($confirm == md5($deleteselected)) {
if ($questionlist = explode(',', $deleteselected)) {
// For each question either hide it if it is in use or delete it.
foreach ($questionlist as $questionid) {
$questionid = (int)$questionid;
question_require_capability_on($questionid, 'edit');
question_delete_question($questionid);
}
\qbank_deletequestion\helper::delete_questions($questionlist, $deleteall);
}
redirect($returnurl);
} else {
@ -98,18 +94,11 @@ if ($deleteselected) {
// Make a list of all the questions that are selected.
$rawquestions = $_REQUEST; // This code is called by both POST forms and GET links, so cannot use data_submitted.
$questionlist = ''; // Comma separated list of ids of questions to be deleted.
$questionnames = ''; // String with names of questions separated by <br/> with an asterix in front of those that are in use.
$inuse = false; // Set to true if at least one of the questions is in use.
foreach ($rawquestions as $key => $value) { // Parse input for question ids.
if (preg_match('!^q([0-9]+)$!', $key, $matches)) {
$key = $matches[1];
$questionlist .= $key.',';
question_require_capability_on((int)$key, 'edit');
if (questions_in_use(array($key))) {
$questionnames .= '* ';
$inuse = true;
}
$questionnames .= $DB->get_field('question', 'name', array('id' => $key)) . '<br />';
}
}
if (!$questionlist) { // No questions were selected.
@ -117,16 +106,17 @@ if ($deleteselected) {
}
$questionlist = rtrim($questionlist, ',');
// Add an explanation about questions in use.
if ($inuse) {
$questionnames .= '<br />'.get_string('questionsinuse', 'question');
}
$deleteurl = new \moodle_url('/question/bank/deletequestion/delete.php',
array('deleteselected' => $questionlist, 'confirm' => md5($questionlist),
'sesskey' => sesskey(), 'returnurl' => $returnurl, 'cmid' => $cmid, 'courseid' => $courseid));
$deleteurl = new \moodle_url('/question/bank/deletequestion/delete.php', [
'deleteselected' => $questionlist, 'deleteall' => $deleteall, 'confirm' => md5($questionlist),
'sesskey' => sesskey(), 'returnurl' => $returnurl, 'cmid' => $cmid, 'courseid' => $courseid,
]);
$continue = new \single_button($deleteurl, get_string('delete'), 'post');
echo $OUTPUT->confirm(get_string('deletequestionscheck', 'question', $questionnames), $continue, $returnurl);
$questionids = explode(',', $questionlist);
[$displayoptions, $message] = qbank_deletequestion\helper::get_delete_confirmation_message($questionids,
$deleteall);
echo $OUTPUT->confirm($message, $continue, $returnurl, $displayoptions);
}
echo $OUTPUT->footer();

View File

@ -51,7 +51,7 @@ Feature: Use the qbank plugin manager page for deletequestion
And I click on "First question second" "checkbox"
And I click on "With selected" "button"
And I click on question bulk action "deleteselected"
And I click on "Delete" "button" in the "Confirm" "dialogue"
And I click on "Delete" "button" in the "Delete questions?" "dialogue"
Then I should not see "First question"
And I should not see "First question second"
@ -68,6 +68,6 @@ Feature: Use the qbank plugin manager page for deletequestion
And I click on "First question" "checkbox"
And I click on "With selected" "button"
And I click on question bulk action "deleteselected"
When I click on "Delete" "button" in the "Confirm" "dialogue"
When I click on "Delete" "button" in the "Delete question?" "dialogue"
Then I should not see "Third question"
And "foo" "autocomplete_selection" should exist

View File

@ -0,0 +1,177 @@
<?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 qbank_deletequestion;
defined('MOODLE_INTERNAL') || die();
global $CFG;
require_once($CFG->dirroot . '/question/engine/tests/helpers.php');
/**
* Class containing unit tests for the helper class
*
* @package qbank_deletequestion
* @copyright 2023 The Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class helper_test extends \advanced_testcase {
/**
* @var \context_module module context.
*/
protected $context;
/**
* @var \stdClass course object.
*/
protected $course;
/**
* @var \component_generator_base question generator.
*/
protected $qgenerator;
/**
* @var \stdClass quiz object.
*/
protected $quiz;
/**
* Called before every test.
*/
protected function setUp(): void {
parent::setUp();
self::setAdminUser();
$this->resetAfterTest();
$datagenerator = $this->getDataGenerator();
$this->course = $datagenerator->create_course();
$this->quiz = $datagenerator->create_module('quiz', ['course' => $this->course->id]);
$this->qgenerator = $datagenerator->get_plugin_generator('core_question');
$this->context = \context_module::instance($this->quiz->cmid);
}
/**
* Test get a confirmation message when deleting the question in the (question bank/history) page.
*
* @covers \qbank_deletequestion\helper::get_delete_confirmation_message
*/
public function test_get_delete_confirmation_message(): void {
$qcategory = $this->qgenerator->create_question_category(['contextid' => $this->context->id]);
$question = $this->qgenerator->create_question('shortanswer', null, ['category' => $qcategory->id,
'name' => 'Question 1']);
$questionfirstversionid = $question->id;
// Verify confirmation title and confirmation message with question not in use in question bank page.
$deleteallversions = true;
[$title1, $message1] = \qbank_deletequestion\helper::get_delete_confirmation_message([$questionfirstversionid],
$deleteallversions);
$this->assertEquals(['confirmtitle' => get_string('deletequestiontitle', 'question')],
$title1);
$this->assertEquals(get_string('deletequestioncheck', 'question',
$question->name). ' v1' . '<br />', $message1);
// Create a new version and adding it to a quiz.
$question2 = $this->qgenerator->update_question($question, null, ['name' => 'Question 1']);
$questionsecondversionid = $question2->id;
// Verify confirmation title and confirmation message with question has multiple versions in question bank page.
$listnameofquestionversion2 = $question->name . ' v1' . '<br />' . $question2->name . ' v2' .'<br />';
[$title2, $message2] = \qbank_deletequestion\helper::get_delete_confirmation_message([$questionsecondversionid],
$deleteallversions);
$this->assertEquals(['confirmtitle' => get_string('deletequestiontitle', 'question')],
$title2);
$this->assertEquals(get_string('deletequestioncheck', 'question',
$listnameofquestionversion2), $message2);
// Verify confirmation title and confirmation message with multiple question selected.
$listnameofquestionversion3 = $question->name . ' v1' . '<br />' . $question2->name . ' v2' .'<br />';
[$title3, $message3] = \qbank_deletequestion\helper::get_delete_confirmation_message([$questionfirstversionid,
$questionsecondversionid], $deleteallversions);
$this->assertEquals(['confirmtitle' => get_string('deletequestiontitle_plural', 'question')],
$title3);
$this->assertEquals(get_string('deletequestionscheck', 'question',
$listnameofquestionversion3), $message3);
// Add second question version to the quiz to become question in use.
quiz_add_quiz_question($questionsecondversionid, $this->quiz);
// Verify confirmation message with question in use and has multiple versions in question bank page.
$listnameofquestionversion4 = $question->name . ' v1' . '<br />' . '* ' . $question2->name . ' v2' . '<br />';
$message4 = \qbank_deletequestion\helper::get_delete_confirmation_message([$questionsecondversionid],
$deleteallversions)[1];
$this->assertEquals(get_string('deletequestioncheck', 'question',
$listnameofquestionversion4) . '<br />' . get_string('questionsinuse',
'question'), $message4);
// Verify confirmation title and confirmation message in history page with one question selected.
$deleteallversions = false;
[$title5, $message5] = \qbank_deletequestion\helper::get_delete_confirmation_message([$questionfirstversionid],
$deleteallversions);
$this->assertEquals(['confirmtitle' => get_string('deleteversiontitle', 'question')],
$title5);
$this->assertEquals(get_string('deleteselectedquestioncheck', 'question',
$question->name) . '<br />', $message5);
// Verify confirmation title and confirmation message in history page with multiple question selected.
$listnameofquestionversion6 = 'Question 1<br />* Question 1<br />';
[$title6, $message6] = \qbank_deletequestion\helper::get_delete_confirmation_message([$questionfirstversionid,
$questionsecondversionid], $deleteallversions);
$this->assertEquals(['confirmtitle' => get_string('deleteversiontitle_plural', 'question')],
$title6);
$this->assertEquals(get_string('deleteselectedquestioncheck', 'question',
$listnameofquestionversion6) . '<br />'. get_string('questionsinuse', 'question'),
$message6);
}
/**
* Test delete questions have single/multiple version.
*
* @covers \qbank_deletequestion\helper::delete_questions
*/
public function test_delete_question_has_multiple_version() {
global $DB;
$qcategory = $this->qgenerator->create_question_category(['contextid' => $this->context->id]);
$question1 = $this->qgenerator->create_question('shortanswer', null, ['category' => $qcategory->id,
'name' => 'Question 1 version 1']);
$question1v1id = $question1->id;
// Create a new version for question 1.
$question1v2 = $this->qgenerator->update_question($question1, null, ['name' => 'Question 1 version 2']);
$question1v2id = $question1v2->id;
$question2 = $this->qgenerator->create_question('shortanswer', null, ['category' => $qcategory->id,
'name' => 'Question 2 version 1']);
$question2v1id = $question2->id;
$question3 = $this->qgenerator->create_question('shortanswer', null, ['category' => $qcategory->id,
'name' => 'Question 3 version 1']);
$question3v1id = $question3->id;
// Do.
\qbank_deletequestion\helper::delete_questions([$question1v2id, $question2v1id], true);
// All the versions of question1 will be deleted.
$this->assertFalse($DB->record_exists('question', ['id' => $question1v1id]));
$this->assertFalse($DB->record_exists('question', ['id' => $question1v2id]));
// The question2 have single version will be deleted.
$this->assertFalse($DB->record_exists('question', ['id' => $question2v1id]));
// Check that we did not delete too much.
$this->assertTrue($DB->record_exists('question', ['id' => $question3v1id]));
}
}

View File

@ -38,6 +38,7 @@ if ($PAGE->course->id == $SITE->id) {
$PAGE->set_primary_active_tab('home');
}
$thispageurl->param('deleteall', 1);
$questionbank = new core_question\local\bank\view($contexts, $thispageurl, $COURSE, $cm);
$context = $contexts->lowest();

View File

@ -309,6 +309,30 @@ abstract class question_bank {
return $DB->get_records_sql($sql, [$questionid]);
}
/**
* Get all the versions of questions.
*
* @param array $questionids Array of question ids.
* @return array List of versions of questions.
*/
public static function get_all_versions_of_questions(array $questionids): array {
global $DB;
[$listquestionid, $params] = $DB->get_in_or_equal($questionids, SQL_PARAMS_NAMED);
$sql = "SELECT qv.questionid, qv.version, qv.questionbankentryid
FROM {question_versions} qv
JOIN {question_versions} qv2 ON qv.questionbankentryid = qv2.questionbankentryid
WHERE qv2.questionid $listquestionid
ORDER BY qv.questionbankentryid, qv.version DESC";
$result = [];
$rows = $DB->get_recordset_sql($sql, $params);
foreach ($rows as $row) {
$result[$row->questionbankentryid][$row->version] = $row->questionid;
}
return $result;
}
/**
* @return question_finder a question finder.
*/

View File

@ -6,14 +6,14 @@ Feature: A teacher can delete questions in the question bank
Background:
Given the following "users" exist:
| username | firstname | lastname | email |
| teacher1 | Teacher | 1 | teacher1@example.com |
| username | firstname | lastname | email |
| teacher1 | Teacher | 1 | teacher1@example.com |
And the following "courses" exist:
| fullname | shortname | format |
| Course 1 | C1 | weeks |
| Course 1 | C1 | weeks |
And the following "course enrolments" exist:
| user | course | role |
| teacher1 | C1 | editingteacher |
| user | course | role |
| teacher1 | C1 | editingteacher |
And the following "question categories" exist:
| contextlevel | reference | name |
| Course | C1 | Test questions |
@ -51,6 +51,8 @@ Feature: A teacher can delete questions in the question bank
| Test used question to be deleted | 1 | 0 |
When I am on the "Course 1" "core_question > course question bank" page
And I choose "Delete" action for "Test used question to be deleted" in the question bank
And I should see "This will delete the following question and all its versions:"
And I should see "* Denotes questions which can't be deleted because they are in use. Instead, they will be hidden in the question bank unless you select 'Show old questions'."
And I press "Delete"
Then I should not see "Test used question to be deleted"
And I set the field "Also show old questions" to "1"
@ -69,3 +71,16 @@ Feature: A teacher can delete questions in the question bank
And I press "Delete"
And I set the field "Also show old questions" to "1"
Then I should not see "Broken question"
@javascript
Scenario: Delete question has multiple versions in question bank page
Given I am on the "Course 1" "core_question > course question bank" page logged in as "teacher1"
When the following "core_question > updated questions" exist:
| questioncategory | question | questiontext |
| Test questions | Test question to be deleted | Test question to be deleted version 2 |
And I choose "Delete" action for "Test question to be deleted" in the question bank
And I should see "This will delete the following question and all its versions:"
And I should not see "* Denotes questions which can't be deleted because they are in use. Instead, they will be hidden in the question bank unless you select 'Show old questions'."
And I press "Delete"
Then I should not see "Test question to be deleted"
And I should not see "Test question to be deleted version2"

View File

@ -288,4 +288,36 @@ class version_test extends \advanced_testcase {
$this->assertEquals(array_slice($questiondefinition, 1, 1)[0]->questionid, $questionid2);
$this->assertEquals(array_slice($questiondefinition, 2, 1)[0]->questionid, $questionid1);
}
/**
* Test that all the versions of questions are available from the method.
*
* @covers ::get_all_versions_of_questions
*/
public function test_get_all_versions_of_questions() {
global $DB;
$questionversions = [];
$qcategory = $this->qgenerator->create_question_category(['contextid' => $this->context->id]);
$question = $this->qgenerator->create_question('shortanswer', null,
[
'category' => $qcategory->id,
'idnumber' => 'id1'
]);
$questionversions[1] = $question->id;
// Create a new version.
$question = $this->qgenerator->update_question($question, null, ['idnumber' => 'id2']);
$questionversions[2] = $question->id;
// Change the id number and get the question object.
$question = $this->qgenerator->update_question($question, null, ['idnumber' => 'id3']);
$questionversions[3] = $question->id;
$questionbankentryid = $DB->get_record('question_versions', ['questionid' => $question->id], 'questionbankentryid');
$questionversionsofquestions = question_bank::get_all_versions_of_questions([$question->id]);
$questionbankentryids = array_keys($questionversionsofquestions)[0];
$this->assertEquals($questionbankentryid->questionbankentryid, $questionbankentryids);
$this->assertEquals($questionversions, $questionversionsofquestions[$questionbankentryids]);
}
}