villebro commented on a change in pull request #14507:
URL: https://github.com/apache/superset/pull/14507#discussion_r636022077
##########
File path: superset/jinja_context.py
##########
@@ -213,6 +176,142 @@ def url_param(
self.cache_key_wrapper(result)
return result
+ def filter_values(
+ self, column: str, default: Optional[str] = None, remove_filter: bool
= False
+ ) -> List[str]:
+ """Gets a values for a particular filter as a list
+
+ This is useful if:
+ - you want to use a filter component to filter a query where the
name of
+ filter component column doesn't match the one in the select
statement
+ - you want to have the ability for filter inside the main query
for speed
+ purposes
+
+ Usage example::
+
+ SELECT action, count(*) as times
+ FROM logs
+ WHERE
+ action in ({{ "'" + "','".join(filter_values('action_type')) +
"'" }})
+ GROUP BY action
+
+ :param column: column/filter name to lookup
+ :param default: default value to return if there's no matching columns
+ :param remove_filter: When set to true, mark the filter as processed,
+ removing it from the outer query. Useful when a filter should
+ only apply to the inner query
+ :return: returns a list of filter values
+ """
+ return_val: List[str] = []
+ filters = self.get_filters(column, remove_filter)
+ for flt in filters:
+ val: Optional[Any] = flt.get("val")
+ if isinstance(val, list):
+ return_val.extend(val)
+ else:
+ return_val.append(str(val))
+
+ if (not return_val) and default:
+ # If no values are found, return the default provided.
+ return_val = [default]
+
+ return return_val
+
+ def get_filters(self, column: str, remove_filter: bool = False) ->
List[Any]:
Review comment:
Now would be a golden opportunity to introduce a `TypedDict` for the
return type. Instead of returning `List[Any]` , it would be really nice to
return `List[Filter]` with based on a similar class construct as here:
https://github.com/apache/superset/blob/a9d888ad402ebb35da45df446997c426d6abee9d/superset/utils/core.py#L184-L186
##########
File path: superset/jinja_context.py
##########
@@ -213,6 +176,142 @@ def url_param(
self.cache_key_wrapper(result)
return result
+ def filter_values(
+ self, column: str, default: Optional[str] = None, remove_filter: bool
= False
+ ) -> List[str]:
+ """Gets a values for a particular filter as a list
+
+ This is useful if:
+ - you want to use a filter component to filter a query where the
name of
+ filter component column doesn't match the one in the select
statement
+ - you want to have the ability for filter inside the main query
for speed
+ purposes
+
+ Usage example::
+
+ SELECT action, count(*) as times
+ FROM logs
+ WHERE
+ action in ({{ "'" + "','".join(filter_values('action_type')) +
"'" }})
+ GROUP BY action
+
+ :param column: column/filter name to lookup
+ :param default: default value to return if there's no matching columns
+ :param remove_filter: When set to true, mark the filter as processed,
+ removing it from the outer query. Useful when a filter should
+ only apply to the inner query
+ :return: returns a list of filter values
+ """
Review comment:
Could we add a deprecation warning here, urging to migrate to
`get_filters` and stating that this function will be removed in 2.0? A similar
warning can be found here:
https://github.com/apache/superset/blob/a9d888ad402ebb35da45df446997c426d6abee9d/superset/app.py#L422-L425
##########
File path: superset/jinja_context.py
##########
@@ -213,6 +176,142 @@ def url_param(
self.cache_key_wrapper(result)
return result
+ def filter_values(
+ self, column: str, default: Optional[str] = None, remove_filter: bool
= False
+ ) -> List[str]:
+ """Gets a values for a particular filter as a list
+
+ This is useful if:
+ - you want to use a filter component to filter a query where the
name of
+ filter component column doesn't match the one in the select
statement
+ - you want to have the ability for filter inside the main query
for speed
+ purposes
+
+ Usage example::
+
+ SELECT action, count(*) as times
+ FROM logs
+ WHERE
+ action in ({{ "'" + "','".join(filter_values('action_type')) +
"'" }})
+ GROUP BY action
+
+ :param column: column/filter name to lookup
+ :param default: default value to return if there's no matching columns
+ :param remove_filter: When set to true, mark the filter as processed,
+ removing it from the outer query. Useful when a filter should
+ only apply to the inner query
+ :return: returns a list of filter values
+ """
+ return_val: List[str] = []
+ filters = self.get_filters(column, remove_filter)
+ for flt in filters:
+ val: Optional[Any] = flt.get("val")
+ if isinstance(val, list):
+ return_val.extend(val)
+ else:
+ return_val.append(str(val))
Review comment:
Why do we want to do `str(val)` here? Wouldn't it be better to keep it
in its original format?
--
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
