Skip to content

Commit da85911

Browse files
author
Muhammad Musfir
committed
chore(fix): adds dependent filters fixes
* adds a fix for correctly selecting first value of a dependent filter * adds a fix for automatically selecting the correct dependent filter value on changing parent filter value * adds dependent filters fix support for time range filter
1 parent 30695d7 commit da85911

File tree

3 files changed

+140
-0
lines changed

3 files changed

+140
-0
lines changed

superset-frontend/cypress-base/cypress/e2e/dashboard/nativeFilters.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import {
4545
SAMPLE_CHART,
4646
visitDashboard,
4747
} from './shared_dashboard_functions';
48+
import { waitForChartLoad } from 'cypress/utils';
4849

4950
function selectFilter(index: number) {
5051
cy.get("[data-test='filter-title-container'] [draggable='true']")
@@ -160,6 +161,66 @@ describe('Native filters', () => {
160161
);
161162
});
162163

164+
it('Dependent filter selects first item based on parent filter selection', () => {
165+
prepareDashboardFilters([
166+
{ name: 'region', column: 'region', datasetId: 2 },
167+
{ name: 'country_name', column: 'country_name', datasetId: 2 },
168+
]);
169+
170+
enterNativeFilterEditModal();
171+
172+
selectFilter(0);
173+
cy.get(nativeFilters.filterConfigurationSections.displayedSection).within(() => {
174+
cy.contains('Select first filter value by default')
175+
.should('be.visible')
176+
.click();
177+
});
178+
cy.get(nativeFilters.filterConfigurationSections.displayedSection).within(() => {
179+
cy.contains('Can select multiple values ')
180+
.should('be.visible')
181+
.click();
182+
});
183+
184+
selectFilter(1);
185+
cy.get(nativeFilters.filterConfigurationSections.displayedSection).within(() => {
186+
cy.contains('Values are dependent on other filters')
187+
.should('be.visible')
188+
.click();
189+
});
190+
cy.get(nativeFilters.filterConfigurationSections.displayedSection).within(() => {
191+
cy.contains('Can select multiple values ')
192+
.should('be.visible')
193+
.click();
194+
});
195+
addParentFilterWithValue(0, testItems.topTenChart.filterColumnRegion);
196+
cy.get(nativeFilters.filterConfigurationSections.displayedSection).within(() => {
197+
cy.contains('Select first filter value by default')
198+
.should('be.visible')
199+
.click();
200+
});
201+
202+
// cannot use saveNativeFilterSettings because there is a bug which
203+
// sometimes does not allow charts to load when enabling the 'Select first filter value by default'
204+
// to be saved when using dependent filters so,
205+
// you reload the window.
206+
cy.get('.ant-modal-footer')
207+
.contains('Save')
208+
.should('be.visible')
209+
.click();
210+
211+
cy.get('.ant-modal-content').should('not.exist');
212+
cy.reload();
213+
[SAMPLE_CHART].forEach(waitForChartLoad);
214+
// saveNativeFilterSettings([SAMPLE_CHART]);
215+
216+
applyNativeFilterValueWithIndex(0, 'North America');
217+
218+
// Check that dependent filter auto-selects the first item
219+
cy.get(nativeFilters.filterFromDashboardView.filterContent)
220+
.eq(1)
221+
.should('contain.text', 'Bermuda');
222+
});
223+
163224
it('User can create filter depend on 2 other filters', () => {
164225
prepareDashboardFilters([
165226
{ name: 'region', column: 'region', datasetId: 2 },

superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ import { FilterControlProps } from './types';
5858
import { getFormData } from '../../utils';
5959
import { useFilterDependencies } from './state';
6060
import { useFilterOutlined } from '../useFilterOutlined';
61+
import { useFilters } from '../state';
6162

6263
const HEIGHT = 32;
6364

@@ -107,6 +108,7 @@ const FilterValue: FC<FilterControlProps> = ({
107108
state => state.dashboardInfo.id,
108109
);
109110

111+
const allFilters = useFilters();
110112
const [error, setError] = useState<ClientErrorObject>();
111113
const [formData, setFormData] = useState<Partial<QueryFormData>>({
112114
inView: false,
@@ -155,6 +157,37 @@ const FilterValue: FC<FilterControlProps> = ({
155157
dashboardId,
156158
});
157159
const filterOwnState = filter.dataMask?.ownState || {};
160+
if (filter?.cascadeParentIds?.length) {
161+
// check if it is a child filter
162+
163+
let selectedParentFilterValueCounts = 0;
164+
165+
filter?.cascadeParentIds?.forEach(pId => {
166+
if (
167+
allFilters[pId]?.controlValues?.defaultToFirstItem ||
168+
allFilters[pId]?.defaultDataMask?.extraFormData?.filters ||
169+
allFilters[pId]?.defaultDataMask?.extraFormData?.time_range
170+
) {
171+
selectedParentFilterValueCounts +=
172+
allFilters[pId]?.defaultDataMask?.extraFormData?.filters?.length ?? 1;
173+
}
174+
});
175+
176+
// check if all parent filters with defaults have a value selected
177+
178+
let depsCount = dependencies.filters?.length ?? 0;
179+
180+
if (dependencies?.time_range) {
181+
depsCount += 1;
182+
}
183+
184+
if (selectedParentFilterValueCounts !== depsCount) {
185+
// child filter should not request backend until it
186+
// has all the required information from parent filters
187+
return;
188+
}
189+
}
190+
158191
// TODO: We should try to improve our useEffect hooks to depend more on
159192
// granular information instead of big objects that require deep comparison.
160193
const customizer = (
@@ -226,6 +259,7 @@ const FilterValue: FC<FilterControlProps> = ({
226259
hasDataSource,
227260
isRefreshing,
228261
shouldRefresh,
262+
allFilters,
229263
]);
230264

231265
useEffect(() => {

superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,8 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
151151
const [col] = groupby;
152152
const [initialColtypeMap] = useState(coltypeMap);
153153
const [search, setSearch] = useState('');
154+
const isChangedByUser = useRef(false);
155+
const prevDataRef = useRef(data);
154156
const [dataMask, dispatchDataMask] = useImmerReducer(reducer, {
155157
extraFormData: {},
156158
filterState,
@@ -271,6 +273,8 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
271273
} else {
272274
updateDataMask(values);
273275
}
276+
277+
isChangedByUser.current = true;
274278
},
275279
[updateDataMask, formData.nativeFilterId, clearAllTrigger],
276280
);
@@ -368,6 +372,47 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
368372
inverseSelection,
369373
]);
370374

375+
useEffect(() => {
376+
const prev = prevDataRef.current;
377+
const curr = data;
378+
379+
const prevFirst = prev?.[0]?.[col];
380+
const currFirst = curr?.[0]?.[col];
381+
382+
// If data actually changed (e.g., due to parent filter), reset flag
383+
if (prevFirst !== currFirst) {
384+
isChangedByUser.current = false;
385+
prevDataRef.current = data;
386+
}
387+
}, [data, col]);
388+
389+
useEffect(() => {
390+
if (isChangedByUser.current) return;
391+
392+
const firstItem: SelectValue = data[0]
393+
? (groupby.map(col => data[0][col]) as string[])
394+
: null;
395+
396+
if (
397+
defaultToFirstItem &&
398+
Object.keys(formData?.extraFormData || {}).length &&
399+
filterState.value !== undefined &&
400+
firstItem !== null &&
401+
filterState.value !== firstItem
402+
) {
403+
if (firstItem?.[0] !== undefined) {
404+
updateDataMask(firstItem);
405+
}
406+
}
407+
}, [
408+
defaultToFirstItem,
409+
updateDataMask,
410+
formData,
411+
data,
412+
JSON.stringify(filterState.value),
413+
isChangedByUser.current,
414+
]);
415+
371416
useEffect(() => {
372417
setDataMask(dataMask);
373418
}, [JSON.stringify(dataMask)]);

0 commit comments

Comments
 (0)