You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by hu...@apache.org on 2018/06/02 18:08:59 UTC

[incubator-superset] branch master updated: URL shortner for dashboards (#4760)

This is an automated email from the ASF dual-hosted git repository.

hugh 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 dc21e0d  URL shortner for dashboards (#4760)
dc21e0d is described below

commit dc21e0dd789cb23e895de6346ab60890a8790885
Author: Tamika Tannis <tt...@alum.mit.edu>
AuthorDate: Sat Jun 2 11:08:43 2018 -0700

    URL shortner for dashboards (#4760)
    
    * Added support for URLShortLinkButton to work for the dashboard case
    
    * Fix lint errors and test
    
    * Change references to 'slice' to 'chart'.
    
    * Add unit tests to improve coverage
    
    * Fixing lint errors
    
    * Refactor to make URLShortLink more generic. Remove history modification code, redirect should be handling this.
    
    * Remove history modification code, redirect should be handling this
    
    * Generate a shorter link without the directory, and delegate default linked to the contents of window.location
    
    * Fix lint errors
    
    * Fix test_shortner test to check for new pattern
    
    * Remove usage of addHistory to manipulate explore shortlink redirection
    
    * Address build failure and using better practices for shortlink defaults
    
    * Fixing alphabetical order
    
    * More syntax mistakes
    
    * Revert explore view history changes
    
    * Fix use of component props, & rebase
---
 .../components/URLShortLinkButton_spec.jsx         |  8 ++++----
 .../components/URLShortLinkButton.jsx              | 24 ++++++++++++++--------
 .../assets/src/dashboard/components/Header.jsx     |  7 +++++++
 .../explore/components/ExploreActionButtons.jsx    | 11 +++++++---
 superset/views/core.py                             |  5 ++---
 tests/core_tests.py                                |  3 ++-
 6 files changed, 38 insertions(+), 20 deletions(-)

diff --git a/superset/assets/spec/javascripts/explore/components/URLShortLinkButton_spec.jsx b/superset/assets/spec/javascripts/components/URLShortLinkButton_spec.jsx
similarity index 76%
rename from superset/assets/spec/javascripts/explore/components/URLShortLinkButton_spec.jsx
rename to superset/assets/spec/javascripts/components/URLShortLinkButton_spec.jsx
index 986daaa..98e6e2c 100644
--- a/superset/assets/spec/javascripts/explore/components/URLShortLinkButton_spec.jsx
+++ b/superset/assets/spec/javascripts/components/URLShortLinkButton_spec.jsx
@@ -4,13 +4,13 @@ import { describe, it } from 'mocha';
 import { shallow } from 'enzyme';
 
 import { OverlayTrigger } from 'react-bootstrap';
-import URLShortLinkButton from '../../../../src/explore/components/URLShortLinkButton';
+import URLShortLinkButton from '../../../src/components/URLShortLinkButton';
 
 describe('URLShortLinkButton', () => {
   const defaultProps = {
-    slice: {
-      querystring: () => 'query string',
-    },
+    url: 'mockURL',
+    emailSubject: 'Mock Subject',
+    emailContent: 'mock content',
   };
 
   it('renders', () => {
diff --git a/superset/assets/src/explore/components/URLShortLinkButton.jsx b/superset/assets/src/components/URLShortLinkButton.jsx
similarity index 65%
rename from superset/assets/src/explore/components/URLShortLinkButton.jsx
rename to superset/assets/src/components/URLShortLinkButton.jsx
index 2238524..aa9ae96 100644
--- a/superset/assets/src/explore/components/URLShortLinkButton.jsx
+++ b/superset/assets/src/components/URLShortLinkButton.jsx
@@ -1,13 +1,14 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 import { Popover, OverlayTrigger } from 'react-bootstrap';
-import CopyToClipboard from './../../components/CopyToClipboard';
-import { getShortUrl } from '../../utils/common';
-import { getExploreLongUrl } from '../exploreUtils';
-import { t } from '../../locales';
+import CopyToClipboard from './CopyToClipboard';
+import { getShortUrl } from '../utils/common';
+import { t } from '../locales';
 
 const propTypes = {
-  latestQueryFormData: PropTypes.object.isRequired,
+  url: PropTypes.string,
+  emailSubject: PropTypes.string,
+  emailContent: PropTypes.string,
 };
 
 export default class URLShortLinkButton extends React.Component {
@@ -25,12 +26,11 @@ export default class URLShortLinkButton extends React.Component {
   }
 
   getCopyUrl() {
-    const longUrl = getExploreLongUrl(this.props.latestQueryFormData);
-    getShortUrl(longUrl, this.onShortUrlSuccess.bind(this));
+    getShortUrl(this.props.url, this.onShortUrlSuccess.bind(this));
   }
 
   renderPopover() {
-    const emailBody = t('Check out this chart: %s', this.state.shortUrl);
+    const emailBody = t('%s%s', this.props.emailContent, this.state.shortUrl);
     return (
       <Popover id="shorturl-popover">
         <CopyToClipboard
@@ -38,7 +38,7 @@ export default class URLShortLinkButton extends React.Component {
           copyNode={<i className="fa fa-clipboard" title={t('Copy to clipboard')} />}
         />
         &nbsp;&nbsp;
-        <a href={`mailto:?Subject=Superset%20Slice%20&Body=${emailBody}`}>
+        <a href={`mailto:?Subject=${this.props.emailSubject}%20&Body=${emailBody}`}>
           <i className="fa fa-envelope" />
         </a>
       </Popover>
@@ -62,4 +62,10 @@ export default class URLShortLinkButton extends React.Component {
   }
 }
 
+URLShortLinkButton.defaultProps = {
+  url: window.location.href.substring(window.location.origin.length),
+  emailSubject: '',
+  emailContent: '',
+};
+
 URLShortLinkButton.propTypes = propTypes;
diff --git a/superset/assets/src/dashboard/components/Header.jsx b/superset/assets/src/dashboard/components/Header.jsx
index 52d3024..eabd3f4 100644
--- a/superset/assets/src/dashboard/components/Header.jsx
+++ b/superset/assets/src/dashboard/components/Header.jsx
@@ -5,6 +5,7 @@ import Controls from './Controls';
 import EditableTitle from '../../components/EditableTitle';
 import Button from '../../components/Button';
 import FaveStar from '../../components/FaveStar';
+import URLShortLinkButton from '../../components/URLShortLinkButton';
 import InfoTooltipWithTrigger from '../../components/InfoTooltipWithTrigger';
 import { t } from '../../locales';
 
@@ -92,6 +93,12 @@ class Header extends React.PureComponent {
           </h1>
         </div>
         <div className="pull-right" style={{ marginTop: '35px' }}>
+          <span className="m-r-5">
+            <URLShortLinkButton
+              emailSubject="Superset Dashboard"
+              emailContent="Check out this dashboard: "
+            />
+          </span>
           {this.renderEditButton()}
           <Controls
             dashboard={dashboard}
diff --git a/superset/assets/src/explore/components/ExploreActionButtons.jsx b/superset/assets/src/explore/components/ExploreActionButtons.jsx
index ec9d214..f383d66 100644
--- a/superset/assets/src/explore/components/ExploreActionButtons.jsx
+++ b/superset/assets/src/explore/components/ExploreActionButtons.jsx
@@ -1,11 +1,11 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 import cx from 'classnames';
-import URLShortLinkButton from './URLShortLinkButton';
+import URLShortLinkButton from '../../components/URLShortLinkButton';
 import EmbedCodeButton from './EmbedCodeButton';
 import DisplayQueryButton from './DisplayQueryButton';
 import { t } from '../../locales';
-import { exportChart } from '../exploreUtils';
+import { exportChart, getExploreLongUrl } from '../exploreUtils';
 
 const propTypes = {
   canDownload: PropTypes.oneOfType([PropTypes.string, PropTypes.bool]).isRequired,
@@ -25,7 +25,12 @@ export default function ExploreActionButtons({
   return (
     <div className="btn-group results" role="group">
       {latestQueryFormData &&
-        <URLShortLinkButton latestQueryFormData={latestQueryFormData} />}
+        <URLShortLinkButton
+          url={getExploreLongUrl(latestQueryFormData)}
+          emailSubject="Superset Chart"
+          emailContent="Check out this chart: "
+        />
+      }
 
       {latestQueryFormData &&
         <EmbedCodeButton latestQueryFormData={latestQueryFormData} />}
diff --git a/superset/views/core.py b/superset/views/core.py
index b4a1689..14380c5 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -745,13 +745,12 @@ class R(BaseSupersetView):
     @expose('/shortner/', methods=['POST', 'GET'])
     def shortner(self):
         url = request.form.get('data')
-        directory = url.split('?')[0][2:]
         obj = models.Url(url=url)
         db.session.add(obj)
         db.session.commit()
         return Response(
-            '{scheme}://{request.headers[Host]}/{directory}?r={obj.id}'.format(
-                scheme=request.scheme, request=request, directory=directory, obj=obj),
+            '{scheme}://{request.headers[Host]}/r/{obj.id}'.format(
+                scheme=request.scheme, request=request, obj=obj),
             mimetype='text/plain')
 
     @expose('/msg/')
diff --git a/tests/core_tests.py b/tests/core_tests.py
index 34d8a64..dd6e3d8 100644
--- a/tests/core_tests.py
+++ b/tests/core_tests.py
@@ -13,6 +13,7 @@ import json
 import logging
 import os
 import random
+import re
 import string
 import unittest
 
@@ -377,7 +378,7 @@ class CoreTests(SupersetTestCase):
             'previous_viz_type=sankey'
         )
         resp = self.client.post('/r/shortner/', data=dict(data=data))
-        assert '?r=' in resp.data.decode('utf-8')
+        assert re.search(r'\/r\/[0-9]+', resp.data.decode('utf-8'))
 
     def test_kv(self):
         self.logout()

-- 
To stop receiving notification emails like this one, please contact
hugh@apache.org.