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 2021/01/22 14:49:56 UTC
[GitHub] [superset] agatapst opened a new pull request #12687: fix(native-filter): Show incompatible native filters indicator
agatapst opened a new pull request #12687:
URL: https://github.com/apache/superset/pull/12687
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
- The aim of this PR was to add **Incompatible** section to filter indicator. ⚠Incompatible filters are created using datasets & fields which are incompatible with a given chart.
- I have also added simple mocks for native filters: _mockStore_ and _mockNativeFilters_ to test displaying filter indicators for native filters.
- I have also slightly changed structure of Filter Badge tests in `FiltersBadge_spec.tsx` - I have divided them into _'for dashboard filters'_ and _'for native filters'_. Section _'for dashboard filters'_ is the same as on master, I have only added analogical tests for native filters.
Fixes #12568
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before incompatible section for native filters was missing.
Now different configurations:
**With some correct native filters applied and one incompatible:**
<img width="1544" alt="Zrzut ekranu 2021-01-22 o 15 02 38" src="https://user-images.githubusercontent.com/47450693/105500327-0bb93f80-5cc3-11eb-9cee-83638ced5a1e.png">
**With trying to apply two incompatible native filters from two different datasets:**
<img width="1545" alt="Zrzut ekranu 2021-01-22 o 15 09 53" src="https://user-images.githubusercontent.com/47450693/105500993-f0026900-5cc3-11eb-9e89-d1e756907e19.png">
**With one dashboard filter applied and one incompatible native filter:**
<img width="1526" alt="Zrzut ekranu 2021-01-22 o 15 12 35" src="https://user-images.githubusercontent.com/47450693/105501375-6a32ed80-5cc4-11eb-96bc-60b447473d50.png">
**With incompatible dashboard filters (without native filters):**
<img width="1773" alt="Zrzut ekranu 2021-01-22 o 15 23 49" src="https://user-images.githubusercontent.com/47450693/105502681-fbef2a80-5cc5-11eb-9cae-a33cee9486c5.png">
### TEST PLAN
Enable native filters: go to `config.py` and set `"DASHBOARD_NATIVE_FILTERS": True,`
Go to dashboards and create native filters. One one them should be created with incompatible dataset, which cannot be applied to this dashboard.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Has associated issue: #12568
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
cc @junlincc @rusackas @villebro @suddjian
@adam-stasiak could you test?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io commented on pull request #12687: fix(native-filters): Show incompatible native filters indicator
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #12687:
URL: https://github.com/apache/superset/pull/12687#issuecomment-765462346
# [Codecov](https://codecov.io/gh/apache/superset/pull/12687?src=pr&el=h1) Report
> Merging [#12687](https://codecov.io/gh/apache/superset/pull/12687?src=pr&el=desc) (bfdd97b) into [master](https://codecov.io/gh/apache/superset/commit/734fb75219c96abd2a8323a0650f2e93caa054e8?el=desc) (734fb75) will **decrease** coverage by `3.53%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/12687?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12687 +/- ##
==========================================
- Coverage 66.85% 63.32% -3.54%
==========================================
Files 1018 486 -532
Lines 49776 29970 -19806
Branches 4869 0 -4869
==========================================
- Hits 33278 18978 -14300
+ Misses 16375 10992 -5383
+ Partials 123 0 -123
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `63.32% <ø> (-0.69%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12687?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12687/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12687/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `32.65% <0.00%> (-59.19%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12687/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12687/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `54.61% <0.00%> (-29.24%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/12687/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `59.64% <0.00%> (-22.81%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12687/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/schemas.py](https://codecov.io/gh/apache/superset/pull/12687/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `93.40% <0.00%> (-6.05%)` | :arrow_down: |
| [superset/databases/dao.py](https://codecov.io/gh/apache/superset/pull/12687/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2Rhby5weQ==) | `94.11% <0.00%> (-5.89%)` | :arrow_down: |
| [superset/databases/api.py](https://codecov.io/gh/apache/superset/pull/12687/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `86.55% <0.00%> (-5.47%)` | :arrow_down: |
| [superset/views/database/validators.py](https://codecov.io/gh/apache/superset/pull/12687/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvdmFsaWRhdG9ycy5weQ==) | `78.94% <0.00%> (-5.27%)` | :arrow_down: |
| ... and [552 more](https://codecov.io/gh/apache/superset/pull/12687/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12687?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12687?src=pr&el=footer). Last update [734fb75...bfdd97b](https://codecov.io/gh/apache/superset/pull/12687?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] adam-stasiak commented on pull request #12687: fix(native-filters): Show incompatible native filters indicator
Posted by GitBox <gi...@apache.org>.
adam-stasiak commented on pull request #12687:
URL: https://github.com/apache/superset/pull/12687#issuecomment-765514493
Scenario:
Set 3 native filters (1 valid for this dashboard)
Enable all filters
Apply change
Check chart filters
Disable one of incorrect native filters and apply change
Observed: I see disabled filter in section Incompatible Filters
Expected: I would see this filter under Unset section.
https://user-images.githubusercontent.com/25153919/105514912-568f8300-5cd4-11eb-9224-480f209e61ba.mov
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] junlincc commented on pull request #12687: fix(native-filters): Show incompatible native filters indicator
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #12687:
URL: https://github.com/apache/superset/pull/12687#issuecomment-765723843
LGTM, thank you for fixing it! ♥️ @agatapst we are very close to demo-able stage.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] adam-stasiak commented on pull request #12687: fix(native-filters): Show incompatible native filters indicator
Posted by GitBox <gi...@apache.org>.
adam-stasiak commented on pull request #12687:
URL: https://github.com/apache/superset/pull/12687#issuecomment-765568241
Retested again and consulted with Agata. It looks like my issue was caused by performance on my machine (on video you can see how long everything takes).
I double checked that after computer restart and saw that it was for a moment in yellow zone without value but after a second it was moved to unset 🆗 .
So no issue there.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] junlincc edited a comment on pull request #12687: fix(native-filters): Show incompatible native filters indicator
Posted by GitBox <gi...@apache.org>.
junlincc edited a comment on pull request #12687:
URL: https://github.com/apache/superset/pull/12687#issuecomment-765723843
LGTM, thank you for fixing it! ♥️ @agatapst we are very close to demo-able stage.
<img width="1826" alt="Screen Shot 2021-01-22 at 1 51 41 PM" src="https://user-images.githubusercontent.com/67837651/105555759-ee827200-5cbe-11eb-849e-a38fbf44bcb0.png">
next step is to get the magnifier work
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] suddjian commented on pull request #12687: fix(native-filters): Show incompatible native filters indicator
Posted by GitBox <gi...@apache.org>.
suddjian commented on pull request #12687:
URL: https://github.com/apache/superset/pull/12687#issuecomment-765571360
Oh apologies Adam, I didn't see your comment when I left my review. Glad it's working!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] zhaoyongjie merged pull request #12687: fix(native-filters): Show incompatible native filters indicator
Posted by GitBox <gi...@apache.org>.
zhaoyongjie merged pull request #12687:
URL: https://github.com/apache/superset/pull/12687
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] adam-stasiak edited a comment on pull request #12687: fix(native-filters): Show incompatible native filters indicator
Posted by GitBox <gi...@apache.org>.
adam-stasiak edited a comment on pull request #12687:
URL: https://github.com/apache/superset/pull/12687#issuecomment-765514493
@agatapst could you check this scenario? @junlincc Could you confirm that my expected behavior is valid?
Scenario:
Set 3 native filters (1 valid for this dashboard)
Enable all filters
Apply change
Check chart filters
Disable one of incorrect native filters and apply change
Observed: I see disabled filter in section Incompatible Filters
Expected: I would see this filter under Unset section.
https://user-images.githubusercontent.com/25153919/105514912-568f8300-5cd4-11eb-9224-480f209e61ba.mov
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org