Show 'From connected filters' option if parameter has fields (#34958)

frontend/src/metabase-lib/parameters/utils/parameter-fields.ts

@@ -7,7 +7,11 @@ import type {
 export const hasFields = (
   parameter: UiParameter,
 ): parameter is FieldFilterUiParameter => {
-  return (parameter as FieldFilterUiParameter).fields != null;
+  return (
+    "fields" in parameter &&
+    Array.isArray(parameter.fields) &&
+    parameter.fields.length > 0
+  );
 };
 
 export const getFields = (parameter: UiParameter): Field[] => {

frontend/src/metabase-lib/parameters/utils/parameter-fields.unit.spec.ts

+import { createMockField } from "metabase-types/api/mocks";
+import { createMockMetadata } from "__support__/metadata";
+import { createMockUiParameter } from "metabase-lib/parameters/mock";
+import type Field from "metabase-lib/metadata/Field";
+import { hasFields } from "./parameter-fields";
+
+describe("parameters/utils/parameter-fields", () => {
+  it("`hasFields` should return true if there is at least one field in fields", () => {
+    const metadata = createMockMetadata({
+      fields: [
+        createMockField({
+          id: 1,
+        }),
+      ],
+    });
+    const field = metadata.field(1) as Field;
+    const parameterWithFields = createMockUiParameter({
+      fields: [field],
+    });
+    const parameterWithoutFields = createMockUiParameter({
+      fields: [],
+    });
+    const parameterWithoutFieldsKey = createMockUiParameter();
+    expect(hasFields(parameterWithFields)).toBe(true);
+    expect(hasFields(parameterWithoutFields)).toBe(false);
+    expect(hasFields(parameterWithoutFieldsKey)).toBe(false);
+  });
+});

frontend/src/metabase/parameters/components/ParameterValueWidget.jsx

@@ -25,12 +25,12 @@ import {
   getNumberParameterArity,
   getStringParameterArity,
 } from "metabase-lib/parameters/utils/operators";
-import { getFields } from "metabase-lib/parameters/utils/parameter-fields";
 import { getQueryType } from "metabase-lib/parameters/utils/parameter-source";
 import {
   isDateParameter,
   isNumberParameter,
 } from "metabase-lib/parameters/utils/parameter-type";
+import { hasFields } from "metabase-lib/parameters/utils/parameter-fields";
 
 import ParameterFieldWidget from "./widgets/ParameterFieldWidget/ParameterFieldWidget";
 import S from "./ParameterValueWidget.css";
@@ -315,9 +315,8 @@ function isTextWidget(parameter) {
 
 function isFieldWidget(parameter) {
   const canQuery = getQueryType(parameter) !== "none";
-  const hasFields = getFields(parameter).length > 0;
 
   return parameter.hasVariableTemplateTagTarget
     ? canQuery
-    : canQuery || hasFields;
+    : canQuery || hasFields(parameter);
 }

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

 import { useCallback, useState } from "react";
-import type {
-  Parameter,
-  ValuesSourceConfig,
-  ValuesSourceType,
-} from "metabase-types/api";
+import type { ValuesSourceConfig, ValuesSourceType } from "metabase-types/api";
 import {
   getSourceConfig,
   getSourceConfigForType,
   getSourceType,
 } from "metabase-lib/parameters/utils/parameter-source";
-import type { ParameterWithTemplateTagTarget } from "metabase-lib/parameters/types";
+import type { UiParameter } from "metabase-lib/parameters/types";
+import { hasFields } from "metabase-lib/parameters/utils/parameter-fields";
 import ValuesSourceTypeModal from "./ValuesSourceTypeModal";
 import ValuesSourceCardModal from "./ValuesSourceCardModal";
 
 type ModalStep = "main" | "card";
 
 interface ModalProps {
-  parameter: Parameter;
+  parameter: UiParameter;
   onSubmit: (
     sourceType: ValuesSourceType,
     sourceConfig: ValuesSourceConfig,
@@ -68,12 +65,14 @@ const ValuesSourceModal = ({
   );
 };
 
-const getInitialSourceType = (parameter: ParameterWithTemplateTagTarget) => {
+/**
+ * after using this function to initialize sourceType, we know that:
+ * (intial sourceType === null) => hasFields(parameter)
+ */
+const getInitialSourceType = (parameter: UiParameter) => {
   const sourceType = getSourceType(parameter);
 
-  return sourceType === null && parameter.hasVariableTemplateTagTarget
-    ? "card"
-    : sourceType;
+  return sourceType === null && !hasFields(parameter) ? "card" : sourceType;
 };
 
 // eslint-disable-next-line import/no-default-export -- deprecated usage

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

@@ -48,14 +48,6 @@ describe("ValuesSourceModal", () => {
   const field2 = checkNotNull(metadata.field(2));
 
   describe("fields source", () => {
-    it("should show a message about not connected fields", async () => {
-      await setup();
-
-      expect(
-        screen.getByText(/You haven’t connected a field to this filter/),
-      ).toBeInTheDocument();
-    });
-
     it("should show a message about missing field values", async () => {
       await setup({
         parameter: createMockUiParameter({
@@ -84,19 +76,28 @@ describe("ValuesSourceModal", () => {
       expect(screen.getByRole("textbox")).toHaveValue("A\nB\nC");
     });
 
-    it("should not show the fields option for variable template tags", async () => {
+    it("should not show the connected fields option if parameter is not wired to any fields", async () => {
+      await setup();
+      expect(
+        screen.queryByRole("radio", { name: "From connected fields" }),
+      ).not.toBeInTheDocument();
+      expect(
+        screen.getByRole("radio", { name: "From another model or question" }),
+      ).toBeChecked();
+    });
+
+    it("should show the fields option if parameter is wired to a field", async () => {
       await setup({
         parameter: createMockUiParameter({
-          hasVariableTemplateTagTarget: true,
+          fields: [field1],
         }),
       });
-
       expect(
         screen.queryByRole("radio", { name: "From connected fields" }),
-      ).not.toBeInTheDocument();
+      ).toBeChecked();
       expect(
         screen.getByRole("radio", { name: "From another model or question" }),
-      ).toBeChecked();
+      ).toBeInTheDocument();
     });
 
     it("should preserve custom list option for variable template tags", async () => {
@@ -368,6 +369,7 @@ describe("ValuesSourceModal", () => {
     it("should preserve the list when changing the source type", async () => {
       await setup({
         parameter: createMockUiParameter({
+          fields: [field1],
           values_source_type: "static-list",
           values_source_config: {
             values: ["Gadget", "Widget"],

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

@@ -24,9 +24,12 @@ import type { State } from "metabase-types/store";
 import type Question from "metabase-lib/Question";
 import type Field from "metabase-lib/metadata/Field";
 import { getQuestionVirtualTableId } from "metabase-lib/metadata/utils/saved-questions";
-import { getFields } from "metabase-lib/parameters/utils/parameter-fields";
 import { isValidSourceConfig } from "metabase-lib/parameters/utils/parameter-source";
-import type { ParameterWithTemplateTagTarget } from "metabase-lib/parameters/types";
+import type {
+  FieldFilterUiParameter,
+  UiParameter,
+} from "metabase-lib/parameters/types";
+import { hasFields } from "metabase-lib/parameters/utils/parameter-fields";
 import type { FetchParameterValuesOpts } from "../../actions";
 import { fetchParameterValues } from "../../actions";
 import { ModalLoadingAndErrorWrapper } from "./ValuesSourceModal.styled";
@@ -45,7 +48,7 @@ import {
 const NEW_LINE = "\n";
 
 interface ModalOwnProps {
-  parameter: ParameterWithTemplateTagTarget;
+  parameter: UiParameter;
   sourceType: ValuesSourceType;
   sourceConfig: ValuesSourceConfig;
   onChangeSourceType: (sourceType: ValuesSourceType) => void;
@@ -94,7 +97,8 @@ const ValuesSourceTypeModal = ({
     >
       {sourceType === null ? (
         <FieldSourceModal
-          parameter={parameter}
+          // if sourceType === null the parameter must have fields
+          parameter={parameter as FieldFilterUiParameter}
           sourceType={sourceType}
           sourceConfig={sourceConfig}
           onFetchParameterValues={onFetchParameterValues}
@@ -126,7 +130,7 @@ const ValuesSourceTypeModal = ({
 };
 
 interface SourceTypeOptionsProps {
-  parameter: Parameter;
+  parameter: UiParameter;
   parameterValues?: ParameterValue[];
   sourceType: ValuesSourceType;
   sourceConfig: ValuesSourceConfig;
@@ -169,7 +173,7 @@ const SourceTypeOptions = ({
 };
 
 interface FieldSourceModalProps {
-  parameter: Parameter;
+  parameter: FieldFilterUiParameter;
   sourceType: ValuesSourceType;
   sourceConfig: ValuesSourceConfig;
   onFetchParameterValues: (
@@ -199,7 +203,6 @@ const FieldSourceModal = ({
     [values],
   );
 
-  const hasFields = getFields(parameter).length > 0;
   const hasEmptyValues = !isLoading && values.length === 0;
 
   return (
@@ -218,11 +221,7 @@ const FieldSourceModal = ({
         </ModalSection>
       </ModalPane>
       <ModalMain>
-        {!hasFields ? (
-          <ModalEmptyState>
-            {t`You haven’t connected a field to this filter yet, so there aren’t any values.`}
-          </ModalEmptyState>
-        ) : hasEmptyValues ? (
+        {hasEmptyValues ? (
           <ModalEmptyState>
             {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>
@@ -422,13 +421,17 @@ const getSupportedFields = (question: Question) => {
   return fields.filter(field => field.isString());
 };
 
+/**
+ * if !hasFields(parameter) then exclude the option to set the source type to
+ * "From connected fields" i.e. values_source_type=null
+ */
 const getSourceTypeOptions = (
-  parameter: ParameterWithTemplateTagTarget,
+  parameter: UiParameter,
 ): RadioOption<ValuesSourceType>[] => {
   return [
-    ...(parameter.hasVariableTemplateTagTarget
-      ? []
-      : [{ name: t`From connected fields`, value: null }]),
+    ...(hasFields(parameter)
+      ? [{ name: t`From connected fields`, value: null }]
+      : []),
     { name: t`From another model or question`, value: "card" },
     { name: t`Custom list`, value: "static-list" },
   ];