Add latest changes from gitlab-org/gitlab@master

12 files changed, 130 insertions, 215 deletions

diff --git a/spec/lib/api/entities/group_spec.rb b/spec/lib/api/entities/group_spec.rb
deleted file mode 100644
index 270ac323c7d..00000000000
--- a/spec/lib/api/entities/group_spec.rb
+++ /dev/null
@@ -1,24 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe API::Entities::Group, feature_category: :groups_and_projects do
-  let_it_be(:group) do
-    base_group = create(:group) { |g| create(:project_statistics, namespace_id: g.id) }
-    Group.with_statistics.find(base_group.id)
-  end
-
-  subject(:json) { described_class.new(group, { with_custom_attributes: true, statistics: true }).as_json }
-
-  it 'returns expected data' do
-    expect(json.keys).to(
-      include(
-        :organization_id, :path, :description, :visibility, :share_with_group_lock, :require_two_factor_authentication,
-        :two_factor_grace_period, :project_creation_level, :auto_devops_enabled,
-        :subgroup_creation_level, :emails_disabled, :emails_enabled, :lfs_enabled, :default_branch_protection,
-        :default_branch_protection_defaults, :avatar_url, :request_access_enabled, :full_name, :full_path, :created_at,
-        :parent_id, :organization_id, :shared_runners_setting, :custom_attributes, :statistics
-      )
-    )
-  end
-end
diff --git a/spec/lib/api/helpers_spec.rb b/spec/lib/api/helpers_spec.rb
index 89fcb4b43a6..c76694b60d3 100644
--- a/spec/lib/api/helpers_spec.rb
+++ b/spec/lib/api/helpers_spec.rb
@@ -406,37 +406,6 @@ RSpec.describe API::Helpers, feature_category: :shared do
     end
   end
 
-  describe '#find_organization!' do
-    let_it_be(:organization) { create(:organization) }
-    let_it_be(:user) { create(:user) }
-
-    before do
-      allow(helper).to receive(:current_user).and_return(user)
-      allow(helper).to receive(:initial_current_user).and_return(user)
-    end
-
-    context 'when user is authenticated' do
-      it 'returns requested organization' do
-        expect(helper.find_organization!(organization.id)).to eq(organization)
-      end
-    end
-
-    context 'when user is not authenticated' do
-      let(:user) { nil }
-
-      it 'returns requested organization' do
-        expect(helper.find_organization!(organization.id)).to eq(organization)
-      end
-    end
-
-    context 'when organization does not exist' do
-      it 'returns nil' do
-        expect(helper).to receive(:render_api_error!).with('404 Organization Not Found', 404)
-        expect(helper.find_organization!(non_existing_record_id)).to be_nil
-      end
-    end
-  end
-
   describe '#find_group!' do
     let_it_be(:group) { create(:group, :public) }
     let_it_be(:user) { create(:user) }
@@ -488,7 +457,7 @@ RSpec.describe API::Helpers, feature_category: :shared do
       end
     end
 
-    context 'with support for IDs and paths as arguments' do
+    context 'support for IDs and paths as arguments' do
       let_it_be(:group) { create(:group) }
 
       let(:user) { group.first_owner }
@@ -534,34 +503,6 @@ RSpec.describe API::Helpers, feature_category: :shared do
         it_behaves_like 'group finder'
       end
     end
-
-    context 'with support for organization as an argument' do
-      let_it_be(:group) { create(:group) }
-      let_it_be(:organization) { create(:organization) }
-
-      before do
-        allow(helper).to receive(:current_user).and_return(group.first_owner)
-        allow(helper).to receive(:job_token_authentication?).and_return(false)
-        allow(helper).to receive(:authenticate_non_public?).and_return(false)
-      end
-
-      subject { helper.find_group!(group.id, organization: organization) }
-
-      context 'when group exists in the organization' do
-        before do
-          group.update!(organization: organization)
-        end
-
-        it { is_expected.to eq(group) }
-      end
-
-      context 'when group does not exist in the organization' do
-        it 'returns nil' do
-          expect(helper).to receive(:render_api_error!).with('404 Group Not Found', 404)
-          is_expected.to be_nil
-        end
-      end
-    end
   end
 
   describe '#find_group_by_full_path!' do
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 1fafa64a535..fdbcd84d3df 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -1642,6 +1642,46 @@ RSpec.describe Group, feature_category: :groups_and_projects do
     it { expect(subject.parent).to be_kind_of(described_class) }
   end
 
+  describe '#has_user?' do
+    let_it_be(:group) { create(:group) }
+    let_it_be(:subgroup) { create(:group, parent: group) }
+    let_it_be(:user) { create(:user) }
+    let_it_be(:user2) { create(:user) }
+
+    subject { group.has_user?(user) }
+
+    context 'when the user is a member' do
+      before_all do
+        group.add_developer(user)
+      end
+
+      it { is_expected.to be_truthy }
+      it { expect(group.has_user?(user2)).to be_falsey }
+
+      it 'returns false for subgroup' do
+        expect(subgroup.has_user?(user)).to be_falsey
+      end
+    end
+
+    context 'when the user is a member with minimal access' do
+      before_all do
+        group.add_member(user, GroupMember::MINIMAL_ACCESS)
+      end
+
+      it { is_expected.to be_falsey }
+    end
+
+    context 'when the user has requested membership' do
+      before_all do
+        create(:group_member, :developer, :access_request, user: user, source: group)
+      end
+
+      it 'returns false' do
+        expect(subject).to be_falsey
+      end
+    end
+  end
+
   describe '#member?' do
     let_it_be(:group) { create(:group) }
     let_it_be(:user) { create(:user) }
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index c256c4f10f8..db1754ef991 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1121,6 +1121,8 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr
     end
 
     it { is_expected.to delegate_method(:members).to(:team).with_prefix(true) }
+    it { is_expected.to delegate_method(:has_user?).to(:team) }
+    it { is_expected.to delegate_method(:member?).to(:team) }
     it { is_expected.to delegate_method(:name).to(:owner).with_prefix(true).allow_nil }
     it { is_expected.to delegate_method(:root_ancestor).to(:namespace).allow_nil }
     it { is_expected.to delegate_method(:certificate_based_clusters_enabled?).to(:namespace).allow_nil }
diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb
index 10a2e967b14..47ab48a6497 100644
--- a/spec/models/project_team_spec.rb
+++ b/spec/models/project_team_spec.rb
@@ -341,22 +341,52 @@ RSpec.describe ProjectTeam, feature_category: :groups_and_projects do
     end
   end
 
+  describe '#has_user?' do
+    let_it_be(:group) { create(:group) }
+    let_it_be(:project) { create(:project, namespace: group) }
+    let_it_be(:user) { create(:user) }
+    let_it_be(:user2) { create(:user) }
+
+    subject { project.team.has_user?(user) }
+
+    context 'when the user is a member' do
+      before_all do
+        project.add_developer(user)
+      end
+
+      it { is_expected.to be_truthy }
+      it { expect(group.has_user?(user2)).to be_falsey }
+    end
+
+    context 'when user is a member with minimal access' do
+      before_all do
+        project.add_member(user, GroupMember::MINIMAL_ACCESS)
+      end
+
+      it { is_expected.to be_falsey }
+    end
+
+    context 'when user is not a direct member of the project' do
+      before_all do
+        create(:group_member, :developer, user: user, source: group)
+      end
+
+      it { is_expected.to be_falsey }
+    end
+  end
+
   describe "#human_max_access" do
-    it 'returns Maintainer role' do
-      user = create(:user)
-      group = create(:group)
-      project = create(:project, namespace: group)
+    let_it_be(:user) { create(:user) }
+    let_it_be(:group) { create(:group) }
+    let_it_be(:project) { create(:project, namespace: group) }
 
+    it 'returns Maintainer role' do
       group.add_maintainer(user)
 
       expect(project.team.human_max_access(user.id)).to eq 'Maintainer'
     end
 
     it 'returns Owner role' do
-      user = create(:user)
-      group = create(:group)
-      project = create(:project, namespace: group)
-
       group.add_owner(user)
 
       expect(project.team.human_max_access(user.id)).to eq 'Owner'
diff --git a/spec/policies/organizations/organization_policy_spec.rb b/spec/policies/organizations/organization_policy_spec.rb
index a1a2f1db305..7eed497d644 100644
--- a/spec/policies/organizations/organization_policy_spec.rb
+++ b/spec/policies/organizations/organization_policy_spec.rb
@@ -20,7 +20,6 @@ RSpec.describe Organizations::OrganizationPolicy, feature_category: :cell do
 
     context 'when admin mode is enabled', :enable_admin_mode do
       it { is_expected.to be_allowed(:admin_organization) }
-      it { is_expected.to be_allowed(:create_group) }
       it { is_expected.to be_allowed(:read_organization) }
       it { is_expected.to be_allowed(:read_organization_user) }
     end
@@ -37,14 +36,12 @@ RSpec.describe Organizations::OrganizationPolicy, feature_category: :cell do
     end
 
     it { is_expected.to be_allowed(:admin_organization) }
-    it { is_expected.to be_allowed(:create_group) }
     it { is_expected.to be_allowed(:read_organization) }
     it { is_expected.to be_allowed(:read_organization_user) }
   end
 
   context 'when the user is not part of the organization' do
     it { is_expected.to be_disallowed(:admin_organization) }
-    it { is_expected.to be_disallowed(:create_group) }
     it { is_expected.to be_disallowed(:read_organization_user) }
     # All organizations are currently public, and hence they are allowed to be read
     # even if the user is not a part of the organization.
diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb
index d1158cba16e..327dfd0a76b 100644
--- a/spec/requests/api/groups_spec.rb
+++ b/spec/requests/api/groups_spec.rb
@@ -1937,59 +1937,6 @@ RSpec.describe API::Groups, feature_category: :groups_and_projects do
       end
     end
 
-    context 'when group is within a provided organization' do
-      let_it_be(:organization) { create(:organization) }
-
-      context 'when user is an organization user' do
-        before_all do
-          create(:organization_user, user: user3, organization: organization)
-        end
-
-        it 'creates group within organization' do
-          post api('/groups', user3), params: attributes_for_group_api(organization_id: organization.id)
-
-          expect(response).to have_gitlab_http_status(:created)
-          expect(json_response['organization_id']).to eq(organization.id)
-        end
-
-        context 'when parent_group is not part of the organization' do
-          it 'does not create the group with not_found' do
-            post(
-              api('/groups', user3),
-              params: attributes_for_group_api(parent_id: group2.id, organization_id: organization.id)
-            )
-
-            expect(response).to have_gitlab_http_status(:not_found)
-          end
-        end
-      end
-
-      context 'when organization does not exist' do
-        it 'does not create the group with not_found' do
-          post api('/groups', user3), params: attributes_for_group_api(organization_id: non_existing_record_id)
-
-          expect(response).to have_gitlab_http_status(:not_found)
-        end
-      end
-
-      context 'when user is not an organization user' do
-        it 'does not create the group' do
-          post api('/groups', user3), params: attributes_for_group_api(organization_id: organization.id)
-
-          expect(response).to have_gitlab_http_status(:forbidden)
-        end
-      end
-
-      context 'when user is an admin' do
-        it 'creates group within organization' do
-          post api('/groups', admin, admin_mode: true), params: attributes_for_group_api(organization_id: organization.id)
-
-          expect(response).to have_gitlab_http_status(:created)
-          expect(json_response['organization_id']).to eq(organization.id)
-        end
-      end
-    end
-
     context "when authenticated as user with group permissions" do
       it "creates group", :aggregate_failures do
         group = attributes_for_group_api request_access_enabled: false
diff --git a/spec/services/groups/create_service_spec.rb b/spec/services/groups/create_service_spec.rb
index 56b1516096a..b2b27a1a075 100644
--- a/spec/services/groups/create_service_spec.rb
+++ b/spec/services/groups/create_service_spec.rb
@@ -6,7 +6,7 @@ RSpec.describe Groups::CreateService, '#execute', feature_category: :groups_and_
   let!(:user) { create(:user) }
   let!(:group_params) { { path: "group_path", visibility_level: Gitlab::VisibilityLevel::PUBLIC } }
 
-  subject(:execute) { service.execute }
+  subject { service.execute }
 
   shared_examples 'has sync-ed traversal_ids' do
     specify { expect(subject.reload.traversal_ids).to eq([subject.parent&.traversal_ids, subject.id].flatten.compact) }
@@ -119,37 +119,6 @@ RSpec.describe Groups::CreateService, '#execute', feature_category: :groups_and_
     end
   end
 
-  describe 'creating a group within a provided organization' do
-    let_it_be(:organization) { create(:organization) }
-
-    let(:current_user) { user }
-    let(:params) { group_params.merge(organization_id: organization.id) }
-    let(:service) { described_class.new(current_user, params) }
-
-    context 'when user can create the group' do
-      before do
-        create(:organization_user, user: user, organization: organization)
-      end
-
-      it { is_expected.to be_persisted }
-    end
-
-    context 'when user is an admin', :enable_admin_mode do
-      let(:current_user) { create(:admin) }
-
-      it { is_expected.to be_persisted }
-    end
-
-    context 'when user can not create the group' do
-      it 'does not save group and returns an error' do
-        expect(execute).not_to be_persisted
-        expect(execute.errors[:organization_id].first)
-          .to eq(s_("CreateGroup|You don't have permission to create a group in the provided organization."))
-        expect(execute.organization_id).to be_nil
-      end
-    end
-  end
-
   describe 'creating subgroup' do
     let!(:group) { create(:group) }
     let!(:service) { described_class.new(user, group_params.merge(parent_id: group.id)) }
diff --git a/spec/services/members/create_service_spec.rb b/spec/services/members/create_service_spec.rb
index b977292bcf4..af151f93dc7 100644
--- a/spec/services/members/create_service_spec.rb
+++ b/spec/services/members/create_service_spec.rb
@@ -98,7 +98,7 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
 
       it 'adds a user to members' do
         expect(execute_service[:status]).to eq(:success)
-        expect(source.users).to include member
+        expect(source).to have_user(member)
         expect(Onboarding::Progress.completed?(source, :user_added)).to be(true)
       end
 
diff --git a/spec/services/resource_access_tokens/revoke_service_spec.rb b/spec/services/resource_access_tokens/revoke_service_spec.rb
index 060697cd1df..aab22cb2815 100644
--- a/spec/services/resource_access_tokens/revoke_service_spec.rb
+++ b/spec/services/resource_access_tokens/revoke_service_spec.rb
@@ -26,7 +26,7 @@ RSpec.describe ResourceAccessTokens::RevokeService, feature_category: :system_ac
       it 'removes membership of bot user' do
         subject
 
-        expect(resource.reload.users).not_to include(resource_bot)
+        expect(resource.reload).not_to have_user(resource_bot)
       end
 
       it 'initiates user removal' do
@@ -56,7 +56,7 @@ RSpec.describe ResourceAccessTokens::RevokeService, feature_category: :system_ac
       it 'does not remove bot from member list' do
         subject
 
-        expect(resource.reload.users).to include(resource_bot)
+        expect(resource.reload).to have_user(resource_bot)
       end
 
       it 'does not transfer issuables of bot user to ghost user' do
diff --git a/spec/support/matchers/have_user.rb b/spec/support/matchers/have_user.rb
new file mode 100644
index 00000000000..64fc84a75cf
--- /dev/null
+++ b/spec/support/matchers/have_user.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+RSpec::Matchers.define :have_user do |user|
+  match do |resource|
+    raise ArgumentError, 'Unknown resource type' unless resource.is_a?(Group) || resource.is_a?(Project)
+
+    expect(resource.has_user?(user)).to be_truthy
+  end
+
+  failure_message do |group|
+    "Expected #{group} to have the user #{user} among its members"
+  end
+end
diff --git a/spec/support/shared_examples/models/member_shared_examples.rb b/spec/support/shared_examples/models/member_shared_examples.rb
index 731500c4510..6f00a5485a2 100644
--- a/spec/support/shared_examples/models/member_shared_examples.rb
+++ b/spec/support/shared_examples/models/member_shared_examples.rb
@@ -108,7 +108,7 @@ RSpec.shared_examples_for "member creation" do
       it 'does not update the member' do
         member = described_class.add_member(source, project_bot, :maintainer, current_user: user)
 
-        expect(source.users.reload).to include(project_bot)
+        expect(source.reload).to have_user(project_bot)
         expect(member).to be_persisted
         expect(member.access_level).to eq(Gitlab::Access::DEVELOPER)
         expect(member.errors.full_messages).to include(/not authorized to update member/)
@@ -119,7 +119,7 @@ RSpec.shared_examples_for "member creation" do
       it 'adds the member' do
         member = described_class.add_member(source, project_bot, :maintainer, current_user: user)
 
-        expect(source.users.reload).to include(project_bot)
+        expect(source.reload).to have_user(project_bot)
         expect(member).to be_persisted
       end
     end
@@ -130,7 +130,7 @@ RSpec.shared_examples_for "member creation" do
       member = described_class.add_member(source, user, :maintainer, current_user: admin)
 
       expect(member).to be_persisted
-      expect(source.users.reload).to include(user)
+      expect(source.reload).to have_user(user)
       expect(member.created_by).to eq(admin)
     end
   end
@@ -140,7 +140,7 @@ RSpec.shared_examples_for "member creation" do
       member = described_class.add_member(source, user, :maintainer, current_user: admin)
 
       expect(member).not_to be_persisted
-      expect(source.users.reload).not_to include(user)
+      expect(source).not_to have_user(user)
       expect(member.errors.full_messages).to include(/not authorized to create member/)
     end
   end
@@ -153,52 +153,52 @@ RSpec.shared_examples_for "member creation" do
 
   described_class.access_levels.each do |sym_key, int_access_level|
     it "accepts the :#{sym_key} symbol as access level", :aggregate_failures do
-      expect(source.users).not_to include(user)
+      expect(source).not_to have_user(user)
 
       member = described_class.add_member(source, user.id, sym_key)
 
       expect(member.access_level).to eq(int_access_level)
-      expect(source.users.reload).to include(user)
+      expect(source.reload).to have_user(user)
     end
 
     it "accepts the #{int_access_level} integer as access level", :aggregate_failures do
-      expect(source.users).not_to include(user)
+      expect(source).not_to have_user(user)
 
       member = described_class.add_member(source, user.id, int_access_level)
 
       expect(member.access_level).to eq(int_access_level)
-      expect(source.users.reload).to include(user)
+      expect(source.reload).to have_user(user)
     end
   end
 
   context 'with no current_user' do
     context 'when called with a known user id' do
       it 'adds the user as a member' do
-        expect(source.users).not_to include(user)
+        expect(source).not_to have_user(user)
 
         described_class.add_member(source, user.id, :maintainer)
 
-        expect(source.users.reload).to include(user)
+        expect(source.reload).to have_user(user)
       end
     end
 
     context 'when called with an unknown user id' do
       it 'does not add the user as a member' do
-        expect(source.users).not_to include(user)
+        expect(source).not_to have_user(user)
 
         described_class.add_member(source, non_existing_record_id, :maintainer)
 
-        expect(source.users.reload).not_to include(user)
+        expect(source.reload).not_to have_user(user)
       end
     end
 
     context 'when called with a user object' do
       it 'adds the user as a member' do
-        expect(source.users).not_to include(user)
+        expect(source).not_to have_user(user)
 
         described_class.add_member(source, user, :maintainer)
 
-        expect(source.users.reload).to include(user)
+        expect(source.reload).to have_user(user)
       end
     end
 
@@ -208,29 +208,29 @@ RSpec.shared_examples_for "member creation" do
       end
 
       it 'adds the requester as a member', :aggregate_failures do
-        expect(source.users).not_to include(user)
+        expect(source.reload).not_to have_user(user)
         expect(source.requesters.exists?(user_id: user)).to eq(true)
 
         described_class.add_member(source, user, :maintainer)
 
-        expect(source.users.reload).to include(user)
-        expect(source.requesters.reload.exists?(user_id: user)).to eq(false)
+        expect(source.reload).to have_user(user)
+        expect(source.requesters.exists?(user_id: user)).to eq(false)
       end
     end
 
     context 'when called with a known user email' do
       it 'adds the user as a member' do
-        expect(source.users).not_to include(user)
+        expect(source).not_to have_user(user)
 
         described_class.add_member(source, user.email, :maintainer)
 
-        expect(source.users.reload).to include(user)
+        expect(source.reload).to have_user(user)
       end
     end
 
     context 'when called with an unknown user email' do
       it 'creates an invited member' do
-        expect(source.users).not_to include(user)
+        expect(source).not_to have_user(user)
 
         described_class.add_member(source, 'user@example.com', :maintainer)
 
@@ -245,18 +245,18 @@ RSpec.shared_examples_for "member creation" do
         described_class.add_member(source, email_starting_with_number, :maintainer)
 
         expect(source.members.invite.pluck(:invite_email)).to include(email_starting_with_number)
-        expect(source.users.reload).not_to include(user)
+        expect(source.reload).not_to have_user(user)
       end
     end
   end
 
   context 'when current_user can update member', :enable_admin_mode do
     it 'creates the member' do
-      expect(source.users).not_to include(user)
+      expect(source).not_to have_user(user)
 
       described_class.add_member(source, user, :maintainer, current_user: admin)
 
-      expect(source.users.reload).to include(user)
+      expect(source.reload).to have_user(user)
     end
 
     context 'when called with a requester user object' do
@@ -265,12 +265,12 @@ RSpec.shared_examples_for "member creation" do
       end
 
       it 'adds the requester as a member', :aggregate_failures do
-        expect(source.users).not_to include(user)
+        expect(source).not_to have_user(user)
         expect(source.requesters.exists?(user_id: user)).to be_truthy
 
         described_class.add_member(source, user, :maintainer, current_user: admin)
 
-        expect(source.users.reload).to include(user)
+        expect(source.reload).to have_user(user)
         expect(source.requesters.reload.exists?(user_id: user)).to be_falsy
       end
     end
@@ -278,11 +278,11 @@ RSpec.shared_examples_for "member creation" do
 
   context 'when current_user cannot update member' do
     it 'does not create the member', :aggregate_failures do
-      expect(source.users).not_to include(user)
+      expect(source).not_to have_user(user)
 
       member = described_class.add_member(source, user, :maintainer, current_user: user)
 
-      expect(source.users.reload).not_to include(user)
+      expect(source.reload).not_to have_user(user)
       expect(member).not_to be_persisted
     end
 
@@ -292,12 +292,12 @@ RSpec.shared_examples_for "member creation" do
       end
 
       it 'does not destroy the requester', :aggregate_failures do
-        expect(source.users).not_to include(user)
+        expect(source).not_to have_user(user)
         expect(source.requesters.exists?(user_id: user)).to be_truthy
 
         described_class.add_member(source, user, :maintainer, current_user: user)
 
-        expect(source.users.reload).not_to include(user)
+        expect(source.reload).not_to have_user(user)
         expect(source.requesters.exists?(user_id: user)).to be_truthy
       end
     end
@@ -311,7 +311,7 @@ RSpec.shared_examples_for "member creation" do
 
       context 'with no current_user' do
         it 'updates the member' do
-          expect(source.users).to include(user)
+          expect(source).to have_user(user)
 
           described_class.add_member(source, user, :maintainer)
 
@@ -321,7 +321,7 @@ RSpec.shared_examples_for "member creation" do
 
       context 'when current_user can update member', :enable_admin_mode do
         it 'updates the member' do
-          expect(source.users).to include(user)
+          expect(source).to have_user(user)
 
           described_class.add_member(source, user, :maintainer, current_user: admin)
 
@@ -331,7 +331,7 @@ RSpec.shared_examples_for "member creation" do
 
       context 'when current_user cannot update member' do
         it 'does not update the member' do
-          expect(source.users).to include(user)
+          expect(source).to have_user(user)
 
           described_class.add_member(source, user, :maintainer, current_user: user)