Skip to content

Commit 514992e

Browse files
eddielauOlga Kopylova
eddielau
authored and
Olga Kopylova
committed
MAGETWO-33157: Conflict restriction checking is missing version checking in enable/disable module CLI
- added more verbose message when checking conflicts
1 parent 3ccbd2f commit 514992e

File tree

4 files changed

+25
-19
lines changed

4 files changed

+25
-19
lines changed

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public function checkConflictWhenEnableModuleDataProvider()
4949
[['Vendor_A', ['Vendor_B' => '0.1']], ['Vendor_B', []]],
5050
['Vendor_A'],
5151
['Vendor_B'],
52-
['Vendor_B' => ['Vendor_A']]
52+
['Vendor_B' => ['Vendor_A conflicts with Vendor_B version 0.1 (current 0.1)']]
5353
],
5454
[
5555
[['Vendor_A', ['Vendor_B' => '0.1']], ['Vendor_B', []]],
@@ -61,7 +61,7 @@ public function checkConflictWhenEnableModuleDataProvider()
6161
[['Vendor_B', ['Vendor_A' => '0.1']], ['Vendor_A', []]],
6262
['Vendor_A'],
6363
['Vendor_B'],
64-
['Vendor_B' => ['Vendor_A']]
64+
['Vendor_B' => ['Vendor_B conflicts with Vendor_A version 0.1 (current 0.1)']]
6565
],
6666
[
6767
[['Vendor_B', ['Vendor_A' => '0.1']], ['Vendor_A', []]],
@@ -85,25 +85,25 @@ public function checkConflictWhenEnableModuleDataProvider()
8585
[['Vendor_A', ['Vendor_C' => '0.1']], ['Vendor_B', []], ['Vendor_C', []]],
8686
['Vendor_A'],
8787
['Vendor_B', 'Vendor_C'],
88-
['Vendor_B' => [], 'Vendor_C' => ['Vendor_A']]
88+
['Vendor_B' => [], 'Vendor_C' => ['Vendor_A conflicts with Vendor_C version 0.1 (current 0.1)']]
8989
],
9090
[
9191
[['Vendor_A', []], ['Vendor_B', ['Vendor_C' => '0.1']], ['Vendor_C', []]],
9292
['Vendor_A'],
9393
['Vendor_B', 'Vendor_C'],
94-
['Vendor_B' => ['Vendor_C'], 'Vendor_C' => ['Vendor_B']]
94+
['Vendor_B' => ['Vendor_B conflicts with Vendor_C version 0.1 (current 0.1)'], 'Vendor_C' => ['Vendor_B conflicts with Vendor_C version 0.1 (current 0.1)']]
9595
],
9696
[
9797
[['Vendor_A', ['Vendor_B' => '>=0.1']], ['Vendor_B', []]],
9898
['Vendor_A'],
9999
['Vendor_B'],
100-
['Vendor_B' => ['Vendor_A']]
100+
['Vendor_B' => ['Vendor_A conflicts with Vendor_B version >=0.1 (current 0.1)']]
101101
],
102102
[
103103
[['Vendor_A', ['Vendor_B' => '~0.1']], ['Vendor_B', []]],
104104
['Vendor_A'],
105105
['Vendor_B'],
106-
['Vendor_B' => ['Vendor_A']]
106+
['Vendor_B' => ['Vendor_A conflicts with Vendor_B version ~0.1 (current 0.1)']]
107107
],
108108
];
109109
}
@@ -131,7 +131,7 @@ public function testCheckConflictWhenEnableModuleDifferentVersion()
131131
->will($this->returnValue($packageInfoMock));
132132
$conflictChecker = new ConflictChecker($moduleListMock, $packageInfoFactoryMock);
133133
$this->assertEquals(
134-
['Vendor_C' => ['Vendor_A']],
134+
['Vendor_C' => ['Vendor_C conflicts with Vendor_A version >=0.2,<0.3 (current 0.2)']],
135135
$conflictChecker->checkConflictsWhenEnableModules(['Vendor_C'])
136136
);
137137
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,9 @@ public function testCheckConstraintsEnableNotAllowed()
9292
"Module_Baz: Module_Foo->Module_Baz",
9393
'Cannot enable Module_Bar, depending on disabled modules:',
9494
"Module_Baz: Module_Bar->Module_Baz",
95-
'Cannot enable Module_Foo, conflicting modules:',
95+
'Cannot enable Module_Foo, conflicting with other modules:',
9696
"Module_Bar",
97-
'Cannot enable Module_Bar, conflicting modules:',
97+
'Cannot enable Module_Bar, conflicting with other modules:',
9898
"Module_Foo",
9999
];
100100
$this->assertEquals($expect, $result);

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

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,9 @@ public function checkConflictsWhenEnableModules($moduleNames)
5252
foreach ($moduleNames as $moduleName) {
5353
$conflicts = [];
5454
foreach ($enabledModules as $enabledModule) {
55-
if ($this->checkIfConflict($enabledModule, $moduleName)) {
56-
$conflicts[] = $enabledModule;
55+
$messages = $this->getConflictMessages($enabledModule, $moduleName);
56+
if (!empty($messages)) {
57+
$conflicts[] = implode("\n", $messages);
5758
}
5859
}
5960
$conflictsAll[$moduleName] = $conflicts;
@@ -62,14 +63,15 @@ public function checkConflictsWhenEnableModules($moduleNames)
6263
}
6364

6465
/**
65-
* Check if two modules are conflicted
66+
* Check if two modules are conflicted and get the message for display
6667
*
6768
* @param string $moduleA
6869
* @param string $moduleB
69-
* @return bool
70+
* @return string[]
7071
*/
71-
private function checkIfConflict($moduleA, $moduleB)
72+
private function getConflictMessages($moduleA, $moduleB)
7273
{
74+
$messages = [];
7375
$versionParser = new VersionParser();
7476
if (isset($this->packageInfo->getConflict($moduleB)[$moduleA]) &&
7577
$this->packageInfo->getConflict($moduleB)[$moduleA] &&
@@ -78,7 +80,9 @@ private function checkIfConflict($moduleA, $moduleB)
7880
$constraintA = $versionParser->parseConstraints($this->packageInfo->getConflict($moduleB)[$moduleA]);
7981
$constraintB = $versionParser->parseConstraints($this->packageInfo->getVersion($moduleA));
8082
if ($constraintA->matches($constraintB)) {
81-
return true;
83+
$messages[] = "$moduleB conflicts with $moduleA version " .
84+
$this->packageInfo->getConflict($moduleB)[$moduleA] .
85+
' (current ' . $this->packageInfo->getVersion($moduleA) . ')';
8286
}
8387
}
8488
if (isset($this->packageInfo->getConflict($moduleA)[$moduleB]) &&
@@ -88,9 +92,11 @@ private function checkIfConflict($moduleA, $moduleB)
8892
$constraintA = $versionParser->parseConstraints($this->packageInfo->getConflict($moduleA)[$moduleB]);
8993
$constraintB = $versionParser->parseConstraints($this->packageInfo->getVersion($moduleB));
9094
if ($constraintA->matches($constraintB)) {
91-
return true;
95+
$messages[] = "$moduleA conflicts with $moduleB version " .
96+
$this->packageInfo->getConflict($moduleA)[$moduleB] .
97+
' (current ' . $this->packageInfo->getVersion($moduleA) . ')';;
9298
}
9399
}
94-
return false;
100+
return $messages;
95101
}
96102
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,8 @@ public function checkConstraints($isEnabled, $modules)
113113

114114
foreach ($errorModulesConflict as $moduleName => $conflictingModules) {
115115
if (!empty($conflictingModules)) {
116-
$errorMessages[] = "Cannot enable $moduleName, conflicting modules:";
117-
$errorMessages[] = implode(', ', $conflictingModules);
116+
$errorMessages[] = "Cannot enable $moduleName, conflicting with other modules:";
117+
$errorMessages[] = implode("\n", $conflictingModules);
118118
}
119119
}
120120

0 commit comments

Comments
 (0)