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/07/19 19:29:56 UTC

[GitHub] [superset] eric-briscoe commented on a diff in pull request #20754: fix: set allow `filter_select` for Query objects in Explore

eric-briscoe commented on code in PR #20754:
URL: https://github.com/apache/superset/pull/20754#discussion_r924882355


##########
superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx:
##########
@@ -359,7 +359,7 @@ const AdhocFilterEditPopoverSimpleTabContent: React.FC<Props> = props => {
       const col = props.adhocFilter.subject;
       const having = props.adhocFilter.clause === CLAUSES.HAVING;
 
-      if (col && datasource && datasource.filter_select && !having) {
+      if (col && datasource?.filter_select !== false && !having) {

Review Comment:
   If we are adding the "filter_select": true in sql_lab.py we can remove this change.  If we keep this change we don't need to make the change in sql_lab.py. 
   
   If we do keep this change, I realized we should actually do a check on datasource because datasource. operations are used within the if block.  so recommend changing to this this:
   
   `if (col && datasource && datasource.filter_select !== false && !having) {`



##########
superset/models/sql_lab.py:
##########
@@ -212,6 +212,7 @@ def columns(self) -> List[ResultSetColumnType]:
     @property
     def data(self) -> Dict[str, Any]:
         return {
+            "filter_select": True,

Review Comment:
   Related to my comment in the frontend change.  If we add this here, the frontend change should not be needed.



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