Split filter and sort methods and simplify type checks

Author: koesie10

extensions/ql-vscode/src/pure/variant-analysis-filter-sort.ts

@@ -76,22 +76,26 @@ export function compareWithResults(filterSortState: RepositoriesFilterSortState
   };
 }
 
-function isFilterOnRepositoryIds(filterSortState: RepositoriesFilterSortState | RepositoriesFilterSortStateWithIds | undefined): filterSortState is RepositoriesFilterSortStateWithIds & Required<Pick<RepositoriesFilterSortStateWithIds, 'repositoryIds'>> {
-  return !!filterSortState && 'repositoryIds' in filterSortState && filterSortState.repositoryIds !== undefined && filterSortState.repositoryIds.length > 0;
-}
+export function filterAndSortRepositoriesWithResultsByName<T extends SortableResult>(repositories: T[] | undefined, filterSortState: RepositoriesFilterSortState | undefined): T[] | undefined {
+  if (!repositories) {
+    return undefined;
+  }
 
-// These define the behavior for undefined input values
-export function filterAndSortRepositoriesWithResults<T extends SortableResult>(repositories: T[], filterSortState: RepositoriesFilterSortState | RepositoriesFilterSortStateWithIds | undefined): T[];
-export function filterAndSortRepositoriesWithResults<T extends SortableResult>(repositories: T[] | undefined, filterSortState: RepositoriesFilterSortState | RepositoriesFilterSortStateWithIds | undefined): T[] | undefined;
+  return repositories
+    .filter(repo => matchesFilter(repo.repository, filterSortState))
+    .sort(compareWithResults(filterSortState));
+}
 
-export function filterAndSortRepositoriesWithResults<T extends SortableResult>(repositories: T[] | undefined, filterSortState: RepositoriesFilterSortState | RepositoriesFilterSortStateWithIds | undefined): T[] | undefined {
+export function filterAndSortRepositoriesWithResults<T extends SortableResult>(repositories: T[] | undefined, filterSortState: RepositoriesFilterSortStateWithIds | undefined): T[] | undefined {
   if (!repositories) {
     return undefined;
   }
 
-  const filteredRepositories = isFilterOnRepositoryIds(filterSortState) ?
-    repositories.filter(repo => filterSortState.repositoryIds.includes(repo.repository.id)) :
-    repositories.filter(repo => matchesFilter(repo.repository, filterSortState));
+  if (filterSortState?.repositoryIds && filterSortState.repositoryIds.length > 0) {
+    return repositories
+      .filter(repo => filterSortState.repositoryIds?.includes(repo.repository.id))
+      .sort(compareWithResults(filterSortState));
+  }
 
-  return filteredRepositories.sort(compareWithResults(filterSortState));
+  return filterAndSortRepositoriesWithResultsByName(repositories, filterSortState);
 }

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

@@ -379,9 +379,9 @@ export class VariantAnalysisManager extends DisposableObject implements VariantA
       throw new Error(`No variant analysis with id: ${variantAnalysisId}`);
     }
 
-    const filteredRepositories = filterAndSortRepositoriesWithResults(variantAnalysis.scannedRepos ?? [], filterSort);
+    const filteredRepositories = filterAndSortRepositoriesWithResults(variantAnalysis.scannedRepos, filterSort);
 
-    const fullNames = filteredRepositories.filter(a => a.resultCount && a.resultCount > 0).map(a => a.repository.fullName);
+    const fullNames = filteredRepositories?.filter(a => a.resultCount && a.resultCount > 0).map(a => a.repository.fullName);
     if (!fullNames || fullNames.length === 0) {
       return;
     }

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

@@ -8,7 +8,7 @@ import {
   VariantAnalysisScannedRepositoryState
 } from '../../remote-queries/shared/variant-analysis';
 import {
-  filterAndSortRepositoriesWithResults,
+  filterAndSortRepositoriesWithResultsByName,
   RepositoriesFilterSortState,
 } from '../../pure/variant-analysis-filter-sort';
 
@@ -55,7 +55,7 @@ export const VariantAnalysisAnalyzedRepos = ({
   }, [repositoryResults]);
 
   const repositories = useMemo(() => {
-    return filterAndSortRepositoriesWithResults(variantAnalysis.scannedRepos, filterSortState);
+    return filterAndSortRepositoriesWithResultsByName(variantAnalysis.scannedRepos, filterSortState);
   }, [filterSortState, variantAnalysis.scannedRepos]);
 
   const onSelectedChange = useCallback((repositoryId: number, selected: boolean) => {

extensions/ql-vscode/src/view/variant-analysis/__tests__/filterSort.spec.ts

@@ -3,6 +3,7 @@ import {
   compareWithResults,
   defaultFilterSortState,
   filterAndSortRepositoriesWithResults,
+  filterAndSortRepositoriesWithResultsByName,
   matchesFilter,
   SortKey,
 } from '../../../pure/variant-analysis-filter-sort';
@@ -294,6 +295,58 @@ describe(compareWithResults.name, () => {
   });
 });
 
+describe(filterAndSortRepositoriesWithResultsByName.name, () => {
+  const repositories = [
+    {
+      repository: {
+        id: 10,
+        fullName: 'github/galaxy',
+      },
+      resultCount: 10,
+    },
+    {
+      repository: {
+        id: 11,
+        fullName: 'github/world',
+      },
+      resultCount: undefined,
+    },
+    {
+      repository: {
+        id: 13,
+        fullName: 'github/planet',
+      },
+      resultCount: 500,
+    },
+    {
+      repository: {
+        id: 783532,
+        fullName: 'github/stars',
+      },
+      resultCount: 8000,
+    }
+  ];
+
+  describe('when sort key is given without filter', () => {
+    it('returns the correct results', () => {
+      expect(filterAndSortRepositoriesWithResultsByName(repositories, {
+        ...defaultFilterSortState,
+        sortKey: SortKey.ResultsCount,
+      })).toEqual([repositories[3], repositories[2], repositories[0], repositories[1]]);
+    });
+  });
+
+  describe('when sort key and search filter are given', () => {
+    it('returns the correct results', () => {
+      expect(filterAndSortRepositoriesWithResultsByName(repositories, {
+        ...defaultFilterSortState,
+        sortKey: SortKey.ResultsCount,
+        searchValue: 'la',
+      })).toEqual([repositories[2], repositories[0]]);
+    });
+  });
+});
+
 describe(filterAndSortRepositoriesWithResults.name, () => {
   const repositories = [
     {