Initial tag load performance improvement (#87)

- Only load lastPostedDiscussion on TagsPage
- For forum payload, only load top-level primary tags and top 3 secondary tags.
- In other cases, load tags in dynamically when needed.


Co-authored-by: Alexander Skvortsov <sasha.skvortsov109@gmail.com>
Co-authored-by: Sami Mazouz <sychocouldy@gmail.com>

extensions/tags/extend.php

@@ -45,6 +45,7 @@ return [
         ->get('/tags', 'tags.index', Controller\ListTagsController::class)
         ->post('/tags', 'tags.create', Controller\CreateTagController::class)
         ->post('/tags/order', 'tags.order', Controller\OrderTagsController::class)
+        ->get('/tags/{slug}', 'tags.show', Controller\ShowTagController::class)
         ->patch('/tags/{id}', 'tags.update', Controller\UpdateTagController::class)
         ->delete('/tags/{id}', 'tags.delete', Controller\DeleteTagController::class),
 
@@ -80,7 +81,7 @@ return [
         ->addInclude(['tags', 'tags.state']),
 
     (new Extend\ApiController(FlarumController\ShowForumController::class))
-        ->addInclude(['tags', 'tags.lastPostedDiscussion', 'tags.parent'])
+        ->addInclude(['tags', 'tags.parent'])
         ->prepareDataForSerialization(LoadForumTagsRelationship::class),
 
     (new Extend\Settings())

extensions/tags/js/src/admin/components/TagsPage.js

@@ -2,6 +2,7 @@ import sortable from 'sortablejs';
 
 import ExtensionPage from 'flarum/components/ExtensionPage';
 import Button from 'flarum/components/Button';
+import LoadingIndicator from 'flarum/components/LoadingIndicator';
 import withAttr from 'flarum/utils/withAttr';
 
 import EditTagModal from './EditTagModal';
@@ -40,15 +41,29 @@ export default class TagsPage extends ExtensionPage {
     // we force a full reconstruction of the DOM by changing the key, which
     // makes mithril completely re-render the component on redraw.
     this.forcedRefreshKey = 0;
+
+    this.loading = true;
+
+    app.store.find('tags', { include: 'parent,lastPostedDiscussion' }).then(() => {
+      this.loading = false;
+
+      m.redraw();
+    });
   }
 
   content() {
+    if (this.loading) {
+      return <LoadingIndicator />;
+    }
+
     const minPrimaryTags = this.setting('flarum-tags.min_primary_tags', 0);
     const maxPrimaryTags = this.setting('flarum-tags.max_primary_tags', 0);
 
     const minSecondaryTags = this.setting('flarum-tags.min_secondary_tags', 0);
     const maxSecondaryTags = this.setting('flarum-tags.max_secondary_tags', 0);
 
+    const tags = sortTags(app.store.all('tags').filter(tag => !tag.parent()));
+    
     return (
       <div className="TagsContent">
         <div className="TagsContent-list">
@@ -56,8 +71,8 @@ export default class TagsPage extends ExtensionPage {
             <div className="TagGroup">
               <label>{app.translator.trans('flarum-tags.admin.tags.primary_heading')}</label>
               <ol className="TagList TagList--primary">
-                {sortTags(app.store.all('tags'))
-                  .filter((tag) => tag.position() !== null && !tag.isChild())
+                {tags
+                  .filter(tag => tag.position() !== null && !tag.isChild())
                   .map(tagItem)}
               </ol>
               {Button.component(
@@ -73,9 +88,8 @@ export default class TagsPage extends ExtensionPage {
             <div className="TagGroup TagGroup--secondary">
               <label>{app.translator.trans('flarum-tags.admin.tags.secondary_heading')}</label>
               <ul className="TagList">
-                {app.store
-                  .all('tags')
-                  .filter((tag) => tag.position() === null)
+                {tags
+                  .filter(tag => tag.position() === null)
                   .sort((a, b) => a.name().localeCompare(b.name()))
                   .map(tagItem)}
               </ul>

extensions/tags/js/src/common/models/Tag.js

@@ -14,6 +14,7 @@ export default class Tag extends mixin(Model, {
 
   position: Model.attribute('position'),
   parent: Model.hasOne('parent'),
+  children: Model.hasMany('children'),
   defaultSort: Model.attribute('defaultSort'),
   isChild: Model.attribute('isChild'),
   isHidden: Model.attribute('isHidden'),

extensions/tags/js/src/forum/addTagComposer.js

@@ -9,7 +9,8 @@ import getSelectableTags from './utils/getSelectableTags';
 
 export default function () {
   extend(IndexPage.prototype, 'newDiscussionAction', function (promise) {
-    const tag = app.store.getBy('tags', 'slug', app.search.params().tags);
+    // From `addTagFilter
+    const tag = this.currentTag();
 
     if (tag) {
       const parent = tag.parent();
@@ -20,6 +21,11 @@ export default function () {
     }
   });
 
+
+  extend(DiscussionComposer.prototype, 'oninit', function () {
+    app.tagList.load(['parent']).then(() => m.redraw())
+  });
+
   // Add tag-selection abilities to the discussion composer.
   DiscussionComposer.prototype.chooseTags = function () {
     const selectableTags = getSelectableTags();

extensions/tags/js/src/forum/addTagFilter.js

@@ -7,9 +7,33 @@ import TagHero from './components/TagHero';
 
 export default function() {
   IndexPage.prototype.currentTag = function() {
-    const slug = app.search.params().tags;
+    if (this.currentActiveTag) {
+      return this.currentActiveTag;
+    }
 
-    if (slug) return app.store.getBy('tags', 'slug', slug);
+    const slug = app.search.params().tags;
+    let tag = null;
+
+    if (slug) {
+      tag = app.store.getBy('tags', 'slug', slug);
+    }
+
+    if (slug && !tag || (tag && !tag.isChild() && !tag.children())) {
+      // Unlike the backend, no need to fetch parent.children because if we're on
+      // a child tag page, then either:
+      //    - We loaded in that child tag (and its siblings) in the API document
+      //    - We first navigated to the current tag's parent, which would have loaded in the current tag's siblings.
+      app.store.find('tags', slug, { include: 'children,children.parent,parent,state'}).then(() => {
+        this.currentActiveTag = app.store.getBy('tags', 'slug', slug);
+
+        m.redraw();
+      });
+    }
+
+    if (tag) {
+      this.currentActiveTag = tag;
+      return this.currentActiveTag;
+    }
   };
 
   // If currently viewing a tag, insert a tag hero at the top of the view.

extensions/tags/js/src/forum/components/TagDiscussionModal.js

@@ -1,6 +1,7 @@
 import Modal from 'flarum/components/Modal';
 import DiscussionPage from 'flarum/components/DiscussionPage';
 import Button from 'flarum/components/Button';
+import LoadingIndicator from 'flarum/components/LoadingIndicator';
 import highlight from 'flarum/helpers/highlight';
 import classList from 'flarum/utils/classList';
 import extractText from 'flarum/utils/extractText';
@@ -16,21 +17,12 @@ export default class TagDiscussionModal extends Modal {
   oninit(vnode) {
     super.oninit(vnode);
 
-    this.tags = getSelectableTags(this.attrs.discussion);
-
-    this.tags = sortTags(this.tags);
+    this.tagsLoading = true;
 
     this.selected = [];
     this.filter = Stream('');
-    this.index = this.tags[0].id();
     this.focused = false;
 
-    if (this.attrs.selectedTags) {
-      this.attrs.selectedTags.map(this.addTag.bind(this));
-    } else if (this.attrs.discussion) {
-      this.attrs.discussion.tags().map(this.addTag.bind(this));
-    }
-
     this.minPrimary = app.forum.attribute('minPrimaryTags');
     this.maxPrimary = app.forum.attribute('maxPrimaryTags');
     this.minSecondary = app.forum.attribute('minSecondaryTags');
@@ -42,6 +34,22 @@ export default class TagDiscussionModal extends Modal {
       .onDown(() => this.setIndex(this.getCurrentNumericIndex() + 1, true))
       .onSelect(this.select.bind(this))
       .onRemove(() => this.selected.splice(this.selected.length - 1, 1));
+
+    app.tagList.load(['parent']).then(() => {
+      this.tagsLoading = false;
+
+      this.tags = sortTags(getSelectableTags(this.attrs.discussion));
+
+      if (this.attrs.selectedTags) {
+        this.attrs.selectedTags.map(this.addTag.bind(this));
+      } else if (this.attrs.discussion) {
+        this.attrs.discussion.tags().map(this.addTag.bind(this));
+      }
+
+      this.index = this.tags[0].id();
+
+      m.redraw();
+    });
   }
 
   primaryCount() {
@@ -113,6 +121,10 @@ export default class TagDiscussionModal extends Modal {
   }
 
   content() {
+    if (this.tagsLoading) {
+      return <LoadingIndicator />;
+    }
+  
     let tags = this.tags;
     const filter = this.filter().toLowerCase();
     const primaryCount = this.primaryCount();

extensions/tags/js/src/forum/components/TagsPage.js

@@ -1,6 +1,7 @@
 import Page from 'flarum/components/Page';
 import IndexPage from 'flarum/components/IndexPage';
 import Link from 'flarum/components/Link';
+import LoadingIndicator from 'flarum/components/LoadingIndicator';
 import listItems from 'flarum/helpers/listItems';
 import humanTime from 'flarum/helpers/humanTime';
 
@@ -12,12 +13,33 @@ export default class TagsPage extends Page {
   oninit(vnode) {
     super.oninit(vnode);
 
-    this.tags = sortTags(app.store.all('tags').filter(tag => !tag.parent()));
-
     app.history.push('tags', app.translator.trans('flarum-tags.forum.header.back_to_tags_tooltip'));
+
+    this.tags = [];
+
+    const preloaded = app.preloadedApiDocument();
+
+    if (preloaded) {
+      this.tags = sortTags(preloaded.filter(tag => !tag.isChild()));
+      return;
+    }    
+
+    this.loading = true;
+
+    app.tagList.load(['children', 'lastPostedDiscussion']).then(() => {
+      this.tags = sortTags(app.store.all('tags').filter(tag => !tag.isChild()));
+
+      this.loading = false;
+
+      m.redraw();
+    });
   }
 
   view() {
+    if (this.loading) {
+      return <LoadingIndicator />;
+    }
+
     const pinned = this.tags.filter(tag => tag.position() !== null);
     const cloud = this.tags.filter(tag => tag.position() === null);
 
@@ -33,7 +55,7 @@ export default class TagsPage extends Page {
             <ul className="TagTiles">
               {pinned.map(tag => {
                 const lastPostedDiscussion = tag.lastPostedDiscussion();
-                const children = sortTags(app.store.all('tags').filter(child => child.parent() === tag));
+                const children = sortTags(tag.children() || []);
 
                 return (
                   <li className={'TagTile ' + (tag.color() ? 'colored' : '')}

extensions/tags/js/src/forum/index.js

@@ -6,6 +6,8 @@ import Tag from '../common/models/Tag';
 import TagsPage from './components/TagsPage';
 import DiscussionTaggedPost from './components/DiscussionTaggedPost';
 
+import TagListState from './states/TagListState';
+
 import addTagList from './addTagList';
 import addTagFilter from './addTagFilter';
 import addTagLabels from './addTagLabels';
@@ -22,6 +24,8 @@ app.initializers.add('flarum-tags', function(app) {
 
   app.store.models.tags = Tag;
 
+  app.tagList = new TagListState();
+
   Discussion.prototype.tags = Model.hasMany('tags');
   Discussion.prototype.canTag = Model.attribute('canTag');
 

extensions/tags/js/src/forum/states/TagListState.js

@@ -0,0 +1,20 @@
+export default class TagListState {
+    constructor() {
+        this.loadedIncludes = new Set();
+    }
+
+    async load(includes = []) {
+        const unloadedIncludes = includes.filter(include => !this.loadedIncludes.has(include));
+
+        if (unloadedIncludes.length === 0) {
+            return Promise.resolve(app.store.all('tags'));
+        }
+
+        return app.store
+            .find('tags', { include: 'parent,lastPostedDiscussion' })
+            .then(val => {
+                unloadedIncludes.forEach(include => this.loadedIncludes.add(include));
+                return val;
+            });
+    }
+}

extensions/tags/src/Api/Controller/ListTagsController.php

@@ -27,14 +27,16 @@ class ListTagsController extends AbstractListController
      * {@inheritdoc}
      */
     public $include = [
-        'parent',
+        'parent'
     ];
 
     /**
      * {@inheritdoc}
      */
     public $optionalInclude = [
+        'children',
         'lastPostedDiscussion',
+        'state'
     ];
 
     /**
@@ -58,6 +60,10 @@ class ListTagsController extends AbstractListController
         $actor = RequestUtil::getActor($request);
         $include = $this->extractInclude($request);
 
+        if (in_array('lastPostedDiscussion', $include)) {
+            $include = array_merge($include, ['lastPostedDiscussion.tags', 'lastPostedDiscussion.state']);
+        }
+
         $tags = $this->tags->whereVisibleTo($actor)->withStateFor($actor)->get();
 
         return $tags->load($include);

extensions/tags/src/Api/Controller/ShowTagController.php

@@ -0,0 +1,58 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Tags\Api\Controller;
+
+use Flarum\Api\Controller\AbstractShowController;
+use Flarum\Tags\Api\Serializer\TagSerializer;
+use Flarum\Tags\Tag;
+use Illuminate\Support\Arr;
+use Psr\Http\Message\ServerRequestInterface;
+use Tobscure\JsonApi\Document;
+
+class ShowTagController extends AbstractShowController
+{
+    public $serializer = TagSerializer::class;
+
+    public $optionalInclude = [
+        'children',
+        'children.parent',
+        'lastPostedDiscussion',
+        'parent',
+        'parent.children',
+        'parent.children.parent',
+        'state'
+    ];
+
+    /**
+     * @var Tag
+     */
+    private $tags;
+
+    public function __construct(Tag $tags)
+    {
+        $this->tags = $tags;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    protected function data(ServerRequestInterface $request, Document $document)
+    {
+        $slug = Arr::get($request->getQueryParams(), 'slug');
+        $actor = $request->getAttribute('actor');
+        $include = $this->extractInclude($request);
+
+        return $this->tags
+            ->whereVisibleTo($actor)
+            ->with($include)
+            ->where('slug', $slug)
+            ->firstOrFail();
+    }
+}

extensions/tags/src/Api/Serializer/TagSerializer.php

@@ -57,6 +57,11 @@ class TagSerializer extends AbstractSerializer
         return $this->hasOne($tag, self::class);
     }
 
+    protected function children($tag)
+    {
+        return $this->hasMany($tag, self::class);
+    }
+
     /**
      * @return \Tobscure\JsonApi\Relationship
      */

extensions/tags/src/Content/Tag.php

@@ -13,12 +13,13 @@ use Flarum\Api\Client;
 use Flarum\Api\Controller\ListDiscussionsController;
 use Flarum\Frontend\Document;
 use Flarum\Http\RequestUtil;
+use Flarum\Tags\Api\Controller\ShowTagController;
 use Flarum\Tags\TagRepository;
 use Flarum\User\User;
-use Illuminate\Contracts\Translation\Translator;
 use Illuminate\Contracts\View\Factory;
 use Illuminate\Support\Arr;
 use Psr\Http\Message\ServerRequestInterface as Request;
+use Symfony\Contracts\Translation\TranslatorInterface;
 
 class Tag
 {
@@ -38,7 +39,7 @@ class Tag
     protected $tags;
 
     /**
-     * @var Translator
+     * @var TranslatorInterface
      */
     protected $translator;
 
@@ -46,9 +47,9 @@ class Tag
      * @param Client $api
      * @param Factory $view
      * @param TagRepository $tags
-     * @param Translator $translator
+     * @param TranslatorInterface $translator
      */
-    public function __construct(Client $api, Factory $view, TagRepository $tags, Translator $translator)
+    public function __construct(Client $api, Factory $view, TagRepository $tags, TranslatorInterface $translator)
     {
         $this->api = $api;
         $this->view = $view;
@@ -74,13 +75,20 @@ class Tag
         $params = [
             'sort' => $sort && isset($sortMap[$sort]) ? $sortMap[$sort] : '',
             'filter' => [
-                'q' => "$q tag:$slug"
+                'tag' => "$slug"
             ],
             'page' => ['offset' => ($page - 1) * 20, 'limit' => 20]
         ];
 
         $apiDocument = $this->getApiDocument($actor, $params);
 
+        $tagsDocument = $this->getTagsDocument($actor, $slug);
+
+        $apiDocument->included[] = $tagsDocument->data;
+        foreach ((array) $tagsDocument->included as $tag) {
+            $apiDocument->included[] = $tag;
+        }
+
         $document->title = $tag->name;
         if ($tag->description) {
             $document->meta['description'] = $tag->description;
@@ -119,4 +127,12 @@ class Tag
     {
         return json_decode($this->api->send(ListDiscussionsController::class, $actor, $params)->getBody());
     }
+
+    private function getTagsDocument(User $actor, string $slug)
+    {
+        return json_decode($this->api->send(ShowTagController::class, $actor, [
+            'slug' => $slug,
+            'include' => 'children,children.parent,parent,parent.children.parent,state'
+        ])->getBody());
+    }
 }

extensions/tags/src/Content/Tags.php

@@ -13,11 +13,13 @@ use Flarum\Api\Client;
 use Flarum\Frontend\Document;
 use Flarum\Http\UrlGenerator;
 use Flarum\Settings\SettingsRepositoryInterface;
+use Flarum\Tags\Api\Controller\ListTagsController;
 use Flarum\Tags\TagRepository;
-use Illuminate\Contracts\Translation\Translator;
+use Flarum\User\User;
 use Illuminate\Contracts\View\Factory;
 use Illuminate\Support\Arr;
 use Psr\Http\Message\ServerRequestInterface as Request;
+use Symfony\Contracts\Translation\TranslatorInterface;
 
 class Tags
 {
@@ -37,7 +39,7 @@ class Tags
     protected $tags;
 
     /**
-     * @var Translator
+     * @var TranslatorInterface
      */
     protected $translator;
 
@@ -55,7 +57,7 @@ class Tags
      * @param Client $api
      * @param Factory $view
      * @param TagRepository $tags
-     * @param Translator $translator
+     * @param TranslatorInterface $translator
      * @param SettingsRepositoryInterface $settings
      * @param UrlGenerator $url
      */
@@ -63,7 +65,7 @@ class Tags
         Client $api,
         Factory $view,
         TagRepository $tags,
-        Translator $translator,
+        TranslatorInterface $translator,
         SettingsRepositoryInterface $settings,
         UrlGenerator $url
     ) {
@@ -77,25 +79,33 @@ class Tags
 
     public function __invoke(Document $document, Request $request)
     {
-        $tags = collect($document->payload['resources'])->where('type', 'tags');
+        $apiDocument = $this->getTagsDocument($request->getAttribute('actor'));
+        $tags = collect(Arr::get($apiDocument, 'data', []));
+
         $childTags = $tags->where('attributes.isChild', true);
         $primaryTags = $tags->where('attributes.isChild', false)->where('attributes.position', '!==', null)->sortBy('attributes.position');
         $secondaryTags = $tags->where('attributes.isChild', false)->where('attributes.position', '===', null)->sortBy('attributes.name');
-        $defaultRoute = $this->settings->get('default_route');
 
         $children = $primaryTags->mapWithKeys(function ($tag) use ($childTags) {
-            $id = Arr::get($tag, 'id');
+            $childIds = collect(Arr::get($tag, 'relationships.children.data'))->pluck('id');
 
-            return [
-                $id => $childTags->where('relationships.parent.data.id', $id)->pluck('attributes')->sortBy('position')
-            ];
+            return [$tag['id'] => $childTags->whereIn('id', $childIds)->sortBy('position')];
         });
 
+        $defaultRoute = $this->settings->get('default_route');
         $document->title = $this->translator->trans('flarum-tags.forum.all_tags.meta_title_text');
         $document->meta['description'] = $this->translator->trans('flarum-tags.forum.all_tags.meta_description_text');
         $document->content = $this->view->make('tags::frontend.content.tags', compact('primaryTags', 'secondaryTags', 'children'));
         $document->canonicalUrl = $this->url->to('forum')->base().($defaultRoute === '/tags' ? '' : $request->getUri()->getPath());
+        $document->payload['apiDocument'] = $apiDocument;
 
         return $document;
     }
+
+    private function getTagsDocument(User $actor)
+    {
+        return json_decode($this->api->send(ListTagsController::class, $actor, [
+            'include' => 'children,lastPostedDiscussion'
+        ])->getBody(), true);
+    }
 }

extensions/tags/src/LoadForumTagsRelationship.php

@@ -28,14 +28,20 @@ class LoadForumTagsRelationship
         // relationship to the /api endpoint. Since the Forum model
         // doesn't actually have a tags relationship, we will manually load and
         // assign the tags data to it using an event listener.
-        $data['tags'] = Tag::whereVisibleTo($actor)
-            ->withStateFor($actor)
-            ->with([
-                'parent',
-                'lastPostedDiscussion',
-                'lastPostedDiscussion.tags',
-                'lastPostedDiscussion.state'
-            ])
+        $data['tags'] = Tag::query()
+            ->where(function ($query) {
+                $query
+                    ->whereNull('parent_id')
+                    ->whereNotNull('position');
+            })
+            ->union(
+                Tag::whereVisibleTo($actor)
+                    ->whereNull('parent_id')
+                    ->whereNull('position')
+                    ->orderBy('discussion_count', 'desc')
+                    ->limit(4) // We get one more than we need so the "more" link can be shown.
+            )
+            ->whereVisibleTo($actor)
             ->get();
     }
 }

extensions/tags/src/Tag.php

@@ -94,6 +94,11 @@ class Tag extends AbstractModel
         return $this->belongsTo(self::class);
     }
 
+    public function children()
+    {
+        return $this->hasMany(self::class, 'parent_id');
+    }
+
     public function lastPostedDiscussion()
     {
         return $this->belongsTo(Discussion::class, 'last_posted_discussion_id');

extensions/tags/views/frontend/content/tags.blade.php

@@ -18,9 +18,9 @@
                             @foreach ($children->get($id) as $child)
                                 <li>
                                     <a href="{{ $url->to('forum')->route('tag', [
-                                                'slug' => $child['slug']
+                                                'slug' => $child['attributes']['slug']
                                             ]) }}">
-                                        {{ $child['name'] }}
+                                        {{ $child['attributes']['name'] }}
                                     </a>
                                 </li>
                             @endforeach