From 964c8059e9d030ddb1c1fd2253700986e502229b Mon Sep 17 00:00:00 2001
From: Kyle Doherty <kdoh@users.noreply.github.com>
Date: Thu, 22 Mar 2018 14:44:17 -0700
Subject: [PATCH] new components for easier question and result loading (#7134)
* new components for easier question and result loading
* code review updates
* prettier
* Cleanup loader component tests, props, and flow types
* Cleanup question/result loaders, add rawQuery, add _internal Question demo app
* lint fix
---
.../containers/AdHocQuestionLoader.jsx | 166 +++++++++++++++++
.../containers/QuestionAndResultLoader.jsx | 59 +++++++
.../metabase/containers/QuestionLoader.jsx | 69 ++++++++
.../containers/QuestionResultLoader.jsx | 151 ++++++++++++++++
.../containers/SavedQuestionLoader.jsx | 167 ++++++++++++++++++
.../internal/components/ColorsApp.jsx | 2 +
.../internal/components/ComponentsApp.jsx | 14 +-
.../metabase/internal/components/IconsApp.jsx | 17 +-
.../internal/components/QuestionApp.jsx | 51 ++++++
frontend/src/metabase/internal/routes.js | 36 ++--
.../src/metabase/meta/types/Visualization.js | 5 +-
.../components/Visualization.jsx | 3 +-
frontend/src/metabase/visualizations/index.js | 1 -
.../visualizations/Progress.jsx | 3 +-
.../AdHocQuestionLoader.unit.spec.js | 76 ++++++++
.../QuestionAndResultLoader.unit.spec.js | 10 ++
.../containers/QuestionLoader.unit.spec.js | 46 +++++
.../QuestionResultLoader.unit.spec.js | 22 +++
.../SavedQuestionLoader.unit.spec.js | 76 ++++++++
19 files changed, 947 insertions(+), 27 deletions(-)
create mode 100644 frontend/src/metabase/containers/AdHocQuestionLoader.jsx
create mode 100644 frontend/src/metabase/containers/QuestionAndResultLoader.jsx
create mode 100644 frontend/src/metabase/containers/QuestionLoader.jsx
create mode 100644 frontend/src/metabase/containers/QuestionResultLoader.jsx
create mode 100644 frontend/src/metabase/containers/SavedQuestionLoader.jsx
create mode 100644 frontend/src/metabase/internal/components/QuestionApp.jsx
create mode 100644 frontend/test/containers/AdHocQuestionLoader.unit.spec.js
create mode 100644 frontend/test/containers/QuestionAndResultLoader.unit.spec.js
create mode 100644 frontend/test/containers/QuestionLoader.unit.spec.js
create mode 100644 frontend/test/containers/QuestionResultLoader.unit.spec.js
create mode 100644 frontend/test/containers/SavedQuestionLoader.unit.spec.js
diff --git a/frontend/src/metabase/containers/AdHocQuestionLoader.jsx b/frontend/src/metabase/containers/AdHocQuestionLoader.jsx
new file mode 100644
index 00000000000..b891b2eebf8
--- /dev/null
+++ b/frontend/src/metabase/containers/AdHocQuestionLoader.jsx
@@ -0,0 +1,166 @@
+/* @flow */
+
+import React from "react";
+import { connect } from "react-redux";
+
+// things that will eventually load the quetsion
+import { deserializeCardFromUrl } from "metabase/lib/card";
+import { loadMetadataForCard } from "metabase/query_builder/actions";
+import { getMetadata } from "metabase/selectors/metadata";
+
+import Question from "metabase-lib/lib/Question";
+
+// type annotations
+import type Metadata from "metabase-lib/lib/metadata/Metadata";
+import type { Card } from "metabase/meta/types/Card";
+
+type ChildProps = {
+ loading: boolean,
+ error: ?any,
+ question: ?Question,
+};
+
+type Props = {
+ questionHash?: string,
+ children?: (props: ChildProps) => React$Element<any>,
+ // provided by redux
+ loadMetadataForCard: (card: Card) => Promise<void>,
+ metadata: Metadata,
+};
+
+type State = {
+ // the question should be of type Question if it is set
+ question: ?Question,
+ card: ?Card,
+ loading: boolean,
+ error: ?any,
+};
+
+/*
+ * AdHocQuestionLoader
+ *
+ * Load a transient quetsion via its encoded URL and return it to the calling
+ * component
+ *
+ * @example
+ *
+ * Render prop style
+ * import AdHocQuestionLoader from 'metabase/containers/AdHocQuestionLoader'
+ *
+ * // assuming
+ * class ExampleAdHocQuestionFeature extends React.Component {
+ * render () {
+ * return (
+ * <AdHocQuestionLoader questionId={this.props.params.questionId}>
+ * { ({ question, loading, error }) => {
+ *
+ * }}
+ * </SavedQuestion>
+ * )
+ * }
+ * }
+ *
+ * @example
+ *
+ * The raw un-connected component is also exported so we can unit test it
+ * without the redux store.
+ */
+export class AdHocQuestionLoader extends React.Component {
+ props: Props;
+
+ state: State = {
+ // this will store the loaded question
+ question: null,
+ // keep a reference to the card as well to help with re-creating question
+ // objects if the underlying metadata changes
+ card: null,
+ loading: false,
+ error: null,
+ };
+
+ componentWillMount() {
+ // load the specified question when the component mounts
+ this._loadQuestion(this.props.questionHash);
+ }
+
+ componentWillReceiveProps(nextProps: Props) {
+ // if the questionHash changes (this will most likely be the result of a
+ // url change) then we need to load this new question
+ if (nextProps.questionHash !== this.props.questionHash) {
+ this._loadQuestion(nextProps.questionHash);
+ }
+
+ // if the metadata changes for some reason we need to make sure we
+ // update the question with that metadata
+ if (nextProps.metadata !== this.props.metadata && this.state.card) {
+ this.setState({
+ question: new Question(nextProps.metadata, this.state.card),
+ });
+ }
+ }
+
+ /*
+ * Load an AdHoc question and any required metadata
+ *
+ * 1. Decode the question via the URL
+ * 2. Load any required metadata into the redux store
+ * 3. Create a new Question object to return to metabase-lib methods can
+ * be used
+ * 4. Set the component state to the new Question
+ */
+ async _loadQuestion(questionHash: ?string) {
+ if (!questionHash) {
+ this.setState({
+ loading: false,
+ error: null,
+ question: null,
+ card: null,
+ });
+ return;
+ }
+ try {
+ this.setState({ loading: true, error: null });
+ // get the card definition from the URL, the "card"
+ const card = deserializeCardFromUrl(questionHash);
+ // pass the decoded card to load any necessary metadata
+ // (tables, source db, segments, etc) into
+ // the redux store, the resulting metadata will be avaliable as metadata on the
+ // component props once it's avaliable
+ await this.props.loadMetadataForCard(card);
+
+ // instantiate a new question object using the metadata and saved question
+ // so we can use metabase-lib methods to retrieve information and modify
+ // the question
+ const question = new Question(this.props.metadata, card);
+
+ // finally, set state to store the Question object so it can be passed
+ // to the component using the loader, keep a reference to the card
+ // as well
+ this.setState({ loading: false, question, card });
+ } catch (error) {
+ this.setState({ loading: false, error });
+ }
+ }
+
+ render() {
+ const { children } = this.props;
+ const { question, loading, error } = this.state;
+ // call the child function with our loaded question
+ return children && children({ question, loading, error });
+ }
+}
+
+// redux stuff
+function mapStateToProps(state) {
+ return {
+ metadata: getMetadata(state),
+ };
+}
+
+const mapDispatchToProps = {
+ loadMetadataForCard,
+};
+
+export default connect(mapStateToProps, mapDispatchToProps)(
+ AdHocQuestionLoader,
+);
diff --git a/frontend/src/metabase/containers/QuestionAndResultLoader.jsx b/frontend/src/metabase/containers/QuestionAndResultLoader.jsx
new file mode 100644
index 00000000000..aac64d22466
--- /dev/null
+++ b/frontend/src/metabase/containers/QuestionAndResultLoader.jsx
@@ -0,0 +1,59 @@
+/* @flow */
+
+import React from "react";
+
+import QuestionLoader from "metabase/containers/QuestionLoader";
+import QuestionResultLoader from "metabase/containers/QuestionResultLoader";
+
+import type { ChildProps as QuestionLoaderChildProps } from "./QuestionLoader";
+import type { ChildProps as QuestionResultLoaderChildProps } from "./QuestionResultLoader";
+
+type ChildProps = QuestionLoaderChildProps & QuestionResultLoaderChildProps;
+
+type Props = {
+ questionId?: ?number,
+ questionHash?: ?string,
+ children?: (props: ChildProps) => React$Element<any>,
+};
+
+/*
+ * QuestionAndResultLoader
+ *
+ * Load a question and also run the query to get the result. Useful when you want
+ * to load both a question and its visualization at the same time.
+ *
+ * @example
+ *
+ * import QuestionAndResultLoader from 'metabase/containers/QuestionAndResultLoader'
+ *
+ * const MyNewFeature = ({ params, location }) =>
+ * <QuestionAndResultLoader question={question}>
+ * { ({ question, result, cancel, reload }) =>
+ * <div>
+ * </div>
+ * </QuestionAndResultLoader>
+ *
+ */
+const QuestionAndResultLoader = ({
+ questionId,
+ questionHash,
+ children,
+}: Props) => (
+ <QuestionLoader questionId={questionId} questionHash={questionHash}>
+ {({ loading: questionLoading, error: questionError, ...questionProps }) => (
+ <QuestionResultLoader question={questionProps.question}>
+ {({ loading: resultLoading, error: resultError, ...resultProps }) =>
+ children &&
+ children({
+ ...questionProps,
+ ...resultProps,
+ loading: resultLoading || questionLoading,
+ error: resultError || questionError,
+ })
+ }
+ </QuestionResultLoader>
+ )}
+ </QuestionLoader>
+);
+
+export default QuestionAndResultLoader;
diff --git a/frontend/src/metabase/containers/QuestionLoader.jsx b/frontend/src/metabase/containers/QuestionLoader.jsx
new file mode 100644
index 00000000000..50ec004f47f
--- /dev/null
+++ b/frontend/src/metabase/containers/QuestionLoader.jsx
@@ -0,0 +1,69 @@
+/* @flow */
+
+import React from "react";
+
+import AdHocQuestionLoader from "metabase/containers/AdHocQuestionLoader";
+import SavedQuestionLoader from "metabase/containers/SavedQuestionLoader";
+
+import Question from "metabase-lib/lib/Question";
+
+export type ChildProps = {
+ loading: boolean,
+ error: ?any,
+ question: ?Question,
+};
+
+type Props = {
+ questionId?: ?number,
+ questionHash?: ?string,
+ children?: (props: ChildProps) => React$Element<any>,
+};
+
+/*
+ * QuestionLoader
+ *
+ * Load either a saved or ad-hoc question depending on which is needed. Use
+ * this component if you need to moved between saved and ad-hoc questions
+ * as part of the same experience in the same part of the app.
+ *
+ * @example
+ * import QuestionLoader from 'metabase/containers/QuestionLoader
+ *
+ * const MyQuestionExplorer = ({ params, location }) =>
+ * <QuestionLoader questionId={params.questionId} questionHash={
+ * { ({ question, loading, error }) =>
+ * <div>
+ * { // display info about the loaded question }
+ * <h1>{ question.displayName() }</h1>
+ *
+ * { // link to a new question created by adding a filter }
+ * <Link
+ * to={
+ * question.query()
+ * .addFilter([
+ * "SEGMENT",
+ * question.query().filterSegmentOptions()[0]
+ * ])
+ * .question()
+ * .getUrl()
+ * }
+ * >
+ * View this ad-hoc exploration
+ * </Link>
+ * </div>
+ * }
+ * </QuestionLoader>
+ *
+ */
+
+const QuestionLoader = ({ questionId, questionHash, children }: Props) =>
+ // if there's a questionHash it means we're in ad-hoc land
+ questionHash ? (
+ <AdHocQuestionLoader questionHash={questionHash} children={children} />
+ ) : // otherwise if there's a non-null questionId it means we're in saved land
+ questionId != null ? (
+ <SavedQuestionLoader questionId={questionId} children={children} />
+ ) : // finally, if neither is present, just don't do anything
+ null;
+
+export default QuestionLoader;
diff --git a/frontend/src/metabase/containers/QuestionResultLoader.jsx b/frontend/src/metabase/containers/QuestionResultLoader.jsx
new file mode 100644
index 00000000000..fb24e9f7ccf
--- /dev/null
+++ b/frontend/src/metabase/containers/QuestionResultLoader.jsx
@@ -0,0 +1,151 @@
+/* @flow */
+
+import React from "react";
+import { defer } from "metabase/lib/promise";
+
+import type { Dataset } from "metabase/meta/types/Dataset";
+import type { RawSeries } from "metabase/meta/types/Visualization";
+
+import Question from "metabase-lib/lib/Question";
+
+export type ChildProps = {
+ loading: boolean,
+ error: ?any,
+ results: ?(Dataset[]),
+ result: ?Dataset,
+ rawSeries: ?RawSeries,
+ cancel: () => void,
+ reload: () => void,
+};
+
+type Props = {
+ question: ?Question,
+ children?: (props: ChildProps) => React$Element<any>,
+};
+
+type State = {
+ results: ?(Dataset[]),
+ loading: boolean,
+ error: ?any,
+};
+
+/*
+ * Question result loader
+ *
+ * Handle runninng, canceling, and reloading Question results
+ *
+ * @example
+ * <QuestionResultLoader question={question}>
+ * { ({ result, cancel, reload }) =>
+ * <div>
+ * { result && (<Visualization ... />) }
+ *
+ * <a onClick={() => reload()}>Reload this please</a>
+ * <a onClick={() => cancel()}>Changed my mind</a>
+ * </div>
+ * }
+ * </QuestionResultLoader>
+ *
+ */
+export class QuestionResultLoader extends React.Component {
+ props: Props;
+ state: State = {
+ results: null,
+ loading: false,
+ error: null,
+ };
+
+ _cancelDeferred: ?() => void;
+
+ componentWillMount() {
+ this._loadResult(this.props.question);
+ }
+
+ componentWillReceiveProps(nextProps: Props) {
+ // if the question is different, we need to do a fresh load, check the
+ // difference by comparing the URL we'd generate for the question
+ if (
+ (nextProps.question && nextProps.question.getUrl()) !==
+ (this.props.question && this.props.question.getUrl())
+ ) {
+ this._loadResult(nextProps.question);
+ }
+ }
+
+ /*
+ * load the result by calling question.apiGetResults
+ */
+ async _loadResult(question: ?Question) {
+ // we need to have a question for anything to happen
+ if (question) {
+ try {
+ // set up a defer for cancelation
+ this._cancelDeferred = defer();
+
+ // begin the request, set cancel in state so the query can be canceled
+ this.setState({ loading: true, results: null, error: null });
+
+ // call apiGetResults and pass our cancel to allow for cancelation
+ const results: Dataset[] = await question.apiGetResults({
+ cancelDeferred: this._cancelDeferred,
+ });
+
+ // setState with our result, remove our cancel since we've finished
+ this.setState({ loading: false, results });
+ } catch (error) {
+ this.setState({ loading: false, error });
+ }
+ } else {
+ // if there's not a question we can't do anything so go back to our initial
+ // state
+ this.setState({ loading: false, results: null, error: null });
+ }
+ }
+
+ /*
+ * a function to pass to the child to allow the component to call
+ * load again
+ */
+ _reload = () => {
+ this._loadResult(this.props.question);
+ };
+
+ /*
+ * a function to pass to the child to allow the component to interrupt
+ * the query
+ */
+ _cancel = () => {
+ // we only want to do things if cancel has been set
+ if (this.state.loading) {
+ // set loading false
+ this.setState({ loading: false });
+ // call our _cancelDeferred to cancel the query
+ if (this._cancelDeferred) {
+ this._cancelDeferred();
+ }
+ }
+ };
+
+ render() {
+ const { question, children } = this.props;
+ const { results, loading, error } = this.state;
+ return (
+ children &&
+ children({
+ results,
+ result: results && results[0],
+ // convienence for <Visualization /> component. Only support single series for now
+ rawSeries:
+ question && results
+ ? [{ card: question.card(), data: results[0].data }]
+ : null,
+ loading,
+ error,
+ cancel: this._cancel,
+ reload: this._reload,
+ })
+ );
+ }
+}
+
+export default QuestionResultLoader;
diff --git a/frontend/src/metabase/containers/SavedQuestionLoader.jsx b/frontend/src/metabase/containers/SavedQuestionLoader.jsx
new file mode 100644
index 00000000000..76c543dd45e
--- /dev/null
+++ b/frontend/src/metabase/containers/SavedQuestionLoader.jsx
@@ -0,0 +1,167 @@
+/* @flow */
+
+import React from "react";
+import { connect } from "react-redux";
+
+// things that will eventually load the quetsion
+import { CardApi } from "metabase/services";
+import { loadMetadataForCard } from "metabase/query_builder/actions";
+import { getMetadata } from "metabase/selectors/metadata";
+
+import Question from "metabase-lib/lib/Question";
+
+// type annotations
+import type Metadata from "metabase-lib/lib/metadata/Metadata";
+import type { Card } from "metabase/meta/types/Card";
+
+type ChildProps = {
+ loading: boolean,
+ error: ?any,
+ question: ?Question,
+};
+
+type Props = {
+ questionId: ?number,
+ children?: (props: ChildProps) => React$Element<any>,
+ // provided by redux
+ loadMetadataForCard: (card: Card) => Promise<void>,
+ metadata: Metadata,
+};
+
+type State = {
+ // the question should be of type Question if it is set
+ question: ?Question,
+ // keep a reference to the card as well to help with re-creating question
+ // objects if the underlying metadata changes
+ card: ?Card,
+ loading: boolean,
+ error: ?any,
+};
+
+/*
+ * SavedQuestionLaoder
+ *
+ * Load a saved quetsion and return it to the calling component
+ *
+ * @example
+ *
+ * Render prop style
+ * import SavedQuestionLoader from 'metabase/containers/SavedQuestionLoader'
+ *
+ * // assuming
+ * class ExampleSavedQuestionFeature extends React.Component {
+ * render () {
+ * return (
+ * <SavedQuestionLoader questionId={this.props.params.questionId}>
+ * { ({ question, loading, error }) => {
+ *
+ * }}
+ * </SavedQuestion>
+ * )
+ * }
+ * }
+ *
+ * @example
+ *
+ * The raw un-connected component is also exported so we can unit test it
+ * without the redux store.
+ */
+export class SavedQuestionLoader extends React.Component {
+ props: Props;
+
+ state: State = {
+ // this will store the loaded question
+ question: null,
+ card: null,
+ loading: false,
+ error: null,
+ };
+
+ componentWillMount() {
+ // load the specified question when the component mounts
+ this._loadQuestion(this.props.questionId);
+ }
+
+ componentWillReceiveProps(nextProps: Props) {
+ // if the questionId changes (this will most likely be the result of a
+ // url change) then we need to load this new question
+ if (nextProps.questionId !== this.props.questionId) {
+ this._loadQuestion(nextProps.questionId);
+ }
+
+ // if the metadata changes for some reason we need to make sure we
+ // update the question with that metadata
+ if (nextProps.metadata !== this.props.metadata && this.state.card) {
+ this.setState({
+ question: new Question(nextProps.metadata, this.state.card),
+ });
+ }
+ }
+
+ /*
+ * Load a saved question and any required metadata
+ *
+ * 1. Get the card from the api
+ * 2. Load any required metadata into the redux store
+ * 3. Create a new Question object to return to metabase-lib methods can
+ * be used
+ * 4. Set the component state to the new Question
+ */
+ async _loadQuestion(questionId: ?number) {
+ if (questionId == null) {
+ this.setState({
+ loading: false,
+ error: null,
+ question: null,
+ card: null,
+ });
+ return;
+ }
+ try {
+ this.setState({ loading: true, error: null });
+ // get the saved question via the card API
+ const card = await CardApi.get({ cardId: questionId });
+
+ // pass the retrieved card to load any necessary metadata
+ // (tables, source db, segments, etc) into
+ // the redux store, the resulting metadata will be avaliable as metadata on the
+ // component props once it's avaliable
+ await this.props.loadMetadataForCard(card);
+
+ // instantiate a new question object using the metadata and saved question
+ // so we can use metabase-lib methods to retrieve information and modify
+ // the question
+ //
+ const question = new Question(this.props.metadata, card);
+
+ // finally, set state to store the Question object so it can be passed
+ // to the component using the loader, keep a reference to the card
+ // as well
+ this.setState({ loading: false, question, card });
+ } catch (error) {
+ this.setState({ loading: false, error });
+ }
+ }
+
+ render() {
+ const { children } = this.props;
+ const { question, loading, error } = this.state;
+ // call the child function with our loaded question
+ return children && children({ question, loading, error });
+ }
+}
+
+// redux stuff
+function mapStateToProps(state) {
+ return {
+ metadata: getMetadata(state),
+ };
+}
+
+const mapDispatchToProps = {
+ loadMetadataForCard,
+};
+
+export default connect(mapStateToProps, mapDispatchToProps)(
+ SavedQuestionLoader,
+);
diff --git a/frontend/src/metabase/internal/components/ColorsApp.jsx b/frontend/src/metabase/internal/components/ColorsApp.jsx
index aa1e941aa12..2c82dccc803 100644
--- a/frontend/src/metabase/internal/components/ColorsApp.jsx
+++ b/frontend/src/metabase/internal/components/ColorsApp.jsx
@@ -1,3 +1,5 @@
+/* @flow */
+
import React from "react";
import cx from "classnames";
diff --git a/frontend/src/metabase/internal/components/ComponentsApp.jsx b/frontend/src/metabase/internal/components/ComponentsApp.jsx
index 045c613dd27..6111b2d3275 100644
--- a/frontend/src/metabase/internal/components/ComponentsApp.jsx
+++ b/frontend/src/metabase/internal/components/ComponentsApp.jsx
@@ -1,5 +1,7 @@
+/* @flow */
+
import React, { Component } from "react";
-import { Link } from "react-router";
+import { Link, Route } from "react-router";
import { slugify } from "metabase/lib/formatting";
import reactElementToJSXString from "react-element-to-jsx-string";
@@ -13,6 +15,7 @@ const Section = ({ title, children }) => (
);
export default class ComponentsApp extends Component {
+ static routes: ?[React$Element<Route>];
render() {
const componentName = slugify(this.props.params.componentName);
const exampleName = slugify(this.props.params.exampleName);
@@ -116,3 +119,12 @@ export default class ComponentsApp extends Component {
);
}
}
+
+ComponentsApp.routes = [
+ <Route path="components" component={ComponentsApp} />,
+ <Route path="components/:componentName" component={ComponentsApp} />,
+ <Route
+ path="components/:componentName/:exampleName"
+ component={ComponentsApp}
+ />,
+];
diff --git a/frontend/src/metabase/internal/components/IconsApp.jsx b/frontend/src/metabase/internal/components/IconsApp.jsx
index 1e688eccfc1..9634fdb832f 100644
--- a/frontend/src/metabase/internal/components/IconsApp.jsx
+++ b/frontend/src/metabase/internal/components/IconsApp.jsx
@@ -1,16 +1,21 @@
+/* @flow */
+
import React, { Component } from "react";
import Icon from "metabase/components/Icon.jsx";
const SIZES = [12, 16];
+type Props = {};
+type State = {
+ size: number,
+};
+
export default class IconsApp extends Component {
- constructor(props) {
- super(props);
- this.state = {
- size: 32,
- };
- }
+ props: Props;
+ state: State = {
+ size: 32,
+ };
render() {
let sizes = SIZES.concat(this.state.size);
return (
diff --git a/frontend/src/metabase/internal/components/QuestionApp.jsx b/frontend/src/metabase/internal/components/QuestionApp.jsx
new file mode 100644
index 00000000000..d27666c80c6
--- /dev/null
+++ b/frontend/src/metabase/internal/components/QuestionApp.jsx
@@ -0,0 +1,51 @@
+/* @flow */
+
+import React from "react";
+import { Route } from "react-router";
+
+import QuestionAndResultLoader from "metabase/containers/QuestionAndResultLoader";
+import Visualization from "metabase/visualizations/components/Visualization";
+
+type Props = {
+ location: {
+ hash: ?string,
+ },
+ params: {
+ questionId?: string,
+ },
+};
+
+export default class QuestionApp extends React.Component {
+ props: Props;
+
+ static routes: ?[React$Element<Route>];
+
+ render() {
+ const { location, params } = this.props;
+ if (!location.hash && !params.questionId) {
+ return (
+ <div className="p4 text-centered flex-full">
+ Visit <strong>/_internal/question/:id</strong> or{" "}
+ <strong>/_internal/question#:hash</strong>.
+ </div>
+ );
+ }
+ return (
+ <QuestionAndResultLoader
+ questionHash={location.hash}
+ questionId={params.questionId ? parseInt(params.questionId) : null}
+ >
+ {({ rawSeries }) =>
+ rawSeries && (
+ <Visualization className="flex-full" rawSeries={rawSeries} />
+ )
+ }
+ </QuestionAndResultLoader>
+ );
+ }
+}
+
+QuestionApp.routes = [
+ <Route path="question" component={QuestionApp} />,
+ <Route path="question/:questionId" component={QuestionApp} />,
+];
diff --git a/frontend/src/metabase/internal/routes.js b/frontend/src/metabase/internal/routes.js
index cefece31d1c..eb2d40d213c 100644
--- a/frontend/src/metabase/internal/routes.js
+++ b/frontend/src/metabase/internal/routes.js
@@ -1,15 +1,20 @@
+/* @flow */
+
import React from "react";
import { Route, IndexRoute } from "react-router";
-import IconsApp from "metabase/internal/components/IconsApp";
-import ColorsApp from "metabase/internal/components/ColorsApp";
-import ComponentsApp from "metabase/internal/components/ComponentsApp";
+// $FlowFixMe: doesn't know about require.context
+const req = require.context(
+ "metabase/internal/components",
+ true,
+ /(\w+)App.jsx$/,
+);
-const PAGES = {
- Icons: IconsApp,
- Colors: ColorsApp,
- Components: ComponentsApp,
-};
+const PAGES = {};
+for (const key of req.keys()) {
+ const name = key.match(/(\w+)App.jsx$/)[1];
+ PAGES[name] = req(key).default;
+}
const WelcomeApp = () => {
return (
@@ -49,13 +54,12 @@ const InternalLayout = ({ children }) => {
export default (
<Route component={InternalLayout}>
<IndexRoute component={WelcomeApp} />
- {Object.entries(PAGES).map(([name, Component]) => (
- <Route path={name.toLowerCase()} component={Component} />
- ))}
- <Route path="components/:componentName" component={ComponentsApp} />
- <Route
- path="components/:componentName/:exampleName"
- component={ComponentsApp}
- />
+ {Object.entries(PAGES).map(
+ ([name, Component]) =>
+ Component &&
+ (Component.routes || (
+ <Route path={name.toLowerCase()} component={Component} />
+ )),
+ )}
</Route>
);
diff --git a/frontend/src/metabase/meta/types/Visualization.js b/frontend/src/metabase/meta/types/Visualization.js
index e59453f964c..860bc38b6df 100644
--- a/frontend/src/metabase/meta/types/Visualization.js
+++ b/frontend/src/metabase/meta/types/Visualization.js
@@ -68,8 +68,11 @@ export type ClickActionPopoverProps = {
};
export type SingleSeries = { card: Card, data: DatasetData };
-export type Series = SingleSeries[] & { _raw: Series };
+export type RawSeries = SingleSeries[];
+export type TransformedSeries = RawSeries & { _raw: Series };
+export type Series = RawSeries | TransformedSeries;
+// These are the props provided to the visualization implementations BY the Visualization component
export type VisualizationProps = {
series: Series,
card: Card,
diff --git a/frontend/src/metabase/visualizations/components/Visualization.jsx b/frontend/src/metabase/visualizations/components/Visualization.jsx
index 682d2a1b360..f97f8e77419 100644
--- a/frontend/src/metabase/visualizations/components/Visualization.jsx
+++ b/frontend/src/metabase/visualizations/components/Visualization.jsx
@@ -46,12 +46,13 @@ import type {
HoverObject,
ClickObject,
Series,
+ RawSeries,
OnChangeCardAndRun,
} from "metabase/meta/types/Visualization";
import Metadata from "metabase-lib/lib/metadata/Metadata";
type Props = {
- rawSeries: Series,
+ rawSeries: RawSeries,
className: string,
diff --git a/frontend/src/metabase/visualizations/index.js b/frontend/src/metabase/visualizations/index.js
index 492ae0a8966..7a62db9110d 100644
--- a/frontend/src/metabase/visualizations/index.js
+++ b/frontend/src/metabase/visualizations/index.js
@@ -70,7 +70,6 @@ export function getVisualizationTransformed(series: Series) {
series = CardVisualization.transformSeries(series);
}
if (series !== lastSeries) {
- // $FlowFixMe
series = [...series];
// $FlowFixMe
series._raw = lastSeries;
diff --git a/frontend/src/metabase/visualizations/visualizations/Progress.jsx b/frontend/src/metabase/visualizations/visualizations/Progress.jsx
index ee312a55d51..ee0a439cd41 100644
--- a/frontend/src/metabase/visualizations/visualizations/Progress.jsx
+++ b/frontend/src/metabase/visualizations/visualizations/Progress.jsx
@@ -110,7 +110,8 @@ export default class Progress extends Component {
onVisualizationClick,
visualizationIsClickable,
} = this.props;
- const value: number = rows[0][0];
+ const value: number =
+ rows[0] && typeof rows[0][0] === "number" ? rows[0][0] : 0;
const goal = settings["progress.goal"] || 0;
const mainColor = settings["progress.color"];
diff --git a/frontend/test/containers/AdHocQuestionLoader.unit.spec.js b/frontend/test/containers/AdHocQuestionLoader.unit.spec.js
new file mode 100644
index 00000000000..f9cc2d0e5c7
--- /dev/null
+++ b/frontend/test/containers/AdHocQuestionLoader.unit.spec.js
@@ -0,0 +1,76 @@
+import React from "react";
+import { shallow, mount } from "enzyme";
+
+import Question from "metabase-lib/lib/Question";
+import { delay } from "metabase/lib/promise";
+
+// import the un-connected component so we can test its internal logic sans
+// redux
+import { AdHocQuestionLoader } from "metabase/containers/AdHocQuestionLoader";
+
+describe("AdHocQuestionLoader", () => {
+ let loadQuestionSpy, loadMetadataSpy, mockChild;
+ beforeEach(() => {
+ // reset mocks between tests so we have fresh spies, etc
+ jest.resetAllMocks();
+ mockChild = jest.fn().mockReturnValue(<div />);
+ loadMetadataSpy = jest.fn();
+ loadQuestionSpy = jest.spyOn(
+ AdHocQuestionLoader.prototype,
+ "_loadQuestion",
+ );
+ });
+
+ it("should load a question given a questionHash", async () => {
+ const q = new Question.create({ databaseId: 1, tableId: 2 });
+ const questionHash = q.getUrl().match(/(#.*)/)[1];
+
+ const wrapper = mount(
+ <AdHocQuestionLoader
+ questionHash={questionHash}
+ loadMetadataForCard={loadMetadataSpy}
+ children={mockChild}
+ />,
+ );
+ expect(mockChild.mock.calls[0][0].loading).toEqual(true);
+ expect(mockChild.mock.calls[0][0].error).toEqual(null);
+
+ // stuff happens asynchronously
+ wrapper.update();
+ await delay(0);
+
+ expect(loadMetadataSpy.mock.calls[0][0]).toEqual(q.card());
+
+ const calls = mockChild.mock.calls;
+ const { question, loading, error } = calls[calls.length - 1][0];
+ expect(question.card()).toEqual(q.card());
+ expect(loading).toEqual(false);
+ expect(error).toEqual(null);
+ });
+
+ it("should load a new question if the question hash changes", () => {
+ // create some junk strigs, real question hashes are more ludicrous but this
+ // is easier to verify
+ const originalQuestionHash = "#abc123";
+ const newQuestionHash = "#def456";
+
+ const wrapper = shallow(
+ <AdHocQuestionLoader
+ questionHash={originalQuestionHash}
+ loadMetadataForCard={loadMetadataSpy}
+ children={mockChild}
+ />,
+ );
+
+ expect(loadQuestionSpy).toHaveBeenCalledWith(originalQuestionHash);
+
+ // update the question hash, a new location.hash in the url would most
+ // likely do this
+ wrapper.setProps({ questionHash: newQuestionHash });
+
+ // question loading should begin with the new ID
+ expect(loadQuestionSpy).toHaveBeenCalledWith(newQuestionHash);
+
+ expect(mockChild).toHaveBeenCalled();
+ });
+});
diff --git a/frontend/test/containers/QuestionAndResultLoader.unit.spec.js b/frontend/test/containers/QuestionAndResultLoader.unit.spec.js
new file mode 100644
index 00000000000..421df4519b5
--- /dev/null
+++ b/frontend/test/containers/QuestionAndResultLoader.unit.spec.js
@@ -0,0 +1,10 @@
+import React from "react";
+import { shallow } from "enzyme";
+
+import QuestionAndResultLoader from "metabase/containers/QuestionAndResultLoader";
+
+describe("QuestionAndResultLoader", () => {
+ it("should load a question and a result", () => {
+ shallow(<QuestionAndResultLoader>{() => <div />}</QuestionAndResultLoader>);
+ });
+});
diff --git a/frontend/test/containers/QuestionLoader.unit.spec.js b/frontend/test/containers/QuestionLoader.unit.spec.js
new file mode 100644
index 00000000000..0a548971c63
--- /dev/null
+++ b/frontend/test/containers/QuestionLoader.unit.spec.js
@@ -0,0 +1,46 @@
+import React from "react";
+import { shallow } from "enzyme";
+
+import QuestionLoader from "metabase/containers/QuestionLoader";
+
+import AdHocQuestionLoader from "metabase/containers/AdHocQuestionLoader";
+import SavedQuestionLoader from "metabase/containers/SavedQuestionLoader";
+
+describe("QuestionLoader", () => {
+ describe("initial load", () => {
+ it("should use SavedQuestionLoader if there is a saved question", () => {
+ const wrapper = shallow(
+ <QuestionLoader questionId={1}>{() => <div />}</QuestionLoader>,
+ );
+
+ expect(wrapper.find(SavedQuestionLoader).length).toBe(1);
+ });
+
+ it("should use AdHocQuestionLoader if there is an ad-hoc question", () => {
+ const wrapper = shallow(
+ <QuestionLoader questionHash={"#abc123"}>
+ {() => <div />}
+ </QuestionLoader>,
+ );
+
+ expect(wrapper.find(AdHocQuestionLoader).length).toBe(1);
+ });
+ });
+ describe("subsequent movement", () => {
+ it("should transition between loaders when props change", () => {
+ // start with a quesitonId
+ const wrapper = shallow(
+ <QuestionLoader questionId={4}>{() => <div />}</QuestionLoader>,
+ );
+
+ expect(wrapper.find(SavedQuestionLoader).length).toBe(1);
+
+ wrapper.setProps({
+ questionId: undefined,
+ questionHash: "#abc123",
+ });
+
+ expect(wrapper.find(AdHocQuestionLoader).length).toBe(1);
+ });
+ });
+});
diff --git a/frontend/test/containers/QuestionResultLoader.unit.spec.js b/frontend/test/containers/QuestionResultLoader.unit.spec.js
new file mode 100644
index 00000000000..020d93dbe86
--- /dev/null
+++ b/frontend/test/containers/QuestionResultLoader.unit.spec.js
@@ -0,0 +1,22 @@
+import React from "react";
+import { shallow } from "enzyme";
+
+import { QuestionResultLoader } from "metabase/containers/QuestionResultLoader";
+
+describe("QuestionResultLoader", () => {
+ it("should load a result given a question", () => {
+ const question = {
+ id: 1,
+ };
+
+ const loadSpy = jest.spyOn(QuestionResultLoader.prototype, "_loadResult");
+
+ shallow(
+ <QuestionResultLoader question={question}>
+ {() => <div />}
+ </QuestionResultLoader>,
+ );
+
+ expect(loadSpy).toHaveBeenCalledWith(question);
+ });
+});
diff --git a/frontend/test/containers/SavedQuestionLoader.unit.spec.js b/frontend/test/containers/SavedQuestionLoader.unit.spec.js
new file mode 100644
index 00000000000..d41f8d0d9c1
--- /dev/null
+++ b/frontend/test/containers/SavedQuestionLoader.unit.spec.js
@@ -0,0 +1,76 @@
+import React from "react";
+import { shallow, mount } from "enzyme";
+
+import Question from "metabase-lib/lib/Question";
+import { delay } from "metabase/lib/promise";
+import { CardApi } from "metabase/services";
+
+// import the un-connected component so we can test its internal logic sans
+// redux
+import { SavedQuestionLoader } from "metabase/containers/SavedQuestionLoader";
+
+// we need to mock the things that try and actually load the question
+jest.mock("metabase/services");
+
+describe("SavedQuestionLoader", () => {
+ let loadQuestionSpy, loadMetadataSpy, mockChild;
+ beforeEach(() => {
+ // reset mocks between tests so we have fresh spies, etc
+ jest.resetAllMocks();
+ mockChild = jest.fn().mockReturnValue(<div />);
+ loadMetadataSpy = jest.fn();
+ loadQuestionSpy = jest.spyOn(
+ SavedQuestionLoader.prototype,
+ "_loadQuestion",
+ );
+ });
+
+ it("should load a question given a questionId", async () => {
+ const questionId = 1;
+ const q = new Question.create({ databaseId: 1, tableId: 2 });
+ jest.spyOn(CardApi, "get").mockReturnValue(q.card());
+
+ const wrapper = mount(
+ <SavedQuestionLoader
+ questionId={questionId}
+ loadMetadataForCard={loadMetadataSpy}
+ children={mockChild}
+ />,
+ );
+ expect(mockChild.mock.calls[0][0].loading).toEqual(true);
+ expect(mockChild.mock.calls[0][0].error).toEqual(null);
+
+ // stuff happens asynchronously
+ wrapper.update();
+ await delay(0);
+
+ expect(loadQuestionSpy).toHaveBeenCalledWith(questionId);
+
+ const calls = mockChild.mock.calls;
+ const { question, loading, error } = calls[calls.length - 1][0];
+ expect(question.card()).toEqual(q.card());
+ expect(loading).toEqual(false);
+ expect(error).toEqual(null);
+ });
+
+ it("should load a new question if the question ID changes", () => {
+ const originalQuestionId = 1;
+ const newQuestionId = 2;
+
+ const wrapper = shallow(
+ <SavedQuestionLoader
+ questionId={originalQuestionId}
+ loadMetadataForCard={loadMetadataSpy}
+ children={mockChild}
+ />,
+ );
+
+ expect(loadQuestionSpy).toHaveBeenCalledWith(originalQuestionId);
+
+ // update the question ID, a new question id param in the url would do this
+ wrapper.setProps({ questionId: newQuestionId });
+
+ // question loading should begin with the new ID
+ expect(loadQuestionSpy).toHaveBeenCalledWith(newQuestionId);
+ });
+});
--
GitLab