diff --git a/.clj-kondo/test/hooks/clojure/test_test.clj b/.clj-kondo/test/hooks/clojure/test_test.clj
index 19bf39b462102075526370191b98a63d2ba91b58..bcd487a1c905beffec5ec3d4a6ebdfb65da9f304 100644
--- a/.clj-kondo/test/hooks/clojure/test_test.clj
+++ b/.clj-kondo/test/hooks/clojure/test_test.clj
@@ -3,7 +3,6 @@
[clj-kondo.hooks-api :as api]
[clj-kondo.impl.utils]
[clojure.edn :as edn]
- [clojure.java.io :as io]
[clojure.test :refer :all]
[hooks.clojure.test]))
@@ -44,9 +43,13 @@
(testing "Make sure we keep hooks.clojure.test/driver-keywords up to date"
(let [driver-keywords (-> (slurp ".clj-kondo/config.edn")
edn/read-string
- (get-in [:linters :metabase/disallow-hardcoded-driver-names-in-tests :drivers]))]
- (doseq [^java.io.File file (.listFiles (io/file "modules/drivers"))
- :when (.isDirectory file)
- :let [driver (keyword (.getName file))]]
+ (get-in [:linters :metabase/disallow-hardcoded-driver-names-in-tests :drivers]))
+ driver-modules (->> (slurp "modules/drivers/deps.edn")
+ edn/read-string
+ :deps
+ vals
+ (keep (comp keyword :local/root))
+ (into #{}))]
+ (doseq [driver driver-modules]
(is (contains? driver-keywords driver)
(format "hooks.clojure.test/driver-keywords should contain %s, please add it" driver))))))
diff --git a/docs/questions/query-builder/expressions-list.md b/docs/questions/query-builder/expressions-list.md
index 2cf9a09baeca59a644c381400f2543564095620d..6e4ff499c89a719b6f68988fe22fa9b4e7924647 100644
--- a/docs/questions/query-builder/expressions-list.md
+++ b/docs/questions/query-builder/expressions-list.md
@@ -767,13 +767,6 @@ Example: `Offset(Sum([Total]), -1)` would get the `Sum([Total])` value from the
## Limitations
- [Aggregation expressions](#aggregations) can only be used in the **Summarize** section of the query builder.
-- Functions that return a boolean value, like [isempty](#isempty) or [contains](#contains), cannot be used to create a custom column. To create a custom column based on one of these functions, you must combine them with another function, like `case`.
-
- For example, to create a new custom column that contains `true` if `[Title]` contain `'Wallet'`, you can use the custom expression
-
- ```
- case(contains([Title], 'Wallet'), true, false)
- ```
## Database limitations
diff --git a/docs/releases.md b/docs/releases.md
index 9f835829be3be0e8046f03dd3b544ecd93deda90..930b6c99e90912238c96867f368831290ff0347b 100644
--- a/docs/releases.md
+++ b/docs/releases.md
@@ -19,6 +19,7 @@ To see what's new, check out all the [major release announcements](https://www.m
## Metabase Enterprise Edition releases
+- [v1.51.2](https://github.com/metabase/metabase/releases/tag/v1.51.2)
- [v1.51.1](https://github.com/metabase/metabase/releases/tag/v1.51.1)
- [v1.50.31](https://github.com/metabase/metabase/releases/tag/v1.50.31)
- [v1.50.30](https://github.com/metabase/metabase/releases/tag/v1.50.30)
@@ -211,6 +212,7 @@ To see what's new, check out all the [major release announcements](https://www.m
## Metabase Open Source Edition releases
+- [v0.51.2](https://github.com/metabase/metabase/releases/tag/v0.51.2)
- [v0.51.1](https://github.com/metabase/metabase/releases/tag/v0.51.1)
- [v0.50.31](https://github.com/metabase/metabase/releases/tag/v0.50.31)
- [v0.50.30](https://github.com/metabase/metabase/releases/tag/v0.50.30)
diff --git a/e2e/support/helpers/e2e-dashboard-helpers.ts b/e2e/support/helpers/e2e-dashboard-helpers.ts
index baf0b34866e6685ff3c1b8e8a44e33e3e91fc1de..4cd7ae0f23c32ef06b73b8fe911e83761293bc1a 100644
--- a/e2e/support/helpers/e2e-dashboard-helpers.ts
+++ b/e2e/support/helpers/e2e-dashboard-helpers.ts
@@ -88,12 +88,10 @@ export function saveDashboard({
cy.button(buttonLabel).click();
if (awaitRequest) {
- cy.wait("@saveDashboardCards").then(() => {
- cy.findByText(editBarText).should("not.exist");
- });
- } else {
- cy.findByText(editBarText).should("not.exist");
+ cy.wait("@saveDashboardCards");
}
+
+ cy.findByText(editBarText).should("not.exist");
cy.wait(waitMs); // this is stupid but necessary to due to the dashboard resizing and detaching elements
}
diff --git a/e2e/test/scenarios/custom-column/cc-boolean-functions.cy.spec.ts b/e2e/test/scenarios/custom-column/cc-boolean-functions.cy.spec.ts
index 977bd95c586b7b3474c7b46bde9c08c0b5b05215..fd926e4803e22671261bf30b2a538d12f302e3bc 100644
--- a/e2e/test/scenarios/custom-column/cc-boolean-functions.cy.spec.ts
+++ b/e2e/test/scenarios/custom-column/cc-boolean-functions.cy.spec.ts
@@ -4,6 +4,7 @@ import {
type StructuredQuestionDetails,
assertQueryBuilderRowCount,
assertTableData,
+ changeSynchronousBatchUpdateSetting,
createDashboard,
createQuestion,
editDashboard,
@@ -625,6 +626,17 @@ describe("scenarios > custom column > boolean functions", () => {
);
}
+ beforeEach(() => {
+ cy.signInAsAdmin();
+ changeSynchronousBatchUpdateSetting(true);
+ cy.signInAsNormalUser();
+ });
+
+ afterEach(() => {
+ cy.signInAsAdmin();
+ changeSynchronousBatchUpdateSetting(false);
+ });
+
it("should be able setup an 'open question' click behavior", () => {
createDashboardWithQuestion().then(dashboard =>
visitDashboard(dashboard.id),
@@ -681,7 +693,9 @@ describe("scenarios > custom column > boolean functions", () => {
.findByText(parameterDetails.name)
.click();
popover().findByText(expressionName).click();
+ cy.intercept("GET", "/api/dashboard/*").as("getDashboard");
saveDashboard();
+ cy.wait("@getDashboard");
cy.log("verify click behavior");
getDashboardCard().within(() => {
diff --git a/e2e/test/scenarios/dashboard/dashboard-reproductions.cy.spec.js b/e2e/test/scenarios/dashboard/dashboard-reproductions.cy.spec.js
index 7f48ea40a206aee3c0dab2547f70d474c8606bb5..3923b30af665a3ebf70e6753eb7feea240a5d176 100644
--- a/e2e/test/scenarios/dashboard/dashboard-reproductions.cy.spec.js
+++ b/e2e/test/scenarios/dashboard/dashboard-reproductions.cy.spec.js
@@ -323,119 +323,120 @@ describe("issue 16559", () => {
restore();
cy.signInAsAdmin();
- cy.createDashboard(dashboardDetails).then(
- ({ body: { id: dashboardId } }) => {
- visitDashboard(dashboardId);
- },
- );
- });
-
- it(
- "should always show the most recent revision (metabase#16559)",
- { tags: "@flaky" },
- () => {
- openDashboardInfoSidebar();
- sidesheet().within(() => {
- cy.findByRole("tab", { name: "History" }).click();
- cy.log("Dashboard creation");
- cy.findByTestId("dashboard-history-list")
- .findAllByRole("listitem")
- .eq(0)
- .findByText("You created this.")
- .should("be.visible");
- });
- closeDashboardInfoSidebar();
-
- cy.log("Edit dashboard");
- editDashboard();
- openQuestionsSidebar();
- sidebar().findByText("Orders, Count").click();
- cy.button("Save").click();
-
- openDashboardInfoSidebar();
- sidesheet().within(() => {
- cy.findByRole("tab", { name: "History" }).click();
- cy.findByTestId("dashboard-history-list")
- .findAllByRole("listitem")
- .eq(0)
- .findByText("You added a card.")
- .should("be.visible");
- });
- closeDashboardInfoSidebar();
+ createDashboard(dashboardDetails).then(response => {
+ visitDashboard(response.body.id);
+ });
- cy.log("Change dashboard name");
- cy.findByTestId("dashboard-name-heading")
- .click()
- .type(" modified")
- .blur();
+ cy.intercept("GET", "/api/collection/tree?*").as("getCollections");
+ cy.intercept("PUT", "/api/dashboard/*").as("saveDashboard");
+ cy.intercept("POST", "/api/card/*/query").as("cardQuery");
+ });
- openDashboardInfoSidebar();
- sidesheet().within(() => {
- cy.findByRole("tab", { name: "History" }).click();
+ it("should always show the most recent revision (metabase#16559)", () => {
+ openDashboardInfoSidebar();
+ sidesheet().within(() => {
+ cy.findByRole("tab", { name: "History" }).click();
+ cy.log("Dashboard creation");
+ cy.findByTestId("dashboard-history-list")
+ .findAllByRole("listitem")
+ .eq(0)
+ .findByText("You created this.")
+ .should("be.visible");
+ });
+ closeDashboardInfoSidebar();
- cy.findByTestId("dashboard-history-list")
- .findAllByRole("listitem")
- .eq(0)
- .findByText(
- 'You renamed this Dashboard from "16559 Dashboard" to "16559 Dashboard modified".',
- )
- .should("be.visible");
+ cy.log("Edit dashboard");
+ editDashboard();
+ openQuestionsSidebar();
+ sidebar().findByText("Orders, Count").click();
+ cy.wait("@cardQuery");
+ cy.button("Save").click();
+ cy.wait("@saveDashboard");
+
+ openDashboardInfoSidebar();
+ sidesheet().within(() => {
+ cy.findByRole("tab", { name: "History" }).click();
+ cy.findByTestId("dashboard-history-list")
+ .findAllByRole("listitem")
+ .eq(0)
+ .findByText("You added a card.")
+ .should("be.visible");
+ });
+ closeDashboardInfoSidebar();
- cy.log("Add description");
- cy.findByRole("tab", { name: "Overview" }).click();
+ cy.log("Change dashboard name");
+ cy.findByTestId("dashboard-name-heading").click().type(" modified").blur();
+ cy.wait("@saveDashboard");
- cy.findByPlaceholderText("Add description")
- .click()
- .type("16559 description")
- .blur();
+ openDashboardInfoSidebar();
+ sidesheet().within(() => {
+ cy.findByRole("tab", { name: "History" }).click();
- cy.findByRole("tab", { name: "History" }).click();
+ cy.findByTestId("dashboard-history-list")
+ .findAllByRole("listitem")
+ .eq(0)
+ .findByText(
+ 'You renamed this Dashboard from "16559 Dashboard" to "16559 Dashboard modified".',
+ )
+ .should("be.visible");
- cy.findByTestId("dashboard-history-list")
- .findAllByRole("listitem")
- .eq(0)
- .findByText("You added a description.")
- .should("be.visible");
+ cy.log("Add description");
+ cy.findByRole("tab", { name: "Overview" }).click();
- cy.log("Toggle auto-apply filters");
- });
- closeDashboardInfoSidebar();
+ cy.findByPlaceholderText("Add description")
+ .click()
+ .type("16559 description")
+ .blur();
+ cy.wait("@saveDashboard");
- openDashboardSettingsSidebar();
- sidesheet().findByText("Auto-apply filters").click();
- closeDashboardSettingsSidebar();
+ cy.findByRole("tab", { name: "History" }).click();
- openDashboardInfoSidebar();
- sidesheet().within(() => {
- cy.findByRole("tab", { name: "History" }).click();
+ cy.findByTestId("dashboard-history-list")
+ .findAllByRole("listitem")
+ .eq(0)
+ .findByText("You added a description.")
+ .should("be.visible");
- cy.findByTestId("dashboard-history-list")
- .findAllByRole("listitem")
- .eq(0)
- .findByText("You set auto apply filters to false.")
- .should("be.visible");
- });
- closeDashboardInfoSidebar();
-
- cy.log("Move dashboard to another collection");
- dashboardHeader().icon("ellipsis").click();
- popover().findByText("Move").click();
- entityPickerModal().within(() => {
- cy.findByText("First collection").click();
- cy.button("Move").click();
- });
+ cy.log("Toggle auto-apply filters");
+ });
+ closeDashboardInfoSidebar();
+
+ openDashboardSettingsSidebar();
+ sidesheet().findByText("Auto-apply filters").click();
+ cy.wait("@saveDashboard");
+ closeDashboardSettingsSidebar();
+
+ openDashboardInfoSidebar();
+ sidesheet().within(() => {
+ cy.findByRole("tab", { name: "History" }).click();
+
+ cy.findByTestId("dashboard-history-list")
+ .findAllByRole("listitem")
+ .eq(0)
+ .findByText("You set auto apply filters to false.")
+ .should("be.visible");
+ });
+ closeDashboardInfoSidebar();
+
+ cy.log("Move dashboard to another collection");
+ dashboardHeader().icon("ellipsis").click();
+ popover().findByText("Move").click();
+ entityPickerModal().within(() => {
+ cy.findByText("First collection").click();
+ cy.button("Move").click();
+ cy.wait(["@saveDashboard", "@getCollections"]);
+ });
- openDashboardInfoSidebar();
- sidesheet().within(() => {
- cy.findByRole("tab", { name: "History" }).click();
- cy.findByTestId("dashboard-history-list")
- .findAllByRole("listitem")
- .eq(0)
- .findByText("You moved this Dashboard to First collection.")
- .should("be.visible");
- });
- },
- );
+ openDashboardInfoSidebar();
+ sidesheet().within(() => {
+ cy.findByRole("tab", { name: "History" }).click();
+ cy.findByTestId("dashboard-history-list")
+ .findAllByRole("listitem")
+ .eq(0)
+ .findByText("You moved this Dashboard to First collection.")
+ .should("be.visible");
+ });
+ });
});
describe("issue 17879", () => {
diff --git a/frontend/src/metabase/lib/icon.ts b/frontend/src/metabase/lib/icon.ts
index 2c4919a9016583704b79868c78c078a7095bba99..7013d100547e9de903a7d29569cb9087b09b3060 100644
--- a/frontend/src/metabase/lib/icon.ts
+++ b/frontend/src/metabase/lib/icon.ts
@@ -9,7 +9,7 @@ import type {
SearchModel,
} from "metabase-types/api";
-type IconModel = SearchModel | CollectionItemModel | "schema";
+export type IconModel = SearchModel | CollectionItemModel | "schema";
export type ObjectWithModel = {
id?: unknown;
@@ -22,7 +22,7 @@ export type ObjectWithModel = {
is_personal?: boolean;
};
-const modelIconMap: Record<IconModel, IconName> = {
+export const modelIconMap: Record<IconModel, IconName> = {
collection: "folder",
database: "database",
table: "table",
diff --git a/frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionDataSource/utils.js b/frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionDataSource/utils.js
index 81253ecbb4ba1e1da729e50fc8c778a7705b95b8..583d67926a1e6361cb30931bbf61b7258013f5cc 100644
--- a/frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionDataSource/utils.js
+++ b/frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionDataSource/utils.js
@@ -17,7 +17,12 @@ import { HeadBreadcrumbs } from "../HeaderBreadcrumbs";
import { IconWrapper, TablesDivider } from "./QuestionDataSource.styled";
-export function getDataSourceParts({ question, subHead, isObjectDetail }) {
+export function getDataSourceParts({
+ question,
+ subHead,
+ isObjectDetail,
+ formatTableAsComponent = true,
+}) {
if (!question) {
return [];
}
@@ -40,6 +45,7 @@ export function getDataSourceParts({ question, subHead, isObjectDetail }) {
icon: !subHead ? "database" : undefined,
name: database.displayName(),
href: database.id >= 0 && Urls.browseDatabase(database),
+ model: "database",
});
}
@@ -50,6 +56,7 @@ export function getDataSourceParts({ question, subHead, isObjectDetail }) {
const isBasedOnSavedQuestion = isVirtualCardId(table.id);
if (!isBasedOnSavedQuestion) {
parts.push({
+ model: "schema",
name: table.schema_name,
href: database.id >= 0 && Urls.browseSchema(table),
});
@@ -82,14 +89,22 @@ export function getDataSourceParts({ question, subHead, isObjectDetail }) {
}),
].filter(isNotNull);
- parts.push(
+ const part = formatTableAsComponent ? (
<QuestionTableBadges
tables={allTables}
subHead={subHead}
hasLink={hasTableLink}
isLast={!isObjectDetail}
- />,
+ />
+ ) : (
+ {
+ name: table.displayName(),
+ href: hasTableLink ? getTableURL(table) : "",
+ model: table.type ?? "table",
+ }
);
+
+ parts.push(part);
}
return parts.filter(part => isValidElement(part) || part.name || part.icon);
diff --git a/frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionDataSource/utils.unit.spec.ts b/frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionDataSource/utils.unit.spec.ts
new file mode 100644
index 0000000000000000000000000000000000000000..e57594e4090a1d7809403dbe643481f653d9b916
--- /dev/null
+++ b/frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionDataSource/utils.unit.spec.ts
@@ -0,0 +1,88 @@
+import { isValidElement } from "react";
+
+import { createMockMetadata } from "__support__/metadata";
+import Question from "metabase-lib/v1/Question";
+import type { Card } from "metabase-types/api";
+import {
+ createMockCard,
+ createMockDatabase,
+ createMockTable,
+} from "metabase-types/api/mocks";
+import { createSampleDatabase } from "metabase-types/api/mocks/presets";
+
+import { getDataSourceParts } from "./utils";
+
+const MULTI_SCHEMA_DB_ID = 2;
+const MULTI_SCHEMA_TABLE1_ID = 100;
+const MULTI_SCHEMA_TABLE2_ID = 101;
+
+function getMetadata() {
+ return createMockMetadata({
+ databases: [
+ createSampleDatabase(),
+ createMockDatabase({
+ id: MULTI_SCHEMA_DB_ID,
+ tables: [
+ createMockTable({
+ id: MULTI_SCHEMA_TABLE1_ID,
+ db_id: MULTI_SCHEMA_DB_ID,
+ schema: "first_schema",
+ }),
+ createMockTable({
+ id: MULTI_SCHEMA_TABLE2_ID,
+ db_id: MULTI_SCHEMA_DB_ID,
+ schema: "second_schema",
+ }),
+ ],
+ }),
+ ],
+ });
+}
+
+const createMockQuestion = (opts?: Partial<Card>) =>
+ new Question(createMockCard(opts), getMetadata());
+
+/** These tests cover new logic in the getDataSourceParts utility that is not covered in QuestionDataSource.unit.spec.js */
+describe("getDataSourceParts", () => {
+ afterEach(() => {
+ jest.restoreAllMocks();
+ });
+ it("returns an array of Records if formatTableAsComponent is false", () => {
+ const parts = getDataSourceParts({
+ question: createMockQuestion(),
+ subHead: false,
+ isObjectDetail: true,
+ formatTableAsComponent: false,
+ });
+ expect(parts).toHaveLength(2);
+ const partsArray = parts as any[];
+ expect(partsArray[0]).toEqual({
+ icon: "database",
+ name: "Sample Database",
+ href: "/browse/databases/1-sample-database",
+ model: "database",
+ });
+ expect(partsArray[1].name).toEqual("Products");
+ expect(partsArray[1].model).toEqual("table");
+ expect(partsArray[1].href).toMatch(/^\/question#[a-zA-Z0-9]{50}/);
+ expect(Object.keys(partsArray[1])).toHaveLength(3);
+ });
+
+ it("returns an array with the table formatted as a component if formatTableAsComponent is true", () => {
+ const parts = getDataSourceParts({
+ question: createMockQuestion(),
+ subHead: false,
+ isObjectDetail: true,
+ formatTableAsComponent: true,
+ });
+ expect(parts).toHaveLength(2);
+ const partsArray = parts as any[];
+ expect(partsArray[0]).toEqual({
+ icon: "database",
+ name: "Sample Database",
+ href: "/browse/databases/1-sample-database",
+ model: "database",
+ });
+ expect(isValidElement(partsArray[1])).toBe(true);
+ });
+});
diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/QuestionDetails.tsx b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/QuestionDetails.tsx
index fca6c9a29e479b32cb6b9de1101d9abe960a4bcc..be1534814f9326ea667f983b2b5b33a94a9a8907 100644
--- a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/QuestionDetails.tsx
+++ b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/QuestionDetails.tsx
@@ -2,22 +2,19 @@ import cx from "classnames";
import { useState } from "react";
import { c, t } from "ttag";
-import { getTableUrl } from "metabase/browse/containers/TableBrowser/TableBrowser";
import { getCollectionName } from "metabase/collections/utils";
import { SidesheetCardSection } from "metabase/common/components/Sidesheet";
import DateTime from "metabase/components/DateTime";
import Link from "metabase/core/components/Link";
import Styles from "metabase/css/core/index.css";
-import { useSelector } from "metabase/lib/redux";
import * as Urls from "metabase/lib/urls";
import { getUserName } from "metabase/lib/user";
-import { getMetadata } from "metabase/selectors/metadata";
import { QuestionPublicLinkPopover } from "metabase/sharing/components/PublicLinkPopover";
import { Box, Flex, FixedSizeIcon as Icon, Text } from "metabase/ui";
import type Question from "metabase-lib/v1/Question";
-import type { Database } from "metabase-types/api";
import SidebarStyles from "./QuestionInfoSidebar.module.css";
+import { QuestionSources } from "./components/QuestionSources";
export const QuestionDetails = ({ question }: { question: Question }) => {
const lastEditInfo = question.lastEditInfo();
@@ -74,52 +71,11 @@ export const QuestionDetails = ({ question }: { question: Question }) => {
</Flex>
</SidesheetCardSection>
<SharingDisplay question={question} />
- <SourceDisplay question={question} />
+ <QuestionSources question={question} />
</>
);
};
-function SourceDisplay({ question }: { question: Question }) {
- const sourceInfo = question.legacyQueryTable();
- const metadata = useSelector(getMetadata);
-
- if (!sourceInfo) {
- return null;
- }
-
- const model = String(sourceInfo.id).includes("card__") ? "card" : "table";
-
- const sourceUrl =
- model === "card"
- ? Urls.browseDatabase(sourceInfo.db as Database)
- : getTableUrl(sourceInfo, metadata);
-
- return (
- <SidesheetCardSection title={t`Based on`}>
- <Flex gap="sm" align="center">
- {sourceInfo.db && (
- <>
- <Text>
- <Link
- to={`/browse/databases/${sourceInfo.db.id}`}
- variant="brand"
- >
- {sourceInfo.db.name}
- </Link>
- </Text>
- {"/"}
- </>
- )}
- <Text>
- <Link to={sourceUrl} variant="brand">
- {sourceInfo?.display_name}
- </Link>
- </Text>
- </Flex>
- </SidesheetCardSection>
- );
-}
-
function SharingDisplay({ question }: { question: Question }) {
const publicUUID = question.publicUUID();
const embeddingEnabled = question._card.enable_embedding;
diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/components/QuestionSources.tsx b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/components/QuestionSources.tsx
new file mode 100644
index 0000000000000000000000000000000000000000..7fa2bba3cc6e124639e32a663b56f15d165a56cf
--- /dev/null
+++ b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/components/QuestionSources.tsx
@@ -0,0 +1,56 @@
+import { Fragment, useMemo } from "react";
+import { c } from "ttag";
+
+import { SidesheetCardSection } from "metabase/common/components/Sidesheet";
+import Link from "metabase/core/components/Link";
+import { Flex, FixedSizeIcon as Icon } from "metabase/ui";
+import type Question from "metabase-lib/v1/Question";
+
+import { getDataSourceParts } from "../../../ViewHeader/components/QuestionDataSource/utils";
+
+import type { QuestionSource } from "./types";
+import { getIconPropsForSource } from "./utils";
+
+export const QuestionSources = ({ question }: { question: Question }) => {
+ const sources = getDataSourceParts({
+ question,
+ subHead: false,
+ isObjectDetail: true,
+ formatTableAsComponent: false,
+ }) as unknown as QuestionSource[];
+
+ const sourcesWithIcons: QuestionSource[] = useMemo(() => {
+ return sources.map(source => ({
+ ...source,
+ iconProps: getIconPropsForSource(source),
+ }));
+ }, [sources]);
+
+ if (!sources.length) {
+ return null;
+ }
+
+ const title = c(
+ "This is a heading that appears above the names of the database, table, and/or question that a question is based on -- the 'sources' for the question. Feel free to translate this heading as though it said 'Based on these sources', if you think that would make more sense in your language.",
+ ).t`Based on`;
+
+ return (
+ <SidesheetCardSection title={title}>
+ <Flex gap="sm" align="flex-start">
+ {sourcesWithIcons.map(({ href, name, iconProps }, index) => (
+ <Fragment key={`${href}-${name}`}>
+ <Link to={href} variant="brand">
+ <Flex gap="sm" lh="1.25rem" maw="20rem">
+ {iconProps ? (
+ <Icon mt={2} c="text-dark" {...iconProps} />
+ ) : null}
+ {name}
+ </Flex>
+ </Link>
+ {index < sources.length - 1 && <Flex lh="1.25rem">{"/"}</Flex>}
+ </Fragment>
+ ))}
+ </Flex>
+ </SidesheetCardSection>
+ );
+};
diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/components/QuestionSources.unit.spec.tsx b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/components/QuestionSources.unit.spec.tsx
new file mode 100644
index 0000000000000000000000000000000000000000..d12e56ed2bf0549c39171d8f665051a5fefbf803
--- /dev/null
+++ b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/components/QuestionSources.unit.spec.tsx
@@ -0,0 +1,145 @@
+import { Route } from "react-router";
+import _ from "underscore";
+
+import { mockSettings } from "__support__/settings";
+import { createMockEntitiesState } from "__support__/store";
+import { renderWithProviders, screen, within } from "__support__/ui";
+import { modelIconMap } from "metabase/lib/icon";
+import { checkNotNull } from "metabase/lib/types";
+import { getMetadata } from "metabase/selectors/metadata";
+import { convertSavedQuestionToVirtualTable } from "metabase-lib/v1/metadata/utils/saved-questions";
+import type { Card, NormalizedTable } from "metabase-types/api";
+import { createMockCard, createMockSettings } from "metabase-types/api/mocks";
+import { createSampleDatabase } from "metabase-types/api/mocks/presets";
+import { createMockState } from "metabase-types/store/mocks";
+
+import { QuestionSources } from "./QuestionSources";
+
+interface SetupOpts {
+ card?: Card;
+ sourceCard?: Card;
+}
+
+const setup = async ({
+ card = createMockCard(),
+ sourceCard,
+}: SetupOpts = {}) => {
+ const state = createMockState({
+ settings: mockSettings(createMockSettings()),
+ entities: createMockEntitiesState({
+ databases: [createSampleDatabase()],
+ questions: _.compact([card, sourceCard]),
+ }),
+ });
+
+ // 😫 all this is necessary to test a card as a question source
+ if (sourceCard) {
+ const virtualTable = convertSavedQuestionToVirtualTable(sourceCard);
+
+ state.entities = {
+ ...state.entities,
+ tables: {
+ ...(state.entities.tables as Record<number, NormalizedTable>),
+ [virtualTable.id]: virtualTable,
+ },
+ databases: {
+ [state.entities.databases[1].id]: {
+ ...state.entities.databases[1],
+ tables: [
+ ...(state.entities.databases[1].tables ?? []),
+ virtualTable.id,
+ ],
+ },
+ },
+ };
+ }
+
+ const metadata = getMetadata(state);
+ const question = checkNotNull(metadata.question(card.id));
+
+ return renderWithProviders(
+ <Route
+ path="/"
+ component={() => <QuestionSources question={question} />}
+ />,
+ {
+ withRouter: true,
+ },
+ );
+};
+
+describe("QuestionSources", () => {
+ it("should show table source information", async () => {
+ const card = createMockCard({
+ name: "Question",
+ });
+ setup({ card });
+ const databaseLink = await screen.findByRole("link", {
+ name: /Sample Database/i,
+ });
+ expect(
+ await within(databaseLink).findByLabelText("database icon"),
+ ).toBeInTheDocument();
+ expect(databaseLink).toHaveAttribute(
+ "href",
+ "/browse/databases/1-sample-database",
+ );
+ expect(screen.getByText("/")).toBeInTheDocument();
+ const tableLink = await screen.findByRole("link", { name: /Products/i });
+ expect(tableLink).toBeInTheDocument();
+ expect(
+ await within(tableLink).findByLabelText(`table icon`),
+ ).toBeInTheDocument();
+ expect(tableLink).toHaveAttribute(
+ "href",
+ expect.stringMatching(/^\/question#[a-zA-Z0-9]{20}/),
+ );
+ });
+
+ it("should show card source information", async () => {
+ const card = createMockCard({
+ name: "My Question",
+ dataset_query: {
+ type: "query",
+ database: 1,
+ query: {
+ "source-table": "card__2",
+ },
+ },
+ });
+
+ const sourceCard = createMockCard({
+ name: "My Source Question",
+ id: 2,
+ });
+
+ await setup({ card, sourceCard });
+
+ const databaseLink = await screen.findByRole("link", {
+ name: /Sample Database/i,
+ });
+
+ expect(
+ await within(databaseLink).findByLabelText("database icon"),
+ ).toBeInTheDocument();
+ expect(databaseLink).toHaveAttribute(
+ "href",
+ "/browse/databases/1-sample-database",
+ );
+
+ expect(screen.getByText("/")).toBeInTheDocument();
+
+ const questionLink = await screen.findByRole("link", {
+ name: /My Source Question/i,
+ });
+ expect(
+ await within(questionLink).findByLabelText(
+ `${modelIconMap["card"]} icon`,
+ ),
+ ).toBeInTheDocument();
+ expect(questionLink).toHaveAttribute(
+ "href",
+ "/question/2-my-source-question",
+ );
+ });
+});
diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/components/types.ts b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/components/types.ts
new file mode 100644
index 0000000000000000000000000000000000000000..3f42bc97578c4810cd874b357f4b8841e1dda8c1
--- /dev/null
+++ b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/components/types.ts
@@ -0,0 +1,8 @@
+import type { IconData } from "metabase/lib/icon";
+
+export interface QuestionSource {
+ href: string;
+ name: string;
+ model?: string;
+ iconProps?: IconData;
+}
diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/components/utils.tsx b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/components/utils.tsx
new file mode 100644
index 0000000000000000000000000000000000000000..dd3b0538ca0d7fe2e4486486b135441387ab2335
--- /dev/null
+++ b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/components/utils.tsx
@@ -0,0 +1,21 @@
+import { match } from "ts-pattern";
+
+import { type IconData, type IconModel, getIcon } from "metabase/lib/icon";
+
+import type { QuestionSource } from "./types";
+
+export const getIconPropsForSource = (
+ source: QuestionSource,
+): IconData | undefined => {
+ const iconModel: IconModel | undefined = match(source.model)
+ .with("question", () => "card" as const)
+ .with("model", () => "dataset" as const)
+ .with("database", () => "database" as const)
+
+ .with("metric", () => "metric" as const)
+ .with("schema", () => undefined)
+ .otherwise(() => "table" as const);
+
+ const iconProps = iconModel ? getIcon({ model: iconModel }) : undefined;
+ return iconProps;
+};
diff --git a/modules/drivers/databricks/src/metabase/driver/databricks.clj b/modules/drivers/databricks/src/metabase/driver/databricks.clj
index 2f55d00a4ac7a9aa1a829c7083a66d5d027b9748..f5e2e0bff493030a2084ff28aa25d01d596c34e9 100644
--- a/modules/drivers/databricks/src/metabase/driver/databricks.clj
+++ b/modules/drivers/databricks/src/metabase/driver/databricks.clj
@@ -3,6 +3,7 @@
[clojure.string :as str]
[honey.sql :as sql]
[java-time.api :as t]
+ [metabase.config :as config]
[metabase.driver :as driver]
[metabase.driver.hive-like :as driver.hive-like]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
@@ -205,6 +206,7 @@
:HttpPath http-path
:uid "token"
:pwd token
+ :UserAgentEntry (format "Metabase/%s" (:tag config/mb-version-info))
:UseNativeQuery 1}
;; Following is used just for tests. See the [[metabase.driver.sql-jdbc.connection-test/perturb-db-details]]
;; and test that is using the function.
diff --git a/modules/drivers/databricks/test/metabase/driver/databricks_test.clj b/modules/drivers/databricks/test/metabase/driver/databricks_test.clj
index ffcae30e1ff8a54d6c7fc01faaecf9c3aed0ee13..c80fb5f47877f46ec5d1ded2fda47b38d9944d01 100644
--- a/modules/drivers/databricks/test/metabase/driver/databricks_test.clj
+++ b/modules/drivers/databricks/test/metabase/driver/databricks_test.clj
@@ -1,7 +1,9 @@
(ns ^:mb/driver-tests metabase.driver.databricks-test
(:require
+ [clojure.java.jdbc :as jdbc]
[clojure.test :refer :all]
[java-time.api :as t]
+ [metabase.config :as config]
[metabase.driver :as driver]
[metabase.driver.databricks :as databricks]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
@@ -242,6 +244,17 @@
(deftest additional-options-test
(mt/test-driver
:databricks
+ (testing "Connections with UserAgentEntry"
+ (sql-jdbc.conn/with-connection-spec-for-testing-connection
+ [spec [:databricks (:details (mt/db))]]
+ (is (= [{:a 1}] (jdbc/query spec ["select 1 as a"]))))
+ (with-redefs [config/mb-version-info {:tag "invalid agent"}]
+ (sql-jdbc.conn/with-connection-spec-for-testing-connection
+ [spec [:databricks (:details (mt/db))]]
+ (is (thrown-with-msg?
+ Exception
+ #"Incorrect format for User-Agent entry"
+ (jdbc/query spec ["select 1 as a"]))))))
(testing "Additional options are added to :subname key of generated spec"
(is (re-find #";IgnoreTransactions=0$"
(->> {:http-path "p/a/t/h",
diff --git a/src/metabase/models/card.clj b/src/metabase/models/card.clj
index 48deda5e0d5e71622f63045d317601f7fb15820b..41bb26a7c43d500278731536cb208337cf09f083 100644
--- a/src/metabase/models/card.clj
+++ b/src/metabase/models/card.clj
@@ -783,6 +783,92 @@
:query_type ;; these first three may not even be changeable
:dataset_query})
+(defn- breakout-->identifier->refs
+ "Generate mapping of of _ref identifier_ -> #{_ref..._}.
+
+ _ref identifier_ is a vector of first 2 elements of ref, eg. [:expression \"xix\"] or [:field 10]"
+ [breakouts]
+ (-> (group-by #(subvec % 0 2) breakouts)
+ (update-vals set)))
+
+(defn- action-for-identifier+refs
+ "Generate _action_ for combination of args.
+
+ _Action_ is to be performed on _parameter mapping_ of a _dashcard_. For more info see
+ the [[update-associated-parameters!]]'s docstring.
+
+ _Action_ has a form of [<action> & args]."
+ [after--identifier->refs identifier before--refs]
+ (let [after--refs (get after--identifier->refs identifier #{})]
+ (when (and (= 1 (count before--refs) (count after--refs))
+ (not= before--refs after--refs))
+ [:update (first after--refs)])))
+
+(defn- breakouts-->identifier->action
+ "Generate mapping of _identifier_ -> _action_.
+
+ _identifier_ is is a vector of first 2 elements of ref, eg. [:expression \"xix\"] or [:field 10]. Action is generated
+ in [[action-for-identifier+refs]] and performed later in [[update-mapping]]."
+ [breakout-before-update breakout-after-update]
+ (let [before--identifier->refs (breakout-->identifier->refs breakout-before-update)
+ after--identifier->refs (breakout-->identifier->refs breakout-after-update)]
+ ;; Remove no-ops to avoid redundant db calls in [[update-associated-parameters!]].
+ (->> before--identifier->refs
+ (m/map-kv-vals #(action-for-identifier+refs after--identifier->refs %1 %2))
+ (m/filter-vals some?)
+ not-empty)))
+
+(defn eligible-mapping?
+ "Decide whether parameter mapping has strucuture so it can be updated presumably using [[update-mapping]]."
+ [{[dim [ref-kind]] :target :as _mapping}]
+ (and (= dim :dimension)
+ (#{:field :expression} ref-kind)))
+
+(defn- update-mapping
+ "Return modifed mapping according to action."
+ [identifier->action {[_dim ref] :target :as mapping}]
+ (let [identifier (subvec ref 0 2)
+ [action arg] (get identifier->action identifier)]
+ (case action
+ :update (assoc-in mapping [:target 1] arg)
+ mapping)))
+
+(defn- updates-for-dashcards
+ [identifier->action dashcards]
+ (not-empty (for [{:keys [id parameter_mappings]} dashcards
+ :let [updated (into [] (map #(if (eligible-mapping? %)
+ (update-mapping identifier->action %)
+ %))
+ parameter_mappings)]
+ :when (not= parameter_mappings updated)]
+ [id {:parameter_mappings updated}])))
+
+(defn- update-associated-parameters!
+ "Update _parameter mappings_ of _dashcards_ that target modified _card_, to reflect the modification.
+
+ This function handles only modifications to breakout.
+
+ Context. Card can have multiple multiple breakout elements referencing same field or expression, having different
+ _temporal unit_. Those refs can be targeted by dashboard _temporal unit parameter_. If refs change, and card is saved,
+ _parameter mappings_ have to be updated to target new, modified refs. This function takes care of that.
+
+ First mappings of _identifier_ -> _action_ are generated. _identifier_ is described
+ eg. in [[breakouts-->identifier->action]] docstring. Then, dashcards are fetched and updates are generated
+ by [[updates-for-dashcards]]. Updates are then executed."
+ [card-before card-after]
+ (let [card->breakout #(-> % :dataset_query mbql.normalize/normalize :query :breakout)
+ breakout-before (card->breakout card-before)
+ breakout-after (card->breakout card-after)]
+ (when-some [identifier->action (breakouts-->identifier->action breakout-before breakout-after)]
+ (let [dashcards (t2/select :model/DashboardCard :card_id (some :id [card-after card-before]))
+ updates (updates-for-dashcards identifier->action dashcards)]
+ ;; Beware. This can have negative impact on card update performance as queries are fired in sequence. I'm not
+ ;; aware of more reasonable way.
+ (when (seq updates)
+ (t2/with-transaction [_conn]
+ (doseq [[id update] updates]
+ (t2/update! :model/DashboardCard :id id update))))))))
+
(defn update-card!
"Update a Card. Metadata is fetched asynchronously. If it is ready before [[metadata-sync-wait-ms]] elapses it will be
included, otherwise the metadata will be saved to the database asynchronously."
@@ -808,7 +894,15 @@
:present #{:collection_id :collection_position :description :cache_ttl :archived_directly}
:non-nil #{:dataset_query :display :name :visualization_settings :archived
:enable_embedding :type :parameters :parameter_mappings :embedding_params
- :result_metadata :collection_preview :verified-result-metadata?})))
+ :result_metadata :collection_preview :verified-result-metadata?}))
+ ;; ok, now update dependent dashcard parameters
+ (try
+ (update-associated-parameters! card-before-update card-updates)
+ (catch Throwable e
+ (log/error "Update of dependent card parameters failed!")
+ (log/debug e
+ "`card-before-update`:" (pr-str card-before-update)
+ "`card-updates`:" (pr-str card-updates)))))
;; Fetch the updated Card from the DB
(let [card (t2/select-one Card :id (:id card-before-update))]
(delete-alerts-if-needed! :old-card card-before-update, :new-card card, :actor actor)
diff --git a/src/metabase/query_processor/pivot.clj b/src/metabase/query_processor/pivot.clj
index ecbca94d08165e47b158348344797add0bde2711..86c736a1968b097482e5a6c39200355c9bd42542 100644
--- a/src/metabase/query_processor/pivot.clj
+++ b/src/metabase/query_processor/pivot.clj
@@ -45,8 +45,15 @@
(mr/def ::num-breakouts ::lib.schema.common/int-greater-than-or-equal-to-zero)
(mr/def ::index ::lib.schema.common/int-greater-than-or-equal-to-zero)
-(mr/def ::pivot-rows [:sequential ::index])
-(mr/def ::pivot-cols [:sequential ::index])
+(mr/def ::pivot-rows [:sequential ::index])
+(mr/def ::pivot-cols [:sequential ::index])
+(mr/def ::pivot-measures [:sequential ::index])
+
+(mr/def ::pivot-opts [:maybe
+ [:map
+ [:pivot-rows {:optional true} [:maybe ::pivot-rows]]
+ [:pivot-cols {:optional true} [:maybe ::pivot-cols]]
+ [:pivot-measures {:optional true} [:maybe ::pivot-measures]]]])
(mu/defn- group-bitmask :- ::bitmask
"Come up with a display name given a combination of breakout `indexes` e.g.
@@ -173,9 +180,7 @@
(mu/defn- generate-queries :- [:sequential ::lib.schema/query]
"Generate the additional queries to perform a generic pivot table"
[query :- ::lib.schema/query
- {:keys [pivot-rows pivot-cols], :as _pivot-options} :- [:map
- [:pivot-rows {:optional true} [:maybe ::pivot-rows]]
- [:pivot-cols {:optional true} [:maybe ::pivot-cols]]]]
+ {:keys [pivot-rows pivot-cols], :as _pivot-options} :- ::pivot-opts]
(try
(let [all-breakouts (lib/breakouts query)
all-queries (for [breakout-indexes (u/prog1 (breakout-combinations (count all-breakouts) pivot-rows pivot-cols)
@@ -342,48 +347,45 @@
qp.pipeline/*reduce* (or reduce qp.pipeline/*reduce*)]
(qp/process-query first-query rff))))
-(mu/defn- pivot-options :- [:map
- [:pivot-rows [:maybe [:sequential [:int {:min 0}]]]]
- [:pivot-cols [:maybe [:sequential [:int {:min 0}]]]]]
+(mu/defn- pivot-options :- ::pivot-opts
"Given a pivot table query and a card ID, looks at the `pivot_table.column_split` key in the card's visualization
settings and generates pivot-rows and pivot-cols to use for generating subqueries."
[query :- [:map
[:database ::lib.schema.id/database]]
viz-settings :- [:maybe :map]]
- (let [column-split (:pivot_table.column_split viz-settings)
- column-split-rows (seq (:rows column-split))
- column-split-columns (seq (:columns column-split))
- index-in-breakouts (when (or column-split-rows
- column-split-columns)
- (let [metadata-provider (or (:lib/metadata query)
- (lib.metadata.jvm/application-database-metadata-provider (:database query)))
- mlv2-query (lib/query metadata-provider query)
- breakouts (into []
- (map-indexed (fn [i col]
- (cond-> col
- true (assoc ::i i)
- ;; if the col has a card-id, we swap the :lib/source to say source/card
- ;; this allows `lib/find-matching-column` to properly match a column that has a join-alias
- ;; but whose source is a model
- (contains? col :lib/card-id) (assoc :lib/source :source/card))))
- (lib/breakouts-metadata mlv2-query))]
- (fn [legacy-ref]
- (try
- (::i (lib.equality/find-column-for-legacy-ref
- mlv2-query
- -1
- legacy-ref
- breakouts))
- (catch Throwable e
- (log/errorf e "Error finding matching column for ref %s" (pr-str legacy-ref))
- nil)))))
-
- pivot-rows (when column-split-rows
- (into [] (keep index-in-breakouts) column-split-rows))
- pivot-cols (when column-split-columns
- (into [] (keep index-in-breakouts) column-split-columns))]
- {:pivot-rows pivot-rows
- :pivot-cols pivot-cols}))
+ (let [{:keys [rows columns values]} (:pivot_table.column_split viz-settings)
+ metadata-provider (or (:lib/metadata query)
+ (lib.metadata.jvm/application-database-metadata-provider (:database query)))
+ mlv2-query (lib/query metadata-provider query)
+ breakouts (into []
+ (map-indexed (fn [i col]
+ (cond-> col
+ true (assoc ::idx i)
+ ;; if the col has a card-id, we swap the :lib/source to say
+ ;; source/card this allows `lib/find-matching-column` to properly
+ ;; match a column that has a join-alias but whose source is a
+ ;; model
+ (contains? col :lib/card-id) (assoc :lib/source :source/card))))
+ (concat (lib/breakouts-metadata mlv2-query)
+ (lib/aggregations-metadata mlv2-query)))
+ index-in-breakouts (fn index-in-breakouts [legacy-ref]
+ (try
+ (::idx (lib.equality/find-column-for-legacy-ref
+ mlv2-query
+ -1
+ legacy-ref
+ breakouts))
+ (catch Throwable e
+ (log/errorf e "Error finding matching column for ref %s" (pr-str legacy-ref))
+ nil)))
+ process-refs (fn process-refs [refs]
+ (when (seq refs)
+ (into [] (keep index-in-breakouts) refs)))
+ pivot-opts {:pivot-rows (process-refs rows)
+ :pivot-cols (process-refs columns)
+ :pivot-measures (process-refs values)}]
+ (when (some some? (vals pivot-opts))
+ pivot-opts)))
(mu/defn- column-mapping-for-subquery :- ::pivot-column-mapping
[num-canonical-cols :- ::lib.schema.common/int-greater-than-or-equal-to-zero
@@ -529,10 +531,11 @@
(qp.setup/with-qp-setup [query query]
(let [rff (or rff qp.reducible/default-rff)
query (lib/query (qp.store/metadata-provider) query)
- pivot-options (or
- (not-empty (select-keys query [:pivot-rows :pivot-cols]))
- (pivot-options query (get-in query [:info :visualization-settings])))
- query (assoc-in query [:middleware :pivot-options] pivot-options)
- all-queries (generate-queries query pivot-options)
+ pivot-opts (or
+ (pivot-options query (get query :viz-settings))
+ (pivot-options query (get-in query [:info :visualization-settings]))
+ (not-empty (select-keys query [:pivot-rows :pivot-cols :pivot-measures])))
+ query (assoc-in query [:middleware :pivot-options] pivot-opts)
+ all-queries (generate-queries query pivot-opts)
column-mapping-fn (make-column-mapping-fn query)]
(process-multiple-queries all-queries rff column-mapping-fn))))))
diff --git a/src/metabase/query_processor/pivot/postprocess.clj b/src/metabase/query_processor/pivot/postprocess.clj
index 7b277a2700147dfd05ee577a4d80133c059c2c13..a70414848ed9fe041381cc4b3efacefd56743a56 100644
--- a/src/metabase/query_processor/pivot/postprocess.clj
+++ b/src/metabase/query_processor/pivot/postprocess.clj
@@ -62,10 +62,19 @@
(mu/defn add-pivot-measures :- ::pivot-spec
"Given a pivot-spec map without the `:pivot-measures` key, determine what key(s) the measures will be and assoc that value into `:pivot-measures`."
- [pivot-spec :- ::pivot-spec]
- (-> pivot-spec
- (assoc :pivot-measures (pivot-measures pivot-spec))
- (assoc :pivot-grouping (pivot-grouping-key (:column-titles pivot-spec)))))
+ [{measure-indices :pivot-measures :as pivot-spec} :- ::pivot-spec]
+ (let [pivot-grouping-key (pivot-grouping-key (:column-titles pivot-spec))]
+ (cond-> pivot-spec
+ ;; if pivot-measures don't already exist (from the pivot qp), we add them ourselves, assuming lowest ID -> highest ID sort order
+ (not (seq measure-indices)) (assoc :pivot-measures (pivot-measures pivot-spec))
+ ;; otherwise, we modify indices to skip over whatever the pivot-grouping idx is, so we pull the correct values per row
+ (seq measure-indices) (update :pivot-measures (fn [indices]
+ (mapv (fn [idx]
+ (if (>= idx pivot-grouping-key)
+ (inc idx)
+ idx))
+ indices)))
+ true (assoc :pivot-grouping pivot-grouping-key))))
(mu/defn add-totals-settings :- ::pivot-spec
"Given a pivot-spec map and `viz-settings`, add the `:row-totals?` and `:col-totals?` keys."
diff --git a/src/metabase/query_processor/streaming/csv.clj b/src/metabase/query_processor/streaming/csv.clj
index 99b2d0173e598b79779b91115482799c3cf34e35..ab1a6529d3d133928a66352d3688e92ff52bf9db 100644
--- a/src/metabase/query_processor/streaming/csv.clj
+++ b/src/metabase/query_processor/streaming/csv.clj
@@ -78,15 +78,14 @@
(begin! [_ {{:keys [ordered-cols results_timezone format-rows? pivot-export-options pivot?]
:or {format-rows? true
pivot? false}} :data} viz-settings]
- (let [opts (when (and pivot? pivot-export-options)
+ (let [col-names (vec (common/column-titles ordered-cols (::mb.viz/column-settings viz-settings) format-rows?))
+ opts (when (and pivot? pivot-export-options)
(-> (merge {:pivot-rows []
:pivot-cols []}
pivot-export-options)
- (assoc :column-titles (mapv :display_name ordered-cols))
+ (assoc :column-titles col-names)
(qp.pivot.postprocess/add-totals-settings viz-settings)
qp.pivot.postprocess/add-pivot-measures))
- ;; col-names are created later when exporting a pivot table, so only create them if there are no pivot options
- col-names (when-not opts (common/column-titles ordered-cols (::mb.viz/column-settings viz-settings) format-rows?))
pivot-grouping-key (qp.pivot.postprocess/pivot-grouping-key col-names)]
;; initialize the pivot-data
@@ -101,7 +100,7 @@
(mapv #(formatter/create-formatter results_timezone % viz-settings format-rows?) ordered-cols))
;; write the column names for non-pivot tables
- (when col-names
+ (when (not opts)
(let [header (m/remove-nth (or pivot-grouping-key (inc (count col-names))) col-names)]
(write-csv writer [header])
(.flush writer)))))
diff --git a/src/metabase/query_processor/streaming/json.clj b/src/metabase/query_processor/streaming/json.clj
index 2f851442d98bd16b36bb520a8d94796c6abe1a7f..05cf3de2c42422b0db0dd77dd377c846bfe4dace 100644
--- a/src/metabase/query_processor/streaming/json.clj
+++ b/src/metabase/query_processor/streaming/json.clj
@@ -41,8 +41,6 @@
(reify qp.si/StreamingResultsWriter
(begin! [_ {{:keys [ordered-cols results_timezone format-rows?]
:or {format-rows? true}} :data} viz-settings]
- ;; TODO -- wouldn't it make more sense if the JSON downloads used `:name` preferentially? Seeing how JSON is
- ;; probably going to be parsed programmatically
(let [cols (common/column-titles ordered-cols (::mb.viz/column-settings viz-settings) format-rows?)
pivot-grouping (qp.pivot.postprocess/pivot-grouping-key cols)]
(when pivot-grouping (vreset! pivot-grouping-idx pivot-grouping))
diff --git a/test/metabase/api/dashboard_test.clj b/test/metabase/api/dashboard_test.clj
index 318ae703120254a547736ed089ee7dc229c0d856..a8239f377496cca7e6d1881510db7754e9826aef 100644
--- a/test/metabase/api/dashboard_test.clj
+++ b/test/metabase/api/dashboard_test.clj
@@ -4864,3 +4864,55 @@
;; dashcards and parameters for each dashcard, linked to a single card. Following is the proof
;; of things working as described.
(is (= 1 @call-count))))))))
+
+;; Exception during scheduled (grouper) update of UserParameterValue is thrown. It is not relevant in context
+;; of tested functionality.
+;; TODO: Address the exception!
+(deftest dependent-dashcard-parameters-test
+ (mt/with-temp [:model/Card {card-id :id} {:name "c1"
+ :dataset_query (mt/mbql-query
+ orders
+ {:aggregation [[:count]]
+ :breakout
+ [!day.$created_at]})}
+ :model/Dashboard {dashboard-id :id} {:name "d1"
+ :parameters []}
+ :model/DashboardCard {dashcard-id :id} {:card_id card-id
+ :dashboard_id dashboard-id
+ :parameter_mappings []}]
+ (t2/update! :model/Dashboard :id dashboard-id {:parameters [{:name "TIME Gr"
+ :slug "tgr"
+ :id "30d7efb0"
+ :type :temporal-unit
+ :sectionId "temporal-unit"}]})
+ (t2/update! :model/DashboardCard :id dashcard-id {:parameter_mappings [{:parameter_id "30d7efb0"
+ :type :temporal-unit
+ :card_id card-id
+ :target [:dimension
+ (mt/$ids orders !day.$created_at)]}]})
+ (testing "Baseline"
+ (is (=? [["2016-04-01T00:00:00Z" 1]
+ ["2016-05-01T00:00:00Z" 19]]
+ (->> (mt/user-http-request
+ :crowberto :post 202
+ (format "dashboard/%d/dashcard/%d/card/%d/query" dashboard-id dashcard-id card-id)
+ {:parameters [{:id "30d7efb0"
+ :type "temporal-unit"
+ :value "month"
+ :target
+ [:dimension
+ (mt/$ids orders !day.$created_at)]}]})
+ mt/rows
+ (take 2)))))
+ (mt/user-http-request
+ :crowberto :put 200
+ (format "card/%d" card-id)
+ {:dataset_query (mt/mbql-query
+ orders
+ {:aggregation [[:count]]
+ :breakout
+ [!year.$created_at]})})
+ (testing "Mapping is adjusted to new target (#49202)"
+ (is (= (mt/$ids orders !year.$created_at)
+ (t2/select-one-fn #(get-in % [:parameter_mappings 0 :target 1])
+ :model/DashboardCard :id dashcard-id))))))
diff --git a/test/metabase/api/downloads_exports_test.clj b/test/metabase/api/downloads_exports_test.clj
index bd0f79f304ace8432fdeab550801648c41584f1a..5c1ff477535d0804d05c67166d1d16821a133eab 100644
--- a/test/metabase/api/downloads_exports_test.clj
+++ b/test/metabase/api/downloads_exports_test.clj
@@ -1061,32 +1061,108 @@
:type :native
:native {:query "SELECT 1234.567 as A"}}}]
(testing "CSV downloads respect the formatted/unformatted setting"
- (let [formatted-json-results (all-downloads card {:export-format :csv :format-rows true})
- unformatted-json-results (all-downloads card {:export-format :csv :format-rows false})]
+ (let [formatted-results (all-downloads card {:export-format :csv :format-rows true})
+ unformatted-results (all-downloads card {:export-format :csv :format-rows false})]
(is (= {:unsaved-card-download [["A"] ["1,234.57"]]
:card-download [["A"] ["1,234.57"]]
:public-question-download [["A"] ["1,234.57"]]
:dashcard-download [["A"] ["1,234.57"]]
:public-dashcard-download [["A"] ["1,234.57"]]}
- formatted-json-results))
+ formatted-results))
(is (= {:unsaved-card-download [["A"] ["1234.567"]]
:card-download [["A"] ["1234.567"]]
:public-question-download [["A"] ["1234.567"]]
:dashcard-download [["A"] ["1234.567"]]
:public-dashcard-download [["A"] ["1234.567"]]}
- unformatted-json-results))))
+ unformatted-results))))
(testing "JSON downloads respect the formatted/unformatted setting"
- (let [formatted-json-results (all-downloads card {:export-format :json :format-rows true})
- unformatted-json-results (all-downloads card {:export-format :json :format-rows false})]
+ (let [formatted-results (all-downloads card {:export-format :json :format-rows true})
+ unformatted-results (all-downloads card {:export-format :json :format-rows false})]
(is (= {:unsaved-card-download [["A"] ["1,234.57"]]
:card-download [["A"] ["1,234.57"]]
:public-question-download [["A"] ["1,234.57"]]
:dashcard-download [["A"] ["1,234.57"]]
:public-dashcard-download [["A"] ["1,234.57"]]}
- formatted-json-results))
+ formatted-results))
(is (= {:unsaved-card-download [["A"] [1234.567]]
:card-download [["A"] [1234.567]]
:public-question-download [["A"] [1234.567]]
:dashcard-download [["A"] [1234.567]]
:public-dashcard-download [["A"] [1234.567]]}
- unformatted-json-results))))))))
+ unformatted-results))))))))
+
+(deftest pivot-measures-order-test
+ (testing "A pivot download will use the user-configured measures order (#48442)."
+ (mt/dataset test-data
+ (mt/with-temp [:model/Card card {:display :pivot
+ :dataset_query {:database (mt/id)
+ :type :query
+ :query
+ {:source-table (mt/id :products)
+ :aggregation [[:count]
+ [:sum [:field (mt/id :products :price) {:base-type :type/Float}]]
+ [:avg [:field (mt/id :products :rating) {:base-type :type/Float}]]]
+ :breakout [[:field (mt/id :products :category) {:base-type :type/Text}]]}}
+ :visualization_settings {:pivot_table.column_split
+ {:rows [[:field (mt/id :products :category) {:base-type :type/Text}]]
+ :columns []
+ :values [[:aggregation 1]
+ [:aggregation 0]
+ [:aggregation 2]]}
+ :column_settings
+ {"[\"name\",\"count\"]" {:column_title "Count Renamed"}
+ "[\"name\",\"sum\"]" {:column_title "Sum Renamed"}
+ "[\"name\",\"avg\"]" {:column_title "Average Renamed"}}}}]
+ (let [expected-header ["Category" "Sum of Price" "Count" "Average of Rating"]
+ formatted-results (all-downloads card {:export-format :csv :format-rows false :pivot true})]
+ (is (= {:unsaved-card-download expected-header
+ :card-download expected-header
+ :public-question-download expected-header
+ :dashcard-download expected-header
+ :public-dashcard-download expected-header}
+ (update-vals formatted-results first))))
+ (testing "The column title changes are used when format-rows is true"
+ (let [expected-header ["Category" "Sum Renamed" "Count Renamed" "Average Renamed"]
+ formatted-results (all-downloads card {:export-format :csv :format-rows true :pivot true})]
+ (is (= {:unsaved-card-download expected-header
+ :card-download expected-header
+ :public-question-download expected-header
+ :dashcard-download expected-header
+ :public-dashcard-download expected-header}
+ (update-vals formatted-results first)))))))))
+
+(deftest pivot-rows-order-test
+ (testing "A pivot download will use the user-configured rows order."
+ (mt/dataset test-data
+ (mt/with-temp [:model/Card card {:display :pivot
+ :dataset_query {:database (mt/id)
+ :type :query
+ :query
+ {:source-table (mt/id :products)
+ :aggregation [[:count]]
+ :breakout [[:field (mt/id :products :category) {:base-type :type/Text}]
+ [:field (mt/id :products :created_at) {:base-type :type/DateTime :temporal-unit :year}]]}}
+ :visualization_settings {:pivot_table.column_split
+ {:rows [[:field (mt/id :products :created_at) {:base-type :type/DateTime :temporal-unit :year}]
+ [:field (mt/id :products :category) {:base-type :type/Text}]]
+ :columns []
+ :values [[:aggregation 0]]}
+ :column_settings
+ {"[\"name\",\"count\"]" {:column_title "Count Renamed"}}}}]
+ (let [expected-header ["Created At" "Category" "Count"]
+ formatted-results (all-downloads card {:export-format :csv :format-rows false :pivot true})]
+ (is (= {:unsaved-card-download expected-header
+ :card-download expected-header
+ :public-question-download expected-header
+ :dashcard-download expected-header
+ :public-dashcard-download expected-header}
+ (update-vals formatted-results first))))
+ (testing "The column title changes are used when format-rows is true"
+ (let [expected-header ["Created At" "Category" "Count Renamed"]
+ formatted-results (all-downloads card {:export-format :csv :format-rows true :pivot true})]
+ (is (= {:unsaved-card-download expected-header
+ :card-download expected-header
+ :public-question-download expected-header
+ :dashcard-download expected-header
+ :public-dashcard-download expected-header}
+ (update-vals formatted-results first)))))))))
diff --git a/test/metabase/api/pivots.clj b/test/metabase/api/pivots.clj
index 359228bafdae263cf4391e6f79c9d44504803e87..5b2541e49ada0bc1e44271a2cd408e26093037ff 100644
--- a/test/metabase/api/pivots.clj
+++ b/test/metabase/api/pivots.clj
@@ -5,9 +5,7 @@
(defn applicable-drivers
"Drivers that these pivot table tests should run on"
[]
- (disj (mt/normal-drivers-with-feature :expressions :left-join)
- ;; mongodb doesn't support foreign keys required by this test
- :mongo
+ (disj (mt/normal-drivers-with-feature :expressions :left-join :metadata/key-constraints)
;; Disable on Redshift due to OutOfMemory issue (see #18834)
:redshift))
diff --git a/test/metabase/events/view_log_test.clj b/test/metabase/events/view_log_test.clj
index 56448181b8d24d29a833350f27bc44ed49b1766b..a377b8e0e0ccb15ec112f158db1da770a0a351cd 100644
--- a/test/metabase/events/view_log_test.clj
+++ b/test/metabase/events/view_log_test.clj
@@ -61,7 +61,12 @@
(latest-view (mt/user->id :crowberto) (u/id coll))))))))
(deftest update-view-dashboard-timestamp-test
- (let [now (t/offset-date-time)
+ ;; the DB might save `last_used_at` with a different level of precision than the JVM does, on some machines
+ ;; `offset-date-time` returns nanosecond precision (9 decimal places) but `last_viewed_at` is coming back with
+ ;; microsecond precision (6 decimal places). We don't care about such a small difference, just strip it off of the
+ ;; times we're comparing.
+ (let [now (-> (t/offset-date-time)
+ (.withNano 0))
one-hour-ago (t/minus now (t/hours 1))
two-hours-ago (t/minus now (t/hours 2))]
(testing "update with multiple dashboards of the same ids will set timestamp to the latest"
diff --git a/test/metabase/models/card_test.clj b/test/metabase/models/card_test.clj
index dacc5199bb63779aaff74bea1bbd4230b8f47276..eee2dc0903cc050907e5ffceaeb0b337ab7375e4 100644
--- a/test/metabase/models/card_test.clj
+++ b/test/metabase/models/card_test.clj
@@ -1037,3 +1037,55 @@
(mt/with-test-user :crowberto
(is (false? (mi/can-read? card)))
(is (false? (mi/can-write? card)))))))))
+
+(deftest breakouts-->identifier->action-fn-test
+ (are [b1 b2 expected--identifier->action] (= expected--identifier->action
+ (#'card/breakouts-->identifier->action b1 b2))
+ [[:field 10 {:temporal-unit :day}]]
+ nil
+ nil
+
+ [[:expression "x" {:temporal-unit :day}]]
+ nil
+ nil
+
+ [[:expression "x" {:temporal-unit :day}]]
+ [[:expression "x" {:temporal-unit :month}]]
+ {[:expression "x"] [:update [:expression "x" {:temporal-unit :month}]]}
+
+ [[:expression "x" {:temporal-unit :day}]]
+ [[:expression "x" {:temporal-unit :day}]]
+ nil
+
+ [[:field 10 {:temporal-unit :day}] [:expression "x" {:temporal-unit :day}]]
+ [[:expression "x" {:temporal-unit :day}] [:field 10 {:temporal-unit :month}]]
+ {[:field 10] [:update [:field 10 {:temporal-unit :month}]]}
+
+ [[:field 10 {:temporal-unit :year}] [:field 10 {:temporal-unit :day-of-week}]]
+ [[:field 10 {:temporal-unit :year}]]
+ nil))
+
+(deftest update-for-dashcard-fn-test
+ (are [indetifier->action quasi-dashcards expected-quasi-dashcards]
+ (= expected-quasi-dashcards
+ (#'card/updates-for-dashcards indetifier->action quasi-dashcards))
+
+ {[:field 10] [:update [:field 10 {:temporal-unit :month}]]}
+ [{:parameter_mappings []}]
+ nil
+
+ {[:field 10] [:update [:field 10 {:temporal-unit :month}]]}
+ [{:id 1 :parameter_mappings [{:target [:dimension [:field 10 nil]]}]}]
+ [[1 {:parameter_mappings [{:target [:dimension [:field 10 {:temporal-unit :month}]]}]}]]
+
+ {[:field 10] [:noop]}
+ [{:id 1 :parameter_mappings [{:target [:dimension [:field 10 nil]]}]}]
+ nil
+
+ {[:field 10] [:update [:field 10 {:temporal-unit :month}]]}
+ [{:id 1 :parameter_mappings [{:target [:dimension [:field 10 {:temporal-unit :year}]]}
+ {:target [:dimension [:field 33 {:temporal-unit :month}]]}
+ {:target [:dimension [:field 10 {:temporal-unit :day}]]}]}]
+ [[1 {:parameter_mappings [{:target [:dimension [:field 10 {:temporal-unit :month}]]}
+ {:target [:dimension [:field 33 {:temporal-unit :month}]]}
+ {:target [:dimension [:field 10 {:temporal-unit :month}]]}]}]]))
diff --git a/test/metabase/query_processor/pivot_test.clj b/test/metabase/query_processor/pivot_test.clj
index ca255bae82cd94174abd26a2a4a3796531325cb6..e2bf1cb002457cddd5fa1d93acbdb616b254ce3e 100644
--- a/test/metabase/query_processor/pivot_test.clj
+++ b/test/metabase/query_processor/pivot_test.clj
@@ -191,7 +191,7 @@
(testing "`pivot-options` correctly generates pivot-rows and pivot-cols from a card's viz settings"
(let [query (api.pivots/pivot-query false)
viz-settings (:visualization_settings (api.pivots/pivot-card))
- pivot-options {:pivot-rows [1 0], :pivot-cols [2]}]
+ pivot-options {:pivot-rows [1 0], :pivot-cols [2] :pivot-measures nil}]
(is (= pivot-options
(#'qp.pivot/pivot-options query viz-settings)))
(are [num-breakouts expected] (= expected
@@ -217,7 +217,7 @@
:columns [[:field "RATING" {:base-type :type/Integer}]]}})
[:pivot_table.column_split])
pivot-options (#'qp.pivot/pivot-options query viz-settings)]
- (is (= {:pivot-rows [], :pivot-cols []}
+ (is (= {:pivot-rows [], :pivot-cols [] :pivot-measures nil}
pivot-options))
(is (= [[0 1] [1] [0] []]
(#'qp.pivot/breakout-combinations 2 (:pivot-rows pivot-options) (:pivot-cols pivot-options)))))))
@@ -245,7 +245,7 @@
{:rows [$category]
:columns [$created_at]}})
pivot-options (#'qp.pivot/pivot-options query viz-settings)]
- (is (= {:pivot-rows [0], :pivot-cols [1]}
+ (is (= {:pivot-rows [0], :pivot-cols [1] :pivot-measures nil}
pivot-options))
(is (= [[0 1] [1] [0] []]
(#'qp.pivot/breakout-combinations 2 (:pivot-rows pivot-options) (:pivot-cols pivot-options))))