Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(native-filters): add optional sort metric to select filter #14346

Merged
merged 10 commits into from
Apr 27, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export interface SupersetResourceSelectProps<T = unknown, V = string> {

const localCache = new Map<string, any>();

const cachedSupersetGet = cacheWrapper(
export const cachedSupersetGet = cacheWrapper(
SupersetClient.get,
localCache,
({ endpoint }) => endpoint || '',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,20 @@ import {
t,
getChartMetadataRegistry,
Behavior,
JsonResponse,
SupersetApiError,
} from '@superset-ui/core';
import { FormInstance } from 'antd/lib/form';
import React, { useCallback } from 'react';
import React, { useCallback, useEffect, useState } from 'react';
import { Metric } from '@superset-ui/chart-controls';
import { Checkbox, Form, Input, Typography } from 'src/common/components';
import { Select } from 'src/components/Select';
import SupersetResourceSelect from 'src/components/SupersetResourceSelect';
import SupersetResourceSelect, {
cachedSupersetGet,
} from 'src/components/SupersetResourceSelect';
import { addDangerToast } from 'src/messageToasts/actions';
import { ClientErrorObject } from 'src/utils/getClientErrorObject';
import SelectControl from 'src/explore/components/controls/SelectControl';
import { ColumnSelect } from './ColumnSelect';
import { NativeFiltersForm } from '../types';
import {
Expand Down Expand Up @@ -98,6 +104,7 @@ export const FiltersConfigForm: React.FC<FiltersConfigFormProps> = ({
form,
parentFilters,
}) => {
const [metrics, setMetrics] = useState<Metric[]>([]);
const forceUpdate = useForceUpdate();
const formFilter = (form.getFieldValue('filters') || {})[filterId];

Expand All @@ -115,16 +122,33 @@ export const FiltersConfigForm: React.FC<FiltersConfigFormProps> = ({
const hasColumn =
hasDataset && !FILTERS_WITHOUT_COLUMN.includes(formFilter?.filterType);

const datasetId = formFilter?.dataset?.value;

useEffect(() => {
if (datasetId) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If #14313 is merged first, that won't be necessary - metrics can be accessed in formFilter.dataset?.metrics

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nice, let's get that one in first then 👍

cachedSupersetGet({
endpoint: `/api/v1/dataset/${datasetId}`,
})
.then((response: JsonResponse) => {
setMetrics(response.json?.result?.metrics);
})
.catch((response: SupersetApiError) => {
addDangerToast(response.message);
});
}
}, [datasetId]);

const hasMetrics = hasColumn && !!metrics.length;

const hasFilledDataset =
!hasDataset ||
(formFilter?.dataset?.value && (formFilter?.column || !hasColumn));
!hasDataset || (datasetId && (formFilter?.column || !hasColumn));

useBackendFormUpdate(form, filterId, filterToEdit, hasDataset, hasColumn);

const initDatasetId = filterToEdit?.targets[0]?.datasetId;
const initColumn = filterToEdit?.targets[0]?.column?.name;
const newFormData = getFormData({
datasetId: formFilter?.dataset?.value,
datasetId,
groupby: hasColumn ? formFilter?.column : undefined,
defaultValue: formFilter?.defaultValue,
...formFilter,
Expand Down Expand Up @@ -203,7 +227,6 @@ export const FiltersConfigForm: React.FC<FiltersConfigFormProps> = ({
onError={onDatasetSelectError}
onChange={e => {
// We need reset column when dataset changed
const datasetId = formFilter?.dataset?.value;
if (datasetId && e?.value !== datasetId) {
setNativeFilterFieldValues(form, filterId, {
column: null,
Expand All @@ -226,7 +249,7 @@ export const FiltersConfigForm: React.FC<FiltersConfigFormProps> = ({
<ColumnSelect
form={form}
filterId={filterId}
datasetId={formFilter?.dataset?.value}
datasetId={datasetId}
onChange={forceUpdate}
/>
</StyledFormItem>
Expand Down Expand Up @@ -297,6 +320,32 @@ export const FiltersConfigForm: React.FC<FiltersConfigFormProps> = ({
form={form}
forceUpdate={forceUpdate}
/>
{hasMetrics && (
<StyledFormItem
// don't show the column select unless we have a dataset
// style={{ display: datasetId == null ? undefined : 'none' }}
name={['filters', filterId, 'sortMetric']}
initialValue={filterToEdit?.sortMetric}
label={<StyledLabel>{t('Sort Metric')}</StyledLabel>}
data-test="field-input"
>
<SelectControl
form={form}
filterId={filterId}
name="sortMetric"
options={metrics.map((metric: Metric) => ({
value: metric.metric_name,
label: metric.metric_name,
Copy link
Member

@zhaoyongjie zhaoyongjie Apr 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one nit:

Suggested change
label: metric.metric_name,
label: metric.verbose_name ?? metric.metric_name,

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @zhaoyongjie ; thanks for catching this!

}))}
onChange={(value: string | null): void => {
setNativeFilterFieldValues(form, filterId, {
sortMetric: value,
});
forceUpdate();
}}
/>
</StyledFormItem>
)}
<FilterScope
updateFormValues={(values: any) =>
setNativeFilterFieldValues(form, filterId, values)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export interface NativeFiltersFormItem {
value: string;
label: string;
};
sortMetric: string | null;
isInstant: boolean;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ export const createHandleSave = (
: [],
scope: formInputs.scope,
isInstant: formInputs.isInstant,
sortMetric: formInputs.sortMetric,
};
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export interface Filter {
controlValues: {
[key: string]: any;
};
sortMetric?: string | null;
}

export type FilterConfiguration = Filter[];
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export const getFormData = ({
defaultValue,
controlValues,
filterType,
sortMetric,
}: Partial<Filter> & {
datasetId?: number;
inputRef?: RefObject<HTMLInputElement>;
Expand Down Expand Up @@ -65,6 +66,7 @@ export const getFormData = ({
time_range_endpoints: ['inclusive', 'exclusive'],
url_params: {},
viz_type: filterType,
sortMetric,
inputRef,
};
};
Expand Down
9 changes: 7 additions & 2 deletions superset-frontend/src/filters/components/Select/buildQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ import { buildQueryContext } from '@superset-ui/core';
import { DEFAULT_FORM_DATA, PluginFilterSelectQueryFormData } from './types';

export default function buildQuery(formData: PluginFilterSelectQueryFormData) {
const { sortAscending } = { ...DEFAULT_FORM_DATA, ...formData };
const { sortAscending, sortMetric } = { ...DEFAULT_FORM_DATA, ...formData };
return buildQueryContext(formData, baseQueryObject => {
const { columns = [], filters = [] } = baseQueryObject;

const sortColumns = sortMetric ? [sortMetric] : columns;
return [
{
...baseQueryObject,
Expand All @@ -31,7 +33,10 @@ export default function buildQuery(formData: PluginFilterSelectQueryFormData) {
filters: filters.concat(
columns.map(column => ({ col: column, op: 'IS NOT NULL' })),
),
orderby: sortAscending ? columns.map(column => [column, true]) : [],
orderby:
sortMetric || sortAscending
? sortColumns.map(column => [column, sortAscending])
: [],
},
];
});
Expand Down
1 change: 1 addition & 0 deletions superset-frontend/src/filters/components/Select/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ interface PluginFilterSelectCustomizeProps {
defaultToFirstItem: boolean;
inputRef?: RefObject<HTMLInputElement>;
sortAscending: boolean;
sortMetric?: string;
}

export type PluginFilterSelectQueryFormData = QueryFormData &
Expand Down