This is an automated email from the ASF dual-hosted git repository.
ccwilliams pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new 90809f6 [sqllab] more robust copy to clipboard (#6180)
90809f6 is described below
commit 90809f6d39010aa889ae6bbc5b1ec2fe4afdc903
Author: Chris Williams <willias...@users.noreply.github.com>
AuthorDate: Wed Oct 24 19:41:32 2018 -0700
[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
---
.../javascripts/sqllab/CopyQueryTabUrl_spec.jsx | 15 ----
.../javascripts/sqllab/ShareSqlLabQuery_spec.jsx | 91 ++++++++++++++++++++++
.../src/SqlLab/components/CopyQueryTabUrl.jsx | 60 --------------
.../assets/src/SqlLab/components/ShareQuery.jsx | 22 ------
.../src/SqlLab/components/ShareSqlLabQuery.jsx | 80 +++++++++++++++++++
.../assets/src/SqlLab/components/SqlEditor.jsx | 4 +-
.../src/SqlLab/components/TabbedSqlEditors.jsx | 4 +-
superset/assets/src/components/CopyToClipboard.jsx | 13 +---
superset/assets/src/components/Hotkeys.jsx | 4 +-
.../assets/src/components/URLShortLinkButton.jsx | 1 +
10 files changed, 181 insertions(+), 113 deletions(-)
diff --git a/superset/assets/spec/javascripts/sqllab/CopyQueryTabUrl_spec.jsx
b/superset/assets/spec/javascripts/sqllab/CopyQueryTabUrl_spec.jsx
deleted file mode 100644
index dd23016..0000000
--- a/superset/assets/spec/javascripts/sqllab/CopyQueryTabUrl_spec.jsx
+++ /dev/null
@@ -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);
- });
-});
diff --git a/superset/assets/spec/javascripts/sqllab/ShareSqlLabQuery_spec.jsx
b/superset/assets/spec/javascripts/sqllab/ShareSqlLabQuery_spec.jsx
new file mode 100644
index 0000000..4746a0b
--- /dev/null
+++ b/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();
+ });
+ });
+});
diff --git a/superset/assets/src/SqlLab/components/CopyQueryTabUrl.jsx
b/superset/assets/src/SqlLab/components/CopyQueryTabUrl.jsx
deleted file mode 100644
index 7042268..0000000
--- a/superset/assets/src/SqlLab/components/CopyQueryTabUrl.jsx
+++ /dev/null
@@ -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;
diff --git a/superset/assets/src/SqlLab/components/ShareQuery.jsx
b/superset/assets/src/SqlLab/components/ShareQuery.jsx
deleted file mode 100644
index 48e8330..0000000
--- a/superset/assets/src/SqlLab/components/ShareQuery.jsx
+++ /dev/null
@@ -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}
- />);
- }
-}
diff --git a/superset/assets/src/SqlLab/components/ShareSqlLabQuery.jsx
b/superset/assets/src/SqlLab/components/ShareSqlLabQuery.jsx
new file mode 100644
index 0000000..55f8c88
--- /dev/null
+++ b/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);
diff --git a/superset/assets/src/SqlLab/components/SqlEditor.jsx
b/superset/assets/src/SqlLab/components/SqlEditor.jsx
index 3dd9d48..7f82e02 100644
--- a/superset/assets/src/SqlLab/components/SqlEditor.jsx
+++ b/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">
diff --git a/superset/assets/src/SqlLab/components/TabbedSqlEditors.jsx
b/superset/assets/src/SqlLab/components/TabbedSqlEditors.jsx
index e693295..9bf3978 100644
--- a/superset/assets/src/SqlLab/components/TabbedSqlEditors.jsx
+++ b/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>
diff --git a/superset/assets/src/components/CopyToClipboard.jsx
b/superset/assets/src/components/CopyToClipboard.jsx
index 9514b18..c2b9c57 100644
--- a/superset/assets/src/components/CopyToClipboard.jsx
+++ b/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();
}
}
diff --git a/superset/assets/src/components/Hotkeys.jsx
b/superset/assets/src/components/Hotkeys.jsx
index fbecee8..97d8c95 100644
--- a/superset/assets/src/components/Hotkeys.jsx
+++ b/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>
diff --git a/superset/assets/src/components/URLShortLinkButton.jsx
b/superset/assets/src/components/URLShortLinkButton.jsx
index b0570c0..47fd332 100644
--- a/superset/assets/src/components/URLShortLinkButton.jsx
+++ b/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()}
