nruhe commented on a change in pull request #9593:
URL:
https://github.com/apache/incubator-superset/pull/9593#discussion_r467287095
##########
File path: superset-frontend/src/dashboard/util/getFilterConfigsFromFormdata.js
##########
@@ -20,89 +20,85 @@
import { TIME_FILTER_MAP } from '../../visualizations/FilterBox/FilterBox';
import { TIME_FILTER_LABELS } from '../../explore/constants';
-export default function getFilterConfigsFromFormdata(form_data = {}) {
+/**
+ * Parse filters for Table chart. All non-metric columns are considered
+ * filterable values.
+ */
+function getFilterConfigsFromTableChart(form_data = {}) {
+ const { groupby = [], all_columns = [] } = form_data;
+ const configs = { columns: {}, labels: {} };
+ // `groupby` is from GROUP BY mode (aggregations)
+ // `all_columns` is from NOT GROUP BY mode (raw records)
+ const columns = groupby.concat(all_columns);
+ columns.forEach(column => {
+ configs.columns[column] = undefined;
+ configs.labels[column] = column;
+ });
+ return configs;
+}
+
+/**
+ * Parse filter configs for FilterBox.
+ */
+function getFilterConfigsFromFilterBox(form_data = {}) {
const {
date_filter,
filter_configs = [],
show_druid_time_granularity,
show_druid_time_origin,
show_sqla_time_column,
show_sqla_time_granularity,
+ table_filter,
} = form_data;
- let configs = filter_configs.reduce(
- ({ columns, labels }, config) => {
- let defaultValues = config.defaultValue;
- // defaultValue could be ; separated values,
- // could be null or ''
- if (config.defaultValue) {
- defaultValues = config.defaultValue.split(';');
- }
- const updatedColumns = {
- ...columns,
- [config.column]: config.vals || defaultValues,
- };
- const updatedLabels = {
- ...labels,
- [config.column]: config.label,
- };
+ const configs = { columns: {}, labels: {} };
- return {
- columns: updatedColumns,
- labels: updatedLabels,
- };
- },
- { columns: {}, labels: {} },
- );
+ filter_configs.forEach(({ column, label, defaultValue, vals }) => {
+ const defaultValues =
+ typeof defaultValue === 'string' ? defaultValue.split(';') :
defaultValue;
+ configs.columns[column] = vals || defaultValues;
+ configs.labels[column] = label;
+ });
if (date_filter) {
- let updatedColumns = {
- ...configs.columns,
- [TIME_FILTER_MAP.time_range]: form_data.time_range,
- };
- const updatedLabels = {
- ...configs.labels,
- ...Object.entries(TIME_FILTER_MAP).reduce(
- (map, [key, value]) => ({
- ...map,
- [value]: TIME_FILTER_LABELS[key],
- }),
- {},
- ),
- };
Review comment:
@ktmud Realize this is old, but wanted to add my 2cents. Performance
considerations aside, I find reduce to frequently be faster to write and more
readable than for loops and forEach. The key reasons being that I can
cognitively follow the steps in sequence by knowing the goal is to turn A into
Z, which means somewhere along the chain, I must take that step to go from A to
C. As soon as that becomes imperative, I lose the ability to follow it
sequentially since variables, intermediary steps, etc. must be declared up
front. This leads me to have to make assumptions about what the purpose of the
code is to understand the context of what I'm looking at. In a similar vein,
that's why I typically hate non-curried underscore / lodash, because it
reverses the call order and makes you start at the innermost function call.
That being said, the real reason why we write code that way is to have the
ability to pause / resume execution at any point in the chain (i.e. generators).
----------------------------------------------------------------
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]
