Changes based on comments

superset/assets/javascripts/SqlLab/actions.js

@@ -298,3 +298,23 @@ export function removeTable(table) {
 export function refreshQueries(alteredQueries) {
   return { type: REFRESH_QUERIES, alteredQueries };
 }
+
+export function popStoredQuery(urlId) {
+  return function (dispatch) {
+    $.ajax({
+      type: 'GET',
+      url: `/kv/${urlId}`,
+      success: (data) => {
+        const newQuery = JSON.parse(data);
+        const queryEditorProps = {
+          title: newQuery.title ? newQuery.title : 'shared query',
+          dbId: newQuery.dbId ? parseInt(newQuery.dbId, 10) : null,
+          schema: newQuery.schema ? newQuery.schema : null,
+          autorun: newQuery.autorun ? newQuery.autorun : false,
+          sql: newQuery.sql ? newQuery.sql : 'SELECT ...',
+        };
+        dispatch(addQueryEditor(queryEditorProps));
+      },
+    });
+  };
+}

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

@@ -1,6 +1,6 @@
 import React from 'react';
 import CopyToClipboard from '../../components/CopyToClipboard';
-import { getQueryLink } from '../../../utils/common';
+import { storeQuery } from '../../../utils/common';
 
 const propTypes = {
   queryEditor: React.PropTypes.object.isRequired,
@@ -16,7 +16,7 @@ export default class CopyQueryTabUrl extends React.PureComponent {
       autorun: qe.autorun,
       sql: qe.sql,
     };
-    getQueryLink(sharedQuery, callback);
+    storeQuery(sharedQuery, callback);
   }
 
   render() {

superset/assets/javascripts/SqlLab/components/QueryTable.jsx

@@ -10,7 +10,7 @@ import ModalTrigger from '../../components/ModalTrigger';
 import HighlightedSql from './HighlightedSql';
 import { STATE_BSSTYLE_MAP } from '../constants';
 import { fDuration } from '../../modules/dates';
-import { getQueryLink } from '../../../utils/common';
+import { storeQuery } from '../../../utils/common';
 
 const propTypes = {
   columns: React.PropTypes.array,
@@ -48,7 +48,7 @@ class QueryTable extends React.PureComponent {
       schema,
       sql,
     };
-    getQueryLink(newQuery, this.callback);
+    storeQuery(newQuery, this.callback);
   }
   hideVisualizeModal() {
     this.setState({ showVisualizeModal: false });
@@ -107,7 +107,7 @@ class QueryTable extends React.PureComponent {
         <div style={{ width: '100px' }}>
           <button
             className="btn btn-link btn-xs"
-            onClick={this.openQuery.bind(this, q.schema, q.dbId, q.sql)}
+            onClick={this.openQuery.bind(this, q.dbId, q.schema, q.sql)}
           >
             <i className="fa fa-external-link" />Open in SQL Editor
           </button>

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

@@ -7,7 +7,6 @@ import SqlEditor from './SqlEditor';
 import CopyQueryTabUrl from './CopyQueryTabUrl';
 import { areArraysShallowEqual } from '../../reduxUtils';
 import { getParamFromQuery } from '../../../utils/common';
-const $ = require('jquery');
 
 const propTypes = {
   actions: React.PropTypes.object.isRequired,
@@ -29,44 +28,39 @@ let queryCount = 1;
 class TabbedSqlEditors extends React.PureComponent {
   constructor(props) {
     super(props);
-    const cleanUri = '/superset/sqllab';
-    let query = null;
-    const search = window.location.search;
-    if (search) {
-      const urlId = getParamFromQuery(search.substring(1), 'id');
-      $.ajax({
-        type: 'GET',
-        async: false,
-        url: `/kv/${urlId}`,
-        success(data) {
-          query = JSON.parse(data);
-        },
-      });
-    }
+    const sqlLabUrl = '/superset/sqllab';
     this.state = {
-      cleanUri,
-      query,
+      sqlLabUrl,
       queriesArray: [],
       dataPreviewQueries: [],
       hideLeftBar: false,
     };
   }
-  componentWillMount() {
-    if (this.state.query) {
-      queryCount++;
-      const newQuery = this.state.query;
-      const queryEditorProps = {
-        title: newQuery.title ? newQuery.title : 'shared query',
-        dbId: newQuery.dbId ? parseInt(newQuery.dbId, 10) : null,
-        schema: newQuery.schema ? newQuery.schema : null,
-        autorun: newQuery.autorun ? newQuery.autorun : false,
-        sql: newQuery.sql ? newQuery.sql : 'SELECT ...',
-      };
-      this.props.actions.addQueryEditor(queryEditorProps);
-      // Clean the url in browser history
-      window.history.replaceState({}, document.title, this.state.cleanUri);
+  componentDidMount() {
+    const search = window.location.search;
+    if (search) {
+      const queryString = search.substring(1);
+      const urlId = getParamFromQuery(queryString, 'id');
+      if (urlId) {
+        this.props.actions.popStoredQuery(urlId);
+      } else {
+        const newQueryEditor = {
+          title: getParamFromQuery(queryString, 'title'),
+          dbId: getParamFromQuery(queryString, 'dbid'),
+          schema: getParamFromQuery(queryString, 'schema'),
+          autorun: getParamFromQuery(queryString, 'autorun'),
+          sql: getParamFromQuery(queryString, 'sql'),
+        };
+        this.props.actions.addQueryEditor(newQueryEditor);
+      }
+      this.popNewTab();
     }
   }
+  popNewTab() {
+    queryCount++;
+    // Clean the url in browser history
+    window.history.replaceState({}, document.title, this.state.sqlLabUrl);
+  }
   componentWillReceiveProps(nextProps) {
     const nextActiveQeId = nextProps.tabHistory[nextProps.tabHistory.length - 1];
     const queriesArray = [];

superset/assets/utils/common.js

@@ -39,16 +39,17 @@ export function getParamFromQuery(query, param) {
   return null;
 }
 
-export function getQueryLink(query, callback) {
+export function storeQuery(query, callback) {
   $.ajax({
     type: 'POST',
     url: '/kv/store/',
     async: false,
     data: {
-      baseUrl: 'superset/sqllab',
       data: JSON.stringify(query),
     },
-    success: (url) => {
+    success: (data) => {
+      const baseUrl = window.location.origin + window.location.pathname;
+      const url = `${baseUrl}?id=${JSON.parse(data).id}`;
       callback(url);
     },
   });

superset/migrations/versions/bcf3126872fc_add_keyvalue.py

@@ -17,14 +17,8 @@
 def upgrade():
     ### commands auto generated by Alembic - please adjust! ###
     op.create_table('keyvalue',
-    sa.Column('created_on', sa.DateTime(), nullable=True),
-    sa.Column('changed_on', sa.DateTime(), nullable=True),
     sa.Column('id', sa.Integer(), nullable=False),
     sa.Column('value', sa.Text(), nullable=False),
-    sa.Column('changed_by_fk', sa.Integer(), nullable=True),
-    sa.Column('created_by_fk', sa.Integer(), nullable=True),
-    sa.ForeignKeyConstraint(['changed_by_fk'], [u'ab_user.id'], ),
-    sa.ForeignKeyConstraint(['created_by_fk'], [u'ab_user.id'], ),
     sa.PrimaryKeyConstraint('id')
     )
     ### end Alembic commands ###

superset/models.py

@@ -210,7 +210,7 @@ class Url(Model, AuditMixinNullable):
     url = Column(Text)
 
 
-class KeyValue(Model, AuditMixinNullable):
+class KeyValue(Model):
 
     """Used for any type of key-value store"""
 

superset/views.py

@@ -18,7 +18,7 @@
 import sqlalchemy as sqla
 
 from flask import (
-    g, request, redirect, flash, Response, render_template, Markup)
+    g, request, redirect, flash, Response, render_template, Markup, url_for)
 from flask_appbuilder import ModelView, CompactCRUDMixin, BaseView, expose
 from flask_appbuilder.actions import action
 from flask_appbuilder.models.sqla.interface import SQLAInterface
@@ -1106,28 +1106,31 @@ def ping():
 
 class KV(BaseSupersetView):
 
-    """used for storing and retrieving key value pairs"""
+    """Used for storing and retrieving key value pairs"""
 
     @log_this
-    @expose("/store/", methods=['POST', 'GET'])
+    @expose("/store/", methods=['POST'])
     def store(self):
-        value = request.form.get('data')
-        base_url = request.form.get('baseUrl') \
-            if request.form.get('baseUrl') else ''
-        obj = models.KeyValue(value=value)
-        db.session.add(obj)
-        db.session.commit()
-        return("http://{request.headers[Host]}/{base_url}?id={obj.id}".format(
-            request=request, base_url=base_url, obj=obj))
+        try:
+            value = request.form.get('data')
+            obj = models.KeyValue(value=value)
+            db.session.add(obj)
+            db.session.commit()
+        except Exception as e:
+            return json_error_response(e)
+        return Response(
+            json.dumps({'id': obj.id}),
+            status=200)
 
     @log_this
-    @expose("/<key_id>/")
+    @expose("/<key_id>/", methods=['GET'])
     def get_value(self, key_id):
-        kv = db.session.query(models.KeyValue).filter_by(id=key_id).first()
-        if kv:
-            return Response(kv.value, status=200)
-        else:
-            return Response("Error: Value cannot be retrieved", status=404)
+        kv = None
+        try:
+            kv = db.session.query(models.KeyValue).filter_by(id=key_id).one()
+        except Exception as e:
+            return json_error_response(e)
+        return Response(kv.value, status=200)
 
 appbuilder.add_view_no_menu(KV)
 

tests/core_tests.py

@@ -259,15 +259,28 @@ def test_shortner(self):
     def test_kv(self):
         self.logout()
         self.login(username='admin')
-        data = {
-            'data': 'this is a test',
-        }
-        resp = self.client.post('/kv/store/', data=dict(data=json.dumps(data)))
+
+        try:
+            resp = self.client.post('/kv/store/', data=dict())
+        except Exception as e:
+            self.assertRaises(TypeError)
+
+        value = json.dumps({'data': 'this is a test'})
+        resp = self.client.post('/kv/store/', data=dict(data=value))
+        self.assertEqual(resp.status_code, 200)
         kv = db.session.query(models.KeyValue).first()
         kv_value = kv.value
-        assert 'this is a test' in kv_value
-        kv_id = 'id={}'.format(kv.id)
-        assert kv_id in resp.data.decode('utf-8')
+        self.assertEqual(json.loads(value), json.loads(kv_value))
+
+        resp = self.client.get('/kv/{}/'.format(kv.id))
+        self.assertEqual(resp.status_code, 200)
+        self.assertEqual(json.loads(value),
+            json.loads(resp.data.decode('utf-8')))
+
+        try:
+            resp = self.client.get('/kv/10001/')
+        except Exception as e:
+            self.assertRaises(TypeError)
 
     def test_save_dash(self, username='admin'):
         self.login(username=username)