diff --git a/CHANGELOG.md b/CHANGELOG.md index 842edaab..8afb87ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ * 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) ## 9.4.1 diff --git a/lib/danger/request_sources/github/github.rb b/lib/danger/request_sources/github/github.rb index ede735fe..bd04a928 100644 --- a/lib/danger/request_sources/github/github.rb +++ b/lib/danger/request_sources/github/github.rb @@ -252,14 +252,16 @@ 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) @@ -267,6 +269,17 @@ def submit_inline_comments!(warnings: [], errors: [], messages: [], markdowns: [ # 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) diff --git a/spec/lib/danger/danger_core/executor_spec.rb b/spec/lib/danger/danger_core/executor_spec.rb index cd37d4e1..057fefec 100644 --- a/spec/lib/danger/danger_core/executor_spec.rb +++ b/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 } diff --git a/spec/lib/danger/request_sources/github_spec.rb b/spec/lib/danger/request_sources/github_spec.rb index 002657ee..e9fc70c7 100644 --- a/spec/lib/danger/request_sources/github_spec.rb +++ b/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)