From 3f9f17f584363141748e1cfefa2c1fcfd48acd58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C4=B1dvan=20Akca?= Date: Fri, 15 Mar 2024 12:38:11 +0300 Subject: [PATCH] refactor: rewrite useRole with RQ --- .../src/graphql/queries/get-role.ee.js | 17 -- .../src/graphql/queries/get-role.ee.test.js | 164 ------------------ .../backend/src/graphql/query-resolvers.js | 2 - packages/backend/src/graphql/schema.graphql | 1 - .../web/src/graphql/queries/get-role.ee.js | 18 -- .../web/src/helpers/computePermissions.ee.js | 10 +- packages/web/src/hooks/useRole.ee.js | 34 ++-- packages/web/src/pages/EditRole/index.ee.jsx | 16 +- 8 files changed, 35 insertions(+), 227 deletions(-) delete mode 100644 packages/backend/src/graphql/queries/get-role.ee.js delete mode 100644 packages/backend/src/graphql/queries/get-role.ee.test.js delete mode 100644 packages/web/src/graphql/queries/get-role.ee.js diff --git a/packages/backend/src/graphql/queries/get-role.ee.js b/packages/backend/src/graphql/queries/get-role.ee.js deleted file mode 100644 index 906535b3..00000000 --- a/packages/backend/src/graphql/queries/get-role.ee.js +++ /dev/null @@ -1,17 +0,0 @@ -import Role from '../../models/role.js'; - -const getRole = async (_parent, params, context) => { - context.currentUser.can('read', 'Role'); - - return await Role.query() - .leftJoinRelated({ - permissions: true, - }) - .withGraphFetched({ - permissions: true, - }) - .findById(params.id) - .throwIfNotFound(); -}; - -export default getRole; diff --git a/packages/backend/src/graphql/queries/get-role.ee.test.js b/packages/backend/src/graphql/queries/get-role.ee.test.js deleted file mode 100644 index 30a5df5c..00000000 --- a/packages/backend/src/graphql/queries/get-role.ee.test.js +++ /dev/null @@ -1,164 +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 Crypto from 'crypto'; -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 getRole query', () => { - let validRole, - invalidRoleId, - queryWithValidRole, - queryWithInvalidRole, - userWithPermissions, - userWithoutPermissions, - tokenWithPermissions, - tokenWithoutPermissions, - permissionOne, - permissionTwo; - - beforeEach(async () => { - validRole = await createRole(); - invalidRoleId = Crypto.randomUUID(); - - queryWithValidRole = ` - query { - getRole(id: "${validRole.id}") { - id - name - key - description - isAdmin - permissions { - id - action - subject - conditions - } - } - } - `; - - queryWithInvalidRole = ` - query { - getRole(id: "${invalidRoleId}") { - id - name - } - } - `; - - permissionOne = await createPermission({ - action: 'read', - subject: 'Role', - roleId: validRole.id, - }); - - permissionTwo = await createPermission({ - action: 'read', - subject: 'User', - roleId: validRole.id, - }); - - userWithPermissions = await createUser({ - roleId: validRole.id, - }); - - userWithoutPermissions = await createUser(); - - 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: queryWithValidRole }) - .expect(200); - - expect(response.body.errors).toBeDefined(); - expect(response.body.errors[0].message).toEqual('Not authorized!'); - }); - }); - - describe('and correct permissions', () => { - it('should return role data for a valid role id', async () => { - const response = await request(app) - .post('/graphql') - .set('Authorization', tokenWithPermissions) - .send({ query: queryWithValidRole }) - .expect(200); - - const expectedResponsePayload = { - data: { - getRole: { - description: validRole.description, - id: validRole.id, - isAdmin: validRole.key === 'admin', - key: validRole.key, - name: validRole.name, - permissions: [ - { - action: permissionOne.action, - conditions: permissionOne.conditions, - id: permissionOne.id, - subject: permissionOne.subject, - }, - { - action: permissionTwo.action, - conditions: permissionTwo.conditions, - id: permissionTwo.id, - subject: permissionTwo.subject, - }, - ], - }, - }, - }; - - expect(response.body).toEqual(expectedResponsePayload); - }); - - it('should return not found for invalid role id', async () => { - const response = await request(app) - .post('/graphql') - .set('Authorization', tokenWithPermissions) - .send({ query: queryWithInvalidRole }) - .expect(200); - - expect(response.body.errors).toBeDefined(); - expect(response.body.errors[0].message).toEqual('NotFoundError'); - }); - }); - }); - - 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: queryWithInvalidRole }) - .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 45db8770..6c86b9db 100644 --- a/packages/backend/src/graphql/query-resolvers.js +++ b/packages/backend/src/graphql/query-resolvers.js @@ -12,7 +12,6 @@ import getFlows from './queries/get-flows.js'; 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 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'; @@ -38,7 +37,6 @@ const queryResolvers = { getInvoices, getNotifications, getPermissionCatalog, - getRole, getSamlAuthProvider, getSamlAuthProviderRoleMappings, getStepWithTestExecutions, diff --git a/packages/backend/src/graphql/schema.graphql b/packages/backend/src/graphql/schema.graphql index 7dcb13fd..1ea98697 100644 --- a/packages/backend/src/graphql/schema.graphql +++ b/packages/backend/src/graphql/schema.graphql @@ -28,7 +28,6 @@ type Query { getConfig(keys: [String]): JSONObject getInvoices: [Invoice] getPermissionCatalog: PermissionCatalog - getRole(id: String!): Role getNotifications: [Notification] getSamlAuthProvider: SamlAuthProvider getSamlAuthProviderRoleMappings(id: String!): [SamlAuthProvidersRoleMapping] diff --git a/packages/web/src/graphql/queries/get-role.ee.js b/packages/web/src/graphql/queries/get-role.ee.js deleted file mode 100644 index e1cddc61..00000000 --- a/packages/web/src/graphql/queries/get-role.ee.js +++ /dev/null @@ -1,18 +0,0 @@ -import { gql } from '@apollo/client'; -export const GET_ROLE = gql` - query GetRole($id: String!) { - getRole(id: $id) { - id - key - name - description - isAdmin - permissions { - id - action - subject - conditions - } - } - } -`; diff --git a/packages/web/src/helpers/computePermissions.ee.js b/packages/web/src/helpers/computePermissions.ee.js index deded9ff..7c6ae20e 100644 --- a/packages/web/src/helpers/computePermissions.ee.js +++ b/packages/web/src/helpers/computePermissions.ee.js @@ -1,20 +1,21 @@ export function getRoleWithComputedPermissions(role) { if (!role) return {}; - const computedPermissions = role.permissions.reduce( + const computedPermissions = role.permissions?.reduce( (computedPermissions, permission) => ({ ...computedPermissions, [permission.subject]: { ...(computedPermissions[permission.subject] || {}), [permission.action]: { conditions: Object.fromEntries( - permission.conditions.map((condition) => [condition, true]) + permission.conditions.map((condition) => [condition, true]), ), value: true, }, }, }), - {} + {}, ); + return { ...role, computedPermissions, @@ -22,6 +23,7 @@ export function getRoleWithComputedPermissions(role) { } export function getPermissions(computedPermissions) { if (!computedPermissions) return []; + return Object.entries(computedPermissions).reduce( (permissions, computedPermissionEntry) => { const [subject, actionsWithConditions] = computedPermissionEntry; @@ -40,6 +42,6 @@ export function getPermissions(computedPermissions) { } return permissions; }, - [] + [], ); } diff --git a/packages/web/src/hooks/useRole.ee.js b/packages/web/src/hooks/useRole.ee.js index 1f2d86bf..a1f12904 100644 --- a/packages/web/src/hooks/useRole.ee.js +++ b/packages/web/src/hooks/useRole.ee.js @@ -1,19 +1,19 @@ -import * as React from 'react'; -import { useLazyQuery } from '@apollo/client'; -import { GET_ROLE } from 'graphql/queries/get-role.ee'; -export default function useRole(roleId) { - const [getRole, { data, loading }] = useLazyQuery(GET_ROLE); - React.useEffect(() => { - if (roleId) { - getRole({ - variables: { - id: roleId, - }, +import { useQuery } from '@tanstack/react-query'; + +import api from 'helpers/api'; + +export default function useRole({ roleId }) { + const query = useQuery({ + queryKey: ['role', roleId], + queryFn: async ({ signal }) => { + const { data } = await api.get(`/v1/admin/roles/${roleId}`, { + signal, }); - } - }, [roleId]); - return { - role: data?.getRole, - loading, - }; + + return data; + }, + enabled: !!roleId, + }); + + return query; } diff --git a/packages/web/src/pages/EditRole/index.ee.jsx b/packages/web/src/pages/EditRole/index.ee.jsx index e70cf8d6..e52ced94 100644 --- a/packages/web/src/pages/EditRole/index.ee.jsx +++ b/packages/web/src/pages/EditRole/index.ee.jsx @@ -6,6 +6,7 @@ import Stack from '@mui/material/Stack'; import useEnqueueSnackbar from 'hooks/useEnqueueSnackbar'; import * as React from 'react'; import { useNavigate, useParams } from 'react-router-dom'; + import Container from 'components/Container'; import Form from 'components/Form'; import PageTitle from 'components/PageTitle'; @@ -19,13 +20,16 @@ import { } from 'helpers/computePermissions.ee'; import useFormatMessage from 'hooks/useFormatMessage'; import useRole from 'hooks/useRole.ee'; + export default function EditRole() { const formatMessage = useFormatMessage(); const [updateRole, { loading }] = useMutation(UPDATE_ROLE); const navigate = useNavigate(); const { roleId } = useParams(); - const { role, loading: roleLoading } = useRole(roleId); + const { data, loading: isRoleLoading } = useRole({ roleId }); + const role = data?.data; const enqueueSnackbar = useEnqueueSnackbar(); + const handleRoleUpdate = async (roleData) => { try { const newPermissions = getPermissions(roleData.computedPermissions); @@ -39,18 +43,22 @@ export default function EditRole() { }, }, }); + enqueueSnackbar(formatMessage('editRole.successfullyUpdated'), { variant: 'success', SnackbarProps: { 'data-test': 'snackbar-edit-role-success', }, }); + navigate(URLS.ROLES); } catch (error) { throw new Error('Failed while updating!'); } }; + const roleWithComputedPermissions = getRoleWithComputedPermissions(role); + return ( @@ -66,13 +74,13 @@ export default function EditRole() { onSubmit={handleRoleUpdate} > - {roleLoading && ( + {isRoleLoading && ( <> )} - {!roleLoading && role && ( + {!isRoleLoading && role && ( <> {formatMessage('editRole.submit')}