villebro commented on a change in pull request #9348: fix: filter_values in 
jinja_context not processing adhoc_filters
URL: 
https://github.com/apache/incubator-superset/pull/9348#discussion_r396934586
 
 

 ##########
 File path: superset/jinja_context.py
 ##########
 @@ -93,18 +94,16 @@ def filter_values(column: str, default: Optional[str] = 
None) -> List[str]:
     :return: returns a list of filter values
     """
     form_data = json.loads(request.form.get("form_data", "{}"))
+    utils.harmonize_query_filters(form_data)
+
     return_val = []
-    for filter_type in ["filters", "extra_filters"]:
-        if filter_type not in form_data:
-            continue
-
-        for f in form_data[filter_type]:
-            if f["col"] == column:
-                if isinstance(f["val"], list):
-                    for v in f["val"]:
-                        return_val.append(v)
-                else:
-                    return_val.append(f["val"])
+    for f in form_data.get("filters", {}):
 
 Review comment:
   Probably something for the refactor backlog. I took another pass at how this 
works in practice, and these are my takeaways:
   - `connectors/sqla/model.py` has no logic for handling `adhoc_metrics`, so 
all filters are expected to be in base filter form: 
https://github.com/apache/incubator-superset/blob/master/superset/connectors/sqla/models.py#L837-L877
 . The same goes for the legacy Druid connector.
   - `QueryObject` used by the new Query API only supports base filters: 
https://github.com/apache/incubator-superset/blob/master/superset/common/query_object.py#L115
 . I think we need to change this so that `adhoc_filters` are allowed in the 
constructor signature and are converted to base filters in `to_dict()`.
   
   Anyway, in the short term, I think this fix is the right 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:
us...@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org

Reply via email to