Merge branch...

Merge branch '325339-invite-members-modal-old-form-is-shown-when-member-lock-is-enabled' into 'master'

Fix invite group form being shown when member locked

See merge request gitlab-org/gitlab!60245

app/helpers/invite_members_helper.rb

@@ -11,6 +11,10 @@ module InviteMembersHelper
     Feature.enabled?(:invite_members_group_modal, project.group) && can_manage_project_members?(project)
   end
 
+  def can_invite_group_for_project?(project)
+    Feature.enabled?(:invite_members_group_modal, project.group) && project.allowed_to_share_with_group?
+  end
+
   def directly_invite_members?
     strong_memoize(:directly_invite_members) do
       can_import_members?

app/views/projects/project_members/index.html.haml

@@ -4,7 +4,7 @@
 .js-remove-member-modal
 .row.gl-mt-3
   .col-lg-12
-    - if can_invite_members_for_project?(@project)
+    - if can_invite_members_for_project?(@project) || can_invite_group_for_project?(@project)
       .row
         .col-md-12.col-lg-6.gl-display-flex
           .gl-flex-direction-column.gl-flex-wrap.align-items-baseline
@@ -18,11 +18,14 @@
                   = html_escape(_("Members can be added by project %{i_open}Maintainers%{i_close} or %{i_open}Owners%{i_close}")) % { i_open: '<i>'.html_safe, i_close: '</i>'.html_safe }
         .col-md-12.col-lg-6
           .gl-display-flex.gl-flex-wrap.gl-justify-content-end
+            - if can_import_members?
               = link_to _("Import a project"),
                 import_project_project_members_path(@project),
                 class: "btn btn-default btn-md gl-button gl-mt-3 gl-sm-w-auto gl-w-full",
                 title: _("Import members from another project")
+            - if @project.allowed_to_share_with_group?
               .js-invite-group-trigger{ data: { classes: 'gl-mt-3 gl-sm-w-auto gl-w-full gl-sm-ml-3', display_text: _('Invite a group') } }
+            - if !membership_locked?
               .js-invite-members-trigger{ data: { variant: 'success', classes: 'gl-mt-3 gl-sm-w-auto gl-w-full gl-sm-ml-3', display_text: _('Invite members') } }
         = render 'projects/invite_members_modal', project: @project
 
@@ -36,7 +39,7 @@
           %p
             = html_escape(_("Members can be added by project %{i_open}Maintainers%{i_close} or %{i_open}Owners%{i_close}")) % { i_open: '<i>'.html_safe, i_close: '</i>'.html_safe }
 
-    - if !can_invite_members_for_project?(@project) && can_manage_project_members?(@project) && project_can_be_shared?
+    - if Feature.disabled?(:invite_members_group_modal, @project.group) && can_manage_project_members?(@project) && project_can_be_shared?
       - if !membership_locked? && @project.allowed_to_share_with_group?
         %ul.nav-links.nav.nav-tabs.gitlab-tabs{ role: 'tablist' }
           %li.nav-tab{ role: 'presentation' }

ee/spec/features/projects/members/invite_group_and_members_spec.rb

@@ -2,15 +2,37 @@
 
 require 'spec_helper'
 
-RSpec.describe 'Project > Members > Invite group and members', :js do
+RSpec.describe 'Project > Members > Invite group and members' do
   include Select2Helper
   include ActionView::Helpers::DateHelper
   include Spec::Support::Helpers::Features::MembersHelpers
 
   let(:maintainer) { create(:user) }
 
+  using RSpec::Parameterized::TableSyntax
+
+  where(:invite_members_group_modal_enabled, :expected_invite_member_selector, :expected_invite_group_selector, :expected_import_button_text) do
+    true  | '.js-invite-members-trigger' | '.js-invite-group-trigger' | 'Import a project'
+    false | '#invite-member-tab'         | '#invite-group-tab'        | 'Import'
+  end
+
+  with_them do
     before do
-    stub_feature_flags(invite_members_group_modal: false)
+      stub_feature_flags(invite_members_group_modal: invite_members_group_modal_enabled)
+    end
+
+    it 'displays either the invite modal button triggers or the form with tabs based on the feature flag' do
+      project = create(:project, namespace: create(:group))
+
+      project.add_maintainer(maintainer)
+      sign_in(maintainer)
+
+      visit project_project_members_path(project)
+
+      expect(page).to have_selector(expected_invite_member_selector)
+      expect(page).to have_selector(expected_invite_group_selector)
+      expect(page).to have_link(expected_import_button_text)
+    end
   end
 
   describe 'Share group lock' do
@@ -18,10 +40,8 @@ RSpec.describe 'Project > Members > Invite group and members', :js do
       it 'user is only able to share with members' do
         visit project_project_members_path(project)
 
-        expect(page).not_to have_selector('#invite-member-tab')
-        expect(page).not_to have_selector('#invite-group-tab')
-        expect(page).not_to have_selector('.invite-group')
-        expect(page).to have_selector('.invite-member')
+        expect(page).not_to have_selector('.js-invite-group-trigger')
+        expect(page).to have_selector('.js-invite-members-trigger')
       end
     end
 
@@ -29,32 +49,26 @@ RSpec.describe 'Project > Members > Invite group and members', :js do
       it 'user is only able to share with groups' do
         visit project_project_members_path(project)
 
-        expect(page).not_to have_selector('#invite-member-tab')
-        expect(page).not_to have_selector('#invite-group-tab')
-        expect(page).not_to have_selector('.invite-member')
-        expect(page).to have_selector('.invite-group')
+        expect(page).not_to have_selector('.js-invite-members-trigger')
+        expect(page).to have_selector('.js-invite-group-trigger')
       end
     end
 
     shared_examples 'the project cannot be shared with groups and members' do
-      it 'no tabs or share content exists' do
+      it 'no invite member or invite group exists' do
         visit project_project_members_path(project)
 
-        expect(page).not_to have_selector('#invite-member-tab')
-        expect(page).not_to have_selector('#invite-group-tab')
-        expect(page).not_to have_selector('.invite-member')
-        expect(page).not_to have_selector('.invite-group')
+        expect(page).not_to have_selector('.js-invite-members-trigger')
+        expect(page).not_to have_selector('.js-invite-group-trigger')
       end
     end
 
     shared_examples 'the project can be shared with groups and members' do
-      it 'both member and group tabs exist' do
+      it 'both member and group buttons exist' do
         visit project_project_members_path(project)
 
-        expect(page).not_to have_selector('.invite-member')
-        expect(page).not_to have_selector('.invite-group')
-        expect(page).to have_selector('#invite-member-tab')
-        expect(page).to have_selector('#invite-group-tab')
+        expect(page).to have_selector('.js-invite-members-trigger')
+        expect(page).to have_selector('.js-invite-group-trigger')
       end
     end
 
@@ -70,44 +84,6 @@ RSpec.describe 'Project > Members > Invite group and members', :js do
 
       context 'when the group has "Share with group lock" and "Member lock" disabled' do
         it_behaves_like 'the project can be shared with groups and members'
-
-        it 'allows the project to be shared with another group using the invite form' do
-          stub_feature_flags(invite_members_group_modal: false)
-
-          visit project_project_members_path(project)
-
-          click_on 'invite-group-tab'
-
-          select2 group_to_share_with.id, from: '#link_group_id'
-          page.find('body').click
-          find('.btn-confirm').click
-
-          click_link 'Groups'
-
-          page.within(members_table) do
-            expect(page).to have_content(group_to_share_with.name)
-          end
-        end
-
-        it 'allows the project to be shared with another group using the invite modal' do
-          stub_feature_flags(invite_members_group_modal: true)
-
-          visit project_project_members_path(project)
-
-          click_on 'Invite a group'
-
-          click_on 'Select a group'
-          wait_for_requests
-          click_button group_to_share_with.name
-          click_button 'Invite'
-
-          visit project_project_members_path(project)
-          click_link 'Groups'
-
-          page.within(members_table) do
-            expect(page).to have_content(group_to_share_with.name)
-          end
-        end
       end
 
       context 'when the group has "Share with group lock" enabled' do

ee/spec/features/projects/settings/user_manages_members_spec.rb

@@ -8,7 +8,6 @@ RSpec.describe "User manages members" do
   let_it_be(:user) { create(:user) }
 
   before do
-    stub_feature_flags(invite_members_group_modal: false)
     sign_in(user)
   end
 
@@ -19,7 +18,9 @@ RSpec.describe "User manages members" do
       visit(project_project_members_path(project))
     end
 
-    it { expect(page).to have_link("Import members").and have_selector(".project-access-select") }
+    it { expect(page).to have_selector(".js-invite-members-trigger") }
+    it { expect(page).to have_selector(".js-invite-group-trigger") }
+    it { expect(page).to have_link("Import a project") }
   end
 
   shared_examples "when group membership is locked" do
@@ -29,11 +30,56 @@ RSpec.describe "User manages members" do
       visit(project_project_members_path(project))
     end
 
+    it { expect(page).to have_no_selector(".js-invite-members-trigger") }
+    it { expect(page).to have_selector(".js-invite-group-trigger") }
+  end
+
+  context "as project maintainer" do
+    before do
+      stub_feature_flags(invite_members_group_modal: true)
+      project.add_maintainer(user)
+    end
+
+    it_behaves_like "when group membership is unlocked"
+    it_behaves_like "when group membership is locked"
+  end
+
+  context "as group owner" do
+    before do
+      stub_feature_flags(invite_members_group_modal: true)
+      group.add_owner(user)
+    end
+
+    it_behaves_like "when group membership is unlocked"
+    it_behaves_like "when group membership is locked"
+  end
+
+  context 'when feature flag :invite_members_group_modal is disabled' do
+    shared_examples "when group membership is unlocked" do
+      before do
+        group.update!(membership_lock: false)
+
+        visit(project_project_members_path(project))
+      end
+
+      it { expect(page).to have_link("Import members").and have_selector(".project-access-select") }
+    end
+
+    shared_examples 'when group membership is locked' do
+      before do
+        group.update!(membership_lock: true)
+        project.add_maintainer(user)
+
+        visit(project_project_members_path(project))
+      end
+
       it { expect(page).to have_no_selector(".invite-users-form") }
+      it { expect(page).to have_selector(".invite-group-form") }
     end
 
     context "as project maintainer" do
       before do
+        stub_feature_flags(invite_members_group_modal: false)
         project.add_maintainer(user)
       end
 
@@ -43,10 +89,12 @@ RSpec.describe "User manages members" do
 
     context "as group owner" do
       before do
+        stub_feature_flags(invite_members_group_modal: false)
         group.add_owner(user)
       end
 
       it_behaves_like "when group membership is unlocked"
       it_behaves_like "when group membership is locked"
     end
+  end
 end

spec/features/groups/members/manage_groups_spec.rb

@@ -12,6 +12,7 @@ RSpec.describe 'Groups > Members > Manage groups', :js do
     sign_in(user)
   end
 
+  context 'with invite_members_group_modal disabled' do
     context 'when group link does not exist' do
       let_it_be(:group) { create(:group) }
       let_it_be(:group_to_add) { create(:group) }
@@ -33,6 +34,26 @@ RSpec.describe 'Groups > Members > Manage groups', :js do
         end
       end
     end
+  end
+
+  context 'when group link does not exist' do
+    it 'can share a group with group' do
+      group = create(:group)
+      group_to_add = create(:group)
+      group.add_owner(user)
+      group_to_add.add_owner(user)
+
+      visit group_group_members_path(group)
+      invite_group(group_to_add.name, 'Reporter')
+
+      click_groups_tab
+
+      page.within(first_row) do
+        expect(page).to have_content(group_to_add.name)
+        expect(page).to have_content('Reporter')
+      end
+    end
+  end
 
   context 'when group link exists' do
     let_it_be(:shared_with_group) { create(:group) }
@@ -126,6 +147,21 @@ RSpec.describe 'Groups > Members > Manage groups', :js do
     end
   end
 
+  def invite_group(name, role)
+    click_on 'Invite a group'
+
+    click_on 'Select a group'
+    wait_for_requests
+    click_button name
+
+    click_button 'Guest'
+    wait_for_requests
+    click_button role
+
+    click_button 'Invite'
+    page.refresh
+  end
+
   def click_groups_tab
     expect(page).to have_link 'Groups'
     click_link "Groups"

spec/features/projects/members/invite_group_spec.rb

@@ -9,22 +9,44 @@ RSpec.describe 'Project > Members > Invite group', :js do
 
   let(:maintainer) { create(:user) }
 
+  using RSpec::Parameterized::TableSyntax
+
+  where(:invite_members_group_modal_enabled, :expected_invite_group_selector) do
+    true  | 'button[data-qa-selector="invite_a_group_button"]'
+    false | '#invite-group-tab'
+  end
+
+  with_them do
     before do
-    stub_feature_flags(invite_members_group_modal: false)
+      stub_feature_flags(invite_members_group_modal: invite_members_group_modal_enabled)
+    end
+
+    it 'displays either the invite group button or the form with tabs based on the feature flag' do
+      project = create(:project, namespace: create(:group))
+
+      project.add_maintainer(maintainer)
+      sign_in(maintainer)
+
+      visit project_project_members_path(project)
+
+      expect(page).to have_selector(expected_invite_group_selector)
+    end
   end
 
   describe 'Share with group lock' do
+    let(:invite_group_selector) { 'button[data-qa-selector="invite_a_group_button"]' }
+
     shared_examples 'the project can be shared with groups' do
-      it 'the "Invite group" tab exists' do
+      it 'the "Invite a group" button exists' do
         visit project_project_members_path(project)
-        expect(page).to have_selector('#invite-group-tab')
+        expect(page).to have_selector(invite_group_selector)
       end
     end
 
     shared_examples 'the project cannot be shared with groups' do
-      it 'the "Invite group" tab does not exist' do
+      it 'the "Invite a group" button does not exist' do
         visit project_project_members_path(project)
-        expect(page).not_to have_selector('#invite-group-tab')
+        expect(page).not_to have_selector(invite_group_selector)
       end
     end
 
@@ -41,7 +63,9 @@ RSpec.describe 'Project > Members > Invite group', :js do
       context 'when the group has "Share with group lock" disabled' do
         it_behaves_like 'the project can be shared with groups'
 
-        it 'the project can be shared with another group' do
+        it 'the project can be shared with another group when the feature flag invite_members_group_modal is disabled' do
+          stub_feature_flags(invite_members_group_modal: false)
+
           visit project_project_members_path(project)
 
           expect(page).not_to have_link 'Groups'
@@ -56,6 +80,27 @@ RSpec.describe 'Project > Members > Invite group', :js do
 
           expect(members_table).to have_content(group_to_share_with.name)
         end
+
+        it 'the project can be shared with another group when the feature flag invite_members_group_modal is enabled' do
+          stub_feature_flags(invite_members_group_modal: true)
+
+          visit project_project_members_path(project)
+
+          expect(page).not_to have_link 'Groups'
+
+          click_on 'Invite a group'
+
+          click_on 'Select a group'
+          wait_for_requests
+          click_button group_to_share_with.name
+          click_button 'Invite'
+
+          visit project_project_members_path(project)
+
+          click_link 'Groups'
+
+          expect(members_table).to have_content(group_to_share_with.name)
+        end
       end
 
       context 'when the group has "Share with group lock" enabled' do
@@ -127,13 +172,14 @@ RSpec.describe 'Project > Members > Invite group', :js do
 
       visit project_project_members_path(project)
 
-      click_on 'invite-group-tab'
-
-      select2 group.id, from: '#link_group_id'
+      click_on 'Invite a group'
+      click_on 'Select a group'
+      wait_for_requests
+      click_button group.name
+      fill_in 'YYYY-MM-DD', with: 5.days.from_now.strftime('%Y-%m-%d')
+      click_button 'Invite'
 
-      fill_in 'expires_at_groups', with: 5.days.from_now.strftime('%Y-%m-%d')
-      click_on 'invite-group-tab'
-      find('.btn-confirm').click
+      page.refresh
     end
 
     it 'the group link shows the expiration time with a warning class' do
@@ -149,29 +195,23 @@ RSpec.describe 'Project > Members > Invite group', :js do
     context 'with multiple groups to choose from' do
       let(:project) { create(:project) }
 
-      before do
+      it 'includes multiple groups' do
         project.add_maintainer(maintainer)
         sign_in(maintainer)
 
-        create(:group).add_owner(maintainer)
-        create(:group).add_owner(maintainer)
+        group1 = create(:group)
+        group1.add_owner(maintainer)
+        group2 = create(:group)
+        group2.add_owner(maintainer)
 
         visit project_project_members_path(project)
 
-        click_link 'Invite group'
-
-        find('.ajax-groups-select.select2-container')
-
-        execute_script 'GROUP_SELECT_PER_PAGE = 1;'
-        open_select2 '#link_group_id'
-      end
-
-      it 'infinitely scrolls' do
-        expect(find('.select2-drop .select2-results')).to have_selector('.select2-result', count: 1)
-
-        scroll_select2_to_bottom('.select2-drop .select2-results:visible')
+        click_on 'Invite a group'
+        click_on 'Select a group'
+        wait_for_requests
 
-        expect(find('.select2-drop .select2-results')).to have_selector('.select2-result', count: 2)
+        expect(page).to have_button(group1.name)
+        expect(page).to have_button(group2.name)
       end
     end
 
@@ -188,16 +228,19 @@ RSpec.describe 'Project > Members > Invite group', :js do
         group_to_share_with.add_maintainer(maintainer)
       end
 
-      it 'the groups dropdown does not show ancestors' do
+      # This behavior should be changed to exclude the ancestor and project
+      # group from the options once issue is fixed for the modal:
+      # https://gitlab.com/gitlab-org/gitlab/-/issues/329835
+      it 'the groups dropdown does show ancestors and the project group' do
         visit project_project_members_path(project)
 
-        click_on 'invite-group-tab'
-        click_link 'Search for a group'
+        click_on 'Invite a group'
+        click_on 'Select a group'
+        wait_for_requests
 
-        page.within '.select2-drop' do
-          expect(page).to have_content(group_to_share_with.name)
-          expect(page).not_to have_content(group.name)
-        end
+        expect(page).to have_button(group_to_share_with.name)
+        expect(page).to have_button(group.name)
+        expect(page).to have_button(nested_group.name)
       end
     end
   end