Skip to content

Commit 932e59f

Browse files
cconard96cconard96
authored
Review custom object system name and label
1 parent fed2f4d commit 932e59f

File tree

15 files changed

+236
-90
lines changed

15 files changed

+236
-90
lines changed

‎ajax/asset/assetdefinition.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@
7373
throw new NotFoundHttpException();
7474
}
7575
} else {
76-
$custom_field->fields['name'] = '';
76+
$custom_field->fields['system_name'] = '';
7777
$custom_field->fields['label'] = $field['label'];
7878
$custom_field->fields['type'] = $field['type'];
7979
$custom_field->fields['itemtype'] = 'Computer'; // Doesn't matter what it is as long as it's not empty

‎install/migrations/update_10.0.x_to_11.0.0/assets.php‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
CREATE TABLE `glpi_assets_assetdefinitions` (
4949
`id` int {$default_key_sign} NOT NULL AUTO_INCREMENT,
5050
`system_name` varchar(255) DEFAULT NULL,
51+
`label` varchar(255) NOT NULL,
5152
`icon` varchar(255) DEFAULT NULL,
5253
`comment` text,
5354
`is_active` tinyint NOT NULL DEFAULT '0',
@@ -69,9 +70,13 @@
6970
foreach (['profiles', 'translations', 'fields_display'] as $field) {
7071
$migration->addField('glpi_assets_assetdefinitions', $field, 'JSON NOT NULL', ['update' => "'[]'"]);
7172
}
73+
$migration->addField('glpi_assets_assetdefinitions', 'label', 'string', [
74+
'after' => 'system_name',
75+
'update' => $DB::quoteName('system_name'),
76+
]);
7277
}
7378

74-
$ADDTODISPLAYPREF['Glpi\\Asset\\AssetDefinition'] = [3, 4, 5, 6];
79+
$ADDTODISPLAYPREF['Glpi\\Asset\\AssetDefinition'] = [2, 3, 4, 5, 6];
7580

7681
if (!$DB->tableExists('glpi_assets_assets')) {
7782
$query = <<<SQL
@@ -201,21 +206,22 @@
201206
CREATE TABLE `glpi_assets_customfielddefinitions` (
202207
`id` int {$default_key_sign} NOT NULL AUTO_INCREMENT,
203208
`assets_assetdefinitions_id` int {$default_key_sign} NOT NULL,
204-
`name` varchar(255) NOT NULL,
209+
`system_name` varchar(255) NOT NULL,
205210
`label` varchar(255) NOT NULL,
206211
`type` varchar(255) NOT NULL,
207212
`field_options` json,
208213
`itemtype` VARCHAR(255) NULL DEFAULT NULL,
209214
`default_value` text,
210215
`translations` JSON NOT NULL,
211216
PRIMARY KEY (`id`),
212-
UNIQUE KEY `unicity` (`assets_assetdefinitions_id`, `name`),
213-
KEY `name` (`name`)
217+
UNIQUE KEY `unicity` (`assets_assetdefinitions_id`, `system_name`),
218+
KEY `system_name` (`system_name`)
214219
) ENGINE=InnoDB DEFAULT CHARSET={$default_charset} COLLATE={$default_collation} ROW_FORMAT=DYNAMIC;
215220
SQL;
216221
$DB->doQuery($query);
217222
} else {
218223
$migration->addField('glpi_assets_customfielddefinitions', 'translations', 'JSON NOT NULL', ['update' => "'[]'"]);
224+
$migration->changeField('glpi_assets_customfielddefinitions', 'name', 'system_name', 'string');
219225
}
220226

221227
// Dev migration

‎install/migrations/update_10.0.x_to_11.0.0/dropdowns.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
CREATE TABLE `glpi_dropdowns_dropdowndefinitions` (
4848
`id` int {$default_key_sign} NOT NULL AUTO_INCREMENT,
4949
`system_name` varchar(255) DEFAULT NULL,
50+
`label` varchar(255) NOT NULL,
5051
`icon` varchar(255) DEFAULT NULL,
5152
`comment` text,
5253
`is_active` tinyint NOT NULL DEFAULT '0',
@@ -62,6 +63,11 @@
6263
) ENGINE=InnoDB DEFAULT CHARSET={$default_charset} COLLATE={$default_collation} ROW_FORMAT=DYNAMIC;
6364
SQL;
6465
$DB->doQuery($query);
66+
} else {
67+
$migration->addField('glpi_dropdowns_dropdowndefinitions', 'label', 'string', [
68+
'after' => 'system_name',
69+
'update' => $DB::quoteName('system_name'),
70+
]);
6571
}
6672

6773
if (!$DB->tableExists('glpi_dropdowns_dropdowns')) {

‎install/mysql/glpi-empty.sql‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9904,6 +9904,7 @@ DROP TABLE IF EXISTS `glpi_assets_assetdefinitions`;
99049904
CREATE TABLE `glpi_assets_assetdefinitions` (
99059905
`id` int unsigned NOT NULL AUTO_INCREMENT,
99069906
`system_name` varchar(255) DEFAULT NULL,
9907+
`label` varchar(255) NOT NULL,
99079908
`icon` varchar(255) DEFAULT NULL,
99089909
`comment` text,
99099910
`is_active` tinyint NOT NULL DEFAULT '0',
@@ -10028,6 +10029,7 @@ DROP TABLE IF EXISTS `glpi_dropdowns_dropdowndefinitions`;
1002810029
CREATE TABLE `glpi_dropdowns_dropdowndefinitions` (
1002910030
`id` int unsigned NOT NULL AUTO_INCREMENT,
1003010031
`system_name` varchar(255) DEFAULT NULL,
10032+
`label` varchar(255) NOT NULL,
1003110033
`icon` varchar(255) DEFAULT NULL,
1003210034
`comment` text,
1003310035
`is_active` tinyint NOT NULL DEFAULT '0',
@@ -10072,16 +10074,16 @@ DROP TABLE IF EXISTS `glpi_assets_customfielddefinitions`;
1007210074
CREATE TABLE `glpi_assets_customfielddefinitions` (
1007310075
`id` int unsigned NOT NULL AUTO_INCREMENT,
1007410076
`assets_assetdefinitions_id` int unsigned NOT NULL,
10075-
`name` varchar(255) NOT NULL,
10077+
`system_name` varchar(255) NOT NULL,
1007610078
`label` varchar(255) NOT NULL,
1007710079
`type` varchar(255) NOT NULL,
1007810080
`field_options` json,
1007910081
`itemtype` VARCHAR(255) NULL DEFAULT NULL,
1008010082
`default_value` text,
1008110083
`translations` JSON NOT NULL,
1008210084
PRIMARY KEY (`id`),
10083-
UNIQUE KEY `unicity` (`assets_assetdefinitions_id`, `name`),
10084-
KEY `name` (`name`)
10085+
UNIQUE KEY `unicity` (`assets_assetdefinitions_id`, `system_name`),
10086+
KEY `system_name` (`system_name`)
1008510087
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci ROW_FORMAT=DYNAMIC;
1008610088

1008710089
SET FOREIGN_KEY_CHECKS=1;

‎js/src/vue/CustomObject/FieldPreview/FieldDisplay.vue‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@
217217
create_edit_field_modal.on('glpi:submit:success', 'form', (e, data) => {
218218
const btn_submit = data.submitter;
219219
const form_data = new FormData(e.target);
220-
const field_key = `custom_${form_data.get('name')}`;
220+
const field_key = `custom_${form_data.get('system_name')}`;
221221
222222
if (btn_submit.attr('name') === 'add') {
223223
new_field_dropdown.data('select2').dataAdapter.query('', (data) => {

‎phpunit/functional/Glpi/Asset/CustomFieldDefinitionTest.php‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,15 @@ public function testCleanDBOnPurge()
6464

6565
$custom_field_definition = $this->createItem(\Glpi\Asset\CustomFieldDefinition::class, [
6666
'assets_assetdefinitions_id' => $asset_definition->getID(),
67-
'name' => 'test_string',
67+
'system_name' => 'test_string',
6868
'label' => 'Test string',
6969
'type' => StringType::class,
7070
'default_value' => 'default',
7171
]);
7272

7373
$custom_field_definition_2 = $this->createItem(\Glpi\Asset\CustomFieldDefinition::class, [
7474
'assets_assetdefinitions_id' => $asset_definition->getID(),
75-
'name' => 'test_text',
75+
'system_name' => 'test_text',
7676
'label' => 'Test text',
7777
'type' => TextType::class,
7878
'default_value' => 'default text',
@@ -282,7 +282,7 @@ public function testGetSearchOption()
282282

283283
$custom_field_definition = $this->createItem(\Glpi\Asset\CustomFieldDefinition::class, [
284284
'assets_assetdefinitions_id' => $asset_definition->getID(),
285-
'name' => 'test_string',
285+
'system_name' => 'test_string',
286286
'label' => 'Test string',
287287
'type' => StringType::class,
288288
'default_value' => 'default',
@@ -296,7 +296,7 @@ public function testGetSearchOption()
296296

297297
$custom_field_definition_2 = $this->createItem(\Glpi\Asset\CustomFieldDefinition::class, [
298298
'assets_assetdefinitions_id' => $asset_definition->getID(),
299-
'name' => 'test_text',
299+
'system_name' => 'test_text',
300300
'label' => 'Test text',
301301
'type' => TextType::class,
302302
'default_value' => 'default text',
@@ -311,7 +311,7 @@ public function testGetSearchOption()
311311

312312
$custom_field_definition_4 = $this->createItem(\Glpi\Asset\CustomFieldDefinition::class, [
313313
'assets_assetdefinitions_id' => $asset_definition->getID(),
314-
'name' => 'test_number',
314+
'system_name' => 'test_number',
315315
'label' => 'Test number',
316316
'type' => NumberType::class,
317317
'default_value' => 420,
@@ -325,7 +325,7 @@ public function testGetSearchOption()
325325

326326
$custom_field_definition_5 = $this->createItem(\Glpi\Asset\CustomFieldDefinition::class, [
327327
'assets_assetdefinitions_id' => $asset_definition->getID(),
328-
'name' => 'test_date',
328+
'system_name' => 'test_date',
329329
'label' => 'Test date',
330330
'type' => DateType::class,
331331
'default_value' => '2021-01-01',
@@ -339,7 +339,7 @@ public function testGetSearchOption()
339339

340340
$custom_field_definition_6 = $this->createItem(\Glpi\Asset\CustomFieldDefinition::class, [
341341
'assets_assetdefinitions_id' => $asset_definition->getID(),
342-
'name' => 'test_datetime',
342+
'system_name' => 'test_datetime',
343343
'label' => 'Test datetime',
344344
'type' => DateTimeType::class,
345345
'default_value' => '2021-01-01 03:25:15',
@@ -353,7 +353,7 @@ public function testGetSearchOption()
353353

354354
$custom_field_definition_7 = $this->createItem(\Glpi\Asset\CustomFieldDefinition::class, [
355355
'assets_assetdefinitions_id' => $asset_definition->getID(),
356-
'name' => 'test_dropdown',
356+
'system_name' => 'test_dropdown',
357357
'label' => 'Test dropdown',
358358
'type' => DropdownType::class,
359359
'itemtype' => \Computer::class,
@@ -366,7 +366,7 @@ public function testGetSearchOption()
366366

367367
$custom_field_definition_8 = $this->createItem(\Glpi\Asset\CustomFieldDefinition::class, [
368368
'assets_assetdefinitions_id' => $asset_definition->getID(),
369-
'name' => 'test_url',
369+
'system_name' => 'test_url',
370370
'label' => 'Test url',
371371
'type' => URLType::class,
372372
'default_value' => 'https://glpi-project.org',
@@ -380,7 +380,7 @@ public function testGetSearchOption()
380380

381381
$custom_field_definition_9 = $this->createItem(\Glpi\Asset\CustomFieldDefinition::class, [
382382
'assets_assetdefinitions_id' => $asset_definition->getID(),
383-
'name' => 'test_bool',
383+
'system_name' => 'test_bool',
384384
'label' => 'Test bool',
385385
'type' => BooleanType::class,
386386
'default_value' => '1',
@@ -401,15 +401,15 @@ public function testSystemNameUnqiue()
401401

402402
$this->assertGreaterThan(0, $field->add([
403403
'assets_assetdefinitions_id' => $asset_definition->getID(),
404-
'name' => 'test',
404+
'system_name' => 'test',
405405
'label' => 'Test',
406406
'type' => StringType::class,
407407
'default_value' => 'default',
408408
]));
409409

410410
$this->assertFalse($field->add([
411411
'assets_assetdefinitions_id' => $asset_definition->getID(),
412-
'name' => 'test',
412+
'system_name' => 'test',
413413
'label' => 'Test',
414414
'type' => StringType::class,
415415
'default_value' => 'default',
@@ -437,7 +437,7 @@ public function testDateTimezones()
437437

438438
$fields_id = $field->add([
439439
'assets_assetdefinitions_id' => $asset_definition->getID(),
440-
'name' => 'test_datetime',
440+
'system_name' => 'test_datetime',
441441
'label' => 'Test datetime',
442442
'type' => DateTimeType::class,
443443
'default_value' => '2021-01-01 03:25:15',
@@ -494,14 +494,14 @@ public function testCustomFieldHistory()
494494

495495
$field_1->add([
496496
'assets_assetdefinitions_id' => $asset_definition->getID(),
497-
'name' => 'test',
497+
'system_name' => 'test',
498498
'label' => 'Test',
499499
'type' => StringType::class,
500500
'default_value' => 'default',
501501
]);
502502
$field_2->add([
503503
'assets_assetdefinitions_id' => $asset_definition->getID(),
504-
'name' => 'test_two',
504+
'system_name' => 'test_two',
505505
'label' => 'Test2',
506506
'type' => DropdownType::class,
507507
'itemtype' => \Computer::class,
@@ -542,7 +542,7 @@ public function testAddTranslationsOnCreate()
542542
$field = new \Glpi\Asset\CustomFieldDefinition();
543543
$this->assertGreaterThan(0, $field->add([
544544
'assets_assetdefinitions_id' => $asset_definition->getID(),
545-
'name' => 'test',
545+
'system_name' => 'test',
546546
'label' => 'Test',
547547
'type' => StringType::class,
548548
'translations' => [
@@ -559,7 +559,7 @@ public function testAddTranslationsOnUpdate()
559559
$field = new \Glpi\Asset\CustomFieldDefinition();
560560
$this->assertGreaterThan(0, $field->add([
561561
'assets_assetdefinitions_id' => $asset_definition->getID(),
562-
'name' => 'test',
562+
'system_name' => 'test',
563563
'label' => 'Test',
564564
'type' => StringType::class,
565565
]));

‎src/Glpi/Asset/Asset.php‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ public function showForm($ID, array $options = [])
333333
{
334334
$this->initForm($ID, $options);
335335
$custom_fields = static::getDefinition()->getCustomFieldDefinitions();
336-
$custom_fields = array_combine(array_map(static fn ($f) => 'custom_' . $f->fields['name'], $custom_fields), $custom_fields);
336+
$custom_fields = array_combine(array_map(static fn ($f) => 'custom_' . $f->fields['system_name'], $custom_fields), $custom_fields);
337337
$fields_display = static::getDefinition()->getDecodedFieldsField();
338338
$core_field_options = [];
339339

@@ -377,7 +377,7 @@ protected function handleCustomFieldsUpdate(array $input): array
377377
$custom_fields = $this->getDecodedCustomFields();
378378

379379
foreach (static::getDefinition()->getCustomFieldDefinitions() as $custom_field) {
380-
$custom_field_name = 'custom_' . $custom_field->fields['name'];
380+
$custom_field_name = 'custom_' . $custom_field->fields['system_name'];
381381
if (!isset($input[$custom_field_name])) {
382382
continue;
383383
}
@@ -406,7 +406,7 @@ public function getEmpty()
406406
}
407407

408408
foreach (static::getDefinition()->getCustomFieldDefinitions() as $custom_field) {
409-
$f_name = 'custom_' . $custom_field->fields['name'];
409+
$f_name = 'custom_' . $custom_field->fields['system_name'];
410410
$this->fields[$f_name] = $custom_field->fields['default_value'];
411411
}
412412
return true;
@@ -422,7 +422,7 @@ public function post_getFromDB()
422422
$custom_field_values = $this->getDecodedCustomFields();
423423

424424
foreach ($custom_field_definitions as $custom_field) {
425-
$custom_field_name = 'custom_' . $custom_field->fields['name'];
425+
$custom_field_name = 'custom_' . $custom_field->fields['system_name'];
426426
$value = $custom_field_values[$custom_field->getID()] ?? $custom_field->fields['default_value'];
427427

428428
$this->fields[$custom_field_name] = $custom_field->getFieldType()->formatValueFromDB($value);
@@ -435,7 +435,7 @@ public function pre_updateInDB()
435435
// Fill old values for custom fields
436436
$custom_field_definitions = static::getDefinition()->getCustomFieldDefinitions();
437437
foreach ($custom_field_definitions as $custom_field) {
438-
$custom_field_name = 'custom_' . $custom_field->fields['name'];
438+
$custom_field_name = 'custom_' . $custom_field->fields['system_name'];
439439
$this->oldvalues[$custom_field_name] = $this->fields[$custom_field_name];
440440
}
441441
}
@@ -445,7 +445,7 @@ public function post_updateItem($history = true)
445445
$this->post_updateItemFromAssignableItem($history);
446446
if ($this->dohistory && $history && in_array('custom_fields', $this->updates, true)) {
447447
foreach (static::getDefinition()->getCustomFieldDefinitions() as $custom_field) {
448-
$custom_field_name = 'custom_' . $custom_field->fields['name'];
448+
$custom_field_name = 'custom_' . $custom_field->fields['system_name'];
449449
$field_type = $custom_field->getFieldType();
450450
$old_value = $field_type->formatValueFromDB($this->oldvalues[$custom_field_name] ?? $field_type->getDefaultValue());
451451
$current_value = $field_type->formatValueFromDB($this->fields[$custom_field_name] ?? null);
@@ -470,7 +470,7 @@ public function post_updateItem($history = true)
470470
public function getNonLoggedFields(): array
471471
{
472472
$ignored_fields = array_map(
473-
static fn (CustomFieldDefinition $field) => 'custom_' . $field->fields['name'],
473+
static fn (CustomFieldDefinition $field) => 'custom_' . $field->fields['system_name'],
474474
static::getDefinition()->getCustomFieldDefinitions()
475475
);
476476
$ignored_fields[] = 'custom_fields';

‎src/Glpi/Asset/AssetDefinition.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -686,7 +686,7 @@ public function getAllFields(): array
686686
];
687687

688688
foreach ($this->getCustomFieldDefinitions() as $custom_field_def) {
689-
$fields['custom_' . $custom_field_def->fields['name']] = [
689+
$fields['custom_' . $custom_field_def->fields['system_name']] = [
690690
'customfields_id' => $custom_field_def->getID(),
691691
'text' => $custom_field_def->computeFriendlyName(),
692692
'type' => $custom_field_def->fields['type'],

‎src/Glpi/Asset/CustomFieldDefinition.php‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ public function cleanDBonPurge()
8989
$fields_display = json_decode($it->current()['fields_display'] ?? '[]', true) ?? [];
9090
$order = 0;
9191
foreach ($fields_display as $k => $field) {
92-
if ($field['key'] === 'custom_' . $this->fields['name']) {
92+
if ($field['key'] === 'custom_' . $this->fields['system_name']) {
9393
$order = $field['order'];
9494
unset($fields_display[$k]);
9595
break;
@@ -157,12 +157,15 @@ private function validateSystemName(array &$input): bool
157157
/** @var \DBmysql $DB */
158158
global $DB;
159159

160-
// Spaces are replaced with underscores and the name is made lowercase. Only lowercase letters and underscores are kept.
161-
$input['name'] = preg_replace('/[^a-z_]/', '', strtolower(str_replace(' ', '_', $input['name'])));
162-
// The name cannot start with an underscore
163-
$input['name'] = ltrim($input['name'], '_');
164-
if ($input['name'] === '') {
165-
Session::addMessageAfterRedirect(__s('The system name must not be empty'), false, ERROR);
160+
if (!is_string($input['system_name']) || preg_match('/^[a-z_]+$/', $input['system_name']) !== 1) {
161+
Session::addMessageAfterRedirect(
162+
htmlescape(sprintf(
163+
__('The following field has an incorrect value: "%s".'),
164+
__('System name')
165+
)),
166+
false,
167+
ERROR
168+
);
166169
return false;
167170
}
168171

@@ -171,7 +174,7 @@ private function validateSystemName(array &$input): bool
171174
'COUNT' => 'cpt',
172175
'FROM' => self::getTable(),
173176
'WHERE' => [
174-
'name' => $input['name'],
177+
'system_name' => $input['system_name'],
175178
AssetDefinition::getForeignKeyField() => $input[self::$items_id],
176179
],
177180
]);
@@ -237,8 +240,8 @@ public function prepareInputForAdd($input)
237240

238241
public function prepareInputForUpdate($input)
239242
{
240-
// Cannot change type or name of existing field
241-
unset($input['type'], $input['name']);
243+
// Cannot change type or system_name of existing field
244+
unset($input['type'], $input['system_name']);
242245
$input = $this->prepareInputForAddAndUpdate($input);
243246
if ($input === false) {
244247
return false;

0 commit comments

Comments
 (0)