diff --git a/frontend/src/metabase-lib/metadata/Field.ts b/frontend/src/metabase-lib/metadata/Field.ts index 6869d187688c9cef040ce5daff59ffa72fd5f73a..eebc941ae8775a38567a2d531592398b18bc6584 100644 --- a/frontend/src/metabase-lib/metadata/Field.ts +++ b/frontend/src/metabase-lib/metadata/Field.ts @@ -68,6 +68,7 @@ class FieldInner extends Base { table_id?: Table["id"]; target?: Field; has_field_values?: "list" | "search" | "none"; + has_more_values?: boolean; values: any[]; metadata?: Metadata; source?: string; @@ -444,6 +445,19 @@ class FieldInner extends Base { return this.isString(); } + searchField(disablePKRemapping = false): Field | null { + if (disablePKRemapping && this.isPK()) { + return this.isSearchable() ? this : null; + } + + const remappedField = this.remappedField(); + if (remappedField && remappedField.isSearchable()) { + return remappedField; + } + + return this.isSearchable() ? this : null; + } + column(extra = {}): DatasetColumn { return this.dimension().column({ source: "fields", diff --git a/frontend/src/metabase-lib/parameters/constants.ts b/frontend/src/metabase-lib/parameters/constants.ts index d448c56810420c018b5b70af4526dc83040d5826..015625060d909f36e643fb2a2d5ba7cf677a806e 100644 --- a/frontend/src/metabase-lib/parameters/constants.ts +++ b/frontend/src/metabase-lib/parameters/constants.ts @@ -150,6 +150,7 @@ export const LOCATION_OPTIONS = [ export const CUSTOM_SOURCE_PARAMETER_TYPES: Record<string, string[]> = { string: ["="], + location: ["="], }; export const TYPE_SUPPORTS_LINKED_FILTERS = [ diff --git a/frontend/src/metabase-lib/parameters/mock.ts b/frontend/src/metabase-lib/parameters/mock.ts index 68e6a3f163dfe84b90d5be32a802a53b71766edb..e09816fbc63719f79b45c27f8bb51a20d708f523 100644 --- a/frontend/src/metabase-lib/parameters/mock.ts +++ b/frontend/src/metabase-lib/parameters/mock.ts @@ -7,8 +7,5 @@ export const createMockUiParameter = ( slug: "slug", name: "Name", type: "string/=", - values_query_type: "list", - values_source_type: null, - values_source_config: {}, ...opts, }); diff --git a/frontend/src/metabase-lib/parameters/utils/parameter-source.ts b/frontend/src/metabase-lib/parameters/utils/parameter-source.ts index 5f2663948de44dd987b7bcd00f17e7f379a571bd..3017e4871472d8e3b71b7dc108ee283f1203d20f 100644 --- a/frontend/src/metabase-lib/parameters/utils/parameter-source.ts +++ b/frontend/src/metabase-lib/parameters/utils/parameter-source.ts @@ -1,27 +1,119 @@ -import { ValuesSourceConfig, ValuesSourceType } from "metabase-types/api"; +import { + Dataset, + FieldValue, + Parameter, + ValuesQueryType, + ValuesSourceConfig, + ValuesSourceType, +} from "metabase-types/api"; +import { getFields } from "metabase-lib/parameters/utils/parameter-fields"; +import Field from "metabase-lib/metadata/Field"; + +export const getQueryType = (parameter: Parameter): ValuesQueryType => { + return parameter.values_query_type ?? "list"; +}; + +export const getSourceType = (parameter: Parameter): ValuesSourceType => { + return parameter.values_source_type ?? null; +}; + +export const getSourceConfig = (parameter: Parameter): ValuesSourceConfig => { + return parameter.values_source_config ?? {}; +}; export const isValidSourceConfig = ( sourceType: ValuesSourceType, - sourceConfig: ValuesSourceConfig, + { card_id, value_field, values }: ValuesSourceConfig, ) => { switch (sourceType) { case "card": - return sourceConfig.card_id != null && sourceConfig.value_field != null; + return card_id != null && value_field != null; case "static-list": - return sourceConfig.values != null && sourceConfig.values.length > 0; + return values != null && values.length > 0; default: return true; } }; -export const getDefaultSourceConfig = ( +export const getSourceConfigForType = ( sourceType: ValuesSourceType, - sourceValues?: string[], + { card_id, value_field, values }: ValuesSourceConfig, ): ValuesSourceConfig => { switch (sourceType) { + case "card": + return { card_id, value_field }; case "static-list": - return { values: sourceValues }; + return { values }; default: return {}; } }; + +export const canListParameterValues = (parameter: Parameter) => { + const fields = getFields(parameter); + + if (getQueryType(parameter) !== "list") { + return false; + } else if (getSourceType(parameter) != null) { + return true; + } else { + return canListFieldValues(fields); + } +}; + +export const canListFieldValues = (fields: Field[]) => { + const hasFields = fields.length > 0; + const hasFieldValues = fields + .filter(field => !field.isVirtual()) + .every(field => field.has_field_values === "list"); + + return hasFields && hasFieldValues; +}; + +export const canSearchParameterValues = ( + parameter: Parameter, + disablePKRemapping = false, +) => { + const fields = getFields(parameter); + + if (getQueryType(parameter) === "none") { + return false; + } else if (getSourceType(parameter) != null) { + return true; + } else { + return canSearchFieldValues(fields, disablePKRemapping); + } +}; + +export const canSearchFieldValues = ( + fields: Field[], + disablePKRemapping = false, +) => { + const hasFields = fields.length > 0; + const canSearch = fields.every(field => + field.searchField(disablePKRemapping), + ); + const hasFieldValues = fields.some( + field => + field.has_field_values === "search" || + (field.has_field_values === "list" && field.has_more_values === true), + ); + + return hasFields && canSearch && hasFieldValues; +}; + +const getUniqueNonNullValues = (values: unknown[]) => { + return Array.from(new Set(values)) + .filter(value => value != null) + .map(value => String(value)); +}; + +export const getFieldSourceValues = (fieldsValues: FieldValue[][]) => { + const allValues = fieldsValues.flatMap(values => values.map(([key]) => key)); + return getUniqueNonNullValues(allValues); +}; + +export const getCardSourceValues = (dataset: Dataset) => { + const allValues = dataset.data.rows.map(([value]) => value); + return getUniqueNonNullValues(allValues); +}; diff --git a/frontend/src/metabase-lib/parameters/utils/parameter-source.unit.spec.ts b/frontend/src/metabase-lib/parameters/utils/parameter-source.unit.spec.ts new file mode 100644 index 0000000000000000000000000000000000000000..f2b08812bb7d886f0ea6a5a4266f005a2a520f6d --- /dev/null +++ b/frontend/src/metabase-lib/parameters/utils/parameter-source.unit.spec.ts @@ -0,0 +1,115 @@ +import { createMockField } from "metabase-types/api/mocks"; +import Field from "metabase-lib/metadata/Field"; +import { createMockUiParameter } from "metabase-lib/parameters/mock"; +import { canListParameterValues } from "./parameter-source"; + +describe("canListParameterValues", () => { + it("should not list the query type other than list", () => { + const parameter = createMockUiParameter({ + fields: [ + new Field( + createMockField({ + id: 1, + has_field_values: "list", + }), + ), + new Field( + createMockField({ + id: 2, + has_field_values: "list", + }), + ), + ], + values_query_type: "none", + }); + + expect(canListParameterValues(parameter)).toBeFalsy(); + }); + + it("should list with the default source when all fields have field values", () => { + const parameter = createMockUiParameter({ + fields: [ + new Field( + createMockField({ + id: 1, + has_field_values: "list", + }), + ), + new Field( + createMockField({ + id: 2, + has_field_values: "list", + }), + ), + ], + }); + + expect(canListParameterValues(parameter)).toBeTruthy(); + }); + + it("should not list with the default source when there are no fields", () => { + const parameter = createMockUiParameter({ + fields: [], + }); + + expect(canListParameterValues(parameter)).toBeFalsy(); + }); + + it("should not list with the default source when some fields don't have field values", () => { + const parameter = createMockUiParameter({ + fields: [ + new Field( + createMockField({ + id: 1, + has_field_values: "list", + }), + ), + new Field( + createMockField({ + id: 2, + has_field_values: "search", + }), + ), + ], + }); + + expect(canListParameterValues(parameter)).toBeFalsy(); + }); + + it("should list with the card source", () => { + const parameter = createMockUiParameter({ + fields: [ + new Field( + createMockField({ + has_field_values: "none", + }), + ), + ], + values_source_type: "card", + values_source_config: { + card_id: 1, + value_field: ["field", 1, null], + }, + }); + + expect(canListParameterValues(parameter)).toBeTruthy(); + }); + + it("should list with the static list source", () => { + const parameter = createMockUiParameter({ + fields: [ + new Field( + createMockField({ + has_field_values: "none", + }), + ), + ], + values_source_type: "static-list", + values_source_config: { + values: ["A", "B"], + }, + }); + + expect(canListParameterValues(parameter)).toBeTruthy(); + }); +}); diff --git a/frontend/src/metabase-lib/parameters/utils/template-tags.ts b/frontend/src/metabase-lib/parameters/utils/template-tags.ts index 9cab3d669649f745b9271defe1e979e4b351a728..60a687e3dd8d7c243b2d7cc182161e298264c18c 100644 --- a/frontend/src/metabase-lib/parameters/utils/template-tags.ts +++ b/frontend/src/metabase-lib/parameters/utils/template-tags.ts @@ -38,9 +38,6 @@ export function getTemplateTagParameter(tag: TemplateTag): ParameterWithTarget { name: tag["display-name"], slug: tag.name, default: tag.default, - values_query_type: "list", - values_source_type: null, - values_source_config: {}, }; } diff --git a/frontend/src/metabase-lib/parameters/utils/template-tags.unit.spec.js b/frontend/src/metabase-lib/parameters/utils/template-tags.unit.spec.js index f0c798ae6b48440c17e515b447e9c9b4f19b035a..9c91b3c79e641e1086018de0c72ab43d64975b21 100644 --- a/frontend/src/metabase-lib/parameters/utils/template-tags.unit.spec.js +++ b/frontend/src/metabase-lib/parameters/utils/template-tags.unit.spec.js @@ -122,9 +122,6 @@ describe("parameters/utils/cards", () => { slug: "a", target: ["variable", ["template-tag", "a"]], type: "foo", - values_query_type: "list", - values_source_type: null, - values_source_config: {}, }, { default: undefined, @@ -133,9 +130,6 @@ describe("parameters/utils/cards", () => { slug: "b", target: ["variable", ["template-tag", "b"]], type: "string/=", - values_query_type: "list", - values_source_type: null, - values_source_config: {}, }, { default: undefined, @@ -144,9 +138,6 @@ describe("parameters/utils/cards", () => { slug: "c", target: ["variable", ["template-tag", "c"]], type: "number/=", - values_query_type: "list", - values_source_type: null, - values_source_config: {}, }, { default: undefined, @@ -155,9 +146,6 @@ describe("parameters/utils/cards", () => { slug: "d", target: ["variable", ["template-tag", "d"]], type: "date/single", - values_query_type: "list", - values_source_type: null, - values_source_config: {}, }, { default: undefined, @@ -166,9 +154,6 @@ describe("parameters/utils/cards", () => { slug: "e", target: ["dimension", ["template-tag", "e"]], type: "foo", - values_query_type: "list", - values_source_type: null, - values_source_config: {}, }, ]; diff --git a/frontend/src/metabase-types/api/field.ts b/frontend/src/metabase-types/api/field.ts index caeba662f68264e1f4c9de1d70050ac17017102d..d9690b756221e4565e181b3ea2ca890d79c600f7 100644 --- a/frontend/src/metabase-types/api/field.ts +++ b/frontend/src/metabase-types/api/field.ts @@ -44,7 +44,9 @@ export type FieldVisibilityType = | "retired"; type HumanReadableFieldValue = string; -type FieldValue = [RowValue] | [RowValue, HumanReadableFieldValue]; +export type FieldValue = [RowValue] | [RowValue, HumanReadableFieldValue]; + +export type FieldValuesType = "list" | "search" | "none"; export type FieldDimension = { name: string; @@ -74,6 +76,7 @@ export interface ConcreteField { max_value?: number; min_value?: number; + has_field_values: FieldValuesType; caveats?: string | null; points_of_interest?: string; @@ -86,6 +89,12 @@ export interface ConcreteField { updated_at: string; } +export interface FieldValues { + field_id: FieldId; + values: FieldValue[]; + has_more_values: boolean; +} + export type Field = Omit<ConcreteField, "id"> & { id?: FieldId; }; diff --git a/frontend/src/metabase-types/api/mocks/field.ts b/frontend/src/metabase-types/api/mocks/field.ts index 26ae847fe99dc8ff7d2a69d644272d504cf77e9c..4b47d62840b65f25b0b134cb4b623a7beaa515d3 100644 --- a/frontend/src/metabase-types/api/mocks/field.ts +++ b/frontend/src/metabase-types/api/mocks/field.ts @@ -1,4 +1,4 @@ -import { Field } from "metabase-types/api"; +import { Field, FieldValues } from "metabase-types/api"; export const createMockField = (opts?: Partial<Field>): Field => ({ id: 1, @@ -18,8 +18,19 @@ export const createMockField = (opts?: Partial<Field>): Field => ({ position: 1, nfc_path: null, + has_field_values: "list", + last_analyzed: new Date().toISOString(), created_at: new Date().toISOString(), updated_at: new Date().toISOString(), ...opts, }); + +export const createMockFieldValues = ( + opts?: Partial<FieldValues>, +): FieldValues => ({ + field_id: 1, + values: [], + has_more_values: false, + ...opts, +}); diff --git a/frontend/src/metabase-types/api/mocks/parameters.ts b/frontend/src/metabase-types/api/mocks/parameters.ts index ec11c75415ea5e8d39d8a647565e4aef7e4ec027..ce15e59058072abcfbf72d80d0a5bd8eda7b2801 100644 --- a/frontend/src/metabase-types/api/mocks/parameters.ts +++ b/frontend/src/metabase-types/api/mocks/parameters.ts @@ -5,8 +5,5 @@ export const createMockParameter = (opts?: Partial<Parameter>): Parameter => ({ name: "text", type: "string/=", slug: "text", - values_query_type: "none", - values_source_type: null, - values_source_config: {}, ...opts, }); diff --git a/frontend/src/metabase-types/api/parameters.ts b/frontend/src/metabase-types/api/parameters.ts index 9c457b35f48c83125b61e2abed4c5a91f8dd1db8..f109eceac708078d4cba2af44642552c8e920fa5 100644 --- a/frontend/src/metabase-types/api/parameters.ts +++ b/frontend/src/metabase-types/api/parameters.ts @@ -44,9 +44,9 @@ export interface Parameter { filteringParameters?: ParameterId[]; isMultiSelect?: boolean; value?: any; - values_query_type: ValuesQueryType; - values_source_type: ValuesSourceType; - values_source_config: ValuesSourceConfig; + values_query_type?: ValuesQueryType; + values_source_type?: ValuesSourceType; + values_source_config?: ValuesSourceConfig; } export type ValuesQueryType = "list" | "search" | "none"; diff --git a/frontend/src/metabase/components/FieldValuesWidget.jsx b/frontend/src/metabase/components/FieldValuesWidget.jsx index 4cc0d018b99ba98e853f3c54679ca2e398ec4aec..f85d23f78f3bef0e72ea641bbefa141e730fccfd 100644 --- a/frontend/src/metabase/components/FieldValuesWidget.jsx +++ b/frontend/src/metabase/components/FieldValuesWidget.jsx @@ -28,6 +28,12 @@ import { isNumberParameter, isStringParameter, } from "metabase-lib/parameters/utils/parameter-type"; +import { + canListFieldValues, + canListParameterValues, + canSearchFieldValues, + canSearchParameterValues, +} from "metabase-lib/parameters/utils/parameter-source"; const MAX_SEARCH_RESULTS = 100; @@ -66,7 +72,7 @@ async function searchFieldValues( { value, fieldId: field.id, - searchFieldId: searchField(field, disablePKRemappingForSearch).id, + searchFieldId: field.searchField(disablePKRemappingForSearch).id, limit: maxResults, }, { cancelled }, @@ -82,16 +88,18 @@ async function searchFieldValues( class FieldValuesWidgetInner extends Component { constructor(props) { super(props); - const { fields, disableSearch, disablePKRemappingForSearch } = props; + const { parameter, fields, disableSearch, disablePKRemappingForSearch } = + props; this.state = { options: [], loadingState: "INIT", lastValue: "", - valuesMode: getValuesMode( + valuesMode: getValuesMode({ + parameter, fields, disableSearch, disablePKRemappingForSearch, - ), + }), }; } @@ -108,7 +116,9 @@ class FieldValuesWidgetInner extends Component { }; componentDidMount() { - if (shouldList(this.props.fields, this.props.disableSearch)) { + const { parameter, fields, disableSearch } = this.props; + + if (shouldList({ parameter, fields, disableSearch })) { this.fetchValues(); } } @@ -129,13 +139,18 @@ class FieldValuesWidgetInner extends Component { valuesMode = has_more_values ? "search" : valuesMode; } else { options = await this.fetchFieldValues(query); - const { fields, disableSearch, disablePKRemappingForSearch } = - this.props; - valuesMode = getValuesMode( + const { + parameter, fields, disableSearch, disablePKRemappingForSearch, - ); + } = this.props; + valuesMode = getValuesMode({ + parameter, + fields, + disableSearch, + disablePKRemappingForSearch, + }); } } finally { this.updateRemappings(options); @@ -193,7 +208,7 @@ class FieldValuesWidgetInner extends Component { const [field] = fields; if ( field.remappedField() === - searchField(field, this.props.disablePKRemappingForSearch) + field.searchField(this.props.disablePKRemappingForSearch) ) { this.props.addRemappings(field.id, options); } @@ -307,11 +322,16 @@ class FieldValuesWidgetInner extends Component { const isListMode = !disableList && - shouldList(fields, disableSearch) && + shouldList({ parameter, fields, disableSearch }) && valuesMode === "list" && !forceTokenField; const isLoading = loadingState === "LOADING"; - const hasListValues = hasList({ fields, disableSearch, options }); + const hasListValues = hasList({ + parameter, + fields, + disableSearch, + options, + }); const parseFreeformValue = value => { return isNumeric(fields[0], parameter) @@ -442,16 +462,14 @@ function showRemapping(fields) { return fields.length === 1; } -function shouldList(fields, disableSearch) { - // Virtual fields come from questions that are based on other questions. - // Currently, the back end does not return `has_field_values` in their metadata, - // so we ignore them for now. - const nonVirtualFields = fields.filter(field => typeof field.id === "number"); - - return ( - !disableSearch && - nonVirtualFields.every(field => field.has_field_values === "list") - ); +function shouldList({ parameter, fields, disableSearch }) { + if (disableSearch) { + return false; + } else { + return parameter + ? canListParameterValues(parameter) + : canListFieldValues(fields); + } } function getNonSearchableTokenFieldPlaceholder(firstField, parameter) { @@ -481,15 +499,7 @@ function getNonSearchableTokenFieldPlaceholder(firstField, parameter) { } export function searchField(field, disablePKRemappingForSearch) { - if (disablePKRemappingForSearch && field.isPK()) { - return field.isSearchable() ? field : null; - } - - const remappedField = field.remappedField(); - if (remappedField && remappedField.isSearchable()) { - return remappedField; - } - return field.isSearchable() ? field : null; + return field.searchField(disablePKRemappingForSearch); } function getSearchableTokenFieldPlaceholder( @@ -501,7 +511,7 @@ function getSearchableTokenFieldPlaceholder( const names = new Set( fields.map(field => - stripId(searchField(field, disablePKRemappingForSearch).display_name), + stripId(field.searchField(disablePKRemappingForSearch).display_name), ), ); @@ -513,7 +523,7 @@ function getSearchableTokenFieldPlaceholder( placeholder = t`Search by ${name}`; if ( firstField.isID() && - firstField !== searchField(firstField, disablePKRemappingForSearch) + firstField !== firstField.searchField(disablePKRemappingForSearch) ) { placeholder += t` or enter an ID`; } @@ -521,8 +531,10 @@ function getSearchableTokenFieldPlaceholder( return placeholder; } -function hasList({ fields, disableSearch, options }) { - return shouldList(fields, disableSearch) && !_.isEmpty(options); +function hasList({ parameter, fields, disableSearch, options }) { + return ( + shouldList({ parameter, fields, disableSearch }) && !_.isEmpty(options) + ); } // if this search is just an extension of the previous search, and the previous search @@ -537,30 +549,21 @@ function isExtensionOfPreviousSearch(value, lastValue, options, maxResults) { } export function isSearchable({ + parameter, fields, disableSearch, disablePKRemappingForSearch, valuesMode, }) { - function everyFieldIsSearchable() { - return fields.every(field => - searchField(field, disablePKRemappingForSearch), - ); - } - - function someFieldIsConfiguredForSearch() { - return fields.some( - f => - f.has_field_values === "search" || - (f.has_field_values === "list" && f.has_more_values === true), - ); + if (disableSearch) { + return false; + } else if (valuesMode === "search") { + return true; + } else if (parameter) { + return canSearchParameterValues(parameter, disablePKRemappingForSearch); + } else { + return canSearchFieldValues(fields, disablePKRemappingForSearch); } - - return ( - !disableSearch && - (valuesMode === "search" || - (everyFieldIsSearchable() && someFieldIsConfiguredForSearch())) - ); } function getTokenFieldPlaceholder({ @@ -580,6 +583,7 @@ function getTokenFieldPlaceholder({ if ( hasList({ + parameter, fields, disableSearch, options, @@ -588,6 +592,7 @@ function getTokenFieldPlaceholder({ return t`Search the list`; } else if ( isSearchable({ + parameter, fields, disableSearch, disablePKRemappingForSearch, @@ -611,6 +616,7 @@ function renderOptions( ) { const { alwaysShowOptions, + parameter, fields, disableSearch, disablePKRemappingForSearch, @@ -622,6 +628,7 @@ function renderOptions( return optionsList; } else if ( hasList({ + parameter, fields, disableSearch, options, @@ -633,6 +640,7 @@ function renderOptions( } } else if ( isSearchable({ + parameter, fields, disableSearch, disablePKRemappingForSearch, @@ -645,7 +653,7 @@ function renderOptions( return ( <NoMatchState fields={fields.map(field => - searchField(field, disablePKRemappingForSearch), + field.searchField(disablePKRemappingForSearch), )} /> ); @@ -667,17 +675,15 @@ function renderValue(fields, formatOptions, value, options) { ); } -export function getValuesMode( +export function getValuesMode({ + parameter, fields, disableSearch, disablePKRemappingForSearch, -) { - if (fields.length === 0) { - return "none"; - } - +}) { if ( isSearchable({ + parameter, fields, disableSearch, disablePKRemappingForSearch, @@ -687,7 +693,7 @@ export function getValuesMode( return "search"; } - if (shouldList(fields, disableSearch)) { + if (shouldList({ parameter, fields, disableSearch })) { return "list"; } diff --git a/frontend/src/metabase/entities/collections/constants.ts b/frontend/src/metabase/entities/collections/constants.ts index 4cac07c895c5ebfe8fdebe43d9b841f1d07e1414..31d0d07829e4654e982927dc8e27383bc9639ddd 100644 --- a/frontend/src/metabase/entities/collections/constants.ts +++ b/frontend/src/metabase/entities/collections/constants.ts @@ -3,7 +3,7 @@ import { t } from "ttag"; export const DEFAULT_COLLECTION_COLOR_ALIAS = "brand"; export const ROOT_COLLECTION = { - id: "root", + id: "root" as const, name: t`Our analytics`, location: "", path: [], diff --git a/frontend/src/metabase/entities/containers/EntityObjectLoader.jsx b/frontend/src/metabase/entities/containers/EntityObjectLoader.jsx index afbcf19c1751eb2cc4c26cbe04d5a2bdfef30cd7..70ea3456de16f3ffd6e850ddb48ac814e0a75f55 100644 --- a/frontend/src/metabase/entities/containers/EntityObjectLoader.jsx +++ b/frontend/src/metabase/entities/containers/EntityObjectLoader.jsx @@ -58,7 +58,8 @@ class EntityObjectLoaderInner extends React.Component { fetch = (query, options) => { const fetch = this.props[this.props.requestType]; - return fetch(query, options); + // errors are handled in redux actions + return fetch(query, options).catch(() => {}); }; UNSAFE_componentWillMount() { diff --git a/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.unit.spec.tsx b/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.unit.spec.tsx new file mode 100644 index 0000000000000000000000000000000000000000..6f35bb4898813d5644ab5d36fc26941c56691978 --- /dev/null +++ b/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.unit.spec.tsx @@ -0,0 +1,57 @@ +import React from "react"; +import userEvent from "@testing-library/user-event"; +import { renderWithProviders, screen } from "__support__/ui"; +import { UiParameter } from "metabase-lib/parameters/types"; +import { createMockUiParameter } from "metabase-lib/parameters/mock"; +import ParameterSettings from "../ParameterSettings"; + +interface SetupOpts { + parameter?: UiParameter; +} + +describe("ParameterSidebar", () => { + it("should allow to change source settings for string parameters", () => { + const { onChangeQueryType } = setup({ + parameter: createMockUiParameter({ + type: "string/=", + sectionId: "string", + }), + }); + + userEvent.click(screen.getByRole("radio", { name: "Search box" })); + + expect(onChangeQueryType).toHaveBeenCalledWith("search"); + }); + + it("should allow to change source settings for location parameters", () => { + const { onChangeQueryType } = setup({ + parameter: createMockUiParameter({ + type: "string/=", + sectionId: "location", + }), + }); + + userEvent.click(screen.getByRole("radio", { name: "Input box" })); + + expect(onChangeQueryType).toHaveBeenCalledWith("none"); + }); +}); + +const setup = ({ parameter = createMockUiParameter() }: SetupOpts = {}) => { + const onChangeQueryType = jest.fn(); + + renderWithProviders( + <ParameterSettings + parameter={parameter} + onChangeName={jest.fn()} + onChangeDefaultValue={jest.fn()} + onChangeIsMultiSelect={jest.fn()} + onChangeQueryType={onChangeQueryType} + onChangeSourceType={jest.fn()} + onChangeSourceConfig={jest.fn()} + onRemoveParameter={jest.fn()} + />, + ); + + return { onChangeQueryType }; +}; diff --git a/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceCardModal.tsx b/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceCardModal.tsx index 3c4b7e493daec63b789f8364fac15f4595e11d91..8c0a00ed75723e3f14bea8fe195a44b1a08a53e0 100644 --- a/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceCardModal.tsx +++ b/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceCardModal.tsx @@ -28,6 +28,7 @@ import { getQuestionIdFromVirtualTableId, getQuestionVirtualTableId, } from "metabase-lib/metadata/utils/saved-questions"; +import { ModalLoadingAndErrorWrapper } from "./ValuesSourceModal.styled"; import { DataPickerContainer, ModalBodyWithSearch, @@ -195,12 +196,12 @@ export default _.compose( Questions.load({ id: (state: State, { sourceConfig: { card_id } }: ModalOwnProps) => card_id, entityAlias: "card", - loadingAndErrorWrapper: false, + LoadingAndErrorWrapper: ModalLoadingAndErrorWrapper, }), Collections.load({ id: (state: State, { card }: ModalCardProps) => card?.collection_id ?? "root", - loadingAndErrorWrapper: false, + LoadingAndErrorWrapper: ModalLoadingAndErrorWrapper, }), connect(mapStateToProps, mapDispatchToProps), )(ValuesSourceCardModal); diff --git a/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.styled.tsx b/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.styled.tsx index 094a8c3c2aacb80b161579e9a04aa8740c385340..67c96e9a95e4963b7cf93115a2100527954d4ff0 100644 --- a/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.styled.tsx +++ b/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.styled.tsx @@ -1,5 +1,13 @@ import styled from "@emotion/styled"; +import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper"; export const ModalBody = styled.div` height: 50vh; `; + +export const ModalLoadingAndErrorWrapper = styled(LoadingAndErrorWrapper)` + display: flex; + justify-content: center; + align-items: center; + height: calc(50vh + 11.25rem); +`; diff --git a/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.tsx b/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.tsx index db95cb5c247559279b24bc3abc11daae9c9434f2..789201aac25444395ebe6651ccda25eecc8fd7e3 100644 --- a/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.tsx +++ b/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.tsx @@ -1,6 +1,11 @@ import React, { useCallback, useMemo, useState } from "react"; import { ValuesSourceConfig, ValuesSourceType } from "metabase-types/api"; import { getNonVirtualFields } from "metabase-lib/parameters/utils/parameter-fields"; +import { + getSourceConfig, + getSourceConfigForType, + getSourceType, +} from "metabase-lib/parameters/utils/parameter-source"; import { UiParameter } from "metabase-lib/parameters/types"; import ValuesSourceTypeModal from "./ValuesSourceTypeModal"; import ValuesSourceCardModal from "./ValuesSourceCardModal"; @@ -22,10 +27,8 @@ const ValuesSourceModal = ({ onClose, }: ModalProps): JSX.Element => { const [step, setStep] = useState<ModalStep>("main"); - const [sourceType, setSourceType] = useState(parameter.values_source_type); - const [sourceConfig, setSourceConfig] = useState( - parameter.values_source_config, - ); + const [sourceType, setSourceType] = useState(getSourceType(parameter)); + const [sourceConfig, setSourceConfig] = useState(getSourceConfig(parameter)); const fields = useMemo(() => { return getNonVirtualFields(parameter); @@ -40,7 +43,7 @@ const ValuesSourceModal = ({ }, []); const handleSubmit = useCallback(() => { - onSubmit(sourceType, sourceConfig); + onSubmit(sourceType, getSourceConfigForType(sourceType, sourceConfig)); onClose(); }, [sourceType, sourceConfig, onSubmit, onClose]); diff --git a/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.unit.spec.tsx b/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.unit.spec.tsx new file mode 100644 index 0000000000000000000000000000000000000000..5d27c7cbd20620dcad90d5556f348c02337d3399 --- /dev/null +++ b/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.unit.spec.tsx @@ -0,0 +1,239 @@ +import React from "react"; +import nock from "nock"; +import userEvent from "@testing-library/user-event"; +import { ROOT_COLLECTION } from "metabase/entities/collections"; +import { Card, FieldValues } from "metabase-types/api"; +import { + createMockCard, + createMockCollection, + createMockField, + createMockFieldValues, +} from "metabase-types/api/mocks"; +import { + setupCardsEndpoints, + setupCollectionsEndpoints, + setupFieldsValuesEndpoints, + setupUnauthorizedCardsEndpoints, +} from "__support__/server-mocks"; +import { renderWithProviders, screen, waitFor } from "__support__/ui"; +import Field from "metabase-lib/metadata/Field"; +import { UiParameter } from "metabase-lib/parameters/types"; +import { createMockUiParameter } from "metabase-lib/parameters/mock"; +import ValuesSourceModal from "./ValuesSourceModal"; + +describe("ValuesSourceModal", () => { + afterEach(() => { + nock.cleanAll(); + }); + + describe("fields source", () => { + it("should show a message about not connected fields", () => { + setup(); + + expect( + screen.getByText(/You haven’t connected a field to this filter/), + ).toBeInTheDocument(); + }); + + it("should show a message about missing field values", async () => { + setup({ + parameter: createMockUiParameter({ + fields: [new Field(createMockField({ id: 1 }))], + }), + fieldsValues: [ + createMockFieldValues({ + field_id: 1, + values: [], + }), + ], + }); + + expect( + await screen.findByText(/We don’t have any cached values/), + ).toBeInTheDocument(); + }); + + it("should show unique non-null mapped fields values", async () => { + setup({ + parameter: createMockUiParameter({ + fields: [ + new Field(createMockField({ id: 1 })), + new Field(createMockField({ id: 2 })), + ], + }), + fieldsValues: [ + createMockFieldValues({ + field_id: 1, + values: [["A"], [null], ["B"], ["A"]], + }), + createMockFieldValues({ + field_id: 2, + values: [["B", "Remapped"], ["C"]], + }), + ], + }); + + await waitFor(() => { + expect(screen.getByRole("textbox")).toHaveValue("A\nB\nC"); + }); + }); + }); + + describe("card source", () => { + it("should show a message when there are no text columns", async () => { + setup({ + parameter: createMockUiParameter({ + values_source_type: "card", + values_source_config: { + card_id: 1, + }, + }), + cards: [ + createMockCard({ + id: 1, + name: "Products", + }), + ], + }); + + expect( + await screen.findByText(/This question doesn’t have any text columns/), + ).toBeInTheDocument(); + }); + + it("should allow to select only text fields", async () => { + const { onSubmit } = setup({ + parameter: createMockUiParameter({ + values_source_type: "card", + values_source_config: { + card_id: 1, + }, + }), + cards: [ + createMockCard({ + id: 1, + name: "Products", + result_metadata: [ + createMockField({ + id: 1, + display_name: "ID", + base_type: "type/BigInteger", + semantic_type: "type/PK", + }), + createMockField({ + id: 2, + display_name: "Category", + base_type: "type/Text", + semantic_type: "type/Category", + }), + ], + }), + ], + }); + + userEvent.click( + await screen.findByRole("button", { name: /Pick a column/ }), + ); + expect( + screen.queryByRole("heading", { name: "ID" }), + ).not.toBeInTheDocument(); + + userEvent.click(screen.getByRole("heading", { name: "Category" })); + userEvent.click(screen.getByRole("button", { name: "Done" })); + expect(onSubmit).toHaveBeenCalledWith("card", { + card_id: 1, + value_field: ["field", 2, null], + }); + }); + + it("should display an error message when the user has no access to the card", async () => { + setup({ + parameter: createMockUiParameter({ + values_source_type: "card", + values_source_config: { + card_id: 1, + }, + }), + cards: [ + createMockCard({ + id: 1, + name: "Products", + }), + ], + hasDataAccess: false, + }); + + expect( + await screen.findByText("You don't have permissions to do that."), + ).toBeInTheDocument(); + }); + }); + + describe("list source", () => { + it("should set static list values", () => { + const { onSubmit } = setup(); + + userEvent.click(screen.getByRole("radio", { name: "Custom list" })); + userEvent.type(screen.getByRole("textbox"), "Gadget\nWidget"); + userEvent.click(screen.getByRole("button", { name: "Done" })); + + expect(onSubmit).toHaveBeenCalledWith("static-list", { + values: ["Gadget", "Widget"], + }); + }); + + it("should preserve the list when changing the source type", () => { + setup({ + parameter: createMockUiParameter({ + values_source_type: "static-list", + values_source_config: { + values: ["Gadget", "Widget"], + }, + }), + }); + + userEvent.click( + screen.getByRole("radio", { name: "From connected fields" }), + ); + userEvent.click(screen.getByRole("radio", { name: "Custom list" })); + + expect(screen.getByRole("textbox")).toHaveValue("Gadget\nWidget"); + }); + }); +}); + +interface SetupOpts { + parameter?: UiParameter; + cards?: Card[]; + fieldsValues?: FieldValues[]; + hasDataAccess?: boolean; +} + +const setup = ({ + parameter = createMockUiParameter(), + cards = [], + fieldsValues = [], + hasDataAccess = true, +}: SetupOpts = {}) => { + const scope = nock(location.origin); + const onSubmit = jest.fn(); + const onClose = jest.fn(); + + if (hasDataAccess) { + setupCollectionsEndpoints(scope, [createMockCollection(ROOT_COLLECTION)]); + setupCardsEndpoints(scope, cards); + setupFieldsValuesEndpoints(scope, fieldsValues); + } else { + setupUnauthorizedCardsEndpoints(scope, cards); + } + + renderWithProviders( + <ValuesSourceModal + parameter={parameter} + onSubmit={onSubmit} + onClose={onClose} + />, + ); + + return { onSubmit }; +}; diff --git a/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceTypeModal.tsx b/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceTypeModal.tsx index 0b5bfb0029afd67d85f514f4caef9271d7ef1e64..9f42e777d32ec9aa4307ccd554391db735a8670d 100644 --- a/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceTypeModal.tsx +++ b/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceTypeModal.tsx @@ -11,21 +11,27 @@ import Select, { import SelectButton from "metabase/core/components/SelectButton"; import ModalContent from "metabase/components/ModalContent"; import QuestionResultLoader from "metabase/containers/QuestionResultLoader"; -import Collections from "metabase/entities/collections"; import Fields from "metabase/entities/fields"; import Tables from "metabase/entities/tables"; import Questions from "metabase/entities/questions"; import { getMetadata } from "metabase/selectors/metadata"; -import { Card, ValuesSourceConfig, ValuesSourceType } from "metabase-types/api"; +import { + Dataset, + Card, + ValuesSourceConfig, + ValuesSourceType, + FieldValue, +} from "metabase-types/api"; import { Dispatch, State } from "metabase-types/store"; -import { Dataset } from "metabase-types/types/Dataset"; import Question from "metabase-lib/Question"; import Field from "metabase-lib/metadata/Field"; import { getQuestionVirtualTableId } from "metabase-lib/metadata/utils/saved-questions"; import { - getDefaultSourceConfig, + getCardSourceValues, + getFieldSourceValues, isValidSourceConfig, } from "metabase-lib/parameters/utils/parameter-source"; +import { ModalLoadingAndErrorWrapper } from "./ValuesSourceModal.styled"; import { ModalHelpMessage, ModalLabel, @@ -64,7 +70,7 @@ interface ModalCardProps { interface ModalStateProps { question: Question | undefined; - fieldValues: string[][][]; + fieldsValues: FieldValue[][]; isLoadingFieldValues: boolean; } @@ -84,7 +90,7 @@ type ModalProps = ModalOwnProps & const ValuesSourceTypeModal = ({ name, fields, - fieldValues, + fieldsValues, isLoadingFieldValues, question, sourceType, @@ -96,18 +102,6 @@ const ValuesSourceTypeModal = ({ onSubmit, onClose, }: ModalProps): JSX.Element => { - const allFieldValues = useMemo(() => { - return getFieldValues(fieldValues); - }, [fieldValues]); - - const handleTypeChange = useCallback( - (sourceType: ValuesSourceType) => { - onChangeSourceType(sourceType); - onChangeSourceConfig(getDefaultSourceConfig(sourceType, allFieldValues)); - }, - [allFieldValues, onChangeSourceType, onChangeSourceConfig], - ); - useEffect(() => { onFetchFieldValues(fields); }, [fields, onFetchFieldValues]); @@ -128,10 +122,10 @@ const ValuesSourceTypeModal = ({ {sourceType === null ? ( <FieldSourceModal fields={fields} - fieldValues={allFieldValues} + fieldsValues={fieldsValues} isLoadingFieldValues={isLoadingFieldValues} sourceType={sourceType} - onChangeSourceType={handleTypeChange} + onChangeSourceType={onChangeSourceType} /> ) : sourceType === "card" ? ( <CardSourceModal @@ -139,15 +133,14 @@ const ValuesSourceTypeModal = ({ sourceType={sourceType} sourceConfig={sourceConfig} onChangeCard={onChangeCard} - onChangeSourceType={handleTypeChange} + onChangeSourceType={onChangeSourceType} onChangeSourceConfig={onChangeSourceConfig} /> ) : sourceType === "static-list" ? ( <ListSourceModal sourceType={sourceType} sourceConfig={sourceConfig} - fieldValues={allFieldValues} - onChangeSourceType={handleTypeChange} + onChangeSourceType={onChangeSourceType} onChangeSourceConfig={onChangeSourceConfig} /> ) : null} @@ -157,7 +150,7 @@ const ValuesSourceTypeModal = ({ interface FieldSourceModalProps { fields: Field[]; - fieldValues: string[]; + fieldsValues: FieldValue[][]; isLoadingFieldValues: boolean; sourceType: ValuesSourceType; onChangeSourceType: (sourceType: ValuesSourceType) => void; @@ -165,18 +158,22 @@ interface FieldSourceModalProps { const FieldSourceModal = ({ fields, - fieldValues, + fieldsValues, isLoadingFieldValues, sourceType, onChangeSourceType, }: FieldSourceModalProps) => { - const hasFields = fields.length > 0; - const hasFieldValues = fieldValues.length > 0; + const fieldValues = useMemo(() => { + return getFieldSourceValues(fieldsValues); + }, [fieldsValues]); const fieldValuesText = useMemo(() => { return getValuesText(fieldValues); }, [fieldValues]); + const hasFields = fields.length > 0; + const hasFieldValues = fieldValues.length > 0; + return ( <ModalBodyWithPane> <ModalPane> @@ -301,7 +298,7 @@ const CardSourceModal = ({ <QuestionResultLoader question={fieldValuesQuestion}> {({ result }: QuestionLoaderProps) => ( <ModalTextArea - value={getValuesText(getDatasetValues(result))} + value={result ? getValuesText(getCardSourceValues(result)) : ""} readOnly fullWidth /> @@ -316,7 +313,6 @@ const CardSourceModal = ({ interface ListSourceModalProps { sourceType: ValuesSourceType; sourceConfig: ValuesSourceConfig; - fieldValues: string[]; onChangeSourceType: (sourceType: ValuesSourceType) => void; onChangeSourceConfig: (sourceConfig: ValuesSourceConfig) => void; } @@ -363,20 +359,6 @@ const getValuesText = (values?: string[]) => { return values?.join(NEW_LINE) ?? ""; }; -const getUniqueValues = (values: string[]) => { - return Array.from(new Set(values)); -}; - -const getFieldValues = (fieldsValues: string[][][]) => { - const allValues = fieldsValues.flatMap(values => values.map(([key]) => key)); - return getUniqueValues(allValues); -}; - -const getDatasetValues = (dataset?: Dataset) => { - const allValues = dataset?.data.rows.map(([value]) => String(value)) ?? []; - return getUniqueValues(allValues); -}; - const getStaticValues = (value: string) => { return value .split(NEW_LINE) @@ -398,7 +380,7 @@ const mapStateToProps = ( { card, fields }: ModalOwnProps & ModalCardProps, ): ModalStateProps => ({ question: card ? new Question(card, getMetadata(state)) : undefined, - fieldValues: fields.map(field => + fieldsValues: fields.map(field => Fields.selectors.getFieldValues(state, { entityId: field.id }), ), isLoadingFieldValues: fields.every(field => @@ -422,17 +404,12 @@ export default _.compose( id: (state: State, { sourceConfig: { card_id } }: ModalOwnProps) => card_id ? getQuestionVirtualTableId(card_id) : undefined, requestType: "fetchMetadata", - loadingAndErrorWrapper: false, + LoadingAndErrorWrapper: ModalLoadingAndErrorWrapper, }), Questions.load({ id: (state: State, { sourceConfig: { card_id } }: ModalOwnProps) => card_id, entityAlias: "card", - loadingAndErrorWrapper: false, - }), - Collections.load({ - id: (state: State, { card }: ModalCardProps) => - card?.collection_id ?? "root", - loadingAndErrorWrapper: false, + LoadingAndErrorWrapper: ModalLoadingAndErrorWrapper, }), connect(mapStateToProps, mapDispatchToProps), )(ValuesSourceTypeModal); diff --git a/frontend/src/metabase/parameters/components/ValuesSourceSettings/ValuesSourceSettings.tsx b/frontend/src/metabase/parameters/components/ValuesSourceSettings/ValuesSourceSettings.tsx index 22b1a8d8afe309a6490ec8e775eb56e5564db176..9d9a1cbcd2385cdcc8ad6b579cbf04bf88f3e954 100644 --- a/frontend/src/metabase/parameters/components/ValuesSourceSettings/ValuesSourceSettings.tsx +++ b/frontend/src/metabase/parameters/components/ValuesSourceSettings/ValuesSourceSettings.tsx @@ -7,6 +7,7 @@ import { ValuesSourceConfig, ValuesSourceType, } from "metabase-types/api"; +import { getQueryType } from "metabase-lib/parameters/utils/parameter-source"; import { UiParameter } from "metabase-lib/parameters/types"; import ValuesSourceModal from "../ValuesSourceModal"; import { @@ -28,7 +29,7 @@ const ValuesSourceSettings = ({ onChangeSourceType, onChangeSourceConfig, }: ValuesSourceSettingsProps): JSX.Element => { - const queryType = parameter.values_query_type; + const queryType = getQueryType(parameter); const [isModalOpened, setIsModalOpened] = useState(false); const radioOptions = useMemo(() => { diff --git a/frontend/src/metabase/parameters/utils/dashboards.ts b/frontend/src/metabase/parameters/utils/dashboards.ts index 3f798412186870a17f5ac6912cc1b3fd75c582aa..ccfc37556fc1888604b0813d3510390da3fb7353 100644 --- a/frontend/src/metabase/parameters/utils/dashboards.ts +++ b/frontend/src/metabase/parameters/utils/dashboards.ts @@ -45,9 +45,6 @@ export function createParameter( id: generateParameterId(), type: option.type, sectionId: option.sectionId, - values_query_type: "list", - values_source_type: null, - values_source_config: {}, }; return setParameterName(parameter, name); diff --git a/frontend/src/metabase/parameters/utils/dashboards.unit.spec.js b/frontend/src/metabase/parameters/utils/dashboards.unit.spec.js index 93fc4360249b6c102c89789f50e03a74cd119089..4728c636bc48c3e493a921e4fcb8a42b0e247ce9 100644 --- a/frontend/src/metabase/parameters/utils/dashboards.unit.spec.js +++ b/frontend/src/metabase/parameters/utils/dashboards.unit.spec.js @@ -30,9 +30,6 @@ describe("metabase/parameters/utils/dashboards", () => { sectionId: "abc", slug: "foo_bar", type: "category", - values_query_type: "list", - values_source_type: null, - values_source_config: {}, }); }); @@ -53,9 +50,6 @@ describe("metabase/parameters/utils/dashboards", () => { sectionId: "abc", slug: "foo_bar_baz", type: "category", - values_query_type: "list", - values_source_type: null, - values_source_config: {}, }); }); @@ -84,9 +78,6 @@ describe("metabase/parameters/utils/dashboards", () => { sectionId: "abc", slug: "foo_bar_1", type: "category", - values_query_type: "list", - values_source_type: null, - values_source_config: {}, }); }); }); diff --git a/frontend/test/__support__/server-mocks/card.ts b/frontend/test/__support__/server-mocks/card.ts index d38e093e042ce4d9bedd042d1770799a0b22edc6..7e40e4b2b2f55a2fd5b306fa2bb5199469e60f37 100644 --- a/frontend/test/__support__/server-mocks/card.ts +++ b/frontend/test/__support__/server-mocks/card.ts @@ -5,6 +5,7 @@ import { getQuestionVirtualTableId, convertSavedQuestionToVirtualTable, } from "metabase-lib/metadata/utils/saved-questions"; +import { PERMISSION_ERROR } from "./constants"; export function setupCardEndpoints(scope: Scope, card: Card) { scope.get(`/api/card/${card.id}`).reply(200, card); @@ -12,20 +13,31 @@ export function setupCardEndpoints(scope: Scope, card: Card) { .put(`/api/card/${card.id}`) .reply(200, (uri, body) => createMockCard(body as Card)); - if (card.dataset) { - const virtualTableId = getQuestionVirtualTableId(card.id); - scope.get(`/api/table/${virtualTableId}/query_metadata`).reply(200, { - ...convertSavedQuestionToVirtualTable(card), - dimension_options: {}, - fields: card.result_metadata.map(field => ({ - ...field, - table_id: virtualTableId, - })), - }); - } + const virtualTableId = getQuestionVirtualTableId(card.id); + scope.get(`/api/table/${virtualTableId}/query_metadata`).reply(200, { + ...convertSavedQuestionToVirtualTable(card), + fields: card.result_metadata.map(field => ({ + ...field, + table_id: virtualTableId, + })), + dimension_options: {}, + }); } export function setupCardsEndpoints(scope: Scope, cards: Card[]) { scope.get("/api/card").reply(200, cards); cards.forEach(card => setupCardEndpoints(scope, card)); } + +export function setupUnauthorizedCardEndpoints(scope: Scope, card: Card) { + scope.get(`/api/card/${card.id}`).reply(403, PERMISSION_ERROR); + + const virtualTableId = getQuestionVirtualTableId(card.id); + scope + .get(`/api/table/${virtualTableId}/query_metadata`) + .reply(403, PERMISSION_ERROR); +} + +export function setupUnauthorizedCardsEndpoints(scope: Scope, cards: Card[]) { + cards.forEach(card => setupUnauthorizedCardEndpoints(scope, card)); +} diff --git a/frontend/test/__support__/server-mocks/constants.ts b/frontend/test/__support__/server-mocks/constants.ts new file mode 100644 index 0000000000000000000000000000000000000000..c9b4f9c3f00de9e4a057a3724b9c141146fb07b3 --- /dev/null +++ b/frontend/test/__support__/server-mocks/constants.ts @@ -0,0 +1 @@ +export const PERMISSION_ERROR = "You don't have permissions to do that."; diff --git a/frontend/test/__support__/server-mocks/database.ts b/frontend/test/__support__/server-mocks/database.ts index 5d65d5d7d28cbb21cd6f7dd43701611b034c251b..458e7ec78ed5a0d28891bfabe8c0ba0dc1915c92 100644 --- a/frontend/test/__support__/server-mocks/database.ts +++ b/frontend/test/__support__/server-mocks/database.ts @@ -1,10 +1,20 @@ import { Scope } from "nock"; import _ from "underscore"; import { Database } from "metabase-types/api"; +import { setupTableEndpoints } from "./table"; export function setupDatabaseEndpoints(scope: Scope, db: Database) { scope.get(`/api/database/${db.id}`).reply(200, db); + setupSchemaEndpoints(scope, db); + db.tables?.forEach(table => setupTableEndpoints(scope, table)); +} + +export function setupDatabasesEndpoints(scope: Scope, dbs: Database[]) { + scope.get("/api/database").reply(200, dbs); + dbs.forEach(db => setupDatabaseEndpoints(scope, db)); +} +export const setupSchemaEndpoints = (scope: Scope, db: Database) => { const schemas = _.groupBy(db.tables ?? [], table => table.schema); const schemaNames = Object.keys(schemas); scope.get(`/api/database/${db.id}/schemas`).reply(200, schemaNames); @@ -14,9 +24,4 @@ export function setupDatabaseEndpoints(scope: Scope, db: Database) { .get(`/api/database/${db.id}/schema/${schema}`) .reply(200, schemas[schema]); }); -} - -export function setupDatabasesEndpoints(scope: Scope, dbs: Database[]) { - scope.get("/api/database").reply(200, dbs); - dbs.forEach(db => setupDatabaseEndpoints(scope, db)); -} +}; diff --git a/frontend/test/__support__/server-mocks/field.ts b/frontend/test/__support__/server-mocks/field.ts index 2062fb5e47c50976448549ba9aaa1a7b7d1feb1e..f380fe265a7d123bbf24977479be501bcec586b5 100644 --- a/frontend/test/__support__/server-mocks/field.ts +++ b/frontend/test/__support__/server-mocks/field.ts @@ -1,6 +1,24 @@ import { Scope } from "nock"; -import { Field } from "metabase-types/api"; +import { Field, FieldValues } from "metabase-types/api"; export function setupFieldEndpoints(scope: Scope, field: Field) { scope.get(`/api/field/${field.id}`).reply(200, field); } + +export function setupFieldValuesEndpoints( + scope: Scope, + fieldValues: FieldValues, +) { + scope + .get(`/api/field/${fieldValues.field_id}/values`) + .reply(200, fieldValues); +} + +export function setupFieldsValuesEndpoints( + scope: Scope, + fieldsValues: FieldValues[], +) { + fieldsValues.forEach(fieldValues => { + setupFieldValuesEndpoints(scope, fieldValues); + }); +} diff --git a/frontend/test/__support__/server-mocks/table.ts b/frontend/test/__support__/server-mocks/table.ts index bd96a20e7f4463985e6b02db009f5bbaf5512e3e..67e5a4bbcedd39b8f80ddc505d8199fe4515f466 100644 --- a/frontend/test/__support__/server-mocks/table.ts +++ b/frontend/test/__support__/server-mocks/table.ts @@ -2,7 +2,7 @@ import { Scope } from "nock"; import { Table } from "metabase-types/api"; import { setupFieldEndpoints } from "./field"; -export function setupTableEndpoint(scope: Scope, table: Table) { +export function setupTableEndpoints(scope: Scope, table: Table) { scope.get(`/api/table/${table.id}`).reply(200, table); scope.get(`/api/table/${table.id}/query_metadata`).reply(200, table); scope.get(`/api/table/${table.id}/fks`).reply(200, []); @@ -11,5 +11,5 @@ export function setupTableEndpoint(scope: Scope, table: Table) { export function setupTablesEndpoints(scope: Scope, tables: Table[]) { scope.get("/api/table").reply(200, tables); - tables.forEach(table => setupTableEndpoint(scope, table)); + tables.forEach(table => setupTableEndpoints(scope, table)); } diff --git a/frontend/test/metabase-lib/lib/Question.unit.spec.js b/frontend/test/metabase-lib/lib/Question.unit.spec.js index 79f404547ed2638d213f52ad2f49e12c98adf34c..c216192e5f870da39725e73b66dc73f658a0472a 100644 --- a/frontend/test/metabase-lib/lib/Question.unit.spec.js +++ b/frontend/test/metabase-lib/lib/Question.unit.spec.js @@ -1130,9 +1130,6 @@ describe("Question", () => { slug: "foo", target: ["dimension", ["template-tag", "foo"]], type: "category", - values_query_type: "list", - values_source_type: null, - values_source_config: {}, }, { default: undefined, @@ -1142,9 +1139,6 @@ describe("Question", () => { slug: "bar", target: ["variable", ["template-tag", "bar"]], type: "category", - values_query_type: "list", - values_source_type: null, - values_source_config: {}, }, ]); }); diff --git a/frontend/test/metabase/components/FieldValuesWidget.unit.spec.js b/frontend/test/metabase/components/FieldValuesWidget.unit.spec.js index b266639c94a958347a90e66457c34255871b029e..c1caac2c4c932b554eedf58cfcb7670088eb7028 100644 --- a/frontend/test/metabase/components/FieldValuesWidget.unit.spec.js +++ b/frontend/test/metabase/components/FieldValuesWidget.unit.spec.js @@ -314,7 +314,7 @@ describe("FieldValuesWidget", () => { describe("getValuesMode", () => { describe("when passed no fields", () => { it("should return 'none'", () => { - expect(getValuesMode([])).toBe("none"); + expect(getValuesMode({ fields: [] })).toBe("none"); }); }); @@ -323,21 +323,21 @@ describe("FieldValuesWidget", () => { const fields = [ mock(PRODUCTS.CATEGORY, { has_field_values: "search" }), ]; - expect(getValuesMode(fields)).toBe("search"); + expect(getValuesMode({ fields })).toBe("search"); }); }); describe("when passed fields that are not searchable but listable", () => { it("should return 'list'", () => { const fields = [mock(PRODUCTS.CATEGORY, { has_field_values: "list" })]; - expect(getValuesMode(fields)).toBe("list"); + expect(getValuesMode({ fields })).toBe("list"); }); }); describe("when passed fields that are not searchable and not listable", () => { it("should return 'none'", () => { const fields = [mock(PRODUCTS.CATEGORY, { has_field_values: "none" })]; - expect(getValuesMode(fields)).toBe("none"); + expect(getValuesMode({ fields })).toBe("none"); }); }); }); diff --git a/frontend/test/metabase/scenarios/dashboard-filters/dashboard-filters-source.cy.spec.js b/frontend/test/metabase/scenarios/dashboard-filters/dashboard-filters-source.cy.spec.js index ed5c51229a5a6789d89e9ac972cf7b5929f34264..111eb4cca9065b97e4fa9314269d2ac7c43af17e 100644 --- a/frontend/test/metabase/scenarios/dashboard-filters/dashboard-filters-source.cy.spec.js +++ b/frontend/test/metabase/scenarios/dashboard-filters/dashboard-filters-source.cy.spec.js @@ -192,7 +192,6 @@ const setupNativeQuestionSource = () => { const setupCustomList = () => { modal().within(() => { cy.findByText("Custom list").click(); - cy.findByRole("textbox").should("contain.value", "Gizmo"); cy.findByRole("textbox").clear().type("Doohickey\nGadget"); cy.button("Done").click(); });