From 6e5c1eb069027cbd2f7e4b21f2a8855b0447e420 Mon Sep 17 00:00:00 2001 From: Maurice Yap Date: Mon, 6 Jan 2025 10:46:55 +0000 Subject: [PATCH] Allow configuration of column order (#4118) In the lookout UI, allow the user to change the order of the columns of the jobs table through a drag-and-drop interface. This change replaces the existing column selector with a new column configuration dialog which extends the current functionality of the column selector with the ability to drag and drop columns into the user's desired order. The existing functionality which has been preserved is: - Hiding and showing columns (via a checkbox) - Adding annotation key columns - Removing and editing annotation key columns which have been added - Indicates when columns are for annotation keys The new column configuration dialog has the following additional functionalities: - Hides columns which are configured to be pinned (i.e. the selector column) - Hides columns which are grouped, and explains this to the user - When the input to add an annotation column is not visible on the screen, it displays a chip indicating its presence. Clicking on this chip will scroll to the input - Validates annotation key columns on input, making sure they have no leading or trailing whitespace, and that there is not already a column for the same annotation key This change also makes the buttons in the job action bar more visually consistent - using primary colours for actions and secondary colours for configuration changes. --- internal/lookout/ui/package.json | 3 + .../AddAnnotationColumnInput.tsx | 69 +++ .../EditAnnotationColumnInput.tsx | 69 +++ .../OrderableColumnListItem.tsx | 130 +++++ .../ColumnConfigurationDialog/index.tsx | 247 +++++++++ .../lookoutV2/ColumnSelect.module.css | 31 -- .../src/components/lookoutV2/ColumnSelect.tsx | 255 --------- .../lookoutV2/CustomViewPicker.module.css | 2 +- .../components/lookoutV2/CustomViewPicker.tsx | 8 +- .../lookoutV2/JobsTableActionBar.module.css | 23 +- .../lookoutV2/JobsTableActionBar.tsx | 145 +++--- .../lookoutV2/JobsTableContainer.test.tsx | 29 +- .../lookoutV2/JobsTableContainer.tsx | 68 ++- .../JobsTablePreferencesService.test.ts | 482 +++++++++++++++++- .../lookoutV2/JobsTablePreferencesService.ts | 38 +- .../lookout/ui/src/utils/jobsTableColumns.tsx | 10 +- internal/lookout/ui/yarn.lock | 41 +- 17 files changed, 1216 insertions(+), 434 deletions(-) create mode 100644 internal/lookout/ui/src/components/lookoutV2/ColumnConfigurationDialog/AddAnnotationColumnInput.tsx create mode 100644 internal/lookout/ui/src/components/lookoutV2/ColumnConfigurationDialog/EditAnnotationColumnInput.tsx create mode 100644 internal/lookout/ui/src/components/lookoutV2/ColumnConfigurationDialog/OrderableColumnListItem.tsx create mode 100644 internal/lookout/ui/src/components/lookoutV2/ColumnConfigurationDialog/index.tsx delete mode 100644 internal/lookout/ui/src/components/lookoutV2/ColumnSelect.module.css delete mode 100644 internal/lookout/ui/src/components/lookoutV2/ColumnSelect.tsx diff --git a/internal/lookout/ui/package.json b/internal/lookout/ui/package.json index 1dc8345ff74..a503197ced5 100644 --- a/internal/lookout/ui/package.json +++ b/internal/lookout/ui/package.json @@ -18,6 +18,9 @@ "fmt": "eslint './src/**/*.{js,ts,tsx}' --max-warnings 0 --fix" }, "dependencies": { + "@dnd-kit/core": "^6.3.1", + "@dnd-kit/modifiers": "^9.0.0", + "@dnd-kit/sortable": "^10.0.0", "@emotion/react": "^11.13.5", "@emotion/styled": "^11.13.5", "@fortawesome/fontawesome-common-types": "^6.7.1", diff --git a/internal/lookout/ui/src/components/lookoutV2/ColumnConfigurationDialog/AddAnnotationColumnInput.tsx b/internal/lookout/ui/src/components/lookoutV2/ColumnConfigurationDialog/AddAnnotationColumnInput.tsx new file mode 100644 index 00000000000..a98b1df957e --- /dev/null +++ b/internal/lookout/ui/src/components/lookoutV2/ColumnConfigurationDialog/AddAnnotationColumnInput.tsx @@ -0,0 +1,69 @@ +import { useState } from "react" + +import { AddCircle } from "@mui/icons-material" +import { FormControl, FormHelperText, IconButton, InputAdornment, InputLabel, OutlinedInput } from "@mui/material" + +const INPUT_LABEL_TEXT = "Annotation key" +const INPUT_ID = "annotation-key" + +export interface AddAnnotationColumnInputProps { + onCreate: (annotationKey: string) => void + existingAnnotationColumnKeysSet: Set +} + +export const AddAnnotationColumnInput = ({ + onCreate, + existingAnnotationColumnKeysSet, +}: AddAnnotationColumnInputProps) => { + const [value, setValue] = useState("") + + const isValueFilled = value.length !== 0 + + const valueHasNoLeadingTrailingWhitespace = value.trim() === value + const valueIsNew = !existingAnnotationColumnKeysSet.has(value) + const isValueValid = [valueHasNoLeadingTrailingWhitespace, valueIsNew].every(Boolean) + + const handleCreate = () => { + onCreate(value) + setValue("") + } + + return ( + + {INPUT_LABEL_TEXT} + { + setValue(target.value) + }} + onKeyDown={(event) => { + if (event.key === "Enter" && isValueFilled && isValueValid) { + handleCreate() + event.currentTarget.blur() + event.preventDefault() + } + }} + endAdornment={ + isValueFilled && isValueValid ? ( + + + + + + ) : undefined + } + /> + {!valueHasNoLeadingTrailingWhitespace && ( + The annotation key must not have leading or trailing whitespace. + )} + {!valueIsNew && A column for this annotation key already exists.} + + ) +} diff --git a/internal/lookout/ui/src/components/lookoutV2/ColumnConfigurationDialog/EditAnnotationColumnInput.tsx b/internal/lookout/ui/src/components/lookoutV2/ColumnConfigurationDialog/EditAnnotationColumnInput.tsx new file mode 100644 index 00000000000..d8edf03e2a2 --- /dev/null +++ b/internal/lookout/ui/src/components/lookoutV2/ColumnConfigurationDialog/EditAnnotationColumnInput.tsx @@ -0,0 +1,69 @@ +import { useState } from "react" + +import { Check } from "@mui/icons-material" +import { FormControl, FormHelperText, IconButton, InputAdornment, InputLabel, OutlinedInput } from "@mui/material" + +const INPUT_LABEL_TEXT = "Annotation key" +const INPUT_ID = "annotation-key" + +export interface EditAnnotationColumnInputProps { + onEdit: (annotationKey: string) => void + currentAnnotationKey: string + existingAnnotationColumnKeys: Set +} + +export const EditAnnotationColumnInput = ({ + onEdit, + existingAnnotationColumnKeys, + currentAnnotationKey, +}: EditAnnotationColumnInputProps) => { + const [value, setValue] = useState(currentAnnotationKey) + + const isValueFilled = value.length !== 0 + + const valueHasNoLeadingTrailingWhitespace = value.trim() === value + const valueIsUnique = !existingAnnotationColumnKeys.has(value) || value === currentAnnotationKey + const isValueValid = [valueHasNoLeadingTrailingWhitespace, valueIsUnique].every(Boolean) + + const handleEdit = () => { + onEdit(value) + } + + return ( + + {INPUT_LABEL_TEXT} + { + setValue(target.value) + }} + onKeyDown={(event) => { + event.stopPropagation() + if (event.key === "Enter" && isValueFilled && isValueValid) { + handleEdit() + } + }} + endAdornment={ + isValueFilled && isValueValid ? ( + + + + + + ) : undefined + } + /> + {!valueHasNoLeadingTrailingWhitespace && ( + The annotation key must not have leading or trailing whitespace. + )} + {!valueIsUnique && A column for this annotation key already exists.} + + ) +} diff --git a/internal/lookout/ui/src/components/lookoutV2/ColumnConfigurationDialog/OrderableColumnListItem.tsx b/internal/lookout/ui/src/components/lookoutV2/ColumnConfigurationDialog/OrderableColumnListItem.tsx new file mode 100644 index 00000000000..00bd3520392 --- /dev/null +++ b/internal/lookout/ui/src/components/lookoutV2/ColumnConfigurationDialog/OrderableColumnListItem.tsx @@ -0,0 +1,130 @@ +import { useState } from "react" + +import { useSortable } from "@dnd-kit/sortable" +import { CSS } from "@dnd-kit/utilities" +import { Cancel, Delete, DragHandle, Edit } from "@mui/icons-material" +import { + Checkbox, + IconButton, + ListItem, + ListItemButton, + ListItemIcon, + ListItemText, + Stack, + styled, +} from "@mui/material" + +import { EditAnnotationColumnInput } from "./EditAnnotationColumnInput" +import { SPACING } from "../../../styling/spacing" +import { + AnnotationColumnId, + fromAnnotationColId, + getColumnMetadata, + JobTableColumn, + toColId, +} from "../../../utils/jobsTableColumns" + +const GrabListItemIcon = styled(ListItemIcon)({ + cursor: "grab", + touchAction: "none", +}) + +const EditAnnotationColumnInputContainer = styled(Stack)({ + width: "100%", +}) + +export interface OrderableColumnListItemProps { + column: JobTableColumn + isVisible: boolean + onToggleVisibility: () => void + removeAnnotationColumn: () => void + editAnnotationColumn: (annotationKey: string) => void + existingAnnotationColumnKeysSet: Set +} + +export const OrderableColumnListItem = ({ + column, + isVisible, + onToggleVisibility, + removeAnnotationColumn, + editAnnotationColumn, + existingAnnotationColumnKeysSet, +}: OrderableColumnListItemProps) => { + const colId = toColId(column.id) + const colMetadata = getColumnMetadata(column) + const colIsAnnotation = colMetadata.annotation ?? false + const { attributes, listeners, setNodeRef, transform, transition } = useSortable({ + id: colId, + }) + + const [isEditing, setIsEditing] = useState(false) + + return ( + + { + setIsEditing(true) + }} + > + + + + + + + ) : undefined + } + ref={setNodeRef} + {...attributes} + {...listeners} + > + + + + {colIsAnnotation && isEditing ? ( + + { + editAnnotationColumn(annotationKey) + setIsEditing(false) + }} + currentAnnotationKey={fromAnnotationColId(colId as AnnotationColumnId)} + existingAnnotationColumnKeys={existingAnnotationColumnKeysSet} + /> +
+ setIsEditing(false)} title="Cancel changes"> + + +
+ + ) : ( + + + + + + + )} + + ) +} diff --git a/internal/lookout/ui/src/components/lookoutV2/ColumnConfigurationDialog/index.tsx b/internal/lookout/ui/src/components/lookoutV2/ColumnConfigurationDialog/index.tsx new file mode 100644 index 00000000000..cfd26aa003c --- /dev/null +++ b/internal/lookout/ui/src/components/lookoutV2/ColumnConfigurationDialog/index.tsx @@ -0,0 +1,247 @@ +import { PointerEvent, useCallback, useMemo, useRef, useState } from "react" + +import { DndContext, DragEndEvent, KeyboardSensor, PointerSensor, useSensor, useSensors } from "@dnd-kit/core" +import { restrictToWindowEdges, restrictToVerticalAxis } from "@dnd-kit/modifiers" +import { arrayMove, SortableContext } from "@dnd-kit/sortable" +import { ArrowDownward, Close } from "@mui/icons-material" +import { + Alert, + Chip, + Dialog, + DialogContent, + DialogContentText, + DialogTitle, + IconButton, + List, + Stack, + styled, + Typography, +} from "@mui/material" + +import { AddAnnotationColumnInput } from "./AddAnnotationColumnInput" +import { OrderableColumnListItem } from "./OrderableColumnListItem" +import { SPACING } from "../../../styling/spacing" +import { + AnnotationColumnId, + ColumnId, + fromAnnotationColId, + getColumnMetadata, + JobTableColumn, + PINNED_COLUMNS, + toColId, +} from "../../../utils/jobsTableColumns" + +const ScrollToAddAnnotationColumnChip = styled(Chip)({ + zIndex: 100, + + position: "absolute", + left: "50%", + bottom: 10, + transform: "translate(-50%, 0)", +}) + +const StyledDialogTitle = styled(DialogTitle)({ + paddingRight: 10, +}) + +const CloseIconButton = styled(IconButton)(({ theme }) => ({ + position: "absolute", + top: 8, + right: 8, + color: theme.palette.text.secondary, +})) + +class ColumnListPointerSensor extends PointerSensor { + static activators = [ + { + eventName: "onPointerDown", + handler: ({ nativeEvent: event }: PointerEvent) => { + // Block DnD event propagation if element has "data-no-dnd" attribute + let cur = event.target as HTMLElement + while (cur) { + if (cur.dataset && cur.dataset.noDnd) { + return false + } + cur = cur.parentElement as HTMLElement + } + return true + }, + }, + ] as (typeof PointerSensor)["activators"] +} + +export interface ColumnConfigurationDialogProps { + open: boolean + onClose: () => void + allColumns: JobTableColumn[] + groupedColumnIds: ColumnId[] + visibleColumnIds: ColumnId[] + columnOrderIds: ColumnId[] + setColumnOrder: (columnOrder: ColumnId[]) => void + toggleColumnVisibility: (columnId: ColumnId) => void + onAddAnnotationColumn: (annotationKey: string) => void + onRemoveAnnotationColumn: (colId: ColumnId) => void + onEditAnnotationColumn: (colId: ColumnId, annotationKey: string) => void +} + +export const ColumnConfigurationDialog = ({ + open, + onClose, + allColumns, + groupedColumnIds, + visibleColumnIds, + columnOrderIds, + setColumnOrder, + toggleColumnVisibility, + onAddAnnotationColumn, + onRemoveAnnotationColumn, + onEditAnnotationColumn, +}: ColumnConfigurationDialogProps) => { + const allColumnsById = useMemo( + () => + allColumns.reduce( + (acc, column) => { + acc[toColId(column.id)] = column + return acc + }, + {} as Record, + ), + [allColumns], + ) + + const orderedColumns = useMemo(() => { + const groupedColumnsSet = new Set(groupedColumnIds) + return columnOrderIds + .filter((id) => !groupedColumnsSet.has(id) && !PINNED_COLUMNS.includes(toColId(id)) && id in allColumnsById) + .map((id) => allColumnsById[id]) + }, [columnOrderIds, groupedColumnIds, allColumnsById]) + + const visibleColumnsSet = useMemo(() => new Set(visibleColumnIds), [visibleColumnIds]) + + const handleDragEnd = useCallback( + ({ active, over }: DragEndEvent) => { + if (over && active.id !== over.id) { + const oldIndex = columnOrderIds.indexOf(active.id as ColumnId) + const newIndex = columnOrderIds.indexOf(over.id as ColumnId) + setColumnOrder(arrayMove(columnOrderIds, oldIndex, newIndex)) + } + }, + [columnOrderIds, setColumnOrder], + ) + + const annotationColumnKeysSet = useMemo( + () => + new Set( + allColumns + .filter((column) => getColumnMetadata(column).annotation) + .map(({ id }) => fromAnnotationColId(id as AnnotationColumnId)), + ), + [allColumns], + ) + + const pointerSensor = useSensor(ColumnListPointerSensor) + const keyboardSensor = useSensor(KeyboardSensor) + const sensors = useSensors(pointerSensor, keyboardSensor) + + const [isAddAnnotationColumnContainerVisible, setIsAddAnnotationColumnContainerVisible] = useState(false) + const addAnnotationColumnContainerObserver = useRef() + const scrollIntoViewAddAnnotationColumnContainer = useRef<() => void>() + const addAnnotationColumnContainerRef = useCallback((node: HTMLDivElement) => { + if (addAnnotationColumnContainerObserver.current) { + addAnnotationColumnContainerObserver.current.disconnect() + } + + addAnnotationColumnContainerObserver.current = new IntersectionObserver(([entry]) => { + setIsAddAnnotationColumnContainerVisible(entry.isIntersecting) + }) + + if (node) { + addAnnotationColumnContainerObserver.current.observe(node) + scrollIntoViewAddAnnotationColumnContainer.current = () => { + node.scrollIntoView({ behavior: "smooth" }) + } + } + }, []) + + return ( + + {!isAddAnnotationColumnContainerVisible && ( + } + color="primary" + /> + )} + Column configuration + + + + + + + Select which columns to view, any additional annotation columns, and the order of the columns. + + {groupedColumnIds.length > 0 && ( + <> + + The following columns cannot be hidden or re-ordered because they are currently grouped: + + + {groupedColumnIds + .map((id) => allColumnsById[id]) + .map((column) => { + const { displayName } = getColumnMetadata(column) + return ( + + {displayName} + + ) + })} + + + )} + + Click and drag the columns into your desired order. + +
+ + toColId(id))}> + + {orderedColumns.map((column) => { + const colId = toColId(column.id) + return ( + toggleColumnVisibility(colId)} + removeAnnotationColumn={() => onRemoveAnnotationColumn(colId)} + editAnnotationColumn={(annotationKey) => onEditAnnotationColumn(colId, annotationKey)} + existingAnnotationColumnKeysSet={annotationColumnKeysSet} + /> + ) + })} + + + +
+
+ Add annotation column + + Annotations are metadata (key-value pairs) that you can add to your job. + + +
+ + + + ) +} diff --git a/internal/lookout/ui/src/components/lookoutV2/ColumnSelect.module.css b/internal/lookout/ui/src/components/lookoutV2/ColumnSelect.module.css deleted file mode 100644 index ca1a3fb7c05..00000000000 --- a/internal/lookout/ui/src/components/lookoutV2/ColumnSelect.module.css +++ /dev/null @@ -1,31 +0,0 @@ -.columnMenu { - display: flex; - flex-direction: row; -} - -.columnSelect { - flex: 1 0 auto; - max-width: 500px; - overflow-y: auto; -} - -.annotationSelectContainer { - flex: 0 0 250px; - padding-left: 8px; - padding-right: 8px; - padding-top: 8px; -} - -.addColumnButton { - margin: 8px; -} - -.addAnnotationButtons { - display: flex; - flex-direction: row; - justify-content: space-between; -} - -.addAnnotationAction { - margin: 8px; -} diff --git a/internal/lookout/ui/src/components/lookoutV2/ColumnSelect.tsx b/internal/lookout/ui/src/components/lookoutV2/ColumnSelect.tsx deleted file mode 100644 index d63001b1316..00000000000 --- a/internal/lookout/ui/src/components/lookoutV2/ColumnSelect.tsx +++ /dev/null @@ -1,255 +0,0 @@ -import { useRef, useState } from "react" - -import { ArrowDropDown, ArrowDropUp, Check, Delete, Edit } from "@mui/icons-material" -import { - Button, - Checkbox, - FormControl, - IconButton, - InputAdornment, - InputLabel, - ListItemText, - MenuItem, - OutlinedInput, - Popover, - TextField, - Typography, -} from "@mui/material" - -import styles from "./ColumnSelect.module.css" -import { ColumnId, getColumnMetadata, JobTableColumn, StandardColumnId, toColId } from "../../utils/jobsTableColumns" - -type ColumnSelectProps = { - selectableColumns: JobTableColumn[] - groupedColumns: ColumnId[] - visibleColumns: ColumnId[] - onAddAnnotation: (annotationKey: string) => void - onToggleColumn: (columnId: ColumnId) => void - onRemoveAnnotation: (columnId: ColumnId) => void - onEditAnnotation: (columnId: ColumnId, newDisplayName: string) => void -} - -export default function ColumnSelect({ - selectableColumns, - groupedColumns, - visibleColumns, - onAddAnnotation, - onToggleColumn, - onRemoveAnnotation, - onEditAnnotation, -}: ColumnSelectProps) { - const anchorEl = useRef(null) - const [isOpen, setIsOpen] = useState(false) - - const [creatingAnnotation, setCreatingAnnotation] = useState(false) - const [newAnnotationKey, setNewAnnotationKey] = useState("") - - const [currentlyEditing, setCurrentlyEditing] = useState(new Map()) - - function clearAddAnnotation() { - setCreatingAnnotation(false) - setNewAnnotationKey("") - } - - function saveNewAnnotation() { - onAddAnnotation(newAnnotationKey.trim()) - clearAddAnnotation() - } - - function edit(key: string, name: string) { - const newCurrentlyEditing = new Map(currentlyEditing) - newCurrentlyEditing.set(key, name) - setCurrentlyEditing(newCurrentlyEditing) - } - - function stopEditing(key: string) { - if (currentlyEditing.has(key)) { - const newCurrentlyEditing = new Map(currentlyEditing) - newCurrentlyEditing.delete(key) - setCurrentlyEditing(newCurrentlyEditing) - } - } - - return ( - <> - - - Columns - - setIsOpen(true)} - type={"button"} - label={"Columns"} - value={`${ - selectableColumns.filter((col) => (visibleColumns as string[]).includes(col.id ?? "")).length - } columns selected`} - endAdornment={{isOpen ? : }} - style={{ paddingRight: 5 }} - /> - setIsOpen(false)} - anchorEl={anchorEl.current} - anchorOrigin={{ - vertical: "bottom", - horizontal: "center", - }} - transformOrigin={{ - vertical: "top", - horizontal: "center", - }} - > -
-
- {selectableColumns.map((column) => { - const colId = toColId(column.id) - const colIsGrouped = groupedColumns.includes(colId) - const colIsVisible = visibleColumns.includes(colId) - const colMetadata = getColumnMetadata(column) - const colIsAnnotation = colMetadata.annotation ?? false - return ( - { - if (!colIsAnnotation) { - onToggleColumn(colId) - } - }} - key={colId} - value={colMetadata.displayName} - disabled={colIsGrouped || colId === StandardColumnId.Count} - > - onToggleColumn(colId)} /> - {colIsAnnotation ? ( - <> - {currentlyEditing.has(colId) ? ( - <> - edit(colId, e.target.value)} - style={{ - maxWidth: 350, - }} - fullWidth={true} - /> - { - if (currentlyEditing.has(colId)) { - onEditAnnotation(colId, currentlyEditing.get(colId) ?? "") - } - stopEditing(colId) - }} - > - - - - ) : ( - <> - - edit(colId, colMetadata.displayName)}> - - - - )} - { - stopEditing(colId) - onRemoveAnnotation(colId) - }} - > - - - - ) : ( - - )} - - ) - })} -
-
- - Click here to add an annotation column. - - - Annotations are metadata (key-value pairs) that you can add to your job. - -
- {creatingAnnotation ? ( - <> - { - setNewAnnotationKey(e.target.value) - }} - onKeyUp={(e) => { - if (e.key === "Enter") { - saveNewAnnotation() - } - }} - /> -
-
- -
-
- -
-
- - ) : ( - - )} -
-
-
- - - - ) -} diff --git a/internal/lookout/ui/src/components/lookoutV2/CustomViewPicker.module.css b/internal/lookout/ui/src/components/lookoutV2/CustomViewPicker.module.css index 96c2d026e9e..c5d3eb77661 100644 --- a/internal/lookout/ui/src/components/lookoutV2/CustomViewPicker.module.css +++ b/internal/lookout/ui/src/components/lookoutV2/CustomViewPicker.module.css @@ -1,4 +1,4 @@ -.settingsButton { +.customizeButton { min-width: 50px; display: flex; justify-content: center; diff --git a/internal/lookout/ui/src/components/lookoutV2/CustomViewPicker.tsx b/internal/lookout/ui/src/components/lookoutV2/CustomViewPicker.tsx index 93bee0ac871..f8608212581 100644 --- a/internal/lookout/ui/src/components/lookoutV2/CustomViewPicker.tsx +++ b/internal/lookout/ui/src/components/lookoutV2/CustomViewPicker.tsx @@ -1,6 +1,6 @@ import { useRef, useState } from "react" -import { Delete, Settings } from "@mui/icons-material" +import { DashboardCustomize, Delete } from "@mui/icons-material" import { Button, Popover, @@ -48,9 +48,9 @@ export const CustomViewPicker = ({ return ( <> -
- setIsOpen(!isOpen)}> - +
+ setIsOpen(!isOpen)}> +
void selectedItemFilters: JobFilter[][] + filtersActive: boolean customViews: string[] activeJobSets: boolean onActiveJobSetsChanged: (newVal: boolean) => void @@ -48,7 +51,10 @@ export const JobsTableActionBar = memo( allColumns, groupedColumns, visibleColumns, + columnOrder, + setColumnOrder, selectedItemFilters, + filtersActive, customViews, activeJobSets, onActiveJobSetsChanged, @@ -68,18 +74,38 @@ export const JobsTableActionBar = memo( onDeleteCustomView, onLoadCustomView, }: JobsTableActionBarProps) => { + const [columnConfigurationDialogOpen, setColumnConfigurationDialogOpen] = useState(false) const [cancelDialogOpen, setCancelDialogOpen] = useState(false) const [reprioritiseDialogOpen, setReprioritiseDialogOpen] = useState(false) - const openSnackbar = useCustomSnackbar() - const selectableColumns = useMemo(() => allColumns.filter((col) => col.enableHiding !== false), [allColumns]) + const numberSelectedColumns = useMemo(() => { + const visibleColumnsSet = new Set(visibleColumns) + return allColumns.filter((col) => { + const colId = toColId(col.id) + return !PINNED_COLUMNS.includes(colId) && visibleColumnsSet.has(colId) + }).length + }, [allColumns, visibleColumns]) const numSelectedItems = selectedItemFilters.length + const columnConfigurationDialogOpenOnClose = useCallback(() => setColumnConfigurationDialogOpen(false), []) const cancelDialogOnClose = useCallback(() => setCancelDialogOpen(false), []) const reprioritiseDialogOnClose = useCallback(() => setReprioritiseDialogOpen(false), []) return (
+ {cancelDialogOpen && ( )} - +
+ +
- - - - { - try { - onAddAnnotationColumn(annotationKey) - } catch (e) { - const err = e as Error - console.error(err.message) - openSnackbar(`Failed to create annotation column: ${err.message}`, "error") - } - }} - onRemoveAnnotation={(columnId) => { - try { - onRemoveAnnotationColumn(columnId) - } catch (e) { - const err = e as Error - console.error(err.message) - openSnackbar(`Failed to remove annotation column: ${err.message}`, "error") - } - }} - onEditAnnotation={(columnId, annotationKey) => { - try { - onEditAnnotationColumn(columnId, annotationKey) - } catch (e) { - const err = e as Error - console.error(err.message) - openSnackbar(`Failed to edit annotation column: ${err.message}`, "error") - } - }} - onToggleColumn={toggleColumnVisibility} - /> +
+ +
+
+ +
+
+ +
+
+ +
- - +
+ +
+
+ +
) diff --git a/internal/lookout/ui/src/containers/lookoutV2/JobsTableContainer.test.tsx b/internal/lookout/ui/src/containers/lookoutV2/JobsTableContainer.test.tsx index 019f61ac397..056d9e0145e 100644 --- a/internal/lookout/ui/src/containers/lookoutV2/JobsTableContainer.test.tsx +++ b/internal/lookout/ui/src/containers/lookoutV2/JobsTableContainer.test.tsx @@ -23,6 +23,12 @@ import FakeGetJobsService from "../../services/lookoutV2/mocks/FakeGetJobsServic import { FakeGetRunInfoService } from "../../services/lookoutV2/mocks/FakeGetRunInfoService" import FakeGroupJobsService from "../../services/lookoutV2/mocks/FakeGroupJobsService" +const intersectionObserverMock = () => ({ + observe: () => null, + disconnect: () => null, +}) +window.IntersectionObserver = vi.fn().mockImplementation(intersectionObserverMock) + vi.setConfig({ // This is quite a heavy component, and tests can timeout on a slower machine testTimeout: 30_000, @@ -177,15 +183,14 @@ describe("JobsTableContainer", () => { ["Job Set", "jobSet"], ["Queue", "queue"], ["State", "state"], - ])("should allow grouping by %s", async (displayString, groupKey) => { + ] as [string, keyof Job][])("should allow grouping by %s", async (displayString, groupKey) => { const jobs = [ ...makeTestJobs(5, "queue-1", "job-set-1", JobState.Queued), ...makeTestJobs(10, "queue-2", "job-set-2", JobState.Pending), ...makeTestJobs(15, "queue-3", "job-set-3", JobState.Running), ] - const jobObjKey = groupKey as keyof Job - const numUniqueForJobKey = new Set(jobs.map((j) => j[jobObjKey])).size + const numUniqueForJobKey = new Set(jobs.map((j) => j[groupKey])).size renderComponent(jobs) await waitForFinishedLoading() @@ -197,10 +202,10 @@ describe("JobsTableContainer", () => { // Expand a row const job = jobs[0] - await expandRow(job[jobObjKey]!.toString()) + await expandRow(job[groupKey]!.toString()) // Check the right number of rows is being shown - const numShownJobs = jobs.filter((j) => j[jobObjKey] === job[jobObjKey]).length + const numShownJobs = jobs.filter((j) => j[groupKey] === job[groupKey]).length await assertNumDataRowsShown(numUniqueForJobKey + numShownJobs) }) @@ -682,21 +687,17 @@ describe("JobsTableContainer", () => { } async function addAnnotationColumn(annotationKey: string) { - const editColumnsButton = await screen.findByRole("button", { name: /columns selected/i }) + const editColumnsButton = await screen.findByRole("button", { name: /configure columns/i }) await userEvent.click(editColumnsButton) - const addColumnButton = await screen.findByRole("button", { name: /Add column/i }) - await userEvent.click(addColumnButton) - - const textbox = await screen.findByRole("textbox", { name: /Annotation key/i }) + const textbox = await screen.findByRole("textbox", { name: /annotation key/i }) await userEvent.type(textbox, annotationKey) - const saveButton = await screen.findByRole("button", { name: /Save/i }) + const saveButton = await screen.findByRole("button", { name: /add a column for annotation/i }) await userEvent.click(saveButton) - // Close pop up - await userEvent.click(screen.getByText(/Click here to add an annotation column/i)) - await userEvent.keyboard("{Escape}") + const closeButton = await screen.findByRole("button", { name: /close/i }) + await userEvent.click(closeButton) } async function clickOnJobRow(jobId: string) { diff --git a/internal/lookout/ui/src/containers/lookoutV2/JobsTableContainer.tsx b/internal/lookout/ui/src/containers/lookoutV2/JobsTableContainer.tsx index 2df50fc58fa..569222b1d59 100644 --- a/internal/lookout/ui/src/containers/lookoutV2/JobsTableContainer.tsx +++ b/internal/lookout/ui/src/containers/lookoutV2/JobsTableContainer.tsx @@ -61,6 +61,7 @@ import { INPUT_PROCESSORS, JOB_COLUMNS, JobTableColumn, + PINNED_COLUMNS, StandardColumnId, toAnnotationColId, toColId, @@ -155,6 +156,7 @@ export const JobsTableContainer = ({ ) const columnResizeMode = useMemo(() => "onChange" as ColumnResizeMode, []) const [columnSizing, setColumnSizing] = useState(initialPrefs.columnSizing ?? {}) + const [columnOrder, setColumnOrder] = useState(initialPrefs.columnOrder) // Grouping const [grouping, setGrouping] = useState(initialPrefs.groupedColumns) @@ -245,24 +247,23 @@ export const JobsTableContainer = ({ [sidebarJobId, jobInfoMap], ) - const prefsFromState = (): JobsTablePreferences => { - return { - groupedColumns: grouping, - expandedState: expanded, - pageIndex, - pageSize, - order: lookoutOrder, - columnSizing: columnSizing, - filters: columnFilterState, - columnMatches: columnMatches, - annotationColumnKeys: getAnnotationKeyCols(allColumns), - visibleColumns: columnVisibility, - sidebarJobId: sidebarJobId, - sidebarWidth: sidebarWidth, - activeJobSets: activeJobSets, - autoRefresh: autoRefresh, - } - } + const prefsFromState = (): JobsTablePreferences => ({ + groupedColumns: grouping, + expandedState: expanded, + columnOrder, + pageIndex, + pageSize, + order: lookoutOrder, + columnSizing: columnSizing, + filters: columnFilterState, + columnMatches: columnMatches, + annotationColumnKeys: getAnnotationKeyCols(allColumns), + visibleColumns: columnVisibility, + sidebarJobId: sidebarJobId, + sidebarWidth: sidebarWidth, + activeJobSets: activeJobSets, + autoRefresh: autoRefresh, + }) const setTextFields = (filters: ColumnFiltersState) => { const filterMap = Object.fromEntries(filters.map((f) => [f.id, f])) @@ -300,6 +301,7 @@ export const JobsTableContainer = ({ setColumnMatches(prefs.columnMatches) const cols = JOB_COLUMNS.concat(...prefs.annotationColumnKeys.map(createAnnotationColumn)) setAllColumns(cols) + setColumnOrder(prefs.columnOrder) setColumnVisibility(prefs.visibleColumns) setSidebarJobId(prefs.sidebarJobId) setSidebarWidth(prefs.sidebarWidth ?? 600) @@ -425,10 +427,12 @@ export const JobsTableContainer = ({ } const annotationCol = createAnnotationColumn(annotationKey) + const annotationColId = toColId(annotationCol.id) const newCols = allColumns.concat([annotationCol]) + setColumnOrder((prev) => [...prev, annotationColId]) setAllColumns(newCols) - if (!colIsVisible(toColId(annotationCol.id))) { - onColumnVisibilityChange(toColId(annotationCol.id)) + if (!colIsVisible(annotationColId)) { + onColumnVisibilityChange(annotationColId) } } @@ -437,6 +441,7 @@ export const JobsTableContainer = ({ if (filtered.length === allColumns.length) { throw new Error(`column "${colId}" was not removed`) } + setColumnOrder((prev) => prev.filter((prevId) => prevId !== colId)) setAllColumns(filtered) onFilterChange((columnFilters) => { return columnFilters.filter((columnFilter) => columnFilter.id !== colId) @@ -453,16 +458,25 @@ export const JobsTableContainer = ({ if (index === -1) { throw new Error(`column "${colId}" not found`) } + + const oldColId = toColId(allColumns[index].id) + // Make old column not visible - if (colIsVisible(toColId(allColumns[index].id))) { - onColumnVisibilityChange(toColId(allColumns[index].id)) + if (colIsVisible(oldColId)) { + onColumnVisibilityChange(oldColId) } + allColumns[index] = createAnnotationColumn(annotationKey) + const newColId = toColId(allColumns[index].id) + // Make new column visible setAllColumns([...allColumns]) - if (!colIsVisible(toColId(allColumns[index].id))) { - onColumnVisibilityChange(toColId(allColumns[index].id)) + if (!colIsVisible(toColId(newColId))) { + onColumnVisibilityChange(toColId(newColId)) } + + // Change column ID in ordering + setColumnOrder((prev) => prev.map((prevId) => (prevId === oldColId ? newColId : prevId))) } const onGroupingChange = useCallback( @@ -644,10 +658,11 @@ export const JobsTableContainer = ({ columnFilters: columnFilterState, rowSelection: selectedRows, columnPinning: { - left: [StandardColumnId.SelectorCol], + left: PINNED_COLUMNS, }, columnVisibility, sorting, + columnOrder, columnSizing, }, getCoreRowModel: getCoreRowModel(), @@ -755,7 +770,10 @@ export const JobsTableContainer = ({ isLoading={rowsToFetch.length > 0} allColumns={columnsForSelect} groupedColumns={grouping} + filtersActive={columnFilterState.length > 0} visibleColumns={visibleColumnIds} + columnOrder={columnOrder} + setColumnOrder={setColumnOrder} selectedItemFilters={selectedItemsFilters} customViews={customViews} activeJobSets={activeJobSets} diff --git a/internal/lookout/ui/src/services/lookoutV2/JobsTablePreferencesService.test.ts b/internal/lookout/ui/src/services/lookoutV2/JobsTablePreferencesService.test.ts index 06c85a41933..9d4669c6e98 100644 --- a/internal/lookout/ui/src/services/lookoutV2/JobsTablePreferencesService.test.ts +++ b/internal/lookout/ui/src/services/lookoutV2/JobsTablePreferencesService.test.ts @@ -2,6 +2,7 @@ import { Location, NavigateFunction, Params } from "react-router-dom" import { DEFAULT_PREFERENCES, + ensurePreferencesAreConsistent, JobsTablePreferences, JobsTablePreferencesService, PREFERENCES_KEY, @@ -10,7 +11,7 @@ import { } from "./JobsTablePreferencesService" import { Match } from "../../models/lookoutV2Models" import { Router } from "../../utils" -import { ColumnId, DEFAULT_COLUMN_ORDER, StandardColumnId } from "../../utils/jobsTableColumns" +import { ColumnId, DEFAULT_COLUMN_ORDERING, StandardColumnId } from "../../utils/jobsTableColumns" class FakeRouter implements Router { location: Location @@ -58,8 +59,8 @@ describe("JobsTablePreferencesService", () => { ...DEFAULT_PREFERENCES, // From query string above pageIndex: 3, - groupedColumns: ["state" as ColumnId], - order: { id: "jobId", direction: "ASC" }, + groupedColumns: [StandardColumnId.State], + order: { id: StandardColumnId.JobID, direction: "ASC" }, }) }) }) @@ -93,9 +94,9 @@ describe("JobsTablePreferencesService", () => { describe("Grouped columns", () => { it("round-trips columns", () => { - savePartialPrefs({ groupedColumns: ["queue", "state"] as ColumnId[] }) + savePartialPrefs({ groupedColumns: [StandardColumnId.Queue, StandardColumnId.State] }) expect(router.location.search).toContain("g[0]=queue&g[1]=state") - expect(service.getUserPrefs().groupedColumns).toStrictEqual(["queue", "state"]) + expect(service.getUserPrefs().groupedColumns).toStrictEqual([StandardColumnId.Queue, StandardColumnId.State]) }) it("round-trips empty list", () => { @@ -127,27 +128,31 @@ describe("JobsTablePreferencesService", () => { describe("Column filters", () => { it("round-trips column filters", () => { - savePartialPrefs({ filters: [{ id: "queue", value: "test" }] }) + savePartialPrefs({ filters: [{ id: StandardColumnId.Queue, value: "test" }] }) expect(router.location.search).toContain("f[0][id]=queue&f[0][value]=test&f[0][match]=startsWith") - expect(service.getUserPrefs().filters).toStrictEqual([{ id: "queue", value: "test" }]) + expect(service.getUserPrefs().filters).toStrictEqual([{ id: StandardColumnId.Queue, value: "test" }]) }) it("round-trips state filter", () => { - savePartialPrefs({ filters: [{ id: "state", value: ["QUEUED", "PENDING", "RUNNING"] }] }) + savePartialPrefs({ filters: [{ id: StandardColumnId.State, value: ["QUEUED", "PENDING", "RUNNING"] }] }) expect(router.location.search).toContain( "f[0][id]=state&f[0][value][0]=QUEUED&f[0][value][1]=PENDING&f[0][value][2]=RUNNING&f[0][match]=anyOf", ) - expect(service.getUserPrefs().filters).toStrictEqual([{ id: "state", value: ["QUEUED", "PENDING", "RUNNING"] }]) - expect(service.getUserPrefs().columnMatches["state"] === Match.AnyOf) + expect(service.getUserPrefs().filters).toStrictEqual([ + { id: StandardColumnId.State, value: ["QUEUED", "PENDING", "RUNNING"] }, + ]) + expect(service.getUserPrefs().columnMatches[StandardColumnId.State] === Match.AnyOf) }) it("round-trips special characters", () => { - savePartialPrefs({ filters: [{ id: "queue", value: "test & why / do $ this" }] }) + savePartialPrefs({ filters: [{ id: StandardColumnId.Queue, value: "test & why / do $ this" }] }) expect(router.location.search).toContain( "f[0][id]=queue&f[0][value]=test%20%26%20why%20%2F%20do%20%24%20this&f[0][match]=startsWith", ) - expect(service.getUserPrefs().filters).toStrictEqual([{ id: "queue", value: "test & why / do $ this" }]) - expect(service.getUserPrefs().columnMatches["queue"] === Match.StartsWith) + expect(service.getUserPrefs().filters).toStrictEqual([ + { id: StandardColumnId.Queue, value: "test & why / do $ this" }, + ]) + expect(service.getUserPrefs().columnMatches[StandardColumnId.Queue] === Match.StartsWith) }) it("round-trips empty list", () => { @@ -158,15 +163,15 @@ describe("JobsTablePreferencesService", () => { describe("Sort order", () => { it("round-trips asc sort order", () => { - savePartialPrefs({ order: { id: "queue", direction: "ASC" } }) + savePartialPrefs({ order: { id: StandardColumnId.Queue, direction: "ASC" } }) expect(router.location.search).toContain("sort[id]=queue&sort[desc]=false") - expect(service.getUserPrefs().order).toStrictEqual({ id: "queue", direction: "ASC" }) + expect(service.getUserPrefs().order).toStrictEqual({ id: StandardColumnId.Queue, direction: "ASC" }) }) it("round-trips desc sort order", () => { - savePartialPrefs({ order: { id: "queue", direction: "DESC" } }) + savePartialPrefs({ order: { id: StandardColumnId.Queue, direction: "DESC" } }) expect(router.location.search).toContain("sort[id]=queue&sort[desc]=true") - expect(service.getUserPrefs().order).toStrictEqual({ id: "queue", direction: "DESC" }) + expect(service.getUserPrefs().order).toStrictEqual({ id: StandardColumnId.Queue, direction: "DESC" }) }) }) @@ -273,10 +278,11 @@ describe("JobsTablePreferencesService", () => { const localStorageParams: JobsTablePreferences = { annotationColumnKeys: ["hello"], expandedState: { foo: true }, - filters: [{ id: "jobId", value: "112233" }], + filters: [{ id: StandardColumnId.JobID, value: "112233" }], columnMatches: { jobId: Match.Exact }, - groupedColumns: ["queue" as ColumnId, "jobSet" as ColumnId], - order: { id: "timeInState", direction: "ASC" }, + groupedColumns: [StandardColumnId.Queue, StandardColumnId.JobSet], + columnOrder: ["annotation_hello", StandardColumnId.JobID], + order: { id: StandardColumnId.TimeInState, direction: "ASC" }, pageIndex: 5, pageSize: 20, sidebarJobId: "223344", @@ -291,7 +297,31 @@ describe("JobsTablePreferencesService", () => { expandedState: {}, filters: [], groupedColumns: [], - order: DEFAULT_COLUMN_ORDER, + columnOrder: [ + "annotation_hello", + StandardColumnId.JobID, + StandardColumnId.Queue, + StandardColumnId.Namespace, + StandardColumnId.JobSet, + StandardColumnId.State, + StandardColumnId.Count, + StandardColumnId.Priority, + StandardColumnId.Owner, + StandardColumnId.CPU, + StandardColumnId.Memory, + StandardColumnId.EphemeralStorage, + StandardColumnId.GPU, + StandardColumnId.PriorityClass, + StandardColumnId.LastTransitionTimeUtc, + StandardColumnId.TimeInState, + StandardColumnId.TimeSubmittedUtc, + StandardColumnId.TimeSubmittedAgo, + StandardColumnId.Node, + StandardColumnId.Cluster, + StandardColumnId.ExitCode, + StandardColumnId.RuntimeSeconds, + ], + order: DEFAULT_COLUMN_ORDERING, pageIndex: 0, pageSize: 50, sidebarJobId: "112233", @@ -308,16 +338,17 @@ describe("JobsTablePreferencesService", () => { match: Match.AnyOf, }, ], - sort: { id: "timeInState", desc: "false" }, - g: ["jobSet"], + sort: { id: StandardColumnId.TimeInState, desc: "false" }, + g: [StandardColumnId.JobSet], } const localStorageParams: JobsTablePreferences = { annotationColumnKeys: ["key"], expandedState: { foo: true }, - filters: [{ id: "jobId", value: "112233" }], + filters: [{ id: StandardColumnId.JobID, value: "112233" }], columnMatches: { jobId: Match.Exact }, - groupedColumns: ["queue" as ColumnId, "jobSet" as ColumnId], - order: { id: "timeInState", direction: "ASC" }, + groupedColumns: [StandardColumnId.Queue, StandardColumnId.JobSet], + columnOrder: [StandardColumnId.JobID, "annotation_key"], + order: { id: StandardColumnId.TimeInState, direction: "ASC" }, pageIndex: 5, pageSize: 20, sidebarJobId: "223344", @@ -337,8 +368,32 @@ describe("JobsTablePreferencesService", () => { }, ], columnMatches: { [StandardColumnId.State]: Match.AnyOf }, - groupedColumns: ["jobSet"], - order: { id: "timeInState", direction: "ASC" }, + groupedColumns: [StandardColumnId.JobSet], + columnOrder: [ + StandardColumnId.JobID, + "annotation_key", + StandardColumnId.Queue, + StandardColumnId.Namespace, + StandardColumnId.JobSet, + StandardColumnId.State, + StandardColumnId.Count, + StandardColumnId.Priority, + StandardColumnId.Owner, + StandardColumnId.CPU, + StandardColumnId.Memory, + StandardColumnId.EphemeralStorage, + StandardColumnId.GPU, + StandardColumnId.PriorityClass, + StandardColumnId.LastTransitionTimeUtc, + StandardColumnId.TimeInState, + StandardColumnId.TimeSubmittedUtc, + StandardColumnId.TimeSubmittedAgo, + StandardColumnId.Node, + StandardColumnId.Cluster, + StandardColumnId.ExitCode, + StandardColumnId.RuntimeSeconds, + ], + order: { id: StandardColumnId.TimeInState, direction: "ASC" }, pageIndex: 0, pageSize: 50, sidebarJobId: undefined, @@ -354,3 +409,374 @@ describe("JobsTablePreferencesService", () => { }) } }) + +describe("ensurePreferencesAreConsistent", () => { + it("does not change valid preferences", () => { + const validPreferences: JobsTablePreferences = { + annotationColumnKeys: ["preferenc.es/foo-alpha", "preferenc.es/foo-bravo", "preferenc.es/foo-charlie"], + expandedState: {}, + filters: [{ id: StandardColumnId.JobID, value: "112233" }], + columnMatches: { jobId: Match.Exact }, + groupedColumns: [StandardColumnId.Queue, StandardColumnId.JobSet], + columnOrder: [ + StandardColumnId.Owner, + "annotation_preferenc.es/foo-bravo", + StandardColumnId.JobID, + StandardColumnId.State, + "annotation_preferenc.es/foo-charlie", + StandardColumnId.RuntimeSeconds, + StandardColumnId.Queue, + StandardColumnId.Namespace, + StandardColumnId.JobSet, + StandardColumnId.Priority, + StandardColumnId.Count, + StandardColumnId.Memory, + StandardColumnId.EphemeralStorage, + StandardColumnId.PriorityClass, + StandardColumnId.TimeSubmittedUtc, + StandardColumnId.CPU, + StandardColumnId.ExitCode, + StandardColumnId.LastTransitionTimeUtc, + StandardColumnId.TimeInState, + StandardColumnId.GPU, + StandardColumnId.TimeSubmittedAgo, + StandardColumnId.Cluster, + "annotation_preferenc.es/foo-alpha", + StandardColumnId.Node, + ], + order: { id: StandardColumnId.TimeInState, direction: "ASC" }, + pageIndex: 5, + pageSize: 20, + sidebarJobId: "223344", + visibleColumns: { + "annotation_preferenc.es/foo-charlie": true, + "annotation_preferenc.es/foo-bravo": true, + queue: true, + jobId: true, + jobSet: true, + timeInState: true, + }, + } + + ensurePreferencesAreConsistent(validPreferences) + + const expected: JobsTablePreferences = { + annotationColumnKeys: ["preferenc.es/foo-alpha", "preferenc.es/foo-bravo", "preferenc.es/foo-charlie"], + expandedState: {}, + filters: [{ id: StandardColumnId.JobID, value: "112233" }], + columnMatches: { jobId: Match.Exact }, + groupedColumns: [StandardColumnId.Queue, StandardColumnId.JobSet], + columnOrder: [ + StandardColumnId.Owner, + "annotation_preferenc.es/foo-bravo", + StandardColumnId.JobID, + StandardColumnId.State, + "annotation_preferenc.es/foo-charlie", + StandardColumnId.RuntimeSeconds, + StandardColumnId.Queue, + StandardColumnId.Namespace, + StandardColumnId.JobSet, + StandardColumnId.Priority, + StandardColumnId.Count, + StandardColumnId.Memory, + StandardColumnId.EphemeralStorage, + StandardColumnId.PriorityClass, + StandardColumnId.TimeSubmittedUtc, + StandardColumnId.CPU, + StandardColumnId.ExitCode, + StandardColumnId.LastTransitionTimeUtc, + StandardColumnId.TimeInState, + StandardColumnId.GPU, + StandardColumnId.TimeSubmittedAgo, + StandardColumnId.Cluster, + "annotation_preferenc.es/foo-alpha", + StandardColumnId.Node, + ], + order: { id: StandardColumnId.TimeInState, direction: "ASC" }, + pageIndex: 5, + pageSize: 20, + sidebarJobId: "223344", + visibleColumns: { + "annotation_preferenc.es/foo-charlie": true, + "annotation_preferenc.es/foo-bravo": true, + queue: true, + jobId: true, + jobSet: true, + timeInState: true, + }, + } + + expect(validPreferences).toEqual(expected) + }) + + it("adds annotation key columns for annotations referenced in filters", () => { + const validPreferences: JobsTablePreferences = { + annotationColumnKeys: ["preferenc.es/foo-alpha"], + expandedState: {}, + filters: [{ id: "annotation_preferenc.es/foo-delta", value: "ddd" }], + columnMatches: { "annotation_preferenc.es/foo-delta": Match.StartsWith }, + groupedColumns: [], + columnOrder: [ + StandardColumnId.JobID, + StandardColumnId.Queue, + StandardColumnId.Namespace, + StandardColumnId.JobSet, + StandardColumnId.State, + StandardColumnId.Count, + StandardColumnId.Priority, + StandardColumnId.Owner, + StandardColumnId.CPU, + StandardColumnId.Memory, + StandardColumnId.EphemeralStorage, + StandardColumnId.GPU, + StandardColumnId.PriorityClass, + StandardColumnId.LastTransitionTimeUtc, + StandardColumnId.TimeInState, + StandardColumnId.TimeSubmittedUtc, + StandardColumnId.TimeSubmittedAgo, + StandardColumnId.Node, + StandardColumnId.Cluster, + StandardColumnId.ExitCode, + StandardColumnId.RuntimeSeconds, + "annotation_preferenc.es/foo-alpha", + ], + order: { id: StandardColumnId.TimeInState, direction: "ASC" }, + pageIndex: 5, + pageSize: 20, + sidebarJobId: "223344", + visibleColumns: { + "annotation_preferenc.es/foo-alpha": true, + queue: true, + jobId: true, + jobSet: true, + timeInState: true, + }, + } + + ensurePreferencesAreConsistent(validPreferences) + + const expected: JobsTablePreferences = { + annotationColumnKeys: [ + "preferenc.es/foo-alpha", + "preferenc.es/foo-delta", // added + ], + expandedState: {}, + filters: [{ id: "annotation_preferenc.es/foo-delta", value: "ddd" }], + columnMatches: { "annotation_preferenc.es/foo-delta": Match.StartsWith }, + groupedColumns: [], + columnOrder: [ + StandardColumnId.JobID, + StandardColumnId.Queue, + StandardColumnId.Namespace, + StandardColumnId.JobSet, + StandardColumnId.State, + StandardColumnId.Count, + StandardColumnId.Priority, + StandardColumnId.Owner, + StandardColumnId.CPU, + StandardColumnId.Memory, + StandardColumnId.EphemeralStorage, + StandardColumnId.GPU, + StandardColumnId.PriorityClass, + StandardColumnId.LastTransitionTimeUtc, + StandardColumnId.TimeInState, + StandardColumnId.TimeSubmittedUtc, + StandardColumnId.TimeSubmittedAgo, + StandardColumnId.Node, + StandardColumnId.Cluster, + StandardColumnId.ExitCode, + StandardColumnId.RuntimeSeconds, + "annotation_preferenc.es/foo-alpha", + "annotation_preferenc.es/foo-delta", // added + ], + order: { id: StandardColumnId.TimeInState, direction: "ASC" }, + pageIndex: 5, + pageSize: 20, + sidebarJobId: "223344", + visibleColumns: { + "annotation_preferenc.es/foo-alpha": true, + "annotation_preferenc.es/foo-delta": true, // added + queue: true, + jobId: true, + jobSet: true, + timeInState: true, + }, + } + + expect(validPreferences).toEqual(expected) + }) + + it("makes grouped, ordered and filtered columns are visible", () => { + const validPreferences: JobsTablePreferences = { + annotationColumnKeys: ["preferenc.es/foo-alpha"], + expandedState: {}, + filters: [ + { id: "annotation_preferenc.es/foo-alpha", value: "aaa" }, + { + id: StandardColumnId.State, + value: ["QUEUED", "PENDING", "RUNNING"], + }, + ], + columnMatches: { "annotation_preferenc.es/foo-alpha": Match.StartsWith, [StandardColumnId.State]: Match.AnyOf }, + groupedColumns: [StandardColumnId.Queue, "annotation_preferenc.es/foo-alpha", StandardColumnId.JobSet], + columnOrder: [ + StandardColumnId.JobID, + "annotation_preferenc.es/foo-alpha", + StandardColumnId.Queue, + StandardColumnId.Namespace, + StandardColumnId.JobSet, + StandardColumnId.State, + StandardColumnId.Count, + StandardColumnId.Priority, + StandardColumnId.Owner, + StandardColumnId.CPU, + StandardColumnId.Memory, + StandardColumnId.EphemeralStorage, + StandardColumnId.GPU, + StandardColumnId.PriorityClass, + StandardColumnId.LastTransitionTimeUtc, + StandardColumnId.TimeInState, + StandardColumnId.TimeSubmittedUtc, + StandardColumnId.TimeSubmittedAgo, + StandardColumnId.Node, + StandardColumnId.Cluster, + StandardColumnId.ExitCode, + StandardColumnId.RuntimeSeconds, + ], + order: { id: StandardColumnId.TimeInState, direction: "ASC" }, + pageIndex: 5, + pageSize: 20, + sidebarJobId: "223344", + visibleColumns: {}, + } + + ensurePreferencesAreConsistent(validPreferences) + + const expected: JobsTablePreferences = { + annotationColumnKeys: ["preferenc.es/foo-alpha"], + expandedState: {}, + filters: [ + { id: "annotation_preferenc.es/foo-alpha", value: "aaa" }, + { + id: StandardColumnId.State, + value: ["QUEUED", "PENDING", "RUNNING"], + }, + ], + columnMatches: { "annotation_preferenc.es/foo-alpha": Match.StartsWith, [StandardColumnId.State]: Match.AnyOf }, + groupedColumns: [StandardColumnId.Queue, "annotation_preferenc.es/foo-alpha", StandardColumnId.JobSet], + columnOrder: [ + StandardColumnId.JobID, + "annotation_preferenc.es/foo-alpha", + StandardColumnId.Queue, + StandardColumnId.Namespace, + StandardColumnId.JobSet, + StandardColumnId.State, + StandardColumnId.Count, + StandardColumnId.Priority, + StandardColumnId.Owner, + StandardColumnId.CPU, + StandardColumnId.Memory, + StandardColumnId.EphemeralStorage, + StandardColumnId.GPU, + StandardColumnId.PriorityClass, + StandardColumnId.LastTransitionTimeUtc, + StandardColumnId.TimeInState, + StandardColumnId.TimeSubmittedUtc, + StandardColumnId.TimeSubmittedAgo, + StandardColumnId.Node, + StandardColumnId.Cluster, + StandardColumnId.ExitCode, + StandardColumnId.RuntimeSeconds, + ], + order: { id: StandardColumnId.TimeInState, direction: "ASC" }, + pageIndex: 5, + pageSize: 20, + sidebarJobId: "223344", + // All added + visibleColumns: { + "annotation_preferenc.es/foo-alpha": true, + jobSet: true, + queue: true, + state: true, + timeInState: true, + }, + } + + expect(validPreferences).toEqual(expected) + }) + + it("adds to the column order includes all unpinned standard columns and annotations and removes any columns which are neither", () => { + const validPreferences: JobsTablePreferences = { + annotationColumnKeys: ["preferenc.es/foo-alpha", "preferenc.es/foo-bravo", "preferenc.es/foo-charlie"], + expandedState: {}, + filters: [], + columnMatches: {}, + groupedColumns: [], + columnOrder: [ + StandardColumnId.TimeInState, + "annotation_rubbish-annotation", + "annotation_preferenc.es/foo-bravo", + "rubbish" as ColumnId, + StandardColumnId.JobID, + ], + order: { id: StandardColumnId.TimeInState, direction: "ASC" }, + pageIndex: 5, + pageSize: 20, + sidebarJobId: "223344", + visibleColumns: { + [StandardColumnId.JobID]: true, + [StandardColumnId.TimeInState]: true, + 'annotation_"preferenc.es/foo-bravo': true, + }, + } + + ensurePreferencesAreConsistent(validPreferences) + + const expected: JobsTablePreferences = { + annotationColumnKeys: ["preferenc.es/foo-alpha", "preferenc.es/foo-bravo", "preferenc.es/foo-charlie"], + expandedState: {}, + filters: [], + columnMatches: {}, + groupedColumns: [], + columnOrder: [ + StandardColumnId.TimeInState, + "annotation_preferenc.es/foo-bravo", + StandardColumnId.JobID, + + // All the following elements are added + "annotation_preferenc.es/foo-alpha", + "annotation_preferenc.es/foo-charlie", + StandardColumnId.Queue, + StandardColumnId.Namespace, + StandardColumnId.JobSet, + StandardColumnId.State, + StandardColumnId.Count, + StandardColumnId.Priority, + StandardColumnId.Owner, + StandardColumnId.CPU, + StandardColumnId.Memory, + StandardColumnId.EphemeralStorage, + StandardColumnId.GPU, + StandardColumnId.PriorityClass, + StandardColumnId.LastTransitionTimeUtc, + StandardColumnId.TimeSubmittedUtc, + StandardColumnId.TimeSubmittedAgo, + StandardColumnId.Node, + StandardColumnId.Cluster, + StandardColumnId.ExitCode, + StandardColumnId.RuntimeSeconds, + ], + order: { id: StandardColumnId.TimeInState, direction: "ASC" }, + pageIndex: 5, + pageSize: 20, + sidebarJobId: "223344", + visibleColumns: { + [StandardColumnId.JobID]: true, + [StandardColumnId.TimeInState]: true, + 'annotation_"preferenc.es/foo-bravo': true, + }, + } + + expect(validPreferences).toEqual(expected) + }) +}) diff --git a/internal/lookout/ui/src/services/lookoutV2/JobsTablePreferencesService.ts b/internal/lookout/ui/src/services/lookoutV2/JobsTablePreferencesService.ts index b47a8ae03e0..c1db172ce09 100644 --- a/internal/lookout/ui/src/services/lookoutV2/JobsTablePreferencesService.ts +++ b/internal/lookout/ui/src/services/lookoutV2/JobsTablePreferencesService.ts @@ -8,16 +8,21 @@ import { AnnotationColumnId, ColumnId, DEFAULT_COLUMN_MATCHES, - DEFAULT_COLUMN_ORDER, + DEFAULT_COLUMN_ORDERING, DEFAULT_COLUMN_VISIBILITY, + JOB_COLUMNS, + toAnnotationColId, + toColId, fromAnnotationColId, isStandardColId, + PINNED_COLUMNS, } from "../../utils/jobsTableColumns" import { matchForColumn } from "../../utils/jobsTableUtils" export interface JobsTablePreferences { annotationColumnKeys: string[] visibleColumns: VisibilityState + columnOrder: ColumnId[] groupedColumns: ColumnId[] expandedState: ExpandedStateList pageIndex: number @@ -36,13 +41,14 @@ export interface JobsTablePreferences { export const DEFAULT_PREFERENCES: JobsTablePreferences = { annotationColumnKeys: [], visibleColumns: DEFAULT_COLUMN_VISIBILITY, + columnOrder: JOB_COLUMNS.filter(({ id }) => !PINNED_COLUMNS.includes(toColId(id))).map(({ id }) => toColId(id)), filters: [], columnMatches: DEFAULT_COLUMN_MATCHES, groupedColumns: [], expandedState: {}, pageIndex: 0, pageSize: 50, - order: DEFAULT_COLUMN_ORDER, + order: DEFAULT_COLUMN_ORDERING, sidebarJobId: undefined, sidebarWidth: 600, columnSizing: {}, @@ -191,7 +197,10 @@ const mergeQueryParamsAndLocalStorage = ( return mergedPrefs } -// Make sure annotations referenced in filters exist, make sure columns referenced in objects are visible +// Ensure that: +// - annotations referenced in filters exist +// - make sure columns referenced in objects are visible +// - the column order includes exactly all unpinned standard columns and annotations export const ensurePreferencesAreConsistent = (preferences: JobsTablePreferences) => { // Make sure annotation columns referenced in filters exist if (preferences.annotationColumnKeys === undefined) { @@ -209,18 +218,35 @@ export const ensurePreferencesAreConsistent = (preferences: JobsTablePreferences } } + // Make sure all and only annotation columns and unpinned standard columns are contained in columnOrder + const columnOrderSet = new Set(preferences.columnOrder) + const annotationKeyColumnIds = preferences.annotationColumnKeys.map(toAnnotationColId) + const unpinnedStandardColumnIds = JOB_COLUMNS.filter(({ id }) => !PINNED_COLUMNS.includes(toColId(id))).map( + ({ id }) => toColId(id), + ) + + // Add missing column IDs + annotationKeyColumnIds.filter((id) => !columnOrderSet.has(id)).forEach((id) => preferences.columnOrder.push(id)) + unpinnedStandardColumnIds.filter((id) => !columnOrderSet.has(id)).forEach((id) => preferences.columnOrder.push(id)) + + // Remove extraneous column IDs + const annotationKeyColumnIdsSet = new Set(annotationKeyColumnIds) + const unpinnedStandardColumnIdsSet = new Set(unpinnedStandardColumnIds) + preferences.columnOrder = preferences.columnOrder.filter( + (id) => annotationKeyColumnIdsSet.has(id as AnnotationColumnId) || unpinnedStandardColumnIdsSet.has(id), + ) + // Make sure grouped columns, order columns, and filtered columns are visible ensureVisible(preferences.visibleColumns, preferences.groupedColumns ?? []) ensureVisible(preferences.visibleColumns, preferences.order === undefined ? [] : [preferences.order.id]) ensureVisible(preferences.visibleColumns, preferences.filters?.map((filter) => filter.id) ?? []) } -export const stringifyQueryParams = (paramObj: any): string => { - return qs.stringify(paramObj, { +export const stringifyQueryParams = (paramObj: any): string => + qs.stringify(paramObj, { encodeValuesOnly: true, strictNullHandling: true, }) -} export class JobsTablePreferencesService { constructor(private router: Router) {} diff --git a/internal/lookout/ui/src/utils/jobsTableColumns.tsx b/internal/lookout/ui/src/utils/jobsTableColumns.tsx index debca4db9ee..80f388e7129 100644 --- a/internal/lookout/ui/src/utils/jobsTableColumns.tsx +++ b/internal/lookout/ui/src/utils/jobsTableColumns.tsx @@ -468,7 +468,15 @@ export const DEFAULT_COLUMN_VISIBILITY: VisibilityState = Object.values(Standard {}, ) -export const DEFAULT_COLUMN_ORDER: LookoutColumnOrder = { id: "jobId", direction: "DESC" } +export const PINNED_COLUMNS: ColumnId[] = [StandardColumnId.SelectorCol] + +// The ordering of each column +export const DEFAULT_COLUMN_ORDERING: LookoutColumnOrder = { id: "jobId", direction: "DESC" } + +// The order of the columns in the table +export const DEFAULT_COLUMN_ORDER = JOB_COLUMNS.filter(({ id }) => !PINNED_COLUMNS.includes(toColId(id))).map( + ({ id }) => toColId(id), +) type Formatter = (val: number | string | string[]) => string diff --git a/internal/lookout/ui/yarn.lock b/internal/lookout/ui/yarn.lock index 6f7d6602b7f..f693a341de7 100644 --- a/internal/lookout/ui/yarn.lock +++ b/internal/lookout/ui/yarn.lock @@ -237,6 +237,45 @@ "@babel/helper-string-parser" "^7.25.9" "@babel/helper-validator-identifier" "^7.25.9" +"@dnd-kit/accessibility@^3.1.1": + version "3.1.1" + resolved "https://registry.yarnpkg.com/@dnd-kit/accessibility/-/accessibility-3.1.1.tgz#3b4202bd6bb370a0730f6734867785919beac6af" + integrity sha512-2P+YgaXF+gRsIihwwY1gCsQSYnu9Zyj2py8kY5fFvUM1qm2WA2u639R6YNVfU4GWr+ZM5mqEsfHZZLoRONbemw== + dependencies: + tslib "^2.0.0" + +"@dnd-kit/core@^6.3.1": + version "6.3.1" + resolved "https://registry.yarnpkg.com/@dnd-kit/core/-/core-6.3.1.tgz#4c36406a62c7baac499726f899935f93f0e6d003" + integrity sha512-xkGBRQQab4RLwgXxoqETICr6S5JlogafbhNsidmrkVv2YRs5MLwpjoF2qpiGjQt8S9AoxtIV603s0GIUpY5eYQ== + dependencies: + "@dnd-kit/accessibility" "^3.1.1" + "@dnd-kit/utilities" "^3.2.2" + tslib "^2.0.0" + +"@dnd-kit/modifiers@^9.0.0": + version "9.0.0" + resolved "https://registry.yarnpkg.com/@dnd-kit/modifiers/-/modifiers-9.0.0.tgz#96a0280c77b10c716ef79d9792ce7ad04370771d" + integrity sha512-ybiLc66qRGuZoC20wdSSG6pDXFikui/dCNGthxv4Ndy8ylErY0N3KVxY2bgo7AWwIbxDmXDg3ylAFmnrjcbVvw== + dependencies: + "@dnd-kit/utilities" "^3.2.2" + tslib "^2.0.0" + +"@dnd-kit/sortable@^10.0.0": + version "10.0.0" + resolved "https://registry.yarnpkg.com/@dnd-kit/sortable/-/sortable-10.0.0.tgz#1f9382b90d835cd5c65d92824fa9dafb78c4c3e8" + integrity sha512-+xqhmIIzvAYMGfBYYnbKuNicfSsk4RksY2XdmJhT+HAC01nix6fHCztU68jooFiMUB01Ky3F0FyOvhG/BZrWkg== + dependencies: + "@dnd-kit/utilities" "^3.2.2" + tslib "^2.0.0" + +"@dnd-kit/utilities@^3.2.2": + version "3.2.2" + resolved "https://registry.yarnpkg.com/@dnd-kit/utilities/-/utilities-3.2.2.tgz#5a32b6af356dc5f74d61b37d6f7129a4040ced7b" + integrity sha512-+MKAJEOfaBe5SmV6t34p80MMKhjvUz0vRrvVJbPT0WElzaOJ/1xs+D+KDv+tD/NE5ujfrChEcshd4fLn0wpiqg== + dependencies: + tslib "^2.0.0" + "@emotion/babel-plugin@^11.13.5": version "11.13.5" resolved "https://registry.yarnpkg.com/@emotion/babel-plugin/-/babel-plugin-11.13.5.tgz#eab8d65dbded74e0ecfd28dc218e75607c4e7bc0" @@ -4860,7 +4899,7 @@ tsconfig-paths@^3.15.0: minimist "^1.2.6" strip-bom "^3.0.0" -tslib@^2.6.2: +tslib@^2.0.0, tslib@^2.6.2: version "2.8.1" resolved "https://registry.yarnpkg.com/tslib/-/tslib-2.8.1.tgz#612efe4ed235d567e8aba5f2a5fab70280ade83f" integrity sha512-oJFu94HQb+KVduSUQL7wnpmqnfmLsOA/nAh6b6EH0wCEoK0/mPeXU6c3wKDV83MkOuHPRHtSXKKU99IBazS/2w==