Merge pull request #8516 from wejendorp/jsonfc-fix

fix: allow partial evaluation of checks in jsonfc factchecker

.changeset/friendly-kids-mix.md

@@ -0,0 +1,8 @@
+---
+'@backstage/plugin-tech-insights': patch
+'@backstage/plugin-tech-insights-backend-module-jsonfc': patch
+---
+
+RunChecks endpoint now handles missing retriever data in checks. Instead of
+showing server errors, the checks will be shown for checks whose retrievers have
+data, and a warning will be shown if no checks are returned.

plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.test.ts

@@ -52,6 +52,26 @@ const testChecks: Record<string, TechInsightJsonRuleCheck[]> = {
       name: 'brokenTestCheck2',
       type: JSON_RULE_ENGINE_CHECK_TYPE,
       description: 'Second Broken Check For Testing',
+      factIds: ['test-factretriever'],
+      rule: {
+        conditions: {
+          any: [
+            {
+              fact: 'somefact',
+              operator: 'lessThan',
+              value: 1,
+            },
+          ],
+        },
+      },
+    },
+  ],
+  brokennotfound: [
+    {
+      id: 'brokenTestCheckNotFound',
+      name: 'brokenTestCheckNotFound',
+      type: JSON_RULE_ENGINE_CHECK_TYPE,
+      description: 'Third Broken Check For Testing',
       factIds: ['non-existing-factretriever'],
       rule: {
         conditions: {
@@ -122,7 +142,14 @@ const latestSchemasMock = jest.fn().mockImplementation(() => [
   },
 ]);
 const factsBetweenTimestampsByIdsMock = jest.fn();
-const latestFactsByIdsMock = jest.fn().mockImplementation(() => ({}));
+const latestFactsByIdsMock = jest.fn().mockResolvedValue({
+  ['test-factretriever']: {
+    id: 'test-factretriever',
+    facts: {
+      testnumberfact: 3,
+    },
+  },
+});
 const mockCheckRegistry = {
   getAll(checks: string[]) {
     return checks.flatMap(check => testChecks[check]);
@@ -156,17 +183,22 @@ describe('JsonRulesEngineFactChecker', () => {
         'Failed to run rules engine, Undefined fact: somefact',
       );
     });
-    it('should respond with result, facts, fact schemas and checks', async () => {
-      latestFactsByIdsMock.mockImplementation(() =>
-        Promise.resolve({
-          ['test-factretriever']: {
-            id: 'test-factretriever',
-            facts: {
-              testnumberfact: 3,
-            },
-          },
+
+    it('should skip checks where fact data is missing', async () => {
+      const skipped = async () =>
+        await factChecker.runChecks('a/a/a', ['brokennotfound']);
+      await expect(skipped()).resolves.toEqual([]);
+
+      const partial = async () =>
+        await factChecker.runChecks('a/a/a', ['brokennotfound', 'simple']);
+      await expect(partial()).resolves.toEqual([
+        expect.objectContaining({
+          check: expect.objectContaining({ id: 'simpleTestCheck' }),
         }),
-      );
+      ]);
+    });
+
+    it('should respond with result, facts, fact schemas and checks', async () => {
       const results = await factChecker.runChecks('a/a/a', ['simple']);
       expect(results).toHaveLength(1);
       expect(results[0]).toMatchObject({
@@ -203,16 +235,6 @@ describe('JsonRulesEngineFactChecker', () => {
     });
 
     it('should gracefully handle multiple check at once', async () => {
-      latestFactsByIdsMock.mockImplementation(() =>
-        Promise.resolve({
-          ['test-factretriever']: {
-            id: 'test-factretriever',
-            facts: {
-              testnumberfact: 3,
-            },
-          },
-        }),
-      );
       const results = await factChecker.runChecks('a/a/a', [
         'simple',
         'simple2',

plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.ts

@@ -88,7 +88,21 @@ export class JsonRulesEngineFactChecker
     techInsightChecks.forEach(techInsightCheck => {
       const rule = techInsightCheck.rule;
       rule.name = techInsightCheck.id;
-      engine.addRule({ ...techInsightCheck.rule, event: noopEvent });
+      // Only run checks that have all the facts available:
+      const hasAllFacts = techInsightCheck.factIds.every(
+        factId => facts[factId],
+      );
+      if (hasAllFacts) {
+        engine.addRule({ ...techInsightCheck.rule, event: noopEvent });
+      } else {
+        this.logger.warn(
+          `Skipping ${
+            rule.name
+          } due to missing facts: ${techInsightCheck.factIds
+            .filter(factId => !facts[factId])
+            .join(', ')}`,
+        );
+      }
     });
     const factValues = Object.values(facts).reduce(
       (acc, it) => ({ ...acc, ...it.facts }),

plugins/tech-insights/src/components/ScorecardsOverview/ChecksOverview.tsx

@@ -21,6 +21,7 @@ import { Content, Page, 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';
 
 const useStyles = makeStyles((theme: BackstageTheme) => ({
   contentScorecards: {
@@ -40,6 +41,9 @@ type Checks = {
 export const ChecksOverview = ({ checks }: Checks) => {
   const classes = useStyles();
   const api = useApi(techInsightsApiRef);
+  if (!checks.length) {
+    return <Alert severity="warning">No checks have any data yet.</Alert>;
+  }
   const checkRenderType = api.getScorecardsDefinition(
     checks[0].check.type,
     checks,