Merge pull request #107 from next-sentence/keep-query-string

add option to keep query string when redirecting from old path to new…

composer.json

@@ -14,6 +14,8 @@
         "doctrine/collections": "^1.6",
         "doctrine/orm": "^2.7",
         "doctrine/persistence": "^1.3 || ^2.1",
+        "league/uri": "^6.0",
+        "league/uri-components": "^2.3",
         "sylius/resource-bundle": "^1.6",
         "symfony/config": "^5.4 || ^6.0",
         "symfony/console": "^5.4 || ^6.0",
@@ -34,7 +36,6 @@
         "phpunit/phpunit": "^9.6",
         "psalm/plugin-phpunit": "^0.18",
         "psalm/plugin-symfony": "^5.0",
-        "roave/security-advisories": "dev-latest",
         "setono/code-quality-pack": "^2.4",
         "setono/sylius-behat-pack": "^0.2",
         "sylius/sylius": "~1.10.14",

features/redirect.feature

@@ -25,3 +25,21 @@ Feature: Redirect
     And the store has a redirect from path "/old-path" to "/new-path" on channel "United States"
     When I try to access "/old-path"
     Then I should still be on "/old-path"
+
+  Scenario: Redirect from old path to new path and keep query string
+    Given I change my current channel to "United States"
+    And the store has a redirect from path "/old-path" to "/new-path"
+    When I try to access "/old-path?q=ts"
+    Then I should be redirected "/new-path?q=ts"
+
+  Scenario: Redirect from old path to new path with query param and keep query string
+    Given I change my current channel to "United States"
+    And the store has a redirect from path "/old-path" to "/new-path?gid=c"
+    When I try to access "/old-path?q=ts"
+    Then I should be redirected "/new-path?gid=c&q=ts"
+
+  Scenario: Redirect from old path to new path with hashtag and keep query string
+    Given I change my current channel to "United States"
+    And the store has a redirect from path "/old-path" to "/new-path#gid=c"
+    When I try to access "/old-path?q=ts"
+    Then I should be redirected "/new-path?q=ts#gid=c"

src/EventListener/ControllerSubscriber.php

@@ -8,14 +8,14 @@
 use Setono\SyliusRedirectPlugin\Resolver\RedirectionPathResolverInterface;
 use Sylius\Component\Channel\Context\ChannelContextInterface;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
-use Symfony\Component\HttpFoundation\RedirectResponse;
-use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Event\ControllerEvent;
 use Symfony\Component\HttpKernel\KernelEvents;
 use Webmozart\Assert\Assert;
 
 final class ControllerSubscriber implements EventSubscriberInterface
 {
+    use RedirectResponseTrait;
+
     /** @var ObjectManager */
     private $objectManager;
 
@@ -44,8 +44,9 @@ public static function getSubscribedEvents(): array
 
     public function onKernelController(ControllerEvent $event): void
     {
+        $request = $event->getRequest();
         $redirectionPath = $this->redirectionPathResolver->resolveFromRequest(
-            $event->getRequest(),
+            $request,
             $this->channelContext->getChannel()
         );
 
@@ -60,11 +61,6 @@ public function onKernelController(ControllerEvent $event): void
         $lastRedirect = $redirectionPath->last();
         Assert::notNull($lastRedirect);
 
-        $event->setController(static function () use ($lastRedirect): RedirectResponse {
-            return new RedirectResponse(
-                $lastRedirect->getDestination(),
-                $lastRedirect->isPermanent() ? Response::HTTP_MOVED_PERMANENTLY : Response::HTTP_FOUND
-            );
-        });
+        $event->setController(static fn () => self::getRedirectResponse($lastRedirect, $request->getQueryString()));
     }
 }

src/EventListener/NotFoundSubscriber.php

@@ -8,7 +8,6 @@
 use Setono\SyliusRedirectPlugin\Resolver\RedirectionPathResolverInterface;
 use Sylius\Component\Channel\Context\ChannelContextInterface;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
-use Symfony\Component\HttpFoundation\RedirectResponse;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Event\ExceptionEvent;
 use Symfony\Component\HttpKernel\Exception\HttpException;
@@ -17,6 +16,8 @@
 
 class NotFoundSubscriber implements EventSubscriberInterface
 {
+    use RedirectResponseTrait;
+
     /** @var ObjectManager */
     private $objectManager;
 
@@ -54,8 +55,9 @@ public function onKernelException(ExceptionEvent $event): void
             return;
         }
 
+        $request = $event->getRequest();
         $redirectionPath = $this->redirectionPathResolver->resolveFromRequest(
-            $event->getRequest(),
+            $request,
             $this->channelContext->getChannel(),
             true
         );
@@ -70,9 +72,6 @@ public function onKernelException(ExceptionEvent $event): void
         $lastRedirect = $redirectionPath->last();
         Assert::notNull($lastRedirect);
 
-        $event->setResponse(new RedirectResponse(
-            $lastRedirect->getDestination(),
-            $lastRedirect->isPermanent() ? Response::HTTP_MOVED_PERMANENTLY : Response::HTTP_FOUND
-        ));
+        $event->setResponse(self::getRedirectResponse($lastRedirect, $request->getQueryString()));
     }
 }

src/EventListener/RedirectResponseTrait.php

@@ -0,0 +1,31 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Setono\SyliusRedirectPlugin\EventListener;
+
+use League\Uri\Uri;
+use League\Uri\UriModifier;
+use Setono\SyliusRedirectPlugin\Model\RedirectInterface;
+use Symfony\Component\HttpFoundation\RedirectResponse;
+use Symfony\Component\HttpFoundation\Response;
+
+/**
+ * @internal
+ */
+trait RedirectResponseTrait
+{
+    public static function getRedirectResponse(RedirectInterface $lastRedirect, string $queryString = null): RedirectResponse
+    {
+        $uri = Uri::createFromString($lastRedirect->getDestination());
+
+        if ($lastRedirect->keepQueryString() && null !== $queryString) {
+            $uri = UriModifier::appendQuery($uri, $queryString);
+        }
+
+        return new RedirectResponse(
+            $uri->__toString(),
+            $lastRedirect->isPermanent() ? Response::HTTP_MOVED_PERMANENTLY : Response::HTTP_FOUND
+        );
+    }
+}

src/Form/Type/RedirectType.php

@@ -39,6 +39,10 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
                 'label' => 'setono_sylius_redirect.form.redirect.only_404',
                 'required' => false,
             ])
+            ->add('keepQueryString', CheckboxType::class, [
+                'label' => 'setono_sylius_redirect.form.redirect.keep_query_string',
+                'required' => false,
+            ])
             ->add('channels', ChannelChoiceType::class, [
                 'multiple' => true,
                 'expanded' => true,

src/Model/Redirect.php

@@ -40,6 +40,9 @@ class Redirect implements RedirectInterface
     /** @var bool */
     protected $only404 = true;
 
+    /** @var bool */
+    protected $keepQueryString = false;
+
     /** @var Collection|ChannelInterface[] */
     protected $channels;
 
@@ -142,4 +145,14 @@ public function hasChannel(BaseChannelInterface $channel): bool
     {
         return $this->channels->contains($channel);
     }
+
+    public function keepQueryString(): bool
+    {
+        return $this->keepQueryString;
+    }
+
+    public function setKeepQueryString(bool $keepQueryString): void
+    {
+        $this->keepQueryString = $keepQueryString;
+    }
 }

src/Model/RedirectInterface.php

@@ -46,4 +46,8 @@ public function setEnabled(?bool $enabled): void;
     public function isOnly404(): bool;
 
     public function setOnly404(bool $only404): void;
+
+    public function keepQueryString(): bool;
+
+    public function setKeepQueryString(bool $keepQueryString): void;
 }

src/Resources/config/doctrine/model/Redirect.orm.xml

@@ -25,6 +25,11 @@
         <field name="lastAccessed" column="last_accessed" type="datetime" nullable="true"/>
         <field name="enabled" column="enabled" type="boolean"/>
         <field name="only404" column="only_404" type="boolean"/>
+        <field name="keepQueryString" column="keep_query_string" type="boolean">
+            <options>
+                <option name="default">0</option>
+            </options>
+        </field>
         <field name="createdAt" column="created_at" type="datetime">
             <gedmo:timestampable on="create"/>
         </field>

src/Resources/translations/messages.en.yaml

@@ -29,3 +29,5 @@ setono_sylius_redirect:
             permanent_help: The most used redirect is a permanent (301) redirect. This is also the one recommended for SEO purposes
             source: Source
             source_placeholder: /en_US/redirect-from
+            keep_query_string: Keep query string
+            keep_query_string_help: 'If this is set, the redirect will keep the query string, e.g /en_US/redirect-from?q=ts will redirect you to the destination keeping all query params: /en_US/redirect-to?q=ts'

src/Resources/views/Admin/Redirect/_form.html.twig

@@ -17,6 +17,10 @@
             <div class="ui info message">
                 {{ 'setono_sylius_redirect.form.redirect.only_404_help'|trans }}
             </div>
+            {{ form_row(form.keepQueryString) }}
+            <div class="ui info message">
+                {{ 'setono_sylius_redirect.form.redirect.keep_query_string_help'|trans }}
+            </div>
         </div>
     </div>
     <div class="column">

tests/Behat/Context/Setup/RedirectContext.php

@@ -44,6 +44,7 @@ private function createRedirect(string $oldPath, string $newPath, ChannelInterfa
         $redirect->setSource($oldPath);
         $redirect->setDestination($newPath);
         $redirect->setPermanent(true);
+        $redirect->setKeepQueryString(true);
         if (null !== $channel) {
             $redirect->addChannel($channel);
         }

tests/Behat/Page/Shop/Page.php

@@ -5,6 +5,7 @@
 namespace Tests\Setono\SyliusRedirectPlugin\Behat\Page\Shop;
 
 use FriendsOfBehat\PageObjectExtension\Page\Page as BasePage;
+use League\Uri\Uri;
 use Webmozart\Assert\Assert;
 
 class Page extends BasePage
@@ -21,9 +22,11 @@ public function openOldPath(string $path): void
 
     public function isOnPath(string $path): void
     {
-        $currentPath = parse_url($this->getSession()->getCurrentUrl(), \PHP_URL_PATH);
+        $currentPath = (Uri::createFromString($this->getSession()->getCurrentUrl()))
+            ->withScheme(null)
+            ->withHost(null);
 
-        Assert::same($currentPath, $path);
+        Assert::same($currentPath->__toString(), $path);
     }
 
     protected function getUrl(array $urlParameters = []): string