From 5f0e7873ae71a1f4d23a1c564bf7eb8830ebd888 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 13 Jun 2017 11:32:21 +0200 Subject: ported EE user service to CE --- spec/services/users/update_service_spec.rb | 33 ++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 spec/services/users/update_service_spec.rb (limited to 'spec/services') diff --git a/spec/services/users/update_service_spec.rb b/spec/services/users/update_service_spec.rb new file mode 100644 index 00000000000..73af9af7507 --- /dev/null +++ b/spec/services/users/update_service_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe Users::UpdateService, services: true do + let(:user) { create(:user) } + let(:admin) { create(:admin) } + let(:user) { create(:empty_user, creator_id: user.id, namespace: user.namespace) } + + describe '#execute' do + it 'updates the name' do + result = update_user(user, user, name: 'New Name') + expect(result).to eq({ status: :success }) + expect(user.name).to eq('New Name') + end + + context 'when updated by an admin' do + it 'updates the name' do + result = update_user(user, admin, name: 'New Name') + expect(result).to eq({ status: :success }) + expect(user.name).to eq('New Name') + end + end + + it 'returns an error result when record cannot be updated' do + result = update_user(user, create(:user), { name: 'New Name' }) + + expect(result).to eq({ status: :error, message: 'User could not be updated' }) + end + + def update_user(current_user, user, opts) + described_class.new(user, user, opts).execute + end + end +end -- cgit v1.2.1 From 11044ab1a02ffc681c46f1edb083dea163954b99 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 13 Jun 2017 13:52:20 +0200 Subject: fix spec --- spec/services/users/update_service_spec.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'spec/services') diff --git a/spec/services/users/update_service_spec.rb b/spec/services/users/update_service_spec.rb index 73af9af7507..87a3957bc3b 100644 --- a/spec/services/users/update_service_spec.rb +++ b/spec/services/users/update_service_spec.rb @@ -3,7 +3,6 @@ require 'spec_helper' describe Users::UpdateService, services: true do let(:user) { create(:user) } let(:admin) { create(:admin) } - let(:user) { create(:empty_user, creator_id: user.id, namespace: user.namespace) } describe '#execute' do it 'updates the name' do @@ -14,20 +13,20 @@ describe Users::UpdateService, services: true do context 'when updated by an admin' do it 'updates the name' do - result = update_user(user, admin, name: 'New Name') + result = update_user(admin, user, name: 'New Name') expect(result).to eq({ status: :success }) expect(user.name).to eq('New Name') end end it 'returns an error result when record cannot be updated' do - result = update_user(user, create(:user), { name: 'New Name' }) - - expect(result).to eq({ status: :error, message: 'User could not be updated' }) + expect do + update_user(user, create(:user), { name: 'New Name' }) + end.to raise_error Gitlab::Access::AccessDeniedError end def update_user(current_user, user, opts) - described_class.new(user, user, opts).execute + described_class.new(current_user, user, opts).execute end end end -- cgit v1.2.1 From cf6286313d2e5296e1009c3bcae7da50d7e438a4 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 13 Jun 2017 16:24:36 +0200 Subject: added bang option to spec --- spec/services/users/update_service_spec.rb | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) (limited to 'spec/services') diff --git a/spec/services/users/update_service_spec.rb b/spec/services/users/update_service_spec.rb index 87a3957bc3b..fd32a4f0f33 100644 --- a/spec/services/users/update_service_spec.rb +++ b/spec/services/users/update_service_spec.rb @@ -7,6 +7,7 @@ describe Users::UpdateService, services: true do describe '#execute' do it 'updates the name' do result = update_user(user, user, name: 'New Name') + expect(result).to eq({ status: :success }) expect(user.name).to eq('New Name') end @@ -14,6 +15,7 @@ describe Users::UpdateService, services: true do context 'when updated by an admin' do it 'updates the name' do result = update_user(admin, user, name: 'New Name') + expect(result).to eq({ status: :success }) expect(user.name).to eq('New Name') end @@ -29,4 +31,32 @@ describe Users::UpdateService, services: true do described_class.new(current_user, user, opts).execute end end + + describe '#execute!' do + it 'updates the name' do + result = update_user(user, user, name: 'New Name') + + expect(result).to be true + expect(user.name).to eq('New Name') + end + + context 'when updated by an admin' do + it 'updates the name' do + result = update_user(admin, user, name: 'New Name') + + expect(result).to be true + expect(user.name).to eq('New Name') + end + end + + it 'returns an error result when record cannot be updated' do + expect do + update_user(user, create(:user), { name: 'New Name' }) + end.to raise_error Gitlab::Access::AccessDeniedError + end + + def update_user(current_user, user, opts) + described_class.new(current_user, user, opts).execute! + end + end end -- cgit v1.2.1 From 3798e894594c6056df5bea2daa65f10bc19843d0 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 16 Jun 2017 14:07:56 +0200 Subject: add emails service specs --- spec/services/emails/create_service_spec.rb | 21 +++++++++++++++++++++ spec/services/emails/destroy_service_spec.rb | 14 ++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 spec/services/emails/create_service_spec.rb create mode 100644 spec/services/emails/destroy_service_spec.rb (limited to 'spec/services') diff --git a/spec/services/emails/create_service_spec.rb b/spec/services/emails/create_service_spec.rb new file mode 100644 index 00000000000..c1f477f551e --- /dev/null +++ b/spec/services/emails/create_service_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe Emails::CreateService, services: true do + let(:user) { create(:user) } + let(:opts) { { email: 'new@email.com' } } + + subject(:service) { described_class.new(user, opts) } + + describe '#execute' do + it 'creates an email with valid attributes' do + expect { service.execute }.to change { Email.count }.by(1) + expect(Email.where(opts)).not_to be_empty + end + + it 'has the right user association' do + service.execute + + expect(user.emails).to eq(Email.where(opts)) + end + end +end diff --git a/spec/services/emails/destroy_service_spec.rb b/spec/services/emails/destroy_service_spec.rb new file mode 100644 index 00000000000..0778b3f50da --- /dev/null +++ b/spec/services/emails/destroy_service_spec.rb @@ -0,0 +1,14 @@ +require 'spec_helper' + +describe Emails::DestroyService, services: true do + let!(:user) { create(:user) } + let!(:email) { create(:email, user: user) } + + subject(:service) { described_class.new(user, opts) } + + describe '#execute' do + it 'creates an email with valid attributes' do + expect { service.execute }.to change { user.emails.count }.by(-1) + end + end +end -- cgit v1.2.1 From cabbfe94fcc929a48b8bf7402cd8cb1cc00612d6 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 16 Jun 2017 14:30:29 +0200 Subject: add more spec examples --- spec/services/emails/create_service_spec.rb | 12 +++++++++++- spec/services/emails/destroy_service_spec.rb | 14 ++++++++++++-- 2 files changed, 23 insertions(+), 3 deletions(-) (limited to 'spec/services') diff --git a/spec/services/emails/create_service_spec.rb b/spec/services/emails/create_service_spec.rb index c1f477f551e..7874da88665 100644 --- a/spec/services/emails/create_service_spec.rb +++ b/spec/services/emails/create_service_spec.rb @@ -4,7 +4,7 @@ describe Emails::CreateService, services: true do let(:user) { create(:user) } let(:opts) { { email: 'new@email.com' } } - subject(:service) { described_class.new(user, opts) } + subject(:service) { described_class.new(user, user, opts) } describe '#execute' do it 'creates an email with valid attributes' do @@ -17,5 +17,15 @@ describe Emails::CreateService, services: true do expect(user.emails).to eq(Email.where(opts)) end + + it 'does not create an email if the user has no permissions' do + expect { described_class.new(create(:user), user, opts).execute }.not_to change { Email.count } + end + + it 'creates an email if we skip authorization' do + expect do + described_class.new(create(:user), user, opts).execute(skip_authorization: true) + end.to change { Email.count }.by(1) + end end end diff --git a/spec/services/emails/destroy_service_spec.rb b/spec/services/emails/destroy_service_spec.rb index 0778b3f50da..186726951f9 100644 --- a/spec/services/emails/destroy_service_spec.rb +++ b/spec/services/emails/destroy_service_spec.rb @@ -4,11 +4,21 @@ describe Emails::DestroyService, services: true do let!(:user) { create(:user) } let!(:email) { create(:email, user: user) } - subject(:service) { described_class.new(user, opts) } + subject(:service) { described_class.new(user, user, email: email.email) } describe '#execute' do - it 'creates an email with valid attributes' do + it 'removes an email' do expect { service.execute }.to change { user.emails.count }.by(-1) end + + it 'does not remove an email if the user has no permissions' do + expect { described_class.new(create(:user), user, opts).execute }.not_to change { Email.count } + end + + it 'removes an email if we skip authorization' do + expect do + described_class.new(create(:user), user, opts).execute(skip_authorization: true) + end.to change { Email.count }.by(-1) + end end end -- cgit v1.2.1 From ad44af2faaaa872ee30922699f66ac78fa402336 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 16 Jun 2017 15:14:46 +0200 Subject: fixed specs --- spec/services/emails/create_service_spec.rb | 2 +- spec/services/emails/destroy_service_spec.rb | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) (limited to 'spec/services') diff --git a/spec/services/emails/create_service_spec.rb b/spec/services/emails/create_service_spec.rb index 7874da88665..9981f5fcc2b 100644 --- a/spec/services/emails/create_service_spec.rb +++ b/spec/services/emails/create_service_spec.rb @@ -19,7 +19,7 @@ describe Emails::CreateService, services: true do end it 'does not create an email if the user has no permissions' do - expect { described_class.new(create(:user), user, opts).execute }.not_to change { Email.count } + expect { described_class.new(create(:user), user, opts).execute }.to raise_error(Gitlab::Access::AccessDeniedError) end it 'creates an email if we skip authorization' do diff --git a/spec/services/emails/destroy_service_spec.rb b/spec/services/emails/destroy_service_spec.rb index 186726951f9..6db050148cb 100644 --- a/spec/services/emails/destroy_service_spec.rb +++ b/spec/services/emails/destroy_service_spec.rb @@ -12,12 +12,14 @@ describe Emails::DestroyService, services: true do end it 'does not remove an email if the user has no permissions' do - expect { described_class.new(create(:user), user, opts).execute }.not_to change { Email.count } + expect do + described_class.new(create(:user), user, email: email.email).execute + end.to raise_error(Gitlab::Access::AccessDeniedError) end it 'removes an email if we skip authorization' do expect do - described_class.new(create(:user), user, opts).execute(skip_authorization: true) + described_class.new(create(:user), user, email: email.email).execute(skip_authorization: true) end.to change { Email.count }.by(-1) end end -- cgit v1.2.1 From 0ee002c70ea3711046b8d254b5cba044762e9c05 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 22 Jun 2017 12:10:28 +0200 Subject: more refactoring --- spec/services/emails/create_service_spec.rb | 10 ---------- spec/services/emails/destroy_service_spec.rb | 12 ------------ 2 files changed, 22 deletions(-) (limited to 'spec/services') diff --git a/spec/services/emails/create_service_spec.rb b/spec/services/emails/create_service_spec.rb index 9981f5fcc2b..76bf35d34e8 100644 --- a/spec/services/emails/create_service_spec.rb +++ b/spec/services/emails/create_service_spec.rb @@ -17,15 +17,5 @@ describe Emails::CreateService, services: true do expect(user.emails).to eq(Email.where(opts)) end - - it 'does not create an email if the user has no permissions' do - expect { described_class.new(create(:user), user, opts).execute }.to raise_error(Gitlab::Access::AccessDeniedError) - end - - it 'creates an email if we skip authorization' do - expect do - described_class.new(create(:user), user, opts).execute(skip_authorization: true) - end.to change { Email.count }.by(1) - end end end diff --git a/spec/services/emails/destroy_service_spec.rb b/spec/services/emails/destroy_service_spec.rb index 6db050148cb..3f5192b620e 100644 --- a/spec/services/emails/destroy_service_spec.rb +++ b/spec/services/emails/destroy_service_spec.rb @@ -10,17 +10,5 @@ describe Emails::DestroyService, services: true do it 'removes an email' do expect { service.execute }.to change { user.emails.count }.by(-1) end - - it 'does not remove an email if the user has no permissions' do - expect do - described_class.new(create(:user), user, email: email.email).execute - end.to raise_error(Gitlab::Access::AccessDeniedError) - end - - it 'removes an email if we skip authorization' do - expect do - described_class.new(create(:user), user, email: email.email).execute(skip_authorization: true) - end.to change { Email.count }.by(-1) - end end end -- cgit v1.2.1 From b804db26485ea09dc93269898dc969ed692130a2 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 23 Jun 2017 11:34:07 +0200 Subject: refactor update user service not to do auth checks --- spec/services/users/update_service_spec.rb | 39 ++++++++---------------------- 1 file changed, 10 insertions(+), 29 deletions(-) (limited to 'spec/services') diff --git a/spec/services/users/update_service_spec.rb b/spec/services/users/update_service_spec.rb index fd32a4f0f33..6c33a232cb0 100644 --- a/spec/services/users/update_service_spec.rb +++ b/spec/services/users/update_service_spec.rb @@ -2,61 +2,42 @@ require 'spec_helper' describe Users::UpdateService, services: true do let(:user) { create(:user) } - let(:admin) { create(:admin) } describe '#execute' do it 'updates the name' do - result = update_user(user, user, name: 'New Name') + result = update_user(user, name: 'New Name') expect(result).to eq({ status: :success }) expect(user.name).to eq('New Name') end - context 'when updated by an admin' do - it 'updates the name' do - result = update_user(admin, user, name: 'New Name') - - expect(result).to eq({ status: :success }) - expect(user.name).to eq('New Name') - end - end - it 'returns an error result when record cannot be updated' do expect do - update_user(user, create(:user), { name: 'New Name' }) - end.to raise_error Gitlab::Access::AccessDeniedError + update_user(user, { email: 'invalid' }) + end.not_to change { user.reload.email } end - def update_user(current_user, user, opts) - described_class.new(current_user, user, opts).execute + def update_user(user, opts) + described_class.new(user, opts).execute end end describe '#execute!' do it 'updates the name' do - result = update_user(user, user, name: 'New Name') + result = update_user(user, name: 'New Name') expect(result).to be true expect(user.name).to eq('New Name') end - context 'when updated by an admin' do - it 'updates the name' do - result = update_user(admin, user, name: 'New Name') - - expect(result).to be true - expect(user.name).to eq('New Name') - end - end - it 'returns an error result when record cannot be updated' do expect do - update_user(user, create(:user), { name: 'New Name' }) - end.to raise_error Gitlab::Access::AccessDeniedError + update_user(user, { email: 'invalid' }) + end.to raise_error(ActiveRecord::RecordInvalid) end - def update_user(current_user, user, opts) - described_class.new(current_user, user, opts).execute! + def update_user(user, opts) + described_class.new(user, opts).execute! end end end -- cgit v1.2.1 From b33c638483d6b87ba71a329275ff12e5eb865d72 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 23 Jun 2017 17:11:31 +0200 Subject: update code based on feedback --- spec/services/users/update_service_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'spec/services') diff --git a/spec/services/users/update_service_spec.rb b/spec/services/users/update_service_spec.rb index 6c33a232cb0..0b2f840c462 100644 --- a/spec/services/users/update_service_spec.rb +++ b/spec/services/users/update_service_spec.rb @@ -7,7 +7,7 @@ describe Users::UpdateService, services: true do it 'updates the name' do result = update_user(user, name: 'New Name') - expect(result).to eq({ status: :success }) + expect(result).to eq(status: :success) expect(user.name).to eq('New Name') end @@ -30,9 +30,9 @@ describe Users::UpdateService, services: true do expect(user.name).to eq('New Name') end - it 'returns an error result when record cannot be updated' do + it 'raises an error when record cannot be updated' do expect do - update_user(user, { email: 'invalid' }) + update_user(user, email: 'invalid') end.to raise_error(ActiveRecord::RecordInvalid) end -- cgit v1.2.1 From efb3d5e70f163bfd186fa3a02c967154103d27f4 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 23 Jun 2017 19:00:22 +0200 Subject: fix spec failures --- spec/services/emails/create_service_spec.rb | 2 +- spec/services/emails/destroy_service_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/services') diff --git a/spec/services/emails/create_service_spec.rb b/spec/services/emails/create_service_spec.rb index 76bf35d34e8..c1f477f551e 100644 --- a/spec/services/emails/create_service_spec.rb +++ b/spec/services/emails/create_service_spec.rb @@ -4,7 +4,7 @@ describe Emails::CreateService, services: true do let(:user) { create(:user) } let(:opts) { { email: 'new@email.com' } } - subject(:service) { described_class.new(user, user, opts) } + subject(:service) { described_class.new(user, opts) } describe '#execute' do it 'creates an email with valid attributes' do diff --git a/spec/services/emails/destroy_service_spec.rb b/spec/services/emails/destroy_service_spec.rb index 3f5192b620e..5e7ab4a40af 100644 --- a/spec/services/emails/destroy_service_spec.rb +++ b/spec/services/emails/destroy_service_spec.rb @@ -4,7 +4,7 @@ describe Emails::DestroyService, services: true do let!(:user) { create(:user) } let!(:email) { create(:email, user: user) } - subject(:service) { described_class.new(user, user, email: email.email) } + subject(:service) { described_class.new(user, email: email.email) } describe '#execute' do it 'removes an email' do -- cgit v1.2.1