From c1814e04ddd6b9b9f9d7c64cf234930d9f1439d6 Mon Sep 17 00:00:00 2001 From: David Lougheed Date: Mon, 26 Aug 2024 16:53:26 -0400 Subject: [PATCH 1/2] fix(explorer): fix variant querying not working --- .../discovery/DiscoveryQueryBuilder.js | 40 ++++++------------- .../discovery/DiscoverySearchForm.js | 3 +- 2 files changed, 14 insertions(+), 29 deletions(-) diff --git a/src/components/discovery/DiscoveryQueryBuilder.js b/src/components/discovery/DiscoveryQueryBuilder.js index 859d1b29f..fa8c7641e 100644 --- a/src/components/discovery/DiscoveryQueryBuilder.js +++ b/src/components/discovery/DiscoveryQueryBuilder.js @@ -106,28 +106,17 @@ const DiscoveryQueryBuilder = ({ activeDataset, dataTypeForms, requiredDataTypes } }, [dispatch, forms, onSubmit]); - const handleFormChange = useCallback( - (dataType, fields) => { - dispatch(updateDataTypeQueryForm(activeDataset, dataType, fields)); - }, - [dispatch, activeDataset], - ); - - const handleVariantHiddenFieldChange = useCallback( - (fields) => { - dispatch(updateDataTypeQueryForm(activeDataset, dataTypesByID["variant"], fields)); - }, - [dispatch, activeDataset, dataTypesByID], - ); + const handleFormChange = (dataType) => (fields) => dispatch(updateDataTypeQueryForm(activeDataset, dataType, fields)); + const handleVariantHiddenFieldChange = useMemo(() => handleFormChange(dataTypesByID["variant"]), [dataTypesByID]); const handleHelpAndSchemasToggle = useCallback(() => { setSchemasModalShown((s) => !s); }, []); - const handleSetFormRef = useCallback((dataType, form) => { + const handleSetFormRef = (dataType) => (form) => { // Without a functional state update, this triggers an infinite loop in rendering variant search - setForms((forms) => ({ ...forms, [dataType.id]: form })); - }, []); + setForms((fs) => ({ ...fs, [dataType.id]: form })); + }; useEffect(() => { if (autoQuery?.isAutoQuery && shouldExecAutoQueryPt2) { @@ -164,7 +153,7 @@ const DiscoveryQueryBuilder = ({ activeDataset, dataTypeForms, requiredDataTypes ]; form.setFields(fields); - handleFormChange(dataType, fields); // Not triggered by setFields; do it manually + handleFormChange(dataType)(fields); // Not triggered by setFields; do it manually (async () => { // Simulate form submission click @@ -178,7 +167,7 @@ const DiscoveryQueryBuilder = ({ activeDataset, dataTypeForms, requiredDataTypes await s; })(); } - }, [dispatch, autoQuery, dataTypesByID, forms, shouldExecAutoQueryPt2, handleFormChange, handleSubmit]); + }, [dispatch, autoQuery, dataTypesByID, forms, shouldExecAutoQueryPt2, handleSubmit]); // --- render --- @@ -214,6 +203,8 @@ const DiscoveryQueryBuilder = ({ activeDataset, dataTypeForms, requiredDataTypes // Use data type label for tab name, unless it isn't specified - then fall back to ID. // This behaviour should be the same everywhere in bento_web or almost anywhere the // data type is shown to 'end users'. + const handleChange = handleFormChange(dataType); + const setFormRef = handleSetFormRef(dataType); const { id, label } = dataType; return { key: id, @@ -223,21 +214,14 @@ const DiscoveryQueryBuilder = ({ activeDataset, dataTypeForms, requiredDataTypes handleSetFormRef(dataType, form)} - onChange={(fields) => handleFormChange(dataType, fields)} + setFormRef={setFormRef} + onChange={handleChange} handleVariantHiddenFieldChange={handleVariantHiddenFieldChange} /> ), }; }), - [ - requiredDataTypes, - dataTypeForms, - searchLoading, - handleSetFormRef, - handleFormChange, - handleVariantHiddenFieldChange, - ], + [requiredDataTypes, dataTypeForms, searchLoading, handleVariantHiddenFieldChange], ); const addConditionsOnDataType = (buttonProps = { style: { float: "right" } }) => ( diff --git a/src/components/discovery/DiscoverySearchForm.js b/src/components/discovery/DiscoverySearchForm.js index 0989cf3f6..1d3dac882 100644 --- a/src/components/discovery/DiscoverySearchForm.js +++ b/src/components/discovery/DiscoverySearchForm.js @@ -250,6 +250,7 @@ const DiscoverySearchForm = ({ onChange, dataType, setFormRef, handleVariantHidd updatedConditionsArray = updateVariantConditions(updatedConditionsArray, "[dataset item].alternative", alt); } + form.setFieldsValue({ conditions: updatedConditionsArray }); // update form separately - not controlled handleVariantHiddenFieldChange([ { name: ["conditions"], @@ -257,7 +258,7 @@ const DiscoverySearchForm = ({ onChange, dataType, setFormRef, handleVariantHidd }, ]); }, - [getConditionsArray, handleVariantHiddenFieldChange], + [form, getConditionsArray, handleVariantHiddenFieldChange], ); const getHelpText = useCallback( From 0466f03e5d0c118cbd6a23f89a3d4e804dd4016c Mon Sep 17 00:00:00 2001 From: David Lougheed Date: Mon, 26 Aug 2024 17:12:26 -0400 Subject: [PATCH 2/2] lint --- src/components/discovery/DiscoveryQueryBuilder.js | 14 ++++++++++---- src/components/discovery/DiscoverySearchForm.js | 2 +- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/components/discovery/DiscoveryQueryBuilder.js b/src/components/discovery/DiscoveryQueryBuilder.js index fa8c7641e..60037535c 100644 --- a/src/components/discovery/DiscoveryQueryBuilder.js +++ b/src/components/discovery/DiscoveryQueryBuilder.js @@ -106,8 +106,14 @@ const DiscoveryQueryBuilder = ({ activeDataset, dataTypeForms, requiredDataTypes } }, [dispatch, forms, onSubmit]); - const handleFormChange = (dataType) => (fields) => dispatch(updateDataTypeQueryForm(activeDataset, dataType, fields)); - const handleVariantHiddenFieldChange = useMemo(() => handleFormChange(dataTypesByID["variant"]), [dataTypesByID]); + const handleFormChange = useCallback( + (dataType) => (fields) => dispatch(updateDataTypeQueryForm(activeDataset, dataType, fields)), + [dispatch, activeDataset], + ); + const handleVariantHiddenFieldChange = useMemo( + () => handleFormChange(dataTypesByID["variant"]), + [handleFormChange, dataTypesByID], + ); const handleHelpAndSchemasToggle = useCallback(() => { setSchemasModalShown((s) => !s); @@ -167,7 +173,7 @@ const DiscoveryQueryBuilder = ({ activeDataset, dataTypeForms, requiredDataTypes await s; })(); } - }, [dispatch, autoQuery, dataTypesByID, forms, shouldExecAutoQueryPt2, handleSubmit]); + }, [dispatch, autoQuery, dataTypesByID, forms, shouldExecAutoQueryPt2, handleFormChange, handleSubmit]); // --- render --- @@ -221,7 +227,7 @@ const DiscoveryQueryBuilder = ({ activeDataset, dataTypeForms, requiredDataTypes ), }; }), - [requiredDataTypes, dataTypeForms, searchLoading, handleVariantHiddenFieldChange], + [requiredDataTypes, dataTypeForms, searchLoading, handleFormChange, handleVariantHiddenFieldChange], ); const addConditionsOnDataType = (buttonProps = { style: { float: "right" } }) => ( diff --git a/src/components/discovery/DiscoverySearchForm.js b/src/components/discovery/DiscoverySearchForm.js index 1d3dac882..61947b7a9 100644 --- a/src/components/discovery/DiscoverySearchForm.js +++ b/src/components/discovery/DiscoverySearchForm.js @@ -250,7 +250,7 @@ const DiscoverySearchForm = ({ onChange, dataType, setFormRef, handleVariantHidd updatedConditionsArray = updateVariantConditions(updatedConditionsArray, "[dataset item].alternative", alt); } - form.setFieldsValue({ conditions: updatedConditionsArray }); // update form separately - not controlled + form.setFieldsValue({ conditions: updatedConditionsArray }); // update form separately - not controlled handleVariantHiddenFieldChange([ { name: ["conditions"],