You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@superset.apache.org by gi...@git.apache.org on 2017/10/18 01:34:24 UTC
[GitHub] Mogball closed pull request #3668: [Explore] Altered Slice Tag
Mogball closed pull request #3668: [Explore] Altered Slice Tag
URL: https://github.com/apache/incubator-superset/pull/3668
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/superset/assets/javascripts/components/AlteredSliceTag.jsx b/superset/assets/javascripts/components/AlteredSliceTag.jsx
new file mode 100644
index 0000000000..eb24424e8d
--- /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/ChartContainer.jsx b/superset/assets/javascripts/explore/components/ChartContainer.jsx
index f3c660adf6..f74a263af7 100644
--- a/superset/assets/javascripts/explore/components/ChartContainer.jsx
+++ b/superset/assets/javascripts/explore/components/ChartContainer.jsx
@@ -8,6 +8,7 @@ import visMap from '../../../visualizations/main';
import { d3format } from '../../modules/utils';
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';
@@ -225,6 +226,13 @@ class ChartContainer extends React.PureComponent {
</div>);
}
+ renderAlteredTag() {
+ const origFormData = (this.props.slice && this.props.slice.form_data) || {};
+ const currentFormData = this.props.formData;
+ const tagProps = { origFormData, currentFormData };
+ return (<AlteredSliceTag {...tagProps} />);
+ }
+
renderChart() {
if (this.props.alert) {
return this.renderAlert();
@@ -296,6 +304,8 @@ class ChartContainer extends React.PureComponent {
</span>
}
+ {this.renderAlteredTag()}
+
<div className="pull-right">
{this.props.chartStatus === 'success' &&
this.props.queryResponse &&
diff --git a/superset/assets/package.json b/superset/assets/package.json
index 3dfdb78240..952d494445 100644
--- a/superset/assets/package.json
+++ b/superset/assets/package.json
@@ -93,6 +93,7 @@
"sprintf-js": "^1.1.1",
"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 0000000000..f3b746bbb3
--- /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 fa72971b70..2a68d02bbc 100644
--- a/superset/utils.py
+++ b/superset/utils.py
@@ -686,10 +686,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 b642c4374a..3a9add30e2 100644
--- a/tests/utils_tests.py
+++ b/tests/utils_tests.py
@@ -117,14 +117,102 @@ def test_merge_extra_filters(self):
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)),
- '<nobr>1990-09-21T19:11:19.626096</nobr>')
+ self.assertEquals(
+ datetime_f(datetime(1990, 9, 21, 19, 11, 19, 626096)),
+ '<nobr>1990-09-21T19:11:19.626096</nobr>'
+ )
self.assertEquals(len(datetime_f(datetime.now())), 28)
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]}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services