summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>2017-04-28 11:50:11 +0300
committerDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>2017-04-28 11:50:11 +0300
commitb8dc4c761dd5277188320c74633122b8a3a3173a (patch)
tree1ea02e1d14cf6a6b2637b850b2ce37d9d8bd7a83
parent00e4ec55c3b8757e6d5fb8bf3176dd7e20966b3a (diff)
downloadgitlab-ce-b8dc4c761dd5277188320c74633122b8a3a3173a.tar.gz
Collect all users by single query when using Member#add_usersdz-improve-add-users-method
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
-rw-r--r--app/models/member.rb5
-rw-r--r--spec/models/member_spec.rb6
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