diff --git a/lib/pronto/comment.rb b/lib/pronto/comment.rb index 69a56dfa..52373edb 100644 --- a/lib/pronto/comment.rb +++ b/lib/pronto/comment.rb @@ -1,5 +1,5 @@ module Pronto - Comment = Struct.new(:sha, :body, :path, :position) do + Comment = Struct.new(:sha, :body, :path, :position, :id) do def ==(other) position == other.position && path == other.path && diff --git a/lib/pronto/formatter/git_formatter.rb b/lib/pronto/formatter/git_formatter.rb index 6a781dfe..5e734b56 100644 --- a/lib/pronto/formatter/git_formatter.rb +++ b/lib/pronto/formatter/git_formatter.rb @@ -6,8 +6,10 @@ def format(messages, repo, patches) existing = existing_comments(messages, client, repo) comments = new_comments(messages, patches) additions = remove_duplicate_comments(existing, comments) + outdated_comments = existing.reject { |key, _| comments.key?(key) }.values.flatten + remove_outdated_comments(client, outdated_comments) if respond_to?(:remove_outdated_comments) submit_comments(client, additions) - + approve_pull_request(comments.count, additions.count, client) if defined?(self.approve_pull_request) "#{additions.count} Pronto messages posted to #{pretty_name} (#{existing.count} existing)" diff --git a/lib/pronto/formatter/github_pull_request_formatter.rb b/lib/pronto/formatter/github_pull_request_formatter.rb index e0778b10..4f8b085b 100644 --- a/lib/pronto/formatter/github_pull_request_formatter.rb +++ b/lib/pronto/formatter/github_pull_request_formatter.rb @@ -16,6 +16,14 @@ def pretty_name def line_number(message, _) message.line&.new_lineno end + + def remove_outdated_comments(client, outdated_comments) + outdated_comments.each do |comment| + client.delete_comment(comment) + end + rescue Octokit::UnprocessableEntity, HTTParty::Error => e + warn "Failed to delete: #{e.message}" + end end end end diff --git a/lib/pronto/github.rb b/lib/pronto/github.rb index dc702cea..5027c25b 100644 --- a/lib/pronto/github.rb +++ b/lib/pronto/github.rb @@ -11,7 +11,7 @@ def pull_comments(sha) @comment_cache["#{pull_id}/#{sha}"] ||= begin client.pull_comments(slug, pull_id).map do |comment| Comment.new( - sha, comment.body, comment.path, comment.line || comment.original_line + sha, comment.body, comment.path, comment.line || comment.original_line, comment.id ) end end @@ -66,6 +66,10 @@ def create_commit_status(status) description: status.description) end + def delete_comment(comment) + client.delete_pull_comment(slug, comment.id) + end + private def create_pull_request_review(comments) diff --git a/spec/pronto/formatter/github_pull_request_formatter_spec.rb b/spec/pronto/formatter/github_pull_request_formatter_spec.rb index bf2689ac..55b13c3f 100644 --- a/spec/pronto/formatter/github_pull_request_formatter_spec.rb +++ b/spec/pronto/formatter/github_pull_request_formatter_spec.rb @@ -25,7 +25,7 @@ module Formatter octokit_client .stub(:pull_comments) .once - .and_return([double(body: 'a comment', path: 'a/path', line: 5)]) + .and_return([double(body: 'a comment', path: 'a/path', line: 5, id: 1_881_471_822)]) end specify do @@ -33,6 +33,8 @@ module Formatter .should_receive(:create_pull_comment) .once + octokit_client.should_receive(:delete_pull_comment).with(nil, 1_881_471_822).and_return(true) + subject end @@ -43,7 +45,7 @@ module Formatter specify do octokit_client.should_receive(:pull_comments).and_return( - [double(body: 'existed', path: 'path/to', line: line.new_lineno)] + [double(body: 'existed', path: 'path/to', line: line.new_lineno, id: 1_881_471_822)] ) octokit_client.should_not_receive(:create_pull_comment) @@ -59,7 +61,7 @@ module Formatter specify do octokit_client.should_receive(:pull_comments).and_return( - [double(body: 'existed', path: 'path/to', line: line.new_lineno)] + [double(body: 'existed', path: 'path/to', line: line.new_lineno, id: 1_881_471_822)] ) octokit_client.should_receive(:create_pull_comment).once @@ -92,6 +94,8 @@ module Formatter .should_receive(:create_pull_comment) .and_raise(error) + octokit_client.should_receive(:delete_pull_comment).with(nil, 1_881_471_822).and_return(true) + $stderr.should_receive(:puts) do |line| line.should =~ /Failed to post/ line.should =~ /Validation Failed/ diff --git a/spec/pronto/formatter/github_pull_request_review_formatter_spec.rb b/spec/pronto/formatter/github_pull_request_review_formatter_spec.rb index c01a37ef..c80a7bca 100644 --- a/spec/pronto/formatter/github_pull_request_review_formatter_spec.rb +++ b/spec/pronto/formatter/github_pull_request_review_formatter_spec.rb @@ -33,7 +33,7 @@ module Formatter specify do octokit_client.should_receive(:pull_comments).and_return( - [double(body: 'existed', path: 'path/to', line: line.new_lineno)] + [double(body: 'existed', path: 'path/to', line: line.new_lineno, id: 1_881_471_822)] ) octokit_client.should_not_receive(:create_pull_request_review) @@ -49,7 +49,7 @@ module Formatter specify do octokit_client.should_receive(:pull_comments).and_return( - [double(body: 'existed', path: 'path/to', line: line.new_lineno)] + [double(body: 'existed', path: 'path/to', line: line.new_lineno, id: 1_881_471_822)] ) octokit_client.should_receive(:create_pull_request_review).once diff --git a/spec/pronto/github_spec.rb b/spec/pronto/github_spec.rb index 0991c7c7..7f4344fd 100644 --- a/spec/pronto/github_spec.rb +++ b/spec/pronto/github_spec.rb @@ -5,7 +5,7 @@ module Pronto let(:ssh_remote_urls) { ["git@github.com:#{github_slug}.git"] } let(:github_slug) { 'prontolabs/pronto' } let(:sha) { '61e4bef' } - let(:comment) { double(body: 'note', path: 'path', line: 1, position: 1) } + let(:comment) { double(body: 'note', path: 'path', line: 1, position: 1, id: 1_881_471_822) } let(:empty_client_options) do { event: 'COMMENT'