Skip to content

Commit a82f163

Browse files
committed
#32464: Magento erroneously assums one category is parent of another based on category path
- Update Unit test to reproduce issue - Fix check for parent category path
1 parent 1090911 commit a82f163

File tree

2 files changed

+17
-10
lines changed

2 files changed

+17
-10
lines changed

app/code/Magento/Catalog/Model/CategoryManagement.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ public function move($categoryId, $parentId, $afterId = null)
128128
$afterId = ($afterId === null || $afterId > $lastId) ? $lastId : $afterId;
129129
}
130130
$parentPath = $parentCategory->getPath() ?? '';
131-
$path = $model->getPath();
131+
$path = $model->getPath() . '/';
132132
if ($path && strpos($parentPath, $path) === 0) {
133133
throw new \Magento\Framework\Exception\LocalizedException(
134134
__('Operation do not allow to move a parent category to any of children category')

app/code/Magento/Catalog/Test/Unit/Model/CategoryManagementTest.php

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -201,8 +201,8 @@ public function testGetTreeForAllScope()
201201

202202
public function testMove()
203203
{
204-
$categoryId = 2;
205-
$parentId = 1;
204+
$categoryId = 4;
205+
$parentId = 40;
206206
$afterId = null;
207207
$categoryMock = $this->getMockBuilder(Category::class)
208208
->setMockClassName('categoryMock')
@@ -214,18 +214,25 @@ public function testMove()
214214
->getMock();
215215

216216
$this->categoryRepositoryMock
217-
->expects($this->exactly(2))
217+
->expects($this->exactly(6))
218218
->method('get')
219219
->willReturnMap([
220220
[$categoryId, null, $categoryMock],
221221
[$parentId, null, $parentCategoryMock],
222222
]);
223-
$parentCategoryMock->expects($this->once())->method('hasChildren')->willReturn(true);
223+
$parentCategoryMock->expects($this->exactly(3))->method('hasChildren')
224+
->willReturn(true, false, false);
224225
$parentCategoryMock->expects($this->once())->method('getChildren')->willReturn('5,6,7');
225-
$categoryMock->expects($this->once())->method('getPath');
226-
$parentCategoryMock->expects($this->once())->method('getPath');
227-
$categoryMock->expects($this->once())->method('move')->with($parentId, '7');
226+
$categoryMock->expects($this->exactly(3))->method('getPath')
227+
->willReturnOnConsecutiveCalls('2/4', '2/3/4', '2/3/4');
228+
$parentCategoryMock->expects($this->exactly(3))->method('getPath')
229+
->willReturnOnConsecutiveCalls('2/40', '2/3/40', '2/3/44/40');
230+
$categoryMock->expects($this->exactly(3))->method('move')
231+
->withConsecutive([$parentId, '7'], [$parentId, null], [$parentId, null]);
232+
228233
$this->assertTrue($this->model->move($categoryId, $parentId, $afterId));
234+
$this->assertTrue($this->model->move($categoryId, $parentId));
235+
$this->assertTrue($this->model->move($categoryId, $parentId));
229236
}
230237

231238
public function testMoveWithException()
@@ -251,8 +258,8 @@ public function testMoveWithException()
251258
[$categoryId, null, $categoryMock],
252259
[$parentId, null, $parentCategoryMock],
253260
]);
254-
$categoryMock->expects($this->once())->method('getPath')->willReturn('test');
255-
$parentCategoryMock->expects($this->once())->method('getPath')->willReturn('test');
261+
$categoryMock->expects($this->once())->method('getPath')->willReturn('test/2');
262+
$parentCategoryMock->expects($this->once())->method('getPath')->willReturn('test/2/1');
256263
$this->model->move($categoryId, $parentId, $afterId);
257264
}
258265

0 commit comments

Comments
 (0)