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 2020/04/01 18:28:11 UTC
[GitHub] [incubator-superset] ktmud opened a new pull request #9439: feat:
change default time range from sql lab to no filter
ktmud opened a new pull request #9439: feat: change default time range from sql lab to no filter
URL: https://github.com/apache/incubator-superset/pull/9439
### CATEGORY
- [x] Enhancement (new features, refinement)
### SUMMARY
Currently when clicking on "Explore" button in SQL Lab results, it defaults the time range to "100 years ago : now", which is basically equivalent to no filter for most datasources. However, it has implication on how the x-axis will be plotted in certain charts (e.g., recently, the Big Number chart introduced a[ fixed range](https://github.com/apache/incubator-superset/pull/9341) option).
This PR changes the default to "No filter" for the time range filter can be more properly utilized.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
When clicking on Explore in SQL Lab:
#### Before
#### After
### TEST PLAN
1. Go to any query results in SQL Lab
2. Click on "Explore"
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [x] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
### REVIEWERS
@kristw
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io commented on issue #9439: feat:
change default time range from sql lab to no filter
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #9439: feat: change default time range from sql lab to no filter
URL: https://github.com/apache/incubator-superset/pull/9439#issuecomment-607424864
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9439?src=pr&el=h1) Report
> Merging [#9439](https://codecov.io/gh/apache/incubator-superset/pull/9439?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/893c95521b8147cce85eafade9f718867990ff75&el=desc) will **not change** coverage by `%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9439?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9439 +/- ##
=======================================
Coverage 59.03% 59.03%
=======================================
Files 379 379
Lines 12181 12181
Branches 3010 3010
=======================================
Hits 7191 7191
Misses 4809 4809
Partials 181 181
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9439?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...end/src/SqlLab/components/ExploreResultsButton.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9439/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0V4cGxvcmVSZXN1bHRzQnV0dG9uLmpzeA==) | `73.75% <ø> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9439?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9439?src=pr&el=footer). Last update [893c955...7a36f71](https://codecov.io/gh/apache/incubator-superset/pull/9439?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on issue #9439: feat: change
default time range from sql lab to no filter
Posted by GitBox <gi...@apache.org>.
ktmud commented on issue #9439: feat: change default time range from sql lab to no filter
URL: https://github.com/apache/incubator-superset/pull/9439#issuecomment-610064973
https://travis-ci.org/github/apache/incubator-superset/builds/670726287?utm_source=github_status&utm_medium=notification
CI has passed but GitHub still shows pending? Can we merge regardless?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] etr2460 commented on a change in pull request
#9439: feat: change default time range from sql lab to no filter
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #9439: feat: change default time range from sql lab to no filter
URL: https://github.com/apache/incubator-superset/pull/9439#discussion_r403294805
##########
File path: superset-frontend/src/SqlLab/components/ExploreResultsButton.jsx
##########
@@ -139,8 +139,8 @@ class ExploreResultsButton extends React.PureComponent {
datasource: `${data.table_id}__table`,
metrics: [],
groupby: [],
+ time_range: 'No filter',
Review comment:
a bit weird that this is just plain text as opposed to `null` or something. I don't think fixing that is required right now, but i think moving "No filter" to a constant and using that constant it here would be reasonable. It looks like "No filter" is used in a few other places too (tests, other components) so maybe you could replace those your new constant too?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kristw closed pull request #9439: feat: change
default time range from sql lab to no filter
Posted by GitBox <gi...@apache.org>.
kristw closed pull request #9439: feat: change default time range from sql lab to no filter
URL: https://github.com/apache/incubator-superset/pull/9439
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud closed pull request #9439: feat: change
default time range from sql lab to no filter
Posted by GitBox <gi...@apache.org>.
ktmud closed pull request #9439: feat: change default time range from sql lab to no filter
URL: https://github.com/apache/incubator-superset/pull/9439
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud opened a new pull request #9439: feat:
change default time range from sql lab to no filter
Posted by GitBox <gi...@apache.org>.
ktmud opened a new pull request #9439: feat: change default time range from sql lab to no filter
URL: https://github.com/apache/incubator-superset/pull/9439
### CATEGORY
- [x] Enhancement (new features, refinement)
### SUMMARY
Currently when clicking on "Explore" button in SQL Lab results, it defaults the time range to "100 years ago : infinity", which is basically equivalent to no filter for most datasources. However, it has implication on how the x-axis will be plotted in certain charts (e.g., recently, the Big Number chart introduced a[ fixed range](https://github.com/apache/incubator-superset/pull/9341) option).
This PR changes the default to "No filter" so the time range filter can be more properly utilized.
There is also a global default value "[Last week](https://github.com/apache/incubator-superset/blob/893c95521b8147cce85eafade9f718867990ff75/superset-frontend/src/explore/controls.jsx#L846)", which is applied when users create a new chart from `/chart/add`. We are not changing this default value in this PR.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
When clicking on Explore in SQL Lab:
#### Before
#### After
### TEST PLAN
1. Go to any query results in SQL Lab
2. Click on "Explore"
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [x] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
### REVIEWERS
@kristw
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on a change in pull request
#9439: feat: change default time range from sql lab to no filter
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #9439: feat: change default time range from sql lab to no filter
URL: https://github.com/apache/incubator-superset/pull/9439#discussion_r403324626
##########
File path: superset-frontend/src/SqlLab/components/ExploreResultsButton.jsx
##########
@@ -139,8 +139,8 @@ class ExploreResultsButton extends React.PureComponent {
datasource: `${data.table_id}__table`,
metrics: [],
groupby: [],
+ time_range: 'No filter',
Review comment:
Agree this is not a good practice. There seems to be many other hard-coded time periods, as well. I am not comfortable with refactoring while changing the default or refactoring just the `'No filter'`. Maybe that refactoring can happen in a later PR?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud opened a new pull request #9439: feat:
change default time range from sql lab to no filter
Posted by GitBox <gi...@apache.org>.
ktmud opened a new pull request #9439: feat: change default time range from sql lab to no filter
URL: https://github.com/apache/incubator-superset/pull/9439
### CATEGORY
- [x] Enhancement (new features, refinement)
### SUMMARY
Currently when clicking on "Explore" button in SQL Lab results, it defaults the time range to "100 years ago : infinity", which is basically equivalent to no filter for most datasources. However, it has implication on how the x-axis will be plotted in certain charts (e.g., recently, the Big Number chart introduced a[ fixed range](https://github.com/apache/incubator-superset/pull/9341) option).
This PR changes the default to "No filter" so the time range filter can be more properly utilized.
There is also a global default value "[Last week](https://github.com/apache/incubator-superset/blob/893c95521b8147cce85eafade9f718867990ff75/superset-frontend/src/explore/controls.jsx#L846)", which is applied when users create a new chart from `/chart/add`. We are not changing this default value in this PR.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
When clicking on Explore in SQL Lab:
#### Before
#### After
### TEST PLAN
1. Go to any query results in SQL Lab
2. Click on "Explore"
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [x] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
### REVIEWERS
@kristw
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud closed pull request #9439: feat: change
default time range from sql lab to no filter
Posted by GitBox <gi...@apache.org>.
ktmud closed pull request #9439: feat: change default time range from sql lab to no filter
URL: https://github.com/apache/incubator-superset/pull/9439
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org