diff --git a/plugins/rbac/src/api/RBACBackendClient.ts b/plugins/rbac/src/api/RBACBackendClient.ts index b185a88e69..6e818391e3 100644 --- a/plugins/rbac/src/api/RBACBackendClient.ts +++ b/plugins/rbac/src/api/RBACBackendClient.ts @@ -16,7 +16,7 @@ import { getKindNamespaceName } from '../utils/rbac-utils'; // @public export type RBACAPI = { getUserAuthorization: () => Promise<{ status: string }>; - getRoles: () => Promise; + getRoles: () => Promise; getPolicies: () => Promise; getAssociatedPolicies: ( entityReference: string, @@ -78,6 +78,11 @@ export class RBACBackendClient implements RBACAPI { ...(idToken && { Authorization: `Bearer ${idToken}` }), }, }); + + if (jsonResponse.status !== 200 && jsonResponse.status !== 204) { + return jsonResponse; + } + return jsonResponse.json(); } diff --git a/plugins/rbac/src/components/RbacPage.test.tsx b/plugins/rbac/src/components/RbacPage.test.tsx index 1d928bb057..3c4bdedd11 100644 --- a/plugins/rbac/src/components/RbacPage.test.tsx +++ b/plugins/rbac/src/components/RbacPage.test.tsx @@ -37,7 +37,11 @@ describe('RbacPage', () => { mockUseRoles.mockReturnValue({ loading: true, data: [], - retry: jest.fn(), + error: { + rolesError: '', + policiesError: '', + }, + retry: { roleRetry: jest.fn(), policiesRetry: jest.fn() }, createRoleAllowed: false, createRoleLoading: false, }); diff --git a/plugins/rbac/src/components/RolesList/RolesList.test.tsx b/plugins/rbac/src/components/RolesList/RolesList.test.tsx index cd0dc0e2c0..a849bf87e8 100644 --- a/plugins/rbac/src/components/RolesList/RolesList.test.tsx +++ b/plugins/rbac/src/components/RolesList/RolesList.test.tsx @@ -63,7 +63,11 @@ describe('RolesList', () => { mockUseRoles.mockReturnValue({ loading: false, data: useRolesMockData, - retry: jest.fn(), + error: { + rolesError: '', + policiesError: '', + }, + retry: { roleRetry: jest.fn(), policiesRetry: jest.fn() }, createRoleAllowed: false, createRoleLoading: false, }); @@ -80,12 +84,17 @@ describe('RolesList', () => { mockUseRoles.mockReturnValue({ loading: false, data: [], - retry: jest.fn(), + error: { + rolesError: '', + policiesError: '', + }, + retry: { roleRetry: jest.fn(), policiesRetry: jest.fn() }, createRoleAllowed: false, createRoleLoading: false, }); - const { getByTestId } = await renderInTestApp(); + const { getByTestId, queryByText } = await renderInTestApp(); expect(getByTestId('roles-table-empty')).not.toBeNull(); + expect(queryByText('Something went wrong')).not.toBeInTheDocument(); }); it('should show delete icon if user is authorized to delete roles', async () => { @@ -96,7 +105,11 @@ describe('RolesList', () => { mockUseRoles.mockReturnValue({ loading: false, data: useRolesMockData, - retry: jest.fn(), + error: { + rolesError: '', + policiesError: '', + }, + retry: { roleRetry: jest.fn(), policiesRetry: jest.fn() }, createRoleAllowed: false, createRoleLoading: false, }); @@ -128,7 +141,11 @@ describe('RolesList', () => { }, }, ], - retry: jest.fn(), + error: { + rolesError: '', + policiesError: '', + }, + retry: { roleRetry: jest.fn(), policiesRetry: jest.fn() }, createRoleAllowed: false, createRoleLoading: false, }); @@ -160,7 +177,11 @@ describe('RolesList', () => { }, }, ], - retry: jest.fn(), + error: { + rolesError: '', + policiesError: '', + }, + retry: { roleRetry: jest.fn(), policiesRetry: jest.fn() }, createRoleAllowed: true, createRoleLoading: false, }); @@ -175,7 +196,11 @@ describe('RolesList', () => { mockUseRoles.mockReturnValue({ loading: false, data: useRolesMockData, - retry: jest.fn(), + error: { + rolesError: '', + policiesError: '', + }, + retry: { roleRetry: jest.fn(), policiesRetry: jest.fn() }, createRoleAllowed: false, createRoleLoading: false, }); @@ -192,7 +217,11 @@ describe('RolesList', () => { mockUseRoles.mockReturnValue({ loading: false, data: useRolesMockData, - retry: jest.fn(), + error: { + rolesError: '', + policiesError: '', + }, + retry: { roleRetry: jest.fn(), policiesRetry: jest.fn() }, createRoleAllowed: true, createRoleLoading: false, }); @@ -209,7 +238,11 @@ describe('RolesList', () => { mockUseRoles.mockReturnValue({ loading: false, data: useRolesMockData, - retry: jest.fn(), + error: { + rolesError: '', + policiesError: '', + }, + retry: { roleRetry: jest.fn(), policiesRetry: jest.fn() }, createRoleAllowed: false, createRoleLoading: false, }); @@ -217,4 +250,23 @@ describe('RolesList', () => { expect(getByTestId('create-role-warning')).not.toBeNull(); }); + + it('should show error message when there is an error fetching the roles', async () => { + RequirePermissionMock.mockImplementation(props => <>{props.children}); + mockUsePermission.mockReturnValue({ loading: false, allowed: true }); + mockUseRoles.mockReturnValue({ + loading: true, + data: [], + error: { + rolesError: 'Something went wrong', + policiesError: '', + }, + retry: { roleRetry: jest.fn(), policiesRetry: jest.fn() }, + createRoleAllowed: false, + createRoleLoading: false, + }); + + const { queryByText } = await renderInTestApp(); + expect(queryByText('Something went wrong')).toBeInTheDocument(); + }); }); diff --git a/plugins/rbac/src/components/RolesList/RolesList.tsx b/plugins/rbac/src/components/RolesList/RolesList.tsx index a26625052b..8ce1bfc719 100644 --- a/plugins/rbac/src/components/RolesList/RolesList.tsx +++ b/plugins/rbac/src/components/RolesList/RolesList.tsx @@ -1,6 +1,6 @@ import React from 'react'; -import { Table } from '@backstage/core-components'; +import { Table, WarningPanel } from '@backstage/core-components'; import { makeStyles } from '@material-ui/core'; @@ -27,12 +27,13 @@ export const RolesList = () => { const [roles, setRoles] = React.useState(); const classes = useStyles(); - const { loading, data, retry, createRoleAllowed, createRoleLoading } = + const { loading, data, retry, createRoleAllowed, createRoleLoading, error } = useRoles(); const closeDialog = () => { setOpenDialog(false); - retry(); + retry.roleRetry(); + retry.policiesRetry(); }; const onAlertClose = () => { @@ -49,6 +50,19 @@ export const RolesList = () => { createRoleAllowed={createRoleAllowed} createRoleLoading={createRoleLoading} /> + {(error?.rolesError || error?.policiesError) && ( +
+ +
+ )} void; + error: { + rolesError: string; + policiesError: string; + }; + retry: { roleRetry: () => void; policiesRetry: () => void }; } => { const rbacApi = useApi(rbacApiRef); const { - loading: rolesLoading, value: roles, retry: roleRetry, + error: rolesError, } = useAsyncRetry(async () => await rbacApi.getRoles()); - const { loading: policiesLoading, value: policies } = useAsyncRetry( - async () => await rbacApi.getPolicies(), - [], - ); + const { + value: policies, + retry: policiesRetry, + error: policiesError, + } = useAsyncRetry(async () => await rbacApi.getPolicies(), []); const deletePermissionResult = usePermission({ permission: policyEntityDeletePermission, @@ -67,7 +72,7 @@ export const useRoles = ( }); const data: RolesData[] = React.useMemo( () => - roles && roles?.length > 0 + Array.isArray(roles) && roles?.length > 0 ? roles.reduce((acc: RolesData[], role: Role) => { const permissions = getPermissions( role.name, @@ -107,10 +112,12 @@ export const useRoles = ( catalogEntityReadPermissionResult, ], ); - const loading = rolesLoading && policiesLoading; + const loading = !rolesError && !policiesError && !roles && !policies; + useInterval( () => { roleRetry(); + policiesRetry(); }, loading ? null : pollInterval || 10000, ); @@ -118,8 +125,18 @@ export const useRoles = ( return { loading, data, + error: { + rolesError: (rolesError?.message || + (typeof roles === 'object' + ? (roles as any as Response)?.statusText + : '')) as string, + policiesError: (policiesError?.message || + (typeof policies === 'object' + ? (policies as any as Response)?.statusText + : '')) as string, + }, createRoleLoading, createRoleAllowed, - retry: roleRetry, + retry: { roleRetry, policiesRetry }, }; };