Skip to content
Snippets Groups Projects
Unverified Commit 4431d9d4 authored by Nicolò Pretto's avatar Nicolò Pretto Committed by GitHub
Browse files

cancel ongoing cards when saving a dashboard (#33128)

* cancels ongoing cards when saving a dashboard

* refactor: change from stub to spy so that we can also see in the inspector that the request has been cancelled

* test that the data will eventually be displayed
parent f40805b1
No related branches found
No related tags found
No related merge requests found
import { import {
editDashboard, editDashboard,
getDashboardCard, getDashboardCard,
openQuestionsSidebar,
popover,
restore, restore,
saveDashboard,
setFilter,
showDashboardCardActions, showDashboardCardActions,
sidebar,
undo, undo,
} from "e2e/support/helpers"; } from "e2e/support/helpers";
const filterDisplayName = "F";
const queryResult = 42;
const parameterValue = 10;
const questionDetails = { const questionDetails = {
name: "Q1", name: "Question 1",
native: { query: "SELECT '42' as ANSWER" }, 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", () => { describe("issue 12926", () => {
...@@ -19,7 +37,7 @@ describe("issue 12926", () => { ...@@ -19,7 +37,7 @@ describe("issue 12926", () => {
describe("card removal while query is in progress", () => { describe("card removal while query is in progress", () => {
it("should stop the ongoing query when removing a card from a dashboard", () => { it("should stop the ongoing query when removing a card from a dashboard", () => {
slowDownQuery(); slowDownDashcardQuery();
cy.createNativeQuestionAndDashboard({ cy.createNativeQuestionAndDashboard({
questionDetails, questionDetails,
...@@ -28,7 +46,7 @@ describe("issue 12926", () => { ...@@ -28,7 +46,7 @@ describe("issue 12926", () => {
}); });
cy.window().then(win => { cy.window().then(win => {
cy.stub(win.XMLHttpRequest.prototype, "abort").as("xhrAbort"); cy.spy(win.XMLHttpRequest.prototype, "abort").as("xhrAbort");
}); });
removeCard(); removeCard();
...@@ -37,7 +55,7 @@ describe("issue 12926", () => { ...@@ -37,7 +55,7 @@ describe("issue 12926", () => {
}); });
it("should re-fetch the query when doing undo on the removal", () => { it("should re-fetch the query when doing undo on the removal", () => {
slowDownQuery(); slowDownDashcardQuery();
cy.createNativeQuestionAndDashboard({ cy.createNativeQuestionAndDashboard({
questionDetails, questionDetails,
...@@ -47,30 +65,73 @@ describe("issue 12926", () => { ...@@ -47,30 +65,73 @@ describe("issue 12926", () => {
removeCard(); removeCard();
restoreQuery(); restoreDashcardQuery();
undo(); 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() { function slowDownCardQuery() {
cy.intercept("POST", "api/dashboard/*/dashcard/*/card/*/query", req => { cy.intercept("POST", "api/card/*/query", req => {
req.on("response", res => { req.on("response", res => {
res.setDelay(10000); res.setDelay(300000);
}); });
}).as("cardQuerySlowed"); }).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 => { cy.intercept("POST", "api/dashboard/*/dashcard/*/card/*/query", req => {
// calling req.continue() will make cypress skip all previously added intercepts // calling req.continue() will make cypress skip all previously added intercepts
req.continue(); req.continue();
}).as("cardQueryRestored"); }).as("dashcardQueryRestored");
} }
function removeCard() { function removeCard() {
......
...@@ -66,6 +66,9 @@ export const FETCH_DASHBOARD_CARD_METADATA = ...@@ -66,6 +66,9 @@ export const FETCH_DASHBOARD_CARD_METADATA =
"metabase/dashboard/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 = "metabase/dashboard/FETCH_CARD_DATA";
export const FETCH_CARD_DATA_PENDING =
"metabase/dashboard/FETCH_CARD_DATA/pending";
export const CANCEL_FETCH_CARD_DATA = export const CANCEL_FETCH_CARD_DATA =
"metabase/dashboard/CANCEL_FETCH_CARD_DATA"; "metabase/dashboard/CANCEL_FETCH_CARD_DATA";
...@@ -259,6 +262,14 @@ export const fetchCardData = createThunkAction( ...@@ -259,6 +262,14 @@ export const fetchCardData = createThunkAction(
FETCH_CARD_DATA, FETCH_CARD_DATA,
function (card, dashcard, { reload, clearCache, ignoreCache } = {}) { function (card, dashcard, { reload, clearCache, ignoreCache } = {}) {
return async function (dispatch, getState) { 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 // If the dataset_query was filtered then we don't have permisison to view this card, so
// shortcircuit and return a fake 403 // shortcircuit and return a fake 403
if (!card.dataset_query) { if (!card.dataset_query) {
...@@ -449,6 +460,11 @@ export const fetchDashboardCardData = ...@@ -449,6 +460,11 @@ export const fetchDashboardCardData =
return dashcard.id; return dashcard.id;
}); });
for (const id of loadingIds) {
const dashcard = getDashCardById(getState(), id);
dispatch(cancelFetchCardData(dashcard.card.id, dashcard.id));
}
dispatch({ dispatch({
type: FETCH_DASHBOARD_CARD_DATA, type: FETCH_DASHBOARD_CARD_DATA,
payload: { payload: {
......
...@@ -43,6 +43,7 @@ import { ...@@ -43,6 +43,7 @@ import {
UNDO_REMOVE_CARD_FROM_DASH, UNDO_REMOVE_CARD_FROM_DASH,
SHOW_AUTO_APPLY_FILTERS_TOAST, SHOW_AUTO_APPLY_FILTERS_TOAST,
tabsReducer, tabsReducer,
FETCH_CARD_DATA_PENDING,
} from "./actions"; } from "./actions";
import { syncParametersAndEmbeddingParams } from "./utils"; import { syncParametersAndEmbeddingParams } from "./utils";
import { INITIAL_DASHBOARD_STATE } from "./constants"; import { INITIAL_DASHBOARD_STATE } from "./constants";
...@@ -380,6 +381,15 @@ const loadingDashCards = handleActions( ...@@ -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]: { [FETCH_CARD_DATA]: {
next: (state, { payload: { dashcard_id, currentTime } }) => { next: (state, { payload: { dashcard_id, currentTime } }) => {
const loadingIds = state.loadingIds.filter(id => id !== dashcard_id); const loadingIds = state.loadingIds.filter(id => id !== dashcard_id);
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment