Skip to content

Commit 40f8a0c

Browse files
eddielaueddielau
committed
MAGETWO-38567: Dependency Validation of Physical Non-Composer Theme from Uninstalled Theme
- added dependency check for unregistered themes
1 parent 871bf0b commit 40f8a0c

File tree

5 files changed

+58
-63
lines changed

5 files changed

+58
-63
lines changed

‎app/code/Magento/Theme/Block/Adminhtml/System/Design/Theme/Edit.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ protected function _prepareLayout()
7070
}
7171

7272
if ($theme->isDeletable()) {
73-
if ($theme->hasVirtualChildThemes()) {
73+
if ($theme->hasChildThemes()) {
7474
$message = __('Are you sure you want to delete this theme?');
7575
$onClick = sprintf(
7676
"deleteConfirm('%s', '%s')",

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

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
use Magento\Framework\Composer\DependencyChecker;
1515
use Magento\Framework\Composer\Remove;
1616
use Magento\Framework\Filesystem;
17-
use Magento\Theme\Model\Theme\Collection;
17+
use Magento\Theme\Model\Theme\Data\Collection;
1818
use Magento\Theme\Model\Theme\ThemeProvider;
1919
use Symfony\Component\Console\Command\Command;
2020
use Symfony\Component\Console\Input\InputInterface;
@@ -305,12 +305,13 @@ private function checkChildTheme($themePaths)
305305
$messages = [];
306306
$themeHasVirtualChildren = [];
307307
$themeHasPhysicalChildren = [];
308+
$parentChildMap = $this->getParentChildThemeMap();
308309
foreach ($themePaths as $themePath) {
309310
$theme = $this->themeProvider->getThemeByFullPath($themePath);
310-
if ($theme->hasVirtualChildThemes()) {
311+
if ($theme->hasChildThemes()) {
311312
$themeHasVirtualChildren[] = $themePath;
312313
}
313-
if ($theme->hasPhysicalChildThemes()) {
314+
if (isset($parentChildMap[$themePath])) {
314315
$themeHasPhysicalChildren[] = $themePath;
315316
}
316317
}
@@ -327,6 +328,24 @@ private function checkChildTheme($themePaths)
327328
return $messages;
328329
}
329330

331+
/**
332+
* Obtain a parent theme -> children themes map from the filesystem
333+
*
334+
* @return array
335+
*/
336+
private function getParentChildThemeMap()
337+
{
338+
$map = [];
339+
$this->themeCollection->addDefaultPattern('*');
340+
/** @var \Magento\Theme\Model\Theme\Data $theme */
341+
foreach ($this->themeCollection as $theme) {
342+
if ($theme->getParentTheme()) {
343+
$map[$theme->getParentTheme()->getFullPath()][] = $theme->getFullPath();
344+
}
345+
}
346+
return $map;
347+
}
348+
330349
/**
331350
* Get package name of a theme by its full theme path
332351
*

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

Lines changed: 8 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -201,38 +201,18 @@ public function isVisible()
201201
}
202202

203203
/**
204-
* Check is theme has virtual child themes
204+
* Check is theme has child virtual themes
205205
*
206206
* @return bool
207207
*/
208-
public function hasVirtualChildThemes()
208+
public function hasChildThemes()
209209
{
210-
return $this->hasChildThemes(true);
211-
}
212-
213-
/**
214-
* Check is theme has physical child themes
215-
*
216-
* @return bool
217-
*/
218-
public function hasPhysicalChildThemes()
219-
{
220-
return $this->hasChildThemes(false);
221-
}
222-
223-
/**
224-
* Helper method for checking child themes
225-
*
226-
* @param $isVirtual
227-
* @return bool
228-
*/
229-
private function hasChildThemes($isVirtual)
230-
{
231-
$type = $isVirtual ? self::TYPE_VIRTUAL : self::TYPE_PHYSICAL;
232-
return (bool)$this->getCollection()
233-
->addTypeFilter($type)
234-
->addFieldToFilter('parent_id', ['eq' => $this->getId()])
235-
->getSize();
210+
return (bool)$this->getCollection()->addTypeFilter(
211+
self::TYPE_VIRTUAL
212+
)->addFieldToFilter(
213+
'parent_id',
214+
['eq' => $this->getId()]
215+
)->getSize();
236216
}
237217

238218
/**

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

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ class ThemeUninstallCommandTest extends \PHPUnit_Framework_TestCase
3232
private $dependencyChecker;
3333

3434
/**
35-
* @var \Magento\Theme\Model\Theme\Collection|\PHPUnit_Framework_MockObject_MockObject
35+
* @var \Magento\Theme\Model\Theme\Data\Collection|\PHPUnit_Framework_MockObject_MockObject
3636
*/
3737
private $collection;
3838

@@ -86,7 +86,7 @@ public function setUp()
8686
'',
8787
false
8888
);
89-
$this->collection = $this->getMock('Magento\Theme\Model\Theme\Collection', [], [], '', false);
89+
$this->collection = $this->getMock('Magento\Theme\Model\Theme\Data\Collection', [], [], '', false);
9090
$this->remove = $this->getMock('Magento\Framework\Composer\Remove', [], [], '', false);
9191
$this->cache = $this->getMock('Magento\Framework\App\Cache', [], [], '', false);
9292
$this->cleanupFiles = $this->getMock('Magento\Framework\App\State\CleanupFiles', [], [], '', false);
@@ -238,9 +238,9 @@ public function setUpPassValidation()
238238
public function setupPassChildThemeCheck()
239239
{
240240
$theme = $this->getMock('Magento\Theme\Model\Theme', [], [], '', false);
241-
$theme->expects($this->any())->method('hasVirtualChildThemes')->willReturn(false);
242-
$theme->expects($this->any())->method('hasPhysicalChildThemes')->willReturn(false);
241+
$theme->expects($this->any())->method('hasChildThemes')->willReturn(false);
243242
$this->themeProvider->expects($this->any())->method('getThemeByFullPath')->willReturn($theme);
243+
$this->collection->expects($this->any())->method('getIterator')->willReturn(new \ArrayIterator([]));
244244
}
245245

246246
/**
@@ -255,9 +255,25 @@ public function testExecuteFailedChildThemeCheck($hasVirtual, $hasPhysical, arra
255255
{
256256
$this->setUpPassValidation();
257257
$theme = $this->getMock('Magento\Theme\Model\Theme', [], [], '', false);
258-
$theme->expects($this->any())->method('hasVirtualChildThemes')->willReturn($hasVirtual);
259-
$theme->expects($this->any())->method('hasPhysicalChildThemes')->willReturn($hasPhysical);
258+
$theme->expects($this->any())->method('hasChildThemes')->willReturn($hasVirtual);
259+
$parentThemeA = $this->getMock('Magento\Theme\Model\Theme', [], [], '', false);
260+
$parentThemeA->expects($this->any())->method('getFullPath')->willReturn('frontend/Magento/a');
261+
$parentThemeB = $this->getMock('Magento\Theme\Model\Theme', [], [], '', false);
262+
$parentThemeB->expects($this->any())->method('getFullPath')->willReturn('frontend/Magento/b');
263+
$childThemeC = $this->getMock('Magento\Theme\Model\Theme', [], [], '', false);
264+
$childThemeC->expects($this->any())->method('getFullPath')->willReturn('frontend/Magento/c');
265+
$childThemeD = $this->getMock('Magento\Theme\Model\Theme', [], [], '', false);
266+
$childThemeD->expects($this->any())->method('getFullPath')->willReturn('frontend/Magento/d');
267+
268+
if ($hasPhysical) {
269+
$childThemeC->expects($this->any())->method('getParentTheme')->willReturn($parentThemeA);
270+
$childThemeD->expects($this->any())->method('getParentTheme')->willReturn($parentThemeB);
271+
}
272+
260273
$this->themeProvider->expects($this->any())->method('getThemeByFullPath')->willReturn($theme);
274+
$this->collection->expects($this->any())
275+
->method('getIterator')
276+
->willReturn(new \ArrayIterator([$childThemeC, $childThemeD]));
261277
$this->tester->execute($input);
262278
$this->assertContains(
263279
$expected,

‎dev/tests/integration/testsuite/Magento/Theme/Model/ThemeTest.php

Lines changed: 5 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -43,41 +43,21 @@ protected function _getThemeValidData()
4343
}
4444

4545
/**
46-
* Test theme on virtual child relations
46+
* Test theme on child relations
4747
*/
48-
public function testVirtualChildRelation()
48+
public function testChildRelation()
4949
{
5050
/** @var $theme \Magento\Framework\View\Design\ThemeInterface */
5151
$theme = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get(
5252
'Magento\Framework\View\Design\ThemeInterface'
5353
);
54-
$virtualCollection = $theme->getCollection()
54+
$collection = $theme->getCollection()
5555
->addTypeFilter(\Magento\Framework\View\Design\ThemeInterface::TYPE_VIRTUAL);
5656
/** @var $currentTheme \Magento\Framework\View\Design\ThemeInterface */
57-
foreach ($virtualCollection as $currentTheme) {
57+
foreach ($collection as $currentTheme) {
5858
$parentTheme = $currentTheme->getParentTheme();
5959
if (!empty($parentTheme)) {
60-
$this->assertTrue($parentTheme->hasVirtualChildThemes());
61-
}
62-
}
63-
}
64-
65-
/**
66-
* Test theme on physical child relations
67-
*/
68-
public function testPhysicalChildRelation()
69-
{
70-
/** @var $theme \Magento\Framework\View\Design\ThemeInterface */
71-
$theme = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get(
72-
'Magento\Framework\View\Design\ThemeInterface'
73-
);
74-
$physicalCollection = $theme->getCollection()
75-
->addTypeFilter(\Magento\Framework\View\Design\ThemeInterface::TYPE_PHYSICAL);
76-
/** @var $currentTheme \Magento\Framework\View\Design\ThemeInterface */
77-
foreach ($physicalCollection as $currentTheme) {
78-
$parentTheme = $currentTheme->getParentTheme();
79-
if (!empty($parentTheme)) {
80-
$this->assertTrue($parentTheme->hasPhysicalChildThemes());
60+
$this->assertTrue($parentTheme->hasChildThemes());
8161
}
8262
}
8363
}

0 commit comments

Comments
 (0)