You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by el...@apache.org on 2023/10/27 18:04:40 UTC
(superset) branch master updated: fix: allow for backward compatible errors (#25640)
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new ed14f36c55 fix: allow for backward compatible errors (#25640)
ed14f36c55 is described below
commit ed14f36c558459c6dd231dcbcdf6fe52ca15998e
Author: Elizabeth Thompson <es...@gmail.com>
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 | 22 +--
.../src/components/ErrorMessage/types.ts | 9 --
superset-frontend/src/utils/errorMessages.ts | 1 +
5 files changed, 107 insertions(+), 83 deletions(-)
diff --git a/superset-frontend/src/components/Datasource/DatasourceEditor.jsx b/superset-frontend/src/components/Datasource/DatasourceEditor.jsx
index 326b8e606f..90defaf8dc 100644
--- a/superset-frontend/src/components/Datasource/DatasourceEditor.jsx
+++ b/superset-frontend/src/components/Datasource/DatasourceEditor.jsx
@@ -1396,7 +1396,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 24536cac00..bb6fce7577 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, { result: {} });
- 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('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 fed45b43f2..3e3acde4b2 100644
--- a/superset-frontend/src/components/Datasource/DatasourceModal.tsx
+++ b/superset-frontend/src/components/Datasource/DatasourceModal.tsx
@@ -31,13 +31,11 @@ import {
import Modal from 'src/components/Modal';
import AsyncEsmComponent from 'src/components/AsyncEsmComponent';
-import {
- genericSupersetError,
- SupersetError,
-} from 'src/components/ErrorMessage/types';
+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'));
@@ -207,16 +205,24 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
})
.catch(response => {
setIsSaving(false);
- response.json().then((errorJson: { errors: SupersetError[] }) => {
+ 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 saving dataset'),
okButtonProps: { danger: true, className: 'btn-danger' },
content: (
<ErrorMessageWithStackTrace
- error={
- errorJson?.errors?.[0] || genericSupersetError(errorJson)
- }
+ 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 c32435d7f4..7c4c3fe94a 100644
--- a/superset-frontend/src/components/ErrorMessage/types.ts
+++ b/superset-frontend/src/components/ErrorMessage/types.ts
@@ -107,12 +107,3 @@ export type ErrorMessageComponentProps<ExtraType = Record<string, any> | null> =
export type ErrorMessageComponent =
React.ComponentType<ErrorMessageComponentProps>;
-
-/* Generic error to be returned when the backend returns an error response that is not
- * SIP-41 compliant. */
-export const genericSupersetError = (extra: object) => ({
- error_type: ErrorTypeEnum.GENERIC_BACKEND_ERROR,
- extra,
- level: 'error' as ErrorLevel,
- message: 'An error occurred',
-});
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: