Group#members_with_parents : Inconsistency with invited members
Background
This was found while fixing Cells: Fix cross joins related to groups (#417455 - closed)
Today the Group#members_with_parents
deliberately excludes invited direct members.
However, for members via group share we include all members - this is probably an oversight.
Likely, there's no impact to users as all callers of this method, we have not found any.
Steps to reproduce
- Apply this diff
index 96ef36a5b75a..0a713e0486e4 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -1868,6 +1868,7 @@ def setup_group_members(group)
let!(:developer) { group.add_member(create(:user), GroupMember::DEVELOPER) }
let!(:pending_maintainer) { create(:group_member, :awaiting, :maintainer, group: group.parent) }
let!(:pending_developer) { create(:group_member, :awaiting, :developer, group: group) }
+ let!(:invited_developer) { create(:group_member, :invited, :developer, group: group) }
let!(:inactive_developer) { group.add_member(create(:user, :deactivated), GroupMember::DEVELOPER) }
it 'returns parents active members' do
@@ -1876,6 +1877,7 @@ def setup_group_members(group)
expect(group.members_with_parents).not_to include(pending_developer)
expect(group.members_with_parents).not_to include(pending_maintainer)
expect(group.members_with_parents).not_to include(inactive_developer)
+ expect(group.members_with_parents).not_to(include(invited_developer))
end
context 'group sharing' do
@@ -1890,6 +1892,8 @@ def setup_group_members(group)
include(developer))
expect(shared_group.members_with_parents).not_to(
include(pending_developer))
+ expect(shared_group.members_with_parents).not_to(
+ include(invited_developer))
end
end
- Run the spec
bundle exec rspec spec/models/group_spec.rb -e members_with_parents
What happens
- Direct membership does not include invited members
- Group share with group member includes invited members
What I expect to happen
- Direct membership does not include invited members
- Group share with group member does not include invited members
Affected places
$ git grep -E "[ .]members_with_parents"
app/helpers/users/group_callouts_helper.rb: group.member_count > 1 || group.members_with_parents.count > 1
app/models/group.rb: members_with_parents.all_owners.exists?(user_id: user)
app/models/group.rb: members_with_parents.maintainers.exists?(user_id: user)
app/models/group.rb: members_with_parents(only_active_users: false)
app/models/group.rb: members_with_parents.pluck(Arel.sql('DISTINCT members.user_id'))
app/models/group.rb: def members_with_parents(only_active_users: true)
app/models/group.rb: members_with_parents.select([:user_id, 'MAX(access_level) AS access_level'])
app/models/group.rb: members_with_parents(only_active_users: false).where(user_id: user.id).group(:user_id).maximum(:access_level)
app/services/todos/destroy/group_private_service.rb: group.members_with_parents.select(:user_id)
ee/app/services/protected_environments/base_service.rb: container.members_with_parents.owners_and_maintainers
ee/app/services/todos/destroy/confidential_epic_service.rb: authorized_users = epic.group.members_with_parents.non_guests.select(:user_id)
Edited by Thong Kuah