Commit b98193c5 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'issue_5541' into 'master'

Allow to block JIRA events for commits and merge requests

implements #5541 

See merge request !7469
parents 53714ddf d399e1ae
...@@ -17,6 +17,8 @@ module ServicesHelper ...@@ -17,6 +17,8 @@ module ServicesHelper
"Event will be triggered when a build status changes" "Event will be triggered when a build status changes"
when "wiki_page" when "wiki_page"
"Event will be triggered when a wiki page is created/updated" "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
end end
......
# == 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 class JiraService < IssueTrackerService
include Gitlab::Routing.url_helpers include Gitlab::Routing.url_helpers
...@@ -30,6 +9,10 @@ class JiraService < IssueTrackerService ...@@ -30,6 +9,10 @@ class JiraService < IssueTrackerService
before_update :reset_password before_update :reset_password
def supported_events
%w(commit merge_request)
end
# {PROJECT-KEY}-{NUMBER} Examples: JIRA-1, PROJECT-1 # {PROJECT-KEY}-{NUMBER} Examples: JIRA-1, PROJECT-1
def reference_pattern def reference_pattern
@reference_pattern ||= %r{(?<issue>\b([A-Z][A-Z0-9_]+-)\d+)} @reference_pattern ||= %r{(?<issue>\b([A-Z][A-Z0-9_]+-)\d+)}
...@@ -137,12 +120,16 @@ class JiraService < IssueTrackerService ...@@ -137,12 +120,16 @@ class JiraService < IssueTrackerService
end end
def create_cross_reference_note(mentioned, noteable, author) 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) } jira_issue = jira_request { client.Issue.find(mentioned.id) }
return false unless jira_issue.present? return unless jira_issue.present?
project = self.project 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 = if noteable.is_a?(Commit)
noteable.id noteable.id
else else
...@@ -193,8 +180,16 @@ class JiraService < IssueTrackerService ...@@ -193,8 +180,16 @@ class JiraService < IssueTrackerService
private 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) 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) commit_id = if entity.is_a?(Commit)
entity.id entity.id
......
...@@ -8,6 +8,7 @@ class Service < ActiveRecord::Base ...@@ -8,6 +8,7 @@ class Service < ActiveRecord::Base
default_value_for :push_events, true default_value_for :push_events, true
default_value_for :issues_events, true default_value_for :issues_events, true
default_value_for :confidential_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 :merge_requests_events, true
default_value_for :tag_push_events, true default_value_for :tag_push_events, true
default_value_for :note_events, true default_value_for :note_events, true
......
---
title: Allow enabling and disabling commit and MR events for JIRA
merge_request:
author:
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
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # 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 # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
...@@ -1031,6 +1031,7 @@ ActiveRecord::Schema.define(version: 20161117114805) do ...@@ -1031,6 +1031,7 @@ ActiveRecord::Schema.define(version: 20161117114805) do
t.boolean "wiki_page_events", default: true t.boolean "wiki_page_events", default: true
t.boolean "pipeline_events", default: false, null: false t.boolean "pipeline_events", default: false, null: false
t.boolean "confidential_issues_events", default: true, null: false t.boolean "confidential_issues_events", default: true, null: false
t.boolean "commit_events", default: true, null: false
end end
add_index "services", ["project_id"], name: "index_services_on_project_id", using: :btree add_index "services", ["project_id"], name: "index_services_on_project_id", using: :btree
......
...@@ -258,6 +258,7 @@ Service: ...@@ -258,6 +258,7 @@ Service:
- template - template
- push_events - push_events
- issues_events - issues_events
- commit_events
- merge_requests_events - merge_requests_events
- tag_push_events - tag_push_events
- note_events - note_events
...@@ -339,4 +340,4 @@ LabelPriority: ...@@ -339,4 +340,4 @@ LabelPriority:
- label_id - label_id
- priority - priority
- created_at - created_at
- updated_at - updated_at
\ No newline at end of file
...@@ -83,7 +83,8 @@ describe JiraService, models: true do ...@@ -83,7 +83,8 @@ describe JiraService, models: true do
url: 'http://jira.example.com', url: 'http://jira.example.com',
username: 'gitlab_jira_username', username: 'gitlab_jira_username',
password: 'gitlab_jira_password', 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. # These stubs are needed to test JiraService#close_issue.
...@@ -177,11 +178,10 @@ describe JiraService, models: true do ...@@ -177,11 +178,10 @@ describe JiraService, models: true do
end end
it "calls the api with jira_issue_transition_id" do 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)) @jira_service.execute(merge_request, ExternalIssue.new("JIRA-123", project))
expect(WebMock).to have_requested(:post, @transitions_url).with( expect(WebMock).to have_requested(:post, @transitions_url).with(
body: /this-is-a-custom-id/ body: /custom-id/
).once ).once
end end
......
...@@ -59,10 +59,14 @@ describe MergeRequests::MergeService, services: true do ...@@ -59,10 +59,14 @@ describe MergeRequests::MergeService, services: true do
include JiraServiceHelper include JiraServiceHelper
let(:jira_tracker) { project.create_jira_service } 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 before do
project.update_attributes!(has_external_issue_tracker: true) project.update_attributes!(has_external_issue_tracker: true)
jira_service_settings jira_service_settings
stub_jira_urls(jira_issue.id)
allow(merge_request).to receive(:commits).and_return([commit])
end end
it 'closes issues on JIRA issue tracker' do it 'closes issues on JIRA issue tracker' do
...@@ -76,6 +80,18 @@ describe MergeRequests::MergeService, services: true do ...@@ -76,6 +80,18 @@ describe MergeRequests::MergeService, services: true do
service.execute(merge_request) service.execute(merge_request)
end 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 context "wrong issue markdown" do
it 'does not close issues on JIRA issue tracker' do it 'does not close issues on JIRA issue tracker' do
jira_issue = ExternalIssue.new('#JIRA-123', project) jira_issue = ExternalIssue.new('#JIRA-123', project)
......
...@@ -536,7 +536,7 @@ describe SystemNoteService, services: true do ...@@ -536,7 +536,7 @@ describe SystemNoteService, services: true do
let(:project) { create(:jira_project) } let(:project) { create(:jira_project) }
let(:author) { create(:user) } let(:author) { create(:user) }
let(:issue) { create(:issue, project: project) } 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_issue) { ExternalIssue.new("JIRA-1", project)}
let(:jira_tracker) { project.jira_service } let(:jira_tracker) { project.jira_service }
let(:commit) { project.commit } let(:commit) { project.commit }
...@@ -545,7 +545,31 @@ describe SystemNoteService, services: true do ...@@ -545,7 +545,31 @@ describe SystemNoteService, services: true do
before { stub_jira_urls(jira_issue.id) } 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 } before { jira_service_settings }
describe "new reference" do describe "new reference" do
......
...@@ -6,7 +6,8 @@ module JiraServiceHelper ...@@ -6,7 +6,8 @@ module JiraServiceHelper
properties = { properties = {
title: "JIRA tracker", title: "JIRA tracker",
url: JIRA_URL, url: JIRA_URL,
project_key: "JIRA" project_key: "JIRA",
jira_issue_transition_id: '1'
} }
jira_tracker.update_attributes(properties: properties, active: true) jira_tracker.update_attributes(properties: properties, active: true)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment