Skip to content

Commit 96b9f0f

Browse files
fabpotfabpot
committed
bug #49833 [Translation] TranslatorBag::diff now iterates over catalogue domains instead of operation domains (welcoMattic)
This PR was merged into the 5.4 branch. Discussion ---------- [Translation] TranslatorBag::diff now iterates over catalogue domains instead of operation domains | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | Fix #47886 <!-- prefix each issue number with "Fix #", no need to create an issue if none exists, explain below instead --> | License | MIT | Doc PR | For Loco Provider (at least, but I'm pretty sure it applies for Lokalise and Crowdin), the `--force` option on `translation:push` command has no effect. The main cause is located in the `TranslatorBag::diff` method which was iterating over `$operation->getDomains()` to add new messages to the diff catalogue. But, the fact is `$operation->getDomains()` returns all domains *AND* their `+intl-icu` variants! Which is certainly not what we want here. See the test that covers the case ```php public function testDiffWithIntlDomain() { $catalogueA = new MessageCatalogue('en', [ 'domain1+intl-icu' => ['foo' => 'foo', 'bar' => 'bar'], 'domain2' => ['baz' => 'baz', 'qux' => 'qux'], ]); $bagA = new TranslatorBag(); $bagA->addCatalogue($catalogueA); $catalogueB = new MessageCatalogue('en', [ 'domain1' => ['foo' => 'foo'], 'domain2' => ['baz' => 'baz', 'corge' => 'corge'], ]); $bagB = new TranslatorBag(); $bagB->addCatalogue($catalogueB); $bagResult = $bagA->diff($bagB); $this->assertEquals([ 'en' => [ 'domain1' => ['bar' => 'bar'], 'domain2' => ['qux' => 'qux'], ], ], $this->getAllMessagesFromTranslatorBag($bagResult)); } ``` Without the change, `'foo' => 'foo'` was in the result bag. Replaces #47986 Commits ------- 178abbf TranslatorBag::diff now iterates over catalogue domains instead of operation domains
2 parents 0c07e5e + 178abbf commit 96b9f0f

File tree

3 files changed

+75
-1
lines changed

3 files changed

+75
-1
lines changed

‎src/Symfony/Component/Translation/Tests/Command/TranslationPushCommandTest.php‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,52 @@ public function testPushNewMessages()
8989
$this->assertStringContainsString('[OK] New local translations has been sent to "null" (for "en, fr" locale(s), and "messages" domain(s)).', trim($tester->getDisplay()));
9090
}
9191

92+
public function testPushNewIntlIcuMessages()
93+
{
94+
$arrayLoader = new ArrayLoader();
95+
$xliffLoader = new XliffFileLoader();
96+
$locales = ['en', 'fr'];
97+
$domains = ['messages'];
98+
99+
// Simulate existing messages on Provider
100+
$providerReadTranslatorBag = new TranslatorBag();
101+
$providerReadTranslatorBag->addCatalogue($arrayLoader->load(['note' => 'NOTE'], 'en'));
102+
$providerReadTranslatorBag->addCatalogue($arrayLoader->load(['note' => 'NOTE'], 'fr'));
103+
104+
$provider = $this->createMock(ProviderInterface::class);
105+
$provider->expects($this->once())
106+
->method('read')
107+
->with($domains, $locales)
108+
->willReturn($providerReadTranslatorBag);
109+
110+
// Create local files, with a new message
111+
$filenameEn = $this->createFile([
112+
'note' => 'NOTE',
113+
'new.foo' => 'newFooIntlIcu',
114+
], 'en', 'messages+intl-icu.%locale%.xlf');
115+
$filenameFr = $this->createFile([
116+
'note' => 'NOTE',
117+
'new.foo' => 'nouveauFooIntlIcu',
118+
], 'fr', 'messages+intl-icu.%locale%.xlf');
119+
$localTranslatorBag = new TranslatorBag();
120+
$localTranslatorBag->addCatalogue($xliffLoader->load($filenameEn, 'en'));
121+
$localTranslatorBag->addCatalogue($xliffLoader->load($filenameFr, 'fr'));
122+
123+
$provider->expects($this->once())
124+
->method('write')
125+
->with($localTranslatorBag->diff($providerReadTranslatorBag));
126+
127+
$provider->expects($this->once())
128+
->method('__toString')
129+
->willReturn('null://default');
130+
131+
$tester = $this->createCommandTester($provider, $locales, $domains);
132+
133+
$tester->execute(['--locales' => ['en', 'fr'], '--domains' => ['messages']]);
134+
135+
$this->assertStringContainsString('[OK] New local translations has been sent to "null" (for "en, fr" locale(s), and "messages" domain(s)).', trim($tester->getDisplay()));
136+
}
137+
92138
public function testPushForceMessages()
93139
{
94140
$xliffLoader = new XliffFileLoader();

‎src/Symfony/Component/Translation/Tests/TranslatorBagTest.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,34 @@ public function testDiff()
6666
], $this->getAllMessagesFromTranslatorBag($bagResult));
6767
}
6868

69+
public function testDiffWithIntlDomain()
70+
{
71+
$catalogueA = new MessageCatalogue('en', [
72+
'domain1+intl-icu' => ['foo' => 'foo', 'bar' => 'bar'],
73+
'domain2' => ['baz' => 'baz', 'qux' => 'qux'],
74+
]);
75+
76+
$bagA = new TranslatorBag();
77+
$bagA->addCatalogue($catalogueA);
78+
79+
$catalogueB = new MessageCatalogue('en', [
80+
'domain1' => ['foo' => 'foo'],
81+
'domain2' => ['baz' => 'baz', 'corge' => 'corge'],
82+
]);
83+
84+
$bagB = new TranslatorBag();
85+
$bagB->addCatalogue($catalogueB);
86+
87+
$bagResult = $bagA->diff($bagB);
88+
89+
$this->assertEquals([
90+
'en' => [
91+
'domain1' => ['bar' => 'bar'],
92+
'domain2' => ['qux' => 'qux'],
93+
],
94+
], $this->getAllMessagesFromTranslatorBag($bagResult));
95+
}
96+
6997
public function testIntersect()
7098
{
7199
$catalogueA = new MessageCatalogue('en', ['domain1' => ['foo' => 'foo', 'bar' => 'bar'], 'domain2' => ['baz' => 'baz', 'qux' => 'qux']]);

‎src/Symfony/Component/Translation/TranslatorBag.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public function diff(TranslatorBagInterface $diffBag): self
7070
$operation->moveMessagesToIntlDomainsIfPossible(AbstractOperation::NEW_BATCH);
7171
$newCatalogue = new MessageCatalogue($locale);
7272

73-
foreach ($operation->getDomains() as $domain) {
73+
foreach ($catalogue->getDomains() as $domain) {
7474
$newCatalogue->add($operation->getNewMessages($domain), $domain);
7575
}
7676

0 commit comments

Comments
 (0)