MDL-87059 customfield: Check if custom field shortname is unique
- If is a shared field check that shortname does not exist anywhere. - If is an entity field check that the shortname does not exist within the same entity fields or in the shared fields.
This commit is contained in:
@@ -487,4 +487,60 @@ class api {
|
||||
];
|
||||
return shared::record_exists_select($sql, $params);
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks if the given shortname is unique for the handler's component-area-itemid combination and shared fields.
|
||||
*
|
||||
* @param handler $handler
|
||||
* @param string $shortname
|
||||
* @param int $fieldid
|
||||
* @return bool
|
||||
*/
|
||||
public static function is_shortname_unique(handler $handler, string $shortname, int $fieldid): bool {
|
||||
global $DB;
|
||||
|
||||
if ($handler->get_component() === 'core_customfield' && $handler->get_area() === 'shared') {
|
||||
// If it's a shared field, just check the shortname is unique among all fields.
|
||||
$params = [
|
||||
'shortname' => $shortname,
|
||||
'fieldid' => $fieldid,
|
||||
];
|
||||
return !$DB->record_exists_select('customfield_field', 'shortname = :shortname AND id <> :fieldid', $params);
|
||||
} else {
|
||||
// Check the shortname is unique for this component-area-itemid combination and shared fields.
|
||||
$query = "
|
||||
SELECT 1
|
||||
FROM {customfield_field} f
|
||||
JOIN {customfield_category} c ON c.id = f.categoryid
|
||||
WHERE f.shortname = :shortname
|
||||
AND (
|
||||
(
|
||||
f.id <> :fieldid
|
||||
AND c.component = :component
|
||||
AND c.area = :area
|
||||
AND c.itemid = :itemid
|
||||
)
|
||||
OR
|
||||
(
|
||||
c.component = :sharedcomponent
|
||||
AND c.area = :sharedarea
|
||||
AND c.itemid = :shareditemid
|
||||
)
|
||||
)
|
||||
";
|
||||
|
||||
$params = [
|
||||
'shortname' => $shortname,
|
||||
'fieldid' => $fieldid,
|
||||
'component' => $handler->get_component(),
|
||||
'area' => $handler->get_area(),
|
||||
'itemid' => $handler->get_itemid(),
|
||||
'sharedcomponent' => 'core_customfield',
|
||||
'sharedarea' => 'shared',
|
||||
'shareditemid' => 0,
|
||||
];
|
||||
|
||||
return !$DB->record_exists_sql($query, $params);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -118,15 +118,11 @@ class field_config_form extends \core_form\dynamic_form {
|
||||
$field = $this->get_field();
|
||||
$handler = $field->get_handler();
|
||||
|
||||
// Check the shortname is specified and is unique for this component-area-itemid combination.
|
||||
// Check the shortname is specified and is unique for this component-area-itemid combination and shared fields.
|
||||
if (!preg_match('/^[a-z0-9_]+$/', $data['shortname'])) {
|
||||
// Check allowed pattern (numbers, letters and underscore).
|
||||
$errors['shortname'] = get_string('invalidshortnameerror', 'core_customfield');
|
||||
} else if ($DB->record_exists_sql('SELECT 1 FROM {customfield_field} f ' .
|
||||
'JOIN {customfield_category} c ON c.id = f.categoryid ' .
|
||||
'WHERE f.shortname = ? AND f.id <> ? AND c.component = ? AND c.area = ? AND c.itemid = ?',
|
||||
[$data['shortname'], $data['id'],
|
||||
$handler->get_component(), $handler->get_area(), $handler->get_itemid()])) {
|
||||
} else if (!\core_customfield\api::is_shortname_unique($handler, $data['shortname'], $field->get('id'))) {
|
||||
$errors['shortname'] = get_string('formfieldcheckshortname', 'core_customfield');
|
||||
}
|
||||
|
||||
|
||||
@@ -19,6 +19,7 @@ namespace core_customfield;
|
||||
/**
|
||||
* Functional test for class \core_customfield\api
|
||||
*
|
||||
* @covers \core_customfield\api
|
||||
* @package core_customfield
|
||||
* @category test
|
||||
* @copyright 2018 Toni Barbera <[email protected]>
|
||||
@@ -271,4 +272,58 @@ final class api_test extends \advanced_testcase {
|
||||
$this->assertCount(6, $DB->get_records_select(\core_customfield\field::TABLE, 'id '.$sql, $p));
|
||||
$this->assertCount(6, $DB->get_records_select(\core_customfield\data::TABLE, 'fieldid '.$sql, $p));
|
||||
}
|
||||
|
||||
/**
|
||||
* Test for function api::is_shortname_unique()
|
||||
*/
|
||||
public function test_is_shortname_unique(): void {
|
||||
$this->resetAfterTest();
|
||||
$this->setAdminUser();
|
||||
|
||||
/** @var \core_customfield_generator $generator */
|
||||
$generator = $this->getDataGenerator()->get_plugin_generator('core_customfield');
|
||||
|
||||
// Create shared category and fields.
|
||||
$sharedcategory = $generator->create_category(['component' => 'core_customfield', 'area' => 'shared']);
|
||||
$generator->create_field([
|
||||
'categoryid' => $sharedcategory->get('id'),
|
||||
'name' => 'My shared field',
|
||||
'shortname' => 'mysharedfield',
|
||||
'type' => 'text',
|
||||
]);
|
||||
$sharedfield2 = $generator->create_field([
|
||||
'categoryid' => $sharedcategory->get('id'),
|
||||
'name' => 'My shared field 2',
|
||||
'shortname' => 'mysharedfield2',
|
||||
'type' => 'text',
|
||||
]);
|
||||
|
||||
// Create a course category and fields.
|
||||
$coursecategory = $generator->create_category(['component' => 'core_course', 'area' => 'course']);
|
||||
$generator->create_field([
|
||||
'categoryid' => $coursecategory->get('id'),
|
||||
'name' => 'My course field',
|
||||
'shortname' => 'mycoursefield',
|
||||
'type' => 'text',
|
||||
]);
|
||||
$coursefield2 = $generator->create_field([
|
||||
'categoryid' => $coursecategory->get('id'),
|
||||
'name' => 'My course field 2',
|
||||
'shortname' => 'mycoursefield2',
|
||||
'type' => 'text',
|
||||
]);
|
||||
|
||||
$coursehandler = \core_course\customfield\course_handler::create();
|
||||
$sharedhandler = \core_customfield\customfield\shared_handler::create();
|
||||
|
||||
$this->assertTrue(api::is_shortname_unique($coursehandler, 'otherfield', $coursefield2->get('id')));
|
||||
$this->assertTrue(api::is_shortname_unique($coursehandler, 'mycoursefield2', $coursefield2->get('id')));
|
||||
$this->assertFalse(api::is_shortname_unique($coursehandler, 'mycoursefield', $coursefield2->get('id')));
|
||||
$this->assertFalse(api::is_shortname_unique($coursehandler, 'mysharedfield', $coursefield2->get('id')));
|
||||
|
||||
$this->assertTrue(api::is_shortname_unique($sharedhandler, 'otherfield', $sharedfield2->get('id')));
|
||||
$this->assertTrue(api::is_shortname_unique($sharedhandler, 'mysharedfield2', $sharedfield2->get('id')));
|
||||
$this->assertFalse(api::is_shortname_unique($sharedhandler, 'mycoursefield', $sharedfield2->get('id')));
|
||||
$this->assertFalse(api::is_shortname_unique($sharedhandler, 'mysharedfield', $sharedfield2->get('id')));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6,8 +6,9 @@ Feature: Teachers can edit course custom fields
|
||||
|
||||
Background:
|
||||
Given the following "custom field categories" exist:
|
||||
| name | component | area | itemid |
|
||||
| Category for test | core_course | course | 0 |
|
||||
| name | component | area | itemid |
|
||||
| Category for test | core_course | course | 0 |
|
||||
| Shared category | core_customfield | shared | 0 |
|
||||
And the following "custom fields" exist:
|
||||
| name | category | type | shortname | description | configdata |
|
||||
| Field 1 | Category for test | text | f1 | d1 | |
|
||||
@@ -15,6 +16,7 @@ Feature: Teachers can edit course custom fields
|
||||
| Field 3 | Category for test | checkbox | f3 | d3 | |
|
||||
| Field 4 | Category for test | date | f4 | d4 | |
|
||||
| Field 5 | Category for test | select | f5 | d5 | {"options":"a\nb\nc"} |
|
||||
| Field 6 | Shared category | text | shf1 | shd1 | |
|
||||
And the following "users" exist:
|
||||
| username | firstname | lastname | email |
|
||||
| teacher1 | Teacher | 1 | teacher1@example.com |
|
||||
@@ -110,5 +112,9 @@ Feature: Teachers can edit course custom fields
|
||||
And I set the field "Short name" to "f1"
|
||||
And I click on "Save changes" "button" in the "Adding a new Short text" "dialogue"
|
||||
Then I should see "Short name already exists" in the "Short name" "form_row"
|
||||
# Let's try to use a shortname that exists in the Shared custom field.
|
||||
And I set the field "Short name" to "shf1"
|
||||
And I click on "Save changes" "button" in the "Adding a new Short text" "dialogue"
|
||||
Then I should see "Short name already exists" in the "Short name" "form_row"
|
||||
And I click on "Cancel" "button" in the "Adding a new Short text" "dialogue"
|
||||
And I log out
|
||||
|
||||
@@ -15,6 +15,37 @@ Feature: Create shared categories and fields
|
||||
And I wait until the page is ready
|
||||
And I wait until "Other fields" "text" does not exist
|
||||
|
||||
Scenario: Shared custom field short name must be unique across all instance fields
|
||||
Given the following "custom field categories" exist:
|
||||
| name | component | area | itemid |
|
||||
| Category for course | core_course | course | 0 |
|
||||
| Category for cohort | core_cohort | cohort | 0 |
|
||||
| Shared category | core_customfield | shared | 0 |
|
||||
And the following "custom fields" exist:
|
||||
| name | category | type | shortname | description |
|
||||
| Field 1 | Category for course | text | f1 | d1 |
|
||||
| Field 2 | Category for cohort | text | f2 | d2 |
|
||||
| Field 3 | Shared category | text | shf1 | shd1 |
|
||||
When I log in as "admin"
|
||||
And I navigate to "Custom fields > Shared custom fields" in site administration
|
||||
And I click on "Add a new custom field" "link"
|
||||
And I click on "Short text" "link"
|
||||
And I set the following fields to these values:
|
||||
| Name | Test field |
|
||||
| Short name | shf1 |
|
||||
And I click on "Save changes" "button" in the "Adding a new Short text" "dialogue"
|
||||
Then I should see "Short name already exists" in the "Short name" "form_row"
|
||||
And I set the field "Short name" to "f1"
|
||||
And I click on "Save changes" "button" in the "Adding a new Short text" "dialogue"
|
||||
Then I should see "Short name already exists" in the "Short name" "form_row"
|
||||
And I set the field "Short name" to "f2"
|
||||
And I click on "Save changes" "button" in the "Adding a new Short text" "dialogue"
|
||||
Then I should see "Short name already exists" in the "Short name" "form_row"
|
||||
And I set the field "Short name" to "f3"
|
||||
And I click on "Save changes" "button" in the "Adding a new Short text" "dialogue"
|
||||
And I should see "Add a new category"
|
||||
And I should see "f3"
|
||||
|
||||
Scenario: Shared customfields are displayed in other entities
|
||||
Given the following "custom field categories" exist:
|
||||
| name | component | area | itemid |
|
||||
|
||||
Reference in New Issue
Block a user