diff --git a/frontend/src/metabase/actions/containers/ActionCreatorModal/ActionCreatorModal.unit.spec.tsx b/frontend/src/metabase/actions/containers/ActionCreatorModal/ActionCreatorModal.unit.spec.tsx
index 098b3cafd4e3fb665d5e3935992e35e44313f8b0..5717b84fce97c454fed39a8cf88123c62cc03c74 100644
--- a/frontend/src/metabase/actions/containers/ActionCreatorModal/ActionCreatorModal.unit.spec.tsx
+++ b/frontend/src/metabase/actions/containers/ActionCreatorModal/ActionCreatorModal.unit.spec.tsx
@@ -2,12 +2,7 @@ import userEvent from "@testing-library/user-event";
import fetchMock from "fetch-mock";
import { Route } from "react-router";
-import {
- renderWithProviders,
- screen,
- waitFor,
- waitForElementToBeRemoved,
-} from "__support__/ui";
+import { renderWithProviders, screen, waitFor } from "__support__/ui";
import {
setupCardsEndpoints,
setupDatabasesEndpoints,
@@ -76,6 +71,10 @@ async function setup({
},
);
+ await waitFor(() => {
+ expect(screen.queryByTestId("loading-spinner")).not.toBeInTheDocument();
+ });
+
return { history: checkNotNull(history) };
}
@@ -97,10 +96,6 @@ describe("actions > containers > ActionCreatorModal", () => {
const initialRoute = `/model/${MODEL.id}/detail/actions/${ACTION_NOT_FOUND_ID}`;
const { history } = await setup({ initialRoute, action: null });
- await waitForElementToBeRemoved(() =>
- screen.queryAllByTestId("loading-spinner"),
- );
-
expect(await screen.findByTestId("mock-model-detail")).toBeInTheDocument();
expect(screen.queryByTestId("action-creator")).not.toBeInTheDocument();
expect(history.getCurrentLocation().pathname).toBe(
@@ -113,10 +108,6 @@ describe("actions > containers > ActionCreatorModal", () => {
const initialRoute = `/model/${MODEL.id}/detail/actions/${action.id}`;
const { history } = await setup({ initialRoute, action });
- await waitForElementToBeRemoved(() =>
- screen.queryAllByTestId("loading-spinner"),
- );
-
expect(await screen.findByTestId("mock-model-detail")).toBeInTheDocument();
expect(screen.queryByTestId("action-creator")).not.toBeInTheDocument();
expect(history.getCurrentLocation().pathname).toBe(
diff --git a/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.tsx b/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.tsx
index 4b839058708ee9c47b6cd46aead39480ff65abc0..ddc17b97f9eff44e9855ec83cc549dec5d51df34 100644
--- a/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.tsx
+++ b/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.tsx
@@ -1,21 +1,25 @@
import type { ComponentType } from "react";
import { useState } from "react";
import { connect } from "react-redux";
+import { push } from "react-router-redux";
+import type { Route } from "react-router";
import { t } from "ttag";
import _ from "underscore";
import { updateIn } from "icepick";
import { useMount } from "react-use";
-import type { Location } from "history";
+import type { Location, LocationDescriptor } from "history";
import title from "metabase/hoc/Title";
import Breadcrumbs from "metabase/components/Breadcrumbs";
import Sidebar from "metabase/admin/databases/components/DatabaseEditApp/Sidebar/Sidebar";
import { getUserIsAdmin } from "metabase/selectors/user";
+import { useCallbackEffect } from "metabase/hooks/use-callback-effect";
import { getSetting } from "metabase/selectors/settings";
+import { LeaveConfirmationModal } from "metabase/components/LeaveConfirmationModal";
import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper";
import { DatabaseForm } from "metabase/databases/components/DatabaseForm";
import ErrorBoundary from "metabase/ErrorBoundary";
@@ -26,7 +30,6 @@ import type {
DatabaseId,
} from "metabase-types/api";
import type { State } from "metabase-types/store";
-import useBeforeUnload from "metabase/hooks/use-before-unload";
import Database from "metabase-lib/metadata/Database";
import { getEditingDatabase, getInitializeError } from "../selectors";
@@ -73,6 +76,8 @@ interface DatabaseEditAppProps {
isAdmin: boolean;
isModelPersistenceEnabled: boolean;
initializeError?: DatabaseEditErrorType;
+ route: Route;
+ onChangeLocation: (location: LocationDescriptor) => void;
}
const mapStateToProps = (state: State) => {
@@ -97,6 +102,7 @@ const mapDispatchToProps = {
discardSavedFieldValues,
deleteDatabase,
selectEngine,
+ onChangeLocation: push,
};
type DatabaseEditErrorType = {
@@ -124,6 +130,8 @@ function DatabaseEditApp(props: DatabaseEditAppProps) {
initializeDatabase,
params,
saveDatabase,
+ route,
+ onChangeLocation,
} = props;
const editingExistingDatabase = database?.id != null;
@@ -131,7 +139,11 @@ function DatabaseEditApp(props: DatabaseEditAppProps) {
const [isDirty, setIsDirty] = useState(false);
- useBeforeUnload(isDirty);
+ /**
+ * Navigation is scheduled so that LeaveConfirmationModal's isEnabled
+ * prop has a chance to re-compute on re-render
+ */
+ const [isCallbackScheduled, scheduleCallback] = useCallbackEffect();
useMount(async () => {
await reset();
@@ -145,6 +157,12 @@ function DatabaseEditApp(props: DatabaseEditAppProps) {
const handleSubmit = async (database: DatabaseData) => {
try {
await saveDatabase(database);
+
+ if (addingNewDatabase) {
+ scheduleCallback(() => {
+ onChangeLocation("/admin/databases?created=true");
+ });
+ }
} catch (error) {
throw getSubmitError(error as DatabaseEditErrorType);
}
@@ -195,6 +213,11 @@ function DatabaseEditApp(props: DatabaseEditAppProps) {
/>
)}
</DatabaseEditMain>
+
+ <LeaveConfirmationModal
+ isEnabled={isDirty && !isCallbackScheduled}
+ route={route}
+ />
</DatabaseEditRoot>
);
}
diff --git a/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.unit.spec.js b/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.unit.spec.js
deleted file mode 100644
index 89bd00a0806bfc9ba78edd3fa110bebeadf3ed8c..0000000000000000000000000000000000000000
--- a/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.unit.spec.js
+++ /dev/null
@@ -1,150 +0,0 @@
-import { IndexRoute, Route } from "react-router";
-
-import userEvent from "@testing-library/user-event";
-import {
- renderWithProviders,
- screen,
- waitForElementToBeRemoved,
- waitFor,
-} from "__support__/ui";
-import { setupEnterpriseTest } from "__support__/enterprise";
-import { mockSettings } from "__support__/settings";
-
-import {
- createMockDatabase,
- createMockTokenFeatures,
-} from "metabase-types/api/mocks";
-import { setupDatabaseEndpoints } from "__support__/server-mocks";
-
-import { BEFORE_UNLOAD_UNSAVED_MESSAGE } from "metabase/hooks/use-before-unload";
-import { callMockEvent } from "__support__/events";
-import DatabaseEditApp from "./DatabaseEditApp";
-
-const ENGINES_MOCK = {
- H2: {
- "details-fields": [
- { "display-name": "Connection String", name: "db", required: true },
- { name: "advanced-options", type: "section", default: true },
- ],
- "driver-name": "H2",
- "superseded-by": null,
- },
- sqlite: {
- "details-fields": [
- { "display-name": "Filename", name: "db", required: true },
- { name: "advanced-options", type: "section", default: true },
- ],
- "driver-name": "SQLite",
- "superseded-by": null,
- },
-};
-
-async function setup({ cachingEnabled = false, databaseIdParam = "" } = {}) {
- const mockEventListener = jest.spyOn(window, "addEventListener");
-
- const settings = mockSettings({
- engines: ENGINES_MOCK,
- "token-features": createMockTokenFeatures({
- cache_granular_controls: true,
- }),
- "enable-query-caching": cachingEnabled,
- });
-
- renderWithProviders(
- <Route path="/">
- <IndexRoute component={DatabaseEditApp} />
- <Route path=":databaseId" component={DatabaseEditApp} />
- </Route>,
- {
- withRouter: true,
- initialRoute: `/${databaseIdParam}`,
- storeInitialState: {
- settings,
- },
- },
- );
-
- await waitForElementToBeRemoved(() => screen.queryByText("Loading..."));
-
- return { mockEventListener };
-}
-
-describe("DatabaseEditApp", () => {
- describe("Database connections", () => {
- afterEach(() => {
- jest.restoreAllMocks();
- });
-
- it("should have save changes button disabled", async () => {
- setupDatabaseEndpoints(createMockDatabase());
- await setup({ databaseIdParam: "1" });
-
- const saveButton = await screen.findByRole("button", {
- name: /save changes/i,
- });
- expect(saveButton).toBeDisabled();
-
- const connectionField = await screen.findByLabelText("Connection String");
- userEvent.type(connectionField, "Test Connection");
-
- await waitFor(() => {
- expect(saveButton).toBeEnabled();
- });
- });
-
- it("should trigger beforeunload event when database connection is edited", async () => {
- const { mockEventListener } = await setup();
-
- const databaseForm = await screen.findByLabelText("Display name");
-
- userEvent.type(databaseForm, "Test database");
- const mockEvent = await waitFor(() => {
- return callMockEvent(mockEventListener, "beforeunload");
- });
- expect(mockEvent.preventDefault).toHaveBeenCalled();
- expect(mockEvent.returnValue).toBe(BEFORE_UNLOAD_UNSAVED_MESSAGE);
- });
-
- it("should not trigger beforeunload event when database connection is unchanged", async () => {
- const { mockEventListener } = await setup();
- const mockEvent = callMockEvent(mockEventListener, "beforeunload");
-
- expect(mockEvent.preventDefault).not.toHaveBeenCalled();
- expect(mockEvent.returnValue).toBe(undefined);
- });
- });
-
- describe("Cache TTL field", () => {
- describe("OSS", () => {
- it("is invisible", async () => {
- await setup({ cachingEnabled: true });
-
- expect(
- screen.queryByText("Default result cache duration"),
- ).not.toBeInTheDocument();
- });
- });
-
- describe("EE", () => {
- beforeEach(() => {
- setupEnterpriseTest();
- });
-
- it("is visible", async () => {
- await setup({ cachingEnabled: true });
-
- expect(
- screen.getByText("Default result cache duration"),
- ).toBeInTheDocument();
- });
-
- it("is invisible when caching disabled", async () => {
- await setup({ cachingEnabled: false });
-
- expect(
- screen.queryByText("Default result cache duration"),
- ).not.toBeInTheDocument();
- });
- });
- });
-});
diff --git a/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.unit.spec.tsx b/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.unit.spec.tsx
new file mode 100644
index 0000000000000000000000000000000000000000..3d34462abeabee138d130a88462529d3669bffaa
--- /dev/null
+++ b/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.unit.spec.tsx
@@ -0,0 +1,264 @@
+import { IndexRoute, Route } from "react-router";
+import userEvent from "@testing-library/user-event";
+
+import {
+ renderWithProviders,
+ screen,
+ waitFor,
+ waitForElementToBeRemoved,
+} from "__support__/ui";
+import { setupEnterpriseTest } from "__support__/enterprise";
+import { mockSettings } from "__support__/settings";
+
+import type { Engine } from "metabase-types/api";
+import {
+ createMockDatabase,
+ createMockEngineSource,
+ createMockTokenFeatures,
+} from "metabase-types/api/mocks";
+import {
+ setupDatabaseEndpoints,
+ setupDatabasesEndpoints,
+} from "__support__/server-mocks";
+
+import { checkNotNull } from "metabase/core/utils/types";
+import { BEFORE_UNLOAD_UNSAVED_MESSAGE } from "metabase/hooks/use-before-unload";
+import { callMockEvent } from "__support__/events";
+import DatabaseEditApp from "./DatabaseEditApp";
+
+const ENGINES_MOCK: Record<string, Engine> = {
+ H2: {
+ "details-fields": [
+ { "display-name": "Connection String", name: "db", required: true },
+ { name: "advanced-options", type: "section", default: true },
+ ],
+ "driver-name": "H2",
+ "superseded-by": null,
+ source: createMockEngineSource(),
+ },
+ sqlite: {
+ "details-fields": [
+ { "display-name": "Filename", name: "db", required: true },
+ { name: "advanced-options", type: "section", default: true },
+ ],
+ "driver-name": "SQLite",
+ "superseded-by": null,
+ source: createMockEngineSource(),
+ },
+};
+
+const MockComponent = () => <div />;
+
+interface SetupOpts {
+ cachingEnabled?: boolean;
+ databaseIdParam?: string;
+ initialRoute?: string;
+}
+
+async function setup({
+ cachingEnabled = false,
+ databaseIdParam = "",
+ initialRoute = `/${databaseIdParam}`,
+}: SetupOpts = {}) {
+ const mockEventListener = jest.spyOn(window, "addEventListener");
+
+ setupDatabasesEndpoints([]);
+
+ const settings = mockSettings({
+ engines: ENGINES_MOCK,
+ "token-features": createMockTokenFeatures({
+ cache_granular_controls: true,
+ }),
+ "enable-query-caching": cachingEnabled,
+ });
+
+ const { history } = renderWithProviders(
+ <Route path="/">
+ <Route path="/home" component={MockComponent} />
+ <Route path="/admin/databases" component={MockComponent} />
+ <IndexRoute component={DatabaseEditApp} />
+ <Route path=":databaseId" component={DatabaseEditApp} />
+ </Route>,
+ {
+ withRouter: true,
+ initialRoute,
+ storeInitialState: {
+ settings,
+ },
+ },
+ );
+
+ await waitFor(() => {
+ expect(screen.queryByText("Loading...")).not.toBeInTheDocument();
+ });
+
+ return {
+ history: checkNotNull(history),
+ mockEventListener,
+ };
+}
+
+describe("DatabaseEditApp", () => {
+ describe("Database connections", () => {
+ afterEach(() => {
+ jest.restoreAllMocks();
+ });
+
+ it("should have save changes button disabled", async () => {
+ setupDatabaseEndpoints(createMockDatabase());
+ await setup({ databaseIdParam: "1" });
+
+ const saveButton = await screen.findByRole("button", {
+ name: /save changes/i,
+ });
+ expect(saveButton).toBeDisabled();
+
+ const connectionField = await screen.findByLabelText("Connection String");
+ userEvent.type(connectionField, "Test Connection");
+
+ await waitFor(() => {
+ expect(saveButton).toBeEnabled();
+ });
+ });
+
+ it("should trigger beforeunload event when database connection is edited", async () => {
+ const { mockEventListener } = await setup();
+
+ const displayNameInput = await screen.findByLabelText("Display name");
+
+ userEvent.type(displayNameInput, "Test database");
+ const mockEvent = await waitFor(() => {
+ return callMockEvent(mockEventListener, "beforeunload");
+ });
+ expect(mockEvent.preventDefault).toHaveBeenCalled();
+ expect(mockEvent.returnValue).toBe(BEFORE_UNLOAD_UNSAVED_MESSAGE);
+ });
+
+ it("should not trigger beforeunload event when database connection is unchanged", async () => {
+ const { mockEventListener } = await setup();
+ const mockEvent = callMockEvent(mockEventListener, "beforeunload");
+
+ expect(mockEvent.preventDefault).not.toHaveBeenCalled();
+ expect(mockEvent.returnValue).toBe(undefined);
+ });
+
+ it("does not show custom warning modal when leaving with no changes via SPA navigation", async () => {
+ const { history } = await setup({ initialRoute: "/home" });
+
+ history.push("/");
+
+ await waitForElementToBeRemoved(() =>
+ screen.queryAllByTestId("loading-spinner"),
+ );
+
+ const displayNameInput = await screen.findByLabelText("Display name");
+ userEvent.type(displayNameInput, "ab");
+ userEvent.type(displayNameInput, "{backspace}{backspace}");
+
+ history.goBack();
+
+ expect(
+ screen.queryByText("Changes were not saved"),
+ ).not.toBeInTheDocument();
+ expect(
+ screen.queryByText(
+ "Navigating away from here will cause you to lose any changes you have made.",
+ ),
+ ).not.toBeInTheDocument();
+ });
+
+ it("shows custom warning modal when leaving with unsaved changes via SPA navigation", async () => {
+ const { history } = await setup({ initialRoute: "/home" });
+
+ history.push("/");
+
+ await waitForElementToBeRemoved(() =>
+ screen.queryAllByTestId("loading-spinner"),
+ );
+ const displayNameInput = await screen.findByLabelText("Display name");
+ userEvent.type(displayNameInput, "Test database");
+
+ history.goBack();
+
+ expect(screen.getByText("Changes were not saved")).toBeInTheDocument();
+ expect(
+ screen.getByText(
+ "Navigating away from here will cause you to lose any changes you have made.",
+ ),
+ ).toBeInTheDocument();
+ });
+
+ it("does not show custom warning modal after creating new database connection", async () => {
+ const { history } = await setup({ initialRoute: "/home" });
+
+ history.push("/");
+
+ await waitForElementToBeRemoved(() =>
+ screen.queryAllByTestId("loading-spinner"),
+ );
+
+ const displayNameInput = await screen.findByLabelText("Display name");
+ userEvent.type(displayNameInput, "Test database");
+ const connectionStringInput = await screen.findByLabelText(
+ "Connection String",
+ );
+ userEvent.type(
+ connectionStringInput,
+ "file:/sample-database.db;USER=GUEST;PASSWORD=guest",
+ );
+
+ userEvent.click(await screen.findByText("Save"));
+
+ await waitFor(() => {
+ expect(history.getCurrentLocation().pathname).toEqual(
+ "/admin/databases",
+ );
+ });
+
+ expect(history.getCurrentLocation().search).toEqual("?created=true");
+
+ expect(
+ screen.queryByText("Changes were not saved"),
+ ).not.toBeInTheDocument();
+ expect(
+ screen.queryByText(
+ "Navigating away from here will cause you to lose any changes you have made.",
+ ),
+ ).not.toBeInTheDocument();
+ });
+ });
+
+ describe("Cache TTL field", () => {
+ describe("OSS", () => {
+ it("is invisible", async () => {
+ await setup({ cachingEnabled: true });
+
+ expect(
+ screen.queryByText("Default result cache duration"),
+ ).not.toBeInTheDocument();
+ });
+ });
+
+ describe("EE", () => {
+ beforeEach(() => {
+ setupEnterpriseTest();
+ });
+
+ it("is visible", async () => {
+ await setup({ cachingEnabled: true });
+
+ expect(
+ screen.getByText("Default result cache duration"),
+ ).toBeInTheDocument();
+ });
+
+ it("is invisible when caching disabled", async () => {
+ await setup({ cachingEnabled: false });
+
+ expect(
+ screen.queryByText("Default result cache duration"),
+ ).not.toBeInTheDocument();
+ });
+ });
+ });
+});
diff --git a/frontend/src/metabase/admin/databases/database.js b/frontend/src/metabase/admin/databases/database.js
index 9b050bc9c92b76938cf25ffce2c64d8acdeefb47..452312b297c6c7e6c995d12eabcd1e58054d25ce 100644
--- a/frontend/src/metabase/admin/databases/database.js
+++ b/frontend/src/metabase/admin/databases/database.js
@@ -166,7 +166,6 @@ export const createDatabase = function (database) {
);
dispatch({ type: CREATE_DATABASE });
- dispatch(push("/admin/databases?created=true"));
} catch (error) {
console.error("error creating a database", error);
MetabaseAnalytics.trackStructEvent(
diff --git a/frontend/src/metabase/query_builder/containers/QueryBuilder.unit.spec.tsx b/frontend/src/metabase/query_builder/containers/QueryBuilder.unit.spec.tsx
index 5e91a045a7273594028d144a027e5bb0ecbdda57..351e4bcb809d5144cf7b6ca4a44d6050b10f6de6 100644
--- a/frontend/src/metabase/query_builder/containers/QueryBuilder.unit.spec.tsx
+++ b/frontend/src/metabase/query_builder/containers/QueryBuilder.unit.spec.tsx
@@ -243,11 +243,9 @@ const setup = async ({
},
);
- if (initialRoute !== "/home") {
- await waitForElementToBeRemoved(() =>
- screen.queryByTestId("loading-spinner"),
- );
- }
+ await waitFor(() => {
+ expect(screen.queryByTestId("loading-spinner")).not.toBeInTheDocument();
+ });
return {
history: checkNotNull(history),
diff --git a/frontend/test/__support__/server-mocks/database.ts b/frontend/test/__support__/server-mocks/database.ts
index a3a6bffdb24a1f233b7a7d2376d5f2a924e4fb23..5cd419798b237c3686c8be5a1a1b722fad6702bf 100644
--- a/frontend/test/__support__/server-mocks/database.ts
+++ b/frontend/test/__support__/server-mocks/database.ts
@@ -39,6 +39,10 @@ export function setupDatabasesEndpoints(
hasSavedQuestions ? [...dbs, SAVED_QUESTIONS_DATABASE] : dbs,
);
fetchMock.get({ url: "path:/api/database", overwriteRoutes: false }, dbs);
+ fetchMock.post("path:/api/database", async url => {
+ const lastCall = fetchMock.lastCall(url);
+ return await lastCall?.request?.json();
+ });
dbs.forEach(db => setupDatabaseEndpoints(db));
}