From 124187a2b13b901cd046981bea37a14066b98f50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Cailly?= Date: Fri, 31 Oct 2025 16:01:57 +0100 Subject: [PATCH 1/4] fix(forms): Display translations already defined even when the original value is empty --- src/Entity.php | 5 +- src/Glpi/Form/Comment.php | 36 ++++++------ src/Glpi/Form/Form.php | 54 ++++++++---------- src/Glpi/Form/Question.php | 36 ++++++------ .../AbstractQuestionTypeSelectable.php | 23 ++++---- .../QuestionType/QuestionTypeLongText.php | 16 +++--- .../QuestionType/QuestionTypeShortText.php | 7 +-- src/Glpi/Form/Section.php | 36 ++++++------ .../Context/TranslationHandler.php | 8 +-- src/Glpi/ItemTranslation/ItemTranslation.php | 17 +++++- .../admin/form/form_translation.html.twig | 39 ++++++++----- tests/cypress/e2e/form/form_translation.cy.js | 55 +++++++++++++++++++ .../Form/Translation/FormTranslationTest.php | 35 +++++++++--- 13 files changed, 216 insertions(+), 151 deletions(-) diff --git a/src/Entity.php b/src/Entity.php index 20deb3402bc..534e5e881c4 100644 --- a/src/Entity.php +++ b/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, diff --git a/src/Glpi/Form/Comment.php b/src/Glpi/Form/Comment.php index 204e64c13df..41dec2c1977 100644 --- a/src/Glpi/Form/Comment.php +++ b/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; } diff --git a/src/Glpi/Form/Form.php b/src/Glpi/Form/Form.php index 1ee1b0c0464..ec1ca3edd42 100644 --- a/src/Glpi/Form/Form.php +++ b/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(), diff --git a/src/Glpi/Form/Question.php b/src/Glpi/Form/Question.php index 4c918104edd..200ca7664a5 100644 --- a/src/Glpi/Form/Question.php +++ b/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) { diff --git a/src/Glpi/Form/QuestionType/AbstractQuestionTypeSelectable.php b/src/Glpi/Form/QuestionType/AbstractQuestionTypeSelectable.php index 8e54b03e63e..27e10a86dab 100644 --- a/src/Glpi/Form/QuestionType/AbstractQuestionTypeSelectable.php +++ b/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; } diff --git a/src/Glpi/Form/QuestionType/QuestionTypeLongText.php b/src/Glpi/Form/QuestionType/QuestionTypeLongText.php index 0c169eed97b..95d62278cd4 100644 --- a/src/Glpi/Form/QuestionType/QuestionTypeLongText.php +++ b/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; } diff --git a/src/Glpi/Form/QuestionType/QuestionTypeShortText.php b/src/Glpi/Form/QuestionType/QuestionTypeShortText.php index d32511cac89..96ddfc5c9d7 100644 --- a/src/Glpi/Form/QuestionType/QuestionTypeShortText.php +++ b/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'] ?? '', ), ]; } diff --git a/src/Glpi/Form/Section.php b/src/Glpi/Form/Section.php index 8b9b2ee3d47..f3047e449ae 100644 --- a/src/Glpi/Form/Section.php +++ b/src/Glpi/Form/Section.php @@ -159,27 +159,23 @@ public function listTranslationsHandlers(): array $key = sprintf('%s_%d', self::getType(), $this->getID()); $category_name = sprintf('%s: %s', self::getTypeName(), $this->getName()); if (count($form->getSections()) > 1) { - if (!empty($this->fields['name'])) { - $handlers[$key][] = new TranslationHandler( - item: $this, - key: self::TRANSLATION_KEY_NAME, - name: __('Section title'), - value: $this->fields['name'], - is_rich_text: false, - category: $category_name - ); - } + $handlers[$key][] = new TranslationHandler( + item: $this, + key: self::TRANSLATION_KEY_NAME, + name: __('Section 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: __('Section description'), - value: $this->fields['description'], - is_rich_text: true, - category: $category_name - ); - } + $handlers[$key][] = new TranslationHandler( + item: $this, + key: self::TRANSLATION_KEY_DESCRIPTION, + name: __('Section description'), + value: $this->fields['description'], + is_rich_text: true, + category: $category_name + ); } $blocks_handlers = []; diff --git a/src/Glpi/ItemTranslation/Context/TranslationHandler.php b/src/Glpi/ItemTranslation/Context/TranslationHandler.php index 472144a58b2..9fa710324c0 100644 --- a/src/Glpi/ItemTranslation/Context/TranslationHandler.php +++ b/src/Glpi/ItemTranslation/Context/TranslationHandler.php @@ -52,7 +52,7 @@ final class TranslationHandler private string $name; /** @var string The default value (in the default language) */ - private string $value; + private ?string $value; /** @var bool Whether this field contains rich text that should be edited in a rich text editor */ private bool $is_rich_text; @@ -72,7 +72,7 @@ public function __construct( CommonDBTM $item, string $key, string $name, - string $value, + ?string $value, bool $is_rich_text = false, ?string $category = null ) { @@ -117,9 +117,9 @@ public function getName(): string /** * Get the default value (in the default language) * - * @return string + * @return string|null */ - public function getValue(): string + public function getValue(): ?string { return $this->value; } diff --git a/src/Glpi/ItemTranslation/ItemTranslation.php b/src/Glpi/ItemTranslation/ItemTranslation.php index 4b14abd18b7..932cb5f0924 100644 --- a/src/Glpi/ItemTranslation/ItemTranslation.php +++ b/src/Glpi/ItemTranslation/ItemTranslation.php @@ -39,10 +39,12 @@ use Gettext\Languages\Language; use Glpi\Form\FormTranslation; use Glpi\ItemTranslation\Context\ProvideTranslationsInterface; +use Glpi\ItemTranslation\Context\TranslationHandler; use LogicException; use Override; use Session; +use function Safe\array_walk_recursive; use function Safe\json_decode; use function Safe\json_encode; @@ -215,7 +217,18 @@ protected function getTranslationsHandlersForStats(): array throw new LogicException('Item does not provide translations'); } - return $item->listTranslationsHandlers(); + // Filter out handlers with empty values and those that do not have a translation yet + return array_map( + fn(array $handlers) => array_filter( + $handlers, + fn(TranslationHandler $handler) => !empty($handler->getValue()) || !empty(self::getForItemKeyAndLanguage( + $item, + $handler->getKey(), + $this->fields['language'] + )?->getTranslation()) + ), + $item->listTranslationsHandlers() + ); } public function getTranslatedPercentage(): int @@ -278,7 +291,7 @@ function ($handler) use (&$translations_to_review) { static::$itemtype => $handler->getItem()->getType(), 'language' => $this->fields['language'], 'key' => $handler->getKey(), - 'hash' => ['!=', md5($handler->getValue())], + 'hash' => ['!=', md5($handler->getValue() ?? '')], ]) ) { if ($translation->isPossiblyObsolete()) { diff --git a/templates/pages/admin/form/form_translation.html.twig b/templates/pages/admin/form/form_translation.html.twig index e2d7469ed2a..f5b262ad227 100644 --- a/templates/pages/admin/form/form_translation.html.twig +++ b/templates/pages/admin/form/form_translation.html.twig @@ -93,21 +93,30 @@ 'type_aria_label': _n('Category', 'Categories', 1) }]) %} {% for handler in handlers %} - {% set entries = entries|merge([{ - 'type': '' ~ handler.getName() ~ '', - 'default': handler.isRichText() ? handler.getValue()|safe_html : handler.getValue()|e, - 'translation': _self.getTranslationInput( - handler, - form_translation.getForItemKeyAndLanguage( - handler.getItem(), - handler.getKey(), - form_translation.fields['language'] - ) - ), - 'type_aria_label': __('Translation name'), - 'default_aria_label': __('Default value'), - 'translation_aria_label': __('Translated value') - }]) %} + {% set item_translation = form_translation.getForItemKeyAndLanguage( + handler.getItem(), + handler.getKey(), + form_translation.fields['language'] + ) %} + {% if handler.getValue() is not empty or (item_translation is not null and item_translation.getTranslation() is not empty) %} + {% set default_value = handler.getValue() %} + {% if handler.getValue() is empty %} + {% set default_value = '' ~ __('No default value') ~ '' %} + {% else %} + {% set default_value = handler.isRichText() ? default_value|safe_html : default_value|e %} + {% endif %} + {% set entries = entries|merge([{ + 'type': '' ~ handler.getName() ~ '', + 'default': default_value, + 'translation': _self.getTranslationInput( + handler, + item_translation + ), + 'type_aria_label': __('Translation name'), + 'default_aria_label': __('Default value'), + 'translation_aria_label': __('Translated value') + }]) %} + {% endif %} {% endfor %} {% endfor %} diff --git a/tests/cypress/e2e/form/form_translation.cy.js b/tests/cypress/e2e/form/form_translation.cy.js index 13b455744b6..80b1b341617 100644 --- a/tests/cypress/e2e/form/form_translation.cy.js +++ b/tests/cypress/e2e/form/form_translation.cy.js @@ -571,4 +571,59 @@ describe('Edit form translations', () => { // Check the translations for the question description cy.findByRole('note', { name: 'Question description' }).should('exist').contains('Ceci est une question avec une description'); }); + + it('translation remain visible when a translation has already been provided and the default value is emptied', () => { + addTranslations(); + + // Go to the form editor + cy.findByRole('tab', { name: 'Form' }).click(); + + // Empty the form description + cy.findAllByLabelText('Form description') + .awaitTinyMCE() + .clear(); + + // Save the form + cy.findByRole('button', { name: 'Save' }).click(); + cy.checkAndCloseAlert('Item successfully updated'); + + // Go to the form translations page + cy.findByRole('tab', { name: 'Form translations 1' }).click(); + + // Open modal + cy.findByRole('button', { name: 'Edit translation' }).click(); + cy.findByRole('dialog').should('have.attr', 'data-cy-shown', 'true'); + + // Check that the translation for form description is visible and marked as "to review" + cy.get('@formTranslationsTable').findAllByRole('row').as('formTranslationsRows'); + cy.get('@formTranslationsRows').eq(3).within(() => { + cy.findByRole('cell', { name: 'Translation name' }).contains('Form description'); + cy.findByRole('cell', { name: 'Default value' }).should('contain.text', 'No default value'); + cy.findByRole('cell', { name: 'Translated value' }) + .findByLabelText('Enter translation') + .parent().parent() + .within(() => { + cy.get('.ti-alert-circle').should('exist'); + }); + }); + + // Clear the translation value + cy.get('@formTranslationsRows').eq(3).within(() => { + cy.findByLabelText('Enter translation').awaitTinyMCE().clear(); + }); + + // Save the translations + cy.findByRole('button', { name: 'Save translation' }).click(); + cy.checkAndCloseAlert('Item successfully updated'); + + // Re-open modal + cy.findByRole('button', { name: 'Edit translation' }).click(); + cy.findByRole('dialog').should('have.attr', 'data-cy-shown', 'true'); + + // Check that the translation for form description is no longer visible + cy.get('@formTranslationsTable').findAllByRole('row').as('formTranslationsRows'); + cy.get('@formTranslationsRows').eq(-1).within(() => { + cy.findByRole('cell', { name: 'Translation name' }).contains('Form description').should('not.exist'); + }); + }); }); diff --git a/tests/functional/Glpi/Form/Translation/FormTranslationTest.php b/tests/functional/Glpi/Form/Translation/FormTranslationTest.php index bb2e109a5c7..3c84a604cfc 100644 --- a/tests/functional/Glpi/Form/Translation/FormTranslationTest.php +++ b/tests/functional/Glpi/Form/Translation/FormTranslationTest.php @@ -46,11 +46,11 @@ use Glpi\Form\QuestionType\QuestionTypeShortText; use Glpi\Form\QuestionType\QuestionTypesManager; use Glpi\Form\QuestionType\TranslationAwareQuestionType; -use Glpi\ItemTranslation\Context\TranslationHandler; use Glpi\Tests\FormBuilder; use Glpi\Tests\FormTesterTrait; use PHPUnit\Framework\Attributes\DataProvider; use Session; +use Symfony\Component\DomCrawler\Crawler; use function Safe\json_encode; @@ -288,16 +288,35 @@ public function testEmptyDefaultValuesAreNotListed( $builder->addQuestion("My question", $type::class, "", $extra_data); $form = $this->createForm($builder); + // Arrange: add translation + $this->addTranslationToForm( + $form, + 'fr_FR', + Form::TRANSLATION_KEY_NAME, + 'My form in fr_FR', + ); + // Act: get translations handler for the question $question = Question::getById($this->getQuestionId($form, "My question")); $handlers = $type->listTranslationsHandlers($question); - // Assert: no default value handlers should exist for the empty question - $key = Question::TRANSLATION_KEY_DEFAULT_VALUE; - $handlers = array_filter( - $handlers, - fn(TranslationHandler $h) => $h->getKey() == $key - ); - $this->assertEmpty($handlers); + ob_start(); + FormTranslation::displayTabContentForItem($form); + $content = ob_get_clean(); + $crawler = new Crawler($content); + + // Assert: no default value handler should be listed + foreach ($handlers as $handler) { + // Assert: translation row exists in the translations table + $row = $crawler->filter('#form-translation-modal-fr_FR tbody tr') + ->reduce(function (Crawler $node) use ($handler) { + return str_contains($node->text(), $handler->getName()); + }); + $this->assertEquals( + 0, + $row->count(), + "Translation row for handler '{$handler->getName()}' should not be listed as the default value is empty." + ); + } } } From 4cdbbb6a6c9eef8447215fe89c7b9e713051225b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Cailly?= Date: Mon, 3 Nov 2025 15:05:48 +0100 Subject: [PATCH 2/4] fix(tests): Update translation tests to ensure visibility of translations with empty default values --- tests/cypress/e2e/form/form_translation.cy.js | 55 ------ .../Form/Translation/FormTranslationTest.php | 160 ++++++++++++++++-- 2 files changed, 142 insertions(+), 73 deletions(-) diff --git a/tests/cypress/e2e/form/form_translation.cy.js b/tests/cypress/e2e/form/form_translation.cy.js index 80b1b341617..13b455744b6 100644 --- a/tests/cypress/e2e/form/form_translation.cy.js +++ b/tests/cypress/e2e/form/form_translation.cy.js @@ -571,59 +571,4 @@ describe('Edit form translations', () => { // Check the translations for the question description cy.findByRole('note', { name: 'Question description' }).should('exist').contains('Ceci est une question avec une description'); }); - - it('translation remain visible when a translation has already been provided and the default value is emptied', () => { - addTranslations(); - - // Go to the form editor - cy.findByRole('tab', { name: 'Form' }).click(); - - // Empty the form description - cy.findAllByLabelText('Form description') - .awaitTinyMCE() - .clear(); - - // Save the form - cy.findByRole('button', { name: 'Save' }).click(); - cy.checkAndCloseAlert('Item successfully updated'); - - // Go to the form translations page - cy.findByRole('tab', { name: 'Form translations 1' }).click(); - - // Open modal - cy.findByRole('button', { name: 'Edit translation' }).click(); - cy.findByRole('dialog').should('have.attr', 'data-cy-shown', 'true'); - - // Check that the translation for form description is visible and marked as "to review" - cy.get('@formTranslationsTable').findAllByRole('row').as('formTranslationsRows'); - cy.get('@formTranslationsRows').eq(3).within(() => { - cy.findByRole('cell', { name: 'Translation name' }).contains('Form description'); - cy.findByRole('cell', { name: 'Default value' }).should('contain.text', 'No default value'); - cy.findByRole('cell', { name: 'Translated value' }) - .findByLabelText('Enter translation') - .parent().parent() - .within(() => { - cy.get('.ti-alert-circle').should('exist'); - }); - }); - - // Clear the translation value - cy.get('@formTranslationsRows').eq(3).within(() => { - cy.findByLabelText('Enter translation').awaitTinyMCE().clear(); - }); - - // Save the translations - cy.findByRole('button', { name: 'Save translation' }).click(); - cy.checkAndCloseAlert('Item successfully updated'); - - // Re-open modal - cy.findByRole('button', { name: 'Edit translation' }).click(); - cy.findByRole('dialog').should('have.attr', 'data-cy-shown', 'true'); - - // Check that the translation for form description is no longer visible - cy.get('@formTranslationsTable').findAllByRole('row').as('formTranslationsRows'); - cy.get('@formTranslationsRows').eq(-1).within(() => { - cy.findByRole('cell', { name: 'Translation name' }).contains('Form description').should('not.exist'); - }); - }); }); diff --git a/tests/functional/Glpi/Form/Translation/FormTranslationTest.php b/tests/functional/Glpi/Form/Translation/FormTranslationTest.php index 3c84a604cfc..bd7d9d7f564 100644 --- a/tests/functional/Glpi/Form/Translation/FormTranslationTest.php +++ b/tests/functional/Glpi/Form/Translation/FormTranslationTest.php @@ -258,7 +258,7 @@ function ($handler) use (&$translation_items) { } } - public static function emptyDefaultValuesAreNotListedProvider(): iterable + public static function defaultValueTranslationsProvider(): iterable { $types = QuestionTypesManager::getInstance()->getQuestionTypes(); foreach ($types as $type) { @@ -278,17 +278,23 @@ public static function emptyDefaultValuesAreNotListedProvider(): iterable } } - #[DataProvider('emptyDefaultValuesAreNotListedProvider')] - public function testEmptyDefaultValuesAreNotListed( + /** + * Create a form with a question and add a translation for the form name. + * + * @param TranslationAwareQuestionType $type + * @param string $default_value + * @param string|null $extra_data + * @return Form + */ + private function createFormWithQuestionAndTranslation( TranslationAwareQuestionType $type, - ?string $extra_data, - ): void { - // Arrange: create a form with a question without default value + string $default_value, + ?string $extra_data + ): Form { $builder = new FormBuilder("My form"); - $builder->addQuestion("My question", $type::class, "", $extra_data); + $builder->addQuestion("My question", $type::class, $default_value, $extra_data); $form = $this->createForm($builder); - // Arrange: add translation $this->addTranslationToForm( $form, 'fr_FR', @@ -296,6 +302,45 @@ public function testEmptyDefaultValuesAreNotListed( 'My form in fr_FR', ); + return $form; + } + + /** + * Assert that handlers are listed (or not) in the translation table. + * + * @param array $handlers + * @param Crawler $crawler + * @param int $expected_count 0 if not listed, 1 if listed + * @param string $message + * @return void + */ + private function assertHandlersInTranslationTable( + array $handlers, + Crawler $crawler, + int $expected_count, + string $message + ): void { + foreach ($handlers as $handler) { + $row = $crawler->filter('#form-translation-modal-fr_FR tbody tr') + ->reduce(function (Crawler $node) use ($handler) { + return str_contains($node->text(), $handler->getName()); + }); + $this->assertEquals( + $expected_count, + $row->count(), + $message . " (handler: '{$handler->getName()}')" + ); + } + } + + #[DataProvider('defaultValueTranslationsProvider')] + public function testEmptyDefaultValuesAreNotListed( + TranslationAwareQuestionType $type, + ?string $extra_data, + ): void { + // Arrange: create a form with a question without default value + $form = $this->createFormWithQuestionAndTranslation($type, "", $extra_data); + // Act: get translations handler for the question $question = Question::getById($this->getQuestionId($form, "My question")); $handlers = $type->listTranslationsHandlers($question); @@ -305,18 +350,97 @@ public function testEmptyDefaultValuesAreNotListed( $content = ob_get_clean(); $crawler = new Crawler($content); - // Assert: no default value handler should be listed + // Assert: no default value handler should be listed when value is empty + $this->assertHandlersInTranslationTable( + $handlers, + $crawler, + 0, + "Translation row should not be listed as the default value is empty." + ); + } + + #[DataProvider('defaultValueTranslationsProvider')] + public function testNonEmptyDefaultValuesAreListed( + TranslationAwareQuestionType $type, + ?string $extra_data, + ): void { + // Arrange: create a form with a question with a non-empty default value + $form = $this->createFormWithQuestionAndTranslation($type, "Default value", $extra_data); + + // Act: get translations handler for the question + $question = Question::getById($this->getQuestionId($form, "My question")); + $handlers = $type->listTranslationsHandlers($question); + + ob_start(); + FormTranslation::displayTabContentForItem($form); + $content = ob_get_clean(); + $crawler = new Crawler($content); + + // Assert: default value handler should be listed when value is not empty + $this->assertHandlersInTranslationTable( + $handlers, + $crawler, + 1, + "Translation row should be listed as the default value is not empty." + ); + } + + #[DataProvider('defaultValueTranslationsProvider')] + public function testEmptyDefaultValuesWithExistingTranslationAreListed( + TranslationAwareQuestionType $type, + ?string $extra_data, + ): void { + // Arrange: create a form with a question without default value + $form = $this->createFormWithQuestionAndTranslation($type, "", $extra_data); + + // Arrange: add translation for the question's default value + $question = Question::getById($this->getQuestionId($form, "My question")); + $handlers = $type->listTranslationsHandlers($question); + foreach ($handlers as $handler) { - // Assert: translation row exists in the translations table - $row = $crawler->filter('#form-translation-modal-fr_FR tbody tr') - ->reduce(function (Crawler $node) use ($handler) { - return str_contains($node->text(), $handler->getName()); - }); - $this->assertEquals( - 0, - $row->count(), - "Translation row for handler '{$handler->getName()}' should not be listed as the default value is empty." + $this->addTranslationToForm( + $handler->getItem(), + 'fr_FR', + $handler->getKey(), + 'Translated default value in fr_FR', ); } + + ob_start(); + FormTranslation::displayTabContentForItem($form); + $content = ob_get_clean(); + $crawler = new Crawler($content); + + // Assert: default value handler should be listed even if original value is empty + // because a translation already exists + $this->assertHandlersInTranslationTable( + $handlers, + $crawler, + 1, + "Translation row should be listed as a translation exists even though the original default value is empty." + ); + + // Arrange: delete the added translation to clean up + foreach ($handlers as $handler) { + $translation = FormTranslation::getForItemKeyAndLanguage( + $handler->getItem(), + $handler->getKey(), + 'fr_FR' + ); + $translation->delete($translation->fields, true); + } + + ob_start(); + FormTranslation::displayTabContentForItem($form); + $content = ob_get_clean(); + $crawler = new Crawler($content); + + // Assert: default value handler should not be listed anymore + $this->assertHandlersInTranslationTable( + $handlers, + $crawler, + 0, + "Translation row should not be listed as the original default value is empty and the translation has been deleted." + ); } } From 25b1d39d10e5d7ac55103854b043ded2464fe9fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Cailly?= Date: Tue, 4 Nov 2025 10:17:42 +0100 Subject: [PATCH 3/4] fix(helpdesk): Handle helpdesk translations + entities restriction --- src/Glpi/Helpdesk/HelpdeskTranslation.php | 16 ++- .../admin/form/form_translation.html.twig | 17 ++- .../admin/helpdesk_home_translation.html.twig | 52 +++++--- .../Translation/HelpdeskTranslationTest.php | 114 ++++++++++++++++++ 4 files changed, 172 insertions(+), 27 deletions(-) diff --git a/src/Glpi/Helpdesk/HelpdeskTranslation.php b/src/Glpi/Helpdesk/HelpdeskTranslation.php index 208ac79ee82..623383c1162 100644 --- a/src/Glpi/Helpdesk/HelpdeskTranslation.php +++ b/src/Glpi/Helpdesk/HelpdeskTranslation.php @@ -41,6 +41,7 @@ use Glpi\Application\View\TemplateRenderer; use Glpi\Helpdesk\Tile\TilesManager; use Glpi\ItemTranslation\Context\ProvideTranslationsInterface; +use Glpi\ItemTranslation\Context\TranslationHandler; use Glpi\ItemTranslation\ItemTranslation; use Override; @@ -125,7 +126,7 @@ public function listTranslationsHandlers(): array $tiles_manager = TilesManager::getInstance(); $entities = array_map( fn($entity_id) => Entity::getById($entity_id), - array_keys((new Entity())->find()) + array_keys((new Entity())->find(getEntitiesRestrictCriteria(Entity::getTable()))) ); $entities_handlers = array_map( fn($entity) => $entity->listTranslationsHandlers(), @@ -183,7 +184,18 @@ public static function getLanguagesCanBeAddedToTranslation(): array #[Override] protected function getTranslationsHandlersForStats(): array { - return $this->listTranslationsHandlers(); + // Filter out handlers with empty values and those that do not have a translation yet + return array_map( + fn(array $handlers) => array_filter( + $handlers, + fn(TranslationHandler $handler) => !empty($handler->getValue()) || !empty(self::getForItemKeyAndLanguage( + $handler->getItem(), + $handler->getKey(), + $this->fields['language'] + )?->getTranslation()) + ), + $this->listTranslationsHandlers() + ); } public static function getSystemSQLCriteria(?string $tablename = null): array diff --git a/templates/pages/admin/form/form_translation.html.twig b/templates/pages/admin/form/form_translation.html.twig index f5b262ad227..0a330e9c43d 100644 --- a/templates/pages/admin/form/form_translation.html.twig +++ b/templates/pages/admin/form/form_translation.html.twig @@ -87,11 +87,7 @@ {% set entries = [] %} {% for handlers in form.listTranslationsHandlers() %} - {% set entries = entries|merge([{ - 'type': '' ~ handlers|first.getCategory() ~ '', - 'type_colspan': 3, - 'type_aria_label': _n('Category', 'Categories', 1) - }]) %} + {% set group_entries = [] %} {% for handler in handlers %} {% set item_translation = form_translation.getForItemKeyAndLanguage( handler.getItem(), @@ -105,7 +101,7 @@ {% else %} {% set default_value = handler.isRichText() ? default_value|safe_html : default_value|e %} {% endif %} - {% set entries = entries|merge([{ + {% set group_entries = group_entries|merge([{ 'type': '' ~ handler.getName() ~ '', 'default': default_value, 'translation': _self.getTranslationInput( @@ -118,6 +114,15 @@ }]) %} {% endif %} {% endfor %} + + {% if group_entries is not empty %} + {% set entries = entries|merge([{ + 'type': '' ~ handlers|first.getCategory() ~ '', + 'type_colspan': 3, + 'type_aria_label': _n('Category', 'Categories', 1) + }]) %} + {% set entries = entries|merge(group_entries) %} + {% endif %} {% endfor %}