diff --git a/app/code/Magento/Theme/Block/Html/Topmenu.php b/app/code/Magento/Theme/Block/Html/Topmenu.php
index 37a2149d29809..88ec35c552b44 100644
--- a/app/code/Magento/Theme/Block/Html/Topmenu.php
+++ b/app/code/Magento/Theme/Block/Html/Topmenu.php
@@ -133,7 +133,7 @@ protected function _countItems($items)
*
* @param Menu $items
* @param int $limit
- * @return array|void
+ * @return array
*
* @todo: Add Depth Level limit, and better logic for columns
*/
@@ -141,7 +141,7 @@ protected function _columnBrake($items, $limit)
{
$total = $this->_countItems($items);
if ($total <= $limit) {
- return;
+ return [];
}
$result[] = ['total' => $total, 'max' => (int)ceil($total / ceil($total / $limit))];
@@ -151,12 +151,9 @@ protected function _columnBrake($items, $limit)
foreach ($items as $item) {
$place = $this->_countItems($item->getChildren()) + 1;
- $count += $place;
+ $count++;
- if ($place >= $limit) {
- $colbrake = !$firstCol;
- $count = 0;
- } elseif ($count >= $limit) {
+ if ($count > $limit) {
$colbrake = !$firstCol;
$count = $place;
} else {
@@ -242,7 +239,7 @@ protected function _getHtml(
}
if ($this->shouldAddNewColumn($colBrakes, $counter)) {
- $html .= '';
+ continue;
}
$html .= '- _getRenderedMenuItemAttributes($child) . '>';
@@ -257,10 +254,6 @@ protected function _getHtml(
$counter++;
}
- if (is_array($colBrakes) && !empty($colBrakes) && $limit) {
- $html = '
';
- }
-
return $html;
}
diff --git a/app/code/Magento/Theme/Test/Unit/Block/Html/TopmenuTest.php b/app/code/Magento/Theme/Test/Unit/Block/Html/TopmenuTest.php
old mode 100644
new mode 100755
index 5bc0f70dd7703..53368d379d05f
--- a/app/code/Magento/Theme/Test/Unit/Block/Html/TopmenuTest.php
+++ b/app/code/Magento/Theme/Test/Unit/Block/Html/TopmenuTest.php
@@ -23,6 +23,8 @@
use Magento\Framework\View\Element\Template\Context;
use Magento\Store\Model\StoreManagerInterface;
use Magento\Theme\Block\Html\Topmenu;
+use Magento\Backend\Model\Menu;
+use Magento\Backend\Model\Menu\Item as MenuItem;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
@@ -76,6 +78,21 @@ class TopmenuTest extends TestCase
*/
private $requestMock;
+ /**
+ * @var Menu
+ */
+ private $menuMock;
+
+ /**
+ * @var MenuItem
+ */
+ private $menuItemMock;
+
+ /**
+ * @var Node
+ */
+ private $nodeMock;
+
// @codingStandardsIgnoreStart
/** @var string */
private $navigationMenuHtml = <<disableOriginalConstructor()
->getMock();
+ $this->menuMock = $this->getMockBuilder(Menu::class)
+ ->onlyMethods(['count', 'getIterator'])
+ ->disableOriginalConstructor()
+ ->getMock();
+
+ $this->menuItemMock = $this->getMockBuilder(MenuItem::class)
+ ->disableOriginalConstructor()
+ ->getMock();
+
+ $this->nodeMock = $this->getMockBuilder(Node::class)
+ ->disableOriginalConstructor()
+ ->addMethods(['getClass'])
+ ->onlyMethods(['getChildren', 'hasChildren', '__call'])
+ ->getMock();
+
$objectManager = new ObjectManager($this);
$this->context = $objectManager->getObject(
Context::class,
@@ -265,14 +297,10 @@ private function buildTree(bool $isCurrentItem): MockObject
$children->expects($this->once())->method('count')->willReturn(10);
- $nodeMock = $this->getMockBuilder(Node::class)
- ->disableOriginalConstructor()
- ->onlyMethods(['getChildren', '__call'])
- ->getMock();
- $nodeMock->expects($this->once())
+ $this->nodeMock->expects($this->once())
->method('getChildren')
->willReturn($children);
- $nodeMock
+ $this->nodeMock
->method('__call')
->willReturnCallback(function ($arg1, $arg2) {
if ($arg1 == 'setOutermostClass') {
@@ -291,13 +319,13 @@ private function buildTree(bool $isCurrentItem): MockObject
$this->nodeFactory->expects($this->any())
->method('create')
->with($nodeMockData)
- ->willReturn($nodeMock);
+ ->willReturn($this->nodeMock);
$this->treeFactory->expects($this->once())
->method('create')
->willReturn($treeMock);
- return $nodeMock;
+ return $this->nodeMock;
}
/**
@@ -315,20 +343,180 @@ public function testGetMenu(): void
'tree' => $treeMock
];
- $nodeMock = $this->getMockBuilder(Node::class)
- ->disableOriginalConstructor()
- ->getMock();
-
$this->nodeFactory->expects($this->any())
->method('create')
->with($nodeMockData)
- ->willReturn($nodeMock);
+ ->willReturn($this->nodeMock);
$this->treeFactory->expects($this->once())
->method('create')
->willReturn($treeMock);
$topmenuBlock = $this->getTopmenu();
- $this->assertEquals($nodeMock, $topmenuBlock->getMenu());
+ $this->assertEquals($this->nodeMock, $topmenuBlock->getMenu());
+ }
+
+ /**
+ * Test counting items when there are no children.
+ * @return void
+ */
+ public function testCountItemsNoChildren():void
+ {
+ $this->menuMock->expects($this->any())
+ ->method('count')
+ ->willReturn(5);
+ $this->menuMock->expects($this->any())
+ ->method('getIterator')
+ ->willReturn(new \ArrayIterator([$this->menuItemMock]));
+
+ $this->menuItemMock->expects($this->any())
+ ->method('hasChildren')
+ ->willReturn(false);
+
+ $method = new \ReflectionMethod(
+ Topmenu::class,
+ '_countItems'
+ );
+ $method->setAccessible(true);
+
+ $this->assertEquals(5, $method->invoke($this->getTopmenu(), $this->menuMock));
+ }
+
+ /**
+ * Test counting items when there are children.
+ * @return void
+ */
+ public function testCountItemsWithChildren(): void
+ {
+ // Setup child menu mock
+ $childMenuMock = $this->createMock(Menu::class);
+ $childMenuMock->expects($this->any())
+ ->method('count')
+ ->willReturn(3);
+ $childMenuMock->expects($this->any())
+ ->method('getIterator')
+ ->willReturn(new \ArrayIterator([]));
+
+ $this->menuItemMock->expects($this->any())
+ ->method('hasChildren')
+ ->willReturn(true);
+ $this->menuItemMock->expects($this->any())
+ ->method('getChildren')
+ ->willReturn($childMenuMock);
+
+ // Setup menu mock
+ $this->menuMock->expects($this->any())
+ ->method('count')
+ ->willReturn(2);
+ $this->menuMock->expects($this->any())
+ ->method('getIterator')
+ ->willReturn(new \ArrayIterator([$this->menuItemMock, $this->menuItemMock]));
+
+ $method = new \ReflectionMethod(
+ Topmenu::class,
+ '_countItems'
+ );
+ $method->setAccessible(true);
+
+ // Total should be 2 (top level) + 2 * 3 (children) = 8
+ $this->assertEquals(8, $method->invoke($this->getTopmenu(), $this->menuMock));
+ }
+
+ /**
+ * @return void
+ * @throws \ReflectionException
+ */
+ public function testColumnBrakeEmptyArray(): void
+ {
+ $this->testCountItemsNoChildren();
+
+ $method = new \ReflectionMethod(
+ Topmenu::class,
+ '_columnBrake'
+ );
+ $method->setAccessible(true);
+
+ $this->assertEquals([], $method->invoke($this->getTopmenu(), $this->menuMock, 5));
+ }
+
+ /**
+ * @return void
+ * @throws \ReflectionException
+ */
+ public function testColumnBrakeWithoutItem(): void
+ {
+ $result = [
+ [ 'total' => 8,
+ 'max' => 2
+ ],
+ [
+ 'place' => 4,
+ 'colbrake' => false
+ ],
+ [
+ 'place' => 4,
+ 'colbrake' => false
+ ]
+ ];
+
+ $this->testCountItemsWithChildren();
+
+ $method = new \ReflectionMethod(
+ Topmenu::class,
+ '_columnBrake'
+ );
+ $method->setAccessible(true);
+
+ $this->assertEquals($result, $method->invoke($this->getTopmenu(), $this->menuMock, 2));
+ }
+
+ /**
+ * @return void
+ */
+ public function testAddSubMenu(): void
+ {
+ $container = $this->createMock(CategoryTree::class);
+
+ $children = $this->getMockBuilder(Collection::class)
+ ->onlyMethods(['count'])
+ ->setConstructorArgs(['container' => $container])
+ ->getMock();
+
+ $this->nodeMock->expects($this->atLeastOnce())
+ ->method('hasChildren')
+ ->willReturn(true);
+
+ $this->nodeMock->expects($this->any())
+ ->method('getChildren')
+ ->willReturn($children);
+
+ $method = new \ReflectionMethod(
+ Topmenu::class,
+ '_addSubMenu'
+ );
+ $method->setAccessible(true);
+
+ $this->assertEquals(
+ '',
+ $method->invoke($this->getTopmenu(), $this->nodeMock, 0, '', 2)
+ );
+ }
+
+ /**
+ * @return void
+ */
+ public function testSetCurrentClass(): void
+ {
+ $this->nodeMock->expects($this->once())
+ ->method('getClass')
+ ->willReturn(null);
+
+ $method = new \ReflectionMethod(
+ Topmenu::class,
+ 'setCurrentClass'
+ );
+ $method->setAccessible(true);
+
+ $method->invoke($this->getTopmenu(), $this->nodeMock, '');
}
}