fix: allow for backward compatible errors (apache#25640)

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 })}

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();
+    });
   });
 });

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}
               />
             ),
           });

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',
-});

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: