From 7cc3770e8786cffaa64f6ac0a4de946e2fec1a09 Mon Sep 17 00:00:00 2001
From: Suraj Tripathi <stripathi@gitlab.com>
Date: Tue, 16 Apr 2024 18:57:29 +0000
Subject: [PATCH] Added MemberApprovalFinder
- Modified Controllers to use the Finder
- Modified Controllers spec to account for empty
- Added Finder spec
- Added Ability check in Finder
EE: true
Changelog: added
---
.../ee/groups/group_members_controller.rb | 10 +-
.../ee/projects/project_members_controller.rb | 10 +-
.../member_approval_finder.rb | 55 +++++++++++
.../member_approval_finder_spec.rb | 96 +++++++++++++++++++
.../groups/group_members_controller_spec.rb | 4 +-
.../project_members_controller_spec.rb | 4 +-
6 files changed, 165 insertions(+), 14 deletions(-)
create mode 100644 ee/app/finders/member_management/member_approval_finder.rb
create mode 100644 ee/spec/finders/member_management/member_approval_finder_spec.rb
diff --git a/ee/app/controllers/ee/groups/group_members_controller.rb b/ee/app/controllers/ee/groups/group_members_controller.rb
index b6783688e7715..74af9530b8e10 100644
--- a/ee/app/controllers/ee/groups/group_members_controller.rb
+++ b/ee/app/controllers/ee/groups/group_members_controller.rb
@@ -165,12 +165,12 @@ def banned_members(params: {})
def pending_promotion_members
return unless can?(current_user, :admin_group_member, group)
- return unless ::Feature.enabled?(:member_promotion_management, type: :wip)
- return unless ::Gitlab::CurrentSettings.enable_member_promotion_management?
- ::Members::MemberApproval.pending_member_approvals(group.id)
- .page(params[:promotion_requests_page])
- .per(MEMBER_PER_PAGE_LIMIT)
+ ::MemberManagement::MemberApprovalFinder
+ .new(current_user: current_user, params: params, source: group)
+ .execute
+ .page(params[:promotion_requests_page])
+ .per(MEMBER_PER_PAGE_LIMIT)
end
end
end
diff --git a/ee/app/controllers/ee/projects/project_members_controller.rb b/ee/app/controllers/ee/projects/project_members_controller.rb
index 8a87d8080cc19..57ea29be89114 100644
--- a/ee/app/controllers/ee/projects/project_members_controller.rb
+++ b/ee/app/controllers/ee/projects/project_members_controller.rb
@@ -38,12 +38,12 @@ def non_invited_members
def pending_promotion_members
return unless can?(current_user, :admin_project_member, project)
- return unless ::Feature.enabled?(:member_promotion_management, type: :wip)
- return unless ::Gitlab::CurrentSettings.enable_member_promotion_management?
- ::Members::MemberApproval.pending_member_approvals(project.project_namespace_id)
- .page(params[:promotion_requests_page])
- .per(MEMBER_PER_PAGE_LIMIT)
+ ::MemberManagement::MemberApprovalFinder
+ .new(current_user: current_user, params: params, source: project)
+ .execute
+ .page(params[:promotion_requests_page])
+ .per(MEMBER_PER_PAGE_LIMIT)
end
end
end
diff --git a/ee/app/finders/member_management/member_approval_finder.rb b/ee/app/finders/member_management/member_approval_finder.rb
new file mode 100644
index 0000000000000..07cbae4daa318
--- /dev/null
+++ b/ee/app/finders/member_management/member_approval_finder.rb
@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+module MemberManagement
+ class MemberApprovalFinder
+ attr_reader :params
+
+ def initialize(current_user:, source:, params: {})
+ @current_user = current_user
+ @params = params
+ validate_and_set_source(source)
+ end
+
+ def execute
+ model = ::Members::MemberApproval
+ return model.none unless feature_and_settings_enabled?
+ return model.none unless allowed_to_query_member_approvals?
+
+ model.pending_member_approvals(@member_namespace_id)
+ end
+
+ private
+
+ attr_reader :current_user, :source, :member_namespace_id, :type
+
+ def validate_and_set_source(source)
+ case source
+ when ::Group
+ @member_namespace_id = source.id
+ @type = :group
+ when ::Project
+ @member_namespace_id = source.project_namespace.id
+ @type = :project
+ else
+ raise ArgumentError, 'Invalid source. Source should be either Group or Project.'
+ end
+ @source = source
+ end
+
+ def feature_and_settings_enabled?
+ ::Feature.enabled?(:member_promotion_management, type: :wip) &&
+ ::Gitlab::CurrentSettings.enable_member_promotion_management?
+ end
+
+ def allowed_to_query_member_approvals?
+ ability = case type
+ when :group
+ :admin_group_member
+ when :project
+ :admin_project_member
+ end
+
+ Ability.allowed?(current_user, ability, source)
+ end
+ end
+end
diff --git a/ee/spec/finders/member_management/member_approval_finder_spec.rb b/ee/spec/finders/member_management/member_approval_finder_spec.rb
new file mode 100644
index 0000000000000..c0dc95b008d76
--- /dev/null
+++ b/ee/spec/finders/member_management/member_approval_finder_spec.rb
@@ -0,0 +1,96 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe MemberManagement::MemberApprovalFinder, feature_category: :seat_cost_management do
+ let_it_be(:user) { create(:user) }
+ let_it_be(:group) { create(:group) }
+ let_it_be(:project) { create(:project) }
+ let(:source) { group }
+
+ subject(:finder) { described_class.new(current_user: user, params: {}, source: source) }
+
+ describe '#initialize' do
+ shared_examples 'invalid source arg' do
+ it 'raises an ArgumentError' do
+ expect do
+ finder
+ end.to raise_error(ArgumentError, 'Invalid source. Source should be either Group or Project.')
+ end
+ end
+
+ context 'when source is nil' do
+ let(:source) { nil }
+
+ it_behaves_like "invalid source arg"
+ end
+
+ context 'when source is neither Project or Group' do
+ let(:source) { create(:user_namespace) }
+
+ it_behaves_like "invalid source arg"
+ end
+ end
+
+ describe '#execute' do
+ shared_examples 'when feature and settings are enabled' do
+ let!(:pending_approval) { create(:member_approval, type, member_namespace: member_namespace) }
+
+ let!(:rejected_approval) { create(:member_approval, type, member_namespace: member_namespace, status: 2) }
+
+ context 'when user has admin access' do
+ before do
+ source.add_owner(user)
+ end
+
+ it 'returns pending member approvals for the source' do
+ expect(finder.execute).to contain_exactly(pending_approval)
+ end
+ end
+
+ context 'when user does not have admin access' do
+ before do
+ source.add_developer(user)
+ end
+
+ it 'returns empty' do
+ expect(finder.execute).to be_empty
+ end
+ end
+ end
+
+ before do
+ stub_feature_flags(member_promotion_management: true)
+ stub_application_setting(enable_member_promotion_management: true)
+ end
+
+ context 'when member promotion management feature is disabled' do
+ it 'returns nil' do
+ stub_feature_flags(member_promotion_management: false)
+ expect(finder.execute).to be_empty
+ end
+ end
+
+ context 'when member promotion management is disabled in settings' do
+ it 'returns nil' do
+ stub_application_setting(enable_member_promotion_management: false)
+ expect(finder.execute).to be_empty
+ end
+ end
+
+ context 'when group is provided' do
+ let(:member_namespace) { group }
+ let(:type) { :for_group_member }
+
+ it_behaves_like 'when feature and settings are enabled'
+ end
+
+ context 'when project is provided' do
+ let(:source) { project }
+ let(:member_namespace) { project.project_namespace }
+ let(:type) { :for_project_member }
+
+ it_behaves_like 'when feature and settings are enabled'
+ end
+ end
+end
diff --git a/ee/spec/requests/groups/group_members_controller_spec.rb b/ee/spec/requests/groups/group_members_controller_spec.rb
index 89ee3f6a2d6fd..e1343f0d65d77 100644
--- a/ee/spec/requests/groups/group_members_controller_spec.rb
+++ b/ee/spec/requests/groups/group_members_controller_spec.rb
@@ -136,10 +136,10 @@
stub_feature_flags(member_promotion_management: false)
end
- it 'assigns @pending_promotion_members as nil' do
+ it 'assigns @pending_promotion_members as empty' do
request
- expect(assigns(:pending_promotion_members)).to eq(nil)
+ expect(assigns(:pending_promotion_members)).to be_empty
end
end
end
diff --git a/ee/spec/requests/projects/project_members_controller_spec.rb b/ee/spec/requests/projects/project_members_controller_spec.rb
index 37e72ec8a5b23..ec098d329451f 100644
--- a/ee/spec/requests/projects/project_members_controller_spec.rb
+++ b/ee/spec/requests/projects/project_members_controller_spec.rb
@@ -69,10 +69,10 @@
stub_feature_flags(member_promotion_management: false)
end
- it 'assigns @pending_promotion_members as nil' do
+ it 'assigns @pending_promotion_members be empty' do
make_request
- expect(assigns(:pending_promotion_members)).to eq(nil)
+ expect(assigns(:pending_promotion_members)).to be_empty
end
end
end
--
GitLab