From 7e73b933c7feca450f89e32d541f346141c844cf Mon Sep 17 00:00:00 2001 From: James Kiesel Date: Thu, 11 Aug 2016 01:38:16 -0400 Subject: [PATCH] First pass --- .../discourse/routes/app-route-map.js.es6 | 1 + .../discourse/routes/tags-show.js.es6 | 6 ++++++ app/controllers/tags_controller.rb | 6 ++++-- config/routes.rb | 1 + lib/topic_query.rb | 19 +++++++++++++++---- spec/components/topic_query_spec.rb | 4 ++++ spec/controllers/tags_controller_spec.rb | 12 ++++++++++++ 7 files changed, 43 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/discourse/routes/app-route-map.js.es6 b/app/assets/javascripts/discourse/routes/app-route-map.js.es6 index 2f26a0248e3..6ce4265544f 100644 --- a/app/assets/javascripts/discourse/routes/app-route-map.js.es6 +++ b/app/assets/javascripts/discourse/routes/app-route-map.js.es6 @@ -132,6 +132,7 @@ export default function() { this.route('showCategory' + filter.capitalize(), {path: '/c/:category/:tag_id/l/' + filter}); this.route('showParentCategory' + filter.capitalize(), {path: '/c/:parent_category/:category/:tag_id/l/' + filter}); }); + this.route('show', {path: 'intersection/:tag_id/:secondary_tag_id'}); }); this.resource('tagGroups', {path: '/tag_groups'}, function() { diff --git a/app/assets/javascripts/discourse/routes/tags-show.js.es6 b/app/assets/javascripts/discourse/routes/tags-show.js.es6 index a828e39c68e..961e28b80a6 100644 --- a/app/assets/javascripts/discourse/routes/tags-show.js.es6 +++ b/app/assets/javascripts/discourse/routes/tags-show.js.es6 @@ -14,6 +14,8 @@ export default Discourse.Route.extend({ var tag = this.store.createRecord("tag", { id: Handlebars.Utils.escapeExpression(params.tag_id) }), f = ''; + this.set("secondaryTagId", params.secondary_tag_id) + if (params.category) { f = 'c/'; if (params.parent_category) { f += params.parent_category + '/'; } @@ -43,6 +45,7 @@ export default Discourse.Route.extend({ const params = controller.getProperties('order', 'ascending'); const categorySlug = this.get('categorySlug'); + const secondaryTagId = this.get('secondaryTagId') const parentCategorySlug = this.get('parentCategorySlug'); const filter = this.get('navMode'); const tag_id = (tag ? tag.id : 'none'); @@ -56,6 +59,9 @@ export default Discourse.Route.extend({ } this.set('category', category); + } else if (secondaryTagId) { + params.filter = `tags/intersection/${tag_id}/${secondaryTagId}`; + this.set('category', null); } else { params.filter = `tags/${tag_id}/l/${filter}`; this.set('category', null); diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index d4290eaebe3..0989f5715ea 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -44,6 +44,7 @@ class TagsController < ::ApplicationController Discourse.filters.each do |filter| define_method("show_#{filter}") do @tag_id = DiscourseTagging.clean_tag(params[:tag_id]) + @secondary_tag_id = DiscourseTagging.clean_tag(params[:secondary_tag_id]) if params[:secondary_tag_id] page = params[:page].to_i list_opts = build_topic_list_options @@ -71,7 +72,7 @@ class TagsController < ::ApplicationController @list.more_topics_url = construct_url_with(:next, list_opts) @list.prev_topics_url = construct_url_with(:prev, list_opts) @rss = "tag" - @description_meta = I18n.t("rss_by_tag", tag: @tag_id) + @description_meta = I18n.t("rss_by_tag", tag: [@tag_id, @secondary_tag_id].compact.join(' & ')) @title = @description_meta canonical_url "#{Discourse.base_url_no_prefix}#{public_send(url_method(params.slice(:category, :parent_category)))}" @@ -297,7 +298,8 @@ class TagsController < ::ApplicationController if params[:tag_id] == 'none' options[:no_tags] = true else - options[:tags] = [params[:tag_id]] + options[:tags] = [params[:tag_id], params[:secondary_tag_id]].compact + options[:match_all_tags] = true end options diff --git a/config/routes.rb b/config/routes.rb index 37a93cd2698..d38ad905f42 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -630,6 +630,7 @@ Discourse::Application.routes.draw do get '/:tag_id' => 'tags#show', as: 'tag_show' get '/c/:category/:tag_id' => 'tags#show', as: 'tag_category_show' get '/c/:parent_category/:category/:tag_id' => 'tags#show', as: 'tag_parent_category_category_show' + get '/intersection/:tag_id/:secondary_tag_id' => 'tags#show', as: 'tag_intersection' get '/:tag_id/notifications' => 'tags#notifications' put '/:tag_id/notifications' => 'tags#update_notifications' put '/:tag_id' => 'tags#update' diff --git a/lib/topic_query.rb b/lib/topic_query.rb index a8e1d77e69a..85cd7cf9fcf 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -20,6 +20,7 @@ class TopicQuery visible category tags + match_all_tags no_tags order ascending @@ -460,11 +461,21 @@ class TopicQuery if @options[:tags] && @options[:tags].size > 0 result = result.joins(:tags) - # ANY of the given tags: - if @options[:tags][0].is_a?(Integer) - result = result.where("tags.id in (?)", @options[:tags]) + + # ALL of the given tags: + if @options[:match_all_tags] + @options[:tags] = Tag.where(name: @options[:tags]).pluck(:id) unless @options[:tags][0].is_a?(Integer) + @options[:tags].each_with_index do |tag, index| + sql_alias = ['t', index].join + result = result.joins("INNER JOIN topic_tags #{sql_alias} ON #{sql_alias}.topic_id = topics.id AND #{sql_alias}.tag_id = #{tag}") + end else - result = result.where("tags.name in (?)", @options[:tags]) + # ANY of the given tags: + if @options[:tags][0].is_a?(Integer) + result = result.where("tags.id in (?)", @options[:tags]) + else + result = result.where("tags.name in (?)", @options[:tags]) + end end elsif @options[:no_tags] # the following will do: ("topics"."id" NOT IN (SELECT DISTINCT "topic_tags"."topic_id" FROM "topic_tags")) diff --git a/spec/components/topic_query_spec.rb b/spec/components/topic_query_spec.rb index 3461a5d2750..32b8d814421 100644 --- a/spec/components/topic_query_spec.rb +++ b/spec/components/topic_query_spec.rb @@ -145,6 +145,10 @@ describe TopicQuery do # expect(TopicQuery.new(moderator, tags: [tag.id, other_tag.id]).list_latest.topics.map(&:id)).to eq([tagged_topic3.id].sort) end + it "can return topics with all specified tags" do + expect(TopicQuery.new(moderator, tags: [tag.name, other_tag.name], match_all_tags: true).list_latest.topics.map(&:id)).to eq([tagged_topic3.id]) + end + it "can return topics with no tags" do expect(TopicQuery.new(moderator, no_tags: true).list_latest.topics.map(&:id)).to eq([no_tags_topic.id]) end diff --git a/spec/controllers/tags_controller_spec.rb b/spec/controllers/tags_controller_spec.rb index 82ea6c13a55..fa814fef60f 100644 --- a/spec/controllers/tags_controller_spec.rb +++ b/spec/controllers/tags_controller_spec.rb @@ -3,9 +3,13 @@ require 'rails_helper' describe TagsController do describe 'show_latest' do let(:tag) { Fabricate(:tag) } + let(:other_tag) { Fabricate(:tag) } let(:category) { Fabricate(:category) } let(:subcategory) { Fabricate(:category, parent_category_id: category.id) } + let(:single_tag_topic) { Fabricate(:topic, tags: [tag]) } + let(:multi_tag_topic) { Fabricate(:topic, tags: [tag, other_tag]) } + context 'tagging disabled' do it "returns 404" do xhr :get, :show_latest, tag_id: tag.name @@ -23,6 +27,14 @@ describe TagsController do expect(response).to be_success end + it "can filter by multiple tags" do + single_tag_topic; multi_tag_topic + xhr :get, :show_latest, tag_id: tag.name, secondary_tag_id: other_tag.name + expect(response).to be_success + expect(assigns(:list).topics).to include multi_tag_topic + expect(assigns(:list).topics).to_not include single_tag_topic + end + it "can filter by category and tag" do xhr :get, :show_latest, tag_id: tag.name, category: category.slug expect(response).to be_success