Explore control panel - Chart control, TimeFilter, GroupBy, Filters #1205

vera-liu · 2016-09-27T17:42:28Z

Sorry I should have divided this to smaller PRs, will work on Todos in future smaller PR
needs-review @ascott

Done:

Endpoint for data attached to a datasource
Chart options, Time Filter, GroupBy, SQL and Filters component
redux-thunk for getting column options for one datasouce
actions/reducers for updating states from form-data
populates fields in control panel with url params retrieved from bootstrap_data.viz.form_data

Todo:

Renovate ugly UI, use panel/O2 stuff
Add support for druid
Multi-select for filter values: (as in here) Filter Enhancement (#933) - Filter Value Dropdown #1061
Tests

Side note:
data for slice stored in { bootstrapData.viz.data }
data for pre-populating form stored in { bootstrapData.viz.form_data }

* create structure for new forked explore view * update component name * add bootstrap data pattern * remove console.log

* Created store and reducers * Added spec * Modifications based on comments

* Associate version to entry files * Modified path joins for configs * Made changes based on comments

GroupBy and Filters

ascott · 2016-09-28T19:08:45Z

caravel/assets/javascripts/explorev2/actions/exploreActions.js

+export const SET_ROW_LIMIT = 'SET_ROW_LIMIT';
+export const TOGGLE_SEARCHBOX = 'TOGGLE_SEARCHBOX';
+export const SET_FILTER_COLUMN_OPTS = 'SET_FILTER_COLUMN_OPTS';
+export const SET_WHERE_CLAUSE = 'SET_SET_WHERE_CLAUSE';


extra SET_ in value

ascott · 2016-09-28T19:09:05Z

caravel/assets/javascripts/explorev2/actions/exploreActions.js

+export const TOGGLE_SEARCHBOX = 'TOGGLE_SEARCHBOX';
+export const SET_FILTER_COLUMN_OPTS = 'SET_FILTER_COLUMN_OPTS';
+export const SET_WHERE_CLAUSE = 'SET_SET_WHERE_CLAUSE';
+export const SET_HAVING_CLAUSE = 'SET_SET_HAVING_CLAUSE';


ascott · 2016-09-28T19:11:37Z

caravel/assets/javascripts/explorev2/actions/exploreActions.js

+    const timeGrainOpts = [];
+
+    if (datasourceId) {
+      const params = [];


could simplify this to:

const params = [`datasource_id=${datasourceId}`, `datasource_type=${datasourceType}`];

ascott · 2016-09-28T19:22:58Z

caravel/assets/javascripts/explorev2/components/ChartControl.jsx

+import React from 'react';
+import Select from 'react-select';
+import { bindActionCreators } from 'redux';
+import * as actions from '../actions/exploreActions';


rather than importing all actions here i would suggest just importing the ones needed for this component...

import { setFormOpts, setDatasource, resetFormData, setVizType } as actions from '../actions/exploreActions';

ascott · 2016-09-28T19:24:34Z

caravel/assets/javascripts/explorev2/components/ChartControl.jsx

+}
+
+ChartControl.propTypes = {
+  actions: React.PropTypes.object,


i think it makes more sense to import the actions in this component rather than passing them in as props.

I saw Sqllab components were doing this, I think it's for mapDispatchToProps, so that we can bind action creators into one, then dispatch from the component props

ascott · 2016-09-28T19:25:16Z

caravel/assets/javascripts/explorev2/components/ChartControl.jsx

+class ChartControl extends React.Component {
+  componentWillMount() {
+    if (this.props.datasourceId) {
+      this.props.actions.setFormOpts(this.props.datasourceId, this.props.datasourceType);


once we import actions above we can use them like this:
setFormOpts(this.props.datasourceId, this.props.datasourceType);

makes more sense to import the actions in this component rather than passing them in as props.

This didn't work.. Suggested by the redux doc here: http://redux.js.org/docs/basics/Actions.html#action-creators
We should bind action creators to dispatch() function, then map dispatch to component's props. I could still explicitly import actions like:
import { setFormOpts, setDatasource, resetFormData, setVizType } from '../actions/exploreActions';

and then do this in mapDispatchToProps:
actions: bindActionCreators({ setFormOpts, setDatasource, resetFormData, setVizType, }, dispatch),
But dispatching actions would still be the same:

this.props.actions.setDatasource(val);

Is there a better way to do this?
@ascott

ah ok, maybe i am carrying over some Alt patterns that don't work in redux. so when you call bindActionCreators the actions are passed as props to the component?

The way I understand it is that bindActionCreators bind all action creators with dispatch function, so that we don't have to write redundant onClick() => dispatch(actionFunc()) statements. mapDispatchToProps() allows us to call dispatch function from the component's props, instead of accessing global store everytime we dispatch.

ascott · 2016-09-28T19:27:14Z

caravel/assets/javascripts/explorev2/actions/exploreActions.js

+  return { type: CLEAR_ALL_OPTS };
+}
+
+export function setFormOpts(datasourceId, datasourceType) {


i think this function belongs in an actionCreators file but it we can change that in a subsequent pr

ascott · 2016-09-28T19:32:23Z

caravel/views.py

+    @expose("/fetch_datasource_metadata")
+    @log_this
+    def fetch_datasource_metadata(self):
+        # TODO: check permissions


doesn't @has_access do permission checking for us?

I think it's only checking access to the endpoint, but we need access checking for datasource/slice here

ah gotcha, good catch

ascott · 2016-09-28T19:34:06Z

caravel/assets/utils/common.js

+  return enhancer;
+}
+
+export const VIZ_TYPES = [


i think this belongs in a explore/constants.js file

ascott · 2016-09-28T19:35:50Z

caravel/assets/javascripts/explorev2/index.jsx

+
+import { exploreReducer } from './reducers/exploreReducer';
+
+const metrics = [];


const metrics = [bootstrapData.viz.form_data.metric]

ascott · 2016-09-28T19:37:33Z

caravel/assets/javascripts/explorev2/components/TimeFilter.jsx

+import * as actions from '../actions/exploreActions';
+import { connect } from 'react-redux';
+
+const sinceOptions = ['1 hour ago', '12 hours ago', '1 day ago',


could move these time constants to explore/constants.js

ascott · 2016-09-28T19:41:14Z

caravel/assets/javascripts/explorev2/components/TimeFilter.jsx

+}
+
+TimeFilter.propTypes = {
+  actions: React.PropTypes.object,


it's a nice pattern to put proptypes at the top of the file like this:

const propTypes = { actions: React.PropTypes.object, timeColumnOpts: React.PropTypes.array, timeColumn: React.PropTypes.string, timeGrainOpts: React.PropTypes.array, timeGrain: React.PropTypes.string, since: React.PropTypes.string, until: React.PropTypes.string, }; const defaultProps = { ... };

and then here at the bottom:

TimeFilter.propTypes = propTypes; TimeFilter.defaultProps = defaultProps;

ascott · 2016-09-28T19:54:45Z

caravel/assets/javascripts/explorev2/components/GroupBy.jsx

+            <Select
+              multi
+              name="select-since"
+              placeholder={'Select metrics'}


don't need curly brackets here
placeholder="Select metrics"

ascott · 2016-09-28T19:55:39Z

caravel/assets/javascripts/explorev2/components/SqlClause.jsx

+  actions: React.PropTypes.object,
+};
+
+SqlClause.defaultProps = {


we can delete this if there are no default props

ascott · 2016-09-28T19:56:31Z

caravel/assets/javascripts/explorev2/components/TimeFilter.jsx

+            <Select
+              className="col-sm-6"
+              name="select-time-column"
+              placeholder={'Select a time column'}


placeholder="Select a time column"

ascott · 2016-09-28T19:58:43Z

caravel/assets/javascripts/explorev2/index.jsx

+import { createStore, applyMiddleware, compose } from 'redux';
+import { Provider } from 'react-redux';
+import thunk from 'redux-thunk';
+import { getDevEnhancer } from '../../utils/common';


i moved this and other redux utils to a common reduxUtils.js file https://github.com/ascott/caravel/blob/master/caravel/assets/javascripts/reduxUtils.js#L67

ascott · 2016-09-28T20:04:59Z

this generally looking good!

i think we could improve the organization of our components, but that can be in a subsequent PR.
i like they way the redux docs use the pattern of container and presentational components: http://redux.js.org/docs/basics/UsageWithReact.html#implementing-components

seems like there maybe some conflicts with master, but we can resolve those once we merge this into alanna-explore-v2-main

another PR for the js tests would be 💯

🚢

ascott · 2016-09-29T21:59:24Z

🎉

* Explore control panel - Chart control, TimeFilter, GroupBy, Filters (#1205) * create structure for new forked explore view (#1099) * create structure for new forked explore view * update component name * add bootstrap data pattern * remove console.log * Associate version to entry files (#1060) * Associate version to entry files * Modified path joins for configs * Made changes based on comments * Created store and reducers (#1108) * Created store and reducers * Added spec * Modifications based on comments * Explore control panel components: Chart control, Time filter, SQL, GroupBy and Filters * Modifications based on comments * accommodate old and new explore urls * move bootstrap data up in scope * fix code climate issues * fix long lines * fix syntax error

…pache#1205) * create structure for new forked explore view (apache#1099) * create structure for new forked explore view * update component name * add bootstrap data pattern * remove console.log * Associate version to entry files (apache#1060) * Associate version to entry files * Modified path joins for configs * Made changes based on comments * Created store and reducers (apache#1108) * Created store and reducers * Added spec * Modifications based on comments * Explore control panel components: Chart control, Time filter, SQL, GroupBy and Filters * Modifications based on comments

* Explore control panel - Chart control, TimeFilter, GroupBy, Filters (#1205) * create structure for new forked explore view (#1099) * create structure for new forked explore view * update component name * add bootstrap data pattern * remove console.log * Associate version to entry files (#1060) * Associate version to entry files * Modified path joins for configs * Made changes based on comments * Created store and reducers (#1108) * Created store and reducers * Added spec * Modifications based on comments * Explore control panel components: Chart control, Time filter, SQL, GroupBy and Filters * Modifications based on comments * Added access check + Druid in endpoint * pull grains to constants * Switch explore.html to old version

…pache#1205) * create structure for new forked explore view (apache#1099) * create structure for new forked explore view * update component name * add bootstrap data pattern * remove console.log * Associate version to entry files (apache#1060) * Associate version to entry files * Modified path joins for configs * Made changes based on comments * Created store and reducers (apache#1108) * Created store and reducers * Added spec * Modifications based on comments * Explore control panel components: Chart control, Time filter, SQL, GroupBy and Filters * Modifications based on comments

…pache#1205) * fix(plugin-chart-echarts): x-filtering improvement for radar chart * fix(plugin-chart-echarts): opacity constants * fix(plugin-chart-echarts): improve constant * fix(plugin-chart-echarts): place treemap constant

ascott force-pushed the alanna-explore-v2-main branch from dcfb604 to b67906c Compare September 27, 2016 19:02

Alanna Scott and others added 3 commits September 27, 2016 12:18

create structure for new forked explore view (apache#1099)

cf084e6

* create structure for new forked explore view * update component name * add bootstrap data pattern * remove console.log

Created store and reducers (apache#1108)

2353549

* Created store and reducers * Added spec * Modifications based on comments

Associate version to entry files (apache#1060)

b75ee66

* Associate version to entry files * Modified path joins for configs * Made changes based on comments

vera-liu force-pushed the vliu_explore_control branch 7 times, most recently from 50c5a95 to 29df846 Compare September 28, 2016 05:46

vera-liu mentioned this pull request Sep 28, 2016

[explore v2] add control components for TimeFilter, GroupBy, Sql and Filter #1156

Closed

vera-liu force-pushed the vliu_explore_control branch 3 times, most recently from 02cc616 to f80bae7 Compare September 28, 2016 18:40

vera-liu changed the title ~~Not ready for review yet...explore controls~~ Explore control panel - Chart control, TimeFilter, GroupBy, Filters Sep 28, 2016

Explore control panel components: Chart control, Time filter, SQL,

ee61d17

GroupBy and Filters

vera-liu force-pushed the vliu_explore_control branch from f80bae7 to ee61d17 Compare September 28, 2016 19:00

ascott reviewed Sep 28, 2016

View reviewed changes

Modifications based on comments

6ad3bf7

vera-liu force-pushed the vliu_explore_control branch from 2ec83db to 6ad3bf7 Compare September 28, 2016 23:14

vera-liu merged commit c0e9d2b into apache:alanna-explore-v2-main Sep 29, 2016

ascott mentioned this pull request Oct 3, 2016

[explore-v2] setup, basic layout, control panels, v2 url #1233

Merged

ym97 mentioned this pull request Oct 27, 2016

Explore control panel - Chart control, TimeFilter, GroupBy, Filters (… #1447

Closed

pinkythalli97 mentioned this pull request Oct 29, 2016

Explore control panel - Chart control, TimeFilter, GroupBy, Filters (… #1476

Closed

dpgaspar mentioned this pull request Dec 9, 2019

Bump FAB to 2.2.1 #8793

Merged

12 tasks

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.5.0 labels Feb 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Explore control panel - Chart control, TimeFilter, GroupBy, Filters #1205

Explore control panel - Chart control, TimeFilter, GroupBy, Filters #1205

vera-liu commented Sep 27, 2016 •

edited

ascott Sep 28, 2016 •

edited

ascott Sep 28, 2016

ascott Sep 28, 2016

ascott Sep 28, 2016 •

edited

ascott Sep 28, 2016 •

edited

vera-liu Sep 28, 2016

ascott Sep 28, 2016 •

edited

vera-liu Sep 28, 2016 •

edited

ascott Sep 29, 2016

vera-liu Sep 29, 2016

ascott Sep 29, 2016

ascott Sep 28, 2016

ascott Sep 28, 2016

vera-liu Sep 28, 2016

ascott Sep 29, 2016

ascott Sep 28, 2016

ascott Sep 28, 2016

ascott Sep 28, 2016

ascott Sep 28, 2016

ascott Sep 28, 2016

ascott Sep 28, 2016

ascott Sep 28, 2016

ascott Sep 28, 2016 •

edited

ascott commented Sep 28, 2016

ascott commented Sep 29, 2016


		import { exploreReducer } from './reducers/exploreReducer';

		const metrics = [];

Explore control panel - Chart control, TimeFilter, GroupBy, Filters #1205

Explore control panel - Chart control, TimeFilter, GroupBy, Filters #1205

Conversation

vera-liu commented Sep 27, 2016 • edited

ascott Sep 28, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ascott Sep 28, 2016 • edited

Choose a reason for hiding this comment

ascott Sep 28, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ascott Sep 28, 2016 • edited

Choose a reason for hiding this comment

vera-liu Sep 28, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ascott Sep 28, 2016 • edited

Choose a reason for hiding this comment

ascott commented Sep 28, 2016

ascott commented Sep 29, 2016

vera-liu commented Sep 27, 2016 •

edited

ascott Sep 28, 2016 •

edited

ascott Sep 28, 2016 •

edited

ascott Sep 28, 2016 •

edited

ascott Sep 28, 2016 •

edited

vera-liu Sep 28, 2016 •

edited

ascott Sep 28, 2016 •

edited