From 5bfec246f0fcc84ec48ea5c5f0ddcd7f791a9aa5 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Mon, 20 Apr 2015 17:24:16 +0200 Subject: [PATCH 1/3] Check if the reference has been already created before adding comment to jira. --- app/models/project_services/jira_service.rb | 34 ++++++++++++++++++--- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb index bacec6cb2a..1e63b484ae 100644 --- a/app/models/project_services/jira_service.rb +++ b/app/models/project_services/jira_service.rb @@ -136,7 +136,7 @@ class JiraService < IssueTrackerService end def add_comment(data, issue_name) - url = add_comment_url(issue_name) + url = comment_url(issue_name) user_name = data[:user][:name] user_url = data[:user][:url] entity_name = data[:entity][:name] @@ -147,9 +147,11 @@ class JiraService < IssueTrackerService message = { body: "[#{user_name}|#{user_url}] mentioned this issue in [a #{entity_name} of #{project_name}|#{entity_url}]." - }.to_json + } - send_message(url, message) + unless existing_comment?(issue_name, message) + send_message(url, message.to_json) + end end @@ -183,6 +185,30 @@ class JiraService < IssueTrackerService Rails.logger.info "#{self.class.name} ERROR: #{e.message}. Hostname: #{url}." end + def existing_comment?(issue_name, new_comment) + result = JiraService.get( + comment_url(issue_name), + headers: { + 'Content-Type' => 'application/json', + 'Authorization' => "Basic #{auth}" + } + ) + + case result.code + when 201, 200 + existing_comments = JSON.parse(result.body)['comments'] + new_comment = new_comment['body'] + + if existing_comments.present? + return existing_comments.map { |comment| comment['body'].include?(new_comment) }.any? + end + end + + false + rescue JSON::ParserError + false + end + def server_url server = URI(project_url) default_ports = [80, 443].include?(server.port) @@ -213,7 +239,7 @@ class JiraService < IssueTrackerService "#{server_url}/rest/api/#{self.api_version}/issue/#{issue_name}/transitions" end - def add_comment_url(issue_name) + def comment_url(issue_name) "#{server_url}/rest/api/#{self.api_version}/issue/#{issue_name}/comment" end end From f72a81089f0921d7e3b3244e0d49a6fc30430be5 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Tue, 21 Apr 2015 16:51:27 +0200 Subject: [PATCH 2/3] Test existing comment query. Add logging when testing jira service settings. --- app/models/project_services/jira_service.rb | 39 ++++++++++-- spec/models/note_spec.rb | 50 +++++++++------- spec/services/git_push_service_spec.rb | 22 +++---- spec/support/jira_service_helper.rb | 66 +++++++++++++++++++++ 4 files changed, 139 insertions(+), 38 deletions(-) create mode 100644 spec/support/jira_service_helper.rb diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb index 1e63b484ae..1821827bd2 100644 --- a/app/models/project_services/jira_service.rb +++ b/app/models/project_services/jira_service.rb @@ -70,7 +70,13 @@ class JiraService < IssueTrackerService end def execute(push, issue = nil) - close_issue(push, issue) if issue + if issue.nil? + # No specific issue, that means + # we just want to test settings + test_settings + else + close_issue(push, issue) + end end def create_cross_reference_note(mentioned, noteable, author) @@ -103,6 +109,28 @@ class JiraService < IssueTrackerService add_comment(data, issue_name) end + def test_settings + result = JiraService.get( + jira_project_url, + headers: { + 'Content-Type' => 'application/json', + 'Authorization' => "Basic #{auth}" + } + ) + + case result.code + when 201, 200 + Rails.logger.info("#{self.class.name} SUCCESS #{result.code}: Sucessfully connected to #{server_url}.") + true + else + Rails.logger.info("#{self.class.name} ERROR #{result.code}: #{result.parsed_response}") + false + end + rescue Errno::ECONNREFUSED => e + Rails.logger.info "#{self.class.name} ERROR: #{e.message}. Hostname: #{server_url}." + false + end + private @@ -149,7 +177,7 @@ class JiraService < IssueTrackerService body: "[#{user_name}|#{user_url}] mentioned this issue in [a #{entity_name} of #{project_name}|#{entity_url}]." } - unless existing_comment?(issue_name, message) + unless existing_comment?(issue_name, message[:body]) send_message(url, message.to_json) end end @@ -181,7 +209,7 @@ class JiraService < IssueTrackerService Rails.logger.info(message) message - rescue URI::InvalidURIError => e + rescue URI::InvalidURIError, Errno::ECONNREFUSED => e Rails.logger.info "#{self.class.name} ERROR: #{e.message}. Hostname: #{url}." end @@ -197,7 +225,6 @@ class JiraService < IssueTrackerService case result.code when 201, 200 existing_comments = JSON.parse(result.body)['comments'] - new_comment = new_comment['body'] if existing_comments.present? return existing_comments.map { |comment| comment['body'].include?(new_comment) }.any? @@ -242,4 +269,8 @@ class JiraService < IssueTrackerService def comment_url(issue_name) "#{server_url}/rest/api/#{self.api_version}/issue/#{issue_name}/comment" end + + def jira_project_url + "#{server_url}/rest/api/#{self.api_version}/project" + end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 5608240c5a..b44e358149 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -20,6 +20,8 @@ require 'spec_helper' describe Note do + include JiraServiceHelper + describe "Associations" do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:noteable) } @@ -332,7 +334,6 @@ describe Note do let(:commit) { project.repository.commit } let(:jira_issue) { JiraIssue.new("JIRA-1", project)} let(:jira_tracker) { project.create_jira_service if project.jira_service.nil? } - let(:api_mention_url) { 'http://jira.example/rest/api/2/issue/JIRA-1/comment' } # Test all of {issue, merge request, commit} in both the referenced and referencing # roles, to ensure that the correct information can be inferred from any argument. @@ -368,7 +369,9 @@ describe Note do context 'in JIRA issue tracker' do before do jira_service_settings - WebMock.stub_request(:post, api_mention_url) + WebMock.stub_request(:post, jira_api_comment_url) + WebMock.stub_request(:get, jira_api_comment_url). + to_return(:body => jira_issue_comments) end after do @@ -402,16 +405,34 @@ describe Note do context 'in JIRA issue tracker' do before do jira_service_settings - WebMock.stub_request(:post, api_mention_url) + WebMock.stub_request(:post, jira_api_comment_url) end after do jira_tracker.destroy! end - subject { Note.create_cross_reference_note(jira_issue, commit, author, project) } + describe "new reference" do + before do + WebMock.stub_request(:get, jira_api_comment_url). + to_return(:body => jira_issue_comments) + end - it { is_expected.to eq(jira_status_message) } + subject { Note.create_cross_reference_note(jira_issue, commit, author, project) } + + it { is_expected.to eq(jira_status_message) } + end + + describe "existing reference" do + before do + message = "[#{author.name}|http://localhost/u/#{author.username}] mentioned this issue in [a commit of #{project.path_with_namespace}|http://localhost/#{project.path_with_namespace}/commit/#{commit.id}]." + WebMock.stub_request(:get, jira_api_comment_url). + to_return(:body => "{\"comments\":[{\"body\":\"#{message}\"}]}") + end + + subject { Note.create_cross_reference_note(jira_issue, commit, author, project) } + it { is_expected.not_to eq(jira_status_message) } + end end end @@ -419,7 +440,9 @@ describe Note do context 'in JIRA issue tracker' do before do jira_service_settings - WebMock.stub_request(:post, api_mention_url) + WebMock.stub_request(:post, jira_api_comment_url) + WebMock.stub_request(:get, jira_api_comment_url). + to_return(:body => jira_issue_comments) end after do @@ -690,19 +713,4 @@ describe Note do let(:backref_text) { issue.gfm_reference } let(:set_mentionable_text) { ->(txt) { subject.note = txt } } end - - def jira_service_settings - properties = { - "title"=>"JIRA tracker", - "project_url"=>"http://jira.example/issues/?jql=project=A", - "issues_url"=>"http://jira.example/browse/JIRA-1", - "new_issue_url"=>"http://jira.example/secure/CreateIssue.jspa" - } - - jira_tracker.update_attributes(properties: properties, active: true) - end - - def jira_status_message - "JiraService SUCCESS 200: Sucessfully posted to #{api_mention_url}." - end end diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 0e6f63c810..ba29498659 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe GitPushService do include RepoHelpers + include JiraServiceHelper let (:user) { create :user } let (:project) { create :project } @@ -237,21 +238,16 @@ describe GitPushService do end context "for jira issue tracker" do - let(:api_transition_url) { 'http://jira.example/rest/api/2/issue/JIRA-1/transitions' } - let(:api_mention_url) { 'http://jira.example/rest/api/2/issue/JIRA-1/comment' } let(:jira_tracker) { project.create_jira_service if project.jira_service.nil? } before do - properties = { - "title"=>"JIRA tracker", - "project_url"=>"http://jira.example/issues/?jql=project=A", - "issues_url"=>"http://jira.example/browse/JIRA-1", - "new_issue_url"=>"http://jira.example/secure/CreateIssue.jspa" - } - jira_tracker.update_attributes(properties: properties, active: true) + jira_service_settings - WebMock.stub_request(:post, api_transition_url) - WebMock.stub_request(:post, api_mention_url) + WebMock.stub_request(:post, jira_api_transition_url) + WebMock.stub_request(:post, jira_api_comment_url) + WebMock.stub_request(:get, jira_api_comment_url). + to_return(:body => jira_issue_comments) + WebMock.stub_request(:get, jira_api_project_url) closing_commit.stub({ issue_closing_regex: Regexp.new(Gitlab.config.gitlab.issue_closing_pattern), @@ -282,7 +278,7 @@ describe GitPushService do }.to_json service.execute(project, user, @oldrev, @newrev, @ref) - WebMock.should have_requested(:post, api_transition_url).with( + WebMock.should have_requested(:post, jira_api_transition_url).with( body: message ).once end @@ -290,7 +286,7 @@ describe GitPushService do it "should initiate one api call to jira server to mention the issue" do service.execute(project, user, @oldrev, @newrev, @ref) - WebMock.should have_requested(:post, api_mention_url).with( + WebMock.should have_requested(:post, jira_api_comment_url).with( body: /mentioned this issue in/ ).once end diff --git a/spec/support/jira_service_helper.rb b/spec/support/jira_service_helper.rb new file mode 100644 index 0000000000..d9ac9e04d2 --- /dev/null +++ b/spec/support/jira_service_helper.rb @@ -0,0 +1,66 @@ +module JiraServiceHelper + + def jira_service_settings + properties = { + "title"=>"JIRA tracker", + "project_url"=>"http://jira.example/issues/?jql=project=A", + "issues_url"=>"http://jira.example/browse/JIRA-1", + "new_issue_url"=>"http://jira.example/secure/CreateIssue.jspa" + } + + jira_tracker.update_attributes(properties: properties, active: true) + end + + def jira_status_message + "JiraService SUCCESS 200: Sucessfully posted to #{jira_api_comment_url}." + end + + def jira_issue_comments + "{\"startAt\":0,\"maxResults\":11,\"total\":11, + \"comments\":[{\"self\":\"http://0.0.0.0:4567/rest/api/2/issue/10002/comment/10609\", + \"id\":\"10609\",\"author\":{\"self\":\"http://0.0.0.0:4567/rest/api/2/user?username=gitlab\", + \"name\":\"gitlab\",\"emailAddress\":\"gitlab@example.com\", + \"avatarUrls\":{\"16x16\":\"http://0.0.0.0:4567/secure/useravatar?size=xsmall&avatarId=10122\", + \"24x24\":\"http://0.0.0.0:4567/secure/useravatar?size=small&avatarId=10122\", + \"32x32\":\"http://0.0.0.0:4567/secure/useravatar?size=medium&avatarId=10122\", + \"48x48\":\"http://0.0.0.0:4567/secure/useravatar?avatarId=10122\"}, + \"displayName\":\"GitLab\",\"active\":true}, + \"body\":\"[Administrator|http://localhost:3000/u/root] mentioned JIRA-1 in Merge request of [gitlab-org/gitlab-test|http://localhost:3000/gitlab-org/gitlab-test/merge_requests/2].\", + \"updateAuthor\":{\"self\":\"http://0.0.0.0:4567/rest/api/2/user?username=gitlab\",\"name\":\"gitlab\",\"emailAddress\":\"gitlab@example.com\", + \"avatarUrls\":{\"16x16\":\"http://0.0.0.0:4567/secure/useravatar?size=xsmall&avatarId=10122\", + \"24x24\":\"http://0.0.0.0:4567/secure/useravatar?size=small&avatarId=10122\", + \"32x32\":\"http://0.0.0.0:4567/secure/useravatar?size=medium&avatarId=10122\", + \"48x48\":\"http://0.0.0.0:4567/secure/useravatar?avatarId=10122\"},\"displayName\":\"GitLab\",\"active\":true}, + \"created\":\"2015-02-12T22:47:07.826+0100\", + \"updated\":\"2015-02-12T22:47:07.826+0100\"}, + {\"self\":\"http://0.0.0.0:4567/rest/api/2/issue/10002/comment/10700\", + \"id\":\"10700\",\"author\":{\"self\":\"http://0.0.0.0:4567/rest/api/2/user?username=gitlab\", + \"name\":\"gitlab\",\"emailAddress\":\"gitlab@example.com\", + \"avatarUrls\":{\"16x16\":\"http://0.0.0.0:4567/secure/useravatar?size=xsmall&avatarId=10122\", + \"24x24\":\"http://0.0.0.0:4567/secure/useravatar?size=small&avatarId=10122\", + \"32x32\":\"http://0.0.0.0:4567/secure/useravatar?size=medium&avatarId=10122\", + \"48x48\":\"http://0.0.0.0:4567/secure/useravatar?avatarId=10122\"},\"displayName\":\"GitLab\",\"active\":true}, + \"body\":\"[Administrator|http://localhost:3000/u/root] mentioned this issue in [a commit of h5bp/html5-boilerplate|http://localhost:3000/h5bp/html5-boilerplate/commit/2439f77897122fbeee3bfd9bb692d3608848433e].\", + \"updateAuthor\":{\"self\":\"http://0.0.0.0:4567/rest/api/2/user?username=gitlab\",\"name\":\"gitlab\",\"emailAddress\":\"gitlab@example.com\", + \"avatarUrls\":{\"16x16\":\"http://0.0.0.0:4567/secure/useravatar?size=xsmall&avatarId=10122\", + \"24x24\":\"http://0.0.0.0:4567/secure/useravatar?size=small&avatarId=10122\", + \"32x32\":\"http://0.0.0.0:4567/secure/useravatar?size=medium&avatarId=10122\", + \"48x48\":\"http://0.0.0.0:4567/secure/useravatar?avatarId=10122\"},\"displayName\":\"GitLab\",\"active\":true}, + \"created\":\"2015-04-01T03:45:55.667+0200\", + \"updated\":\"2015-04-01T03:45:55.667+0200\" + } + ]}" + end + + def jira_api_comment_url + 'http://jira.example/rest/api/2/issue/JIRA-1/comment' + end + + def jira_api_transition_url + 'http://jira.example/rest/api/2/issue/JIRA-1/transitions' + end + + def jira_api_project_url + 'http://jira.example/rest/api/2/project' + end +end From 8602374432e94c3159f5ea9e832da6b1f17b107b Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Fri, 24 Apr 2015 10:16:24 +0200 Subject: [PATCH 3/3] Update changelog-ee. --- CHANGELOG-EE | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG-EE b/CHANGELOG-EE index 57da8dc139..2fe29a7382 100644 --- a/CHANGELOG-EE +++ b/CHANGELOG-EE @@ -1,10 +1,13 @@ -v 7.10.0 (unreleased) +v 7.11.1 + - Check if comment exists in Jira before sending a reference + +v 7.10.0 - Improve UI for next pages: Group LDAP sync, Project git hooks, Project share with groups, Admin -> Appearance settigns - Default git hooks for new projects - Fix LDAP group links page by using new group members route. - Skip email confirmation when updated via LDAP. -v 7.9.0 (unreleased) +v 7.9.0 - Strip prefixes and suffixes from synced SSH keys: `SSHKey:ssh-rsa keykeykey` and `ssh-rsa keykeykey (SSH key)` will now work - Check if LDAP admin group exists before querying for user membership