[Translation][FrameworkBundle] Adding Translation Providers #37462

welcoMattic · 2020-06-30T16:52:21Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	Fix #36543
License	MIT
Doc PR	todo

This PR refers to the early opened RFC #36543 about adding third party translation SaaS into Symfony.
I worked in collaboration with @odolbeau on this first draft.

We have implemented only Loco Provider for now, to validate the main workflow of the feature.
We are not very sure about some naming convention, such as ~~Remote Storage~~Provider, it might be renamed in Transport, to correspond to the naming of third party services in Mailer and Notifier Components.

This PR brings 2 new commands in FrameworkBundle: translation:push and translation:pull
It adds also new configuration entry:

framework:
    default_locale: fr
    translator:
        default_path: '%kernel.project_dir%/translations'
        enabled_locales: '%locales%'
        fallbacks:
            - en
            - it
        providers:
            loco:
                dsn: '%env(LOCO_DSN)%'
                domains: ['messages']
                locales: '%locales%'

To do

Transifex Provider

To do:

Ensure that the creation of a Resource per translation key is the right way to do, if not re-work this part
Implement API call for Translation creation
Implement API call for Resource (means translation key) deletion

Crowdin Provider

To do:

Implement Add Storage and Add File API calls in order to create translations keys in Crowdin from the default locale local file
Implement Add Translation API call to send translation messages
Implement Update File API call in order to add or delete a translation key from a File

YaFou · 2020-07-01T07:17:06Z

I'm really interested in this new feature! Thank you, it's promising! 👏 I also have another adapter that can be implemented: Crowdin.

src/Symfony/Component/Translation/Remote/Dsn.php

src/Symfony/Component/Translation/TranslatorBagInterface.php

YaFou · 2020-08-08T16:28:55Z

@welcoMattic for the Transifex bridge, have you try to use %2F to escape / for the token?

welcoMattic · 2020-09-01T12:46:59Z

Before continuing the work on this PR, I ask myself several questions:

Should we rename RemoteStorage to Transport? (like in Notifier, Mailer, ...)
How many built-in SaaS client should we support for this first PR (should we release other "Transports" later in separate packages as it's done in Notifier?)
Should we simplify the AbstractFactory part? (after all I think is very complex for no obvious reason)

fabpot

I've just done a very very quick review to better understand the PR and its goals.

src/Symfony/Bundle/FrameworkBundle/Command/TranslationPullCommand.php

src/Symfony/Bundle/FrameworkBundle/Command/TranslationPushCommand.php

src/Symfony/Component/Translation/Remote/NullRemote.php

fabpot · 2020-09-07T15:20:40Z

src/Symfony/Component/Translation/Remote/RemoteDecorator.php

+
+use Symfony\Component\Translation\TranslatorBag;
+
+class RemoteDecorator implements RemoteInterface


Why is it useful?

It is in the read method, that we merge wanted domains (passed as arguments in commands) with default domains (defined in configuration)

src/Symfony/Component/Translation/Remote/RemoteInterface.php

fabpot · 2020-09-07T15:24:33Z

I think we would need at least 2 (or 3) providers to exercise the API and be sure it's generic and powerful enough.
I like the feature and I'd love to add it to 5.2 if possible.

welcoMattic · 2020-09-12T12:37:09Z

Thank you for the review. I'm ok to add 2 or 3 Providers (I will rename RemoteStorage for Provider).
I hope too include this feature in 5.2 release, I will do my best to work regularly on it, in order to be ready for 5.2.

welcoMattic · 2020-09-29T07:49:20Z

@andrii-bodnar Hi, sorry but I do not understand how Crowdin API works for uploading translations. You mentioned the Upload Translations endpoint, but in the documentation, there is no mention of content uploading. How do I upload existing translations and in which format?

andrii-bodnar · 2020-09-29T08:03:32Z

Hi @welcoMattic,

let's imagine you have domain admin.en.xlf and French translations - admin.fr.xlf

File admin.en.xlf is being added to the Crowdin project.

Now, if you want to upload admin.fr.xlf translations to Crowdin, you need to create a Storage with this file and use Upload Translations method. This method accepts storageId (admin.fr.xlf) and fileId (admin.en.xlf file ID)

welcoMattic · 2020-09-29T08:07:02Z

@andrii-bodnar thank you for the quick answer. Ok, it's not very easy to understand the concepts of Storage and File, moreover with an Upload operation between them. But ok, I will try to implement this workflow in Symfony. Thanks again.

welcoMattic · 2020-09-29T12:28:00Z

@andrii-bodnar can we discuss by email or in Symfony Slack? I still have difficulties to send existing translations to Crowdin.

andrii-bodnar · 2020-09-29T12:40:04Z

@welcoMattic, sure, I'll try to join Symfony Slack.

stephane-lou · 2020-09-30T08:26:56Z

This is a very welcome feature, thanks for the work guys !

welcoMattic · 2020-10-01T12:20:29Z

Well, there's still a lot of work here before we get to a complete and stable functionality (especially for the Provider Crowdin), and then the functional tests are not written yet.

For Crowdin, we have been working the last few days with @andrii-bodnar in order to implement all needed API calls, and it remains the delete method to implement, but Crowdin API does not allow fine deletion for translations keys. It's planned on their side to introduce such an endpoint very soon.

For Transifex, I need to work again the Provider, and explore the API more deeply to find the right way to do things.

For all these reasons, I think we should target a merge for 5.3, it's more reasonable than speed up work and merge an unstable feature in 5.2.

fabpot · 2020-10-07T15:52:43Z

We've just moved away from master as the main branch to use 5.x instead. Unfortunately, I cannot reopen the PR and change the target branch to 5.x. Can you open a new PR referencing this one to not loose the discussion? Thank you for your understanding and for your help.

This PR was merged into the 5.3-dev branch. Discussion ---------- [Translation] Adding Translation Providers | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Fix #36543 | License | MIT | Doc PR | todo > Follow up of #37462 This PR refers to the early opened RFC #36543 about adding third party translation SaaS into Symfony. I worked in collaboration with `@odolbeau` on this first draft. We have implemented only Loco Provider for now, to validate the main workflow of the feature. ~~We are not very sure about some naming convention, such as Remote Storage, it might be renamed in Transport, to correspond to the naming of third party services in Mailer and Notifier Components.~~ We use Provider name. This PR brings 2 new commands in Translation component: `translation:push` and `translation:pull` It adds also new configuration entry: ```yaml framework: default_locale: fr translator: default_path: '%kernel.project_dir%/translations' enabled_locales: '%locales%' fallbacks: - en - it providers: loco: dsn: '%env(LOCO_DSN)%' domains: ['messages'] locales: '%locales%' ``` ## To do - [x] Implement Provider into Translation component - [x] Plug it into FrameworkBundle - [x] Implement pull and push commands - [x] Implement Loco adapter - [x] Tests - [ ] Documentation - [x] Update CHANGELOG.md files in FrameworkBundle and Translation Component Docs: Adapt language settings in Lokalise to be sure that Symfony locales match with Lokalise languages ![language-setting-lokalise](https://user-images.githubusercontent.com/773875/102089496-997c5200-3e1c-11eb-8bff-bd2f9a5fe100.png) Todo: - [x] Implement POEditor Provider (:warning: there is a trick to do in POEditor Dashboard in order to make XLF export works, it will have to be documented explicitly in the symfony/symfony-docs PR) - [x] Implement Lokalise Provider - [x] Implement Crowdin Provider These 3 providers are implemented separately. They are not fully tested yet, it is planned to make it done by the end of April 2021. Commits ------- 6e55fa8 Added Translation Providers

carsonbot added Status: Needs Review Feature labels Jun 30, 2020

welcoMattic changed the title ~~Feature/remote storages~~ [Translation][FrameworkBundle] Adding translation remote storages Jun 30, 2020

nicolas-grekas added this to the next milestone Jun 30, 2020

yceruto added FrameworkBundle Translation labels Jun 30, 2020

welcoMattic force-pushed the feature/remote-storages branch from 326c05e to de0138f Compare July 1, 2020 08:52

welcoMattic mentioned this pull request Jul 1, 2020

[Translator] Intl message converter #28486

Closed

welcoMattic force-pushed the feature/remote-storages branch 2 times, most recently from 5beef58 to b9f5792 Compare July 1, 2020 13:05

Taluu reviewed Jul 1, 2020

View reviewed changes

src/Symfony/Component/Translation/Remote/Dsn.php Outdated Show resolved Hide resolved

Taluu approved these changes Jul 1, 2020

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Jul 1, 2020

welcoMattic force-pushed the feature/remote-storages branch from b9f5792 to 5cd37f5 Compare July 2, 2020 08:24

Pierstoval reviewed Jul 2, 2020

View reviewed changes

src/Symfony/Component/Translation/TranslatorBagInterface.php Outdated Show resolved Hide resolved

welcoMattic mentioned this pull request Jul 13, 2020

ICU - Main topic php-translation/symfony-bundle#399

Closed

welcoMattic marked this pull request as ready for review August 7, 2020 10:14

welcoMattic force-pushed the feature/remote-storages branch 3 times, most recently from 43a9251 to 9a230fd Compare August 7, 2020 14:12

welcoMattic force-pushed the feature/remote-storages branch from aa50e05 to db0a76d Compare August 10, 2020 12:22

fabpot requested changes Sep 7, 2020

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Reviewed labels Sep 7, 2020

welcoMattic added 10 commits September 28, 2020 23:14

First working state push and pull commands

1653289

Revert signature change because of possible BC Break

33f192a

Init Crowdin remote storage

a1e23e6

Improve DX in Remotes Storages

0fd2e89

Taken into consideration first review

d665197

Rename Remote to Provider

b4ad507

WIP Crowdin API Client

5f866c5

POEditor Provider

5cc4379

Modified POEditor Provider to use xlf export

26050f6

Transifex Provider in progress

db959f6

welcoMattic force-pushed the feature/remote-storages branch from 7d48c0c to 5afca3b Compare September 28, 2020 21:16

Implemented write method for Crodwin

d88294e

welcoMattic force-pushed the feature/remote-storages branch from 5afca3b to d88294e Compare September 30, 2020 09:03

fabpot closed this Oct 7, 2020

nicolas-grekas added the for 5.x label Oct 7, 2020

welcoMattic mentioned this pull request Oct 7, 2020

[Translation] Adding Translation Providers #38475

Merged

10 tasks

nicolas-grekas modified the milestones: 5.x, 5.2 Oct 14, 2020

andrii-bodnar mentioned this pull request Feb 8, 2021

Added Crowdin Translation Provider php-translation/symfony#2

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Translation][FrameworkBundle] Adding Translation Providers #37462

[Translation][FrameworkBundle] Adding Translation Providers #37462

welcoMattic commented Jun 30, 2020 •

edited

Loading

YaFou commented Jul 1, 2020 •

edited

Loading

YaFou commented Aug 8, 2020

welcoMattic commented Sep 1, 2020

fabpot left a comment

fabpot Sep 7, 2020

welcoMattic Sep 21, 2020

fabpot commented Sep 7, 2020

welcoMattic commented Sep 12, 2020

welcoMattic commented Sep 29, 2020

andrii-bodnar commented Sep 29, 2020

welcoMattic commented Sep 29, 2020

welcoMattic commented Sep 29, 2020

andrii-bodnar commented Sep 29, 2020

stephane-lou commented Sep 30, 2020

welcoMattic commented Oct 1, 2020

fabpot commented Oct 7, 2020


		use Symfony\Component\Translation\TranslatorBag;

		class RemoteDecorator implements RemoteInterface

[Translation][FrameworkBundle] Adding Translation Providers #37462

[Translation][FrameworkBundle] Adding Translation Providers #37462

Conversation

welcoMattic commented Jun 30, 2020 • edited Loading

To do

Transifex Provider

Crowdin Provider

YaFou commented Jul 1, 2020 • edited Loading

YaFou commented Aug 8, 2020

welcoMattic commented Sep 1, 2020

fabpot left a comment

Choose a reason for hiding this comment

fabpot Sep 7, 2020

Choose a reason for hiding this comment

welcoMattic Sep 21, 2020

Choose a reason for hiding this comment

fabpot commented Sep 7, 2020

welcoMattic commented Sep 12, 2020

welcoMattic commented Sep 29, 2020

andrii-bodnar commented Sep 29, 2020

welcoMattic commented Sep 29, 2020

welcoMattic commented Sep 29, 2020

andrii-bodnar commented Sep 29, 2020

stephane-lou commented Sep 30, 2020

welcoMattic commented Oct 1, 2020

fabpot commented Oct 7, 2020

welcoMattic commented Jun 30, 2020 •

edited

Loading

YaFou commented Jul 1, 2020 •

edited

Loading