diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb
index d854b95cb9386f80e25f2a2b5b91e5386f5552b7..53e810035c547e58b27da51c93bcd920fb4c4ebf 100644
--- a/app/services/system_hooks_service.rb
+++ b/app/services/system_hooks_service.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
class SystemHooksService
- BUILDER_DRIVEN_EVENT_DATA_AVAILABLE_FOR_CLASSES = [GroupMember, Group, ProjectMember].freeze
+ BUILDER_DRIVEN_EVENT_DATA_AVAILABLE_FOR_CLASSES = [GroupMember, Group, ProjectMember, User].freeze
def execute_hooks_for(model, event)
data = build_event_data(model, event)
@@ -47,15 +47,6 @@ def build_event_data(model, event)
if event == :rename || event == :transfer
data[:old_path_with_namespace] = model.old_path_with_namespace
end
- when User
- data.merge!(user_data(model))
-
- case event
- when :rename
- data[:old_username] = model.username_before_last_save
- when :failed_login
- data[:state] = model.state
- end
end
data
@@ -79,15 +70,6 @@ def project_data(model)
}
end
- def user_data(model)
- {
- name: model.name,
- email: model.email,
- user_id: model.id,
- username: model.username
- }
- end
-
def builder_driven_event_data_available?(model)
model.class.in?(BUILDER_DRIVEN_EVENT_DATA_AVAILABLE_FOR_CLASSES)
end
@@ -100,10 +82,10 @@ def builder_driven_event_data(model, event)
Gitlab::HookData::GroupBuilder
when ProjectMember
Gitlab::HookData::ProjectMemberBuilder
+ when User
+ Gitlab::HookData::UserBuilder
end
builder_class.new(model).build(event)
end
end
-
-SystemHooksService.prepend_if_ee('EE::SystemHooksService')
diff --git a/ee/app/services/ee/system_hooks_service.rb b/ee/app/services/ee/system_hooks_service.rb
deleted file mode 100644
index 56654aeffe63d43bcffbc06a43fda05504a9f2ce..0000000000000000000000000000000000000000
--- a/ee/app/services/ee/system_hooks_service.rb
+++ /dev/null
@@ -1,25 +0,0 @@
-# frozen_string_literal: true
-
-module EE
- module SystemHooksService
- extend ::Gitlab::Utils::Override
-
- private
-
- override :user_data
- def user_data(model)
- super.tap do |data|
- data.merge!(email_opted_in_data(model)) if ::Gitlab.com?
- end
- end
-
- def email_opted_in_data(model)
- {
- email_opted_in: model.email_opted_in,
- email_opted_in_ip: model.email_opted_in_ip,
- email_opted_in_source: model.email_opted_in_source,
- email_opted_in_at: model.email_opted_in_at
- }
- end
- end
-end
diff --git a/ee/lib/ee/gitlab/hook_data/user_builder.rb b/ee/lib/ee/gitlab/hook_data/user_builder.rb
new file mode 100644
index 0000000000000000000000000000000000000000..3432aa7f4576c12effda4fc148cdec256167fa2b
--- /dev/null
+++ b/ee/lib/ee/gitlab/hook_data/user_builder.rb
@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+module EE
+ module Gitlab
+ module HookData
+ module UserBuilder
+ extend ::Gitlab::Utils::Override
+
+ # Sample data
+ # {
+ # :created_at=>"2021-04-02T09:56:49Z",
+ # :updated_at=>"2021-04-02T09:56:49Z",
+ # :event_name=>"user_create",
+ # :name=>"John Doe",
+ # :email=>"john@example.com",
+ # :user_id=>2,
+ # :username=>"johndoe",
+ # :email_opted_in=>"john@example.com",
+ # :email_opted_in_ip=>"192.168.1.1",
+ # :email_opted_in_source=>"Gitlab.com",
+ # :email_opted_in_at=>"2021-03-31T10:30:58Z"
+ # }
+
+ private
+
+ override :user_data
+ def user_data
+ super.tap do |data|
+ data.merge!(email_opted_in_data) if ::Gitlab.com?
+ end
+ end
+
+ def email_opted_in_data
+ {
+ email_opted_in: user.email_opted_in,
+ email_opted_in_ip: user.email_opted_in_ip,
+ email_opted_in_source: user.email_opted_in_source,
+ email_opted_in_at: user.email_opted_in_at
+ }
+ end
+ end
+ end
+ end
+end
diff --git a/ee/spec/lib/ee/gitlab/hook_data/user_builder_spec.rb b/ee/spec/lib/ee/gitlab/hook_data/user_builder_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..0c76502fb10013efdecf63fd346c2680fb340b8e
--- /dev/null
+++ b/ee/spec/lib/ee/gitlab/hook_data/user_builder_spec.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+RSpec.describe Gitlab::HookData::UserBuilder do
+ let_it_be(:user) { create(:user) }
+
+ describe '#build' do
+ let(:event) { :create }
+ let(:data) { described_class.new(user).build(event) }
+
+ context 'data for Gitlab.com' do
+ context 'contains `email_opted_in` attributes' do
+ let(:user) { create(:user, name: 'John Doe', username: 'johndoe', email: 'john@example.com') }
+
+ before do
+ expect(Gitlab).to receive(:com?).and_return(true)
+ end
+
+ it 'returns correct email_opted_in data' do
+ allow(user).to receive(:email_opted_in).and_return(user.email)
+ allow(user).to receive(:email_opted_in_ip).and_return('192.168.1.1')
+ allow(user).to receive(:email_opted_in_source).and_return('Gitlab.com')
+ allow(user).to receive(:email_opted_in_at).and_return('2021-03-31T10:30:58Z')
+
+ expect(data[:email_opted_in]).to eq('john@example.com')
+ expect(data[:email_opted_in_ip]).to eq('192.168.1.1')
+ expect(data[:email_opted_in_source]).to eq('Gitlab.com')
+ expect(data[:email_opted_in_at]).to eq('2021-03-31T10:30:58Z')
+ end
+ end
+ end
+ end
+end
diff --git a/ee/spec/services/ee/system_hooks_service_spec.rb b/ee/spec/services/ee/system_hooks_service_spec.rb
deleted file mode 100644
index 7dc4bfa4330944591e702ab67ddc87c32ad4d703..0000000000000000000000000000000000000000
--- a/ee/spec/services/ee/system_hooks_service_spec.rb
+++ /dev/null
@@ -1,51 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe EE::SystemHooksService do
- context 'when group member' do
- let(:group) { create(:group) }
- let(:group_member) { create(:group_member, group: group) }
-
- context 'event data' do
- it { expect(event_data(group_member, :create)).to include(:event_name, :created_at, :updated_at, :group_name, :group_path, :group_plan, :group_id, :user_name, :user_username, :user_email, :user_id, :group_access) }
- it { expect(event_data(group_member, :destroy)).to include(:event_name, :created_at, :updated_at, :group_name, :group_path, :group_plan, :group_id, :user_name, :user_username, :user_email, :user_id, :group_access) }
- end
-
- context 'with a Ultimate plan' do
- let(:group) { create(:group_with_plan, plan: :ultimate_plan) }
-
- it 'returns correct group_plan' do
- expect(event_data(group_member, :create)[:group_plan]).to eq('ultimate')
- end
- end
- end
-
- context 'when user' do
- let_it_be(:user) { create(:user) }
-
- context 'event data' do
- context 'for GitLab.com' do
- before do
- expect(Gitlab).to receive(:com?).and_return(true)
- end
-
- it { expect(event_data(user, :create)).to include(:event_name, :name, :created_at, :updated_at, :email, :user_id, :username, :email_opted_in, :email_opted_in_ip, :email_opted_in_source, :email_opted_in_at) }
- it { expect(event_data(user, :destroy)).to include(:event_name, :name, :created_at, :updated_at, :email, :user_id, :username, :email_opted_in, :email_opted_in_ip, :email_opted_in_source, :email_opted_in_at) }
- end
-
- context 'for non-GitLab.com' do
- before do
- expect(Gitlab).to receive(:com?).and_return(false)
- end
-
- it { expect(event_data(user, :create)).to include(:event_name, :name, :created_at, :updated_at, :email, :user_id, :username) }
- it { expect(event_data(user, :destroy)).to include(:event_name, :name, :created_at, :updated_at, :email, :user_id, :username) }
- end
- end
- end
-
- def event_data(*args)
- SystemHooksService.new.send :build_event_data, *args
- end
-end
diff --git a/lib/gitlab/hook_data/user_builder.rb b/lib/gitlab/hook_data/user_builder.rb
new file mode 100644
index 0000000000000000000000000000000000000000..537245e948f6a0eb3b6abbe83298086bc5b489c8
--- /dev/null
+++ b/lib/gitlab/hook_data/user_builder.rb
@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module HookData
+ class UserBuilder < BaseBuilder
+ alias_method :user, :object
+
+ # Sample data
+ # {
+ # :created_at=>"2021-04-02T10:00:26Z",
+ # :updated_at=>"2021-04-02T10:00:26Z",
+ # :event_name=>"user_create",
+ # :name=>"John Doe",
+ # :email=>"john@example.com",
+ # :user_id=>1,
+ # :username=>"johndoe"
+ # }
+
+ def build(event)
+ [
+ timestamps_data,
+ event_data(event),
+ user_data,
+ event_specific_user_data(event)
+ ].reduce(:merge)
+ end
+
+ private
+
+ def user_data
+ {
+ name: user.name,
+ email: user.email,
+ user_id: user.id,
+ username: user.username
+ }
+ end
+
+ def event_specific_user_data(event)
+ case event
+ when :rename
+ { old_username: user.username_before_last_save }
+ when :failed_login
+ { state: user.state }
+ else
+ {}
+ end
+ end
+ end
+ end
+end
+
+Gitlab::HookData::UserBuilder.prepend_if_ee('EE::Gitlab::HookData::UserBuilder')
diff --git a/spec/lib/gitlab/hook_data/user_builder_spec.rb b/spec/lib/gitlab/hook_data/user_builder_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..f971089850b4836f4d1a2fcfeab0e3567ac66e0a
--- /dev/null
+++ b/spec/lib/gitlab/hook_data/user_builder_spec.rb
@@ -0,0 +1,90 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::HookData::UserBuilder do
+ let_it_be(:user) { create(:user, name: 'John Doe', username: 'johndoe', email: 'john@example.com') }
+
+ describe '#build' do
+ let(:data) { described_class.new(user).build(event) }
+ let(:event_name) { data[:event_name] }
+ let(:attributes) do
+ [
+ :event_name, :created_at, :updated_at, :name, :email, :user_id, :username
+ ]
+ end
+
+ context 'data' do
+ shared_examples_for 'includes the required attributes' do
+ it 'includes the required attributes' do
+ expect(data).to include(*attributes)
+
+ expect(data[:name]).to eq('John Doe')
+ expect(data[:email]).to eq('john@example.com')
+ expect(data[:user_id]).to eq(user.id)
+ expect(data[:username]).to eq('johndoe')
+ expect(data[:created_at]).to eq(user.created_at.xmlschema)
+ expect(data[:updated_at]).to eq(user.updated_at.xmlschema)
+ end
+ end
+
+ shared_examples_for 'does not include old username attributes' do
+ it 'does not include old username attributes' do
+ expect(data).not_to include(:old_username)
+ end
+ end
+
+ shared_examples_for 'does not include state attributes' do
+ it 'does not include state attributes' do
+ expect(data).not_to include(:state)
+ end
+ end
+
+ context 'on create' do
+ let(:event) { :create }
+
+ it { expect(event_name).to eq('user_create') }
+ it_behaves_like 'includes the required attributes'
+ it_behaves_like 'does not include old username attributes'
+ it_behaves_like 'does not include state attributes'
+ end
+
+ context 'on destroy' do
+ let(:event) { :destroy }
+
+ it { expect(event_name).to eq('user_destroy') }
+ it_behaves_like 'includes the required attributes'
+ it_behaves_like 'does not include old username attributes'
+ it_behaves_like 'does not include state attributes'
+ end
+
+ context 'on rename' do
+ let(:event) { :rename }
+
+ it { expect(event_name).to eq('user_rename') }
+ it_behaves_like 'includes the required attributes'
+ it_behaves_like 'does not include state attributes'
+
+ it 'includes old username details' do
+ allow(user).to receive(:username_before_last_save).and_return('old-username')
+
+ expect(data[:old_username]).to eq(user.username_before_last_save)
+ end
+ end
+
+ context 'on failed_login' do
+ let(:event) { :failed_login }
+
+ it { expect(event_name).to eq('user_failed_login') }
+ it_behaves_like 'includes the required attributes'
+ it_behaves_like 'does not include old username attributes'
+
+ it 'includes state details' do
+ user.ldap_block!
+
+ expect(data[:state]).to eq('ldap_blocked')
+ end
+ end
+ end
+ end
+end
diff --git a/spec/services/system_hooks_service_spec.rb b/spec/services/system_hooks_service_spec.rb
index 446325e5f71395dccc44082c8256e5b425a74672..d8435c72896a642ae80a1a849969942dce95fdfb 100644
--- a/spec/services/system_hooks_service_spec.rb
+++ b/spec/services/system_hooks_service_spec.rb
@@ -113,37 +113,9 @@
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_before_last_save).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_before_last_save)
- end
- end
-
- context 'user_failed_login' do
- it 'contains state of user' do
- user.ldap_block!
-
- data = event_data(user, :failed_login)
-
- expect(data).to include(:event_name, :name, :created_at, :updated_at, :email, :user_id, :username, :state)
- expect(data[:username]).to eq(user.username)
- expect(data[:state]).to eq('ldap_blocked')
- 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(user, :failed_login)).to eq 'user_failed_login' }
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" }