Merge pull request #1729 from github/koesie10/failed-download-view

Handle failed result download in view

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

@@ -77,6 +77,22 @@ SucceededDownloading.args = {
   downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.InProgress,
 };
 
+export const SucceededSuccessfulDownload = Template.bind({});
+SucceededSuccessfulDownload.args = {
+  ...Pending.args,
+  status: VariantAnalysisRepoStatus.Succeeded,
+  resultCount: 198,
+  downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.Succeeded,
+};
+
+export const SucceededFailedDownload = Template.bind({});
+SucceededFailedDownload.args = {
+  ...Pending.args,
+  status: VariantAnalysisRepoStatus.Succeeded,
+  resultCount: 198,
+  downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.Failed,
+};
+
 export const InterpretedResults = Template.bind({});
 InterpretedResults.args = {
   ...Pending.args,

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

@@ -3,7 +3,10 @@ import styled from 'styled-components';
 import { AnalysisAlert, AnalysisRawResults } from '../../remote-queries/shared/analysis-result';
 import AnalysisAlertResult from '../remote-queries/AnalysisAlertResult';
 import RawResultsTable from '../remote-queries/RawResultsTable';
-import { VariantAnalysisRepoStatus } from '../../remote-queries/shared/variant-analysis';
+import {
+  VariantAnalysisRepoStatus,
+  VariantAnalysisScannedRepositoryDownloadStatus,
+} from '../../remote-queries/shared/variant-analysis';
 import { Alert } from '../common';
 
 const ContentContainer = styled.div`
@@ -32,7 +35,8 @@ const RawResultsContainer = styled.div`
 `;
 
 export type AnalyzedRepoItemContentProps = {
-  status: VariantAnalysisRepoStatus;
+  status?: VariantAnalysisRepoStatus;
+  downloadStatus?: VariantAnalysisScannedRepositoryDownloadStatus;
 
   interpretedResults?: AnalysisAlert[];
   rawResults?: AnalysisRawResults;
@@ -40,6 +44,7 @@ export type AnalyzedRepoItemContentProps = {
 
 export const AnalyzedRepoItemContent = ({
   status,
+  downloadStatus,
   interpretedResults,
   rawResults,
 }: AnalyzedRepoItemContentProps) => {
@@ -66,6 +71,13 @@ export const AnalyzedRepoItemContent = ({
           message="The variant analysis or this repository was canceled."
         />
       </AlertContainer>}
+      {downloadStatus === VariantAnalysisScannedRepositoryDownloadStatus.Failed && <AlertContainer>
+        <Alert
+          type="error"
+          title="Download failed"
+          message="The query was successful on this repository, but the extension failed to download the results for this repository."
+        />
+      </AlertContainer>}
       {interpretedResults && (
         <InterpretedResultsContainer>
           {interpretedResults.map((r, i) =>

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

@@ -82,6 +82,50 @@ export type RepoRowProps = {
   rawResults?: AnalysisRawResults;
 }
 
+const canExpand = (
+  status: VariantAnalysisRepoStatus | undefined,
+  downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus | undefined,
+): boolean => {
+  if (!status) {
+    return false;
+  }
+
+  if (!isCompletedAnalysisRepoStatus(status)) {
+    return false;
+  }
+
+  if (status !== VariantAnalysisRepoStatus.Succeeded) {
+    return true;
+  }
+
+  return downloadStatus === VariantAnalysisScannedRepositoryDownloadStatus.Succeeded || downloadStatus === VariantAnalysisScannedRepositoryDownloadStatus.Failed;
+};
+
+const isExpandableContentLoaded = (
+  status: VariantAnalysisRepoStatus | undefined,
+  downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus | undefined,
+  resultsLoaded: boolean,
+): boolean => {
+  if (!canExpand(status, downloadStatus)) {
+    return false;
+  }
+
+  if (!status) {
+    return false;
+  }
+
+  if (status !== VariantAnalysisRepoStatus.Succeeded) {
+    return true;
+  }
+
+  if (downloadStatus === VariantAnalysisScannedRepositoryDownloadStatus.Failed) {
+    // If the download has failed, we allow expansion to show the error
+    return true;
+  }
+
+  return resultsLoaded;
+};
+
 export const RepoRow = ({
   repository,
   status,
@@ -99,7 +143,7 @@ export const RepoRow = ({
       return;
     }
 
-    if (resultsLoaded || status !== VariantAnalysisRepoStatus.Succeeded) {
+    if (resultsLoaded || status !== VariantAnalysisRepoStatus.Succeeded || downloadStatus !== VariantAnalysisScannedRepositoryDownloadStatus.Succeeded) {
       setExpanded(oldIsExpanded => !oldIsExpanded);
       return;
     }
@@ -110,7 +154,7 @@ export const RepoRow = ({
     });
 
     setResultsLoading(true);
-  }, [resultsLoading, resultsLoaded, repository.fullName, status]);
+  }, [resultsLoading, resultsLoaded, repository.fullName, status, downloadStatus]);
 
   useEffect(() => {
     if (resultsLoaded && resultsLoading) {
@@ -119,8 +163,8 @@ export const RepoRow = ({
     }
   }, [resultsLoaded, resultsLoading]);
 
-  const disabled = !status || !isCompletedAnalysisRepoStatus(status) || (status === VariantAnalysisRepoStatus.Succeeded && downloadStatus !== VariantAnalysisScannedRepositoryDownloadStatus.Succeeded);
-  const expandableContentLoaded = status && (status !== VariantAnalysisRepoStatus.Succeeded || resultsLoaded);
+  const disabled = !canExpand(status, downloadStatus);
+  const expandableContentLoaded = isExpandableContentLoaded(status, downloadStatus, resultsLoaded);
 
   return (
     <div>
@@ -139,13 +183,20 @@ export const RepoRow = ({
           {!status && <WarningIcon />}
         </span>
         {downloadStatus === VariantAnalysisScannedRepositoryDownloadStatus.InProgress && <LoadingIcon label="Downloading" />}
+        {downloadStatus === VariantAnalysisScannedRepositoryDownloadStatus.Failed && <WarningIcon label="Failed to download the results" />}
         <MetadataContainer>
           <div><StarCount starCount={repository.stargazersCount} /></div>
           <LastUpdated lastUpdated={repository.updatedAt} />
         </MetadataContainer>
       </TitleContainer>
-      {isExpanded && expandableContentLoaded &&
-        <AnalyzedRepoItemContent status={status} interpretedResults={interpretedResults} rawResults={rawResults} />}
+      {isExpanded && expandableContentLoaded && (
+        <AnalyzedRepoItemContent
+          status={status}
+          downloadStatus={downloadStatus}
+          interpretedResults={interpretedResults}
+          rawResults={rawResults}
+        />
+      )}
     </div>
   );
 };

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

@@ -1,6 +1,9 @@
 import * as React from 'react';
 import { render as reactRender, screen } from '@testing-library/react';
-import { VariantAnalysisRepoStatus } from '../../../remote-queries/shared/variant-analysis';
+import {
+  VariantAnalysisRepoStatus,
+  VariantAnalysisScannedRepositoryDownloadStatus
+} from '../../../remote-queries/shared/variant-analysis';
 import { AnalyzedRepoItemContent, AnalyzedRepoItemContentProps } from '../AnalyzedRepoItemContent';
 
 describe(AnalyzedRepoItemContent.name, () => {
@@ -112,4 +115,13 @@ describe(AnalyzedRepoItemContent.name, () => {
 
     expect(screen.getByText('Error: Canceled')).toBeInTheDocument();
   });
+
+  it('renders the failed download state', () => {
+    render({
+      status: VariantAnalysisRepoStatus.Succeeded,
+      downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.Failed,
+    });
+
+    expect(screen.getByText('Error: Download failed')).toBeInTheDocument();
+  });
 });

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

@@ -104,6 +104,21 @@ describe(RepoRow.name, () => {
     })).toBeEnabled();
   });
 
+  it('renders the succeeded state with failed download status', () => {
+    render({
+      status: VariantAnalysisRepoStatus.Succeeded,
+      resultCount: 178,
+      downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.Failed,
+    });
+
+    expect(screen.getByRole<HTMLButtonElement>('button', {
+      expanded: false
+    })).toBeEnabled();
+    expect(screen.getByRole('img', {
+      name: 'Failed to download the results',
+    })).toBeInTheDocument();
+  });
+
   it('renders the failed state', () => {
     render({
       status: VariantAnalysisRepoStatus.Failed,