From 434e786890baafdc5ffd28b2bb1891beab5ee73a Mon Sep 17 00:00:00 2001
From: Anton Kulyk <kuliks.anton@gmail.com>
Date: Tue, 17 Jan 2023 17:05:50 +0000
Subject: [PATCH] Clean up `PublicApp` component and add tests (#27724)
* Move `PublicApp` to its own directory
* Fix `renderWithProviders` for public environment
* Clean up `PublicApp` and add tests
* Extend coverage
* Wasn't planning to commit this yet
* Swap condition
---
.../metabase/public/components/LogoBadge.jsx | 2 +-
.../metabase/public/containers/PublicApp.jsx | 27 -----
.../public/containers/PublicApp/PublicApp.tsx | 36 ++++++
.../PublicApp/PublicApp.unit.spec.tsx | 104 ++++++++++++++++++
.../public/containers/PublicApp/index.ts | 1 +
frontend/test/__support__/ui.tsx | 17 ++-
6 files changed, 153 insertions(+), 34 deletions(-)
delete mode 100644 frontend/src/metabase/public/containers/PublicApp.jsx
create mode 100644 frontend/src/metabase/public/containers/PublicApp/PublicApp.tsx
create mode 100644 frontend/src/metabase/public/containers/PublicApp/PublicApp.unit.spec.tsx
create mode 100644 frontend/src/metabase/public/containers/PublicApp/index.ts
diff --git a/frontend/src/metabase/public/components/LogoBadge.jsx b/frontend/src/metabase/public/components/LogoBadge.jsx
index a007d73bd40..1dbf8989079 100644
--- a/frontend/src/metabase/public/components/LogoBadge.jsx
+++ b/frontend/src/metabase/public/components/LogoBadge.jsx
@@ -13,7 +13,7 @@ const LogoBadge = ({ dark }) => (
<LogoIcon height={28} dark={dark} />
<span className="text-small">
<span className="ml1 md-ml2 text-medium">{jt`Powered by ${(
- <span className={dark ? "text-white" : "text-brand"}>
+ <span key="metabase" className={dark ? "text-white" : "text-brand"}>
{t`Metabase`}
</span>
)}`}</span>
diff --git a/frontend/src/metabase/public/containers/PublicApp.jsx b/frontend/src/metabase/public/containers/PublicApp.jsx
deleted file mode 100644
index 45bfd28d448..00000000000
--- a/frontend/src/metabase/public/containers/PublicApp.jsx
+++ /dev/null
@@ -1,27 +0,0 @@
-/* eslint-disable react/prop-types */
-import React, { Component } from "react";
-import { connect } from "react-redux";
-
-import PublicNotFound from "metabase/public/components/PublicNotFound";
-import PublicError from "metabase/public/components/PublicError";
-
-const mapStateToProps = (state, props) => ({
- errorPage: state.app.errorPage,
-});
-
-class PublicApp extends Component {
- render() {
- const { children, errorPage } = this.props;
- if (errorPage) {
- if (errorPage.status === 404) {
- return <PublicNotFound />;
- } else {
- return <PublicError />;
- }
- } else {
- return children;
- }
- }
-}
-
-export default connect(mapStateToProps)(PublicApp);
diff --git a/frontend/src/metabase/public/containers/PublicApp/PublicApp.tsx b/frontend/src/metabase/public/containers/PublicApp/PublicApp.tsx
new file mode 100644
index 00000000000..e5131395f9e
--- /dev/null
+++ b/frontend/src/metabase/public/containers/PublicApp/PublicApp.tsx
@@ -0,0 +1,36 @@
+import React from "react";
+import { connect } from "react-redux";
+
+import { getErrorPage } from "metabase/selectors/app";
+
+import PublicNotFound from "metabase/public/components/PublicNotFound";
+import PublicError from "metabase/public/components/PublicError";
+
+import type { AppErrorDescriptor, State } from "metabase-types/store";
+
+interface OwnProps {
+ children: JSX.Element;
+}
+
+interface StateProps {
+ errorPage?: AppErrorDescriptor | null;
+}
+
+type Props = OwnProps & StateProps;
+
+function mapStateToProps(state: State) {
+ return {
+ errorPage: getErrorPage(state),
+ };
+}
+
+function PublicApp({ errorPage, children }: Props) {
+ if (errorPage) {
+ return errorPage.status === 404 ? <PublicNotFound /> : <PublicError />;
+ }
+ return children;
+}
+
+export default connect<StateProps, unknown, OwnProps, State>(mapStateToProps)(
+ PublicApp,
+);
diff --git a/frontend/src/metabase/public/containers/PublicApp/PublicApp.unit.spec.tsx b/frontend/src/metabase/public/containers/PublicApp/PublicApp.unit.spec.tsx
new file mode 100644
index 00000000000..a3496e513f6
--- /dev/null
+++ b/frontend/src/metabase/public/containers/PublicApp/PublicApp.unit.spec.tsx
@@ -0,0 +1,104 @@
+import React from "react";
+import userEvent from "@testing-library/user-event";
+
+import { getIcon, renderWithProviders, screen } from "__support__/ui";
+import { mockSettings } from "__support__/settings";
+
+import { AppErrorDescriptor } from "metabase-types/store";
+import { createMockAppState } from "metabase-types/store/mocks";
+
+import EmbedFrame from "../../components/EmbedFrame";
+import PublicApp from "./PublicApp";
+
+type SetupOpts = {
+ name?: string;
+ description?: string;
+ actionButtons?: JSX.Element[];
+ error?: AppErrorDescriptor;
+ hasEmbedBranding?: boolean;
+};
+
+function setup({
+ error,
+ hasEmbedBranding = true,
+ ...embedFrameProps
+}: SetupOpts = {}) {
+ const app = createMockAppState({ errorPage: error });
+ const settings = mockSettings({ "hide-embed-branding?": !hasEmbedBranding });
+
+ renderWithProviders(
+ <PublicApp>
+ <EmbedFrame {...embedFrameProps}>
+ <h1 data-testid="test-content">Test</h1>
+ </EmbedFrame>
+ </PublicApp>,
+ { mode: "public", storeInitialState: { app, settings }, withRouter: true },
+ );
+}
+
+describe("PublicApp", () => {
+ it("renders children", () => {
+ setup();
+ expect(screen.getByTestId("test-content")).toBeInTheDocument();
+ });
+
+ it("renders name", () => {
+ setup({ name: "My Title", description: "My Description" });
+ expect(screen.getByText("My Title")).toBeInTheDocument();
+ expect(screen.queryByText("My Description")).not.toBeInTheDocument();
+ });
+
+ it("renders description", () => {
+ setup({ name: "My Title", description: "My Description" });
+ userEvent.hover(getIcon("info"));
+ expect(screen.getByText("My Description")).toBeInTheDocument();
+ });
+
+ it("renders action buttons", () => {
+ setup({ actionButtons: [<button key="test">Click Me</button>] });
+ expect(
+ screen.getByRole("button", { name: "Click Me" }),
+ ).toBeInTheDocument();
+ });
+
+ it("renders branding", () => {
+ setup();
+ expect(screen.getByText(/Powered by/i)).toBeInTheDocument();
+ expect(screen.getByText(/Metabase/)).toBeInTheDocument();
+ });
+
+ it("renders not found page on error", () => {
+ setup({ error: { status: 404 } });
+ expect(screen.getByText("Not found")).toBeInTheDocument();
+ expect(screen.queryByTestId("test-content")).not.toBeInTheDocument();
+ });
+
+ it("renders error message", () => {
+ setup({
+ error: {
+ status: 500,
+ data: { error_code: "error", message: "Something went wrong" },
+ },
+ });
+ expect(screen.getByText("Something went wrong")).toBeInTheDocument();
+ expect(screen.queryByTestId("test-content")).not.toBeInTheDocument();
+ });
+
+ it("renders fallback error message", () => {
+ setup({ error: { status: 500 } });
+ expect(screen.getByText(/An error occurred/)).toBeInTheDocument();
+ expect(screen.queryByTestId("test-content")).not.toBeInTheDocument();
+ });
+
+ it("renders branding in error states", () => {
+ setup({ error: { status: 404 } });
+ expect(screen.getByText(/Powered by/i)).toBeInTheDocument();
+ expect(screen.getByText(/Metabase/)).toBeInTheDocument();
+ });
+
+ it("hides branding in error states if it's turned off", () => {
+ setup({ error: { status: 404 }, hasEmbedBranding: false });
+ expect(screen.queryByText(/Powered by/i)).not.toBeInTheDocument();
+ expect(screen.queryByText(/Metabase/)).not.toBeInTheDocument();
+ });
+});
diff --git a/frontend/src/metabase/public/containers/PublicApp/index.ts b/frontend/src/metabase/public/containers/PublicApp/index.ts
new file mode 100644
index 00000000000..6d79d4f68e1
--- /dev/null
+++ b/frontend/src/metabase/public/containers/PublicApp/index.ts
@@ -0,0 +1 @@
+export { default } from "./PublicApp";
diff --git a/frontend/test/__support__/ui.tsx b/frontend/test/__support__/ui.tsx
index 81c77f6c1e3..2da9bc423be 100644
--- a/frontend/test/__support__/ui.tsx
+++ b/frontend/test/__support__/ui.tsx
@@ -1,6 +1,7 @@
import React from "react";
import { render, screen } from "@testing-library/react";
import { merge } from "icepick";
+import _ from "underscore";
import { createMemoryHistory } from "history";
import { Router, Route } from "react-router";
import { Provider } from "react-redux";
@@ -43,16 +44,19 @@ export function renderWithProviders(
...options
}: RenderWithProvidersOptions = {},
) {
- const initialReduxState = createMockState(
+ let initialState = createMockState(
withSampleDatabase
? merge(sampleDatabaseReduxState, storeInitialState)
: storeInitialState,
);
- const store = getStore(
- mode === "default" ? mainReducers : publicReducers,
- initialReduxState,
- );
+ if (mode === "public") {
+ const publicReducerNames = Object.keys(publicReducers);
+ initialState = _.pick(initialState, ...publicReducerNames) as State;
+ }
+
+ const reducers = mode === "default" ? mainReducers : publicReducers;
+ const store = getStore(reducers, initialState);
const wrapper = (props: any) => (
<Wrapper
@@ -106,10 +110,11 @@ function MaybeRouter({
if (!hasRouter) {
return children;
}
+
const history = createMemoryHistory({ entries: ["/"] });
function Page(props: any) {
- return React.cloneElement(children, props);
+ return React.cloneElement(children, _.omit(props, "children"));
}
return (
--
GitLab