symfony-bundle icon indicating copy to clipboard operation
symfony-bundle copied to clipboard

Translation keys not in Sync with Loco | Duplicated from Loco

Open dkozickis opened this issue 7 years ago • 12 comments

Hi!

I have an issue with translation keys going in and out of Loco and getting duplicated a lot. I've traced this down (I think), and wanted your opinion on what's the correct way forward fixing it.

First of all - here's a reproducer - https://github.com/dkozickis/translation-issue-reproducer Just change the Loco AP key in config/packages/php_translation.yml

templates/base.html.twig contains the example string

Run: php bin/console translation:extract

Below is generated, as you can see translation id is X8pvpvu:

<?xml version="1.0" encoding="utf-8"?>
<xliff xmlns="urn:oasis:names:tc:xliff:document:2.0" version="2.0" srcLang="en" trgLang="en">
  <file id="messages.en">
    <unit id="X8pvpvu">
      <notes>
        <note category="file-source" priority="1">templates/base.html.twig:10</note>
        <note category="state" priority="1">new</note>
      </notes>
      <segment>
        <source>test_translate</source>
        <target></target>
      </segment>
    </unit>
  </file>
</xliff>

First php bin/console translate:sync app gives expected results, string goes into Loco. Second php bin/console translate:sync app gives unexpected results:

<?xml version="1.0" encoding="utf-8"?>
<xliff xmlns="urn:oasis:names:tc:xliff:document:2.0" version="2.0" srcLang="en" trgLang="en">
  <file id="messages.en">
    <unit id="X8pvpvu">
      <notes>
        <note category="file-source" priority="1">templates/base.html.twig:10</note>
      </notes>
      <segment>
        <source>test_translate</source>
        <target></target>
      </segment>
    </unit>
    <unit id="CYJwdyK">
      <segment>
        <source>X8pvpvu</source>
        <target>test_translate</target>
      </segment>
    </unit>
  </file>
</xliff>

As you see, we now have a duplicate with source same as original string key.

There are couple things that cause this.

  1. By default sync command runs following:

https://github.com/php-translation/symfony-bundle/blob/d38e64602bb3b966844ef2a079335fed1efbee49/Service/StorageService.php#L92-L93

Meaning that during sync, with Loco adapter, first all data from Loco will be downloaded, then all data will be uploaded to Loco.

Given that we've already done first sync, second sync will produce following XLIFF result from Loco:

<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" version="1.2">
    <file source-language="en" target-language="en" tool-id="loco" datatype="plaintext" original="file.ext">
        <header>
            <tool tool-id="loco" tool-name="Loco for Symfony" tool-version="1.0.18 20180126-1" tool-company="Loco" />
        </header>
        <body>
            <trans-unit id="loco:5a7185a4820ee945368b4567">
                <source>X8pvpvu</source>
                <target>test_translate</target>
            </trans-unit>
        </body>
    </file>
</xliff>

For whatever the magic reason, Loco have put <unit id=> from initial exract into <source> At this point, Xliff Dumper (part of symfony-storage lib) grabs the <source> from Loco dump, and generates a new <unit id=>, thus we arrive to unexpected results mentioned above.

After that data is uploaded to Loco, and the new key is now reimported into Loco, essentially duplicating everything, after every translate:sync

My current workaround for this is to have custom Xliff Dumper overriding standard symfony/translation Xliff Dumper (works during translate:extract) and custom fork of symfony-storage overring Xliff Dumper there (duplication in symfony-storage). My Xliff Dumpers just take <source> as <unit id>, thus, when I load into Loco, I load with the same key as source, and when I download from Loco, I get <source> same as existing <key> in existing translation files, this helps me avoid duplication.

Hope my description was clear...

What I want to know:

  • Is my Loco setup incorrect?
  • Is my bundle setup incorrect?
  • Is this intended behaviour (and caused by my incorrect setup)?
  • If this is a bug - how would you like to fix it @Nyholm ?

Thank you!

dkozickis avatar Jan 31 '18 09:01 dkozickis

@Nyholm I know it's a bit of a long read - but if you could just let me know which way I should be going according to you vision of this bundle family I'll be happy to work on it and submit a PR :)

dkozickis avatar Feb 04 '18 15:02 dkozickis

It seams to be something strange when downloading from Loco.

Check the Loco::export function. Is there any other format or extension that works? Maybe check the index parameter? https://localise.biz/api/docs/export/exportlocale

Nyholm avatar Feb 04 '18 15:02 Nyholm

I have same problem. I have not been able to work with Loco. Sources changed, duplicated translations, copied all translations in all domain files...

h3llr4iser avatar Feb 07 '18 09:02 h3llr4iser

Hello. I found the problem, it is indeed the index key as @Nyholm was saying. For the scenarios we use "source" as the id for the translation we need to tell Loco that "index" is "text" instead of "id"

Basically changes are:

# /vendor/php-translation/loco-adapter/src/Loco.php:170 looks
- ['format' => 'symfony', 'status' => 'translated']
+ ['format' => 'symfony', 'status' => 'translated', 'index' => 'text']
# vendor/php-translation/loco-adapter/src/Loco.php:187
- $this->client->import()->import($projectKey, 'xliff', $data, ['locale' => $locale, 'async' => 1]);
+ $this->client->import()->import($projectKey, 'xliff', $data, ['locale' => $locale, 'async' => 1, 'index' => 'text']);

More info here: https://localise.biz/api/docs/import/import

joshlopes avatar Feb 09 '18 13:02 joshlopes

@dkozickis will the changes in https://github.com/php-translation/loco-adapter master fix your problem?

Nyholm avatar Feb 10 '18 16:02 Nyholm

@joshlopes @Nyholm thank you guys for your work!

The problem still persists, but it boils down to how Loco works.

When Loco gets below XLIFF with "index" set to "text", is still uses <unit id="X8pvpvu"> as index. Translation ends up with "key" X8pvpvu instead of test_translate, and exporting any entries from Loco is then useless, as key is already mismatching.

Interestingly - if we remove id="X8pvpvu" from <unit>, Loco will pickup test_translate as key, when given "index" of "text", which is the wanted behavior.

<?xml version="1.0" encoding="utf-8"?>
<xliff xmlns="urn:oasis:names:tc:xliff:document:2.0" version="2.0" srcLang="en" trgLang="en">
  <file id="messages.en">
    <unit id="X8pvpvu">
      <notes>
        <note category="file-source" priority="1">templates/base.html.twig:10</note>
        <note category="state" priority="1">new</note>
      </notes>
      <segment>
        <source>test_translate</source>
        <target></target>
      </segment>
    </unit>
  </file>
</xliff>

@Nyholm do you know if this is a bug or a feature of Loco? :) You seem to have a lot of experience working with them.

If this is a feature - do we modify Loco adapter to drop id part of <unit> if "index" is set to "text"?

dkozickis avatar Feb 16 '18 14:02 dkozickis

@dkozickis I'm currently using this on my projects. It's working as expected. Delete everything on loco and sync again may help, as it may be confused. Also if your id's are already there LOCO will use ids i think! So you will need to delete everything and sync again.

joshlopes avatar Feb 16 '18 15:02 joshlopes

@joshlopes are you using "text" as "index"? I will try deleting and making a new project in Loco, thank you 👍

dkozickis avatar Feb 16 '18 16:02 dkozickis

@dkozickis yes i'm using text which basically tells Loco that <source>My translation</source> is how it going to appear on Loco to translate. The random ID is still generated and my translation files are still showing all this random ids like rw.123$@s

joshlopes avatar Feb 16 '18 17:02 joshlopes

Hi ! It seems we went through the same problem. I managed to work with Loco only with download by uploading my previous translation files (YAML) directly on Loco. This way, i have all my previous french translations we worked on as source language (the project was only in french, first) and asset's ID are equal to my source keys (translation keys). I add english as a second language on both Loco and php translation configuration. Then i tried to sync but Loco read unit id as asset's ID. So there's no way to merge up my translations without it adding assets with wrong ids.

@dkozickis you said that you have overridden some Xliff Dumpers. Do you still do that ?

What is your current workflow when creating new keys to translate ? ~~I thought about requiring translators and the PO to add keys to Loco but what if they miss some keys on review ?~~ The idea of replacing unit id value by source on Loco::import would suits the project i'm working on.

kayneth avatar Feb 26 '18 16:02 kayneth

Hi all. Sorry to barge in here, and I know it's a bit late.

Your original issue here is a "fault" at Loco's end, but there is a reason for it (and a fix).

Loco has/had an issue with Symonfy's implementation of XLIFF 2.0 which Tobias kindly fixed. XLIFF 2 files must have the translation key in the name attribute, or the importer will fall back to the id attribute and NOT the <source>.

That fallback may seem like a bug, except that Loco's XLIFF parser is not specific to Symfony. Although XLIFF is great for readability and metadata, it also gives platforms the flexibility to go their own way. (e.g. Xcode uses the id attribute for translation keys, which is arguably slightly less wrong than using the <source> element). Ultimately Loco needs a dedicated import/symfony endpoint, but currently it has one generic importer for all XLIFF files. For now, the above fix is the only way to prevent the id attribute from being observed.

If you have any queries relating to the Loco API, as opposed to library code, feel free to email me directly at support at localise dot biz.

timwhitlock avatar Apr 10 '18 10:04 timwhitlock

Hi, I just retry to reproduce this bug with the given reproducer on the first message here. It does not happen anymore. Could you confirm @dkozickis? If yes, I will close the issue

welcoMattic avatar Jan 20 '20 21:01 welcoMattic