You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2025/01/21 21:14:16 UTC
(superset) 05/05: Revert "getting tests to pass"
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch removing-DASHBOARD_CROSS_FILTERS-flag
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 7d2eff9a4d8d918e7483e0eb6270dfb11b3a871e
Author: Evan Rusackas <ev...@rusackas.com>
AuthorDate: Tue Jan 21 14:12:03 2025 -0700
Revert "getting tests to pass"
This reverts commit d6f78f0f7c7ade0f08a80e822855f1e6485f46df.
---
.../src/dashboard/actions/dashboardState.test.js | 15 +--
.../src/dashboard/components/Dashboard.test.jsx | 116 +++++----------------
.../OverwriteConfirmModal.test.tsx | 41 ++------
.../FilterBarSettings/FilterBarSettings.test.tsx | 24 ++---
.../components/nativeFilters/utils.test.ts | 20 ----
.../dashboard/components/nativeFilters/utils.ts | 22 +---
6 files changed, 47 insertions(+), 191 deletions(-)
diff --git a/superset-frontend/src/dashboard/actions/dashboardState.test.js b/superset-frontend/src/dashboard/actions/dashboardState.test.js
index 0bb4fbffda..4e30c38fb1 100644
--- a/superset-frontend/src/dashboard/actions/dashboardState.test.js
+++ b/superset-frontend/src/dashboard/actions/dashboardState.test.js
@@ -57,7 +57,6 @@ describe('dashboardState actions', () => {
present: mockDashboardData.positions,
future: {},
},
- charts: {},
};
const newDashboardData = mockDashboardData;
@@ -154,12 +153,7 @@ describe('dashboardState actions', () => {
it('dispatches SET_OVERRIDE_CONFIRM when an inspect value has diff', async () => {
const id = 192;
- const { getState, dispatch } = setup({
- charts: {
- 123: { id: 123 },
- 456: { id: 456 },
- },
- });
+ const { getState, dispatch } = setup();
const thunk = saveDashboardRequest(
newDashboardData,
id,
@@ -178,12 +172,7 @@ describe('dashboardState actions', () => {
it('should post dashboard data with after confirm the overwrite values', async () => {
const id = 192;
- const { getState, dispatch } = setup({
- charts: {
- 123: { id: 123 },
- 456: { id: 456 },
- },
- });
+ const { getState, dispatch } = setup();
const confirmedDashboardData = {
...newDashboardData,
css: updatedCss,
diff --git a/superset-frontend/src/dashboard/components/Dashboard.test.jsx b/superset-frontend/src/dashboard/components/Dashboard.test.jsx
index 3d841661b1..e3421ee040 100644
--- a/superset-frontend/src/dashboard/components/Dashboard.test.jsx
+++ b/superset-frontend/src/dashboard/components/Dashboard.test.jsx
@@ -48,7 +48,6 @@ describe('Dashboard', () => {
removeSliceFromDashboard() {},
triggerQuery() {},
logEvent() {},
- clearDataMaskState() {},
},
dashboardState,
dashboardInfo,
@@ -62,7 +61,6 @@ describe('Dashboard', () => {
userId: dashboardInfo.userId,
impressionId: 'id',
loadStats: {},
- dashboardId: 123,
};
const ChildrenComponent = () => <div>Test</div>;
@@ -127,30 +125,9 @@ describe('Dashboard', () => {
let refreshSpy;
beforeEach(() => {
- jest.clearAllMocks();
- getRelatedCharts.mockReset();
-
- // Create a deep copy of OVERRIDE_FILTERS for prevProps
- const initialFilters = JSON.parse(JSON.stringify(OVERRIDE_FILTERS));
-
- // Setup with required props
- wrapper = setup({
- activeFilters: initialFilters,
- editMode: false, // Ensure not in edit mode
- chartConfiguration: {}, // Ensure chartConfiguration exists
- ownDataCharts: {},
- });
- wrapper.instance().appliedFilters = initialFilters;
-
- // Set up prevProps with the initial filters
- prevProps = {
- ...wrapper.instance().props,
- activeFilters: initialFilters,
- editMode: false,
- chartConfiguration: {},
- ownDataCharts: {},
- };
-
+ wrapper = setup({ activeFilters: OVERRIDE_FILTERS });
+ wrapper.instance().appliedFilters = OVERRIDE_FILTERS;
+ prevProps = wrapper.instance().props;
refreshSpy = sinon.spy(wrapper.instance(), 'refreshCharts');
});
@@ -171,36 +148,27 @@ describe('Dashboard', () => {
});
it('should not call refresh when there is no change', () => {
- const sameFilters = JSON.parse(JSON.stringify(OVERRIDE_FILTERS));
wrapper.setProps({
- activeFilters: sameFilters,
+ activeFilters: OVERRIDE_FILTERS,
});
wrapper.instance().componentDidUpdate(prevProps);
expect(refreshSpy.callCount).toBe(0);
- expect(wrapper.instance().appliedFilters).toEqual(sameFilters);
+ expect(wrapper.instance().appliedFilters).toBe(OVERRIDE_FILTERS);
});
it('should call refresh when native filters changed', () => {
getRelatedCharts.mockReturnValue([230]);
-
- const newFilters = {
- ...OVERRIDE_FILTERS,
- ...getAllActiveFilters({
- dataMask: dataMaskWith1Filter,
- nativeFilters: singleNativeFiltersState.filters,
- allSliceIds: [227, 229, 230],
- }),
- };
-
wrapper.setProps({
- ...wrapper.instance().props,
- activeFilters: newFilters,
- editMode: false,
- chartConfiguration: {},
+ activeFilters: {
+ ...OVERRIDE_FILTERS,
+ ...getAllActiveFilters({
+ dataMask: dataMaskWith1Filter,
+ nativeFilters: singleNativeFiltersState.filters,
+ allSliceIds: [227, 229, 230],
+ }),
+ },
});
-
wrapper.instance().componentDidUpdate(prevProps);
-
expect(refreshSpy.callCount).toBe(1);
expect(wrapper.instance().appliedFilters).toEqual({
...OVERRIDE_FILTERS,
@@ -222,51 +190,34 @@ describe('Dashboard', () => {
it('should call refresh if a filter is added', () => {
getRelatedCharts.mockReturnValue([1]);
-
const newFilter = {
gender: { values: ['boy', 'girl'], scope: [1] },
};
-
- const newProps = {
- ...wrapper.instance().props,
+ wrapper.setProps({
activeFilters: newFilter,
- };
- wrapper.setProps(newProps);
- wrapper.instance().componentDidUpdate(prevProps);
-
+ });
expect(refreshSpy.callCount).toBe(1);
expect(wrapper.instance().appliedFilters).toEqual(newFilter);
});
it('should call refresh if a filter is removed', () => {
getRelatedCharts.mockReturnValue([]);
-
- const newProps = {
- ...wrapper.instance().props,
+ wrapper.setProps({
activeFilters: {},
- };
- wrapper.setProps(newProps);
- wrapper.instance().componentDidUpdate(prevProps);
-
+ });
expect(refreshSpy.callCount).toBe(1);
expect(wrapper.instance().appliedFilters).toEqual({});
});
it('should call refresh if a filter is changed', () => {
getRelatedCharts.mockReturnValue([1]);
-
const newFilters = {
...OVERRIDE_FILTERS,
'1_region': { values: ['Canada'], scope: [1] },
};
-
- const newProps = {
- ...wrapper.instance().props,
+ wrapper.setProps({
activeFilters: newFilters,
- };
- wrapper.setProps(newProps);
- wrapper.instance().componentDidUpdate(prevProps);
-
+ });
expect(refreshSpy.callCount).toBe(1);
expect(wrapper.instance().appliedFilters).toEqual(newFilters);
expect(refreshSpy.getCall(0).args[0]).toEqual([1]);
@@ -274,58 +225,41 @@ describe('Dashboard', () => {
it('should call refresh with multiple chart ids', () => {
getRelatedCharts.mockReturnValue([1, 2]);
-
const newFilters = {
...OVERRIDE_FILTERS,
'2_country_name': { values: ['New Country'], scope: [1, 2] },
};
-
- const newProps = {
- ...wrapper.instance().props,
+ wrapper.setProps({
activeFilters: newFilters,
- };
- wrapper.setProps(newProps);
- wrapper.instance().componentDidUpdate(prevProps);
-
+ });
expect(refreshSpy.callCount).toBe(1);
expect(wrapper.instance().appliedFilters).toEqual(newFilters);
expect(refreshSpy.getCall(0).args[0]).toEqual([1, 2]);
});
it('should call refresh if a filter scope is changed', () => {
- getRelatedCharts.mockReturnValue([2]);
-
const newFilters = {
...OVERRIDE_FILTERS,
'3_country_name': { values: ['USA'], scope: [2] },
};
- const newProps = {
- ...wrapper.instance().props,
+ wrapper.setProps({
activeFilters: newFilters,
- };
- wrapper.setProps(newProps);
- wrapper.instance().componentDidUpdate(prevProps);
-
+ });
expect(refreshSpy.callCount).toBe(1);
expect(refreshSpy.getCall(0).args[0]).toEqual([2]);
});
it('should call refresh with empty [] if a filter is changed but scope is not applicable', () => {
getRelatedCharts.mockReturnValue([]);
-
const newFilters = {
...OVERRIDE_FILTERS,
'3_country_name': { values: ['CHINA'], scope: [] },
};
- const newProps = {
- ...wrapper.instance().props,
+ wrapper.setProps({
activeFilters: newFilters,
- };
- wrapper.setProps(newProps);
- wrapper.instance().componentDidUpdate(prevProps);
-
+ });
expect(refreshSpy.callCount).toBe(1);
expect(refreshSpy.getCall(0).args[0]).toEqual([]);
});
diff --git a/superset-frontend/src/dashboard/components/OverwriteConfirm/OverwriteConfirmModal.test.tsx b/superset-frontend/src/dashboard/components/OverwriteConfirm/OverwriteConfirmModal.test.tsx
index a28f85f779..5d93fd6900 100644
--- a/superset-frontend/src/dashboard/components/OverwriteConfirm/OverwriteConfirmModal.test.tsx
+++ b/superset-frontend/src/dashboard/components/OverwriteConfirm/OverwriteConfirmModal.test.tsx
@@ -20,7 +20,6 @@ import thunk from 'redux-thunk';
import configureStore from 'redux-mock-store';
import fetchMock from 'fetch-mock';
import { mockAllIsIntersecting } from 'react-intersection-observer/test-utils';
-import { act } from 'react-test-renderer';
import { fireEvent, render, waitFor } from 'spec/helpers/testing-library';
import { overwriteConfirmMetadata } from 'spec/fixtures/mockDashboardState';
@@ -62,24 +61,10 @@ test('requests update dashboard api when save button is clicked', async () => {
last_modified_time: +new Date(),
result: overwriteConfirmMetadata.data,
});
-
- // Initialize store with required state
const store = mockStore({
- dashboardLayout: {
- present: {},
- },
+ dashboardLayout: {},
dashboardFilters: {},
- dashboardInfo: {
- metadata: {},
- crossFiltersEnabled: false,
- },
- charts: {},
- dataMask: {},
- nativeFilters: {
- filters: {},
- },
});
-
const { findByTestId } = render(
<OverwriteConfirmModal
overwriteConfirmMetadata={overwriteConfirmMetadata}
@@ -89,23 +74,17 @@ test('requests update dashboard api when save button is clicked', async () => {
store,
},
);
-
const saveButton = await findByTestId('overwrite-confirm-save-button');
expect(fetchMock.calls(updateDashboardEndpoint)).toHaveLength(0);
-
- await act(async () => {
- mockAllIsIntersecting(true);
- fireEvent.click(saveButton);
- });
-
- // Wait for the fetch mock to be called and verify the request body
- await waitFor(() => {
- const calls = fetchMock.calls(updateDashboardEndpoint);
- expect(calls).toHaveLength(1);
- expect(JSON.parse(calls[0][1].body)).toEqual(overwriteConfirmMetadata.data);
- });
-
- // Verify the action was dispatched
+ fireEvent.click(saveButton);
+ expect(fetchMock.calls(updateDashboardEndpoint)).toHaveLength(0);
+ mockAllIsIntersecting(true);
+ fireEvent.click(saveButton);
+ await waitFor(() =>
+ expect(fetchMock.calls(updateDashboardEndpoint)?.[0]?.[1]?.body).toEqual(
+ JSON.stringify(overwriteConfirmMetadata.data),
+ ),
+ );
await waitFor(() =>
expect(store.getActions()).toContainEqual({
type: 'SET_OVERRIDE_CONFIRM',
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarSettings/FilterBarSettings.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarSettings/FilterBarSettings.test.tsx
index 6b4e94a0c8..2863a07931 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarSettings/FilterBarSettings.test.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarSettings/FilterBarSettings.test.tsx
@@ -20,18 +20,12 @@
import fetchMock from 'fetch-mock';
import { waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
-import { render, screen } from 'spec/helpers/testing-library';
+import { render, screen, within } from 'spec/helpers/testing-library';
import { DashboardInfo, FilterBarOrientation } from 'src/dashboard/types';
import * as mockedMessageActions from 'src/components/MessageToasts/actions';
import { FeatureFlag } from '@superset-ui/core';
import FilterBarSettings from '.';
-// Mock the number formatter registration to avoid duplicate warnings
-jest.mock('@superset-ui/core', () => ({
- ...jest.requireActual('@superset-ui/core'),
- getNumberFormatter: jest.fn(),
-}));
-
const initialState: { dashboardInfo: DashboardInfo } = {
dashboardInfo: {
id: 1,
@@ -141,14 +135,10 @@ test('Popover opens with "Vertical" selected', async () => {
await setup();
userEvent.click(screen.getByLabelText('gear'));
userEvent.hover(screen.getByText('Orientation of filter bar'));
-
expect(await screen.findByText('Vertical (Left)')).toBeInTheDocument();
expect(screen.getByText('Horizontal (Top)')).toBeInTheDocument();
-
- // Look for the check icon in a different way
- const menuItems = screen.getAllByRole('menuitem');
expect(
- menuItems[2].querySelector('[aria-label="check"]'),
+ within(screen.getAllByRole('menuitem')[2]).getByLabelText('check'),
).toBeInTheDocument();
});
@@ -160,13 +150,10 @@ test('Popover opens with "Horizontal" selected', async () => {
await setup({ filterBarOrientation: FilterBarOrientation.Horizontal });
userEvent.click(screen.getByLabelText('gear'));
userEvent.hover(screen.getByText('Orientation of filter bar'));
-
expect(await screen.findByText('Vertical (Left)')).toBeInTheDocument();
expect(screen.getByText('Horizontal (Top)')).toBeInTheDocument();
-
- const menuItems = screen.getAllByRole('menuitem');
expect(
- menuItems[3].querySelector('[aria-label="check"]'),
+ within(screen.getAllByRole('menuitem')[3]).getByLabelText('check'),
).toBeInTheDocument();
});
@@ -189,9 +176,10 @@ test('On selection change, send request and update checked value', async () => {
userEvent.click(screen.getByLabelText('gear'));
userEvent.hover(screen.getByText('Orientation of filter bar'));
- const menuItems = screen.getAllByRole('menuitem');
+ expect(await screen.findByText('Vertical (Left)')).toBeInTheDocument();
+ expect(screen.getByText('Horizontal (Top)')).toBeInTheDocument();
expect(
- menuItems[2].querySelector('[aria-label="check"]'),
+ within(screen.getAllByRole('menuitem')[2]).getByLabelText('check'),
).toBeInTheDocument();
userEvent.click(screen.getByText('Horizontal (Top)'));
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/utils.test.ts b/superset-frontend/src/dashboard/components/nativeFilters/utils.test.ts
index fa27873732..0f402ee134 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/utils.test.ts
+++ b/superset-frontend/src/dashboard/components/nativeFilters/utils.test.ts
@@ -45,12 +45,6 @@ describe('nativeFilterGate', () => {
});
it('should return false for native filter chart with cross filter support', () => {
- isFeatureEnabledMock.mockImplementation(feature => {
- if (feature === FeatureFlag.DASHBOARD_CROSS_FILTERS) return false;
- if (feature === FeatureFlag.DASHBOARD_NATIVE_FILTERS) return false;
- return false;
- });
-
expect(
nativeFilterGate([Behavior.NativeFilter, Behavior.InteractiveChart]),
).toEqual(false);
@@ -62,20 +56,6 @@ describe('nativeFilterGate', () => {
});
describe('with cross filters and experimental feature flag enabled', () => {
- beforeAll(() => {
- isFeatureEnabledMock = jest
- .spyOn(uiCore, 'isFeatureEnabled')
- .mockImplementation(feature => {
- if (feature === FeatureFlag.DASHBOARD_CROSS_FILTERS) return true;
- if (feature === FeatureFlag.DASHBOARD_NATIVE_FILTERS) return true;
- return false;
- });
- });
-
- afterAll(() => {
- isFeatureEnabledMock.mockRestore();
- });
-
it('should return true for regular chart', () => {
expect(nativeFilterGate([])).toEqual(true);
});
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts
index 868803254c..ef577b354b 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts
+++ b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts
@@ -27,8 +27,6 @@ import {
getChartMetadataRegistry,
QueryFormData,
t,
- isFeatureEnabled,
- FeatureFlag,
} from '@superset-ui/core';
import { LayoutItem } from 'src/dashboard/types';
import extractUrlParams from 'src/dashboard/util/extractUrlParams';
@@ -148,22 +146,10 @@ export function getExtraFormData(
}
export function nativeFilterGate(behaviors: Behavior[]): boolean {
- const hasNativeFilter = behaviors.includes(Behavior.NativeFilter);
-
- // If it's not a native filter, always allow
- if (!hasNativeFilter) {
- return true;
- }
-
- // Native filters are not allowed when feature flags are disabled
- if (!isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS)) {
- return false;
- }
-
- // When feature flag is enabled:
- // - Pure native filters are not allowed
- // - Native filters with interactive chart behavior are allowed
- return behaviors.includes(Behavior.InteractiveChart);
+ return (
+ !behaviors.includes(Behavior.NativeFilter) ||
+ behaviors.includes(Behavior.InteractiveChart)
+ );
}
export const findTabsWithChartsInScope = (