From ebd4a917f7f2dafb18681cdf7e9f4ebdfc5898d2 Mon Sep 17 00:00:00 2001 From: AAfghahi <48933336+AAfghahi@users.noreply.github.com> Date: Thu, 18 Mar 2021 17:07:24 -0400 Subject: [PATCH] refactor: Share sql lab query (#13630) --- .../sqllab/ShareSqlLabQuery_spec.jsx | 153 ++++++------------ ...reSqlLabQuery.jsx => ShareSqlLabQuery.tsx} | 121 +++++++------- 2 files changed, 115 insertions(+), 159 deletions(-) rename superset-frontend/src/SqlLab/components/{ShareSqlLabQuery.jsx => ShareSqlLabQuery.tsx} (64%) diff --git a/superset-frontend/spec/javascripts/sqllab/ShareSqlLabQuery_spec.jsx b/superset-frontend/spec/javascripts/sqllab/ShareSqlLabQuery_spec.jsx index fcbea6e6d..cb32ab182 100644 --- a/superset-frontend/spec/javascripts/sqllab/ShareSqlLabQuery_spec.jsx +++ b/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 }) => ( + + {children} + +); + +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( - , - ).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', () => { - expect.assertions(4); - const storeQuerySpy = jest.spyOn(utils, 'storeQuery'); - - 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(); + it('calls storeQuery() with the query when getCopyUrl() is called', async () => { + await act(async () => { + render(, { + wrapper: standardProvider, }); + }); + 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(, { + wrapper: standardProvider, + }); + }); const storeQuerySpy = jest.spyOn(utils, 'storeQuery'); - - const wrapper = setup(); - const instance = wrapper.instance(); - - instance.getCopyUrl(clipboardSpy); - + const button = screen.getByRole('button'); + userEvent.click(button); 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', () => { - const wrapper = setup({ + it('button is disabled and there is a request to save the query', async () => { + const updatedProps = { queryEditor: { ...defaultProps.queryEditor, remoteId: undefined, }, + }; + await act(async () => { + render(, { + wrapper: standardProvider, + }); }); - - expect(wrapper.find(CopyToClipboard)).toHaveLength(0); - expect(wrapper.find('.btn-disabled')).toHaveLength(1); - expect(wrapper.find(Tooltip)).toHaveProp( - 'title', - expect.stringContaining('Save the query'), - ); + const button = screen.getByRole('button', { name: /copy link/i }); + const style = window.getComputedStyle(button); + expect(style.color).toBe('rgb(102, 102, 102)'); }); }); }); diff --git a/superset-frontend/src/SqlLab/components/ShareSqlLabQuery.jsx b/superset-frontend/src/SqlLab/components/ShareSqlLabQuery.tsx similarity index 64% rename from superset-frontend/src/SqlLab/components/ShareSqlLabQuery.jsx rename to superset-frontend/src/SqlLab/components/ShareSqlLabQuery.tsx index e47d0cea4..66c709c5d 100644 --- a/superset-frontend/src/SqlLab/components/ShareSqlLabQuery.jsx +++ b/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 = { - queryEditor: PropTypes.shape({ - dbId: PropTypes.number, - title: PropTypes.string, - schema: PropTypes.string, - autorun: PropTypes.bool, - sql: PropTypes.string, - remoteId: PropTypes.number, - }).isRequired, - addDangerToast: PropTypes.func.isRequired, -}; +interface ShareSqlLabQueryPropTypes { + queryEditor: { + dbId: number; + title: string; + schema: string; + autorun: boolean; + sql: string; + remoteId: number | null; + }; + addDangerToast: (msg: string) => void; +} const Styles = styled.div` .btn-disabled { @@ -56,16 +55,12 @@ const Styles = styled.div` } `; -class ShareSqlLabQuery extends React.Component { - getCopyUrl(callback) { - if (isFeatureEnabled(FeatureFlag.SHARE_QUERIES_VIA_KV_STORE)) { - return this.getCopyUrlForKvStore(callback); - } - return this.getCopyUrlForSavedQuery(callback); - } - - getCopyUrlForKvStore(callback) { - const { dbId, title, schema, autorun, sql } = this.props.queryEditor; +function ShareSqlLabQuery({ + queryEditor, + addDangerToast, +}: ShareSqlLabQueryPropTypes) { + const getCopyUrlForKvStore = (callback: Function) => { + const { dbId, title, schema, autorun, sql } = 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 ( @@ -114,36 +115,38 @@ class ShareSqlLabQuery extends React.Component { ); - } + }; - render() { - const canShare = - this.props.queryEditor.remoteId || - isFeatureEnabled(FeatureFlag.SHARE_QUERIES_VIA_KV_STORE); - return ( - - {canShare ? ( - this.getCopyUrl(callback)} - wrapped={false} - copyNode={this.buildButton()} - /> - ) : ( - this.buildButton() - )} - - ); - } + const canShare = + queryEditor.remoteId || + isFeatureEnabled(FeatureFlag.SHARE_QUERIES_VIA_KV_STORE); + + return ( + + {canShare ? ( + + ) : ( + buildButton() + )} + + ); } -ShareSqlLabQuery.propTypes = propTypes; - export default withToasts(ShareSqlLabQuery);