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

feat(welcome): make examples tab customizable #22302

Merged
merged 14 commits into from
Dec 21, 2022

Conversation

villebro
Copy link
Member

@villebro villebro commented Dec 1, 2022

SUMMARY

Currently the Welcome page features an "Examples" tab for Dashboards and Charts. While this is ok for test environments utilizing the examples datasets/charts/dashboards, this is of little utility in production environments. To make the Welcome page more useful in Production environments, this PR adds a simple configuration option WELCOME_PAGE_LAST_TAB to show "All" charts/dashboards that would be accessible via the list views, and also makes it possible to define a custom title and filters for more custom use cases.

This also fixes the birth names example datasets, charts and dashboards, which unlike other examples, were created by the admin user, causing them not to show up on the Examples tab. I also decided to clean up some types along the way, and that turned into a pretty horrible journey 😱 I think I need to follow up with some additional cleanup later.

AFTER

By changing WELCOME_PAGE_LAST_TAB to "all", the last tab displays all dashboards/charts that the users would see in their respective list views:
image

By creating the following custom tab config, the last tab only shows charts/dashboards created by user number 10:

WELCOME_PAGE_LAST_TAB = ("Xyz", [{"col": 'created_by', "opr": 'rel_o_m', "value": 10}])

image

BEFORE

Previously only examples were available in the dashboard and chart sections:
image

TESTING INSTRUCTIONS

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

@codecov
Copy link

codecov bot commented Dec 1, 2022

Codecov Report

Merging #22302 (f5160cf) into master (2679ee2) will decrease coverage by 0.83%.
The diff coverage is 63.63%.

@@            Coverage Diff             @@
##           master   #22302      +/-   ##
==========================================
- Coverage   66.90%   66.07%   -0.84%     
==========================================
  Files        1850     1851       +1     
  Lines       70701    70693       -8     
  Branches     7750     7758       +8     
==========================================
- Hits        47306    46707     -599     
- Misses      21379    21967     +588     
- Partials     2016     2019       +3     
Flag Coverage Δ
hive ?
mysql ?
postgres ?
presto 52.35% <62.50%> (-0.01%) ⬇️
python 79.48% <62.50%> (-1.77%) ⬇️
sqlite 76.49% <62.50%> (-0.02%) ⬇️
unit 50.96% <12.50%> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
superset-frontend/src/featureFlags.ts 28.57% <ø> (ø)
superset-frontend/src/types/bootstrapTypes.ts 100.00% <ø> (ø)
superset-frontend/src/utils/localStorageHelpers.ts 90.00% <ø> (ø)
superset-frontend/src/utils/urlUtils.ts 50.90% <ø> (ø)
...tend/src/views/CRUD/data/database/DatabaseList.tsx 65.68% <ø> (ø)
superset-frontend/src/views/components/Menu.tsx 49.23% <ø> (ø)
...perset-frontend/src/views/components/RightMenu.tsx 78.23% <ø> (ø)
superset-frontend/src/views/components/SubMenu.tsx 86.88% <ø> (ø)
superset/examples/supported_charts_dashboard.py 0.00% <0.00%> (ø)
superset/views/base.py 76.28% <ø> (ø)
... and 52 more

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

@rusackas
Copy link
Member

rusackas commented Dec 1, 2022

This is a great tweak, and I appreciate all the cleanup work along the way. I also appreciate that the default config keeps the default functionality the same, so it's a seamless transition.

My only curiosity is if you think it's worth considering making ALL the tabs configurable... I can see just as many users not being concerned about "Favorites" or wanting to add a fourth tab fo their own (e.g. "My Team"). The config could be an array of tab definitions, perhaps... though on the other hand that might be pushing it in terms of complexity for a config option ;)

@villebro
Copy link
Member Author

villebro commented Dec 1, 2022

This is a great tweak, and I appreciate all the cleanup work along the way. I also appreciate that the default config keeps the default functionality the same, so it's a seamless transition.

My only curiosity is if you think it's worth considering making ALL the tabs configurable... I can see just as many users not being concerned about "Favorites" or wanting to add a fourth tab fo their own (e.g. "My Team"). The config could be an array of tab definitions, perhaps... though on the other hand that might be pushing it in terms of complexity for a config option ;)

That's a REALLY good idea. Let me see if this could be done easily.

@villebro
Copy link
Member Author

villebro commented Dec 7, 2022

@rusackas it turns out making ithis more generic would require a pretty substantial refactor, so I propose tackling that in a separate PR

@rusackas
Copy link
Member

rusackas commented Dec 8, 2022

/testenv up

Copy link
Member

@rusackas rusackas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all the cleanup in this! Changes seem sensible to me, though a second pair of eyes sure would be very welcomed - there's a lot to comb through here!

@rusackas
Copy link
Member

rusackas commented Dec 8, 2022

cc @eric-briscoe for a potential reviewer as well

@github-actions
Copy link
Contributor

github-actions bot commented Dec 8, 2022

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

@villebro
Copy link
Member Author

Thanks for the valuable reviews @kgabryje and @michael-s-molina ! ❤️ I'll address the comments shortly.

@villebro
Copy link
Member Author

/testenv up

@github-actions
Copy link
Contributor

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

@villebro
Copy link
Member Author

/testenv up

@github-actions
Copy link
Contributor

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

@villebro
Copy link
Member Author

/testenv up

@github-actions
Copy link
Contributor

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

@villebro
Copy link
Member Author

@jinghua-qa I've created an eph env http://54.245.133.177:8080/ where you can verify that the welcome page works as expected with the defaults (should display "Examples" charts/dashboards). Note the following:

  • I've created a chart and dashboard in the eph env to show that the "Mine" tab is working as expected. I've also favorited one own and one example chart/dashboard
  • Previous to this PR, the birth names dataset, charts and dashboards were displayed as being created by Admin user. That's now changed so that the birth names examples don't have a creator (same as the other examples).

@jinghua-qa
Copy link
Member

@jinghua-qa I've created an eph env http://54.245.133.177:8080/ where you can verify that the welcome page works as expected with the defaults (should display "Examples" charts/dashboards). Note the following:

  • I've created a chart and dashboard in the eph env to show that the "Mine" tab is working as expected. I've also favorited one own and one example chart/dashboard
  • Previous to this PR, the birth names dataset, charts and dashboards were displayed as being created by Admin user. That's now changed so that the birth names examples don't have a creator (same as the other examples).

Thank you Ville for the testing instruction!

Copy link
Member

@jinghua-qa jinghua-qa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested welcome page with default, LGTM

@villebro villebro merged commit b954f8f into apache:master Dec 21, 2022
@villebro villebro deleted the villebro/welcome branch December 21, 2022 07:28
@github-actions
Copy link
Contributor

Ephemeral environment shutdown and build artifacts deleted.

@codyml codyml mentioned this pull request Dec 21, 2022
9 tasks
@villebro villebro mentioned this pull request Dec 22, 2022
9 tasks
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 and removed 🚢 2.1.3 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels need:qa-review Requires QA review size/XXL 🚢 2.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants