From d399e1ae2b92bc399962a286fddc59eaa09670a7 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Mon, 14 Nov 2016 19:30:01 -0200 Subject: Allow enabling and disabling commit and MR events for JIRA --- app/helpers/services_helper.rb | 2 + app/models/project_services/jira_service.rb | 43 ++++++++++------------ app/models/service.rb | 1 + changelogs/unreleased/issue_5541.yml | 4 ++ ...20161118183841_add_commit_events_to_services.rb | 15 ++++++++ db/schema.rb | 3 +- .../gitlab/import_export/safe_model_attributes.yml | 3 +- spec/models/project_services/jira_service_spec.rb | 6 +-- spec/services/merge_requests/merge_service_spec.rb | 16 ++++++++ spec/services/system_note_service_spec.rb | 28 +++++++++++++- spec/support/jira_service_helper.rb | 3 +- 11 files changed, 92 insertions(+), 32 deletions(-) create mode 100644 changelogs/unreleased/issue_5541.yml create mode 100644 db/migrate/20161118183841_add_commit_events_to_services.rb diff --git a/app/helpers/services_helper.rb b/app/helpers/services_helper.rb index 3d4abf76419..9bab140e60a 100644 --- a/app/helpers/services_helper.rb +++ b/app/helpers/services_helper.rb @@ -17,6 +17,8 @@ module ServicesHelper "Event will be triggered when a build status changes" when "wiki_page" "Event will be triggered when a wiki page is created/updated" + when "commit" + "Event will be triggered when a commit is created/updated" end end diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb index 8915c06b633..2caf6179ef8 100644 --- a/app/models/project_services/jira_service.rb +++ b/app/models/project_services/jira_service.rb @@ -1,24 +1,3 @@ -# == Schema Information -# -# Table name: services -# -# id :integer not null, primary key -# type :string(255) -# title :string(255) -# project_id :integer -# created_at :datetime -# updated_at :datetime -# active :boolean default(FALSE), not null -# properties :text -# template :boolean default(FALSE) -# push_events :boolean default(TRUE) -# issues_events :boolean default(TRUE) -# merge_requests_events :boolean default(TRUE) -# tag_push_events :boolean default(TRUE) -# note_events :boolean default(TRUE), not null -# build_events :boolean default(FALSE), not null -# - class JiraService < IssueTrackerService include Gitlab::Routing.url_helpers @@ -30,6 +9,10 @@ class JiraService < IssueTrackerService before_update :reset_password + def supported_events + %w(commit merge_request) + end + # {PROJECT-KEY}-{NUMBER} Examples: JIRA-1, PROJECT-1 def reference_pattern @reference_pattern ||= %r{(?\b([A-Z][A-Z0-9_]+-)\d+)} @@ -137,12 +120,16 @@ class JiraService < IssueTrackerService end def create_cross_reference_note(mentioned, noteable, author) + unless can_cross_reference?(noteable) + return "Events for #{noteable.model_name.plural.humanize(capitalize: false)} are disabled." + end + jira_issue = jira_request { client.Issue.find(mentioned.id) } - return false unless jira_issue.present? + return unless jira_issue.present? project = self.project - noteable_name = noteable.class.name.underscore.downcase + noteable_name = noteable.model_name.singular noteable_id = if noteable.is_a?(Commit) noteable.id else @@ -193,8 +180,16 @@ class JiraService < IssueTrackerService private + def can_cross_reference?(noteable) + case noteable + when Commit then commit_events + when MergeRequest then merge_requests_events + else true + end + end + def close_issue(entity, issue) - return if issue.nil? || issue.resolution.present? + return if issue.nil? || issue.resolution.present? || !jira_issue_transition_id.present? commit_id = if entity.is_a?(Commit) entity.id diff --git a/app/models/service.rb b/app/models/service.rb index 9d6ff190cdf..e10805f5a1f 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -8,6 +8,7 @@ class Service < ActiveRecord::Base default_value_for :push_events, true default_value_for :issues_events, true default_value_for :confidential_issues_events, true + default_value_for :commit_events, true default_value_for :merge_requests_events, true default_value_for :tag_push_events, true default_value_for :note_events, true diff --git a/changelogs/unreleased/issue_5541.yml b/changelogs/unreleased/issue_5541.yml new file mode 100644 index 00000000000..cf553cf8d80 --- /dev/null +++ b/changelogs/unreleased/issue_5541.yml @@ -0,0 +1,4 @@ +--- +title: Allow enabling and disabling commit and MR events for JIRA +merge_request: +author: diff --git a/db/migrate/20161118183841_add_commit_events_to_services.rb b/db/migrate/20161118183841_add_commit_events_to_services.rb new file mode 100644 index 00000000000..4f9b5dd2281 --- /dev/null +++ b/db/migrate/20161118183841_add_commit_events_to_services.rb @@ -0,0 +1,15 @@ +class AddCommitEventsToServices < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default(:services, :commit_events, :boolean, default: true, allow_null: false) + end + + def down + remove_column(:services, :commit_events) + end +end diff --git a/db/schema.rb b/db/schema.rb index 19bd6b63acb..b098392581b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20161117114805) do +ActiveRecord::Schema.define(version: 20161118183841) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1023,6 +1023,7 @@ ActiveRecord::Schema.define(version: 20161117114805) do t.boolean "wiki_page_events", default: true t.boolean "pipeline_events", default: false, null: false t.boolean "confidential_issues_events", default: true, null: false + t.boolean "commit_events", default: true, null: false end add_index "services", ["project_id"], name: "index_services_on_project_id", using: :btree diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 07a2c316899..d6807941b31 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -258,6 +258,7 @@ Service: - template - push_events - issues_events +- commit_events - merge_requests_events - tag_push_events - note_events @@ -339,4 +340,4 @@ LabelPriority: - label_id - priority - created_at -- updated_at \ No newline at end of file +- updated_at diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index d8c47322220..f5da967cd14 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -83,7 +83,8 @@ describe JiraService, models: true do url: 'http://jira.example.com', username: 'gitlab_jira_username', password: 'gitlab_jira_password', - project_key: 'GitLabProject' + project_key: 'GitLabProject', + jira_issue_transition_id: "custom-id" ) # These stubs are needed to test JiraService#close_issue. @@ -177,11 +178,10 @@ describe JiraService, models: true do end it "calls the api with jira_issue_transition_id" do - @jira_service.jira_issue_transition_id = 'this-is-a-custom-id' @jira_service.execute(merge_request, ExternalIssue.new("JIRA-123", project)) expect(WebMock).to have_requested(:post, @transitions_url).with( - body: /this-is-a-custom-id/ + body: /custom-id/ ).once end diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index 1fd9f5a4910..7db32a33c93 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -59,10 +59,14 @@ describe MergeRequests::MergeService, services: true do include JiraServiceHelper let(:jira_tracker) { project.create_jira_service } + let(:jira_issue) { ExternalIssue.new('JIRA-123', project) } + let(:commit) { double('commit', safe_message: "Fixes #{jira_issue.to_reference}") } before do project.update_attributes!(has_external_issue_tracker: true) jira_service_settings + stub_jira_urls(jira_issue.id) + allow(merge_request).to receive(:commits).and_return([commit]) end it 'closes issues on JIRA issue tracker' do @@ -76,6 +80,18 @@ describe MergeRequests::MergeService, services: true do service.execute(merge_request) end + context "when jira_issue_transition_id is not present" do + before { allow_any_instance_of(JIRA::Resource::Issue).to receive(:resolution).and_return(nil) } + + it "does not close issue" do + allow(jira_tracker).to receive_messages(jira_issue_transition_id: nil) + + expect_any_instance_of(JiraService).not_to receive(:transition_issue) + + service.execute(merge_request) + end + end + context "wrong issue markdown" do it 'does not close issues on JIRA issue tracker' do jira_issue = ExternalIssue.new('#JIRA-123', project) diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 56d39e9a005..150e21574a1 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -536,7 +536,7 @@ describe SystemNoteService, services: true do let(:project) { create(:jira_project) } let(:author) { create(:user) } let(:issue) { create(:issue, project: project) } - let(:mergereq) { create(:merge_request, :simple, target_project: project, source_project: project) } + let(:merge_request) { create(:merge_request, :simple, target_project: project, source_project: project) } let(:jira_issue) { ExternalIssue.new("JIRA-1", project)} let(:jira_tracker) { project.jira_service } let(:commit) { project.commit } @@ -545,7 +545,31 @@ describe SystemNoteService, services: true do before { stub_jira_urls(jira_issue.id) } - context 'in issue' do + noteable_types = ["merge_requests", "commit"] + + noteable_types.each do |type| + context "when noteable is a #{type}" do + it "blocks cross reference when #{type.underscore}_events is false" do + jira_tracker.update("#{type}_events" => false) + + noteable = type == "commit" ? commit : merge_request + result = described_class.cross_reference(jira_issue, noteable, author) + + expect(result).to eq("Events for #{noteable.class.to_s.underscore.humanize.pluralize.downcase} are disabled.") + end + + it "blocks cross reference when #{type.underscore}_events is true" do + jira_tracker.update("#{type}_events" => true) + + noteable = type == "commit" ? commit : merge_request + result = described_class.cross_reference(jira_issue, noteable, author) + + expect(result).to eq(success_message) + end + end + end + + context 'in JIRA issue tracker' do before { jira_service_settings } describe "new reference" do diff --git a/spec/support/jira_service_helper.rb b/spec/support/jira_service_helper.rb index 7437ba2688d..929fc0c5182 100644 --- a/spec/support/jira_service_helper.rb +++ b/spec/support/jira_service_helper.rb @@ -6,7 +6,8 @@ module JiraServiceHelper properties = { title: "JIRA tracker", url: JIRA_URL, - project_key: "JIRA" + project_key: "JIRA", + jira_issue_transition_id: '1' } jira_tracker.update_attributes(properties: properties, active: true) -- cgit v1.2.1