MDL-60260 course: validate external fields on create/update course.
Ensure course fullname/shortname fields are not empty.
This commit is contained in:
+15
-2
@@ -855,6 +855,13 @@ class core_course_external extends external_api {
|
||||
}
|
||||
require_capability('moodle/course:create', $context);
|
||||
|
||||
// Fullname and short name are required to be non-empty.
|
||||
if (trim($course['fullname']) === '') {
|
||||
throw new moodle_exception('errorinvalidparam', 'webservice', '', 'fullname');
|
||||
} else if (trim($course['shortname']) === '') {
|
||||
throw new moodle_exception('errorinvalidparam', 'webservice', '', 'shortname');
|
||||
}
|
||||
|
||||
// Make sure lang is valid
|
||||
if (array_key_exists('lang', $course)) {
|
||||
if (empty($availablelangs[$course['lang']])) {
|
||||
@@ -1040,14 +1047,20 @@ class core_course_external extends external_api {
|
||||
$course['category'] = $course['categoryid'];
|
||||
}
|
||||
|
||||
// Check if the user can change fullname.
|
||||
// Check if the user can change fullname, and the new value is non-empty.
|
||||
if (array_key_exists('fullname', $course) && ($oldcourse->fullname != $course['fullname'])) {
|
||||
require_capability('moodle/course:changefullname', $context);
|
||||
if (trim($course['fullname']) === '') {
|
||||
throw new moodle_exception('errorinvalidparam', 'webservice', '', 'fullname');
|
||||
}
|
||||
}
|
||||
|
||||
// Check if the user can change shortname.
|
||||
// Check if the user can change shortname, and the new value is non-empty.
|
||||
if (array_key_exists('shortname', $course) && ($oldcourse->shortname != $course['shortname'])) {
|
||||
require_capability('moodle/course:changeshortname', $context);
|
||||
if (trim($course['shortname']) === '') {
|
||||
throw new moodle_exception('errorinvalidparam', 'webservice', '', 'shortname');
|
||||
}
|
||||
}
|
||||
|
||||
// Check if the user can change the idnumber.
|
||||
|
||||
@@ -551,6 +551,80 @@ class core_course_externallib_testcase extends externallib_advanced_testcase {
|
||||
$createdsubcats = core_course_external::create_courses($courses);
|
||||
}
|
||||
|
||||
/**
|
||||
* Data provider for testing empty fields produce expected exceptions
|
||||
*
|
||||
* @see test_create_courses_empty_field
|
||||
* @see test_update_courses_empty_field
|
||||
*
|
||||
* @return array
|
||||
*/
|
||||
public function course_empty_field_provider(): array {
|
||||
return [
|
||||
[[
|
||||
'fullname' => '',
|
||||
'shortname' => 'ws101',
|
||||
], 'fullname'],
|
||||
[[
|
||||
'fullname' => ' ',
|
||||
'shortname' => 'ws101',
|
||||
], 'fullname'],
|
||||
[[
|
||||
'fullname' => 'Web Services',
|
||||
'shortname' => '',
|
||||
], 'shortname'],
|
||||
[[
|
||||
'fullname' => 'Web Services',
|
||||
'shortname' => ' ',
|
||||
], 'shortname'],
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* Test creating courses with empty fields throws an exception
|
||||
*
|
||||
* @param array $course
|
||||
* @param string $expectedemptyfield
|
||||
*
|
||||
* @dataProvider course_empty_field_provider
|
||||
*/
|
||||
public function test_create_courses_empty_field(array $course, string $expectedemptyfield): void {
|
||||
$this->resetAfterTest();
|
||||
$this->setAdminUser();
|
||||
|
||||
// Create a category for the new course.
|
||||
$course['categoryid'] = $this->getDataGenerator()->create_category()->id;
|
||||
|
||||
$this->expectException(moodle_exception::class);
|
||||
$this->expectExceptionMessageRegExp("/{$expectedemptyfield}/");
|
||||
core_course_external::create_courses([$course]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test updating courses with empty fields returns warnings
|
||||
*
|
||||
* @param array $course
|
||||
* @param string $expectedemptyfield
|
||||
*
|
||||
* @dataProvider course_empty_field_provider
|
||||
*/
|
||||
public function test_update_courses_empty_field(array $course, string $expectedemptyfield): void {
|
||||
$this->resetAfterTest();
|
||||
$this->setAdminUser();
|
||||
|
||||
// Create a course to update.
|
||||
$course['id'] = $this->getDataGenerator()->create_course()->id;
|
||||
|
||||
$result = core_course_external::update_courses([$course]);
|
||||
$result = core_course_external::clean_returnvalue(core_course_external::update_courses_returns(), $result);
|
||||
|
||||
$this->assertCount(1, $result['warnings']);
|
||||
|
||||
$warning = reset($result['warnings']);
|
||||
$this->assertEquals('errorinvalidparam', $warning['warningcode']);
|
||||
$this->assertContains($expectedemptyfield, $warning['message']);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test delete_courses
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user