From af43a9cf1dcece0d7f890f636d33b11ece4a4230 Mon Sep 17 00:00:00 2001 From: Thierry Bugier Date: Tue, 10 Oct 2023 16:02:52 +0200 Subject: [PATCH 1/6] fix(dropdownfield): display of username in Formcreator 2.4.2, user was displayed with first and last name --- inc/field/dropdownfield.class.php | 12 +++++++++--- tests/3-unit/PluginFormcreatorForm.php | 2 +- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/inc/field/dropdownfield.class.php b/inc/field/dropdownfield.class.php index d93229f3c..5fb5f92cd 100644 --- a/inc/field/dropdownfield.class.php +++ b/inc/field/dropdownfield.class.php @@ -357,11 +357,17 @@ public function getRenderedHtml($domain, $canEdit = true): string { $item = new $itemtype(); $value = ''; if ($item->getFromDB($this->value)) { - $column = 'name'; + $value = $item->fields['name']; if ($item instanceof CommonTreeDropdown) { - $column = 'completename'; + $value = $item->fields['completename']; + } else { + /** @var CommonDBTM $item */ + switch ($item->getType()) { + case User::class: + $value = (new DbUtils())->getUserName($item->getID()); + break; + } } - $value = $item->fields[$column]; } return $value; diff --git a/tests/3-unit/PluginFormcreatorForm.php b/tests/3-unit/PluginFormcreatorForm.php index 6ecc8a538..9b14fae41 100644 --- a/tests/3-unit/PluginFormcreatorForm.php +++ b/tests/3-unit/PluginFormcreatorForm.php @@ -485,7 +485,7 @@ public function testCreateValidationNotification(User $requester, User $validato $form = $this->getForm([ 'name' => 'validation notification', - 'validation_required' => \PluginFormcreatorForm_Validator::VALIDATION_USER, + 'validation_required' => PluginFormcreatorForm_Validator::VALIDATION_USER, '_validator_users' => [$validator->getID()], ]); $this->getSection([ From c5e3077822dd0d76e6f05d1b39b52ed06fd1dcad Mon Sep 17 00:00:00 2001 From: Thierry Bugier Date: Tue, 17 Oct 2023 09:18:47 +0200 Subject: [PATCH 2/6] test(glpiselectfield): check non editable rendering with user type --- .../Formcreator/Field/GlpiSelectField.php | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/3-unit/GlpiPlugin/Formcreator/Field/GlpiSelectField.php b/tests/3-unit/GlpiPlugin/Formcreator/Field/GlpiSelectField.php index e402d2555..53b0ae073 100644 --- a/tests/3-unit/GlpiPlugin/Formcreator/Field/GlpiSelectField.php +++ b/tests/3-unit/GlpiPlugin/Formcreator/Field/GlpiSelectField.php @@ -33,6 +33,7 @@ namespace GlpiPlugin\Formcreator\Field\tests\units; use GlpiPlugin\Formcreator\Tests\CommonTestCase; use Computer; +use User; class GlpiselectField extends CommonTestCase { @@ -330,4 +331,43 @@ public function testGetValueForApi($input, $expected) { $output = $instance->getValueForApi(); $this->array($output)->isEqualTo($expected); } + + + public function providerGetRenderedHtml() { + $question = $this->getQuestion([ + 'fieldtype' => 'glpiselect', + 'itemtype' => User::class, + ]); + $field = $question->getSubField(); + + yield [ + 'field' => $field, + 'value' => User::getIdByName('glpi'), + 'expectFunction' => function () use ($field) { + $this->string($field->getRenderedHtml('', false))->isEqualTo('glpi'); + }, + ]; + + $login = $this->getUniqueString(); + $this->getGlpiCoreItem(User::class, [ + 'name' => $login, + 'firstname' => 'Alan', + 'realname' => 'Turing' + ]); + yield [ + 'field' => $field, + 'value' => User::getIdByName($login), + 'expectFunction' => function () use ($field) { + $this->string($field->getRenderedHtml('', false))->isEqualTo('Turing Alan'); + }, + ]; + } + + /** + * @dataProvider providerGetRenderedHtml + */ + public function testGetRenderedHtml($field, $value, $expectFunction) { + $field->deserializeValue($value); + $expectFunction(); + } } From 63999d06863e0512c0c00d71e3e1e27304adf28a Mon Sep 17 00:00:00 2001 From: Thierry Bugier Date: Mon, 23 Oct 2023 14:57:22 +0200 Subject: [PATCH 3/6] feat(glpiselectfield): attach existing documents to targets --- inc/abstractitiltarget.class.php | 23 ++++++++ inc/field/dropdownfield.class.php | 12 +++- inc/formanswer.class.php | 2 +- inc/targetticket.class.php | 1 + .../Formcreator/Field/DropdownField.php | 58 ++++++++++--------- 5 files changed, 66 insertions(+), 30 deletions(-) diff --git a/inc/abstractitiltarget.class.php b/inc/abstractitiltarget.class.php index 86681fbd4..6d91ea76d 100644 --- a/inc/abstractitiltarget.class.php +++ b/inc/abstractitiltarget.class.php @@ -2375,6 +2375,29 @@ protected function prepareUploadsFromTextarea(array $data, PluginFormcreatorForm return $data; } + /** + * Undocumented function + * + * @param array $data + * @param PluginFormcreatorFormAnswer $formanswer + * @return array + */ + protected function setDocuments($data, PluginFormcreatorFormAnswer $formanswer): array { + foreach ($formanswer->getQuestionFields($formanswer->getForm()->getID()) ?? [] as $field) { + $question = $field->getQuestion(); + if ($question->fields['fieldtype'] !== 'glpiselect') { + continue; + } + if ($question->fields['itemtype'] !== Document::class) { + continue; + } + + $data['_documents_id'][] = $field->getRawValue(); + } + + return $data; + } + /** * Emulate file uploads for documents provided to file questions * diff --git a/inc/field/dropdownfield.class.php b/inc/field/dropdownfield.class.php index 5fb5f92cd..11400c597 100644 --- a/inc/field/dropdownfield.class.php +++ b/inc/field/dropdownfield.class.php @@ -60,7 +60,6 @@ use OLA; use QueryExpression; use QuerySubQuery; -use QueryUnion; use GlpiPlugin\Formcreator\Exception\ComparisonException; use Glpi\Application\View\TemplateRenderer; use Plugin; @@ -366,6 +365,10 @@ public function getRenderedHtml($domain, $canEdit = true): string { case User::class: $value = (new DbUtils())->getUserName($item->getID()); break; + case Document::class: + /** @var Document $item */ + $value = $item->getDownloadLink($this->form_answer); + break; } } } @@ -427,7 +430,12 @@ public function moveUploads() { } public function getDocumentsForTarget(): array { - return []; + $itemtype = $this->getSubItemtype(); + if ($itemtype !== Document::class) { + return []; + } + + return [$this->value]; // Array of a single document ID } public static function getName(): string { diff --git a/inc/formanswer.class.php b/inc/formanswer.class.php index eae0edaf4..a83b7a8a3 100644 --- a/inc/formanswer.class.php +++ b/inc/formanswer.class.php @@ -1720,7 +1720,7 @@ private function updateIssue() { /** * get all fields from a form * - * @param int $formId ID of the form where come the fileds to load + * @param int $formId ID of the form where come the fields to load * @return PluginFormcreatorAbstractField[] */ public function getQuestionFields($formId) : array { diff --git a/inc/targetticket.class.php b/inc/targetticket.class.php index fd2d92b35..865256c0c 100644 --- a/inc/targetticket.class.php +++ b/inc/targetticket.class.php @@ -893,6 +893,7 @@ public function save(PluginFormcreatorFormAnswer $formanswer): ?CommonDBTM { $data = $this->assignedGroups + $data; } + $data = $this->setDocuments($data, $formanswer); $data = $this->prepareUploadedFiles($data, $formanswer); $data = $this->appendFieldsData($data, $formanswer); diff --git a/tests/3-unit/GlpiPlugin/Formcreator/Field/DropdownField.php b/tests/3-unit/GlpiPlugin/Formcreator/Field/DropdownField.php index e165c6eea..762332208 100644 --- a/tests/3-unit/GlpiPlugin/Formcreator/Field/DropdownField.php +++ b/tests/3-unit/GlpiPlugin/Formcreator/Field/DropdownField.php @@ -29,7 +29,10 @@ * --------------------------------------------------------------------- */ namespace GlpiPlugin\Formcreator\Field\tests\units; + use GlpiPlugin\Formcreator\Tests\CommonTestCase; +use Computer; +use ITILCategory; use Location; class DropdownField extends CommonTestCase { public function beforeTestMethod($method) { @@ -52,14 +55,14 @@ public function providerPrepareQuestionInputForSave() { [ 'input' => [ 'name' => $name, - 'itemtype' => \Location::class, + 'itemtype' => Location::class, 'show_tree_depth' => '5', 'show_tree_root' => '0', 'selectable_tree_root' => '0', ], 'expected' => [ 'name' => $name, - 'itemtype' => \Location::class, + 'itemtype' => Location::class, 'values' => json_encode([ 'show_tree_depth' => '5', 'show_tree_root' => '0', @@ -71,7 +74,7 @@ public function providerPrepareQuestionInputForSave() { [ 'input' => [ 'name' => $name, - 'itemtype' => \ITILCategory::class, + 'itemtype' => ITILCategory::class, 'show_ticket_categories' => '2', 'show_tree_depth' => '3', 'default_values' => '', @@ -112,7 +115,7 @@ public function testisPublicFormCompatible() { public function testIsPrerequisites() { $instance = $this->newTestedInstance($this->getQuestion([ - 'itemtype' => \Computer::class + 'itemtype' => Computer::class ])); $output = $instance->isPrerequisites(); $this->boolean($output)->isEqualTo(true); @@ -133,12 +136,13 @@ public function testGetValueForDesign() { } public function testGetDocumentsForTarget() { - $instance = $this->newTestedInstance(new \PluginFormcreatorQuestion()); + $question = $this->getQuestion(); + $instance = $question->getSubField(); $this->array($instance->getDocumentsForTarget())->hasSize(0); } public function providerIsValid() { - $location = new \Location(); + $location = new Location(); $locationId = $location->import([ 'completename' => 'foo', 'entities_id' => $_SESSION['glpiactive_entity'] @@ -148,13 +152,13 @@ public function providerIsValid() { [ 'question' => $this->getQuestion([ 'name' => 'fieldname', - 'itemtype' => \Location::class, + 'itemtype' => Location::class, 'values' => '', 'required' => '0', 'default_values' => '0', ]), 'input' => [ - 'dropdown_values' => \Location::class, + 'dropdown_values' => Location::class, 'dropdown_default_value' => '0', 'show_tree_depth' => '5', 'show_tree_root' => '0', @@ -164,12 +168,12 @@ public function providerIsValid() { [ 'question' => $this->getQuestion([ 'name' => 'fieldname', - 'itemtype' => \Location::class, + 'itemtype' => Location::class, 'values' => '', 'required' => '1', ]), 'input' => [ - 'dropdown_values' => \Location::class, + 'dropdown_values' => Location::class, 'dropdown_default_value' => '0', 'show_tree_depth' => '5', 'show_tree_root' => '0', @@ -179,13 +183,13 @@ public function providerIsValid() { [ 'question' => $this->getQuestion([ 'name' => 'fieldname', - 'itemtype' => \Location::class, + 'itemtype' => Location::class, 'values' => '', 'required' => '1', 'default_values' => '', ]), 'input' => [ - 'dropdown_values' => \Location::class, + 'dropdown_values' => Location::class, 'dropdown_default_value' => '42', 'show_tree_depth' => '5', 'show_tree_root' => '0', @@ -195,13 +199,13 @@ public function providerIsValid() { [ 'question' => $this->getQuestion([ 'name' => 'fieldname', - 'itemtype' => \Location::class, + 'itemtype' => Location::class, 'values' => '', 'required' => '1', 'default_values' => $locationId, ]), 'input' => [ - 'dropdown_values' => \Location::class, + 'dropdown_values' => Location::class, 'dropdown_default_value' => '42', 'show_tree_depth' => '5', 'show_tree_root' => '0', @@ -222,7 +226,7 @@ public function testIsValid($question, $input, $expectedValidity) { } public function providerGetValueForTargetText() { - $location = new \Location(); + $location = new Location(); $location->add([ 'name' => $this->getUniqueString(), ]); @@ -230,10 +234,10 @@ public function providerGetValueForTargetText() { [ 'fields' => $this->getQuestion([ 'name' => 'fieldname', - 'itemtype' => \Location::class, + 'itemtype' => Location::class, 'values' => '', 'required' => '1', - 'dropdown_values' => \Location::class, + 'dropdown_values' => Location::class, 'dropdown_default_value' => '42', ]), 'value' => "", @@ -242,10 +246,10 @@ public function providerGetValueForTargetText() { [ 'fields' => $this->getQuestion([ 'name' => 'fieldname', - 'itemtype' =>\Location::class, + 'itemtype' =>Location::class, 'values' =>'', 'required' => '1', - 'dropdown_values' => \Location::class, + 'dropdown_values' => Location::class, 'dropdown_default_value' => '', ]), 'value' => $location->getID(), @@ -266,8 +270,8 @@ public function testGetValueForTargetText($fields, $value, $expected) { } public function providerEquals() { - $location1 = new \Location(); - $location2 = new \Location(); + $location1 = new Location(); + $location2 = new Location(); $location1Id = $location1->add([ 'name' => $this->getUniqueString() ]); @@ -278,7 +282,7 @@ public function providerEquals() { return [ [ 'fields' => $this->getQuestion([ - 'itemtype' => \Location::class, + 'itemtype' => Location::class, ]), 'value' => $location1->fields['completename'], 'answer' => (string) $location1Id, @@ -286,7 +290,7 @@ public function providerEquals() { ], [ 'fields' => $this->getQuestion([ - 'itemtype' => \Location::class, + 'itemtype' => Location::class, ]), 'value' => $location2->fields['completename'], 'answer' => (string) $location1Id, @@ -305,8 +309,8 @@ public function testEquals($fields, $value, $answer, $expected) { } public function providerNotEquals() { - $location1 = new \Location(); - $location2 = new \Location(); + $location1 = new Location(); + $location2 = new Location(); $location1Id = $location1->add([ 'name' => $this->getUniqueString() ]); @@ -317,7 +321,7 @@ public function providerNotEquals() { return [ [ 'fields' => $this->getQuestion([ - 'itemtype' => \Location::class, + 'itemtype' => Location::class, ]), 'value' => $location1->fields['completename'], 'answer' => (string) $location1Id, @@ -325,7 +329,7 @@ public function providerNotEquals() { ], [ 'fields' => $this->getQuestion([ - 'itemtype' => \Location::class, + 'itemtype' => Location::class, ]), 'value' => $location2->fields['completename'], 'answer' => (string) $location1Id, From 4da93e38e785365af884f9fbd2f7c8bd9ae05a22 Mon Sep 17 00:00:00 2001 From: Thierry Bugier Date: Mon, 30 Oct 2023 14:02:27 +0100 Subject: [PATCH 4/6] fix(glpiselectfield,dropdownfield): prevent php warning --- inc/field/dropdownfield.class.php | 6 +++--- inc/field/glpiselectfield.class.php | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/inc/field/dropdownfield.class.php b/inc/field/dropdownfield.class.php index 11400c597..d725b0d9d 100644 --- a/inc/field/dropdownfield.class.php +++ b/inc/field/dropdownfield.class.php @@ -602,7 +602,7 @@ private function getMyGroups($userID) { } public function equals($value): bool { - $value = html_entity_decode($value); + $value = html_entity_decode($value ?? ''); $itemtype = $this->question->fields['itemtype']; $dropdown = new $itemtype(); if ($dropdown->isNewId($this->value)) { @@ -624,7 +624,7 @@ public function notEquals($value): bool { } public function greaterThan($value): bool { - $value = html_entity_decode($value); + $value = html_entity_decode($value ?? ''); $itemtype = $this->question->fields['itemtype']; $dropdown = new $itemtype(); if (!$dropdown->getFromDB($this->value)) { @@ -643,7 +643,7 @@ public function lessThan($value): bool { } public function regex($value): bool { - $value = html_entity_decode($value); + $value = html_entity_decode($value ?? ''); $itemtype = $this->question->fields['itemtype']; $dropdown = new $itemtype(); if (!$dropdown->getFromDB($this->value)) { diff --git a/inc/field/glpiselectfield.class.php b/inc/field/glpiselectfield.class.php index 8450f7e8d..71a1f67dd 100644 --- a/inc/field/glpiselectfield.class.php +++ b/inc/field/glpiselectfield.class.php @@ -160,7 +160,7 @@ public function buildParams($rand = null) { } public function equals($value): bool { - $value = html_entity_decode($value); + $value = html_entity_decode($value ?? ''); $itemtype = $this->getSubItemtype(); $item = new $itemtype(); if ($item->isNewId($this->value)) { @@ -177,7 +177,7 @@ public function notEquals($value): bool { } public function greaterThan($value): bool { - $value = html_entity_decode($value); + $value = html_entity_decode($value ?? ''); $itemtype = $this->getSubItemtype(); $item = new $itemtype(); if (!$item->getFromDB($this->value)) { From c5167c16cc2b36e0b16691c5d297be35ec7048b2 Mon Sep 17 00:00:00 2001 From: Thierry Bugier Date: Tue, 31 Oct 2023 08:45:57 +0100 Subject: [PATCH 5/6] fix(target_actor): actors ID not converted when duplicating a form --- inc/target_actor.class.php | 85 +++++++++++++++++++------------------- 1 file changed, 43 insertions(+), 42 deletions(-) diff --git a/inc/target_actor.class.php b/inc/target_actor.class.php index 7f3866712..69ad45170 100644 --- a/inc/target_actor.class.php +++ b/inc/target_actor.class.php @@ -172,51 +172,52 @@ public static function import(PluginFormcreatorLinker $linker, $input = [], $con 'uuid', $input['uuid'] ); - // Convert UUIDs or names into IDs - switch ($input['actor_type']) { - case self::ACTOR_TYPE_QUESTION_PERSON : - case self::ACTOR_TYPE_QUESTION_GROUP : - case self::ACTOR_TYPE_QUESTION_SUPPLIER : - case self::ACTOR_TYPE_QUESTION_ACTORS : - case self::ACTOR_TYPE_GROUP_FROM_OBJECT : - case self::ACTOR_TYPE_TECH_GROUP_FROM_OBJECT : - /** @var PluginFormcreatorQuestion $question */ - $question = $linker->getObject($input['actor_value'], PluginFormcreatorQuestion::class); - if ($question === false) { - $linker->postpone($input[$idKey], $item->getType(), $input, $containerId); - return false; - } - $input['actor_value'] = $question->getID(); - break; + } - case self::ACTOR_TYPE_PERSON: - case self::ACTOR_TYPE_AUTHORS_SUPERVISOR: - $user = new User; - $users_id = plugin_formcreator_getFromDBByField($user, 'name', $input['actor_value']); - if ($users_id === false) { - throw new ImportFailureException(sprintf(__('Failed to find a user: %1$s'), $input['actor_value'])); - } - $input['actor_value'] = $users_id; - break; + // Convert UUIDs or names into IDs + switch ($input['actor_type']) { + case self::ACTOR_TYPE_QUESTION_PERSON : + case self::ACTOR_TYPE_QUESTION_GROUP : + case self::ACTOR_TYPE_QUESTION_SUPPLIER : + case self::ACTOR_TYPE_QUESTION_ACTORS : + case self::ACTOR_TYPE_GROUP_FROM_OBJECT : + case self::ACTOR_TYPE_TECH_GROUP_FROM_OBJECT : + /** @var PluginFormcreatorQuestion $question */ + $question = $linker->getObject($input['actor_value'], PluginFormcreatorQuestion::class); + if ($question === false) { + $linker->postpone($input[$idKey], $item->getType(), $input, $containerId); + return false; + } + $input['actor_value'] = $question->getID(); + break; - case self::ACTOR_TYPE_GROUP: - $group = new Group; - $groups_id = plugin_formcreator_getFromDBByField($group, 'completename', $input['actor_value']); - if ($groups_id === false) { - throw new ImportFailureException(sprintf(__('Failed to find a group: %1$s'), $input['actor_value'])); - } - $input['actor_value'] = $groups_id; - break; + case self::ACTOR_TYPE_PERSON: + case self::ACTOR_TYPE_AUTHORS_SUPERVISOR: + $user = new User; + $users_id = plugin_formcreator_getFromDBByField($user, 'name', $input['actor_value']); + if ($users_id === false) { + throw new ImportFailureException(sprintf(__('Failed to find a user: %1$s'), $input['actor_value'])); + } + $input['actor_value'] = $users_id; + break; - case self::ACTOR_TYPE_SUPPLIER: - $supplier = new Supplier; - $suppliers_id = plugin_formcreator_getFromDBByField($supplier, 'name', $input['actor_value']); - if ($suppliers_id === false) { - throw new ImportFailureException(sprintf(__('Failed to find a supplier: %1$s'), $input['actor_value'])); - } - $input['actor_value'] = $suppliers_id; - break; - } + case self::ACTOR_TYPE_GROUP: + $group = new Group; + $groups_id = plugin_formcreator_getFromDBByField($group, 'completename', $input['actor_value']); + if ($groups_id === false) { + throw new ImportFailureException(sprintf(__('Failed to find a group: %1$s'), $input['actor_value'])); + } + $input['actor_value'] = $groups_id; + break; + + case self::ACTOR_TYPE_SUPPLIER: + $supplier = new Supplier; + $suppliers_id = plugin_formcreator_getFromDBByField($supplier, 'name', $input['actor_value']); + if ($suppliers_id === false) { + throw new ImportFailureException(sprintf(__('Failed to find a supplier: %1$s'), $input['actor_value'])); + } + $input['actor_value'] = $suppliers_id; + break; } $originalId = $input[$idKey]; From 46559a0a167d4e512a3bd2bf0f3c1fd4e68c4735 Mon Sep 17 00:00:00 2001 From: Thierry Bugier Date: Tue, 31 Oct 2023 11:00:56 +0100 Subject: [PATCH 6/6] docs: type hint for IDE --- inc/exportabletrait.class.php | 1 + 1 file changed, 1 insertion(+) diff --git a/inc/exportabletrait.class.php b/inc/exportabletrait.class.php index a7280e62a..2fcbbba23 100644 --- a/inc/exportabletrait.class.php +++ b/inc/exportabletrait.class.php @@ -111,6 +111,7 @@ public function importChildrenObjects(PluginFormcreatorExportableInterface $pare } } // Delete all other restrictions + /** @var PluginFormcreatorExportableInterface $subItem */ $subItem = new $itemtype(); $subItem->deleteObsoleteItems($parent, $importedItems); }