This is an automated email from the ASF dual-hosted git repository. michaelsmolina pushed a commit to branch 3.0 in repository https://gitbox.apache.org/repos/asf/superset.git
commit 2f468900c87c6d384d928815cf85e572215b81c5 Author: Elizabeth Thompson <[email protected]> AuthorDate: Fri Oct 27 11:04:33 2023 -0700 fix: allow for backward compatible errors (#25640) --- .../src/components/Datasource/DatasourceEditor.jsx | 2 +- .../components/Datasource/DatasourceModal.test.jsx | 156 ++++++++++++--------- .../src/components/Datasource/DatasourceModal.tsx | 26 +++- .../src/components/ErrorMessage/types.ts | 2 +- superset-frontend/src/utils/errorMessages.ts | 1 + 5 files changed, 115 insertions(+), 72 deletions(-) diff --git a/superset-frontend/src/components/Datasource/DatasourceEditor.jsx b/superset-frontend/src/components/Datasource/DatasourceEditor.jsx index ffbba4c7db..195545a2f6 100644 --- a/superset-frontend/src/components/Datasource/DatasourceEditor.jsx +++ b/superset-frontend/src/components/Datasource/DatasourceEditor.jsx @@ -1386,7 +1386,7 @@ class DatasourceEditor extends React.PureComponent { const { theme } = this.props; return ( - <DatasourceContainer> + <DatasourceContainer data-test="datasource-editor"> {this.renderErrors()} <Alert css={theme => ({ marginBottom: theme.gridUnit * 4 })} diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx b/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx index 5bcb705b68..6d991f24a0 100644 --- a/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx +++ b/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx @@ -18,30 +18,35 @@ */ import React from 'react'; import { act } from 'react-dom/test-utils'; -import { mount } from 'enzyme'; -import { Provider } from 'react-redux'; +import { + render, + screen, + waitFor, + fireEvent, + cleanup, +} from '@testing-library/react'; import fetchMock from 'fetch-mock'; +import { Provider } from 'react-redux'; import sinon from 'sinon'; -import { supersetTheme, ThemeProvider } from '@superset-ui/core'; - -import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint'; +import { + supersetTheme, + ThemeProvider, + SupersetClient, +} from '@superset-ui/core'; import { defaultStore as store } from 'spec/helpers/testing-library'; -import Modal from 'src/components/Modal'; import { DatasourceModal } from 'src/components/Datasource'; -import DatasourceEditor from 'src/components/Datasource/DatasourceEditor'; import * as uiCore from '@superset-ui/core'; import mockDatasource from 'spec/fixtures/mockDatasource'; -import { api } from 'src/hooks/apiResources/queryApi'; - -const datasource = mockDatasource['7__table']; +// Define your constants here const SAVE_ENDPOINT = 'glob:*/api/v1/dataset/7'; const SAVE_PAYLOAD = { new: 'data' }; const SAVE_DATASOURCE_ENDPOINT = 'glob:*/api/v1/dataset/7'; const GET_DATASOURCE_ENDPOINT = SAVE_DATASOURCE_ENDPOINT; +const GET_DATABASE_ENDPOINT = 'glob:*/api/v1/database/?q=*'; const mockedProps = { - datasource, + datasource: mockDatasource['7__table'], addSuccessToast: () => {}, addDangerToast: () => {}, onChange: () => {}, @@ -50,80 +55,101 @@ const mockedProps = { onDatasourceSave: sinon.spy(), }; -async function mountAndWait(props = mockedProps) { - const mounted = mount( +let container; +let isFeatureEnabledMock; + +async function renderAndWait(props = mockedProps) { + const { container: renderedContainer } = render( <Provider store={store}> - <DatasourceModal {...props} /> + <ThemeProvider theme={supersetTheme}> + <DatasourceModal {...props} /> + </ThemeProvider> </Provider>, - { - wrappingComponent: ThemeProvider, - wrappingComponentProps: { theme: supersetTheme }, - }, ); - await waitForComponentToPaint(mounted); - return mounted; + container = renderedContainer; } +beforeEach(() => { + fetchMock.reset(); + cleanup(); + isFeatureEnabledMock = jest.spyOn(uiCore, 'isFeatureEnabled'); + renderAndWait(); + fetchMock.post(SAVE_ENDPOINT, SAVE_PAYLOAD); + fetchMock.put(SAVE_DATASOURCE_ENDPOINT, {}); + fetchMock.get(GET_DATASOURCE_ENDPOINT, { result: {} }); + fetchMock.get(GET_DATABASE_ENDPOINT, { result: [] }); +}); + +afterEach(() => { + isFeatureEnabledMock.mockRestore(); +}); + describe('DatasourceModal', () => { - let wrapper; - let isFeatureEnabledMock; - beforeEach(async () => { - isFeatureEnabledMock = jest.spyOn(uiCore, 'isFeatureEnabled'); - fetchMock.reset(); - wrapper = await mountAndWait(); + it('renders', async () => { + expect(container).toBeDefined(); }); - afterAll(() => { - isFeatureEnabledMock.restore(); - act(() => { - store.dispatch(api.util.resetApiState()); - }); + it('renders the component', () => { + expect(screen.getByText('Edit Dataset')).toBeInTheDocument(); }); - it('renders', () => { - expect(wrapper.find(DatasourceModal)).toExist(); + it('renders a Modal', async () => { + expect(screen.getByRole('dialog')).toBeInTheDocument(); }); - it('renders a Modal', () => { - expect(wrapper.find(Modal)).toExist(); + it('renders a DatasourceEditor', async () => { + expect(screen.getByTestId('datasource-editor')).toBeInTheDocument(); }); - it('renders a DatasourceEditor', () => { - expect(wrapper.find(DatasourceEditor)).toExist(); + it('renders a legacy data source btn', () => { + const button = screen.getByTestId('datasource-modal-legacy-edit'); + expect(button).toBeInTheDocument(); }); - it('saves on confirm', async () => { - const callsP = fetchMock.post(SAVE_ENDPOINT, SAVE_PAYLOAD); - fetchMock.put(SAVE_DATASOURCE_ENDPOINT, {}); - fetchMock.get(GET_DATASOURCE_ENDPOINT, {}); - act(() => { - wrapper - .find('button[data-test="datasource-modal-save"]') - .props() - .onClick(); + it('disables the save button when the datasource is managed externally', () => { + // the render is currently in a before operation, so it needs to be cleaned up + // we could alternatively move all the renders back into the tests or find a better + // way to automatically render but still allow to pass in props with the tests + cleanup(); + + renderAndWait({ + ...mockedProps, + datasource: { ...mockedProps.datasource, is_managed_externally: true }, }); - await waitForComponentToPaint(wrapper); - act(() => { - const okButton = wrapper.find( - '.ant-modal-confirm .ant-modal-confirm-btns .ant-btn-primary', - ); - okButton.simulate('click'); + const saveButton = screen.getByTestId('datasource-modal-save'); + expect(saveButton).toBeDisabled(); + }); + + it('calls the onDatasourceSave function when the save button is clicked', async () => { + cleanup(); + const onDatasourceSave = jest.fn(); + + renderAndWait({ ...mockedProps, onDatasourceSave }); + const saveButton = screen.getByTestId('datasource-modal-save'); + await act(async () => { + fireEvent.click(saveButton); + const okButton = await screen.findByRole('button', { name: 'OK' }); + okButton.click(); + }); + await waitFor(() => { + expect(onDatasourceSave).toHaveBeenCalled(); }); - await waitForComponentToPaint(wrapper); - // one call to PUT, then one to GET - const expected = [ - 'http://localhost/api/v1/dataset/7', - 'http://localhost/api/v1/dataset/7', - ]; - expect(callsP._calls.map(call => call[0])).toEqual( - expected, - ); /* eslint no-underscore-dangle: 0 */ }); - it('renders a legacy data source btn', () => { - expect( - wrapper.find('button[data-test="datasource-modal-legacy-edit"]'), - ).toExist(); + it.only('should render error dialog', async () => { + jest + .spyOn(SupersetClient, 'put') + .mockRejectedValue(new Error('Something went wrong')); + await act(async () => { + const saveButton = screen.getByTestId('datasource-modal-save'); + fireEvent.click(saveButton); + const okButton = await screen.findByRole('button', { name: 'OK' }); + okButton.click(); + }); + await act(async () => { + const errorTitle = await screen.findByText('Error saving dataset'); + expect(errorTitle).toBeInTheDocument(); + }); }); }); diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.tsx b/superset-frontend/src/components/Datasource/DatasourceModal.tsx index f9c40c47ba..031609e09a 100644 --- a/superset-frontend/src/components/Datasource/DatasourceModal.tsx +++ b/superset-frontend/src/components/Datasource/DatasourceModal.tsx @@ -28,12 +28,13 @@ import { SupersetClient, t, } from '@superset-ui/core'; - import Modal from 'src/components/Modal'; import AsyncEsmComponent from 'src/components/AsyncEsmComponent'; -import { getClientErrorObject } from 'src/utils/getClientErrorObject'; +import { SupersetError } from 'src/components/ErrorMessage/types'; +import ErrorMessageWithStackTrace from 'src/components/ErrorMessage/ErrorMessageWithStackTrace'; import withToasts from 'src/components/MessageToasts/withToasts'; import { useSelector } from 'react-redux'; +import { getClientErrorObject } from 'src/utils/getClientErrorObject'; const DatasourceEditor = AsyncEsmComponent(() => import('./DatasourceEditor')); @@ -202,11 +203,26 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({ }) .catch(response => { setIsSaving(false); - getClientErrorObject(response).then(({ error }) => { + getClientErrorObject(response).then(error => { + let errorResponse: SupersetError | undefined; + let errorText: string | undefined; + // sip-40 error response + if (error?.errors?.length) { + errorResponse = error.errors[0]; + } else if (typeof error.error === 'string') { + // backward compatible with old error messages + errorText = error.error; + } modal.error({ - title: t('Error'), - content: error || t('An error has occurred'), + title: t('Error saving dataset'), okButtonProps: { danger: true, className: 'btn-danger' }, + content: ( + <ErrorMessageWithStackTrace + error={errorResponse} + source="crud" + fallback={errorText} + /> + ), }); }); }); diff --git a/superset-frontend/src/components/ErrorMessage/types.ts b/superset-frontend/src/components/ErrorMessage/types.ts index d3fe5bfdf7..4375a9dec1 100644 --- a/superset-frontend/src/components/ErrorMessage/types.ts +++ b/superset-frontend/src/components/ErrorMessage/types.ts @@ -88,7 +88,7 @@ export type ErrorType = ValueOf<typeof ErrorTypeEnum>; // Keep in sync with superset/views/errors.py export type ErrorLevel = 'info' | 'warning' | 'error'; -export type ErrorSource = 'dashboard' | 'explore' | 'sqllab'; +export type ErrorSource = 'dashboard' | 'explore' | 'sqllab' | 'crud'; export type SupersetError<ExtraType = Record<string, any> | null> = { error_type: ErrorType; diff --git a/superset-frontend/src/utils/errorMessages.ts b/superset-frontend/src/utils/errorMessages.ts index 16a04105c4..d5bfbdc17b 100644 --- a/superset-frontend/src/utils/errorMessages.ts +++ b/superset-frontend/src/utils/errorMessages.ts @@ -16,6 +16,7 @@ * specific language governing permissions and limitations * under the License. */ + // Error messages used in many places across applications const COMMON_ERR_MESSAGES = { SESSION_TIMED_OUT:
