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/05/23 01:48:46 UTC
[GitHub] [incubator-superset] graceguo-supercat opened a new pull request #9891: fix: [dashboard] should not trigger chart refresh when filter not applicable
graceguo-supercat opened a new pull request #9891:
URL: https://github.com/apache/incubator-superset/pull/9891
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
### TEST PLAN
<!--- What steps should be taken to verify the changes -->
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #9891: fix: [dashboard] should not trigger chart refresh when filter not applicable
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #9891:
URL: https://github.com/apache/incubator-superset/pull/9891#issuecomment-632966454
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=h1) Report
> Merging [#9891](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/6d4e23663e95659a4f53cbff6d65770ebff562ce&el=desc) will **decrease** coverage by `5.11%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9891 +/- ##
==========================================
- Coverage 71.24% 66.13% -5.12%
==========================================
Files 584 584
Lines 30789 30609 -180
Branches 3238 3240 +2
==========================================
- Hits 21937 20244 -1693
- Misses 8742 10178 +1436
- Partials 110 187 +77
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `59.45% <100.00%> (+0.01%)` | :arrow_up: |
| #python | `70.88% <ø> (+6.58%)` | :arrow_up: |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...et-frontend/src/dashboard/components/Dashboard.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZC5qc3g=) | `77.50% <100.00%> (-5.62%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/chart/ChartContainer.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L0NoYXJ0Q29udGFpbmVyLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupFormatters.js](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRm9ybWF0dGVycy5qcw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [183 more](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9891?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/9891?src=pr&el=footer). Last update [6d4e236...3bee8c1](https://codecov.io/gh/apache/incubator-superset/pull/9891?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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #9891: fix: [dashboard] should not trigger chart refresh when filter not applicable
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #9891:
URL: https://github.com/apache/incubator-superset/pull/9891#issuecomment-632966454
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=h1) Report
> Merging [#9891](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/6d4e23663e95659a4f53cbff6d65770ebff562ce&el=desc) will **decrease** coverage by `4.82%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9891 +/- ##
==========================================
- Coverage 71.24% 66.42% -4.83%
==========================================
Files 584 584
Lines 30789 30792 +3
Branches 3238 3240 +2
==========================================
- Hits 21937 20453 -1484
- Misses 8742 10152 +1410
- Partials 110 187 +77
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `59.45% <100.00%> (+0.01%)` | :arrow_up: |
| #python | `71.32% <ø> (+7.01%)` | :arrow_up: |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...et-frontend/src/dashboard/components/Dashboard.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZC5qc3g=) | `77.50% <100.00%> (-5.62%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/chart/ChartContainer.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L0NoYXJ0Q29udGFpbmVyLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupFormatters.js](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRm9ybWF0dGVycy5qcw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [142 more](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9891?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/9891?src=pr&el=footer). Last update [6d4e236...3bee8c1](https://codecov.io/gh/apache/incubator-superset/pull/9891?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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #9891: fix: [dashboard] should not trigger chart refresh when filter not applicable
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #9891:
URL: https://github.com/apache/incubator-superset/pull/9891#issuecomment-632966454
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=h1) Report
> Merging [#9891](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/6d4e23663e95659a4f53cbff6d65770ebff562ce&el=desc) will **decrease** coverage by `0.19%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9891 +/- ##
==========================================
- Coverage 71.24% 71.05% -0.20%
==========================================
Files 584 584
Lines 30789 30792 +3
Branches 3238 3240 +2
==========================================
- Hits 21937 21879 -58
- Misses 8742 8796 +54
- Partials 110 117 +7
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `53.04% <0.00%> (-11.53%)` | :arrow_down: |
| #javascript | `59.45% <100.00%> (+0.01%)` | :arrow_up: |
| #python | `71.39% <ø> (+7.08%)` | :arrow_up: |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...et-frontend/src/dashboard/components/Dashboard.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZC5qc3g=) | `83.75% <100.00%> (+0.63%)` | :arrow_up: |
| [...et-frontend/src/SqlLab/reducers/getInitialState.js](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9yZWR1Y2Vycy9nZXRJbml0aWFsU3RhdGUuanM=) | `33.33% <0.00%> (-16.67%)` | :arrow_down: |
| [superset-frontend/src/reduxUtils.js](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3JlZHV4VXRpbHMuanM=) | `76.47% <0.00%> (-10.30%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `61.13% <0.00%> (-5.68%)` | :arrow_down: |
| [...rontend/src/SqlLab/components/TabbedSqlEditors.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1RhYmJlZFNxbEVkaXRvcnMuanN4) | `76.22% <0.00%> (-5.60%)` | :arrow_down: |
| [...rontend/src/SqlLab/components/SqlEditorLeftBar.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NxbEVkaXRvckxlZnRCYXIuanN4) | `44.00% <0.00%> (-4.00%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/reducers/sqlLab.js](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9yZWR1Y2Vycy9zcWxMYWIuanM=) | `37.44% <0.00%> (-3.30%)` | :arrow_down: |
| [...end/src/SqlLab/components/TemplateParamsEditor.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1RlbXBsYXRlUGFyYW1zRWRpdG9yLmpzeA==) | `88.57% <0.00%> (-2.86%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/SqlEditor.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NxbEVkaXRvci5qc3g=) | `53.84% <0.00%> (-1.29%)` | :arrow_down: |
| [...rontend/src/SqlLab/components/AceEditorWrapper.tsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0FjZUVkaXRvcldyYXBwZXIudHN4) | `55.91% <0.00%> (-1.08%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9891?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/9891?src=pr&el=footer). Last update [6d4e236...3bee8c1](https://codecov.io/gh/apache/incubator-superset/pull/9891?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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #9891: fix: [dashboard] should not trigger chart refresh when filter not applicable
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #9891:
URL: https://github.com/apache/incubator-superset/pull/9891#issuecomment-632966454
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=h1) Report
> Merging [#9891](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/6d4e23663e95659a4f53cbff6d65770ebff562ce&el=desc) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9891 +/- ##
=======================================
Coverage 71.24% 71.25%
=======================================
Files 584 584
Lines 30789 30792 +3
Branches 3238 3240 +2
=======================================
+ Hits 21937 21940 +3
Misses 8742 8742
Partials 110 110
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `53.82% <0.00%> (-10.75%)` | :arrow_down: |
| #javascript | `59.45% <100.00%> (+0.01%)` | :arrow_up: |
| #python | `71.39% <ø> (+7.08%)` | :arrow_up: |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...et-frontend/src/dashboard/components/Dashboard.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZC5qc3g=) | `83.75% <100.00%> (+0.63%)` | :arrow_up: |
| [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `66.81% <0.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9891?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/9891?src=pr&el=footer). Last update [6d4e236...3bee8c1](https://codecov.io/gh/apache/incubator-superset/pull/9891?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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] mistercrunch commented on a change in pull request #9891: fix: [dashboard] should not trigger chart refresh when filter not applicable
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #9891:
URL: https://github.com/apache/incubator-superset/pull/9891#discussion_r429652117
##########
File path: superset-frontend/src/dashboard/components/Dashboard.jsx
##########
@@ -159,11 +159,27 @@ class Dashboard extends React.PureComponent {
// added filter?
[].push.apply(affectedChartIds, activeFilters[filterKey].scope);
} else {
- // changed filter field value or scope?
- const affectedScope = (activeFilters[filterKey].scope || []).concat(
- appliedFilters[filterKey].scope || [],
- );
- [].push.apply(affectedChartIds, affectedScope);
+ // has filter field value change?
+ if (
+ !areObjectsEqual(
+ appliedFilters[filterKey].values,
+ activeFilters[filterKey].values,
+ )
+ ) {
+ [].push.apply(affectedChartIds, activeFilters[filterKey].scope);
Review comment:
NIT(optional ): I had to lookup what `[].push.apply`might do. For me `affectedChartIds.push(...activeFilters[filterKey].scope)` would be more readable (I think it's equivalent?).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] mistercrunch commented on a change in pull request #9891: fix: [dashboard] should not trigger chart refresh when filter not applicable
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #9891:
URL: https://github.com/apache/incubator-superset/pull/9891#discussion_r429651988
##########
File path: superset-frontend/src/dashboard/components/Dashboard.jsx
##########
@@ -159,11 +159,27 @@ class Dashboard extends React.PureComponent {
// added filter?
[].push.apply(affectedChartIds, activeFilters[filterKey].scope);
} else {
- // changed filter field value or scope?
- const affectedScope = (activeFilters[filterKey].scope || []).concat(
- appliedFilters[filterKey].scope || [],
- );
- [].push.apply(affectedChartIds, affectedScope);
+ // has filter field value change?
Review comment:
NIT(optional ): `componentDidUpdate` is getting pretty large and opaque, it could be could to refactor into a `applyFilters` or some better name.
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #9891: fix: [dashboard] should not trigger chart refresh when filter not applicable
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #9891:
URL: https://github.com/apache/incubator-superset/pull/9891#discussion_r430052816
##########
File path: superset-frontend/src/dashboard/components/Dashboard.jsx
##########
@@ -205,6 +181,56 @@ class Dashboard extends React.PureComponent {
return Object.values(this.props.charts);
}
+ applyFilters() {
+ const appliedFilters = this.appliedFilters;
+ const { activeFilters } = this.props;
+
+ // refresh charts if a filter was removed, added, or changed
+ const currFilterKeys = Object.keys(activeFilters);
+ const appliedFilterKeys = Object.keys(appliedFilters);
+
+ const allKeys = new Set(currFilterKeys.concat(appliedFilterKeys));
+ const affectedChartIds = [];
+ [...allKeys].forEach(filterKey => {
+ if (!currFilterKeys.includes(filterKey)) {
+ // removed filter?
+ affectedChartIds.push(...appliedFilters[filterKey].scope);
+ } else if (!appliedFilterKeys.includes(filterKey)) {
+ // added filter?
+ affectedChartIds.push(...activeFilters[filterKey].scope);
+ } else {
+ // if filter changes value
+ // update charts in its scope
+ if (
+ !areObjectsEqual(
+ appliedFilters[filterKey].values,
+ activeFilters[filterKey].values,
+ )
+ ) {
+ affectedChartIds.push(...activeFilters[filterKey].scope);
+ }
+
+ // if filter change scope
+ // update all charts in its scope
+ if (
+ !areObjectsEqual(
+ appliedFilters[filterKey].scope,
+ activeFilters[filterKey].scope,
+ )
+ ) {
+ const chartsInScope = (activeFilters[filterKey].scope || []).concat(
+ appliedFilters[filterKey].scope || [],
+ );
+ affectedChartIds.push(...chartsInScope);
+ }
+ }
+ });
+
+ const idSet = new Set(affectedChartIds);
+ this.refreshCharts([...idSet]);
Review comment:
Nit:
```
this.refreshCharts([...new Set(affectedChartIds)]);
```
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #9891: fix: [dashboard] should not trigger chart refresh when filter not applicable
Posted by GitBox <gi...@apache.org>.
graceguo-supercat merged pull request #9891:
URL: https://github.com/apache/incubator-superset/pull/9891
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #9891: fix: [dashboard] should not trigger chart refresh when filter not applicable
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #9891:
URL: https://github.com/apache/incubator-superset/pull/9891#issuecomment-632966454
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter commented on pull request #9891: fix: [dashboard] should not trigger chart refresh when filter not applicable
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #9891:
URL: https://github.com/apache/incubator-superset/pull/9891#issuecomment-632966454
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=h1) Report
> Merging [#9891](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/6d4e23663e95659a4f53cbff6d65770ebff562ce&el=desc) will **decrease** coverage by `0.36%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9891 +/- ##
==========================================
- Coverage 71.24% 70.88% -0.37%
==========================================
Files 584 184 -400
Lines 30789 17889 -12900
Branches 3238 0 -3238
==========================================
- Hits 21937 12681 -9256
+ Misses 8742 5208 -3534
+ Partials 110 0 -110
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `70.88% <ø> (+6.58%)` | :arrow_up: |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `58.92% <0.00%> (-21.43%)` | :arrow_down: |
| [superset/utils/cache.py](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2FjaGUucHk=) | `45.83% <0.00%> (-20.84%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `78.26% <0.00%> (-13.05%)` | :arrow_down: |
| [superset/views/database/validators.py](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvdmFsaWRhdG9ycy5weQ==) | `77.77% <0.00%> (-5.56%)` | :arrow_down: |
| [superset/views/tags.py](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvdGFncy5weQ==) | `35.13% <0.00%> (-4.11%)` | :arrow_down: |
| [superset/models/sql\_types/presto\_sql\_types.py](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL3NxbF90eXBlcy9wcmVzdG9fc3FsX3R5cGVzLnB5) | `72.41% <0.00%> (-4.06%)` | :arrow_down: |
| [superset/models/tags.py](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL3RhZ3MucHk=) | `58.58% <0.00%> (-3.80%)` | :arrow_down: |
| [superset/views/database/api.py](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvYXBpLnB5) | `83.90% <0.00%> (-3.45%)` | :arrow_down: |
| [superset/views/sql\_lab.py](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3Mvc3FsX2xhYi5weQ==) | `59.06% <0.00%> (-3.29%)` | :arrow_down: |
| [superset/cli.py](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2xpLnB5) | `36.72% <0.00%> (-3.14%)` | :arrow_down: |
| ... and [433 more](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9891?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/9891?src=pr&el=footer). Last update [6d4e236...3bee8c1](https://codecov.io/gh/apache/incubator-superset/pull/9891?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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #9891: fix: [dashboard] should not trigger chart refresh when filter not applicable
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #9891:
URL: https://github.com/apache/incubator-superset/pull/9891#issuecomment-632966454
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=h1) Report
> Merging [#9891](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/6d4e23663e95659a4f53cbff6d65770ebff562ce&el=desc) will **decrease** coverage by `4.78%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9891 +/- ##
==========================================
- Coverage 71.24% 66.46% -4.79%
==========================================
Files 584 584
Lines 30789 30792 +3
Branches 3238 3240 +2
==========================================
- Hits 21937 20465 -1472
- Misses 8742 10140 +1398
- Partials 110 187 +77
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `59.45% <100.00%> (+0.01%)` | :arrow_up: |
| #python | `71.39% <ø> (+7.08%)` | :arrow_up: |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9891?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...et-frontend/src/dashboard/components/Dashboard.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZC5qc3g=) | `77.50% <100.00%> (-5.62%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/chart/ChartContainer.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L0NoYXJ0Q29udGFpbmVyLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupFormatters.js](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRm9ybWF0dGVycy5qcw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [139 more](https://codecov.io/gh/apache/incubator-superset/pull/9891/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9891?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/9891?src=pr&el=footer). Last update [6d4e236...3bee8c1](https://codecov.io/gh/apache/incubator-superset/pull/9891?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
---------------------------------------------------------------------
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 a change in pull request #9891: fix: [dashboard] should not trigger chart refresh when filter not applicable
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #9891:
URL: https://github.com/apache/incubator-superset/pull/9891#discussion_r430048293
##########
File path: superset-frontend/src/dashboard/components/Dashboard.jsx
##########
@@ -159,11 +159,27 @@ class Dashboard extends React.PureComponent {
// added filter?
[].push.apply(affectedChartIds, activeFilters[filterKey].scope);
} else {
- // changed filter field value or scope?
- const affectedScope = (activeFilters[filterKey].scope || []).concat(
- appliedFilters[filterKey].scope || [],
- );
- [].push.apply(affectedChartIds, affectedScope);
+ // has filter field value change?
Review comment:
added `applyFilters` function.
##########
File path: superset-frontend/src/dashboard/components/Dashboard.jsx
##########
@@ -159,11 +159,27 @@ class Dashboard extends React.PureComponent {
// added filter?
[].push.apply(affectedChartIds, activeFilters[filterKey].scope);
} else {
- // changed filter field value or scope?
- const affectedScope = (activeFilters[filterKey].scope || []).concat(
- appliedFilters[filterKey].scope || [],
- );
- [].push.apply(affectedChartIds, affectedScope);
+ // has filter field value change?
+ if (
+ !areObjectsEqual(
+ appliedFilters[filterKey].values,
+ activeFilters[filterKey].values,
+ )
+ ) {
+ [].push.apply(affectedChartIds, activeFilters[filterKey].scope);
Review comment:
`affectedChartIds.push(...activeFilters[filterKey].scope)` just try to merge 2 arrays and assign back to `affectedChartIds`.
I agree es6 syntax is better. fixed.
##########
File path: superset-frontend/src/dashboard/components/Dashboard.jsx
##########
@@ -159,11 +159,27 @@ class Dashboard extends React.PureComponent {
// added filter?
[].push.apply(affectedChartIds, activeFilters[filterKey].scope);
} else {
- // changed filter field value or scope?
- const affectedScope = (activeFilters[filterKey].scope || []).concat(
- appliedFilters[filterKey].scope || [],
- );
- [].push.apply(affectedChartIds, affectedScope);
+ // has filter field value change?
+ if (
+ !areObjectsEqual(
+ appliedFilters[filterKey].values,
+ activeFilters[filterKey].values,
+ )
+ ) {
+ [].push.apply(affectedChartIds, activeFilters[filterKey].scope);
Review comment:
`[].push.apply(affectedChartIds, activeFilters[filterKey].scope)` just try to merge 2 arrays and assign back to `affectedChartIds`.
I agree es6 syntax is better. fixed.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org