Skip to content

Commit

Permalink
fix: don't filter single runs in the comparison view (#9789)
Browse files Browse the repository at this point in the history
(cherry picked from commit cdbbedd)
  • Loading branch information
ashtonG authored and determined-ci committed Aug 5, 2024
1 parent 0b89fad commit 1841a8e
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 32 deletions.
4 changes: 2 additions & 2 deletions webui/react/src/components/FilterForm/components/type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ export type FilterFormSet = {
showArchived: boolean;
};

type FormFieldWithoutId = Omit<FormField, 'id'>;
export type FormFieldWithoutId = Omit<FormField, 'id'>;

type FormGroupWithoutId = {
export type FormGroupWithoutId = {
readonly kind: typeof FormKind.Group;
conjunction: Conjunction;
children: (FormGroupWithoutId | FormFieldWithoutId)[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,23 @@ describe('RunFilterInterstitialModalComponent', () => {
expect(filterFormSetString).toBeDefined();
const filterFormSet = JSON.parse(filterFormSetString || '');

// TODO: is there a better way to test this expectation?
// TODO: is there a better way to test these expectations?
expect(filterFormSet.showArchived).toBeTruthy();
const [filterGroup, idFilterGroup] = filterFormSet.filterGroup.children?.[0].children || [];
expect(filterGroup).toEqual(expectedFilterGroup);

const idFilters = idFilterGroup.children;
expect(idFilters.every((f: FormField) => f.operator === '!=')).toBeTruthy();
expect(idFilters.map((f: FormField) => f.value)).toEqual(expectedExclusions);
const [, , idFilter] = filterFormSet.filterGroup.children;
for (const child of expectedFilterGroup.children) {
expect(filterFormSet.filterGroup.children).toContainEqual(child);
}

for (const exclusion of expectedExclusions) {
expect(idFilter?.children[0].children).toContainEqual({
columnName: 'id',
kind: 'field',
location: 'LOCATION_TYPE_RUN',
operator: '!=',
type: 'COLUMN_TYPE_NUMBER',
value: exclusion,
});
}
});

it('calls server with filter describing visual selection', () => {
Expand All @@ -139,7 +148,7 @@ describe('RunFilterInterstitialModalComponent', () => {
const filterFormSet = JSON.parse(filterFormSetString || '');

expect(filterFormSet.showArchived).toBe(false);
const idFilters = filterFormSet.filterGroup.children?.[0].children || [];
const idFilters = filterFormSet.filterGroup.children || [];
expect(idFilters.every((f: FormField) => f.operator === '=')).toBe(true);
expect(idFilters.map((f: FormField) => f.value)).toEqual(expectedSelection);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { useAsync } from 'hooks/useAsync';
import { searchRuns } from 'services/api';
import { SelectionType } from 'types';
import { DetError } from 'utils/error';
import { combine } from 'utils/filterFormSet';
import { getIdsFilter } from 'utils/flatRun';
import mergeAbortControllers from 'utils/mergeAbortControllers';
import { observable } from 'utils/observable';
Expand Down Expand Up @@ -77,7 +78,22 @@ export const RunFilterInterstitialModalComponent = forwardRef<ControlledModalRef
async (canceler) => {
if (!isOpen) return NotLoaded;
const mergedCanceler = mergeAbortControllers(canceler, closeController.current);
const filter = getIdsFilter(filterFormSet, selection);
const filterWithSingleFilter = combine(filterFormSet.filterGroup, 'and', {
columnName: 'searcherType',
kind: 'field',
location: 'LOCATION_TYPE_RUN',
operator: '!=',
type: 'COLUMN_TYPE_TEXT',
value: 'single',
});
const filter: FilterFormSetWithoutId = getIdsFilter(
{
...filterFormSet,
filterGroup: filterWithSingleFilter,
},
selection,
);

try {
const results = await searchRuns(
{
Expand Down
28 changes: 28 additions & 0 deletions webui/react/src/utils/filterFormSet.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import {
Conjunction,
FormFieldWithoutId,
FormGroupWithoutId,
} from 'components/FilterForm/components/type';

/**
* build a new filter group given an existing one and a child. will add the child
* as a child of the current formGroup if the conjunction matches, otherwise
* will create a new wrapper group having both as children
*/
export const combine = (
filterGroup: FormGroupWithoutId,
conjunction: Conjunction,
child: FormGroupWithoutId | FormFieldWithoutId,
): FormGroupWithoutId => {
if (filterGroup.conjunction === conjunction) {
return {
...filterGroup,
children: [...filterGroup.children, child],
};
}
return {
children: [filterGroup, child],
conjunction,
kind: 'group',
};
};
28 changes: 7 additions & 21 deletions webui/react/src/utils/flatRun.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import {
import { PermissionsHook } from 'hooks/usePermissions';
import { FlatRun, FlatRunAction, RunState, SelectionType } from 'types';

import { combine } from './filterFormSet';

type FlatRunChecker = (flatRun: Readonly<FlatRun>) => boolean;

type FlatRunPermissionSet = Pick<
Expand Down Expand Up @@ -89,12 +91,11 @@ const idToFilter = (operator: Operator, id: number) =>
export const getIdsFilter = (
filterFormSet: FilterFormSetWithoutId,
selection: SelectionType,
): FilterFormSetWithoutId | undefined => {
): FilterFormSetWithoutId => {
const filterGroup: FilterFormSetWithoutId['filterGroup'] =
selection.type === 'ALL_EXCEPT'
? {
? combine(filterFormSet.filterGroup, 'and', {
children: [
filterFormSet.filterGroup,
{
children: selection.exclusions.map(idToFilter.bind(this, '!=')),
conjunction: 'and',
Expand All @@ -103,30 +104,15 @@ export const getIdsFilter = (
],
conjunction: 'and',
kind: 'group',
}
})
: {
children: selection.selections.map(idToFilter.bind(this, '=')),
conjunction: 'or',
kind: 'group',
};

const filter: FilterFormSetWithoutId = {
return {
...filterFormSet,
filterGroup: {
children: [
filterGroup,
{
columnName: 'searcherType',
kind: 'field',
location: 'LOCATION_TYPE_RUN',
operator: '!=',
type: 'COLUMN_TYPE_TEXT',
value: 'single',
} as const,
],
conjunction: 'and',
kind: 'group',
},
filterGroup,
};
return filter;
};

0 comments on commit 1841a8e

Please sign in to comment.