Merge remote-tracking branch...

Merge remote-tracking branch 'origin/MAGETWO-38833-fix-error-messages-theme-uninstall' into PR_Branch

app/code/Magento/Theme/Console/Command/ThemeUninstallCommand.php

@@ -194,25 +194,24 @@ class ThemeUninstallCommand extends Command
      */
     protected function execute(InputInterface $input, OutputInterface $output)
     {
+        $messages = [];
         $themePaths = $input->getArgument(self::INPUT_KEY_THEMES);
-        $validationMessages = $this->validate($themePaths);
-        if (!empty($validationMessages)) {
-            $output->writeln($validationMessages);
-            return;
-        }
-        $isThemeInUseMessages = $this->themeValidator->validateIsThemeInUse($themePaths);
-        if (!empty($isThemeInUseMessages)) {
-            $output->writeln($isThemeInUseMessages);
-            return;
-        }
-        $childThemeCheckMessages = $this->checkChildTheme($themePaths);
-        if (!empty($childThemeCheckMessages)) {
-            $output->writeln($childThemeCheckMessages);
+        $messages = array_merge($messages, $this->validate($themePaths));
+        if (!empty($messages)) {
+            $output->writeln($messages);
             return;
         }
-        $dependencyMessages = $this->checkDependencies($themePaths);
-        if (!empty($dependencyMessages)) {
-            $output->writeln($dependencyMessages);
+        $messages = array_merge(
+            $messages,
+            $this->themeValidator->validateIsThemeInUse($themePaths),
+            $this->checkChildTheme($themePaths),
+            $this->checkDependencies($themePaths)
+        );
+        if (!empty($messages)) {
+            $output->writeln(
+                '<error>Unable to uninstall. Please resolve the following issues:</error>'
+                . PHP_EOL . implode(PHP_EOL, $messages)
+            );
             return;
         }
 
@@ -290,8 +289,8 @@ class ThemeUninstallCommand extends Command
         foreach ($dependencies as $package => $dependingPackages) {
             if (!empty($dependingPackages)) {
                 $messages[] =
-                    '<error>Cannot uninstall ' . $packageToPath[$package] .
-                    " because the following package(s) depend on it:</error>" .
+                    '<error>' . $packageToPath[$package] .
+                    " has the following dependent package(s):</error>" .
                     PHP_EOL . "\t<error>" . implode('</error>' . PHP_EOL . "\t<error>", $dependingPackages)
                     . "</error>";
             }
@@ -322,13 +321,13 @@ class ThemeUninstallCommand extends Command
         }
         if (!empty($themeHasVirtualChildren)) {
             $text = count($themeHasVirtualChildren) > 1 ? ' are parents of' : ' is a parent of';
-            $messages[] = '<error>Unable to uninstall. '
-                . implode(', ', $themeHasVirtualChildren) . $text . ' virtual theme</error>';
+            $messages[] = '<error>' . implode(', ', $themeHasVirtualChildren) . $text . ' virtual theme.'
+                . ' Parent themes cannot be uninstalled.</error>';
         }
         if (!empty($themeHasPhysicalChildren)) {
             $text = count($themeHasPhysicalChildren) > 1 ? ' are parents of' : ' is a parent of';
-            $messages[] = '<error>Unable to uninstall. '
-                . implode(', ', $themeHasPhysicalChildren) . $text . ' physical theme</error>';
+            $messages[] = '<error>' . implode(', ', $themeHasPhysicalChildren) . $text . ' physical theme.'
+                . ' Parent themes cannot be uninstalled.</error>';
         }
         return $messages;
     }

app/code/Magento/Theme/Model/ThemeValidator.php

@@ -75,15 +75,15 @@ class ThemeValidator
         foreach ($configData as $row) {
             switch($row['scope']) {
                 case 'default':
-                    $messages[] = $themesById[$row['value']] . ' is in use in default config';
+                    $messages[] = '<error>' . $themesById[$row['value']] . ' is in use in default config' . '</error>';
                     break;
                 case ScopeInterface::SCOPE_WEBSITES:
-                    $messages[] = $themesById[$row['value']] . ' is in use in website '
-                        . $this->storeManager->getWebsite($row['scope_id'])->getName();
+                    $messages[] = '<error>' . $themesById[$row['value']] . ' is in use in website '
+                        . $this->storeManager->getWebsite($row['scope_id'])->getName() . '</error>';
                     break;
                 case ScopeInterface::SCOPE_STORES:
-                    $messages[] = $themesById[$row['value']] . ' is in use in store '
-                        . $this->storeManager->getStore($row['scope_id'])->getName();
+                    $messages[] = '<error>' . $themesById[$row['value']] . ' is in use in store '
+                        . $this->storeManager->getStore($row['scope_id'])->getName() . '</error>';
                     break;
             }
         }

app/code/Magento/Theme/Test/Unit/Console/Command/ThemeUninstallCommandTest.php

@@ -250,6 +250,33 @@ class ThemeUninstallCommandTest extends \PHPUnit_Framework_TestCase
         $this->collection->expects($this->any())->method('getIterator')->willReturn(new \ArrayIterator([]));
     }
 
+    public function setupPassThemeInUseCheck()
+    {
+        $this->themeValidator->expects($this->once())->method('validateIsThemeInUse')->willReturn([]);
+    }
+
+    public function setupPassDependencyCheck()
+    {
+        $this->dependencyChecker->expects($this->once())->method('checkDependencies')->willReturn([]);
+    }
+
+    public function testExecuteFailedThemeInUseCheck()
+    {
+        $this->setUpPassValidation();
+        $this->setupPassChildThemeCheck();
+        $this->setupPassDependencyCheck();
+        $this->themeValidator
+            ->expects($this->once())
+            ->method('validateIsThemeInUse')
+            ->willReturn(['frontend/Magento/a is in use in default config']);
+        $this->tester->execute(['theme' => ['frontend/Magento/a']]);
+        $this->assertEquals(
+            'Unable to uninstall. Please resolve the following issues:' . PHP_EOL
+            . 'frontend/Magento/a is in use in default config' . PHP_EOL,
+            $this->tester->getDisplay()
+        );
+    }
+
     /**
      * @dataProvider executeFailedChildThemeCheckDataProvider
      * @param bool $hasVirtual
@@ -261,6 +288,8 @@ class ThemeUninstallCommandTest extends \PHPUnit_Framework_TestCase
     public function testExecuteFailedChildThemeCheck($hasVirtual, $hasPhysical, array $input, $expected)
     {
         $this->setUpPassValidation();
+        $this->setupPassThemeInUseCheck();
+        $this->setupPassDependencyCheck();
         $theme = $this->getMock('Magento\Theme\Model\Theme', [], [], '', false);
         $theme->expects($this->any())->method('hasChildThemes')->willReturn($hasVirtual);
         $parentThemeA = $this->getMock('Magento\Theme\Model\Theme', [], [], '', false);
@@ -282,10 +311,7 @@ class ThemeUninstallCommandTest extends \PHPUnit_Framework_TestCase
             ->method('getIterator')
             ->willReturn(new \ArrayIterator([$childThemeC, $childThemeD]));
         $this->tester->execute($input);
-        $this->assertContains(
-            $expected,
-            $this->tester->getDisplay()
-        );
+        $this->assertContains($expected, $this->tester->getDisplay());
     }
 
     /**
@@ -298,65 +324,65 @@ class ThemeUninstallCommandTest extends \PHPUnit_Framework_TestCase
                 true,
                 false,
                 ['theme' => ['frontend/Magento/a']],
-                'Unable to uninstall. frontend/Magento/a is a parent of virtual theme'
+                'Unable to uninstall. Please resolve the following issues:' . PHP_EOL
+                . 'frontend/Magento/a is a parent of virtual theme. Parent themes cannot be uninstalled.'
             ],
             [
                 true,
                 false,
                 ['theme' => ['frontend/Magento/a', 'frontend/Magento/b']],
-                'Unable to uninstall. frontend/Magento/a, frontend/Magento/b are parents of virtual theme'
+                'Unable to uninstall. Please resolve the following issues:' . PHP_EOL .
+                'frontend/Magento/a, frontend/Magento/b are parents of virtual theme.'
+                . ' Parent themes cannot be uninstalled.'
             ],
             [
                 false,
                 true,
                 ['theme' => ['frontend/Magento/a']],
-                'Unable to uninstall. frontend/Magento/a is a parent of physical theme'
+                'Unable to uninstall. Please resolve the following issues:' . PHP_EOL .
+                'frontend/Magento/a is a parent of physical theme. Parent themes cannot be uninstalled.'
             ],
             [
                 false,
                 true,
                 ['theme' => ['frontend/Magento/a', 'frontend/Magento/b']],
-                'Unable to uninstall. frontend/Magento/a, frontend/Magento/b are parents of physical theme'
+                'Unable to uninstall. Please resolve the following issues:' . PHP_EOL .
+                'frontend/Magento/a, frontend/Magento/b are parents of physical theme.'
+                . ' Parent themes cannot be uninstalled.'
             ],
             [
                 true,
                 true,
                 ['theme' => ['frontend/Magento/a']],
-                'Unable to uninstall. frontend/Magento/a is a parent of virtual theme' . PHP_EOL .
-                'Unable to uninstall. frontend/Magento/a is a parent of physical theme'
+                'Unable to uninstall. Please resolve the following issues:' . PHP_EOL .
+                'frontend/Magento/a is a parent of virtual theme. Parent themes cannot be uninstalled.' . PHP_EOL .
+                'frontend/Magento/a is a parent of physical theme. Parent themes cannot be uninstalled.'
             ],
             [
                 true,
                 true,
                 ['theme' => ['frontend/Magento/a', 'frontend/Magento/b']],
-                'Unable to uninstall. frontend/Magento/a, frontend/Magento/b are parents of virtual theme' . PHP_EOL .
-                'Unable to uninstall. frontend/Magento/a, frontend/Magento/b are parents of physical theme'
+                'frontend/Magento/a, frontend/Magento/b are parents of virtual theme.'
+                . ' Parent themes cannot be uninstalled.' . PHP_EOL .
+                'frontend/Magento/a, frontend/Magento/b are parents of physical theme.'
+                . ' Parent themes cannot be uninstalled.'
             ],
         ];
     }
 
-    public function testExecuteFailedThemeInUseCheck()
-    {
-        $this->setUpPassValidation();
-        $this->themeValidator
-            ->expects($this->once())
-            ->method('validateIsThemeInUse')
-            ->willReturn(['frontend/Magento/a is in use in default config']);
-        $this->tester->execute(['theme' => ['frontend/Magento/a']]);
-        $this->assertEquals('frontend/Magento/a is in use in default config' . PHP_EOL, $this->tester->getDisplay());
-    }
-
     public function testExecuteFailedDependencyCheck()
     {
         $this->setUpPassValidation();
+        $this->setupPassThemeInUseCheck();
         $this->setupPassChildThemeCheck();
         $this->dependencyChecker->expects($this->once())
             ->method('checkDependencies')
             ->willReturn(['magento/theme-a' => ['magento/theme-b', 'magento/theme-c']]);
         $this->tester->execute(['theme' => ['frontend/Magento/a']]);
         $this->assertContains(
-            'Cannot uninstall frontend/Magento/a because the following package(s) ' .
-            'depend on it:' . PHP_EOL . "\tmagento/theme-b" . PHP_EOL . "\tmagento/theme-c",
+            'Unable to uninstall. Please resolve the following issues:' . PHP_EOL .
+            'frontend/Magento/a has the following dependent package(s):'
+            . PHP_EOL . "\tmagento/theme-b" . PHP_EOL . "\tmagento/theme-c",
             $this->tester->getDisplay()
         );
     }
@@ -364,8 +390,9 @@ class ThemeUninstallCommandTest extends \PHPUnit_Framework_TestCase
     public function setUpExecute()
     {
         $this->setUpPassValidation();
+        $this->setupPassThemeInUseCheck();
         $this->setupPassChildThemeCheck();
-        $this->dependencyChecker->expects($this->once())->method('checkDependencies')->willReturn([]);
+        $this->setupPassDependencyCheck();
         $this->remove->expects($this->once())->method('remove');
         $this->cache->expects($this->once())->method('clean');
         $theme = $this->getMock('Magento\Theme\Model\Theme', [], [], '', false);

app/code/Magento/Theme/Test/Unit/Model/ThemeValidatorTest.php

@@ -81,9 +81,9 @@ class ThemeValidatorTest extends \PHPUnit_Framework_TestCase
         $result = $this->themeValidator->validateIsThemeInUse(['frontend/Magento/a']);
         $this->assertEquals(
             [
-                'frontend/Magento/a is in use in default config',
-                'frontend/Magento/a is in use in website websiteA',
-                'frontend/Magento/a is in use in store storeA'
+                '<error>frontend/Magento/a is in use in default config</error>',
+                '<error>frontend/Magento/a is in use in website websiteA</error>',
+                '<error>frontend/Magento/a is in use in store storeA</error>'
             ],
             $result
         );