BAP-12368: Incorrect oro:translation:pack work with dump option (#5492)

* BAP-12368: Incorrect oro:translation:pack work with dump option - added translator helper for get duplicates and process it * BAP-12368: Incorrect oro:translation:pack work with dump option - updated translation dump helper - updated method for removed duplicates * BAP-12368 Incorrect oro:translation:pack work with dump option - change logic for generated language translation for all namespaces * BAP-12368: Incorrect oro:translation:pack work with dump option Fixed bug for upload translation on crowdin * BAP-12368: Incorrect oro:translation:pack work with dump option - Added --force option - Change logic for upload files for crowdin with new option - Update command.md documentation * BAP-12368: Incorrect oro:translation:pack work with dump option - fixed phpcs * BAP-12368: Incorrect oro:translation:pack work with dump option - updated unit test * BAP-12368 Incorrect oro:translation:pack work with dump option - fixed required argument. Changed argument "project" to non required for --dump command - removed "hardcode" for namespaces. Changed "hardcode" to regexp for check all oro namespaces * BAP-12368 Incorrect oro:translation:pack work with dump option - fixed for codereview * BAP-12368 Incorrect oro:translation:pack work with dump option - fixed unit test
oroinc · Jan 3, 2017 · a0c5a48 · a0c5a48
1 parent 67792db
commit a0c5a48
Show file tree

Hide file tree

Showing 10 changed files with 208 additions and 40 deletions.
diff --git a/src/Oro/Bundle/TranslationBundle/Command/OroTranslationPackCommand.php b/src/Oro/Bundle/TranslationBundle/Command/OroTranslationPackCommand.php
@@ -31,7 +31,7 @@ protected function configure()
             ->setDescription('Dump translation messages and optionally upload them to third-party service')
             ->setDefinition(
                 array(
-                    new InputArgument('project', InputArgument::REQUIRED, 'The project [e.g Oro, OroCRM etc]'),
+                    new InputArgument('project', InputArgument::OPTIONAL, 'The project [e.g Oro, OroCRM etc]'),
                     new InputArgument(
                         'locale',
                         InputArgument::OPTIONAL,
@@ -94,6 +94,12 @@ protected function configure()
                         InputOption::VALUE_NONE,
                         'Download all language packs from project at translation service'
                     ),
+                    new InputOption(
+                        'force',
+                        'f',
+                        InputOption::VALUE_NONE,
+                        'Usage with --upload-mode <info>update</info> option. Replace all translation file on crowdin'
+                    )
                 )
             )
             ->setHelp(
@@ -146,17 +152,29 @@ protected function execute(InputInterface $input, OutputInterface $output)
         }
 
         if ($input->getOption('upload') === true) {
+            if (!$input->getArgument('project')) {
+                $output->writeln('<info>Input argument "project" is required</info>');
+
+                return 1;
+            }
             $translationService = $this->getTranslationService($input, $output);
 
             $langPackDirs = [];
             foreach ($namespaces as $namespace) {
                 $langPackDirs[$namespace] = $this->getLangPackDir($namespace);
             }
 
-            $this->upload($translationService, $input->getOption('upload-mode'), $langPackDirs);
+            $force = (bool) $input->getOption('force');
+
+            $this->upload($translationService, $input->getOption('upload-mode'), $langPackDirs, $force);
         }
 
         if ($input->getOption('download') === true) {
+            if (!$input->getArgument('project')) {
+                $output->writeln('<info>Input argument "project" is required</info>');
+
+                return 1;
+            }
             $this->download($input, $output);
         }
 
@@ -167,13 +185,14 @@ protected function execute(InputInterface $input, OutputInterface $output)
      * @param TranslationServiceProvider $translationService
      * @param string                     $mode
      * @param array                      $languagePackPath one or few dirs
+     * @param bool                       $force
      *
      * @return array
      */
-    protected function upload(TranslationServiceProvider $translationService, $mode, $languagePackPath)
+    protected function upload(TranslationServiceProvider $translationService, $mode, $languagePackPath, $force = false)
     {
         if ('update' == $mode) {
-            $translationService->update($languagePackPath);
+            $translationService->update($languagePackPath, $force);
         } else {
             $translationService->upload($languagePackPath);
         }
@@ -264,22 +283,21 @@ protected function getAdapterFromInput(InputInterface $input)
      */
     protected function dump($projectNamespace, $locale, OutputInterface $output, $outputFormat)
     {
-        $output->writeln(sprintf('Dumping language pack for <info>%s</info>', $projectNamespace));
+        $output->writeln('Dumping language pack');
 
         $container = $this->getContainer();
         $dumper = new TranslationPackDumper(
             $container->get('translation.writer'),
             $container->get('translation.extractor'),
             $container->get('translation.loader'),
             new Filesystem(),
-            $container->get('kernel')->getBundles()
+            $container->get('kernel')->getBundles(),
+            $container->get('oro_translation.utils.translation_dump_helper')
         );
         $dumper->setLogger(new OutputLogger($output));
 
-        $languagePackPath = $this->getLangPackDir($projectNamespace);
         $dumper->dump(
-            $languagePackPath,
-            $projectNamespace,
+            $this->path,
             $outputFormat,
             $locale
         );

diff --git a/src/Oro/Bundle/TranslationBundle/Provider/AbstractAPIAdapter.php b/src/Oro/Bundle/TranslationBundle/Provider/AbstractAPIAdapter.php
@@ -112,18 +112,19 @@ protected function getFileFolders(array $files)
         $dirs = [];
 
         foreach ($files as $remotePath) {
-            $subFolders = explode(DIRECTORY_SEPARATOR, dirname($remotePath));
+            $remotePath = str_replace(DIRECTORY_SEPARATOR, '/', dirname($remotePath));
+            $subFolders = array_filter(explode('/', $remotePath));
 
             $currentDir = [];
             foreach ($subFolders as $folderName) {
                 $currentDir[] = $folderName;
 
                 // crowdin understand only "/" as directory separator
-                $path         = implode('/', $currentDir);
-                $dirs[]  = $path;
+                $path = implode('/', $currentDir);
+                $dirs[] = $path;
             }
         }
 
-        return $dirs;
+        return array_unique($dirs);
     }
 }
diff --git a/src/Oro/Bundle/TranslationBundle/Provider/CrowdinAdapter.php b/src/Oro/Bundle/TranslationBundle/Provider/CrowdinAdapter.php
@@ -64,8 +64,9 @@ public function createDirectories($dirs)
     {
         $this->logger->info('Creating directories');
 
+        $current = 1;
+
         foreach ($dirs as $index => $dir) {
-            $current = $index + 1;
             $result  = $this->addDirectory($dir);
 
             if (false == $result['success'] && isset($result['error'])) {
@@ -82,13 +83,15 @@ public function createDirectories($dirs)
                     sprintf('%0.2f%% [created] <info>%s</info> directory', $current * 100 / count($dirs), $dir)
                 );
             }
+
+            $current++;
         }
 
         return $this;
     }
 
     /**
-     * @param string $files
+     * @param array $files
      * @param string $mode 'add' or 'update'
      *
      * @return bool

diff --git a/src/Oro/Bundle/TranslationBundle/Provider/TranslationPackDumper.php b/src/Oro/Bundle/TranslationBundle/Provider/TranslationPackDumper.php
@@ -16,6 +16,8 @@
 
 use Symfony\Bundle\FrameworkBundle\Translation\TranslationLoader;
 
+use Oro\Bundle\TranslationBundle\Utils\TranslationDumpHelper;
+
 class TranslationPackDumper implements LoggerAwareInterface
 {
     /** @var TranslationWriter */
@@ -39,26 +41,32 @@ class TranslationPackDumper implements LoggerAwareInterface
     /** @var MessageCatalogue[] Translations loaded from yaml files, existing */
     protected $loadedTranslations = [];
 
+    /** @var  TranslationDumpHelper */
+    protected $helper;
+
     /**
-     * @param TranslationWriter  $writer
-     * @param ExtractorInterface $extractor
-     * @param TranslationLoader  $loader
-     * @param Filesystem         $filesystem
-     * @param array              $bundles
+     * @param TranslationWriter     $writer
+     * @param ExtractorInterface    $extractor
+     * @param TranslationLoader     $loader
+     * @param Filesystem            $filesystem
+     * @param array                 $bundles
+     * @param TranslationDumpHelper $helper
      */
     public function __construct(
         TranslationWriter $writer,
         ExtractorInterface $extractor,
         TranslationLoader $loader,
         Filesystem $filesystem,
-        array $bundles
+        array $bundles,
+        TranslationDumpHelper $helper
     ) {
         $this->writer     = $writer;
         $this->extractor  = $extractor;
         $this->loader     = $loader;
         $this->filesystem = $filesystem;
         $this->bundles    = $bundles;
         $this->logger     = new NullLogger();
+        $this->helper     = $helper;
     }
 
     /**
@@ -70,17 +78,17 @@ public function setLogger(LoggerInterface $logger)
     }
 
     /**
-     * @param string $langPackDir       language pack dir in temp folder
-     * @param string $projectNamespace  e.g. Oro, OroCRM, etc
+     * @param string $systemPath        system path for get language temp folder
      * @param string $outputFormat      xml, yml, etc
      * @param string $locale            en, en_US, fr, etc
      */
-    public function dump($langPackDir, $projectNamespace, $outputFormat, $locale)
+    public function dump($systemPath, $outputFormat, $locale)
     {
         $this->preloadExistingTranslations($locale);
         foreach ($this->bundles as $bundle) {
             // skip bundles from other projects
-            if ($projectNamespace != $this->getBundlePrefix($bundle->getNamespace())) {
+            $namespace = $this->getBundlePrefix($bundle->getNamespace());
+            if (!preg_match('/^Oro.+/', $namespace)) {
                 continue;
             }
 
@@ -95,7 +103,10 @@ public function dump($langPackDir, $projectNamespace, $outputFormat, $locale)
             $messageCatalogue = $operation->getResult();
 
             $isEmptyCatalogue = $this->validateAndFilter($messageCatalogue);
+            $messageCatalogue = $this->helper->removeDuplicate($bundle->getName(), $messageCatalogue);
             if (!$isEmptyCatalogue) {
+                $langPackDir = $this->helper->getLangPackDir($systemPath, $namespace);
+
                 $translationsDir = $langPackDir . DIRECTORY_SEPARATOR .
                     $bundle->getName() . DIRECTORY_SEPARATOR . 'translations';
                 $this->filesystem->mkdir($translationsDir);

diff --git a/src/Oro/Bundle/TranslationBundle/Provider/TranslationServiceProvider.php b/src/Oro/Bundle/TranslationBundle/Provider/TranslationServiceProvider.php
@@ -69,15 +69,18 @@ public function __construct(
      * merge generated files over downloaded and upload result back to remote
      *
      * @param array|string[] $dirs
+     * @param bool           $force
+     *
+     * @return void
      */
-    public function update($dirs)
+    public function update($dirs, $force)
     {
         $targetDir  = $this->getTmpDir('oro-trans');
         $pathToSave = $targetDir . DIRECTORY_SEPARATOR . 'update';
         $targetDir  = $targetDir . DIRECTORY_SEPARATOR . self::DEFAULT_SOURCE_LOCALE . DIRECTORY_SEPARATOR;
 
         $isDownloaded = $this->download($pathToSave, [], self::DEFAULT_SOURCE_LOCALE, false);
-        if (!$isDownloaded) {
+        if (!$isDownloaded && !$force) {
             return false;
         }
 
@@ -89,7 +92,7 @@ public function update($dirs)
                 $localContents = file($fileInfo);
                 $remoteFile    = $targetDir . $fileInfo->getRelativePathname();
 
-                if (file_exists($remoteFile)) {
+                if (file_exists($remoteFile) && !$force) {
                     $remoteContents = file($remoteFile);
                     array_shift($remoteContents); // remove dashes from the beginning of file
                 } else {
@@ -115,20 +118,31 @@ public function update($dirs)
     /**
      * Upload translations
      *
-     * @param string $dir
+     * @param string|array $dirs
      * @param string $mode
      *
      * @return mixed
      */
-    public function upload($dir, $mode = 'add')
+    public function upload($dirs, $mode = 'add')
     {
-        $finder = Finder::create()->files()->name('*.yml')->in($dir);
+        $dirs = $this->processDirs($dirs);
+
+        $finder = Finder::create()->files()->name('*.yml')->in($dirs);
 
         /** $file \SplFileInfo */
         $files = [];
         foreach ($finder->files() as $file) {
+            $apiPath = (string)$file;
+            foreach ($dirs as $dir) {
+                if (strpos($apiPath, $dir) !== false) {
+                    $apiPath = str_replace($dir, '', $apiPath);
+                    break;
+                }
+            }
+
             // crowdin understand only "/" as directory separator :)
-            $apiPath         = str_replace([$dir, DIRECTORY_SEPARATOR], ['', '/'], (string)$file);
+            $apiPath = str_replace(DIRECTORY_SEPARATOR, '/', $apiPath);
+
             $files[$apiPath] = (string)$file;
         }
 
@@ -171,7 +185,23 @@ public function download($pathToSave, array $projects, $locale = null, $toApply
             $this->cleanup($targetDir);
         }
 
-        return $isExtracted && $isDownloaded;
+        return $isExtracted;
+    }
+
+    /**
+     * @param string|array $dirs
+     * @return array
+     */
+    protected function processDirs($dirs)
+    {
+        $dirs = is_array($dirs) ? $dirs : [$dirs];
+
+        return array_map(
+            function ($path) {
+                return rtrim($path, DIRECTORY_SEPARATOR);
+            },
+            $dirs
+        );
     }
 
     /**

diff --git a/src/Oro/Bundle/TranslationBundle/Resources/config/services.yml b/src/Oro/Bundle/TranslationBundle/Resources/config/services.yml
@@ -187,3 +187,6 @@ services:
         class: '%oro_translation.strategy.provider.class%'
         arguments:
             - '@oro_translation.strategy.default'
+
+    oro_translation.utils.translation_dump_helper:
+        class: Oro\Bundle\TranslationBundle\Utils\TranslationDumpHelper
diff --git a/src/Oro/Bundle/TranslationBundle/Resources/doc/reference/commands.md b/src/Oro/Bundle/TranslationBundle/Resources/doc/reference/commands.md
@@ -73,7 +73,13 @@ app/console oro:translation:pack --dump OroCRM
 ```
 
 **Upload translation pack:**
-Note: you must call dump command before using this one, otherwise system won't have anything to upload or will upload earlier generated files if there were left.
+Note: you must call dump command before using this one, otherwise system won't have anything to upload or will upload earlier generated files if there were left. This command with "-m update" option without "--force" option does't replace data on crowdin only add strings. 
 ```bash
 app/console oro:translation:pack -i project-key -k abc1234567890c23ee33a767adb --upload OroCRM
 ```
+**Upload and replace translation pack on crowdin:**
+The flag "force" tell that you want to replaced all crowdin translation with local translation
+Note: the --force option works only with "-m update" option
+```bash
+app/console oro:translation:pack -i project-key -k abc1234567890c23ee33a767adb --upload OroCRM -m update --force
+```
diff --git a/src/Oro/Bundle/TranslationBundle/Tests/Unit/Command/OroTranslationPackCommandTest.php b/src/Oro/Bundle/TranslationBundle/Tests/Unit/Command/OroTranslationPackCommandTest.php
@@ -82,10 +82,9 @@ public function executeInputProvider()
             'error if project not specified'        => array(
                 array('--dump' => true),
                 array(
-                    'dump'   => 0,
+                    'dump'   => 1,
                     'upload' => 0
-                ),
-                '\RuntimeException'
+                )
             ),
             'dump action should perform'            => array(
                 array('--dump' => true, 'project' => 'SomeProject'),

diff --git a/src/Oro/Bundle/TranslationBundle/Tests/Unit/Provider/TranslationServiceTest.php b/src/Oro/Bundle/TranslationBundle/Tests/Unit/Provider/TranslationServiceTest.php
@@ -96,7 +96,7 @@ public function testUpload()
     /**
      * @dataProvider updateDataProvider
      */
-    public function testUpdate($isDownloaded, $downloadFileExists)
+    public function testUpdateWithoutForce($isDownloaded, $downloadFileExists)
     {
         $service = $this->getServiceMock(
             ['download', 'upload', 'cleanup'],
@@ -131,7 +131,7 @@ function ($pathToSave) {
                 ->method('download')
                 ->will($this->returnValue(false));
 
-            $this->assertFalse($service->update($dir));
+            $this->assertFalse($service->update($dir, false));
             return;
         }
 
@@ -142,7 +142,7 @@ function ($pathToSave) {
         $service->expects($this->once())
             ->method('cleanup');
 
-        $service->update([$dir]);
+        $service->update([$dir], false);
     }
 
     public function testDownload()