From 9d782f46a8c9e2dc1e68d48faf51a36d774ef9b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20Sanz=20Garc=C3=ADa?= <esanz-garcia@gitlab.com> Date: Tue, 11 Jul 2023 21:15:58 +0200 Subject: [PATCH 1/4] Create UI to list and create custom roles in SM New UI to list and create custom (member) roles. The UI is based on personal access token UI, which was updated recently. We turned on the `custom_roles_ui_self_managed` feature flag. This flag was used for incremental development. Instead of removing it, we turned it on, just in case it causes problems. Changelog: changed EE: true --- app/assets/javascripts/api.js | 2 + .../components/entity_select/group_select.vue | 17 +- .../components/entity_select/utils.js | 26 +- config/application.rb | 1 + .../roles_and_permissions/index.js | 3 + .../components/create_member_role.vue | 203 +++++++++++ .../components/list_member_roles.vue | 242 +++++++++++++ .../roles_and_permissions_self_managed.vue | 36 ++ .../roles_and_permissions/constants.js | 88 +++++ .../roles_and_permissions/index.js | 18 + .../page_bundles/member_roles.scss | 7 + .../roles_and_permissions/index.html.haml | 4 +- .../custom_roles_ui_self_managed.yml | 2 +- .../components/create_member_role_spec.js | 149 ++++++++ .../components/list_member_roles_spec.js | 334 ++++++++++++++++++ ...roles_and_permissions_self_managed_spec.js | 43 +++ locale/gitlab.pot | 93 +++++ 17 files changed, 1251 insertions(+), 17 deletions(-) create mode 100644 ee/app/assets/javascripts/pages/admin/application_settings/roles_and_permissions/index.js create mode 100644 ee/app/assets/javascripts/roles_and_permissions/components/create_member_role.vue create mode 100644 ee/app/assets/javascripts/roles_and_permissions/components/list_member_roles.vue create mode 100644 ee/app/assets/javascripts/roles_and_permissions/components/roles_and_permissions_self_managed.vue create mode 100644 ee/app/assets/javascripts/roles_and_permissions/constants.js create mode 100644 ee/app/assets/javascripts/roles_and_permissions/index.js create mode 100644 ee/app/assets/stylesheets/page_bundles/member_roles.scss create mode 100644 ee/spec/frontend/roles_and_permissions/components/create_member_role_spec.js create mode 100644 ee/spec/frontend/roles_and_permissions/components/list_member_roles_spec.js create mode 100644 ee/spec/frontend/roles_and_permissions/components/roles_and_permissions_self_managed_spec.js diff --git a/app/assets/javascripts/api.js b/app/assets/javascripts/api.js index 185cdaa1c996e2..101e27b1d364b5 100644 --- a/app/assets/javascripts/api.js +++ b/app/assets/javascripts/api.js @@ -17,6 +17,8 @@ const Api = { groupPath: '/api/:version/groups/:id', groupMembersPath: '/api/:version/groups/:id/members', groupMilestonesPath: '/api/:version/groups/:id/milestones', + memberRolesPath: '/api/:version/groups/:id/member_roles', + deleteMemberRolesPath: '/api/:version/groups/:id/member_roles/:member_role_id', subgroupsPath: '/api/:version/groups/:id/subgroups', descendantGroupsPath: '/api/:version/groups/:id/descendant_groups', namespacesPath: '/api/:version/namespaces.json', diff --git a/app/assets/javascripts/vue_shared/components/entity_select/group_select.vue b/app/assets/javascripts/vue_shared/components/entity_select/group_select.vue index 71e3bf4ff630af..941963475d053e 100644 --- a/app/assets/javascripts/vue_shared/components/entity_select/group_select.vue +++ b/app/assets/javascripts/vue_shared/components/entity_select/group_select.vue @@ -62,17 +62,14 @@ export default { async fetchGroups(searchString = '', page = 1) { let groups = []; let totalPages = 0; + const params = { + search: searchString, + per_page: DEFAULT_PER_PAGE, + page, + }; try { - const { data = [], headers } = await axios.get( - Api.buildUrl(groupsPath(this.groupsFilter, this.parentGroupID)), - { - params: { - search: searchString, - per_page: DEFAULT_PER_PAGE, - page, - }, - }, - ); + const { url, config } = groupsPath(this.groupsFilter, this.parentGroupID, params); + const { data = [], headers } = await axios.get(url, config); groups = data.map((group) => ({ ...group, text: group.full_name, diff --git a/app/assets/javascripts/vue_shared/components/entity_select/utils.js b/app/assets/javascripts/vue_shared/components/entity_select/utils.js index 0a4622269f45de..c94f3d9940983a 100644 --- a/app/assets/javascripts/vue_shared/components/entity_select/utils.js +++ b/app/assets/javascripts/vue_shared/components/entity_select/utils.js @@ -1,15 +1,31 @@ import Api from '~/api'; -export const groupsPath = (groupsFilter, parentGroupID) => { - if (groupsFilter !== undefined && parentGroupID === undefined) { +/** + * @param {'descendant_groups'|'subgroups'|'top_level_only'|null} groupsFilter - type of group filtering + * @param {string|null} parentGroupID - parent group is needed for 'descendant_groups' and 'subgroups' filtering. + * @param {object} params - optional attributes + */ +export const groupsPath = (groupsFilter, parentGroupID, params = {}) => { + if (['descendant_groups', 'subgroups'].includes(groupsFilter) && parentGroupID === null) { throw new Error('Cannot use groupsFilter without a parentGroupID'); } + const config = { params }; + let url = ''; switch (groupsFilter) { case 'descendant_groups': - return Api.descendantGroupsPath.replace(':id', parentGroupID); + url = Api.descendantGroupsPath.replace(':id', parentGroupID); + break; case 'subgroups': - return Api.subgroupsPath.replace(':id', parentGroupID); + url = Api.subgroupsPath.replace(':id', parentGroupID); + break; + case 'top_level_only': + config.params.top_level_only = true; + url = Api.groupsPath; + break; default: - return Api.groupsPath; + url = Api.groupsPath; + break; } + + return { url: Api.buildUrl(url), config }; }; diff --git a/config/application.rb b/config/application.rb index adf9f29db98c61..d8d992ce97ccad 100644 --- a/config/application.rb +++ b/config/application.rb @@ -318,6 +318,7 @@ class Application < Rails::Application config.assets.precompile << "page_bundles/learn_gitlab.css" config.assets.precompile << "page_bundles/login.css" config.assets.precompile << "page_bundles/marketing_popover.css" + config.assets.precompile << "page_bundles/member_roles.css" config.assets.precompile << "page_bundles/members.css" config.assets.precompile << "page_bundles/merge_conflicts.css" config.assets.precompile << "page_bundles/merge_request_analytics.css" diff --git a/ee/app/assets/javascripts/pages/admin/application_settings/roles_and_permissions/index.js b/ee/app/assets/javascripts/pages/admin/application_settings/roles_and_permissions/index.js new file mode 100644 index 00000000000000..658bab37d45b3d --- /dev/null +++ b/ee/app/assets/javascripts/pages/admin/application_settings/roles_and_permissions/index.js @@ -0,0 +1,3 @@ +import { initRolesAndPermissionsSelfManagedApp } from 'ee/roles_and_permissions'; + +initRolesAndPermissionsSelfManagedApp(); diff --git a/ee/app/assets/javascripts/roles_and_permissions/components/create_member_role.vue b/ee/app/assets/javascripts/roles_and_permissions/components/create_member_role.vue new file mode 100644 index 00000000000000..df5d795a6b7cf7 --- /dev/null +++ b/ee/app/assets/javascripts/roles_and_permissions/components/create_member_role.vue @@ -0,0 +1,203 @@ +<script> +import { + GlButton, + GlForm, + GlFormCheckbox, + GlFormCheckboxGroup, + GlFormGroup, + GlFormInput, + GlFormSelect, + GlFormTextarea, +} from '@gitlab/ui'; +import { ACCESS_LEVEL_LABELS } from '~/access_level/constants'; +import { createAlert, VARIANT_DANGER } from '~/alert'; +import Api from '~/api'; +import axios from '~/lib/utils/axios_utils'; +import { + I18N_CANCEL, + I18N_CREATE_ROLE, + I18N_CREATION_ERROR, + I18N_FIELD_FORM_ERROR, + I18N_NEW_ROLE_BASE_ROLE_DESCRIPTION, + I18N_NEW_ROLE_BASE_ROLE_LABEL, + I18N_NEW_ROLE_DESCRIPTION_LABEL, + I18N_NEW_ROLE_NAME_DESCRIPTION, + I18N_NEW_ROLE_NAME_LABEL, + I18N_NEW_ROLE_NAME_PLACEHOLDER, + PERMISSIONS, +} from '../constants'; + +export default { + name: 'CreateMemberRole', + components: { + GlButton, + GlForm, + GlFormCheckboxGroup, + GlFormCheckbox, + GlFormGroup, + GlFormInput, + GlFormSelect, + GlFormTextarea, + }, + props: { + groupId: { + type: String, + required: true, + }, + }, + data() { + return { + alert: null, + // Remove the default `Guest` role when additional base access roles are supported. + baseRole: '10', + baseRoleValid: true, + description: '', + name: '', + nameValid: true, + permissions: [], + permissionsValid: null, + }; + }, + computed: { + availablePermissions() { + return this.baseRole ? Object.values(PERMISSIONS[this.baseRole]) : []; + }, + }, + methods: { + areFieldsValid() { + this.baseRoleValid = true; + this.nameValid = true; + this.permissionsValid = null; + + if (!this.baseRole) { + this.baseRoleValid = false; + } + + if (!this.name) { + this.nameValid = false; + } + + if (this.permissions.length === 0) { + this.permissionsValid = false; + } + + if (this.baseRoleValid && this.nameValid && this.permissionsValid === null) { + return true; + } + + return false; + }, + cancel() { + this.$emit('cancel'); + }, + async createMemberRole() { + this.alert?.dismiss(); + + if (!this.areFieldsValid()) { + return; + } + + const url = Api.buildUrl(Api.memberRolesPath.replace(':id', this.groupId)); + const data = { + base_access_level: this.baseRole, + name: this.name, + description: this.description, + }; + this.permissions.forEach((permission) => { + data[permission] = 1; + }); + + try { + await axios.post(url, data); + this.$emit('success'); + } catch (error) { + this.alert = createAlert({ + message: error?.response?.data?.message || I18N_CREATION_ERROR, + variant: VARIANT_DANGER, + }); + } + }, + }, + baseRoleOptions: Object.keys(PERMISSIONS).map((accessLevel) => ({ + text: ACCESS_LEVEL_LABELS[accessLevel], + value: accessLevel, + })), + i18n: { + baseRole: { + label: I18N_NEW_ROLE_BASE_ROLE_LABEL, + description: I18N_NEW_ROLE_BASE_ROLE_DESCRIPTION, + }, + fieldFormError: I18N_FIELD_FORM_ERROR, + cancel: I18N_CANCEL, + createRole: I18N_CREATE_ROLE, + description: { + label: I18N_NEW_ROLE_DESCRIPTION_LABEL, + }, + name: { + label: I18N_NEW_ROLE_NAME_LABEL, + placeholder: I18N_NEW_ROLE_NAME_PLACEHOLDER, + description: I18N_NEW_ROLE_NAME_DESCRIPTION, + }, + }, +}; +</script> + +<template> + <gl-form> + <h4 class="gl-mt-0">{{ $options.i18n.createRole }}</h4> + <div class="row"> + <gl-form-group + class="col-md-4" + :label="$options.i18n.baseRole.label" + :description="$options.i18n.baseRole.description" + :invalid-feedback="$options.i18n.fieldFormError" + > + <gl-form-select + v-model="baseRole" + :options="$options.baseRoleOptions" + required + :state="baseRoleValid" + /> + </gl-form-group> + + <gl-form-group + class="col-md-4" + :label="$options.i18n.name.label" + :description="$options.i18n.name.description" + :invalid-feedback="$options.i18n.fieldFormError" + > + <gl-form-input + v-model="name" + :placeholder="$options.i18n.name.placeholder" + :state="nameValid" + /> + </gl-form-group> + + <gl-form-group class="col-lg-8" :label="$options.i18n.description.label"> + <gl-form-textarea v-model="description" /> + </gl-form-group> + </div> + + <gl-form-group label="Permissions"> + <gl-form-checkbox-group v-model="permissions" :state="permissionsValid"> + <gl-form-checkbox + v-for="permission in availablePermissions" + :key="permission.value" + :value="permission.value" + > + {{ permission.text }} + <template v-if="permission.help" #help> + {{ permission.help }} + </template> + </gl-form-checkbox> + </gl-form-checkbox-group> + </gl-form-group> + + <div class="gl-display-flex gl-flex-wrap gl-gap-3"> + <gl-button data-testid="submit" variant="confirm" @click="createMemberRole">{{ + $options.i18n.createRole + }}</gl-button> + <gl-button data-testid="cancel" @click="cancel">{{ $options.i18n.cancel }}</gl-button> + </div> + </gl-form> +</template> diff --git a/ee/app/assets/javascripts/roles_and_permissions/components/list_member_roles.vue b/ee/app/assets/javascripts/roles_and_permissions/components/list_member_roles.vue new file mode 100644 index 00000000000000..a226f717e76c90 --- /dev/null +++ b/ee/app/assets/javascripts/roles_and_permissions/components/list_member_roles.vue @@ -0,0 +1,242 @@ +<script> +import { GlBadge, GlButton, GlCard, GlEmptyState, GlModal, GlTable, GlToast } from '@gitlab/ui'; +import Vue from 'vue'; +import { ACCESS_LEVEL_LABELS } from '~/access_level/constants'; +import { createAlert, VARIANT_DANGER } from '~/alert'; +import Api from '~/api'; +import axios from '~/lib/utils/axios_utils'; +import { + FIELDS, + I18N_ADD_NEW_ROLE, + I18N_CANCEL, + I18N_CARD_TITLE, + I18N_CREATION_SUCCESS, + I18N_DELETE_ROLE, + I18N_DELETION_ERROR, + I18N_DELETION_SUCCESS, + I18N_EMPTY_TITLE, + I18N_FETCH_ERROR, + I18N_LICENSE_ERROR, + I18N_MODAL_TITLE, + I18N_MODAL_WARNING, + PERMISSIONS, +} from '../constants'; +import CreateMemberRole from './create_member_role.vue'; + +Vue.use(GlToast); + +export default { + name: 'ListMemberRoles', + components: { + CreateMemberRole, + GlBadge, + GlButton, + GlCard, + GlEmptyState, + GlModal, + GlTable, + }, + props: { + emptyText: { + type: String, + required: false, + default: null, + }, + groupId: { + type: String, + required: false, + default: null, + }, + }, + data() { + return { + alert: null, + loading: false, + memberRoles: [], + memberRoleToDelete: null, + showConfirmModal: false, + showCreateMemberForm: false, + }; + }, + watch: { + groupId: function newFetch(newGroupId) { + this.fetchMemberRoles(newGroupId); + }, + }, + created() { + this.fetchMemberRoles(this.groupId); + }, + methods: { + async deleteMemberRole() { + this.alert?.dismiss(); + + const url = Api.buildUrl( + Api.deleteMemberRolesPath + .replace(':id', this.groupId) + .replace(':member_role_id', this.memberRoleToDelete), + ); + + try { + await axios.delete(url); + this.$toast.show(I18N_DELETION_SUCCESS); + this.fetchMemberRoles(this.groupId); + } catch (error) { + this.alert = createAlert({ + message: error.response?.data?.message || I18N_DELETION_ERROR, + variant: VARIANT_DANGER, + }); + } finally { + this.memberRoleToDelete = null; + } + }, + async fetchMemberRoles(groupId) { + this.alert?.dismiss(); + + if (!groupId) { + this.memberRoles = []; + return; + } + + const url = Api.buildUrl(Api.memberRolesPath.replace(':id', groupId)); + this.loading = true; + + try { + const { data } = await axios.get(url); + this.memberRoles = data; + } catch (error) { + this.memberRoles = []; + if (error.response.status === 404) { + this.alert = createAlert({ + message: I18N_LICENSE_ERROR, + variant: VARIANT_DANGER, + }); + } else { + this.alert = createAlert({ + message: error?.response?.data?.message || I18N_FETCH_ERROR, + variant: VARIANT_DANGER, + }); + } + } finally { + this.loading = false; + } + }, + listPermissions(item) { + return Object.entries(item) + .filter(([, value]) => value === true) + .map(([key]) => PERMISSIONS[item.base_access_level]?.[key]?.text || key); + }, + nameAccessLevel(value) { + return ACCESS_LEVEL_LABELS[value]; + }, + onCreatedMemberRole() { + this.$toast.show(I18N_CREATION_SUCCESS); + this.showCreateMemberForm = false; + this.fetchMemberRoles(this.groupId); + }, + showConfirm(memberRoleId) { + this.showConfirmModal = true; + this.memberRoleToDelete = memberRoleId; + }, + }, + FIELDS, + i18n: { + addNewRole: I18N_ADD_NEW_ROLE, + cardTitle: I18N_CARD_TITLE, + deleteRole: I18N_DELETE_ROLE, + emptyTitle: I18N_EMPTY_TITLE, + }, + modal: { + actionPrimary: { + text: I18N_DELETE_ROLE, + attributes: { + variant: 'danger', + }, + }, + actionSecondary: { + text: I18N_CANCEL, + attributes: { + variant: 'default', + }, + }, + id: 'confirm-delete-role', + title: I18N_MODAL_TITLE, + warning: I18N_MODAL_WARNING, + }, +}; +</script> + +<template> + <gl-card header-class="gl-new-card-header" body-class="gl-new-card-body gl-px-0 gl-bg-gray-10"> + <template #header> + <div class="gl-new-card-title-wrapper"> + <h3 class="gl-new-card-title"> + {{ $options.i18n.cardTitle }} + <span class="gl-new-card-count">{{ memberRoles.length }}</span> + </h3> + </div> + <div class="gl-new-card-actions"> + <gl-button :disabled="!groupId" size="small" @click="showCreateMemberForm = true">{{ + $options.i18n.addNewRole + }}</gl-button> + </div> + </template> + + <div v-if="showCreateMemberForm" class="gl-new-card-add-form gl-m-3"> + <create-member-role + :group-id="groupId" + @cancel="showCreateMemberForm = false" + @success="onCreatedMemberRole" + /> + </div> + + <gl-table + :fields="$options.FIELDS" + :items="memberRoles" + :busy="loading" + show-empty + stacked="sm" + > + <template #empty> + <gl-empty-state :title="$options.i18n.emptyTitle" :description="emptyText" /> + </template> + + <template #cell(base_access_level)="{ item: { base_access_level } }"> + <gl-badge class="gl-my-n4">{{ nameAccessLevel(base_access_level) }}</gl-badge> + </template> + + <template #cell(permissions)="{ item }"> + <div class="gl-display-flex gl-flex-wrap gl-gap-3 justify-end-on-sm"> + <gl-badge + v-for="(permission, index) in listPermissions(item)" + :key="index" + variant="success" + size="sm" + >{{ permission }}</gl-badge + > + </div> + </template> + + <template #cell(actions)="{ item: { id } }"> + <gl-button + class="gl-my-n4" + category="tertiary" + :aria-label="$options.i18n.deleteRole" + icon="remove" + @click="showConfirm(id)" + /> + </template> + </gl-table> + + <gl-modal + v-model="showConfirmModal" + :modal-id="$options.modal.id" + size="sm" + :title="$options.modal.title" + :action-primary="$options.modal.actionPrimary" + :action-secondary="$options.modal.actionSecondary" + @primary="deleteMemberRole" + > + <p>{{ $options.modal.warning }}</p> + </gl-modal> + </gl-card> +</template> diff --git a/ee/app/assets/javascripts/roles_and_permissions/components/roles_and_permissions_self_managed.vue b/ee/app/assets/javascripts/roles_and_permissions/components/roles_and_permissions_self_managed.vue new file mode 100644 index 00000000000000..0e3f6f5f3bfa80 --- /dev/null +++ b/ee/app/assets/javascripts/roles_and_permissions/components/roles_and_permissions_self_managed.vue @@ -0,0 +1,36 @@ +<script> +import GroupSelect from '~/vue_shared/components/entity_select/group_select.vue'; +import { I18N_EMPTY_TEXT_SELF_MANAGED } from '../constants'; +import ListMemberRoles from './list_member_roles.vue'; + +export default { + name: 'RolesAndPermissionsSelfManaged', + components: { + GroupSelect, + ListMemberRoles, + }, + data() { + return { groupId: null }; + }, + methods: { + updateGroup({ value }) { + this.groupId = value; + }, + }, + i18n: { + emptyText: I18N_EMPTY_TEXT_SELF_MANAGED, + }, +}; +</script> + +<template> + <div class="col"> + <group-select + input-id="group-selector" + input-name="group-selector" + groups-filter="top_level_only" + @input="updateGroup" + /> + <list-member-roles :group-id="groupId" :empty-text="$options.i18n.emptyText" /> + </div> +</template> diff --git a/ee/app/assets/javascripts/roles_and_permissions/constants.js b/ee/app/assets/javascripts/roles_and_permissions/constants.js new file mode 100644 index 00000000000000..d0165b6843793f --- /dev/null +++ b/ee/app/assets/javascripts/roles_and_permissions/constants.js @@ -0,0 +1,88 @@ +import { __, s__ } from '~/locale'; +import { ACCESS_LEVEL_GUEST_INTEGER } from '~/access_level/constants'; + +// GUEST permissions +export const READ_CODE = 'read_code'; +export const READ_VULNERABILITY = 'read_vulnerability'; +export const ADMIN_VULNERABILITY = 'admin_vulnerability'; + +export const GUEST_PERMISSIONS = Object.freeze({ + [READ_CODE]: { + help: s__('MemberRoles|Allows read only access to the source code.'), + text: s__('MemberRoles|Read code'), + value: READ_CODE, + }, + [READ_VULNERABILITY]: { + help: s__('MemberRoles|Allows read only access to the vulnerability reports.'), + text: s__('MemberRoles|Read vulnerability'), + value: READ_VULNERABILITY, + }, + [ADMIN_VULNERABILITY]: { + help: s__( + "MemberRoles|Allows admin access to the vulnerability reports. 'Read vulnerability' must be selected in order to take effect.", + ), + text: s__('MemberRoles|Admin vulnerability'), + value: ADMIN_VULNERABILITY, + }, +}); + +export const PERMISSIONS = Object.freeze({ + [ACCESS_LEVEL_GUEST_INTEGER]: GUEST_PERMISSIONS, +}); + +export const FIELDS = [ + { + key: 'name', + label: s__('MemberRoles|Name'), + sortable: true, + }, + { + key: 'id', + label: s__('MemberRoles|ID'), + sortable: true, + }, + { + key: 'base_access_level', + label: s__('MemberRoles|Base role'), + sortable: true, + }, + { + key: 'permissions', + label: s__('MemberRoles|Permissions'), + sortable: false, + }, + { + key: 'actions', + label: s__('MemberRoles|Actions'), + sortable: false, + }, +]; + +// Translations +export const I18N_ADD_NEW_ROLE = s__('MemberRoles|Add new role'); +export const I18N_CANCEL = __('Cancel'); +export const I18N_CARD_TITLE = s__('MemberRoles|Custom roles'); +export const I18N_CREATE_ROLE = s__('MemberRoles|Create new role'); +export const I18N_CREATION_ERROR = s__('MemberRoles|Fail to create role.'); +export const I18N_CREATION_SUCCESS = s__('MemberRoles|Role successfully created.'); +export const I18N_DELETE_ROLE = s__('MemberRoles|Delete role'); +export const I18N_DELETION_ERROR = s__('MemberRoles|Fail to delete the role.'); +export const I18N_DELETION_SUCCESS = s__('MemberRoles|Role successfully deleted.'); +export const I18N_EMPTY_TITLE = s__('MemberRoles|No custom roles for this group'); +export const I18N_EMPTY_TEXT_SELF_MANAGED = s__( + "MemberRoles|To add a new role select a group and then 'Add new role'.", +); +export const I18N_FETCH_ERROR = s__('MemberRoles|Fail to fetch roles.'); +export const I18N_FIELD_FORM_ERROR = __('This field is required.'); +export const I18N_LICENSE_ERROR = s__('MemberRoles|Make sure the group has an Ultimate license.'); +export const I18N_MODAL_TITLE = s__('MemberRoles|Are you sure you want to delete this role?'); +export const I18N_MODAL_WARNING = s__(`MemberRoles|Removing a custom role also removes all members with this custom role from the group. +If you decide to delete a custom role, you must re-add these users to the group.`); +export const I18N_NEW_ROLE_BASE_ROLE_LABEL = s__('MemberRoles|Base role to use as template'); +export const I18N_NEW_ROLE_BASE_ROLE_DESCRIPTION = s__( + 'MemberRoles|Select a standard role to add permissions.', +); +export const I18N_NEW_ROLE_DESCRIPTION_LABEL = s__('MemberRoles|Description'); +export const I18N_NEW_ROLE_NAME_DESCRIPTION = s__('MemberRoles|Enter a short name.'); +export const I18N_NEW_ROLE_NAME_LABEL = s__('MemberRoles|Role name'); +export const I18N_NEW_ROLE_NAME_PLACEHOLDER = s__('MemberRoles|Incident manager'); diff --git a/ee/app/assets/javascripts/roles_and_permissions/index.js b/ee/app/assets/javascripts/roles_and_permissions/index.js new file mode 100644 index 00000000000000..a7e65abf414e5d --- /dev/null +++ b/ee/app/assets/javascripts/roles_and_permissions/index.js @@ -0,0 +1,18 @@ +import Vue from 'vue'; +import RolesAndPermissionsSelfManaged from './components/roles_and_permissions_self_managed.vue'; + +export const initRolesAndPermissionsSelfManagedApp = () => { + const el = document.querySelector('#js-roles-and-permissions-self-managed'); + + if (!el) { + return null; + } + + return new Vue({ + el, + name: 'RolesAndPermissionsSelfManagedRoot', + render(h) { + return h(RolesAndPermissionsSelfManaged); + }, + }); +}; diff --git a/ee/app/assets/stylesheets/page_bundles/member_roles.scss b/ee/app/assets/stylesheets/page_bundles/member_roles.scss new file mode 100644 index 00000000000000..2fc79768b85a91 --- /dev/null +++ b/ee/app/assets/stylesheets/page_bundles/member_roles.scss @@ -0,0 +1,7 @@ +@import 'page_bundles/mixins_and_variables_and_functions'; + +.justify-end-on-sm { + @include gl-media-breakpoint-down(sm) { + @include gl-justify-content-end; + } +} diff --git a/ee/app/views/admin/application_settings/roles_and_permissions/index.html.haml b/ee/app/views/admin/application_settings/roles_and_permissions/index.html.haml index a6856acb62a9bb..d7fcf76da43113 100644 --- a/ee/app/views/admin/application_settings/roles_and_permissions/index.html.haml +++ b/ee/app/views/admin/application_settings/roles_and_permissions/index.html.haml @@ -1,3 +1,5 @@ - page_title _('Roles and Permissions') +- add_page_specific_style 'page_bundles/member_roles' -= gl_loading_icon(css_class: 'gl-my-5', size: 'md') +#js-roles-and-permissions-self-managed + = gl_loading_icon(css_class: 'gl-my-5', size: 'md') diff --git a/ee/config/feature_flags/development/custom_roles_ui_self_managed.yml b/ee/config/feature_flags/development/custom_roles_ui_self_managed.yml index eb514eea292c83..78fc1fa619759b 100644 --- a/ee/config/feature_flags/development/custom_roles_ui_self_managed.yml +++ b/ee/config/feature_flags/development/custom_roles_ui_self_managed.yml @@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/416644 milestone: '16.2' type: development group: group::authentication and authorization -default_enabled: false +default_enabled: true diff --git a/ee/spec/frontend/roles_and_permissions/components/create_member_role_spec.js b/ee/spec/frontend/roles_and_permissions/components/create_member_role_spec.js new file mode 100644 index 00000000000000..4309ad32fee769 --- /dev/null +++ b/ee/spec/frontend/roles_and_permissions/components/create_member_role_spec.js @@ -0,0 +1,149 @@ +import { GlFormInput, GlFormSelect, GlFormTextarea, GlFormCheckbox } from '@gitlab/ui'; +import MockAdapter from 'axios-mock-adapter'; +import { nextTick } from 'vue'; +import { createAlert, VARIANT_DANGER } from '~/alert'; +import axios from '~/lib/utils/axios_utils'; +import { HTTP_STATUS_INTERNAL_SERVER_ERROR, HTTP_STATUS_OK } from '~/lib/utils/http_status'; +import { __ } from '~/locale'; +import CreateMemberRole from 'ee/roles_and_permissions/components/create_member_role.vue'; +import { I18N_CREATION_ERROR } from 'ee/roles_and_permissions/constants'; +import { mountExtended } from 'helpers/vue_test_utils_helper'; +import waitForPromises from 'helpers/wait_for_promises'; + +const mockAlertDismiss = jest.fn(); +jest.mock('~/alert', () => ({ + createAlert: jest.fn().mockImplementation(() => ({ + dismiss: mockAlertDismiss, + })), +})); + +describe('CreateMemberRole', () => { + let mockAxios; + let wrapper; + + const createComponent = () => { + wrapper = mountExtended(CreateMemberRole, { + propsData: { groupId: '4' }, + stubs: { GlFormSelect }, + }); + }; + + const findButtonSubmit = () => wrapper.findByTestId('submit'); + const findButtonCancel = () => wrapper.findByTestId('cancel'); + const findNameField = () => wrapper.findComponent(GlFormInput); + const findCheckboxes = () => wrapper.findAllComponents(GlFormCheckbox); + const findSelect = () => wrapper.findComponent(GlFormSelect); + const findOptions = () => findSelect().findAll('option'); + const findTextArea = () => wrapper.findComponent(GlFormTextarea); + + const fillForm = () => { + findNameField().setValue('My role name'); + findTextArea().setValue('My description'); + findCheckboxes().at(0).find('input').setChecked(); + }; + + beforeEach(() => { + window.gon = { api_version: 'v4' }; + mockAxios = new MockAdapter(axios); + createComponent(); + }); + + it('has only one select options', () => { + const options = findOptions(); + expect(options).toHaveLength(1); + expect(options.at(0).attributes()).toMatchObject({ value: '10' }); + expect(options.at(0).text()).toBe(__('Guest')); + }); + + it('has multiple checkbox permissions', () => { + const checkboxes = findCheckboxes(); + expect(checkboxes.length).toBeGreaterThan(1); + expect(checkboxes.at(0).find('p[class=help-text]').exists()).toBe(true); + }); + + it('emits cancel event', () => { + expect(wrapper.emitted('cancel')).toBeUndefined(); + + findButtonCancel().trigger('click'); + + expect(wrapper.emitted('cancel')).toHaveLength(1); + }); + + describe('field validation', () => { + it('shows warnings in the name field', async () => { + expect(findNameField().classes()).toContain('is-valid'); + + findButtonSubmit().trigger('click'); + await nextTick(); + + expect(findNameField().classes()).toContain('is-invalid'); + }); + + it('shows warnings if permissions are unchecked', async () => { + expect(findCheckboxes().at(0).find('input').classes()).not.toContain('is-invalid'); + + findButtonSubmit().trigger('click'); + await nextTick(); + + expect(findCheckboxes().at(0).find('input').classes()).toContain('is-invalid'); + }); + }); + + describe('when successful submission', () => { + beforeEach(() => { + fillForm(); + + mockAxios.onPost('/api/v4/groups/4/member_roles').replyOnce(HTTP_STATUS_OK); + }); + + it('sends the correct data', async () => { + findButtonSubmit().trigger('click'); + await waitForPromises(); + + expect(mockAxios.history.post[0].data).toBe( + '{"base_access_level":"10","name":"My role name","description":"My description","read_code":1}', + ); + }); + + it('emits success event', async () => { + expect(wrapper.emitted('success')).toBeUndefined(); + + findButtonSubmit().trigger('click'); + await waitForPromises(); + + expect(wrapper.emitted('success')).toHaveLength(1); + }); + }); + + describe('when unsuccessful submission', () => { + beforeEach(() => { + fillForm(); + + mockAxios + .onPost('/api/v4/groups/4/member_roles') + .replyOnce(HTTP_STATUS_INTERNAL_SERVER_ERROR); + // .replyOnce(HTTP_STATUS_OK); + }); + + it('shows alert', async () => { + findButtonSubmit().trigger('click'); + await waitForPromises(); + + expect(createAlert).toHaveBeenCalledWith({ + message: I18N_CREATION_ERROR, + variant: VARIANT_DANGER, + }); + }); + + it('dismisses previous alert', async () => { + findButtonSubmit().trigger('click'); + await waitForPromises(); + + expect(mockAlertDismiss).toHaveBeenCalledTimes(0); + + findButtonSubmit().trigger('click'); + + expect(mockAlertDismiss).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/ee/spec/frontend/roles_and_permissions/components/list_member_roles_spec.js b/ee/spec/frontend/roles_and_permissions/components/list_member_roles_spec.js new file mode 100644 index 00000000000000..5836e43c82fcb6 --- /dev/null +++ b/ee/spec/frontend/roles_and_permissions/components/list_member_roles_spec.js @@ -0,0 +1,334 @@ +import { GlBadge, GlButton, GlCard, GlEmptyState, GlModal, GlTable } from '@gitlab/ui'; +import { mount, shallowMount } from '@vue/test-utils'; +import MockAdapter from 'axios-mock-adapter'; +import { nextTick } from 'vue'; +import { createAlert, VARIANT_DANGER } from '~/alert'; +import axios from '~/lib/utils/axios_utils'; +import { + HTTP_STATUS_INTERNAL_SERVER_ERROR, + HTTP_STATUS_NOT_FOUND, + HTTP_STATUS_NO_CONTENT, + HTTP_STATUS_OK, +} from '~/lib/utils/http_status'; +import { __, s__ } from '~/locale'; +import CreateMemberRole from 'ee/roles_and_permissions/components/create_member_role.vue'; +import { + I18N_CREATION_SUCCESS, + I18N_DELETION_ERROR, + I18N_DELETION_SUCCESS, + I18N_FETCH_ERROR, + I18N_LICENSE_ERROR, +} from 'ee/roles_and_permissions/constants'; +import ListMemberRoles from 'ee/roles_and_permissions/components/list_member_roles.vue'; +import waitForPromises from 'helpers/wait_for_promises'; + +const mockAlertDismiss = jest.fn(); +jest.mock('~/alert', () => ({ + createAlert: jest.fn().mockImplementation(() => ({ + dismiss: mockAlertDismiss, + })), +})); + +describe('RolesAndPermissionsSelfManaged', () => { + const emptyText = 'blah, blah'; + const groupId = '49'; + let mockAxios; + const mockResponse = [ + { + id: 1, + name: 'My custom Guest', + base_access_level: 10, + read_code: true, + read_vulnerability: true, + admin_vulnerability: true, + }, + { + id: 2, + name: 'My name Developer', + base_access_level: 30, + non_standard_permission: true, + ignored: false, + }, + ]; + const mockToastShow = jest.fn(); + let wrapper; + + const createComponent = (props = {}, mountFn = shallowMount) => { + wrapper = mountFn(ListMemberRoles, { + propsData: { emptyText, ...props }, + stubs: { GlCard }, + mocks: { + $toast: { + show: mockToastShow, + }, + }, + }); + }; + + const findAddRoleButton = () => wrapper.find('.gl-new-card-actions').findComponent(GlButton); + const findCounter = () => wrapper.find('.gl-new-card-count'); + const findCreateMemberRole = () => wrapper.findComponent(CreateMemberRole); + const findEmptyState = () => wrapper.findComponent(GlEmptyState); + const findModal = () => wrapper.findComponent(GlModal); + const findTable = () => wrapper.findComponent(GlTable); + const findTableHeader = (idx) => findTable().findAll('th').at(idx); + const findCell = (trIdx, tdIdx) => { + return findTable().find('tbody').findAll('tr').at(trIdx).findAll('td').at(tdIdx).find('div'); + }; + + beforeEach(() => { + window.gon = { api_version: 'v4' }; + mockAxios = new MockAdapter(axios); + mockAxios.onGet('/api/v4/groups/49/member_roles').replyOnce(HTTP_STATUS_OK, mockResponse); + }); + + it('shows empty state', () => { + createComponent({ groupId: null }); + expect(wrapper.find('.gl-new-card-title').text()).toMatch(ListMemberRoles.i18n.cardTitle); + expect(findCounter().text()).toBe('0'); + expect(findAddRoleButton().props('disabled')).toBe(true); + expect(findEmptyState().props()).toMatchObject({ + description: emptyText, + title: ListMemberRoles.i18n.emptyTitle, + }); + expect(findCreateMemberRole().exists()).toBe(false); + }); + + describe('fetching roles', () => { + it('toggles the table busy state', async () => { + createComponent({ groupId }); + expect(findTable().attributes('busy')).toBe('true'); + await waitForPromises(); + + expect(findTable().attributes('busy')).toBeUndefined(); + }); + + it('upon changes in the groupId', async () => { + createComponent({ groupId: null }); + await waitForPromises(); + expect(mockAxios.history.get).toHaveLength(0); + + wrapper.vm.$options.watch.groupId.call(wrapper.vm, groupId); + await waitForPromises(); + expect(mockAxios.history.get).toHaveLength(1); + }); + + it('shows license alert for 404 responses', async () => { + createComponent({ groupId: '100' }); + mockAxios.onGet('/api/v4/groups/100/member_roles').replyOnce(HTTP_STATUS_NOT_FOUND); + await waitForPromises(); + + expect(createAlert).toHaveBeenCalledWith({ + message: I18N_LICENSE_ERROR, + variant: VARIANT_DANGER, + }); + }); + + it('shows generic alert for non-404 responses', async () => { + createComponent({ groupId: '100' }); + mockAxios + .onGet('/api/v4/groups/100/member_roles') + .replyOnce(HTTP_STATUS_INTERNAL_SERVER_ERROR); + await waitForPromises(); + + expect(createAlert).toHaveBeenCalledWith({ + message: I18N_FETCH_ERROR, + variant: VARIANT_DANGER, + }); + }); + + it('dismisses previous alerts', async () => { + createComponent({ groupId: '100' }); + mockAxios + .onGet('/api/v4/groups/100/member_roles') + .replyOnce(HTTP_STATUS_INTERNAL_SERVER_ERROR); + await waitForPromises(); + expect(mockAlertDismiss).toHaveBeenCalledTimes(0); + + wrapper.vm.$options.watch.groupId.call(wrapper.vm, groupId); + await waitForPromises(); + expect(mockAlertDismiss).toHaveBeenCalledTimes(1); + }); + }); + + describe('create role form', () => { + beforeEach(async () => { + // Show form + createComponent({ groupId }); + findAddRoleButton().vm.$emit('click'); + await waitForPromises(); + }); + + it('toggles display', async () => { + expect(findCreateMemberRole().exists()).toBe(true); + + findCreateMemberRole().vm.$emit('cancel'); + await nextTick(); + + expect(findCreateMemberRole().exists()).toBe(false); + }); + + describe('when successfully creates a new role', () => { + it('shows toast', () => { + findCreateMemberRole().vm.$emit('success'); + + expect(mockToastShow).toHaveBeenCalledWith(I18N_CREATION_SUCCESS); + }); + + it('fetches roles', async () => { + expect(mockAxios.history.get).toHaveLength(1); + + findCreateMemberRole().vm.$emit('success'); + await waitForPromises(); + + expect(mockAxios.history.get).toHaveLength(2); + }); + }); + }); + + describe('table of roles', () => { + it('shows name and id', async () => { + createComponent({ groupId }, mount); + await waitForPromises(); + + expect(findCell(0, 0).text()).toBe(mockResponse[0].name); + expect(findCell(0, 1).text()).toBe(`${mockResponse[0].id}`); + }); + + it('sorts columns by name', async () => { + createComponent({ groupId }, mount); + await waitForPromises(); + expect(findCell(0, 0).text()).toBe(mockResponse[0].name); + expect(findCell(1, 0).text()).toBe(mockResponse[1].name); + + findTableHeader(0).trigger('click'); + findTableHeader(0).trigger('click'); + await nextTick(); + + expect(findCell(0, 0).text()).toBe(mockResponse[1].name); + expect(findCell(1, 0).text()).toBe(mockResponse[0].name); + }); + + it('sorts columns by ID', async () => { + createComponent({ groupId }, mount); + await waitForPromises(); + expect(findCell(0, 1).text()).toBe(`${mockResponse[0].id}`); + expect(findCell(1, 1).text()).toBe(`${mockResponse[1].id}`); + + findTableHeader(1).trigger('click'); + findTableHeader(1).trigger('click'); + await nextTick(); + + expect(findCell(0, 1).text()).toBe(`${mockResponse[1].id}`); + expect(findCell(1, 1).text()).toBe(`${mockResponse[0].id}`); + }); + + it('sorts columns by base role', async () => { + createComponent({ groupId }, mount); + await waitForPromises(); + expect(findCell(0, 2).findComponent(GlBadge).text()).toBe(__('Guest')); + expect(findCell(1, 2).findComponent(GlBadge).text()).toBe(__('Developer')); + + findTableHeader(2).trigger('click'); + findTableHeader(2).trigger('click'); + await nextTick(); + + expect(findCell(0, 2).findComponent(GlBadge).text()).toBe(__('Developer')); + expect(findCell(1, 2).findComponent(GlBadge).text()).toBe(__('Guest')); + }); + + it('shows list of standard permissions', async () => { + createComponent({ groupId }, mount); + await waitForPromises(); + + expect(findCell(0, 3).findAllComponents(GlBadge).at(0).text()).toBe( + s__('MemberRoles|Read code'), + ); + expect(findCell(0, 3).findAllComponents(GlBadge).at(1).text()).toBe( + s__('MemberRoles|Read vulnerability'), + ); + expect(findCell(0, 3).findAllComponents(GlBadge).at(2).text()).toBe( + s__('MemberRoles|Admin vulnerability'), + ); + }); + + it('shows list of non-standard permissions', async () => { + createComponent({ groupId }, mount); + await waitForPromises(); + + const badges = findCell(1, 3).findAllComponents(GlBadge); + expect(badges).toHaveLength(1); + expect(badges.at(0).text()).toBe('non_standard_permission'); + }); + }); + + describe('delete role', () => { + const clickRoleDelete = () => { + findCell(0, 4).findComponent(GlButton).trigger('click'); + return nextTick(); + }; + + beforeEach(async () => { + createComponent({ groupId }, mount); + await waitForPromises(); + }); + + it('shows confirm modal', async () => { + expect(findModal().props('visible')).toBe(false); + + await clickRoleDelete(); + expect(findModal().props('visible')).toBe(true); + }); + + describe('when successful deletion', () => { + beforeEach(async () => { + mockAxios.onDelete('/api/v4/groups/49/member_roles/1').replyOnce(HTTP_STATUS_NO_CONTENT); + await clickRoleDelete(); + }); + + it('delete the role', async () => { + expect(mockAxios.history.delete).toHaveLength(0); + + findModal().vm.$emit('primary'); + await waitForPromises(); + + expect(mockAxios.history.delete).toHaveLength(1); + }); + + it('shows toast', async () => { + findModal().vm.$emit('primary'); + await waitForPromises(); + + expect(mockToastShow).toHaveBeenCalledWith(I18N_DELETION_SUCCESS); + }); + + it('fetches roles', async () => { + expect(mockAxios.history.get).toHaveLength(1); + + findModal().vm.$emit('primary'); + await waitForPromises(); + + expect(mockAxios.history.get).toHaveLength(2); + }); + }); + + describe('when unsuccessful fetch of roles', () => { + beforeEach(async () => { + mockAxios + .onDelete('/api/v4/groups/49/member_roles/1') + .replyOnce(HTTP_STATUS_INTERNAL_SERVER_ERROR); + await clickRoleDelete(); + }); + + it('shows alert', async () => { + findModal().vm.$emit('primary'); + await waitForPromises(); + + expect(createAlert).toHaveBeenCalledWith({ + message: I18N_DELETION_ERROR, + variant: VARIANT_DANGER, + }); + }); + }); + }); +}); diff --git a/ee/spec/frontend/roles_and_permissions/components/roles_and_permissions_self_managed_spec.js b/ee/spec/frontend/roles_and_permissions/components/roles_and_permissions_self_managed_spec.js new file mode 100644 index 00000000000000..dd58bba60cc9da --- /dev/null +++ b/ee/spec/frontend/roles_and_permissions/components/roles_and_permissions_self_managed_spec.js @@ -0,0 +1,43 @@ +import { shallowMount } from '@vue/test-utils'; +import { nextTick } from 'vue'; +import GroupSelect from '~/vue_shared/components/entity_select/group_select.vue'; +import ListMemberRoles from 'ee/roles_and_permissions/components/list_member_roles.vue'; +import RolesAndPermissionsSelfManaged from 'ee/roles_and_permissions/components/roles_and_permissions_self_managed.vue'; + +describe('RolesAndPermissionsSelfManaged', () => { + let wrapper; + const createComponent = () => { + wrapper = shallowMount(RolesAndPermissionsSelfManaged); + }; + + const findGroupSelect = () => wrapper.findComponent(GroupSelect); + const findListMemberRoles = () => wrapper.findComponent(ListMemberRoles); + + beforeEach(() => { + createComponent(); + }); + + it('has a GroupSelect component', () => { + expect(findGroupSelect().exists()).toBe(true); + }); + + it('has a `ListMemberRoles` component', () => { + expect(findListMemberRoles().exists()).toBe(true); + }); + + it('correctly sets props to `ListMemberRoles`', async () => { + expect(findListMemberRoles().props()).toMatchObject({ + emptyText: RolesAndPermissionsSelfManaged.i18n.emptyText, + groupId: null, + }); + + const newGroupId = '36'; + findGroupSelect().vm.$emit('input', { value: newGroupId }); + await nextTick(); + + expect(findListMemberRoles().props()).toMatchObject({ + emptyText: RolesAndPermissionsSelfManaged.i18n.emptyText, + groupId: newGroupId, + }); + }); +}); diff --git a/locale/gitlab.pot b/locale/gitlab.pot index fe76559a34ca63..258cce5809603e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -28833,6 +28833,99 @@ msgstr "" msgid "MemberInviteEmail|Invitation to join the %{project_or_group} %{project_or_group_name}" msgstr "" +msgid "MemberRoles|Actions" +msgstr "" + +msgid "MemberRoles|Add new role" +msgstr "" + +msgid "MemberRoles|Admin vulnerability" +msgstr "" + +msgid "MemberRoles|Allows admin access to the vulnerability reports. 'Read vulnerability' must be selected in order to take effect." +msgstr "" + +msgid "MemberRoles|Allows read only access to the source code." +msgstr "" + +msgid "MemberRoles|Allows read only access to the vulnerability reports." +msgstr "" + +msgid "MemberRoles|Are you sure you want to delete this role?" +msgstr "" + +msgid "MemberRoles|Base role" +msgstr "" + +msgid "MemberRoles|Base role to use as template" +msgstr "" + +msgid "MemberRoles|Create new role" +msgstr "" + +msgid "MemberRoles|Custom roles" +msgstr "" + +msgid "MemberRoles|Delete role" +msgstr "" + +msgid "MemberRoles|Description" +msgstr "" + +msgid "MemberRoles|Enter a short name." +msgstr "" + +msgid "MemberRoles|Fail to create role." +msgstr "" + +msgid "MemberRoles|Fail to delete the role." +msgstr "" + +msgid "MemberRoles|Fail to fetch roles." +msgstr "" + +msgid "MemberRoles|ID" +msgstr "" + +msgid "MemberRoles|Incident manager" +msgstr "" + +msgid "MemberRoles|Make sure the group has an Ultimate license." +msgstr "" + +msgid "MemberRoles|Name" +msgstr "" + +msgid "MemberRoles|No custom roles for this group" +msgstr "" + +msgid "MemberRoles|Permissions" +msgstr "" + +msgid "MemberRoles|Read code" +msgstr "" + +msgid "MemberRoles|Read vulnerability" +msgstr "" + +msgid "MemberRoles|Removing a custom role also removes all members with this custom role from the group. If you decide to delete a custom role, you must re-add these users to the group." +msgstr "" + +msgid "MemberRoles|Role name" +msgstr "" + +msgid "MemberRoles|Role successfully created." +msgstr "" + +msgid "MemberRoles|Role successfully deleted." +msgstr "" + +msgid "MemberRoles|Select a standard role to add permissions." +msgstr "" + +msgid "MemberRoles|To add a new role select a group and then 'Add new role'." +msgstr "" + msgid "MemberRole|%{requirement} has to be enabled in order to enable %{permission}." msgstr "" -- GitLab From 2235358242c23129a5897dee71d031d87eadddb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20Sanz=20Garc=C3=ADa?= <esanz-garcia@gitlab.com> Date: Tue, 22 Aug 2023 21:00:39 +0200 Subject: [PATCH 2/4] Apply feedback from frontend review --- app/assets/javascripts/api.js | 2 - .../javascripts/api/member_roles_api.js | 25 ++++ app/assets/javascripts/rest_api.js | 1 + .../components/entity_select/group_select.vue | 12 +- .../components/entity_select/utils.js | 17 +-- .../components/excluded_namespaces_form.vue | 2 +- .../components/create_member_role.vue | 34 ++++-- .../components/list_member_roles.vue | 75 ++++++------ .../roles_and_permissions_self_managed.vue | 3 +- .../roles_and_permissions/constants.js | 13 +- .../roles_and_permissions/index.js | 3 + .../page_bundles/member_roles.scss | 1 + .../custom_roles_ui_self_managed.yml | 2 +- .../components/create_member_role_spec.js | 40 ++++--- .../components/list_member_roles_spec.js | 111 +++++++----------- locale/gitlab.pot | 10 +- .../components/entity_select/utils_spec.js | 12 +- 17 files changed, 196 insertions(+), 167 deletions(-) create mode 100644 app/assets/javascripts/api/member_roles_api.js diff --git a/app/assets/javascripts/api.js b/app/assets/javascripts/api.js index 101e27b1d364b5..185cdaa1c996e2 100644 --- a/app/assets/javascripts/api.js +++ b/app/assets/javascripts/api.js @@ -17,8 +17,6 @@ const Api = { groupPath: '/api/:version/groups/:id', groupMembersPath: '/api/:version/groups/:id/members', groupMilestonesPath: '/api/:version/groups/:id/milestones', - memberRolesPath: '/api/:version/groups/:id/member_roles', - deleteMemberRolesPath: '/api/:version/groups/:id/member_roles/:member_role_id', subgroupsPath: '/api/:version/groups/:id/subgroups', descendantGroupsPath: '/api/:version/groups/:id/descendant_groups', namespacesPath: '/api/:version/namespaces.json', diff --git a/app/assets/javascripts/api/member_roles_api.js b/app/assets/javascripts/api/member_roles_api.js new file mode 100644 index 00000000000000..65b5aedda1fdca --- /dev/null +++ b/app/assets/javascripts/api/member_roles_api.js @@ -0,0 +1,25 @@ +import axios from '../lib/utils/axios_utils'; +import { buildApiUrl } from './api_utils'; + +const MEMBER_ROLES_PATH = '/api/:version/groups/:id/member_roles'; +const DELETE_MEMBER_ROLES_PATH = '/api/:version/groups/:id/member_roles/:member_role_id'; + +export function createMemberRole(groupId, data) { + const url = buildApiUrl(MEMBER_ROLES_PATH.replace(':id', groupId)); + + return axios.post(url, data); +} + +export function getMemberRoles(groupId) { + const url = buildApiUrl(MEMBER_ROLES_PATH.replace(':id', groupId)); + + return axios.get(url); +} + +export function deleteMemberRole(groupId, roleId) { + const url = buildApiUrl( + DELETE_MEMBER_ROLES_PATH.replace(':id', groupId).replace(':member_role_id', roleId), + ); + + return axios.delete(url); +} diff --git a/app/assets/javascripts/rest_api.js b/app/assets/javascripts/rest_api.js index 87996d0bb851d6..0250557e6879f6 100644 --- a/app/assets/javascripts/rest_api.js +++ b/app/assets/javascripts/rest_api.js @@ -8,6 +8,7 @@ export * from './api/tags_api'; export * from './api/alert_management_alerts_api'; export * from './api/harbor_registry'; export * from './api/environments_api'; +export * from './api/member_roles_api'; // Note: It's not possible to spy on methods imported from this file in // Jest tests. diff --git a/app/assets/javascripts/vue_shared/components/entity_select/group_select.vue b/app/assets/javascripts/vue_shared/components/entity_select/group_select.vue index 941963475d053e..eb7b20fa4c12db 100644 --- a/app/assets/javascripts/vue_shared/components/entity_select/group_select.vue +++ b/app/assets/javascripts/vue_shared/components/entity_select/group_select.vue @@ -19,6 +19,11 @@ export default { EntitySelect, }, props: { + apiParams: { + type: Object, + required: false, + default: () => ({}), + }, label: { type: String, required: false, @@ -48,7 +53,7 @@ export default { default: null, }, groupsFilter: { - type: String, + type: String, // Two supported values: `descendant_groups` and `subgroups` See app/assets/javascripts/vue_shared/components/entity_select/utils.js. required: false, default: null, }, @@ -66,10 +71,11 @@ export default { search: searchString, per_page: DEFAULT_PER_PAGE, page, + ...this.apiParams, }; try { - const { url, config } = groupsPath(this.groupsFilter, this.parentGroupID, params); - const { data = [], headers } = await axios.get(url, config); + const url = groupsPath(this.groupsFilter, this.parentGroupID); + const { data = [], headers } = await axios.get(url, { params }); groups = data.map((group) => ({ ...group, text: group.full_name, diff --git a/app/assets/javascripts/vue_shared/components/entity_select/utils.js b/app/assets/javascripts/vue_shared/components/entity_select/utils.js index c94f3d9940983a..857a3ab4c74064 100644 --- a/app/assets/javascripts/vue_shared/components/entity_select/utils.js +++ b/app/assets/javascripts/vue_shared/components/entity_select/utils.js @@ -1,15 +1,14 @@ import Api from '~/api'; /** - * @param {'descendant_groups'|'subgroups'|'top_level_only'|null} groupsFilter - type of group filtering - * @param {string|null} parentGroupID - parent group is needed for 'descendant_groups' and 'subgroups' filtering. - * @param {object} params - optional attributes + * @param {'descendant_groups'|'subgroups'|null} [groupsFilter] - type of group filtering + * @param {string|null} [parentGroupID] - parent group is needed for 'descendant_groups' and 'subgroups' filtering. */ -export const groupsPath = (groupsFilter, parentGroupID, params = {}) => { - if (['descendant_groups', 'subgroups'].includes(groupsFilter) && parentGroupID === null) { +export const groupsPath = (groupsFilter, parentGroupID) => { + if (groupsFilter && !parentGroupID) { throw new Error('Cannot use groupsFilter without a parentGroupID'); } - const config = { params }; + let url = ''; switch (groupsFilter) { case 'descendant_groups': @@ -18,14 +17,10 @@ export const groupsPath = (groupsFilter, parentGroupID, params = {}) => { case 'subgroups': url = Api.subgroupsPath.replace(':id', parentGroupID); break; - case 'top_level_only': - config.params.top_level_only = true; - url = Api.groupsPath; - break; default: url = Api.groupsPath; break; } - return { url: Api.buildUrl(url), config }; + return Api.buildUrl(url); }; diff --git a/ee/app/assets/javascripts/pages/admin/namespace_limits/components/excluded_namespaces_form.vue b/ee/app/assets/javascripts/pages/admin/namespace_limits/components/excluded_namespaces_form.vue index e72c54b9b11b6d..105d9c7c0446cc 100644 --- a/ee/app/assets/javascripts/pages/admin/namespace_limits/components/excluded_namespaces_form.vue +++ b/ee/app/assets/javascripts/pages/admin/namespace_limits/components/excluded_namespaces_form.vue @@ -80,7 +80,7 @@ export default { let groups = []; let totalPages = 0; try { - const { data = [], headers } = await axios.get(Api.buildUrl(groupsPath()), { + const { data = [], headers } = await axios.get(groupsPath(), { params: { search: searchString, per_page: DEFAULT_PER_PAGE, diff --git a/ee/app/assets/javascripts/roles_and_permissions/components/create_member_role.vue b/ee/app/assets/javascripts/roles_and_permissions/components/create_member_role.vue index df5d795a6b7cf7..47e7962315cc06 100644 --- a/ee/app/assets/javascripts/roles_and_permissions/components/create_member_role.vue +++ b/ee/app/assets/javascripts/roles_and_permissions/components/create_member_role.vue @@ -9,10 +9,9 @@ import { GlFormSelect, GlFormTextarea, } from '@gitlab/ui'; -import { ACCESS_LEVEL_LABELS } from '~/access_level/constants'; +import { ACCESS_LEVEL_LABELS, ACCESS_LEVEL_GUEST_INTEGER } from '~/access_level/constants'; import { createAlert, VARIANT_DANGER } from '~/alert'; -import Api from '~/api'; -import axios from '~/lib/utils/axios_utils'; +import { createMemberRole } from '~/rest_api'; import { I18N_CANCEL, I18N_CREATE_ROLE, @@ -24,6 +23,7 @@ import { I18N_NEW_ROLE_NAME_DESCRIPTION, I18N_NEW_ROLE_NAME_LABEL, I18N_NEW_ROLE_NAME_PLACEHOLDER, + I18N_NEW_ROLE_PERMISSIONS_LABEL, PERMISSIONS, } from '../constants'; @@ -49,7 +49,7 @@ export default { return { alert: null, // Remove the default `Guest` role when additional base access roles are supported. - baseRole: '10', + baseRole: `${ACCESS_LEVEL_GUEST_INTEGER}`, baseRoleValid: true, description: '', name: '', @@ -97,7 +97,6 @@ export default { return; } - const url = Api.buildUrl(Api.memberRolesPath.replace(':id', this.groupId)); const data = { base_access_level: this.baseRole, name: this.name, @@ -108,7 +107,7 @@ export default { }); try { - await axios.post(url, data); + await createMemberRole(this.groupId, data); this.$emit('success'); } catch (error) { this.alert = createAlert({ @@ -127,23 +126,26 @@ export default { label: I18N_NEW_ROLE_BASE_ROLE_LABEL, description: I18N_NEW_ROLE_BASE_ROLE_DESCRIPTION, }, - fieldFormError: I18N_FIELD_FORM_ERROR, cancel: I18N_CANCEL, createRole: I18N_CREATE_ROLE, description: { label: I18N_NEW_ROLE_DESCRIPTION_LABEL, }, + fieldFormError: I18N_FIELD_FORM_ERROR, name: { label: I18N_NEW_ROLE_NAME_LABEL, placeholder: I18N_NEW_ROLE_NAME_PLACEHOLDER, description: I18N_NEW_ROLE_NAME_DESCRIPTION, }, + permissions: { + label: I18N_NEW_ROLE_PERMISSIONS_LABEL, + }, }, }; </script> <template> - <gl-form> + <gl-form @submit.prevent="createMemberRole"> <h4 class="gl-mt-0">{{ $options.i18n.createRole }}</h4> <div class="row"> <gl-form-group @@ -151,8 +153,10 @@ export default { :label="$options.i18n.baseRole.label" :description="$options.i18n.baseRole.description" :invalid-feedback="$options.i18n.fieldFormError" + label-for="group-1" > <gl-form-select + id="group-1" v-model="baseRole" :options="$options.baseRoleOptions" required @@ -165,20 +169,22 @@ export default { :label="$options.i18n.name.label" :description="$options.i18n.name.description" :invalid-feedback="$options.i18n.fieldFormError" + label-for="group-2" > <gl-form-input + id="group-2" v-model="name" :placeholder="$options.i18n.name.placeholder" :state="nameValid" /> </gl-form-group> - <gl-form-group class="col-lg-8" :label="$options.i18n.description.label"> - <gl-form-textarea v-model="description" /> + <gl-form-group class="col-lg-8" :label="$options.i18n.description.label" label-for="group-3"> + <gl-form-textarea id="group-3" v-model="description" /> </gl-form-group> </div> - <gl-form-group label="Permissions"> + <gl-form-group :label="$options.i18n.permissions.label"> <gl-form-checkbox-group v-model="permissions" :state="permissionsValid"> <gl-form-checkbox v-for="permission in availablePermissions" @@ -194,10 +200,12 @@ export default { </gl-form-group> <div class="gl-display-flex gl-flex-wrap gl-gap-3"> - <gl-button data-testid="submit" variant="confirm" @click="createMemberRole">{{ + <gl-button type="submit" data-testid="submit-button" variant="confirm" :disabled="false">{{ $options.i18n.createRole }}</gl-button> - <gl-button data-testid="cancel" @click="cancel">{{ $options.i18n.cancel }}</gl-button> + <gl-button type="reset" data-testid="cancel-button" @click="cancel">{{ + $options.i18n.cancel + }}</gl-button> </div> </gl-form> </template> diff --git a/ee/app/assets/javascripts/roles_and_permissions/components/list_member_roles.vue b/ee/app/assets/javascripts/roles_and_permissions/components/list_member_roles.vue index a226f717e76c90..167b7fb3e448e2 100644 --- a/ee/app/assets/javascripts/roles_and_permissions/components/list_member_roles.vue +++ b/ee/app/assets/javascripts/roles_and_permissions/components/list_member_roles.vue @@ -1,10 +1,9 @@ <script> -import { GlBadge, GlButton, GlCard, GlEmptyState, GlModal, GlTable, GlToast } from '@gitlab/ui'; -import Vue from 'vue'; +import { GlBadge, GlButton, GlCard, GlEmptyState, GlModal, GlTable } from '@gitlab/ui'; import { ACCESS_LEVEL_LABELS } from '~/access_level/constants'; import { createAlert, VARIANT_DANGER } from '~/alert'; -import Api from '~/api'; -import axios from '~/lib/utils/axios_utils'; +import { HTTP_STATUS_NOT_FOUND } from '~/lib/utils/http_status'; +import { deleteMemberRole, getMemberRoles } from '~/rest_api'; import { FIELDS, I18N_ADD_NEW_ROLE, @@ -23,8 +22,6 @@ import { } from '../constants'; import CreateMemberRole from './create_member_role.vue'; -Vue.use(GlToast); - export default { name: 'ListMemberRoles', components: { @@ -54,10 +51,14 @@ export default { loading: false, memberRoles: [], memberRoleToDelete: null, - showConfirmModal: false, showCreateMemberForm: false, }; }, + computed: { + isModalVisible() { + return this.memberRoleToDelete !== null; + }, + }, watch: { groupId: function newFetch(newGroupId) { this.fetchMemberRoles(newGroupId); @@ -70,14 +71,8 @@ export default { async deleteMemberRole() { this.alert?.dismiss(); - const url = Api.buildUrl( - Api.deleteMemberRolesPath - .replace(':id', this.groupId) - .replace(':member_role_id', this.memberRoleToDelete), - ); - try { - await axios.delete(url); + await deleteMemberRole(this.groupId, this.memberRoleToDelete); this.$toast.show(I18N_DELETION_SUCCESS); this.fetchMemberRoles(this.groupId); } catch (error) { @@ -96,16 +91,14 @@ export default { this.memberRoles = []; return; } - - const url = Api.buildUrl(Api.memberRolesPath.replace(':id', groupId)); this.loading = true; try { - const { data } = await axios.get(url); + const { data } = await getMemberRoles(groupId); this.memberRoles = data; } catch (error) { this.memberRoles = []; - if (error.response.status === 404) { + if (error.response.status === HTTP_STATUS_NOT_FOUND) { this.alert = createAlert({ message: I18N_LICENSE_ERROR, variant: VARIANT_DANGER, @@ -121,9 +114,12 @@ export default { } }, listPermissions(item) { - return Object.entries(item) - .filter(([, value]) => value === true) - .map(([key]) => PERMISSIONS[item.base_access_level]?.[key]?.text || key); + return Object.entries(item).flatMap(([key, value]) => { + if (value !== true) { + return []; + } + return [PERMISSIONS[item.base_access_level]?.[key]?.text || key]; + }); }, nameAccessLevel(value) { return ACCESS_LEVEL_LABELS[value]; @@ -133,8 +129,10 @@ export default { this.showCreateMemberForm = false; this.fetchMemberRoles(this.groupId); }, + onModalHide() { + this.memberRoleToDelete = null; + }, showConfirm(memberRoleId) { - this.showConfirmModal = true; this.memberRoleToDelete = memberRoleId; }, }, @@ -169,15 +167,19 @@ export default { <gl-card header-class="gl-new-card-header" body-class="gl-new-card-body gl-px-0 gl-bg-gray-10"> <template #header> <div class="gl-new-card-title-wrapper"> - <h3 class="gl-new-card-title"> + <h3 class="gl-new-card-title" data-testid="card-title"> {{ $options.i18n.cardTitle }} - <span class="gl-new-card-count">{{ memberRoles.length }}</span> + <span class="gl-new-card-count" data-testid="counter">{{ memberRoles.length }}</span> </h3> </div> <div class="gl-new-card-actions"> - <gl-button :disabled="!groupId" size="small" @click="showCreateMemberForm = true">{{ - $options.i18n.addNewRole - }}</gl-button> + <gl-button + :disabled="!groupId" + size="small" + data-testid="add-role" + @click="showCreateMemberForm = true" + >{{ $options.i18n.addNewRole }}</gl-button + > </div> </template> @@ -189,17 +191,13 @@ export default { /> </div> - <gl-table - :fields="$options.FIELDS" - :items="memberRoles" - :busy="loading" - show-empty - stacked="sm" - > - <template #empty> - <gl-empty-state :title="$options.i18n.emptyTitle" :description="emptyText" /> - </template> + <gl-empty-state + v-if="memberRoles.length === 0" + :title="$options.i18n.emptyTitle" + :description="emptyText" + /> + <gl-table v-else :fields="$options.FIELDS" :items="memberRoles" :busy="loading" stacked="sm"> <template #cell(base_access_level)="{ item: { base_access_level } }"> <gl-badge class="gl-my-n4">{{ nameAccessLevel(base_access_level) }}</gl-badge> </template> @@ -228,13 +226,14 @@ export default { </gl-table> <gl-modal - v-model="showConfirmModal" + :visible="isModalVisible" :modal-id="$options.modal.id" size="sm" :title="$options.modal.title" :action-primary="$options.modal.actionPrimary" :action-secondary="$options.modal.actionSecondary" @primary="deleteMemberRole" + @hide="onModalHide" > <p>{{ $options.modal.warning }}</p> </gl-modal> diff --git a/ee/app/assets/javascripts/roles_and_permissions/components/roles_and_permissions_self_managed.vue b/ee/app/assets/javascripts/roles_and_permissions/components/roles_and_permissions_self_managed.vue index 0e3f6f5f3bfa80..0d77481a3a952c 100644 --- a/ee/app/assets/javascripts/roles_and_permissions/components/roles_and_permissions_self_managed.vue +++ b/ee/app/assets/javascripts/roles_and_permissions/components/roles_and_permissions_self_managed.vue @@ -17,6 +17,7 @@ export default { this.groupId = value; }, }, + apiParams: { top_level_only: '1' }, i18n: { emptyText: I18N_EMPTY_TEXT_SELF_MANAGED, }, @@ -28,7 +29,7 @@ export default { <group-select input-id="group-selector" input-name="group-selector" - groups-filter="top_level_only" + :api-params="$options.apiParams" @input="updateGroup" /> <list-member-roles :group-id="groupId" :empty-text="$options.i18n.emptyText" /> diff --git a/ee/app/assets/javascripts/roles_and_permissions/constants.js b/ee/app/assets/javascripts/roles_and_permissions/constants.js index d0165b6843793f..0f5a2b42dc9ce0 100644 --- a/ee/app/assets/javascripts/roles_and_permissions/constants.js +++ b/ee/app/assets/javascripts/roles_and_permissions/constants.js @@ -8,12 +8,12 @@ export const ADMIN_VULNERABILITY = 'admin_vulnerability'; export const GUEST_PERMISSIONS = Object.freeze({ [READ_CODE]: { - help: s__('MemberRoles|Allows read only access to the source code.'), + help: s__('MemberRoles|Allows read-only access to the source code.'), text: s__('MemberRoles|Read code'), value: READ_CODE, }, [READ_VULNERABILITY]: { - help: s__('MemberRoles|Allows read only access to the vulnerability reports.'), + help: s__('MemberRoles|Allows read-only access to the vulnerability reports.'), text: s__('MemberRoles|Read vulnerability'), value: READ_VULNERABILITY, }, @@ -49,12 +49,10 @@ export const FIELDS = [ { key: 'permissions', label: s__('MemberRoles|Permissions'), - sortable: false, }, { key: 'actions', label: s__('MemberRoles|Actions'), - sortable: false, }, ]; @@ -63,16 +61,16 @@ export const I18N_ADD_NEW_ROLE = s__('MemberRoles|Add new role'); export const I18N_CANCEL = __('Cancel'); export const I18N_CARD_TITLE = s__('MemberRoles|Custom roles'); export const I18N_CREATE_ROLE = s__('MemberRoles|Create new role'); -export const I18N_CREATION_ERROR = s__('MemberRoles|Fail to create role.'); +export const I18N_CREATION_ERROR = s__('MemberRoles|Failed to create role.'); export const I18N_CREATION_SUCCESS = s__('MemberRoles|Role successfully created.'); export const I18N_DELETE_ROLE = s__('MemberRoles|Delete role'); -export const I18N_DELETION_ERROR = s__('MemberRoles|Fail to delete the role.'); +export const I18N_DELETION_ERROR = s__('MemberRoles|Failed to delete the role.'); export const I18N_DELETION_SUCCESS = s__('MemberRoles|Role successfully deleted.'); export const I18N_EMPTY_TITLE = s__('MemberRoles|No custom roles for this group'); export const I18N_EMPTY_TEXT_SELF_MANAGED = s__( "MemberRoles|To add a new role select a group and then 'Add new role'.", ); -export const I18N_FETCH_ERROR = s__('MemberRoles|Fail to fetch roles.'); +export const I18N_FETCH_ERROR = s__('MemberRoles|Failed to fetch roles.'); export const I18N_FIELD_FORM_ERROR = __('This field is required.'); export const I18N_LICENSE_ERROR = s__('MemberRoles|Make sure the group has an Ultimate license.'); export const I18N_MODAL_TITLE = s__('MemberRoles|Are you sure you want to delete this role?'); @@ -86,3 +84,4 @@ export const I18N_NEW_ROLE_DESCRIPTION_LABEL = s__('MemberRoles|Description'); export const I18N_NEW_ROLE_NAME_DESCRIPTION = s__('MemberRoles|Enter a short name.'); export const I18N_NEW_ROLE_NAME_LABEL = s__('MemberRoles|Role name'); export const I18N_NEW_ROLE_NAME_PLACEHOLDER = s__('MemberRoles|Incident manager'); +export const I18N_NEW_ROLE_PERMISSIONS_LABEL = s__('MemberRoles|Permissions'); diff --git a/ee/app/assets/javascripts/roles_and_permissions/index.js b/ee/app/assets/javascripts/roles_and_permissions/index.js index a7e65abf414e5d..f9601abca41a76 100644 --- a/ee/app/assets/javascripts/roles_and_permissions/index.js +++ b/ee/app/assets/javascripts/roles_and_permissions/index.js @@ -1,6 +1,9 @@ +import { GlToast } from '@gitlab/ui'; import Vue from 'vue'; import RolesAndPermissionsSelfManaged from './components/roles_and_permissions_self_managed.vue'; +Vue.use(GlToast); + export const initRolesAndPermissionsSelfManagedApp = () => { const el = document.querySelector('#js-roles-and-permissions-self-managed'); diff --git a/ee/app/assets/stylesheets/page_bundles/member_roles.scss b/ee/app/assets/stylesheets/page_bundles/member_roles.scss index 2fc79768b85a91..18812880bb82a2 100644 --- a/ee/app/assets/stylesheets/page_bundles/member_roles.scss +++ b/ee/app/assets/stylesheets/page_bundles/member_roles.scss @@ -1,5 +1,6 @@ @import 'page_bundles/mixins_and_variables_and_functions'; +// Remove this class once https://gitlab.com/gitlab-org/gitlab-ui/-/merge_requests/3634 is merged .justify-end-on-sm { @include gl-media-breakpoint-down(sm) { @include gl-justify-content-end; diff --git a/ee/config/feature_flags/development/custom_roles_ui_self_managed.yml b/ee/config/feature_flags/development/custom_roles_ui_self_managed.yml index 78fc1fa619759b..eb514eea292c83 100644 --- a/ee/config/feature_flags/development/custom_roles_ui_self_managed.yml +++ b/ee/config/feature_flags/development/custom_roles_ui_self_managed.yml @@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/416644 milestone: '16.2' type: development group: group::authentication and authorization -default_enabled: true +default_enabled: false diff --git a/ee/spec/frontend/roles_and_permissions/components/create_member_role_spec.js b/ee/spec/frontend/roles_and_permissions/components/create_member_role_spec.js index 4309ad32fee769..0460cbd64632bc 100644 --- a/ee/spec/frontend/roles_and_permissions/components/create_member_role_spec.js +++ b/ee/spec/frontend/roles_and_permissions/components/create_member_role_spec.js @@ -4,7 +4,6 @@ import { nextTick } from 'vue'; import { createAlert, VARIANT_DANGER } from '~/alert'; import axios from '~/lib/utils/axios_utils'; import { HTTP_STATUS_INTERNAL_SERVER_ERROR, HTTP_STATUS_OK } from '~/lib/utils/http_status'; -import { __ } from '~/locale'; import CreateMemberRole from 'ee/roles_and_permissions/components/create_member_role.vue'; import { I18N_CREATION_ERROR } from 'ee/roles_and_permissions/constants'; import { mountExtended } from 'helpers/vue_test_utils_helper'; @@ -28,8 +27,8 @@ describe('CreateMemberRole', () => { }); }; - const findButtonSubmit = () => wrapper.findByTestId('submit'); - const findButtonCancel = () => wrapper.findByTestId('cancel'); + const findButtonSubmit = () => wrapper.findByTestId('submit-button'); + const findButtonCancel = () => wrapper.findByTestId('cancel-button'); const findNameField = () => wrapper.findComponent(GlFormInput); const findCheckboxes = () => wrapper.findAllComponents(GlFormCheckbox); const findSelect = () => wrapper.findComponent(GlFormSelect); @@ -48,17 +47,29 @@ describe('CreateMemberRole', () => { createComponent(); }); - it('has only one select options', () => { + it('has only one select option', () => { const options = findOptions(); expect(options).toHaveLength(1); expect(options.at(0).attributes()).toMatchObject({ value: '10' }); - expect(options.at(0).text()).toBe(__('Guest')); + expect(options.at(0).text()).toBe('Guest'); }); it('has multiple checkbox permissions', () => { const checkboxes = findCheckboxes(); - expect(checkboxes.length).toBeGreaterThan(1); - expect(checkboxes.at(0).find('p[class=help-text]').exists()).toBe(true); + const checkboxOneText = checkboxes.at(0).text(); + const checkboxTwoText = checkboxes.at(1).text(); + const checkboxThreeText = checkboxes.at(2).text(); + + expect(checkboxOneText).toContain('Read code'); + expect(checkboxOneText).toContain('Allows read-only access to the source code.'); + + expect(checkboxTwoText).toContain('Read vulnerability'); + expect(checkboxTwoText).toContain('Allows read-only access to the vulnerability reports.'); + + expect(checkboxThreeText).toContain('Admin vulnerability'); + expect(checkboxThreeText).toContain( + "Allows admin access to the vulnerability reports. 'Read vulnerability' must be selected in order to take effect.", + ); }); it('emits cancel event', () => { @@ -73,7 +84,7 @@ describe('CreateMemberRole', () => { it('shows warnings in the name field', async () => { expect(findNameField().classes()).toContain('is-valid'); - findButtonSubmit().trigger('click'); + findButtonSubmit().trigger('submit'); await nextTick(); expect(findNameField().classes()).toContain('is-invalid'); @@ -82,7 +93,7 @@ describe('CreateMemberRole', () => { it('shows warnings if permissions are unchecked', async () => { expect(findCheckboxes().at(0).find('input').classes()).not.toContain('is-invalid'); - findButtonSubmit().trigger('click'); + findButtonSubmit().trigger('submit'); await nextTick(); expect(findCheckboxes().at(0).find('input').classes()).toContain('is-invalid'); @@ -97,7 +108,7 @@ describe('CreateMemberRole', () => { }); it('sends the correct data', async () => { - findButtonSubmit().trigger('click'); + findButtonSubmit().trigger('submit'); await waitForPromises(); expect(mockAxios.history.post[0].data).toBe( @@ -108,7 +119,7 @@ describe('CreateMemberRole', () => { it('emits success event', async () => { expect(wrapper.emitted('success')).toBeUndefined(); - findButtonSubmit().trigger('click'); + findButtonSubmit().trigger('submit'); await waitForPromises(); expect(wrapper.emitted('success')).toHaveLength(1); @@ -122,11 +133,10 @@ describe('CreateMemberRole', () => { mockAxios .onPost('/api/v4/groups/4/member_roles') .replyOnce(HTTP_STATUS_INTERNAL_SERVER_ERROR); - // .replyOnce(HTTP_STATUS_OK); }); it('shows alert', async () => { - findButtonSubmit().trigger('click'); + findButtonSubmit().trigger('submit'); await waitForPromises(); expect(createAlert).toHaveBeenCalledWith({ @@ -136,12 +146,12 @@ describe('CreateMemberRole', () => { }); it('dismisses previous alert', async () => { - findButtonSubmit().trigger('click'); + findButtonSubmit().trigger('submit'); await waitForPromises(); expect(mockAlertDismiss).toHaveBeenCalledTimes(0); - findButtonSubmit().trigger('click'); + findButtonSubmit().trigger('submit'); expect(mockAlertDismiss).toHaveBeenCalledTimes(1); }); diff --git a/ee/spec/frontend/roles_and_permissions/components/list_member_roles_spec.js b/ee/spec/frontend/roles_and_permissions/components/list_member_roles_spec.js index 5836e43c82fcb6..8c2207c02bb1de 100644 --- a/ee/spec/frontend/roles_and_permissions/components/list_member_roles_spec.js +++ b/ee/spec/frontend/roles_and_permissions/components/list_member_roles_spec.js @@ -1,5 +1,4 @@ -import { GlBadge, GlButton, GlCard, GlEmptyState, GlModal, GlTable } from '@gitlab/ui'; -import { mount, shallowMount } from '@vue/test-utils'; +import { GlCard, GlEmptyState, GlModal, GlTable } from '@gitlab/ui'; import MockAdapter from 'axios-mock-adapter'; import { nextTick } from 'vue'; import { createAlert, VARIANT_DANGER } from '~/alert'; @@ -10,7 +9,6 @@ import { HTTP_STATUS_NO_CONTENT, HTTP_STATUS_OK, } from '~/lib/utils/http_status'; -import { __, s__ } from '~/locale'; import CreateMemberRole from 'ee/roles_and_permissions/components/create_member_role.vue'; import { I18N_CREATION_SUCCESS, @@ -20,6 +18,7 @@ import { I18N_LICENSE_ERROR, } from 'ee/roles_and_permissions/constants'; import ListMemberRoles from 'ee/roles_and_permissions/components/list_member_roles.vue'; +import { mountExtended, shallowMountExtended } from 'helpers/vue_test_utils_helper'; import waitForPromises from 'helpers/wait_for_promises'; const mockAlertDismiss = jest.fn(); @@ -29,7 +28,7 @@ jest.mock('~/alert', () => ({ })), })); -describe('RolesAndPermissionsSelfManaged', () => { +describe('ListMemberRoles', () => { const emptyText = 'blah, blah'; const groupId = '49'; let mockAxios; @@ -53,7 +52,7 @@ describe('RolesAndPermissionsSelfManaged', () => { const mockToastShow = jest.fn(); let wrapper; - const createComponent = (props = {}, mountFn = shallowMount) => { + const createComponent = (props = {}, mountFn = shallowMountExtended) => { wrapper = mountFn(ListMemberRoles, { propsData: { emptyText, ...props }, stubs: { GlCard }, @@ -65,16 +64,16 @@ describe('RolesAndPermissionsSelfManaged', () => { }); }; - const findAddRoleButton = () => wrapper.find('.gl-new-card-actions').findComponent(GlButton); - const findCounter = () => wrapper.find('.gl-new-card-count'); + const findAddRoleButton = () => wrapper.findByTestId('add-role'); + const findButtonByText = (text) => wrapper.findByRole('button', { name: text }); + const findCounter = () => wrapper.findByTestId('counter'); const findCreateMemberRole = () => wrapper.findComponent(CreateMemberRole); const findEmptyState = () => wrapper.findComponent(GlEmptyState); const findModal = () => wrapper.findComponent(GlModal); const findTable = () => wrapper.findComponent(GlTable); - const findTableHeader = (idx) => findTable().findAll('th').at(idx); - const findCell = (trIdx, tdIdx) => { - return findTable().find('tbody').findAll('tr').at(trIdx).findAll('td').at(tdIdx).find('div'); - }; + const findTableHeader = (index) => findTable().findAll('th').at(index); + const findCellByText = (text) => wrapper.findByRole('cell', { name: text }); + const findCells = () => wrapper.findAllByRole('cell'); beforeEach(() => { window.gon = { api_version: 'v4' }; @@ -84,7 +83,7 @@ describe('RolesAndPermissionsSelfManaged', () => { it('shows empty state', () => { createComponent({ groupId: null }); - expect(wrapper.find('.gl-new-card-title').text()).toMatch(ListMemberRoles.i18n.cardTitle); + expect(wrapper.findByTestId('card-title').text()).toMatch(ListMemberRoles.i18n.cardTitle); expect(findCounter().text()).toBe('0'); expect(findAddRoleButton().props('disabled')).toBe(true); expect(findEmptyState().props()).toMatchObject({ @@ -97,10 +96,12 @@ describe('RolesAndPermissionsSelfManaged', () => { describe('fetching roles', () => { it('toggles the table busy state', async () => { createComponent({ groupId }); - expect(findTable().attributes('busy')).toBe('true'); await waitForPromises(); - expect(findTable().attributes('busy')).toBeUndefined(); + + wrapper.setProps({ groupId: '9' }); + await nextTick(); + expect(findTable().attributes('busy')).toBe('true'); }); it('upon changes in the groupId', async () => { @@ -108,7 +109,7 @@ describe('RolesAndPermissionsSelfManaged', () => { await waitForPromises(); expect(mockAxios.history.get).toHaveLength(0); - wrapper.vm.$options.watch.groupId.call(wrapper.vm, groupId); + wrapper.setProps({ groupId }); await waitForPromises(); expect(mockAxios.history.get).toHaveLength(1); }); @@ -145,7 +146,7 @@ describe('RolesAndPermissionsSelfManaged', () => { await waitForPromises(); expect(mockAlertDismiss).toHaveBeenCalledTimes(0); - wrapper.vm.$options.watch.groupId.call(wrapper.vm, groupId); + wrapper.setProps({ groupId }); await waitForPromises(); expect(mockAlertDismiss).toHaveBeenCalledTimes(1); }); @@ -188,88 +189,66 @@ describe('RolesAndPermissionsSelfManaged', () => { describe('table of roles', () => { it('shows name and id', async () => { - createComponent({ groupId }, mount); + createComponent({ groupId }, mountExtended); await waitForPromises(); - expect(findCell(0, 0).text()).toBe(mockResponse[0].name); - expect(findCell(0, 1).text()).toBe(`${mockResponse[0].id}`); + expect(findCellByText(mockResponse[0].name).exists()).toBe(true); + expect(findCellByText(`${mockResponse[0].id}`).exists()).toBe(true); }); - it('sorts columns by name', async () => { - createComponent({ groupId }, mount); + const expectSortedByHeader = async (index) => { + createComponent({ groupId }, mountExtended); await waitForPromises(); - expect(findCell(0, 0).text()).toBe(mockResponse[0].name); - expect(findCell(1, 0).text()).toBe(mockResponse[1].name); + expect(findTableHeader(index).attributes('aria-sort')).toBe('none'); - findTableHeader(0).trigger('click'); - findTableHeader(0).trigger('click'); + findTableHeader(index).trigger('click'); await nextTick(); + expect(findTableHeader(index).attributes('aria-sort')).toBe('ascending'); - expect(findCell(0, 0).text()).toBe(mockResponse[1].name); - expect(findCell(1, 0).text()).toBe(mockResponse[0].name); - }); - - it('sorts columns by ID', async () => { - createComponent({ groupId }, mount); - await waitForPromises(); - expect(findCell(0, 1).text()).toBe(`${mockResponse[0].id}`); - expect(findCell(1, 1).text()).toBe(`${mockResponse[1].id}`); - - findTableHeader(1).trigger('click'); - findTableHeader(1).trigger('click'); + findTableHeader(index).trigger('click'); await nextTick(); + expect(findTableHeader(index).attributes('aria-sort')).toBe('descending'); + }; - expect(findCell(0, 1).text()).toBe(`${mockResponse[1].id}`); - expect(findCell(1, 1).text()).toBe(`${mockResponse[0].id}`); + it('sorts columns by name', () => { + expectSortedByHeader(0); }); - it('sorts columns by base role', async () => { - createComponent({ groupId }, mount); - await waitForPromises(); - expect(findCell(0, 2).findComponent(GlBadge).text()).toBe(__('Guest')); - expect(findCell(1, 2).findComponent(GlBadge).text()).toBe(__('Developer')); - - findTableHeader(2).trigger('click'); - findTableHeader(2).trigger('click'); - await nextTick(); + it('sorts columns by ID', () => { + expectSortedByHeader(1); + }); - expect(findCell(0, 2).findComponent(GlBadge).text()).toBe(__('Developer')); - expect(findCell(1, 2).findComponent(GlBadge).text()).toBe(__('Guest')); + it('sorts columns by base role', () => { + expectSortedByHeader(2); }); it('shows list of standard permissions', async () => { - createComponent({ groupId }, mount); + createComponent({ groupId }, mountExtended); await waitForPromises(); - expect(findCell(0, 3).findAllComponents(GlBadge).at(0).text()).toBe( - s__('MemberRoles|Read code'), - ); - expect(findCell(0, 3).findAllComponents(GlBadge).at(1).text()).toBe( - s__('MemberRoles|Read vulnerability'), - ); - expect(findCell(0, 3).findAllComponents(GlBadge).at(2).text()).toBe( - s__('MemberRoles|Admin vulnerability'), - ); + const badgesText = findCells().at(3).text(); + expect(badgesText).toContain('Read code'); + expect(badgesText).toContain('Read vulnerability'); + expect(badgesText).toContain('Admin vulnerability'); }); it('shows list of non-standard permissions', async () => { - createComponent({ groupId }, mount); + createComponent({ groupId }, mountExtended); await waitForPromises(); - const badges = findCell(1, 3).findAllComponents(GlBadge); - expect(badges).toHaveLength(1); - expect(badges.at(0).text()).toBe('non_standard_permission'); + const badgesText = findCells().at(8).text(); + expect(badgesText).toBe('non_standard_permission'); }); }); describe('delete role', () => { const clickRoleDelete = () => { - findCell(0, 4).findComponent(GlButton).trigger('click'); + findButtonByText('Delete role').trigger('click'); return nextTick(); }; beforeEach(async () => { - createComponent({ groupId }, mount); + createComponent({ groupId }, mountExtended); await waitForPromises(); }); diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 258cce5809603e..b7c510fbe54e71 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -28845,10 +28845,10 @@ msgstr "" msgid "MemberRoles|Allows admin access to the vulnerability reports. 'Read vulnerability' must be selected in order to take effect." msgstr "" -msgid "MemberRoles|Allows read only access to the source code." +msgid "MemberRoles|Allows read-only access to the source code." msgstr "" -msgid "MemberRoles|Allows read only access to the vulnerability reports." +msgid "MemberRoles|Allows read-only access to the vulnerability reports." msgstr "" msgid "MemberRoles|Are you sure you want to delete this role?" @@ -28875,13 +28875,13 @@ msgstr "" msgid "MemberRoles|Enter a short name." msgstr "" -msgid "MemberRoles|Fail to create role." +msgid "MemberRoles|Failed to create role." msgstr "" -msgid "MemberRoles|Fail to delete the role." +msgid "MemberRoles|Failed to delete the role." msgstr "" -msgid "MemberRoles|Fail to fetch roles." +msgid "MemberRoles|Failed to fetch roles." msgstr "" msgid "MemberRoles|ID" diff --git a/spec/frontend/vue_shared/components/entity_select/utils_spec.js b/spec/frontend/vue_shared/components/entity_select/utils_spec.js index 9aa1baf204ef08..1d73924aa58400 100644 --- a/spec/frontend/vue_shared/components/entity_select/utils_spec.js +++ b/spec/frontend/vue_shared/components/entity_select/utils_spec.js @@ -2,12 +2,16 @@ import { groupsPath } from '~/vue_shared/components/entity_select/utils'; describe('entity_select utils', () => { describe('groupsPath', () => { + beforeEach(() => { + window.gon = { api_version: 'v4' }; + }); + it.each` groupsFilter | parentGroupID | expectedPath - ${undefined} | ${undefined} | ${'/api/:version/groups.json'} - ${undefined} | ${1} | ${'/api/:version/groups.json'} - ${'descendant_groups'} | ${1} | ${'/api/:version/groups/1/descendant_groups'} - ${'subgroups'} | ${1} | ${'/api/:version/groups/1/subgroups'} + ${undefined} | ${undefined} | ${'/api/v4/groups.json'} + ${undefined} | ${1} | ${'/api/v4/groups.json'} + ${'descendant_groups'} | ${1} | ${'/api/v4/groups/1/descendant_groups'} + ${'subgroups'} | ${1} | ${'/api/v4/groups/1/subgroups'} `( 'returns $expectedPath with groupsFilter = $groupsFilter and parentGroupID = $parentGroupID', ({ groupsFilter, parentGroupID, expectedPath }) => { -- GitLab From 148234496ff941c75b427de4808202638951cab8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20Sanz=20Garc=C3=ADa?= <esanz-garcia@gitlab.com> Date: Tue, 5 Sep 2023 19:09:43 +0200 Subject: [PATCH 3/4] Implement feedback from frontend review * use $options for storing IDs * add test for API * add `js-no-auto-disable` to the submit button --- app/assets/javascripts/rest_api.js | 1 - .../javascripts/api/member_roles_api.js | 18 ++++- ee/app/assets/javascripts/rest_api.js | 1 + .../components/create_member_role.vue | 31 ++++++--- .../components/list_member_roles.vue | 6 +- ee/spec/frontend/api/member_roles_api_spec.js | 61 ++++++++++++++++ .../components/create_member_role_spec.js | 30 ++++---- .../components/list_member_roles_spec.js | 69 +++++++------------ 8 files changed, 140 insertions(+), 77 deletions(-) rename {app => ee/app}/assets/javascripts/api/member_roles_api.js (60%) create mode 100644 ee/spec/frontend/api/member_roles_api_spec.js diff --git a/app/assets/javascripts/rest_api.js b/app/assets/javascripts/rest_api.js index 0250557e6879f6..87996d0bb851d6 100644 --- a/app/assets/javascripts/rest_api.js +++ b/app/assets/javascripts/rest_api.js @@ -8,7 +8,6 @@ export * from './api/tags_api'; export * from './api/alert_management_alerts_api'; export * from './api/harbor_registry'; export * from './api/environments_api'; -export * from './api/member_roles_api'; // Note: It's not possible to spy on methods imported from this file in // Jest tests. diff --git a/app/assets/javascripts/api/member_roles_api.js b/ee/app/assets/javascripts/api/member_roles_api.js similarity index 60% rename from app/assets/javascripts/api/member_roles_api.js rename to ee/app/assets/javascripts/api/member_roles_api.js index 65b5aedda1fdca..61b4701167031a 100644 --- a/app/assets/javascripts/api/member_roles_api.js +++ b/ee/app/assets/javascripts/api/member_roles_api.js @@ -1,21 +1,35 @@ -import axios from '../lib/utils/axios_utils'; -import { buildApiUrl } from './api_utils'; +import axios from '~/lib/utils/axios_utils'; +import { buildApiUrl } from '~/api/api_utils'; const MEMBER_ROLES_PATH = '/api/:version/groups/:id/member_roles'; const DELETE_MEMBER_ROLES_PATH = '/api/:version/groups/:id/member_roles/:member_role_id'; +/** + * Creates a new member role on a group (groupId). + * @param {string} groupId + * @param {object} data + */ export function createMemberRole(groupId, data) { const url = buildApiUrl(MEMBER_ROLES_PATH.replace(':id', groupId)); return axios.post(url, data); } +/** + * Fetches all the member roles associated to a group (groupId). + * @param {string} groupId + */ export function getMemberRoles(groupId) { const url = buildApiUrl(MEMBER_ROLES_PATH.replace(':id', groupId)); return axios.get(url); } +/** + * Deletes the member role (roleId) associated to a group (groupId). + * @param {string} groupId + * @param {string} roleId + */ export function deleteMemberRole(groupId, roleId) { const url = buildApiUrl( DELETE_MEMBER_ROLES_PATH.replace(':id', groupId).replace(':member_role_id', roleId), diff --git a/ee/app/assets/javascripts/rest_api.js b/ee/app/assets/javascripts/rest_api.js index cc8586201a4f9d..d1792478301b96 100644 --- a/ee/app/assets/javascripts/rest_api.js +++ b/ee/app/assets/javascripts/rest_api.js @@ -3,3 +3,4 @@ export * from './api/subscriptions_api'; export * from './api/dora_api'; export * from './api/arkose_labs_api'; export * from './api/status_check_api'; +export * from './api/member_roles_api'; diff --git a/ee/app/assets/javascripts/roles_and_permissions/components/create_member_role.vue b/ee/app/assets/javascripts/roles_and_permissions/components/create_member_role.vue index 47e7962315cc06..a19ea472f350ed 100644 --- a/ee/app/assets/javascripts/roles_and_permissions/components/create_member_role.vue +++ b/ee/app/assets/javascripts/roles_and_permissions/components/create_member_role.vue @@ -9,9 +9,9 @@ import { GlFormSelect, GlFormTextarea, } from '@gitlab/ui'; +import { createMemberRole } from 'ee/rest_api'; import { ACCESS_LEVEL_LABELS, ACCESS_LEVEL_GUEST_INTEGER } from '~/access_level/constants'; import { createAlert, VARIANT_DANGER } from '~/alert'; -import { createMemberRole } from '~/rest_api'; import { I18N_CANCEL, I18N_CREATE_ROLE, @@ -123,16 +123,19 @@ export default { })), i18n: { baseRole: { + id: 'group-1', label: I18N_NEW_ROLE_BASE_ROLE_LABEL, description: I18N_NEW_ROLE_BASE_ROLE_DESCRIPTION, }, cancel: I18N_CANCEL, createRole: I18N_CREATE_ROLE, description: { + id: 'group-2', label: I18N_NEW_ROLE_DESCRIPTION_LABEL, }, fieldFormError: I18N_FIELD_FORM_ERROR, name: { + id: 'group-3', label: I18N_NEW_ROLE_NAME_LABEL, placeholder: I18N_NEW_ROLE_NAME_PLACEHOLDER, description: I18N_NEW_ROLE_NAME_DESCRIPTION, @@ -153,10 +156,10 @@ export default { :label="$options.i18n.baseRole.label" :description="$options.i18n.baseRole.description" :invalid-feedback="$options.i18n.fieldFormError" - label-for="group-1" + :label-for="$options.i18n.baseRole.id" > <gl-form-select - id="group-1" + :id="$options.i18n.baseRole.id" v-model="baseRole" :options="$options.baseRoleOptions" required @@ -169,18 +172,22 @@ export default { :label="$options.i18n.name.label" :description="$options.i18n.name.description" :invalid-feedback="$options.i18n.fieldFormError" - label-for="group-2" + :label-for="$options.i18n.name.id" > <gl-form-input - id="group-2" + :id="$options.i18n.name.id" v-model="name" :placeholder="$options.i18n.name.placeholder" :state="nameValid" /> </gl-form-group> - <gl-form-group class="col-lg-8" :label="$options.i18n.description.label" label-for="group-3"> - <gl-form-textarea id="group-3" v-model="description" /> + <gl-form-group + class="col-lg-8" + :label="$options.i18n.description.label" + :label-for="$options.i18n.description.id" + > + <gl-form-textarea :id="$options.i18n.description.id" v-model="description" /> </gl-form-group> </div> @@ -200,9 +207,13 @@ export default { </gl-form-group> <div class="gl-display-flex gl-flex-wrap gl-gap-3"> - <gl-button type="submit" data-testid="submit-button" variant="confirm" :disabled="false">{{ - $options.i18n.createRole - }}</gl-button> + <gl-button + type="submit" + data-testid="submit-button" + variant="confirm" + class="js-no-auto-disable" + >{{ $options.i18n.createRole }}</gl-button + > <gl-button type="reset" data-testid="cancel-button" @click="cancel">{{ $options.i18n.cancel }}</gl-button> diff --git a/ee/app/assets/javascripts/roles_and_permissions/components/list_member_roles.vue b/ee/app/assets/javascripts/roles_and_permissions/components/list_member_roles.vue index 167b7fb3e448e2..cc5d0d39b76871 100644 --- a/ee/app/assets/javascripts/roles_and_permissions/components/list_member_roles.vue +++ b/ee/app/assets/javascripts/roles_and_permissions/components/list_member_roles.vue @@ -1,9 +1,9 @@ <script> import { GlBadge, GlButton, GlCard, GlEmptyState, GlModal, GlTable } from '@gitlab/ui'; +import { deleteMemberRole, getMemberRoles } from 'ee/rest_api'; import { ACCESS_LEVEL_LABELS } from '~/access_level/constants'; import { createAlert, VARIANT_DANGER } from '~/alert'; import { HTTP_STATUS_NOT_FOUND } from '~/lib/utils/http_status'; -import { deleteMemberRole, getMemberRoles } from '~/rest_api'; import { FIELDS, I18N_ADD_NEW_ROLE, @@ -98,7 +98,7 @@ export default { this.memberRoles = data; } catch (error) { this.memberRoles = []; - if (error.response.status === HTTP_STATUS_NOT_FOUND) { + if (error?.response?.status === HTTP_STATUS_NOT_FOUND) { this.alert = createAlert({ message: I18N_LICENSE_ERROR, variant: VARIANT_DANGER, @@ -133,7 +133,7 @@ export default { this.memberRoleToDelete = null; }, showConfirm(memberRoleId) { - this.memberRoleToDelete = memberRoleId; + this.memberRoleToDelete = `${memberRoleId}`; }, }, FIELDS, diff --git a/ee/spec/frontend/api/member_roles_api_spec.js b/ee/spec/frontend/api/member_roles_api_spec.js new file mode 100644 index 00000000000000..2baade24c75d46 --- /dev/null +++ b/ee/spec/frontend/api/member_roles_api_spec.js @@ -0,0 +1,61 @@ +import MockAdapter from 'axios-mock-adapter'; +import { createMemberRole, deleteMemberRole, getMemberRoles } from 'ee/api/member_roles_api'; +import axios from '~/lib/utils/axios_utils'; +import { HTTP_STATUS_OK } from '~/lib/utils/http_status'; + +describe('member_roles_api.js', () => { + let mockAxios; + + beforeEach(() => { + window.gon = { api_version: 'v4' }; + mockAxios = new MockAdapter(axios); + }); + + describe('createMemberRole', () => { + const data = { + base_access_level: '10', + name: 'My name', + description: 'My description', + read_code: 1, + }; + beforeEach(() => { + mockAxios.onPost('/api/v4/groups/4/member_roles').replyOnce(HTTP_STATUS_OK); + }); + + it('posts data to create a new member role', async () => { + expect(mockAxios.history.post).toHaveLength(0); + + await createMemberRole('4', data); + + expect(mockAxios.history.post[0].data).toBe(JSON.stringify(data)); + }); + }); + + describe('getMemberRoles', () => { + beforeEach(() => { + mockAxios.onGet('/api/v4/groups/4/member_roles').replyOnce(HTTP_STATUS_OK); + }); + + it('fetches member roles', async () => { + expect(mockAxios.history.get).toHaveLength(0); + + await getMemberRoles('4'); + + expect(mockAxios.history.get).toHaveLength(1); + }); + }); + + describe('deleteMemberRole', () => { + beforeEach(() => { + mockAxios.onDelete('/api/v4/groups/4/member_roles/8').replyOnce(HTTP_STATUS_OK); + }); + + it('fetches member roles', async () => { + expect(mockAxios.history.delete).toHaveLength(0); + + await deleteMemberRole('4', '8'); + + expect(mockAxios.history.delete).toHaveLength(1); + }); + }); +}); diff --git a/ee/spec/frontend/roles_and_permissions/components/create_member_role_spec.js b/ee/spec/frontend/roles_and_permissions/components/create_member_role_spec.js index 0460cbd64632bc..ad4a80a063444d 100644 --- a/ee/spec/frontend/roles_and_permissions/components/create_member_role_spec.js +++ b/ee/spec/frontend/roles_and_permissions/components/create_member_role_spec.js @@ -1,14 +1,14 @@ import { GlFormInput, GlFormSelect, GlFormTextarea, GlFormCheckbox } from '@gitlab/ui'; -import MockAdapter from 'axios-mock-adapter'; import { nextTick } from 'vue'; import { createAlert, VARIANT_DANGER } from '~/alert'; -import axios from '~/lib/utils/axios_utils'; -import { HTTP_STATUS_INTERNAL_SERVER_ERROR, HTTP_STATUS_OK } from '~/lib/utils/http_status'; +import { createMemberRole } from 'ee/api/member_roles_api'; import CreateMemberRole from 'ee/roles_and_permissions/components/create_member_role.vue'; import { I18N_CREATION_ERROR } from 'ee/roles_and_permissions/constants'; import { mountExtended } from 'helpers/vue_test_utils_helper'; import waitForPromises from 'helpers/wait_for_promises'; +jest.mock('ee/api/member_roles_api'); + const mockAlertDismiss = jest.fn(); jest.mock('~/alert', () => ({ createAlert: jest.fn().mockImplementation(() => ({ @@ -17,7 +17,6 @@ jest.mock('~/alert', () => ({ })); describe('CreateMemberRole', () => { - let mockAxios; let wrapper; const createComponent = () => { @@ -35,15 +34,15 @@ describe('CreateMemberRole', () => { const findOptions = () => findSelect().findAll('option'); const findTextArea = () => wrapper.findComponent(GlFormTextarea); + const name = 'My role name'; + const description = 'My description'; const fillForm = () => { - findNameField().setValue('My role name'); - findTextArea().setValue('My description'); + findNameField().setValue(name); + findTextArea().setValue(description); findCheckboxes().at(0).find('input').setChecked(); }; beforeEach(() => { - window.gon = { api_version: 'v4' }; - mockAxios = new MockAdapter(axios); createComponent(); }); @@ -103,17 +102,18 @@ describe('CreateMemberRole', () => { describe('when successful submission', () => { beforeEach(() => { fillForm(); - - mockAxios.onPost('/api/v4/groups/4/member_roles').replyOnce(HTTP_STATUS_OK); }); it('sends the correct data', async () => { findButtonSubmit().trigger('submit'); await waitForPromises(); - expect(mockAxios.history.post[0].data).toBe( - '{"base_access_level":"10","name":"My role name","description":"My description","read_code":1}', - ); + expect(createMemberRole).toHaveBeenCalledWith('4', { + base_access_level: '10', + name, + description, + read_code: 1, + }); }); it('emits success event', async () => { @@ -130,9 +130,7 @@ describe('CreateMemberRole', () => { beforeEach(() => { fillForm(); - mockAxios - .onPost('/api/v4/groups/4/member_roles') - .replyOnce(HTTP_STATUS_INTERNAL_SERVER_ERROR); + createMemberRole.mockRejectedValue(new Error()); }); it('shows alert', async () => { diff --git a/ee/spec/frontend/roles_and_permissions/components/list_member_roles_spec.js b/ee/spec/frontend/roles_and_permissions/components/list_member_roles_spec.js index 8c2207c02bb1de..19fbcad3d976e3 100644 --- a/ee/spec/frontend/roles_and_permissions/components/list_member_roles_spec.js +++ b/ee/spec/frontend/roles_and_permissions/components/list_member_roles_spec.js @@ -1,14 +1,8 @@ import { GlCard, GlEmptyState, GlModal, GlTable } from '@gitlab/ui'; -import MockAdapter from 'axios-mock-adapter'; import { nextTick } from 'vue'; import { createAlert, VARIANT_DANGER } from '~/alert'; -import axios from '~/lib/utils/axios_utils'; -import { - HTTP_STATUS_INTERNAL_SERVER_ERROR, - HTTP_STATUS_NOT_FOUND, - HTTP_STATUS_NO_CONTENT, - HTTP_STATUS_OK, -} from '~/lib/utils/http_status'; +import { HTTP_STATUS_INTERNAL_SERVER_ERROR, HTTP_STATUS_NOT_FOUND } from '~/lib/utils/http_status'; +import { getMemberRoles, deleteMemberRole } from 'ee/api/member_roles_api'; import CreateMemberRole from 'ee/roles_and_permissions/components/create_member_role.vue'; import { I18N_CREATION_SUCCESS, @@ -21,6 +15,8 @@ import ListMemberRoles from 'ee/roles_and_permissions/components/list_member_rol import { mountExtended, shallowMountExtended } from 'helpers/vue_test_utils_helper'; import waitForPromises from 'helpers/wait_for_promises'; +jest.mock('ee/api/member_roles_api'); + const mockAlertDismiss = jest.fn(); jest.mock('~/alert', () => ({ createAlert: jest.fn().mockImplementation(() => ({ @@ -31,7 +27,6 @@ jest.mock('~/alert', () => ({ describe('ListMemberRoles', () => { const emptyText = 'blah, blah'; const groupId = '49'; - let mockAxios; const mockResponse = [ { id: 1, @@ -71,14 +66,11 @@ describe('ListMemberRoles', () => { const findEmptyState = () => wrapper.findComponent(GlEmptyState); const findModal = () => wrapper.findComponent(GlModal); const findTable = () => wrapper.findComponent(GlTable); - const findTableHeader = (index) => findTable().findAll('th').at(index); const findCellByText = (text) => wrapper.findByRole('cell', { name: text }); const findCells = () => wrapper.findAllByRole('cell'); beforeEach(() => { - window.gon = { api_version: 'v4' }; - mockAxios = new MockAdapter(axios); - mockAxios.onGet('/api/v4/groups/49/member_roles').replyOnce(HTTP_STATUS_OK, mockResponse); + getMemberRoles.mockResolvedValue({ data: mockResponse }); }); it('shows empty state', () => { @@ -107,16 +99,16 @@ describe('ListMemberRoles', () => { it('upon changes in the groupId', async () => { createComponent({ groupId: null }); await waitForPromises(); - expect(mockAxios.history.get).toHaveLength(0); + expect(getMemberRoles).toHaveBeenCalledTimes(0); wrapper.setProps({ groupId }); await waitForPromises(); - expect(mockAxios.history.get).toHaveLength(1); + expect(getMemberRoles).toHaveBeenCalledTimes(1); }); it('shows license alert for 404 responses', async () => { + getMemberRoles.mockRejectedValue({ response: { status: HTTP_STATUS_NOT_FOUND } }); createComponent({ groupId: '100' }); - mockAxios.onGet('/api/v4/groups/100/member_roles').replyOnce(HTTP_STATUS_NOT_FOUND); await waitForPromises(); expect(createAlert).toHaveBeenCalledWith({ @@ -126,10 +118,8 @@ describe('ListMemberRoles', () => { }); it('shows generic alert for non-404 responses', async () => { + getMemberRoles.mockRejectedValue({ response: { status: HTTP_STATUS_INTERNAL_SERVER_ERROR } }); createComponent({ groupId: '100' }); - mockAxios - .onGet('/api/v4/groups/100/member_roles') - .replyOnce(HTTP_STATUS_INTERNAL_SERVER_ERROR); await waitForPromises(); expect(createAlert).toHaveBeenCalledWith({ @@ -139,10 +129,8 @@ describe('ListMemberRoles', () => { }); it('dismisses previous alerts', async () => { + getMemberRoles.mockRejectedValue({ response: { status: HTTP_STATUS_INTERNAL_SERVER_ERROR } }); createComponent({ groupId: '100' }); - mockAxios - .onGet('/api/v4/groups/100/member_roles') - .replyOnce(HTTP_STATUS_INTERNAL_SERVER_ERROR); await waitForPromises(); expect(mockAlertDismiss).toHaveBeenCalledTimes(0); @@ -177,12 +165,12 @@ describe('ListMemberRoles', () => { }); it('fetches roles', async () => { - expect(mockAxios.history.get).toHaveLength(1); + expect(getMemberRoles).toHaveBeenCalledTimes(1); findCreateMemberRole().vm.$emit('success'); await waitForPromises(); - expect(mockAxios.history.get).toHaveLength(2); + expect(getMemberRoles).toHaveBeenCalledTimes(2); }); }); }); @@ -196,30 +184,25 @@ describe('ListMemberRoles', () => { expect(findCellByText(`${mockResponse[0].id}`).exists()).toBe(true); }); - const expectSortedByHeader = async (index) => { + const expectSortableColumn = async (fieldKey) => { createComponent({ groupId }, mountExtended); await waitForPromises(); - expect(findTableHeader(index).attributes('aria-sort')).toBe('none'); - findTableHeader(index).trigger('click'); - await nextTick(); - expect(findTableHeader(index).attributes('aria-sort')).toBe('ascending'); + const { fields } = findTable().vm.$attrs; - findTableHeader(index).trigger('click'); - await nextTick(); - expect(findTableHeader(index).attributes('aria-sort')).toBe('descending'); + expect(fields.find((field) => field.key === fieldKey)?.sortable).toBe(true); }; it('sorts columns by name', () => { - expectSortedByHeader(0); + expectSortableColumn('name'); }); it('sorts columns by ID', () => { - expectSortedByHeader(1); + expectSortableColumn('id'); }); it('sorts columns by base role', () => { - expectSortedByHeader(2); + expectSortableColumn('base_access_level'); }); it('shows list of standard permissions', async () => { @@ -261,17 +244,15 @@ describe('ListMemberRoles', () => { describe('when successful deletion', () => { beforeEach(async () => { - mockAxios.onDelete('/api/v4/groups/49/member_roles/1').replyOnce(HTTP_STATUS_NO_CONTENT); + deleteMemberRole.mockResolvedValue(); await clickRoleDelete(); }); it('delete the role', async () => { - expect(mockAxios.history.delete).toHaveLength(0); - findModal().vm.$emit('primary'); await waitForPromises(); - expect(mockAxios.history.delete).toHaveLength(1); + expect(deleteMemberRole).toHaveBeenNthCalledWith(1, '49', '1'); }); it('shows toast', async () => { @@ -282,20 +263,18 @@ describe('ListMemberRoles', () => { }); it('fetches roles', async () => { - expect(mockAxios.history.get).toHaveLength(1); + expect(getMemberRoles).toHaveBeenCalledTimes(1); findModal().vm.$emit('primary'); await waitForPromises(); - expect(mockAxios.history.get).toHaveLength(2); + expect(getMemberRoles).toHaveBeenCalledTimes(2); }); }); - describe('when unsuccessful fetch of roles', () => { + describe('when unsuccessful deletion of a role', () => { beforeEach(async () => { - mockAxios - .onDelete('/api/v4/groups/49/member_roles/1') - .replyOnce(HTTP_STATUS_INTERNAL_SERVER_ERROR); + deleteMemberRole.mockRejectedValue(new Error()); await clickRoleDelete(); }); -- GitLab From 14547150178fc5804078c937038b5742925178b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20Sanz=20Garc=C3=ADa?= <esanz-garcia@gitlab.com> Date: Fri, 8 Sep 2023 09:42:27 +0200 Subject: [PATCH 4/4] Remove unnecessary CSS rule --- config/application.rb | 1 - .../components/list_member_roles.vue | 4 +++- ee/app/assets/stylesheets/page_bundles/member_roles.scss | 8 -------- .../roles_and_permissions/index.html.haml | 1 - 4 files changed, 3 insertions(+), 11 deletions(-) delete mode 100644 ee/app/assets/stylesheets/page_bundles/member_roles.scss diff --git a/config/application.rb b/config/application.rb index d8d992ce97ccad..adf9f29db98c61 100644 --- a/config/application.rb +++ b/config/application.rb @@ -318,7 +318,6 @@ class Application < Rails::Application config.assets.precompile << "page_bundles/learn_gitlab.css" config.assets.precompile << "page_bundles/login.css" config.assets.precompile << "page_bundles/marketing_popover.css" - config.assets.precompile << "page_bundles/member_roles.css" config.assets.precompile << "page_bundles/members.css" config.assets.precompile << "page_bundles/merge_conflicts.css" config.assets.precompile << "page_bundles/merge_request_analytics.css" diff --git a/ee/app/assets/javascripts/roles_and_permissions/components/list_member_roles.vue b/ee/app/assets/javascripts/roles_and_permissions/components/list_member_roles.vue index cc5d0d39b76871..1759212e8e7028 100644 --- a/ee/app/assets/javascripts/roles_and_permissions/components/list_member_roles.vue +++ b/ee/app/assets/javascripts/roles_and_permissions/components/list_member_roles.vue @@ -203,7 +203,9 @@ export default { </template> <template #cell(permissions)="{ item }"> - <div class="gl-display-flex gl-flex-wrap gl-gap-3 justify-end-on-sm"> + <div + class="gl-display-flex gl-flex-wrap gl-gap-3 gl-justify-content-end gl-sm-justify-content-start" + > <gl-badge v-for="(permission, index) in listPermissions(item)" :key="index" diff --git a/ee/app/assets/stylesheets/page_bundles/member_roles.scss b/ee/app/assets/stylesheets/page_bundles/member_roles.scss deleted file mode 100644 index 18812880bb82a2..00000000000000 --- a/ee/app/assets/stylesheets/page_bundles/member_roles.scss +++ /dev/null @@ -1,8 +0,0 @@ -@import 'page_bundles/mixins_and_variables_and_functions'; - -// Remove this class once https://gitlab.com/gitlab-org/gitlab-ui/-/merge_requests/3634 is merged -.justify-end-on-sm { - @include gl-media-breakpoint-down(sm) { - @include gl-justify-content-end; - } -} diff --git a/ee/app/views/admin/application_settings/roles_and_permissions/index.html.haml b/ee/app/views/admin/application_settings/roles_and_permissions/index.html.haml index d7fcf76da43113..be8816bec6460a 100644 --- a/ee/app/views/admin/application_settings/roles_and_permissions/index.html.haml +++ b/ee/app/views/admin/application_settings/roles_and_permissions/index.html.haml @@ -1,5 +1,4 @@ - page_title _('Roles and Permissions') -- add_page_specific_style 'page_bundles/member_roles' #js-roles-and-permissions-self-managed = gl_loading_icon(css_class: 'gl-my-5', size: 'md') -- GitLab