Merge pull request #458 from bento-platform/fix/data-type-summary-fetch

fix: error boundary for data type summaries + fix variant crash

src/components/datasets/Dataset.js

@@ -13,7 +13,7 @@ import {
   deleteDatasetLinkedFieldSetIfPossible,
 } from "@/modules/metadata/actions";
 
-import { fetchDatasetDataTypesSummariesIfPossible, fetchDatasetSummariesIfPossible } from "@/modules/datasets/actions";
+import { fetchDatasetDataTypesIfPossible, fetchDatasetSummariesIfNeeded } from "@/modules/datasets/actions";
 
 import { INITIAL_DATA_USE_VALUE } from "@/duo";
 import { simpleDeepCopy, nop } from "@/utils/misc";
@@ -335,8 +335,8 @@ const mapDispatchToProps = (dispatch, ownProps) => ({
   deleteProjectDataset: (dataset) => dispatch(deleteProjectDatasetIfPossible(ownProps.project, dataset)),
   deleteLinkedFieldSet: (dataset, linkedFieldSet, linkedFieldSetIndex) =>
     dispatch(deleteDatasetLinkedFieldSetIfPossible(dataset, linkedFieldSet, linkedFieldSetIndex)),
-  fetchDatasetSummary: (datasetId) => dispatch(fetchDatasetSummariesIfPossible(datasetId)),
-  fetchDatasetDataTypesSummary: (datasetId) => dispatch(fetchDatasetDataTypesSummariesIfPossible(datasetId)),
+  fetchDatasetSummary: (datasetId) => dispatch(fetchDatasetSummariesIfNeeded(datasetId)),
+  fetchDatasetDataTypesSummary: (datasetId) => dispatch(fetchDatasetDataTypesIfPossible(datasetId)),
 });
 
 export default connect(mapStateToProps, mapDispatchToProps)(Dataset);

src/components/datasets/DatasetDataTypes.js

@@ -1,14 +1,15 @@
 import { memo, useCallback, useMemo, useState } from "react";
-import { useSelector, useDispatch } from "react-redux";
 import PropTypes from "prop-types";
 
 import { Button, Col, Dropdown, Row, Table, Typography } from "antd";
 import { DeleteOutlined, DownOutlined, ImportOutlined } from "@ant-design/icons";
 
 import { datasetPropTypesShape, projectPropTypesShape } from "@/propTypes";
-import { fetchDatasetDataTypesSummariesIfPossible } from "@/modules/datasets/actions";
+import { fetchDatasetDataTypesIfPossible, invalidateDatasetSummaries } from "@/modules/datasets/actions";
+import { useDatasetDataTypesByID, useDatasetSummariesByID } from "@/modules/datasets/hooks";
 import { clearDatasetDataType } from "@/modules/metadata/actions";
 import { useWorkflows } from "@/modules/services/hooks";
+import { useAppDispatch } from "@/store";
 import { useStartIngestionFlow } from "../manager/workflowCommon";
 
 import genericConfirm from "../ConfirmationModal";
@@ -17,20 +18,19 @@ import DataTypeSummaryModal from "./datatype/DataTypeSummaryModal";
 const NA_TEXT = <span style={{ color: "#999", fontStyle: "italic" }}>N/A</span>;
 
 const DatasetDataTypes = memo(({ isPrivate, project, dataset }) => {
-  const dispatch = useDispatch();
-  const datasetDataTypes = useSelector((state) => state.datasetDataTypes.itemsByID[dataset.identifier]);
-  const datasetDataTypeValues = useMemo(() => Object.values(datasetDataTypes?.itemsByID ?? {}), [datasetDataTypes]);
-  const datasetSummaries = useSelector((state) => state.datasetSummaries.itemsByID[dataset.identifier]);
+  const dispatch = useAppDispatch();
+  const datasetDataTypes = useDatasetDataTypesByID(dataset.identifier);
+  const datasetDataTypeValues = useMemo(() => Object.values(datasetDataTypes.dataTypesByID), [datasetDataTypes]);
+  const datasetSummaries = useDatasetSummariesByID(dataset.identifier);
 
-  const isFetchingDataset = datasetDataTypes?.isFetching ?? false;
+  const isFetchingDataset = datasetDataTypes.isFetching ?? false;
 
   const { workflowsByType } = useWorkflows();
   const ingestionWorkflows = workflowsByType.ingestion.items;
 
-  const [datatypeSummaryVisible, setDatatypeSummaryVisible] = useState(false);
   const [selectedDataType, setSelectedDataType] = useState(null);
 
-  const selectedSummary = datasetSummaries?.data?.[selectedDataType?.id] ?? {};
+  const selectedDataTypeSummary = datasetSummaries?.data?.[selectedDataType?.id] ?? {};
 
   const handleClearDataType = useCallback(
     (dataType) => {
@@ -41,7 +41,8 @@ const DatasetDataTypes = memo(({ isPrivate, project, dataset }) => {
           "will be deleted permanently, and will no longer be available for exploration.",
         onOk: async () => {
           await dispatch(clearDatasetDataType(dataset.identifier, dataType.id));
-          await dispatch(fetchDatasetDataTypesSummariesIfPossible(dataset.identifier));
+          await dispatch(fetchDatasetDataTypesIfPossible(dataset.identifier));
+          dispatch(invalidateDatasetSummaries(dataset.identifier));
         },
       });
     },
@@ -50,7 +51,6 @@ const DatasetDataTypes = memo(({ isPrivate, project, dataset }) => {
 
   const showDataTypeSummary = useCallback((dataType) => {
     setSelectedDataType(dataType);
-    setDatatypeSummaryVisible(true);
   }, []);
 
   const startIngestionFlow = useStartIngestionFlow();
@@ -124,15 +124,16 @@ const DatasetDataTypes = memo(({ isPrivate, project, dataset }) => {
     [isPrivate, project, dataset, handleClearDataType, ingestionWorkflows, startIngestionFlow, showDataTypeSummary],
   );
 
-  const onDataTypeSummaryModalCancel = useCallback(() => setDatatypeSummaryVisible(false), []);
+  const onDataTypeSummaryModalCancel = useCallback(() => setSelectedDataType(null), []);
 
   return (
     <>
       <DataTypeSummaryModal
         dataType={selectedDataType}
-        summary={selectedSummary}
-        open={datatypeSummaryVisible}
+        summary={selectedDataTypeSummary}
+        open={selectedDataType !== null}
         onCancel={onDataTypeSummaryModalCancel}
+        isFetching={datasetSummaries?.isFetching}
       />
 
       <Typography.Title level={4} style={{ marginTop: 0 }}>

src/components/datasets/datatype/DataTypeSummaryModal.js

@@ -1,17 +1,14 @@
-import { useSelector } from "react-redux";
 import PropTypes from "prop-types";
 
-import { Modal, Skeleton } from "antd";
+import { Alert, Modal, Skeleton } from "antd";
 
 import { summaryPropTypesShape } from "@/propTypes";
 
 import GenericSummary from "./GenericSummary";
 import PhenopacketSummary from "./PhenopacketSummary";
 import VariantSummary from "./VariantSummary";
 
-const DataTypeSummaryModal = ({ dataType, summary, onCancel, open }) => {
-  const isFetchingSummaries = useSelector((state) => state.datasetDataTypes.isFetchingAll);
-
+const DataTypeSummaryModal = ({ dataType, summary, onCancel, open, isFetching }) => {
   if (!dataType) {
     return <></>;
   }
@@ -38,7 +35,9 @@ const DataTypeSummaryModal = ({ dataType, summary, onCancel, open }) => {
       width={960}
       footer={null}
     >
-      {!summaryData || isFetchingSummaries ? <Skeleton /> : <Summary summary={summaryData} />}
+      <Alert.ErrorBoundary>
+        {!summaryData || isFetching ? <Skeleton /> : <Summary summary={summaryData} />}
+      </Alert.ErrorBoundary>
     </Modal>
   );
 };
@@ -48,6 +47,7 @@ DataTypeSummaryModal.propTypes = {
   summary: summaryPropTypesShape,
   onCancel: PropTypes.func,
   open: PropTypes.bool,
+  isFetching: PropTypes.bool,
 };
 
 export default DataTypeSummaryModal;

src/components/datasets/datatype/VariantSummary.js

@@ -1,21 +1,16 @@
 import { Col, Row, Statistic } from "antd";
-import { FileOutlined } from "@ant-design/icons";
 
+import { EM_DASH } from "@/constants";
 import { summaryPropTypesShape } from "@/propTypes";
 
 const VariantSummary = ({ summary }) => (
   <Row gutter={16}>
-    <Col span={8}>
+    <Col span={12}>
       <Statistic title="Variants" value={summary.count} />
     </Col>
-    <Col span={8}>
-      <Statistic title="Samples" value={summary.data_type_specific.samples} />
+    <Col span={12}>
+      <Statistic title="Samples" value={summary.data_type_specific?.samples ?? EM_DASH} />
     </Col>
-    {summary.data_type_specific?.vcf_files !== undefined ? (
-      <Col span={8}>
-        <Statistic title="VCF Files" prefix={<FileOutlined />} value={summary.data_type_specific.vcf_files} />
-      </Col>
-    ) : null}
   </Row>
 );
 

src/modules/datasets/actions.js

@@ -2,20 +2,21 @@ import { beginFlow, createFlowActionTypes, createNetworkActionTypes, endFlow, ne
 import { getDataServices } from "../services/utils";
 
 export const FETCHING_DATASETS_DATA_TYPES = createFlowActionTypes("FETCHING_DATASETS_DATA_TYPES");
-export const FETCH_DATASET_DATA_TYPES_SUMMARY = createNetworkActionTypes("FETCH_DATASET_DATA_TYPES_SUMMARY");
+export const FETCH_DATASET_DATA_TYPES = createNetworkActionTypes("FETCH_DATASET_DATA_TYPES");
 
-export const FETCH_DATASET_SUMMARY = createNetworkActionTypes("FETCH_DATASET_SUMMARY");
-export const FETCHING_ALL_DATASET_SUMMARIES = createFlowActionTypes("FETCHING_ALL_DATASET_SUMMARIES");
+export const FETCH_SERVICE_DATASET_SUMMARY = createNetworkActionTypes("FETCH_SERVICE_DATASET_SUMMARY");
+export const FETCHING_DATASET_SUMMARIES = createFlowActionTypes("FETCHING_DATASET_SUMMARIES");
+export const INVALIDATE_DATASET_SUMMARIES = "INVALIDATE_DATASET_SUMMARIES";
 
 export const FETCH_DATASET_RESOURCES = createNetworkActionTypes("FETCH_DATASET_RESOURCES");
 
 const fetchDatasetDataTypesSummary = networkAction((serviceInfo, datasetID) => ({
-  types: FETCH_DATASET_DATA_TYPES_SUMMARY,
+  types: FETCH_DATASET_DATA_TYPES,
   params: { serviceInfo, datasetID },
   url: `${serviceInfo.url}/datasets/${datasetID}/data-types`,
 }));
 
-export const fetchDatasetDataTypesSummariesIfPossible = (datasetID) => async (dispatch, getState) => {
+export const fetchDatasetDataTypesIfPossible = (datasetID) => async (dispatch, getState) => {
   if (getState().datasetDataTypes.itemsByID?.[datasetID]?.isFetching) return;
   await Promise.all(
     getDataServices(getState()).map((serviceInfo) => dispatch(fetchDatasetDataTypesSummary(serviceInfo, datasetID))),
@@ -26,27 +27,30 @@ export const fetchDatasetsDataTypes = () => async (dispatch, getState) => {
   dispatch(beginFlow(FETCHING_DATASETS_DATA_TYPES));
   await Promise.all(
     Object.keys(getState().projects.datasetsByID).map((datasetID) =>
-      dispatch(fetchDatasetDataTypesSummariesIfPossible(datasetID)),
+      dispatch(fetchDatasetDataTypesIfPossible(datasetID)),
     ),
   );
   dispatch(endFlow(FETCHING_DATASETS_DATA_TYPES));
 };
 
-const fetchDatasetSummary = networkAction((serviceInfo, datasetID) => ({
-  types: FETCH_DATASET_SUMMARY,
+const fetchServiceDatasetSummary = networkAction((serviceInfo, datasetID) => ({
+  types: FETCH_SERVICE_DATASET_SUMMARY,
   params: { serviceInfo, datasetID },
   url: `${serviceInfo.url}/datasets/${datasetID}/summary`,
 }));
 
-export const fetchDatasetSummariesIfPossible = (datasetID) => async (dispatch, getState) => {
-  if (getState().datasetSummaries.isFetchingAll) return;
-  dispatch(beginFlow(FETCHING_ALL_DATASET_SUMMARIES));
+export const fetchDatasetSummariesIfNeeded = (datasetID) => async (dispatch, getState) => {
+  const existingSummaryState = getState().datasetSummaries.itemsByID[datasetID] ?? {};
+  if (existingSummaryState.isFetching || (!existingSummaryState.isInvalid && existingSummaryState.hasAttempted)) return;
+  dispatch(beginFlow(FETCHING_DATASET_SUMMARIES, { datasetID }));
   await Promise.all(
-    getDataServices(getState()).map((serviceInfo) => dispatch(fetchDatasetSummary(serviceInfo, datasetID))),
+    getDataServices(getState()).map((serviceInfo) => dispatch(fetchServiceDatasetSummary(serviceInfo, datasetID))),
   );
-  dispatch(endFlow(FETCHING_ALL_DATASET_SUMMARIES));
+  dispatch(endFlow(FETCHING_DATASET_SUMMARIES, { datasetID }));
 };
 
+export const invalidateDatasetSummaries = (datasetID) => ({ type: INVALIDATE_DATASET_SUMMARIES, datasetID });
+
 const fetchDatasetResources = networkAction((datasetID) => (_dispatch, getState) => ({
   types: FETCH_DATASET_RESOURCES,
   params: { datasetID },

src/modules/datasets/hooks.js

@@ -1,7 +1,11 @@
 import { useEffect, useMemo } from "react";
 import { useProjects } from "@/modules/metadata/hooks";
 import { useAppDispatch, useAppSelector } from "@/store";
-import { fetchDatasetDataTypesSummariesIfPossible, fetchDatasetsDataTypes } from "@/modules/datasets/actions";
+import {
+  fetchDatasetDataTypesIfPossible,
+  fetchDatasetsDataTypes,
+  fetchDatasetSummariesIfNeeded,
+} from "@/modules/datasets/actions";
 
 export const useDatasetDataTypes = () => {
   /**
@@ -23,17 +27,30 @@ export const useDatasetDataTypesByID = (datasetId) => {
    * Fetches the data types ONLY for the given dataset.
    * Returns the store's value for the given dataset ID.
    */
+
   const dispatch = useAppDispatch();
   useEffect(() => {
-    dispatch(fetchDatasetDataTypesSummariesIfPossible(datasetId));
+    dispatch(fetchDatasetDataTypesIfPossible(datasetId)).catch(console.error);
   }, [dispatch, datasetId]);
 
   const dataTypes = useAppSelector((state) => state.datasetDataTypes.itemsByID[datasetId]);
   return useMemo(() => {
     return {
-      dataTypesByID: dataTypes?.itemsByID,
+      dataTypesByID: dataTypes?.itemsByID ?? {},
       isFetchingDataTypes: dataTypes?.isFetching ?? true,
       hasAttemptedDataTypes: dataTypes?.hasAttempted ?? false,
     };
   }, [dataTypes]);
 };
+
+export const useDatasetSummariesByID = (datasetId) => {
+  /**
+   * Fetches the data type summaries ONLY for the given dataset.
+   * Returns the store's value for the given dataset ID.
+   */
+  const dispatch = useAppDispatch();
+  useEffect(() => {
+    dispatch(fetchDatasetSummariesIfNeeded(datasetId)).catch(console.error);
+  }, [dispatch, datasetId]);
+  return useAppSelector((state) => state.datasetSummaries.itemsByID[datasetId]);
+};

src/modules/datasets/reducers.js

@@ -1,9 +1,10 @@
 import {
-  FETCHING_DATASETS_DATA_TYPES,
-  FETCH_DATASET_DATA_TYPES_SUMMARY,
-  FETCH_DATASET_SUMMARY,
-  FETCHING_ALL_DATASET_SUMMARIES,
+  FETCH_DATASET_DATA_TYPES,
   FETCH_DATASET_RESOURCES,
+  FETCH_SERVICE_DATASET_SUMMARY,
+  FETCHING_DATASET_SUMMARIES,
+  FETCHING_DATASETS_DATA_TYPES,
+  INVALIDATE_DATASET_SUMMARIES,
 } from "./actions";
 
 export const datasetDataTypes = (
@@ -18,7 +19,7 @@ export const datasetDataTypes = (
       return { ...state, isFetchingAll: true };
     case FETCHING_DATASETS_DATA_TYPES.END:
       return { ...state, isFetchingAll: false };
-    case FETCH_DATASET_DATA_TYPES_SUMMARY.REQUEST: {
+    case FETCH_DATASET_DATA_TYPES.REQUEST: {
       const { datasetID } = action;
       return {
         ...state,
@@ -32,7 +33,7 @@ export const datasetDataTypes = (
         },
       };
     }
-    case FETCH_DATASET_DATA_TYPES_SUMMARY.RECEIVE: {
+    case FETCH_DATASET_DATA_TYPES.RECEIVE: {
       const { datasetID } = action;
       const itemsByID = Object.fromEntries(action.data.map((d) => [d.id, d]));
       return {
@@ -48,8 +49,8 @@ export const datasetDataTypes = (
         },
       };
     }
-    case FETCH_DATASET_DATA_TYPES_SUMMARY.FINISH:
-    case FETCH_DATASET_DATA_TYPES_SUMMARY.ERROR: {
+    case FETCH_DATASET_DATA_TYPES.FINISH:
+    case FETCH_DATASET_DATA_TYPES.ERROR: {
       const { datasetID } = action;
       return {
         ...state,
@@ -78,7 +79,7 @@ const datasetItemSet = (oldState, datasetID, key, value) => {
           ...value,
         }
       : value;
-  const newState = {
+  return {
     ...oldState,
     itemsByID: {
       ...oldState.itemsByID,
@@ -88,28 +89,36 @@ const datasetItemSet = (oldState, datasetID, key, value) => {
       },
     },
   };
-  return newState;
 };
 
 export const datasetSummaries = (
   state = {
-    isFetchingAll: false,
     itemsByID: {},
   },
   action,
 ) => {
+  // This reducer is a bit funky, since it is combining data from multiple services.
   switch (action.type) {
-    case FETCH_DATASET_SUMMARY.REQUEST:
-      return datasetItemSet(state, action.datasetID, "isFetching", true);
-    case FETCH_DATASET_SUMMARY.RECEIVE:
+    case FETCH_SERVICE_DATASET_SUMMARY.RECEIVE:
       return datasetItemSet(state, action.datasetID, "data", action.data);
-    case FETCH_DATASET_SUMMARY.FINISH:
-      return datasetItemSet(state, action.datasetID, "isFetching", false);
-    case FETCHING_ALL_DATASET_SUMMARIES.BEGIN:
-      return { ...state, isFetchingAll: true };
-    case FETCHING_ALL_DATASET_SUMMARIES.END:
-    case FETCHING_ALL_DATASET_SUMMARIES.TERMINATE:
-      return { ...state, isFetchingAll: false };
+    case FETCHING_DATASET_SUMMARIES.BEGIN:
+      return datasetItemSet(state, action.datasetID, "isFetching", true);
+    case FETCHING_DATASET_SUMMARIES.END:
+    case FETCHING_DATASET_SUMMARIES.TERMINATE:
+      return {
+        ...state,
+        itemsByID: {
+          ...state.itemsByID,
+          [action.datasetID]: {
+            ...(state.itemsByID[action.datasetID] ?? {}),
+            isFetching: false,
+            isInvalid: false,
+            hasAttempted: true,
+          },
+        },
+      };
+    case INVALIDATE_DATASET_SUMMARIES:
+      return datasetItemSet(state, action.datasetID, "isInvalid", true);
     default:
       return state;
   }

src/utils/actions.js

@@ -157,6 +157,6 @@ const formatErrorMessage = (errorMessageIntro, errorDetail) => {
   return errorMessageIntro ? errorMessageIntro + (errorDetail ? `: ${errorDetail}` : "") : errorDetail;
 };
 
-export const beginFlow = (types) => (dispatch) => dispatch({ type: types.BEGIN });
-export const endFlow = (types) => (dispatch) => dispatch({ type: types.END });
-export const terminateFlow = (types) => (dispatch) => dispatch({ type: types.TERMINATE });
+export const beginFlow = (types, params) => (dispatch) => dispatch({ type: types.BEGIN, ...(params ?? {}) });
+export const endFlow = (types, params) => (dispatch) => dispatch({ type: types.END, ...(params ?? {}) });
+export const terminateFlow = (types, params) => (dispatch) => dispatch({ type: types.TERMINATE, ...(params ?? {}) });