Merge branch 'MDL-84080_main' of https://github.com/marxjohnson/moodle
This commit is contained in:
@@ -105,17 +105,17 @@ class transfer_question_categories extends adhoc_task {
|
||||
switch ($oldcontext->contextlevel) {
|
||||
case CONTEXT_SYSTEM:
|
||||
$course = get_site();
|
||||
$bankname = get_string('systembank', 'question');
|
||||
$bankname = question_bank_helper::get_bank_name_string('systembank', 'question');
|
||||
break;
|
||||
case CONTEXT_COURSECAT:
|
||||
$coursecategory = core_course_category::get($oldcontext->instanceid);
|
||||
$courseshortname = "{$coursecategory->name}-{$coursecategory->id}";
|
||||
$course = $this->create_course($coursecategory, $courseshortname);
|
||||
$bankname = get_string("sharedbank", "mod_qbank", $coursecategory->name);
|
||||
$bankname = question_bank_helper::get_bank_name_string('sharedbank', 'mod_qbank', $coursecategory->name);
|
||||
break;
|
||||
case CONTEXT_COURSE:
|
||||
$course = get_course($oldcontext->instanceid);
|
||||
$bankname = get_string("sharedbank", "mod_qbank", $course->shortname);
|
||||
$bankname = question_bank_helper::get_bank_name_string('sharedbank', 'mod_qbank', $course->shortname);
|
||||
break;
|
||||
default:
|
||||
// This shouldn't be possible, so we can't really transfer it.
|
||||
|
||||
@@ -55,6 +55,7 @@ class mod_qbank_mod_form extends moodleform_mod {
|
||||
}
|
||||
$mform->addHelpButton('name', 'qbankname', 'mod_qbank');
|
||||
$mform->addRule('name', null, 'required', null, 'client');
|
||||
$mform->addRule('name', null, 'maxlength', \core_question\local\bank\question_bank_helper::BANK_NAME_MAX_LENGTH, 'client');
|
||||
|
||||
// Add intro editor.
|
||||
$mform->addElement('editor', 'introeditor', get_string('moduleintro'), ['rows' => 10], [
|
||||
|
||||
+1
-1
@@ -57,7 +57,7 @@ if ($createdefault) {
|
||||
require_sesskey();
|
||||
question_bank_helper::create_default_open_instance(
|
||||
$course,
|
||||
get_string('defaultbank', 'core_question', ['coursename' => $course->fullname])
|
||||
question_bank_helper::get_bank_name_string('defaultbank', 'core_question', ['coursename' => $course->fullname]),
|
||||
);
|
||||
\core\notification::add(get_string('defaultcreated', 'question'), \core\notification::SUCCESS);
|
||||
redirect($pageurl);
|
||||
|
||||
@@ -65,6 +65,11 @@ class question_bank_helper {
|
||||
*/
|
||||
private const CATEGORY_DELIMITER = '<->';
|
||||
|
||||
/**
|
||||
* Maximum length for the question bank name database field.
|
||||
*/
|
||||
public const BANK_NAME_MAX_LENGTH = 255;
|
||||
|
||||
/**
|
||||
* Modules that share questions via FEATURE_PUBLISHES_QUESTIONS.
|
||||
*
|
||||
@@ -411,7 +416,11 @@ class question_bank_helper {
|
||||
}
|
||||
|
||||
if (!$systembank && $createifnotexists) {
|
||||
$systembank = self::create_default_open_instance($course, get_string('systembank', 'question'), self::TYPE_SYSTEM);
|
||||
$systembank = self::create_default_open_instance(
|
||||
$course,
|
||||
self::get_bank_name_string('systembank', 'question'),
|
||||
self::TYPE_SYSTEM,
|
||||
);
|
||||
}
|
||||
|
||||
return $systembank;
|
||||
@@ -440,7 +449,11 @@ class question_bank_helper {
|
||||
}
|
||||
|
||||
if (!$previewbank && $createifnotexists) {
|
||||
$previewbank = self::create_default_open_instance($site, get_string('previewbank', 'question'), self::TYPE_PREVIEW);
|
||||
$previewbank = self::create_default_open_instance(
|
||||
$site,
|
||||
self::get_bank_name_string('previewbank', 'question'),
|
||||
self::TYPE_PREVIEW
|
||||
);
|
||||
}
|
||||
|
||||
return $previewbank;
|
||||
@@ -521,6 +534,13 @@ class question_bank_helper {
|
||||
}
|
||||
}
|
||||
|
||||
if (strlen($bankname) > self::BANK_NAME_MAX_LENGTH) {
|
||||
throw new \coding_exception(
|
||||
'The provided bankname is too long for the database field.',
|
||||
'Use question_bank_helper::get_bank_name_string to get a suitably truncated name.',
|
||||
);
|
||||
}
|
||||
|
||||
$data = new stdClass();
|
||||
$data->section = 0;
|
||||
$data->visible = 0;
|
||||
@@ -585,4 +605,39 @@ class question_bank_helper {
|
||||
global $CFG;
|
||||
return $CFG->corequestion_defaultqbankmod ?? 'qbank';
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the requested language string, with parameters truncated to ensure the result fits in the database.
|
||||
*
|
||||
* Since we may be generating a question bank name based on an existing course or category name, we need to ensure
|
||||
* that the resulting string isn't longer than the maximum module name.
|
||||
*
|
||||
* @param string $identifier The string identifier
|
||||
* @param string $component The string component
|
||||
* @param mixed|null $params The string parameters (a single string, array or object as accepted by get_string)
|
||||
* @return string The string truncated to a length that will fit in the database.
|
||||
*/
|
||||
public static function get_bank_name_string(string $identifier, string $component, mixed $params = null): string {
|
||||
if (is_object($params)) {
|
||||
$shortparams = (array) $params;
|
||||
} else {
|
||||
$shortparams = $params;
|
||||
}
|
||||
$bankname = get_string($identifier, $component, $shortparams);
|
||||
if (!is_null($shortparams)) {
|
||||
$trimlength = self::BANK_NAME_MAX_LENGTH - 4;
|
||||
while (\core_text::strlen($bankname) > self::BANK_NAME_MAX_LENGTH && $trimlength > 0) {
|
||||
// Gradually shorten the string parameters until the resulting string is short enough.
|
||||
if (is_array($shortparams)) {
|
||||
$shortparams = array_map(fn($param) => shorten_text(trim($param), $trimlength), $shortparams);
|
||||
} else {
|
||||
$shortparams = shorten_text(trim($shortparams), $trimlength);
|
||||
}
|
||||
$bankname = get_string($identifier, $component, $shortparams);
|
||||
$trimlength -= 10;
|
||||
}
|
||||
}
|
||||
// As a failsafe, limit the length of the final string in case the lang string is too long.
|
||||
return shorten_text($bankname, self::BANK_NAME_MAX_LENGTH);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -200,6 +200,28 @@ final class question_bank_helper_test extends \advanced_testcase {
|
||||
$this->assertEquals(1, $cminfo->showdescription);
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a default instance, passing a name that is too long for the database.
|
||||
*
|
||||
* @return void
|
||||
* @throws \coding_exception
|
||||
* @throws \dml_exception
|
||||
* @throws \moodle_exception
|
||||
*/
|
||||
public function test_create_default_open_instance_with_long_name(): void {
|
||||
$this->resetAfterTest();
|
||||
self::setAdminUser();
|
||||
|
||||
$coursename = random_string(question_bank_helper::BANK_NAME_MAX_LENGTH);
|
||||
$course = self::getDataGenerator()->create_course(['shortname' => $coursename]);
|
||||
|
||||
$this->expectExceptionMessage('The provided bankname is too long for the database field.');
|
||||
question_bank_helper::create_default_open_instance(
|
||||
$course,
|
||||
get_string('defaultbank', 'core_question', ['coursename' => $coursename]),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Assert that viewing a question bank logs the view for that user up to a maximum of 5 unique bank views.
|
||||
*
|
||||
@@ -310,4 +332,80 @@ final class question_bank_helper_test extends \advanced_testcase {
|
||||
$modrecord = $DB->get_record('qbank', ['id' => $qbank->instance]);
|
||||
$this->assertEquals(question_bank_helper::TYPE_SYSTEM, $modrecord->type);
|
||||
}
|
||||
|
||||
/**
|
||||
* Assert that get_bank_name_string returns suitably truncated strings.
|
||||
*
|
||||
* @dataProvider bank_name_strings
|
||||
* @param string $identifier
|
||||
* @param string $component
|
||||
* @param mixed $params
|
||||
* @param string $expected
|
||||
*/
|
||||
public function test_get_bank_name_string(string $identifier, string $component, mixed $params, string $expected): void {
|
||||
$this->assertEquals($expected, question_bank_helper::get_bank_name_string($identifier, $component, $params));
|
||||
}
|
||||
|
||||
/**
|
||||
* Get string examples with different parameter types and lengths.
|
||||
*
|
||||
* @return array[]
|
||||
*/
|
||||
public static function bank_name_strings(): array {
|
||||
$longname = 'One two three four five six seven eight nine ten eleven twelve thirteen fourteen fifteen sixteen seventeen ' .
|
||||
'eighteen nineteen twenty twenty-one twenty-two twenty-three twenty-four twenty-five twenty-six twenty-seven ' .
|
||||
'twenty-eight twenty-nine thirty thirty-one';
|
||||
return [
|
||||
'String with no parameters' => [
|
||||
'systembank',
|
||||
'question',
|
||||
null,
|
||||
'System shared question bank',
|
||||
],
|
||||
'String with short string parameter' => [
|
||||
'topfor',
|
||||
'question',
|
||||
'Test course',
|
||||
'Top for Test course',
|
||||
],
|
||||
'String with long string parameter' => [
|
||||
'topfor',
|
||||
'question',
|
||||
$longname,
|
||||
'Top for One two three four five six seven eight nine ten eleven twelve thirteen fourteen fifteen sixteen ' .
|
||||
'seventeen eighteen nineteen twenty twenty-one twenty-two twenty-three twenty-four twenty-five twenty-six ' .
|
||||
'twenty-seven twenty-eight ...',
|
||||
],
|
||||
'String with short array parameter' => [
|
||||
'defaultbank',
|
||||
'question',
|
||||
['coursename' => 'Test course'],
|
||||
'Test course course question bank',
|
||||
],
|
||||
'String with long array parameter' => [
|
||||
'defaultbank',
|
||||
'question',
|
||||
['coursename' => $longname],
|
||||
'One two three four five six seven eight nine ten eleven twelve thirteen fourteen fifteen sixteen seventeen ' .
|
||||
'eighteen nineteen twenty twenty-one twenty-two twenty-three twenty-four twenty-five twenty-six ' .
|
||||
'twenty-seven twenty-eight ... course question bank',
|
||||
],
|
||||
'String with multiple long array parameters' => [
|
||||
'markoutofmax',
|
||||
'question',
|
||||
['mark' => $longname, 'max' => $longname],
|
||||
'Mark One two three four five six seven eight nine ten eleven twelve thirteen fourteen fifteen sixteen seventeen ' .
|
||||
'eighteen ... out of One two three four five six seven eight nine ten eleven twelve thirteen fourteen ' .
|
||||
'fifteen sixteen seventeen eighteen ...',
|
||||
],
|
||||
'Long lang string' => [
|
||||
'howquestionsbehave_help',
|
||||
'question',
|
||||
null,
|
||||
'Students can interact with the questions in the quiz in various different ways. For example, you may wish the ' .
|
||||
'students to enter an answer to each question and then submit the entire quiz, before anything is graded or ' .
|
||||
'they get any feedback. That would ...',
|
||||
],
|
||||
];
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user