diff --git a/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Light_Theme_Date_Filter_Month_Year.png b/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Light_Theme_Date_Filter_Month_Year.png index afd454ba58eb2a2aa80b3aa56d98b779d9284671..7a94df0ac1ccf39b2cb02bee8b645bd441689e8a 100644 Binary files a/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Light_Theme_Date_Filter_Month_Year.png and b/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Light_Theme_Date_Filter_Month_Year.png differ diff --git a/frontend/src/metabase-lib/v1/Dimension.ts b/frontend/src/metabase-lib/v1/Dimension.ts index 9b8bcb6d0daa8df51aab56cce9aebe8f4372a5c3..278ec19ce3d39bfd08bce893ebcf4bd608a15d50 100644 --- a/frontend/src/metabase-lib/v1/Dimension.ts +++ b/frontend/src/metabase-lib/v1/Dimension.ts @@ -752,7 +752,7 @@ export class FieldDimension extends Dimension { const identifierProp = this._getIdentifierProp(); const fieldIdentifier = this.fieldIdOrName(); if (this._query) { - const queryTableFields = this._query.table()?.fields; + const queryTableFields = this._query.table?.()?.fields; return _.findWhere(queryTableFields, { [identifierProp]: fieldIdentifier, }); @@ -1166,7 +1166,7 @@ export class ExpressionDimension extends Dimension { field() { try { const query = this._query; - const table = query ? query.table() : null; + const table = query ? query.table?.() : null; // fallback const baseTypeOption = this.getOption("base-type"); diff --git a/frontend/src/metabase-lib/v1/metadata/Field.ts b/frontend/src/metabase-lib/v1/metadata/Field.ts index 75b60c2525484fa8603e41e2b6af6a824fa786eb..03f72fd7c1679dd0ae58b4ae8e6c3509a06cf55b 100644 --- a/frontend/src/metabase-lib/v1/metadata/Field.ts +++ b/frontend/src/metabase-lib/v1/metadata/Field.ts @@ -413,24 +413,6 @@ export default class Field extends Base { }); } - remappingOptions = () => { - const table = this.table; - if (!table) { - return []; - } - - const { fks } = table - .legacyQuery({ useStructuredQuery: true }) - .fieldOptions(); - return fks - .filter(({ field }) => field.id === this.id) - .map(({ field, dimension, dimensions }) => ({ - field, - dimension, - dimensions: dimensions.filter(d => d.isValidFKRemappingTarget()), - })); - }; - clone(fieldMetadata?: FieldMetadata) { if (fieldMetadata instanceof Field) { throw new Error("`fieldMetadata` arg must be a plain object"); diff --git a/frontend/src/metabase-lib/v1/metadata/Table.ts b/frontend/src/metabase-lib/v1/metadata/Table.ts index 0f52fc3a6e648a1e16076248e16cb5b1aeade56f..42aa0df69803147a4cae250f37a5c490c393975e 100644 --- a/frontend/src/metabase-lib/v1/metadata/Table.ts +++ b/frontend/src/metabase-lib/v1/metadata/Table.ts @@ -4,7 +4,6 @@ import _ from "underscore"; import { singularize } from "metabase/lib/formatting"; import { isVirtualCardId } from "metabase-lib/v1/metadata/utils/saved-questions"; import { getAggregationOperators } from "metabase-lib/v1/operators/utils"; -import type StructuredQuery from "metabase-lib/v1/queries/StructuredQuery"; import type { NormalizedTable } from "metabase-types/api"; import Question from "../Question"; @@ -81,22 +80,6 @@ class Table { }); } - savedQuestionId() { - const match = String(this.id).match(/card__(\d+)/); - return match ? parseInt(match[1]) : null; - } - - legacyQuery(query = {}) { - return ( - this.question().legacyQuery({ - useStructuredQuery: true, - }) as StructuredQuery - ).updateQuery(q => ({ - ...q, - ...query, - })); - } - dimensions() { return this.getFields().map(field => field.dimension()); } diff --git a/frontend/src/metabase-lib/v1/queries/Query.ts b/frontend/src/metabase-lib/v1/queries/Query.ts index 05c4349163635b2aa2c72b38a428c73f3ec365d7..15ac0dca0201bc71936e08759fca8f4071b9643f 100644 --- a/frontend/src/metabase-lib/v1/queries/Query.ts +++ b/frontend/src/metabase-lib/v1/queries/Query.ts @@ -2,7 +2,7 @@ // @ts-nocheck import _ from "underscore"; -import Dimension from "metabase-lib/v1/Dimension"; +import type Dimension from "metabase-lib/v1/Dimension"; import DimensionOptions from "metabase-lib/v1/DimensionOptions"; import type Question from "metabase-lib/v1/Question"; import type Metadata from "metabase-lib/v1/metadata/Metadata"; @@ -86,10 +86,6 @@ class Query { variables(_filter?: (variable: Variable) => boolean): TemplateTagVariable[] { return []; } - - parseFieldReference(fieldRef, query = this): Dimension | null | undefined { - return Dimension.parseMBQL(fieldRef, this._metadata, query); - } } // eslint-disable-next-line import/no-default-export -- deprecated usage diff --git a/frontend/src/metabase-lib/v1/queries/StructuredQuery.ts b/frontend/src/metabase-lib/v1/queries/StructuredQuery.ts index b1217dbda07b9fe74957d686c8a4a1a871faa8a3..16550a94376c6f4de36441edcab6433e4eb5883c 100644 --- a/frontend/src/metabase-lib/v1/queries/StructuredQuery.ts +++ b/frontend/src/metabase-lib/v1/queries/StructuredQuery.ts @@ -3,32 +3,21 @@ /** * Represents a structured MBQL query. */ -import { updateIn } from "icepick"; import _ from "underscore"; import * as Lib from "metabase-lib"; -import type { Dimension } from "metabase-lib/v1/Dimension"; -import { FieldDimension } from "metabase-lib/v1/Dimension"; -import DimensionOptions from "metabase-lib/v1/DimensionOptions"; import type { - DatabaseId, DatasetQuery, StructuredDatasetQuery, - StructuredQuery as StructuredQueryObject, TableId, } from "metabase-types/api"; import type { Query } from "../../types"; import type Question from "../Question"; -import type Database from "../metadata/Database"; -import type Field from "../metadata/Field"; import type Table from "../metadata/Table"; import AtomicQuery from "./AtomicQuery"; -type DimensionFilterFn = (dimension: Dimension) => boolean; -export type FieldFilterFn = (filter: Field) => boolean; - export const STRUCTURED_QUERY_TEMPLATE = { database: null, type: "query", @@ -36,6 +25,7 @@ export const STRUCTURED_QUERY_TEMPLATE = { "source-table": null, }, }; + /** * A wrapper around an MBQL (`query` type @type {DatasetQuery}) object */ @@ -63,38 +53,6 @@ class StructuredQuery extends AtomicQuery { return this.question().query(); } - /* Query superclass methods */ - - /* AtomicQuery superclass methods */ - - /** - * @returns all tables in the currently selected database that can be used. - */ - tables(): Table[] | null | undefined { - const database = this._database(); - return (database && database.tables) || null; - } - - /** - * @returns the currently selected database ID, if any is selected. - * @deprecated Use MLv2 - */ - _databaseId(): DatabaseId | null | undefined { - // same for both structured and native - return this._structuredDatasetQuery.database; - } - - /** - * @returns the currently selected database metadata, if a database is selected and loaded. - * @deprecated Use MLv2 - */ - _database(): Database | null | undefined { - const databaseId = this._databaseId(); - return databaseId != null ? this._metadata.database(databaseId) : null; - } - - /* Methods unique to this query type */ - /** * @returns the underlying MBQL query object */ @@ -102,12 +60,6 @@ class StructuredQuery extends AtomicQuery { return this._structuredDatasetQuery.query; } - updateQuery( - fn: (q: StructuredQueryObject) => StructuredQueryObject, - ): StructuredQuery { - return this._updateQuery(fn, []); - } - /** * @returns the table ID, if a table is selected. * @deprecated Use MLv2 @@ -126,104 +78,6 @@ class StructuredQuery extends AtomicQuery { const metadata = question.metadata(); return metadata.table(this._sourceTableId()); }); - - // DIMENSION OPTIONS - dimensionOptions( - dimensionFilter: DimensionFilterFn = _dimension => true, - ): DimensionOptions { - const dimensionOptions = { - count: 0, - fks: [], - dimensions: [], - }; - - const table = this.table(); - - if (table) { - const filteredNonFKDimensions = this.dimensions().filter(dimensionFilter); - - for (const dimension of filteredNonFKDimensions) { - dimensionOptions.count++; - dimensionOptions.dimensions.push(dimension); - } - - const dimensionIsFKReference = dimension => dimension.field?.().isFK(); - const fkDimensions = this.dimensions().filter(dimensionIsFKReference); - - for (const dimension of fkDimensions) { - const field = dimension.field(); - - const isNestedCardTable = table?.isVirtualCard(); - const tableHasExplicitJoin = - isNestedCardTable && - table.fields.find( - tableField => tableField.id === field.fk_target_field_id, - ); - - if (tableHasExplicitJoin) { - continue; - } - - const fkDimensions = dimension - .dimensions([FieldDimension]) - .filter(dimensionFilter); - - if (fkDimensions.length > 0) { - dimensionOptions.count += fkDimensions.length; - dimensionOptions.fks.push({ - field: field, - dimension: dimension, - dimensions: fkDimensions, - }); - } - } - } - - return new DimensionOptions(dimensionOptions); - } - - // FIELD OPTIONS - fieldOptions(fieldFilter: FieldFilterFn = _field => true): DimensionOptions { - const dimensionFilter = dimension => { - const field = dimension.field && dimension.field(); - return !field || (field.isDimension() && fieldFilter(field)); - }; - - return this.dimensionOptions(dimensionFilter); - } - - // DIMENSIONS - dimensions(): Dimension[] { - return this.tableDimensions(); - } - - tableDimensions = _.once((): Dimension[] => { - const table: Table = this.table(); - return table // HACK: ensure the dimensions are associated with this query - ? table - .dimensions() - .map(d => (d._query ? d : this.parseFieldReference(d.mbql()))) - : []; - }); - - setDatasetQuery(datasetQuery: DatasetQuery): StructuredQuery { - return new StructuredQuery(this._originalQuestion, datasetQuery); - } - - // INTERNAL - _updateQuery( - updateFunction: ( - query: StructuredQueryObject, - ...args: any[] - ) => StructuredQueryObject, - args: any[] = [], - ): StructuredQuery { - return this.setDatasetQuery( - updateIn(this._datasetQuery, ["query"], query => - updateFunction(query, ...args), - ), - ); - } } // eslint-disable-next-line import/no-default-export -- deprecated usage diff --git a/frontend/src/metabase/admin/datamodel/components/DimensionList/DimensionList.jsx b/frontend/src/metabase/admin/datamodel/components/DimensionList/DimensionList.jsx deleted file mode 100644 index 835d5b0fcc2729bdb2e4fb439addab0028acfdb3..0000000000000000000000000000000000000000 --- a/frontend/src/metabase/admin/datamodel/components/DimensionList/DimensionList.jsx +++ /dev/null @@ -1,216 +0,0 @@ -/* eslint-disable react/prop-types */ -import cx from "classnames"; -import { Component } from "react"; -import _ from "underscore"; - -import PopoverWithTrigger from "metabase/components/PopoverWithTrigger"; -import AccordionList from "metabase/core/components/AccordionList"; -import ListS from "metabase/css/components/list.module.css"; -import CS from "metabase/css/core/index.css"; -import { Box, Icon } from "metabase/ui"; -import { FieldDimension } from "metabase-lib/v1/Dimension"; - -import { DimensionPicker } from "../DimensionPicker"; - -import { FieldListGroupingTrigger } from "./DimensionList.styled"; - -const SUBMENU_TETHER_OPTIONS = { - attachment: "top left", - targetAttachment: "top right", - targetOffset: "0 0", - constraints: [ - { - to: "window", - attachment: "together", - pin: true, - }, - ], -}; - -/** - * @deprecated use MLv2 - */ -export class DimensionList extends Component { - state = { - sections: [], - }; - - UNSAFE_componentWillMount() { - this.updateSections(this.props.sections); - } - - UNSAFE_componentWillReceiveProps(nextProps) { - if (this.props.sections !== nextProps.sections) { - this.updateSections(nextProps.sections); - } - } - - updateSections(sections = []) { - this.setState({ - sections: sections.map(section => ({ - ...section, - items: section.items.map(item => ({ - ...item, - name: item.name || item.dimension?.displayName(), - icon: item.icon || item.dimension?.icon(), - })), - })), - }); - } - - getDimensions() { - return ( - this.props.dimensions || - (this.props.dimension ? [this.props.dimension] : []) - ); - } - - itemIsSelected = item => { - const dimensions = this.getDimensions(); - return ( - item.dimension && - _.any(dimensions, dimension => { - // sometimes `item.dimension` has a join-alias and `dimension` doesn't - // with/without is equivalent in this scenario - return dimension.isSameBaseDimension(item.dimension.withoutJoinAlias()); - }) - ); - }; - - renderItemExtra = (item, isSelected) => { - const { enableSubDimensions, preventNumberSubDimensions } = this.props; - - const surpressSubDimensions = - preventNumberSubDimensions && item.dimension.field().isSummable(); - - const subDimensions = - enableSubDimensions && - // Do not display sub dimension if this is an FK (metabase#16787) - !item.dimension?.field().isFK() && - // Or if this is a custom expression (metabase#11371) - !item.dimension?.isExpression() && - !surpressSubDimensions && - item.dimension.dimensions(); - - const sectionDimension = this.props.dimension - ? this.props.dimension - : _.find( - this.getDimensions(), - dimension => dimension.field() === item.dimension.field(), - ); - - return ( - <Box className="Field-extra"> - {item.dimension?.tag && ( - <span className={cx(CS.h5, CS.textLight, CS.px1)}> - {item.dimension.tag} - </span> - )} - {subDimensions?.length > 0 ? ( - <PopoverWithTrigger - className={this.props.className} - hasArrow={false} - triggerElement={this.renderSubDimensionTrigger(item.dimension)} - tetherOptions={SUBMENU_TETHER_OPTIONS} - sizeToFit - > - {({ onClose }) => ( - <DimensionPicker - className={CS.scrollY} - dimension={sectionDimension} - dimensions={subDimensions} - onChangeDimension={dimension => { - this.props.onChangeDimension(dimension, { - isSubDimension: true, - }); - onClose(); - }} - /> - )} - </PopoverWithTrigger> - ) : null} - </Box> - ); - }; - - renderSubDimensionTrigger(otherDimension) { - const dimensions = this.getDimensions(); - const subDimension = - _.find(dimensions, dimension => - dimension.isSameBaseDimension(otherDimension), - ) || otherDimension.defaultDimension(); - const name = subDimension?.subTriggerDisplayName() ?? null; - - return ( - <FieldListGroupingTrigger - className={cx( - ListS.FieldListGroupingTrigger, - CS.textWhiteHover, - CS.flex, - CS.alignCenter, - CS.p1, - CS.cursorPointer, - )} - data-testid="dimension-list-item-binning" - > - {name && <h4>{name}</h4>} - <Icon name="chevronright" className={CS.ml1} size={16} /> - </FieldListGroupingTrigger> - ); - } - - getDimensionFromItem(item) { - const { - enableSubDimensions, - useOriginalDimension, - preventNumberSubDimensions, - } = this.props; - const dimension = useOriginalDimension - ? item.dimension - : item.dimension.defaultDimension() || item.dimension; - const shouldExcludeBinning = - !enableSubDimensions && - !useOriginalDimension && - dimension instanceof FieldDimension && - dimension.binningStrategy(); - - if ( - shouldExcludeBinning || - (preventNumberSubDimensions && dimension.field().isSummable()) || - dimension?.field().isFK() - ) { - // If we don't let user choose the sub-dimension, we don't want to treat the field - // as a binned field (which would use the default binning) - // Let's unwrap the base field of the binned field instead - return dimension.baseDimension(); - } else { - return dimension; - } - } - - handleChange = item => { - const { dimension, onChangeDimension, onChangeOther } = this.props; - if (dimension != null && this.itemIsSelected(item)) { - // ensure if we select the same item we don't reset the subdimension - onChangeDimension(dimension, item); - } else if (item.dimension) { - onChangeDimension(this.getDimensionFromItem(item), item); - } else if (onChangeOther) { - onChangeOther(item); - } - }; - - render() { - return ( - <AccordionList - {...this.props} - itemTestId="dimension-list-item" - sections={this.state.sections} - onChange={this.handleChange} - itemIsSelected={this.itemIsSelected} - renderItemExtra={this.renderItemExtra} - getItemClassName={() => cx(CS.hoverParent, CS.hoverDisplay)} - /> - ); - } -} diff --git a/frontend/src/metabase/admin/datamodel/components/DimensionList/DimensionList.styled.tsx b/frontend/src/metabase/admin/datamodel/components/DimensionList/DimensionList.styled.tsx deleted file mode 100644 index ff687c38c5f0b8cb2567f49a61411ba1f3e0516b..0000000000000000000000000000000000000000 --- a/frontend/src/metabase/admin/datamodel/components/DimensionList/DimensionList.styled.tsx +++ /dev/null @@ -1,10 +0,0 @@ -import styled from "@emotion/styled"; - -import { alpha } from "metabase/lib/colors"; - -export const FieldListGroupingTrigger = styled.div` - display: flex; - visibility: hidden; - border-left: 2px solid ${() => alpha("filter", 0.1)}; - color: ${() => alpha("text-white", 0.5)}; -`; diff --git a/frontend/src/metabase/admin/datamodel/components/DimensionList/index.ts b/frontend/src/metabase/admin/datamodel/components/DimensionList/index.ts deleted file mode 100644 index f5be00c00ff033cc92fff9526d123ad16dfdee4e..0000000000000000000000000000000000000000 --- a/frontend/src/metabase/admin/datamodel/components/DimensionList/index.ts +++ /dev/null @@ -1 +0,0 @@ -export * from "./DimensionList"; diff --git a/frontend/src/metabase/admin/datamodel/components/DimensionPicker/DimensionPicker.jsx b/frontend/src/metabase/admin/datamodel/components/DimensionPicker/DimensionPicker.jsx deleted file mode 100644 index 21f9d3b22965b8540d7e2d4cffdbbc684bf1c885..0000000000000000000000000000000000000000 --- a/frontend/src/metabase/admin/datamodel/components/DimensionPicker/DimensionPicker.jsx +++ /dev/null @@ -1,60 +0,0 @@ -import cx from "classnames"; -import PropTypes from "prop-types"; - -import ListS from "metabase/css/components/list.module.css"; -import CS from "metabase/css/core/index.css"; - -import { DimensionListItem } from "./DimensionPicker.styled"; - -const propTypes = { - style: PropTypes.object, - className: PropTypes.string, - dimension: PropTypes.object, - dimensions: PropTypes.array, - onChangeDimension: PropTypes.func.isRequired, -}; - -/** - * @deprecated use MLv2 - */ -export const DimensionPicker = ({ - style, - className, - dimension, - dimensions, - onChangeDimension, -}) => { - return ( - <ul className={cx(className, CS.px2, CS.py1)} style={style}> - {dimensions.map((d, index) => { - const isSelected = d.isEqual(dimension); - return ( - <DimensionListItem - aria-selected={isSelected} - key={index} - data-element-id="list-item" - className={cx(ListS.ListItem, { - [ListS.ListItemSelected]: isSelected, - })} - > - <a - data-element-id="list-item-title" - className={cx( - ListS.ListItemTitle, - CS.full, - CS.px2, - CS.py1, - CS.cursorPointer, - )} - onClick={() => onChangeDimension(d)} - > - {d.subDisplayName()} - </a> - </DimensionListItem> - ); - })} - </ul> - ); -}; - -DimensionPicker.propTypes = propTypes; diff --git a/frontend/src/metabase/admin/datamodel/components/DimensionPicker/DimensionPicker.styled.tsx b/frontend/src/metabase/admin/datamodel/components/DimensionPicker/DimensionPicker.styled.tsx deleted file mode 100644 index a1dc93b37cfdac40b5a1071d519bb3e1f047d78f..0000000000000000000000000000000000000000 --- a/frontend/src/metabase/admin/datamodel/components/DimensionPicker/DimensionPicker.styled.tsx +++ /dev/null @@ -1,7 +0,0 @@ -import styled from "@emotion/styled"; - -import { alpha } from "metabase/lib/colors"; - -export const DimensionListItem = styled.li` - border-color: ${() => alpha("accent2", 0.2)}; -`; diff --git a/frontend/src/metabase/admin/datamodel/components/DimensionPicker/index.ts b/frontend/src/metabase/admin/datamodel/components/DimensionPicker/index.ts deleted file mode 100644 index d14938da063994df54d6aa8b8b126febd80d1470..0000000000000000000000000000000000000000 --- a/frontend/src/metabase/admin/datamodel/components/DimensionPicker/index.ts +++ /dev/null @@ -1 +0,0 @@ -export * from "./DimensionPicker"; diff --git a/frontend/src/metabase/admin/datamodel/components/FieldList/FieldList.jsx b/frontend/src/metabase/admin/datamodel/components/FieldList/FieldList.jsx deleted file mode 100644 index 40a1a702063c7db6ff2c4c033808d828c7f8edbb..0000000000000000000000000000000000000000 --- a/frontend/src/metabase/admin/datamodel/components/FieldList/FieldList.jsx +++ /dev/null @@ -1,74 +0,0 @@ -/* eslint-disable react/prop-types */ -import { Component } from "react"; - -import Dimension from "metabase-lib/v1/Dimension"; -import DimensionOptions from "metabase-lib/v1/DimensionOptions"; - -import { DimensionList } from "../DimensionList"; - -/** - * @deprecated use MLv2 - */ -export class FieldList extends Component { - state = { - sections: [], - }; - - UNSAFE_componentWillMount() { - this._updateSections(this.props); - } - UNSAFE_componentWillReceiveProps(nextProps) { - this._updateSections(nextProps); - } - _updateSections({ - fieldOptions = { dimensions: [], fks: [] }, - segmentOptions = [], - } = {}) { - const sections = new DimensionOptions(fieldOptions).sections({ - extraItems: segmentOptions.map(segment => ({ - filter: ["segment", segment.id], - name: segment.name, - icon: "star", - className: "List-item--segment", - })), - }); - this.setState({ sections }); - } - - handleChangeDimension = (dimension, item) => { - this.props.onFieldChange(dimension.mbql(), item); - }; - - handleChangeOther = item => { - if (item.filter && this.props.onFilterChange) { - this.props.onFilterChange(item.filter); - } - }; - - render() { - const { field, query, metadata } = this.props; - const dimension = - field && - (query - ? query.parseFieldReference(field) - : Dimension.parseMBQL(field, metadata)); - - return ( - <DimensionList - sections={this.state.sections} - dimension={dimension} - onChangeDimension={this.handleChangeDimension} - onChangeOther={this.handleChangeOther} - // forward AccordionList props - className={this.props.className} - maxHeight={this.props.maxHeight} - width={this.props.width} - alwaysExpanded={this.props.alwaysExpanded} - // forward DimensionList props - useOriginalDimension={this.props.useOriginalDimension} - enableSubDimensions={this.props.enableSubDimensions} - preventNumberSubDimensions={this.props.preventNumberSubDimensions} - /> - ); - } -} diff --git a/frontend/src/metabase/admin/datamodel/components/FieldList/index.ts b/frontend/src/metabase/admin/datamodel/components/FieldList/index.ts deleted file mode 100644 index cd5026f9b2cacabaac764d5b1f3995e67277655b..0000000000000000000000000000000000000000 --- a/frontend/src/metabase/admin/datamodel/components/FieldList/index.ts +++ /dev/null @@ -1 +0,0 @@ -export * from "./FieldList"; diff --git a/frontend/src/metabase/admin/datamodel/metadata/components/FieldRemappingSettings/FieldRemappingSettings.jsx b/frontend/src/metabase/admin/datamodel/metadata/components/FieldRemappingSettings/FieldRemappingSettings.jsx index 1abbfb2ae575dc6594367f4ff86812630b521392..5bf4ba86f6ef6729a9a4e27f2313f3503293796c 100644 --- a/frontend/src/metabase/admin/datamodel/metadata/components/FieldRemappingSettings/FieldRemappingSettings.jsx +++ b/frontend/src/metabase/admin/datamodel/metadata/components/FieldRemappingSettings/FieldRemappingSettings.jsx @@ -6,15 +6,12 @@ import { t } from "ttag"; import _ from "underscore"; import ButtonWithStatus from "metabase/components/ButtonWithStatus"; -import PopoverWithTrigger from "metabase/components/PopoverWithTrigger"; import Select from "metabase/core/components/Select"; import CS from "metabase/css/core/index.css"; import Fields from "metabase/entities/fields"; +import { PLUGIN_FEATURE_LEVEL_PERMISSIONS } from "metabase/plugins"; +import { FieldDataSelector } from "metabase/query_builder/components/DataSelector"; import { getMetadataUnfiltered } from "metabase/selectors/metadata"; -import { - getFieldTargetId, - hasSourceField, -} from "metabase-lib/v1/queries/utils/field-ref"; import { isEntityName, isFK } from "metabase-lib/v1/types/utils/isa"; import FieldSeparator from "../FieldSeparator"; @@ -24,7 +21,6 @@ import { FieldMappingRoot, FieldSelectButton, FieldValueMappingInput, - ForeignKeyList, } from "./FieldRemappingSettings.styled"; const MAP_OPTIONS = { @@ -63,8 +59,10 @@ class FieldRemappingSettings extends Component { throw new Error(t`Unrecognized mapping type`); }; - hasForeignKeys = () => { - return isFK(this.props.field) && this.getForeignKeys().length > 0; + hasForeignKeyTargetFields = () => { + return ( + isFK(this.props.field) && this.getForeignKeyTargetFields().length > 0 + ); }; hasMappableNumeralValues = () => { @@ -84,7 +82,7 @@ class FieldRemappingSettings extends Component { getAvailableMappingTypes = () => { const mappingTypes = [ MAP_OPTIONS.original, - ...(this.hasForeignKeys() ? [MAP_OPTIONS.foreign] : []), + ...(this.hasForeignKeyTargetFields() ? [MAP_OPTIONS.foreign] : []), ...(this.hasMappableNumeralValues() > 0 ? [MAP_OPTIONS.custom] : []), ]; @@ -98,17 +96,9 @@ class FieldRemappingSettings extends Component { }; getFKTargetTableEntityNameOrNull = () => { - const fks = this.getForeignKeys(); - const fkTargetFields = fks[0] && fks[0].dimensions.map(dim => dim.field()); - - if (fkTargetFields) { - const nameField = fkTargetFields.find(field => isEntityName(field)); - return nameField ? nameField.id : null; - } else { - throw new Error( - t`Current field isn't a foreign key or FK target table metadata is missing`, - ); - } + const fkTargetFields = this.getForeignKeyTargetFields(); + const nameField = fkTargetFields.find(field => isEntityName(field)); + return nameField ? nameField.id : null; }; clearEditingStates = () => { @@ -161,25 +151,20 @@ class FieldRemappingSettings extends Component { } }; - onForeignKeyFieldChange = async foreignKeyClause => { + onForeignKeyFieldChange = async fkFieldId => { const { field, updateFieldDimension } = this.props; this.clearEditingStates(); - if (hasSourceField(foreignKeyClause)) { - await updateFieldDimension( - { id: field.id }, - { - type: "external", - name: field.display_name, - human_readable_field_id: getFieldTargetId(foreignKeyClause), - }, - ); - - this.fkPopover.current?.close(); - } else { - throw new Error(t`The selected field isn't a foreign key`); - } + await updateFieldDimension( + { id: field.id }, + { + type: "external", + name: field.display_name, + human_readable_field_id: fkFieldId, + }, + ); + this.fkPopover.current?.close(); }; onUpdateRemappings = remappings => { @@ -187,26 +172,14 @@ class FieldRemappingSettings extends Component { return updateFieldValues({ id: field.id }, Array.from(remappings)); }; - getForeignKeys = () => { - const { field, metadata } = this.props; - return metadata.field(field.id).remappingOptions(); - }; - - onFkPopoverDismiss = () => { - const { isChoosingInitialFkTarget } = this.state; - - if (isChoosingInitialFkTarget) { - this.setState({ dismissedInitialFkTargetPopover: true }); - } + getForeignKeyTargetFields = () => { + const { fkTargetField } = this.props; + return fkTargetField?.table?.fields ?? []; }; render() { - const { field, table, metadata, fieldsError } = this.props; - const { - isChoosingInitialFkTarget, - hasChanged, - dismissedInitialFkTargetPopover, - } = this.state; + const { field, table, metadata, fieldsError, fkTargetField } = this.props; + const { hasChanged, isChoosingInitialFkTarget } = this.state; const remapping = new Map(field.remappedValues()); const isFieldsAccessRestricted = fieldsError?.status === 403; @@ -232,13 +205,19 @@ class FieldRemappingSettings extends Component { {mappingType === MAP_OPTIONS.foreign && ( <> <FieldSeparator /> - <PopoverWithTrigger - key="foreignKeyName" - ref={this.fkPopover} + <FieldDataSelector + isInitiallyOpen={isChoosingInitialFkTarget} + databases={table?.database ? [table.database] : []} + selectedDatabase={table?.database} + selectedDatabaseId={table?.database?.id} + selectedTable={fkTargetField?.table} + selectedTableId={fkTargetField?.table?.id} + selectedField={fkMappingField} + selectedFieldId={fkMappingField?.id} triggerElement={ <FieldSelectButton hasValue={hasFKMappingValue} - hasError={dismissedInitialFkTargetPopover} + hasError={!fkMappingField} > {fkMappingField ? ( fkMappingField.display_name @@ -247,26 +226,8 @@ class FieldRemappingSettings extends Component { )} </FieldSelectButton> } - isInitiallyOpen={isChoosingInitialFkTarget} - onClose={this.onFkPopoverDismiss} - > - <ForeignKeyList - field={fkMappingField} - fieldOptions={{ - count: 0, - dimensions: [], - fks: this.getForeignKeys(), - }} - table={table} - onFieldChange={this.onForeignKeyFieldChange} - hideSingleSectionTitle - /> - </PopoverWithTrigger> - {dismissedInitialFkTargetPopover && ( - <div - className={cx(CS.textError, CS.ml2)} - >{t`Please select a column to use for display.`}</div> - )} + setFieldFn={this.onForeignKeyFieldChange} + /> </> )} </FieldMappingContainer> @@ -463,7 +424,12 @@ const mapDispatchToProps = { deleteFieldDimension: Fields.actions.deleteFieldDimension, }; -export default connect( - mapStateToProps, - mapDispatchToProps, +export default _.compose( + Fields.load({ + id: (_state, { field }) => field.fk_target_field_id, + entityAlias: "fkTargetField", + query: PLUGIN_FEATURE_LEVEL_PERMISSIONS.dataModelQueryProps, + loadingAndErrorWrapper: false, + }), + connect(mapStateToProps, mapDispatchToProps), )(FieldRemappingSettings); diff --git a/frontend/src/metabase/admin/datamodel/metadata/components/FieldRemappingSettings/FieldRemappingSettings.styled.tsx b/frontend/src/metabase/admin/datamodel/metadata/components/FieldRemappingSettings/FieldRemappingSettings.styled.tsx index 962543317914f196824e183e9807c840faf587ea..606db8601f015522c00b83bf2ab0474479c18417 100644 --- a/frontend/src/metabase/admin/datamodel/metadata/components/FieldRemappingSettings/FieldRemappingSettings.styled.tsx +++ b/frontend/src/metabase/admin/datamodel/metadata/components/FieldRemappingSettings/FieldRemappingSettings.styled.tsx @@ -1,6 +1,5 @@ import styled from "@emotion/styled"; -import { FieldList } from "metabase/admin/datamodel/components/FieldList"; import InputBlurChange from "metabase/components/InputBlurChange"; import SelectButton from "metabase/core/components/SelectButton"; import { alpha, color } from "metabase/lib/colors"; @@ -25,10 +24,6 @@ export const FieldSelectButton = styled(SelectButton)<FieldSelectButtonProps>` props.hasError ? color("error") : alpha("accent2", 0.2)}; `; -export const ForeignKeyList = styled(FieldList)` - color: var(--mb-color-filter); -`; - export const FieldValueMappingInput = styled(InputBlurChange)` width: auto; `; diff --git a/frontend/src/metabase/admin/datamodel/metadata/components/MetadataFieldSettings/MetadataFieldSettings.unit.spec.tsx b/frontend/src/metabase/admin/datamodel/metadata/components/MetadataFieldSettings/MetadataFieldSettings.unit.spec.tsx index e0e2519564d0ecc291e2412a3dc86b9f0a23b928..c7c6fe852ceab4564db3b3f5820fbc9ee0ed2b4f 100644 --- a/frontend/src/metabase/admin/datamodel/metadata/components/MetadataFieldSettings/MetadataFieldSettings.unit.spec.tsx +++ b/frontend/src/metabase/admin/datamodel/metadata/components/MetadataFieldSettings/MetadataFieldSettings.unit.spec.tsx @@ -6,6 +6,7 @@ import { setupDatabasesEndpoints, setupFieldValuesEndpoints, setupSearchEndpoints, + setupUnauthorizedFieldEndpoint, setupUnauthorizedFieldValuesEndpoints, } from "__support__/server-mocks"; import { @@ -114,7 +115,8 @@ interface SetupOpts { table?: Table; field?: Field; fieldValues?: GetFieldValuesResponse; - hasDataAccess?: boolean; + unauthorizedField?: Field; + hasFieldValuesAccess?: boolean; } const setup = async ({ @@ -122,12 +124,16 @@ const setup = async ({ table = ORDERS_TABLE, field = ORDERS_ID_FIELD, fieldValues = createMockFieldValues({ field_id: Number(field.id) }), - hasDataAccess = true, + unauthorizedField, + hasFieldValuesAccess = true, }: SetupOpts = {}) => { setupDatabasesEndpoints([database]); setupSearchEndpoints([]); - if (hasDataAccess) { + if (unauthorizedField) { + setupUnauthorizedFieldEndpoint(unauthorizedField); + } + if (hasFieldValuesAccess) { setupFieldValuesEndpoints(fieldValues); } else { setupUnauthorizedFieldValuesEndpoints(fieldValues); @@ -247,7 +253,10 @@ describe("MetadataFieldSettings", () => { }); it("should show an access denied error if the foreign key field has an inaccessible target", async () => { - await setup({ field: ORDERS_USER_ID_FIELD }); + await setup({ + field: ORDERS_USER_ID_FIELD, + unauthorizedField: PEOPLE_ID_FIELD, + }); expect(screen.getByText("Field access denied")).toBeInTheDocument(); }); @@ -258,7 +267,10 @@ describe("MetadataFieldSettings", () => { }); it("should show an access denied error if is custom mapping without data permissions", async () => { - await setup({ field: ORDERS_QUANTITY_FIELD, hasDataAccess: false }); + await setup({ + field: ORDERS_QUANTITY_FIELD, + hasFieldValuesAccess: false, + }); expect(screen.getByText("Custom mapping")).toBeInTheDocument(); expect(screen.queryByDisplayValue("1 remapped")).not.toBeInTheDocument(); expect( diff --git a/frontend/src/metabase/query_builder/components/DataSelector/DataSelector.jsx b/frontend/src/metabase/query_builder/components/DataSelector/DataSelector.jsx index b7904b0c441ad801c16d75700a289026686b9392..2097f6ed5c83c3b4dc067305890595ef523ade45 100644 --- a/frontend/src/metabase/query_builder/components/DataSelector/DataSelector.jsx +++ b/frontend/src/metabase/query_builder/components/DataSelector/DataSelector.jsx @@ -114,6 +114,16 @@ export function SchemaTableAndFieldDataSelector(props) { ); } +export function FieldDataSelector(props) { + return ( + <DataSelector + steps={[FIELD_STEP]} + getTriggerElementContent={FieldTrigger} + {...props} + /> + ); +} + export class UnconnectedDataSelector extends Component { constructor(props) { super(); diff --git a/frontend/src/metabase/query_builder/components/DataSelector/DataSelectorFieldPicker/DataSelectorFieldPicker.styled.tsx b/frontend/src/metabase/query_builder/components/DataSelector/DataSelectorFieldPicker/DataSelectorFieldPicker.styled.tsx index 0222c8445e64101a7253d8178dbe3616cab2460a..0ea60c23ebd8b5e2fe70992765a4112e192df407 100644 --- a/frontend/src/metabase/query_builder/components/DataSelector/DataSelectorFieldPicker/DataSelectorFieldPicker.styled.tsx +++ b/frontend/src/metabase/query_builder/components/DataSelector/DataSelectorFieldPicker/DataSelectorFieldPicker.styled.tsx @@ -12,10 +12,10 @@ export const HeaderContainer = styled.div` color: var(--mb-color-text-medium); cursor: pointer; display: flex; + gap: ${space(1)}; `; export const HeaderName = styled.span` - margin-left: ${space(1)}; overflow-wrap: anywhere; word-break: break-word; word-wrap: anywhere; diff --git a/frontend/src/metabase/query_builder/components/DataSelector/DataSelectorFieldPicker/DataSelectorFieldPicker.tsx b/frontend/src/metabase/query_builder/components/DataSelector/DataSelectorFieldPicker/DataSelectorFieldPicker.tsx index 7864061a619544645b5647edb83c8c1c492e0752..d8a519467289c9e773ea2f3ba19f743f7e39fd65 100644 --- a/frontend/src/metabase/query_builder/components/DataSelector/DataSelectorFieldPicker/DataSelectorFieldPicker.tsx +++ b/frontend/src/metabase/query_builder/components/DataSelector/DataSelectorFieldPicker/DataSelectorFieldPicker.tsx @@ -27,12 +27,12 @@ type DataSelectorFieldPickerProps = { isLoading?: boolean; selectedField?: Field; selectedTable?: Table; - onBack: () => void; + onBack?: () => void; onChangeField: (field: Field) => void; }; type HeaderProps = { - onBack: DataSelectorFieldPickerProps["onBack"]; + onBack?: DataSelectorFieldPickerProps["onBack"]; selectedTable: DataSelectorFieldPickerProps["selectedTable"]; }; @@ -109,7 +109,7 @@ function renderItemWrapper(content: ReactNode) { const Header = ({ onBack, selectedTable }: HeaderProps) => ( <HeaderContainer onClick={onBack}> - <Icon name="chevronleft" size={18} /> + {onBack && <Icon name="chevronleft" size={18} />} <HeaderName>{selectedTable?.display_name || t`Fields`}</HeaderName> </HeaderContainer> ); diff --git a/frontend/test/__support__/server-mocks/field.ts b/frontend/test/__support__/server-mocks/field.ts index 9abb69476823172c804bca7b740038ab9010f9ae..16d7b8a387641703e7dc12af32e0e3ae0858121f 100644 --- a/frontend/test/__support__/server-mocks/field.ts +++ b/frontend/test/__support__/server-mocks/field.ts @@ -19,11 +19,11 @@ export function setupFieldValuesEndpoints(fieldValues: GetFieldValuesResponse) { fetchMock.get(`path:/api/field/${fieldValues.field_id}/values`, fieldValues); } -export function setupFieldValuesGeneralEndpoint() { - fetchMock.get( - { url: /\/api\/field\/\d+\/values/, overwriteRoutes: false }, - [], - ); +export function setupUnauthorizedFieldEndpoint(field: Field) { + fetchMock.get(`path:/api/field/${field.id}`, { + status: 403, + body: PERMISSION_ERROR, + }); } export function setupUnauthorizedFieldValuesEndpoints( diff --git a/frontend/test/metabase-lib/lib/queries/StructuredQuery.unit.spec.js b/frontend/test/metabase-lib/lib/queries/StructuredQuery.unit.spec.js index aec652b27ed2368de270899ce57e7bc680e41321..94655d755c5dfb76b268f11f6e90e33fedce56e4 100644 --- a/frontend/test/metabase-lib/lib/queries/StructuredQuery.unit.spec.js +++ b/frontend/test/metabase-lib/lib/queries/StructuredQuery.unit.spec.js @@ -51,36 +51,6 @@ function makeQuery(query) { const query = makeQuery({}); describe("StructuredQuery", () => { - describe("DB METADATA METHODS", () => { - describe("tables", () => { - it("Tables should return multiple tables", () => { - expect(Array.isArray(query.tables())).toBe(true); - }); - - it("Tables should return a table map that includes fields", () => { - expect(Array.isArray(query.tables()[0].fields)).toBe(true); - }); - }); - - describe("table", () => { - it("Return the table wrapper object for the query", () => { - expect(query.table()).toBe(ordersTable); - }); - }); - - describe("_databaseId", () => { - it("returns the Database ID of the wrapped query", () => { - expect(query._databaseId()).toBe(SAMPLE_DB_ID); - }); - }); - - describe("_database", () => { - it("returns a dictionary with the underlying database of the wrapped query", () => { - expect(query._database().id).toBe(SAMPLE_DB_ID); - }); - }); - }); - describe("SIMPLE QUERY MANIPULATION METHODS", () => { describe("query", () => { it("returns the wrapper for the query dictionary", () => { @@ -96,36 +66,4 @@ describe("StructuredQuery", () => { }); }); }); - - describe("DIMENSION METHODS", () => { - describe("fieldOptions", () => { - it("includes the correct number of dimensions", () => { - // Should just include the non-fk keys from the current table - expect(query.fieldOptions().dimensions.length).toBe(9); - }); - - it("returns correct count of foreign keys", () => { - expect(query.fieldOptions().fks.length).toBe(2); - }); - - it("returns a correct count of fields", () => { - expect(query.fieldOptions().count).toBe(30); - }); - }); - }); - - describe("DATASET QUERY METHODS", () => { - describe("setDatasetQuery", () => { - it("replaces the previous dataset query with the provided one", () => { - const newDatasetQuery = makeDatasetQuery({ - "source-table": ORDERS_ID, - aggregation: [["count"]], - }); - - expect(query.setDatasetQuery(newDatasetQuery).datasetQuery()).toBe( - newDatasetQuery, - ); - }); - }); - }); });