fix(forms): Display translations already defined even when the original value is empty

src/Entity.php

@@ -3385,10 +3385,7 @@ public function listTranslationsHandlers(): array
         $handlers = [];
         $key = sprintf('%s_%d', self::getType(), $this->getID());
         $category_name = sprintf('%s: %s', self::getTypeName(), $this->getName());
-        if (
-            !empty($this->fields['custom_helpdesk_home_title'])
-            && $this->fields['custom_helpdesk_home_title'] != self::CONFIG_PARENT
-        ) {
+        if ($this->fields['custom_helpdesk_home_title'] != self::CONFIG_PARENT) {
             $handlers[$key][] = new TranslationHandler(
                 item: $this,
                 key: self::TRANSLATION_KEY_CUSTOM_HELPDESK_HOME_TITLE,

src/Glpi/Form/Comment.php

@@ -179,27 +179,23 @@ public function listTranslationsHandlers(): array
         $category_name = sprintf('%s: %s', self::getTypeName(), $this->getName());
         $handlers = [];
 
-        if (!empty($this->fields['name'])) {
-            $handlers[$key][] = new TranslationHandler(
-                item: $this,
-                key: self::TRANSLATION_KEY_NAME,
-                name: __('Comment title'),
-                value: $this->fields['name'],
-                is_rich_text: false,
-                category: $category_name
-            );
-        }
+        $handlers[$key][] = new TranslationHandler(
+            item: $this,
+            key: self::TRANSLATION_KEY_NAME,
+            name: __('Comment title'),
+            value: $this->fields['name'],
+            is_rich_text: false,
+            category: $category_name
+        );
 
-        if (!empty($this->fields['description'])) {
-            $handlers[$key][] = new TranslationHandler(
-                item: $this,
-                key: self::TRANSLATION_KEY_DESCRIPTION,
-                name: __('Comment description'),
-                value: $this->fields['description'],
-                is_rich_text: true,
-                category: $category_name
-            );
-        }
+        $handlers[$key][] = new TranslationHandler(
+            item: $this,
+            key: self::TRANSLATION_KEY_DESCRIPTION,
+            name: __('Comment description'),
+            value: $this->fields['description'],
+            is_rich_text: true,
+            category: $category_name
+        );
 
         return $handlers;
     }

src/Glpi/Form/Form.php

@@ -451,38 +451,32 @@ public function listTranslationsHandlers(): array
         $key = sprintf('%s_%d', self::getType(), $this->getID());
         $category_name = __('Form properties');
         $handlers = [];
-        if (!empty($this->fields['name'])) {
-            $handlers[$key][] = new TranslationHandler(
-                item: $this,
-                key: self::TRANSLATION_KEY_NAME,
-                name: __('Form title'),
-                value: $this->fields['name'],
-                is_rich_text: false,
-                category: $category_name
-            );
-        }
+        $handlers[$key][] = new TranslationHandler(
+            item: $this,
+            key: self::TRANSLATION_KEY_NAME,
+            name: __('Form title'),
+            value: $this->fields['name'],
+            is_rich_text: false,
+            category: $category_name
+        );
 
-        if (!empty($this->fields['header'])) {
-            $handlers[$key][] = new TranslationHandler(
-                item: $this,
-                key: self::TRANSLATION_KEY_HEADER,
-                name: __('Form description'),
-                value: $this->fields['header'],
-                is_rich_text: true,
-                category: $category_name
-            );
-        }
+        $handlers[$key][] = new TranslationHandler(
+            item: $this,
+            key: self::TRANSLATION_KEY_HEADER,
+            name: __('Form description'),
+            value: $this->fields['header'],
+            is_rich_text: true,
+            category: $category_name
+        );
 
-        if (!empty($this->fields['description'])) {
-            $handlers[$key][] = new TranslationHandler(
-                item: $this,
-                key: self::TRANSLATION_KEY_DESCRIPTION,
-                name: __('Service catalog description'),
-                value: $this->fields['description'],
-                is_rich_text: true,
-                category: $category_name
-            );
-        }
+        $handlers[$key][] = new TranslationHandler(
+            item: $this,
+            key: self::TRANSLATION_KEY_DESCRIPTION,
+            name: __('Service catalog description'),
+            value: $this->fields['description'],
+            is_rich_text: true,
+            category: $category_name
+        );
 
         $sections_handlers = array_map(
             fn($section) => $section->listTranslationsHandlers(),

src/Glpi/Form/Migration/FormMigration.php

@@ -34,6 +34,7 @@
 
 namespace Glpi\Form\Migration;
 
+use AbstractRightsDropdown;
 use DBmysql;
 use DBmysqlIterator;
 use Entity;
@@ -90,6 +91,12 @@ class FormMigration extends AbstractPluginMigration
      */
     private array $forms = [];
 
+    /**
+     * Store the original form raw data indexed by plugin form ID for quick access
+     * @var array<int, array>
+     */
+    private array $formcreator_raw_forms = [];
+
     public function __construct(
         DBmysql $db,
         FormAccessControlManager $formAccessControlManager,
@@ -131,11 +138,13 @@ public function getTypesConvertMap(): array
      *
      * @return array Mapping between access type constants and strategy classes
      */
-    private function getStrategyForAccessTypes(): array
+    private function getStrategyForAccessTypes(array $formcreator_raw_form): array
     {
         return [
             self::PUBLIC_ACCESS_TYPE => DirectAccess::class,
-            self::PRIVATE_ACCESS_TYPE => DirectAccess::class,
+            self::PRIVATE_ACCESS_TYPE => $formcreator_raw_form['is_visible']
+                ? AllowList::class
+                : DirectAccess::class,
             self::RESTRICTED_ACCESS_TYPE => AllowList::class,
         ];
     }
@@ -223,23 +232,42 @@ private function getLogicOperatorFromLegacy(int $logic_operator): LogicOperator
      * @return JsonFieldInterface The configuration object for the access control strategy
      * @throws LogicException When no strategy config is found for the given access type
      */
-    private function getStrategyConfigForAccessTypes(array $form_access_rights): JsonFieldInterface
-    {
+    private function getStrategyConfigForAccessTypes(
+        array $form_access_rights,
+        array $formcreator_raw_form,
+    ): JsonFieldInterface {
         $clean_ids = static fn(array $ids) => array_unique(array_filter($ids, fn(mixed $id) => is_int($id)));
 
-        if (in_array($form_access_rights['access_rights'], [self::PUBLIC_ACCESS_TYPE, self::PRIVATE_ACCESS_TYPE])) {
-            return new DirectAccessConfig(
-                allow_unauthenticated: $form_access_rights['access_rights'] === self::PUBLIC_ACCESS_TYPE
-            );
-        } elseif ($form_access_rights['access_rights'] === self::RESTRICTED_ACCESS_TYPE) {
-            return new AllowListConfig(
-                user_ids: $clean_ids(json_decode($form_access_rights['user_ids'], associative: true, flags: JSON_THROW_ON_ERROR)),
-                group_ids: $clean_ids(json_decode($form_access_rights['group_ids'], associative: true, flags: JSON_THROW_ON_ERROR)),
-                profile_ids: $clean_ids(json_decode($form_access_rights['profile_ids'], associative: true, flags: JSON_THROW_ON_ERROR))
-            );
-        }
-
-        throw new LogicException("Strategy config not found for access type {$form_access_rights['access_rights']}");
+        return match ($form_access_rights['access_rights']) {
+            self::PUBLIC_ACCESS_TYPE => new DirectAccessConfig(
+                allow_unauthenticated: true,
+            ),
+            self::PRIVATE_ACCESS_TYPE => $formcreator_raw_form['is_visible']
+                ? new AllowListConfig(
+                    user_ids: [AbstractRightsDropdown::ALL_USERS],
+                )
+                : new DirectAccessConfig(
+                    allow_unauthenticated: false,
+                ),
+            self::RESTRICTED_ACCESS_TYPE => new AllowListConfig(
+                user_ids: $clean_ids(json_decode(
+                    $form_access_rights['user_ids'],
+                    associative: true,
+                    flags: JSON_THROW_ON_ERROR
+                )),
+                group_ids: $clean_ids(json_decode(
+                    $form_access_rights['group_ids'],
+                    associative: true,
+                    flags: JSON_THROW_ON_ERROR
+                )),
+                profile_ids: $clean_ids(json_decode(
+                    $form_access_rights['profile_ids'],
+                    associative: true,
+                    flags: JSON_THROW_ON_ERROR
+                )),
+            ),
+            default => throw new LogicException("Strategy config not found for access type {$form_access_rights['access_rights']}"),
+        };
     }
 
     private function getFormWhereCriteria(): array
@@ -433,6 +461,7 @@ private function processMigrationOfBasicProperties(): void
                 'is_recursive',
                 'is_active',
                 'is_deleted',
+                'is_visible',
             ],
             'FROM'   => 'glpi_plugin_formcreator_forms',
             'WHERE'  => $this->getFormWhereCriteria(),
@@ -468,6 +497,7 @@ private function processMigrationOfBasicProperties(): void
 
             // Store the form for later use
             $this->forms[$raw_form['id']] = $form;
+            $this->formcreator_raw_forms[$raw_form['id']] = $raw_form;
 
             $this->mapItem(
                 'PluginFormcreatorForm',
@@ -877,12 +907,13 @@ private function processMigrationOfAccessControls(): void
         foreach ($raw_form_access_rights as $form_access_rights) {
             // Use the stored form instead of loading it from the database
             $form = $this->forms[$form_access_rights['forms_id']] ?? null;
+            $formcreator_raw_form = $this->formcreator_raw_forms[$form_access_rights['forms_id']] ?? null;
 
-            if ($form === null) {
+            if ($form === null || $formcreator_raw_form === null) {
                 throw new LogicException("Form with plugin_id {$form_access_rights['forms_id']} not found in memory");
             }
 
-            $strategy_class = $this->getStrategyForAccessTypes()[$form_access_rights['access_rights']] ?? null;
+            $strategy_class = $this->getStrategyForAccessTypes($formcreator_raw_form)[$form_access_rights['access_rights']] ?? null;
             if ($strategy_class === null) {
                 throw new LogicException("Strategy class not found for access type {$form_access_rights['access_rights']}");
             }
@@ -895,7 +926,7 @@ private function processMigrationOfAccessControls(): void
                 [
                     Form::getForeignKeyField() => $form->getID(),
                     'strategy'                 => $strategy_class,
-                    '_config'                  => self::getStrategyConfigForAccessTypes($form_access_rights)->jsonSerialize(),
+                    '_config'                  => self::getStrategyConfigForAccessTypes($form_access_rights, $formcreator_raw_form)->jsonSerialize(),
                     'is_active'                => true,
                 ],
                 [
@@ -1246,7 +1277,7 @@ private function processMigrationOfTranslations(): void
 
             foreach ($form->listTranslationsHandlers() as $handlers) {
                 foreach ($handlers as $handler) {
-                    if (isset($translations[$handler->getValue()])) {
+                    if (!empty($handler->getValue()) && isset($translations[$handler->getValue()])) {
                         $this->importItem(
                             FormTranslation::class,
                             [
@@ -1377,7 +1408,10 @@ private function processMigrationOfVisibilityConditions(): void
                 continue;
             }
 
-            $value = $raw_condition['show_value'];
+            // The value can be null for formcreator, make sure to fallback to
+            // an empty string
+            $value = $raw_condition['show_value'] ?? "";
+
             $value_operator = $this->getValueOperatorFromLegacy(
                 $raw_condition['show_condition'],
                 $value,

src/Glpi/Form/Question.php

@@ -135,27 +135,23 @@ public function listTranslationsHandlers(): array
         $key = sprintf('%s_%d', self::getType(), $this->getID());
         $category_name = sprintf('%s: %s', self::getTypeName(), $this->getName());
         $handlers = [];
-        if (!empty($this->fields['name'])) {
-            $handlers[$key][] = new TranslationHandler(
-                item: $this,
-                key: self::TRANSLATION_KEY_NAME,
-                name: __('Question name'),
-                value: $this->fields['name'],
-                is_rich_text: false,
-                category: $category_name
-            );
-        }
+        $handlers[$key][] = new TranslationHandler(
+            item: $this,
+            key: self::TRANSLATION_KEY_NAME,
+            name: __('Question name'),
+            value: $this->fields['name'],
+            is_rich_text: false,
+            category: $category_name
+        );
 
-        if (!empty($this->fields['description'])) {
-            $handlers[$key][] = new TranslationHandler(
-                item: $this,
-                key: self::TRANSLATION_KEY_DESCRIPTION,
-                name: __('Question description'),
-                value: $this->fields['description'],
-                is_rich_text: true,
-                category: $category_name
-            );
-        }
+        $handlers[$key][] = new TranslationHandler(
+            item: $this,
+            key: self::TRANSLATION_KEY_DESCRIPTION,
+            name: __('Question description'),
+            value: $this->fields['description'],
+            is_rich_text: true,
+            category: $category_name
+        );
 
         $question_type = $this->getQuestionType();
         if ($question_type instanceof TranslationAwareQuestionType) {

src/Glpi/Form/QuestionType/AbstractQuestionTypeSelectable.php

@@ -243,20 +243,17 @@ public function convertExtraData(array $rawData): array
     #[Override]
     public function listTranslationsHandlers(Question $question): array
     {
-        $handlers = [];
         $options = $this->getOptions($question);
-        if ($options !== []) {
-            $handlers = array_map(
-                fn($uuid, $option) => new TranslationHandler(
-                    item: $question,
-                    key: sprintf('%s-%s', self::TRANSLATION_KEY_OPTION, $uuid),
-                    name: sprintf('%s %s', self::getName(), __('Option')),
-                    value: $option,
-                ),
-                array_keys($options),
-                $options
-            );
-        }
+        $handlers = array_map(
+            fn($uuid, $option) => new TranslationHandler(
+                item: $question,
+                key: sprintf('%s-%s', self::TRANSLATION_KEY_OPTION, $uuid),
+                name: sprintf('%s %s', self::getName(), __('Option')),
+                value: $option,
+            ),
+            array_keys($options),
+            $options
+        );
 
         return $handlers;
     }

src/Glpi/Form/QuestionType/QuestionTypeLongText.php

@@ -227,15 +227,13 @@ public function listTranslationsHandlers(Question $question): array
     {
         $handlers = [];
 
-        if (!empty($question->fields['default_value'])) {
-            $handlers[] = new TranslationHandler(
-                item: $question,
-                key: Question::TRANSLATION_KEY_DEFAULT_VALUE,
-                name: __('Default value'),
-                value: $question->fields['default_value'],
-                is_rich_text: true
-            );
-        }
+        $handlers[] = new TranslationHandler(
+            item: $question,
+            key: Question::TRANSLATION_KEY_DEFAULT_VALUE,
+            name: __('Default value'),
+            value: $question->fields['default_value'],
+            is_rich_text: true
+        );
 
         return $handlers;
     }

src/Glpi/Form/QuestionType/QuestionTypeShortText.php

@@ -80,17 +80,12 @@ public function getConditionHandlers(
     #[Override]
     public function listTranslationsHandlers(Question $question): array
     {
-        $default_value = $question->fields['default_value'] ?? '';
-        if (empty($default_value)) {
-            return [];
-        }
-
         return [
             new TranslationHandler(
                 item: $question,
                 key: Question::TRANSLATION_KEY_DEFAULT_VALUE,
                 name: __('Default value'),
-                value: $default_value,
+                value: $question->fields['default_value'] ?? '',
             ),
         ];
     }