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

frontend/src/metabase/containers/AdHocQuestionLoader.jsx

+/* @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,
+);

frontend/src/metabase/containers/QuestionAndResultLoader.jsx

+/* @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;

frontend/src/metabase/containers/QuestionLoader.jsx

+/* @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;

frontend/src/metabase/containers/QuestionResultLoader.jsx

+/* @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;

frontend/src/metabase/containers/SavedQuestionLoader.jsx

+/* @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,
+);

frontend/src/metabase/internal/components/ColorsApp.jsx

+/* @flow */
+
 import React from "react";
 
 import cx from "classnames";

frontend/src/metabase/internal/components/ComponentsApp.jsx

+/* @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}
+  />,
+];

frontend/src/metabase/internal/components/IconsApp.jsx

+/* @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 (

frontend/src/metabase/internal/components/QuestionApp.jsx

+/* @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} />,
+];

frontend/src/metabase/internal/routes.js

+/* @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>
 );

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,

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,
 

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;

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"];

frontend/test/containers/AdHocQuestionLoader.unit.spec.js

+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();
+  });
+});

frontend/test/containers/QuestionAndResultLoader.unit.spec.js

+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>);
+  });
+});

frontend/test/containers/QuestionLoader.unit.spec.js

+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);
+    });
+  });
+});

frontend/test/containers/QuestionResultLoader.unit.spec.js

+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);
+  });
+});

frontend/test/containers/SavedQuestionLoader.unit.spec.js

+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);
+  });
+});