Merge pull request #4315 from tgxworld/fix_tags_not_in_category_showing

Tags which are not allowed in a category showing in drop down.

app/assets/javascripts/discourse/components/tag-drop.js.es6

@@ -1,4 +1,5 @@
 import { setting } from 'discourse/lib/computed';
+import computed from 'ember-addons/ember-computed-decorators';
 
 export default Ember.Component.extend({
   classNameBindings: [':tag-drop', 'tag::no-category', 'tags:has-drop','categoryStyle','tagClass'],
@@ -10,13 +11,14 @@ export default Ember.Component.extend({
 
   tagName: 'li',
 
-  tags: function() {
-    if (this.siteSettings.tags_sort_alphabetically && Discourse.Site.currentProp('top_tags')) {
-      return Discourse.Site.currentProp('top_tags').sort();
+  @computed('site.top_tags')
+  tags(topTags) {
+    if (this.siteSettings.tags_sort_alphabetically && topTags) {
+      return topTags.sort();
     } else {
-      return Discourse.Site.currentProp('top_tags');
+      return topTags;
     }
-  }.property('site.top_tags'),
+  },
 
   iconClass: function() {
     if (this.get('expanded')) { return "fa fa-caret-down"; }

app/assets/javascripts/discourse/routes/build-category-route.js.es6

@@ -67,6 +67,9 @@ export default (filter, params) => {
       return findTopicList(this.store, this.topicTrackingState, listFilter, findOpts, extras).then(list => {
         TopicList.hideUniformCategory(list, category);
         this.set('topics', list);
+        if (list.topic_list.tags) {
+          Discourse.Site.currentProp('top_tags', list.topic_list.tags);
+        }
         return list;
       });
     },

app/models/tag.rb

@@ -23,10 +23,21 @@ class Tag < ActiveRecord::Base
                              .where("topics.category_id = ?", category.id)
   end
 
-  def self.top_tags(limit_arg=nil)
-    self.tags_by_count_query(limit: limit_arg || SiteSetting.max_tags_in_filter_list)
-        .count
-        .map {|name, count| name}
+  def self.top_tags(limit_arg: nil, category: nil)
+    limit = limit_arg || SiteSetting.max_tags_in_filter_list
+
+    tags =
+      if category
+        self.category_tags_by_count_query(category, limit: limit)
+      else
+        self.tags_by_count_query(limit: limit)
+      end
+
+    tags.count.map {|name, _| name}
+  end
+
+  def self.include_tags?
+    SiteSetting.tagging_enabled && SiteSetting.show_filter_by_tag
   end
 end
 

app/models/topic_list.rb

@@ -13,7 +13,8 @@ class TopicList
                 :draft_sequence,
                 :filter,
                 :for_period,
-                :per_page
+                :per_page,
+                :tags
 
   def initialize(filter, current_user, topics, opts=nil)
     @filter = filter
@@ -21,16 +22,24 @@ class TopicList
     @topics_input = topics
     @opts = opts || {}
 
+    if @opts[:category]
+      @category = Category.find_by(id: @opts[:category_id])
+    end
+
     preloaded_custom_fields << DiscourseTagging::TAGS_FIELD_NAME if SiteSetting.tagging_enabled
   end
 
-  def preload_key
-    if @opts[:category]
-      c = Category.where(id: @opts[:category_id]).first
-      return "topic_list_#{c.url.sub(/^\//, '')}/l/#{@filter}" if c
-    end
+  def tags
+    opts = @category ? { category: @category } : {}
+    Tag.top_tags(opts)
+  end
 
-    "topic_list_#{@filter}"
+  def preload_key
+    if @category
+      "topic_list_#{@category.url.sub(/^\//, '')}/l/#{@filter}"
+    else
+      "topic_list_#{@filter}"
+    end
   end
 
   # Lazy initialization

app/serializers/category_serializer.rb

@@ -84,7 +84,7 @@ class CategorySerializer < BasicCategorySerializer
   end
 
   def allowed_tags
-    object.tags.pluck(:name)
+    Tag.top_tags(category: object)
   end
 
   def include_allowed_tag_groups?
@@ -94,5 +94,4 @@ class CategorySerializer < BasicCategorySerializer
   def allowed_tag_groups
     object.tag_groups.pluck(:name)
   end
-
 end

app/serializers/site_serializer.rb

@@ -98,15 +98,16 @@ class SiteSerializer < ApplicationSerializer
   def include_tags_filter_regexp?
     SiteSetting.tagging_enabled
   end
+
   def tags_filter_regexp
     DiscourseTagging::TAGS_FILTER_REGEXP.source
   end
 
   def include_top_tags?
-    SiteSetting.tagging_enabled && SiteSetting.show_filter_by_tag
+    Tag.include_tags?
   end
+
   def top_tags
     Tag.top_tags
   end
-
 end

app/serializers/topic_list_serializer.rb

@@ -24,11 +24,6 @@ class TopicListSerializer < ApplicationSerializer
   end
 
   def include_tags?
-    SiteSetting.tagging_enabled && SiteSetting.show_filter_by_tag
+    Tag.include_tags?
   end
-
-  def tags
-    Tag.top_tags
-  end
-
 end

lib/topic_query.rb

@@ -9,7 +9,6 @@ require_dependency 'topic_query_sql'
 require_dependency 'avatar_lookup'
 
 class TopicQuery
-  # Could be rewritten to %i if Ruby 1.9 is no longer supported
   VALID_OPTIONS = %i(except_topic_ids
                      exclude_category_ids
                      limit
@@ -460,7 +459,6 @@ 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])

spec/models/topic_list_spec.rb

@@ -36,4 +36,28 @@ describe TopicList do
       expect(topic_list.preloaded_custom_fields).to eq(Set.new([DiscourseTagging::TAGS_FIELD_NAME]))
     end
   end
+
+  describe '#tags' do
+    let(:tag) { Fabricate(:tag, topics: [topic]) }
+    let(:other_tag) { Fabricate(:tag, topics: [topic]) }
+
+    it 'should return the right tags' do
+      output = [tag.name, other_tag.name]
+      expect(topic_list.tags.sort).to eq(output.sort)
+    end
+
+    describe 'when topic list is filtered by category' do
+      let(:category) { Fabricate(:category) }
+      let(:topic) { Fabricate(:topic, category: category) }
+      let(:tag) { Fabricate(:tag, topics: [topic], categories: [category]) }
+      let(:topic_list) { TopicList.new('latest', topic.user, [topic], { category: category.id, category_id: category.id }) }
+
+      it 'should only return tags allowed in the category' do
+        other_tag
+        output = [tag.name]
+
+        expect(topic_list.tags).to eq(output)
+      end
+    end
+  end
 end