bug symfony#44085 [Translation] Fix TranslationPullCommand with ICU t…

…ranslations (Kocal) This PR was squashed before being merged into the 5.3 branch. Discussion ---------- [Translation] Fix TranslationPullCommand with ICU translations | Q | A | ------------- | --- | Branch? | 5.3 | Bug fix? | yes | New feature? | no  | Deprecations? | no  | Tickets | Fix #...  | License | MIT | Doc PR | symfony/symfony-docs#...  Today I've faced an issue when trying to pull (ICU and non-ICU) translations from Loco: - I have a non-ICU translation `say_hello` tagged as `messages` - and a ICU translation `say_hello_icu` tagged as `messages+intl-icu` When running `bin/console translation:pull loco --domains messages --domains messages+intl-icu`, it **only** creates a single file `messages.fr.xlf` that contains the two translations, so `say_hello_icu` will never be interpreted as an ICU-translation: ```xml <?xml version="1.0" encoding="utf-8"?> <xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" version="1.2"> <file source-language="fr" target-language="fr" datatype="plaintext" original="file.ext"> <header> <tool tool-id="symfony" tool-name="Symfony"/> </header> <body> <trans-unit id="xQEX2ok" resname="say_hello_icu"> <source>say_hello_icu</source> <target>Bonjour {name} !</target> </trans-unit> <trans-unit id="1IHotcu" resname="say_hello"> <source>say_hello</source> <target>Bonjour %name% !</target> </trans-unit> </body> </file> </xliff> ``` With this fix, it creates two files as expected: - `messages.fr.xlf`: ```xml <?xml version="1.0" encoding="utf-8"?> <xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" version="1.2"> <file source-language="fr" target-language="fr" datatype="plaintext" original="file.ext"> <header> <tool tool-id="symfony" tool-name="Symfony"/> </header> <body> <trans-unit id="1IHotcu" resname="say_hello"> <source>say_hello</source> <target>Bonjour %name% !</target> </trans-unit> </body> </file> </xliff> ``` - `messages+intl-icu.fr.xlf`: ```xml <?xml version="1.0" encoding="utf-8"?> <xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" version="1.2"> <file source-language="fr" target-language="fr" datatype="plaintext" original="file.ext"> <header> <tool tool-id="symfony" tool-name="Symfony"/> </header> <body> <trans-unit id="xQEX2ok" resname="say_hello_icu"> <source>say_hello_icu</source> <target>Bonjour {name} !</target> </trans-unit> </body> </file> </xliff> ``` I've also added more tests to `LocoProvider` and `TranslationPullCommand`, ensuring it still handle ICU translations in the future. Ping `@welcoMattic` Commits ------- 1802488 [Translation] Fix TranslationPullCommand with ICU translations
Stefan39 · Dec 25, 2021 · 758c07f · 758c07f
2 parents 231918b + 1802488
commit 758c07f
Show file tree

Hide file tree

Showing 5 changed files with 93 additions and 9 deletions.
diff --git a/src/Symfony/Component/Translation/Bridge/Loco/Tests/LocoProviderTest.php b/src/Symfony/Component/Translation/Bridge/Loco/Tests/LocoProviderTest.php
@@ -426,12 +426,16 @@ public function getResponsesForManyLocalesAndManyDomains(): \Generator
         $expectedTranslatorBag = new TranslatorBag();
         $expectedTranslatorBag->addCatalogue($arrayLoader->load([
             'index.hello' => 'Hello',
-            'index.greetings' => 'Welcome, {firstname}!',
         ], 'en'));
+        $expectedTranslatorBag->addCatalogue($arrayLoader->load([
+            'index.greetings' => 'Welcome, {firstname}!',
+        ], 'en', 'messages+intl-icu'));
         $expectedTranslatorBag->addCatalogue($arrayLoader->load([
             'index.hello' => 'Bonjour',
-            'index.greetings' => 'Bienvenue, {firstname} !',
         ], 'fr'));
+        $expectedTranslatorBag->addCatalogue($arrayLoader->load([
+            'index.greetings' => 'Bienvenue, {firstname} !',
+        ], 'fr', 'messages+intl-icu'));
         $expectedTranslatorBag->addCatalogue($arrayLoader->load([
             'firstname.error' => 'Firstname must contains only letters.',
             'lastname.error' => 'Lastname must contains only letters.',
@@ -443,7 +447,7 @@ public function getResponsesForManyLocalesAndManyDomains(): \Generator
 
         yield [
             ['en', 'fr'],
-            ['messages', 'validators'],
+            ['messages', 'messages+intl-icu', 'validators'],
             [
                 'en' => [
                     'messages' => <<<'XLIFF'
@@ -458,6 +462,19 @@ public function getResponsesForManyLocalesAndManyDomains(): \Generator
         <source>index.hello</source>
         <target state="translated">Hello</target>
       </trans-unit>
+    </body>
+  </file>
+</xliff>
+XLIFF
+                    ,
+                    'messages+intl-icu' => <<<'XLIFF'
+<?xml version="1.0" encoding="UTF-8"?>
+<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 http://docs.oasis-open.org/xliff/v1.2/os/xliff-core-1.2-strict.xsd">
+  <file original="https://localise.biz/user/symfony-translation-provider" source-language="en" datatype="database" tool-id="loco">
+    <header>
+      <tool tool-id="loco" tool-name="Loco" tool-version="1.0.25 20201211-1" tool-company="Loco"/>
+    </header>
+    <body>
       <trans-unit id="loco:5fd89b8542e5aa5cc27457e2" resname="index.greetings" datatype="plaintext" extradata="loco:format=icu">
         <source>index.greetings</source>
         <target state="translated">Welcome, {firstname}!</target>
@@ -502,6 +519,19 @@ public function getResponsesForManyLocalesAndManyDomains(): \Generator
         <source>index.hello</source>
         <target state="translated">Bonjour</target>
       </trans-unit>
+    </body>
+  </file>
+</xliff>
+XLIFF
+                    ,
+                    'messages+intl-icu' => <<<'XLIFF'
+<?xml version="1.0" encoding="UTF-8"?>
+<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 http://docs.oasis-open.org/xliff/v1.2/os/xliff-core-1.2-strict.xsd">
+  <file original="https://localise.biz/user/symfony-translation-provider" source-language="en" datatype="database" tool-id="loco">
+    <header>
+      <tool tool-id="loco" tool-name="Loco" tool-version="1.0.25 20201211-1" tool-company="Loco"/>
+    </header>
+    <body>
       <trans-unit id="loco:5fd89b8542e5aa5cc27457e2" resname="index.greetings" datatype="plaintext" extradata="loco:format=icu">
         <source>index.greetings</source>
         <target state="translated">Bienvenue, {firstname} !</target>

diff --git a/src/Symfony/Component/Translation/Catalogue/AbstractOperation.php b/src/Symfony/Component/Translation/Catalogue/AbstractOperation.php
@@ -83,7 +83,18 @@ public function __construct(MessageCatalogueInterface $source, MessageCatalogueI
     public function getDomains()
     {
         if (null === $this->domains) {
-            $this->domains = array_values(array_unique(array_merge($this->source->getDomains(), $this->target->getDomains())));
+            $domains = [];
+            foreach ([$this->source, $this->target] as $catalogue) {
+                foreach ($catalogue->getDomains() as $domain) {
+                    $domains[$domain] = $domain;
+
+                    if ($catalogue->all($domainIcu = $domain.MessageCatalogueInterface::INTL_DOMAIN_SUFFIX)) {
+                        $domains[$domainIcu] = $domainIcu;
+                    }
+                }
+            }
+
+            $this->domains = array_values($domains);
         }
 
         return $this->domains;

diff --git a/src/Symfony/Component/Translation/Tests/Catalogue/MergeOperationTest.php b/src/Symfony/Component/Translation/Tests/Catalogue/MergeOperationTest.php
@@ -58,7 +58,7 @@ public function testGetResultFromIntlDomain()
         $this->assertEquals(
             new MessageCatalogue('en', [
                 'messages' => ['a' => 'old_a', 'b' => 'old_b'],
-                'messages+intl-icu' => ['d' => 'old_d', 'c' => 'new_c'],
+                'messages+intl-icu' => ['d' => 'old_d', 'c' => 'new_c', 'a' => 'new_a'],
             ]),
             $this->createOperation(
                 new MessageCatalogue('en', ['messages' => ['a' => 'old_a', 'b' => 'old_b'], 'messages+intl-icu' => ['d' => 'old_d']]),

diff --git a/src/Symfony/Component/Translation/Tests/Catalogue/TargetOperationTest.php b/src/Symfony/Component/Translation/Tests/Catalogue/TargetOperationTest.php
@@ -72,6 +72,7 @@ public function testGetResultWithMixedDomains()
         $this->assertEquals(
             new MessageCatalogue('en', [
                 'messages' => ['a' => 'old_a'],
+                'messages+intl-icu' => ['a' => 'new_a'],
             ]),
             $this->createOperation(
                 new MessageCatalogue('en', ['messages' => ['a' => 'old_a']]),
@@ -103,7 +104,7 @@ public function testGetResultWithMixedDomains()
         $this->assertEquals(
             new MessageCatalogue('en', [
                 'messages' => ['a' => 'old_a'],
-                'messages+intl-icu' => ['b' => 'new_b'],
+                'messages+intl-icu' => ['b' => 'new_b', 'a' => 'new_a'],
             ]),
             $this->createOperation(
                 new MessageCatalogue('en', ['messages' => ['a' => 'old_a']]),

diff --git a/src/Symfony/Component/Translation/Tests/Command/TranslationPullCommandTest.php b/src/Symfony/Component/Translation/Tests/Command/TranslationPullCommandTest.php
@@ -46,19 +46,27 @@ public function testPullNewXlf12Messages()
     {
         $arrayLoader = new ArrayLoader();
         $filenameEn = $this->createFile();
+        $filenameEnIcu = $this->createFile(['say_hello' => 'Welcome, {firstname}!'], 'en', 'messages+intl-icu.%locale%.xlf');
         $filenameFr = $this->createFile(['note' => 'NOTE'], 'fr');
+        $filenameFrIcu = $this->createFile(['say_hello' => 'Bonjour, {firstname}!'], 'fr', 'messages+intl-icu.%locale%.xlf');
         $locales = ['en', 'fr'];
-        $domains = ['messages'];
+        $domains = ['messages', 'messages+intl-icu'];
 
         $providerReadTranslatorBag = new TranslatorBag();
         $providerReadTranslatorBag->addCatalogue($arrayLoader->load([
             'note' => 'NOTE',
             'new.foo' => 'newFoo',
         ], 'en'));
+        $providerReadTranslatorBag->addCatalogue($arrayLoader->load([
+            'say_hello' => 'Welcome, {firstname}!',
+        ], 'en', 'messages+intl-icu'));
         $providerReadTranslatorBag->addCatalogue($arrayLoader->load([
             'note' => 'NOTE',
             'new.foo' => 'nouveauFoo',
         ], 'fr'));
+        $providerReadTranslatorBag->addCatalogue($arrayLoader->load([
+            'say_hello' => 'Bonjour, {firstname}!',
+        ], 'fr', 'messages+intl-icu'));
 
         $provider = $this->createMock(ProviderInterface::class);
         $provider->expects($this->once())
@@ -71,9 +79,9 @@ public function testPullNewXlf12Messages()
             ->willReturn('null://default');
 
         $tester = $this->createCommandTester($provider, $locales, $domains);
-        $tester->execute(['--locales' => ['en', 'fr'], '--domains' => ['messages']]);
+        $tester->execute(['--locales' => ['en', 'fr'], '--domains' => ['messages', 'messages+intl-icu']]);
 
-        $this->assertStringContainsString('[OK] New translations from "null" has been written locally (for "en, fr" locale(s), and "messages" domain(s)).', trim($tester->getDisplay()));
+        $this->assertStringContainsString('[OK] New translations from "null" has been written locally (for "en, fr" locale(s), and "messages, messages+intl-icu"', trim($tester->getDisplay()));
         $this->assertXmlStringEqualsXmlString(<<<XLIFF
 <?xml version="1.0"?>
 <xliff version="1.2" xmlns="urn:oasis:names:tc:xliff:document:1.2">
@@ -97,6 +105,23 @@ public function testPullNewXlf12Messages()
             , file_get_contents($filenameEn));
         $this->assertXmlStringEqualsXmlString(<<<XLIFF
 <?xml version="1.0"?>
+<xliff version="1.2" xmlns="urn:oasis:names:tc:xliff:document:1.2">
+    <file source-language="en" target-language="en" datatype="plaintext" original="file.ext">
+        <header>
+            <tool tool-id="symfony" tool-name="Symfony"/>
+        </header>
+        <body>
+            <trans-unit id="1IHotcu" resname="say_hello">
+                <source>say_hello</source>
+                <target>Welcome, {firstname}!</target>
+            </trans-unit>
+        </body>
+    </file>
+</xliff>
+XLIFF
+            , file_get_contents($filenameEnIcu));
+        $this->assertXmlStringEqualsXmlString(<<<XLIFF
+<?xml version="1.0"?>
 <xliff version="1.2" xmlns="urn:oasis:names:tc:xliff:document:1.2">
     <file source-language="en" target-language="fr" datatype="plaintext" original="file.ext">
         <header>
@@ -116,6 +141,23 @@ public function testPullNewXlf12Messages()
 </xliff>
 XLIFF
             , file_get_contents($filenameFr));
+        $this->assertXmlStringEqualsXmlString(<<<XLIFF
+<?xml version="1.0"?>
+<xliff version="1.2" xmlns="urn:oasis:names:tc:xliff:document:1.2">
+    <file source-language="en" target-language="fr" datatype="plaintext" original="file.ext">
+        <header>
+            <tool tool-id="symfony" tool-name="Symfony"/>
+        </header>
+        <body>
+            <trans-unit id="1IHotcu" resname="say_hello">
+                <source>say_hello</source>
+                <target>Bonjour, {firstname}!</target>
+            </trans-unit>
+        </body>
+    </file>
+</xliff>
+XLIFF
+            , file_get_contents($filenameFrIcu));
     }
 
     public function testPullNewXlf20Messages()