FIX: Double approval should do nothing. (#9)

FIX: Double approval should do nothing. (#9)

  • FIX: Double approval should do nothing.

  • FIX: Redirect user to homepage if no more pending topics.

diff --git a/app/controllers/discourse_code_review/code_review_controller.rb b/app/controllers/discourse_code_review/code_review_controller.rb
index 5e20d6c..239bac1 100644
--- a/app/controllers/discourse_code_review/code_review_controller.rb
+++ b/app/controllers/discourse_code_review/code_review_controller.rb
@@ -54,25 +54,27 @@ module DiscourseCodeReview
 
       tags = topic.tags.pluck(:name)
 
-      tags -= [
-        SiteSetting.code_review_approved_tag,
-        SiteSetting.code_review_pending_tag
-      ]
-
-      tags << SiteSetting.code_review_followup_tag
-
-      DiscourseTagging.tag_topic_by_names(topic, Guardian.new(current_user), tags)
-
-      topic.add_moderator_post(
-        current_user,
-        nil,
-        bump: false,
-        post_type: Post.types[:small_action],
-        action_code: "followup"
-      )
+      if !tags.include?(SiteSetting.code_review_followup_tag)
+        tags -= [
+          SiteSetting.code_review_approved_tag,
+          SiteSetting.code_review_pending_tag
+        ]
+
+        tags << SiteSetting.code_review_followup_tag
+
+        DiscourseTagging.tag_topic_by_names(topic, Guardian.new(current_user), tags)
+
+        topic.add_moderator_post(
+          current_user,
+          nil,
+          bump: false,
+          post_type: Post.types[:small_action],
+          action_code: "followup"
+        )
 
-      if SiteSetting.code_review_auto_assign_on_followup && topic.user.staff?
-        DiscourseEvent.trigger(:assign_topic, topic, topic.user, current_user)
+        if SiteSetting.code_review_auto_assign_on_followup && topic.user.staff?
+          DiscourseEvent.trigger(:assign_topic, topic, topic.user, current_user)
+        end
       end
 
       render_next_topic(topic.category_id)
@@ -89,25 +91,27 @@ module DiscourseCodeReview
 
       tags = topic.tags.pluck(:name)
 
-      tags -= [
-        SiteSetting.code_review_followup_tag,
-        SiteSetting.code_review_pending_tag
-      ]
+      if !tags.include?(SiteSetting.code_review_approved_tag)
+        tags -= [
+          SiteSetting.code_review_followup_tag,
+          SiteSetting.code_review_pending_tag
+        ]
 
-      tags << SiteSetting.code_review_approved_tag
+        tags << SiteSetting.code_review_approved_tag
 
-      DiscourseTagging.tag_topic_by_names(topic, Guardian.new(current_user), tags)
+        DiscourseTagging.tag_topic_by_names(topic, Guardian.new(current_user), tags)
 
-      topic.add_moderator_post(
-        current_user,
-        nil,
-        bump: false,
-        post_type: Post.types[:small_action],
-        action_code: "approved"
-      )
+        topic.add_moderator_post(
+          current_user,
+          nil,
+          bump: false,
+          post_type: Post.types[:small_action],
+          action_code: "approved"
+        )
 
-      if SiteSetting.code_review_auto_unassign_on_approve && topic.user.staff?
-        DiscourseEvent.trigger(:unassign_topic, topic, current_user)
+        if SiteSetting.code_review_auto_unassign_on_approve && topic.user.staff?
+          DiscourseEvent.trigger(:unassign_topic, topic, current_user)
+        end
       end
 
       render_next_topic(topic.category_id)
diff --git a/assets/javascripts/discourse/initializers/init-code-review.js.es6 b/assets/javascripts/discourse/initializers/init-code-review.js.es6
index e27cc67..387c357 100644
--- a/assets/javascripts/discourse/initializers/init-code-review.js.es6
+++ b/assets/javascripts/discourse/initializers/init-code-review.js.es6
@@ -13,6 +13,8 @@ function actOnCommit(topic, action) {
     .then(result => {
       if (result.next_topic_url) {
         DiscourseURL.routeTo(result.next_topic_url);
+      } else {
+        DiscourseURL.routeTo("/");
       }
     })
     .catch(popupAjaxError);
diff --git a/spec/requests/discourse_code_review/code_review_controller_spec.rb b/spec/requests/discourse_code_review/code_review_controller_spec.rb
index 585bc91..ff38f20 100644
--- a/spec/requests/discourse_code_review/code_review_controller_spec.rb
+++ b/spec/requests/discourse_code_review/code_review_controller_spec.rb
@@ -80,6 +80,14 @@ describe DiscourseCodeReview::CodeReviewController do
 
       expect(commit.topic.tags.pluck(:name)).to include("hi", SiteSetting.code_review_approved_tag)
     end
+
+    it 'does nothing when approving already approved posts' do
+      sign_in Fabricate(:admin)
+      commit = create_post(raw: "this is a fake commit", tags: ["hi", SiteSetting.code_review_pending_tag])
+
+      expect { post '/code-review/approve.json', params: { topic_id: commit.topic_id } }.to change { commit.topic.posts.count }.by(1)
+      expect { post '/code-review/approve.json', params: { topic_id: commit.topic_id } }.to change { commit.topic.posts.count }.by(0)
+    end
   end
 
   context '.followup' do
@@ -97,6 +105,14 @@ describe DiscourseCodeReview::CodeReviewController do
 
       expect(commit.topic.tags.pluck(:name)).to include("hi", SiteSetting.code_review_followup_tag)
     end
+
+    it 'does nothing when following-up already followed-up posts' do
+      sign_in Fabricate(:admin)
+      commit = create_post(raw: "this is a fake commit", tags: ["hi", SiteSetting.code_review_pending_tag])
+
+      expect { post '/code-review/followup.json', params: { topic_id: commit.topic_id } }.to change { commit.topic.posts.count }.by(1)
+      expect { post '/code-review/followup.json', params: { topic_id: commit.topic_id } }.to change { commit.topic.posts.count }.by(0)
+    end
   end
 
   context '.render_next_topic' do

GitHub sha: b12d56f2