Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Repository } from './repository';
import { AnalysisAlert, AnalysisRawResults } from './analysis-result';

export interface VariantAnalysis {
id: number,
Expand Down Expand Up @@ -78,6 +79,12 @@ export interface VariantAnalysisSkippedRepositoryGroup {
}>
}

export interface VariantAnalysisScannedRepositoryResult {
repositoryId: number;
interpretedResults?: AnalysisAlert[];
rawResults?: AnalysisRawResults;
}

Comment on lines +82 to +87
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very take-it-or-leave-it suggestion, but instead of having two optional fields, could we encode in the type system that we expect precisely one of them to be defined? Maybe something like the following:

Suggested change
export interface VariantAnalysisScannedRepositoryResult {
repositoryId: number;
interpretedResults?: AnalysisAlert[];
rawResults?: AnalysisRawResults;
}
export type VariantAnalysisScannedRepositoryResult = { repositoryId: number } & (VariantAnalysisInterpretedScannedRepositoryResult | VariantAnalysisRawScannedRepositoryResult);
export interface VariantAnalysisInterpretedScannedRepositoryResult {
resultsType: 'interpreted';
interpretedResults: AnalysisAlert[];
}
export interface VariantAnalysisRawScannedRepositoryResult {
resultsType: 'raw';
rawResults: AnalysisRawResults;
}

Then later on you can do things like if (results?.resultsType === 'interpreted') { // use results.interpretedResults for something }

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that would make it more explicit. I don't think we even need the resultsType to make it explicit that only one of the two is defined.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, now that I try implementing it, I think it might make it harder to pass down props. For example, it would make this much harder:

interpretedResults={results?.interpretedResults}
rawResults={results?.rawResults}

I would ideally like to keep those components independent from the exact structure of the VariantAnalysisScannedRepositoryResult, so I think I'll leave it like this for now.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I tried it out I changed those props to instead take a VariantAnalysisScannedRepositoryResult, but I agree perhaps that's making them a bit too tied to that type. So keeping things as they are is perfectly fine 👍

/**
* Captures information needed to submit a variant
* analysis for processing.
Expand All @@ -102,16 +109,24 @@ export interface VariantAnalysisSubmission {
}

/**
* @param repo
* @returns whether the repo scan is in a completed state, i.e. it cannot normally change state anymore
* @param status
* @returns whether the status is in a completed state, i.e. it cannot normally change state anymore
*/
export function hasRepoScanCompleted(repo: VariantAnalysisScannedRepository): boolean {
export function isCompletedAnalysisRepoStatus(status: VariantAnalysisRepoStatus): 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);
].includes(status);
}

/**
* @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 isCompletedAnalysisRepoStatus(repo.analysisStatus);
}

/**
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
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';

export default {
title: 'Variant Analysis/Analyzed Repo Item',
component: VariantAnalysisAnalyzedRepoItem,
decorators: [
(Story) => (
<VariantAnalysisContainer>
<Story />
</VariantAnalysisContainer>
)
],
} as ComponentMeta<typeof VariantAnalysisAnalyzedRepoItem>;

const Template: ComponentStory<typeof VariantAnalysisAnalyzedRepoItem> = (args) => (
<VariantAnalysisAnalyzedRepoItem {...args} />
);

export const Pending = Template.bind({});
Pending.args = {
repository: {
id: 63537249,
fullName: 'facebook/create-react-app',
private: false,
},
status: VariantAnalysisRepoStatus.Pending,
};

export const InProgress = Template.bind({});
InProgress.args = {
...Pending.args,
status: VariantAnalysisRepoStatus.InProgress,
interpretedResults: undefined,
};

export const Failed = Template.bind({});
Failed.args = {
...Pending.args,
status: VariantAnalysisRepoStatus.Failed,
interpretedResults: undefined,
};

export const TimedOut = Template.bind({});
TimedOut.args = {
...Pending.args,
status: VariantAnalysisRepoStatus.TimedOut,
};

export const Canceled = Template.bind({});
Canceled.args = {
...Pending.args,
status: VariantAnalysisRepoStatus.Canceled,
};

export const InterpretedResults = Template.bind({});
InterpretedResults.args = {
...Pending.args,
status: VariantAnalysisRepoStatus.Succeeded,
resultCount: 198,
interpretedResults: analysesResults.analysesResults.find(v => v.nwo === 'facebook/create-react-app')?.interpretedResults as unknown as AnalysisAlert[],
};

export const RawResults = Template.bind({});
RawResults.args = {
...InterpretedResults.args,
interpretedResults: undefined,
resultCount: 1,
rawResults: rawResults as unknown as AnalysisRawResults,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
import React from 'react';

import { ComponentMeta, ComponentStory } from '@storybook/react';

import { VariantAnalysisContainer } from '../../view/variant-analysis/VariantAnalysisContainer';
import { VariantAnalysisAnalyzedRepos } from '../../view/variant-analysis/VariantAnalysisAnalyzedRepos';
import {
VariantAnalysisQueryLanguage,
VariantAnalysisRepoStatus,
VariantAnalysisStatus
} from '../../remote-queries/shared/variant-analysis';
import { AnalysisAlert } from '../../remote-queries/shared/analysis-result';

import analysesResults from '../remote-queries/data/analysesResultsMessage.json';

export default {
title: 'Variant Analysis/Analyzed Repos',
component: VariantAnalysisAnalyzedRepos,
decorators: [
(Story) => (
<VariantAnalysisContainer>
<Story />
</VariantAnalysisContainer>
)
],
} as ComponentMeta<typeof VariantAnalysisAnalyzedRepos>;

const Template: ComponentStory<typeof VariantAnalysisAnalyzedRepos> = (args) => (
<VariantAnalysisAnalyzedRepos {...args} />
);

const interpretedResultsForRepo = (nwo: string): AnalysisAlert[] | undefined => {
return analysesResults.analysesResults.find(v => v.nwo === nwo)?.interpretedResults as unknown as AnalysisAlert[];
};

export const Example = Template.bind({});
Example.args = {
variantAnalysis: {
id: 1,
controllerRepoId: 1,
query: {
name: 'Query name',
filePath: 'example.ql',
language: VariantAnalysisQueryLanguage.Javascript,
},
databases: {},
status: VariantAnalysisStatus.InProgress,
scannedRepos: [
{
repository: {
id: 63537249,
fullName: 'facebook/create-react-app',
private: false,
},
analysisStatus: VariantAnalysisRepoStatus.Succeeded, resultCount: 198,
},
{
repository: {
id: 167174,
fullName: 'jquery/jquery',
private: false,
},
analysisStatus: VariantAnalysisRepoStatus.Succeeded,
resultCount: 67,
},
{
repository: {
id: 237159,
fullName: 'expressjs/express',
private: false,
},
analysisStatus: VariantAnalysisRepoStatus.Succeeded,
resultCount: 26,
},
{
repository: {
id: 15062869,
fullName: 'facebook/jest',
private: false,
},
analysisStatus: VariantAnalysisRepoStatus.Failed,
},
{
repository: {
id: 24195339,
fullName: 'angular/angular',
private: false,
},
analysisStatus: VariantAnalysisRepoStatus.InProgress,
},
{
repository: {
id: 24560307,
fullName: 'babel/babel',
private: false,
},
analysisStatus: VariantAnalysisRepoStatus.Pending,
},
]
},
repositoryResults: [
{
repositoryId: 63537249,
interpretedResults: interpretedResultsForRepo('facebook/create-react-app'),
},
{
repositoryId: 167174,
interpretedResults: interpretedResultsForRepo('jquery/jquery'),
},
{
repositoryId: 237159,
interpretedResults: interpretedResultsForRepo('expressjs/express'),
}
]
}
;
13 changes: 13 additions & 0 deletions extensions/ql-vscode/src/view/common/icon/LoadingIcon.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import * as React from 'react';
import { Codicon } from './Codicon';
import classNames from 'classnames';

type Props = {
label?: string;
className?: string;
}

export const LoadingIcon = ({
label = 'Loading...',
className,
}: Props) => <Codicon name="loading" label={label} className={classNames(className, 'codicon-modifier-spin')} />;
1 change: 1 addition & 0 deletions extensions/ql-vscode/src/view/common/icon/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export * from './Codicon';
export * from './ErrorIcon';
export * from './LoadingIcon';
export * from './SuccessIcon';
export * from './WarningIcon';
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import * as React from 'react';
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 { Alert } from '../common';

const ContentContainer = styled.div`
display: flex;
flex-direction: column;
`;

const AlertContainer = styled.div`
margin-top: 1em;
`;

const InterpretedResultsContainer = styled.ul`
list-style-type: none;
margin: 1em 0 0;
padding: 0.5em 0 0 0;
`;

const InterpretedResultItem = styled.li`
margin-bottom: 1em;
background-color: var(--vscode-notifications-background);
`;

const RawResultsContainer = styled.div`
display: block;
margin-top: 0.5em;
`;

export type AnalyzedRepoItemContentProps = {
status: VariantAnalysisRepoStatus;

interpretedResults?: AnalysisAlert[];
rawResults?: AnalysisRawResults;
}

export const AnalyzedRepoItemContent = ({
status,
interpretedResults,
rawResults,
}: AnalyzedRepoItemContentProps) => {
return (
<ContentContainer>
{status === VariantAnalysisRepoStatus.Failed && <AlertContainer>
<Alert
type="error"
title="Failed"
message="The query failed to run on this repository."
/>
</AlertContainer>}
{status === VariantAnalysisRepoStatus.TimedOut && <AlertContainer>
<Alert
type="error"
title="Timed out"
message="The analysis ran out of time and we couldn't scan the repository."
/>
</AlertContainer>}
{status === VariantAnalysisRepoStatus.Canceled && <AlertContainer>
<Alert
type="error"
title="Canceled"
message="The variant analysis or this repository was canceled."
/>
</AlertContainer>}
{interpretedResults && (
<InterpretedResultsContainer>
{interpretedResults.map((r, i) =>
<InterpretedResultItem key={i}>
<AnalysisAlertResult alert={r} />
</InterpretedResultItem>)}
</InterpretedResultsContainer>
)}
{rawResults && (
<RawResultsContainer>
<RawResultsTable
schema={rawResults.schema}
results={rawResults.resultSet}
fileLinkPrefix={rawResults.fileLinkPrefix}
sourceLocationPrefix={rawResults.sourceLocationPrefix} />
</RawResultsContainer>
)}
</ContentContainer>
);
};
Loading