diff --git a/e2e/test/scenarios/dashboard/reproductions/12926-editing-dashboards-doesnt-cancel-running-queries.cy.spec.js b/e2e/test/scenarios/dashboard/reproductions/12926-editing-dashboards-doesnt-cancel-running-queries.cy.spec.js index 5c17063d08c575cd302a7850642060540bb22e9e..d913935b7138b7945fa6cc5186810d1770a381d1 100644 --- a/e2e/test/scenarios/dashboard/reproductions/12926-editing-dashboards-doesnt-cancel-running-queries.cy.spec.js +++ b/e2e/test/scenarios/dashboard/reproductions/12926-editing-dashboards-doesnt-cancel-running-queries.cy.spec.js @@ -1,14 +1,32 @@ import { editDashboard, getDashboardCard, + openQuestionsSidebar, + popover, restore, + saveDashboard, + setFilter, showDashboardCardActions, + sidebar, undo, } from "e2e/support/helpers"; +const filterDisplayName = "F"; +const queryResult = 42; +const parameterValue = 10; const questionDetails = { - name: "Q1", - native: { query: "SELECT '42' as ANSWER" }, + name: "Question 1", + native: { + query: `SELECT ${queryResult} [[+{{F}}]] as ANSWER`, + "template-tags": { + F: { + type: "number", + name: "F", + id: "b22a5ce2-fe1d-44e3-8df4-f8951f7921bc", + "display-name": filterDisplayName, + }, + }, + }, }; describe("issue 12926", () => { @@ -19,7 +37,7 @@ describe("issue 12926", () => { describe("card removal while query is in progress", () => { it("should stop the ongoing query when removing a card from a dashboard", () => { - slowDownQuery(); + slowDownDashcardQuery(); cy.createNativeQuestionAndDashboard({ questionDetails, @@ -28,7 +46,7 @@ describe("issue 12926", () => { }); cy.window().then(win => { - cy.stub(win.XMLHttpRequest.prototype, "abort").as("xhrAbort"); + cy.spy(win.XMLHttpRequest.prototype, "abort").as("xhrAbort"); }); removeCard(); @@ -37,7 +55,7 @@ describe("issue 12926", () => { }); it("should re-fetch the query when doing undo on the removal", () => { - slowDownQuery(); + slowDownDashcardQuery(); cy.createNativeQuestionAndDashboard({ questionDetails, @@ -47,30 +65,73 @@ describe("issue 12926", () => { removeCard(); - restoreQuery(); + restoreDashcardQuery(); undo(); - cy.wait("@cardQueryRestored"); + cy.wait("@dashcardQueryRestored"); - getDashboardCard().findByText("42"); + getDashboardCard().findByText(queryResult); + }); + }); + + describe("saving a dashboard that retriggers a non saved query (negative id)", () => { + it("should stop the ongoing query", () => { + // this test requires the card to be manually added to the dashboard, as it requires the dashcard id to be negative + cy.createNativeQuestion(questionDetails); + + cy.createDashboard().then(({ body: { id: dashboardId } }) => { + cy.visit(`/dashboard/${dashboardId}`); + }); + + editDashboard(); + + openQuestionsSidebar(); + slowDownCardQuery(); + sidebar().findByText(questionDetails.name).click(); + + setFilter("Number", "Equal to"); + sidebar().findByText("No default").click(); + popover().findByPlaceholderText("Enter a number").type(parameterValue); + popover().findByText("Add filter").click(); + + cy.window().then(win => { + cy.spy(win.XMLHttpRequest.prototype, "abort").as("xhrAbort"); + }); + + getDashboardCard().findByText("Select…").click(); + popover().contains(filterDisplayName).eq(0).click(); + + saveDashboard(); + + cy.get("@xhrAbort").should("have.been.calledOnce"); + + getDashboardCard().findByText(queryResult + parameterValue); }); }); }); -function slowDownQuery() { - cy.intercept("POST", "api/dashboard/*/dashcard/*/card/*/query", req => { +function slowDownCardQuery() { + cy.intercept("POST", "api/card/*/query", req => { req.on("response", res => { - res.setDelay(10000); + res.setDelay(300000); }); }).as("cardQuerySlowed"); } -function restoreQuery() { +function slowDownDashcardQuery() { + cy.intercept("POST", "api/dashboard/*/dashcard/*/card/*/query", req => { + req.on("response", res => { + res.setDelay(5000); + }); + }).as("dashcardQuerySlowed"); +} + +function restoreDashcardQuery() { cy.intercept("POST", "api/dashboard/*/dashcard/*/card/*/query", req => { // calling req.continue() will make cypress skip all previously added intercepts req.continue(); - }).as("cardQueryRestored"); + }).as("dashcardQueryRestored"); } function removeCard() { diff --git a/frontend/src/metabase/dashboard/actions/data-fetching.js b/frontend/src/metabase/dashboard/actions/data-fetching.js index 645c770b8955dc8162730dc3147e3c25fad9cb2a..1e02a01d088f126fd822f35f0274d56e14680785 100644 --- a/frontend/src/metabase/dashboard/actions/data-fetching.js +++ b/frontend/src/metabase/dashboard/actions/data-fetching.js @@ -66,6 +66,9 @@ 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 CANCEL_FETCH_CARD_DATA = "metabase/dashboard/CANCEL_FETCH_CARD_DATA"; @@ -259,6 +262,14 @@ export const fetchCardData = createThunkAction( FETCH_CARD_DATA, function (card, dashcard, { reload, clearCache, ignoreCache } = {}) { return async function (dispatch, getState) { + dispatch({ + type: FETCH_CARD_DATA_PENDING, + payload: { + dashcard_id: dashcard.id, + card_id: card.id, + }, + }); + // If the dataset_query was filtered then we don't have permisison to view this card, so // shortcircuit and return a fake 403 if (!card.dataset_query) { @@ -449,6 +460,11 @@ export const fetchDashboardCardData = return dashcard.id; }); + for (const id of loadingIds) { + const dashcard = getDashCardById(getState(), id); + dispatch(cancelFetchCardData(dashcard.card.id, dashcard.id)); + } + dispatch({ type: FETCH_DASHBOARD_CARD_DATA, payload: { diff --git a/frontend/src/metabase/dashboard/reducers.js b/frontend/src/metabase/dashboard/reducers.js index 27f3adf83dc788a1db9d26a538554c2843fffdf8..9e5dd6474582931b7ada87362ca7301852d45905 100644 --- a/frontend/src/metabase/dashboard/reducers.js +++ b/frontend/src/metabase/dashboard/reducers.js @@ -43,6 +43,7 @@ import { UNDO_REMOVE_CARD_FROM_DASH, SHOW_AUTO_APPLY_FILTERS_TOAST, tabsReducer, + FETCH_CARD_DATA_PENDING, } from "./actions"; import { syncParametersAndEmbeddingParams } from "./utils"; import { INITIAL_DASHBOARD_STATE } from "./constants"; @@ -380,6 +381,15 @@ const loadingDashCards = handleActions( }; }, }, + [FETCH_CARD_DATA_PENDING]: { + next: (state, { payload: { dashcard_id } }) => { + const loadingIds = (state.loadingIds ?? []).concat(dashcard_id); + return { + ...state, + loadingIds, + }; + }, + }, [FETCH_CARD_DATA]: { next: (state, { payload: { dashcard_id, currentTime } }) => { const loadingIds = state.loadingIds.filter(id => id !== dashcard_id);