Add system hooks user_rename and group_rename
This commit is contained in:
parent
7961c235d5
commit
6f1a4ba457
|
|
@ -42,6 +42,7 @@ class Group < Namespace
|
|||
after_create :post_create_hook
|
||||
after_destroy :post_destroy_hook
|
||||
after_save :update_two_factor_requirement
|
||||
after_update :path_changed_hook, if: :path_changed?
|
||||
|
||||
class << self
|
||||
def supports_nested_groups?
|
||||
|
|
@ -295,6 +296,12 @@ class Group < Namespace
|
|||
list_of_ids.reverse.map { |group| variables[group.id] }.compact.flatten
|
||||
end
|
||||
|
||||
def full_path_was
|
||||
return path_was unless has_parent?
|
||||
|
||||
"#{parent.full_path}/#{path_was}"
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def update_two_factor_requirement
|
||||
|
|
@ -303,6 +310,10 @@ class Group < Namespace
|
|||
users.find_each(&:update_two_factor_requirement)
|
||||
end
|
||||
|
||||
def path_changed_hook
|
||||
system_hook_service.execute_hooks_for(self, :rename)
|
||||
end
|
||||
|
||||
def visibility_level_allowed_by_parent
|
||||
return if visibility_level_allowed_by_parent?
|
||||
|
||||
|
|
|
|||
|
|
@ -168,6 +168,7 @@ class User < ActiveRecord::Base
|
|||
before_save :skip_reconfirmation!, if: ->(user) { user.email_changed? && user.read_only_attribute?(:email) }
|
||||
before_save :check_for_verified_email, if: ->(user) { user.email_changed? && !user.new_record? }
|
||||
after_save :ensure_namespace_correct
|
||||
after_update :username_changed_hook, if: :username_changed?
|
||||
after_destroy :post_destroy_hook
|
||||
after_commit :update_emails_with_primary_email, on: :update, if: -> { previous_changes.key?('email') }
|
||||
after_commit :update_invalid_gpg_signatures, on: :update, if: -> { previous_changes.key?('email') }
|
||||
|
|
@ -871,6 +872,10 @@ class User < ActiveRecord::Base
|
|||
end
|
||||
end
|
||||
|
||||
def username_changed_hook
|
||||
system_hook_service.execute_hooks_for(self, :rename)
|
||||
end
|
||||
|
||||
def post_destroy_hook
|
||||
log_info("User \"#{name}\" (#{email}) was removed")
|
||||
system_hook_service.execute_hooks_for(self, :destroy)
|
||||
|
|
|
|||
|
|
@ -35,24 +35,22 @@ class SystemHooksService
|
|||
data[:old_path_with_namespace] = model.old_path_with_namespace
|
||||
end
|
||||
when User
|
||||
data.merge!({
|
||||
name: model.name,
|
||||
email: model.email,
|
||||
user_id: model.id,
|
||||
username: model.username
|
||||
})
|
||||
data.merge!(user_data(model))
|
||||
|
||||
if event == :rename
|
||||
data[:old_username] = model.username_was
|
||||
end
|
||||
when ProjectMember
|
||||
data.merge!(project_member_data(model))
|
||||
when Group
|
||||
owner = model.owner
|
||||
data.merge!(group_data(model))
|
||||
|
||||
data.merge!(
|
||||
name: model.name,
|
||||
path: model.path,
|
||||
group_id: model.id,
|
||||
owner_name: owner.respond_to?(:name) ? owner.name : nil,
|
||||
owner_email: owner.respond_to?(:email) ? owner.email : nil
|
||||
)
|
||||
if event == :rename
|
||||
data.merge!(
|
||||
old_path: model.path_was,
|
||||
old_full_path: model.full_path_was
|
||||
)
|
||||
end
|
||||
when GroupMember
|
||||
data.merge!(group_member_data(model))
|
||||
end
|
||||
|
|
@ -104,6 +102,19 @@ class SystemHooksService
|
|||
}
|
||||
end
|
||||
|
||||
def group_data(model)
|
||||
owner = model.owner
|
||||
|
||||
{
|
||||
name: model.name,
|
||||
path: model.path,
|
||||
full_path: model.full_path,
|
||||
group_id: model.id,
|
||||
owner_name: owner.try(:name),
|
||||
owner_email: owner.try(:email)
|
||||
}
|
||||
end
|
||||
|
||||
def group_member_data(model)
|
||||
{
|
||||
group_name: model.group.name,
|
||||
|
|
@ -116,4 +127,13 @@ class SystemHooksService
|
|||
group_access: model.human_access
|
||||
}
|
||||
end
|
||||
|
||||
def user_data(model)
|
||||
{
|
||||
name: model.name,
|
||||
email: model.email,
|
||||
user_id: model.id,
|
||||
username: model.username
|
||||
}
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Add system hooks user_rename and group_rename
|
||||
merge_request: 15123
|
||||
author:
|
||||
type: changed
|
||||
|
|
@ -1,6 +1,24 @@
|
|||
# System hooks
|
||||
|
||||
Your GitLab instance can perform HTTP POST requests on the following events: `project_create`, `project_destroy`, `project_rename`, `project_transfer`, `project_update`, `user_add_to_team`, `user_remove_from_team`, `user_create`, `user_destroy`, `key_create`, `key_destroy`, `group_create`, `group_destroy`, `user_add_to_group` and `user_remove_from_group`.
|
||||
Your GitLab instance can perform HTTP POST requests on the following events:
|
||||
|
||||
- `project_create`
|
||||
- `project_destroy`
|
||||
- `project_rename`
|
||||
- `project_transfer`
|
||||
- `project_update`
|
||||
- `user_add_to_team`
|
||||
- `user_remove_from_team`
|
||||
- `user_create`
|
||||
- `user_destroy`
|
||||
- `user_rename`
|
||||
- `key_create`
|
||||
- `key_destroy`
|
||||
- `group_create`
|
||||
- `group_destroy`
|
||||
- `group_rename`
|
||||
- `user_add_to_group`
|
||||
- `user_remove_from_group`
|
||||
|
||||
The triggers for most of these are self-explanatory, but `project_update` and `project_rename` deserve some clarification: `project_update` is fired any time an attribute of a project is changed (name, description, tags, etc.) *unless* the `path` attribute is also changed. In that case, a `project_rename` is triggered instead (so that, for instance, if all you care about is the repo URL, you can just listen for `project_rename`).
|
||||
|
||||
|
|
@ -72,6 +90,9 @@ X-Gitlab-Event: System Hook
|
|||
}
|
||||
```
|
||||
|
||||
Note that `project_rename` is not triggered if the namespace changes.
|
||||
Please refer to `group_rename` and `user_rename` for that case.
|
||||
|
||||
**Project transferred:**
|
||||
|
||||
```json
|
||||
|
|
@ -175,6 +196,21 @@ X-Gitlab-Event: System Hook
|
|||
}
|
||||
```
|
||||
|
||||
**User renamed:**
|
||||
|
||||
```json
|
||||
{
|
||||
"event_name": "user_rename",
|
||||
"created_at": "2017-11-01T11:21:04Z",
|
||||
"updated_at": "2017-11-01T14:04:47Z",
|
||||
"name": "new-name",
|
||||
"email": "best-email@example.tld",
|
||||
"user_id": 58,
|
||||
"username": "new-exciting-name",
|
||||
"old_username": "old-boring-name"
|
||||
}
|
||||
```
|
||||
|
||||
**Key added**
|
||||
|
||||
```json
|
||||
|
|
@ -209,13 +245,15 @@ X-Gitlab-Event: System Hook
|
|||
"updated_at": "2012-07-21T07:38:22Z",
|
||||
"event_name": "group_create",
|
||||
"name": "StoreCloud",
|
||||
"owner_email": "johnsmith@gmail.com",
|
||||
"owner_name": "John Smith",
|
||||
"owner_email": null,
|
||||
"owner_name": null,
|
||||
"path": "storecloud",
|
||||
"group_id": 78
|
||||
}
|
||||
```
|
||||
|
||||
`owner_name` and `owner_email` are always `null`. Please see https://gitlab.com/gitlab-org/gitlab-ce/issues/39675.
|
||||
|
||||
**Group removed:**
|
||||
|
||||
```json
|
||||
|
|
@ -224,13 +262,35 @@ X-Gitlab-Event: System Hook
|
|||
"updated_at": "2012-07-21T07:38:22Z",
|
||||
"event_name": "group_destroy",
|
||||
"name": "StoreCloud",
|
||||
"owner_email": "johnsmith@gmail.com",
|
||||
"owner_name": "John Smith",
|
||||
"owner_email": null,
|
||||
"owner_name": null,
|
||||
"path": "storecloud",
|
||||
"group_id": 78
|
||||
}
|
||||
```
|
||||
|
||||
`owner_name` and `owner_email` are always `null`. Please see https://gitlab.com/gitlab-org/gitlab-ce/issues/39675.
|
||||
|
||||
**Group renamed:**
|
||||
|
||||
```json
|
||||
{
|
||||
"event_name": "group_rename",
|
||||
"created_at": "2017-10-30T15:09:00Z",
|
||||
"updated_at": "2017-11-01T10:23:52Z",
|
||||
"name": "Better Name",
|
||||
"path": "better-name",
|
||||
"full_path": "parent-group/better-name",
|
||||
"group_id": 64,
|
||||
"owner_name": null,
|
||||
"owner_email": null,
|
||||
"old_path": "old-name",
|
||||
"old_full_path": "parent-group/old-name"
|
||||
}
|
||||
```
|
||||
|
||||
`owner_name` and `owner_email` are always `null`. Please see https://gitlab.com/gitlab-org/gitlab-ce/issues/39675.
|
||||
|
||||
**New Group Member:**
|
||||
|
||||
```json
|
||||
|
|
|
|||
|
|
@ -134,6 +134,7 @@ describe Group, 'Routable' do
|
|||
|
||||
context 'with RequestStore active', :request_store do
|
||||
it 'does not load the route table more than once' do
|
||||
group.expires_full_path_cache
|
||||
expect(group).to receive(:uncached_full_path).once.and_call_original
|
||||
|
||||
3.times { group.full_path }
|
||||
|
|
|
|||
|
|
@ -488,6 +488,47 @@ describe Group do
|
|||
end
|
||||
end
|
||||
|
||||
describe '#path_changed_hook' do
|
||||
let(:system_hook_service) { SystemHooksService.new }
|
||||
|
||||
context 'for a new group' do
|
||||
let(:group) { build(:group) }
|
||||
|
||||
before do
|
||||
expect(group).to receive(:system_hook_service).and_return(system_hook_service)
|
||||
end
|
||||
|
||||
it 'does not trigger system hook' do
|
||||
expect(system_hook_service).to receive(:execute_hooks_for).with(group, :create)
|
||||
|
||||
group.save!
|
||||
end
|
||||
end
|
||||
|
||||
context 'for an existing group' do
|
||||
let(:group) { create(:group, path: 'old-path') }
|
||||
|
||||
context 'when the path is changed' do
|
||||
let(:new_path) { 'very-new-path' }
|
||||
|
||||
it 'triggers the rename system hook' do
|
||||
expect(group).to receive(:system_hook_service).and_return(system_hook_service)
|
||||
expect(system_hook_service).to receive(:execute_hooks_for).with(group, :rename)
|
||||
|
||||
group.update_attributes!(path: new_path)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the path is not changed' do
|
||||
it 'does not trigger system hook' do
|
||||
expect(group).not_to receive(:system_hook_service)
|
||||
|
||||
group.update_attributes!(name: 'new name')
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#secret_variables_for' do
|
||||
let(:project) { create(:project, group: group) }
|
||||
|
||||
|
|
|
|||
|
|
@ -2217,6 +2217,42 @@ describe User do
|
|||
end
|
||||
end
|
||||
|
||||
describe '#username_changed_hook' do
|
||||
context 'for a new user' do
|
||||
let(:user) { build(:user) }
|
||||
|
||||
it 'does not trigger system hook' do
|
||||
expect(user).not_to receive(:system_hook_service)
|
||||
|
||||
user.save!
|
||||
end
|
||||
end
|
||||
|
||||
context 'for an existing user' do
|
||||
let(:user) { create(:user, username: 'old-username') }
|
||||
|
||||
context 'when the username is changed' do
|
||||
let(:new_username) { 'very-new-name' }
|
||||
|
||||
it 'triggers the rename system hook' do
|
||||
system_hook_service = SystemHooksService.new
|
||||
expect(system_hook_service).to receive(:execute_hooks_for).with(user, :rename)
|
||||
expect(user).to receive(:system_hook_service).and_return(system_hook_service)
|
||||
|
||||
user.update_attributes!(username: new_username)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the username is not changed' do
|
||||
it 'does not trigger system hook' do
|
||||
expect(user).not_to receive(:system_hook_service)
|
||||
|
||||
user.update_attributes!(email: 'asdf@asdf.com')
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#sync_attribute?' do
|
||||
let(:user) { described_class.new }
|
||||
|
||||
|
|
|
|||
|
|
@ -69,11 +69,48 @@ describe SystemHooksService do
|
|||
|
||||
expect(data[:project_visibility]).to eq('private')
|
||||
end
|
||||
|
||||
context 'group_rename' do
|
||||
it 'contains old and new path' do
|
||||
allow(group).to receive(:path_was).and_return('old-path')
|
||||
|
||||
data = event_data(group, :rename)
|
||||
|
||||
expect(data).to include(:event_name, :name, :created_at, :updated_at, :full_path, :path, :group_id, :old_path, :old_full_path)
|
||||
expect(data[:path]).to eq(group.path)
|
||||
expect(data[:full_path]).to eq(group.path)
|
||||
expect(data[:old_path]).to eq(group.path_was)
|
||||
expect(data[:old_full_path]).to eq(group.path_was)
|
||||
end
|
||||
|
||||
it 'contains old and new full_path for subgroup' do
|
||||
subgroup = create(:group, parent: group)
|
||||
allow(subgroup).to receive(:path_was).and_return('old-path')
|
||||
|
||||
data = event_data(subgroup, :rename)
|
||||
|
||||
expect(data[:full_path]).to eq(subgroup.full_path)
|
||||
expect(data[:old_path]).to eq('old-path')
|
||||
end
|
||||
end
|
||||
|
||||
context 'user_rename' do
|
||||
it 'contains old and new username' do
|
||||
allow(user).to receive(:username_was).and_return('old-username')
|
||||
|
||||
data = event_data(user, :rename)
|
||||
|
||||
expect(data).to include(:event_name, :name, :created_at, :updated_at, :email, :user_id, :username, :old_username)
|
||||
expect(data[:username]).to eq(user.username)
|
||||
expect(data[:old_username]).to eq(user.username_was)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'event names' do
|
||||
it { expect(event_name(user, :create)).to eq "user_create" }
|
||||
it { expect(event_name(user, :destroy)).to eq "user_destroy" }
|
||||
it { expect(event_name(user, :rename)).to eq 'user_rename' }
|
||||
it { expect(event_name(project, :create)).to eq "project_create" }
|
||||
it { expect(event_name(project, :destroy)).to eq "project_destroy" }
|
||||
it { expect(event_name(project, :rename)).to eq "project_rename" }
|
||||
|
|
@ -85,6 +122,7 @@ describe SystemHooksService do
|
|||
it { expect(event_name(key, :destroy)).to eq 'key_destroy' }
|
||||
it { expect(event_name(group, :create)).to eq 'group_create' }
|
||||
it { expect(event_name(group, :destroy)).to eq 'group_destroy' }
|
||||
it { expect(event_name(group, :rename)).to eq 'group_rename' }
|
||||
it { expect(event_name(group_member, :create)).to eq 'user_add_to_group' }
|
||||
it { expect(event_name(group_member, :destroy)).to eq 'user_remove_from_group' }
|
||||
end
|
||||
|
|
|
|||
Loading…
Reference in New Issue