Allow enabling and disabling commit and MR events for JIRAissue_5541

11 files changed, 92 insertions, 32 deletions

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{(?<issue>\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)