From 8c9d390cacafce80c622f94ec00661d8a5278bd0 Mon Sep 17 00:00:00 2001
From: Neil Lalonde <neillalonde@gmail.com>
Date: Fri, 28 Oct 2016 15:11:43 -0400
Subject: [PATCH] FIX: Tags used only on deleted topics could not be used again

---
 app/controllers/tags_controller.rb | 16 +++-------------
 app/models/tag.rb                  | 13 +++++++++++--
 spec/models/tag_spec.rb            |  6 +++---
 3 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb
index bbf7a69c3ee..43e6f8aee9e 100644
--- a/app/controllers/tags_controller.rb
+++ b/app/controllers/tags_controller.rb
@@ -16,12 +16,12 @@ class TagsController < ::ApplicationController
                          .where("id in (?)", guardian.allowed_category_ids)
                          .preload(:tags)
     category_tag_counts = categories.map do |c|
-      h = Tag.category_tags_by_count_query(c, limit: 300).count
+      h = Tag.category_tags_by_count_query(c, limit: 300).count(Tag::COUNT_ARG)
       h.merge!(c.tags.where.not(name: h.keys).inject({}) { |sum,t| sum[t.name] = 0; sum }) # unused tags
       {id: c.id, tags: self.class.tag_counts_json(h)}
     end
 
-    tag_counts = self.class.tags_by_count(guardian, limit: 300).count
+    tag_counts = self.class.tags_by_count(guardian, limit: 300).count(Tag::COUNT_ARG)
     @tags = self.class.tag_counts_json(tag_counts)
 
     @description_meta = I18n.t("tags.title")
@@ -145,17 +145,7 @@ class TagsController < ::ApplicationController
       }
     )
 
-    tags = tags_with_counts.count.map {|t, c| { id: t, text: t, count: c } }
-
-    unused_tags = DiscourseTagging.filter_allowed_tags(
-      Tag.where(topic_count: 0),
-      guardian,
-      { for_input: params[:filterForInput], term: params[:q], category: category, selected_tags: params[:selected_tags] }
-    )
-
-    unused_tags.each do |t|
-      tags << { id: t.name, text: t.name, count: 0 }
-    end
+    tags = tags_with_counts.count(Tag::COUNT_ARG).map {|t, c| { id: t, text: t, count: c } }
 
     json_response = { results: tags }
 
diff --git a/app/models/tag.rb b/app/models/tag.rb
index f6d4661b2fe..95a944776c4 100644
--- a/app/models/tag.rb
+++ b/app/models/tag.rb
@@ -12,8 +12,17 @@ class Tag < ActiveRecord::Base
   has_many :tag_group_memberships
   has_many :tag_groups, through: :tag_group_memberships
 
+  COUNT_ARG = "topic_tags.id"
+
+  # Apply more activerecord filters to the tags_by_count_query, and then
+  # fetch the result with .count(Tag::COUNT_ARG).
+  #
+  # e.g., Tag.tags_by_count_query.where("topics.category_id = ?", category.id).count(Tag::COUNT_ARG)
   def self.tags_by_count_query(opts={})
-    q = TopicTag.joins(:tag, :topic).group("topic_tags.tag_id, tags.name").order('count_all DESC')
+    q = Tag.joins("LEFT JOIN topic_tags ON tags.id = topic_tags.tag_id")
+           .joins("LEFT JOIN topics ON topics.id = topic_tags.topic_id")
+           .group("tags.id, tags.name")
+           .order('count_topic_tags_id DESC')
     q = q.limit(opts[:limit]) if opts[:limit]
     q
   end
@@ -37,7 +46,7 @@ class Tag < ActiveRecord::Base
       category: category
     )
 
-    tags.count.map {|name, _| name}
+    tags.count(COUNT_ARG).map {|name, _| name}
   end
 
   def self.include_tags?
diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb
index d09cd732448..3cce61225ec 100644
--- a/spec/models/tag_spec.rb
+++ b/spec/models/tag_spec.rb
@@ -17,7 +17,7 @@ describe Tag do
 
   describe '#tags_by_count_query' do
     it "returns empty hash if nothing is tagged" do
-      expect(described_class.tags_by_count_query.count).to eq({})
+      expect(described_class.tags_by_count_query.count(Tag::COUNT_ARG)).to eq({})
     end
 
     context "with some tagged topics" do
@@ -31,13 +31,13 @@ describe Tag do
       end
 
       it "returns tag names with topic counts in a hash" do
-        counts = described_class.tags_by_count_query.count
+        counts = described_class.tags_by_count_query.count(Tag::COUNT_ARG)
         expect(counts[@tags[0].name]).to eq(2)
         expect(counts[@tags[1].name]).to eq(1)
       end
 
       it "can be used to filter before doing the count" do
-        counts = described_class.tags_by_count_query.where("topics.id = ?", @topics[1].id).count
+        counts = described_class.tags_by_count_query.where("topics.id = ?", @topics[1].id).count(Tag::COUNT_ARG)
         expect(counts).to eq({@tags[0].name => 1})
       end
     end