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/03/28 06:48:43 UTC
[GitHub] [incubator-superset] graceguo-supercat opened a new pull request
#9408: [fix] allow force refresh for No Results chart
graceguo-supercat opened a new pull request #9408: [fix] allow force refresh for No Results chart
URL: https://github.com/apache/incubator-superset/pull/9408
### CATEGORY
Choose one
- [x] Bug Fix
### SUMMARY
When there is no results return for a chart, we show No Results message. but this chart's `Force refresh` button is disabled by a bug:
<img width="610" alt="Screen Shot 2020-03-25 at 3 12 25 PM" src="https://user-images.githubusercontent.com/27990562/77817069-46dbe980-7085-11ea-9a79-a4ddd3f375c6.png">
### TEST PLAN
manual test
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
### REVIEWERS
@etr2460 @ktmud
----------------------------------------------------------------
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 #9408: [fix] allow
force refresh for No Results chart
Posted by GitBox <gi...@apache.org>.
ktmud commented on issue #9408: [fix] allow force refresh for No Results chart
URL: https://github.com/apache/incubator-superset/pull/9408#issuecomment-606188510
Wasn't that the intention? I asked because [the original commit](https://github.com/apache/incubator-superset/commit/6ec9e1a4e94beb1d6a1f49e25569473ba99715c1) that introduced this disabled state says:
* [Dashboard] disable force refresh when chart is still loading
----------------------------------------------------------------
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] graceguo-supercat merged pull request #9408:
[fix] allow force refresh for No Results chart
Posted by GitBox <gi...@apache.org>.
graceguo-supercat merged pull request #9408: [fix] allow force refresh for No Results chart
URL: https://github.com/apache/incubator-superset/pull/9408
----------------------------------------------------------------
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 #9408: [fix]
allow force refresh for No Results chart
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #9408: [fix] allow force refresh for No Results chart
URL: https://github.com/apache/incubator-superset/pull/9408#issuecomment-605406032
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9408?src=pr&el=h1) Report
> Merging [#9408](https://codecov.io/gh/apache/incubator-superset/pull/9408?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/cfa786850f337353578e45470c473cea3ca35c89&el=desc) will **not change** coverage by `%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9408?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9408 +/- ##
=======================================
Coverage 59.01% 59.01%
=======================================
Files 376 378 +2
Lines 12162 12162
Branches 3004 3004
=======================================
Hits 7177 7177
Misses 4805 4805
Partials 180 180
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9408?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/chart/chartReducer.js](https://codecov.io/gh/apache/incubator-superset/pull/9408/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L2NoYXJ0UmVkdWNlci5qcw==) | `19.04% <ø> (ø)` | |
| [superset-frontend/src/chart/Chart.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9408/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L0NoYXJ0LmpzeA==) | `10.41% <0.00%> (ø)` | |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9408/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (ø)` | |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9408/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (ø)` | |
| [...t-frontend/src/explore/controlPanels/Partition.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9408/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29udHJvbFBhbmVscy9QYXJ0aXRpb24uanN4) | `0.00% <0.00%> (ø)` | |
| [...erset-frontend/src/explore/controlPanels/Mapbox.js](https://codecov.io/gh/apache/incubator-superset/pull/9408/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29udHJvbFBhbmVscy9NYXBib3guanM=) | `0.00% <0.00%> (ø)` | |
| [superset-frontend/src/explore/controls.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9408/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29udHJvbHMuanN4) | `39.69% <0.00%> (+0.88%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9408?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/9408?src=pr&el=footer). Last update [cfa7868...8f2a959](https://codecov.io/gh/apache/incubator-superset/pull/9408?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] graceguo-supercat commented on issue #9408:
[fix] allow force refresh for No Results chart
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on issue #9408: [fix] allow force refresh for No Results chart
URL: https://github.com/apache/incubator-superset/pull/9408#issuecomment-606200063
yeah, i think it's reasonable to use status === "loading".
Added fix.
----------------------------------------------------------------
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] graceguo-supercat commented on issue #9408:
[fix] allow force refresh for No Results chart
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on issue #9408: [fix] allow force refresh for No Results chart
URL: https://github.com/apache/incubator-superset/pull/9408#issuecomment-606186997
> This works, but why not change
>
> https://github.com/apache/incubator-superset/blob/8bc30da6211df67ee8bec21e160ce587817ba85b/superset-frontend/src/dashboard/components/SliceHeaderControls.jsx#L136
>
> to `disabled={this.props.chartStatus !== 'loading'}`?
Chart's initial state is "loading":
https://github.com/apache/incubator-superset/blob/291306392443a5a0d0e2ee0cc4a95d37c56d4589/superset-frontend/src/chart/chartReducer.js#L24
----------------------------------------------------------------
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] graceguo-supercat edited a comment on issue
#9408: [fix] allow force refresh for No Results chart
Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on issue #9408: [fix] allow force refresh for No Results chart
URL: https://github.com/apache/incubator-superset/pull/9408#issuecomment-606200063
yeah, i think it's reasonable to check if current chart status === "loading".
Added fix.
----------------------------------------------------------------
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