Patch #43946 » reduce_queries_in_group_user_added.patch
| app/models/group.rb | ||
|---|---|---|
| 72 | 72 |
end |
| 73 | 73 | |
| 74 | 74 |
def user_added(user) |
| 75 |
members.preload(:member_roles).each do |member| |
|
| 76 |
next if member.project_id.nil? |
|
| 77 |
# skip if the group is a member without roles in the project |
|
| 78 |
next if member.member_roles.empty? |
|
| 79 | ||
| 80 |
user_member = |
|
| 81 |
Member.find_or_initialize_by(:project_id => member.project_id, :user_id => user.id) |
|
| 82 |
user_member.member_roles << |
|
| 83 |
member.member_roles.pluck(:id, :role_id).map do |id, role_id| |
|
| 84 |
MemberRole.new(:role_id => role_id, :inherited_from => id) |
|
| 85 |
end |
|
| 86 |
user_member.save! |
|
| 75 |
group_members = members.preload(:member_roles).select do |m| |
|
| 76 |
m.project_id.present? && m.member_roles.any? |
|
| 77 |
end |
|
| 78 |
return if group_members.empty? |
|
| 79 | ||
| 80 |
direct_project_ids = group_members.map(&:project_id) |
|
| 81 |
inheriting_children = collect_inheriting_descendants(direct_project_ids) |
|
| 82 |
all_project_ids = direct_project_ids + inheriting_children.values.flatten |
|
| 83 | ||
| 84 |
existing_members = Member.where(:user_id => user.id, :project_id => all_project_ids). |
|
| 85 |
preload(:member_roles).index_by(&:project_id) |
|
| 86 |
existing_role_keys = Set.new |
|
| 87 |
existing_members.each do |project_id, m| |
|
| 88 |
m.member_roles.each do |mr| |
|
| 89 |
existing_role_keys << [project_id, mr.inherited_from] if mr.inherited_from |
|
| 90 |
end |
|
| 91 |
end |
|
| 92 | ||
| 93 |
group_members.each do |member| |
|
| 94 |
group_role_data = member.member_roles.map { |mr| [mr.id, mr.role_id] }
|
|
| 95 | ||
| 96 |
user_member = existing_members[member.project_id] || |
|
| 97 |
Member.new(:project_id => member.project_id, :user_id => user.id) |
|
| 98 |
new_roles = group_role_data.filter_map do |mr_id, role_id| |
|
| 99 |
next if existing_role_keys.include?([member.project_id, mr_id]) |
|
| 100 | ||
| 101 |
build_inherited_member_role(role_id, mr_id) |
|
| 102 |
end |
|
| 103 |
unless new_roles.empty? |
|
| 104 |
user_member.member_roles << new_roles |
|
| 105 |
user_member.save! |
|
| 106 |
existing_members[member.project_id] = user_member |
|
| 107 |
new_roles.each { |mr| existing_role_keys << [member.project_id, mr.inherited_from] }
|
|
| 108 |
end |
|
| 109 | ||
| 110 |
propagate_member_roles_to_descendants( |
|
| 111 |
user, new_roles, member.project_id, |
|
| 112 |
inheriting_children, existing_members, existing_role_keys |
|
| 113 |
) |
|
| 87 | 114 |
end |
| 88 | 115 |
end |
| 89 | 116 | |
| ... | ... | |
| 112 | 139 | |
| 113 | 140 |
private |
| 114 | 141 | |
| 142 |
def build_inherited_member_role(role_id, inherited_from) |
|
| 143 |
mr = MemberRole.new(:role_id => role_id, :inherited_from => inherited_from) |
|
| 144 |
mr.skip_subproject_propagation = true |
|
| 145 |
mr |
|
| 146 |
end |
|
| 147 | ||
| 148 |
def collect_inheriting_descendants(root_project_ids) |
|
| 149 |
result = {}
|
|
| 150 |
current_ids = root_project_ids |
|
| 151 |
while current_ids.any? |
|
| 152 |
children = Project.where(:parent_id => current_ids, :inherit_members => true).pluck(:id, :parent_id) |
|
| 153 |
break if children.empty? |
|
| 154 | ||
| 155 |
children.each do |child_id, parent_id| |
|
| 156 |
(result[parent_id] ||= []) << child_id |
|
| 157 |
end |
|
| 158 |
current_ids = children.map(&:first) |
|
| 159 |
end |
|
| 160 |
result |
|
| 161 |
end |
|
| 162 | ||
| 163 |
def propagate_member_roles_to_descendants(user, source_member_roles, parent_project_id, |
|
| 164 |
inheriting_children, existing_members, existing_role_keys) |
|
| 165 |
return if source_member_roles.empty? |
|
| 166 | ||
| 167 |
child_project_ids = inheriting_children[parent_project_id] |
|
| 168 |
return unless child_project_ids |
|
| 169 | ||
| 170 |
child_project_ids.each do |child_project_id| |
|
| 171 |
child_member = existing_members[child_project_id] || |
|
| 172 |
Member.new(:project_id => child_project_id, :user_id => user.id) |
|
| 173 |
child_roles = source_member_roles.filter_map do |source_mr| |
|
| 174 |
next if existing_role_keys.include?([child_project_id, source_mr.id]) |
|
| 175 | ||
| 176 |
build_inherited_member_role(source_mr.role_id, source_mr.id) |
|
| 177 |
end |
|
| 178 |
unless child_roles.empty? |
|
| 179 |
child_member.member_roles << child_roles |
|
| 180 |
child_member.save! |
|
| 181 |
existing_members[child_project_id] = child_member |
|
| 182 |
child_roles.each { |mr| existing_role_keys << [child_project_id, mr.inherited_from] }
|
|
| 183 |
end |
|
| 184 | ||
| 185 |
propagate_member_roles_to_descendants( |
|
| 186 |
user, child_roles, child_project_id, |
|
| 187 |
inheriting_children, existing_members, existing_role_keys |
|
| 188 |
) |
|
| 189 |
end |
|
| 190 |
end |
|
| 191 | ||
| 115 | 192 |
# Removes references that are not handled by associations |
| 116 | 193 |
def remove_references_before_destroy |
| 117 | 194 |
return if self.id.nil? |
| app/models/member_role.rb | ||
|---|---|---|
| 21 | 21 |
belongs_to :member |
| 22 | 22 |
belongs_to :role |
| 23 | 23 | |
| 24 |
attr_accessor :skip_subproject_propagation |
|
| 25 | ||
| 24 | 26 |
after_create :add_role_to_group_users, :add_role_to_subprojects |
| 25 | 27 |
after_destroy :remove_member_if_empty |
| 26 | 28 | |
| ... | ... | |
| 68 | 70 |
end |
| 69 | 71 | |
| 70 | 72 |
def add_role_to_subprojects |
| 73 |
return if skip_subproject_propagation |
|
| 71 | 74 |
return if member.project.leaf? |
| 72 | 75 | |
| 73 | 76 |
member.project.children.where(:inherit_members => true).ids.each do |subproject_id| |
| test/unit/group_test.rb | ||
|---|---|---|
| 183 | 183 |
assert_nothing_raised {group.users << user}
|
| 184 | 184 |
assert group.users.include?(user) |
| 185 | 185 |
end |
| 186 | ||
| 187 |
def test_user_added_should_propagate_roles_to_subprojects |
|
| 188 |
group = Group.find(11) |
|
| 189 |
user = User.find(9) |
|
| 190 |
parent = Project.generate! |
|
| 191 |
child = Project.generate_with_parent!(parent, :inherit_members => true) |
|
| 192 | ||
| 193 |
Member.create!(:principal => group, :project => parent, :role_ids => [1, 2]) |
|
| 194 |
group.users << user |
|
| 195 | ||
| 196 |
# User should be a member of both parent and child |
|
| 197 |
assert user.member_of?(parent) |
|
| 198 |
assert user.member_of?(child) |
|
| 199 | ||
| 200 |
# User's roles in child should match those in parent |
|
| 201 |
parent_user_member = Member.find_by(:project_id => parent.id, :user_id => user.id) |
|
| 202 |
child_user_member = Member.find_by(:project_id => child.id, :user_id => user.id) |
|
| 203 |
assert_not_nil child_user_member |
|
| 204 |
assert_equal parent_user_member.roles.sort, child_user_member.roles.sort |
|
| 205 | ||
| 206 |
# Child user member roles should be inherited from parent user member roles |
|
| 207 |
parent_user_mr_ids = parent_user_member.member_roles.select(&:inherited?).map(&:id).sort |
|
| 208 |
child_user_inherited_from = child_user_member.member_roles.select { |mr|
|
|
| 209 |
parent_user_mr_ids.include?(mr.inherited_from) |
|
| 210 |
} |
|
| 211 |
assert_equal parent_user_mr_ids.size, child_user_inherited_from.size |
|
| 212 |
end |
|
| 213 | ||
| 214 |
def test_user_added_should_propagate_roles_to_grandchild_projects |
|
| 215 |
group = Group.find(11) |
|
| 216 |
user = User.find(9) |
|
| 217 |
grandparent = Project.generate! |
|
| 218 |
parent = Project.generate_with_parent!(grandparent, :inherit_members => true) |
|
| 219 |
child = Project.generate_with_parent!(parent, :inherit_members => true) |
|
| 220 | ||
| 221 |
Member.create!(:principal => group, :project => grandparent, :role_ids => [1, 2]) |
|
| 222 |
group.users << user |
|
| 223 | ||
| 224 |
# User should be a member of all three levels |
|
| 225 |
assert user.member_of?(grandparent) |
|
| 226 |
assert user.member_of?(parent) |
|
| 227 |
assert user.member_of?(child) |
|
| 228 | ||
| 229 |
# Verify inherited_from chain: child -> parent -> grandparent |
|
| 230 |
gp_user_member = Member.find_by(:project_id => grandparent.id, :user_id => user.id) |
|
| 231 |
p_user_member = Member.find_by(:project_id => parent.id, :user_id => user.id) |
|
| 232 |
c_user_member = Member.find_by(:project_id => child.id, :user_id => user.id) |
|
| 233 | ||
| 234 |
assert_equal gp_user_member.roles.sort, p_user_member.roles.sort |
|
| 235 |
assert_equal p_user_member.roles.sort, c_user_member.roles.sort |
|
| 236 | ||
| 237 |
# Parent's inherited MemberRoles should point to grandparent's user MemberRoles |
|
| 238 |
gp_user_mr_ids = gp_user_member.member_roles.select(&:inherited?).map(&:id).sort |
|
| 239 |
p_inherited_from = p_user_member.member_roles.select { |mr|
|
|
| 240 |
gp_user_mr_ids.include?(mr.inherited_from) |
|
| 241 |
} |
|
| 242 |
assert_equal gp_user_mr_ids.size, p_inherited_from.size |
|
| 243 | ||
| 244 |
# Child's inherited MemberRoles should point to parent's user MemberRoles |
|
| 245 |
p_user_mr_ids = p_user_member.member_roles.select(&:inherited?).map(&:id).sort |
|
| 246 |
c_inherited_from = c_user_member.member_roles.select { |mr|
|
|
| 247 |
p_user_mr_ids.include?(mr.inherited_from) |
|
| 248 |
} |
|
| 249 |
assert_equal p_user_mr_ids.size, c_inherited_from.size |
|
| 250 |
end |
|
| 251 | ||
| 252 |
def test_user_added_should_propagate_roles_to_sibling_subprojects |
|
| 253 |
group = Group.find(11) |
|
| 254 |
user = User.find(9) |
|
| 255 |
parent = Project.generate! |
|
| 256 |
child1 = Project.generate_with_parent!(parent, :inherit_members => true) |
|
| 257 |
child2 = Project.generate_with_parent!(parent, :inherit_members => true) |
|
| 258 | ||
| 259 |
Member.create!(:principal => group, :project => parent, :role_ids => [1, 2]) |
|
| 260 |
group.users << user |
|
| 261 | ||
| 262 |
# User should be a member of parent and both siblings |
|
| 263 |
assert user.member_of?(parent) |
|
| 264 |
assert user.member_of?(child1) |
|
| 265 |
assert user.member_of?(child2) |
|
| 266 | ||
| 267 |
# Both siblings should have the same roles as parent |
|
| 268 |
parent_user_member = Member.find_by(:project_id => parent.id, :user_id => user.id) |
|
| 269 |
child1_user_member = Member.find_by(:project_id => child1.id, :user_id => user.id) |
|
| 270 |
child2_user_member = Member.find_by(:project_id => child2.id, :user_id => user.id) |
|
| 271 |
assert_not_nil child1_user_member |
|
| 272 |
assert_not_nil child2_user_member |
|
| 273 |
assert_equal parent_user_member.roles.sort, child1_user_member.roles.sort |
|
| 274 |
assert_equal parent_user_member.roles.sort, child2_user_member.roles.sort |
|
| 275 | ||
| 276 |
# Both siblings' member_roles should be inherited from parent's member_roles |
|
| 277 |
parent_user_mr_ids = parent_user_member.member_roles.select(&:inherited?).map(&:id).sort |
|
| 278 |
[child1_user_member, child2_user_member].each do |child_member| |
|
| 279 |
child_inherited_from = child_member.member_roles.select { |mr|
|
|
| 280 |
parent_user_mr_ids.include?(mr.inherited_from) |
|
| 281 |
} |
|
| 282 |
assert_equal parent_user_mr_ids.size, child_inherited_from.size |
|
| 283 |
end |
|
| 284 |
end |
|
| 186 | 285 |
end |