Skip to content

Commit

Permalink
fix(sqllab): flaky json explore modal due to over-rendering (#26791)
Browse files Browse the repository at this point in the history
  • Loading branch information
justinpark authored Feb 14, 2024
1 parent 152cd70 commit 7b59c94
Show file tree
Hide file tree
Showing 14 changed files with 716 additions and 305 deletions.
20 changes: 19 additions & 1 deletion superset-frontend/src/SqlLab/components/App/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
import React from 'react';
import { AnyAction, combineReducers } from 'redux';
import Mousetrap from 'mousetrap';
import configureStore from 'redux-mock-store';
import thunk from 'redux-thunk';
import { render } from 'spec/helpers/testing-library';
Expand All @@ -37,6 +38,9 @@ jest.mock('src/SqlLab/components/TabbedSqlEditors', () => () => (
jest.mock('src/SqlLab/components/QueryAutoRefresh', () => () => (
<div data-test="mock-query-auto-refresh" />
));
jest.mock('mousetrap', () => ({
reset: jest.fn(),
}));

const sqlLabReducer = combineReducers({
localStorageUsageInKilobytes: reducers.localStorageUsageInKilobytes,
Expand All @@ -48,6 +52,14 @@ describe('SqlLab App', () => {
const mockStore = configureStore(middlewares);
const store = mockStore(sqlLabReducer(undefined, mockAction));

beforeEach(() => {
jest.clearAllMocks();
jest.useFakeTimers();
});
afterEach(() => {
jest.useRealTimers();
});

it('is valid', () => {
expect(React.isValidElement(<App />)).toBe(true);
});
Expand All @@ -58,7 +70,13 @@ describe('SqlLab App', () => {
expect(getByTestId('mock-tabbed-sql-editors')).toBeInTheDocument();
});

it('logs current usage warning', async () => {
it('reset hotkey events on unmount', () => {
const { unmount } = render(<App />, { useRedux: true, store });
unmount();
expect(Mousetrap.reset).toHaveBeenCalled();
});

it('logs current usage warning', () => {
const localStorageUsageInKilobytes = LOCALSTORAGE_MAX_USAGE_KB + 10;
const initialState = {
localStorageUsageInKilobytes,
Expand Down
3 changes: 3 additions & 0 deletions superset-frontend/src/SqlLab/components/App/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import React from 'react';
import { connect } from 'react-redux';
import { Redirect } from 'react-router-dom';
import Mousetrap from 'mousetrap';
import { css, styled, t } from '@superset-ui/core';
import { throttle } from 'lodash';
import {
Expand Down Expand Up @@ -165,6 +166,8 @@ class App extends React.PureComponent<AppProps, AppState> {

// And now we need to reset the overscroll behavior back to the default.
document.body.style.overscrollBehaviorX = 'auto';

Mousetrap.reset();
}

onHashChanged() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@
import React from 'react';
import { render, screen } from 'spec/helpers/testing-library';
import QueryHistory from 'src/SqlLab/components/QueryHistory';
import { initialState } from 'src/SqlLab/fixtures';

const mockedProps = {
queries: [],
queryEditorId: 123,
displayLimit: 1000,
latestQueryId: 'yhMUZCGb',
};
Expand All @@ -32,7 +33,7 @@ const setup = (overrides = {}) => (

describe('QueryHistory', () => {
it('Renders an empty state for query history', () => {
render(setup());
render(setup(), { useRedux: true, initialState });

const emptyStateText = screen.getByText(
/run a query to display query history/i,
Expand Down
29 changes: 22 additions & 7 deletions superset-frontend/src/SqlLab/components/QueryHistory/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@
* specific language governing permissions and limitations
* under the License.
*/
import React from 'react';
import React, { useMemo } from 'react';
import { shallowEqual, useSelector } from 'react-redux';
import { EmptyStateMedium } from 'src/components/EmptyState';
import { t, styled, QueryResponse } from '@superset-ui/core';
import { t, styled } from '@superset-ui/core';
import QueryTable from 'src/SqlLab/components/QueryTable';
import { SqlLabRootState } from 'src/SqlLab/types';

interface QueryHistoryProps {
queries: QueryResponse[];
queryEditorId: string | number;
displayLimit: number;
latestQueryId: string | undefined;
}
Expand All @@ -39,11 +41,23 @@ const StyledEmptyStateWrapper = styled.div`
`;

const QueryHistory = ({
queries,
queryEditorId,
displayLimit,
latestQueryId,
}: QueryHistoryProps) =>
queries.length > 0 ? (
}: QueryHistoryProps) => {
const queries = useSelector(
({ sqlLab: { queries } }: SqlLabRootState) => queries,
shallowEqual,
);
const editorQueries = useMemo(
() =>
Object.values(queries).filter(
({ sqlEditorId }) => String(sqlEditorId) === String(queryEditorId),
),
[queries, queryEditorId],
);

return editorQueries.length > 0 ? (
<QueryTable
columns={[
'state',
Expand All @@ -55,7 +69,7 @@ const QueryHistory = ({
'results',
'actions',
]}
queries={queries}
queries={editorQueries}
displayLimit={displayLimit}
latestQueryId={latestQueryId}
/>
Expand All @@ -67,5 +81,6 @@ const QueryHistory = ({
/>
</StyledEmptyStateWrapper>
);
};

export default QueryHistory;
3 changes: 1 addition & 2 deletions superset-frontend/src/SqlLab/components/QueryTable/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -251,8 +251,7 @@ const QueryTable = ({
modalBody={
<ResultSet
showSql
user={user}
query={query}
queryId={query.id}
height={400}
displayLimit={displayLimit}
defaultQueryLimit={1000}
Expand Down
Loading

0 comments on commit 7b59c94

Please sign in to comment.