From 42f1fa12721dfb06df9c19dc1d056b3e429e35b3 Mon Sep 17 00:00:00 2001 From: Toby Zerner Date: Tue, 7 Jul 2015 20:35:18 +0930 Subject: [PATCH] Improve fulltext search API and interface --- js/forum/src/components/discussion-list.js | 3 ++ .../components/discussions-search-results.js | 2 +- js/lib/helpers/highlight.js | 2 +- less/forum/index.less | 11 ++++-- .../DiscussionsServiceProvider.php | 2 +- .../Discussions/Search/DiscussionSearch.php | 21 +++--------- .../Discussions/Search/Fulltext/Driver.php | 13 +++++++ .../Search/Fulltext/DriverInterface.php | 6 ---- .../Search/Fulltext/MySqlFulltextDriver.php | 18 ++++++++-- .../Search/Gambits/FulltextGambit.php | 28 +++++++-------- src/Core/Posts/PostRepository.php | 34 +------------------ src/Forum/Actions/IndexAction.php | 3 +- 12 files changed, 62 insertions(+), 81 deletions(-) create mode 100644 src/Core/Discussions/Search/Fulltext/Driver.php delete mode 100644 src/Core/Discussions/Search/Fulltext/DriverInterface.php diff --git a/js/forum/src/components/discussion-list.js b/js/forum/src/components/discussion-list.js index 5921ec2b4..4a3257d77 100644 --- a/js/forum/src/components/discussion-list.js +++ b/js/forum/src/components/discussion-list.js @@ -23,7 +23,10 @@ export default class DiscussionList extends Component { } params.sort = this.sortMap()[params.sort]; if (params.q) { + params.filter = params.filter || {}; + params.filter.q = params.q; params.include.push('relevantPosts', 'relevantPosts.discussion', 'relevantPosts.user'); + delete params.q; } return params; } diff --git a/js/forum/src/components/discussions-search-results.js b/js/forum/src/components/discussions-search-results.js index bf698439c..5f89099ee 100644 --- a/js/forum/src/components/discussions-search-results.js +++ b/js/forum/src/components/discussions-search-results.js @@ -9,7 +9,7 @@ export default class DiscussionsSearchResults { search(string) { this.results[string] = []; - return app.store.find('discussions', {filter: {q: string}, page: {limit: 3}, include: 'relevantPosts,relevantPosts.discussion'}).then(results => { + return app.store.find('discussions', {filter: {q: string}, page: {limit: 3}, include: 'relevantPosts,relevantPosts.discussion,relevantPosts.user'}).then(results => { this.results[string] = results; }); } diff --git a/js/lib/helpers/highlight.js b/js/lib/helpers/highlight.js index 75cd5fd20..80a939b5c 100644 --- a/js/lib/helpers/highlight.js +++ b/js/lib/helpers/highlight.js @@ -5,7 +5,7 @@ export default function(string, phrase, length) { return string; } - const regexp = regexp instanceof RegExp ? phrase : new RegExp(phrase, 'gi'); + const regexp = phrase instanceof RegExp ? phrase : new RegExp(phrase, 'gi'); let highlightedString = string; let start = 0; diff --git a/less/forum/index.less b/less/forum/index.less index a0e550a5d..40dd199f8 100644 --- a/less/forum/index.less +++ b/less/forum/index.less @@ -113,7 +113,8 @@ font-size: 14px; } & .relevant-posts { - display: none; + margin-left: -52px; + margin-right: -65px; } & .count { margin-top: 11px; @@ -242,7 +243,7 @@ } } & .relevant-posts { - margin-bottom: 20px; + padding-bottom: 15px; @media @phone { margin-left: -45px; @@ -254,6 +255,12 @@ display: block; padding: 10px 15px; border-bottom: 2px dotted @fl-body-bg; + color: @fl-body-muted-color; + transition: border-color 0.2s; + + .discussion-list-item:hover & { + border-color: lighten(@fl-body-secondary-color, 3%); + } & .avatar, & time { display: none; diff --git a/src/Core/Discussions/DiscussionsServiceProvider.php b/src/Core/Discussions/DiscussionsServiceProvider.php index 53b4cc828..96eb93de0 100644 --- a/src/Core/Discussions/DiscussionsServiceProvider.php +++ b/src/Core/Discussions/DiscussionsServiceProvider.php @@ -45,7 +45,7 @@ class DiscussionsServiceProvider extends ServiceProvider public function register() { $this->app->bind( - 'Flarum\Core\Discussions\Search\Fulltext\DriverInterface', + 'Flarum\Core\Discussions\Search\Fulltext\Driver', 'Flarum\Core\Discussions\Search\Fulltext\MySqlFulltextDriver' ); diff --git a/src/Core/Discussions/Search/DiscussionSearch.php b/src/Core/Discussions/Search/DiscussionSearch.php index 29d8909be..5d9099969 100644 --- a/src/Core/Discussions/Search/DiscussionSearch.php +++ b/src/Core/Discussions/Search/DiscussionSearch.php @@ -30,26 +30,13 @@ class DiscussionSearch extends Search } /** - * Set the relevant post IDs for a result. + * Set the relevant post IDs for the results. * - * @param int $discussionId - * @param int[] $postIds + * @param array $relevantPostIds * @return void */ - public function setRelevantPostIds($discussionId, array $postIds) + public function setRelevantPostIds(array $relevantPostIds) { - $this->relevantPostIds[$discussionId] = $postIds; - } - - /** - * Add a relevant post ID for a discussion result. - * - * @param int $discussionId - * @param int $postId - * @return void - */ - public function addRelevantPostId($discussionId, $postId) - { - $this->relevantPostIds[$discussionId][] = $postId; + $this->relevantPostIds = $relevantPostIds; } } diff --git a/src/Core/Discussions/Search/Fulltext/Driver.php b/src/Core/Discussions/Search/Fulltext/Driver.php new file mode 100644 index 000000000..9bed81b92 --- /dev/null +++ b/src/Core/Discussions/Search/Fulltext/Driver.php @@ -0,0 +1,13 @@ +whereRaw('MATCH (`content`) AGAINST (? IN BOOLEAN MODE)', [$string]) ->orderByRaw('MATCH (`content`) AGAINST (?) DESC', [$string]) - ->lists('id'); + ->lists('discussion_id', 'id'); + + $relevantPostIds = []; + + foreach ($discussionIds as $postId => $discussionId) { + $relevantPostIds[$discussionId][] = $postId; + } + + return $relevantPostIds; } } diff --git a/src/Core/Discussions/Search/Gambits/FulltextGambit.php b/src/Core/Discussions/Search/Gambits/FulltextGambit.php index 088dd2c6a..4af8e7aac 100644 --- a/src/Core/Discussions/Search/Gambits/FulltextGambit.php +++ b/src/Core/Discussions/Search/Gambits/FulltextGambit.php @@ -1,7 +1,7 @@ posts = $posts; + $this->fulltext = $fulltext; } /** @@ -30,18 +30,14 @@ class FulltextGambit implements Gambit throw new LogicException('This gambit can only be applied on a DiscussionSearch'); } - $posts = $this->posts->findByContent($bit, $search->getActor()); + $relevantPostIds = $this->fulltext->match($bit); - $discussions = []; - foreach ($posts as $post) { - $discussions[] = $id = $post->discussion_id; - $search->addRelevantPostId($id, $post->id); - } - $discussions = array_unique($discussions); + $discussionIds = array_keys($relevantPostIds); - // TODO: implement negate (match for - at start of string) - $search->getQuery()->whereIn('id', $discussions); + $search->setRelevantPostIds($relevantPostIds); - $search->setDefaultSort(['id' => $discussions]); + $search->getQuery()->whereIn('id', $discussionIds); + + $search->setDefaultSort(['id' => $discussionIds]); } } diff --git a/src/Core/Posts/PostRepository.php b/src/Core/Posts/PostRepository.php index da929f840..9e48e5495 100644 --- a/src/Core/Posts/PostRepository.php +++ b/src/Core/Posts/PostRepository.php @@ -4,7 +4,7 @@ use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\ModelNotFoundException; use Flarum\Core\Users\User; use Flarum\Core\Discussions\Discussion; -use Flarum\Core\Discussions\Search\Fulltext\DriverInterface; +use Flarum\Core\Discussions\Search\Fulltext\Driver; // TODO: In some cases, the use of a post repository incurs extra query expense, // because for every post retrieved we need to check if the discussion it's in @@ -28,13 +28,6 @@ use Flarum\Core\Discussions\Search\Fulltext\DriverInterface; class PostRepository { - protected $fulltext; - - public function __construct(DriverInterface $fulltext) - { - $this->fulltext = $fulltext; - } - /** * Find a post by ID, optionally making sure it is visible to a certain * user, or throw an exception. @@ -99,31 +92,6 @@ class PostRepository return $this->filterVisibleTo($posts, $actor); } - /** - * Find posts by matching a string of words against their content, - * optionally making sure they are visible to a certain user. - * - * @param string $string - * @param \Flarum\Core\Users\User|null $actor - * @return \Illuminate\Database\Eloquent\Collection - */ - public function findByContent($string, User $actor = null) - { - $ids = $this->fulltext->match($string); - - $ids = $this->filterDiscussionVisibleTo($ids, $actor); - - $query = Post::select('id', 'discussion_id')->whereIn('id', $ids); - - foreach ($ids as $id) { - $query->orderByRaw('id != ?', [$id]); - } - - $posts = $query->get(); - - return $this->filterVisibleTo($posts, $actor); - } - /** * Get the position within a discussion where a post with a certain number * is. If the post with that number does not exist, the index of the diff --git a/src/Forum/Actions/IndexAction.php b/src/Forum/Actions/IndexAction.php index 6e038ab4b..51505ffe0 100644 --- a/src/Forum/Actions/IndexAction.php +++ b/src/Forum/Actions/IndexAction.php @@ -30,7 +30,8 @@ class IndexAction extends ClientAction $params = [ 'sort' => $sort ? $this->sortMap[$sort] : '', - 'q' => $q + 'filter' => ['q' => $q], + 'include' => $q ? 'startUser,lastUser,relevantPosts,relevantPosts.discussion,relevantPosts.user' : '' ]; // FIXME: make sure this is extensible. Support pagination.