diff --git a/.changeset/friendly-kids-mix.md b/.changeset/friendly-kids-mix.md new file mode 100644 index 0000000000..b59209290c --- /dev/null +++ b/.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. diff --git a/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.test.ts b/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.test.ts index 77b3e6c8b1..5c33e9e369 100644 --- a/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.test.ts +++ b/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.test.ts @@ -52,6 +52,26 @@ const testChecks: Record = { 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', diff --git a/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.ts b/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.ts index ae1f18a373..2623a5c2ee 100644 --- a/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.ts +++ b/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 }), diff --git a/plugins/tech-insights/src/components/ScorecardsOverview/ChecksOverview.tsx b/plugins/tech-insights/src/components/ScorecardsOverview/ChecksOverview.tsx index 10025d1ba6..b07eeab339 100644 --- a/plugins/tech-insights/src/components/ScorecardsOverview/ChecksOverview.tsx +++ b/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 No checks have any data yet.; + } const checkRenderType = api.getScorecardsDefinition( checks[0].check.type, checks,