MC-19713: Relative Category links created using PageBuilder have the store name as a URL parameter

bubasuma · bubasuma · commit 7d19fe02eaae · 2019-09-03T10:17:11.000-05:00
- Remove store query param from category and product URL on cms pages
diff --git a/app/code/Magento/Catalog/Block/Widget/Link.php b/app/code/Magento/Catalog/Block/Widget/Link.php
@@ -4,17 +4,15 @@
  * See COPYING.txt for license details.
  */
 
-/**
- * Widget to display catalog link
- *
- * @author     Magento Core Team <core@magentocommerce.com>
- */
 namespace Magento\Catalog\Block\Widget;
 
 use Magento\CatalogUrlRewrite\Model\ProductUrlRewriteGenerator;
 use Magento\UrlRewrite\Model\UrlFinderInterface;
 use Magento\UrlRewrite\Service\V1\Data\UrlRewrite;
 
+/**
+ * Render the URL of given entity
+ */
 class Link extends \Magento\Framework\View\Element\Html\Link implements \Magento\Widget\Block\BlockInterface
 {
     /**
@@ -63,10 +61,9 @@ public function __construct(
 
     /**
      * Prepare url using passed id path and return it
-     * or return false if path was not found in url rewrites.
      *
      * @throws \RuntimeException
-     * @return string|false
+     * @return string|false if path was not found in url rewrites.
      * @SuppressWarnings(PHPMD.NPathComplexity)
      */
     public function getHref()
@@ -92,10 +89,6 @@ public function getHref()
 
             if ($rewrite) {
                 $href = $store->getUrl('', ['_direct' => $rewrite->getRequestPath()]);
-
-                if (strpos($href, '___store') === false) {
-                    $href .= (strpos($href, '?') === false ? '?' : '&') . '___store=' . $store->getCode();
-                }
             }
             $this->_href = $href;
         }
@@ -121,6 +114,7 @@ protected function parseIdPath($idPath)
 
     /**
      * Prepare label using passed text as parameter.
+     *
      * If anchor text was not specified get entity name from DB.
      *
      * @return string
@@ -150,9 +144,8 @@ public function getLabel()
 
     /**
      * Render block HTML
-     * or return empty string if url can't be prepared
      *
-     * @return string
+     * @return string empty string if url can't be prepared
      */
     protected function _toHtml()
     {
diff --git a/app/code/Magento/Catalog/Test/Unit/Block/Widget/LinkTest.php b/app/code/Magento/Catalog/Test/Unit/Block/Widget/LinkTest.php
@@ -5,54 +5,81 @@
  */
 namespace Magento\Catalog\Test\Unit\Block\Widget;
 
+use Exception;
+use Magento\Catalog\Block\Widget\Link;
+use Magento\Catalog\Model\ResourceModel\AbstractResource;
 use Magento\CatalogUrlRewrite\Model\ProductUrlRewriteGenerator;
+use Magento\Framework\App\Config\ReinitableConfigInterface;
 use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
+use Magento\Framework\Url;
+use Magento\Framework\Url\ModifierInterface;
+use Magento\Framework\View\Element\Template\Context;
+use Magento\Store\Model\Store;
+use Magento\Store\Model\StoreManagerInterface;
+use Magento\UrlRewrite\Model\UrlFinderInterface;
 use Magento\UrlRewrite\Service\V1\Data\UrlRewrite;
+use PHPUnit\Framework\TestCase;
+use PHPUnit_Framework_MockObject_MockObject;
+use ReflectionClass;
+use RuntimeException;
 
-class LinkTest extends \PHPUnit\Framework\TestCase
+/**
+ * @SuppressWarnings(PHPMD.UnusedFormalParameter)
+ * @SuppressWarnings(PHPMD.UnusedLocalVariable)
+ * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
+ */
+class LinkTest extends TestCase
 {
     /**
-     * @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Store\Model\StoreManagerInterface
+     * @var PHPUnit_Framework_MockObject_MockObject|StoreManagerInterface
      */
     protected $storeManager;
 
     /**
-     * @var \PHPUnit_Framework_MockObject_MockObject|\Magento\UrlRewrite\Model\UrlFinderInterface
+     * @var PHPUnit_Framework_MockObject_MockObject|UrlFinderInterface
      */
     protected $urlFinder;
 
     /**
-     * @var \Magento\Catalog\Block\Widget\Link
+     * @var Link
      */
     protected $block;
 
     /**
-     * @var \Magento\Catalog\Model\ResourceModel\AbstractResource|\PHPUnit_Framework_MockObject_MockObject
+     * @var AbstractResource|PHPUnit_Framework_MockObject_MockObject
      */
     protected $entityResource;
 
+    /**
+     * @inheritDoc
+     */
     protected function setUp()
     {
-        $this->storeManager = $this->createMock(\Magento\Store\Model\StoreManagerInterface::class);
-        $this->urlFinder = $this->createMock(\Magento\UrlRewrite\Model\UrlFinderInterface::class);
+        $this->storeManager = $this->createMock(StoreManagerInterface::class);
+        $this->urlFinder = $this->createMock(UrlFinderInterface::class);
 
-        $context = $this->createMock(\Magento\Framework\View\Element\Template\Context::class);
+        $context = $this->createMock(Context::class);
         $context->expects($this->any())
             ->method('getStoreManager')
             ->will($this->returnValue($this->storeManager));
 
         $this->entityResource =
-            $this->createMock(\Magento\Catalog\Model\ResourceModel\AbstractResource::class);
-
-        $this->block = (new ObjectManager($this))->getObject(\Magento\Catalog\Block\Widget\Link::class, [
-            'context' => $context,
-            'urlFinder' => $this->urlFinder,
-            'entityResource' => $this->entityResource
-        ]);
+            $this->createMock(AbstractResource::class);
+
+        $this->block = (new ObjectManager($this))->getObject(
+            Link::class,
+            [
+                'context' => $context,
+                'urlFinder' => $this->urlFinder,
+                'entityResource' => $this->entityResource
+            ]
+        );
     }
 
     /**
-     * @expectedException \RuntimeException
+     * Tests getHref with wrong id_path
+     *
+     * @expectedException RuntimeException
      * @expectedExceptionMessage Parameter id_path is not set.
      */
     public function testGetHrefWithoutSetIdPath()
@@ -61,7 +88,9 @@ public function testGetHrefWithoutSetIdPath()
     }
 
     /**
-     * @expectedException \RuntimeException
+     * Tests getHref with wrong id_path
+     *
+     * @expectedException RuntimeException
      * @expectedExceptionMessage Wrong id_path structure.
      */
     public function testGetHrefIfSetWrongIdPath()
@@ -70,27 +99,30 @@ public function testGetHrefIfSetWrongIdPath()
         $this->block->getHref();
     }
 
+    /**
+     * Tests getHref with wrong store ID
+     *
+     * @expectedException Exception
+     */
     public function testGetHrefWithSetStoreId()
     {
         $this->block->setData('id_path', 'type/id');
         $this->block->setData('store_id', 'store_id');
-
         $this->storeManager->expects($this->once())
-            ->method('getStore')->with('store_id')
-            // interrupt test execution
-            ->will($this->throwException(new \Exception()));
-
-        try {
-            $this->block->getHref();
-        } catch (\Exception $e) {
-        }
+            ->method('getStore')
+            ->with('store_id')
+            ->will($this->throwException(new Exception()));
+        $this->block->getHref();
     }
 
+    /**
+     * Tests getHref with not found URL
+     */
     public function testGetHrefIfRewriteIsNotFound()
     {
         $this->block->setData('id_path', 'entity_type/entity_id');
 
-        $store = $this->createPartialMock(\Magento\Store\Model\Store::class, ['getId', '__wakeUp']);
+        $store = $this->createPartialMock(Store::class, ['getId', '__wakeUp']);
         $store->expects($this->any())
             ->method('getId');
 
@@ -105,59 +137,104 @@ public function testGetHrefIfRewriteIsNotFound()
     }
 
     /**
-     * @param string $url
-     * @param string $separator
+     * Tests getHref whether it should include the store code or not
+     *
      * @dataProvider dataProviderForTestGetHrefWithoutUrlStoreSuffix
+     * @param string $path
+     * @param bool $includeStoreCode
+     * @param string $expected
+     * @throws \ReflectionException
      */
-    public function testGetHrefWithoutUrlStoreSuffix($url, $separator)
-    {
-        $storeId = 15;
-        $storeCode = 'store-code';
-        $requestPath = 'request-path';
+    public function testStoreCodeShouldBeIncludedInURLOnlyIfItIsConfiguredSo(
+        string $path,
+        bool $includeStoreCode,
+        string $expected
+    ) {
         $this->block->setData('id_path', 'entity_type/entity_id');
-
-        $rewrite = $this->createMock(\Magento\UrlRewrite\Service\V1\Data\UrlRewrite::class);
-        $rewrite->expects($this->once())
-            ->method('getRequestPath')
-            ->will($this->returnValue($requestPath));
-
-        $store = $this->createPartialMock(
-            \Magento\Store\Model\Store::class,
-            ['getId', 'getUrl', 'getCode', '__wakeUp']
+        $objectManager = new ObjectManager($this);
+
+        $rewrite = $this->createPartialMock(UrlRewrite::class, ['getRequestPath']);
+        $url = $this->createPartialMock(Url::class, ['setScope', 'getUrl']);
+        $urlModifier = $this->getMockForAbstractClass(ModifierInterface::class);
+        $config = $this->getMockForAbstractClass(ReinitableConfigInterface::class);
+        $store = $objectManager->getObject(
+            Store::class,
+            [
+                'storeManager' => $this->storeManager,
+                'url' => $url,
+                'config' => $config
+            ]
         );
-        $store->expects($this->once())
-            ->method('getId')
-            ->will($this->returnValue($storeId));
-        $store->expects($this->once())
+        $property = (new ReflectionClass(get_class($store)))->getProperty('urlModifier');
+        $property->setAccessible(true);
+        $property->setValue($store, $urlModifier);
+
+        $urlModifier->expects($this->any())
+            ->method('execute')
+            ->willReturnArgument(0);
+        $config->expects($this->any())
+            ->method('getValue')
+            ->willReturnMap(
+                [
+                    [Store::XML_PATH_USE_REWRITES, ReinitableConfigInterface::SCOPE_TYPE_DEFAULT, null, true],
+                    [
+                        Store::XML_PATH_STORE_IN_URL,
+                        ReinitableConfigInterface::SCOPE_TYPE_DEFAULT,
+                        null, $includeStoreCode
+                    ]
+                ]
+            );
+
+        $url->expects($this->any())
+            ->method('setScope')
+            ->willReturnSelf();
+
+        $url->expects($this->any())
             ->method('getUrl')
-            ->with('', ['_direct' => $requestPath])
-            ->will($this->returnValue($url));
-        $store->expects($this->once())
-            ->method('getCode')
-            ->will($this->returnValue($storeCode));
+            ->willReturnCallback(
+                function ($route, $params) use ($store) {
+                    return rtrim($store->getBaseUrl(), '/') .'/'. ltrim($params['_direct'], '/');
+                }
+            );
 
-        $this->storeManager->expects($this->once())
+        $store->addData(['store_id' => 1, 'code' => 'french']);
+
+        $this->storeManager
+            ->expects($this->any())
             ->method('getStore')
-            ->will($this->returnValue($store));
+            ->willReturn($store);
 
-        $this->urlFinder->expects($this->once())->method('findOneByData')
-            ->with([
+        $this->urlFinder->expects($this->once())
+            ->method('findOneByData')
+            ->with(
+                [
                     UrlRewrite::ENTITY_ID => 'entity_id',
                     UrlRewrite::ENTITY_TYPE => 'entity_type',
-                    UrlRewrite::STORE_ID => $storeId,
-                ])
+                    UrlRewrite::STORE_ID => $store->getStoreId(),
+                ]
+            )
             ->will($this->returnValue($rewrite));
 
-        $this->assertEquals($url . $separator . '___store=' . $storeCode, $this->block->getHref());
+        $rewrite->expects($this->once())
+            ->method('getRequestPath')
+            ->will($this->returnValue($path));
+
+        $this->assertContains($expected, $this->block->getHref());
     }
 
+    /**
+     * Tests getLabel with custom text
+     */
     public function testGetLabelWithCustomText()
     {
         $customText = 'Some text';
         $this->block->setData('anchor_text', $customText);
         $this->assertEquals($customText, $this->block->getLabel());
     }
 
+    /**
+     * Tests getLabel without custom text
+     */
     public function testGetLabelWithoutCustomText()
     {
         $category = 'Some text';
@@ -178,17 +255,20 @@ public function testGetLabelWithoutCustomText()
     public function dataProviderForTestGetHrefWithoutUrlStoreSuffix()
     {
         return [
-            ['url', '?'],
-            ['url?some_parameter', '&'],
+            ['/accessories.html', true, 'french/accessories.html'],
+            ['/accessories.html', false, '/accessories.html'],
         ];
     }
 
+    /**
+     * Tests getHref with product entity and additional category id in the id_path
+     */
     public function testGetHrefWithForProductWithCategoryIdParameter()
     {
         $storeId = 15;
         $this->block->setData('id_path', ProductUrlRewriteGenerator::ENTITY_TYPE . '/entity_id/category_id');
 
-        $store = $this->createPartialMock(\Magento\Store\Model\Store::class, ['getId', '__wakeUp']);
+        $store = $this->createPartialMock(Store::class, ['getId', '__wakeUp']);
         $store->expects($this->any())
             ->method('getId')
             ->will($this->returnValue($storeId));
@@ -197,13 +277,16 @@ public function testGetHrefWithForProductWithCategoryIdParameter()
             ->method('getStore')
             ->will($this->returnValue($store));
 
-        $this->urlFinder->expects($this->once())->method('findOneByData')
-            ->with([
-                UrlRewrite::ENTITY_ID => 'entity_id',
-                UrlRewrite::ENTITY_TYPE => ProductUrlRewriteGenerator::ENTITY_TYPE,
-                UrlRewrite::STORE_ID => $storeId,
-                UrlRewrite::METADATA => ['category_id' => 'category_id'],
-            ])
+        $this->urlFinder->expects($this->once())
+            ->method('findOneByData')
+            ->with(
+                [
+                    UrlRewrite::ENTITY_ID => 'entity_id',
+                    UrlRewrite::ENTITY_TYPE => ProductUrlRewriteGenerator::ENTITY_TYPE,
+                    UrlRewrite::STORE_ID => $storeId,
+                    UrlRewrite::METADATA => ['category_id' => 'category_id'],
+                ]
+            )
             ->will($this->returnValue(false));
 
         $this->block->getHref();
