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/03/02 16:26:20 UTC
[GitHub] [superset] kgabryje opened a new pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
kgabryje opened a new pull request #18998:
URL: https://github.com/apache/superset/pull/18998
### SUMMARY
When going from the dashboard to Explore view with extra filters (native filters or filter box), the filters are only appended to the first query. This PR iterates through form data and adds extra filters to every field that matches pattern `adhoc_filters.*`.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
![Uploading image.png…]()
After:
<img width="1792" alt="image" src="https://user-images.githubusercontent.com/15073128/156403586-b0fac9cf-1dbe-4b8c-91e6-c108801f0ef8.png">
### TESTING INSTRUCTIONS
1. Create a mixed timeseries chart and add it to a dashboard
2. Go to that dashboard and apply some native filters
3. Click View chart in explore on mixed timeseries chart
4. Verify that native filter is applied in both Query A and Query B adhoc filter control
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Has associated issue: #15858
- [ ] Required feature flags:
- [ ] Changes UI
- [x] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [x] Migration is atomic, supports rollback & is backwards-compatible
- [x] Confirm DB migration upgrade and downgrade tested
- [x] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
DB migration stats:
```
Current: 0.42 s
10+: 0.44 s
100+: 0.26 s
1000+: 0.25 s
```
This PR replaces #18740
https://app.shortcut.com/preset/story/37242/explore-mixed-time-series-chart-native-filter-only-apply-to-query-a-but-not-query-b-when-open-mixed-time-series-chart-from
--
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 #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #18998:
URL: https://github.com/apache/superset/pull/18998#issuecomment-1058445700
⚠️ @kgabryje Your base branch `master` has just also updated `superset/migrations`.
❗ **Please consider rebasing your branch to avoid db migration conflicts.**
--
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] ktmud commented on a change in pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #18998:
URL: https://github.com/apache/superset/pull/18998#discussion_r819002083
##########
File path: superset/utils/core.py
##########
@@ -1097,11 +1097,13 @@ def merge_extra_form_data(form_data: Dict[str, Any]) -> None:
{"isExtra": True, **fltr} for fltr in append_adhoc_filters # type: ignore
)
if append_filters:
- adhoc_filters.extend(
- simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
- for fltr in append_filters
- if fltr
- )
+ for key, value in form_data.items():
+ if re.match("adhoc_filter.*", key):
+ value.extend(
+ simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
+ for fltr in append_filters
+ if fltr
+ )
Review comment:
Another argument for using more explicit field names is minimizing unintentional side-effects. While the probability is small, there is no mechanism preventing someone from naming a non adhoc-filter field with `adhoc_filter` in the name. The opposite is also true---one may name an adhoc filters field without it starting with `adhoc_filter`. We've seen it for metrics before, which is why we have the [METRIC_KEYS](https://github.com/apache/superset/blob/760dab9abdb2199b50006e01cb1e9cc0743cf966/superset/viz.py#L104-L113) constant.
--
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] ktmud commented on a change in pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #18998:
URL: https://github.com/apache/superset/pull/18998#discussion_r818931014
##########
File path: superset/utils/core.py
##########
@@ -1097,11 +1097,13 @@ def merge_extra_form_data(form_data: Dict[str, Any]) -> None:
{"isExtra": True, **fltr} for fltr in append_adhoc_filters # type: ignore
)
if append_filters:
- adhoc_filters.extend(
- simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
- for fltr in append_filters
- if fltr
- )
+ for key, value in form_data.items():
+ if re.match("adhoc_filter.*", key):
+ value.extend(
+ simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
+ for fltr in append_filters
+ if fltr
+ )
Review comment:
```suggestion
for key in ["adhoc_filters", "adhoc_filters_b"]:
form_data[key] = form_data.get(key, []) + [
simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
for fltr in append_filters if fltr
]
```
I think we can be more explicit about the field names to avoid looping through all fields and a simple get can avoid db migration? Also, the extended value should be assigned back to `form_data` as L1091 may also see `adhoc_filters` not exists in `form_data`.
--
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] edited a comment on pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18998:
URL: https://github.com/apache/superset/pull/18998#issuecomment-1057161941
# [Codecov](https://codecov.io/gh/apache/superset/pull/18998?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 [#18998](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (013dfe1) into [master](https://codecov.io/gh/apache/superset/commit/299b5dc64448d04abe6b35ee85fbd2b938c781bc?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (299b5dc) will **increase** coverage by `0.02%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 013dfe1 differs from pull request most recent head 26d9363. Consider uploading reports for the commit 26d9363 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #18998 +/- ##
==========================================
+ Coverage 66.56% 66.58% +0.02%
==========================================
Files 1641 1641
Lines 63495 63550 +55
Branches 6425 6424 -1
==========================================
+ Hits 42265 42317 +52
- Misses 19550 19554 +4
+ Partials 1680 1679 -1
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `52.60% <0.00%> (+0.01%)` | :arrow_up: |
| javascript | `51.38% <ø> (+<0.01%)` | :arrow_up: |
| mysql | `81.83% <100.00%> (+0.02%)` | :arrow_up: |
| postgres | `81.88% <100.00%> (+0.01%)` | :arrow_up: |
| presto | `52.45% <0.00%> (+0.01%)` | :arrow_up: |
| python | `82.31% <100.00%> (+0.01%)` | :arrow_up: |
| sqlite | `81.57% <100.00%> (+0.02%)` | :arrow_up: |
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/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...chart-controls/src/shared-controls/dndControls.tsx](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9kbmRDb250cm9scy50c3g=) | `35.89% <ø> (ø)` | |
| [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | `90.29% <100.00%> (+0.07%)` | :arrow_up: |
| [...set-frontend/src/dashboard/util/getDropPosition.js](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldERyb3BQb3NpdGlvbi5qcw==) | `87.27% <0.00%> (-1.41%)` | :arrow_down: |
| [superset-frontend/src/components/Select/utils.ts](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L3V0aWxzLnRz) | `57.89% <0.00%> (-1.20%)` | :arrow_down: |
| [superset-frontend/src/explore/controls.jsx](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29udHJvbHMuanN4) | `31.03% <0.00%> (-0.55%)` | :arrow_down: |
| [...et-ui-chart-controls/src/shared-controls/index.tsx](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9pbmRleC50c3g=) | `36.00% <0.00%> (-0.37%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `77.39% <0.00%> (-0.09%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `90.04% <0.00%> (-0.07%)` | :arrow_down: |
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvdml6LnB5) | `58.26% <0.00%> (ø)` | |
| [superset/constants.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvY29uc3RhbnRzLnB5) | `100.00% <0.00%> (ø)` | |
| ... and [20 more](https://codecov.io/gh/apache/superset/pull/18998/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) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [299b5dc...26d9363](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] codecov[bot] edited a comment on pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18998:
URL: https://github.com/apache/superset/pull/18998#issuecomment-1057161941
# [Codecov](https://codecov.io/gh/apache/superset/pull/18998?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 [#18998](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7397eeb) into [master](https://codecov.io/gh/apache/superset/commit/bd63a1bd98c1faf152205b3b862119a1c59b2f05?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bd63a1b) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #18998 +/- ##
=======================================
Coverage 66.58% 66.58%
=======================================
Files 1641 1641
Lines 63533 63537 +4
Branches 6424 6424
=======================================
+ Hits 42303 42307 +4
Misses 19551 19551
Partials 1679 1679
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `52.59% <20.00%> (-0.01%)` | :arrow_down: |
| mysql | `81.83% <100.00%> (+<0.01%)` | :arrow_up: |
| postgres | `81.88% <100.00%> (+<0.01%)` | :arrow_up: |
| presto | `52.43% <20.00%> (-0.01%)` | :arrow_down: |
| python | `82.31% <100.00%> (+<0.01%)` | :arrow_up: |
| sqlite | `81.57% <100.00%> (+<0.01%)` | :arrow_up: |
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/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...chart-controls/src/shared-controls/dndControls.tsx](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9kbmRDb250cm9scy50c3g=) | `35.89% <ø> (ø)` | |
| [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | `90.32% <100.00%> (+0.04%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [bd63a1b...7397eeb](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] villebro commented on a change in pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #18998:
URL: https://github.com/apache/superset/pull/18998#discussion_r819759406
##########
File path: superset/utils/core.py
##########
@@ -1097,11 +1097,13 @@ def merge_extra_form_data(form_data: Dict[str, Any]) -> None:
{"isExtra": True, **fltr} for fltr in append_adhoc_filters # type: ignore
)
if append_filters:
- adhoc_filters.extend(
- simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
- for fltr in append_filters
- if fltr
- )
+ for key, value in form_data.items():
+ if re.match("adhoc_filter.*", key):
+ value.extend(
+ simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
+ for fltr in append_filters
+ if fltr
+ )
Review comment:
I agree that we should make all these things as explicit as possible, at a minimum documenting magic behavior like this. While on this subject, the whole notion of `QueryFormData` automatically mapping properties like `metric` and `secondary_metric` to `metrics` in `QueryObject` is really opaque, and should probably both 1) be more thoroughly documented 2) be phased out in coming major versions. But in the short term I propose adding documentation around this functionality to at least explain how this stuff works right now.
--
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] edited a comment on pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18998:
URL: https://github.com/apache/superset/pull/18998#issuecomment-1057161941
# [Codecov](https://codecov.io/gh/apache/superset/pull/18998?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 [#18998](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (013dfe1) into [master](https://codecov.io/gh/apache/superset/commit/be88cb9ba00a706dc5402e0b5311975bb013972a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (be88cb9) will **decrease** coverage by `0.04%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #18998 +/- ##
==========================================
- Coverage 66.58% 66.53% -0.05%
==========================================
Files 1641 1641
Lines 63548 63550 +2
Branches 6424 6424
==========================================
- Hits 42312 42284 -28
- Misses 19555 19585 +30
Partials 1681 1681
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `52.60% <0.00%> (-0.01%)` | :arrow_down: |
| mysql | `?` | |
| postgres | `81.86% <100.00%> (-0.02%)` | :arrow_down: |
| presto | `52.45% <0.00%> (-0.01%)` | :arrow_down: |
| python | `82.21% <100.00%> (-0.10%)` | :arrow_down: |
| sqlite | `?` | |
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/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...chart-controls/src/shared-controls/dndControls.tsx](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9kbmRDb250cm9scy50c3g=) | `35.89% <ø> (ø)` | |
| [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | `90.29% <100.00%> (+0.02%)` | :arrow_up: |
| [superset/common/utils/dataframe\_utils.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvY29tbW9uL3V0aWxzL2RhdGFmcmFtZV91dGlscy5weQ==) | `85.71% <0.00%> (-7.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `91.89% <0.00%> (-5.41%)` | :arrow_down: |
| [superset/connectors/sqla/utils.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL3V0aWxzLnB5) | `88.23% <0.00%> (-3.93%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `93.97% <0.00%> (-3.62%)` | :arrow_down: |
| [superset/reports/commands/log\_prune.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvcmVwb3J0cy9jb21tYW5kcy9sb2dfcHJ1bmUucHk=) | `85.71% <0.00%> (-3.58%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `86.20% <0.00%> (-3.45%)` | :arrow_down: |
| [superset/commands/importers/v1/utils.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvY29tbWFuZHMvaW1wb3J0ZXJzL3YxL3V0aWxzLnB5) | `89.13% <0.00%> (-2.18%)` | :arrow_down: |
| [superset/result\_set.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvcmVzdWx0X3NldC5weQ==) | `96.77% <0.00%> (-1.62%)` | :arrow_down: |
| ... and [8 more](https://codecov.io/gh/apache/superset/pull/18998/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) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [be88cb9...013dfe1](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] codecov[bot] edited a comment on pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18998:
URL: https://github.com/apache/superset/pull/18998#issuecomment-1057161941
# [Codecov](https://codecov.io/gh/apache/superset/pull/18998?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 [#18998](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (26d9363) into [master](https://codecov.io/gh/apache/superset/commit/299b5dc64448d04abe6b35ee85fbd2b938c781bc?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (299b5dc) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #18998 +/- ##
=======================================
Coverage 66.56% 66.56%
=======================================
Files 1641 1641
Lines 63495 63497 +2
Branches 6425 6425
=======================================
+ Hits 42265 42267 +2
Misses 19550 19550
Partials 1680 1680
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `52.59% <0.00%> (-0.01%)` | :arrow_down: |
| mysql | `81.81% <100.00%> (+<0.01%)` | :arrow_up: |
| postgres | `81.86% <100.00%> (+<0.01%)` | :arrow_up: |
| presto | `52.43% <0.00%> (-0.01%)` | :arrow_down: |
| python | `82.29% <100.00%> (+<0.01%)` | :arrow_up: |
| sqlite | `81.55% <100.00%> (+<0.01%)` | :arrow_up: |
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/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...chart-controls/src/shared-controls/dndControls.tsx](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9kbmRDb250cm9scy50c3g=) | `35.89% <ø> (ø)` | |
| [...et-ui-chart-controls/src/shared-controls/index.tsx](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9pbmRleC50c3g=) | `36.36% <ø> (ø)` | |
| [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | `90.25% <100.00%> (+0.02%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [299b5dc...26d9363](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] codecov[bot] edited a comment on pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18998:
URL: https://github.com/apache/superset/pull/18998#issuecomment-1057161941
# [Codecov](https://codecov.io/gh/apache/superset/pull/18998?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 [#18998](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (013dfe1) into [master](https://codecov.io/gh/apache/superset/commit/be88cb9ba00a706dc5402e0b5311975bb013972a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (be88cb9) will **decrease** coverage by `0.02%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #18998 +/- ##
==========================================
- Coverage 66.58% 66.55% -0.03%
==========================================
Files 1641 1641
Lines 63548 63550 +2
Branches 6424 6424
==========================================
- Hits 42312 42297 -15
- Misses 19555 19572 +17
Partials 1681 1681
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `52.60% <0.00%> (-0.01%)` | :arrow_down: |
| mysql | `?` | |
| postgres | `81.86% <100.00%> (-0.02%)` | :arrow_down: |
| presto | `52.45% <0.00%> (-0.01%)` | :arrow_down: |
| python | `82.25% <100.00%> (-0.06%)` | :arrow_down: |
| sqlite | `81.57% <100.00%> (+<0.01%)` | :arrow_up: |
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/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...chart-controls/src/shared-controls/dndControls.tsx](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9kbmRDb250cm9scy50c3g=) | `35.89% <ø> (ø)` | |
| [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | `90.29% <100.00%> (+0.02%)` | :arrow_up: |
| [superset/common/utils/dataframe\_utils.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvY29tbW9uL3V0aWxzL2RhdGFmcmFtZV91dGlscy5weQ==) | `85.71% <0.00%> (-7.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `93.97% <0.00%> (-3.62%)` | :arrow_down: |
| [superset/reports/commands/log\_prune.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvcmVwb3J0cy9jb21tYW5kcy9sb2dfcHJ1bmUucHk=) | `85.71% <0.00%> (-3.58%)` | :arrow_down: |
| [superset/commands/importers/v1/utils.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvY29tbWFuZHMvaW1wb3J0ZXJzL3YxL3V0aWxzLnB5) | `89.13% <0.00%> (-2.18%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.19% <0.00%> (-0.73%)` | :arrow_down: |
| [superset/common/query\_object.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X29iamVjdC5weQ==) | `95.13% <0.00%> (-0.55%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `76.95% <0.00%> (-0.45%)` | :arrow_down: |
| [superset/reports/commands/execute.py](https://codecov.io/gh/apache/superset/pull/18998/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) | `91.14% <0.00%> (-0.37%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [be88cb9...013dfe1](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] ktmud commented on a change in pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #18998:
URL: https://github.com/apache/superset/pull/18998#discussion_r818931014
##########
File path: superset/utils/core.py
##########
@@ -1097,11 +1097,13 @@ def merge_extra_form_data(form_data: Dict[str, Any]) -> None:
{"isExtra": True, **fltr} for fltr in append_adhoc_filters # type: ignore
)
if append_filters:
- adhoc_filters.extend(
- simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
- for fltr in append_filters
- if fltr
- )
+ for key, value in form_data.items():
+ if re.match("adhoc_filter.*", key):
+ value.extend(
+ simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
+ for fltr in append_filters
+ if fltr
+ )
Review comment:
```suggestion
for key in ["adhoc_filters", "adhoc_filters_b"]:
form_data[key] = form_data.get(key, []) + [
simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
for fltr in append_filters if fltr
]
```
I think we can be more explicit about the field names to avoid looping through all form_data and a simple get can avoid db migration? Also, the extended value should be assigned back to `form_data` as L1091 may also see `adhoc_filters` not exists in `form_data`.
--
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] ktmud commented on a change in pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #18998:
URL: https://github.com/apache/superset/pull/18998#discussion_r818931014
##########
File path: superset/utils/core.py
##########
@@ -1097,11 +1097,13 @@ def merge_extra_form_data(form_data: Dict[str, Any]) -> None:
{"isExtra": True, **fltr} for fltr in append_adhoc_filters # type: ignore
)
if append_filters:
- adhoc_filters.extend(
- simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
- for fltr in append_filters
- if fltr
- )
+ for key, value in form_data.items():
+ if re.match("adhoc_filter.*", key):
+ value.extend(
+ simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
+ for fltr in append_filters
+ if fltr
+ )
Review comment:
```suggestion
for key in ["adhoc_filters", "adhoc_filters_b"]:
if key in form_data:
form_data[key] = form_data.get(key) or [] + [
simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
for fltr in append_filters if fltr
]
```
I think we can be more explicit about the field names to avoid looping through all fields and a simple get can avoid db migration?
--
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] zhaoyongjie commented on a change in pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #18998:
URL: https://github.com/apache/superset/pull/18998#discussion_r818398443
##########
File path: superset/utils/core.py
##########
@@ -1056,6 +1056,16 @@ def form_data_to_adhoc(form_data: Dict[str, Any], clause: str) -> AdhocFilterCla
return result
+def append_filters_to_adhoc_filters(
Review comment:
No place for reuse. We don't need to create functions.
--
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] edited a comment on pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18998:
URL: https://github.com/apache/superset/pull/18998#issuecomment-1057161941
--
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] ktmud commented on a change in pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #18998:
URL: https://github.com/apache/superset/pull/18998#discussion_r819739692
##########
File path: superset/utils/core.py
##########
@@ -1097,11 +1097,13 @@ def merge_extra_form_data(form_data: Dict[str, Any]) -> None:
{"isExtra": True, **fltr} for fltr in append_adhoc_filters # type: ignore
)
if append_filters:
- adhoc_filters.extend(
- simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
- for fltr in append_filters
- if fltr
- )
+ for key, value in form_data.items():
+ if re.match("adhoc_filter.*", key):
+ value.extend(
+ simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
+ for fltr in append_filters
+ if fltr
+ )
Review comment:
I still believe in general regex should be used sparingly and explicit is better than implicit. Since you can't stop someone from naming a new filter field things like "series_b_filters", the bug may still come back even with a regex.
In any case, we should need a consistent way of mapping field names to field types because most of the time what you want is "do something special based on filter type", not filed names. One obvious solution is to create an `ADHOC_FILTER_KEYS` constant and put it together with `METRIC_KEYS` in some `constants.py` file, but I agree this by itself probably doesn't make much difference on the grander scheme of things.
--
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] edited a comment on pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18998:
URL: https://github.com/apache/superset/pull/18998#issuecomment-1057161941
# [Codecov](https://codecov.io/gh/apache/superset/pull/18998?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 [#18998](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7397eeb) into [master](https://codecov.io/gh/apache/superset/commit/bd63a1bd98c1faf152205b3b862119a1c59b2f05?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bd63a1b) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 7397eeb differs from pull request most recent head 14a2553. Consider uploading reports for the commit 14a2553 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #18998 +/- ##
=======================================
Coverage 66.58% 66.58%
=======================================
Files 1641 1641
Lines 63533 63537 +4
Branches 6424 6424
=======================================
+ Hits 42303 42307 +4
Misses 19551 19551
Partials 1679 1679
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `52.59% <20.00%> (-0.01%)` | :arrow_down: |
| mysql | `81.83% <100.00%> (+<0.01%)` | :arrow_up: |
| postgres | `81.88% <100.00%> (+<0.01%)` | :arrow_up: |
| presto | `52.43% <20.00%> (-0.01%)` | :arrow_down: |
| python | `82.31% <100.00%> (+<0.01%)` | :arrow_up: |
| sqlite | `81.57% <100.00%> (+<0.01%)` | :arrow_up: |
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/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...chart-controls/src/shared-controls/dndControls.tsx](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9kbmRDb250cm9scy50c3g=) | `35.89% <ø> (ø)` | |
| [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | `90.32% <100.00%> (+0.04%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [bd63a1b...14a2553](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] github-actions[bot] commented on pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #18998:
URL: https://github.com/apache/superset/pull/18998#issuecomment-1057239455
⚠️ @kgabryje Your base branch `master` has just also updated `superset/migrations`.
❗ **Please consider rebasing your branch to avoid db migration conflicts.**
--
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] kgabryje commented on a change in pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
kgabryje commented on a change in pull request #18998:
URL: https://github.com/apache/superset/pull/18998#discussion_r818461871
##########
File path: superset/utils/core.py
##########
@@ -1097,11 +1107,9 @@ def merge_extra_form_data(form_data: Dict[str, Any]) -> None:
{"isExtra": True, **fltr} for fltr in append_adhoc_filters # type: ignore
)
if append_filters:
- adhoc_filters.extend(
- simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
- for fltr in append_filters
- if fltr
- )
+ for key, value in form_data.items():
+ if re.match("adhoc_filter.*", key):
+ append_filters_to_adhoc_filters(value, append_filters)
Review comment:
The point of iterating through form_data keys and checking regex is to make it more generic and future proof - it should work with any control that's name is like `adhoc_filter.*`, so not only `adhoc_filter` and `adhoc_filter_b`
##########
File path: superset/utils/core.py
##########
@@ -1056,6 +1056,16 @@ def form_data_to_adhoc(form_data: Dict[str, Any], clause: str) -> AdhocFilterCla
return result
+def append_filters_to_adhoc_filters(
Review comment:
good point!
--
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] kgabryje commented on a change in pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
kgabryje commented on a change in pull request #18998:
URL: https://github.com/apache/superset/pull/18998#discussion_r819439672
##########
File path: superset/utils/core.py
##########
@@ -1097,11 +1097,13 @@ def merge_extra_form_data(form_data: Dict[str, Any]) -> None:
{"isExtra": True, **fltr} for fltr in append_adhoc_filters # type: ignore
)
if append_filters:
- adhoc_filters.extend(
- simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
- for fltr in append_filters
- if fltr
- )
+ for key, value in form_data.items():
+ if re.match("adhoc_filter.*", key):
+ value.extend(
+ simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
+ for fltr in append_filters
+ if fltr
+ )
Review comment:
Thanks for discussion. Do we agree that there's no action/change required here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] zhaoyongjie commented on a change in pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #18998:
URL: https://github.com/apache/superset/pull/18998#discussion_r818397409
##########
File path: superset/utils/core.py
##########
@@ -1097,11 +1107,9 @@ def merge_extra_form_data(form_data: Dict[str, Any]) -> None:
{"isExtra": True, **fltr} for fltr in append_adhoc_filters # type: ignore
)
if append_filters:
- adhoc_filters.extend(
- simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
- for fltr in append_filters
- if fltr
- )
+ for key, value in form_data.items():
+ if re.match("adhoc_filter.*", key):
+ append_filters_to_adhoc_filters(value, append_filters)
Review comment:
To make this logic seem more straightforward(Which iteme are we going to update), I suggest this:
```suggestion
append_filters.extend(form_data.get('adhoc_filter', []))
append_filters.extend(form_data.get('adhoc_filters', []))
append_filters.extend(form_data.get('adhoc_filters_b', []))
append_filters = [simple_filter_to_adhoc({"isExtra": True, **f}) for f in append_filters if f]
```
--
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] edited a comment on pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18998:
URL: https://github.com/apache/superset/pull/18998#issuecomment-1057161941
# [Codecov](https://codecov.io/gh/apache/superset/pull/18998?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 [#18998](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (12ddb78) into [master](https://codecov.io/gh/apache/superset/commit/bd63a1bd98c1faf152205b3b862119a1c59b2f05?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bd63a1b) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 12ddb78 differs from pull request most recent head 14a2553. Consider uploading reports for the commit 14a2553 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #18998 +/- ##
=======================================
Coverage 66.58% 66.58%
=======================================
Files 1641 1641
Lines 63533 63537 +4
Branches 6424 6424
=======================================
+ Hits 42303 42307 +4
Misses 19551 19551
Partials 1679 1679
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `51.38% <ø> (ø)` | |
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/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...chart-controls/src/shared-controls/dndControls.tsx](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9kbmRDb250cm9scy50c3g=) | `35.89% <ø> (ø)` | |
| [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | `90.32% <100.00%> (+0.04%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [bd63a1b...14a2553](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] codecov[bot] edited a comment on pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18998:
URL: https://github.com/apache/superset/pull/18998#issuecomment-1057161941
# [Codecov](https://codecov.io/gh/apache/superset/pull/18998?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 [#18998](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (013dfe1) into [master](https://codecov.io/gh/apache/superset/commit/be88cb9ba00a706dc5402e0b5311975bb013972a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (be88cb9) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #18998 +/- ##
=======================================
Coverage 66.58% 66.58%
=======================================
Files 1641 1641
Lines 63548 63550 +2
Branches 6424 6424
=======================================
+ Hits 42312 42314 +2
Misses 19555 19555
Partials 1681 1681
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `52.60% <0.00%> (-0.01%)` | :arrow_down: |
| mysql | `81.83% <100.00%> (+<0.01%)` | :arrow_up: |
| postgres | `81.88% <100.00%> (+<0.01%)` | :arrow_up: |
| presto | `52.45% <0.00%> (-0.01%)` | :arrow_down: |
| python | `82.31% <100.00%> (+<0.01%)` | :arrow_up: |
| sqlite | `81.57% <100.00%> (+<0.01%)` | :arrow_up: |
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/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...chart-controls/src/shared-controls/dndControls.tsx](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9kbmRDb250cm9scy50c3g=) | `35.89% <ø> (ø)` | |
| [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | `90.29% <100.00%> (+0.02%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [be88cb9...013dfe1](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] codecov[bot] edited a comment on pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18998:
URL: https://github.com/apache/superset/pull/18998#issuecomment-1057161941
# [Codecov](https://codecov.io/gh/apache/superset/pull/18998?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 [#18998](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (013dfe1) into [master](https://codecov.io/gh/apache/superset/commit/be88cb9ba00a706dc5402e0b5311975bb013972a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (be88cb9) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #18998 +/- ##
=======================================
Coverage 66.58% 66.58%
=======================================
Files 1641 1641
Lines 63548 63550 +2
Branches 6424 6424
=======================================
+ Hits 42312 42317 +5
+ Misses 19555 19554 -1
+ Partials 1681 1679 -2
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `52.60% <0.00%> (-0.01%)` | :arrow_down: |
| javascript | `51.38% <ø> (+<0.01%)` | :arrow_up: |
| mysql | `81.83% <100.00%> (+<0.01%)` | :arrow_up: |
| postgres | `81.88% <100.00%> (+<0.01%)` | :arrow_up: |
| presto | `52.45% <0.00%> (-0.01%)` | :arrow_down: |
| python | `82.31% <100.00%> (+<0.01%)` | :arrow_up: |
| sqlite | `81.57% <100.00%> (+<0.01%)` | :arrow_up: |
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/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...chart-controls/src/shared-controls/dndControls.tsx](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9kbmRDb250cm9scy50c3g=) | `35.89% <ø> (ø)` | |
| [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | `90.29% <100.00%> (+0.02%)` | :arrow_up: |
| [...eFilters/FiltersConfigModal/FiltersConfigModal.tsx](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvRmlsdGVyc0NvbmZpZ01vZGFsL0ZpbHRlcnNDb25maWdNb2RhbC50c3g=) | `79.39% <0.00%> (+1.81%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [be88cb9...013dfe1](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] villebro commented on a change in pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #18998:
URL: https://github.com/apache/superset/pull/18998#discussion_r819458290
##########
File path: superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx
##########
@@ -80,7 +80,7 @@ export const dndEntity: typeof dndGroupByControl = {
export const dnd_adhoc_filters: SharedControlConfig<'DndFilterSelect'> = {
type: 'DndFilterSelect',
label: t('Filters'),
- default: null,
+ default: [],
Review comment:
Do we also need to update the non-DnD control? https://github.com/apache/superset/blob/299b5dc64448d04abe6b35ee85fbd2b938c781bc/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/index.tsx#L461
--
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] kgabryje commented on a change in pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
kgabryje commented on a change in pull request #18998:
URL: https://github.com/apache/superset/pull/18998#discussion_r819533042
##########
File path: superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx
##########
@@ -80,7 +80,7 @@ export const dndEntity: typeof dndGroupByControl = {
export const dnd_adhoc_filters: SharedControlConfig<'DndFilterSelect'> = {
type: 'DndFilterSelect',
label: t('Filters'),
- default: null,
+ default: [],
Review comment:
Done, tested and works fine (old charts and new charts)
--
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] kgabryje merged pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
kgabryje merged pull request #18998:
URL: https://github.com/apache/superset/pull/18998
--
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] villebro commented on a change in pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #18998:
URL: https://github.com/apache/superset/pull/18998#discussion_r818993921
##########
File path: superset/utils/core.py
##########
@@ -1097,11 +1097,13 @@ def merge_extra_form_data(form_data: Dict[str, Any]) -> None:
{"isExtra": True, **fltr} for fltr in append_adhoc_filters # type: ignore
)
if append_filters:
- adhoc_filters.extend(
- simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
- for fltr in append_filters
- if fltr
- )
+ for key, value in form_data.items():
+ if re.match("adhoc_filter.*", key):
+ value.extend(
+ simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
+ for fltr in append_filters
+ if fltr
+ )
Review comment:
@ktmud in offline discussions I was the person who proposed using a regex to loop through this, so don't blame @kgabryje for this 😄 My reasoning went something like this:
1) it's fairly uncommon for there to be more than 20 controls. So performance impact of looping through all control keys is negligible
2) While I know that this is put here mostly for the Mixed Timeseries chart, I don't see any reason why someone wouldn't consider having 3 (or more) query sections. Only supporting one additional control called `adhoc_filters_b`, but not, say, `adhoc_filters_c` or `adhoc_filters_2`, feels slightly arbitrary. The logic behind my reasoning here was, "any control that starts with `adhoc_filters` should be considered an adhoc filter control, and extra filters should apply to them all". While both solutions solve this specific problem, I still feel the regex solution is less coupled to the Mixed Timeseries chart than only matching `["adhoc_filters", "adhoc_filters_b"]`
Having said that, this was really just a minor nit and likely irrelevant in the grand scheme of things, so I won't get hung up on this either way 🙂
--
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] edited a comment on pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18998:
URL: https://github.com/apache/superset/pull/18998#issuecomment-1057161941
# [Codecov](https://codecov.io/gh/apache/superset/pull/18998?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 [#18998](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (013dfe1) into [master](https://codecov.io/gh/apache/superset/commit/be88cb9ba00a706dc5402e0b5311975bb013972a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (be88cb9) will **decrease** coverage by `0.01%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #18998 +/- ##
==========================================
- Coverage 66.58% 66.56% -0.02%
==========================================
Files 1641 1641
Lines 63548 63550 +2
Branches 6424 6424
==========================================
- Hits 42312 42301 -11
- Misses 19555 19568 +13
Partials 1681 1681
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `52.60% <0.00%> (-0.01%)` | :arrow_down: |
| mysql | `?` | |
| postgres | `81.88% <100.00%> (+<0.01%)` | :arrow_up: |
| presto | `52.45% <0.00%> (-0.01%)` | :arrow_down: |
| python | `82.27% <100.00%> (-0.05%)` | :arrow_down: |
| sqlite | `81.57% <100.00%> (+<0.01%)` | :arrow_up: |
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/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...chart-controls/src/shared-controls/dndControls.tsx](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9kbmRDb250cm9scy50c3g=) | `35.89% <ø> (ø)` | |
| [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | `90.29% <100.00%> (+0.02%)` | :arrow_up: |
| [superset/common/utils/dataframe\_utils.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvY29tbW9uL3V0aWxzL2RhdGFmcmFtZV91dGlscy5weQ==) | `85.71% <0.00%> (-7.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `93.97% <0.00%> (-3.62%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.19% <0.00%> (-0.73%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `76.95% <0.00%> (-0.45%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [be88cb9...013dfe1](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] ktmud commented on a change in pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #18998:
URL: https://github.com/apache/superset/pull/18998#discussion_r819071109
##########
File path: superset/utils/core.py
##########
@@ -1097,11 +1097,13 @@ def merge_extra_form_data(form_data: Dict[str, Any]) -> None:
{"isExtra": True, **fltr} for fltr in append_adhoc_filters # type: ignore
)
if append_filters:
- adhoc_filters.extend(
- simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
- for fltr in append_filters
- if fltr
- )
+ for key, value in form_data.items():
+ if re.match("adhoc_filter.*", key):
+ value.extend(
+ simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
+ for fltr in append_filters
+ if fltr
+ )
Review comment:
True.
--
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] zhaoyongjie commented on a change in pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #18998:
URL: https://github.com/apache/superset/pull/18998#discussion_r818525138
##########
File path: superset/utils/core.py
##########
@@ -1097,11 +1107,9 @@ def merge_extra_form_data(form_data: Dict[str, Any]) -> None:
{"isExtra": True, **fltr} for fltr in append_adhoc_filters # type: ignore
)
if append_filters:
- adhoc_filters.extend(
- simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
- for fltr in append_filters
- if fltr
- )
+ for key, value in form_data.items():
+ if re.match("adhoc_filter.*", key):
+ append_filters_to_adhoc_filters(value, append_filters)
Review comment:
Okay! Thanks for the explanation.
--
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] edited a comment on pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18998:
URL: https://github.com/apache/superset/pull/18998#issuecomment-1057161941
# [Codecov](https://codecov.io/gh/apache/superset/pull/18998?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 [#18998](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (26d9363) into [master](https://codecov.io/gh/apache/superset/commit/299b5dc64448d04abe6b35ee85fbd2b938c781bc?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (299b5dc) will **decrease** coverage by `0.18%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #18998 +/- ##
==========================================
- Coverage 66.56% 66.37% -0.19%
==========================================
Files 1641 1641
Lines 63495 63497 +2
Branches 6425 6425
==========================================
- Hits 42265 42149 -116
- Misses 19550 19668 +118
Partials 1680 1680
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `52.59% <0.00%> (-0.01%)` | :arrow_down: |
| mysql | `?` | |
| postgres | `?` | |
| presto | `52.43% <0.00%> (-0.01%)` | :arrow_down: |
| python | `81.91% <100.00%> (-0.38%)` | :arrow_down: |
| sqlite | `81.55% <100.00%> (+<0.01%)` | :arrow_up: |
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/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...chart-controls/src/shared-controls/dndControls.tsx](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9kbmRDb250cm9scy50c3g=) | `35.89% <ø> (ø)` | |
| [...et-ui-chart-controls/src/shared-controls/index.tsx](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9pbmRleC50c3g=) | `36.36% <ø> (ø)` | |
| [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | `90.25% <100.00%> (+0.02%)` | :arrow_up: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `31.37% <0.00%> (-56.87%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `60.34% <0.00%> (-20.69%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/common/utils/dataframe\_utils.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvY29tbW9uL3V0aWxzL2RhdGFmcmFtZV91dGlscy5weQ==) | `85.71% <0.00%> (-7.15%)` | :arrow_down: |
| [superset/databases/api.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `87.98% <0.00%> (-6.01%)` | :arrow_down: |
| [superset/databases/dao.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvZGF0YWJhc2VzL2Rhby5weQ==) | `94.44% <0.00%> (-5.56%)` | :arrow_down: |
| ... and [14 more](https://codecov.io/gh/apache/superset/pull/18998/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) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [299b5dc...26d9363](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] codecov[bot] edited a comment on pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18998:
URL: https://github.com/apache/superset/pull/18998#issuecomment-1057161941
# [Codecov](https://codecov.io/gh/apache/superset/pull/18998?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 [#18998](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (26d9363) into [master](https://codecov.io/gh/apache/superset/commit/299b5dc64448d04abe6b35ee85fbd2b938c781bc?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (299b5dc) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #18998 +/- ##
=======================================
Coverage 66.56% 66.56%
=======================================
Files 1641 1641
Lines 63495 63497 +2
Branches 6425 6425
=======================================
+ Hits 42265 42267 +2
Misses 19550 19550
Partials 1680 1680
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `52.59% <0.00%> (-0.01%)` | :arrow_down: |
| javascript | `51.37% <ø> (ø)` | |
| mysql | `81.81% <100.00%> (+<0.01%)` | :arrow_up: |
| postgres | `81.86% <100.00%> (+<0.01%)` | :arrow_up: |
| presto | `52.43% <0.00%> (-0.01%)` | :arrow_down: |
| python | `82.29% <100.00%> (+<0.01%)` | :arrow_up: |
| sqlite | `81.55% <100.00%> (+<0.01%)` | :arrow_up: |
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/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...chart-controls/src/shared-controls/dndControls.tsx](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9kbmRDb250cm9scy50c3g=) | `35.89% <ø> (ø)` | |
| [...et-ui-chart-controls/src/shared-controls/index.tsx](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9pbmRleC50c3g=) | `36.36% <ø> (ø)` | |
| [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | `90.25% <100.00%> (+0.02%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [299b5dc...26d9363](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] codecov[bot] commented on pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #18998:
URL: https://github.com/apache/superset/pull/18998#issuecomment-1057161941
# [Codecov](https://codecov.io/gh/apache/superset/pull/18998?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 [#18998](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7397eeb) into [master](https://codecov.io/gh/apache/superset/commit/bd63a1bd98c1faf152205b3b862119a1c59b2f05?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bd63a1b) will **decrease** coverage by `0.12%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #18998 +/- ##
==========================================
- Coverage 66.58% 66.45% -0.13%
==========================================
Files 1641 1641
Lines 63533 63537 +4
Branches 6424 6424
==========================================
- Hits 42303 42225 -78
- Misses 19551 19633 +82
Partials 1679 1679
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `81.88% <100.00%> (+<0.01%)` | :arrow_up: |
| presto | `52.43% <20.00%> (-0.01%)` | :arrow_down: |
| python | `82.05% <100.00%> (-0.27%)` | :arrow_down: |
| sqlite | `81.57% <100.00%> (+<0.01%)` | :arrow_up: |
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/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...chart-controls/src/shared-controls/dndControls.tsx](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9kbmRDb250cm9scy50c3g=) | `35.89% <ø> (ø)` | |
| [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | `90.20% <100.00%> (-0.08%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.19%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `70.00% <0.00%> (-15.77%)` | :arrow_down: |
| [superset/common/utils/dataframe\_utils.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvY29tbW9uL3V0aWxzL2RhdGFmcmFtZV91dGlscy5weQ==) | `85.71% <0.00%> (-7.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `93.97% <0.00%> (-3.62%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.19% <0.00%> (-0.73%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `76.95% <0.00%> (-0.45%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `89.14% <0.00%> (-0.34%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.85% <0.00%> (-0.19%)` | :arrow_down: |
| ... and [1 more](https://codecov.io/gh/apache/superset/pull/18998/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) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [bd63a1b...7397eeb](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] codecov[bot] edited a comment on pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18998:
URL: https://github.com/apache/superset/pull/18998#issuecomment-1057161941
# [Codecov](https://codecov.io/gh/apache/superset/pull/18998?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 [#18998](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7397eeb) into [master](https://codecov.io/gh/apache/superset/commit/bd63a1bd98c1faf152205b3b862119a1c59b2f05?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bd63a1b) will **decrease** coverage by `0.01%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #18998 +/- ##
==========================================
- Coverage 66.58% 66.56% -0.02%
==========================================
Files 1641 1641
Lines 63533 63537 +4
Branches 6424 6424
==========================================
- Hits 42303 42294 -9
- Misses 19551 19564 +13
Partials 1679 1679
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `52.59% <20.00%> (-0.01%)` | :arrow_down: |
| mysql | `?` | |
| postgres | `81.88% <100.00%> (+<0.01%)` | :arrow_up: |
| presto | `52.43% <20.00%> (-0.01%)` | :arrow_down: |
| python | `82.27% <100.00%> (-0.04%)` | :arrow_down: |
| sqlite | `81.57% <100.00%> (+<0.01%)` | :arrow_up: |
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/18998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...chart-controls/src/shared-controls/dndControls.tsx](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9kbmRDb250cm9scy50c3g=) | `35.89% <ø> (ø)` | |
| [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | `90.32% <100.00%> (+0.04%)` | :arrow_up: |
| [superset/common/utils/dataframe\_utils.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvY29tbW9uL3V0aWxzL2RhdGFmcmFtZV91dGlscy5weQ==) | `85.71% <0.00%> (-7.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `93.97% <0.00%> (-3.62%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.19% <0.00%> (-0.73%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/18998/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-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `76.95% <0.00%> (-0.45%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [bd63a1b...7397eeb](https://codecov.io/gh/apache/superset/pull/18998?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] ktmud commented on a change in pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #18998:
URL: https://github.com/apache/superset/pull/18998#discussion_r818931014
##########
File path: superset/utils/core.py
##########
@@ -1097,11 +1097,13 @@ def merge_extra_form_data(form_data: Dict[str, Any]) -> None:
{"isExtra": True, **fltr} for fltr in append_adhoc_filters # type: ignore
)
if append_filters:
- adhoc_filters.extend(
- simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
- for fltr in append_filters
- if fltr
- )
+ for key, value in form_data.items():
+ if re.match("adhoc_filter.*", key):
+ value.extend(
+ simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
+ for fltr in append_filters
+ if fltr
+ )
Review comment:
```suggestion
for key in ["adhoc_filters", "adhoc_filters_b"]:
if key in form_data:
form_data[key] = (form_data.get(key) or []) + [
simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
for fltr in append_filters if fltr
]
```
I think we can be more explicit about the field names to avoid looping through all fields?
--
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] raags commented on pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
raags commented on pull request #18998:
URL: https://github.com/apache/superset/pull/18998#issuecomment-1065182782
How to get this backported to 1.4? Or check in which version this fix would be included in?
--
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 #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #18998:
URL: https://github.com/apache/superset/pull/18998#issuecomment-1058720463
⚠️ @kgabryje Your base branch `master` has just also updated `superset/migrations`.
❗ **Please consider rebasing your branch to avoid db migration conflicts.**
--
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] zhaoyongjie commented on a change in pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #18998:
URL: https://github.com/apache/superset/pull/18998#discussion_r818397409
##########
File path: superset/utils/core.py
##########
@@ -1097,11 +1107,9 @@ def merge_extra_form_data(form_data: Dict[str, Any]) -> None:
{"isExtra": True, **fltr} for fltr in append_adhoc_filters # type: ignore
)
if append_filters:
- adhoc_filters.extend(
- simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
- for fltr in append_filters
- if fltr
- )
+ for key, value in form_data.items():
+ if re.match("adhoc_filter.*", key):
+ append_filters_to_adhoc_filters(value, append_filters)
Review comment:
To make this logic seem more straightforward(Which iteme are we going to update), I suggest this:
```suggestion
append_filters.extend(form_data.get('adhoc_filters_b', []))
append_filters = [simple_filter_to_adhoc({"isExtra": True, **f}) for f in append_filters if f]
```
--
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] zhaoyongjie commented on a change in pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #18998:
URL: https://github.com/apache/superset/pull/18998#discussion_r818397409
##########
File path: superset/utils/core.py
##########
@@ -1097,11 +1107,9 @@ def merge_extra_form_data(form_data: Dict[str, Any]) -> None:
{"isExtra": True, **fltr} for fltr in append_adhoc_filters # type: ignore
)
if append_filters:
- adhoc_filters.extend(
- simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
- for fltr in append_filters
- if fltr
- )
+ for key, value in form_data.items():
+ if re.match("adhoc_filter.*", key):
+ append_filters_to_adhoc_filters(value, append_filters)
Review comment:
To make this logic seem more straightforward(Which iteme are we going to update), I suggest this:
```suggestion
append_filters.extend(form_data.get('adhoc_filter', []))
append_filters.extend(form_data.get('adhoc_filters', []))
append_filters = [simple_filter_to_adhoc({"isExtra": True, **f}) for f in append_filters if f]
```
--
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] villebro commented on a change in pull request #18998: fix(plugin-chart-echarts): Apply temporary filters to Query B in explore
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #18998:
URL: https://github.com/apache/superset/pull/18998#discussion_r819005970
##########
File path: superset/utils/core.py
##########
@@ -1097,11 +1097,13 @@ def merge_extra_form_data(form_data: Dict[str, Any]) -> None:
{"isExtra": True, **fltr} for fltr in append_adhoc_filters # type: ignore
)
if append_filters:
- adhoc_filters.extend(
- simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
- for fltr in append_filters
- if fltr
- )
+ for key, value in form_data.items():
+ if re.match("adhoc_filter.*", key):
+ value.extend(
+ simple_filter_to_adhoc({"isExtra": True, **fltr}) # type: ignore
+ for fltr in append_filters
+ if fltr
+ )
Review comment:
This is true. However, naming a control that doesn't contain adhoc filters with the prefix "adhoc_filters" feels less likely than than the opposite.
--
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