From dcbb5001eca02fd8aa6d8dbd06a401fc49716988 Mon Sep 17 00:00:00 2001 From: Suraj Tripathi <stripathi@gitlab.com> Date: Fri, 12 Apr 2024 19:34:34 +0000 Subject: [PATCH] Added pending member for project listing page - Added pending member approval list - Fixed order of method param - Fixed minor errors, and added spec for presenter - Fixed Presenter - Adds controller request spec for member pending promotions - Adds controller request spec for project member pending promotions - Fixed structure of promotion_request - Added pagination json - Fixed corresponding specs - Modified list to be visible only to admin/owner - Applied Review feedback EE: true Changelog: added --- app/helpers/groups/group_members_helper.rb | 2 +- .../projects/project_members_helper.rb | 3 +- app/models/members/members/member_approval.rb | 6 + .../members/member_approval_presenter.rb | 9 ++ .../groups/group_members/index.html.haml | 4 +- .../projects/project_members/index.html.haml | 3 +- .../ee/groups/group_members_controller.rb | 13 ++ .../ee/projects/project_members_controller.rb | 21 +++ .../helpers/ee/groups/group_members_helper.rb | 7 +- ee/app/helpers/ee/members_helper.rb | 14 ++ .../ee/projects/project_members_helper.rb | 9 +- .../ee/members/member_approval_presenter.rb | 39 +++++ .../member_approval_entity.rb | 68 +++++++++ .../member_approval_serializer.rb | 7 + .../ee/groups/group_members_helper_spec.rb | 23 ++- .../projects/project_members_helper_spec.rb | 24 ++- .../members/member_approval_presenter_spec.rb | 58 +++++++ .../groups/group_members_controller_spec.rb | 142 +++++++++++++----- .../project_members_controller_spec.rb | 80 ++++++++++ .../member_approval_entity_spec.rb | 34 +++++ .../member_approval_serializer_spec.rb | 33 ++++ .../promotion_request_shared_examples.rb | 24 +++ .../groups/group_members_helper_spec.rb | 6 +- .../projects/project_members_helper_spec.rb | 6 +- 24 files changed, 581 insertions(+), 54 deletions(-) create mode 100644 app/presenters/members/member_approval_presenter.rb create mode 100644 ee/app/presenters/ee/members/member_approval_presenter.rb create mode 100644 ee/app/serializers/member_management/member_approval_entity.rb create mode 100644 ee/app/serializers/member_management/member_approval_serializer.rb create mode 100644 ee/spec/presenters/ee/members/member_approval_presenter_spec.rb create mode 100644 ee/spec/requests/projects/project_members_controller_spec.rb create mode 100644 ee/spec/serializers/member_management/member_approval_entity_spec.rb create mode 100644 ee/spec/serializers/member_management/member_approval_serializer_spec.rb create mode 100644 ee/spec/support/shared_examples/helpers/promotion_request_shared_examples.rb diff --git a/app/helpers/groups/group_members_helper.rb b/app/helpers/groups/group_members_helper.rb index 076723433842b..17f4689a7fb8f 100644 --- a/app/helpers/groups/group_members_helper.rb +++ b/app/helpers/groups/group_members_helper.rb @@ -5,7 +5,7 @@ module Groups::GroupMembersHelper AVATAR_SIZE = 40 - def group_members_app_data(group, members:, invited:, access_requests:, banned:, include_relations:, search:) + def group_members_app_data(group, members:, invited:, access_requests:, banned:, include_relations:, search:, pending_members:) { user: group_members_list_data(group, members, { param_name: :page, params: { invited_members_page: nil, search_invited: nil } }), group: group_group_links_list_data(group, include_relations, search), diff --git a/app/helpers/projects/project_members_helper.rb b/app/helpers/projects/project_members_helper.rb index 3385dd4a043d8..c076c935b8464 100644 --- a/app/helpers/projects/project_members_helper.rb +++ b/app/helpers/projects/project_members_helper.rb @@ -18,7 +18,8 @@ def project_member_header_subtext(project) private - def project_members_app_data(project, members:, invited:, access_requests:, include_relations:, search:) + def project_members_app_data( + project, members:, invited:, access_requests:, include_relations:, search:, pending_members:) # rubocop:disable Lint/UnusedMethodArgument -- Argument used in EE { user: project_members_list_data(project, members, { param_name: :page, params: { search_groups: nil } }), group: project_group_links_list_data(project, include_relations, search), diff --git a/app/models/members/members/member_approval.rb b/app/models/members/members/member_approval.rb index 5475d4a55d265..996a78df7c200 100644 --- a/app/models/members/members/member_approval.rb +++ b/app/models/members/members/member_approval.rb @@ -2,6 +2,8 @@ module Members class MemberApproval < ApplicationRecord + include Presentable + enum status: { pending: 0, approved: 1, denied: 2 } belongs_to :user @@ -17,6 +19,10 @@ class MemberApproval < ApplicationRecord validates :member_namespace, presence: true validate :validate_unique_pending_approval, on: [:create, :update] + scope :pending_member_approvals, ->(member_namespace_id) do + where(member_namespace_id: member_namespace_id).where(status: statuses[:pending]) + end + private def validate_unique_pending_approval diff --git a/app/presenters/members/member_approval_presenter.rb b/app/presenters/members/member_approval_presenter.rb new file mode 100644 index 0000000000000..a81e07a8d0b5d --- /dev/null +++ b/app/presenters/members/member_approval_presenter.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module Members + class MemberApprovalPresenter < ::Gitlab::View::Presenter::Delegated + presents ::Members::MemberApproval, as: :member_approval + end +end + +Members::MemberApprovalPresenter.prepend_mod diff --git a/app/views/groups/group_members/index.html.haml b/app/views/groups/group_members/index.html.haml index bcf847d7f9d8a..fc576b0d66904 100644 --- a/app/views/groups/group_members/index.html.haml +++ b/app/views/groups/group_members/index.html.haml @@ -31,5 +31,7 @@ access_requests: @requesters, banned: @banned || [], include_relations: @include_relations, - search: params[:search_groups]).to_json } } + search: params[:search_groups], + pending_members: @pending_promotion_members + ).to_json } } = gl_loading_icon(css_class: 'gl-my-5', size: 'md') diff --git a/app/views/projects/project_members/index.html.haml b/app/views/projects/project_members/index.html.haml index c91d8923a0c8e..7a1c9e3c89c6a 100644 --- a/app/views/projects/project_members/index.html.haml +++ b/app/views/projects/project_members/index.html.haml @@ -45,5 +45,6 @@ invited: @invited_members, access_requests: @requesters, include_relations: @include_relations, - search: params[:search_groups]) } } + search: params[:search_groups], + pending_members: @pending_promotion_members) } } = gl_loading_icon(css_class: 'gl-my-5', size: 'md') diff --git a/ee/app/controllers/ee/groups/group_members_controller.rb b/ee/app/controllers/ee/groups/group_members_controller.rb index e0f62fd0b08ca..0d872a0a9b480 100644 --- a/ee/app/controllers/ee/groups/group_members_controller.rb +++ b/ee/app/controllers/ee/groups/group_members_controller.rb @@ -6,6 +6,8 @@ module GroupMembersController extend ActiveSupport::Concern extend ::Gitlab::Utils::Override + MEMBER_PER_PAGE_LIMIT = 50 + class_methods do extend ::Gitlab::Utils::Override @@ -36,6 +38,7 @@ def index # rubocop:disable Gitlab/ModuleWithInstanceVariables @banned = presented_banned_members @memberships_with_custom_role = present_group_members(group_memberships_with_custom_role) + @pending_promotion_members = pending_promotion_members # rubocop:enable Gitlab/ModuleWithInstanceVariables end @@ -158,6 +161,16 @@ def banned_members(params: {}) .execute(include_relations: %i[direct descendants]) .banned_from(group) end + + 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) + end 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 a8fa5477e3f80..b14f681c90449 100644 --- a/ee/app/controllers/ee/projects/project_members_controller.rb +++ b/ee/app/controllers/ee/projects/project_members_controller.rb @@ -6,6 +6,17 @@ module ProjectMembersController extend ActiveSupport::Concern extend ::Gitlab::Utils::Override + MEMBER_PER_PAGE_LIMIT = 50 + + override :index + def index + super + + # rubocop:disable Gitlab/ModuleWithInstanceVariables -- Need to initialize pending members + @pending_promotion_members = pending_promotion_members + # rubocop:enable Gitlab/ModuleWithInstanceVariables + end + private prepended do @@ -23,6 +34,16 @@ def invited_members def non_invited_members super.non_awaiting end + + 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) + end end end end diff --git a/ee/app/helpers/ee/groups/group_members_helper.rb b/ee/app/helpers/ee/groups/group_members_helper.rb index 1e50d6ef68245..560639ad69b4a 100644 --- a/ee/app/helpers/ee/groups/group_members_helper.rb +++ b/ee/app/helpers/ee/groups/group_members_helper.rb @@ -12,13 +12,16 @@ def group_members_list_data(group, _members, _pagination = {}) end override :group_members_app_data - def group_members_app_data(group, members:, invited:, access_requests:, banned:, include_relations:, search:) + def group_members_app_data( + group, members:, invited:, access_requests:, banned:, include_relations:, search:, pending_members: + ) super.merge!({ can_export_members: can?(current_user, :export_group_memberships, group), export_csv_path: export_csv_group_group_members_path(group), can_filter_by_enterprise: group.domain_verification_available? && can?(current_user, :admin_group_member, group), banned: group_members_list_data(group, banned), - manage_member_roles_path: manage_member_roles_path(group) + manage_member_roles_path: manage_member_roles_path(group), + promotion_request: pending_members.present? ? promotion_pending_members_list_data(pending_members) : [] }) end diff --git a/ee/app/helpers/ee/members_helper.rb b/ee/app/helpers/ee/members_helper.rb index dc5da0df5dcb7..c80d33cbf248f 100644 --- a/ee/app/helpers/ee/members_helper.rb +++ b/ee/app/helpers/ee/members_helper.rb @@ -9,6 +9,20 @@ def members_page? private + def promotion_pending_members_list_data(pending_promotion_members) + pagination = { param_name: :promotion_requests_page, params: { page: nil } } + { + data: promotion_pending_members_serialized(pending_promotion_members), + pagination: members_pagination_data(pending_promotion_members, pagination) + } + end + + def promotion_pending_members_serialized(pending_promotion_members) + ::MemberManagement::MemberApprovalSerializer.new.represent( + pending_promotion_members, { current_user: current_user } + ) + end + def member_header_manage_namespace_members_text(namespace) manage_text = _( 'To manage seats for all members associated with this group and its subgroups and projects, ' \ diff --git a/ee/app/helpers/ee/projects/project_members_helper.rb b/ee/app/helpers/ee/projects/project_members_helper.rb index e477d1bc09e50..b918e91152d29 100644 --- a/ee/app/helpers/ee/projects/project_members_helper.rb +++ b/ee/app/helpers/ee/projects/project_members_helper.rb @@ -6,8 +6,13 @@ module ProjectMembersHelper extend ::Gitlab::Utils::Override override :project_members_app_data - def project_members_app_data(project, ...) - super.merge(manage_member_roles_path: manage_member_roles_path(project)) + def project_members_app_data( + project, members:, invited:, access_requests:, include_relations:, search:, pending_members: + ) + super.merge( + manage_member_roles_path: manage_member_roles_path(project), + promotion_request: pending_members.present? ? promotion_pending_members_list_data(pending_members) : [] + ) end def project_member_header_subtext(project) diff --git a/ee/app/presenters/ee/members/member_approval_presenter.rb b/ee/app/presenters/ee/members/member_approval_presenter.rb new file mode 100644 index 0000000000000..f37d9a0241388 --- /dev/null +++ b/ee/app/presenters/ee/members/member_approval_presenter.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module EE + module Members + module MemberApprovalPresenter + extend ActiveSupport::Concern + + def human_new_access_level + ::Gitlab::Access.human_access(new_access_level) + end + + def human_old_access_level + ::Gitlab::Access.human_access(old_access_level) + end + + def source_id + return member_namespace.project.id if project_namespace? + + member_namespace.id + end + + def source_name + member_namespace.name + end + + def source_web_url + return member_namespace.project.web_url if project_namespace? + + member_namespace.web_url + end + + private + + def project_namespace? + member_namespace.is_a?(::Namespaces::ProjectNamespace) + end + end + end +end diff --git a/ee/app/serializers/member_management/member_approval_entity.rb b/ee/app/serializers/member_management/member_approval_entity.rb new file mode 100644 index 0000000000000..660aa57feb9c7 --- /dev/null +++ b/ee/app/serializers/member_management/member_approval_entity.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +module MemberManagement + class MemberApprovalEntity < Grape::Entity + include RequestAwareEntity + + expose :id + expose :created_at + expose :updated_at + + expose :requested_by, if: ->(member_approval) { member_approval.requested_by.present? } do |member_approval| + UserEntity.represent(member_approval.requested_by, only: [:name, :web_url]) + end + + expose :reviewed_by, if: ->(member_approval) { member_approval.reviewed_by.present? } do |member_approval| + UserEntity.represent(member_approval.reviewed_by, only: [:name, :web_url]) + end + + expose :new_access_level do + expose :human_new_access_level, as: :string_value + expose :new_access_level, as: :integer_value + expose :member_role_id + end + + expose :old_access_level, if: ->(member_approval) { member_approval.old_access_level.present? } do + expose :human_old_access_level, as: :string_value + expose :old_access_level, as: :integer_value + end + + expose :source do + expose :source_id, as: :id + expose :source_name, as: :full_name + expose :source_web_url, as: :web_url + end + + expose :user do |member_approval| + MemberUserEntity.represent(member_approval.user, options) + end + + private + + alias_method :member_approval, :object + + def presenter + @member_presenter ||= member_approval.present + end + + def human_new_access_level + presenter.human_new_access_level + end + + def human_old_access_level + presenter.human_old_access_level + end + + def source_id + presenter.source_id + end + + def source_name + presenter.source_name + end + + def source_web_url + presenter.source_web_url + end + end +end diff --git a/ee/app/serializers/member_management/member_approval_serializer.rb b/ee/app/serializers/member_management/member_approval_serializer.rb new file mode 100644 index 0000000000000..f1cc071875354 --- /dev/null +++ b/ee/app/serializers/member_management/member_approval_serializer.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module MemberManagement + class MemberApprovalSerializer < BaseSerializer + entity MemberApprovalEntity + end +end diff --git a/ee/spec/helpers/ee/groups/group_members_helper_spec.rb b/ee/spec/helpers/ee/groups/group_members_helper_spec.rb index f5aeca63fdcad..c70f8a11c8d95 100644 --- a/ee/spec/helpers/ee/groups/group_members_helper_spec.rb +++ b/ee/spec/helpers/ee/groups/group_members_helper_spec.rb @@ -21,7 +21,8 @@ access_requests: [], banned: banned, include_relations: [:inherited, :direct], - search: nil + search: nil, + pending_members: [] ) end @@ -84,6 +85,26 @@ expect(subject[:banned][:member_path]).to eq('/groups/foo-bar/-/group_members/:id') end end + + context 'with promotion_request' do + let(:type) { :for_group_member } + let(:member_namespace) { group } + + subject(:helper_app_data) do + helper.group_members_app_data( + group, + members: [], + invited: [], + access_requests: [], + banned: banned, + include_relations: [:inherited, :direct], + search: nil, + pending_members: pending_members + ) + end + + it_behaves_like 'adding promotion_request in app data' + end end describe '#group_member_header_subtext' do diff --git a/ee/spec/helpers/projects/project_members_helper_spec.rb b/ee/spec/helpers/projects/project_members_helper_spec.rb index cba6445a22c0c..9fd5391118cd9 100644 --- a/ee/spec/helpers/projects/project_members_helper_spec.rb +++ b/ee/spec/helpers/projects/project_members_helper_spec.rb @@ -51,7 +51,8 @@ def call_project_members_app_data_json invited: [], access_requests: [], include_relations: [:inherited, :direct], - search: nil + search: nil, + pending_members: [] ) end @@ -71,6 +72,27 @@ def initialize(user) end end + describe '#project_members_app_data' do + subject(:helper_app_data) do + helper.project_members_app_data( + project, + members: [], + invited: [], + access_requests: [], + include_relations: [:inherited, :direct], + search: nil, + pending_members: pending_members + ) + end + + context 'with promotion_request' do + let(:type) { :for_project_member } + let(:member_namespace) { project.project_namespace } + + it_behaves_like 'adding promotion_request in app data' + end + end + describe '#project_member_header_subtext' do let(:base_subtext) { "You can invite a new member to <strong>#{current_project.name}</strong> or invite another group." } let(:standard_subtext) { "^#{base_subtext}$" } diff --git a/ee/spec/presenters/ee/members/member_approval_presenter_spec.rb b/ee/spec/presenters/ee/members/member_approval_presenter_spec.rb new file mode 100644 index 0000000000000..f95909a475c7b --- /dev/null +++ b/ee/spec/presenters/ee/members/member_approval_presenter_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe EE::Members::MemberApprovalPresenter, feature_category: :seat_cost_management do + let(:project_namespace) { build_stubbed(:project_namespace) } + let(:project) { build_stubbed(:project, project_namespace: project_namespace) } + let(:group) { build_stubbed(:group) } + let(:namespace) { project.project_namespace } + let(:member_approval) { build_stubbed(:member_approval, member_namespace: namespace) } + + let(:user) { build_stubbed(:user) } + let(:presenter) { member_approval.present(current_user: user) } + + describe '#human_new_access_level' do + subject(:human_new_access_level) { presenter.human_new_access_level } + + it 'returns the human-readable string for new access level' do + expect(::Gitlab::Access).to receive(:human_access).with(member_approval.new_access_level).and_return('Owner') + + expect(human_new_access_level).to eq('Owner') + end + end + + describe '#human_old_access_level' do + subject(:human_old_access_level) { presenter.human_old_access_level } + + it 'returns the human-readable string for old access level' do + expect(::Gitlab::Access).to receive(:human_access).with(member_approval.old_access_level).and_return('Developer') + + expect(human_old_access_level).to eq('Developer') + end + end + + context 'when member_namespace is a Group' do + let(:namespace) { group } + + it 'has the expected attributes' do + expect(presenter.source_id).to eq(group.id) + expect(presenter.source_web_url).to eq(group.web_url) + end + end + + context 'when member_namespace is a ProjectNamespace' do + it 'has the expected attributes' do + expect(presenter.source_id).to eq(project.id) + expect(presenter.source_web_url).to eq(project.web_url) + end + end + + describe '#source_name' do + subject(:source_name) { presenter.source_name } + + it 'returns the name of the namespace' do + expect(source_name).to eq(member_approval.member_namespace.name) + 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 500cf7126c600..89ee3f6a2d6fd 100644 --- a/ee/spec/requests/groups/group_members_controller_spec.rb +++ b/ee/spec/requests/groups/group_members_controller_spec.rb @@ -15,71 +15,133 @@ end describe 'GET /groups/*group_id/-/group_members' do - let(:banned_member) { create(:group_member, :developer, group: group) } - let(:licensed_feature_available) { true } - - before do - stub_licensed_features(unique_project_download_limit: licensed_feature_available) - - create(:namespace_ban, namespace: group, user: banned_member.user) - end - subject(:request) do - get group_group_members_path(group_id: group) + get group_group_members_path(group_id: group), params: param end - it 'pushes feature flag to frontend' do - request + let!(:param) { {} } - expect(response.body).to have_pushed_frontend_feature_flags(limitUniqueProjectDownloadsPerNamespaceUser: true) - end + context 'with banned members' do + let(:banned_member) { create(:group_member, :developer, group: group) } + let(:licensed_feature_available) { true } - it 'sets @banned to include banned group members' do - request + before do + stub_licensed_features(unique_project_download_limit: licensed_feature_available) - expect(assigns(:banned).map(&:user_id)).to contain_exactly(banned_member.user.id) - end + create(:namespace_ban, namespace: group, user: banned_member.user) + end - it 'sets @members not to include banned group members' do - request + it 'pushes feature flag to frontend' do + request - expect(assigns(:members).map(&:user_id)).not_to include(banned_member.user.id) - end + expect(response.body).to have_pushed_frontend_feature_flags(limitUniqueProjectDownloadsPerNamespaceUser: true) + end - shared_examples 'assigns @banned and @members correctly' do - it 'does not assign @banned' do + it 'sets @banned to include banned group members' do request - expect(assigns(:banned)).to be_nil + expect(assigns(:banned).map(&:user_id)).to contain_exactly(banned_member.user.id) end - it 'sets @members to include banned group members' do + it 'sets @members not to include banned group members' do request - expect(assigns(:members).map(&:user_id)).to include(banned_member.user.id) + expect(assigns(:members).map(&:user_id)).not_to include(banned_member.user.id) end - end - context 'when licensed feature is not available' do - let(:licensed_feature_available) { false } + shared_examples 'assigns @banned and @members correctly' do + it 'does not assign @banned' do + request + + expect(assigns(:banned)).to be_nil + end - it_behaves_like 'assigns @banned and @members correctly' - end + it 'sets @members to include banned group members' do + request - context 'when feature flag is disabled' do - before do - stub_feature_flags(limit_unique_project_downloads_per_namespace_user: false) + expect(assigns(:members).map(&:user_id)).to include(banned_member.user.id) + end end - it_behaves_like 'assigns @banned and @members correctly' + context 'when licensed feature is not available' do + let(:licensed_feature_available) { false } + + it_behaves_like 'assigns @banned and @members correctly' + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(limit_unique_project_downloads_per_namespace_user: false) + end + + it_behaves_like 'assigns @banned and @members correctly' + end + + context 'when sub-group' do + before do + group.update!(parent: create(:group)) + end + + it_behaves_like 'assigns @banned and @members correctly' + end end - context 'when sub-group' do - before do - group.update!(parent: create(:group)) + context 'with member pending promotions' do + let!(:pending_member_approvals) do + create_list(:member_approval, 2, :for_group_member, member_namespace: group) end - it_behaves_like 'assigns @banned and @members correctly' + context 'with member_promotion management feature enabled' do + before do + stub_feature_flags(member_promotion_management: true) + stub_application_setting(enable_member_promotion_management: true) + end + + context 'when user can admin group' do + it 'assigns @pending_promotion_members with the correct pending members' do + request + + expect(assigns(:pending_promotion_members)).to match_array(pending_member_approvals) + end + + context 'with pagination' do + let(:param) { { promotion_requests_page: 2 } } + + it 'paginates @pending_promotion_members correctly' do + stub_const("EE::#{described_class}::MEMBER_PER_PAGE_LIMIT", 1) + + request + + expect(assigns(:pending_promotion_members).size).to eq(1) + expect(assigns(:pending_promotion_members)).to contain_exactly(pending_member_approvals.second) + end + end + end + + context 'when user cannot admin group' do + it 'does not assigns @pending_promotion_members' do + user = create(:user) + sign_in(user) + group.add_developer(user) + + request + + expect(assigns(:pending_promotion_members)).to eq(nil) + end + end + end + + context 'with member_promotion management feature disabled' do + before do + stub_feature_flags(member_promotion_management: false) + end + + it 'assigns @pending_promotion_members as nil' do + request + + expect(assigns(:pending_promotion_members)).to eq(nil) + end + 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 new file mode 100644 index 0000000000000..37e72ec8a5b23 --- /dev/null +++ b/ee/spec/requests/projects/project_members_controller_spec.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::ProjectMembersController, feature_category: :groups_and_projects do + let_it_be(:user) { create(:user) } + let(:group) { create(:group, :public) } + let(:project) { create(:project, :public, namespace: group) } + let(:project_member) { create(:project_member, source: project) } + + before do + group.add_owner(user) + sign_in(user) + end + + describe 'GET /*namespace_id/:project_id/-/project_members' do + subject(:make_request) do + get namespace_project_project_members_path(group, project), params: param + end + + let(:param) { {} } + + context 'with member pending promotions' do + let!(:pending_member_approvals) do + create_list(:member_approval, 2, :for_project_member, member_namespace: project.project_namespace) + end + + context 'with member_promotion management feature enabled' do + before do + stub_feature_flags(member_promotion_management: true) + stub_application_setting(enable_member_promotion_management: true) + end + + context 'when user can admin project' do + it 'assigns @pending_promotion_members' do + make_request + expect(assigns(:pending_promotion_members)).to match_array(pending_member_approvals) + end + + context 'with pagination' do + let(:param) { { promotion_requests_page: 2 } } + + it 'paginates @pending_promotion_members correctly' do + group.add_owner(user) + stub_const("EE::#{described_class}::MEMBER_PER_PAGE_LIMIT", 1) + + make_request + expect(assigns(:pending_promotion_members).size).to eq(1) + expect(assigns(:pending_promotion_members)).to contain_exactly(pending_member_approvals.second) + end + end + end + + context 'when user cannot admin project' do + it 'does not assigns @pending_promotion_members' do + user = create(:user) + sign_in(user) + project.add_developer(user) + + make_request + + expect(assigns(:pending_promotion_members)).to eq(nil) + end + end + end + + context 'with member_promotion management feature disabled' do + before do + stub_feature_flags(member_promotion_management: false) + end + + it 'assigns @pending_promotion_members as nil' do + make_request + + expect(assigns(:pending_promotion_members)).to eq(nil) + end + end + end + end +end diff --git a/ee/spec/serializers/member_management/member_approval_entity_spec.rb b/ee/spec/serializers/member_management/member_approval_entity_spec.rb new file mode 100644 index 0000000000000..2b78816532de2 --- /dev/null +++ b/ee/spec/serializers/member_management/member_approval_entity_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MemberManagement::MemberApprovalEntity, feature_category: :seat_cost_management do + let(:group) { build_stubbed(:group) } + let(:pending_member_approval) { build_stubbed(:member_approval, member_namespace: group) } + + subject(:member_approval_entity) { described_class.new(pending_member_approval) } + + describe '#as_json' do + it 'includes member approval attributes' do + json_response = member_approval_entity.as_json + + expect(json_response.keys).to match_array([:created_at, :id, :new_access_level, :old_access_level, + :requested_by, :reviewed_by, :source, :updated_at, :user]) + expect(json_response[:new_access_level].keys).to match_array([:integer_value, :member_role_id, :string_value]) + expect(json_response[:old_access_level].keys).to match_array([:integer_value, :string_value]) + expect(json_response[:source].keys).to match_array([:full_name, :id, :web_url]) + expect(json_response[:reviewed_by].keys).to match_array([:name, :web_url]) + expect(json_response[:requested_by].keys).to match_array([:name, :web_url]) + end + end + + describe 'when assigning the member presenter' do + it 'is only set once' do + expect(Members::MemberApprovalPresenter).to receive(:new) + .with(pending_member_approval) + .and_call_original + .once + member_approval_entity.as_json + end + end +end diff --git a/ee/spec/serializers/member_management/member_approval_serializer_spec.rb b/ee/spec/serializers/member_management/member_approval_serializer_spec.rb new file mode 100644 index 0000000000000..e33bf23801c6c --- /dev/null +++ b/ee/spec/serializers/member_management/member_approval_serializer_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MemberManagement::MemberApprovalSerializer, feature_category: :seat_cost_management do + describe '#represent' do + context 'when there are no member approvals' do + it 'returns an empty array' do + result = described_class.new.represent([]) + + expect(result).to eq([]) + end + end + + context 'when there are member approvals' do + it 'returns member approval attributes' do + group = build_stubbed(:group) + pending_member_approval = build_stubbed(:member_approval, member_namespace: group) + pending_member_approvals = [pending_member_approval] + + result = described_class.new.represent(pending_member_approvals) + + expect(result.first.keys).to match_array([:created_at, :id, :new_access_level, :old_access_level, + :requested_by, :reviewed_by, :source, :updated_at, :user]) + expect(result.first[:new_access_level].keys).to match_array([:integer_value, :member_role_id, :string_value]) + expect(result.first[:old_access_level].keys).to match_array([:integer_value, :string_value]) + expect(result.first[:source].keys).to match_array([:full_name, :id, :web_url]) + expect(result.first[:reviewed_by].keys).to match_array([:name, :web_url]) + expect(result.first[:requested_by].keys).to match_array([:name, :web_url]) + end + end + end +end diff --git a/ee/spec/support/shared_examples/helpers/promotion_request_shared_examples.rb b/ee/spec/support/shared_examples/helpers/promotion_request_shared_examples.rb new file mode 100644 index 0000000000000..a74bf412b3873 --- /dev/null +++ b/ee/spec/support/shared_examples/helpers/promotion_request_shared_examples.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'adding promotion_request in app data' do + context 'when pending_members is nil' do + let!(:pending_members) { nil } + + it 'returns `promotion_request` property with []' do + expect(helper_app_data[:promotion_request]).to eq [] + end + end + + context 'when pending_members is not nil' do + let!(:pending_members) do + create_list(:member_approval, 2, type, member_namespace: member_namespace) + end + + it 'returns valid `promotion_request`' do + expect(helper_app_data[:promotion_request].keys).to match_array([:data, :pagination]) + expect(helper_app_data[:promotion_request][:data].size).to eq(2) + expect(helper_app_data[:promotion_request][:data].first.keys).to match_array [:id, :created_at, + :updated_at, :requested_by, :reviewed_by, :new_access_level, :old_access_level, :source, :user] + end + end +end diff --git a/spec/helpers/groups/group_members_helper_spec.rb b/spec/helpers/groups/group_members_helper_spec.rb index 3a7c852b683a2..98db6650d0433 100644 --- a/spec/helpers/groups/group_members_helper_spec.rb +++ b/spec/helpers/groups/group_members_helper_spec.rb @@ -32,7 +32,8 @@ access_requests: present_members(access_requests), banned: [], include_relations: [:inherited, :direct], - search: nil + search: nil, + pending_members: [] ) end @@ -107,7 +108,8 @@ access_requests: present_members(access_requests), banned: [], include_relations: include_relations, - search: nil + search: nil, + pending_members: [] ) end diff --git a/spec/helpers/projects/project_members_helper_spec.rb b/spec/helpers/projects/project_members_helper_spec.rb index 0a85b28c91911..e43d4612701e4 100644 --- a/spec/helpers/projects/project_members_helper_spec.rb +++ b/spec/helpers/projects/project_members_helper_spec.rb @@ -28,7 +28,8 @@ invited: present_members(invited), access_requests: present_members(access_requests), include_relations: [:inherited, :direct], - search: nil + search: nil, + pending_members: [] ) ) end @@ -134,7 +135,8 @@ invited: present_members(invited), access_requests: present_members(access_requests), include_relations: include_relations, - search: nil + search: nil, + pending_members: [] ) ) end -- GitLab