Skip to content

Commit 9a40d6e

Browse files
author
Fred Sung
committed
MAGETWO-38180: Check if the theme is not a base for any virtual theme
- Code style fixed - Move ChildVirtualTheme checking mechanism after validation() to avoid cyclomatic complexity and n-path complexity. - Remove unused objectManager member in test case - Test cases added to cover ChildVirtualTheme check.
1 parent 3efac0f commit 9a40d6e

File tree

2 files changed

+64
-32
lines changed

2 files changed

+64
-32
lines changed

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

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,11 @@ protected function execute(InputInterface $input, OutputInterface $output)
197197
$output->writeln($validationMessages);
198198
return;
199199
}
200+
$childVirtualThemeCheckMessages = $this->checkChildVirtualTheme($themePaths);
201+
if (!empty($childVirtualThemeCheckMessages)) {
202+
$output->writeln($childVirtualThemeCheckMessages);
203+
return;
204+
}
200205
$dependencyMessages = $this->checkDependencies($themePaths);
201206
if (!empty($dependencyMessages)) {
202207
$output->writeln($dependencyMessages);
@@ -240,7 +245,6 @@ private function validate($themePaths)
240245
$messages = [];
241246
$unknownPackages = [];
242247
$unknownThemes = [];
243-
$themeHasChildren = [];
244248
$installedPackages = $this->composer->getRootRequiredPackages();
245249
foreach ($themePaths as $themePath) {
246250
if (array_search($this->getPackageName($themePath), $installedPackages) === false) {
@@ -249,17 +253,6 @@ private function validate($themePaths)
249253
if (!$this->themeCollection->hasTheme($this->themeCollection->getThemeByFullPath($themePath))) {
250254
$unknownThemes[] = $themePath;
251255
}
252-
else {
253-
$theme = $this->themeProvider->getThemeByFullPath($themePath);
254-
if($theme->hasChildThemes())
255-
$themeHasChildren[] = $themePath;
256-
}
257-
}
258-
if(!empty($themeHasChildren)) {
259-
$text = count($themeHasChildren) > 1 ?
260-
' are bases of' : ' is a base of';
261-
$messages[] = '<error>Unable to delete. '
262-
. implode(', ', $themeHasChildren) . $text . ' virtual theme</error>';
263256
}
264257
$unknownPackages = array_diff($unknownPackages, $unknownThemes);
265258
if (!empty($unknownPackages)) {
@@ -299,6 +292,31 @@ private function checkDependencies($themePaths)
299292
return $messages;
300293
}
301294

295+
/**
296+
* Check theme if has child virtual theme
297+
*
298+
* @param string[] $themePaths
299+
* @return string[] $messages
300+
*/
301+
private function checkChildVirtualTheme($themePaths)
302+
{
303+
$messages = [];
304+
$themeHasChildren = [];
305+
foreach ($themePaths as $themePath) {
306+
$theme = $this->themeProvider->getThemeByFullPath($themePath);
307+
if ($theme->hasChildThemes()) {
308+
$themeHasChildren[] = $themePath;
309+
}
310+
}
311+
if (!empty($themeHasChildren)) {
312+
$text = count($themeHasChildren) > 1 ?
313+
' are bases of' : ' is a base of';
314+
$messages[] = '<error>Unable to delete. '
315+
. implode(', ', $themeHasChildren) . $text . ' virtual theme</error>';
316+
}
317+
return $messages;
318+
}
319+
302320
/**
303321
* Get package name of a theme by its full theme path
304322
*

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

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,6 @@ class ThemeUninstallCommandTest extends \PHPUnit_Framework_TestCase
2626
*/
2727
private $maintenanceMode;
2828

29-
/**
30-
* @var \Magento\Framework\ObjectManagerInterface|\PHPUnit_Framework_MockObject_MockObject
31-
*/
32-
private $objectManager;
33-
3429
/**
3530
* @var \Magento\Framework\Filesystem|\PHPUnit_Framework_MockObject_MockObject
3631
*/
@@ -204,8 +199,7 @@ public function testExecuteFailedValidationMixed()
204199
[
205200
['test1/composer.json', null, null, '{"name": "dummy1"}'],
206201
['test2/composer.json', null, null, '{"name": "magento/theme-b"}'],
207-
['test4/composer.json', null, null, '{"name": "dummy2"}'],
208-
['test5/composer.json', null, null, '{"name": "magento/theme-b"}']
202+
['test4/composer.json', null, null, '{"name": "dummy2"}']
209203
]
210204
));
211205
$dirRead->expects($this->any())
@@ -215,8 +209,7 @@ public function testExecuteFailedValidationMixed()
215209
['test1/composer.json', true],
216210
['test2/composer.json', true],
217211
['test3/composer.json', false],
218-
['test4/composer.json', true],
219-
['test5/composer.json', true]
212+
['test4/composer.json', true]
220213
]
221214
));
222215
$this->collection->expects($this->any())
@@ -226,17 +219,11 @@ public function testExecuteFailedValidationMixed()
226219
$this->collection->expects($this->at(3))->method('hasTheme')->willReturn(true);
227220
$this->collection->expects($this->at(5))->method('hasTheme')->willReturn(false);
228221
$this->collection->expects($this->at(7))->method('hasTheme')->willReturn(true);
229-
$this->collection->expects($this->at(9))->method('hasTheme')->willReturn(true);
230222
$this->filesystem->expects($this->any())
231223
->method('getDirectoryRead')
232224
->with(DirectoryList::THEMES)
233225
->willReturn($dirRead);
234-
$theme = $this->getMock('Magento\Theme\Model\Theme', [], [], '', false);
235-
$theme->expects($this->at(1))->method('hasChildThemes')->willReturn(false);
236-
$theme->expects($this->at(2))->method('hasChildThemes')->willReturn(false);
237-
$theme->expects($this->at(3))->method('hasChildThemes')->willReturn(true);
238-
$this->themeProvider->expects($this->any())->method('getThemeByFullPath')->willReturn($theme);
239-
$this->tester->execute(['theme' => ['test1', 'test2', 'test3', 'test4', 'test5']]);
226+
$this->tester->execute(['theme' => ['test1', 'test2', 'test3', 'test4']]);
240227
$this->assertContains(
241228
'test1, test4 are not installed composer packages',
242229
$this->tester->getDisplay()
@@ -249,10 +236,6 @@ public function testExecuteFailedValidationMixed()
249236
'Unknown theme(s): test3' . PHP_EOL,
250237
$this->tester->getDisplay()
251238
);
252-
$this->assertContains(
253-
'test5 is a base of virtual theme',
254-
$this->tester->getDisplay()
255-
);
256239
}
257240

258241
public function setUpPassValidation()
@@ -274,14 +257,45 @@ public function setUpPassValidation()
274257
->method('getThemeByFullPath')
275258
->willReturn($this->getMockForAbstractClass('Magento\Framework\View\Design\ThemeInterface', [], '', false));
276259
$this->collection->expects($this->any())->method('hasTheme')->willReturn(true);
260+
}
261+
262+
public function setupPassChildVirtualThemeCheck()
263+
{
277264
$theme = $this->getMock('Magento\Theme\Model\Theme', [], [], '', false);
278265
$theme->expects($this->any())->method('hasChildThemes')->willReturn(false);
279266
$this->themeProvider->expects($this->any())->method('getThemeByFullPath')->willReturn($theme);
280267
}
281268

269+
public function testExecuteFailedChildVirtualThemeCheck()
270+
{
271+
$this->setUpPassValidation();
272+
$theme = $this->getMock('Magento\Theme\Model\Theme', [], [], '', false);
273+
$theme->expects($this->any())->method('hasChildThemes')->willReturn(true);
274+
$this->themeProvider->expects($this->any())->method('getThemeByFullPath')->willReturn($theme);
275+
$this->tester->execute(['theme' => ['frontend/Magento/a']]);
276+
$this->assertContains(
277+
'Unable to delete. frontend/Magento/a is a base of virtual theme',
278+
$this->tester->getDisplay()
279+
);
280+
}
281+
282+
public function testExecuteFailedMultipleChildVirtualThemeCheck()
283+
{
284+
$this->setUpPassValidation();
285+
$theme = $this->getMock('Magento\Theme\Model\Theme', [], [], '', false);
286+
$theme->expects($this->any())->method('hasChildThemes')->willReturn(true);
287+
$this->themeProvider->expects($this->any())->method('getThemeByFullPath')->willReturn($theme);
288+
$this->tester->execute(['theme' => ['frontend/Magento/a', 'frontend/Magento/b']]);
289+
$this->assertContains(
290+
'Unable to delete. frontend/Magento/a, frontend/Magento/b are bases of virtual theme',
291+
$this->tester->getDisplay()
292+
);
293+
}
294+
282295
public function testExecuteFailedDependencyCheck()
283296
{
284297
$this->setUpPassValidation();
298+
$this->setupPassChildVirtualThemeCheck();
285299
$this->dependencyChecker->expects($this->once())
286300
->method('checkDependencies')
287301
->willReturn(['magento/theme-a' => ['magento/theme-b', 'magento/theme-c']]);

0 commit comments

Comments
 (0)