Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial tag load performance improvement #87

Merged
merged 22 commits into from
May 4, 2021
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion extend.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
->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),

Expand Down Expand Up @@ -80,7 +81,7 @@
->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())
Expand Down
24 changes: 19 additions & 5 deletions js/src/admin/components/TagsPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -40,24 +41,38 @@ 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">
<div className="container" key={this.forcedRefreshKey} oncreate={this.onListOnCreate.bind(this)}><div className="SettingsGroups">
<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(
Expand All @@ -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>
Expand Down
1 change: 1 addition & 0 deletions js/src/common/models/Tag.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
8 changes: 7 additions & 1 deletion js/src/forum/addTagComposer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand Down
26 changes: 25 additions & 1 deletion js/src/forum/addTagFilter.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,33 @@ import TagHero from './components/TagHero';

export default function() {
IndexPage.prototype.currentTag = function() {
if (this.currentActiveTag) {
return this.currentActiveTag;
}

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);

if (slug) return 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.
Expand Down
32 changes: 22 additions & 10 deletions js/src/forum/components/TagDiscussionModal.js
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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');
Expand All @@ -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() {
Expand Down Expand Up @@ -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();
Expand Down
28 changes: 25 additions & 3 deletions js/src/forum/components/TagsPage.js
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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);

Expand All @@ -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' : '')}
Expand Down
4 changes: 4 additions & 0 deletions js/src/forum/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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');

Expand Down
20 changes: 20 additions & 0 deletions js/src/forum/states/TagListState.js
Original file line number Diff line number Diff line change
@@ -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' })
SychO9 marked this conversation as resolved.
Show resolved Hide resolved
.then(val => {
unloadedIncludes.forEach(include => this.loadedIncludes.add(include));
return val;
});
}
}
4 changes: 3 additions & 1 deletion src/Api/Controller/ListTagsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,16 @@ class ListTagsController extends AbstractListController
* {@inheritdoc}
*/
public $include = [
'parent',
'parent'
];

/**
* {@inheritdoc}
*/
public $optionalInclude = [
'children',
'lastPostedDiscussion',
'state'
askvortsov1 marked this conversation as resolved.
Show resolved Hide resolved
];

/**
Expand Down
58 changes: 58 additions & 0 deletions src/Api/Controller/ShowTagController.php
Original file line number Diff line number Diff line change
@@ -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();
}
}
5 changes: 5 additions & 0 deletions src/Api/Serializer/TagSerializer.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ protected function parent($tag)
return $this->hasOne($tag, self::class);
}

protected function children($tag)
{
return $this->hasMany($tag, self::class);
}

/**
* @return \Tobscure\JsonApi\Relationship
*/
Expand Down
Loading