refactor: Share sql lab query (#13630)

superset-frontend/spec/javascripts/sqllab/ShareSqlLabQuery_spec.jsx

@@ -21,24 +21,41 @@ import configureStore from 'redux-mock-store';
 import thunk from 'redux-thunk';
 import fetchMock from 'fetch-mock';
 import * as featureFlags from 'src/featureFlags';
-import { shallow } from 'enzyme';
+import { Provider } from 'react-redux';
-
+import { supersetTheme, ThemeProvider } from '@superset-ui/core';
+import { render, screen, act } from '@testing-library/react';
+import '@testing-library/jest-dom/extend-expect';
+import userEvent from '@testing-library/user-event';
 import * as utils from 'src/utils/common';
 import ShareSqlLabQuery from 'src/SqlLab/components/ShareSqlLabQuery';
-import CopyToClipboard from 'src/components/CopyToClipboard';
-import { Tooltip } from 'src/common/components/Tooltip';
 
 const mockStore = configureStore([thunk]);
-const store = mockStore();
+const store = mockStore({});
 let isFeatureEnabledMock;
 
-const clipboardSpy = jest.fn();
+const standardProvider = ({ children }) => (
+  <ThemeProvider theme={supersetTheme}>
+    <Provider store={store}>{children}</Provider>
+  </ThemeProvider>
+);
+
+const defaultProps = {
+  queryEditor: {
+    dbId: 0,
+    title: 'query title',
+    schema: 'query_schema',
+    autorun: false,
+    sql: 'SELECT * FROM ...',
+    remoteId: 999,
+  },
+  addDangerToast: jest.fn(),
+};
 
 describe('ShareSqlLabQuery', () => {
   const storeQueryUrl = 'glob:*/kv/store/';
   const storeQueryMockId = '123';
 
-  beforeEach(() => {
+  beforeEach(async () => {
     fetchMock.post(storeQueryUrl, () => ({ id: storeQueryMockId }), {
       overwriteRoutes: true,
     });
@@ -48,33 +65,6 @@ describe('ShareSqlLabQuery', () => {
 
   afterAll(fetchMock.reset);
 
-  const defaultProps = {
-    queryEditor: {
-      dbId: 0,
-      title: 'query title',
-      schema: 'query_schema',
-      autorun: false,
-      sql: 'SELECT * FROM ...',
-      remoteId: 999,
-    },
-  };
-
-  const storedQueryAttributes = {
-    dbId: 0,
-    title: 'query title',
-    schema: 'query_schema',
-    autorun: false,
-    sql: 'SELECT * FROM ...',
-  };
-
-  function setup(overrideProps) {
-    const wrapper = shallow(
-      <ShareSqlLabQuery store={store} {...defaultProps} {...overrideProps} />,
-    ).dive(); // wrapped in withToasts HOC
-
-    return wrapper;
-  }
-
   describe('via /kv/store', () => {
     beforeAll(() => {
       isFeatureEnabledMock = jest
@@ -86,54 +76,20 @@ describe('ShareSqlLabQuery', () => {
       isFeatureEnabledMock.restore();
     });
 
-    it('calls storeQuery() with the query when getCopyUrl() is called', () => {
+    it('calls storeQuery() with the query when getCopyUrl() is called', async () => {
-      expect.assertions(4);
+      await act(async () => {
-      const storeQuerySpy = jest.spyOn(utils, 'storeQuery');
+        render(<ShareSqlLabQuery {...defaultProps} />, {
-
+          wrapper: standardProvider,
-      const wrapper = setup();
-      const instance = wrapper.instance();
-
-      return instance.getCopyUrl(clipboardSpy).then(() => {
-        expect(storeQuerySpy.mock.calls).toHaveLength(1);
-        expect(fetchMock.calls(storeQueryUrl)).toHaveLength(1);
-        expect(storeQuerySpy.mock.calls[0][0]).toMatchObject(
-          storedQueryAttributes,
-        );
-        expect(clipboardSpy).toHaveBeenCalledWith(
-          expect.stringContaining('?id='),
-        );
-
-        storeQuerySpy.mockRestore();
-
-        return Promise.resolve();
-      });
-    });
-
-    it('dispatches an error toast upon fetching failure', () => {
-      expect.assertions(3);
-      const error = 'There was an error with your request';
-      const addDangerToastSpy = jest.fn();
-      fetchMock.post(
-        storeQueryUrl,
-        { throws: error },
-        { overwriteRoutes: true },
-      );
-      const wrapper = setup();
-      wrapper.setProps({ addDangerToast: addDangerToastSpy });
-
-      return wrapper
-        .instance()
-        .getCopyUrl(clipboardSpy)
-        .then(() => {
-          // Fails then retries thrice
-          expect(fetchMock.calls(storeQueryUrl)).toHaveLength(4);
-          expect(addDangerToastSpy.mock.calls).toHaveLength(1);
-          expect(addDangerToastSpy.mock.calls[0][0]).toBe(error);
-
-          return Promise.resolve();
         });
+      });
+      const button = screen.getByRole('button');
+      const storeQuerySpy = jest.spyOn(utils, 'storeQuery');
+      userEvent.click(button);
+      expect(storeQuerySpy.mock.calls).toHaveLength(1);
+      storeQuerySpy.mockRestore();
     });
   });
+
   describe('via saved query', () => {
     beforeAll(() => {
       isFeatureEnabledMock = jest
@@ -145,37 +101,34 @@ describe('ShareSqlLabQuery', () => {
       isFeatureEnabledMock.restore();
     });
 
-    it('does not call storeQuery() with the query when getCopyUrl() is called', () => {
+    it('does not call storeQuery() with the query when getCopyUrl() is called and feature is not enabled', async () => {
+      await act(async () => {
+        render(<ShareSqlLabQuery {...defaultProps} />, {
+          wrapper: standardProvider,
+        });
+      });
       const storeQuerySpy = jest.spyOn(utils, 'storeQuery');
-
+      const button = screen.getByRole('button');
-      const wrapper = setup();
+      userEvent.click(button);
-      const instance = wrapper.instance();
-
-      instance.getCopyUrl(clipboardSpy);
-
       expect(storeQuerySpy.mock.calls).toHaveLength(0);
-      expect(fetchMock.calls(storeQueryUrl)).toHaveLength(0);
-      expect(clipboardSpy).toHaveBeenCalledWith(
-        expect.stringContaining('savedQueryId'),
-      );
-
       storeQuerySpy.mockRestore();
     });
 
-    it('shows a request to save the query when the query is not yet saved', () => {
+    it('button is disabled and there is a request to save the query', async () => {
-      const wrapper = setup({
+      const updatedProps = {
         queryEditor: {
           ...defaultProps.queryEditor,
           remoteId: undefined,
         },
+      };
+      await act(async () => {
+        render(<ShareSqlLabQuery {...updatedProps} />, {
+          wrapper: standardProvider,
+        });
       });
-
+      const button = screen.getByRole('button', { name: /copy link/i });
-      expect(wrapper.find(CopyToClipboard)).toHaveLength(0);
+      const style = window.getComputedStyle(button);
-      expect(wrapper.find('.btn-disabled')).toHaveLength(1);
+      expect(style.color).toBe('rgb(102, 102, 102)');
-      expect(wrapper.find(Tooltip)).toHaveProp(
-        'title',
-        expect.stringContaining('Save the query'),
-      );
     });
   });
 });

superset-frontend/src/SqlLab/components/ShareSqlLabQuery.tsx

@@ -17,7 +17,6 @@
  * under the License.
  */
 import React from 'react';
-import PropTypes from 'prop-types';
 import { Tooltip } from 'src/common/components/Tooltip';
 import { t, styled, supersetTheme } from '@superset-ui/core';
 import cx from 'classnames';
@@ -30,17 +29,17 @@ import { storeQuery } from 'src/utils/common';
 import { getClientErrorObject } from 'src/utils/getClientErrorObject';
 import { FeatureFlag, isFeatureEnabled } from '../../featureFlags';
 
-const propTypes = {
+interface ShareSqlLabQueryPropTypes {
-  queryEditor: PropTypes.shape({
+  queryEditor: {
-    dbId: PropTypes.number,
+    dbId: number;
-    title: PropTypes.string,
+    title: string;
-    schema: PropTypes.string,
+    schema: string;
-    autorun: PropTypes.bool,
+    autorun: boolean;
-    sql: PropTypes.string,
+    sql: string;
-    remoteId: PropTypes.number,
+    remoteId: number | null;
-  }).isRequired,
+  };
-  addDangerToast: PropTypes.func.isRequired,
+  addDangerToast: (msg: string) => void;
-};
+}
 
 const Styles = styled.div`
   .btn-disabled {
@@ -56,16 +55,12 @@ const Styles = styled.div`
   }
 `;
 
-class ShareSqlLabQuery extends React.Component {
+function ShareSqlLabQuery({
-  getCopyUrl(callback) {
+  queryEditor,
-    if (isFeatureEnabled(FeatureFlag.SHARE_QUERIES_VIA_KV_STORE)) {
+  addDangerToast,
-      return this.getCopyUrlForKvStore(callback);
+}: ShareSqlLabQueryPropTypes) {
-    }
+  const getCopyUrlForKvStore = (callback: Function) => {
-    return this.getCopyUrlForSavedQuery(callback);
+    const { dbId, title, schema, autorun, sql } = queryEditor;
-  }
-
-  getCopyUrlForKvStore(callback) {
-    const { dbId, title, schema, autorun, sql } = this.props.queryEditor;
     const sharedQuery = { dbId, title, schema, autorun, sql };
 
     return storeQuery(sharedQuery)
@@ -74,28 +69,34 @@ class ShareSqlLabQuery extends React.Component {
       })
       .catch(response => {
         getClientErrorObject(response).then(() => {
-          this.props.addDangerToast(t('There was an error with your request'));
+          addDangerToast(t('There was an error with your request'));
         });
       });
-  }
+  };
 
-  getCopyUrlForSavedQuery(callback) {
+  const getCopyUrlForSavedQuery = (callback: Function) => {
     let savedQueryToastContent;
 
-    if (this.props.queryEditor.remoteId) {
+    if (queryEditor.remoteId) {
       savedQueryToastContent = `${
         window.location.origin + window.location.pathname
-      }?savedQueryId=${this.props.queryEditor.remoteId}`;
+      }?savedQueryId=${queryEditor.remoteId}`;
       callback(savedQueryToastContent);
     } else {
       savedQueryToastContent = t('Please save the query to enable sharing');
       callback(savedQueryToastContent);
     }
-  }
+  };
+  const getCopyUrl = (callback: Function) => {
+    if (isFeatureEnabled(FeatureFlag.SHARE_QUERIES_VIA_KV_STORE)) {
+      return getCopyUrlForKvStore(callback);
+    }
+    return getCopyUrlForSavedQuery(callback);
+  };
 
-  buildButton() {
+  const buildButton = () => {
     const canShare =
-      this.props.queryEditor.remoteId ||
+      queryEditor.remoteId ||
       isFeatureEnabled(FeatureFlag.SHARE_QUERIES_VIA_KV_STORE);
     return (
       <Styles>
@@ -114,36 +115,38 @@ class ShareSqlLabQuery extends React.Component {
         </Button>
       </Styles>
     );
-  }
+  };
 
-  render() {
+  const canShare =
-    const canShare =
+    queryEditor.remoteId ||
-      this.props.queryEditor.remoteId ||
+    isFeatureEnabled(FeatureFlag.SHARE_QUERIES_VIA_KV_STORE);
-      isFeatureEnabled(FeatureFlag.SHARE_QUERIES_VIA_KV_STORE);
+
-    return (
+  return (
-      <Tooltip
+    <Tooltip
-        id="copy_link"
+      id="copy_link"
-        placement="top"
+      placement="top"
-        title={
+      overlayStyle={{
-          canShare
+        fontSize: supersetTheme.typography.sizes.s,
-            ? t('Copy query link to your clipboard')
+        lineHeight: '1.6',
-            : t('Save the query to copy the link')
+        maxWidth: '125px',
-        }
+      }}
-      >
+      title={
-        {canShare ? (
+        canShare
-          <CopyToClipboard
+          ? t('Copy query link to your clipboard')
-            getText={callback => this.getCopyUrl(callback)}
+          : t('Save the query to enable this feature')
-            wrapped={false}
+      }
-            copyNode={this.buildButton()}
+    >
-          />
+      {canShare ? (
-        ) : (
+        <CopyToClipboard
-          this.buildButton()
+          getText={getCopyUrl}
-        )}
+          wrapped={false}
-      </Tooltip>
+          copyNode={buildButton()}
-    );
+        />
-  }
+      ) : (
+        buildButton()
+      )}
+    </Tooltip>
+  );
 }
 
-ShareSqlLabQuery.propTypes = propTypes;
-
 export default withToasts(ShareSqlLabQuery);