Misc cleanup and tests

frontend/src/metabase/dashboard/components/DashCard.jsx

@@ -135,6 +135,7 @@ export default class DashCard extends Component {
                     replacementContent={isEditingParameter && <DashCardParameterMapper dashcard={dashcard} />}
                     metadata={metadata}
                     onChangeCardAndRun={(card) => {
+                        // ensure the original card ID is included to track lineage
                         card = { ...card, id: series[0].card.id };
                         onChangeCardAndRun(card, dashcard);
                     }}

frontend/src/metabase/dashboard/dashboard.js

@@ -496,6 +496,10 @@ export const deletePublicLink = createAction(DELETE_PUBLIC_LINK, async ({ id })
 
 import { push } from "react-router-redux";
 
+/** All navigation actions from dashboards to cards (e.x. clicking a title, drill through)
+ *  should go through this action, which merges any currently applied dashboard filters
+ *  into the new card / URL parmeters.
+ */
 const CHANGE_CARD_AND_RUN = "metabase/database/CHANGE_CARD_AND_RUN";
 export const onChangeCardAndRun = createThunkAction(CHANGE_CARD_AND_RUN, (card, dashcard = null) =>
     (dispatch, getState) => {

frontend/src/metabase/meta/Card.js

@@ -143,7 +143,7 @@ export function applyParameters(
     return datasetQuery;
 }
 
-
+/** returns a question URL with parameters added to query string or MBQL filters */
 export function questionUrlWithParameters(
     card: Card,
     parameters: Parameter[],
@@ -152,12 +152,18 @@ export function questionUrlWithParameters(
 ): DatasetQuery {
     card = Utils.copy(card);
 
-    const datasetQuery = applyParameters(card, parameters, parameterValues, parameterMappings);
     const cardParameters = getParameters(card);
+    const datasetQuery = applyParameters(
+        card,
+        parameters,
+        parameterValues,
+        parameterMappings
+    );
 
     const query = {};
     for (const datasetParameter of datasetQuery.parameters) {
-        const cardParameter = _.find(cardParameters, p => Utils.equals(p.target, datasetParameter.target));
+        const cardParameter = _.find(cardParameters, p =>
+            Utils.equals(p.target, datasetParameter.target));
         if (cardParameter) {
             // if the card has a real parameter we can use, use that
             query[cardParameter.slug] = datasetParameter.value;
@@ -166,14 +172,13 @@ export function questionUrlWithParameters(
             const filter = parameterToMBQLFilter(datasetParameter);
             if (filter) {
                 card = updateIn(card, ["dataset_query", "query"], query =>
-                    Query.addFilter(query, filter)
-                );
+                    Query.addFilter(query, filter));
             } else {
-                console.warn("UNHANDLED PARAMETER", datasetParameter)
+                console.warn("UNHANDLED PARAMETER", datasetParameter);
             }
         } else {
-            console.warn("UNHANDLED PARAMETER", datasetParameter)
+            console.warn("UNHANDLED PARAMETER", datasetParameter);
         }
     }
-    return Urls.question(card.id, card.dataset_query ? card : undefined, query)
+    return Urls.question(card.id, card.dataset_query ? card : undefined, query);
 }

frontend/src/metabase/meta/Card.spec.js

+import * as Card from "./Card";
+
+import { assocIn, dissoc } from "icepick";
+
+describe("metabase/meta/Card", () => {
+    describe("questionUrlWithParameters", () => {
+        const parameters = [
+            {
+                id: 1,
+                slug: "param_string",
+                type: "category"
+            },
+            {
+                id: 2,
+                slug: "param_number",
+                type: "category"
+            },
+            {
+                id: 3,
+                slug: "param_date",
+                type: "date/month"
+            }
+        ];
+
+        describe("with SQL card", () => {
+            const card = {
+                id: 1,
+                dataset_query: {
+                    type: "native",
+                    native: {
+                        template_tags: {
+                            baz: { name: "baz", type: "text" }
+                        }
+                    }
+                }
+            };
+            const parameterMappings = [
+                {
+                    card_id: 1,
+                    parameter_id: 1,
+                    target: ["variable", ["template-tag", "baz"]]
+                }
+            ];
+            it("should return question URL with no parameters", () => {
+                const url = Card.questionUrlWithParameters(card, []);
+                expect(parseUrl(url)).toEqual({
+                    pathname: "/question/1",
+                    query: {},
+                    card: dissoc(card, "id")
+                });
+            });
+            it("should return question URL with query string parameter", () => {
+                const url = Card.questionUrlWithParameters(
+                    card,
+                    parameters,
+                    { "1": "bar" },
+                    parameterMappings
+                );
+                expect(parseUrl(url)).toEqual({
+                    pathname: "/question/1",
+                    query: { baz: "bar" },
+                    card: dissoc(card, "id")
+                });
+            });
+        });
+        describe("with structured card", () => {
+            const card = {
+                id: 1,
+                dataset_query: {
+                    type: "query",
+                    query: {
+                        source_table: 1
+                    }
+                }
+            };
+            const parameterMappings = [
+                {
+                    card_id: 1,
+                    parameter_id: 1,
+                    target: ["dimension", ["field-id", 1]]
+                },
+                {
+                    card_id: 1,
+                    parameter_id: 2,
+                    target: ["dimension", ["field-id", 2]]
+                },
+                {
+                    card_id: 1,
+                    parameter_id: 3,
+                    target: ["dimension", ["field-id", 3]]
+                }
+            ];
+            it("should return question URL with no parameters", () => {
+                const url = Card.questionUrlWithParameters(card, []);
+                expect(parseUrl(url)).toEqual({
+                    pathname: "/question/1",
+                    query: {},
+                    card: dissoc(card, "id")
+                });
+            });
+            it("should return question URL with string MBQL filter added", () => {
+                const url = Card.questionUrlWithParameters(
+                    card,
+                    parameters,
+                    { "1": "bar" },
+                    parameterMappings
+                );
+                expect(parseUrl(url)).toEqual({
+                    pathname: "/question/1",
+                    query: {},
+                    card: assocIn(
+                        dissoc(card, "id"),
+                        ["dataset_query", "query", "filter"],
+                        ["AND", ["=", 1, "bar"]]
+                    )
+                });
+            });
+            it("should return question URL with number MBQL filter added", () => {
+                const url = Card.questionUrlWithParameters(
+                    card,
+                    parameters,
+                    { "2": "123" },
+                    parameterMappings
+                );
+                expect(parseUrl(url)).toEqual({
+                    pathname: "/question/1",
+                    query: {},
+                    card: assocIn(
+                        dissoc(card, "id"),
+                        ["dataset_query", "query", "filter"],
+                        ["AND", ["=", 1, 123]]
+                    )
+                });
+            });
+            it("should return question URL with date MBQL filter added", () => {
+                const url = Card.questionUrlWithParameters(
+                    card,
+                    parameters,
+                    { "3": "2017-05" },
+                    parameterMappings
+                );
+                expect(parseUrl(url)).toEqual({
+                    pathname: "/question/1",
+                    query: {},
+                    card: assocIn(
+                        dissoc(card, "id"),
+                        ["dataset_query", "query", "filter"],
+                        ["AND", ["=", ["datetime-field", 1, "month"], "FIXME: what should this be?"]]
+                    )
+                });
+            });
+        });
+    });
+});
+
+import { parse } from "url";
+import { deserializeCardFromUrl } from "metabase/lib/card";
+
+function parseUrl(url) {
+    const parsed = parse(url, true);
+    return {
+        card: parsed.hash && deserializeCardFromUrl(parsed.hash),
+        query: parsed.query,
+        pathname: parsed.pathname
+    };
+}

frontend/src/metabase/meta/types/Query.js

@@ -21,12 +21,13 @@ export type DatetimeUnit = "default" | "minute" | "minute-of-hour" | "hour" | "h
 
 export type TemplateTagId = string;
 export type TemplateTagName = string;
+export type TemplateTagType = "text" | "number" | "date" | "dimension";
 
 export type TemplateTag = {
     id:           TemplateTagId,
     name:         TemplateTagName,
     display_name: string,
-    type:         string,
+    type:         TemplateTagType,
     dimension?:   LocalFieldReference,
     widget_type?: ParameterType,
     required?:    boolean,