how about using redux instead of prop drilling

iterative · May 5, 2023 · 2c9eb22 · 2c9eb22
1 parent ed25fc6
commit 2c9eb22
Show file tree

Hide file tree

Showing 9 changed files with 177 additions and 85 deletions.
diff --git a/webview/src/experiments/components/App.test.tsx b/webview/src/experiments/components/App.test.tsx
@@ -990,7 +990,7 @@ describe('App', () => {
       expect(screen.queryAllByRole('menuitem')).toHaveLength(10)
     })
 
-    it('should enable the Remove experiment option for the checkpoint tips', () => {
+    it('should enable the remove option for an experiment', () => {
       renderTableWithoutRunningExperiments()
 
       const target = screen.getByText('4fb124a')
@@ -1034,6 +1034,23 @@ describe('App', () => {
       })
     })
 
+    it('should disable the remove selected option if an experiment is running in the workspace', () => {
+      renderTable()
+
+      clickRowCheckbox('4fb124a')
+      clickRowCheckbox('42b8736')
+
+      const target = screen.getByText('4fb124a')
+      fireEvent.contextMenu(target, { bubbles: true })
+
+      advanceTimersByTime(100)
+      const menuitems = screen.getAllByRole('menuitem')
+      const itemLabels = menuitems
+        .filter(item => !item.className.includes('disabled'))
+        .map(item => item.textContent)
+      expect(itemLabels).not.toContain('Remove Selected')
+    })
+
     it('should enable the push option if only experiments are selected', () => {
       renderTableWithoutRunningExperiments()
 
@@ -1064,6 +1081,23 @@ describe('App', () => {
       })
     })
 
+    it('should disable the push selected option if an experiment is running in the workspace', () => {
+      renderTable()
+
+      clickRowCheckbox('4fb124a')
+      clickRowCheckbox('42b8736')
+
+      const target = screen.getByText('4fb124a')
+      fireEvent.contextMenu(target, { bubbles: true })
+
+      advanceTimersByTime(100)
+      const menuitems = screen.getAllByRole('menuitem')
+      const itemLabels = menuitems
+        .filter(item => !item.className.includes('disabled'))
+        .map(item => item.textContent)
+      expect(itemLabels).not.toContain('Push Selected')
+    })
+
     it('should enable the stop option if only running experiments are selected', () => {
       renderTable()
 
@@ -1093,6 +1127,53 @@ describe('App', () => {
       })
     })
 
+    it('should enable the stop option if multiple running experiments are selected', () => {
+      renderTable()
+
+      clickRowCheckbox('4fb124a')
+      clickRowCheckbox('[exp-83425]')
+
+      const target = screen.getByText('4fb124a')
+      fireEvent.contextMenu(target, { bubbles: true })
+
+      advanceTimersByTime(100)
+      const menuitems = screen.getAllByRole('menuitem')
+      const itemLabels = menuitems
+        .filter(item => !item.className.includes('disabled'))
+        .map(item => item.textContent)
+      expect(itemLabels).toContain('Stop')
+
+      const stopOption = menuitems.find(item =>
+        item.textContent?.includes('Stop')
+      )
+
+      expect(stopOption).toBeDefined()
+
+      stopOption && fireEvent.click(stopOption)
+
+      expect(sendMessage).toHaveBeenCalledWith({
+        payload: ['exp-e7a67', 'exp-83425'],
+        type: MessageFromWebviewType.STOP_EXPERIMENT
+      })
+    })
+
+    it('should disable the stop option if finished experiments are selected', () => {
+      renderTable()
+
+      clickRowCheckbox('4fb124a')
+      clickRowCheckbox('90aea7f')
+
+      const target = screen.getByText('4fb124a')
+      fireEvent.contextMenu(target, { bubbles: true })
+
+      advanceTimersByTime(100)
+      const menuitems = screen.getAllByRole('menuitem')
+      const itemLabels = menuitems
+        .filter(item => !item.className.includes('disabled'))
+        .map(item => item.textContent)
+      expect(itemLabels).not.toContain('Stop')
+    })
+
     it('should enable the user to stop an experiment running in the workspace', () => {
       renderTable()
 
@@ -1149,7 +1230,7 @@ describe('App', () => {
       })
     })
 
-    it('should not enable the user to share an experiment whilst an experiment is running', () => {
+    it('should not enable the user to share a running experiment', () => {
       renderTable()
 
       const target = screen.getByText('4fb124a')
@@ -1163,6 +1244,20 @@ describe('App', () => {
       expect(itemLabels).not.toContain('Push')
     })
 
+    it('should not enable the user to share an experiment whilst one is running in the workspace', () => {
+      renderTable()
+
+      const target = screen.getByText('42b8736')
+      fireEvent.contextMenu(target, { bubbles: true })
+
+      advanceTimersByTime(100)
+      const menuitems = screen.getAllByRole('menuitem')
+      const itemLabels = menuitems
+        .filter(item => !item.className.includes('disabled'))
+        .map(item => item.textContent)
+      expect(itemLabels).not.toContain('Push')
+    })
+
     it('should always enable the Plots options if multiple rows are selected', () => {
       renderTableWithoutRunningExperiments()
 

diff --git a/webview/src/experiments/components/App.tsx b/webview/src/experiments/components/App.tsx
@@ -74,7 +74,7 @@ export const App: React.FC<Record<string, unknown>> = () => {
               case 'hasMoreCommits':
                 dispatch(updateHasMoreCommits(data.data.hasMoreCommits))
                 continue
-              case 'hasRunningExperiment':
+              case 'hasRunningWorkspaceExperiment':
                 dispatch(
                   updateHasRunningWorkspaceExperiment(
                     data.data.hasRunningWorkspaceExperiment

diff --git a/webview/src/experiments/components/table/body/ExperimentGroup.tsx b/webview/src/experiments/components/table/body/ExperimentGroup.tsx
@@ -5,14 +5,5 @@ import { RowProp } from '../../../util/interfaces'
 
 export const ExperimentGroup: React.FC<RowProp & BatchSelectionProp> = ({
   row,
-  projectHasCheckpoints,
-  hasRunningWorkspaceExperiment,
   batchRowSelection
-}) => (
-  <NestedRow
-    row={row}
-    projectHasCheckpoints={projectHasCheckpoints}
-    hasRunningWorkspaceExperiment={hasRunningWorkspaceExperiment}
-    batchRowSelection={batchRowSelection}
-  />
-)
+}) => <NestedRow row={row} batchRowSelection={batchRowSelection} />
diff --git a/webview/src/experiments/components/table/body/NestedRow.tsx b/webview/src/experiments/components/table/body/NestedRow.tsx
@@ -5,16 +5,12 @@ import { RowProp } from '../../../util/interfaces'
 
 export const NestedRow: React.FC<RowProp & BatchSelectionProp> = ({
   row,
-  projectHasCheckpoints,
-  hasRunningWorkspaceExperiment,
   batchRowSelection
 }) => {
   return (
     <RowContent
       row={row}
       className={styles.nestedRow}
-      projectHasCheckpoints={projectHasCheckpoints}
-      hasRunningWorkspaceExperiment={hasRunningWorkspaceExperiment}
       batchRowSelection={batchRowSelection}
     />
   )

diff --git a/webview/src/experiments/components/table/body/Row.tsx b/webview/src/experiments/components/table/body/Row.tsx
@@ -19,13 +19,7 @@ export type BatchSelectionProp = {
 
 export const RowContent: React.FC<
   RowProp & { className?: string } & BatchSelectionProp
-> = ({
-  row,
-  className,
-  projectHasCheckpoints,
-  hasRunningWorkspaceExperiment,
-  batchRowSelection
-}): JSX.Element => {
+> = ({ row, className, batchRowSelection }): JSX.Element => {
   const changes = useSelector(
     (state: ExperimentsState) => state.tableData.changes
   )
@@ -73,15 +67,7 @@ export const RowContent: React.FC<
   const isOdd = index % 2 !== 0 && !isRowSelected
 
   return (
-    <ContextMenu
-      content={
-        <RowContextMenu
-          row={row}
-          projectHasCheckpoints={projectHasCheckpoints}
-          hasRunningWorkspaceExperiment={hasRunningWorkspaceExperiment}
-        />
-      }
-    >
+    <ContextMenu content={<RowContextMenu row={row} />}>
       <tr
         className={cx(
           className,