From 4a717723ae7f5778b9e30e470f28e5806f301994 Mon Sep 17 00:00:00 2001
From: Dalton <daltojohnso@users.noreply.github.com>
Date: Tue, 30 Aug 2022 13:35:23 -0400
Subject: [PATCH] Use the `Questions` entity to fetch cards/questions for the
query builder (#25102)
* Make QB use Questions entity to fetch
* Pass down 'getState' to avoid HACK function
* Fix tests
---
.../containers/SavedQuestionLoader.jsx | 15 +++++---
frontend/src/metabase/lib/card.js | 10 ++++--
.../query_builder/actions/core/core.js | 6 ++--
.../actions/core/initializeQB.ts | 34 +++++++++++++++----
.../actions/core/initializeQB.unit.spec.ts | 6 ++++
.../SavedQuestionLoader.unit.spec.js | 4 +--
6 files changed, 57 insertions(+), 18 deletions(-)
diff --git a/frontend/src/metabase/containers/SavedQuestionLoader.jsx b/frontend/src/metabase/containers/SavedQuestionLoader.jsx
index 4ec739c4d12..3fcd9e813f1 100644
--- a/frontend/src/metabase/containers/SavedQuestionLoader.jsx
+++ b/frontend/src/metabase/containers/SavedQuestionLoader.jsx
@@ -2,12 +2,11 @@
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";
+import Questions from "metabase/entities/questions";
// type annotations
@@ -91,7 +90,7 @@ export class SavedQuestionLoader extends React.Component {
try {
this.setState({ loading: true, error: null });
// get the saved question via the card API
- const card = await CardApi.get({ cardId: questionId });
+ const card = await this.props.fetchQuestion(questionId);
// pass the retrieved card to load any necessary metadata
// (tables, source db, segments, etc) into
@@ -129,8 +128,14 @@ function mapStateToProps(state) {
};
}
-const mapDispatchToProps = {
- loadMetadataForCard,
+const mapDispatchToProps = dispatch => {
+ return {
+ loadMetadataForCard: card => dispatch(loadMetadataForCard(card)),
+ fetchQuestion: async id => {
+ const action = await dispatch(Questions.actions.fetch({ id }));
+ return Questions.HACK_getObjectFromAction(action);
+ },
+ };
};
export default connect(
diff --git a/frontend/src/metabase/lib/card.js b/frontend/src/metabase/lib/card.js
index 7af2667ef8e..469cd855782 100644
--- a/frontend/src/metabase/lib/card.js
+++ b/frontend/src/metabase/lib/card.js
@@ -2,8 +2,8 @@ import _ from "underscore";
import * as Q_DEPRECATED from "metabase/lib/query";
import Utils from "metabase/lib/utils";
-import { CardApi } from "metabase/services";
import { b64hash_to_utf8, utf8_to_b64url } from "metabase/lib/encoding";
+import Questions from "metabase/entities/questions";
export function createCard(name = null) {
return {
@@ -25,9 +25,13 @@ export function startNewCard(type, databaseId, tableId) {
// load a card either by ID or from a base64 serialization. if both are present then they are merged, which the serialized version taking precedence
// TODO: move to redux
-export async function loadCard(cardId) {
+export async function loadCard(cardId, { dispatch, getState }) {
try {
- return await CardApi.get({ cardId: cardId });
+ await dispatch(Questions.actions.fetch({ id: cardId }, { reload: true }));
+ const card = Questions.selectors.getObject(getState(), {
+ entityId: cardId,
+ });
+ return card;
} catch (error) {
console.log("error loading card", error);
throw error;
diff --git a/frontend/src/metabase/query_builder/actions/core/core.js b/frontend/src/metabase/query_builder/actions/core/core.js
index e2b7e1eda9f..9bcdbb5e0bd 100644
--- a/frontend/src/metabase/query_builder/actions/core/core.js
+++ b/frontend/src/metabase/query_builder/actions/core/core.js
@@ -96,7 +96,7 @@ export const setCardAndRun = (nextCard, shouldUpdateUrl = true) => {
const originalCard = card.original_card_id
? // If the original card id is present, dynamically load its information for showing lineage
- await loadCard(card.original_card_id)
+ await loadCard(card.original_card_id, { dispatch, getState })
: // Otherwise, use a current card as the original card if the card has been saved
// This is needed for checking whether the card is in dirty state or not
card.id
@@ -131,7 +131,9 @@ export const navigateToNewCardInsideQB = createThunkAction(
// Do not reload questions with breakouts when clicked on a legend item
} else if (cardIsEquivalent(previousCard, nextCard)) {
// This is mainly a fallback for scenarios where a visualization legend is clicked inside QB
- dispatch(setCardAndRun(await loadCard(nextCard.id)));
+ dispatch(
+ setCardAndRun(await loadCard(nextCard.id, { dispatch, getState })),
+ );
} else {
const card = getCardAfterVisualizationClick(nextCard, previousCard);
const url = Urls.serializedQuestion(card);
diff --git a/frontend/src/metabase/query_builder/actions/core/initializeQB.ts b/frontend/src/metabase/query_builder/actions/core/initializeQB.ts
index 0085108ace3..477e10f6d9b 100644
--- a/frontend/src/metabase/query_builder/actions/core/initializeQB.ts
+++ b/frontend/src/metabase/query_builder/actions/core/initializeQB.ts
@@ -14,6 +14,7 @@ import { getMetadata } from "metabase/selectors/metadata";
import { getUser } from "metabase/selectors/user";
import Snippets from "metabase/entities/snippets";
+import Questions from "metabase/entities/questions";
import { fetchAlertsForQuestion } from "metabase/alert/alert";
import Question from "metabase-lib/lib/Question";
@@ -103,8 +104,12 @@ function deserializeCard(serializedCard: string) {
return card;
}
-async function fetchAndPrepareSavedQuestionCards(cardId: number) {
- const card = await loadCard(cardId);
+async function fetchAndPrepareSavedQuestionCards(
+ cardId: number,
+ dispatch: Dispatch,
+ getState: GetState,
+) {
+ const card = await loadCard(cardId, { dispatch, getState });
const originalCard = { ...card };
// for showing the "started from" lineage correctly when adding filters/breakouts and when going back and forth
@@ -114,7 +119,11 @@ async function fetchAndPrepareSavedQuestionCards(cardId: number) {
return { card, originalCard };
}
-async function fetchAndPrepareAdHocQuestionCards(deserializedCard: Card) {
+async function fetchAndPrepareAdHocQuestionCards(
+ deserializedCard: Card,
+ dispatch: Dispatch,
+ getState: GetState,
+) {
if (!deserializedCard.original_card_id) {
return {
card: deserializedCard,
@@ -122,7 +131,10 @@ async function fetchAndPrepareAdHocQuestionCards(deserializedCard: Card) {
};
}
- const originalCard = await loadCard(deserializedCard.original_card_id);
+ const originalCard = await loadCard(deserializedCard.original_card_id, {
+ dispatch,
+ getState,
+ });
if (cardIsEquivalent(deserializedCard, originalCard)) {
return {
@@ -146,10 +158,14 @@ async function resolveCards({
cardId,
deserializedCard,
options,
+ dispatch,
+ getState,
}: {
cardId?: number;
deserializedCard?: Card;
options: BlankQueryOptions;
+ dispatch: Dispatch;
+ getState: GetState;
}): Promise<ResolveCardsResult> {
if (!cardId && !deserializedCard) {
return {
@@ -157,8 +173,12 @@ async function resolveCards({
};
}
return cardId
- ? fetchAndPrepareSavedQuestionCards(cardId)
- : fetchAndPrepareAdHocQuestionCards(deserializedCard as Card);
+ ? fetchAndPrepareSavedQuestionCards(cardId, dispatch, getState)
+ : fetchAndPrepareAdHocQuestionCards(
+ deserializedCard as Card,
+ dispatch,
+ getState,
+ );
}
function parseHash(hash?: string) {
@@ -209,6 +229,8 @@ async function handleQBInit(
cardId,
deserializedCard,
options,
+ dispatch,
+ getState,
});
if (isSavedCard(card) && card.archived) {
diff --git a/frontend/src/metabase/query_builder/actions/core/initializeQB.unit.spec.ts b/frontend/src/metabase/query_builder/actions/core/initializeQB.unit.spec.ts
index e6a7f460c66..6a933ae58b6 100644
--- a/frontend/src/metabase/query_builder/actions/core/initializeQB.unit.spec.ts
+++ b/frontend/src/metabase/query_builder/actions/core/initializeQB.unit.spec.ts
@@ -123,6 +123,8 @@ async function setup({
});
}
+ jest.spyOn(CardLib, "loadCard").mockReturnValue(Promise.resolve({ ...card }));
+
return baseSetup({ location, params, ...opts });
}
@@ -438,6 +440,10 @@ describe("QB Actions > initializeQB", () => {
body: JSON.stringify(originalQuestion.card()),
});
+ jest
+ .spyOn(CardLib, "loadCard")
+ .mockReturnValueOnce(Promise.resolve({ ...originalQuestion.card() }));
+
return setup({ question: q, ...opts });
}
diff --git a/frontend/test/metabase/containers/SavedQuestionLoader.unit.spec.js b/frontend/test/metabase/containers/SavedQuestionLoader.unit.spec.js
index f41654189f9..2fed8f722fb 100644
--- a/frontend/test/metabase/containers/SavedQuestionLoader.unit.spec.js
+++ b/frontend/test/metabase/containers/SavedQuestionLoader.unit.spec.js
@@ -3,7 +3,6 @@ import { render } from "@testing-library/react";
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
@@ -25,12 +24,13 @@ describe("SavedQuestionLoader", () => {
it("should load a question given a questionId", async () => {
const questionId = 1;
const q = Question.create({ databaseId: 1, tableId: 2 });
- jest.spyOn(CardApi, "get").mockReturnValue(q.card());
+ const mockFetchQuestion = jest.fn().mockResolvedValue(q.card());
render(
<SavedQuestionLoader
questionId={questionId}
loadMetadataForCard={loadMetadataSpy}
+ fetchQuestion={mockFetchQuestion}
>
{mockChild}
</SavedQuestionLoader>,
--
GitLab