Add "Replace question" action to dashboard cards #36744

kulyk · 2023-12-13T11:30:41Z

Adds a "Replace" button to dashboard cards with questions or models. It makes it possible to swap the dashboard card's query while keeping the dashboard layout intact. It's important to call out that we would reset properties of a dash card that no longer makes sense with a new query: parameter mappings, click behavior, visualization settings, and added series.

How to verify

Open any dashboard that has a few cards
Enter the editing mode
Hover a dashboard card using a question or a model
Find and click the "Replace" action in the top-right floating panel
Select a new question or a model from the modal picker
Ensure the dashboard card's content is replaced, and the query is rerun
Ensure parameters auto-wiring worked (your dashboard should have another card using the same table and mapped to some parameters)
Ensure you can undo the revert
Ensure changes are persisted after clicking "Save"

Demo

Basic

CleanShot.2023-12-13.at.11.25.41.mp4

Filters auto-wiring

CleanShot.2023-12-13.at.11.29.29.mp4

Checklist

Tests have been added/updated to cover changes in this PR

deploysentinel · 2023-12-13T11:43:56Z

No failed tests 🎉

Previously, `card_id` in `update-dashboard-card!` only applied if the dashcard was an action card. There was a cryptic comment about this, but it also indicates that maybe it's ok to update. This unsticks FE work but still needs BE unit tests.

The new test checks the state of a dashboard with a GET then PUTs the update setting the new card id and verifies the result.

Under the old behavior, only action dashcards allowed changing their card ids. Now card ids can be changed for any dashcard type. This test captured the old behavior and has been updated to capture the new behavior.

…o 36497-replace-dashcard-card

snoe

BE LGTM

calherries · 2023-12-13T17:50:04Z

If Case is cool with updating card IDs, you should change the logic in models.dashboard-card.update-dashboard-card! which prevents it

metabase/src/metabase/models/dashboard_card.clj

Lines 182 to 184 in ffe0385

    
           ;; Allow changing card_id for action dashcards, but not for card dashcards. 
        
           ;; This is to preserve the existing behavior of questions and card_id 
        
           ;; I don't know why card_id couldn't be changed for cards though.

markbastian · 2023-12-13T17:54:35Z

If Case is cool with updating card IDs, you should change the logic in models.dashboard-card.update-dashboard-card! which prevents it

metabase/src/metabase/models/dashboard_card.clj

Lines 182 to 184 in ffe0385

;; Allow changing card_id for action dashcards, but not for card dashcards.

;; This is to preserve the existing behavior of questions and card_id

;; I don't know why card_id couldn't be changed for cards though.

Isn't this addressed here? https://github.com/metabase/metabase/pull/36744/files#diff-b6abb82aa02f4647f6ac74d17bfa75dda523bf75c2eed11954c56127a6d7446dR180-R189

calherries · 2023-12-13T18:12:09Z

Ah, so sorry, I didn't see that was part of the PR. I just read the test. All clear then!

alxnddr · 2023-12-13T21:49:24Z

It would be great if users could replace errored cards, I think it can be a valid use case for the functionality

alxnddr · 2023-12-13T22:00:12Z

frontend/src/metabase/dashboard/selectors/selectors-typed.ts

@@ -5,6 +5,10 @@ export function getDashboardId(state: State) {
  return state.dashboard.dashboardId;
 }

+export function getDashboardBeforeEditing(state: State) {
+  return state.dashboard.isEditing;


Oh, isEditing is a dashboard instead of a boolean 🤯

I'm afraid you're right 😢

alxnddr

Nice and clean! Please have a look at the suggestion about errored cards

kulyk · 2023-12-14T12:24:28Z

It would be great if users could replace errored cards, I think it can be a valid use case for the functionality

@alxnddr, love the idea, added in 92bc305. Thanks!

Demo

CleanShot.2023-12-14.at.12.10.19.mp4

cypress · 2023-12-14T12:50:22Z

Passing run #200 ↗︎

0	139	9	0	0

Details:

Add "Replace question" action to dashboard cards
Project: Metabase e2e	Commit: `5762787c85`
Status: Passed	Duration: 09:00 💡
Started: Dec 14, 2023 12:59 PM	Ended: Dec 14, 2023 1:08 PM

Review all test suite changes for PR #36744 ↗︎

perivamsi · 2023-12-14T12:55:28Z

It would be great if users could replace errored cards, I think it can be a valid use case for the functionality

@alxnddr, love the idea, added in 92bc305. Thanks!

Demo

If it is possible to show the "replace card" prompt directly on the error card, that will be helpful.

So change "There was a problem displaying this card." by adding a "replace it" link or button at the end? This is more discoverable.

kulyk · 2023-12-14T14:34:13Z

It would be great if users could replace errored cards, I think it can be a valid use case for the functionality

@alxnddr, love the idea, added in 92bc305. Thanks!

Demo

If it is possible to show the "replace card" prompt directly on the error card, that will be helpful.

So change "There was a problem displaying this card." by adding a "replace it" link or button at the end? This is more discoverable.

For context, we figured we'd prefer to keep the action as is. People might prefer to fix the issue instead of replacing the card

github-actions · 2023-12-14T14:34:50Z

@kulyk Did you forget to add a milestone to the issue for this PR? When and where should I add a milestone?

Part of #36497 ### Description In #36744 we tracked dashcard replacement with a new event name, but did not add this event name to the dashboard snowplow schema. In this PR we add the name to the schema, and update an e2e test to ensure the event is being tracked. ### How to verify Follow this [guide](https://www.notion.so/metabase/Snowplow-integration-5da1f874beda4153b4fccfa6c1e77caa) to run snowplow, then try replacing a dashcard locally, you should see the log in the console. ### Demo https://github.com/metabase/metabase/assets/37751258/e8566f09-b9b8-47de-b215-fe0e25c7d920 ### Checklist - [x] Tests have been added/updated to cover changes in this PR

kulyk added no-backport Do not backport this PR to any branch .Team/DashViz Dashboard and Viz team labels Dec 13, 2023

kulyk requested review from kdoh, cdeweyx and a team December 13, 2023 11:30

kulyk self-assigned this Dec 13, 2023

kulyk force-pushed the 36497-replace-dashcard-card branch from e364299 to 94dd166 Compare December 13, 2023 13:28

kulyk changed the base branch from master to handle-mantine-modals-in-e2e December 13, 2023 13:29

Base automatically changed from handle-mantine-modals-in-e2e to master December 13, 2023 14:34

kulyk and others added 18 commits December 13, 2023 14:35

Remove isVirtualDashCard prop

c4ccf2e

Add "replace" button to DashCardActionsPanel

9e0a584

Add QuestionPickerModal

441d504

Add replaceCard action

6b7f0ee

Use /card/:id/query endpoint for swapped cards

20194a1

Implement card replace

7e21043

Add basic undo toast

2f2ffd6

update-dashboard-card will update :card_id

7986019

Previously, `card_id` in `update-dashboard-card!` only applied if the dashcard was an action card. There was a cryptic comment about this, but it also indicates that maybe it's ok to update. This unsticks FE work but still needs BE unit tests.

Adding unit test for updating dashcard's card

b746d46

The new test checks the state of a dashboard with a GET then PUTs the update setting the new card id and verifies the result.

Clean up DashCard test suite

a8232aa

Add tests for DashCard component

86c0c1f

Add tests for QuestionPickerModal component

c9fe261

Add tests for replaceCard action

8eb23ad

Remove redundant action argument

b0ba9b6

Add test for parameters auto-wiring

a099d1b

Fix unexpected card query endpoint usage

e11c81d

Use more accessible selector in saveDashboard

f575c91

Add e2e tests

efde45a

markbastian self-assigned this Dec 13, 2023

markbastian and others added 4 commits December 13, 2023 09:45

Updating update-action-cards-test test

8b2a89f

Under the old behavior, only action dashcards allowed changing their card ids. Now card ids can be changed for any dashcard type. This test captured the old behavior and has been updated to capture the new behavior.

Remove previous events

eff7b4c

Track dashboard_card_replaced event

bf2db12

Merge remote-tracking branch 'origin/36497-replace-dashcard-card' int…

c8249d2

…o 36497-replace-dashcard-card

markbastian requested review from qnkhuat, calherries and snoe December 13, 2023 17:09

snoe approved these changes Dec 13, 2023

View reviewed changes

alxnddr reviewed Dec 13, 2023

View reviewed changes

alxnddr approved these changes Dec 13, 2023

View reviewed changes

kulyk added 2 commits December 14, 2023 11:53

Merge branch 'master' into 36497-replace-dashcard-card

6e00cd2

Show "replace" action for erroring queries

92bc305

kulyk merged commit d478a78 into master Dec 14, 2023
106 checks passed

kulyk deleted the 36497-replace-dashcard-card branch December 14, 2023 14:34

kulyk added this to the 0.49 milestone Dec 14, 2023

kulyk mentioned this pull request Dec 20, 2023

Fix dashcard query sometimes errors after replacing its card #36987

Merged

EmmadUsmani mentioned this pull request Feb 2, 2024

add dashcard replace event to snowplow schema and e2e test #38411

Merged

1 task

kulyk mentioned this pull request Feb 7, 2024

Dashboard sections #38463

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add "Replace question" action to dashboard cards #36744

Add "Replace question" action to dashboard cards #36744

kulyk commented Dec 13, 2023

deploysentinel bot commented Dec 13, 2023 •

edited

snoe left a comment

calherries commented Dec 13, 2023

markbastian commented Dec 13, 2023

calherries commented Dec 13, 2023 •

edited

alxnddr commented Dec 13, 2023

alxnddr Dec 13, 2023

kulyk Dec 14, 2023

alxnddr left a comment

kulyk commented Dec 14, 2023 •

edited

cypress bot commented Dec 14, 2023 •

edited

perivamsi commented Dec 14, 2023

kulyk commented Dec 14, 2023

github-actions bot commented Dec 14, 2023

Add "Replace question" action to dashboard cards #36744

Add "Replace question" action to dashboard cards #36744

Conversation

kulyk commented Dec 13, 2023

How to verify

Demo

Checklist

deploysentinel bot commented Dec 13, 2023 • edited

snoe left a comment

Choose a reason for hiding this comment

calherries commented Dec 13, 2023

markbastian commented Dec 13, 2023

calherries commented Dec 13, 2023 • edited

alxnddr commented Dec 13, 2023

alxnddr Dec 13, 2023

Choose a reason for hiding this comment

kulyk Dec 14, 2023

Choose a reason for hiding this comment

alxnddr left a comment

Choose a reason for hiding this comment

kulyk commented Dec 14, 2023 • edited

cypress bot commented Dec 14, 2023 • edited

Passing run #200 ↗︎

Review all test suite changes for PR #36744 ↗︎

perivamsi commented Dec 14, 2023

kulyk commented Dec 14, 2023

github-actions bot commented Dec 14, 2023

deploysentinel bot commented Dec 13, 2023 •

edited

calherries commented Dec 13, 2023 •

edited

kulyk commented Dec 14, 2023 •

edited

cypress bot commented Dec 14, 2023 •

edited