[sqllab] more robust copy to clipboard (#6180)

* [sqllab] implement more robust share query button

* [sqllab] fix Hotkeys react warnings

* [CopyToClipboard] don't bind in render functions

* [explorer] fix short url button position

* [sqllab] remove share query item in tab dropdown menu

* [sqllab] reference new ShareSqlLabQuery component

* [sqllab][share query robustness] clean up

* [sqllab] fix tests

* [sqllab][share query] add unit tests

superset/assets/spec/javascripts/sqllab/CopyQueryTabUrl_spec.jsx

@@ -1,15 +0,0 @@
-import React from 'react';
-import { initialState } from './fixtures';
-
-import CopyQueryTabUrl from '../../../src/SqlLab/components/CopyQueryTabUrl';
-
-describe('CopyQueryTabUrl', () => {
-  const mockedProps = {
-    queryEditor: initialState.sqlLab.queryEditors[0],
-  };
-  it('is valid with props', () => {
-    expect(
-      React.isValidElement(<CopyQueryTabUrl {...mockedProps} />),
-    ).toBe(true);
-  });
-});

superset/assets/spec/javascripts/sqllab/ShareSqlLabQuery_spec.jsx

@@ -0,0 +1,91 @@
+import React from 'react';
+import configureStore from 'redux-mock-store';
+import thunk from 'redux-thunk';
+import { OverlayTrigger } from 'react-bootstrap';
+import fetchMock from 'fetch-mock';
+import { shallow } from 'enzyme';
+
+import * as utils from '../../../src/utils/common';
+import Button from '../../../src/components/Button';
+import ShareSqlLabQuery from '../../../src/SqlLab/components/ShareSqlLabQuery';
+
+const mockStore = configureStore([thunk]);
+const store = mockStore();
+
+describe('ShareSqlLabQuery', () => {
+  const storeQueryUrl = 'glob:*/kv/store/';
+  const storeQueryMockId = '123';
+
+  beforeEach(() => {
+    fetchMock.post(storeQueryUrl, () => ({ id: storeQueryMockId }), {
+      overwriteRoutes: true,
+    });
+    fetchMock.resetHistory();
+  });
+
+  afterAll(fetchMock.reset);
+
+  const defaultProps = {
+    queryEditor: {
+      dbId: 0,
+      title: 'query title',
+      schema: 'query_schema',
+      autorun: false,
+      sql: 'SELECT * FROM ...',
+    },
+  };
+
+  function setup(overrideProps) {
+    const wrapper = shallow(<ShareSqlLabQuery {...defaultProps} {...overrideProps} />, {
+      context: { store },
+    }).dive(); // wrapped in withToasts HOC
+
+    return wrapper;
+  }
+
+  it('renders an OverlayTrigger with Button', () => {
+    const wrapper = setup();
+    const trigger = wrapper.find(OverlayTrigger);
+    const button = trigger.find(Button);
+
+    expect(trigger).toHaveLength(1);
+    expect(button).toHaveLength(1);
+  });
+
+  it('calls storeQuery() with the query when getCopyUrl() is called and saves the url', () => {
+    expect.assertions(4);
+    const storeQuerySpy = jest.spyOn(utils, 'storeQuery');
+
+    const wrapper = setup();
+    const instance = wrapper.instance();
+
+    return instance.getCopyUrl().then(() => {
+      expect(storeQuerySpy.mock.calls).toHaveLength(1);
+      expect(fetchMock.calls(storeQueryUrl)).toHaveLength(1);
+      expect(storeQuerySpy.mock.calls[0][0]).toMatchObject(defaultProps.queryEditor);
+      expect(instance.state.shortUrl).toContain(storeQueryMockId);
+
+      return Promise.resolve();
+    });
+  });
+
+  it('dispatches an error toast upon fetching failure', () => {
+    expect.assertions(3);
+    const error = 'error';
+    const addDangerToastSpy = jest.fn();
+    fetchMock.post(storeQueryUrl, { throws: error }, { overwriteRoutes: true });
+    const wrapper = setup();
+    wrapper.setProps({ addDangerToast: addDangerToastSpy });
+
+    return wrapper
+      .instance()
+      .getCopyUrl()
+      .then(() => {
+        expect(fetchMock.calls(storeQueryUrl)).toHaveLength(1);
+        expect(addDangerToastSpy.mock.calls).toHaveLength(1);
+        expect(addDangerToastSpy.mock.calls[0][0]).toBe(error);
+
+        return Promise.resolve();
+      });
+  });
+});

superset/assets/src/SqlLab/components/CopyQueryTabUrl.jsx

@@ -1,60 +0,0 @@
-/* eslint no-alert: 0 */
-import React from 'react';
-import PropTypes from 'prop-types';
-import CopyToClipboard from '../../components/CopyToClipboard';
-import { storeQuery } from '../../utils/common';
-import { t } from '../../locales';
-
-const propTypes = {
-  queryEditor: PropTypes.object.isRequired,
-};
-
-export default class CopyQueryTabUrl extends React.PureComponent {
-  constructor(props) {
-    super(props);
-    this.getUrl = this.getUrl.bind(this);
-  }
-
-  getUrl(callback) {
-    const qe = this.props.queryEditor;
-    const sharedQuery = {
-      dbId: qe.dbId,
-      title: qe.title,
-      schema: qe.schema,
-      autorun: qe.autorun,
-      sql: qe.sql,
-    };
-
-    // the fetch call to get a url is async, but execCommand('copy') must be sync
-    // get around this with 2 timeouts. calling a timeout from within a timeout is not considered
-    // a short-lived, user-initiated sync event
-    let url;
-    storeQuery(sharedQuery).then((shareUrl) => { url = shareUrl; });
-    const longTimeout = setTimeout(() => { if (url) callback(url); }, 750);
-    setTimeout(() => {
-      if (url) {
-        callback(url);
-        clearTimeout(longTimeout);
-      }
-    }, 150);
-
-  }
-
-  render() {
-    return (
-      <CopyToClipboard
-        inMenu
-        copyNode={
-          <div>
-            <i className="fa fa-clipboard" /> <span>{t('share query')}</span>
-          </div>
-        }
-        tooltipText={t('copy URL to clipboard')}
-        shouldShowText={false}
-        getText={this.getUrl}
-      />
-    );
-  }
-}
-
-CopyQueryTabUrl.propTypes = propTypes;

superset/assets/src/SqlLab/components/ShareQuery.jsx

@@ -1,22 +0,0 @@
-import React from 'react';
-
-import CopyToClipboard from '../../components/CopyToClipboard';
-import CopyQueryTabUrl from './CopyQueryTabUrl';
-import Button from '../../components/Button';
-import { t } from '../../locales';
-
-export default class ShareQuery extends CopyQueryTabUrl {
-  render() {
-    return (
-      <CopyToClipboard
-        copyNode={(
-          <Button bsSize="small" className="toggleSave">
-            <i className="fa fa-clipboard" /> {t('Share Query')}
-          </Button>
-      )}
-        tooltipText={t('copy URL to clipboard')}
-        shouldShowText={false}
-        getText={this.getUrl}
-      />);
-  }
-}

superset/assets/src/SqlLab/components/ShareSqlLabQuery.jsx

@@ -0,0 +1,80 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+import { Popover, OverlayTrigger } from 'react-bootstrap';
+
+import Button from '../../components/Button';
+import CopyToClipboard from '../../components/CopyToClipboard';
+import { storeQuery } from '../../utils/common';
+import { getClientErrorObject } from '../../modules/utils';
+import { t } from '../../locales';
+import withToasts from '../../messageToasts/enhancers/withToasts';
+
+const propTypes = {
+  queryEditor: PropTypes.shape({
+    dbId: PropTypes.number,
+    title: PropTypes.string,
+    schema: PropTypes.string,
+    autorun: PropTypes.bool,
+    sql: PropTypes.string,
+  }).isRequired,
+  addDangerToast: PropTypes.func.isRequired,
+};
+
+class ShareSqlLabQuery extends React.Component {
+  constructor(props) {
+    super(props);
+    this.state = {
+      shortUrl: 'Loading ...',
+      showOverlay: false,
+    };
+    this.getCopyUrl = this.getCopyUrl.bind(this);
+  }
+
+  getCopyUrl() {
+    const { dbId, title, schema, autorun, sql } = this.props.queryEditor;
+    const sharedQuery = { dbId, title, schema, autorun, sql };
+
+    return storeQuery(sharedQuery)
+      .then((shortUrl) => {
+        this.setState({ shortUrl });
+      })
+      .catch((response) => {
+        getClientErrorObject(response).then(({ error }) => {
+          this.props.addDangerToast(error);
+          this.setState({ shortUrl: t('Error') });
+        });
+      });
+  }
+
+  renderPopover() {
+    return (
+      <Popover id="sqllab-shareurl-popover">
+        <CopyToClipboard
+          text={this.state.shortUrl || 'Loading ...'}
+          copyNode={<i className="fa fa-clipboard" title={t('Copy to clipboard')} />}
+        />
+      </Popover>
+    );
+  }
+
+  render() {
+    return (
+      <OverlayTrigger
+        trigger="click"
+        placement="top"
+        onEnter={this.getCopyUrl}
+        rootClose
+        shouldUpdatePosition
+        overlay={this.renderPopover()}
+      >
+        <Button bsSize="small" className="toggleSave">
+          <i className="fa fa-clipboard" /> {t('Share Query')}
+        </Button>
+      </OverlayTrigger>
+    );
+  }
+}
+
+ShareSqlLabQuery.propTypes = propTypes;
+
+export default withToasts(ShareSqlLabQuery);

superset/assets/src/SqlLab/components/SqlEditor.jsx

@@ -19,7 +19,7 @@ import Button from '../../components/Button';
 import TemplateParamsEditor from './TemplateParamsEditor';
 import SouthPane from './SouthPane';
 import SaveQuery from './SaveQuery';
-import ShareQuery from './ShareQuery';
+import ShareSqlLabQuery from './ShareSqlLabQuery';
 import Timer from '../../components/Timer';
 import Hotkeys from '../../components/Hotkeys';
 import SqlEditorLeftBar from './SqlEditorLeftBar';
@@ -235,7 +235,7 @@ class SqlEditor extends React.PureComponent {
               />
             </span>
             <span className="m-r-5">
-              <ShareQuery queryEditor={qe} />
+              <ShareSqlLabQuery queryEditor={qe} />
             </span>
             {ctasControls}
             <span className="m-l-5">

superset/assets/src/SqlLab/components/TabbedSqlEditors.jsx

@@ -7,7 +7,6 @@ import URI from 'urijs';
 
 import * as Actions from '../actions';
 import SqlEditor from './SqlEditor';
-import CopyQueryTabUrl from './CopyQueryTabUrl';
 import { areArraysShallowEqual } from '../../reduxUtils';
 import { t } from '../../locales';
 import TabStatusIcon from './TabStatusIcon';
@@ -189,8 +188,7 @@ class TabbedSqlEditors extends React.PureComponent {
               </div>
               {t('Rename tab')}
             </MenuItem>
-            {qe && <CopyQueryTabUrl queryEditor={qe} />}
-            <MenuItem eventKey="4" onClick={this.toggleLeftBar.bind(this)}>
+            <MenuItem eventKey="3" onClick={this.toggleLeftBar.bind(this)}>
               <div className="icon-container">
                 <i className="fa fa-cogs" />
               </div>

superset/assets/src/components/CopyToClipboard.jsx

@@ -32,6 +32,7 @@ export default class CopyToClipboard extends React.Component {
     this.copyToClipboard = this.copyToClipboard.bind(this);
     this.resetTooltipText = this.resetTooltipText.bind(this);
     this.onMouseOut = this.onMouseOut.bind(this);
+    this.onClick = this.onClick.bind(this);
   }
 
   onMouseOut() {
@@ -105,7 +106,7 @@ export default class CopyToClipboard extends React.Component {
           overlay={this.renderTooltip()}
           trigger={['hover']}
           bsStyle="link"
-          onClick={this.onClick.bind(this)}
+          onClick={this.onClick}
           onMouseOut={this.onMouseOut}
         >
           {this.props.copyNode}
@@ -119,7 +120,7 @@ export default class CopyToClipboard extends React.Component {
       <OverlayTrigger placement="top" overlay={this.renderTooltip()} trigger={['hover']}>
         <MenuItem>
           <span
-            onClick={this.onClick.bind(this)}
+            onClick={this.onClick}
             onMouseOut={this.onMouseOut}
           >
             {this.props.copyNode}
@@ -138,13 +139,7 @@ export default class CopyToClipboard extends React.Component {
   }
 
   render() {
-    let html;
-    if (this.props.inMenu) {
-      html = this.renderInMenu();
-    } else {
-      html = this.renderLink();
-    }
-    return html;
+    return this.props.inMenu ? this.renderInMenu() : this.renderLink();
   }
 }
 

superset/assets/src/components/Hotkeys.jsx

@@ -26,7 +26,7 @@ export default class Hotkeys extends React.PureComponent {
     const { header, hotkeys } = this.props;
 
     return (
-      <Popover title={header} style={{ width: '300px' }}>
+      <Popover id="hotkey-popover" title={header} style={{ width: '300px' }}>
         <table className="table table-condensed">
           <thead>
             <tr>
@@ -36,7 +36,7 @@ export default class Hotkeys extends React.PureComponent {
           </thead>
           <tbody>
             {hotkeys.map(({ key, descr }) => (
-              <tr>
+              <tr key={key}>
                 <td><code>{key}</code></td>
                 <td>{descr}</td>
               </tr>

superset/assets/src/components/URLShortLinkButton.jsx

@@ -54,6 +54,7 @@ class URLShortLinkButton extends React.Component {
       <OverlayTrigger
         trigger="click"
         rootClose
+        shouldUpdatePosition
         placement="left"
         onEnter={this.getCopyUrl}
         overlay={this.renderPopover()}