You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2023/10/31 14:35:40 UTC
(superset) 09/11: fix: allow for backward compatible errors (#25640)
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 <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 | 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: