Merge pull request #1477 from iangmaia/remove-stale-comments

Delete stale violations when there are no new ones

CHANGELOG.md

@@ -12,6 +12,7 @@
 
 <!-- Your comment below here -->
 * Add keyword arguments to MessageAggregator.aggregate for Ruby 3.X compatibility - [@dirtyhabits97](https://github.com/dirtyhabits97) [#1466](https://github.com/danger/danger/pull/1466)
+* Fix: remove stale violations when there are no new violations to report - [@iangmaia](https://github.com/iangmaia) [#1477](https://github.com/danger/danger/pull/1477)
 <!-- Your comment above here -->
 
 ## 9.4.1

lib/danger/request_sources/github/github.rb

@@ -252,21 +252,34 @@ def delete_old_comments!(except: nil, danger_id: "danger")
       end
 
       def submit_inline_comments!(warnings: [], errors: [], messages: [], markdowns: [], previous_violations: [], danger_id: "danger")
-        # Avoid doing any fetchs if there's no inline comments
-        return {} if (warnings + errors + messages + markdowns).select(&:inline?).empty?
-
-        diff_lines = self.pr_diff.lines
         pr_comments = client.pull_request_comments(ci_source.repo_slug, ci_source.pull_request_id)
         danger_comments = pr_comments.select { |comment| Comment.from_github(comment).generated_by_danger?(danger_id) }
         non_danger_comments = pr_comments - danger_comments
 
+        if (warnings + errors + messages + markdowns).select(&:inline?).empty?
+          delete_old_inline_violations(danger_comments: danger_comments, non_danger_comments: non_danger_comments)
+          return {}
+        end
+
+        diff_lines = self.pr_diff.lines
         warnings = submit_inline_comments_for_kind!(:warning, warnings, diff_lines, danger_comments, previous_violations["warning"], danger_id: danger_id)
         errors = submit_inline_comments_for_kind!(:error, errors, diff_lines, danger_comments, previous_violations["error"], danger_id: danger_id)
         messages = submit_inline_comments_for_kind!(:message, messages, diff_lines, danger_comments, previous_violations["message"], danger_id: danger_id)
         markdowns = submit_inline_comments_for_kind!(:markdown, markdowns, diff_lines, danger_comments, [], danger_id: danger_id)
 
         # submit removes from the array all comments that are still in force
         # so we strike out all remaining ones
+        delete_old_inline_violations(danger_comments: danger_comments, non_danger_comments: non_danger_comments)
+
+        {
+          warnings: warnings,
+          errors: errors,
+          messages: messages,
+          markdowns: markdowns
+        }
+      end
+
+      def delete_old_inline_violations(danger_comments: [], non_danger_comments: [])
         danger_comments.each do |comment|
           violation = violations_from_table(comment["body"]).first
           if !violation.nil? && violation.sticky
@@ -285,13 +298,6 @@ def submit_inline_comments!(warnings: [], errors: [], messages: [], markdowns: [
             client.delete_pull_request_comment(ci_source.repo_slug, comment["id"]) if replies.empty?
           end
         end
-
-        {
-          warnings: warnings,
-          errors: errors,
-          messages: messages,
-          markdowns: markdowns
-        }
       end
 
       def messages_are_equivalent(m1, m2)

spec/lib/danger/danger_core/executor_spec.rb

@@ -262,6 +262,7 @@ def swiftweekly_issue_89_comments_as_json
           allow(fake_client).to receive(:pull_request) { swiftweekly_pr_89_as_json(head_sha, base_sha) }
           allow(fake_client).to receive(:get) { swiftweekly_issues_89_as_json }
           allow(fake_client).to receive(:issue_comments) { swiftweekly_issue_89_comments_as_json }
+          allow(fake_client).to receive(:pull_request_comments) { swiftweekly_issue_89_comments_as_json }
           allow(fake_client).to receive(:delete_comment) { true }
           allow(fake_client).to receive(:create_status) { true }
 

spec/lib/danger/request_sources/github_spec.rb

@@ -298,6 +298,8 @@
       end
 
       it "creates a comment if no danger comments exist" do
+        allow(@g.client).to receive(:pull_request_comments).with("artsy/eigen", "800").and_return([])
+
         comments = []
         allow(@g.client).to receive(:issue_comments).with("artsy/eigen", "800").and_return(comments)
 
@@ -308,6 +310,8 @@
       end
 
       it "updates the issue if no danger comments exist" do
+        allow(@g.client).to receive(:pull_request_comments).with("artsy/eigen", "800").and_return([])
+
         comments = [{ "body" => '"generated_by_danger"', "id" => "12" }]
         allow(@g.client).to receive(:issue_comments).with("artsy/eigen", "800").and_return(comments)
 
@@ -318,6 +322,8 @@
       end
 
       it "creates a new comment instead of updating the issue if --new-comment is provided" do
+        allow(@g.client).to receive(:pull_request_comments).with("artsy/eigen", "800").and_return([])
+
         comments = [{ "body" => '"generated_by_danger"', "id" => "12" }]
         allow(@g.client).to receive(:issue_comments).with("artsy/eigen", "800").and_return(comments)
 
@@ -328,6 +334,8 @@
       end
 
       it "updates the issue if no danger comments exist and a custom danger_id is provided" do
+        allow(@g.client).to receive(:pull_request_comments).with("artsy/eigen", "800").and_return([])
+
         comments = [{ "body" => '"generated_by_another_danger"', "id" => "12" }]
         allow(@g.client).to receive(:issue_comments).with("artsy/eigen", "800").and_return(comments)
 
@@ -338,6 +346,8 @@
       end
 
       it "deletes existing comments if danger doesnt need to say anything" do
+        allow(@g.client).to receive(:pull_request_comments).with("artsy/eigen", "800").and_return([])
+
         comments = [{ "body" => '"generated_by_danger"', "id" => "12" }]
         allow(@g.client).to receive(:issue_comments).with("artsy/eigen", "800").and_return(comments)
 
@@ -348,6 +358,8 @@
       end
 
       it "deletes existing comments if danger doesnt need to say anything and a custom danger_id is provided" do
+        allow(@g.client).to receive(:pull_request_comments).with("artsy/eigen", "800").and_return([])
+
         comments = [{ "body" => '"generated_by_another_danger"', "id" => "12" }]
         allow(@g.client).to receive(:issue_comments).with("artsy/eigen", "800").and_return(comments)
         expect(@g.client).to receive(:delete_comment).with("artsy/eigen", "12").and_return({})
@@ -402,6 +414,8 @@
       end
 
       it "deletes all inline comments if there are no violations at all" do
+        allow(@g.client).to receive(:pull_request_comments).with("artsy/eigen", "800").and_return([])
+
         allow(@g.client).to receive(:delete_comment).with("artsy/eigen", main_issue_id)
         allow(@g.client).to receive(:delete_comment).with("artsy/eigen", inline_issue_id_1)
         allow(@g.client).to receive(:delete_comment).with("artsy/eigen", inline_issue_id_2)