rebenitez1802 commented on code in PR #37681:
URL: https://github.com/apache/superset/pull/37681#discussion_r2880222770
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx:
##########
@@ -132,540 +109,689 @@ describe('FilterBar', () => {
],
};
});
+}
+
+function createFilter(overrides: Record<string, unknown> = {}) {
+ const id = (overrides.id as string) || 'test-filter';
+ return {
+ id,
+ name: 'Test Filter',
+ filterType: 'filter_select',
+ targets: [{ datasetId: 1, column: { name: 'test_column' } }],
+ defaultDataMask: { filterState: { value: null }, extraFormData: {} },
+ controlValues: {},
+ cascadeParentIds: [],
+ scope: { rootPath: ['ROOT_ID'], excluded: [] },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [],
+ tabsInScope: [],
+ ...overrides,
+ };
+}
+
+function createDataMask(
+ filterId: string,
+ value: unknown = undefined,
+ extraFormData: Record<string, unknown> = {},
+) {
+ return {
+ id: filterId,
+ filterState: { value },
+ extraFormData,
+ };
+}
+
+function createDivider(overrides: Record<string, unknown> = {}) {
+ return {
+ id: 'NATIVE_FILTER_DIVIDER-1',
+ type: 'DIVIDER',
+ scope: { rootPath: ['ROOT_ID'], excluded: [] },
+ title: 'Select time range',
+ description: 'Select year/month etc..',
+ chartsInScope: [],
+ tabsInScope: [],
+ ...overrides,
+ };
+}
- const getTimeRangeNoFilterMockUrl =
- 'glob:*/api/v1/time_range/?q=%27No%20filter%27';
- const getTimeRangeLastDayMockUrl =
- 'glob:*/api/v1/time_range/?q=%27Last%20day%27';
- const getTimeRangeLastWeekMockUrl =
- 'glob:*/api/v1/time_range/?q=%27Last%20week%27';
-
- beforeEach(() => {
- jest.clearAllMocks();
-
- fetchMock.removeRoute(getTimeRangeNoFilterMockUrl);
- fetchMock.get(
- getTimeRangeNoFilterMockUrl,
- {
- result: { since: '', until: '', timeRange: 'No filter' },
+function createStateWithFilter(
+ filter: ReturnType<typeof createFilter>,
+ dataMask: ReturnType<typeof createDataMask>,
+ dashboardInfoOverrides: Record<string, unknown> = {},
+) {
+ return {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
+ metadata: {
+ native_filter_configuration: [filter],
},
- { name: getTimeRangeNoFilterMockUrl },
- );
-
- fetchMock.removeRoute(getTimeRangeLastDayMockUrl);
- fetchMock.get(
- getTimeRangeLastDayMockUrl,
- {
- result: {
- since: '2021-04-13T00:00:00',
- until: '2021-04-14T00:00:00',
- timeRange: 'Last day',
- },
+ ...dashboardInfoOverrides,
+ },
+ dashboardState: {
+ ...stateWithoutNativeFilters.dashboardState,
+ activeTabs: ['ROOT_ID'],
+ },
+ dataMask: { [filter.id]: dataMask },
+ nativeFilters: {
+ filters: { [filter.id]: filter },
+ filtersState: {},
+ },
+ };
+}
+
+function setupTimeRangeMocks() {
+ const urls = {
+ noFilter: 'glob:*/api/v1/time_range/?q=%27No%20filter%27',
+ lastDay: 'glob:*/api/v1/time_range/?q=%27Last%20day%27',
+ lastWeek: 'glob:*/api/v1/time_range/?q=%27Last%20week%27',
+ };
+
+ fetchMock.removeRoute(urls.noFilter);
+ fetchMock.get(
+ urls.noFilter,
+ { result: { since: '', until: '', timeRange: 'No filter' } },
+ { name: urls.noFilter },
+ );
+
+ fetchMock.removeRoute(urls.lastDay);
+ fetchMock.get(
+ urls.lastDay,
+ {
+ result: {
+ since: '2021-04-13T00:00:00',
+ until: '2021-04-14T00:00:00',
+ timeRange: 'Last day',
},
- { name: getTimeRangeLastDayMockUrl },
- );
-
- fetchMock.removeRoute(getTimeRangeLastWeekMockUrl);
- fetchMock.get(
- getTimeRangeLastWeekMockUrl,
- {
- result: {
- since: '2021-04-07T00:00:00',
- until: '2021-04-14T00:00:00',
- timeRange: 'Last week',
- },
+ },
+ { name: urls.lastDay },
+ );
+
+ fetchMock.removeRoute(urls.lastWeek);
+ fetchMock.get(
+ urls.lastWeek,
+ {
+ result: {
+ since: '2021-04-07T00:00:00',
+ until: '2021-04-14T00:00:00',
+ timeRange: 'Last week',
},
- { name: getTimeRangeLastWeekMockUrl },
- );
+ },
+ { name: urls.lastWeek },
+ );
+}
- mockedMakeApi.mockReturnValue(mockApi);
- });
+function renderFilterBar(
+ props: { filtersOpen: boolean; toggleFiltersBar: jest.Mock },
+ state?: object,
+) {
+ return render(
+ <FilterBar
+ orientation={FilterBarOrientation.Vertical}
+ verticalConfig={{
+ width: 280,
+ height: 400,
+ offset: 0,
+ ...props,
+ }}
+ />,
+ {
+ initialState: state,
+ useDnd: true,
+ useRedux: true,
+ useRouter: true,
+ },
+ );
+}
- const renderWrapper = (props = closedBarProps, state?: object) =>
- render(
- <FilterBar
- orientation={FilterBarOrientation.Vertical}
- verticalConfig={{
- width: 280,
- height: 400,
- offset: 0,
- ...props,
- }}
- />,
- {
- initialState: state,
- useDnd: true,
- useRedux: true,
- useRouter: true,
- },
- );
+test('FilterBar renders without crashing', () => {
+ const props = createClosedBarProps();
+ const { container } = renderFilterBar(props);
+ expect(container).toBeInTheDocument();
+});
- test('should render', () => {
- const { container } = renderWrapper();
- expect(container).toBeInTheDocument();
- });
+test('FilterBar renders "Filters and controls" heading', () => {
+ const props = createClosedBarProps();
+ renderFilterBar(props);
+ expect(screen.getByText('Filters and controls')).toBeInTheDocument();
+});
- test('should render the "Filters and controls" heading', () => {
- renderWrapper();
- expect(screen.getByText('Filters and controls')).toBeInTheDocument();
- });
+test('FilterBar renders "Clear all" button', () => {
+ const props = createClosedBarProps();
+ renderFilterBar(props);
+ expect(screen.getByText('Clear all')).toBeInTheDocument();
+});
- test('should render the "Clear all" option', () => {
- renderWrapper();
- expect(screen.getByText('Clear all')).toBeInTheDocument();
- });
+test('FilterBar renders "Apply filters" button', () => {
+ const props = createClosedBarProps();
+ renderFilterBar(props);
+ expect(screen.getByText('Apply filters')).toBeInTheDocument();
+});
- test('should render the "Apply filters" option', () => {
- renderWrapper();
- expect(screen.getByText('Apply filters')).toBeInTheDocument();
- });
+test('FilterBar renders collapse icon', () => {
+ const props = createClosedBarProps();
+ renderFilterBar(props);
+ expect(
+ screen.getByRole('img', { name: 'vertical-align' }),
+ ).toBeInTheDocument();
+});
- test('should render the collapse icon', () => {
- renderWrapper();
- expect(
- screen.getByRole('img', { name: 'vertical-align' }),
- ).toBeInTheDocument();
- });
+test('FilterBar renders filter icon', () => {
+ const props = createClosedBarProps();
+ renderFilterBar(props);
+ expect(screen.getByRole('img', { name: 'filter' })).toBeInTheDocument();
+});
- test('should render the filter icon', () => {
- renderWrapper();
- expect(screen.getByRole('img', { name: 'filter' })).toBeInTheDocument();
- });
+test('FilterBar calls toggleFiltersBar when collapse icon is clicked', () => {
+ const toggleFiltersBar = jest.fn();
+ const props = createClosedBarProps(toggleFiltersBar);
+ renderFilterBar(props);
- test('should toggle', () => {
- renderWrapper();
- const collapse = screen.getByRole('img', {
- name: 'vertical-align',
- });
- expect(toggleFiltersBar).not.toHaveBeenCalled();
- userEvent.click(collapse);
- expect(toggleFiltersBar).toHaveBeenCalled();
- });
+ const collapse = screen.getByRole('img', { name: 'vertical-align' });
+ expect(toggleFiltersBar).not.toHaveBeenCalled();
- test('open filter bar', () => {
- renderWrapper();
- expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
- expect(screen.getByTestId(getTestId('expand-button'))).toBeInTheDocument();
+ userEvent.click(collapse);
+ expect(toggleFiltersBar).toHaveBeenCalled();
+});
- userEvent.click(screen.getByTestId(getTestId('collapsable')));
- expect(toggleFiltersBar).toHaveBeenCalledWith(true);
- });
+test('FilterBar opens when expand button is clicked', () => {
+ const toggleFiltersBar = jest.fn();
+ const props = createClosedBarProps(toggleFiltersBar);
+ renderFilterBar(props);
- test('no edit filter button by disabled permissions', () => {
- renderWrapper(openedBarProps, {
- ...stateWithoutNativeFilters,
- dashboardInfo: { metadata: {} },
- });
+ expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
+ expect(screen.getByTestId(getTestId('expand-button'))).toBeInTheDocument();
- expect(
- screen.queryByTestId(getTestId('create-filter')),
- ).not.toBeInTheDocument();
- });
+ userEvent.click(screen.getByTestId(getTestId('collapsable')));
+ expect(toggleFiltersBar).toHaveBeenCalledWith(true);
+});
- test('close filter bar', () => {
- renderWrapper(openedBarProps);
- const collapseButton = screen.getByTestId(getTestId('collapse-button'));
+test('FilterBar hides edit filter button when user lacks permissions', () => {
+ const props = createOpenedBarProps();
+ const stateWithoutPermissions = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: { metadata: {} },
+ };
- expect(collapseButton).toBeInTheDocument();
- userEvent.click(collapseButton);
+ renderFilterBar(props, stateWithoutPermissions);
- expect(toggleFiltersBar).toHaveBeenCalledWith(false);
- });
+ expect(
+ screen.queryByTestId(getTestId('create-filter')),
+ ).not.toBeInTheDocument();
+});
- test('no filters', () => {
- renderWrapper(openedBarProps, stateWithoutNativeFilters);
+test('FilterBar closes when collapse button is clicked', () => {
+ const toggleFiltersBar = jest.fn();
+ const props = createOpenedBarProps(toggleFiltersBar);
+ renderFilterBar(props);
- expect(screen.getByTestId(getTestId('clear-button'))).toBeDisabled();
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
- });
+ const collapseButton = screen.getByTestId(getTestId('collapse-button'));
+ expect(collapseButton).toBeInTheDocument();
- test('renders dividers', async () => {
- const divider = {
- id: 'NATIVE_FILTER_DIVIDER-1',
- type: 'DIVIDER',
- scope: {
- rootPath: ['ROOT_ID'],
- excluded: [],
- },
- title: 'Select time range',
- description: 'Select year/month etc..',
- chartsInScope: [],
- tabsInScope: [],
- };
- const stateWithDivider = {
- ...stateWithoutNativeFilters,
- dashboardInfo: {
- ...stateWithoutNativeFilters.dashboardInfo,
- metadata: {
- ...stateWithoutNativeFilters.dashboardInfo.metadata,
- native_filter_configuration: [divider],
- },
- },
- nativeFilters: {
- filters: {
- 'NATIVE_FILTER_DIVIDER-1': divider,
- },
+ userEvent.click(collapseButton);
+ expect(toggleFiltersBar).toHaveBeenCalledWith(false);
+});
+
+test('FilterBar disables buttons when there are no filters', () => {
+ const props = createOpenedBarProps();
+ renderFilterBar(props, stateWithoutNativeFilters);
+
+ expect(screen.getByTestId(getTestId('clear-button'))).toBeDisabled();
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
+});
+
+test('FilterBar renders dividers with title and description', async () => {
+ const props = createOpenedBarProps();
+ const divider = createDivider();
+ const stateWithDivider = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ ...stateWithoutNativeFilters.dashboardInfo,
+ metadata: {
+ ...stateWithoutNativeFilters.dashboardInfo.metadata,
+ native_filter_configuration: [divider],
},
- };
+ },
+ nativeFilters: {
+ filters: { [divider.id]: divider },
+ },
+ };
- renderWrapper(openedBarProps, stateWithDivider);
+ renderFilterBar(props, stateWithDivider);
- await act(async () => {
- jest.advanceTimersByTime(1000); // 1s
- });
+ await act(async () => {
+ jest.advanceTimersByTime(1000);
+ });
+
+ const title = await screen.findByText('Select time range');
+ const description = await screen.findByText('Select year/month etc..');
+
+ expect(title.tagName).toBe('H3');
+ expect(description.tagName).toBe('P');
+ expect(screen.getByTestId(getTestId('clear-button'))).toBeDisabled();
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
+});
+
+test('FilterBar apply button is disabled after creating a filter', async () =>
{
+ setupTimeRangeMocks();
+ mockedMakeApi.mockReturnValue(createMockApi());
+
+ const props = createOpenedBarProps();
+ renderFilterBar(props, stateWithoutNativeFilters);
+
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
+
+ // Simulate add filter flow
+ userEvent.click(screen.getByTestId(getTestId('collapsable')));
+ userEvent.click(screen.getByLabelText('setting'));
+ userEvent.click(screen.getByText('Add or edit filters and controls'));
+ userEvent.click(screen.getByText('Value'));
+ userEvent.click(screen.getByText('Time range'));
+ userEvent.type(
+ screen.getByTestId(getModalTestId('name-input')),
+ 'Time filter 1',
+ );
+ userEvent.click(screen.getByText('Save'));
+
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
+});
- const title = await screen.findByText('Select time range');
- const description = await screen.findByText('Select year/month etc..');
+test('FilterBar renders without errors when filter has required
controlValues', () => {
+ const props = createOpenedBarProps();
+ const filter = createFilter({
+ id: 'test-filter',
+ controlValues: { enableEmptyFilter: true },
+ });
+ const dataMask = createDataMask('test-filter', undefined, {});
+ const state = createStateWithFilter(filter, dataMask);
+
+ const { container } = renderFilterBar(props, state);
+ expect(container).toBeInTheDocument();
+});
- expect(title.tagName).toBe('H3');
- expect(description.tagName).toBe('P');
- // Do not enable buttons if there are not filters
- expect(screen.getByTestId(getTestId('clear-button'))).toBeDisabled();
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
+test('FilterBar does not crash when filter has value but empty extraFormData',
async () => {
+ const filterId = 'test-filter-auto-apply';
+ const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
+ const props = createOpenedBarProps();
+
+ const filter = createFilter({
+ id: filterId,
+ requiredFirst: true,
+ controlValues: { enableEmptyFilter: true },
+ defaultDataMask: {
+ filterState: { value: ['value1'] },
+ extraFormData: {},
+ },
});
- test('create filter and apply it flow', async () => {
- renderWrapper(openedBarProps, stateWithoutNativeFilters);
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
+ const dataMask = createDataMask(filterId, ['value1'], {});
+ const state = createStateWithFilter(filter, dataMask);
- await addFilterFlow();
+ renderFilterBar(props, state);
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
+ await act(async () => {
+ jest.advanceTimersByTime(300);
});
- test('should render without errors with proper state setup', () => {
- const stateWithFilter = {
- ...stateWithoutNativeFilters,
- dashboardInfo: {
- id: 1,
- },
- dataMask: {
- 'test-filter': {
- id: 'test-filter',
- filterState: { value: undefined },
- extraFormData: {},
- },
- },
- nativeFilters: {
- filters: {
- 'test-filter': {
- id: 'test-filter',
- name: 'Test Filter',
- filterType: 'filter_select',
- targets: [{ datasetId: 1, column: { name: 'test_column' } }],
- defaultDataMask: {
- filterState: { value: undefined },
- extraFormData: {},
- },
- controlValues: {
- enableEmptyFilter: true,
- },
- cascadeParentIds: [],
- scope: {
- rootPath: ['ROOT_ID'],
- excluded: [],
- },
- type: 'NATIVE_FILTER',
- description: '',
- chartsInScope: [],
- tabsInScope: [],
- },
- },
- filtersState: {},
+ expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
+ expect(screen.getByText('Filters and controls')).toBeInTheDocument();
+
+ // Auto-apply dispatches updateDataMask when the filter plugin initializes
+ expect(updateDataMaskSpy).not.toHaveBeenCalledWith(
Review Comment:
consider adding a positive assertion like
`expect(updateDataMaskSpy).toHaveBeenCalledWith(...`
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
