Arvados

22143-process-page-crash @ 62c20a4bdb2f56ead82573ff7590ada4614b92da

developer-run-tests-services-workbench2: #1235

• ✅ All agreed upon points are implemented / addressed.
• ✅ Anything not implemented (discovered or discussed during work) has a follow-up story.
• ✅ Code is tested and passing, both automated and manual, what manual testing was done is described
• - Documentation has been updated.
• - Behaves appropriately at the intended scale (describe intended scale).
• ✅ Considered backwards and forwards compatibility issues between client and server.
  no changes
• ✅ Follows our coding standards and GUI style guidelines.

• I was able to reproduce the bug at one point but then I was unable to reproduce it again. I believe when I reproduced it, I had switched from one branch to another with a different package.json without re-installing, but I am unable to confirm this. I addressed what I could by adding a null check where the stack trace points, but until we can reproduce it we can't be 100% sure that it's fixed.

I think this is ok as a workaround, but I'm also wondering about one of the uses of mapNodeValue. setNodeValue and all uses of setNodeValueWith explicitly pass in anonymous functions for mapFn, and the last use, mapTreeValues, is mostly called passing in an anonymous function, the only exception being 2 uses where it uses services.collectionService.extendFileURL as the mapFn. I noticed that one of these in collection-panel-files-actions.ts references the collectionService using servicesProvider.getServices().collectionService.extendFileURL instead of using a thunk to access services, which results in extendFileURL being inferred as an any type, and I'm wondering if maybe there isn't a guarantee that getServices() is reliable. So I would suggest trying to change that action into a thunk which seems to satisfy the type system better and makes mapped be a proper tree type instead of any. I have no clue if this will fix the issue but I think it would be good to do as long as the collection file browser seems unaffected, which from what I can tell should be fine.

--- a/services/workbench2/src/components/collection-panel-files/collection-panel-files.tsx
+++ b/services/workbench2/src/components/collection-panel-files/collection-panel-files.tsx
@@ -335,12 +335,12 @@ export const CollectionPanelFiles = withStyles(styles)(
                 fetchData([leftKey, rightKey], true);
             }
             // eslint-disable-next-line react-hooks/exhaustive-deps
         }, [currentPDH]); 

         React.useEffect(() => {
             if (rightData) {
                 const filtered = rightData.filter(({ name }) => name.indexOf(rightSearch) > -1);
-                setCollectionFiles(filtered, false)(dispatch);
+                dispatch(setCollectionFiles(filtered, false));
             }
         }, [rightData, dispatch, rightSearch]);
diff --git a/services/workbench2/src/store/collection-panel/collection-panel-files/collection-panel-files-actions.ts b/services/workbench2/src/store/collection-panel/collection-panel-files/collection-panel-files-actions.ts
index 547f1534d1..4e943e42f7 100644
--- a/services/workbench2/src/store/collection-panel/collection-panel-files/collection-panel-files-actions.ts
+++ b/services/workbench2/src/store/collection-panel/collection-panel-files/collection-panel-files-actions.ts
@@ -29,10 +29,10 @@ export type CollectionPanelFilesAction = UnionOf<typeof collectionPanelFilesActi

 export const COLLECTION_PANEL_LOAD_FILES = 'collectionPanelLoadFiles';

-export const setCollectionFiles = (files, joinParents = true) => (dispatch: any) => {
+export const setCollectionFiles = (files, joinParents = true) => (dispatch: Dispatch, getState: () => RootState, services: ServiceRepository) => {
     const tree = createCollectionFilesTree(files, joinParents);
     const sorted = sortFilesTree(tree);
-    const mapped = mapTreeValues(servicesProvider.getServices().collectionService.extendFileURL)(sorted);
+    const mapped = mapTreeValues(services.collectionService.extendFileURL)(sorted);
     dispatch(collectionPanelFilesAction.SET_COLLECTION_FILES(mapped));
 };

22143-process-page-crash @ commit: 89d4d8b32e7783855ed8ab3b2ea3861a3e60b28f

developer-run-tests-services-workbench2: #1238

I took your suggestion and also added typing for the `pathData` variable while I was there.

This lgtm!