Addressed review comments

Signed-off-by: Yi Cai <yicai@redhat.com>
janus-idp · May 31, 2024 · fa31127 · fa31127
1 parent 2a31062
commit fa31127
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 17 deletions.
diff --git a/plugins/rbac/src/components/RolesList/RolesList.tsx b/plugins/rbac/src/components/RolesList/RolesList.tsx
@@ -2,8 +2,7 @@ import React from 'react';
 
 import { Table, WarningPanel } from '@backstage/core-components';
 
-import { Box, makeStyles } from '@material-ui/core';
-import { Alert } from '@material-ui/lab';
+import { makeStyles } from '@material-ui/core';
 
 import { useLocationToast } from '../../hooks/useLocationToast';
 import { useRoles } from '../../hooks/useRoles';
@@ -45,27 +44,42 @@ export const RolesList = () => {
     setRoles(searchResults.length);
   };
 
+  const getErrorWarning = () => {
+    const errorTitleBase = 'Something went wrong while fetching the';
+    const errorWarningArr = [
+      { message: error?.rolesError, title: `${errorTitleBase} roles` },
+      {
+        message: error?.policiesError,
+        title: `${errorTitleBase} permission policies`,
+      },
+      {
+        message: error?.roleConditionError,
+        title: `${errorTitleBase} role conditions`,
+      },
+    ];
+
+    return (
+      errorWarningArr.find(({ message }) => message) || {
+        message: '',
+        title: '',
+      }
+    );
+  };
+
+  const errorWarning = getErrorWarning();
+
   return (
     <>
       <SnackbarAlert toastMessage={toastMessage} onAlertClose={onAlertClose} />
-      {error?.roleConditionError && (
-        <Box>
-          <Alert severity="warning">{error?.roleConditionError}</Alert>
-        </Box>
-      )}
       <RolesListToolbar
         createRoleAllowed={createRoleAllowed}
         createRoleLoading={createRoleLoading}
       />
-      {(error?.rolesError || error?.policiesError) && (
+      {errorWarning.message && (
         <div style={{ paddingBottom: '16px' }}>
           <WarningPanel
-            message={error.rolesError || error.policiesError}
-            title={
-              error.rolesError
-                ? 'Something went wrong while fetching the roles'
-                : 'Something went wrong while fetching the permission policies'
-            }
+            message={errorWarning.message}
+            title={errorWarning.title}
             severity="error"
           />
         </div>

diff --git a/plugins/rbac/src/hooks/useRoles.ts b/plugins/rbac/src/hooks/useRoles.ts
@@ -38,7 +38,7 @@ export const useRoles = (
   const [newRoles, setNewRoles] = React.useState<
     RoleWithConditionalPoliciesCount[]
   >([]);
-  const [fetchRoleConditionsError, setFetchRoleConditionsError] =
+  const [roleConditionError, setRoleConditionError] =
     React.useState<string>('');
   const {
     value: roles,
@@ -102,7 +102,7 @@ export const useRoles = (
               : 0,
           };
         } catch (error) {
-          setFetchRoleConditionsError(
+          setRoleConditionError(
             `Error fetching role conditions for role ${role.name}, please try again later.`,
           );
           return {
@@ -186,7 +186,7 @@ export const useRoles = (
         (typeof policies === 'object'
           ? (policies as any as Response)?.statusText
           : '')) as string,
-      roleConditionError: fetchRoleConditionsError,
+      roleConditionError,
     },
     createRoleLoading,
     createRoleAllowed,