Skip to content

Commit 31f505d

Browse files
eddielauOlga Kopylova
eddielau
authored and
Olga Kopylova
committed
MAGETWO-33156: Technical constraint is still checked in enable/disable module CLI tool even if no changes will be made
- simplified algorithm
1 parent d9d4ae0 commit 31f505d

File tree

4 files changed

+55
-33
lines changed

4 files changed

+55
-33
lines changed

‎dev/tests/unit/testsuite/Magento/Framework/Module/StatusTest.php

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -156,14 +156,40 @@ public function testSetIsEnabledUnknown()
156156
$this->object->setIsEnabled(true, ['Module_Baz']);
157157
}
158158

159-
public function testGetUnchangedModules()
159+
/**
160+
* @dataProvider getModulesToChangeDataProvider
161+
* @param bool $firstEnabled
162+
* @param bool $secondEnabled
163+
* @param bool $thirdEnabled
164+
* @param bool $isEnabled
165+
* @param string[] $expected
166+
*/
167+
public function testGetModulesToChange($firstEnabled, $secondEnabled, $thirdEnabled, $isEnabled, $expected)
160168
{
161169
$modules = ['Module_Foo' => '', 'Module_Bar' => '', 'Module_Baz' => ''];
162170
$this->loader->expects($this->once())->method('load')->willReturn($modules);
163-
$this->moduleList->expects($this->at(0))->method('has')->with('Module_Foo')->willReturn(true);
164-
$this->moduleList->expects($this->at(1))->method('has')->with('Module_Bar')->willReturn(true);
165-
$this->moduleList->expects($this->at(2))->method('has')->with('Module_Baz')->willReturn(true);
166-
$result = $this->object->getUnchangedModules(true, ['Module_Foo', 'Module_Bar', 'Module_Baz']);
167-
$this->assertEquals(array_keys($modules), $result);
171+
$this->moduleList->expects($this->at(0))->method('has')->with('Module_Foo')->willReturn($firstEnabled);
172+
$this->moduleList->expects($this->at(1))->method('has')->with('Module_Bar')->willReturn($secondEnabled);
173+
$this->moduleList->expects($this->at(2))->method('has')->with('Module_Baz')->willReturn($thirdEnabled);
174+
$result = $this->object->getModulesToChange($isEnabled, ['Module_Foo', 'Module_Bar', 'Module_Baz']);
175+
$this->assertEquals($expected, $result);
176+
}
177+
178+
/**
179+
* @return array
180+
*/
181+
public function getModulesToChangeDataProvider()
182+
{
183+
return [
184+
[true, true, true, true, []],
185+
[true, true, false, true, ['Module_Baz']],
186+
[true, false, true, true, ['Module_Bar']],
187+
[true, false, false, true, ['Module_Bar', 'Module_Baz']],
188+
[false, false, false, true, ['Module_Foo', 'Module_Bar', 'Module_Baz']],
189+
[true, false, false, false, ['Module_Foo']],
190+
[false, true, false, false, ['Module_Bar']],
191+
[false, true, true, false, ['Module_Bar', 'Module_Baz']],
192+
[true, true, true, false, ['Module_Foo', 'Module_Bar', 'Module_Baz']],
193+
];
168194
}
169195
}

‎dev/tests/unit/testsuite/Magento/Setup/Controller/ConsoleControllerTest.php

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ public function testHelpActionNoType()
316316
public function testModuleAction($command, $modules, $isForce, $expectedIsEnabled, $expectedModules)
317317
{
318318
$status = $this->getModuleActionMocks($command, $modules, $isForce, false);
319-
$status->expects($this->once())->method('getUnchangedModules')->willReturn([]);
319+
$status->expects($this->once())->method('getModulesToChange')->willReturn($expectedModules);
320320
if (!$isForce) {
321321
$status->expects($this->once())->method('checkConstraints')->willReturn([]);
322322
}
@@ -352,9 +352,9 @@ public function testModuleActionNoChanges($command, $modules, $isForce, $expecte
352352
{
353353
$status = $this->getModuleActionMocks($command, $modules, $isForce, true);
354354
$status->expects($this->once())
355-
->method('getUnchangedModules')
355+
->method('getModulesToChange')
356356
->with($expectedIsEnabled, $expectedModules)
357-
->willReturn($expectedModules);
357+
->willReturn([]);
358358
$status->expects($this->never())->method('checkConstraints');
359359
$status->expects($this->never())->method('setIsEnabled');
360360
$this->consoleLogger->expects($this->once())->method('log');
@@ -367,7 +367,7 @@ public function testModuleActionNoChanges($command, $modules, $isForce, $expecte
367367
* @param bool $isForce
368368
* @param bool $expectedIsEnabled
369369
* @param string[] $expectedModules
370-
* @param string[] $unchangedModules
370+
* @param string[] $modulesToChange
371371
* @dataProvider moduleActionPartialNoChangesDataProvider
372372
*/
373373
public function testModuleActionPartialNoChanges(
@@ -376,16 +376,16 @@ public function testModuleActionPartialNoChanges(
376376
$isForce,
377377
$expectedIsEnabled,
378378
$expectedModules,
379-
$unchangedModules
379+
$modulesToChange
380380
) {
381381
$status = $this->getModuleActionMocks($command, $modules, $isForce, false);
382-
$status->expects($this->once())->method('getUnchangedModules')->willReturn($unchangedModules);
382+
$status->expects($this->once())->method('getModulesToChange')->willReturn($modulesToChange);
383383
if (!$isForce) {
384384
$status->expects($this->once())->method('checkConstraints')->willReturn([]);
385385
}
386386
$status->expects($this->once())
387387
->method('setIsEnabled')
388-
->with($expectedIsEnabled, array_diff($expectedModules, $unchangedModules));
388+
->with($expectedIsEnabled, $modulesToChange);
389389
$this->consoleLogger->expects($this->once())->method('log');
390390
$this->controller->moduleAction();
391391
}
@@ -402,31 +402,31 @@ public function moduleActionPartialNoChangesDataProvider()
402402
false,
403403
true,
404404
['Module_Foo', 'Module_Bar'],
405-
['Module_Foo'],
405+
['Module_Bar'],
406406
],
407407
[
408408
ConsoleController::CMD_MODULE_ENABLE,
409409
'Module_Foo,Module_Bar',
410410
true,
411411
true,
412412
['Module_Foo', 'Module_Bar'],
413-
['Module_Foo'],
413+
['Module_Bar'],
414414
],
415415
[
416416
ConsoleController::CMD_MODULE_DISABLE,
417417
'Module_Foo,Module_Bar',
418418
false,
419419
false,
420420
['Module_Foo', 'Module_Bar'],
421-
['Module_Foo'],
421+
['Module_Bar'],
422422
],
423423
[
424424
ConsoleController::CMD_MODULE_DISABLE,
425425
'Module_Foo,Module_Bar',
426426
true,
427427
false,
428428
['Module_Foo', 'Module_Bar'],
429-
['Module_Foo'],
429+
['Module_Bar'],
430430
],
431431
];
432432
}
@@ -466,7 +466,7 @@ public function testModuleActionNotAllowed()
466466
false,
467467
false
468468
);
469-
$status->expects($this->once())->method('getUnchangedModules')->willReturn([]);
469+
$status->expects($this->once())->method('getModulesToChange')->willReturn(['Module_Foo', 'Module_Bar']);
470470
$status->expects($this->once())
471471
->method('checkConstraints')
472472
->willReturn(['Circular dependency of Foo and Bar']);

‎lib/internal/Magento/Framework/Module/Status.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -148,24 +148,24 @@ public function setIsEnabled($isEnabled, $modules)
148148
}
149149

150150
/**
151-
* Get a list of modules that will not be changed
151+
* Get a list of modules that will be changed
152152
*
153153
* @param bool $isEnabled
154154
* @param string[] $modules
155155
* @return string[]
156156
*/
157-
public function getUnchangedModules($isEnabled, $modules)
157+
public function getModulesToChange($isEnabled, $modules)
158158
{
159-
$unchanged = [];
159+
$changed = [];
160160
foreach ($this->getAllModules($modules) as $name) {
161161
$currentStatus = $this->list->has($name);
162162
if (in_array($name, $modules)) {
163-
if ($isEnabled == $currentStatus) {
164-
$unchanged[] = $name;
163+
if ($isEnabled != $currentStatus) {
164+
$changed[] = $name;
165165
}
166166
}
167167
}
168-
return $unchanged;
168+
return $changed;
169169
}
170170

171171
/**

‎setup/src/Magento/Setup/Controller/ConsoleController.php

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -487,27 +487,23 @@ public function moduleAction()
487487
/** @var \Magento\Framework\Module\Status $status */
488488
$status = $this->getObjectManager()->create('Magento\Framework\Module\Status');
489489

490-
$unchangedModules = $status->getUnchangedModules($isEnable, $modules);
491-
$modules = array_diff($modules, $unchangedModules);
492-
if (!empty($modules)) {
490+
$modulesToChange = $status->getModulesToChange($isEnable, $modules);
491+
if (!empty($modulesToChange)) {
493492
if (!$request->getParam('force')) {
494-
$constraints = $status->checkConstraints($isEnable, $modules);
493+
$constraints = $status->checkConstraints($isEnable, $modulesToChange);
495494
if ($constraints) {
496495
$message = "Unable to change status of modules because of the following constraints:\n"
497496
. implode("\n", $constraints);
498497
throw new \Magento\Setup\Exception($message);
499498
}
500499
}
501-
$status->setIsEnabled($isEnable, $modules);
500+
$status->setIsEnabled($isEnable, $modulesToChange);
502501
if ($isEnable) {
503502
$message = 'The following modules have been enabled:';
504503
} else {
505504
$message = 'The following modules have been disabled:';
506505
}
507-
$message .= ' ' . implode(', ', $modules);
508-
if (!empty($unchangedModules)) {
509-
$message .= "\nThe following modules have no changes: " . implode(', ', $unchangedModules);
510-
}
506+
$message .= ' ' . implode(', ', $modulesToChange);
511507
} else {
512508
$message = 'There have been no changes to any modules.';
513509
}

0 commit comments

Comments
 (0)