From d29867e0845ebbb296fc10878471c5f367e67b64 Mon Sep 17 00:00:00 2001
From: Kamil Mielnik <kamil@kamilmielnik.com>
Date: Fri, 6 Oct 2023 17:33:47 +0700
Subject: [PATCH] Add custom warning message when leaving changing Data Model
(segments and metrics) (#34398)
* Rename component to match the filename
* Fix typo
* Replace useBeforeUnload with LeaveConfirmationModal in SegmentForm
* Replace useBeforeUnload with LeaveConfirmationModal in MetricForm
* Update setup of MetricForm tests to allow testing navigation
* Update setup of SegmentForm tests to allow testing navigation
* Use proper history in MetricForm
* Refactor SegmentForm test setup to be consistent with the one of MetricForm
* Make MetricForm and SegmentForm tests consistent
* Add tests for not showing custom warning
* Add tests for showing custom warning when leaving with unsaved changes
* Extract FORM_URL
* Remove redundant asyncs and awaits
* Rename MetricForm.unit.spec.tsx & SegmentForm.unit.spec.tsx to MetricApp.unit.spec.tsx & SegmentApp.unit.spec.tsx and move them near the components they're testing
* Improve a11y
* Add missing endpoint mocks
* Improve selectors thanks to proper a11y
* Add tests for not showing warning modal when saving changes
* Convert CreateMetricForm to a functional component
* Convert CreateSegmentForm to a functional component
* Convert UpdateMetricFormInner to a functional component
* Convert UpdateSegmentFormInner to a functional component
* Disable LeaveConfirmationModal on scheduled navigation
* Setup metrics & segments POST endpoints
* Properly mock metric and segment creation
* Lift LeaveConfirmationModal from SegmentForm to SegmentApp
* Lift LeaveConfirmationModal from MetricForm to MetricApp
* Sort props
* Get rid of useCallbackEffect usage in MetricApp
* Get rid of useCallbackEffect usage in SegmentApp
---
.../src/metabase-types/api/mocks/table.ts | 2 +
.../components/FormLabel/FormLabel.tsx | 12 +-
.../components/MetricForm/MetricForm.tsx | 14 +-
.../MetricForm/MetricForm.unit.spec.tsx | 57 -------
.../components/SegmentForm/SegmentForm.tsx | 14 +-
.../SegmentForm/SegmentForm.unit.spec.tsx | 60 -------
.../admin/datamodel/containers/MetricApp.jsx | 114 ++++++++-----
.../containers/MetricApp.unit.spec.tsx | 157 ++++++++++++++++++
.../admin/datamodel/containers/SegmentApp.jsx | 114 ++++++++-----
.../containers/SegmentApp.unit.spec.tsx | 157 ++++++++++++++++++
.../StatusListing/StatusListing.tsx | 4 +-
.../test/__support__/server-mocks/metric.ts | 7 +-
.../test/__support__/server-mocks/segment.ts | 7 +-
13 files changed, 517 insertions(+), 202 deletions(-)
delete mode 100644 frontend/src/metabase/admin/datamodel/components/MetricForm/MetricForm.unit.spec.tsx
delete mode 100644 frontend/src/metabase/admin/datamodel/components/SegmentForm/SegmentForm.unit.spec.tsx
create mode 100644 frontend/src/metabase/admin/datamodel/containers/MetricApp.unit.spec.tsx
create mode 100644 frontend/src/metabase/admin/datamodel/containers/SegmentApp.unit.spec.tsx
diff --git a/frontend/src/metabase-types/api/mocks/table.ts b/frontend/src/metabase-types/api/mocks/table.ts
index 8ce4f5e5aad..16e59f6b579 100644
--- a/frontend/src/metabase-types/api/mocks/table.ts
+++ b/frontend/src/metabase-types/api/mocks/table.ts
@@ -12,6 +12,8 @@ export const createMockTable = (opts?: Partial<Table>): Table => {
visibility_type: null,
field_order: "database",
initial_sync_status: "complete",
+ metrics: [],
+ segments: [],
...opts,
};
};
diff --git a/frontend/src/metabase/admin/datamodel/components/FormLabel/FormLabel.tsx b/frontend/src/metabase/admin/datamodel/components/FormLabel/FormLabel.tsx
index 9c7e06a333f..f87d47949dc 100644
--- a/frontend/src/metabase/admin/datamodel/components/FormLabel/FormLabel.tsx
+++ b/frontend/src/metabase/admin/datamodel/components/FormLabel/FormLabel.tsx
@@ -1,4 +1,9 @@
-import type { HTMLAttributes, ReactNode, Ref } from "react";
+import type {
+ HTMLAttributes,
+ LabelHTMLAttributes,
+ ReactNode,
+ Ref,
+} from "react";
import { forwardRef } from "react";
import {
FormLabelContent,
@@ -8,19 +13,20 @@ import {
} from "./FormLabel.styled";
interface FormLabelProps extends HTMLAttributes<HTMLDivElement> {
+ htmlFor?: LabelHTMLAttributes<HTMLLabelElement>["htmlFor"];
title?: string;
description?: string;
children?: ReactNode;
}
const FormLabel = forwardRef(function FormLabel(
- { title, description, children, ...props }: FormLabelProps,
+ { htmlFor, title, description, children, ...props }: FormLabelProps,
ref: Ref<HTMLDivElement>,
) {
return (
<FormLabelRoot {...props} ref={ref}>
<FormLabelContent>
- {title && <FormLabelTitle>{title}</FormLabelTitle>}
+ {title && <FormLabelTitle htmlFor={htmlFor}>{title}</FormLabelTitle>}
{description && (
<FormLabelDescription>{description}</FormLabelDescription>
)}
diff --git a/frontend/src/metabase/admin/datamodel/components/MetricForm/MetricForm.tsx b/frontend/src/metabase/admin/datamodel/components/MetricForm/MetricForm.tsx
index 598bbd96621..98e6b47365d 100644
--- a/frontend/src/metabase/admin/datamodel/components/MetricForm/MetricForm.tsx
+++ b/frontend/src/metabase/admin/datamodel/components/MetricForm/MetricForm.tsx
@@ -1,3 +1,4 @@
+import { useEffect } from "react";
import { Link } from "react-router";
import { useFormik } from "formik";
import type { FieldInputProps } from "formik";
@@ -6,7 +7,6 @@ import { formatValue } from "metabase/lib/formatting";
import Button from "metabase/core/components/Button";
import FieldSet from "metabase/components/FieldSet";
import type { Metric, StructuredQuery } from "metabase-types/api";
-import useBeforeUnload from "metabase/hooks/use-before-unload";
import * as Q from "metabase-lib/queries/utils/query";
import FormInput from "../FormInput";
import FormLabel from "../FormLabel";
@@ -31,6 +31,7 @@ export interface MetricFormProps {
metric?: Metric;
previewSummary?: string;
updatePreviewSummary: (previewSummary: string) => void;
+ onIsDirtyChange: (isDirty: boolean) => void;
onSubmit: (values: Partial<Metric>) => void;
}
@@ -38,6 +39,7 @@ const MetricForm = ({
metric,
previewSummary,
updatePreviewSummary,
+ onIsDirtyChange,
onSubmit,
}: MetricFormProps): JSX.Element => {
const isNew = metric == null;
@@ -50,7 +52,9 @@ const MetricForm = ({
onSubmit,
});
- useBeforeUnload(dirty);
+ useEffect(() => {
+ onIsDirtyChange(dirty);
+ }, [dirty, onIsDirtyChange]);
return (
<FormRoot onSubmit={handleSubmit}>
@@ -73,33 +77,39 @@ const MetricForm = ({
</FormLabel>
<FormBodyContent>
<FormLabel
+ htmlFor="name"
title={t`Name Your Metric`}
description={t`Give your metric a name to help others find it.`}
>
<FormInput
{...getFieldProps("name")}
{...getFieldMeta("name")}
+ id="name"
placeholder={t`Something descriptive but not too long`}
/>
</FormLabel>
<FormLabel
+ htmlFor="description"
title={t`Describe Your Metric`}
description={t`Give your metric a description to help others understand what it's about.`}
>
<FormTextArea
{...getFieldProps("description")}
{...getFieldMeta("description")}
+ id="description"
placeholder={t`This is a good place to be more specific about less obvious metric rules`}
/>
</FormLabel>
{!isNew && (
<FieldSet legend={t`Reason For Changes`} noPadding={false}>
<FormLabel
+ htmlFor="revision_message"
description={t`Leave a note to explain what changes you made and why they were required.`}
>
<FormTextArea
{...getFieldProps("revision_message")}
{...getFieldMeta("revision_message")}
+ id="revision_message"
placeholder={t`This will show up in the revision history for this metric to help everyone remember why things changed`}
/>
</FormLabel>
diff --git a/frontend/src/metabase/admin/datamodel/components/MetricForm/MetricForm.unit.spec.tsx b/frontend/src/metabase/admin/datamodel/components/MetricForm/MetricForm.unit.spec.tsx
deleted file mode 100644
index ed1a83d4f15..00000000000
--- a/frontend/src/metabase/admin/datamodel/components/MetricForm/MetricForm.unit.spec.tsx
+++ /dev/null
@@ -1,57 +0,0 @@
-import userEvent from "@testing-library/user-event";
-import { renderWithProviders, screen, waitFor } from "__support__/ui";
-import MetricApp from "metabase/admin/datamodel/containers/MetricApp";
-import { Route } from "metabase/hoc/Title";
-import { callMockEvent } from "__support__/events";
-import { BEFORE_UNLOAD_UNSAVED_MESSAGE } from "metabase/hooks/use-before-unload";
-import {
- setupDatabasesEndpoints,
- setupSearchEndpoints,
-} from "__support__/server-mocks";
-import { createMockDatabase } from "metabase-types/api/mocks";
-
-const setup = () => {
- setupDatabasesEndpoints([createMockDatabase()]);
- setupSearchEndpoints([]);
- renderWithProviders(
- <Route path="/admin/datamodel/metric/create" component={MetricApp} />,
- {
- initialRoute: "/admin/datamodel/metric/create",
- withRouter: true,
- },
- );
-
- const mockEventListener = jest.spyOn(window, "addEventListener");
-
- return { mockEventListener };
-};
-
-describe("MetricForm", () => {
- afterEach(() => {
- jest.restoreAllMocks();
- });
-
- it("should have beforeunload event when user makes edits to a metric", async () => {
- const { mockEventListener } = setup();
-
- const descriptionInput = screen.getByPlaceholderText(
- "Something descriptive but not too long",
- );
- userEvent.type(descriptionInput, "01189998819991197253");
-
- const mockEvent = await waitFor(() => {
- return callMockEvent(mockEventListener, "beforeunload");
- });
- expect(mockEvent.preventDefault).toHaveBeenCalled();
- expect(mockEvent.returnValue).toBe(BEFORE_UNLOAD_UNSAVED_MESSAGE);
- });
-
- it("should not have an beforeunload event when metric is unedited", async () => {
- const { mockEventListener } = setup();
-
- const mockEvent = callMockEvent(mockEventListener, "beforeunload");
-
- expect(mockEvent.preventDefault).not.toHaveBeenCalled();
- expect(mockEvent.returnValue).toBe(undefined);
- });
-});
diff --git a/frontend/src/metabase/admin/datamodel/components/SegmentForm/SegmentForm.tsx b/frontend/src/metabase/admin/datamodel/components/SegmentForm/SegmentForm.tsx
index 259b781ab71..2a975752441 100644
--- a/frontend/src/metabase/admin/datamodel/components/SegmentForm/SegmentForm.tsx
+++ b/frontend/src/metabase/admin/datamodel/components/SegmentForm/SegmentForm.tsx
@@ -1,3 +1,4 @@
+import { useEffect } from "react";
import { Link } from "react-router";
import { useFormik } from "formik";
import type { FieldInputProps } from "formik";
@@ -6,7 +7,6 @@ import { formatValue } from "metabase/lib/formatting";
import Button from "metabase/core/components/Button/Button";
import FieldSet from "metabase/components/FieldSet";
import type { Segment, StructuredQuery } from "metabase-types/api";
-import useBeforeUnload from "metabase/hooks/use-before-unload";
import * as Q from "metabase-lib/queries/utils/query";
import FormInput from "../FormInput";
import FormLabel from "../FormLabel";
@@ -30,6 +30,7 @@ export interface SegmentFormProps {
segment?: Segment;
previewSummary?: string;
updatePreviewSummary: (previewSummary: string) => void;
+ onIsDirtyChange: (isDirty: boolean) => void;
onSubmit: (values: Partial<Segment>) => void;
}
@@ -37,6 +38,7 @@ const SegmentForm = ({
segment,
previewSummary,
updatePreviewSummary,
+ onIsDirtyChange,
onSubmit,
}: SegmentFormProps): JSX.Element => {
const isNew = segment == null;
@@ -49,7 +51,9 @@ const SegmentForm = ({
onSubmit,
});
- useBeforeUnload(dirty);
+ useEffect(() => {
+ onIsDirtyChange(dirty);
+ }, [dirty, onIsDirtyChange]);
return (
<FormRoot onSubmit={handleSubmit}>
@@ -72,33 +76,39 @@ const SegmentForm = ({
</FormLabel>
<FormBodyContent>
<FormLabel
+ htmlFor="name"
title={t`Name Your Segment`}
description={t`Give your segment a name to help others find it.`}
>
<FormInput
{...getFieldProps("name")}
{...getFieldMeta("name")}
+ id="name"
placeholder={t`Something descriptive but not too long`}
/>
</FormLabel>
<FormLabel
+ htmlFor="description"
title={t`Describe Your Segment`}
description={t`Give your segment a description to help others understand what it's about.`}
>
<FormTextArea
{...getFieldProps("description")}
{...getFieldMeta("description")}
+ id="description"
placeholder={t`This is a good place to be more specific about less obvious segment rules`}
/>
</FormLabel>
{!isNew && (
<FieldSet legend={t`Reason For Changes`} noPadding={false}>
<FormLabel
+ htmlFor="revision_message"
description={t`Leave a note to explain what changes you made and why they were required.`}
>
<FormTextArea
{...getFieldProps("revision_message")}
{...getFieldMeta("revision_message")}
+ id="revision_message"
placeholder={t`This will show up in the revision history for this segment to help everyone remember why things changed`}
/>
</FormLabel>
diff --git a/frontend/src/metabase/admin/datamodel/components/SegmentForm/SegmentForm.unit.spec.tsx b/frontend/src/metabase/admin/datamodel/components/SegmentForm/SegmentForm.unit.spec.tsx
deleted file mode 100644
index 323995f53dd..00000000000
--- a/frontend/src/metabase/admin/datamodel/components/SegmentForm/SegmentForm.unit.spec.tsx
+++ /dev/null
@@ -1,60 +0,0 @@
-import userEvent from "@testing-library/user-event";
-import { renderWithProviders, screen, waitFor } from "__support__/ui";
-import { Route } from "metabase/hoc/Title";
-import { callMockEvent } from "__support__/events";
-import { BEFORE_UNLOAD_UNSAVED_MESSAGE } from "metabase/hooks/use-before-unload";
-import SegmentApp from "metabase/admin/datamodel/containers/SegmentApp";
-import {
- setupDatabasesEndpoints,
- setupSearchEndpoints,
-} from "__support__/server-mocks";
-import { createSampleDatabase } from "metabase-types/api/mocks/presets/sample_database";
-
-const setup = () => {
- renderWithProviders(
- <Route path="/admin/datamodel/segment/create" component={SegmentApp} />,
- {
- initialRoute: "/admin/datamodel/segment/create",
- withRouter: true,
- },
- );
-
- const mockEventListener = jest.spyOn(window, "addEventListener");
-
- return { mockEventListener };
-};
-
-describe("SegmentForm", () => {
- beforeEach(() => {
- setupDatabasesEndpoints([createSampleDatabase()]);
- setupSearchEndpoints([]);
- });
-
- afterEach(() => {
- jest.restoreAllMocks();
- });
-
- it("should have beforeunload event when user makes edits to a segment", async () => {
- const { mockEventListener } = setup();
-
- const descriptionInput = screen.getByPlaceholderText(
- "Something descriptive but not too long",
- );
- userEvent.type(descriptionInput, "01189998819991197253");
-
- const mockEvent = await waitFor(() => {
- return callMockEvent(mockEventListener, "beforeunload");
- });
- expect(mockEvent.preventDefault).toHaveBeenCalled();
- expect(mockEvent.returnValue).toBe(BEFORE_UNLOAD_UNSAVED_MESSAGE);
- });
-
- it("should not have an beforeunload event when metric is segment", async () => {
- const { mockEventListener } = setup();
-
- const mockEvent = callMockEvent(mockEventListener, "beforeunload");
-
- expect(mockEvent.preventDefault).not.toHaveBeenCalled();
- expect(mockEvent.returnValue).toBe(undefined);
- });
-});
diff --git a/frontend/src/metabase/admin/datamodel/containers/MetricApp.jsx b/frontend/src/metabase/admin/datamodel/containers/MetricApp.jsx
index 195f1c2b6ce..c8a778038bf 100644
--- a/frontend/src/metabase/admin/datamodel/containers/MetricApp.jsx
+++ b/frontend/src/metabase/admin/datamodel/containers/MetricApp.jsx
@@ -1,10 +1,11 @@
/* eslint-disable react/prop-types */
-import { Component } from "react";
+import { useCallback, useState } from "react";
import { connect } from "react-redux";
import { push } from "react-router-redux";
import * as MetabaseAnalytics from "metabase/lib/analytics";
import Metrics from "metabase/entities/metrics";
+import { LeaveConfirmationModal } from "metabase/components/LeaveConfirmationModal";
import { updatePreviewSummary } from "../datamodel";
import { getPreviewSummary } from "../selectors";
@@ -21,52 +22,91 @@ const mapStateToProps = (state, props) => ({
previewSummary: getPreviewSummary(state),
});
-class UpdateMetricFormInner extends Component {
- onSubmit = async metric => {
- await this.props.updateMetric(metric);
- MetabaseAnalytics.trackStructEvent("Data Model", "Metric Updated");
- this.props.onChangeLocation(`/admin/datamodel/metrics`);
- };
+const UpdateMetricFormInner = ({
+ route,
+ metric,
+ updateMetric,
+ onChangeLocation,
+ ...props
+}) => {
+ const [isDirty, setIsDirty] = useState(false);
- render() {
- const { metric, ...props } = this.props;
- return (
+ const handleSubmit = useCallback(
+ async metric => {
+ setIsDirty(false);
+
+ try {
+ await updateMetric(metric);
+ MetabaseAnalytics.trackStructEvent("Data Model", "Metric Updated");
+ onChangeLocation("/admin/datamodel/metrics");
+ } catch (error) {
+ setIsDirty(isDirty);
+ }
+ },
+ [updateMetric, isDirty, onChangeLocation],
+ );
+
+ return (
+ <>
<MetricForm
{...props}
metric={metric.getPlainObject()}
- onSubmit={this.onSubmit}
+ onIsDirtyChange={setIsDirty}
+ onSubmit={handleSubmit}
/>
- );
- }
-}
+ <LeaveConfirmationModal isEnabled={isDirty} route={route} />
+ </>
+ );
+};
const UpdateMetricForm = Metrics.load({
id: (state, props) => parseInt(props.params.id),
})(UpdateMetricFormInner);
-class CreateMetricForm extends Component {
- onSubmit = async metric => {
- await this.props.createMetric({
- ...metric,
- table_id: metric.definition["source-table"],
- });
- MetabaseAnalytics.trackStructEvent("Data Model", "Metric Updated");
- this.props.onChangeLocation(`/admin/datamodel/metrics`);
- };
-
- render() {
- return <MetricForm {...this.props} onSubmit={this.onSubmit} />;
- }
-}
-
-class MetricApp extends Component {
- render() {
- return this.props.params.id ? (
- <UpdateMetricForm {...this.props} />
- ) : (
- <CreateMetricForm {...this.props} />
- );
+const CreateMetricForm = ({
+ route,
+ createMetric,
+ onChangeLocation,
+ ...props
+}) => {
+ const [isDirty, setIsDirty] = useState(false);
+
+ const handleSubmit = useCallback(
+ async metric => {
+ setIsDirty(false);
+
+ try {
+ await createMetric({
+ ...metric,
+ table_id: metric.definition["source-table"],
+ });
+ MetabaseAnalytics.trackStructEvent("Data Model", "Metric Updated");
+ onChangeLocation("/admin/datamodel/metrics");
+ } catch (error) {
+ setIsDirty(isDirty);
+ }
+ },
+ [createMetric, isDirty, onChangeLocation],
+ );
+
+ return (
+ <>
+ <MetricForm
+ {...props}
+ onIsDirtyChange={setIsDirty}
+ onSubmit={handleSubmit}
+ />
+ <LeaveConfirmationModal isEnabled={isDirty} route={route} />
+ </>
+ );
+};
+
+const MetricApp = props => {
+ if (props.params.id) {
+ return <UpdateMetricForm {...props} />;
}
-}
+
+ return <CreateMetricForm {...props} />;
+};
export default connect(mapStateToProps, mapDispatchToProps)(MetricApp);
diff --git a/frontend/src/metabase/admin/datamodel/containers/MetricApp.unit.spec.tsx b/frontend/src/metabase/admin/datamodel/containers/MetricApp.unit.spec.tsx
new file mode 100644
index 00000000000..c52a6b593d6
--- /dev/null
+++ b/frontend/src/metabase/admin/datamodel/containers/MetricApp.unit.spec.tsx
@@ -0,0 +1,157 @@
+import userEvent from "@testing-library/user-event";
+
+import { callMockEvent } from "__support__/events";
+import {
+ setupCardDataset,
+ setupDatabasesEndpoints,
+ setupMetricsEndpoints,
+ setupSearchEndpoints,
+} from "__support__/server-mocks";
+import { renderWithProviders, screen, waitFor } from "__support__/ui";
+import { createSampleDatabase } from "metabase-types/api/mocks/presets";
+import { checkNotNull } from "metabase/core/utils/types";
+import { Route } from "metabase/hoc/Title";
+import { BEFORE_UNLOAD_UNSAVED_MESSAGE } from "metabase/hooks/use-before-unload";
+
+import MetricApp from "./MetricApp";
+
+const TestHome = () => <div />;
+
+const METRICS_URL = "/admin/datamodel/metrics";
+const FORM_URL = "/admin/datamodel/metric/create";
+
+interface SetupOpts {
+ initialRoute?: string;
+}
+
+const setup = ({ initialRoute = FORM_URL }: SetupOpts = {}) => {
+ setupDatabasesEndpoints([createSampleDatabase()]);
+ setupSearchEndpoints([]);
+ setupCardDataset();
+ setupMetricsEndpoints([]);
+
+ const { history } = renderWithProviders(
+ <>
+ <Route path="/" component={TestHome} />
+ <Route path={METRICS_URL} component={TestHome} />
+ <Route path={FORM_URL} component={MetricApp} />
+ </>,
+ {
+ initialRoute,
+ withRouter: true,
+ },
+ );
+
+ const mockEventListener = jest.spyOn(window, "addEventListener");
+
+ return {
+ history: checkNotNull(history),
+ mockEventListener,
+ };
+};
+
+describe("MetricApp", () => {
+ afterEach(() => {
+ jest.restoreAllMocks();
+ });
+
+ it("should have beforeunload event when user makes edits to a metric", async () => {
+ const { mockEventListener } = setup();
+
+ userEvent.type(screen.getByLabelText("Name Your Metric"), "Name");
+
+ const mockEvent = await waitFor(() => {
+ return callMockEvent(mockEventListener, "beforeunload");
+ });
+ expect(mockEvent.preventDefault).toHaveBeenCalled();
+ expect(mockEvent.returnValue).toBe(BEFORE_UNLOAD_UNSAVED_MESSAGE);
+ });
+
+ it("should not have an beforeunload event when metric is unedited", () => {
+ const { mockEventListener } = 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", () => {
+ const { history } = setup({ initialRoute: "/" });
+
+ history.push(FORM_URL);
+
+ 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", () => {
+ const { history } = setup({ initialRoute: "/" });
+
+ history.push(FORM_URL);
+
+ userEvent.type(screen.getByLabelText("Name Your Metric"), "Name");
+
+ 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 when saving changes", async () => {
+ const { history } = setup();
+
+ userEvent.click(screen.getByText("Select a table"));
+
+ await waitFor(() => {
+ expect(screen.queryByTestId("loading-spinner")).not.toBeInTheDocument();
+ });
+
+ userEvent.click(screen.getByText("Orders"));
+
+ await waitFor(() => {
+ expect(screen.queryByTestId("loading-spinner")).not.toBeInTheDocument();
+ });
+
+ userEvent.click(screen.getByText("Add filters to narrow your answer"));
+ userEvent.click(screen.getByText("ID"));
+ userEvent.type(screen.getByPlaceholderText("Enter an ID"), "1");
+ userEvent.click(screen.getByText("Add filter"));
+ userEvent.type(screen.getByLabelText("Name Your Metric"), "Name");
+ userEvent.type(
+ screen.getByLabelText("Describe Your Metric"),
+ "Description",
+ );
+
+ await waitFor(() => {
+ expect(screen.getByText("Save changes")).toBeEnabled();
+ });
+
+ userEvent.click(screen.getByText("Save changes"));
+
+ await waitFor(() => {
+ expect(history.getCurrentLocation().pathname).toBe(METRICS_URL);
+ });
+
+ 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();
+ });
+});
diff --git a/frontend/src/metabase/admin/datamodel/containers/SegmentApp.jsx b/frontend/src/metabase/admin/datamodel/containers/SegmentApp.jsx
index dcdcab222dc..e605d3fef4c 100644
--- a/frontend/src/metabase/admin/datamodel/containers/SegmentApp.jsx
+++ b/frontend/src/metabase/admin/datamodel/containers/SegmentApp.jsx
@@ -1,10 +1,11 @@
/* eslint-disable react/prop-types */
-import { Component } from "react";
+import { useCallback, useState } from "react";
import { connect } from "react-redux";
import { push } from "react-router-redux";
import * as MetabaseAnalytics from "metabase/lib/analytics";
import Segments from "metabase/entities/segments";
+import { LeaveConfirmationModal } from "metabase/components/LeaveConfirmationModal";
import { updatePreviewSummary } from "../datamodel";
import { getPreviewSummary } from "../selectors";
@@ -21,52 +22,91 @@ const mapStateToProps = (state, props) => ({
previewSummary: getPreviewSummary(state),
});
-class UpdateSegmentFormInner extends Component {
- onSubmit = async segment => {
- await this.props.updateSegment(segment);
- MetabaseAnalytics.trackStructEvent("Data Model", "Segment Updated");
- this.props.onChangeLocation(`/admin/datamodel/segments`);
- };
+const UpdateSegmentFormInner = ({
+ route,
+ segment,
+ updateSegment,
+ onChangeLocation,
+ ...props
+}) => {
+ const [isDirty, setIsDirty] = useState(false);
- render() {
- const { segment, ...props } = this.props;
- return (
+ const handleSubmit = useCallback(
+ async segment => {
+ setIsDirty(false);
+
+ try {
+ await updateSegment(segment);
+ MetabaseAnalytics.trackStructEvent("Data Model", "Segment Updated");
+ onChangeLocation("/admin/datamodel/segments");
+ } catch (error) {
+ setIsDirty(isDirty);
+ }
+ },
+ [updateSegment, isDirty, onChangeLocation],
+ );
+
+ return (
+ <>
<SegmentForm
{...props}
segment={segment.getPlainObject()}
- onSubmit={this.onSubmit}
+ onIsDirtyChange={setIsDirty}
+ onSubmit={handleSubmit}
/>
- );
- }
-}
+ <LeaveConfirmationModal isEnabled={isDirty} route={route} />
+ </>
+ );
+};
const UpdateSegmentForm = Segments.load({
id: (state, props) => parseInt(props.params.id),
})(UpdateSegmentFormInner);
-class CreateSegmentForm extends Component {
- onSubmit = async segment => {
- await this.props.createSegment({
- ...segment,
- table_id: segment.definition["source-table"],
- });
- MetabaseAnalytics.trackStructEvent("Data Model", "Segment Updated");
- this.props.onChangeLocation(`/admin/datamodel/segments`);
- };
-
- render() {
- return <SegmentForm {...this.props} onSubmit={this.onSubmit} />;
- }
-}
-
-class SegmentApp extends Component {
- render() {
- return this.props.params.id ? (
- <UpdateSegmentForm {...this.props} />
- ) : (
- <CreateSegmentForm {...this.props} />
- );
+const CreateSegmentForm = ({
+ route,
+ createSegment,
+ onChangeLocation,
+ ...props
+}) => {
+ const [isDirty, setIsDirty] = useState(false);
+
+ const handleSubmit = useCallback(
+ async segment => {
+ setIsDirty(false);
+
+ try {
+ await createSegment({
+ ...segment,
+ table_id: segment.definition["source-table"],
+ });
+ MetabaseAnalytics.trackStructEvent("Data Model", "Segment Updated");
+ onChangeLocation("/admin/datamodel/segments");
+ } catch (error) {
+ setIsDirty(isDirty);
+ }
+ },
+ [createSegment, isDirty, onChangeLocation],
+ );
+
+ return (
+ <>
+ <SegmentForm
+ {...props}
+ onIsDirtyChange={setIsDirty}
+ onSubmit={handleSubmit}
+ />
+ <LeaveConfirmationModal isEnabled={isDirty} route={route} />
+ </>
+ );
+};
+
+const SegmentApp = props => {
+ if (props.params.id) {
+ return <UpdateSegmentForm {...props} />;
}
-}
+
+ return <CreateSegmentForm {...props} />;
+};
export default connect(mapStateToProps, mapDispatchToProps)(SegmentApp);
diff --git a/frontend/src/metabase/admin/datamodel/containers/SegmentApp.unit.spec.tsx b/frontend/src/metabase/admin/datamodel/containers/SegmentApp.unit.spec.tsx
new file mode 100644
index 00000000000..71c3c5046c9
--- /dev/null
+++ b/frontend/src/metabase/admin/datamodel/containers/SegmentApp.unit.spec.tsx
@@ -0,0 +1,157 @@
+import userEvent from "@testing-library/user-event";
+
+import { callMockEvent } from "__support__/events";
+import {
+ setupCardDataset,
+ setupDatabasesEndpoints,
+ setupSearchEndpoints,
+ setupSegmentsEndpoints,
+} from "__support__/server-mocks";
+import { renderWithProviders, screen, waitFor } from "__support__/ui";
+import { createSampleDatabase } from "metabase-types/api/mocks/presets";
+import { checkNotNull } from "metabase/core/utils/types";
+import { Route } from "metabase/hoc/Title";
+import { BEFORE_UNLOAD_UNSAVED_MESSAGE } from "metabase/hooks/use-before-unload";
+
+import SegmentApp from "./SegmentApp";
+
+const TestHome = () => <div />;
+
+const SEGMENTS_URL = "/admin/datamodel/segments";
+const FORM_URL = "/admin/datamodel/segment/create";
+
+interface SetupOpts {
+ initialRoute?: string;
+}
+
+const setup = ({ initialRoute = FORM_URL }: SetupOpts = {}) => {
+ setupDatabasesEndpoints([createSampleDatabase()]);
+ setupSearchEndpoints([]);
+ setupCardDataset();
+ setupSegmentsEndpoints([]);
+
+ const { history } = renderWithProviders(
+ <>
+ <Route path="/" component={TestHome} />
+ <Route path={SEGMENTS_URL} component={TestHome} />
+ <Route path={FORM_URL} component={SegmentApp} />
+ </>,
+ {
+ initialRoute,
+ withRouter: true,
+ },
+ );
+
+ const mockEventListener = jest.spyOn(window, "addEventListener");
+
+ return {
+ history: checkNotNull(history),
+ mockEventListener,
+ };
+};
+
+describe("SegmentApp", () => {
+ afterEach(() => {
+ jest.restoreAllMocks();
+ });
+
+ it("should have beforeunload event when user makes edits to a segment", async () => {
+ const { mockEventListener } = setup();
+
+ userEvent.type(screen.getByLabelText("Name Your Segment"), "Name");
+
+ const mockEvent = await waitFor(() => {
+ return callMockEvent(mockEventListener, "beforeunload");
+ });
+ expect(mockEvent.preventDefault).toHaveBeenCalled();
+ expect(mockEvent.returnValue).toBe(BEFORE_UNLOAD_UNSAVED_MESSAGE);
+ });
+
+ it("should not have an beforeunload event when segment is unedited", () => {
+ const { mockEventListener } = 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", () => {
+ const { history } = setup({ initialRoute: "/" });
+
+ history.push(FORM_URL);
+
+ 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", () => {
+ const { history } = setup({ initialRoute: "/" });
+
+ history.push(FORM_URL);
+
+ userEvent.type(screen.getByLabelText("Name Your Segment"), "Name");
+
+ 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 when saving changes", async () => {
+ const { history } = setup();
+
+ userEvent.click(screen.getByText("Select a table"));
+
+ await waitFor(() => {
+ expect(screen.queryByTestId("loading-spinner")).not.toBeInTheDocument();
+ });
+
+ userEvent.click(screen.getByText("Orders"));
+
+ await waitFor(() => {
+ expect(screen.queryByTestId("loading-spinner")).not.toBeInTheDocument();
+ });
+
+ userEvent.click(screen.getByText("Add filters to narrow your answer"));
+ userEvent.click(screen.getByText("ID"));
+ userEvent.type(screen.getByPlaceholderText("Enter an ID"), "1");
+ userEvent.click(screen.getByText("Add filter"));
+ userEvent.type(screen.getByLabelText("Name Your Segment"), "Name");
+ userEvent.type(
+ screen.getByLabelText("Describe Your Segment"),
+ "Description",
+ );
+
+ await waitFor(() => {
+ expect(screen.getByText("Save changes")).toBeEnabled();
+ });
+
+ userEvent.click(screen.getByText("Save changes"));
+
+ await waitFor(() => {
+ expect(history.getCurrentLocation().pathname).toBe(SEGMENTS_URL);
+ });
+
+ 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();
+ });
+});
diff --git a/frontend/src/metabase/status/components/StatusListing/StatusListing.tsx b/frontend/src/metabase/status/components/StatusListing/StatusListing.tsx
index 5a16a220be2..6866432017b 100644
--- a/frontend/src/metabase/status/components/StatusListing/StatusListing.tsx
+++ b/frontend/src/metabase/status/components/StatusListing/StatusListing.tsx
@@ -10,7 +10,7 @@ import DatabaseStatus from "../../containers/DatabaseStatus";
import FileUploadStatus from "../FileUploadStatus";
import { StatusListingRoot } from "./StatusListing.styled";
-const StatusListingView = () => {
+const StatusListing = () => {
const isLoggedIn = !!useSelector(getUser);
const isAdmin = useSelector(getUserIsAdmin);
@@ -34,4 +34,4 @@ const StatusListingView = () => {
};
// eslint-disable-next-line import/no-default-export -- deprecated usage
-export default StatusListingView;
+export default StatusListing;
diff --git a/frontend/test/__support__/server-mocks/metric.ts b/frontend/test/__support__/server-mocks/metric.ts
index e95961155a5..430badba0e9 100644
--- a/frontend/test/__support__/server-mocks/metric.ts
+++ b/frontend/test/__support__/server-mocks/metric.ts
@@ -1,11 +1,16 @@
import fetchMock from "fetch-mock";
import type { Metric } from "metabase-types/api";
+import { createMockMetric } from "metabase-types/api/mocks";
export function setupMetricEndpoint(metric: Metric) {
fetchMock.get(`path:/api/metric/${metric.id}`, metric);
}
export function setupMetricsEndpoints(metrics: Metric[]) {
- fetchMock.get(`path:/api/metric`, metrics);
+ fetchMock.post("path:/api/metric", async url => {
+ const metric = await fetchMock.lastCall(url)?.request?.json();
+ return createMockMetric(metric);
+ });
+ fetchMock.get("path:/api/metric", metrics);
metrics.forEach(metric => setupMetricEndpoint(metric));
}
diff --git a/frontend/test/__support__/server-mocks/segment.ts b/frontend/test/__support__/server-mocks/segment.ts
index 9d31883db62..8a07f257d74 100644
--- a/frontend/test/__support__/server-mocks/segment.ts
+++ b/frontend/test/__support__/server-mocks/segment.ts
@@ -1,11 +1,16 @@
import fetchMock from "fetch-mock";
import type { Segment } from "metabase-types/api";
+import { createMockSegment } from "metabase-types/api/mocks";
export function setupSegmentEndpoint(segment: Segment) {
fetchMock.get(`path:/api/segment/${segment.id}`, segment);
}
export function setupSegmentsEndpoints(segments: Segment[]) {
- fetchMock.get(`path:/api/segment`, segments);
+ fetchMock.post("path:/api/segment", async url => {
+ const metric = await fetchMock.lastCall(url)?.request?.json();
+ return createMockSegment(metric);
+ });
+ fetchMock.get("path:/api/segment", segments);
segments.forEach(segment => setupSegmentEndpoint(segment));
}
--
GitLab