Skip to content

Commit 19c6d33

Browse files
kravets-levkoarikfr
authored andcommitted
Refine routes definitions (#4579)
* Refine routes definitions * Replace HoC wrappers with functions to create route definition * Some updates for code consistency * ItemsList component: remove currentRoute dependency * Prepare route parametes in wrapper functions
1 parent a36b101 commit 19c6d33

30 files changed

+317
-630
lines changed

client/app/components/ApplicationArea/AuthenticatedPageWrapper.jsx

Lines changed: 0 additions & 59 deletions
This file was deleted.

client/app/components/ApplicationArea/SignedOutPageWrapper.jsx

Lines changed: 0 additions & 41 deletions
This file was deleted.
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import React, { useEffect, useState, useContext } from "react";
2+
import PropTypes from "prop-types";
3+
import { ErrorBoundaryContext } from "@/components/ErrorBoundary";
4+
import { Auth } from "@/services/auth";
5+
6+
// This wrapper modifies `route.render` function and instead of passing `currentRoute` passes an object
7+
// that contains:
8+
// - `currentRoute.routeParams`
9+
// - `pageTitle` field which is equal to `currentRoute.title`
10+
// - `onError` field which is a `handleError` method of nearest error boundary
11+
// - `apiKey` field
12+
13+
function ApiKeySessionWrapper({ apiKey, currentRoute, renderChildren }) {
14+
const [isAuthenticated, setIsAuthenticated] = useState(false);
15+
const { handleError } = useContext(ErrorBoundaryContext);
16+
17+
useEffect(() => {
18+
let isCancelled = false;
19+
Auth.setApiKey(apiKey);
20+
Auth.loadConfig()
21+
.then(() => {
22+
if (!isCancelled) {
23+
setIsAuthenticated(true);
24+
}
25+
})
26+
.catch(() => {
27+
if (!isCancelled) {
28+
setIsAuthenticated(false);
29+
}
30+
});
31+
return () => {
32+
isCancelled = true;
33+
};
34+
}, [apiKey]);
35+
36+
if (!isAuthenticated) {
37+
return null;
38+
}
39+
40+
return (
41+
<React.Fragment key={currentRoute.key}>
42+
{renderChildren({ ...currentRoute.routeParams, pageTitle: currentRoute.title, onError: handleError, apiKey })}
43+
</React.Fragment>
44+
);
45+
}
46+
47+
ApiKeySessionWrapper.propTypes = {
48+
apiKey: PropTypes.string.isRequired,
49+
renderChildren: PropTypes.func,
50+
};
51+
52+
ApiKeySessionWrapper.defaultProps = {
53+
renderChildren: () => null,
54+
};
55+
56+
export default function routeWithApiKeySession({ render, getApiKey, ...rest }) {
57+
return {
58+
...rest,
59+
render: currentRoute => (
60+
<ApiKeySessionWrapper apiKey={getApiKey(currentRoute)} currentRoute={currentRoute} renderChildren={render} />
61+
),
62+
};
63+
}
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
import React, { useEffect, useState } from "react";
2+
import PropTypes from "prop-types";
3+
import ErrorBoundary, { ErrorBoundaryContext } from "@/components/ErrorBoundary";
4+
import { Auth } from "@/services/auth";
5+
import organizationStatus from "@/services/organizationStatus";
6+
import ApplicationHeader from "./ApplicationHeader";
7+
import ErrorMessage from "./ErrorMessage";
8+
9+
// This wrapper modifies `route.render` function and instead of passing `currentRoute` passes an object
10+
// that contains:
11+
// - `currentRoute.routeParams`
12+
// - `pageTitle` field which is equal to `currentRoute.title`
13+
// - `onError` field which is a `handleError` method of nearest error boundary
14+
15+
function UserSessionWrapper({ bodyClass, currentRoute, renderChildren }) {
16+
const [isAuthenticated, setIsAuthenticated] = useState(!!Auth.isAuthenticated());
17+
18+
useEffect(() => {
19+
let isCancelled = false;
20+
Promise.all([Auth.requireSession(), organizationStatus.refresh()])
21+
.then(() => {
22+
if (!isCancelled) {
23+
setIsAuthenticated(!!Auth.isAuthenticated());
24+
}
25+
})
26+
.catch(() => {
27+
if (!isCancelled) {
28+
setIsAuthenticated(false);
29+
}
30+
});
31+
return () => {
32+
isCancelled = true;
33+
};
34+
}, []);
35+
36+
useEffect(() => {
37+
if (bodyClass) {
38+
document.body.classList.toggle(bodyClass, true);
39+
return () => {
40+
document.body.classList.toggle(bodyClass, false);
41+
};
42+
}
43+
}, [bodyClass]);
44+
45+
if (!isAuthenticated) {
46+
return null;
47+
}
48+
49+
return (
50+
<React.Fragment>
51+
<ApplicationHeader />
52+
<React.Fragment key={currentRoute.key}>
53+
<ErrorBoundary renderError={error => <ErrorMessage error={error} />}>
54+
<ErrorBoundaryContext.Consumer>
55+
{({ handleError }) =>
56+
renderChildren({ ...currentRoute.routeParams, pageTitle: currentRoute.title, onError: handleError })
57+
}
58+
</ErrorBoundaryContext.Consumer>
59+
</ErrorBoundary>
60+
</React.Fragment>
61+
</React.Fragment>
62+
);
63+
}
64+
65+
UserSessionWrapper.propTypes = {
66+
bodyClass: PropTypes.string,
67+
renderChildren: PropTypes.func,
68+
};
69+
70+
UserSessionWrapper.defaultProps = {
71+
bodyClass: null,
72+
renderChildren: () => null,
73+
};
74+
75+
export default function routeWithUserSession({ render, bodyClass, ...rest }) {
76+
return {
77+
...rest,
78+
render: currentRoute => (
79+
<UserSessionWrapper bodyClass={bodyClass} currentRoute={currentRoute} renderChildren={render} />
80+
),
81+
};
82+
}

client/app/components/ErrorBoundary.jsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ const logger = debug("redash:errors");
88

99
export const ErrorBoundaryContext = React.createContext({
1010
handleError: error => {
11-
throw error;
11+
// Allow calling chain to roll up, and then throw the error in global context
12+
setTimeout(() => {
13+
throw error;
14+
});
1215
},
1316
reset: () => {},
1417
});

client/app/components/items-list/ItemsList.jsx

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import hoistNonReactStatics from "hoist-non-react-statics";
55
import { clientConfig } from "@/services/auth";
66

77
export const ControllerType = PropTypes.shape({
8-
// values of props declared by wrapped component, current route's locals (`resolve: { ... }`) and title
8+
// values of props declared by wrapped component and some additional props from items list
99
params: PropTypes.object.isRequired,
1010

1111
isLoaded: PropTypes.bool.isRequired,
@@ -38,13 +38,11 @@ export const ControllerType = PropTypes.shape({
3838
export function wrap(WrappedComponent, createItemsSource, createStateStorage) {
3939
class ItemsListWrapper extends React.Component {
4040
static propTypes = {
41-
...omit(WrappedComponent.propTypes, ["controller"]),
4241
onError: PropTypes.func,
4342
children: PropTypes.node,
4443
};
4544

4645
static defaultProps = {
47-
...omit(WrappedComponent.defaultProps, ["controller"]),
4846
onError: error => {
4947
// Allow calling chain to roll up, and then throw the error in global context
5048
setTimeout(() => {
@@ -102,20 +100,13 @@ export function wrap(WrappedComponent, createItemsSource, createStateStorage) {
102100

103101
// eslint-disable-next-line class-methods-use-this
104102
getState({ isLoaded, totalCount, pageItems, params, ...rest }) {
105-
params = {
106-
// Custom params from items source
107-
...params,
108-
109-
title: this.props.currentRoute.title,
110-
...this.props.routeParams,
111-
112-
// Add to params all props except of own ones
113-
...omit(this.props, ["onError", "children", "currentRoute", "routeParams"]),
114-
};
115103
return {
116104
...rest,
117105

118-
params,
106+
params: {
107+
...params, // custom params from items source
108+
...omit(this.props, ["onError", "children"]), // add all props except of own ones
109+
},
119110

120111
isLoaded,
121112
isEmpty: !isLoaded || totalCount === 0,
@@ -128,8 +119,11 @@ export function wrap(WrappedComponent, createItemsSource, createStateStorage) {
128119
render() {
129120
// don't pass own props to wrapped component
130121
const { children, onError, ...props } = this.props;
131-
props.controller = this.state;
132-
return <WrappedComponent {...props}>{children}</WrappedComponent>;
122+
return (
123+
<WrappedComponent {...props} controller={this.state}>
124+
{children}
125+
</WrappedComponent>
126+
);
133127
}
134128
}
135129

client/app/pages/admin/Jobs.jsx

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,8 @@ import { axios } from "@/services/axios";
55
import Alert from "antd/lib/alert";
66
import Tabs from "antd/lib/tabs";
77
import * as Grid from "antd/lib/grid";
8-
import AuthenticatedPageWrapper from "@/components/ApplicationArea/AuthenticatedPageWrapper";
8+
import routeWithUserSession from "@/components/ApplicationArea/routeWithUserSession";
99
import Layout from "@/components/admin/Layout";
10-
import { ErrorBoundaryContext } from "@/components/ErrorBoundary";
1110
import { CounterCard, WorkersTable, QueuesTable, OtherJobsTable } from "@/components/admin/RQStatus";
1211

1312
import location from "@/services/location";
@@ -121,14 +120,8 @@ class Jobs extends React.Component {
121120
}
122121
}
123122

124-
export default {
123+
export default routeWithUserSession({
125124
path: "/admin/queries/jobs",
126125
title: "RQ Status",
127-
render: currentRoute => (
128-
<AuthenticatedPageWrapper key={currentRoute.key}>
129-
<ErrorBoundaryContext.Consumer>
130-
{({ handleError }) => <Jobs {...currentRoute.routeParams} onError={handleError} />}
131-
</ErrorBoundaryContext.Consumer>
132-
</AuthenticatedPageWrapper>
133-
),
134-
};
126+
render: pageProps => <Jobs {...pageProps} />,
127+
});

client/app/pages/admin/OutdatedQueries.jsx

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,12 @@ import { axios } from "@/services/axios";
44

55
import Switch from "antd/lib/switch";
66
import * as Grid from "antd/lib/grid";
7-
import AuthenticatedPageWrapper from "@/components/ApplicationArea/AuthenticatedPageWrapper";
7+
import routeWithUserSession from "@/components/ApplicationArea/routeWithUserSession";
88
import Paginator from "@/components/Paginator";
99
import { QueryTagsControl } from "@/components/tags-control/TagsControl";
1010
import SchedulePhrase from "@/components/queries/SchedulePhrase";
1111
import TimeAgo from "@/components/TimeAgo";
1212
import Layout from "@/components/admin/Layout";
13-
import { ErrorBoundaryContext } from "@/components/ErrorBoundary";
1413

1514
import { wrap as itemsList, ControllerType } from "@/components/items-list/ItemsList";
1615
import { ItemsSource } from "@/components/items-list/classes/ItemsSource";
@@ -170,20 +169,8 @@ const OutdatedQueriesPage = itemsList(
170169
() => new StateStorage({ orderByField: "created_at", orderByReverse: true })
171170
);
172171

173-
export default {
172+
export default routeWithUserSession({
174173
path: "/admin/queries/outdated",
175174
title: "Outdated Queries",
176-
render: currentRoute => (
177-
<AuthenticatedPageWrapper key={currentRoute.key}>
178-
<ErrorBoundaryContext.Consumer>
179-
{({ handleError }) => (
180-
<OutdatedQueriesPage
181-
routeParams={{ ...currentRoute.routeParams, currentPage: "outdated_queries" }}
182-
currentRoute={currentRoute}
183-
onError={handleError}
184-
/>
185-
)}
186-
</ErrorBoundaryContext.Consumer>
187-
</AuthenticatedPageWrapper>
188-
),
189-
};
175+
render: pageProps => <OutdatedQueriesPage {...pageProps} currentPage="outdated_queries" />,
176+
});

0 commit comments

Comments
 (0)