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