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: added support to configure the default explorer viz #13610

Merged
merged 3 commits into from
Mar 26, 2021
Merged

feat: added support to configure the default explorer viz #13610

merged 3 commits into from
Mar 26, 2021

Conversation

cccs-jc
Copy link
Contributor

@cccs-jc cccs-jc commented Mar 13, 2021

Added support for configuring what the default visualization used by the chart explorer.

@codecov
Copy link

codecov bot commented Mar 13, 2021

Codecov Report

Merging #13610 (cbf3e77) into master (67ffea8) will increase coverage by 0.09%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #13610      +/-   ##
==========================================
+ Coverage   77.31%   77.40%   +0.09%     
==========================================
  Files         911      934      +23     
  Lines       46378    47250     +872     
  Branches     5613     5798     +185     
==========================================
+ Hits        35858    36575     +717     
- Misses      10384    10531     +147     
- Partials      136      144       +8     
Flag Coverage Δ
cypress 56.04% <100.00%> (-0.67%) ⬇️
hive ?
javascript 63.73% <0.00%> (+0.56%) ⬆️
mysql 80.58% <100.00%> (+0.22%) ⬆️
postgres 80.61% <100.00%> (+0.20%) ⬆️
presto 80.27% <100.00%> (+0.19%) ⬆️
python 80.86% <100.00%> (-0.06%) ⬇️
sqlite 80.17% <100.00%> (+0.15%) ⬆️

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

Impacted Files Coverage Δ
superset/views/base.py 75.73% <ø> (ø)
superset-frontend/src/explore/store.js 92.85% <100.00%> (ø)
superset/config.py 90.74% <100.00%> (+0.03%) ⬆️
superset/db_engines/hive.py 0.00% <0.00%> (-85.72%) ⬇️
superset/db_engine_specs/hive.py 74.23% <0.00%> (-16.54%) ⬇️
...odal/FiltersConfigForm/FilterScope/FilterScope.tsx 86.20% <0.00%> (-9.95%) ⬇️
...src/dashboard/components/FiltersBadge/selectors.ts 80.58% <0.00%> (-9.90%) ⬇️
superset-frontend/src/middleware/asyncEvent.ts 78.03% <0.00%> (-9.32%) ⬇️
...ents/nativeFilters/FilterBar/FilterSets/Footer.tsx 47.61% <0.00%> (-8.64%) ⬇️
superset-frontend/src/common/components/Select.tsx 91.66% <0.00%> (-8.34%) ⬇️
... and 178 more

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 67ffea8...cbf3e77. Read the comment docs.

@junlincc junlincc added the new:contributor The author is a new contributor label Mar 14, 2021
@junlincc junlincc requested a review from villebro March 14, 2021 05:52
@junlincc junlincc changed the title added support to configure the default explorer viz feat: added support to configure the default explorer viz Mar 14, 2021
@junlincc
Copy link
Member

@cccs-jc thanks for the contribution! could you provide more details what this change is for, in PR description? thanks!

@junlincc junlincc added the !deprecated-label:viz:explore:others Deprecated label - Use explore instead label Mar 14, 2021
@cccs-jc
Copy link
Contributor Author

cccs-jc commented Mar 15, 2021

We are building a custom visualization based on agGrid and would like this custom table to be the default visualization when a user starts a chart explorer.

This also enables us to control the default row limit and whether to start in aggregation or raw query mode.

@cccs-jc
Copy link
Contributor Author

cccs-jc commented Mar 16, 2021

@junlincc did that answer your question?

@junlincc
Copy link
Member

@cccs-jc thanks for the answer, is this change going to affect current plug-in structure? are you building the custom viz using dynamic viz plug-in?

cc @suddjian

@junlincc junlincc requested review from suddjian and removed request for villebro March 17, 2021 05:35
Copy link
Member

@suddjian suddjian left a comment

Choose a reason for hiding this comment

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

This looks good. Shouldn't affect any existing features @junlincc. Just a config option for what viz is the default in the viz picker.

Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

This is a good idea - a minor improvement proposal, other than that LGTM

superset/config.py Outdated Show resolved Hide resolved
@cccs-jc
Copy link
Contributor Author

cccs-jc commented Mar 19, 2021

@villebro Did the change you suggested. Renamed config to DEFAULT_VIZ_TYPE

@villebro
Copy link
Member

@cccs-jc LGTM, but you need to fix the linting error:

/home/runner/work/superset/superset/superset-frontend/src/explore/store.js
  44:18  error  Insert `⏎···`  prettier/prettier

@cccs-jc
Copy link
Contributor Author

cccs-jc commented Mar 23, 2021

@villebro I'm not sure what the linter is complaining about. I check the line break in case it might be a windows line break. But looks good to me..

@suddjian
Copy link
Member

The issue is Prettier's formatting guidelines says the line is too long and wants it broken up. You can run npm run format to auto-fix.

@suddjian suddjian merged commit 24f805e into apache:master Mar 26, 2021
@ktmud
Copy link
Member

ktmud commented Mar 26, 2021

It would be great to allow setting this per user profile! (another reason to add meta storage for user configs)

amitmiran137 pushed a commit that referenced this pull request Mar 31, 2021
* master: (56 commits)
  test: Adds tests and storybook to CertifiedIcon component (#13457)
  chore: Moves CheckboxIcons to Checkbox folder (#13459)
  chore: Removes Popover duplication (#13462)
  build(deps): bump elliptic from 6.5.3 to 6.5.4 in /docs (#13527)
  fix: allow spaces in DB names (#13800)
  chore: Update PR template for SIP-59 DB migrations process (#13855)
  Add CODEOWNERS (#13759)
  feat(alerts & reports): Easier to read execution logs (#13752)
  fix: Disallows negative options remaining (#13749)
  Fix broken link (#13861)
  fix(native-filters): add global async query support to native filters (#13837)
  Displays row limit warning with Alert component (#13854)
  fix(errors): Downgrade error on stop query to a warning (#13826)
  fix(alerts and reports): Unify timestamp format on execution log view (#13718)
  fix(sqllab): warning message when rows limited (#13841)
  chore: add success log whenever a connection is working (#13811)
  fix(native-filters): improve loading styles for filter component (#13794)
  chore: update change log with cherry-picks for release 1.1 (#13824)
  feat: added support to configure the default explorer viz (#13610)
  fix(#13734): Properly escape special characters in CSV output  (#13735)
  ...
allanco91 pushed a commit to allanco91/superset that referenced this pull request May 21, 2021
* added support to configure the default explorer viz

* code review fix

* lint

Co-authored-by: cccs-jc <cccs-jc@cyber.gc.ca>
Co-authored-by: Ville Brofeldt <ville.v.brofeldt@gmail.com>
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.2.0 labels Mar 12, 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 !deprecated-label:viz:explore:others Deprecated label - Use explore instead new:contributor The author is a new contributor size/XS 🚢 1.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants