Add "Parents" Columns (#4691)

extension/src/experiments/columns/collect/order.ts

@@ -1,5 +1,9 @@
 import { Column, ColumnType } from '../../webview/contract'
-import { EXPERIMENT_COLUMN_ID } from '../constants'
+import {
+  EXPERIMENT_COLUMN_ID,
+  BRANCH_COLUMN_ID,
+  COMMIT_COLUMN_ID
+} from '../constants'
 
 export const collectColumnOrder = async (
   existingColumnOrder: string[],
@@ -25,6 +29,14 @@ export const collectColumnOrder = async (
     existingColumnOrder.unshift(EXPERIMENT_COLUMN_ID)
   }
 
+  if (!existingColumnOrder.includes(BRANCH_COLUMN_ID)) {
+    existingColumnOrder.splice(1, 0, BRANCH_COLUMN_ID)
+  }
+
+  if (!existingColumnOrder.includes(COMMIT_COLUMN_ID)) {
+    existingColumnOrder.splice(2, 0, COMMIT_COLUMN_ID)
+  }
+
   return [
     ...existingColumnOrder,
     ...acc.timestamp,

extension/src/experiments/columns/constants.ts

@@ -11,3 +11,12 @@ export const timestampColumn: Column = {
 }
 
 export const EXPERIMENT_COLUMN_ID = 'id'
+
+export const COMMIT_COLUMN_ID = 'commit'
+export const BRANCH_COLUMN_ID = 'branch'
+
+export const DEFAULT_COLUMN_IDS = [
+  EXPERIMENT_COLUMN_ID,
+  BRANCH_COLUMN_ID,
+  COMMIT_COLUMN_ID
+]

extension/src/experiments/columns/model.ts

@@ -12,6 +12,11 @@ import {
   limitSummaryOrder
 } from './util'
 import { collectColumnOrder } from './collect/order'
+import {
+  BRANCH_COLUMN_ID,
+  COMMIT_COLUMN_ID,
+  DEFAULT_COLUMN_IDS
+} from './constants'
 import { Column, ColumnType } from '../webview/contract'
 import { ExpShowOutput } from '../../cli/dvc/contract'
 import { PersistenceKey } from '../../persistence/constants'
@@ -104,10 +109,10 @@ export class ColumnsModel extends PathSelectionModel<Column> {
 
   public selectFirst(firstColumns: string[]) {
     const columnOrder = [
-      'id',
+      ...DEFAULT_COLUMN_IDS,
       ...firstColumns,
       ...this.getColumnOrder().filter(
-        column => !['id', ...firstColumns].includes(column)
+        column => ![...DEFAULT_COLUMN_IDS, ...firstColumns].includes(column)
       )
     ]
     this.setColumnOrder(columnOrder)
@@ -201,6 +206,13 @@ export class ColumnsModel extends PathSelectionModel<Column> {
         return this.setColumnOrderFromData(selectedColumns)
       }
     }
+
+    const maybeMissingDefaultColumns = [COMMIT_COLUMN_ID, BRANCH_COLUMN_ID]
+    for (const id of maybeMissingDefaultColumns) {
+      if (!this.columnOrderState.includes(id)) {
+        return this.setColumnOrderFromData(selectedColumns)
+      }
+    }
   }
 
   private transformAndSetChanges(data: ExpShowOutput) {

extension/src/test/fixtures/expShow/base/columns.ts

@@ -10,6 +10,8 @@ const nestedParamsFile = join('nested', 'params.yaml')
 
 export const dataColumnOrder: string[] = [
   'id',
+  'branch',
+  'commit',
   'Created',
   'metrics:summary.json:accuracy',
   'metrics:summary.json:loss',

extension/src/test/suite/experiments/columns/tree.test.ts

@@ -363,8 +363,9 @@ suite('Experiments Columns Tree Test Suite', () => {
 
       const firstColumns = []
       const otherColumns = []
+      const defaultColumns = ['id', 'branch', 'commit']
       for (const column of columnsOrder) {
-        if (column === 'id') {
+        if (defaultColumns.includes(column)) {
           continue
         }
         if (
@@ -410,7 +411,7 @@ suite('Experiments Columns Tree Test Suite', () => {
       ])
 
       expect(columnsModel.getColumnOrder()).to.deep.equal([
-        'id',
+        ...defaultColumns,
         ...firstColumns,
         ...otherColumns
       ])

extension/src/test/suite/experiments/index.test.ts

@@ -1096,9 +1096,12 @@ suite('Experiments Test Suite', () => {
       })
       await messageSent
 
-      const [id, firstColumn] = messageSpy.lastCall.args[0].columnOrder
+      const [id, branch, commit, firstColumn] =
+        messageSpy.lastCall.args[0].columnOrder
 
       expect(id).to.equal('id')
+      expect(commit).to.equal('commit')
+      expect(branch).to.equal('branch')
       expect(firstColumn).to.equal(movedColumn)
     }).timeout(WEBVIEW_TEST_TIMEOUT)
 
@@ -1126,8 +1129,11 @@ suite('Experiments Test Suite', () => {
 
       expect(paramsYamlColumns).to.be.greaterThan(6)
 
-      const [id, ...columns] = messageSpy.lastCall.args[0].columnOrder
+      const [id, branch, commit, ...columns] =
+        messageSpy.lastCall.args[0].columnOrder
       expect(id).to.equal('id')
+      expect(branch).to.equal('branch')
+      expect(commit).to.equal('commit')
 
       let params = 0
       let other = 0

webview/src/experiments/components/App.test.tsx

@@ -8,6 +8,7 @@ import {
 } from '@testing-library/react'
 import '@testing-library/jest-dom'
 import tableDataFixture from 'dvc/src/test/fixtures/expShow/base/tableData'
+import sortedTableData from 'dvc/src/test/fixtures/expShow/sorted/tableData'
 import { MessageFromWebviewType } from 'dvc/src/webview/contract'
 import {
   Column,
@@ -25,7 +26,7 @@ import { vsCodeApi } from '../../shared/api'
 import {
   commonColumnFields,
   expectHeaders,
-  tableData as sortingTableDataFixture
+  tableData as simplifiedSortedTableDataFixture
 } from '../../test/sort'
 import {
   NORMAL_TOOLTIP_DELAY,
@@ -61,6 +62,11 @@ const tableStateFixture = {
   columnData: collectColumnData(tableDataFixture.columns)
 }
 
+const sortedTableStateFixture = {
+  ...sortedTableData,
+  columnData: collectColumnData(sortedTableData.columns)
+}
+
 jest.mock('../../shared/api')
 jest.mock('../../util/styles')
 jest.mock('./overflowHoverTooltip/useIsFullyContained', () => ({
@@ -99,6 +105,19 @@ describe('App', () => {
     expect(noColumnsState).toBeInTheDocument()
   })
 
+  it('should show the no columns selected empty state when there are no columns provided and the table is sorted', () => {
+    const { columns } = tableStateFixture
+    const sortPath = columns[columns.length - 1].path
+    renderTable({
+      ...tableDataFixture,
+      columns: [],
+      sorts: [{ descending: true, path: sortPath }]
+    })
+
+    const noColumnsState = screen.queryByText('No Columns Selected.')
+    expect(noColumnsState).toBeInTheDocument()
+  })
+
   it('should not show the no columns selected empty state when only the timestamp column is provided', () => {
     renderTable({
       ...tableStateFixture,
@@ -144,9 +163,9 @@ describe('App', () => {
     const { getDraggableHeaderFromText } = renderTableWithSortingData()
 
     setTableData({
-      ...sortingTableDataFixture,
+      ...simplifiedSortedTableDataFixture,
       columns: [
-        ...sortingTableDataFixture.columns,
+        ...simplifiedSortedTableDataFixture.columns,
         {
           ...commonColumnFields,
           id: 'D',
@@ -164,6 +183,48 @@ describe('App', () => {
     await expectHeaders(['A', 'C', 'D', 'B'])
   })
 
+  it('should add a "branch/tags" column if the table is sorted', () => {
+    renderTable(sortedTableStateFixture)
+
+    const branchHeader = screen.getByTestId('header-branch')
+    expect(branchHeader).toBeInTheDocument()
+
+    const branchHeaderTextContent =
+      within(branchHeader).getByText('Branch/Tags')
+    expect(branchHeader).toBeInTheDocument()
+
+    fireEvent.mouseEnter(branchHeaderTextContent, { bubbles: true })
+    expect(screen.getByRole('tooltip')).toBeInTheDocument()
+    expect(screen.getByRole('tooltip')).toHaveTextContent(
+      'The table has limited functionality while sorted. Clear all sorts to have nested rows and increase/decrease commits.'
+    )
+
+    expect(screen.getByTestId('branch___main').textContent).toStrictEqual(
+      'main'
+    )
+  })
+
+  it('should add a "parent" column if the table is sorted', () => {
+    renderTable(sortedTableStateFixture)
+
+    const commitHeader = screen.getByTestId('header-commit')
+    expect(commitHeader).toBeInTheDocument()
+
+    const commitHeaderTextContent = within(commitHeader).getByText('Parent')
+    expect(commitHeader).toBeInTheDocument()
+
+    fireEvent.mouseEnter(commitHeaderTextContent, { bubbles: true })
+    expect(screen.getByRole('tooltip')).toBeInTheDocument()
+    expect(screen.getByRole('tooltip')).toHaveTextContent(
+      'The table has limited functionality while sorted. Clear all sorts to have nested rows and increase/decrease commits.'
+    )
+
+    expect(screen.getByTestId('commit___main').textContent).toStrictEqual('')
+    expect(screen.getByTestId('commit___exp-83425').textContent).toStrictEqual(
+      '53c3851'
+    )
+  })
+
   describe('Row expansion', () => {
     const experimentLabel = '4fb124a'
 

webview/src/experiments/components/Experiments.tsx

@@ -25,6 +25,7 @@ import { WebviewWrapper } from '../../shared/components/webviewWrapper/WebviewWr
 import { EmptyState } from '../../shared/components/emptyState/EmptyState'
 import { ExperimentsState } from '../store'
 import { resizeColumn } from '../util/messages'
+import { isDefaultColumn } from '../util/columns'
 
 const DEFAULT_COLUMN_WIDTH = 90
 const MINIMUM_COLUMN_WIDTH = 90
@@ -56,12 +57,15 @@ export const ExperimentsTable: React.FC = () => {
     columnOrder: columnOrderData,
     columnWidths,
     hasConfig,
-    rows: data
+    rows: data,
+    sorts
   } = useSelector((state: ExperimentsState) => state.tableData)
 
   const [expanded, setExpanded] = useState({})
 
-  const [columns, setColumns] = useState(buildColumns(columnData))
+  const [columns, setColumns] = useState(
+    buildColumns(columnData, sorts.length > 0)
+  )
   const [columnSizing, setColumnSizing] =
     useState<ColumnSizingState>(columnWidths)
   const [columnOrder, setColumnOrder] = useState(columnOrderData)
@@ -72,8 +76,8 @@ export const ExperimentsTable: React.FC = () => {
   }, [columnSizing, columnWidths])
 
   useEffect(() => {
-    setColumns(buildColumns(columnData))
-  }, [columnData])
+    setColumns(buildColumns(columnData, sorts.length > 0))
+  }, [columnData, sorts])
 
   const getRowId = useCallback(
     (experiment: Commit, relativeIndex: number, parent?: TableRow<Commit>) =>
@@ -108,7 +112,9 @@ export const ExperimentsTable: React.FC = () => {
     toggleAllRowsExpanded()
   }, [toggleAllRowsExpanded])
 
-  const hasOnlyDefaultColumns = columns.length <= 1
+  const hasOnlyDefaultColumns = columns.every(
+    ({ id }) => id && isDefaultColumn(id)
+  )
   if (hasOnlyDefaultColumns) {
     return <AddColumns />
   }