diff options
author | Douwe Maan <douwe@gitlab.com> | 2017-10-09 15:56:42 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2017-10-09 15:56:42 +0000 |
commit | bc7c6dde9ae3dcf0e2a0d7fe8f6afc912cacc87c (patch) | |
tree | 3e77c6d08786700ccb50d2f15686d49cb3aa5d98 /app | |
parent | 11c67e7c2f992299ff5918ce67995b73d1e0be6d (diff) | |
parent | f070265a6ddd0173c8924bfcd7791ecafa15ab7e (diff) | |
download | gitlab-ce-bc7c6dde9ae3dcf0e2a0d7fe8f6afc912cacc87c.tar.gz |
Merge branch '34284-add-changes-to-issuable-webhook-data' into 'master'
Include the changes in issuable webhook payloads
Closes #34284
See merge request gitlab-org/gitlab-ce!14308
Diffstat (limited to 'app')
-rw-r--r-- | app/models/concerns/issuable.rb | 29 | ||||
-rw-r--r-- | app/models/issue.rb | 18 | ||||
-rw-r--r-- | app/models/merge_request.rb | 22 | ||||
-rw-r--r-- | app/services/issuable_base_service.rb | 2 | ||||
-rw-r--r-- | app/services/issues/base_service.rb | 14 | ||||
-rw-r--r-- | app/services/merge_requests/base_service.rb | 14 | ||||
-rw-r--r-- | app/services/merge_requests/refresh_service.rb | 2 |
7 files changed, 38 insertions, 63 deletions
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index fc30d008dea..27f4dedffd3 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -256,23 +256,22 @@ module Issuable participants(user).include?(user) end - def to_hook_data(user) - hook_data = { - object_kind: self.class.name.underscore, - user: user.hook_attrs, - project: project.hook_attrs, - object_attributes: hook_attrs, - labels: labels.map(&:hook_attrs), - # DEPRECATED - repository: project.hook_attrs.slice(:name, :url, :description, :homepage) - } - if self.is_a?(Issue) - hook_data[:assignees] = assignees.map(&:hook_attrs) if assignees.any? - else - hook_data[:assignee] = assignee.hook_attrs if assignee + def to_hook_data(user, old_labels: [], old_assignees: []) + changes = previous_changes + + if old_labels != labels + changes[:labels] = [old_labels.map(&:hook_attrs), labels.map(&:hook_attrs)] + end + + if old_assignees != assignees + if self.is_a?(Issue) + changes[:assignees] = [old_assignees.map(&:hook_attrs), assignees.map(&:hook_attrs)] + else + changes[:assignee] = [old_assignees&.first&.hook_attrs, assignee&.hook_attrs] + end end - hook_data + Gitlab::HookData::IssuableBuilder.new(self).build(user: user, changes: changes) end def labels_array diff --git a/app/models/issue.rb b/app/models/issue.rb index 155c5d972b7..36e4108b9d6 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -74,20 +74,6 @@ class Issue < ActiveRecord::Base end end - def hook_attrs - assignee_ids = self.assignee_ids - - attrs = { - total_time_spent: total_time_spent, - human_total_time_spent: human_total_time_spent, - human_time_estimate: human_time_estimate, - assignee_ids: assignee_ids, - assignee_id: assignee_ids.first # This key is deprecated - } - - attributes.merge!(attrs) - end - def self.reference_prefix '#' end @@ -131,6 +117,10 @@ class Issue < ActiveRecord::Base "id DESC") end + def hook_attrs + Gitlab::HookData::IssueBuilder.new(self).build + end + # Returns a Hash of attributes to be used for Twitter card metadata def card_attributes { diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 292122f779e..75e9bdaaa45 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -179,6 +179,10 @@ class MergeRequest < ActiveRecord::Base work_in_progress?(title) ? title : "WIP: #{title}" end + def hook_attrs + Gitlab::HookData::MergeRequestBuilder.new(self).build + end + # Returns a Hash of attributes to be used for Twitter card metadata def card_attributes { @@ -587,24 +591,6 @@ class MergeRequest < ActiveRecord::Base !discussions_to_be_resolved? end - def hook_attrs - attrs = { - source: source_project.try(:hook_attrs), - target: target_project.hook_attrs, - last_commit: nil, - work_in_progress: work_in_progress?, - total_time_spent: total_time_spent, - human_total_time_spent: human_total_time_spent, - human_time_estimate: human_time_estimate - } - - if diff_head_commit - attrs[:last_commit] = diff_head_commit.hook_attrs - end - - attributes.merge!(attrs) - end - def for_fork? target_project != source_project end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index f83ece7098f..d61a342ebad 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -255,7 +255,7 @@ class IssuableBaseService < BaseService invalidate_cache_counts(issuable, users: affected_assignees.compact) after_update(issuable) issuable.create_new_cross_references!(current_user) - execute_hooks(issuable, 'update') + execute_hooks(issuable, 'update', old_labels: old_labels, old_assignees: old_assignees) issuable.update_project_counter_caches if update_project_counters end diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index 4c198fc96ea..735257c4779 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -1,10 +1,10 @@ module Issues class BaseService < ::IssuableBaseService - def hook_data(issue, action) - issue_data = issue.to_hook_data(current_user) - issue_url = Gitlab::UrlBuilder.build(issue) - issue_data[:object_attributes].merge!(url: issue_url, action: action) - issue_data + def hook_data(issue, action, old_labels: [], old_assignees: []) + hook_data = issue.to_hook_data(current_user, old_labels: old_labels, old_assignees: old_assignees) + hook_data[:object_attributes][:action] = action + + hook_data end def reopen_service @@ -22,8 +22,8 @@ module Issues issue, issue.project, current_user, old_assignees) end - def execute_hooks(issue, action = 'open') - issue_data = hook_data(issue, action) + def execute_hooks(issue, action = 'open', old_labels: [], old_assignees: []) + issue_data = hook_data(issue, action, old_labels: old_labels, old_assignees: old_assignees) hooks_scope = issue.confidential? ? :confidential_issue_hooks : :issue_hooks issue.project.execute_hooks(issue_data, hooks_scope) issue.project.execute_services(issue_data, hooks_scope) diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 35ccff26262..112606a82d7 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -18,19 +18,19 @@ module MergeRequests super if changed_title end - def hook_data(merge_request, action, oldrev = nil) - hook_data = merge_request.to_hook_data(current_user) - hook_data[:object_attributes][:url] = Gitlab::UrlBuilder.build(merge_request) + def hook_data(merge_request, action, old_rev: nil, old_labels: [], old_assignees: []) + hook_data = merge_request.to_hook_data(current_user, old_labels: old_labels, old_assignees: old_assignees) hook_data[:object_attributes][:action] = action - if oldrev && !Gitlab::Git.blank_ref?(oldrev) - hook_data[:object_attributes][:oldrev] = oldrev + if old_rev && !Gitlab::Git.blank_ref?(old_rev) + hook_data[:object_attributes][:oldrev] = old_rev end + hook_data end - def execute_hooks(merge_request, action = 'open', oldrev = nil) + def execute_hooks(merge_request, action = 'open', old_rev: nil, old_labels: [], old_assignees: []) if merge_request.project - merge_data = hook_data(merge_request, action, oldrev) + merge_data = hook_data(merge_request, action, old_rev: old_rev, old_labels: old_labels, old_assignees: old_assignees) merge_request.project.execute_hooks(merge_data, :merge_request_hooks) merge_request.project.execute_services(merge_data, :merge_request_hooks) end diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index bc4a13cf4bc..fc100580c4f 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -166,7 +166,7 @@ module MergeRequests # Call merge request webhook with update branches def execute_mr_web_hooks merge_requests_for_source_branch.each do |merge_request| - execute_hooks(merge_request, 'update', @oldrev) + execute_hooks(merge_request, 'update', old_rev: @oldrev) end end |