Add frontend support for sharing groups with groups

Implement frontend support for sharing groups with groups
Unify templates for sharing projects and groups with groups

app/assets/javascripts/pages/groups/group_members/index/index.js

@@ -3,9 +3,12 @@
 import Members from 'ee_else_ce/members';
 import memberExpirationDate from '~/member_expiration_date';
 import UsersSelect from '~/users_select';
+import groupsSelect from '~/groups_select';
 
 document.addEventListener('DOMContentLoaded', () => {
   memberExpirationDate();
+  memberExpirationDate('.js-access-expiration-date-groups');
   new Members();
+  groupsSelect();
   new UsersSelect();
 });

app/assets/stylesheets/pages/members.scss

@@ -3,7 +3,7 @@
   border-bottom: 1px solid $border-color;
 }
 
-.users-project-form {
+.invite-users-form {
   .btn-success {
     margin-right: 10px;
   }

app/controllers/groups/group_links_controller.rb

@@ -3,6 +3,7 @@
 class Groups::GroupLinksController < Groups::ApplicationController
   before_action :check_feature_flag!
   before_action :authorize_admin_group!
+  before_action :group_link, only: [:update, :destroy]
 
   def create
     shared_with_group = Group.find(params[:shared_with_group_id]) if params[:shared_with_group_id].present?
@@ -22,12 +23,35 @@ class Groups::GroupLinksController < Groups::ApplicationController
     redirect_to group_group_members_path(group)
   end
 
+  def update
+    @group_link.update(group_link_params)
+  end
+
+  def destroy
+    Groups::GroupLinks::DestroyService.new(nil, nil).execute(@group_link)
+
+    respond_to do |format|
+      format.html do
+        redirect_to group_group_members_path(group), status: :found
+      end
+      format.js { head :ok }
+    end
+  end
+
   private
 
+  def group_link
+    @group_link ||= group.shared_with_group_links.find(params[:id])
+  end
+
   def group_link_create_params
     params.permit(:shared_group_access, :expires_at)
   end
 
+  def group_link_params
+    params.require(:group_link).permit(:group_access, :expires_at)
+  end
+
   def check_feature_flag!
     render_404 unless Feature.enabled?(:share_group_with_group)
   end

app/controllers/groups/group_members_controller.rb

@@ -20,28 +20,17 @@ class Groups::GroupMembersController < Groups::ApplicationController
                                   :override
 
   def index
-    can_manage_members = can?(current_user, :admin_group_member, @group)
-
     @sort = params[:sort].presence || sort_value_name
     @project = @group.projects.find(params[:project_id]) if params[:project_id]
     @members = find_members
 
     if can_manage_members
-      @invited_members = @members.invite
-      @invited_members = @invited_members.search_invite_email(params[:search_invited]) if params[:search_invited].present?
-      @invited_members = present_members(@invited_members.page(params[:invited_members_page]).per(MEMBER_PER_PAGE_LIMIT))
+      @skip_groups = @group.related_group_ids
+      @invited_members = present_invited_members(@members)
     end
 
     @members = @members.non_invite
-    @members = @members.search(params[:search]) if params[:search].present?
-    @members = @members.sort_by_attribute(@sort)
-
-    if can_manage_members && params[:two_factor].present?
-      @members = @members.filter_by_2fa(params[:two_factor])
-    end
-
-    @members = @members.page(params[:page]).per(MEMBER_PER_PAGE_LIMIT)
-    @members = present_members(@members)
+    @members = present_group_members(@members)
 
     @requesters = present_members(
       AccessRequestsFinder.new(@group).execute(current_user))
@@ -54,8 +43,30 @@ class Groups::GroupMembersController < Groups::ApplicationController
 
   private
 
+  def present_invited_members(members)
+    invited_members = members.invite
+
+    if params[:search_invited].present?
+      invited_members = invited_members.search_invite_email(params[:search_invited])
+    end
+
+    present_members(invited_members
+          .page(params[:invited_members_page])
+          .per(MEMBER_PER_PAGE_LIMIT))
+  end
+
   def find_members
-    GroupMembersFinder.new(@group).execute(include_relations: requested_relations)
+    filter_params = params.slice(:two_factor, :search).merge(sort: @sort)
+    GroupMembersFinder.new(@group, current_user).execute(include_relations: requested_relations, params: filter_params)
+  end
+
+  def can_manage_members
+    can?(current_user, :admin_group_member, @group)
+  end
+
+  def present_group_members(original_members)
+    members = original_members.page(params[:page]).per(MEMBER_PER_PAGE_LIMIT)
+    present_members(members)
   end
 end
 

app/finders/group_members_finder.rb

 # frozen_string_literal: true
 
 class GroupMembersFinder < UnionFinder
-  def initialize(group)
+  # Params can be any of the following:
+  #   two_factor: string. 'enabled' or 'disabled' are returning different set of data, other values are not effective.
+  #   sort:       string
+  #   search:     string
+
+  def initialize(group, user = nil)
     @group = group
+    @user = user
   end
 
   # rubocop: disable CodeReuse/ActiveRecord
-  def execute(include_relations: [:inherited, :direct])
-    group_members = @group.members
+  def execute(include_relations: [:inherited, :direct], params: {})
+    group_members = group.members
     relations = []
 
     return group_members if include_relations == [:direct]
 
     relations << group_members if include_relations.include?(:direct)
 
-    if include_relations.include?(:inherited) && @group.parent
+    if include_relations.include?(:inherited) && group.parent
       parents_members = GroupMember.non_request
-        .where(source_id: @group.ancestors.select(:id))
-        .where.not(user_id: @group.users.select(:id))
+        .where(source_id: group.ancestors.select(:id))
+        .where.not(user_id: group.users.select(:id))
 
       relations << parents_members
     end
 
     if include_relations.include?(:descendants)
       descendant_members = GroupMember.non_request
-        .where(source_id: @group.descendants.select(:id))
-        .where.not(user_id: @group.users.select(:id))
+        .where(source_id: group.descendants.select(:id))
+        .where.not(user_id: group.users.select(:id))
 
       relations << descendant_members
     end
 
-    find_union(relations, GroupMember)
+    members = find_union(relations, GroupMember)
+    filter_members(members, params)
   end
   # rubocop: enable CodeReuse/ActiveRecord
+
+  private
+
+  attr_reader :user, :group
+
+  def filter_members(members, params)
+    members = members.search(params[:search]) if params[:search].present?
+    members = members.sort_by_attribute(params[:sort]) if params[:sort].present?
+
+    if can_manage_members && params[:two_factor].present?
+      members = members.filter_by_2fa(params[:two_factor])
+    end
+
+    members
+  end
+
+  def can_manage_members
+    Ability.allowed?(user, :admin_group_member, group)
+  end
 end
 
 GroupMembersFinder.prepend_if_ee('EE::GroupMembersFinder')

app/helpers/groups/group_members_helper.rb

@@ -4,6 +4,10 @@ module Groups::GroupMembersHelper
   def group_member_select_options
     { multiple: true, class: 'input-clamp qa-member-select-field ', scope: :all, email_user: true }
   end
+
+  def render_invite_member_for_group(group, default_access_level)
+    render 'shared/members/invite_member', submit_url: group_group_members_path(group), access_levels: GroupMember.access_level_roles, default_access_level: default_access_level
+  end
 end
 
 Groups::GroupMembersHelper.prepend_if_ee('EE::Groups::GroupMembersHelper')

app/helpers/selects_helper.rb

@@ -85,7 +85,8 @@ module SelectsHelper
       first_user: opts[:first_user] && current_user ? current_user.username : false,
       current_user: opts[:current_user] || false,
       author_id: opts[:author_id] || '',
-      skip_users: opts[:skip_users] ? opts[:skip_users].map(&:id) : nil
+      skip_users: opts[:skip_users] ? opts[:skip_users].map(&:id) : nil,
+      qa_selector: opts[:qa_selector] || ''
     }
   end
 end

app/models/group.rb

@@ -420,6 +420,12 @@ class Group < Namespace
     GroupMember.where(source_id: self_and_ancestors_ids, user_id: user.id).order(:access_level).last
   end
 
+  def related_group_ids
+    [id,
+     *ancestors.pluck(:id),
+     *shared_with_group_links.pluck(:shared_with_group_id)]
+  end
+
   def hashed_storage?(_feature)
     false
   end

app/models/group_group_link.rb

@@ -20,4 +20,8 @@ class GroupGroupLink < ApplicationRecord
   def self.default_access
     Gitlab::Access::DEVELOPER
   end
+
+  def human_access
+    Gitlab::Access.human_access(self.group_access)
+  end
 end

app/models/project_group_link.rb

@@ -21,6 +21,8 @@ class ProjectGroupLink < ApplicationRecord
 
   after_commit :refresh_group_members_authorized_projects
 
+  alias_method :shared_with_group, :group
+
   def self.access_options
     Gitlab::Access.options
   end

app/views/groups/group_members/_new_group_member.html.haml

-= form_for @group_member, url: group_group_members_path(@group), html: { class: 'users-project-form users-group-form' } do |f|
-  .row
-    .col-md-4.col-lg-6
-      = users_select_tag(:user_ids, group_member_select_options)
-      .form-text.text-muted.append-bottom-10
-        Search for members by name, username, or email, or invite new ones using their email address.
-
-    .col-md-3.col-lg-2
-      = select_tag :access_level, options_for_select(GroupMember.access_level_roles, @group_member.access_level), class: "form-control project-access-select"
-      .form-text.text-muted.append-bottom-10
-        = link_to "Read more", help_page_path("user/permissions")
-        about role permissions
-
-    .col-md-3.col-lg-2
-      .clearable-input
-        = text_field_tag :expires_at, nil, class: 'form-control js-access-expiration-date', placeholder: 'Expiration date'
-        %i.clear-icon.js-clear-input
-      .form-text.text-muted.append-bottom-10
-        On this date, the member(s) will automatically lose access to this group and all of its projects.
-
-    .col-md-2
-      = f.submit 'Add to group', class: "btn btn-success btn-block", data: { qa_selector: 'add_to_group_button' }

app/views/groups/group_members/index.html.haml

-- page_title _("Members")
+- page_title _("Group members")
 - can_manage_members = can?(current_user, :admin_group_member, @group)
 - show_invited_members = can_manage_members && @invited_members.exists?
 - pending_active = params[:search_invited].present?
+- total_count = @members.count + @group.shared_with_group_links.count
 
 .project-members-page.prepend-top-default
   %h4
-    = _("Members")
+    = _("Group members")
   %hr
   - if can_manage_members
-    .project-members-new.append-bottom-default
-      %p.clearfix
-        = _("Add new member to %{strong_start}%{group_name}%{strong_end}").html_safe % { group_name: @group.name, strong_start: '<strong>'.html_safe, strong_end: '</strong>'.html_safe }
-      = render "new_group_member"
+    - if Feature.enabled?(:share_group_with_group)
+      %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' }
+            %a.nav-link{ href: '#invite-group-pane', id: 'invite-group-tab', data: { toggle: 'tab', qa_selector: 'invite_group_tab' }, role: 'tab' }= _("Invite group")
+      .tab-content.gitlab-tab-content
+        .tab-pane.active{ id: 'invite-member-pane', role: 'tabpanel' }
+          = render_invite_member_for_group(@group, @group_member.access_level)
+        - if Feature.enabled?(:share_group_with_group)
+          .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
 
@@ -19,10 +30,10 @@
 
   %ul.nav-links.mobile-separator.nav.nav-tabs.clearfix
     %li.nav-item
-      = link_to "#existing_members", class: ["nav-link", ("active" unless pending_active)] , 'data-toggle' => 'tab' do
+      = link_to "#existing_shares", class: ["nav-link", ("active" unless pending_active)] , 'data-toggle' => 'tab' do
         %span
-          = _("Existing")
-        %span.badge.badge-pill= @members.total_count
+          = _("Existing shares")
+        %span.badge.badge-pill= total_count
     - if show_invited_members
       %li.nav-item
         = link_to "#invited_members", class: ["nav-link", ("active" if pending_active)], 'data-toggle' => 'tab' do
@@ -31,7 +42,16 @@
           %span.badge.badge-pill= @invited_members.total_count
 
   .tab-content
-    #existing_members.tab-pane{ :class => ("active" unless pending_active) }
+    #existing_shares.tab-pane{ :class => ("active" unless pending_active) }
+      - if @group.shared_with_group_links.any?
+        .card.card-without-border
+          .d-flex.flex-column.flex-md-row.row-content-block.second-block
+            %span.flex-grow-1.align-self-md-center.col-form-label
+              = _("Groups with access to %{strong_start}%{group_name}%{strong_end}").html_safe % { group_name: @group.name, strong_start: '<strong>'.html_safe, strong_end: '</strong>'.html_safe }
+          %ul.content-list.members-list{ data: { qa_selector: "groups_list" } }
+            - can_admin_member = can?(current_user, :admin_group_member, @group)
+            - @group.shared_with_group_links.each do |group_link|
+              = render 'shared/members/group', group_link: group_link, can_admin_member: can_admin_member, group_link_path: group_group_link_path(@group, group_link)
       .card.card-without-border
         .d-flex.flex-column.flex-md-row.row-content-block.second-block
           %span.flex-grow-1.align-self-md-center.col-form-label
@@ -46,7 +66,7 @@
                 = label_tag '2fa', '2FA', class: 'col-form-label label-bold pr-md-2'
                 = render 'shared/members/filter_2fa_dropdown'
               = render 'shared/members/sort_dropdown'
-        %ul.content-list.members-list
+        %ul.content-list.members-list{ data: { qa_selector: "members_list" } }
           = render partial: 'shared/members/member', collection: @members, as: :member
       = paginate @members, theme: 'gitlab'
 

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

@@ -3,4 +3,6 @@
     = _("Groups with access to <strong>%{project_name}</strong>").html_safe % { project_name: sanitize(@project.name, tags: []) }
     %span.badge.badge-pill= group_links.size
   %ul.content-list.members-list
-    = render partial: 'shared/members/group', collection: group_links, as: :group_link
+    - can_admin_member = can?(current_user, :admin_project_member, @project)
+    - @group_links.each do |group_link|
+      = render 'shared/members/group', group_link: group_link, can_admin_member: can_admin_member, group_link_path: project_group_link_path(@project, group_link)

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

@@ -13,5 +13,5 @@
           %button.user-search-btn{ type: "submit", "aria-label" => _("Submit search") }
             = icon("search")
         = render 'shared/members/sort_dropdown'
-  %ul.content-list.members-list.qa-members-list
+  %ul.content-list.members-list{ data: { qa_selector: 'members_list' } }
     = render partial: 'shared/members/member', collection: members, as: :member

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

@@ -23,13 +23,13 @@
 
           .tab-content.gitlab-tab-content
             .tab-pane.active{ id: 'invite-member-pane', role: 'tabpanel' }
-              = render 'projects/project_members/new_project_member', tab_title: _('Invite member')
+              = render 'shared/members/invite_member', submit_url: project_project_members_path(@project), access_levels: ProjectMember.access_level_roles, default_access_level: @project_member.access_level, can_import_members?: can_import_members?, import_path: import_project_project_members_path(@project)
             .tab-pane{ id: 'invite-group-pane', role: 'tabpanel', class: ('active' if membership_locked?) }
-              = render 'projects/project_members/new_project_group', tab_title: _('Invite group')
+              = render 'shared/members/invite_group', submit_url: project_group_links_path(@project), access_levels: ProjectGroupLink.access_options, default_access_level: ProjectGroupLink.default_access, group_link_field: 'link_group_id', group_access_field: 'link_group_access'
         - elsif !membership_locked?
-          .invite-member= render 'projects/project_members/new_project_member', tab_title: _('Invite member')
+          .invite-member= render 'shared/members/invite_member', submit_url: project_project_members_path(@project), access_levels: ProjectMember.access_level_roles, default_access_level: @project_member.access_level, can_import_members?: can_import_members?, import_path: import_project_project_members_path(@project)
         - elsif @project.allowed_to_share_with_group?
-          .invite-group= render 'projects/project_members/new_project_group', tab_title: _('Invite group')
+          .invite-group= render 'shared/members/invite_group', access_levels: ProjectGroupLink.access_options, default_access_level: ProjectGroupLink.default_access, submit_url: project_group_links_path(@project), group_link_field: 'link_group_id', group_access_field: 'link_group_access'
 
         = render 'shared/members/requests', membership_source: @project, requesters: @requesters
         .clearfix

app/views/shared/members/_group.html.haml

 - group_link = local_assigns[:group_link]
-- group = group_link.group
-- can_admin_member = can?(current_user, :admin_project_member, @project)
+- group = group_link.shared_with_group
+- can_admin_member = local_assigns[:can_admin_member]
+- group_link_path = local_assigns[:group_link_path]
 - dom_id = "group_member_#{group_link.id}"
 
 -# Note this is just for groups. For individual members please see shared/members/_member
@@ -17,7 +18,7 @@
           %span{ class: ('text-warning' if group_link.expires_soon?) }
             = _("Expires in %{expires_at}").html_safe % { expires_at: distance_of_time_in_words_to_now(group_link.expires_at) }
   .controls.member-controls.align-items-center
-    = form_tag project_group_link_path(@project, group_link), method: :put, remote: true, class: 'js-edit-member-form form-group d-sm-flex' do
+    = form_tag group_link_path, method: :put, remote: true, class: 'js-edit-member-form form-group d-sm-flex' do
       = hidden_field_tag "group_link[group_access]", group_link.group_access
       .member-form-control.dropdown.mr-sm-2.d-sm-inline-block
         %button.dropdown-menu-toggle.js-member-permissions-dropdown{ type: "button",
@@ -39,7 +40,7 @@
         = text_field_tag 'group_link[expires_at]', group_link.expires_at, class: 'form-control js-access-expiration-date js-member-update-control', placeholder: _('Expiration date'), id: "member_expires_at_#{group.id}", disabled: !can_admin_member
         %i.clear-icon.js-clear-input
     - if can_admin_member
-      = link_to project_group_link_path(@project, group_link),
+      = link_to group_link_path,
         method: :delete,
         data: { confirm: _("Are you sure you want to remove %{group_name}?") % { group_name: group.name }, qa_selector: 'delete_group_access_link' },
         class: 'btn btn-remove m-0 ml-sm-2 align-self-center' do

app/views/projects/project_members/_new_project_group.html.haml → app/views/shared/members/_invite_group.html.haml

+- access_levels = local_assigns[:access_levels]
+- default_access_level = local_assigns[:default_access_level]
+- submit_url = local_assigns[:submit_url]
+- group_link_field = local_assigns[:group_link_field]
+- group_access_field = local_assigns[:group_access_field]
 .row
   .col-sm-12
-    = form_tag project_group_links_path(@project), class: 'js-requires-input', method: :post do
+    = form_tag submit_url, class: 'invite-group-form js-requires-input', method: :post do
       .form-group
-        = label_tag :link_group_id, _("Select a group to invite"), class: "label-bold"
-        = groups_select_tag(:link_group_id, data: { skip_groups: @skip_groups }, class: "input-clamp qa-group-select-field", required: true)
+        = label_tag group_link_field, _("Select a group to invite"), class: "label-bold"
+        = groups_select_tag(group_link_field, data: { skip_groups: @skip_groups }, class: 'input-clamp qa-group-select-field', required: true)
       .form-group
-        = label_tag :link_group_access, _("Max access level"), class: "label-bold"
+        = label_tag group_access_field, _("Max access level"), class: "label-bold"
         .select-wrapper
-          = select_tag :link_group_access, options_for_select(ProjectGroupLink.access_options, ProjectGroupLink.default_access), class: "form-control select-control"
+          = select_tag group_access_field, options_for_select(access_levels, default_access_level), data: { qa_selector: 'group_access_field' }, class: "form-control select-control"
           = icon('chevron-down')
         .form-text.text-muted.append-bottom-10
           - permissions_docs_path = help_page_path('user/permissions')

app/views/projects/project_members/_new_project_member.html.haml → app/views/shared/members/_invite_member.html.haml

+- access_levels = local_assigns[:access_levels]
+- default_access_level = local_assigns[:default_access_level]
+- submit_url = local_assigns[:submit_url]
+- can_import_members = local_assigns[:can_import_members?]
+- import_path = local_assigns[:import_path]
 .row
   .col-sm-12
-    = form_for @project_member, as: :project_member, url: project_project_members_path(@project), html: { class: 'users-project-form' } do |f|
+    = form_tag submit_url, class: 'invite-users-form', method: :post do
       .form-group
         = label_tag :user_ids, _("GitLab member or Email address"), class: "label-bold"
-        = users_select_tag(:user_ids, multiple: true, class: "input-clamp qa-member-select-input", scope: :all, email_user: true, placeholder: "Search for members to update or invite")
+        = users_select_tag(:user_ids, multiple: true, class: 'input-clamp qa-member-select-field', scope: :all, email_user: true, placeholder: 'Search for members to update or invite')
       .form-group
         = label_tag :access_level, _("Choose a role permission"), class: "label-bold"
         .select-wrapper
-          = select_tag :access_level, options_for_select(ProjectMember.access_level_roles, @project_member.access_level), class: "form-control project-access-select select-control"
+          = select_tag :access_level, options_for_select(access_levels, default_access_level), class: "form-control project-access-select select-control"
           = icon('chevron-down')
         .form-text.text-muted.append-bottom-10
           - permissions_docs_path = help_page_path('user/permissions')
@@ -18,6 +23,6 @@
           = label_tag :expires_at, _('Access expiration date'), class: 'label-bold'
           = text_field_tag :expires_at, nil, class: 'form-control js-access-expiration-date', placeholder: 'Expiration date'
           %i.clear-icon.js-clear-input
-      = f.submit _("Add to project"), class: "btn btn-success qa-add-member-button"
-      - if can_import_members?
-        = link_to _("Import"), import_project_project_members_path(@project), class: "btn btn-default", title: _("Import members from another project")
+      = submit_tag _("Invite"), class: "btn btn-success", data: { qa_selector: 'invite_member_button' }
+      - if can_import_members
+        = link_to _("Import"), import_path, class: "btn btn-default", title: _("Import members from another project")

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

@@ -28,7 +28,7 @@ describe "User manages members" do
       visit(project_project_members_path(project))
     end
 
-    it { expect(page).to have_no_button("Add members").and have_no_link("Import members") }
+    it { expect(page).to have_no_selector(".invite-users-form") }
   end
 
   context "as project maintainer" do

locale/gitlab.pot

@@ -1068,9 +1068,6 @@ msgstr ""
 msgid "Add new directory"
 msgstr ""
 
-msgid "Add new member to %{strong_start}%{group_name}%{strong_end}"
-msgstr ""
-
 msgid "Add or subtract spent time"
 msgstr ""
 
@@ -1095,9 +1092,6 @@ msgstr ""
 msgid "Add to merge train when pipeline succeeds"
 msgstr ""
 
-msgid "Add to project"
-msgstr ""
-
 msgid "Add to review"
 msgstr ""
 
@@ -7458,10 +7452,10 @@ msgstr ""
 msgid "Excluding merge commits. Limited to 6,000 commits."
 msgstr ""
 
-msgid "Existing"
+msgid "Existing members and groups"
 msgstr ""
 
-msgid "Existing members and groups"
+msgid "Existing shares"
 msgstr ""
 
 msgid "Expand"
@@ -9122,6 +9116,9 @@ msgstr ""
 msgid "Group maintainers can register group runners in the %{link}"
 msgstr ""
 
+msgid "Group members"
+msgstr ""
+
 msgid "Group name"
 msgstr ""
 
@@ -9392,6 +9389,9 @@ msgstr ""
 msgid "Groups can also be nested by creating %{subgroup_docs_link_start}subgroups%{subgroup_docs_link_end}."
 msgstr ""
 
+msgid "Groups with access to %{strong_start}%{group_name}%{strong_end}"
+msgstr ""
+
 msgid "Groups with access to <strong>%{project_name}</strong>"
 msgstr ""
 

qa/qa/page/group/sub_menus/members.rb

@@ -7,12 +7,9 @@ module QA
         class Members < Page::Base
           include Page::Component::UsersSelect
 
-          view 'app/views/groups/group_members/_new_group_member.html.haml' do
-            element :add_to_group_button
-          end
-
-          view 'app/helpers/groups/group_members_helper.rb' do
+          view 'app/views/shared/members/_invite_member.html.haml' do
             element :member_select_field
+            element :invite_member_button
           end
 
           view 'app/views/shared/members/_member.html.haml' do
@@ -24,7 +21,7 @@ module QA
 
           def add_member(username)
             select_user :member_select_field, username
-            click_element :add_to_group_button
+            click_element :invite_member_button
           end
 
           def update_access_level(username, access_level)

qa/qa/page/project/settings/members.rb

@@ -8,9 +8,9 @@ module QA
           include Page::Component::UsersSelect
           include QA::Page::Component::Select2
 
-          view 'app/views/projects/project_members/_new_project_member.html.haml' do
-            element :member_select_input
-            element :add_member_button
+          view 'app/views/shared/members/_invite_member.html.haml' do
+            element :member_select_field
+            element :invite_member_button
           end
 
           view 'app/views/projects/project_members/_team.html.haml' do
@@ -21,7 +21,7 @@ module QA
             element :invite_group_tab
           end
 
-          view 'app/views/projects/project_members/_new_project_group.html.haml' do
+          view 'app/views/shared/members/_invite_group.html.haml' do
             element :group_select_field
             element :invite_group_button
           end
@@ -43,8 +43,8 @@ module QA
           end
 
           def add_member(username)
-            select_user :member_select_input, username
-            click_element :add_member_button
+            select_user :member_select_field, username
+            click_element :invite_member_button
           end
 
           def remove_group(group_name)

spec/controllers/groups/group_links_controller_spec.rb

@@ -111,4 +111,100 @@ describe Groups::GroupLinksController do
       end
     end
   end
+
+  describe '#update' do
+    let!(:link) do
+      create(:group_group_link, { shared_group: shared_group,
+                                  shared_with_group: shared_with_group })
+    end
+
+    let(:expiry_date) { 1.month.from_now.to_date }
+
+    subject do
+      post(:update, params: { group_id: shared_group,
+                               id: link.id,
+                               group_link: { group_access: Gitlab::Access::GUEST,
+                                             expires_at: expiry_date } })
+    end
+
+    context 'when user has admin access to the shared group' do
+      before do
+        shared_group.add_owner(user)
+      end
+
+      it 'updates existing link' do
+        expect(link.group_access).to eq(Gitlab::Access::DEVELOPER)
+        expect(link.expires_at).to be_nil
+
+        subject
+
+        link.reload
+
+        expect(link.group_access).to eq(Gitlab::Access::GUEST)
+        expect(link.expires_at).to eq(expiry_date)
+      end
+    end
+
+    context 'when user does not have admin access to the shared group' do
+      it 'renders 404' do
+        subject
+
+        expect(response).to have_gitlab_http_status(404)
+      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(404)
+      end
+    end
+  end
+
+  describe '#destroy' do
+    let!(:link) do
+      create(:group_group_link, { shared_group: shared_group,
+                                  shared_with_group: shared_with_group })
+    end
+
+    subject do
+      post(:destroy, params: { group_id: shared_group,
+                               id: link.id })
+    end
+
+    context 'when user has admin access to the shared group' do
+      before do
+        shared_group.add_owner(user)
+      end
+
+      it 'deletes existing link' do
+        expect { subject }.to change(GroupGroupLink, :count).by(-1)
+      end
+    end
+
+    context 'when user does not have admin access to the shared group' do
+      it 'renders 404' do
+        subject
+
+        expect(response).to have_gitlab_http_status(404)
+      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(404)
+      end
+    end
+  end
 end

spec/controllers/groups/group_members_controller_spec.rb

@@ -31,6 +31,12 @@ describe Groups::GroupMembersController do
         expect(assigns(:invited_members).map(&:invite_email)).to match_array(invited.map(&:invite_email))
       end
 
+      it 'assigns skip groups' do
+        get :index, params: { group_id: group }
+
+        expect(assigns(:skip_groups)).to match_array(group.related_group_ids)
+      end
+
       it 'restricts search to one email' do
         get :index, params: { group_id: group, search_invited: invited.first.invite_email }
 

spec/features/admin/admin_groups_spec.rb

@@ -167,14 +167,14 @@ describe 'Admin Groups' do
     it 'adds admin a to a group as developer', :js do
       visit group_group_members_path(group)
 
-      page.within '.users-group-form' do
+      page.within '.invite-users-form' do
         select2(current_user.id, from: '#user_ids', multiple: true)
         select 'Developer', from: 'access_level'
       end
 
-      click_button 'Add to group'
+      click_button 'Invite'
 
-      page.within '.content-list' do
+      page.within '[data-qa-selector="members_list"]' do
         expect(page).to have_content(current_user.name)
         expect(page).to have_content('Developer')
       end
@@ -187,7 +187,7 @@ describe 'Admin Groups' do
 
       visit group_group_members_path(group)
 
-      page.within '.content-list' do
+      page.within '[data-qa-selector="members_list"]' do
         expect(page).to have_content(current_user.name)
         expect(page).to have_content('Developer')
       end
@@ -196,7 +196,7 @@ describe 'Admin Groups' do
 
       visit group_group_members_path(group)
 
-      page.within '.content-list' do
+      page.within '[data-qa-selector="members_list"]' do
         expect(page).not_to have_content(current_user.name)
         expect(page).not_to have_content('Developer')
       end

spec/features/admin/admin_projects_spec.rb

@@ -98,12 +98,12 @@ describe "Admin::Projects" do
     it 'adds admin a to a project as developer', :js do
       visit project_project_members_path(project)
 
-      page.within '.users-project-form' do
+      page.within '.invite-users-form' do
         select2(current_user.id, from: '#user_ids', multiple: true)
         select 'Developer', from: 'access_level'
       end
 
-      click_button 'Add to project'
+      click_button 'Invite'
 
       page.within '.content-list' do
         expect(page).to have_content(current_user.name)

spec/features/groups/members/manage_groups_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'Groups > Members > Manage groups', :js do
+  include Select2Helper
+  include Spec::Support::Helpers::Features::ListRowsHelpers
+
+  let(:user) { create(:user) }
+  let(:shared_with_group) { create(:group) }
+  let(:shared_group) { create(:group) }
+
+  before do
+    shared_group.add_owner(user)
+    sign_in(user)
+  end
+
+  context 'with share groups with groups feature flag' do
+    before do
+      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')
+
+      page.within(first_row) do
+        expect(page).to have_content(shared_with_group.name)
+        expect(page).to have_content('Reporter')
+      end
+    end
+
+    it 'remove user from group' do
+      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).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
+
+      wait_for_requests
+
+      expect(page).not_to have_content(shared_with_group.name)
+    end
+
+    it 'update group to owner level' do
+      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)
+
+      page.within(first_row) do
+        click_button('Developer')
+        click_link('Maintainer')
+
+        wait_for_requests
+
+        expect(page).to have_button('Maintainer')
+      end
+    end
+
+    def add_group(id, role)
+      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
+    before do
+      stub_feature_flags(share_group_with_group: false)
+    end
+
+    it 'does not render invitation form and tabs' do
+      visit group_group_members_path(shared_group)
+
+      expect(page).not_to have_link('Invite member')
+      expect(page).not_to have_link('Invite group')
+    end
+  end
+end

spec/features/groups/members/manage_members_spec.rb

@@ -113,7 +113,8 @@ describe 'Groups > Members > Manage members' do
 
     visit group_group_members_path(group)
 
-    expect(page).not_to have_button 'Add to group'
+    expect(page).not_to have_selector '.invite-users-form'
+    expect(page).not_to have_selector '.invite-group-form'
 
     page.within(second_row) do
       # Can not modify user2 role
@@ -125,11 +126,10 @@ describe 'Groups > Members > Manage members' do
   end
 
   def add_user(id, role)
-    page.within ".users-group-form" do
+    page.within ".invite-users-form" do
       select2(id, from: "#user_ids", multiple: true)
       select(role, from: "access_level")
+      click_button "Invite"
     end
-
-    click_button "Add to group"
   end
 end

spec/features/groups/members/search_members_spec.rb

@@ -24,7 +24,7 @@ describe 'Search group member' do
       find('.user-search-btn').click
     end
 
-    group_members_list = find(".card .content-list")
+    group_members_list = find('[data-qa-selector="members_list"]')
     expect(group_members_list).to have_content(member.name)
     expect(group_members_list).not_to have_content(user.name)
   end

spec/features/projects/members/list_spec.rb

@@ -87,12 +87,12 @@ describe 'Project members list' do
   end
 
   def add_user(id, role)
-    page.within ".users-project-form" do
+    page.within ".invite-users-form" do
       select2(id, from: "#user_ids", multiple: true)
       select(role, from: "access_level")
     end
 
-    click_button "Add to project"
+    click_button "Invite"
   end
 
   def visit_members_page

spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb

@@ -20,10 +20,10 @@ describe 'Projects > Members > Maintainer adds member with expiration date', :js
       date = 4.days.from_now
       visit project_project_members_path(project)
 
-      page.within '.users-project-form' do
+      page.within '.invite-users-form' do
         select2(new_member.id, from: '#user_ids', multiple: true)
         fill_in 'expires_at', with: date.to_s(:medium) + "\n"
-        click_on 'Add to project'
+        click_on 'Invite'
       end
 
       page.within "#project_member_#{new_member.project_members.first.id}" do

spec/features/projects/settings/user_manages_project_members_spec.rb

@@ -37,7 +37,7 @@ describe 'Projects > Settings > User manages project members' do
 
     visit(project_project_members_path(project))
 
-    page.within('.users-project-form') do
+    page.within('.invite-users-form') do
       click_link('Import')
     end
 

spec/finders/group_members_finder_spec.rb

@@ -10,6 +10,7 @@ describe GroupMembersFinder, '#execute' do
   let(:user2) { create(:user) }
   let(:user3) { create(:user) }
   let(:user4) { create(:user) }
+  let(:user5) { create(:user, :two_factor_via_otp) }
 
   it 'returns members for top-level group' do
     member1 = group.add_maintainer(user1)
@@ -67,4 +68,43 @@ describe GroupMembersFinder, '#execute' do
 
     expect(result.to_a).to match_array([member1, member2, member3, member4])
   end
+
+  it 'returns searched members if requested' do
+    group.add_maintainer(user2)
+    nested_group.add_maintainer(user2)
+    nested_group.add_maintainer(user3)
+    nested_group.add_maintainer(user4)
+    member = group.add_maintainer(user1)
+
+    result = described_class.new(group).execute(include_relations: [:direct, :descendants], params: { search: user1.name })
+
+    expect(result.to_a).to match_array([member])
+  end
+
+  it 'returns members with two-factor auth if requested by owner' do
+    group.add_owner(user2)
+    group.add_maintainer(user1)
+    nested_group.add_maintainer(user2)
+    nested_group.add_maintainer(user3)
+    nested_group.add_maintainer(user4)
+    member = group.add_maintainer(user5)
+
+    result = described_class.new(group, user2).execute(include_relations: [:direct, :descendants], params: { two_factor: 'enabled' })
+
+    expect(result.to_a).to contain_exactly(member)
+  end
+
+  it 'returns members without two-factor auth if requested by owner' do
+    member1 = group.add_owner(user2)
+    member2 = group.add_maintainer(user1)
+    nested_group.add_maintainer(user2)
+    member3 = nested_group.add_maintainer(user3)
+    member4 = nested_group.add_maintainer(user4)
+    member_with_2fa = group.add_maintainer(user5)
+
+    result = described_class.new(group, user2).execute(include_relations: [:direct, :descendants], params: { two_factor: 'disabled' })
+
+    expect(result.to_a).not_to include(member_with_2fa)
+    expect(result.to_a).to match_array([member1, member2, member3, member4])
+  end
 end

spec/models/group_group_link_spec.rb

@@ -33,4 +33,12 @@ describe GroupGroupLink do
         validate_inclusion_of(:group_access).in_array(Gitlab::Access.values))
     end
   end
+
+  describe '#human_access' do
+    it 'delegates to Gitlab::Access' do
+      expect(Gitlab::Access).to receive(:human_access).with(group_group_link.group_access)
+
+      group_group_link.human_access
+    end
+  end
 end

spec/models/group_spec.rb

@@ -1003,6 +1003,57 @@ describe Group do
     end
   end
 
+  describe '#related_group_ids' do
+    let(:nested_group) { create(:group, parent: group) }
+    let(:shared_with_group) { create(:group, parent: group) }
+
+    before do
+      create(:group_group_link, shared_group: nested_group,
+                                shared_with_group: shared_with_group)
+    end
+
+    subject(:related_group_ids) { nested_group.related_group_ids }
+
+    it 'returns id' do
+      expect(related_group_ids).to include(nested_group.id)
+    end
+
+    it 'returns ancestor id' do
+      expect(related_group_ids).to include(group.id)
+    end
+
+    it 'returns shared with group id' do
+      expect(related_group_ids).to include(shared_with_group.id)
+    end
+
+    context 'with more than one ancestor group' do
+      let(:ancestor_group) { create(:group) }
+
+      before do
+        group.update(parent: ancestor_group)
+      end
+
+      it 'returns all ancestor group ids' do
+        expect(related_group_ids).to(
+          include(group.id, ancestor_group.id))
+      end
+    end
+
+    context 'with more than one shared with group' do
+      let(:another_shared_with_group) { create(:group, parent: group) }
+
+      before do
+        create(:group_group_link, shared_group: nested_group,
+               shared_with_group: another_shared_with_group)
+      end
+
+      it 'returns all shared with group ids' do
+        expect(related_group_ids).to(
+          include(shared_with_group.id, another_shared_with_group.id))
+      end
+    end
+  end
+
   context 'with uploads' do
     it_behaves_like 'model with uploads', true do
       let(:model_object) { create(:group, :with_avatar) }

spec/support/shared_examples/features/master_manages_access_requests_shared_example.rb

@@ -25,7 +25,7 @@ RSpec.shared_examples 'Maintainer manages access requests' do
 
     expect_no_visible_access_request(entity, user)
 
-    page.within('.members-list') do
+    page.within('[data-qa-selector="members_list"]') do
       expect(page).to have_content user.name
     end
   end