Read about our upcoming Code of Conduct on this issue

Commit 8c01f311 authored by Douwe Maan's avatar Douwe Maan
Browse files

Merge branch 'winh-namespace-rename-hooks' into 'master'

Add system hooks user_rename and group_rename

Closes #39596

See merge request gitlab-org/gitlab-ce!15123
......@@ -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 @@ def secret_variables_for(ref, project)
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 @@ def update_two_factor_requirement
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 @@ def update_tracked_fields!(request)
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 @@ def ensure_namespace_correct
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 @@ def build_event_data(model, event)
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 @@ def project_member_data(model)
}
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 @@ def group_member_data(model)
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
---
title: Add system hooks user_rename and group_rename
merge_request: 15123
author:
type: changed
# 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 @@
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 @@ def setup_group_members(group)
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 @@ def add_user(access)
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 @@
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 @@
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
......
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