You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ma...@apache.org on 2017/11/11 05:33:35 UTC
[incubator-superset] branch master updated: [Explore] Altered Slice
Tag (#3668)
This is an automated email from the ASF dual-hosted git repository.
maximebeauchemin 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 4d48d5d [Explore] Altered Slice Tag (#3668)
4d48d5d is described below
commit 4d48d5d854e23d469c80105a6e384e3c4db8942d
Author: Jeff Niu <je...@gmail.com>
AuthorDate: Fri Nov 10 21:33:31 2017 -0800
[Explore] Altered Slice Tag (#3668)
* Added altered tag to explore slice view and fixes #3616
* unit tests
* Moved getDiffs logic into AlteredSliceTag
* code style fixs
---
.../javascripts/components/AlteredSliceTag.jsx | 145 +++++++++++++
.../explore/components/ExploreChartHeader.jsx | 10 +
superset/assets/package.json | 1 +
.../components/AlteredSliceTag_spec.jsx | 235 +++++++++++++++++++++
superset/utils.py | 36 +++-
tests/utils_tests.py | 87 +++++++-
6 files changed, 510 insertions(+), 4 deletions(-)
diff --git a/superset/assets/javascripts/components/AlteredSliceTag.jsx b/superset/assets/javascripts/components/AlteredSliceTag.jsx
new file mode 100644
index 0000000..eb24424
--- /dev/null
+++ b/superset/assets/javascripts/components/AlteredSliceTag.jsx
@@ -0,0 +1,145 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+import { Table, Tr, Td, Thead, Th } from 'reactable';
+import { isEqual, isEmpty } from 'underscore';
+
+import TooltipWrapper from './TooltipWrapper';
+import { controls } from '../explore/stores/controls';
+import ModalTrigger from './ModalTrigger';
+import { t } from '../locales';
+
+const propTypes = {
+ origFormData: PropTypes.object.isRequired,
+ currentFormData: PropTypes.object.isRequired,
+};
+
+export default class AlteredSliceTag extends React.Component {
+
+ constructor(props) {
+ super(props);
+ const diffs = this.getDiffs(props);
+ this.state = { diffs, hasDiffs: !isEmpty(diffs) };
+ }
+
+ componentWillReceiveProps(newProps) {
+ // Update differences if need be
+ if (isEqual(this.props, newProps)) {
+ return;
+ }
+ const diffs = this.getDiffs(newProps);
+ this.setState({ diffs, hasDiffs: !isEmpty(diffs) });
+ }
+
+ getDiffs(props) {
+ // Returns all properties that differ in the
+ // current form data and the saved form data
+ const ofd = props.origFormData;
+ const cfd = props.currentFormData;
+ const fdKeys = Object.keys(cfd);
+ const diffs = {};
+ for (const fdKey of fdKeys) {
+ // Ignore values that are undefined/nonexisting in either
+ if (!ofd[fdKey] && !cfd[fdKey]) {
+ continue;
+ }
+ if (!isEqual(ofd[fdKey], cfd[fdKey])) {
+ diffs[fdKey] = { before: ofd[fdKey], after: cfd[fdKey] };
+ }
+ }
+ return diffs;
+ }
+
+ formatValue(value, key) {
+ // Format display value based on the control type
+ // or the value type
+ if (value === undefined) {
+ return 'N/A';
+ } else if (value === null) {
+ return 'null';
+ } else if (controls[key] && controls[key].type === 'FilterControl') {
+ if (!value.length) {
+ return '[]';
+ }
+ return value.map((v) => {
+ const filterVal = v.val.constructor === Array ? `[${v.val.join(', ')}]` : v.val;
+ return `${v.col} ${v.op} ${filterVal}`;
+ }).join(', ');
+ } else if (controls[key] && controls[key].type === 'BoundsControl') {
+ return `Min: ${value[0]}, Max: ${value[1]}`;
+ } else if (controls[key] && controls[key].type === 'CollectionControl') {
+ return value.map(v => JSON.stringify(v)).join(', ');
+ } else if (typeof value === 'boolean') {
+ return value ? 'true' : 'false';
+ } else if (value.constructor === Array) {
+ return value.length ? value.join(', ') : '[]';
+ } else if (typeof value === 'string' || typeof value === 'number') {
+ return value;
+ }
+ return JSON.stringify(value);
+ }
+
+ renderRows() {
+ const diffs = this.state.diffs;
+ const rows = [];
+ for (const key in diffs) {
+ rows.push(
+ <Tr key={key}>
+ <Td column="control" data={(controls[key] && controls[key].label) || key} />
+ <Td column="before">{this.formatValue(diffs[key].before, key)}</Td>
+ <Td column="after">{this.formatValue(diffs[key].after, key)}</Td>
+ </Tr>,
+ );
+ }
+ return rows;
+ }
+
+ renderModalBody() {
+ return (
+ <Table className="table" sortable>
+ <Thead>
+ <Th column="control">Control</Th>
+ <Th column="before">Before</Th>
+ <Th column="after">After</Th>
+ </Thead>
+ {this.renderRows()}
+ </Table>
+ );
+ }
+
+ renderTriggerNode() {
+ return (
+ <TooltipWrapper
+ label="difference"
+ tooltip={t('Click to see difference')}
+ >
+ <span
+ className="label label-warning m-l-5"
+ style={{ fontSize: '12px' }}
+ >
+ {t('Altered')}
+ </span>
+ </TooltipWrapper>
+ );
+ }
+
+ render() {
+ // Return nothing if there are no differences
+ if (!this.state.hasDiffs) {
+ return null;
+ }
+ // Render the label-warning 'Altered' tag which the user may
+ // click to open a modal containing a table summarizing the
+ // differences in the slice
+ return (
+ <ModalTrigger
+ animation
+ triggerNode={this.renderTriggerNode()}
+ modalTitle={t('Slice changes')}
+ bsSize="large"
+ modalBody={this.renderModalBody()}
+ />
+ );
+ }
+}
+
+AlteredSliceTag.propTypes = propTypes;
diff --git a/superset/assets/javascripts/explore/components/ExploreChartHeader.jsx b/superset/assets/javascripts/explore/components/ExploreChartHeader.jsx
index 8c2e1f3..3750fc0 100644
--- a/superset/assets/javascripts/explore/components/ExploreChartHeader.jsx
+++ b/superset/assets/javascripts/explore/components/ExploreChartHeader.jsx
@@ -4,6 +4,7 @@ import PropTypes from 'prop-types';
import { chartPropType } from '../../chart/chartReducer';
import ExploreActionButtons from './ExploreActionButtons';
import EditableTitle from '../../components/EditableTitle';
+import AlteredSliceTag from '../../components/AlteredSliceTag';
import FaveStar from '../../components/FaveStar';
import TooltipWrapper from '../../components/TooltipWrapper';
import Timer from '../../components/Timer';
@@ -54,6 +55,13 @@ class ExploreChartHeader extends React.PureComponent {
});
}
+ renderAlteredTag() {
+ const origFormData = (this.props.slice && this.props.slice.form_data) || {};
+ const currentFormData = this.props.form_data;
+ const tagProps = { origFormData, currentFormData };
+ return (<AlteredSliceTag {...tagProps} />);
+ }
+
renderChartTitle() {
let title;
if (this.props.slice) {
@@ -106,6 +114,8 @@ class ExploreChartHeader extends React.PureComponent {
</span>
}
+ {this.renderAlteredTag()}
+
<div className="pull-right">
{this.props.chart.chartStatus === 'success' &&
queryResponse &&
diff --git a/superset/assets/package.json b/superset/assets/package.json
index 43aa268..99f8afc 100644
--- a/superset/assets/package.json
+++ b/superset/assets/package.json
@@ -96,6 +96,7 @@
"srcdoc-polyfill": "^1.0.0",
"supercluster": "https://github.com/georgeke/supercluster/tarball/ac3492737e7ce98e07af679623aad452373bbc40",
"urijs": "^1.18.10",
+ "underscore": "^1.8.3",
"viewport-mercator-project": "^2.1.0"
},
"devDependencies": {
diff --git a/superset/assets/spec/javascripts/components/AlteredSliceTag_spec.jsx b/superset/assets/spec/javascripts/components/AlteredSliceTag_spec.jsx
new file mode 100644
index 0000000..f3b746b
--- /dev/null
+++ b/superset/assets/spec/javascripts/components/AlteredSliceTag_spec.jsx
@@ -0,0 +1,235 @@
+import React from 'react';
+import { shallow } from 'enzyme';
+import { describe, it } from 'mocha';
+import { expect } from 'chai';
+
+import { Table, Thead, Td, Th, Tr } from 'reactable';
+
+import AlteredSliceTag from '../../../javascripts/components/AlteredSliceTag';
+import ModalTrigger from '../../../javascripts/components/ModalTrigger';
+import TooltipWrapper from '../../../javascripts/components/TooltipWrapper';
+
+const defaultProps = {
+ origFormData: {
+ filters: [{ col: 'a', op: '==', val: 'hello' }],
+ y_axis_bounds: [10, 20],
+ column_collection: [{ 1: 'a', b: ['6', 'g'] }],
+ bool: false,
+ alpha: undefined,
+ gucci: [1, 2, 3, 4],
+ never: 5,
+ ever: { a: 'b', c: 'd' },
+ },
+ currentFormData: {
+ filters: [{ col: 'b', op: 'in', val: ['hello', 'my', 'name'] }],
+ y_axis_bounds: [15, 16],
+ column_collection: [{ 1: 'a', b: [9, '15'], t: 'gggg' }],
+ bool: true,
+ alpha: null,
+ gucci: ['a', 'b', 'c', 'd'],
+ never: 10,
+ ever: { x: 'y', z: 'z' },
+ },
+};
+
+const expectedDiffs = {
+ filters: {
+ before: [{ col: 'a', op: '==', val: 'hello' }],
+ after: [{ col: 'b', op: 'in', val: ['hello', 'my', 'name'] }],
+ },
+ y_axis_bounds: {
+ before: [10, 20],
+ after: [15, 16],
+ },
+ column_collection: {
+ before: [{ 1: 'a', b: ['6', 'g'] }],
+ after: [{ 1: 'a', b: [9, '15'], t: 'gggg' }],
+ },
+ bool: {
+ before: false,
+ after: true,
+ },
+ gucci: {
+ before: [1, 2, 3, 4],
+ after: ['a', 'b', 'c', 'd'],
+ },
+ never: {
+ before: 5,
+ after: 10,
+ },
+ ever: {
+ before: { a: 'b', c: 'd' },
+ after: { x: 'y', z: 'z' },
+ },
+};
+
+describe('AlteredSliceTag', () => {
+ let wrapper;
+ let props;
+
+ beforeEach(() => {
+ props = Object.assign({}, defaultProps);
+ wrapper = shallow(<AlteredSliceTag {...props} />);
+ });
+
+ it('correctly determines form data differences', () => {
+ const diffs = wrapper.instance().getDiffs(props);
+ expect(diffs).to.deep.equal(expectedDiffs);
+ expect(wrapper.instance().state.diffs).to.deep.equal(expectedDiffs);
+ expect(wrapper.instance().state.hasDiffs).to.equal(true);
+ });
+
+ it('does not run when there are no differences', () => {
+ props = {
+ origFormData: props.origFormData,
+ currentFormData: props.origFormData,
+ };
+ wrapper = shallow(<AlteredSliceTag {...props} />);
+ expect(wrapper.instance().state.diffs).to.deep.equal({});
+ expect(wrapper.instance().state.hasDiffs).to.equal(false);
+ expect(wrapper.instance().render()).to.equal(null);
+ });
+
+ it('sets new diffs when receiving new props', () => {
+ const newProps = {
+ currentFormData: Object.assign({}, props.currentFormData),
+ origFormData: Object.assign({}, props.origFormData),
+ };
+ newProps.currentFormData.beta = 10;
+ wrapper = shallow(<AlteredSliceTag {...props} />);
+ wrapper.instance().componentWillReceiveProps(newProps);
+ const newDiffs = wrapper.instance().state.diffs;
+ const expectedBeta = { before: undefined, after: 10 };
+ expect(newDiffs.beta).to.deep.equal(expectedBeta);
+ });
+
+ it('does not set new state when props are the same', () => {
+ const currentDiff = wrapper.instance().state.diffs;
+ wrapper.instance().componentWillReceiveProps(props);
+ // Check equal references
+ expect(wrapper.instance().state.diffs).to.equal(currentDiff);
+ });
+
+ it('renders a ModalTrigger', () => {
+ expect(wrapper.find(ModalTrigger)).to.have.lengthOf(1);
+ });
+
+ describe('renderTriggerNode', () => {
+ it('renders a TooltipWrapper', () => {
+ const triggerNode = shallow(<div>{wrapper.instance().renderTriggerNode()}</div>);
+ expect(triggerNode.find(TooltipWrapper)).to.have.lengthOf(1);
+ });
+ });
+
+ describe('renderModalBody', () => {
+ it('renders a Table', () => {
+ const modalBody = shallow(<div>{wrapper.instance().renderModalBody()}</div>);
+ expect(modalBody.find(Table)).to.have.lengthOf(1);
+ });
+
+ it('renders a Thead', () => {
+ const modalBody = shallow(<div>{wrapper.instance().renderModalBody()}</div>);
+ expect(modalBody.find(Thead)).to.have.lengthOf(1);
+ });
+
+ it('renders Th', () => {
+ const modalBody = shallow(<div>{wrapper.instance().renderModalBody()}</div>);
+ const th = modalBody.find(Th);
+ expect(th).to.have.lengthOf(3);
+ ['control', 'before', 'after'].forEach((v, i) => {
+ expect(th.get(i).props.column).to.equal(v);
+ });
+ });
+
+ it('renders the correct number of Tr', () => {
+ const modalBody = shallow(<div>{wrapper.instance().renderModalBody()}</div>);
+ const tr = modalBody.find(Tr);
+ expect(tr).to.have.lengthOf(7);
+ });
+
+ it('renders the correct number of Td', () => {
+ const modalBody = shallow(<div>{wrapper.instance().renderModalBody()}</div>);
+ const td = modalBody.find(Td);
+ expect(td).to.have.lengthOf(21);
+ ['control', 'before', 'after'].forEach((v, i) => {
+ expect(td.get(i).props.column).to.equal(v);
+ });
+ });
+ });
+
+ describe('renderRows', () => {
+ it('returns an array of rows with one Tr and three Td', () => {
+ const rows = wrapper.instance().renderRows();
+ expect(rows).to.have.lengthOf(7);
+ const fakeRow = shallow(<div>{rows[0]}</div>);
+ expect(fakeRow.find(Tr)).to.have.lengthOf(1);
+ expect(fakeRow.find(Td)).to.have.lengthOf(3);
+ });
+ });
+
+ describe('formatValue', () => {
+ it('returns "N/A" for undefined values', () => {
+ expect(wrapper.instance().formatValue(undefined, 'b')).to.equal('N/A');
+ });
+
+ it('returns "null" for null values', () => {
+ expect(wrapper.instance().formatValue(null, 'b')).to.equal('null');
+ });
+
+ it('returns "Max" and "Min" for BoundsControl', () => {
+ expect(wrapper.instance().formatValue([5, 6], 'y_axis_bounds')).to.equal(
+ 'Min: 5, Max: 6',
+ );
+ });
+
+ it('returns stringified objects for CollectionControl', () => {
+ const value = [{ 1: 2, alpha: 'bravo' }, { sent: 'imental', w0ke: 5 }];
+ const expected = '{"1":2,"alpha":"bravo"}, {"sent":"imental","w0ke":5}';
+ expect(wrapper.instance().formatValue(value, 'column_collection')).to.equal(expected);
+ });
+
+ it('returns boolean values as string', () => {
+ expect(wrapper.instance().formatValue(true, 'b')).to.equal('true');
+ expect(wrapper.instance().formatValue(false, 'b')).to.equal('false');
+ });
+
+ it('returns Array joined by commas', () => {
+ const value = [5, 6, 7, 8, 'hello', 'goodbye'];
+ const expected = '5, 6, 7, 8, hello, goodbye';
+ expect(wrapper.instance().formatValue(value)).to.equal(expected);
+ });
+
+ it('stringifies objects', () => {
+ const value = { 1: 2, alpha: 'bravo' };
+ const expected = '{"1":2,"alpha":"bravo"}';
+ expect(wrapper.instance().formatValue(value)).to.equal(expected);
+ });
+
+ it('does nothing to strings and numbers', () => {
+ expect(wrapper.instance().formatValue(5)).to.equal(5);
+ expect(wrapper.instance().formatValue('hello')).to.equal('hello');
+ });
+
+ it('returns "[]" for empty filters', () => {
+ expect(wrapper.instance().formatValue([], 'filters')).to.equal('[]');
+ });
+
+ it('correctly formats filters with array values', () => {
+ const filters = [
+ { col: 'a', op: 'in', val: ['1', 'g', '7', 'ho'] },
+ { col: 'b', op: 'not in', val: ['hu', 'ho', 'ha'] },
+ ];
+ const expected = 'a in [1, g, 7, ho], b not in [hu, ho, ha]';
+ expect(wrapper.instance().formatValue(filters, 'filters')).to.equal(expected);
+ });
+
+ it('correctly formats filters with string values', () => {
+ const filters = [
+ { col: 'a', op: '==', val: 'gucci' },
+ { col: 'b', op: 'LIKE', val: 'moshi moshi' },
+ ];
+ const expected = 'a == gucci, b LIKE moshi moshi';
+ expect(wrapper.instance().formatValue(filters, 'filters')).to.equal(expected);
+ });
+ });
+});
diff --git a/superset/utils.py b/superset/utils.py
index 74d8dfb..f3f1a60 100644
--- a/superset/utils.py
+++ b/superset/utils.py
@@ -684,10 +684,40 @@ def merge_extra_filters(form_data):
'__time_origin': 'druid_time_origin',
'__granularity': 'granularity',
}
+ # Grab list of existing filters 'keyed' on the column and operator
+
+ def get_filter_key(f):
+ return f['col'] + '__' + f['op']
+ existing_filters = {}
+ for existing in form_data['filters']:
+ existing_filters[get_filter_key(existing)] = existing['val']
for filtr in form_data['extra_filters']:
- if date_options.get(filtr['col']): # merge date options
+ # Pull out time filters/options and merge into form data
+ if date_options.get(filtr['col']):
if filtr.get('val'):
form_data[date_options[filtr['col']]] = filtr['val']
- else:
- form_data['filters'] += [filtr] # merge col filters
+ elif filtr['val'] and len(filtr['val']):
+ # Merge column filters
+ filter_key = get_filter_key(filtr)
+ if filter_key in existing_filters:
+ # Check if the filter already exists
+ if isinstance(filtr['val'], list):
+ if isinstance(existing_filters[filter_key], list):
+ # Add filters for unequal lists
+ # order doesn't matter
+ if (
+ sorted(existing_filters[filter_key]) !=
+ sorted(filtr['val'])
+ ):
+ form_data['filters'] += [filtr]
+ else:
+ form_data['filters'] += [filtr]
+ else:
+ # Do not add filter if same value already exists
+ if filtr['val'] != existing_filters[filter_key]:
+ form_data['filters'] += [filtr]
+ else:
+ # Filter not found, add it
+ form_data['filters'] += [filtr]
+ # Remove extra filters from the form data since no longer needed
del form_data['extra_filters']
diff --git a/tests/utils_tests.py b/tests/utils_tests.py
index 5096f80..dc6b454 100644
--- a/tests/utils_tests.py
+++ b/tests/utils_tests.py
@@ -110,6 +110,89 @@ class UtilsTestCase(unittest.TestCase):
merge_extra_filters(form_data)
self.assertEquals(form_data, expected)
+ def test_merge_extra_filters_ignores_empty_filters(self):
+ form_data = {'extra_filters': [
+ {'col': 'a', 'op': 'in', 'val': ''},
+ {'col': 'B', 'op': '==', 'val': []},
+ ]}
+ expected = {'filters': []}
+ merge_extra_filters(form_data)
+ self.assertEquals(form_data, expected)
+
+ def test_merge_extra_filters_ignores_equal_filters(self):
+ form_data = {
+ 'extra_filters': [
+ {'col': 'a', 'op': 'in', 'val': 'someval'},
+ {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
+ ],
+ 'filters': [
+ {'col': 'a', 'op': 'in', 'val': 'someval'},
+ {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
+ ],
+ }
+ expected = {'filters': [
+ {'col': 'a', 'op': 'in', 'val': 'someval'},
+ {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
+ ]}
+ merge_extra_filters(form_data)
+ self.assertEquals(form_data, expected)
+
+ def test_merge_extra_filters_merges_different_val_types(self):
+ form_data = {
+ 'extra_filters': [
+ {'col': 'a', 'op': 'in', 'val': ['g1', 'g2']},
+ {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
+ ],
+ 'filters': [
+ {'col': 'a', 'op': 'in', 'val': 'someval'},
+ {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
+ ],
+ }
+ expected = {'filters': [
+ {'col': 'a', 'op': 'in', 'val': 'someval'},
+ {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
+ {'col': 'a', 'op': 'in', 'val': ['g1', 'g2']},
+ ]}
+ merge_extra_filters(form_data)
+ self.assertEquals(form_data, expected)
+ form_data = {
+ 'extra_filters': [
+ {'col': 'a', 'op': 'in', 'val': 'someval'},
+ {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
+ ],
+ 'filters': [
+ {'col': 'a', 'op': 'in', 'val': ['g1', 'g2']},
+ {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
+ ],
+ }
+ expected = {'filters': [
+ {'col': 'a', 'op': 'in', 'val': ['g1', 'g2']},
+ {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
+ {'col': 'a', 'op': 'in', 'val': 'someval'},
+ ]}
+ merge_extra_filters(form_data)
+ self.assertEquals(form_data, expected)
+
+ def test_merge_extra_filters_adds_unequal_lists(self):
+ form_data = {
+ 'extra_filters': [
+ {'col': 'a', 'op': 'in', 'val': ['g1', 'g2', 'g3']},
+ {'col': 'B', 'op': '==', 'val': ['c1', 'c2', 'c3']},
+ ],
+ 'filters': [
+ {'col': 'a', 'op': 'in', 'val': ['g1', 'g2']},
+ {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
+ ],
+ }
+ expected = {'filters': [
+ {'col': 'a', 'op': 'in', 'val': ['g1', 'g2']},
+ {'col': 'B', 'op': '==', 'val': ['c1', 'c2']},
+ {'col': 'a', 'op': 'in', 'val': ['g1', 'g2', 'g3']},
+ {'col': 'B', 'op': '==', 'val': ['c1', 'c2', 'c3']},
+ ]}
+ merge_extra_filters(form_data)
+ self.assertEquals(form_data, expected)
+
def test_datetime_f(self):
self.assertEquals(
datetime_f(datetime(1990, 9, 21, 19, 11, 19, 626096)),
@@ -119,7 +202,9 @@ class UtilsTestCase(unittest.TestCase):
self.assertEquals(datetime_f(None), '<nobr>None</nobr>')
iso = datetime.now().isoformat()[:10].split('-')
[a, b, c] = [int(v) for v in iso]
- self.assertEquals(datetime_f(datetime(a, b, c)), '<nobr>00:00:00</nobr>')
+ self.assertEquals(
+ datetime_f(datetime(a, b, c)), '<nobr>00:00:00</nobr>',
+ )
def test_json_encoded_obj(self):
obj = {'a': 5, 'b': ['a', 'g', 5]}
--
To stop receiving notification emails like this one, please contact
['"commits@superset.apache.org" <co...@superset.apache.org>'].