You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/05/23 23:57:49 UTC
[GitHub] [superset] lyndsiWilliams opened a new pull request, #20166: feat(explore): Dataset Panel Options when Source = Query
lyndsiWilliams opened a new pull request, #20166:
URL: https://github.com/apache/superset/pull/20166
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
This adds a different dataset panel dropdown when a chart's datasource is a query.
### SCREENSHOTS / ANIMATED GIF
<!--- Skip this if not applicable -->
<img width="695" alt="queryDatasourcePanel" src="https://user-images.githubusercontent.com/55605634/169922171-4ff59a63-8d69-4b44-8351-44611c76e967.png">
<img width="1339" alt="queryDatasourceSaveDatasetModal" src="https://user-images.githubusercontent.com/55605634/169922178-cacf88c2-d6ac-4cfc-857a-b34465812f0e.png">
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
- Open a chart with a query datasource type
- Click the thee dots next to the chart name to reveal the dataset options dropdown
- Observe that the options are now "Query preview", "View in SQL Lab", and "Save as dataset"
- Click "View in SQL Lab" to be taken to SQL Lab
- Click "Save as dataset" and observe the save dataset modal
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] lyndsiWilliams commented on pull request #20166: feat(explore): Dataset Panel Options when Source = Query
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on PR #20166:
URL: https://github.com/apache/superset/pull/20166#issuecomment-1149199354
Reopened in this PR: https://github.com/apache/superset/pull/20299
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] pkdotson commented on a diff in pull request #20166: feat(explore): Dataset Panel Options when Source = Query
Posted by GitBox <gi...@apache.org>.
pkdotson commented on code in PR #20166:
URL: https://github.com/apache/superset/pull/20166#discussion_r882307501
##########
superset-frontend/plugins/plugin-chart-handlebars/src/plugin/controls/metrics.tsx:
##########
@@ -35,10 +36,10 @@ const percentMetrics: typeof sharedControls.metrics = {
visibility: isAggMode,
resetOnHide: false,
mapStateToProps: ({ datasource, controls }, controlState) => ({
- columns: datasource?.columns || [],
- savedMetrics: datasource?.metrics || [],
+ columns: (datasource as Dataset)?.columns || [],
Review Comment:
nvm i see it int he type. lol
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] pkdotson commented on a diff in pull request #20166: feat(explore): Dataset Panel Options when Source = Query
Posted by GitBox <gi...@apache.org>.
pkdotson commented on code in PR #20166:
URL: https://github.com/apache/superset/pull/20166#discussion_r880951364
##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx:
##########
@@ -36,15 +36,16 @@ export const dndGroupByControl: SharedControlConfig<'DndColumnSelect'> = {
),
mapStateToProps(state, { includeTime }) {
const newState: ExtraControlProps = {};
+ const dataset = state.datasource as Dataset;
if (state.datasource) {
Review Comment:
nit: if (dataset) ? to save some chars
##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx:
##########
@@ -36,15 +36,16 @@ export const dndGroupByControl: SharedControlConfig<'DndColumnSelect'> = {
),
mapStateToProps(state, { includeTime }) {
const newState: ExtraControlProps = {};
+ const dataset = state.datasource as Dataset;
if (state.datasource) {
Review Comment:
nit: if (dataset) ? to save some chars
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] lyndsiWilliams closed pull request #20166: feat(explore): Dataset Panel Options when Source = Query
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams closed pull request #20166: feat(explore): Dataset Panel Options when Source = Query
URL: https://github.com/apache/superset/pull/20166
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] pkdotson commented on a diff in pull request #20166: feat(explore): Dataset Panel Options when Source = Query
Posted by GitBox <gi...@apache.org>.
pkdotson commented on code in PR #20166:
URL: https://github.com/apache/superset/pull/20166#discussion_r880967435
##########
superset-frontend/plugins/plugin-chart-handlebars/src/plugin/controls/metrics.tsx:
##########
@@ -35,10 +36,10 @@ const percentMetrics: typeof sharedControls.metrics = {
visibility: isAggMode,
resetOnHide: false,
mapStateToProps: ({ datasource, controls }, controlState) => ({
- columns: datasource?.columns || [],
- savedMetrics: datasource?.metrics || [],
+ columns: (datasource as Dataset)?.columns || [],
Review Comment:
from my understanding this will be dataset and query and table correct? will there be. type that encompass all three?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] pkdotson commented on a diff in pull request #20166: feat(explore): Dataset Panel Options when Source = Query
Posted by GitBox <gi...@apache.org>.
pkdotson commented on code in PR #20166:
URL: https://github.com/apache/superset/pull/20166#discussion_r880966111
##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/index.tsx:
##########
@@ -131,8 +132,9 @@ const groupByControl: SharedControlConfig<'SelectControl', ColumnMeta> = {
promptTextCreator: (label: unknown) => label,
mapStateToProps(state, { includeTime }) {
const newState: ExtraControlProps = {};
+ const dataset = state.datasource as Dataset;
if (state.datasource) {
Review Comment:
same here
##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/index.tsx:
##########
@@ -131,8 +132,9 @@ const groupByControl: SharedControlConfig<'SelectControl', ColumnMeta> = {
promptTextCreator: (label: unknown) => label,
mapStateToProps(state, { includeTime }) {
const newState: ExtraControlProps = {};
+ const dataset = state.datasource as Dataset;
if (state.datasource) {
Review Comment:
same here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org