Merge pull request #1560 from github/koesie10/reduce-duplication-repo-rows

Combine repository row components into a single component

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

@@ -3,16 +3,16 @@ import React from 'react';
 import { ComponentMeta, ComponentStory } from '@storybook/react';
 
 import { VariantAnalysisContainer } from '../../view/variant-analysis/VariantAnalysisContainer';
-import { VariantAnalysisAnalyzedRepoItem } from '../../view/variant-analysis/VariantAnalysisAnalyzedRepoItem';
 import { VariantAnalysisRepoStatus } from '../../remote-queries/shared/variant-analysis';
 import { AnalysisAlert, AnalysisRawResults } from '../../remote-queries/shared/analysis-result';
 
 import analysesResults from '../remote-queries/data/analysesResultsMessage.json';
 import rawResults from '../remote-queries/data/rawResults.json';
+import { RepoRow } from '../../view/variant-analysis/RepoRow';
 
 export default {
-  title: 'Variant Analysis/Analyzed Repo Item',
-  component: VariantAnalysisAnalyzedRepoItem,
+  title: 'Variant Analysis/Repo Row',
+  component: RepoRow,
   decorators: [
     (Story) => (
       <VariantAnalysisContainer>
@@ -20,10 +20,10 @@ export default {
       </VariantAnalysisContainer>
     )
   ],
-} as ComponentMeta<typeof VariantAnalysisAnalyzedRepoItem>;
+} as ComponentMeta<typeof RepoRow>;
 
-const Template: ComponentStory<typeof VariantAnalysisAnalyzedRepoItem> = (args) => (
-  <VariantAnalysisAnalyzedRepoItem {...args} />
+const Template: ComponentStory<typeof RepoRow> = (args) => (
+  <RepoRow {...args} />
 );
 
 export const Pending = Template.bind({});
@@ -77,3 +77,26 @@ RawResults.args = {
   resultCount: 1,
   rawResults: rawResults as unknown as AnalysisRawResults,
 };
+
+export const SkippedOnlyFullName = Template.bind({});
+SkippedOnlyFullName.args = {
+  repository: {
+    fullName: 'octodemo/hello-globe',
+  }
+};
+
+export const SkippedPublic = Template.bind({});
+SkippedPublic.args = {
+  repository: {
+    fullName: 'octodemo/hello-globe',
+    private: false,
+  }
+};
+
+export const SkippedPrivate = Template.bind({});
+SkippedPrivate.args = {
+  repository: {
+    fullName: 'octodemo/hello-globe',
+    private: true,
+  }
+};

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

@@ -1,45 +0,0 @@
-import React from 'react';
-
-import { ComponentMeta, ComponentStory } from '@storybook/react';
-
-import { VariantAnalysisContainer } from '../../view/variant-analysis/VariantAnalysisContainer';
-import { VariantAnalysisSkippedRepositoryRow } from '../../view/variant-analysis/VariantAnalysisSkippedRepositoryRow';
-
-export default {
-  title: 'Variant Analysis/Variant Analysis Skipped Repository',
-  component: VariantAnalysisSkippedRepositoryRow,
-  decorators: [
-    (Story) => (
-      <VariantAnalysisContainer>
-        <Story />
-      </VariantAnalysisContainer>
-    )
-  ],
-} as ComponentMeta<typeof VariantAnalysisSkippedRepositoryRow>;
-
-const Template: ComponentStory<typeof VariantAnalysisSkippedRepositoryRow> = (args) => (
-  <VariantAnalysisSkippedRepositoryRow {...args} />
-);
-
-export const OnlyFullName = Template.bind({});
-OnlyFullName.args = {
-  repository: {
-    fullName: 'octodemo/hello-globe',
-  }
-};
-
-export const Public = Template.bind({});
-Public.args = {
-  repository: {
-    fullName: 'octodemo/hello-globe',
-    private: false,
-  }
-};
-
-export const Private = Template.bind({});
-Private.args = {
-  repository: {
-    fullName: 'octodemo/hello-globe',
-    private: true,
-  }
-};

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

@@ -1,10 +1,10 @@
 import * as React from 'react';
 import { useCallback, useState } from 'react';
 import styled from 'styled-components';
-import { VSCodeBadge } from '@vscode/webview-ui-toolkit/react';
+import { VSCodeBadge, VSCodeCheckbox } from '@vscode/webview-ui-toolkit/react';
 import { isCompletedAnalysisRepoStatus, VariantAnalysisRepoStatus } from '../../remote-queries/shared/variant-analysis';
 import { formatDecimal } from '../../pure/number';
-import { Codicon, ErrorIcon, LoadingIcon, SuccessIcon } from '../common';
+import { Codicon, ErrorIcon, LoadingIcon, SuccessIcon, WarningIcon } from '../common';
 import { Repository } from '../../remote-queries/shared/repository';
 import { AnalysisAlert, AnalysisRawResults } from '../../remote-queries/shared/analysis-result';
 import { AnalyzedRepoItemContent } from './AnalyzedRepoItemContent';
@@ -31,20 +31,22 @@ const TitleContainer = styled.button`
   }
 `;
 
-const Visibility = styled.span`
+const VisibilityText = styled.span`
   font-size: 0.85em;
   color: var(--vscode-descriptionForeground);
 `;
 
-export type VariantAnalysisAnalyzedRepoItemProps = {
-  repository: Repository;
-  status: VariantAnalysisRepoStatus;
-  resultCount?: number;
-
-  interpretedResults?: AnalysisAlert[];
-  rawResults?: AnalysisRawResults;
+type VisibilityProps = {
+  isPrivate?: boolean;
 }
 
+const Visibility = ({ isPrivate }: VisibilityProps) => {
+  if (isPrivate === undefined) {
+    return null;
+  }
+  return <VisibilityText>{isPrivate ? 'private' : 'public'}</VisibilityText>;
+};
+
 const getErrorLabel = (status: VariantAnalysisRepoStatus.Failed | VariantAnalysisRepoStatus.TimedOut | VariantAnalysisRepoStatus.Canceled): string => {
   switch (status) {
     case VariantAnalysisRepoStatus.Failed:
@@ -56,35 +58,50 @@ const getErrorLabel = (status: VariantAnalysisRepoStatus.Failed | VariantAnalysi
   }
 };
 
-export const VariantAnalysisAnalyzedRepoItem = ({
+export type RepoRowProps = {
+  // Only fullName is required
+  repository: Partial<Repository> & Pick<Repository, 'fullName'>;
+  status?: VariantAnalysisRepoStatus;
+  resultCount?: number;
+
+  interpretedResults?: AnalysisAlert[];
+  rawResults?: AnalysisRawResults;
+}
+
+export const RepoRow = ({
   repository,
   status,
   resultCount,
   interpretedResults,
   rawResults,
-}: VariantAnalysisAnalyzedRepoItemProps) => {
+}: RepoRowProps) => {
   const [isExpanded, setExpanded] = useState(false);
 
   const toggleExpanded = useCallback(() => {
     setExpanded(oldIsExpanded => !oldIsExpanded);
   }, []);
 
-  const disabled = !isCompletedAnalysisRepoStatus(status);
+  const disabled = !status || !isCompletedAnalysisRepoStatus(status);
 
   return (
     <div>
       <TitleContainer onClick={toggleExpanded} disabled={disabled} aria-expanded={isExpanded}>
-        {isExpanded ? <ExpandCollapseCodicon name="chevron-down" label="Collapse" /> : <ExpandCollapseCodicon name="chevron-right" label="Expand" />}
+        <VSCodeCheckbox disabled />
+        {isExpanded ? <ExpandCollapseCodicon name="chevron-down" label="Collapse" /> :
+          <ExpandCollapseCodicon name="chevron-right" label="Expand" />}
         <VSCodeBadge>{resultCount === undefined ? '-' : formatDecimal(resultCount)}</VSCodeBadge>
         <span>{repository.fullName}</span>
-        <Visibility>{repository.private ? 'private' : 'public'}</Visibility>
+        <Visibility isPrivate={repository.private} />
         <span>
           {status === VariantAnalysisRepoStatus.Succeeded && <SuccessIcon />}
-          {(status === VariantAnalysisRepoStatus.Failed || status === VariantAnalysisRepoStatus.TimedOut || status === VariantAnalysisRepoStatus.Canceled) && <ErrorIcon label={getErrorLabel(status)} />}
+          {(status === VariantAnalysisRepoStatus.Failed || status === VariantAnalysisRepoStatus.TimedOut || status === VariantAnalysisRepoStatus.Canceled) &&
+            <ErrorIcon label={getErrorLabel(status)} />}
           {status === VariantAnalysisRepoStatus.InProgress && <LoadingIcon label="In progress" />}
+          {!status && <WarningIcon />}
         </span>
       </TitleContainer>
-      {isExpanded && <AnalyzedRepoItemContent status={status} interpretedResults={interpretedResults} rawResults={rawResults} />}
+      {isExpanded && status &&
+        <AnalyzedRepoItemContent status={status} interpretedResults={interpretedResults} rawResults={rawResults} />}
     </div>
   );
 };

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

@@ -1,13 +1,13 @@
 import * as React from 'react';
 import styled from 'styled-components';
 import { VariantAnalysis, VariantAnalysisScannedRepositoryResult } from '../../remote-queries/shared/variant-analysis';
-import { VariantAnalysisAnalyzedRepoItem } from './VariantAnalysisAnalyzedRepoItem';
+import { RepoRow } from './RepoRow';
 import { useMemo } from 'react';
 
 const Container = styled.div`
   display: flex;
   flex-direction: column;
-  gap: 1em;
+  gap: 0.5em;
 `;
 
 export type VariantAnalysisAnalyzedReposProps = {
@@ -33,7 +33,7 @@ export const VariantAnalysisAnalyzedRepos = ({
         const results = repositoryResultsById.get(repository.repository.id);
 
         return (
-          <VariantAnalysisAnalyzedRepoItem
+          <RepoRow
             key={repository.repository.id}
             repository={repository.repository}
             status={repository.analysisStatus}

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

@@ -2,7 +2,7 @@ import * as React from 'react';
 import styled from 'styled-components';
 import { VariantAnalysisSkippedRepositoryGroup } from '../../remote-queries/shared/variant-analysis';
 import { Alert } from '../common';
-import { VariantAnalysisSkippedRepositoryRow } from './VariantAnalysisSkippedRepositoryRow';
+import { RepoRow } from './RepoRow';
 
 export type VariantAnalysisSkippedRepositoriesTabProps = {
   alertTitle: string,
@@ -44,7 +44,7 @@ export const VariantAnalysisSkippedRepositoriesTab = ({
     <Container>
       {getSkipReasonAlert(alertTitle, alertMessage, skippedRepositoryGroup)}
       {skippedRepositoryGroup.repositories.map((repo) =>
-        <VariantAnalysisSkippedRepositoryRow key={`repo/${repo.fullName}`} repository={repo} />
+        <RepoRow key={`repo/${repo.fullName}`} repository={repo} />
       )}
     </Container>
   );

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

@@ -1,48 +0,0 @@
-import { VSCodeBadge, VSCodeCheckbox } from '@vscode/webview-ui-toolkit/react';
-import * as React from 'react';
-import styled from 'styled-components';
-import { Codicon, WarningIcon } from '../common';
-import { VariantAnalysisSkippedRepository as SkippedRepo } from '../../remote-queries/shared/variant-analysis';
-
-export type VariantAnalysisSkippedRepositoryRowProps = {
-  repository: SkippedRepo,
-};
-
-const Row = styled.div`
-  display: flex;
-  flex-direction: row;
-  gap: 0.5em;
-  align-items: center;
-`;
-
-const ChevronIcon = styled(Codicon)`
-  color: var(--vscode-disabledForeground);
-`;
-
-const PrivacyText = styled.span`
-  font-size: small;
-  color: var(--vscode-descriptionForeground);
-`;
-
-function getPrivacyElement(isPrivate: boolean | undefined) {
-  if (isPrivate === undefined) {
-    return undefined;
-  }
-  const text = isPrivate ? 'private' : 'public';
-  return <PrivacyText>{text}</PrivacyText>;
-}
-
-export const VariantAnalysisSkippedRepositoryRow = ({
-  repository,
-}: VariantAnalysisSkippedRepositoryRowProps) => {
-  return (
-    <Row>
-      <VSCodeCheckbox />
-      <ChevronIcon name='chevron-right' label='Expand' />
-      <VSCodeBadge>-</VSCodeBadge>
-      <span>{repository.fullName}</span>
-      {getPrivacyElement(repository.private)}
-      <WarningIcon />
-    </Row>
-  );
-};

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

@@ -1,16 +1,13 @@
 import * as React from 'react';
 import { render as reactRender, screen } from '@testing-library/react';
 import { VariantAnalysisRepoStatus } from '../../../remote-queries/shared/variant-analysis';
-import {
-  VariantAnalysisAnalyzedRepoItem,
-  VariantAnalysisAnalyzedRepoItemProps
-} from '../VariantAnalysisAnalyzedRepoItem';
 import userEvent from '@testing-library/user-event';
+import { RepoRow, RepoRowProps } from '../RepoRow';
 
-describe(VariantAnalysisAnalyzedRepoItem.name, () => {
-  const render = (props: Partial<VariantAnalysisAnalyzedRepoItemProps> = {}) => {
+describe(RepoRow.name, () => {
+  const render = (props: Partial<RepoRowProps> = {}) => {
     return reactRender(
-      <VariantAnalysisAnalyzedRepoItem
+      <RepoRow
         repository={{
           id: 1,
           fullName: 'octodemo/hello-world-1',
@@ -105,7 +102,17 @@ describe(VariantAnalysisAnalyzedRepoItem.name, () => {
     })).toBeEnabled();
   });
 
-  it('shows the repo as public', () => {
+  it('shows repository name', async () => {
+    render({
+      repository: {
+        fullName: 'octodemo/hello-world',
+      }
+    });
+
+    expect(screen.getByText('octodemo/hello-world')).toBeInTheDocument();
+  });
+
+  it('shows visibility when public', () => {
     render({
       repository: {
         id: 1,
@@ -117,7 +124,7 @@ describe(VariantAnalysisAnalyzedRepoItem.name, () => {
     expect(screen.getByText('public')).toBeInTheDocument();
   });
 
-  it('shows the repo as private', () => {
+  it('shows visibility when private', () => {
     render({
       repository: {
         id: 1,
@@ -129,6 +136,19 @@ describe(VariantAnalysisAnalyzedRepoItem.name, () => {
     expect(screen.getByText('private')).toBeInTheDocument();
   });
 
+  it('does not show visibility when unknown', () => {
+    render({
+      repository: {
+        id: undefined,
+        fullName: 'octodemo/hello-world-1',
+        private: undefined,
+      }
+    });
+
+    expect(screen.queryByText('public')).not.toBeInTheDocument();
+    expect(screen.queryByText('private')).not.toBeInTheDocument();
+  });
+
   it('can expand the repo item', async () => {
     render({
       status: VariantAnalysisRepoStatus.TimedOut,
@@ -143,4 +163,14 @@ describe(VariantAnalysisAnalyzedRepoItem.name, () => {
     });
     screen.getByText('Error: Timed out');
   });
+
+  it('does not allow expanding the repo item when status is undefined', async () => {
+    render({
+      status: undefined,
+    });
+
+    expect(screen.getByRole('button', {
+      expanded: false
+    })).toBeDisabled();
+  });
 });

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

@@ -1,53 +0,0 @@
-import * as React from 'react';
-import { render as reactRender, screen } from '@testing-library/react';
-import { VariantAnalysisSkippedRepositoryRow, VariantAnalysisSkippedRepositoryRowProps } from '../VariantAnalysisSkippedRepositoryRow';
-
-describe(VariantAnalysisSkippedRepositoryRow.name, () => {
-  const render = (props: VariantAnalysisSkippedRepositoryRowProps) =>
-    reactRender(<VariantAnalysisSkippedRepositoryRow {...props} />);
-
-  it('shows repository name', async () => {
-    render({
-      repository: {
-        fullName: 'octodemo/hello-world',
-      }
-    });
-
-    expect(screen.getByText('octodemo/hello-world')).toBeInTheDocument();
-  });
-
-  it('shows visibility when public', async () => {
-    render({
-      repository: {
-        fullName: 'octodemo/hello-world',
-        private: false,
-      }
-    });
-
-    expect(screen.getByText('public')).toBeInTheDocument();
-    expect(screen.queryByText('private')).not.toBeInTheDocument();
-  });
-
-  it('shows visibility when private', async () => {
-    render({
-      repository: {
-        fullName: 'octodemo/hello-world',
-        private: true,
-      }
-    });
-
-    expect(screen.queryByText('public')).not.toBeInTheDocument();
-    expect(screen.getByText('private')).toBeInTheDocument();
-  });
-
-  it('does not show visibility when unknown', async () => {
-    render({
-      repository: {
-        fullName: 'octodemo/hello-world',
-      }
-    });
-
-    expect(screen.queryByText('public')).not.toBeInTheDocument();
-    expect(screen.queryByText('private')).not.toBeInTheDocument();
-  });
-});