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/04/23 00:03:25 UTC

[GitHub] [superset] graceguo-supercat opened a new issue #14307: [native filter] Enable empty filter broke charts in the dashboard

graceguo-supercat opened a new issue #14307:
URL: https://github.com/apache/superset/issues/14307


   
   
   #### Screenshots
   
   If applicable, add screenshots to help explain your problem.
   
   #### How to reproduce the bug
   
   1. Enable native filter feature flag, and create new dashboard
   2. add a new chart, for example, "Names count" chart from birth_names dataset
   3. add native filter in the dashboard. Check **Enable empty filter** and click Apply button.
   4. Chart shows No Results. If you open chart in explore view, there is strange `1 = 0` is added as adhoc filter.
   <img width="838" alt="Screen Shot 2021-04-22 at 4 55 34 PM" src="https://user-images.githubusercontent.com/27990562/115798718-bfe26900-a38b-11eb-9945-fc3da40cc226.png">
   
   
   ### Environment
   
   latest master
   
   ### Checklist
   
   Make sure to follow these steps before submitting your issue - thank you!
   
   - [x] I have checked the superset logs for python stacktraces and included it here as text if there are any.
   - [x] I have reproduced the issue with at least the latest released version of superset.
   - [x] I have checked the issue tracker for the same issue and I haven't found one similar.
   
   ### Additional context
   
   I am not sure what exactly "Enable empty filter" control mean. I assume it match with old filter_box option "Required"?
   
   <img width="446" alt="Screen Shot 2021-04-22 at 5 01 16 PM" src="https://user-images.githubusercontent.com/27990562/115799014-6af32280-a38c-11eb-9636-9ec9f77d1f44.png">
   
   But no matter this filter field is required or not, it should not add strange clause. 
   cc @junlincc @amitmiran137 


-- 
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] graceguo-supercat edited a comment on issue #14307: [native filter] Enable empty filter broke charts in the dashboard

Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on issue #14307:
URL: https://github.com/apache/superset/issues/14307#issuecomment-825844738


   **consolidate "Required" with the "Enable empty filter" option** will be the best option. 
   - Please let me know how to match old filter_config `clearable` to the new filter component attribute.
   - I still prefer an _actionable_ message in the filter field itself, when user must fill in some value for a filter. Most of our dashboard users are not dashboard builder (who knows some fields have extra requirements, etc), so when "No Results" shows up they don't know why and how to fix it....


-- 
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] graceguo-supercat commented on issue #14307: [native filter] Enable empty filter broke charts in the dashboard

Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on issue #14307:
URL: https://github.com/apache/superset/issues/14307#issuecomment-825802620


   Hi @villebro Thank you for the reply. So this option should match with old filter_box `Required` option.
   
   1. `enable empty filter` is very misleading message. It should just say `Required filed` as before.
   2. When user click APPLY button but did not fill a required filter, right now it just shows **No Results**, user has no idea about some filter is wrong, or actually no results.  Could we highlight / show error message on the required filter field when user clicks APPLY button? It will clearly tell user how to use this filter.  
   
   cc @junlincc this is a very misleading bug, please prioritize it.


-- 
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] graceguo-supercat edited a comment on issue #14307: [native filter] Enable empty filter broke charts in the dashboard

Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on issue #14307:
URL: https://github.com/apache/superset/issues/14307#issuecomment-825802620


   Hi @villebro Thank you for the reply. So this option should match with old filter_box `Required` option. "Enable empty filter" means _**is required**_ (not clearable). 
   
   1. `enable empty filter` is very misleading message. It should just say `Required` as before.
   2. When user click APPLY button but did not fill a required filter, right now it just shows **No Results**, user has no idea about some filter is wrong, or actually no results.  Could we **highlight / show error message** on the required filter field? It will clearly tell user how to use this filter.  
   
   cc @junlincc this is a very misleading bug, please prioritize it.


-- 
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] graceguo-supercat edited a comment on issue #14307: [native filter] Enable empty filter broke charts in the dashboard

Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on issue #14307:
URL: https://github.com/apache/superset/issues/14307#issuecomment-825844738


   **consolidate "Required" with the "Enable empty filter" option** will be the best option. 
   - Please let me know know how to match old filter_config `clearable` to the new filter component attribute.
   - I still prefer a actionable message in the filter field itself, when user must fill in some value for the filter. Most of our dashboard users are not dashboard builder (who knows some fields have extra requirements, etc), so when "No Results" shows up they don't know why and how to fix it....


-- 
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] villebro commented on issue #14307: [native filter] Enable empty filter broke charts in the dashboard

Posted by GitBox <gi...@apache.org>.
villebro commented on issue #14307:
URL: https://github.com/apache/superset/issues/14307#issuecomment-825549259


   This is on purpose: the "enable empty filter" is supposed to issue a WHERE clause that is always false if no selection is made. This feature has been requested by the community to minimize load on databases where issuing unfiltered queries causes performance issues.


-- 
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] graceguo-supercat edited a comment on issue #14307: [native filter] Enable empty filter broke charts in the dashboard

Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on issue #14307:
URL: https://github.com/apache/superset/issues/14307#issuecomment-825802620


   Hi @villebro Thank you for the reply. So this option should match with old filter_box `Required` option. "Enable empty filter" means _**is required**_ (not clearable). 
   
   1. `enable empty filter` is very misleading message. It should just say `Required` as before.
   2. When user click APPLY button but did not fill a required filter, right now it just shows **No Results**, user has no idea about some filter is wrong, or actually no results.  Could we highlight / show error message on the required filter field when user clicks APPLY button? It will clearly tell user how to use this filter.  
   
   cc @junlincc this is a very misleading bug, please prioritize it.


-- 
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] villebro edited a comment on issue #14307: [native filter] Enable empty filter broke charts in the dashboard

Posted by GitBox <gi...@apache.org>.
villebro edited a comment on issue #14307:
URL: https://github.com/apache/superset/issues/14307#issuecomment-825839702


   I believe the "Enable empty filter" and "Required" options try to address the same thing. The main difference I believe is as follows:
   - When a dashboard with a required field is loaded, it initially loads with no filters applied. In the following example I have a filter box where the user_id field is set to Required, but the dashboard loads up with no filters applied:
   ![image](https://user-images.githubusercontent.com/33317356/115913695-99026080-a479-11eb-9c9a-8ff3ed8aa9a7.png)
   In this case, "Enable empty filter" would load up with all charts showing "No data".
   
   - Also, this may be a bug in Filter Box or me just using it wrong, but I was not able to get "Required" to work - in the screenshot below, `user_id` is required, but `event_id` is not, and I was still able to apply the filter on `event_id` despite `user_id` being unset:
   ![image](https://user-images.githubusercontent.com/33317356/115913910-e67ecd80-a479-11eb-8b91-8c9bd49281bd.png)
   ![image](https://user-images.githubusercontent.com/33317356/115914005-0615f600-a47a-11eb-961d-c07ec7053e9a.png)
   
   I'm happy to work on adding the intended functionality for the "Required" option, and if we can consolidate it somehow with the "Enable empty filter" option then that would be optimal.


-- 
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] villebro commented on issue #14307: [native filter] Enable empty filter broke charts in the dashboard

Posted by GitBox <gi...@apache.org>.
villebro commented on issue #14307:
URL: https://github.com/apache/superset/issues/14307#issuecomment-825839702


   I believe the "Enable empty filter" and "Required" options try to address the same thing. The main difference I believe is as follows:
   - When a dashboard with a required field is loaded, it initially loads with no filters applied. In the following example I have a filter box where the user_id field is set to Required, but the dashboard loads up with no filters applied:
   ![image](https://user-images.githubusercontent.com/33317356/115913695-99026080-a479-11eb-9c9a-8ff3ed8aa9a7.png)
   - This may be a bug, but I was not able to get "Required" to work - in the screenshot below, `user_id` is required, but `event_id` is not, and I was still able to apply the filter on `event_id` despite `user_id` being unset:
   ![image](https://user-images.githubusercontent.com/33317356/115913910-e67ecd80-a479-11eb-8b91-8c9bd49281bd.png)
   ![image](https://user-images.githubusercontent.com/33317356/115914005-0615f600-a47a-11eb-961d-c07ec7053e9a.png)
   
   I'm happy to work on adding the intended functionality for the "Required" option, and if we can consolidate it somehow with the "Enable empty filter" option then that would be optimal.


-- 
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] graceguo-supercat edited a comment on issue #14307: [native filter] Enable empty filter broke charts in the dashboard

Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on issue #14307:
URL: https://github.com/apache/superset/issues/14307#issuecomment-825844738


   **consolidate "Required" with the "Enable empty filter" option** will be the best option. 
   - Please let me know how to match old filter_config `clearable` to the new filter component attribute.
   - I still prefer an _actionable_ message in the filter field itself, when user must fill in some value for a filter. Most of our dashboard users are not dashboard builder (who knows some fields have extra requirements, etc), so when "No Results" shows up they don't know why and will go panic....


-- 
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] villebro edited a comment on issue #14307: [native filter] Enable empty filter broke charts in the dashboard

Posted by GitBox <gi...@apache.org>.
villebro edited a comment on issue #14307:
URL: https://github.com/apache/superset/issues/14307#issuecomment-825549259


   This is on purpose: the "enable empty filter" is supposed to issue a WHERE clause that is always false if no selection is made. This feature has been requested by the community to minimize load on databases where issuing unfiltered queries causes performance issues. The description should probably be clearer (adding a tooltip would with a more verbose description would probably be the best solution).


-- 
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] graceguo-supercat commented on issue #14307: [native filter] Enable empty filter broke charts in the dashboard

Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on issue #14307:
URL: https://github.com/apache/superset/issues/14307#issuecomment-825844738


   **consolidate it somehow with the "Enable empty filter" option** will be the best option. 
   - Please let me know know how to match old filter_config `clearable` to the new filter component attribute.
   - I still prefer a actionable message in the filter field itself, when user must fill in some value for the filter. Most of our dashboard users are not dashboard builder (who knows some fields have extra requirements, etc), so when "No Results" shows up they don't know why and how to fix it....


-- 
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] villebro closed issue #14307: [native filter] Enable empty filter broke charts in the dashboard

Posted by GitBox <gi...@apache.org>.
villebro closed issue #14307:
URL: https://github.com/apache/superset/issues/14307


   


-- 
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] graceguo-supercat edited a comment on issue #14307: [native filter] Enable empty filter broke charts in the dashboard

Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on issue #14307:
URL: https://github.com/apache/superset/issues/14307#issuecomment-825802620


   Hi @villebro Thank you for the reply. So this option should match with old filter_box `Required` option. "Enable empty filter" means _**is required**_ (not clearable). 
   
   1. `enable empty filter` is very misleading message. It should just say `Required filed` as before.
   2. When user click APPLY button but did not fill a required filter, right now it just shows **No Results**, user has no idea about some filter is wrong, or actually no results.  Could we highlight / show error message on the required filter field when user clicks APPLY button? It will clearly tell user how to use this filter.  
   
   cc @junlincc this is a very misleading bug, please prioritize it.


-- 
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] villebro commented on issue #14307: [native filter] Enable empty filter broke charts in the dashboard

Posted by GitBox <gi...@apache.org>.
villebro commented on issue #14307:
URL: https://github.com/apache/superset/issues/14307#issuecomment-847026125


   Choosing "Required" will now work like in Filter Box, except in the case that the selection is empty (multiple select or default value is left empty), in which case the `1 = 0` filter will be triggered.


-- 
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