MAGETWO-38180: Check if the theme is not a base for any virtual theme

 - Code updated from CR feedback.

Author: Fred Sung

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

@@ -24,6 +24,7 @@
 use Magento\Framework\App\DeploymentConfig;
 use Magento\Framework\Setup\BackupRollbackFactory;
 use Magento\Framework\App\Filesystem\DirectoryList;
+use Magento\Framework\Exception\LocalizedException;
 
 /**
  * Command for uninstalling theme and backup-code feature
@@ -41,39 +42,51 @@ class ThemeUninstallCommand extends Command
     const INPUT_KEY_CLEAR_STATIC_CONTENT = 'clear-static-content';
 
     /**
+     * Deployment Configuration
+     *
      * @var DeploymentConfig
      */
     private $deploymentConfig;
 
     /**
+     * Maintenance Mode
+     *
      * @var MaintenanceMode
      */
     private $maintenanceMode;
 
     /**
+     * Composer general dependency checker
+     *
      * @var DependencyChecker
      */
     private $dependencyChecker;
 
     /**
+     * Root composer.json information
+     *
      * @var ComposerInformation
      */
     private $composer;
 
     /**
+     * File operation to read theme directory
+     *
      * @var Filesystem
      */
     private $filesystem;
 
     /**
+     * Code remover
+     *
      * @var Remove
      */
     private $remove;
 
     /**
      * Theme collection in filesystem
      *
-     * @var \Magento\Theme\Model\Theme\Collection
+     * @var Collection
      */
     private $themeCollection;
 
@@ -85,21 +98,29 @@ class ThemeUninstallCommand extends Command
     private $themeProvider;
 
     /**
+     * Application States
+     *
      * @var State
      */
     private $appState;
 
     /**
+     * System cache model
+     *
      * @var Cache
      */
     private $cache;
 
     /**
+     * Cleaning up application state service
+     *
      * @var State\CleanupFiles
      */
     private $cleanupFiles;
 
     /**
+     * BackupRollback factory
+     *
      * @var BackupRollbackFactory
      */
     private $backupRollbackFactory;
@@ -119,7 +140,7 @@ class ThemeUninstallCommand extends Command
      * @param Remove $remove
      * @param State $appState
      * @param BackupRollbackFactory $backupRollbackFactory
-     * @throws \Magento\Framework\Exception\LocalizedException
+     * @throws LocalizedException
      */
     public function __construct(
         Cache $cache,
@@ -256,7 +277,7 @@ private function validate($themePaths)
         $unknownPackages = array_diff($unknownPackages, $unknownThemes);
         if (!empty($unknownPackages)) {
             $text = count($unknownPackages) > 1 ?
-                ' are not installed composer packages' : ' is not an installed composer package';
+                ' are not installed Composer packages' : ' is not an installed Composer package';
             $messages[] = '<error>' . implode(', ', $unknownPackages) . $text . '</error>';
         }
         if (!empty($unknownThemes)) {
@@ -308,9 +329,8 @@ private function checkChildVirtualTheme($themePaths)
             }
         }
         if (!empty($themeHasChildren)) {
-            $text = count($themeHasChildren) > 1 ?
-                ' are bases of' : ' is a base of';
-            $messages[] = '<error>Unable to delete. '
+            $text = count($themeHasChildren) > 1 ? ' are parents of' : ' is a parent of';
+            $messages[] = '<error>Unable to uninstall. '
                 . implode(', ', $themeHasChildren) . $text . ' virtual theme</error>';
         }
         return $messages;

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

@@ -154,16 +154,13 @@ public function testExecuteFailedValidationNotPackage()
             ->method('getThemeByFullPath')
             ->willReturn($this->getMockForAbstractClass('Magento\Framework\View\Design\ThemeInterface', [], '', false));
         $this->collection->expects($this->any())->method('hasTheme')->willReturn(true);
-        $theme = $this->getMock('Magento\Theme\Model\Theme', [], [], '', false);
-        $this->themeProvider->expects($this->any())->method('getThemeByFullPath')->willReturn($theme);
-        $this->themeProvider->expects($this->any())->method('hasChildThemes')->willReturn(false);
         $this->tester->execute(['theme' => ['test1', 'test2']]);
         $this->assertContains(
-            'test1 is not an installed composer package',
+            'test1 is not an installed Composer package',
             $this->tester->getDisplay()
         );
         $this->assertNotContains(
-            'test2 is not an installed composer package',
+            'test2 is not an installed Composer package',
             $this->tester->getDisplay()
         );
     }
@@ -225,11 +222,11 @@ public function testExecuteFailedValidationMixed()
             ->willReturn($dirRead);
         $this->tester->execute(['theme' => ['test1', 'test2', 'test3', 'test4']]);
         $this->assertContains(
-            'test1, test4 are not installed composer packages',
+            'test1, test4 are not installed Composer packages',
             $this->tester->getDisplay()
         );
         $this->assertNotContains(
-            'test2 is not an installed composer package',
+            'test2 is not an installed Composer package',
             $this->tester->getDisplay()
         );
         $this->assertContains(
@@ -274,7 +271,7 @@ public function testExecuteFailedChildVirtualThemeCheck()
         $this->themeProvider->expects($this->any())->method('getThemeByFullPath')->willReturn($theme);
         $this->tester->execute(['theme' => ['frontend/Magento/a']]);
         $this->assertContains(
-            'Unable to delete. frontend/Magento/a is a base of virtual theme',
+            'Unable to uninstall. frontend/Magento/a is a parent of virtual theme',
             $this->tester->getDisplay()
         );
     }
@@ -287,7 +284,7 @@ public function testExecuteFailedMultipleChildVirtualThemeCheck()
         $this->themeProvider->expects($this->any())->method('getThemeByFullPath')->willReturn($theme);
         $this->tester->execute(['theme' => ['frontend/Magento/a', 'frontend/Magento/b']]);
         $this->assertContains(
-            'Unable to delete. frontend/Magento/a, frontend/Magento/b are bases of virtual theme',
+            'Unable to uninstall. frontend/Magento/a, frontend/Magento/b are parents of virtual theme',
             $this->tester->getDisplay()
         );
     }