Merge branch 'MDL-29905-30' of git://github.com/FMCorz/moodle into MOODLE_30_STABLE

This commit is contained in:
David Monllao
2016-05-30 15:41:29 +08:00
4 changed files with 101 additions and 14 deletions
+1 -1
View File
@@ -65,7 +65,7 @@ $string['categorycurrent'] = 'Current category';
$string['categorycurrentuse'] = 'Use this category';
$string['categorydoesnotexist'] = 'This category does not exist';
$string['categoryinfo'] = 'Category info';
$string['categorymove'] = 'The category \'{$a->name}\' contains {$a->count} questions (some of them may be old, hidden, questions that are still in use in some existing quizzes). Please choose another category to move them to.';
$string['categorymove'] = 'The category \'{$a->name}\' contains {$a->count} questions (some of them may be old, hidden, questions, or Random questions that are still in use in some existing quizzes). Please choose another category to move them to.';
$string['categorymoveto'] = 'Save in category';
$string['categorynamecantbeblank'] = 'The category name cannot be blank.';
$string['clickflag'] = 'Flag question';
+31
View File
@@ -220,6 +220,37 @@ function match_grade_options($gradeoptionsfull, $grade, $matchgrades = 'error')
}
}
/**
* Remove stale questions from a category.
*
* While questions should not be left behind when they are not used any more,
* it does happen, maybe via restore, or old logic, or uncovered scenarios. When
* this happens, the users are unable to delete the question category unless
* they move those stale questions to another one category, but to them the
* category is empty as it does not contain anything. The purpose of this function
* is to detect the questions that may have gone stale and remove them.
*
* You will typically use this prior to checking if the category contains questions.
*
* The stale questions (unused and hidden to the user) handled are:
* - hidden questions
* - random questions
*
* @param int $categoryid The category ID.
*/
function question_remove_stale_questions_from_category($categoryid) {
global $DB;
$select = 'category = :categoryid AND (qtype = :qtype OR hidden = :hidden)';
$params = ['categoryid' => $categoryid, 'qtype' => 'random', 'hidden' => 1];
$questions = $DB->get_recordset_select("question", $select, $params, '', 'id');
foreach ($questions as $question) {
// The function question_delete_question does not delete questions in use.
question_delete_question($question->id);
}
$questions->close();
}
/**
* Category is about to be deleted,
* 1/ All questions are deleted for this question category.
+50
View File
@@ -397,4 +397,54 @@ class core_questionlib_testcase extends advanced_testcase {
$criteria = array('category' => $qcat->id);
$this->assertEquals(0, $DB->count_records('question', $criteria));
}
public function test_question_remove_stale_questions_from_category() {
global $DB;
$this->resetAfterTest(true);
$dg = $this->getDataGenerator();
$course = $dg->create_course();
$quiz = $dg->create_module('quiz', ['course' => $course->id]);
$qgen = $dg->get_plugin_generator('core_question');
$context = context_system::instance();
$qcat1 = $qgen->create_question_category(['contextid' => $context->id]);
$q1a = $qgen->create_question('shortanswer', null, ['category' => $qcat1->id]); // Will be hidden.
$q1b = $qgen->create_question('random', null, ['category' => $qcat1->id]); // Will not be used.
$DB->set_field('question', 'hidden', 1, ['id' => $q1a->id]);
$qcat2 = $qgen->create_question_category(['contextid' => $context->id]);
$q2a = $qgen->create_question('shortanswer', null, ['category' => $qcat2->id]); // Will be hidden.
$q2b = $qgen->create_question('shortanswer', null, ['category' => $qcat2->id]); // Will be hidden but used.
$q2c = $qgen->create_question('random', null, ['category' => $qcat2->id]); // Will not be used.
$q2d = $qgen->create_question('random', null, ['category' => $qcat2->id]); // Will be used.
$DB->set_field('question', 'hidden', 1, ['id' => $q2a->id]);
$DB->set_field('question', 'hidden', 1, ['id' => $q2b->id]);
quiz_add_quiz_question($q2b->id, $quiz);
quiz_add_quiz_question($q2d->id, $quiz);
$this->assertEquals(2, $DB->count_records('question', ['category' => $qcat1->id]));
$this->assertEquals(4, $DB->count_records('question', ['category' => $qcat2->id]));
// Non-existing category, nothing will happen.
question_remove_stale_questions_from_category(0);
$this->assertEquals(2, $DB->count_records('question', ['category' => $qcat1->id]));
$this->assertEquals(4, $DB->count_records('question', ['category' => $qcat2->id]));
// First category, should be empty afterwards.
question_remove_stale_questions_from_category($qcat1->id);
$this->assertEquals(0, $DB->count_records('question', ['category' => $qcat1->id]));
$this->assertEquals(4, $DB->count_records('question', ['category' => $qcat2->id]));
$this->assertFalse($DB->record_exists('question', ['id' => $q1a->id]));
$this->assertFalse($DB->record_exists('question', ['id' => $q1b->id]));
// Second category, used questions should be left untouched.
question_remove_stale_questions_from_category($qcat2->id);
$this->assertEquals(0, $DB->count_records('question', ['category' => $qcat1->id]));
$this->assertEquals(2, $DB->count_records('question', ['category' => $qcat2->id]));
$this->assertFalse($DB->record_exists('question', ['id' => $q2a->id]));
$this->assertTrue($DB->record_exists('question', ['id' => $q2b->id]));
$this->assertFalse($DB->record_exists('question', ['id' => $q2c->id]));
$this->assertTrue($DB->record_exists('question', ['id' => $q2d->id]));
}
}
+19 -13
View File
@@ -81,21 +81,27 @@ if ($param->moveupcontext || $param->movedowncontext) {
// The previous line does a redirect().
}
if ($param->delete && ($questionstomove = $DB->count_records("question", array("category" => $param->delete)))) {
if (!$category = $DB->get_record("question_categories", array("id" => $param->delete))) { // security
if ($param->delete) {
if (!$category = $DB->get_record("question_categories", array("id" => $param->delete))) {
print_error('nocate', 'question', $thispageurl->out(), $param->delete);
}
$categorycontext = context::instance_by_id($category->contextid);
$qcobject->moveform = new question_move_form($thispageurl,
array('contexts'=>array($categorycontext), 'currentcat'=>$param->delete));
if ($qcobject->moveform->is_cancelled()){
redirect($thispageurl);
} elseif ($formdata = $qcobject->moveform->get_data()) {
/// 'confirm' is the category to move existing questions to
list($tocategoryid, $tocontextid) = explode(',', $formdata->category);
$qcobject->move_questions_and_delete_category($formdata->delete, $tocategoryid);
$thispageurl->remove_params('cat', 'category');
redirect($thispageurl);
question_remove_stale_questions_from_category($param->delete);
$questionstomove = $DB->count_records("question", array("category" => $param->delete));
// Second pass, if we still have questions to move, setup the form.
if ($questionstomove) {
$categorycontext = context::instance_by_id($category->contextid);
$qcobject->moveform = new question_move_form($thispageurl,
array('contexts' => array($categorycontext), 'currentcat' => $param->delete));
if ($qcobject->moveform->is_cancelled()) {
redirect($thispageurl);
} else if ($formdata = $qcobject->moveform->get_data()) {
list($tocategoryid, $tocontextid) = explode(',', $formdata->category);
$qcobject->move_questions_and_delete_category($formdata->delete, $tocategoryid);
$thispageurl->remove_params('cat', 'category');
redirect($thispageurl);
}
}
} else {
$questionstomove = 0;