diff --git a/enterprise/frontend/src/embedding-sdk/components/public/StaticQuestion/StaticQuestion.tsx b/enterprise/frontend/src/embedding-sdk/components/public/StaticQuestion/StaticQuestion.tsx index f53b5f1dc84d15c102dcf7795368d553939d199e..12ce4fe6bc66c97f634bb3615c1b409e15c57b29 100644 --- a/enterprise/frontend/src/embedding-sdk/components/public/StaticQuestion/StaticQuestion.tsx +++ b/enterprise/frontend/src/embedding-sdk/components/public/StaticQuestion/StaticQuestion.tsx @@ -1,5 +1,4 @@ import cx from "classnames"; -import { useEffect, useState } from "react"; import { t } from "ttag"; import { @@ -7,11 +6,10 @@ import { SdkLoader, withPublicComponentWrapper, } from "embedding-sdk/components/private/PublicComponentWrapper"; +import { useLoadStaticQuestion } from "embedding-sdk/hooks/private/use-load-static-question"; import { getDefaultVizHeight } from "embedding-sdk/lib/default-height"; -import { loadStaticQuestion } from "embedding-sdk/lib/load-static-question"; import CS from "metabase/css/core/index.css"; import { useValidatedEntityId } from "metabase/lib/entity-id/hooks/use-validated-entity-id"; -import type { GenericErrorResponse } from "metabase/lib/errors"; import { getResponseErrorMessage } from "metabase/lib/errors"; import { useSelector } from "metabase/lib/redux"; import QueryVisualization from "metabase/query_builder/components/QueryVisualization"; @@ -23,7 +21,7 @@ import { getMetadata } from "metabase/selectors/metadata"; import { Box, Group } from "metabase/ui"; import { PublicMode } from "metabase/visualizations/click-actions/modes/PublicMode"; import Question from "metabase-lib/v1/Question"; -import type { Card, CardEntityId, CardId, Dataset } from "metabase-types/api"; +import type { CardEntityId, CardId, Dataset } from "metabase-types/api"; export type StaticQuestionProps = { questionId: CardId | CardEntityId; @@ -32,13 +30,6 @@ export type StaticQuestionProps = { parameterValues?: Record<string, string | number>; }; -type State = { - loading: boolean; - card: Card | null; - result: Dataset | null; - error: GenericErrorResponse | null; -}; - type StaticQuestionVisualizationSelectorProps = { question: Question; result: Dataset | null; @@ -87,63 +78,8 @@ const StaticQuestionInner = ({ const metadata = useSelector(getMetadata); - const [{ loading, card, result, error }, setState] = useState<State>({ - loading: false, - card: null, - result: null, - error: null, - }); - - useEffect(() => { - async function loadCardData() { - setState(prevState => ({ - ...prevState, - loading: true, - })); - - if (!questionId) { - return; - } - - try { - const { card, result } = await loadStaticQuestion({ - questionId, - parameterValues, - }); - - setState(prevState => ({ - ...prevState, - card, - result, - loading: false, - error: null, - })); - } catch (error) { - if (typeof error === "object") { - setState(prevState => ({ - ...prevState, - result: null, - card: null, - loading: false, - error, - })); - } else { - console.error("error loading static question", error); - } - } - } - - loadCardData(); - }, [questionId, parameterValues]); - - const changeVisualization = (newQuestion: Question) => { - setState({ - card: newQuestion.card(), - result: result, - loading: false, - error: null, - }); - }; + const { card, loading, result, error, updateQuestion } = + useLoadStaticQuestion(questionId, parameterValues); const isLoading = loading || (!result && !error) || isValidatingEntityId; @@ -173,7 +109,7 @@ const StaticQuestionInner = ({ <StaticQuestionVisualizationSelector question={question} result={result} - onUpdateQuestion={changeVisualization} + onUpdateQuestion={updateQuestion} /> )} <QueryVisualization diff --git a/enterprise/frontend/src/embedding-sdk/components/public/StaticQuestion/StaticQuestion.unit.spec.tsx b/enterprise/frontend/src/embedding-sdk/components/public/StaticQuestion/StaticQuestion.unit.spec.tsx index 9901398f3f31a16be649b4c093f60a8923e168fa..2a595bb5bc2a51a7d3c948584a2802575e59a43a 100644 --- a/enterprise/frontend/src/embedding-sdk/components/public/StaticQuestion/StaticQuestion.unit.spec.tsx +++ b/enterprise/frontend/src/embedding-sdk/components/public/StaticQuestion/StaticQuestion.unit.spec.tsx @@ -1,3 +1,4 @@ +import { act } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import fetchMock from "fetch-mock"; @@ -85,7 +86,7 @@ const setup = ({ setupCardQueryEndpoints(card, TEST_DATASET); - renderWithProviders( + return renderWithProviders( <StaticQuestion questionId={TEST_QUESTION_ID} showVisualizationSelector={showVisualizationSelector} @@ -174,4 +175,19 @@ describe("StaticQuestion", () => { value: 1024, }); }); + + it("should cancel the request when the component unmounts", async () => { + const abortSpy = jest.spyOn(AbortController.prototype, "abort"); + + const { unmount } = setup(); + + await act(async () => unmount()); + + expect(abortSpy).toHaveBeenCalled(); + abortSpy.mockRestore(); + + // sanity check that the two requests were made initially + expect(fetchMock.calls(`path:/api/card/1`).length).toBe(1); + expect(fetchMock.calls(`path:/api/card/1/query`).length).toBe(1); + }); }); diff --git a/enterprise/frontend/src/embedding-sdk/hooks/private/use-load-static-question.ts b/enterprise/frontend/src/embedding-sdk/hooks/private/use-load-static-question.ts new file mode 100644 index 0000000000000000000000000000000000000000..3abe084469d3c4298ae5264bf6478f0b4a90bf1a --- /dev/null +++ b/enterprise/frontend/src/embedding-sdk/hooks/private/use-load-static-question.ts @@ -0,0 +1,81 @@ +import { useEffect, useState } from "react"; + +import { loadStaticQuestion } from "embedding-sdk/lib/load-static-question"; +import type { GenericErrorResponse } from "metabase/lib/errors"; +import { defer } from "metabase/lib/promise"; +import type Question from "metabase-lib/v1/Question"; +import type { Card, Dataset } from "metabase-types/api"; + +type QuestionState = { + loading: boolean; + card: Card | null; + result: Dataset | null; + error: GenericErrorResponse | null; +}; + +export function useLoadStaticQuestion( + questionId: number | null, + parameterValues?: Record<string, string | number>, +) { + const [questionState, setQuestionState] = useState<QuestionState>({ + loading: false, + card: null, + result: null, + error: null, + }); + + const updateQuestion = (newQuestion: Question) => + setQuestionState(state => ({ + ...state, + card: newQuestion.card(), + loading: false, + error: null, + })); + + useEffect(() => { + const cancelDeferred = defer(); + + async function loadCardData() { + setQuestionState(state => ({ ...state, loading: true })); + + if (!questionId) { + return; + } + + try { + const { card, result } = await loadStaticQuestion({ + questionId, + parameterValues, + cancelDeferred, + }); + + setQuestionState({ + card, + result, + loading: false, + error: null, + }); + } catch (error) { + if (typeof error === "object") { + setQuestionState({ + result: null, + card: null, + loading: false, + error, + }); + } else { + console.error("error loading static question", error); + } + } + } + + loadCardData(); + + return () => { + // cancel pending requests upon unmount + cancelDeferred.resolve(); + }; + }, [questionId, parameterValues]); + + return { ...questionState, updateQuestion }; +} diff --git a/enterprise/frontend/src/embedding-sdk/lib/load-static-question.ts b/enterprise/frontend/src/embedding-sdk/lib/load-static-question.ts index ab4bd19044450bc34db2644f7d8fceba99383f2c..d29c7da86f5b7a92fce93df52ab3920f30e06346 100644 --- a/enterprise/frontend/src/embedding-sdk/lib/load-static-question.ts +++ b/enterprise/frontend/src/embedding-sdk/lib/load-static-question.ts @@ -1,24 +1,28 @@ +import type { Deferred } from "metabase/lib/promise"; import { CardApi } from "metabase/services"; import type { Card, Dataset, ParameterQueryObject } from "metabase-types/api"; interface Options { questionId: number; parameterValues?: Record<string, string | number>; + cancelDeferred?: Deferred; } type ParameterQueryInput = { id: string } & ParameterQueryObject; export async function loadStaticQuestion(options: Options) { - const { questionId, parameterValues } = options; + const { questionId, parameterValues, cancelDeferred } = options; let card: Card | null; let result: Dataset | null; + const cancelled = cancelDeferred?.promise; + [card, result] = await Promise.all([ - CardApi.get({ cardId: questionId }), + CardApi.get({ cardId: questionId }, { cancelled }), // Query the card in parallel when no parameters are provided. - !parameterValues && CardApi.query({ cardId: questionId }), + !parameterValues && CardApi.query({ cardId: questionId }, { cancelled }), ]); if (parameterValues && card?.parameters) { @@ -31,10 +35,10 @@ export async function loadStaticQuestion(options: Options) { value: parameterValues[parameter.slug], })); - result = await CardApi.query({ - cardId: questionId, - parameters, - }); + result = await CardApi.query( + { cardId: questionId, parameters }, + { cancelled }, + ); } return { card, result };