fix(scorecard): improve error handling in scorecard (#1470)

* fix(scorecard): improve error handling in scorecard

* increase missing permission title size

* addressed review comments

* addressed review comments

* fix to match new threshold on metric error

* fix conflicts

* fix pointer

* fix pointer

* fix pointer

Author: Eswaraiahsapram

workspaces/scorecard/plugins/scorecard/README.md

@@ -24,6 +24,30 @@ To install the Scorecard plugin, run the following command:
 yarn workspace app add @red-hat-developer-hub/backstage-plugin-scorecard
 ```
 
+**Note**
+
+### Permission Framework Support
+
+The Scorecard plugin has support for the permission framework.
+
+- When [RBAC permission](https://github.com/backstage/community-plugins/tree/main/workspaces/rbac/plugins/rbac-backend#installation) framework is enabled, for non-admin users to access scorecard UI, the role associated with your user should have the following permission policies associated with it. Add the following in your permission policies configuration file named `rbac-policy.csv`:
+
+```CSV
+p, role:default/team_a, scorecard.metric.read, read, allow
+
+g, user:default/<your-user-name>, role:default/team_a
+```
+
+You can specify the path to this configuration file in your application configuration:
+
+```yaml
+permission:
+  enabled: true
+  rbac:
+    policies-csv-file: /some/path/rbac-policy.csv
+    policyFileReload: true
+```
+
 ### Configuration
 
 1. Add the Scorecard page to you Entity overview page by modifying `packages/app/src/components/catalog/EntityPage.tsx`:

workspaces/scorecard/plugins/scorecard/package.json

@@ -39,6 +39,7 @@
     "@backstage/core-components": "^0.17.4",
     "@backstage/core-plugin-api": "^1.10.9",
     "@backstage/plugin-catalog-react": "^1.19.1",
+    "@backstage/plugin-permission-react": "^0.4.36",
     "@backstage/theme": "^0.6.7",
     "@mui/icons-material": "5.18.0",
     "@mui/material": "5.18.0",

…mponents/Scorecard/NoScorecardsState.tsx …/components/Common/NoScorecardsState.tsxworkspaces/scorecard/plugins/scorecard/src/components/Scorecard/NoScorecardsState.tsx renamed to workspaces/scorecard/plugins/scorecard/src/components/Common/NoScorecardsState.tsx workspaces/scorecard/plugins/scorecard/src/components/Scorecard/NoScorecardsState.tsx renamed to workspaces/scorecard/plugins/scorecard/src/components/Common/NoScorecardsState.tsx

@@ -0,0 +1,97 @@
+/*
+ * Copyright Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import Box from '@mui/material/Box';
+import Grid from '@mui/material/Grid';
+import Button from '@mui/material/Button';
+import Typography from '@mui/material/Typography';
+import OpenInNewIcon from '@mui/icons-material/OpenInNew';
+
+import permissionRequiredSvg from '../../images/permission-required.svg';
+
+const PermissionRequiredState = () => {
+  return (
+    <Box sx={{ p: 4, height: '100%', maxWidth: '1592px', margin: 'auto' }}>
+      <Grid
+        container
+        spacing={4}
+        alignItems="center"
+        justifyContent="center"
+        height="100%"
+      >
+        <Grid item xs={12} md={6} sx={{ textAlign: 'left' }}>
+          <Typography
+            sx={theme => ({
+              fontSize: '2.5rem',
+              fontWeight: 400,
+              color: theme.palette.text.primary,
+              mb: 2,
+            })}
+          >
+            Missing permission
+          </Typography>
+
+          <Typography
+            sx={theme => ({
+              fontSize: '1rem',
+              color: theme.palette.text.secondary,
+              mb: 3,
+              lineHeight: 1.5,
+            })}
+          >
+            To view Scorecard plugin, contact your administrator to give the{' '}
+            <Typography
+              component="span"
+              sx={theme => ({
+                fontWeight: 'bold',
+                color: theme.palette.text.primary,
+              })}
+            >
+              scorecard.metric.read
+            </Typography>{' '}
+            permission.
+          </Typography>
+
+          <Button
+            variant="outlined"
+            target="_blank"
+            href="https://github.com/redhat-developer/rhdh-plugins/blob/main/workspaces/scorecard/plugins/scorecard/README.md#permission-framework-support"
+            sx={theme => ({
+              color: theme.palette.primary.main,
+            })}
+          >
+            Read more &nbsp; <OpenInNewIcon />
+          </Button>
+        </Grid>
+
+        <Grid item xs={12} md={6} sx={{ textAlign: 'right' }}>
+          <Box
+            component="img"
+            src={permissionRequiredSvg}
+            alt="No scorecards"
+            sx={{
+              width: '100%',
+              maxWidth: '600px',
+              height: 'auto',
+            }}
+          />
+        </Grid>
+      </Grid>
+    </Box>
+  );
+};
+
+export default PermissionRequiredState;

workspaces/scorecard/plugins/scorecard/src/components/Common/PermissionRequiredState.tsx

@@ -0,0 +1,137 @@
+/*
+ * Copyright Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { render, screen } from '@testing-library/react';
+import { ThemeProvider, createTheme } from '@mui/material/styles';
+import { BrowserRouter } from 'react-router-dom';
+
+import PermissionRequiredState from '../PermissionRequiredState';
+
+// Mock the SVG import
+jest.mock(
+  '../../../images/permission-required.svg',
+  () => 'mocked-permission-required.svg',
+);
+
+// Mock the EmptyState component from Backstage
+jest.mock('@backstage/core-components', () => ({
+  EmptyState: function MockEmptyState(props: any) {
+    const React = require('react');
+    return React.createElement('div', { 'data-testid': 'empty-state' }, [
+      React.createElement(
+        'div',
+        { 'data-testid': 'empty-state-title', key: 'title' },
+        props.title,
+      ),
+      React.createElement(
+        'div',
+        { 'data-testid': 'empty-state-description', key: 'description' },
+        props.description,
+      ),
+      React.createElement(
+        'div',
+        { 'data-testid': 'empty-state-missing', key: 'missing' },
+        props.missing && props.missing.customImage,
+      ),
+      React.createElement(
+        'div',
+        { 'data-testid': 'empty-state-action', key: 'action' },
+        props.action,
+      ),
+    ]);
+  },
+}));
+
+// Mock the OpenInNew icon
+jest.mock('@mui/icons-material/OpenInNew', () => {
+  return function MockOpenInNewIcon() {
+    const React = require('react');
+    return React.createElement(
+      'span',
+      { 'data-testid': 'OpenInNewIcon' },
+      'OpenInNew',
+    );
+  };
+});
+
+const TestWrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => {
+  const theme = createTheme();
+  return (
+    <BrowserRouter>
+      <ThemeProvider theme={theme}>{children}</ThemeProvider>
+    </BrowserRouter>
+  );
+};
+
+const renderWithProviders = (component: React.ReactElement) => {
+  return render(component, { wrapper: TestWrapper });
+};
+
+describe('PermissionRequiredState Component', () => {
+  it('should render the main title', () => {
+    renderWithProviders(<PermissionRequiredState />);
+
+    expect(screen.getByText('Missing permission')).toBeInTheDocument();
+  });
+
+  it('should render the description text with permission name', () => {
+    renderWithProviders(<PermissionRequiredState />);
+
+    expect(
+      screen.getByText(
+        /To view Scorecard plugin, contact your administrator to give the/,
+      ),
+    ).toBeInTheDocument();
+    expect(screen.getByText('scorecard.metric.read')).toBeInTheDocument();
+    expect(screen.getByText(/permission\./)).toBeInTheDocument();
+  });
+
+  it('should render the permission name with bold styling', () => {
+    renderWithProviders(<PermissionRequiredState />);
+
+    const permissionText = screen.getByText('scorecard.metric.read');
+    expect(permissionText).toBeInTheDocument();
+    expect(permissionText.tagName).toBe('SPAN');
+  });
+
+  it('should render the read more link button with correct text', () => {
+    renderWithProviders(<PermissionRequiredState />);
+
+    const linkButton = screen.getByRole('link', { name: /read more/i });
+    expect(linkButton).toBeInTheDocument();
+  });
+
+  it('should render the OpenInNew icon in the link button', () => {
+    renderWithProviders(<PermissionRequiredState />);
+
+    const linkButton = screen.getByRole('link', { name: /read more/i });
+    expect(linkButton).toBeInTheDocument();
+
+    const icon = linkButton.querySelector('[data-testid="OpenInNewIcon"]');
+    expect(icon).toBeInTheDocument();
+  });
+
+  it('should have correct href for the read more button', () => {
+    renderWithProviders(<PermissionRequiredState />);
+
+    const linkButton = screen.getByRole('link', { name: /read more/i });
+    expect(linkButton).toHaveAttribute(
+      'href',
+      'https://github.com/redhat-developer/rhdh-plugins/blob/main/workspaces/scorecard/plugins/scorecard/README.md#permission-framework-support',
+    );
+    expect(linkButton).toHaveAttribute('target', '_blank');
+  });
+});

…ard/__tests__/NoScorecardsState.test.tsx …mon/__tests__/NoScorecardsState.test.tsxworkspaces/scorecard/plugins/scorecard/src/components/Scorecard/__tests__/NoScorecardsState.test.tsx renamed to workspaces/scorecard/plugins/scorecard/src/components/Common/__tests__/NoScorecardsState.test.tsx workspaces/scorecard/plugins/scorecard/src/components/Scorecard/__tests__/NoScorecardsState.test.tsx renamed to workspaces/scorecard/plugins/scorecard/src/components/Common/__tests__/NoScorecardsState.test.tsx

@@ -20,15 +20,22 @@ import { ResponseErrorPanel } from '@backstage/core-components';
 import Box from '@mui/material/Box';
 import CircularProgress from '@mui/material/CircularProgress';
 
-import NoScorecardsState from './NoScorecardsState';
+import NoScorecardsState from '../Common/NoScorecardsState';
 import Scorecard from './Scorecard';
 import { useScorecards } from '../../hooks/useScorecards';
 import { getStatusConfig } from '../../utils/utils';
+import { useScorecardMetricsReadPermission } from '../../hooks/useScorecardMetricsReadPermission';
+import PermissionRequiredState from '../Common/PermissionRequiredState';
 
 export const EntityScorecardContent = () => {
   const { scorecards, loadingData, error } = useScorecards();
 
-  if (loadingData) {
+  const {
+    allowed: hasReadScorecardMetricsPermission,
+    loading: loadingPermission,
+  } = useScorecardMetricsReadPermission();
+
+  if (loadingData || loadingPermission) {
     return (
       <Box
         display="flex"
@@ -41,6 +48,10 @@ export const EntityScorecardContent = () => {
     );
   }
 
+  if (!hasReadScorecardMetricsPermission) {
+    return <PermissionRequiredState />;
+  }
+
   if (error) {
     return <ResponseErrorPanel error={error} />;
   }
@@ -57,13 +68,19 @@ export const EntityScorecardContent = () => {
       sx={{ alignItems: 'flex-start' }}
     >
       {scorecards?.map((metric: MetricResult) => {
-        if (metric.status === 'error') {
-          return null;
-        }
+        // Check if metric data unavailable
+        const isMetricDataError =
+          metric.status === 'error' || metric.result?.value === undefined;
 
-        const statusConfig = getStatusConfig(
-          metric.result.thresholdResult?.evaluation,
-        );
+        // Check if threshold has an error
+        const isThresholdError =
+          metric.result?.thresholdResult?.status === 'error';
+
+        const statusConfig = getStatusConfig({
+          evaluation: metric.result?.thresholdResult?.evaluation,
+          thresholdStatus: metric.result?.thresholdResult?.status,
+          metricStatus: metric.status,
+        });
 
         return (
           <Scorecard
@@ -72,9 +89,13 @@ export const EntityScorecardContent = () => {
             description={metric.metadata.description}
             loading={false}
             statusColor={statusConfig.color}
-            StatusIcon={statusConfig.icon}
-            value={metric.result.value}
-            thresholds={metric.result.thresholdResult}
+            StatusIcon={statusConfig.icon ?? (() => null)}
+            value={metric.result?.value}
+            thresholds={metric.result?.thresholdResult}
+            isMetricDataError={isMetricDataError}
+            metricDataError={metric?.error}
+            isThresholdError={isThresholdError}
+            thresholdError={metric.result?.thresholdResult?.error}
           />
         );
       })}