diff --git a/frontend/src/metabase/lib/query.js b/frontend/src/metabase/lib/query.js index b7baaecd10e62ac8f6452e7341efc677c37c0772..4f498231bd13a752efaaa6be39125315b9f68540 100644 --- a/frontend/src/metabase/lib/query.js +++ b/frontend/src/metabase/lib/query.js @@ -41,6 +41,9 @@ export function createQuery(type = "query", databaseId, tableId) { return dataset_query; } +const mbqlCanonicalize = (a) => a.toLowerCase().replace(/_/g, "-"); +const mbqlCompare = (a, b) => mbqlCanonicalize(a) === mbqlCanonicalize(b) + var Query = { isStructured(dataset_query) { @@ -438,23 +441,23 @@ var Query = { }, isLocalField(field) { - return Array.isArray(field) && field[0] === "field-id"; + return Array.isArray(field) && mbqlCompare(field[0], "field-id"); }, isForeignKeyField(field) { - return Array.isArray(field) && field[0] === "fk->"; + return Array.isArray(field) && mbqlCompare(field[0], "fk->"); }, isDatetimeField(field) { - return Array.isArray(field) && field[0] === "datetime_field"; + return Array.isArray(field) && mbqlCompare(field[0], "datetime-field"); }, isExpressionField(field) { - return Array.isArray(field) && field.length === 2 && field[0] === "expression"; + return Array.isArray(field) && field.length === 2 && mbqlCompare(field[0], "expression"); }, isAggregateField(field) { - return Array.isArray(field) && field[0] === "aggregation"; + return Array.isArray(field) && mbqlCompare(field[0], "aggregation"); }, isValidField(field) { @@ -462,7 +465,10 @@ var Query = { (Query.isRegularField(field)) || (Query.isLocalField(field)) || (Query.isForeignKeyField(field) && Query.isRegularField(field[1]) && Query.isRegularField(field[2])) || - (Query.isDatetimeField(field) && Query.isValidField(field[1]) && field[2] === "as" && typeof field[3] === "string") || + (Query.isDatetimeField(field) && Query.isValidField(field[1]) && + (field.length === 4 ? + (field[2] === "as" && typeof field[3] === "string") : // deprecated + typeof field[2] === "string")) || (Query.isExpressionField(field) && _.isString(field[1])) || (Query.isAggregateField(field) && typeof field[1] === "number") ); @@ -491,17 +497,20 @@ var Query = { }, // gets the table and field definitions from from a raw, fk->, or datetime_field field - getFieldTarget: function(field, tableDef) { + getFieldTarget: function(field, tableDef, path = []) { if (Query.isRegularField(field)) { - return { table: tableDef, field: tableDef.fields_lookup && tableDef.fields_lookup[field] }; + return { table: tableDef, field: tableDef.fields_lookup && tableDef.fields_lookup[field], path }; } else if (Query.isLocalField(field)) { - return Query.getFieldTarget(field[1], tableDef); + return Query.getFieldTarget(field[1], tableDef, path); } else if (Query.isForeignKeyField(field)) { let fkFieldDef = tableDef.fields_lookup && tableDef.fields_lookup[field[1]]; let targetTableDef = fkFieldDef && fkFieldDef.target.table; - return Query.getFieldTarget(field[2], targetTableDef); + return Query.getFieldTarget(field[2], targetTableDef, path.concat(fkFieldDef)); } else if (Query.isDatetimeField(field)) { - return Query.getFieldTarget(field[1], tableDef); + return { + ...Query.getFieldTarget(field[1], tableDef, path), + unit: Query.getDatetimeUnit(field) + }; } else if (Query.isExpressionField(field)) { // hmmm, since this is a dynamic field we'll need to build this here let fieldDef = { @@ -525,13 +534,22 @@ var Query = { return { table: tableDef, - field: fieldDef + field: fieldDef, + path: path } } console.warn("Unknown field type: ", field); }, + getDatetimeUnit(field) { + if (field.length === 4) { + return field[3]; // deprecated + } else { + return field[2]; + } + }, + getFieldOptions(fields, includeJoins = false, filterFn = (fields) => fields, usedFields = {}) { var results = { count: 0, diff --git a/frontend/src/metabase/query_builder/FieldName.jsx b/frontend/src/metabase/query_builder/FieldName.jsx index 009c9d837c3f329132f72dbe719b1d52a9708bc9..6043de2bb348a4c215ad5b5e6487540e041b0d17 100644 --- a/frontend/src/metabase/query_builder/FieldName.jsx +++ b/frontend/src/metabase/query_builder/FieldName.jsx @@ -3,10 +3,9 @@ import React, { Component, PropTypes } from "react"; import Icon from "metabase/components/Icon.jsx"; import Query from "metabase/lib/query"; -import { formatBucketing, parseFieldBucketing, parseFieldTarget } from "metabase/lib/query_time"; +import { formatBucketing } from "metabase/lib/query_time"; import { stripId } from "metabase/lib/formatting"; -import _ from "underscore"; import cx from "classnames"; export default class FieldName extends Component { @@ -24,65 +23,32 @@ export default class FieldName extends Component { }; render() { - let targetTitle, fkTitle, fkIcon, bucketingTitle; - let { field, fieldOptions } = this.props; - - let bucketing = parseFieldBucketing(field); - field = parseFieldTarget(field); - - let fieldDef; - if (Array.isArray(field) && field[0] === 'fk->') { - let fkDef = _.find(fieldOptions.fks, (fk) => _.isEqual(fk.field.id, field[1])); - if (fkDef) { - fkTitle = (<span>{stripId(fkDef.field.display_name)}</span>); - fieldDef = _.find(fkDef.fields, (f) => _.isEqual(f.id, field[2])); - if (fieldDef) { - fkIcon = (<span className="px1"><Icon name="connections" width="10" height="10" /></span>); - } - } - } else if (Query.isExpressionField(field)) { - fieldDef = { - display_name: field[1] - }; - } else { - fieldDef = _.find(fieldOptions.fields, (f) => _.isEqual(f.id, field)); - } - - if (fieldDef) { - targetTitle = (<span>{fieldDef.display_name}</span>); - } - - // if this is a datetime field then add an extra bit of labeling about the time bucket - if (fieldDef && bucketing) { - bucketingTitle = ": " + formatBucketing(bucketing); - } - - var titleElement; - if (fkTitle || targetTitle) { - titleElement = <span className="QueryOption">{fkTitle}{fkIcon}{targetTitle}{bucketingTitle}</span>; - } else { - titleElement = <span className="QueryOption">field</span>; + let { field, tableMetadata, className } = this.props; + let fieldTarget = Query.getFieldTarget(field, tableMetadata); + + let parts = []; + // fk path + for (let [index, fkField] of Object.entries(fieldTarget.path)) { + parts.push(<span key={"fkName"+index}>{stripId(fkField.display_name)}</span>); + parts.push(<span key={"fkIcon"+index} className="px1"><Icon name="connections" width="10" height="10" /></span>); } - - var classes = cx({ - 'selected': Query.isValidField(field) - }); - - var removeButton; - if (this.props.removeField) { - removeButton = ( - <a className="text-grey-2 no-decoration pr1 flex align-center" onClick={this.props.removeField}> - <Icon name='close' width="14px" height="14px" /> - </a> - ) + // target field itself + parts.push(<span key="field">{fieldTarget.field.display_name}</span>); + // datetime-field unit + if (fieldTarget.unit != null) { + parts.push(<span key="unit">{": " + formatBucketing(fieldTarget.unit)}</span>); } return ( <div className="flex align-center"> - <div className={this.props.className + " " + classes} onClick={this.props.onClick}> - {titleElement} + <div className={cx(className, { selected: Query.isValidField(field) })} onClick={this.props.onClick}> + <span className="QueryOption">{parts}</span> </div> - {removeButton} + { this.props.removeField && + <a className="text-grey-2 no-decoration pr1 flex align-center" onClick={this.props.removeField}> + <Icon name='close' width="14px" height="14px" /> + </a> + } </div> ); } diff --git a/frontend/test/unit/lib/query.spec.js b/frontend/test/unit/lib/query.spec.js index 1fde115f8957d17f39af2c6e4b9c48b562116645..158f950dc770e998db4b4369ff472ce7fa37c9f4 100644 --- a/frontend/test/unit/lib/query.spec.js +++ b/frontend/test/unit/lib/query.spec.js @@ -241,6 +241,83 @@ describe('Query', () => { expect(query.order_by).toBe(undefined); }); }); + + describe('getFieldTarget', () => { + let field2 = { + display_name: "field2", + } + let table2 = { + display_name: "table2", + fields_lookup: { + 2: field2 + } + } + let field1 = { + display_name: "field1", + target: { + table: table2 + } + } + let table1 = { + display_name: "table1", + fields_lookup: { + 1: field1 + } + } + + it('should return field object for old-style local field', () => { + let target = Query.getFieldTarget(1, table1); + expect(target.table).toEqual(table1); + expect(target.field).toEqual(field1); + expect(target.path).toEqual([]); + expect(target.unit).toEqual(undefined); + }); + it('should return field object for new-style local field', () => { + let target = Query.getFieldTarget(["field-id", 1], table1); + expect(target.table).toEqual(table1); + expect(target.field).toEqual(field1); + expect(target.path).toEqual([]); + expect(target.unit).toEqual(undefined); + }); + it('should return unit object for old-style datetime_field', () => { + let target = Query.getFieldTarget(["datetime-field", 1, "as", "day"], table1); + expect(target.table).toEqual(table1); + expect(target.field).toEqual(field1); + expect(target.path).toEqual([]); + expect(target.unit).toEqual("day"); + }); + it('should return unit object for new-style datetime_field', () => { + let target = Query.getFieldTarget(["datetime-field", 1, "as", "day"], table1); + expect(target.table).toEqual(table1); + expect(target.field).toEqual(field1); + expect(target.path).toEqual([]); + expect(target.unit).toEqual("day"); + }); + + it('should return field object and table for fk field', () => { + let target = Query.getFieldTarget(["fk->", 1, 2], table1); + expect(target.table).toEqual(table2); + expect(target.field).toEqual(field2); + expect(target.path).toEqual([field1]); + expect(target.unit).toEqual(undefined); + }); + + it('should return field object and table and unit for fk + datetime field', () => { + let target = Query.getFieldTarget(["datetime-field", ["fk->", 1, 2], "day"], table1); + expect(target.table).toEqual(table2); + expect(target.field).toEqual(field2); + expect(target.path).toEqual([field1]); + expect(target.unit).toEqual("day"); + }); + + it('should return field object and table for expression', () => { + let target = Query.getFieldTarget(["expression", "foo"], table1); + expect(target.table).toEqual(table1); + expect(target.field.display_name).toEqual("foo"); + expect(target.path).toEqual([]); + expect(target.unit).toEqual(undefined); + }); + }) });