feat(dashboard): add dashboard description field in modal and apis #17203

opus-42 · 2021-10-22T22:14:43Z

SUMMARY

This pull request add a dashboard description field in the dashboard APIs and in the dashboard modal UI.
This evolution is part of an effort to bring documentation of Superset object, discussion about this issue can be found in the following issue: #14510

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before

After

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Has associated issue: [dashboard] No UI to edit dashboard description #14510
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

codecov · 2021-10-25T11:54:33Z

Codecov Report

Merging #17203 (824dc71) into master (8c52fe3) will decrease coverage by 10.67%.
The diff coverage is n/a.

❗ Current head 824dc71 differs from pull request most recent head 85d3154. Consider uploading reports for the commit 85d3154 to get more accurate results

@@             Coverage Diff             @@
##           master   #17203       +/-   ##
===========================================
- Coverage   66.52%   55.84%   -10.68%     
===========================================
  Files        1641     1831      +190     
  Lines       63476    70002     +6526     
  Branches     6444     7570     +1126     
===========================================
- Hits        42227    39094     -3133     
- Misses      19585    28950     +9365     
- Partials     1664     1958      +294

Flag	Coverage Δ
hive	`52.88% <ø> (+0.28%)`	⬆️
javascript	`53.77% <ø> (+2.48%)`	⬆️
mysql	`?`
postgres	`?`
presto	`52.78% <ø> (+0.34%)`	⬆️
python	`58.09% <ø> (-24.20%)`	⬇️
sqlite	`?`
unit	`51.26% <ø> (?)`

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

Impacted Files	Coverage Δ
...ntrols/src/components/CertifiedIconWithTooltip.tsx	`80.00% <ø> (ø)`
...-ui-chart-controls/src/components/ColumnOption.tsx	`85.71% <ø> (ø)`
...src/components/ColumnTypeLabel/ColumnTypeLabel.tsx	`100.00% <ø> (ø)`
...controls/src/components/InfoTooltipWithTrigger.tsx	`100.00% <ø> (ø)`
...-ui-chart-controls/src/components/MetricOption.tsx	`90.90% <ø> (-3.83%)`	⬇️
...et-ui-chart-controls/src/components/SQLPopover.tsx	`100.00% <ø> (ø)`
...erset-ui-chart-controls/src/components/Tooltip.tsx	`80.00% <ø> (ø)`
...et-ui-chart-controls/src/components/labelUtils.tsx	`100.00% <ø> (+10.52%)`	⬆️
...ckages/superset-ui-chart-controls/src/constants.ts	`100.00% <ø> (ø)`
...ackages/superset-ui-chart-controls/src/fixtures.ts	`100.00% <ø> (ø)`
... and 1210 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

geido · 2021-10-25T12:00:27Z

Hello @opus-42 thanks for the contribution. It appears that you have some linting issues. Would you please fix those so that I can spin up a test env? Thank you!

superset-frontend/src/dashboard/components/PropertiesModal/index.jsx

villebro · 2021-10-25T13:16:08Z

@opus-42 I love this addition! It would also be great to add this as a tooltip next to the dashboard name on the Dashboard list view when defined. What do you think?

opus-42 · 2021-10-25T18:01:41Z

Hello @villebro, yes I think this is a great addition. I was not sure what the next step are (tooltip, additional field in the list, addition to the Dashboard UI...) and could be a good way to do it. Let me take a look at how to do it, or it can be kept for another pull request so that the display format can be discussed separately. What do you think ?

opus-42 · 2021-10-25T22:20:46Z

Here is a screenshot of last state of the pull request with a TextArea.

NB: Linting errors should be resolved.

opus-42 · 2021-11-14T20:38:06Z

Hello sorry, I had no time to get back to that feature, trying now to resolve the last front-end test that was blocking, linked to change in modal format. Should be hopefully good now (tested locally) ! Thks

riahk

@opus-42 This looks good! Can we get it rebased please?

opus-42 · 2021-11-24T22:35:59Z

Hello @riahk, thank you ! There was some merge conflicts with master. Could you check out everything is still fine after the merge ? Thanks again

riahk · 2021-11-29T20:22:56Z

@opus-42 It looks like your spec for PropertiesModal needs to be updated! A Certification section got added that needs to be accounted for. :) Other than that it still looks good!

opus-42 · 2021-12-16T22:53:23Z

@riahk
Thank you, I'll finalise this pull request now so that it can be integrated in subsequent versions.

opus-42 · 2022-01-10T17:34:29Z

Hello @riahk. Could you check if everything is ok to be merged ? (CI/CD needs review) Thanks 🙏

geido · 2022-01-11T14:13:35Z

Hello @opus-42. Thank you for moving this forward! I see there's is a conflict. Would you mind checking that out?

Also, if you are interested in contributing more to Superset and haven't already, join the Superset Slack . I go with the same username over there. I'll be glad to help you!

geido · 2022-02-08T12:37:30Z

/testenv up

github-actions · 2022-02-08T12:39:29Z

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

geido · 2022-02-08T13:34:30Z

Hello @opus-42 below some comments after manual testing your PR

When adding a description, applying, saving and then reopening the properties modal again, the description isn't appearing. This happens also when the Dashboard is reloaded, making me think it is not being saved.

Unicode.Test.3.mp4

What do you think of showing the description in the Dashboards list in a tooltip just after each Dashboard title?

opus-42 · 2022-02-08T13:43:19Z

When adding a description, applying, saving and then reopening the properties modal again, the description isn't appearing

Ok, this seems only to happen in the dashboard Edit Mode. I'll patch that.

What do you think of showing the description in the Dashboards list in a tooltip just after each Dashboard title?
Yes that would be a great addition. I'll look at it also.

opus-42 · 2022-02-08T18:32:43Z

@geido

Here is a proposition for the tooltip

Or maybe we could put the tool tip after the link (with SupersetTheme color as well)

NB: when put after there is some slight Icon shift on overing due to Tooltip default style

.ant-tooltip-open {
            display: inline-block;
            &::after {
              content: '';
              display: block;
            }
         }

geido · 2022-02-09T11:45:33Z

@geido

Here is a proposition for the tooltip

Or maybe we could put the tool tip after the link (with SupersetTheme color as well)

NB: when put after there is some slight Icon shift on overing due to Tooltip default style
.ant-tooltip-open {
            display: inline-block;
            &::after {
              content: '';
              display: block;
            }
         }

Hello @opus-42 thanks for the adding. I think the icon on the right is the proper place as it is consistent with the certification icon. I am running CI and spinning up a test env for more testing

opus-42 · 2022-02-09T19:14:30Z

@geido
Here is a proposition for the tooltip
Or maybe we could put the tool tip after the link (with SupersetTheme color as well)
NB: when put after there is some slight Icon shift on overing due to Tooltip default style
.ant-tooltip-open {
            display: inline-block;
            &::after {
              content: '';
              display: block;
            }
         }
Hello @opus-42 thanks for the adding. I think the icon on the right is the proper place as it is consistent with the certification icon. I am running CI and spinning up a test env for more testing

Thks. Let me finalize first, I haven't pushed the changes yet + test on my side, I'll tag you then.

…42/incubator-superset into feat/dashboard-description

rusackas · 2022-12-14T23:45:16Z

I'm for some reason no able to re-start the CI actions on this PR, which I hope are just flaky. I'm going to close and re-open this in hopes that it kick-starts things! 🤞

github-actions · 2022-12-14T23:45:41Z

Ephemeral environment shutdown and build artifacts deleted.

rusackas · 2023-02-02T19:50:41Z

@opus-42 it's been a while since this PR saw any love, but I think it's safe to say we're all still quite interested in getting this feature merged, if you're able to rebase the PR and help it across the finish line 🤞❤️

one-data-cookie · 2023-09-14T17:06:12Z

@michael-s-molina Isn't it a bit of shame to close this, since it's been almost done? Or are there any plans to implement it via a different PR?

michael-s-molina · 2023-09-14T17:08:59Z

@michael-s-molina Isn't it a bit of shame to close this, since it's been almost done? Or are there any plans to implement it via a different PR?

Yes, it is but the PR is stale as you can see by @rusackas comment that was not replied:

@opus-42 it's been a while since this PR saw any love, but I think it's safe to say we're all still quite interested in getting this feature merged, if you're able to rebase the PR and help it across the finish line 🤞❤️

@one-data-cookie Are you interested in reopening the PR and work on it?

one-data-cookie · 2023-09-14T17:15:06Z

@michael-s-molina Not at the moment, I'm afraid.

@opus-42 Any chance you'd be able to finish and merge this? 🤞

pull-request-size bot added the size/M label Oct 22, 2021

opus-42 changed the title ~~[WIP] feat(dashboard): add dashboard description field in modal and apis~~ feat(dashboard): add dashboard description field in modal and apis Oct 22, 2021

villebro reviewed Oct 25, 2021

View reviewed changes

superset-frontend/src/dashboard/components/PropertiesModal/index.jsx Outdated Show resolved Hide resolved

villebro added the need:enable-community label Nov 11, 2021

riahk reviewed Nov 24, 2021

View reviewed changes

snyk-bot mentioned this pull request Jan 15, 2022

[Snyk] Fix for 4 vulnerabilities XirdigH/superset#70

Closed

geido removed the need:enable-community label Feb 3, 2022

pull-request-size bot added size/S size/L and removed size/M size/S labels Feb 6, 2022

pull-request-size bot removed the size/L label Mar 6, 2022

Emmanuel Bavoux added 11 commits November 14, 2022 22:45

Change modal field to textarea and lint

93da5b0

fixing PropertiesModal test after rebase

5f11fa5

Fix description with typescript PropertyModal

c721e48

Fix dasbhoard properties Modal

4b47b06

Linting PropertiesModal.tsx

722464a

Compact dashboard api module

f4f23f6

Update description from dashboard edit mode

64413ea

Correct Lint typescript-react

929b247

Add dashboard information tooltip

dcf4bb4

Adding refresh tooltip after edit

2a2cbca

Merge branch 'feat/dashboard-description' of https://github.com/opus-…

85d3154

…42/incubator-superset into feat/dashboard-description

opus-42 requested review from jinghua-qa, geido, craig-rueda, dpgaspar, michael-s-molina, ktmud and a team as code owners November 14, 2022 21:50

snyk-bot mentioned this pull request Nov 15, 2022

[Snyk] Security upgrade @storybook/react from 5.3.21 to 6.4.13 m-ajay/superset#94

Open

m-ajay mentioned this pull request Nov 28, 2022

[Snyk] Security upgrade @storybook/react from 5.3.21 to 6.4.13 m-ajay/superset#99

Open

rusackas closed this Dec 14, 2022

rusackas reopened this Dec 14, 2022

rusackas mentioned this pull request Feb 2, 2023

[dashboard] No UI to edit dashboard description #14510

Closed

3 tasks

michael-s-molina closed this Sep 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(dashboard): add dashboard description field in modal and apis #17203

feat(dashboard): add dashboard description field in modal and apis #17203

opus-42 commented Oct 22, 2021 •

edited

Loading

codecov bot commented Oct 25, 2021 •

edited

Loading

geido commented Oct 25, 2021

villebro commented Oct 25, 2021

opus-42 commented Oct 25, 2021

opus-42 commented Oct 25, 2021

opus-42 commented Nov 14, 2021

riahk left a comment

opus-42 commented Nov 24, 2021

riahk commented Nov 29, 2021

opus-42 commented Dec 16, 2021

opus-42 commented Jan 10, 2022

geido commented Jan 11, 2022 •

edited

Loading

geido commented Feb 8, 2022

github-actions bot commented Feb 8, 2022

geido commented Feb 8, 2022

opus-42 commented Feb 8, 2022

opus-42 commented Feb 8, 2022 •

edited

Loading

geido commented Feb 9, 2022

opus-42 commented Feb 9, 2022

rusackas commented Dec 14, 2022

github-actions bot commented Dec 14, 2022

rusackas commented Feb 2, 2023

one-data-cookie commented Sep 14, 2023

michael-s-molina commented Sep 14, 2023

one-data-cookie commented Sep 14, 2023

feat(dashboard): add dashboard description field in modal and apis #17203

feat(dashboard): add dashboard description field in modal and apis #17203

Conversation

opus-42 commented Oct 22, 2021 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov bot commented Oct 25, 2021 • edited Loading

Codecov Report

geido commented Oct 25, 2021

villebro commented Oct 25, 2021

opus-42 commented Oct 25, 2021

opus-42 commented Oct 25, 2021

opus-42 commented Nov 14, 2021

riahk left a comment

Choose a reason for hiding this comment

opus-42 commented Nov 24, 2021

riahk commented Nov 29, 2021

opus-42 commented Dec 16, 2021

opus-42 commented Jan 10, 2022

geido commented Jan 11, 2022 • edited Loading

geido commented Feb 8, 2022

github-actions bot commented Feb 8, 2022

geido commented Feb 8, 2022

opus-42 commented Feb 8, 2022

opus-42 commented Feb 8, 2022 • edited Loading

geido commented Feb 9, 2022

opus-42 commented Feb 9, 2022

rusackas commented Dec 14, 2022

github-actions bot commented Dec 14, 2022

rusackas commented Feb 2, 2023

one-data-cookie commented Sep 14, 2023

michael-s-molina commented Sep 14, 2023

one-data-cookie commented Sep 14, 2023

opus-42 commented Oct 22, 2021 •

edited

Loading

codecov bot commented Oct 25, 2021 •

edited

Loading

geido commented Jan 11, 2022 •

edited

Loading

opus-42 commented Feb 8, 2022 •

edited

Loading