diff --git a/frontend/src/metabase-lib/lib/metadata/Table.ts b/frontend/src/metabase-lib/lib/metadata/Table.ts index 2e046785768150379ecefdf7b9047c8e9ee6ff32..f296c851599c8f3dfc0b098aa9b06938b453a525 100644 --- a/frontend/src/metabase-lib/lib/metadata/Table.ts +++ b/frontend/src/metabase-lib/lib/metadata/Table.ts @@ -5,7 +5,7 @@ import Question from "../Question"; import Schema from "./Schema"; import Base from "./Base"; import { singularize } from "metabase/lib/formatting"; -import { getAggregationOperatorsWithFields } from "metabase/lib/schema_metadata"; +import { getAggregationOperators } from "metabase/lib/schema_metadata"; import { memoize, createLookupByProperty } from "metabase-lib/lib/utils"; /** @@ -93,7 +93,7 @@ export default class Table extends Base { // AGGREGATIONS @memoize aggregationOperators() { - return getAggregationOperatorsWithFields(this); + return getAggregationOperators(this); } @memoize diff --git a/frontend/src/metabase/lib/schema_metadata.js b/frontend/src/metabase/lib/schema_metadata.js index c8184e6fee396feba26f6b7fb178b147d26eae49..03daaf9e01a99083d4fa1e9d12c229fe7a3994f7 100644 --- a/frontend/src/metabase/lib/schema_metadata.js +++ b/frontend/src/metabase/lib/schema_metadata.js @@ -695,32 +695,27 @@ function populateFields(aggregationOperator, fields) { }; } -// TODO: unit test -export function getAggregationOperators(table) { - return AGGREGATION_OPERATORS.filter( - aggregationOperator => - !( - aggregationOperator.requiredDriverFeature && - table.db && - !_.contains( - table.db.features, - aggregationOperator.requiredDriverFeature, - ) - ), - ).map(aggregationOperator => - populateFields(aggregationOperator, table.fields), - ); +export function getSupportedAggregationOperators(table) { + return AGGREGATION_OPERATORS.filter(operator => { + if (!operator.requiredDriverFeature) { + return true; + } + return ( + table.db && table.db.features.includes(operator.requiredDriverFeature) + ); + }); } -export function getAggregationOperatorsWithFields(table) { - return getAggregationOperators(table).filter( - aggregation => - !aggregation.requiresField || - aggregation.fields.every(fields => fields.length > 0), - ); +export function getAggregationOperators(table) { + return getSupportedAggregationOperators(table) + .map(operator => populateFields(operator, table.fields)) + .filter( + aggregation => + !aggregation.requiresField || + aggregation.fields.every(fields => fields.length > 0), + ); } -// TODO: unit test export function getAggregationOperator(short) { return _.findWhere(AGGREGATION_OPERATORS, { short: short }); } @@ -738,7 +733,7 @@ export function addValidOperatorsToFields(table) { for (const field of table.fields) { field.filter_operators = getFilterOperators(field, table); } - table.aggregation_operators = getAggregationOperatorsWithFields(table); + table.aggregation_operators = getAggregationOperators(table); return table; } diff --git a/frontend/test/metabase/lib/schema_metadata.unit.spec.js b/frontend/test/metabase/lib/schema_metadata.unit.spec.js index 33fc0b719c86b2a3ee183a29f3dbcb9d7eccdc6c..f99c733677095cc3fa007845f0f658c3d98e679e 100644 --- a/frontend/test/metabase/lib/schema_metadata.unit.spec.js +++ b/frontend/test/metabase/lib/schema_metadata.unit.spec.js @@ -1,3 +1,4 @@ +import _ from "underscore"; import { getFieldType, TEMPORAL, @@ -14,6 +15,8 @@ import { doesOperatorExist, getOperatorByTypeAndName, getFilterOperators, + getSupportedAggregationOperators, + getAggregationOperators, isFuzzyOperator, getSemanticTypeIcon, getSemanticTypeName, @@ -31,15 +34,18 @@ describe("schema_metadata", () => { expect(getFieldType({ effective_type: TYPE.DateTime })).toEqual(TEMPORAL); expect(getFieldType({ effective_type: TYPE.Time })).toEqual(TEMPORAL); }); + it("should know a number", () => { expect(getFieldType({ base_type: TYPE.BigInteger })).toEqual(NUMBER); expect(getFieldType({ base_type: TYPE.Integer })).toEqual(NUMBER); expect(getFieldType({ base_type: TYPE.Float })).toEqual(NUMBER); expect(getFieldType({ base_type: TYPE.Decimal })).toEqual(NUMBER); }); + it("should know a string", () => { expect(getFieldType({ base_type: TYPE.Text })).toEqual(STRING); }); + it("should know things that are types of strings", () => { expect( getFieldType({ base_type: TYPE.Text, semantic_type: TYPE.Name }), @@ -54,18 +60,22 @@ describe("schema_metadata", () => { getFieldType({ base_type: TYPE.Text, semantic_type: TYPE.URL }), ).toEqual(STRING); }); + it("should know a pk", () => { expect( getFieldType({ base_type: TYPE.Integer, semantic_type: TYPE.PK }), ).toEqual(PRIMARY_KEY); }); + it("should know a bool", () => { expect(getFieldType({ base_type: TYPE.Boolean })).toEqual(BOOLEAN); }); + it("should know a location", () => { expect(getFieldType({ semantic_type: TYPE.City })).toEqual(LOCATION); expect(getFieldType({ semantic_type: TYPE.Country })).toEqual(LOCATION); }); + it("should know a coordinate", () => { expect(getFieldType({ semantic_type: TYPE.Latitude })).toEqual( COORDINATE, @@ -74,16 +84,19 @@ describe("schema_metadata", () => { COORDINATE, ); }); + describe("should know something that is string-like", () => { it("TYPE.TextLike", () => { expect(getFieldType({ base_type: TYPE.TextLike })).toEqual(STRING_LIKE); }); + it("TYPE.IPAddress", () => { expect(getFieldType({ base_type: TYPE.IPAddress })).toEqual( STRING_LIKE, ); }); }); + it("should still recognize some types as a string regardless of its base type", () => { // TYPE.Float can occur in a field filter expect( @@ -93,6 +106,7 @@ describe("schema_metadata", () => { getFieldType({ base_type: TYPE.Float, semantic_type: TYPE.Category }), ).toEqual(STRING); }); + it("should know what it doesn't know", () => { expect(getFieldType({ base_type: "DERP DERP DERP" })).toEqual(undefined); }); @@ -102,9 +116,11 @@ describe("schema_metadata", () => { it("should work with null input", () => { expect(foreignKeyCountsByOriginTable(null)).toEqual(null); }); + it("should require an array as input", () => { expect(foreignKeyCountsByOriginTable({})).toEqual(null); }); + it("should count occurrences by origin.table.id", () => { expect( foreignKeyCountsByOriginTable([ @@ -196,6 +212,7 @@ describe("schema_metadata", () => { "not-null", ]); }); + it("should return proper filter operators for type/Text primary key", () => { expect( getFilterOperators({ @@ -215,6 +232,7 @@ describe("schema_metadata", () => { "ends-with", ]); }); + it("should return proper filter operators for type/TextLike foreign key", () => { expect( getFilterOperators({ @@ -266,4 +284,224 @@ describe("schema_metadata", () => { expect(getSemanticTypeName("foo")).toBeUndefined(); }); }); + + describe("getSupportedAggregationOperators", () => { + function getTable(features) { + return { + db: { + features, + }, + }; + } + + it("returns nothing without DB features", () => { + const table = getTable([]); + const operators = getSupportedAggregationOperators(table); + expect(operators).toHaveLength(0); + }); + + it("returns correct basic aggregation operators", () => { + const table = getTable(["basic-aggregations"]); + const operators = getSupportedAggregationOperators(table); + expect(operators.map(o => o.short)).toEqual([ + "rows", + "count", + "sum", + "avg", + "distinct", + "cum-sum", + "cum-count", + "min", + "max", + ]); + }); + + it("filters out operators not supported by database", () => { + const table = getTable(["standard-deviation-aggregations"]); + const operators = getSupportedAggregationOperators(table); + expect(operators).toEqual([ + expect.objectContaining({ + short: "stddev", + requiredDriverFeature: "standard-deviation-aggregations", + }), + ]); + }); + }); + + describe("getAggregationOperators", () => { + function setup({ fields = [] } = {}) { + const table = { + fields, + db: { + features: ["basic-aggregations", "standard-deviation-aggregations"], + }, + }; + const fullOperators = getAggregationOperators(table); + return { + fullOperators, + operators: fullOperators.map(operator => operator.short), + operatorByName: _.indexBy(fullOperators, "short"), + }; + } + + function getTypedFields(type) { + return [ + { base_type: type }, + { effective_type: type }, + { semantic_type: type }, + ]; + } + + const PK = { semantic_type: TYPE.PK }; + const FK = { semantic_type: TYPE.FK }; + const ENTITY_NAME = { semantic_type: TYPE.Name }; + const ADDRESS = { semantic_type: TYPE.Address }; + const CATEGORY = { semantic_type: TYPE.Category }; + const NUMBERS = getTypedFields(TYPE.Number); + const STRINGS = getTypedFields(TYPE.Text); + const TEMPORALS = getTypedFields(TYPE.Temporal); + + describe("count", () => { + it("offers without fields", () => { + const { operators } = setup(); + expect(operators).toEqual(expect.arrayContaining(["count"])); + }); + + it("offers 'count' with fields", () => { + const { operators } = setup({ fields: [PK, FK, STRINGS] }); + expect(operators).toEqual(expect.arrayContaining(["count"])); + }); + }); + + describe("distinct", () => { + it("is not available without fields", () => { + const { operators } = setup(); + expect(operators).toEqual(expect.not.arrayContaining(["distinct"])); + }); + + it("is available with any field", () => { + const fields = [ + PK, + FK, + ADDRESS, + CATEGORY, + ...NUMBERS, + ...STRINGS, + ...TEMPORALS, + ]; + const { operators, operatorByName } = setup({ fields }); + expect(operators).toEqual(expect.arrayContaining(["distinct"])); + expect(operatorByName.distinct.fields[0]).toHaveLength(fields.length); + }); + }); + + describe("cumulative count", () => { + it("is available without fields", () => { + const { operators } = setup(); + expect(operators).toEqual(expect.arrayContaining(["cum-count"])); + }); + + it("is available with any field", () => { + const { operators, operatorByName } = setup({ + fields: [ + PK, + FK, + ADDRESS, + CATEGORY, + ...NUMBERS, + ...STRINGS, + ...TEMPORALS, + ], + }); + expect(operators).toEqual(expect.arrayContaining(["cum-count"])); + expect(operatorByName["cum-count"].fields).toHaveLength(0); + }); + }); + + describe("summable operators", () => { + ["sum", "avg", "cum-sum", "stddev"].forEach(operator => { + describe(operator, () => { + it("is not available without fields", () => { + const { operators } = setup(); + expect(operators).toEqual(expect.not.arrayContaining([operator])); + }); + + it("is not available without summable fields", () => { + const { operators } = setup({ + fields: [PK, FK, ADDRESS, ...STRINGS, ...TEMPORALS], + }); + expect(operators).toEqual(expect.not.arrayContaining([operator])); + }); + + ["base_type", "effective_type", "semantic_type"].forEach(type => { + it(`is available with numeric field's ${type}`, () => { + const field = { [type]: TYPE.Number }; + const { operators, operatorByName } = setup({ fields: [field] }); + expect(operators).toEqual(expect.arrayContaining([operator])); + expect(operatorByName[operator].fields[0]).toEqual([field]); + }); + }); + }); + }); + }); + + describe("scoping operators", () => { + ["min", "max"].forEach(operator => { + describe(operator, () => { + it("is not available without fields", () => { + const { operators } = setup(); + expect(operators).toEqual(expect.not.arrayContaining([operator])); + }); + + it("is not available without scope fields", () => { + const { operators } = setup({ fields: [ADDRESS] }); + expect(operators).toEqual(expect.not.arrayContaining([operator])); + }); + + ["base_type", "effective_type", "semantic_type"].forEach(type => { + it(`is available with numeric field's ${type}`, () => { + const field = { [type]: TYPE.Number }; + const { operators, operatorByName } = setup({ fields: [field] }); + expect(operators).toEqual(expect.arrayContaining([operator])); + expect(operatorByName[operator].fields[0]).toEqual([field]); + }); + + it(`is available with temporal field's ${type}`, () => { + const field = { [type]: TYPE.Temporal }; + const { operators, operatorByName } = setup({ fields: [field] }); + expect(operators).toEqual(expect.arrayContaining([operator])); + expect(operatorByName[operator].fields[0]).toEqual([field]); + }); + + it(`is available with string field's ${type}`, () => { + const field = { [type]: TYPE.Text }; + const { operators, operatorByName } = setup({ fields: [field] }); + expect(operators).toEqual(expect.arrayContaining([operator])); + expect(operatorByName[operator].fields[0]).toEqual([field]); + }); + }); + + [ + { field: PK, name: "PK" }, + { field: FK, name: "FK" }, + { field: ENTITY_NAME, name: "entity name" }, + { field: CATEGORY, name: "category" }, + { field: { base_type: TYPE.Boolean }, name: "boolean (base type)" }, + { + field: { effective_type: TYPE.Boolean }, + name: "boolean (effective type)", + }, + ].forEach(testCase => { + const { field, name } = testCase; + + it(`is available for ${name} fields`, () => { + const { operators, operatorByName } = setup({ fields: [field] }); + expect(operators).toEqual(expect.arrayContaining([operator])); + expect(operatorByName[operator].fields[0]).toEqual([field]); + }); + }); + }); + }); + }); + }); });