From 9218091c339564aff8111fc725b92e0bf9171f41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C4=B1dvan=20Akca?= Date: Thu, 14 Mar 2024 16:33:08 +0300 Subject: [PATCH 1/2] refactor: rewrite useRoles with RQ --- .../web/src/components/RoleList/index.ee.jsx | 12 ++++++---- packages/web/src/hooks/useRoles.ee.js | 22 ++++++++++++------- .../RoleMappingsFieldsArray.jsx | 14 +++++++++--- .../Authentication/SamlConfiguration.jsx | 17 +++++++++++--- packages/web/src/pages/CreateUser/index.jsx | 12 ++++++++-- packages/web/src/pages/EditUser/index.jsx | 12 ++++++++-- 6 files changed, 67 insertions(+), 22 deletions(-) diff --git a/packages/web/src/components/RoleList/index.ee.jsx b/packages/web/src/components/RoleList/index.ee.jsx index 28306144..096b65e6 100644 --- a/packages/web/src/components/RoleList/index.ee.jsx +++ b/packages/web/src/components/RoleList/index.ee.jsx @@ -11,14 +11,18 @@ import Paper from '@mui/material/Paper'; import IconButton from '@mui/material/IconButton'; import Typography from '@mui/material/Typography'; import EditIcon from '@mui/icons-material/Edit'; + import DeleteRoleButton from 'components/DeleteRoleButton/index.ee'; import ListLoader from 'components/ListLoader'; import useFormatMessage from 'hooks/useFormatMessage'; import useRoles from 'hooks/useRoles.ee'; import * as URLS from 'config/urls'; + export default function RoleList() { const formatMessage = useFormatMessage(); - const { roles, loading } = useRoles(); + const { data, isLoading: isRolesLoading } = useRoles(); + const roles = data?.data; + return ( @@ -46,15 +50,15 @@ export default function RoleList() { - {loading && ( + {isRolesLoading && ( )} - {!loading && - roles.map((role) => ( + {!isRolesLoading && + roles?.map((role) => ( { + const { data } = await api.get('/v1/admin/roles', { + signal, + }); + + return data; + }, }); - return { - roles: data?.getRoles || [], - loading, - }; + + return query; } diff --git a/packages/web/src/pages/Authentication/RoleMappingsFieldsArray.jsx b/packages/web/src/pages/Authentication/RoleMappingsFieldsArray.jsx index 6eb61dda..be6c08fc 100644 --- a/packages/web/src/pages/Authentication/RoleMappingsFieldsArray.jsx +++ b/packages/web/src/pages/Authentication/RoleMappingsFieldsArray.jsx @@ -4,24 +4,32 @@ import Stack from '@mui/material/Stack'; import DeleteIcon from '@mui/icons-material/Delete'; import IconButton from '@mui/material/IconButton'; import Button from '@mui/material/Button'; +import { Divider, Typography } from '@mui/material'; + import useRoles from 'hooks/useRoles.ee'; import useFormatMessage from 'hooks/useFormatMessage'; import ControlledAutocomplete from 'components/ControlledAutocomplete'; import TextField from 'components/TextField'; -import { Divider, Typography } from '@mui/material'; + function generateRoleOptions(roles) { return roles?.map(({ name: label, id: value }) => ({ label, value })); } + function RoleMappingsFieldArray() { const formatMessage = useFormatMessage(); const { control } = useFormContext(); - const { roles, loading: rolesLoading } = useRoles(); + const { data, isLoading: isRolesLoading } = useRoles(); + const roles = data?.data; + const { fields, append, remove } = useFieldArray({ control, name: 'roleMappings', }); + const handleAppendMapping = () => append({ roleId: '', remoteRoleName: '' }); + const handleRemoveMapping = (index) => () => remove(index); + return ( <> {fields.length === 0 && ( @@ -60,7 +68,7 @@ function RoleMappingsFieldArray() { label={formatMessage('roleMappingsForm.role')} /> )} - loading={rolesLoading} + loading={isRolesLoading} required /> diff --git a/packages/web/src/pages/Authentication/SamlConfiguration.jsx b/packages/web/src/pages/Authentication/SamlConfiguration.jsx index b187a328..adffe002 100644 --- a/packages/web/src/pages/Authentication/SamlConfiguration.jsx +++ b/packages/web/src/pages/Authentication/SamlConfiguration.jsx @@ -2,8 +2,9 @@ import { useMutation } from '@apollo/client'; import LoadingButton from '@mui/lab/LoadingButton'; import Stack from '@mui/material/Stack'; import MuiTextField from '@mui/material/TextField'; -import useEnqueueSnackbar from 'hooks/useEnqueueSnackbar'; import * as React from 'react'; + +import useEnqueueSnackbar from 'hooks/useEnqueueSnackbar'; import ControlledAutocomplete from 'components/ControlledAutocomplete'; import Form from 'components/Form'; import Switch from 'components/Switch'; @@ -11,6 +12,7 @@ import TextField from 'components/TextField'; import { UPSERT_SAML_AUTH_PROVIDER } from 'graphql/mutations/upsert-saml-auth-provider'; import useFormatMessage from 'hooks/useFormatMessage'; import useRoles from 'hooks/useRoles.ee'; + const defaultValues = { active: false, name: '', @@ -24,16 +26,21 @@ const defaultValues = { roleAttributeName: '', defaultRoleId: '', }; + function generateRoleOptions(roles) { return roles?.map(({ name: label, id: value }) => ({ label, value })); } + function SamlConfiguration({ provider, providerLoading, refetchProvider }) { const formatMessage = useFormatMessage(); - const { roles, loading: rolesLoading } = useRoles(); + const { data, loading: isRolesLoading } = useRoles(); + const roles = data?.data; const enqueueSnackbar = useEnqueueSnackbar(); + const [upsertSamlAuthProvider, { loading }] = useMutation( UPSERT_SAML_AUTH_PROVIDER, ); + const handleProviderUpdate = async (providerDataToUpdate) => { try { const { @@ -66,9 +73,11 @@ function SamlConfiguration({ provider, providerLoading, refetchProvider }) { }, }, }); + if (!provider?.id) { await refetchProvider(); } + enqueueSnackbar(formatMessage('authenticationForm.successfullySaved'), { variant: 'success', SnackbarProps: { @@ -79,9 +88,11 @@ function SamlConfiguration({ provider, providerLoading, refetchProvider }) { throw new Error('Failed while saving!'); } }; + if (providerLoading) { return null; } + return (
)} - loading={rolesLoading} + loading={isRolesLoading} /> ({ label, value })); } + export default function CreateUser() { const navigate = useNavigate(); const formatMessage = useFormatMessage(); const [createUser, { loading }] = useMutation(CREATE_USER); - const { roles, loading: rolesLoading } = useRoles(); + const { data, loading: isRolesLoading } = useRoles(); + const roles = data?.data; const enqueueSnackbar = useEnqueueSnackbar(); + const handleUserCreation = async (userData) => { try { await createUser({ @@ -39,6 +44,7 @@ export default function CreateUser() { }, }, }); + enqueueSnackbar(formatMessage('createUser.successfullyCreated'), { variant: 'success', persist: true, @@ -46,11 +52,13 @@ export default function CreateUser() { 'data-test': 'snackbar-create-user-success', }, }); + navigate(URLS.USERS); } catch (error) { throw new Error('Failed while creating!'); } }; + return ( @@ -101,7 +109,7 @@ export default function CreateUser() { label={formatMessage('userForm.role')} /> )} - loading={rolesLoading} + loading={isRolesLoading} /> diff --git a/packages/web/src/pages/EditUser/index.jsx b/packages/web/src/pages/EditUser/index.jsx index 03f3abed..c13d04b4 100644 --- a/packages/web/src/pages/EditUser/index.jsx +++ b/packages/web/src/pages/EditUser/index.jsx @@ -7,6 +7,7 @@ import MuiTextField from '@mui/material/TextField'; import useEnqueueSnackbar from 'hooks/useEnqueueSnackbar'; import * as React from 'react'; import { useNavigate, useParams } from 'react-router-dom'; + import Can from 'components/Can'; import Container from 'components/Container'; import ControlledAutocomplete from 'components/ControlledAutocomplete'; @@ -18,17 +19,21 @@ import { UPDATE_USER } from 'graphql/mutations/update-user.ee'; import useFormatMessage from 'hooks/useFormatMessage'; import useRoles from 'hooks/useRoles.ee'; import useUser from 'hooks/useUser'; + function generateRoleOptions(roles) { return roles?.map(({ name: label, id: value }) => ({ label, value })); } + export default function EditUser() { const formatMessage = useFormatMessage(); const [updateUser, { loading }] = useMutation(UPDATE_USER); const { userId } = useParams(); const { user, loading: userLoading } = useUser(userId); - const { roles, loading: rolesLoading } = useRoles(); + const { data, loading: isRolesLoading } = useRoles(); + const roles = data?.data; const enqueueSnackbar = useEnqueueSnackbar(); const navigate = useNavigate(); + const handleUserUpdate = async (userDataToUpdate) => { try { await updateUser({ @@ -43,6 +48,7 @@ export default function EditUser() { }, }, }); + enqueueSnackbar(formatMessage('editUser.successfullyUpdated'), { variant: 'success', SnackbarProps: { @@ -50,11 +56,13 @@ export default function EditUser() { persist: true, }, }); + navigate(URLS.USERS); } catch (error) { throw new Error('Failed while updating!'); } }; + return ( @@ -106,7 +114,7 @@ export default function EditUser() { label={formatMessage('userForm.role')} /> )} - loading={rolesLoading} + loading={isRolesLoading} /> From ecc9379d7e2ee4eaf20df85f709bb435e9f5df4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C4=B1dvan=20Akca?= Date: Thu, 14 Mar 2024 16:35:51 +0300 Subject: [PATCH 2/2] refactor: remove get-roles GQL query --- .../src/graphql/queries/get-roles.ee.js | 9 -- .../src/graphql/queries/get-roles.ee.test.js | 134 ------------------ .../backend/src/graphql/query-resolvers.js | 2 - packages/backend/src/graphql/schema.graphql | 1 - .../components/DeleteRoleButton/index.ee.jsx | 16 ++- .../web/src/graphql/queries/get-roles.ee.js | 12 -- packages/web/src/hooks/useRoles.ee.js | 1 + 7 files changed, 12 insertions(+), 163 deletions(-) delete mode 100644 packages/backend/src/graphql/queries/get-roles.ee.js delete mode 100644 packages/backend/src/graphql/queries/get-roles.ee.test.js delete mode 100644 packages/web/src/graphql/queries/get-roles.ee.js diff --git a/packages/backend/src/graphql/queries/get-roles.ee.js b/packages/backend/src/graphql/queries/get-roles.ee.js deleted file mode 100644 index 900af081..00000000 --- a/packages/backend/src/graphql/queries/get-roles.ee.js +++ /dev/null @@ -1,9 +0,0 @@ -import Role from '../../models/role.js'; - -const getRoles = async (_parent, params, context) => { - context.currentUser.can('read', 'Role'); - - return await Role.query().orderBy('name'); -}; - -export default getRoles; diff --git a/packages/backend/src/graphql/queries/get-roles.ee.test.js b/packages/backend/src/graphql/queries/get-roles.ee.test.js deleted file mode 100644 index 143ecc6c..00000000 --- a/packages/backend/src/graphql/queries/get-roles.ee.test.js +++ /dev/null @@ -1,134 +0,0 @@ -import { vi, describe, it, expect, beforeEach } from 'vitest'; -import request from 'supertest'; -import app from '../../app'; -import createAuthTokenByUserId from '../../helpers/create-auth-token-by-user-id'; -import { createRole } from '../../../test/factories/role'; -import { createPermission } from '../../../test/factories/permission'; -import { createUser } from '../../../test/factories/user'; -import * as license from '../../helpers/license.ee'; - -describe('graphQL getRoles query', () => { - let currentUserRole, - roleOne, - roleSecond, - query, - userWithPermissions, - userWithoutPermissions, - tokenWithPermissions, - tokenWithoutPermissions; - - beforeEach(async () => { - currentUserRole = await createRole({ name: 'Current user role' }); - roleOne = await createRole({ name: 'Role one' }); - roleSecond = await createRole({ name: 'Role second' }); - - query = ` - query { - getRoles { - id - key - name - description - isAdmin - } - } - `; - - await createPermission({ - action: 'read', - subject: 'Role', - roleId: currentUserRole.id, - }); - - userWithPermissions = await createUser({ - roleId: currentUserRole.id, - }); - - userWithoutPermissions = await createUser({ - roleId: roleOne.id, - }); - - tokenWithPermissions = createAuthTokenByUserId(userWithPermissions.id); - tokenWithoutPermissions = createAuthTokenByUserId( - userWithoutPermissions.id - ); - }); - - describe('and with valid license', () => { - beforeEach(async () => { - vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true); - }); - - describe('and without permissions', () => { - it('should throw not authorized error', async () => { - const response = await request(app) - .post('/graphql') - .set('Authorization', tokenWithoutPermissions) - .send({ query }) - .expect(200); - - expect(response.body.errors).toBeDefined(); - expect(response.body.errors[0].message).toEqual('Not authorized!'); - }); - }); - - describe('and correct permissions', () => { - it('should return roles data', async () => { - const response = await request(app) - .post('/graphql') - .set('Authorization', tokenWithPermissions) - .send({ query }) - .expect(200); - - const expectedResponsePayload = { - data: { - getRoles: [ - { - description: currentUserRole.description, - id: currentUserRole.id, - isAdmin: currentUserRole.key === 'admin', - key: currentUserRole.key, - name: currentUserRole.name, - }, - { - description: roleOne.description, - id: roleOne.id, - isAdmin: roleOne.key === 'admin', - key: roleOne.key, - name: roleOne.name, - }, - { - description: roleSecond.description, - id: roleSecond.id, - isAdmin: roleSecond.key === 'admin', - key: roleSecond.key, - name: roleSecond.name, - }, - ], - }, - }; - - expect(response.body).toEqual(expectedResponsePayload); - }); - }); - }); - - describe('and without valid license', () => { - beforeEach(async () => { - vi.spyOn(license, 'hasValidLicense').mockResolvedValue(false); - }); - - describe('and correct permissions', () => { - it('should throw not authorized error', async () => { - const response = await request(app) - .post('/graphql') - .set('Authorization', tokenWithPermissions) - .send({ query }) - .expect(200); - - expect(response.body.errors).toBeDefined(); - expect(response.body.errors[0].message).toEqual('Not authorized!'); - }); - }); - }); -}); diff --git a/packages/backend/src/graphql/query-resolvers.js b/packages/backend/src/graphql/query-resolvers.js index 60dadee9..45db8770 100644 --- a/packages/backend/src/graphql/query-resolvers.js +++ b/packages/backend/src/graphql/query-resolvers.js @@ -13,7 +13,6 @@ import getInvoices from './queries/get-invoices.ee.js'; import getNotifications from './queries/get-notifications.js'; import getPermissionCatalog from './queries/get-permission-catalog.ee.js'; import getRole from './queries/get-role.ee.js'; -import getRoles from './queries/get-roles.ee.js'; import getSamlAuthProviderRoleMappings from './queries/get-saml-auth-provider-role-mappings.ee.js'; import getSamlAuthProvider from './queries/get-saml-auth-provider.ee.js'; import getStepWithTestExecutions from './queries/get-step-with-test-executions.js'; @@ -40,7 +39,6 @@ const queryResolvers = { getNotifications, getPermissionCatalog, getRole, - getRoles, getSamlAuthProvider, getSamlAuthProviderRoleMappings, getStepWithTestExecutions, diff --git a/packages/backend/src/graphql/schema.graphql b/packages/backend/src/graphql/schema.graphql index da091e00..7dcb13fd 100644 --- a/packages/backend/src/graphql/schema.graphql +++ b/packages/backend/src/graphql/schema.graphql @@ -29,7 +29,6 @@ type Query { getInvoices: [Invoice] getPermissionCatalog: PermissionCatalog getRole(id: String!): Role - getRoles: [Role] getNotifications: [Notification] getSamlAuthProvider: SamlAuthProvider getSamlAuthProviderRoleMappings(id: String!): [SamlAuthProvidersRoleMapping] diff --git a/packages/web/src/components/DeleteRoleButton/index.ee.jsx b/packages/web/src/components/DeleteRoleButton/index.ee.jsx index 69dec7e0..e55ee021 100644 --- a/packages/web/src/components/DeleteRoleButton/index.ee.jsx +++ b/packages/web/src/components/DeleteRoleButton/index.ee.jsx @@ -4,6 +4,8 @@ import DeleteIcon from '@mui/icons-material/Delete'; import IconButton from '@mui/material/IconButton'; import useEnqueueSnackbar from 'hooks/useEnqueueSnackbar'; import * as React from 'react'; +import { useQueryClient } from '@tanstack/react-query'; + import Can from 'components/Can'; import ConfirmationDialog from 'components/ConfirmationDialog'; import { DELETE_ROLE } from 'graphql/mutations/delete-role.ee'; @@ -12,15 +14,18 @@ import useFormatMessage from 'hooks/useFormatMessage'; function DeleteRoleButton(props) { const { disabled, roleId } = props; const [showConfirmation, setShowConfirmation] = React.useState(false); - const [deleteRole] = useMutation(DELETE_ROLE, { - variables: { input: { id: roleId } }, - refetchQueries: ['GetRoles'], - }); const formatMessage = useFormatMessage(); const enqueueSnackbar = useEnqueueSnackbar(); + const queryClient = useQueryClient(); + + const [deleteRole] = useMutation(DELETE_ROLE, { + variables: { input: { id: roleId } }, + }); + const handleConfirm = React.useCallback(async () => { try { await deleteRole(); + queryClient.invalidateQueries({ queryKey: ['roles'] }); setShowConfirmation(false); enqueueSnackbar(formatMessage('deleteRoleButton.successfullyDeleted'), { variant: 'success', @@ -31,7 +36,8 @@ function DeleteRoleButton(props) { } catch (error) { throw new Error('Failed while deleting!'); } - }, [deleteRole]); + }, [deleteRole, enqueueSnackbar, formatMessage, queryClient]); + return ( <> diff --git a/packages/web/src/graphql/queries/get-roles.ee.js b/packages/web/src/graphql/queries/get-roles.ee.js deleted file mode 100644 index e6846ea9..00000000 --- a/packages/web/src/graphql/queries/get-roles.ee.js +++ /dev/null @@ -1,12 +0,0 @@ -import { gql } from '@apollo/client'; -export const GET_ROLES = gql` - query GetRoles { - getRoles { - id - key - name - description - isAdmin - } - } -`; diff --git a/packages/web/src/hooks/useRoles.ee.js b/packages/web/src/hooks/useRoles.ee.js index 0d4923cc..835f8170 100644 --- a/packages/web/src/hooks/useRoles.ee.js +++ b/packages/web/src/hooks/useRoles.ee.js @@ -3,6 +3,7 @@ import api from 'helpers/api'; export default function useRoles() { const query = useQuery({ + staleTime: 0, queryKey: ['roles'], queryFn: async ({ signal }) => { const { data } = await api.get('/v1/admin/roles', {