diff options
author | Grzegorz Bizon <grzegorz@gitlab.com> | 2017-08-27 07:54:22 +0300 |
---|---|---|
committer | Simon Knox <psimyn@gmail.com> | 2017-08-27 08:32:05 +0300 |
commit | 7e7d2524580672480d079c05a3e5effdaee74cf6 (patch) | |
tree | fc883f7352db943a252d9fa2b81720bd1b2a07b6 | |
parent | 9031bb47e07572d1d75039345003d8144e10b806 (diff) |
Merge branch 'sh-system-hooks-ldap-users' into 'master'
Fire system hooks when a user is created via LDAP or OAuth
Closes #37073
See merge request !13846
-rw-r--r-- | app/services/concerns/users/new_user_notifier.rb | 9 | ||||
-rw-r--r-- | app/services/users/create_service.rb | 8 | ||||
-rw-r--r-- | app/services/users/update_service.rb | 6 | ||||
-rw-r--r-- | changelogs/unreleased/sh-system-hooks-ldap-users.yml | 5 | ||||
-rw-r--r-- | spec/services/users/update_service_spec.rb | 17 |
5 files changed, 39 insertions, 6 deletions
diff --git a/app/services/concerns/users/new_user_notifier.rb b/app/services/concerns/users/new_user_notifier.rb new file mode 100644 index 00000000000..231693ce7a9 --- /dev/null +++ b/app/services/concerns/users/new_user_notifier.rb @@ -0,0 +1,9 @@ +module Users + module NewUserNotifier + def notify_new_user(user, reset_token) + log_info("User \"#{user.name}\" (#{user.email}) was created") + notification_service.new_user(user, reset_token) if reset_token + system_hook_service.execute_hooks_for(user, :create) + end + end +end diff --git a/app/services/users/create_service.rb b/app/services/users/create_service.rb index 74abc017cea..c8a3c461d60 100644 --- a/app/services/users/create_service.rb +++ b/app/services/users/create_service.rb @@ -1,5 +1,7 @@ module Users class CreateService < BaseService + include NewUserNotifier + def initialize(current_user, params = {}) @current_user = current_user @params = params.dup @@ -10,11 +12,7 @@ module Users @reset_token = user.generate_reset_token if user.recently_sent_password_reset? - if user.save - log_info("User \"#{user.name}\" (#{user.email}) was created") - notification_service.new_user(user, @reset_token) if @reset_token - system_hook_service.execute_hooks_for(user, :create) - end + notify_new_user(user, @reset_token) if user.save user end diff --git a/app/services/users/update_service.rb b/app/services/users/update_service.rb index dfbd6016c3f..2f9855273dc 100644 --- a/app/services/users/update_service.rb +++ b/app/services/users/update_service.rb @@ -1,5 +1,7 @@ module Users class UpdateService < BaseService + include NewUserNotifier + def initialize(user, params = {}) @user = user @params = params.dup @@ -10,7 +12,11 @@ module Users assign_attributes(&block) + user_exists = @user.persisted? + if @user.save(validate: validate) + notify_new_user(@user, nil) unless user_exists + success else error(@user.errors.full_messages.uniq.join('. ')) diff --git a/changelogs/unreleased/sh-system-hooks-ldap-users.yml b/changelogs/unreleased/sh-system-hooks-ldap-users.yml new file mode 100644 index 00000000000..87514ec00ea --- /dev/null +++ b/changelogs/unreleased/sh-system-hooks-ldap-users.yml @@ -0,0 +1,5 @@ +--- +title: Fire system hooks when a user is created via LDAP +merge_request: +author: +type: fixed diff --git a/spec/services/users/update_service_spec.rb b/spec/services/users/update_service_spec.rb index 343804e3de0..256eb0de73a 100644 --- a/spec/services/users/update_service_spec.rb +++ b/spec/services/users/update_service_spec.rb @@ -24,7 +24,10 @@ describe Users::UpdateService do describe '#execute!' do it 'updates the name' do - result = update_user(user, name: 'New Name') + service = described_class.new(user, name: 'New Name') + expect(service).not_to receive(:notify_new_user) + + result = service.execute! expect(result).to be true expect(user.name).to eq('New Name') @@ -36,6 +39,18 @@ describe Users::UpdateService do end.to raise_error(ActiveRecord::RecordInvalid) end + it 'fires system hooks when a new user is saved' do + system_hook_service = spy(:system_hook_service) + user = build(:user) + service = described_class.new(user, name: 'John Doe') + expect(service).to receive(:notify_new_user).and_call_original + expect(service).to receive(:system_hook_service).and_return(system_hook_service) + + service.execute + + expect(system_hook_service).to have_received(:execute_hooks_for).with(user, :create) + end + def update_user(user, opts) described_class.new(user, opts).execute! end |