From e7226a8c84a267b7151558cbf4e880f92e59587f Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Thu, 14 Nov 2019 10:40:26 +1000 Subject: [PATCH] FEATURE: Allow scoping search to tag (#8345) * When viewing a tag, the search widget will now show a checkbox to scope the search by tag, which will limit search results to that tag on desktop and mobile --- .../javascripts/discourse/lib/search.js.es6 | 5 +- .../javascripts/discourse/models/tag.js.es6 | 5 + .../discourse/routes/tags-show.js.es6 | 6 + .../discourse/templates/full-page-search.hbs | 1 + .../discourse/widgets/header.js.es6 | 2 +- .../widgets/search-menu-controls.js.es6 | 2 +- app/controllers/search_controller.rb | 4 +- config/locales/client.en.yml | 1 + lib/search.rb | 5 + spec/components/search_spec.rb | 19 +++- spec/requests/search_controller_spec.rb | 17 +++ .../javascripts/acceptance/search-test.js.es6 | 8 ++ .../fixtures/discovery_fixtures.js.es6 | 107 ++++++++++++++++++ test/javascripts/lib/search-test.js.es6 | 29 ++++- 14 files changed, 204 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/discourse/lib/search.js.es6 b/app/assets/javascripts/discourse/lib/search.js.es6 index 2824268e66..426a630483 100644 --- a/app/assets/javascripts/discourse/lib/search.js.es6 +++ b/app/assets/javascripts/discourse/lib/search.js.es6 @@ -145,7 +145,8 @@ export function searchForTerm(term, opts) { if (opts.searchContext) { data.search_context = { type: opts.searchContext.type, - id: opts.searchContext.id + id: opts.searchContext.id, + name: opts.searchContext.name }; } @@ -167,6 +168,8 @@ export function searchContextDescription(type, name) { return I18n.t("search.context.user", { username: name }); case "category": return I18n.t("search.context.category", { category: name }); + case "tag": + return I18n.t("search.context.tag", { tag: name }); case "private_messages": return I18n.t("search.context.private_messages"); } diff --git a/app/assets/javascripts/discourse/models/tag.js.es6 b/app/assets/javascripts/discourse/models/tag.js.es6 index ebfa93d93d..0985bb3dfb 100644 --- a/app/assets/javascripts/discourse/models/tag.js.es6 +++ b/app/assets/javascripts/discourse/models/tag.js.es6 @@ -10,5 +10,10 @@ export default RestModel.extend({ @discourseComputed("count", "pm_count") pmOnly(count, pmCount) { return count === 0 && pmCount > 0; + }, + + @discourseComputed("id") + searchContext(id) { + return { type: "tag", id, tag: this, name: id }; } }); diff --git a/app/assets/javascripts/discourse/routes/tags-show.js.es6 b/app/assets/javascripts/discourse/routes/tags-show.js.es6 index 0883b5cc1b..5d8f44aceb 100644 --- a/app/assets/javascripts/discourse/routes/tags-show.js.es6 +++ b/app/assets/javascripts/discourse/routes/tags-show.js.es6 @@ -165,6 +165,12 @@ export default DiscourseRoute.extend({ tagNotification: this.tagNotification, noSubcategories: this.noSubcategories }); + this.searchService.set("searchContext", model.get("searchContext")); + }, + + deactivate() { + this._super(...arguments); + this.searchService.set("searchContext", null); }, actions: { diff --git a/app/assets/javascripts/discourse/templates/full-page-search.hbs b/app/assets/javascripts/discourse/templates/full-page-search.hbs index c2c15bb389..9d5e739cab 100644 --- a/app/assets/javascripts/discourse/templates/full-page-search.hbs +++ b/app/assets/javascripts/discourse/templates/full-page-search.hbs @@ -17,6 +17,7 @@ {{/if}} + {{!-- context is only provided when searching from mobile view --}}
{{#if context}}
diff --git a/app/assets/javascripts/discourse/widgets/header.js.es6 b/app/assets/javascripts/discourse/widgets/header.js.es6 index c86931260b..e08bab8741 100644 --- a/app/assets/javascripts/discourse/widgets/header.js.es6 +++ b/app/assets/javascripts/discourse/widgets/header.js.es6 @@ -267,7 +267,7 @@ createWidget("header-cloak", { scheduleRerender() {} }); -const forceContextEnabled = ["category", "user", "private_messages"]; +const forceContextEnabled = ["category", "user", "private_messages", "tag"]; let additionalPanels = []; export function attachAdditionalPanel(name, toggle, transformAttrs) { diff --git a/app/assets/javascripts/discourse/widgets/search-menu-controls.js.es6 b/app/assets/javascripts/discourse/widgets/search-menu-controls.js.es6 index de2ea2ac42..7492f646e6 100644 --- a/app/assets/javascripts/discourse/widgets/search-menu-controls.js.es6 +++ b/app/assets/javascripts/discourse/widgets/search-menu-controls.js.es6 @@ -54,7 +54,7 @@ createWidget("search-context", { if (ctx) { const description = searchContextDescription( get(ctx, "type"), - get(ctx, "user.username") || get(ctx, "category.name") + get(ctx, "user.username") || get(ctx, "category.name") || get(ctx, "tag.id") ); result.push( h("label", [ diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 4fea08f068..3f0914e2dc 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -7,7 +7,7 @@ class SearchController < ApplicationController before_action :cancel_overloaded_search, only: [:query] def self.valid_context_types - %w{user topic category private_messages} + %w{user topic category private_messages tag} end def show @@ -169,6 +169,8 @@ class SearchController < ApplicationController context_obj = Category.find_by(id: search_context[:id].to_i) elsif 'topic' == search_context[:type] context_obj = Topic.find_by(id: search_context[:id].to_i) + elsif 'tag' == search_context[:type] + context_obj = Tag.where_name(search_context[:name]).first end type_filter = nil diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index d0c7d3322d..5e723f73cb 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1867,6 +1867,7 @@ en: context: user: "Search posts by @{{username}}" category: "Search the #{{category}} category" + tag: "Search the #{{tag}} tag" topic: "Search this topic" private_messages: "Search messages" diff --git a/lib/search.rb b/lib/search.rb index 1f618cd56f..4e163cb23c 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -862,6 +862,11 @@ class Search elsif @search_context.is_a?(Topic) posts.where("topics.id = #{@search_context.id}") .order("posts.post_number #{@order == :latest ? "DESC" : ""}") + elsif @search_context.is_a?(Tag) + posts = posts + .joins("LEFT JOIN topic_tags ON topic_tags.topic_id = topics.id") + .joins("LEFT JOIN tags ON tags.id = topic_tags.tag_id") + posts.where("tags.id = #{@search_context.id}") end else posts = categories_ignored(posts) unless @category_filter_matched diff --git a/spec/components/search_spec.rb b/spec/components/search_spec.rb index 815302501b..122a2e67b8 100644 --- a/spec/components/search_spec.rb +++ b/spec/components/search_spec.rb @@ -784,14 +784,29 @@ describe Search do sub_topic = Fabricate(:topic, category: subcategory) post = Fabricate(:post, topic: topic, user: topic.user) - _another_post = Fabricate(:post, topic: topic_no_cat, user: topic.user) + Fabricate(:post, topic: topic_no_cat, user: topic.user) sub_post = Fabricate(:post, raw: 'I am saying hello from a subcategory', topic: sub_topic, user: topic.user) search = Search.execute('hello', search_context: category) - expect(search.posts.map(&:id).sort).to eq([post.id, sub_post.id].sort) + expect(search.posts.map(&:id)).to match_array([post.id, sub_post.id]) expect(search.posts.length).to eq(2) end + it 'can use tag as a search context' do + tag = Fabricate(:tag, name: 'important-stuff') + + topic = Fabricate(:topic) + topic_no_tag = Fabricate(:topic) + Fabricate(:topic_tag, tag: tag, topic: topic) + + post = Fabricate(:post, topic: topic, user: topic.user, raw: 'This is my hello') + Fabricate(:post, topic: topic_no_tag, user: topic.user) + + search = Search.execute('hello', search_context: tag) + expect(search.posts.map(&:id)).to contain_exactly(post.id) + expect(search.posts.length).to eq(1) + end + end describe 'Chinese search' do diff --git a/spec/requests/search_controller_spec.rb b/spec/requests/search_controller_spec.rb index 72dc29a004..41664a3d17 100644 --- a/spec/requests/search_controller_spec.rb +++ b/spec/requests/search_controller_spec.rb @@ -251,6 +251,23 @@ describe SearchController do end end + context "with a tag" do + it "raises an error if the tag does not exist" do + get "/search/query.json", params: { + term: 'test', search_context: { type: 'tag', id: 'important-tag', name: 'important-tag' } + } + expect(response).to be_forbidden + end + + it 'performs the query with a search context' do + Fabricate(:tag, name: 'important-tag') + get "/search/query.json", params: { + term: 'test', search_context: { type: 'tag', id: 'important-tag', name: 'important-tag' } + } + + expect(response.status).to eq(200) + end + end end context "#click" do diff --git a/test/javascripts/acceptance/search-test.js.es6 b/test/javascripts/acceptance/search-test.js.es6 index a92a7c62c5..2aa186de2b 100644 --- a/test/javascripts/acceptance/search-test.js.es6 +++ b/test/javascripts/acceptance/search-test.js.es6 @@ -50,6 +50,14 @@ QUnit.test("search for a tag", async assert => { }); QUnit.test("search scope checkbox", async assert => { + await visit("/tags/important"); + await click("#search-button"); + assert.ok( + exists(".search-context input:checked"), + "scope to tag checkbox is checked" + ); + await click("#search-button"); + await visit("/c/bug"); await click("#search-button"); assert.ok( diff --git a/test/javascripts/fixtures/discovery_fixtures.js.es6 b/test/javascripts/fixtures/discovery_fixtures.js.es6 index 1de2598980..afcdf8fe46 100644 --- a/test/javascripts/fixtures/discovery_fixtures.js.es6 +++ b/test/javascripts/fixtures/discovery_fixtures.js.es6 @@ -3756,6 +3756,113 @@ export default { ] } }, + "/tags/important/l/latest.json": { + users: [{ id: 1, username: "sam", avatar_template: "/images/avatar.png" }], + primary_groups: [], + topic_list: { + can_create_topic: true, + draft: null, + draft_key: "new_topic", + draft_sequence: 4, + per_page: 30, + tags: [ + { + id: 1, + name: "test", + topic_count: 2, + staff: false + } + ], + topics: [ + { + id: 16, + title: "Dinosaurs are the best", + fancy_title: "Dinosaurs are the best", + slug: "dinosaurs-are-the-best", + posts_count: 1, + reply_count: 0, + highest_post_number: 1, + image_url: null, + created_at: "2019-11-12T05:19:52.300Z", + last_posted_at: "2019-11-12T05:19:52.848Z", + bumped: true, + bumped_at: "2019-11-12T05:19:52.848Z", + unseen: false, + last_read_post_number: 1, + unread: 0, + new_posts: 0, + pinned: false, + unpinned: null, + visible: true, + closed: false, + archived: false, + notification_level: 3, + bookmarked: false, + liked: false, + tags: ["test"], + views: 2, + like_count: 0, + has_summary: false, + archetype: "regular", + last_poster_username: "sam", + category_id: 1, + pinned_globally: false, + featured_link: null, + posters: [ + { + extras: "latest single", + description: "Original Poster, Most Recent Poster", + user_id: 1, + primary_group_id: null + } + ] + }, + { + id: 15, + title: "This is a test tagged post", + fancy_title: "This is a test tagged post", + slug: "this-is-a-test-tagged-post", + posts_count: 1, + reply_count: 0, + highest_post_number: 1, + image_url: null, + created_at: "2019-11-12T05:19:32.032Z", + last_posted_at: "2019-11-12T05:19:32.516Z", + bumped: true, + bumped_at: "2019-11-12T05:19:32.516Z", + unseen: false, + last_read_post_number: 1, + unread: 0, + new_posts: 0, + pinned: false, + unpinned: null, + visible: true, + closed: false, + archived: false, + notification_level: 3, + bookmarked: false, + liked: false, + tags: ["test"], + views: 1, + like_count: 0, + has_summary: false, + archetype: "regular", + last_poster_username: "sam", + category_id: 3, + pinned_globally: false, + featured_link: null, + posters: [ + { + extras: "latest single", + description: "Original Poster, Most Recent Poster", + user_id: 1, + primary_group_id: null + } + ] + } + ] + } + }, "/c/feature/l/latest.json": { users: [ { id: 1, username: "sam", avatar_template: "/images/avatar.png" }, diff --git a/test/javascripts/lib/search-test.js.es6 b/test/javascripts/lib/search-test.js.es6 index 30918007db..6fe1f466c8 100644 --- a/test/javascripts/lib/search-test.js.es6 +++ b/test/javascripts/lib/search-test.js.es6 @@ -1,4 +1,7 @@ -import { translateResults } from "discourse/lib/search"; +import { + translateResults, + searchContextDescription +} from "discourse/lib/search"; QUnit.module("lib:search"); @@ -31,3 +34,27 @@ QUnit.test("unescapesEmojisInBlurbs", assert => { assert.ok(blurb.indexOf(" { + assert.equal( + searchContextDescription("topic"), + I18n.t("search.context.topic") + ); + assert.equal( + searchContextDescription("user", "silvio.dante"), + I18n.t("search.context.user", { username: "silvio.dante" }) + ); + assert.equal( + searchContextDescription("category", "staff"), + I18n.t("search.context.category", { category: "staff" }) + ); + assert.equal( + searchContextDescription("tag", "important"), + I18n.t("search.context.tag", { tag: "important" }) + ); + assert.equal( + searchContextDescription("private_messages"), + I18n.t("search.context.private_messages") + ); + assert.equal(searchContextDescription("bad_type"), null); +});