Commit 8d5607a8 authored by Imre Farkas's avatar Imre Farkas

Remove share_group_with_group feature flag

The feature is default enabled since 12.7.
parent 972c347a
# frozen_string_literal: true # frozen_string_literal: true
class Groups::GroupLinksController < Groups::ApplicationController class Groups::GroupLinksController < Groups::ApplicationController
before_action :check_feature_flag!
before_action :authorize_admin_group! before_action :authorize_admin_group!
before_action :group_link, only: [:update, :destroy] before_action :group_link, only: [:update, :destroy]
...@@ -51,8 +50,4 @@ class Groups::GroupLinksController < Groups::ApplicationController ...@@ -51,8 +50,4 @@ class Groups::GroupLinksController < Groups::ApplicationController
def group_link_params def group_link_params
params.require(:group_link).permit(:group_access, :expires_at) params.require(:group_link).permit(:group_access, :expires_at)
end end
def check_feature_flag!
render_404 unless Feature.enabled?(:share_group_with_group, default_enabled: true)
end
end end
...@@ -520,8 +520,6 @@ class Group < Namespace ...@@ -520,8 +520,6 @@ class Group < Namespace
end end
def max_member_access_for_user_from_shared_groups(user) def max_member_access_for_user_from_shared_groups(user)
return unless Feature.enabled?(:share_group_with_group, default_enabled: true)
group_group_link_table = GroupGroupLink.arel_table group_group_link_table = GroupGroupLink.arel_table
group_member_table = GroupMember.arel_table group_member_table = GroupMember.arel_table
......
...@@ -9,20 +9,16 @@ ...@@ -9,20 +9,16 @@
= _("Group members") = _("Group members")
%hr %hr
- if can_manage_members - if can_manage_members
- if Feature.enabled?(:share_group_with_group, default_enabled: true) %ul.nav-links.nav.nav-tabs.gitlab-tabs{ role: 'tablist' }
%ul.nav-links.nav.nav-tabs.gitlab-tabs{ role: 'tablist' } %li.nav-tab{ role: 'presentation' }
%a.nav-link.active{ href: '#invite-member-pane', id: 'invite-member-tab', data: { toggle: 'tab' }, role: 'tab' }= _("Invite member")
%li.nav-tab{ role: 'presentation' } %li.nav-tab{ role: 'presentation' }
%a.nav-link.active{ href: '#invite-member-pane', id: 'invite-member-tab', data: { toggle: 'tab' }, role: 'tab' }= _("Invite member") %a.nav-link{ href: '#invite-group-pane', id: 'invite-group-tab', data: { toggle: 'tab', qa_selector: 'invite_group_tab' }, role: 'tab' }= _("Invite group")
%li.nav-tab{ role: 'presentation' } .tab-content.gitlab-tab-content
%a.nav-link{ href: '#invite-group-pane', id: 'invite-group-tab', data: { toggle: 'tab', qa_selector: 'invite_group_tab' }, role: 'tab' }= _("Invite group") .tab-pane.active{ id: 'invite-member-pane', role: 'tabpanel' }
.tab-content.gitlab-tab-content = render_invite_member_for_group(@group, @group_member.access_level)
.tab-pane.active{ id: 'invite-member-pane', role: 'tabpanel' } .tab-pane{ id: 'invite-group-pane', role: 'tabpanel' }
= render_invite_member_for_group(@group, @group_member.access_level) = render 'shared/members/invite_group', submit_url: group_group_links_path(@group), access_levels: GroupMember.access_level_roles, default_access_level: @group_member.access_level, group_link_field: 'shared_with_group_id', group_access_field: 'shared_group_access'
- if Feature.enabled?(:share_group_with_group, default_enabled: true)
.tab-pane{ id: 'invite-group-pane', role: 'tabpanel' }
= render 'shared/members/invite_group', submit_url: group_group_links_path(@group), access_levels: GroupMember.access_level_roles, default_access_level: @group_member.access_level, group_link_field: 'shared_with_group_id', group_access_field: 'shared_group_access'
- else
= render_invite_member_for_group(@group, @group_member.access_level)
= render 'shared/members/requests', membership_source: @group, requesters: @requesters = render 'shared/members/requests', membership_source: @group, requesters: @requesters
......
...@@ -401,14 +401,10 @@ module EE ...@@ -401,14 +401,10 @@ module EE
# Members belonging to Groups invited to collaborate with Groups and Subgroups # Members belonging to Groups invited to collaborate with Groups and Subgroups
def billed_shared_group_members def billed_shared_group_members
return ::GroupMember.none unless ::Feature.enabled?(:share_group_with_group)
invited_or_shared_group_members(invited_group_in_groups) invited_or_shared_group_members(invited_group_in_groups)
end end
def billed_shared_non_guests_group_members def billed_shared_non_guests_group_members
return ::GroupMember.none unless ::Feature.enabled?(:share_group_with_group)
invited_or_shared_group_members(invited_non_guest_group_in_groups) invited_or_shared_group_members(invited_non_guest_group_in_groups)
end end
......
...@@ -965,61 +965,44 @@ describe Namespace do ...@@ -965,61 +965,44 @@ describe Namespace do
shared_group: group }) shared_group: group })
end end
context 'when feature is not enabled' do it 'includes active users from the shared group to the billed members', :aggregate_failures do
before do expect(group.billed_user_ids).to match_array([shared_group_developer.id, developer.id])
stub_feature_flags(share_group_with_group: false) expect(shared_group.billed_user_ids).not_to include([developer.id])
end
it 'does not include users coming from the shared groups', :aggregate_failures do
expect(group.billed_user_ids).to match_array([developer.id])
expect(shared_group.billed_user_ids).not_to include([developer.id])
end
end end
context 'when feature is enabled' do context 'when subgroup invited another group to collaborate' do
before do let(:another_shared_group) { create(:group) }
stub_feature_flags(share_group_with_group: true) let(:another_shared_group_developer) { create(:user) }
end
it 'includes active users from the shared group to the billed members', :aggregate_failures do before do
expect(group.billed_user_ids).to match_array([shared_group_developer.id, developer.id]) another_shared_group.add_developer(another_shared_group_developer)
expect(shared_group.billed_user_ids).not_to include([developer.id]) another_shared_group.add_guest(create(:user))
another_shared_group.add_developer(create(:user, :blocked))
end end
context 'when subgroup invited another group to collaborate' do context 'when subgroup invites another group as non guest' do
let(:another_shared_group) { create(:group) }
let(:another_shared_group_developer) { create(:user) }
before do before do
another_shared_group.add_developer(another_shared_group_developer) subgroup = create(:group, parent: group)
another_shared_group.add_guest(create(:user)) create(:group_group_link, { shared_with_group: another_shared_group,
another_shared_group.add_developer(create(:user, :blocked)) shared_group: subgroup })
end end
context 'when subgroup invites another group as non guest' do it 'includes all the active and non guest users from the shared group', :aggregate_failures do
before do expect(group.billed_user_ids).to match_array([shared_group_developer.id, developer.id, another_shared_group_developer.id])
subgroup = create(:group, parent: group) expect(shared_group.billed_user_ids).not_to include([developer.id])
create(:group_group_link, { shared_with_group: another_shared_group, expect(another_shared_group.billed_user_ids).not_to include([developer.id, shared_group_developer.id])
shared_group: subgroup })
end
it 'includes all the active and non guest users from the shared group', :aggregate_failures do
expect(group.billed_user_ids).to match_array([shared_group_developer.id, developer.id, another_shared_group_developer.id])
expect(shared_group.billed_user_ids).not_to include([developer.id])
expect(another_shared_group.billed_user_ids).not_to include([developer.id, shared_group_developer.id])
end
end end
end
context 'when subgroup invites another group as guest' do context 'when subgroup invites another group as guest' do
before do before do
subgroup = create(:group, parent: group) subgroup = create(:group, parent: group)
create(:group_group_link, :guest, { shared_with_group: another_shared_group, create(:group_group_link, :guest, { shared_with_group: another_shared_group,
shared_group: subgroup }) shared_group: subgroup })
end end
it 'does not includes any user from the shared group from the subgroup' do it 'does not includes any user from the shared group from the subgroup' do
expect(group.billed_user_ids).to match_array([shared_group_developer.id, developer.id]) expect(group.billed_user_ids).to match_array([shared_group_developer.id, developer.id])
end
end end
end end
end end
...@@ -1089,25 +1072,9 @@ describe Namespace do ...@@ -1089,25 +1072,9 @@ describe Namespace do
shared_group: group }) shared_group: group })
end end
context 'when feature is not enabled' do it 'includes active users from the shared group including guests', :aggregate_failures do
before do expect(group.billed_user_ids).to match_array([developer.id, guest.id, shared_group_developer.id, shared_group_guest.id])
stub_feature_flags(share_group_with_group: false) expect(shared_group.billed_user_ids).to match_array([shared_group_developer.id, shared_group_guest.id])
end
it 'does not include users coming from the shared groups' do
expect(group.billed_user_ids).to match_array([developer.id, guest.id])
end
end
context 'when feature is enabled' do
before do
stub_feature_flags(share_group_with_group: true)
end
it 'includes active users from the shared group including guests', :aggregate_failures do
expect(group.billed_user_ids).to match_array([developer.id, guest.id, shared_group_developer.id, shared_group_guest.id])
expect(shared_group.billed_user_ids).to match_array([shared_group_developer.id, shared_group_guest.id])
end
end end
end end
end end
...@@ -1186,24 +1153,8 @@ describe Namespace do ...@@ -1186,24 +1153,8 @@ describe Namespace do
shared_group: group }) shared_group: group })
end end
context 'when feature is not enabled' do it 'includes active users from the shared group to the billed members count' do
before do expect(group.billable_members_count).to eq(2)
stub_feature_flags(share_group_with_group: false)
end
it 'does not include users coming from the shared groups' do
expect(group.billable_members_count).to eq(1)
end
end
context 'when feature is enabled' do
before do
stub_feature_flags(share_group_with_group: true)
end
it 'includes active users from the shared group to the billed members count' do
expect(group.billable_members_count).to eq(2)
end
end end
end end
end end
...@@ -1260,24 +1211,8 @@ describe Namespace do ...@@ -1260,24 +1211,8 @@ describe Namespace do
shared_group: group }) shared_group: group })
end end
context 'when feature is not enabled' do it 'includes active users from the shared group including guests to the billed members count' do
before do expect(group.billable_members_count).to eq(4)
stub_feature_flags(share_group_with_group: false)
end
it 'does not include users coming from the shared groups' do
expect(group.billable_members_count).to eq(2)
end
end
context 'when feature is enabled' do
before do
stub_feature_flags(share_group_with_group: true)
end
it 'includes active users from the shared group including guests to the billed members count' do
expect(group.billable_members_count).to eq(4)
end
end end
end end
end end
......
...@@ -99,18 +99,6 @@ describe Groups::GroupLinksController do ...@@ -99,18 +99,6 @@ describe Groups::GroupLinksController do
expect(flash[:alert]).to eq('error') expect(flash[:alert]).to eq('error')
end end
end end
context 'when feature flag is disabled' do
before do
stub_feature_flags(share_group_with_group: false)
end
it 'renders 404' do
subject
expect(response).to have_gitlab_http_status(:not_found)
end
end
end end
context 'when user does not have access to the group' do context 'when user does not have access to the group' do
...@@ -184,18 +172,6 @@ describe Groups::GroupLinksController do ...@@ -184,18 +172,6 @@ describe Groups::GroupLinksController do
expect(response).to have_gitlab_http_status(:not_found) expect(response).to have_gitlab_http_status(:not_found)
end end
end end
context 'when feature flag is disabled' do
before do
stub_feature_flags(share_group_with_group: false)
end
it 'renders 404' do
subject
expect(response).to have_gitlab_http_status(:not_found)
end
end
end end
describe '#destroy' do describe '#destroy' do
...@@ -231,17 +207,5 @@ describe Groups::GroupLinksController do ...@@ -231,17 +207,5 @@ describe Groups::GroupLinksController do
expect(response).to have_gitlab_http_status(:not_found) expect(response).to have_gitlab_http_status(:not_found)
end end
end end
context 'when feature flag is disabled' do
before do
stub_feature_flags(share_group_with_group: false)
end
it 'renders 404' do
subject
expect(response).to have_gitlab_http_status(:not_found)
end
end
end end
end end
...@@ -15,75 +15,56 @@ describe 'Groups > Members > Manage groups', :js do ...@@ -15,75 +15,56 @@ describe 'Groups > Members > Manage groups', :js do
sign_in(user) sign_in(user)
end end
context 'with share groups with groups feature flag' do it 'add group to group' do
before do visit group_group_members_path(shared_group)
stub_feature_flags(shared_with_group: true)
end
it 'add group to group' do
visit group_group_members_path(shared_group)
add_group(shared_with_group.id, 'Reporter') add_group(shared_with_group.id, 'Reporter')
page.within(first_row) do page.within(first_row) do
expect(page).to have_content(shared_with_group.name) expect(page).to have_content(shared_with_group.name)
expect(page).to have_content('Reporter') expect(page).to have_content('Reporter')
end
end end
end
it 'remove user from group' do it 'remove user from group' do
create(:group_group_link, shared_group: shared_group, create(:group_group_link, shared_group: shared_group,
shared_with_group: shared_with_group, group_access: ::Gitlab::Access::DEVELOPER) shared_with_group: shared_with_group, group_access: ::Gitlab::Access::DEVELOPER)
visit group_group_members_path(shared_group)
expect(page).to have_content(shared_with_group.name)
accept_confirm do visit group_group_members_path(shared_group)
find(:css, '#existing_shares li', text: shared_with_group.name).find(:css, 'a.btn-remove').click
end
wait_for_requests expect(page).to have_content(shared_with_group.name)
expect(page).not_to have_content(shared_with_group.name) accept_confirm do
find(:css, '#existing_shares li', text: shared_with_group.name).find(:css, 'a.btn-remove').click
end end
it 'update group to owner level' do wait_for_requests
create(:group_group_link, shared_group: shared_group,
shared_with_group: shared_with_group, group_access: ::Gitlab::Access::DEVELOPER)
visit group_group_members_path(shared_group) expect(page).not_to have_content(shared_with_group.name)
end
page.within(first_row) do it 'update group to owner level' do
click_button('Developer') create(:group_group_link, shared_group: shared_group,
click_link('Maintainer') shared_with_group: shared_with_group, group_access: ::Gitlab::Access::DEVELOPER)
wait_for_requests visit group_group_members_path(shared_group)
expect(page).to have_button('Maintainer') page.within(first_row) do
end click_button('Developer')
end click_link('Maintainer')
def add_group(id, role) wait_for_requests
page.click_link 'Invite group'
page.within ".invite-group-form" do
select2(id, from: "#shared_with_group_id")
select(role, from: "shared_group_access")
click_button "Invite"
end
end
end
context 'without share groups with groups feature flag' do expect(page).to have_button('Maintainer')
before do
stub_feature_flags(share_group_with_group: false)
end end
end
it 'does not render invitation form and tabs' do def add_group(id, role)
visit group_group_members_path(shared_group) page.click_link 'Invite group'
page.within ".invite-group-form" do
expect(page).not_to have_link('Invite member') select2(id, from: "#shared_with_group_id")
expect(page).not_to have_link('Invite group') select(role, from: "shared_group_access")
click_button "Invite"
end end
end end
end end
...@@ -560,114 +560,72 @@ describe Group do ...@@ -560,114 +560,72 @@ describe Group do
group_access: GroupMember::DEVELOPER }) group_access: GroupMember::DEVELOPER })
end end
context 'when feature flag share_group_with_group is enabled' do context 'with user in the group' do
before do let(:user) { group_user }
stub_feature_flags(share_group_with_group: true)
end
context 'with user in the group' do
let(:user) { group_user }
it 'returns correct access level' do it 'returns correct access level' do
expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::DEVELOPER) expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::DEVELOPER)
expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::DEVELOPER) expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::DEVELOPER)
end
context 'with lower group access level than max access level for share' do
let(:user) { create(:user) }
it 'returns correct access level' do
group.add_reporter(user)
expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::REPORTER)
expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::REPORTER)
end
end
end end
context 'with user in the parent group' do context 'with lower group access level than max access level for share' do
let(:user) { parent_group_user } let(:user) { create(:user) }
it 'returns correct access level' do it 'returns correct access level' do
expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) group.add_reporter(user)
expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
end
end
context 'with user in the child group' do
let(:user) { child_group_user }
it 'returns correct access level' do
expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::REPORTER)
expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::REPORTER)
end end
end end
end
context 'unrelated project owner' do context 'with user in the parent group' do
let(:common_id) { [Project.maximum(:id).to_i, Namespace.maximum(:id).to_i].max + 999 } let(:user) { parent_group_user }
let!(:group) { create(:group, id: common_id) }
let!(:unrelated_project) { create(:project, id: common_id) }
let(:user) { unrelated_project.owner }
it 'returns correct access level' do it 'returns correct access level' do
expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
end
end end
end
context 'user without accepted access request' do context 'with user in the child group' do
let!(:user) { create(:user) } let(:user) { child_group_user }
before do
create(:group_member, :developer, :access_request, user: user, group: group)
end
it 'returns correct access level' do it 'returns correct access level' do
expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
end
end end
end end
context 'when feature flag share_group_with_group is disabled' do context 'unrelated project owner' do
before do let(:common_id) { [Project.maximum(:id).to_i, Namespace.maximum(:id).to_i].max + 999 }
stub_feature_flags(share_group_with_group: false) let!(:group) { create(:group, id: common_id) }
end let!(:unrelated_project) { create(:project, id: common_id) }
let(:user) { unrelated_project.owner }
context 'with user in the group' do
let(:user) { group_user }
it 'returns correct access level' do it 'returns correct access level' do
expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
end
end end
end
context 'with user in the parent group' do context 'user without accepted access request' do
let(:user) { parent_group_user } let!(:user) { create(:user) }
it 'returns correct access level' do before do
expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) create(:group_member, :developer, :access_request, user: user, group: group)
expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
end
end end
context 'with user in the child group' do it 'returns correct access level' do
let(:user) { child_group_user } expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
it 'returns correct access level' do expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
end
end end
end end
end end
...@@ -679,8 +637,6 @@ describe Group do ...@@ -679,8 +637,6 @@ describe Group do
let(:shared_group) { create(:group, :private, parent: shared_group_parent) } let(:shared_group) { create(:group, :private, parent: shared_group_parent) }
before do before do
stub_feature_flags(share_group_with_group: true)
group.add_owner(user) group.add_owner(user)
create(:group_group_link, { shared_with_group: group, create(:group_group_link, { shared_with_group: group,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment