diff options
author | Douwe Maan <douwe@gitlab.com> | 2017-04-28 15:02:45 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2017-04-28 15:02:45 +0000 |
commit | 2bd552041661ad1f2e5364a643a21d1419a9e6b3 (patch) | |
tree | d0589df69095b5e323106df1ec247de8b153c2e7 | |
parent | 17a4c2a1c22a7bc6cafeebd36f3de4ad041fb939 (diff) | |
parent | b8dc4c761dd5277188320c74633122b8a3a3173a (diff) | |
download | gitlab-ce-2bd552041661ad1f2e5364a643a21d1419a9e6b3.tar.gz |
Merge branch 'dz-improve-add-users-method' into 'master'
Collect all users by single query when using Member#add_users
See merge request !10975
-rw-r--r-- | app/models/member.rb | 5 | ||||
-rw-r--r-- | spec/models/member_spec.rb | 6 |
2 files changed, 9 insertions, 2 deletions
diff --git a/app/models/member.rb b/app/models/member.rb index 97fba501759..7228e82e978 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -154,6 +154,11 @@ class Member < ActiveRecord::Base def add_users(source, users, access_level, current_user: nil, expires_at: nil) return [] unless users.present? + # Collect all user ids into separate array + # so we can use single sql query to get user objects + user_ids = users.select { |user| user =~ /\A\d+\Z/ } + users = users - user_ids + User.where(id: user_ids) + self.transaction do users.map do |user| add_user( diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index b0f3657d3b5..ccc3deac199 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -390,13 +390,15 @@ describe Member, models: true do %w[project group].each do |source_type| context "when source is a #{source_type}" do let!(:source) { create(source_type, :public, :access_requestable) } - let!(:user) { create(:user) } let!(:admin) { create(:admin) } + let(:user1) { create(:user) } + let(:user2) { create(:user) } it 'returns a <Source>Member objects' do - members = described_class.add_users(source, [user], :master) + members = described_class.add_users(source, [user1, user2], :master) expect(members).to be_a Array + expect(members.size).to eq(2) expect(members.first).to be_a "#{source_type.classify}Member".constantize expect(members.first).to be_persisted end |