Merge pull request #287 from bento-platform/fix/dataset-delete

feat!: delete variants on project/dataset deletion; load data types/workflows from service-registry; rm hardcoding artifacts
bento-platform · Sep 1, 2023 · d670e39 · d670e39
2 parents 9cc0865 + f7c61a2
commit d670e39
Show file tree

Hide file tree

Showing 13 changed files with 134 additions and 257 deletions.
diff --git a/src/components/datasets/Dataset.js b/src/components/datasets/Dataset.js
@@ -13,8 +13,8 @@ import {
 } from "../../modules/metadata/actions";
 
 import {
-    fetchDatasetDataTypesSummaryIfPossible,
-    fetchDatasetSummaryIfPossible,
+    fetchDatasetDataTypesSummariesIfPossible,
+    fetchDatasetSummariesIfPossible,
 } from "../../modules/datasets/actions";
 
 import {INITIAL_DATA_USE_VALUE} from "../../duo";
@@ -315,8 +315,8 @@ const mapDispatchToProps = (dispatch, ownProps) => ({
     deleteProjectDataset: dataset => dispatch(deleteProjectDatasetIfPossible(ownProps.project, dataset)),
     deleteLinkedFieldSet: (dataset, linkedFieldSet, linkedFieldSetIndex) =>
         dispatch(deleteDatasetLinkedFieldSetIfPossible(dataset, linkedFieldSet, linkedFieldSetIndex)),
-    fetchDatasetSummary: (datasetId) => dispatch(fetchDatasetSummaryIfPossible(datasetId)),
-    fetchDatasetDataTypesSummary: (datasetId) => dispatch(fetchDatasetDataTypesSummaryIfPossible(datasetId)),
+    fetchDatasetSummary: (datasetId) => dispatch(fetchDatasetSummariesIfPossible(datasetId)),
+    fetchDatasetDataTypesSummary: (datasetId) => dispatch(fetchDatasetDataTypesSummariesIfPossible(datasetId)),
 });
 
 export default connect(mapStateToProps, mapDispatchToProps)(Dataset);
diff --git a/src/components/datasets/DatasetDataTypes.js b/src/components/datasets/DatasetDataTypes.js
@@ -5,7 +5,7 @@ import { Button, Col, Row, Table, Typography } from "antd";
 import PropTypes from "prop-types";
 import { datasetPropTypesShape, projectPropTypesShape } from "../../propTypes";
 import { clearDatasetDataType } from "../../modules/metadata/actions";
-import { fetchDatasetDataTypesSummaryIfPossible } from "../../modules/datasets/actions";
+import { fetchDatasetDataTypesSummariesIfPossible } from "../../modules/datasets/actions";
 import genericConfirm from "../ConfirmationModal";
 import DataTypeSummaryModal from "./datatype/DataTypeSummaryModal";
 import { nop } from "../../utils/misc";
@@ -16,7 +16,7 @@ const DatasetDataTypes = React.memo(
     ({isPrivate, project, dataset, onDatasetIngest}) => {
         const dispatch = useDispatch();
         const datasetDataTypes = useSelector((state) => Object.values(
-            state.datasetDataTypes.itemsById[dataset.identifier]?.itemsById));
+            state.datasetDataTypes.itemsById[dataset.identifier]?.itemsById ?? {}));
         const datasetSummaries = useSelector((state) => state.datasetSummaries.itemsById[dataset.identifier]);
         const isFetchingDataset = useSelector(
             (state) => state.datasetDataTypes.itemsById[dataset.identifier]?.isFetching);
@@ -35,7 +35,7 @@ const DatasetDataTypes = React.memo(
                 "will be deleted permanently, and will no longer be available for exploration.",
                 onOk: async () => {
                     await dispatch(clearDatasetDataType(dataset.identifier, dataType.id));
-                    await dispatch(fetchDatasetDataTypesSummaryIfPossible(dataset.identifier));
+                    await dispatch(fetchDatasetDataTypesSummariesIfPossible(dataset.identifier));
                 },
             });
         }, [dispatch, dataset]);

diff --git a/src/components/discovery/DiscoveryQueryBuilder.js b/src/components/discovery/DiscoveryQueryBuilder.js
@@ -254,14 +254,15 @@ DiscoveryQueryBuilder.propTypes = {
 
 const mapStateToProps = state => ({
     servicesInfo: state.services.items,
-    dataTypes: state.serviceDataTypes.dataTypesByServiceID,
     dataTypesByID: state.serviceDataTypes.itemsByID,
     dataTypesByDataset: state.datasetDataTypes,
 
     autoQuery: state.explorer.autoQuery,
     isFetchingTextSearch: state.explorer.fetchingTextSearch || false,
 
-    dataTypesLoading: state.services.isFetching || state.datasetDataTypes.isFetchingAll,
+    dataTypesLoading: state.services.isFetching
+        || state.serviceDataTypes.isFetching
+        || state.datasetDataTypes.isFetchingAll,
 });
 
 const mapDispatchToProps = (dispatch) => ({

diff --git a/src/components/manager/DataTypeSelect.js b/src/components/manager/DataTypeSelect.js
@@ -6,17 +6,15 @@ import { useSelector } from "react-redux";
 const DataTypeSelect = ({value, workflows, onChange}) => {
     const [selected, setSelected] = useState(value ?? undefined);
     const servicesFetching = useSelector((state) => state.services.isFetchingAll);
-    const workflowsFetching = useSelector((state) => state.serviceWorkflows.isFetchingAll);
+    const workflowsFetching = useSelector((state) => state.serviceWorkflows.isFetching);
     const {
-        itemsByID: dataTypes,
-        isFetchingAll: isFetchingDataTypes,
+        items: dataTypes,
+        isFetching: isFetchingDataTypes,
     } = useSelector((state) => state.serviceDataTypes);
 
     const labels = useMemo(() => {
         if (!dataTypes) return {};
-        return Object.fromEntries(
-            Object.values(dataTypes).map(dt => [dt.id, dt.label]),
-        );
+        return Object.fromEntries(dataTypes.map(dt => [dt.id, dt.label]));
     }, dataTypes);
 
     useEffect(() => {
@@ -34,16 +32,16 @@ const DataTypeSelect = ({value, workflows, onChange}) => {
         if (!Array.isArray(workflows)) {
             return [];
         }
-        const dataTypes = new Set(workflows.map((w) => w.data_type));
-        return Array.from(dataTypes)
+        const workflowDataTypes = new Set(workflows.map((w) => w.data_type));
+        return Array.from(workflowDataTypes)
             // filter out workflow types for which we have no labels (mcode)
             .filter(dt => dt in labels)
             .map((dt) =>
                 <Select.Option value={dt} key={dt}>
                     {labels[dt]} ({<span style={{fontFamily: "monospace"}}>{dt}</span>})
                 </Select.Option>,
             );
-    }, [workflows, dataTypes, labels]);
+    }, [workflows, labels]);
 
     return (
         <Spin spinning={servicesFetching || workflowsFetching || isFetchingDataTypes}>

diff --git a/src/components/manager/projects/RoutedProject.js b/src/components/manager/projects/RoutedProject.js
@@ -150,12 +150,6 @@ RoutedProject.propTypes = {
     services: PropTypes.arrayOf(serviceInfoPropTypesShape),
     servicesByID: PropTypes.objectOf(serviceInfoPropTypesShape),
 
-    serviceDataTypesByServiceID: PropTypes.objectOf(PropTypes.shape({
-        items: PropTypes.array,  // TODO: Shape
-        itemsByID: PropTypes.object,  // TODO: Shape
-        isFetching: PropTypes.bool,
-    })),
-
     projects: PropTypes.arrayOf(projectPropTypesShape),
     projectsByID: PropTypes.objectOf(projectPropTypesShape),
 
@@ -178,8 +172,6 @@ const mapStateToProps = state => ({
     services: state.services.items,
     servicesByID: state.services.itemsByID,
 
-    serviceDataTypesByServiceID: state.serviceDataTypes.dataTypesByServiceID,
-
     projects: state.projects.items,
     projectsByID: state.projects.itemsByID,
 

diff --git a/src/modules/datasets/actions.js b/src/modules/datasets/actions.js
@@ -1,28 +1,30 @@
 import {beginFlow, createFlowActionTypes, createNetworkActionTypes, endFlow, networkAction} from "../../utils/actions";
+import {getDataServices} from "../services/utils";
 
-export const FETCHING_DATASETS_DATATYPE = createFlowActionTypes("FETCHING_DATASETS_DATATYPE");
-export const FETCH_DATASET_DATATYPE = createNetworkActionTypes("FETCH_DATASET_DATATYPE");
+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_SUMMARY = createNetworkActionTypes("FETCH_DATASET_SUMMARY");
 
-const fetchDatasetDataTypeSummary = networkAction((serviceInfo, datasetID) => ({
-    types: FETCH_DATASET_DATATYPE,
+const fetchDatasetDataTypesSummary = networkAction((serviceInfo, datasetID) => ({
+    types: FETCH_DATASET_DATA_TYPES_SUMMARY,
     params: {serviceInfo, datasetID},
     url: `${serviceInfo.url}/datasets/${datasetID}/data-types`,
 }));
 
-export const fetchDatasetDataTypesSummaryIfPossible = (datasetID) => async (dispatch, getState) => {
-    if (getState().datasetDataTypes.isFetching) return;
-    await dispatch(fetchDatasetDataTypeSummary(getState().services.itemsByArtifact.metadata, datasetID));
-    await dispatch(fetchDatasetDataTypeSummary(getState().services.itemsByArtifact.gohan, datasetID));
+export const fetchDatasetDataTypesSummariesIfPossible = (datasetID) => async (dispatch, getState) => {
+    if (getState().datasetDataTypes.isFetchingAll) return;
+    await Promise.all(
+        getDataServices(getState()).map(serviceInfo => dispatch(fetchDatasetDataTypesSummary(serviceInfo, datasetID))),
+    );
 };
 
 export const fetchDatasetsDataTypes = () => async (dispatch, getState) => {
-    dispatch(beginFlow(FETCHING_DATASETS_DATATYPE));
+    dispatch(beginFlow(FETCHING_DATASETS_DATA_TYPES));
     await Promise.all(
         Object.keys(getState().projects.datasetsByID).map(datasetID =>
-            dispatch(fetchDatasetDataTypesSummaryIfPossible(datasetID))),
+            dispatch(fetchDatasetDataTypesSummariesIfPossible(datasetID))),
     );
-    dispatch(endFlow(FETCHING_DATASETS_DATATYPE));
+    dispatch(endFlow(FETCHING_DATASETS_DATA_TYPES));
 };
 
 const fetchDatasetSummary = networkAction((serviceInfo, datasetID) => ({
@@ -31,8 +33,9 @@ const fetchDatasetSummary = networkAction((serviceInfo, datasetID) => ({
     url: `${serviceInfo.url}/datasets/${datasetID}/summary`,
 }));
 
-export const fetchDatasetSummaryIfPossible = (datasetID) => async (dispatch, getState) => {
+export const fetchDatasetSummariesIfPossible = (datasetID) => async (dispatch, getState) => {
     if (getState().datasetSummaries.isFetching) return;
-    await dispatch(fetchDatasetSummary(getState().services.itemsByArtifact.metadata, datasetID));
-    await dispatch(fetchDatasetSummary(getState().services.itemsByArtifact.gohan, datasetID));
+    await Promise.all(
+        getDataServices(getState()).map(serviceInfo => fetchDatasetSummary(serviceInfo, datasetID)),
+    );
 };
diff --git a/src/modules/datasets/reducers.js b/src/modules/datasets/reducers.js
@@ -1,4 +1,4 @@
-import {FETCHING_DATASETS_DATATYPE, FETCH_DATASET_DATATYPE, FETCH_DATASET_SUMMARY} from "./actions";
+import {FETCHING_DATASETS_DATA_TYPES, FETCH_DATASET_DATA_TYPES_SUMMARY, FETCH_DATASET_SUMMARY} from "./actions";
 
 export const datasetDataTypes = (
     state = {
@@ -8,24 +8,24 @@ export const datasetDataTypes = (
     action,
 ) => {
     switch (action.type) {
-        case FETCHING_DATASETS_DATATYPE.BEGIN:
+        case FETCHING_DATASETS_DATA_TYPES.BEGIN:
             return {...state, isFetchingAll: true};
-        case FETCHING_DATASETS_DATATYPE.END:
+        case FETCHING_DATASETS_DATA_TYPES.END:
             return {...state, isFetchingAll: false};
-        case FETCH_DATASET_DATATYPE.REQUEST:{
+        case FETCH_DATASET_DATA_TYPES_SUMMARY.REQUEST:{
             const {datasetID} = action;
             return {
                 ...state,
                 itemsById: {
                     ...state.itemsById,
                     [datasetID]: {
-                        itemsById: {...(state.itemsById[datasetID]?.itemsById ?? {})},
+                        itemsById: state.itemsById[datasetID]?.itemsById ?? {},
                         isFetching: true,
                     },
                 },
             };
         }
-        case FETCH_DATASET_DATATYPE.RECEIVE:{
+        case FETCH_DATASET_DATA_TYPES_SUMMARY.RECEIVE:{
             const {datasetID} = action;
             const itemsByID = Object.fromEntries(action.data.map(d => [d.id, d]));
             return {
@@ -41,8 +41,8 @@ export const datasetDataTypes = (
                 },
             };
         }
-        case FETCH_DATASET_DATATYPE.FINISH:
-        case FETCH_DATASET_DATATYPE.ERROR:{
+        case FETCH_DATASET_DATA_TYPES_SUMMARY.FINISH:
+        case FETCH_DATASET_DATA_TYPES_SUMMARY.ERROR:{
             const {datasetID} = action;
             return {
                 ...state,

diff --git a/src/modules/metadata/actions.js b/src/modules/metadata/actions.js
@@ -28,18 +28,18 @@ export const FETCH_OVERVIEW_SUMMARY = createNetworkActionTypes("FETCH_OVERVIEW_S
 
 export const DELETE_DATASET_DATA_TYPE = createNetworkActionTypes("DELETE_DATASET_DATA_TYPE");
 
-export const clearDatasetDataType = networkAction((datasetId, dataType) => (dispatch, getState) => {
-    // TODO: more robust mapping from dataType to url.
-    const serviceUrl = dataType === "variant"
-        ? getState().services.itemsByKind.gohan.url
-        : getState().services.itemsByKind.metadata.url;
-    console.log(serviceUrl);
+export const clearDatasetDataType = networkAction((datasetId, dataTypeID) => (dispatch, getState) => {
+    const {service_base_url: serviceBaseUrl} = getState().serviceDataTypes.itemsByID[dataTypeID];
     return {
         types: DELETE_DATASET_DATA_TYPE,
-        url: `${serviceUrl}/datasets/${datasetId}/data-types/${dataType}`,
+        url: `${serviceBaseUrl}datasets/${datasetId}/data-types/${dataTypeID}`,
         req: {
             method: "DELETE",
         },
+        onError: (error) => {
+            // Needs to re throw for project/dataset deletion error handling
+            throw error;
+        },
     };
 });
 
@@ -125,11 +125,25 @@ export const deleteProject = networkAction(project => (dispatch, getState) => ({
     onSuccess: () => message.success(`Project '${project.title}' deleted!`),
 }));
 
-export const deleteProjectIfPossible = project => (dispatch, getState) => {
+export const deleteProjectIfPossible = project => async (dispatch, getState) => {
     if (getState().projects.isDeleting) return;
-    return dispatch(deleteProject(project));
+
+    // Remove data without destroying project/datasets first
+    try {
+        await Promise.all(project.datasets.map(ds => dispatch(clearDatasetDataTypes(ds.identifier))));
+        await dispatch(deleteProject(project));
+    } catch (err) {
+        console.error(err);
+        message.error(`Error deleting project '${project.title}'`);
+    }
 };
 
+export const clearDatasetDataTypes = datasetId => async (dispatch, getState) => {
+    // only clear data types which can yield counts - `queryable` is a proxy for this
+    const dataTypes = Object.values(getState().datasetDataTypes.itemsById[datasetId].itemsById)
+        .filter(dt => dt.queryable);
+    return await Promise.all(dataTypes.map(dt => dispatch(clearDatasetDataType(datasetId, dt.id))));
+};
 
 const saveProject = networkAction(project => (dispatch, getState) => ({
     types: SAVE_PROJECT,
@@ -181,11 +195,17 @@ export const deleteProjectDataset = networkAction((project, dataset) => (dispatc
     err: `Error deleting dataset '${dataset.title}'`,
 }));
 
-export const deleteProjectDatasetIfPossible = (project, dataset) => (dispatch, getState) => {
+export const deleteProjectDatasetIfPossible = (project, dataset) => async (dispatch, getState) => {
     if (getState().projects.isAddingDataset
         || getState().projects.isSavingDataset
         || getState().projects.isDeletingDataset) return;
-    return dispatch(deleteProjectDataset(project, dataset));
+    try {
+        await dispatch(clearDatasetDataTypes(dataset.identifier));
+        await dispatch(deleteProjectDataset(project, dataset));
+    } catch (err) {
+        console.error(err);
+        message.error(`Error deleting dataset '${dataset.title}'`);
+    }
 };
 
 

diff --git a/src/modules/metadata/reducers.js b/src/modules/metadata/reducers.js
@@ -146,6 +146,9 @@ export const projects = (
             };
         }
 
+        case ADD_PROJECT_DATASET.FINISH:
+            return {...state, isAddingDataset: false};
+
 
         // DELETE_PROJECT_DATASET
         case DELETE_PROJECT_DATASET.REQUEST: