Skip to content

Commit

Permalink
Optimize initial dashboard load (#42901)
Browse files Browse the repository at this point in the history
  • Loading branch information
ranquild committed May 23, 2024
1 parent e5e879a commit 3713a16
Show file tree
Hide file tree
Showing 35 changed files with 477 additions and 375 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
10 changes: 3 additions & 7 deletions e2e/test/scenarios/metrics/metrics-dashboard.cy.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -278,12 +278,8 @@ function combineAndVerifyMetrics(metric1, metric2) {
visitDashboard(dashboard.id);
},
);
cy.findByTestId("dashboard-header").within(() => {
cy.findByLabelText("Edit dashboard").click();
cy.findByLabelText("Add questions").click();
});
cy.findByTestId("add-card-sidebar").findByText(metric1.name).click();
getDashboardCard(1).realHover().findByTestId("add-series-button").click();
editDashboard();
getDashboardCard().realHover().findByTestId("add-series-button").click();
modal().within(() => {
cy.findByText(metric2.name).click();
cy.findByLabelText("Legend").within(() => {
Expand All @@ -293,7 +289,7 @@ function combineAndVerifyMetrics(metric1, metric2) {
cy.button("Done").click();
});
saveDashboard();
getDashboardCard(1).within(() => {
getDashboardCard().within(() => {
cy.findByLabelText("Legend").within(() => {
cy.findByText(metric1.name).should("be.visible");
cy.findByText(metric2.name).should("be.visible");
Expand Down
9 changes: 6 additions & 3 deletions frontend/src/metabase-lib/v1/parameters/utils/targets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ export function getParameterTargetField(
}

const fieldRef = target[1];
const query = question.query();
const metadata = question.metadata();

// native queries
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ import {
executeRowAction,
reloadDashboardCards,
} from "metabase/dashboard/actions";
import { getEditingDashcardId } from "metabase/dashboard/selectors";
import {
getEditingDashcardId,
getParameterValues,
} from "metabase/dashboard/selectors";
import { getActionIsEnabledInDatabase } from "metabase/dashboard/utils";
import type { VisualizationProps } from "metabase/visualizations/types";
import type {
Expand Down Expand Up @@ -139,6 +142,7 @@ const ConnectedActionComponent = connect()(ActionComponent);

function mapStateToProps(state: State, props: ActionProps) {
return {
parameterValues: getParameterValues(state),
isEditingDashcard: getEditingDashcardId(state) === props.dashcard.id,
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
createMockStructuredDatasetQuery,
createMockDatabase,
} from "metabase-types/api/mocks";
import { createMockDashboardState } from "metabase-types/store/mocks";

import type { ActionProps } from "./Action";
import Action from "./Action";
Expand Down Expand Up @@ -156,6 +157,9 @@ async function setup({
entities: createMockEntitiesState({
databases: [database],
}),
dashboard: createMockDashboardState({
parameterValues,
}),
},
},
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,7 @@ export function FieldValuesWidgetInner({
}}
onInputChange={onInputChange}
parseFreeformValue={parseFreeformValue}
updateOnInputBlur
/>
)}
</div>
Expand Down
14 changes: 1 addition & 13 deletions frontend/src/metabase/dashboard/actions/data-fetching-typed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,8 @@ import { loadMetadataForDashboard } from "metabase/dashboard/actions/metadata";
import {
getDashboardById,
getDashCardById,
getInitialSelectedTabId,
getParameterValues,
getQuestions,
getSelectedTabId,
} from "metabase/dashboard/selectors";
import {
expandInlineDashboard,
Expand Down Expand Up @@ -127,17 +125,7 @@ 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));
await dispatch(loadMetadataForDashboard(result.dashcards));
}

const isUsingCachedResults = entities != null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ describe("fetchDashboard", () => {
).toHaveLength(1);
});

it("should fetch metadata for cards on the first tab when there are tabs", async () => {
it("should fetch metadata for all cards when there are tabs", async () => {
const dashboard = createMockDashboard({
dashcards: [
createMockDashboardCard({
Expand Down Expand Up @@ -147,7 +147,7 @@ describe("fetchDashboard", () => {
).toHaveLength(1);
expect(
fetchMock.calls(`path:/api/table/${ORDERS_ID}/query_metadata`),
).toHaveLength(0);
).toHaveLength(1);
});

it("should cancel previous dashboard fetch when a new one is initiated (metabase#35959)", async () => {
Expand Down
16 changes: 7 additions & 9 deletions frontend/src/metabase/dashboard/actions/metadata.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import Questions from "metabase/entities/questions";
import { getLinkTargets } from "metabase/lib/click-behavior";
import { loadMetadataForCard } from "metabase/questions/actions";
import { loadMetadataForCards } from "metabase/questions/actions";

import { isVirtualDashCard } from "../utils";

Expand All @@ -10,17 +10,15 @@ export const loadMetadataForDashboard = dashCards => async dispatch => {
.flatMap(dc => [dc.card].concat(dc.series || []));

await Promise.all([
dispatch(loadMetadataForCards(cards)),
dispatch(loadMetadataForAvailableCards(cards)),
dispatch(loadMetadataForLinkedTargets(dashCards)),
]);
};

const loadMetadataForCards = cards => (dispatch, getState) => {
return Promise.all(
cards
.filter(card => card.dataset_query) // exclude queries without perms
.map(card => dispatch(loadMetadataForCard(card))),
);
const loadMetadataForAvailableCards = cards => dispatch => {
// exclude queries without perms
const availableCards = cards.filter(card => card.dataset_query);
return dispatch(loadMetadataForCards(availableCards));
};

const loadMetadataForLinkedTargets =
Expand All @@ -43,5 +41,5 @@ const loadMetadataForLinkedTargets =
)
.filter(card => card != null);

await dispatch(loadMetadataForCards(cards));
await dispatch(loadMetadataForAvailableCards(cards));
};
Loading

0 comments on commit 3713a16

Please sign in to comment.