From 148bfc9be5093a6013ae532fc243953208144760 Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Fri, 17 May 2019 16:26:00 +0800
Subject: [PATCH] DEV: Simplify client and server side code to support removing
 tags.

Follow up to 834c86678fc9b0900d8ce83365068c41bc34f63f.
---
 .../javascripts/discourse/models/topic.js.es6 | 12 ++---
 app/controllers/topics_controller.rb          |  2 +-
 lib/post_revisor.rb                           | 11 -----
 spec/components/post_revisor_spec.rb          |  8 ----
 spec/requests/topics_controller_spec.rb       | 45 ++++++++++++++++++-
 5 files changed, 47 insertions(+), 31 deletions(-)

diff --git a/app/assets/javascripts/discourse/models/topic.js.es6 b/app/assets/javascripts/discourse/models/topic.js.es6
index e769d7e87f1..1f54f1c3347 100644
--- a/app/assets/javascripts/discourse/models/topic.js.es6
+++ b/app/assets/javascripts/discourse/models/topic.js.es6
@@ -652,15 +652,9 @@ Topic.reopenClass({
       delete props.category_id;
     }
 
-    // Annoyingly, empty arrays are not sent across the wire. This
-    // allows us to make a distinction between arrays that were not
-    // sent and arrays that we specifically want to be empty.
-    Object.keys(props).forEach(function(k) {
-      const v = props[k];
-      if (v instanceof Array && v.length === 0) {
-        props[`${k}_empty_array`] = true;
-      }
-    });
+    if (props.tags && props.tags.length === 0) {
+      props.tags = [""];
+    }
 
     return ajax(topic.get("url"), { type: "PUT", data: props }).then(result => {
       // The title can be cleaned up server side
diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb
index 98e914743e8..f578291f146 100644
--- a/app/controllers/topics_controller.rb
+++ b/app/controllers/topics_controller.rb
@@ -324,13 +324,13 @@ class TopicsController < ApplicationController
     end
 
     changes = {}
+
     PostRevisor.tracked_topic_fields.each_key do |f|
       changes[f] = params[f] if params.has_key?(f)
     end
 
     changes.delete(:title) if topic.title == changes[:title]
     changes.delete(:category_id) if topic.category_id.to_i == changes[:category_id].to_i
-    changes.delete(:tags_empty_array) if !topic.tags.exists?
 
     success = true
 
diff --git a/lib/post_revisor.rb b/lib/post_revisor.rb
index 1f94183f186..6189a81ca9c 100644
--- a/lib/post_revisor.rb
+++ b/lib/post_revisor.rb
@@ -101,17 +101,6 @@ class PostRevisor
     end
   end
 
-  track_topic_field(:tags_empty_array) do |tc, val|
-    if val.present? && tc.guardian.can_tag_topics?
-      prev_tags = tc.topic.tags.map(&:name)
-      if !DiscourseTagging.tag_topic_by_names(tc.topic, tc.guardian, [])
-        tc.check_result(false)
-        next
-      end
-      tc.record_change('tags', prev_tags, nil)
-    end
-  end
-
   track_topic_field(:featured_link) do |topic_changes, featured_link|
     if SiteSetting.topic_featured_link_enabled &&
        topic_changes.guardian.can_edit_featured_link?(topic_changes.topic.category_id)
diff --git a/spec/components/post_revisor_spec.rb b/spec/components/post_revisor_spec.rb
index 7f18ac2421b..1cd9ddea2f7 100644
--- a/spec/components/post_revisor_spec.rb
+++ b/spec/components/post_revisor_spec.rb
@@ -677,14 +677,6 @@ describe PostRevisor do
             expect(post.topic.tags.size).to eq(0)
           end
 
-          it "can remove all tags using tags_empty_array" do
-            topic.tags = [Fabricate(:tag, name: "stuff")]
-            result = subject.revise!(user, raw: "lets totally update the body", tags_empty_array: "true")
-            expect(result).to eq(true)
-            post.reload
-            expect(post.topic.tags.size).to eq(0)
-          end
-
           it "can't add staff-only tags" do
             create_staff_tags(['important'])
             result = subject.revise!(user, raw: "lets totally update the body", tags: ['important', 'stuff'])
diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb
index e30cca34e11..2964a9178d7 100644
--- a/spec/requests/topics_controller_spec.rb
+++ b/spec/requests/topics_controller_spec.rb
@@ -1002,14 +1002,55 @@ RSpec.describe TopicsController do
         it "doesn't call the PostRevisor when there is no changes" do
           expect do
             put "/t/#{topic.slug}/#{topic.id}.json", params: {
-              category_id: topic.category_id,
-              tags_empty_array: true
+              category_id: topic.category_id
             }
           end.not_to change(PostRevision.all, :count)
 
           expect(response.status).to eq(200)
         end
 
+        context 'tags' do
+          fab!(:tag) { Fabricate(:tag) }
+
+          before do
+            SiteSetting.tagging_enabled = true
+          end
+
+          it "can add a tag to topic" do
+            expect do
+              put "/t/#{topic.slug}/#{topic.id}.json", params: {
+                tags: [tag.name]
+              }
+            end.to change { topic.reload.first_post.revisions.count }.by(1)
+
+            expect(response.status).to eq(200)
+            expect(topic.tags.pluck(:id)).to contain_exactly(tag.id)
+          end
+
+          it 'does not remove tag if no params is given' do
+            topic.tags << tag
+
+            expect do
+              put "/t/#{topic.slug}/#{topic.id}.json"
+            end.to_not change { topic.reload.tags.count }
+
+            expect(response.status).to eq(200)
+          end
+
+          it 'can remove a tag' do
+            topic.tags << tag
+
+            expect do
+              put "/t/#{topic.slug}/#{topic.id}.json", params: {
+                tags: [""]
+              }
+            end.to change { topic.reload.first_post.revisions.count }.by(1)
+
+            expect(response.status).to eq(200)
+            expect(topic.tags).to eq([])
+          end
+        end
+
         context 'when topic is private' do
           before do
             topic.update!(