Load dashcard metadata in parallel with the query (#42953)

* Split card data and query * Split card data and query * Split card data and query * Fix loading state * Postpone card check * Postpone card check * Postpone field check * Postpone field check * Postpone field check * Postpone field check * Postpone field check * Add await * Add await * Add await * Add await * Fix tests * Fix data loading * Fix data loading * Fix data loading * Fix data loading * Revert "Fix data loading" This reverts commit 2b782b9. * Fix data loading * Fix test * Fix test * Fix embedding * Fix xrays * Fix xrays
metabase · May 22, 2024 · a0a4a26 · a0a4a26
1 parent bdb006b
commit a0a4a26
Show file tree

Hide file tree

Showing 16 changed files with 103 additions and 190 deletions.
diff --git a/e2e/test/scenarios/dashboard-filters/dashboard-filters-nested.cy.spec.js b/e2e/test/scenarios/dashboard-filters/dashboard-filters-nested.cy.spec.js
@@ -78,8 +78,11 @@ describe("scenarios > dashboard > filters > nested questions", () => {
     // Add multiple values (metabase#18113)
     filterWidget().click();
     popover().within(() => {
-      cy.findByText("Gizmo").click();
-      cy.findByText("Gadget").click();
+      cy.findByPlaceholderText("Enter some text")
+        .type("Gizmo")
+        .blur()
+        .type("Gadget")
+        .blur();
     });
 
     cy.button("Add filter").click();

diff --git a/frontend/src/metabase-lib/v1/parameters/utils/targets.ts b/frontend/src/metabase-lib/v1/parameters/utils/targets.ts
@@ -45,7 +45,6 @@ export function getParameterTargetField(
   }
 
   const fieldRef = target[1];
-  const query = question.query();
   const metadata = question.metadata();
 
   // native queries
@@ -60,8 +59,12 @@ export function getParameterTargetField(
 
   if (isConcreteFieldReference(fieldRef)) {
     const fieldId = fieldRef[1];
-    const tableId = Lib.sourceTableOrCardId(query);
-    return metadata.field(fieldId, tableId) ?? metadata.field(fieldId);
+    const resultMetadata = question.getResultMetadata();
+    const fieldMetadata = resultMetadata.find(field => field.id === fieldId);
+    return (
+      metadata.field(fieldId, fieldMetadata?.table_id) ??
+      metadata.field(fieldId)
+    );
   }
 
   return null;

diff --git a/frontend/src/metabase/components/FieldValuesWidget/FieldValuesWidget.tsx b/frontend/src/metabase/components/FieldValuesWidget/FieldValuesWidget.tsx
@@ -498,6 +498,7 @@ export function FieldValuesWidgetInner({
             }}
             onInputChange={onInputChange}
             parseFreeformValue={parseFreeformValue}
+            updateOnInputBlur
           />
         )}
       </div>

diff --git a/frontend/src/metabase/dashboard/actions/cards-typed.ts b/frontend/src/metabase/dashboard/actions/cards-typed.ts
@@ -1,3 +1,4 @@
+import { loadMetadataForDashcards } from "metabase/dashboard/actions/metadata";
 import Questions from "metabase/entities/questions";
 import {
   DEFAULT_CARD_SIZE,
@@ -40,7 +41,6 @@ import {
   setDashCardAttributes,
 } from "./core";
 import { cancelFetchCardData, fetchCardData } from "./data-fetching";
-import { loadMetadataForDashboard } from "./metadata";
 import { getExistingDashCards } from "./utils";
 
 export type NewDashCardOpts = {
@@ -158,7 +158,7 @@ export const addCardToDashboard =
     );
 
     dispatch(fetchCardData(card, dashcard, { reload: true, clearCache: true }));
-    await dispatch(loadMetadataForDashboard([dashcard]));
+    await dispatch(loadMetadataForDashcards([dashcard]));
     dispatch(autoWireParametersToNewCard({ dashcard_id: dashcardId }));
   };
 
@@ -233,7 +233,7 @@ export const replaceCard =
     const dashcard = getDashCardById(getState(), dashcardId);
 
     dispatch(fetchCardData(card, dashcard, { reload: true, clearCache: true }));
-    await dispatch(loadMetadataForDashboard([dashcard]));
+    await dispatch(loadMetadataForDashcards([dashcard]));
     dispatch(autoWireParametersToNewCard({ dashcard_id: dashcardId }));
 
     dashboardId && trackQuestionReplaced(dashboardId);

diff --git a/frontend/src/metabase/dashboard/actions/data-fetching-typed.ts b/frontend/src/metabase/dashboard/actions/data-fetching-typed.ts
@@ -1,13 +1,10 @@
 import { denormalize, normalize, schema } from "normalizr";
 
-import { loadMetadataForDashboard } from "metabase/dashboard/actions/metadata";
 import {
   getDashboardById,
   getDashCardById,
-  getInitialSelectedTabId,
   getParameterValues,
   getQuestions,
-  getSelectedTabId,
 } from "metabase/dashboard/selectors";
 import {
   expandInlineDashboard,
@@ -126,20 +123,6 @@ export const fetchDashboard = createAsyncThunk(
 
       fetchDashboardCancellation = null;
 
-      if (dashboardType === "normal" || dashboardType === "transient") {
-        const selectedTabId =
-          getSelectedTabId(getState()) ?? getInitialSelectedTabId(result);
-
-        const cards =
-          selectedTabId == null
-            ? result.dashcards
-            : result.dashcards.filter(
-                (c: DashboardCard) => c.dashboard_tab_id === selectedTabId,
-              );
-
-        await dispatch(loadMetadataForDashboard(cards));
-      }
-
       const isUsingCachedResults = entities != null;
       if (!isUsingCachedResults) {
         // copy over any virtual cards from the dashcard to the underlying card/question
@@ -154,10 +137,10 @@ export const fetchDashboard = createAsyncThunk(
       }
 
       if (result.param_values) {
-        dispatch(addParamValues(result.param_values));
+        await dispatch(addParamValues(result.param_values));
       }
       if (result.param_fields) {
-        dispatch(addFields(result.param_fields));
+        await dispatch(addFields(result.param_fields));
       }
 
       const metadata = getMetadata(getState());

diff --git a/frontend/src/metabase/dashboard/actions/data-fetching.js b/frontend/src/metabase/dashboard/actions/data-fetching.js
@@ -33,20 +33,19 @@ import {
   getCurrentTabDashboardCards,
 } from "../utils";
 
-import { loadMetadataForDashboard } from "./metadata";
+import { loadMetadataForDashcards } from "./metadata";
 
 export const FETCH_DASHBOARD_CARD_DATA =
   "metabase/dashboard/FETCH_DASHBOARD_CARD_DATA";
 export const CANCEL_FETCH_DASHBOARD_CARD_DATA =
   "metabase/dashboard/CANCEL_FETCH_DASHBOARD_CARD_DATA";
 
-export const FETCH_DASHBOARD_CARD_METADATA =
-  "metabase/dashboard/FETCH_DASHBOARD_CARD_METADATA";
-
 export const FETCH_CARD_DATA = "metabase/dashboard/FETCH_CARD_DATA";
 export const FETCH_CARD_DATA_PENDING =
   "metabase/dashboard/FETCH_CARD_DATA/pending";
 
+export const FETCH_CARD_QUERY = "metabase/dashboard/FETCH_CARD_QUERY";
+
 export const CANCEL_FETCH_CARD_DATA =
   "metabase/dashboard/CANCEL_FETCH_CARD_DATA";
 
@@ -118,7 +117,7 @@ const loadingComplete = createThunkAction(
 
 export const fetchCardData = createThunkAction(
   FETCH_CARD_DATA,
-  function (card, dashcard, { reload, clearCache, ignoreCache } = {}) {
+  function (card, dashcard, options) {
     return async function (dispatch, getState) {
       dispatch({
         type: FETCH_CARD_DATA_PENDING,
@@ -128,6 +127,28 @@ export const fetchCardData = createThunkAction(
         },
       });
 
+      const requests = [dispatch(fetchCardQuery(card, dashcard, options))];
+
+      const dashboardType = getDashboardType(dashcard.dashboard_id);
+      if (dashboardType === "normal" || dashboardType === "transient") {
+        requests.push(dispatch(loadMetadataForDashcards([dashcard])));
+      }
+
+      const [{ payload }] = await Promise.all(requests);
+
+      return {
+        dashcard_id: dashcard.id,
+        card_id: card.id,
+        ...payload,
+      };
+    };
+  },
+);
+
+export const fetchCardQuery = createThunkAction(
+  FETCH_CARD_QUERY,
+  function (card, dashcard, { reload, clearCache, ignoreCache } = {}) {
+    return async function (dispatch, getState) {
       // If the dataset_query was filtered then we don't have permission to view this card, so
       // shortcircuit and return a fake 403
       if (!card.dataset_query) {
@@ -376,20 +397,6 @@ export const fetchDashboardCardData =
     }
   };
 
-export const fetchDashboardCardMetadata = createThunkAction(
-  FETCH_DASHBOARD_CARD_METADATA,
-  () => async (dispatch, getState) => {
-    const allDashCards = getDashboardComplete(getState()).dashcards;
-    const selectedTabId = getSelectedTabId(getState());
-
-    const cards = allDashCards.filter(
-      dc =>
-        selectedTabId !== undefined && dc.dashboard_tab_id === selectedTabId,
-    );
-    await dispatch(loadMetadataForDashboard(cards));
-  },
-);
-
 export const reloadDashboardCards = () => async (dispatch, getState) => {
   const dashboard = getDashboardComplete(getState());
 

diff --git a/frontend/src/metabase/dashboard/actions/data-fetching.unit.spec.js b/frontend/src/metabase/dashboard/actions/data-fetching.unit.spec.js
@@ -1,5 +1,3 @@
-import fetchMock from "fetch-mock";
-
 import { getStore } from "__support__/entities-store";
 import {
   setupDashboardsEndpoints,
@@ -8,18 +6,10 @@ import {
 import { createMockEntitiesState } from "__support__/store";
 import { Api } from "metabase/api";
 import {
-  createMockCard,
   createMockDashboard,
-  createMockDashboardCard,
-  createMockDashboardTab,
   createMockSettings,
-  createMockStructuredDatasetQuery,
 } from "metabase-types/api/mocks";
-import {
-  createSampleDatabase,
-  ORDERS_ID,
-  PRODUCTS_ID,
-} from "metabase-types/api/mocks/presets";
+import { createSampleDatabase } from "metabase-types/api/mocks/presets";
 import { createMockDashboardState } from "metabase-types/store/mocks";
 
 import { dashboardReducers } from "../reducers";
@@ -54,102 +44,6 @@ function setup({ dashboards = [] }) {
 }
 
 describe("fetchDashboard", () => {
-  it("should fetch metadata for all cards when there are no tabs", async () => {
-    const dashboard = createMockDashboard({
-      dashcards: [
-        createMockDashboardCard({
-          card: createMockCard({
-            dataset_query: createMockStructuredDatasetQuery({
-              query: {
-                "source-table": PRODUCTS_ID,
-              },
-            }),
-          }),
-        }),
-        createMockDashboardCard({
-          card: createMockCard({
-            dataset_query: createMockStructuredDatasetQuery({
-              query: {
-                "source-table": ORDERS_ID,
-              },
-            }),
-          }),
-        }),
-      ],
-    });
-    const store = setup({
-      dashboards: [dashboard],
-    });
-
-    await store.dispatch(
-      fetchDashboard({
-        dashId: dashboard.id,
-        queryParams: {},
-        options: {},
-      }),
-    );
-
-    expect(
-      fetchMock.calls(`path:/api/table/${PRODUCTS_ID}/query_metadata`),
-    ).toHaveLength(1);
-    expect(
-      fetchMock.calls(`path:/api/table/${ORDERS_ID}/query_metadata`),
-    ).toHaveLength(1);
-  });
-
-  it("should fetch metadata for cards on the first tab when there are tabs", async () => {
-    const dashboard = createMockDashboard({
-      dashcards: [
-        createMockDashboardCard({
-          card: createMockCard({
-            dataset_query: createMockStructuredDatasetQuery({
-              query: {
-                "source-table": PRODUCTS_ID,
-              },
-            }),
-          }),
-          dashboard_tab_id: 1,
-        }),
-        createMockDashboardCard({
-          card: createMockCard({
-            dataset_query: createMockStructuredDatasetQuery({
-              query: {
-                "source-table": ORDERS_ID,
-              },
-            }),
-          }),
-          dashboard_tab_id: 2,
-        }),
-      ],
-      tabs: [
-        createMockDashboardTab({
-          id: 1,
-        }),
-        createMockDashboardTab({
-          id: 2,
-        }),
-      ],
-    });
-    const store = setup({
-      dashboards: [dashboard],
-    });
-
-    await store.dispatch(
-      fetchDashboard({
-        dashId: dashboard.id,
-        queryParams: {},
-        options: {},
-      }),
-    );
-
-    expect(
-      fetchMock.calls(`path:/api/table/${PRODUCTS_ID}/query_metadata`),
-    ).toHaveLength(1);
-    expect(
-      fetchMock.calls(`path:/api/table/${ORDERS_ID}/query_metadata`),
-    ).toHaveLength(0);
-  });
-
   it("should cancel previous dashboard fetch when a new one is initiated (metabase#35959)", async () => {
     const store = setup({
       dashboards: [

diff --git a/frontend/src/metabase/dashboard/actions/metadata.js b/frontend/src/metabase/dashboard/actions/metadata.js
@@ -4,14 +4,14 @@ import { loadMetadataForCard } from "metabase/questions/actions";
 
 import { isVirtualDashCard } from "../utils";
 
-export const loadMetadataForDashboard = dashCards => async dispatch => {
-  const cards = dashCards
+export const loadMetadataForDashcards = dashcards => async dispatch => {
+  const cards = dashcards
     .filter(dc => !isVirtualDashCard(dc)) // exclude text cards
     .flatMap(dc => [dc.card].concat(dc.series || []));
 
   await Promise.all([
     dispatch(loadMetadataForCards(cards)),
-    dispatch(loadMetadataForLinkedTargets(dashCards)),
+    dispatch(loadMetadataForLinkedTargets(dashcards)),
   ]);
 };
 

diff --git a/frontend/src/metabase/dashboard/actions/navigation.js b/frontend/src/metabase/dashboard/actions/navigation.js
@@ -1,3 +1,4 @@
+import { loadMetadataForDashcards } from "metabase/dashboard/actions/metadata";
 import { createThunkAction } from "metabase/lib/redux";
 import * as Urls from "metabase/lib/urls";
 import { getParametersMappedToDashcard } from "metabase/parameters/utils/dashboards";
@@ -43,7 +44,9 @@ export const NAVIGATE_TO_NEW_CARD = "metabase/dashboard/NAVIGATE_TO_NEW_CARD";
 export const navigateToNewCardFromDashboard = createThunkAction(
   NAVIGATE_TO_NEW_CARD,
   ({ nextCard, previousCard, dashcard, objectId }) =>
-    (dispatch, getState) => {
+    async (dispatch, getState) => {
+      await dispatch(loadMetadataForDashcards([dashcard]));
+
       const metadata = getMetadata(getState());
       const { dashboardId, dashboards, parameterValues } = getState().dashboard;
       const dashboard = dashboards[dashboardId];

diff --git a/frontend/src/metabase/dashboard/components/DashCard/DashCardMenu/DashCardMenu.styled.tsx b/frontend/src/metabase/dashboard/components/DashCard/DashCardMenu/DashCardMenu.styled.tsx
@@ -3,7 +3,7 @@ import styled from "@emotion/styled";
 import EntityMenu from "metabase/components/EntityMenu";
 import { color, lighten } from "metabase/lib/colors";
 
-export const CardMenuRoot = styled(EntityMenu)`
+export const CardEntityMenu = styled(EntityMenu)`
   display: flex;
   align-items: center;
   margin: 0 0 0 0.5rem;