From 7b40de5ac47e45a7c63a7976a93ab16c9de5932a Mon Sep 17 00:00:00 2001 From: Jakub Macina Date: Thu, 20 Jul 2017 18:07:13 +0200 Subject: [PATCH 1/6] Add attribute to grouped search results for more available posts. --- app/serializers/grouped_search_result_serializer.rb | 2 +- lib/search/grouped_search_results.rb | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/serializers/grouped_search_result_serializer.rb b/app/serializers/grouped_search_result_serializer.rb index e3cfbe855f0..bc4a5a6a7e3 100644 --- a/app/serializers/grouped_search_result_serializer.rb +++ b/app/serializers/grouped_search_result_serializer.rb @@ -2,7 +2,7 @@ class GroupedSearchResultSerializer < ApplicationSerializer has_many :posts, serializer: SearchPostSerializer has_many :users, serializer: SearchResultUserSerializer has_many :categories, serializer: BasicCategorySerializer - attributes :more_posts, :more_users, :more_categories, :term, :search_log_id + attributes :more_posts, :more_users, :more_categories, :term, :search_log_id, :more_full_page_results def search_log_id object.search_log_id diff --git a/lib/search/grouped_search_results.rb b/lib/search/grouped_search_results.rb index d6677155c28..6f9295c8226 100644 --- a/lib/search/grouped_search_results.rb +++ b/lib/search/grouped_search_results.rb @@ -19,7 +19,8 @@ class Search :more_users, :term, :search_context, - :include_blurbs + :include_blurbs, + :more_full_page_results ) attr_accessor :search_log_id @@ -50,7 +51,9 @@ class Search def add(object) type = object.class.to_s.downcase.pluralize - if !@type_filter.present? && send(type).length == Search.per_facet + if @type_filter.present? && send(type).length == Search.per_filter + @more_full_page_results = true + elsif !@type_filter.present? && send(type).length == Search.per_facet instance_variable_set("@more_#{type}".to_sym, true) else (send type) << object From e5ee4ccc489243a398b3196ea4d8df27b2891644 Mon Sep 17 00:00:00 2001 From: Jakub Macina Date: Thu, 20 Jul 2017 18:12:34 +0200 Subject: [PATCH 2/6] Add pagination and checking for more results to search. --- app/controllers/search_controller.rb | 5 +++- lib/search.rb | 36 ++++++++++++++++++---------- 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 1c29e71fa55..f5734a08d1c 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -13,7 +13,10 @@ class SearchController < ApplicationController type_filter: 'topic', guardian: guardian, include_blurbs: true, - blurb_length: 300 + blurb_length: 300, + page: if params[:page].to_i <= 10 + [params[:page].to_i, 1].max + end } context, type = lookup_search_context diff --git a/lib/search.rb b/lib/search.rb index 2f2dfd64207..0c06e9fa20c 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -159,8 +159,8 @@ class Search @search_context = @opts[:search_context] @include_blurbs = @opts[:include_blurbs] || false @blurb_length = @opts[:blurb_length] - @limit = Search.per_facet @valid = true + @page = @opts[:page] # Removes any zero-width characters from search terms term.to_s.gsub!(/[\u200B-\u200D\uFEFF]/, '') @@ -178,10 +178,6 @@ class Search @search_context = @guardian.user end - if @opts[:type_filter].present? - @limit = Search.per_filter - end - @results = GroupedSearchResults.new( @opts[:type_filter], clean_term, @@ -191,6 +187,22 @@ class Search ) end + def limit + if @opts[:type_filter].present? + Search.per_filter + 1 + else + Search.per_facet + 1 + end + end + + def offset + if @page && @opts[:type_filter].present? + (@page - 1) * Search.per_filter + else + 0 + end + end + def valid? @valid end @@ -547,7 +559,6 @@ class Search raise Discourse::InvalidAccess.new("invalid type filter") unless Search.facets.include?(@results.type_filter) send("#{@results.type_filter}_search") else - @limit = Search.per_facet + 1 unless @search_context user_search if @term.present? category_search if @term.present? @@ -605,7 +616,7 @@ class Search .references(:category_search_data) .order("topics_month DESC") .secured(@guardian) - .limit(@limit) + .limit(limit) categories.each do |category| @results.add(category) @@ -622,7 +633,7 @@ class Search .where("user_search_data.search_data @@ #{ts_query("simple")}") .order("CASE WHEN username_lower = '#{@original_term.downcase}' THEN 0 ELSE 1 END") .order("last_posted_at DESC") - .limit(@limit) + .limit(limit) users.each do |user| @results.add(user) @@ -744,6 +755,7 @@ class Search posts = posts.where("(categories.id IS NULL) OR (NOT categories.read_restricted)").references(:categories) end + posts = posts.offset(offset) posts.limit(limit) end @@ -787,10 +799,10 @@ class Search query = if @order == :likes # likes are a pain to aggregate so skip - posts_query(@limit, private_messages: opts[:private_messages]) + posts_query(limit, private_messages: opts[:private_messages]) .select('topics.id', "posts.post_number") else - posts_query(@limit, aggregate_search: true, private_messages: opts[:private_messages]) + posts_query(limit, aggregate_search: true, private_messages: opts[:private_messages]) .select('topics.id', "#{min_or_max}(posts.post_number) post_number") .group('topics.id') end @@ -825,7 +837,7 @@ class Search added += 1 end - if added < @limit + if added < limit aggregate_posts(post_sql[:remaining]).each {|p| @results.add(p) } end end @@ -838,7 +850,7 @@ class Search def topic_search if @search_context.is_a?(Topic) - posts = posts_eager_loads(posts_query(@limit)) + posts = posts_eager_loads(posts_query(limit)) .where('posts.topic_id = ?', @search_context.id) posts.each do |post| From 44bd8f3837ce42a7f058adb91c162aa353fc0195 Mon Sep 17 00:00:00 2001 From: Jakub Macina Date: Fri, 21 Jul 2017 09:35:04 +0200 Subject: [PATCH 3/6] Add info about more results to full-page search frontend. --- .../discourse/templates/full-page-search.hbs | 10 +++++++--- config/locales/client.en.yml | 1 + 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/discourse/templates/full-page-search.hbs b/app/assets/javascripts/discourse/templates/full-page-search.hbs index e3962efe12f..c1f0e6fa0ea 100644 --- a/app/assets/javascripts/discourse/templates/full-page-search.hbs +++ b/app/assets/javascripts/discourse/templates/full-page-search.hbs @@ -125,9 +125,13 @@ {{/each}} {{#if hasResults}} -

- {{i18n "search.no_more_results"}} -

+

+ {{#if model.grouped_search_result.more_full_page_results}} + {{i18n "search.more_results"}} + {{else}} + {{i18n "search.no_more_results"}} + {{/if}} +

{{/if}} {{/conditional-loading-spinner}} diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 6c80ba69c20..b196405329f 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1321,6 +1321,7 @@ en: searching: "Searching ..." post_format: "#{{post_number}} by {{username}}" results_page: "Search Results" + more_results: "There are more results. Please narrow your search criteria." context: user: "Search posts by @{{username}}" From 4eb7f7cd1061437257033a0e9df19371817a6995 Mon Sep 17 00:00:00 2001 From: Jakub Macina Date: Fri, 21 Jul 2017 10:43:02 +0200 Subject: [PATCH 4/6] Add rspec tests for search pagination. --- spec/components/search_spec.rb | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/spec/components/search_spec.rb b/spec/components/search_spec.rb index a17c6e8f609..395255274e8 100644 --- a/spec/components/search_spec.rb +++ b/spec/components/search_spec.rb @@ -802,4 +802,32 @@ describe Search do end end + context 'pagination' do + let(:number_of_results) { 2 } + before do + 5.times { Fabricate(:post) } + Search.stubs(:per_filter).returns(number_of_results) + end + + it 'returns more results flag' do + results = Search.execute('hello', type_filter: 'topic') + results2 = Search.execute('hello', type_filter: 'topic', page: 2) + + expect(results.posts.length).to eq(number_of_results) + expect(results.more_full_page_results).to eq(true) + expect(results2.posts.length).to eq(number_of_results) + expect(results2.more_full_page_results).to eq(true) + end + + it 'correctly search with page parameter' do + search = Search.new('hello', type_filter: 'topic', page: 3) + results = search.execute + + expect(search.offset).to eq(2 * number_of_results) + expect(results.posts.length).to eq(1) + expect(results.more_full_page_results).to eq(nil) + end + + end + end From 2d45b3fc6d5d33b1bf92cf2e8a087d6a064d9fee Mon Sep 17 00:00:00 2001 From: Jakub Macina Date: Tue, 25 Jul 2017 15:33:25 +0200 Subject: [PATCH 5/6] Add infinite loading to full page search. --- .../controllers/full-page-search.js.es6 | 53 ++++++++++---- .../discourse/templates/full-page-search.hbs | 72 ++++++++++--------- 2 files changed, 80 insertions(+), 45 deletions(-) diff --git a/app/assets/javascripts/discourse/controllers/full-page-search.js.es6 b/app/assets/javascripts/discourse/controllers/full-page-search.js.es6 index d85760f6325..3d77fcbc2dd 100644 --- a/app/assets/javascripts/discourse/controllers/full-page-search.js.es6 +++ b/app/assets/javascripts/discourse/controllers/full-page-search.js.es6 @@ -14,12 +14,13 @@ const SortOrders = [ {name: I18n.t('search.latest_topic'), id: 4, term: 'order:latest_topic'}, ]; +const PAGE_LIMIT = 10; export default Ember.Controller.extend({ application: Ember.inject.controller(), bulkSelectEnabled: null, - loading: Em.computed.not("model"), + loading: false, queryParams: ["q", "expanded", "context_id", "context", "skip_context"], q: null, selected: [], @@ -30,11 +31,8 @@ export default Ember.Controller.extend({ sortOrder: 0, sortOrders: SortOrders, invalidSearch: false, - - @computed('model.posts') - resultCount(posts) { - return posts && posts.length; - }, + page: 1, + resultCount: null, @computed('resultCount') hasResults(resultCount) { @@ -113,6 +111,7 @@ export default Ember.Controller.extend({ @observes('sortOrder') triggerSearch() { if (this._searchOnSortChange) { + this.set("page", 1); this._search(); } }, @@ -143,6 +142,11 @@ export default Ember.Controller.extend({ this.set("application.showFooter", !this.get("loading")); }, + @observes('model.posts.length') + resultCountChanged() { + this.set("resultCount", this.get("model.posts.length")); + }, + @computed('hasResults') canBulkSelect(hasResults) { return this.currentUser && this.currentUser.staff && hasResults; @@ -158,6 +162,11 @@ export default Ember.Controller.extend({ return iconHTML(expanded ? "caret-down" : "caret-right"); }, + @computed('page') + isLastPage(page) { + return page == PAGE_LIMIT; + }, + _search() { if (this.get("searching")) { return; } @@ -169,10 +178,11 @@ export default Ember.Controller.extend({ } this.set("searching", true); + this.set("loading", true); this.set('bulkSelectEnabled', false); this.get('selected').clear(); - var args = { q: searchTerm }; + var args = { q: searchTerm, page: this.get('page') }; const sortOrder = this.get("sortOrder"); if (sortOrder && SortOrders[sortOrder].term) { @@ -180,7 +190,6 @@ export default Ember.Controller.extend({ } this.set("q", args.q); - this.set("model", null); const skip = this.get("skip_context"); if ((!skip && this.get('context')) || skip==="false"){ @@ -199,9 +208,20 @@ export default Ember.Controller.extend({ this.set('q', results.grouped_search_result.term); } - setTransient('lastSearch', { searchKey, model }, 5); - this.set("model", model); - }).finally(() => this.set("searching", false)); + if(args.page > 1){ + if (model){ + this.get("model").posts.pushObjects(model.posts); + this.get("model").topics.pushObjects(model.topics); + this.get("model").set('grouped_search_result', results.grouped_search_result); + } + }else{ + setTransient('lastSearch', { searchKey, model }, 5); + this.set("model", model); + } + }).finally(() => { + this.set("searching", false); + this.set("loading", false); + }); }, actions: { @@ -227,11 +247,20 @@ export default Ember.Controller.extend({ }, search() { + this.set("page", 1); this._search(); }, toggleAdvancedSearch() { this.toggleProperty('expanded'); - } + }, + + loadMore() { + var page = this.get('page'); + if (this.get('model.grouped_search_result.more_full_page_results') && !this.get("loading") && page < PAGE_LIMIT){ + this.incrementProperty("page"); + this._search(); + } + }, } }); diff --git a/app/assets/javascripts/discourse/templates/full-page-search.hbs b/app/assets/javascripts/discourse/templates/full-page-search.hbs index c1f0e6fa0ea..e823110ace6 100644 --- a/app/assets/javascripts/discourse/templates/full-page-search.hbs +++ b/app/assets/javascripts/discourse/templates/full-page-search.hbs @@ -45,32 +45,23 @@ {{/if}} - {{#conditional-loading-spinner condition=loading}} - - {{#unless hasResults}} -

- {{#if searchActive}} - {{i18n "search.no_results"}} - {{/if}} -

- {{/unless}} - - {{#if hasResults}} -
-
- - {{{i18n "search.result_count" count=resultCount term=noSortQ}}} - -
-
- - {{i18n "search.sort_by"}} - - {{combo-box value=sortOrder content=sortOrders castInteger="true"}} -
+ {{#if hasResults}} +
+
+ + {{{i18n "search.result_count" count=resultCount term=noSortQ}}} +
- {{/if}} +
+ + {{i18n "search.sort_by"}} + + {{combo-box value=sortOrder content=sortOrders castInteger="true"}} +
+
+ {{/if}} + {{#load-more selector=".fps-result" action="loadMore"}} {{#each model.posts as |result|}}
@@ -124,15 +115,30 @@
{{/each}} - {{#if hasResults}} -

- {{#if model.grouped_search_result.more_full_page_results}} - {{i18n "search.more_results"}} - {{else}} - {{i18n "search.no_more_results"}} - {{/if}} + {{#conditional-loading-spinner condition=loading }} + {{#unless hasResults}} +

+ {{#if searchActive}} + {{i18n "search.no_results"}} + {{/if}}

- {{/if}} + {{/unless}} + + {{#if hasResults}} + {{#unless loading}} +

+ {{#if model.grouped_search_result.more_full_page_results}} + {{#if isLastPage }} + {{i18n "search.more_results"}} + {{/if}} + {{else}} + {{i18n "search.no_more_results"}} + {{/if}} +

+ {{/unless}} + {{/if}} + {{/conditional-loading-spinner}} + + {{/load-more}} - {{/conditional-loading-spinner}} {{/d-section}} From f8edf2636cb18324a7f9972ff1a821e598f9b6fb Mon Sep 17 00:00:00 2001 From: Jakub Macina Date: Wed, 26 Jul 2017 12:10:19 +0200 Subject: [PATCH 6/6] Fix rspec tests for search pagination. --- .../discourse/controllers/full-page-search.js.es6 | 2 +- spec/components/search_spec.rb | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/discourse/controllers/full-page-search.js.es6 b/app/assets/javascripts/discourse/controllers/full-page-search.js.es6 index 3d77fcbc2dd..d153a234570 100644 --- a/app/assets/javascripts/discourse/controllers/full-page-search.js.es6 +++ b/app/assets/javascripts/discourse/controllers/full-page-search.js.es6 @@ -164,7 +164,7 @@ export default Ember.Controller.extend({ @computed('page') isLastPage(page) { - return page == PAGE_LIMIT; + return page === PAGE_LIMIT; }, _search() { diff --git a/spec/components/search_spec.rb b/spec/components/search_spec.rb index 395255274e8..7ad0e7f0dfe 100644 --- a/spec/components/search_spec.rb +++ b/spec/components/search_spec.rb @@ -804,8 +804,12 @@ describe Search do context 'pagination' do let(:number_of_results) { 2 } + let!(:post1) { Fabricate(:post, raw: 'hello hello hello hello hello')} + let!(:post2) { Fabricate(:post, raw: 'hello hello hello hello')} + let!(:post3) { Fabricate(:post, raw: 'hello hello hello')} + let!(:post4) { Fabricate(:post, raw: 'hello hello')} + let!(:post5) { Fabricate(:post, raw: 'hello')} before do - 5.times { Fabricate(:post) } Search.stubs(:per_filter).returns(number_of_results) end @@ -814,8 +818,10 @@ describe Search do results2 = Search.execute('hello', type_filter: 'topic', page: 2) expect(results.posts.length).to eq(number_of_results) + expect(results.posts.map(&:id)).to eq([post1.id, post2.id]) expect(results.more_full_page_results).to eq(true) expect(results2.posts.length).to eq(number_of_results) + expect(results2.posts.map(&:id)).to eq([post3.id, post4.id]) expect(results2.more_full_page_results).to eq(true) end @@ -825,6 +831,7 @@ describe Search do expect(search.offset).to eq(2 * number_of_results) expect(results.posts.length).to eq(1) + expect(results.posts).to eq([post5]) expect(results.more_full_page_results).to eq(nil) end