diff --git a/frontend/src/metabase-lib/Question.ts b/frontend/src/metabase-lib/Question.ts index 48640b541e819004dac5853def100f7880d9f78f..4737804d21bdf8da64ff8876f2dfe904bb368707 100644 --- a/frontend/src/metabase-lib/Question.ts +++ b/frontend/src/metabase-lib/Question.ts @@ -580,7 +580,7 @@ class QuestionInner { type: "query", database: this.databaseId(), query: { - "source-table": getQuestionVirtualTableId(this.card()), + "source-table": getQuestionVirtualTableId(this.id()), }, }, }; @@ -597,7 +597,7 @@ class QuestionInner { type: "query", database: this.databaseId(), query: { - "source-table": getQuestionVirtualTableId(this.card()), + "source-table": getQuestionVirtualTableId(this.id()), }, }); } @@ -1005,7 +1005,7 @@ class QuestionInner { if (this.isDataset() && this.isSaved()) { dependencies.push({ type: "table", - id: getQuestionVirtualTableId(this.card()), + id: getQuestionVirtualTableId(this.id()), }); } diff --git a/frontend/src/metabase-lib/metadata/utils/models.ts b/frontend/src/metabase-lib/metadata/utils/models.ts index 3a9ae3c9f80d62f23e01c3fe9087eda71993032d..953a9bcd1768060c869c57f1b13d7cb9476a5b39 100644 --- a/frontend/src/metabase-lib/metadata/utils/models.ts +++ b/frontend/src/metabase-lib/metadata/utils/models.ts @@ -122,7 +122,7 @@ export function isAdHocModelQuestionCard(card: Card, originalCard?: Card) { const isSameCard = card.id === originalCard.id; const { query } = card.dataset_query as StructuredDatasetQuery; const isSelfReferencing = - query["source-table"] === getQuestionVirtualTableId(originalCard); + query["source-table"] === getQuestionVirtualTableId(originalCard.id); return isModel && isSameCard && isSelfReferencing; } diff --git a/frontend/src/metabase-lib/metadata/utils/saved-questions.js b/frontend/src/metabase-lib/metadata/utils/saved-questions.js index 1edeb3c6d34f8605a150e03f2b979dac6a3550f6..6c7eed9ecf8c9f513efca33968bc7b54b5bcdd9e 100644 --- a/frontend/src/metabase-lib/metadata/utils/saved-questions.js +++ b/frontend/src/metabase-lib/metadata/utils/saved-questions.js @@ -28,8 +28,8 @@ export function getRootCollectionVirtualSchemaId({ isModels }) { return getCollectionVirtualSchemaId(null, { isDatasets: isModels }); } -export function getQuestionVirtualTableId(card) { - return `card__${card.id}`; +export function getQuestionVirtualTableId(id) { + return `card__${id}`; } export function isVirtualCardId(tableId) { @@ -46,7 +46,7 @@ export function getQuestionIdFromVirtualTableId(tableId) { export function convertSavedQuestionToVirtualTable(card) { return { - id: getQuestionVirtualTableId(card), + id: getQuestionVirtualTableId(card.id), display_name: card.name, description: card.description, moderated_status: card.moderated_status, diff --git a/frontend/src/metabase-lib/metadata/utils/saved-questions.unit.spec.js b/frontend/src/metabase-lib/metadata/utils/saved-questions.unit.spec.js index 91d1ec0ada61b8306baf60224c3de2072274b3b4..b6ca3c67661c26317f8db7a52e24f4ab5dd3b67b 100644 --- a/frontend/src/metabase-lib/metadata/utils/saved-questions.unit.spec.js +++ b/frontend/src/metabase-lib/metadata/utils/saved-questions.unit.spec.js @@ -82,7 +82,7 @@ describe("saved question helpers", () => { describe("getQuestionVirtualTableId", () => { it("returns question prefixed question ID", () => { - expect(getQuestionVirtualTableId({ id: 7 })).toBe("card__7"); + expect(getQuestionVirtualTableId(7)).toBe("card__7"); }); }); diff --git a/frontend/src/metabase-lib/queries/utils/nested-card-query-table.ts b/frontend/src/metabase-lib/queries/utils/nested-card-query-table.ts index 732f899cbee64c4591bf64a6088f59479cac3af3..be42ed5f23326c92f1b1bae9076611cdde58445f 100644 --- a/frontend/src/metabase-lib/queries/utils/nested-card-query-table.ts +++ b/frontend/src/metabase-lib/queries/utils/nested-card-query-table.ts @@ -60,7 +60,7 @@ function createVirtualTableUsingQuestionMetadata(question: Question): Table { }); return createVirtualTable({ - id: getQuestionVirtualTableId(question.card()), + id: getQuestionVirtualTableId(question.id()), name: questionDisplayName, display_name: questionDisplayName, db: question?.database(), diff --git a/frontend/src/metabase-types/api/mocks/parameters.ts b/frontend/src/metabase-types/api/mocks/parameters.ts index 6cb34f605eabe4119e9aa0a81111f6a3626bb78c..ceb6e138021a6b32936ad89843061006fe65a746 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, ParameterSourceConfig } from "metabase-types/api"; +import { Parameter, ValuesSourceConfig } from "metabase-types/api"; export const createMockParameter = (opts?: Partial<Parameter>): Parameter => ({ id: "1", @@ -8,8 +8,8 @@ export const createMockParameter = (opts?: Partial<Parameter>): Parameter => ({ ...opts, }); -export const createMockParameterSourceOptions = ( - opts?: Partial<ParameterSourceConfig>, -): ParameterSourceConfig => ({ +export const createMockValuesSourceConfig = ( + opts?: Partial<ValuesSourceConfig>, +): ValuesSourceConfig => ({ ...opts, }); diff --git a/frontend/src/metabase-types/api/parameters.ts b/frontend/src/metabase-types/api/parameters.ts index e4441fd7277024c30b54618420156aad37acfda8..ff3330c1b06953624a17a5143e8c86c783109a3c 100644 --- a/frontend/src/metabase-types/api/parameters.ts +++ b/frontend/src/metabase-types/api/parameters.ts @@ -1,3 +1,5 @@ +import { CardId } from "./card"; + export type StringParameterType = | "string/=" | "string/!=" @@ -40,12 +42,14 @@ export interface Parameter { filteringParameters?: ParameterId[]; isMultiSelect?: boolean; value?: any; - values_source_type?: ParameterSourceType; - values_source_config?: ParameterSourceConfig; + values_source_type?: ValuesSourceType; + values_source_config?: ValuesSourceConfig; } -export type ParameterSourceType = null | "static-list"; +export type ValuesSourceType = null | "card" | "static-list"; -export interface ParameterSourceConfig { +export interface ValuesSourceConfig { values?: string[]; + card_id?: CardId; + value_field?: unknown[]; } diff --git a/frontend/src/metabase/components/ListField/ListField.tsx b/frontend/src/metabase/components/ListField/ListField.tsx index 884c2f887b6d9a14923be9b2771d310f287e7ca4..34a89cecb33ffab5e5a702df2e3def175664c1e5 100644 --- a/frontend/src/metabase/components/ListField/ListField.tsx +++ b/frontend/src/metabase/components/ListField/ListField.tsx @@ -129,8 +129,8 @@ const ListField = ({ )} <OptionsList isDashboardFilter={isDashboardFilter}> - {filteredOptions.map(option => ( - <OptionContainer key={option[0]}> + {filteredOptions.map((option, index) => ( + <OptionContainer key={index}> <Checkbox data-testid={`${option[0]}-filter-value`} checkedColor={ diff --git a/frontend/src/metabase/containers/DataPicker/DataSearch/DataSearch.tsx b/frontend/src/metabase/containers/DataPicker/DataSearch/DataSearch.tsx index f4e93c90ffaec423b1db9c9403a3639da8be3989..8bc6ecda2a139ac5a9da9597e9d3d409bde4cd36 100644 --- a/frontend/src/metabase/containers/DataPicker/DataSearch/DataSearch.tsx +++ b/frontend/src/metabase/containers/DataPicker/DataSearch/DataSearch.tsx @@ -23,6 +23,7 @@ interface DataSearchProps { } type TableSearchResult = { + id: number; database_id: number; table_schema: string; table_id: number; @@ -71,7 +72,7 @@ function getValueForVirtualTable(table: TableSearchResult): DataPickerValue { databaseId: SAVED_QUESTIONS_VIRTUAL_DB_ID, schemaId, collectionId: table.collection?.id || "root", - tableIds: [getQuestionVirtualTableId(table)], + tableIds: [getQuestionVirtualTableId(table.id)], }; } diff --git a/frontend/src/metabase/containers/DataPicker/DataTypePicker/DataTypePicker.styled.tsx b/frontend/src/metabase/containers/DataPicker/DataTypePicker/DataTypePicker.styled.tsx index 40093aff987de026815db7ddc28e14d4e04f19b3..2d947a345b6aa3e3445e3b8c50820d1e2ce129aa 100644 --- a/frontend/src/metabase/containers/DataPicker/DataTypePicker/DataTypePicker.styled.tsx +++ b/frontend/src/metabase/containers/DataPicker/DataTypePicker/DataTypePicker.styled.tsx @@ -7,8 +7,6 @@ import { color } from "metabase/lib/colors"; import { space } from "metabase/styled-components/theme"; export const List = styled(SelectList)` - padding: ${space(0)} ${space(1)} 12px ${space(1)}; - ${SelectList.BaseItem.Root} { &:hover { background-color: ${color("brand")}; diff --git a/frontend/src/metabase/containers/DataPicker/PanePicker/PanePicker.styled.tsx b/frontend/src/metabase/containers/DataPicker/PanePicker/PanePicker.styled.tsx index 99019f734e104b5b21aa5ac6fd4265a4a60524f4..a89e834668498061c28d2722d56165a1c6a2b9d7 100644 --- a/frontend/src/metabase/containers/DataPicker/PanePicker/PanePicker.styled.tsx +++ b/frontend/src/metabase/containers/DataPicker/PanePicker/PanePicker.styled.tsx @@ -37,7 +37,6 @@ export const BackButton = styled.a` color: ${color("text-dark")}; font-weight: 700; - margin-left: 1rem; padding-bottom: 1rem; &:hover { diff --git a/frontend/src/metabase/containers/DataPicker/useDataPickerValue.ts b/frontend/src/metabase/containers/DataPicker/useDataPickerValue.ts index 8a20406c0af67b0e719b1f7768e55a563b945cb4..c89f235d6d47e7dbe03276dcbca8f11eb4d85679 100644 --- a/frontend/src/metabase/containers/DataPicker/useDataPickerValue.ts +++ b/frontend/src/metabase/containers/DataPicker/useDataPickerValue.ts @@ -40,12 +40,18 @@ function cleanCollectionValue({ } function cleanValue(value: Partial<DataPickerValue>): DataPickerValue { + const type = value.type; + const databaseId = cleanDatabaseValue({ ...value, type }); + const schemaId = cleanSchemaValue({ ...value, databaseId }); + const collectionId = cleanCollectionValue({ ...value, type, databaseId }); + const tableIds = cleanTablesValue({ ...value, databaseId, schemaId }); + return { - type: value.type, - databaseId: cleanDatabaseValue(value), - schemaId: cleanSchemaValue(value), - collectionId: cleanCollectionValue(value), - tableIds: cleanTablesValue(value), + type, + databaseId, + schemaId, + collectionId, + tableIds, }; } diff --git a/frontend/src/metabase/dashboard/actions/parameters.js b/frontend/src/metabase/dashboard/actions/parameters.js index d8bd498b10c3a742967e866d773512dd76b7724a..003e8cdbc3c925f081fd40025f5573836f524c4a 100644 --- a/frontend/src/metabase/dashboard/actions/parameters.js +++ b/frontend/src/metabase/dashboard/actions/parameters.js @@ -207,16 +207,16 @@ export const setParameterSourceType = createThunkAction( }, ); -export const SET_PARAMETER_SOURCE_OPTIONS = - "metabase/dashboard/SET_PARAMETER_SOURCE_OPTIONS"; -export const setParameterSourceOptions = createThunkAction( - SET_PARAMETER_SOURCE_OPTIONS, - (parameterId, sourceOptions) => (dispatch, getState) => { +export const SET_PARAMETER_SOURCE_CONFIG = + "metabase/dashboard/SET_PARAMETER_SOURCE_CONFIG"; +export const setParameterSourceConfig = createThunkAction( + SET_PARAMETER_SOURCE_CONFIG, + (parameterId, sourceConfig) => (dispatch, getState) => { updateParameter(dispatch, getState, parameterId, parameter => ({ ...parameter, - values_source_config: sourceOptions, + values_source_config: sourceConfig, })); - return { id: parameterId, sourceOptions }; + return { id: parameterId, sourceConfig }; }, ); diff --git a/frontend/src/metabase/dashboard/components/DashboardSidebars.jsx b/frontend/src/metabase/dashboard/components/DashboardSidebars.jsx index bbc0a6704bbe4f3ffc0e6912fba3f90f7d9b4333..b757a135d172d5d4e70f49f667b8a826bb02de2a 100644 --- a/frontend/src/metabase/dashboard/components/DashboardSidebars.jsx +++ b/frontend/src/metabase/dashboard/components/DashboardSidebars.jsx @@ -28,7 +28,7 @@ DashboardSidebars.propTypes = { setParameterDefaultValue: PropTypes.func.isRequired, setParameterIsMultiSelect: PropTypes.func.isRequired, setParameterSourceType: PropTypes.func.isRequired, - setParameterSourceOptions: PropTypes.func.isRequired, + setParameterSourceConfig: PropTypes.func.isRequired, setParameterFilteringParameters: PropTypes.func.isRequired, dashcardData: PropTypes.object, isSharing: PropTypes.bool.isRequired, @@ -60,7 +60,7 @@ export function DashboardSidebars({ setParameterDefaultValue, setParameterIsMultiSelect, setParameterSourceType, - setParameterSourceOptions, + setParameterSourceConfig, setParameterFilteringParameters, dashcardData, isFullscreen, @@ -125,7 +125,7 @@ export function DashboardSidebars({ onChangeDefaultValue={setParameterDefaultValue} onChangeIsMultiSelect={setParameterIsMultiSelect} onChangeSourceType={setParameterSourceType} - onChangeSourceOptions={setParameterSourceOptions} + onChangeSourceConfig={setParameterSourceConfig} onChangeFilteringParameters={setParameterFilteringParameters} onRemoveParameter={removeParameter} onShowAddParameterPopover={showAddParameterPopover} diff --git a/frontend/src/metabase/entities/containers/EntityObjectLoader.jsx b/frontend/src/metabase/entities/containers/EntityObjectLoader.jsx index 1d2d2768c835ffcc57af937ab28a78693b27f501..afbcf19c1751eb2cc4c26cbe04d5a2bdfef30cd7 100644 --- a/frontend/src/metabase/entities/containers/EntityObjectLoader.jsx +++ b/frontend/src/metabase/entities/containers/EntityObjectLoader.jsx @@ -20,6 +20,7 @@ const CONSUMED_PROPS = [ "loadingAndErrorWrapper", "LoadingAndErrorWrapper", "selectorName", + "requestType", ]; // NOTE: Memoize entityQuery so we don't re-render even if a new but identical @@ -31,6 +32,7 @@ const getMemoizedEntityQuery = createMemoizedSelector( class EntityObjectLoaderInner extends React.Component { static defaultProps = { + requestType: "fetch", loadingAndErrorWrapper: true, LoadingAndErrorWrapper: LoadingAndErrorWrapper, reload: false, @@ -54,10 +56,15 @@ class EntityObjectLoaderInner extends React.Component { ); } + fetch = (query, options) => { + const fetch = this.props[this.props.requestType]; + return fetch(query, options); + }; + UNSAFE_componentWillMount() { - const { entityId, entityQuery, fetch, dispatchApiErrorEvent } = this.props; + const { entityId, entityQuery, dispatchApiErrorEvent } = this.props; if (entityId != null) { - fetch( + this.fetch( { id: entityId, ...entityQuery }, { reload: this.props.reload, @@ -72,7 +79,7 @@ class EntityObjectLoaderInner extends React.Component { nextProps.entityId !== this.props.entityId && nextProps.entityId != null ) { - nextProps.fetch( + this.fetch( { id: nextProps.entityId, ...nextProps.entityQuery }, { reload: nextProps.reload, @@ -122,7 +129,7 @@ class EntityObjectLoaderInner extends React.Component { } reload = () => { - return this.props.fetch( + return this.fetch( { id: this.props.entityId }, { reload: true, @@ -147,6 +154,7 @@ const EntityObjectLoader = _.compose( entityId, entityQuery, selectorName = "getObject", + requestType = "fetch", ...props }, ) => { @@ -157,13 +165,15 @@ const EntityObjectLoader = _.compose( entityQuery = entityQuery(state, props); } + const entityOptions = { entityId, requestType }; + return { entityId, entityQuery: getMemoizedEntityQuery(state, entityQuery), - object: entityDef.selectors[selectorName](state, { entityId }), - fetched: entityDef.selectors.getFetched(state, { entityId }), - loading: entityDef.selectors.getLoading(state, { entityId }), - error: entityDef.selectors.getError(state, { entityId }), + object: entityDef.selectors[selectorName](state, entityOptions), + fetched: entityDef.selectors.getFetched(state, entityOptions), + loading: entityDef.selectors.getLoading(state, entityOptions), + error: entityDef.selectors.getError(state, entityOptions), }; }, ), diff --git a/frontend/src/metabase/entities/schemas.js b/frontend/src/metabase/entities/schemas.js index 1da652bd58a0522763976f40b08d1aab0492606e..52a3bbfcab14adeba031823f00c222772019d699 100644 --- a/frontend/src/metabase/entities/schemas.js +++ b/frontend/src/metabase/entities/schemas.js @@ -63,7 +63,7 @@ export default createEntity({ if (!state[schema]) { return state; } - const virtualQuestionId = getQuestionVirtualTableId(question); + const virtualQuestionId = getQuestionVirtualTableId(question.id); return updateIn(state, [schema, "tables"], tables => addTableAvoidingDuplicates(tables, virtualQuestionId), ); @@ -82,7 +82,7 @@ export default createEntity({ isDatasets: question.dataset, }); - const virtualQuestionId = getQuestionVirtualTableId(question); + const virtualQuestionId = getQuestionVirtualTableId(question.id); const previousSchemaContainingTheQuestion = getPreviousSchemaContainingTheQuestion( state, diff --git a/frontend/src/metabase/entities/tables.js b/frontend/src/metabase/entities/tables.js index b9cc0445dfbbbc53725e94e6f61b0373ca703f31..686e460b5c8ed030bcbee313aa648f9af9c7ede3 100644 --- a/frontend/src/metabase/entities/tables.js +++ b/frontend/src/metabase/entities/tables.js @@ -158,7 +158,7 @@ const Tables = createEntity({ if (type === Questions.actionTypes.UPDATE && !error) { const card = payload.question; - const virtualQuestionId = getQuestionVirtualTableId(card); + const virtualQuestionId = getQuestionVirtualTableId(card.id); if (card.archived && state[virtualQuestionId]) { delete state[virtualQuestionId]; diff --git a/frontend/src/metabase/parameters/components/CardValuesSourceModal/CardStepModal.styled.tsx b/frontend/src/metabase/parameters/components/CardValuesSourceModal/CardStepModal.styled.tsx new file mode 100644 index 0000000000000000000000000000000000000000..aa97a2f3d6e4650847c683ac6f57079026c52ae3 --- /dev/null +++ b/frontend/src/metabase/parameters/components/CardValuesSourceModal/CardStepModal.styled.tsx @@ -0,0 +1,6 @@ +import styled from "@emotion/styled"; + +export const ModalBody = styled.div` + height: 50vh; + overflow-y: auto; +`; diff --git a/frontend/src/metabase/parameters/components/CardValuesSourceModal/CardStepModal.tsx b/frontend/src/metabase/parameters/components/CardValuesSourceModal/CardStepModal.tsx new file mode 100644 index 0000000000000000000000000000000000000000..d0e7fd1a98c02fc26c8f75e52e7d796767b5e4b8 --- /dev/null +++ b/frontend/src/metabase/parameters/components/CardValuesSourceModal/CardStepModal.tsx @@ -0,0 +1,123 @@ +import React, { useCallback } from "react"; +import { connect } from "react-redux"; +import { t } from "ttag"; +import _ from "underscore"; +import Button from "metabase/core/components/Button/Button"; +import ModalContent from "metabase/components/ModalContent"; +import DataPickerContainer, { + DataPickerValue, + useDataPickerValue, +} from "metabase/containers/DataPicker"; +import Questions from "metabase/entities/questions"; +import Collections from "metabase/entities/collections"; +import { getMetadata } from "metabase/selectors/metadata"; +import { Card, CardId, Collection } from "metabase-types/api"; +import { State } from "metabase-types/store"; +import Question from "metabase-lib/Question"; +import { + getCollectionVirtualSchemaId, + getQuestionIdFromVirtualTableId, + getQuestionVirtualTableId, +} from "metabase-lib/metadata/utils/saved-questions"; +import { ModalBody } from "./CardStepModal.styled"; + +interface CardStepModalOwnProps { + cardId: CardId | undefined; + onChangeCard: (cardId: CardId | undefined) => void; + onSubmit: () => void; + onClose: () => void; +} + +interface CardStepModalCardProps { + card: Card | undefined; +} + +interface CardStepModalCollectionProps { + collection: Collection | undefined; +} + +interface CardStepModalStateProps { + question: Question | undefined; +} + +type CardStepModalProps = CardStepModalOwnProps & + CardStepModalCardProps & + CardStepModalCollectionProps & + CardStepModalStateProps; + +const CardStepModal = ({ + question, + collection, + onChangeCard, + onSubmit, + onClose, +}: CardStepModalProps): JSX.Element => { + const initialValue = getInitialValue(question, collection); + const [value, setValue] = useDataPickerValue(initialValue); + const cardId = getCardIdFromValue(value); + + const handleSubmit = useCallback(() => { + onChangeCard(cardId); + onSubmit(); + }, [cardId, onChangeCard, onSubmit]); + + return ( + <ModalContent + title={t`Pick a model or question to use for the values of this widget`} + footer={[ + <Button key="cancel" onClick={onClose}>{t`Cancel`}</Button>, + <Button + key="submit" + primary + disabled={cardId == null} + onClick={handleSubmit} + >{t`Select column`}</Button>, + ]} + onClose={onClose} + > + <ModalBody> + <DataPickerContainer value={value} onChange={setValue} /> + </ModalBody> + </ModalContent> + ); +}; + +const getInitialValue = ( + question?: Question, + collection?: Collection, +): Partial<DataPickerValue> | undefined => { + if (question) { + const id = question.id(); + const isDatasets = question.isDataset(); + + return { + type: isDatasets ? "models" : "questions", + schemaId: getCollectionVirtualSchemaId(collection, { isDatasets }), + collectionId: collection?.id, + tableIds: [getQuestionVirtualTableId(id)], + }; + } +}; + +const getCardIdFromValue = ({ tableIds }: DataPickerValue) => { + if (tableIds.length) { + const cardId = getQuestionIdFromVirtualTableId(tableIds[0]); + if (cardId != null) { + return cardId; + } + } +}; + +export default _.compose( + Questions.load({ + id: (state: State, { cardId }: CardStepModalOwnProps) => cardId, + entityAlias: "card", + }), + Collections.load({ + id: (state: State, { card }: CardStepModalCardProps) => + card?.collection_id ?? "root", + }), + connect((state: State, { card }: CardStepModalCardProps) => ({ + question: card ? new Question(card, getMetadata(state)) : undefined, + })), +)(CardStepModal); diff --git a/frontend/src/metabase/parameters/components/CardValuesSourceModal/CardValuesSourceModal.tsx b/frontend/src/metabase/parameters/components/CardValuesSourceModal/CardValuesSourceModal.tsx new file mode 100644 index 0000000000000000000000000000000000000000..33d9b154f4f081e891a369726dd0311fe159b472 --- /dev/null +++ b/frontend/src/metabase/parameters/components/CardValuesSourceModal/CardValuesSourceModal.tsx @@ -0,0 +1,64 @@ +import React, { useCallback, useState } from "react"; +import { ValuesSourceConfig } from "metabase-types/api"; +import CardStepModal from "./CardStepModal"; +import FieldStepModal from "./FieldStepModal"; + +type ModalStep = "card" | "field"; + +export interface CardValuesSourceModalProps { + sourceConfig: ValuesSourceConfig; + onChangeSourceConfig: (sourceConfig: ValuesSourceConfig) => void; + onClose: () => void; +} + +const CardValuesSourceModal = ({ + sourceConfig, + onChangeSourceConfig, + onClose, +}: CardValuesSourceModalProps): JSX.Element | null => { + const [step, setStep] = useState<ModalStep>("card"); + const [cardId, setCardId] = useState(sourceConfig.card_id); + const [fieldReference, setFieldReference] = useState( + sourceConfig.value_field, + ); + + const handleCardSubmit = useCallback(() => { + setStep("field"); + }, []); + + const handleFieldSubmit = useCallback(() => { + onChangeSourceConfig({ card_id: cardId, value_field: fieldReference }); + onClose(); + }, [cardId, fieldReference, onChangeSourceConfig, onClose]); + + const handleFieldCancel = useCallback(() => { + setStep("card"); + }, []); + + switch (step) { + case "card": + return ( + <CardStepModal + cardId={cardId} + onChangeCard={setCardId} + onSubmit={handleCardSubmit} + onClose={onClose} + /> + ); + case "field": + return ( + <FieldStepModal + cardId={cardId} + fieldReference={fieldReference} + onChangeField={setFieldReference} + onSubmit={handleFieldSubmit} + onCancel={handleFieldCancel} + onClose={onClose} + /> + ); + default: + return null; + } +}; + +export default CardValuesSourceModal; diff --git a/frontend/src/metabase/parameters/components/CardValuesSourceModal/FieldStepModal.styled.tsx b/frontend/src/metabase/parameters/components/CardValuesSourceModal/FieldStepModal.styled.tsx new file mode 100644 index 0000000000000000000000000000000000000000..aa97a2f3d6e4650847c683ac6f57079026c52ae3 --- /dev/null +++ b/frontend/src/metabase/parameters/components/CardValuesSourceModal/FieldStepModal.styled.tsx @@ -0,0 +1,6 @@ +import styled from "@emotion/styled"; + +export const ModalBody = styled.div` + height: 50vh; + overflow-y: auto; +`; diff --git a/frontend/src/metabase/parameters/components/CardValuesSourceModal/FieldStepModal.tsx b/frontend/src/metabase/parameters/components/CardValuesSourceModal/FieldStepModal.tsx new file mode 100644 index 0000000000000000000000000000000000000000..a32b1d224b34bd850e962da58973f0fb17646e79 --- /dev/null +++ b/frontend/src/metabase/parameters/components/CardValuesSourceModal/FieldStepModal.tsx @@ -0,0 +1,97 @@ +import React, { useCallback, useMemo } from "react"; +import { t } from "ttag"; +import _ from "underscore"; +import Button from "metabase/core/components/Button/Button"; +import Select, { + Option, + SelectChangeEvent, +} from "metabase/core/components/Select"; +import ModalContent from "metabase/components/ModalContent"; +import Tables from "metabase/entities/tables"; +import { CardId } from "metabase-types/api"; +import { State } from "metabase-types/store"; +import Field from "metabase-lib/metadata/Field"; +import Table from "metabase-lib/metadata/Table"; +import { getQuestionVirtualTableId } from "metabase-lib/metadata/utils/saved-questions"; +import { ModalBody } from "./FieldStepModal.styled"; + +interface FieldStepModalOwnProps { + cardId: CardId | undefined; + fieldReference: unknown[] | undefined; + onChangeField: (field: unknown[]) => void; + onSubmit: () => void; + onCancel: () => void; + onClose: () => void; +} + +interface FieldStepModalTableProps { + table: Table; +} + +type FieldStepModalProps = FieldStepModalOwnProps & FieldStepModalTableProps; + +const FieldStepModal = ({ + table, + fieldReference, + onChangeField, + onSubmit, + onCancel, + onClose, +}: FieldStepModalProps): JSX.Element => { + const fields = useMemo(() => { + return getSupportedFields(table); + }, [table]); + + const selectedField = useMemo(() => { + return fieldReference && getFieldByReference(fields, fieldReference); + }, [fields, fieldReference]); + + const handleChange = useCallback( + (event: SelectChangeEvent<Field>) => { + onChangeField(event.target.value.reference()); + }, + [onChangeField], + ); + + return ( + <ModalContent + title={t`Which column from ${table.displayName()} should be used`} + footer={[ + <Button key="cancel" onClick={onCancel}>{t`Back`}</Button>, + <Button + key="submit" + primary + disabled={!selectedField} + onClick={onSubmit} + >{t`Done`}</Button>, + ]} + onClose={onClose} + > + <ModalBody> + <Select + value={selectedField} + placeholder={t`Pick a column`} + onChange={handleChange} + > + {fields.map((field, index) => ( + <Option key={index} name={field.displayName()} value={field} /> + ))} + </Select> + </ModalBody> + </ModalContent> + ); +}; + +const getFieldByReference = (fields: Field[], fieldReference: unknown[]) => { + return fields.find(field => _.isEqual(field.reference(), fieldReference)); +}; + +const getSupportedFields = (table: Table) => { + return table.fields.filter(field => field.isString()); +}; + +export default Tables.load({ + id: (state: State, { cardId }: FieldStepModalOwnProps) => + getQuestionVirtualTableId(cardId), + requestType: "fetchMetadata", +})(FieldStepModal); diff --git a/frontend/src/metabase/parameters/components/CardValuesSourceModal/index.ts b/frontend/src/metabase/parameters/components/CardValuesSourceModal/index.ts new file mode 100644 index 0000000000000000000000000000000000000000..9d270024f7990fd0bdd5456f564ec4e3d2d717e0 --- /dev/null +++ b/frontend/src/metabase/parameters/components/CardValuesSourceModal/index.ts @@ -0,0 +1 @@ +export { default } from "./CardValuesSourceModal"; diff --git a/frontend/src/metabase/parameters/components/ParameterListSourceModal/ParameterListSourceModal.styled.tsx b/frontend/src/metabase/parameters/components/ListValuesSourceModal/ListValuesSourceModal.styled.tsx similarity index 100% rename from frontend/src/metabase/parameters/components/ParameterListSourceModal/ParameterListSourceModal.styled.tsx rename to frontend/src/metabase/parameters/components/ListValuesSourceModal/ListValuesSourceModal.styled.tsx diff --git a/frontend/src/metabase/parameters/components/ParameterListSourceModal/ParameterListSourceModal.tsx b/frontend/src/metabase/parameters/components/ListValuesSourceModal/ListValuesSourceModal.tsx similarity index 55% rename from frontend/src/metabase/parameters/components/ParameterListSourceModal/ParameterListSourceModal.tsx rename to frontend/src/metabase/parameters/components/ListValuesSourceModal/ListValuesSourceModal.tsx index baf3ab1585f997e51b55e5b01af416b4c4dcfb3a..978f70fb64a1dedab6a2fa00e31dad4983e269f2 100644 --- a/frontend/src/metabase/parameters/components/ParameterListSourceModal/ParameterListSourceModal.tsx +++ b/frontend/src/metabase/parameters/components/ListValuesSourceModal/ListValuesSourceModal.tsx @@ -2,27 +2,25 @@ import React, { ChangeEvent, useCallback, useState } from "react"; import { t } from "ttag"; import Button from "metabase/core/components/Button"; import ModalContent from "metabase/components/ModalContent"; -import { getSourceOptions } from "metabase/parameters/utils/dashboards"; -import { ParameterSourceConfig, ParameterSourceType } from "metabase-types/api"; -import { UiParameter } from "metabase-lib/parameters/types"; -import { ModalMessage, ModalTextArea } from "./ParameterListSourceModal.styled"; +import { ValuesSourceConfig } from "metabase-types/api"; +import { ModalMessage, ModalTextArea } from "./ListValuesSourceModal.styled"; const NEW_LINE = "\n"; const PLACEHOLDER = [t`banana`, t`orange`].join(NEW_LINE); -export interface ParameterListSourceModalProps { - parameter: UiParameter; - onChangeSourceOptions: (sourceOptions: ParameterSourceConfig) => void; - onClose?: () => void; +export interface ListValuesSourceModalProps { + sourceConfig: ValuesSourceConfig; + onChangeSourceConfig: (sourceConfig: ValuesSourceConfig) => void; + onClose: () => void; } -const ParameterListSourceModal = ({ - parameter, - onChangeSourceOptions, +const ListValuesSourceModal = ({ + sourceConfig, + onChangeSourceConfig, onClose, -}: ParameterListSourceModalProps): JSX.Element => { - const options = getSourceOptions(parameter); - const [value, setValue] = useState(getInputValue(options.values)); +}: ListValuesSourceModalProps): JSX.Element => { + const [value, setValue] = useState(getInputValue(sourceConfig.values)); + const isEmpty = !value.trim().length; const handleChange = useCallback( (event: ChangeEvent<HTMLTextAreaElement>) => { @@ -32,16 +30,21 @@ const ParameterListSourceModal = ({ ); const handleSubmit = useCallback(() => { - onChangeSourceOptions({ values: getSourceValues(value) }); - onClose?.(); - }, [value, onChangeSourceOptions, onClose]); + onChangeSourceConfig({ values: getSourceValues(value) }); + onClose(); + }, [value, onChangeSourceConfig, onClose]); return ( <ModalContent title={t`Create a custom list`} footer={[ <Button key="cancel" onClick={onClose}>{t`Cancel`}</Button>, - <Button key="submit" primary onClick={handleSubmit}>{t`Done`}</Button>, + <Button + key="submit" + primary + disabled={isEmpty} + onClick={handleSubmit} + >{t`Done`}</Button>, ]} onClose={onClose} > @@ -70,4 +73,4 @@ const getSourceValues = (value: string) => { .filter(line => line.length > 0); }; -export default ParameterListSourceModal; +export default ListValuesSourceModal; diff --git a/frontend/src/metabase/parameters/components/ListValuesSourceModal/ListValuesSourceModal.unit.spec.tsx b/frontend/src/metabase/parameters/components/ListValuesSourceModal/ListValuesSourceModal.unit.spec.tsx new file mode 100644 index 0000000000000000000000000000000000000000..2188d327ca68966ccc0618bcadd6c589596ded38 --- /dev/null +++ b/frontend/src/metabase/parameters/components/ListValuesSourceModal/ListValuesSourceModal.unit.spec.tsx @@ -0,0 +1,46 @@ +import React from "react"; +import { render, screen } from "@testing-library/react"; +import userEvent, { specialChars } from "@testing-library/user-event"; +import { createMockValuesSourceConfig } from "metabase-types/api/mocks"; +import ListValuesSourceModal, { + ListValuesSourceModalProps, +} from "./ListValuesSourceModal"; + +describe("ListValuesSourceModal", () => { + it("should trim and set source values", () => { + const props = getProps(); + + render(<ListValuesSourceModal {...props} />); + + const input = screen.getByRole("textbox"); + userEvent.type(input, `Gadget ${specialChars.enter}`); + userEvent.type(input, `Widget ${specialChars.enter}`); + userEvent.click(screen.getByText("Done")); + + expect(props.onChangeSourceConfig).toHaveBeenCalledWith({ + values: ["Gadget", "Widget"], + }); + }); + + it("should not allow to submit empty values", () => { + const props = getProps({ + sourceConfig: createMockValuesSourceConfig({ + values: ["Gadget", "Gizmo"], + }), + }); + + render(<ListValuesSourceModal {...props} />); + userEvent.clear(screen.getByRole("textbox")); + + expect(screen.getByRole("button", { name: "Done" })).toBeDisabled(); + }); +}); + +const getProps = ( + opts?: Partial<ListValuesSourceModalProps>, +): ListValuesSourceModalProps => ({ + sourceConfig: createMockValuesSourceConfig(), + onChangeSourceConfig: jest.fn(), + onClose: jest.fn(), + ...opts, +}); diff --git a/frontend/src/metabase/parameters/components/ListValuesSourceModal/index.ts b/frontend/src/metabase/parameters/components/ListValuesSourceModal/index.ts new file mode 100644 index 0000000000000000000000000000000000000000..93b838895a5dfc4530955158158c1d13acbbea13 --- /dev/null +++ b/frontend/src/metabase/parameters/components/ListValuesSourceModal/index.ts @@ -0,0 +1 @@ +export { default } from "./ListValuesSourceModal"; diff --git a/frontend/src/metabase/parameters/components/ParameterListSourceModal/ParameterListSourceModal.unit.spec.tsx b/frontend/src/metabase/parameters/components/ParameterListSourceModal/ParameterListSourceModal.unit.spec.tsx deleted file mode 100644 index c5f974c4d1941024b928b8c00b5504055ba86749..0000000000000000000000000000000000000000 --- a/frontend/src/metabase/parameters/components/ParameterListSourceModal/ParameterListSourceModal.unit.spec.tsx +++ /dev/null @@ -1,50 +0,0 @@ -import React from "react"; -import { render, screen } from "@testing-library/react"; -import userEvent, { specialChars } from "@testing-library/user-event"; -import { createMockParameterSourceOptions } from "metabase-types/api/mocks"; -import { createMockUiParameter } from "metabase-lib/mocks"; -import ParameterListSourceModal, { - ParameterListSourceModalProps, -} from "./ParameterListSourceModal"; - -describe("ParameterListSourceModal", () => { - it("should trim and set source values", () => { - const props = getProps(); - - render(<ParameterListSourceModal {...props} />); - - const input = screen.getByRole("textbox"); - userEvent.type(input, `Gadget ${specialChars.enter}`); - userEvent.type(input, `Widget ${specialChars.enter}`); - userEvent.click(screen.getByText("Done")); - - expect(props.onChangeSourceOptions).toHaveBeenCalledWith({ - values: ["Gadget", "Widget"], - }); - }); - - it("should clear source values", () => { - const props = getProps({ - parameter: createMockUiParameter({ - values_source_config: createMockParameterSourceOptions({ - values: ["Gadget", "Gizmo"], - }), - }), - }); - - render(<ParameterListSourceModal {...props} />); - userEvent.clear(screen.getByRole("textbox")); - userEvent.click(screen.getByText("Done")); - - expect(props.onChangeSourceOptions).toHaveBeenCalledWith({ values: [] }); - }); -}); - -const getProps = ( - opts?: Partial<ParameterListSourceModalProps>, -): ParameterListSourceModalProps => ({ - parameter: createMockUiParameter(), - onChangeSourceOptions: jest.fn(), - onClose: jest.fn(), - ...opts, -}); diff --git a/frontend/src/metabase/parameters/components/ParameterListSourceModal/index.ts b/frontend/src/metabase/parameters/components/ParameterListSourceModal/index.ts deleted file mode 100644 index 151a7d90444ca21ddbff68d86e025bda31bfe366..0000000000000000000000000000000000000000 --- a/frontend/src/metabase/parameters/components/ParameterListSourceModal/index.ts +++ /dev/null @@ -1 +0,0 @@ -export { default } from "./ParameterListSourceModal"; diff --git a/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.tsx b/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.tsx index 3483a70a02a06e1822a9c9c2a9cb002b7106fe0e..6722cc369568dffe431c0ef301b4fef084f0bc30 100644 --- a/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.tsx +++ b/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.tsx @@ -8,7 +8,7 @@ import React, { import { t } from "ttag"; import Input from "metabase/core/components/Input"; import Radio from "metabase/core/components/Radio"; -import { ParameterSourceConfig, ParameterSourceType } from "metabase-types/api"; +import { ValuesSourceConfig, ValuesSourceType } from "metabase-types/api"; import { UiParameter } from "metabase-lib/parameters/types"; import { getIsMultiSelect } from "../../utils/dashboards"; import { @@ -34,8 +34,8 @@ export interface ParameterSettingsProps { onChangeName: (name: string) => void; onChangeDefaultValue: (value: unknown) => void; onChangeIsMultiSelect: (isMultiSelect: boolean) => void; - onChangeSourceType: (sourceType: ParameterSourceType) => void; - onChangeSourceOptions: (sourceOptions: ParameterSourceConfig) => void; + onChangeSourceType: (sourceType: ValuesSourceType) => void; + onChangeSourceConfig: (sourceOptions: ValuesSourceConfig) => void; onRemoveParameter: () => void; } @@ -43,7 +43,7 @@ const ParameterSettings = ({ parameter, onChangeName, onChangeSourceType, - onChangeSourceOptions, + onChangeSourceConfig, onChangeDefaultValue, onChangeIsMultiSelect, onRemoveParameter, @@ -60,7 +60,7 @@ const ParameterSettings = ({ <ParameterSourceSettings parameter={parameter} onChangeSourceType={onChangeSourceType} - onChangeSourceOptions={onChangeSourceOptions} + onChangeSourceConfig={onChangeSourceConfig} /> </SettingSection> )} diff --git a/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.unit.spec.tsx b/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.unit.spec.tsx index 8a17379cec2785e2e7347dd38d2160113acc5ab5..cfeb4a64b44dd6635d1bdfbf57cd71de324bd9bc 100644 --- a/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.unit.spec.tsx +++ b/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.unit.spec.tsx @@ -1,6 +1,5 @@ import React from "react"; import { render, screen } from "@testing-library/react"; -import userEvent from "@testing-library/user-event"; import { createMockUiParameter } from "metabase-lib/mocks"; import ParameterSettings, { ParameterSettingsProps } from "./ParameterSettings"; @@ -38,7 +37,7 @@ const getProps = ( onChangeDefaultValue: jest.fn(), onChangeIsMultiSelect: jest.fn(), onChangeSourceType: jest.fn(), - onChangeSourceOptions: jest.fn(), + onChangeSourceConfig: jest.fn(), onRemoveParameter: jest.fn(), ...opts, }); diff --git a/frontend/src/metabase/parameters/components/ParameterSidebar/ParameterSidebar.tsx b/frontend/src/metabase/parameters/components/ParameterSidebar/ParameterSidebar.tsx index 7799d29f2aba00aef004b3c3083ebca8a995b3e2..60d9cea10ab5004e56c23be8116f639871fc2750 100644 --- a/frontend/src/metabase/parameters/components/ParameterSidebar/ParameterSidebar.tsx +++ b/frontend/src/metabase/parameters/components/ParameterSidebar/ParameterSidebar.tsx @@ -4,8 +4,8 @@ import Radio from "metabase/core/components/Radio"; import Sidebar from "metabase/dashboard/components/Sidebar"; import { ParameterId, - ParameterSourceConfig, - ParameterSourceType, + ValuesSourceConfig, + ValuesSourceType, } from "metabase-types/api"; import { UiParameter } from "metabase-lib/parameters/types"; import { canUseLinkedFilters } from "../../utils/linked-filters"; @@ -24,11 +24,11 @@ export interface ParameterSidebarProps { ) => void; onChangeSourceType: ( parameterId: ParameterId, - sourceType: ParameterSourceType, + sourceType: ValuesSourceType, ) => void; - onChangeSourceOptions: ( + onChangeSourceConfig: ( parameterId: ParameterId, - sourceOptions: ParameterSourceConfig, + sourceOptions: ValuesSourceConfig, ) => void; onChangeFilteringParameters: ( parameterId: ParameterId, @@ -46,7 +46,7 @@ const ParameterSidebar = ({ onChangeDefaultValue, onChangeIsMultiSelect, onChangeSourceType, - onChangeSourceOptions, + onChangeSourceConfig, onChangeFilteringParameters, onRemoveParameter, onShowAddParameterPopover, @@ -78,17 +78,17 @@ const ParameterSidebar = ({ ); const handleSourceTypeChange = useCallback( - (sourceType: ParameterSourceType) => { + (sourceType: ValuesSourceType) => { onChangeSourceType(parameterId, sourceType); }, [parameterId, onChangeSourceType], ); - const handleSourceOptionsChange = useCallback( - (sourceOptions: ParameterSourceConfig) => { - onChangeSourceOptions(parameterId, sourceOptions); + const handleSourceConfigChange = useCallback( + (sourceOptions: ValuesSourceConfig) => { + onChangeSourceConfig(parameterId, sourceOptions); }, - [parameterId, onChangeSourceOptions], + [parameterId, onChangeSourceConfig], ); const handleFilteringParametersChange = useCallback( @@ -121,7 +121,7 @@ const ParameterSidebar = ({ onChangeDefaultValue={handleDefaultValueChange} onChangeIsMultiSelect={handleIsMultiSelectChange} onChangeSourceType={handleSourceTypeChange} - onChangeSourceOptions={handleSourceOptionsChange} + onChangeSourceConfig={handleSourceConfigChange} onRemoveParameter={handleRemove} /> ) : ( diff --git a/frontend/src/metabase/parameters/components/ParameterSourceSettings/ParameterSourceSettings.tsx b/frontend/src/metabase/parameters/components/ParameterSourceSettings/ParameterSourceSettings.tsx index 80b9b5f2587cd5d4b6527a350ed7dea11d333d35..7ba3740129f58200ba22fde77a67d0d8f92ac975 100644 --- a/frontend/src/metabase/parameters/components/ParameterSourceSettings/ParameterSourceSettings.tsx +++ b/frontend/src/metabase/parameters/components/ParameterSourceSettings/ParameterSourceSettings.tsx @@ -3,13 +3,13 @@ import { t } from "ttag"; import Radio from "metabase/core/components/Radio/Radio"; import Modal from "metabase/components/Modal"; import { - getSourceOptions, + getSourceConfig, getSourceType, - hasValidSourceOptions, } from "metabase/parameters/utils/dashboards"; -import { ParameterSourceConfig, ParameterSourceType } from "metabase-types/api"; +import { ValuesSourceConfig, ValuesSourceType } from "metabase-types/api"; import { UiParameter } from "metabase-lib/parameters/types"; -import ParameterListSourceModal from "../ParameterListSourceModal"; +import CardValuesSourceModal from "../CardValuesSourceModal"; +import ListValuesSourceModal from "../ListValuesSourceModal"; import { RadioLabelButton, RadioLabelRoot, @@ -18,18 +18,18 @@ import { export interface ParameterSourceSettingsProps { parameter: UiParameter; - onChangeSourceType: (sourceType: ParameterSourceType) => void; - onChangeSourceOptions: (sourceOptions: ParameterSourceConfig) => void; + onChangeSourceType: (sourceType: ValuesSourceType) => void; + onChangeSourceConfig: (sourceConfig: ValuesSourceConfig) => void; } const ParameterSourceSettings = ({ parameter, onChangeSourceType, - onChangeSourceOptions, + onChangeSourceConfig, }: ParameterSourceSettingsProps): JSX.Element => { const sourceType = getSourceType(parameter); - const sourceOptions = getSourceOptions(parameter); - const [editingType, setEditingType] = useState<ParameterSourceType>(); + const sourceConfig = getSourceConfig(parameter); + const [editingType, setEditingType] = useState<ValuesSourceType>(); const radioOptions = useMemo( () => getRadioOptions(sourceType, setEditingType), @@ -37,26 +37,25 @@ const ParameterSourceSettings = ({ ); const handleSourceTypeChange = useCallback( - (sourceType: ParameterSourceType) => { - if (hasValidSourceOptions(sourceType, sourceOptions)) { + (sourceType: ValuesSourceType) => { + if (sourceType == null) { onChangeSourceType(sourceType); + onChangeSourceConfig({}); } else { setEditingType(sourceType); } }, - [sourceOptions, onChangeSourceType], + [onChangeSourceType, onChangeSourceConfig], ); - const handleSourceOptionsChange = useCallback( - (sourceOptions: ParameterSourceConfig) => { - if (editingType && hasValidSourceOptions(editingType, sourceOptions)) { + const handleSourceConfigChange = useCallback( + (sourceConfig: ValuesSourceConfig) => { + if (editingType) { onChangeSourceType(editingType); - } else { - onChangeSourceType(null); + onChangeSourceConfig(sourceConfig); } - onChangeSourceOptions(sourceOptions); }, - [editingType, onChangeSourceType, onChangeSourceOptions], + [editingType, onChangeSourceType, onChangeSourceConfig], ); const handleClose = useCallback(() => { @@ -71,11 +70,20 @@ const ParameterSourceSettings = ({ vertical onChange={handleSourceTypeChange} /> + {editingType === "card" && ( + <Modal medium onClose={handleClose}> + <CardValuesSourceModal + sourceConfig={sourceConfig} + onChangeSourceConfig={handleSourceConfigChange} + onClose={handleClose} + /> + </Modal> + )} {editingType === "static-list" && ( <Modal onClose={handleClose}> - <ParameterListSourceModal - parameter={parameter} - onChangeSourceOptions={handleSourceOptionsChange} + <ListValuesSourceModal + sourceConfig={sourceConfig} + onChangeSourceConfig={handleSourceConfigChange} onClose={handleClose} /> </Modal> @@ -106,8 +114,8 @@ const RadioLabel = ({ }; const getRadioOptions = ( - sourceType: ParameterSourceType, - onEdit: (sourceType: ParameterSourceType) => void, + sourceType: ValuesSourceType, + onEdit: (sourceType: ValuesSourceType) => void, ) => { return [ { @@ -119,6 +127,16 @@ const getRadioOptions = ( ), value: null, }, + { + name: ( + <RadioLabel + title={t`Values from a model or question`} + isSelected={sourceType === "card"} + onEditClick={() => onEdit("card")} + /> + ), + value: "card", + }, { name: ( <RadioLabel diff --git a/frontend/src/metabase/parameters/components/ParameterSourceSettings/ParameterSourceSettings.unit.spec.tsx b/frontend/src/metabase/parameters/components/ParameterSourceSettings/ParameterSourceSettings.unit.spec.tsx index 461fd24eaaee65136550e43749c112c022e46959..63da33cb293e8c2ab5df223e840da6a4b98d5695 100644 --- a/frontend/src/metabase/parameters/components/ParameterSourceSettings/ParameterSourceSettings.unit.spec.tsx +++ b/frontend/src/metabase/parameters/components/ParameterSourceSettings/ParameterSourceSettings.unit.spec.tsx @@ -1,6 +1,7 @@ import React from "react"; -import { render, screen } from "@testing-library/react"; +import { screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; +import { renderWithProviders } from "__support__/ui"; import { createMockUiParameter } from "metabase-lib/mocks"; import ParameterSourceSettings, { ParameterSourceSettingsProps, @@ -14,7 +15,7 @@ describe("ParameterSourceSettings", () => { }), }); - render(<ParameterSourceSettings {...props} />); + renderWithProviders(<ParameterSourceSettings {...props} />); userEvent.click(screen.getByText("Values from column")); expect(props.onChangeSourceType).toHaveBeenCalledWith(null); @@ -23,13 +24,13 @@ describe("ParameterSourceSettings", () => { it("should set up the static list source via the modal", () => { const props = getProps(); - render(<ParameterSourceSettings {...props} />); + renderWithProviders(<ParameterSourceSettings {...props} />); userEvent.click(screen.getByText("Custom list")); userEvent.type(screen.getByRole("textbox"), "Gadget"); userEvent.click(screen.getByText("Done")); expect(props.onChangeSourceType).toHaveBeenCalledWith("static-list"); - expect(props.onChangeSourceOptions).toHaveBeenCalledWith({ + expect(props.onChangeSourceConfig).toHaveBeenCalledWith({ values: ["Gadget"], }); }); @@ -42,14 +43,14 @@ describe("ParameterSourceSettings", () => { }), }); - render(<ParameterSourceSettings {...props} />); + renderWithProviders(<ParameterSourceSettings {...props} />); userEvent.click(screen.getByText("Edit")); userEvent.clear(screen.getByRole("textbox")); userEvent.type(screen.getByRole("textbox"), "Widget"); userEvent.click(screen.getByText("Done")); expect(props.onChangeSourceType).toHaveBeenCalledWith("static-list"); - expect(props.onChangeSourceOptions).toHaveBeenCalledWith({ + expect(props.onChangeSourceConfig).toHaveBeenCalledWith({ values: ["Widget"], }); }); @@ -57,29 +58,12 @@ describe("ParameterSourceSettings", () => { it("should not change the source type if the modal was dismissed", () => { const props = getProps(); - render(<ParameterSourceSettings {...props} />); + renderWithProviders(<ParameterSourceSettings {...props} />); userEvent.click(screen.getByText("Custom list")); userEvent.click(screen.getByText("Cancel")); expect(props.onChangeSourceType).not.toHaveBeenCalled(); - expect(props.onChangeSourceOptions).not.toHaveBeenCalled(); - }); - - it("should set the default source type if the static list is empty", () => { - const props = getProps({ - parameter: createMockUiParameter({ - values_source_type: "static-list", - values_source_config: { values: ["Gadget"] }, - }), - }); - - render(<ParameterSourceSettings {...props} />); - userEvent.click(screen.getByText("Edit")); - userEvent.clear(screen.getByRole("textbox")); - userEvent.click(screen.getByText("Done")); - - expect(props.onChangeSourceType).toHaveBeenCalledWith(null); - expect(props.onChangeSourceOptions).toHaveBeenCalledWith({ values: [] }); + expect(props.onChangeSourceConfig).not.toHaveBeenCalled(); }); }); @@ -88,6 +72,6 @@ const getProps = ( ): ParameterSourceSettingsProps => ({ parameter: createMockUiParameter(), onChangeSourceType: jest.fn(), - onChangeSourceOptions: jest.fn(), + onChangeSourceConfig: jest.fn(), ...opts, }); diff --git a/frontend/src/metabase/parameters/utils/dashboards.ts b/frontend/src/metabase/parameters/utils/dashboards.ts index 68401832deba7d0299adfa32ace4f68ce1060208..8e6e5040d28c884c59652c886b8148330c4b8e36 100644 --- a/frontend/src/metabase/parameters/utils/dashboards.ts +++ b/frontend/src/metabase/parameters/utils/dashboards.ts @@ -8,8 +8,8 @@ import type { Dashboard, DashboardParameterMapping, DashboardOrderedCard, - ParameterSourceType, - ParameterSourceConfig, + ValuesSourceType, + ValuesSourceConfig, Parameter, } from "metabase-types/api"; import { isFieldFilterParameter } from "metabase-lib/parameters/utils/parameter-type"; @@ -66,26 +66,14 @@ export function setParameterName( }; } -export function getSourceType(parameter: Parameter): ParameterSourceType { +export function getSourceType(parameter: Parameter): ValuesSourceType { return parameter.values_source_type ?? null; } -export function getSourceOptions(parameter: Parameter): ParameterSourceConfig { +export function getSourceConfig(parameter: Parameter): ValuesSourceConfig { return parameter.values_source_config ?? {}; } -export function hasValidSourceOptions( - sourceType: ParameterSourceType, - sourceOptions: ParameterSourceConfig, -): boolean { - switch (sourceType) { - case "static-list": - return sourceOptions.values != null && sourceOptions.values.length > 0; - default: - return true; - } -} - export function getIsMultiSelect(parameter: Parameter): boolean { return parameter.isMultiSelect == null ? true : parameter.isMultiSelect; } diff --git a/frontend/src/metabase/query_builder/components/DataSelector/DataSelector.jsx b/frontend/src/metabase/query_builder/components/DataSelector/DataSelector.jsx index d39a88faa65a7246e04153b27c9f68fcf64138e5..a43b3931816b6e43a308dbc47806028278ccb514 100644 --- a/frontend/src/metabase/query_builder/components/DataSelector/DataSelector.jsx +++ b/frontend/src/metabase/query_builder/components/DataSelector/DataSelector.jsx @@ -954,7 +954,7 @@ export class UnconnectedDataSelector extends Component { }); handleCollectionDatasetSelect = async dataset => { - const tableId = getQuestionVirtualTableId(dataset); + const tableId = getQuestionVirtualTableId(dataset.id); await this.props.fetchFields(tableId); if (this.props.setSourceTableFn) { this.props.setSourceTableFn(tableId); diff --git a/frontend/src/metabase/query_builder/components/DataSelector/data-search/utils.js b/frontend/src/metabase/query_builder/components/DataSelector/data-search/utils.js index e9e55a3cce015e178e039b01b890a87249a5f8b2..3f4ef5d771492247f662ac6350112dcd0375b65e 100644 --- a/frontend/src/metabase/query_builder/components/DataSelector/data-search/utils.js +++ b/frontend/src/metabase/query_builder/components/DataSelector/data-search/utils.js @@ -9,7 +9,7 @@ export function convertSearchResultToTableLikeItem(searchResultItem) { ) { return { ...searchResultItem, - id: getQuestionVirtualTableId(searchResultItem), + id: getQuestionVirtualTableId(searchResultItem.id), }; } 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 16e6736d6f474acd9ec02779e038d2f4ccf9bc95..07f126b7de5757b76acd75839ea24a8e23801058 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 @@ -9,16 +9,33 @@ import { } from "__support__/e2e/helpers"; import { SAMPLE_DATABASE } from "__support__/e2e/cypress_sample_database"; -const { ORDERS_ID } = SAMPLE_DATABASE; +const { PRODUCTS_ID, PRODUCTS } = SAMPLE_DATABASE; -const questionDetails = { +const dashboardQuestionDetails = { display: "scalar", query: { - "source-table": ORDERS_ID, + "source-table": PRODUCTS_ID, aggregation: [["count"]], }, }; +const structuredQuestionDetails = { + name: "GUI categories", + query: { + "source-table": PRODUCTS_ID, + aggregation: [["count"]], + breakout: [["field", PRODUCTS.CATEGORY, null]], + filter: ["!=", ["field", PRODUCTS.CATEGORY, null], "Gizmo"], + }, +}; + +const nativeQuestionDetails = { + name: "SQL categories", + native: { + query: "select distinct CATEGORY from PRODUCTS order by CATEGORY limit 2", + }, +}; + describe("scenarios > dashboard > filters", () => { beforeEach(() => { restore(); @@ -26,36 +43,114 @@ describe("scenarios > dashboard > filters", () => { cy.intercept("POST", "/api/dashboard/**/query").as("getCardQuery"); }); - it("should be able to use a static list source", () => { - cy.createQuestionAndDashboard({ questionDetails }).then( - ({ body: { dashboard_id } }) => { - visitDashboard(dashboard_id); - }, - ); + it("should be able to use a structured question source", () => { + cy.createQuestion(structuredQuestionDetails); + cy.createQuestionAndDashboard({ + questionDetails: dashboardQuestionDetails, + }).then(({ body: { dashboard_id } }) => { + visitDashboard(dashboard_id); + }); editDashboard(); setFilter("Text or Category", "Dropdown"); - cy.findByText("Custom list").click(); - modal().within(() => { - cy.findByPlaceholderText(/banana/).type("Apple\nGoogle"); - cy.button("Done").click(); - }); - cy.findByText("Select…").click(); - popover().within(() => { - cy.findByText("Source").click(); - }); + setupStructuredQuestionSource(); + mapFilterToQuestion(); saveDashboard(); + filterDashboard(); + }); - cy.findByText("Text").click(); - popover().within(() => { - cy.findByText("Apple").should("be.visible"); - cy.findByText("Google").should("be.visible"); + it("should be able to use a native question source", () => { + cy.createNativeQuestion(nativeQuestionDetails); + cy.createQuestionAndDashboard({ + questionDetails: dashboardQuestionDetails, + }).then(({ body: { dashboard_id } }) => { + visitDashboard(dashboard_id); + }); - cy.findByPlaceholderText("Search the list").type("Goo"); - cy.findByText("Apple").should("not.exist"); - cy.findByText("Google").click(); - cy.button("Add filter").click(); + editDashboard(); + setFilter("Text or Category", "Dropdown"); + setupNativeQuestionSource(); + mapFilterToQuestion(); + saveDashboard(); + filterDashboard(); + }); + + it("should be able to use a static list source", () => { + cy.createQuestionAndDashboard({ + questionDetails: dashboardQuestionDetails, + }).then(({ body: { dashboard_id } }) => { + visitDashboard(dashboard_id); }); - cy.wait("@getCardQuery"); + + editDashboard(); + setFilter("Text or Category", "Dropdown"); + setupCustomList(); + mapFilterToQuestion(); + saveDashboard(); + filterDashboard(); }); }); + +const setupStructuredQuestionSource = () => { + cy.findByText("Values from a model or question").click(); + modal().within(() => { + cy.findByText("Saved Questions").click(); + cy.findByText("GUI categories").click(); + cy.button("Select column").click(); + }); + modal().within(() => { + cy.findByText("Pick a column").click(); + }); + popover().within(() => { + cy.findByText("Category").click(); + }); + modal().within(() => { + cy.button("Done").click(); + }); +}; + +const setupNativeQuestionSource = () => { + cy.findByText("Values from a model or question").click(); + modal().within(() => { + cy.findByText("Saved Questions").click(); + cy.findByText("SQL categories").click(); + cy.button("Select column").click(); + }); + modal().within(() => { + cy.findByText("Pick a column").click(); + }); + popover().within(() => { + cy.findByText("CATEGORY").click(); + }); + modal().within(() => { + cy.button("Done").click(); + }); +}; + +const setupCustomList = () => { + cy.findByText("Custom list").click(); + modal().within(() => { + cy.findByPlaceholderText(/banana/).type("Doohickey\nGadget"); + cy.button("Done").click(); + }); +}; + +const mapFilterToQuestion = () => { + cy.findByText("Select…").click(); + popover().within(() => cy.findByText("Category").click()); +}; + +const filterDashboard = () => { + cy.findByText("Text").click(); + popover().within(() => { + cy.findByText("Doohickey").should("be.visible"); + cy.findByText("Gadget").should("be.visible"); + cy.findByText("Gizmo").should("not.exist"); + + cy.findByPlaceholderText("Search the list").type("Gadget"); + cy.findByText("Doohickey").should("not.exist"); + cy.findByText("Gadget").click(); + cy.button("Add filter").click(); + }); + cy.wait("@getCardQuery"); +};