Improve performance of adding or removing members of a group (#36696).

Patch by Go MAEDA.


git-svn-id: http://svn.redmine.org/redmine/trunk@21447 e93f8b46-1217-0410-a6f0-8f06a7374b81

2 files changed, 24 insertions, 27 deletions

diff --git a/app/models/group.rb b/app/models/group.rb
index 66f8eda35..a42d3ac4a 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -78,27 +78,24 @@ class Group < Principal
   end
 
   def user_added(user)
-    members.each do |member|
-      next if member.project.nil?
+    members.preload(:member_roles).each do |member|
+      next if member.project_id.nil?
 
       user_member =
-        Member.find_by_project_id_and_user_id(member.project_id, user.id) ||
-          Member.new(:project_id => member.project_id, :user_id => user.id)
-      member.member_roles.each do |member_role|
-        user_member.member_roles << MemberRole.new(:role => member_role.role,
-                                                   :inherited_from => member_role.id)
-      end
+        Member.find_or_initialize_by(:project_id => member.project_id, :user_id => user.id)
+      user_member.member_roles <<
+        member.member_roles.pluck(:id, :role_id).map do |id, role_id|
+          MemberRole.new(:role_id => role_id, :inherited_from => id)
+        end
       user_member.save!
     end
   end
 
   def user_removed(user)
-    members.each do |member|
-      MemberRole.
-        joins(:member).
-        where("#{Member.table_name}.user_id = ? AND #{MemberRole.table_name}.inherited_from IN (?)", user.id, member.member_role_ids).
-        each(&:destroy)
-    end
+    MemberRole.
+      joins(:member).
+      where("#{Member.table_name}.user_id = ? AND #{MemberRole.table_name}.inherited_from IN (?)", user.id, MemberRole.select(:id).where(:member => members)).
+      destroy_all
   end
 
   def self.human_attribute_name(attribute_key_name, *args)
diff --git a/app/models/member_role.rb b/app/models/member_role.rb
index 2a02994aa..01dfc289e 100644
--- a/app/models/member_role.rb
+++ b/app/models/member_role.rb
@@ -30,7 +30,7 @@ class MemberRole < ActiveRecord::Base
   validate :validate_role_member
 
   def validate_role_member
-    errors.add :role_id, :invalid if role && !role.member?
+    errors.add :role_id, :invalid unless role&.member?
   end
 
   def inherited?
@@ -58,22 +58,22 @@ class MemberRole < ActiveRecord::Base
   end
 
   def add_role_to_group_users
-    if member.principal.is_a?(Group) && !inherited?
-      member.principal.users.each do |user|
-        user_member = Member.find_or_new(member.project_id, user.id)
-        user_member.member_roles << MemberRole.new(:role => role, :inherited_from => id)
-        user_member.save!
-      end
+    return if inherited? || !member.principal.is_a?(Group)
+
+    member.principal.users.ids.each do |user_id|
+      user_member = Member.find_or_initialize_by(:project_id => member.project_id, :user_id => user_id)
+      user_member.member_roles << MemberRole.new(:role_id => role_id, :inherited_from => id)
+      user_member.save!
     end
   end
 
   def add_role_to_subprojects
-    member.project.children.each do |subproject|
-      if subproject.inherit_members?
-        child_member = Member.find_or_new(subproject.id, member.user_id)
-        child_member.member_roles << MemberRole.new(:role => role, :inherited_from => id)
-        child_member.save!
-      end
+    return if member.project.leaf?
+
+    member.project.children.where(:inherit_members => true).ids.each do |subproject_id|
+      child_member = Member.find_or_initialize_by(:project_id => subproject_id, :user_id => member.user_id)
+      child_member.member_roles << MemberRole.new(:role => role, :inherited_from => id)
+      child_member.save!
     end
   end