feat(explore): Fill dashboard name when adding new chart from dashboard view #20129

kgabryje · 2022-05-19T10:49:54Z

SUMMARY

When user enters chart creation flow starting from a dashboard (e.g. with "Create chart" button in empty state or charts list in dashboard edit mode), the chart's save modal should have that dashboard's name prefilled. To achieve that, we pass dashboard_id as a url param to /chart/add page and from there to explore.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen.Recording.2022-05-19.at.12.43.24.mov

TESTING INSTRUCTIONS

Open a dashboard -> edit mode -> charts tab -> click "Create new chart" OR Open a dashboard and create a new chart using the plus icon button in top right corner.
Create a chart and click save
Verify that dashboard's name is prefilled
Create a new dashboard and click "Create new chart" button in empty state. Create a chart and verify that dashboard's name is prefilled in save modal
Create a chart from any other view (for example from Welcome page). Verify that save modal is not prefilled

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

CC @kasiazjc

kgabryje · 2022-05-19T10:54:49Z

superset-frontend/src/explore/components/ExploreViewContainer/index.jsx

@@ -699,12 +699,17 @@ function mapStateToProps(state) {
  const chartKey = Object.keys(charts)[0];
  const chart = charts[chartKey];

+  let dashboardId = Number(explore.form_data?.dashboardId);
+  if (Number.isNaN(dashboardId)) {


?? operator doesn't exclude NaNs. I also don't want to use || operator in order not to exclude dashboards with id 0

codecov · 2022-05-19T14:32:12Z

Codecov Report

Merging #20129 (b016a37) into master (c8fe518) will decrease coverage by 0.00%.
The diff coverage is 36.36%.

@@            Coverage Diff             @@
##           master   #20129      +/-   ##
==========================================
- Coverage   66.47%   66.47%   -0.01%     
==========================================
  Files        1727     1727              
  Lines       64724    64731       +7     
  Branches     6822     6824       +2     
==========================================
+ Hits        43024    43028       +4     
- Misses      19969    19970       +1     
- Partials     1731     1733       +2

Flag	Coverage Δ
javascript	`51.34% <36.36%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...rontend/src/dashboard/components/DashboardGrid.jsx	`62.50% <0.00%> (ø)`
...t-frontend/src/dashboard/components/SliceAdder.jsx	`66.17% <0.00%> (ø)`
...nd/src/dashboard/components/gridComponents/Tab.jsx	`83.67% <ø> (ø)`
...rontend/src/dashboard/containers/DashboardGrid.jsx	`100.00% <ø> (ø)`
...t-frontend/src/dashboard/containers/SliceAdder.jsx	`0.00% <ø> (ø)`
...rset-frontend/src/explore/components/SaveModal.tsx	`66.19% <ø> (ø)`
...perset-frontend/src/views/components/MenuRight.tsx	`59.79% <33.33%> (-0.85%)`	⬇️
...perset-frontend/src/addSlice/AddSliceContainer.tsx	`61.11% <50.00%> (-0.66%)`	⬇️
.../explore/components/ExploreViewContainer/index.jsx	`52.84% <66.66%> (+0.54%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c8fe518...b016a37. Read the comment docs.

…rd view

AAfghahi

Looks good! Just some questions, no feedback

AAfghahi · 2022-05-31T14:49:28Z

superset-frontend/src/addSlice/AddSliceContainer.tsx

    const formData = encodeURIComponent(
      JSON.stringify({
        viz_type: this.state.visType,
        datasource: this.state.datasource?.value,
+        ...(!isNullish(dashboardId) && { dashboardId }),


this is a cool function, why nullish?

!dashboardId would exclude dashboardId === 0, which is a valid id 😄

AAfghahi · 2022-05-31T14:52:02Z

superset-frontend/src/dashboard/stylesheets/dashboard.less

@@ -19,6 +19,7 @@
 /* header has mysterious extra margin */
 header.top {
  margin-bottom: 2px;
+  z-index: 10;


why is this necessary?

The main menu items on the right were not clickable because in edit mode the right panel was covering them (they were visible but unclickable because of invisible part of that panel covering them)

You can test that by opening dashboard in edit mode and then try to click the "+" icon in top right corner

AAfghahi · 2022-05-31T14:53:44Z

/testenv up

github-actions · 2022-05-31T14:55:54Z

@AAfghahi Ephemeral environment spinning up at http://34.221.68.91:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

github-actions · 2022-05-31T16:11:28Z

Ephemeral environment shutdown and build artifacts deleted.

…rd view (apache#20129) * feat(explore): Fill dashboard name when adding new chart from dashboard view * Update import paths * Update test * Fix test

kgabryje requested review from villebro, geido and michael-s-molina May 19, 2022 10:49

superset-github-bot bot added the preset-io label May 19, 2022

pull-request-size bot added the size/M label May 19, 2022

kgabryje commented May 19, 2022

View reviewed changes

AAfghahi mentioned this pull request May 20, 2022

feat: (Explore) Pre-populate dashboard in Explore #19964

Closed

9 tasks

kgabryje added 4 commits May 31, 2022 13:50

feat(explore): Fill dashboard name when adding new chart from dashboa…

8511426

…rd view

Update import paths

91b8de6

Update test

8d3be47

Fix test

b016a37

kgabryje force-pushed the feat/prefill-dashboard-name branch from 99de199 to b016a37 Compare May 31, 2022 11:50

AAfghahi approved these changes May 31, 2022

View reviewed changes

AAfghahi added the need:qa-review Requires QA review label May 31, 2022

kgabryje merged commit 3e3fbcc into apache:master May 31, 2022

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.0.0 labels Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(explore): Fill dashboard name when adding new chart from dashboard view #20129

feat(explore): Fill dashboard name when adding new chart from dashboard view #20129

kgabryje commented May 19, 2022

kgabryje May 19, 2022

codecov bot commented May 19, 2022 •

edited

Loading

AAfghahi left a comment

AAfghahi May 31, 2022

kgabryje May 31, 2022

AAfghahi May 31, 2022

kgabryje May 31, 2022

kgabryje May 31, 2022

AAfghahi commented May 31, 2022

github-actions bot commented May 31, 2022

github-actions bot commented May 31, 2022

feat(explore): Fill dashboard name when adding new chart from dashboard view #20129

feat(explore): Fill dashboard name when adding new chart from dashboard view #20129

Conversation

kgabryje commented May 19, 2022

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

kgabryje May 19, 2022

Choose a reason for hiding this comment

codecov bot commented May 19, 2022 • edited Loading

Codecov Report

AAfghahi left a comment

Choose a reason for hiding this comment

AAfghahi May 31, 2022

Choose a reason for hiding this comment

kgabryje May 31, 2022

Choose a reason for hiding this comment

AAfghahi May 31, 2022

Choose a reason for hiding this comment

kgabryje May 31, 2022

Choose a reason for hiding this comment

kgabryje May 31, 2022

Choose a reason for hiding this comment

AAfghahi commented May 31, 2022

github-actions bot commented May 31, 2022

github-actions bot commented May 31, 2022

codecov bot commented May 19, 2022 •

edited

Loading