Add latest changes from gitlab-org/gitlab@15-0-stable-eev15.0.0-rc42

4 files changed, 185 insertions, 176 deletions

diff --git a/spec/services/groups/group_links/create_service_spec.rb b/spec/services/groups/group_links/create_service_spec.rb
index 03dac14be54..bfbaedbd06f 100644
--- a/spec/services/groups/group_links/create_service_spec.rb
+++ b/spec/services/groups/group_links/create_service_spec.rb
@@ -3,23 +3,13 @@
 require 'spec_helper'
 
 RSpec.describe Groups::GroupLinks::CreateService, '#execute' do
-  let(:parent_group_user) { create(:user) }
-  let(:group_user) { create(:user) }
-  let(:child_group_user) { create(:user) }
-  let(:prevent_sharing) { false }
+  let_it_be(:shared_with_group_parent) { create(:group, :private) }
+  let_it_be(:shared_with_group) { create(:group, :private, parent: shared_with_group_parent) }
+  let_it_be(:shared_with_group_child) { create(:group, :private, parent: shared_with_group) }
 
   let_it_be(:group_parent) { create(:group, :private) }
-  let_it_be(:group) { create(:group, :private, parent: group_parent) }
-  let_it_be(:group_child) { create(:group, :private, parent: group) }
 
-  let(:ns_for_parent) { create(:namespace_settings, prevent_sharing_groups_outside_hierarchy: prevent_sharing) }
-  let(:shared_group_parent) { create(:group, :private, namespace_settings: ns_for_parent) }
-  let(:shared_group) { create(:group, :private, parent: shared_group_parent) }
-  let(:shared_group_child) { create(:group, :private, parent: shared_group) }
-
-  let(:project_parent) { create(:project, group: shared_group_parent) }
-  let(:project) { create(:project, group: shared_group) }
-  let(:project_child) { create(:project, group: shared_group_child) }
+  let(:group) { create(:group, :private, parent: group_parent) }
 
   let(:opts) do
     {
@@ -28,127 +18,161 @@ RSpec.describe Groups::GroupLinks::CreateService, '#execute' do
     }
   end
 
-  let(:user) { group_user }
+  subject { described_class.new(group, shared_with_group, user, opts) }
 
-  subject { described_class.new(shared_group, group, user, opts) }
+  shared_examples_for 'not shareable' do
+    it 'does not share and returns an error' do
+      expect do
+        result = subject.execute
 
-  before do
-    group.add_guest(group_user)
-    shared_group.add_owner(group_user)
+        expect(result[:status]).to eq(:error)
+        expect(result[:http_status]).to eq(404)
+      end.not_to change { group.shared_with_group_links.count }
+    end
   end
 
-  it 'adds group to another group' do
-    expect { subject.execute }.to change { group.shared_group_links.count }.from(0).to(1)
-  end
+  shared_examples_for 'shareable' do
+    it 'adds group to another group' do
+      expect do
+        result = subject.execute
 
-  it 'returns false if shared group is blank' do
-    expect { described_class.new(nil, group, user, opts) }.not_to change { group.shared_group_links.count }
+        expect(result[:status]).to eq(:success)
+      end.to change { group.shared_with_group_links.count }.from(0).to(1)
+    end
   end
 
-  context 'user does not have access to group' do
-    let(:user) { create(:user) }
-
-    before do
-      shared_group.add_owner(user)
-    end
+  context 'when user has proper membership to share a group' do
+    let_it_be(:group_user) { create(:user) }
 
-    it 'returns error' do
-      result = subject.execute
+    let(:user) { group_user }
 
-      expect(result[:status]).to eq(:error)
-      expect(result[:http_status]).to eq(404)
+    before do
+      shared_with_group.add_guest(group_user)
+      group.add_owner(group_user)
     end
-  end
 
-  context 'user does not have admin access to shared group' do
-    let(:user) { create(:user) }
+    it_behaves_like 'shareable'
 
-    before do
-      group.add_guest(user)
-      shared_group.add_developer(user)
-    end
+    context 'when sharing outside the hierarchy is disabled' do
+      let_it_be(:group_parent) do
+        create(:group,
+               namespace_settings: create(:namespace_settings, prevent_sharing_groups_outside_hierarchy: true))
+      end
 
-    it 'returns error' do
-      result = subject.execute
+      it_behaves_like 'not shareable'
 
-      expect(result[:status]).to eq(:error)
-      expect(result[:http_status]).to eq(404)
-    end
-  end
+      context 'when group is inside hierarchy' do
+        let(:shared_with_group) { create(:group, :private, parent: group_parent) }
 
-  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)
+        it_behaves_like 'shareable'
+      end
     end
 
-    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
+    context 'project authorizations based on group hierarchies' do
+      let_it_be(:child_group_user) { create(:user) }
+      let_it_be(:parent_group_user) { create(:user) }
 
-        subject.execute
+      before do
+        shared_with_group_parent.add_owner(parent_group_user)
+        shared_with_group.add_owner(group_user)
+        shared_with_group_child.add_owner(child_group_user)
       end
-    end
 
-    context 'project authorizations' do
-      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
-
-          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
+        let(:group_child) { create(:group, :private, parent: group) }
+        let(:project_parent) { create(:project, group: group_parent) }
+        let(:project) { create(:project, group: group) }
+        let(:project_child) { create(:project, group: group_child) }
 
-        it 'create proper authorizations' do
-          subject.execute
+        context 'group user' do
+          let(:user) { group_user }
+
+          it 'create proper authorizations' do
+            subject.execute
 
-          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
+          it 'create proper authorizations' do
+            subject.execute
+
+            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
 
-          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
+        context 'child group user' do
+          let(:user) { child_group_user }
+
+          it 'create proper authorizations' do
+            subject.execute
+
+            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
   end
 
-  context 'sharing outside the hierarchy is disabled' do
-    let(:prevent_sharing) { true }
+  context 'user does not have access to group' do
+    let(:user) { create(:user) }
 
-    it 'prevents sharing with a group outside the hierarchy' do
-      result = subject.execute
+    before do
+      group.add_owner(user)
+    end
 
-      expect(group.reload.shared_group_links.count).to eq(0)
-      expect(result[:status]).to eq(:error)
-      expect(result[:http_status]).to eq(404)
+    it_behaves_like 'not shareable'
+  end
+
+  context 'user does not have admin access to shared group' do
+    let(:user) { create(:user) }
+
+    before do
+      shared_with_group.add_guest(user)
+      group.add_developer(user)
     end
 
-    it 'allows sharing with a group within the hierarchy' do
-      sibling_group = create(:group, :private, parent: shared_group_parent)
-      sibling_group.add_guest(group_user)
+    it_behaves_like 'not shareable'
+  end
+
+  context 'when group is blank' do
+    let(:group_user) { create(:user) }
+    let(:user) { group_user }
+    let(:group) { nil }
 
-      result = described_class.new(shared_group, sibling_group, user, opts).execute
+    it 'does not share and returns an error' do
+      expect do
+        result = subject.execute
 
-      expect(sibling_group.reload.shared_group_links.count).to eq(1)
-      expect(result[:status]).to eq(:success)
+        expect(result[:status]).to eq(:error)
+        expect(result[:http_status]).to eq(404)
+      end.not_to change { shared_with_group.shared_group_links.count }
     end
   end
+
+  context 'when shared_with_group is blank' do
+    let(:group_user) { create(:user) }
+    let(:user) { group_user }
+    let(:shared_with_group) { nil }
+
+    it_behaves_like 'not shareable'
+  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 e63adc07313..6aaf5f45069 100644
--- a/spec/services/groups/group_links/destroy_service_spec.rb
+++ b/spec/services/groups/group_links/destroy_service_spec.rb
@@ -3,54 +3,77 @@
 require 'spec_helper'
 
 RSpec.describe Groups::GroupLinks::DestroyService, '#execute' do
-  let(:user) { create(:user) }
-
+  let_it_be(:user) { create(:user) }
   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_it_be(:owner) { create(:user) }
 
-  before do
-    group.add_developer(owner)
-    shared_group.add_owner(owner)
-  end
-
   subject { described_class.new(shared_group, owner) }
 
-  context 'single link' do
-    let!(:link) { create(:group_group_link, shared_group: shared_group, shared_with_group: group) }
+  context 'when authorizing by user' do
+    before do
+      group.add_developer(owner)
+      shared_group.add_owner(owner)
+    end
+
+    context 'single link' do
+      let!(:link) { create(:group_group_link, shared_group: shared_group, shared_with_group: group) }
 
-    it 'destroys link' do
-      expect { subject.execute(link) }.to change { shared_group.shared_with_group_links.count }.from(1).to(0)
+      it 'destroys the link' do
+        expect { subject.execute(link) }.to change { shared_group.shared_with_group_links.count }.from(1).to(0)
+      end
+
+      it 'revokes project authorization', :sidekiq_inline do
+        group.add_developer(user)
+
+        expect { subject.execute(link) }.to(
+          change { Ability.allowed?(user, :read_project, project) }.from(true).to(false))
+      end
     end
 
-    it 'revokes project authorization', :sidekiq_inline do
-      group.add_developer(user)
+    context 'multiple links' do
+      let_it_be(:another_group) { create(:group, :private) }
+      let_it_be(:another_shared_group) { create(:group, :private) }
+
+      let!(:links) do
+        [
+          create(:group_group_link, shared_group: shared_group, shared_with_group: group),
+          create(:group_group_link, shared_group: shared_group, shared_with_group: another_group),
+          create(:group_group_link, shared_group: another_shared_group, shared_with_group: group),
+          create(:group_group_link, shared_group: another_shared_group, shared_with_group: another_group)
+        ]
+      end
 
-      expect { subject.execute(link) }.to(
-        change { Ability.allowed?(user, :read_project, project) }.from(true).to(false))
+      it 'updates project authorization once per group' do
+        expect(GroupGroupLink).to receive(:delete).and_call_original
+        expect(group).to receive(:refresh_members_authorized_projects).with(direct_members_only: true, blocking: false).once
+        expect(another_group).to receive(:refresh_members_authorized_projects).with(direct_members_only: true, blocking: false).once
+
+        subject.execute(links)
+      end
     end
   end
 
-  context 'multiple links' do
-    let_it_be(:another_group) { create(:group, :private) }
-    let_it_be(:another_shared_group) { create(:group, :private) }
-
-    let!(:links) do
-      [
-        create(:group_group_link, shared_group: shared_group, shared_with_group: group),
-        create(:group_group_link, shared_group: shared_group, shared_with_group: another_group),
-        create(:group_group_link, shared_group: another_shared_group, shared_with_group: group),
-        create(:group_group_link, shared_group: another_shared_group, shared_with_group: another_group)
-      ]
+  context 'when skipping authorization' do
+    let!(:link) { create(:group_group_link, shared_group: shared_group, shared_with_group: group) }
+
+    context 'with provided group and owner' do
+      it 'destroys the link' do
+        expect do
+          subject.execute(link, skip_authorization: true)
+        end.to change { shared_group.shared_with_group_links.count }.from(1).to(0)
+      end
     end
 
-    it 'updates project authorization once per group' do
-      expect(GroupGroupLink).to receive(:delete).and_call_original
-      expect(group).to receive(:refresh_members_authorized_projects).with(direct_members_only: true, blocking: false).once
-      expect(another_group).to receive(:refresh_members_authorized_projects).with(direct_members_only: true, blocking: false).once
+    context 'without providing group or owner' do
+      subject { described_class.new(nil, nil) }
 
-      subject.execute(links)
+      it 'destroys the link' do
+        expect do
+          subject.execute(link, skip_authorization: true)
+        end.to change { shared_group.shared_with_group_links.count }.from(1).to(0)
+      end
     end
   end
 end
diff --git a/spec/services/groups/open_issues_count_service_spec.rb b/spec/services/groups/open_issues_count_service_spec.rb
index 7dd8c2a59a0..fca09bfdebe 100644
--- a/spec/services/groups/open_issues_count_service_spec.rb
+++ b/spec/services/groups/open_issues_count_service_spec.rb
@@ -3,18 +3,12 @@
 require 'spec_helper'
 
 RSpec.describe Groups::OpenIssuesCountService, :use_clean_rails_memory_store_caching do
-  let_it_be(:group) { create(:group, :public) }
+  let_it_be(:group) { create(:group, :public)}
   let_it_be(:project) { create(:project, :public, namespace: group) }
-  let_it_be(:admin) { create(:user, :admin) }
   let_it_be(:user) { create(:user) }
-  let_it_be(:banned_user) { create(:user, :banned) }
-
-  before do
-    create(:issue, :opened, project: project)
-    create(:issue, :opened, confidential: true, project: project)
-    create(:issue, :opened, author: banned_user, project: project)
-    create(:issue, :closed, project: project)
-  end
+  let_it_be(:issue) { create(:issue, :opened, project: project) }
+  let_it_be(:confidential) { create(:issue, :opened, confidential: true, project: project) }
+  let_it_be(:closed) { create(:issue, :closed, project: project) }
 
   subject { described_class.new(group, user) }
 
@@ -26,27 +20,17 @@ RSpec.describe Groups::OpenIssuesCountService, :use_clean_rails_memory_store_cac
     it 'uses the IssuesFinder to scope issues' do
       expect(IssuesFinder)
         .to receive(:new)
-        .with(user, group_id: group.id, state: 'opened', non_archived: true, include_subgroups: true, public_only: true, include_hidden: false)
+        .with(user, group_id: group.id, state: 'opened', non_archived: true, include_subgroups: true, public_only: true)
 
       subject.count
     end
   end
 
   describe '#count' do
-    shared_examples 'counts public issues, does not count hidden or confidential' do
-      it 'counts only public issues' do
-        expect(subject.count).to eq(1)
-      end
-
-      it 'uses PUBLIC_COUNT_WITHOUT_HIDDEN_KEY cache key' do
-        expect(subject.cache_key).to include('group_open_public_issues_without_hidden_count')
-      end
-    end
-
     context 'when user is nil' do
-      let(:user) { nil }
-
-      it_behaves_like 'counts public issues, does not count hidden or confidential'
+      it 'does not include confidential issues in the issue count' do
+        expect(described_class.new(group).count).to eq(1)
+      end
     end
 
     context 'when user is provided' do
@@ -55,13 +39,9 @@ RSpec.describe Groups::OpenIssuesCountService, :use_clean_rails_memory_store_cac
           group.add_reporter(user)
         end
 
-        it 'includes confidential issues and does not include hidden issues in count' do
+        it 'returns the right count with confidential issues' do
           expect(subject.count).to eq(2)
         end
-
-        it 'uses TOTAL_COUNT_WITHOUT_HIDDEN_KEY cache key' do
-          expect(subject.cache_key).to include('group_open_issues_without_hidden_count')
-        end
       end
 
       context 'when user cannot read confidential issues' do
@@ -69,24 +49,8 @@ RSpec.describe Groups::OpenIssuesCountService, :use_clean_rails_memory_store_cac
           group.add_guest(user)
         end
 
-        it_behaves_like 'counts public issues, does not count hidden or confidential'
-      end
-
-      context 'when user is an admin' do
-        let(:user) { admin }
-
-        context 'when admin mode is enabled', :enable_admin_mode do
-          it 'includes confidential and hidden issues in count' do
-            expect(subject.count).to eq(3)
-          end
-
-          it 'uses TOTAL_COUNT_KEY cache key' do
-            expect(subject.cache_key).to include('group_open_issues_including_hidden_count')
-          end
-        end
-
-        context 'when admin mode is disabled' do
-          it_behaves_like 'counts public issues, does not count hidden or confidential'
+        it 'does not include confidential issues' do
+          expect(subject.count).to eq(1)
         end
       end
 
@@ -97,13 +61,11 @@ RSpec.describe Groups::OpenIssuesCountService, :use_clean_rails_memory_store_cac
   describe '#clear_all_cache_keys' do
     it 'calls `Rails.cache.delete` with the correct keys' do
       expect(Rails.cache).to receive(:delete)
-        .with(['groups', 'open_issues_count_service', 1, group.id, described_class::PUBLIC_COUNT_WITHOUT_HIDDEN_KEY])
+        .with(['groups', 'open_issues_count_service', 1, group.id, described_class::PUBLIC_COUNT_KEY])
       expect(Rails.cache).to receive(:delete)
         .with(['groups', 'open_issues_count_service', 1, group.id, described_class::TOTAL_COUNT_KEY])
-      expect(Rails.cache).to receive(:delete)
-        .with(['groups', 'open_issues_count_service', 1, group.id, described_class::TOTAL_COUNT_WITHOUT_HIDDEN_KEY])
 
-      described_class.new(group).clear_all_cache_keys
+      subject.clear_all_cache_keys
     end
   end
 end
diff --git a/spec/services/groups/transfer_service_spec.rb b/spec/services/groups/transfer_service_spec.rb
index 1c4b7aac87e..20ea8b2bf1b 100644
--- a/spec/services/groups/transfer_service_spec.rb
+++ b/spec/services/groups/transfer_service_spec.rb
@@ -574,7 +574,7 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do
 
         context 'resets project authorizations' do
           let_it_be(:old_parent_group) { create(:group) }
-          let_it_be_with_reload(:group) { create(:group, :private, parent: old_parent_group) }
+          let_it_be_with_refind(:group) { create(:group, :private, parent: old_parent_group) }
           let_it_be(:new_group_member) { create(:user) }
           let_it_be(:old_group_member) { create(:user) }
           let_it_be(:unique_subgroup_member) { create(:user) }