Merge pull request #1534 from github/koesie10/variant-analysis-header-domain-model

Use domain model for VariantAnalysisHeader

extensions/ql-vscode/src/remote-queries/shared/variant-analysis.ts

@@ -34,6 +34,7 @@ export enum VariantAnalysisStatus {
   InProgress = 'inProgress',
   Succeeded = 'succeeded',
   Failed = 'failed',
+  Canceled = 'canceled',
 }
 
 export enum VariantAnalysisFailureReason {
@@ -95,3 +96,41 @@ export interface VariantAnalysisSubmission {
     repositoryOwners?: string[],
   }
 }
+
+/**
+ * @param repo
+ * @returns whether the repo scan is in a completed state, i.e. it cannot normally change state anymore
+ */
+export function hasRepoScanCompleted(repo: VariantAnalysisScannedRepository): boolean {
+  return [
+    // All states that indicates the repository has been scanned and cannot
+    // change status anymore.
+    VariantAnalysisRepoStatus.Succeeded, VariantAnalysisRepoStatus.Failed,
+    VariantAnalysisRepoStatus.Canceled, VariantAnalysisRepoStatus.TimedOut,
+  ].includes(repo.analysisStatus);
+}
+
+/**
+ * @param repos
+ * @returns the total number of results. Will be `undefined` when there are no repos with results.
+ */
+export function getTotalResultCount(repos: VariantAnalysisScannedRepository[] | undefined): number | undefined {
+  const reposWithResultCounts = repos?.filter(repo => repo.resultCount !== undefined);
+  if (reposWithResultCounts === undefined || reposWithResultCounts.length === 0) {
+    return undefined;
+  }
+
+  return reposWithResultCounts.reduce((acc, repo) => acc + (repo.resultCount ?? 0), 0);
+}
+
+/**
+ * @param skippedRepos
+ * @returns the total number of skipped repositories.
+ */
+export function getSkippedRepoCount(skippedRepos: VariantAnalysisSkippedRepositories | undefined): number {
+  if (!skippedRepos) {
+    return 0;
+  }
+
+  return Object.values(skippedRepos).reduce((acc, group) => acc + group.repositoryCount, 0);
+}

extensions/ql-vscode/src/stories/variant-analysis/VariantAnalysisHeader.stories.tsx

@@ -1,10 +1,16 @@
 import React from 'react';
 
-import { ComponentStory, ComponentMeta } from '@storybook/react';
+import { ComponentMeta, ComponentStory } from '@storybook/react';
 
 import { VariantAnalysisContainer } from '../../view/variant-analysis/VariantAnalysisContainer';
 import { VariantAnalysisHeader } from '../../view/variant-analysis/VariantAnalysisHeader';
-import { VariantAnalysisStatus } from '../../remote-queries/shared/variant-analysis';
+import {
+  VariantAnalysis,
+  VariantAnalysisQueryLanguage,
+  VariantAnalysisRepoStatus,
+  VariantAnalysisScannedRepository,
+  VariantAnalysisStatus
+} from '../../remote-queries/shared/variant-analysis';
 
 export default {
   title: 'Variant Analysis/Variant Analysis Header',
@@ -60,22 +66,65 @@ const Template: ComponentStory<typeof VariantAnalysisHeader> = (args) => (
   <VariantAnalysisHeader {...args} />
 );
 
+const buildVariantAnalysis = (data: Partial<VariantAnalysis>) => ({
+  id: 1,
+  controllerRepoId: 1,
+  query: {
+    name: 'Query name',
+    filePath: 'example.ql',
+    language: VariantAnalysisQueryLanguage.Javascript,
+  },
+  databases: {},
+  status: VariantAnalysisStatus.InProgress,
+  ...data,
+});
+
+const buildScannedRepo = (id: number, data?: Partial<VariantAnalysisScannedRepository>): VariantAnalysisScannedRepository => ({
+  repository: {
+    id: id,
+    fullName: `octodemo/hello-world-${id}`,
+    private: false,
+  },
+  analysisStatus: VariantAnalysisRepoStatus.Pending,
+  ...data,
+});
+
 export const InProgress = Template.bind({});
 InProgress.args = {
-  queryName: 'Query name',
-  queryFileName: 'example.ql',
-  variantAnalysisStatus: VariantAnalysisStatus.InProgress,
-  totalRepositoryCount: 10,
-  completedRepositoryCount: 2,
-  resultCount: 99_999,
+  variantAnalysis: buildVariantAnalysis({
+    scannedRepos: [
+      buildScannedRepo(1, {
+        analysisStatus: VariantAnalysisRepoStatus.Succeeded,
+        resultCount: 99_999,
+      }),
+      buildScannedRepo(2, {
+        analysisStatus: VariantAnalysisRepoStatus.Failed,
+      }),
+      buildScannedRepo(3, {
+        analysisStatus: VariantAnalysisRepoStatus.Succeeded,
+        resultCount: 0,
+      }),
+      buildScannedRepo(4),
+      buildScannedRepo(5),
+      buildScannedRepo(6),
+      buildScannedRepo(7),
+      buildScannedRepo(8),
+      buildScannedRepo(9),
+      buildScannedRepo(10),
+    ]
+  }),
 };
 
 export const Succeeded = Template.bind({});
 Succeeded.args = {
   ...InProgress.args,
-  variantAnalysisStatus: VariantAnalysisStatus.Succeeded,
-  totalRepositoryCount: 1000,
-  completedRepositoryCount: 1000,
+  variantAnalysis: buildVariantAnalysis({
+    status: VariantAnalysisStatus.Succeeded,
+    scannedRepos: Array.from({ length: 1000 }, (_, i) => buildScannedRepo(i + 1, {
+      analysisStatus: VariantAnalysisRepoStatus.Succeeded,
+      resultCount: 100,
+    }))
+  }),
   duration: 720_000,
   completedAt: new Date(1661263446000),
 };
@@ -83,7 +132,9 @@ Succeeded.args = {
 export const Failed = Template.bind({});
 Failed.args = {
   ...InProgress.args,
-  variantAnalysisStatus: VariantAnalysisStatus.Failed,
+  variantAnalysis: buildVariantAnalysis({
+    status: VariantAnalysisStatus.Failed,
+  }),
   duration: 10_000,
   completedAt: new Date(1661263446000),
 };

extensions/ql-vscode/src/stories/variant-analysis/VariantAnalysisStats.stories.tsx

@@ -46,7 +46,7 @@ Started.args = {
 export const StartedWithWarnings = Template.bind({});
 StartedWithWarnings.args = {
   ...Starting.args,
-  queryResult: 'warning',
+  hasWarnings: true,
 };
 
 export const Succeeded = Template.bind({});
@@ -64,7 +64,7 @@ SucceededWithWarnings.args = {
   ...Succeeded.args,
   totalRepositoryCount: 10,
   completedRepositoryCount: 2,
-  queryResult: 'warning',
+  hasWarnings: true,
 };
 
 export const Failed = Template.bind({});
@@ -78,5 +78,5 @@ Failed.args = {
 export const Stopped = Template.bind({});
 Stopped.args = {
   ...SucceededWithWarnings.args,
-  queryResult: 'stopped',
+  variantAnalysisStatus: VariantAnalysisStatus.Canceled,
 };

extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx

@@ -1,16 +1,113 @@
 import * as React from 'react';
-import { VariantAnalysisStatus } from '../../remote-queries/shared/variant-analysis';
+
+import {
+  VariantAnalysis as VariantAnalysisDomainModel,
+  VariantAnalysisQueryLanguage,
+  VariantAnalysisRepoStatus,
+  VariantAnalysisStatus
+} from '../../remote-queries/shared/variant-analysis';
 import { VariantAnalysisContainer } from './VariantAnalysisContainer';
 import { VariantAnalysisHeader } from './VariantAnalysisHeader';
 
+const variantAnalysis: VariantAnalysisDomainModel = {
+  id: 1,
+  controllerRepoId: 1,
+  query: {
+    name: 'Example query',
+    filePath: 'example.ql',
+    language: VariantAnalysisQueryLanguage.Javascript,
+  },
+  databases: {},
+  status: VariantAnalysisStatus.InProgress,
+  scannedRepos: [
+    {
+      repository: {
+        id: 1,
+        fullName: 'octodemo/hello-world-1',
+        private: false,
+      },
+      analysisStatus: VariantAnalysisRepoStatus.Pending,
+    },
+    {
+      repository: {
+        id: 2,
+        fullName: 'octodemo/hello-world-2',
+        private: false,
+      },
+      analysisStatus: VariantAnalysisRepoStatus.Pending,
+    },
+    {
+      repository: {
+        id: 3,
+        fullName: 'octodemo/hello-world-3',
+        private: false,
+      },
+      analysisStatus: VariantAnalysisRepoStatus.Pending,
+    },
+    {
+      repository: {
+        id: 4,
+        fullName: 'octodemo/hello-world-4',
+        private: false,
+      },
+      analysisStatus: VariantAnalysisRepoStatus.Pending,
+    },
+    {
+      repository: {
+        id: 5,
+        fullName: 'octodemo/hello-world-5',
+        private: false,
+      },
+      analysisStatus: VariantAnalysisRepoStatus.Pending,
+    },
+    {
+      repository: {
+        id: 6,
+        fullName: 'octodemo/hello-world-6',
+        private: false,
+      },
+      analysisStatus: VariantAnalysisRepoStatus.Pending,
+    },
+    {
+      repository: {
+        id: 7,
+        fullName: 'octodemo/hello-world-7',
+        private: false,
+      },
+      analysisStatus: VariantAnalysisRepoStatus.Pending,
+    },
+    {
+      repository: {
+        id: 8,
+        fullName: 'octodemo/hello-world-8',
+        private: false,
+      },
+      analysisStatus: VariantAnalysisRepoStatus.Pending,
+    },
+    {
+      repository: {
+        id: 9,
+        fullName: 'octodemo/hello-world-9',
+        private: false,
+      },
+      analysisStatus: VariantAnalysisRepoStatus.Pending,
+    },
+    {
+      repository: {
+        id: 10,
+        fullName: 'octodemo/hello-world-10',
+        private: false,
+      },
+      analysisStatus: VariantAnalysisRepoStatus.Pending,
+    },
+  ]
+};
+
 export function VariantAnalysis(): JSX.Element {
   return (
     <VariantAnalysisContainer>
       <VariantAnalysisHeader
-        queryName="Example query"
-        queryFileName="example.ql"
-        totalRepositoryCount={10}
-        variantAnalysisStatus={VariantAnalysisStatus.InProgress}
+        variantAnalysis={variantAnalysis}
         onOpenQueryFileClick={() => console.log('Open query')}
         onViewQueryTextClick={() => console.log('View query')}
         onStopQueryClick={() => console.log('Stop query')}

extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisHeader.tsx

@@ -1,21 +1,18 @@
 import * as React from 'react';
+import { useMemo } from 'react';
 import styled from 'styled-components';
-import { VariantAnalysisStatus } from '../../remote-queries/shared/variant-analysis';
+import {
+  getSkippedRepoCount, getTotalResultCount,
+  hasRepoScanCompleted,
+  VariantAnalysis,
+} from '../../remote-queries/shared/variant-analysis';
 import { QueryDetails } from './QueryDetails';
 import { VariantAnalysisActions } from './VariantAnalysisActions';
 import { VariantAnalysisStats } from './VariantAnalysisStats';
 
 export type VariantAnalysisHeaderProps = {
-  queryName: string;
-  queryFileName: string;
-  variantAnalysisStatus: VariantAnalysisStatus;
+  variantAnalysis: VariantAnalysis;
 
-  totalRepositoryCount: number;
-  completedRepositoryCount?: number | undefined;
-
-  queryResult?: 'warning' | 'stopped';
-
-  resultCount?: number | undefined;
   duration?: number | undefined;
   completedAt?: Date | undefined;
 
@@ -42,15 +39,9 @@ const Row = styled.div`
 `;
 
 export const VariantAnalysisHeader = ({
-  queryName,
-  queryFileName,
-  totalRepositoryCount,
-  completedRepositoryCount,
-  queryResult,
-  resultCount,
+  variantAnalysis,
   duration,
   completedAt,
-  variantAnalysisStatus,
   onOpenQueryFileClick,
   onViewQueryTextClick,
   onStopQueryClick,
@@ -58,28 +49,41 @@ export const VariantAnalysisHeader = ({
   onExportResultsClick,
   onViewLogsClick,
 }: VariantAnalysisHeaderProps) => {
+  const totalScannedRepositoryCount = useMemo(() => {
+    return variantAnalysis.scannedRepos?.length ?? 0;
+  }, [variantAnalysis.scannedRepos]);
+  const completedRepositoryCount = useMemo(() => {
+    return variantAnalysis.scannedRepos?.filter(repo => hasRepoScanCompleted(repo))?.length ?? 0;
+  }, [variantAnalysis.scannedRepos]);
+  const resultCount = useMemo(() => {
+    return getTotalResultCount(variantAnalysis.scannedRepos);
+  }, [variantAnalysis.scannedRepos]);
+  const hasSkippedRepos = useMemo(() => {
+    return getSkippedRepoCount(variantAnalysis.skippedRepos) > 0;
+  }, [variantAnalysis.skippedRepos]);
+
   return (
     <Container>
       <Row>
         <QueryDetails
-          queryName={queryName}
-          queryFileName={queryFileName}
+          queryName={variantAnalysis.query.name}
+          queryFileName={variantAnalysis.query.filePath}
           onOpenQueryFileClick={onOpenQueryFileClick}
           onViewQueryTextClick={onViewQueryTextClick}
         />
         <VariantAnalysisActions
-          variantAnalysisStatus={variantAnalysisStatus}
+          variantAnalysisStatus={variantAnalysis.status}
           onStopQueryClick={onStopQueryClick}
           onCopyRepositoryListClick={onCopyRepositoryListClick}
           onExportResultsClick={onExportResultsClick}
         />
       </Row>
       <VariantAnalysisStats
-        variantAnalysisStatus={variantAnalysisStatus}
-        totalRepositoryCount={totalRepositoryCount}
+        variantAnalysisStatus={variantAnalysis.status}
+        totalRepositoryCount={totalScannedRepositoryCount}
         completedRepositoryCount={completedRepositoryCount}
-        queryResult={queryResult}
         resultCount={resultCount}
+        hasWarnings={hasSkippedRepos}
         duration={duration}
         completedAt={completedAt}
         onViewLogsClick={onViewLogsClick}

extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisRepositoriesStats.tsx

@@ -9,14 +9,14 @@ type Props = {
   totalRepositoryCount: number;
   completedRepositoryCount?: number | undefined;
 
-  queryResult?: 'warning' | 'stopped';
+  showWarning?: boolean;
 };
 
 export const VariantAnalysisRepositoriesStats = ({
   variantAnalysisStatus,
   totalRepositoryCount,
   completedRepositoryCount = 0,
-  queryResult,
+  showWarning,
 }: Props) => {
   if (variantAnalysisStatus === VariantAnalysisStatus.Failed) {
     return (
@@ -29,8 +29,8 @@ export const VariantAnalysisRepositoriesStats = ({
   return (
     <>
       {formatDecimal(completedRepositoryCount)}/{formatDecimal(totalRepositoryCount)}
-      {queryResult && <><HorizontalSpace size={2} /><WarningIcon /></>}
-      {!queryResult && variantAnalysisStatus === VariantAnalysisStatus.Succeeded &&
+      {showWarning && <><HorizontalSpace size={2} /><WarningIcon /></>}
+      {!showWarning && variantAnalysisStatus === VariantAnalysisStatus.Succeeded &&
         <><HorizontalSpace size={2} /><SuccessIcon label="Completed" /></>}
     </>
   );

extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisStats.tsx

@@ -14,7 +14,7 @@ export type VariantAnalysisStatsProps = {
   totalRepositoryCount: number;
   completedRepositoryCount?: number | undefined;
 
-  queryResult?: 'warning' | 'stopped';
+  hasWarnings?: boolean;
 
   resultCount?: number | undefined;
   duration?: number | undefined;
@@ -33,7 +33,7 @@ export const VariantAnalysisStats = ({
   variantAnalysisStatus,
   totalRepositoryCount,
   completedRepositoryCount = 0,
-  queryResult,
+  hasWarnings,
   resultCount,
   duration,
   completedAt,
@@ -48,16 +48,16 @@ export const VariantAnalysisStats = ({
       return 'Failed';
     }
 
-    if (queryResult === 'warning') {
-      return 'Succeeded warnings';
-    }
-
-    if (queryResult === 'stopped') {
+    if (variantAnalysisStatus === VariantAnalysisStatus.Canceled) {
       return 'Stopped';
     }
 
+    if (variantAnalysisStatus === VariantAnalysisStatus.Succeeded && hasWarnings) {
+      return 'Succeeded warnings';
+    }
+
     return 'Succeeded';
-  }, [variantAnalysisStatus, queryResult]);
+  }, [variantAnalysisStatus, hasWarnings]);
 
   return (
     <Row>
@@ -69,7 +69,7 @@ export const VariantAnalysisStats = ({
           variantAnalysisStatus={variantAnalysisStatus}
           totalRepositoryCount={totalRepositoryCount}
           completedRepositoryCount={completedRepositoryCount}
-          queryResult={queryResult}
+          showWarning={hasWarnings}
         />
       </StatItem>
       <StatItem title="Duration">

extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisStats.spec.tsx

@@ -39,16 +39,8 @@ describe(VariantAnalysisStats.name, () => {
     expect(screen.getByText('654,321/123,456')).toBeInTheDocument();
   });
 
-  it('renders a warning icon when the query result is a warning', () => {
-    render({ queryResult: 'warning' });
-
-    expect(screen.getByRole('img', {
-      name: 'Warning',
-    })).toBeInTheDocument();
-  });
-
-  it('renders a warning icon when the query result is stopped', () => {
-    render({ queryResult: 'stopped' });
+  it('renders a warning icon when has warnings is set', () => {
+    render({ hasWarnings: true });
 
     expect(screen.getByRole('img', {
       name: 'Warning',
@@ -77,4 +69,35 @@ describe(VariantAnalysisStats.name, () => {
     userEvent.click(screen.getByText('View logs'));
     expect(onViewLogsClick).toHaveBeenCalledTimes(1);
   });
+
+  it('renders a running text when the variant analysis status is in progress', () => {
+    render({ variantAnalysisStatus: VariantAnalysisStatus.InProgress });
+
+    expect(screen.getByText('Running')).toBeInTheDocument();
+  });
+
+  it('renders a failed text when the variant analysis status is failed', () => {
+    render({ variantAnalysisStatus: VariantAnalysisStatus.Failed });
+
+    expect(screen.getByText('Failed')).toBeInTheDocument();
+  });
+
+  it('renders a stopped text when the variant analysis status is canceled', () => {
+    render({ variantAnalysisStatus: VariantAnalysisStatus.Canceled });
+
+    expect(screen.getByText('Stopped')).toBeInTheDocument();
+  });
+
+  it('renders a succeeded warnings text when the variant analysis status is succeeded and has warnings', () => {
+    render({ variantAnalysisStatus: VariantAnalysisStatus.Succeeded, hasWarnings: true });
+
+    expect(screen.getByText('Succeeded warnings')).toBeInTheDocument();
+  });
+
+  it('renders a succeeded text when the variant analysis status is succeeded', () => {
+    render({ variantAnalysisStatus: VariantAnalysisStatus.Succeeded });
+
+    expect(screen.getByText('Succeeded')).toBeInTheDocument();
+    expect(screen.queryByText('Succeeded warnings')).not.toBeInTheDocument();
+  });
 });