Centralize pagination/canonical meta URL generation in Document (#3077)

* Centralize pagination/canonical meta URL generation in Document * Apply fixes from StyleCI [ci skip] [skip ci] * Use translations for title template * Apply fixes from StyleCI [ci skip] [skip ci] * Dont add translator to title driver interface It's an implementation detail, and can be made available on specific implementations as needed. Co-authored-by: Alexander Skvortsov <askvortsov1@users.noreply.github.com>
flarum · Dec 3, 2021 · dcc9868 · dcc9868
1 parent 02f3510
commit dcc9868
Show file tree

Hide file tree

Showing 5 changed files with 99 additions and 3 deletions.
diff --git a/locale/core.yml b/locale/core.yml
@@ -628,6 +628,11 @@ core:
       submit_button: => core.ref.save_changes
       title: => core.ref.reset_your_password
 
+    # Translations in this namespace are used to format page meta titles.
+    meta_titles:
+      with_page_title: "{pageNumber, plural, =1 {{pageTitle} - {forumName}} other {{pageTitle}: Page # - {forumName}}}"
+      without_page_title: "{pageNumber, plural, =1 {{forumName}} other {Page # - {forumName}}}"
+
   # Translations in this namespace are used in messages output by the API.
   api:
     invalid_username_message: "The username may only contain letters, numbers, and dashes."

diff --git a/src/Forum/Content/Discussion.php b/src/Forum/Content/Discussion.php
@@ -97,10 +97,13 @@ public function __invoke(Document $document, Request $request)
         $hasNextPage = $page < 1 + intval($apiDocument->data->attributes->commentCount / 20);
 
         $document->title = $apiDocument->data->attributes->title;
-        $document->canonicalUrl = $url(['page' => $page]);
         $document->content = $this->view->make('flarum.forum::frontend.content.discussion', compact('apiDocument', 'page', 'hasPrevPage', 'hasNextPage', 'getResource', 'posts', 'url'));
         $document->payload['apiDocument'] = $apiDocument;
 
+        $document->canonicalUrl = $url([]);
+        $document->page = $page;
+        $document->hasNextPage = $hasNextPage;
+
         return $document;
     }
 

diff --git a/src/Forum/Content/Index.php b/src/Forum/Content/Index.php
@@ -88,7 +88,10 @@ public function __invoke(Document $document, Request $request)
         $document->title = $this->translator->trans('core.forum.index.meta_title_text');
         $document->content = $this->view->make('flarum.forum::frontend.content.index', compact('apiDocument', 'page'));
         $document->payload['apiDocument'] = $apiDocument;
+
         $document->canonicalUrl = $this->url->to('forum')->base().($defaultRoute === '/all' ? '' : $request->getUri()->getPath());
+        $document->page = $page;
+        $document->hasNextPage = isset($apiDocument->links->next);
 
         return $document;
     }

diff --git a/src/Frontend/Document.php b/src/Frontend/Document.php
@@ -95,6 +95,24 @@ class Document implements Renderable
      */
     public $canonicalUrl;
 
+    /**
+     * Which page of content are we on?
+     *
+     * This is used to build prev/next meta links for SEO.
+     *
+     * @var null|int
+     */
+    public $page;
+
+    /**
+     * Is there a next page?
+     *
+     * This is used with $page to build next meta links for SEO.
+     *
+     * @var null|bool
+     */
+    public $hasNextPage;
+
     /**
      * An array of strings to append to the page's <head>.
      *
@@ -243,8 +261,17 @@ protected function makeHead(): string
             return '<link rel="stylesheet" href="'.e($url).'">';
         }, $this->css);
 
+        if ($this->page) {
+            if ($this->page > 1) {
+                $head[] = '<link rel="prev" href="'.e(self::setPageParam($this->canonicalUrl, $this->page - 1)).'">';
+            }
+            if ($this->hasNextPage) {
+                $head[] = '<link rel="next" href="'.e(self::setPageParam($this->canonicalUrl, $this->page + 1)).'">';
+            }
+        }
+
         if ($this->canonicalUrl) {
-            $head[] = '<link rel="canonical" href="'.e($this->canonicalUrl).'">';
+            $head[] = '<link rel="canonical" href="'.e(self::setPageParam($this->canonicalUrl, $this->page)).'">';
         }
 
         $head = array_merge($head, $this->makePreloads());
@@ -289,4 +316,43 @@ public function setForumApiDocument(array $forumApiDocument)
     {
         $this->forumApiDocument = $forumApiDocument;
     }
+
+    public static function setPageParam(string $url, ?int $page)
+    {
+        if (! $page || $page === 1) {
+            return self::setQueryParam($url, 'page', null);
+        }
+
+        return self::setQueryParam($url, 'page', (string) $page);
+    }
+
+    /**
+     * Set or override a query param on a string URL to a particular value.
+     */
+    protected static function setQueryParam(string $url, string $key, ?string $value)
+    {
+        if (filter_var($url, FILTER_VALIDATE_URL)) {
+            $urlParts = parse_url($url);
+            if (isset($urlParts['query'])) {
+                parse_str($urlParts['query'], $urlQueryArgs);
+
+                if ($value === null) {
+                    unset($urlQueryArgs[$key]);
+                } else {
+                    $urlQueryArgs[$key] = $value;
+                }
+
+                $urlParts['query'] = http_build_query($urlQueryArgs);
+                $newUrl = $urlParts['scheme'].'://'.$urlParts['host'].$urlParts['path'].'?'.$urlParts['query'];
+            } elseif ($value !== null) {
+                $newUrl = $url.'?'.http_build_query([$key => $value]);
+            } else {
+                return $url;
+            }
+
+            return $newUrl;
+        } else {
+            return $url;
+        }
+    }
 }
diff --git a/src/Frontend/Driver/BasicTitleDriver.php b/src/Frontend/Driver/BasicTitleDriver.php
@@ -12,13 +12,32 @@
 use Flarum\Frontend\Document;
 use Illuminate\Support\Arr;
 use Psr\Http\Message\ServerRequestInterface;
+use Symfony\Contracts\Translation\TranslatorInterface;
 
 class BasicTitleDriver implements TitleDriverInterface
 {
+    /**
+     * @var TranslatorInterface
+     */
+    protected $translator;
+
+    public function __construct(TranslatorInterface $translator)
+    {
+        $this->translator = $translator;
+    }
+
     public function makeTitle(Document $document, ServerRequestInterface $request, array $forumApiDocument): string
     {
         $onHomePage = rtrim($request->getUri()->getPath(), '/') === '';
 
-        return ($document->title && ! $onHomePage ? $document->title.' - ' : '').Arr::get($forumApiDocument, 'data.attributes.title');
+        $params = [
+            'pageTitle' => $document->title ?? '',
+            'forumName' => Arr::get($forumApiDocument, 'data.attributes.title'),
+            'pageNumber' => $document->page ?? 1,
+        ];
+
+        return $onHomePage || ! $document->title
+            ? $this->translator->trans('core.views.meta_titles.without_page_title', $params)
+            : $this->translator->trans('core.views.meta_titles.with_page_title', $params);
     }
 }