From afe5d7d209a4088d71e35d6382e6523b89f94ebe Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Thu, 19 Feb 2015 05:02:57 +0000 Subject: [PATCH 01/12] Issue #595: Support Slack notifications upon issue and merge request events 1) Adds a DB migration for all services to toggle on push, issue, and merge events. 2) Upon an issue or merge request event, fire service hooks. 3) Slack service supports custom messages for each of these events. Other services not supported at the moment. 4) Label merge request hooks with their corresponding actions. --- CHANGELOG | 3 + .../projects/services_controller.rb | 3 +- app/models/project.rb | 5 +- app/models/project_services/asana_service.rb | 7 ++ .../project_services/assembla_service.rb | 11 +- app/models/project_services/bamboo_service.rb | 9 +- .../project_services/buildbox_service.rb | 7 ++ .../project_services/campfire_service.rb | 7 ++ app/models/project_services/ci_service.rb | 4 + .../emails_on_push_service.rb | 7 ++ .../project_services/flowdock_service.rb | 7 ++ .../project_services/gemnasium_service.rb | 7 ++ .../project_services/gitlab_ci_service.rb | 4 + .../gitlab_issue_tracker_service.rb | 4 + .../project_services/hipchat_service.rb | 7 ++ .../project_services/issue_tracker_service.rb | 7 ++ app/models/project_services/jira_service.rb | 4 + .../pivotaltracker_service.rb | 7 ++ .../project_services/pushover_service.rb | 7 ++ .../project_services/redmine_service.rb | 4 + app/models/project_services/slack_message.rb | 110 ------------------ .../slack_messages/slack_base_message.rb | 31 +++++ .../slack_messages/slack_issue_message.rb | 56 +++++++++ .../slack_messages/slack_merge_message.rb | 54 +++++++++ .../slack_messages/slack_push_message.rb | 110 ++++++++++++++++++ app/models/project_services/slack_service.rb | 38 +++++- .../project_services/teamcity_service.rb | 11 +- app/models/service.rb | 14 +++ app/services/git_push_service.rb | 2 +- app/services/issues/base_service.rb | 12 +- app/services/merge_requests/base_service.rb | 16 ++- app/views/projects/services/_form.html.haml | 32 +++++ .../20150219004514_add_events_to_services.rb | 8 ++ db/schema.rb | 8 +- lib/gitlab/push_data_builder.rb | 1 + .../project_services/assembla_service_spec.rb | 20 ++-- .../project_services/buildbox_service_spec.rb | 20 ++-- .../project_services/flowdock_service_spec.rb | 20 ++-- .../gemnasium_service_spec.rb | 20 ++-- .../gitlab_ci_service_spec.rb | 20 ++-- .../project_services/pushover_service_spec.rb | 20 ++-- .../slack_issue_message_spec.rb | 55 +++++++++ .../slack_merge_message_spec.rb | 50 ++++++++ .../slack_push_message_spec.rb} | 4 +- .../project_services/slack_service_spec.rb | 59 ++++++++-- spec/models/service_spec.rb | 4 + spec/services/git_push_service_spec.rb | 1 + 47 files changed, 722 insertions(+), 195 deletions(-) delete mode 100644 app/models/project_services/slack_message.rb create mode 100644 app/models/project_services/slack_messages/slack_base_message.rb create mode 100644 app/models/project_services/slack_messages/slack_issue_message.rb create mode 100644 app/models/project_services/slack_messages/slack_merge_message.rb create mode 100644 app/models/project_services/slack_messages/slack_push_message.rb create mode 100644 db/migrate/20150219004514_add_events_to_services.rb create mode 100644 spec/models/project_services/slack_messages/slack_issue_message_spec.rb create mode 100644 spec/models/project_services/slack_messages/slack_merge_message_spec.rb rename spec/models/project_services/{slack_message_spec.rb => slack_messages/slack_push_message_spec.rb} (94%) diff --git a/CHANGELOG b/CHANGELOG index 6a28772097e..8011817d0ae 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,4 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. +v 7.9.0 (unreleased) + - Added issue and merge request events to Slack service (Stan Hu) + - Fix broken access control for note attachments (Hannes Rosenögger) v 7.9.0 (unreleased) - Move labels/milestones tabs to sidebar diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb index 82aad329c1a..087579de106 100644 --- a/app/controllers/projects/services_controller.rb +++ b/app/controllers/projects/services_controller.rb @@ -51,7 +51,8 @@ class Projects::ServicesController < Projects::ApplicationController :user_key, :device, :priority, :sound, :bamboo_url, :username, :password, :build_key, :server, :teamcity_url, :build_type, :description, :issues_url, :new_issue_url, :restrict_to_branch, :channel, - :colorize_messages, :channels + :colorize_messages, :channels, + :push_events, :issues_events, :merge_requests_events, :tag_push_events ) end end diff --git a/app/models/project.rb b/app/models/project.rb index 907f331d8f1..c45338bf4eb 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -479,8 +479,9 @@ class Project < ActiveRecord::Base end end - def execute_services(data) - services.select(&:active).each do |service| + def execute_services(data, hooks_scope = :push_hooks) + # Call only service hooks that are active for this scope + services.send(hooks_scope).each do |service| service.async_execute(data) end end diff --git a/app/models/project_services/asana_service.rb b/app/models/project_services/asana_service.rb index 66b72572b9c..2b530390aeb 100644 --- a/app/models/project_services/asana_service.rb +++ b/app/models/project_services/asana_service.rb @@ -11,6 +11,10 @@ # 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) # require 'asana' @@ -62,6 +66,9 @@ automatically inspected. Leave blank to include all branches.' end def execute(push) + object_kind = push[:object_kind] + return unless object_kind == "push" + Asana.configure do |client| client.api_key = api_key end diff --git a/app/models/project_services/assembla_service.rb b/app/models/project_services/assembla_service.rb index cf7598f35eb..01c647c1705 100644 --- a/app/models/project_services/assembla_service.rb +++ b/app/models/project_services/assembla_service.rb @@ -11,6 +11,10 @@ # 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) # class AssemblaService < Service @@ -38,8 +42,11 @@ class AssemblaService < Service ] end - def execute(push) + def execute(data) + object_kind = data[:object_kind] + return unless object_kind == "push" + url = "https://atlas.assembla.com/spaces/#{subdomain}/github_tool?secret_key=#{token}" - AssemblaService.post(url, body: { payload: push }.to_json, headers: { 'Content-Type' => 'application/json' }) + AssemblaService.post(url, body: { payload: data }.to_json, headers: { 'Content-Type' => 'application/json' }) end end diff --git a/app/models/project_services/bamboo_service.rb b/app/models/project_services/bamboo_service.rb index df68803152f..6ff52af040d 100644 --- a/app/models/project_services/bamboo_service.rb +++ b/app/models/project_services/bamboo_service.rb @@ -11,6 +11,10 @@ # 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) # class BambooService < CiService @@ -118,7 +122,10 @@ class BambooService < CiService end end - def execute(_data) + def execute(data) + object_kind = data[:object_kind] + return unless object_kind == "push" + # Bamboo requires a GET and does not take any data. self.class.get("#{bamboo_url}/updateAndBuild.action?buildKey=#{build_key}", verify: false) diff --git a/app/models/project_services/buildbox_service.rb b/app/models/project_services/buildbox_service.rb index 058c890ae45..201bfc560a3 100644 --- a/app/models/project_services/buildbox_service.rb +++ b/app/models/project_services/buildbox_service.rb @@ -11,6 +11,10 @@ # 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) # require "addressable/uri" @@ -33,6 +37,9 @@ class BuildboxService < CiService end def execute(data) + object_kind = data[:object_kind] + return unless object_kind == "push" + service_hook.execute(data) end diff --git a/app/models/project_services/campfire_service.rb b/app/models/project_services/campfire_service.rb index 14b6b87a0b7..41ab6c56ad8 100644 --- a/app/models/project_services/campfire_service.rb +++ b/app/models/project_services/campfire_service.rb @@ -11,6 +11,10 @@ # 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) # class CampfireService < Service @@ -38,6 +42,9 @@ class CampfireService < Service end def execute(push_data) + object_kind = push_data[:object_kind] + return unless object_kind == "push" + room = gate.find_room_by_name(self.room) return true unless room diff --git a/app/models/project_services/ci_service.rb b/app/models/project_services/ci_service.rb index 5a26c25b3c3..e58d6d7a233 100644 --- a/app/models/project_services/ci_service.rb +++ b/app/models/project_services/ci_service.rb @@ -11,6 +11,10 @@ # active :boolean default(FALSE), not null # properties :text # template :boolean default(FALSE) +# push_events :boolean +# issues_events :boolean +# merge_requests_events :boolean +# tag_push_events :boolean # # Base class for CI services diff --git a/app/models/project_services/emails_on_push_service.rb b/app/models/project_services/emails_on_push_service.rb index 86693ad0c7e..28be15c3b35 100644 --- a/app/models/project_services/emails_on_push_service.rb +++ b/app/models/project_services/emails_on_push_service.rb @@ -11,6 +11,10 @@ # active :boolean default(FALSE), not null # properties :text # template :boolean default(FALSE) +# push_events :boolean +# issues_events :boolean +# merge_requests_events :boolean +# tag_push_events :boolean # class EmailsOnPushService < Service @@ -30,6 +34,9 @@ class EmailsOnPushService < Service end def execute(push_data) + object_kind = push_data[:object_kind] + return unless object_kind == "push" + EmailsOnPushWorker.perform_async(project_id, recipients, push_data) end diff --git a/app/models/project_services/flowdock_service.rb b/app/models/project_services/flowdock_service.rb index 13e2dfceb1a..9cc0e367882 100644 --- a/app/models/project_services/flowdock_service.rb +++ b/app/models/project_services/flowdock_service.rb @@ -11,6 +11,10 @@ # active :boolean default(FALSE), not null # properties :text # template :boolean default(FALSE) +# push_events :boolean +# issues_events :boolean +# merge_requests_events :boolean +# tag_push_events :boolean # require "flowdock-git-hook" @@ -38,6 +42,9 @@ class FlowdockService < Service end def execute(push_data) + object_kind = push_data[:object_kind] + return unless object_kind == "push" + Flowdock::Git.post( push_data[:ref], push_data[:before], diff --git a/app/models/project_services/gemnasium_service.rb b/app/models/project_services/gemnasium_service.rb index a2c87ae88f1..130c9eaeb4b 100644 --- a/app/models/project_services/gemnasium_service.rb +++ b/app/models/project_services/gemnasium_service.rb @@ -11,6 +11,10 @@ # active :boolean default(FALSE), not null # properties :text # template :boolean default(FALSE) +# push_events :boolean +# issues_events :boolean +# merge_requests_events :boolean +# tag_push_events :boolean # require "gemnasium/gitlab_service" @@ -39,6 +43,9 @@ class GemnasiumService < Service end def execute(push_data) + object_kind = push_data[:object_kind] + return unless object_kind == "push" + Gemnasium::GitlabService.execute( ref: push_data[:ref], before: push_data[:before], diff --git a/app/models/project_services/gitlab_ci_service.rb b/app/models/project_services/gitlab_ci_service.rb index f4b463e8199..a64b24b5ef1 100644 --- a/app/models/project_services/gitlab_ci_service.rb +++ b/app/models/project_services/gitlab_ci_service.rb @@ -11,6 +11,10 @@ # active :boolean default(FALSE), not null # properties :text # template :boolean default(FALSE) +# push_events :boolean +# issues_events :boolean +# merge_requests_events :boolean +# tag_push_events :boolean # class GitlabCiService < CiService diff --git a/app/models/project_services/gitlab_issue_tracker_service.rb b/app/models/project_services/gitlab_issue_tracker_service.rb index 05c048e4e45..00f8d430fd5 100644 --- a/app/models/project_services/gitlab_issue_tracker_service.rb +++ b/app/models/project_services/gitlab_issue_tracker_service.rb @@ -11,6 +11,10 @@ # 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) # class GitlabIssueTrackerService < IssueTrackerService diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb index 003e06a4c80..462478812a1 100644 --- a/app/models/project_services/hipchat_service.rb +++ b/app/models/project_services/hipchat_service.rb @@ -11,6 +11,10 @@ # active :boolean default(FALSE), not null # properties :text # template :boolean default(FALSE) +# push_events :boolean +# issues_events :boolean +# merge_requests_events :boolean +# tag_push_events :boolean # class HipchatService < Service @@ -41,6 +45,9 @@ class HipchatService < Service end def execute(push_data) + object_kind = push_data.fetch(:object_kind) + return unless object_kind == "push" + gate[room].send('GitLab', create_message(push_data)) end diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb index c991a34ecdb..0d9e5c13992 100644 --- a/app/models/project_services/issue_tracker_service.rb +++ b/app/models/project_services/issue_tracker_service.rb @@ -11,6 +11,10 @@ # 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) # class IssueTrackerService < Service @@ -66,6 +70,9 @@ class IssueTrackerService < Service end def execute(data) + object_kind = data[:object_kind] + return unless object_kind == "push" + message = "#{self.type} was unable to reach #{self.project_url}. Check the url and try again." result = false diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb index 4c056605ea8..20611eeb60c 100644 --- a/app/models/project_services/jira_service.rb +++ b/app/models/project_services/jira_service.rb @@ -11,6 +11,10 @@ # 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) # class JiraService < IssueTrackerService diff --git a/app/models/project_services/pivotaltracker_service.rb b/app/models/project_services/pivotaltracker_service.rb index 287812c57a5..4bb2a978ed1 100644 --- a/app/models/project_services/pivotaltracker_service.rb +++ b/app/models/project_services/pivotaltracker_service.rb @@ -11,6 +11,10 @@ # 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) # class PivotaltrackerService < Service @@ -38,6 +42,9 @@ class PivotaltrackerService < Service end def execute(push) + object_kind = push[:object_kind] + return unless object_kind == "push" + url = 'https://www.pivotaltracker.com/services/v5/source_commits' push[:commits].each do |commit| message = { diff --git a/app/models/project_services/pushover_service.rb b/app/models/project_services/pushover_service.rb index 3a3af59390a..4aa7e0afa77 100644 --- a/app/models/project_services/pushover_service.rb +++ b/app/models/project_services/pushover_service.rb @@ -11,6 +11,10 @@ # 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) # class PushoverService < Service @@ -77,6 +81,9 @@ class PushoverService < Service end def execute(push_data) + object_kind = push_data[:object_kind] + return unless object_kind == "push" + ref = push_data[:ref].gsub('refs/heads/', '') before = push_data[:before] after = push_data[:after] diff --git a/app/models/project_services/redmine_service.rb b/app/models/project_services/redmine_service.rb index e1dc10415e0..f96eae2daa1 100644 --- a/app/models/project_services/redmine_service.rb +++ b/app/models/project_services/redmine_service.rb @@ -11,6 +11,10 @@ # 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) # class RedmineService < IssueTrackerService diff --git a/app/models/project_services/slack_message.rb b/app/models/project_services/slack_message.rb deleted file mode 100644 index 6c6446db45f..00000000000 --- a/app/models/project_services/slack_message.rb +++ /dev/null @@ -1,110 +0,0 @@ -require 'slack-notifier' - -class SlackMessage - attr_reader :after - attr_reader :before - attr_reader :commits - attr_reader :project_name - attr_reader :project_url - attr_reader :ref - attr_reader :username - - def initialize(params) - @after = params.fetch(:after) - @before = params.fetch(:before) - @commits = params.fetch(:commits, []) - @project_name = params.fetch(:project_name) - @project_url = params.fetch(:project_url) - @ref = params.fetch(:ref).gsub('refs/heads/', '') - @username = params.fetch(:user_name) - end - - def pretext - format(message) - end - - def attachments - return [] if new_branch? || removed_branch? - - commit_message_attachments - end - - private - - def message - if new_branch? - new_branch_message - elsif removed_branch? - removed_branch_message - else - push_message - end - end - - def format(string) - Slack::Notifier::LinkFormatter.format(string) - end - - def new_branch_message - "#{username} pushed new branch #{branch_link} to #{project_link}" - end - - def removed_branch_message - "#{username} removed branch #{ref} from #{project_link}" - end - - def push_message - "#{username} pushed to branch #{branch_link} of #{project_link} (#{compare_link})" - end - - def commit_messages - commits.each_with_object('') do |commit, str| - str << compose_commit_message(commit) - end.chomp - end - - def commit_message_attachments - [{ text: format(commit_messages), color: attachment_color }] - end - - def compose_commit_message(commit) - author = commit.fetch(:author).fetch(:name) - id = commit.fetch(:id)[0..8] - message = commit.fetch(:message) - url = commit.fetch(:url) - - "[#{id}](#{url}): #{message} - #{author}\n" - end - - def new_branch? - before.include?('000000') - end - - def removed_branch? - after.include?('000000') - end - - def branch_url - "#{project_url}/commits/#{ref}" - end - - def compare_url - "#{project_url}/compare/#{before}...#{after}" - end - - def branch_link - "[#{ref}](#{branch_url})" - end - - def project_link - "[#{project_name}](#{project_url})" - end - - def compare_link - "[Compare changes](#{compare_url})" - end - - def attachment_color - '#345' - end -end diff --git a/app/models/project_services/slack_messages/slack_base_message.rb b/app/models/project_services/slack_messages/slack_base_message.rb new file mode 100644 index 00000000000..c2fc27884bf --- /dev/null +++ b/app/models/project_services/slack_messages/slack_base_message.rb @@ -0,0 +1,31 @@ +require 'slack-notifier' + +module SlackMessages + class SlackBaseMessage + def initialize(params) + raise NotImplementedError + end + + def pretext + format(message) + end + + def attachments + raise NotImplementedError + end + + private + + def message + raise NotImplementedError + end + + def format(string) + Slack::Notifier::LinkFormatter.format(string) + end + + def attachment_color + '#345' + end + end +end diff --git a/app/models/project_services/slack_messages/slack_issue_message.rb b/app/models/project_services/slack_messages/slack_issue_message.rb new file mode 100644 index 00000000000..0c3a492aae7 --- /dev/null +++ b/app/models/project_services/slack_messages/slack_issue_message.rb @@ -0,0 +1,56 @@ +module SlackMessages + class SlackIssueMessage < SlackBaseMessage + attr_reader :username + attr_reader :title + attr_reader :project_name + attr_reader :project_url + attr_reader :issue_iid + attr_reader :issue_url + attr_reader :action + attr_reader :state + attr_reader :description + + def initialize(params) + @username = params[:user][:username] + @project_name = params[:project_name] + @project_url = params[:project_url] + + obj_attr = params[:object_attributes] + obj_attr = HashWithIndifferentAccess.new(obj_attr) + @title = obj_attr[:title] + @issue_iid = obj_attr[:iid] + @issue_url = obj_attr[:url] + @action = obj_attr[:action] + @state = obj_attr[:state] + @description = obj_attr[:description] + end + + def attachments + return [] unless opened_issue? + + description_message + end + + private + + def message + "#{username} #{state} issue #{issue_link} in #{project_link}: #{title}" + end + + def opened_issue? + action == "open" + end + + def description_message + [{ text: format(description), color: attachment_color }] + end + + def project_link + "[#{project_name}](#{project_url})" + end + + def issue_link + "[##{issue_iid}](#{issue_url})" + end + end +end diff --git a/app/models/project_services/slack_messages/slack_merge_message.rb b/app/models/project_services/slack_messages/slack_merge_message.rb new file mode 100644 index 00000000000..bc49a963a9b --- /dev/null +++ b/app/models/project_services/slack_messages/slack_merge_message.rb @@ -0,0 +1,54 @@ +module SlackMessages + class SlackMergeMessage < SlackBaseMessage + attr_reader :username + attr_reader :project_name + attr_reader :project_url + attr_reader :merge_request_id + attr_reader :source_branch + attr_reader :target_branch + attr_reader :state + + def initialize(params) + @username = params[:user][:username] + @project_name = params[:project_name] + @project_url = params[:project_url] + + obj_attr = params[:object_attributes] + obj_attr = HashWithIndifferentAccess.new(obj_attr) + @merge_request_id = obj_attr[:iid] + @source_branch = obj_attr[:source_branch] + @target_branch = obj_attr[:target_branch] + @state = obj_attr[:state] + end + + def pretext + format(message) + end + + def attachments + [] + end + + private + + def message + merge_request_message + end + + def project_link + "[#{project_name}](#{project_url})" + end + + def merge_request_message + "#{username} #{state} merge request #{merge_request_link} in #{project_link}" + end + + def merge_request_link + "[##{merge_request_id}](#{merge_request_url})" + end + + def merge_request_url + "#{project_url}/merge_requests/#{merge_request_id}" + end + end +end diff --git a/app/models/project_services/slack_messages/slack_push_message.rb b/app/models/project_services/slack_messages/slack_push_message.rb new file mode 100644 index 00000000000..c7769bbeda1 --- /dev/null +++ b/app/models/project_services/slack_messages/slack_push_message.rb @@ -0,0 +1,110 @@ +require 'slack-notifier' + +module SlackMessages + class SlackPushMessage < SlackBaseMessage + attr_reader :after + attr_reader :before + attr_reader :commits + attr_reader :project_name + attr_reader :project_url + attr_reader :ref + attr_reader :username + + def initialize(params) + @after = params[:after] + @before = params[:before] + @commits = params.fetch(:commits, []) + @project_name = params[:project_name] + @project_url = params[:project_url] + @ref = params[:ref].gsub('refs/heads/', '') + @username = params[:user_name] + end + + def pretext + format(message) + end + + def attachments + return [] if new_branch? || removed_branch? + + commit_message_attachments + end + + private + + def message + if new_branch? + new_branch_message + elsif removed_branch? + removed_branch_message + else + push_message + end + end + + def format(string) + Slack::Notifier::LinkFormatter.format(string) + end + + def new_branch_message + "#{username} pushed new branch #{branch_link} to #{project_link}" + end + + def removed_branch_message + "#{username} removed branch #{ref} from #{project_link}" + end + + def push_message + "#{username} pushed to branch #{branch_link} of #{project_link} (#{compare_link})" + end + + def commit_messages + commits.map { |commit| compose_commit_message(commit) }.join("\n") + end + + def commit_message_attachments + [{ text: format(commit_messages), color: attachment_color }] + end + + def compose_commit_message(commit) + author = commit[:author][:name] + id = Commit.truncate_sha(commit[:id]) + message = commit[:message] + url = commit[:url] + + "[#{id}](#{url}): #{message} - #{author}" + end + + def new_branch? + before.include?('000000') + end + + def removed_branch? + after.include?('000000') + end + + def branch_url + "#{project_url}/commits/#{ref}" + end + + def compare_url + "#{project_url}/compare/#{before}...#{after}" + end + + def branch_link + "[#{ref}](#{branch_url})" + end + + def project_link + "[#{project_name}](#{project_url})" + end + + def compare_link + "[Compare changes](#{compare_url})" + end + + def attachment_color + '#345' + end + end +end diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb index c7cbff63fe5..1318a1ed1b8 100644 --- a/app/models/project_services/slack_service.rb +++ b/app/models/project_services/slack_service.rb @@ -11,7 +11,14 @@ # 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) # +require "slack_messages/slack_issue_message" +require "slack_messages/slack_push_message" +require "slack_messages/slack_merge_message" class SlackService < Service prop_accessor :webhook, :username, :channel @@ -38,20 +45,37 @@ class SlackService < Service ] end - def execute(push_data) + def execute(data) return unless webhook.present? - message = SlackMessage.new(push_data.merge( + object_kind = data[:object_kind] + + data = data.merge( project_url: project_url, project_name: project_name - )) + ) + + # WebHook events often have an 'update' event that follows a 'open' or + # 'close' action. Ignore update events for now to prevent duplicate + # messages from arriving. + + message = case object_kind + when "push" + message = SlackMessages::SlackPushMessage.new(data) + when "issue" + message = SlackMessages::SlackIssueMessage.new(data) unless is_update?(data) + when "merge_request" + message = SlackMessages::SlackMergeMessage.new(data) unless is_update?(data) + end opt = {} opt[:channel] = channel if channel opt[:username] = username if username - notifier = Slack::Notifier.new(webhook, opt) - notifier.ping(message.pretext, attachments: message.attachments) + if message + notifier = Slack::Notifier.new(webhook, opt) + notifier.ping(message.pretext, attachments: message.attachments) + end end private @@ -63,4 +87,8 @@ class SlackService < Service def project_url project.web_url end + + def is_update?(data) + data[:object_attributes][:action] == 'update' + end end diff --git a/app/models/project_services/teamcity_service.rb b/app/models/project_services/teamcity_service.rb index b6932f1c77b..07facfb6d06 100644 --- a/app/models/project_services/teamcity_service.rb +++ b/app/models/project_services/teamcity_service.rb @@ -11,6 +11,10 @@ # 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) # class TeamcityService < CiService @@ -115,13 +119,16 @@ class TeamcityService < CiService end end - def execute(push) + def execute(data) + object_kind = data[:object_kind] + return unless object_kind == "push" + auth = { username: username, password: password, } - branch = push[:ref].gsub('refs/heads/', '') + branch = data[:ref].gsub('refs/heads/', '') self.class.post("#{teamcity_url}/httpAuth/app/rest/buildQueue", body: ""\ diff --git a/app/models/service.rb b/app/models/service.rb index f4e97da3212..9d6866f26d0 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -11,6 +11,11 @@ # active :boolean default(FALSE), not null # properties :text # template :boolean default(FALSE) +# push_events :boolean +# issues_events :boolean +# merge_requests_events :boolean +# tag_push_events :boolean +# # To add new service you should build a class inherited from Service # and implement a set of methods @@ -19,6 +24,10 @@ class Service < ActiveRecord::Base serialize :properties, JSON default_value_for :active, false + default_value_for :push_events, true + default_value_for :issues_events, true + default_value_for :merge_requests_events, true + default_value_for :tag_push_events, true after_initialize :initialize_properties @@ -29,6 +38,11 @@ class Service < ActiveRecord::Base scope :visible, -> { where.not(type: 'GitlabIssueTrackerService') } + scope :push_hooks, -> { where(push_events: true, active: true) } + scope :tag_push_hooks, -> { where(tag_push_events: true, active: true) } + scope :issue_hooks, -> { where(issues_events: true, active: true) } + scope :merge_request_hooks, -> { where(merge_requests_events: true, active: true) } + def activated? active end diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index f21e6ac207d..13def127763 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -54,7 +54,7 @@ class GitPushService @push_data = post_receive_data(oldrev, newrev, ref) EventCreateService.new.push(project, user, @push_data) project.execute_hooks(@push_data.dup, :push_hooks) - project.execute_services(@push_data.dup) + project.execute_services(@push_data.dup, :push_hooks) end end diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index 755c0ef45a8..c3ca04a4343 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -1,13 +1,19 @@ module Issues class BaseService < ::IssuableBaseService + def hook_data(issue, action) + issue_data = issue.to_hook_data(current_user) + issue_url = Gitlab::UrlBuilder.new(:issue).build(issue.id) + issue_data[:object_attributes].merge!(url: issue_url, action: action) + issue_data + end + private def execute_hooks(issue, action = 'open') - issue_data = issue.to_hook_data(current_user) - issue_url = Gitlab::UrlBuilder.new(:issue).build(issue.id) - issue_data[:object_attributes].merge!(url: issue_url, action: action) + issue_data = hook_data(issue, action) issue.project.execute_hooks(issue_data, :issue_hooks) + issue.project.execute_services(issue_data, :issue_hooks) end end end diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index b4199d1c800..f6e1ae6f283 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -5,13 +5,19 @@ module MergeRequests Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state, nil) end + def hook_data(merge_request, action) + hook_data = merge_request.to_hook_data(current_user) + merge_request_url = Gitlab::UrlBuilder.new(:merge_request).build(merge_request.id) + hook_data[:object_attributes][:url] = merge_request_url + hook_data[:object_attributes][:action] = action + hook_data + end + def execute_hooks(merge_request, action = 'open') if merge_request.project - hook_data = merge_request.to_hook_data(current_user) - merge_request_url = Gitlab::UrlBuilder.new(:merge_request).build(merge_request.id) - hook_data[:object_attributes][:url] = merge_request_url - hook_data[:object_attributes][:action] = action - merge_request.project.execute_hooks(hook_data, :merge_request_hooks) + merge_data = hook_data(merge_request, action) + merge_request.project.execute_hooks(merge_data, :merge_request_hooks) + merge_request.project.execute_services(merge_data, :merge_request_hooks) end end end diff --git a/app/views/projects/services/_form.html.haml b/app/views/projects/services/_form.html.haml index 8db6d67e06b..0519c8150e9 100644 --- a/app/views/projects/services/_form.html.haml +++ b/app/views/projects/services/_form.html.haml @@ -27,6 +27,38 @@ .col-sm-10 = f.check_box :active + .form-group + = f.label :url, "Trigger", class: 'control-label' + .col-sm-10 + %div + = f.check_box :push_events, class: 'pull-left' + .prepend-left-20 + = f.label :push_events, class: 'list-label' do + %strong Push events + %p.light + This url will be triggered by a push to the repository + %div + = f.check_box :tag_push_events, class: 'pull-left' + .prepend-left-20 + = f.label :tag_push_events, class: 'list-label' do + %strong Tag push events + %p.light + This url will be triggered when a new tag is pushed to the repository + %div + = f.check_box :issues_events, class: 'pull-left' + .prepend-left-20 + = f.label :issues_events, class: 'list-label' do + %strong Issues events + %p.light + This url will be triggered when an issue is created + %div + = f.check_box :merge_requests_events, class: 'pull-left' + .prepend-left-20 + = f.label :merge_requests_events, class: 'list-label' do + %strong Merge Request events + %p.light + This url will be triggered when a merge request is created + - @service.fields.each do |field| - name = field[:name] - value = @service.send(name) unless field[:type] == 'password' diff --git a/db/migrate/20150219004514_add_events_to_services.rb b/db/migrate/20150219004514_add_events_to_services.rb new file mode 100644 index 00000000000..cf73a0174f4 --- /dev/null +++ b/db/migrate/20150219004514_add_events_to_services.rb @@ -0,0 +1,8 @@ +class AddEventsToServices < ActiveRecord::Migration + def change + add_column :services, :push_events, :boolean, :default => true + add_column :services, :issues_events, :boolean, :default => true + add_column :services, :merge_requests_events, :boolean, :default => true + add_column :services, :tag_push_events, :boolean, :default => true + end +end diff --git a/db/schema.rb b/db/schema.rb index 2659efe4df9..1a9b512e159 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -364,9 +364,13 @@ ActiveRecord::Schema.define(version: 20150223022001) do t.integer "project_id" t.datetime "created_at" t.datetime "updated_at" - t.boolean "active", default: false, null: false + t.boolean "active", default: false, null: false t.text "properties" - t.boolean "template", default: false + t.boolean "template", default: false + t.boolean "push_events", default: true + t.boolean "issues_events", default: true + t.boolean "merge_requests_events", default: true + t.boolean "tag_push_events", default: true end add_index "services", ["created_at", "id"], name: "index_services_on_created_at_and_id", using: :btree diff --git a/lib/gitlab/push_data_builder.rb b/lib/gitlab/push_data_builder.rb index 9aa5c8967a7..9d8d3ea3d22 100644 --- a/lib/gitlab/push_data_builder.rb +++ b/lib/gitlab/push_data_builder.rb @@ -29,6 +29,7 @@ module Gitlab # Hash to be passed as post_receive_data data = { + object_kind: "push", before: oldrev, after: newrev, ref: ref, diff --git a/spec/models/project_services/assembla_service_spec.rb b/spec/models/project_services/assembla_service_spec.rb index ee7f780c8f6..cd34e006ebe 100644 --- a/spec/models/project_services/assembla_service_spec.rb +++ b/spec/models/project_services/assembla_service_spec.rb @@ -2,14 +2,18 @@ # # Table name: services # -# id :integer not null, primary key -# type :string(255) -# title :string(255) -# project_id :integer not null -# created_at :datetime -# updated_at :datetime -# active :boolean default(FALSE), not null -# properties :text +# id :integer not null, primary key +# type :string(255) +# title :string(255) +# project_id :integer not null +# created_at :datetime +# updated_at :datetime +# active :boolean default(FALSE), not null +# properties :text +# push_events :boolean +# issues_events :boolean +# merge_requests_events :boolean +# tag_push_events :boolean # require 'spec_helper' diff --git a/spec/models/project_services/buildbox_service_spec.rb b/spec/models/project_services/buildbox_service_spec.rb index 050363e14c7..c246e1c9d41 100644 --- a/spec/models/project_services/buildbox_service_spec.rb +++ b/spec/models/project_services/buildbox_service_spec.rb @@ -2,14 +2,18 @@ # # Table name: services # -# id :integer not null, primary key -# type :string(255) -# title :string(255) -# project_id :integer not null -# created_at :datetime -# updated_at :datetime -# active :boolean default(FALSE), not null -# properties :text +# id :integer not null, primary key +# type :string(255) +# title :string(255) +# project_id :integer not null +# created_at :datetime +# updated_at :datetime +# active :boolean default(FALSE), not null +# properties :text +# push_events :boolean +# issues_events :boolean +# merge_requests_events :boolean +# tag_push_events :boolean # require 'spec_helper' diff --git a/spec/models/project_services/flowdock_service_spec.rb b/spec/models/project_services/flowdock_service_spec.rb index b34e36bc940..2ec167a7330 100644 --- a/spec/models/project_services/flowdock_service_spec.rb +++ b/spec/models/project_services/flowdock_service_spec.rb @@ -2,14 +2,18 @@ # # Table name: services # -# id :integer not null, primary key -# type :string(255) -# title :string(255) -# project_id :integer not null -# created_at :datetime -# updated_at :datetime -# active :boolean default(FALSE), not null -# properties :text +# id :integer not null, primary key +# type :string(255) +# title :string(255) +# project_id :integer not null +# created_at :datetime +# updated_at :datetime +# active :boolean default(FALSE), not null +# properties :text +# push_events :boolean +# issues_events :boolean +# merge_requests_events :boolean +# tag_push_events :boolean # require 'spec_helper' diff --git a/spec/models/project_services/gemnasium_service_spec.rb b/spec/models/project_services/gemnasium_service_spec.rb index fe5d62b2f53..5f665fadfff 100644 --- a/spec/models/project_services/gemnasium_service_spec.rb +++ b/spec/models/project_services/gemnasium_service_spec.rb @@ -2,14 +2,18 @@ # # Table name: services # -# id :integer not null, primary key -# type :string(255) -# title :string(255) -# project_id :integer not null -# created_at :datetime -# updated_at :datetime -# active :boolean default(FALSE), not null -# properties :text +# id :integer not null, primary key +# type :string(255) +# title :string(255) +# project_id :integer not null +# created_at :datetime +# updated_at :datetime +# active :boolean default(FALSE), not null +# properties :text +# push_events :boolean +# issues_events :boolean +# merge_requests_events :boolean +# tag_push_events :boolean # require 'spec_helper' diff --git a/spec/models/project_services/gitlab_ci_service_spec.rb b/spec/models/project_services/gitlab_ci_service_spec.rb index 0cd255f08ea..fcb33b11732 100644 --- a/spec/models/project_services/gitlab_ci_service_spec.rb +++ b/spec/models/project_services/gitlab_ci_service_spec.rb @@ -2,14 +2,18 @@ # # Table name: services # -# id :integer not null, primary key -# type :string(255) -# title :string(255) -# project_id :integer not null -# created_at :datetime -# updated_at :datetime -# active :boolean default(FALSE), not null -# properties :text +# id :integer not null, primary key +# type :string(255) +# title :string(255) +# project_id :integer not null +# created_at :datetime +# updated_at :datetime +# active :boolean default(FALSE), not null +# properties :text +# push_events :boolean +# issues_events :boolean +# merge_requests_events :boolean +# tag_push_events :boolean # require 'spec_helper' diff --git a/spec/models/project_services/pushover_service_spec.rb b/spec/models/project_services/pushover_service_spec.rb index 188626a7a27..bb2e72c3ac1 100644 --- a/spec/models/project_services/pushover_service_spec.rb +++ b/spec/models/project_services/pushover_service_spec.rb @@ -2,14 +2,18 @@ # # Table name: services # -# id :integer not null, primary key -# type :string(255) -# title :string(255) -# project_id :integer not null -# created_at :datetime -# updated_at :datetime -# active :boolean default(FALSE), not null -# properties :text +# id :integer not null, primary key +# type :string(255) +# title :string(255) +# project_id :integer not null +# created_at :datetime +# updated_at :datetime +# active :boolean default(FALSE), not null +# properties :text +# push_events :boolean +# issues_events :boolean +# merge_requests_events :boolean +# tag_push_events :boolean # require 'spec_helper' diff --git a/spec/models/project_services/slack_messages/slack_issue_message_spec.rb b/spec/models/project_services/slack_messages/slack_issue_message_spec.rb new file mode 100644 index 00000000000..49a8eea0a58 --- /dev/null +++ b/spec/models/project_services/slack_messages/slack_issue_message_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +describe SlackMessages::SlackIssueMessage do + subject { SlackMessages::SlackIssueMessage.new(args) } + + let(:args) { + { + user: { + username: 'username' + }, + project_name: 'project_name', + project_url: 'somewhere.com', + + object_attributes: { + title: 'Issue title', + id: 10, + iid: 100, + assignee_id: 1, + url: 'url', + action: 'open', + state: 'opened', + description: 'issue description' + } + } + } + + let(:color) { '#345' } + + context 'open' do + it 'returns a message regarding opening of issues' do + expect(subject.pretext).to eq( + 'username opened issue in : '\ + 'Issue title') + expect(subject.attachments).to eq([ + { + text: "issue description", + color: color, + } + ]) + end + end + + context 'close' do + before do + args[:object_attributes][:action] = 'close' + args[:object_attributes][:state] = 'closed' + end + it 'returns a message regarding closing of issues' do + expect(subject.pretext). to eq( + 'username closed issue in : '\ + 'Issue title') + expect(subject.attachments).to be_empty + end + end +end diff --git a/spec/models/project_services/slack_messages/slack_merge_message_spec.rb b/spec/models/project_services/slack_messages/slack_merge_message_spec.rb new file mode 100644 index 00000000000..ef76c3312ea --- /dev/null +++ b/spec/models/project_services/slack_messages/slack_merge_message_spec.rb @@ -0,0 +1,50 @@ +require 'spec_helper' + +describe SlackMessages::SlackMergeMessage do + subject { SlackMessages::SlackMergeMessage.new(args) } + + let(:args) { + { + user: { + username: 'username' + }, + project_name: 'project_name', + project_url: 'somewhere.com', + + object_attributes: { + title: 'Issue title', + id: 10, + iid: 100, + assignee_id: 1, + url: 'url', + state: 'opened', + description: 'issue description', + source_branch: 'source_branch', + target_branch: 'target_branch', + } + } + } + + let(:color) { '#345' } + + context 'open' do + it 'returns a message regarding opening of merge requests' do + expect(subject.pretext).to eq( + 'username opened merge request '\ + 'in ') + expect(subject.attachments).to be_empty + end + end + + context 'close' do + before do + args[:object_attributes][:state] = 'closed' + end + it 'returns a message regarding closing of merge requests' do + expect(subject.pretext).to eq( + 'username closed merge request '\ + 'in ') + expect(subject.attachments).to be_empty + end + end +end diff --git a/spec/models/project_services/slack_message_spec.rb b/spec/models/project_services/slack_messages/slack_push_message_spec.rb similarity index 94% rename from spec/models/project_services/slack_message_spec.rb rename to spec/models/project_services/slack_messages/slack_push_message_spec.rb index 7197a94e53f..f11614d6921 100644 --- a/spec/models/project_services/slack_message_spec.rb +++ b/spec/models/project_services/slack_messages/slack_push_message_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' -describe SlackMessage do - subject { SlackMessage.new(args) } +describe SlackMessages::SlackPushMessage do + subject { SlackMessages::SlackPushMessage.new(args) } let(:args) { { diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb index 8a75d8987ab..49c48d0b65c 100644 --- a/spec/models/project_services/slack_service_spec.rb +++ b/spec/models/project_services/slack_service_spec.rb @@ -2,14 +2,18 @@ # # Table name: services # -# id :integer not null, primary key -# type :string(255) -# title :string(255) -# project_id :integer not null -# created_at :datetime -# updated_at :datetime -# active :boolean default(FALSE), not null -# properties :text +# id :integer not null, primary key +# type :string(255) +# title :string(255) +# project_id :integer not null +# created_at :datetime +# updated_at :datetime +# active :boolean default(FALSE), not null +# properties :text +# push_events :boolean +# issues_events :boolean +# merge_requests_events :boolean +# tag_push_events :boolean # require 'spec_helper' @@ -34,7 +38,7 @@ describe SlackService do let(:slack) { SlackService.new } let(:user) { create(:user) } let(:project) { create(:project) } - let(:sample_data) { Gitlab::PushDataBuilder.build_sample(project, user) } + let(:push_sample_data) { Gitlab::PushDataBuilder.build_sample(project, user) } let(:webhook_url) { 'https://hooks.slack.com/services/SVRWFV0VVAR97N/B02R25XN3/ZBqu7xMupaEEICInN685' } let(:username) { 'slack_username' } let(:channel) { 'slack_channel' } @@ -48,10 +52,43 @@ describe SlackService do ) WebMock.stub_request(:post, webhook_url) + + opts = { + title: 'Awesome issue', + description: 'please fix' + } + + issue_service = Issues::CreateService.new(project, user, opts) + @issue = issue_service.execute + @issues_sample_data = issue_service.hook_data(@issue, 'open') + + opts = { + title: 'Awesome merge_request', + description: 'please fix', + source_branch: 'stable', + target_branch: 'master' + } + merge_service = MergeRequests::CreateService.new(project, + user, opts) + @merge_request = merge_service.execute + @merge_sample_data = merge_service.hook_data(@merge_request, + 'open') end - it "should call Slack API" do - slack.execute(sample_data) + it "should call Slack API for pull requests" do + slack.execute(push_sample_data) + + WebMock.should have_requested(:post, webhook_url).once + end + + it "should call Slack API for issue events" do + slack.execute(@issues_sample_data) + + WebMock.should have_requested(:post, webhook_url).once + end + + it "should call Slack API for merge requests events" do + slack.execute(@merge_sample_data) expect(WebMock).to have_requested(:post, webhook_url).once end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 9a1248055b1..cc047a20dd2 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -11,6 +11,10 @@ # active :boolean default(FALSE), not null # properties :text # template :boolean default(FALSE) +# push_events :boolean +# issues_events :boolean +# merge_requests_events :boolean +# tag_push_events :boolean # require 'spec_helper' diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 9924935094e..e264072b573 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -49,6 +49,7 @@ describe GitPushService do subject { @push_data } + it { is_expected.to include(object_kind: 'push') } it { is_expected.to include(before: @oldrev) } it { is_expected.to include(after: @newrev) } it { is_expected.to include(ref: @ref) } From d9ff616fd803c8bd9d208c22d01770acc8d58a38 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 20 Feb 2015 14:49:26 +0100 Subject: [PATCH 02/12] Code style, directory structure. --- app/models/project_services/asana_service.rb | 10 ++++---- .../project_services/campfire_service.rb | 6 ++--- .../emails_on_push_service.rb | 6 ++--- .../project_services/flowdock_service.rb | 10 ++++---- .../project_services/gemnasium_service.rb | 10 ++++---- .../project_services/hipchat_service.rb | 6 ++--- .../pivotaltracker_service.rb | 6 ++--- .../project_services/pushover_service.rb | 22 ++++++++--------- app/models/project_services/slack_service.rb | 24 ++++++++++--------- .../slack_base_message.rb | 4 ++-- .../slack_issue_message.rb | 4 ++-- .../slack_merge_message.rb | 4 ++-- .../slack_push_message.rb | 4 ++-- 13 files changed, 59 insertions(+), 57 deletions(-) rename app/models/project_services/{slack_messages => slack_service}/slack_base_message.rb (89%) rename app/models/project_services/{slack_messages => slack_service}/slack_issue_message.rb (94%) rename app/models/project_services/{slack_messages => slack_service}/slack_merge_message.rb (94%) rename app/models/project_services/{slack_messages => slack_service}/slack_push_message.rb (97%) diff --git a/app/models/project_services/asana_service.rb b/app/models/project_services/asana_service.rb index 2b530390aeb..a5686c48bc6 100644 --- a/app/models/project_services/asana_service.rb +++ b/app/models/project_services/asana_service.rb @@ -65,16 +65,16 @@ automatically inspected. Leave blank to include all branches.' ] end - def execute(push) - object_kind = push[:object_kind] + def execute(data) + object_kind = data[:object_kind] return unless object_kind == "push" Asana.configure do |client| client.api_key = api_key end - user = push[:user_name] - branch = push[:ref].gsub('refs/heads/', '') + user = data[:user_name] + branch = data[:ref].gsub('refs/heads/', '') branch_restriction = restrict_to_branch.to_s @@ -86,7 +86,7 @@ automatically inspected. Leave blank to include all branches.' project_name = project.name_with_namespace push_msg = user + ' pushed to branch ' + branch + ' of ' + project_name - push[:commits].each do |commit| + data[:commits].each do |commit| check_commit(' ( ' + commit[:url] + ' ): ' + commit[:message], push_msg) end end diff --git a/app/models/project_services/campfire_service.rb b/app/models/project_services/campfire_service.rb index 41ab6c56ad8..7af6882329c 100644 --- a/app/models/project_services/campfire_service.rb +++ b/app/models/project_services/campfire_service.rb @@ -41,14 +41,14 @@ class CampfireService < Service ] end - def execute(push_data) - object_kind = push_data[:object_kind] + def execute(data) + object_kind = data[:object_kind] return unless object_kind == "push" room = gate.find_room_by_name(self.room) return true unless room - message = build_message(push_data) + message = build_message(data) room.speak(message) end diff --git a/app/models/project_services/emails_on_push_service.rb b/app/models/project_services/emails_on_push_service.rb index 28be15c3b35..1b7ce481c1f 100644 --- a/app/models/project_services/emails_on_push_service.rb +++ b/app/models/project_services/emails_on_push_service.rb @@ -33,11 +33,11 @@ class EmailsOnPushService < Service 'emails_on_push' end - def execute(push_data) - object_kind = push_data[:object_kind] + def execute(data) + object_kind = data[:object_kind] return unless object_kind == "push" - EmailsOnPushWorker.perform_async(project_id, recipients, push_data) + EmailsOnPushWorker.perform_async(project_id, recipients, data) end def fields diff --git a/app/models/project_services/flowdock_service.rb b/app/models/project_services/flowdock_service.rb index 9cc0e367882..e4ea84cb617 100644 --- a/app/models/project_services/flowdock_service.rb +++ b/app/models/project_services/flowdock_service.rb @@ -41,14 +41,14 @@ class FlowdockService < Service ] end - def execute(push_data) - object_kind = push_data[:object_kind] + def execute(data) + object_kind = data[:object_kind] return unless object_kind == "push" Flowdock::Git.post( - push_data[:ref], - push_data[:before], - push_data[:after], + data[:ref], + data[:before], + data[:after], token: token, repo: project.repository.path_to_repo, repo_url: "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}", diff --git a/app/models/project_services/gemnasium_service.rb b/app/models/project_services/gemnasium_service.rb index 130c9eaeb4b..ada61b78047 100644 --- a/app/models/project_services/gemnasium_service.rb +++ b/app/models/project_services/gemnasium_service.rb @@ -42,14 +42,14 @@ class GemnasiumService < Service ] end - def execute(push_data) - object_kind = push_data[:object_kind] + def execute(data) + object_kind = data[:object_kind] return unless object_kind == "push" Gemnasium::GitlabService.execute( - ref: push_data[:ref], - before: push_data[:before], - after: push_data[:after], + ref: data[:ref], + before: data[:before], + after: data[:after], token: token, api_key: api_key, repo: project.repository.path_to_repo diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb index 462478812a1..965ecdc684b 100644 --- a/app/models/project_services/hipchat_service.rb +++ b/app/models/project_services/hipchat_service.rb @@ -44,11 +44,11 @@ class HipchatService < Service ] end - def execute(push_data) - object_kind = push_data.fetch(:object_kind) + def execute(data) + object_kind = data[:object_kind] return unless object_kind == "push" - gate[room].send('GitLab', create_message(push_data)) + gate[room].send('GitLab', create_message(data)) end private diff --git a/app/models/project_services/pivotaltracker_service.rb b/app/models/project_services/pivotaltracker_service.rb index 4bb2a978ed1..dd9e7f35c17 100644 --- a/app/models/project_services/pivotaltracker_service.rb +++ b/app/models/project_services/pivotaltracker_service.rb @@ -41,12 +41,12 @@ class PivotaltrackerService < Service ] end - def execute(push) - object_kind = push[:object_kind] + def execute(data) + object_kind = data[:object_kind] return unless object_kind == "push" url = 'https://www.pivotaltracker.com/services/v5/source_commits' - push[:commits].each do |commit| + data[:commits].each do |commit| message = { 'source_commit' => { 'commit_id' => commit[:id], diff --git a/app/models/project_services/pushover_service.rb b/app/models/project_services/pushover_service.rb index 4aa7e0afa77..a715e7b2cc5 100644 --- a/app/models/project_services/pushover_service.rb +++ b/app/models/project_services/pushover_service.rb @@ -80,24 +80,24 @@ class PushoverService < Service ] end - def execute(push_data) - object_kind = push_data[:object_kind] + def execute(data) + object_kind = data[:object_kind] return unless object_kind == "push" - ref = push_data[:ref].gsub('refs/heads/', '') - before = push_data[:before] - after = push_data[:after] + ref = data[:ref].gsub('refs/heads/', '') + before = data[:before] + after = data[:after] if before.include?('000000') - message = "#{push_data[:user_name]} pushed new branch \"#{ref}\"." + message = "#{data[:user_name]} pushed new branch \"#{ref}\"." elsif after.include?('000000') - message = "#{push_data[:user_name]} deleted branch \"#{ref}\"." + message = "#{data[:user_name]} deleted branch \"#{ref}\"." else - message = "#{push_data[:user_name]} push to branch \"#{ref}\"." + message = "#{data[:user_name]} push to branch \"#{ref}\"." end - if push_data[:total_commits_count] > 0 - message << "\nTotal commits count: #{push_data[:total_commits_count]}" + if data[:total_commits_count] > 0 + message << "\nTotal commits count: #{data[:total_commits_count]}" end pushover_data = { @@ -107,7 +107,7 @@ class PushoverService < Service priority: priority, title: "#{project.name_with_namespace}", message: message, - url: push_data[:repository][:homepage], + url: data[:repository][:homepage], url_title: "See project #{project.name_with_namespace}" } diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb index 1318a1ed1b8..8289e474031 100644 --- a/app/models/project_services/slack_service.rb +++ b/app/models/project_services/slack_service.rb @@ -16,9 +16,6 @@ # merge_requests_events :boolean default(TRUE) # tag_push_events :boolean default(TRUE) # -require "slack_messages/slack_issue_message" -require "slack_messages/slack_push_message" -require "slack_messages/slack_merge_message" class SlackService < Service prop_accessor :webhook, :username, :channel @@ -59,14 +56,15 @@ class SlackService < Service # 'close' action. Ignore update events for now to prevent duplicate # messages from arriving. - message = case object_kind - when "push" - message = SlackMessages::SlackPushMessage.new(data) - when "issue" - message = SlackMessages::SlackIssueMessage.new(data) unless is_update?(data) - when "merge_request" - message = SlackMessages::SlackMergeMessage.new(data) unless is_update?(data) - end + message = \ + case object_kind + when "push" + PushMessage.new(data) + when "issue" + IssueMessage.new(data) unless is_update?(data) + when "merge_request" + MergeMessage.new(data) unless is_update?(data) + end opt = {} opt[:channel] = channel if channel @@ -92,3 +90,7 @@ class SlackService < Service data[:object_attributes][:action] == 'update' end end + +require "slack_service/issue_message" +require "slack_service/push_message" +require "slack_service/merge_message" \ No newline at end of file diff --git a/app/models/project_services/slack_messages/slack_base_message.rb b/app/models/project_services/slack_service/slack_base_message.rb similarity index 89% rename from app/models/project_services/slack_messages/slack_base_message.rb rename to app/models/project_services/slack_service/slack_base_message.rb index c2fc27884bf..aa00d6061a1 100644 --- a/app/models/project_services/slack_messages/slack_base_message.rb +++ b/app/models/project_services/slack_service/slack_base_message.rb @@ -1,7 +1,7 @@ require 'slack-notifier' -module SlackMessages - class SlackBaseMessage +class SlackService + class BaseMessage def initialize(params) raise NotImplementedError end diff --git a/app/models/project_services/slack_messages/slack_issue_message.rb b/app/models/project_services/slack_service/slack_issue_message.rb similarity index 94% rename from app/models/project_services/slack_messages/slack_issue_message.rb rename to app/models/project_services/slack_service/slack_issue_message.rb index 0c3a492aae7..cb2e3f7421f 100644 --- a/app/models/project_services/slack_messages/slack_issue_message.rb +++ b/app/models/project_services/slack_service/slack_issue_message.rb @@ -1,5 +1,5 @@ -module SlackMessages - class SlackIssueMessage < SlackBaseMessage +module SlackService + class IssueMessage < BaseMessage attr_reader :username attr_reader :title attr_reader :project_name diff --git a/app/models/project_services/slack_messages/slack_merge_message.rb b/app/models/project_services/slack_service/slack_merge_message.rb similarity index 94% rename from app/models/project_services/slack_messages/slack_merge_message.rb rename to app/models/project_services/slack_service/slack_merge_message.rb index bc49a963a9b..309983e9f1c 100644 --- a/app/models/project_services/slack_messages/slack_merge_message.rb +++ b/app/models/project_services/slack_service/slack_merge_message.rb @@ -1,5 +1,5 @@ -module SlackMessages - class SlackMergeMessage < SlackBaseMessage +module SlackService + class MergeMessage < BaseMessage attr_reader :username attr_reader :project_name attr_reader :project_url diff --git a/app/models/project_services/slack_messages/slack_push_message.rb b/app/models/project_services/slack_service/slack_push_message.rb similarity index 97% rename from app/models/project_services/slack_messages/slack_push_message.rb rename to app/models/project_services/slack_service/slack_push_message.rb index c7769bbeda1..ab2d48f9fb2 100644 --- a/app/models/project_services/slack_messages/slack_push_message.rb +++ b/app/models/project_services/slack_service/slack_push_message.rb @@ -1,7 +1,7 @@ require 'slack-notifier' -module SlackMessages - class SlackPushMessage < SlackBaseMessage +module SlackService + class PushMessage < BaseMessage attr_reader :after attr_reader :before attr_reader :commits From 19f04cf989a75f425af09a8551957dae42b1ff31 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 20 Feb 2015 14:49:36 +0100 Subject: [PATCH 03/12] Execute services for tag push. --- app/services/git_tag_push_service.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb index 46d8987f12d..5fd8f642fad 100644 --- a/app/services/git_tag_push_service.rb +++ b/app/services/git_tag_push_service.rb @@ -8,6 +8,7 @@ class GitTagPushService EventCreateService.new.push(project, user, @push_data) project.repository.expire_cache project.execute_hooks(@push_data.dup, :tag_push_hooks) + project.execute_services(@push_data.dup, :tag_push_hooks) if project.gitlab_ci? project.gitlab_ci_service.async_execute(@push_data) From d86c0cda24a76c9330b5fed59e857ce9e4150b9b Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 20 Feb 2015 16:05:40 +0100 Subject: [PATCH 04/12] Fix specs. --- app/models/project_services/slack_service.rb | 2 +- .../{slack_base_message.rb => base_message.rb} | 0 .../{slack_issue_message.rb => issue_message.rb} | 2 +- .../{slack_merge_message.rb => merge_message.rb} | 2 +- .../{slack_push_message.rb => push_message.rb} | 4 +--- .../issue_message_spec.rb} | 4 ++-- .../merge_message_spec.rb} | 4 ++-- .../push_message_spec.rb} | 8 ++++---- 8 files changed, 12 insertions(+), 14 deletions(-) rename app/models/project_services/slack_service/{slack_base_message.rb => base_message.rb} (100%) rename app/models/project_services/slack_service/{slack_issue_message.rb => issue_message.rb} (98%) rename app/models/project_services/slack_service/{slack_merge_message.rb => merge_message.rb} (98%) rename app/models/project_services/slack_service/{slack_push_message.rb => push_message.rb} (98%) rename spec/models/project_services/{slack_messages/slack_issue_message_spec.rb => slack_service/issue_message_spec.rb} (92%) rename spec/models/project_services/{slack_messages/slack_merge_message_spec.rb => slack_service/merge_message_spec.rb} (92%) rename spec/models/project_services/{slack_messages/slack_push_message_spec.rb => slack_service/push_message_spec.rb} (87%) diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb index 8289e474031..279abad8081 100644 --- a/app/models/project_services/slack_service.rb +++ b/app/models/project_services/slack_service.rb @@ -93,4 +93,4 @@ end require "slack_service/issue_message" require "slack_service/push_message" -require "slack_service/merge_message" \ No newline at end of file +require "slack_service/merge_message" diff --git a/app/models/project_services/slack_service/slack_base_message.rb b/app/models/project_services/slack_service/base_message.rb similarity index 100% rename from app/models/project_services/slack_service/slack_base_message.rb rename to app/models/project_services/slack_service/base_message.rb diff --git a/app/models/project_services/slack_service/slack_issue_message.rb b/app/models/project_services/slack_service/issue_message.rb similarity index 98% rename from app/models/project_services/slack_service/slack_issue_message.rb rename to app/models/project_services/slack_service/issue_message.rb index cb2e3f7421f..e2fed0bb1b9 100644 --- a/app/models/project_services/slack_service/slack_issue_message.rb +++ b/app/models/project_services/slack_service/issue_message.rb @@ -1,4 +1,4 @@ -module SlackService +class SlackService class IssueMessage < BaseMessage attr_reader :username attr_reader :title diff --git a/app/models/project_services/slack_service/slack_merge_message.rb b/app/models/project_services/slack_service/merge_message.rb similarity index 98% rename from app/models/project_services/slack_service/slack_merge_message.rb rename to app/models/project_services/slack_service/merge_message.rb index 309983e9f1c..4dcce1d15a0 100644 --- a/app/models/project_services/slack_service/slack_merge_message.rb +++ b/app/models/project_services/slack_service/merge_message.rb @@ -1,4 +1,4 @@ -module SlackService +class SlackService class MergeMessage < BaseMessage attr_reader :username attr_reader :project_name diff --git a/app/models/project_services/slack_service/slack_push_message.rb b/app/models/project_services/slack_service/push_message.rb similarity index 98% rename from app/models/project_services/slack_service/slack_push_message.rb rename to app/models/project_services/slack_service/push_message.rb index ab2d48f9fb2..2e566bc317b 100644 --- a/app/models/project_services/slack_service/slack_push_message.rb +++ b/app/models/project_services/slack_service/push_message.rb @@ -1,6 +1,4 @@ -require 'slack-notifier' - -module SlackService +class SlackService class PushMessage < BaseMessage attr_reader :after attr_reader :before diff --git a/spec/models/project_services/slack_messages/slack_issue_message_spec.rb b/spec/models/project_services/slack_service/issue_message_spec.rb similarity index 92% rename from spec/models/project_services/slack_messages/slack_issue_message_spec.rb rename to spec/models/project_services/slack_service/issue_message_spec.rb index 49a8eea0a58..a23a7cc068e 100644 --- a/spec/models/project_services/slack_messages/slack_issue_message_spec.rb +++ b/spec/models/project_services/slack_service/issue_message_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' -describe SlackMessages::SlackIssueMessage do - subject { SlackMessages::SlackIssueMessage.new(args) } +describe SlackService::IssueMessage do + subject { SlackService::IssueMessage.new(args) } let(:args) { { diff --git a/spec/models/project_services/slack_messages/slack_merge_message_spec.rb b/spec/models/project_services/slack_service/merge_message_spec.rb similarity index 92% rename from spec/models/project_services/slack_messages/slack_merge_message_spec.rb rename to spec/models/project_services/slack_service/merge_message_spec.rb index ef76c3312ea..25d03cd8736 100644 --- a/spec/models/project_services/slack_messages/slack_merge_message_spec.rb +++ b/spec/models/project_services/slack_service/merge_message_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' -describe SlackMessages::SlackMergeMessage do - subject { SlackMessages::SlackMergeMessage.new(args) } +describe SlackService::MergeMessage do + subject { SlackService::MergeMessage.new(args) } let(:args) { { diff --git a/spec/models/project_services/slack_messages/slack_push_message_spec.rb b/spec/models/project_services/slack_service/push_message_spec.rb similarity index 87% rename from spec/models/project_services/slack_messages/slack_push_message_spec.rb rename to spec/models/project_services/slack_service/push_message_spec.rb index f11614d6921..ef0e7a6ee30 100644 --- a/spec/models/project_services/slack_messages/slack_push_message_spec.rb +++ b/spec/models/project_services/slack_service/push_message_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' -describe SlackMessages::SlackPushMessage do - subject { SlackMessages::SlackPushMessage.new(args) } +describe SlackService::PushMessage do + subject { SlackService::PushMessage.new(args) } let(:args) { { @@ -31,8 +31,8 @@ describe SlackMessages::SlackPushMessage do ) expect(subject.attachments).to eq([ { - text: ": message1 - author1\n"\ - ": message2 - author2", + text: ": message1 - author1\n"\ + ": message2 - author2", color: color, } ]) From f13567edc4b9ce68179d12562a711540c8994206 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 24 Feb 2015 14:29:21 +0100 Subject: [PATCH 05/12] Only execute GitlabCiService for push events. --- app/models/project_services/gitlab_ci_service.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/models/project_services/gitlab_ci_service.rb b/app/models/project_services/gitlab_ci_service.rb index a64b24b5ef1..34a20f55792 100644 --- a/app/models/project_services/gitlab_ci_service.rb +++ b/app/models/project_services/gitlab_ci_service.rb @@ -22,8 +22,6 @@ class GitlabCiService < CiService validates :project_url, presence: true, if: :activated? validates :token, presence: true, if: :activated? - delegate :execute, to: :service_hook, prefix: nil - after_save :compose_service_hook, if: :activated? def compose_service_hook @@ -32,6 +30,13 @@ class GitlabCiService < CiService hook.save end + def execute(data) + object_kind = data[:object_kind] + return unless object_kind == "push" + + service_hook.execute(data) + end + def commit_status_path(sha) project_url + "/commits/#{sha}/status.json?token=#{token}" end From ca56d9ff9ff8b28172d5e3dae7e09b77e2e6b835 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 24 Feb 2015 14:29:41 +0100 Subject: [PATCH 06/12] Don't execute GitlabCiService twice for pushed tags. --- app/services/git_tag_push_service.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb index 5fd8f642fad..725ef01ff23 100644 --- a/app/services/git_tag_push_service.rb +++ b/app/services/git_tag_push_service.rb @@ -10,10 +10,6 @@ class GitTagPushService project.execute_hooks(@push_data.dup, :tag_push_hooks) project.execute_services(@push_data.dup, :tag_push_hooks) - if project.gitlab_ci? - project.gitlab_ci_service.async_execute(@push_data) - end - true end From bbcb12f2719d5d8747339ad1bcb3457217870dc2 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 24 Feb 2015 14:31:08 +0100 Subject: [PATCH 07/12] Execute tag_push services and hooks when tag is created through web UI. --- app/services/create_tag_service.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/services/create_tag_service.rb b/app/services/create_tag_service.rb index a735d3f7f20..850077006ea 100644 --- a/app/services/create_tag_service.rb +++ b/app/services/create_tag_service.rb @@ -21,12 +21,12 @@ class CreateTagService < BaseService new_tag = repository.find_tag(tag_name) if new_tag - if project.gitlab_ci? - push_data = create_push_data(project, current_user, new_tag) - project.gitlab_ci_service.async_execute(push_data) - end - EventCreateService.new.push_ref(project, current_user, new_tag, 'add', 'refs/tags') + + push_data = create_push_data(project, current_user, new_tag) + project.execute_hooks(push_data.dup, :tag_push_hooks) + project.execute_services(push_data.dup, :tag_push_hooks) + success(new_tag) else error('Invalid reference name') From 85fa334eb6fd2069287a660e6ffa2295ea3a787f Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 24 Feb 2015 14:35:14 +0100 Subject: [PATCH 08/12] Execute GitlabCiService for both push and tag_push events. --- app/models/project_services/gitlab_ci_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/project_services/gitlab_ci_service.rb b/app/models/project_services/gitlab_ci_service.rb index 34a20f55792..bfc7a1fee32 100644 --- a/app/models/project_services/gitlab_ci_service.rb +++ b/app/models/project_services/gitlab_ci_service.rb @@ -32,7 +32,7 @@ class GitlabCiService < CiService def execute(data) object_kind = data[:object_kind] - return unless object_kind == "push" + return unless %w(push tag_push).include?(object_kind) service_hook.execute(data) end From d57e809cbd56aea8a49c6595663fc4b7250c5a34 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sat, 28 Feb 2015 17:33:18 +0100 Subject: [PATCH 09/12] Set supported events per project service. --- app/models/project_services/asana_service.rb | 7 ++- .../project_services/assembla_service.rb | 7 ++- app/models/project_services/bamboo_service.rb | 7 ++- .../project_services/buildbox_service.rb | 7 ++- .../project_services/campfire_service.rb | 7 ++- app/models/project_services/ci_service.rb | 4 ++ .../emails_on_push_service.rb | 7 ++- .../project_services/flowdock_service.rb | 7 ++- .../project_services/gemnasium_service.rb | 7 ++- .../project_services/gitlab_ci_service.rb | 7 ++- .../project_services/hipchat_service.rb | 7 ++- .../project_services/issue_tracker_service.rb | 7 ++- .../pivotaltracker_service.rb | 7 ++- .../project_services/pushover_service.rb | 7 ++- app/models/project_services/slack_service.rb | 5 ++ .../project_services/teamcity_service.rb | 7 ++- app/models/service.rb | 6 ++ app/views/admin/services/_form.html.haml | 37 +++++++++++ app/views/projects/services/_form.html.haml | 63 ++++++++++--------- 19 files changed, 156 insertions(+), 57 deletions(-) diff --git a/app/models/project_services/asana_service.rb b/app/models/project_services/asana_service.rb index a5686c48bc6..8ad1ad6267a 100644 --- a/app/models/project_services/asana_service.rb +++ b/app/models/project_services/asana_service.rb @@ -65,9 +65,12 @@ automatically inspected. Leave blank to include all branches.' ] end + def supported_events + %w(push) + end + def execute(data) - object_kind = data[:object_kind] - return unless object_kind == "push" + return unless supported_events.include?(data[:object_kind]) Asana.configure do |client| client.api_key = api_key diff --git a/app/models/project_services/assembla_service.rb b/app/models/project_services/assembla_service.rb index 01c647c1705..02aa7c972e7 100644 --- a/app/models/project_services/assembla_service.rb +++ b/app/models/project_services/assembla_service.rb @@ -42,9 +42,12 @@ class AssemblaService < Service ] end + def supported_events + %w(push) + end + def execute(data) - object_kind = data[:object_kind] - return unless object_kind == "push" + return unless supported_events.include?(data[:object_kind]) url = "https://atlas.assembla.com/spaces/#{subdomain}/github_tool?secret_key=#{token}" AssemblaService.post(url, body: { payload: data }.to_json, headers: { 'Content-Type' => 'application/json' }) diff --git a/app/models/project_services/bamboo_service.rb b/app/models/project_services/bamboo_service.rb index 6ff52af040d..6c6d74c615e 100644 --- a/app/models/project_services/bamboo_service.rb +++ b/app/models/project_services/bamboo_service.rb @@ -73,6 +73,10 @@ class BambooService < CiService ] end + def supported_events + %w(push) + end + def build_info(sha) url = URI.parse("#{bamboo_url}/rest/api/latest/result?label=#{sha}") @@ -123,8 +127,7 @@ class BambooService < CiService end def execute(data) - object_kind = data[:object_kind] - return unless object_kind == "push" + return unless supported_events.include?(data[:object_kind]) # Bamboo requires a GET and does not take any data. self.class.get("#{bamboo_url}/updateAndBuild.action?buildKey=#{build_key}", diff --git a/app/models/project_services/buildbox_service.rb b/app/models/project_services/buildbox_service.rb index 201bfc560a3..96428c91711 100644 --- a/app/models/project_services/buildbox_service.rb +++ b/app/models/project_services/buildbox_service.rb @@ -36,9 +36,12 @@ class BuildboxService < CiService hook.save end + def supported_events + %w(push) + end + def execute(data) - object_kind = data[:object_kind] - return unless object_kind == "push" + return unless supported_events.include?(data[:object_kind]) service_hook.execute(data) end diff --git a/app/models/project_services/campfire_service.rb b/app/models/project_services/campfire_service.rb index 7af6882329c..2f86fbe7a03 100644 --- a/app/models/project_services/campfire_service.rb +++ b/app/models/project_services/campfire_service.rb @@ -41,9 +41,12 @@ class CampfireService < Service ] end + def supported_events + %w(push) + end + def execute(data) - object_kind = data[:object_kind] - return unless object_kind == "push" + return unless supported_events.include?(data[:object_kind]) room = gate.find_room_by_name(self.room) return true unless room diff --git a/app/models/project_services/ci_service.rb b/app/models/project_services/ci_service.rb index e58d6d7a233..646783c8733 100644 --- a/app/models/project_services/ci_service.rb +++ b/app/models/project_services/ci_service.rb @@ -25,6 +25,10 @@ class CiService < Service :ci end + def supported_events + %w(push) + end + # Return complete url to build page # # Ex. diff --git a/app/models/project_services/emails_on_push_service.rb b/app/models/project_services/emails_on_push_service.rb index 1b7ce481c1f..21041e08a20 100644 --- a/app/models/project_services/emails_on_push_service.rb +++ b/app/models/project_services/emails_on_push_service.rb @@ -33,9 +33,12 @@ class EmailsOnPushService < Service 'emails_on_push' end + def supported_events + %w(push) + end + def execute(data) - object_kind = data[:object_kind] - return unless object_kind == "push" + return unless supported_events.include?(data[:object_kind]) EmailsOnPushWorker.perform_async(project_id, recipients, data) end diff --git a/app/models/project_services/flowdock_service.rb b/app/models/project_services/flowdock_service.rb index e4ea84cb617..443dca72a8c 100644 --- a/app/models/project_services/flowdock_service.rb +++ b/app/models/project_services/flowdock_service.rb @@ -41,9 +41,12 @@ class FlowdockService < Service ] end + def supported_events + %w(push) + end + def execute(data) - object_kind = data[:object_kind] - return unless object_kind == "push" + return unless supported_events.include?(data[:object_kind]) Flowdock::Git.post( data[:ref], diff --git a/app/models/project_services/gemnasium_service.rb b/app/models/project_services/gemnasium_service.rb index ada61b78047..41eedc215d5 100644 --- a/app/models/project_services/gemnasium_service.rb +++ b/app/models/project_services/gemnasium_service.rb @@ -42,9 +42,12 @@ class GemnasiumService < Service ] end + def supported_events + %w(push) + end + def execute(data) - object_kind = data[:object_kind] - return unless object_kind == "push" + return unless supported_events.include?(data[:object_kind]) Gemnasium::GitlabService.execute( ref: data[:ref], diff --git a/app/models/project_services/gitlab_ci_service.rb b/app/models/project_services/gitlab_ci_service.rb index bfc7a1fee32..02bf305f8f2 100644 --- a/app/models/project_services/gitlab_ci_service.rb +++ b/app/models/project_services/gitlab_ci_service.rb @@ -30,9 +30,12 @@ class GitlabCiService < CiService hook.save end + def supported_events + %w(push tag_push) + end + def execute(data) - object_kind = data[:object_kind] - return unless %w(push tag_push).include?(object_kind) + return unless supported_events.include?(data[:object_kind]) service_hook.execute(data) end diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb index 965ecdc684b..b85863d2f0d 100644 --- a/app/models/project_services/hipchat_service.rb +++ b/app/models/project_services/hipchat_service.rb @@ -44,9 +44,12 @@ class HipchatService < Service ] end + def supported_events + %w(push) + end + def execute(data) - object_kind = data[:object_kind] - return unless object_kind == "push" + return unless supported_events.include?(data[:object_kind]) gate[room].send('GitLab', create_message(data)) end diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb index 0d9e5c13992..bfc65b5379f 100644 --- a/app/models/project_services/issue_tracker_service.rb +++ b/app/models/project_services/issue_tracker_service.rb @@ -69,9 +69,12 @@ class IssueTrackerService < Service end end + def supported_events + %w(push) + end + def execute(data) - object_kind = data[:object_kind] - return unless object_kind == "push" + return unless supported_events.include?(data[:object_kind]) message = "#{self.type} was unable to reach #{self.project_url}. Check the url and try again." result = false diff --git a/app/models/project_services/pivotaltracker_service.rb b/app/models/project_services/pivotaltracker_service.rb index dd9e7f35c17..a2fa9788f10 100644 --- a/app/models/project_services/pivotaltracker_service.rb +++ b/app/models/project_services/pivotaltracker_service.rb @@ -41,9 +41,12 @@ class PivotaltrackerService < Service ] end + def supported_events + %w(push) + end + def execute(data) - object_kind = data[:object_kind] - return unless object_kind == "push" + return unless supported_events.include?(data[:object_kind]) url = 'https://www.pivotaltracker.com/services/v5/source_commits' data[:commits].each do |commit| diff --git a/app/models/project_services/pushover_service.rb b/app/models/project_services/pushover_service.rb index a715e7b2cc5..586d9e94a95 100644 --- a/app/models/project_services/pushover_service.rb +++ b/app/models/project_services/pushover_service.rb @@ -80,9 +80,12 @@ class PushoverService < Service ] end + def supported_events + %w(push) + end + def execute(data) - object_kind = data[:object_kind] - return unless object_kind == "push" + return unless supported_events.include?(data[:object_kind]) ref = data[:ref].gsub('refs/heads/', '') before = data[:before] diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb index 279abad8081..64d6f4327b8 100644 --- a/app/models/project_services/slack_service.rb +++ b/app/models/project_services/slack_service.rb @@ -42,7 +42,12 @@ class SlackService < Service ] end + def supported_events + %w(push issue merge_request) + end + def execute(data) + return unless supported_events.include?(data[:object_kind]) return unless webhook.present? object_kind = data[:object_kind] diff --git a/app/models/project_services/teamcity_service.rb b/app/models/project_services/teamcity_service.rb index 07facfb6d06..686e6225a2e 100644 --- a/app/models/project_services/teamcity_service.rb +++ b/app/models/project_services/teamcity_service.rb @@ -61,6 +61,10 @@ class TeamcityService < CiService 'teamcity' end + def supported_events + %w(push) + end + def fields [ { type: 'text', name: 'teamcity_url', @@ -120,8 +124,7 @@ class TeamcityService < CiService end def execute(data) - object_kind = data[:object_kind] - return unless object_kind == "push" + return unless supported_events.include?(data[:object_kind]) auth = { username: username, diff --git a/app/models/service.rb b/app/models/service.rb index 9d6866f26d0..98bd40ae95e 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -80,6 +80,10 @@ class Service < ActiveRecord::Base [] end + def supported_events + %w(push tag_push issue merge_request) + end + def execute # implement inside child end @@ -105,6 +109,8 @@ class Service < ActiveRecord::Base end def async_execute(data) + return unless supported_events.include?(data[:object_kind]) + Sidekiq::Client.enqueue(ProjectServiceWorker, id, data) end diff --git a/app/views/admin/services/_form.html.haml b/app/views/admin/services/_form.html.haml index 5df8849317b..62f4001ca66 100644 --- a/app/views/admin/services/_form.html.haml +++ b/app/views/admin/services/_form.html.haml @@ -14,6 +14,43 @@ = preserve do = markdown @service.help + .form-group + = f.label :url, "Trigger", class: 'control-label' + - if @service.supported_events.length > 1 + .col-sm-10 + - if @service.supported_events.include?("push") + %div + = f.check_box :push_events, class: 'pull-left' + .prepend-left-20 + = f.label :push_events, class: 'list-label' do + %strong Push events + %p.light + This url will be triggered by a push to the repository + - if @service.supported_events.include?("tag_push") + %div + = f.check_box :tag_push_events, class: 'pull-left' + .prepend-left-20 + = f.label :tag_push_events, class: 'list-label' do + %strong Tag push events + %p.light + This url will be triggered when a new tag is pushed to the repository + - if @service.supported_events.include?("issue") + %div + = f.check_box :issues_events, class: 'pull-left' + .prepend-left-20 + = f.label :issues_events, class: 'list-label' do + %strong Issues events + %p.light + This url will be triggered when an issue is created + - if @service.supported_events.include?("merge_request") + %div + = f.check_box :merge_requests_events, class: 'pull-left' + .prepend-left-20 + = f.label :merge_requests_events, class: 'list-label' do + %strong Merge Request events + %p.light + This url will be triggered when a merge request is created + - @service.fields.each do |field| - name = field[:name] - value = @service.send(name) unless field[:type] == 'password' diff --git a/app/views/projects/services/_form.html.haml b/app/views/projects/services/_form.html.haml index 0519c8150e9..55ac85c32b9 100644 --- a/app/views/projects/services/_form.html.haml +++ b/app/views/projects/services/_form.html.haml @@ -29,35 +29,40 @@ .form-group = f.label :url, "Trigger", class: 'control-label' - .col-sm-10 - %div - = f.check_box :push_events, class: 'pull-left' - .prepend-left-20 - = f.label :push_events, class: 'list-label' do - %strong Push events - %p.light - This url will be triggered by a push to the repository - %div - = f.check_box :tag_push_events, class: 'pull-left' - .prepend-left-20 - = f.label :tag_push_events, class: 'list-label' do - %strong Tag push events - %p.light - This url will be triggered when a new tag is pushed to the repository - %div - = f.check_box :issues_events, class: 'pull-left' - .prepend-left-20 - = f.label :issues_events, class: 'list-label' do - %strong Issues events - %p.light - This url will be triggered when an issue is created - %div - = f.check_box :merge_requests_events, class: 'pull-left' - .prepend-left-20 - = f.label :merge_requests_events, class: 'list-label' do - %strong Merge Request events - %p.light - This url will be triggered when a merge request is created + - if @service.supported_events.length > 1 + .col-sm-10 + - if @service.supported_events.include?("push") + %div + = f.check_box :push_events, class: 'pull-left' + .prepend-left-20 + = f.label :push_events, class: 'list-label' do + %strong Push events + %p.light + This url will be triggered by a push to the repository + - if @service.supported_events.include?("tag_push") + %div + = f.check_box :tag_push_events, class: 'pull-left' + .prepend-left-20 + = f.label :tag_push_events, class: 'list-label' do + %strong Tag push events + %p.light + This url will be triggered when a new tag is pushed to the repository + - if @service.supported_events.include?("issue") + %div + = f.check_box :issues_events, class: 'pull-left' + .prepend-left-20 + = f.label :issues_events, class: 'list-label' do + %strong Issues events + %p.light + This url will be triggered when an issue is created + - if @service.supported_events.include?("merge_request") + %div + = f.check_box :merge_requests_events, class: 'pull-left' + .prepend-left-20 + = f.label :merge_requests_events, class: 'list-label' do + %strong Merge Request events + %p.light + This url will be triggered when a merge request is created - @service.fields.each do |field| - name = field[:name] From 5c910b94cef084fc1fae398fdf72a220f800e7ad Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sat, 28 Feb 2015 17:41:01 +0100 Subject: [PATCH 10/12] Set correct object_kind on tag push data. --- app/services/create_tag_service.rb | 4 +++- app/services/git_tag_push_service.rb | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/services/create_tag_service.rb b/app/services/create_tag_service.rb index 850077006ea..8cd65724cb9 100644 --- a/app/services/create_tag_service.rb +++ b/app/services/create_tag_service.rb @@ -40,7 +40,9 @@ class CreateTagService < BaseService end def create_push_data(project, user, tag) - Gitlab::PushDataBuilder. + data = Gitlab::PushDataBuilder. build(project, user, Gitlab::Git::BLANK_SHA, tag.target, 'refs/tags/' + tag.name, []) + data[:object_kind] = "tag_push" + data end end diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb index 725ef01ff23..cd92f50b02a 100644 --- a/app/services/git_tag_push_service.rb +++ b/app/services/git_tag_push_service.rb @@ -16,7 +16,8 @@ class GitTagPushService private def create_push_data(oldrev, newrev, ref) - Gitlab::PushDataBuilder. - build(project, user, oldrev, newrev, ref, []) + data = Gitlab::PushDataBuilder.build(project, user, oldrev, newrev, ref, []) + data[:object_kind] = "tag_push" + data end end From 2c7baed3946dcc724e091698978419a18c7d6930 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 3 Mar 2015 11:20:01 +0100 Subject: [PATCH 11/12] Fix changelog. --- CHANGELOG | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 8011817d0ae..50e18f1006e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,7 +1,4 @@ Please view this file on the master branch, on stable branches it's out of date. -v 7.9.0 (unreleased) - - Added issue and merge request events to Slack service (Stan Hu) - - Fix broken access control for note attachments (Hannes Rosenögger) v 7.9.0 (unreleased) - Move labels/milestones tabs to sidebar @@ -19,6 +16,7 @@ v 7.9.0 (unreleased) - Allow user confirmation to be skipped for new users via API - Add a service to send updates to an Irker gateway (Romain Coltel) - Add brakeman (security scanner for Ruby on Rails) + - Added issue and merge request events to Slack service (Stan Hu) v 7.8.1 - Fix run of custom post receive hooks From fc6160816119504e1cea0954453cd557231341a1 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 3 Mar 2015 13:09:45 +0100 Subject: [PATCH 12/12] Fix specs. --- spec/models/project_services/slack_service_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb index 49c48d0b65c..9024e53f0ff 100644 --- a/spec/models/project_services/slack_service_spec.rb +++ b/spec/models/project_services/slack_service_spec.rb @@ -99,8 +99,8 @@ describe SlackService do with(webhook_url, username: username). and_return( double(:slack_service).as_null_object - ) - slack.execute(sample_data) + ) + slack.execute(push_sample_data) end it 'should use the channel as an option when it is configured' do @@ -110,7 +110,7 @@ describe SlackService do and_return( double(:slack_service).as_null_object ) - slack.execute(sample_data) + slack.execute(push_sample_data) end end end