Genomics downloads handle invalid param selection (#462)

* Only store param value if submission is successful * Update enum param validation to take constraints into account * Handle minSelectedCount === 1 * Handle invalid parameter selection
VEuPathDB · Aug 30, 2023 · 13c28c0 · 13c28c0
1 parent 57802c3
commit 13c28c0
Show file tree

Hide file tree

Showing 6 changed files with 90 additions and 76 deletions.
diff --git a/packages/libs/wdk-client/src/StoreModules/QuestionStoreModule.ts b/packages/libs/wdk-client/src/StoreModules/QuestionStoreModule.ts
@@ -535,34 +535,6 @@ const observeLoadQuestionSuccess: QuestionEpic = (action$) =>
     )
   );
 
-const observeStoreUpdatedParams: QuestionEpic = (
-  action$,
-  state$,
-  { paramValueStore }
-) =>
-  action$.pipe(
-    ofType<Action, UpdateParamValueAction>(UPDATE_PARAM_VALUE),
-    mergeMap(async (action: UpdateParamValueAction) => {
-      const searchName = action.payload.searchName;
-      const questionState = state$.value.question.questions[searchName];
-
-      if (questionState == null) {
-        return EMPTY;
-      }
-
-      const { globalParamMapping, paramValues: newParamValues } = questionState;
-
-      await updateLastParamValues(
-        paramValueStore,
-        searchName,
-        newParamValues,
-        globalParamMapping
-      );
-      return EMPTY;
-    }),
-    mergeAll()
-  );
-
 type ActionAffectingGroupCount =
   | ChangeGroupVisibilityAction
   | UpdateParamValueAction;
@@ -757,13 +729,6 @@ const observeQuestionSubmit: QuestionEpic = (action$, state$, services) =>
               wdkWeight: Number.isNaN(weight) ? DEFAULT_STEP_WEIGHT : weight,
             };
 
-            updateLastParamValues(
-              services.paramValueStore,
-              searchName,
-              paramValues,
-              globalParamMapping
-            );
-
             if (submissionMetadata.type === 'edit-step') {
               return of(
                 requestReviseStep(
@@ -957,6 +922,16 @@ const observeQuestionSubmit: QuestionEpic = (action$, state$, services) =>
                 )
             );
           })
+          .then((nextAction) => {
+            const { paramValues, globalParamMapping, question } = questionState;
+            updateLastParamValues(
+              services.paramValueStore,
+              question.urlSegment,
+              paramValues,
+              globalParamMapping
+            );
+            return nextAction;
+          })
       ).pipe(
         mergeAll(),
         catchError((error: any) =>
@@ -989,7 +964,6 @@ export const observeQuestion: QuestionEpic = combineEpics(
   observeLoadQuestion,
   observeLoadQuestionSuccess,
   observeAutoRun,
-  observeStoreUpdatedParams,
   observeUpdateDependentParams,
   observeLoadGroupCount,
   observeQuestionSubmit,
@@ -1063,13 +1037,6 @@ async function loadQuestion(
 
     const wdkWeight = step == null ? undefined : step.searchConfig.wdkWeight;
 
-    await updateLastParamValues(
-      paramValueStore,
-      searchName,
-      paramValues,
-      globalParamMapping
-    );
-
     return questionLoaded({
       autoRun,
       prepopulateWithLastParamValues,
@@ -1137,7 +1104,7 @@ function extracParamValues(
   return pick(initialParamData, paramNames);
 }
 
-function updateLastParamValues(
+export function updateLastParamValues(
   paramValueStore: ParamValueStore,
   searchName: string,
   newParamValues: ParameterValues,

diff --git a/packages/libs/wdk-client/src/Views/Question/Params/EnumParam.tsx b/packages/libs/wdk-client/src/Views/Question/Params/EnumParam.tsx
@@ -25,6 +25,7 @@ import {
   isMultiPick,
   toMultiValueString,
   toMultiValueArray,
+  countInBounds,
 } from '../../../Views/Question/Params/EnumParamUtils';
 
 // TODO: Move TreeBox state into TreeBoxEnumParam component
@@ -39,11 +40,16 @@ export default createParamModule({
 
 function isParamValueValid(context: Context<EnumParam>) {
   let value = context.paramValues[context.parameter.name];
-  return (
-    typeof value === 'string' &&
-    (context.parameter.type !== 'multi-pick-vocabulary' ||
-      isValidEnumJson(value))
-  );
+  if (context.parameter.type === 'multi-pick-vocabulary') {
+    if (!isValidEnumJson(value)) return false;
+    const typedValue = toMultiValueArray(value);
+    return countInBounds(
+      typedValue.length,
+      context.parameter.minSelectedCount,
+      context.parameter.maxSelectedCount
+    );
+  }
+  return typeof value === 'string';
 }
 
 function isType(parameter: Parameter): parameter is EnumParam {

diff --git a/packages/libs/wdk-client/src/Views/Question/Params/EnumParamUtils.ts b/packages/libs/wdk-client/src/Views/Question/Params/EnumParamUtils.ts
@@ -37,9 +37,14 @@ export function isMultiPick(parameter: Parameter): boolean {
 }
 
 export function isValidEnumJson(value: string): boolean {
-  const validationResult = enumJsonDecoder(value);
-
-  return validationResult.status === 'ok';
+  try {
+    const parsedValue = JSON.parse(value);
+    const validationResult = enumJsonDecoder(parsedValue);
+
+    return validationResult.status === 'ok';
+  } catch {
+    return false;
+  }
 }
 
 const enumJsonDecoder = arrayOf(string);
diff --git a/packages/libs/wdk-client/src/Views/Question/Params/SelectionInfo.tsx b/packages/libs/wdk-client/src/Views/Question/Params/SelectionInfo.tsx
@@ -22,7 +22,7 @@ export default function SelectionInfo(props: Props) {
       ? `${
           isSingleSelect ? '' : 'between ' + minSelectedCount + ' and '
         }${maxSelectedCount} ${valueDescription(maxSelectedCount)} required`
-      : hasMin && selectedCount > 0
+      : hasMin && (selectedCount > 0 || minSelectedCount === 1)
       ? `at least ${minSelectedCount} ${valueDescription(
           minSelectedCount
         )} required`

diff --git a/.../sites/genomics-site/webapp/wdkCustomization/js/client/components/Downloads/Downloads.tsx b/.../sites/genomics-site/webapp/wdkCustomization/js/client/components/Downloads/Downloads.tsx
@@ -1,7 +1,14 @@
-import React, { useMemo, useState } from 'react';
-import { useHistory, useLocation, useRouteMatch } from 'react-router';
+import Banner from '@veupathdb/coreui/lib/components/banners/Banner';
 import { parseQueryString } from '@veupathdb/wdk-client/lib/Core/RouteEntry';
+import { RootState } from '@veupathdb/wdk-client/lib/Core/State/Types';
+import { useWdkDependenciesEffect } from '@veupathdb/wdk-client/lib/Hooks/WdkDependenciesEffect';
+import { updateLastParamValues } from '@veupathdb/wdk-client/lib/StoreModules/QuestionStoreModule';
 import { SearchConfig } from '@veupathdb/wdk-client/lib/Utils/WdkModel';
+import { isParamValueValid } from '@veupathdb/wdk-client/lib/Views/Question/Params';
+import { isEqual } from 'lodash';
+import React, { useMemo } from 'react';
+import { useSelector } from 'react-redux';
+import { useHistory, useLocation, useRouteMatch } from 'react-router';
 import { DownloadsFilter } from './DownloadsFilter';
 import { DownloadsTable } from './DownloadsTable';
 import './Downloads.scss';
@@ -11,7 +18,6 @@ const TABLE_QUESTION_NAME = 'GetAllFileRecords';
 const BULK_QUESTION_NAME = 'GetFileRecordsByID';
 
 export function Downloads() {
-  const [searchConfig, setSearchConfig] = useState<SearchConfig>();
   const location = useLocation();
   const history = useHistory();
   const match = useRouteMatch();
@@ -20,6 +26,43 @@ export function Downloads() {
     [history, location, match]
   );
 
+  const { searchConfig, isValid } = useSelector(
+    (state: RootState) => {
+      const questionState = state.question.questions[TABLE_QUESTION_NAME];
+      const searchConfig: SearchConfig | undefined =
+        questionState?.paramValues && {
+          parameters: questionState.paramValues,
+        };
+      const isValid = questionState?.paramValues
+        ? questionState.question.parameters.every((parameter) =>
+            isParamValueValid(
+              {
+                searchName: TABLE_QUESTION_NAME,
+                paramValues: questionState?.paramValues,
+                parameter,
+              },
+              questionState.paramUIState[parameter.name]
+            )
+          )
+        : true;
+      return { searchConfig, isValid };
+    },
+    (left, right) => isEqual(left, right)
+  );
+
+  useWdkDependenciesEffect(
+    ({ paramValueStore }) => {
+      if (searchConfig == null || !isValid) return;
+      updateLastParamValues(
+        paramValueStore,
+        TABLE_QUESTION_NAME,
+        searchConfig?.parameters,
+        undefined
+      );
+    },
+    [searchConfig, isValid]
+  );
+
   return (
     <div className="Downloads">
       <h1>Download Data Files</h1>
@@ -31,17 +74,24 @@ export function Downloads() {
         <DownloadsFilter
           recordName={RECORD_NAME}
           questionName={TABLE_QUESTION_NAME}
-          onChange={setSearchConfig}
           initialParamData={initialParamData}
         />
       </div>
-      {searchConfig && (
+      {!isValid ? (
+        <Banner
+          banner={{
+            type: 'error',
+            message:
+              'One or more parameter selections above is invalid. Fix them to see results.',
+          }}
+        />
+      ) : searchConfig ? (
         <DownloadsTable
           tableSearchName={TABLE_QUESTION_NAME}
           bulkSearchName={BULK_QUESTION_NAME}
           searchConfig={searchConfig}
         />
-      )}
+      ) : null}
     </div>
   );
 }
diff --git a/.../genomics-site/webapp/wdkCustomization/js/client/components/Downloads/DownloadsFilter.tsx b/.../genomics-site/webapp/wdkCustomization/js/client/components/Downloads/DownloadsFilter.tsx
@@ -1,20 +1,16 @@
+import { mapValues } from 'lodash';
+import React, { useMemo } from 'react';
 import { SubmissionMetadata } from '@veupathdb/wdk-client/lib/Actions/QuestionActions';
 import { QuestionController } from '@veupathdb/wdk-client/lib/Controllers';
-import { RootState } from '@veupathdb/wdk-client/lib/Core/State/Types';
-import { SearchConfig } from '@veupathdb/wdk-client/lib/Utils/WdkModel';
 import {
   Props as FormProps,
   renderDefaultParamGroup,
 } from '@veupathdb/wdk-client/lib/Views/Question/DefaultQuestionForm';
-import React, { useEffect, useMemo } from 'react';
-import { useSelector } from 'react-redux';
-import { mapValues } from 'lodash';
 
 interface Props {
   recordName: string;
   questionName: string;
   initialParamData: Record<string, string>;
-  onChange: (searchConfig: SearchConfig) => void;
 }
 
 const submissionMetadata: SubmissionMetadata = {
@@ -23,17 +19,7 @@ const submissionMetadata: SubmissionMetadata = {
 };
 
 export function DownloadsFilter(props: Props) {
-  const { recordName, questionName, initialParamData, onChange } = props;
-  const paramValues = useSelector(
-    (state: RootState) => state.question.questions[questionName]?.paramValues
-  );
-  useEffect(() => {
-    if (paramValues) {
-      onChange({
-        parameters: paramValues,
-      });
-    }
-  }, [paramValues, onChange]);
+  const { recordName, questionName, initialParamData } = props;
 
   return (
     <QuestionController