From 7166d7de9a72b8174cf253ab9b7081ae3f505d01 Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Fri, 19 Oct 2018 13:44:43 +0100
Subject: [PATCH] FIX: Prevent duplicate tags in tag-choosers (#6512)

* FIX: Prevent duplicate tags in tag-choosers

This reverts 5685b45, which fixes the duplicate tags problem.
The fix introduced by 5685b45 is re-implemented on the server.
---
 .../select-kit/components/mini-tag-chooser.js.es6     |  6 ------
 .../select-kit/components/tag-chooser.js.es6          |  6 ------
 .../select-kit/components/tag-group-chooser.js.es6    |  6 ------
 app/controllers/tags_controller.rb                    |  7 ++++++-
 spec/requests/tags_controller_spec.rb                 | 11 +++++++++++
 5 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/app/assets/javascripts/select-kit/components/mini-tag-chooser.js.es6 b/app/assets/javascripts/select-kit/components/mini-tag-chooser.js.es6
index 765aae62dde..5484dbae8e2 100644
--- a/app/assets/javascripts/select-kit/components/mini-tag-chooser.js.es6
+++ b/app/assets/javascripts/select-kit/components/mini-tag-chooser.js.es6
@@ -192,12 +192,6 @@ export default ComboBox.extend(TagsMixin, {
       return { id: result.text, name: result.text, count: result.count };
     });
 
-    // if forbidden we probably have an existing tag which is not in the list of
-    // returned tags, so we manually add it at the top
-    if (json.forbidden) {
-      results.unshift({ id: json.forbidden, name: json.forbidden, count: 0 });
-    }
-
     return results;
   },
 
diff --git a/app/assets/javascripts/select-kit/components/tag-chooser.js.es6 b/app/assets/javascripts/select-kit/components/tag-chooser.js.es6
index 5b0946e6923..90c291b4de3 100644
--- a/app/assets/javascripts/select-kit/components/tag-chooser.js.es6
+++ b/app/assets/javascripts/select-kit/components/tag-chooser.js.es6
@@ -132,12 +132,6 @@ export default MultiSelectComponent.extend(TagsMixin, {
       return { id: result.text, name: result.text, count: result.count };
     });
 
-    // if forbidden we probably have an existing tag which is not in the list of
-    // returned tags, so we manually add it at the top
-    if (json.forbidden) {
-      results.unshift({ id: json.forbidden, name: json.forbidden, count: 0 });
-    }
-
     return results;
   }
 });
diff --git a/app/assets/javascripts/select-kit/components/tag-group-chooser.js.es6 b/app/assets/javascripts/select-kit/components/tag-group-chooser.js.es6
index 2bfa1b41fa1..4a563c0cbd9 100644
--- a/app/assets/javascripts/select-kit/components/tag-group-chooser.js.es6
+++ b/app/assets/javascripts/select-kit/components/tag-group-chooser.js.es6
@@ -88,12 +88,6 @@ export default MultiSelectComponent.extend(TagsMixin, {
       return { id: result.text, name: result.text, count: result.count };
     });
 
-    // if forbidden we probably have an existing tag which is not in the list of
-    // returned tags, so we manually add it at the top
-    if (json.forbidden) {
-      results.unshift({ id: json.forbidden, name: json.forbidden, count: 0 });
-    }
-
     return results;
   }
 });
diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb
index a493d9c25ca..9b73d523553 100644
--- a/app/controllers/tags_controller.rb
+++ b/app/controllers/tags_controller.rb
@@ -178,8 +178,13 @@ class TagsController < ::ApplicationController
   def search
     category = params[:categoryId] ? Category.find_by_id(params[:categoryId]) : nil
 
+    # Prioritize exact matches when ordering
+    order_query = Tag.sanitize_sql_for_order(
+      ["lower(name) = lower(?) DESC, topic_count DESC", params[:q]]
+    )
+
     tags_with_counts = DiscourseTagging.filter_allowed_tags(
-      Tag.order('topic_count DESC').limit(params[:limit]),
+      Tag.order(order_query).limit(params[:limit]),
       guardian,
       for_input: params[:filterForInput],
       term: params[:q],
diff --git a/spec/requests/tags_controller_spec.rb b/spec/requests/tags_controller_spec.rb
index c24712daff7..4a0633301ce 100644
--- a/spec/requests/tags_controller_spec.rb
+++ b/spec/requests/tags_controller_spec.rb
@@ -308,6 +308,17 @@ describe TagsController do
         expect(json["results"].map { |j| j["id"] }.sort).to eq(['stuff', 'stumped'])
       end
 
+      it "returns tags ordered by topic_count, and prioritises exact matches" do
+        Fabricate(:tag, name: 'tag1', topic_count: 10)
+        Fabricate(:tag, name: 'tag2', topic_count: 100)
+        Fabricate(:tag, name: 'tag', topic_count: 1)
+
+        get '/tags/filter/search.json', params: { q: 'tag', limit: 2 }
+        expect(response.status).to eq(200)
+        json = ::JSON.parse(response.body)
+        expect(json['results'].map { |j| j['id'] }).to eq(['tag', 'tag2'])
+      end
+
       it "can say if given tag is not allowed" do
         yup, nope = Fabricate(:tag, name: 'yup'), Fabricate(:tag, name: 'nope')
         category = Fabricate(:category, tags: [yup])