diff --git a/ee/app/controllers/ee/groups/group_members_controller.rb b/ee/app/controllers/ee/groups/group_members_controller.rb index b6783688e771588a9e6a81cb6462aa4f068b0730..74af9530b8e10c9ae58872f54b55cc0c8f80662e 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 8a87d8080cc19bb37ac65cf585732499f6132e58..57ea29be8911429e1dbd276b4bb87c131536ba43 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 0000000000000000000000000000000000000000..07cbae4daa3186a39a64438591a35abdf0a1e212 --- /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 0000000000000000000000000000000000000000..c0dc95b008d76c50577e588a33100d6ce0f50b5d --- /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 89ee3f6a2d6fda03d0eefe5e03580ba70e42bb16..e1343f0d65d770728e17e7f27385ec94c5fec906 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 37e72ec8a5b231a55453422c30f5181acbf255dc..ec098d329451fd349e4be0564ede2359c3feae32 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