From f308529e9616b3ccf5af59d55bfd765ced90f0c9 Mon Sep 17 00:00:00 2001 From: Alex Buijs <abuijs@gitlab.com> Date: Mon, 29 Jan 2024 20:28:20 +0000 Subject: [PATCH] Add manage roles link to roles dropdowm Add footer to role select dropdown in members table and invite modal Changelog: added EE: true --- .../components/invite_modal_base.vue | 8 ++- .../init_invite_members_modal.js | 1 + .../members/components/table/max_role.vue | 3 + app/assets/javascripts/members/index.js | 2 + .../projects/project_members_helper.rb | 28 ++++++---- .../action_dropdowns/ldap_dropdown_footer.vue | 14 ++--- .../manage_roles_dropdown_footer.vue | 26 +++++++++ .../helpers/ee/groups/group_members_helper.rb | 3 +- ee/app/helpers/ee/invite_members_helper.rb | 2 + .../ee/projects/project_members_helper.rb | 7 +++ ee/app/helpers/member_roles_helper.rb | 14 +++++ .../members/override_ldap_memberships_spec.rb | 5 +- .../components/invite_modal_base_spec.js | 7 +++ .../ldap_dropdown_item_spec.js | 4 +- .../manage_roles_dropdown_footer_spec.js | 34 +++++++++++ .../members/components/table/max_role_spec.js | 7 +++ .../ee/groups/group_members_helper_spec.rb | 5 ++ .../helpers/ee/invite_members_helper_spec.rb | 12 ++++ ee/spec/helpers/member_roles_helper_spec.rb | 56 +++++++++++++++++++ .../projects/project_members_helper_spec.rb | 7 +++ locale/gitlab.pot | 5 +- 21 files changed, 224 insertions(+), 26 deletions(-) create mode 100644 ee/app/assets/javascripts/members/components/action_dropdowns/manage_roles_dropdown_footer.vue create mode 100644 ee/app/helpers/member_roles_helper.rb create mode 100644 ee/spec/frontend/members/components/action_dropdowns/manage_roles_dropdown_footer_spec.js create mode 100644 ee/spec/helpers/member_roles_helper_spec.rb diff --git a/app/assets/javascripts/invite_members/components/invite_modal_base.vue b/app/assets/javascripts/invite_members/components/invite_modal_base.vue index 574bbacc498de..04dc1cd376d8a 100644 --- a/app/assets/javascripts/invite_members/components/invite_modal_base.vue +++ b/app/assets/javascripts/invite_members/components/invite_modal_base.vue @@ -45,6 +45,8 @@ export default { GlSprintf, GlButton, ContentTransition, + ManageRolesDropdownFooter: () => + import('ee_component/members/components/action_dropdowns/manage_roles_dropdown_footer.vue'), }, mixins: [Tracking.mixin()], inheritAttrs: false, @@ -332,7 +334,11 @@ export default { :items="accessLevelOptions.formatted" :loading="isLoadingRoles" block - /> + > + <template #footer> + <manage-roles-dropdown-footer /> + </template> + </gl-collapsible-listbox> </gl-form-group> <gl-form-group diff --git a/app/assets/javascripts/invite_members/init_invite_members_modal.js b/app/assets/javascripts/invite_members/init_invite_members_modal.js index bd291ecc90f33..361ad00789f8b 100644 --- a/app/assets/javascripts/invite_members/init_invite_members_modal.js +++ b/app/assets/javascripts/invite_members/init_invite_members_modal.js @@ -28,6 +28,7 @@ export default (function initInviteMembersModal() { newUsersUrl: el.dataset.newUsersUrl, isCurrentUserAdmin: parseBoolean(el.dataset.isCurrentUserAdmin), isEmailSignupEnabled: parseBoolean(el.dataset.isSignupEnabled), + manageMemberRolesPath: el.dataset.manageMemberRolesPath, }, render: (createElement) => createElement(InviteMembersModal, { diff --git a/app/assets/javascripts/members/components/table/max_role.vue b/app/assets/javascripts/members/components/table/max_role.vue index 8978010851863..92006e73678d7 100644 --- a/app/assets/javascripts/members/components/table/max_role.vue +++ b/app/assets/javascripts/members/components/table/max_role.vue @@ -16,6 +16,8 @@ export default { LdapDropdownFooter: () => import('ee_component/members/components/action_dropdowns/ldap_dropdown_footer.vue'), CustomPermissions: () => import('ee_component/members/components/table/custom_permissions.vue'), + ManageRolesDropdownFooter: () => + import('ee_component/members/components/action_dropdowns/manage_roles_dropdown_footer.vue'), }, inject: ['namespace', 'group'], props: { @@ -120,6 +122,7 @@ export default { v-if="permissions.canOverride && member.isOverridden" :member-id="member.id" /> + <manage-roles-dropdown-footer /> </template> </gl-collapsible-listbox> diff --git a/app/assets/javascripts/members/index.js b/app/assets/javascripts/members/index.js index ad477d8b4b6b1..6962503152e78 100644 --- a/app/assets/javascripts/members/index.js +++ b/app/assets/javascripts/members/index.js @@ -27,6 +27,7 @@ export const initMembersApp = (el, options) => { exportCsvPath, groupName, groupPath, + manageMemberRolesPath, ...vuexStoreAttributes } = parseDataAttributes(el); @@ -73,6 +74,7 @@ export const initMembersApp = (el, options) => { canFilterByEnterprise, canExportMembers, exportCsvPath, + manageMemberRolesPath, group: { name: groupName, path: groupPath, diff --git a/app/helpers/projects/project_members_helper.rb b/app/helpers/projects/project_members_helper.rb index 20af643d159ef..3385dd4a043d8 100644 --- a/app/helpers/projects/project_members_helper.rb +++ b/app/helpers/projects/project_members_helper.rb @@ -1,18 +1,8 @@ # frozen_string_literal: true module Projects::ProjectMembersHelper - def project_members_app_data_json(project, members:, invited:, access_requests:, include_relations:, search:) - { - user: project_members_list_data(project, members, { param_name: :page, params: { search_groups: nil } }), - group: project_group_links_list_data(project, include_relations, search), - invite: project_members_list_data(project, invited.nil? ? [] : invited), - access_request: project_members_list_data(project, access_requests.nil? ? [] : access_requests), - source_id: project.id, - can_manage_members: Ability.allowed?(current_user, :admin_project_member, project), - can_manage_access_requests: Ability.allowed?(current_user, :admin_member_access_request, project), - group_name: project.group&.name, - group_path: project.group&.full_path - }.to_json + def project_members_app_data_json(...) + project_members_app_data(...).to_json end def project_member_header_subtext(project) @@ -28,6 +18,20 @@ def project_member_header_subtext(project) private + def project_members_app_data(project, members:, invited:, access_requests:, include_relations:, search:) + { + user: project_members_list_data(project, members, { param_name: :page, params: { search_groups: nil } }), + group: project_group_links_list_data(project, include_relations, search), + invite: project_members_list_data(project, invited.nil? ? [] : invited), + access_request: project_members_list_data(project, access_requests.nil? ? [] : access_requests), + source_id: project.id, + can_manage_members: Ability.allowed?(current_user, :admin_project_member, project), + can_manage_access_requests: Ability.allowed?(current_user, :admin_member_access_request, project), + group_name: project.group&.name, + group_path: project.group&.full_path + } + end + def share_project_description(project) share_with_group = project.allowed_to_share_with_group? share_with_members = !membership_locked? diff --git a/ee/app/assets/javascripts/members/components/action_dropdowns/ldap_dropdown_footer.vue b/ee/app/assets/javascripts/members/components/action_dropdowns/ldap_dropdown_footer.vue index 7ca7ec5655f65..33d4584568cb3 100644 --- a/ee/app/assets/javascripts/members/components/action_dropdowns/ldap_dropdown_footer.vue +++ b/ee/app/assets/javascripts/members/components/action_dropdowns/ldap_dropdown_footer.vue @@ -1,5 +1,5 @@ <script> -import { GlListboxItem } from '@gitlab/ui'; +import { GlButton } from '@gitlab/ui'; // eslint-disable-next-line no-restricted-imports import { mapActions } from 'vuex'; import { s__ } from '~/locale'; @@ -7,7 +7,7 @@ import { s__ } from '~/locale'; export default { name: 'LdapDropdownFooter', components: { - GlListboxItem, + GlButton, }, inject: ['namespace'], props: { @@ -36,9 +36,9 @@ export default { </script> <template> - <ul class="gl-border-t-1 gl-border-t-solid gl-border-t-gray-200 gl-new-dropdown-contents"> - <gl-listbox-item @select="handleClick"> - {{ s__('Members|Revert to LDAP group sync settings') }} - </gl-listbox-item> - </ul> + <div class="gl-border-t gl-border-gray-200 gl-p-2"> + <gl-button category="tertiary" class="gl-w-full" @click="handleClick"> + {{ s__('Members|Revert to LDAP synced settings') }} + </gl-button> + </div> </template> diff --git a/ee/app/assets/javascripts/members/components/action_dropdowns/manage_roles_dropdown_footer.vue b/ee/app/assets/javascripts/members/components/action_dropdowns/manage_roles_dropdown_footer.vue new file mode 100644 index 0000000000000..b86aaddcfa5b3 --- /dev/null +++ b/ee/app/assets/javascripts/members/components/action_dropdowns/manage_roles_dropdown_footer.vue @@ -0,0 +1,26 @@ +<script> +import { GlButton } from '@gitlab/ui'; + +export default { + components: { + GlButton, + }, + inject: { + manageMemberRolesPath: { + default: null, + }, + }, +}; +</script> + +<template> + <div v-if="manageMemberRolesPath" class="gl-border-t gl-border-gray-200 gl-p-2"> + <gl-button + :href="manageMemberRolesPath" + category="tertiary" + class="gl-w-full gl-justify-content-start!" + > + {{ s__('MemberRole|Manage roles') }} + </gl-button> + </div> +</template> diff --git a/ee/app/helpers/ee/groups/group_members_helper.rb b/ee/app/helpers/ee/groups/group_members_helper.rb index 30dbed426d534..1e50d6ef68245 100644 --- a/ee/app/helpers/ee/groups/group_members_helper.rb +++ b/ee/app/helpers/ee/groups/group_members_helper.rb @@ -17,7 +17,8 @@ def group_members_app_data(group, members:, invited:, access_requests:, banned:, 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) + banned: group_members_list_data(group, banned), + manage_member_roles_path: manage_member_roles_path(group) }) end diff --git a/ee/app/helpers/ee/invite_members_helper.rb b/ee/app/helpers/ee/invite_members_helper.rb index becc828ca4917..9d9c1fee09ef7 100644 --- a/ee/app/helpers/ee/invite_members_helper.rb +++ b/ee/app/helpers/ee/invite_members_helper.rb @@ -28,6 +28,8 @@ def common_invite_modal_dataset(source) ) end + dataset[:manage_member_roles_path] = manage_member_roles_path(source) + dataset end diff --git a/ee/app/helpers/ee/projects/project_members_helper.rb b/ee/app/helpers/ee/projects/project_members_helper.rb index dcc543d1cdf57..e477d1bc09e50 100644 --- a/ee/app/helpers/ee/projects/project_members_helper.rb +++ b/ee/app/helpers/ee/projects/project_members_helper.rb @@ -3,6 +3,13 @@ module EE module Projects 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)) + end + def project_member_header_subtext(project) if project.group && ::Namespaces::FreeUserCap::Enforcement.new(project.root_ancestor).enforce_cap? && diff --git a/ee/app/helpers/member_roles_helper.rb b/ee/app/helpers/member_roles_helper.rb new file mode 100644 index 0000000000000..65b00ea886a61 --- /dev/null +++ b/ee/app/helpers/member_roles_helper.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module MemberRolesHelper + def manage_member_roles_path(source) + root_group = source&.root_ancestor + return unless root_group&.custom_roles_enabled? + + if ::Gitlab::Saas.feature_available?(:group_custom_roles) && can?(current_user, :admin_group_member, root_group) + group_settings_roles_and_permissions_path(root_group) + elsif current_user&.can_admin_all_resources? + admin_application_settings_roles_and_permissions_path + end + end +end diff --git a/ee/spec/features/groups/members/override_ldap_memberships_spec.rb b/ee/spec/features/groups/members/override_ldap_memberships_spec.rb index 25b71bb5bf7c9..3cdcdf3bf3c7e 100644 --- a/ee/spec/features/groups/members/override_ldap_memberships_spec.rb +++ b/ee/spec/features/groups/members/override_ldap_memberships_spec.rb @@ -81,7 +81,8 @@ expect(page).not_to have_selector user_action_dropdown expect(page).to have_button 'Guest', disabled: false - select_from_listbox('Revert to LDAP group sync settings', from: 'Guest') + click_button 'Guest' + click_button 'Revert to LDAP synced settings' wait_for_requests @@ -97,7 +98,7 @@ click_button 'Guest' - expect(page).not_to have_content 'Revert to LDAP group sync settings' + expect(page).not_to have_content 'Revert to LDAP synced settings' end end end diff --git a/ee/spec/frontend/invite_members/components/invite_modal_base_spec.js b/ee/spec/frontend/invite_members/components/invite_modal_base_spec.js index f78a4074a44ee..dc433cc6b0298 100644 --- a/ee/spec/frontend/invite_members/components/invite_modal_base_spec.js +++ b/ee/spec/frontend/invite_members/components/invite_modal_base_spec.js @@ -7,6 +7,7 @@ import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import ContentTransition from '~/vue_shared/components/content_transition.vue'; import CEInviteModalBase from '~/invite_members/components/invite_modal_base.vue'; import EEInviteModalBase from 'ee/invite_members/components/invite_modal_base.vue'; +import ManageRolesDropdownFooter from 'ee/members/components/action_dropdowns/manage_roles_dropdown_footer.vue'; import { OVERAGE_MODAL_TITLE, OVERAGE_MODAL_CONTINUE_BUTTON, @@ -413,4 +414,10 @@ describe('EEInviteModalBase', () => { expect(findCEBase().props('invalidFeedbackMessage')).toBe('invalid message'); }); }); + + it('renders the ManageRolesDropdownFooter component', () => { + createComponent(); + + expect(wrapper.findComponent(ManageRolesDropdownFooter).exists()).toBe(true); + }); }); diff --git a/ee/spec/frontend/members/components/action_dropdowns/ldap_dropdown_item_spec.js b/ee/spec/frontend/members/components/action_dropdowns/ldap_dropdown_item_spec.js index 2f335e2eef9c4..c46cc45844124 100644 --- a/ee/spec/frontend/members/components/action_dropdowns/ldap_dropdown_item_spec.js +++ b/ee/spec/frontend/members/components/action_dropdowns/ldap_dropdown_item_spec.js @@ -1,4 +1,4 @@ -import { GlListboxItem } from '@gitlab/ui'; +import { GlButton } from '@gitlab/ui'; import { mount } from '@vue/test-utils'; import Vue from 'vue'; // eslint-disable-next-line no-restricted-imports @@ -51,7 +51,7 @@ describe('LdapDropdownFooter', () => { beforeEach(() => { createComponent(); - wrapper.findComponent(GlListboxItem).trigger('click'); + wrapper.findComponent(GlButton).trigger('click'); }); it('calls `updateLdapOverride` action', () => { diff --git a/ee/spec/frontend/members/components/action_dropdowns/manage_roles_dropdown_footer_spec.js b/ee/spec/frontend/members/components/action_dropdowns/manage_roles_dropdown_footer_spec.js new file mode 100644 index 0000000000000..ec2e075813263 --- /dev/null +++ b/ee/spec/frontend/members/components/action_dropdowns/manage_roles_dropdown_footer_spec.js @@ -0,0 +1,34 @@ +import { GlButton } from '@gitlab/ui'; +import { shallowMount } from '@vue/test-utils'; +import ManageRolesDropdownFooter from 'ee/members/components/action_dropdowns/manage_roles_dropdown_footer.vue'; + +describe('ManageRolesDropdownFooter', () => { + let wrapper; + + const manageMemberRolesPath = 'some path'; + + const createComponent = (provide = {}) => { + wrapper = shallowMount(ManageRolesDropdownFooter, { + provide: { + manageMemberRolesPath, + ...provide, + }, + }); + }; + + const findButton = () => wrapper.findComponent(GlButton); + + it('renders a button with the correct text and link', () => { + createComponent(); + + expect(findButton().exists()).toBe(true); + expect(findButton().text()).toBe('Manage roles'); + expect(findButton().attributes('href')).toBe(manageMemberRolesPath); + }); + + it('renders no button when no `manageMemberRolesPath` is provided', () => { + createComponent({ manageMemberRolesPath: null }); + + expect(findButton().exists()).toBe(false); + }); +}); diff --git a/ee/spec/frontend/members/components/table/max_role_spec.js b/ee/spec/frontend/members/components/table/max_role_spec.js index 89aa06b1ce906..d5df495d05fd7 100644 --- a/ee/spec/frontend/members/components/table/max_role_spec.js +++ b/ee/spec/frontend/members/components/table/max_role_spec.js @@ -4,6 +4,7 @@ import Vue from 'vue'; // eslint-disable-next-line no-restricted-imports import Vuex from 'vuex'; import LdapDropdownFooter from 'ee/members/components/action_dropdowns/ldap_dropdown_footer.vue'; +import ManageRolesDropdownFooter from 'ee/members/components/action_dropdowns/manage_roles_dropdown_footer.vue'; import { guestOverageConfirmAction } from 'ee/members/guest_overage_confirm_action'; import waitForPromises from 'helpers/wait_for_promises'; import MaxRole from '~/members/components/table/max_role.vue'; @@ -156,6 +157,12 @@ describe('MaxRole', () => { }); }); + it('renders the ManageRolesDropdownFooter component', () => { + createComponent(); + + expect(wrapper.findComponent(ManageRolesDropdownFooter).exists()).toBe(true); + }); + describe('when member has custom roles', () => { it('renders static and custom roles', () => { createComponent(); 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 7767326c65018..e0990fc2cb18b 100644 --- a/ee/spec/helpers/ee/groups/group_members_helper_spec.rb +++ b/ee/spec/helpers/ee/groups/group_members_helper_spec.rb @@ -31,6 +31,7 @@ before do allow(helper).to receive(:override_group_group_member_path).with(group, ':id').and_return('/groups/foo-bar/-/group_members/:id/override') allow(helper).to receive(:group_group_member_path).with(group, ':id').and_return('/groups/foo-bar/-/group_members/:id') + allow(helper).to receive(:manage_member_roles_path).with(group).and_return(admin_application_settings_roles_and_permissions_path) allow(helper).to receive(:can?).with(current_user, :admin_group_member, group).and_return(true) allow(helper).to receive(:can?).with(current_user, :admin_member_access_request, group).and_return(true) allow(helper).to receive(:can?).with(current_user, :export_group_memberships, group).and_return(true) @@ -48,6 +49,10 @@ expect(subject[:export_csv_path]).not_to be_nil end + it 'adds `manage_member_roles_path`' do + expect(subject[:manage_member_roles_path]).to eq(admin_application_settings_roles_and_permissions_path) + end + describe '`can_filter_by_enterprise`', :saas do where(:domain_verification_availabe_for_group, :can_admin_group_member, :expected_value) do true | true | true diff --git a/ee/spec/helpers/ee/invite_members_helper_spec.rb b/ee/spec/helpers/ee/invite_members_helper_spec.rb index 0b608f7901f99..82ff4bee3914b 100644 --- a/ee/spec/helpers/ee/invite_members_helper_spec.rb +++ b/ee/spec/helpers/ee/invite_members_helper_spec.rb @@ -127,6 +127,18 @@ expect(helper.common_invite_modal_dataset(namespace)).not_to have_key(:active_trial_dataset) end end + + describe 'including the manage_member_roles_path' do + before do + allow(helper).to receive(:manage_member_roles_path).with(project) + .and_return(admin_application_settings_roles_and_permissions_path) + end + + it 'does not include users limit notification data' do + expect(helper.common_invite_modal_dataset(project)) + .to include(manage_member_roles_path: admin_application_settings_roles_and_permissions_path) + end + end end describe '#users_filter_data' do diff --git a/ee/spec/helpers/member_roles_helper_spec.rb b/ee/spec/helpers/member_roles_helper_spec.rb new file mode 100644 index 0000000000000..3da1b9cafe7ab --- /dev/null +++ b/ee/spec/helpers/member_roles_helper_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MemberRolesHelper, feature_category: :permissions do + let_it_be(:user) { build_stubbed(:user) } + let_it_be(:source) { build_stubbed(:group) } + let_it_be(:root_group) { source.root_ancestor } + + before do + stub_licensed_features(custom_roles: true) + allow(helper).to receive(:current_user).and_return(user) + end + + describe '#manage_member_roles_path' do + subject { helper.manage_member_roles_path(source) } + + context 'when on saas', :saas do + it { is_expected.to be_nil } + + context 'as owner' do + before do + allow(helper).to receive(:can?).with(user, :admin_group_member, root_group).and_return(true) + end + + it { is_expected.to eq(group_settings_roles_and_permissions_path(root_group)) } + + context 'when custom roles are not available' do + before do + stub_licensed_features(custom_roles: false) + end + + it { is_expected.to be_nil } + end + end + end + + context 'when in admin mode', :enable_admin_mode do + it { is_expected.to be_nil } + + context 'as admin' do + let_it_be(:user) { build_stubbed(:user, :admin) } + + it { is_expected.to eq(admin_application_settings_roles_and_permissions_path) } + + context 'when custom roles are not available' do + before do + stub_licensed_features(custom_roles: false) + end + + it { is_expected.to be_nil } + end + end + end + end +end diff --git a/ee/spec/helpers/projects/project_members_helper_spec.rb b/ee/spec/helpers/projects/project_members_helper_spec.rb index 359ad1eac6543..cba6445a22c0c 100644 --- a/ee/spec/helpers/projects/project_members_helper_spec.rb +++ b/ee/spec/helpers/projects/project_members_helper_spec.rb @@ -19,6 +19,8 @@ create_schedule_with_user(project, current_user) allow(helper).to receive(:can_admin_project_member?).and_return(true) allow(helper).to receive(:can?).and_return(true) + allow(helper).to receive(:manage_member_roles_path).with(project) + .and_return(admin_application_settings_roles_and_permissions_path) end it 'does not execute N+1' do @@ -37,6 +39,11 @@ expect { call_project_members_app_data_json }.not_to exceed_query_limit(control).with_threshold(11) # existing n+1 end + it 'includes `manage_member_roles_path` data' do + expect(Gitlab::Json.parse(call_project_members_app_data_json)) + .to include('manage_member_roles_path' => admin_application_settings_roles_and_permissions_path) + end + def call_project_members_app_data_json helper.project_members_app_data_json( project, diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 7b59eded61feb..8ddbd061f0b40 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -30168,6 +30168,9 @@ msgstr "" msgid "MemberRole|Incident manager" msgstr "" +msgid "MemberRole|Manage roles" +msgstr "" + msgid "MemberRole|Name" msgstr "" @@ -30373,7 +30376,7 @@ msgstr "" msgid "Members|Remove group" msgstr "" -msgid "Members|Revert to LDAP group sync settings" +msgid "Members|Revert to LDAP synced settings" msgstr "" msgid "Members|Reverted to LDAP group sync settings." -- GitLab