diff --git a/frontend/src/metabase-lib/Question.ts b/frontend/src/metabase-lib/Question.ts index cfc2c58c6a4e477cfa4255263e3593ed110fb10c..9850476d6e9c1b0462fb0d4daccd7ff0e27b6836 100644 --- a/frontend/src/metabase-lib/Question.ts +++ b/frontend/src/metabase-lib/Question.ts @@ -562,15 +562,6 @@ class QuestionInner { return query.question().toUnderlyingRecords(); } - toFieldValues(field: Field): Question { - const query = this.composeThisQuery()?.query(); - if (query instanceof StructuredQuery) { - return query.setFields([field.reference()]).question(); - } - - return this; - } - toUnderlyingRecords(): Question { const query = this.query(); if (!(query instanceof StructuredQuery)) { diff --git a/frontend/src/metabase-lib/parameters/utils/parameter-source.ts b/frontend/src/metabase-lib/parameters/utils/parameter-source.ts index 9c1685ac1b5e0b63c97c914a93b14dc15ef8cdfc..de7ba4d2e7044c6cba288e4420d5512db42a6a25 100644 --- a/frontend/src/metabase-lib/parameters/utils/parameter-source.ts +++ b/frontend/src/metabase-lib/parameters/utils/parameter-source.ts @@ -1,6 +1,4 @@ import { - Dataset, - FieldValue, Parameter, ValuesQueryType, ValuesSourceConfig, @@ -118,19 +116,3 @@ export const canSearchFieldValues = ( 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-values.js b/frontend/src/metabase-lib/parameters/utils/parameter-values.js index fb8eab9cbb2d64be541ba33b20cb98aa4083c857..d13f2eb7504fe190f8296255cd6b215955d07baf 100644 --- a/frontend/src/metabase-lib/parameters/utils/parameter-values.js +++ b/frontend/src/metabase-lib/parameters/utils/parameter-values.js @@ -30,8 +30,9 @@ export function hasParameterValue(value) { export function normalizeParameter(parameter) { return { id: parameter.id, + name: parameter.name, + slug: parameter.slug, type: parameter.type, - value: normalizeParameterValue(parameter.type, parameter.value), target: parameter.target, values_query_type: getQueryType(parameter), values_source_type: getSourceType(parameter), @@ -42,7 +43,12 @@ export function normalizeParameter(parameter) { export function normalizeParameters(parameters) { return parameters .filter(parameter => _.has(parameter, "value")) - .map(normalizeParameter); + .map(({ type, value, target, id }) => ({ + id, + type, + value: normalizeParameterValue(type, value), + target, + })); } export function normalizeParameterValue(type, value) { diff --git a/frontend/src/metabase-types/api/mocks/parameters.ts b/frontend/src/metabase-types/api/mocks/parameters.ts index ce15e59058072abcfbf72d80d0a5bd8eda7b2801..17fd51327dd4e004e20336f89ab00c1536b0ecbb 100644 --- a/frontend/src/metabase-types/api/mocks/parameters.ts +++ b/frontend/src/metabase-types/api/mocks/parameters.ts @@ -1,4 +1,4 @@ -import { Parameter } from "metabase-types/api"; +import { Parameter, ParameterValues } from "metabase-types/api"; export const createMockParameter = (opts?: Partial<Parameter>): Parameter => ({ id: "1", @@ -7,3 +7,11 @@ export const createMockParameter = (opts?: Partial<Parameter>): Parameter => ({ slug: "text", ...opts, }); + +export const createMockParameterValues = ( + opts?: Partial<ParameterValues>, +): ParameterValues => ({ + values: [], + has_more_values: false, + ...opts, +}); diff --git a/frontend/src/metabase-types/api/parameters.ts b/frontend/src/metabase-types/api/parameters.ts index c4ddf6f814f2214726e67e798d1ecb2227c9f1e9..01036689c42fd0e9ed9979a87731294bc84ef6e6 100644 --- a/frontend/src/metabase-types/api/parameters.ts +++ b/frontend/src/metabase-types/api/parameters.ts @@ -1,4 +1,7 @@ +import { DashboardId } from "./dashboard"; +import { RowValue } from "./dataset"; import { CardId } from "./card"; +import { FieldId } from "./field"; export type StringParameterType = | "string/=" @@ -61,3 +64,10 @@ export interface ValuesSourceConfig { card_id?: CardId; value_field?: unknown[]; } + +export type ParameterValue = [RowValue]; + +export interface ParameterValues { + values: ParameterValue[]; + has_more_values: boolean; +} diff --git a/frontend/src/metabase-types/store/mocks/index.ts b/frontend/src/metabase-types/store/mocks/index.ts index ff4de746f4f0f424b6a2a76c2f9357ff2ec5f9b6..cadf854c2ff96b638be8bfb93228cfa29da229c5 100644 --- a/frontend/src/metabase-types/store/mocks/index.ts +++ b/frontend/src/metabase-types/store/mocks/index.ts @@ -3,6 +3,7 @@ export * from "./app"; export * from "./dashboard"; export * from "./embed"; export * from "./entities"; +export * from "./parameters"; export * from "./qb"; export * from "./settings"; export * from "./setup"; diff --git a/frontend/src/metabase-types/store/mocks/parameters.ts b/frontend/src/metabase-types/store/mocks/parameters.ts new file mode 100644 index 0000000000000000000000000000000000000000..d73dcacb8010e152c48c2a0c52367aea3de4dc3f --- /dev/null +++ b/frontend/src/metabase-types/store/mocks/parameters.ts @@ -0,0 +1,8 @@ +import { ParametersState } from "metabase-types/store/parameters"; + +export const createMockParametersState = ( + opts?: Partial<ParametersState>, +): ParametersState => ({ + parameterValuesCache: {}, + ...opts, +}); diff --git a/frontend/src/metabase-types/store/mocks/state.ts b/frontend/src/metabase-types/store/mocks/state.ts index 303f51bb92f17265d0a8298448799e6d717f4936..264e95f991b93de762a50fca7c535a041efa9399 100644 --- a/frontend/src/metabase-types/store/mocks/state.ts +++ b/frontend/src/metabase-types/store/mocks/state.ts @@ -6,6 +6,7 @@ import { createMockDashboardState, createMockEmbedState, createMockEntitiesState, + createMockParametersState, createMockQueryBuilderState, createMockSettingsState, createMockSetupState, @@ -18,6 +19,7 @@ export const createMockState = (opts?: Partial<State>): State => ({ dashboard: createMockDashboardState(), embed: createMockEmbedState(), entities: createMockEntitiesState(), + parameters: createMockParametersState(), qb: createMockQueryBuilderState(), settings: createMockSettingsState(), setup: createMockSetupState(), diff --git a/frontend/src/metabase-types/store/parameters.ts b/frontend/src/metabase-types/store/parameters.ts new file mode 100644 index 0000000000000000000000000000000000000000..24b26cb6b688d0b0bf770e9f4c84f2e32b3d5172 --- /dev/null +++ b/frontend/src/metabase-types/store/parameters.ts @@ -0,0 +1,7 @@ +import { ParameterValues } from "metabase-types/api"; + +export type ParameterValuesCache = Record<string, ParameterValues>; + +export interface ParametersState { + parameterValuesCache: ParameterValuesCache; +} diff --git a/frontend/src/metabase-types/store/state.ts b/frontend/src/metabase-types/store/state.ts index 45c4214c3b54d6dadc59ebb1c8213858b33b731d..290e2ae0da466e16c8e87a66e44ed8e5edca7975 100644 --- a/frontend/src/metabase-types/store/state.ts +++ b/frontend/src/metabase-types/store/state.ts @@ -5,6 +5,7 @@ import { DashboardState } from "./dashboard"; import { EmbedState } from "./embed"; import { EntitiesState } from "./entities"; import { QueryBuilderState } from "./qb"; +import { ParametersState } from "./parameters"; import { SettingsState } from "./settings"; import { SetupState } from "./setup"; @@ -16,6 +17,7 @@ export interface State { embed: EmbedState; entities: EntitiesState; qb: QueryBuilderState; + parameters: ParametersState; settings: SettingsState; setup: SetupState; } diff --git a/frontend/src/metabase/components/FieldValuesWidget.jsx b/frontend/src/metabase/components/FieldValuesWidget.jsx index 98ee9a8ae42aa07777c85977716f6ee85f5b469d..cac0e281d01681ada556515640e16e69c2178db9 100644 --- a/frontend/src/metabase/components/FieldValuesWidget.jsx +++ b/frontend/src/metabase/components/FieldValuesWidget.jsx @@ -22,9 +22,9 @@ import { defer } from "metabase/lib/promise"; import { stripId } from "metabase/lib/formatting"; import { fetchParameterValues, - fetchQuestionParameterValues, + fetchCardParameterValues, + fetchDashboardParameterValues, } from "metabase/parameters/actions"; -import { fetchDashboardParameterValues } from "metabase/dashboard/actions"; import Fields from "metabase/entities/fields"; import { @@ -54,7 +54,7 @@ const mapDispatchToProps = { addRemappings, fetchFieldValues, fetchParameterValues, - fetchQuestionParameterValues, + fetchCardParameterValues, fetchDashboardParameterValues, }; @@ -139,20 +139,21 @@ class FieldValuesWidgetInner extends Component { let valuesMode = this.state.valuesMode; try { if (canUseDashboardEndpoints(this.props.dashboard)) { - const { results, has_more_values } = + const { values, has_more_values } = await this.fetchDashboardParameterValues(query); - options = results; + options = values; valuesMode = has_more_values ? "search" : valuesMode; - } else if (canUseQuestionEndpoints(this.props.question)) { - const { results, has_more_values } = - await this.fetchQuestionParameterValues(query); - options = results; + } else if (canUseCardEndpoints(this.props.question)) { + const { values, has_more_values } = await this.fetchCardParameterValues( + query, + ); + options = values; valuesMode = has_more_values ? "search" : valuesMode; } else if (canUseParameterEndpoints(this.props.parameter)) { - const { results, has_more_values } = await this.fetchParameterValues( + const { values, has_more_values } = await this.fetchParameterValues( query, ); - options = results; + options = values; valuesMode = has_more_values ? "search" : valuesMode; } else { options = await this.fetchFieldValues(query); @@ -217,11 +218,11 @@ class FieldValuesWidgetInner extends Component { }); }; - fetchQuestionParameterValues = async query => { + fetchCardParameterValues = async query => { const { question, parameter } = this.props; - return this.props.fetchQuestionParameterValues({ - question, + return this.props.fetchCardParameterValues({ + cardId: question.id(), parameter, query, }); @@ -492,7 +493,7 @@ function canUseParameterEndpoints(parameter) { return parameter != null; } -function canUseQuestionEndpoints(question) { +function canUseCardEndpoints(question) { return question?.isSaved(); } diff --git a/frontend/src/metabase/dashboard/actions/actions.unit.spec.js b/frontend/src/metabase/dashboard/actions/actions.unit.spec.js index 8fb5878f828a3c37862f9a54092a0454477c74a7..8f0e4be3200598196792876d8633ffdba291f262 100644 --- a/frontend/src/metabase/dashboard/actions/actions.unit.spec.js +++ b/frontend/src/metabase/dashboard/actions/actions.unit.spec.js @@ -12,8 +12,6 @@ import { openAddQuestionSidebar, removeParameter, SET_DASHBOARD_ATTRIBUTES, - fetchDashboardParameterValuesWithCache, - FETCH_DASHBOARD_PARAMETER_FIELD_VALUES_WITH_CACHE, } from "./index"; DashboardApi.parameterSearch = jest.fn(); @@ -153,158 +151,4 @@ describe("dashboard actions", () => { }); }); }); - - describe("fetchDashboardParameterValuesWithCache", () => { - const dashboardId = 1; - const parameter = { id: "a" }; - const parameterWithFilteringParameters = { - id: "a", - filteringParameters: ["b", "c"], - }; - const parameters = [ - { id: "a", value: "aaa" }, - { id: "b", value: "bbb" }, - { id: "c", value: null }, - ]; - - let parameterValuesSearchCache; - const getState = () => ({ - dashboard: { - parameterValuesSearchCache, - }, - }); - - beforeEach(() => { - DashboardApi.parameterSearch.mockReset(); - DashboardApi.parameterSearch.mockResolvedValue({ - has_more_values: false, - values: [1, 2, 3], - }); - DashboardApi.parameterValues.mockReset(); - DashboardApi.parameterValues.mockResolvedValue({ - has_more_values: true, - values: [4, 5, 6], - }); - parameterValuesSearchCache = {}; - }); - - it("should fetch parameter values using the given query string and always set has_more_values to true", async () => { - const action = await fetchDashboardParameterValuesWithCache({ - dashboardId, - parameter, - parameters, - query: "foo", - })(dispatch, getState); - - expect(DashboardApi.parameterSearch).toHaveBeenCalledWith({ - dashId: 1, - paramId: "a", - query: "foo", - }); - expect(action).toEqual({ - payload: { - cacheKey: - "dashboardId: 1, parameterId: a, query: foo, filteringParameterValues: []", - has_more_values: true, - results: [[1], [2], [3]], - }, - type: FETCH_DASHBOARD_PARAMETER_FIELD_VALUES_WITH_CACHE, - }); - }); - - it("should fetch parameter values without a query string", async () => { - const action = await fetchDashboardParameterValuesWithCache({ - dashboardId, - parameter, - parameters, - })(dispatch, getState); - - expect(DashboardApi.parameterValues).toHaveBeenCalledWith({ - dashId: 1, - paramId: "a", - query: undefined, - }); - expect(action).toEqual({ - payload: { - cacheKey: - "dashboardId: 1, parameterId: a, query: null, filteringParameterValues: []", - results: [[4], [5], [6]], - has_more_values: true, - }, - type: FETCH_DASHBOARD_PARAMETER_FIELD_VALUES_WITH_CACHE, - }); - }); - - it("should fetch parameter values using a query string and filtering parameters", async () => { - const action = await fetchDashboardParameterValuesWithCache({ - dashboardId, - parameter: parameterWithFilteringParameters, - parameters, - query: "bar", - })(dispatch, getState); - - expect(DashboardApi.parameterSearch).toHaveBeenCalledWith({ - dashId: 1, - paramId: "a", - query: "bar", - b: "bbb", - }); - expect(action).toEqual({ - payload: { - cacheKey: - 'dashboardId: 1, parameterId: a, query: bar, filteringParameterValues: [["b","bbb"]]', - results: [[1], [2], [3]], - has_more_values: true, - }, - type: FETCH_DASHBOARD_PARAMETER_FIELD_VALUES_WITH_CACHE, - }); - }); - - it("should fetch parameter values without a query string but with filtering parameters", async () => { - const action = await fetchDashboardParameterValuesWithCache({ - dashboardId, - parameter: parameterWithFilteringParameters, - parameters, - })(dispatch, getState); - - expect(DashboardApi.parameterValues).toHaveBeenCalledWith({ - dashId: 1, - paramId: "a", - query: undefined, - b: "bbb", - }); - expect(action).toEqual({ - payload: { - cacheKey: - 'dashboardId: 1, parameterId: a, query: null, filteringParameterValues: [["b","bbb"]]', - results: [[4], [5], [6]], - has_more_values: true, - }, - type: FETCH_DASHBOARD_PARAMETER_FIELD_VALUES_WITH_CACHE, - }); - }); - - it("should not query when values exist in the cache", async () => { - const cacheKey = - 'dashboardId: 1, parameterId: a, query: bar, filteringParameterValues: [["b","bbb"]]'; - parameterValuesSearchCache = { - [cacheKey]: [[1], [2], [3]], - }; - - const action = await fetchDashboardParameterValuesWithCache({ - dashboardId, - parameter: parameterWithFilteringParameters, - parameters, - query: "bar", - })(dispatch, getState); - - expect(DashboardApi.parameterSearch).not.toHaveBeenCalled(); - expect(DashboardApi.parameterValues).not.toHaveBeenCalled(); - - expect(action).toEqual({ - payload: undefined, - type: FETCH_DASHBOARD_PARAMETER_FIELD_VALUES_WITH_CACHE, - }); - }); - }); }); diff --git a/frontend/src/metabase/dashboard/actions/parameters.js b/frontend/src/metabase/dashboard/actions/parameters.js index 089dc2e43d722fb3e7f959fcce405ae5d2464961..6fa3ede8501d8565a6ca13bc67085e661c0fab9f 100644 --- a/frontend/src/metabase/dashboard/actions/parameters.js +++ b/frontend/src/metabase/dashboard/actions/parameters.js @@ -6,23 +6,13 @@ import { createAction, createThunkAction } from "metabase/lib/redux"; import { createParameter, setParameterName as setParamName, - getFilteringParameterValuesMap, - getParameterValuesSearchKey, } from "metabase/parameters/utils/dashboards"; import { getParameterValuesByIdFromQueryParams } from "metabase/parameters/utils/parameter-values"; import { SIDEBAR_NAME } from "metabase/dashboard/constants"; -import { DashboardApi } from "metabase/services"; - import { getMetadata } from "metabase/selectors/metadata"; import { isActionDashCard } from "metabase/actions/utils"; -import { - getDashboard, - getParameterValues, - getDashboardParameterValuesSearchCache, - getDashboardParameterValuesCache, - getParameters, -} from "../selectors"; +import { getDashboard, getParameterValues, getParameters } from "../selectors"; import { isVirtualDashCard } from "../utils"; @@ -273,58 +263,6 @@ export const HIDE_ADD_PARAMETER_POPOVER = "metabase/dashboard/HIDE_ADD_PARAMETER_POPOVER"; export const hideAddParameterPopover = createAction(HIDE_ADD_PARAMETER_POPOVER); -export const FETCH_DASHBOARD_PARAMETER_FIELD_VALUES_WITH_CACHE = - "metabase/dashboard/FETCH_DASHBOARD_PARAMETER_FIELD_VALUES_WITH_CACHE"; - -export const fetchDashboardParameterValuesWithCache = createThunkAction( - FETCH_DASHBOARD_PARAMETER_FIELD_VALUES_WITH_CACHE, - ({ dashboardId, parameter, parameters, query }) => - async (dispatch, getState) => { - const parameterValuesSearchCache = getDashboardParameterValuesSearchCache( - getState(), - ); - const filteringParameterValues = getFilteringParameterValuesMap( - parameter, - parameters, - ); - const cacheKey = getParameterValuesSearchKey({ - dashboardId, - parameterId: parameter.id, - query, - filteringParameterValues, - }); - - if (parameterValuesSearchCache[cacheKey]) { - return; - } - - const endpoint = query - ? DashboardApi.parameterSearch - : DashboardApi.parameterValues; - const { values, has_more_values } = await endpoint({ - paramId: parameter.id, - dashId: dashboardId, - query, - ...filteringParameterValues, - }); - - return { - cacheKey, - results: values.map(value => [].concat(value)), - has_more_values: query ? true : has_more_values, - }; - }, -); - -export const fetchDashboardParameterValues = - args => async (dispatch, getState) => { - await dispatch(fetchDashboardParameterValuesWithCache(args)); - const dashboardParameterValuesCache = getDashboardParameterValuesCache( - getState(), - ); - return dashboardParameterValuesCache.get(args) || []; - }; - export const setOrUnsetParameterValues = parameterIdValuePairs => (dispatch, getState) => { const parameterValues = getParameterValues(getState()); diff --git a/frontend/src/metabase/dashboard/reducers.js b/frontend/src/metabase/dashboard/reducers.js index 67ad16ab95b05645b42226c7980a73f1f60b976a..228862ec314815eae9966a9a8c6a52f84b8829a6 100644 --- a/frontend/src/metabase/dashboard/reducers.js +++ b/frontend/src/metabase/dashboard/reducers.js @@ -31,8 +31,6 @@ import { HIDE_ADD_PARAMETER_POPOVER, SET_SIDEBAR, CLOSE_SIDEBAR, - FETCH_DASHBOARD_PARAMETER_FIELD_VALUES_WITH_CACHE, - SAVE_DASHBOARD_AND_CARDS, SET_DOCUMENT_TITLE, SET_SHOW_LOADING_COMPLETE_FAVICON, RESET, @@ -291,26 +289,6 @@ const parameterValues = handleActions( {}, ); -const parameterValuesSearchCache = handleActions( - { - [INITIALIZE]: { next: () => ({}) }, - [SAVE_DASHBOARD_AND_CARDS]: { - next: () => ({}), - }, - [FETCH_DASHBOARD_PARAMETER_FIELD_VALUES_WITH_CACHE]: { - next: (state, { payload }) => - payload - ? assoc(state, payload.cacheKey, { - results: payload.results, - has_more_values: payload.has_more_values, - }) - : state, - }, - [RESET]: { next: state => ({}) }, - }, - {}, -); - const loadingDashCards = handleActions( { [INITIALIZE]: { @@ -433,6 +411,5 @@ export default combineReducers({ loadingDashCards, isAddParameterPopoverOpen, sidebar, - parameterValuesSearchCache, missingActionParameters, }); diff --git a/frontend/src/metabase/dashboard/reducers.unit.spec.js b/frontend/src/metabase/dashboard/reducers.unit.spec.js index 05092ef82d85b3b407f4eb2bbcce5d882ade2d2f..4c340f186232754d5308847cd86ea2b5dc66d52c 100644 --- a/frontend/src/metabase/dashboard/reducers.unit.spec.js +++ b/frontend/src/metabase/dashboard/reducers.unit.spec.js @@ -31,7 +31,6 @@ describe("dashboard reducers", () => { loadingStatus: "idle", }, parameterValues: {}, - parameterValuesSearchCache: {}, sidebar: { props: {} }, slowCards: {}, loadingControls: {}, diff --git a/frontend/src/metabase/dashboard/selectors.js b/frontend/src/metabase/dashboard/selectors.js index 4c6fba87e8dab369541371fecad8b7f0869df5dc..73742c229ac5278c30e152a7fe428148b63e2e92 100644 --- a/frontend/src/metabase/dashboard/selectors.js +++ b/frontend/src/metabase/dashboard/selectors.js @@ -4,11 +4,7 @@ import { createSelector } from "reselect"; import { getMetadata } from "metabase/selectors/metadata"; import { LOAD_COMPLETE_FAVICON } from "metabase/hoc/Favicon"; -import { - getDashboardUiParameters, - getFilteringParameterValuesMap, - getParameterValuesSearchKey, -} from "metabase/parameters/utils/dashboards"; +import { getDashboardUiParameters } from "metabase/parameters/utils/dashboards"; import { getParameterMappingOptions as _getParameterMappingOptions } from "metabase/parameters/utils/mapping-options"; import { SIDEBAR_NAME } from "metabase/dashboard/constants"; @@ -200,34 +196,6 @@ export const getDefaultParametersById = createSelector( }, {}), ); -export const getDashboardParameterValuesSearchCache = state => - state.dashboard.parameterValuesSearchCache; - -export const getDashboardParameterValuesCache = state => { - return { - get: ({ dashboardId, parameter, parameters, query }) => { - if (!parameter) { - return undefined; - } - - const { parameterValuesSearchCache } = state.dashboard; - - const filteringParameterValues = getFilteringParameterValuesMap( - parameter, - parameters, - ); - - const cacheKey = getParameterValuesSearchKey({ - dashboardId, - parameterId: parameter.id, - query, - filteringParameterValues, - }); - return parameterValuesSearchCache[cacheKey]; - }, - }; -}; - export const getIsHeaderVisible = createSelector( [getIsEmbedded, getEmbedOptions], (isEmbedded, embedOptions) => !isEmbedded || embedOptions.header, diff --git a/frontend/src/metabase/parameters/actions.ts b/frontend/src/metabase/parameters/actions.ts index cab961bcecead0d31c442b9c764f04e7719540bd..126eb4fefd80ac1b36849ffd6cdcac3aa8dd7673 100644 --- a/frontend/src/metabase/parameters/actions.ts +++ b/frontend/src/metabase/parameters/actions.ts @@ -1,50 +1,167 @@ -import { CardApi, ParameterApi } from "metabase/services"; -import { Parameter } from "metabase-types/api"; -import Question from "metabase-lib/Question"; +import { CardApi, DashboardApi, ParameterApi } from "metabase/services"; +import { + CardId, + DashboardId, + FieldId, + Parameter, + ParameterId, + ParameterValues, +} from "metabase-types/api"; +import { Dispatch, GetState } from "metabase-types/store"; import { getNonVirtualFields } from "metabase-lib/parameters/utils/parameter-fields"; import { normalizeParameter } from "metabase-lib/parameters/utils/parameter-values"; +import { getParameterValuesCache } from "./selectors"; +import { getFilteringParameterValuesMap } from "./utils/dashboards"; -interface FetchParameterValuesOpts { +export const FETCH_PARAMETER_VALUES = + "metabase/parameters/FETCH_PARAMETER_VALUES"; + +export interface FetchParameterValuesOpts { parameter: Parameter; query?: string; } +export interface FetchParameterValuesPayload { + requestKey: string; + response: ParameterValues; +} + export const fetchParameterValues = ({ parameter, query }: FetchParameterValuesOpts) => - async () => { - const apiArgs = { + (dispatch: Dispatch, getState: GetState) => { + const request = { parameter: normalizeParameter(parameter), - field_ids: getNonVirtualFields(parameter).map(field => field.id), + field_ids: getNonVirtualFields(parameter).map(field => Number(field.id)), query, }; - const { values, has_more_values } = query - ? await ParameterApi.parameterSearch(apiArgs) - : await ParameterApi.parameterValues(apiArgs); + return fetchParameterValuesWithCache( + request, + loadParameterValues, + dispatch, + getState, + ); + }; - return { - results: values.map((value: any) => [].concat(value)), - has_more_values: query ? true : has_more_values, +export interface FetchCardParameterValuesOpts { + cardId: CardId; + parameter: Parameter; + query?: string; +} + +export const fetchCardParameterValues = + ({ cardId, parameter, query }: FetchCardParameterValuesOpts) => + (dispatch: Dispatch, getState: GetState) => { + const request = { + cardId, + paramId: parameter.id, + query, }; + + return fetchParameterValuesWithCache( + request, + loadCardParameterValues, + dispatch, + getState, + ); }; -interface FetchQuestionParameterValuesOpts { - question: Question; +export interface FetchDashboardParameterValuesOpts { + dashboardId: DashboardId; parameter: Parameter; + parameters: Parameter[]; query?: string; } -export const fetchQuestionParameterValues = - ({ question, parameter, query }: FetchQuestionParameterValuesOpts) => - async () => { - const apiArgs = { cardId: question.id(), paramId: parameter.id, query }; +export const fetchDashboardParameterValues = + ({ + dashboardId, + parameter, + parameters, + query, + }: FetchDashboardParameterValuesOpts) => + (dispatch: Dispatch, getState: GetState) => { + const request = { + paramId: parameter.id, + dashId: dashboardId, + query, + ...getFilteringParameterValuesMap(parameter, parameters), + }; - const { values, has_more_values } = query - ? await CardApi.parameterSearch(apiArgs) - : await CardApi.parameterValues(apiArgs); + return fetchParameterValuesWithCache( + request, + loadDashboardParameterValues, + dispatch, + getState, + ); + }; - return { - results: values.map((value: any) => [].concat(value)), - has_more_values: query ? true : has_more_values, - }; +interface ParameterValuesRequest { + parameter: Parameter; + field_ids: FieldId[]; + query?: string; +} + +const loadParameterValues = async (request: ParameterValuesRequest) => { + const { values, has_more_values } = request.query + ? await ParameterApi.parameterSearch(request) + : await ParameterApi.parameterValues(request); + + return { + values: values.map((value: any) => [].concat(value)), + has_more_values: request.query ? true : has_more_values, + }; +}; + +interface CardParameterValuesRequest { + cardId: CardId; + paramId: ParameterId; + query?: string; +} + +const loadCardParameterValues = async (request: CardParameterValuesRequest) => { + const { values, has_more_values } = request.query + ? await CardApi.parameterSearch(request) + : await CardApi.parameterValues(request); + + return { + values: values.map((value: any) => [].concat(value)), + has_more_values: request.query ? true : has_more_values, }; +}; + +interface DashboardParameterValuesRequest { + dashId: DashboardId; + paramId: ParameterId; + query?: string; +} + +const loadDashboardParameterValues = async ( + request: DashboardParameterValuesRequest, +) => { + const { values, has_more_values } = request.query + ? await DashboardApi.parameterSearch(request) + : await DashboardApi.parameterValues(request); + + return { + values: values.map((value: any) => [].concat(value)), + has_more_values: request.query ? true : has_more_values, + }; +}; + +const fetchParameterValuesWithCache = async <T>( + request: T, + loadValues: (request: T) => Promise<ParameterValues>, + dispatch: Dispatch, + getState: GetState, +) => { + const requestKey = JSON.stringify(request); + const requestCache = getParameterValuesCache(getState()); + const response = requestCache[requestKey] + ? requestCache[requestKey] + : await loadValues(request); + + const payload = { requestKey, response }; + dispatch({ type: FETCH_PARAMETER_VALUES, payload }); + return response; +}; diff --git a/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceCardModal.tsx b/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceCardModal.tsx index 8c0a00ed75723e3f14bea8fe195a44b1a08a53e0..a18f18f91cd65e70d7fa3f383dc3fa8f4f7d6b37 100644 --- a/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceCardModal.tsx +++ b/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceCardModal.tsx @@ -19,6 +19,7 @@ import { Card, CardId, Collection, + Parameter, ValuesSourceConfig, } from "metabase-types/api"; import { State } from "metabase-types/store"; @@ -41,7 +42,7 @@ const DATA_PICKER_FILTERS = { }; interface ModalOwnProps { - name: string; + parameter: Parameter; sourceConfig: ValuesSourceConfig; onChangeSourceConfig: (sourceConfig: ValuesSourceConfig) => void; onSubmit: () => void; @@ -72,7 +73,7 @@ type ModalProps = ModalOwnProps & ModalDispatchProps; const ValuesSourceCardModal = ({ - name, + parameter, question, collection, onFetchCard, @@ -100,7 +101,7 @@ const ValuesSourceCardModal = ({ return ( <DataPicker.Provider> <ModalContent - title={t`Selectable values for ${name}`} + title={t`Selectable values for ${parameter.name}`} footer={[ <Button key="cancel" onClick={onSubmit}>{t`Back`}</Button>, <Button diff --git a/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.tsx b/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.tsx index 88f221f2e9d79b2d9c29a8ff0d97b2ae77766b82..0d764a3690016d2403ea7a3bdcbc66c854aaa214 100644 --- a/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.tsx +++ b/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.tsx @@ -1,15 +1,15 @@ -import React, { useCallback, useMemo, useState } from "react"; +import React, { useCallback, useState } from "react"; import { Parameter, 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 { ParameterWithTemplateTagTarget } from "metabase-lib/parameters/types"; import ValuesSourceTypeModal from "./ValuesSourceTypeModal"; import ValuesSourceCardModal from "./ValuesSourceCardModal"; @@ -30,13 +30,9 @@ const ValuesSourceModal = ({ onClose, }: ModalProps): JSX.Element => { const [step, setStep] = useState<ModalStep>("main"); - const [sourceType, setSourceType] = useState(getSourceType(parameter)); + const [sourceType, setSourceType] = useState(getInitialSourceType(parameter)); const [sourceConfig, setSourceConfig] = useState(getSourceConfig(parameter)); - const fields = useMemo(() => { - return getNonVirtualFields(parameter); - }, [parameter]); - const handlePickerOpen = useCallback(() => { setStep("card"); }, []); @@ -52,8 +48,7 @@ const ValuesSourceModal = ({ return step === "main" ? ( <ValuesSourceTypeModal - name={parameter.name} - fields={fields} + parameter={parameter} sourceType={sourceType} sourceConfig={sourceConfig} onChangeSourceType={setSourceType} @@ -64,7 +59,7 @@ const ValuesSourceModal = ({ /> ) : ( <ValuesSourceCardModal - name={parameter.name} + parameter={parameter} sourceConfig={sourceConfig} onChangeSourceConfig={setSourceConfig} onSubmit={handlePickerClose} @@ -73,4 +68,10 @@ const ValuesSourceModal = ({ ); }; +const getInitialSourceType = (parameter: ParameterWithTemplateTagTarget) => { + return parameter.hasVariableTemplateTagTarget + ? "card" + : getSourceType(parameter); +}; + export default ValuesSourceModal; diff --git a/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.unit.spec.tsx b/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.unit.spec.tsx index 5d27c7cbd20620dcad90d5556f348c02337d3399..d9d89710740cdc89acbc458b7a752b4d53905df9 100644 --- a/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.unit.spec.tsx +++ b/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.unit.spec.tsx @@ -2,17 +2,17 @@ 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 { Card, ParameterValues } from "metabase-types/api"; import { createMockCard, createMockCollection, createMockField, - createMockFieldValues, + createMockParameterValues, } from "metabase-types/api/mocks"; import { setupCardsEndpoints, setupCollectionsEndpoints, - setupFieldsValuesEndpoints, + setupParameterValuesEndpoints, setupUnauthorizedCardsEndpoints, } from "__support__/server-mocks"; import { renderWithProviders, screen, waitFor } from "__support__/ui"; @@ -40,12 +40,9 @@ describe("ValuesSourceModal", () => { parameter: createMockUiParameter({ fields: [new Field(createMockField({ id: 1 }))], }), - fieldsValues: [ - createMockFieldValues({ - field_id: 1, - values: [], - }), - ], + parameterValues: createMockParameterValues({ + values: [], + }), }); expect( @@ -53,7 +50,7 @@ describe("ValuesSourceModal", () => { ).toBeInTheDocument(); }); - it("should show unique non-null mapped fields values", async () => { + it("should show field values", async () => { setup({ parameter: createMockUiParameter({ fields: [ @@ -61,22 +58,30 @@ describe("ValuesSourceModal", () => { new Field(createMockField({ id: 2 })), ], }), - fieldsValues: [ - createMockFieldValues({ - field_id: 1, - values: [["A"], [null], ["B"], ["A"]], - }), - createMockFieldValues({ - field_id: 2, - values: [["B", "Remapped"], ["C"]], - }), - ], + parameterValues: createMockParameterValues({ + values: [["A"], ["B"], ["C"]], + }), }); await waitFor(() => { expect(screen.getByRole("textbox")).toHaveValue("A\nB\nC"); }); }); + + it("should not show the fields option for variable template tags", () => { + setup({ + parameter: createMockUiParameter({ + hasVariableTemplateTagTarget: true, + }), + }); + + expect( + screen.queryByRole("radio", { name: "From connected fields" }), + ).not.toBeInTheDocument(); + expect( + screen.getByRole("radio", { name: "From another model or question" }), + ).toBeChecked(); + }); }); describe("card source", () => { @@ -146,6 +151,39 @@ describe("ValuesSourceModal", () => { }); }); + it("should show card values", async () => { + setup({ + parameter: createMockUiParameter({ + values_source_type: "card", + values_source_config: { + card_id: 1, + value_field: ["field", 2, null], + }, + }), + parameterValues: createMockParameterValues({ + values: [["A"], ["B"], ["C"]], + }), + cards: [ + createMockCard({ + id: 1, + name: "Products", + result_metadata: [ + createMockField({ + id: 2, + display_name: "Category", + base_type: "type/Text", + semantic_type: "type/Category", + }), + ], + }), + ], + }); + + await waitFor(() => { + expect(screen.getByRole("textbox")).toHaveValue("A\nB\nC"); + }); + }); + it("should display an error message when the user has no access to the card", async () => { setup({ parameter: createMockUiParameter({ @@ -204,15 +242,15 @@ describe("ValuesSourceModal", () => { interface SetupOpts { parameter?: UiParameter; + parameterValues?: ParameterValues; cards?: Card[]; - fieldsValues?: FieldValues[]; hasDataAccess?: boolean; } const setup = ({ parameter = createMockUiParameter(), + parameterValues = createMockParameterValues(), cards = [], - fieldsValues = [], hasDataAccess = true, }: SetupOpts = {}) => { const scope = nock(location.origin); @@ -222,7 +260,7 @@ const setup = ({ if (hasDataAccess) { setupCollectionsEndpoints(scope, [createMockCollection(ROOT_COLLECTION)]); setupCardsEndpoints(scope, cards); - setupFieldsValuesEndpoints(scope, fieldsValues); + setupParameterValuesEndpoints(scope, parameterValues); } else { setupUnauthorizedCardsEndpoints(scope, cards); } diff --git a/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceTypeModal.tsx b/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceTypeModal.tsx index 9f42e777d32ec9aa4307ccd554391db735a8670d..56660258cef4073f2453869ad0e01f88420193e8 100644 --- a/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceTypeModal.tsx +++ b/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceTypeModal.tsx @@ -1,36 +1,41 @@ -import React, { ChangeEvent, useCallback, useEffect, useMemo } from "react"; +import React, { + ChangeEvent, + useCallback, + useEffect, + useMemo, + useState, +} from "react"; import { connect } from "react-redux"; import { t } from "ttag"; import _ from "underscore"; import Button from "metabase/core/components/Button"; -import Radio from "metabase/core/components/Radio"; +import Radio, { RadioOption } from "metabase/core/components/Radio"; import Select, { Option, SelectChangeEvent, } from "metabase/core/components/Select"; import SelectButton from "metabase/core/components/SelectButton"; import ModalContent from "metabase/components/ModalContent"; -import QuestionResultLoader from "metabase/containers/QuestionResultLoader"; -import Fields from "metabase/entities/fields"; +import { useSafeAsyncFunction } from "metabase/hooks/use-safe-async-function"; import Tables from "metabase/entities/tables"; import Questions from "metabase/entities/questions"; import { getMetadata } from "metabase/selectors/metadata"; import { - Dataset, Card, ValuesSourceConfig, ValuesSourceType, - FieldValue, + Parameter, + ParameterValues, + ParameterValue, } from "metabase-types/api"; -import { Dispatch, State } from "metabase-types/store"; +import { State } from "metabase-types/store"; import Question from "metabase-lib/Question"; import Field from "metabase-lib/metadata/Field"; import { getQuestionVirtualTableId } from "metabase-lib/metadata/utils/saved-questions"; -import { - getCardSourceValues, - getFieldSourceValues, - isValidSourceConfig, -} from "metabase-lib/parameters/utils/parameter-source"; +import { getFields } from "metabase-lib/parameters/utils/parameter-fields"; +import { isValidSourceConfig } from "metabase-lib/parameters/utils/parameter-source"; +import { ParameterWithTemplateTagTarget } from "metabase-lib/parameters/types"; +import { fetchParameterValues, FetchParameterValuesOpts } from "../../actions"; import { ModalLoadingAndErrorWrapper } from "./ValuesSourceModal.styled"; import { ModalHelpMessage, @@ -46,15 +51,8 @@ import { const NEW_LINE = "\n"; -const SOURCE_TYPE_OPTIONS = [ - { name: t`From connected fields`, value: null }, - { name: t`From another model or question`, value: "card" }, - { name: t`Custom list`, value: "static-list" }, -]; - interface ModalOwnProps { - name: string; - fields: Field[]; + parameter: ParameterWithTemplateTagTarget; sourceType: ValuesSourceType; sourceConfig: ValuesSourceConfig; onChangeSourceType: (sourceType: ValuesSourceType) => void; @@ -70,16 +68,12 @@ interface ModalCardProps { interface ModalStateProps { question: Question | undefined; - fieldsValues: FieldValue[][]; - isLoadingFieldValues: boolean; } interface ModalDispatchProps { - onFetchFieldValues: (fields: Field[]) => void; -} - -interface QuestionLoaderProps { - result?: Dataset; + onFetchParameterValues: ( + opts: FetchParameterValuesOpts, + ) => Promise<ParameterValues>; } type ModalProps = ModalOwnProps & @@ -88,27 +82,24 @@ type ModalProps = ModalOwnProps & ModalDispatchProps; const ValuesSourceTypeModal = ({ - name, - fields, - fieldsValues, - isLoadingFieldValues, + parameter, question, sourceType, sourceConfig, - onFetchFieldValues, + onFetchParameterValues, onChangeSourceType, onChangeSourceConfig, onChangeCard, onSubmit, onClose, }: ModalProps): JSX.Element => { - useEffect(() => { - onFetchFieldValues(fields); - }, [fields, onFetchFieldValues]); + const sourceTypeOptions = useMemo(() => { + return getSourceTypeOptions(parameter); + }, [parameter]); return ( <ModalContent - title={t`Selectable values for ${name}`} + title={t`Selectable values for ${parameter.name}`} footer={[ <Button key="submit" @@ -121,17 +112,21 @@ const ValuesSourceTypeModal = ({ > {sourceType === null ? ( <FieldSourceModal - fields={fields} - fieldsValues={fieldsValues} - isLoadingFieldValues={isLoadingFieldValues} + parameter={parameter} sourceType={sourceType} + sourceTypeOptions={sourceTypeOptions} + sourceConfig={sourceConfig} + onFetchParameterValues={onFetchParameterValues} onChangeSourceType={onChangeSourceType} /> ) : sourceType === "card" ? ( <CardSourceModal + parameter={parameter} question={question} sourceType={sourceType} + sourceTypeOptions={sourceTypeOptions} sourceConfig={sourceConfig} + onFetchParameterValues={onFetchParameterValues} onChangeCard={onChangeCard} onChangeSourceType={onChangeSourceType} onChangeSourceConfig={onChangeSourceConfig} @@ -139,6 +134,7 @@ const ValuesSourceTypeModal = ({ ) : sourceType === "static-list" ? ( <ListSourceModal sourceType={sourceType} + sourceTypeOptions={sourceTypeOptions} sourceConfig={sourceConfig} onChangeSourceType={onChangeSourceType} onChangeSourceConfig={onChangeSourceConfig} @@ -149,30 +145,38 @@ const ValuesSourceTypeModal = ({ }; interface FieldSourceModalProps { - fields: Field[]; - fieldsValues: FieldValue[][]; - isLoadingFieldValues: boolean; + parameter: Parameter; sourceType: ValuesSourceType; + sourceTypeOptions: RadioOption<ValuesSourceType>[]; + sourceConfig: ValuesSourceConfig; + onFetchParameterValues: ( + opts: FetchParameterValuesOpts, + ) => Promise<ParameterValues>; onChangeSourceType: (sourceType: ValuesSourceType) => void; } const FieldSourceModal = ({ - fields, - fieldsValues, - isLoadingFieldValues, + parameter, sourceType, + sourceTypeOptions, + sourceConfig, + onFetchParameterValues, onChangeSourceType, }: FieldSourceModalProps) => { - const fieldValues = useMemo(() => { - return getFieldSourceValues(fieldsValues); - }, [fieldsValues]); + const parameterValues = useParameterValues({ + parameter, + sourceType, + sourceConfig, + onFetchParameterValues, + }); - const fieldValuesText = useMemo(() => { - return getValuesText(fieldValues); - }, [fieldValues]); + const parameterValuesText = useMemo( + () => getParameterValuesText(parameterValues), + [parameterValues], + ); - const hasFields = fields.length > 0; - const hasFieldValues = fieldValues.length > 0; + const hasFields = getFields(parameter).length > 0; + const hasEmptyValues = parameterValues && parameterValues.length === 0; return ( <ModalBodyWithPane> @@ -181,7 +185,7 @@ const FieldSourceModal = ({ <ModalLabel>{t`Where values should come from`}</ModalLabel> <Radio value={sourceType} - options={SOURCE_TYPE_OPTIONS} + options={sourceTypeOptions} vertical onChange={onChangeSourceType} /> @@ -192,12 +196,12 @@ const FieldSourceModal = ({ <ModalEmptyState> {t`You haven’t connected a field to this filter yet, so there aren’t any values.`} </ModalEmptyState> - ) : !hasFieldValues && !isLoadingFieldValues ? ( + ) : 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> ) : ( - <ModalTextArea value={fieldValuesText} readOnly fullWidth /> + <ModalTextArea value={parameterValuesText} readOnly fullWidth /> )} </ModalMain> </ModalBodyWithPane> @@ -205,18 +209,26 @@ const FieldSourceModal = ({ }; interface CardSourceModalProps { + parameter: Parameter; question: Question | undefined; sourceType: ValuesSourceType; + sourceTypeOptions: RadioOption<ValuesSourceType>[]; sourceConfig: ValuesSourceConfig; + onFetchParameterValues: ( + opts: FetchParameterValuesOpts, + ) => Promise<ParameterValues>; onChangeCard: () => void; onChangeSourceType: (sourceType: ValuesSourceType) => void; onChangeSourceConfig: (sourceConfig: ValuesSourceConfig) => void; } const CardSourceModal = ({ + parameter, question, sourceType, + sourceTypeOptions, sourceConfig, + onFetchParameterValues, onChangeCard, onChangeSourceType, onChangeSourceConfig, @@ -229,9 +241,17 @@ const CardSourceModal = ({ return getFieldByReference(fields, sourceConfig.value_field); }, [fields, sourceConfig]); - const fieldValuesQuestion = useMemo(() => { - return question && selectedField && question.toFieldValues(selectedField); - }, [question, selectedField]); + const parameterValues = useParameterValues({ + parameter, + sourceType, + sourceConfig, + onFetchParameterValues, + }); + + const parameterValuesText = useMemo( + () => getParameterValuesText(parameterValues), + [parameterValues], + ); const handleFieldChange = useCallback( (event: SelectChangeEvent<Field>) => { @@ -250,7 +270,7 @@ const CardSourceModal = ({ <ModalLabel>{t`Where values should come from`}</ModalLabel> <Radio value={sourceType} - options={SOURCE_TYPE_OPTIONS} + options={sourceTypeOptions} vertical onChange={onChangeSourceType} /> @@ -295,15 +315,7 @@ const CardSourceModal = ({ ) : !selectedField ? ( <ModalEmptyState>{t`Pick a column`}</ModalEmptyState> ) : ( - <QuestionResultLoader question={fieldValuesQuestion}> - {({ result }: QuestionLoaderProps) => ( - <ModalTextArea - value={result ? getValuesText(getCardSourceValues(result)) : ""} - readOnly - fullWidth - /> - )} - </QuestionResultLoader> + <ModalTextArea value={parameterValuesText} readOnly fullWidth /> )} </ModalMain> </ModalBodyWithPane> @@ -312,6 +324,7 @@ const CardSourceModal = ({ interface ListSourceModalProps { sourceType: ValuesSourceType; + sourceTypeOptions: RadioOption<ValuesSourceType>[]; sourceConfig: ValuesSourceConfig; onChangeSourceType: (sourceType: ValuesSourceType) => void; onChangeSourceConfig: (sourceConfig: ValuesSourceConfig) => void; @@ -319,6 +332,7 @@ interface ListSourceModalProps { const ListSourceModal = ({ sourceType, + sourceTypeOptions, sourceConfig, onChangeSourceType, onChangeSourceConfig, @@ -337,7 +351,7 @@ const ListSourceModal = ({ <ModalLabel>{t`Where values should come from`}</ModalLabel> <Radio value={sourceType} - options={SOURCE_TYPE_OPTIONS} + options={sourceTypeOptions} vertical onChange={onChangeSourceType} /> @@ -346,7 +360,7 @@ const ListSourceModal = ({ </ModalPane> <ModalMain> <ModalTextArea - defaultValue={getValuesText(sourceConfig.values)} + defaultValue={getStaticValuesText(sourceConfig.values)} fullWidth onChange={handleValuesChange} /> @@ -355,8 +369,12 @@ const ListSourceModal = ({ ); }; -const getValuesText = (values?: string[]) => { - return values?.join(NEW_LINE) ?? ""; +const getParameterValuesText = (values: ParameterValue[] = []) => { + return values.map(([value]) => value).join(NEW_LINE); +}; + +const getStaticValuesText = (values: string[] = []) => { + return values.join(NEW_LINE); }; const getStaticValues = (value: string) => { @@ -375,29 +393,67 @@ const getSupportedFields = (question: Question) => { return fields.filter(field => field.isString()); }; +const getSourceTypeOptions = ( + parameter: ParameterWithTemplateTagTarget, +): RadioOption<ValuesSourceType>[] => { + return [ + ...(parameter.hasVariableTemplateTagTarget + ? [] + : [{ name: t`From connected fields`, value: null }]), + { name: t`From another model or question`, value: "card" }, + { name: t`Custom list`, value: "static-list" }, + ]; +}; + +interface UseParameterValuesOpts { + parameter: Parameter; + sourceType: ValuesSourceType; + sourceConfig: ValuesSourceConfig; + onFetchParameterValues: ( + opts: FetchParameterValuesOpts, + ) => Promise<ParameterValues>; +} + +const useParameterValues = ({ + parameter: initialParameter, + sourceType, + sourceConfig, + onFetchParameterValues, +}: UseParameterValuesOpts) => { + const [values, setValues] = useState<ParameterValue[]>(); + const handleFetchValues = useSafeAsyncFunction(onFetchParameterValues); + const isValidSource = isValidSourceConfig(sourceType, sourceConfig); + + const parameter = useMemo( + () => ({ + ...initialParameter, + values_source_type: sourceType, + values_source_config: sourceConfig, + }), + [initialParameter, sourceType, sourceConfig], + ); + + useEffect(() => { + if (isValidSource) { + handleFetchValues({ parameter }) + .then(({ values }) => setValues(values)) + .catch(() => setValues([])); + } + }, [parameter, isValidSource, handleFetchValues]); + + return values; +}; + const mapStateToProps = ( state: State, - { card, fields }: ModalOwnProps & ModalCardProps, + { card }: ModalOwnProps & ModalCardProps, ): ModalStateProps => ({ question: card ? new Question(card, getMetadata(state)) : undefined, - fieldsValues: fields.map(field => - Fields.selectors.getFieldValues(state, { entityId: field.id }), - ), - isLoadingFieldValues: fields.every(field => - Fields.selectors.getLoading(state, { - entityId: field.id, - requestType: "values", - }), - ), }); -const mapDispatchToProps = (dispatch: Dispatch): ModalDispatchProps => ({ - onFetchFieldValues: (fields: Field[]) => { - fields.forEach(field => - dispatch(Fields.actions.fetchFieldValues({ id: field.id })), - ); - }, -}); +const mapDispatchToProps = { + onFetchParameterValues: fetchParameterValues, +}; export default _.compose( Tables.load({ diff --git a/frontend/src/metabase/parameters/reducers.ts b/frontend/src/metabase/parameters/reducers.ts new file mode 100644 index 0000000000000000000000000000000000000000..2a123c7b6e7c1aeaa654cf4f05b176cdebd4cf82 --- /dev/null +++ b/frontend/src/metabase/parameters/reducers.ts @@ -0,0 +1,36 @@ +import { handleActions } from "redux-actions"; +import { + INITIALIZE, + RESET, + SAVE_DASHBOARD_AND_CARDS, +} from "metabase/dashboard/actions"; +import { + API_UPDATE_QUESTION, + INITIALIZE_QB, + RESET_QB, +} from "metabase/query_builder/actions"; +import { ParameterValuesCache } from "metabase-types/store/parameters"; +import { FETCH_PARAMETER_VALUES, FetchParameterValuesPayload } from "./actions"; + +export const parameterValuesCache = handleActions< + ParameterValuesCache, + FetchParameterValuesPayload +>( + { + [FETCH_PARAMETER_VALUES]: { + next: (state, { payload: { requestKey, response } }) => + state[requestKey] !== response + ? { ...state, [requestKey]: response } + : state, + }, + // dashboards + [INITIALIZE]: { next: () => ({}) }, + [SAVE_DASHBOARD_AND_CARDS]: { next: () => ({}) }, + [RESET]: { next: () => ({}) }, + // query builder + [INITIALIZE_QB]: { next: () => ({}) }, + [API_UPDATE_QUESTION]: { next: () => ({}) }, + [RESET_QB]: { next: () => ({}) }, + }, + {}, +); diff --git a/frontend/src/metabase/parameters/selectors.ts b/frontend/src/metabase/parameters/selectors.ts new file mode 100644 index 0000000000000000000000000000000000000000..56971f3a9ee6e6170ec11a398b36bc5d83e6de8c --- /dev/null +++ b/frontend/src/metabase/parameters/selectors.ts @@ -0,0 +1,5 @@ +import { State } from "metabase-types/store"; + +export const getParameterValuesCache = (state: State) => { + return state.parameters.parameterValuesCache; +}; diff --git a/frontend/src/metabase/parameters/utils/dashboards.ts b/frontend/src/metabase/parameters/utils/dashboards.ts index ccfc37556fc1888604b0813d3510390da3fb7353..741a417552c641c991c371806efbb3a4f6ab11dd 100644 --- a/frontend/src/metabase/parameters/utils/dashboards.ts +++ b/frontend/src/metabase/parameters/utils/dashboards.ts @@ -228,25 +228,3 @@ export function getFilteringParameterValuesMap( return filteringParameterValues; } - -export function getParameterValuesSearchKey({ - dashboardId, - parameterId, - query = null, - filteringParameterValues = {}, -}: { - dashboardId: number; - parameterId: string; - query: string | null; - filteringParameterValues: { [parameterId: string]: unknown }; -}) { - const BY_PARAMETER_ID = "0"; - // sorting the filteringParameterValues map by its parameter id key to ensure entry order doesn't affect the outputted cache key - const sortedParameterValues = _.sortBy( - Object.entries(filteringParameterValues), - BY_PARAMETER_ID, - ); - const stringifiedParameterValues = JSON.stringify(sortedParameterValues); - - return `dashboardId: ${dashboardId}, parameterId: ${parameterId}, query: ${query}, filteringParameterValues: ${stringifiedParameterValues}`; -} diff --git a/frontend/src/metabase/parameters/utils/dashboards.unit.spec.js b/frontend/src/metabase/parameters/utils/dashboards.unit.spec.js index 4728c636bc48c3e493a921e4fcb8a42b0e247ce9..9b7e104d8a6271b3892ff0c6ff7786f43ba042ff 100644 --- a/frontend/src/metabase/parameters/utils/dashboards.unit.spec.js +++ b/frontend/src/metabase/parameters/utils/dashboards.unit.spec.js @@ -6,7 +6,6 @@ import { getParametersMappedToDashcard, hasMatchingParameters, getFilteringParameterValuesMap, - getParameterValuesSearchKey, getDashboardUiParameters, } from "metabase/parameters/utils/dashboards"; import { PRODUCTS, metadata } from "__support__/sample_database_fixture"; @@ -517,66 +516,6 @@ describe("metabase/parameters/utils/dashboards", () => { }); }); - describe("getParameterValuesSearchKey", () => { - it("should return a string using the given props related to parameter value searching", () => { - expect( - getParameterValuesSearchKey({ - dashboardId: "123", - parameterId: "456", - query: "foo", - filteringParameterValues: { - a: "aaa", - b: "bbb", - }, - }), - ).toEqual( - 'dashboardId: 123, parameterId: 456, query: foo, filteringParameterValues: [["a","aaa"],["b","bbb"]]', - ); - }); - - it("should default `query` to null", () => { - expect( - getParameterValuesSearchKey({ - dashboardId: "123", - parameterId: "456", - filteringParameterValues: { - a: "aaa", - b: "bbb", - }, - }), - ).toEqual( - 'dashboardId: 123, parameterId: 456, query: null, filteringParameterValues: [["a","aaa"],["b","bbb"]]', - ); - }); - - it("should sort the entries in the `filteringParameterValues` object", () => { - expect( - getParameterValuesSearchKey({ - dashboardId: "123", - parameterId: "456", - filteringParameterValues: { - b: "bbb", - a: "aaa", - }, - }), - ).toEqual( - 'dashboardId: 123, parameterId: 456, query: null, filteringParameterValues: [["a","aaa"],["b","bbb"]]', - ); - }); - - it("should handle there being no filteringParameterValues", () => { - expect( - getParameterValuesSearchKey({ - dashboardId: "123", - parameterId: "456", - query: "abc", - }), - ).toEqual( - "dashboardId: 123, parameterId: 456, query: abc, filteringParameterValues: []", - ); - }); - }); - describe("getDashboardUiParameters", () => { const dashboard = { id: 1, diff --git a/frontend/src/metabase/reducers-main.js b/frontend/src/metabase/reducers-main.js index a5079895bdc96dd7e3b69ffaa2fc2291bcb31d91..ed95c962029429b7236096bedb18a5dea370998b 100644 --- a/frontend/src/metabase/reducers-main.js +++ b/frontend/src/metabase/reducers-main.js @@ -12,6 +12,11 @@ import * as setup from "metabase/setup/reducers"; /* dashboards */ import dashboard from "metabase/dashboard/reducers"; + +/* parameters */ +import * as parameters from "metabase/parameters/reducers"; + +/* home page */ import * as home from "metabase/home/reducers"; /* query builder */ @@ -28,6 +33,7 @@ import alert from "metabase/alert/alert"; /* pulses */ import * as pulse from "metabase/pulse/reducers"; + import commonReducers from "./reducers-common"; export default { @@ -36,6 +42,7 @@ export default { // main app reducers alert, dashboard, + parameters: combineReducers(parameters), home: combineReducers(home), pulse: combineReducers(pulse), qb: combineReducers(qb), diff --git a/frontend/src/metabase/reducers-public.js b/frontend/src/metabase/reducers-public.js index c237c79489a59e446d9f1d2c49c153097bc1d921..629b601d8bad847dd6a2cb21d48540998dccdae4 100644 --- a/frontend/src/metabase/reducers-public.js +++ b/frontend/src/metabase/reducers-public.js @@ -1,8 +1,11 @@ // Reducers needed for public questions and dashboards +import { combineReducers } from "redux"; import dashboard from "metabase/dashboard/reducers"; +import * as parameters from "metabase/parameters/reducers"; import commonReducers from "./reducers-common"; export default { ...commonReducers, dashboard, + parameters: combineReducers(parameters), }; diff --git a/frontend/test/__support__/server-mocks/dataset.ts b/frontend/test/__support__/server-mocks/dataset.ts new file mode 100644 index 0000000000000000000000000000000000000000..b87e3ca5d39e69a3ea7c5c24784f601bbbf32373 --- /dev/null +++ b/frontend/test/__support__/server-mocks/dataset.ts @@ -0,0 +1,9 @@ +import { Scope } from "nock"; +import { ParameterValues } from "metabase-types/api"; + +export function setupParameterValuesEndpoints( + scope: Scope, + parameterValues: ParameterValues, +) { + scope.post("/api/dataset/parameter/values").reply(200, parameterValues); +} diff --git a/frontend/test/__support__/server-mocks/index.ts b/frontend/test/__support__/server-mocks/index.ts index c2987d4270a526ef1e27eef07136cf29fc593163..2b1094ae0fa2bd89b00207f00990ccce27952cbc 100644 --- a/frontend/test/__support__/server-mocks/index.ts +++ b/frontend/test/__support__/server-mocks/index.ts @@ -2,5 +2,6 @@ export * from "./action"; export * from "./card"; export * from "./collection"; export * from "./database"; +export * from "./dataset"; export * from "./field"; export * from "./table"; diff --git a/frontend/test/metabase/scenarios/native-filters/helpers/e2e-field-filter-helpers.js b/frontend/test/metabase/scenarios/native-filters/helpers/e2e-field-filter-helpers.js index 0fdad6b446e459dbf49d351aece409a6b9d15c61..2c08d3e920d2fbc861899628aa36d0e2f9eca2de 100644 --- a/frontend/test/metabase/scenarios/native-filters/helpers/e2e-field-filter-helpers.js +++ b/frontend/test/metabase/scenarios/native-filters/helpers/e2e-field-filter-helpers.js @@ -123,6 +123,12 @@ export function openEntryForm(isFilterRequired) { selector.click(); } +export function closeEntryForm() { + popover().within(() => { + cy.get("input").type("{esc}"); + }); +} + // LOCAL SCOPE /** diff --git a/frontend/test/metabase/scenarios/native-filters/sql-filters-source.cy.spec.js b/frontend/test/metabase/scenarios/native-filters/sql-filters-source.cy.spec.js index 9726c5bf88998e4897e0bd2df30a4a399ab8699d..b9614fe222df8cf4e5747ac8bd10abd2a3225392 100644 --- a/frontend/test/metabase/scenarios/native-filters/sql-filters-source.cy.spec.js +++ b/frontend/test/metabase/scenarios/native-filters/sql-filters-source.cy.spec.js @@ -7,6 +7,7 @@ import { visitEmbeddedPage, visitPublicQuestion, popover, + modal, } from "__support__/e2e/helpers"; import { SAMPLE_DATABASE } from "__support__/e2e/cypress_sample_database"; import * as SQLFilter from "./helpers/e2e-sql-filter-helpers"; @@ -40,6 +41,10 @@ describe("scenarios > filters > sql filters > values source", () => { cy.intercept("GET", "/api/session/properties").as("sessionProperties"); cy.intercept("PUT", "/api/card/*").as("updateQuestion"); cy.intercept("POST", "/api/card/*/query").as("cardQuery"); + cy.intercept("POST", "/api/dataset/parameter/values").as("parameterValues"); + cy.intercept("GET", "/api/card/*/params/*/values").as( + "cardParameterValues", + ); }); describe("structured question source", () => { @@ -93,6 +98,41 @@ describe("scenarios > filters > sql filters > values source", () => { SQLFilter.runQuery("dataset"); }); + it("should properly cache parameter values api calls", () => { + cy.createQuestion(structuredSourceQuestion); + openNativeEditor(); + SQLFilter.enterParameterizedQuery( + "SELECT * FROM PRODUCTS WHERE CATEGORY = {{tag}}", + ); + + setFilterQuestionSource({ question: "MBQL source", field: "Category" }); + FieldFilter.openEntryForm(); + cy.wait("@parameterValues"); + checkFilterValueInList("Gadget"); + FieldFilter.closeEntryForm(); + FieldFilter.openEntryForm(); + checkFilterValueInList("Gadget"); + cy.get("@parameterValues.all").should("have.length", 1); + setFilterListSource({ values: ["A", "B"] }); + FieldFilter.openEntryForm(); + cy.wait("@parameterValues"); + checkFilterValueInList("A"); + + saveQuestion("SQL filter"); + FieldFilter.openEntryForm(); + cy.wait("@cardParameterValues"); + checkFilterValueInList("A"); + FieldFilter.closeEntryForm(); + FieldFilter.openEntryForm(); + checkFilterValueInList("A"); + cy.get("@cardParameterValues.all").should("have.length", 1); + setFilterQuestionSource({ question: "MBQL source", field: "Category" }); + updateQuestion(); + FieldFilter.openEntryForm(); + cy.wait("@cardParameterValues"); + checkFilterValueInList("Gadget"); + }); + it("should be able to use a structured question source when embedded", () => { cy.createQuestion(structuredSourceQuestion).then( ({ body: { id: sourceQuestionId } }) => { @@ -299,6 +339,18 @@ const getListTargetQuestion = () => { }); }; +const updateQuestion = () => { + cy.findByText("Save").click(); + modal().button("Save").click(); + cy.wait("@updateQuestion"); +}; + +const checkFilterValueInList = value => { + popover().within(() => { + cy.findByText(value).should("exist"); + }); +}; + const checkFilterValueNotInList = value => { popover().within(() => { cy.findByText(value).should("not.exist");