Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "Replace question" action to dashboard cards #36744

Merged
merged 26 commits into from
Dec 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
c4ccf2e
Remove `isVirtualDashCard` prop
kulyk Dec 8, 2023
9e0a584
Add "replace" button to `DashCardActionsPanel`
kulyk Dec 8, 2023
441d504
Add `QuestionPickerModal`
kulyk Dec 8, 2023
6b7f0ee
Add `replaceCard` action
kulyk Dec 8, 2023
20194a1
Use `/card/:id/query` endpoint for swapped cards
kulyk Dec 8, 2023
7e21043
Implement card replace
kulyk Dec 8, 2023
2f2ffd6
Add basic undo toast
kulyk Dec 8, 2023
7986019
`update-dashboard-card` will update `:card_id`
markbastian Dec 8, 2023
b746d46
Adding unit test for updating dashcard's card
markbastian Dec 8, 2023
a8232aa
Clean up `DashCard` test suite
kulyk Dec 11, 2023
86c0c1f
Add tests for `DashCard` component
kulyk Dec 12, 2023
c9fe261
Add tests for `QuestionPickerModal` component
kulyk Dec 12, 2023
8eb23ad
Add tests for `replaceCard` action
kulyk Dec 12, 2023
b0ba9b6
Remove redundant action argument
kulyk Dec 12, 2023
a099d1b
Add test for parameters auto-wiring
kulyk Dec 12, 2023
e11c81d
Fix unexpected card query endpoint usage
kulyk Dec 13, 2023
f575c91
Use more accessible selector in `saveDashboard`
kulyk Dec 13, 2023
efde45a
Add e2e tests
kulyk Dec 13, 2023
94ae13c
Remove `justAdded` from `createMockDashboardCard`
kulyk Dec 13, 2023
970d809
Fix test setup
kulyk Dec 13, 2023
8b2a89f
Updating `update-action-cards-test` test
markbastian Dec 13, 2023
eff7b4c
Remove previous events
kulyk Dec 13, 2023
bf2db12
Track `dashboard_card_replaced` event
kulyk Dec 13, 2023
c8249d2
Merge remote-tracking branch 'origin/36497-replace-dashcard-card' int…
markbastian Dec 13, 2023
6e00cd2
Merge branch 'master' into 36497-replace-dashcard-card
kulyk Dec 14, 2023
92bc305
Show "replace" action for erroring queries
kulyk Dec 14, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion e2e/support/helpers/e2e-dashboard-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export function saveDashboard({
buttonLabel = "Save",
editBarText = "You're editing this dashboard.",
} = {}) {
cy.findByText(buttonLabel).click();
cy.button(buttonLabel).click();
cy.findByText(editBarText).should("not.exist");
cy.wait(1); // this is stupid but necessary to due to the dashboard resizing and detaching elements
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,284 @@
import {
saveDashboard,
modal,
popover,
restore,
visitDashboard,
} from "e2e/support/helpers";
import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database";
import { USER_GROUPS } from "e2e/support/cypress_data";
import {
FIRST_COLLECTION_ID,
ORDERS_COUNT_QUESTION_ID,
} from "e2e/support/cypress_sample_instance_data";
import {
createMockDashboardCard,
createMockHeadingDashboardCard,
createMockParameter,
} from "metabase-types/api/mocks";

const { PRODUCTS, PRODUCTS_ID } = SAMPLE_DATABASE;

const PARAMETER = {
DATE: createMockParameter({
id: "1",
name: "Created At",
type: "date/all-options",
}),
CATEGORY: createMockParameter({
id: "2",
name: "Category",
type: "string/=",
}),
UNUSED: createMockParameter({
id: "3",
name: "Not mapped to anything",
type: "number/=",
}),
};

const DASHBOARD_CREATE_INFO = {
parameters: Object.values(PARAMETER),
};

// Question to be used as a reference for filters auto-wiring
const MAPPED_QUESTION_CREATE_INFO = {
name: "Question with mapped parameters",
query: { "source-table": PRODUCTS_ID },
};

const NEXT_QUESTION_CREATE_INFO = {
name: "Next question",
collection_id: FIRST_COLLECTION_ID,
query: { "source-table": PRODUCTS_ID },
};

function getDashboardCards(mappedQuestionId) {
const mappedQuestionDashcard = createMockDashboardCard({
id: 2,
card_id: mappedQuestionId,
parameter_mappings: [
{
parameter_id: PARAMETER.DATE.id,
card_id: mappedQuestionId,
target: ["dimension", ["field", PRODUCTS.CREATED_AT, null]],
},
{
parameter_id: PARAMETER.CATEGORY.id,
card_id: mappedQuestionId,
target: ["dimension", ["field", PRODUCTS.CATEGORY, null]],
},
],
row: 1,
size_x: 6,
size_y: 4,
});

const targetDashcard = createMockDashboardCard({
id: 3,
card_id: ORDERS_COUNT_QUESTION_ID,
row: 1,
col: 6,
size_x: 6,
size_y: 4,
});

return [
createMockHeadingDashboardCard({ id: 1, size_x: 24 }),
mappedQuestionDashcard,
targetDashcard,
];
}

describe("scenarios > dashboard cards > replace question", () => {
beforeEach(() => {
restore();
cy.signInAsNormalUser();

cy.intercept("POST", `/api/card/*/query`).as("cardQuery");

cy.createQuestion(MAPPED_QUESTION_CREATE_INFO).then(
({ body: { id: mappedQuestionId } }) => {
cy.createQuestion(NEXT_QUESTION_CREATE_INFO).then(() => {
cy.createDashboard(DASHBOARD_CREATE_INFO).then(
({ body: { id: dashboardId } }) => {
cy.request("PUT", `/api/dashboard/${dashboardId}`, {
dashcards: getDashboardCards(mappedQuestionId),
}).then(() => {
cy.wrap(dashboardId).as("dashboardId");
});
},
);
});
},
);
});

it("should replace a dashboard card question", () => {
visitDashboardAndEdit();

findDashCardAction(findHeadingDashcard(), "Replace").should("not.exist");

// Ensure can replace with a question
replaceQuestion(findTargetDashcard(), {
nextQuestionName: "Next question",
collectionName: "First collection",
});
findTargetDashcard().within(() => {
assertDashCardTitle("Next question");
cy.findByText("Ean").should("exist");
cy.findByText("Rustic Paper Wallet").should("exist");
});

// Ensure can replace with a model
replaceQuestion(findTargetDashcard(), { nextQuestionName: "Orders Model" });
findTargetDashcard().within(() => {
assertDashCardTitle("Orders Model");
cy.findByText("Product ID").should("exist");
cy.findByText("User ID").should("exist");
});

// Ensure changes are persisted
saveDashboard();
findTargetDashcard().within(() => {
assertDashCardTitle("Orders Model");
cy.findByText("Product ID").should("exist");
cy.findByText("User ID").should("exist");
});
});

it("should undo the question replace action", () => {
visitDashboardAndEdit();

overwriteDashCardTitle(findTargetDashcard(), "Custom name");
connectDashboardFilter(findTargetDashcard(), {
filterName: PARAMETER.UNUSED.name,
columnName: "Discount",
});

replaceQuestion(findTargetDashcard(), {
nextQuestionName: "Next question",
collectionName: "First collection",
});

// There're two toasts: "Undo replace" and "Undo parameters auto-wiring"
cy.findAllByTestId("toast-undo").eq(0).button("Undo").click();

// Ensure we kept viz settings and parameter mapping changes from before
findTargetDashcard().within(() => {
assertDashCardTitle("Custom name");
cy.findByText("18,760").should("exist");
cy.findByText("Ean").should("not.exist");
cy.findByText("Rustic Paper Wallet").should("not.exist");
});
assertDashboardFilterMapping(findTargetDashcard(), {
filterName: PARAMETER.UNUSED.name,
expectedColumName: "Order.Discount",
});

// Ensure changes are persisted
saveDashboard();
findTargetDashcard().within(() => {
assertDashCardTitle("Custom name");
cy.findByText("18,760").should("exist");
cy.findByText("Ean").should("not.exist");
cy.findByText("Rustic Paper Wallet").should("not.exist");
});
});

it("should handle questions with limited permissions", () => {
cy.signInAsAdmin();
cy.updateCollectionGraph({
[USER_GROUPS.ALL_USERS_GROUP]: { [FIRST_COLLECTION_ID]: "read" },
});

cy.signIn("nodata");
visitDashboardAndEdit();

// Replacing with a read-only question with limited data perms
replaceQuestion(findTargetDashcard(), {
nextQuestionName: "Next question",
collectionName: "First collection",
});
findTargetDashcard().within(() => {
assertDashCardTitle("Next question");
cy.findByText("Ean").should("exist");
cy.findByText("Rustic Paper Wallet").should("exist");
});

// Ensure changes are persisted
saveDashboard();
findTargetDashcard().within(() => {
assertDashCardTitle("Next question");
cy.findByText("Ean").should("exist");
cy.findByText("Rustic Paper Wallet").should("exist");
});
});
});

function visitDashboardAndEdit() {
cy.get("@dashboardId").then(dashboardId => {
visitDashboard(dashboardId);
cy.findByLabelText("Edit dashboard").click();
});
}

function findHeadingDashcard() {
return cy.findAllByTestId("dashcard").eq(0);
}

function findTargetDashcard() {
return cy.findAllByTestId("dashcard").eq(2);
}

function findDashCardAction(dashcardElement, labelText) {
return dashcardElement.realHover().findByLabelText(labelText);
}

function replaceQuestion(
dashcardElement,
{ nextQuestionName, collectionName },
) {
dashcardElement.realHover().findByLabelText("Replace").click();
modal().within(() => {
if (collectionName) {
cy.findByText(collectionName).click();
}
cy.findByText(nextQuestionName).click();
});
cy.wait("@cardQuery");
}

function assertDashCardTitle(title) {
cy.findByTestId("legend-caption-title").should("have.text", title);
}

function overwriteDashCardTitle(dashcardElement, textTitle) {
findDashCardAction(dashcardElement, "Show visualization options").click();
modal().within(() => {
cy.findByLabelText("Title").type(`{selectall}{del}${textTitle}`);
cy.button("Done").click();
});
}

function connectDashboardFilter(dashcardElement, { filterName, columnName }) {
const filterPanel = cy.findByTestId(
"edit-dashboard-parameters-widget-container",
);
filterPanel.findByText(filterName).click();
dashcardElement.button(/Select/).click();
popover().findByText(columnName).click();
filterPanel.findByText(filterName).click();
}

function assertDashboardFilterMapping(
dashcardElement,
{ filterName, expectedColumName },
) {
const filterPanel = cy.findByTestId(
"edit-dashboard-parameters-widget-container",
);
filterPanel.findByText(filterName).click();
dashcardElement.findByText(expectedColumName).should("exist");
filterPanel.findByText(filterName).click();
}
1 change: 0 additions & 1 deletion frontend/src/metabase-types/api/mocks/dashboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ export const createMockDashboardCard = (
card: createMockCard(),
created_at: "2020-01-01T12:30:30.000000",
updated_at: "2020-01-01T12:30:30.000000",
justAdded: false,
parameter_mappings: [],
...opts,
});
Expand Down
42 changes: 40 additions & 2 deletions frontend/src/metabase/dashboard/actions/cards.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@ import { createCard } from "metabase/lib/card";

import { getVisualizationRaw } from "metabase/visualizations";
import { autoWireParametersToNewCard } from "metabase/dashboard/actions/auto-wire-parameters/actions";
import { trackCardCreated } from "../analytics";
import { getDashCardById } from "../selectors";
import { trackCardCreated, trackQuestionReplaced } from "../analytics";
import { getDashCardById, getDashboardId } from "../selectors";
import { isVirtualDashCard } from "../utils";
import {
ADD_CARD_TO_DASH,
REMOVE_CARD_FROM_DASH,
UNDO_REMOVE_CARD_FROM_DASH,
setDashCardAttributes,
} from "./core";
import { cancelFetchCardData, fetchCardData } from "./data-fetching";
import { loadMetadataForDashboard } from "./metadata";
Expand Down Expand Up @@ -79,6 +80,43 @@ export const addCardToDashboard =
);
};

export const replaceCard =
({ dashcardId, nextCardId }) =>
async (dispatch, getState) => {
const dashboardId = getDashboardId(getState());

let dashcard = getDashCardById(getState(), dashcardId);
if (isVirtualDashCard(dashcard)) {
return;
}

await dispatch(Questions.actions.fetch({ id: nextCardId }));
const card = Questions.selectors
.getObject(getState(), { entityId: nextCardId })
.card();

await dispatch(
setDashCardAttributes({
id: dashcardId,
attributes: {
card,
card_id: card.id,
series: [],
parameter_mappings: [],
visualization_settings: {},
},
}),
);

dashcard = getDashCardById(getState(), dashcardId);

dispatch(fetchCardData(card, dashcard, { reload: true, clearCache: true }));
await dispatch(loadMetadataForDashboard([dashcard]));
dispatch(autoWireParametersToNewCard({ dashcard_id: dashcardId }));

trackQuestionReplaced(dashboardId);
};

export const removeCardFromDashboard = createThunkAction(
REMOVE_CARD_FROM_DASH,
({ dashcardId, cardId }) =>
Expand Down