From 344dba38df3fcc06775609f8d2c97583e66ecebb Mon Sep 17 00:00:00 2001
From: "Mahatthana (Kelvin) Nomsawadi" <me@bboykelvin.dev>
Date: Wed, 16 Oct 2024 14:22:14 +0700
Subject: [PATCH] Do not open sidesheets on SDK (#48630)
* Don't open sidesheet on SDK dashboards
* Add E2E test
* Fix `className` not passed
* Fix unit tests
* Fix E2E tests
---
e2e/support/helpers/e2e-search-helpers.js | 2 +-
.../editable-dashboard.cy.spec.js | 65 +++++++++++++++++++
.../LastEditInfoLabel/LastEditInfoLabel.tsx | 30 ++++++---
.../LastEditInfoLabel.unit.spec.js | 35 ++++++++--
.../DashboardHeader/DashboardHeader.tsx | 9 ++-
.../DashboardHeader/DashboardHeaderView.tsx | 2 +-
.../InfoText/InfoText.unit.spec.tsx | 26 ++++----
7 files changed, 138 insertions(+), 31 deletions(-)
create mode 100644 e2e/test/scenarios/embedding-sdk/editable-dashboard.cy.spec.js
diff --git a/e2e/support/helpers/e2e-search-helpers.js b/e2e/support/helpers/e2e-search-helpers.js
index b185d6073a3..2c8f685a04e 100644
--- a/e2e/support/helpers/e2e-search-helpers.js
+++ b/e2e/support/helpers/e2e-search-helpers.js
@@ -48,7 +48,7 @@ export function expectSearchResultContent({
});
}
if (expectedSearchResult.timestamp) {
- cy.findByTestId("revision-history-button").findByText(
+ cy.findByTestId("revision-history-text").findByText(
expectedSearchResult.timestamp,
);
}
diff --git a/e2e/test/scenarios/embedding-sdk/editable-dashboard.cy.spec.js b/e2e/test/scenarios/embedding-sdk/editable-dashboard.cy.spec.js
new file mode 100644
index 00000000000..eae3b03515a
--- /dev/null
+++ b/e2e/test/scenarios/embedding-sdk/editable-dashboard.cy.spec.js
@@ -0,0 +1,65 @@
+import {
+ restore,
+ setTokenFeatures,
+ visitFullAppEmbeddingUrl,
+} from "e2e/support/helpers";
+import {
+ EMBEDDING_SDK_STORY_HOST,
+ describeSDK,
+} from "e2e/support/helpers/e2e-embedding-sdk-helpers";
+import {
+ JWT_SHARED_SECRET,
+ setupJwt,
+} from "e2e/support/helpers/e2e-jwt-helpers";
+
+describeSDK("scenarios > embedding-sdk > editable-dashboard", () => {
+ beforeEach(() => {
+ restore();
+ cy.signInAsAdmin();
+ setTokenFeatures("all");
+ setupJwt();
+ cy.request("PUT", "/api/setting", {
+ "enable-embedding-sdk": true,
+ });
+
+ cy.createDashboard(
+ {
+ name: "Embedding SDK Test Dashboard",
+ },
+ { wrapId: true },
+ );
+
+ cy.signOut();
+
+ cy.intercept("GET", "/api/dashboard/*").as("getDashboard");
+ cy.intercept("GET", "/api/user/current").as("getUser");
+ cy.intercept("POST", "/api/dashboard/*/dashcard/*/card/*/query").as(
+ "dashcardQuery",
+ );
+ });
+
+ it("Should not open sidesheet when clicking last edit info (metabase#48354)", () => {
+ cy.get("@dashboardId").then(dashboardId => {
+ visitFullAppEmbeddingUrl({
+ url: EMBEDDING_SDK_STORY_HOST,
+ qs: {
+ id: "embeddingsdk-editabledashboard--default",
+ viewMode: "story",
+ },
+ onBeforeLoad: window => {
+ window.JWT_SHARED_SECRET = JWT_SHARED_SECRET;
+ window.METABASE_INSTANCE_URL = Cypress.config().baseUrl;
+ window.DASHBOARD_ID = dashboardId;
+ },
+ });
+ });
+
+ cy.get("#metabase-sdk-root")
+ .findByText("Edited a few seconds ago by Bobby Tables")
+ .click()
+ .should("be.visible");
+ cy.findByRole("heading", { name: "Info" }).should("not.exist");
+ cy.findByRole("tab", { name: "Overview" }).should("not.exist");
+ cy.findByRole("tab", { name: "History" }).should("not.exist");
+ });
+});
diff --git a/frontend/src/metabase/components/LastEditInfoLabel/LastEditInfoLabel.tsx b/frontend/src/metabase/components/LastEditInfoLabel/LastEditInfoLabel.tsx
index 7097cb434c1..c141b62e206 100644
--- a/frontend/src/metabase/components/LastEditInfoLabel/LastEditInfoLabel.tsx
+++ b/frontend/src/metabase/components/LastEditInfoLabel/LastEditInfoLabel.tsx
@@ -9,7 +9,7 @@ import type { NamedUser } from "metabase/lib/user";
import { getFullName } from "metabase/lib/user";
import { getUser } from "metabase/selectors/user";
import type { TooltipProps } from "metabase/ui";
-import { Tooltip } from "metabase/ui";
+import { Text, Tooltip } from "metabase/ui";
import type { User } from "metabase-types/api";
export type ItemWithLastEditInfo = {
@@ -93,14 +93,26 @@ function LastEditInfoLabel({
return (
<Tooltip disabled={!timeLabel} {...tooltipProps}>
- <TextButton
- size="small"
- className={className}
- onClick={onClick}
- data-testid="revision-history-button"
- >
- {children}
- </TextButton>
+ {onClick ? (
+ <TextButton
+ className={className}
+ size="small"
+ onClick={onClick}
+ data-testid="revision-history-button"
+ >
+ {children}
+ </TextButton>
+ ) : (
+ <Text
+ className={className}
+ size="sm"
+ fw="bold"
+ c="var(--mb-color-text-secondary)"
+ data-testid="revision-history-text"
+ >
+ {children}
+ </Text>
+ )}
</Tooltip>
);
}
diff --git a/frontend/src/metabase/components/LastEditInfoLabel/LastEditInfoLabel.unit.spec.js b/frontend/src/metabase/components/LastEditInfoLabel/LastEditInfoLabel.unit.spec.js
index bb7c8afacb7..e79450efe8f 100644
--- a/frontend/src/metabase/components/LastEditInfoLabel/LastEditInfoLabel.unit.spec.js
+++ b/frontend/src/metabase/components/LastEditInfoLabel/LastEditInfoLabel.unit.spec.js
@@ -20,7 +20,10 @@ describe("LastEditInfoLabel", () => {
email: "john@metabase.test",
});
- function setup({ isLastEditedByCurrentUser = false } = {}) {
+ function setup({
+ isLastEditedByCurrentUser = false,
+ onClick = jest.fn(),
+ } = {}) {
const testItem = {
"last-edit-info": {
...TEST_USER,
@@ -32,11 +35,14 @@ describe("LastEditInfoLabel", () => {
? TEST_USER
: { ...TEST_USER, id: TEST_USER.id + 1 };
- return renderWithProviders(<LastEditInfoLabel item={testItem} />, {
- storeInitialState: {
- currentUser,
+ return renderWithProviders(
+ <LastEditInfoLabel item={testItem} onClick={onClick} />,
+ {
+ storeInitialState: {
+ currentUser,
+ },
},
- });
+ );
}
const A_FEW_SECONDS_AGO = moment().add(5, "seconds");
@@ -93,4 +99,23 @@ describe("LastEditInfoLabel", () => {
new RegExp(`Edited .* by you`),
);
});
+
+ it("should not be clickable when `onClick` is not passed (currently only in SDK context) (metabase#48354)", () => {
+ setup({ onClick: null });
+ expect(screen.getByText(/Edited .* by .*/)).toBeInTheDocument();
+ expect(
+ screen.queryByRole("button", {
+ name: /Edited .* by .*/,
+ }),
+ ).not.toBeInTheDocument();
+ });
+
+ it("should be clickable when `onClick` is passed (metabase#48354)", () => {
+ setup();
+ expect(
+ screen.getByRole("button", {
+ name: /Edited .* by .*/,
+ }),
+ ).toBeInTheDocument();
+ });
});
diff --git a/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeader.tsx b/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeader.tsx
index 769d0132516..4a44085a2f9 100644
--- a/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeader.tsx
+++ b/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeader.tsx
@@ -22,6 +22,7 @@ import type {
DashboardNightModeControls,
DashboardRefreshPeriodControls,
} from "metabase/dashboard/types";
+import { isEmbeddingSdk } from "metabase/env";
import { useDispatch, useSelector } from "metabase/lib/redux";
import { fetchPulseFormInput } from "metabase/pulse/actions";
import { getSetting } from "metabase/selectors/settings";
@@ -153,8 +154,12 @@ export const DashboardHeaderInner = ({
: "",
)}
editingButtons={editingButtons}
- onLastEditInfoClick={() =>
- dispatch(setSidebar({ name: SIDEBAR_NAME.info }))
+ onLastEditInfoClick={
+ isEmbeddingSdk
+ ? undefined
+ : () => {
+ dispatch(setSidebar({ name: SIDEBAR_NAME.info }));
+ }
}
refreshPeriod={refreshPeriod}
onRefreshPeriodChange={onRefreshPeriodChange}
diff --git a/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeaderView.tsx b/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeaderView.tsx
index 72319b5a1f5..47d8e4e33e5 100644
--- a/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeaderView.tsx
+++ b/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeaderView.tsx
@@ -55,7 +55,7 @@ type DashboardHeaderViewProps = {
collection: Collection;
isBadgeVisible: boolean;
isLastEditInfoVisible: boolean;
- onLastEditInfoClick: () => void;
+ onLastEditInfoClick?: () => void;
} & DashboardFullscreenControls &
DashboardRefreshPeriodControls &
DashboardNightModeControls;
diff --git a/frontend/src/metabase/search/components/InfoText/InfoText.unit.spec.tsx b/frontend/src/metabase/search/components/InfoText/InfoText.unit.spec.tsx
index a79c0fa9899..fd0401493fc 100644
--- a/frontend/src/metabase/search/components/InfoText/InfoText.unit.spec.tsx
+++ b/frontend/src/metabase/search/components/InfoText/InfoText.unit.spec.tsx
@@ -131,7 +131,7 @@ describe("InfoText", () => {
`/collection/${MOCK_COLLECTION.id}-collection-name`,
);
- expect(screen.getByTestId("revision-history-button")).toHaveTextContent(
+ expect(screen.getByTestId("revision-history-text")).toHaveTextContent(
`Updated ${LAST_EDITED_DURATION}`,
);
});
@@ -143,7 +143,7 @@ describe("InfoText", () => {
const collectionElement = screen.getByText("Collection");
expect(collectionElement).toBeInTheDocument();
- expect(screen.getByTestId("revision-history-button")).toHaveTextContent(
+ expect(screen.getByTestId("revision-history-text")).toHaveTextContent(
`Updated ${LAST_EDITED_DURATION}`,
);
});
@@ -153,7 +153,7 @@ describe("InfoText", () => {
model: "database",
});
expect(screen.getByText("Database")).toBeInTheDocument();
- expect(screen.getByTestId("revision-history-button")).toHaveTextContent(
+ expect(screen.getByTestId("revision-history-text")).toHaveTextContent(
`Updated ${LAST_EDITED_DURATION}`,
);
});
@@ -169,7 +169,7 @@ describe("InfoText", () => {
`/question#?db=${MOCK_DATABASE.id}&table=${MOCK_TABLE.id}`,
);
- expect(screen.getByTestId("revision-history-button")).toHaveTextContent(
+ expect(screen.getByTestId("revision-history-text")).toHaveTextContent(
`Updated ${LAST_EDITED_DURATION}`,
);
});
@@ -186,7 +186,7 @@ describe("InfoText", () => {
`/browse/databases/${MOCK_DATABASE.id}-database-name`,
);
- expect(screen.getByTestId("revision-history-button")).toHaveTextContent(
+ expect(screen.getByTestId("revision-history-text")).toHaveTextContent(
`Updated ${LAST_EDITED_DURATION}`,
);
});
@@ -203,7 +203,7 @@ describe("InfoText", () => {
`/collection/${MOCK_COLLECTION.id}-collection-name`,
);
- expect(screen.getByTestId("revision-history-button")).toHaveTextContent(
+ expect(screen.getByTestId("revision-history-text")).toHaveTextContent(
`Updated ${LAST_EDITED_DURATION}`,
);
});
@@ -213,7 +213,7 @@ describe("InfoText", () => {
it("should show last_edited_by when available", async () => {
await setup();
- expect(screen.getByTestId("revision-history-button")).toHaveTextContent(
+ expect(screen.getByTestId("revision-history-text")).toHaveTextContent(
`Updated ${LAST_EDITED_DURATION} by ${MOCK_OTHER_USER.common_name}`,
);
});
@@ -227,7 +227,7 @@ describe("InfoText", () => {
},
});
- expect(screen.getByTestId("revision-history-button")).toHaveTextContent(
+ expect(screen.getByTestId("revision-history-text")).toHaveTextContent(
`Created by you`,
);
});
@@ -241,7 +241,7 @@ describe("InfoText", () => {
},
});
- expect(screen.getByTestId("revision-history-button")).toHaveTextContent(
+ expect(screen.getByTestId("revision-history-text")).toHaveTextContent(
`Created ${CREATED_AT_DURATION}`,
);
});
@@ -254,7 +254,7 @@ describe("InfoText", () => {
},
});
- expect(screen.getByTestId("revision-history-button")).toHaveTextContent(
+ expect(screen.getByTestId("revision-history-text")).toHaveTextContent(
`Updated ${LAST_EDITED_DURATION}`,
);
});
@@ -268,7 +268,7 @@ describe("InfoText", () => {
},
});
- expect(screen.getByTestId("revision-history-button")).toHaveTextContent(
+ expect(screen.getByTestId("revision-history-text")).toHaveTextContent(
`Updated ${LAST_EDITED_DURATION}`,
);
});
@@ -282,7 +282,7 @@ describe("InfoText", () => {
},
});
- expect(screen.getByTestId("revision-history-button")).toHaveTextContent(
+ expect(screen.getByTestId("revision-history-text")).toHaveTextContent(
`Created ${CREATED_AT_DURATION}`,
);
});
@@ -299,7 +299,7 @@ describe("InfoText", () => {
expect(screen.queryByText("•")).not.toBeInTheDocument();
expect(
- screen.queryByTestId("revision-history-button"),
+ screen.queryByTestId("revision-history-text"),
).not.toBeInTheDocument();
});
});
--
GitLab