[Translation] Adding Translation Providers #38475

welcoMattic · 2020-10-07T21:42:29Z

Q	A
Branch?	5.x
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	Fix #36543
License	MIT
Doc PR	symfony/symfony-docs#15310

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:

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

Implement Provider into Translation component
Plug it into FrameworkBundle
Implement pull and push commands
Implement Loco adapter
Tests
Documentation
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

Todo:

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)
Implement Lokalise Provider
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.

welcoMattic · 2020-12-17T14:10:43Z

@fabpot @nicolas-grekas @Nyholm

I've continued this PR this week and I've reached a level where I think a first code review seems necessary.

What has been done:

Added 2 new commands translation:push and translation:pull with the options to choose the locales and domains to be processed, and 2 flags --force and --delete-obsolete.
3 Providers: Loco, PoEditor and Lokalise.
The internal mechanics for Providers creation via a Factory. (thank you @odolbeau)

What I have a doubt about:

The relevance of the --force flag for the push command. It forces the update of remote translations from the local translations. However, in my last conference at SymfonyWorld, I recommended never to update local translations, directly in the XLIFF files, but to always use the SaaS UI and pull to update local files. What do you think about removing the flag and therefore not allowing remote translations to be updated from local files?
The number of API calls for Loco: the initial creation of all translation keys will make one API call per key. This can quickly lead to a scaling problem.
The division of Providers into Bridges, as it is done for Notifier and Mailer. I simply reproduced what was done, but I'm not sure I did everything correctly.

I'm listening to comments and feedback to move this feature forward, in order to make it available for 5.3.

Nyholm

Thank you. Great work!

I understand you don't want to add symfony/http-client as a dependency to the Translator component, but we should not be so generous with optional arguments, ie the Factories and providers.

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

src/Symfony/Component/Translation/Catalogue/AbstractOperation.php

src/Symfony/Component/Translation/Provider/TranslationProviders.php

src/Symfony/Component/Translation/MessageCatalogue.php

src/Symfony/Component/Translation/Provider/ProvidersFactory.php

src/Symfony/Component/Translation/Provider/ProviderDecorator.php

src/Symfony/Component/Translation/Provider/ProviderInterface.php

src/Symfony/Component/Translation/Provider/ProvidersFactory.php

welcoMattic · 2021-04-20T09:21:23Z

@fabpot @Nyholm @nicolas-grekas All comments have been addressed, the CI failures on AppVeyor are unrelated to this PR. Thank you all for your reviews and your help to fix it.

UPGRADE-6.0.md

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

src/Symfony/Bundle/FrameworkBundle/Resources/config/translation_providers.php

src/Symfony/Component/Translation/CHANGELOG.md

welcoMattic · 2021-04-20T10:11:20Z

Thank you @fabpot, I've addressed your last comments

Co-authored-by: Olivier Dolbeau <github@a.bbnt.me>

welcoMattic · 2021-04-21T09:12:57Z

I've just rebased the branch on 5.x

fabpot · 2021-04-21T12:06:17Z

Thank you @welcoMattic.

fabpot · 2021-04-21T12:07:16Z

I will create the subtree split for the Loco provider after having merged the PRs that will add the other providers.

Nyholm · 2021-04-21T12:07:33Z

Wohoo. Thank you @welcoMattic, @odolbeau and others for all the work you've put in.

Kocal · 2021-04-21T14:05:55Z

Thanks @welcoMattic, great job!!

OskarStark · 2021-04-22T08:55:51Z

src/Symfony/Component/Translation/Bridge/Loco/Provider/LocoProviderFactory.php

+        }
+
+        $endpoint = sprintf('%s%s', 'default' === $dsn->getHost() ? self::HOST : $dsn->getHost(), $dsn->getPort() ? ':'.$dsn->getPort() : '');
+        $client = $this->client->withOptions([


In notifier bridges we do this in the transport. Imho the Factory does not need to know the endpoint.

plus we are consistent for mailer notifier and translation afterwards and the mailer and notifier bridges are quite bulletproof

…ii-bodnar) This PR was squashed before being merged into the 5.3-dev branch. Discussion ---------- [Translation] Added Crowdin Translation Provider | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets |  | License | MIT | Doc PR |  To follow up on #38475, this PR adds [Crowdin](https://crowdin.com/) Provider. This provider was removed a few weeks ago from the Translation Providers feature by `@welcoMattic`. We discussed all the recent changes made on `ProviderInterface`, `TranslatorBagInterface`, and others and I already applied these changes to Crowdin Provider. Also, this Provider is adapted to work with both [Crowdin](https://crowdin.com/) and [Crowdin Enterprise](https://crowdin.com/enterprise). The todo list to make it ready is: - [x] Write integration tests by mocking HTTP Responses I will make it done before the beginning of May. Commits ------- d7fda16 [Translation] Added Crowdin Translation Provider

This PR was merged into the 5.3-dev branch. Discussion ---------- [Translation] Added PoEditor Provider | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | | License | MIT | Doc PR | symfony/symfony-docs#15310 To follow up on #38475, this PR adds [PoEditor](https://poeditor.com/) Provider. The todo list to make it ready is: - [x] Apply recent changes that have been made on `ProviderInterface` and `TranslatorBagInterface` (we removed the `all()` and `getDomains()` method from TranslatorBagInterface) - [x] Add PoEditorProvider to `src/Symfony/Bundle/FrameworkBundle/Resources/config/translation_providers.php` file - [x] Add PoEditor case to `Symfony\Component\Translation\Exception\UnsupportedSchemeException` - [x] Write integration tests by mocking HTTP Responses The major part of the remaining work concerns tests, I will make it done before the beginning of May. Commits ------- 240ac22 Added PoEditor Provider

This PR was merged into the 5.3-dev branch. Discussion ---------- [Translation] Added Lokalise Provider | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | | License | MIT | Doc PR | symfony/symfony-docs#15310 To follow up on #38475, this PR adds [Lokalise](https://lokalise.com/) Provider. The todo list to make it ready is: - [x] Apply recent changes that have been made on `ProviderInterface` and `TranslatorBagInterface` (we removed the `all()` and `getDomains()` method from TranslatorBagInterface) - [x] Add LokaliseProvider to `src/Symfony/Bundle/FrameworkBundle/Resources/config/translation_providers.php` file - [x] Add Lokalise case to `Symfony\Component\Translation\Exception\UnsupportedSchemeException` - [x] Move `LokaliseProvider` and `LokaliseProviderFactory` from `Symfony\Component\Translation\Bridge\Lokalise\Provider` to `Symfony\Component\Translation\Bridge\Lokalise` namespace - [x] Write integration tests by mocking HTTP Responses The major part of the remaining work concerns tests, I will make it done before the beginning of May. Commits ------- 022d828 Added Lokalise Provider

This PR was submitted for the 5.4 branch but it was squashed and merged into the 5.3 branch instead. Discussion ---------- [Translation] Introduce Translation Providers Docs for symfony/symfony#38475, symfony/symfony#40926, symfony/symfony#40927, and symfony/symfony#40947 Ready for first review, but I'm not sure that I've written documentation in the right and all required places. ATM, Translation Providers Bridges packages doesn't exists, so Flex recipes are not created yet. Commits ------- 943a63f [Translation] Introduce Translation Providers

carsonbot added Status: Needs Review Feature labels Oct 7, 2020

lyrixx changed the title ~~Feature/remote storages~~ [Translation] remote storages Oct 7, 2020

welcoMattic changed the title ~~[Translation] remote storages~~ [Translation][FrameworkBundle] Adding Translation Providers Oct 8, 2020

nicolas-grekas added this to the 5.x milestone Oct 12, 2020

derrabus added the Translation label Oct 14, 2020

welcoMattic mentioned this pull request Nov 20, 2020

Is this repository maintained? php-translation/phraseapp-adapter#18

Closed

welcoMattic force-pushed the feature/remote-storages branch 11 times, most recently from 9fe79c7 to 6406e7b Compare December 17, 2020 13:49

welcoMattic force-pushed the feature/remote-storages branch 2 times, most recently from 7abc84f to 2869a15 Compare December 18, 2020 14:37

welcoMattic force-pushed the feature/remote-storages branch 3 times, most recently from f8121cb to f64d8e5 Compare January 21, 2021 11:25

andrii-bodnar mentioned this pull request Feb 8, 2021

Added Crowdin Translation Provider php-translation/symfony#2

Merged

welcoMattic force-pushed the feature/remote-storages branch from 5fceba0 to a97bd14 Compare February 16, 2021 08:48

Nyholm requested changes Feb 25, 2021

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Needs Review labels Feb 25, 2021

carsonbot changed the title ~~[Translation][FrameworkBundle] Adding Translation Providers~~ [Translation] Adding Translation Providers Feb 25, 2021

fabpot requested changes Apr 20, 2021

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Reviewed labels Apr 20, 2021

welcoMattic force-pushed the feature/remote-storages branch from 9095890 to 7c182f5 Compare April 20, 2021 10:00

Added Translation Providers

6e55fa8

Co-authored-by: Olivier Dolbeau <github@a.bbnt.me>

welcoMattic force-pushed the feature/remote-storages branch from 7c182f5 to 6e55fa8 Compare April 21, 2021 09:11

nicolas-grekas added Status: Reviewed and removed Status: Needs Work labels Apr 21, 2021

fabpot approved these changes Apr 21, 2021

View reviewed changes

fabpot merged commit 6ff9c97 into symfony:5.x Apr 21, 2021

welcoMattic deleted the feature/remote-storages branch April 21, 2021 15:22

OskarStark reviewed Apr 22, 2021

View reviewed changes

This was referenced Apr 23, 2021

[Translation] Added PoEditor Provider #40926

Merged

[Translation] Added Lokalise Provider #40927

Merged

andrii-bodnar mentioned this pull request Apr 25, 2021

[Translation] Added Crowdin Translation Provider #40947

Merged

1 task

fabpot mentioned this pull request May 1, 2021

Release v5.3.0-BETA2 #41019

Merged

welcoMattic mentioned this pull request May 5, 2021

[Translation] Introduce Translation Providers symfony/symfony-docs#15310

Merged

welcoMattic mentioned this pull request May 20, 2021

Added the 3 first Translation Providers symfony/recipes#949

Merged

ylcdx mentioned this pull request Jun 4, 2021

Add PHP 8 and Symfony 5.2 support. php-translation/phraseapp-adapter#19

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Translation] Adding Translation Providers #38475

[Translation] Adding Translation Providers #38475

welcoMattic commented Oct 7, 2020 •

edited

Loading

welcoMattic commented Dec 17, 2020

Nyholm left a comment

welcoMattic commented Apr 20, 2021

welcoMattic commented Apr 20, 2021

welcoMattic commented Apr 21, 2021

fabpot commented Apr 21, 2021

fabpot commented Apr 21, 2021

Nyholm commented Apr 21, 2021

Kocal commented Apr 21, 2021

OskarStark Apr 22, 2021

[Translation] Adding Translation Providers #38475

[Translation] Adding Translation Providers #38475

Conversation

welcoMattic commented Oct 7, 2020 • edited Loading

To do

welcoMattic commented Dec 17, 2020

Nyholm left a comment

Choose a reason for hiding this comment

welcoMattic commented Apr 20, 2021

welcoMattic commented Apr 20, 2021

welcoMattic commented Apr 21, 2021

fabpot commented Apr 21, 2021

fabpot commented Apr 21, 2021

Nyholm commented Apr 21, 2021

Kocal commented Apr 21, 2021

OskarStark Apr 22, 2021

Choose a reason for hiding this comment

welcoMattic commented Oct 7, 2020 •

edited

Loading