Rework experiment table context menus to vary based on whether or not…

… experiments have checkpoints (#1739) * update experiment table context menus based on whether or not there are checkpoints in the project * evaluate is not checkpoint once * be smarter with how we push items into the menu
iterative · May 19, 2022 · cef7e89 · cef7e89
1 parent 8bcc0ca
commit cef7e89
Show file tree

Hide file tree

Showing 14 changed files with 66 additions and 18 deletions.
diff --git a/extension/src/experiments/index.ts b/extension/src/experiments/index.ts
@@ -379,6 +379,7 @@ export class Experiments extends BaseRepository<TableData> {
       columnOrder: this.columns.getColumnOrder(),
       columnWidths: this.columns.getColumnWidths(),
       columns: this.columns.getSelected(),
+      hasCheckpoints: this.hasCheckpoints(),
       rows: this.experiments.getRowData(),
       sorts: this.experiments.getSorts()
     }

diff --git a/extension/src/experiments/webview/contract.ts b/extension/src/experiments/webview/contract.ts
@@ -43,12 +43,13 @@ export interface Column extends ColumnAggregateData {
 }
 
 export type TableData = {
-  rows: Row[]
-  columns: Column[]
-  sorts: SortDefinition[]
   changes: string[]
   columnOrder: string[]
+  columns: Column[]
   columnWidths: Record<string, number>
+  hasCheckpoints: boolean
+  rows: Row[]
+  sorts: SortDefinition[]
 }
 
 export type InitiallyUndefinedTableData = TableData | undefined
diff --git a/extension/src/test/fixtures/expShow/deeplyNested.ts b/extension/src/test/fixtures/expShow/deeplyNested.ts
@@ -83,6 +83,7 @@ const deeplyNestedTableData: TableData = {
   changes: [],
   columnOrder: [],
   columnWidths: {},
+  hasCheckpoints: false,
   sorts: [
     {
       path: 'params:params.yaml:nested1.doubled',

diff --git a/extension/src/test/fixtures/expShow/tableData.ts b/extension/src/test/fixtures/expShow/tableData.ts
@@ -5,6 +5,7 @@ import columnsFixture from './columns'
 const tableDataFixture: TableData = {
   rows: rowsFixture,
   columns: columnsFixture,
+  hasCheckpoints: true,
   sorts: [],
   changes: [],
   columnOrder: [],

diff --git a/extension/src/test/suite/experiments/index.test.ts b/extension/src/test/suite/experiments/index.test.ts
@@ -112,6 +112,7 @@ suite('Experiments Test Suite', () => {
         columnOrder: [],
         columnWidths: {},
         columns: columnsFixture,
+        hasCheckpoints: true,
         rows: rowsFixture,
         sorts: []
       }

diff --git a/extension/src/test/suite/experiments/model/filterBy/tree.test.ts b/extension/src/test/suite/experiments/model/filterBy/tree.test.ts
@@ -109,6 +109,7 @@ suite('Experiments Filter By Tree Test Suite', () => {
         columnOrder: [],
         columnWidths: {},
         columns: columnsFixture,
+        hasCheckpoints: true,
         rows: filteredRows,
         sorts: []
       })
@@ -133,6 +134,7 @@ suite('Experiments Filter By Tree Test Suite', () => {
         columnOrder: [],
         columnWidths: {},
         columns: columnsFixture,
+        hasCheckpoints: true,
         rows: [workspace, main],
         sorts: []
       }

diff --git a/webview/src/experiments/components/App.test.tsx b/webview/src/experiments/components/App.test.tsx
@@ -25,6 +25,7 @@ import {
   DND_DIRECTION_RIGHT
 } from 'react-beautiful-dnd-test-utils'
 import {
+  Column,
   ColumnType,
   Row,
   TableData
@@ -221,7 +222,7 @@ describe('App', () => {
           id: 'D',
           name: 'D',
           path: 'params:D'
-        }
+        } as Column
       ]
     }
 

diff --git a/webview/src/experiments/components/Experiments.tsx b/webview/src/experiments/components/Experiments.tsx
@@ -134,6 +134,7 @@ export const ExperimentsTable: React.FC<{
         columnOrder: [],
         columnWidths: {},
         columns: [],
+        hasCheckpoints: false,
         rows: [],
         sorts: [],
         ...initiallyUndefinedTableData

diff --git a/webview/src/experiments/components/table/Row.tsx b/webview/src/experiments/components/table/Row.tsx
@@ -40,6 +40,7 @@ const experimentMenuOption = (
 }
 
 export const RowContextMenu: React.FC<RowProp> = ({
+  projectHasCheckpoints = false,
   row: {
     original: { queued },
     depth,
@@ -75,17 +76,25 @@ export const RowContextMenu: React.FC<RowProp> = ({
       )
     ])
 
-    pushIf(depth <= 1 || isWorkspace, [
+    const isNotCheckpoint = depth <= 1 || isWorkspace
+
+    pushIf(isNotCheckpoint, [
       experimentMenuOption(
         id,
-        'Modify and Run',
+        projectHasCheckpoints ? 'Modify and Resume' : 'Modify and Run',
         MessageFromWebviewType.VARY_EXPERIMENT_PARAMS_AND_RUN
-      ),
+      )
+    ])
+
+    pushIf(isNotCheckpoint && projectHasCheckpoints, [
       experimentMenuOption(
         id,
-        'Modify Reset and Run',
+        'Modify, Reset and Run',
         MessageFromWebviewType.VARY_EXPERIMENT_PARAMS_RESET_AND_RUN
-      ),
+      )
+    ])
+
+    pushIf(isNotCheckpoint, [
       experimentMenuOption(
         id,
         'Modify and Queue',
@@ -94,14 +103,20 @@ export const RowContextMenu: React.FC<RowProp> = ({
     ])
 
     return menuOptions
-  }, [queued, isWorkspace, depth, id])
+  }, [queued, isWorkspace, depth, id, projectHasCheckpoints])
 
   return <MessagesMenu options={contextMenuOptions} />
 }
 
 export const RowContent: React.FC<
   RowProp & { className?: string } & WithChanges
-> = ({ row, className, changes, contextMenuDisabled }): JSX.Element => {
+> = ({
+  row,
+  className,
+  changes,
+  contextMenuDisabled,
+  projectHasCheckpoints
+}): JSX.Element => {
   const {
     getRowProps,
     cells: [firstCell, ...cells],
@@ -124,7 +139,12 @@ export const RowContent: React.FC<
   return (
     <ContextMenu
       disabled={contextMenuDisabled}
-      content={<RowContextMenu row={row} />}
+      content={
+        <RowContextMenu
+          row={row}
+          projectHasCheckpoints={projectHasCheckpoints}
+        />
+      }
     >
       <div
         {...getRowProps({

diff --git a/webview/src/experiments/components/table/Table.test.tsx b/webview/src/experiments/components/table/Table.test.tsx
@@ -116,6 +116,7 @@ describe('Table', () => {
     columnOrder: [],
     columnWidths: {},
     columns: [],
+    hasCheckpoints: false,
     rows: [],
     sorts: []
   }

diff --git a/webview/src/experiments/components/table/Table.tsx b/webview/src/experiments/components/table/Table.tsx
@@ -9,22 +9,25 @@ import { InstanceProp, RowProp, TableProps, WithChanges } from './interfaces'
 export const NestedRow: React.FC<RowProp & InstanceProp> = ({
   row,
   instance,
-  contextMenuDisabled
+  contextMenuDisabled,
+  projectHasCheckpoints
 }) => {
   instance.prepareRow(row)
   return (
     <RowContent
       row={row}
       className={styles.nestedRow}
       contextMenuDisabled={contextMenuDisabled}
+      projectHasCheckpoints={projectHasCheckpoints}
     />
   )
 }
 
 export const ExperimentGroup: React.FC<RowProp & InstanceProp> = ({
   row,
   instance,
-  contextMenuDisabled
+  contextMenuDisabled,
+  projectHasCheckpoints
 }) => {
   instance.prepareRow(row)
   return (
@@ -38,6 +41,7 @@ export const ExperimentGroup: React.FC<RowProp & InstanceProp> = ({
         row={row}
         instance={instance}
         contextMenuDisabled={contextMenuDisabled}
+        projectHasCheckpoints={projectHasCheckpoints}
       />
       {row.isExpanded &&
         row.subRows.map(row => (
@@ -46,6 +50,7 @@ export const ExperimentGroup: React.FC<RowProp & InstanceProp> = ({
             instance={instance}
             key={row.id}
             contextMenuDisabled={contextMenuDisabled}
+            projectHasCheckpoints={projectHasCheckpoints}
           />
         ))}
     </div>
@@ -56,7 +61,8 @@ export const TableBody: React.FC<RowProp & InstanceProp & WithChanges> = ({
   row,
   instance,
   changes,
-  contextMenuDisabled
+  contextMenuDisabled,
+  projectHasCheckpoints
 }) => {
   instance.prepareRow(row)
   return (
@@ -73,6 +79,7 @@ export const TableBody: React.FC<RowProp & InstanceProp & WithChanges> = ({
     >
       <RowContent
         row={row}
+        projectHasCheckpoints={projectHasCheckpoints}
         changes={changes}
         contextMenuDisabled={contextMenuDisabled}
       />
@@ -83,6 +90,7 @@ export const TableBody: React.FC<RowProp & InstanceProp & WithChanges> = ({
             instance={instance}
             key={subRow.values.id}
             contextMenuDisabled={contextMenuDisabled}
+            projectHasCheckpoints={projectHasCheckpoints}
           />
         ))}
     </div>
@@ -94,7 +102,13 @@ export const Table: React.FC<TableProps & WithChanges> = ({
   tableData
 }) => {
   const { getTableProps, rows } = instance
-  const { sorts, columns, changes, rows: experimentRows } = tableData
+  const {
+    sorts,
+    columns,
+    changes,
+    rows: experimentRows,
+    hasCheckpoints
+  } = tableData
 
   const someExperimentsRunning = React.useMemo(() => {
     const findRunningExperiment: (experiments?: Row[]) => boolean = (
@@ -120,6 +134,7 @@ export const Table: React.FC<TableProps & WithChanges> = ({
             key={row.id}
             changes={changes}
             contextMenuDisabled={someExperimentsRunning}
+            projectHasCheckpoints={hasCheckpoints}
           />
         ))}
       </div>

diff --git a/webview/src/experiments/components/table/interfaces.ts b/webview/src/experiments/components/table/interfaces.ts
@@ -16,6 +16,7 @@ export interface WithChanges {
 export interface RowProp {
   row: Row<Experiment>
   contextMenuDisabled?: boolean
+  projectHasCheckpoints?: boolean
 }
 
 export interface CellProp {

diff --git a/webview/src/stories/Table.stories.tsx b/webview/src/stories/Table.stories.tsx
@@ -17,6 +17,7 @@ const tableData: TableData = {
     'params:params.yaml:dvc_logs_dir': 300
   },
   columns: columnsFixture,
+  hasCheckpoints: true,
   rows: rowsFixture.map(row => ({
     ...row,
     subRows: row.subRows?.map(experiment => ({

diff --git a/webview/src/test/sort.ts b/webview/src/test/sort.ts
@@ -1,5 +1,5 @@
 import { screen } from '@testing-library/react'
-import { ColumnType } from 'dvc/src/experiments/webview/contract'
+import { ColumnType, TableData } from 'dvc/src/experiments/webview/contract'
 import { DND_DRAGGABLE_DATA_ATTR } from 'react-beautiful-dnd-test-utils'
 
 export const defaultColumns = ['Experiment', 'Timestamp']
@@ -31,11 +31,12 @@ export const columns = [
   }
 ]
 
-export const tableData = {
+export const tableData: TableData = {
   changes: [],
   columnOrder: [],
   columnWidths: {},
   columns,
+  hasCheckpoints: false,
   rows: [
     {
       id: 'workspace',