mirror of
https://github.com/moodle/moodle.git
synced 2025-04-21 00:12:56 +02:00
MDL-66796 question bank: fix more bugs with category editing
This commit is contained in:
Tim Hunt
parent
9c4f0fa746
commit
468d7ead0a
@ -164,7 +164,7 @@ class question_category_list_item extends list_item {
|
||||
|
||||
|
||||
/**
|
||||
* Class representing q question category
|
||||
* Class for performing operations on question categories.
|
||||
*
|
||||
* @copyright 1999 onwards Martin Dougiamas {@link http://moodle.com}
|
||||
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
|
||||
@ -180,7 +180,6 @@ class question_category_object {
|
||||
* @var array nested lists to display categories.
|
||||
*/
|
||||
public $editlists = array();
|
||||
public $newtable;
|
||||
public $tab;
|
||||
public $tabsize = 3;
|
||||
|
||||
@ -195,12 +194,17 @@ class question_category_object {
|
||||
public $catform;
|
||||
|
||||
/**
|
||||
* Constructor
|
||||
* Constructor.
|
||||
*
|
||||
* Gets necessary strings and sets relevant path information
|
||||
* @param int $page page number
|
||||
* @param moodle_url $pageurl base URL of the display categories page. Used for redirects.
|
||||
* @param context[] $contexts contexts where the current user can edit categories.
|
||||
* @param int $currentcat id of the category to be edited. 0 if none.
|
||||
* @param int|null $defaultcategory id of the current category. null if none.
|
||||
* @param int $todelete id of the category to delete. 0 if none.
|
||||
* @param context[] $addcontexts contexts where the current user can add questions.
|
||||
*/
|
||||
public function __construct($page, $pageurl, $contexts, $currentcat, $defaultcategory, $todelete, $addcontexts) {
|
||||
global $CFG, $COURSE, $OUTPUT;
|
||||
|
||||
$this->tab = str_repeat(' ', $this->tabsize);
|
||||
|
||||
@ -433,7 +437,19 @@ class question_category_object {
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a new category with given params
|
||||
* Create a new category.
|
||||
*
|
||||
* Data is expected to come from question_category_edit_form.
|
||||
*
|
||||
* By default redirects on success, unless $return is true.
|
||||
*
|
||||
* @param string $newparent 'categoryid,contextid' of the parent category.
|
||||
* @param string $newcategory the name.
|
||||
* @param string $newinfo the description.
|
||||
* @param bool $return if true, return rather than redirecting.
|
||||
* @param int|string $newinfoformat description format. One of the FORMAT_ constants.
|
||||
* @param null $idnumber the idnumber. '' is converted to null.
|
||||
* @return bool|int New category id if successful, else false.
|
||||
*/
|
||||
public function add_category($newparent, $newcategory, $newinfo, $return = false, $newinfoformat = FORMAT_HTML,
|
||||
$idnumber = null) {
|
||||
@ -487,16 +503,17 @@ class question_category_object {
|
||||
}
|
||||
|
||||
/**
|
||||
* Updates an existing category with given params
|
||||
* Updates an existing category with given params.
|
||||
*
|
||||
* @param int $updateid
|
||||
* @param int $newparent
|
||||
* @param string $newname
|
||||
* @param string $newinfo
|
||||
* @param int $newinfoformat
|
||||
* @param int $idnumber
|
||||
* @param bool $redirect
|
||||
* @return int
|
||||
* Warning! parameter order and meaning confusingly different from add_category in some ways!
|
||||
*
|
||||
* @param int $updateid id of the category to update.
|
||||
* @param int $newparent 'categoryid,contextid' of the parent category to set.
|
||||
* @param string $newname category name.
|
||||
* @param string $newinfo category description.
|
||||
* @param int|string $newinfoformat description format. One of the FORMAT_ constants.
|
||||
* @param int $idnumber the idnumber. '' is converted to null.
|
||||
* @param bool $redirect if true, will redirect once the DB is updated (default).
|
||||
*/
|
||||
public function update_category($updateid, $newparent, $newname, $newinfo, $newinfoformat = FORMAT_HTML,
|
||||
$idnumber = null, $redirect = true) {
|
||||
@ -536,8 +553,9 @@ class question_category_object {
|
||||
$idnumber = null;
|
||||
} else if (!empty($tocontextid)) {
|
||||
// While this check already exists in the form validation, this is a backstop preventing unnecessary errors.
|
||||
if ($DB->record_exists('question_categories',
|
||||
['idnumber' => $idnumber, 'contextid' => $tocontextid])) {
|
||||
if ($DB->record_exists_select('question_categories',
|
||||
'idnumber = ? AND contextid = ? AND id <> ?',
|
||||
[$idnumber, $tocontextid, $updateid])) {
|
||||
$idnumber = null;
|
||||
}
|
||||
}
|
||||
|
||||
178
question/tests/category_class_test.php
Normal file
178
question/tests/category_class_test.php
Normal file
@ -0,0 +1,178 @@
|
||||
<?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/>.
|
||||
|
||||
/**
|
||||
* Events tests.
|
||||
*
|
||||
* @package core_question
|
||||
* @copyright 2019 the Open University
|
||||
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
|
||||
*/
|
||||
|
||||
defined('MOODLE_INTERNAL') || die();
|
||||
|
||||
global $CFG;
|
||||
|
||||
require_once($CFG->dirroot . '/question/editlib.php');
|
||||
require_once($CFG->dirroot . '/question/category_class.php');
|
||||
|
||||
class core_question_category_class_testcase extends advanced_testcase {
|
||||
|
||||
/**
|
||||
* @var question_category_object used in the tests.
|
||||
*/
|
||||
protected $qcobject;
|
||||
|
||||
/**
|
||||
* @var context a context to use.
|
||||
*/
|
||||
protected $context;
|
||||
|
||||
/**
|
||||
* @var stdClass top category in context.
|
||||
*/
|
||||
protected $topcat;
|
||||
|
||||
protected function setUp() {
|
||||
parent::setUp();
|
||||
self::setAdminUser();
|
||||
$this->resetAfterTest();
|
||||
$this->context = context_course::instance(SITEID);
|
||||
$contexts = new question_edit_contexts($this->context);
|
||||
$this->topcat = question_get_top_category($this->context->id, true);
|
||||
$this->qcobject = new question_category_object(null,
|
||||
new moodle_url('/question/category.php', ['courseid' => SITEID]),
|
||||
$contexts->having_one_edit_tab_cap('categories'), 0, null, 0,
|
||||
$contexts->having_cap('moodle/question:add'));
|
||||
}
|
||||
|
||||
/**
|
||||
* Test creating a category.
|
||||
*/
|
||||
public function test_add_category_no_idnumber() {
|
||||
global $DB;
|
||||
|
||||
$id = $this->qcobject->add_category($this->topcat->id . ',' . $this->topcat->contextid,
|
||||
'New category', '', true, FORMAT_HTML, ''); // No idnumber passed as '' to match form data.
|
||||
|
||||
$newcat = $DB->get_record('question_categories', ['id' => $id], '*', MUST_EXIST);
|
||||
$this->assertSame('New category', $newcat->name);
|
||||
$this->assertNull($newcat->idnumber);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test creating a category with a tricky idnumber.
|
||||
*/
|
||||
public function test_add_category_set_idnumber_0() {
|
||||
global $DB;
|
||||
|
||||
$id = $this->qcobject->add_category($this->topcat->id . ',' . $this->topcat->contextid,
|
||||
'New category', '', true, FORMAT_HTML, '0');
|
||||
|
||||
$newcat = $DB->get_record('question_categories', ['id' => $id], '*', MUST_EXIST);
|
||||
$this->assertSame('New category', $newcat->name);
|
||||
$this->assertSame('0', $newcat->idnumber);
|
||||
}
|
||||
|
||||
/**
|
||||
* Trying to add a category with duplicate idnumber blanks it.
|
||||
* (In reality, this would probably get caught by form validation.)
|
||||
*/
|
||||
public function test_add_category_try_to_set_duplicate_idnumber() {
|
||||
global $DB;
|
||||
|
||||
$this->qcobject->add_category($this->topcat->id . ',' . $this->topcat->contextid,
|
||||
'Existing category', '', true, FORMAT_HTML, 'frog');
|
||||
|
||||
$id = $this->qcobject->add_category($this->topcat->id . ',' . $this->topcat->contextid,
|
||||
'New category', '', true, FORMAT_HTML, 'frog');
|
||||
|
||||
$newcat = $DB->get_record('question_categories', ['id' => $id], '*', MUST_EXIST);
|
||||
$this->assertSame('New category', $newcat->name);
|
||||
$this->assertNull($newcat->idnumber);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test updating a category.
|
||||
*/
|
||||
public function test_update_category() {
|
||||
global $DB;
|
||||
|
||||
$id = $this->qcobject->add_category($this->topcat->id . ',' . $this->topcat->contextid,
|
||||
'Old name', 'Description', true, FORMAT_HTML, 'frog');
|
||||
|
||||
$this->qcobject->update_category($id, $this->topcat->id . ',' . $this->topcat->contextid,
|
||||
'New name', 'New description', FORMAT_HTML, '0', false);
|
||||
|
||||
$newcat = $DB->get_record('question_categories', ['id' => $id], '*', MUST_EXIST);
|
||||
$this->assertSame('New name', $newcat->name);
|
||||
$this->assertSame('0', $newcat->idnumber);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test updating a category to remove the idnumber.
|
||||
*/
|
||||
public function test_update_category_removing_idnumber() {
|
||||
global $DB;
|
||||
|
||||
$id = $this->qcobject->add_category($this->topcat->id . ',' . $this->topcat->contextid,
|
||||
'Old name', 'Description', true, FORMAT_HTML, 'frog');
|
||||
|
||||
$this->qcobject->update_category($id, $this->topcat->id . ',' . $this->topcat->contextid,
|
||||
'New name', 'New description', FORMAT_HTML, '', false);
|
||||
|
||||
$newcat = $DB->get_record('question_categories', ['id' => $id], '*', MUST_EXIST);
|
||||
$this->assertSame('New name', $newcat->name);
|
||||
$this->assertNull($newcat->idnumber);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test updating a category without changing the idnumber.
|
||||
*/
|
||||
public function test_update_category_dont_change_idnumber() {
|
||||
global $DB;
|
||||
|
||||
$id = $this->qcobject->add_category($this->topcat->id . ',' . $this->topcat->contextid,
|
||||
'Old name', 'Description', true, FORMAT_HTML, 'frog');
|
||||
|
||||
$this->qcobject->update_category($id, $this->topcat->id . ',' . $this->topcat->contextid,
|
||||
'New name', 'New description', FORMAT_HTML, 'frog', false);
|
||||
|
||||
$newcat = $DB->get_record('question_categories', ['id' => $id], '*', MUST_EXIST);
|
||||
$this->assertSame('New name', $newcat->name);
|
||||
$this->assertSame('frog', $newcat->idnumber);
|
||||
}
|
||||
|
||||
/**
|
||||
* Trying to update a category so its idnumber duplicates idnumber blanks it.
|
||||
* (In reality, this would probably get caught by form validation.)
|
||||
*/
|
||||
public function test_update_category_try_to_set_duplicate_idnumber() {
|
||||
global $DB;
|
||||
|
||||
$this->qcobject->add_category($this->topcat->id . ',' . $this->topcat->contextid,
|
||||
'Existing category', '', true, FORMAT_HTML, 'toad');
|
||||
$id = $this->qcobject->add_category($this->topcat->id . ',' . $this->topcat->contextid,
|
||||
'old name', '', true, FORMAT_HTML, 'frog');
|
||||
|
||||
$this->qcobject->update_category($id, $this->topcat->id . ',' . $this->topcat->contextid,
|
||||
'New name', '', FORMAT_HTML, 'toad', false);
|
||||
|
||||
$newcat = $DB->get_record('question_categories', ['id' => $id], '*', MUST_EXIST);
|
||||
$this->assertSame('New name', $newcat->name);
|
||||
$this->assertNull($newcat->idnumber);
|
||||
}
|
||||
}
|
||||
Loading…
x
Reference in New Issue
Block a user