Resolve "Improve handling of projects shared with a group"

5 files changed, 91 insertions, 72 deletions

diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb
index ae49490f31c..65d6cd1a295 100644
--- a/spec/controllers/groups_controller_spec.rb
+++ b/spec/controllers/groups_controller_spec.rb
@@ -38,14 +38,6 @@ describe GroupsController do
       project
     end
 
-    context 'as html' do
-      it 'assigns whether or not a group has children' do
-        get :show, id: group.to_param
-
-        expect(assigns(:has_children)).to be_truthy
-      end
-    end
-
     context 'as atom' do
       it 'assigns events for all the projects in the group' do
         create(:event, project: project)
diff --git a/spec/features/projects/members/share_with_group_spec.rb b/spec/features/projects/members/invite_group_spec.rb
index c6d85e5d22f..1ea4934cff1 100644
--- a/spec/features/projects/members/share_with_group_spec.rb
+++ b/spec/features/projects/members/invite_group_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe 'Project > Members > Share with Group', :js do
+describe 'Project > Members > Invite group', :js do
   include Select2Helper
   include ActionView::Helpers::DateHelper
 
@@ -8,17 +8,17 @@ describe 'Project > Members > Share with Group', :js do
 
   describe 'Share with group lock' do
     shared_examples 'the project can be shared with groups' do
-      it 'the "Share with group" tab exists' do
+      it 'the "Invite group" tab exists' do
         visit project_settings_members_path(project)
-        expect(page).to have_selector('#share-with-group-tab')
+        expect(page).to have_selector('#invite-group-tab')
       end
     end
 
     shared_examples 'the project cannot be shared with groups' do
-      it 'the "Share with group" tab does not exist' do
+      it 'the "Invite group" tab does not exist' do
         visit project_settings_members_path(project)
-        expect(page).to have_selector('#add-member-tab')
-        expect(page).not_to have_selector('#share-with-group-tab')
+        expect(page).to have_selector('#invite-member-tab')
+        expect(page).not_to have_selector('#invite-group-tab')
       end
     end
 
@@ -31,13 +31,13 @@ describe 'Project > Members > Share with Group', :js do
         sign_in(maintainer)
       end
 
-      context 'when the group has "Share with group lock" disabled' do
+      context 'when the group has "Invite group lock" disabled' do
         it_behaves_like 'the project can be shared with groups'
 
         it 'the project can be shared with another group' do
           visit project_settings_members_path(project)
 
-          click_on 'share-with-group-tab'
+          click_on 'invite-group-tab'
 
           select2 group_to_share_with.id, from: '#link_group_id'
           page.find('body').click
@@ -49,7 +49,7 @@ describe 'Project > Members > Share with Group', :js do
         end
       end
 
-      context 'when the group has "Share with group lock" enabled' do
+      context 'when the group has "Invite group lock" enabled' do
         before do
           project.namespace.update_column(:share_with_group_lock, true)
         end
@@ -69,12 +69,12 @@ describe 'Project > Members > Share with Group', :js do
         sign_in(maintainer)
       end
 
-      context 'when the root_group has "Share with group lock" disabled' do
-        context 'when the subgroup has "Share with group lock" disabled' do
+      context 'when the root_group has "Invite group lock" disabled' do
+        context 'when the subgroup has "Invite group lock" disabled' do
           it_behaves_like 'the project can be shared with groups'
         end
 
-        context 'when the subgroup has "Share with group lock" enabled' do
+        context 'when the subgroup has "Invite group lock" enabled' do
           before do
             subgroup.update_column(:share_with_group_lock, true)
           end
@@ -83,16 +83,16 @@ describe 'Project > Members > Share with Group', :js do
         end
       end
 
-      context 'when the root_group has "Share with group lock" enabled' do
+      context 'when the root_group has "Invite group lock" enabled' do
         before do
           root_group.update_column(:share_with_group_lock, true)
         end
 
-        context 'when the subgroup has "Share with group lock" disabled (parent overridden)' do
+        context 'when the subgroup has "Invite group lock" disabled (parent overridden)' do
           it_behaves_like 'the project can be shared with groups'
         end
 
-        context 'when the subgroup has "Share with group lock" enabled' do
+        context 'when the subgroup has "Invite group lock" enabled' do
           before do
             subgroup.update_column(:share_with_group_lock, true)
           end
@@ -117,12 +117,12 @@ describe 'Project > Members > Share with Group', :js do
 
       visit project_settings_members_path(project)
 
-      click_on 'share-with-group-tab'
+      click_on 'invite-group-tab'
 
       select2 group.id, from: '#link_group_id'
 
       fill_in 'expires_at_groups', with: (Time.now + 4.5.days).strftime('%Y-%m-%d')
-      click_on 'share-with-group-tab'
+      click_on 'invite-group-tab'
       find('.btn-create').click
     end
 
@@ -150,7 +150,7 @@ describe 'Project > Members > Share with Group', :js do
 
         visit project_settings_members_path(project)
 
-        click_link 'Share with group'
+        click_link 'Invite group'
 
         find('.ajax-groups-select.select2-container')
 
@@ -183,7 +183,7 @@ describe 'Project > Members > Share with Group', :js do
       it 'the groups dropdown does not show ancestors', :nested_groups do
         visit project_settings_members_path(project)
 
-        click_on 'share-with-group-tab'
+        click_on 'invite-group-tab'
         click_link 'Search for a group'
 
         page.within '.select2-drop' do
diff --git a/spec/features/projects/settings/user_manages_group_links_spec.rb b/spec/features/projects/settings/user_manages_group_links_spec.rb
index 2f1824d7849..676659b90c3 100644
--- a/spec/features/projects/settings/user_manages_group_links_spec.rb
+++ b/spec/features/projects/settings/user_manages_group_links_spec.rb
@@ -26,13 +26,13 @@ describe 'Projects > Settings > User manages group links' do
     end
   end
 
-  it 'shares a project with a group', :js do
-    click_link('Share with group')
+  it 'invites a group to a project', :js do
+    click_link('Invite group')
 
     select2(group_market.id, from: '#link_group_id')
     select('Maintainer', from: 'link_group_access')
 
-    click_button('Share')
+    click_button('Invite')
 
     page.within('.project-members-groups') do
       expect(page).to have_content('Market')
diff --git a/spec/finders/group_descendants_finder_spec.rb b/spec/finders/group_descendants_finder_spec.rb
index 796d40cb625..c64abdc3619 100644
--- a/spec/finders/group_descendants_finder_spec.rb
+++ b/spec/finders/group_descendants_finder_spec.rb
@@ -108,6 +108,15 @@ describe GroupDescendantsFinder do
         end
       end
     end
+
+    it 'does not include projects shared with the group' do
+      project = create(:project, namespace: group)
+      other_project = create(:project)
+      other_project.project_group_links.create(group: group,
+                                               group_access: ProjectGroupLink::MASTER)
+
+      expect(finder.execute).to contain_exactly(project)
+    end
   end
 
   context 'with nested groups', :nested_groups do
diff --git a/spec/javascripts/groups/components/app_spec.js b/spec/javascripts/groups/components/app_spec.js
index 03d4b472b87..76933cf337b 100644
--- a/spec/javascripts/groups/components/app_spec.js
+++ b/spec/javascripts/groups/components/app_spec.js
@@ -9,9 +9,14 @@ import GroupsStore from '~/groups/store/groups_store';
 import GroupsService from '~/groups/service/groups_service';
 
 import {
-  mockEndpoint, mockGroups, mockSearchedGroups,
-  mockRawPageInfo, mockParentGroupItem, mockRawChildren,
-  mockChildren, mockPageInfo,
+  mockEndpoint,
+  mockGroups,
+  mockSearchedGroups,
+  mockRawPageInfo,
+  mockParentGroupItem,
+  mockRawChildren,
+  mockChildren,
+  mockPageInfo,
 } from '../mock_data';
 
 const createComponent = (hideProjects = false) => {
@@ -28,22 +33,23 @@ const createComponent = (hideProjects = false) => {
   });
 };
 
-const returnServicePromise = (data, failed) => new Promise((resolve, reject) => {
-  if (failed) {
-    reject(data);
-  } else {
-    resolve({
-      json() {
-        return data;
-      },
-    });
-  }
-});
+const returnServicePromise = (data, failed) =>
+  new Promise((resolve, reject) => {
+    if (failed) {
+      reject(data);
+    } else {
+      resolve({
+        json() {
+          return data;
+        },
+      });
+    }
+  });
 
 describe('AppComponent', () => {
   let vm;
 
-  beforeEach((done) => {
+  beforeEach(done => {
     Vue.component('group-folder', groupFolderComponent);
     Vue.component('group-item', groupItemComponent);
 
@@ -94,7 +100,7 @@ describe('AppComponent', () => {
     });
 
     describe('fetchGroups', () => {
-      it('should call `getGroups` with all the params provided', (done) => {
+      it('should call `getGroups` with all the params provided', done => {
         spyOn(vm.service, 'getGroups').and.returnValue(returnServicePromise(mockGroups));
 
         vm.fetchGroups({
@@ -110,8 +116,10 @@ describe('AppComponent', () => {
         }, 0);
       });
 
-      it('should set headers to store for building pagination info when called with `updatePagination`', (done) => {
-        spyOn(vm.service, 'getGroups').and.returnValue(returnServicePromise({ headers: mockRawPageInfo }));
+      it('should set headers to store for building pagination info when called with `updatePagination`', done => {
+        spyOn(vm.service, 'getGroups').and.returnValue(
+          returnServicePromise({ headers: mockRawPageInfo }),
+        );
         spyOn(vm, 'updatePagination');
 
         vm.fetchGroups({ updatePagination: true });
@@ -122,7 +130,7 @@ describe('AppComponent', () => {
         }, 0);
       });
 
-      it('should show flash error when request fails', (done) => {
+      it('should show flash error when request fails', done => {
         spyOn(vm.service, 'getGroups').and.returnValue(returnServicePromise(null, true));
         spyOn($, 'scrollTo');
         spyOn(window, 'Flash');
@@ -138,7 +146,7 @@ describe('AppComponent', () => {
     });
 
     describe('fetchAllGroups', () => {
-      it('should fetch default set of groups', (done) => {
+      it('should fetch default set of groups', done => {
         spyOn(vm, 'fetchGroups').and.returnValue(returnServicePromise(mockGroups));
         spyOn(vm, 'updatePagination').and.callThrough();
         spyOn(vm, 'updateGroups').and.callThrough();
@@ -153,7 +161,7 @@ describe('AppComponent', () => {
         }, 0);
       });
 
-      it('should fetch matching set of groups when app is loaded with search query', (done) => {
+      it('should fetch matching set of groups when app is loaded with search query', done => {
         spyOn(vm, 'fetchGroups').and.returnValue(returnServicePromise(mockSearchedGroups));
         spyOn(vm, 'updateGroups').and.callThrough();
 
@@ -173,7 +181,7 @@ describe('AppComponent', () => {
     });
 
     describe('fetchPage', () => {
-      it('should fetch groups for provided page details and update window state', (done) => {
+      it('should fetch groups for provided page details and update window state', done => {
         spyOn(vm, 'fetchGroups').and.returnValue(returnServicePromise(mockGroups));
         spyOn(vm, 'updateGroups').and.callThrough();
         const mergeUrlParams = spyOnDependency(appComponent, 'mergeUrlParams').and.callThrough();
@@ -193,9 +201,13 @@ describe('AppComponent', () => {
           expect(vm.isLoading).toBe(false);
           expect($.scrollTo).toHaveBeenCalledWith(0);
           expect(mergeUrlParams).toHaveBeenCalledWith({ page: 2 }, jasmine.any(String));
-          expect(window.history.replaceState).toHaveBeenCalledWith({
-            page: jasmine.any(String),
-          }, jasmine.any(String), jasmine.any(String));
+          expect(window.history.replaceState).toHaveBeenCalledWith(
+            {
+              page: jasmine.any(String),
+            },
+            jasmine.any(String),
+            jasmine.any(String),
+          );
           expect(vm.updateGroups).toHaveBeenCalled();
           done();
         }, 0);
@@ -211,7 +223,7 @@ describe('AppComponent', () => {
         groupItem.isChildrenLoading = false;
       });
 
-      it('should fetch children of given group and expand it if group is collapsed and children are not loaded', (done) => {
+      it('should fetch children of given group and expand it if group is collapsed and children are not loaded', done => {
         spyOn(vm, 'fetchGroups').and.returnValue(returnServicePromise(mockRawChildren));
         spyOn(vm.store, 'setGroupChildren');
 
@@ -244,7 +256,7 @@ describe('AppComponent', () => {
         expect(groupItem.isOpen).toBe(false);
       });
 
-      it('should set `isChildrenLoading` back to `false` if load request fails', (done) => {
+      it('should set `isChildrenLoading` back to `false` if load request fails', done => {
         spyOn(vm, 'fetchGroups').and.returnValue(returnServicePromise({}, true));
 
         vm.toggleChildren(groupItem);
@@ -272,7 +284,9 @@ describe('AppComponent', () => {
         expect(vm.groupLeaveConfirmationMessage).toBe('');
         vm.showLeaveGroupModal(group, mockParentGroupItem);
         expect(vm.showModal).toBe(true);
-        expect(vm.groupLeaveConfirmationMessage).toBe(`Are you sure you want to leave the "${group.fullName}" group?`);
+        expect(vm.groupLeaveConfirmationMessage).toBe(
+          `Are you sure you want to leave the "${group.fullName}" group?`,
+        );
       });
     });
 
@@ -299,7 +313,7 @@ describe('AppComponent', () => {
         vm.targetParentGroup = groupItem;
       });
 
-      it('hides modal confirmation leave group and remove group item from tree', (done) => {
+      it('hides modal confirmation leave group and remove group item from tree', done => {
         const notice = `You left the "${childGroupItem.fullName}" group.`;
         spyOn(vm.service, 'leaveGroup').and.returnValue(returnServicePromise({ notice }));
         spyOn(vm.store, 'removeGroup').and.callThrough();
@@ -318,9 +332,11 @@ describe('AppComponent', () => {
         }, 0);
       });
 
-      it('should show error flash message if request failed to leave group', (done) => {
+      it('should show error flash message if request failed to leave group', done => {
         const message = 'An error occurred. Please try again.';
-        spyOn(vm.service, 'leaveGroup').and.returnValue(returnServicePromise({ status: 500 }, true));
+        spyOn(vm.service, 'leaveGroup').and.returnValue(
+          returnServicePromise({ status: 500 }, true),
+        );
         spyOn(vm.store, 'removeGroup').and.callThrough();
         spyOn(window, 'Flash');
 
@@ -335,9 +351,11 @@ describe('AppComponent', () => {
         }, 0);
       });
 
-      it('should show appropriate error flash message if request forbids to leave group', (done) => {
+      it('should show appropriate error flash message if request forbids to leave group', done => {
         const message = 'Failed to leave the group. Please make sure you are not the only owner.';
-        spyOn(vm.service, 'leaveGroup').and.returnValue(returnServicePromise({ status: 403 }, true));
+        spyOn(vm.service, 'leaveGroup').and.returnValue(
+          returnServicePromise({ status: 403 }, true),
+        );
         spyOn(vm.store, 'removeGroup').and.callThrough();
         spyOn(window, 'Flash');
 
@@ -388,7 +406,7 @@ describe('AppComponent', () => {
   });
 
   describe('created', () => {
-    it('should bind event listeners on eventHub', (done) => {
+    it('should bind event listeners on eventHub', done => {
       spyOn(eventHub, '$on');
 
       const newVm = createComponent();
@@ -405,21 +423,21 @@ describe('AppComponent', () => {
       });
     });
 
-    it('should initialize `searchEmptyMessage` prop with correct string when `hideProjects` is `false`', (done) => {
+    it('should initialize `searchEmptyMessage` prop with correct string when `hideProjects` is `false`', done => {
       const newVm = createComponent();
       newVm.$mount();
       Vue.nextTick(() => {
-        expect(newVm.searchEmptyMessage).toBe('Sorry, no groups or projects matched your search');
+        expect(newVm.searchEmptyMessage).toBe('No groups or projects matched your search');
         newVm.$destroy();
         done();
       });
     });
 
-    it('should initialize `searchEmptyMessage` prop with correct string when `hideProjects` is `true`', (done) => {
+    it('should initialize `searchEmptyMessage` prop with correct string when `hideProjects` is `true`', done => {
       const newVm = createComponent(true);
       newVm.$mount();
       Vue.nextTick(() => {
-        expect(newVm.searchEmptyMessage).toBe('Sorry, no groups matched your search');
+        expect(newVm.searchEmptyMessage).toBe('No groups matched your search');
         newVm.$destroy();
         done();
       });
@@ -427,7 +445,7 @@ describe('AppComponent', () => {
   });
 
   describe('beforeDestroy', () => {
-    it('should unbind event listeners on eventHub', (done) => {
+    it('should unbind event listeners on eventHub', done => {
       spyOn(eventHub, '$off');
 
       const newVm = createComponent();
@@ -454,7 +472,7 @@ describe('AppComponent', () => {
       vm.$destroy();
     });
 
-    it('should render loading icon', (done) => {
+    it('should render loading icon', done => {
       vm.isLoading = true;
       Vue.nextTick(() => {
         expect(vm.$el.querySelector('.loading-animation')).toBeDefined();
@@ -463,7 +481,7 @@ describe('AppComponent', () => {
       });
     });
 
-    it('should render groups tree', (done) => {
+    it('should render groups tree', done => {
       vm.store.state.groups = [mockParentGroupItem];
       vm.isLoading = false;
       vm.store.state.pageInfo = mockPageInfo;
@@ -473,7 +491,7 @@ describe('AppComponent', () => {
       });
     });
 
-    it('renders modal confirmation dialog', (done) => {
+    it('renders modal confirmation dialog', done => {
       vm.groupLeaveConfirmationMessage = 'Are you sure you want to leave the "foo" group?';
       vm.showModal = true;
       Vue.nextTick(() => {