Skip to content

Commit

Permalink
fix(sqllab): reverts apache#22695 (apache#22861)
Browse files Browse the repository at this point in the history
  • Loading branch information
justinpark authored and PawankumarES committed Feb 13, 2023
1 parent 1ee1911 commit 5f10ff1
Show file tree
Hide file tree
Showing 8 changed files with 20 additions and 207 deletions.
14 changes: 1 addition & 13 deletions superset-frontend/src/SqlLab/actions/sqlLab.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,22 +128,10 @@ export function getUpToDateQuery(rootState, queryEditor, key) {
sqlLab: { unsavedQueryEditor },
} = rootState;
const id = key ?? queryEditor.id;
const updatedQueryEditor = {
return {
...queryEditor,
...(id === unsavedQueryEditor.id && unsavedQueryEditor),
};

if (
'schema' in updatedQueryEditor &&
!updatedQueryEditor.schemaOptions?.find(
({ value }) => value === updatedQueryEditor.schema,
)
) {
// remove the deprecated schema option
delete updatedQueryEditor.schema;
}

return updatedQueryEditor;
}

export function resetState() {
Expand Down
48 changes: 0 additions & 48 deletions superset-frontend/src/SqlLab/actions/sqlLab.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -316,54 +316,6 @@ describe('async actions', () => {
});
});

describe('getUpToDateQuery', () => {
const id = 'randomidvalue2';
const unsavedQueryEditor = {
id,
sql: 'some query here',
schemaOptions: [{ value: 'testSchema' }, { value: 'schema2' }],
};

it('returns payload with the updated queryEditor', () => {
const queryEditor = {
id,
name: 'Dummy query editor',
schema: 'testSchema',
};
const state = {
sqlLab: {
tabHistory: [id],
queryEditors: [queryEditor],
unsavedQueryEditor,
},
};
expect(actions.getUpToDateQuery(state, queryEditor)).toEqual({
...queryEditor,
...unsavedQueryEditor,
});
});

it('ignores the deprecated schema option', () => {
const queryEditor = {
id,
name: 'Dummy query editor',
schema: 'oldSchema',
};
const state = {
sqlLab: {
tabHistory: [id],
queryEditors: [queryEditor],
unsavedQueryEditor,
},
};
expect(actions.getUpToDateQuery(state, queryEditor)).toEqual({
...queryEditor,
...unsavedQueryEditor,
schema: undefined,
});
});
});

describe('postStopQuery', () => {
const stopQueryEndpoint = 'glob:*/api/v1/query/stop';
fetchMock.post(stopQueryEndpoint, {});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ const mockQueryEditor = {
autorun: false,
sql: 'SELECT * FROM ...',
remoteId: 999,
schemaOptions: [{ value: 'query_schema' }, { value: 'query_schema_updated' }],
};
const disabled = {
id: 'disabledEditorId',
Expand Down Expand Up @@ -83,7 +82,6 @@ const standardProviderWithUnsaved: React.FC = ({ children }) => (
...initialState,
sqlLab: {
...initialState.sqlLab,
queryEditors: [mockQueryEditor],
unsavedQueryEditor,
},
})}
Expand Down Expand Up @@ -125,7 +123,7 @@ describe('ShareSqlLabQuery', () => {
});
});
const button = screen.getByRole('button');
const { id, remoteId, schemaOptions, ...expected } = mockQueryEditor;
const { id, remoteId, ...expected } = mockQueryEditor;
const storeQuerySpy = jest.spyOn(utils, 'storeQuery');
userEvent.click(button);
expect(storeQuerySpy.mock.calls).toHaveLength(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,22 +41,15 @@ const middlewares = [thunk];
const mockStore = configureStore(middlewares);
const store = mockStore(initialState);

beforeEach(() => {
fetchMock.get('glob:*/api/v1/database/**', { result: [] });
fetchMock.get('glob:*/api/v1/database/*/schemas/?*', { result: [] });
fetchMock.get('glob:*/superset/tables/**', {
options: [
{
label: 'ab_user',
value: 'ab_user',
},
],
tableLength: 1,
});
});

afterEach(() => {
fetchMock.restore();
fetchMock.get('glob:*/api/v1/database/*/schemas/?*', { result: [] });
fetchMock.get('glob:*/superset/tables/**', {
options: [
{
label: 'ab_user',
value: 'ab_user',
},
],
tableLength: 1,
});

const renderAndWait = (props, store) =>
Expand Down Expand Up @@ -117,9 +110,8 @@ test('should toggle the table when the header is clicked', async () => {
userEvent.click(header);

await waitFor(() => {
expect(store.getActions()[store.getActions().length - 1].type).toEqual(
'COLLAPSE_TABLE',
);
expect(store.getActions()).toHaveLength(4);
expect(store.getActions()[3].type).toEqual('COLLAPSE_TABLE');
});
});

Expand All @@ -137,77 +129,14 @@ test('When changing database the table list must be updated', async () => {
database_name: 'new_db',
backend: 'postgresql',
}}
queryEditorId={defaultQueryEditor.id}
queryEditor={{ ...mockedProps.queryEditor, schema: 'new_schema' }}
tables={[{ ...mockedProps.tables[0], dbId: 2, name: 'new_table' }]}
/>,
{
useRedux: true,
store: mockStore({
...initialState,
sqlLab: {
...initialState.sqlLab,
unsavedQueryEditor: {
id: defaultQueryEditor.id,
schema: 'new_schema',
},
},
}),
initialState,
},
);
expect(await screen.findByText(/new_db/i)).toBeInTheDocument();
expect(await screen.findByText(/new_table/i)).toBeInTheDocument();
});

test('ignore schema api when current schema is deprecated', async () => {
const invalidSchemaName = 'None';
await renderAndWait(
mockedProps,
mockStore({
...initialState,
sqlLab: {
...initialState.sqlLab,
unsavedQueryEditor: {
id: defaultQueryEditor.id,
schema: invalidSchemaName,
},
},
}),
);

expect(await screen.findByText(/Database/i)).toBeInTheDocument();
expect(screen.queryByText(/None/i)).not.toBeInTheDocument();
expect(fetchMock.calls()).not.toContainEqual(
expect.arrayContaining([
expect.stringContaining(
`/tables/${mockedProps.database.id}/${invalidSchemaName}/`,
),
]),
);
});

test('fetches schema api when current schema is among the list', async () => {
const validSchema = 'schema1';
await renderAndWait(
mockedProps,
mockStore({
...initialState,
sqlLab: {
...initialState.sqlLab,
unsavedQueryEditor: {
id: defaultQueryEditor.id,
schema: validSchema,
schemaOptions: [{ name: validSchema, value: validSchema }],
},
},
}),
);

expect(await screen.findByText(/schema1/i)).toBeInTheDocument();
expect(fetchMock.calls()).toContainEqual(
expect.arrayContaining([
expect.stringContaining(
`/tables/${mockedProps.database.id}/${validSchema}/`,
),
]),
);
});
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ const SqlEditorLeftBar = ({
}: SqlEditorLeftBarProps) => {
const dispatch = useDispatch();
const queryEditor = useQueryEditor(queryEditorId, ['dbId', 'schema']);

const [emptyResultsWithSearch, setEmptyResultsWithSearch] = useState(false);
const [userSelectedDb, setUserSelected] = useState<DatabaseObject | null>(
null,
Expand Down
33 changes: 2 additions & 31 deletions superset-frontend/src/SqlLab/hooks/useQueryEditor/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
* specific language governing permissions and limitations
* under the License.
*/
import { useMemo } from 'react';
import pick from 'lodash/pick';
import { shallowEqual, useSelector } from 'react-redux';
import { SqlLabRootState, QueryEditor } from 'src/SqlLab/types';
Expand All @@ -25,43 +24,15 @@ export default function useQueryEditor<T extends keyof QueryEditor>(
sqlEditorId: string,
attributes: ReadonlyArray<T>,
) {
const queryEditor = useSelector<
SqlLabRootState,
Pick<QueryEditor, T | 'id' | 'schema'>
>(
return useSelector<SqlLabRootState, Pick<QueryEditor, T | 'id'>>(
({ sqlLab: { unsavedQueryEditor, queryEditors } }) =>
pick(
{
...queryEditors.find(({ id }) => id === sqlEditorId),
...(sqlEditorId === unsavedQueryEditor.id && unsavedQueryEditor),
},
['id'].concat(attributes),
) as Pick<QueryEditor, T | 'id' | 'schema'>,
) as Pick<QueryEditor, T | 'id'>,
shallowEqual,
);
const { schema, schemaOptions } = useSelector<
SqlLabRootState,
Pick<QueryEditor, 'schema' | 'schemaOptions'>
>(
({ sqlLab: { unsavedQueryEditor, queryEditors } }) =>
pick(
{
...queryEditors.find(({ id }) => id === sqlEditorId),
...(sqlEditorId === unsavedQueryEditor.id && unsavedQueryEditor),
},
['schema', 'schemaOptions'],
) as Pick<QueryEditor, T | 'schema' | 'schemaOptions'>,
shallowEqual,
);

const schemaOptionsMap = useMemo(
() => new Set(schemaOptions?.map(({ value }) => value)),
[schemaOptions],
);

if ('schema' in queryEditor && schema && !schemaOptionsMap.has(schema)) {
delete queryEditor.schema;
}

return queryEditor as Pick<QueryEditor, T | 'id'>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ test('includes id implicitly', () => {
test('returns updated values from unsaved change', () => {
const expectedSql = 'SELECT updated_column\nFROM updated_table\nWHERE';
const { result } = renderHook(
() => useQueryEditor(defaultQueryEditor.id, ['id', 'sql', 'schema']),
() => useQueryEditor(defaultQueryEditor.id, ['id', 'sql']),
{
wrapper: createWrapper({
useRedux: true,
Expand All @@ -88,31 +88,5 @@ test('returns updated values from unsaved change', () => {
},
);
expect(result.current.id).toEqual(defaultQueryEditor.id);
expect(result.current.schema).toEqual(defaultQueryEditor.schema);
expect(result.current.sql).toEqual(expectedSql);
});

test('skips the deprecated schema option', () => {
const expectedSql = 'SELECT updated_column\nFROM updated_table\nWHERE';
const { result } = renderHook(
() => useQueryEditor(defaultQueryEditor.id, ['schema']),
{
wrapper: createWrapper({
useRedux: true,
store: mockStore({
...initialState,
sqlLab: {
...initialState.sqlLab,
unsavedQueryEditor: {
id: defaultQueryEditor.id,
sql: expectedSql,
schema: 'deprecated schema',
},
},
}),
}),
},
);
expect(result.current.schema).not.toEqual(defaultQueryEditor.schema);
expect(result.current.schema).toBeUndefined();
});
2 changes: 1 addition & 1 deletion superset-frontend/src/SqlLab/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export interface QueryEditor {
id: string;
dbId?: number;
name: string;
schema?: string;
schema: string;
autorun: boolean;
sql: string;
remoteId: number | null;
Expand Down

0 comments on commit 5f10ff1

Please sign in to comment.