diff --git a/frontend/src/metabase/dashboard/components/DashCard.jsx b/frontend/src/metabase/dashboard/components/DashCard.jsx
index b24fd3a65369c5882233f8ab2d57214271d541df..0e9a9aca987eb11959df519613b268a2edc6ca85 100644
--- a/frontend/src/metabase/dashboard/components/DashCard.jsx
+++ b/frontend/src/metabase/dashboard/components/DashCard.jsx
@@ -34,6 +34,7 @@ export default class DashCard extends Component {
parameterValues: PropTypes.object.isRequired,
markNewCardSeen: PropTypes.func.isRequired,
fetchCardData: PropTypes.func.isRequired,
+ navigateToNewCardFromDashboard: PropTypes.func.isRequired
};
async componentDidMount() {
@@ -60,7 +61,7 @@ export default class DashCard extends Component {
isEditingParameter,
onAddSeries,
onRemove,
- navigateToNewCard,
+ navigateToNewCardFromDashboard,
metadata
} = this.props;
@@ -134,8 +135,9 @@ export default class DashCard extends Component {
onUpdateVisualizationSettings={this.props.onUpdateVisualizationSettings}
replacementContent={isEditingParameter && <DashCardParameterMapper dashcard={dashcard} />}
metadata={metadata}
- onChangeCardAndRun={ navigateToNewCard ? (card: UnsavedCard) => {
- navigateToNewCard(card, dashcard)
+ onChangeCardAndRun={ navigateToNewCardFromDashboard ? ({ nextCard, previousCard }) => {
+ // navigateToNewCardFromDashboard needs `dashcard` for applying active filters to the query
+ navigateToNewCardFromDashboard({ nextCard, previousCard, dashcard })
} : null}
/>
</div>
diff --git a/frontend/src/metabase/dashboard/components/DashboardGrid.jsx b/frontend/src/metabase/dashboard/components/DashboardGrid.jsx
index b1ab2839a3bd0217bec0f776911e6a6e99de07be..ce36abfd94c59470b0421a9600e69f82b3313f37 100644
--- a/frontend/src/metabase/dashboard/components/DashboardGrid.jsx
+++ b/frontend/src/metabase/dashboard/components/DashboardGrid.jsx
@@ -193,7 +193,7 @@ export default class DashboardGrid extends Component {
onAddSeries={this.onDashCardAddSeries.bind(this, dc)}
onUpdateVisualizationSettings={this.props.onUpdateDashCardVisualizationSettings.bind(this, dc.id)}
onReplaceAllVisualizationSettings={this.props.onReplaceAllDashCardVisualizationSettings.bind(this, dc.id)}
- navigateToNewCard={this.props.navigateToNewCard}
+ navigateToNewCardFromDashboard={this.props.navigateToNewCardFromDashboard}
metadata={this.props.metadata}
/>
)
diff --git a/frontend/src/metabase/dashboard/dashboard.js b/frontend/src/metabase/dashboard/dashboard.js
index 2bff4047b2ccefd3e7a7b1d656efa228f65bacc1..24aebceb82f09b61cc71f0925de46df7259400ad 100644
--- a/frontend/src/metabase/dashboard/dashboard.js
+++ b/frontend/src/metabase/dashboard/dashboard.js
@@ -14,7 +14,7 @@ import { applyParameters, questionUrlWithParameters } from "metabase/meta/Card";
import { getParametersBySlug } from "metabase/meta/Parameter";
import type { DashboardWithCards, DashCard, DashCardId } from "metabase/meta/types/Dashboard";
-import type { UnsavedCard, Card, CardId } from "metabase/meta/types/Card";
+import type { Card, CardId } from "metabase/meta/types/Card";
import Utils from "metabase/lib/utils";
import { getPositionForNewDashCard } from "metabase/lib/dashboard_grid";
@@ -25,6 +25,7 @@ import { push } from "react-router-redux";
import { DashboardApi, CardApi, RevisionApi, PublicApi, EmbedApi } from "metabase/services";
import { getDashboard, getDashboardComplete } from "./selectors";
+import {getCardAfterVisualizationClick} from "metabase/visualizations/lib/utils";
const DATASET_SLOW_TIMEOUT = 15 * 1000;
@@ -491,24 +492,43 @@ export const deletePublicLink = createAction(DELETE_PUBLIC_LINK, async ({ id })
return { id };
});
-/** 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 parameters.
+/**
+ * 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 parameters.
+ *
+ * User-triggered events that are handled here:
+ * - clicking a dashcard legend:
+ * * question title legend (only for single-question cards)
+ * * series legend (multi-aggregation, multi-breakout, multiple questions)
+ * - clicking the visualization inside dashcard
+ * * drill-through (single series, multi-aggregation, multi-breakout, multiple questions)
+ * * (not in 0.24.2 yet: drag on line/area/bar visualization)
+ * - those all can be applied without or with a dashboard filter
*/
-// TODO Atte Keinänen 5/2/17: This could be combined with `setCardAndRun` of query_builder/actions.js
-// Having two separate actions for very similar behavior was a source of initial confusion for me
const NAVIGATE_TO_NEW_CARD = "metabase/dashboard/NAVIGATE_TO_NEW_CARD";
-export const navigateToNewCard = createThunkAction(NAVIGATE_TO_NEW_CARD, (card: UnsavedCard, dashcard: DashCard) =>
- (dispatch, getState) => {
- const { metadata } = getState();
- const { dashboardId, dashboards, parameterValues } = getState().dashboard;
- const dashboard = dashboards[dashboardId];
+export const navigateToNewCardFromDashboard = createThunkAction(
+ NAVIGATE_TO_NEW_CARD,
+ ({ nextCard, previousCard, dashcard }) =>
+ (dispatch, getState) => {
+ const {metadata} = getState();
+ const {dashboardId, dashboards, parameterValues} = getState().dashboard;
+ const dashboard = dashboards[dashboardId];
+ const cardIsDirty = !_.isEqual(previousCard.dataset_query, nextCard.dataset_query);
+
+ const url = questionUrlWithParameters(
+ getCardAfterVisualizationClick(nextCard, previousCard),
+ metadata,
+ dashboard.parameters,
+ parameterValues,
+ dashcard && dashcard.parameter_mappings,
+ cardIsDirty
+ );
- // $FlowFixMe
- const url = questionUrlWithParameters(card, metadata, dashboard.parameters, parameterValues, dashcard && dashcard.parameter_mappings);
- dispatch(push(url));
- });
+ dispatch(push(url));
+ }
+);
// reducers
diff --git a/frontend/src/metabase/meta/Card.js b/frontend/src/metabase/meta/Card.js
index ca2bc5d82cb5a536716ab632b543452689bbdb03..6db71543c4a7ca406d39036225f722aa2c660dde 100644
--- a/frontend/src/metabase/meta/Card.js
+++ b/frontend/src/metabase/meta/Card.js
@@ -131,7 +131,11 @@ export function applyParameters(
continue;
}
- const mapping = _.findWhere(parameterMappings, { card_id: card.id, parameter_id: parameter.id });
+ const mapping = _.findWhere(parameterMappings, {
+ // $FlowFixMe original_card_id not included in the flow type of card
+ card_id: card.id || card.original_card_id,
+ parameter_id: parameter.id
+ });
if (mapping) {
// mapped target, e.x. on a dashboard
datasetQuery.parameters.push({
@@ -158,7 +162,8 @@ export function questionUrlWithParameters(
metadata: Metadata,
parameters: Parameter[],
parameterValues: ParameterValues = {},
- parameterMappings: ParameterMapping[] = []
+ parameterMappings: ParameterMapping[] = [],
+ cardIsDirty: boolean = true
): DatasetQuery {
if (!card.dataset_query) {
return Urls.question(card.id);
@@ -174,6 +179,11 @@ export function questionUrlWithParameters(
parameterMappings
);
+ // If we have a clean question without parameters applied, don't add the dataset query hash
+ if (!cardIsDirty && datasetQuery.parameters && datasetQuery.parameters.length === 0) {
+ return Urls.question(card.id);
+ }
+
const query = {};
for (const datasetParameter of datasetQuery.parameters || []) {
const cardParameter = _.find(cardParameters, p =>
diff --git a/frontend/src/metabase/meta/Card.spec.js b/frontend/src/metabase/meta/Card.spec.js
index 8830d733ab259bb4bdc9441ec8ee4124670040ed..f94eef948e3822f26ec664669b05071d275f8e4f 100644
--- a/frontend/src/metabase/meta/Card.spec.js
+++ b/frontend/src/metabase/meta/Card.spec.js
@@ -135,6 +135,26 @@ describe("metabase/meta/Card", () => {
)
});
});
+ it("should return question URL even if only original_card_id is present", () => {
+ const cardWithOnlyOriginalCardId = { ...card, id: undefined, original_card_id: card.id };
+
+ const url = Card.questionUrlWithParameters(
+ cardWithOnlyOriginalCardId,
+ metadata,
+ parameters,
+ { "1": "bar" },
+ parameterMappings
+ );
+ expect(parseUrl(url)).toEqual({
+ pathname: "/question",
+ query: {},
+ card: assocIn(
+ cardWithOnlyOriginalCardId,
+ ["dataset_query", "query", "filter"],
+ ["AND", ["=", ["field-id", 1], "bar"]]
+ )
+ });
+ });
it("should return question URL with number MBQL filter added", () => {
const url = Card.questionUrlWithParameters(
card,
diff --git a/frontend/src/metabase/meta/types/Visualization.js b/frontend/src/metabase/meta/types/Visualization.js
index e172b2037c269d320e2fa536e560514346cb0744..bc5b2132e500b6e8e182f167d87913e4e5317d51 100644
--- a/frontend/src/metabase/meta/types/Visualization.js
+++ b/frontend/src/metabase/meta/types/Visualization.js
@@ -53,7 +53,7 @@ export type ClickActionProps = {
}
export type ClickActionPopoverProps = {
- onChangeCardAndRun: (card: ?Card) => void,
+ onChangeCardAndRun: (Object) => void,
onClose: () => void,
}
@@ -83,7 +83,7 @@ export type VisualizationProps = {
onHoverChange: (?HoverObject) => void,
onVisualizationClick: (?ClickObject) => void,
visualizationIsClickable: (?ClickObject) => boolean,
- onChangeCardAndRun: (card: Card) => void,
+ onChangeCardAndRun: (Object) => void,
onUpdateVisualizationSettings: ({ [key: string]: any }) => void
}
diff --git a/frontend/src/metabase/qb/components/actions/PivotByAction.jsx b/frontend/src/metabase/qb/components/actions/PivotByAction.jsx
index a26f75a88c74bcb73115d693338025dbe63f935f..c52cddaddc5e101b9b02e9c1859d496d4812c5e7 100644
--- a/frontend/src/metabase/qb/components/actions/PivotByAction.jsx
+++ b/frontend/src/metabase/qb/components/actions/PivotByAction.jsx
@@ -88,9 +88,14 @@ export default (name: string, icon: string, fieldFilter: FieldFilter) =>
fieldOptions={fieldOptions}
customFieldOptions={customFieldOptions}
onCommitBreakout={breakout => {
- onChangeCardAndRun(
- pivot(card, breakout, tableMetadata, dimensions)
- );
+ onChangeCardAndRun({
+ nextCard: pivot(
+ card,
+ breakout,
+ tableMetadata,
+ dimensions
+ )
+ });
}}
onClose={onClose}
/>
diff --git a/frontend/src/metabase/qb/components/actions/SummarizeBySegmentMetricAction.jsx b/frontend/src/metabase/qb/components/actions/SummarizeBySegmentMetricAction.jsx
index 851400885e1aa61b750b3a204911d6c1c8afa231..94242389f2afeee09121e3b649f4823841bf8c33 100644
--- a/frontend/src/metabase/qb/components/actions/SummarizeBySegmentMetricAction.jsx
+++ b/frontend/src/metabase/qb/components/actions/SummarizeBySegmentMetricAction.jsx
@@ -34,9 +34,13 @@ export default ({ card, tableMetadata }: ClickActionProps): ClickAction[] => {
customFields={Query.getExpressions(query)}
availableAggregations={tableMetadata.aggregation_options}
onCommitAggregation={aggregation => {
- onChangeCardAndRun(
- summarize(card, aggregation, tableMetadata)
- );
+ onChangeCardAndRun({
+ nextCard: summarize(
+ card,
+ aggregation,
+ tableMetadata
+ )
+ });
onClose && onClose();
}}
/>
diff --git a/frontend/src/metabase/qb/lib/actions.js b/frontend/src/metabase/qb/lib/actions.js
index a5c8ecd6210696f2091ec967f21006af6e179239..e3578fc6d8ac5b2bcab242efd9b3fd350cc7c1a1 100644
--- a/frontend/src/metabase/qb/lib/actions.js
+++ b/frontend/src/metabase/qb/lib/actions.js
@@ -19,6 +19,7 @@ export const toUnderlyingData = (card: CardObject): ?CardObject => {
const newCard = startNewCard("query");
newCard.dataset_query = card.dataset_query;
newCard.display = "table";
+ newCard.original_card_id = card.id;
return newCard;
};
diff --git a/frontend/src/metabase/query_builder/actions.js b/frontend/src/metabase/query_builder/actions.js
index 009fe04415527402dfb205af9cbaf697ecf64fac..b02e64cbb780a4dca19a4294a40c84ebe63f35d2 100644
--- a/frontend/src/metabase/query_builder/actions.js
+++ b/frontend/src/metabase/query_builder/actions.js
@@ -29,6 +29,7 @@ import { MetabaseApi, CardApi, UserApi } from "metabase/services";
import { parse as urlParse } from "url";
import querystring from "querystring";
+import {getCardAfterVisualizationClick} from "metabase/visualizations/lib/utils";
export const SET_CURRENT_STATE = "metabase/qb/SET_CURRENT_STATE"; const setCurrentState = createAction(SET_CURRENT_STATE);
@@ -491,9 +492,12 @@ export const reloadCard = createThunkAction(RELOAD_CARD, () => {
};
});
-// setCardAndRun
-// Used when navigating browser history, when drilling through in visualizations / action widget,
-// and when having the entity details view open and clicking its cells
+/**
+ * `setCardAndRun` is used when:
+ * - navigating browser history
+ * - clicking in the entity details view
+ * - `navigateToNewCardInsideQB` is being called (see below)
+ */
export const SET_CARD_AND_RUN = "metabase/qb/SET_CARD_AND_RUN";
export const setCardAndRun = createThunkAction(SET_CARD_AND_RUN, (nextCard, shouldUpdateUrl = true) => {
return async (dispatch, getState) => {
@@ -518,8 +522,31 @@ export const setCardAndRun = createThunkAction(SET_CARD_AND_RUN, (nextCard, shou
};
});
+/**
+ * User-triggered events that are handled with this action:
+ * - clicking a legend:
+ * * series legend (multi-aggregation, multi-breakout, multiple questions)
+ * - clicking the visualization itself
+ * * drill-through (single series, multi-aggregation, multi-breakout, multiple questions)
+ * * (not in 0.24.2 yet: drag on line/area/bar visualization)
+ * - clicking an action widget action
+ *
+ * All these events can be applied either for an unsaved question or a saved question.
+ */
+export const NAVIGATE_TO_NEW_CARD = "metabase/qb/NAVIGATE_TO_NEW_CARD";
+export const navigateToNewCardInsideQB = createThunkAction(NAVIGATE_TO_NEW_CARD, ({ nextCard, previousCard }) => {
+ return async (dispatch, getState) => {
+ const nextCardIsClean = _.isEqual(previousCard.dataset_query, nextCard.dataset_query) && previousCard.display === nextCard.display;
+
+ if (nextCardIsClean) {
+ // This is mainly a fallback for scenarios where a visualization legend is clicked inside QB
+ dispatch(setCardAndRun(await loadCard(nextCard.id)));
+ } else {
+ dispatch(setCardAndRun(getCardAfterVisualizationClick(nextCard, previousCard)));
+ }
+ }
+});
-// setDatasetQuery
export const SET_DATASET_QUERY = "metabase/qb/SET_DATASET_QUERY";
export const setDatasetQuery = createThunkAction(SET_DATASET_QUERY, (dataset_query, run = false) => {
return (dispatch, getState) => {
diff --git a/frontend/src/metabase/query_builder/components/ActionsWidget.jsx b/frontend/src/metabase/query_builder/components/ActionsWidget.jsx
index 981c66149e17f7b953854e00fd9d54ff83196ce8..eba3825141b79b5e1df080e34d80d6612c170acc 100644
--- a/frontend/src/metabase/query_builder/components/ActionsWidget.jsx
+++ b/frontend/src/metabase/query_builder/components/ActionsWidget.jsx
@@ -12,7 +12,7 @@ import MetabaseAnalytics from "metabase/lib/analytics";
import cx from "classnames";
import _ from "underscore";
-import type { Card, UnsavedCard } from "metabase/meta/types/Card";
+import type { Card, UnsavedCard} from "metabase/meta/types/Card";
import type { QueryMode, ClickAction } from "metabase/meta/types/Visualization";
import type { TableMetadata } from "metabase/meta/types/Metadata";
@@ -21,7 +21,7 @@ type Props = {
mode: QueryMode,
card: Card,
tableMetadata: TableMetadata,
- setCardAndRun: (card: Card) => void
+ navigateToNewCardInsideQB: any => void
};
const CIRCLE_SIZE = 48;
@@ -71,18 +71,9 @@ export default class ActionsWidget extends Component<*, Props, *> {
});
};
- handleOnChangeCardAndRun(nextCard: UnsavedCard|Card) {
- const { card } = this.props;
-
- // Include the original card id if present for showing the lineage next to title
- const nextCardWithOriginalId = {
- ...nextCard,
- // $FlowFixMe
- original_card_id: card.id || card.original_card_id
- };
- if (nextCardWithOriginalId) {
- this.props.setCardAndRun(nextCardWithOriginalId);
- }
+ handleOnChangeCardAndRun = ({ nextCard }: { nextCard: Card|UnsavedCard}) => {
+ const { card: previousCard } = this.props;
+ this.props.navigateToNewCardInsideQB({ nextCard, previousCard });
}
handleActionClick = (index: number) => {
@@ -94,7 +85,7 @@ export default class ActionsWidget extends Component<*, Props, *> {
const nextCard = action.card();
if (nextCard) {
MetabaseAnalytics.trackEvent("Actions", "Executed Action", `${action.section||""}:${action.name||""}`);
- this.handleOnChangeCardAndRun(nextCard);
+ this.handleOnChangeCardAndRun({ nextCard });
}
this.close();
}
@@ -170,12 +161,12 @@ export default class ActionsWidget extends Component<*, Props, *> {
</div>
</div>
<PopoverComponent
- onChangeCardAndRun={(card) => {
- if (card) {
+ onChangeCardAndRun={({ nextCard }) => {
+ if (nextCard) {
if (selectedAction) {
MetabaseAnalytics.trackEvent("Actions", "Executed Action", `${selectedAction.section||""}:${selectedAction.name||""}`);
}
- this.handleOnChangeCardAndRun(card)
+ this.handleOnChangeCardAndRun({ nextCard })
}
}}
onClose={this.close}
diff --git a/frontend/src/metabase/query_builder/components/VisualizationResult.jsx b/frontend/src/metabase/query_builder/components/VisualizationResult.jsx
index d959a936de3aa3bd747248fb0e3213abc5c18bb3..def5c8c63df9aadc09776c1812e99a8390de311c 100644
--- a/frontend/src/metabase/query_builder/components/VisualizationResult.jsx
+++ b/frontend/src/metabase/query_builder/components/VisualizationResult.jsx
@@ -7,7 +7,7 @@ import VisualizationErrorMessage from './VisualizationErrorMessage';
import Visualization from "metabase/visualizations/components/Visualization.jsx";
import { datasetContainsNoResults } from "metabase/lib/dataset";
-const VisualizationResult = ({card, isObjectDetail, lastRunDatasetQuery, result, ...props}) => {
+const VisualizationResult = ({card, isObjectDetail, lastRunDatasetQuery, navigateToNewCardInsideQB, result, ...props}) => {
const noResults = datasetContainsNoResults(result.data);
if (isObjectDetail) {
@@ -34,7 +34,7 @@ const VisualizationResult = ({card, isObjectDetail, lastRunDatasetQuery, result,
};
return <Visualization
series={[{ card: vizCard, data: result.data }]}
- onChangeCardAndRun={props.setCardAndRun}
+ onChangeCardAndRun={navigateToNewCardInsideQB}
isEditing={true}
// Table:
{...props}
@@ -43,11 +43,11 @@ const VisualizationResult = ({card, isObjectDetail, lastRunDatasetQuery, result,
}
VisualizationResult.propTypes = {
- card: PropTypes.object.isRequired,
- isObjectDetail: PropTypes.bool.isRequired,
- lastRunDatasetQuery: PropTypes.object.isRequired,
- result: PropTypes.object.isRequired,
- setCardAndRun: PropTypes.func,
+ card: PropTypes.object.isRequired,
+ isObjectDetail: PropTypes.bool.isRequired,
+ lastRunDatasetQuery: PropTypes.object.isRequired,
+ result: PropTypes.object.isRequired,
+ navigateToNewCardInsideQB: PropTypes.func,
}
export default VisualizationResult;
diff --git a/frontend/src/metabase/visualizations/components/ChartClickActions.jsx b/frontend/src/metabase/visualizations/components/ChartClickActions.jsx
index 13182daf6a5ece303c9310479606c5a9592959e6..e2065002f6913ac87c419d10c3a1505e2077a9df 100644
--- a/frontend/src/metabase/visualizations/components/ChartClickActions.jsx
+++ b/frontend/src/metabase/visualizations/components/ChartClickActions.jsx
@@ -9,7 +9,6 @@ import Popover from "metabase/components/Popover";
import MetabaseAnalytics from "metabase/lib/analytics";
import type { ClickObject, ClickAction } from "metabase/meta/types/Visualization";
-import type { Card } from "metabase/meta/types/Card";
import _ from "underscore";
@@ -54,7 +53,7 @@ Object.values(SECTIONS).map((section, index) => {
type Props = {
clicked: ClickObject,
clickActions: ?ClickAction[],
- onChangeCardAndRun: (card: ?Card) => void,
+ onChangeCardAndRun: (Object) => void,
onClose: () => void
};
@@ -79,12 +78,12 @@ export default class ChartClickActions extends Component<*, Props, State> {
if (action.popover) {
this.setState({ popoverAction: action });
} else if (action.card) {
- const card = action.card();
+ const nextCard = action.card();
MetabaseAnalytics.trackEvent("Actions", "Executed Click Action", `${action.section||""}:${action.name||""}`);
- onChangeCardAndRun(card);
+ onChangeCardAndRun({ nextCard });
this.close();
}
- }
+ };
render() {
const { clicked, clickActions, onChangeCardAndRun } = this.props;
@@ -99,11 +98,11 @@ export default class ChartClickActions extends Component<*, Props, State> {
const PopoverContent = popoverAction.popover;
popover = (
<PopoverContent
- onChangeCardAndRun={(card) => {
+ onChangeCardAndRun={({ nextCard }) => {
if (popoverAction) {
MetabaseAnalytics.trackEvent("Action", "Executed Click Action", `${popoverAction.section||""}:${popoverAction.name||""}`);
}
- onChangeCardAndRun(card);
+ onChangeCardAndRun({ nextCard });
}}
onClose={() => {
MetabaseAnalytics.trackEvent("Action", "Dismissed Click Action Menu");
diff --git a/frontend/src/metabase/visualizations/components/LegendHeader.jsx b/frontend/src/metabase/visualizations/components/LegendHeader.jsx
index bec9352dd3b06ea6fec5dbb6a499bbfbc9e63cc2..bd7aa4221770d9a310cb5375dfb44daec8fbdaff 100644
--- a/frontend/src/metabase/visualizations/components/LegendHeader.jsx
+++ b/frontend/src/metabase/visualizations/components/LegendHeader.jsx
@@ -71,7 +71,7 @@ export default class LegendHeader extends Component {
onClick={s.clicked && visualizationIsClickable(s.clicked) ?
((e) => onVisualizationClick({ ...s.clicked, element: e.currentTarget }))
: onChangeCardAndRun ?
- ((e) => onChangeCardAndRun(s.card))
+ (() => onChangeCardAndRun({ nextCard: s.card, seriesIndex: index }))
: null }
/>,
onRemoveSeries && index > 0 &&
diff --git a/frontend/src/metabase/visualizations/components/LineAreaBarChart.jsx b/frontend/src/metabase/visualizations/components/LineAreaBarChart.jsx
index 17a6b75f3aa7260907b4b564b50b75d5ee7dd2f6..a30ae39db976c6efb3155c89650cdc3ab7f18844 100644
--- a/frontend/src/metabase/visualizations/components/LineAreaBarChart.jsx
+++ b/frontend/src/metabase/visualizations/components/LineAreaBarChart.jsx
@@ -188,11 +188,15 @@ export default class LineAreaBarChart extends Component<*, VisualizationProps, *
let originalSeries = series._raw || series;
let cardIds = _.uniq(originalSeries.map(s => s.card.id))
+ const isComposedOfMultipleQuestions = cardIds.length > 1;
if (showTitle && settings["card.title"]) {
titleHeaderSeries = [{ card: {
name: settings["card.title"],
- id: cardIds.length === 1 ? cardIds[0] : null
+ ...(isComposedOfMultipleQuestions ? {} : {
+ id: cardIds[0],
+ dataset_query: originalSeries[0].card.dataset_query
+ }),
}}];
}
@@ -208,7 +212,9 @@ export default class LineAreaBarChart extends Component<*, VisualizationProps, *
series={titleHeaderSeries}
description={settings["card.description"]}
actionButtons={actionButtons}
- onChangeCardAndRun={onChangeCardAndRun}
+ // If a dashboard card is composed of multiple questions, its custom card title
+ // shouldn't act as a link as it's ambiguous that which question it should open
+ onChangeCardAndRun={ isComposedOfMultipleQuestions ? null : onChangeCardAndRun }
/>
: null }
{ multiseriesHeaderSeries || (!titleHeaderSeries && actionButtons) ? // always show action buttons if we have them
diff --git a/frontend/src/metabase/visualizations/components/Visualization.jsx b/frontend/src/metabase/visualizations/components/Visualization.jsx
index c740ea684b457633a6dfa912e29a1abdc9985c49..c352b55afbad0fcaa97601c2ab4f60561f711bc7 100644
--- a/frontend/src/metabase/visualizations/components/Visualization.jsx
+++ b/frontend/src/metabase/visualizations/components/Visualization.jsx
@@ -31,7 +31,7 @@ import cx from "classnames";
export const ERROR_MESSAGE_GENERIC = "There was a problem displaying this chart.";
export const ERROR_MESSAGE_PERMISSION = "Sorry, you don't have permission to see this card."
-import type { UnsavedCard, VisualizationSettings} from "metabase/meta/types/Card";
+import type { VisualizationSettings} from "metabase/meta/types/Card";
import type { HoverObject, ClickObject, Series } from "metabase/meta/types/Visualization";
import type { Metadata } from "metabase/meta/types/Metadata";
@@ -63,7 +63,7 @@ type Props = {
// for click actions
metadata: Metadata,
- onChangeCardAndRun: (card: UnsavedCard) => void,
+ onChangeCardAndRun: any => void,
// used for showing content in place of visualization, e.x. dashcard filter mapping
replacementContent: Element<any>,
@@ -221,27 +221,16 @@ export default class Visualization extends Component<*, Props, State> {
setTimeout(() => {
this.setState({ clicked });
}, 100);
- }
+ };
- handleOnChangeCardAndRun = (card: UnsavedCard) => {
+ // Add the underlying card of current series to onChangeCardAndRun if available
+ handleOnChangeCardAndRun = ({ nextCard, seriesIndex }) => {
const { series, clicked } = this.state;
- const index = (clicked && clicked.seriesIndex) || 0;
- const originalCard = series && series[index] && series[index].card;
+ const index = seriesIndex || (clicked && clicked.seriesIndex) || 0;
+ const previousCard = series && series[index] && series[index].card;
- let cardId = card.id || card.original_card_id;
- // if the supplied card doesn't have an id, get it from the original card
- if (cardId == null && originalCard) {
- // $FlowFixMe
- cardId = originalCard.id || originalCard.original_card_id;
- }
-
- this.props.onChangeCardAndRun({
- ...card,
- id: cardId,
- // $FlowFixMe
- original_card_id: cardId
- });
+ this.props.onChangeCardAndRun({ nextCard, previousCard });
}
onRender = ({ yAxisSplit, warnings = [] } = {}) => {
diff --git a/frontend/src/metabase/visualizations/lib/utils.js b/frontend/src/metabase/visualizations/lib/utils.js
index c776525135fcb9e7ac46f0d0841c7020bb82785c..5aa2aa826e3e930abb3b51c26fa759567b2889f3 100644
--- a/frontend/src/metabase/visualizations/lib/utils.js
+++ b/frontend/src/metabase/visualizations/lib/utils.js
@@ -249,3 +249,32 @@ function wrapMethod(object, name, method) {
}
}
}
+// TODO Atte Keinänen 5/30/17 Extract to metabase-lib card/question logic
+export const cardHasBecomeDirty = (nextCard, previousCard) =>
+ !_.isEqual(previousCard.dataset_query, nextCard.dataset_query) || previousCard.display !== nextCard.display;
+
+export function getCardAfterVisualizationClick(nextCard, previousCard) {
+ if (cardHasBecomeDirty(nextCard, previousCard)) {
+ const isMultiseriesQuestion = !nextCard.id;
+ const alreadyHadLineage = !!previousCard.original_card_id;
+
+ return {
+ ...nextCard,
+ // Original card id is needed for showing the "started from" lineage in dirty cards.
+ original_card_id: alreadyHadLineage
+ // Just recycle the original card id of previous card if there was one
+ ? previousCard.original_card_id
+ // A multi-aggregation or multi-breakout series legend / drill-through action
+ // should always use the id of underlying/previous card
+ : (isMultiseriesQuestion ? previousCard.id : nextCard.id)
+ }
+ } else {
+ // Even though the card is currently clean, we might still apply dashboard parameters to it,
+ // so add the original_card_id to ensure a correct behavior in that context
+ return {
+ ...nextCard,
+ original_card_id: nextCard.id
+ };
+ }
+}
+
diff --git a/frontend/src/metabase/visualizations/lib/utils.spec.js b/frontend/src/metabase/visualizations/lib/utils.spec.js
new file mode 100644
index 0000000000000000000000000000000000000000..4123abdd97ab53dc8c74e3356cea72ec1c1d4959
--- /dev/null
+++ b/frontend/src/metabase/visualizations/lib/utils.spec.js
@@ -0,0 +1,171 @@
+import {cardHasBecomeDirty, getCardAfterVisualizationClick} from "./utils";
+import _ from "underscore";
+
+// TODO Atte Keinänen 5/31/17 Rewrite tests using metabase-lib methods instead of a raw format
+
+const baseQuery = {
+ "database": 1,
+ "type": "query",
+ "query": {
+ "source_table": 2,
+ "aggregation": [
+ [
+ "count"
+ ]
+ ],
+ "breakout": [
+ [
+ "field-id",
+ 2
+ ]
+ ]
+ }
+};
+const derivedQuery = {
+ ...baseQuery,
+ "query": {
+ ...baseQuery.query,
+ "filter": [
+ "time-interval",
+ [
+ "field-id",
+ 1
+ ],
+ -7,
+ "day"
+ ]
+ }
+};
+
+const breakoutMultiseriesQuery = {
+ ...baseQuery,
+ "query": {
+ ...baseQuery.query,
+ "breakout": [
+ ...baseQuery.query.breakout,
+ [
+ "fk->",
+ 1,
+ 10
+ ]
+ ]
+ }
+};
+const derivedBreakoutMultiseriesQuery = {
+ ...breakoutMultiseriesQuery,
+ "query": {
+ ...breakoutMultiseriesQuery.query,
+ "filter": [
+ "time-interval",
+ [
+ "field-id",
+ 1
+ ],
+ -7,
+ "day"
+ ]
+ }
+};
+
+const savedCard = {
+ id: 3,
+ dataset_query: baseQuery,
+ display: "line"
+};
+const clonedSavedCard = {
+ id: 3,
+ dataset_query: _.clone(baseQuery),
+ display: "line"
+};
+const dirtyCardOnlyOriginalId = {
+ original_card_id: 7,
+ dataset_query: baseQuery,
+ display: "line"
+};
+
+const derivedCard = {
+ ...dirtyCardOnlyOriginalId,
+ dataset_query: derivedQuery
+};
+const derivedCardModifiedId = {
+ ...savedCard,
+ dataset_query: derivedQuery
+};
+const derivedDirtyCard = {
+ ...dirtyCardOnlyOriginalId,
+ dataset_query: derivedQuery
+};
+
+const derivedCardWithDifferentDisplay = {
+ ...savedCard,
+ display: "table"
+};
+
+const savedMultiseriesCard = {
+ ...savedCard,
+ dataset_query: breakoutMultiseriesQuery
+};
+const derivedMultiseriesCard = {
+ // id is not present when drilling through series / multiseries
+ dataset_query: derivedBreakoutMultiseriesQuery,
+ display: savedCard.display
+};
+const newCard = {
+ dataset_query: baseQuery,
+ display: "line"
+};
+const modifiedNewCard = {
+ dataset_query: derivedQuery,
+ display: "line"
+};
+
+describe("metabase/visualization/lib/utils", () => {
+ describe("cardHasBecomeDirty", () => {
+ it("should consider cards with different display types dirty", () => {
+ // mostly for action widget actions that only change the display type
+ expect(cardHasBecomeDirty(derivedCardWithDifferentDisplay, savedCard)).toEqual(true);
+ });
+
+ it("should consider cards with different data data dirty", () => {
+ expect(cardHasBecomeDirty(derivedCard, savedCard)).toEqual(true);
+ });
+
+ it("should consider cards with same display type and data clean", () => {
+ // i.e. the card is practically the same as original card
+ expect(cardHasBecomeDirty(clonedSavedCard, savedCard)).toEqual(false);
+ });
+ });
+
+ describe("getCardAfterVisualizationClick", () => {
+ it("should use the id of a previous card in case of a multi-breakout visualization", () => {
+ expect(getCardAfterVisualizationClick(derivedMultiseriesCard, savedMultiseriesCard))
+ .toMatchObject({original_card_id: savedMultiseriesCard.id})
+ });
+
+ // TODO: Atte Keinänen 5/31/17 This scenario is a little fuzzy at the moment as there have been
+ // some specific corner cases where the id in previousCard is wrong/missing
+ // We should validate that previousCard always has an id as it should
+ it("if the new card contains the id it's more reliable to use it for initializing lineage", () => {
+ expect(getCardAfterVisualizationClick(derivedCardModifiedId, savedCard))
+ .toMatchObject({original_card_id: derivedCardModifiedId.id})
+ });
+
+ it("should be able to continue the lineage even if the previous question was dirty already", () => {
+ expect(getCardAfterVisualizationClick(derivedDirtyCard, dirtyCardOnlyOriginalId))
+ .toMatchObject({original_card_id: dirtyCardOnlyOriginalId.original_card_id})
+ });
+
+ it("should just pass the new question if the previous question was new", () => {
+ expect(getCardAfterVisualizationClick(modifiedNewCard, newCard))
+ .toMatchObject(modifiedNewCard)
+ });
+
+ it("should populate original_card_id even if the question isn't modified", () => {
+ // This is the hack to interoperate with questionUrlWithParameters when
+ // dashboard parameters are applied to a dashcards
+ expect(getCardAfterVisualizationClick(clonedSavedCard, savedCard))
+ .toMatchObject({original_card_id: savedCard.id})
+ });
+ })
+});
+
diff --git a/frontend/src/metabase/visualizations/visualizations/Scalar.jsx b/frontend/src/metabase/visualizations/visualizations/Scalar.jsx
index a973509d374442caa81a4fb5942033d0378b5d20..730f9b614a93ac56b148a10d331bea4b7ce89e77 100644
--- a/frontend/src/metabase/visualizations/visualizations/Scalar.jsx
+++ b/frontend/src/metabase/visualizations/visualizations/Scalar.jsx
@@ -194,7 +194,7 @@ export default class Scalar extends Component<*, VisualizationProps, *> {
<div className={styles.Title + " flex align-center"}>
<Ellipsified tooltip={card.name}>
<span
- onClick={onChangeCardAndRun && (() => onChangeCardAndRun(card))}
+ onClick={onChangeCardAndRun && (() => onChangeCardAndRun({ nextCard: card }))}
className={cx("fullscreen-normal-text fullscreen-night-text", {
"cursor-pointer": !!onChangeCardAndRun
})}