Process remove tag endpoint through Slim controller

5 files changed, 135 insertions, 32 deletions

diff --git a/application/front/controllers/TagController.php b/application/front/controllers/TagController.php
index 598275b0..a1d5ad5b 100644
--- a/application/front/controllers/TagController.php
+++ b/application/front/controllers/TagController.php
@@ -35,7 +35,7 @@ class TagController extends ShaarliController
             return $response->withRedirect('./');
         }
 
-        $currentUrl = parse_url($this->container->environment['HTTP_REFERER']);
+        $currentUrl = parse_url($referer);
         parse_str($currentUrl['query'] ?? '', $params);
 
         if (null === $newTag) {
@@ -71,4 +71,50 @@ class TagController extends ShaarliController
 
         return $response->withRedirect(($currentUrl['path'] ?? './') .'?'. http_build_query($params));
     }
+
+    /**
+     * Remove a tag from the current search through an HTTP redirection.
+     *
+     * @param array $args Should contain `tag` key as tag to remove from current search
+     */
+    public function removeTag(Request $request, Response $response, array $args): Response
+    {
+        $referer = $this->container->environment['HTTP_REFERER'] ?? null;
+
+        // If the referrer is not provided, we can update the search, so we failback on the bookmark list
+        if (empty($referer)) {
+            return $response->withRedirect('./');
+        }
+
+        $tagToRemove = $args['tag'] ?? null;
+        $currentUrl = parse_url($referer);
+        parse_str($currentUrl['query'] ?? '', $params);
+
+        if (null === $tagToRemove) {
+            return $response->withRedirect(($currentUrl['path'] ?? './') .'?'. http_build_query($params));
+        }
+
+        // Prevent redirection loop
+        if (isset($params['removetag'])) {
+            unset($params['removetag']);
+        }
+
+        if (isset($params['searchtags'])) {
+            $tags = explode(' ', $params['searchtags']);
+            // Remove value from array $tags.
+            $tags = array_diff($tags, [$tagToRemove]);
+            $params['searchtags'] = implode(' ', $tags);
+
+            if (empty($params['searchtags'])) {
+                unset($params['searchtags']);
+            }
+
+            // We also remove page (keeping the same page has no sense, since the results are different)
+            unset($params['page']);
+        }
+
+        $queryParams = count($params) > 0 ? '?' . http_build_query($params) : '';
+
+        return $response->withRedirect(($currentUrl['path'] ?? './') . $queryParams);
+    }
 }
diff --git a/index.php b/index.php
index 04ec0d73..c0e0c66d 100644
--- a/index.php
+++ b/index.php
@@ -451,35 +451,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM
 
     // -------- User clicks on a tag in result count: Remove the tag from the list of searched tags (searchtags=...)
     if (isset($_GET['removetag'])) {
-        // Get previous URL (http_referer) and remove the tag from the searchtags parameters in query.
-        if (empty($_SERVER['HTTP_REFERER'])) {
-            header('Location: ?');
-            exit;
-        }
-
-        // In case browser does not send HTTP_REFERER
-        parse_str(parse_url($_SERVER['HTTP_REFERER'], PHP_URL_QUERY), $params);
-
-        // Prevent redirection loop
-        if (isset($params['removetag'])) {
-            unset($params['removetag']);
-        }
-
-        if (isset($params['searchtags'])) {
-            $tags = explode(' ', $params['searchtags']);
-            // Remove value from array $tags.
-            $tags = array_diff($tags, array($_GET['removetag']));
-            $params['searchtags'] = implode(' ', $tags);
-
-            if (empty($params['searchtags'])) {
-                unset($params['searchtags']);
-            }
-
-            // We also remove page (keeping the same page has no sense, since
-            // the results are different)
-            unset($params['page']);
-        }
-        header('Location: ?'.http_build_query($params));
+        header('Location: ./remove-tag/'. $_GET['removetag']);
         exit;
     }
 
@@ -1576,6 +1548,7 @@ $app->group('', function () {
     $this->get('/open-search', '\Shaarli\Front\Controller\OpenSearchController:index')->setName('opensearch');
 
     $this->get('/add-tag/{newTag}', '\Shaarli\Front\Controller\TagController:addTag')->setName('add-tag');
+    $this->get('/remove-tag/{tag}', '\Shaarli\Front\Controller\TagController:removeTag')->setName('remove-tag');
 })->add('\Shaarli\Front\ShaarliMiddleware');
 
 $response = $app->run(true);
diff --git a/tests/front/controller/TagControllerTest.php b/tests/front/controller/TagControllerTest.php
index 5eea537b..2184cb11 100644
--- a/tests/front/controller/TagControllerTest.php
+++ b/tests/front/controller/TagControllerTest.php
@@ -157,4 +157,86 @@ class TagControllerTest extends TestCase
         static::assertSame(302, $result->getStatusCode());
         static::assertSame(['./'], $result->getHeader('location'));
     }
+
+    public function testRemoveTagWithoutMatchingTag(): void
+    {
+        $this->createValidContainerMockSet();
+
+        $this->container->environment = ['HTTP_REFERER' => 'http://shaarli/controller/?searchtags=def'];
+
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $tags = ['tag' => 'abc'];
+
+        $result = $this->controller->removeTag($request, $response, $tags);
+
+        static::assertInstanceOf(Response::class, $result);
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/controller/?searchtags=def'], $result->getHeader('location'));
+    }
+
+    public function testRemoveTagWithoutTagsearch(): void
+    {
+        $this->createValidContainerMockSet();
+
+        $this->container->environment = ['HTTP_REFERER' => 'http://shaarli/controller/'];
+
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $tags = ['tag' => 'abc'];
+
+        $result = $this->controller->removeTag($request, $response, $tags);
+
+        static::assertInstanceOf(Response::class, $result);
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/controller/'], $result->getHeader('location'));
+    }
+
+    public function testRemoveTagWithoutReferer(): void
+    {
+        $this->createValidContainerMockSet();
+
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $tags = ['tag' => 'abc'];
+
+        $result = $this->controller->removeTag($request, $response, $tags);
+
+        static::assertInstanceOf(Response::class, $result);
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['./'], $result->getHeader('location'));
+    }
+
+    public function testRemoveTagWithoutTag(): void
+    {
+        $this->createValidContainerMockSet();
+
+        $this->container->environment = ['HTTP_REFERER' => 'http://shaarli/controller/?searchtag=abc'];
+
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $result = $this->controller->removeTag($request, $response, []);
+
+        static::assertInstanceOf(Response::class, $result);
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/controller/?searchtag=abc'], $result->getHeader('location'));
+    }
+
+    public function testRemoveTagWithoutTagWithoutReferer(): void
+    {
+        $this->createValidContainerMockSet();
+
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $result = $this->controller->removeTag($request, $response, []);
+
+        static::assertInstanceOf(Response::class, $result);
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['./'], $result->getHeader('location'));
+    }
 }
diff --git a/tpl/default/linklist.html b/tpl/default/linklist.html
index d8a15823..e574a109 100644
--- a/tpl/default/linklist.html
+++ b/tpl/default/linklist.html
@@ -94,7 +94,9 @@
           {'tagged'|t}
           {loop="$exploded_tags"}
               <span class="label label-tag" title="{'Remove tag'|t}">
-                <a href="?removetag={function="urlencode($value)"}" aria-label="{'Remove tag'|t}">{$value}<span class="remove"><i class="fa fa-times" aria-hidden="true"></i></span></a>
+                <a href="./remove-tag/{function="urlencode($value)"}" aria-label="{'Remove tag'|t}">
+                  {$value}<span class="remove"><i class="fa fa-times" aria-hidden="true"></i></span>
+                </a>
               </span>
           {/loop}
         {/if}
diff --git a/tpl/vintage/linklist.html b/tpl/vintage/linklist.html
index 8f1ded95..502abcf9 100644
--- a/tpl/vintage/linklist.html
+++ b/tpl/vintage/linklist.html
@@ -66,7 +66,7 @@
                 tagged
                 {loop="$exploded_tags"}
                     <span class="linktag" title="Remove tag">
-                        <a href="?removetag={function="urlencode($value)"}">{$value} <span class="remove">x</span></a>
+                        <a href="./remove-tag/{function="urlencode($value)"}">{$value} <span class="remove">x</span></a>
                     </span>
                 {/loop}
             {elseif="$search_tags === false"}