diff --git a/frontend/src/metabase-lib/lib/metadata/Field.ts b/frontend/src/metabase-lib/lib/metadata/Field.ts index 8f9d246ef1b4d576a2ebc19cd9c1ace8c26bb9c1..7bb47cc2e120675d3678bb6b1158c594ce3814d7 100644 --- a/frontend/src/metabase-lib/lib/metadata/Field.ts +++ b/frontend/src/metabase-lib/lib/metadata/Field.ts @@ -46,6 +46,8 @@ class FieldInner extends Base { id: number | FieldRef; name: string; semantic_type: string | null; + fingerprint: any; + base_type: string | null; table?: Table; target?: Field; diff --git a/frontend/src/metabase/core/components/Ellipsified/Ellipsified.styled.tsx b/frontend/src/metabase/core/components/Ellipsified/Ellipsified.styled.tsx index 5c0552501d9af7534ab9fd602e64be3e9b2036b6..4c23ea3708115a7440f11d4162dadfaa27bf4a6e 100644 --- a/frontend/src/metabase/core/components/Ellipsified/Ellipsified.styled.tsx +++ b/frontend/src/metabase/core/components/Ellipsified/Ellipsified.styled.tsx @@ -16,6 +16,7 @@ const clampCss = (props: EllipsifiedRootProps) => css` interface EllipsifiedRootProps { lines?: number; + "data-testId"?: string; } export const EllipsifiedRoot = styled.div<EllipsifiedRootProps>` diff --git a/frontend/src/metabase/core/components/Ellipsified/Ellipsified.tsx b/frontend/src/metabase/core/components/Ellipsified/Ellipsified.tsx index 01d38e488f7e6ea4ace126b0ebb12f6d7faca722..df16d02d15b2d9d948edcfaae698fb38f650d4bc 100644 --- a/frontend/src/metabase/core/components/Ellipsified/Ellipsified.tsx +++ b/frontend/src/metabase/core/components/Ellipsified/Ellipsified.tsx @@ -16,6 +16,7 @@ interface EllipsifiedProps { tooltipMaxWidth?: React.CSSProperties["maxWidth"]; lines?: number; placement?: Placement; + "data-testid"?: string; } const Ellipsified = ({ @@ -28,6 +29,7 @@ const Ellipsified = ({ tooltipMaxWidth, lines, placement = "top", + "data-testid": dataTestId, }: EllipsifiedProps) => { const [isTruncated, setIsTruncated] = useState(false); const rootRef = useRef<HTMLDivElement | null>(null); @@ -62,6 +64,7 @@ const Ellipsified = ({ className={className} lines={lines} style={style} + data-testid={dataTestId} > {children} </EllipsifiedRoot> diff --git a/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterList/BulkFilterList.tsx b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterList/BulkFilterList.tsx index fc0820cdcfad5ce404b348d8d3b5af372e5c6f54..97cd11c11831f67f2d6145a5d1aed7413ec55ba7 100644 --- a/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterList/BulkFilterList.tsx +++ b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterList/BulkFilterList.tsx @@ -8,7 +8,6 @@ import StructuredQuery, { isSegmentOption, } from "metabase-lib/lib/queries/StructuredQuery"; import Dimension from "metabase-lib/lib/Dimension"; -import { isSegment } from "metabase/lib/query/filter"; import { ModalDivider } from "../BulkFilterModal/BulkFilterModal.styled"; import Filter from "metabase-lib/lib/queries/structured/Filter"; import { BulkFilterSelect, SegmentFilterSelect } from "../BulkFilterSelect"; @@ -18,6 +17,7 @@ import { ListRowContent, ListRowLabel, } from "./BulkFilterList.styled"; +import { sortDimensions } from "./utils"; export interface BulkFilterListProps { query: StructuredQuery; @@ -39,7 +39,10 @@ const BulkFilterList = ({ onClearSegments, }: BulkFilterListProps): JSX.Element => { const [dimensions, segments] = useMemo( - () => [options.filter(isDimensionOption), options.filter(isSegmentOption)], + () => [ + options.filter(isDimensionOption).sort(sortDimensions), + options.filter(isSegmentOption), + ], [options], ); @@ -92,7 +95,9 @@ const BulkFilterListItem = ({ return ( <ListRow> - <ListRowLabel>{dimension.displayName()}</ListRowLabel> + <ListRowLabel data-testid="dimension-filter-label"> + {dimension.displayName()} + </ListRowLabel> <ListRowContent> {options.map((filter, index) => ( <BulkFilterSelect diff --git a/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterList/utils.ts b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterList/utils.ts new file mode 100644 index 0000000000000000000000000000000000000000..7ee307888ac3dd9825f177e4bb67aba6da81c5bf --- /dev/null +++ b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterList/utils.ts @@ -0,0 +1,56 @@ +import { DimensionOption } from "metabase-lib/lib/queries/StructuredQuery"; + +const LONG_TEXT_MIN = 80; + +type PriorityMap = { [key: string]: number | undefined }; + +const fieldSortPriorities: PriorityMap = { + "type/CreationTemporal": 10, + "type/CreationTimestamp": 10, + "type/CreationDate": 10, + "type/CreationTime": 11, + "type/Boolean": 20, + "type/Category": 30, + "type/Currency": 40, + "type/Price": 40, + "type/Discount": 40, + "type/GrossMargin": 40, + "type/Cost": 40, + "type/Location": 50, + "type/Address": 50, + "type/City": 51, + "type/State": 52, + "type/ZipCode": 53, + "type/Country": 54, + "type/Number": 60, + "type/Float": 60, + "type/BigInteger": 60, + "type/Integer": 60, + "type/Text": 70, + "type/Date": 80, + "type/PK": 90, + "type/Latitude": 210, + "type/Longitude": 211, + "type/LongText": 220, // not a "real" metabase type, but having it as part of this list makes it easier to sort + "type/FK": 230, + "type/JSON": 240, + "": undefined, +}; + +const getSortValue = (dimensionOption: DimensionOption): number => { + const field = dimensionOption.dimension.field(); + + const isLongText = + field?.fingerprint?.type?.["type/Text"]?.["average-length"] >= + LONG_TEXT_MIN; + + return ( + (isLongText ? fieldSortPriorities["type/LongText"] : undefined) ?? + fieldSortPriorities[field.semantic_type ?? ""] ?? + fieldSortPriorities[field.base_type ?? ""] ?? + 900 + ); +}; + +export const sortDimensions = (a: DimensionOption, b: DimensionOption) => + getSortValue(a) - getSortValue(b); diff --git a/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterList/utils.unit.spec.ts b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterList/utils.unit.spec.ts new file mode 100644 index 0000000000000000000000000000000000000000..b62214fc3005c64a276216b6504aa3236bd35a58 --- /dev/null +++ b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterList/utils.unit.spec.ts @@ -0,0 +1,85 @@ +import { sortDimensions } from "./utils"; +import Dimension from "metabase-lib/lib/Dimension"; +import { DimensionOption } from "metabase-lib/lib/queries/StructuredQuery"; + +const mockDimensionOption = ( + semantic_type: string, + base_type: string, + text_length: number = 0, +): DimensionOption => { + const dimension = { + field: () => ({ + semantic_type, + base_type, + fingerprint: { + type: { + "type/Text": { + "average-length": text_length, + }, + }, + }, + }), + } as any; + return { dimension } as DimensionOption; +}; + +describe("sortDimensionOptions", () => { + it("should sort created-at fields before numbers", () => { + const sorted = [ + mockDimensionOption("", "type/Float"), + mockDimensionOption("type/CreationTimestamp", "type/Text"), + mockDimensionOption("", "type/Float"), + ].sort(sortDimensions); + + expect(sorted[0].dimension.field().semantic_type).toBe( + "type/CreationTimestamp", + ); + }); + + it("should sort latitudes before jsons", () => { + const sorted = [ + mockDimensionOption("", "type/JSON"), + mockDimensionOption("type/Latitude", "type/Float"), + ].sort(sortDimensions); + + expect(sorted[0].dimension.field().semantic_type).toBe("type/Latitude"); + }); + + it("should sort city before primary key", () => { + const sorted = [ + mockDimensionOption("type/PK", "type/BigInteger"), + mockDimensionOption("type/City", "type/Text"), + ].sort(sortDimensions); + + expect(sorted[0].dimension.field().semantic_type).toBe("type/City"); + }); + + it("should sort booleans before categories", () => { + const sorted = [ + mockDimensionOption("type/Category", "type/Text"), + mockDimensionOption("", "type/Boolean"), + ].sort(sortDimensions); + + expect(sorted[0].dimension.field().base_type).toBe("type/Boolean"); + }); + + it("should sort short text before long text", () => { + const sorted = [ + mockDimensionOption("", "type/Text", 400), + mockDimensionOption("", "type/Text", 10), + mockDimensionOption("", "type/Text", 200), + ].sort(sortDimensions); + + expect(sorted[0].dimension.field().base_type).toBe("type/Text"); + expect( + sorted[0].dimension.field().fingerprint.type["type/Text"][ + "average-length" + ], + ).toBe(10); + expect( + sorted[1].dimension.field().fingerprint.type["type/Text"][ + "average-length" + ], + ).toBe(400); + }); +}); diff --git a/frontend/test/metabase/scenarios/filters/filter-bulk.cy.spec.js b/frontend/test/metabase/scenarios/filters/filter-bulk.cy.spec.js index c0537910b72aab58bb8cc12f1daa78f3f40cc46d..805a92e994be229166883787d410f924d5c38482 100644 --- a/frontend/test/metabase/scenarios/filters/filter-bulk.cy.spec.js +++ b/frontend/test/metabase/scenarios/filters/filter-bulk.cy.spec.js @@ -56,6 +56,25 @@ describe("scenarios > filters > bulk filtering", () => { cy.signInAsAdmin(); }); + it("should sort database fields by relevance", () => { + visitQuestionAdhoc(rawQuestionDetails); + openFilterModal(); + + modal().within(() => { + cy.findAllByTestId("dimension-filter-label") + .eq(0) + .should("have.text", "Created At"); + + cy.findAllByTestId("dimension-filter-label") + .eq(1) + .should("have.text", "Discount"); + + cy.findAllByTestId("dimension-filter-label") + .last() + .should("include.text", "ID"); + }); + }); + it("should add a filter for a raw query", () => { visitQuestionAdhoc(rawQuestionDetails); openFilterModal();