Merge pull request #13093 from kielosz/scorecards

[TechInsights] Use different renderers for each CheckResult in scorecards
2022-08-24 10:06:50 +02:00
parent bc6fb1ccc3 beb15973cc
commit 1e21d126b2
18 changed files with 242 additions and 146 deletions
@@ -0,0 +1,16 @@
+---
+'@backstage/plugin-tech-insights': minor
+---
+
+Added the possibility to display check results of different types on a single scorecard.
+
+- **BREAKING** Removed the `getScorecardsDefinition` method from the `TechInsightsApi` interface. Added the `getCheckResultRenderers` method that returns rendering components for given types.
+- **BREAKING** The `CheckResultRenderer` type now exposes the `component` factory method that creates a React component used to display a result of a provided check result.
+- The `TechInsightsClient` constructor accepts now the optional `renderers` parameter that can be used to inject a custom renderer.
+- **BREAKING** The `title` parameter in the `EntityTechInsightsScorecardContent` and `EntityTechInsightsScorecardCard` components is now mandatory.
+- The `jsonRulesEngineCheckResultRenderer` used to render `json-rules-engine` check results is exported.
+- The `BooleanCheck` component that can be used to render other check results types is also exported.
+
+If you were overriding the `getScorecardsDefinition` method to adjust the rendering of check results, you should now provide a custom renderer using `renderers` parameter in the `TechInsightsClient` class.
+
+See the [README](https://github.com/backstage/backstage/tree/master/plugins/tech-insights/README.md) for more details.
@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-tech-insights-backend': patch
+---
+
+Changed the description of the `techdocsAnnotationFactName` fact.
@@ -110,6 +110,7 @@ import {
 } from '@backstage/plugin-rollbar';
 import { EntitySentryContent } from '@backstage/plugin-sentry';
 import { EntityTechdocsContent } from '@backstage/plugin-techdocs';
+import { EntityTechInsightsScorecardCard } from '@backstage/plugin-tech-insights';
 import { EntityTodoContent } from '@backstage/plugin-todo';
 import { Button, Grid } from '@material-ui/core';
 import BadgeIcon from '@material-ui/icons/CallToAction';
@@ -445,6 +446,24 @@ const serviceEntityPage = (
      <EntityGithubInsightsContent />
    </EntityLayout.Route>

+    <EntityLayout.Route path="/tech-insights" title="Scorecards">
+      <Grid container spacing={3} alignItems="stretch">
+        <Grid item xs={12} md={6}>
+          <EntityTechInsightsScorecardCard
+            title="Scorecard 1"
+            description="This is a sample scorecard no. 1"
+            checksId={['titleCheck']}
+          />
+        </Grid>
+        <Grid item xs={12} md={6}>
+          <EntityTechInsightsScorecardCard
+            title="Scorecard 2"
+            checksId={['techDocsCheck']}
+          />
+        </Grid>
+      </Grid>
+    </EntityLayout.Route>
+
    <EntityLayout.Route path="/code-coverage" title="Code Coverage">
      <EntityCodeCoverageContent />
    </EntityLayout.Route>
@@ -57,33 +57,34 @@ export default async function createPlugin(
      logger: env.logger,
      checks: [
        {
-          id: 'simpleTestCheck',
+          id: 'titleCheck',
          type: JSON_RULE_ENGINE_CHECK_TYPE,
-          name: 'simpleTestCheck',
-          description: 'Simple Check For Testing',
-          factIds: [
-            'entityMetadataFactRetriever',
-            'techdocsFactRetriever',
-            'entityOwnershipFactRetriever',
-          ],
+          name: 'Title Check',
+          description:
+            'Verifies that a Title, used to improve readability, has been set for this entity',
+          factIds: ['entityMetadataFactRetriever'],
          rule: {
            conditions: {
              all: [
-                {
-                  fact: 'hasGroupOwner',
-                  operator: 'equal',
-                  value: true,
-                },
                {
                  fact: 'hasTitle',
                  operator: 'equal',
                  value: true,
                },
-                {
-                  fact: 'hasDescription',
-                  operator: 'equal',
-                  value: true,
-                },
+              ],
+            },
+          },
+        },
+        {
+          id: 'techDocsCheck',
+          type: JSON_RULE_ENGINE_CHECK_TYPE,
+          name: 'TechDocs Check',
+          description:
+            'Verifies that TechDocs has been enabled for this entity',
+          factIds: ['techdocsFactRetriever'],
+          rule: {
+            conditions: {
+              all: [
                {
                  fact: 'hasAnnotationBackstageIoTechdocsRef',
                  operator: 'equal',
@@ -40,7 +40,7 @@ export const techdocsFactRetriever: FactRetriever = {
  schema: {
    [techdocsAnnotationFactName]: {
      type: 'boolean',
-      description: 'The entity has a title in metadata',
+      description: 'The entity has a TechDocs reference annotation',
    },
  },
  handler: async ({
@@ -44,11 +44,11 @@ const serviceEntityPage = (
 );
 ```

-It is not obligatory to pass title and description props to `EntityTechInsightsScorecardContent`. If those are left out, default values from `defaultCheckResultRenderers` in `CheckResultRenderer` will be taken, hence `Boolean scorecard` and `This card represents an overview of default boolean Backstage checks`.
+It is obligatory to pass `title` prop to `EntityTechInsightsScorecardContent`, `description` prop is optional.

 If you like to display multiple cards in a `EntityLayout.Route` use `EntityTechInsightsScorecardCard`.

-You can pass an array `checksId` as a prop with the [Fact Retrievers ids](../tech-insights-backend#creating-fact-retrievers) to limit which checks you want to show in this card, If you don't pass, the default value is show all checks.
+You can pass an array `checksId` as a prop with the [Fact Retrievers ids](../tech-insights-backend#creating-fact-retrievers) to limit which checks you want to show in this card. If you don't pass, the default value is show all checks.

 ```tsx
 <EntityTechInsightsScorecardContent
@@ -87,6 +87,43 @@ const overviewContent = (

 ## Boolean Scorecard Example

-If you follow the [Backend Example](https://github.com/backstage/backstage/tree/master/plugins/tech-insights-backend#backend-example), once the needed facts have been generated the boolean scorecard will look like this:
+If you follow the [Backend Example](https://github.com/backstage/backstage/tree/master/plugins/tech-insights-backend#backend-example), once the needed facts have been generated the default boolean scorecard will look like this:

 ![Boolean Scorecard Example](./docs/boolean-scorecard-example.png)
+
+## Adding custom rendering components
+
+Default scorecard implementation displays only `json-rules-engine` check results. If you would like to support different types, you need to inject custom rendering components to the `TechInsightsClient` constructor.
+
+```ts
+// packages/app/src/apis.ts
+
+export const apis: AnyApiFactory[] = [
+...
+  createApiFactory({
+    api: techInsightsApiRef,
+    deps: { discoveryApi: discoveryApiRef, identityApi: identityApiRef },
+    factory: ({ discoveryApi, identityApi }) =>
+      new TechInsightsClient({
+        discoveryApi,
+        identityApi,
+        renderers: [
+          jsonRulesEngineCheckResultRenderer, // default json-rules-engine renderer
+          myCustomBooleanRenderer, // custom renderer
+        ],
+      }),
+  }),
+...
+];
+```
+
+```tsx
+// packages/app/src/components/myCustomBooleanRenderer.tsx
+
+export const myCustomBooleanRenderer: CheckResultRenderer = {
+  type: 'boolean',
+  component: (checkResult: CheckResult) => (
+    <BooleanCheck checkResult={checkResult} />
+  ),
+};
+```
@@ -15,6 +15,13 @@ import { IdentityApi } from '@backstage/core-plugin-api';
 import { default as React_2 } from 'react';
 import { RouteRef } from '@backstage/core-plugin-api';

+// @public (undocumented)
+export const BooleanCheck: ({
+  checkResult,
+}: {
+  checkResult: CheckResult;
+}) => JSX.Element;
+
 // @public
 export type Check = {
  id: string;
@@ -27,36 +34,32 @@ export type Check = {
 // @public
 export type CheckResultRenderer = {
  type: string;
-  title: string;
-  description: string;
-  component: React_2.ReactElement;
+  component: (check: CheckResult) => React_2.ReactElement;
 };

 // @public (undocumented)
 export const EntityTechInsightsScorecardCard: (props: {
-  title?: string | undefined;
+  title: string;
  description?: string | undefined;
  checksId?: string[] | undefined;
 }) => JSX.Element;

 // @public (undocumented)
 export const EntityTechInsightsScorecardContent: (props: {
-  title?: string | undefined;
+  title: string;
  description?: string | undefined;
  checksId?: string[] | undefined;
 }) => JSX.Element;

+// @public
+export const jsonRulesEngineCheckResultRenderer: CheckResultRenderer;
+
 // @public
 export interface TechInsightsApi {
  // (undocumented)
  getAllChecks(): Promise<Check[]>;
  // (undocumented)
-  getScorecardsDefinition: (
-    type: string,
-    value: CheckResult[],
-    title?: string,
-    description?: string,
-  ) => CheckResultRenderer | undefined;
+  getCheckResultRenderers: (types: string[]) => CheckResultRenderer[];
  // (undocumented)
  runBulkChecks(
    entities: CompoundEntityRef[],
@@ -77,16 +80,12 @@ export class TechInsightsClient implements TechInsightsApi {
  constructor(options: {
    discoveryApi: DiscoveryApi;
    identityApi: IdentityApi;
+    renderers?: CheckResultRenderer[];
  });
  // (undocumented)
  getAllChecks(): Promise<Check[]>;
  // (undocumented)
-  getScorecardsDefinition(
-    type: string,
-    value: CheckResult[],
-    title?: string,
-    description?: string,
-  ): CheckResultRenderer | undefined;
+  getCheckResultRenderers(types: string[]): CheckResultRenderer[];
  // (undocumented)
  runBulkChecks(
    entities: CompoundEntityRef[],
@@ -23,7 +23,7 @@ import {
 createDevApp()
  .registerPlugin(techInsightsPlugin)
  .addPage({
-    element: <EntityTechInsightsScorecardContent />,
+    element: <EntityTechInsightsScorecardContent title="Test scorecard" />,
    title: 'Root Page',
    path: '/tech-insight-scorecard',
  })
@@ -38,12 +38,7 @@ export const techInsightsApiRef = createApiRef<TechInsightsApi>({
 * @public
 */
 export interface TechInsightsApi {
-  getScorecardsDefinition: (
-    type: string,
-    value: CheckResult[],
-    title?: string,
-    description?: string,
-  ) => CheckResultRenderer | undefined;
+  getCheckResultRenderers: (types: string[]) => CheckResultRenderer[];
  getAllChecks(): Promise<Check[]>;
  runChecks(
    entityParams: CompoundEntityRef,
@@ -26,34 +26,28 @@ import { CompoundEntityRef } from '@backstage/catalog-model';

 import {
  CheckResultRenderer,
-  defaultCheckResultRenderers,
+  jsonRulesEngineCheckResultRenderer,
 } from '../components/CheckResultRenderer';

 /** @public */
 export class TechInsightsClient implements TechInsightsApi {
  private readonly discoveryApi: DiscoveryApi;
  private readonly identityApi: IdentityApi;
+  private readonly renderers?: CheckResultRenderer[];

  constructor(options: {
    discoveryApi: DiscoveryApi;
    identityApi: IdentityApi;
+    renderers?: CheckResultRenderer[];
  }) {
    this.discoveryApi = options.discoveryApi;
    this.identityApi = options.identityApi;
+    this.renderers = options.renderers;
  }

-  getScorecardsDefinition(
-    type: string,
-    value: CheckResult[],
-    title?: string,
-    description?: string,
-  ): CheckResultRenderer | undefined {
-    const resultRenderers = defaultCheckResultRenderers(
-      value,
-      title,
-      description,
-    );
-    return resultRenderers.find(d => d.type === type);
+  getCheckResultRenderers(types: string[]): CheckResultRenderer[] {
+    const renderers = this.renderers ?? [jsonRulesEngineCheckResultRenderer];
+    return renderers.filter(d => types.includes(d.type));
  }

  async getAllChecks(): Promise<Check[]> {
@@ -15,46 +15,17 @@
 */

 import React from 'react';
-import { makeStyles, List, ListItem, ListItemText } from '@material-ui/core';
 import CheckCircleOutline from '@material-ui/icons/CheckCircleOutline';
 import ErrorOutlineIcon from '@material-ui/icons/ErrorOutline';
 import { CheckResult } from '@backstage/plugin-tech-insights-common';
-import { BackstageTheme } from '@backstage/theme';

-const useStyles = makeStyles((theme: BackstageTheme) => ({
-  listItemText: {
-    paddingRight: theme.spacing(0.5),
-    flex: '0 1 auto',
-  },
-  icon: {
-    marginLeft: 'auto',
-  },
-}));
-
-type Prop = {
-  checkResult: CheckResult[];
-};
-
-export const BooleanCheck = ({ checkResult }: Prop) => {
-  const classes = useStyles();
-
-  return (
-    <List>
-      {checkResult.map((check, index) => (
-        <ListItem key={check.check.id}>
-          <ListItemText
-            key={index}
-            primary={check.check.name}
-            secondary={check.check.description}
-            className={classes.listItemText}
-          />
-          {check.result ? (
-            <CheckCircleOutline className={classes.icon} color="primary" />
-          ) : (
-            <ErrorOutlineIcon className={classes.icon} color="error" />
-          )}
-        </ListItem>
-      ))}
-    </List>
+/**
+ * @public
+ */
+export const BooleanCheck = ({ checkResult }: { checkResult: CheckResult }) => {
+  return !!checkResult.result ? (
+    <CheckCircleOutline color="primary" />
+  ) : (
+    <ErrorOutlineIcon color="error" />
  );
 };
@@ -1,5 +1,5 @@
 /*
- * Copyright 2020 The Backstage Authors
+ * Copyright 2022 The Backstage Authors
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
@@ -19,30 +19,23 @@ import React from 'react';
 import { BooleanCheck } from './BooleanCheck';

 /**
- * Defines a react component that is responsible for rendering a results of a given type.
+ * Defines a react component that is responsible for rendering a result of a given type.
 *
 * @public
 */
 export type CheckResultRenderer = {
  type: string;
-  title: string;
-  description: string;
-  component: React.ReactElement;
+  component: (check: CheckResult) => React.ReactElement;
 };

-export function defaultCheckResultRenderers(
-  value: CheckResult[],
-  title?: string,
-  description?: string,
-): CheckResultRenderer[] {
-  return [
-    {
-      type: 'json-rules-engine',
-      title: title || 'Boolean scorecard',
-      description:
-        description ||
-        'This card represents an overview of default boolean Backstage checks:',
-      component: <BooleanCheck checkResult={value} />,
-    },
-  ];
-}
+/**
+ * Default renderer for json-rules-engine check results.
+ *
+ * @public
+ */
+export const jsonRulesEngineCheckResultRenderer: CheckResultRenderer = {
+  type: 'json-rules-engine',
+  component: (checkResult: CheckResult) => (
+    <BooleanCheck checkResult={checkResult} />
+  ),
+};
@@ -25,7 +25,7 @@ import { useEntity } from '@backstage/plugin-catalog-react';
 import { getCompoundEntityRef } from '@backstage/catalog-model';

 export const ScorecardsCard = (props: {
-  title?: string;
+  title: string;
  description?: string;
  checksId?: string[];
 }) => {
@@ -46,7 +46,7 @@ export const ScorecardsCard = (props: {
    <ScorecardInfo
      title={title}
      description={description}
-      checks={value || []}
+      checkResults={value || []}
    />
  );
 };
@@ -33,7 +33,7 @@ const useStyles = makeStyles(() => ({
 }));

 export const ScorecardsContent = (props: {
-  title?: string;
+  title: string;
  description?: string;
  checksId?: string[];
 }) => {
@@ -57,7 +57,7 @@ export const ScorecardsContent = (props: {
        <ScorecardInfo
          title={title}
          description={description}
-          checks={value || []}
+          checkResults={value || []}
        />
      </Content>
    </Page>
@@ -16,12 +16,11 @@

 import React from 'react';
 import { makeStyles, Grid, Typography } from '@material-ui/core';
-import { useApi } from '@backstage/core-plugin-api';
 import { InfoCard } from '@backstage/core-components';
 import { CheckResult } from '@backstage/plugin-tech-insights-common';
-import { techInsightsApiRef } from '../../api/TechInsightsApi';
 import { BackstageTheme } from '@backstage/theme';
 import { Alert } from '@material-ui/lab';
+import { ScorecardsList } from '../ScorecardsList';

 const useStyles = makeStyles((theme: BackstageTheme) => ({
  subheader: {
@@ -31,16 +30,18 @@ const useStyles = makeStyles((theme: BackstageTheme) => ({
 }));

 const infoCard = (
-  title: string | undefined,
+  title: string,
  description: string | undefined,
  className: string,
  element: JSX.Element,
 ) => (
  <Grid item xs={12}>
    <InfoCard title={title}>
-      <Typography className={className} variant="body1" gutterBottom>
-        {description}
-      </Typography>
+      {description && (
+        <Typography className={className} variant="body1" gutterBottom>
+          {description}
+        </Typography>
+      )}
      <Grid item xs={12}>
        {element}
      </Grid>
@@ -49,15 +50,14 @@ const infoCard = (
 );

 export const ScorecardInfo = (props: {
-  checks: CheckResult[];
-  title?: string;
+  checkResults: CheckResult[];
+  title: string;
  description?: string;
 }) => {
-  const { checks, title, description } = props;
+  const { checkResults, title, description } = props;
  const classes = useStyles();
-  const api = useApi(techInsightsApiRef);

-  if (!checks.length) {
+  if (!checkResults.length) {
    return infoCard(
      title,
      description,
@@ -66,21 +66,10 @@ export const ScorecardInfo = (props: {
    );
  }

-  const checkRenderType = api.getScorecardsDefinition(
-    checks[0].check.type,
-    checks,
+  return infoCard(
    title,
    description,
+    classes.subheader,
+    <ScorecardsList checkResults={checkResults} />,
  );
-
-  if (checkRenderType) {
-    return infoCard(
-      checkRenderType.title,
-      checkRenderType.description,
-      classes.subheader,
-      checkRenderType.component,
-    );
-  }
-
-  return <></>;
 };
@@ -0,0 +1,58 @@
+/*
+ * Copyright 2021 The Backstage Authors
+ *
+ * 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 React from 'react';
+import { useApi } from '@backstage/core-plugin-api';
+import { makeStyles, List, ListItem, ListItemText } from '@material-ui/core';
+import { techInsightsApiRef } from '../../api/TechInsightsApi';
+import { CheckResult } from '@backstage/plugin-tech-insights-common';
+import { BackstageTheme } from '@backstage/theme';
+import { Alert } from '@material-ui/lab';
+
+const useStyles = makeStyles((theme: BackstageTheme) => ({
+  listItemText: {
+    paddingRight: theme.spacing(0.5),
+  },
+}));
+
+export const ScorecardsList = (props: { checkResults: CheckResult[] }) => {
+  const { checkResults } = props;
+  const classes = useStyles();
+  const api = useApi(techInsightsApiRef);
+
+  const types = [...new Set(checkResults.map(({ check }) => check.type))];
+  const checkResultRenderers = api.getCheckResultRenderers(types);
+
+  return (
+    <List>
+      {checkResults.map((result, index) => (
+        <ListItem key={result.check.id}>
+          <ListItemText
+            key={index}
+            primary={result.check.name}
+            secondary={result.check.description}
+            className={classes.listItemText}
+          />
+          {checkResultRenderers
+            .find(({ type }) => type === result.check.type)
+            ?.component(result) ?? (
+            <Alert severity="error">Unknown type.</Alert>
+          )}
+        </ListItem>
+      ))}
+    </List>
+  );
+};
@@ -0,0 +1,17 @@
+/*
+ * Copyright 2021 The Backstage Authors
+ *
+ * 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.
+ */
+
+export { ScorecardsList } from './ScorecardsList';
@@ -21,4 +21,6 @@ export {

 export { techInsightsApiRef, TechInsightsClient } from './api';
 export type { TechInsightsApi, Check } from './api';
+export { BooleanCheck } from './components/BooleanCheck';
+export { jsonRulesEngineCheckResultRenderer } from './components/CheckResultRenderer';
 export type { CheckResultRenderer } from './components/CheckResultRenderer';