Add latest changes from gitlab-org/gitlab@13-10-stable-eev13.10.0-rc40

5 files changed, 63 insertions, 39 deletions

diff --git a/spec/services/groups/destroy_service_spec.rb b/spec/services/groups/destroy_service_spec.rb
index 2f9bb72939a..a5fce315d91 100644
--- a/spec/services/groups/destroy_service_spec.rb
+++ b/spec/services/groups/destroy_service_spec.rb
@@ -229,10 +229,10 @@ RSpec.describe Groups::DestroyService do
           # will still be executed for the nested group as they fall under the same hierarchy
           # and hence we need to account for this scenario.
           expect(UserProjectAccessChangedService)
-            .to receive(:new).with(shared_with_group.user_ids_for_project_authorizations).and_call_original
+            .to receive(:new).with(shared_with_group.users_ids_of_direct_members).and_call_original
 
           expect(UserProjectAccessChangedService)
-            .not_to receive(:new).with(shared_group.user_ids_for_project_authorizations)
+            .not_to receive(:new).with(shared_group.users_ids_of_direct_members)
 
           destroy_group(shared_group, user, false)
         end
@@ -246,7 +246,7 @@ RSpec.describe Groups::DestroyService do
 
         it 'makes use of a specific service to update project authorizations' do
           expect(UserProjectAccessChangedService)
-            .to receive(:new).with(shared_with_group.user_ids_for_project_authorizations).and_call_original
+            .to receive(:new).with(shared_with_group.users_ids_of_direct_members).and_call_original
 
           destroy_group(shared_with_group, user, false)
         end
diff --git a/spec/services/groups/group_links/create_service_spec.rb b/spec/services/groups/group_links/create_service_spec.rb
index fb88433d8f6..df994b9f2a3 100644
--- a/spec/services/groups/group_links/create_service_spec.rb
+++ b/spec/services/groups/group_links/create_service_spec.rb
@@ -74,46 +74,56 @@ RSpec.describe Groups::GroupLinks::CreateService, '#execute' do
     end
   end
 
-  context 'group hierarchies' do
+  context 'project authorizations based on group hierarchies' do
     before do
       group_parent.add_owner(parent_group_user)
       group.add_owner(group_user)
       group_child.add_owner(child_group_user)
     end
 
-    context 'group user' do
-      let(:user) { group_user }
+    context 'project authorizations refresh' do
+      it 'is executed only for the direct members of the group' do
+        expect(UserProjectAccessChangedService).to receive(:new).with(contain_exactly(group_user.id)).and_call_original
 
-      it 'create proper authorizations' do
         subject.execute(shared_group)
-
-        expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey
-        expect(Ability.allowed?(user, :read_project, project)).to be_truthy
-        expect(Ability.allowed?(user, :read_project, project_child)).to be_truthy
       end
     end
 
-    context 'parent group user' do
-      let(:user) { parent_group_user }
+    context 'project authorizations' do
+      context 'group user' do
+        let(:user) { group_user }
 
-      it 'create proper authorizations' do
-        subject.execute(shared_group)
+        it 'create proper authorizations' do
+          subject.execute(shared_group)
 
-        expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey
-        expect(Ability.allowed?(user, :read_project, project)).to be_falsey
-        expect(Ability.allowed?(user, :read_project, project_child)).to be_falsey
+          expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey
+          expect(Ability.allowed?(user, :read_project, project)).to be_truthy
+          expect(Ability.allowed?(user, :read_project, project_child)).to be_truthy
+        end
       end
-    end
 
-    context 'child group user' do
-      let(:user) { child_group_user }
+      context 'parent group user' do
+        let(:user) { parent_group_user }
 
-      it 'create proper authorizations' do
-        subject.execute(shared_group)
+        it 'create proper authorizations' do
+          subject.execute(shared_group)
+
+          expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey
+          expect(Ability.allowed?(user, :read_project, project)).to be_falsey
+          expect(Ability.allowed?(user, :read_project, project_child)).to be_falsey
+        end
+      end
+
+      context 'child group user' do
+        let(:user) { child_group_user }
+
+        it 'create proper authorizations' do
+          subject.execute(shared_group)
 
-        expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey
-        expect(Ability.allowed?(user, :read_project, project)).to be_falsey
-        expect(Ability.allowed?(user, :read_project, project_child)).to be_falsey
+          expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey
+          expect(Ability.allowed?(user, :read_project, project)).to be_falsey
+          expect(Ability.allowed?(user, :read_project, project_child)).to be_falsey
+        end
       end
     end
   end
diff --git a/spec/services/groups/group_links/destroy_service_spec.rb b/spec/services/groups/group_links/destroy_service_spec.rb
index 22fe8a1d58b..97fe23e9147 100644
--- a/spec/services/groups/group_links/destroy_service_spec.rb
+++ b/spec/services/groups/group_links/destroy_service_spec.rb
@@ -47,8 +47,8 @@ RSpec.describe Groups::GroupLinks::DestroyService, '#execute' do
 
     it 'updates project authorization once per group' do
       expect(GroupGroupLink).to receive(:delete).and_call_original
-      expect(group).to receive(:refresh_members_authorized_projects).once
-      expect(another_group).to receive(:refresh_members_authorized_projects).once
+      expect(group).to receive(:refresh_members_authorized_projects).with(direct_members_only: true).once
+      expect(another_group).to receive(:refresh_members_authorized_projects).with(direct_members_only: true).once
 
       subject.execute(links)
     end
diff --git a/spec/services/groups/group_links/update_service_spec.rb b/spec/services/groups/group_links/update_service_spec.rb
index e4ff83d7926..436cdf89a0f 100644
--- a/spec/services/groups/group_links/update_service_spec.rb
+++ b/spec/services/groups/group_links/update_service_spec.rb
@@ -8,7 +8,7 @@ RSpec.describe Groups::GroupLinks::UpdateService, '#execute' do
   let_it_be(:group) { create(:group, :private) }
   let_it_be(:shared_group) { create(:group, :private) }
   let_it_be(:project) { create(:project, group: shared_group) }
-  let(:group_member) { create(:user) }
+  let(:group_member_user) { create(:user) }
   let!(:link) { create(:group_group_link, shared_group: shared_group, shared_with_group: group) }
 
   let(:expiry_date) { 1.month.from_now.to_date }
@@ -20,7 +20,7 @@ RSpec.describe Groups::GroupLinks::UpdateService, '#execute' do
   subject { described_class.new(link).execute(group_link_params) }
 
   before do
-    group.add_developer(group_member)
+    group.add_developer(group_member_user)
   end
 
   it 'updates existing link' do
@@ -36,11 +36,11 @@ RSpec.describe Groups::GroupLinks::UpdateService, '#execute' do
   end
 
   it 'updates project permissions' do
-    expect { subject }.to change { group_member.can?(:create_release, project) }.from(true).to(false)
+    expect { subject }.to change { group_member_user.can?(:create_release, project) }.from(true).to(false)
   end
 
   it 'executes UserProjectAccessChangedService' do
-    expect_next_instance_of(UserProjectAccessChangedService) do |service|
+    expect_next_instance_of(UserProjectAccessChangedService, [group_member_user.id]) do |service|
       expect(service).to receive(:execute)
     end
 
diff --git a/spec/services/groups/import_export/import_service_spec.rb b/spec/services/groups/import_export/import_service_spec.rb
index 0c7765dcd38..ad5c4364deb 100644
--- a/spec/services/groups/import_export/import_service_spec.rb
+++ b/spec/services/groups/import_export/import_service_spec.rb
@@ -54,7 +54,7 @@ RSpec.describe Groups::ImportExport::ImportService do
   end
 
   context 'with group_import_ndjson feature flag disabled' do
-    let(:user) { create(:admin) }
+    let(:user) { create(:user) }
     let(:group) { create(:group) }
     let(:import_logger) { instance_double(Gitlab::Import::Logger) }
 
@@ -63,6 +63,8 @@ RSpec.describe Groups::ImportExport::ImportService do
     before do
       stub_feature_flags(group_import_ndjson: false)
 
+      group.add_owner(user)
+
       ImportExportUpload.create!(group: group, import_file: import_file)
 
       allow(Gitlab::Import::Logger).to receive(:build).and_return(import_logger)
@@ -95,7 +97,7 @@ RSpec.describe Groups::ImportExport::ImportService do
     end
 
     context 'when importing a ndjson export' do
-      let(:user) { create(:admin) }
+      let(:user) { create(:user) }
       let(:group) { create(:group) }
       let(:service) { described_class.new(group: group, user: user) }
       let(:import_file) { fixture_file_upload('spec/fixtures/group_export.tar.gz') }
@@ -115,6 +117,10 @@ RSpec.describe Groups::ImportExport::ImportService do
       end
 
       context 'when user has correct permissions' do
+        before do
+          group.add_owner(user)
+        end
+
         it 'imports group structure successfully' do
           expect(subject).to be_truthy
         end
@@ -147,8 +153,6 @@ RSpec.describe Groups::ImportExport::ImportService do
       end
 
       context 'when user does not have correct permissions' do
-        let(:user) { create(:user) }
-
         it 'logs the error and raises an exception' do
           expect(import_logger).to receive(:error).with(
             group_id:   group.id,
@@ -188,6 +192,10 @@ RSpec.describe Groups::ImportExport::ImportService do
       context 'when there are errors with the sub-relations' do
         let(:import_file) { fixture_file_upload('spec/fixtures/group_export_invalid_subrelations.tar.gz') }
 
+        before do
+          group.add_owner(user)
+        end
+
         it 'successfully imports the group' do
           expect(subject).to be_truthy
         end
@@ -207,7 +215,7 @@ RSpec.describe Groups::ImportExport::ImportService do
     end
 
     context 'when importing a json export' do
-      let(:user) { create(:admin) }
+      let(:user) { create(:user) }
       let(:group) { create(:group) }
       let(:service) { described_class.new(group: group, user: user) }
       let(:import_file) { fixture_file_upload('spec/fixtures/legacy_group_export.tar.gz') }
@@ -227,6 +235,10 @@ RSpec.describe Groups::ImportExport::ImportService do
       end
 
       context 'when user has correct permissions' do
+        before do
+          group.add_owner(user)
+        end
+
         it 'imports group structure successfully' do
           expect(subject).to be_truthy
         end
@@ -259,8 +271,6 @@ RSpec.describe Groups::ImportExport::ImportService do
       end
 
       context 'when user does not have correct permissions' do
-        let(:user) { create(:user) }
-
         it 'logs the error and raises an exception' do
           expect(import_logger).to receive(:error).with(
             group_id:   group.id,
@@ -300,6 +310,10 @@ RSpec.describe Groups::ImportExport::ImportService do
       context 'when there are errors with the sub-relations' do
         let(:import_file) { fixture_file_upload('spec/fixtures/legacy_group_export_invalid_subrelations.tar.gz') }
 
+        before do
+          group.add_owner(user)
+        end
+
         it 'successfully imports the group' do
           expect(subject).to be_truthy
         end