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/07/21 16:11:00 UTC

[GitHub] [superset] AAfghahi opened a new pull request, #20809: feat: Create Chart onClick Functionality

AAfghahi opened a new pull request, #20809:
URL: https://github.com/apache/superset/pull/20809

   <!---
   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 changes the functionality of the create chart button to be in line with the new feature of a query powered chart. Essentially when the create chart button is now clicked the user will have a new tab open with a table chart using all their current data. 
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   
   https://user-images.githubusercontent.com/48933336/180262255-7d78988c-578a-444b-8ac0-2922bc842875.mov
   
   
   
   
   ### TESTING INSTRUCTIONS
   <!--- Required! What steps can be taken to manually verify the changes? -->
   
   ### 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:
   - [ ] 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] hughhhh commented on a diff in pull request #20809: feat: Create Chart onClick Functionality

Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #20809:
URL: https://github.com/apache/superset/pull/20809#discussion_r926949260


##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -213,6 +219,26 @@ export default class ResultSet extends React.PureComponent<
     }
   }
 
+  async createExploreResultsOnClick() {
+    const { results } = this.props.query;
+
+    if (results.query_id) {
+      const key = await postFormData(results.query_id, 'table', {

Review Comment:
   the type should be a `query`



-- 
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] hughhhh commented on a diff in pull request #20809: feat: Create Chart onClick Functionality

Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #20809:
URL: https://github.com/apache/superset/pull/20809#discussion_r926873975


##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -223,7 +248,7 @@ export default class ResultSet extends React.PureComponent<
       // Added compute logic to stop user from being able to Save & Explore
       const { showSaveDatasetModal } = this.state;
       const { query } = this.props;
-
+      console.log('this is query', query);

Review Comment:
   remove this console.log



-- 
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] AAfghahi commented on pull request #20809: feat: Create Chart onClick Functionality

Posted by GitBox <gi...@apache.org>.
AAfghahi commented on PR #20809:
URL: https://github.com/apache/superset/pull/20809#issuecomment-1191684786

   /testenv up
   


-- 
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] AAfghahi merged pull request #20809: feat: Create Chart onClick Functionality

Posted by GitBox <gi...@apache.org>.
AAfghahi merged PR #20809:
URL: https://github.com/apache/superset/pull/20809


-- 
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] github-actions[bot] commented on pull request #20809: feat: Create Chart onClick Functionality

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #20809:
URL: https://github.com/apache/superset/pull/20809#issuecomment-1192001800

   @AAfghahi Container image not yet published for this PR. Please try again when build is complete.


-- 
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] github-actions[bot] commented on pull request #20809: feat: Create Chart onClick Functionality

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #20809:
URL: https://github.com/apache/superset/pull/20809#issuecomment-1192001806

   @AAfghahi Ephemeral environment creation failed. Please check the Actions logs for details.


-- 
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] AAfghahi commented on a diff in pull request #20809: feat: Create Chart onClick Functionality

Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #20809:
URL: https://github.com/apache/superset/pull/20809#discussion_r926921228


##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -213,6 +219,28 @@ export default class ResultSet extends React.PureComponent<
     }
   }
 
+  async createExploreResultsOnClick() {
+    const { results } = this.props.query;
+    // There is currently redux / state issue where sometimes a query will have serverId

Review Comment:
   changed
   



-- 
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] hughhhh commented on a diff in pull request #20809: feat: Create Chart onClick Functionality

Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #20809:
URL: https://github.com/apache/superset/pull/20809#discussion_r926949422


##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -213,6 +219,26 @@ export default class ResultSet extends React.PureComponent<
     }
   }
 
+  async createExploreResultsOnClick() {
+    const { results } = this.props.query;
+
+    if (results.query_id) {
+      const key = await postFormData(results.query_id, 'table', {
+        ...EXPLORE_CHART_DEFAULT,
+        datasource: `${results.query_id}__table`,

Review Comment:
   type should be `query`



-- 
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] AAfghahi commented on a diff in pull request #20809: feat: Create Chart onClick Functionality

Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #20809:
URL: https://github.com/apache/superset/pull/20809#discussion_r926917532


##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -213,6 +219,28 @@ export default class ResultSet extends React.PureComponent<
     }
   }
 
+  async createExploreResultsOnClick() {
+    const { results } = this.props.query;
+    // There is currently redux / state issue where sometimes a query will have serverId

Review Comment:
   No I didn't, but wasn't sure so kept it in. Will delete. 



-- 
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] hughhhh commented on a diff in pull request #20809: feat: Create Chart onClick Functionality

Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #20809:
URL: https://github.com/apache/superset/pull/20809#discussion_r926875374


##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -250,18 +275,7 @@ export default class ResultSet extends React.PureComponent<
               this.props.database?.allows_virtual_table_explore && (
                 <ExploreResultsButton
                   database={this.props.database}
-                  onClick={() => {
-                    // There is currently redux / state issue where sometimes a query will have serverId
-                    // and other times it will not.  We need this attribute consistently for this to work
-                    // const qid = this.props?.query?.results?.query_id;
-                    // if (qid) {
-                    //   // This will open explore using the query as datasource
-                    //   window.location.href = `/explore/?dataset_type=query&dataset_id=${qid}`;
-                    // } else {
-                    //   this.setState({ showSaveDatasetModal: true });
-                    // }
-                    this.setState({ showSaveDatasetModal: true });
-                  }}
+                  onClick={this.createExploreResultsOnClick}

Review Comment:
   we still want to hide this feature so leave the original code in place, and add a comment saying to enable chart source to query change the onClick to `onClick={this.createExploreResultsOnClick}`



-- 
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] eric-briscoe commented on a diff in pull request #20809: feat: Create Chart onClick Functionality

Posted by GitBox <gi...@apache.org>.
eric-briscoe commented on code in PR #20809:
URL: https://github.com/apache/superset/pull/20809#discussion_r926961562


##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -213,6 +219,26 @@ export default class ResultSet extends React.PureComponent<
     }
   }
 
+  async createExploreResultsOnClick() {
+    const { results } = this.props.query;
+
+    if (results.query_id) {
+      const key = await postFormData(results.query_id, 'table', {
+        ...EXPLORE_CHART_DEFAULT,
+        datasource: `${results.query_id}__table`,
+        ...{
+          all_columns: results.columns.map(column => column.name),
+        },
+      });
+      const url = mountExploreUrl(null, {
+        [URL_PARAMS.formDataKey.name]: key,
+      });
+      window.open(url, '_blank', 'noreferrer');
+    } else {
+      this.setState({ showSaveDatasetModal: true });

Review Comment:
   I think we can remove this `this.setState({ showSaveDatasetModal: true });.`  Initially I was trying to use serverId which was not always present and just defaulted to the original behavior.  with use of queryId, it should always be there and if it is not we have a bug.  We may to change the else to show an error toast indicating the query has no id and cannot be used to create a chart.  This should never happen and provides visual indication if something is broken



##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -213,6 +219,28 @@ export default class ResultSet extends React.PureComponent<
     }
   }
 
+  async createExploreResultsOnClick() {

Review Comment:
   if we use arrow function syntax here, you don't have to add the .bind call on line 141,142:
   `async createExploreResultsOnClick = () => {`
   Arrow functions automatically bind `this` to where the function is defined which has the same effect as calling `this.createExploreResultsOnClick =
         this.createExploreResultsOnClick.bind(this);`
   I have been updating files I work in to convert to arrow functions and remove the bind calls because it removes the second step and extra code.  



##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -213,6 +219,26 @@ export default class ResultSet extends React.PureComponent<
     }
   }
 
+  async createExploreResultsOnClick() {
+    const { results } = this.props.query;
+
+    if (results.query_id) {
+      const key = await postFormData(results.query_id, 'table', {

Review Comment:
   One thing I noticed with this await is that if you set your network speed in dev tools to slow 3G to represent a poor connection, the UI looks unresponsive after clicking the button.  We could consider setting state before the await to  trigger a re-render and show UI that indicates the user's button clicked is being processed.  Not critical, and maybe a separate ticket, but these types of things often get missed in our local dev environment where responses are near instant and have no real world latency / bandwidth restrictions



-- 
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] AAfghahi commented on a diff in pull request #20809: feat: Create Chart onClick Functionality

Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #20809:
URL: https://github.com/apache/superset/pull/20809#discussion_r926888448


##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -250,18 +275,7 @@ export default class ResultSet extends React.PureComponent<
               this.props.database?.allows_virtual_table_explore && (
                 <ExploreResultsButton
                   database={this.props.database}
-                  onClick={() => {
-                    // There is currently redux / state issue where sometimes a query will have serverId
-                    // and other times it will not.  We need this attribute consistently for this to work
-                    // const qid = this.props?.query?.results?.query_id;
-                    // if (qid) {
-                    //   // This will open explore using the query as datasource
-                    //   window.location.href = `/explore/?dataset_type=query&dataset_id=${qid}`;
-                    // } else {
-                    //   this.setState({ showSaveDatasetModal: true });
-                    // }
-                    this.setState({ showSaveDatasetModal: true });
-                  }}
+                  onClick={this.createExploreResultsOnClick}

Review Comment:
   yeah that makes sense, just did that, though will reverse it after this has gone through code review so that QA can test the new flow. 



-- 
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] eric-briscoe commented on a diff in pull request #20809: feat: Create Chart onClick Functionality

Posted by GitBox <gi...@apache.org>.
eric-briscoe commented on code in PR #20809:
URL: https://github.com/apache/superset/pull/20809#discussion_r926913431


##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -213,6 +219,28 @@ export default class ResultSet extends React.PureComponent<
     }
   }
 
+  async createExploreResultsOnClick() {
+    const { results } = this.props.query;
+    // There is currently redux / state issue where sometimes a query will have serverId

Review Comment:
   This comment is probably no longer relevant - did you encounter this issue at all in this workflow?



-- 
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] hughhhh commented on pull request #20809: feat: Create Chart onClick Functionality

Posted by GitBox <gi...@apache.org>.
hughhhh commented on PR #20809:
URL: https://github.com/apache/superset/pull/20809#issuecomment-1191767573

   Looking at the video it seems like you are registering the query as a dataset not a query 


-- 
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] codecov[bot] commented on pull request #20809: feat: Create Chart onClick Functionality

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #20809:
URL: https://github.com/apache/superset/pull/20809#issuecomment-1191808143

   # [Codecov](https://codecov.io/gh/apache/superset/pull/20809?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#20809](https://codecov.io/gh/apache/superset/pull/20809?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9039880) into [master](https://codecov.io/gh/apache/superset/commit/84b852c45751930fdf8850c519509bdb2e5a5e38?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (84b852c) will **decrease** coverage by `11.49%`.
   > The diff coverage is `75.00%`.
   
   > :exclamation: Current head 9039880 differs from pull request most recent head 30c99ee. Consider uploading reports for the commit 30c99ee to get more accurate results
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #20809       +/-   ##
   ===========================================
   - Coverage   66.28%   54.78%   -11.50%     
   ===========================================
     Files        1757     1757               
     Lines       66796    66796               
     Branches     7052     7052               
   ===========================================
   - Hits        44276    36596     -7680     
   - Misses      20721    28401     +7680     
     Partials     1799     1799               
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `53.24% <ø> (ø)` | |
   | mysql | `?` | |
   | postgres | `?` | |
   | presto | `53.10% <ø> (ø)` | |
   | python | `57.80% <ø> (-23.70%)` | :arrow_down: |
   | sqlite | `?` | |
   | unit | `50.24% <ø> (ø)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/20809?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...frontend/src/SqlLab/components/ResultSet/index.tsx](https://codecov.io/gh/apache/superset/pull/20809/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC9pbmRleC50c3g=) | `54.07% <75.00%> (ø)` | |
   | [superset/utils/dashboard\_import\_export.py](https://codecov.io/gh/apache/superset/pull/20809/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvZGFzaGJvYXJkX2ltcG9ydF9leHBvcnQucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset/key\_value/commands/update.py](https://codecov.io/gh/apache/superset/pull/20809/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `0.00% <0.00%> (-88.89%)` | :arrow_down: |
   | [superset/key\_value/commands/delete.py](https://codecov.io/gh/apache/superset/pull/20809/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2RlbGV0ZS5weQ==) | `0.00% <0.00%> (-85.30%)` | :arrow_down: |
   | [superset/key\_value/commands/delete\_expired.py](https://codecov.io/gh/apache/superset/pull/20809/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2RlbGV0ZV9leHBpcmVkLnB5) | `0.00% <0.00%> (-80.77%)` | :arrow_down: |
   | [superset/dashboards/commands/importers/v0.py](https://codecov.io/gh/apache/superset/pull/20809/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGFzaGJvYXJkcy9jb21tYW5kcy9pbXBvcnRlcnMvdjAucHk=) | `15.62% <0.00%> (-76.25%)` | :arrow_down: |
   | [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/20809/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `25.30% <0.00%> (-68.68%)` | :arrow_down: |
   | [superset/datasets/commands/create.py](https://codecov.io/gh/apache/superset/pull/20809/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvY3JlYXRlLnB5) | `29.41% <0.00%> (-68.63%)` | :arrow_down: |
   | [superset/datasets/commands/importers/v0.py](https://codecov.io/gh/apache/superset/pull/20809/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvaW1wb3J0ZXJzL3YwLnB5) | `24.03% <0.00%> (-67.45%)` | :arrow_down: |
   | [superset/reports/commands/execute.py](https://codecov.io/gh/apache/superset/pull/20809/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcmVwb3J0cy9jb21tYW5kcy9leGVjdXRlLnB5) | `24.45% <0.00%> (-67.16%)` | :arrow_down: |
   | ... and [279 more](https://codecov.io/gh/apache/superset/pull/20809/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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] daryazata commented on a diff in pull request #20809: feat: Create Chart onClick Functionality

Posted by GitBox <gi...@apache.org>.
daryazata commented on code in PR #20809:
URL: https://github.com/apache/superset/pull/20809#discussion_r931620499


##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -223,7 +248,7 @@ export default class ResultSet extends React.PureComponent<
       // Added compute logic to stop user from being able to Save & Explore
       const { showSaveDatasetModal } = this.state;
       const { query } = this.props;
-
+      console.log('this is query', query);

Review Comment:
   @AAfghahi maybe it would be a good idea to get warning on `console` in linter



-- 
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