Fix showing file contents for record item (#2455)

fix #2446 to test: create a schema with a field using files: % schema a % add hello /stdout display bundles in record mode % display record a
codalab · Jun 19, 2020 · 4814d6e · 4814d6e
1 parent 0d9d47b
commit 4814d6e
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 31 deletions.
diff --git a/frontend/src/components/worksheets/items/RecordItem.js b/frontend/src/components/worksheets/items/RecordItem.js
@@ -2,7 +2,9 @@ import * as React from 'react';
 import * as Mousetrap from '../../../util/ws_mousetrap_fork';
 import BundleDetail from '../BundleDetail';
 import NewRun from '../NewRun';
-import { worksheetItemPropsChanged } from '../../../util/worksheet_utils';
+import { useEffect } from 'react';
+import { fetchAsyncBundleContents } from '../../../util/async_loading_utils';
+import { FETCH_STATUS_SCHEMA } from '../../../constants';
 
 class RecordItem extends React.Component {
     /** Constructor. */
@@ -28,13 +30,6 @@ class RecordItem extends React.Component {
         this.setState({ bundleInfoUpdates: { ...bundleInfoUpdates, ...update } });
     };
 
-    shouldComponentUpdate(nextProps, nextState) {
-        return (
-            worksheetItemPropsChanged(this.props, nextProps) ||
-            this.state.showDetail !== nextState.showDetail
-        );
-    }
-
     rerunItem = (runProp) => {
         this.setState({
             showDetail: false,
@@ -130,4 +125,33 @@ class RecordItem extends React.Component {
     }
 }
 
-export default RecordItem;
+const RecordWrapper = (props) => {
+    const { item, onAsyncItemLoad } = props;
+    useEffect(() => {
+        (async function() {
+            if (item.status.code === FETCH_STATUS_SCHEMA.BRIEFLY_LOADED) {
+                try {
+                    const { contents } = await fetchAsyncBundleContents({ contents: item.rows });
+                    onAsyncItemLoad({
+                        ...item,
+                        rows: contents,
+                        status: {
+                            code: FETCH_STATUS_SCHEMA.READY,
+                            error_message: '',
+                        },
+                    });
+                } catch (e) {
+                    console.error(e);
+                    // TODO: better error message handling here.
+                }
+            }
+        })();
+        // TODO: see how we can add onAsyncItemLoad as a dependency, if needed.
+        // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, [item.rows, item.status]);
+    return <RecordItem {...props} />;
+};
+
+export default RecordWrapper;
+
+// export default RecordItem;
diff --git a/frontend/src/components/worksheets/items/TableItem/TableItem.js b/frontend/src/components/worksheets/items/TableItem/TableItem.js
@@ -1,18 +1,13 @@
 // @flow
 import React, { useEffect } from 'react';
-import $ from 'jquery';
 import { withStyles } from '@material-ui/core';
 import Table from '@material-ui/core/Table';
 import TableHead from '@material-ui/core/TableHead';
 import TableCell from './TableCell';
 import TableRow from '@material-ui/core/TableRow';
 import BundleRow from './BundleRow';
-import Checkbox from '@material-ui/core/Checkbox';
-import CheckBoxOutlineBlankIcon from '@material-ui/icons/CheckBoxOutlineBlank';
-import SvgIcon from '@material-ui/core/SvgIcon';
-import CheckBoxIcon from '@material-ui/icons/CheckBox';
 import { getIds } from '../../../../util/worksheet_utils';
-import { semaphore } from '../../../../util/async_loading_utils';
+import { fetchAsyncBundleContents } from '../../../../util/async_loading_utils';
 import { FETCH_STATUS_SCHEMA } from '../../../../constants';
 
 class TableItem extends React.Component<{
@@ -233,28 +228,13 @@ const styles = (theme) => ({
 
 const TableContainer = withStyles(styles)(_TableContainer);
 
-async function fetchAsyncTableContents({ contents }) {
-    return semaphore.use(async () => {
-        const response = await $.ajax({
-            type: 'POST',
-            contentType: 'application/json',
-            url: '/rest/interpret/genpath-table-contents',
-            async: true,
-            data: JSON.stringify({ contents }),
-            dataType: 'json',
-            cache: false,
-        });
-        return response;
-    });
-}
-
 const TableWrapper = (props) => {
     const { item, onAsyncItemLoad } = props;
     useEffect(() => {
         (async function() {
             if (item.status.code === FETCH_STATUS_SCHEMA.BRIEFLY_LOADED) {
                 try {
-                    const { contents } = await fetchAsyncTableContents({ contents: item.rows });
+                    const { contents } = await fetchAsyncBundleContents({ contents: item.rows });
                     onAsyncItemLoad({
                         ...item,
                         rows: contents,

diff --git a/frontend/src/util/async_loading_utils.js b/frontend/src/util/async_loading_utils.js
@@ -1,5 +1,22 @@
 import { Semaphore } from 'await-semaphore';
+import $ from 'jquery';
 
 // Limit concurrent requests for async resolving items
 const MAX_CONCURRENT_REQUESTS = 3;
 export const semaphore = new Semaphore(MAX_CONCURRENT_REQUESTS);
+
+export async function fetchAsyncBundleContents({ contents }) {
+    // used in table and record items
+    return semaphore.use(async () => {
+        const response = await $.ajax({
+            type: 'POST',
+            contentType: 'application/json',
+            url: '/rest/interpret/genpath-table-contents',
+            async: true,
+            data: JSON.stringify({ contents }),
+            dataType: 'json',
+            cache: false,
+        });
+        return response;
+    });
+}