Commit bbbfdba0 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'bugagazavr/gitlab-ce-extend-hooks' into 'master'

Add new data to project in push, issue, merge-request and note webhooks data

_Originally opened at !2738 by @bugagazavr._

- - -

**What does this MR do?**

Adds more data to hooks, makes repository data in all hook more dry ( use one hook_attrs method for repository data in all hooks )

**Are there points in the code the reviewer needs to double check?**

1. https://gitlab.com/bugagazavr/gitlab-ce/blob/extend-hooks/app/models/project.rb#L738
2. https://gitlab.com/bugagazavr/gitlab-ce/blob/extend-hooks/app/models/concerns/issuable.rb#L132
3. https://gitlab.com/bugagazavr/gitlab-ce/blob/extend-hooks/lib/gitlab/push_data_builder.rb#L52
4. https://gitlab.com/bugagazavr/gitlab-ce/blob/extend-hooks/lib/gitlab/note_data_builder.rb#L56

**Why was this MR needed?**

More information about user for push events, and more data about repositiry ( project )

**What are the relevant issue numbers?**

No

**Screenshots (if relevant)**

No

See merge request !2788
parents 4ae21313 1de95137
...@@ -34,6 +34,11 @@ v 8.5.0 (unreleased) ...@@ -34,6 +34,11 @@ v 8.5.0 (unreleased)
- Update the ExternalIssue regex pattern (Blake Hitchcock) - Update the ExternalIssue regex pattern (Blake Hitchcock)
- Remember user's inline/side-by-side diff view preference in a cookie (Kirill Katsnelson) - Remember user's inline/side-by-side diff view preference in a cookie (Kirill Katsnelson)
- Optimized performance of finding issues to be closed by a merge request - Optimized performance of finding issues to be closed by a merge request
- Add `avatar_url`, `description`, `git_ssh_url`, `git_http_url`, `path_with_namespace`
and `default_branch` in `project` in push, issue, merge-request and note webhooks data (Kirill Zaitsev)
- Deprecate the `ssh_url` in favor of `git_ssh_url` and `http_url` in favor of `git_http_url`
in `project` for push, issue, merge-request and note webhooks data (Kirill Zaitsev)
- Deprecate the `repository` key in push, issue, merge-request and note webhooks data, use `project` instead (Kirill Zaitsev)
- API: Expose MergeRequest#merge_status (Andrei Dziahel) - API: Expose MergeRequest#merge_status (Andrei Dziahel)
- Revert "Add IP check against DNSBLs at account sign-up" - Revert "Add IP check against DNSBLs at account sign-up"
- Actually use the `skip_merges` option in Repository#commits (Tony Chu) - Actually use the `skip_merges` option in Repository#commits (Tony Chu)
......
...@@ -129,13 +129,10 @@ module Issuable ...@@ -129,13 +129,10 @@ module Issuable
hook_data = { hook_data = {
object_kind: self.class.name.underscore, object_kind: self.class.name.underscore,
user: user.hook_attrs, user: user.hook_attrs,
repository: { project: project.hook_attrs,
name: project.name, object_attributes: hook_attrs,
url: project.url_to_repo, # DEPRECATED
description: project.description, repository: project.hook_attrs.slice(:name, :url, :description, :homepage)
homepage: project.web_url
},
object_attributes: hook_attrs
} }
hook_data.merge!(assignee: assignee.hook_attrs) if assignee hook_data.merge!(assignee: assignee.hook_attrs) if assignee
......
...@@ -738,11 +738,20 @@ class Project < ActiveRecord::Base ...@@ -738,11 +738,20 @@ class Project < ActiveRecord::Base
def hook_attrs def hook_attrs
{ {
name: name, name: name,
ssh_url: ssh_url_to_repo, description: description,
http_url: http_url_to_repo,
web_url: web_url, web_url: web_url,
avatar_url: avatar_url,
git_ssh_url: ssh_url_to_repo,
git_http_url: http_url_to_repo,
namespace: namespace.name, namespace: namespace.name,
visibility_level: visibility_level visibility_level: visibility_level,
path_with_namespace: path_with_namespace,
default_branch: default_branch,
# Backward compatibility
homepage: web_url,
url: url_to_repo,
ssh_url: ssh_url_to_repo,
http_url: http_url_to_repo
} }
end end
......
...@@ -112,7 +112,7 @@ class PushoverService < Service ...@@ -112,7 +112,7 @@ class PushoverService < Service
priority: priority, priority: priority,
title: "#{project.name_with_namespace}", title: "#{project.name_with_namespace}",
message: message, message: message,
url: data[:repository][:homepage], url: data[:project][:web_url],
url_title: "See project #{project.name_with_namespace}" url_title: "See project #{project.name_with_namespace}"
} }
......
This diff is collapsed.
...@@ -53,13 +53,10 @@ module Gitlab ...@@ -53,13 +53,10 @@ module Gitlab
object_kind: "note", object_kind: "note",
user: user.hook_attrs, user: user.hook_attrs,
project_id: project.id, project_id: project.id,
repository: { project: project.hook_attrs,
name: project.name, object_attributes: note.hook_attrs,
url: project.url_to_repo, # DEPRECATED
description: project.description, repository: project.hook_attrs.slice(:name, :url, :description, :homepage)
homepage: project.web_url,
},
object_attributes: note.hook_attrs
} }
base_data[:object_attributes][:url] = base_data[:object_attributes][:url] =
......
...@@ -47,18 +47,14 @@ module Gitlab ...@@ -47,18 +47,14 @@ module Gitlab
user_id: user.id, user_id: user.id,
user_name: user.name, user_name: user.name,
user_email: user.email, user_email: user.email,
user_avatar: user.avatar_url,
project_id: project.id, project_id: project.id,
repository: { project: project.hook_attrs,
name: project.name,
url: project.url_to_repo,
description: project.description,
homepage: project.web_url,
git_http_url: project.http_url_to_repo,
git_ssh_url: project.ssh_url_to_repo,
visibility_level: project.visibility_level
},
commits: commit_attrs, commits: commit_attrs,
total_commits_count: commits_count total_commits_count: commits_count,
# DEPRECATED
repository: project.hook_attrs.slice(:name, :url, :description, :homepage,
:git_http_url, :git_ssh_url, :visibility_level)
} }
data data
......
...@@ -16,62 +16,80 @@ describe 'Gitlab::NoteDataBuilder', lib: true do ...@@ -16,62 +16,80 @@ describe 'Gitlab::NoteDataBuilder', lib: true do
end end
describe 'When asking for a note on commit' do describe 'When asking for a note on commit' do
let(:note) { create(:note_on_commit) } let(:note) { create(:note_on_commit, project: project) }
it 'returns the note and commit-specific data' do it 'returns the note and commit-specific data' do
expect(data).to have_key(:commit) expect(data).to have_key(:commit)
end end
include_examples 'project hook data'
include_examples 'deprecated repository hook data'
end end
describe 'When asking for a note on commit diff' do describe 'When asking for a note on commit diff' do
let(:note) { create(:note_on_commit_diff) } let(:note) { create(:note_on_commit_diff, project: project) }
it 'returns the note and commit-specific data' do it 'returns the note and commit-specific data' do
expect(data).to have_key(:commit) expect(data).to have_key(:commit)
end end
include_examples 'project hook data'
include_examples 'deprecated repository hook data'
end end
describe 'When asking for a note on issue' do describe 'When asking for a note on issue' do
let(:issue) { create(:issue, created_at: fixed_time, updated_at: fixed_time) } let(:issue) { create(:issue, created_at: fixed_time, updated_at: fixed_time) }
let(:note) { create(:note_on_issue, noteable_id: issue.id) } let(:note) { create(:note_on_issue, noteable_id: issue.id, project: project) }
it 'returns the note and issue-specific data' do it 'returns the note and issue-specific data' do
expect(data).to have_key(:issue) expect(data).to have_key(:issue)
expect(data[:issue].except('updated_at')).to eq(issue.hook_attrs.except('updated_at')) expect(data[:issue].except('updated_at')).to eq(issue.hook_attrs.except('updated_at'))
expect(data[:issue]['updated_at']).to be > issue.hook_attrs['updated_at'] expect(data[:issue]['updated_at']).to be > issue.hook_attrs['updated_at']
end end
include_examples 'project hook data'
include_examples 'deprecated repository hook data'
end end
describe 'When asking for a note on merge request' do describe 'When asking for a note on merge request' do
let(:merge_request) { create(:merge_request, created_at: fixed_time, updated_at: fixed_time) } let(:merge_request) { create(:merge_request, created_at: fixed_time, updated_at: fixed_time) }
let(:note) { create(:note_on_merge_request, noteable_id: merge_request.id) } let(:note) { create(:note_on_merge_request, noteable_id: merge_request.id, project: project) }
it 'returns the note and merge request data' do it 'returns the note and merge request data' do
expect(data).to have_key(:merge_request) expect(data).to have_key(:merge_request)
expect(data[:merge_request].except('updated_at')).to eq(merge_request.hook_attrs.except('updated_at')) expect(data[:merge_request].except('updated_at')).to eq(merge_request.hook_attrs.except('updated_at'))
expect(data[:merge_request]['updated_at']).to be > merge_request.hook_attrs['updated_at'] expect(data[:merge_request]['updated_at']).to be > merge_request.hook_attrs['updated_at']
end end
include_examples 'project hook data'
include_examples 'deprecated repository hook data'
end end
describe 'When asking for a note on merge request diff' do describe 'When asking for a note on merge request diff' do
let(:merge_request) { create(:merge_request, created_at: fixed_time, updated_at: fixed_time) } let(:merge_request) { create(:merge_request, created_at: fixed_time, updated_at: fixed_time) }
let(:note) { create(:note_on_merge_request_diff, noteable_id: merge_request.id) } let(:note) { create(:note_on_merge_request_diff, noteable_id: merge_request.id, project: project) }
it 'returns the note and merge request diff data' do it 'returns the note and merge request diff data' do
expect(data).to have_key(:merge_request) expect(data).to have_key(:merge_request)
expect(data[:merge_request].except('updated_at')).to eq(merge_request.hook_attrs.except('updated_at')) expect(data[:merge_request].except('updated_at')).to eq(merge_request.hook_attrs.except('updated_at'))
expect(data[:merge_request]['updated_at']).to be > merge_request.hook_attrs['updated_at'] expect(data[:merge_request]['updated_at']).to be > merge_request.hook_attrs['updated_at']
end end
include_examples 'project hook data'
include_examples 'deprecated repository hook data'
end end
describe 'When asking for a note on project snippet' do describe 'When asking for a note on project snippet' do
let!(:snippet) { create(:project_snippet, created_at: fixed_time, updated_at: fixed_time) } let!(:snippet) { create(:project_snippet, created_at: fixed_time, updated_at: fixed_time) }
let!(:note) { create(:note_on_project_snippet, noteable_id: snippet.id) } let!(:note) { create(:note_on_project_snippet, noteable_id: snippet.id, project: project) }
it 'returns the note and project snippet data' do it 'returns the note and project snippet data' do
expect(data).to have_key(:snippet) expect(data).to have_key(:snippet)
expect(data[:snippet].except('updated_at')).to eq(snippet.hook_attrs.except('updated_at')) expect(data[:snippet].except('updated_at')).to eq(snippet.hook_attrs.except('updated_at'))
expect(data[:snippet]['updated_at']).to be > snippet.hook_attrs['updated_at'] expect(data[:snippet]['updated_at']).to be > snippet.hook_attrs['updated_at']
end end
include_examples 'project hook data'
include_examples 'deprecated repository hook data'
end end
end end
...@@ -13,13 +13,13 @@ describe 'Gitlab::PushDataBuilder', lib: true do ...@@ -13,13 +13,13 @@ describe 'Gitlab::PushDataBuilder', lib: true do
it { expect(data[:after]).to eq('5937ac0a7beb003549fc5fd26fc247adbce4a52e') } it { expect(data[:after]).to eq('5937ac0a7beb003549fc5fd26fc247adbce4a52e') }
it { expect(data[:ref]).to eq('refs/heads/master') } it { expect(data[:ref]).to eq('refs/heads/master') }
it { expect(data[:commits].size).to eq(3) } it { expect(data[:commits].size).to eq(3) }
it { expect(data[:repository][:git_http_url]).to eq(project.http_url_to_repo) }
it { expect(data[:repository][:git_ssh_url]).to eq(project.ssh_url_to_repo) }
it { expect(data[:repository][:visibility_level]).to eq(project.visibility_level) }
it { expect(data[:total_commits_count]).to eq(3) } it { expect(data[:total_commits_count]).to eq(3) }
it { expect(data[:commits].first[:added]).to eq(["gitlab-grack"]) } it { expect(data[:commits].first[:added]).to eq(["gitlab-grack"]) }
it { expect(data[:commits].first[:modified]).to eq([".gitmodules"]) } it { expect(data[:commits].first[:modified]).to eq([".gitmodules"]) }
it { expect(data[:commits].first[:removed]).to eq([]) } it { expect(data[:commits].first[:removed]).to eq([]) }
include_examples 'project hook data'
include_examples 'deprecated repository hook data'
end end
describe :build do describe :build do
......
...@@ -69,27 +69,28 @@ describe Issue, "Issuable" do ...@@ -69,27 +69,28 @@ describe Issue, "Issuable" do
end end
describe "#to_hook_data" do describe "#to_hook_data" do
let(:hook_data) { issue.to_hook_data(user) } let(:data) { issue.to_hook_data(user) }
let(:project) { issue.project }
it "returns correct hook data" do it "returns correct hook data" do
expect(hook_data[:object_kind]).to eq("issue") expect(data[:object_kind]).to eq("issue")
expect(hook_data[:user]).to eq(user.hook_attrs) expect(data[:user]).to eq(user.hook_attrs)
expect(hook_data[:repository][:name]).to eq(issue.project.name) expect(data[:object_attributes]).to eq(issue.hook_attrs)
expect(hook_data[:repository][:url]).to eq(issue.project.url_to_repo) expect(data).to_not have_key(:assignee)
expect(hook_data[:repository][:description]).to eq(issue.project.description)
expect(hook_data[:repository][:homepage]).to eq(issue.project.web_url)
expect(hook_data[:object_attributes]).to eq(issue.hook_attrs)
expect(hook_data).to_not have_key(:assignee)
end end
context "issue is assigned" do context "issue is assigned" do
before { issue.update_attribute(:assignee, user) } before { issue.update_attribute(:assignee, user) }
it "returns correct hook data" do it "returns correct hook data" do
expect(hook_data[:object_attributes]['assignee_id']).to eq(user.id) expect(data[:object_attributes]['assignee_id']).to eq(user.id)
expect(hook_data[:assignee]).to eq(user.hook_attrs) expect(data[:assignee]).to eq(user.hook_attrs)
end end
end end
include_examples 'project hook data'
include_examples 'deprecated repository hook data'
end end
describe '#card_attributes' do describe '#card_attributes' do
......
...@@ -254,13 +254,22 @@ describe MergeRequest, models: true do ...@@ -254,13 +254,22 @@ describe MergeRequest, models: true do
end end
describe "#hook_attrs" do describe "#hook_attrs" do
let(:attrs_hash) { subject.hook_attrs.to_h }
[:source, :target].each do |key|
describe "#{key} key" do
include_examples 'project hook data', project_key: key do
let(:data) { attrs_hash }
let(:project) { subject.send("#{key}_project") }
end
end
end
it "has all the required keys" do it "has all the required keys" do
attrs = subject.hook_attrs expect(attrs_hash).to include(:source)
attrs = attrs.to_h expect(attrs_hash).to include(:target)
expect(attrs).to include(:source) expect(attrs_hash).to include(:last_commit)
expect(attrs).to include(:target) expect(attrs_hash).to include(:work_in_progress)
expect(attrs).to include(:last_commit)
expect(attrs).to include(:work_in_progress)
end end
end end
......
RSpec.shared_examples 'project hook data' do |project_key: :project|
it 'contains project data' do
expect(data[project_key][:name]).to eq(project.name)
expect(data[project_key][:description]).to eq(project.description)
expect(data[project_key][:web_url]).to eq(project.web_url)
expect(data[project_key][:avatar_url]).to eq(project.avatar_url)
expect(data[project_key][:git_http_url]).to eq(project.http_url_to_repo)
expect(data[project_key][:git_ssh_url]).to eq(project.ssh_url_to_repo)
expect(data[project_key][:namespace]).to eq(project.namespace.name)
expect(data[project_key][:visibility_level]).to eq(project.visibility_level)
expect(data[project_key][:path_with_namespace]).to eq(project.path_with_namespace)
expect(data[project_key][:default_branch]).to eq(project.default_branch)
expect(data[project_key][:homepage]).to eq(project.web_url)
expect(data[project_key][:url]).to eq(project.url_to_repo)
expect(data[project_key][:ssh_url]).to eq(project.ssh_url_to_repo)
expect(data[project_key][:http_url]).to eq(project.http_url_to_repo)
end
end
RSpec.shared_examples 'deprecated repository hook data' do |project_key: :project|
it 'contains deprecated repository data' do
expect(data[:repository][:name]).to eq(project.name)
expect(data[:repository][:description]).to eq(project.description)
expect(data[:repository][:url]).to eq(project.url_to_repo)
expect(data[:repository][:homepage]).to eq(project.web_url)
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment