Skip to content

Commit

Permalink
fix: improve sorting for multi-type columns (#10123)
Browse files Browse the repository at this point in the history
  • Loading branch information
ashtonG authored and thiagodallacqua-hpe committed Oct 31, 2024
1 parent 918d6c7 commit 998dcb9
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 13 deletions.
63 changes: 58 additions & 5 deletions webui/react/src/components/MultiSortMenu.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
import Badge from 'hew/Badge';
import Button from 'hew/Button';
import { DirectionType, Sort, validSort } from 'hew/DataGrid/DataGrid';
import Dropdown, { MenuItem } from 'hew/Dropdown';
import Icon from 'hew/Icon';
import Select, { Option } from 'hew/Select';
import { Loadable } from 'hew/utils/loadable';
import { groupBy, mapValues } from 'lodash';
import { Fragment, useMemo } from 'react';

import { runColumns } from 'pages/FlatRuns/columns';
import { V1ColumnType } from 'services/api-ts-sdk';
import { ProjectColumn } from 'types';
import { removeColumnTypePrefix } from 'utils/flatRun';

import css from './MultiSortMenu.module.scss';

Expand All @@ -25,6 +30,7 @@ interface MultiSortRowProps {
onChange?: (sort: Sort) => void;
onRemove?: () => void;
bannedSortColumns: Set<string>;
typeMap: Record<string, V1ColumnType[]>;
}
interface DirectionOptionsProps {
onChange?: (direction: DirectionType) => void;
Expand All @@ -36,6 +42,7 @@ interface ColumnOptionsProps {
onChange?: (column: string) => void;
value?: string;
bannedSortColumns: Set<string>;
typeMap: Record<string, V1ColumnType[]>;
}

export const optionsByColumnType = {
Expand Down Expand Up @@ -153,6 +160,7 @@ const ColumnOptions: React.FC<ColumnOptionsProps> = ({
columns,
value,
bannedSortColumns,
typeMap,
}) => (
<Select
autoFocus
Expand All @@ -161,10 +169,31 @@ const ColumnOptions: React.FC<ColumnOptionsProps> = ({
loading={Loadable.isNotLoaded(columns)}
options={Loadable.getOrElse([], columns)
.filter((c) => !bannedSortColumns.has(c.column))
.map((c) => ({
label: c.displayName || c.column,
value: c.column,
}))}
.map((c) => {
const badges = typeMap[c.column].map((type) => {
const copy =
(runColumns as readonly string[]).includes(c.column) &&
type === V1ColumnType.UNSPECIFIED
? 'BOOLEAN'
: removeColumnTypePrefix(type);
return (
<Fragment key={type}>
<Badge text={copy} />{' '}
</Fragment>
);
});
const title = c.displayName || c.column;
const label = (
<>
{title} {badges}
</>
);
return {
label,
title,
value: c.column,
};
})}
placeholder="Select column"
value={value}
width="100%"
Expand All @@ -178,6 +207,7 @@ const MultiSortRow: React.FC<MultiSortRowProps> = ({
onChange,
onRemove,
bannedSortColumns,
typeMap,
}) => {
const valueType =
Loadable.getOrElse([], columns).find((c) => c.column === sort.column)?.type ||
Expand All @@ -188,6 +218,7 @@ const MultiSortRow: React.FC<MultiSortRowProps> = ({
<ColumnOptions
bannedSortColumns={bannedSortColumns}
columns={columns}
typeMap={typeMap}
value={sort.column}
onChange={(column) => onChange?.({ ...sort, column })}
/>
Expand Down Expand Up @@ -231,14 +262,35 @@ const MultiSort: React.FC<MultiSortProps> = ({ sorts, columns, onChange, bannedS
window.document.body.dispatchEvent(new Event('mousedown', { bubbles: true }));
onChange?.([EMPTY_SORT]);
};
// replace duplicate columns with a single unspecified column for copy
// reasons. maintain types so we can display in the select
const [mergedColumns, typeMap] = useMemo(() => {
const loadableTuple = columns.map((c) => {
const columnGroups = groupBy(c, 'column');
const fixedColumns = Object.values(columnGroups).flatMap((group) => {
if (group.length > 1) {
return [
{
...group[0],
type: V1ColumnType.UNSPECIFIED,
},
];
}
return group;
});
const typeMap = mapValues(columnGroups, (group) => group.map((column) => column.type));
return [fixedColumns, typeMap] as const;
});
return [loadableTuple.map((l) => l[0]), loadableTuple.map((l) => l[1]).getOrElse({})];
}, [columns]);

return (
<div className={css.base} data-test-component="multiSort">
<div>{SORT_MENU_TITLE}</div>
<div className={css.rows} data-test="rows">
{sorts.map((sort, idx) => {
const seenColumns = sorts.slice(0, idx).map((s) => s.column);
const columnOptions = Loadable.map(columns, (cols) =>
const columnOptions = mergedColumns.map((cols) =>
cols.filter((c) => !seenColumns.includes(c.column)),
);
return (
Expand All @@ -247,6 +299,7 @@ const MultiSort: React.FC<MultiSortProps> = ({ sorts, columns, onChange, bannedS
columns={columnOptions}
key={sort.column || idx}
sort={sort}
typeMap={typeMap}
onChange={makeOnRowChange(idx)}
onRemove={makeOnRowRemove(idx)}
/>
Expand Down
34 changes: 26 additions & 8 deletions webui/react/src/pages/FlatRuns/FlatRuns.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import Pagination from 'hew/Pagination';
import Row from 'hew/Row';
import { useToast } from 'hew/Toast';
import { Loadable, Loaded, NotLoaded } from 'hew/utils/loadable';
import { groupBy, keyBy, mapValues } from 'lodash';
import { useObservable } from 'micro-observables';
import React, { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react';
import { v4 as uuidv4 } from 'uuid';
Expand Down Expand Up @@ -178,6 +179,7 @@ const FlatRuns: React.FC<Props> = ({ projectId, workspaceId, searchId }) => {
const [runs, setRuns] = useState<Loadable<FlatRun>[]>(INITIAL_LOADING_RUNS);

const [sorts, setSorts] = useState<Sort[]>([EMPTY_SORT]);

const sortString = useMemo(() => makeSortString(sorts.filter(validSort.is)), [sorts]);
const loadableFormset = useObservable(formStore.formset);
const rootFilterChildren: Array<FormGroup | FormField> = Loadable.match(loadableFormset, {
Expand Down Expand Up @@ -237,6 +239,24 @@ const FlatRuns: React.FC<Props> = ({ projectId, workspaceId, searchId }) => {
}
}, [projectId]);

// expand sorts to include types from metadata columns so the column ids match
const expandedSorts = useMemo(() => {
return projectColumns.match({
_: () => sorts,
Loaded(pc) {
const groupedColumns = groupBy(pc, (c) => c.column);
const columnToExpandedColumns = mapValues(groupedColumns, (cols) =>
cols.map((c) => formatColumnKey(c)),
);
return sorts
.filter(validSort.is)
.flatMap((sort) =>
columnToExpandedColumns[sort.column].map((ec) => ({ ...sort, column: ec })),
);
},
});
}, [sorts, projectColumns]);

const arrayTypeColumns = useMemo(() => {
const arrayTypeColumns = projectColumns
.getOrElse([])
Expand Down Expand Up @@ -292,13 +312,11 @@ const FlatRuns: React.FC<Props> = ({ projectId, workspaceId, searchId }) => {
return [...STATIC_COLUMNS, ...settings.columns.slice(0, settings.pinnedColumnsCount)];
}, [settings.columns, settings.pinnedColumnsCount]);

const projectColumnsMap = useMemo(() => {
return projectColumns.map((columns) => keyBy(columns, formatColumnKey));
}, [projectColumns]);

const columns: ColumnDef<FlatRun>[] = useMemo(() => {
const projectColumnsMap: Loadable<Record<string, ProjectColumn>> = Loadable.map(
projectColumns,
(columns) => {
return columns.reduce((acc, col) => ({ ...acc, [formatColumnKey(col)]: col }), {});
},
);
const columnDefs = getColumnDefs({
appTheme,
columnWidths: settings.columnWidths,
Expand Down Expand Up @@ -455,7 +473,7 @@ const FlatRuns: React.FC<Props> = ({ projectId, workspaceId, searchId }) => {
appTheme,
columnsIfLoaded,
isDarkMode,
projectColumns,
projectColumnsMap,
projectHeatmap,
dataGridSelection.rows,
settings.columnWidths,
Expand Down Expand Up @@ -956,7 +974,7 @@ const FlatRuns: React.FC<Props> = ({ projectId, workspaceId, searchId }) => {
[
bannedFilterColumns,
bannedSortColumns,
projectColumns,
projectColumnsMap,
settings.pinnedColumnsCount,
settings.heatmapOn,
settings.heatmapSkipped,
Expand Down

0 comments on commit 998dcb9

Please sign in to comment.