Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removing old bitbucket pull request commits #421

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion lib/pronto/bitbucket.rb
Expand Up @@ -3,7 +3,8 @@ class Bitbucket < Client
def pull_comments(sha)
@comment_cache["#{pull_id}/#{sha}"] ||= begin
client.pull_comments(slug, pull_id).map do |comment|
Comment.new(sha, comment.content, comment.filename, comment.line_to)
Comment.new(sha, comment.content, comment.filename,
comment.line_to, comment&.id)
end
end
end
Expand Down Expand Up @@ -39,6 +40,12 @@ def approve_pull_request
def unapprove_pull_request
client.unapprove_pull_request(slug, pull_id)
end

def delete_comment(comment_id)
@config.logger.log("Delete comment id:#{comment_id}")
client.delete_comment(slug, pull_id, comment_id)
end

private

def slug
Expand Down
6 changes: 5 additions & 1 deletion lib/pronto/clients/bitbucket_client.rb
Expand Up @@ -21,7 +21,7 @@ def create_commit_comment(slug, sha, body, path, position)
end

def pull_comments(slug, pull_id)
response = get("/#{slug}/pullrequests/#{pull_id}/comments?pagelen=100")
response = get("/#{slug}/pullrequests/#{pull_id}/comments?q=deleted=false&pagelen=100")
parse_comments(openstruct(response))
result = parse_comments(openstruct(response))
while (response['next'])
Expand All @@ -48,6 +48,10 @@ def unapprove_pull_request(slug, pull_id)
self.class.delete("/#{slug}/pullrequests/#{pull_id}/approve")
end

def delete_comment(slug, pull_id, comment_id)
self.class.delete("/#{slug}/pullrequests/#{pull_id}/comments/#{comment_id}")
end

private

def openstruct(response)
Expand Down
2 changes: 1 addition & 1 deletion 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, :client_id) do
def ==(other)
position == other.position &&
path == other.path &&
Expand Down
21 changes: 21 additions & 0 deletions lib/pronto/formatter/bitbucket_pull_request_formatter.rb
@@ -1,6 +1,27 @@
module Pronto
module Formatter
class BitbucketPullRequestFormatter < PullRequestFormatter
def format(messages, repo, patches)
client = client_module.new(repo)
existing = existing_comments(messages, client, repo)
comments = new_comments(messages, patches)
additions = remove_duplicate_comments(existing, comments)

remove_comments(client, existing, comments) if existing.any?
Comment on lines +8 to +10
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be optional. I'm not sure how we'd go about it, maybe a separate PullRequestFormatter which deletes comments?


submit_comments(client, additions)

approve_pull_request(comments.count, additions.count, client) if defined?(approve_pull_request)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need to check this? It's defined below.


"#{additions.count} Pronto messages posted to #{pretty_name}"
end

def remove_comments(client, existing, comments)
removed_comments = dedupe_comments(ungrouped_comments(comments),
ungrouped_comments(existing))
removed_comments.map { |c| client.delete_comment(c.client_id) }
end

def client_module
Bitbucket
end
Expand Down
6 changes: 6 additions & 0 deletions lib/pronto/formatter/git_formatter.rb
Expand Up @@ -41,6 +41,12 @@ def grouped_comments(comments)
comments.group_by { |comment| [comment.path, comment.position] }
end

def ungrouped_comments(comments)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see why this should be in git_formatter, should directly be in bitbucket_pull_request_formatter.

return [] unless comments.any?

comments.values.flatten
end

def consolidate_comments(comments)
comment = comments.first
if comments.length > 1
Expand Down
1 change: 1 addition & 0 deletions pronto.gemspec
Expand Up @@ -48,6 +48,7 @@ Gem::Specification.new do |s|
s.add_runtime_dependency('rugged', '>= 0.23.0', '< 2.0')
s.add_runtime_dependency('thor', '>= 0.20.3', '< 2.0')
s.add_development_dependency('bundler', '>= 1.15')
s.add_development_dependency('pry', '>= 0.14.1')
s.add_development_dependency('pronto-rubocop', '~> 0.10.0')
s.add_development_dependency('rake', '~> 12.0')
s.add_development_dependency('rspec', '~> 3.4')
Expand Down
2 changes: 1 addition & 1 deletion spec/pronto/bitbucket_spec.rb
Expand Up @@ -8,7 +8,7 @@ module Pronto
end
let(:sha) { '61e4bef' }
let(:comment) do
double(content: 'note', filename: 'path', line_to: 1, position: 1)
double(content: 'note', filename: 'path', line_to: 1, position: 1, id: 1)
end

describe '#slug' do
Expand Down
13 changes: 13 additions & 0 deletions spec/pronto/clients/bitbucket_client_spec.rb
Expand Up @@ -29,6 +29,19 @@ module Pronto
end
end

describe '#delete_comment' do
subject { client.delete_comment(slug, pull_id, comment_id) }
let(:slug) { 'prontolabs/pronto' }
let(:pull_id) { 1 }
let(:comment_id) { 11 }

context 'success' do
before { BitbucketClient.stub(:delete).and_return(response) }
let(:response) { double('Response', success?: true) }
its(:success?) { should be_truthy }
end
end

describe '#unapprove_pull_request' do
subject { client.unapprove_pull_request(slug, pull_id) }
let(:slug) { 'prontolabs/pronto' }
Expand Down