diff --git a/e2e/test/scenarios/visualizations-tabular/table.cy.spec.js b/e2e/test/scenarios/visualizations-tabular/table.cy.spec.js index bada9fc232a7ae850bd4a228e7d1e3119aa5b2f3..09b73e718d12489fbcf8d27a33f4709175d3394f 100644 --- a/e2e/test/scenarios/visualizations-tabular/table.cy.spec.js +++ b/e2e/test/scenarios/visualizations-tabular/table.cy.spec.js @@ -326,6 +326,25 @@ describe("scenarios > visualizations > table", () => { expect(isScrollableHorizontally($popover[0])).to.be.false; }); }); + + it("should show the slow loading text when the query is taking too long", () => { + openOrdersTable({ mode: "notebook" }); + + cy.intercept("POST", "/api/dataset", req => { + req.on("response", res => { + res.setDelay(10000); + }); + }); + + cy.button("Visualize").click(); + + cy.clock(); + cy.tick(1000); + cy.findByTestId("query-builder-main").findByText("Doing science..."); + + cy.tick(5000); + cy.findByTestId("query-builder-main").findByText("Waiting for results..."); + }); }); describe("scenarios > visualizations > table > conditional formatting", () => { diff --git a/enterprise/frontend/src/metabase-enterprise/settings/selectors.ts b/enterprise/frontend/src/metabase-enterprise/settings/selectors.ts index 668a6a4290ca047f857c6c72c59bbd7115abeaba..c9d1d07ce4d7c09aca6646230af1ed99ef15ffc7 100644 --- a/enterprise/frontend/src/metabase-enterprise/settings/selectors.ts +++ b/enterprise/frontend/src/metabase-enterprise/settings/selectors.ts @@ -17,8 +17,9 @@ const getCustomLogoUrl = (settingValues: EnterpriseSettings) => { export const getLogoUrl = (state: EnterpriseState) => getCustomLogoUrl(getSettings(state)); -export const getLoadingMessage = (state: EnterpriseState) => - LOADING_MESSAGE_BY_SETTING[getSetting(state, "loading-message")]; +export const getLoadingMessage = (state: EnterpriseState) => { + return LOADING_MESSAGE_BY_SETTING[getSetting(state, "loading-message")].value; +}; // eslint-disable-next-line no-literal-metabase-strings -- This is a Metabase string we want to keep. It's used for comparison. const DEFAULT_APPLICATION_NAME = "Metabase"; diff --git a/enterprise/frontend/src/metabase-enterprise/settings/selectors.unit.spec.ts b/enterprise/frontend/src/metabase-enterprise/settings/selectors.unit.spec.ts index 4605cca0a80fcc6c0ae3bd1d93a9fa672b22221b..dc10c95c0a943c2c4978df459abf134fb0791795 100644 --- a/enterprise/frontend/src/metabase-enterprise/settings/selectors.unit.spec.ts +++ b/enterprise/frontend/src/metabase-enterprise/settings/selectors.unit.spec.ts @@ -50,7 +50,7 @@ describe("getLoadingMessage", () => { const expectedLoadingMessage = "Doing science..."; - expect(getLoadingMessage(states)).toBe(expectedLoadingMessage); + expect(getLoadingMessage(states)(false)).toBe(expectedLoadingMessage); }); it('should show correct loading message when "loading-message" is set to "running-query"', () => { @@ -63,7 +63,7 @@ describe("getLoadingMessage", () => { const expectedLoadingMessage = "Loading results..."; (""); - expect(getLoadingMessage(states)).toBe(expectedLoadingMessage); + expect(getLoadingMessage(states)(false)).toBe(expectedLoadingMessage); }); it('should show correct loading message when "loading-message" is set to "loading-results"', () => { @@ -75,7 +75,7 @@ describe("getLoadingMessage", () => { const expectedLoadingMessage = "Running query..."; - expect(getLoadingMessage(states)).toBe(expectedLoadingMessage); + expect(getLoadingMessage(states)(false)).toBe(expectedLoadingMessage); }); }); diff --git a/enterprise/frontend/src/metabase-enterprise/whitelabel/lib/loading-message.ts b/enterprise/frontend/src/metabase-enterprise/whitelabel/lib/loading-message.ts index 190652ff2cd8af84c0639769ce21f91d337dfa2a..cc0534aa7ca74c1b3de14a53f9de930f69dc7dd5 100644 --- a/enterprise/frontend/src/metabase-enterprise/whitelabel/lib/loading-message.ts +++ b/enterprise/frontend/src/metabase-enterprise/whitelabel/lib/loading-message.ts @@ -1,13 +1,23 @@ import { t } from "ttag"; export const LOADING_MESSAGE_BY_SETTING = { - "doing-science": t`Doing science...`, - "running-query": t`Running query...`, - "loading-results": t`Loading results...`, + "doing-science": { + name: t`Doing science...`, + value: (isSlow: boolean) => + isSlow ? t`Waiting for results...` : t`Doing science...`, + }, + "running-query": { + name: t`Running query...`, + value: (_: boolean) => t`Running query...`, + }, + "loading-results": { + name: t`Loading results...`, + value: (_: boolean) => t`Loading results...`, + }, }; export const getLoadingMessageOptions = () => - Object.entries(LOADING_MESSAGE_BY_SETTING).map(([value, name]) => ({ + Object.entries(LOADING_MESSAGE_BY_SETTING).map(([value, option]) => ({ + name: option.name, value, - name, })); diff --git a/frontend/src/metabase/metabot/components/MetabotQueryBuilder/MetabotQueryBuilder.tsx b/frontend/src/metabase/metabot/components/MetabotQueryBuilder/MetabotQueryBuilder.tsx index 48a09940292f93b3314ae4a8bf7c1ee509f71812..7bccda9cbde23f962b8290d9d17ae4ef397f2b6a 100644 --- a/frontend/src/metabase/metabot/components/MetabotQueryBuilder/MetabotQueryBuilder.tsx +++ b/frontend/src/metabase/metabot/components/MetabotQueryBuilder/MetabotQueryBuilder.tsx @@ -2,7 +2,6 @@ import { connect } from "react-redux"; import { t } from "ttag"; import { getResponseErrorMessage } from "metabase/lib/errors"; -import { getWhiteLabeledLoadingMessage } from "metabase/selectors/whitelabel"; import type { Dataset } from "metabase-types/api"; import type { MetabotQueryStatus, State } from "metabase-types/store"; @@ -28,7 +27,6 @@ import { } from "./MetabotQueryBuilder.styled"; interface StateProps { - loadingMessage: string; queryStatus: MetabotQueryStatus; queryResults: [Dataset] | null; queryError: unknown; @@ -38,7 +36,6 @@ interface StateProps { type MetabotQueryBuilderProps = StateProps; const mapStateToProps = (state: State): StateProps => ({ - loadingMessage: getWhiteLabeledLoadingMessage(state), queryStatus: getQueryStatus(state), queryResults: getQueryResults(state), queryError: getQueryError(state), @@ -46,7 +43,6 @@ const mapStateToProps = (state: State): StateProps => ({ }); const MetabotQueryBuilder = ({ - loadingMessage, queryStatus, queryResults, queryError, @@ -60,7 +56,7 @@ const MetabotQueryBuilder = ({ <QueryBuilderRoot> {hasResults && <MetabotQueryEditor />} {isRunning ? ( - <QueryRunningState loadingMessage={loadingMessage} /> + <RunningStateRoot /> ) : hasErrors ? ( <QueryErrorState queryError={queryError} /> ) : hasResults ? ( @@ -81,14 +77,6 @@ const QueryIdleState = () => { ); }; -interface QueryRunningStateProps { - loadingMessage: string; -} - -const QueryRunningState = ({ loadingMessage }: QueryRunningStateProps) => { - return <RunningStateRoot loadingMessage={loadingMessage} />; -}; - interface QueryErrorStateProps { queryError: unknown; } diff --git a/frontend/src/metabase/plugins/index.ts b/frontend/src/metabase/plugins/index.ts index 600d387d6d94f0689feaa0741f91f5c7c6a97168..703d82c94a7f0b53c7e68debf59b10b664902ed0 100644 --- a/frontend/src/metabase/plugins/index.ts +++ b/frontend/src/metabase/plugins/index.ts @@ -133,7 +133,8 @@ export const PLUGIN_IS_PASSWORD_USER: ((user: User) => boolean)[] = []; // selectors that customize behavior between app versions export const PLUGIN_SELECTORS = { canWhitelabel: (_state: State) => false, - getLoadingMessage: (_state: State) => t`Doing science...`, + getLoadingMessage: (_state: State) => (isSlow: boolean) => + isSlow ? t`Waiting for results...` : t`Doing science...`, getIsWhiteLabeling: (_state: State) => false, // eslint-disable-next-line no-literal-metabase-strings -- This is the actual Metabase name, so we don't want to translate it. getApplicationName: (_state: State) => "Metabase", diff --git a/frontend/src/metabase/query_builder/components/QueryVisualization.jsx b/frontend/src/metabase/query_builder/components/QueryVisualization.jsx index 2031d2f0ea32678148d6e4e1754414579cbe6650..11fc11c62f10213a4fa116828b60e59eb853a5c4 100644 --- a/frontend/src/metabase/query_builder/components/QueryVisualization.jsx +++ b/frontend/src/metabase/query_builder/components/QueryVisualization.jsx @@ -1,9 +1,12 @@ /* eslint-disable react/prop-types */ import cx from "classnames"; -import { Component } from "react"; +import { useState } from "react"; +import { useTimeout } from "react-use"; import { t } from "ttag"; import LoadingSpinner from "metabase/components/LoadingSpinner"; +import { useSelector } from "metabase/lib/redux"; +import { getWhiteLabeledLoadingMessage } from "metabase/selectors/whitelabel"; import { HARD_ROW_LIMIT } from "metabase-lib/queries/utils"; import RunButtonWithTooltip from "./RunButtonWithTooltip"; @@ -11,85 +14,63 @@ import { VisualizationError } from "./VisualizationError"; import VisualizationResult from "./VisualizationResult"; import Warnings from "./Warnings"; -export default class QueryVisualization extends Component { - constructor(props, context) { - super(props, context); - this.state = {}; - } +const SLOW_MESSAGE_TIMEOUT = 4000; - static defaultProps = { - // NOTE: this should be more dynamic from the backend, it's set based on the query lang - maxTableRows: HARD_ROW_LIMIT, - }; +export default function QueryVisualization(props) { + const { + className, + question, + isRunning, + isObjectDetail, + isResultDirty, + isNativeEditorOpen, + result, + maxTableRows = HARD_ROW_LIMIT, + } = props; - runQuery = () => { - const { isResultDirty } = this.props; - // ignore the cache if we're hitting "Refresh" (which we only show if isResultDirty = false) - this.props.runQuestionQuery({ ignoreCache: !isResultDirty }); - }; + const [warnings, setWarnings] = useState([]); - handleUpdateWarnings = warnings => { - this.setState({ warnings }); - }; - - render() { - const { - className, - question, - isRunning, - isObjectDetail, - isResultDirty, - isNativeEditorOpen, - result, - loadingMessage, - } = this.props; - - return ( - <div className={cx(className, "relative stacking-context full-height")}> - {isRunning ? ( - <VisualizationRunningState - className="spread z2" - loadingMessage={loadingMessage} - /> - ) : null} - <VisualizationDirtyState - {...this.props} - hidden={!isResultDirty || isRunning || isNativeEditorOpen} - className="spread z2" + return ( + <div className={cx(className, "relative stacking-context full-height")}> + {isRunning ? <VisualizationRunningState className="spread z2" /> : null} + <VisualizationDirtyState + {...props} + hidden={!isResultDirty || isRunning || isNativeEditorOpen} + className="spread z2" + /> + {!isObjectDetail && ( + <Warnings + warnings={warnings} + className="absolute top right mt2 mr2 z2" + size={18} /> - {!isObjectDetail && ( - <Warnings - warnings={this.state.warnings} - className="absolute top right mt2 mr2 z2" - size={18} + )} + <div + className={cx("spread Visualization z1", { + "Visualization--loading": isRunning, + })} + > + {result?.error ? ( + <VisualizationError + className="spread" + error={result.error} + via={result.via} + question={question} + duration={result.duration} /> - )} - <div - className={cx("spread Visualization z1", { - "Visualization--loading": isRunning, - })} - > - {result?.error ? ( - <VisualizationError - className="spread" - error={result.error} - via={result.via} - question={question} - duration={result.duration} - /> - ) : result?.data ? ( - <VisualizationResult - {...this.props} - className="spread" - onUpdateWarnings={this.handleUpdateWarnings} - /> - ) : !isRunning ? ( - <VisualizationEmptyState className="spread" /> - ) : null} - </div> + ) : result?.data ? ( + <VisualizationResult + {...props} + maxTableRows={maxTableRows} + className="spread" + onUpdateWarnings={setWarnings} + /> + ) : !isRunning ? ( + <VisualizationEmptyState className="spread" /> + ) : null} </div> - ); - } + </div> + ); } export const VisualizationEmptyState = ({ className }) => ( @@ -98,22 +79,29 @@ export const VisualizationEmptyState = ({ className }) => ( </div> ); -export const VisualizationRunningState = ({ - className = "", - loadingMessage, -}) => ( - <div - className={cx( - className, - "Loading flex flex-column layout-centered text-brand", - )} - > - <LoadingSpinner /> - <h2 className="Loading-message text-brand text-uppercase my3"> - {loadingMessage} - </h2> - </div> -); +export function VisualizationRunningState({ className = "" }) { + const [isSlow] = useTimeout(SLOW_MESSAGE_TIMEOUT); + + const loadingMessage = useSelector(getWhiteLabeledLoadingMessage); + + // show the slower loading message only when the loadingMessage is + // not customised + const message = loadingMessage(isSlow()); + + return ( + <div + className={cx( + className, + "Loading flex flex-column layout-centered text-brand", + )} + > + <LoadingSpinner /> + <h2 className="Loading-message text-brand text-uppercase my3"> + {message} + </h2> + </div> + ); +} export const VisualizationDirtyState = ({ className, diff --git a/frontend/src/metabase/query_builder/components/QueryVisualization.unit.spec.tsx b/frontend/src/metabase/query_builder/components/QueryVisualization.unit.spec.tsx new file mode 100644 index 0000000000000000000000000000000000000000..d4834a40c689293d2860c0e9a10ca8842c6b2a0c --- /dev/null +++ b/frontend/src/metabase/query_builder/components/QueryVisualization.unit.spec.tsx @@ -0,0 +1,41 @@ +import { renderWithProviders, screen } from "__support__/ui"; +import { PLUGIN_SELECTORS } from "metabase/plugins"; + +import { VisualizationRunningState } from "./QueryVisualization"; + +type SetupOpts = { + customMessage?: (isSlow: boolean) => string; +}; + +function setup({ customMessage }: SetupOpts = {}) { + if (customMessage) { + jest + .spyOn(PLUGIN_SELECTORS, "getLoadingMessage") + .mockImplementation(() => customMessage); + } + + renderWithProviders(<VisualizationRunningState />); +} + +describe("VisualizationRunningState", () => { + it("should render the different loading messages after a while", () => { + jest.useFakeTimers(); + + setup(); + expect(screen.getByText("Doing science...")).toBeInTheDocument(); + + jest.advanceTimersByTime(5000); + expect(screen.getByText("Waiting for results...")).toBeInTheDocument(); + }); + + it("should only render the custom loading message when it was customized", () => { + const customMessage = (isSlow: boolean) => + isSlow ? `Custom message (slow)...` : `Custom message...`; + + setup({ customMessage }); + expect(screen.getByText("Custom message...")).toBeInTheDocument(); + + jest.advanceTimersByTime(5000); + expect(screen.getByText("Custom message (slow)...")).toBeInTheDocument(); + }); +}); diff --git a/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx b/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx index 13165b9ffd24930e23df580de61aac1b9f720179..64c32b17180cfe133d6df2c77358d8b18969b713 100644 --- a/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx +++ b/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx @@ -26,7 +26,6 @@ import { getUserIsAdmin, canManageSubscriptions, } from "metabase/selectors/user"; -import { getWhiteLabeledLoadingMessage } from "metabase/selectors/whitelabel"; import * as actions from "../actions"; import View from "../components/view/View"; @@ -166,7 +165,6 @@ const mapStateToProps = (state, props) => { documentTitle: getDocumentTitle(state), pageFavicon: getPageFavicon(state), isLoadingComplete: getIsLoadingComplete(state), - loadingMessage: getWhiteLabeledLoadingMessage(state), reportTimezone: getSetting(state, "report-timezone-long"), diff --git a/frontend/src/metabase/selectors/whitelabel/tests/common.unit.spec.ts b/frontend/src/metabase/selectors/whitelabel/tests/common.unit.spec.ts index de6de6d3fc10eeec41c57781e02b316eb018071d..5b72bb1a5d1a912851ef3afe341053f6c7c7a394 100644 --- a/frontend/src/metabase/selectors/whitelabel/tests/common.unit.spec.ts +++ b/frontend/src/metabase/selectors/whitelabel/tests/common.unit.spec.ts @@ -12,19 +12,34 @@ describe("getWhiteLabeledLoadingMessage (OSS)", () => { it("should return 'Doing science...' when loading-message is set to 'doing-science'", () => { const { getState } = setup({ loadingMessage: "doing-science" }); - expect(getWhiteLabeledLoadingMessage(getState())).toBe("Doing science..."); + expect(getWhiteLabeledLoadingMessage(getState())(false)).toBe( + "Doing science...", + ); + expect(getWhiteLabeledLoadingMessage(getState())(true)).toBe( + "Waiting for results...", + ); }); it("should return 'Doing science...' when loading-message is set to 'loading-results'", () => { const { getState } = setup({ loadingMessage: "loading-results" }); - expect(getWhiteLabeledLoadingMessage(getState())).toBe("Doing science..."); + expect(getWhiteLabeledLoadingMessage(getState())(false)).toBe( + "Doing science...", + ); + expect(getWhiteLabeledLoadingMessage(getState())(true)).toBe( + "Waiting for results...", + ); }); it("should return 'Doing science...' when loading-message is set to 'running-query'", () => { const { getState } = setup({ loadingMessage: "running-query" }); - expect(getWhiteLabeledLoadingMessage(getState())).toBe("Doing science..."); + expect(getWhiteLabeledLoadingMessage(getState())(false)).toBe( + "Doing science...", + ); + expect(getWhiteLabeledLoadingMessage(getState())(true)).toBe( + "Waiting for results...", + ); }); }); diff --git a/frontend/src/metabase/selectors/whitelabel/tests/enterprise.unit.spec.ts b/frontend/src/metabase/selectors/whitelabel/tests/enterprise.unit.spec.ts index 7417f79774b703f610c0db19e6fe180fb98afa79..32c8a42486b23fdb5d5e14acc025a5d8f39e7e5e 100644 --- a/frontend/src/metabase/selectors/whitelabel/tests/enterprise.unit.spec.ts +++ b/frontend/src/metabase/selectors/whitelabel/tests/enterprise.unit.spec.ts @@ -17,19 +17,34 @@ describe("getWhiteLabeledLoadingMessage (EE without token)", () => { it("should return 'Doing science...' when loading-message is set to 'doing-science'", () => { const { getState } = setup({ loadingMessage: "doing-science" }); - expect(getWhiteLabeledLoadingMessage(getState())).toBe("Doing science..."); + expect(getWhiteLabeledLoadingMessage(getState())(false)).toBe( + "Doing science...", + ); + expect(getWhiteLabeledLoadingMessage(getState())(true)).toBe( + "Waiting for results...", + ); }); it("should return 'Doing science...' when loading-message is set to 'loading-results'", () => { const { getState } = setup({ loadingMessage: "loading-results" }); - expect(getWhiteLabeledLoadingMessage(getState())).toBe("Doing science..."); + expect(getWhiteLabeledLoadingMessage(getState())(false)).toBe( + "Doing science...", + ); + expect(getWhiteLabeledLoadingMessage(getState())(true)).toBe( + "Waiting for results...", + ); }); it("should return 'Doing science...' when loading-message is set to 'running-query'", () => { const { getState } = setup({ loadingMessage: "running-query" }); - expect(getWhiteLabeledLoadingMessage(getState())).toBe("Doing science..."); + expect(getWhiteLabeledLoadingMessage(getState())(false)).toBe( + "Doing science...", + ); + expect(getWhiteLabeledLoadingMessage(getState())(true)).toBe( + "Waiting for results...", + ); }); }); diff --git a/frontend/src/metabase/selectors/whitelabel/tests/premium.unit.spec.ts b/frontend/src/metabase/selectors/whitelabel/tests/premium.unit.spec.ts index db3c29eab40e15a09c11421515eb5356a07f3777..7b8fbb663c1b60c58f21d681718eb250e57d71bb 100644 --- a/frontend/src/metabase/selectors/whitelabel/tests/premium.unit.spec.ts +++ b/frontend/src/metabase/selectors/whitelabel/tests/premium.unit.spec.ts @@ -21,13 +21,21 @@ describe("getWhiteLabeledLoadingMessage (EE with token)", () => { it("should return 'Doing science...' when loading-message is set to 'doing-science'", () => { const { getState } = setup({ loadingMessage: "doing-science" }); - expect(getWhiteLabeledLoadingMessage(getState())).toBe("Doing science..."); + expect(getWhiteLabeledLoadingMessage(getState())(false)).toBe( + "Doing science...", + ); + expect(getWhiteLabeledLoadingMessage(getState())(true)).toBe( + "Waiting for results...", + ); }); it("should return 'Loading results...' when loading-message is set to 'loading-results'", () => { const { getState } = setup({ loadingMessage: "loading-results" }); - expect(getWhiteLabeledLoadingMessage(getState())).toBe( + expect(getWhiteLabeledLoadingMessage(getState())(false)).toBe( + "Loading results...", + ); + expect(getWhiteLabeledLoadingMessage(getState())(true)).toBe( "Loading results...", ); }); @@ -35,7 +43,12 @@ describe("getWhiteLabeledLoadingMessage (EE with token)", () => { it("should return 'Running query...' when loading-message is set to 'running-query'", () => { const { getState } = setup({ loadingMessage: "running-query" }); - expect(getWhiteLabeledLoadingMessage(getState())).toBe("Running query..."); + expect(getWhiteLabeledLoadingMessage(getState())(false)).toBe( + "Running query...", + ); + expect(getWhiteLabeledLoadingMessage(getState())(true)).toBe( + "Running query...", + ); }); });