You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by gr...@apache.org on 2018/03/08 21:19:45 UTC

[incubator-superset] branch master updated: [Explore] Save custom url parameters when user save slices (#4578)

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

graceguo 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 9edbd64  [Explore] Save custom url parameters when user save slices (#4578)
9edbd64 is described below

commit 9edbd64c5d2ac05c7a4a9eba8bb6de0d1ba4de03
Author: Grace Guo <gr...@airbnb.com>
AuthorDate: Thu Mar 8 13:19:41 2018 -0800

    [Explore] Save custom url parameters when user save slices (#4578)
    
    * [Explore] Save url parameters when user save slices
    
    * remove print
    
    (cherry picked from commit bd9ecbe)
    
    * add unit test
    
    (cherry picked from commit 0f350ad)
    
    * wrapping all request params into url_params
    
    (cherry picked from commit 17197c1)
---
 .../explore/components/ExploreViewContainer.jsx         |  7 +++++++
 superset/assets/javascripts/explore/index.jsx           |  2 ++
 superset/assets/javascripts/explore/stores/store.js     |  7 +++++++
 superset/jinja_context.py                               | 15 +++++++++++----
 superset/utils.py                                       |  9 +++++++++
 superset/views/core.py                                  |  8 +++++++-
 tests/utils_tests.py                                    | 17 ++++++++++++++++-
 7 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/superset/assets/javascripts/explore/components/ExploreViewContainer.jsx b/superset/assets/javascripts/explore/components/ExploreViewContainer.jsx
index e1b7acb..a254ded 100644
--- a/superset/assets/javascripts/explore/components/ExploreViewContainer.jsx
+++ b/superset/assets/javascripts/explore/components/ExploreViewContainer.jsx
@@ -293,6 +293,13 @@ ExploreViewContainer.propTypes = propTypes;
 
 function mapStateToProps({ explore, charts, impressionId }) {
   const form_data = getFormDataFromControls(explore.controls);
+  // fill in additional params stored in form_data but not used by control
+  Object.keys(explore.rawFormData)
+    .forEach((key) => {
+      if (form_data[key] === undefined) {
+        form_data[key] = explore.rawFormData[key];
+      }
+    });
   const chartKey = Object.keys(charts)[0];
   const chart = charts[chartKey];
   return {
diff --git a/superset/assets/javascripts/explore/index.jsx b/superset/assets/javascripts/explore/index.jsx
index 35eb68d..74f5a0b 100644
--- a/superset/assets/javascripts/explore/index.jsx
+++ b/superset/assets/javascripts/explore/index.jsx
@@ -25,6 +25,7 @@ initJQueryAjax();
 const exploreViewContainer = document.getElementById('app');
 const bootstrapData = JSON.parse(exploreViewContainer.getAttribute('data-bootstrap'));
 const controls = getControlsState(bootstrapData, bootstrapData.form_data);
+const rawFormData = { ...bootstrapData.form_data };
 delete bootstrapData.form_data;
 delete bootstrapData.common.locale;
 delete bootstrapData.common.language_pack;
@@ -32,6 +33,7 @@ delete bootstrapData.common.language_pack;
 // Initial state
 const bootstrappedState = Object.assign(
   bootstrapData, {
+    rawFormData,
     controls,
     filterColumnOpts: [],
     isDatasourceMetaLoading: false,
diff --git a/superset/assets/javascripts/explore/stores/store.js b/superset/assets/javascripts/explore/stores/store.js
index 02bd121..4f1e7b5 100644
--- a/superset/assets/javascripts/explore/stores/store.js
+++ b/superset/assets/javascripts/explore/stores/store.js
@@ -109,6 +109,13 @@ export function applyDefaultFormData(form_data) {
       formData[k] = form_data[k];
     }
   });
+  // fill in additional params stored in form_data but not used by control
+  Object.keys(form_data)
+    .forEach((key) => {
+      if (formData[key] === undefined) {
+        formData[key] = form_data[key];
+      }
+    });
   return formData;
 }
 
diff --git a/superset/jinja_context.py b/superset/jinja_context.py
index fe93594..ce8795a 100644
--- a/superset/jinja_context.py
+++ b/superset/jinja_context.py
@@ -7,6 +7,7 @@ from __future__ import unicode_literals
 
 from datetime import datetime, timedelta
 import inspect
+import json
 import random
 import time
 import uuid
@@ -30,15 +31,21 @@ BASE_CONTEXT.update(config.get('JINJA_CONTEXT_ADDONS', {}))
 
 
 def url_param(param, default=None):
-    """Get a url paramater
+    """Get a url or post data parameter
 
-    :param param: the url parameter to lookup
+    :param param: the parameter to lookup
     :type param: str
     :param default: the value to return in the absence of the parameter
     :type default: str
     """
-    print(request.args)
-    return request.args.get(param, default)
+    if request.args.get(param):
+        return request.args.get(param, default)
+    # Supporting POST as well as get
+    if request.form.get('form_data'):
+        form_data = json.loads(request.form.get('form_data'))
+        url_params = form_data['url_params'] or {}
+        return url_params.get(param, default)
+    return default
 
 
 def current_user_id():
diff --git a/superset/utils.py b/superset/utils.py
index c60f128..ab3cef8 100644
--- a/superset/utils.py
+++ b/superset/utils.py
@@ -835,6 +835,15 @@ def merge_extra_filters(form_data):
         del form_data['extra_filters']
 
 
+def merge_request_params(form_data, params):
+    url_params = {}
+    for key, value in params.items():
+        if key in ('form_data', 'r'):
+            continue
+        url_params[key] = value
+    form_data['url_params'] = url_params
+
+
 def get_update_perms_flag():
     val = os.environ.get('SUPERSET_UPDATE_PERMS')
     return val.lower() not in ('0', 'false', 'no') if val else True
diff --git a/superset/views/core.py b/superset/views/core.py
index d5cd168..4330e7d 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -43,7 +43,9 @@ from superset.legacy import cast_form_data
 import superset.models.core as models
 from superset.models.sql_lab import Query
 from superset.sql_parse import SupersetQuery
-from superset.utils import has_access, merge_extra_filters, QueryStatus
+from superset.utils import (
+    has_access, merge_extra_filters, merge_request_params, QueryStatus,
+)
 from .base import (
     api, BaseSupersetView, CsvResponse, DeleteMixin,
     generate_download_headers, get_error_msg, get_user_roles,
@@ -1255,6 +1257,10 @@ class Superset(BaseSupersetView):
         # On explore, merge extra filters into the form data
         merge_extra_filters(form_data)
 
+        # merge request url params
+        if request.method == 'GET':
+            merge_request_params(form_data, request.args)
+
         # handle save or overwrite
         action = request.args.get('action')
 
diff --git a/tests/utils_tests.py b/tests/utils_tests.py
index 1728189..674c6f4 100644
--- a/tests/utils_tests.py
+++ b/tests/utils_tests.py
@@ -14,7 +14,8 @@ import numpy
 
 from superset.utils import (
     base_json_conv, datetime_f, json_int_dttm_ser, json_iso_dttm_ser,
-    JSONEncodedDict, memoized, merge_extra_filters, parse_human_timedelta,
+    JSONEncodedDict, memoized, merge_extra_filters, merge_request_params,
+    parse_human_timedelta,
     SupersetException, validate_json, zlib_compress, zlib_decompress_to_string,
 )
 
@@ -216,6 +217,20 @@ class UtilsTestCase(unittest.TestCase):
         merge_extra_filters(form_data)
         self.assertEquals(form_data, expected)
 
+    def test_merge_request_params(self):
+        form_data = {
+            'since': '2000',
+            'until': 'now',
+        }
+        url_params = {
+            'form_data': form_data,
+            'dashboard_ids': '(1,2,3,4,5)',
+        }
+        merge_request_params(form_data, url_params)
+        self.assertIn('url_params', form_data.keys())
+        self.assertIn('dashboard_ids', form_data['url_params'])
+        self.assertNotIn('form_data', form_data.keys())
+
     def test_datetime_f(self):
         self.assertEquals(
             datetime_f(datetime(1990, 9, 21, 19, 11, 19, 626096)),

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