diff --git a/bin/version b/bin/version
index 5b86e7d0bf337fd7028a69de863906770a91f540..4d1b9d6a3c3355980158c56cf0fdd39b5de208c5 100755
--- a/bin/version
+++ b/bin/version
@@ -1,6 +1,6 @@
#!/usr/bin/env bash
-VERSION="v0.22.0-snapshot"
+VERSION="v0.23.0-snapshot"
# dynamically pull more interesting stuff from latest git commit
HASH=$(git show-ref --head --hash=7 head) # first 7 letters of hash should be enough; that's what GitHub uses
diff --git a/frontend/src/metabase/components/ColorPicker.jsx b/frontend/src/metabase/components/ColorPicker.jsx
index 184325747951a5309b2d1b0ecb7f50132d56c560..076c097e7c50f88d74a744f6c1a070fb9d6b571f 100644
--- a/frontend/src/metabase/components/ColorPicker.jsx
+++ b/frontend/src/metabase/components/ColorPicker.jsx
@@ -1,60 +1,72 @@
-import React, { Component } from "react";
+import React, { Component, PropTypes } from "react";
-import Icon from "metabase/components/Icon";
import PopoverWithTrigger from "metabase/components/PopoverWithTrigger";
-import { normal, saturated, desaturated } from "metabase/lib/colors";
+import { normal } from "metabase/lib/colors";
-const COLORS = [
- ...Object.values(normal),
- ...Object.values(saturated),
- ...Object.values(desaturated),
-];
+const DEFAULT_COLOR_SQUARE_SIZE = 32;
-const COLOR_SQUARE_SIZE = 32;
-const COLOR_SQUARE = {
- width: COLOR_SQUARE_SIZE,
- height: COLOR_SQUARE_SIZE
-};
-
-const ColorSquare = ({ color }) =>
+const ColorSquare = ({ color, size }) =>
<div style={{
- ...COLOR_SQUARE,
+ width: size,
+ height: size,
backgroundColor: color,
- borderRadius: 4
+ borderRadius: size / 8
}}></div>
class ColorPicker extends Component {
+ static defaultProps = {
+ colors: [...Object.values(normal)],
+ size: DEFAULT_COLOR_SQUARE_SIZE,
+ triggerSize: DEFAULT_COLOR_SQUARE_SIZE,
+ padding: 4
+ }
+
+ static propTypes = {
+ colors: PropTypes.array,
+ onChange: PropTypes.func.isRequired,
+ size: PropTypes.number,
+ triggerSize: PropTypes.number,
+ value: PropTypes.string
+ }
+
render () {
- const { value, onChange } = this.props;
+ const { colors, onChange, padding, size, triggerSize, value } = this.props;
return (
<div className="inline-block">
<PopoverWithTrigger
ref="colorPopover"
triggerElement={
- <div className="bordered p1 rounded flex align-center">
- <ColorSquare color={value} />
- <Icon
- className="ml1"
- name="chevrondown"
- />
+ <div
+ className="bordered rounded flex align-center"
+ style={{ padding: triggerSize / 4 }}
+ >
+ <ColorSquare color={value} size={triggerSize} />
</div>
}
>
- <ol className="flex p1">
- { COLORS.map((color, index) =>
- <li
- className="cursor-pointer mr1 mb1"
- key={index}
- onClick={() => {
- onChange(color);
- this.refs.colorPopover.close();
- }}
- >
- <ColorSquare color={color} />
- </li>
- )}
- </ol>
+ <div className="p1">
+ <ol
+ className="flex flex-wrap"
+ style={{
+ maxWidth: 120
+ }}
+ >
+ { colors.map((color, index) =>
+ <li
+ className="cursor-pointer"
+ style={{ padding }}
+ key={index}
+ onClick={() => {
+ onChange(color);
+ this.refs.colorPopover.close();
+ }}
+ >
+ <ColorSquare color={color} size={size} />
+ </li>
+ )}
+ </ol>
+ </div>
</PopoverWithTrigger>
</div>
);
diff --git a/frontend/src/metabase/components/HeaderBar.jsx b/frontend/src/metabase/components/HeaderBar.jsx
index fe4ead539e062161064cb7a326c1bbae02957f14..5a67451b39f67b4906b083486018f5df22e6f04e 100644
--- a/frontend/src/metabase/components/HeaderBar.jsx
+++ b/frontend/src/metabase/components/HeaderBar.jsx
@@ -43,7 +43,7 @@ export default class Header extends Component {
return (
<div className={cx("QueryBuilder-section flex align-center", className)}>
- <div className={cx("Entity py1 relative", { "pt2": badge })}>
+ <div className={cx("py1 relative flex-full", { "pt2": badge })}>
{ badge &&
<div className="absolute top left">{badge}</div>
}
diff --git a/frontend/src/metabase/components/Modal.jsx b/frontend/src/metabase/components/Modal.jsx
index dfd2ea3bf77f798360f03b3757577e50b7dbed98..f36654e2fddd45959368b6285be20460d3dcf413 100644
--- a/frontend/src/metabase/components/Modal.jsx
+++ b/frontend/src/metabase/components/Modal.jsx
@@ -116,13 +116,13 @@ export class FullPageModal extends Component {
this._modalElement.className = "Modal--full";
document.querySelector('body').appendChild(this._modalElement);
- this.componentDidUpdate();
-
// save the scroll position, scroll to the top left, and disable scrolling
this._scrollX = window.scrollX;
this._scrollY = window.scrollY;
window.scrollTo(0,0);
document.body.style.overflow = "hidden";
+
+ this.componentDidUpdate();
}
componentDidUpdate() {
diff --git a/frontend/src/metabase/css/components/modal.css b/frontend/src/metabase/css/components/modal.css
index ba111df10102426b27652f3c19f14f690d0e4c79..1cc813e3dfbe8d8763cb6274f176acd245d917a7 100644
--- a/frontend/src/metabase/css/components/modal.css
+++ b/frontend/src/metabase/css/components/modal.css
@@ -1,5 +1,5 @@
.ModalContainer {
- z-index: 3;
+ z-index: 4;
}
.Modal {
diff --git a/frontend/src/metabase/css/home.css b/frontend/src/metabase/css/home.css
index 4e301f009750f23d0f3afde5ba30955f0bb5028a..f16e6f9dbfcd5d4f0df8b75450ed60b232c2a21b 100644
--- a/frontend/src/metabase/css/home.css
+++ b/frontend/src/metabase/css/home.css
@@ -1,5 +1,5 @@
.Nav {
- z-index: 3;
+ z-index: 4;
}
.CheckBg {
diff --git a/frontend/src/metabase/lib/expressions/index.js b/frontend/src/metabase/lib/expressions/index.js
index e12463eba035c88843a7f6549fca59f27c84237e..fa83efa0f87b584ba4fb63c6449321cb891bf600 100644
--- a/frontend/src/metabase/lib/expressions/index.js
+++ b/frontend/src/metabase/lib/expressions/index.js
@@ -6,12 +6,26 @@ import { VALID_OPERATORS, VALID_AGGREGATIONS } from "./tokens";
export { VALID_OPERATORS, VALID_AGGREGATIONS } from "./tokens";
+const AGG_NAMES_MAP = new Map(Array.from(VALID_AGGREGATIONS).map(([short, displayName]) =>
+ // case-insensitive
+ [displayName.toLowerCase(), short]
+));
+
+export function getAggregationFromName(name) {
+ // case-insensitive
+ return AGG_NAMES_MAP.get(name.toLowerCase());
+}
+
+export function isReservedWord(word) {
+ return !!getAggregationFromName(word);
+}
+
export function formatAggregationName(aggregationOption) {
return VALID_AGGREGATIONS.get(aggregationOption.short);
}
function formatIdentifier(name) {
- return /^\w+$/.test(name) ?
+ return /^\w+$/.test(name) && !isReservedWord(name) ?
name :
JSON.stringify(name);
}
diff --git a/frontend/src/metabase/lib/expressions/parser.js b/frontend/src/metabase/lib/expressions/parser.js
index d0ef13d025968e9737cfb459cab55ac86b2e2cd4..2746330d4a8398a74e5757c84152027bcccd47b2 100644
--- a/frontend/src/metabase/lib/expressions/parser.js
+++ b/frontend/src/metabase/lib/expressions/parser.js
@@ -2,10 +2,9 @@ import { Lexer, Parser, getImage } from "chevrotain";
import _ from "underscore";
-import { formatFieldName, formatExpressionName, formatAggregationName } from "../expressions";
+import { formatFieldName, formatExpressionName, formatAggregationName, getAggregationFromName } from "../expressions";
import {
- VALID_AGGREGATIONS,
allTokens,
LParen, RParen,
AdditiveOperator, MultiplicativeOperator,
@@ -16,8 +15,6 @@ import {
const ExpressionsLexer = new Lexer(allTokens);
-const aggregationsMap = new Map(Array.from(VALID_AGGREGATIONS).map(([a,b]) => [b,a]));
-
class ExpressionsParser extends Parser {
constructor(input, options = {}) {
const parserOptions = {
@@ -195,7 +192,7 @@ class ExpressionsParserMBQL extends ExpressionsParser {
return initial;
}
_aggregation(aggregation, lParen, arg, rParen) {
- const agg = aggregationsMap.get(aggregation.image)
+ const agg = getAggregationFromName(getImage(aggregation));
return arg == null ? [agg] : [agg, arg];
}
_metricReference(metricName, metricId) {
@@ -395,7 +392,7 @@ export function suggest(source, {
if (!outsideAggregation) {
let fields = [];
if (startRule === "aggregation" && currentAggregationToken) {
- let aggregationShort = aggregationsMap.get(getImage(currentAggregationToken));
+ let aggregationShort = getAggregationFromName(getImage(currentAggregationToken));
let aggregationOption = _.findWhere(tableMetadata.aggregation_options, { short: aggregationShort });
fields = aggregationOption && aggregationOption.fields && aggregationOption.fields[0] || []
} else if (startRule === "expression") {
diff --git a/frontend/src/metabase/lib/query.js b/frontend/src/metabase/lib/query.js
index 6ad1e12a27bd0c5961a83981cf32ae2f7cbfa7ef..5070477ff69d6820c9ae44be21b0f8b6953d56fc 100644
--- a/frontend/src/metabase/lib/query.js
+++ b/frontend/src/metabase/lib/query.js
@@ -487,11 +487,15 @@ var Query = {
getAggregationDescription(tableMetadata, query, options) {
return conjunctList(Query.getAggregations(query).map(aggregation => {
+ if (NamedClause.isNamed(aggregation)) {
+ return [NamedClause.getName(aggregation)];
+ }
+ if (AggregationClause.isMetric(aggregation)) {
+ let metric = _.findWhere(tableMetadata.metrics, { id: AggregationClause.getMetric(aggregation) });
+ let name = metric ? metric.name : "[Unknown Metric]";
+ return [options.jsx ? <span className="text-green text-bold">{name}</span> : name];
+ }
switch (aggregation[0]) {
- case "METRIC":
- let metric = _.findWhere(tableMetadata.metrics, { id: aggregation[1] });
- let name = metric ? metric.name : "[Unknown Metric]";
- return [options.jsx ? <span className="text-green text-bold">{name}</span> : name];
case "rows": return ["Raw data"];
case "count": return ["Count"];
case "cum_count": return ["Cumulative count"];
diff --git a/frontend/src/metabase/query_builder/components/DownloadWidget.jsx b/frontend/src/metabase/query_builder/components/DownloadWidget.jsx
index bf0fb87cb2a7c908bceb13284d691251c69c6039..1ecef684daedaa5c42c2e11af0eb7ff1f2369be2 100644
--- a/frontend/src/metabase/query_builder/components/DownloadWidget.jsx
+++ b/frontend/src/metabase/query_builder/components/DownloadWidget.jsx
@@ -7,6 +7,8 @@ import DownloadButton from "metabase/components/DownloadButton.jsx";
import FieldSet from "metabase/components/FieldSet.jsx";
+import _ from "underscore";
+
const DownloadWidget = ({ className, card, datasetQuery, isLarge }) =>
<PopoverWithTrigger
triggerElement={<Icon className={className} title="Download this data" name='download' size={16} />}
@@ -29,7 +31,8 @@ const DownloadWidget = ({ className, card, datasetQuery, isLarge }) =>
}
params={card.id != null ?
{ parameters: JSON.stringify(datasetQuery.parameters) } :
- { query: JSON.stringify(datasetQuery) }
+ // exclude `constraints` to ensure we download all rows (up to hard-coded 1M):
+ { query: JSON.stringify(_.omit(datasetQuery, "constraints")) }
}
extensions={[type]}
>
diff --git a/frontend/src/metabase/questions/collections.js b/frontend/src/metabase/questions/collections.js
index 119f1b28aeb09d46ed5337f10d6e5b1427bab00a..98ffd30aa0967674657997ae18af6a66f19d9069 100644
--- a/frontend/src/metabase/questions/collections.js
+++ b/frontend/src/metabase/questions/collections.js
@@ -1,7 +1,7 @@
import { createAction, createThunkAction, handleActions, combineReducers } from "metabase/lib/redux";
import { reset } from 'redux-form';
-import { push } from "react-router-redux";
+import { replace } from "react-router-redux";
import Urls from "metabase/lib/urls";
import _ from "underscore";
@@ -37,7 +37,8 @@ export const saveCollection = createThunkAction(SAVE_COLLECTION, (collection) =>
if (response.id != null) {
dispatch(reset("collection"));
}
- dispatch(push(Urls.collection(response)));
+ // use `replace` so form url doesn't appear in history
+ dispatch(replace(Urls.collection(response)));
return response;
} catch (e) {
// redux-form expects an object with either { field: error } or { _error: error }
diff --git a/frontend/src/metabase/questions/containers/CollectionEditorForm.jsx b/frontend/src/metabase/questions/containers/CollectionEditorForm.jsx
index 4b530c66804580718b8866f3c5d15c818873f0c3..834a5d4a1a6a19d51a2a1c3a204a763d8193defe 100644
--- a/frontend/src/metabase/questions/containers/CollectionEditorForm.jsx
+++ b/frontend/src/metabase/questions/containers/CollectionEditorForm.jsx
@@ -8,6 +8,16 @@ import Modal from "metabase/components/Modal";
import { reduxForm } from "redux-form";
+import { normal } from "metabase/lib/colors";
+
+const COLLECTION_COLORS = [
+ ...Object.values(normal),
+ '#F1B556',
+ '#A6E7F3',
+ '#7172AD',
+ '#7B8797',
+];
+
@reduxForm({
form: 'collection',
fields: ['id', 'name', 'description', 'color'],
@@ -23,7 +33,12 @@ import { reduxForm } from "redux-form";
}
return errors;
},
- initialValues: { name: "", description: "", color: "#509EE3" }
+ initialValues: {
+ name: "",
+ description: "",
+ // pick a random color to start so everything isn't blue all the time
+ color: COLLECTION_COLORS[Math.floor(Math.random() * COLLECTION_COLORS.length)]
+ }
})
export default class CollectionEditorForm extends Component {
render() {
@@ -71,6 +86,7 @@ export default class CollectionEditorForm extends Component {
>
<ColorPicker
{...fields.color}
+ colors={COLLECTION_COLORS}
/>
</FormField>
</div>
diff --git a/frontend/src/metabase/questions/containers/CollectionPage.jsx b/frontend/src/metabase/questions/containers/CollectionPage.jsx
index ee97c700ec86fc04e884bee96f934497e819bf16..45f73d2de67caf8169bab7415611dd44dd58f398 100644
--- a/frontend/src/metabase/questions/containers/CollectionPage.jsx
+++ b/frontend/src/metabase/questions/containers/CollectionPage.jsx
@@ -22,7 +22,7 @@ const mapDispatchToProps = ({
goBack,
goToQuestions: () => push(`/questions`),
editCollection: (id) => push(`/collections/${id}`),
- editPermissions: (id) => push(`/collections/permissions?collection=${id}`),
+ editPermissions: (id) => push(`/collections/permissions?collectionId=${id}`),
loadCollections,
})
diff --git a/frontend/src/metabase/visualizations/components/settings/ChartSettingColorPicker.jsx b/frontend/src/metabase/visualizations/components/settings/ChartSettingColorPicker.jsx
index 631b8506451958f3f902ab703e9374d4aaa307f8..2a2731d01a5f75c9acaac6b3a1efc1eab116c7f4 100644
--- a/frontend/src/metabase/visualizations/components/settings/ChartSettingColorPicker.jsx
+++ b/frontend/src/metabase/visualizations/components/settings/ChartSettingColorPicker.jsx
@@ -1,46 +1,13 @@
import React, { Component, PropTypes } from "react";
-import { normal } from 'metabase/lib/colors'
-const DEFAULT_COLOR_HARMONY = Object.values(normal);
-
-import PopoverWithTrigger from "metabase/components/PopoverWithTrigger.jsx";
+import ColorPicker from "metabase/components/ColorPicker";
export default class ChartSettingColorPicker extends Component {
render() {
- const { value, onChange, title } = this.props;
return (
- <div className="flex align-center">
- <PopoverWithTrigger
- ref="colorPopover"
- hasArrow={false}
- tetherOptions={{
- attachment: 'middle left',
- targetAttachment: 'middle right',
- targetOffset: '0 0',
- constraints: [{ to: 'window', attachment: 'together', pin: ['left', 'right']}]
- }}
- triggerElement={
- <span className="ml1 mr2 bordered inline-block cursor-pointer" style={{ padding: 4, borderRadius: 3 }}>
- <div style={{ width: 15, height: 15, backgroundColor: value }} />
- </span>
- }
- >
- <ol className="p1">
- {DEFAULT_COLOR_HARMONY.map((color, colorIndex) =>
- <li
- key={colorIndex}
- className="CardSettings-colorBlock"
- style={{ backgroundColor: color }}
- onClick={() => {
- onChange(color);
- this.refs.colorPopover.close();
- }}
- ></li>
- )}
- </ol>
- </PopoverWithTrigger>
-
- <span className="text-bold">{title}</span>
+ <div className="flex align-center mb1">
+ <ColorPicker {...this.props} triggerSize={12} />
+ {this.props.title && <h4 className="ml1">{this.props.title}</h4>}
</div>
);
}
diff --git a/frontend/src/metabase/visualizations/components/settings/ChartSettingColorsPicker.jsx b/frontend/src/metabase/visualizations/components/settings/ChartSettingColorsPicker.jsx
index 9ca6cae19b3b9397fc44162c89f360e0990e87a2..740e52a670fe48a8c43a12c4283f0398e0c03696 100644
--- a/frontend/src/metabase/visualizations/components/settings/ChartSettingColorsPicker.jsx
+++ b/frontend/src/metabase/visualizations/components/settings/ChartSettingColorsPicker.jsx
@@ -10,9 +10,15 @@ export default class ChartSettingColorsPicker extends Component {
{ seriesTitles.map((title, index) =>
<ChartSettingColorPicker
key={index}
- value={value[index]}
- onChange={(color) => onChange([...value.slice(0, index), color, ...value.slice(index + 1)])}
+ onChange={color =>
+ onChange([
+ ...value.slice(0, index),
+ color,
+ ...value.slice(index + 1)
+ ])
+ }
title={title}
+ value={value[index]}
/>
)}
</div>
diff --git a/frontend/test/unit/lib/expressions/formatter.spec.js b/frontend/test/unit/lib/expressions/formatter.spec.js
index 0cd7715fcd1d7239ceb32fd779732e7450ac4fc8..adea7131fbcd83480b91a42e14a5c64d337a00cc 100644
--- a/frontend/test/unit/lib/expressions/formatter.spec.js
+++ b/frontend/test/unit/lib/expressions/formatter.spec.js
@@ -6,7 +6,9 @@ const mockMetadata = {
{id: 1, display_name: "A"},
{id: 2, display_name: "B"},
{id: 3, display_name: "C"},
- {id: 10, display_name: "Toucan Sam"}
+ {id: 10, display_name: "Toucan Sam"},
+ {id: 11, display_name: "count"},
+ {id: 12, display_name: "Count"}
],
metrics: [
{id: 1, name: "foo bar"},
@@ -29,6 +31,11 @@ describe("lib/expressions/parser", () => {
expect(format(["+", ["/", ["field-id", 1], ["field-id", 10]], ["field-id", 3]], mockMetadata)).toEqual("(A / \"Toucan Sam\") + C");
});
+ it("quotes fields that conflict with reserved words", () => {
+ expect(format(["+", 1, ["field-id", 11]], mockMetadata)).toEqual('1 + "count"');
+ expect(format(["+", 1, ["field-id", 12]], mockMetadata)).toEqual('1 + "Count"');
+ });
+
it("format aggregations", () => {
expect(format(["count"], mockMetadata)).toEqual("Count");
expect(format(["sum", ["field-id", 1]], mockMetadata)).toEqual("Sum(A)");
diff --git a/frontend/test/unit/lib/expressions/parser.spec.js b/frontend/test/unit/lib/expressions/parser.spec.js
index 151c27319432d494072983bbdb11929088274dbc..28064e1743ddb4f2d2d58bf1b962fd187a92187e 100644
--- a/frontend/test/unit/lib/expressions/parser.spec.js
+++ b/frontend/test/unit/lib/expressions/parser.spec.js
@@ -7,7 +7,8 @@ const mockMetadata = {
{id: 1, display_name: "A"},
{id: 2, display_name: "B"},
{id: 3, display_name: "C"},
- {id: 10, display_name: "Toucan Sam"}
+ {id: 10, display_name: "Toucan Sam"},
+ {id: 11, display_name: "count"}
],
metrics: [
{id: 1, name: "foo bar"},
@@ -91,6 +92,12 @@ describe("lib/expressions/parser", () => {
expect(() => compile("1 + ", expressionOpts)).toThrow();
});
+ it("should treat aggregations as case-insensitive", () => {
+ expect(compile("count", aggregationOpts)).toEqual(["count"]);
+ expect(compile("cOuNt", aggregationOpts)).toEqual(["count"]);
+ expect(compile("average(A)", aggregationOpts)).toEqual(["avg", ["field-id", 1]]);
+ });
+
// fks
// multiple tables with the same field name resolution
});
@@ -107,7 +114,10 @@ describe("lib/expressions/parser", () => {
})
it("should suggest fields after an operator", () => {
expect(cleanSuggestions(suggest("1 + ", expressionOpts))).toEqual([
+ // quoted because has a space
{ type: 'fields', text: '"Toucan Sam" ' },
+ // quoted because conflicts with aggregation
+ { type: 'fields', text: '"count" ' },
{ type: 'fields', text: 'A ' },
{ type: 'fields', text: 'B ' },
{ type: 'fields', text: 'C ' },
@@ -121,9 +131,16 @@ describe("lib/expressions/parser", () => {
})
it("should suggest partial matches in expression", () => {
expect(cleanSuggestions(suggest("1 + C", expressionOpts))).toEqual([
+ { type: 'fields', text: '"count" ' },
{ type: 'fields', text: 'C ' },
]);
})
+ it("should suggest partial matches after an aggregation", () => {
+ expect(cleanSuggestions(suggest("average(c", expressionOpts))).toEqual([
+ { type: 'fields', text: '"count" ' },
+ { type: 'fields', text: 'C ' }
+ ]);
+ })
})
describe("compile() in syntax mode", () => {
diff --git a/frontend/test/unit/lib/query.spec.js b/frontend/test/unit/lib/query.spec.js
index 2f9aade317f18a2b2f388d67d4f2b02d5b4fe6f6..c548429682a51359782f86dfd920993c49f3cda9 100644
--- a/frontend/test/unit/lib/query.spec.js
+++ b/frontend/test/unit/lib/query.spec.js
@@ -1,6 +1,12 @@
import Query from "metabase/lib/query";
import { createQuery, AggregationClause, BreakoutClause } from "metabase/lib/query";
+const mockTableMetadata = {
+ display_name: "Order",
+ fields: [
+ { id: 1, display_name: "Total" }
+ ]
+}
describe('Query', () => {
describe('createQuery', () => {
@@ -284,6 +290,20 @@ describe('Query', () => {
})
});
+describe("generateQueryDescription", () => {
+ it("should work with multiple aggregations", () => {
+ expect(Query.generateQueryDescription(mockTableMetadata, {
+ source_table: 1,
+ aggregation: [["count"], ["sum", ["field-id", 1]]]
+ })).toEqual("Orders, Count and Sum of Total")
+ })
+ it("should work with named aggregations", () => {
+ expect(Query.generateQueryDescription(mockTableMetadata, {
+ source_table: 1,
+ aggregation: [["named", ["sum", ["field-id", 1]], "Revenue"]]
+ })).toEqual("Orders, Revenue")
+ })
+})
describe('AggregationClause', () => {
diff --git a/src/metabase/api/card.clj b/src/metabase/api/card.clj
index d205feba259d27fe23b66617360b47a355886154..02ba3e9dfa2dc63a2dde0d3145884979ab995686 100644
--- a/src/metabase/api/card.clj
+++ b/src/metabase/api/card.clj
@@ -324,12 +324,12 @@
;;; ------------------------------------------------------------ Running a Query ------------------------------------------------------------
-(defn- run-query-for-card [card-id parameters]
+(defn- run-query-for-card [card-id parameters constraints]
{:pre [(u/maybe? sequential? parameters)]}
(let [card (read-check Card card-id)
query (assoc (:dataset_query card)
:parameters parameters
- :constraints dataset-api/query-constraints)
+ :constraints constraints)
options {:executed-by *current-user-id*
:card-id card-id}]
(qp/dataset-query query options)))
@@ -337,19 +337,19 @@
(defendpoint POST "/:card-id/query"
"Run the query associated with a Card."
[card-id :as {{:keys [parameters]} :body}]
- (run-query-for-card card-id parameters))
+ (run-query-for-card card-id parameters dataset-api/query-constraints))
(defendpoint POST "/:card-id/query/csv"
"Run the query associated with a Card, and return its results as CSV. Note that this expects the parameters as serialized JSON in the 'parameters' parameter"
[card-id parameters]
{parameters (s/maybe su/JSONString)}
- (dataset-api/as-csv (run-query-for-card card-id (json/parse-string parameters keyword))))
+ (dataset-api/as-csv (run-query-for-card card-id (json/parse-string parameters keyword) nil)))
(defendpoint POST "/:card-id/query/json"
"Run the query associated with a Card, and return its results as JSON. Note that this expects the parameters as serialized JSON in the 'parameters' parameter"
[card-id parameters]
{parameters (s/maybe su/JSONString)}
- (dataset-api/as-json (run-query-for-card card-id (json/parse-string parameters keyword))))
+ (dataset-api/as-json (run-query-for-card card-id (json/parse-string parameters keyword) nil)))
(define-routes)
diff --git a/src/metabase/api/dataset.clj b/src/metabase/api/dataset.clj
index 42c7789c124eb657047527b80fcc82ce8c9c7815..9fc602ee412f4f90230cee28a86dd270a648c334 100644
--- a/src/metabase/api/dataset.clj
+++ b/src/metabase/api/dataset.clj
@@ -85,7 +85,7 @@
{query su/JSONString}
(let [query (json/parse-string query keyword)]
(read-check Database (:database query))
- (as-csv (qp/dataset-query query {:executed-by *current-user-id*}))))
+ (as-csv (qp/dataset-query (dissoc query :constraints) {:executed-by *current-user-id*}))))
(defendpoint POST "/json"
"Execute a query and download the result data as a JSON file."
@@ -93,7 +93,7 @@
{query su/JSONString}
(let [query (json/parse-string query keyword)]
(read-check Database (:database query))
- (as-json (qp/dataset-query query {:executed-by *current-user-id*}))))
+ (as-json (qp/dataset-query (dissoc query :constraints) {:executed-by *current-user-id*}))))
(define-routes)
diff --git a/src/metabase/api/segment.clj b/src/metabase/api/segment.clj
index 47eb004c7f8de5dd94eadd1800f31f680167118b..054571bd0d177a02143f2c03550a192f92da93bc 100644
--- a/src/metabase/api/segment.clj
+++ b/src/metabase/api/segment.clj
@@ -33,7 +33,7 @@
(defendpoint GET "/"
"Fetch *all* `Segments`."
[]
- (filter models/can-read? (-> (db/select Segment, :is_active true)
+ (filter models/can-read? (-> (db/select Segment, :is_active true, {:order-by [[:%lower.name :asc]]})
(hydrate :creator))))
diff --git a/src/metabase/driver.clj b/src/metabase/driver.clj
index c2e2a1a2887dca4ef1bd1d596f9743bfd521f877..efd574a575c881cb340d2a035da1895a2ab38f4f 100644
--- a/src/metabase/driver.clj
+++ b/src/metabase/driver.clj
@@ -145,6 +145,12 @@
The lazy sequence should not return more than `max-sync-lazy-seq-results`, which is currently `10000`.
For drivers that provide a chunked implementation, a recommended chunk size is `field-values-lazy-seq-chunk-size`, which is currently `500`.")
+ (format-custom-field-name ^String [this, ^String custom-field-name]
+ "*OPTIONAL*. Return the custom name passed via an MBQL `:named` clause so it matches the way it is returned in the results.
+ This is used by the post-processing annotation stage to find the correct metadata to include with fields in the results.
+ The default implementation is `identity`, meaning the resulting field will have exactly the same name as passed to the `:named` clause.
+ Certain drivers like Redshift always lowercase these names, so this method is provided for those situations.")
+
(humanize-connection-error-message ^String [this, ^String message]
"*OPTIONAL*. Return a humanized (user-facing) version of an connection error message string.
Generic error messages are provided in the constant `connection-error-messages`; return one of these whenever possible.")
@@ -236,6 +242,7 @@
:date-interval (u/drop-first-arg u/relative-date)
:describe-table-fks (constantly nil)
:features (constantly nil)
+ :format-custom-field-name (u/drop-first-arg identity)
:humanize-connection-error-message (u/drop-first-arg identity)
:notify-database-updated (constantly nil)
:process-query-in-context (u/drop-first-arg identity)
diff --git a/src/metabase/driver/bigquery.clj b/src/metabase/driver/bigquery.clj
index abd938494cc07cdf4e40130149dc8b6cb295d88a..3b720948b1391f37b053d4077638b440533537c2 100644
--- a/src/metabase/driver/bigquery.clj
+++ b/src/metabase/driver/bigquery.clj
@@ -87,15 +87,17 @@
{:pre [client (seq project-id) (seq dataset-id) (seq table-id)]}
(google/execute (.get (.tables client) project-id dataset-id table-id))))
-(def ^:private ^:const bigquery-type->base-type
- {"BOOLEAN" :type/Boolean
- "FLOAT" :type/Float
- "INTEGER" :type/Integer
- "RECORD" :type/Dictionary ; RECORD -> field has a nested schema
- "STRING" :type/Text
- "DATE" :type/Date
- "DATETIME" :type/DateTime
- "TIMESTAMP" :type/DateTime})
+(defn- bigquery-type->base-type [field-type]
+ (case field-type
+ "BOOLEAN" :type/Boolean
+ "FLOAT" :type/Float
+ "INTEGER" :type/Integer
+ "RECORD" :type/Dictionary ; RECORD -> field has a nested schema
+ "STRING" :type/Text
+ "DATE" :type/Date
+ "DATETIME" :type/DateTime
+ "TIMESTAMP" :type/DateTime
+ :type/*))
(defn- table-schema->metabase-field-info [^TableSchema schema]
(for [^TableFieldSchema field (.getFields schema)]
diff --git a/src/metabase/driver/crate.clj b/src/metabase/driver/crate.clj
index 38417e30a249bb18b9e66b088e13e2a6e9750172..00cef3a39a50eea126019b0060b006a3059e6731 100644
--- a/src/metabase/driver/crate.clj
+++ b/src/metabase/driver/crate.clj
@@ -1,6 +1,5 @@
(ns metabase.driver.crate
(:require [clojure.java.jdbc :as jdbc]
- [clojure.set :as set]
[honeysql.core :as hsql]
[metabase.driver :as driver]
[metabase.driver.crate.util :as crate-util]
@@ -69,7 +68,7 @@
:details-fields (constantly [{:name "hosts"
:display-name "Hosts"
:default "localhost:5432"}])
- :features (comp (u/rpartial set/difference #{:foreign-keys}) sql/features)})
+ :features (comp (u/rpartial disj :foreign-keys) sql/features)})
sql/ISQLDriver
(merge (sql/ISQLDriverDefaultsMixin)
{:connection-details->spec (u/drop-first-arg crate-spec)
diff --git a/src/metabase/driver/druid/query_processor.clj b/src/metabase/driver/druid/query_processor.clj
index c617e1a7b7328c02ffba3c49237d8187a4ea2e4b..c4bd5f2e763fd897d3767e6e2ec1fc5c448a4587 100644
--- a/src/metabase/driver/druid/query_processor.clj
+++ b/src/metabase/driver/druid/query_processor.clj
@@ -258,7 +258,7 @@
(defn- expression-post-aggregation [{:keys [operator args], :as expression}]
{:type :arithmetic
- :name (annotate/expression-aggregation-name expression)
+ :name (annotate/aggregation-name expression)
:fn operator
:fields (for [arg args]
(cond
diff --git a/src/metabase/driver/generic_sql/query_processor.clj b/src/metabase/driver/generic_sql/query_processor.clj
index 7841d353e6e0ecec2ad6832e9cde59dd106094f0..f8cc780a6fbc5199fdfdfce774e6716962993400 100644
--- a/src/metabase/driver/generic_sql/query_processor.clj
+++ b/src/metabase/driver/generic_sql/query_processor.clj
@@ -151,11 +151,11 @@
(defn- apply-expression-aggregation [driver honeysql-form expression]
(h/merge-select honeysql-form [(expression-aggregation->honeysql driver expression)
- (hx/escape-dots (annotate/expression-aggregation-name expression))]))
+ (hx/escape-dots (annotate/aggregation-name expression))]))
(defn- apply-single-aggregation [driver honeysql-form {:keys [aggregation-type field], :as aggregation}]
(h/merge-select honeysql-form [(aggregation->honeysql driver aggregation-type field)
- (hx/escape-dots (annotate/expression-aggregation-name aggregation))]))
+ (hx/escape-dots (annotate/aggregation-name aggregation))]))
(defn apply-aggregation
"Apply a `aggregation` clauses to HONEYSQL-FORM. Default implementation of `apply-aggregation` for SQL drivers."
diff --git a/src/metabase/driver/googleanalytics.clj b/src/metabase/driver/googleanalytics.clj
index 98c1ca524293bdd636fd4ecb85f748e7a017e6c9..5babdde581b53674e452160a0fe47de60e766346 100644
--- a/src/metabase/driver/googleanalytics.clj
+++ b/src/metabase/driver/googleanalytics.clj
@@ -82,7 +82,7 @@
"ga:hour"
"ga:dayOfWeek"
"ga:day"
- "ga:yearWeek"
+ "ga:isoYearIsoWeek"
"ga:week"
"ga:yearMonth"
"ga:month"
diff --git a/src/metabase/driver/googleanalytics/query_processor.clj b/src/metabase/driver/googleanalytics/query_processor.clj
index 3f6143f155b0a260c9c09d71479abf30ea4a2b13..25db1a5213f4bc3d22d667c23ee3f83856acb8c1 100644
--- a/src/metabase/driver/googleanalytics/query_processor.clj
+++ b/src/metabase/driver/googleanalytics/query_processor.clj
@@ -86,7 +86,7 @@
:day "ga:date"
:day-of-week "ga:dayOfWeek"
:day-of-month "ga:day"
- :week "ga:yearWeek"
+ :week "ga:isoYearIsoWeek"
:week-of-year "ga:week"
:month "ga:yearMonth"
:month-of-year "ga:month"
@@ -208,17 +208,17 @@
(edn/read-string (s/replace s #"^0+(.+)$" "$1")))
(def ^:private ga-dimension->date-format-fn
- {"ga:minute" parse-number
- "ga:dateHour" (partial u/parse-date "yyyyMMddHH")
- "ga:hour" parse-number
- "ga:date" (partial u/parse-date "yyyyMMdd")
- "ga:dayOfWeek" (comp inc parse-number)
- "ga:day" parse-number
- "ga:yearWeek" (partial u/parse-date "YYYYww")
- "ga:week" parse-number
- "ga:yearMonth" (partial u/parse-date "yyyyMM")
- "ga:month" parse-number
- "ga:year" parse-number})
+ {"ga:minute" parse-number
+ "ga:dateHour" (partial u/parse-date "yyyyMMddHH")
+ "ga:hour" parse-number
+ "ga:date" (partial u/parse-date "yyyyMMdd")
+ "ga:dayOfWeek" (comp inc parse-number)
+ "ga:day" parse-number
+ "ga:isoYearIsoWeek" (partial u/parse-date "YYYYww")
+ "ga:week" parse-number
+ "ga:yearMonth" (partial u/parse-date "yyyyMM")
+ "ga:month" parse-number
+ "ga:year" parse-number})
(defn- header->column [^GaData$ColumnHeaders header]
(let [date-parser (ga-dimension->date-format-fn (.getName header))]
diff --git a/src/metabase/driver/mongo.clj b/src/metabase/driver/mongo.clj
index 6ebee0cef2d8340681fd80d1139fc3bc96dcadaf..22fba6b25d9dab1bb25983fa261ec15bc70bcddb 100644
--- a/src/metabase/driver/mongo.clj
+++ b/src/metabase/driver/mongo.clj
@@ -1,7 +1,6 @@
(ns metabase.driver.mongo
"MongoDB Driver."
- (:require (clojure [set :as set]
- [string :as s])
+ (:require [clojure.string :as s]
[clojure.tools.logging :as log]
[cheshire.core :as json]
(monger [collection :as mc]
@@ -116,7 +115,7 @@
(defn- describe-database [database]
(with-mongo-connection [^com.mongodb.DB conn database]
- {:tables (set (for [collection (set/difference (mdb/get-collection-names conn) #{"system.indexes"})]
+ {:tables (set (for [collection (disj (mdb/get-collection-names conn) "system.indexes")]
{:schema nil, :name collection}))}))
(defn- describe-table [database table]
diff --git a/src/metabase/driver/redshift.clj b/src/metabase/driver/redshift.clj
index 51fe8db4fc3b13ac029b390379692c6b5f1456ef..08ebe41f5dac576be673bc5e7585ca6637099420 100644
--- a/src/metabase/driver/redshift.clj
+++ b/src/metabase/driver/redshift.clj
@@ -1,6 +1,7 @@
(ns metabase.driver.redshift
"Amazon Redshift Driver."
(:require [clojure.java.jdbc :as jdbc]
+ [clojure.string :as str]
[honeysql.core :as hsql]
[metabase.config :as config]
[metabase.db.spec :as dbspec]
@@ -59,29 +60,30 @@
(u/strict-extend RedshiftDriver
driver/IDriver
(merge (sql/IDriverSQLDefaultsMixin)
- {:date-interval (u/drop-first-arg date-interval)
- :describe-table-fks (u/drop-first-arg describe-table-fks)
- :details-fields (constantly [{:name "host"
- :display-name "Host"
- :placeholder "my-cluster-name.abcd1234.us-east-1.redshift.amazonaws.com"
- :required true}
- {:name "port"
- :display-name "Port"
- :type :integer
- :default 5439}
- {:name "db"
- :display-name "Database name"
- :placeholder "toucan_sightings"
- :required true}
- {:name "user"
- :display-name "Database username"
- :placeholder "cam"
- :required true}
- {:name "password"
- :display-name "Database user password"
- :type :password
- :placeholder "*******"
- :required true}])})
+ {:date-interval (u/drop-first-arg date-interval)
+ :describe-table-fks (u/drop-first-arg describe-table-fks)
+ :details-fields (constantly [{:name "host"
+ :display-name "Host"
+ :placeholder "my-cluster-name.abcd1234.us-east-1.redshift.amazonaws.com"
+ :required true}
+ {:name "port"
+ :display-name "Port"
+ :type :integer
+ :default 5439}
+ {:name "db"
+ :display-name "Database name"
+ :placeholder "toucan_sightings"
+ :required true}
+ {:name "user"
+ :display-name "Database username"
+ :placeholder "cam"
+ :required true}
+ {:name "password"
+ :display-name "Database user password"
+ :type :password
+ :placeholder "*******"
+ :required true}])
+ :format-custom-field-name (u/drop-first-arg str/lower-case)})
sql/ISQLDriver
(merge postgres/PostgresISQLDriverMixin
diff --git a/src/metabase/models/card.clj b/src/metabase/models/card.clj
index 4fba7f12fe350754121a8760ecde644cc78a38c6..e7ad6046b38375230872aa2e2e156726de37e970 100644
--- a/src/metabase/models/card.clj
+++ b/src/metabase/models/card.clj
@@ -118,7 +118,6 @@
(merge defaults card)
card)))
-
(defn- pre-insert [{:keys [dataset_query], :as card}]
;; TODO - make sure if `collection_id` is specified that we have write permissions for tha tcollection
(u/prog1 card
@@ -129,6 +128,12 @@
(let [database (db/select-one ['Database :id :name], :id (:database dataset_query))]
(qp-perms/throw-if-cannot-run-new-native-query-referencing-db database)))))
+(defn- pre-update [{archived? :archived, :as card}]
+ (u/prog1 card
+ ;; if the Card is archived, then remove it from any Dashboards
+ (when archived?
+ (db/cascade-delete! 'DashboardCard :card_id (u/get-id card)))))
+
(defn- pre-cascade-delete [{:keys [id]}]
(db/cascade-delete! 'PulseCard :card_id id)
(db/cascade-delete! 'Revision :model "Card", :model_id id)
@@ -146,7 +151,7 @@
:timestamped? (constantly true)
:can-read? (partial i/current-user-has-full-permissions? :read)
:can-write? (partial i/current-user-has-full-permissions? :write)
- :pre-update populate-query-fields
+ :pre-update (comp populate-query-fields pre-update)
:pre-insert (comp populate-query-fields pre-insert)
:pre-cascade-delete pre-cascade-delete
:perms-objects-set perms-objects-set})
diff --git a/src/metabase/query_processor/annotate.clj b/src/metabase/query_processor/annotate.clj
index fe6e778d5d501001083edeadd71489478413ff18..1f556bd0061eee3a8cfb47b38c465b3a4c0fca6b 100644
--- a/src/metabase/query_processor/annotate.clj
+++ b/src/metabase/query_processor/annotate.clj
@@ -4,6 +4,7 @@
[clojure.tools.logging :as log]
[medley.core :as m]
[metabase.db :as db]
+ [metabase.driver :as driver]
[metabase.models.field :refer [Field]]
[metabase.query-processor.interface :as i]
[metabase.util :as u])
@@ -85,44 +86,38 @@
{:post [(keyword? (:field-name %))]}
(assoc field :field-name (keyword (str/join \. (rest (i/qualified-name-components field))))))
-(defn- ag-type->field-name
- "Return the (keyword) name that should be used for the column in the results. This is the same as the name of the aggregation,
- except for `distinct`, which is called `:count` for unknown reasons and/or historical accident."
- [ag-type]
- {:pre [(keyword? ag-type)]}
- (if (= ag-type :distinct)
- :count
- ag-type))
-
-;; TODO - rename to something like `aggregation-name` or `aggregation-subclause-name` now that this handles any sort of aggregation
-(defn expression-aggregation-name
- "Return an appropriate name for an `:aggregation` subclause (an aggregation or expression)."
+(defn aggregation-name
+ "Return an appropriate field *and* display name for an `:aggregation` subclause (an aggregation or expression)."
^String [{custom-name :custom-name, aggregation-type :aggregation-type, :as ag}]
+ (when-not i/*driver*
+ (throw (Exception. "metabase.query-processor.interface/*driver* is unbound.")))
(cond
;; if a custom name was provided use it
- custom-name custom-name
+ custom-name (driver/format-custom-field-name i/*driver* custom-name)
;; for unnamed expressions, just compute a name like "sum + count"
(instance? Expression ag) (let [{:keys [operator args]} ag]
(str/join (str " " (name operator) " ")
(for [arg args]
(if (instance? Expression arg)
- (str "(" (expression-aggregation-name arg) ")")
- (expression-aggregation-name arg)))))
+ (str "(" (aggregation-name arg) ")")
+ (aggregation-name arg)))))
;; for unnamed normal aggregations, the column alias is always the same as the ag type except for `:distinct` with is called `:count` (WHY?)
aggregation-type (if (= (keyword aggregation-type) :distinct)
"count"
(name aggregation-type))))
(defn- expression-aggregate-field-info [expression]
- (let [ag-name (expression-aggregation-name expression)]
+ (let [ag-name (aggregation-name expression)]
{:source :aggregation
:field-name ag-name
:field-display-name ag-name
:base-type :type/Number
:special-type :type/Number}))
-(defn- aggregate-field-info [{ag-type :aggregation-type, ag-field :field}]
- (merge (let [field-name (ag-type->field-name ag-type)]
+(defn- aggregate-field-info
+ "Return appropriate column metadata for an `:aggregation` clause."
+ [{ag-type :aggregation-type, ag-field :field, :as ag}]
+ (merge (let [field-name (aggregation-name ag)]
{:source :aggregation
:field-name field-name
:field-display-name field-name
@@ -139,23 +134,57 @@
{:base-type :type/Float
:special-type :type/Number})))
-(defn- add-aggregate-field-if-needed
- "Add a Field containing information about an aggregate column such as `:count` or `:distinct` if needed."
- [{aggregations :aggregation} fields]
- (if (or (empty? aggregations)
- (= (:aggregation-type (first aggregations)) :rows))
+(defn- has-aggregation?
+ "Does QUERY have an aggregation?"
+ [{aggregations :aggregation}]
+ (or (empty? aggregations)
+ ;; TODO - Not sure this needs to be checked anymore since `:rows` is a legacy way to specifiy "no aggregations" and should be stripped out during preprocessing
+ (= (:aggregation-type (first aggregations)) :rows)))
+
+(defn- add-aggregate-fields-if-needed
+ "Add a Field containing information about an aggregate columns such as `:count` or `:distinct` if needed."
+ [{aggregations :aggregation, :as query} fields]
+ (if (has-aggregation? query)
fields
(concat fields (for [ag aggregations]
(if (instance? Expression ag)
(expression-aggregate-field-info ag)
(aggregate-field-info ag))))))
+
+(defn- generic-info-for-missing-key
+ "Return a set of bare-bones metadata for a Field named K when all else fails."
+ [k]
+ {:base-type :type/*
+ :preview-display true
+ :special-type nil
+ :field-name k
+ :field-display-name k})
+
+(defn- info-for-duplicate-field
+ "The Clojure JDBC driver automatically appends suffixes like `count_2` to duplicate columns if multiple columns come back with the same name;
+ since at this time we can't resolve those normally (#1786) fall back to using the metadata for the first column (e.g., `count`).
+ This is definitely a HACK, but in most cases this should be correct (or at least better than the generic info) for the important things like type information."
+ [fields k]
+ (when-let [[_ field-name-without-suffix] (re-matches #"^(.*)_\d+$" (name k))]
+ (some (fn [{field-name :field-name, :as field}]
+ (when (= (name field-name) field-name-without-suffix)
+ (merge (generic-info-for-missing-key k)
+ (select-keys field [:base-type :special-type :source]))))
+ fields)))
+
+(defn- info-for-missing-key
+ "Metadata for a field named K, which we weren't able to resolve normally.
+ If possible, we work around This defaults to generic information "
+ [fields k]
+ (or (info-for-duplicate-field fields k)
+ (generic-info-for-missing-key k)))
+
(defn- add-unknown-fields-if-needed
"When create info maps for any fields we didn't expect to come back from the query.
Ideally, this should never happen, but on the off chance it does we still want to return it in the results."
[actual-keys fields]
- {:pre [(set? actual-keys)
- (every? keyword? actual-keys)]}
+ {:pre [(set? actual-keys) (every? keyword? actual-keys)]}
(let [expected-keys (u/prog1 (set (map :field-name fields))
(assert (every? keyword? <>)))
missing-keys (set/difference actual-keys expected-keys)]
@@ -163,14 +192,10 @@
(log/warn (u/format-color 'yellow "There are fields we weren't expecting in the results: %s\nExpected: %s\nActual: %s"
missing-keys expected-keys actual-keys)))
(concat fields (for [k missing-keys]
- {:base-type :type/*
- :preview-display true
- :special-type nil
- :field-name k
- :field-display-name k}))))
+ (info-for-missing-key fields k)))))
-;;; ## Field Sorting
+;;; ## Field Sorting (TODO - Maybe move this into a separate namespace (`metabase.query-processor.sort`?)
;; We sort Fields with a "importance" vector like [source-importance position special-type-importance name]
@@ -282,7 +307,7 @@
(when (seq result-keys)
(->> (collect-fields (dissoc query :expressions))
(map qualify-field-name)
- (add-aggregate-field-if-needed query)
+ (add-aggregate-fields-if-needed query)
(map (u/rpartial update :field-name keyword))
(add-unknown-fields-if-needed result-keys)
(sort-fields query)
diff --git a/test/metabase/api/metric_test.clj b/test/metabase/api/metric_test.clj
index 48c69599d8262586dd2e733f12a73f2eaaa37a0b..1ec3c9700c1a5ac91574333405795d7b8198fcdd 100644
--- a/test/metabase/api/metric_test.clj
+++ b/test/metabase/api/metric_test.clj
@@ -368,8 +368,8 @@
;;; GET /api/metric/
-(tu/expect-with-temp [Metric [metric-1]
- Metric [metric-2]
+(tu/expect-with-temp [Metric [metric-1 {:name "Metric A"}]
+ Metric [metric-2 {:name "Metric B"}]
Metric [_ {:is_active false}]] ; inactive metrics shouldn't show up
(tu/mappify (hydrate [metric-1
metric-2] :creator))
diff --git a/test/metabase/db/metadata_queries_test.clj b/test/metabase/db/metadata_queries_test.clj
index e1ff690ef9b5373039e0f724f049913d1d3ccc16..763812f0038e08f0e35f10b0d8998760203f3dd4 100644
--- a/test/metabase/db/metadata_queries_test.clj
+++ b/test/metabase/db/metadata_queries_test.clj
@@ -1,6 +1,5 @@
(ns metabase.db.metadata-queries-test
- (:require [clojure.set :as set]
- [expectations :refer :all]
+ (:require [expectations :refer :all]
[metabase.db :as db]
[metabase.db.metadata-queries :refer :all]
(metabase.models [field :refer [Field]]
@@ -12,7 +11,7 @@
;; Redshift & Crate tests are randomly failing -- see https://github.com/metabase/metabase/issues/2767
(def ^:private ^:const metadata-queries-test-engines
- (set/difference qp-test/non-timeseries-engines #{:redshift :crate}))
+ (disj qp-test/non-timeseries-engines :redshift :crate))
;; ### FIELD-DISTINCT-COUNT
(datasets/expect-with-engines metadata-queries-test-engines
diff --git a/test/metabase/models/card_test.clj b/test/metabase/models/card_test.clj
index 8f08d7e0e111920053eab332c72e23d0a29b4179..25ceaccc32baa97655b8043e46d63291bdef5b23 100644
--- a/test/metabase/models/card_test.clj
+++ b/test/metabase/models/card_test.clj
@@ -3,13 +3,15 @@
[metabase.api.common :refer [*current-user-permissions-set* *is-superuser?*]]
[metabase.db :as db]
(metabase.models [card :refer :all]
+ [dashboard :refer [Dashboard]]
[dashboard-card :refer [DashboardCard]]
[interface :as models]
[permissions :as perms])
[metabase.query-processor.expand :as ql]
[metabase.test.data :refer [id]]
[metabase.test.data.users :refer :all]
- [metabase.test.util :refer [random-name with-temp]]))
+ [metabase.test.util :refer [random-name with-temp], :as tu]
+ [metabase.util :as u]))
(defn- create-dash! [dash-name]
@@ -121,3 +123,13 @@
#{"/db/0/"}
(query-perms-set (mbql {:filter [:WOW 100 200]})
:read))
+
+
+;; Test that when somebody archives a Card, it is removed from any Dashboards it belongs to
+(expect
+ 0
+ (tu/with-temp* [Dashboard [dashboard]
+ Card [card]
+ DashboardCard [dashcard {:dashboard_id (u/get-id dashboard), :card_id (u/get-id card)}]]
+ (db/update! Card (u/get-id card) :archived true)
+ (db/select-one-count DashboardCard :dashboard_id (u/get-id dashboard))))
diff --git a/test/metabase/models/metric_test.clj b/test/metabase/models/metric_test.clj
index e040ae06ec444b0a1bdae31cd060196833be7204..0856bfc1eb0938ecb4a66d8feb2b63446f27687d 100644
--- a/test/metabase/models/metric_test.clj
+++ b/test/metabase/models/metric_test.clj
@@ -40,7 +40,7 @@
:is_active true
:definition {:clause ["a" "b"]}}
(tu/with-temp* [Database [{database-id :id}]
- Table [{:keys [id]} {:db_id database-id}]]
+ Table [{:keys [id]} {:db_id database-id}]]
(create-metric-then-select! id "I only want *these* things" nil (user->id :rasta) {:clause ["a" "b"]})))
@@ -49,12 +49,12 @@
[true
false]
(tu/with-temp* [Database [{database-id :id}]
- Table [{table-id :id} {:db_id database-id}]
- Metric [{metric-id :id} {:table_id table-id
- :definition {:database 45
- :query {:filter ["yay"]}}}]]
+ Table [{table-id :id} {:db_id database-id}]
+ Metric [{metric-id :id} {:table_id table-id
+ :definition {:database 45
+ :query {:filter ["yay"]}}}]]
[(metric/exists? metric-id)
- (metric/exists? 34)]))
+ (metric/exists? Integer/MAX_VALUE)])) ; a Metric that definitely doesn't exist
;; retrieve-metric
@@ -71,10 +71,10 @@
:definition {:database 45
:query {:filter ["yay"]}}}
(tu/with-temp* [Database [{database-id :id}]
- Table [{table-id :id} {:db_id database-id}]
- Metric [{metric-id :id} {:table_id table-id
- :definition {:database 45
- :query {:filter ["yay"]}}}]]
+ Table [{table-id :id} {:db_id database-id}]
+ Metric [{metric-id :id} {:table_id table-id
+ :definition {:database 45
+ :query {:filter ["yay"]}}}]]
(let [{:keys [creator] :as metric} (retrieve-metric metric-id)]
(update (dissoc metric :id :table_id :created_at :updated_at)
:creator (u/rpartial dissoc :date_joined :last_login)))))
diff --git a/test/metabase/query_processor/parameters_test.clj b/test/metabase/query_processor/parameters_test.clj
index bc22aaa50c0c76918539273e944267d1b1b1273d..f5262498246a10d42c023f4ea490c976b60d5de0 100644
--- a/test/metabase/query_processor/parameters_test.clj
+++ b/test/metabase/query_processor/parameters_test.clj
@@ -1,7 +1,6 @@
(ns metabase.query-processor.parameters-test
"Tests for *MBQL* parameter substitution."
- (:require [clojure.set :as set]
- (clj-time [core :as t]
+ (:require (clj-time [core :as t]
[format :as tf])
[expectations :refer :all]
[metabase.driver :as driver]
@@ -134,7 +133,7 @@
;;; +-------------------------------------------------------------------------------------------------------+
;; for some reason param substitution tests fail on Redshift & (occasionally) Crate so just don't run those for now
-(def ^:private ^:const params-test-engines (set/difference non-timeseries-engines #{:redshift :crate}))
+(def ^:private ^:const params-test-engines (disj non-timeseries-engines :redshift :crate))
;; check that date ranges work correctly
(datasets/expect-with-engines params-test-engines
diff --git a/test/metabase/query_processor/sql_parameters_test.clj b/test/metabase/query_processor/sql_parameters_test.clj
index 56c7ecf8c5b95e614743df6e5d3d57b7159b8f60..b09527d2211b0c14a02754c8d9180c18e9b33607 100644
--- a/test/metabase/query_processor/sql_parameters_test.clj
+++ b/test/metabase/query_processor/sql_parameters_test.clj
@@ -1,6 +1,5 @@
(ns metabase.query-processor.sql-parameters-test
- (:require [clojure.set :as set]
- [clj-time.core :as t]
+ (:require [clj-time.core :as t]
[expectations :refer :all]
(metabase [db :as db]
[driver :as driver])
@@ -362,7 +361,7 @@
;; as with the MBQL parameters tests Redshift and Crate fail for unknown reasons; disable their tests for now
(def ^:private ^:const sql-parameters-engines
- (set/difference (engines-that-support :native-parameters) #{:redshift :crate}))
+ (disj (engines-that-support :native-parameters) :redshift :crate))
(defn- process-native {:style/indent 0} [& kvs]
(qp/process-query
diff --git a/test/metabase/query_processor_test/aggregation_test.clj b/test/metabase/query_processor_test/aggregation_test.clj
index 174ed64dd1e87a250307e13d00c5780610a09050..759d5e26e57a62c485d473a0a5534ccaf22ad3fd 100644
--- a/test/metabase/query_processor_test/aggregation_test.clj
+++ b/test/metabase/query_processor_test/aggregation_test.clj
@@ -148,6 +148,18 @@
(rows (data/run-query venues
(ql/aggregation (ql/avg $price) (ql/count) (ql/sum $price))))))
+;; make sure that multiple aggregations of the same type have the correct metadata (#4003)
+;; (TODO - this isn't tested against Mongo or BigQuery because those drivers don't currently work correctly with multiple columns with the same name)
+(datasets/expect-with-engines (disj non-timeseries-engines :mongo :bigquery)
+ [(aggregate-col :count)
+ (assoc (aggregate-col :count)
+ :display_name "count_2"
+ :name "count_2"
+ :preview_display true)]
+ (-> (data/run-query venues
+ (ql/aggregation (ql/count) (ql/count)))
+ :data :cols))
+
;;; ------------------------------------------------------------ CUMULATIVE SUM ------------------------------------------------------------
diff --git a/test/metabase/query_processor_test/expression_aggregations_test.clj b/test/metabase/query_processor_test/expression_aggregations_test.clj
index 66a30e1a2721db11a17faedb286c062f3c217c2c..2de64107ed2cd97cf8c12b3cb7ec16cc59bcad33 100644
--- a/test/metabase/query_processor_test/expression_aggregations_test.clj
+++ b/test/metabase/query_processor_test/expression_aggregations_test.clj
@@ -1,12 +1,13 @@
(ns metabase.query-processor-test.expression-aggregations-test
- "Tests for expression aggregations."
+ "Tests for expression aggregations and for named aggregations."
(:require [expectations :refer :all]
+ [metabase.driver :as driver]
[metabase.models.metric :refer [Metric]]
[metabase.query-processor :as qp]
[metabase.query-processor.expand :as ql]
[metabase.query-processor-test :refer :all]
[metabase.test.data :as data]
- [metabase.test.data.datasets :as datasets, :refer [*engine*]]
+ [metabase.test.data.datasets :as datasets, :refer [*engine* *driver*]]
[metabase.test.util :as tu]
[metabase.util :as u]))
@@ -167,7 +168,7 @@
[3 52]
[4 30]]
:columns [(data/format-name "price")
- (if (= *engine* :redshift) "new price" "New Price")]} ; Redshift annoyingly always lowercases column aliases
+ (driver/format-custom-field-name *driver* "New Price")]} ; Redshift annoyingly always lowercases column aliases
(format-rows-by [int int]
(rows+column-names (data/run-query venues
(ql/aggregation (ql/named (ql/sum (ql/+ $price 1)) "New Price"))
@@ -180,7 +181,7 @@
[3 -2]
[4 -17]]
:columns [(data/format-name "price")
- (if (= *engine* :redshift) "sum-41" "Sum-41")]}
+ (driver/format-custom-field-name *driver* "Sum-41")]}
(format-rows-by [int int]
(rows+column-names (data/run-query venues
(ql/aggregation (ql/named (ql/- (ql/sum $price) 41) "Sum-41"))
@@ -208,7 +209,7 @@
[3 39]
[4 24]]
:columns [(data/format-name "price")
- (if (= *engine* :redshift) "my cool metric" "My Cool Metric")]}
+ (driver/format-custom-field-name *driver* "My Cool Metric")]}
(tu/with-temp Metric [metric {:table_id (data/id :venues)
:definition {:aggregation [:sum [:field-id (data/id :venues :price)]]
:filter [:> [:field-id (data/id :venues :price)] 1]}}]
@@ -226,7 +227,7 @@
[3 39]
[4 24]]
:columns [(data/format-name "price")
- (if (= *engine* :redshift) "my cool metric" "My Cool Metric")]}
+ (driver/format-custom-field-name *driver* "My Cool Metric")]}
(tu/with-temp Metric [metric {:table_id (data/id :venues)
:definition {:aggregation [[:sum [:field-id (data/id :venues :price)]]]
:filter [:> [:field-id (data/id :venues :price)] 1]}}]
@@ -237,3 +238,16 @@
:query {:source-table (data/id :venues)
:aggregation [[:named ["METRIC" (u/get-id metric)] "My Cool Metric"]]
:breakout [(ql/breakout (ql/field-id (data/id :venues :price)))]}})))))
+
+;; check that named aggregations come back with the correct column metadata (#4002)
+(datasets/expect-with-engines (engines-that-support :expression-aggregations)
+ (let [col-name (driver/format-custom-field-name *driver* "Count of Things")]
+ (assoc (aggregate-col :count)
+ :name col-name
+ :display_name col-name))
+ (-> (qp/process-query
+ {:database (data/id)
+ :type :query
+ :query {:source-table (data/id :venues)
+ :aggregation [[:named ["COUNT"] "Count of Things"]]}})
+ :data :cols first))