Skip to content

Commit

Permalink
feat(native-filters): add optional sort metric to select filter (#14346)
Browse files Browse the repository at this point in the history
* feat(native-filters): add optional sort metric to select filter

* use verbose name when defined

* fixes

* lint

* disable flaky test

* disable flaky test

* disable flaky test

(cherry picked from commit 40fb94d)
  • Loading branch information
villebro authored and amitmiran137 committed Apr 28, 2021
1 parent 4ca132d commit 6a5330b
Show file tree
Hide file tree
Showing 11 changed files with 86 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@
import { CHART_LIST } from '../chart_list/chart_list.helper';
import { DASHBOARD_LIST } from '../dashboard_list/dashboard_list.helper';

// TODO: fix flaky init logic and re-enable
const milliseconds = new Date().getTime();
const dashboard = `Test Dashboard${milliseconds}`;
describe('Nativefilters', () => {
xdescribe('Nativefilters', () => {
before(() => {
cy.login();
cy.visit(DASHBOARD_LIST);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ import { TimeFilterPlugin, SelectFilterPlugin } from 'src/filters/components';
import { DATE_FILTER_CONTROL_TEST_ID } from 'src/explore/components/controls/DateFilterControl/DateFilterLabel';
import fetchMock from 'fetch-mock';
import { waitFor } from '@testing-library/react';
import mockDatasource, {
id as datasourceId,
datasourceId as fullDatasourceId,
} from 'spec/fixtures/mockDatasource';
import FilterBar, { FILTER_BAR_TEST_ID } from '.';
import { FILTERS_CONFIG_MODAL_TEST_ID } from '../FiltersConfigModal/FiltersConfigModal';

Expand All @@ -52,6 +56,10 @@ class MainPreset extends Preset {
}
}

fetchMock.get(`glob:*/api/v1/dataset/${datasourceId}`, {
result: [mockDatasource[fullDatasourceId]],
});

const getTestId = testWithId<string>(FILTER_BAR_TEST_ID, true);
const getModalTestId = testWithId<string>(FILTERS_CONFIG_MODAL_TEST_ID, true);
const getDateControlTestId = testWithId<string>(
Expand Down Expand Up @@ -349,7 +357,8 @@ describe('FilterBar', () => {
expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
});

it('add and edit filter set', async () => {
// TODO: fix flakiness and re-enable
it.skip('add and edit filter set', async () => {
// @ts-ignore
global.featureFlags = {
[FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET]: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,9 @@ const FiltersHeader: FC<FiltersHeaderProps> = ({ dataMask, filterSet }) => {
const getFilterRow = ({ id, name }: { id: string; name: string }) => {
const changedFilter =
filterSet &&
!areObjectsEqual(filters[id], filterSet?.nativeFilters?.[id]);
!areObjectsEqual(filters[id], filterSet?.nativeFilters?.[id], {
ignoreUndefined: true,
});
const removedFilter = !Object.keys(filters).includes(id);

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@ import {
SupersetApiError,
t,
} from '@superset-ui/core';
import { ColumnMeta, DatasourceMeta } from '@superset-ui/chart-controls';
import {
ColumnMeta,
DatasourceMeta,
Metric,
} from '@superset-ui/chart-controls';
import { FormInstance } from 'antd/lib/form';
import React, { useCallback, useEffect, useState } from 'react';
import { useSelector } from 'react-redux';
Expand All @@ -39,6 +43,7 @@ import AdhocFilterControl from 'src/explore/components/controls/FilterControl/Ad
import DateFilterControl from 'src/explore/components/controls/DateFilterControl';
import { addDangerToast } from 'src/messageToasts/actions';
import { ClientErrorObject } from 'src/utils/getClientErrorObject';
import SelectControl from 'src/explore/components/controls/SelectControl';
import Button from 'src/components/Button';
import { getChartDataRequest } from 'src/chart/chartAction';
import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
Expand Down Expand Up @@ -115,6 +120,7 @@ export const FiltersConfigForm: React.FC<FiltersConfigFormProps> = ({
form,
parentFilters,
}) => {
const [metrics, setMetrics] = useState<Metric[]>([]);
const forceUpdate = useForceUpdate();
const [datasetDetails, setDatasetDetails] = useState<Record<string, any>>();

Expand Down Expand Up @@ -158,6 +164,7 @@ export const FiltersConfigForm: React.FC<FiltersConfigFormProps> = ({
endpoint: `/api/v1/dataset/${datasetId}`,
})
.then((response: JsonResponse) => {
setMetrics(response.json?.result?.metrics);
const dataset = response.json?.result;
// modify the response to fit structure expected by AdhocFilterControl
dataset.type = dataset.datasource_type;
Expand All @@ -170,6 +177,8 @@ export const FiltersConfigForm: React.FC<FiltersConfigFormProps> = ({
}
}, [datasetId, hasColumn]);

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

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

Expand Down Expand Up @@ -469,6 +478,34 @@ 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.verbose_name ?? metric.metric_name,
}))}
onChange={(value: string | null): void => {
if (value !== undefined) {
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 @@ -36,6 +36,7 @@ export interface NativeFiltersFormItem {
value: string;
label: string;
};
sortMetric: string | null;
isInstant: boolean;
adhoc_filters?: AdhocFilter[];
time_range?: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,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;
adhoc_filters?: AdhocFilter[];
time_range?: string;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export const getFormData = ({
defaultValue,
controlValues,
filterType,
sortMetric,
adhoc_filters,
time_range,
}: Partial<Filter> & {
Expand All @@ -48,13 +49,20 @@ export const getFormData = ({
adhoc_filters?: AdhocFilter[];
time_range?: string;
}): Partial<QueryFormData> => {
const otherProps: { datasource?: string; groupby?: string[] } = {};
const otherProps: {
datasource?: string;
groupby?: string[];
sortMetric?: string;
} = {};
if (datasetId) {
otherProps.datasource = `${datasetId}__table`;
}
if (groupby) {
otherProps.groupby = [groupby];
}
if (sortMetric) {
otherProps.sortMetric = sortMetric;
}
return {
...controlValues,
...otherProps,
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
16 changes: 13 additions & 3 deletions superset-frontend/src/reduxUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import shortid from 'shortid';
import { compose } from 'redux';
import persistState, { StorageAdapter } from 'redux-localstorage';
import { isEqual } from 'lodash';
import { isEqual, omitBy, isUndefined } from 'lodash';

export function addToObject(
state: Record<string, any>,
Expand Down Expand Up @@ -169,6 +169,16 @@ export function areArraysShallowEqual(arr1: unknown[], arr2: unknown[]) {
return true;
}

export function areObjectsEqual(obj1: any, obj2: any) {
return isEqual(obj1, obj2);
export function areObjectsEqual(
obj1: any,
obj2: any,
opts = { ignoreUndefined: false },
) {
let comp1 = obj1;
let comp2 = obj2;
if (opts.ignoreUndefined) {
comp1 = omitBy(obj1, isUndefined);
comp2 = omitBy(obj2, isUndefined);
}
return isEqual(comp1, comp2);
}

0 comments on commit 6a5330b

Please sign in to comment.