🔃 [Magento Community Engineering] Community Contributions - 2.4-develop

Accepted Community Pull Requests:
 - #25603: Fix removing query string from url after redirect (by @arendarenko)


Fixed GitHub Issues:
 - #18717: UrlRewrite removes query string from url, if url has trailing slash (reported by @sergeynezbritskiy) has been fixed in #25603 by @arendarenko in 2.4-develop branch
   Related commits:
     1. 0693cc2
     2. 19b7bda
     3. f63475c
     4. b1a2a29

app/code/Magento/UrlRewrite/Controller/Router.php

@@ -118,7 +118,7 @@ protected function processRedirect($request, $rewrite)
         if ($rewrite->getEntityType() !== Rewrite::ENTITY_TYPE_CUSTOM
             || ($prefix = substr($target, 0, 6)) !== 'http:/' && $prefix !== 'https:'
         ) {
-            $target = $this->url->getUrl('', ['_direct' => $target]);
+            $target = $this->url->getUrl('', ['_direct' => $target, '_query' => $request->getParams()]);
         }
         return $this->redirect($request, $target, $rewrite->getRedirectType());
     }

app/code/Magento/UrlRewrite/Test/Unit/Controller/RouterTest.php

@@ -260,6 +260,7 @@ public function testNoRewriteAfterStoreSwitcherWhenOldRewriteEqualsToNewOne()
      */
     public function testMatchWithRedirect()
     {
+        $queryParams = [];
         $this->storeManager->expects($this->any())->method('getStore')->will($this->returnValue($this->store));
         $urlRewrite = $this->getMockBuilder(UrlRewrite::class)
             ->disableOriginalConstructor()->getMock();
@@ -268,7 +269,11 @@ public function testMatchWithRedirect()
         $this->urlFinder->expects($this->any())->method('findOneByData')->will($this->returnValue($urlRewrite));
         $this->response->expects($this->once())->method('setRedirect')
             ->with('new-target-path', 'redirect-code');
-        $this->url->expects($this->once())->method('getUrl')->with('', ['_direct' => 'target-path'])
+        $this->request->expects($this->once())->method('getParams')->willReturn($queryParams);
+        $this->url->expects($this->once())->method('getUrl')->with(
+            '',
+            ['_direct' => 'target-path', '_query' => $queryParams]
+        )
             ->will($this->returnValue('new-target-path'));
         $this->request->expects($this->once())->method('setDispatched')->with(true);
         $this->actionFactory->expects($this->once())->method('create')
@@ -282,15 +287,20 @@ public function testMatchWithRedirect()
      */
     public function testMatchWithCustomInternalRedirect()
     {
+        $queryParams = [];
         $this->storeManager->expects($this->any())->method('getStore')->will($this->returnValue($this->store));
         $urlRewrite = $this->getMockBuilder(UrlRewrite::class)
             ->disableOriginalConstructor()->getMock();
         $urlRewrite->expects($this->any())->method('getEntityType')->will($this->returnValue('custom'));
         $urlRewrite->expects($this->any())->method('getRedirectType')->will($this->returnValue('redirect-code'));
         $urlRewrite->expects($this->any())->method('getTargetPath')->will($this->returnValue('target-path'));
         $this->urlFinder->expects($this->any())->method('findOneByData')->will($this->returnValue($urlRewrite));
+        $this->request->expects($this->any())->method('getParams')->willReturn($queryParams);
         $this->response->expects($this->once())->method('setRedirect')->with('a', 'redirect-code');
-        $this->url->expects($this->once())->method('getUrl')->with('', ['_direct' => 'target-path'])->willReturn('a');
+        $this->url->expects($this->once())->method('getUrl')->with(
+            '',
+            ['_direct' => 'target-path', '_query' => $queryParams]
+        )->willReturn('a');
         $this->request->expects($this->once())->method('setDispatched')->with(true);
         $this->actionFactory->expects($this->once())->method('create')
             ->with(\Magento\Framework\App\Action\Redirect::class);
@@ -312,6 +322,7 @@ public function testMatchWithCustomExternalRedirect($targetPath)
         $urlRewrite->expects($this->any())->method('getTargetPath')->will($this->returnValue($targetPath));
         $this->urlFinder->expects($this->any())->method('findOneByData')->will($this->returnValue($urlRewrite));
         $this->response->expects($this->once())->method('setRedirect')->with($targetPath, 'redirect-code');
+        $this->request->expects($this->never())->method('getParams');
         $this->url->expects($this->never())->method('getUrl');
         $this->request->expects($this->once())->method('setDispatched')->with(true);
         $this->actionFactory->expects($this->once())->method('create')

dev/tests/integration/testsuite/Magento/UrlRewrite/Controller/UrlRewriteTest.php

@@ -44,8 +44,7 @@ public function testMatchUrlRewrite(
             $location = $response->getHeader('Location')->getFieldValue();
             $this->assertStringEndsWith(
                 $redirect,
-                $location,
-                'Invalid location header'
+                $location
             );
         }
     }
@@ -80,6 +79,60 @@ public function requestDataProvider(): array
                 'request' => '/page-similar/',
                 'redirect' => '/page-b',
             ],
+            'Use Case #7: Rewrite: page-similar --(301)--> page-a; '
+            . 'Request: page-similar?param=1 --(301)--> page-a?param=1' => [
+                'request' => '/page-similar?param=1',
+                'redirect' => '/page-a?param=1',
+            ],
+            'Use Case #8: Rewrite: page-similar/ --(301)--> page-b; '
+            . 'Request: page-similar/?param=1 --(301)--> page-b?param=1' => [
+                'request' => '/page-similar/?param=1',
+                'redirect' => '/page-b?param=1',
+            ],
+            'Use Case #9: Rewrite: page-similar-query-param --(301)--> page-d?param1=1;'
+            . 'Request: page-similar-query-param --(301)--> page-d?param1=1' => [
+                'request' => '/page-similar-query-param',
+                'redirect' => '/page-d?param1=1',
+            ],
+            'Use Case #10: Rewrite: page-similar-query-param --(301)--> page-d?param1=1; '
+            . 'Request: page-similar-query-param?param2=1 --(301)--> page-d?param1=1&param2=1' => [
+                'request' => '/page-similar-query-param?param2=1',
+                'redirect' => '/page-d?param1=1&param2=1',
+            ],
+            'Use Case #11: Rewrite: page-similar-query-param/ --(301)--> page-e?param1=1; '
+            . 'Request: page-similar-query-param/ --(301)--> page-e?param1=1' => [
+                'request' => '/page-similar-query-param/',
+                'redirect' => '/page-e?param1=1',
+            ],
+            'Use Case #12: Rewrite: page-similar-query-param/ --(301)--> page-e?param1=1;'
+            . 'Request: page-similar-query-param/?param2=1 --(301)--> page-e?param1=1&param2=1' => [
+                'request' => '/page-similar-query-param/?param2=1',
+                'redirect' => '/page-e?param1=1&param2=1',
+            ],
+            'Use Case #13: Rewrite: page-external1 --(301)--> http://example.com/external;'
+            . 'Request: page-external1?param1=1 --(301)--> http://example.com/external (not fills get params)' => [
+                'request' => '/page-external1?param1=1',
+                'redirect' => 'http://example.com/external',
+            ],
+            'Use Case #14: Rewrite: page-external2/ --(301)--> https://example.com/external2/;'
+            . 'Request: page-external2?param2=1 --(301)--> https://example.com/external2/ (not fills get params)' => [
+                'request' => '/page-external2?param2=1',
+                'redirect' => 'https://example.com/external2/',
+            ],
+            'Use Case #15: Rewrite: page-external3 --(301)--> http://example.com/external?param1=value1;'
+            . 'Request: page-external3?param1=custom1&param2=custom2 --(301)--> '
+            . 'http://example.com/external?param1=value1'
+            . ' (fills get param from target path)' => [
+                'request' => '/page-external3?param1=custom1&param2=custom2',
+                'redirect' => 'http://example.com/external?param1=value1',
+            ],
+            'Use Case #16: Rewrite: page-external4/ --(301)--> https://example.com/external2/?param2=value2;'
+            . 'Request: page-external4?param1=custom1&param2=custom2 --(301)--> '
+            . 'https://example.com/external2/?param2=value2 '
+            . ' (fills get param from target path)' => [
+                'request' => '/page-external4?param1=custom1&param2=custom2',
+                'redirect' => 'https://example.com/external2/?param2=value2',
+            ],
         ];
     }
 }

dev/tests/integration/testsuite/Magento/UrlRewrite/_files/url_rewrite.php

@@ -64,6 +64,26 @@
     ->setStores([$storeID, $secondStoreId]);
 $pageResource->save($page);
 
+$page = $objectManager->create(Page::class);
+$page->setTitle('Cms D')
+    ->setIdentifier('page-d')
+    ->setIsActive(1)
+    ->setContent('<h1>Cms Page D</h1>')
+    ->setPageLayout('1column')
+    ->setCustomTheme('Magento/blank')
+    ->setStores([$storeID, $secondStoreId]);
+$pageResource->save($page);
+
+$page = $objectManager->create(Page::class);
+$page->setTitle('Cms E')
+    ->setIdentifier('page-e')
+    ->setIsActive(1)
+    ->setContent('<h1>Cms Page E</h1>')
+    ->setPageLayout('1column')
+    ->setCustomTheme('Magento/blank')
+    ->setStores([$storeID, $secondStoreId]);
+$pageResource->save($page);
+
 $rewrite = $objectManager->create(UrlRewrite::class);
 $rewrite->setEntityType('custom')
     ->setRequestPath('page-one/')
@@ -88,7 +108,7 @@
     ->setTargetPath('page-a')
     ->setRedirectType(OptionProvider::PERMANENT)
     ->setStoreId($storeID)
-    ->setDescription('From age-similar without trailing slash to page-a');
+    ->setDescription('From page-similar without trailing slash to page-a');
 $rewriteResource->save($rewrite);
 
 $rewrite = $objectManager->create(UrlRewrite::class);
@@ -97,7 +117,7 @@
     ->setTargetPath('page-b')
     ->setRedirectType(OptionProvider::PERMANENT)
     ->setStoreId($storeID)
-    ->setDescription('From age-similar with trailing slash to page-b');
+    ->setDescription('From page-similar with trailing slash to page-b');
 $rewriteResource->save($rewrite);
 
 //Emulating auto-generated aliases (like the ones used for categories).
@@ -117,3 +137,57 @@
     ->setRedirectType(0)
     ->setStoreId($secondStoreId);
 $rewriteResource->save($rewrite);
+
+$rewrite = $objectManager->create(UrlRewrite::class);
+$rewrite->setEntityType('custom')
+    ->setRequestPath('page-similar-query-param')
+    ->setTargetPath('page-d?param1=1')
+    ->setRedirectType(OptionProvider::PERMANENT)
+    ->setStoreId($storeID)
+    ->setDescription('From page-similar-query-param to page-d with query param');
+$rewriteResource->save($rewrite);
+
+$rewrite = $objectManager->create(UrlRewrite::class);
+$rewrite->setEntityType('custom')
+    ->setRequestPath('page-similar-query-param/')
+    ->setTargetPath('page-e?param1=1')
+    ->setRedirectType(OptionProvider::PERMANENT)
+    ->setStoreId($storeID)
+    ->setDescription('From page-similar-query-param with trailing slash to page-e with query param');
+$rewriteResource->save($rewrite);
+
+$rewrite = $objectManager->create(UrlRewrite::class);
+$rewrite->setEntityType('custom')
+    ->setRequestPath('page-external1')
+    ->setTargetPath('http://example.com/external')
+    ->setRedirectType(OptionProvider::PERMANENT)
+    ->setStoreId($storeID)
+    ->setDescription('From page-external to external URL');
+$rewriteResource->save($rewrite);
+
+$rewrite = $objectManager->create(UrlRewrite::class);
+$rewrite->setEntityType('custom')
+    ->setRequestPath('page-external2/')
+    ->setTargetPath('https://example.com/external2/')
+    ->setRedirectType(OptionProvider::PERMANENT)
+    ->setStoreId($storeID)
+    ->setDescription('From page-external with trailing slash to external URL');
+$rewriteResource->save($rewrite);
+
+$rewrite = $objectManager->create(UrlRewrite::class);
+$rewrite->setEntityType('custom')
+    ->setRequestPath('page-external3')
+    ->setTargetPath('http://example.com/external?param1=value1')
+    ->setRedirectType(OptionProvider::PERMANENT)
+    ->setStoreId($storeID)
+    ->setDescription('From page-external to external URL');
+$rewriteResource->save($rewrite);
+
+$rewrite = $objectManager->create(UrlRewrite::class);
+$rewrite->setEntityType('custom')
+    ->setRequestPath('page-external4/')
+    ->setTargetPath('https://example.com/external2/?param2=value2')
+    ->setRedirectType(OptionProvider::PERMANENT)
+    ->setStoreId($storeID)
+    ->setDescription('From page-external with trailing slash to external URL');
+$rewriteResource->save($rewrite);

dev/tests/integration/testsuite/Magento/UrlRewrite/_files/url_rewrite_rollback.php

@@ -19,6 +19,8 @@
 $pageRepository->deleteById('page-a');
 $pageRepository->deleteById('page-b');
 $pageRepository->deleteById('page-c');
+$pageRepository->deleteById('page-d');
+$pageRepository->deleteById('page-e');
 
 /** @var \Magento\Catalog\Api\ProductRepositoryInterface $productRepository */
 $productRepository = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()
@@ -29,7 +31,21 @@
     ->create(\Magento\UrlRewrite\Model\ResourceModel\UrlRewriteCollection::class);
 $collection = $urlRewriteCollection
     ->addFieldToFilter('entity_type', 'custom')
-    ->addFieldToFilter('target_path', ['page-a/', 'page-a', 'page-b', 'page-c'])
+    ->addFieldToFilter(
+        'target_path',
+        [
+            'page-a/',
+            'page-a',
+            'page-b',
+            'page-c',
+            'page-d?param1=1',
+            'page-e?param1=1',
+            'http://example.com/external',
+            'https://example.com/external2/',
+            'http://example.com/external?param1=value1',
+            'https://example.com/external2/?param2=value2'
+        ]
+    )
     ->load()
     ->walk('delete');