From 56edd5ec288d3310209d521b7bdfe7c2639ac892 Mon Sep 17 00:00:00 2001 From: Peter Deltchev Date: Sun, 17 Jan 2016 02:33:58 -0800 Subject: [PATCH] #1: Search relevancy improvements and other tweaks. --- app/Library/Search.php | 16 +++--- app/Models/Album.php | 7 +++ app/Models/Playlist.php | 22 +++++++++ app/Models/Track.php | 15 +++++- app/Models/User.php | 7 +++ app/Traits/IndexedInElasticsearchTrait.php | 23 ++++++--- .../2016_01_14_021607_setup_elasticsearch.php | 24 ++++----- public/templates/directives/search.html | 28 ++++++----- .../scripts/app/directives/search.coffee | 7 ++- resources/assets/styles/content.less | 49 +++++++++++++------ 10 files changed, 143 insertions(+), 55 deletions(-) diff --git a/app/Library/Search.php b/app/Library/Search.php index 6e31a1ab..caa07bf9 100644 --- a/app/Library/Search.php +++ b/app/Library/Search.php @@ -43,7 +43,7 @@ class Search { * @param int $resultsPerContentType * @return array */ - public function searchAllContent(string $query, int $resultsPerContentType = 10) { + public function searchAllContent(string $query) { $results = $this->elasticsearch->msearch([ 'index' => $this->index, 'body' => [ @@ -60,9 +60,10 @@ class Search { 'track_type', 'show_songs^2', ], + 'tie_breaker' => 0.3, ], ], - 'size' => $resultsPerContentType + 'size' => 11 ], //===== Albums =====// @@ -76,9 +77,10 @@ class Search { 'artist', 'tracks', ], + 'tie_breaker' => 0.3, ], ], - 'size' => $resultsPerContentType + 'size' => 3 ], //===== Playlists =====// @@ -92,9 +94,10 @@ class Search { 'curator', 'tracks^2', ], + 'tie_breaker' => 0.3, ], ], - 'size' => $resultsPerContentType + 'size' => 3 ], //===== Users =====// @@ -104,12 +107,13 @@ class Search { 'multi_match' => [ 'query' => $query, 'fields' => [ - 'display_name^2', + 'display_name', 'tracks', ], + 'tie_breaker' => 0.3, ], ], - 'size' => $resultsPerContentType + 'size' => 3 ], ] ]); diff --git a/app/Models/Album.php b/app/Models/Album.php index f32c69a8..38ff9588 100644 --- a/app/Models/Album.php +++ b/app/Models/Album.php @@ -420,4 +420,11 @@ class Album extends Model 'tracks' => $this->tracks->pluck('title'), ]; } + + /** + * @inheritdoc + */ + public function shouldBeIndexed():bool { + return $this->track_count > 0 && !$this->trashed(); + } } diff --git a/app/Models/Playlist.php b/app/Models/Playlist.php index 327dd67e..8a12d3fd 100644 --- a/app/Models/Playlist.php +++ b/app/Models/Playlist.php @@ -67,6 +67,19 @@ class Playlist extends Model protected $table = 'playlists'; protected $dates = ['deleted_at']; + protected $casts = [ + 'id' => 'integer', + 'user_id' => 'integer', + 'title' => 'string', + 'description' => 'string', + 'is_public' => 'boolean', + 'track_count' => 'integer', + 'view_count' => 'integer', + 'download_count' => 'integer', + 'favourte_count' => 'integer', + 'follow_count' => 'integer', + 'comment_count' => 'integer', + ]; public static function summary() { @@ -301,4 +314,13 @@ class Playlist extends Model 'tracks' => $this->tracks->pluck('title'), ]; } + + /** + * @inheritdoc + */ + public function shouldBeIndexed():bool { + return $this->is_public && + $this->track_count > 0 && + !$this->trashed(); + } } diff --git a/app/Models/Track.php b/app/Models/Track.php index 6dbd1af2..f3cbaf66 100644 --- a/app/Models/Track.php +++ b/app/Models/Track.php @@ -831,9 +831,20 @@ class Track extends Model return 'track-' . $this->id . '-' . $key; } - //============= Elasticsearch stuff ==================// - public function toElasticsearch() { + /** + * @inheritdoc + */ + public function shouldBeIndexed():bool { + return $this->is_listed && + $this->published_at !== null && + !$this->trashed(); + } + + /** + * @inheritdoc + */ + public function toElasticsearch():array { return [ 'title' => $this->title, 'artist' => $this->user->display_name, diff --git a/app/Models/User.php b/app/Models/User.php index 0a656b57..63371a66 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -279,4 +279,11 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon 'tracks' => $this->tracks->pluck('title'), ]; } + + /** + * @inheritdoc + */ + public function shouldBeIndexed():bool { + return $this->disabled_at === null; + } } diff --git a/app/Traits/IndexedInElasticsearchTrait.php b/app/Traits/IndexedInElasticsearchTrait.php index de0c0ac6..92297a9a 100644 --- a/app/Traits/IndexedInElasticsearchTrait.php +++ b/app/Traits/IndexedInElasticsearchTrait.php @@ -21,6 +21,7 @@ namespace Poniverse\Ponyfm\Traits; use Elasticsearch; +use Elasticsearch\Common\Exceptions\Missing404Exception; /** * Class IndexedInElasticsearch @@ -40,13 +41,19 @@ trait IndexedInElasticsearchTrait */ abstract public function toElasticsearch(); - public static function bootIndexedInElasticsearch() { + /** + * @return bool whether this particular object should be indexed or not + */ + abstract public function shouldBeIndexed():bool; + + + public static function bootIndexedInElasticsearchTrait() { static::saved(function ($model) { - $model->createOrUpdateElasticsearchEntry(); + $model->ensureElasticsearchEntryIsUpToDate(); }); static::deleted(function ($model) { - $model->deleteElasticsearchEntry(); + $model->ensureElasticsearchEntryIsUpToDate(); }); } @@ -76,17 +83,17 @@ trait IndexedInElasticsearchTrait try { Elasticsearch::connection()->delete($this->getElasticsearchParameters(false)); - } catch (\Elasticsearch\Common\Exceptions\Missing404Exception $e) { - // If the track we're trying to delete isn't indexed in Elasticsearch, + } catch (Missing404Exception $e) { + // If the entity we're trying to delete isn't indexed in Elasticsearch, // that's fine. } } public function ensureElasticsearchEntryIsUpToDate() { - if (method_exists($this, 'trashed') && $this->trashed()) { - $this->deleteElasticsearchEntry(); - } else { + if ($this->shouldBeIndexed()) { $this->createOrUpdateElasticsearchEntry(); + } else { + $this->deleteElasticsearchEntry(); } } } diff --git a/database/migrations/2016_01_14_021607_setup_elasticsearch.php b/database/migrations/2016_01_14_021607_setup_elasticsearch.php index 2c697187..33e48dfc 100644 --- a/database/migrations/2016_01_14_021607_setup_elasticsearch.php +++ b/database/migrations/2016_01_14_021607_setup_elasticsearch.php @@ -40,17 +40,17 @@ class SetupElasticsearch extends Migration '_source' => ['enabled' => true], 'dynamic' => 'strict', 'properties' => [ - 'title' => ['type' => 'string'], - 'artist' => ['type' => 'string', 'index' => 'not_analyzed'], + 'title' => ['type' => 'string', 'analyzer' => 'english'], + 'artist' => ['type' => 'string'], 'published_at' => ['type' => 'date'], - 'genre' => ['type' => 'string', 'index' => 'not_analyzed'], + 'genre' => ['type' => 'string', 'analyzer' => 'english'], 'track_type' => ['type' => 'string', 'index' => 'not_analyzed'], // This field is intended to be used as an array. // Note that all Elasticsearch fields can technically be used as arrays. // See: https://www.elastic.co/guide/en/elasticsearch/reference/current/array.html - 'show_songs' => ['type' => 'string', 'index' => 'not_analyzed'], + 'show_songs' => ['type' => 'string'], ] ], @@ -58,13 +58,13 @@ class SetupElasticsearch extends Migration '_source' => ['enabled' => true], 'dynamic' => 'strict', 'properties' => [ - 'title' => ['type' => 'string'], - 'artist' => ['type' => 'string', 'index' => 'not_analyzed'], + 'title' => ['type' => 'string', 'analyzer' => 'english'], + 'artist' => ['type' => 'string'], // This field is intended to be used as an array. // Note that all Elasticsearch fields can technically be used as arrays. // See: https://www.elastic.co/guide/en/elasticsearch/reference/current/array.html - 'tracks' => ['type' => 'string'] + 'tracks' => ['type' => 'string', 'analyzer' => 'english'] ] ], @@ -72,13 +72,13 @@ class SetupElasticsearch extends Migration '_source' => ['enabled' => true], 'dynamic' => 'strict', 'properties' => [ - 'title' => ['type' => 'string'], - 'curator' => ['type' => 'string', 'index' => 'not_analyzed'], + 'title' => ['type' => 'string', 'analyzer' => 'english'], + 'curator' => ['type' => 'string'], // This field is intended to be used as an array. // Note that all Elasticsearch fields can technically be used as arrays. // See: https://www.elastic.co/guide/en/elasticsearch/reference/current/array.html - 'tracks' => ['type' => 'string'] + 'tracks' => ['type' => 'string', 'analyzer' => 'english'] ] ], @@ -87,12 +87,12 @@ class SetupElasticsearch extends Migration 'dynamic' => 'strict', 'properties' => [ 'username' => ['type' => 'string', 'index' => 'not_analyzed'], - 'display_name' => ['type' => 'string', 'index' => 'not_analyzed'], + 'display_name' => ['type' => 'string'], // This field is intended to be used as an array. // Note that all Elasticsearch fields can technically be used as arrays. // See: https://www.elastic.co/guide/en/elasticsearch/reference/current/array.html - 'tracks' => ['type' => 'string'] + 'tracks' => ['type' => 'string', 'analyzer' => 'english'] ] ], ] diff --git a/public/templates/directives/search.html b/public/templates/directives/search.html index 36e9a740..079f023a 100644 --- a/public/templates/directives/search.html +++ b/public/templates/directives/search.html @@ -8,21 +8,27 @@ />
-
-

Matching tracks

- - +
+

Type something to begin searching!

-
-

Matching users

- +
+
+

Matching tracks

+ -

Matching albums

- +
-

Matching playlists

- +
+

Matching users

+ + +

Matching albums

+ + +

Matching playlists

+ +
diff --git a/resources/assets/scripts/app/directives/search.coffee b/resources/assets/scripts/app/directives/search.coffee index 53d5abde..673123d0 100644 --- a/resources/assets/scripts/app/directives/search.coffee +++ b/resources/assets/scripts/app/directives/search.coffee @@ -41,13 +41,16 @@ angular.module('ponyfm').directive 'pfmSearch', () -> $scope.$watch 'searchQuery', _.debounce((searchQuery)-> $scope.$apply ()-> - clearResults() - return if searchQuery.length <3 + if searchQuery.length <3 + clearResults() + $scope.searchInProgress = false + return $scope.searchInProgress = true search.searchAllContent(searchQuery) .then (results)-> + clearResults() for track in results.tracks $scope.tracks.push(track) diff --git a/resources/assets/styles/content.less b/resources/assets/styles/content.less index bc9cf0d9..37ab8e4f 100644 --- a/resources/assets/styles/content.less +++ b/resources/assets/styles/content.less @@ -345,20 +345,7 @@ html .single-player .play-button { html { li { &.empty { - .border-radius(0px); - background: lighten(@pfm-purple, 30%); - border: 1px solid lighten(@pfm-purple, 10%); - color: lighten(@pfm-purple, 3%); - float: none !important; - width: auto !important; - display: block; - margin-top: 5px; - padding: 5px; - font-size: 9pt; - - &:hover { - background-color: lighten(@pfm-purple, 30%); - } + .empty-box; } .cache-loading { @@ -374,6 +361,23 @@ html { } } +.empty-box { + .border-radius(0px); + background: lighten(@pfm-purple, 30%); + border: 1px solid lighten(@pfm-purple, 10%); + color: lighten(@pfm-purple, 3%); + float: none !important; + width: auto !important; + display: block; + margin-top: 5px; + padding: 5px; + font-size: 9pt; + + &:hover { + background-color: lighten(@pfm-purple, 30%); + } +} + .tracks-listing { margin: 0px; padding: 0px; @@ -487,3 +491,20 @@ html { } } } + +.users-listing { + &.-condensed { + .image { + height: 30px; + width: 30px; + } + + .info { + margin-left: 40px; + } + + .published { + display: none; + } + } +}