Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added url shortner for sharing query link #1314

Merged
merged 5 commits into from
Oct 25, 2016
Merged

Added url shortner for sharing query link #1314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0373f1d
Added url shortner for sharing query link
vera-liu Oct 11, 2016
ad662cc
Move shortener outside CopyToClipboard and move ajax call to common.js
vera-liu Oct 11, 2016
b0bac1e
transfer dbId to int to avoid failed prop
vera-liu Oct 12, 2016
eaf51d8
Merge branch 'master' into vliu_share_query_shortner
vera-liu Oct 24, 2016
cbc21d2
Merge branch 'master' into vliu_share_query_shortner
vera-liu Oct 24, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions caravel/assets/javascripts/SqlLab/components/CopyQueryTabUrl.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import CopyToClipboard from '../../components/CopyToClipboard';
import { getShortUrl } from '../../../utils/common';

const propTypes = {
qe: React.PropTypes.object,
Expand All @@ -12,18 +13,12 @@ const defaultProps = {
export default class CopyQueryTabUrl extends React.Component {
constructor(props) {
super(props);
const uri = window.location.toString();
const search = window.location.search;
const cleanUri = search ? uri.substring(0, uri.indexOf('?')) : uri;
const query = search.substring(1);
this.state = {
uri,
cleanUri,
query,
shortUrl: '',
};
}

getQueryLink() {
componentWillMount() {
const params = [];
const qe = this.props.qe;
if (qe.dbId) params.push('dbid=' + qe.dbId);
Expand All @@ -33,16 +28,21 @@ export default class CopyQueryTabUrl extends React.Component {
if (qe.sql) params.push('sql=' + encodeURIComponent(qe.sql));

const queryString = params.join('&');
const queryLink = this.state.cleanUri + '?' + queryString;
const queryLink = window.location.pathname + '?' + queryString;
getShortUrl(queryLink, this.onShortUrlSuccess.bind(this));
}

return queryLink;
onShortUrlSuccess(data) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like you are not handling error case here, what setting the state directly in the componentWillMount ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for the error case the state won't get updated, so the shortUrl stays as empty string.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm a big fan of more, smaller methods, rather than putting everything in line. this seems cleaner than putting in directly in componentWillMount, imo.

this.setState({
shortUrl: data,
});
}

render() {
return (
<CopyToClipboard
inMenu
text={this.getQueryLink()}
text={this.state.shortUrl}
copyNode={<span>share query</span>}
tooltipText="copy URL to clipboard"
shouldShowText={false}
Expand Down
14 changes: 2 additions & 12 deletions caravel/assets/javascripts/SqlLab/components/TabbedSqlEditors.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { bindActionCreators } from 'redux';
import * as Actions from '../actions';
import SqlEditor from './SqlEditor';
import shortid from 'shortid';
import { getParamFromQuery, getLink } from '../../../utils/common';
import { getParamFromQuery } from '../../../utils/common';
import CopyQueryTabUrl from './CopyQueryTabUrl';

const propTypes = {
Expand Down Expand Up @@ -41,7 +41,7 @@ class TabbedSqlEditors extends React.Component {
const queryEditorProps = {
id: shortid.generate(),
title: getParamFromQuery(this.state.query, 'title'),
dbId: getParamFromQuery(this.state.query, 'dbid'),
dbId: parseInt(getParamFromQuery(this.state.query, 'dbid'), 10),
Copy link
Contributor Author

@vera-liu vera-liu Oct 12, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously the database was not set in DatabaseSelect when loading the shared url because the passed string failed the prop as integer. Fixed it here

schema: getParamFromQuery(this.state.query, 'schema'),
autorun: getParamFromQuery(this.state.query, 'autorun'),
sql: getParamFromQuery(this.state.query, 'sql'),
Expand All @@ -51,16 +51,6 @@ class TabbedSqlEditors extends React.Component {
window.history.replaceState({}, document.title, this.state.cleanUri);
}
}
getQueryLink(qe) {
const params = [];
if (qe.dbId) params.push('dbid=' + qe.dbId);
if (qe.title) params.push('title=' + qe.title);
if (qe.schema) params.push('schema=' + qe.schema);
if (qe.autorun) params.push('autorun=' + qe.autorun);
if (qe.sql) params.push('sql=' + qe.sql);

return getLink(this.state.cleanUri, params);
}
renameTab(qe) {
/* eslint no-alert: 0 */
const newTitle = prompt('Enter a new title for the tab');
Expand Down
31 changes: 10 additions & 21 deletions caravel/assets/javascripts/explore/components/URLShortLinkButton.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { PropTypes } from 'react';
import { Popover, OverlayTrigger } from 'react-bootstrap';
import CopyToClipboard from './../../components/CopyToClipboard';
import $ from 'jquery';
import { getShortUrl } from '../../../utils/common';

const propTypes = {
slice: PropTypes.object.isRequired,
Expand All @@ -15,28 +15,17 @@ export default class URLShortLinkButton extends React.Component {
};
}

getShortUrl() {
$.ajax({
type: 'POST',
url: '/r/shortner/',
data: {
data: '/' + window.location.pathname + window.location.search,
},
success: (data) => {
this.setState({
shortUrl: data,
});
},
error: (error) => {
/* eslint no-console: 0 */
if (console && console.warn) {
console.warn('Something went wrong...');
console.warn(error);
}
},
onShortUrlSuccess(data) {
this.setState({
shortUrl: data,
});
}

getCopyUrl() {
const longUrl = window.location.pathname + window.location.search;
getShortUrl(longUrl, this.onShortUrlSuccess.bind(this));
}

renderPopover() {
return (
<Popover id="shorturl-popover">
Expand All @@ -54,7 +43,7 @@ export default class URLShortLinkButton extends React.Component {
trigger="click"
rootClose
placement="left"
onEnter={this.getShortUrl.bind(this)}
onEnter={this.getCopyUrl.bind(this)}
overlay={this.renderPopover()}
>
<span className="btn btn-default btn-sm">
Expand Down
21 changes: 21 additions & 0 deletions caravel/assets/utils/common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* eslint global-require: 0 */
import $ from 'jquery';
const d3 = window.d3 || require('d3');

export const EARTH_CIRCUMFERENCE_KM = 40075.16;
Expand Down Expand Up @@ -53,3 +54,23 @@ export function getParamsFromUrl() {
});
return newParams;
}

export function getShortUrl(longUrl, callBack) {
$.ajax({
type: 'POST',
url: '/r/shortner/',
data: {
data: '/' + longUrl,
},
success: (data) => {
callBack(data);
},
error: (error) => {
/* eslint no-console: 0 */
if (console && console.warn) {
console.warn('Something went wrong...');
console.warn(error);
}
},
});
}