You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "Aitema-gmbh (via GitHub)" <gi...@apache.org> on 2026/04/09 19:01:37 UTC
[PR] fix(a11y): WCAG 1.3.3 — add aria-pressed/expanded to toggle buttons [superset]
Aitema-gmbh opened a new pull request, #39231:
URL: https://github.com/apache/superset/pull/39231
### SUMMARY
Implements WCAG 2.1 criterion 1.3.3 (Sensory Characteristics, Level A).
- Add `aria-pressed` to toggle buttons (Grid/List view, Favorite star)
- Add `aria-expanded` to collapsible filter bar with descriptive `aria-label`
- Add `aria-pressed` to auto-refresh indicator toggle
- Ensure information is not conveyed solely through visual appearance
### TESTING INSTRUCTIONS
1. Open Dashboard list → toggle Grid/List view → verify `aria-pressed` updates
2. Open a Dashboard → collapse/expand filter bar → verify `aria-expanded` toggles
3. Screen reader should announce toggle states
### ADDITIONAL INFORMATION
- [x] Changes UI
Part of a series of 16 individual WCAG 2.1 accessibility PRs. See also #38342.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(a11y): WCAG 1.3.3 — add aria-pressed/expanded to toggle buttons [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #39231:
URL: https://github.com/apache/superset/pull/39231#issuecomment-4282919969
I see there are a lot of these PRs and I value all this work. Thank you for contributing. Again, I think we might want to consider a lint rule that catches this stuff in future regressions or contributions.Also, there's still a lot of bot comments on this one too, so please assess, address, and comment on each one.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(a11y): WCAG 1.3.3 — add aria-pressed/expanded to toggle buttons [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #39231:
URL: https://github.com/apache/superset/pull/39231#issuecomment-4216765737
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(a11y): WCAG 1.3.3 — add aria-pressed/expanded to toggle buttons [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #39231:
URL: https://github.com/apache/superset/pull/39231#issuecomment-4773799294
The a11y attributes look right, thanks @Aitema-gmbh. One thing though: `FilterBar.test.tsx` was flat `test()` cases and this wraps it all back into a `describe('FilterBar')` with vaguer names. We're trying to go the other way ([avoid nesting](https://kentcdodds.com/blog/avoid-nesting-when-youre-testing)). Can we drop the rewrap and keep that file's changes to just the aria assertions? Also needs a rebase... holler if help here is needed.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(a11y): WCAG 1.3.3 — add aria-pressed/expanded to toggle buttons [superset]
Posted by "Aitema-gmbh (via GitHub)" <gi...@apache.org>.
Aitema-gmbh commented on code in PR #39231:
URL: https://github.com/apache/superset/pull/39231#discussion_r3232633312
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx:
##########
@@ -56,49 +55,73 @@ class MainPreset extends Preset {
}
}
-new MainPreset().register();
-
fetchMock.get('glob:*/api/v1/dataset/7', {
description_columns: {},
id: 1,
- label_columns: { columns: 'Columns', table_name: 'Table Name' },
+ label_columns: {
+ columns: 'Columns',
+ table_name: 'Table Name',
+ },
result: {
metrics: [],
- columns: [{ column_name: 'Column A', id: 1 }],
+ columns: [
+ {
+ column_name: 'Column A',
+ id: 1,
+ },
+ ],
table_name: 'birth_names',
id: 1,
},
show_columns: ['id', 'table_name'],
});
-// Cleanup between tests
-beforeEach(() => {
- jest.clearAllMocks();
-});
-
const getTestId = testWithId<string>(FILTER_BAR_TEST_ID, true);
const getModalTestId = testWithId<string>(FILTERS_CONFIG_MODAL_TEST_ID, true);
-function createClosedBarProps(toggleFiltersBar = jest.fn()) {
- return { filtersOpen: false, toggleFiltersBar };
-}
+const FILTER_NAME = 'Time filter 1';
-function createOpenedBarProps(toggleFiltersBar = jest.fn()) {
- return { filtersOpen: true, toggleFiltersBar };
-}
+const addFilterFlow = async () => {
+ // open filter config modals
+ userEvent.click(screen.getByTestId(getTestId('collapsable')));
+ userEvent.click(screen.getByLabelText('setting'));
+ userEvent.click(screen.getByText('Add or edit filters and controls'));
+ // select filter
+ userEvent.click(screen.getByText('Value'));
+ userEvent.click(screen.getByText('Time range'));
+ userEvent.type(screen.getByTestId(getModalTestId('name-input')), FILTER_NAME);
+ userEvent.click(screen.getByText('Save'));
+ // TODO: fix this flaky test
+ // await screen.findByText('All filters (1)');
Review Comment:
Addressed in e71b0d57 — addFilterFlow now asserts `await screen.findByText('All filters (1)')` after Save to confirm the filter was persisted. Resolving — thanks for the catch.
##########
superset-frontend/src/dashboard/components/AutoRefreshIndicator/index.tsx:
##########
@@ -124,6 +124,7 @@ export const AutoRefreshIndicator: FC<AutoRefreshIndicatorProps> = ({
css={iconButtonStyles}
onClick={onTogglePause}
aria-label={tooltipTitle}
+ aria-pressed={isPaused}
Review Comment:
Addressed in e71b0d57 — aria-label is now the static 'Toggle auto-refresh' with aria-pressed={isPaused} carrying the state, exactly per WCAG 1.3.3 toggle-button guidance. Inline comment in the component explains the choice. Resolving — thanks for the catch.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(a11y): WCAG 1.3.3 — add aria-pressed/expanded to toggle buttons [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #39231:
URL: https://github.com/apache/superset/pull/39231#discussion_r3060079397
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/Header.test.tsx:
##########
@@ -53,3 +53,21 @@ test('should toggle', () => {
userEvent.click(expandBtn);
expect(mockedProps.toggleFiltersBar).toHaveBeenCalled();
});
+
+test('collapse button should have aria-expanded attribute', () => {
+ const mockedProps = createProps();
+ render(<Header {...mockedProps} />, { useRedux: true });
+ const collapseBtn = screen.getByRole('button', {
+ name: 'vertical-align',
+ });
+ expect(collapseBtn).toHaveAttribute('aria-expanded', 'true');
+});
+
+test('collapse button should have aria-label for accessibility', () => {
+ const mockedProps = createProps();
+ render(<Header {...mockedProps} />, { useRedux: true });
+ const collapseBtn = screen.getByRole('button', {
+ name: 'vertical-align',
Review Comment:
**Suggestion:** The new tests look up the collapse button using the old accessible name (`vertical-align`), but the component now sets `aria-label="Collapse filters"`, which overrides the icon name for button accessibility. This makes the query fail at runtime. Query the button by its actual accessible name (`Collapse filters`) so the tests match the updated behavior. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Frontend filter bar header tests fail in CI.
- ⚠️ WCAG accessibility tests misrepresent button accessible name.
```
</details>
```suggestion
name: 'Collapse filters',
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the frontend unit tests that include `Header.test.tsx` at
`superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/Header.test.tsx:26-73`.
2. The test `"collapse button should have aria-label for accessibility"` at lines 66-72
renders `<Header />` and executes `screen.getByRole('button', { name: 'vertical-align' })`
at lines 69-71 to find the collapse button.
3. The `Header` component rendered in that test (from
`superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/index.tsx:69-81`)
defines the button with `aria-label={t('Collapse filters')}` at line 80, which makes the
button's accessible name `"Collapse filters"` rather than the icon name
`"vertical-align"`.
4. Because there is no button with role `button` and name `"vertical-align"`, `getByRole`
throws an error and the test fails before reaching the assertion on `aria-label`,
demonstrating that the query is using an outdated accessible name; updating the query to
`name: 'Collapse filters'` would align the test with the actual accessible name.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/Header.test.tsx
**Line:** 70:70
**Comment:**
*Possible Bug: The new tests look up the collapse button using the old accessible name (`vertical-align`), but the component now sets `aria-label="Collapse filters"`, which overrides the icon name for button accessibility. This makes the query fail at runtime. Query the button by its actual accessible name (`Collapse filters`) so the tests match the updated behavior.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39231&comment_hash=d8e66315080278463a127d02d4c719b213f86f0f4941064a5602894ba722f0e1&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39231&comment_hash=d8e66315080278463a127d02d4c719b213f86f0f4941064a5602894ba722f0e1&reaction=dislike'>👎</a>
##########
superset-frontend/src/dashboard/components/Header/index.tsx:
##########
@@ -757,6 +757,7 @@ const Header = (): JSX.Element => {
className="action-button"
css={editButtonStyle}
aria-label={t('Edit dashboard')}
+ aria-pressed={editMode}
Review Comment:
**Suggestion:** This control is conditionally rendered only when not in edit mode, so the pressed state can never become true while the button exists. Keeping `aria-pressed` here exposes an incorrect toggle semantic to assistive technologies and does not communicate real state changes. Remove the pressed state from this non-persistent action button. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Screen readers see non-toggling edit button as toggle.
- ⚠️ Dashboard editing header a11y semantics fail WCAG expectations.
```
</details>
```suggestion
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open any dashboard in the web UI, which renders `DashboardPage`
(`superset-frontend/src/dashboard/containers/DashboardPage.tsx`, lines 10–26 in the tool
output) and, via lazy-loading, `DashboardBuilder` from
`superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx` (import
at lines 9–10).
2. Within `DashboardBuilder`, the JSX memo `headerContent` includes `<DashboardHeader />`
when `hideDashboardHeader` is false (`DashboardBuilder.tsx`, lines 506–512), so `Header`
from `superset-frontend/src/dashboard/components/Header/index.tsx` is rendered on every
normal dashboard view.
3. In `Header`, `editMode` is derived from Redux state (`index.tsx`, lines 41–68 of the
tool output), and the header renders `{editMode ? <UndoRedoKeyListeners ... /> : <div
css={actionButtonsStyle}>...<Button ... aria-label={t('Edit dashboard')}
aria-pressed={editMode}>}` (`index.tsx`, lines 28–47 of the tool output). When the
dashboard first loads in view mode, `editMode` is `false`, so the branch with the "Edit
dashboard" button is rendered and `aria-pressed={editMode}` evaluates to `false`, exposing
a button with `aria-pressed="false"` to assistive technologies.
4. Activate the "Edit dashboard" button. `handleEnterEditMode` (`index.tsx`, lines 28–32
of the tool output) calls `toggleEditMode()`, which flips `editMode` to `true` in Redux.
On the next render, the ternary `{editMode ? ... : ...}` now takes the `true` branch and
shows `<UndoRedoKeyListeners>` instead of the button, so the original button with
`aria-pressed` is unmounted. At no point does a persistent control expose
`aria-pressed="true"`; the pressed state is never communicated, and the control is not a
true toggle, so the `aria-pressed` usage is semantically incorrect as the suggestion
describes.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/dashboard/components/Header/index.tsx
**Line:** 760:760
**Comment:**
*Logic Error: This control is conditionally rendered only when not in edit mode, so the pressed state can never become true while the button exists. Keeping `aria-pressed` here exposes an incorrect toggle semantic to assistive technologies and does not communicate real state changes. Remove the pressed state from this non-persistent action button.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39231&comment_hash=00eea698a78c147e12f89a665d023cbb10050c7944c346dac7b75aaa217522de&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39231&comment_hash=00eea698a78c147e12f89a665d023cbb10050c7944c346dac7b75aaa217522de&reaction=dislike'>👎</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(a11y): WCAG 1.3.3 — add aria-pressed/expanded to toggle buttons [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #39231:
URL: https://github.com/apache/superset/pull/39231#discussion_r3060092888
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx:
##########
@@ -109,684 +132,547 @@ function createMockApi(filterName = 'Time filter 1') {
],
};
});
-}
-
-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,
- };
-}
-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],
+ 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' },
},
- ...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: getTimeRangeNoFilterMockUrl },
+ );
+
+ fetchMock.removeRoute(getTimeRangeLastDayMockUrl);
+ fetchMock.get(
+ getTimeRangeLastDayMockUrl,
+ {
+ result: {
+ since: '2021-04-13T00:00:00',
+ until: '2021-04-14T00:00:00',
+ timeRange: 'Last day',
+ },
},
- },
- { 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: 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.lastWeek },
- );
-}
+ { name: getTimeRangeLastWeekMockUrl },
+ );
-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,
- },
- );
-}
-
-test('FilterBar renders without crashing', () => {
- const props = createClosedBarProps();
- const { container } = renderFilterBar(props);
- expect(container).toBeInTheDocument();
-});
-
-test('FilterBar renders "Filters and controls" heading', () => {
- const props = createClosedBarProps();
- renderFilterBar(props);
- 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('FilterBar renders "Apply filters" button', () => {
- const props = createClosedBarProps();
- renderFilterBar(props);
- 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('FilterBar renders filter icon', () => {
- const props = createClosedBarProps();
- renderFilterBar(props);
- 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);
-
- const collapse = screen.getByRole('img', { name: 'vertical-align' });
- expect(toggleFiltersBar).not.toHaveBeenCalled();
-
- userEvent.click(collapse);
- expect(toggleFiltersBar).toHaveBeenCalled();
-});
-
-test('FilterBar opens when expand button is clicked', () => {
- const toggleFiltersBar = jest.fn();
- const props = createClosedBarProps(toggleFiltersBar);
- renderFilterBar(props);
-
- expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
- expect(screen.getByTestId(getTestId('expand-button'))).toBeInTheDocument();
-
- userEvent.click(screen.getByTestId(getTestId('collapsable')));
- expect(toggleFiltersBar).toHaveBeenCalledWith(true);
-});
-
-test('FilterBar hides edit filter button when user lacks permissions', () => {
- const props = createOpenedBarProps();
- const stateWithoutPermissions = {
- ...stateWithoutNativeFilters,
- dashboardInfo: { metadata: {} },
- };
+ mockedMakeApi.mockReturnValue(mockApi);
+ });
- renderFilterBar(props, stateWithoutPermissions);
+ 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,
+ },
+ );
- expect(
- screen.queryByTestId(getTestId('create-filter')),
- ).not.toBeInTheDocument();
-});
+ test('should render', () => {
+ const { container } = renderWrapper();
+ expect(container).toBeInTheDocument();
+ });
-test('FilterBar closes when collapse button is clicked', () => {
- const toggleFiltersBar = jest.fn();
- const props = createOpenedBarProps(toggleFiltersBar);
- renderFilterBar(props);
+ test('should render the "Filters and controls" heading', () => {
+ renderWrapper();
+ expect(screen.getByText('Filters and controls')).toBeInTheDocument();
+ });
- const collapseButton = screen.getByTestId(getTestId('collapse-button'));
- expect(collapseButton).toBeInTheDocument();
+ test('should render the "Clear all" option', () => {
+ renderWrapper();
+ expect(screen.getByText('Clear all')).toBeInTheDocument();
+ });
- userEvent.click(collapseButton);
- expect(toggleFiltersBar).toHaveBeenCalledWith(false);
-});
+ test('should render the "Apply filters" option', () => {
+ renderWrapper();
+ expect(screen.getByText('Apply filters')).toBeInTheDocument();
+ });
-test('FilterBar disables buttons when there are no filters', () => {
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithoutNativeFilters);
+ test('should render the collapse icon', () => {
+ renderWrapper();
+ expect(
+ screen.getByRole('img', { name: 'vertical-align' }),
+ ).toBeInTheDocument();
+ });
- expect(screen.getByTestId(getTestId('clear-button'))).toBeDisabled();
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
-});
+ test('should render the filter icon', () => {
+ renderWrapper();
+ expect(screen.getByRole('img', { name: 'filter' })).toBeInTheDocument();
+ });
-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 },
- },
- };
+ test('should toggle', () => {
+ renderWrapper();
+ const collapse = screen.getByRole('img', {
+ name: 'vertical-align',
+ });
+ expect(toggleFiltersBar).not.toHaveBeenCalled();
+ userEvent.click(collapse);
+ expect(toggleFiltersBar).toHaveBeenCalled();
+ });
- renderFilterBar(props, stateWithDivider);
+ test('open filter bar', () => {
+ renderWrapper();
+ expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
+ expect(screen.getByTestId(getTestId('expand-button'))).toBeInTheDocument();
- await act(async () => {
- jest.advanceTimersByTime(1000);
+ userEvent.click(screen.getByTestId(getTestId('collapsable')));
+ expect(toggleFiltersBar).toHaveBeenCalledWith(true);
});
- const title = await screen.findByText('Select time range');
- const description = await screen.findByText('Select year/month etc..');
+ test('collapsed filter bar should have aria-expanded=false', () => {
+ renderWrapper();
+ const collapsedBar = screen.getByTestId(getTestId('collapsable'));
+ expect(collapsedBar).toHaveAttribute('aria-expanded', 'false');
+ expect(collapsedBar).toHaveAttribute('aria-label', 'Expand filters');
+ });
- 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('no edit filter button by disabled permissions', () => {
+ renderWrapper(openedBarProps, {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: { metadata: {} },
+ });
-test('FilterBar apply button is disabled after creating a filter', async () => {
- setupTimeRangeMocks();
- mockedMakeApi.mockReturnValue(createMockApi());
+ expect(
+ screen.queryByTestId(getTestId('create-filter')),
+ ).not.toBeInTheDocument();
+ });
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithoutNativeFilters);
+ test('close filter bar', () => {
+ renderWrapper(openedBarProps);
+ const collapseButton = screen.getByTestId(getTestId('collapse-button'));
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
+ expect(collapseButton).toBeInTheDocument();
+ userEvent.click(collapseButton);
- // 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(toggleFiltersBar).toHaveBeenCalledWith(false);
+ });
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
-});
+ test('no filters', () => {
+ renderWrapper(openedBarProps, stateWithoutNativeFilters);
-test('FilterBar renders without errors when filter has required controlValues', () => {
- const props = createOpenedBarProps();
- const filter = createFilter({
- id: 'test-filter',
- controlValues: { enableEmptyFilter: true },
+ expect(screen.getByTestId(getTestId('clear-button'))).toBeDisabled();
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
});
- const dataMask = createDataMask('test-filter', undefined, {});
- const state = createStateWithFilter(filter, dataMask);
- const { container } = renderFilterBar(props, state);
- expect(container).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,
+ },
+ },
+ };
-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: {},
- },
- });
+ renderWrapper(openedBarProps, stateWithDivider);
- const dataMask = createDataMask(filterId, ['value1'], {});
- const state = createStateWithFilter(filter, dataMask);
+ await act(async () => {
+ jest.advanceTimersByTime(1000); // 1s
+ });
- renderFilterBar(props, state);
+ const title = await screen.findByText('Select time range');
+ const description = await screen.findByText('Select year/month etc..');
- await act(async () => {
- jest.advanceTimersByTime(300);
+ 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();
});
- expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
- expect(screen.getByText('Filters and controls')).toBeInTheDocument();
+ test('create filter and apply it flow', async () => {
+ renderWrapper(openedBarProps, stateWithoutNativeFilters);
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
- // The filter value should not be cleared during initialization
- expect(updateDataMaskSpy).not.toHaveBeenCalled();
+ await addFilterFlow();
- updateDataMaskSpy.mockRestore();
-});
-
-test('FilterBar renders correctly when filter has complete extraFormData', async () => {
- const filterId = 'test-filter-complete';
- const props = createOpenedBarProps();
- const filter = createFilter({
- id: filterId,
- controlValues: { enableEmptyFilter: true },
- defaultDataMask: {
- filterState: { value: ['value1'] },
- extraFormData: {
- filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
- },
- },
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
});
- const dataMask = createDataMask(filterId, ['value1'], {
- filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
- });
- const state = createStateWithFilter(filter, dataMask);
- renderFilterBar(props, state);
+ 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: {},
+ },
+ };
- await act(async () => {
- jest.advanceTimersByTime(100);
+ const { container } = renderWrapper(openedBarProps, stateWithFilter);
+ expect(container).toBeInTheDocument();
});
- expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
-});
+ test('auto-applies filter when extraFormData is empty in applied state', async () => {
+ const filterId = 'test-filter-auto-apply';
+ const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
-test('handleClearAll dispatches updateDataMask with value undefined for filter_select', async () => {
- const filterId = 'NATIVE_FILTER-clear-select';
- const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
- const selectFilter = createFilter({
- id: filterId,
- name: 'Region',
- filterType: 'filter_select',
- targets: [{ datasetId: 7, column: { name: 'region' } }],
- defaultDataMask: { filterState: { value: null }, extraFormData: {} },
- chartsInScope: [18],
- });
- const stateWithSelect = {
- ...stateWithoutNativeFilters,
- dashboardInfo: {
- id: 1,
- dash_edit_perm: true,
- filterBarOrientation: FilterBarOrientation.Vertical,
- metadata: {
- native_filter_configuration: [selectFilter],
- chart_configuration: {},
+ const stateWithIncompleteFilter = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
},
- },
- dashboardState: {
- ...stateWithoutNativeFilters.dashboardState,
- activeTabs: ['ROOT_ID'],
- },
- dataMask: {
- [filterId]: createDataMask(filterId, ['East']),
- },
- nativeFilters: {
- filters: { [filterId]: selectFilter },
- filtersState: {},
- },
- };
+ dataMask: {
+ [filterId]: {
+ id: filterId,
+ filterState: { value: ['value1', 'value2'] },
+ extraFormData: {},
+ },
+ },
+ nativeFilters: {
+ filters: {
+ [filterId]: {
+ id: filterId,
+ name: 'Test Filter',
+ filterType: 'filter_select',
+ targets: [{ datasetId: 1, column: { name: 'test_column' } }],
+ defaultDataMask: {
+ filterState: { value: ['value1', 'value2'] },
+ extraFormData: {},
+ },
+ controlValues: {
+ enableEmptyFilter: true,
+ },
+ cascadeParentIds: [],
+ scope: {
+ rootPath: ['ROOT_ID'],
+ excluded: [],
+ },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [],
+ tabsInScope: [],
+ },
+ },
+ filtersState: {},
+ },
+ };
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithSelect);
- await act(async () => {
- jest.advanceTimersByTime(300);
- });
+ renderWrapper(openedBarProps, stateWithIncompleteFilter);
- const clearBtn = screen.getByTestId(getTestId('clear-button'));
- expect(clearBtn).not.toBeDisabled();
- await act(async () => {
- userEvent.click(clearBtn);
- });
+ await act(async () => {
+ jest.advanceTimersByTime(200);
+ });
- expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, {
- filterState: { value: undefined },
- extraFormData: {},
- });
- updateDataMaskSpy.mockRestore();
-});
+ expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
Review Comment:
**Suggestion:** This test claims to verify auto-apply behavior but never asserts that the data-mask update action was dispatched, so it can pass even when the feature is broken. Add an assertion on the spy before restoring it. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Auto-apply behavior regressions may bypass unit test coverage.
- ⚠️ Test name misleads about verified dashboard filter behavior.
```
</details>
```suggestion
expect(updateDataMaskSpy).toHaveBeenCalled();
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open `superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx` and
note the `handleFilterSelectionChange` callback at lines 224-246: it computes
`needsAutoApply` (lines 11-24 in the snippet) and, when `shouldDispatch` is true (lines
26-30), calls `dispatch(updateDataMask(filter.id, dataMaskForRedux))` at line 45 to
auto-apply filters whose applied value is missing corresponding `extraFormData`.
2. In
`superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx`,
the test `auto-applies filter when extraFormData is empty in applied state` at lines
389-444 sets up exactly that scenario: it spies on `dataMaskActions.updateDataMask` (line
391), builds `stateWithIncompleteFilter` with `filterState.value` set and `extraFormData:
{}` both in `dataMask` and `nativeFilters.filters[filterId].defaultDataMask` (lines
393-432), then renders `<FilterBar>` via `renderWrapper(openedBarProps,
stateWithIncompleteFilter)` at line 435.
3. The test advances fake timers by 200ms via `jest.advanceTimersByTime(200)` inside `act`
at lines 437-439, which should be sufficient for any debounced or asynchronous auto-apply
logic in `FilterBar` to run and dispatch `updateDataMask` as implemented in
`FilterBar/index.tsx:224-46`.
4. Despite this setup, the only assertion in the test is
`expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();` at line 441,
followed by `updateDataMaskSpy.mockRestore();` at line 443; the spy is never asserted. As
a result, if the `dispatch(updateDataMask(...))` call in `FilterBar/index.tsx` is
accidentally removed, short-circuited, or its condition changes (regressing the auto-apply
behavior), this test will still pass as long as the FilterBar renders its filter icon,
meaning regressions in the auto-apply path are not caught by this behavior-specific test.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx
**Line:** 442:442
**Comment:**
*Logic Error: This test claims to verify auto-apply behavior but never asserts that the data-mask update action was dispatched, so it can pass even when the feature is broken. Add an assertion on the spy before restoring it.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39231&comment_hash=34a8beefd2457d7ceb1697655c7892da8f1b90e2f0d27bd13f6c80f34c4fd058&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39231&comment_hash=34a8beefd2457d7ceb1697655c7892da8f1b90e2f0d27bd13f6c80f34c4fd058&reaction=dislike'>👎</a>
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx:
##########
@@ -56,49 +55,73 @@ class MainPreset extends Preset {
}
}
-new MainPreset().register();
-
fetchMock.get('glob:*/api/v1/dataset/7', {
description_columns: {},
id: 1,
- label_columns: { columns: 'Columns', table_name: 'Table Name' },
+ label_columns: {
+ columns: 'Columns',
+ table_name: 'Table Name',
+ },
result: {
metrics: [],
- columns: [{ column_name: 'Column A', id: 1 }],
+ columns: [
+ {
+ column_name: 'Column A',
+ id: 1,
+ },
+ ],
table_name: 'birth_names',
id: 1,
},
show_columns: ['id', 'table_name'],
});
-// Cleanup between tests
-beforeEach(() => {
- jest.clearAllMocks();
-});
-
const getTestId = testWithId<string>(FILTER_BAR_TEST_ID, true);
const getModalTestId = testWithId<string>(FILTERS_CONFIG_MODAL_TEST_ID, true);
-function createClosedBarProps(toggleFiltersBar = jest.fn()) {
- return { filtersOpen: false, toggleFiltersBar };
-}
+const FILTER_NAME = 'Time filter 1';
-function createOpenedBarProps(toggleFiltersBar = jest.fn()) {
- return { filtersOpen: true, toggleFiltersBar };
-}
+const addFilterFlow = async () => {
+ // open filter config modals
+ userEvent.click(screen.getByTestId(getTestId('collapsable')));
+ userEvent.click(screen.getByLabelText('setting'));
+ userEvent.click(screen.getByText('Add or edit filters and controls'));
+ // select filter
+ userEvent.click(screen.getByText('Value'));
+ userEvent.click(screen.getByText('Time range'));
+ userEvent.type(screen.getByTestId(getModalTestId('name-input')), FILTER_NAME);
+ userEvent.click(screen.getByText('Save'));
Review Comment:
**Suggestion:** The interaction helper performs multiple UI actions without waiting for modal/menu rendering between steps, so element lookups can run before the UI is updated and cause flaky failures. Make each interaction awaitable and use async queries (`findBy...`) for elements that appear after clicks. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ FilterBar create-filter flow test fails intermittently in CI.
- ⚠️ Flaky tests slow WCAG FilterBar UI iterations.
```
</details>
```suggestion
await userEvent.click(screen.getByTestId(getTestId('collapsable')));
await userEvent.click(screen.getByLabelText('setting'));
await userEvent.click(
await screen.findByText('Add or edit filters and controls'),
);
// select filter
await userEvent.click(await screen.findByText('Value'));
await userEvent.click(await screen.findByText('Time range'));
await userEvent.type(
await screen.findByTestId(getModalTestId('name-input')),
FILTER_NAME,
);
await userEvent.click(await screen.findByText('Save'));
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the Jest test suite targeting `FilterBar.test.tsx` (e.g. `yarn test
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx`),
which executes the `create filter and apply it flow` test defined at
`superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx:334-341`.
2. Inside this test, `await addFilterFlow()` is invoked (line 338), which calls the
`addFilterFlow` helper defined at
`superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx:84-96`.
3. `addFilterFlow` performs a sequence of synchronous queries and clicks without awaiting
UI updates: it clicks the collapsible button and the settings control, then immediately
calls `screen.getByText('Add or edit filters and controls')` and
`screen.getByText('Value')` / `'Time range'` (lines 86-93). These menu items are rendered
by `FilterBarSettings` only after the settings click, via the item defined at
`superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarSettings/index.tsx:7-18`
(`{t('Add or edit filters and controls')}`), and the empty-state guidance in
`Vertical.tsx` at
`superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx:10-16`.
4. Because `userEvent.click` is asynchronous under `spec/helpers/testing-library` and no
`await`/`findBy...` is used, the `getByText`/`getByTestId` calls in `addFilterFlow` can
run before React finishes rendering the menu/modal; when this happens, Testing Library
throws (e.g. failing to find `"Add or edit filters and controls"` at line 88), causing the
`create filter and apply it flow` test to fail intermittently. The in-file comment `//
TODO: fix this flaky test` at line 94 indicates this flakiness is already observed in
practice.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx
**Line:** 86:93
**Comment:**
*Possible Bug: The interaction helper performs multiple UI actions without waiting for modal/menu rendering between steps, so element lookups can run before the UI is updated and cause flaky failures. Make each interaction awaitable and use async queries (`findBy...`) for elements that appear after clicks.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39231&comment_hash=34f7641b42b6d89ea541700967995e11376aaaaf1cb07b7894253b923c4d85b3&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39231&comment_hash=34f7641b42b6d89ea541700967995e11376aaaaf1cb07b7894253b923c4d85b3&reaction=dislike'>👎</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(a11y): WCAG 1.3.3 — add aria-pressed/expanded to toggle buttons [superset]
Posted by "Aitema-gmbh (via GitHub)" <gi...@apache.org>.
Aitema-gmbh commented on code in PR #39231:
URL: https://github.com/apache/superset/pull/39231#discussion_r3232630687
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/Header.test.tsx:
##########
@@ -53,3 +53,21 @@ test('should toggle', () => {
userEvent.click(expandBtn);
expect(mockedProps.toggleFiltersBar).toHaveBeenCalled();
});
+
+test('collapse button should have aria-expanded attribute', () => {
+ const mockedProps = createProps();
+ render(<Header {...mockedProps} />, { useRedux: true });
+ const collapseBtn = screen.getByRole('button', {
+ name: 'vertical-align',
+ });
+ expect(collapseBtn).toHaveAttribute('aria-expanded', 'true');
+});
+
+test('collapse button should have aria-label for accessibility', () => {
+ const mockedProps = createProps();
+ render(<Header {...mockedProps} />, { useRedux: true });
+ const collapseBtn = screen.getByRole('button', {
+ name: 'vertical-align',
Review Comment:
Addressed in 7a006443 — switched test selectors from 'vertical-align' to 'Collapse filters' to match the new aria-label. Resolving — thanks for the catch.
##########
superset-frontend/src/dashboard/components/Header/index.tsx:
##########
@@ -757,6 +757,7 @@ const Header = (): JSX.Element => {
className="action-button"
css={editButtonStyle}
aria-label={t('Edit dashboard')}
+ aria-pressed={editMode}
Review Comment:
Addressed in aabe2d99 — removed aria-pressed from the non-toggling 'Edit dashboard' button since it unmounts when entering edit mode rather than persisting as a true toggle. Resolving — thanks for the catch.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(a11y): WCAG 1.3.3 — add aria-pressed/expanded to toggle buttons [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #39231:
URL: https://github.com/apache/superset/pull/39231#issuecomment-4217232040
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(a11y): WCAG 1.3.3 — add aria-pressed/expanded to toggle buttons [superset]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #39231:
URL: https://github.com/apache/superset/pull/39231#discussion_r3112527259
##########
superset-frontend/src/dashboard/components/Header/Header.test.tsx:
##########
@@ -574,6 +574,18 @@ test('should toggle the edit mode', () => {
expect(logEvent).toHaveBeenCalled();
});
+test('edit dashboard button should have aria-pressed attribute', () => {
+ const canEditState = {
+ dashboardInfo: {
+ ...initialState.dashboardInfo,
+ dash_edit_perm: true,
+ },
+ };
+ setup(canEditState);
+ const editButton = screen.getByTestId('edit-dashboard-button');
+ expect(editButton).toHaveAttribute('aria-pressed', 'false');
+});
Review Comment:
This test expects the “Edit dashboard” button to have `aria-pressed`, but the current `Header` implementation renders a regular button without `aria-pressed` (and it isn’t a persistent toggle in the UI). Either add an `aria-pressed` attribute driven by edit mode state in the component, or adjust/remove this assertion so the test matches the actual behavior.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(a11y): WCAG 1.3.3 — add aria-pressed/expanded to toggle buttons [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #39231:
URL: https://github.com/apache/superset/pull/39231#issuecomment-4276326463
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | e71b0d57abe186281a9dc87b0182a8426ad6a043 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/69e500c4ab2f21000831790e |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-39231--superset-docs-preview.netlify.app](https://deploy-preview-39231--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-39231--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(a11y): WCAG 1.3.3 — add aria-pressed/expanded to toggle buttons [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #39231:
URL: https://github.com/apache/superset/pull/39231#issuecomment-4403306979
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(a11y): WCAG 1.3.3 — add aria-pressed/expanded to toggle buttons [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #39231:
URL: https://github.com/apache/superset/pull/39231#discussion_r3060168657
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx:
##########
@@ -56,49 +55,73 @@ class MainPreset extends Preset {
}
}
-new MainPreset().register();
-
fetchMock.get('glob:*/api/v1/dataset/7', {
description_columns: {},
id: 1,
- label_columns: { columns: 'Columns', table_name: 'Table Name' },
+ label_columns: {
+ columns: 'Columns',
+ table_name: 'Table Name',
+ },
result: {
metrics: [],
- columns: [{ column_name: 'Column A', id: 1 }],
+ columns: [
+ {
+ column_name: 'Column A',
+ id: 1,
+ },
+ ],
table_name: 'birth_names',
id: 1,
},
show_columns: ['id', 'table_name'],
});
-// Cleanup between tests
-beforeEach(() => {
- jest.clearAllMocks();
-});
-
const getTestId = testWithId<string>(FILTER_BAR_TEST_ID, true);
const getModalTestId = testWithId<string>(FILTERS_CONFIG_MODAL_TEST_ID, true);
-function createClosedBarProps(toggleFiltersBar = jest.fn()) {
- return { filtersOpen: false, toggleFiltersBar };
-}
+const FILTER_NAME = 'Time filter 1';
-function createOpenedBarProps(toggleFiltersBar = jest.fn()) {
- return { filtersOpen: true, toggleFiltersBar };
-}
+const addFilterFlow = async () => {
+ // open filter config modals
+ userEvent.click(screen.getByTestId(getTestId('collapsable')));
+ userEvent.click(screen.getByLabelText('setting'));
+ userEvent.click(screen.getByText('Add or edit filters and controls'));
+ // select filter
+ userEvent.click(screen.getByText('Value'));
+ userEvent.click(screen.getByText('Time range'));
+ userEvent.type(screen.getByTestId(getModalTestId('name-input')), FILTER_NAME);
+ userEvent.click(screen.getByText('Save'));
+ // TODO: fix this flaky test
+ // await screen.findByText('All filters (1)');
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing test assertion for filter addition</b></div>
<div id="fix">
Uncomment the assertion and use the correct async Jest pattern, for example:
```js
expect(await screen.findByText('All filters (1)')).toBeInTheDocument();
```
</div>
</div>
<small><i>Code Review Run #e671b2</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/index.tsx:
##########
@@ -76,6 +76,8 @@ const Header: FC<HeaderProps> = ({ toggleFiltersBar }) => (
buttonStyle="link"
buttonSize="xsmall"
onClick={() => toggleFiltersBar(false)}
+ aria-expanded
+ aria-label={t('Collapse filters')}
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incomplete aria-expanded implementation</b></div>
<div id="fix">
The aria-expanded attribute is currently hardcoded to true, but it should dynamically reflect whether the filters bar is expanded. This ensures correct accessibility behavior for screen readers. The aria-label correctly uses the translation function as expected.
</div>
</div>
<small><i>Code Review Run #e671b2</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/dashboard/components/AutoRefreshIndicator/index.tsx:
##########
@@ -124,6 +124,7 @@ export const AutoRefreshIndicator: FC<AutoRefreshIndicatorProps> = ({
css={iconButtonStyles}
onClick={onTogglePause}
aria-label={tooltipTitle}
+ aria-pressed={isPaused}
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Accessibility: Static aria-label for toggle button</b></div>
<div id="fix">
The aria-label is currently dynamic, changing between 'Resume auto-refresh' and 'Pause auto-refresh', which may confuse screen readers. For toggle buttons, the accessible name should be static, with aria-pressed indicating the state.
</div>
</div>
<small><i>Code Review Run #e671b2</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/Header.test.tsx:
##########
@@ -53,3 +53,21 @@ test('should toggle', () => {
userEvent.click(expandBtn);
expect(mockedProps.toggleFiltersBar).toHaveBeenCalled();
});
+
+test('collapse button should have aria-expanded attribute', () => {
+ const mockedProps = createProps();
+ render(<Header {...mockedProps} />, { useRedux: true });
+ const collapseBtn = screen.getByRole('button', {
+ name: 'vertical-align',
+ });
+ expect(collapseBtn).toHaveAttribute('aria-expanded', 'true');
+});
+
+test('collapse button should have aria-label for accessibility', () => {
+ const mockedProps = createProps();
+ render(<Header {...mockedProps} />, { useRedux: true });
+ const collapseBtn = screen.getByRole('button', {
+ name: 'vertical-align',
+ });
+ expect(collapseBtn).toHaveAttribute('aria-label', 'Collapse filters');
+});
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incorrect test selector</b></div>
<div id="fix">
The test selectors use 'vertical-align' as the accessible name, but the button has aria-label='Collapse filters', making 'Collapse filters' the correct accessible name. This will cause the tests to fail since they won't find the button.
</div>
</div>
<small><i>Code Review Run #e671b2</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(a11y): WCAG 1.3.3 — add aria-pressed/expanded to toggle buttons [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #39231:
URL: https://github.com/apache/superset/pull/39231#issuecomment-4276428726
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(a11y): WCAG 1.3.3 — add aria-pressed/expanded to toggle buttons [superset]
Posted by "Aitema-gmbh (via GitHub)" <gi...@apache.org>.
Aitema-gmbh commented on code in PR #39231:
URL: https://github.com/apache/superset/pull/39231#discussion_r3232635473
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx:
##########
@@ -109,684 +139,550 @@ function createMockApi(filterName = 'Time filter 1') {
],
};
});
-}
-
-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,
- };
-}
-
-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],
+ 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' },
},
- ...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: getTimeRangeNoFilterMockUrl },
+ );
+
+ fetchMock.removeRoute(getTimeRangeLastDayMockUrl);
+ fetchMock.get(
+ getTimeRangeLastDayMockUrl,
+ {
+ result: {
+ since: '2021-04-13T00:00:00',
+ until: '2021-04-14T00:00:00',
+ timeRange: 'Last day',
+ },
},
- },
- { 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: 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.lastWeek },
- );
-}
-
-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,
- },
- );
-}
-
-test('FilterBar renders without crashing', () => {
- const props = createClosedBarProps();
- const { container } = renderFilterBar(props);
- expect(container).toBeInTheDocument();
-});
+ { name: getTimeRangeLastWeekMockUrl },
+ );
-test('FilterBar renders "Filters and controls" heading', () => {
- const props = createClosedBarProps();
- renderFilterBar(props);
- 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('FilterBar renders "Apply filters" button', () => {
- const props = createClosedBarProps();
- renderFilterBar(props);
- 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('FilterBar renders filter icon', () => {
- const props = createClosedBarProps();
- renderFilterBar(props);
- 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);
-
- const collapse = screen.getByRole('img', { name: 'vertical-align' });
- expect(toggleFiltersBar).not.toHaveBeenCalled();
-
- userEvent.click(collapse);
- expect(toggleFiltersBar).toHaveBeenCalled();
-});
-
-test('FilterBar opens when expand button is clicked', () => {
- const toggleFiltersBar = jest.fn();
- const props = createClosedBarProps(toggleFiltersBar);
- renderFilterBar(props);
-
- expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
- expect(screen.getByTestId(getTestId('expand-button'))).toBeInTheDocument();
-
- userEvent.click(screen.getByTestId(getTestId('collapsable')));
- expect(toggleFiltersBar).toHaveBeenCalledWith(true);
-});
-
-test('FilterBar hides edit filter button when user lacks permissions', () => {
- const props = createOpenedBarProps();
- const stateWithoutPermissions = {
- ...stateWithoutNativeFilters,
- dashboardInfo: { metadata: {} },
- };
+ mockedMakeApi.mockReturnValue(mockApi);
+ });
- renderFilterBar(props, stateWithoutPermissions);
+ 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,
+ },
+ );
- expect(
- screen.queryByTestId(getTestId('create-filter')),
- ).not.toBeInTheDocument();
-});
+ test('should render', () => {
+ const { container } = renderWrapper();
+ expect(container).toBeInTheDocument();
+ });
-test('FilterBar closes when collapse button is clicked', () => {
- const toggleFiltersBar = jest.fn();
- const props = createOpenedBarProps(toggleFiltersBar);
- renderFilterBar(props);
+ test('should render the "Filters and controls" heading', () => {
+ renderWrapper();
+ expect(screen.getByText('Filters and controls')).toBeInTheDocument();
+ });
- const collapseButton = screen.getByTestId(getTestId('collapse-button'));
- expect(collapseButton).toBeInTheDocument();
+ test('should render the "Clear all" option', () => {
+ renderWrapper();
+ expect(screen.getByText('Clear all')).toBeInTheDocument();
+ });
- userEvent.click(collapseButton);
- expect(toggleFiltersBar).toHaveBeenCalledWith(false);
-});
+ test('should render the "Apply filters" option', () => {
+ renderWrapper();
+ expect(screen.getByText('Apply filters')).toBeInTheDocument();
+ });
-test('FilterBar disables buttons when there are no filters', () => {
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithoutNativeFilters);
+ test('should render the collapse icon', () => {
+ renderWrapper();
+ expect(
+ screen.getByRole('img', { name: 'vertical-align' }),
+ ).toBeInTheDocument();
+ });
- expect(screen.getByTestId(getTestId('clear-button'))).toBeDisabled();
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
-});
+ test('should render the filter icon', () => {
+ renderWrapper();
+ expect(screen.getByRole('img', { name: 'filter' })).toBeInTheDocument();
+ });
-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 },
- },
- };
+ test('should toggle', () => {
+ renderWrapper();
+ const collapse = screen.getByRole('img', {
+ name: 'vertical-align',
+ });
+ expect(toggleFiltersBar).not.toHaveBeenCalled();
+ userEvent.click(collapse);
+ expect(toggleFiltersBar).toHaveBeenCalled();
+ });
- renderFilterBar(props, stateWithDivider);
+ test('open filter bar', () => {
+ renderWrapper();
+ expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
+ expect(screen.getByTestId(getTestId('expand-button'))).toBeInTheDocument();
- await act(async () => {
- jest.advanceTimersByTime(1000);
+ userEvent.click(screen.getByTestId(getTestId('collapsable')));
+ expect(toggleFiltersBar).toHaveBeenCalledWith(true);
});
- const title = await screen.findByText('Select time range');
- const description = await screen.findByText('Select year/month etc..');
+ test('collapsed filter bar should have aria-expanded=false', () => {
+ renderWrapper();
+ const collapsedBar = screen.getByTestId(getTestId('collapsable'));
+ expect(collapsedBar).toHaveAttribute('aria-expanded', 'false');
+ expect(collapsedBar).toHaveAttribute('aria-label', 'Expand filters');
+ });
- 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('no edit filter button by disabled permissions', () => {
+ renderWrapper(openedBarProps, {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: { metadata: {} },
+ });
-test('FilterBar apply button is disabled after creating a filter', async () => {
- setupTimeRangeMocks();
- mockedMakeApi.mockReturnValue(createMockApi());
+ expect(
+ screen.queryByTestId(getTestId('create-filter')),
+ ).not.toBeInTheDocument();
+ });
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithoutNativeFilters);
+ test('close filter bar', () => {
+ renderWrapper(openedBarProps);
+ const collapseButton = screen.getByTestId(getTestId('collapse-button'));
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
+ expect(collapseButton).toBeInTheDocument();
+ userEvent.click(collapseButton);
- // 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(toggleFiltersBar).toHaveBeenCalledWith(false);
+ });
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
-});
+ test('no filters', () => {
+ renderWrapper(openedBarProps, stateWithoutNativeFilters);
-test('FilterBar renders without errors when filter has required controlValues', () => {
- const props = createOpenedBarProps();
- const filter = createFilter({
- id: 'test-filter',
- controlValues: { enableEmptyFilter: true },
+ expect(screen.getByTestId(getTestId('clear-button'))).toBeDisabled();
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
});
- const dataMask = createDataMask('test-filter', undefined, {});
- const state = createStateWithFilter(filter, dataMask);
- const { container } = renderFilterBar(props, state);
- expect(container).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,
+ },
+ },
+ };
-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: {},
- },
- });
+ renderWrapper(openedBarProps, stateWithDivider);
- const dataMask = createDataMask(filterId, ['value1'], {});
- const state = createStateWithFilter(filter, dataMask);
+ await act(async () => {
+ jest.advanceTimersByTime(1000); // 1s
+ });
- renderFilterBar(props, state);
+ const title = await screen.findByText('Select time range');
+ const description = await screen.findByText('Select year/month etc..');
- await act(async () => {
- jest.advanceTimersByTime(300);
+ 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();
});
- expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
- expect(screen.getByText('Filters and controls')).toBeInTheDocument();
+ test('create filter and apply it flow', async () => {
+ renderWrapper(openedBarProps, stateWithoutNativeFilters);
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
- // The filter value should not be cleared during initialization
- expect(updateDataMaskSpy).not.toHaveBeenCalled();
+ await addFilterFlow();
- updateDataMaskSpy.mockRestore();
-});
-
-test('FilterBar renders correctly when filter has complete extraFormData', async () => {
- const filterId = 'test-filter-complete';
- const props = createOpenedBarProps();
- const filter = createFilter({
- id: filterId,
- controlValues: { enableEmptyFilter: true },
- defaultDataMask: {
- filterState: { value: ['value1'] },
- extraFormData: {
- filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
- },
- },
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
});
- const dataMask = createDataMask(filterId, ['value1'], {
- filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
- });
- const state = createStateWithFilter(filter, dataMask);
- renderFilterBar(props, state);
+ 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: {},
+ },
+ };
- await act(async () => {
- jest.advanceTimersByTime(100);
+ const { container } = renderWrapper(openedBarProps, stateWithFilter);
+ expect(container).toBeInTheDocument();
});
- expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
-});
+ test('auto-applies filter when extraFormData is empty in applied state', async () => {
+ const filterId = 'test-filter-auto-apply';
+ const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
-test('handleClearAll dispatches updateDataMask with value undefined for filter_select', async () => {
- const filterId = 'NATIVE_FILTER-clear-select';
- const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
- const selectFilter = createFilter({
- id: filterId,
- name: 'Region',
- filterType: 'filter_select',
- targets: [{ datasetId: 7, column: { name: 'region' } }],
- defaultDataMask: { filterState: { value: null }, extraFormData: {} },
- chartsInScope: [18],
- });
- const stateWithSelect = {
- ...stateWithoutNativeFilters,
- dashboardInfo: {
- id: 1,
- dash_edit_perm: true,
- filterBarOrientation: FilterBarOrientation.Vertical,
- metadata: {
- native_filter_configuration: [selectFilter],
- chart_configuration: {},
+ const stateWithIncompleteFilter = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
},
- },
- dashboardState: {
- ...stateWithoutNativeFilters.dashboardState,
- activeTabs: ['ROOT_ID'],
- },
- dataMask: {
- [filterId]: createDataMask(filterId, ['East']),
- },
- nativeFilters: {
- filters: { [filterId]: selectFilter },
- filtersState: {},
- },
- };
+ dataMask: {
+ [filterId]: {
+ id: filterId,
+ filterState: { value: ['value1', 'value2'] },
+ extraFormData: {},
+ },
+ },
+ nativeFilters: {
+ filters: {
+ [filterId]: {
+ id: filterId,
+ name: 'Test Filter',
+ filterType: 'filter_select',
+ targets: [{ datasetId: 1, column: { name: 'test_column' } }],
+ defaultDataMask: {
+ filterState: { value: ['value1', 'value2'] },
+ extraFormData: {},
+ },
+ controlValues: {
+ enableEmptyFilter: true,
+ },
+ cascadeParentIds: [],
+ scope: {
+ rootPath: ['ROOT_ID'],
+ excluded: [],
+ },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [],
+ tabsInScope: [],
+ },
+ },
+ filtersState: {},
+ },
+ };
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithSelect);
- await act(async () => {
- jest.advanceTimersByTime(300);
- });
+ renderWrapper(openedBarProps, stateWithIncompleteFilter);
- const clearBtn = screen.getByTestId(getTestId('clear-button'));
- expect(clearBtn).not.toBeDisabled();
- await act(async () => {
- userEvent.click(clearBtn);
- });
+ await act(async () => {
+ jest.advanceTimersByTime(200);
+ });
- expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, {
- filterState: { value: undefined },
- extraFormData: {},
- });
- updateDataMaskSpy.mockRestore();
-});
+ expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
+ // The test name promises that auto-apply ran; without this assertion the
+ // test passed even when the dispatch was silently skipped.
+ expect(updateDataMaskSpy).toHaveBeenCalled();
-test('handleClearAll dispatches updateDataMask with [null, null] for filter_range', async () => {
- fetchMock.post('glob:*/api/v1/chart/data', {
- result: [{ data: [{ min: 0, max: 100 }] }],
- });
- const filterId = 'NATIVE_FILTER-clear-range';
- const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
- const rangeFilter = createFilter({
- id: filterId,
- name: 'Age',
- filterType: 'filter_range',
- targets: [{ datasetId: 7, column: { name: 'age' } }],
- defaultDataMask: { filterState: { value: null }, extraFormData: {} },
- chartsInScope: [18],
+ updateDataMaskSpy.mockRestore();
});
- const stateWithRange = {
- ...stateWithoutNativeFilters,
- dashboardInfo: {
- id: 1,
- dash_edit_perm: true,
- filterBarOrientation: FilterBarOrientation.Vertical,
- metadata: {
- native_filter_configuration: [rangeFilter],
- chart_configuration: {},
+
+ test('renders correctly when filter has complete extraFormData', async () => {
+ const filterId = 'test-filter-complete';
+ const stateWithCompleteFilter = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
},
- },
- dashboardState: {
- ...stateWithoutNativeFilters.dashboardState,
- activeTabs: ['ROOT_ID'],
- },
- dataMask: {
- [filterId]: createDataMask(filterId, [10, 50]),
- },
- nativeFilters: {
- filters: { [filterId]: rangeFilter },
- filtersState: {},
- },
- };
+ dataMask: {
+ [filterId]: {
+ id: filterId,
+ filterState: { value: ['value1'] },
+ extraFormData: {
+ filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
+ },
+ },
+ },
+ nativeFilters: {
+ filters: {
+ [filterId]: {
+ id: filterId,
+ name: 'Test Filter',
+ filterType: 'filter_select',
+ targets: [{ datasetId: 1, column: { name: 'test_column' } }],
+ defaultDataMask: {
+ filterState: { value: ['value1'] },
+ extraFormData: {
+ filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
+ },
+ },
+ controlValues: {
+ enableEmptyFilter: true,
+ },
+ cascadeParentIds: [],
+ scope: {
+ rootPath: ['ROOT_ID'],
+ excluded: [],
+ },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [],
+ tabsInScope: [],
+ },
+ },
+ filtersState: {},
+ },
+ };
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithRange);
- await act(async () => {
- jest.advanceTimersByTime(300);
- });
+ renderWrapper(openedBarProps, stateWithCompleteFilter);
- const clearBtn = screen.getByTestId(getTestId('clear-button'));
- expect(clearBtn).not.toBeDisabled();
- await act(async () => {
- userEvent.click(clearBtn);
- });
+ await act(async () => {
+ jest.advanceTimersByTime(100);
+ });
- expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, {
- filterState: { value: [null, null] },
- extraFormData: {},
+ expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
});
- updateDataMaskSpy.mockRestore();
-});
-test('handleClearAll only dispatches for filters present in dataMask', async () => {
- const idInMask = 'NATIVE_FILTER-has-value';
- const idNotInMask = 'NATIVE_FILTER-no-value';
- const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
- const filterInMask = createFilter({
- id: idInMask,
- name: 'A',
- filterType: 'filter_select',
- targets: [{ datasetId: 7, column: { name: 'x' } }],
- chartsInScope: [18],
- });
- const filterNotInMask = createFilter({
- id: idNotInMask,
- name: 'B',
- filterType: 'filter_select',
- targets: [{ datasetId: 7, column: { name: 'x' } }],
- chartsInScope: [18],
- });
- const stateWithTwoFilters = {
- ...stateWithoutNativeFilters,
- dashboardInfo: {
- id: 1,
- dash_edit_perm: true,
- filterBarOrientation: FilterBarOrientation.Vertical,
- metadata: {
- native_filter_configuration: [filterInMask, filterNotInMask],
- chart_configuration: {},
+ test('handleClearAll dispatches updateDataMask with value null for filter_select', async () => {
+ const filterId = 'NATIVE_FILTER-clear-select';
+ const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
+ const selectFilterConfig = {
+ id: filterId,
+ name: 'Region',
+ filterType: 'filter_select',
+ targets: [{ datasetId: 7, column: { name: 'region' } }],
+ defaultDataMask: { filterState: { value: null }, extraFormData: {} },
+ cascadeParentIds: [],
+ scope: { rootPath: ['ROOT_ID'], excluded: [] },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [18],
+ tabsInScope: [],
+ };
+ const stateWithSelect = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
+ filterBarOrientation: FilterBarOrientation.Vertical,
+ metadata: {
+ native_filter_configuration: [selectFilterConfig],
+ chart_configuration: {},
+ },
},
- },
- dashboardState: {
- ...stateWithoutNativeFilters.dashboardState,
- activeTabs: ['ROOT_ID'],
- },
- dataMask: {
- [idInMask]: createDataMask(idInMask, ['v']),
- },
- nativeFilters: {
- filters: {
- [idInMask]: filterInMask,
- [idNotInMask]: filterNotInMask,
+ dataMask: {
+ [filterId]: {
+ id: filterId,
+ filterState: { value: ['East'] },
+ extraFormData: {},
+ },
},
- filtersState: {},
- },
- };
+ };
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithTwoFilters);
- await act(async () => {
- jest.advanceTimersByTime(300);
- });
+ renderWrapper(openedBarProps, stateWithSelect);
+ await act(async () => {
+ jest.advanceTimersByTime(300);
+ });
- const clearBtn = screen.getByTestId(getTestId('clear-button'));
- await act(async () => {
- userEvent.click(clearBtn);
- });
+ const clearBtn = screen.getByTestId(getTestId('clear-button'));
+ expect(clearBtn).not.toBeDisabled();
+ await act(async () => {
+ userEvent.click(clearBtn);
Review Comment:
Addressed in f71a5361 — added `await` before userEvent.click(clearBtn) in the filter_select clear-all test. Resolving — thanks for the catch.
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx:
##########
@@ -109,684 +139,550 @@ function createMockApi(filterName = 'Time filter 1') {
],
};
});
-}
-
-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,
- };
-}
-
-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],
+ 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' },
},
- ...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: getTimeRangeNoFilterMockUrl },
+ );
+
+ fetchMock.removeRoute(getTimeRangeLastDayMockUrl);
+ fetchMock.get(
+ getTimeRangeLastDayMockUrl,
+ {
+ result: {
+ since: '2021-04-13T00:00:00',
+ until: '2021-04-14T00:00:00',
+ timeRange: 'Last day',
+ },
},
- },
- { 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: 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.lastWeek },
- );
-}
-
-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,
- },
- );
-}
-
-test('FilterBar renders without crashing', () => {
- const props = createClosedBarProps();
- const { container } = renderFilterBar(props);
- expect(container).toBeInTheDocument();
-});
+ { name: getTimeRangeLastWeekMockUrl },
+ );
-test('FilterBar renders "Filters and controls" heading', () => {
- const props = createClosedBarProps();
- renderFilterBar(props);
- 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('FilterBar renders "Apply filters" button', () => {
- const props = createClosedBarProps();
- renderFilterBar(props);
- 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('FilterBar renders filter icon', () => {
- const props = createClosedBarProps();
- renderFilterBar(props);
- 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);
-
- const collapse = screen.getByRole('img', { name: 'vertical-align' });
- expect(toggleFiltersBar).not.toHaveBeenCalled();
-
- userEvent.click(collapse);
- expect(toggleFiltersBar).toHaveBeenCalled();
-});
-
-test('FilterBar opens when expand button is clicked', () => {
- const toggleFiltersBar = jest.fn();
- const props = createClosedBarProps(toggleFiltersBar);
- renderFilterBar(props);
-
- expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
- expect(screen.getByTestId(getTestId('expand-button'))).toBeInTheDocument();
-
- userEvent.click(screen.getByTestId(getTestId('collapsable')));
- expect(toggleFiltersBar).toHaveBeenCalledWith(true);
-});
-
-test('FilterBar hides edit filter button when user lacks permissions', () => {
- const props = createOpenedBarProps();
- const stateWithoutPermissions = {
- ...stateWithoutNativeFilters,
- dashboardInfo: { metadata: {} },
- };
+ mockedMakeApi.mockReturnValue(mockApi);
+ });
- renderFilterBar(props, stateWithoutPermissions);
+ 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,
+ },
+ );
- expect(
- screen.queryByTestId(getTestId('create-filter')),
- ).not.toBeInTheDocument();
-});
+ test('should render', () => {
+ const { container } = renderWrapper();
+ expect(container).toBeInTheDocument();
+ });
-test('FilterBar closes when collapse button is clicked', () => {
- const toggleFiltersBar = jest.fn();
- const props = createOpenedBarProps(toggleFiltersBar);
- renderFilterBar(props);
+ test('should render the "Filters and controls" heading', () => {
+ renderWrapper();
+ expect(screen.getByText('Filters and controls')).toBeInTheDocument();
+ });
- const collapseButton = screen.getByTestId(getTestId('collapse-button'));
- expect(collapseButton).toBeInTheDocument();
+ test('should render the "Clear all" option', () => {
+ renderWrapper();
+ expect(screen.getByText('Clear all')).toBeInTheDocument();
+ });
- userEvent.click(collapseButton);
- expect(toggleFiltersBar).toHaveBeenCalledWith(false);
-});
+ test('should render the "Apply filters" option', () => {
+ renderWrapper();
+ expect(screen.getByText('Apply filters')).toBeInTheDocument();
+ });
-test('FilterBar disables buttons when there are no filters', () => {
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithoutNativeFilters);
+ test('should render the collapse icon', () => {
+ renderWrapper();
+ expect(
+ screen.getByRole('img', { name: 'vertical-align' }),
+ ).toBeInTheDocument();
+ });
- expect(screen.getByTestId(getTestId('clear-button'))).toBeDisabled();
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
-});
+ test('should render the filter icon', () => {
+ renderWrapper();
+ expect(screen.getByRole('img', { name: 'filter' })).toBeInTheDocument();
+ });
-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 },
- },
- };
+ test('should toggle', () => {
+ renderWrapper();
+ const collapse = screen.getByRole('img', {
+ name: 'vertical-align',
+ });
+ expect(toggleFiltersBar).not.toHaveBeenCalled();
+ userEvent.click(collapse);
+ expect(toggleFiltersBar).toHaveBeenCalled();
+ });
- renderFilterBar(props, stateWithDivider);
+ test('open filter bar', () => {
+ renderWrapper();
+ expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
+ expect(screen.getByTestId(getTestId('expand-button'))).toBeInTheDocument();
- await act(async () => {
- jest.advanceTimersByTime(1000);
+ userEvent.click(screen.getByTestId(getTestId('collapsable')));
+ expect(toggleFiltersBar).toHaveBeenCalledWith(true);
});
- const title = await screen.findByText('Select time range');
- const description = await screen.findByText('Select year/month etc..');
+ test('collapsed filter bar should have aria-expanded=false', () => {
+ renderWrapper();
+ const collapsedBar = screen.getByTestId(getTestId('collapsable'));
+ expect(collapsedBar).toHaveAttribute('aria-expanded', 'false');
+ expect(collapsedBar).toHaveAttribute('aria-label', 'Expand filters');
+ });
- 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('no edit filter button by disabled permissions', () => {
+ renderWrapper(openedBarProps, {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: { metadata: {} },
+ });
-test('FilterBar apply button is disabled after creating a filter', async () => {
- setupTimeRangeMocks();
- mockedMakeApi.mockReturnValue(createMockApi());
+ expect(
+ screen.queryByTestId(getTestId('create-filter')),
+ ).not.toBeInTheDocument();
+ });
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithoutNativeFilters);
+ test('close filter bar', () => {
+ renderWrapper(openedBarProps);
+ const collapseButton = screen.getByTestId(getTestId('collapse-button'));
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
+ expect(collapseButton).toBeInTheDocument();
+ userEvent.click(collapseButton);
- // 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(toggleFiltersBar).toHaveBeenCalledWith(false);
+ });
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
-});
+ test('no filters', () => {
+ renderWrapper(openedBarProps, stateWithoutNativeFilters);
-test('FilterBar renders without errors when filter has required controlValues', () => {
- const props = createOpenedBarProps();
- const filter = createFilter({
- id: 'test-filter',
- controlValues: { enableEmptyFilter: true },
+ expect(screen.getByTestId(getTestId('clear-button'))).toBeDisabled();
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
});
- const dataMask = createDataMask('test-filter', undefined, {});
- const state = createStateWithFilter(filter, dataMask);
- const { container } = renderFilterBar(props, state);
- expect(container).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,
+ },
+ },
+ };
-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: {},
- },
- });
+ renderWrapper(openedBarProps, stateWithDivider);
- const dataMask = createDataMask(filterId, ['value1'], {});
- const state = createStateWithFilter(filter, dataMask);
+ await act(async () => {
+ jest.advanceTimersByTime(1000); // 1s
+ });
- renderFilterBar(props, state);
+ const title = await screen.findByText('Select time range');
+ const description = await screen.findByText('Select year/month etc..');
- await act(async () => {
- jest.advanceTimersByTime(300);
+ 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();
});
- expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
- expect(screen.getByText('Filters and controls')).toBeInTheDocument();
+ test('create filter and apply it flow', async () => {
+ renderWrapper(openedBarProps, stateWithoutNativeFilters);
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
- // The filter value should not be cleared during initialization
- expect(updateDataMaskSpy).not.toHaveBeenCalled();
+ await addFilterFlow();
- updateDataMaskSpy.mockRestore();
-});
-
-test('FilterBar renders correctly when filter has complete extraFormData', async () => {
- const filterId = 'test-filter-complete';
- const props = createOpenedBarProps();
- const filter = createFilter({
- id: filterId,
- controlValues: { enableEmptyFilter: true },
- defaultDataMask: {
- filterState: { value: ['value1'] },
- extraFormData: {
- filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
- },
- },
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
});
- const dataMask = createDataMask(filterId, ['value1'], {
- filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
- });
- const state = createStateWithFilter(filter, dataMask);
- renderFilterBar(props, state);
+ 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: {},
+ },
+ };
- await act(async () => {
- jest.advanceTimersByTime(100);
+ const { container } = renderWrapper(openedBarProps, stateWithFilter);
+ expect(container).toBeInTheDocument();
});
- expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
-});
+ test('auto-applies filter when extraFormData is empty in applied state', async () => {
+ const filterId = 'test-filter-auto-apply';
+ const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
-test('handleClearAll dispatches updateDataMask with value undefined for filter_select', async () => {
- const filterId = 'NATIVE_FILTER-clear-select';
- const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
- const selectFilter = createFilter({
- id: filterId,
- name: 'Region',
- filterType: 'filter_select',
- targets: [{ datasetId: 7, column: { name: 'region' } }],
- defaultDataMask: { filterState: { value: null }, extraFormData: {} },
- chartsInScope: [18],
- });
- const stateWithSelect = {
- ...stateWithoutNativeFilters,
- dashboardInfo: {
- id: 1,
- dash_edit_perm: true,
- filterBarOrientation: FilterBarOrientation.Vertical,
- metadata: {
- native_filter_configuration: [selectFilter],
- chart_configuration: {},
+ const stateWithIncompleteFilter = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
},
- },
- dashboardState: {
- ...stateWithoutNativeFilters.dashboardState,
- activeTabs: ['ROOT_ID'],
- },
- dataMask: {
- [filterId]: createDataMask(filterId, ['East']),
- },
- nativeFilters: {
- filters: { [filterId]: selectFilter },
- filtersState: {},
- },
- };
+ dataMask: {
+ [filterId]: {
+ id: filterId,
+ filterState: { value: ['value1', 'value2'] },
+ extraFormData: {},
+ },
+ },
+ nativeFilters: {
+ filters: {
+ [filterId]: {
+ id: filterId,
+ name: 'Test Filter',
+ filterType: 'filter_select',
+ targets: [{ datasetId: 1, column: { name: 'test_column' } }],
+ defaultDataMask: {
+ filterState: { value: ['value1', 'value2'] },
+ extraFormData: {},
+ },
+ controlValues: {
+ enableEmptyFilter: true,
+ },
+ cascadeParentIds: [],
+ scope: {
+ rootPath: ['ROOT_ID'],
+ excluded: [],
+ },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [],
+ tabsInScope: [],
+ },
+ },
+ filtersState: {},
+ },
+ };
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithSelect);
- await act(async () => {
- jest.advanceTimersByTime(300);
- });
+ renderWrapper(openedBarProps, stateWithIncompleteFilter);
- const clearBtn = screen.getByTestId(getTestId('clear-button'));
- expect(clearBtn).not.toBeDisabled();
- await act(async () => {
- userEvent.click(clearBtn);
- });
+ await act(async () => {
+ jest.advanceTimersByTime(200);
+ });
- expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, {
- filterState: { value: undefined },
- extraFormData: {},
- });
- updateDataMaskSpy.mockRestore();
-});
+ expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
+ // The test name promises that auto-apply ran; without this assertion the
+ // test passed even when the dispatch was silently skipped.
+ expect(updateDataMaskSpy).toHaveBeenCalled();
-test('handleClearAll dispatches updateDataMask with [null, null] for filter_range', async () => {
- fetchMock.post('glob:*/api/v1/chart/data', {
- result: [{ data: [{ min: 0, max: 100 }] }],
- });
- const filterId = 'NATIVE_FILTER-clear-range';
- const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
- const rangeFilter = createFilter({
- id: filterId,
- name: 'Age',
- filterType: 'filter_range',
- targets: [{ datasetId: 7, column: { name: 'age' } }],
- defaultDataMask: { filterState: { value: null }, extraFormData: {} },
- chartsInScope: [18],
+ updateDataMaskSpy.mockRestore();
});
- const stateWithRange = {
- ...stateWithoutNativeFilters,
- dashboardInfo: {
- id: 1,
- dash_edit_perm: true,
- filterBarOrientation: FilterBarOrientation.Vertical,
- metadata: {
- native_filter_configuration: [rangeFilter],
- chart_configuration: {},
+
+ test('renders correctly when filter has complete extraFormData', async () => {
+ const filterId = 'test-filter-complete';
+ const stateWithCompleteFilter = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
},
- },
- dashboardState: {
- ...stateWithoutNativeFilters.dashboardState,
- activeTabs: ['ROOT_ID'],
- },
- dataMask: {
- [filterId]: createDataMask(filterId, [10, 50]),
- },
- nativeFilters: {
- filters: { [filterId]: rangeFilter },
- filtersState: {},
- },
- };
+ dataMask: {
+ [filterId]: {
+ id: filterId,
+ filterState: { value: ['value1'] },
+ extraFormData: {
+ filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
+ },
+ },
+ },
+ nativeFilters: {
+ filters: {
+ [filterId]: {
+ id: filterId,
+ name: 'Test Filter',
+ filterType: 'filter_select',
+ targets: [{ datasetId: 1, column: { name: 'test_column' } }],
+ defaultDataMask: {
+ filterState: { value: ['value1'] },
+ extraFormData: {
+ filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
+ },
+ },
+ controlValues: {
+ enableEmptyFilter: true,
+ },
+ cascadeParentIds: [],
+ scope: {
+ rootPath: ['ROOT_ID'],
+ excluded: [],
+ },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [],
+ tabsInScope: [],
+ },
+ },
+ filtersState: {},
+ },
+ };
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithRange);
- await act(async () => {
- jest.advanceTimersByTime(300);
- });
+ renderWrapper(openedBarProps, stateWithCompleteFilter);
- const clearBtn = screen.getByTestId(getTestId('clear-button'));
- expect(clearBtn).not.toBeDisabled();
- await act(async () => {
- userEvent.click(clearBtn);
- });
+ await act(async () => {
+ jest.advanceTimersByTime(100);
+ });
- expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, {
- filterState: { value: [null, null] },
- extraFormData: {},
+ expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
});
- updateDataMaskSpy.mockRestore();
-});
-test('handleClearAll only dispatches for filters present in dataMask', async () => {
- const idInMask = 'NATIVE_FILTER-has-value';
- const idNotInMask = 'NATIVE_FILTER-no-value';
- const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
- const filterInMask = createFilter({
- id: idInMask,
- name: 'A',
- filterType: 'filter_select',
- targets: [{ datasetId: 7, column: { name: 'x' } }],
- chartsInScope: [18],
- });
- const filterNotInMask = createFilter({
- id: idNotInMask,
- name: 'B',
- filterType: 'filter_select',
- targets: [{ datasetId: 7, column: { name: 'x' } }],
- chartsInScope: [18],
- });
- const stateWithTwoFilters = {
- ...stateWithoutNativeFilters,
- dashboardInfo: {
- id: 1,
- dash_edit_perm: true,
- filterBarOrientation: FilterBarOrientation.Vertical,
- metadata: {
- native_filter_configuration: [filterInMask, filterNotInMask],
- chart_configuration: {},
+ test('handleClearAll dispatches updateDataMask with value null for filter_select', async () => {
+ const filterId = 'NATIVE_FILTER-clear-select';
+ const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
+ const selectFilterConfig = {
+ id: filterId,
+ name: 'Region',
+ filterType: 'filter_select',
+ targets: [{ datasetId: 7, column: { name: 'region' } }],
+ defaultDataMask: { filterState: { value: null }, extraFormData: {} },
+ cascadeParentIds: [],
+ scope: { rootPath: ['ROOT_ID'], excluded: [] },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [18],
+ tabsInScope: [],
+ };
+ const stateWithSelect = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
+ filterBarOrientation: FilterBarOrientation.Vertical,
+ metadata: {
+ native_filter_configuration: [selectFilterConfig],
+ chart_configuration: {},
+ },
},
- },
- dashboardState: {
- ...stateWithoutNativeFilters.dashboardState,
- activeTabs: ['ROOT_ID'],
- },
- dataMask: {
- [idInMask]: createDataMask(idInMask, ['v']),
- },
- nativeFilters: {
- filters: {
- [idInMask]: filterInMask,
- [idNotInMask]: filterNotInMask,
+ dataMask: {
+ [filterId]: {
+ id: filterId,
+ filterState: { value: ['East'] },
+ extraFormData: {},
+ },
},
- filtersState: {},
- },
- };
+ };
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithTwoFilters);
- await act(async () => {
- jest.advanceTimersByTime(300);
- });
+ renderWrapper(openedBarProps, stateWithSelect);
+ await act(async () => {
+ jest.advanceTimersByTime(300);
+ });
- const clearBtn = screen.getByTestId(getTestId('clear-button'));
- await act(async () => {
- userEvent.click(clearBtn);
- });
+ const clearBtn = screen.getByTestId(getTestId('clear-button'));
+ expect(clearBtn).not.toBeDisabled();
+ await act(async () => {
+ userEvent.click(clearBtn);
+ });
- expect(updateDataMaskSpy).toHaveBeenCalledTimes(1);
- expect(updateDataMaskSpy).toHaveBeenCalledWith(idInMask, {
- filterState: { value: undefined },
- extraFormData: {},
+ expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, {
+ filterState: { value: undefined },
+ extraFormData: {},
+ });
+ updateDataMaskSpy.mockRestore();
});
- updateDataMaskSpy.mockRestore();
-});
-test('FilterBar Clear All only clears in-scope filters, not out-of-scope ones', async () => {
- const inScopeFilterId = 'NATIVE_FILTER-in-scope';
- const outOfScopeRequiredFilterId = 'NATIVE_FILTER-out-of-scope-required';
- const outOfScopeNonRequiredFilterId =
- 'NATIVE_FILTER-out-of-scope-non-required';
- const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
-
- const dashboardLayoutWithTabs = {
- ROOT_ID: { id: 'ROOT_ID', type: 'ROOT', children: ['TABS-1'] },
- 'TABS-1': {
- id: 'TABS-1',
- type: 'TABS',
- children: ['TAB-active', 'TAB-inactive'],
- },
- 'TAB-active': {
- id: 'TAB-active',
- type: 'TAB',
- children: ['CHART_ROW-1'],
- meta: { text: 'Active Tab' },
- parents: ['ROOT_ID', 'TABS-1'],
- },
- 'TAB-inactive': {
- id: 'TAB-inactive',
- type: 'TAB',
- children: ['CHART_ROW-2'],
- meta: { text: 'Inactive Tab' },
- parents: ['ROOT_ID', 'TABS-1'],
- },
- 'CHART_ROW-1': {
- id: 'CHART_ROW-1',
- type: 'CHART',
- meta: { chartId: 1 },
- parents: ['ROOT_ID', 'TABS-1', 'TAB-active'],
- },
- 'CHART_ROW-2': {
- id: 'CHART_ROW-2',
- type: 'CHART',
- meta: { chartId: 2 },
- parents: ['ROOT_ID', 'TABS-1', 'TAB-inactive'],
- },
- };
+ test('handleClearAll dispatches updateDataMask with value [null, null] for filter_range', async () => {
+ fetchMock.post('glob:*/api/v1/chart/data', {
+ result: [{ data: [{ min: 0, max: 100 }] }],
+ });
+ const filterId = 'NATIVE_FILTER-clear-range';
+ const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
+ const rangeFilterConfig = {
+ id: filterId,
+ name: 'Age',
+ filterType: 'filter_range',
+ targets: [{ datasetId: 7, column: { name: 'age' } }],
+ defaultDataMask: { filterState: { value: null }, extraFormData: {} },
+ cascadeParentIds: [],
+ scope: { rootPath: ['ROOT_ID'], excluded: [] },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [18],
+ tabsInScope: [],
+ };
+ const stateWithRange = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
+ filterBarOrientation: FilterBarOrientation.Vertical,
+ metadata: {
+ native_filter_configuration: [rangeFilterConfig],
+ chart_configuration: {},
+ },
+ },
+ dataMask: {
+ [filterId]: {
+ id: filterId,
+ filterState: { value: [10, 50] },
+ extraFormData: {},
+ },
+ },
+ };
- const inScopeFilter = createFilter({
- id: inScopeFilterId,
- name: 'In Scope Filter',
- targets: [{ datasetId: 1, column: { name: 'column1' } }],
- controlValues: { enableEmptyFilter: false },
- chartsInScope: [1],
- tabsInScope: ['TAB-active'],
- });
+ renderWrapper(openedBarProps, stateWithRange);
+ await act(async () => {
+ jest.advanceTimersByTime(300);
+ });
- const outOfScopeRequiredFilter = createFilter({
- id: outOfScopeRequiredFilterId,
- name: 'Out of Scope Required Filter',
- targets: [{ datasetId: 1, column: { name: 'column2' } }],
- controlValues: { enableEmptyFilter: true },
- chartsInScope: [2],
- tabsInScope: ['TAB-inactive'],
- });
+ const clearBtn = screen.getByTestId(getTestId('clear-button'));
+ expect(clearBtn).not.toBeDisabled();
+ await act(async () => {
+ userEvent.click(clearBtn);
Review Comment:
Addressed in f71a5361 — added `await` before userEvent.click(clearBtn) in the filter_range clear-all test. Resolving — thanks for the catch.
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx:
##########
@@ -109,684 +139,550 @@ function createMockApi(filterName = 'Time filter 1') {
],
};
});
-}
-
-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,
- };
-}
-
-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],
+ 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' },
},
- ...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: getTimeRangeNoFilterMockUrl },
+ );
+
+ fetchMock.removeRoute(getTimeRangeLastDayMockUrl);
+ fetchMock.get(
+ getTimeRangeLastDayMockUrl,
+ {
+ result: {
+ since: '2021-04-13T00:00:00',
+ until: '2021-04-14T00:00:00',
+ timeRange: 'Last day',
+ },
},
- },
- { 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: 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.lastWeek },
- );
-}
-
-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,
- },
- );
-}
-
-test('FilterBar renders without crashing', () => {
- const props = createClosedBarProps();
- const { container } = renderFilterBar(props);
- expect(container).toBeInTheDocument();
-});
+ { name: getTimeRangeLastWeekMockUrl },
+ );
-test('FilterBar renders "Filters and controls" heading', () => {
- const props = createClosedBarProps();
- renderFilterBar(props);
- 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('FilterBar renders "Apply filters" button', () => {
- const props = createClosedBarProps();
- renderFilterBar(props);
- 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('FilterBar renders filter icon', () => {
- const props = createClosedBarProps();
- renderFilterBar(props);
- 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);
-
- const collapse = screen.getByRole('img', { name: 'vertical-align' });
- expect(toggleFiltersBar).not.toHaveBeenCalled();
-
- userEvent.click(collapse);
- expect(toggleFiltersBar).toHaveBeenCalled();
-});
-
-test('FilterBar opens when expand button is clicked', () => {
- const toggleFiltersBar = jest.fn();
- const props = createClosedBarProps(toggleFiltersBar);
- renderFilterBar(props);
-
- expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
- expect(screen.getByTestId(getTestId('expand-button'))).toBeInTheDocument();
-
- userEvent.click(screen.getByTestId(getTestId('collapsable')));
- expect(toggleFiltersBar).toHaveBeenCalledWith(true);
-});
-
-test('FilterBar hides edit filter button when user lacks permissions', () => {
- const props = createOpenedBarProps();
- const stateWithoutPermissions = {
- ...stateWithoutNativeFilters,
- dashboardInfo: { metadata: {} },
- };
+ mockedMakeApi.mockReturnValue(mockApi);
+ });
- renderFilterBar(props, stateWithoutPermissions);
+ 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,
+ },
+ );
- expect(
- screen.queryByTestId(getTestId('create-filter')),
- ).not.toBeInTheDocument();
-});
+ test('should render', () => {
+ const { container } = renderWrapper();
+ expect(container).toBeInTheDocument();
+ });
-test('FilterBar closes when collapse button is clicked', () => {
- const toggleFiltersBar = jest.fn();
- const props = createOpenedBarProps(toggleFiltersBar);
- renderFilterBar(props);
+ test('should render the "Filters and controls" heading', () => {
+ renderWrapper();
+ expect(screen.getByText('Filters and controls')).toBeInTheDocument();
+ });
- const collapseButton = screen.getByTestId(getTestId('collapse-button'));
- expect(collapseButton).toBeInTheDocument();
+ test('should render the "Clear all" option', () => {
+ renderWrapper();
+ expect(screen.getByText('Clear all')).toBeInTheDocument();
+ });
- userEvent.click(collapseButton);
- expect(toggleFiltersBar).toHaveBeenCalledWith(false);
-});
+ test('should render the "Apply filters" option', () => {
+ renderWrapper();
+ expect(screen.getByText('Apply filters')).toBeInTheDocument();
+ });
-test('FilterBar disables buttons when there are no filters', () => {
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithoutNativeFilters);
+ test('should render the collapse icon', () => {
+ renderWrapper();
+ expect(
+ screen.getByRole('img', { name: 'vertical-align' }),
+ ).toBeInTheDocument();
+ });
- expect(screen.getByTestId(getTestId('clear-button'))).toBeDisabled();
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
-});
+ test('should render the filter icon', () => {
+ renderWrapper();
+ expect(screen.getByRole('img', { name: 'filter' })).toBeInTheDocument();
+ });
-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 },
- },
- };
+ test('should toggle', () => {
+ renderWrapper();
+ const collapse = screen.getByRole('img', {
+ name: 'vertical-align',
+ });
+ expect(toggleFiltersBar).not.toHaveBeenCalled();
+ userEvent.click(collapse);
+ expect(toggleFiltersBar).toHaveBeenCalled();
+ });
- renderFilterBar(props, stateWithDivider);
+ test('open filter bar', () => {
+ renderWrapper();
+ expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
+ expect(screen.getByTestId(getTestId('expand-button'))).toBeInTheDocument();
- await act(async () => {
- jest.advanceTimersByTime(1000);
+ userEvent.click(screen.getByTestId(getTestId('collapsable')));
+ expect(toggleFiltersBar).toHaveBeenCalledWith(true);
});
- const title = await screen.findByText('Select time range');
- const description = await screen.findByText('Select year/month etc..');
+ test('collapsed filter bar should have aria-expanded=false', () => {
+ renderWrapper();
+ const collapsedBar = screen.getByTestId(getTestId('collapsable'));
+ expect(collapsedBar).toHaveAttribute('aria-expanded', 'false');
+ expect(collapsedBar).toHaveAttribute('aria-label', 'Expand filters');
+ });
- 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('no edit filter button by disabled permissions', () => {
+ renderWrapper(openedBarProps, {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: { metadata: {} },
+ });
-test('FilterBar apply button is disabled after creating a filter', async () => {
- setupTimeRangeMocks();
- mockedMakeApi.mockReturnValue(createMockApi());
+ expect(
+ screen.queryByTestId(getTestId('create-filter')),
+ ).not.toBeInTheDocument();
+ });
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithoutNativeFilters);
+ test('close filter bar', () => {
+ renderWrapper(openedBarProps);
+ const collapseButton = screen.getByTestId(getTestId('collapse-button'));
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
+ expect(collapseButton).toBeInTheDocument();
+ userEvent.click(collapseButton);
- // 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(toggleFiltersBar).toHaveBeenCalledWith(false);
+ });
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
-});
+ test('no filters', () => {
+ renderWrapper(openedBarProps, stateWithoutNativeFilters);
-test('FilterBar renders without errors when filter has required controlValues', () => {
- const props = createOpenedBarProps();
- const filter = createFilter({
- id: 'test-filter',
- controlValues: { enableEmptyFilter: true },
+ expect(screen.getByTestId(getTestId('clear-button'))).toBeDisabled();
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
});
- const dataMask = createDataMask('test-filter', undefined, {});
- const state = createStateWithFilter(filter, dataMask);
- const { container } = renderFilterBar(props, state);
- expect(container).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,
+ },
+ },
+ };
-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: {},
- },
- });
+ renderWrapper(openedBarProps, stateWithDivider);
- const dataMask = createDataMask(filterId, ['value1'], {});
- const state = createStateWithFilter(filter, dataMask);
+ await act(async () => {
+ jest.advanceTimersByTime(1000); // 1s
+ });
- renderFilterBar(props, state);
+ const title = await screen.findByText('Select time range');
+ const description = await screen.findByText('Select year/month etc..');
- await act(async () => {
- jest.advanceTimersByTime(300);
+ 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();
});
- expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
- expect(screen.getByText('Filters and controls')).toBeInTheDocument();
+ test('create filter and apply it flow', async () => {
+ renderWrapper(openedBarProps, stateWithoutNativeFilters);
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
- // The filter value should not be cleared during initialization
- expect(updateDataMaskSpy).not.toHaveBeenCalled();
+ await addFilterFlow();
- updateDataMaskSpy.mockRestore();
-});
-
-test('FilterBar renders correctly when filter has complete extraFormData', async () => {
- const filterId = 'test-filter-complete';
- const props = createOpenedBarProps();
- const filter = createFilter({
- id: filterId,
- controlValues: { enableEmptyFilter: true },
- defaultDataMask: {
- filterState: { value: ['value1'] },
- extraFormData: {
- filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
- },
- },
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
});
- const dataMask = createDataMask(filterId, ['value1'], {
- filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
- });
- const state = createStateWithFilter(filter, dataMask);
- renderFilterBar(props, state);
+ 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: {},
+ },
+ };
- await act(async () => {
- jest.advanceTimersByTime(100);
+ const { container } = renderWrapper(openedBarProps, stateWithFilter);
+ expect(container).toBeInTheDocument();
});
- expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
-});
+ test('auto-applies filter when extraFormData is empty in applied state', async () => {
+ const filterId = 'test-filter-auto-apply';
+ const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
-test('handleClearAll dispatches updateDataMask with value undefined for filter_select', async () => {
- const filterId = 'NATIVE_FILTER-clear-select';
- const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
- const selectFilter = createFilter({
- id: filterId,
- name: 'Region',
- filterType: 'filter_select',
- targets: [{ datasetId: 7, column: { name: 'region' } }],
- defaultDataMask: { filterState: { value: null }, extraFormData: {} },
- chartsInScope: [18],
- });
- const stateWithSelect = {
- ...stateWithoutNativeFilters,
- dashboardInfo: {
- id: 1,
- dash_edit_perm: true,
- filterBarOrientation: FilterBarOrientation.Vertical,
- metadata: {
- native_filter_configuration: [selectFilter],
- chart_configuration: {},
+ const stateWithIncompleteFilter = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
},
- },
- dashboardState: {
- ...stateWithoutNativeFilters.dashboardState,
- activeTabs: ['ROOT_ID'],
- },
- dataMask: {
- [filterId]: createDataMask(filterId, ['East']),
- },
- nativeFilters: {
- filters: { [filterId]: selectFilter },
- filtersState: {},
- },
- };
+ dataMask: {
+ [filterId]: {
+ id: filterId,
+ filterState: { value: ['value1', 'value2'] },
+ extraFormData: {},
+ },
+ },
+ nativeFilters: {
+ filters: {
+ [filterId]: {
+ id: filterId,
+ name: 'Test Filter',
+ filterType: 'filter_select',
+ targets: [{ datasetId: 1, column: { name: 'test_column' } }],
+ defaultDataMask: {
+ filterState: { value: ['value1', 'value2'] },
+ extraFormData: {},
+ },
+ controlValues: {
+ enableEmptyFilter: true,
+ },
+ cascadeParentIds: [],
+ scope: {
+ rootPath: ['ROOT_ID'],
+ excluded: [],
+ },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [],
+ tabsInScope: [],
+ },
+ },
+ filtersState: {},
+ },
+ };
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithSelect);
- await act(async () => {
- jest.advanceTimersByTime(300);
- });
+ renderWrapper(openedBarProps, stateWithIncompleteFilter);
- const clearBtn = screen.getByTestId(getTestId('clear-button'));
- expect(clearBtn).not.toBeDisabled();
- await act(async () => {
- userEvent.click(clearBtn);
- });
+ await act(async () => {
+ jest.advanceTimersByTime(200);
+ });
- expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, {
- filterState: { value: undefined },
- extraFormData: {},
- });
- updateDataMaskSpy.mockRestore();
-});
+ expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
+ // The test name promises that auto-apply ran; without this assertion the
+ // test passed even when the dispatch was silently skipped.
+ expect(updateDataMaskSpy).toHaveBeenCalled();
-test('handleClearAll dispatches updateDataMask with [null, null] for filter_range', async () => {
- fetchMock.post('glob:*/api/v1/chart/data', {
- result: [{ data: [{ min: 0, max: 100 }] }],
- });
- const filterId = 'NATIVE_FILTER-clear-range';
- const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
- const rangeFilter = createFilter({
- id: filterId,
- name: 'Age',
- filterType: 'filter_range',
- targets: [{ datasetId: 7, column: { name: 'age' } }],
- defaultDataMask: { filterState: { value: null }, extraFormData: {} },
- chartsInScope: [18],
+ updateDataMaskSpy.mockRestore();
});
- const stateWithRange = {
- ...stateWithoutNativeFilters,
- dashboardInfo: {
- id: 1,
- dash_edit_perm: true,
- filterBarOrientation: FilterBarOrientation.Vertical,
- metadata: {
- native_filter_configuration: [rangeFilter],
- chart_configuration: {},
+
+ test('renders correctly when filter has complete extraFormData', async () => {
+ const filterId = 'test-filter-complete';
+ const stateWithCompleteFilter = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
},
- },
- dashboardState: {
- ...stateWithoutNativeFilters.dashboardState,
- activeTabs: ['ROOT_ID'],
- },
- dataMask: {
- [filterId]: createDataMask(filterId, [10, 50]),
- },
- nativeFilters: {
- filters: { [filterId]: rangeFilter },
- filtersState: {},
- },
- };
+ dataMask: {
+ [filterId]: {
+ id: filterId,
+ filterState: { value: ['value1'] },
+ extraFormData: {
+ filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
+ },
+ },
+ },
+ nativeFilters: {
+ filters: {
+ [filterId]: {
+ id: filterId,
+ name: 'Test Filter',
+ filterType: 'filter_select',
+ targets: [{ datasetId: 1, column: { name: 'test_column' } }],
+ defaultDataMask: {
+ filterState: { value: ['value1'] },
+ extraFormData: {
+ filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
+ },
+ },
+ controlValues: {
+ enableEmptyFilter: true,
+ },
+ cascadeParentIds: [],
+ scope: {
+ rootPath: ['ROOT_ID'],
+ excluded: [],
+ },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [],
+ tabsInScope: [],
+ },
+ },
+ filtersState: {},
+ },
+ };
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithRange);
- await act(async () => {
- jest.advanceTimersByTime(300);
- });
+ renderWrapper(openedBarProps, stateWithCompleteFilter);
- const clearBtn = screen.getByTestId(getTestId('clear-button'));
- expect(clearBtn).not.toBeDisabled();
- await act(async () => {
- userEvent.click(clearBtn);
- });
+ await act(async () => {
+ jest.advanceTimersByTime(100);
+ });
- expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, {
- filterState: { value: [null, null] },
- extraFormData: {},
+ expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
});
- updateDataMaskSpy.mockRestore();
-});
-test('handleClearAll only dispatches for filters present in dataMask', async () => {
- const idInMask = 'NATIVE_FILTER-has-value';
- const idNotInMask = 'NATIVE_FILTER-no-value';
- const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
- const filterInMask = createFilter({
- id: idInMask,
- name: 'A',
- filterType: 'filter_select',
- targets: [{ datasetId: 7, column: { name: 'x' } }],
- chartsInScope: [18],
- });
- const filterNotInMask = createFilter({
- id: idNotInMask,
- name: 'B',
- filterType: 'filter_select',
- targets: [{ datasetId: 7, column: { name: 'x' } }],
- chartsInScope: [18],
- });
- const stateWithTwoFilters = {
- ...stateWithoutNativeFilters,
- dashboardInfo: {
- id: 1,
- dash_edit_perm: true,
- filterBarOrientation: FilterBarOrientation.Vertical,
- metadata: {
- native_filter_configuration: [filterInMask, filterNotInMask],
- chart_configuration: {},
+ test('handleClearAll dispatches updateDataMask with value null for filter_select', async () => {
+ const filterId = 'NATIVE_FILTER-clear-select';
+ const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
+ const selectFilterConfig = {
+ id: filterId,
+ name: 'Region',
+ filterType: 'filter_select',
+ targets: [{ datasetId: 7, column: { name: 'region' } }],
+ defaultDataMask: { filterState: { value: null }, extraFormData: {} },
+ cascadeParentIds: [],
+ scope: { rootPath: ['ROOT_ID'], excluded: [] },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [18],
+ tabsInScope: [],
+ };
+ const stateWithSelect = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
+ filterBarOrientation: FilterBarOrientation.Vertical,
+ metadata: {
+ native_filter_configuration: [selectFilterConfig],
+ chart_configuration: {},
+ },
},
- },
- dashboardState: {
- ...stateWithoutNativeFilters.dashboardState,
- activeTabs: ['ROOT_ID'],
- },
- dataMask: {
- [idInMask]: createDataMask(idInMask, ['v']),
- },
- nativeFilters: {
- filters: {
- [idInMask]: filterInMask,
- [idNotInMask]: filterNotInMask,
+ dataMask: {
+ [filterId]: {
+ id: filterId,
+ filterState: { value: ['East'] },
+ extraFormData: {},
+ },
},
- filtersState: {},
- },
- };
+ };
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithTwoFilters);
- await act(async () => {
- jest.advanceTimersByTime(300);
- });
+ renderWrapper(openedBarProps, stateWithSelect);
+ await act(async () => {
+ jest.advanceTimersByTime(300);
+ });
- const clearBtn = screen.getByTestId(getTestId('clear-button'));
- await act(async () => {
- userEvent.click(clearBtn);
- });
+ const clearBtn = screen.getByTestId(getTestId('clear-button'));
+ expect(clearBtn).not.toBeDisabled();
+ await act(async () => {
+ userEvent.click(clearBtn);
+ });
- expect(updateDataMaskSpy).toHaveBeenCalledTimes(1);
- expect(updateDataMaskSpy).toHaveBeenCalledWith(idInMask, {
- filterState: { value: undefined },
- extraFormData: {},
+ expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, {
+ filterState: { value: undefined },
+ extraFormData: {},
+ });
+ updateDataMaskSpy.mockRestore();
});
- updateDataMaskSpy.mockRestore();
-});
-test('FilterBar Clear All only clears in-scope filters, not out-of-scope ones', async () => {
- const inScopeFilterId = 'NATIVE_FILTER-in-scope';
- const outOfScopeRequiredFilterId = 'NATIVE_FILTER-out-of-scope-required';
- const outOfScopeNonRequiredFilterId =
- 'NATIVE_FILTER-out-of-scope-non-required';
- const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
-
- const dashboardLayoutWithTabs = {
- ROOT_ID: { id: 'ROOT_ID', type: 'ROOT', children: ['TABS-1'] },
- 'TABS-1': {
- id: 'TABS-1',
- type: 'TABS',
- children: ['TAB-active', 'TAB-inactive'],
- },
- 'TAB-active': {
- id: 'TAB-active',
- type: 'TAB',
- children: ['CHART_ROW-1'],
- meta: { text: 'Active Tab' },
- parents: ['ROOT_ID', 'TABS-1'],
- },
- 'TAB-inactive': {
- id: 'TAB-inactive',
- type: 'TAB',
- children: ['CHART_ROW-2'],
- meta: { text: 'Inactive Tab' },
- parents: ['ROOT_ID', 'TABS-1'],
- },
- 'CHART_ROW-1': {
- id: 'CHART_ROW-1',
- type: 'CHART',
- meta: { chartId: 1 },
- parents: ['ROOT_ID', 'TABS-1', 'TAB-active'],
- },
- 'CHART_ROW-2': {
- id: 'CHART_ROW-2',
- type: 'CHART',
- meta: { chartId: 2 },
- parents: ['ROOT_ID', 'TABS-1', 'TAB-inactive'],
- },
- };
+ test('handleClearAll dispatches updateDataMask with value [null, null] for filter_range', async () => {
+ fetchMock.post('glob:*/api/v1/chart/data', {
+ result: [{ data: [{ min: 0, max: 100 }] }],
+ });
+ const filterId = 'NATIVE_FILTER-clear-range';
+ const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
+ const rangeFilterConfig = {
+ id: filterId,
+ name: 'Age',
+ filterType: 'filter_range',
+ targets: [{ datasetId: 7, column: { name: 'age' } }],
+ defaultDataMask: { filterState: { value: null }, extraFormData: {} },
+ cascadeParentIds: [],
+ scope: { rootPath: ['ROOT_ID'], excluded: [] },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [18],
+ tabsInScope: [],
+ };
+ const stateWithRange = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
+ filterBarOrientation: FilterBarOrientation.Vertical,
+ metadata: {
+ native_filter_configuration: [rangeFilterConfig],
+ chart_configuration: {},
+ },
+ },
+ dataMask: {
+ [filterId]: {
+ id: filterId,
+ filterState: { value: [10, 50] },
+ extraFormData: {},
+ },
+ },
+ };
- const inScopeFilter = createFilter({
- id: inScopeFilterId,
- name: 'In Scope Filter',
- targets: [{ datasetId: 1, column: { name: 'column1' } }],
- controlValues: { enableEmptyFilter: false },
- chartsInScope: [1],
- tabsInScope: ['TAB-active'],
- });
+ renderWrapper(openedBarProps, stateWithRange);
+ await act(async () => {
+ jest.advanceTimersByTime(300);
+ });
- const outOfScopeRequiredFilter = createFilter({
- id: outOfScopeRequiredFilterId,
- name: 'Out of Scope Required Filter',
- targets: [{ datasetId: 1, column: { name: 'column2' } }],
- controlValues: { enableEmptyFilter: true },
- chartsInScope: [2],
- tabsInScope: ['TAB-inactive'],
- });
+ const clearBtn = screen.getByTestId(getTestId('clear-button'));
+ expect(clearBtn).not.toBeDisabled();
+ await act(async () => {
+ userEvent.click(clearBtn);
+ });
- const outOfScopeNonRequiredFilter = createFilter({
- id: outOfScopeNonRequiredFilterId,
- name: 'Out of Scope Non-Required Filter',
- targets: [{ datasetId: 1, column: { name: 'column3' } }],
- controlValues: { enableEmptyFilter: false },
- chartsInScope: [2],
- tabsInScope: ['TAB-inactive'],
+ expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, {
+ filterState: { value: [null, null] },
+ extraFormData: {},
+ });
+ updateDataMaskSpy.mockRestore();
});
- const stateWithTabsAndFilters = {
- ...stateWithoutNativeFilters,
- dashboardLayout: {
- present: dashboardLayoutWithTabs,
- past: [],
- future: [],
- },
- dashboardState: {
- ...stateWithoutNativeFilters.dashboardState,
- activeTabs: ['TAB-active'],
- },
- dashboardInfo: {
- id: 1,
- dash_edit_perm: true,
- metadata: {
- native_filter_configuration: [
- inScopeFilter,
- outOfScopeRequiredFilter,
- outOfScopeNonRequiredFilter,
- ],
+ test('handleClearAll only dispatches for filters present in dataMask', async () => {
+ const idInMask = 'NATIVE_FILTER-has-value';
+ const idNotInMask = 'NATIVE_FILTER-no-value';
+ const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
+ const baseFilter = {
+ targets: [{ datasetId: 7, column: { name: 'x' } }],
+ defaultDataMask: { filterState: { value: null }, extraFormData: {} },
+ cascadeParentIds: [],
+ scope: { rootPath: ['ROOT_ID'], excluded: [] },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [18],
+ tabsInScope: [],
+ };
+ const stateWithTwoFiltersOneInMask = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
+ filterBarOrientation: FilterBarOrientation.Vertical,
+ metadata: {
+ native_filter_configuration: [
+ {
+ ...baseFilter,
+ id: idInMask,
+ name: 'A',
+ filterType: 'filter_select',
+ },
+ {
+ ...baseFilter,
+ id: idNotInMask,
+ name: 'B',
+ filterType: 'filter_select',
+ },
+ ],
+ chart_configuration: {},
+ },
},
- },
- dataMask: {
- [inScopeFilterId]: createDataMask(inScopeFilterId, ['value1'], {
- filters: [{ col: 'column1', op: 'IN', val: ['value1'] }],
- }),
- [outOfScopeRequiredFilterId]: createDataMask(
- outOfScopeRequiredFilterId,
- ['value2'],
- { filters: [{ col: 'column2', op: 'IN', val: ['value2'] }] },
- ),
- [outOfScopeNonRequiredFilterId]: createDataMask(
- outOfScopeNonRequiredFilterId,
- ['value3'],
- { filters: [{ col: 'column3', op: 'IN', val: ['value3'] }] },
- ),
- },
- nativeFilters: {
- filters: {
- [inScopeFilterId]: inScopeFilter,
- [outOfScopeRequiredFilterId]: outOfScopeRequiredFilter,
- [outOfScopeNonRequiredFilterId]: outOfScopeNonRequiredFilter,
+ dataMask: {
+ [idInMask]: {
+ id: idInMask,
+ filterState: { value: ['v'] },
+ extraFormData: {},
+ },
},
- filtersState: {},
- },
- };
-
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithTabsAndFilters);
-
- await act(async () => {
- jest.advanceTimersByTime(300);
- });
-
- const clearButton = screen.getByTestId(getTestId('clear-button'));
- expect(clearButton).toBeInTheDocument();
+ };
- await act(async () => {
- userEvent.click(clearButton);
- });
+ renderWrapper(openedBarProps, stateWithTwoFiltersOneInMask);
+ await act(async () => {
+ jest.advanceTimersByTime(300);
+ });
- // Verify only the in-scope filter was cleared, not the out-of-scope ones
- const clearedFilterIds = updateDataMaskSpy.mock.calls.map(call => call[0]);
- expect(clearedFilterIds).toContain(inScopeFilterId);
- expect(clearedFilterIds).not.toContain(outOfScopeRequiredFilterId);
- expect(clearedFilterIds).not.toContain(outOfScopeNonRequiredFilterId);
+ const clearBtn = screen.getByTestId(getTestId('clear-button'));
+ await act(async () => {
+ userEvent.click(clearBtn);
Review Comment:
Addressed in f71a5361 — added `await` before userEvent.click(clearBtn) in the only-dispatch-once test. Resolving — thanks for the catch.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(a11y): WCAG 1.3.3 — add aria-pressed/expanded to toggle buttons [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #39231:
URL: https://github.com/apache/superset/pull/39231#discussion_r3232636798
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/Header.test.tsx:
##########
@@ -53,3 +53,21 @@ test('should toggle', () => {
userEvent.click(expandBtn);
expect(mockedProps.toggleFiltersBar).toHaveBeenCalled();
});
+
+test('collapse button should have aria-expanded attribute', () => {
+ const mockedProps = createProps();
+ render(<Header {...mockedProps} />, { useRedux: true });
+ const collapseBtn = screen.getByRole('button', {
+ name: 'vertical-align',
+ });
+ expect(collapseBtn).toHaveAttribute('aria-expanded', 'true');
+});
+
+test('collapse button should have aria-label for accessibility', () => {
+ const mockedProps = createProps();
+ render(<Header {...mockedProps} />, { useRedux: true });
+ const collapseBtn = screen.getByRole('button', {
+ name: 'vertical-align',
+ });
+ expect(collapseBtn).toHaveAttribute('aria-label', 'Collapse filters');
+});
Review Comment:
<!-- Bito Reply -->
The `/resolve` command is not supported on inline comments. Please use it in a top-level comment instead.
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx:
##########
@@ -56,49 +55,73 @@ class MainPreset extends Preset {
}
}
-new MainPreset().register();
-
fetchMock.get('glob:*/api/v1/dataset/7', {
description_columns: {},
id: 1,
- label_columns: { columns: 'Columns', table_name: 'Table Name' },
+ label_columns: {
+ columns: 'Columns',
+ table_name: 'Table Name',
+ },
result: {
metrics: [],
- columns: [{ column_name: 'Column A', id: 1 }],
+ columns: [
+ {
+ column_name: 'Column A',
+ id: 1,
+ },
+ ],
table_name: 'birth_names',
id: 1,
},
show_columns: ['id', 'table_name'],
});
-// Cleanup between tests
-beforeEach(() => {
- jest.clearAllMocks();
-});
-
const getTestId = testWithId<string>(FILTER_BAR_TEST_ID, true);
const getModalTestId = testWithId<string>(FILTERS_CONFIG_MODAL_TEST_ID, true);
-function createClosedBarProps(toggleFiltersBar = jest.fn()) {
- return { filtersOpen: false, toggleFiltersBar };
-}
+const FILTER_NAME = 'Time filter 1';
-function createOpenedBarProps(toggleFiltersBar = jest.fn()) {
- return { filtersOpen: true, toggleFiltersBar };
-}
+const addFilterFlow = async () => {
+ // open filter config modals
+ userEvent.click(screen.getByTestId(getTestId('collapsable')));
+ userEvent.click(screen.getByLabelText('setting'));
+ userEvent.click(screen.getByText('Add or edit filters and controls'));
+ // select filter
+ userEvent.click(screen.getByText('Value'));
+ userEvent.click(screen.getByText('Time range'));
+ userEvent.type(screen.getByTestId(getModalTestId('name-input')), FILTER_NAME);
+ userEvent.click(screen.getByText('Save'));
+ // TODO: fix this flaky test
+ // await screen.findByText('All filters (1)');
Review Comment:
<!-- Bito Reply -->
The `/resolve` command is not supported on inline comments. Please use it in a top-level comment instead.
##########
superset-frontend/src/dashboard/components/AutoRefreshIndicator/index.tsx:
##########
@@ -124,6 +124,7 @@ export const AutoRefreshIndicator: FC<AutoRefreshIndicatorProps> = ({
css={iconButtonStyles}
onClick={onTogglePause}
aria-label={tooltipTitle}
+ aria-pressed={isPaused}
Review Comment:
<!-- Bito Reply -->
The `/resolve` command is not supported on inline comments. Please use it in a top-level comment instead.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(a11y): WCAG 1.3.3 — add aria-pressed/expanded to toggle buttons [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #39231:
URL: https://github.com/apache/superset/pull/39231#discussion_r3060082775
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx:
##########
@@ -267,6 +267,9 @@ const VerticalFilterBar: FC<VerticalBarProps> = ({
className={cx({ open: !filtersOpen })}
onClick={openFiltersBar}
role="button"
Review Comment:
**Suggestion:** The element is now keyboard-focusable via `tabIndex={0}` but still only handles mouse clicks, so pressing Enter/Space on the focused toggle will not open the filter bar. Add a keyboard handler that mirrors the click behavior to make the control operable and avoid a non-functional interactive element. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Keyboard users cannot expand dashboard filter bar.
- ⚠️ Screen readers announce toggle state but not operable.
- ⚠️ Violates WCAG 2.1 keyboard accessibility requirements.
```
</details>
```suggestion
onKeyDown={event => {
if (event.key === 'Enter' || event.key === ' ') {
event.preventDefault();
openFiltersBar();
}
}}
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open any dashboard with native filters enabled so `DashboardBuilder` renders the
vertical filter bar; this path goes through `DashboardBuilder.tsx:458` (`showFilterBar =
!editMode && nativeFiltersEnabled`) and `DashboardBuilder.tsx:587-595`, which render
`<FilterBar orientation={FilterBarOrientation.Vertical} ... />`.
2. Collapse the filter bar (for example by clicking the header toggle wired to
`toggleDashboardFiltersOpen` passed as `verticalConfig.toggleFiltersBar` in
`DashboardBuilder.tsx:587-593`), so `filtersOpen` becomes `false` and the collapsed
trigger `<CollapsedBar>` is shown.
3. With the bar collapsed, use keyboard navigation (Tab) until focus lands on the
collapsed filter bar trigger rendered by `CollapsedBar` in `Vertical.tsx:265-273`, which
has `role="button"` and `tabIndex={0}` with `onClick={openFiltersBar}` at
`Vertical.tsx:268` and `aria-label={t('Expand filters')}` at `Vertical.tsx:272`.
4. Press Enter or Space while that element is focused; observe that `openFiltersBar`
(defined at `Vertical.tsx:150-152` as `() => toggleFiltersBar(true)`) is only attached to
`onClick` and there is no `onKeyDown` handler, so no action occurs and the filter bar
remains collapsed, making the control effectively unusable for keyboard-only users.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx
**Line:** 269:269
**Comment:**
*Logic Error: The element is now keyboard-focusable via `tabIndex={0}` but still only handles mouse clicks, so pressing Enter/Space on the focused toggle will not open the filter bar. Add a keyboard handler that mirrors the click behavior to make the control operable and avoid a non-functional interactive element.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39231&comment_hash=47a8eb1b8c774206aba5ccd2fa044a76f41e9ce9c7dc66b2b5ee150da399817e&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39231&comment_hash=47a8eb1b8c774206aba5ccd2fa044a76f41e9ce9c7dc66b2b5ee150da399817e&reaction=dislike'>👎</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(a11y): WCAG 1.3.3 — add aria-pressed/expanded to toggle buttons [superset]
Posted by "Aitema-gmbh (via GitHub)" <gi...@apache.org>.
Aitema-gmbh commented on code in PR #39231:
URL: https://github.com/apache/superset/pull/39231#discussion_r3232632034
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx:
##########
@@ -267,6 +267,9 @@ const VerticalFilterBar: FC<VerticalBarProps> = ({
className={cx({ open: !filtersOpen })}
onClick={openFiltersBar}
role="button"
Review Comment:
Addressed in aabe2d99 — added onKeyDown handler that mirrors onClick for Enter/Space so keyboard users can expand the collapsed filter bar. Resolving — thanks for the catch.
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx:
##########
@@ -56,49 +55,73 @@ class MainPreset extends Preset {
}
}
-new MainPreset().register();
-
fetchMock.get('glob:*/api/v1/dataset/7', {
description_columns: {},
id: 1,
- label_columns: { columns: 'Columns', table_name: 'Table Name' },
+ label_columns: {
+ columns: 'Columns',
+ table_name: 'Table Name',
+ },
result: {
metrics: [],
- columns: [{ column_name: 'Column A', id: 1 }],
+ columns: [
+ {
+ column_name: 'Column A',
+ id: 1,
+ },
+ ],
table_name: 'birth_names',
id: 1,
},
show_columns: ['id', 'table_name'],
});
-// Cleanup between tests
-beforeEach(() => {
- jest.clearAllMocks();
-});
-
const getTestId = testWithId<string>(FILTER_BAR_TEST_ID, true);
const getModalTestId = testWithId<string>(FILTERS_CONFIG_MODAL_TEST_ID, true);
-function createClosedBarProps(toggleFiltersBar = jest.fn()) {
- return { filtersOpen: false, toggleFiltersBar };
-}
+const FILTER_NAME = 'Time filter 1';
-function createOpenedBarProps(toggleFiltersBar = jest.fn()) {
- return { filtersOpen: true, toggleFiltersBar };
-}
+const addFilterFlow = async () => {
+ // open filter config modals
+ userEvent.click(screen.getByTestId(getTestId('collapsable')));
+ userEvent.click(screen.getByLabelText('setting'));
+ userEvent.click(screen.getByText('Add or edit filters and controls'));
+ // select filter
+ userEvent.click(screen.getByText('Value'));
+ userEvent.click(screen.getByText('Time range'));
+ userEvent.type(screen.getByTestId(getModalTestId('name-input')), FILTER_NAME);
+ userEvent.click(screen.getByText('Save'));
Review Comment:
Addressed in e71b0d57 / f71a5361 — addFilterFlow now uses async findByText/findByTestId and awaits every userEvent.click, with a final assertion on 'All filters (1)'. Resolving — thanks for the catch.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(a11y): WCAG 1.3.3 — add aria-pressed/expanded to toggle buttons [superset]
Posted by "Aitema-gmbh (via GitHub)" <gi...@apache.org>.
Aitema-gmbh commented on code in PR #39231:
URL: https://github.com/apache/superset/pull/39231#discussion_r3232634409
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/index.tsx:
##########
@@ -76,6 +76,8 @@ const Header: FC<HeaderProps> = ({ toggleFiltersBar }) => (
buttonStyle="link"
buttonSize="xsmall"
onClick={() => toggleFiltersBar(false)}
+ aria-expanded
+ aria-label={t('Collapse filters')}
Review Comment:
Intentional — the Header component is only rendered inside the open FilterBar (BarWrapper has `open: filtersOpen` class), so whenever the collapse button exists, the panel is by definition expanded. Making the value dynamic would be a state attribute that is mathematically constant from the user's perspective. Static `aria-expanded="true"` is the correct WCAG signal — the dynamic counterpart lives on the collapsed-bar trigger in Vertical.tsx. Resolving.
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/Header.test.tsx:
##########
@@ -53,3 +53,21 @@ test('should toggle', () => {
userEvent.click(expandBtn);
expect(mockedProps.toggleFiltersBar).toHaveBeenCalled();
});
+
+test('collapse button should have aria-expanded attribute', () => {
+ const mockedProps = createProps();
+ render(<Header {...mockedProps} />, { useRedux: true });
+ const collapseBtn = screen.getByRole('button', {
+ name: 'vertical-align',
+ });
+ expect(collapseBtn).toHaveAttribute('aria-expanded', 'true');
+});
+
+test('collapse button should have aria-label for accessibility', () => {
+ const mockedProps = createProps();
+ render(<Header {...mockedProps} />, { useRedux: true });
+ const collapseBtn = screen.getByRole('button', {
+ name: 'vertical-align',
+ });
+ expect(collapseBtn).toHaveAttribute('aria-label', 'Collapse filters');
+});
Review Comment:
Addressed in 7a006443 — test selectors now use 'Collapse filters' (the actual accessible name from aria-label) instead of 'vertical-align'. Resolving — thanks for the catch.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(a11y): WCAG 1.3.3 — add aria-pressed/expanded to toggle buttons [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #39231:
URL: https://github.com/apache/superset/pull/39231#discussion_r3107130199
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx:
##########
@@ -109,684 +139,550 @@ function createMockApi(filterName = 'Time filter 1') {
],
};
});
-}
-
-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,
- };
-}
-
-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],
+ 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' },
},
- ...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: getTimeRangeNoFilterMockUrl },
+ );
+
+ fetchMock.removeRoute(getTimeRangeLastDayMockUrl);
+ fetchMock.get(
+ getTimeRangeLastDayMockUrl,
+ {
+ result: {
+ since: '2021-04-13T00:00:00',
+ until: '2021-04-14T00:00:00',
+ timeRange: 'Last day',
+ },
},
- },
- { 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: 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.lastWeek },
- );
-}
-
-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,
- },
- );
-}
-
-test('FilterBar renders without crashing', () => {
- const props = createClosedBarProps();
- const { container } = renderFilterBar(props);
- expect(container).toBeInTheDocument();
-});
+ { name: getTimeRangeLastWeekMockUrl },
+ );
-test('FilterBar renders "Filters and controls" heading', () => {
- const props = createClosedBarProps();
- renderFilterBar(props);
- 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('FilterBar renders "Apply filters" button', () => {
- const props = createClosedBarProps();
- renderFilterBar(props);
- 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('FilterBar renders filter icon', () => {
- const props = createClosedBarProps();
- renderFilterBar(props);
- 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);
-
- const collapse = screen.getByRole('img', { name: 'vertical-align' });
- expect(toggleFiltersBar).not.toHaveBeenCalled();
-
- userEvent.click(collapse);
- expect(toggleFiltersBar).toHaveBeenCalled();
-});
-
-test('FilterBar opens when expand button is clicked', () => {
- const toggleFiltersBar = jest.fn();
- const props = createClosedBarProps(toggleFiltersBar);
- renderFilterBar(props);
-
- expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
- expect(screen.getByTestId(getTestId('expand-button'))).toBeInTheDocument();
-
- userEvent.click(screen.getByTestId(getTestId('collapsable')));
- expect(toggleFiltersBar).toHaveBeenCalledWith(true);
-});
-
-test('FilterBar hides edit filter button when user lacks permissions', () => {
- const props = createOpenedBarProps();
- const stateWithoutPermissions = {
- ...stateWithoutNativeFilters,
- dashboardInfo: { metadata: {} },
- };
+ mockedMakeApi.mockReturnValue(mockApi);
+ });
- renderFilterBar(props, stateWithoutPermissions);
+ 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,
+ },
+ );
- expect(
- screen.queryByTestId(getTestId('create-filter')),
- ).not.toBeInTheDocument();
-});
+ test('should render', () => {
+ const { container } = renderWrapper();
+ expect(container).toBeInTheDocument();
+ });
-test('FilterBar closes when collapse button is clicked', () => {
- const toggleFiltersBar = jest.fn();
- const props = createOpenedBarProps(toggleFiltersBar);
- renderFilterBar(props);
+ test('should render the "Filters and controls" heading', () => {
+ renderWrapper();
+ expect(screen.getByText('Filters and controls')).toBeInTheDocument();
+ });
- const collapseButton = screen.getByTestId(getTestId('collapse-button'));
- expect(collapseButton).toBeInTheDocument();
+ test('should render the "Clear all" option', () => {
+ renderWrapper();
+ expect(screen.getByText('Clear all')).toBeInTheDocument();
+ });
- userEvent.click(collapseButton);
- expect(toggleFiltersBar).toHaveBeenCalledWith(false);
-});
+ test('should render the "Apply filters" option', () => {
+ renderWrapper();
+ expect(screen.getByText('Apply filters')).toBeInTheDocument();
+ });
-test('FilterBar disables buttons when there are no filters', () => {
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithoutNativeFilters);
+ test('should render the collapse icon', () => {
+ renderWrapper();
+ expect(
+ screen.getByRole('img', { name: 'vertical-align' }),
+ ).toBeInTheDocument();
+ });
- expect(screen.getByTestId(getTestId('clear-button'))).toBeDisabled();
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
-});
+ test('should render the filter icon', () => {
+ renderWrapper();
+ expect(screen.getByRole('img', { name: 'filter' })).toBeInTheDocument();
+ });
-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 },
- },
- };
+ test('should toggle', () => {
+ renderWrapper();
+ const collapse = screen.getByRole('img', {
+ name: 'vertical-align',
+ });
+ expect(toggleFiltersBar).not.toHaveBeenCalled();
+ userEvent.click(collapse);
+ expect(toggleFiltersBar).toHaveBeenCalled();
+ });
- renderFilterBar(props, stateWithDivider);
+ test('open filter bar', () => {
+ renderWrapper();
+ expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
+ expect(screen.getByTestId(getTestId('expand-button'))).toBeInTheDocument();
- await act(async () => {
- jest.advanceTimersByTime(1000);
+ userEvent.click(screen.getByTestId(getTestId('collapsable')));
+ expect(toggleFiltersBar).toHaveBeenCalledWith(true);
});
- const title = await screen.findByText('Select time range');
- const description = await screen.findByText('Select year/month etc..');
+ test('collapsed filter bar should have aria-expanded=false', () => {
+ renderWrapper();
+ const collapsedBar = screen.getByTestId(getTestId('collapsable'));
+ expect(collapsedBar).toHaveAttribute('aria-expanded', 'false');
+ expect(collapsedBar).toHaveAttribute('aria-label', 'Expand filters');
+ });
- 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('no edit filter button by disabled permissions', () => {
+ renderWrapper(openedBarProps, {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: { metadata: {} },
+ });
-test('FilterBar apply button is disabled after creating a filter', async () => {
- setupTimeRangeMocks();
- mockedMakeApi.mockReturnValue(createMockApi());
+ expect(
+ screen.queryByTestId(getTestId('create-filter')),
+ ).not.toBeInTheDocument();
+ });
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithoutNativeFilters);
+ test('close filter bar', () => {
+ renderWrapper(openedBarProps);
+ const collapseButton = screen.getByTestId(getTestId('collapse-button'));
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
+ expect(collapseButton).toBeInTheDocument();
+ userEvent.click(collapseButton);
- // 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(toggleFiltersBar).toHaveBeenCalledWith(false);
+ });
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
-});
+ test('no filters', () => {
+ renderWrapper(openedBarProps, stateWithoutNativeFilters);
-test('FilterBar renders without errors when filter has required controlValues', () => {
- const props = createOpenedBarProps();
- const filter = createFilter({
- id: 'test-filter',
- controlValues: { enableEmptyFilter: true },
+ expect(screen.getByTestId(getTestId('clear-button'))).toBeDisabled();
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
});
- const dataMask = createDataMask('test-filter', undefined, {});
- const state = createStateWithFilter(filter, dataMask);
- const { container } = renderFilterBar(props, state);
- expect(container).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,
+ },
+ },
+ };
-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: {},
- },
- });
+ renderWrapper(openedBarProps, stateWithDivider);
- const dataMask = createDataMask(filterId, ['value1'], {});
- const state = createStateWithFilter(filter, dataMask);
+ await act(async () => {
+ jest.advanceTimersByTime(1000); // 1s
+ });
- renderFilterBar(props, state);
+ const title = await screen.findByText('Select time range');
+ const description = await screen.findByText('Select year/month etc..');
- await act(async () => {
- jest.advanceTimersByTime(300);
+ 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();
});
- expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
- expect(screen.getByText('Filters and controls')).toBeInTheDocument();
+ test('create filter and apply it flow', async () => {
+ renderWrapper(openedBarProps, stateWithoutNativeFilters);
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
- // The filter value should not be cleared during initialization
- expect(updateDataMaskSpy).not.toHaveBeenCalled();
+ await addFilterFlow();
- updateDataMaskSpy.mockRestore();
-});
-
-test('FilterBar renders correctly when filter has complete extraFormData', async () => {
- const filterId = 'test-filter-complete';
- const props = createOpenedBarProps();
- const filter = createFilter({
- id: filterId,
- controlValues: { enableEmptyFilter: true },
- defaultDataMask: {
- filterState: { value: ['value1'] },
- extraFormData: {
- filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
- },
- },
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
});
- const dataMask = createDataMask(filterId, ['value1'], {
- filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
- });
- const state = createStateWithFilter(filter, dataMask);
- renderFilterBar(props, state);
+ 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: {},
+ },
+ };
- await act(async () => {
- jest.advanceTimersByTime(100);
+ const { container } = renderWrapper(openedBarProps, stateWithFilter);
+ expect(container).toBeInTheDocument();
});
- expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
-});
+ test('auto-applies filter when extraFormData is empty in applied state', async () => {
+ const filterId = 'test-filter-auto-apply';
+ const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
-test('handleClearAll dispatches updateDataMask with value undefined for filter_select', async () => {
- const filterId = 'NATIVE_FILTER-clear-select';
- const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
- const selectFilter = createFilter({
- id: filterId,
- name: 'Region',
- filterType: 'filter_select',
- targets: [{ datasetId: 7, column: { name: 'region' } }],
- defaultDataMask: { filterState: { value: null }, extraFormData: {} },
- chartsInScope: [18],
- });
- const stateWithSelect = {
- ...stateWithoutNativeFilters,
- dashboardInfo: {
- id: 1,
- dash_edit_perm: true,
- filterBarOrientation: FilterBarOrientation.Vertical,
- metadata: {
- native_filter_configuration: [selectFilter],
- chart_configuration: {},
+ const stateWithIncompleteFilter = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
},
- },
- dashboardState: {
- ...stateWithoutNativeFilters.dashboardState,
- activeTabs: ['ROOT_ID'],
- },
- dataMask: {
- [filterId]: createDataMask(filterId, ['East']),
- },
- nativeFilters: {
- filters: { [filterId]: selectFilter },
- filtersState: {},
- },
- };
+ dataMask: {
+ [filterId]: {
+ id: filterId,
+ filterState: { value: ['value1', 'value2'] },
+ extraFormData: {},
+ },
+ },
+ nativeFilters: {
+ filters: {
+ [filterId]: {
+ id: filterId,
+ name: 'Test Filter',
+ filterType: 'filter_select',
+ targets: [{ datasetId: 1, column: { name: 'test_column' } }],
+ defaultDataMask: {
+ filterState: { value: ['value1', 'value2'] },
+ extraFormData: {},
+ },
+ controlValues: {
+ enableEmptyFilter: true,
+ },
+ cascadeParentIds: [],
+ scope: {
+ rootPath: ['ROOT_ID'],
+ excluded: [],
+ },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [],
+ tabsInScope: [],
+ },
+ },
+ filtersState: {},
+ },
+ };
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithSelect);
- await act(async () => {
- jest.advanceTimersByTime(300);
- });
+ renderWrapper(openedBarProps, stateWithIncompleteFilter);
- const clearBtn = screen.getByTestId(getTestId('clear-button'));
- expect(clearBtn).not.toBeDisabled();
- await act(async () => {
- userEvent.click(clearBtn);
- });
+ await act(async () => {
+ jest.advanceTimersByTime(200);
+ });
- expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, {
- filterState: { value: undefined },
- extraFormData: {},
- });
- updateDataMaskSpy.mockRestore();
-});
+ expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
+ // The test name promises that auto-apply ran; without this assertion the
+ // test passed even when the dispatch was silently skipped.
+ expect(updateDataMaskSpy).toHaveBeenCalled();
-test('handleClearAll dispatches updateDataMask with [null, null] for filter_range', async () => {
- fetchMock.post('glob:*/api/v1/chart/data', {
- result: [{ data: [{ min: 0, max: 100 }] }],
- });
- const filterId = 'NATIVE_FILTER-clear-range';
- const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
- const rangeFilter = createFilter({
- id: filterId,
- name: 'Age',
- filterType: 'filter_range',
- targets: [{ datasetId: 7, column: { name: 'age' } }],
- defaultDataMask: { filterState: { value: null }, extraFormData: {} },
- chartsInScope: [18],
+ updateDataMaskSpy.mockRestore();
});
- const stateWithRange = {
- ...stateWithoutNativeFilters,
- dashboardInfo: {
- id: 1,
- dash_edit_perm: true,
- filterBarOrientation: FilterBarOrientation.Vertical,
- metadata: {
- native_filter_configuration: [rangeFilter],
- chart_configuration: {},
+
+ test('renders correctly when filter has complete extraFormData', async () => {
+ const filterId = 'test-filter-complete';
+ const stateWithCompleteFilter = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
},
- },
- dashboardState: {
- ...stateWithoutNativeFilters.dashboardState,
- activeTabs: ['ROOT_ID'],
- },
- dataMask: {
- [filterId]: createDataMask(filterId, [10, 50]),
- },
- nativeFilters: {
- filters: { [filterId]: rangeFilter },
- filtersState: {},
- },
- };
+ dataMask: {
+ [filterId]: {
+ id: filterId,
+ filterState: { value: ['value1'] },
+ extraFormData: {
+ filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
+ },
+ },
+ },
+ nativeFilters: {
+ filters: {
+ [filterId]: {
+ id: filterId,
+ name: 'Test Filter',
+ filterType: 'filter_select',
+ targets: [{ datasetId: 1, column: { name: 'test_column' } }],
+ defaultDataMask: {
+ filterState: { value: ['value1'] },
+ extraFormData: {
+ filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
+ },
+ },
+ controlValues: {
+ enableEmptyFilter: true,
+ },
+ cascadeParentIds: [],
+ scope: {
+ rootPath: ['ROOT_ID'],
+ excluded: [],
+ },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [],
+ tabsInScope: [],
+ },
+ },
+ filtersState: {},
+ },
+ };
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithRange);
- await act(async () => {
- jest.advanceTimersByTime(300);
- });
+ renderWrapper(openedBarProps, stateWithCompleteFilter);
- const clearBtn = screen.getByTestId(getTestId('clear-button'));
- expect(clearBtn).not.toBeDisabled();
- await act(async () => {
- userEvent.click(clearBtn);
- });
+ await act(async () => {
+ jest.advanceTimersByTime(100);
+ });
- expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, {
- filterState: { value: [null, null] },
- extraFormData: {},
+ expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
});
- updateDataMaskSpy.mockRestore();
-});
-test('handleClearAll only dispatches for filters present in dataMask', async () => {
- const idInMask = 'NATIVE_FILTER-has-value';
- const idNotInMask = 'NATIVE_FILTER-no-value';
- const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
- const filterInMask = createFilter({
- id: idInMask,
- name: 'A',
- filterType: 'filter_select',
- targets: [{ datasetId: 7, column: { name: 'x' } }],
- chartsInScope: [18],
- });
- const filterNotInMask = createFilter({
- id: idNotInMask,
- name: 'B',
- filterType: 'filter_select',
- targets: [{ datasetId: 7, column: { name: 'x' } }],
- chartsInScope: [18],
- });
- const stateWithTwoFilters = {
- ...stateWithoutNativeFilters,
- dashboardInfo: {
- id: 1,
- dash_edit_perm: true,
- filterBarOrientation: FilterBarOrientation.Vertical,
- metadata: {
- native_filter_configuration: [filterInMask, filterNotInMask],
- chart_configuration: {},
+ test('handleClearAll dispatches updateDataMask with value null for filter_select', async () => {
+ const filterId = 'NATIVE_FILTER-clear-select';
+ const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
+ const selectFilterConfig = {
+ id: filterId,
+ name: 'Region',
+ filterType: 'filter_select',
+ targets: [{ datasetId: 7, column: { name: 'region' } }],
+ defaultDataMask: { filterState: { value: null }, extraFormData: {} },
+ cascadeParentIds: [],
+ scope: { rootPath: ['ROOT_ID'], excluded: [] },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [18],
+ tabsInScope: [],
+ };
+ const stateWithSelect = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
+ filterBarOrientation: FilterBarOrientation.Vertical,
+ metadata: {
+ native_filter_configuration: [selectFilterConfig],
+ chart_configuration: {},
+ },
},
- },
- dashboardState: {
- ...stateWithoutNativeFilters.dashboardState,
- activeTabs: ['ROOT_ID'],
- },
- dataMask: {
- [idInMask]: createDataMask(idInMask, ['v']),
- },
- nativeFilters: {
- filters: {
- [idInMask]: filterInMask,
- [idNotInMask]: filterNotInMask,
+ dataMask: {
+ [filterId]: {
+ id: filterId,
+ filterState: { value: ['East'] },
+ extraFormData: {},
+ },
},
- filtersState: {},
- },
- };
+ };
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithTwoFilters);
- await act(async () => {
- jest.advanceTimersByTime(300);
- });
+ renderWrapper(openedBarProps, stateWithSelect);
+ await act(async () => {
+ jest.advanceTimersByTime(300);
+ });
- const clearBtn = screen.getByTestId(getTestId('clear-button'));
- await act(async () => {
- userEvent.click(clearBtn);
- });
+ const clearBtn = screen.getByTestId(getTestId('clear-button'));
+ expect(clearBtn).not.toBeDisabled();
+ await act(async () => {
+ userEvent.click(clearBtn);
+ });
- expect(updateDataMaskSpy).toHaveBeenCalledTimes(1);
- expect(updateDataMaskSpy).toHaveBeenCalledWith(idInMask, {
- filterState: { value: undefined },
- extraFormData: {},
+ expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, {
+ filterState: { value: undefined },
+ extraFormData: {},
+ });
+ updateDataMaskSpy.mockRestore();
});
- updateDataMaskSpy.mockRestore();
-});
-test('FilterBar Clear All only clears in-scope filters, not out-of-scope ones', async () => {
- const inScopeFilterId = 'NATIVE_FILTER-in-scope';
- const outOfScopeRequiredFilterId = 'NATIVE_FILTER-out-of-scope-required';
- const outOfScopeNonRequiredFilterId =
- 'NATIVE_FILTER-out-of-scope-non-required';
- const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
-
- const dashboardLayoutWithTabs = {
- ROOT_ID: { id: 'ROOT_ID', type: 'ROOT', children: ['TABS-1'] },
- 'TABS-1': {
- id: 'TABS-1',
- type: 'TABS',
- children: ['TAB-active', 'TAB-inactive'],
- },
- 'TAB-active': {
- id: 'TAB-active',
- type: 'TAB',
- children: ['CHART_ROW-1'],
- meta: { text: 'Active Tab' },
- parents: ['ROOT_ID', 'TABS-1'],
- },
- 'TAB-inactive': {
- id: 'TAB-inactive',
- type: 'TAB',
- children: ['CHART_ROW-2'],
- meta: { text: 'Inactive Tab' },
- parents: ['ROOT_ID', 'TABS-1'],
- },
- 'CHART_ROW-1': {
- id: 'CHART_ROW-1',
- type: 'CHART',
- meta: { chartId: 1 },
- parents: ['ROOT_ID', 'TABS-1', 'TAB-active'],
- },
- 'CHART_ROW-2': {
- id: 'CHART_ROW-2',
- type: 'CHART',
- meta: { chartId: 2 },
- parents: ['ROOT_ID', 'TABS-1', 'TAB-inactive'],
- },
- };
+ test('handleClearAll dispatches updateDataMask with value [null, null] for filter_range', async () => {
+ fetchMock.post('glob:*/api/v1/chart/data', {
+ result: [{ data: [{ min: 0, max: 100 }] }],
+ });
+ const filterId = 'NATIVE_FILTER-clear-range';
+ const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
+ const rangeFilterConfig = {
+ id: filterId,
+ name: 'Age',
+ filterType: 'filter_range',
+ targets: [{ datasetId: 7, column: { name: 'age' } }],
+ defaultDataMask: { filterState: { value: null }, extraFormData: {} },
+ cascadeParentIds: [],
+ scope: { rootPath: ['ROOT_ID'], excluded: [] },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [18],
+ tabsInScope: [],
+ };
+ const stateWithRange = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
+ filterBarOrientation: FilterBarOrientation.Vertical,
+ metadata: {
+ native_filter_configuration: [rangeFilterConfig],
+ chart_configuration: {},
+ },
+ },
+ dataMask: {
+ [filterId]: {
+ id: filterId,
+ filterState: { value: [10, 50] },
+ extraFormData: {},
+ },
+ },
+ };
- const inScopeFilter = createFilter({
- id: inScopeFilterId,
- name: 'In Scope Filter',
- targets: [{ datasetId: 1, column: { name: 'column1' } }],
- controlValues: { enableEmptyFilter: false },
- chartsInScope: [1],
- tabsInScope: ['TAB-active'],
- });
+ renderWrapper(openedBarProps, stateWithRange);
+ await act(async () => {
+ jest.advanceTimersByTime(300);
+ });
- const outOfScopeRequiredFilter = createFilter({
- id: outOfScopeRequiredFilterId,
- name: 'Out of Scope Required Filter',
- targets: [{ datasetId: 1, column: { name: 'column2' } }],
- controlValues: { enableEmptyFilter: true },
- chartsInScope: [2],
- tabsInScope: ['TAB-inactive'],
- });
+ const clearBtn = screen.getByTestId(getTestId('clear-button'));
+ expect(clearBtn).not.toBeDisabled();
+ await act(async () => {
+ userEvent.click(clearBtn);
+ });
- const outOfScopeNonRequiredFilter = createFilter({
- id: outOfScopeNonRequiredFilterId,
- name: 'Out of Scope Non-Required Filter',
- targets: [{ datasetId: 1, column: { name: 'column3' } }],
- controlValues: { enableEmptyFilter: false },
- chartsInScope: [2],
- tabsInScope: ['TAB-inactive'],
+ expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, {
+ filterState: { value: [null, null] },
+ extraFormData: {},
+ });
+ updateDataMaskSpy.mockRestore();
});
- const stateWithTabsAndFilters = {
- ...stateWithoutNativeFilters,
- dashboardLayout: {
- present: dashboardLayoutWithTabs,
- past: [],
- future: [],
- },
- dashboardState: {
- ...stateWithoutNativeFilters.dashboardState,
- activeTabs: ['TAB-active'],
- },
- dashboardInfo: {
- id: 1,
- dash_edit_perm: true,
- metadata: {
- native_filter_configuration: [
- inScopeFilter,
- outOfScopeRequiredFilter,
- outOfScopeNonRequiredFilter,
- ],
+ test('handleClearAll only dispatches for filters present in dataMask', async () => {
+ const idInMask = 'NATIVE_FILTER-has-value';
+ const idNotInMask = 'NATIVE_FILTER-no-value';
+ const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
+ const baseFilter = {
+ targets: [{ datasetId: 7, column: { name: 'x' } }],
+ defaultDataMask: { filterState: { value: null }, extraFormData: {} },
+ cascadeParentIds: [],
+ scope: { rootPath: ['ROOT_ID'], excluded: [] },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [18],
+ tabsInScope: [],
+ };
+ const stateWithTwoFiltersOneInMask = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
+ filterBarOrientation: FilterBarOrientation.Vertical,
+ metadata: {
+ native_filter_configuration: [
+ {
+ ...baseFilter,
+ id: idInMask,
+ name: 'A',
+ filterType: 'filter_select',
+ },
+ {
+ ...baseFilter,
+ id: idNotInMask,
+ name: 'B',
+ filterType: 'filter_select',
+ },
+ ],
+ chart_configuration: {},
+ },
},
- },
- dataMask: {
- [inScopeFilterId]: createDataMask(inScopeFilterId, ['value1'], {
- filters: [{ col: 'column1', op: 'IN', val: ['value1'] }],
- }),
- [outOfScopeRequiredFilterId]: createDataMask(
- outOfScopeRequiredFilterId,
- ['value2'],
- { filters: [{ col: 'column2', op: 'IN', val: ['value2'] }] },
- ),
- [outOfScopeNonRequiredFilterId]: createDataMask(
- outOfScopeNonRequiredFilterId,
- ['value3'],
- { filters: [{ col: 'column3', op: 'IN', val: ['value3'] }] },
- ),
- },
- nativeFilters: {
- filters: {
- [inScopeFilterId]: inScopeFilter,
- [outOfScopeRequiredFilterId]: outOfScopeRequiredFilter,
- [outOfScopeNonRequiredFilterId]: outOfScopeNonRequiredFilter,
+ dataMask: {
+ [idInMask]: {
+ id: idInMask,
+ filterState: { value: ['v'] },
+ extraFormData: {},
+ },
},
- filtersState: {},
- },
- };
-
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithTabsAndFilters);
-
- await act(async () => {
- jest.advanceTimersByTime(300);
- });
-
- const clearButton = screen.getByTestId(getTestId('clear-button'));
- expect(clearButton).toBeInTheDocument();
+ };
- await act(async () => {
- userEvent.click(clearButton);
- });
+ renderWrapper(openedBarProps, stateWithTwoFiltersOneInMask);
+ await act(async () => {
+ jest.advanceTimersByTime(300);
+ });
- // Verify only the in-scope filter was cleared, not the out-of-scope ones
- const clearedFilterIds = updateDataMaskSpy.mock.calls.map(call => call[0]);
- expect(clearedFilterIds).toContain(inScopeFilterId);
- expect(clearedFilterIds).not.toContain(outOfScopeRequiredFilterId);
- expect(clearedFilterIds).not.toContain(outOfScopeNonRequiredFilterId);
+ const clearBtn = screen.getByTestId(getTestId('clear-button'));
+ await act(async () => {
+ userEvent.click(clearBtn);
Review Comment:
**Suggestion:** The async click is not awaited in this test path either, which can lead to nondeterministic timing where call-count assertions run too early. Await the click to guarantee deterministic behavior. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Filter clear-all call-count test may randomly fail in CI.
- ⚠️ Test-suite flakiness complicates debugging FilterBar regressions.
```
</details>
```suggestion
await userEvent.click(clearBtn);
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the Jest test file `FilterBar.test.tsx` at
`superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx:624-687`
(e.g. via `npm test -- FilterBar`).
2. The test `"handleClearAll only dispatches for filters present in dataMask"` sets up two
filters and a spy `updateDataMaskSpy` (lines 624-669), renders `FilterBar`, and advances
timers using `await act(async () => { jest.advanceTimersByTime(300); });` (lines 671-674).
3. It then clears all filters with `await act(async () => { userEvent.click(clearBtn);
});` at lines 676-679; `userEvent.click` is provided by `@testing-library/user-event`
through `spec/helpers/testing-library.tsx:45,137` and is asynchronous, but its returned
promise is not awaited inside the `act` callback.
4. Since the async `userEvent.click(clearBtn)` is not awaited, `act` can resolve before
the click-triggered `handleClearAll` logic in `FilterBar/index.tsx:90-155` finishes
dispatching Redux actions, so the call-count assertion
`expect(updateDataMaskSpy).toHaveBeenCalledTimes(1);` at lines 681-685 may sometimes run
before the dispatch completes, causing flaky assertion failures on this test path.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fsrc%2Fdashboard%2Fcomponents%2FnativeFilters%2FFilterBar%2FFilterBar.test.tsx%0A%2A%2ALine%3A%2A%2A%20678%3A678%0A%2A%2AComment%3A%2A%2A%0A%09%2APossible%20Bug%3A%20The%20async%20click%20is%20not%20awaited%20in%20this%20test%20path%20either%2C%20which%20can%20lead%20to%20nondeterministic%20timing%20where%20call-count%20assertions%20run%20too%20early.%20Await%20the%20click%20to%20guarantee%20deterministic%20behavior.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0A) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-front
end%2Fsrc%2Fdashboard%2Fcomponents%2FnativeFilters%2FFilterBar%2FFilterBar.test.tsx%0A%2A%2ALine%3A%2A%2A%20678%3A678%0A%2A%2AComment%3A%2A%2A%0A%09%2APossible%20Bug%3A%20The%20async%20click%20is%20not%20awaited%20in%20this%20test%20path%20either%2C%20which%20can%20lead%20to%20nondeterministic%20timing%20where%20call-count%20assertions%20run%20too%20early.%20Await%20the%20click%20to%20guarantee%20deterministic%20behavior.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0A)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx
**Line:** 678:678
**Comment:**
*Possible Bug: The async click is not awaited in this test path either, which can lead to nondeterministic timing where call-count assertions run too early. Await the click to guarantee deterministic behavior.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39231&comment_hash=60fd481ce19e4c1a3c5afaa70034b72cd626c55f34c2b80d95f0b9cfd24a6117&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39231&comment_hash=60fd481ce19e4c1a3c5afaa70034b72cd626c55f34c2b80d95f0b9cfd24a6117&reaction=dislike'>👎</a>
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx:
##########
@@ -109,684 +139,550 @@ function createMockApi(filterName = 'Time filter 1') {
],
};
});
-}
-
-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,
- };
-}
-
-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],
+ 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' },
},
- ...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: getTimeRangeNoFilterMockUrl },
+ );
+
+ fetchMock.removeRoute(getTimeRangeLastDayMockUrl);
+ fetchMock.get(
+ getTimeRangeLastDayMockUrl,
+ {
+ result: {
+ since: '2021-04-13T00:00:00',
+ until: '2021-04-14T00:00:00',
+ timeRange: 'Last day',
+ },
},
- },
- { 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: 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.lastWeek },
- );
-}
-
-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,
- },
- );
-}
-
-test('FilterBar renders without crashing', () => {
- const props = createClosedBarProps();
- const { container } = renderFilterBar(props);
- expect(container).toBeInTheDocument();
-});
+ { name: getTimeRangeLastWeekMockUrl },
+ );
-test('FilterBar renders "Filters and controls" heading', () => {
- const props = createClosedBarProps();
- renderFilterBar(props);
- 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('FilterBar renders "Apply filters" button', () => {
- const props = createClosedBarProps();
- renderFilterBar(props);
- 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('FilterBar renders filter icon', () => {
- const props = createClosedBarProps();
- renderFilterBar(props);
- 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);
-
- const collapse = screen.getByRole('img', { name: 'vertical-align' });
- expect(toggleFiltersBar).not.toHaveBeenCalled();
-
- userEvent.click(collapse);
- expect(toggleFiltersBar).toHaveBeenCalled();
-});
-
-test('FilterBar opens when expand button is clicked', () => {
- const toggleFiltersBar = jest.fn();
- const props = createClosedBarProps(toggleFiltersBar);
- renderFilterBar(props);
-
- expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
- expect(screen.getByTestId(getTestId('expand-button'))).toBeInTheDocument();
-
- userEvent.click(screen.getByTestId(getTestId('collapsable')));
- expect(toggleFiltersBar).toHaveBeenCalledWith(true);
-});
-
-test('FilterBar hides edit filter button when user lacks permissions', () => {
- const props = createOpenedBarProps();
- const stateWithoutPermissions = {
- ...stateWithoutNativeFilters,
- dashboardInfo: { metadata: {} },
- };
+ mockedMakeApi.mockReturnValue(mockApi);
+ });
- renderFilterBar(props, stateWithoutPermissions);
+ 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,
+ },
+ );
- expect(
- screen.queryByTestId(getTestId('create-filter')),
- ).not.toBeInTheDocument();
-});
+ test('should render', () => {
+ const { container } = renderWrapper();
+ expect(container).toBeInTheDocument();
+ });
-test('FilterBar closes when collapse button is clicked', () => {
- const toggleFiltersBar = jest.fn();
- const props = createOpenedBarProps(toggleFiltersBar);
- renderFilterBar(props);
+ test('should render the "Filters and controls" heading', () => {
+ renderWrapper();
+ expect(screen.getByText('Filters and controls')).toBeInTheDocument();
+ });
- const collapseButton = screen.getByTestId(getTestId('collapse-button'));
- expect(collapseButton).toBeInTheDocument();
+ test('should render the "Clear all" option', () => {
+ renderWrapper();
+ expect(screen.getByText('Clear all')).toBeInTheDocument();
+ });
- userEvent.click(collapseButton);
- expect(toggleFiltersBar).toHaveBeenCalledWith(false);
-});
+ test('should render the "Apply filters" option', () => {
+ renderWrapper();
+ expect(screen.getByText('Apply filters')).toBeInTheDocument();
+ });
-test('FilterBar disables buttons when there are no filters', () => {
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithoutNativeFilters);
+ test('should render the collapse icon', () => {
+ renderWrapper();
+ expect(
+ screen.getByRole('img', { name: 'vertical-align' }),
+ ).toBeInTheDocument();
+ });
- expect(screen.getByTestId(getTestId('clear-button'))).toBeDisabled();
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
-});
+ test('should render the filter icon', () => {
+ renderWrapper();
+ expect(screen.getByRole('img', { name: 'filter' })).toBeInTheDocument();
+ });
-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 },
- },
- };
+ test('should toggle', () => {
+ renderWrapper();
+ const collapse = screen.getByRole('img', {
+ name: 'vertical-align',
+ });
+ expect(toggleFiltersBar).not.toHaveBeenCalled();
+ userEvent.click(collapse);
+ expect(toggleFiltersBar).toHaveBeenCalled();
+ });
- renderFilterBar(props, stateWithDivider);
+ test('open filter bar', () => {
+ renderWrapper();
+ expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
+ expect(screen.getByTestId(getTestId('expand-button'))).toBeInTheDocument();
- await act(async () => {
- jest.advanceTimersByTime(1000);
+ userEvent.click(screen.getByTestId(getTestId('collapsable')));
+ expect(toggleFiltersBar).toHaveBeenCalledWith(true);
});
- const title = await screen.findByText('Select time range');
- const description = await screen.findByText('Select year/month etc..');
+ test('collapsed filter bar should have aria-expanded=false', () => {
+ renderWrapper();
+ const collapsedBar = screen.getByTestId(getTestId('collapsable'));
+ expect(collapsedBar).toHaveAttribute('aria-expanded', 'false');
+ expect(collapsedBar).toHaveAttribute('aria-label', 'Expand filters');
+ });
- 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('no edit filter button by disabled permissions', () => {
+ renderWrapper(openedBarProps, {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: { metadata: {} },
+ });
-test('FilterBar apply button is disabled after creating a filter', async () => {
- setupTimeRangeMocks();
- mockedMakeApi.mockReturnValue(createMockApi());
+ expect(
+ screen.queryByTestId(getTestId('create-filter')),
+ ).not.toBeInTheDocument();
+ });
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithoutNativeFilters);
+ test('close filter bar', () => {
+ renderWrapper(openedBarProps);
+ const collapseButton = screen.getByTestId(getTestId('collapse-button'));
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
+ expect(collapseButton).toBeInTheDocument();
+ userEvent.click(collapseButton);
- // 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(toggleFiltersBar).toHaveBeenCalledWith(false);
+ });
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
-});
+ test('no filters', () => {
+ renderWrapper(openedBarProps, stateWithoutNativeFilters);
-test('FilterBar renders without errors when filter has required controlValues', () => {
- const props = createOpenedBarProps();
- const filter = createFilter({
- id: 'test-filter',
- controlValues: { enableEmptyFilter: true },
+ expect(screen.getByTestId(getTestId('clear-button'))).toBeDisabled();
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
});
- const dataMask = createDataMask('test-filter', undefined, {});
- const state = createStateWithFilter(filter, dataMask);
- const { container } = renderFilterBar(props, state);
- expect(container).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,
+ },
+ },
+ };
-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: {},
- },
- });
+ renderWrapper(openedBarProps, stateWithDivider);
- const dataMask = createDataMask(filterId, ['value1'], {});
- const state = createStateWithFilter(filter, dataMask);
+ await act(async () => {
+ jest.advanceTimersByTime(1000); // 1s
+ });
- renderFilterBar(props, state);
+ const title = await screen.findByText('Select time range');
+ const description = await screen.findByText('Select year/month etc..');
- await act(async () => {
- jest.advanceTimersByTime(300);
+ 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();
});
- expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
- expect(screen.getByText('Filters and controls')).toBeInTheDocument();
+ test('create filter and apply it flow', async () => {
+ renderWrapper(openedBarProps, stateWithoutNativeFilters);
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
- // The filter value should not be cleared during initialization
- expect(updateDataMaskSpy).not.toHaveBeenCalled();
+ await addFilterFlow();
- updateDataMaskSpy.mockRestore();
-});
-
-test('FilterBar renders correctly when filter has complete extraFormData', async () => {
- const filterId = 'test-filter-complete';
- const props = createOpenedBarProps();
- const filter = createFilter({
- id: filterId,
- controlValues: { enableEmptyFilter: true },
- defaultDataMask: {
- filterState: { value: ['value1'] },
- extraFormData: {
- filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
- },
- },
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
});
- const dataMask = createDataMask(filterId, ['value1'], {
- filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
- });
- const state = createStateWithFilter(filter, dataMask);
- renderFilterBar(props, state);
+ 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: {},
+ },
+ };
- await act(async () => {
- jest.advanceTimersByTime(100);
+ const { container } = renderWrapper(openedBarProps, stateWithFilter);
+ expect(container).toBeInTheDocument();
});
- expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
-});
+ test('auto-applies filter when extraFormData is empty in applied state', async () => {
+ const filterId = 'test-filter-auto-apply';
+ const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
-test('handleClearAll dispatches updateDataMask with value undefined for filter_select', async () => {
- const filterId = 'NATIVE_FILTER-clear-select';
- const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
- const selectFilter = createFilter({
- id: filterId,
- name: 'Region',
- filterType: 'filter_select',
- targets: [{ datasetId: 7, column: { name: 'region' } }],
- defaultDataMask: { filterState: { value: null }, extraFormData: {} },
- chartsInScope: [18],
- });
- const stateWithSelect = {
- ...stateWithoutNativeFilters,
- dashboardInfo: {
- id: 1,
- dash_edit_perm: true,
- filterBarOrientation: FilterBarOrientation.Vertical,
- metadata: {
- native_filter_configuration: [selectFilter],
- chart_configuration: {},
+ const stateWithIncompleteFilter = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
},
- },
- dashboardState: {
- ...stateWithoutNativeFilters.dashboardState,
- activeTabs: ['ROOT_ID'],
- },
- dataMask: {
- [filterId]: createDataMask(filterId, ['East']),
- },
- nativeFilters: {
- filters: { [filterId]: selectFilter },
- filtersState: {},
- },
- };
+ dataMask: {
+ [filterId]: {
+ id: filterId,
+ filterState: { value: ['value1', 'value2'] },
+ extraFormData: {},
+ },
+ },
+ nativeFilters: {
+ filters: {
+ [filterId]: {
+ id: filterId,
+ name: 'Test Filter',
+ filterType: 'filter_select',
+ targets: [{ datasetId: 1, column: { name: 'test_column' } }],
+ defaultDataMask: {
+ filterState: { value: ['value1', 'value2'] },
+ extraFormData: {},
+ },
+ controlValues: {
+ enableEmptyFilter: true,
+ },
+ cascadeParentIds: [],
+ scope: {
+ rootPath: ['ROOT_ID'],
+ excluded: [],
+ },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [],
+ tabsInScope: [],
+ },
+ },
+ filtersState: {},
+ },
+ };
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithSelect);
- await act(async () => {
- jest.advanceTimersByTime(300);
- });
+ renderWrapper(openedBarProps, stateWithIncompleteFilter);
- const clearBtn = screen.getByTestId(getTestId('clear-button'));
- expect(clearBtn).not.toBeDisabled();
- await act(async () => {
- userEvent.click(clearBtn);
- });
+ await act(async () => {
+ jest.advanceTimersByTime(200);
+ });
- expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, {
- filterState: { value: undefined },
- extraFormData: {},
- });
- updateDataMaskSpy.mockRestore();
-});
+ expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
+ // The test name promises that auto-apply ran; without this assertion the
+ // test passed even when the dispatch was silently skipped.
+ expect(updateDataMaskSpy).toHaveBeenCalled();
-test('handleClearAll dispatches updateDataMask with [null, null] for filter_range', async () => {
- fetchMock.post('glob:*/api/v1/chart/data', {
- result: [{ data: [{ min: 0, max: 100 }] }],
- });
- const filterId = 'NATIVE_FILTER-clear-range';
- const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
- const rangeFilter = createFilter({
- id: filterId,
- name: 'Age',
- filterType: 'filter_range',
- targets: [{ datasetId: 7, column: { name: 'age' } }],
- defaultDataMask: { filterState: { value: null }, extraFormData: {} },
- chartsInScope: [18],
+ updateDataMaskSpy.mockRestore();
});
- const stateWithRange = {
- ...stateWithoutNativeFilters,
- dashboardInfo: {
- id: 1,
- dash_edit_perm: true,
- filterBarOrientation: FilterBarOrientation.Vertical,
- metadata: {
- native_filter_configuration: [rangeFilter],
- chart_configuration: {},
+
+ test('renders correctly when filter has complete extraFormData', async () => {
+ const filterId = 'test-filter-complete';
+ const stateWithCompleteFilter = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
},
- },
- dashboardState: {
- ...stateWithoutNativeFilters.dashboardState,
- activeTabs: ['ROOT_ID'],
- },
- dataMask: {
- [filterId]: createDataMask(filterId, [10, 50]),
- },
- nativeFilters: {
- filters: { [filterId]: rangeFilter },
- filtersState: {},
- },
- };
+ dataMask: {
+ [filterId]: {
+ id: filterId,
+ filterState: { value: ['value1'] },
+ extraFormData: {
+ filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
+ },
+ },
+ },
+ nativeFilters: {
+ filters: {
+ [filterId]: {
+ id: filterId,
+ name: 'Test Filter',
+ filterType: 'filter_select',
+ targets: [{ datasetId: 1, column: { name: 'test_column' } }],
+ defaultDataMask: {
+ filterState: { value: ['value1'] },
+ extraFormData: {
+ filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
+ },
+ },
+ controlValues: {
+ enableEmptyFilter: true,
+ },
+ cascadeParentIds: [],
+ scope: {
+ rootPath: ['ROOT_ID'],
+ excluded: [],
+ },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [],
+ tabsInScope: [],
+ },
+ },
+ filtersState: {},
+ },
+ };
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithRange);
- await act(async () => {
- jest.advanceTimersByTime(300);
- });
+ renderWrapper(openedBarProps, stateWithCompleteFilter);
- const clearBtn = screen.getByTestId(getTestId('clear-button'));
- expect(clearBtn).not.toBeDisabled();
- await act(async () => {
- userEvent.click(clearBtn);
- });
+ await act(async () => {
+ jest.advanceTimersByTime(100);
+ });
- expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, {
- filterState: { value: [null, null] },
- extraFormData: {},
+ expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
});
- updateDataMaskSpy.mockRestore();
-});
-test('handleClearAll only dispatches for filters present in dataMask', async () => {
- const idInMask = 'NATIVE_FILTER-has-value';
- const idNotInMask = 'NATIVE_FILTER-no-value';
- const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
- const filterInMask = createFilter({
- id: idInMask,
- name: 'A',
- filterType: 'filter_select',
- targets: [{ datasetId: 7, column: { name: 'x' } }],
- chartsInScope: [18],
- });
- const filterNotInMask = createFilter({
- id: idNotInMask,
- name: 'B',
- filterType: 'filter_select',
- targets: [{ datasetId: 7, column: { name: 'x' } }],
- chartsInScope: [18],
- });
- const stateWithTwoFilters = {
- ...stateWithoutNativeFilters,
- dashboardInfo: {
- id: 1,
- dash_edit_perm: true,
- filterBarOrientation: FilterBarOrientation.Vertical,
- metadata: {
- native_filter_configuration: [filterInMask, filterNotInMask],
- chart_configuration: {},
+ test('handleClearAll dispatches updateDataMask with value null for filter_select', async () => {
+ const filterId = 'NATIVE_FILTER-clear-select';
+ const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
+ const selectFilterConfig = {
+ id: filterId,
+ name: 'Region',
+ filterType: 'filter_select',
+ targets: [{ datasetId: 7, column: { name: 'region' } }],
+ defaultDataMask: { filterState: { value: null }, extraFormData: {} },
+ cascadeParentIds: [],
+ scope: { rootPath: ['ROOT_ID'], excluded: [] },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [18],
+ tabsInScope: [],
+ };
+ const stateWithSelect = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
+ filterBarOrientation: FilterBarOrientation.Vertical,
+ metadata: {
+ native_filter_configuration: [selectFilterConfig],
+ chart_configuration: {},
+ },
},
- },
- dashboardState: {
- ...stateWithoutNativeFilters.dashboardState,
- activeTabs: ['ROOT_ID'],
- },
- dataMask: {
- [idInMask]: createDataMask(idInMask, ['v']),
- },
- nativeFilters: {
- filters: {
- [idInMask]: filterInMask,
- [idNotInMask]: filterNotInMask,
+ dataMask: {
+ [filterId]: {
+ id: filterId,
+ filterState: { value: ['East'] },
+ extraFormData: {},
+ },
},
- filtersState: {},
- },
- };
+ };
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithTwoFilters);
- await act(async () => {
- jest.advanceTimersByTime(300);
- });
+ renderWrapper(openedBarProps, stateWithSelect);
+ await act(async () => {
+ jest.advanceTimersByTime(300);
+ });
- const clearBtn = screen.getByTestId(getTestId('clear-button'));
- await act(async () => {
- userEvent.click(clearBtn);
- });
+ const clearBtn = screen.getByTestId(getTestId('clear-button'));
+ expect(clearBtn).not.toBeDisabled();
+ await act(async () => {
+ userEvent.click(clearBtn);
Review Comment:
**Suggestion:** The click interaction is asynchronous, but it is called without `await` inside an async `act` block. This can let assertions run before the click side effects finish, making the test flaky. Await the click promise so state updates are fully flushed before assertions. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Filter clear-all select test may intermittently fail in CI.
- ⚠️ CI pipeline stability reduced due to flakey unit tests.
```
</details>
```suggestion
await userEvent.click(clearBtn);
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the Jest test file `FilterBar.test.tsx` at
`superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx:513-565`
(e.g. via `npm test -- FilterBar`).
2. The test `"handleClearAll dispatches updateDataMask with value null for filter_select"`
renders `FilterBar` and sets up `updateDataMaskSpy` (lines 513-547), then advances timers
with `await act(async () => { jest.advanceTimersByTime(300); });` (lines 549-552).
3. The test then clicks the clear button via `await act(async () => {
userEvent.click(clearBtn); });` at lines 554-558, where `userEvent` is imported from
`@testing-library/user-event` in `spec/helpers/testing-library.tsx:45,137` and its `click`
API is asynchronous but the returned promise is not awaited inside the `act` callback.
4. Because the async `userEvent.click(clearBtn)` is not awaited, the `act` callback
resolves before all click side effects (Redux `updateDataMask` dispatch from
`handleClearAll` in `FilterBar/index.tsx:90-155` and subsequent React updates) complete,
so the following assertion `expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId,
{...})` at lines 560-563 can occasionally run before the spy has been invoked, causing
intermittent, non-deterministic test failures.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fsrc%2Fdashboard%2Fcomponents%2FnativeFilters%2FFilterBar%2FFilterBar.test.tsx%0A%2A%2ALine%3A%2A%2A%20557%3A557%0A%2A%2AComment%3A%2A%2A%0A%09%2APossible%20Bug%3A%20The%20click%20interaction%20is%20asynchronous%2C%20but%20it%20is%20called%20without%20%60await%60%20inside%20an%20async%20%60act%60%20block.%20This%20can%20let%20assertions%20run%20before%20the%20click%20side%20effects%20finish%2C%20making%20the%20test%20flaky.%20Await%20the%20click%20promise%20so%20state%20updates%20are%20fully%20flushed%20before%20assertions.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0A) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&
prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fsrc%2Fdashboard%2Fcomponents%2FnativeFilters%2FFilterBar%2FFilterBar.test.tsx%0A%2A%2ALine%3A%2A%2A%20557%3A557%0A%2A%2AComment%3A%2A%2A%0A%09%2APossible%20Bug%3A%20The%20click%20interaction%20is%20asynchronous%2C%20but%20it%20is%20called%20without%20%60await%60%20inside%20an%20async%20%60act%60%20block.%20This%20can%20let%20assertions%20run%20before%20the%20click%20side%20effects%20finish%2C%20making%20the%20test%20flaky.%20Await%20the%20click%20promise%20so%20state%20updates%20are%20fully%20flushed%20before%20assertions.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0A)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx
**Line:** 557:557
**Comment:**
*Possible Bug: The click interaction is asynchronous, but it is called without `await` inside an async `act` block. This can let assertions run before the click side effects finish, making the test flaky. Await the click promise so state updates are fully flushed before assertions.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39231&comment_hash=06eb6579b6cf0b0ca13c5350bbe5aef004760b5aea1c974692d552ed9a6f9deb&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39231&comment_hash=06eb6579b6cf0b0ca13c5350bbe5aef004760b5aea1c974692d552ed9a6f9deb&reaction=dislike'>👎</a>
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx:
##########
@@ -109,684 +139,550 @@ function createMockApi(filterName = 'Time filter 1') {
],
};
});
-}
-
-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,
- };
-}
-
-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],
+ 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' },
},
- ...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: getTimeRangeNoFilterMockUrl },
+ );
+
+ fetchMock.removeRoute(getTimeRangeLastDayMockUrl);
+ fetchMock.get(
+ getTimeRangeLastDayMockUrl,
+ {
+ result: {
+ since: '2021-04-13T00:00:00',
+ until: '2021-04-14T00:00:00',
+ timeRange: 'Last day',
+ },
},
- },
- { 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: 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.lastWeek },
- );
-}
-
-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,
- },
- );
-}
-
-test('FilterBar renders without crashing', () => {
- const props = createClosedBarProps();
- const { container } = renderFilterBar(props);
- expect(container).toBeInTheDocument();
-});
+ { name: getTimeRangeLastWeekMockUrl },
+ );
-test('FilterBar renders "Filters and controls" heading', () => {
- const props = createClosedBarProps();
- renderFilterBar(props);
- 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('FilterBar renders "Apply filters" button', () => {
- const props = createClosedBarProps();
- renderFilterBar(props);
- 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('FilterBar renders filter icon', () => {
- const props = createClosedBarProps();
- renderFilterBar(props);
- 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);
-
- const collapse = screen.getByRole('img', { name: 'vertical-align' });
- expect(toggleFiltersBar).not.toHaveBeenCalled();
-
- userEvent.click(collapse);
- expect(toggleFiltersBar).toHaveBeenCalled();
-});
-
-test('FilterBar opens when expand button is clicked', () => {
- const toggleFiltersBar = jest.fn();
- const props = createClosedBarProps(toggleFiltersBar);
- renderFilterBar(props);
-
- expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
- expect(screen.getByTestId(getTestId('expand-button'))).toBeInTheDocument();
-
- userEvent.click(screen.getByTestId(getTestId('collapsable')));
- expect(toggleFiltersBar).toHaveBeenCalledWith(true);
-});
-
-test('FilterBar hides edit filter button when user lacks permissions', () => {
- const props = createOpenedBarProps();
- const stateWithoutPermissions = {
- ...stateWithoutNativeFilters,
- dashboardInfo: { metadata: {} },
- };
+ mockedMakeApi.mockReturnValue(mockApi);
+ });
- renderFilterBar(props, stateWithoutPermissions);
+ 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,
+ },
+ );
- expect(
- screen.queryByTestId(getTestId('create-filter')),
- ).not.toBeInTheDocument();
-});
+ test('should render', () => {
+ const { container } = renderWrapper();
+ expect(container).toBeInTheDocument();
+ });
-test('FilterBar closes when collapse button is clicked', () => {
- const toggleFiltersBar = jest.fn();
- const props = createOpenedBarProps(toggleFiltersBar);
- renderFilterBar(props);
+ test('should render the "Filters and controls" heading', () => {
+ renderWrapper();
+ expect(screen.getByText('Filters and controls')).toBeInTheDocument();
+ });
- const collapseButton = screen.getByTestId(getTestId('collapse-button'));
- expect(collapseButton).toBeInTheDocument();
+ test('should render the "Clear all" option', () => {
+ renderWrapper();
+ expect(screen.getByText('Clear all')).toBeInTheDocument();
+ });
- userEvent.click(collapseButton);
- expect(toggleFiltersBar).toHaveBeenCalledWith(false);
-});
+ test('should render the "Apply filters" option', () => {
+ renderWrapper();
+ expect(screen.getByText('Apply filters')).toBeInTheDocument();
+ });
-test('FilterBar disables buttons when there are no filters', () => {
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithoutNativeFilters);
+ test('should render the collapse icon', () => {
+ renderWrapper();
+ expect(
+ screen.getByRole('img', { name: 'vertical-align' }),
+ ).toBeInTheDocument();
+ });
- expect(screen.getByTestId(getTestId('clear-button'))).toBeDisabled();
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
-});
+ test('should render the filter icon', () => {
+ renderWrapper();
+ expect(screen.getByRole('img', { name: 'filter' })).toBeInTheDocument();
+ });
-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 },
- },
- };
+ test('should toggle', () => {
+ renderWrapper();
+ const collapse = screen.getByRole('img', {
+ name: 'vertical-align',
+ });
+ expect(toggleFiltersBar).not.toHaveBeenCalled();
+ userEvent.click(collapse);
+ expect(toggleFiltersBar).toHaveBeenCalled();
+ });
- renderFilterBar(props, stateWithDivider);
+ test('open filter bar', () => {
+ renderWrapper();
+ expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
+ expect(screen.getByTestId(getTestId('expand-button'))).toBeInTheDocument();
- await act(async () => {
- jest.advanceTimersByTime(1000);
+ userEvent.click(screen.getByTestId(getTestId('collapsable')));
+ expect(toggleFiltersBar).toHaveBeenCalledWith(true);
});
- const title = await screen.findByText('Select time range');
- const description = await screen.findByText('Select year/month etc..');
+ test('collapsed filter bar should have aria-expanded=false', () => {
+ renderWrapper();
+ const collapsedBar = screen.getByTestId(getTestId('collapsable'));
+ expect(collapsedBar).toHaveAttribute('aria-expanded', 'false');
+ expect(collapsedBar).toHaveAttribute('aria-label', 'Expand filters');
+ });
- 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('no edit filter button by disabled permissions', () => {
+ renderWrapper(openedBarProps, {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: { metadata: {} },
+ });
-test('FilterBar apply button is disabled after creating a filter', async () => {
- setupTimeRangeMocks();
- mockedMakeApi.mockReturnValue(createMockApi());
+ expect(
+ screen.queryByTestId(getTestId('create-filter')),
+ ).not.toBeInTheDocument();
+ });
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithoutNativeFilters);
+ test('close filter bar', () => {
+ renderWrapper(openedBarProps);
+ const collapseButton = screen.getByTestId(getTestId('collapse-button'));
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
+ expect(collapseButton).toBeInTheDocument();
+ userEvent.click(collapseButton);
- // 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(toggleFiltersBar).toHaveBeenCalledWith(false);
+ });
- expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
-});
+ test('no filters', () => {
+ renderWrapper(openedBarProps, stateWithoutNativeFilters);
-test('FilterBar renders without errors when filter has required controlValues', () => {
- const props = createOpenedBarProps();
- const filter = createFilter({
- id: 'test-filter',
- controlValues: { enableEmptyFilter: true },
+ expect(screen.getByTestId(getTestId('clear-button'))).toBeDisabled();
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
});
- const dataMask = createDataMask('test-filter', undefined, {});
- const state = createStateWithFilter(filter, dataMask);
- const { container } = renderFilterBar(props, state);
- expect(container).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,
+ },
+ },
+ };
-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: {},
- },
- });
+ renderWrapper(openedBarProps, stateWithDivider);
- const dataMask = createDataMask(filterId, ['value1'], {});
- const state = createStateWithFilter(filter, dataMask);
+ await act(async () => {
+ jest.advanceTimersByTime(1000); // 1s
+ });
- renderFilterBar(props, state);
+ const title = await screen.findByText('Select time range');
+ const description = await screen.findByText('Select year/month etc..');
- await act(async () => {
- jest.advanceTimersByTime(300);
+ 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();
});
- expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
- expect(screen.getByText('Filters and controls')).toBeInTheDocument();
+ test('create filter and apply it flow', async () => {
+ renderWrapper(openedBarProps, stateWithoutNativeFilters);
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
- // The filter value should not be cleared during initialization
- expect(updateDataMaskSpy).not.toHaveBeenCalled();
+ await addFilterFlow();
- updateDataMaskSpy.mockRestore();
-});
-
-test('FilterBar renders correctly when filter has complete extraFormData', async () => {
- const filterId = 'test-filter-complete';
- const props = createOpenedBarProps();
- const filter = createFilter({
- id: filterId,
- controlValues: { enableEmptyFilter: true },
- defaultDataMask: {
- filterState: { value: ['value1'] },
- extraFormData: {
- filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
- },
- },
+ expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
});
- const dataMask = createDataMask(filterId, ['value1'], {
- filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
- });
- const state = createStateWithFilter(filter, dataMask);
- renderFilterBar(props, state);
+ 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: {},
+ },
+ };
- await act(async () => {
- jest.advanceTimersByTime(100);
+ const { container } = renderWrapper(openedBarProps, stateWithFilter);
+ expect(container).toBeInTheDocument();
});
- expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
-});
+ test('auto-applies filter when extraFormData is empty in applied state', async () => {
+ const filterId = 'test-filter-auto-apply';
+ const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
-test('handleClearAll dispatches updateDataMask with value undefined for filter_select', async () => {
- const filterId = 'NATIVE_FILTER-clear-select';
- const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
- const selectFilter = createFilter({
- id: filterId,
- name: 'Region',
- filterType: 'filter_select',
- targets: [{ datasetId: 7, column: { name: 'region' } }],
- defaultDataMask: { filterState: { value: null }, extraFormData: {} },
- chartsInScope: [18],
- });
- const stateWithSelect = {
- ...stateWithoutNativeFilters,
- dashboardInfo: {
- id: 1,
- dash_edit_perm: true,
- filterBarOrientation: FilterBarOrientation.Vertical,
- metadata: {
- native_filter_configuration: [selectFilter],
- chart_configuration: {},
+ const stateWithIncompleteFilter = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
},
- },
- dashboardState: {
- ...stateWithoutNativeFilters.dashboardState,
- activeTabs: ['ROOT_ID'],
- },
- dataMask: {
- [filterId]: createDataMask(filterId, ['East']),
- },
- nativeFilters: {
- filters: { [filterId]: selectFilter },
- filtersState: {},
- },
- };
+ dataMask: {
+ [filterId]: {
+ id: filterId,
+ filterState: { value: ['value1', 'value2'] },
+ extraFormData: {},
+ },
+ },
+ nativeFilters: {
+ filters: {
+ [filterId]: {
+ id: filterId,
+ name: 'Test Filter',
+ filterType: 'filter_select',
+ targets: [{ datasetId: 1, column: { name: 'test_column' } }],
+ defaultDataMask: {
+ filterState: { value: ['value1', 'value2'] },
+ extraFormData: {},
+ },
+ controlValues: {
+ enableEmptyFilter: true,
+ },
+ cascadeParentIds: [],
+ scope: {
+ rootPath: ['ROOT_ID'],
+ excluded: [],
+ },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [],
+ tabsInScope: [],
+ },
+ },
+ filtersState: {},
+ },
+ };
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithSelect);
- await act(async () => {
- jest.advanceTimersByTime(300);
- });
+ renderWrapper(openedBarProps, stateWithIncompleteFilter);
- const clearBtn = screen.getByTestId(getTestId('clear-button'));
- expect(clearBtn).not.toBeDisabled();
- await act(async () => {
- userEvent.click(clearBtn);
- });
+ await act(async () => {
+ jest.advanceTimersByTime(200);
+ });
- expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, {
- filterState: { value: undefined },
- extraFormData: {},
- });
- updateDataMaskSpy.mockRestore();
-});
+ expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
+ // The test name promises that auto-apply ran; without this assertion the
+ // test passed even when the dispatch was silently skipped.
+ expect(updateDataMaskSpy).toHaveBeenCalled();
-test('handleClearAll dispatches updateDataMask with [null, null] for filter_range', async () => {
- fetchMock.post('glob:*/api/v1/chart/data', {
- result: [{ data: [{ min: 0, max: 100 }] }],
- });
- const filterId = 'NATIVE_FILTER-clear-range';
- const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
- const rangeFilter = createFilter({
- id: filterId,
- name: 'Age',
- filterType: 'filter_range',
- targets: [{ datasetId: 7, column: { name: 'age' } }],
- defaultDataMask: { filterState: { value: null }, extraFormData: {} },
- chartsInScope: [18],
+ updateDataMaskSpy.mockRestore();
});
- const stateWithRange = {
- ...stateWithoutNativeFilters,
- dashboardInfo: {
- id: 1,
- dash_edit_perm: true,
- filterBarOrientation: FilterBarOrientation.Vertical,
- metadata: {
- native_filter_configuration: [rangeFilter],
- chart_configuration: {},
+
+ test('renders correctly when filter has complete extraFormData', async () => {
+ const filterId = 'test-filter-complete';
+ const stateWithCompleteFilter = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
},
- },
- dashboardState: {
- ...stateWithoutNativeFilters.dashboardState,
- activeTabs: ['ROOT_ID'],
- },
- dataMask: {
- [filterId]: createDataMask(filterId, [10, 50]),
- },
- nativeFilters: {
- filters: { [filterId]: rangeFilter },
- filtersState: {},
- },
- };
+ dataMask: {
+ [filterId]: {
+ id: filterId,
+ filterState: { value: ['value1'] },
+ extraFormData: {
+ filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
+ },
+ },
+ },
+ nativeFilters: {
+ filters: {
+ [filterId]: {
+ id: filterId,
+ name: 'Test Filter',
+ filterType: 'filter_select',
+ targets: [{ datasetId: 1, column: { name: 'test_column' } }],
+ defaultDataMask: {
+ filterState: { value: ['value1'] },
+ extraFormData: {
+ filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
+ },
+ },
+ controlValues: {
+ enableEmptyFilter: true,
+ },
+ cascadeParentIds: [],
+ scope: {
+ rootPath: ['ROOT_ID'],
+ excluded: [],
+ },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [],
+ tabsInScope: [],
+ },
+ },
+ filtersState: {},
+ },
+ };
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithRange);
- await act(async () => {
- jest.advanceTimersByTime(300);
- });
+ renderWrapper(openedBarProps, stateWithCompleteFilter);
- const clearBtn = screen.getByTestId(getTestId('clear-button'));
- expect(clearBtn).not.toBeDisabled();
- await act(async () => {
- userEvent.click(clearBtn);
- });
+ await act(async () => {
+ jest.advanceTimersByTime(100);
+ });
- expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, {
- filterState: { value: [null, null] },
- extraFormData: {},
+ expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
});
- updateDataMaskSpy.mockRestore();
-});
-test('handleClearAll only dispatches for filters present in dataMask', async () => {
- const idInMask = 'NATIVE_FILTER-has-value';
- const idNotInMask = 'NATIVE_FILTER-no-value';
- const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
- const filterInMask = createFilter({
- id: idInMask,
- name: 'A',
- filterType: 'filter_select',
- targets: [{ datasetId: 7, column: { name: 'x' } }],
- chartsInScope: [18],
- });
- const filterNotInMask = createFilter({
- id: idNotInMask,
- name: 'B',
- filterType: 'filter_select',
- targets: [{ datasetId: 7, column: { name: 'x' } }],
- chartsInScope: [18],
- });
- const stateWithTwoFilters = {
- ...stateWithoutNativeFilters,
- dashboardInfo: {
- id: 1,
- dash_edit_perm: true,
- filterBarOrientation: FilterBarOrientation.Vertical,
- metadata: {
- native_filter_configuration: [filterInMask, filterNotInMask],
- chart_configuration: {},
+ test('handleClearAll dispatches updateDataMask with value null for filter_select', async () => {
+ const filterId = 'NATIVE_FILTER-clear-select';
+ const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
+ const selectFilterConfig = {
+ id: filterId,
+ name: 'Region',
+ filterType: 'filter_select',
+ targets: [{ datasetId: 7, column: { name: 'region' } }],
+ defaultDataMask: { filterState: { value: null }, extraFormData: {} },
+ cascadeParentIds: [],
+ scope: { rootPath: ['ROOT_ID'], excluded: [] },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [18],
+ tabsInScope: [],
+ };
+ const stateWithSelect = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
+ filterBarOrientation: FilterBarOrientation.Vertical,
+ metadata: {
+ native_filter_configuration: [selectFilterConfig],
+ chart_configuration: {},
+ },
},
- },
- dashboardState: {
- ...stateWithoutNativeFilters.dashboardState,
- activeTabs: ['ROOT_ID'],
- },
- dataMask: {
- [idInMask]: createDataMask(idInMask, ['v']),
- },
- nativeFilters: {
- filters: {
- [idInMask]: filterInMask,
- [idNotInMask]: filterNotInMask,
+ dataMask: {
+ [filterId]: {
+ id: filterId,
+ filterState: { value: ['East'] },
+ extraFormData: {},
+ },
},
- filtersState: {},
- },
- };
+ };
- const props = createOpenedBarProps();
- renderFilterBar(props, stateWithTwoFilters);
- await act(async () => {
- jest.advanceTimersByTime(300);
- });
+ renderWrapper(openedBarProps, stateWithSelect);
+ await act(async () => {
+ jest.advanceTimersByTime(300);
+ });
- const clearBtn = screen.getByTestId(getTestId('clear-button'));
- await act(async () => {
- userEvent.click(clearBtn);
- });
+ const clearBtn = screen.getByTestId(getTestId('clear-button'));
+ expect(clearBtn).not.toBeDisabled();
+ await act(async () => {
+ userEvent.click(clearBtn);
+ });
- expect(updateDataMaskSpy).toHaveBeenCalledTimes(1);
- expect(updateDataMaskSpy).toHaveBeenCalledWith(idInMask, {
- filterState: { value: undefined },
- extraFormData: {},
+ expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, {
+ filterState: { value: undefined },
+ extraFormData: {},
+ });
+ updateDataMaskSpy.mockRestore();
});
- updateDataMaskSpy.mockRestore();
-});
-test('FilterBar Clear All only clears in-scope filters, not out-of-scope ones', async () => {
- const inScopeFilterId = 'NATIVE_FILTER-in-scope';
- const outOfScopeRequiredFilterId = 'NATIVE_FILTER-out-of-scope-required';
- const outOfScopeNonRequiredFilterId =
- 'NATIVE_FILTER-out-of-scope-non-required';
- const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
-
- const dashboardLayoutWithTabs = {
- ROOT_ID: { id: 'ROOT_ID', type: 'ROOT', children: ['TABS-1'] },
- 'TABS-1': {
- id: 'TABS-1',
- type: 'TABS',
- children: ['TAB-active', 'TAB-inactive'],
- },
- 'TAB-active': {
- id: 'TAB-active',
- type: 'TAB',
- children: ['CHART_ROW-1'],
- meta: { text: 'Active Tab' },
- parents: ['ROOT_ID', 'TABS-1'],
- },
- 'TAB-inactive': {
- id: 'TAB-inactive',
- type: 'TAB',
- children: ['CHART_ROW-2'],
- meta: { text: 'Inactive Tab' },
- parents: ['ROOT_ID', 'TABS-1'],
- },
- 'CHART_ROW-1': {
- id: 'CHART_ROW-1',
- type: 'CHART',
- meta: { chartId: 1 },
- parents: ['ROOT_ID', 'TABS-1', 'TAB-active'],
- },
- 'CHART_ROW-2': {
- id: 'CHART_ROW-2',
- type: 'CHART',
- meta: { chartId: 2 },
- parents: ['ROOT_ID', 'TABS-1', 'TAB-inactive'],
- },
- };
+ test('handleClearAll dispatches updateDataMask with value [null, null] for filter_range', async () => {
+ fetchMock.post('glob:*/api/v1/chart/data', {
+ result: [{ data: [{ min: 0, max: 100 }] }],
+ });
+ const filterId = 'NATIVE_FILTER-clear-range';
+ const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
+ const rangeFilterConfig = {
+ id: filterId,
+ name: 'Age',
+ filterType: 'filter_range',
+ targets: [{ datasetId: 7, column: { name: 'age' } }],
+ defaultDataMask: { filterState: { value: null }, extraFormData: {} },
+ cascadeParentIds: [],
+ scope: { rootPath: ['ROOT_ID'], excluded: [] },
+ type: 'NATIVE_FILTER',
+ description: '',
+ chartsInScope: [18],
+ tabsInScope: [],
+ };
+ const stateWithRange = {
+ ...stateWithoutNativeFilters,
+ dashboardInfo: {
+ id: 1,
+ dash_edit_perm: true,
+ filterBarOrientation: FilterBarOrientation.Vertical,
+ metadata: {
+ native_filter_configuration: [rangeFilterConfig],
+ chart_configuration: {},
+ },
+ },
+ dataMask: {
+ [filterId]: {
+ id: filterId,
+ filterState: { value: [10, 50] },
+ extraFormData: {},
+ },
+ },
+ };
- const inScopeFilter = createFilter({
- id: inScopeFilterId,
- name: 'In Scope Filter',
- targets: [{ datasetId: 1, column: { name: 'column1' } }],
- controlValues: { enableEmptyFilter: false },
- chartsInScope: [1],
- tabsInScope: ['TAB-active'],
- });
+ renderWrapper(openedBarProps, stateWithRange);
+ await act(async () => {
+ jest.advanceTimersByTime(300);
+ });
- const outOfScopeRequiredFilter = createFilter({
- id: outOfScopeRequiredFilterId,
- name: 'Out of Scope Required Filter',
- targets: [{ datasetId: 1, column: { name: 'column2' } }],
- controlValues: { enableEmptyFilter: true },
- chartsInScope: [2],
- tabsInScope: ['TAB-inactive'],
- });
+ const clearBtn = screen.getByTestId(getTestId('clear-button'));
+ expect(clearBtn).not.toBeDisabled();
+ await act(async () => {
+ userEvent.click(clearBtn);
Review Comment:
**Suggestion:** This click is also asynchronous and currently not awaited, so the test may continue before Redux dispatches and component updates complete. Add `await` to avoid intermittent failures. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Filter clear-all range test may intermittently fail in CI.
- ⚠️ CI reliability weakened by non-deterministic FilterBar tests.
```
</details>
```suggestion
await userEvent.click(clearBtn);
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the Jest test file `FilterBar.test.tsx` at
`superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx:567-621`
(e.g. via `npm test -- FilterBar`).
2. The test `"handleClearAll dispatches updateDataMask with value [null, null] for
filter_range"` renders `FilterBar` with a range filter and advances timers with `await
act(async () => { jest.advanceTimersByTime(300); });` (lines 606-609).
3. It then invokes `await act(async () => { userEvent.click(clearBtn); });` at lines
611-615 to clear the filter, where `userEvent.click` comes from
`@testing-library/user-event` via `spec/helpers/testing-library.tsx:45,137` and is
asynchronous, but its promise is not awaited inside the `act` callback.
4. Because `userEvent.click(clearBtn)` is not awaited, the `act` block can finish before
all click-driven updates (including the `updateDataMask` dispatch from `handleClearAll` in
`FilterBar/index.tsx:90-155`) are flushed, so the subsequent assertion
`expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, { filterState: { value: [null,
null] }, extraFormData: {} });` at lines 617-620 may occasionally execute before the spy
is called, leading to racey, flaky failures in this test.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fsrc%2Fdashboard%2Fcomponents%2FnativeFilters%2FFilterBar%2FFilterBar.test.tsx%0A%2A%2ALine%3A%2A%2A%20614%3A614%0A%2A%2AComment%3A%2A%2A%0A%09%2APossible%20Bug%3A%20This%20click%20is%20also%20asynchronous%20and%20currently%20not%20awaited%2C%20so%20the%20test%20may%20continue%20before%20Redux%20dispatches%20and%20component%20updates%20complete.%20Add%20%60await%60%20to%20avoid%20intermittent%20failures.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0A) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fsrc%2Fd
ashboard%2Fcomponents%2FnativeFilters%2FFilterBar%2FFilterBar.test.tsx%0A%2A%2ALine%3A%2A%2A%20614%3A614%0A%2A%2AComment%3A%2A%2A%0A%09%2APossible%20Bug%3A%20This%20click%20is%20also%20asynchronous%20and%20currently%20not%20awaited%2C%20so%20the%20test%20may%20continue%20before%20Redux%20dispatches%20and%20component%20updates%20complete.%20Add%20%60await%60%20to%20avoid%20intermittent%20failures.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0A)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx
**Line:** 614:614
**Comment:**
*Possible Bug: This click is also asynchronous and currently not awaited, so the test may continue before Redux dispatches and component updates complete. Add `await` to avoid intermittent failures.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39231&comment_hash=6f7f3a7f6e085061b3df8125a76f86dad1808410b7b7eeb6951da1dcc5d7a142&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39231&comment_hash=6f7f3a7f6e085061b3df8125a76f86dad1808410b7b7eeb6951da1dcc5d7a142&reaction=dislike'>👎</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(a11y): WCAG 1.3.3 — add aria-pressed/expanded to toggle buttons [superset]
Posted by "Aitema-gmbh (via GitHub)" <gi...@apache.org>.
Aitema-gmbh commented on PR #39231:
URL: https://github.com/apache/superset/pull/39231#issuecomment-4276323701
Thanks @bito-code-review @codeant-ai-for-open-source — addressed the remaining findings in `e71b0d57` (earlier follow-ups `aabe2d99` and `7a006443` already covered the keyboard handler on the vertical filter bar, the incorrect `aria-pressed` on the dashboard header button, the test selectors, and the dynamic `aria-expanded` on `Vertical.tsx`):
- `AutoRefreshIndicator`: accessible name is now the static `t('Toggle auto-refresh')` and `aria-pressed` carries the state. A dynamic name made screen readers announce two different buttons rather than one toggle whose state changed.
- `FilterBar/Header/index.tsx`: `aria-expanded="true"` is now explicit (was the boolean-shorthand form). The `Header` component only renders while the filter bar is expanded, so `true` is the correct constant value.
- `FilterBar.test.tsx`:
- `addFilterFlow` promoted to real `async/await` with `findBy...` queries so each step waits for the previous UI transition — this was the source of the `TODO: fix this flaky test` comment.
- Restored the `'All filters (1)'` post-save assertion.
- Added the missing `expect(updateDataMaskSpy).toHaveBeenCalled()` to the auto-apply test — without it the spy was created and restored without verifying the data-mask dispatch, so a silent auto-apply regression would still pass CI.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org