magento · rogerdz · Jan 5, 2024 · Aug 21, 2024 · Sep 9, 2024 · Sep 10, 2024
diff --git a/app/code/Magento/Theme/Block/Html/Topmenu.php b/app/code/Magento/Theme/Block/Html/Topmenu.php
@@ -133,15 +133,15 @@ 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
      */
     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 .= '</ul></li><li class="column"><ul>';
+                continue;
             }
 
             $html .= '<li ' . $this->_getRenderedMenuItemAttributes($child) . '>';
@@ -257,10 +254,6 @@ protected function _getHtml(
             $counter++;
         }
 
-        if (is_array($colBrakes) && !empty($colBrakes) && $limit) {
-            $html = '<li class="column"><ul>' . $html . '</ul></li>';
-        }
-
         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
@@ -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 = <<<HTML
@@ -107,6 +124,21 @@ protected function setUp(): void
             ->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(
+            '<ul class="level0 "></ul>',
+            $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, '');
     }
 }