Show query error in the parameter modal (#28119)

frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.unit.spec.tsx

@@ -12,6 +12,7 @@ import {
 import {
   setupCardsEndpoints,
   setupCollectionsEndpoints,
+  setupErrorParameterValuesEndpoints,
   setupParameterValuesEndpoints,
   setupUnauthorizedCardsEndpoints,
 } from "__support__/server-mocks";
@@ -135,6 +136,10 @@ describe("ValuesSourceModal", () => {
         }),
       });
 
+      expect(
+        await screen.findByText(/We don’t have any cached values/),
+      ).toBeInTheDocument();
+
       userEvent.click(screen.getByRole("radio", { name: "Custom list" }));
       expect(screen.getByRole("radio", { name: "Custom list" })).toBeChecked();
       expect(screen.getByRole("textbox")).toHaveValue("A\nB");
@@ -241,7 +246,7 @@ describe("ValuesSourceModal", () => {
       });
     });
 
-    it("should display an error message when the user has no access to the card", async () => {
+    it("should display a message when the user has no access to the card", async () => {
       setup({
         parameter: createMockUiParameter({
           values_source_type: "card",
@@ -263,6 +268,37 @@ describe("ValuesSourceModal", () => {
       ).toBeInTheDocument();
     });
 
+    it("should display a message when there is an error in the underlying query", async () => {
+      setup({
+        parameter: createMockUiParameter({
+          values_source_type: "card",
+          values_source_config: {
+            card_id: 1,
+            value_field: ["field", 2, null],
+          },
+        }),
+        cards: [
+          createMockCard({
+            id: 1,
+            name: "Products",
+            result_metadata: [
+              createMockField({
+                id: 2,
+                display_name: "Category",
+                base_type: "type/Text",
+                semantic_type: "type/Category",
+              }),
+            ],
+          }),
+        ],
+        hasParameterValuesError: true,
+      });
+
+      expect(
+        await screen.findByText("An error occurred in your query"),
+      ).toBeInTheDocument();
+    });
+
     it("should copy card values when switching to custom list", async () => {
       setup({
         parameter: createMockUiParameter({
@@ -339,6 +375,7 @@ interface SetupOpts {
   parameterValues?: ParameterValues;
   cards?: Card[];
   hasDataAccess?: boolean;
+  hasParameterValuesError?: boolean;
 }
 
 const setup = ({
@@ -346,6 +383,7 @@ const setup = ({
   parameterValues = createMockParameterValues(),
   cards = [],
   hasDataAccess = true,
+  hasParameterValuesError = false,
 }: SetupOpts = {}) => {
   const scope = nock(location.origin);
   const onSubmit = jest.fn();
@@ -354,7 +392,12 @@ const setup = ({
   if (hasDataAccess) {
     setupCollectionsEndpoints(scope, [createMockCollection(ROOT_COLLECTION)]);
     setupCardsEndpoints(scope, cards);
-    setupParameterValuesEndpoints(scope, parameterValues);
+
+    if (!hasParameterValuesError) {
+      setupParameterValuesEndpoints(scope, parameterValues);
+    } else {
+      setupErrorParameterValuesEndpoints(scope);
+    }
   } else {
     setupUnauthorizedCardsEndpoints(scope, cards);
   }

frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceTypeModal.tsx

 import React, {
   ChangeEvent,
   useCallback,
-  useEffect,
+  useLayoutEffect,
   useMemo,
   useState,
 } from "react";
@@ -201,20 +201,20 @@ const FieldSourceModal = ({
   onChangeSourceType,
   onChangeSourceConfig,
 }: FieldSourceModalProps) => {
-  const parameterValues = useParameterValues({
+  const { values, isLoading } = useParameterValues({
     parameter,
     sourceType,
     sourceConfig,
     onFetchParameterValues,
   });
 
-  const parameterValuesText = useMemo(
-    () => getValuesText(getSourceValues(parameterValues)),
-    [parameterValues],
+  const valuesText = useMemo(
+    () => getValuesText(getSourceValues(values)),
+    [values],
   );
 
   const hasFields = getFields(parameter).length > 0;
-  const hasEmptyValues = parameterValues && parameterValues.length === 0;
+  const hasEmptyValues = !isLoading && values.length === 0;
 
   return (
     <ModalBodyWithPane>
@@ -223,7 +223,7 @@ const FieldSourceModal = ({
           <ModalLabel>{t`Where values should come from`}</ModalLabel>
           <SourceTypeOptions
             parameter={parameter}
-            parameterValues={parameterValues}
+            parameterValues={values}
             sourceType={sourceType}
             sourceConfig={sourceConfig}
             onChangeSourceType={onChangeSourceType}
@@ -241,7 +241,7 @@ const FieldSourceModal = ({
             {t`We don’t have any cached values for the connected fields. Try one of the other options, or change this widget to a search box.`}
           </ModalEmptyState>
         ) : (
-          <ModalTextArea value={parameterValuesText} readOnly fullWidth />
+          <ModalTextArea value={valuesText} readOnly fullWidth />
         )}
       </ModalMain>
     </ModalBodyWithPane>
@@ -279,16 +279,16 @@ const CardSourceModal = ({
     return getFieldByReference(fields, sourceConfig.value_field);
   }, [fields, sourceConfig]);
 
-  const parameterValues = useParameterValues({
+  const { values, isError } = useParameterValues({
     parameter,
     sourceType,
     sourceConfig,
     onFetchParameterValues,
   });
 
-  const parameterValuesText = useMemo(
-    () => getValuesText(getSourceValues(parameterValues)),
-    [parameterValues],
+  const valuesText = useMemo(
+    () => getValuesText(getSourceValues(values)),
+    [values],
   );
 
   const handleFieldChange = useCallback(
@@ -308,7 +308,7 @@ const CardSourceModal = ({
           <ModalLabel>{t`Where values should come from`}</ModalLabel>
           <SourceTypeOptions
             parameter={parameter}
-            parameterValues={parameterValues}
+            parameterValues={values}
             sourceType={sourceType}
             sourceConfig={sourceConfig}
             onChangeSourceType={onChangeSourceType}
@@ -354,8 +354,10 @@ const CardSourceModal = ({
           <ModalEmptyState>{t`Pick a model or question`}</ModalEmptyState>
         ) : !selectedField ? (
           <ModalEmptyState>{t`Pick a column`}</ModalEmptyState>
+        ) : isError ? (
+          <ModalEmptyState>{t`An error occurred in your query`}</ModalEmptyState>
         ) : (
-          <ModalTextArea value={parameterValuesText} readOnly fullWidth />
+          <ModalTextArea value={valuesText} readOnly fullWidth />
         )}
       </ModalMain>
     </ModalBodyWithPane>
@@ -446,6 +448,12 @@ const getSourceTypeOptions = (
   ];
 };
 
+interface ParameterValuesState {
+  values: ParameterValue[];
+  isLoading?: boolean;
+  isError?: boolean;
+}
+
 interface UseParameterValuesOpts {
   parameter: Parameter;
   sourceType: ValuesSourceType;
@@ -461,7 +469,7 @@ const useParameterValues = ({
   sourceConfig,
   onFetchParameterValues,
 }: UseParameterValuesOpts) => {
-  const [values, setValues] = useState<ParameterValue[]>();
+  const [state, setState] = useState<ParameterValuesState>({ values: [] });
   const handleFetchValues = useSafeAsyncFunction(onFetchParameterValues);
   const isValidSource = isValidSourceConfig(sourceType, sourceConfig);
 
@@ -474,15 +482,17 @@ const useParameterValues = ({
     [initialParameter, sourceType, sourceConfig],
   );
 
-  useEffect(() => {
+  useLayoutEffect(() => {
     if (isValidSource) {
+      setState(({ values }) => ({ values, isLoading: true }));
+
       handleFetchValues({ parameter })
-        .then(({ values }) => setValues(values))
-        .catch(() => setValues([]));
+        .then(({ values }) => setState({ values }))
+        .catch(() => setState({ values: [], isError: true }));
     }
   }, [parameter, isValidSource, handleFetchValues]);
 
-  return values;
+  return state;
 };
 
 const mapStateToProps = (

frontend/test/__support__/server-mocks/dataset.ts

@@ -7,3 +7,7 @@ export function setupParameterValuesEndpoints(
 ) {
   scope.post("/api/dataset/parameter/values").reply(200, parameterValues);
 }
+
+export function setupErrorParameterValuesEndpoints(scope: Scope) {
+  scope.post("/api/dataset/parameter/values").reply(500);
+}