Merge branch...

Merge branch '355939-administrator-can-t-use-invite-group-modal-to-invite-groups-unless-they-are-a-member' into 'master'

Fix allowing of groups and projects to invite groups for admin

See merge request gitlab-org/gitlab!83002

app/assets/javascripts/invite_members/components/group_select.vue

@@ -24,10 +24,6 @@ export default {
     prop: 'selectedGroup',
   },
   props: {
-    accessLevels: {
-      type: Object,
-      required: true,
-    },
     groupsFilter: {
       type: String,
       required: false,
@@ -58,13 +54,6 @@ export default {
     isFetchResultEmpty() {
       return this.groups.length === 0;
     },
-    defaultFetchOptions() {
-      return {
-        exclude_internal: true,
-        active: true,
-        min_access_level: this.accessLevels.Guest,
-      };
-    },
   },
   watch: {
     searchTerm() {
@@ -107,9 +96,13 @@ export default {
     fetchGroups() {
       switch (this.groupsFilter) {
         case GROUP_FILTERS.DESCENDANT_GROUPS:
-          return getDescendentGroups(this.parentGroupId, this.searchTerm, this.defaultFetchOptions);
+          return getDescendentGroups(
+            this.parentGroupId,
+            this.searchTerm,
+            this.$options.defaultFetchOptions,
+          );
         default:
-          return getGroups(this.searchTerm, this.defaultFetchOptions);
+          return getGroups(this.searchTerm, this.$options.defaultFetchOptions);
       }
     },
   },
@@ -118,6 +111,10 @@ export default {
     searchPlaceholder: s__('GroupSelect|Search groups'),
     emptySearchResult: s__('GroupSelect|No matching results'),
   },
+  defaultFetchOptions: {
+    exclude_internal: true,
+    active: true,
+  },
 };
 </script>
 <template>

app/assets/javascripts/invite_members/components/invite_groups_modal.vue

@@ -155,7 +155,6 @@ export default {
     <template #select>
       <group-select
         v-model="groupToBeSharedWith"
-        :access-levels="accessLevels"
         :groups-filter="groupSelectFilter"
         :parent-group-id="groupSelectParentId"
         :invalid-groups="invalidGroups"

spec/features/groups/members/manage_groups_spec.rb

@@ -3,7 +3,6 @@
 require 'spec_helper'
 
 RSpec.describe 'Groups > Members > Manage groups', :js do
-  include Select2Helper
   include Spec::Support::Helpers::Features::MembersHelpers
   include Spec::Support::Helpers::Features::InviteMembersModalHelper
   include Spec::Support::Helpers::ModalHelpers
@@ -119,16 +118,92 @@ RSpec.describe 'Groups > Members > Manage groups', :js do
 
   describe 'group search results' do
     let_it_be(:group, refind: true) { create(:group) }
-    let_it_be(:group_within_hierarchy) { create(:group, parent: group) }
-    let_it_be(:group_outside_hierarchy) { create(:group) }
 
-    before_all do
-      group.add_owner(user)
-      group_within_hierarchy.add_owner(user)
-      group_outside_hierarchy.add_owner(user)
+    context 'with instance admin considerations' do
+      let_it_be(:group_to_share) { create(:group) }
+
+      context 'when user is an admin' do
+        let_it_be(:admin) { create(:admin) }
+
+        before do
+          sign_in(admin)
+          gitlab_enable_admin_mode_sign_in(admin)
+        end
+
+        it 'shows groups where the admin has no direct membership' do
+          visit group_group_members_path(group)
+
+          click_on 'Invite a group'
+          click_on 'Select a group'
+          wait_for_requests
+
+          page.within(group_dropdown_selector) do
+            expect_to_have_group(group_to_share)
+            expect_not_to_have_group(group)
+          end
+        end
+
+        it 'shows groups where the admin has at least guest level membership' do
+          group_to_share.add_guest(admin)
+
+          visit group_group_members_path(group)
+
+          click_on 'Invite a group'
+          click_on 'Select a group'
+          wait_for_requests
+
+          page.within(group_dropdown_selector) do
+            expect_to_have_group(group_to_share)
+            expect_not_to_have_group(group)
+          end
+        end
+      end
+
+      context 'when user is not an admin' do
+        before do
+          group.add_owner(user)
+        end
+
+        it 'shows groups where the user has no direct membership' do
+          visit group_group_members_path(group)
+
+          click_on 'Invite a group'
+          click_on 'Select a group'
+          wait_for_requests
+
+          page.within(group_dropdown_selector) do
+            expect_not_to_have_group(group_to_share)
+            expect_not_to_have_group(group)
+          end
+        end
+
+        it 'shows groups where the user has at least guest level membership' do
+          group_to_share.add_guest(user)
+
+          visit group_group_members_path(group)
+
+          click_on 'Invite a group'
+          click_on 'Select a group'
+          wait_for_requests
+
+          page.within(group_dropdown_selector) do
+            expect_to_have_group(group_to_share)
+            expect_not_to_have_group(group)
+          end
+        end
+      end
     end
 
-    context 'when the invite members group modal is enabled' do
+    context 'when user is not an admin and there are hierarchy considerations' do
+      let_it_be(:group_within_hierarchy) { create(:group, parent: group) }
+      let_it_be(:group_outside_hierarchy) { create(:group) }
+
+      before_all do
+        group.add_owner(user)
+        group_within_hierarchy.add_owner(user)
+        group_outside_hierarchy.add_owner(user)
+      end
+
       it 'does not show self or ancestors', :aggregate_failures do
         group_sibbling = create(:group, parent: group)
         group_sibbling.add_owner(user)
@@ -139,40 +214,45 @@ RSpec.describe 'Groups > Members > Manage groups', :js do
         click_on 'Select a group'
         wait_for_requests
 
-        page.within('[data-testid="group-select-dropdown"]') do
-          expect(page).to have_selector("[entity-id='#{group_outside_hierarchy.id}']")
-          expect(page).to have_selector("[entity-id='#{group_sibbling.id}']")
-          expect(page).not_to have_selector("[entity-id='#{group.id}']")
-          expect(page).not_to have_selector("[entity-id='#{group_within_hierarchy.id}']")
+        page.within(group_dropdown_selector) do
+          expect_to_have_group(group_outside_hierarchy)
+          expect_to_have_group(group_sibbling)
+          expect_not_to_have_group(group)
+          expect_not_to_have_group(group_within_hierarchy)
         end
       end
-    end
 
-    context 'when sharing with groups outside the hierarchy is enabled' do
-      it 'shows groups within and outside the hierarchy in search results' do
-        visit group_group_members_path(group)
+      context 'when sharing with groups outside the hierarchy is enabled' do
+        it 'shows groups within and outside the hierarchy in search results' do
+          visit group_group_members_path(group)
 
-        click_on 'Invite a group'
-        click_on 'Select a group'
+          click_on 'Invite a group'
+          click_on 'Select a group'
+          wait_for_requests
 
-        expect(page).to have_text group_within_hierarchy.name
-        expect(page).to have_text group_outside_hierarchy.name
+          page.within(group_dropdown_selector) do
+            expect_to_have_group(group_within_hierarchy)
+            expect_to_have_group(group_outside_hierarchy)
+          end
+        end
       end
-    end
 
-    context 'when sharing with groups outside the hierarchy is disabled' do
-      before do
-        group.namespace_settings.update!(prevent_sharing_groups_outside_hierarchy: true)
-      end
+      context 'when sharing with groups outside the hierarchy is disabled' do
+        before do
+          group.namespace_settings.update!(prevent_sharing_groups_outside_hierarchy: true)
+        end
 
-      it 'shows only groups within the hierarchy in search results' do
-        visit group_group_members_path(group)
+        it 'shows only groups within the hierarchy in search results' do
+          visit group_group_members_path(group)
 
-        click_on 'Invite a group'
-        click_on 'Select a group'
+          click_on 'Invite a group'
+          click_on 'Select a group'
 
-        expect(page).to have_text group_within_hierarchy.name
-        expect(page).not_to have_text group_outside_hierarchy.name
+          page.within(group_dropdown_selector) do
+            expect_to_have_group(group_within_hierarchy)
+            expect_not_to_have_group(group_outside_hierarchy)
+          end
+        end
       end
     end
   end

spec/features/projects/members/invite_group_spec.rb

@@ -17,20 +17,18 @@ RSpec.describe 'Project > Members > Invite group', :js do
 
     visit project_project_members_path(project)
 
-    expect(page).to have_selector('button[data-test-id="invite-group-button"]')
+    expect(page).to have_selector(invite_group_selector)
   end
 
-  it 'does not display  the  button when visiting the page not signed in' do
+  it 'does not display the button when visiting the page not signed in' do
     project = create(:project, namespace: create(:group))
 
     visit project_project_members_path(project)
 
-    expect(page).not_to have_selector('button[data-test-id="invite-group-button"]')
+    expect(page).not_to have_selector(invite_group_selector)
   end
 
   describe 'Share with group lock' do
-    let(:invite_group_selector) { 'button[data-test-id="invite-group-button"]' }
-
     shared_examples 'the project can be shared with groups' do
       it 'the "Invite a group" button exists' do
         visit project_project_members_path(project)
@@ -158,21 +156,95 @@ RSpec.describe 'Project > Members > Invite group', :js do
   describe 'the groups dropdown' do
     let_it_be(:parent_group) { create(:group, :public) }
     let_it_be(:project_group) { create(:group, :public, parent: parent_group) }
-    let_it_be(:public_sub_subgroup) { create(:group, :public, parent: project_group) }
-    let_it_be(:public_sibbling_group) { create(:group, :public, parent: parent_group) }
-    let_it_be(:private_sibbling_group) { create(:group, :private, parent: parent_group) }
-    let_it_be(:private_membership_group) { create(:group, :private) }
-    let_it_be(:public_membership_group) { create(:group, :public) }
     let_it_be(:project) { create(:project, group: project_group) }
 
-    before do
-      private_membership_group.add_guest(maintainer)
-      public_membership_group.add_maintainer(maintainer)
+    context 'with instance admin considerations' do
+      let_it_be(:group_to_share) { create(:group) }
 
-      sign_in(maintainer)
+      context 'when user is an admin' do
+        let_it_be(:admin) { create(:admin) }
+
+        before do
+          sign_in(admin)
+          gitlab_enable_admin_mode_sign_in(admin)
+        end
+
+        it 'shows groups where the admin has no direct membership' do
+          visit project_project_members_path(project)
+
+          click_on 'Invite a group'
+          click_on 'Select a group'
+          wait_for_requests
+
+          page.within(group_dropdown_selector) do
+            expect_to_have_group(group_to_share)
+          end
+        end
+
+        it 'shows groups where the admin has at least guest level membership' do
+          group_to_share.add_guest(admin)
+
+          visit project_project_members_path(project)
+
+          click_on 'Invite a group'
+          click_on 'Select a group'
+          wait_for_requests
+
+          page.within(group_dropdown_selector) do
+            expect_to_have_group(group_to_share)
+          end
+        end
+      end
+
+      context 'when user is not an admin' do
+        before do
+          project.add_maintainer(maintainer)
+          sign_in(maintainer)
+        end
+
+        it 'does not show groups where the user has no direct membership' do
+          visit project_project_members_path(project)
+
+          click_on 'Invite a group'
+          click_on 'Select a group'
+          wait_for_requests
+
+          page.within(group_dropdown_selector) do
+            expect_not_to_have_group(group_to_share)
+          end
+        end
+
+        it 'shows groups where the user has at least guest level membership' do
+          group_to_share.add_guest(maintainer)
+
+          visit project_project_members_path(project)
+
+          click_on 'Invite a group'
+          click_on 'Select a group'
+          wait_for_requests
+
+          page.within(group_dropdown_selector) do
+            expect_to_have_group(group_to_share)
+          end
+        end
+      end
     end
 
     context 'for a project in a nested group' do
+      let_it_be(:public_sub_subgroup) { create(:group, :public, parent: project_group) }
+      let_it_be(:public_sibbling_group) { create(:group, :public, parent: parent_group) }
+      let_it_be(:private_sibbling_group) { create(:group, :private, parent: parent_group) }
+      let_it_be(:private_membership_group) { create(:group, :private) }
+      let_it_be(:public_membership_group) { create(:group, :public) }
+      let_it_be(:project) { create(:project, group: project_group) }
+
+      before do
+        private_membership_group.add_guest(maintainer)
+        public_membership_group.add_maintainer(maintainer)
+
+        sign_in(maintainer)
+      end
+
       it 'does not show the groups inherited from projects' do
         project.add_maintainer(maintainer)
         public_sibbling_group.add_maintainer(maintainer)
@@ -183,7 +255,7 @@ RSpec.describe 'Project > Members > Invite group', :js do
         click_on 'Select a group'
         wait_for_requests
 
-        page.within('[data-testid="group-select-dropdown"]') do
+        page.within(group_dropdown_selector) do
           expect_to_have_group(public_membership_group)
           expect_to_have_group(public_sibbling_group)
           expect_to_have_group(private_membership_group)
@@ -204,7 +276,7 @@ RSpec.describe 'Project > Members > Invite group', :js do
         click_on 'Select a group'
         wait_for_requests
 
-        page.within('[data-testid="group-select-dropdown"]') do
+        page.within(group_dropdown_selector) do
           expect_to_have_group(public_membership_group)
           expect_to_have_group(public_sibbling_group)
           expect_to_have_group(private_membership_group)
@@ -215,14 +287,10 @@ RSpec.describe 'Project > Members > Invite group', :js do
           expect_not_to_have_group(project_group)
         end
       end
-
-      def expect_to_have_group(group)
-        expect(page).to have_selector("[entity-id='#{group.id}']")
-      end
-
-      def expect_not_to_have_group(group)
-        expect(page).not_to have_selector("[entity-id='#{group.id}']")
-      end
     end
   end
+
+  def invite_group_selector
+    'button[data-test-id="invite-group-button"]'
+  end
 end

spec/frontend/invite_members/components/group_select_spec.js

@@ -4,7 +4,6 @@ import waitForPromises from 'helpers/wait_for_promises';
 import * as groupsApi from '~/api/groups_api';
 import GroupSelect from '~/invite_members/components/group_select.vue';
 
-const accessLevels = { Guest: 10, Reporter: 20, Developer: 30, Maintainer: 40, Owner: 50 };
 const group1 = { id: 1, full_name: 'Group One', avatar_url: 'test' };
 const group2 = { id: 2, full_name: 'Group Two', avatar_url: 'test' };
 const allGroups = [group1, group2];
@@ -13,7 +12,6 @@ const createComponent = (props = {}) => {
   return mount(GroupSelect, {
     propsData: {
       invalidGroups: [],
-      accessLevels,
       ...props,
     },
   });
@@ -66,9 +64,8 @@ describe('GroupSelect', () => {
       resolveApiRequest({ data: allGroups });
 
       expect(groupsApi.getGroups).toHaveBeenCalledWith(group1.name, {
-        active: true,
         exclude_internal: true,
-        min_access_level: accessLevels.Guest,
+        active: true,
       });
     });
 

spec/support/helpers/features/invite_members_modal_helper.rb

@@ -43,6 +43,18 @@ module Spec
 
             fill_in 'YYYY-MM-DD', with: expires_at.strftime('%Y-%m-%d') if expires_at
           end
+
+          def group_dropdown_selector
+            '[data-testid="group-select-dropdown"]'
+          end
+
+          def expect_to_have_group(group)
+            expect(page).to have_selector("[entity-id='#{group.id}']")
+          end
+
+          def expect_not_to_have_group(group)
+            expect(page).not_to have_selector("[entity-id='#{group.id}']")
+          end
         end
       end
     end